On Mon, Aug 29, 2016 at 7:52 PM, Reid Kleckner <r...@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 > > > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
