[clang-tidy] Add cppcoreguidelines-prefer-scoped-enums

[janosbenjaminantal]

Summary

This check aims to flag every occurence of unscoped enumerations and provide useful fix to convert them to scoped enumerations.

It works for the most cases, except when an enumeration is defined within a macro, but the name of enumeration is a macro argument. This is indicated in the documentation.

[Eugene.Zelenko]

Please mention new check in Release Notes.

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-scoped-enums-over-unscoped.rst

22 ↗

(On Diff #284531)

Please enclose enum in double back-ticks.

65 ↗

(On Diff #284531)

Please add newline.

[njames93]

Is 'over-unscoped' really needed in the name, would just 'prefer-scoped-enum' be better, WDYT?

For the case of macro, you can check if the location is a macro using SourceLocation::isMacroID().

For this to work you would also need to check every single usage of the the values in the enum to make sure they are converted to use the scoped access.
You're best bet would be to actually store a map indexed by unscoped enum decls with a set of all their locations and maybe a flag to say if the fix can be applied or not.
For instance a fix couldn't be applied if any of the usages or decls are in macros.
This map could then be checked using the endOfTranslationUnit virtual method, with all the diags and fixes being spawned there.

clang-tools-extra/clang-tidy/cppcoreguidelines/PreferScopedEnumsOverUnscopedCheck.cpp

31–32 ↗

(On Diff #284531)

Please hoist this logic into the matcher.

enumDecl(unless(isScoped())).bind("enumDecls")`

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-scoped-enums-over-unscoped.rst

20 ↗

(On Diff #284531)

Semi-colon after the struct definition.

29 ↗

(On Diff #284531)

Semi-colon after the struct definition.

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-scoped-enums-over-unscoped.cpp

27–28 ↗

(On Diff #284531)

nit:

[janosbenjaminantal]

In D85697#2209272, @njames93 wrote:

Is 'over-unscoped' really needed in the name, would just 'prefer-scoped-enum' be better, WDYT?

For the case of macro, you can check if the location is a macro using SourceLocation::isMacroID().

For this to work you would also need to check every single usage of the the values in the enum to make sure they are converted to use the scoped access.
You're best bet would be to actually store a map indexed by unscoped enum decls with a set of all their locations and maybe a flag to say if the fix can be applied or not.
For instance a fix couldn't be applied if any of the usages or decls are in macros.
This map could then be checked using the endOfTranslationUnit virtual method, with all the diags and fixes being spawned there.

Yes, it is better without 'over-unscoped'. I will change this.

I checked how other checks are dealing with macros, but I haven't found anything useful. Your idea sounds good, I will try to implement it.

• Fix not just the declarations, but also the enum usages
• Fix only the enums that are not declared/used in macros

As I am totally new to llvm, it might take a few days to come with a proper solutions, but I will do my best.

[janosbenjaminantal]

I addressed all of the review comments. Apart from the small fixes I extended a checker logic.

The check collects the following information from every unscoped enum (into the FoundEnumInfo struct):

1. The enum's definition: Every information from an enum is attached to the enum definition. If an enum is not defined in the translation unit, then the first forward declaration will represent the enum.
2. Whether the enum definition can be fixed: if the enum definition appears in a macro, then it cannot be fixed. otherwise it can be fixed by inserting the class keyword into the definition.
3. Fixable usages of enum values: these are the usages that can be fixed by inserting the right qualifier.
4. Fixable forward declarations of the enum: these can and must be fixed by inserting the class keyword similarly to the definition.
5. Usages that cannot be fixed: these are the usages that are in a macro.
6. Forward declarations that cannot be fixed: similarly to the unfixable usages, these are in a macro.
7. Implicit casts: the implicit casts prevent the enum from being fixed, because the scoped enumerations cannot be implicitly casted to numerical types.

If any of the definition/usages/forward declarations/implicit casts prevent the enum from being fixed, a note is prompted out for every occurrences to inform the users about this.

I though about more cases that can prevent an enum from being fixed, but I haven't found any other reason. If you know anything else, feel free to mention it.

I tried to come up with relevant, complex and stressful test cases. I already spot a few implementation bugs with them. If you miss anything, feel free to let me know.

The known "limitations" are mostly similar to other checks:

• It cannot detect unfixable cases from other translation units. Practically that means if an enum is used in multiple source files, one of them might end up not fixed. I tried to work around this, but I haven't found any solution for this, moreover this cause problems for other checks also. Therefore I think it shouldn't be a blocking issue.
• It doesn't take into account if an enum is defined in a header that is filtered out. Similarly to the previous one, I tried to find a solution for this, but I was unable (the ClangTidyCheck class can access the header filter information, but it doesn't expose it for the descendant classes). I also checked other checks, and they behave in the same way. Therefore I also think it is shouldn't be a blocking issue.
• The checks doesn't take into consideration the aliases in the diagnostic messages and fixes: always the original name is showed/inserted. I checked other checks also, and for the diagnostics message they also show the original name. I didn't find a check where the alias can be relevant when the fixes are applied. This issue might be a problem, however in my opinion it is not serious issue. Either it is okay for you or not, if you know how it could be improved, please provide me some points where to start searching. Even if this patch is got approved, I plan to improve it in the future and this point is a good things to improve.

It is not strongly connected to this review, but in the future I am planning to extend the check with:

• options to exclude enums, because changing them to scoped enumerations might not be suitable for every cases
• options to force the doable fixes: based on my little experience it might be easier to force the doable fixes and manually fix the remaining ones

If you have any opinion, thoughts or counter-argument against the planned extensions, please let me know.

[janosbenjaminantal]

Fix clang-tidy issues.

[njames93]

It may be a pain, but I'd also like to see an option like RemovePrefix.
Generally most unscoped enums constants have prefixes based on the enum name. A simple way around this would just be to get the longest common prefix in all the enums constants. There is probably some other heuristics that could be used like maybe stop after you reach an underscore or change of case but probably not essential.

clang-tools-extra/docs/ReleaseNotes.rst

82

Could this perhaps be a little more informative?

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-scoped-enums.rst

35–36

I'd prefer if this was user controlled, An option like UseEnumStruct that defaults to false. Just because its mainly used, doesn't mean there aren't some projects that prefer to use enum struct.

[njames93]

In D85697#2234468, @janosbenjaminantal wrote:

The known "limitations" are mostly similar to other checks:

• It cannot detect unfixable cases from other translation units. Practically that means if an enum is used in multiple source files, one of them might end up not fixed. I tried to work around this, but I haven't found any solution for this, moreover this cause problems for other checks also. Therefore I think it shouldn't be a blocking issue.

That's what the run_clang_tidy.py script is meant to handle.

• It doesn't take into account if an enum is defined in a header that is filtered out. Similarly to the previous one, I tried to find a solution for this, but I was unable (the ClangTidyCheck class can access the header filter information, but it doesn't expose it for the descendant classes). I also checked other checks, and they behave in the same way. Therefore I also think it is shouldn't be a blocking issue.

I recently pushed an upgrade to readability-identifier-naming where it would check the naming style for identifiers declared in header files, maybe thats something this could also use, this is the commit 4888c9ce97d8

It is not strongly connected to this review, but in the future I am planning to extend the check with:

• options to exclude enums, because changing them to scoped enumerations might not be suitable for every cases

Not strictly necessary, if people don't want the fix they could annotate the code with a // NOLINT(*prefer-unscoped-enums) comment.

• options to force the doable fixes: based on my little experience it might be easier to force the doable fixes and manually fix the remaining ones

Forcing the fix is usually just a case of converting implicit cast usages of the constants into static casts.

[aaron.ballman]

I tend to be very skeptical of the value of checks that basically throw out entire usable chunks of the base language because the check is effectively impossible to apply to an existing code base. Have you run the check over large C++ code bases to see just how often the diagnostic triggers and whether there are ways we might want to reduce false-positives that the C++ Core Guidelines haven't considered as part of their enforcement strategy?

Btw, one fear I have with this check is that the automated fixits are somewhat risky -- unscoped vs scoped enumerations has ABI implications and changing from one to the other may break the ABI.

clang-tools-extra/clang-tidy/cppcoreguidelines/PreferScopedEnumsCheck.cpp

22

FWIW, Clang accepts scoped enumerations as a conforming language extension in older language modes. Should this check be enabled for any C++ mode?

26

Drop top-level const qualifiers on things that are not pointers or references (it's not a pattern we typically use in the project). This applies elsewhere in the patch as well.

28

This should be checking whether the enumeration comes from a system header or not -- we should definitely not warn about enumerations the user cannot change.

91

This feels a bit like it's a user option given that enum struct is also used. I think using class is a sensible default, though.

94–95

Please do not use auto when the type is not spelled out in the initialization (or is blindingly obvious from context, like iterators, or is unutterable, like lambdas).

124

I would drop the top-level const pointer qualification (elsewhere as well). The object being pointed at is fine, I'm only suggesting removing the qualification for the pointer itself.

134

llvm::for_each()? This may actually shorten many of the loops in this function.

167

Please don't use auto here.

194

Why are you using the internal interfaces here?

clang-tools-extra/clang-tidy/cppcoreguidelines/PreferScopedEnumsCheck.h

33

Does this need to be public?

38–42

Please remove the unnecessary empty initializers.

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-scoped-enums.rst

3

Underlining looks to be the wrong length.

35–36

+1. Also, we should document when the check doesn't trigger (like macros or system headers), discuss forward reference behavior, etc.

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-scoped-enums.cpp

33

Another interesting test case:

enum BigEnumConstants {
  TooBig = 0xFFFF'FFFF'0
};

Changing this unscoped enumeration into a scoped enumeration without specifying the fixed underlying type will break code in this case because the enum constant value is too large to be represented by an int.

165

What should happen in cases like:

// A case where the user has manually added a prefix they want; it
// seems like fixing this to use a scoped enumeration changes the
// expected way you interface with the enumeration by name.
namespace EnumPrefix {
enum Whatever {
  One,
  Two
};
}

// Another form of the same idea above.
struct AnotherPrefix {
  enum Whatever {
    One,
    Two
  };
};

// What about use in class hierarchies?
// Header file the user controls
enum SomeEnum {
  One,
  Two
};

struct SomeType {
  virtual void func(SomeEnum E) = 0; // Changing this may break the hierarchy
};

// Here's another situation where even warning is a bit suspect
enum E : int;
extern void func(E);

[njames93]

In D85697#2339055, @aaron.ballman wrote:

I tend to be very skeptical of the value of checks that basically throw out entire usable chunks of the base language because the check is effectively impossible to apply to an existing code base. Have you run the check over large C++ code bases to see just how often the diagnostic triggers and whether there are ways we might want to reduce false-positives that the C++ Core Guidelines haven't considered as part of their enforcement strategy?

I disagree with this mentality, This is following what the core guideline states perfectly. If a codebase doesn't want to adhere to those guidelines, they don't need to run this check. If you think there are shortcomings in the guidelines, raise an issue on their github.

clang-tools-extra/clang-tidy/cppcoreguidelines/PreferScopedEnumsCheck.cpp

22

If you go down that route, you lose portability as the transformed code will only compile on versions of clang, Could be option controlled I guess, WDYT?

28

This should be checking whether the enumeration comes from a system header or not -- we should definitely not warn about enumerations the user cannot change.

Won't clang-tidy handle this by suppressing system header diagnostics, I thought that's what the feature was there for.

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-scoped-enums.cpp

165

Maybe the check should only flag enumerations that appear at TU level, any other enumeration is kind of scoped already.

[janosbenjaminantal]

In D85697#2338249, @njames93 wrote:

In D85697#2234468, @janosbenjaminantal wrote:

The known "limitations" are mostly similar to other checks:

• It cannot detect unfixable cases from other translation units. Practically that means if an enum is used in multiple source files, one of them might end up not fixed. I tried to work around this, but I haven't found any solution for this, moreover this cause problems for other checks also. Therefore I think it shouldn't be a blocking issue.

That's what the run_clang_tidy.py script is meant to handle.

It is good to know, so it is not an issue.

• It doesn't take into account if an enum is defined in a header that is filtered out. Similarly to the previous one, I tried to find a solution for this, but I was unable (the ClangTidyCheck class can access the header filter information, but it doesn't expose it for the descendant classes). I also checked other checks, and they behave in the same way. Therefore I also think it is shouldn't be a blocking issue.

I recently pushed an upgrade to readability-identifier-naming where it would check the naming style for identifiers declared in header files, maybe thats something this could also use, this is the commit 4888c9ce97d8

Will check.

It is not strongly connected to this review, but in the future I am planning to extend the check with:

• options to exclude enums, because changing them to scoped enumerations might not be suitable for every cases

Not strictly necessary, if people don't want the fix they could annotate the code with a // NOLINT(*prefer-unscoped-enums) comment.

I think with the inline suppression the users should annotate every usage of the enum while with the option it would be enough to list the enum's name to ignore it from the whole check. However it is just an improvement, when I reach that point I will do further digging about this topic, how the suppression actually work etc.

• options to force the doable fixes: based on my little experience it might be easier to force the doable fixes and manually fix the remaining ones

Forcing the fix is usually just a case of converting implicit cast usages of the constants into static casts.

What I meant is an option to express that the user don't care if there are some cases for an enum that cannot be fixed: fixed the other occurrences and the user will handle the rest. Currently the automated fix only fix the enums where every occurrence can be fixed. As a second thought this might be superfluous, because the other way is always possible: the user first fix the occurrences that cannot be fixed automatically and then use the check to fix the rest. So I think it is something that wouldn't mean real value.

clang-tools-extra/clang-tidy/cppcoreguidelines/PreferScopedEnumsCheck.cpp

22

I think by default the check shouldn't do that (source codes that uses multiple compilers?), but it might worth an option to be able to turn it on on "clang-only" code bases.

91

I would like to keep this as small as possible, so I didn't planned to add this option as part of the first version. Do you think it is necessary to add this to approve the review? If yes, then I will add it of course, otherwise maybe in a different review in the future.

clang-tools-extra/clang-tidy/cppcoreguidelines/PreferScopedEnumsCheck.h

33

The original function in MatchCallback is public, so I think it is reasonable to have it as a public function here also.

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-scoped-enums.cpp

33

That's an excellent catch. I think comparing the size of int and the underlying type of the enum might help with some extra logic, because

enum class ValidWithLongLongInt {
  Value = 0x7FFF'FFFF'FFFF'FFFF
};

enum InvalidWithLongLongInt {
  Value = 0x8FFF'FFFF'FFFF'FFFF
};

The second one must use unsigned int. I think this is manageable and will have a look at it.

165

I think there is no good solution for the first two cases. We cannot be sure whether it is meant to be a prefix or not. It is just a very educated guess (if the the scope contains only one enum, then it is very like just a prefix, but might not be true in every cases). I don't have a very strong opinion about this, but I think this check can be useful even without supporting this case. Do you think it is a very common solution?

For the third one, I think if the fix is applied for every files, then it won't be a problem. If the fix cannot be applied to all of the files (e.g.: in case of a lib where the users can inherit from the classes defined in the lib) it will definitely break the hierarchy, however I don't think we can do anything about this. I am not totally sure, but I think this a problem for a lot of other checks too, e.g.: google-explicit-constructor or readability-identifier-naming. However I am afraid I misunderstood something, so let me in that case.

The last case with the extern function is a perfect place to suppress the warning in my opinion. How it could be decided, whether the user can change the function or not? Therefore I would keep the warning in that case and trust the users to suppress it if necessary.

[janosbenjaminantal]

In D85697#2339055, @aaron.ballman wrote:

I tend to be very skeptical of the value of checks that basically throw out entire usable chunks of the base language because the check is effectively impossible to apply to an existing code base. Have you run the check over large C++ code bases to see just how often the diagnostic triggers and whether there are ways we might want to reduce false-positives that the C++ Core Guidelines haven't considered as part of their enforcement strategy?

No, I haven't checked over large C++ code bases, but I will do this.

Btw, one fear I have with this check is that the automated fixits are somewhat risky -- unscoped vs scoped enumerations has ABI implications and changing from one to the other may break the ABI.

I am not familiar with these specific ABI implications, if you could help me with some keywords/references/link to start to investigate, I am happy to deep dive into it. I am also willing to discard the automated fixes if it makes this review better. What do you think?

[njames93]

In D85697#2340317, @janosbenjaminantal wrote:

In D85697#2338249, @njames93 wrote:

In D85697#2234468, @janosbenjaminantal wrote:

It is not strongly connected to this review, but in the future I am planning to extend the check with:

• options to exclude enums, because changing them to scoped enumerations might not be suitable for every cases

Not strictly necessary, if people don't want the fix they could annotate the code with a // NOLINT(*prefer-unscoped-enums) comment.

I think with the inline suppression the users should annotate every usage of the enum while with the option it would be enough to list the enum's name to ignore it from the whole check. However it is just an improvement, when I reach that point I will do further digging about this topic, how the suppression actually work etc.

Ah so an issue here is you emit a warning for the enum declaration as well as each usage and the NOLINT will only affect warnings emitted for the line its declared on.
Perhaps this needs a reshape.

Only emit a warning for the enum declaration and make sure the fix-its are attached to that warning, This is a safer way around this, and its also how RenamerClangTidy checks work. It means that if there is any conflict when trying to apply one of the fix-its none of them will be applied.
If you want to emit notes about usages that need updating you can, but don't attach fixes there, they wont be applied.

Due to how diagnostics works this will make it the code slightly more finicky, you can only have one DiagnosticBuilder active at once, and the notes need to be added after the warning has been emitted.

Maybe if it can be fixed, Emit a warning for the enum declaration, then add fix-its for the decl, any forward declares and any usages, then emit that diagnostic (Happens when the DiagnosticBuilder gets destroyed). After that you could emit notes for the forward declarations and usages(though I don't think we really needs notes for those)
If it can't be fixed its kind of similar, emit a warning for the declaration, then a note for each forward declaration and fault for why a fix can't be applied.
This has the added bonus that clang-tidy will output the diagnostics for an unscoped enum and all its uses next to each other.
If you want I've done the legwork to change the diagnostics behaviour to this and fix up the test cases if you want to have a look, should apply cleanly atop this version.

ScopedEnums.patch24 KBDownload

[aaron.ballman]

In D85697#2340258, @njames93 wrote:

In D85697#2339055, @aaron.ballman wrote:

I tend to be very skeptical of the value of checks that basically throw out entire usable chunks of the base language because the check is effectively impossible to apply to an existing code base. Have you run the check over large C++ code bases to see just how often the diagnostic triggers and whether there are ways we might want to reduce false-positives that the C++ Core Guidelines haven't considered as part of their enforcement strategy?

I disagree with this mentality, This is following what the core guideline states perfectly. If a codebase doesn't want to adhere to those guidelines, they don't need to run this check. If you think there are shortcomings in the guidelines, raise an issue on their github.

As with other C++ core guideline checks, the usual approach is for the patch authors to go back to the C++ Core Guideline folks with these issues to find out how they should be addressed. This has produced better C++ Core Guidelines and better checkers in the past and is the way I think we should continue to proceed. That said, I agree with you that we will eventually go with whatever the guideline authors want -- but this is us doing our due diligence since the authors put very little thought into enforcement of the rules in general, and is appropriate (for any guidelines, not just the C++ core guidelines). Note, this is in keeping with the guidelines themselves which state that they're not meant to subset the language and are intended to allow for gradual adoption of rules (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#innot-non-aims).

In D85697#2340381, @janosbenjaminantal wrote:

In D85697#2339055, @aaron.ballman wrote:

Btw, one fear I have with this check is that the automated fixits are somewhat risky -- unscoped vs scoped enumerations has ABI implications and changing from one to the other may break the ABI.

I am not familiar with these specific ABI implications, if you could help me with some keywords/references/link to start to investigate, I am happy to deep dive into it. I am also willing to discard the automated fixes if it makes this review better. What do you think?

I did some digging and I'm less certain I'm worried about this now. I was thinking that the mangling was different for these two cases, but it's the same mangling for func() either way: https://godbolt.org/z/xhcd7E The automated fixits still worry me in the places where they may take working code and change it to not be working (like narrowing conversions with the enumerator values, etc), but if those are addressed, then I think the fix-its are a benefit rather than a risk.

clang-tools-extra/clang-tidy/cppcoreguidelines/PreferScopedEnumsCheck.cpp

22

The impression I have from the guidelines is that they're intended for C++11 and later code bases as a way to improve the coding practices. They don't really seem to imply there's a requirement that code remains portable, but that sure seems like a reasonable goal to me. I think I'm fine leaving this disabled unless you're in C++11 mode, but we may want to see if there's a policy decision here for the core guideline checks (I have no idea how consistent we're being with considering older standards for these particular checks).

28

Good point! I forgot that clang-tidy already handled that automatically.

91

Given that two reviewers both thought it was worthwhile, I think it's probably not a bad idea to support as an option. It could be done in a follow-up patch if that's the way you'd like to proceed, though.

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-scoped-enums.cpp

165

Do you think it is a very common solution?

I think it likely is -- it was a common tactic used in pre-C++11 code bases (for instance, Clang uses this quite often, as in: https://github.com/llvm/llvm-project/blob/master/clang/include/clang/AST/Decl.h#L837)

For the third one, I think if the fix is applied for every files, then it won't be a problem.

Agreed -- the scenario I was thinking about is when the virtual hierarchy is part of a published interface, like in a plugin system. Changing the base declaration may be easy, but you may not even *see* the other derived classes. I'm not certain it's a solvable issue.

The last case with the extern function is a perfect place to suppress the warning in my opinion.

Agreed.

Maybe the check should only flag enumerations that appear at TU level, any other enumeration is kind of scoped already.

I was sort of wondering the same thing, but perhaps instead of TU level, I'd go with TU or anonymous namespace so that we still diagnose things like these:

namespace {
enum E { One }; // Seems reasonable to suggest scoping
namespace {
enum F { Two }; // Same here even though there are nested namespaces
}
}

[janosbenjaminantal]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-scoped-enums.cpp

165

I think the scoped vs unscoped enumeration is not about the scope itself, more likely the conversion rules applied to them: the unscoped enumerations are easily, even mistakenly convertible to/from ints. The core guideline reasons with this: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Renum-class

From that point of view, it is reasonable to convert the enums that are in their own namespaces also. Of course, it would be nice to eliminate the extra scope, but I don't think we can detect it. From my understanding, the only we could do is just check whether and enumeration is the only thing in the namespace, and if yes, then remove the extra namespace. However I think this is not very reliable.

I will try to find a pre C++11 projects which use this and check there how it could be solved.

[janosbenjaminantal]

clang-tools-extra/clang-tidy/cppcoreguidelines/PreferScopedEnumsCheck.cpp

91

Will implement this. Not sure whether in a follow-up patch or not. If it is just a few lines of code, then I will include here, otherwise I will create a follow-up patch.

[aaron.ballman]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-scoped-enums.cpp

165

From an academic and API perspective, I definitely agree that the conversion rules and the fixed underlying type for scoped enumerations are really valuable. However, I think it's not uncommon for developers to primarily be worried about how people access their interfaces and so scoped nature of the name is the property they're most interested in. e.g.,

struct Prefix {
  enum TypeName {
    Name,
    OtherName
  };
};

Whether you access via Prefix::Name or Prefix::TypeName::Name matters for readability of code.

One possibility is that we make it a configuration option so the user can decide.

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

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_library(clangTidyCppCoreGuidelinesModule
   AvoidGotoCheck.cpp
   AvoidNonConstGlobalVariablesCheck.cpp
   CppCoreGuidelinesTidyModule.cpp
   InitVariablesCheck.cpp
   InterfacesGlobalInitCheck.cpp
   MacroUsageCheck.cpp
   NarrowingConversionsCheck.cpp
   NoMallocCheck.cpp
   OwningMemoryCheck.cpp
+  PreferScopedEnumsCheck.cpp
   ProBoundsArrayToPointerDecayCheck.cpp
   ProBoundsConstantArrayIndexCheck.cpp
   ProBoundsPointerArithmeticCheck.cpp
   ProTypeConstCastCheck.cpp
   ProTypeCstyleCastCheck.cpp
   ProTypeMemberInitCheck.cpp
   ProTypeReinterpretCastCheck.cpp
   ProTypeStaticCastDowncastCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

 #include "AvoidGotoCheck.h"
 #include "AvoidNonConstGlobalVariablesCheck.h"
 #include "InitVariablesCheck.h"
 #include "InterfacesGlobalInitCheck.h"
 #include "MacroUsageCheck.h"
 #include "NarrowingConversionsCheck.h"
 #include "NoMallocCheck.h"
 #include "OwningMemoryCheck.h"
+#include "PreferScopedEnumsCheck.h"
 #include "ProBoundsArrayToPointerDecayCheck.h"
 #include "ProBoundsConstantArrayIndexCheck.h"
 #include "ProBoundsPointerArithmeticCheck.h"
 #include "ProTypeConstCastCheck.h"
 #include "ProTypeCstyleCastCheck.h"
 #include "ProTypeMemberInitCheck.h"
 #include "ProTypeReinterpretCastCheck.h"
 #include "ProTypeStaticCastDowncastCheck.h"
CheckFactories.registerCheck<MacroUsageCheck>(
         "cppcoreguidelines-macro-usage");
     CheckFactories.registerCheck<NarrowingConversionsCheck>(
         "cppcoreguidelines-narrowing-conversions");
     CheckFactories.registerCheck<NoMallocCheck>("cppcoreguidelines-no-malloc");
     CheckFactories.registerCheck<misc::NonPrivateMemberVariablesInClassesCheck>(
         "cppcoreguidelines-non-private-member-variables-in-classes");
     CheckFactories.registerCheck<OwningMemoryCheck>(
         "cppcoreguidelines-owning-memory");
+    CheckFactories.registerCheck<PreferScopedEnumsCheck>(
+        "cppcoreguidelines-prefer-scoped-enums");
     CheckFactories.registerCheck<ProBoundsArrayToPointerDecayCheck>(
         "cppcoreguidelines-pro-bounds-array-to-pointer-decay");
     CheckFactories.registerCheck<ProBoundsConstantArrayIndexCheck>(
         "cppcoreguidelines-pro-bounds-constant-array-index");
     CheckFactories.registerCheck<ProBoundsPointerArithmeticCheck>(
         "cppcoreguidelines-pro-bounds-pointer-arithmetic");
     CheckFactories.registerCheck<ProTypeConstCastCheck>(
         "cppcoreguidelines-pro-type-const-cast");

clang-tools-extra/clang-tidy/cppcoreguidelines/PreferScopedEnumsCheck.h

+//===--- PreferScopedEnumsCheck.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_CPPCOREGUIDELINES_PREFERSCOPEDENUMSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PREFERSCOPEDENUMSCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseMap.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Checks Enum.3:
+/// Flags every unscoped enumeration declaration.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-prefer-scoped-enums.html
+class PreferScopedEnumsCheck : public ClangTidyCheck {
+public:
+  PreferScopedEnumsCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+  void onEndOfTranslationUnit() override;
+
+private:
+  struct FoundEnumInfo {
+    bool DeclCanBeFixed{true};
+    std::vector<const DeclRefExpr *> Usages{};
+    std::vector<const EnumDecl *> ForwardDecls{};
+    std::vector<const DeclRefExpr *> UsagesPreventFix{};
+    std::vector<const EnumDecl *> ForwardDeclsPreventFix{};
+    std::vector<const ImplicitCastExpr *> ImplicitCasts{};
+  };
+
+  void handleUnscopedEnum(const EnumDecl *UnscopedEnumDecl);
+  void handleUnscopedEnumUsage(const EnumDecl *UnscopedEnumDecl,
+                               const DeclRefExpr *EnumValueRef);
+
+  llvm::DenseMap<const EnumDecl *, FoundEnumInfo> UnscopedEnumInfos;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PREFERSCOPEDENUMSCHECK_H

clang-tools-extra/clang-tidy/cppcoreguidelines/PreferScopedEnumsCheck.cpp

+//===--- PreferScopedEnumsCheck.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 "PreferScopedEnumsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/SmallVector.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+bool PreferScopedEnumsCheck::isLanguageVersionSupported(
+    const LangOptions &LangOpts) const {
+  return LangOpts.CPlusPlus11;
+}
+
+void PreferScopedEnumsCheck::registerMatchers(MatchFinder *Finder) {
+  const auto UnscopedEnumDecl = []() { return enumDecl(unless(isScoped())); };
+
+  Finder->addMatcher(UnscopedEnumDecl().bind("enumDecl"), this);
+
+  Finder->addMatcher(
+      declRefExpr(allOf(hasType(UnscopedEnumDecl().bind("declRefTypeDecl")),
+                        hasDeclaration(enumConstantDecl())))
+          .bind("declRef"),
+      this);
+
+  Finder->addMatcher(
+      implicitCastExpr(allOf(hasSourceExpression(hasType(
+                                 UnscopedEnumDecl().bind("sourceEnumDecl"))),
+                             anyOf(hasCastKind(CK_IntegralCast),
+                                   hasCastKind(CK_IntegralToBoolean),
+                                   hasCastKind(CK_IntegralToFloating))))
+          .bind("implicitCast"),
+      this);
+}
+
+void PreferScopedEnumsCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *MatchedEnumDecl =
+          Result.Nodes.getNodeAs<EnumDecl>("enumDecl")) {
+    handleUnscopedEnum(MatchedEnumDecl);
+  } else if (const auto *MatchedEnumUsage =
+                 Result.Nodes.getNodeAs<DeclRefExpr>("declRef")) {
+    handleUnscopedEnumUsage(Result.Nodes.getNodeAs<EnumDecl>("declRefTypeDecl"),
+                            MatchedEnumUsage);
+  } else if (const auto *MatchedImplicitCast =
+                 Result.Nodes.getNodeAs<ImplicitCastExpr>("implicitCast")) {
+    const auto *SourceEnumDecl =
+        Result.Nodes.getNodeAs<EnumDecl>("sourceEnumDecl");
+    assert(UnscopedEnumInfos.count(SourceEnumDecl) == 1 &&
+           "The enum definition or a forward declaration must appear before an "
+           "implicit cast can happen'");
+    UnscopedEnumInfos[SourceEnumDecl].ImplicitCasts.push_back(
+        MatchedImplicitCast);
+  }
+}
+
+void PreferScopedEnumsCheck::onEndOfTranslationUnit() {
+  const auto DiagWarning = [this](SourceLocation Location,
+                                  const EnumDecl *UnscopedEnumDecl) {
+    return diag(Location, "enumeration %0 is not a scoped enumeration")
+           << UnscopedEnumDecl;
+  };
+
+  const auto DiagDeclWarning =
+      [&DiagWarning](const EnumDecl *UnscopedEnumDecl) {
+        return DiagWarning(UnscopedEnumDecl->getLocation(), UnscopedEnumDecl);
+      };
+
+  const auto DiagUsageWarning =
+      [&DiagWarning](const DeclRefExpr *EnumUsage,
+                     const EnumDecl *UnscopedEnumDecl) {
+        return DiagWarning(EnumUsage->getLocation(), UnscopedEnumDecl);
+      };
+
+  const auto CanBeFixed = [](const FoundEnumInfo &EnumInfo) {
+    return EnumInfo.DeclCanBeFixed && EnumInfo.UsagesPreventFix.empty() &&
+           EnumInfo.ForwardDeclsPreventFix.empty() &&
+           EnumInfo.ImplicitCasts.empty();
+  };
+
+  llvm::SmallString<128> Buffer;
+  const StringRef ClassInsertion{"class "};
+
+  for (const auto &EnumDeclAndInfo : UnscopedEnumInfos) {
+    const auto *UnscopedEnumDecl = EnumDeclAndInfo.first;
+    const auto &UnscopedEnumInfo = EnumDeclAndInfo.second;
+
+    if (CanBeFixed(UnscopedEnumInfo)) {
+      DiagDeclWarning(UnscopedEnumDecl) << FixItHint::CreateInsertion(
+          UnscopedEnumDecl->getLocation(), ClassInsertion);
+
+      for (const auto *ForwardDecl : UnscopedEnumInfo.ForwardDecls)
+        DiagDeclWarning(ForwardDecl) << FixItHint::CreateInsertion(
+            ForwardDecl->getLocation(), ClassInsertion);
+
+      const auto FixInsertion =
+          (UnscopedEnumDecl->getName() + "::").toStringRef(Buffer);
+      for (const auto *EnumUsage : UnscopedEnumInfo.Usages) {
+        DiagUsageWarning(EnumUsage, UnscopedEnumDecl)
+            << FixItHint::CreateInsertion(EnumUsage->getLocation(),
+                                          FixInsertion);
+      }
+      Buffer.clear();
+    } else {
+      DiagDeclWarning(UnscopedEnumDecl);
+
+      if (!UnscopedEnumInfo.DeclCanBeFixed) {
+        diag(UnscopedEnumDecl->getLocation(),
+             "%select{forward declaration|definition}0 in macro prevents %1 "
+             "from being fixed",
+             DiagnosticIDs::Note)
+            << UnscopedEnumDecl->isCompleteDefinition() << UnscopedEnumDecl;
+      }
+
+      for (const auto *const EnumUsage : UnscopedEnumInfo.Usages)
+        DiagUsageWarning(EnumUsage, UnscopedEnumDecl);
+
+      for (const auto *const EnumUsage : UnscopedEnumInfo.UsagesPreventFix) {
+        DiagUsageWarning(EnumUsage, UnscopedEnumDecl);
+        diag(EnumUsage->getBeginLoc(),
+             "usage in macro prevents %0 from being fixed", DiagnosticIDs::Note)
+            << UnscopedEnumDecl;
+      }
+
+      for (const auto *const ForwardDecl : UnscopedEnumInfo.ForwardDecls)
+        DiagDeclWarning(ForwardDecl);
+
+      for (const auto *const ForwardDecl :
+           UnscopedEnumInfo.ForwardDeclsPreventFix) {
+        DiagDeclWarning(ForwardDecl);
+        diag(ForwardDecl->getBeginLoc(),
+             "forward declaration in macro prevents %0 from being fixed",
+             DiagnosticIDs::Note)
+            << UnscopedEnumDecl;
+      }
+
+      for (const auto *const ImplicitCast : UnscopedEnumInfo.ImplicitCasts) {
+        diag(ImplicitCast->getBeginLoc(),
+             "implicit cast prevents %0 from being fixed", DiagnosticIDs::Note)
+            << UnscopedEnumDecl;
+      }
+    }
+  }
+}
+
+void PreferScopedEnumsCheck::handleUnscopedEnum(
+    const EnumDecl *UnscopedEnumDecl) {
+  if (UnscopedEnumDecl->isCompleteDefinition()) {
+    assert(
+        (UnscopedEnumInfos.count(UnscopedEnumDecl) == 0 ||
+         !UnscopedEnumInfos[UnscopedEnumDecl].ForwardDecls.empty() ||
+         !UnscopedEnumInfos[UnscopedEnumDecl].ForwardDeclsPreventFix.empty()) &&
+        "EnumDecl cannot appear twice without "
+        "forward declarations");
+    UnscopedEnumInfos[UnscopedEnumDecl].DeclCanBeFixed =
+        UnscopedEnumDecl->getLocation().isFileID();
+  } else if (const auto *Definition = UnscopedEnumDecl->getDefinition()) {
+    auto &EnumInfo = UnscopedEnumInfos[Definition];
+
+    if (UnscopedEnumDecl->getLocation().isFileID())
+      EnumInfo.ForwardDecls.push_back(UnscopedEnumDecl);
+    else
+      EnumInfo.ForwardDeclsPreventFix.push_back(UnscopedEnumDecl);
+  } else {
+    // Unsatisfied forward declaration
+    UnscopedEnumInfos[UnscopedEnumDecl].DeclCanBeFixed =
+        UnscopedEnumDecl->getLocation().isFileID();
+  }
+}
+
+void PreferScopedEnumsCheck::handleUnscopedEnumUsage(
+    const EnumDecl *UnscopedEnumDecl, const DeclRefExpr *EnumValueRef) {
+  assert(UnscopedEnumInfos.find(UnscopedEnumDecl) != UnscopedEnumInfos.end() &&
+         "The usage must appear after the EnumDecl is handled");
+
+  const bool NeedsToBeFixed = [UnscopedEnumDecl, EnumValueRef]() {
+    if (!EnumValueRef->hasQualifier())
+      return true;
+
+    auto *const Qualifier = EnumValueRef->getQualifier();
+    if (Qualifier->getKind() != NestedNameSpecifier::SpecifierKind::TypeSpec)
+      return true;
+
+    return UnscopedEnumDecl->getTypeForDecl()->getCanonicalTypeInternal() !=
+           Qualifier->getAsType()->getCanonicalTypeInternal();
+  }();
+
+  if (!NeedsToBeFixed) {
+    return;
+  }
+
+  auto &EnumInfo = UnscopedEnumInfos[UnscopedEnumDecl];
+  if (EnumValueRef->getLocation().isFileID())
+    EnumInfo.Usages.push_back(EnumValueRef);
+  else
+    EnumInfo.UsagesPreventFix.push_back(EnumValueRef);
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 - Improved :doc:`readability-identifier-naming
   <clang-tidy/checks/readability-identifier-naming>` check.
 
   Added an option `GetConfigPerFile` to support including files which use
   different naming styles.
 
+- New :doc:`cppcoreguidelines-prefer-scoped-enums
+  <clang-tidy/checks/cppcoreguidelines-prefer-scoped-enums>` check.
+
+  Checks for unscoped enumerations.
+
+
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 
 Improvements to clang-include-fixer
 -----------------------------------
 

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-prefer-scoped-enums.rst

+.. title:: clang-tidy - cppcoreguidelines-prefer-scoped-enums
+
+cppcoreguidelines-prefer-scoped-enums
+===================================================
+
+Values of unscoped enumerations are implicitly-convertible to integral types.
+To avoid such unwanted conversions, use scoped enumerations.
+
+This check implements 
+`Enum.3 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Renum-class>`_ 
+from the CppCoreGuidelines.
+
+Example:
+
+.. code-block:: c++
+
+  enum Foo {
+    A,
+    B
+  };
+
+  Foo dummyValue = A;
+
+After the fix is applied, the ``enum`` will become:
+
+.. code-block:: c++
+
+  enum class Foo {
+    A,
+    B
+  };
+
+  Foo dummyValue = Foo::A;
+
+Note: Although ``enum struct`` and ``enum class`` are exactly equivalent, the
+latter is used mainly.

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

.. csv-table::
    `cppcoreguidelines-avoid-goto <cppcoreguidelines-avoid-goto.html>`_,
    `cppcoreguidelines-avoid-non-const-global-variables <cppcoreguidelines-avoid-non-const-global-variables.html>`_,
    `cppcoreguidelines-init-variables <cppcoreguidelines-init-variables.html>`_, "Yes"
    `cppcoreguidelines-interfaces-global-init <cppcoreguidelines-interfaces-global-init.html>`_,
    `cppcoreguidelines-macro-usage <cppcoreguidelines-macro-usage.html>`_,
    `cppcoreguidelines-narrowing-conversions <cppcoreguidelines-narrowing-conversions.html>`_,
    `cppcoreguidelines-no-malloc <cppcoreguidelines-no-malloc.html>`_,
    `cppcoreguidelines-owning-memory <cppcoreguidelines-owning-memory.html>`_,
+   `cppcoreguidelines-prefer-scoped-enums <cppcoreguidelines-prefer-scoped-enums.html>`_, "Yes"
    `cppcoreguidelines-pro-bounds-array-to-pointer-decay <cppcoreguidelines-pro-bounds-array-to-pointer-decay.html>`_,
    `cppcoreguidelines-pro-bounds-constant-array-index <cppcoreguidelines-pro-bounds-constant-array-index.html>`_, "Yes"
    `cppcoreguidelines-pro-bounds-pointer-arithmetic <cppcoreguidelines-pro-bounds-pointer-arithmetic.html>`_,
    `cppcoreguidelines-pro-type-const-cast <cppcoreguidelines-pro-type-const-cast.html>`_,
    `cppcoreguidelines-pro-type-cstyle-cast <cppcoreguidelines-pro-type-cstyle-cast.html>`_, "Yes"
    `cppcoreguidelines-pro-type-member-init <cppcoreguidelines-pro-type-member-init.html>`_, "Yes"
    `cppcoreguidelines-pro-type-reinterpret-cast <cppcoreguidelines-pro-type-reinterpret-cast.html>`_,
    `cppcoreguidelines-pro-type-static-cast-downcast <cppcoreguidelines-pro-type-static-cast-downcast.html>`_, "Yes"

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-scoped-enums.cpp

+// RUN: %check_clang_tidy %s cppcoreguidelines-prefer-scoped-enums %t --
+
+enum ForwardDeclaredUnscopedEnumUnsatisfied : int;
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: enumeration 'ForwardDeclaredUnscopedEnumUnsatisfied' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+// CHECK-FIXES: {{^}}enum class ForwardDeclaredUnscopedEnumUnsatisfied : int;{{$}}
+
+enum ForwardDeclaredUnscopedEnum : int;
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: enumeration 'ForwardDeclaredUnscopedEnum' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+// CHECK-FIXES: {{^}}enum class ForwardDeclaredUnscopedEnum : int;{{$}}
+
+enum ForwardDeclaredUnscopedEnum : int {
+  // CHECK-NOTES: :[[@LINE-1]]:6: warning: enumeration 'ForwardDeclaredUnscopedEnum' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+  // CHECK-FIXES: {{^}}enum class ForwardDeclaredUnscopedEnum : int {{{$}}
+  FWUE_FirstValue,
+  FWUE_SecondValue,
+};
+
+const auto unqualifiedFWUE = FWUE_FirstValue;
+// CHECK-NOTES: :[[@LINE-1]]:30: warning: enumeration 'ForwardDeclaredUnscopedEnum' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+// CHECK-FIXES: {{^}}const auto unqualifiedFWUE = ForwardDeclaredUnscopedEnum::FWUE_FirstValue;{{$}}
+
+const auto qualifiedFWUE = ForwardDeclaredUnscopedEnum::FWUE_SecondValue;
+
+using AliasedFDUE = ForwardDeclaredUnscopedEnum;
+const auto qualifiedAFWUE = AliasedFDUE::FWUE_SecondValue;
+
+enum UnscopedEnum {
+  // CHECK-NOTES: :[[@LINE-1]]:6: warning: enumeration 'UnscopedEnum' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+  // CHECK-FIXES: {{^}}enum class UnscopedEnum {{{$}}
+  UE_FirstValue,
+  UE_SecondValue,
+};
+
+auto qualifiedUE = UnscopedEnum::UE_FirstValue;
+
+auto unqualifiedUE = UE_FirstValue;
+// CHECK-NOTES: :[[@LINE-1]]:22: warning: enumeration 'UnscopedEnum' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+// CHECK-FIXES: {{^}}auto unqualifiedUE = UnscopedEnum::UE_FirstValue;{{$}}
+
+enum UnscopedEnumWithFixedUnderlyingType : char {
+  // CHECK-NOTES: :[[@LINE-1]]:6: warning: enumeration 'UnscopedEnumWithFixedUnderlyingType' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+  UEWFUT_FirstValue,
+  UEWFUT_SecondValue,
+};
+// CHECK-FIXES: {{^}}enum class UnscopedEnumWithFixedUnderlyingType : char {{{$}}
+
+enum OpaqueUnscopedEnum : char;
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: enumeration 'OpaqueUnscopedEnum' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+// CHECK-FIXES: {{^}}enum class OpaqueUnscopedEnum : char;{{$}}
+
+enum UnfixableBecauseCast {
+  // CHECK-NOTES: :[[@LINE-1]]:6: warning: enumeration 'UnfixableBecauseCast' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+  UBC_FirstValue,
+  UBC_SecondValue,
+};
+
+const int castedUBC = UBC_FirstValue;
+// CHECK-NOTES: :[[@LINE-1]]:23: warning: enumeration 'UnfixableBecauseCast' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+
+const auto binaryOrUBC = UBC_FirstValue | UBC_SecondValue;
+// CHECK-NOTES: :[[@LINE-1]]:26: warning: enumeration 'UnfixableBecauseCast' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+// CHECK-NOTES: :[[@LINE-2]]:43: warning: enumeration 'UnfixableBecauseCast' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+// notes about implicit casts are prompted after all of the warnings for the enums
+// CHECK-NOTES: :[[@LINE-7]]:23: note: implicit cast prevents 'UnfixableBecauseCast' from being fixed
+// CHECK-NOTES: :[[@LINE-5]]:26: note: implicit cast prevents 'UnfixableBecauseCast' from being fixed
+// CHECK-NOTES: :[[@LINE-6]]:43: note: implicit cast prevents 'UnfixableBecauseCast' from being fixed
+
+enum UnfixableBecauseCastDefaultArgument {
+  // CHECK-NOTES: :[[@LINE-1]]:6: warning: enumeration 'UnfixableBecauseCastDefaultArgument' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+  UBCDA_FirstValue,
+  UBCDA_SecondValue,
+};
+
+void foo(int baz = UBCDA_FirstValue);
+// CHECK-NOTES: :[[@LINE-1]]:20: warning: enumeration 'UnfixableBecauseCastDefaultArgument' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+// CHECK-NOTES: :[[@LINE-2]]:20: note: implicit cast prevents 'UnfixableBecauseCastDefaultArgument' from being fixed
+
+enum UnfixableBecauseCastTemplateArgument {
+  // CHECK-NOTES: :[[@LINE-1]]:6: warning: enumeration 'UnfixableBecauseCastTemplateArgument' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+  UBCTA_FirstValue,
+  UBCTA_SecondValue,
+};
+
+template <int Foo>
+struct TemplatedStruct {};
+
+using FirstAliasType = TemplatedStruct<UBCTA_FirstValue>;
+// CHECK-NOTES: :[[@LINE-1]]:40: warning: enumeration 'UnfixableBecauseCastTemplateArgument' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+// CHECK-NOTES: :[[@LINE-2]]:40: note: implicit cast prevents 'UnfixableBecauseCastTemplateArgument' from being fixed
+using SecondAliasType = TemplatedStruct<UnfixableBecauseCastTemplateArgument::UBCTA_SecondValue>;
+// CHECK-NOTES: :[[@LINE-1]]:41: note: implicit cast prevents 'UnfixableBecauseCastTemplateArgument' from being fixed
+
+enum UnfixableOpaqueBecauseCast : int;
+// CHECK-NOTES: :[[@LINE-1]]:6: warning: enumeration 'UnfixableOpaqueBecauseCast' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+
+void foo(UnfixableOpaqueBecauseCast lhs, UnfixableOpaqueBecauseCast rhs) {
+  const int baz = lhs | rhs;
+  // CHECK-NOTES: :[[@LINE-1]]:19: note: implicit cast prevents 'UnfixableOpaqueBecauseCast' from being fixed
+  // CHECK-NOTES: :[[@LINE-2]]:25: note: implicit cast prevents 'UnfixableOpaqueBecauseCast' from being fixed
+}
+
+enum UnfixableBecauseCastWithQualified {
+  // CHECK-NOTES: :[[@LINE-1]]:6: warning: enumeration 'UnfixableBecauseCastWithQualified' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+  UBCWQ_FirstValue,
+  UBCWQ_SecondValue,
+};
+
+const int castedUBCWQ = UnfixableBecauseCastWithQualified::UBCWQ_FirstValue;
+// CHECK-NOTES: :[[@LINE-1]]:25: note: implicit cast prevents 'UnfixableBecauseCastWithQualified' from being fixed
+
+#define FORWARD_DECLARE_UNSCOPED_DEFINED_IN_MACRO enum UnscopedDefinedInMacro : int
+
+FORWARD_DECLARE_UNSCOPED_DEFINED_IN_MACRO;
+// CHECK-NOTES: :[[@LINE-1]]:1: warning: enumeration 'UnscopedDefinedInMacro' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+// CHECK-NOTES: :[[@LINE-4]]:56: note: expanded from macro 'FORWARD_DECLARE_UNSCOPED_DEFINED_IN_MACRO'
+// CHECK-NOTES: :[[@LINE-3]]:1: note: forward declaration in macro prevents 'UnscopedDefinedInMacro' from being fixed
+// CHECK-NOTES: :[[@LINE-6]]:51: note: expanded from macro 'FORWARD_DECLARE_UNSCOPED_DEFINED_IN_MACRO'
+
+#define FORWARD_DECLARE(EnumName) enum EnumName : int
+
+FORWARD_DECLARE(UnscopedDefinedInMacro);
+// CHECK-NOTES: :[[@LINE-1]]:17: warning: enumeration 'UnscopedDefinedInMacro' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+// CHECK-NOTES: :[[@LINE-2]]:1: note: forward declaration in macro prevents 'UnscopedDefinedInMacro' from being fixed
+// CHECK-NOTES: :[[@LINE-5]]:35: note: expanded from macro 'FORWARD_DECLARE'
+
+#define CREATE_ENUM enum UnscopedDefinedInMacro : int { \
+  UDIM_FirstValue,                                      \
+  UDIM_SecondValue,                                     \
+}
+
+CREATE_ENUM;
+// CHECK-NOTES: :[[@LINE-1]]:1: warning: enumeration 'UnscopedDefinedInMacro' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+// CHECK-NOTES: :[[@LINE-7]]:26: note: expanded from macro 'CREATE_ENUM'
+// CHECK-NOTES: :[[@LINE-3]]:1: note: definition in macro prevents 'UnscopedDefinedInMacro' from being fixed
+// CHECK-NOTES: :[[@LINE-9]]:26: note: expanded from macro 'CREATE_ENUM'
+
+enum UnscopedUsedInMacro {
+  // CHECK-NOTES: :[[@LINE-1]]:6: warning: enumeration 'UnscopedUsedInMacro' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+  UUIM_FirstValue,
+  UUIM_SecondValue,
+};
+
+#define CREATE_VARIABLE(Name, Value) const auto Name = Value;
+
+CREATE_VARIABLE(UsageInMacro, UUIM_FirstValue);
+// CHECK-NOTES: :[[@LINE-1]]:31: warning: enumeration 'UnscopedUsedInMacro' is not a scoped enumeration [cppcoreguidelines-prefer-scoped-enums]
+// CHECK-NOTES: :[[@LINE-2]]:31: note: usage in macro prevents 'UnscopedUsedInMacro' from being fixed
+
+enum class ScopedEnumWithClass {
+  SEWC_FirstValue,
+  SEWC_SecondValue,
+};
+
+auto qualifiedSEWC = ScopedEnumWithClass::SEWC_FirstValue;
+
+enum struct ScopedEnumWithStruct {
+  SEWS_FirstValue,
+  SEWS_SecondValue,
+};
+
+auto qualifiedSEWS = ScopedEnumWithStruct::SEWS_FirstValue;
+
+enum class OpaqueScopedEnum;
+
+enum class OpaqueScopedEnumWithFixedUnderlyingType : unsigned;