-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Constant Evaluation Assertion Failure with C++ Concepts #82849
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
@llvm/issue-subscribers-clang-frontend Author: Yuxuan Chen (yuxuanchen1997)
If a concept contains a lambda expression, the template-id expression
The assertion failure is
It's worth noting that with an assertion-disabled I can think of a few possible fixes:
Even though a Release build is not affected and won't block any production users (with doubt), I think this problem is still worth solving. I would like to consult upstream which approach I should take. |
@llvm/issue-subscribers-c-20 Author: Yuxuan Chen (yuxuanchen1997)
If a concept contains a lambda expression, the template-id expression
The assertion failure is
It's worth noting that with an assertion-disabled I can think of a few possible fixes:
Even though a Release build is not affected and won't block any production users (with doubt), I think this problem is still worth solving. I would like to consult upstream which approach I should take. |
I’m also wondering if we could resolve this by refining the lambda dependency calculation e.g. If the transformed lambda doesn’t involve any template parameters, then it should be of NeverDependent type. llvm-project/clang/lib/Sema/TreeTransform.h Lines 13474 to 13485 in d5c9d40
|
@zyn0217 |
The TypeDecl's dependency was determined by |
If I am reading this right, that ended up calling isDependentContext on the parent DeclContext. Which belongs to the function template that is never instantiated.
…________________________________
From: Younan Zhang ***@***.***>
Sent: Friday, February 23, 2024 6:09:11 PM
To: llvm/llvm-project ***@***.***>
Cc: Yuxuan Chen ***@***.***>; Author ***@***.***>
Subject: Re: [llvm/llvm-project] Constant Evaluation Assertion Failure with C++ Concepts (Issue #82849)
The lambda dependency ultimately came from the CXXRecordDecl's TypeDecl. In template instantiation we provided the parent DeclContext, which in the concept's case is dependent. The TypeDecl's dependency was determined by DeclContext: : isDependentContext(),
ZjQcmQRYFpfptBannerStart
This Message Is From an External Sender
ZjQcmQRYFpfptBannerEnd
The lambda dependency ultimately came from the CXXRecordDecl's TypeDecl. In template instantiation we provided the parent DeclContext, which in the concept's case is dependent.
The TypeDecl's dependency was determined by DeclContext::isDependentContext(), which does consider the lambda's dependency, right?
Moreover, I have experimented with a patch that sets the DependencyKind to always LDK_NeverDependent, and the crash was gone then.
—
Reply to this email directly, view it on GitHub<#82849 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACNH36MWWLEOPBJ3CAQP7UDYVFDUPAVCNFSM6AAAAABDXRU7I6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRSGIYTMMJZHA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
That sounds wrong. The parent context of the lambda should not be the function template -- we should presumably be switching out the current context to that of the concept before we do a satisfaction check. I'd expect this means we get access checks wrong too. And we do: https://godbolt.org/z/bcYavzTzP (interestingly, GCC and EDG have the same bug...) |
[temp.inst]p17 I wonder why |
This is not a "type-constraint [or] requires-clause of a template specialization or member function" -- it's a concept-id, which has different rules. See [temp.names]/9. |
Confirmed: https://godbolt.org/z/88PcYd94r Assertion: clang++: /root/llvm-project/clang/lib/AST/ExprConstant.cpp:14963:
bool clang::Expr::EvaluateAsConstantExpr(clang::Expr::EvalResult&, const clang::ASTContext&, clang::Expr::ConstantExprKind) const:
Assertion `!isValueDependent() && "Expression evaluator can't be called on a dependent expression."' failed. |
@zygoloid this sounds like the most appropriate fix here. However, |
…83997) The dependency of a lambda inside of a `RequiresExprBodyDecl` was previously affected by its parent, e.g., `ClassTemplateSpecializationDecl`. This made the lambda always dependent regardless of the template arguments we had, which caused some crashes on the constraint evaluation later. This fixes #56556, fixes #82849 and a case demonstrated by #49570 (comment).
If a concept contains a lambda expression, and if the arguments provided to the concept template-id expression are not dependent,
Sema
runs into an assertion failure.Consider the following program:
The assertion failure is
ExprConstant
complains that this lambda after instantiation is still value dependent.getTypeDeclType
on the instantiated lambda shows that the type for the lambdaCXXRecordDecl
is dependent. The reason being the parentDeclContext
was dependent, which is also true in this case as we aren't instantiating function templateb
here.It's worth noting that with an assertion-disabled
clang++
, this program compiles successfully and, as far as I can tell, correctly.I can think of a few possible fixes:
template-id
expression here causes the constraint to be checked if the function templateb
is not being instantiated? Can the fix just be that defer checking the constraint to performing pending instantiations? InSema::CheckConceptTemplateId
, we eagerly calculate the constraint satisfaction if the arguments to the concept template aren't dependent.DeclContext
look at the parent context to tell if it's instantiated? This causes any AST nodes created with template instantiation we do during concept template constraints checking to be considered dependent.ExprConstant
evaluation. Since in our case, no actual dependent values can be reached, which explains why the assertion-free build successfully compiles this code.Even though a Release build is not affected and won't block any production users (with doubt), I think this problem is still worth solving. I would like to consult upstream which approach I should take.
The text was updated successfully, but these errors were encountered: