[PATCH] D23895: [ms] Add support for parsing uuid as a MS attribute

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 30 08:19:34 PDT 2016


On Mon, Aug 29, 2016 at 7:52 PM, Reid Kleckner <rnk at google.com> wrote:
> rnk added a comment.
>
> In https://reviews.llvm.org/D23895#528208, @aaron.ballman wrote:
>
>> You are removing a lot of instances of `MaybeParseMicrosoftAttributes()`, but I'm not certain I understand why. Can you describe why you need to remove those?
>
>
> That shouldn't change functionality, Nico wrote: "It moved calls of MaybeParseMicrosoftAttributes(attrs) that precede ParseExternalDeclaration(attrs) into ParseExternalDeclaration(). As far as I can tell, every call of ParseExternalDeclaration() was preceded by a call to MaybeParseMicrosoftAttributes(), so this should be behavior-preserving."

Thank you for that explanation, I missed that!

> ================
> Comment at: include/clang/Sema/AttributeList.h:111
> @@ -108,3 +110,3 @@
>      /// #pragma ...
> -    AS_Pragma
> +    AS_Pragma,
>    };
> ----------------
> aaron.ballman wrote:
>> Please remove the trailing comma.
> IMO trailing commas in long lists are nice. There's a reason they got added to C++11.

This list isn't exactly long, and the change is unrelated to the
patch. I don't feel overly strongly about it except that I don't think
it's a particularly useful drive-by.

~Aaron

>
>
> https://reviews.llvm.org/D23895
>
>
>


More information about the cfe-commits mailing list