mizvekov marked an inline comment as done.
mizvekov added inline comments.
================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:559
+ // Get the resolved template arguments from the canonical type.
+ // FIXME: Handle the as-written arguments so the sugar is not lost.
+ ArrayRef<TemplateArgument> PResolved =
----------------
rsmith wrote:
> Does this matter on the `P` side? (Can we remove this FIXME?)
Even though P sugar is not relevant to the deduced type, it is relevant to
diagnostics when there is a deduction failure. This has been relevant in the
other parts of this patch, so I don't see why we would not at least want to
preserve it here, though it might be too much trouble to be worth the effort.
I will keep the comment for now, if we have any discussion where it becomes
clear there would be no difference here, I will make a one line NFC and remove
it.
================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:561
+ ArrayRef<TemplateArgument> PResolved =
+ TP->getCanonicalTypeInternal()
+ ->castAs<TemplateSpecializationType>()
----------------
rsmith wrote:
> Can we avoid using the `Internal` version here? (The only difference is
> whether we preserve top-level qualifiers that are outside any type sugar.)
>
> Might also be worth a comment here explaining that we're going to the
> canonical type first here to step over any alias templates.
The Internal version is more convenient because TP is a `Type *` not a
`QualType`, so the suggestion as you have written would not work.
Removing the alias template would be handled by `getUnqualifiedDesugaredType`,
above, which my understanding is that will remove all top level sugar.
But I see that I went all this way around to preserving the non-canonical TP
but all I needed really from it was the templateName, which probably is always
the same as the canonical one, though not sure if that is by design.
I just made a change to get the canonical type on TP above instead, and we can
circle back to this later.
================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:571-573
+ // FIXME: Should not lose sugar here.
+ if (const auto *SA = dyn_cast<TemplateSpecializationType>(
+ UA->getCanonicalTypeInternal())) {
----------------
rsmith wrote:
> I think we can retain type sugar here without too much effort.
That does not work, blows up some 80 tests.
I will leave the FIXME and circle back to this after I am done with my pile of
patches.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110216/new/
https://reviews.llvm.org/D110216
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
