Support full function-like macro expansions and allow for whitespace in PP directives #350
Conversation
for more information, see https://pre-commit.ci
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #350 +/- ##
==========================================
+ Coverage 87.51% 87.70% +0.19%
==========================================
Files 35 35
Lines 4756 4791 +35
==========================================
+ Hits 4162 4202 +40
+ Misses 594 589 -5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thank you for your efforts on this.
After review, I have a few suggestions:
- It appears there are some style changes in the diffs not essential to this PR.
Could we revert these to maintain consistency with the original code? - Regarding the REGEX modifications, I've observed instances of unnecessary group
captures and alterations in expression matching that may introduce side effects.
I recommend we take another look at these to ensure accuracy and reliability. - I've attempted to provide a more Pythonic approach for certain additions
to the code, aiming for clarity and conciseness.
Overall, great work. With a few adjustments based on the feedback,
I believe this will be ready for merge. Looking forward to the final touches.
|
|
||
| out_line = text | ||
| out_line = replace_defined(out_line) |
There was a problem hiding this comment.
AFAIK these changes can be reverted
| @@ -2038,6 +2043,7 @@ def replace_ops(expr: str): | |||
| expr = expr.replace("!=", " <> ") | |||
| expr = expr.replace("!", " not ") | |||
| expr = expr.replace(" <> ", " != ") | |||
|
|
|||
There was a problem hiding this comment.
I think this is a remainder from cherry-picking, o need for this diff.
There was a problem hiding this comment.
I think this test is redundant and does not perform any additional checks that are not covered by the ewrite() function macro e.g.
"```fortran90\n#define ewrite(priority, format)"
" if (priority <= 3) write((priority), format)\n```"
Am I missing something?
| def append_multiline_macro(pp_defs: dict, def_name: str, line: str): | ||
| def_value = pp_defs[def_name] | ||
| def_args = None | ||
| if isinstance(def_value, tuple): | ||
| def_args, def_value = def_value | ||
|
|
||
| def_value += line | ||
|
|
||
| if def_args is not None: | ||
| def_value = (def_args, def_value) | ||
|
|
||
| pp_defs[def_name] = def_value |
There was a problem hiding this comment.
| def append_multiline_macro(pp_defs: dict, def_name: str, line: str): | |
| def_value = pp_defs[def_name] | |
| def_args = None | |
| if isinstance(def_value, tuple): | |
| def_args, def_value = def_value | |
| def_value += line | |
| if def_args is not None: | |
| def_value = (def_args, def_value) | |
| pp_defs[def_name] = def_value | |
| def append_multiline_macro(def_value: str | tuple, line: str): | |
| def_args = None | |
| if isinstance(def_value, tuple): | |
| def_args, def_value = def_value | |
| def_value += line | |
| if def_args is not None: | |
| return (def_args, def_value) | |
| return def_value |
|
I will have another pass at the added python code once the diffs clearup a bit. |
|
I have taken the commits from this PR + the suggestions and merged them to #368. The PR just got merged in master, so I am closing this. Thanks for the code @emanspeaks . I will be issuing a major release soon. |
This is a refactor of #341 to separate out the vendor-agnostic changes. (See earlier conversation there for some context.)
Closes #297
Addresses spaces on either side of the hash in preprocessor lines.
Additionally, provides more robust handling of define directives as well as support for function-like macros inherited from C-style preprocessors.