[PATCH] D106030: [Clang] add support for error+warning fn attrs

Nick Desaulniers via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 2 14:49:37 PDT 2021


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



More information about the cfe-commits mailing list