aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!



================
Comment at: clang/lib/Sema/SemaType.cpp:261
+    /// necessary.
+    QualType ReplaceAutoType(QualType TypeWithAuto, QualType Replacement) {
+      QualType T = sema.ReplaceAutoType(TypeWithAuto, Replacement);
----------------
vsapsai wrote:
> aaron.ballman wrote:
> > I think this work should be done within `SubstituteDeducedTypeTransform` 
> > rather than on the side. Any caller to `Sema::ReplaceAutoType()` should get 
> > this same behavior.
> Doing this work in `SubstituteDeducedTypeTransform` involves teaching 
> `SubstituteDeducedTypeTransform` about `TypeProcessingState` and probably 
> adding `TypeProcessingState` to `Sema::ReplaceAutoType()`. As for me, it 
> exposes `TypeProcessingState` in more places than necessary. And it feels 
> somewhat awkward that `SubstituteDeducedTypeTransform` is used in multiple 
> places but `TypeProcessingState` is required only here.
> 
> I've modelled my approach after `TypeProcessingState::getAttributedType` 
> where it forwards the call to Sema and keeps `AttrsForTypes` up to date. Do 
> you still think `SubstituteDeducedTypeTransform` would be a better place for 
> this code?
Hmm, yeah, it doesn't seem like it would make sense there. This is keeping the 
`TypeProcessingState` up to date which is really only of interest during some 
stages of processing.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58659/new/

https://reviews.llvm.org/D58659



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to