aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Basic/Attr.td:1321
let Documentation = [NoMergeDocs];
+ let Subjects = SubjectList<[Function], ErrorDiag>;
+ let SimpleHandler = 1;
----------------
Related to my comments in ClangAttrEmitter.cpp, I think you should make these
changes instead.
================
Comment at: clang/test/CodeGen/attr-nomerge.cpp:73
+// CHECK-DAG: attributes #[[ATTR2]] = {{{.*}}nomerge{{.*}}}
+// CHECK-DAG: attributes #[[ATTR3]] = {{{.*}}nomerge{{.*}}}
----------------
zequanwu wrote:
> aaron.ballman wrote:
> > 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.
> Do you mean something like this?
Close. I was thinking something like:
```
int g(int i); // No attr
void something() {
// call g() here
}
[[clang::nomerge]] int g(int i);
void something_else() {
// call g() here
}
int g(int i) { return i; }
void something_else_again() {
// call g() here
}
```
So this tests that the attribute is inherited on redeclarations properly and it
also tests the merging behavior when the declaration may not have yet had the
attribute attached.
================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:3438-3439
}
+ if (DeclOrStmt)
+ DiagList.push_back("statements");
}
----------------
I think this will do the wrong thing when the subject list has more than one
entry (I think this will add `statements` to the list once for each subject).
As an experiment, can you add `ObjCMethod` to the list of subjects for
`NoMerge` in Attr.td? Do the resulting diagnostics list "statements" once or
twice?
Thinking a bit deeper on this -- I think my original advice may have been too
ambitious (we don't yet have support for automatically grabbing the names of
statements for diagnostics like we do for declarations). Trying to add that as
part of this patch would be a big ask, so I think a better approach is to use a
custom diagnostic in Attr.td. I'll add a comment there to this effect.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92800/new/
https://reviews.llvm.org/D92800
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
