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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
