rsandifo-arm added inline comments.

================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2000
   "overridden virtual function is here">;
+def err_conflicting_overriding_attributes : Error<
+  "virtual function %0 has different attributes "
----------------
sdesmalen wrote:
> aaron.ballman wrote:
> > sdesmalen wrote:
> > > aaron.ballman wrote:
> > > > This error and the one below make me wonder whether an attribute 
> > > > spelling is the appropriate way to surface this functionality as 
> > > > opposed to a keyword. Attributes don't typically don't cause errors in 
> > > > these situations, but because these attributes are strongly coupled to 
> > > > their type system effects I can see why you want these to be errors.
> > > > This error and the one below make me wonder whether an attribute 
> > > > spelling is the appropriate way to surface this functionality as 
> > > > opposed to a keyword. 
> > > I'm not sure I understand what you mean, do you have an example?
> > `override` and `final` could have been surfaced via attributes, and in 
> > Clang we semantically express them as such (see `Final` and `Override` in 
> > Attr.td), but instead they are surfaced to the user as keywords in the 
> > language standard. You're not under the same constraints as the standard 
> > (you're making a vendor-specific attribute). We're not super consistent 
> > with whether we use the same approach as the standard (we have some type 
> > attributes that are spelled as attributes like `vector_size` and others 
> > that are spelled via keywords like `_Nullable`.
> > 
> > So you could spell your type attributes the way you have been with 
> > `__attribute__`, or you could come up with keywords for them (so instead of 
> > using `GNU<"whatever">` for the attribute, you could use 
> > `Keyword<_Whatever>` or `Keyword<__whatever>` (you'd also need to add them 
> > as recognized keyword tokens, parse them as appropriate, etc).
> > 
> > Note: I don't insist on you using a keyword for these, but I am wondering 
> > if that approach was considered or not given how non-portable the 
> > attributes are (if an implementation ignores this attribute, it sounds like 
> > the program semantics are unlikely to be correct).
> @rsandifo-arm can you shed some light on whether using a keyword instead of 
> an `attribute` was considered?
Thanks @aaron.ballman for the reviews.
 
Yeah, we did consider using keywords instead.  The main reason for sticking 
with GNU attributes is that they were specifically designed as an extensible 
way of attaching information or requirements to the source code, and they 
provide well-settled semantics.  It seemed better to reuse an existing 
mechanism rather than invent something new.

Also, as C++ evolves, the semantics of GNU attributes evolve to match.  If we 
surface the SME features as GNU attributes, we inherit this development in the 
underlying semantics, without having to maintain our own evolving specification 
for where the keywords can be placed.  For example, when lambdas were 
introduced, GNU attributes were extended to support lambdas.  Something similar 
could happen in future.

We could have defined the semantics of the keywords to be that they behave 
exactly like GNU attributes.  However:

# If we do that, the advantage of using a keyword is less obvious.  (I can see 
the argument that the syntax looks nicer though.)
# Like you say in the review, the semantics of GNU attributes carry some 
historical baggage.  If would be difficult to justify keeping that historical 
baggage for something that is ostensibly new and not a GNU attribute as such.

A minor, but still nontrivial, reason is that there is less appetite in the GCC 
community for supporting target-specific extensions to the core language.  
Adding a target-specific keyword is likely to be rejected.  It would be 
acceptable to make the “keyword” be a predefined macro that expands to a GNU 
attribute under the hood, but I don't think that would address the core issue.

I can definitely understand the unease about using attributes for things that 
affect semantics.  Like you say, that's going against a core principle of the 
standard-defined attributes.  But I think in a way, it's unfortunate that 
GNU-style attributes and standard-defined attributes are both called 
“attributes”, because I don't think there's a prohibition (even in theory) 
against GNU attributes affecting semantics.  I think GNU attributes are 
designed to be more general than that, probably through historical accretion 
rather than an up-front choice.

Like you say, `vector_size` is one example of something that significantly 
affect semantics.  But there are quite a few others too.  For example:
* GNU targets traditionally provide access to naked functions and interrupt 
handlers through attributes.
* Different calling conventions also use attributes.
* The `target` attribute switches between ISAs in ways that do affect semantics.
* `always_inline` functions must be inlined for correctness.
* `weak` and `visibility` in effect alter the ODR.
* In GCC, functions that return twice (like `setjmp`) must be declared 
`returns_twice`.

It's unfortunate that using SME attributes will only trigger a warning in older 
compilers.  The warning is enabled by default though.  And in practice, I think 
most SME code would rely on other constructs that older compilers wouldn't 
provide, such as intrinsics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127762

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to