[cfe-commits] r166372 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaType.cpp test/CXX/except/except.spec/p15.cpp test/CXX/except/except.spec/p4.cpp test/CXX/special/class.dtor/p3.cpp
Richard Smith
richard at metafoo.co.uk
Tue Jul 16 17:13:11 PDT 2013
On Tue, Jul 16, 2013 at 3:24 PM, Nico Weber <thakis at chromium.org> wrote:
> According to the commit message, it sounds like
> http://llvm.org/bugs/show_bug.cgi?id=16638 is working as intended. Is
> that correct? I found this diagnostic very confusing. (The code in the bug
> is reduced from v8 code, built with stlport.)
>
Yes, this is working as intended, although the diagnostic could be clearer.
I wouldn't object to downgrading the operator delete case from an ExtWarn
to an Extension, if you feel so inclined.
> On Sat, Oct 20, 2012 at 1:26 AM, Richard Smith <richard-llvm at metafoo.co.uk
> > wrote:
>
>> Author: rsmith
>> Date: Sat Oct 20 03:26:51 2012
>> New Revision: 166372
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=166372&view=rev
>> Log:
>> Rework implementation of DR1492: Apply the resolution to operator delete
>> too,
>> since it also has an implicit exception specification. Downgrade the
>> error to
>> an extwarn, since at least for operator delete, system headers like to
>> declare
>> it as 'noexcept' whereas the implicit definition does not have an explicit
>> exception specification. Move the exception specification for
>> user-declared
>> 'operator delete' functions from the type-as-written into the type, to
>> reflect
>> reality and to allow us to detect whether there was an implicit exception
>> spec
>> or not.
>>
>> Added:
>> cfe/trunk/test/CXX/except/except.spec/p4.cpp
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/lib/Sema/SemaDecl.cpp
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
>> cfe/trunk/lib/Sema/SemaType.cpp
>> cfe/trunk/test/CXX/except/except.spec/p15.cpp
>> cfe/trunk/test/CXX/special/class.dtor/p3.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=166372&r1=166371&r2=166372&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sat Oct 20
>> 03:26:51 2012
>> @@ -5425,6 +5425,10 @@
>> "defaulting this %select{default constructor|copy constructor|move "
>> "constructor|copy assignment operator|move assignment
>> operator|destructor}0 "
>> "would delete it after its first declaration">;
>> +def ext_implicit_exception_spec_mismatch : ExtWarn<
>> + "function previously declared with an %select{explicit|implicit}0
>> exception "
>> + "specification redeclared with an %select{implicit|explicit}0
>> exception "
>> + "specification">,
>> InGroup<DiagGroup<"implicit-exception-spec-mismatch">>;
>>
>> def warn_ptr_arith_precedes_bounds : Warning<
>> "the pointer decremented by %0 refers before the beginning of the
>> array">,
>>
>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=166372&r1=166371&r2=166372&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Sat Oct 20 03:26:51 2012
>> @@ -5519,6 +5519,20 @@
>> diag::err_static_out_of_line)
>> <<
>> FixItHint::CreateRemoval(D.getDeclSpec().getStorageClassSpecLoc());
>> }
>> +
>> + // C++11 [except.spec]p15:
>> + // A deallocation function with no exception-specification is
>> treated
>> + // as if it were specified with noexcept(true).
>> + const FunctionProtoType *FPT = R->getAs<FunctionProtoType>();
>> + if ((Name.getCXXOverloadedOperator() == OO_Delete ||
>> + Name.getCXXOverloadedOperator() == OO_Array_Delete) &&
>> + getLangOpts().CPlusPlus0x && FPT && !FPT->hasExceptionSpec()) {
>> + FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
>> + EPI.ExceptionSpecType = EST_BasicNoexcept;
>> + NewFD->setType(Context.getFunctionType(FPT->getResultType(),
>> + FPT->arg_type_begin(),
>> + FPT->getNumArgs(), EPI));
>> + }
>> }
>>
>> // Filter out previous declarations that don't match the scope.
>>
>> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=166372&r1=166371&r2=166372&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Sat Oct 20 03:26:51 2012
>> @@ -9379,7 +9379,7 @@
>> }
>>
>> static bool
>> -CheckOperatorDeleteDeclaration(Sema &SemaRef, const FunctionDecl
>> *FnDecl) {
>> +CheckOperatorDeleteDeclaration(Sema &SemaRef, FunctionDecl *FnDecl) {
>> // C++ [basic.stc.dynamic.deallocation]p1:
>> // A program is ill-formed if deallocation functions are declared in
>> a
>> // namespace scope other than global scope or declared static in
>> global
>>
>> Modified: cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExceptionSpec.cpp?rev=166372&r1=166371&r2=166372&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaExceptionSpec.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExceptionSpec.cpp Sat Oct 20 03:26:51 2012
>> @@ -120,21 +120,22 @@
>> return SourceDecl->getType()->castAs<FunctionProtoType>();
>> }
>>
>> -/// Get the type that a function had prior to adjustment of the exception
>> +/// Determine whether a function has an implicitly-generated exception
>> /// specification.
>> -static const FunctionProtoType *getUnadjustedFunctionType(FunctionDecl
>> *Decl) {
>> - if (isa<CXXDestructorDecl>(Decl) && Decl->getTypeSourceInfo()) {
>> - const FunctionProtoType *Ty =
>> - Decl->getTypeSourceInfo()->getType()->getAs<FunctionProtoType>();
>> - if (!Ty->hasExceptionSpec())
>> - // The type will be adjusted. Use the EST_None exception
>> specification
>> - // from the type as written.
>> - return Ty;
>> - }
>> +static bool hasImplicitExceptionSpec(FunctionDecl *Decl) {
>> + if (!isa<CXXDestructorDecl>(Decl) &&
>> + Decl->getDeclName().getCXXOverloadedOperator() != OO_Delete &&
>> + Decl->getDeclName().getCXXOverloadedOperator() != OO_Array_Delete)
>> + return false;
>> +
>> + // If the user didn't declare the function, its exception
>> specification must
>> + // be implicit.
>> + if (!Decl->getTypeSourceInfo())
>> + return true;
>>
>> - // Use whatever type the function now has. The TypeSourceInfo does not
>> contain
>> - // an instantiated exception specification for a function template,
>> - return Decl->getType()->getAs<FunctionProtoType>();
>> + const FunctionProtoType *Ty =
>> + Decl->getTypeSourceInfo()->getType()->getAs<FunctionProtoType>();
>> + return !Ty->hasExceptionSpec();
>> }
>>
>> bool Sema::CheckEquivalentExceptionSpec(FunctionDecl *Old, FunctionDecl
>> *New) {
>> @@ -150,18 +151,31 @@
>> // specification adjustment is applied.
>> if (!CheckEquivalentExceptionSpec(
>> PDiag(DiagID), PDiag(diag::note_previous_declaration),
>> - getUnadjustedFunctionType(Old), Old->getLocation(),
>> - getUnadjustedFunctionType(New), New->getLocation(),
>> + Old->getType()->getAs<FunctionProtoType>(), Old->getLocation(),
>> + New->getType()->getAs<FunctionProtoType>(), New->getLocation(),
>> &MissingExceptionSpecification,
>> &MissingEmptyExceptionSpecification,
>> - /*AllowNoexceptAllMatchWithNoSpec=*/true, IsOperatorNew))
>> + /*AllowNoexceptAllMatchWithNoSpec=*/true, IsOperatorNew)) {
>> + // C++11 [except.spec]p4 [DR1492]:
>> + // If a declaration of a function has an implicit
>> + // exception-specification, other declarations of the function
>> shall
>> + // not specify an exception-specification.
>> + if (getLangOpts().CPlusPlus0x &&
>> + hasImplicitExceptionSpec(Old) != hasImplicitExceptionSpec(New)) {
>> + Diag(New->getLocation(),
>> diag::ext_implicit_exception_spec_mismatch)
>> + << hasImplicitExceptionSpec(Old);
>> + if (!Old->getLocation().isInvalid())
>> + Diag(Old->getLocation(), diag::note_previous_declaration);
>> + }
>> return false;
>> + }
>>
>> // The failure was something other than an empty exception
>> // specification; return an error.
>> if (!MissingExceptionSpecification &&
>> !MissingEmptyExceptionSpecification)
>> return true;
>>
>> - const FunctionProtoType *NewProto = getUnadjustedFunctionType(New);
>> + const FunctionProtoType *NewProto =
>> + New->getType()->getAs<FunctionProtoType>();
>>
>> // The new function declaration is only missing an empty exception
>> // specification "throw()". If the throw() specification came from a
>> @@ -186,7 +200,8 @@
>> }
>>
>> if (MissingExceptionSpecification && NewProto) {
>> - const FunctionProtoType *OldProto = getUnadjustedFunctionType(Old);
>> + const FunctionProtoType *OldProto =
>> + Old->getType()->getAs<FunctionProtoType>();
>>
>> FunctionProtoType::ExtProtoInfo EPI = NewProto->getExtProtoInfo();
>> EPI.ExceptionSpecType = OldProto->getExceptionSpecType();
>> @@ -310,6 +325,10 @@
>>
>> /// CheckEquivalentExceptionSpec - Check if the two types have compatible
>> /// exception specifications. See C++ [except.spec]p3.
>> +///
>> +/// \return \c false if the exception specifications match, \c true if
>> there is
>> +/// a problem. If \c true is returned, either a diagnostic has already
>> been
>> +/// produced or \c *MissingExceptionSpecification is set to \c true.
>> bool Sema::CheckEquivalentExceptionSpec(const PartialDiagnostic &DiagID,
>> const PartialDiagnostic & NoteID,
>> const FunctionProtoType *Old,
>>
>> Modified: cfe/trunk/lib/Sema/SemaType.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=166372&r1=166371&r2=166372&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaType.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaType.cpp Sat Oct 20 03:26:51 2012
>> @@ -2169,16 +2169,6 @@
>> ASTContext &Context = S.Context;
>> const LangOptions &LangOpts = S.getLangOpts();
>>
>> - bool ImplicitlyNoexcept = false;
>> - if (D.getName().getKind() == UnqualifiedId::IK_OperatorFunctionId &&
>> - LangOpts.CPlusPlus0x) {
>> - OverloadedOperatorKind OO = D.getName().OperatorFunctionId.Operator;
>> - /// In C++0x, deallocation functions (normal and array operator
>> delete)
>> - /// are implicitly noexcept.
>> - if (OO == OO_Delete || OO == OO_Array_Delete)
>> - ImplicitlyNoexcept = true;
>> - }
>> -
>> // The name we're declaring, if any.
>> DeclarationName Name;
>> if (D.getIdentifier())
>> @@ -2578,12 +2568,6 @@
>> Exceptions,
>> EPI);
>>
>> - if (FTI.getExceptionSpecType() == EST_None &&
>> - ImplicitlyNoexcept && chunkIndex == 0) {
>> - // Only the outermost chunk is marked noexcept, of course.
>> - EPI.ExceptionSpecType = EST_BasicNoexcept;
>> - }
>> -
>> T = Context.getFunctionType(T, ArgTys.data(), ArgTys.size(),
>> EPI);
>> }
>>
>>
>> Modified: cfe/trunk/test/CXX/except/except.spec/p15.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p15.cpp?rev=166372&r1=166371&r2=166372&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/CXX/except/except.spec/p15.cpp (original)
>> +++ cfe/trunk/test/CXX/except/except.spec/p15.cpp Sat Oct 20 03:26:51 2012
>> @@ -9,16 +9,20 @@
>> delete[] new int[1];
>> }
>>
>> -void operator delete(void*) noexcept;
>> -void operator delete[](void*) noexcept;
>> +void operator delete(void*);
>> +void operator delete[](void*);
>> +
>> +static_assert(noexcept(operator delete(0)), "");
>> +static_assert(noexcept(operator delete[](0)), "");
>>
>> // Same goes for explicit declarations.
>> void operator delete(void*, float);
>> -void operator delete(void*, float) noexcept;
>> -
>> void operator delete[](void*, float);
>> -void operator delete[](void*, float) noexcept;
>> +
>> +static_assert(noexcept(operator delete(0, 0.f)), "");
>> +static_assert(noexcept(operator delete[](0, 0.f)), "");
>>
>> // But explicit specs stay.
>> void operator delete(void*, double) throw(int); // expected-note
>> {{previous}}
>> +static_assert(!noexcept(operator delete(0, 0.)), "");
>> void operator delete(void*, double) noexcept; // expected-error {{does
>> not match}}
>>
>> Added: cfe/trunk/test/CXX/except/except.spec/p4.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p4.cpp?rev=166372&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/CXX/except/except.spec/p4.cpp (added)
>> +++ cfe/trunk/test/CXX/except/except.spec/p4.cpp Sat Oct 20 03:26:51 2012
>> @@ -0,0 +1,36 @@
>> +// RUN: %clang_cc1 -std=c++11 %s -verify -fcxx-exceptions
>> +
>> +// We permit overriding an implicit exception specification with an
>> explicit one
>> +// as an extension, for compatibility with existing code.
>> +
>> +struct S {
>> + void a(); // expected-note {{here}}
>> + ~S(); // expected-note {{here}}
>> + void operator delete(void*); // expected-note {{here}}
>> +};
>> +
>> +void S::a() noexcept {} // expected-error {{does not match previous}}
>> +S::~S() noexcept {} // expected-warning {{function previously declared
>> with an implicit exception specification redeclared with an explicit
>> exception specification}}
>> +void S::operator delete(void*) noexcept {} // expected-warning
>> {{function previously declared with an implicit exception specification
>> redeclared with an explicit exception specification}}
>> +
>> +struct T {
>> + void a() noexcept; // expected-note {{here}}
>> + ~T() noexcept; // expected-note {{here}}
>> + void operator delete(void*) noexcept; // expected-note {{here}}
>> +};
>> +
>> +void T::a() {} // expected-warning {{missing exception specification
>> 'noexcept'}}
>> +T::~T() {} // expected-warning {{function previously declared with an
>> explicit exception specification redeclared with an implicit exception
>> specification}}
>> +void T::operator delete(void*) {} // expected-warning {{function
>> previously declared with an explicit exception specification redeclared
>> with an implicit exception specification}}
>> +
>> +
>> +// The extension does not extend to function templates.
>> +
>> +template<typename T> struct U {
>> + T t;
>> + ~U(); // expected-note {{here}}
>> + void operator delete(void*); // expected-note {{here}}
>> +};
>> +
>> +template<typename T> U<T>::~U() noexcept(true) {} // expected-error
>> {{exception specification in declaration does not match previous
>> declaration}}
>> +template<typename T> void U<T>::operator delete(void*) noexcept(false)
>> {} // expected-error {{exception specification in declaration does not
>> match previous declaration}}
>>
>> Modified: cfe/trunk/test/CXX/special/class.dtor/p3.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.dtor/p3.cpp?rev=166372&r1=166371&r2=166372&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/CXX/special/class.dtor/p3.cpp (original)
>> +++ cfe/trunk/test/CXX/special/class.dtor/p3.cpp Sat Oct 20 03:26:51 2012
>> @@ -4,10 +4,10 @@
>> // the exception specification adjustment occurs.
>> namespace DR1492 {
>> struct A { ~A(); }; // expected-note {{here}}
>> - A::~A() noexcept {} // expected-error {{does not match previous
>> declaration}}
>> + A::~A() noexcept {} // expected-warning {{previously declared with an
>> implicit exception specification}}
>>
>> struct B { ~B() noexcept; }; // expected-note {{here}}
>> - B::~B() {} // expected-warning {{~B' is missing exception
>> specification 'noexcept'}}
>> + B::~B() {} // expected-warning {{previously declared with an explicit
>> exception specification}}
>>
>> template<typename T> struct C {
>> T t;
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130716/754c8515/attachment.html>
More information about the cfe-commits
mailing list