[PATCH] D124066: [clang-tidy] Ignore macros defined within declarations in modernize-macro-to-enum

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 22 12:07:18 PDT 2022


aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

In D124066#3463109 <https://reviews.llvm.org/D124066#3463109>, @LegalizeAdulthood wrote:

> In D124066#3463008 <https://reviews.llvm.org/D124066#3463008>, @aaron.ballman wrote:
>
>> This seems like a case where we might want a configuration option (maybe). [...]
>> WDYT?
>
> In my bug report on this problem, I sketch out a plan of attack:
>
> 1. **DON'T BREAK MY CODE** -- that is this review `:)`
> 2. Do some analysis of macro expansion locations to determine the nearest enclosing scope at which the enum should be declared.
>
> https://github.com/llvm/llvm-project/issues/54883

Ah, thank you for the explanation. I like not breaking code. :-) LGTM aside from a possible simplification for the matchers.



================
Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:561-567
+  Finder->addMatcher(varDecl(TopLevelDecl).bind("top"), this);
+  Finder->addMatcher(functionDecl(TopLevelDecl).bind("top"), this);
+  Finder->addMatcher(recordDecl(TopLevelDecl).bind("top"), this);
+  Finder->addMatcher(typeAliasDecl(TopLevelDecl).bind("top"), this);
+  Finder->addMatcher(functionTemplateDecl(TopLevelDecl).bind("top"), this);
+  Finder->addMatcher(classTemplateDecl(TopLevelDecl).bind("top"), this);
+  Finder->addMatcher(typeAliasTemplateDecl(TopLevelDecl).bind("top"), this);
----------------
Can we mix these together with `anyOf()` so we're not adding so many matchers? e.g.,
```
Finder->addMatcher(anyOf(varDecl(TopLevelDecl), functionDecl(topLevelDecl()), ...).bind("top"), this);
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124066/new/

https://reviews.llvm.org/D124066



More information about the cfe-commits mailing list