This problem is addressed by https://reviews.llvm.org/D36353.
Thanks, --Serge 2017-08-05 8:44 GMT+07:00 Alexander Kornienko <ale...@google.com>: > Thank you for the analysis! It would be nice if you could implement a > short term workaround to avoid crashes. Maybe Richard has ideas on what a > proper solution should look like. > > > On Wed, Aug 2, 2017 at 8:05 PM, Serge Pavlov <sepavl...@gmail.com> wrote: > >> At first thank you for the nice test case. >> >> This crash is not caused by r305903. The root cause is that instantiation >> of f<int> is triggered when parse of f<T> is not finished yet. This is the >> case just addressed by that change. >> The instantiation is requested by the code (https://github.com/llvm-mirro >> r/clang/blob/master/lib/Sema/SemaExpr.cpp#L13622): >> >> else if (Func->isConstexpr()) >> // Do not defer instantiations of constexpr functions, to avoid >> the >> // expression evaluator needing to call back into Sema if it sees >> a >> // call to such a function. >> InstantiateFunctionDefinition(PointOfInstantiation, Func); >> >> >> It looks like a violation of C++ Standard, as function is instantiated >> when there is no use of it. However if this check is removed and >> instantiation is postponed as for non-constexpr functions, lots of tests >> fail. >> >> Without r305903 the call to `getDefinition` for templated declaration of >> f<T> returned null, declaration f<int> was moved to the list of pending >> instantiations and crash is not observed. It does not mean however that >> original source code would be compiled correctly. >> >> It this is urgent problem, I can prepare a patch that will mimic behavior >> before r305903 for instantiation of constexpr functions. If there is some >> time I would investigate this problem in depth, as it manifests itself in >> number of cases, for instance in https://bugs.llvm.org/show_ >> bug.cgi?id=33561. >> >> Thanks, >> --Serge >> >> 2017-08-01 23:28 GMT+07:00 Serge Pavlov <sepavl...@gmail.com>: >> >>> Yes, sure, I will investigate it. >>> >>> Thanks, >>> --Serge >>> >>> 2017-08-01 21:32 GMT+07:00 Alexander Kornienko <ale...@google.com>: >>> >>>> This change causes an assertion failure on valid code. Could you take a >>>> look at fixing this? >>>> >>>> A reduced test case: >>>> >>>> $ cat /tmp/SemaTemplateInstantiateDecl-crash2.cpp >>>> template <class T> >>>> constexpr void f(T) { >>>> f(0); >>>> } >>>> $ clang -fsyntax-only -std=c++11 /tmp/SemaTemplateInstantiateDe >>>> cl-crash2.cpp >>>> assert.h assertion failed at llvm/tools/clang/lib/Sema/Sema >>>> TemplateInstantiateDecl.cpp:3840 in void clang::Sema::InstantiateFuncti >>>> onDefinition(clang::SourceLocation, clang::FunctionDecl *, bool, bool, >>>> bool): (Pattern | >>>> | PatternDecl->isDefaulted()) && "unexpected kind of function template >>>> definition" >>>> *** Check failure stack trace: *** >>>> @ 0x6094c4a __assert_fail >>>> @ 0x2705bfe clang::Sema::InstantiateFunctionDefinition() >>>> @ 0x2c1fb13 clang::Sema::MarkFunctionReferenced() >>>> @ 0x2bec07b clang::Sema::MarkAnyDeclReferenced() >>>> @ 0x2c2327f MarkExprReferenced() >>>> @ 0x2be8f0a clang::Sema::MarkDeclRefReferenced() >>>> @ 0x2980ac0 clang::Sema::FixOverloadedFunctionReference() >>>> @ 0x2982e66 FinishOverloadedCallExpr() >>>> @ 0x2982b39 clang::Sema::BuildOverloadedCallExpr() >>>> @ 0x2be461b clang::Sema::ActOnCallExpr() >>>> @ 0x2571e90 clang::Parser::ParsePostfixExpressionSuffix() >>>> @ 0x25784cb clang::Parser::ParseCastExpression() >>>> @ 0x2570865 clang::Parser::ParseCastExpression() >>>> @ 0x256f1e3 clang::Parser::ParseAssignmentExpression() >>>> @ 0x256f0bf clang::Parser::ParseExpression() >>>> @ 0x2517eeb clang::Parser::ParseExprStatement() >>>> @ 0x2517074 clang::Parser::ParseStatement >>>> OrDeclarationAfterAttributes() >>>> @ 0x2516b50 clang::Parser::ParseStatementOrDeclaration() >>>> @ 0x251deb4 clang::Parser::ParseCompoundStatementBody() >>>> @ 0x251ea53 clang::Parser::ParseFunctionStatementBody() >>>> @ 0x24f5b23 clang::Parser::ParseFunctionDefinition() >>>> @ 0x25082a5 clang::Parser::ParseSingleDec >>>> larationAfterTemplate() >>>> @ 0x2507652 clang::Parser::ParseTemplateD >>>> eclarationOrSpecialization() >>>> @ 0x2506fa5 clang::Parser::ParseDeclarati >>>> onStartingWithTemplate() >>>> @ 0x25b6853 clang::Parser::ParseDeclaration() >>>> @ 0x24f36d5 clang::Parser::ParseExternalDeclaration() >>>> @ 0x24f2739 clang::Parser::ParseTopLevelDecl() >>>> @ 0x24f220e clang::Parser::ParseFirstTopLevelDecl() >>>> @ 0x24ed582 clang::ParseAST() >>>> >>>> On Wed, Jun 21, 2017 at 2:46 PM, Serge Pavlov via cfe-commits < >>>> cfe-commits@lists.llvm.org> wrote: >>>> >>>>> Author: sepavloff >>>>> Date: Wed Jun 21 07:46:57 2017 >>>>> New Revision: 305903 >>>>> >>>>> URL: http://llvm.org/viewvc/llvm-project?rev=305903&view=rev >>>>> Log: >>>>> Function with unparsed body is a definition >>>>> >>>>> While a function body is being parsed, the function declaration is not >>>>> considered >>>>> as a definition because it does not have a body yet. In some cases it >>>>> leads to >>>>> incorrect interpretation, the case is presented in >>>>> https://bugs.llvm.org/show_bug.cgi?id=14785: >>>>> ``` >>>>> template<typename T> struct Somewhat { >>>>> void internal() const {} >>>>> friend void operator+(int const &, Somewhat<T> const &) {} >>>>> }; >>>>> void operator+(int const &, Somewhat<char> const &x) { x.internal(); } >>>>> ``` >>>>> When statement `x.internal()` in the body of global `operator+` is >>>>> parsed, the type >>>>> of `x` must be completed, so the instantiation of `Somewhat<char>` is >>>>> started. It >>>>> instantiates the declaration of `operator+` defined inline, and makes >>>>> a check for >>>>> redefinition. The check does not detect another definition because the >>>>> declaration >>>>> of `operator+` is still not defining as does not have a body yet. >>>>> >>>>> To solves this problem the function `isThisDeclarationADefinition` >>>>> considers >>>>> a function declaration as a definition if it has flag `WillHaveBody` >>>>> set. >>>>> >>>>> This change fixes PR14785. >>>>> >>>>> Differential Revision: https://reviews.llvm.org/D30375 >>>>> >>>>> This is a recommit of 305379, reverted in 305381, with small changes. >>>>> >>>>> Modified: >>>>> cfe/trunk/include/clang/AST/Decl.h >>>>> cfe/trunk/lib/Sema/SemaCUDA.cpp >>>>> cfe/trunk/lib/Sema/SemaDecl.cpp >>>>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp >>>>> cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp >>>>> cfe/trunk/test/SemaCXX/friend2.cpp >>>>> >>>>> Modified: cfe/trunk/include/clang/AST/Decl.h >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ >>>>> AST/Decl.h?rev=305903&r1=305902&r2=305903&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/include/clang/AST/Decl.h (original) >>>>> +++ cfe/trunk/include/clang/AST/Decl.h Wed Jun 21 07:46:57 2017 >>>>> @@ -1874,7 +1874,7 @@ public: >>>>> /// >>>>> bool isThisDeclarationADefinition() const { >>>>> return IsDeleted || IsDefaulted || Body || IsLateTemplateParsed || >>>>> - hasDefiningAttr(); >>>>> + WillHaveBody || hasDefiningAttr(); >>>>> } >>>>> >>>>> /// doesThisDeclarationHaveABody - Returns whether this specific >>>>> >>>>> Modified: cfe/trunk/lib/Sema/SemaCUDA.cpp >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaC >>>>> UDA.cpp?rev=305903&r1=305902&r2=305903&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/lib/Sema/SemaCUDA.cpp (original) >>>>> +++ cfe/trunk/lib/Sema/SemaCUDA.cpp Wed Jun 21 07:46:57 2017 >>>>> @@ -629,12 +629,6 @@ static bool IsKnownEmitted(Sema &S, Func >>>>> // emitted, because (say) the definition could include "inline". >>>>> FunctionDecl *Def = FD->getDefinition(); >>>>> >>>>> - // We may currently be parsing the body of FD, in which case >>>>> - // FD->getDefinition() will be null, but we still want to treat FD >>>>> as though >>>>> - // it's a definition. >>>>> - if (!Def && FD->willHaveBody()) >>>>> - Def = FD; >>>>> - >>>>> if (Def && >>>>> !isDiscardableGVALinkage(S.getASTContext().GetGVALinkageFor >>>>> Function(Def))) >>>>> return true; >>>>> >>>>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaD >>>>> ecl.cpp?rev=305903&r1=305902&r2=305903&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) >>>>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Jun 21 07:46:57 2017 >>>>> @@ -12232,6 +12232,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl >>>>> >>>>> if (FD) { >>>>> FD->setBody(Body); >>>>> + FD->setWillHaveBody(false); >>>>> >>>>> if (getLangOpts().CPlusPlus14) { >>>>> if (!FD->isInvalidDecl() && Body && !FD->isDependentContext() && >>>>> >>>>> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaD >>>>> eclCXX.cpp?rev=305903&r1=305902&r2=305903&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) >>>>> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Jun 21 07:46:57 2017 >>>>> @@ -13878,6 +13878,9 @@ void Sema::SetDeclDeleted(Decl *Dcl, Sou >>>>> return; >>>>> } >>>>> >>>>> + // Deleted function does not have a body. >>>>> + Fn->setWillHaveBody(false); >>>>> + >>>>> if (const FunctionDecl *Prev = Fn->getPreviousDecl()) { >>>>> // Don't consider the implicit declaration we generate for >>>>> explicit >>>>> // specializations. FIXME: Do not generate these implicit >>>>> declarations. >>>>> >>>>> Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaT >>>>> emplateInstantiateDecl.cpp?rev=305903&r1=305902&r2=305903&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original) >>>>> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Jun 21 >>>>> 07:46:57 2017 >>>>> @@ -1782,6 +1782,9 @@ Decl *TemplateDeclInstantiator::VisitFun >>>>> Previous.clear(); >>>>> } >>>>> >>>>> + if (isFriend) >>>>> + Function->setObjectOfFriendDecl(); >>>>> + >>>>> SemaRef.CheckFunctionDeclaration(/*Scope*/ nullptr, Function, >>>>> Previous, >>>>> isExplicitSpecialization); >>>>> >>>>> >>>>> Modified: cfe/trunk/test/SemaCXX/friend2.cpp >>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/f >>>>> riend2.cpp?rev=305903&r1=305902&r2=305903&view=diff >>>>> ============================================================ >>>>> ================== >>>>> --- cfe/trunk/test/SemaCXX/friend2.cpp (original) >>>>> +++ cfe/trunk/test/SemaCXX/friend2.cpp Wed Jun 21 07:46:57 2017 >>>>> @@ -170,3 +170,40 @@ struct Test { >>>>> template class Test<int>; >>>>> >>>>> } >>>>> + >>>>> +namespace pr14785 { >>>>> +template<typename T> >>>>> +struct Somewhat { >>>>> + void internal() const { } >>>>> + friend void operator+(int const &, Somewhat<T> const &) {} // >>>>> expected-error{{redefinition of 'operator+'}} >>>>> +}; >>>>> + >>>>> +void operator+(int const &, Somewhat<char> const &x) { // >>>>> expected-note {{previous definition is here}} >>>>> + x.internal(); // expected-note{{in instantiation of template class >>>>> 'pr14785::Somewhat<char>' requested here}} >>>>> +} >>>>> +} >>>>> + >>>>> +namespace D30375 { >>>>> +template <typename K> struct B { >>>>> + template <typename A> bool insert(A &); >>>>> +}; >>>>> + >>>>> +template <typename K> >>>>> +template <typename A> bool B<K>::insert(A &x) { return x < x; } >>>>> + >>>>> +template <typename K> class D { >>>>> + B<K> t; >>>>> + >>>>> +public: >>>>> + K x; >>>>> + bool insert() { return t.insert(x); } >>>>> + template <typename K1> friend bool operator<(const D<K1> &, const >>>>> D<K1> &); >>>>> +}; >>>>> + >>>>> +template <typename K> bool operator<(const D<K> &, const D<K> &); >>>>> + >>>>> +void func() { >>>>> + D<D<int>> cache; >>>>> + cache.insert(); >>>>> +} >>>>> +} >>>>> >>>>> >>>>> _______________________________________________ >>>>> 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