[PATCH] D23765: Fix for clang PR 29087
This revision was automatically updated to reflect the committed changes.
Closed by commit rL287999: Adjust type-trait evaluation to properly handle
Using(Shadow)Decls (authored by hfinkel).
Changed prior to commit:
https://reviews.llvm.org/D23765?vs=76817=79354#toc
Repository:
rL LLVM
https://reviews.llvm.org/D23765
Files:
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/SemaCXX/cxx11-crashes.cpp
Index: cfe/trunk/test/SemaCXX/cxx11-crashes.cpp
===
--- cfe/trunk/test/SemaCXX/cxx11-crashes.cpp
+++ cfe/trunk/test/SemaCXX/cxx11-crashes.cpp
@@ -91,3 +91,15 @@
Foo(lambda);
}
}
+
+namespace pr29091 {
+ struct X{ X(const X ); };
+ struct Y: X { using X::X; };
+ bool foo() { return __has_nothrow_constructor(Y); }
+ bool bar() { return __has_nothrow_copy(Y); }
+
+ struct A { template A(); };
+ struct B : A { using A::A; };
+ bool baz() { return __has_nothrow_constructor(B); }
+ bool qux() { return __has_nothrow_copy(B); }
+}
Index: cfe/trunk/lib/Sema/SemaExprCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp
@@ -4416,9 +4416,12 @@
// A template constructor is never a copy constructor.
// FIXME: However, it may actually be selected at the actual overload
// resolution point.
-if (isa(ND))
+if (isa(ND->getUnderlyingDecl()))
continue;
-const CXXConstructorDecl *Constructor = cast(ND);
+// UsingDecl itself is not a constructor
+if (isa(ND))
+ continue;
+auto *Constructor = cast(ND->getUnderlyingDecl());
if (Constructor->isCopyConstructor(FoundTQs)) {
FoundConstructor = true;
const FunctionProtoType *CPT
@@ -4452,9 +4455,12 @@
bool FoundConstructor = false;
for (const auto *ND : Self.LookupConstructors(RD)) {
// FIXME: In C++0x, a constructor template can be a default
constructor.
-if (isa(ND))
+if (isa(ND->getUnderlyingDecl()))
+ continue;
+// UsingDecl itself is not a constructor
+if (isa(ND))
continue;
-const CXXConstructorDecl *Constructor = cast(ND);
+auto *Constructor = cast(ND->getUnderlyingDecl());
if (Constructor->isDefaultConstructor()) {
FoundConstructor = true;
const FunctionProtoType *CPT
Index: cfe/trunk/test/SemaCXX/cxx11-crashes.cpp
===
--- cfe/trunk/test/SemaCXX/cxx11-crashes.cpp
+++ cfe/trunk/test/SemaCXX/cxx11-crashes.cpp
@@ -91,3 +91,15 @@
Foo(lambda);
}
}
+
+namespace pr29091 {
+ struct X{ X(const X ); };
+ struct Y: X { using X::X; };
+ bool foo() { return __has_nothrow_constructor(Y); }
+ bool bar() { return __has_nothrow_copy(Y); }
+
+ struct A { template A(); };
+ struct B : A { using A::A; };
+ bool baz() { return __has_nothrow_constructor(B); }
+ bool qux() { return __has_nothrow_copy(B); }
+}
Index: cfe/trunk/lib/Sema/SemaExprCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp
@@ -4416,9 +4416,12 @@
// A template constructor is never a copy constructor.
// FIXME: However, it may actually be selected at the actual overload
// resolution point.
-if (isa(ND))
+if (isa(ND->getUnderlyingDecl()))
continue;
-const CXXConstructorDecl *Constructor = cast(ND);
+// UsingDecl itself is not a constructor
+if (isa(ND))
+ continue;
+auto *Constructor = cast(ND->getUnderlyingDecl());
if (Constructor->isCopyConstructor(FoundTQs)) {
FoundConstructor = true;
const FunctionProtoType *CPT
@@ -4452,9 +4455,12 @@
bool FoundConstructor = false;
for (const auto *ND : Self.LookupConstructors(RD)) {
// FIXME: In C++0x, a constructor template can be a default constructor.
-if (isa(ND))
+if (isa(ND->getUnderlyingDecl()))
+ continue;
+// UsingDecl itself is not a constructor
+if (isa(ND))
continue;
-const CXXConstructorDecl *Constructor = cast(ND);
+auto *Constructor = cast(ND->getUnderlyingDecl());
if (Constructor->isDefaultConstructor()) {
FoundConstructor = true;
const FunctionProtoType *CPT
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23765: Fix for clang PR 29087
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Looks good for trunk and 3.9 branch. https://reviews.llvm.org/D23765 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23765: Fix for clang PR 29087
gnzlbg added a comment. ping https://reviews.llvm.org/D23765 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23765: Fix for clang PR 29087
twoh updated this revision to Diff 76817.
twoh added a comment.
Addressing comments from @rsmith. Thanks!
https://reviews.llvm.org/D23765
Files:
lib/Sema/SemaExprCXX.cpp
test/SemaCXX/cxx11-crashes.cpp
Index: test/SemaCXX/cxx11-crashes.cpp
===
--- test/SemaCXX/cxx11-crashes.cpp
+++ test/SemaCXX/cxx11-crashes.cpp
@@ -91,3 +91,15 @@
Foo(lambda);
}
}
+
+namespace pr29091 {
+ struct X{ X(const X ); };
+ struct Y: X { using X::X; };
+ bool foo() { return __has_nothrow_constructor(Y); }
+ bool bar() { return __has_nothrow_copy(Y); }
+
+ struct A { template A(); };
+ struct B : A { using A::A; };
+ bool baz() { return __has_nothrow_constructor(B); }
+ bool qux() { return __has_nothrow_copy(B); }
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -4398,9 +4398,12 @@
// A template constructor is never a copy constructor.
// FIXME: However, it may actually be selected at the actual overload
// resolution point.
-if (isa(ND))
+if (isa(ND->getUnderlyingDecl()))
continue;
-const CXXConstructorDecl *Constructor = cast(ND);
+// UsingDecl itself is not a constructor
+if (isa(ND))
+ continue;
+auto *Constructor = cast(ND->getUnderlyingDecl());
if (Constructor->isCopyConstructor(FoundTQs)) {
FoundConstructor = true;
const FunctionProtoType *CPT
@@ -4434,9 +4437,12 @@
bool FoundConstructor = false;
for (const auto *ND : Self.LookupConstructors(RD)) {
// FIXME: In C++0x, a constructor template can be a default
constructor.
-if (isa(ND))
+if (isa(ND->getUnderlyingDecl()))
+ continue;
+// UsingDecl itself is not a constructor
+if (isa(ND))
continue;
-const CXXConstructorDecl *Constructor = cast(ND);
+auto *Constructor = cast(ND->getUnderlyingDecl());
if (Constructor->isDefaultConstructor()) {
FoundConstructor = true;
const FunctionProtoType *CPT
Index: test/SemaCXX/cxx11-crashes.cpp
===
--- test/SemaCXX/cxx11-crashes.cpp
+++ test/SemaCXX/cxx11-crashes.cpp
@@ -91,3 +91,15 @@
Foo(lambda);
}
}
+
+namespace pr29091 {
+ struct X{ X(const X ); };
+ struct Y: X { using X::X; };
+ bool foo() { return __has_nothrow_constructor(Y); }
+ bool bar() { return __has_nothrow_copy(Y); }
+
+ struct A { template A(); };
+ struct B : A { using A::A; };
+ bool baz() { return __has_nothrow_constructor(B); }
+ bool qux() { return __has_nothrow_copy(B); }
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -4398,9 +4398,12 @@
// A template constructor is never a copy constructor.
// FIXME: However, it may actually be selected at the actual overload
// resolution point.
-if (isa(ND))
+if (isa(ND->getUnderlyingDecl()))
continue;
-const CXXConstructorDecl *Constructor = cast(ND);
+// UsingDecl itself is not a constructor
+if (isa(ND))
+ continue;
+auto *Constructor = cast(ND->getUnderlyingDecl());
if (Constructor->isCopyConstructor(FoundTQs)) {
FoundConstructor = true;
const FunctionProtoType *CPT
@@ -4434,9 +4437,12 @@
bool FoundConstructor = false;
for (const auto *ND : Self.LookupConstructors(RD)) {
// FIXME: In C++0x, a constructor template can be a default constructor.
-if (isa(ND))
+if (isa(ND->getUnderlyingDecl()))
+ continue;
+// UsingDecl itself is not a constructor
+if (isa(ND))
continue;
-const CXXConstructorDecl *Constructor = cast(ND);
+auto *Constructor = cast(ND->getUnderlyingDecl());
if (Constructor->isDefaultConstructor()) {
FoundConstructor = true;
const FunctionProtoType *CPT
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23765: Fix for clang PR 29087
rsmith added inline comments.
Comment at: lib/Sema/SemaExprCXX.cpp:4390-4391
// resolution point.
if (isa(ND))
continue;
+// UsingDecl itself is not a constructor
You also need to handle the case of an inherited constructor template
(`isa(ND->getUnderlyingDecl())` will do the right thing
in both cases).
Comment at: lib/Sema/SemaExprCXX.cpp:4395-4397
+const CXXConstructorDecl *Constructor = nullptr;
+if (auto *CSD = dyn_cast(ND)) {
+ Constructor = cast(CSD->getTargetDecl());
This is better written as
auto *Constructor = cast(ND->getUnderlyingDecl());
Comment at: lib/Sema/SemaExprCXX.cpp:4398-4401
+ // Default constructor and copy/move constructor are not inherited.
+ if (Constructor->isDefaultConstructor() ||
+ Constructor->isCopyOrMoveConstructor())
+continue;
This is not necessary: Sema has already filtered out the ones that aren't
inherited.
https://reviews.llvm.org/D23765
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23765: Fix for clang PR 29087
twoh marked an inline comment as not done. twoh added a comment. Ping https://reviews.llvm.org/D23765 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23765: Fix for clang PR 29087
twoh updated this revision to Diff 75441.
twoh marked an inline comment as done.
twoh added a comment.
Addressing comments from @sepavloff. Thanks!
https://reviews.llvm.org/D23765
Files:
lib/Sema/SemaExprCXX.cpp
test/SemaCXX/cxx11-crashes.cpp
Index: test/SemaCXX/cxx11-crashes.cpp
===
--- test/SemaCXX/cxx11-crashes.cpp
+++ test/SemaCXX/cxx11-crashes.cpp
@@ -91,3 +91,10 @@
Foo(lambda);
}
}
+
+namespace pr29091 {
+ struct X{ X(const X ); };
+ struct Y: X { using X::X; };
+ bool foo() { return __has_nothrow_constructor(Y); }
+ bool bar() { return __has_nothrow_copy(Y); }
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -4389,7 +4389,18 @@
// resolution point.
if (isa(ND))
continue;
-const CXXConstructorDecl *Constructor = cast(ND);
+// UsingDecl itself is not a constructor
+if (isa(ND))
+ continue;
+const CXXConstructorDecl *Constructor = nullptr;
+if (auto *CSD = dyn_cast(ND)) {
+ Constructor = cast(CSD->getTargetDecl());
+ // Default constructor and copy/move constructor are not inherited.
+ if (Constructor->isDefaultConstructor() ||
+ Constructor->isCopyOrMoveConstructor())
+continue;
+} else
+ Constructor = cast(ND);
if (Constructor->isCopyConstructor(FoundTQs)) {
FoundConstructor = true;
const FunctionProtoType *CPT
@@ -4425,7 +4436,18 @@
// FIXME: In C++0x, a constructor template can be a default
constructor.
if (isa(ND))
continue;
-const CXXConstructorDecl *Constructor = cast(ND);
+// UsingDecl itself is not a constructor
+if (isa(ND))
+ continue;
+const CXXConstructorDecl *Constructor = nullptr;
+if (auto *CSD = dyn_cast(ND)) {
+ Constructor = cast(CSD->getTargetDecl());
+ // Default constructor and copy/move constructor are not inherited.
+ if (Constructor->isDefaultConstructor() ||
+ Constructor->isCopyOrMoveConstructor())
+continue;
+} else
+ Constructor = cast(ND);
if (Constructor->isDefaultConstructor()) {
FoundConstructor = true;
const FunctionProtoType *CPT
Index: test/SemaCXX/cxx11-crashes.cpp
===
--- test/SemaCXX/cxx11-crashes.cpp
+++ test/SemaCXX/cxx11-crashes.cpp
@@ -91,3 +91,10 @@
Foo(lambda);
}
}
+
+namespace pr29091 {
+ struct X{ X(const X ); };
+ struct Y: X { using X::X; };
+ bool foo() { return __has_nothrow_constructor(Y); }
+ bool bar() { return __has_nothrow_copy(Y); }
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -4389,7 +4389,18 @@
// resolution point.
if (isa(ND))
continue;
-const CXXConstructorDecl *Constructor = cast(ND);
+// UsingDecl itself is not a constructor
+if (isa(ND))
+ continue;
+const CXXConstructorDecl *Constructor = nullptr;
+if (auto *CSD = dyn_cast(ND)) {
+ Constructor = cast(CSD->getTargetDecl());
+ // Default constructor and copy/move constructor are not inherited.
+ if (Constructor->isDefaultConstructor() ||
+ Constructor->isCopyOrMoveConstructor())
+continue;
+} else
+ Constructor = cast(ND);
if (Constructor->isCopyConstructor(FoundTQs)) {
FoundConstructor = true;
const FunctionProtoType *CPT
@@ -4425,7 +4436,18 @@
// FIXME: In C++0x, a constructor template can be a default constructor.
if (isa(ND))
continue;
-const CXXConstructorDecl *Constructor = cast(ND);
+// UsingDecl itself is not a constructor
+if (isa(ND))
+ continue;
+const CXXConstructorDecl *Constructor = nullptr;
+if (auto *CSD = dyn_cast(ND)) {
+ Constructor = cast(CSD->getTargetDecl());
+ // Default constructor and copy/move constructor are not inherited.
+ if (Constructor->isDefaultConstructor() ||
+ Constructor->isCopyOrMoveConstructor())
+continue;
+} else
+ Constructor = cast(ND);
if (Constructor->isDefaultConstructor()) {
FoundConstructor = true;
const FunctionProtoType *CPT
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23765: Fix for clang PR 29087
sepavloff added inline comments.
Comment at: lib/Sema/SemaExprCXX.cpp:4231
+const CXXConstructorDecl *Constructor = nullptr;
+if (const ConstructorUsingShadowDecl *CSD =
+dyn_cast(ND)) {
Use `auto` here. Type of `CSD` is clear from `dyn_cast`.
Comment at: lib/Sema/SemaExprCXX.cpp:4233
+dyn_cast(ND)) {
+ assert(isa(CSD->getTargetDecl()));
+ Constructor = cast(CSD->getTargetDecl());
This `assert` is excessive. The subsequent `cast` makes this check.
Comment at: lib/Sema/SemaExprCXX.cpp:4239-4240
+continue;
+}
+else
+ Constructor = cast(ND);
Put `else` on the same line as `}`.
Comment at: lib/Sema/SemaExprCXX.cpp:4281
+const CXXConstructorDecl *Constructor = nullptr;
+if (const ConstructorUsingShadowDecl *CSD =
+dyn_cast(ND)) {
Use `auto` here.
Comment at: lib/Sema/SemaExprCXX.cpp:4283
+dyn_cast(ND)) {
+ assert(isa(CSD->getTargetDecl()));
+ Constructor = cast(CSD->getTargetDecl());
The `assert` is excessive.
Comment at: lib/Sema/SemaExprCXX.cpp:4289-4290
+continue;
+}
+else
+ Constructor = cast(ND);
Put `else` on the same line as `}`.
https://reviews.llvm.org/D23765
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23765: Fix for clang PR 29087
twoh added a comment. ping https://reviews.llvm.org/D23765 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23765: Fix for clang PR 29087
twoh added a comment. ping https://reviews.llvm.org/D23765 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23765: Fix for clang PR 29087
twoh added a comment. ping https://reviews.llvm.org/D23765 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23765: Fix for clang PR 29087
twoh added a comment. ping https://reviews.llvm.org/D23765 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23765: Fix for clang PR 29087
twoh added a comment. @rsmith Thank you for your review! I added tests to cxx11-crashes.cpp, as the goal of this patch is not handling __has_* traits right but preventing ICE. Also, I tried to use ConstructorUsingShadowDecl::getConstructor instead of ConstructorUsingShadowDecl::getTargetDecl followed by casting, but clang build was failed with undefined references. I wonder if the definition of ConstructorUsingShadowDecl::getConstructor is missed in https://reviews.llvm.org/rL274049. https://reviews.llvm.org/D23765 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23765: Fix for clang PR 29087
twoh updated this revision to Diff 71711.
twoh added a comment.
Updated diff. For ConstructorUsingShadowDecl, test with its target
CXXConstructorDecl, but only when it is not a default/copy/move constructor.
https://reviews.llvm.org/D23765
Files:
lib/Sema/SemaExprCXX.cpp
test/SemaCXX/cxx11-crashes.cpp
Index: test/SemaCXX/cxx11-crashes.cpp
===
--- test/SemaCXX/cxx11-crashes.cpp
+++ test/SemaCXX/cxx11-crashes.cpp
@@ -91,3 +91,10 @@
Foo(lambda);
}
}
+
+namespace pr29091 {
+ struct X{ X(const X ); };
+ struct Y: X { using X::X; };
+ bool foo() { return __has_nothrow_constructor(Y); }
+ bool bar() { return __has_nothrow_copy(Y); }
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -292,7 +292,7 @@
if (isDependent) {
// We didn't find our type, but that's okay: it's dependent
// anyway.
-
+
// FIXME: What if we have no nested-name-specifier?
QualType T = CheckTypenameType(ETK_None, SourceLocation(),
SS.getWithLocInContext(Context),
@@ -326,14 +326,14 @@
ParsedType Sema::getDestructorType(const DeclSpec& DS, ParsedType ObjectType) {
if (DS.getTypeSpecType() == DeclSpec::TST_error || !ObjectType)
return nullptr;
-assert(DS.getTypeSpecType() == DeclSpec::TST_decltype
+assert(DS.getTypeSpecType() == DeclSpec::TST_decltype
&& "only get destructor types from declspecs");
QualType T = BuildDecltypeType(DS.getRepAsExpr(), DS.getTypeSpecTypeLoc());
QualType SearchType = GetTypeFromParser(ObjectType);
if (SearchType->isDependentType() || Context.hasSameUnqualifiedType(SearchType, T)) {
return ParsedType::make(T);
}
-
+
Diag(DS.getTypeSpecTypeLoc(), diag::err_destructor_expr_type_mismatch)
<< T << SearchType;
return nullptr;
@@ -662,7 +662,7 @@
IsThrownVarInScope = true;
break;
}
-
+
if (S->getFlags() &
(Scope::FnScope | Scope::ClassScope | Scope::BlockScope |
Scope::FunctionPrototypeScope | Scope::ObjCMethodScope |
@@ -672,11 +672,11 @@
}
}
}
-
+
return BuildCXXThrow(OpLoc, Ex, IsThrownVarInScope);
}
-ExprResult Sema::BuildCXXThrow(SourceLocation OpLoc, Expr *Ex,
+ExprResult Sema::BuildCXXThrow(SourceLocation OpLoc, Expr *Ex,
bool IsThrownVarInScope) {
// Don't report an error if 'throw' is used in system headers.
if (!getLangOpts().CXXExceptions &&
@@ -903,10 +903,10 @@
I-- && isa(FunctionScopes[I]);
CurDC = getLambdaAwareParentOfDeclContext(CurDC)) {
CurLSI = cast(FunctionScopes[I]);
-
-if (!CurLSI->isCXXThisCaptured())
+
+if (!CurLSI->isCXXThisCaptured())
continue;
-
+
auto C = CurLSI->getCXXThisCapture();
if (C.isCopyCapture()) {
@@ -922,7 +922,7 @@
assert(CurLSI);
assert(isGenericLambdaCallOperatorSpecialization(CurLSI->CallOperator));
assert(CurDC == getLambdaAwareParentOfDeclContext(CurLSI->CallOperator));
-
+
auto IsThisCaptured =
[](CXXRecordDecl *Closure, bool , bool ) {
IsConst = false;
@@ -992,10 +992,10 @@
return ThisTy;
}
-Sema::CXXThisScopeRAII::CXXThisScopeRAII(Sema ,
+Sema::CXXThisScopeRAII::CXXThisScopeRAII(Sema ,
Decl *ContextDecl,
unsigned CXXThisTypeQuals,
- bool Enabled)
+ bool Enabled)
: S(S), OldCXXThisTypeOverride(S.CXXThisTypeOverride), Enabled(false)
{
if (!Enabled || !ContextDecl)
@@ -1006,13 +1006,13 @@
Record = Template->getTemplatedDecl();
else
Record = cast(ContextDecl);
-
+
// We care only for CVR qualifiers here, so cut everything else.
CXXThisTypeQuals &= Qualifiers::FastMask;
S.CXXThisTypeOverride
= S.Context.getPointerType(
S.Context.getRecordType(Record).withCVRQualifiers(CXXThisTypeQuals));
-
+
this->Enabled = true;
}
@@ -1026,7 +1026,7 @@
static Expr *captureThis(Sema , ASTContext , RecordDecl *RD,
QualType ThisTy, SourceLocation Loc,
const bool ByCopy) {
-
+
QualType AdjustedThisTy = ThisTy;
// The type of the corresponding data member (not a 'this' pointer if 'by
// copy').
@@ -1039,7 +1039,7 @@
CaptureThisFieldTy.removeLocalCVRQualifiers(Qualifiers::CVRMask);
AdjustedThisTy = Context.getPointerType(CaptureThisFieldTy);
}
-
+
FieldDecl *Field = FieldDecl::Create(
Context, RD, Loc, Loc, nullptr, CaptureThisFieldTy,
Context.getTrivialTypeSourceInfo(CaptureThisFieldTy, Loc), nullptr, false,
@@ -1065,41 +1065,41 @@
return This;
}
-bool
Re: [PATCH] D23765: Fix for clang PR 29087
rsmith added inline comments.
Comment at: lib/Sema/SemaExprCXX.cpp:4227
@@ -4226,1 +4226,3 @@
continue;
+// Using(Shadow)Decl itself is not a constructor
+if (isa(ND) || isa(ND))
This isn't really right: a `UsingShadowDecl` whose underlying declaration is a
constructor should itself act like a constructor. This could matter in some
obscure cases:
struct B;
struct A { A(B&); };
struct B : A { using A::A; };
What does `__has_nothrow_copy(B)` return? It should probably be `false`, since
copying a non-const `B` object will invoke the `A(B&)` constructor, which may
throw, even though the `B(const B&)` constructor does not.
However, these `__has_*` traits should be considered deprecated and are
essentially useless, so perhaps it doesn't matter too much whether we get these
corner cases right. We also get the constructor template case "wrong" here,
which I would imagine comes up a lot more frequently.
Comment at: test/SemaCXX/crash-has-nothrow-constructor.cpp:1
@@ +1,2 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s
+
Please add these tests into some existing test file for the type traits rather
than adding two new files.
https://reviews.llvm.org/D23765
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23765: Fix for clang PR 29087
twoh updated this revision to Diff 71560.
twoh added a comment.
Tests added
https://reviews.llvm.org/D23765
Files:
lib/Sema/SemaExprCXX.cpp
test/SemaCXX/crash-has-nothrow-constructor.cpp
test/SemaCXX/crash-has-nothrow-copy.cpp
Index: test/SemaCXX/crash-has-nothrow-copy.cpp
===
--- test/SemaCXX/crash-has-nothrow-copy.cpp
+++ test/SemaCXX/crash-has-nothrow-copy.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s
+
+#define assert(x) if (x) {}
+
+struct X {
+ X(const X ) {}
+};
+
+struct Y : public X {
+public:
+ using X::X;
+};
+
+int main() {
+ assert(__has_nothrow_copy(Y));
+}
Index: test/SemaCXX/crash-has-nothrow-constructor.cpp
===
--- test/SemaCXX/crash-has-nothrow-constructor.cpp
+++ test/SemaCXX/crash-has-nothrow-constructor.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s
+
+#define assert(x) if (x) {}
+
+struct X {
+ X() {}
+};
+
+struct Y : public X {
+public:
+ using X::X;
+};
+
+int main() {
+ assert(__has_nothrow_constructor(Y));
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -292,7 +292,7 @@
if (isDependent) {
// We didn't find our type, but that's okay: it's dependent
// anyway.
-
+
// FIXME: What if we have no nested-name-specifier?
QualType T = CheckTypenameType(ETK_None, SourceLocation(),
SS.getWithLocInContext(Context),
@@ -326,14 +326,14 @@
ParsedType Sema::getDestructorType(const DeclSpec& DS, ParsedType ObjectType) {
if (DS.getTypeSpecType() == DeclSpec::TST_error || !ObjectType)
return nullptr;
-assert(DS.getTypeSpecType() == DeclSpec::TST_decltype
+assert(DS.getTypeSpecType() == DeclSpec::TST_decltype
&& "only get destructor types from declspecs");
QualType T = BuildDecltypeType(DS.getRepAsExpr(), DS.getTypeSpecTypeLoc());
QualType SearchType = GetTypeFromParser(ObjectType);
if (SearchType->isDependentType() || Context.hasSameUnqualifiedType(SearchType, T)) {
return ParsedType::make(T);
}
-
+
Diag(DS.getTypeSpecTypeLoc(), diag::err_destructor_expr_type_mismatch)
<< T << SearchType;
return nullptr;
@@ -662,7 +662,7 @@
IsThrownVarInScope = true;
break;
}
-
+
if (S->getFlags() &
(Scope::FnScope | Scope::ClassScope | Scope::BlockScope |
Scope::FunctionPrototypeScope | Scope::ObjCMethodScope |
@@ -672,11 +672,11 @@
}
}
}
-
+
return BuildCXXThrow(OpLoc, Ex, IsThrownVarInScope);
}
-ExprResult Sema::BuildCXXThrow(SourceLocation OpLoc, Expr *Ex,
+ExprResult Sema::BuildCXXThrow(SourceLocation OpLoc, Expr *Ex,
bool IsThrownVarInScope) {
// Don't report an error if 'throw' is used in system headers.
if (!getLangOpts().CXXExceptions &&
@@ -903,10 +903,10 @@
I-- && isa(FunctionScopes[I]);
CurDC = getLambdaAwareParentOfDeclContext(CurDC)) {
CurLSI = cast(FunctionScopes[I]);
-
-if (!CurLSI->isCXXThisCaptured())
+
+if (!CurLSI->isCXXThisCaptured())
continue;
-
+
auto C = CurLSI->getCXXThisCapture();
if (C.isCopyCapture()) {
@@ -922,7 +922,7 @@
assert(CurLSI);
assert(isGenericLambdaCallOperatorSpecialization(CurLSI->CallOperator));
assert(CurDC == getLambdaAwareParentOfDeclContext(CurLSI->CallOperator));
-
+
auto IsThisCaptured =
[](CXXRecordDecl *Closure, bool , bool ) {
IsConst = false;
@@ -992,10 +992,10 @@
return ThisTy;
}
-Sema::CXXThisScopeRAII::CXXThisScopeRAII(Sema ,
+Sema::CXXThisScopeRAII::CXXThisScopeRAII(Sema ,
Decl *ContextDecl,
unsigned CXXThisTypeQuals,
- bool Enabled)
+ bool Enabled)
: S(S), OldCXXThisTypeOverride(S.CXXThisTypeOverride), Enabled(false)
{
if (!Enabled || !ContextDecl)
@@ -1006,13 +1006,13 @@
Record = Template->getTemplatedDecl();
else
Record = cast(ContextDecl);
-
+
// We care only for CVR qualifiers here, so cut everything else.
CXXThisTypeQuals &= Qualifiers::FastMask;
S.CXXThisTypeOverride
= S.Context.getPointerType(
S.Context.getRecordType(Record).withCVRQualifiers(CXXThisTypeQuals));
-
+
this->Enabled = true;
}
@@ -1026,7 +1026,7 @@
static Expr *captureThis(Sema , ASTContext , RecordDecl *RD,
QualType ThisTy, SourceLocation Loc,
const bool ByCopy) {
-
+
QualType AdjustedThisTy = ThisTy;
// The type of the corresponding data member (not a 'this' pointer if 'by
// copy').
@@
Re: [PATCH] D23765: Fix for clang PR 29087
sepavloff added a subscriber: sepavloff. sepavloff added a comment. You need to provide a test for the fix. https://reviews.llvm.org/D23765 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23765: Fix for clang PR 29087
twoh added a comment. Ping. https://reviews.llvm.org/D23765 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23765: Fix for clang PR 29087
twoh added a comment. ping https://reviews.llvm.org/D23765 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23765: Fix for clang PR 29087
twoh created this revision.
twoh added a reviewer: rsmith.
twoh added a subscriber: cfe-commits.
Since rL274049, for an inheriting constructor declaration, the name of the
using declaration (and using shadow declaration comes from the using
declaration) is the name of a derived class, not the base class (line 8225-8232
of lib/Sema/SemaDeclCXX.cpp in https://reviews.llvm.org/rL274049). Because of
this, name-based lookup performed inside Sema::LookupConstructors returns not
only CXXConstructorDecls but also Using(Shadow)Decls, which results assertion
failure reported in PR 29087.
https://reviews.llvm.org/D23765
Files:
include/clang/AST/DeclCXX.h
lib/Sema/SemaExprCXX.cpp
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -292,7 +292,7 @@
if (isDependent) {
// We didn't find our type, but that's okay: it's dependent
// anyway.
-
+
// FIXME: What if we have no nested-name-specifier?
QualType T = CheckTypenameType(ETK_None, SourceLocation(),
SS.getWithLocInContext(Context),
@@ -326,14 +326,14 @@
ParsedType Sema::getDestructorType(const DeclSpec& DS, ParsedType ObjectType) {
if (DS.getTypeSpecType() == DeclSpec::TST_error || !ObjectType)
return nullptr;
-assert(DS.getTypeSpecType() == DeclSpec::TST_decltype
+assert(DS.getTypeSpecType() == DeclSpec::TST_decltype
&& "only get destructor types from declspecs");
QualType T = BuildDecltypeType(DS.getRepAsExpr(), DS.getTypeSpecTypeLoc());
QualType SearchType = GetTypeFromParser(ObjectType);
if (SearchType->isDependentType() || Context.hasSameUnqualifiedType(SearchType, T)) {
return ParsedType::make(T);
}
-
+
Diag(DS.getTypeSpecTypeLoc(), diag::err_destructor_expr_type_mismatch)
<< T << SearchType;
return nullptr;
@@ -662,7 +662,7 @@
IsThrownVarInScope = true;
break;
}
-
+
if (S->getFlags() &
(Scope::FnScope | Scope::ClassScope | Scope::BlockScope |
Scope::FunctionPrototypeScope | Scope::ObjCMethodScope |
@@ -672,11 +672,11 @@
}
}
}
-
+
return BuildCXXThrow(OpLoc, Ex, IsThrownVarInScope);
}
-ExprResult Sema::BuildCXXThrow(SourceLocation OpLoc, Expr *Ex,
+ExprResult Sema::BuildCXXThrow(SourceLocation OpLoc, Expr *Ex,
bool IsThrownVarInScope) {
// Don't report an error if 'throw' is used in system headers.
if (!getLangOpts().CXXExceptions &&
@@ -903,10 +903,10 @@
I-- && isa(FunctionScopes[I]);
CurDC = getLambdaAwareParentOfDeclContext(CurDC)) {
CurLSI = cast(FunctionScopes[I]);
-
-if (!CurLSI->isCXXThisCaptured())
+
+if (!CurLSI->isCXXThisCaptured())
continue;
-
+
auto C = CurLSI->getCXXThisCapture();
if (C.isCopyCapture()) {
@@ -922,7 +922,7 @@
assert(CurLSI);
assert(isGenericLambdaCallOperatorSpecialization(CurLSI->CallOperator));
assert(CurDC == getLambdaAwareParentOfDeclContext(CurLSI->CallOperator));
-
+
auto IsThisCaptured =
[](CXXRecordDecl *Closure, bool , bool ) {
IsConst = false;
@@ -992,10 +992,10 @@
return ThisTy;
}
-Sema::CXXThisScopeRAII::CXXThisScopeRAII(Sema ,
+Sema::CXXThisScopeRAII::CXXThisScopeRAII(Sema ,
Decl *ContextDecl,
unsigned CXXThisTypeQuals,
- bool Enabled)
+ bool Enabled)
: S(S), OldCXXThisTypeOverride(S.CXXThisTypeOverride), Enabled(false)
{
if (!Enabled || !ContextDecl)
@@ -1006,13 +1006,13 @@
Record = Template->getTemplatedDecl();
else
Record = cast(ContextDecl);
-
+
// We care only for CVR qualifiers here, so cut everything else.
CXXThisTypeQuals &= Qualifiers::FastMask;
S.CXXThisTypeOverride
= S.Context.getPointerType(
S.Context.getRecordType(Record).withCVRQualifiers(CXXThisTypeQuals));
-
+
this->Enabled = true;
}
@@ -1026,7 +1026,7 @@
static Expr *captureThis(Sema , ASTContext , RecordDecl *RD,
QualType ThisTy, SourceLocation Loc,
const bool ByCopy) {
-
+
QualType AdjustedThisTy = ThisTy;
// The type of the corresponding data member (not a 'this' pointer if 'by
// copy').
@@ -1039,7 +1039,7 @@
CaptureThisFieldTy.removeLocalCVRQualifiers(Qualifiers::CVRMask);
AdjustedThisTy = Context.getPointerType(CaptureThisFieldTy);
}
-
+
FieldDecl *Field = FieldDecl::Create(
Context, RD, Loc, Loc, nullptr, CaptureThisFieldTy,
Context.getTrivialTypeSourceInfo(CaptureThisFieldTy, Loc), nullptr, false,
@@ -1065,41 +1065,41 @@
return This;
}
-bool Sema::CheckCXXThisCapture(SourceLocation Loc, const
