rsmith added inline comments.
================
Comment at: clang/include/clang/Parse/Parser.h:1873
+ ExprResult ParseInitializer(Decl *DeclForInitializer = nullptr) {
+ Actions.ExprEvalContexts.back().DeclForInitializer = DeclForInitializer;
+ ExprResult init;
----------------
This should be done by calling a function on Sema (add an
`ActOnStartDeclInitializer` or similar), not by directly modifying Sema's
internal state.
================
Comment at: clang/include/clang/Parse/Parser.h:1880
+ }
+ Actions.ExprEvalContexts.back().DeclForInitializer = nullptr;
+ return init;
----------------
Please add an assertion when you first set this that the old value was null.
================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2263
+ VarDecl *VD) {
+ AnalysisDeclContext AC(nullptr, VD);
+
----------------
Consider grabbing `VarDeclPossiblyUnreachableDiags[VD]` first and avoiding
constructing the `AnalysisDeclContext` at all in the (presumably overwhelmingly
common case) that there are no such diagnostics.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:11853
+ AnalysisWarnings.IssueWarningsForRegisteredVarDecl(var);
+
----------------
We should (presumably) only do this for file-scope variables. The initializers
for block-scope variables will be checked when checking the enclosing function
(if the variables' declarations are reachable at all).
================
Comment at: clang/lib/Sema/SemaExpr.cpp:4884
+ AnalysisWarnings.IssueWarningsForRegisteredVarDecl(Param);
+
----------------
We should only do this once, when we instantiate the default argument, rather
than once each time we use it. (Move this up to just before line 4856?)
================
Comment at: clang/lib/Sema/SemaExpr.cpp:16670
case ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed:
if (!Stmts.empty() && getCurFunctionOrMethodDecl()) {
+ FunctionScopes.back()->PossiblyUnreachableDiags.push_back(
----------------
This call to `getCurFunctionOrMethodDecl()` is wrong; it will skip over
lambda-expressions. (This is why you're still seeing diagnostics in file-scope
lambdas.) Something like this should work:
```
if (Stmts.empty()) {
Diag(Loc, PD);
return true;
}
if (FunctionScopeInfo *FSI = getCurFunction()) {
FunctionScopes.back()->PossiblyUnreachableDiags.push_back(
PossiblyUnreachableDiag(PD, Loc, Stmts));
return true;
}
// [handle VarDecl case]
```
================
Comment at: clang/lib/Sema/SemaExpr.cpp:16680-16682
+ if (VD->getDefinition()) {
+ VD = VD->getDefinition();
+ }
----------------
No braces here please. Also, this appears to be wrong: we want the declaration
with the initializer (which is always `VD`), not the definition (which might be
a different declaration for a static data member), don't we?
================
Comment at: clang/lib/Sema/SemaExpr.cpp:16690
break;
- // FIXME: For any other kind of variable, we should build a CFG for its
- // initializer and check whether the context in question is reachable.
+ // FIXME: Some cases aren't picked up by path analysis currently
+ if (!Stmts.empty() && VD->isFileVarDecl()) {
----------------
What does this fixme mean?
================
Comment at: clang/test/SemaTemplate/instantiate-static-var.cpp:9
static const T value = 10 / Divisor; // expected-error{{in-class initializer
for static data member is not a constant expression}}
+ //expected-warning@-1 {{division by zero is undefined}}
};
----------------
We should not warn here (because this initializer is required to be a constant
expression).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63889/new/
https://reviews.llvm.org/D63889
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
