erichkeane updated this revision to Diff 201673. erichkeane marked 5 inline comments as done. erichkeane added a comment.
Added warning + other comments from @aaron.ballman The exception state was further along than I expected, so I was able to make the warning better than I thought! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62435/new/ https://reviews.llvm.org/D62435 Files: clang/include/clang-c/Index.h clang/include/clang/AST/Decl.h clang/include/clang/AST/Type.h clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Basic/ExceptionSpecificationType.h clang/include/clang/Sema/DeclSpec.h clang/lib/AST/ASTContext.cpp clang/lib/AST/JSONNodeDumper.cpp clang/lib/AST/Type.cpp clang/lib/Sema/SemaDeclAttr.cpp clang/lib/Sema/SemaDeclCXX.cpp clang/lib/Sema/SemaExprCXX.cpp clang/lib/Sema/SemaType.cpp clang/test/SemaCXX/nothrow-vs-exception-specs.cpp clang/tools/libclang/CXType.cpp
Index: clang/tools/libclang/CXType.cpp =================================================================== --- clang/tools/libclang/CXType.cpp +++ clang/tools/libclang/CXType.cpp @@ -742,6 +742,8 @@ return CXCursor_ExceptionSpecificationKind_MSAny; case EST_BasicNoexcept: return CXCursor_ExceptionSpecificationKind_BasicNoexcept; + case EST_NoThrow: + return CXCursor_ExceptionSpecificationKind_NoThrow; case EST_NoexceptFalse: case EST_NoexceptTrue: case EST_DependentNoexcept: Index: clang/test/SemaCXX/nothrow-vs-exception-specs.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/nothrow-vs-exception-specs.cpp @@ -0,0 +1,55 @@ +// RUN: %clang_cc1 %s -fcxx-exceptions -fdeclspec -fsyntax-only -Wexceptions -verify -std=c++14 +// RUN: %clang_cc1 %s -fcxx-exceptions -fdeclspec -fsyntax-only -Wexceptions -verify -std=c++17 -DCPP17 + +__attribute__((nothrow)) void f1(); +static_assert(noexcept(f1()), ""); +void f1() noexcept; +// expected-error@+2 {{exception specification in declaration does not match previous declaration}} +// expected-note@-2 {{previous declaration is here}} +void f1() noexcept(false); + +__attribute__((nothrow)) void f2(); +static_assert(noexcept(f2()), ""); +// expected-error@+2 {{exception specification in declaration does not match previous declaration}} +// expected-note@-3 {{previous declaration is here}} +void f2() noexcept(false); + +void f3() __attribute__((nothrow)); +static_assert(noexcept(f3()), ""); +void f3() noexcept; +// expected-error@+2 {{exception specification in declaration does not match previous declaration}} +// expected-note@-2 {{previous declaration is here}} +void f3() noexcept(false); + +// Still noexcept due to throw() +__attribute__((nothrow)) void f4() throw(); +static_assert(noexcept(f4()), ""); + +// Still noexcept due to noexcept +__attribute__((nothrow)) void f5() noexcept; +static_assert(noexcept(f5()), ""); + +// Still noexcept due to noexcept(true) +__attribute__((nothrow)) void f6() noexcept(true); +static_assert(noexcept(f6()), ""); + +#ifndef CPP17 +// Doesn't override C++ implementation. +// expected-warning@+1{{nothrow attribute conflicts with exception specification; attribute ignored}} +__attribute__((nothrow)) void f7() throw(int); +static_assert(!noexcept(f7()), ""); +#endif + +// Doesn't override C++ implementation. +// expected-warning@+1{{nothrow attribute conflicts with exception specification; attribute ignored}} +__attribute__((nothrow)) void f8() noexcept(false); +static_assert(!noexcept(f8()), ""); + +__declspec(nothrow) void foo1() noexcept; +__declspec(nothrow) void foo2() noexcept(true); +// expected-warning@+1{{nothrow attribute conflicts with exception specification; attribute ignored}} +__declspec(nothrow) void foo3() noexcept(false); +__declspec(nothrow) void foo4() noexcept(noexcept(foo1())); +__declspec(nothrow) void foo5() noexcept(noexcept(foo2())); +// expected-warning@+1{{nothrow attribute conflicts with exception specification; attribute ignored}} +__declspec(nothrow) void foo6() noexcept(noexcept(foo3())); Index: clang/lib/Sema/SemaType.cpp =================================================================== --- clang/lib/Sema/SemaType.cpp +++ clang/lib/Sema/SemaType.cpp @@ -130,6 +130,7 @@ case ParsedAttr::AT_Regparm: \ case ParsedAttr::AT_AnyX86NoCallerSavedRegisters: \ case ParsedAttr::AT_AnyX86NoCfCheck: \ + case ParsedAttr::AT_NoThrow: \ CALLING_CONV_ATTRS_CASELIST // Microsoft-specific type qualifiers. @@ -4516,7 +4517,7 @@ // If the function declarator has a prototype (i.e. it is not () and // does not have a K&R-style identifier list), then the arguments are part // of the type, otherwise the argument list is (). - const DeclaratorChunk::FunctionTypeInfo &FTI = DeclType.Fun; + DeclaratorChunk::FunctionTypeInfo &FTI = DeclType.Fun; IsQualifiedFunction = FTI.hasMethodTypeQualifiers() || FTI.hasRefQualifier(); @@ -6945,6 +6946,58 @@ return true; } + if (attr.getKind() == ParsedAttr::AT_NoThrow) { + if (S.CheckAttrNoArgs(attr)) + return true; + + // Delay if this is not a function type. + if (!unwrapped.isFunctionType()) + return false; + + // Otherwise we can process right away. + auto *Proto = unwrapped.get()->getAs<FunctionProtoType>(); + + // In the case where this is a FunctionNoProtoType instead of a + // FunctionProtoType, let the existing NoThrowAttr implementation do its + // thing. + if (!Proto) + return false; + + attr.setUsedAsTypeAttr(); + + // MSVC ignores nothrow for exception specification if it is in conflict. + if (Proto->hasExceptionSpec()) { + switch (Proto->getExceptionSpecType()) { + case EST_None: llvm_unreachable("This doesn't have an exception spec!"); + case EST_DynamicNone: + case EST_BasicNoexcept: + case EST_NoexceptTrue: + case EST_NoThrow: + // Exception spec doesn't conflict with nothrow, so don't warn. + break; + + case EST_Dynamic: + case EST_MSAny: + case EST_NoexceptFalse: + case EST_DependentNoexcept: + case EST_Unevaluated: + case EST_Uninstantiated: + case EST_Unparsed: + S.Diag(attr.getLoc(), diag::warn_nothrow_attribute_ignored); + break; + } + return true; + } + + type = unwrapped.wrap( + S, S.Context + .getFunctionTypeWithExceptionSpec( + QualType{Proto, 0}, + FunctionProtoType::ExceptionSpecInfo{EST_NoThrow}) + ->getAs<FunctionType>()); + return true; + } + // Delay if the type didn't work out to a function. if (!unwrapped.isFunctionType()) return false; Index: clang/lib/Sema/SemaExprCXX.cpp =================================================================== --- clang/lib/Sema/SemaExprCXX.cpp +++ clang/lib/Sema/SemaExprCXX.cpp @@ -6058,6 +6058,8 @@ if (EST2 == EST_NoexceptFalse) return ESI2; // If either of them is non-throwing, the result is the other. + if (EST1 == EST_NoThrow) return ESI2; + if (EST2 == EST_NoThrow) return ESI1; if (EST1 == EST_DynamicNone) return ESI2; if (EST2 == EST_DynamicNone) return ESI1; if (EST1 == EST_BasicNoexcept) return ESI2; @@ -6086,6 +6088,7 @@ case EST_DependentNoexcept: case EST_NoexceptFalse: case EST_NoexceptTrue: + case EST_NoThrow: llvm_unreachable("handled above"); case EST_Dynamic: { Index: clang/lib/Sema/SemaDeclCXX.cpp =================================================================== --- clang/lib/Sema/SemaDeclCXX.cpp +++ clang/lib/Sema/SemaDeclCXX.cpp @@ -192,6 +192,7 @@ // If this function has a basic noexcept, it doesn't affect the outcome. case EST_BasicNoexcept: case EST_NoexceptTrue: + case EST_NoThrow: return; // If we're still at noexcept(true) and there's a throw() callee, // change to that specification. @@ -15457,6 +15458,7 @@ case EST_Uninstantiated: case EST_Unevaluated: case EST_BasicNoexcept: + case EST_NoThrow: case EST_DynamicNone: case EST_MSAny: case EST_None: Index: clang/lib/Sema/SemaDeclAttr.cpp =================================================================== --- clang/lib/Sema/SemaDeclAttr.cpp +++ clang/lib/Sema/SemaDeclAttr.cpp @@ -6853,7 +6853,8 @@ handleNoCfCheckAttr(S, D, AL); break; case ParsedAttr::AT_NoThrow: - handleSimpleAttribute<NoThrowAttr>(S, D, AL); + if (!AL.isUsedAsTypeAttr()) + handleSimpleAttribute<NoThrowAttr>(S, D, AL); break; case ParsedAttr::AT_CUDAShared: handleSharedAttr(S, D, AL); Index: clang/lib/AST/Type.cpp =================================================================== --- clang/lib/AST/Type.cpp +++ clang/lib/AST/Type.cpp @@ -3077,6 +3077,7 @@ case EST_DynamicNone: case EST_BasicNoexcept: case EST_NoexceptTrue: + case EST_NoThrow: return CT_Cannot; case EST_None: Index: clang/lib/AST/JSONNodeDumper.cpp =================================================================== --- clang/lib/AST/JSONNodeDumper.cpp +++ clang/lib/AST/JSONNodeDumper.cpp @@ -464,7 +464,9 @@ //JOS.attributeWithCall("exceptionSpecExpr", // [this, E]() { Visit(E.ExceptionSpec.NoexceptExpr); }); break; - + case EST_NoThrow: + JOS.attribute("exceptionSpec", "nothrow"); + break; // FIXME: I cannot find a way to trigger these cases while dumping the AST. I // suspect you can only run into them when executing an AST dump from within // the debugger, which is not a use case we worry about for the JSON dumping Index: clang/lib/AST/ASTContext.cpp =================================================================== --- clang/lib/AST/ASTContext.cpp +++ clang/lib/AST/ASTContext.cpp @@ -3742,7 +3742,10 @@ break; } - case EST_DynamicNone: case EST_BasicNoexcept: case EST_NoexceptTrue: + case EST_DynamicNone: + case EST_BasicNoexcept: + case EST_NoexceptTrue: + case EST_NoThrow: CanonicalEPI.ExceptionSpec.Type = EST_BasicNoexcept; break; Index: clang/include/clang/Sema/DeclSpec.h =================================================================== --- clang/include/clang/Sema/DeclSpec.h +++ clang/include/clang/Sema/DeclSpec.h @@ -1544,6 +1544,12 @@ const ParsedAttributesView &getAttrs() const { return AttrList; } ParsedAttributesView &getAttrs() { return AttrList; } + bool hasAttr(ParsedAttr::Kind Kind) const { + return llvm::find_if(getAttrs(), [Kind](const ParsedAttr &P) { + return P.getKind() == Kind; + }) != getAttrs().end(); + } + /// Return a DeclaratorChunk for a pointer. static DeclaratorChunk getPointer(unsigned TypeQuals, SourceLocation Loc, SourceLocation ConstQualLoc, Index: clang/include/clang/Basic/ExceptionSpecificationType.h =================================================================== --- clang/include/clang/Basic/ExceptionSpecificationType.h +++ clang/include/clang/Basic/ExceptionSpecificationType.h @@ -22,6 +22,7 @@ EST_DynamicNone, ///< throw() EST_Dynamic, ///< throw(T1, T2) EST_MSAny, ///< Microsoft throw(...) extension + EST_NoThrow, ///< Microsoft __declspec(nothrow) extension EST_BasicNoexcept, ///< noexcept EST_DependentNoexcept,///< noexcept(expression), value-dependent EST_NoexceptFalse, ///< noexcept(expression), evals to 'false' @@ -41,7 +42,8 @@ } inline bool isNoexceptExceptionSpec(ExceptionSpecificationType ESpecType) { - return ESpecType == EST_BasicNoexcept || isComputedNoexcept(ESpecType); + return ESpecType == EST_BasicNoexcept || ESpecType == EST_NoThrow || + isComputedNoexcept(ESpecType); } inline bool isUnresolvedExceptionSpec(ExceptionSpecificationType ESpecType) { Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2786,6 +2786,9 @@ InGroup<IgnoredAttributes>; def warn_attribute_ignored : Warning<"%0 attribute ignored">, InGroup<IgnoredAttributes>; +def warn_nothrow_attribute_ignored : Warning<"nothrow attribute conflicts with" + " exception specification; attribute ignored">, + InGroup<IgnoredAttributes>; def warn_attribute_ignored_on_inline : Warning<"%0 attribute ignored on inline function">, InGroup<IgnoredAttributes>; Index: clang/include/clang/AST/Type.h =================================================================== --- clang/include/clang/AST/Type.h +++ clang/include/clang/AST/Type.h @@ -3855,6 +3855,7 @@ case EST_MSAny: case EST_BasicNoexcept: case EST_Unparsed: + case EST_NoThrow: return {0, 0, 0}; case EST_Dynamic: Index: clang/include/clang/AST/Decl.h =================================================================== --- clang/include/clang/AST/Decl.h +++ clang/include/clang/AST/Decl.h @@ -2330,6 +2330,14 @@ return T->castAs<FunctionType>()->getReturnType(); } + /// Gets the ExceptionSpecificationType as declared. + ExceptionSpecificationType getExceptionSpecType() const { + auto *TSI = getTypeSourceInfo(); + QualType T = TSI ? TSI->getType() : getType(); + const auto *FPT = T->getAs<FunctionProtoType>(); + return FPT ? FPT->getExceptionSpecType() : EST_None; + } + /// Attempt to compute an informative source range covering the /// function exception specification, if any. SourceRange getExceptionSpecSourceRange() const; Index: clang/include/clang-c/Index.h =================================================================== --- clang/include/clang-c/Index.h +++ clang/include/clang-c/Index.h @@ -32,7 +32,7 @@ * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable. */ #define CINDEX_VERSION_MAJOR 0 -#define CINDEX_VERSION_MINOR 57 +#define CINDEX_VERSION_MINOR 58 #define CINDEX_VERSION_ENCODE(major, minor) ( \ ((major) * 10000) \ @@ -221,7 +221,12 @@ /** * The exception specification has not been parsed yet. */ - CXCursor_ExceptionSpecificationKind_Unparsed + CXCursor_ExceptionSpecificationKind_Unparsed, + + /** + * The cursor has a __declspec(nothrow) exception specification. + */ + CXCursor_ExceptionSpecificationKind_NoThrow }; /**
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits