[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions
Alexander Shaposhnikov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 16 13:27:01 PDT 2023
alexander-shaposhnikov added a comment.
@shafik - it's a bit early to review this patch, this was the first attempt to fix the issue related to the current behavior of clang::Sema::TemplateParameterListsAreEqual that causes Clang to mishandle out-of-line definitions involving constraints (and possibly is the root cause of a few other issues related to concepts).
It's still work-in-progress. What's happening is roughly the following: there is a mismatch of depths of types (TemplateTypeParmType) referenced by ConceptSpecializationExpr.
E.g. for the code
template <class T>
concept Concept = true;
template<bool>
struct a
{
template<Concept T1>
void c(T1&& t);
};
template<>
template<Concept T2>
void a<true>::c(T2&& t)
{}
lldb) p OldConstr->dump()
ConceptSpecializationExpr 0x555564bc4c50 '_Bool' Concept 0x555564bc4608 'Concept'
|-ImplicitConceptSpecializationDecl 0x555564bc4be0
| `-TemplateArgument type 'type-parameter-1-0'
| `-TemplateTypeParmType 0x555564bc4b70 'type-parameter-1-0' dependent depth 1 index 0
`-TemplateArgument type 'T1'
`-TemplateTypeParmType 0x555564bc4ba0 'T1' dependent depth 1 index 0
`-TemplateTypeParm 0x555564bc4ac8 'T1'
(lldb) p NewConstr->dump()
ConceptSpecializationExpr 0x555564bc5140 '_Bool' Concept 0x555564bc4608 'Concept'
|-ImplicitConceptSpecializationDecl 0x555564bc50d0
| `-TemplateArgument type 'type-parameter-0-0'
| `-TemplateTypeParmType 0x555564bc4580 'type-parameter-0-0' dependent depth 0 index 0
`-TemplateArgument type 'T2'
`-TemplateTypeParmType 0x555564bc5090 'T2' dependent depth 0 index 0
`-TemplateTypeParm 0x555564bc4ff0 'T2'
Inside Sema::AreConstraintExpressionsEqual there is some logic do adjust the depths, but at the moment i can't claim that i fully understand it - still investigating.
I think @erichkeane has also looked into the issue / debugged what's going on here - if I'm missing something / or something is not right - any corrections would be appreciated.
My current plan is to try to adopt the suggestion from
In D146178#4199382 <https://reviews.llvm.org/D146178#4199382>, @erichkeane wrote:
> In D146178#4199263 <https://reviews.llvm.org/D146178#4199263>, @erichkeane wrote:
>
>> After a night of sleeping on it, the use of a AdjustConstraintDepth::Diff and AdjustConstraintDepth::Value feels like a hacky solution to SOMETHING here, though I'm not sure what yet. The depth of a template shouldn't change between the equality and constraint checking, it is usually a property of the specialization level. I could definitely believe that `getTemplateInstantiationArgs` needs some sort of change here to better detect its own state, but this solution doesn't seem right to me.
>
> I debugged a bit: It isn't correct I think that `FunctionTemplateDecl` doesn't pick up its template arguments in `getTemplateInstantiationArgs`. Additionally, instead of picking up its `DeclContext`, it probably needs its `LexicalDeclContext` as the next step, which I believe fixes the problem (plus your change in SemaOverload.cpp)
>
> EDIT: `FunctionTemplateDecl` doesn't pick up its template arguments on purpose: there ARE no template arguments, so that is me being silly. However, the difference here is that it isn't picking its lexical decl context here. There is likely a similar solution here for VarTemplateDecl.
>
> I'm leaning toward the solution here being in the CalculateTemplateDepth here instead: Recognize those two types, and set the correct DeclContext to pass to the getTemplateInstantiationArgs (that is, the lexical decl context).
and see if i can make it functional, this will take some time.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146178/new/
https://reviews.llvm.org/D146178
More information about the llvm-commits
mailing list