[PATCH] D51473: Improve attribute documentation to list which spellings are used in which syntaxes.

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 31 05:07:29 PDT 2018


On Thu, Aug 30, 2018 at 4:12 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Thu, 30 Aug 2018 at 12:27, Aaron Ballman via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
>>
>> On Thu, Aug 30, 2018 at 3:21 PM, Richard Smith - zygoloid via
>> Phabricator <reviews at reviews.llvm.org> wrote:
>> > rsmith marked an inline comment as done.
>> > rsmith added inline comments.
>> >
>> >
>> > ================
>> > Comment at: utils/TableGen/ClangAttrEmitter.cpp:3881
>> > +    SpellingKind K = (SpellingKind)Kind;
>> > +    // FIXME: Why are Microsoft spellings not listed?
>> > +    if (K == SpellingKind::Microsoft)
>> > ----------------
>> > aaron.ballman wrote:
>> >> We don't actually support Microsoft's attribute spellings currently and
>> >> have no attributes there to document. I think the fixme should probably read
>> >> "TODO: support documenting Microsoft spellings" or something more concrete.
>> > Done. (I accidentally pushed the old version, so this is done in
>> > r341100.)
>> >
>> > For what it's worth, we have one `Microsoft` spelling listed in the .td
>> > file already (but I assume this has no effect):
>> >
>> > ```
>> > def Uuid : InheritableAttr {
>> >   let Spellings = [Declspec<"uuid">, Microsoft<"uuid">];
>> > ```
>>
>> Hmm, I take it back, we do support a Microsoft attribute, only to warn
>> about it being deprecated and telling users to use __declspec instead:
>> https://godbolt.org/z/_0ZxWq
>>
>> I remember when we tried to add more support for parsing Microsoft
>> attributes, but I had the impression we didn't support them beyond the
>> very basics of parsing. Perhaps we do want to document them though,
>> since there's at least one?
>
>
> Given that doing so will make the "supported syntaxes" table wider for all
> attributes, and it's already about as wide as seems reasonable, and we
> consider all attributes of this form to be deprecated, I don't think it's
> worth it. Maybe if we only included non-empty columns in the syntax table?

I think for right now I'd prefer to keep a consistent syntax table
layout. It makes it easier to visually scan for information when all
the columns roughly line up vertically, and we only support this one
spelling just to tell users not to use it anyway. We can try out the
column approach later if we find need.

~Aaron


More information about the cfe-commits mailing list