[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

Thu Sep 23 15:29:04 PDT 2021

mizvekov added a comment.

In D110216#3019070 <https://reviews.llvm.org/D110216#3019070>, @sammccall wrote:

> A few comments on the behavior & clangd changes. I can't say I understand enough about how deduction is implemented to give a proper review of the guts :-(

Thanks!

> In general eliminating type-parameter-0-0 is great and retaining sugar gives us more options for display.
> Thanks for updating clangd, I have some quibbles but feel free to land this in ~any form where the tests pass, there are no critical regressions and I'm happy to polish the behavior afterwards.

I think this will take a while to get merged, both because it's a big change in an obscure area, and also because there are some regressions in the ASTImporter support which I am not sure what their impact are, and if so the underlying problems there would need to be fixed first.

> If I understand, a deduced decltype(auto) now desugars as `decltype(auto) -> decltype(foo) -> Foo`.
> This seems principled, but at least clangd we usually call getDeducedType() to display the underlying type somehow, and `decltype(foo)` isn't terribly enlightening. (The asymmetry between `auto` and `decltype(auto)` is a bit unfortunate).

I am a bit thorn on that issue as well. Ideally we may want to keep that kind of information because it might be useful for some kind analysis. On the other hand it is mostly a distraction for HMI purposes.

You raise a valid point that it is a bit silly to have this for decltype(auto) but not C++14 auto and C++11 auto, but I think the reason we have it in the first place was just an engineering / economical purpose: We had the code that built it from an expression as a monolith, and it was easier to just use it, than to expose the machinery that just calculates the underlying type. And when storing it on the AutoType, that sugar would be lost anyway.

Maybe It would be useful to create such nodes for the other autos as well, but then we would need a spelling for it, unlike decltype which already is in the vocabulary :)

> For clangd, unwrapping it in the `clangd::getDeducedType()` helper seems fine, so I only really mention this because I get the same feeling about some of the clang diagnostic messages.
> It might be that the DeducedAsType for `decltype(auto)` should skip the DeclType (when possible, i.e. unless dependent), WDYT?

I think the only case we could possibly want to not skip when presenting to the user is not only when it is just dependent, but specifically when it is DependentTy (aka some completely unknown type (aka `T::foo`)).

As things stand, whenever we se an AutoType which is a `decltype(auto)`, and which has a non-null DeducedAsType, a decltype node is guaranteed to follow.

Perhaps we can store the expression in the AutoType itself instead? Though that is a big change.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216