mizvekov added inline comments.
================
Comment at: clang/include/clang/Sema/SemaConcept.h:54-68
+ if (ArgA.getKind() == TemplateArgument::Type &&
+ ArgB.getKind() == TemplateArgument::Type)
+ if (const auto *SubstA =
+ ArgA.getAsType()->getAs<SubstTemplateTypeParmType>())
+ if (const auto *SubstB =
+ ArgB.getAsType()->getAs<SubstTemplateTypeParmType>()) {
+ QualType ReplacementA = SubstA->getReplacementType();
----------------
mizvekov wrote:
> ychen wrote:
> > mizvekov wrote:
> > > It's a bit odd to find `SubstTemplateTypeParmType` necessary to implement
> > > the semantics of this change.
> > >
> > > This is just type sugar we leave behind in the template instantiator to
> > > mark where type replacement happened.
> > >
> > > There are several potential issues here:
> > > 1) This could be marking a substitution that happened at any template
> > > depth. Ie this could be marking a substitution from an outer template.
> > > Does the level not matter here at all?
> > > 2) If the level does matter, you won't be able to reconstitute that
> > > easily without further improvements. See
> > > https://github.com/llvm/llvm-project/issues/55886 for example.
> > > 3) A substitution can replace a dependent type for another one, and when
> > > that other gets replaced, we lose track of that because
> > > `SubstTemplateTypeParmType` only holds a canonical underlying type.
> > >
> > > ----
> > >
> > > Leaving that aside, I get the impression you are trying to work around
> > > the fact that when an expression appears in a canonical type, presumably
> > > because that expression is dependent on an NTTP, we can't rely on
> > > uniquing anymore to compare if they are the same type, as we lack in
> > > Clang the equivalent concept of canonicalization for expressions.
> > >
> > > But this however is a bit hard to implement. Are we sure the standard
> > > requires this, or can we simply consider these types always different?
> > > It's a bit odd to find SubstTemplateTypeParmType necessary to implement
> > > the semantics of this change.
> >
> > Odd indeed.
> >
> > > Leaving that aside, I get the impression you are trying to work around
> > > the fact that when an expression appears in a canonical type, presumably
> > > because that expression is dependent on an NTTP, we can't rely on
> > > uniquing anymore to compare if they are the same type, as we lack in
> > > Clang the equivalent concept of canonicalization for expressions.
> >
> > Yeah, sort of . This workaround is to deal with the fact that
> > `DecltypeType` is not uniqued. However, the injected template argument for
> > `t` of `template<auto t>` is `decltype(t)` (which on a side note, might be
> > wrong since `auto` means using template arg deduct rules; `decltype(auto)`
> > means using `decltype(expr)` type, let's keep it this way now since this
> > code path is still needed when Clang starts to support `decltype(auto)` as
> > NTTP type) and concepts partial ordering rules need to compare these
> > concept template arguments (https://eel.is/c++draft/temp.constr#atomic-1).
> >
> > Looking at the motivation why `DecltypeType` is not uniqued
> > (https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/ASTContext.cpp#L5637-L5640),
> > I think maybe we should unique decltype on the expr to deal with concepts
> > cleanly. Thoughts?
> I see, there should be no problem with a change to unique a DecltypeType, but
> it might not help you, because expressions typically have source location
> information embedded in them.
By the way, I just became aware of something I missed, and this could totally
work here.
We don't have 'canonical expressions', but we do have profiling based on the
canonical form of an expression, eg see the `Expr::Profile` method, it has a
`Canonical` argument to ask for this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128750/new/
https://reviews.llvm.org/D128750
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
