carlosgalvezp added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/EnumToIntCheck.cpp:31
+ const auto *MatchedExpr = Result.Nodes.getNodeAs<Expr>("x");
+ diag(MatchedExpr->getBeginLoc(), "enum is implictly converted to an
integral")
+ << MatchedExpr->getSourceRange();
----------------
Check that this pointer is not nullptr, before dereferencing it. Typically we
do it like this in all other checks:
if (const auto* MatchedExpr = Result.Nodes.getNodeAs<Expr>("x")) {
...
}
If you don't like the extra nesting you can return early.
if (!MatchedExpr)
return;
================
Comment at: clang-tools-extra/clang-tidy/bugprone/EnumToIntCheck.cpp:34
+ auto Note = diag(MatchedExpr->getBeginLoc(),
+ "insert an explicit cast to silence this warning",
+ DiagnosticIDs::Note);
----------------
I think this text is redundant given the fix-it note. Also, I find "silence
this warning" a bit confusing, since in order to do that one would use
`NOLINT`. By adding the cast one is not just "silencing the warning", one is
fixing the code to be explicit.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/EnumToIntCheck.cpp:45
+ } else {
+ Note << FixItHint::CreateInsertion(MatchedExpr->getBeginLoc(), "(int)");
+ }
----------------
I don't think we can assume the type of the enum is `int`, see for example:
```
enum Foo : unsigned int
{
a,
b
};
void f(unsigned int);
int main()
{
f(a);
}
```
Even if there is no explicit underlying type, isn't the underlying type
implementation-defined?
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-int.cpp:1
+// RUN: %check_clang_tidy -std=c++11-or-later --fix-notes %s
bugprone-enum-to-int %t
+
----------------
Please add a test using an enum with fixed underlying type that is not "int".
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-int.cpp:3
+
+enum A { e1,
+ e2 };
----------------
carlosgalvezp wrote:
> Please use a consistent style
>
> enum A {
> e1,
> e2,
> };
Please add a test to verify the C-style cast fix.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone/enum-to-int.cpp:3-4
+
+enum A { e1,
+ e2 };
+
----------------
Please use a consistent style
enum A {
e1,
e2,
};
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129570/new/
https://reviews.llvm.org/D129570
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
