r322316 - Handle scoped_lockable objects being returned by value in C++17.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 11 14:13:57 PST 2018
Author: rsmith
Date: Thu Jan 11 14:13:57 2018
New Revision: 322316
URL: http://llvm.org/viewvc/llvm-project?rev=322316&view=rev
Log:
Handle scoped_lockable objects being returned by value in C++17.
In C++17, guaranteed copy elision means that there isn't necessarily a
constructor call when a local variable is initialized by a function call that
returns a scoped_lockable by value. In order to model the effects of
initializing a local variable with a function call returning a scoped_lockable,
pretend that the move constructor was invoked within the caller at the point of
return.
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=322316&r1=322315&r2=322316&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Thu Jan 11 14:13:57 2018
@@ -1689,7 +1689,7 @@ void BuildLockset::handleCall(Expr *Exp,
CapExprSet ScopedExclusiveReqs, ScopedSharedReqs;
StringRef CapDiagKind = "mutex";
- // Figure out if we're calling the constructor of scoped lockable class
+ // Figure out if we're constructing an object of scoped lockable class
bool isScopedVar = false;
if (VD) {
if (const CXXConstructorDecl *CD = dyn_cast<const CXXConstructorDecl>(D)) {
@@ -1991,6 +1991,33 @@ void BuildLockset::VisitCXXConstructExpr
// FIXME -- only handles constructors in DeclStmt below.
}
+static CXXConstructorDecl *
+findConstructorForByValueReturn(const CXXRecordDecl *RD) {
+ // Prefer a move constructor over a copy constructor. If there's more than
+ // one copy constructor or more than one move constructor, we arbitrarily
+ // pick the first declared such constructor rather than trying to guess which
+ // one is more appropriate.
+ CXXConstructorDecl *CopyCtor = nullptr;
+ for (CXXConstructorDecl *Ctor : RD->ctors()) {
+ if (Ctor->isDeleted())
+ continue;
+ if (Ctor->isMoveConstructor())
+ return Ctor;
+ if (!CopyCtor && Ctor->isCopyConstructor())
+ CopyCtor = Ctor;
+ }
+ return CopyCtor;
+}
+
+static Expr *buildFakeCtorCall(CXXConstructorDecl *CD, ArrayRef<Expr *> Args,
+ SourceLocation Loc) {
+ ASTContext &Ctx = CD->getASTContext();
+ return CXXConstructExpr::Create(Ctx, Ctx.getRecordType(CD->getParent()), Loc,
+ CD, true, Args, false, false, false, false,
+ CXXConstructExpr::CK_Complete,
+ SourceRange(Loc, Loc));
+}
+
void BuildLockset::VisitDeclStmt(DeclStmt *S) {
// adjust the context
LVarCtx = Analyzer->LocalVarMap.getNextContext(CtxIndex, S, LVarCtx);
@@ -1998,15 +2025,34 @@ void BuildLockset::VisitDeclStmt(DeclStm
for (auto *D : S->getDeclGroup()) {
if (VarDecl *VD = dyn_cast_or_null<VarDecl>(D)) {
Expr *E = VD->getInit();
+ if (!E)
+ continue;
+ E = E->IgnoreParens();
+
// handle constructors that involve temporaries
- if (ExprWithCleanups *EWC = dyn_cast_or_null<ExprWithCleanups>(E))
+ if (auto *EWC = dyn_cast<ExprWithCleanups>(E))
E = EWC->getSubExpr();
+ if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E))
+ E = BTE->getSubExpr();
- if (CXXConstructExpr *CE = dyn_cast_or_null<CXXConstructExpr>(E)) {
+ if (CXXConstructExpr *CE = dyn_cast<CXXConstructExpr>(E)) {
NamedDecl *CtorD = dyn_cast_or_null<NamedDecl>(CE->getConstructor());
if (!CtorD || !CtorD->hasAttrs())
- return;
- handleCall(CE, CtorD, VD);
+ continue;
+ handleCall(E, CtorD, VD);
+ } else if (isa<CallExpr>(E) && E->isRValue()) {
+ // If the object is initialized by a function call that returns a
+ // scoped lockable by value, use the attributes on the copy or move
+ // constructor to figure out what effect that should have on the
+ // lockset.
+ // FIXME: Is this really the best way to handle this situation?
+ auto *RD = E->getType()->getAsCXXRecordDecl();
+ if (!RD || !RD->hasAttr<ScopedLockableAttr>())
+ continue;
+ CXXConstructorDecl *CtorD = findConstructorForByValueReturn(RD);
+ if (!CtorD || !CtorD->hasAttrs())
+ continue;
+ handleCall(buildFakeCtorCall(CtorD, {E}, E->getLocStart()), CtorD, VD);
}
}
}
Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=322316&r1=322315&r2=322316&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Thu Jan 11 14:13:57 2018
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=0 %s
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=1 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=0 %s
// FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 -Wc++98-compat %s
// FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety %s
@@ -5248,3 +5249,28 @@ struct C {
C c;
void f() { c[A()]->g(); }
} // namespace PR34800
+
+namespace ReturnScopedLockable {
+ template<typename Object> class SCOPED_LOCKABLE ReadLockedPtr {
+ public:
+ ReadLockedPtr(Object *ptr) SHARED_LOCK_FUNCTION((*this)->mutex);
+ ReadLockedPtr(ReadLockedPtr &&) SHARED_LOCK_FUNCTION((*this)->mutex);
+ ~ReadLockedPtr() UNLOCK_FUNCTION();
+
+ Object *operator->() const { return object; }
+
+ private:
+ Object *object;
+ };
+
+ struct Object {
+ int f() SHARED_LOCKS_REQUIRED(mutex);
+ Mutex mutex;
+ };
+
+ ReadLockedPtr<Object> get();
+ int use() {
+ auto ptr = get();
+ return ptr->f();
+ }
+}
More information about the cfe-commits
mailing list