It IS an attribute that works on Linux as well (about ½ way down this page: https://gcc.gnu.org/onlinedocs/gcc-4.7.2/gcc/Function-Attributes.html), though used rarely (at least in C++ code) as far as I understand.
The fact that a mistake like this is in the ATL headers is frustrating, but I’m not sure the best way to deal with this. Aaron and I both believed the warning to be meaningful/useful (since this conflict is the same as saying throw() noexcept(false)), so I’m not sure removing it is a good idea (if that’s what you’re suggesting). However, if this is something reasonably common on Microsoft implementations, I wonder if it is a warning we can make off-by-default on clang-cl. It would be a shame to lose it, but if this is a commonly enough used problem we might be stuck with it. From: Nico Weber [mailto:tha...@chromium.org] Sent: Friday, May 31, 2019 6:58 AM To: Keane, Erich <erich.ke...@intel.com> Cc: Aaron Ballman <aa...@aaronballman.com>; cfe-commits <cfe-commits@lists.llvm.org> Subject: Re: r362119 - Add Attribute NoThrow as an Exception Specifier Type I'm not sure this is the best fix. From the patch description, it sounds like this is a Microsoft-specific change. So splitting this into a new diag group that then everyone using the Microsoft headers has to disable seems a bit roundabout – don't we get the same behavior by not emitting this warning in the first place, except that we don't require everyone using clang-cl to explicitly disable this warning then? On Fri, May 31, 2019 at 9:38 AM Keane, Erich <erich.ke...@intel.com<mailto:erich.ke...@intel.com>> wrote: Presumably the right choice for you on this one is to separate the diagnostic into a new group. Copying Aaron since he might have an idea of an existing group, otherwise I’ll push a commit to put it in a new one. Thanks for the report! -Erich From: Nico Weber [mailto:tha...@chromium.org<mailto:tha...@chromium.org>] Sent: Thursday, May 30, 2019 6:15 PM To: Keane, Erich <erich.ke...@intel.com<mailto:erich.ke...@intel.com>> Cc: cfe-commits <cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> Subject: Re: r362119 - Add Attribute NoThrow as an Exception Specifier Type Hello, this causes this diagnostic when building on Windows: In file included from ../../ui/accessibility/platform/ax_platform_node_textchildprovider_win.cc:10: ../..\ui/accessibility/platform/ax_platform_node_textprovider_win.h(22,3): error: 'nothrow' attribute conflicts with exception specification; attribute ignored [-Werror,-Wignored-attributes] END_COM_MAP() ^ ..\..\third_party\depot_tools\win_toolchain\vs_files\ 818a152b3f1da991c1725d85be19a0f27af6bab4\VC\Tools\MSVC\14.16.27023\atlmfc\include\atlcom.h(2358,2): note: expanded from macro 'END_COM_MAP' STDMETHOD(QueryInterface)( \ ^ ..\..\third_party\depot_tools\win_toolchain\vs_files\ 818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(170,42): note: expanded from macro 'STDMETHOD' #define STDMETHOD(method) virtual COM_DECLSPEC_NOTHROW HRESULT STDMETHODCALLTYPE method ^ ..\..\third_party\depot_tools\win_toolchain\vs_files\ 818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\combaseapi.h(165,30): note: expanded from macro 'COM_DECLSPEC_NOTHROW' #define COM_DECLSPEC_NOTHROW DECLSPEC_NOTHROW ^ ..\..\third_party\depot_tools\win_toolchain\vs_files\ 818a152b3f1da991c1725d85be19a0f27af6bab4\win_sdk\Include\10.0.17763.0\um\winnt.h(200,39): note: expanded from macro 'DECLSPEC_NOTHROW' #define DECLSPEC_NOTHROW __declspec(nothrow) ^ Suggestions? We don't want to use -Wno-ignored-attributes since that disables this warning for all attributes. All the involved macros are in system headers and we have no control over them. On Thu, May 30, 2019 at 1:28 PM Erich Keane via cfe-commits <cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote: Author: erichkeane Date: Thu May 30 10:31:54 2019 New Revision: 362119 URL: http://llvm.org/viewvc/llvm-project?rev=362119&view=rev Log: Add Attribute NoThrow as an Exception Specifier Type In response to https://bugs.llvm.org/show_bug.cgi?id=33235, it became clear that the current mechanism of hacking through checks for the exception specification of a function gets confused really quickly when there are alternate exception specifiers. This patch introcues EST_NoThrow, which is the equivilent of EST_noexcept when caused by EST_noThrow. The existing implementation is left in place to cover functions with no FunctionProtoType. Differential Revision: https://reviews.llvm.org/D62435 Added: cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp (with props) Modified: cfe/trunk/include/clang-c/Index.h cfe/trunk/include/clang/AST/Decl.h cfe/trunk/include/clang/AST/Type.h cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/include/clang/Basic/ExceptionSpecificationType.h cfe/trunk/lib/AST/ASTContext.cpp cfe/trunk/lib/AST/JSONNodeDumper.cpp cfe/trunk/lib/AST/Type.cpp cfe/trunk/lib/Sema/SemaDeclAttr.cpp cfe/trunk/lib/Sema/SemaDeclCXX.cpp cfe/trunk/lib/Sema/SemaExprCXX.cpp cfe/trunk/lib/Sema/SemaType.cpp cfe/trunk/tools/libclang/CXType.cpp Modified: cfe/trunk/include/clang-c/Index.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=362119&r1=362118&r2=362119&view=diff ============================================================================== --- cfe/trunk/include/clang-c/Index.h (original) +++ cfe/trunk/include/clang-c/Index.h Thu May 30 10:31:54 2019 @@ -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 @@ enum CXCursor_ExceptionSpecificationKind /** * 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 }; /** Modified: cfe/trunk/include/clang/AST/Decl.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=362119&r1=362118&r2=362119&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/Decl.h (original) +++ cfe/trunk/include/clang/AST/Decl.h Thu May 30 10:31:54 2019 @@ -2330,6 +2330,14 @@ public: 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; Modified: cfe/trunk/include/clang/AST/Type.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=362119&r1=362118&r2=362119&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/Type.h (original) +++ cfe/trunk/include/clang/AST/Type.h Thu May 30 10:31:54 2019 @@ -3855,6 +3855,7 @@ private: case EST_MSAny: case EST_BasicNoexcept: case EST_Unparsed: + case EST_NoThrow: return {0, 0, 0}; case EST_Dynamic: Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=362119&r1=362118&r2=362119&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu May 30 10:31:54 2019 @@ -2792,6 +2792,9 @@ def warn_dllimport_dropped_from_inline_f 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>; Modified: cfe/trunk/include/clang/Basic/ExceptionSpecificationType.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/ExceptionSpecificationType.h?rev=362119&r1=362118&r2=362119&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/ExceptionSpecificationType.h (original) +++ cfe/trunk/include/clang/Basic/ExceptionSpecificationType.h Thu May 30 10:31:54 2019 @@ -22,6 +22,7 @@ enum ExceptionSpecificationType { 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 isComputedNoexcept(Exception } 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) { Modified: cfe/trunk/lib/AST/ASTContext.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=362119&r1=362118&r2=362119&view=diff ============================================================================== --- cfe/trunk/lib/AST/ASTContext.cpp (original) +++ cfe/trunk/lib/AST/ASTContext.cpp Thu May 30 10:31:54 2019 @@ -3742,7 +3742,10 @@ QualType ASTContext::getFunctionTypeInte 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; Modified: cfe/trunk/lib/AST/JSONNodeDumper.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/JSONNodeDumper.cpp?rev=362119&r1=362118&r2=362119&view=diff ============================================================================== --- cfe/trunk/lib/AST/JSONNodeDumper.cpp (original) +++ cfe/trunk/lib/AST/JSONNodeDumper.cpp Thu May 30 10:31:54 2019 @@ -464,7 +464,9 @@ void JSONNodeDumper::VisitFunctionProtoT //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 Modified: cfe/trunk/lib/AST/Type.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=362119&r1=362118&r2=362119&view=diff ============================================================================== --- cfe/trunk/lib/AST/Type.cpp (original) +++ cfe/trunk/lib/AST/Type.cpp Thu May 30 10:31:54 2019 @@ -3077,6 +3077,7 @@ CanThrowResult FunctionProtoType::canThr case EST_DynamicNone: case EST_BasicNoexcept: case EST_NoexceptTrue: + case EST_NoThrow: return CT_Cannot; case EST_None: Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=362119&r1=362118&r2=362119&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu May 30 10:31:54 2019 @@ -6853,7 +6853,8 @@ static void ProcessDeclAttribute(Sema &S 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); Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=362119&r1=362118&r2=362119&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu May 30 10:31:54 2019 @@ -192,6 +192,7 @@ Sema::ImplicitExceptionSpecification::Ca // 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 @@ bool Sema::checkThisInStaticMemberFuncti case EST_Uninstantiated: case EST_Unevaluated: case EST_BasicNoexcept: + case EST_NoThrow: case EST_DynamicNone: case EST_MSAny: case EST_None: Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=362119&r1=362118&r2=362119&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original) +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Thu May 30 10:31:54 2019 @@ -6045,6 +6045,8 @@ mergeExceptionSpecs(Sema &S, FunctionPro 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; @@ -6073,6 +6075,7 @@ mergeExceptionSpecs(Sema &S, FunctionPro case EST_DependentNoexcept: case EST_NoexceptFalse: case EST_NoexceptTrue: + case EST_NoThrow: llvm_unreachable("handled above"); case EST_Dynamic: { Modified: cfe/trunk/lib/Sema/SemaType.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=362119&r1=362118&r2=362119&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaType.cpp (original) +++ cfe/trunk/lib/Sema/SemaType.cpp Thu May 30 10:31:54 2019 @@ -130,6 +130,7 @@ static void diagnoseBadTypeAttribute(Sem 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 @@ static TypeSourceInfo *GetFullTypeForDec // 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,61 @@ static bool handleFunctionTypeAttr(TypeP 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 if it is in conflict with an explicit exception + // specification. + if (Proto->hasExceptionSpec()) { + switch (Proto->getExceptionSpecType()) { + case EST_None: + llvm_unreachable("This doesn't have an exception spec!"); + LLVM_FALLTHROUGH; + 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; Added: cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp?rev=362119&view=auto ============================================================================== --- cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp (added) +++ cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp Thu May 30 10:31:54 2019 @@ -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())); Propchange: cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp ------------------------------------------------------------------------------ svn:eol-style = native Propchange: cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp ------------------------------------------------------------------------------ svn:keywords = Author Date Id Rev URL Propchange: cfe/trunk/test/SemaCXX/nothrow-vs-exception-specs.cpp ------------------------------------------------------------------------------ svn:mime-type = text/plain Modified: cfe/trunk/tools/libclang/CXType.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXType.cpp?rev=362119&r1=362118&r2=362119&view=diff ============================================================================== --- cfe/trunk/tools/libclang/CXType.cpp (original) +++ cfe/trunk/tools/libclang/CXType.cpp Thu May 30 10:31:54 2019 @@ -742,6 +742,8 @@ getExternalExceptionSpecificationKind(Ex 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: _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
