[PATCH] D129752: Thread safety analysis: Handle additional cast in scoped capability construction

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, efriedma.
Herald added a reviewer: NoQ.
Herald added a project: All.
aaronpuchert requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We might have a CK_NoOp cast and a further CK_ConstructorConversion.
As an optimization, drop some IgnoreParens calls: inside of the
CK_{Constructor,UserDefined}Conversion should be no more parentheses,
and inside the CXXBindTemporaryExpr should also be none.

Lastly, we factor out the unpacking so that we can reuse it for
MaterializeTemporaryExprs later on.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129752

Files:
  clang/lib/Analysis/ThreadSafety.cpp
  clang/test/SemaCXX/warn-thread-safety-analysis.cpp


Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp
===================================================================
--- clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -1683,6 +1683,13 @@
     a = 5;
   }
 
+#ifdef __cpp_guaranteed_copy_elision
+  void const_lock() {
+    const MutexLock mulock = MutexLock(&mu1);
+    a = 5;
+  }
+#endif
+
   void foo2() {
     ReaderMutexLock mulock1(&mu1);
     if (getBool()) {
Index: clang/lib/Analysis/ThreadSafety.cpp
===================================================================
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -2087,6 +2087,19 @@
                                   SourceRange(Loc, Loc));
 }
 
+static Expr *UnpackConstruction(Expr *E) {
+  if (auto *CE = dyn_cast<CastExpr>(E))
+    if (CE->getCastKind() == CK_NoOp)
+      E = CE->getSubExpr()->IgnoreParens();
+  if (auto *CE = dyn_cast<CastExpr>(E))
+    if (CE->getCastKind() == CK_ConstructorConversion ||
+        CE->getCastKind() == CK_UserDefinedConversion)
+      E = CE->getSubExpr();
+  if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E))
+    E = BTE->getSubExpr();
+  return E;
+}
+
 void BuildLockset::VisitDeclStmt(const DeclStmt *S) {
   // adjust the context
   LVarCtx = Analyzer->LocalVarMap.getNextContext(CtxIndex, S, LVarCtx);
@@ -2101,13 +2114,7 @@
       // handle constructors that involve temporaries
       if (auto *EWC = dyn_cast<ExprWithCleanups>(E))
         E = EWC->getSubExpr()->IgnoreParens();
-      if (auto *CE = dyn_cast<CastExpr>(E))
-        if (CE->getCastKind() == CK_NoOp ||
-            CE->getCastKind() == CK_ConstructorConversion ||
-            CE->getCastKind() == CK_UserDefinedConversion)
-          E = CE->getSubExpr()->IgnoreParens();
-      if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E))
-        E = BTE->getSubExpr()->IgnoreParens();
+      E = UnpackConstruction(E);
 
       if (const auto *CE = dyn_cast<CXXConstructExpr>(E)) {
         const auto *CtorD = dyn_cast_or_null<NamedDecl>(CE->getConstructor());



Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp
===================================================================
--- clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -1683,6 +1683,13 @@
     a = 5;
   }
 
+#ifdef __cpp_guaranteed_copy_elision
+  void const_lock() {
+    const MutexLock mulock = MutexLock(&mu1);
+    a = 5;
+  }
+#endif
+
   void foo2() {
     ReaderMutexLock mulock1(&mu1);
     if (getBool()) {
Index: clang/lib/Analysis/ThreadSafety.cpp
===================================================================
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -2087,6 +2087,19 @@
                                   SourceRange(Loc, Loc));
 }
 
+static Expr *UnpackConstruction(Expr *E) {
+  if (auto *CE = dyn_cast<CastExpr>(E))
+    if (CE->getCastKind() == CK_NoOp)
+      E = CE->getSubExpr()->IgnoreParens();
+  if (auto *CE = dyn_cast<CastExpr>(E))
+    if (CE->getCastKind() == CK_ConstructorConversion ||
+        CE->getCastKind() == CK_UserDefinedConversion)
+      E = CE->getSubExpr();
+  if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E))
+    E = BTE->getSubExpr();
+  return E;
+}
+
 void BuildLockset::VisitDeclStmt(const DeclStmt *S) {
   // adjust the context
   LVarCtx = Analyzer->LocalVarMap.getNextContext(CtxIndex, S, LVarCtx);
@@ -2101,13 +2114,7 @@
       // handle constructors that involve temporaries
       if (auto *EWC = dyn_cast<ExprWithCleanups>(E))
         E = EWC->getSubExpr()->IgnoreParens();
-      if (auto *CE = dyn_cast<CastExpr>(E))
-        if (CE->getCastKind() == CK_NoOp ||
-            CE->getCastKind() == CK_ConstructorConversion ||
-            CE->getCastKind() == CK_UserDefinedConversion)
-          E = CE->getSubExpr()->IgnoreParens();
-      if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E))
-        E = BTE->getSubExpr()->IgnoreParens();
+      E = UnpackConstruction(E);
 
       if (const auto *CE = dyn_cast<CXXConstructExpr>(E)) {
         const auto *CtorD = dyn_cast_or_null<NamedDecl>(CE->getConstructor());

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