Different attribute positions for 'noderef' has surprising behaviors

[AaronBallman]

While working on an unrelated crash, I noticed that the noderef attribute is highly sensitive to syntactic position as a GNU-style attribute, and the behavior seems inconsistent.

// When written before the declaration, it works.
__attribute__((noderef)) int * a;
int w = *a;

// When written in this location on the type, it works.
int __attribute__((noderef)) * b;
int x = *b;

// But when written in this location on the type, it gets loudly ignored.
int * __attribute__((noderef)) c;
int y = *c;

// And when written on the declarator, it gets quietly ignored, but only
// sort of (note the AST dump).
int * d __attribute__((noderef));
int z = *d;

https://godbolt.org/z/hesz5M4K8

I'm mostly surprised by the behavior of c and d. I would expect c to work because the attribute is written on a pointer type and I would sort of expect d to ignore the attribute loudly because it's a type attribute written on a declarator (I could also see "sliding" the attribute to the type because this is a GNU-style spelling).

[llvmbot]

@llvm/issue-subscribers-bug

Author: Aaron Ballman (AaronBallman)

While working on an unrelated crash, I noticed that the `noderef` attribute is highly sensitive to syntactic position as a GNU-style attribute, and the behavior seems inconsistent. ``` // When written before the declaration, it works. __attribute__((noderef)) int * a; int w = *a;

// When written in this location on the type, it works.
int attribute((noderef)) * b;
int x = *b;

// But when written in this location on the type, it gets loudly ignored.
int * attribute((noderef)) c;
int y = *c;

// And when written on the declarator, it gets quietly ignored, but only
// sort of (note the AST dump).
int * d attribute((noderef));
int z = *d;

https://godbolt.org/z/hesz5M4K8

I'm mostly surprised by the behavior of `c` and `d`. I would expect `c` to work because the attribute is written on a pointer type and I would sort of expect `d` to ignore the attribute loudly because it's a type attribute written on a declarator (I could also see "sliding" the attribute to the type because this is a GNU-style spelling).
</details>

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Aaron Ballman (AaronBallman)

While working on an unrelated crash, I noticed that the `noderef` attribute is highly sensitive to syntactic position as a GNU-style attribute, and the behavior seems inconsistent. ``` // When written before the declaration, it works. __attribute__((noderef)) int * a; int w = *a;

// When written in this location on the type, it works.
int attribute((noderef)) * b;
int x = *b;

// But when written in this location on the type, it gets loudly ignored.
int * attribute((noderef)) c;
int y = *c;

// And when written on the declarator, it gets quietly ignored, but only
// sort of (note the AST dump).
int * d attribute((noderef));
int z = *d;

https://godbolt.org/z/hesz5M4K8

I'm mostly surprised by the behavior of `c` and `d`. I would expect `c` to work because the attribute is written on a pointer type and I would sort of expect `d` to ignore the attribute loudly because it's a type attribute written on a declarator (I could also see "sliding" the attribute to the type because this is a GNU-style spelling).
</details>

[zygoloid]

Allowing c seems a bit scary, given that

typedef int * __attribute__((noderef)) p;
p a;
p *b;

might be expected to interpret the attribute in completely different ways. In general, I think we should be a bit cautious about sliding the attribute from pointer to pointee when it could be meaningful in both places.

We produce a warning on the declaration of p, saying that the attribute can't go there, but it actually does work and causes *b (and not *a) to produce warnings. I wonder if we should suppress that attribute when declaring a typedef.

Definitely makes sense to me to reject either the declaration of d (no sliding) or the dereference of it (sliding it some distance to the left).

Though as a general concern: moving attributes to somewhere other than where they were written is a semantically horrible thing to do, and I don't think it's good for our users. Doing it for GCC's attributes makes sense for compatibility (though it'd be nice to warn when we do so), but for Clang-only attributes maybe we should not do so as a matter of policy (regardless of whether GNU attribute syntax is used)?

[AaronBallman]

In general, I think we should be a bit cautious about sliding the attribute from pointer to pointee when it could be meaningful in both places.

That's a fair point! But at the same time, people frequently use macros to hide their attributes, C++ has very specific syntactic requirements for where attributes are written, and this can be spelled __attribute__((noderef)) or [[clang::noderef]]. This means it's very plausible to have macros like:

#if __has_cpp_attribute(clang::noderef)
#define NODEREF [[clang::noderef]]
#elif __has_attribute(noderef)
#define NODEREF __attribute__((noderef))
#else
#define NODEREF
#endif

and write code like:

int * NODEREF c;

GCC doesn't currently support this attribute, and so such code is basically always wrong today. But if GCC (or EDG, etc) were to support noderef as a GNU-style attribute but not a [[]] attribute with the clang vendor prefix, the user is put in an awkward position as to where to write the macro in a declaration. Note, they already have this problem somewhat in that NODEREF int *c; works with the GNU-style attribute but not the `[[]]`` style one, but the point still stands that there's literally nowhere they could write the macro that would work in both cases. This applies to any type attribute which appertains to a pointer, I believe.

[AaronBallman]

Though as a general concern: moving attributes to somewhere other than where they were written is a semantically horrible thing to do, and I don't think it's good for our users. Doing it for GCC's attributes makes sense for compatibility (though it'd be nice to warn when we do so), but for Clang-only attributes maybe we should not do so as a matter of policy (regardless of whether GNU attribute syntax is used)?

In general, I agree. The problem there is that Clang sometimes adds attributes which GCC then later picks up. But I suppose we could then relax Clang's behavior in response to GCC picking up the attribute later, so maybe that's not a bad idea. CC @erichkeane for additional thoughts

[erichkeane]

Though as a general concern: moving attributes to somewhere other than where they were written is a semantically horrible thing to do, and I don't think it's good for our users. Doing it for GCC's attributes makes sense for compatibility (though it'd be nice to warn when we do so), but for Clang-only attributes maybe we should not do so as a matter of policy (regardless of whether GNU attribute syntax is used)?

In general, I agree. The problem there is that Clang sometimes adds attributes which GCC then later picks up. But I suppose we could then relax Clang's behavior in response to GCC picking up the attribute later, so maybe that's not a bad idea. CC @erichkeane for additional thoughts

Since GCC doesn't have this, I'd be completely fine making BOTH c and d ill-formed declarations (that is, loudly ignored), it seems to make the most sense to me. Macro-people aren't getting what they want right now anyway, and if GCC implements it with some additional sliding, and folks care enough to submit a patch, we can re-visit it then.

But for now, I would consider "making d ignored-loudly" to be a win/good state.