[clang-tidy] Add modernize-macro-to-enum check

[LegalizeAdulthood]

Summary

This check performs basic analysis of macros and replaces them
with an anonymous unscoped enum. Using an unscoped anonymous enum
ensures that everywhere the macro token was used previously, the
enumerator name may be safely used.

Potential macros for replacement must meet the following constraints:

• Macros must expand only to integral literal tokens.
• Macros must be defined on sequential source file lines, or with only comment lines in between macro definitions.
• Macros must all be defined in the same source file.
• Macros must not be defined within a conditional compilation block.
• Macros must not be defined adjacent to other preprocessor directives.
• Macros must not be used in preprocessor conditions

Each cluster of macros meeting the above constraints is presumed to
be a set of values suitable for replacement by an anonymous enum.
From there, a developer can give the anonymous enum a name and
continue refactoring to a scoped enum if desired. Comments on the
same line as a macro definition or between subsequent macro definitions
are preserved in the output. No formatting is assumed in the provided
replacements.

Fixes #27408

[LegalizeAdulthood]

RFC: Should we create a cppcoreguidelines alias since this implements
"Enum.1: Prefer enumerations over macros"?

This check tries to be very conservative so as to not generate false
positives or invalid code.

[njames93]

This comment has been deleted.

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

107

117

Make this a switch?

135

Is there any practical reason for these to be defined outline?

148–149

No need to create a std::string here, just keep it as a StringRef.

183

This ConditionScope checks looks like it would prevent warning in header files that use a header guard(instead of pragma once) to prevent multiple inclusion.

193–195

This seems a strange way to decect changes in file when you can just override the FileChanged callback.

213–219

Probably a syntax error there, but something like that would be much more readable.

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h

14

IWYU Violation

20

FIXME

clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

8–16

These checks aren't needed as notes are implicitly ignored by the check_clang_tidy script.
Same goes for below

38–41

Not essential for this to go in, but it would be nice if we could detect the longest common prefix for all items in a group and potentially use that to name the enum. This would only be valid if the generated name is not currently a recognised token,

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h

20

Please do this :-)

clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp

63

Please address this.

clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst

7

Please make first statement same as in Release Notes. This check should be omitted.

46

It'll be reasonable to support indentation. Option could be used to specify string (tab or several spaces).

[njames93]

clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst

46

That's not actually necessary as clang-tidy runs clang-format on the fixes it generates

[LegalizeAdulthood]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

135

Is there any practical reason for it to be inline?

I don't like making large inline functions. It's my understanding that
the compiler may inline small "outlined" functions anyway, where it
has a better idea of what "small" means.

183

Oh, good catch, you're probably right. I'll test that manually.

(Gee, another case where we need check_clang_tidy.py to validate
changes to header files! This keeps coming up! My implementation
of this from several years ago died in review hell and was never born.)

193–195

I'll try that. Does it fire once at the beginning?

213–219

I was unaware of llvm::erase, presumably it's a range-like algo that
does the boiler plate of begin(), end(), std::remove_if and
std::vector<T>::erase?

Looks like I need to switch to llvm::SmallVector for that to work.
I'll try that.

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h

14

Good catch. This is a leftover when I had more methods declared
on the Check class.

20

Oops :)

clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp

63

Yep, I didn't clang-format this file after add_new_check ran.

clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst

46

It can optionally do this, but it is off by default.

clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

38–41

Yeah, for this example that would yield a reasonable enum
name of CoordMode, but many macros have acronym prefixes
and they wouldn't yield useful names, e.g. WM_COMMAND
would get a name like WM which isn't particularly useful and
may cause other problems.

IMO, introducing names should be a step that's invoked
manually by the developer.

I intend to write another check that migrates a named, unscoped
enum to a scoped enum. Once everything has been lifted to an
enum from macros, then I can use matchers to locate the enumerators
and perform usage checks to make sure that lifting the enumerator
to a scoped identifier doesn't create invalid code. At such time, the
common prefix of the enumerators would be stripped to be replaced
with the scoped name.

[LegalizeAdulthood]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

193–195

Tests continue to pass after switching to FileChanged, so looks good.
I'll double check in the debugger.

[LegalizeAdulthood]

Addresses most of the review comments, still need to:

• Verify that include guards don't interfere with analysis of headers

[LegalizeAdulthood]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

183

Any ideas for an algorithm that detects a header guard condition
from some other condition? I don't see anything obvious.

[LegalizeAdulthood]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

183

So I created a little state machine and that covers my test cases,
but I worry that it might be too fragile, so I'm open to suggestions
for improvement on my state machine algorithm and/or test cases
that could break the algorithm.

[LegalizeAdulthood]

• Handle macros inside header files with include guards
• Track state per-file and push/pop that state as files are included

[LegalizeAdulthood]

• clang-format
• prefer llvm::any_of over std::any_of

[LegalizeAdulthood]

• Tweak documentation, make sure sphinx runs without errors

[LegalizeAdulthood]

Testing on iterated dynamics revealed a number of shortcomings;
update the code to address those:

• Improve handling of include guards
• Handle conditional compilation blocks better
• Reject macros that expand to floating point literals
• Start a new enum when macros have intervening text

[LegalizeAdulthood]

Switch back to SmallVector after debugging with std::vector
Drop unnecessary includes

[LegalizeAdulthood]

Update from additional testing:

• reject floating-point literals (they can't be enumerations)
• add more test cases

[LegalizeAdulthood]

clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

2

Ah, this leaked out from incremental testing I was doing;
it doesn't actually require C++14 or later (although it did
at one time while I was working on rejecting floating-point
literals.)

[LegalizeAdulthood]

Remove -std for test

[LegalizeAdulthood]

This situation isn't properly diagnosed (false positive):

#ifdef USE_FOO
#if USE_FOO
// code
#endif
#endif

Eh, never mind this comment it's unrelated to this check :)

[LegalizeAdulthood]

Ping.

This check has gone for a week without review comments.

[carlosgalvezp]

Since this is a modernize check, shouldn't it be proposing to use enum class instead of enum? This will conflict with Enum.3 from cppcoreguidelines, and I don't think it's unreasonable that people enable both modernize* and cppcoreguidelines*. Not sure if such a check for Enum.3 exists today but it could easily be added in the future.

I understand that the fix perhaps cannot be automated just as easily, but maybe it's OK? Or at least make the default use enum class and leave it as an option to use unscoped enums to be able to apply the fix. Still I find it a bit inconsistent with the rest of the modernize module (modernize as in "use post-C++11 stuff").

Another point is that macros typically have a different naming convention than enums or constants, so users will likely have to do manual work anyway, or rely on the "readability-identifier-naming" check - how does it play out then if 2 checks propose different fixes?

[LegalizeAdulthood]

I think it helps to understand this check better if we consider what
we would do manually if we were modernizing code with a lot of
macro-style enums in it. I've done this myself; in fact all the checks
I've written for clang-tidy have been motivated by modernizing dusty
deck C code that was converted to C++.

The motivating example is on github and you can find my manual
refactorings in the commit history.

A simple behavior preserving first pass is to hoist all integral constant
macros to anonymous unscoped enums. A second pass is to examine
related anonymous unscoped enums and hoist them into named unscoped
enums. If they are used only as discriminating values, e.g. not used in
arithmetic expressions, then they can be hoisted to a named scoped
enum and you can "lean on the compiler" to adjust the relevant
declarations of variables and function parameters.

This check implements the first pass. My intention is to create another
check that assists in the "hoist a named unscoped enum to a named
scoped enum" which a) removes any common enumeration prefix from
all the enumerations (since they are now uniquely identified by scope),
b) adds the necessary scope qualifiers to all uses of the enumerations,
c) propagates the necessary declaration changes to all variables and
parameters. Obviously such a check involves quite a bit of analysis
of all the usages of an enumeration and may not even be feasible in
the context of clang-tidy since it can only examine a single translation
unit at a time, but this is the goal.

Note that this still leaves humans in the middle deciding which clumps
of anonymous unscoped enums represent semantically related items
and giving them an appropriate name.

In D117522#3290604, @carlosgalvezp wrote:

Since this is a modernize check, shouldn't it be proposing to use enum class instead of enum?

One step at a time :). In short, the answer is no because of the
way preprocessor macros interact with the rest of the code. While
it is safe to hoist the macro into an unscoped enum without analyzing
all the macro expansion sites, the same can't be said for a scoped
enum.

First, we would have to determine a name for the scoped enum. This
may not be possible, depending on the macros. For instance some
macros are really just named constants, not enumerations, but we
can't tell from the macro itself. (There are heuristics you can apply,
and I have done some of that in this check, but the only way to know
for certain whether a macro is a named constant or intended to be
a set of related names is manual inspection.)

Second, this check can apply to C source files as well as C++ source
files. C doesn't have scoped enums, but it does have enums and they
can be anonymous.

This will conflict with Enum.3 from cppcoreguidelines,

You're correct that both this check and cppcoreguidelines-macro-usage
will make suggestions for the same macro. Again, the problem with
a macro is that it's hard to identify which is an enum and which is a
named constant.

If I have a block of macros that represent enumerations, I'm not going
to be happy with cppcoreguidelines-macro-usage suggesting that
they all be turned into constexpr declarations.

If I have a block of macros that represent named constants, I'm not
going to be happy with modernize-macro-to-enum suggesting that
they all be turned into anonymous enum declarations.

I don't think there's any clear winner over which should be chosen by
an automated tool.

If it were me, I'd apply modernize-macro-to-enum first and then
cppcoreguidelines-macro-usage. Why? Because I'd then have enums
to scoop up all the integral constants, which is a more specific language
construct, and then I'd have constexpr constants for the macros that
expand to floating-point and string values. I'd still be left with function-
like macros that macro-usage warned me about and didn't fix.

Or at least make the default use enum class and leave it as an
option to use unscoped enums to be able to apply the fix.

An additional problem with scoped enums, besides having to invent
a name for it, is that you have to analyze all the expansion sites of the
defined macros in order to determine if the macro expands into a value
that is used in an expression.

Consider this code:

#define FOO_ARG1 0x1
#define FOO_ARG2 0x2
#define FOO_ARG3 0x3

void f()
{
  int options = FOO_ARG1 | FOO_ARG3;
  g(options);
}

If I replace the macros with a scoped enum, now I've created a compilation
error at the line initializing options and I've created a compilation error
at the line calling g(). An anonymous unscoped enum doesn't suffer from
these problems.

Still I find it a bit inconsistent with the rest of the modernize module
(modernize as in "use post-C++11 stuff").

There is a lot of legacy C-isms in C++ code and not all modernization
transformations imply a shift to C++11. For instance, another modernizing
checker that I wrote -- modernize-redundant-void-arg -- applies to C++98
code.

Another point is that macros typically have a different naming convention
than enums or constants, so users will likely have to do manual work anyway,
or rely on the "readability-identifier-naming" check - how does it play out
then if 2 checks propose different fixes?

The difficulty of naming is why this check doesn't propose names. They have
to be chosen by humans. The readability check imposes a naming convention
on the identifiers but it doesn't impute semantics to those identifiers, it simply
checks that they follow a certain lexicographical convention.

[LegalizeAdulthood]

Oh, I think I misinterpreted what you were saying in the last bit:

In D117522#3290604, @carlosgalvezp wrote:

Another point is that macros typically have a different naming
convention than enums or constants, so users will likely have
to do manual work anyway, or rely on the
"readability-identifier-naming" check - how does it play out
then if 2 checks propose different fixes?

All the checks look at the original version of the source file, so if
you run modernize-macro-to-enum and readability-identifier-naming
checks at the same time, they won't conflict with each other.
You're correct that once a macro has been hoisted to an
anonymous enum, running the identifier naming check on the
transformed result might then complain about the names being
stylized as a macro and not as an enum. Since the identifier
naming check provides fix-its, I don't see this as a problem.
If you want the was-a-macro-now-is-an-enum name to follow
a different identifier naming convention, it makes sense to run
the one check after the other in order to enforce this. IMO, it's
better to have small, discrete, well-defined transformation steps
than it is to have a "kitchen sink" transformation that attempts to
do too much.

However, I've seen many coding style guidelines (current workplace
is one example) that explicitly say to name enumerations with the
same identifier convention as are used for macros, so this is style
guide specific.

[LegalizeAdulthood]

In D117522#3290604, @carlosgalvezp wrote:

This will conflict with Enum.3 from cppcoreguidelines

I went back and looked at Enum.3 Prefer class enums over “plain” enums,
and implementing that is the second-pass check I discussed earlier.

This check is more about implementing Enum.1 Prefer enumerations over macros.

This brings up the question of whether or not there should be a cppguidelines alias
for this check.

[carlosgalvezp]

Ok, thanks for the explanation! I'm mostly interested on the warning message, we've had situations before where the warning describes the problem and the solution, which can easily lead to confusion. From the tests I can see the message is quite generic "use an enum", so it won't push users to prefer one variant over the other.

I'd like to play a bit with the patch and see what pops in our codebase but somehow I get an error when downloading, do you happen to know what could be wrong? Alternatively if there's any other easy way to checkout the patch and test it :)

$ arc patch D117522
 Exception 
preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated
(Run with `--trace` for a full exception trace.)

This check is more about implementing Enum.1 Prefer enumerations over macros.

Should a cppcoreguidelines alias be added in that case?

[LegalizeAdulthood]

In D117522#3299423, @carlosgalvezp wrote:

I'm mostly interested on the warning message, [...]

I'm open to changes in the wording of the warning message.

I get an error when downloading, do you happen to know what could be wrong?

I haven't used arcanist, I've created the review by uploading the diff from git.

You should be able to download the raw diff and then use git apply to apply
the diff to your working tree.

I tested with a clone of [[ https://github.com/LegalizeAdulthood/iterated-dynamics | iterated dynamics ]]from github.
That code originates as multi-decade old C code which I've incrementally modernized
and has lots of integral constant macros in it. That led me to some improvements over
the first version of this that I put up for review.

This check is more about implementing Enum.1 Prefer enumerations over macros.

Should a cppcoreguidelines alias be added in that case?

Yeah, I think I'm going to add that.

[LegalizeAdulthood]

Mmm... just realized that this check should invalidate any macro
that is used in a conditional compilation expression, because enums
can't be used there.

[LegalizeAdulthood]

• Rejects macros that are used in preprocessor conditionals
• cppcoreguidelines-macro-to-enum is an alias for modernize-macro-to-enum
• Document alias

[LegalizeAdulthood]

• clang-format

[LegalizeAdulthood]

• Move small methods to class decl

[LegalizeAdulthood]

Ping. Waiting for review feedback for about a week now.

[carlosgalvezp]

I tried to download the diff and apply it from the root of llvm-project, but I must be doing something wrong...

$ git apply D117522.diff
D117522.diff:808: trailing whitespace.
                                - attempt to free an illegal 
D117522.diff:809: trailing whitespace.
                                  cmap entry 
D117522.diff:810: trailing whitespace.
                                - attempt to store into a read-only 
D117522.diff:824: trailing whitespace.
// CHECK-FIXES-NEXT:                                 - attempt to free an illegal 
D117522.diff:825: trailing whitespace.
// CHECK-FIXES-NEXT:                                   cmap entry 
error: clang-tidy/modernize/CMakeLists.txt: No such file or directory
error: clang-tidy/modernize/ModernizeTidyModule.cpp: No such file or directory
error: docs/ReleaseNotes.rst: No such file or directory
error: docs/clang-tidy/checks/list.rst: No such file or directory

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

383

Maybe wrap the raw strings inside a StringRef for a more robust and readable comparison? Not a big fan of the magic 6 there :)

[carlosgalvezp]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

390

Same here

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-to-enum.rst

8–9

Would it make sense to mention this check covers the rule Enum.1?

I see that we follow a standard pattern for aliases where we simply redirect to the main check, but maybe it's good to point this out anyway? Otherwise it might be unclear exactly which rule this check is referring to.

One drawback though is that aliases redirect to the main check after 5 seconds...

clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst

10–14

Oh, it's right here :) I suppose as a user I would expect to find this info in the cppcoreguidelines doc, not here. But again I don't know what the de-facto pattern is with aliases so I'll leave that to someone that knows better.

20

Hmm, what about this situation?

// This is some macro
#define FOO 123
// This is some other unrelated macro
#define BAR 321

Would the check group these 2 together? Personally I'd put an empty line between the two to show they are unrelated, but I know many people prefer to save vertical space and keep everything tight without empty lines.

[LegalizeAdulthood]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

383

OK, I've added some intention revealing functions that I think clean this up. I'm testing now and will upload a diff when the tests pass.

clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst

10–14

readability-magic-numbers is similar; the alias simply links to the readability check and the readability check cites the C++ Core Guideline with a link.

20

They're "grouped" into the same anonymous enum. The basic heuristic is that blank lines separate groups of related identifiers, not comment lines. This is the most common pattern in header files with macros that I've observed for decades.

[LegalizeAdulthood]

• Add intention revealing functions for details of pragma once parsing

[LegalizeAdulthood]

Ping. Another week waiting for reviews...

[aaron.ballman]

In D117522#3364387, @LegalizeAdulthood wrote:

Ping. Another week waiting for reviews...

Thanks for the ping! FWIW, it's also not uncommon for there to be week delays (reviewers go on vacation, have other obligations, etc), so hopefully the delays are not too frustrating. We do our best to review in a timely manner.

Overall, I think this is a really neat new check. One thing I think we should do is entirely ignore macros that are defined in system headers. We don't diagnose in system headers by default, but there's no reason to even do the processing work within a system header because those macros can't be changed (not only can the user not change them because it's a system header, but it's also likely that the macro is required for standards conformance). I think we can get some good compile-time performance wins from bailing on system headers, but this is speculative.

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

48

I'm a bit confused by this one as this is not a valid line ending (it's either three valid line endings or two valid line endings, depending on how you look at it). Can you explain why this is needed?

84–85

Won't this cause a problem for hex literals like 0xE?

107

Maybe not worth worrying about, but should this be a uint64_t to handle massive source files?

316

We don't seem to have any tests for literal suffixes and I *think* those will show up here as additional tokens after the literal. e.g., #define FOO +1ULL, so I think the size check here may be a problem.

319

It's worth pointing out that both of these are expressions that operate on a literal, and if we're going to allow expressions that operator on a literal, why only these two? e.g. why not allow #define FOO ~0U or #define BAR FOO + 1? Someday (not today), it would be nice for this to work on any expression that's a valid constant expression.

419

Diagnostics in clang-tidy don't start with a capital letter or have trailing punctuation.

420

I *think* you don't need to call getName() here because the diagnostics engine already knows how to handle an IdentifierInfo * (but I could be remembering wrong)

429

clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp

88–92

Unrelated formatting changes? (Feel free to land separately)

clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

68–69

What about an #undef that's not adjacent to any macros? e.g.,

#define FOO 1
#define BAR 2
#define BAZ 3

int i = 12;

#if defined(FROBBLE)
#undef FOO
#endif

I'm worried that perhaps other code elsewhere will be checking defined(FOO) perhaps in cases conditionally compiled away, and switching FOO to be an enum constant will break other configurations. To be honest, I'm a bit worried about that for all of the transformations here... and I don't know a good way to address that aside from "don't use the check". It'd be interesting to know what kind of false positive rate we have for the fixes if we ran it over a large corpus of code.

[LegalizeAdulthood]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

48

It's a state machine, where the states are named for what we've seen so far and we're looking for two consecutive line endings, not just one. Does it make sense now?

84–85

Good catch, I'll add a test.

107

I use the type returned by getSpellingLineNumber.

316

On an earlier iteration I had some tests for literals with suffixes and the suffix is included in the literal token. I can add some test cases just to make it clear what the behavior is when they are encountered.

319

A later enhancement can generalize to literal expressions (which are valid initializers for an enumeration), but I wanted to cover the most common case of simple negative integers in this first pass.

clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp

88–92

Probably, I just routinely clang-format the whole file instead of just isolated changes.

clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

68–69

Yeah, the problem arises whenever you make any changes to a header file. Did you also change all translation units that include the header? What about conditionally compiled code that was "off" in the translation unit for the automated change? Currently, we don't have a way of analyzing a group of translation units together for a cohesive change, nor do we have any way of inspecting more deeply into conditionally compiled code. Addressing those concerns is beyond the scope of this check (or any clang-tidy check) as it involves improvements to the entire infrastructure.

However, I think it is worth noting in the documentation about possible caveats. I think the way clang-tidy avoids this problem now is that you have to request fixes and the default mode is to issue warnings and leave it up to the reader as to whether or not they should apply the fixes.

I believe I already have logic to disqualify any cluster of macros where any one of them are used in a preprocessor condition (that was the last functional change I made to this check). Looks like I need to extend that slightly to include checking for macros that are #undef'ed.

[LegalizeAdulthood]

clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

68–69

OK, looks like I was already handling this, LOL. See line 135

// Undefining an enum-like macro results in the enum set being dropped.

[aaron.ballman]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

48

Thanks, I understood it was a state machine, but it's a confused one to me. \r was the line ending on Mac Classic, I've not seen it used outside of that platform (and I've not seen anyone write code for that platform in a long time). So, to me, the only valid combinations of line endings to worry about are: LF LF; CRLF CRLF; CRLF LF; LF CRLF.

LF LF returns false (Nothing -> LF -> return false)
CRLF CRLF returns false (Nothing -> CR -> CRLF -> CRLFCR -> return false)
CRLF LF returns true (Nothing -> CR -> CRLF -> LF -> finish loop)
LF CRLF returns true (Nothing -> LF -> CR -> CRLF -> finish loop)

(If you also intend to support Mac Classic line endings for some reason, this gets even more complicated.)

107

Sounds like a good enough reason to me, thanks!

316

Thanks, I'd appreciate adding the tests just to be sure we handle the case properly.

319

I'm less worried about the arbitrary constant expressions than I am about not supporting ~ because ~0U is not uncommon in macros as a way to set all bits to 1. It's certainly more common than seeing a unary +, at least in my experience. However, an even more important use case that I should have thought of first is surrounding the value in parens (which is another common idiom with macros). e.g, #define ONE (1)

Some examples of this in the wild (search the files for ~0U):

https://codesearch.isocpp.org/actcd19/main/l/linux/linux_4.15.17-1/drivers/gpu/drm/i915/gvt/handlers.c
https://codesearch.isocpp.org/actcd19/main/w/wine/wine_4.0-1/dlls/d3d8/d3d8_private.h
https://codesearch.isocpp.org/actcd19/main/q/qtwebengine-opensource-src/qtwebengine-opensource-src_5.11.3+dfsg-2/src/3rdparty/chromium/third_party/vulkan/include/vulkan/vulkan.h

(There's plenty of other examples to be found on the same site.)

I'm fine not completing the set in the initial patch, but the current behavior is a bit confusing (+ is almost of negligible importance). I think ~ and parens need to be supported; I'd prefer in this patch, but I'm fine if it comes in a subsequent patch so long as those two are supported before we release.

clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp

88–92

Please don't clang-format the whole file (it makes code reviews more difficult; we document this request a bit in https://llvm.org/docs/CodingStandards.html#introduction), there's a script for formatting just the isolated changes: https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting that I've found works really well in most cases.

clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst

46

+1 to it not being necessary, there's a command line option to reformat fixes ( --format-style=<string>).

clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

68–69

Yeah, you already have the code for handling this somewhat (that's one of the reasons why I brought this particular use case up). My greater concern is: how many false positives does this check generate on real world code? Documentation may help alleviate those concerns well enough, but if the false positive rate is sufficiently high that you basically have to disable this check for real world code, we need to do better. I don't fully trust my intuition on this one because preprocessor code in the real world has 40+ years worth of accumulated oddities, so having some actual measurements against real world code would be very informative.

[LegalizeAdulthood]

I think I've got all the changes incorporated, but I'm getting a test failure so I haven't uploaded a new diff.

Honestly, I don't understand the test failure output. On Windows, the test failure output is all mangled and
difficult to interpret correctly as to what exactly is the problem, so I'm still trying to figure it out.

Improving the readability of the test failures is one of the things I would like to address in a future change.
I suspect it's only a problem on Windows as it seems most of the clang-tidy devs are using unix?

[aaron.ballman]

In D117522#3390136, @LegalizeAdulthood wrote:

I think I've got all the changes incorporated, but I'm getting a test failure so I haven't uploaded a new diff.

Honestly, I don't understand the test failure output. On Windows, the test failure output is all mangled and
difficult to interpret correctly as to what exactly is the problem, so I'm still trying to figure it out.

If you're able to post the output you're getting, I can try to help psychically debug it.

Improving the readability of the test failures is one of the things I would like to address in a future change.
I suspect it's only a problem on Windows as it seems most of the clang-tidy devs are using unix?

FWIW, I use Windows almost exclusively. Also, clang-tidy is frequently integrated into people's CI pipelines so even folks on *nix can be impacted by the behavior on Windows in those cases. That said, I have no idea what the test failures look like or how likely they are to be hit, so it may be reasonable to improve in a follow-up.

[LegalizeAdulthood]

I've worked through this issue before, I just need to spend some time on it.

The basic problem is that the test fails and dumps a bunch of output to "help" you
understand the failure, but the way it is formatted and mangled doesn't end up
being useful.

[LegalizeAdulthood]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

316

I added tests in my latest diff, which I'll upload now even though the tests are failing. (I'll upload again after I've got the test failure figured out.)

319

The difficulty in supporting more complex expressions is that we have NO AST support here and it involves manually matching tokens in the macro definition.

However, your point about ~ is well taken and that's easy to add based on what I've got here. I thought it important to handle negative literals, so I added support for unary -. I added support for unary + out of symmetry.

clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp

88–92

Most of the time it doesn't result in any new diffs besides those I'm adding myself. This is particularly true because most of the time I'm contributing new checks (which means whole new files) or fixing bugs on my existing checks (which have already had the whole file clang-format'ed).

I was unaware of the script, I'll take a look at that, thanks.

Should we cross link to the docs for this script in the "contributing" docs for clang-tidy?

[LegalizeAdulthood]

• Update from review comments
• check-clang-tools is reporting a test failure that still needs to be diagnosed (I think it is a mismatch between a CHECK-MESSAGES line and the exact output)

[LegalizeAdulthood]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

48

I was trying to follow "be liberal in what you accept as input and conservative in what you generate as output" maxim. I can remove the CR as a line ending case if you think it's too obscure.

clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

68–69

In the latest diff, I added a test case to make it clear that even if you #undef a macro later in the file, it invalidates all the surrounding macros in the cluster containing the undef'ed macro.

I'm open to suggestions for code bases on which to run this. Since this was motivated by my modernization of the fractint code base, I will run it on an old version of the code from my repo before I had manually converted the defines to enums.

LLVM isn't a good test case here, because LLVM doesn't use macros as enums :)

For this scenario (a macro that is later undef'ed), I don't believe I will emit any false positives.

[LegalizeAdulthood]

In D117522#3392122, @aaron.ballman wrote:

In D117522#3390136, @LegalizeAdulthood wrote:

I think I've got all the changes incorporated, but I'm getting a test failure so I haven't uploaded a new diff.

If you're able to post the output you're getting, I can try to help psychically debug it.

I finally figured it out! I previously had written:

void MacroToEnumCallbacks::warnMacroEnum(const EnumMacro &Macro) const {
  Check->diag(Macro.Directive->getLocation(),
              "macro '%0' defines an integral constant; prefer an enum instead")
      << Macro.Name.getIdentifierInfo()->getName();
}

and you suggested that I could drop the ->getName() as it had an insertion operator
for identifier info. Well, that was true, but what I didn't realize is that this would add an
extra single quotes around the identifier, so my diagnostic output now had doubled-up
single quotes everywhere and I couldn't figure out what was doing this as I couldn't find
doubled up single quotes in my format string and I was suspecting the python script
was somehow treating single quotes special somewhere.

Once I accounted for this, everything passes, so I'll be uploading a new diff shortly.

[LegalizeAdulthood]

• Tests pass again

[LegalizeAdulthood]

• Recognize bitwise negated integers

[LegalizeAdulthood]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

319

I've added support for bitwise negated integers. I didn't go further and try to recognize parenthesized literals (this just seems dumb, anyway... the extra parentheses do nothing and aren't ever needed).

[LegalizeAdulthood]

• Undo change introduced by clang-format

[aaron.ballman]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

48

If Clang supports it as a line ending, we probably should too, but... how do we handle CRLF vs "I mixed a CR with an LF by accident" kind of inputs? (Maybe we just treat that as CRLF and if the behavior is bad, the user shouldn't mix their line endings that way; I think that's defensible.) That seems to be similar to the scenario that's confusing me above where the user mixed an LF and CRLF by accident.

319

Thanks for adding the ~ support! I think we'll want parens supported as well because there's very common guidance to parenthesize EVERYTHING in the macro expansion list, but at the same time, I think that can be done once we get a bug report from a user.

clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

68–69

In the latest diff, I added a test case to make it clear that even if you #undef a macro later in the file, it invalidates all the surrounding macros in the cluster containing the undef'ed macro.

Oh, thank you! I had the impression we only cared if it was undefed within the same "run" we used to determine the macro.

I'm open to suggestions for code bases on which to run this.

I'd recommend some FreeBSD or Linux packages (or the whole distro if that's easy enough), as those tend to have a fair number of surprises. If that's harder and you'd rather just do one big project, I'd recommend something openssl, sqlite3, maybe postgres for C projects likely to make a fair amount of use of macros (from what I recall of them anyway).

[LegalizeAdulthood]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

48

Well, as far as Clang is concerned it's all just "whitespace" that gets eaten up by the preprocessor. Actually, that gives me a thought. A preprocessing directive is considered to end at the physical line ending, so I should look to see what sort of characters it considers to "end the line".

For the accidental mix-up, I'm not going to worry about that here. Your input files are assumed to be "well formed". The worst that happens in this check is that two blocks of macros that look like they are separated by a blank line are considered as a single clump by this check.

In other words, the worst that can happen is:

• Two clumps of macros are considered together.
• One clump of macros that is discarded because it doesn't follow the constraints "taints" an adjacent clump of macros that do follow the constraints.

Either way, nothing harmful happens to your code. It will still compile and be syntactically and semantically equivalent to what was there before.

clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

68–69

Those are all good suggestions, I'll see what I can do.

[aaron.ballman]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

48

Actually, that gives me a thought. A preprocessing directive is considered to end at the physical line ending, so I should look to see what sort of characters it considers to "end the line".

All of \r, \n, \r\n I believe (you can double-check in Lexer::LexTokenInternal()

Either way, nothing harmful happens to your code. It will still compile and be syntactically and semantically equivalent to what was there before.

Oh, that's a very good point, thank you. I think that's reasonable fallback behavior for these weird edge cases.

[LegalizeAdulthood]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

48

Well..... maybe.

If you look at Lexer::ReadToEndOfLine which is used to skip to the end of a preprocessor directive you'll see that it considers the first of '\r', '\n' or '\0' (end of file) as the end of the "line". This is around line 2835 of Lexer.cpp in my tree.

[LegalizeAdulthood]

• clang-format

[aaron.ballman]

Thanks for the discussion on this new check, it LGTM!

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

48

Yeah, I saw that as well (that's typically used for error recovery in the preprocessor). We also have Lexer::SkipWhitespace() which skips all vertical whitespace, but not \0.

[RKSimon]

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

217

@LegalizeAdulthood I'm seeing errors in https://lab.llvm.org/buildbot/#/builders/93/builds/7956

/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:216:22: error: declaration of ‘const clang::LangOptions& clang::tidy::modernize::{anonymous}::MacroToEnumCallbacks::LangOptions’ changes meaning of ‘LangOptions’ [-fpermissive]
  216 |   const LangOptions &LangOptions;

[dyung]

I've reverted this change in order to get the build bots green again.

clang-tools-extra/clang-tidy/modernize/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_library(clangTidyModernizeModule
   AvoidBindCheck.cpp
   AvoidCArraysCheck.cpp
   ConcatNestedNamespacesCheck.cpp
   DeprecatedHeadersCheck.cpp
   DeprecatedIosBaseAliasesCheck.cpp
   LoopConvertCheck.cpp
   LoopConvertUtils.cpp
+  MacroToEnumCheck.cpp
   MakeSharedCheck.cpp
   MakeSmartPtrCheck.cpp
   MakeUniqueCheck.cpp
   ModernizeTidyModule.cpp
   PassByValueCheck.cpp
   RawStringLiteralCheck.cpp
   RedundantVoidArgCheck.cpp
   ReplaceAutoPtrCheck.cpp

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h

+//===--- MacroToEnumCheck.h - clang-tidy ------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MACROTOENUMCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MACROTOENUMCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Replaces groups of related macros with an unscoped anonymous enum.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-macro-to-enum.html
+class MacroToEnumCheck : public ClangTidyCheck {
+public:
+  MacroToEnumCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+                           Preprocessor *ModuleExpanderPP) override;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MACROTOENUMCHECK_H

clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp

+//===--- MacroToEnumCheck.cpp - clang-tidy --------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "MacroToEnumCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/STLExtras.h"
+#include <algorithm>
+#include <cassert>
+#include <cctype>
+#include <string>
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+static bool hasOnlyComments(SourceLocation Loc, const LangOptions &Options,
+                            StringRef Text) {
+  // Use a lexer to look for tokens; if we find something other than a single
+  // hash, then there were intervening tokens between macro definitions.
+  std::string Buffer{Text};
+  Lexer Lex(Loc, Options, Buffer.c_str(), Buffer.c_str(),
+            Buffer.c_str() + Buffer.size());
+  Token Tok;
+  bool SeenHash = false;
+  while (!Lex.LexFromRawLexer(Tok)) {
+    if (Tok.getKind() == tok::hash && !SeenHash) {
+      SeenHash = true;
+      continue;
+    }
+    return false;
+  }
+
+  // Everything in between was whitespace, so now just look for a blank line,
+  // consisting of two consecutive EOL sequences, either '\n', '\r' or '\r\n'.
+  enum class WhiteSpace {
+    Nothing,
+    CR,
+    LF,
+    CRLF,
+    CRLFCR,
+  };
+
+  WhiteSpace State = WhiteSpace::Nothing;
+  for (char C : Text) {
+    switch (C) {
+    case '\r':
+      if (State == WhiteSpace::CR)
+        return false;
+
+      State = State == WhiteSpace::CRLF ? WhiteSpace::CRLFCR : WhiteSpace::CR;
+      break;
+
+    case '\n':
+      if (State == WhiteSpace::LF || State == WhiteSpace::CRLFCR)
+        return false;
+
+      State = State == WhiteSpace::CR ? WhiteSpace::CRLF : WhiteSpace::LF;
+      break;
+
+    default:
+      State = WhiteSpace::Nothing;
+      break;
+    }
+  }
+
+  return true;
+}
+
+// Validate that this literal token is a valid integer literal.
+// A literal token could be a floating-point token, which isn't
+// acceptable as a value for an enumeration.  A floating-point
+// token must either have a decimal point or an exponent.
+static bool isIntegralConstant(const Token &Token) {
+  const char *Begin = Token.getLiteralData();
+  const char *End = Begin + Token.getLength();
+  return std::none_of(
+      Begin, End, [](char C) { return C == '.' || std::toupper(C) == 'E'; });
+}
+
+namespace {
+
+struct EnumMacro {
+  EnumMacro(Token Name, const MacroDirective *Directive)
+      : Name(Name), Directive(Directive) {}
+
+  Token Name;
+  const MacroDirective *Directive;
+};
+
+using MacroList = SmallVector<EnumMacro>;
+
+enum class IncludeGuard { None, FileChanged, IfGuard, DefineGuard };
+
+struct FileState {
+  FileState()
+      : ConditionScopes(0), LastLine(0), GuardScanner(IncludeGuard::None) {}
+
+  int ConditionScopes;
+  unsigned int LastLine;
+  IncludeGuard GuardScanner;
+  SourceLocation LastMacroLocation;
+};
+
+class MacroToEnumCallbacks : public PPCallbacks {
+public:
+  MacroToEnumCallbacks(MacroToEnumCheck *Check, const LangOptions &LangOptions,
+                       const SourceManager &SM)
+      : Check(Check), LangOptions(LangOptions), SM(SM) {}
+
+  void FileChanged(SourceLocation Loc, FileChangeReason Reason,
+                   SrcMgr::CharacteristicKind FileType,
+                   FileID PrevFID) override;
+
+  void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
+                          StringRef FileName, bool IsAngled,
+                          CharSourceRange FilenameRange, const FileEntry *File,
+                          StringRef SearchPath, StringRef RelativePath,
+                          const Module *Imported,
+                          SrcMgr::CharacteristicKind FileType) override {
+    clearCurrentEnum(HashLoc);
+  }
+
+  // Keep track of macro definitions that look like enums.
+  void MacroDefined(const Token &MacroNameTok,
+                    const MacroDirective *MD) override;
+
+  // Undefining an enum-like macro results in the enum set being dropped.
+  void MacroUndefined(const Token &MacroNameTok, const MacroDefinition &MD,
+                      const MacroDirective *Undef) override;
+
+  // Conditional compilation clears any adjacent enum-like macros.
+  // Macros used in conditional expressions clear any adjacent enum-like
+  // macros.
+  // Include guards are either
+  //   #if !defined(GUARD)
+  // or
+  //   #ifndef GUARD
+  void If(SourceLocation Loc, SourceRange ConditionRange,
+          ConditionValueKind ConditionValue) override;
+  void Ifndef(SourceLocation Loc, const Token &MacroNameTok,
+              const MacroDefinition &MD) override;
+  void Ifdef(SourceLocation Loc, const Token &MacroNameTok,
+             const MacroDefinition &MD) override;
+  void Elif(SourceLocation Loc, SourceRange ConditionRange,
+            ConditionValueKind ConditionValue, SourceLocation IfLoc) override;
+  void Elifdef(SourceLocation Loc, const Token &MacroNameTok,
+               const MacroDefinition &MD) override;
+  void Elifndef(SourceLocation Loc, const Token &MacroNameTok,
+                const MacroDefinition &MD) override;
+  void Endif(SourceLocation Loc, SourceLocation IfLoc) override;
+  void PragmaDirective(SourceLocation Loc,
+                       PragmaIntroducerKind Introducer) override;
+
+  // After we've seen everything, issue warnings and fix-its.
+  void EndOfMainFile() override;
+
+private:
+  void newEnum() {
+    if (Enums.empty() || !Enums.back().empty())
+      Enums.emplace_back();
+  }
+  bool insideConditional() const {
+    return (CurrentFile->GuardScanner == IncludeGuard::DefineGuard &&
+            CurrentFile->ConditionScopes > 1) ||
+           (CurrentFile->GuardScanner != IncludeGuard::DefineGuard &&
+            CurrentFile->ConditionScopes > 0);
+  }
+  bool isConsecutiveMacro(const MacroDirective *MD) const;
+  void rememberLastMacroLocation(const MacroDirective *MD) {
+    CurrentFile->LastLine = SM.getSpellingLineNumber(MD->getLocation());
+    CurrentFile->LastMacroLocation = Lexer::getLocForEndOfToken(
+        MD->getMacroInfo()->getDefinitionEndLoc(), 0, SM, LangOptions);
+  }
+  void clearLastMacroLocation() {
+    CurrentFile->LastLine = 0;
+    CurrentFile->LastMacroLocation = SourceLocation{};
+  }
+  void clearCurrentEnum(SourceLocation Loc);
+  void conditionStart(const SourceLocation &Loc);
+  void checkCondition(SourceRange ConditionRange);
+  void checkName(const Token &MacroNameTok);
+  void warnMacroEnum(const EnumMacro &Macro) const;
+  void fixEnumMacro(const MacroList &MacroList) const;
+
+  MacroToEnumCheck *Check;
+  const LangOptions &LangOptions;
+  const SourceManager &SM;
+  SmallVector<MacroList> Enums;
+  SmallVector<FileState> Files;
+  FileState *CurrentFile = nullptr;
+};
+
+bool MacroToEnumCallbacks::isConsecutiveMacro(const MacroDirective *MD) const {
+  if (CurrentFile->LastMacroLocation.isInvalid())
+    return false;
+
+  SourceLocation Loc = MD->getLocation();
+  if (CurrentFile->LastLine + 1 == SM.getSpellingLineNumber(Loc))
+    return true;
+
+  SourceLocation Define =
+      SM.translateLineCol(SM.getFileID(Loc), SM.getSpellingLineNumber(Loc), 1);
+  CharSourceRange BetweenMacros{
+      SourceRange{CurrentFile->LastMacroLocation, Define}, true};
+  CharSourceRange CharRange =
+      Lexer::makeFileCharRange(BetweenMacros, SM, LangOptions);
+  StringRef BetweenText = Lexer::getSourceText(CharRange, SM, LangOptions);
+  return hasOnlyComments(Define, LangOptions, BetweenText);
+}
+
+void MacroToEnumCallbacks::clearCurrentEnum(SourceLocation Loc) {
+  // Only drop the most recent Enum set if the directive immediately follows.
+  if (!Enums.empty() && !Enums.back().empty() &&
+      SM.getSpellingLineNumber(Loc) == CurrentFile->LastLine + 1)
+    Enums.pop_back();
+
+  clearLastMacroLocation();
+}
+
+void MacroToEnumCallbacks::conditionStart(const SourceLocation &Loc) {
+  ++CurrentFile->ConditionScopes;
+  clearCurrentEnum(Loc);
+  if (CurrentFile->GuardScanner == IncludeGuard::FileChanged) {
+    CurrentFile->GuardScanner = IncludeGuard::IfGuard;
+  }
+}
+
+void MacroToEnumCallbacks::checkCondition(SourceRange Range) {
+  CharSourceRange CharRange = Lexer::makeFileCharRange(
+      CharSourceRange::getTokenRange(Range), SM, LangOptions);
+  std::string Text = Lexer::getSourceText(CharRange, SM, LangOptions).str();
+  Lexer Lex(CharRange.getBegin(), LangOptions, Text.data(), Text.data(),
+            Text.data() + Text.size());
+  Token Tok;
+  bool End = false;
+  while (!End) {
+    End = Lex.LexFromRawLexer(Tok);
+    if (Tok.is(tok::raw_identifier) && Tok.getRawIdentifier().str() != "defined")
+      checkName(Tok);
+  }
+}
+
+void MacroToEnumCallbacks::checkName(const Token &MacroNameTok) {
+  std::string Id;
+  if (MacroNameTok.is(tok::raw_identifier))
+      Id = MacroNameTok.getRawIdentifier().str();
+  else if (MacroNameTok.is(tok::identifier))
+      Id = MacroNameTok.getIdentifierInfo()->getName().str();
+  else {
+    assert(false && "Expected either an identifier or raw identifier token");
+    return;
+  }
+
+  llvm::erase_if(Enums, [&Id](const MacroList &MacroList) {
+    return llvm::any_of(MacroList, [&Id](const EnumMacro &Macro) {
+      return Macro.Name.getIdentifierInfo()->getName().str() == Id;
+    });
+  });
+}
+
+void MacroToEnumCallbacks::FileChanged(SourceLocation Loc,
+                                       FileChangeReason Reason,
+                                       SrcMgr::CharacteristicKind FileType,
+                                       FileID PrevFID) {
+  newEnum();
+  if (Reason == EnterFile) {
+    Files.emplace_back();
+    if (!SM.isInMainFile(Loc))
+      Files.back().GuardScanner = IncludeGuard::FileChanged;
+  } else if (Reason == ExitFile) {
+    assert(CurrentFile->ConditionScopes == 0);
+    Files.pop_back();
+  }
+  CurrentFile = &Files.back();
+}
+
+void MacroToEnumCallbacks::MacroDefined(const Token &MacroNameTok,
+                                        const MacroDirective *MD) {
+  // Include guards are never candidates for becoming an enum.
+  if (CurrentFile->GuardScanner == IncludeGuard::IfGuard) {
+    CurrentFile->GuardScanner = IncludeGuard::DefineGuard;
+    return;
+  }
+
+  if (insideConditional())
+    return;
+
+  if (SM.getFilename(MD->getLocation()).empty())
+    return;
+
+  const MacroInfo *Info = MD->getMacroInfo();
+  if (Info->isFunctionLike() || Info->isBuiltinMacro() ||
+      Info->tokens().empty() || Info->tokens().size() > 2)
+    return;
+
+  // It can be +Lit, -Lit or just Lit.
+  Token Tok = Info->tokens().front();
+  if (Info->tokens().size() == 2) {
+    if (!Tok.isOneOf(tok::TokenKind::minus, tok::TokenKind::plus))
+      return;
+    Tok = Info->tokens().back();
+  }
+  if (!Tok.isLiteral() || isStringLiteral(Tok.getKind()) ||
+      !isIntegralConstant(Tok))
+    return;
+
+  if (!isConsecutiveMacro(MD))
+    newEnum();
+  Enums.back().emplace_back(MacroNameTok, MD);
+  rememberLastMacroLocation(MD);
+}
+
+// Any macro that is undefined removes all adjacent macros from consideration as
+// an enum and starts a new enum scan.
+void MacroToEnumCallbacks::MacroUndefined(const Token &MacroNameTok,
+                                          const MacroDefinition &MD,
+                                          const MacroDirective *Undef) {
+  auto MatchesToken = [&MacroNameTok](const EnumMacro &Macro) {
+    return Macro.Name.getIdentifierInfo()->getName() ==
+           MacroNameTok.getIdentifierInfo()->getName();
+  };
+
+  auto It = llvm::find_if(Enums, [MatchesToken](const MacroList &MacroList) {
+    return llvm::any_of(MacroList, MatchesToken);
+  });
+  if (It != Enums.end())
+    Enums.erase(It);
+
+  clearLastMacroLocation();
+  CurrentFile->GuardScanner = IncludeGuard::None;
+}
+
+void MacroToEnumCallbacks::If(SourceLocation Loc, SourceRange ConditionRange,
+                              ConditionValueKind ConditionValue) {
+  conditionStart(Loc);
+  checkCondition(ConditionRange);
+}
+
+void MacroToEnumCallbacks::Ifndef(SourceLocation Loc, const Token &MacroNameTok,
+                                  const MacroDefinition &MD) {
+  conditionStart(Loc);
+  checkName(MacroNameTok);
+}
+
+void MacroToEnumCallbacks::Ifdef(SourceLocation Loc, const Token &MacroNameTok,
+                                 const MacroDefinition &MD) {
+  conditionStart(Loc);
+  checkName(MacroNameTok);
+}
+
+void MacroToEnumCallbacks::Elif(SourceLocation Loc, SourceRange ConditionRange,
+    ConditionValueKind ConditionValue, SourceLocation IfLoc) {
+  checkCondition(ConditionRange);
+}
+
+void MacroToEnumCallbacks::Elifdef(SourceLocation Loc,
+    const Token &MacroNameTok, const MacroDefinition &MD) {
+  checkName(MacroNameTok);
+}
+
+void MacroToEnumCallbacks::Elifndef(SourceLocation Loc,
+    const Token &MacroNameTok, const MacroDefinition &MD) {
+  checkName(MacroNameTok);
+}
+
+void MacroToEnumCallbacks::Endif(SourceLocation Loc, SourceLocation IfLoc) {
+  // The if directive for the include guard isn't counted in the
+  // ConditionScopes.
+  if (CurrentFile->ConditionScopes == 0 &&
+      CurrentFile->GuardScanner == IncludeGuard::DefineGuard)
+    return;
+
+  // We don't need to clear the current enum because the start of the
+  // conditional block already took care of that.
+  assert(CurrentFile->ConditionScopes > 0);
+  --CurrentFile->ConditionScopes;
+}
+
+void MacroToEnumCallbacks::PragmaDirective(SourceLocation Loc,
+                                           PragmaIntroducerKind Introducer) {
+  if (CurrentFile->GuardScanner != IncludeGuard::FileChanged)
+    return;
+
+  bool Invalid = false;
+  const char *Text = SM.getCharacterData(
+      Lexer::getLocForEndOfToken(Loc, 0, SM, LangOptions), &Invalid);
+  if (Invalid)
+    return;
+
+  while (*Text && std::isspace(*Text))
+    ++Text;
+
+  if (std::strncmp("pragma", Text, 6) != 0)
+    return;
+
+  Text += 6;
+  while (*Text && std::isspace(*Text))
+    ++Text;
+
+  if (std::strncmp("once", Text, 4) == 0)
+    CurrentFile->GuardScanner = IncludeGuard::IfGuard;
+}
+
+void MacroToEnumCallbacks::EndOfMainFile() {
+  for (const MacroList &MacroList : Enums) {
+    if (MacroList.empty())
+      continue;
+
+    for (const EnumMacro &Macro : MacroList)
+      warnMacroEnum(Macro);
+
+    fixEnumMacro(MacroList);
+  }
+}
+
+void MacroToEnumCallbacks::warnMacroEnum(const EnumMacro &Macro) const {
+  Check->diag(Macro.Directive->getLocation(),
+              "Macro '%0' defines an integral constant; prefer an enum instead")
+      << Macro.Name.getIdentifierInfo()->getName();
+}
+
+void MacroToEnumCallbacks::fixEnumMacro(const MacroList &MacroList) const {
+  SourceLocation Begin =
+      MacroList.front().Directive->getMacroInfo()->getDefinitionLoc();
+  Begin = SM.translateLineCol(SM.getFileID(Begin),
+                              SM.getSpellingLineNumber(Begin), 1);
+  DiagnosticBuilder Diagnostic =
+      Check->diag(Begin, "Replace macro with enum")
+      << FixItHint::CreateInsertion(Begin, "enum {\n");
+
+  for (size_t I = 0u; I < MacroList.size(); ++I) {
+    const EnumMacro &Macro = MacroList[I];
+    SourceLocation DefineEnd =
+        Macro.Directive->getMacroInfo()->getDefinitionLoc();
+    SourceLocation DefineBegin = SM.translateLineCol(
+        SM.getFileID(DefineEnd), SM.getSpellingLineNumber(DefineEnd), 1);
+    CharSourceRange DefineRange;
+    DefineRange.setBegin(DefineBegin);
+    DefineRange.setEnd(DefineEnd);
+    Diagnostic << FixItHint::CreateRemoval(DefineRange);
+
+    SourceLocation NameEnd = Lexer::getLocForEndOfToken(
+        Macro.Directive->getMacroInfo()->getDefinitionLoc(), 0, SM,
+        LangOptions);
+    Diagnostic << FixItHint::CreateInsertion(NameEnd, " =");
+
+    SourceLocation ValueEnd = Lexer::getLocForEndOfToken(
+        Macro.Directive->getMacroInfo()->getDefinitionEndLoc(), 0, SM,
+        LangOptions);
+    if (I < MacroList.size() - 1)
+      Diagnostic << FixItHint::CreateInsertion(ValueEnd, ",");
+  }
+
+  SourceLocation End = Lexer::getLocForEndOfToken(
+      MacroList.back().Directive->getMacroInfo()->getDefinitionEndLoc(), 0, SM,
+      LangOptions);
+  End = SM.translateLineCol(SM.getFileID(End),
+                            SM.getSpellingLineNumber(End) + 1, 1);
+  Diagnostic << FixItHint::CreateInsertion(End, "};\n");
+}
+
+} // namespace
+
+void MacroToEnumCheck::registerPPCallbacks(const SourceManager &SM,
+                                           Preprocessor *PP,
+                                           Preprocessor *ModuleExpanderPP) {
+  PP->addPPCallbacks(
+      std::make_unique<MacroToEnumCallbacks>(this, getLangOpts(), SM));
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp

 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "AvoidBindCheck.h"
 #include "AvoidCArraysCheck.h"
 #include "ConcatNestedNamespacesCheck.h"
 #include "DeprecatedHeadersCheck.h"
 #include "DeprecatedIosBaseAliasesCheck.h"
 #include "LoopConvertCheck.h"
+#include "MacroToEnumCheck.h"
 #include "MakeSharedCheck.h"
 #include "MakeUniqueCheck.h"
 #include "PassByValueCheck.h"
 #include "RawStringLiteralCheck.h"
 #include "RedundantVoidArgCheck.h"
 #include "ReplaceAutoPtrCheck.h"
 #include "ReplaceDisallowCopyAndAssignMacroCheck.h"
 #include "ReplaceRandomShuffleCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<AvoidCArraysCheck>("modernize-avoid-c-arrays");
     CheckFactories.registerCheck<ConcatNestedNamespacesCheck>(
         "modernize-concat-nested-namespaces");
     CheckFactories.registerCheck<DeprecatedHeadersCheck>(
         "modernize-deprecated-headers");
     CheckFactories.registerCheck<DeprecatedIosBaseAliasesCheck>(
         "modernize-deprecated-ios-base-aliases");
     CheckFactories.registerCheck<LoopConvertCheck>("modernize-loop-convert");
+    CheckFactories.registerCheck<MacroToEnumCheck>("modernize-macro-to-enum");
     CheckFactories.registerCheck<MakeSharedCheck>("modernize-make-shared");
     CheckFactories.registerCheck<MakeUniqueCheck>("modernize-make-unique");
     CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value");
     CheckFactories.registerCheck<RawStringLiteralCheck>(
         "modernize-raw-string-literal");
     CheckFactories.registerCheck<RedundantVoidArgCheck>(
         "modernize-redundant-void-arg");
     CheckFactories.registerCheck<ReplaceAutoPtrCheck>(
         "modernize-replace-auto-ptr");
     CheckFactories.registerCheck<ReplaceDisallowCopyAndAssignMacroCheck>(
         "modernize-replace-disallow-copy-and-assign-macro");
     CheckFactories.registerCheck<ReplaceRandomShuffleCheck>(
         "modernize-replace-random-shuffle");
     CheckFactories.registerCheck<ReturnBracedInitListCheck>(
         "modernize-return-braced-init-list");
     CheckFactories.registerCheck<ShrinkToFitCheck>("modernize-shrink-to-fit");
     CheckFactories.registerCheck<UnaryStaticAssertCheck>(
         "modernize-unary-static-assert");
     CheckFactories.registerCheck<UseAutoCheck>("modernize-use-auto");
     CheckFactories.registerCheck<UseBoolLiteralsCheck>(
         "modernize-use-bool-literals");
     CheckFactories.registerCheck<UseDefaultMemberInitCheck>(
         "modernize-use-default-member-init");
     CheckFactories.registerCheck<UseEmplaceCheck>("modernize-use-emplace");
-    CheckFactories.registerCheck<UseEqualsDefaultCheck>("modernize-use-equals-default");
+    CheckFactories.registerCheck<UseEqualsDefaultCheck>(
+        "modernize-use-equals-default");
     CheckFactories.registerCheck<UseEqualsDeleteCheck>(
         "modernize-use-equals-delete");
-    CheckFactories.registerCheck<UseNodiscardCheck>(
-        "modernize-use-nodiscard");
+    CheckFactories.registerCheck<UseNodiscardCheck>("modernize-use-nodiscard");
     CheckFactories.registerCheck<UseNoexceptCheck>("modernize-use-noexcept");
     CheckFactories.registerCheck<UseNullptrCheck>("modernize-use-nullptr");
     CheckFactories.registerCheck<UseOverrideCheck>("modernize-use-override");
     CheckFactories.registerCheck<UseTrailingReturnTypeCheck>(
         "modernize-use-trailing-return-type");
     CheckFactories.registerCheck<UseTransparentFunctorsCheck>(
         "modernize-use-transparent-functors");
     CheckFactories.registerCheck<UseUncaughtExceptionsCheck>(

clang-tools-extra/docs/ReleaseNotes.rst

 
 New checks
 ^^^^^^^^^^
 
 - New :doc:`bugprone-shared-ptr-array-mismatch <clang-tidy/checks/bugprone-shared-ptr-array-mismatch>` check.
 
   Finds initializations of C++ shared pointers to non-array type that are initialized with an array.
 
+- New :doc:`modernize-macro-to-enum
+  <clang-tidy/checks/modernize-macro-to-enum>` check.
+
+  Replaces groups of adjacent macros with an unscoped anonymous enum.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
+- New alias :doc:`cppcoreguidelines-macro-to-enum
+  <clang-tidy/checks/cppcoreguidelines-macro-to-enum>` to :doc:`modernize-macro-to-enum
+  <clang-tidy/checks/modernize-macro-to-enum>` was added.
+
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Removed checks
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-to-enum.rst

+.. title:: clang-tidy - cppcoreguidelines-macro-to-enum
+.. meta::
+   :http-equiv=refresh: 5;URL=modernize-macro-to-enum.html
+
+cppcoreguidelines-macro-to-enum
+===============================
+
+The cppcoreguidelines-macro-to-enum check is an alias, please see
+:doc:`modernize-macro-to-enum <modernize-macro-to-enum>` for more information.

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `misc-unused-parameters <misc-unused-parameters.html>`_, "Yes"
    `misc-unused-using-decls <misc-unused-using-decls.html>`_, "Yes"
    `modernize-avoid-bind <modernize-avoid-bind.html>`_, "Yes"
    `modernize-avoid-c-arrays <modernize-avoid-c-arrays.html>`_,
    `modernize-concat-nested-namespaces <modernize-concat-nested-namespaces.html>`_, "Yes"
    `modernize-deprecated-headers <modernize-deprecated-headers.html>`_, "Yes"
    `modernize-deprecated-ios-base-aliases <modernize-deprecated-ios-base-aliases.html>`_, "Yes"
    `modernize-loop-convert <modernize-loop-convert.html>`_, "Yes"
+   `modernize-macro-to-enum <modernize-macro-to-enum.html>`_, "Yes"
    `modernize-make-shared <modernize-make-shared.html>`_, "Yes"
    `modernize-make-unique <modernize-make-unique.html>`_, "Yes"
    `modernize-pass-by-value <modernize-pass-by-value.html>`_, "Yes"
    `modernize-raw-string-literal <modernize-raw-string-literal.html>`_, "Yes"
    `modernize-redundant-void-arg <modernize-redundant-void-arg.html>`_, "Yes"
    `modernize-replace-auto-ptr <modernize-replace-auto-ptr.html>`_, "Yes"
    `modernize-replace-disallow-copy-and-assign-macro <modernize-replace-disallow-copy-and-assign-macro.html>`_, "Yes"
    `modernize-replace-random-shuffle <modernize-replace-random-shuffle.html>`_, "Yes"
.. csv-table:: Aliases..
    `clang-analyzer-unix.MismatchedDeallocator <clang-analyzer-unix.MismatchedDeallocator.html>`_, `Clang Static Analyzer <https://clang.llvm.org/docs/analyzer/checkers.html>`_,
    `clang-analyzer-unix.Vfork <clang-analyzer-unix.Vfork.html>`_, `Clang Static Analyzer <https://clang.llvm.org/docs/analyzer/checkers.html>`_,
    `clang-analyzer-unix.cstring.BadSizeArg <clang-analyzer-unix.cstring.BadSizeArg.html>`_, `Clang Static Analyzer <https://clang.llvm.org/docs/analyzer/checkers.html>`_,
    `clang-analyzer-unix.cstring.NullArg <clang-analyzer-unix.cstring.NullArg.html>`_, `Clang Static Analyzer <https://clang.llvm.org/docs/analyzer/checkers.html>`_,
    `cppcoreguidelines-avoid-c-arrays <cppcoreguidelines-avoid-c-arrays.html>`_, `modernize-avoid-c-arrays <modernize-avoid-c-arrays.html>`_,
    `cppcoreguidelines-avoid-magic-numbers <cppcoreguidelines-avoid-magic-numbers.html>`_, `readability-magic-numbers <readability-magic-numbers.html>`_,
    `cppcoreguidelines-c-copy-assignment-signature <cppcoreguidelines-c-copy-assignment-signature.html>`_, `misc-unconventional-assign-operator <misc-unconventional-assign-operator.html>`_,
    `cppcoreguidelines-explicit-virtual-functions <cppcoreguidelines-explicit-virtual-functions.html>`_, `modernize-use-override <modernize-use-override.html>`_, "Yes"
+   `cppcoreguidelines-macro-to-enum <cppcoreguidelines-macro-to-enum.html>`_, `modernize-macro-to-enum <modernize-macro-to-enum.html>`_, "Yes"
    `cppcoreguidelines-non-private-member-variables-in-classes <cppcoreguidelines-non-private-member-variables-in-classes.html>`_, `misc-non-private-member-variables-in-classes <misc-non-private-member-variables-in-classes.html>`_,
    `fuchsia-header-anon-namespaces <fuchsia-header-anon-namespaces.html>`_, `google-build-namespaces <google-build-namespaces.html>`_,
    `google-readability-braces-around-statements <google-readability-braces-around-statements.html>`_, `readability-braces-around-statements <readability-braces-around-statements.html>`_, "Yes"
    `google-readability-function-size <google-readability-function-size.html>`_, `readability-function-size <readability-function-size.html>`_,
    `google-readability-namespace-comments <google-readability-namespace-comments.html>`_, `llvm-namespace-comment <llvm-namespace-comment.html>`_,
    `hicpp-avoid-c-arrays <hicpp-avoid-c-arrays.html>`_, `modernize-avoid-c-arrays <modernize-avoid-c-arrays.html>`_,
    `hicpp-braces-around-statements <hicpp-braces-around-statements.html>`_, `readability-braces-around-statements <readability-braces-around-statements.html>`_, "Yes"
    `hicpp-deprecated-headers <hicpp-deprecated-headers.html>`_, `modernize-deprecated-headers <modernize-deprecated-headers.html>`_, "Yes"

clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst

+.. title:: clang-tidy - modernize-macro-to-enum
+
+modernize-macro-to-enum
+=======================
+
+Replaces groups of adjacent macros with an unscoped anonymous enum.
+Using an unscoped anonymous enum ensures that everywhere the macro
+token was used previously, the enumerator name may be safely used.
+
+This check can be used to enforce the C++ core guideline `Enum.1:
+Prefer enumerations over macros
+<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#enum1-prefer-enumerations-over-macros>`_,
+within the constraints outlined below.
+
+Potential macros for replacement must meet the following constraints:
+
+- Macros must expand only to integral literal tokens.
+- Macros must be defined on sequential source file lines, or with
+  only comment lines in between macro definitions.
+- Macros must all be defined in the same source file.
+- Macros must not be defined within a conditional compilation block.
+  (Conditional include guards are exempt from this constraint.)
+- Macros must not be defined adjacent to other preprocessor directives.
+- Macros must not be used in any conditional preprocessing directive.
+
+Each cluster of macros meeting the above constraints is presumed to
+be a set of values suitable for replacement by an anonymous enum.
+From there, a developer can give the anonymous enum a name and
+continue refactoring to a scoped enum if desired.  Comments on the
+same line as a macro definition or between subsequent macro definitions
+are preserved in the output.  No formatting is assumed in the provided
+replacements, although clang-tidy can optionally format all fixes.
+
+Examples:
+
+.. code-block:: c++
+
+  #define RED   0xFF0000
+  #define GREEN 0x00FF00
+  #define BLUE  0x0000FF
+
+  #define TM_ONE 1    // Use tailored method one.
+  #define TM_TWO 2    // Use tailored method two.  Method two
+                      // is preferable to method one.
+  #define TM_THREE 3  // Use tailored method three.
+
+becomes
+
+.. code-block:: c++
+
+  enum {
+  RED = 0xFF0000,
+  GREEN = 0x00FF00,
+  BLUE = 0x0000FF
+  };
+
+  enum {
+  TM_ONE = 1,    // Use tailored method one.
+  TM_TWO = 2,    // Use tailored method two.  Method two
+                      // is preferable to method one.
+  TM_THREE = 3  // Use tailored method three.
+  };

clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum.h

+#if !defined(MODERNIZE_MACRO_TO_ENUM_H)
+#define MODERNIZE_MACRO_TO_ENUM_H
+
+#include "modernize-macro-to-enum2.h"
+
+#define GG_RED 0xFF0000
+#define GG_GREEN 0x00FF00
+#define GG_BLUE 0x0000FF
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG_RED' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG_GREEN' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG_BLUE' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: GG_RED = 0xFF0000,
+// CHECK-FIXES-NEXT: GG_GREEN = 0x00FF00,
+// CHECK-FIXES-NEXT: GG_BLUE = 0x0000FF
+// CHECK-FIXES-NEXT: };
+
+#if 1
+#define RR_RED 1
+#define RR_GREEN 2
+#define RR_BLUE 3
+#endif
+
+#endif

clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum2.h

+#ifndef MODERNIZE_MACRO_TO_ENUM2_H
+#define MODERNIZE_MACRO_TO_ENUM2_H
+
+#include "modernize-macro-to-enum3.h"
+
+#define GG2_RED 0xFF0000
+#define GG2_GREEN 0x00FF00
+#define GG2_BLUE 0x0000FF
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG2_RED' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG2_GREEN' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG2_BLUE' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: GG2_RED = 0xFF0000,
+// CHECK-FIXES-NEXT: GG2_GREEN = 0x00FF00,
+// CHECK-FIXES-NEXT: GG2_BLUE = 0x0000FF
+// CHECK-FIXES-NEXT: };
+
+#if 1
+#define RR2_RED 1
+#define RR2_GREEN 2
+#define RR2_BLUE 3
+#endif
+
+#endif

clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum3.h

+#pragma once
+
+#define GG3_RED 0xFF0000
+#define GG3_GREEN 0x00FF00
+#define GG3_BLUE 0x0000FF
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG3_RED' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG3_GREEN' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG3_BLUE' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: GG3_RED = 0xFF0000,
+// CHECK-FIXES-NEXT: GG3_GREEN = 0x00FF00,
+// CHECK-FIXES-NEXT: GG3_BLUE = 0x0000FF
+// CHECK-FIXES-NEXT: };
+
+#if 1
+#define RR3_RED 1
+#define RR3_GREEN 2
+#define RR3_BLUE 3
+#endif

clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

+// RUN: %check_clang_tidy %s modernize-macro-to-enum %t -- -- -I%S/Inputs/modernize-macro-to-enum
+
+#if 1
+#include "modernize-macro-to-enum.h"
+
+// These macros are skipped due to being inside a conditional compilation block.
+#define GOO_RED 1
+#define GOO_GREEN 2
+#define GOO_BLUE 3
+
+#endif
+
+#define RED 0xFF0000
+#define GREEN 0x00FF00
+#define BLUE 0x0000FF
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum [modernize-macro-to-enum]
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'RED' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GREEN' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BLUE' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: RED = 0xFF0000,
+// CHECK-FIXES-NEXT: GREEN = 0x00FF00,
+// CHECK-FIXES-NEXT: BLUE = 0x0000FF
+// CHECK-FIXES-NEXT: };
+
+// Verify that comments are preserved.
+#define CoordModeOrigin         0   /* relative to the origin */
+#define CoordModePrevious       1   /* relative to previous point */
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModeOrigin' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModePrevious' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: CoordModeOrigin =         0,   /* relative to the origin */
+// CHECK-FIXES-NEXT: CoordModePrevious =       1   /* relative to previous point */
+// CHECK-FIXES-NEXT: };
+
+// Verify that multiline comments are preserved.
+#define BadDrawable         9   /* parameter not a Pixmap or Window */
+#define BadAccess           10  /* depending on context:
+                                - key/button already grabbed
+                                - attempt to free an illegal 
+                                  cmap entry 
+                                - attempt to store into a read-only 
+                                  color map entry. */
+                                // - attempt to modify the access control
+                                //   list from other than the local host.
+                                //
+#define BadAlloc            11  /* insufficient resources */
+// CHECK-MESSAGES: :[[@LINE-11]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadDrawable' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadAccess' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BadAlloc' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: BadDrawable =         9,   /* parameter not a Pixmap or Window */
+// CHECK-FIXES-NEXT: BadAccess =           10,  /* depending on context:
+// CHECK-FIXES-NEXT:                                 - key/button already grabbed
+// CHECK-FIXES-NEXT:                                 - attempt to free an illegal 
+// CHECK-FIXES-NEXT:                                   cmap entry 
+// CHECK-FIXES-NEXT:                                 - attempt to store into a read-only 
+// CHECK-FIXES-NEXT:                                   color map entry. */
+// CHECK-FIXES-NEXT:                                 // - attempt to modify the access control
+// CHECK-FIXES-NEXT:                                 //   list from other than the local host.
+// CHECK-FIXES-NEXT:                                 //
+// CHECK-FIXES-NEXT: BadAlloc =            11  /* insufficient resources */
+// CHECK-FIXES-NEXT: };
+
+// Undefining a macro invalidates adjacent macros
+// from being considered as an enum.
+#define REMOVED1 1
+#define REMOVED2 2
+#define REMOVED3 3
+#undef REMOVED2
+#define VALID1 1
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: Macro 'VALID1' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: VALID1 = 1
+// CHECK-FIXES-NEXT: };
+
+// Integral constants can have an optional sign
+#define SIGNED1 +1
+#define SIGNED2 -1
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'SIGNED1' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'SIGNED2' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: SIGNED1 = +1,
+// CHECK-FIXES-NEXT: SIGNED2 = -1
+// CHECK-FIXES-NEXT: };
+
+// Macros appearing in conditional expressions can't be replaced
+// by enums.
+#define USE_FOO 1
+#define USE_BAR 0
+#define USE_IF 1
+#define USE_ELIF 1
+#define USE_IFDEF 1
+#define USE_IFNDEF 1
+
+#if defined(USE_FOO) && USE_FOO
+extern void foo();
+#else
+inline void foo() {}
+#endif
+
+#if USE_BAR
+extern void bar();
+#else
+inline void bar() {}
+#endif
+
+#if USE_IF
+inline void used_if() {}
+#endif
+
+#if 0
+#elif USE_ELIF
+inline void used_elif() {}
+#endif
+
+#ifdef USE_IFDEF
+inline void used_ifdef() {}
+#endif
+
+#ifndef USE_IFNDEF
+#else
+inline void used_ifndef() {}
+#endif
+
+// Regular conditional compilation blocks should leave previous
+// macro enums alone.
+#if 0
+#include <non-existent.h>
+#endif
+
+// Conditional compilation blocks invalidate adjacent macros
+// from being considered as an enum.  Conditionally compiled
+// blocks could contain macros that should rightly be included
+// in the enum, but we can't explore multiple branches of a
+// conditionally compiled section in clang-tidy, only the active
+// branch based on compilation options.
+#define CONDITION1 1
+#define CONDITION2 2
+#if 0
+#define CONDITION3 3
+#else
+#define CONDITION3 -3
+#endif
+
+#define IFDEF1 1
+#define IFDEF2 2
+#ifdef FROB
+#define IFDEF3 3
+#endif
+
+#define IFNDEF1 1
+#define IFNDEF2 2
+#ifndef GOINK
+#define IFNDEF3 3
+#endif
+
+// These macros do not expand to integral constants.
+#define HELLO "Hello, "
+#define WORLD "World"
+#define EPS1 1.0F
+#define EPS2 1e5
+#define EPS3 1.
+
+#define DO_RED draw(RED)
+#define DO_GREEN draw(GREEN)
+#define DO_BLUE draw(BLUE)
+
+#define FN_RED(x) draw(RED | x)
+#define FN_GREEN(x) draw(GREEN | x)
+#define FN_BLUE(x) draw(BLUE | x)
+
+extern void draw(unsigned int Color);
+
+void f()
+{
+  draw(RED);
+  draw(GREEN);
+  draw(BLUE);
+  DO_RED;
+  DO_GREEN;
+  DO_BLUE;
+  FN_RED(0);
+  FN_GREEN(0);
+  FN_BLUE(0);
+}