[libcxx-commits] [PATCH] D91311: Add new 'preferred_name' attribute.

[Richard Smith - zygoloid via Phabricator via libcxx-commits]

rsmith marked 2 inline comments as done.
rsmith added inline comments.


================
Comment at: libcxx/include/iosfwd:188
 
+#ifdef _LIBCPP_PREFERRED_NAME
+template <class _CharT, class _Traits>
----------------
aaron.ballman wrote:
> We always define `_LIBCPP_PREFERRED_NAME` so is this actually needed?
Thanks, I was trying to avoid the redundant redeclarations when the attribute is unavailable, but clearly this doesn't do that! Fixed.


================
Comment at: libcxx/include/regex:2520
+    _LIBCPP_PREFERRED_NAME(wregex)
+    basic_regex
 {
----------------
Quuxplusone wrote:
> Why does this attribute go on the class template? Shouldn't it be an attribute on the typedef, so that you don't have to repeat yourself? I mean, I'd much rather see
> 
>     template<class T> class BasicFoo { };
>     using [[preferred]] Foo = BasicFoo<A>;
>     using [[preferred]] WFoo = BasicFoo<B>;
> 
> than
> 
>     template<class> class BasicFoo;
>     using Foo = BasicFoo<A>;
>     using WFoo = BasicFoo<B>;
>     template<class T> class [[preferred(Foo, WFoo)]] BasicFoo { };
> 
> The latter repeats every identifier one extra time, compared to the former.
> 
> And then, in fact, couldn't you go one step further and say that typedefs in the same scope as the class template itself should //always// implicitly have this attribute? Even if the attribute doesn't appear in the source code, we still want to print `basic_regex<char>` as `regex`, right? It shouldn't cost any additional work for the compiler to figure that out.
I don't think we want arbitrary typedefs to be able to declare themselves to be names for the class template after the fact: this is a decision that the (author of the) template itself should be making, not a decision for the (author of the) typedef. Also, we need the information when looking at the class template, not when looking at the typedef, so attaching it to the typedef would require us to internally attach it to the class template instead anyway, and generally it's undesirable and problematic to mutate an existing declaration -- it's much cleaner to introduce new properties of a declaration by redeclaring it. It's certainly not ideal that this results in extra forward declarations in some cases, but given that this attribute is only expected to be written a few dozen times in total, it doesn't seem worth worrying about too much.

I don't think we want typedefs to automatically get this behavior, even if they're in the same namespace. I think it would be problematic if, for example:

```
namespace absl {
  template<typename T> struct basic_string_view;
  using format_arg = basic_string_view<char>;
  std::string format(format_arg, ...);
}
```

... resulted in `absl::string_view` sometimes displaying as the (to a user) unrelated type `absl::format_arg` depending on (presumably) `#include` order, and I don't think we want to be in the business of inventing heuristics to pick the "right" typedef.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91311