aaron.ballman added a comment.
Note, I'm in C standards committee meetings all week this week, so I expect I
won't be doing many reviews this week and I'll be catching up as best I can
starting next week.
================
Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:324
{
- IntegralLiteralExpressionMatcher Matcher(MacroTokens);
- return Matcher.match();
+ IntegralLiteralExpressionMatcher Matcher(MacroTokens, LangOpts.C99 == 0);
+ bool Matched = Matcher.match();
----------------
Huh? Comma wasn't allowed in C89 either... In fact, there's no language mode
which allows top-level commas in either C or C++ -- the issue with the comma
operator is a parsing one. You can't tell the difference between the comma
being part of the initializer expression or the comma being the separator
between enumerators.
================
Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:326
+ bool Matched = Matcher.match();
+ if (LangOpts.C99 &&
+ (Matcher.largestLiteralSize() != LiteralSize::Int &&
----------------
And C89?
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.c:1
+// RUN: %check_clang_tidy %s modernize-macro-to-enum %t
+
----------------
It'd be useful to run this test in C++ mode as well to demonstrate the
behavioral differences.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.c:3-7
+// C requires enum values to fit into an int.
+#define TOO_BIG1 1L
+#define TOO_BIG2 1UL
+#define TOO_BIG3 1LL
+#define TOO_BIG4 1ULL
----------------
How do we want to handle the fact that Clang has an extension to allow this?
Perhaps we want a config option for pedantic vs non-pedantic fixes?
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.c:9
+
+// C forbids comma operator in initializing expressions.
+#define BAD_OP 1, 2
----------------
It's also forbidden in C++.
================
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
+
----------------
Can you add a test:
```
#define GOOD_OP (1, 2)
```
to make sure it still gets converted to an enum?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125622/new/
https://reviews.llvm.org/D125622
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
