nickdesaulniers added inline comments.
================
Comment at: clang/include/clang/Basic/Attr.td:3816
+
+def Error : Attr {
+ let Spellings = [GCC<"error">];
----------------
aaron.ballman wrote:
> nickdesaulniers wrote:
> > aaron.ballman wrote:
> > > I think this should be an inheritable attribute (same below) so that
> > > redeclarations get the marking as well.
> > >
> > > However, this does make for a bit of a novel situation with regards to
> > > other attributes. The typical behavior for function attributes is:
> > > ```
> > > void func(void);
> > >
> > > void use1(void) {
> > > func(); // Func is not marked yet, no attribute behavior
> > > }
> > >
> > > void func(void) __attribute__((whatever)));
> > >
> > > void use2(void) {
> > > func(); // Func is marked, attribute does something
> > > }
> > >
> > > void func(void) {} // Func is still marked because the attribute is
> > > inheritted
> > >
> > > void use3(void) {
> > > func(); // Func is marked, attribute does something
> > > ```
> > > but because all of the interesting work is happening in the backend, I
> > > believe the unmarked use will still act as though the attribute was
> > > marked.
> > Changing this def to:
> > ```
> > -def Error : Attr {
> > +def Error : InheritableAttr {
> > ```
> > doesn't seem to make your test case work; is there some other method I
> > should be using to find the re-declaration and check the attributes against
> > that?
> That's what I meant by this being novel -- I don't think those test cases
> will work with this design (having the backend decide whether to diagnose),
> and so this attribute will behave somewhat inconsistently with other function
> attributes. This may cause confusion for users or for tooling. I think for
> users, we can document the behavior and that will be fine. For tooling, we
> may have to get more creative (such as applying the attribute to all
> redeclarations of the same function), but perhaps we can hold off on that
> until we see if tooling runs into a problem in practice.
I'm having a very difficult time getting inheritable attr's to work correctly.
For example:
```
void foo(void);
void x0(void) { foo(); }
__attribute__((error("oh no foo")))
void foo(void);
void x1(void) { foo(); }
void foo(void);
void x2(void) { foo(); }
```
I can get all `Decl`s of `foo` to have the attribute (via `Inherit` in the ast
dump), but the first call to `foo()` doesn't diagnose; at that point we haven't
encountered a `Decl` that has the attribute. So it's hard to match GCC's
behavior of warning on all three. Is there something I should be doing
differently in `CodeGenModule::SetFunctionAttributes` to check the "final decl"
rather than the current/latest `Decl`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106030/new/
https://reviews.llvm.org/D106030
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
