[PATCH] D125622: [clang-tidy] Reject invalid enum initializers in C files

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.c:10
+// C forbids comma operator in initializing expressions.
+#define BAD_OP 1, 2
+
----------------
LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > Can you add a test:
> > ```
> > #define GOOD_OP (1, 2)
> > ```
> > to make sure it still gets converted to an enum?
> Yeah, another one I was thinking of is when someone does [[ https://godbolt.org/z/c677je8s1 | something as disgusting as this. ]]
> 
> ```
> #define ARGS 1, 2
> #define OTHER_ARGS (1, 2)
> 
> int f(int x, int y) { return x + y; }
> 
> int main()
> {
>     return f(ARGS) + f OTHER_ARGS;
> }
> ```
> 
> However, the only real way to handle avoiding conversion of the 2nd case is to examine the context of macro expansion.  This is another edge case that will have to be handled subsequently.
> 
> This gets tricky because AFAIK there is no way to select expressions in the AST that result from macro expansion.  You have to match the macro expansion locations against AST nodes to identify the node(s) that match the expansion location yourself.
That's a good test case (for some definition of good)! :-)

At this point, there's a few options:

1) Go back to the way things were -- disallow comma expressions, even in parens. Document it as a limitation.
2) Accept comma expressions in parens, ignore the problem case like you described above. Document it as a limitation?
3) Try to solve every awful code construct we can ever imagine. Document the check is perfect in every way, but probably not land it for several years.

I think I'd be happy with #2 but I could also live with #1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125622