r305903 - Function with unparsed body is a definition
Serge Pavlov via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 2 11:05:13 PDT 2017
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 <sepavloff at gmail.com>:
> Yes, sure, I will investigate it.
>
> Thanks,
> --Serge
>
> 2017-08-01 21:32 GMT+07:00 Alexander Kornienko <alexfh at 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 at 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 at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170803/9edc0585/attachment-0001.html>
More information about the cfe-commits
mailing list