feat: add function object support#1163
feat: add function object support#1163gennaroprota wants to merge 2 commits intocppalliance:developfrom
Conversation
✨ Highlights
🧾 Changes by Scope
🔝 Top Files
|
|
An automated preview of the documentation is available at https://1163.mrdocs.prtest2.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-03-06 15:47:12 UTC |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #1163 +/- ##
===========================================
+ Coverage 76.38% 76.69% +0.31%
===========================================
Files 311 314 +3
Lines 29672 30117 +445
Branches 5863 5949 +86
===========================================
+ Hits 22664 23098 +434
+ Misses 4735 4720 -15
- Partials 2273 2299 +26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
19d7451 to
e9b70af
Compare
alandefreitas
left a comment
There was a problem hiding this comment.
Isn't "functors" formal enough to be used here? It would be a much shorter command and a more concise way to describe it over and over again without needing a "_".
The fixtures should go to better directories, though. Maybe the directories need better documentation, but you can see they shouldn't go there by comparing with the siblings (and you'll see it's definitely not "core", which is used for really core functionality that's not optional). Symbol types and their features are tested in the symbols directory for each symbol. And since there's an option involved, every new option should be tested in the options directory. Then we have new features for variables that should be tested. Then we have a new field in variables, and that's not reflected there. Then we have a folder for javadoc commands that should be tested, and we need a test there because we introduced a new command.
The test cases in symbols/variable should also explore what happens when there is non-conflicting type information, but the variable is forced to be treated as a function object anyway. There are real-world patterns like this. Another pattern we see very often is the combination of template parameters and arguments: impl function with template, different variables with the same function but different params, and so on.
In the existing test, I'm not sure I understand the mechanism through which it goes to the list of functions in the interface. I'm not sure if cppalliance does that or shows it as a separate kind of entity. Because they do behave differently. All niebloids also have a special banner describing how that function works differently. Another thing to explore is how all of that works when one is visible, and the other is an implementation detail, and so on. Another thing to test and design is what happens when documentation goes on the other symbol.
In the output of the existing test, all functors look wrong. They look nothing like niebloids in cppreference. Many of them look arbitrary and not like valid C++ syntax at all. For instance, it tends to look like:
bool single_overload();
(not very different from an overload set with the main function name replaced from operator() to the variable name and with const removed).
rather than:
constexpr /* implementation-defined */ single_overload = {};
bool
operator()() const;
One more thing missing is the presentation of this feature on the landing page. The snippets on the landing page are meant to showcase features that only Mrdocs can support, and this is definitely one of them.
Good work here overall, though. This is a big design decision.
|
|
||
| /** Whether this variable is a function object. | ||
|
|
||
| A function object variable is a constexpr variable whose type |
There was a problem hiding this comment.
I know I'm nitpicking, but I don't think it has to be constexpr.
| {{~/if~}} | ||
| {{~#if name ~}} | ||
| {{~#if parent.isFunctionObject ~}} | ||
| () |
There was a problem hiding this comment.
If functions don't have "()" as part of their qualified name, why should function objects have it?
There was a problem hiding this comment.
Hmm, right. I always use "()" after function names, but MrDocs doesn't.
| the variable is documented as a callable, and its type is hidden | ||
| as implementation-defined. | ||
| */ | ||
| bool IsFunctionObject = false; |
There was a problem hiding this comment.
Mmmm... I often got the recommendation to use a single bool IsFunctionObject in the variable, rather than creating a new symbol type for that. That is valid, but it's not anything but simple. For instance,
- How do I know if something is only the "impl" of a function object? Does it also get a boolean?
- And if I don't, how do I remove it from indices in the end?
- And how do we ensure the generator doesn't generate a page for them?
- And since function objects have a completely different way of being presented to the user, will we create a new template with ways of presenting them to the user that look more like functions (signature templates, etc), or will we sprinkle a lot of if/else throughout the templates?
- How do we get the impl object from the variable?
- If we don't, how do we render information from the impl object in the variable page if we have to? That could happen very often.
I'll probably get some of these answers as I review the PR, but I'm leaving them here so I don't lose context.
| auto& | ||
| allMembers(VariableSymbol const& T) | ||
| { | ||
| return T.FunctionObjectOverloads; |
There was a problem hiding this comment.
So the overloads are both members of the impl object and the variable? Is there a mechanism to ensure visitors don't go through them twice?
There was a problem hiding this comment.
Yes. The operator() functions are removed from the record's interface and re-parented under the variable. So visitors won't encounter them twice.
| --}} | ||
| {{~#if (and parent parent.parent)~}} | ||
| {{~> symbol/qualified-name-title parent is-qualified-name-parent=true ~}}{{#if parent.name }}::{{/if}} | ||
| {{~> symbol/qualified-name-title parent is-qualified-name-parent=true ~}}{{#if parent.name }}{{~#unless parent.isFunctionObject~}}::{{~/unless~}}{{/if}} |
There was a problem hiding this comment.
There's something wrong with this pattern. Functions work without it. My intuition and the PR is forcing this template to do something is was not meant to do. qualified names in the standard don't include ()
| // constructor, copy/move assignment operator, or destructor | ||
| // ([special]). | ||
| bool | ||
| isSpecialMemberFunction( |
There was a problem hiding this comment.
Same thing. I just remember doing this somewhere already.
There was a problem hiding this comment.
Aha. I think you mean the code in DocComment/Function.hpp. I see some issues with that:
-
It doesn't handle default constructors with all-default parameters (only no parameters).
-
It doesn't handle by-value copy assignment.
-
It doesn't check for non-template.
I could fix these issues and re-use it. Should we extract those helpers (which are currently in an unnamed namespace) to a shared location? (BTW, unnamed namespaces in include files are bad! :-))
There was a problem hiding this comment.
Yes. I think so. Whatever you think is best, though. You understand the problem much better than I do. :)
| FunctionObjectFinalizer:: | ||
| markImplementationDefined(RecordSymbol& R) | ||
| { | ||
| R.Extraction = ExtractionMode::ImplementationDefined; |
There was a problem hiding this comment.
Oh... This answers one of my questions above. Good strategy :)
| markImplementationDefined(RecordSymbol& R) | ||
| { | ||
| R.Extraction = ExtractionMode::ImplementationDefined; | ||
| for (SymbolID const& childId : allMembers(R.Interface)) |
There was a problem hiding this comment.
What about potentially deeper levels?
There was a problem hiding this comment.
Yup, fixed :-).
| } | ||
|
|
||
| void | ||
| merge(VariableSymbol& I, VariableSymbol&& Other) |
There was a problem hiding this comment.
All these merge functions could use some reflection. That's one of the strongest use cases. I don't know if this should be incremental or in a single issue though.
4c1e267 to
fa5731c
Compare
When MrDocs encounters a variable that is a "function object" (its type
is a record whose only public non-special members are operator()
overloads), it now synthesizes free-function entries named after the
variable. The type is marked implementation-defined and hidden from the
output. Multi-overload function objects are naturally grouped by
OverloadsFinalizer.
For example:
struct abs_fn {
double operator()(double x) const noexcept;
};
constexpr abs_fn abs = {};
is documented as if abs were a free function: double abs(double x).
Detection:
Auto-detection: records with only operator() overloads and special
member functions (constructors, destructor, assignment operators).
Controlled by the auto-function-objects config option (default: true).
Explicit: @functionobject or @functor doc commands on the variable or on
the type, e.g. for types that have extra public members. Template types:
auto-detected only when the type lives in the same scope or an inner
scope (e.g. detail::) relative to the variable. This avoids false
positives from unrelated types like std::hash<T>.
What's generated:
Each operator() overload becomes a synthetic FunctionSymbol with the
variable's name, parented under the variable's parent scope. For
variable templates, the synthetic function inherits the variable's
template parameters. A "NOTE: This function object does not participate
in ADL..." disclaimer is appended to each synthetic function's doc.
FunctionSymbol::FunctionObjectImpl back-references the implementation
type.
An example of this feature is added to the landing page (the abs example
above).
Closes cppalliance#564.
This moves isSpecialMemberFunction() out of FunctionObjectFinalizer's unnamed namespace into Function.hpp/cpp as a shared API, and introduces individual helpers (isDefaultConstructor(), isCopyConstructor(), isMoveConstructor(), isCopyAssignment(), isMoveAssignment())---isSpecialMemberFunction() becomes a composition of these individual helpers. Contextually, this fixes three bugs in the existing detection logic in DocComment/Function.hpp: - Default constructors with all-default parameters were not recognized (only empty parameter lists were handled). - Default constructors with parameter packs were not recognized. - By-value copy assignment operator=(X) was not recognized.
fa5731c to
9ed3493
Compare
alandefreitas
left a comment
There was a problem hiding this comment.
Really nice tests. Everything about this PR is really cool 😎😄
| constexpr CustomFlagCommand customFlagCommands[] = { | ||
| {"functionobject", &DocComment::IsFunctionObject}, | ||
| // For consistency, the name of the command must be "functionobject" | ||
| // and not "function_object; but that's not very readable, so we |
There was a problem hiding this comment.
We don't need to describe what we're not doing. :)
| { | ||
| // Merge the function-object flag before comparing so | ||
| // the flag alone doesn't cause spurious inequality. | ||
| I.IsFunctionObject |= other.IsFunctionObject; |
There was a problem hiding this comment.
We need to replace these merge functions over time :)
| { | ||
| if (llvm::StringRef(cmd->Name) == custom.name) | ||
| { | ||
| jd_.*(custom.flag) = true; |
| doc.emplace(); | ||
| } | ||
| doc::ParagraphBlock para; | ||
| para.children.push_back( |
There was a problem hiding this comment.
I think we should do this at the handlebars template level before we get complaints so the user can customize this to do whatever they'd like with it.
| name() const; | ||
| }; | ||
|
|
||
| /** @functionobject |
There was a problem hiding this comment.
Should we document that the command has no effect when auto-function-object is true?
| operator()() const; | ||
| }; | ||
|
|
||
| /** Variable-level documentation. */ |
There was a problem hiding this comment.
Should we document that any comment on the variable or on the struct gets ignored? Or did I get the logic wrong?
| template_ctor_fn() = default; | ||
|
|
||
| template<class U> | ||
| explicit template_ctor_fn(U); |
There was a problem hiding this comment.
There are many variants of this pattern that use templates at this level. It's a reasonably common pattern to support templates in this case (for instance, all range algorithms). This will probably bite us on capy soon because it's really common.
The other pattern, where the template goes in the struct, is a little rarer and harder to implement. It's also harder to implement because you have to actually instantiate the thing.
It's up to you, but I'd recommend implementing the easier one and opening a "Explore ..." issue for the hard one.
Edit: This description is now obsolete. Please refer to the commit messages for up to date info.
This PR adds detection of constexpr variables whose type is a record providing only operator() overloads, and documents them as callable objects in the Functions tranche. The type of the variable is hidden as implementation-defined, and breadcrumbs render in f() style for readability, but operator() signatures remain fully visible in the variable synopsis and each overload gets its own detail page---preserving the fact that the callable is a function object, not an ordinary function.
Detection triggers:
Auto-detection (on by default, controlled by
auto-function-objects) for records whose only public non-special members are operator() overloads, including class templates when the variable lives in an enclosing scope of the type.The
@function_objectdocumentation command for cases that fail auto-detection (e.g. types with extra members).Key implementation details:
FunctionObjectFinalizer runs before OverloadsFinalizer: hides the type of the variable, restores operator() to Regular extraction, re-parents overloads under the variable, and appends a fixed disclaimer note.
VariableSymbol gains FunctionObjectOverloads and satisfies the SymbolParent concept, so MultiPageVisitor generates detail pages for each operator() overload.
Handlebars templates render linked operator() signatures in the variable synopsis with "» more..." links to the detail pages.
Closes #564.