Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't compress macro names with their definitions #6

Open
catb0t opened this issue Jun 16, 2017 · 2 comments
Open

Don't compress macro names with their definitions #6

catb0t opened this issue Jun 16, 2017 · 2 comments

Comments

@catb0t
Copy link

catb0t commented Jun 16, 2017

I'm currently writing a patch for this and I'll create a pull request tomorrow, but basically this:

#define A ((uint8_t) 3)

is turned into

#define A((uint8_t)3)

which is probably not what you wanted

And this:

#define a(c, x) bv(c, x)

becomes

#define a(c,x)bv(c,x)

which is almost certainly not what you want

the patch is almost complete, i just need to clean up the fix a little and write tests.

@catb0t catb0t changed the title Don't compress macros names with their definitions Don't compress macro names with their definitions Jun 16, 2017
@catb0t
Copy link
Author

catb0t commented Jun 16, 2017

Nevermind, I can't wrap my head around the recursivity at play here.

This is the patch I started writing. Unfortunately, though it appears to fix the mentioned problems in the first pass, the whitespace is again removed by another pass of minify_operator(op)(line).

The fix requires a restructure of the minification steps themselves. If you ever get around to that, it'd probably be best to just rewrite it in Python 3 anyways.

If it seems like this duplicates the work that fix_unary_operators does for macros, that might be the case except that function doesn't work, as illustrated in my original comment.

diff --git a/minifier.py b/minifier.py
index 624210e..ad31090 100755
--- a/minifier.py
+++ b/minifier.py
@@ -83,7 +83,31 @@ def minify_operator(op):
     repl = op
     if op in SPACED_OPS:
         repl += " "
-    return lambda string: regex.sub(repl, string)
+    def replace_function(string):
+        macro_parts = is_simple_macro(string)
+        if macro_parts:
+            macro, name, val = macro_parts.groups()
+            # print "simple", name, val
+            # if the last part exists we can minify it too
+            if val is not None:
+                for op in OPS:
+                    if op in val:
+                        val = minify_operator(op)(val)
+            # val might not exist so concat empty string
+            return macro + " " + name + " " + (val or "") + "\n"
+
+        flmd = is_function_like_macro(string)
+        if flmd:
+            macro, name, val = flmd.groups()
+            print "function", name, val
+            for op in OPS:
+                if op in name: name = minify_operator(op)(name)
+                if op in val:  val = minify_operator(op)(val)
+            print "fmin", name, val
+            return macro + " " + name + " " + val + "\n"
+
+        return regex.sub(repl, string)
+    return replace_function
 
 
 def fix_spaced_ops(minified_txt):
@@ -144,6 +168,23 @@ def reinsert_preprocessor_newlines(lines):
 def is_preprocessor_directive(line):
     return line.startswith(PREPROCESSOR_TOKEN)
 
+def is_simple_macro(line):
+    macro_parts = re.compile(
+        r"^({}define)\s+([a-z0-9_]+)(?:\s+(.*?)\s+)?$"
+            .format(PREPROCESSOR_TOKEN),
+        re.IGNORECASE
+    )
+    result = macro_parts.match(line)
+    return result if (result is not None) else False
+
+def is_function_like_macro(line):
+    flmd = re.compile(
+        r"^({}define)\s+([a-z0-9_]+\(\s*[a-z0-9_]+(?:,\s+[a-z0-9_]+)\s*\))(?:\s+(.*)\s+)?$"
+            .format(PREPROCESSOR_TOKEN),
+        re.IGNORECASE
+    )
+    result = flmd.match(line)
+    return result if (result is not None) else False
 
 def minify_source(orig_source, args=None):
     """

@catb0t
Copy link
Author

catb0t commented Jun 16, 2017

And by the way, the best, simplest actual fix for this, and to get working C code since the byte savings from minifying macros is so little, is to change minify_operator to end in this line:

return lambda string: string if is_preprocessor_directive(string) else regex.sub(repl, string)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant