Documentation for `counted_by` incorrectly claims support for C++

[Kmeakin]

This code generates a warning when compiled in C++ mode, but not in C mode: https://godbolt.org/z/7zTW63exP

#include <stddef.h>
#include <stdint.h>

struct slice {
    size_t len;
    uint32_t* data __attribute__((counted_by(len)));
};

<source>:6:35: warning: 'counted_by' attribute ignored [-Wignored-attributes]
    6 |     uint32_t* data __attribute__((counted_by(len)));
      |                                   ^

This warning could be improved by telling the user that counted_by is only allowed in C mode, and will be ignored in C++ mode. The documentation could also be improved to state this explicitly.

[shafik]

I was looking at this quickly and it is not obvious it is a good first bug, it looks like some extra pieces need to be put in place but maybe it is simpler than I think.

CC @rapidsna @Bigcheese

[GustavoARSilva]

This warning could be improved by telling the user that counted_by is only allowed in C mode, and will be ignored in C++ mode. The documentation could also be improved to state this explicitly.

It was explicitly mentioned in the release notes:
"Clang now supports the C-only attribute counted_by."

And, perhaps, they thought this sentence was enough (in the docs): "Clang supports the counted_by attribute on the flexible array member of a structure in C."

[rapidsna]

Thanks for reporting the issue! It seems the document is wrong in that it listed the attribute as available in C++11 too. We will fix it. Hope that will make it clear. And the diagnostics could also be improved, I agree.

https://clang.llvm.org/docs/AttributeReference.html#counted-by

[AaronBallman]

The reason the documentation is incorrect is because the documentation emitter isn't LangOpt-aware. We'd probably need to modify WriteDocumentation() in ClangAttrEmitter.cpp to either understand common language options, or we'd need some new tablegen markup to allow attributes to override the default behavior.

I'm changing the title of the issue to reflect what the problem is (the behavior itself for the attribute is correct) and confirming.

[per-mathisen-arm]

Is there a forward-compatible way to detect if this attribute is supported under C++ (I assume it will be one day)? I notice that #if __has_attribute(counted_by) evaluates true for c++ with 19.1, which is a bit unfortunate when it really isn't supported yet.

[AaronBallman]

It looks like the machinery for __has_attribute and __has_cpp_attribute are also not aware of LangOpt, same as documentation. :-( Unfortunately, that's going to make feature test detection pretty frustrating here. CC @erichkeane

https://godbolt.org/z/We6665fbh

[erichkeane]

Hmm... that is unfortunate. It seems that it would be a decent amount of work there to ensure those are enabled correctly, though I think we already do a decent job of __has_attribute and __has_cpp_attribute meaning "you can use this", so IDK why we didn't.

[per-mathisen-arm]

Continuing on that topic, I think I saw some plans to add this to function arguments as well? That would be quite useful, but if so, what would be the attribute check to use for that extension of the feature?

[AaronBallman]

Continuing on that topic, I think I saw some plans to add this to function arguments as well? That would be quite useful, but if so, what would be the attribute check to use for that extension of the feature?

The attribute feature testing macros return 0 to say "not supported" and nonzero to say "supported", and we often modify the resulting nonzero value as the feature changes so you can test for specific versions. e.g., __has_attribute(foo) may return 202001L for its initial feature set, and 202501L for its next feature set.