aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land.
LGTM aside from a minor nit in the testing. ================ Comment at: clang/lib/Sema/SemaDecl.cpp:4633-4637 Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, DeclSpec &DS, RecordDecl *&AnonRecord) { - return ParsedFreeStandingDeclSpec(S, AS, DS, MultiTemplateParamsArg(), false, + return ParsedFreeStandingDeclSpec(S, AS, DS, ParsedAttributesView::none(), + MultiTemplateParamsArg(), false, AnonRecord); ---------------- mboehme wrote: > aaron.ballman wrote: > > It's surprising that we don't need to update this interface as well; is > > that inconsistency desired? > This is OK because every caller of this overload happens to do > `ProhibitAttributes(DeclAttrs)`. > > But now that you bring this up, I realize that this is risky. What if we add > another caller in the future that doesn’t do > `ProhibitAttribubtes(DeclAttrs)`? And even in the current situation, I think > it makes sense to be explicit that we’re passing an empty list of attributes. > > So I’ve added the `DeclAttrs` parameter to this function too, and now pass > `ParsedAttributesView::none()` at the callsites. Thanks, I think this is more clear. ================ Comment at: clang/test/SemaCXX/attr-declspec-ignored.cpp:3-6 +// For this test, we want the contents of the namespaces to be indented, which +// clang-format doesn't like. Also, clang-format reformats the long lines in a +// way that's difficult to read. +// clang-format off ---------------- We don't typically put clang-format markings into tests -- we don't require that tests be formatted, and it's a bug that precommit CI continues to fail because of it (https://github.com/llvm/llvm-project/issues/55982). Feel free to remove this bit when landing. ================ Comment at: clang/test/SemaCXX/attr-declspec-ignored.cpp:62 + +// clang-format on ---------------- Same here for this one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128499/new/ https://reviews.llvm.org/D128499 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits