erichkeane accepted this revision.
erichkeane added inline comments.
================
Comment at: clang/lib/AST/ASTContext.cpp:5119
// Find the insert position again.
- DependentTemplateSpecializationTypes.FindNodeOrInsertPos(ID, InsertPos);
+ [[maybe_unused]] auto *Nothing =
+ DependentTemplateSpecializationTypes.FindNodeOrInsertPos(ID,
InsertPos);
----------------
mizvekov wrote:
> erichkeane wrote:
> > Instead of `maybe_unused` put this and the assert inside of a `#if NDEBUG`
> > so we don't do the `Nothing` work. Same below.
> We still need to do the `FindNodeOrInsertPos` bit, to refresh the insert
> position.
> So I think this would end up too noisy to add two sets of blocks.
Ah! I didn't realize we were counting on the side-effects here. Thanks!
================
Comment at: clang/lib/Sema/SemaTemplate.cpp:5817
// fall back to just producing individual arguments.
- Converted.insert(Converted.end(),
- ArgumentPack.begin(), ArgumentPack.end());
+ for (const TemplateArgument &I : ArgumentPack)
+ Converted.push_back(Context.getCanonicalTemplateArgument(I));
----------------
mizvekov wrote:
> erichkeane wrote:
> > Ooh, thats a transform!
> >
> > `llvm::transform(ArgumentPack, std::back_inserter(Converted), [](const auto
> > &I) { return Context.getCanonicalTemplateArgument(I));`
> That is true, though unless we can come up with a clearer spelling for that,
> it looks less readable to me.
But I like transform :(
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133072/new/
https://reviews.llvm.org/D133072
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
