bloomberg / clang-p2996

Experimental clang support for WG21 P2996 (Reflection).
https://github.com/bloomberg/clang-p2996/tree/p2996/P2996.md
51 stars 8 forks source link

[Regression] return type of name_of function #61

Closed Yaraslaut closed 2 months ago

Yaraslaut commented 2 months ago

Hi, want to bring to your attention that https://github.com/bloomberg/clang-p2996/commit/0c79694e3db6199146352d1eed760506fda5da13 broke printing of results from name_of function, now name_of return u8string_view, and this view can not be formatted, so for example godbolt fails to compile:

#include <experimental/meta>
#include <print>

int main() {
    constexpr int value{1};
    constexpr auto refl = ^value;
    std::println("{}", name_of(refl));
}

error: call to deleted constructor of 'formatter<u8string_view, char>'

Possible solution is to change default type

-template<typename T = u8string_view>
+template<typename T = string_view>
 consteval auto name_of(info r) -> T {

But this will break following test https://github.com/bloomberg/clang-p2996/blob/f4b2be333b8c6c27f29d7049860c14dea9c01efd/libcxx/test/std/experimental/reflection/define-class.pass.cpp#L259

katzdm commented 2 months ago

Hey @Yaraslaut ! Thanks for reaching out about this one. This isn't quite a regression, but it is the unfortunate status quo for the time being. We're working with WG21's Unicode study group (SG16) to figure out how best to support identifiers containing international text, and the current idea (as you've observed) from P2996R3 is to define name_of as a template whose parameter can be either a u8string_view or a string_view.

Choosing u8string_view as the default type is a forward-looking decision: It's quite unfortunate that for the moment neither std::cout nor std::print support u8string_view, but the thinking is that this will be fixed, and that choosing u8string_view as the default will age better over time. Note that there is already a P3258R0 paper that hopes to address the particular gap demonstrated here: It will make it possible to format a u8string_view into a string, after which std::println("{}", name_of(refl) would again compile.

More discussion is needed with LEWG and SG16, and there's a chance that this design could still change (alas! what else can you expect from an experimental compiler based on a still-evolving paper 😉). For now, I'd recommend writing name_of<std::string_view>(refl) when you need to print or format a name.

Yaraslaut commented 2 months ago

Thanks for the prompt and detailed response @katzdm :) hope this will be resolved in the future, also, in only for formatting, one can use something like this

template<>
struct std::formatter<std::basic_string_view<char8_t>> {
  template <class ParseContext>
  constexpr ParseContext::iterator parse(ParseContext &ctx) {return ctx.begin();}
  template <class FmtContext>
  FmtContext::iterator format(std::basic_string_view<char8_t> s, FmtContext &ctx) const {
    return format_to(ctx.out(), "{}", std::string{s.begin(),s.end()});
  }
};

not to change calls for name_of in formatting context