[clang-tools-extra] r343768 - [clang-tidy] Sequence statements with multiple parents correctly (PR39149)
Martin Bohme via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 4 04:36:39 PDT 2018
Author: mboehme
Date: Thu Oct 4 04:36:39 2018
New Revision: 343768
URL: http://llvm.org/viewvc/llvm-project?rev=343768&view=rev
Log:
[clang-tidy] Sequence statements with multiple parents correctly (PR39149)
Summary:
Before this fix, the bugprone-use-after-move check could incorrectly
conclude that a use and move in a function template were not sequenced.
For details, see
https://bugs.llvm.org/show_bug.cgi?id=39149
Reviewers: alexfh, hokein, aaron.ballman, JonasToth
Reviewed By: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52782
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.h
clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
Modified: clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp?rev=343768&r1=343767&r2=343768&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp Thu Oct 4 04:36:39 2018
@@ -102,8 +102,9 @@ bool UseAfterMoveFinder::find(Stmt *Func
if (!TheCFG)
return false;
- Sequence.reset(new ExprSequence(TheCFG.get(), Context));
- BlockMap.reset(new StmtToBlockMap(TheCFG.get(), Context));
+ Sequence =
+ llvm::make_unique<ExprSequence>(TheCFG.get(), FunctionBody, Context);
+ BlockMap = llvm::make_unique<StmtToBlockMap>(TheCFG.get(), Context);
Visited.clear();
const CFGBlock *Block = BlockMap->blockContainingStmt(MovingCall);
Modified: clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp?rev=343768&r1=343767&r2=343768&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.cpp Thu Oct 4 04:36:39 2018
@@ -63,8 +63,9 @@ bool isDescendantOrEqual(const Stmt *Des
}
}
-ExprSequence::ExprSequence(const CFG *TheCFG, ASTContext *TheContext)
- : Context(TheContext) {
+ExprSequence::ExprSequence(const CFG *TheCFG, const Stmt *Root,
+ ASTContext *TheContext)
+ : Context(TheContext), Root(Root) {
for (const auto &SyntheticStmt : TheCFG->synthetic_stmts()) {
SyntheticStmtSourceMap[SyntheticStmt.first] = SyntheticStmt.second;
}
@@ -99,6 +100,11 @@ bool ExprSequence::potentiallyAfter(cons
const Stmt *ExprSequence::getSequenceSuccessor(const Stmt *S) const {
for (const Stmt *Parent : getParentStmts(S, Context)) {
+ // If a statement has multiple parents, make sure we're using the parent
+ // that lies within the sub-tree under Root.
+ if (!isDescendantOrEqual(Parent, Root, Context))
+ continue;
+
if (const auto *BO = dyn_cast<BinaryOperator>(Parent)) {
// Comma operator: Right-hand side is sequenced after the left-hand side.
if (BO->getLHS() == S && BO->getOpcode() == BO_Comma)
Modified: clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.h?rev=343768&r1=343767&r2=343768&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.h (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/ExprSequence.h Thu Oct 4 04:36:39 2018
@@ -69,8 +69,8 @@ namespace utils {
class ExprSequence {
public:
/// Initializes this `ExprSequence` with sequence information for the given
- /// `CFG`.
- ExprSequence(const CFG *TheCFG, ASTContext *TheContext);
+ /// `CFG`. `Root` is the root statement the CFG was built from.
+ ExprSequence(const CFG *TheCFG, const Stmt *Root, ASTContext *TheContext);
/// Returns whether \p Before is sequenced before \p After.
bool inSequence(const Stmt *Before, const Stmt *After) const;
@@ -94,6 +94,7 @@ private:
const Stmt *resolveSyntheticStmt(const Stmt *S) const;
ASTContext *Context;
+ const Stmt *Root;
llvm::DenseMap<const Stmt *, const Stmt *> SyntheticStmtSourceMap;
};
Modified: clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp?rev=343768&r1=343767&r2=343768&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp Thu Oct 4 04:36:39 2018
@@ -1195,6 +1195,18 @@ void ifWhileAndSwitchSequenceInitDeclAnd
}
}
+// Some statements in templates (e.g. null, break and continue statements) may
+// be shared between the uninstantiated and instantiated versions of the
+// template and therefore have multiple parents. Make sure the sequencing code
+// handles this correctly.
+template <class> void nullStatementSequencesInTemplate() {
+ int c = 0;
+ (void)c;
+ ;
+ std::move(c);
+}
+template void nullStatementSequencesInTemplate<int>();
+
namespace PR33020 {
class D {
~D();
More information about the cfe-commits
mailing list