aaron.ballman added a comment.

In https://reviews.llvm.org/D33672#1283778, @ZaMaZaN4iK wrote:

> Which other changes and/or approvals are required for merging into trunk?


There's one unresolved comment from earlier and a few very minor nits that I 
found, but I think this is ready to land otherwise. Do you need someone to 
commit on your behalf? If so, then please rebase off ToT and update the patch 
and someone here can commit for you.



================
Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:84-86
+          new BuiltinBug(this, "enum cast out of range",
+                         "the value provided to the cast expression is not in "
+                         "the valid range of values for the enum"));
----------------
NoQ wrote:
> > diagnostics should not be full sentences or grammatically correct, so drop 
> > the capitalization and full stop
> 
> No, in fact, Static Analyzer diagnostics are traditionally capitalized, 
> unlike other warnings, so i'm afraid this will need to be changed back >.< 
> Still no fullstop though.
This comment is still unresolved.


================
Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:96
+  // Get the value of the expression to cast.
+  const auto ValueToCastOptional =
+      C.getSVal(CE->getSubExpr()).getAs<DefinedOrUnknownSVal>();
----------------
`const auto *`


================
Comment at: test/Analysis/enum-cast-out-of-range.cpp:38
+struct S {
+    unscoped_unspecified_t E : 5;
+};
----------------
Formatting is off here.


https://reviews.llvm.org/D33672



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to