On Mon, Feb 12, 2024 at 7:04 AM Patrice Roy via SG14 <sg14_at_[hidden]>
wrote:

>
> * P3132 Accept attributes with user-defined prefixes
> * P3134 Attribute [[asserts_rvo]]
>
> P3132 aims to make it easier to do tooling through user-defined attributes
> and has a number of additional upsides.
> P3134 aims to provide guarantees that a function's implementation can make
> it possible for appropriately written calls to that function to lead to the
> return value optimization
>
> [...] If you would be so kind as to (a) take a look at them, and ideally
> (b) provide suggestions / constructive criticism on this list or at the
> SG14 meeting this week, I would integrate this feedback into the official
> papers. In particular, I get the impression that the audience should be
> LEWG but I'd be open to knowing what the groups thinks.
>

Both papers propose core-language features with no library impact, so they
should be EWG (in fact, EWGI).

Here's my feedback on P3132: *I think it's mistaken and DOA.*

Seems to me that P3132's motivation is simply based on a misunderstanding
of fact. Compilers are *already* required to accept (i.e. parse
successfully) attributes with custom prefixes, according to the
already-standardized attribute grammar.
    struct [[myPrefix::foobar]] Widget {};
    [[myPrefix::unused]] Widget w;
is already a well-formed C++ program that must be accepted by every
compiler, and indeed it is accepted (Godbolt
<https://godbolt.org/z/nfc5Ea1h9>).
The reason the right side of the Tony Table at the top of page 4 isn't
currently accepted is that it doesn't follow the grammar. You write:

class PatrolPath {
  [[myCompany::RegisterClass]] Waypoints m_waypoints;
  [[myCompany::Property(INIT() RUNTIMEEDITABLE)]]
};

This is ungrammatical (Godbolt <https://godbolt.org/z/hK5aezrxG>) because
an attribute always has to be attached to *something* — it can't just be
free-floating in the middle of the file.
The compiler will treat the [[myCompany::RegisterClass]] attribute as
applying to the declaration it's part of (i.e. `m_waypoints`).
The compiler will reject [[myCompany::Property]] because it's not part of
*any* declaration. All vendors except EDG will grudgingly accept it
<https://godbolt.org/z/bTanEvj67> (contra the paper standard) if you add a
trailing semicolon, so that the attribute appears to pertain to an "empty
statement" in the middle of the class. (Classes aren't, technically,
allowed to contain free-floating statements, either.)

>From the attributes' names, I'm sure the programmer meant them to apply to
the class itself, not to the members. The way you'd do that in C++ is (
Godbolt <https://godbolt.org/z/TrjGenaq4>):

class [[myCompany::RegisterClass]] [[myCompany::Property(INIT()
RUNTIMEEDITABLE)]] PatrolPath {
   Waypoints m_waypoints;
};

Examples of attributes that work this way (on a class
declaration/definition) include [[nodiscard]], [[clang::trivial_abi]], and
my proposed [[trivially_relocatable]].

The Tony Table at the bottom of page 4 already works correctly out of the
box (Godbolt <https://godbolt.org/z/q14xGn9cd>):
    [[gnu::flatten]] [[msvc::flatten]] void f();
is accepted by all vendors. Of course a good compiler *will* give you a
warning, because logically one of these two situations must apply:
- The compiler doesn't recognize the attribute you typed, which means you
might have made a typo. E.g. maybe you wrote [[gnu::flatten]] when you
meant [[gnu::flat]].
- The compiler does recognize the attribute you typed (or a prefix of it),
but doesn't support it, which means you're asking for something the
compiler knows it literally can't provide.
Vendors rightly shy away from simply swallowing those errors; they want to
helpfully surface them to the average user.

If you're a power user who knows exactly what you're doing, then you can
turn off that warning in any of several ways — the big two being:
- Put -Wattributes in your build flags (this is the big "I know what I'm
doing" hammer: not recommended)
- Use a macro like `#define FLATTEN [[...]]` to eliminate the attribute on
platforms that would otherwise warn about a lack of support

Finally, P3132 links to two StackOverflow questions specifically about
libclang:
- How to access parsed C++11 attributes via clang tooling
<https://stackoverflow.com/questions/20180917/how-to-access-parsed-c11-attributes-via-clang-tooling>
(Nov 2013)
- How to use custom C++ attributes with Clang libTooling without modifying
Clang code?
<https://stackoverflow.com/questions/70610120/how-to-use-custom-c-attributes-with-clang-libtooling-without-modifying-clang-c>
(Jan
2022)
The gist of these questions/answers, IIUC, is that libclang will *parse*
unknown attributes, but will not *represent* them in the AST; therefore,
libclang users can't see any representation of what attributes were present
at parse time. Basically, libclang treats unknown attributes like comments
or preprocessor directives; they just disappear. This does seem suboptimal
from the libclang-user's point of view! But I guess they do it that way
because it's hard to know how to represent an unknown macro's "token soup"
argument list, and/or to save memory (same reason they discard the text of
comments).
The reader (like me) will be sympathetic to those SO questioners. But
that's 100% a bug report you should file against the LLVM project; it has
nothing at all to do with the paper standard. In fact, someone's beaten you
to it! See llvm/llvm-project #57748
<https://github.com/llvm/llvm-project/issues/57748>.

I think P3132 would be better off not being in the February mailing at all.
Of course you should feel free to (and probably go ahead and) submit it
anyway — don't take my opinion as utter gospel — but you should at least be
prepared for WG21 to reject it for basically the above reasons: "This is an
ill-informed (duplicate) Clang bug report, not a WG21 proposal at all."

my $.02,
Arthur