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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
