In CodeGen::CodeGenFunction::EmitCallArgs: Assertion `[...] && "type mismatch in call argument!"' failed #44345
Comments
|
assigned to @aaronpuchert |
|
Had a brief look in the debugger. So there is a type mismatch in the AST, which makes this a frontend issue. |
|
I was able to simplify the reproducer: struct a {};
template <typename l>
int k{[](a = {}) -> int {}()};
template int k<int>;The type of the InitListExpr is 'a' for the VarTemplateDecl, but 'void' for the VarTemplateSpecializationDecl with argument 'int'. So this looks like a bug in the template instantiation. |
Thanks for looking into it! As I'm quite unfamiliar with those areas of Clang, would you have time to look further into it, or who would be most familiar with that particular area? |
Most familiar is probably Richard Smith (already on CC). I have a healthy respect for the template instantiation machinery. But I also have a bug of my own in that area, so who knows, maybe I can carve out the time. |
|
I think I've narrowed down of the critical bits to this, in Sema::SubstParmVarDecl llvm-project/clang/lib/Sema/SemaTemplateInstantiate.cpp Lines 2340 to 2359 in b4c3a76 } else if (Expr *Arg = OldParm->getDefaultArg()) { If I force this to take the isLexicallyWithinFunctionOrMethod() case, the type of the InitListExpr remains correct and there's no inconsistency in the AST. |
What is strange is that default arguments are otherwise instantiated when building the CallExpr via Sema::CheckCXXDefaultArgExpr, but Param->hasUninstantiatedDefaultArg() is false in this case. In any event, I think we need to find out whether DR1484 applies to this case, i.e. do we need to instantiate the default argument even if it isn't needed? The way Clang sees it, this can't be a local class because a VarDecl isn't a DeclContext. |
|
Turns out that lambda default arguments are instantiated anyway in TreeTransform::TransformLambdaExpr, specifically in this loop: for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
I != NumParams; ++I) {
auto *P = NewCallOperator->getParamDecl(I);
if (P->hasUninstantiatedDefaultArg()) {
EnterExpressionEvaluationContext Eval(
getSema(),
Sema::ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed, P);
ExprResult R = getDerived().TransformExpr(
E->getCallOperator()->getParamDecl(I)->getDefaultArg());
P->setDefaultArg(R.get());
}
}This is also where the void type is coming from, because from TransformInitListExpr eventually calls Sema::BuildInitList, which does E->setType(Context.VoidTy); // FIXME: just a place holder for now.before returning E. That's understandable, we don't know the type here. In general, there has to be a proper initialization. We have to replace P->setDefaultArg(R.get());by getSema().SetParamDefaultArgument(P, R.get(), R.get()->getBeginLoc());So I proposed https://reviews.llvm.org/D76038. Still unsure how to test this, but given that we were missing an initialization there is a lot we can choose from, this also works: template <typename T>
int k = [](T x = 0.0) -> int { return x; }();
template int k<int>;Currently we produce: Instead of the correct |
|
Awesome, thanks for taking the time to fix this! |
|
Change has landed in master. Maybe you could go and try if this fixes your original issue? Not sure if this should be ported back to the 10.0.x codeline, do you have an opinion on that? |
It does indeed seem to fix my original issue as well - thanks for taking the time for fixing it! Regarding backporting to 10.0.x, I think that'd be a great idea as it fixes a rare but valid cornercase - but I guess that's mostly up to Richard about whether he thinks there's too much risk of regression associated with it. |
My opinion is that it might not be wrong to wait a bit and let the early adopters (= users of Clang master) test this is a bit. But you're right, Richard can probably best assess what the risks are. @Tom: What do you think about adding this to the 10.0.1 blockers but not act on it for now, deciding later whether we want this? |
That's fine. |
I think that's reasonable. The patch seems fine to merge to me if no-one complains for a week or so. |
Now one week later - I don't think there's been any complaints (at least I don't see any in the phab review thread) - so hopefully we can dare to backport it? |
|
Merged: 9c80516 |
|
mentioned in issue #44654 |
|
mentioned in issue #49178 |
Extended Description
Building current Qt dev branch for mingw targets triggers failed asserts in Clang. This is not a regression, but the same code seems to trigger failed asserts similarly in the last few releases at least. The reduced testcase triggers errors for any target, not only for mingw.
Not sure how it behaves in builds with asserts disabled (working fine i.e. assert incorrect for this case, or triggering undefined behaviours) though.
To reproduce:
The text was updated successfully, but these errors were encountered: