frederic-tingaud-sonarsource created this revision.
frederic-tingaud-sonarsource added a reviewer: dcoughlin.
Herald added a subscriber: martong.
Herald added a project: All.
frederic-tingaud-sonarsource requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Dead store detection automatically checks that an expression is a
CXXConstructor and skips it because of potential side effects. In C++17, with
guaranteed copy elision, this check can fail because we actually receive the
implicit cast of a CXXConstructor.
Most checks in the dead store analysis were already stripping all casts and
parenthesis and those that weren't were either forgotten (like the constructor)
or would not suffer from it, so this patch proposes to factorize the stripping.
It has an impact on where the dead store warning is reported in the case of an
explicit cast, from
auto a = static_cast<B>(A());
^~~~~~~~~~~~~~~~~~~
to
auto a = static_cast<B>(A());
^~~
which we think is an improvement.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D126534
Files:
clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
clang/test/Analysis/dead-stores.cpp
Index: clang/test/Analysis/dead-stores.cpp
===================================================================
--- clang/test/Analysis/dead-stores.cpp
+++ clang/test/Analysis/dead-stores.cpp
@@ -12,6 +12,10 @@
// RUN: -analyzer-checker=deadcode.DeadStores -Wno-unreachable-code \
// RUN: -verify=non-nested,nested %s
+// RUN: %clang_analyze_cc1 -fcxx-exceptions -fexceptions -fblocks -std=c++17 \
+// RUN: -analyzer-checker=deadcode.DeadStores -Wno-unreachable-code \
+// RUN: -verify=non-nested,nested %s
+
//===----------------------------------------------------------------------===//
// Basic dead store checking (but in C++ mode).
//===----------------------------------------------------------------------===//
@@ -52,6 +56,17 @@
return x;
}
+class TestConstructor {
+public:
+ TestConstructor(int &y);
+};
+void copy(int x) {
+ TestConstructor tc1 = x; // no-warning
+ TestConstructor tc2 = TestConstructor(x); // no-warning
+ TestConstructor tc3 = (TestConstructor(x)); // no-warning
+ TestConstructor tc4 = (TestConstructor)(x); // no-warning
+}
+
//===----------------------------------------------------------------------===//
// Dead store checking involving CXXTemporaryExprs
//===----------------------------------------------------------------------===//
Index: clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
===================================================================
--- clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
+++ clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
@@ -432,7 +432,7 @@
<array>
<dict>
<key>line</key><integer>139</integer>
- <key>col</key><integer>13</integer>
+ <key>col</key><integer>35</integer>
<key>file</key><integer>0</integer>
</dict>
<dict>
@@ -500,7 +500,7 @@
<array>
<dict>
<key>line</key><integer>144</integer>
- <key>col</key><integer>13</integer>
+ <key>col</key><integer>33</integer>
<key>file</key><integer>0</integer>
</dict>
<dict>
@@ -568,7 +568,7 @@
<array>
<dict>
<key>line</key><integer>145</integer>
- <key>col</key><integer>13</integer>
+ <key>col</key><integer>26</integer>
<key>file</key><integer>0</integer>
</dict>
<dict>
@@ -636,7 +636,7 @@
<array>
<dict>
<key>line</key><integer>146</integer>
- <key>col</key><integer>13</integer>
+ <key>col</key><integer>33</integer>
<key>file</key><integer>0</integer>
</dict>
<dict>
@@ -1046,7 +1046,7 @@
<array>
<dict>
<key>line</key><integer>150</integer>
- <key>col</key><integer>19</integer>
+ <key>col</key><integer>48</integer>
<key>file</key><integer>0</integer>
</dict>
<dict>
@@ -1114,7 +1114,7 @@
<array>
<dict>
<key>line</key><integer>151</integer>
- <key>col</key><integer>21</integer>
+ <key>col</key><integer>52</integer>
<key>file</key><integer>0</integer>
</dict>
<dict>
@@ -1182,7 +1182,7 @@
<array>
<dict>
<key>line</key><integer>152</integer>
- <key>col</key><integer>19</integer>
+ <key>col</key><integer>39</integer>
<key>file</key><integer>0</integer>
</dict>
<dict>
@@ -1250,7 +1250,7 @@
<array>
<dict>
<key>line</key><integer>153</integer>
- <key>col</key><integer>21</integer>
+ <key>col</key><integer>43</integer>
<key>file</key><integer>0</integer>
</dict>
<dict>
Index: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -103,8 +103,9 @@
static const Expr *
LookThroughTransitiveAssignmentsAndCommaOperators(const Expr *Ex) {
while (Ex) {
+ Ex = Ex->IgnoreParenCasts();
const BinaryOperator *BO =
- dyn_cast<BinaryOperator>(Ex->IgnoreParenCasts());
+ dyn_cast<BinaryOperator>(Ex);
if (!BO)
break;
BinaryOperatorKind Op = BO->getOpcode();
@@ -332,7 +333,6 @@
// This is a common form of defensive programming.
const Expr *RHS =
LookThroughTransitiveAssignmentsAndCommaOperators(B->getRHS());
- RHS = RHS->IgnoreParenCasts();
QualType T = VD->getType();
if (T.isVolatileQualified())
@@ -416,7 +416,7 @@
return;
if (const DeclRefExpr *DRE =
- dyn_cast<DeclRefExpr>(E->IgnoreParenCasts()))
+ dyn_cast<DeclRefExpr>(E))
if (const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
// Special case: check for initialization from constant
// variables.
@@ -450,8 +450,9 @@
bool isConstant(const InitListExpr *Candidate) const {
// We consider init list to be constant if each member of the list can be
// interpreted as constant.
- return llvm::all_of(Candidate->inits(),
- [this](const Expr *Init) { return isConstant(Init); });
+ return llvm::all_of(Candidate->inits(), [this](const Expr *Init) {
+ return isConstant(Init->IgnoreParenCasts());
+ });
}
/// Return true if the given expression can be interpreted as constant
@@ -461,7 +462,7 @@
return true;
// We should also allow defensive initialization of structs, i.e. { 0 }
- if (const auto *ILE = dyn_cast<InitListExpr>(E->IgnoreParenCasts())) {
+ if (const auto *ILE = dyn_cast<InitListExpr>(E)) {
return isConstant(ILE);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
