[PATCH] D128499: [Clang] Fix: Restore warning inadvertently removed by D126061.
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 27 06:29:44 PDT 2022
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
More information about the cfe-commits
mailing list