feat: uniform attribute extraction#1237
Conversation
✨ Highlights
🧾 Changes by Scope
🔝 Top Files
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1237 +/- ##
========================================
Coverage 83.16% 83.16%
========================================
Files 35 35
Lines 3658 3658
Branches 843 843
========================================
Hits 3042 3042
Misses 409 409
Partials 207 207
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b62ee704ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
An automated preview of the documentation is available at https://1237.mrdocs.prtest2.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-06-22 04:50:33 UTC |
alandefreitas
force-pushed
the
feat/document-attributes
branch
from
b62ee70 to
608432a
Compare
|
Hi Alan, very good PR. I only left some minor comments. The most important one is about the duplication of |
| | `attributes` | ||
| | `<<attribute-fields,Attribute Object[]>>` | ||
| | The C++ attributes attached to the symbol (e.g., `[[deprecated]]`, `[[nodiscard]]`). Present for every symbol kind; absent when the symbol has no attributes. | ||
|
|
There was a problem hiding this comment.
"Present for every symbol kind" but then "absent when..." sounds strange :-). Perhaps we could say "Applies to every symbol kind"?
| // brackets and string/character literals. | ||
| std::vector<std::string> | ||
| splitAttributeArgs(std::string_view const s) | ||
| { |
There was a problem hiding this comment.
Do we normally use top-level const on function parameters?
| | Render the shape. | ||
| |=== | ||
|
|
||
|
|
There was a problem hiding this comment.
Why the extra empty lines?
There was a problem hiding this comment.
There's a template there but handlebars also creates an extra line when the template is empty. Let me see what I can do about that.
| IsVariadic, IsDefaulted, IsExplicitlyDefaulted, IsDeleted, | ||
| IsDeletedAsWritten, IsNoReturn, HasOverrideAttr, HasTrailingReturn, | ||
| IsNodiscard, IsExplicitObjectMemberFunction, Constexpr, | ||
| IsExplicitObjectMemberFunction, Constexpr, |
There was a problem hiding this comment.
I think you forgot to remove IsNoReturn.
| return args; | ||
| } | ||
| } // (anon) | ||
|
|
There was a problem hiding this comment.
Very nice implementation. It's only con is it doesn't correctly handle raw string literals (very rare, I guess, in an attribute, but I thought to mention that).
There was a problem hiding this comment.
Yes. This is the literal expression because clang doesn't give us the individual arguments in the general case. 😢
This is literally parsing std::string const text = getSourceCode(attr->getRange());.
On the other hand, I think we need the literal expression anyway. Otherwise, we would have to reconstruct the attribute on the other side.
I'll investigate the actual grammar and come up with something here.
| @@ -0,0 +1,6 @@ | |||
| {{! Deprecation admonition, shown when the symbol carries a [[deprecated]] attribute. }} | |||
| {{~#each symbol.attributes}} | |||
| {{~#if (eq name "deprecated")}} | |||
There was a problem hiding this comment.
I think this won't match [[gnu::deprecated]] (not sure if it will match __declspec(deprecated)).
There was a problem hiding this comment.
I noticed this one when writing it. I thought the template would become a mess if we tried to detect all kinds here, with little benefit. Because people who don’t use these other deprecated attributes usually do so because they don’t have access to C++20, but MrDocs does.
So if they use [[gnu::deprecated]], they probably are using macros for mrdocs and other C++20 compilers to see and document [[deprecated]] instead. So I thought accepting other kinds was just reinforcing the error somehow. So the template would become giant for very little benefit.
The solution I had in mind for this was to propose an optional feature that normalizes non-standard attributes if people complain. This would happen long before the templates. But I can’t really propose this now because I’m not so sure how that would go and if people actually care.
What do you think?
There was a problem hiding this comment.
Yes, I agree, normalization is the way to go; but let's wait to see if people actually care.
3 participants
Reads attributes for every kind of symbol and shows them in the generated pages. Before, it only read attributes on functions and variables, and a deprecated symbol gave no sign that it was deprecated. For other symbols, it showed nothing. Now every symbol carries its attributes, deprecated symbols get a notice with the message in an admonition, and attributes appear in signatures and member tables.
Fixes #1233.
Changes
In the source code, attributes now live on the common symbol base class. Each attribute now contains its name and all its arguments. The templates render them in each signature, add a deprecation notice, and tag deprecated entries in member tables the same way constructors are tagged. The output schema and the golden tests were updated to match.
Testing
New metadata fixtures cover attribute extraction for each symbol type. A new generator example renders the attributes to AsciiDoc and HTML.
Documentation
The data model reference section now describes the new attribute fields.