erichkeane added inline comments.
================ Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:133 } +Response HandlePartialClassTemplateSpec( ---------------- alexander-shaposhnikov wrote: > alexander-shaposhnikov wrote: > > alexander-shaposhnikov wrote: > > > alexander-shaposhnikov wrote: > > > > HandlePartialClassTemplateSpec is from Erich's diff > > > > (https://reviews.llvm.org/D147722) > > > @erichkeane : previously (see https://reviews.llvm.org/D147722) we were > > > producing a real layer of arguments (if the depth is greater than 1), to > > > the best of my knowledge this is incorrect since it'd trigger unexpected > > > depth adjustment, we should produce retained layers instead. > > (depth adjustment during the substitution) > to make it a bit easier to reason about I've changed the names in the test > case: > > ``` > //enum class Enum { E1 }; > > template <typename T1> > concept some_concept = true; > //inline constexpr bool some_concept = true; > > template <typename T1, int> > struct S { > template <typename T2> > requires(some_concept<T2>) > void func(const T2 &); > }; > > template <typename T11> > struct S<T11, 0> { > template <typename T22> > requires(some_concept<T22>) > void func(const T22 &); > }; > > template <typename T111> > template <typename T222> > requires (some_concept<T222>) > inline void S<T111, 0>::func(const T222 &) {} > ``` > > Previously the code was doing the following: > ``` > PartialClassTemplSpec->getTemplateDepth() = 1 > NumRetainedOuterLevels: 0 > 0: <> > before subst: > ParenExpr 0x558fb90be6a8 '_Bool' > `-ConceptSpecializationExpr 0x558fb90be640 '_Bool' Concept 0x558fb909fbd8 > 'some_concept' > |-ImplicitConceptSpecializationDecl 0x558fb90be5d0 > | `-TemplateArgument type 'type-parameter-1-0' > | `-TemplateTypeParmType 0x558fb90a01d0 'type-parameter-1-0' dependent > depth 1 index 0 > `-TemplateArgument type 'T22' > `-TemplateTypeParmType 0x558fb90be590 'T22' dependent depth 1 index 0 > `-TemplateTypeParm 0x558fb90be540 'T22' > after subst: > ParenExpr 0x558fb90bf168 '_Bool' > `-ConceptSpecializationExpr 0x558fb90bf100 '_Bool' Concept 0x558fb909fbd8 > 'some_concept' > |-ImplicitConceptSpecializationDecl 0x558fb90bf090 > | `-TemplateArgument type 'type-parameter-0-0' > | `-TemplateTypeParmType 0x558fb909fb50 'type-parameter-0-0' dependent > depth 0 index 0 > `-TemplateArgument type 'T22' > `-TemplateTypeParmType 0x558fb90bf050 'T22' dependent depth 0 index 0 > `-TemplateTypeParm 0x558fb90be540 'T22' > ``` > (as one can see the depths have changed) > > now: > ``` > PartialClassTemplSpec->getTemplateDepth() = 1 > NumRetainedOuterLevels: 1 > before subst: > ParenExpr 0x55f7813ae6a8 '_Bool' > `-ConceptSpecializationExpr 0x55f7813ae640 '_Bool' Concept 0x55f78138fbd8 > 'some_concept' > |-ImplicitConceptSpecializationDecl 0x55f7813ae5d0 > | `-TemplateArgument type 'type-parameter-1-0' > | `-TemplateTypeParmType 0x55f7813901d0 'type-parameter-1-0' dependent > depth 1 index 0 > `-TemplateArgument type 'T22' > `-TemplateTypeParmType 0x55f7813ae590 'T22' dependent depth 1 index 0 > `-TemplateTypeParm 0x55f7813ae540 'T22' > after subst: > ParenExpr 0x55f7813af138 '_Bool' > `-ConceptSpecializationExpr 0x55f7813af0d0 '_Bool' Concept 0x55f78138fbd8 > 'some_concept' > |-ImplicitConceptSpecializationDecl 0x55f7813af060 > | `-TemplateArgument type 'type-parameter-1-0' > | `-TemplateTypeParmType 0x55f7813901d0 'type-parameter-1-0' dependent > depth 1 index 0 > `-TemplateArgument type 'T22' > `-TemplateTypeParmType 0x55f7813ae590 'T22' dependent depth 1 index 0 > `-TemplateTypeParm 0x55f7813ae540 'T22' > ``` > > So the thought behind the 'empty' level was for a case where the `PartialClassTemplSpec` is defined inside of another type that actually DOES have arguments we need to compare. Though, I guess for the purposes of substitution, it isn't really needed (since if we have a `PartialClassTemplSpec`, we can't fully instantiate ANYWAY)... and I can't think of a case where it totally matters anyway. I think I'm OK with this as is, but I'm a touch suspicious. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
