aaron.ballman added a comment.
In general, I think this is reasonable -- there's a bit more work to be done
before it's ready to land, but this is heading in the right direction.
================
Comment at: clang/include/clang/Basic/Attr.td:559
+/// A attribute is either a declaration attribute or a statement attribute.
+class DeclOrStmtAttr : Attr;
+
----------------
I think this should be an `InheritableAttr`, like `DeclOrTypeAttr`. With
type/decl attributes, those are almost always for things like calling
conventions or other cases where inheritance of the attribute is really likely
to be the correct default. It's less clear to me that the same is true for
stmt/decl attributes aside from the observation that most decl attributes are
inheritable. We may need to come up with a better approach for inheritance here
at some point when we find a stmt/decl attribute that should not be inheritable.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:2185
+static void handleNoMergeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ if (!isFunctionOrMethod(D)) {
----------------
No need for this function, you can set `let SimpleHandler = 1;` in Attr.td --
the common attribute handler code should handle checking the subject list for
you automatically.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7757
+ case ParsedAttr::AT_NoMerge:
+ handleNoMergeAttr(S, D, AL);
+ break;
----------------
Then this can become `handleSimpleAttribute<NoMergeAttr>(S, D, AL);`
================
Comment at: clang/test/CodeGen/attr-nomerge.cpp:73
+// CHECK-DAG: attributes #[[ATTR2]] = {{{.*}}nomerge{{.*}}}
+// CHECK-DAG: attributes #[[ATTR3]] = {{{.*}}nomerge{{.*}}}
----------------
Can you also add a test case to demonstrate that inheriting the attribute on a
later redeclaration works? It can either be a codegen test or an AST dump test.
================
Comment at: clang/test/Sema/attr-nomerge.cpp:11
- [[clang::nomerge]] label: bar(); // expected-error {{'nomerge' attribute
cannot be applied to a declaration}}
+ [[clang::nomerge]] label: bar(); // expected-warning {{'nomerge' attribute
only applies to functions and methods}}
----------------
This diagnostic is no longer correct -- nomerge now applies to more things. You
may need to teach the clang attribute emitter about stmt/decl attribute types
so that it can generate a better diagnostic in `diagnoseAppertainsTo()`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92800/new/
https://reviews.llvm.org/D92800
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
