[PATCH] D41553: Support parsing double square-bracket attributes in ObjC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: include/clang/Basic/Attr.td:239
+  bit IncludeC = includeC;
+}
 
----------------
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.


https://reviews.llvm.org/D41553