Use DiagRuntimeBehavior for -Wunsequenced to weed out false positives

where either the modification or the other access is unreachable.

This reverts r359984 (which reverted r359962). The bug in clang-tidy's
test suite exposed by the original commit was fixed in r360009.

llvm-svn: 360010
This commit is contained in:
Richard Smith 2019-05-06 04:14:01 +00:00
parent 9d4bb51f57
commit 7d02ca4878
7 changed files with 43 additions and 23 deletions

View File

@ -84,11 +84,11 @@ class PossiblyUnreachableDiag {
public:
PartialDiagnostic PD;
SourceLocation Loc;
const Stmt *stmt;
llvm::TinyPtrVector<const Stmt*> Stmts;
PossiblyUnreachableDiag(const PartialDiagnostic &PD, SourceLocation Loc,
const Stmt *stmt)
: PD(PD), Loc(Loc), stmt(stmt) {}
ArrayRef<const Stmt *> Stmts)
: PD(PD), Loc(Loc), Stmts(Stmts) {}
};
/// Retains information about a function, method, or block that is

View File

@ -4252,6 +4252,10 @@ public:
/// If it is unreachable, the diagnostic will not be emitted.
bool DiagRuntimeBehavior(SourceLocation Loc, const Stmt *Statement,
const PartialDiagnostic &PD);
/// Similar, but diagnostic is only produced if all the specified statements
/// are reachable.
bool DiagRuntimeBehavior(SourceLocation Loc, ArrayRef<const Stmt*> Stmts,
const PartialDiagnostic &PD);
// Primary Expressions.
SourceRange getExprRange(Expr *E) const;

View File

@ -2089,16 +2089,16 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
// Register the expressions with the CFGBuilder.
for (const auto &D : fscope->PossiblyUnreachableDiags) {
if (D.stmt)
AC.registerForcedBlockExpression(D.stmt);
for (const Stmt *S : D.Stmts)
AC.registerForcedBlockExpression(S);
}
if (AC.getCFG()) {
analyzed = true;
for (const auto &D : fscope->PossiblyUnreachableDiags) {
bool processed = false;
if (D.stmt) {
const CFGBlock *block = AC.getBlockForRegisteredExpression(D.stmt);
bool AllReachable = true;
for (const Stmt *S : D.Stmts) {
const CFGBlock *block = AC.getBlockForRegisteredExpression(S);
CFGReverseBlockReachabilityAnalysis *cra =
AC.getCFGReachablityAnalysis();
// FIXME: We should be able to assert that block is non-null, but
@ -2106,15 +2106,17 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
// edge cases; see test/Sema/vla-2.c.
if (block && cra) {
// Can this block be reached from the entrance?
if (cra->isReachable(&AC.getCFG()->getEntry(), block))
S.Diag(D.Loc, D.PD);
processed = true;
if (!cra->isReachable(&AC.getCFG()->getEntry(), block)) {
AllReachable = false;
break;
}
}
// If we cannot map to a basic block, assume the statement is
// reachable.
}
if (!processed) {
// Emit the warning anyway if we cannot map to a basic block.
if (AllReachable)
S.Diag(D.Loc, D.PD);
}
}
}

View File

@ -12155,10 +12155,11 @@ class SequenceChecker : public EvaluatedExprVisitor<SequenceChecker> {
if (OtherKind == UK_Use)
std::swap(Mod, ModOrUse);
SemaRef.Diag(Mod->getExprLoc(),
IsModMod ? diag::warn_unsequenced_mod_mod
: diag::warn_unsequenced_mod_use)
<< O << SourceRange(ModOrUse->getExprLoc());
SemaRef.DiagRuntimeBehavior(
Mod->getExprLoc(), {Mod, ModOrUse},
SemaRef.PDiag(IsModMod ? diag::warn_unsequenced_mod_mod
: diag::warn_unsequenced_mod_use)
<< O << SourceRange(ModOrUse->getExprLoc()));
UI.Diagnosed = true;
}

View File

@ -16079,7 +16079,7 @@ void Sema::MarkDeclarationsReferencedInExpr(Expr *E,
/// behavior of a program, such as passing a non-POD value through an ellipsis.
/// Failure to do so will likely result in spurious diagnostics or failures
/// during overload resolution or within sizeof/alignof/typeof/typeid.
bool Sema::DiagRuntimeBehavior(SourceLocation Loc, const Stmt *Statement,
bool Sema::DiagRuntimeBehavior(SourceLocation Loc, ArrayRef<const Stmt*> Stmts,
const PartialDiagnostic &PD) {
switch (ExprEvalContexts.back().Context) {
case ExpressionEvaluationContext::Unevaluated:
@ -16095,9 +16095,9 @@ bool Sema::DiagRuntimeBehavior(SourceLocation Loc, const Stmt *Statement,
case ExpressionEvaluationContext::PotentiallyEvaluated:
case ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed:
if (Statement && getCurFunctionOrMethodDecl()) {
if (!Stmts.empty() && getCurFunctionOrMethodDecl()) {
FunctionScopes.back()->PossiblyUnreachableDiags.
push_back(sema::PossiblyUnreachableDiag(PD, Loc, Statement));
push_back(sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
return true;
}
@ -16122,6 +16122,12 @@ bool Sema::DiagRuntimeBehavior(SourceLocation Loc, const Stmt *Statement,
return false;
}
bool Sema::DiagRuntimeBehavior(SourceLocation Loc, const Stmt *Statement,
const PartialDiagnostic &PD) {
return DiagRuntimeBehavior(
Loc, Statement ? llvm::makeArrayRef(Statement) : llvm::None, PD);
}
bool Sema::CheckCallReturnType(QualType ReturnType, SourceLocation Loc,
CallExpr *CE, FunctionDecl *FD) {
if (ReturnType->isVoidType() || !ReturnType->isIncompleteType())

View File

@ -95,4 +95,11 @@ void test() {
_Alignof(++a) + ++a; // expected-warning {{extension}}
__builtin_constant_p(f(++a, 0)) ? f(f(++a, 0), f(++a, 0)) : 0;
if (0) ++a + ++a; // ok, unreachable
}
void g(const char *p, int n) {
// This resembles code produced by some macros in glibc's <string.h>.
__builtin_constant_p(p) && __builtin_constant_p(++n) && (++n + ++n);
}

View File

@ -486,8 +486,8 @@ int Foo<X>::Run() {
// cxx17-warning@-2 {{unsequenced modification and access to 'num'}}
foo(num++, num++);
// cxx11-warning@-1 2{{multiple unsequenced modifications to 'num'}}
// cxx17-warning@-2 2{{multiple unsequenced modifications to 'num'}}
// cxx11-warning@-1 {{multiple unsequenced modifications to 'num'}}
// cxx17-warning@-2 {{multiple unsequenced modifications to 'num'}}
return 1;
}