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

[Matheus Izvekov via Phabricator via lldb-commits]

mizvekov added a comment.

In D110216#3137375 <https://reviews.llvm.org/D110216#3137375>, @hans wrote:

>> I am not sure how to reproduce this.
>
> Attaching preprocessed source. With Clang built at 508aa4fe0c82b3b409e2e194d591ee6d665c8623 it reproduces for me like this:
>
>   $ clang++ -c /tmp/a.ii
>   ../../base/containers/span_unittest.cc:11:13: error: no viable conversion from 'span<T, [...]>' to 'span<int, [...]>'
>     span<int> s = make_span(v);
>               ^   ~~~~~~~~~~~~
>   ../../base/containers/span.h:340:13: note: candidate constructor not viable: no known conversion from 'span<T, Extent::value>' (aka 'span<const int, Extent::value>') to 'const base::span<int, 18446744073709551615> &' for 1st argument
>     constexpr span(const span& other) noexcept = default;
>               ^

Oh so that one was a curve ball!!!! `T` was not a template parameter, just a typedef, see the implementation of make_span in that preprocessed file:

  template <int&... ExplicitArgumentBarrier, typename Container>
  constexpr auto make_span(Container&& container) noexcept {
    using T =
        std::remove_pointer_t<decltype(base::data(std::declval<Container>()))>;
    using Extent = internal::Extent<Container>;
    return span<T, Extent::value>(std::forward<Container>(container));
  }

So, some notes:

- We should probably also do an AKA on the type diff.
- Chromium should use a clearer name for that typedef perhaps? Though calling the first template parameter of span as `T` is common, maybe a better name here would be `element_type`.

Having a convenient way to just name a type like that also seems useful, so maybe the rename coupled with the AKA would be an improvement overall?

Otherwise, if we get too much trouble with with legacy codebases which cannot be updated with that simple rename, we could perhaps make the type printer be context sensitive, and when printing type nodes tied to NamedDecls, just step over those that refer to a name which is not accessible in the current context?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216