From 03f705fe92dc51c1b2a7746d9b25c59040aede43 Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Tue, 8 Jul 2014 18:18:04 +0000 Subject: [PATCH] Sema: Don't allow CVR qualifiers before structors We would silently accept volatile ~S() when the user probably intended to write virtual ~S(). This fixes PR20238. llvm-svn: 212555 --- clang/include/clang/Sema/Sema.h | 8 +++++++ clang/lib/Sema/SemaDeclCXX.cpp | 27 +++++++++++++++++---- clang/lib/Sema/SemaType.cpp | 48 ++++++++++++++++++++------------------ clang/test/SemaCXX/constructor.cpp | 2 ++ clang/test/SemaCXX/destructor.cpp | 6 +++++ 5 files changed, 64 insertions(+), 27 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index f3c3d47..c771ff9 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1556,6 +1556,14 @@ public: bool diagnoseQualifiedDeclaration(CXXScopeSpec &SS, DeclContext *DC, DeclarationName Name, SourceLocation Loc); + void + diagnoseIgnoredQualifiers(unsigned DiagID, unsigned Quals, + SourceLocation FallbackLoc, + SourceLocation ConstQualLoc = SourceLocation(), + SourceLocation VolatileQualLoc = SourceLocation(), + SourceLocation RestrictQualLoc = SourceLocation(), + SourceLocation AtomicQualLoc = SourceLocation()); + static bool adjustContextForLocalExternDecl(DeclContext *&DC); void DiagnoseFunctionSpecifiers(const DeclSpec &DS); void CheckShadow(Scope *S, VarDecl *D, const LookupResult& R); diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 621ba1d..99eedf3 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -6279,6 +6279,15 @@ QualType Sema::CheckConstructorDeclarator(Declarator &D, QualType R, SC = SC_None; } + if (unsigned TypeQuals = D.getDeclSpec().getTypeQualifiers()) { + diagnoseIgnoredQualifiers( + diag::err_constructor_return_type, TypeQuals, SourceLocation(), + D.getDeclSpec().getConstSpecLoc(), D.getDeclSpec().getVolatileSpecLoc(), + D.getDeclSpec().getRestrictSpecLoc(), + D.getDeclSpec().getAtomicSpecLoc()); + D.setInvalidType(); + } + DeclaratorChunk::FunctionTypeInfo &FTI = D.getFunctionTypeInfo(); if (FTI.TypeQuals != 0) { if (FTI.TypeQuals & Qualifiers::Const) @@ -6426,7 +6435,7 @@ QualType Sema::CheckDestructorDeclarator(Declarator &D, QualType R, SC = SC_None; } - if (D.getDeclSpec().hasTypeSpecifier() && !D.isInvalidType()) { + if (!D.isInvalidType()) { // Destructors don't have return types, but the parser will // happily parse something like: // @@ -6435,9 +6444,19 @@ QualType Sema::CheckDestructorDeclarator(Declarator &D, QualType R, // }; // // The return type will be eliminated later. - Diag(D.getIdentifierLoc(), diag::err_destructor_return_type) - << SourceRange(D.getDeclSpec().getTypeSpecTypeLoc()) - << SourceRange(D.getIdentifierLoc()); + if (D.getDeclSpec().hasTypeSpecifier()) + Diag(D.getIdentifierLoc(), diag::err_destructor_return_type) + << SourceRange(D.getDeclSpec().getTypeSpecTypeLoc()) + << SourceRange(D.getIdentifierLoc()); + else if (unsigned TypeQuals = D.getDeclSpec().getTypeQualifiers()) { + diagnoseIgnoredQualifiers(diag::err_destructor_return_type, TypeQuals, + SourceLocation(), + D.getDeclSpec().getConstSpecLoc(), + D.getDeclSpec().getVolatileSpecLoc(), + D.getDeclSpec().getRestrictSpecLoc(), + D.getDeclSpec().getAtomicSpecLoc()); + D.setInvalidType(); + } } DeclaratorChunk::FunctionTypeInfo &FTI = D.getFunctionTypeInfo(); diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 29c03f8..ff48ebc 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -1989,18 +1989,15 @@ static void inferARCWriteback(TypeProcessingState &state, // TODO: mark whether we did this inference? } -static void diagnoseIgnoredQualifiers( - Sema &S, unsigned Quals, - SourceLocation FallbackLoc, - SourceLocation ConstQualLoc = SourceLocation(), - SourceLocation VolatileQualLoc = SourceLocation(), - SourceLocation RestrictQualLoc = SourceLocation(), - SourceLocation AtomicQualLoc = SourceLocation()) { +void Sema::diagnoseIgnoredQualifiers(unsigned DiagID, unsigned Quals, + SourceLocation FallbackLoc, + SourceLocation ConstQualLoc, + SourceLocation VolatileQualLoc, + SourceLocation RestrictQualLoc, + SourceLocation AtomicQualLoc) { if (!Quals) return; - const SourceManager &SM = S.getSourceManager(); - struct Qual { unsigned Mask; const char *Name; @@ -2027,7 +2024,8 @@ static void diagnoseIgnoredQualifiers( SourceLocation QualLoc = QualKinds[I].Loc; if (!QualLoc.isInvalid()) { FixIts[NumQuals] = FixItHint::CreateRemoval(QualLoc); - if (Loc.isInvalid() || SM.isBeforeInTranslationUnit(QualLoc, Loc)) + if (Loc.isInvalid() || + getSourceManager().isBeforeInTranslationUnit(QualLoc, Loc)) Loc = QualLoc; } @@ -2035,7 +2033,7 @@ static void diagnoseIgnoredQualifiers( } } - S.Diag(Loc.isInvalid() ? FallbackLoc : Loc, diag::warn_qual_return_type) + Diag(Loc.isInvalid() ? FallbackLoc : Loc, DiagID) << QualStr << NumQuals << FixIts[0] << FixIts[1] << FixIts[2] << FixIts[3]; } @@ -2046,8 +2044,9 @@ static void diagnoseRedundantReturnTypeQualifiers(Sema &S, QualType RetTy, if (D.getTypeObject(FunctionChunkIndex).Fun.hasTrailingReturnType()) { // FIXME: TypeSourceInfo doesn't preserve location information for // qualifiers. - diagnoseIgnoredQualifiers(S, RetTy.getLocalCVRQualifiers(), - D.getIdentifierLoc()); + S.diagnoseIgnoredQualifiers(diag::warn_qual_return_type, + RetTy.getLocalCVRQualifiers(), + D.getIdentifierLoc()); return; } @@ -2061,8 +2060,9 @@ static void diagnoseRedundantReturnTypeQualifiers(Sema &S, QualType RetTy, case DeclaratorChunk::Pointer: { DeclaratorChunk::PointerTypeInfo &PTI = OuterChunk.Ptr; - diagnoseIgnoredQualifiers( - S, PTI.TypeQuals, + S.diagnoseIgnoredQualifiers( + diag::warn_qual_return_type, + PTI.TypeQuals, SourceLocation(), SourceLocation::getFromRawEncoding(PTI.ConstQualLoc), SourceLocation::getFromRawEncoding(PTI.VolatileQualLoc), @@ -2079,8 +2079,9 @@ static void diagnoseRedundantReturnTypeQualifiers(Sema &S, QualType RetTy, // FIXME: We can't currently provide an accurate source location and a // fix-it hint for these. unsigned AtomicQual = RetTy->isAtomicType() ? DeclSpec::TQ_atomic : 0; - diagnoseIgnoredQualifiers(S, RetTy.getCVRQualifiers() | AtomicQual, - D.getIdentifierLoc()); + S.diagnoseIgnoredQualifiers(diag::warn_qual_return_type, + RetTy.getCVRQualifiers() | AtomicQual, + D.getIdentifierLoc()); return; } @@ -2095,12 +2096,13 @@ static void diagnoseRedundantReturnTypeQualifiers(Sema &S, QualType RetTy, // Just parens all the way out to the decl specifiers. Diagnose any qualifiers // which are present there. - diagnoseIgnoredQualifiers(S, D.getDeclSpec().getTypeQualifiers(), - D.getIdentifierLoc(), - D.getDeclSpec().getConstSpecLoc(), - D.getDeclSpec().getVolatileSpecLoc(), - D.getDeclSpec().getRestrictSpecLoc(), - D.getDeclSpec().getAtomicSpecLoc()); + S.diagnoseIgnoredQualifiers(diag::warn_qual_return_type, + D.getDeclSpec().getTypeQualifiers(), + D.getIdentifierLoc(), + D.getDeclSpec().getConstSpecLoc(), + D.getDeclSpec().getVolatileSpecLoc(), + D.getDeclSpec().getRestrictSpecLoc(), + D.getDeclSpec().getAtomicSpecLoc()); } static QualType GetDeclSpecTypeForDeclarator(TypeProcessingState &state, diff --git a/clang/test/SemaCXX/constructor.cpp b/clang/test/SemaCXX/constructor.cpp index f3b910d..fa930bd 100644 --- a/clang/test/SemaCXX/constructor.cpp +++ b/clang/test/SemaCXX/constructor.cpp @@ -17,6 +17,8 @@ class Foo { int Foo(int, int); // expected-error{{constructor cannot have a return type}} \ // expected-error{{member 'Foo' has the same name as its class}} + + volatile Foo(float); // expected-error{{constructor cannot have a return type}} }; Foo::Foo(const Foo&) { } diff --git a/clang/test/SemaCXX/destructor.cpp b/clang/test/SemaCXX/destructor.cpp index d0a0731..5305ff5 100644 --- a/clang/test/SemaCXX/destructor.cpp +++ b/clang/test/SemaCXX/destructor.cpp @@ -380,3 +380,9 @@ namespace PR7900 { namespace PR16892 { auto p = &A::~A; // expected-error{{taking the address of a destructor}} } + +namespace PR20238 { +struct S { + volatile ~S() { } // expected-error{{destructor cannot have a return type}} +}; +} -- 2.7.4