[clang] [clang] WIP: Warn on mismatched RequiresCapability attributes (PR #67520)

Aaron Puchert via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 22 16:16:04 PDT 2024


aaronpuchert wrote:

> > Yeah, it's a tricky question. On one hand there have been previous issues like #42194 (which this would basically address), and it would definitely improve consistency and prevent beginner's mistakes. On the other hand it seems useful to allow adding attributes e.g. to third-party libraries where you don't control the headers.
> 
> I'm not sure that this mistake is one that only beginners would make.

Sure, don't read too much into that statement.

But as a rule of thumb, static analysis attributes always belong on the publicly visible declaration. Otherwise, the caller can't see them. This is true not only for thread safety attributes, but also nullability attributes, handle attributes, type state attributes, and so on. I'm not sure how they're handling it. For `[[noreturn]]` we [have to emit an error](https://eel.is/c++draft/dcl.attr.noreturn#1).

> But I agree that adding attributes to functions from third-party libraries is important. I think this is why I first suggested the following in the [downstream tracker](https://issues.redhat.com/browse/RHEL-7269): "I propose that clang should warn if a function definition has TSA attributes, a previous declaration of the same function exists and the attribute isn't present on **at least one** of the previous declarations."

Ok, that's an interesting twist. I could live with that, but I'm not sure if we handle any other atttributes this way.

> But come to think of it, I'm not sure any more if this really makes a difference – typically, you don't even have a definition for functions from third-party libraries that could trigger the warning, only declarations. The exception are inline functions in headers, but even then, what you get is a declaration with the attribute and a definition without it, which still wouldn't trigger the warning. So do we actually have a problem here?

What I was thinking about was warning on any redeclaration that's adding attributes. Sure, if you only do this on definitions, adding attributes to third-party library wouldn't be an issue.

https://github.com/llvm/llvm-project/pull/67520


More information about the cfe-commits mailing list