rjmccall added inline comments.
================
Comment at: include/clang/Basic/Attr.td:239
+ bit IncludeC = includeC;
+}
----------------
aaron.ballman wrote:
> rjmccall wrote:
> > aaron.ballman wrote:
> > > rjmccall wrote:
> > > > aaron.ballman wrote:
> > > > > rjmccall wrote:
> > > > > > I have no objection to allowing ObjC attributes to be spelled in
> > > > > > [[clang::objc_whatever]] style. We can debate giving them a more
> > > > > > appropriate standard name in the objc namespace at a later time —
> > > > > > or even the primary namespace, if we really want to flex our
> > > > > > Somewhat Major Language muscles.
> > > > > >
> > > > > > I feel like this IncludeC is not the best name for this tablegen
> > > > > > property. Perhaps AllowInC?
> > > > > >
> > > > > > Also, a random positional 1 argument is pretty obscure TableGen
> > > > > > design. Do we really expect to be making very many attributes that
> > > > > > intentionally disallow C2x? What would these be, just C++-specific
> > > > > > attributes without C analogues? It doesn't seem important to
> > > > > > prohibit those at the parsing level rather than at the semantic
> > > > > > level, since, after all, we are still allowing the GNU-style
> > > > > > spelling, and these would still qualified with ``clang::``.
> > > > > >
> > > > > > I would suggest standardizing in the opposite direction: instead
> > > > > > of forcing attributes to opt in to being allowed in C, we should
> > > > > > make attributes that we really don't want to allow in the C2x
> > > > > > [[clang::]] namespace be explicit about it. If there are a lot of
> > > > > > C++-specific attributes like that, we can make a ClangCXXOnly
> > > > > > subclass.
> > > > > > I have no objection to allowing ObjC attributes to be spelled in
> > > > > > [[clang::objc_whatever]] style. We can debate giving them a more
> > > > > > appropriate standard name in the objc namespace at a later time —
> > > > > > or even the primary namespace, if we really want to flex our
> > > > > > Somewhat Major Language muscles.
> > > > >
> > > > > Thanks -- are you okay with where the attributes are allowed in the
> > > > > syntax? I tried to follow the position they're allowed in C and C++
> > > > > as closely as I could, but having confirmation would be nice.
> > > > >
> > > > > As for putting the attributes in the primary namespace, that would be
> > > > > reasonable for using the attributes in ObjC or ObjC++ but not so much
> > > > > for using the same attributes in a pure C or C++ compilation.
> > > > >
> > > > > > I feel like this IncludeC is not the best name for this tablegen
> > > > > > property. Perhaps AllowInC?
> > > > >
> > > > > I'm fine with that name. I'll change it in D41317 when I commit that.
> > > > >
> > > > > > Also, a random positional 1 argument is pretty obscure TableGen
> > > > > > design. Do we really expect to be making very many attributes that
> > > > > > intentionally disallow C2x? What would these be, just C++-specific
> > > > > > attributes without C analogues?
> > > > >
> > > > > I agree -- I was toying with using a named entity rather than a
> > > > > numeric literal, but I wanted to see how the design shook out in
> > > > > practice once I had a better feel for how many attributes are
> > > > > impacted. I'm glad you recommend going the opposite direction as that
> > > > > was my ultimate goal. :-) Basically, my initial approach is to
> > > > > disallow everything in C and then start enabling the attributes that
> > > > > make sense. At some point, I believe we'll have more attributes in
> > > > > both language modes than not, and then I plan to reverse the meaning
> > > > > of the flag so that an attribute has to opt-out rather than opt-in. I
> > > > > don't expect we'll have many attributes that are disallowed in C2x. I
> > > > > think they'll fall into two categories: C++ attributes that don't
> > > > > make sense in C and attributes that are governed by some other body.
> > > > >
> > > > > > It doesn't seem important to prohibit those at the parsing level
> > > > > > rather than at the semantic level, since, after all, we are still
> > > > > > allowing the GNU-style spelling, and these would still qualified
> > > > > > with `clang::`.
> > > > >
> > > > > I think it's important for C targets when
> > > > > `-fdouble-square-bracket-attributes` is not enabled, as the
> > > > > attributes cannot syntactically appear in those positions.
> > > > >
> > > > > > I would suggest standardizing in the opposite direction
> > > > >
> > > > > I suspect I will ultimately get there. I chose this approach to be
> > > > > more conservative about what we expose.
> > > > > Thanks -- are you okay with where the attributes are allowed in the
> > > > > syntax? I tried to follow the position they're allowed in C and C++
> > > > > as closely as I could, but having confirmation would be nice.
> > > >
> > > > I'll leave comments on the various places. For the most part, no, I
> > > > think these are the wrong places; where we allow attributes in ObjC is
> > > > actually pretty carefully thought-out already, and it's better to
> > > > follow the places where we parse GNU attributes than to try to imitate
> > > > the C grammar.
> > > >
> > > > > As for putting the attributes in the primary namespace, that would be
> > > > > reasonable for using the attributes in ObjC or ObjC++ but not so much
> > > > > for using the same attributes in a pure C or C++ compilation.
> > > >
> > > > Yes, please ignore that. I was just idly thinking about what we would
> > > > do if we were adopting this feature as a more standard thing, i.e.
> > > > leaving the implementation namespace. I think we'd want to rename some
> > > > of the attributes for consistency, and then I think we'd just put them
> > > > in the objc:: namespace. None of this affects what we're doing now.
> > > >
> > > > > I think it's important for C targets when
> > > > > -fdouble-square-bracket-attributes is not enabled, as the attributes
> > > > > cannot syntactically appear in those positions.
> > > >
> > > > Okay, that's fair.
> > > >
> > > > > I suspect I will ultimately get there. I chose this approach to be
> > > > > more conservative about what we expose.
> > > >
> > > > Sure, if this is really just for staging, I won't worry about
> > > > accidentally leaving attributes out.
> > > > I'll leave comments on the various places. For the most part, no, I
> > > > think these are the wrong places; where we allow attributes in ObjC is
> > > > actually pretty carefully thought-out already, and it's better to
> > > > follow the places where we parse GNU attributes than to try to imitate
> > > > the C grammar.
> > >
> > > The C grammar was also carefully thought-out to consistently map the
> > > location of the attribute syntax in source code to the entity the
> > > attribute appertains to. The basic rule is: the attribute always
> > > appertains to what's immediately to the left unless the attribute is at
> > > the start of the line, in which case it applies to each declaration in
> > > the decl group. This logic was taken from C++ attributes where it's
> > > proven very valuable over the past seven or so years. If we follow the
> > > GNU syntactic placement, that means this rule of thumb doesn't apply in
> > > Objective-C and it negates one of the most powerful aspects of the
> > > language feature: syntactic consistency (which is something GNU-style
> > > attributes absolutely lacks, to everyone's great annoyance).
> > >
> > > Can you explain the rationale about why the GNU-style placement is a
> > > better approach to the C- and C++-style placement? I think that the
> > > syntactic location of the attribute syntax should remain consistent
> > > between ObjC constructs and C constructs because both constructs are
> > > likely to be mixed together in the same code base. I think we hurt the
> > > usability of the language feature by being inconsistent in this fashion
> > > (which was one of the reasons we standardized attributes in the first
> > > place).
> > The ObjC grammar is overall quite different from the C grammar. There are
> > no decl groups. Tags like `@interface` are always declaration introducers,
> > rather than part of the type grammar. Types are not incidentally
> > declared/defined in the type-specifier of a possible object or function
> > declaration. More subjectively, nobody looks at the primary Objective-C
> > declarations like `@interface` or `@protocol` and thinks they look like
> > anything else from C or C++, which means consistency is not really an issue.
> >
> > C++ puts class attributes after the `class-key` because there's literally
> > no other place to put attributes that wouldn't be conflated with an
> > attribute on the object if an object were being simultaneously declared.
> > The C grammar paints them into a corner. We don't have that specific
> > problem in ObjC because, as mentioned, ObjC declarations are standalone,
> > not wedged into the object/function declaration grammar.
> >
> > C++ seems to be inconsistent about where it allows keywords when it does
> > have a proper declaration-introducer. For example, the grammar for a
> > `namespace` declaration is `'namespace' attribute-specifier-seq?
> > identifier?`, presumably following the lead of `class`. But the grammar
> > for an alias declaration is `'using' identifier attribute-specifier-seq?
> > '=' defining-type-id`, not `'using' attribute-specifier-seq? identifier '='
> > defining-type-id`. And of course there are the context-sensitive class
> > attributes (e.g. `final`) that are written in a special position, which is
> > allowable under the grammar because they only need to apply to definitions.
> >
> > I understand that the GNU/Microsoft/Borland/whatever attribute positions
> > often seem //ad hoc// and that the C++ committee tried to improve on that
> > when standardizing attributes. In contrast, I think the ObjC community is
> > satisfied with where attributes are written in ObjC declarations today.
> > When I said that those positions were carefully thought-out, I mean that
> > we've already talked a lot about where attributes should go, and the
> > current positions are the result of those conversations. That process
> > seems to have been a success; the only complaints I can remember ever
> > getting about attribute parsing have been related to where C requires them,
> > not ObjC. In general, ObjC pushes attributes to the beginning or (in the
> > case of methods) the end of the declaration, which people see as a good
> > thing: attributes can get quite large, so piling them up on the fringes of
> > the declaration ensures that they don't interfere with reading the more
> > immediately important parts of the declaration. While C/C++ did not have a
> > choice about where to allow attributes on `class` declarations, objectively
> > it is very awkward to put attributes immediately after the tag because it
> > separates the two most important things in the declaration: the declaration
> > introducer and the name being declared. That is not something we want to
> > imitate.
> >
> > I am not just speaking for myself here. This is where we would like the
> > attributes to appear.
> Thank you for the helpful explanation, I appreciate it.
>
> Do you have issue with where C is placing these attributes for constructs
> that are supported by C, or are those also fine for use in ObjC? Basically,
> I'm wondering whether you would want this syntax at all in ObjC, or whether
> you're okay with the C placement for C constructs and the ObjC placement for
> ObjC constructs?
>
> I'm a bit concerned about this scenario and would like your thoughts:
> ```
> @interface I
> -(void)Foo;
> @end
>
> @implementation I
> -(void)Foo [[clang::minsize]] { // OK, appertains to the function declaration
>
> }
>
> void bar(void) [[clang::minsize]] { // Error, minsize does not appertain to
> the function type
>
> }
> @end
> ```
We should just use the C rules for C constructs in ObjC mode, even if they're
intermingled with ObjC constructs. I see your point about the perceived
inconsistency, but I don't see a good resolution besides acceptance. Allowing
new locations for C attributes would be a mistake.
https://reviews.llvm.org/D41553
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
