ChuanqiXu added a comment. In D126907#3588708 <https://reviews.llvm.org/D126907#3588708>, @erichkeane wrote:
> In D126907#3588417 <https://reviews.llvm.org/D126907#3588417>, @ChuanqiXu > wrote: > >> From what I can see, the crash reason would be the mismatch depth and the >> setting of MultiLevelTemplateArgumentList. In >> `::CheckConstraintSatisfaction`, the depth of `RawCompletionToken ` is 0, >> while the depth of corresponding MultiLevelTemplateArgumentList is 2. So the >> compiler would get the outermost template argument incorrectly (which is a >> template pack in the example) in TransformTemplateTypeParmType. >> >> The first though was that we should set the depth of `RawCompletionToken ` >> to 1 correctly. But I felt this might not be good later since in the normal >> process of template instantiation (without concepts and constraints), the >> depth of `RawCompletionToken ` is 0 indeed. What is different that time is >> the depth of corresponding `MultiLevelTemplateArgumentList ` is 1. So it >> looks like the process is constructed on the fly. It makes sense for the >> perspective of compilation speed. >> >> So I feel like what we should do here is in >> `Sema::CheckInstantiatedFunctionTemplateConstraints`, when we are computing >> the desired MultiLevelTemplateArgumentList, we should compute a result with >> depth of 1 in the particular case. >> >> --- >> >> Another idea is to not do instantiation when we're checking constraints. But >> I need to think more about this. > > I don't know of the 'another idea'? We have to do instantiation before > checking, and if we do it too early, we aren't doing the deferred > instantiation. And the problem is that the RawCompletionToken uses a concept > to refer to 'itself'. However, this version of 'itself' isn't valid, since > the 'depth' is wrong once it tries to instantiate relative to the 'top > level'. However, this IS happening during instantiation? > > My latest thought is that perhaps the "IsEvaluatingAConstraint" should NOT > happen at the Sema level (but at the instantiator level), since it ends up > catching things it shouldn't? I tried it and have a handful of failures of > trying to check uninstantiated constraints, but I've not dug completely into > it. Yeah, we have to do instantiation before checking. My point is that it looks like we're doing **another** instantiation when we check the concepts. And my point is that if it we should avoid the second instantiation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/ https://reviews.llvm.org/D126907 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
