On 23 August 2017 at 08:01, Alex L via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> Hi Richard, > > It looks like this commit caused PR34298 (https://bugs.llvm.org/show_bu > g.cgi?id=34298). Now Clang fails to compile a libc++ std::function member > variable with an incomplete return type. Libstdc++ works though, so I > assume that the right way to fix this is to change libc++? > I've added some thoughts to the bug. It appears that the rejected code is not required to work per the C++ standard rules, but it really ought to, and this looks like a libc++ bug rather than a Clang bug. (Though the notes we produce aren't particularly clear as to how we got there, sadly.) > Cheers, > Alex > > On 19 October 2016 at 00:39, Richard Smith via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: rsmith >> Date: Tue Oct 18 18:39:12 2016 >> New Revision: 284549 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=284549&view=rev >> Log: >> DR1330: instantiate exception-specifications when "needed". We previously >> did >> not instantiate exception specifications of functions if they were only >> used in >> unevaluated contexts (other than 'noexcept' expressions). >> >> In C++17 onwards, this becomes essential since the exception >> specification is >> now part of the function's type. >> >> Note that this means that constructs like the following no longer work: >> >> struct A { >> static T f() noexcept(...); >> decltype(f()) *p; >> }; >> >> ... because the decltype expression now needs the exception specification >> of >> 'f', which has not yet been parsed. >> >> Modified: >> cfe/trunk/lib/Sema/SemaExpr.cpp >> cfe/trunk/lib/Sema/SemaOverload.cpp >> cfe/trunk/test/CXX/drs/dr13xx.cpp >> cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp >> cfe/trunk/test/SemaCXX/cxx0x-defaulted-functions.cpp >> cfe/trunk/test/SemaCXX/libstdcxx_pair_swap_hack.cpp >> cfe/trunk/test/SemaTemplate/instantiate-exception-spec-cxx11.cpp >> cfe/trunk/www/cxx_dr_status.html >> >> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaE >> xpr.cpp?rev=284549&r1=284548&r2=284549&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Oct 18 18:39:12 2016 >> @@ -2889,6 +2889,14 @@ ExprResult Sema::BuildDeclarationNameExp >> >> { >> QualType type = VD->getType(); >> + if (auto *FPT = type->getAs<FunctionProtoType>()) { >> + // C++ [except.spec]p17: >> + // An exception-specification is considered to be needed when: >> + // - in an expression, the function is the unique lookup result >> or >> + // the selected member of a set of overloaded functions. >> + ResolveExceptionSpec(Loc, FPT); >> + type = VD->getType(); >> + } >> ExprValueKind valueKind = VK_RValue; >> >> switch (D->getKind()) { >> @@ -13138,6 +13146,19 @@ void Sema::MarkFunctionReferenced(Source >> Func->getMemberSpecializationInfo())) >> checkSpecializationVisibility(Loc, Func); >> >> + // C++14 [except.spec]p17: >> + // An exception-specification is considered to be needed when: >> + // - the function is odr-used or, if it appears in an unevaluated >> operand, >> + // would be odr-used if the expression were potentially-evaluated; >> + // >> + // Note, we do this even if MightBeOdrUse is false. That indicates >> that the >> + // function is a pure virtual function we're calling, and in that case >> the >> + // function was selected by overload resolution and we need to resolve >> its >> + // exception specification for a different reason. >> + const FunctionProtoType *FPT = Func->getType()->getAs<Functio >> nProtoType>(); >> + if (FPT && isUnresolvedExceptionSpec(FPT->getExceptionSpecType())) >> + ResolveExceptionSpec(Loc, FPT); >> + >> // If we don't need to mark the function as used, and we don't need to >> // try to provide a definition, there's nothing more to do. >> if ((Func->isUsed(/*CheckUsedAttr=*/false) || !OdrUse) && >> @@ -13196,12 +13217,6 @@ void Sema::MarkFunctionReferenced(Source >> // FIXME: Is this really right? >> if (CurContext == Func) return; >> >> - // Resolve the exception specification for any function which is >> - // used: CodeGen will need it. >> - const FunctionProtoType *FPT = Func->getType()->getAs<Functio >> nProtoType>(); >> - if (FPT && isUnresolvedExceptionSpec(FPT->getExceptionSpecType())) >> - ResolveExceptionSpec(Loc, FPT); >> - >> // Implicit instantiation of function templates and member functions of >> // class templates. >> if (Func->isImplicitlyInstantiable()) { >> >> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaO >> verload.cpp?rev=284549&r1=284548&r2=284549&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Tue Oct 18 18:39:12 2016 >> @@ -13166,6 +13166,13 @@ Expr *Sema::FixOverloadedFunctionReferen >> UnOp->getOperatorLoc()); >> } >> >> + // C++ [except.spec]p17: >> + // An exception-specification is considered to be needed when: >> + // - in an expression the function is the unique lookup result or the >> + // selected member of a set of overloaded functions >> + if (auto *FPT = Fn->getType()->getAs<FunctionProtoType>()) >> + ResolveExceptionSpec(E->getExprLoc(), FPT); >> + >> if (UnresolvedLookupExpr *ULE = dyn_cast<UnresolvedLookupExpr>(E)) { >> // FIXME: avoid copy. >> TemplateArgumentListInfo TemplateArgsBuffer, *TemplateArgs = nullptr; >> >> Modified: cfe/trunk/test/CXX/drs/dr13xx.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/drs/ >> dr13xx.cpp?rev=284549&r1=284548&r2=284549&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/test/CXX/drs/dr13xx.cpp (original) >> +++ cfe/trunk/test/CXX/drs/dr13xx.cpp Tue Oct 18 18:39:12 2016 >> @@ -3,6 +3,91 @@ >> // RUN: %clang_cc1 -std=c++14 %s -verify -fexceptions -fcxx-exceptions >> -pedantic-errors >> // RUN: %clang_cc1 -std=c++1z %s -verify -fexceptions -fcxx-exceptions >> -pedantic-errors >> >> +namespace dr1330 { // dr1330: 4.0 c++11 >> + // exception-specifications are parsed in a context where the class is >> complete. >> + struct A { >> + void f() throw(T) {} >> + struct T {}; >> + >> +#if __cplusplus >= 201103L >> + void g() noexcept(&a == b) {} >> + static int a; >> + static constexpr int *b = &a; >> +#endif >> + }; >> + >> + void (A::*af1)() throw(A::T) = &A::f; >> + void (A::*af2)() throw() = &A::f; // expected-error-re {{{{not >> superset|different exception spec}}}} >> + >> +#if __cplusplus >= 201103L >> + static_assert(noexcept(A().g()), ""); >> +#endif >> + >> + // Likewise, they're instantiated separately from an enclosing class >> template. >> + template<typename U> >> + struct B { >> + void f() throw(T, typename U::type) {} >> + struct T {}; >> + >> +#if __cplusplus >= 201103L >> + void g() noexcept(&a == b && U::value) {} >> + static int a; >> + static constexpr int *b = &a; >> +#endif >> + }; >> + >> + B<int> bi; // ok >> + >> + struct P { >> + typedef int type; >> + static const int value = true; >> + }; >> + >> + void (B<P>::*bpf1)() throw(B<P>::T, int) = &B<P>::f; >> +#if __cplusplus < 201103L >> + // expected-error@-2 {{not superset}} >> + // FIXME: We only delay instantiation in C++11 onwards. In C++98, >> something >> + // weird happens: instantiation of B<P> fails because it references T >> before >> + // it's instantiated, but the diagnostic is suppressed in >> + // Sema::FindInstantiatedDecl because we've already hit an error. This >> is >> + // obviously a bad way to react to this situation; we should still >> producing >> + // the "T has not yet been instantiated" error here, rather than giving >> + // confusing errors later on. >> +#endif >> + void (B<P>::*bpf2)() throw(int) = &B<P>::f; // expected-error-re >> {{{{not superset|different exception spec}}}} >> + void (B<P>::*bpf3)() = &B<P>::f; >> + >> +#if __cplusplus >= 201103L >> + static_assert(noexcept(B<P>().g()), ""); >> + struct Q { static const int value = false; }; >> + static_assert(!noexcept(B<Q>().g()), ""); >> +#endif >> + >> + template<typename T> int f() throw(typename T::error) { return 0; } // >> expected-error 1-4{{prior to '::'}} expected-note 0-1{{instantiation of}} >> + // An exception-specification is needed even if the function is only >> used in >> + // an unevaluated operand. >> + int f1 = sizeof(f<int>()); // expected-note {{instantiation of}} >> +#if __cplusplus >= 201103L >> + decltype(f<char>()) f2; // expected-note {{instantiation of}} >> + bool f3 = noexcept(f<float>()); // expected-note {{instantiation of}} >> +#endif >> + template int f<short>(); // expected-note {{instantiation of}} >> + >> + template<typename T> struct C { >> + C() throw(typename T::type); // expected-error 1-2{{prior to '::'}} >> + }; >> + struct D : C<void> {}; // ok >> +#if __cplusplus < 201103L >> + // expected-note@-2 {{instantiation of}} >> +#endif >> + void f(D &d) { d = d; } // ok >> + >> + // FIXME: In C++11 onwards, we should also note the declaration of 'e' >> as the >> + // line that triggers the use of E::E()'s exception specification. >> + struct E : C<int> {}; // expected-note {{in instantiation of}} >> + E e; >> +} >> + >> namespace dr1346 { // dr1346: 3.5 >> auto a(1); // expected-error 0-1{{extension}} >> auto b(1, 2); // expected-error {{multiple expressions}} >> expected-error 0-1{{extension}} >> >> Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ >> constant-expression-cxx1z.cpp?rev=284549&r1=284548&r2=284549&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp (original) >> +++ cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp Tue Oct 18 >> 18:39:12 2016 >> @@ -25,3 +25,15 @@ namespace BaseClassAggregateInit { >> constexpr D d5 = { __INT_MAX__ }; // expected-error {{must be >> initialized by a constant expression}} >> // expected-note-re@-1 {{in call to 'A({{.*}})'}} >> } >> + >> +namespace NoexceptFunctionTypes { >> + template<typename T> constexpr bool f() noexcept(true) { return true; } >> + static_assert(f<int>()); >> + >> + template<typename T> struct A { >> + constexpr bool f() noexcept(true) { return true; } >> + constexpr bool g() { return f(); } >> + }; >> + static_assert(A<int>().f()); >> + static_assert(A<int>().g()); >> +} >> >> Modified: cfe/trunk/test/SemaCXX/cxx0x-defaulted-functions.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ >> cxx0x-defaulted-functions.cpp?rev=284549&r1=284548&r2=284549&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/test/SemaCXX/cxx0x-defaulted-functions.cpp (original) >> +++ cfe/trunk/test/SemaCXX/cxx0x-defaulted-functions.cpp Tue Oct 18 >> 18:39:12 2016 >> @@ -89,30 +89,34 @@ namespace DefaultedFnExceptionSpec { >> struct Error { >> void f() noexcept(T::error); >> >> - Error() noexcept(T::error); // expected-error {{type 'int' cannot be >> used prior to '::' because it has no members}} >> - Error(const Error&) noexcept(T::error); >> - Error(Error&&) noexcept(T::error); >> - Error &operator=(const Error&) noexcept(T::error); >> - Error &operator=(Error&&) noexcept(T::error); >> - ~Error() noexcept(T::error); // expected-error {{type 'int' cannot >> be used prior to '::' because it has no members}} >> + Error() noexcept(T::error); // expected-error {{type 'int' cannot be >> used prior to '::' because it has no members}} expected-error {{type >> 'char'}} >> + Error(const Error&) noexcept(T::error); // expected-error {{type >> 'int' cannot be used prior to '::' because it has no members}} >> + Error(Error&&) noexcept(T::error); // expected-error {{type 'int' >> cannot be used prior to '::' because it has no members}} >> + Error &operator=(const Error&) noexcept(T::error); // expected-error >> {{type 'int' cannot be used prior to '::' because it has no members}} >> expected-error {{type 'double'}} >> + Error &operator=(Error&&) noexcept(T::error); // expected-error >> {{type 'int' cannot be used prior to '::' because it has no members}} >> + ~Error() noexcept(T::error); // expected-error {{type 'int' cannot >> be used prior to '::' because it has no members}} expected-error {{type >> 'char'}} >> }; >> >> + Error<char> c; // expected-note 2{{instantiation of}} >> struct DelayImplicit { >> - Error<int> e; >> + // FIXME: The location of this note is terrible. The instantiation >> was >> + // triggered by the uses of the functions in the decltype >> expressions below. >> + Error<int> e; // expected-note 6{{instantiation of}} >> }; >> + Error<float> *e; >> >> - // Don't instantiate the exception specification here. >> + // An exception specification is needed if the exception specification >> for a >> + // a defaulted special member function that calls the function is >> needed. >> + // Use in an unevaluated operand still results in the exception spec >> being >> + // needed. >> void test1(decltype(declval<DelayImplicit>() = >> DelayImplicit(DelayImplicit()))); >> void test2(decltype(declval<DelayImplicit>() = declval<const >> DelayImplicit>())); >> void test3(decltype(DelayImplicit(declval<const DelayImplicit>()))); >> >> - // Any odr-use causes the exception specification to be evaluated. >> - struct OdrUse { // \ >> - expected-note {{instantiation of exception specification for >> 'Error'}} \ >> - expected-note {{instantiation of exception specification for >> '~Error'}} >> - Error<int> e; >> - }; >> - OdrUse use; // expected-note {{implicit default constructor for >> 'DefaultedFnExceptionSpec::OdrUse' first required here}} >> + // Any odr-use needs the exception specification. >> + void f(Error<double> *p) { >> + *p = *p; // expected-note {{instantiation of}} >> + } >> } >> >> namespace PR13527 { >> >> Modified: cfe/trunk/test/SemaCXX/libstdcxx_pair_swap_hack.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ >> libstdcxx_pair_swap_hack.cpp?rev=284549&r1=284548&r2=284549&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/test/SemaCXX/libstdcxx_pair_swap_hack.cpp (original) >> +++ cfe/trunk/test/SemaCXX/libstdcxx_pair_swap_hack.cpp Tue Oct 18 >> 18:39:12 2016 >> @@ -69,6 +69,7 @@ namespace sad { >> >> template<typename A, typename B> struct CLASS { >> void swap(CLASS &other) noexcept(noexcept(swap(*this, other))); // >> expected-error {{too many arguments}} expected-note {{declared here}} >> + // expected-error@-1{{uses itself}} expected-note@-1{{in >> instantiation of}} >> }; >> >> CLASS<int, int> pi; >> >> Modified: cfe/trunk/test/SemaTemplate/instantiate-exception-spec-cxx11 >> .cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTempl >> ate/instantiate-exception-spec-cxx11.cpp?rev=284549&r1= >> 284548&r2=284549&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/test/SemaTemplate/instantiate-exception-spec-cxx11.cpp >> (original) >> +++ cfe/trunk/test/SemaTemplate/instantiate-exception-spec-cxx11.cpp Tue >> Oct 18 18:39:12 2016 >> @@ -30,8 +30,6 @@ template<unsigned N> struct S { >> // expected-error {{no member named 'recurse'}} \ >> // expected-note 9{{instantiation of exception spec}} >> }; >> -decltype(S<0>::recurse()) *pVoid1 = 0; // ok, exception spec not needed >> -decltype(&S<0>::recurse) pFn = 0; // ok, exception spec not needed >> >> template<> struct S<10> {}; >> void (*pFn2)() noexcept = &S<0>::recurse; // expected-note >> {{instantiation of exception spec}} expected-error {{not superset}} >> @@ -39,7 +37,7 @@ void (*pFn2)() noexcept = &S<0>::recurse >> >> namespace dr1330_example { >> template <class T> struct A { >> - void f(...) throw (typename T::X); // expected-error {{'int'}} >> + void f(...) throw (typename T::X); >> void f(int); >> }; >> >> @@ -48,16 +46,14 @@ namespace dr1330_example { >> } >> >> struct S { >> - template<typename T> >> - static int f() noexcept(noexcept(A<T>().f("boo!"))) { return 0; } >> // \ >> - // expected-note {{instantiation of exception spec}} >> - typedef decltype(f<S>()) X; >> + template<typename T> static void f() throw(typename T::X); >> + typedef decltype(f<S>()) X; // expected-error {{exception >> specification is not available}} >> }; >> >> - int test2() { >> - S().f<S>(); // ok >> - S().f<int>(); // expected-note {{instantiation of exception spec}} >> - } >> + struct T { >> + template<typename T> static void f() throw(T**); >> + typedef decltype(f<S>()) X; // expected-error {{exception >> specification is not available}} >> + }; >> >> template<typename T> >> struct U { >> >> Modified: cfe/trunk/www/cxx_dr_status.html >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_dr_sta >> tus.html?rev=284549&r1=284548&r2=284549&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/www/cxx_dr_status.html (original) >> +++ cfe/trunk/www/cxx_dr_status.html Tue Oct 18 18:39:12 2016 >> @@ -3613,7 +3613,7 @@ and <I>POD class</I></td> >> <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed >> .html#595">595</a></td> >> <td>dup</td> >> <td>Exception specifications in templates instantiated from class >> bodies</td> >> - <td class="none" align="center">Duplicate of <a >> href="#1330">1330</a></td> >> + <td class="svn" align="center">Duplicate of <a >> href="#1330">1330</a></td> >> </tr> >> <tr class="open" id="596"> >> <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active >> .html#596">596</a></td> >> @@ -7795,7 +7795,7 @@ and <I>POD class</I></td> >> <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defect >> s.html#1330">1330</a></td> >> <td>CD3</td> >> <td>Delayed instantiation of <TT>noexcept</TT> specifiers</td> >> - <td class="none" align="center">Unknown</td> >> + <td class="svn" align="center">SVN (C++11 onwards)</td> >> </tr> >> <tr class="open" id="1331"> >> <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed >> .html#1331">1331</a></td> >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
