Skip to content

Conversation

alandefreitas
Copy link
Collaborator

@alandefreitas alandefreitas commented Oct 8, 2025

This PR includes several small refactors and fixes that:

  • make previously invalid constructs that caused recurring bugs impossible
  • fix foundational blockers that were blocking many new features
  • fix bugs found along the way while implementing these changes

This is a complete list of the changes:

  • Library:
    • Support recursive inline elements in documentation
    • Optional nullable traits
    • Optional references replace std::reference_wrapper
    • Expected references replace std::reference_wrapper
  • Fix:
    • Do not attempt to format logs without parameters (a segmentation fault for some strings)
  • Refactors:
    • Polymorphic is a value-type (using the optional nullable traits)
    • Enforce non-optional polymorphic types instead of unused Optionals
    • Use a consistent pattern for families of metadata base and derived classes
    • Use mrdocs::Optional throughout the public API
    • Info contains SourceInfo as composition
  • Build:
  • Tests:
    • Test references to parent contexts (boost.openmethod issue)
  • Doc:
    • Minor improvement to style guide
  • Style:
    • Version printer mimics clang style
    • show-enum-constants option (analogous to show namespaces)
    • Location objects include column number
    • Render source line and caret in warning output

Commits include longer bodies explaining the rationale. One commit led to another as these problems were blocking each other. However, they should simplify other PRs from now on.

This commit adjusts the Polymorphic class so that it becomes a value-type that matches the standard std::polymorphic. It also implements an implementation detail shared with Optional, allowing the private nullable state to be used for storage optimization.

The semantics become less convenient in the case of optional polymorphic objects, but the application becomes much safer because non-nullable objects are never in an invalid state. After adapting all objects, most polymorphic objects are, in fact, not nullable.

As future work, we can define a custom type for nullable polymorphic objects that has simpler semantics.
Split base and derived classes into their own headers to remove circular dependencies in the code.
Since an EnumConstant is a regular information type according to the Clang AST, it supports all Javadoc features. For consistency, we create an option analogous to 'show-namespaces' for 'enum-constants', so the user has the ability to render their own doc comments instead of discarding them. The logic is also important because it opens the possibility of conditionally rendering enum constants.
@cppalliance-bot
Copy link

cppalliance-bot commented Oct 8, 2025

An automated preview of the documentation is available at https://1063.mrdocs.prtest2.cppalliance.org/index.html

If more commits are pushed to the pull request, the docs will rebuild at the same URL.

2025-10-09 23:10:42 UTC

@alandefreitas alandefreitas force-pushed the develop branch 2 times, most recently from 2b15f39 to c7532ff Compare October 9, 2025 00:52
@mizvekov
Copy link
Collaborator

mizvekov commented Oct 9, 2025

This PR is so huge there is no way it can even be reviewed through the web interface, it freezes GitHub :)

At least the Lua removal should be separable, without much hassle, I can't see why not, and that would be the biggest chunk.

@alandefreitas
Copy link
Collaborator Author

This PR is so huge there is no way it can even be reviewed through the web interface, it freezes GitHub :)
At least the Lua removal should be separable, without much hassle, I can't see why not, and that would be the biggest chunk.

Yes. One thing led to another because the PR is dealing with blockers. But everything, including the Lua removal, is in separate commits.

For instance, you can see the Lua commit in the UI here: 3a9b125

Although there's not much there. It's mostly, well, the removal.

@K-ballo
Copy link
Collaborator

K-ballo commented Oct 9, 2025

Please move the clang-formatting changes into a separate PR as well.

@alandefreitas
Copy link
Collaborator Author

alandefreitas commented Oct 9, 2025

Please move the clang-formatting changes into a separate PR as well.

There are no pure clang format changes in any commit and we hope there will never be any.

Please have a quick look at the comments and documentation in 149190f

Many people at the alliance hate clang format and these formatting commits is one of reasons. That’s why I put it here solely as a reference / starting point. Anything else would be too controversial for now.

@K-ballo
Copy link
Collaborator

K-ballo commented Oct 9, 2025

There are no pure clang format changes in any PR and we hope there will never be any.

Agreed.

That’s why I put it here solely as a reference / starting point.

Please move the clang formatting changes to a separate PR so we can have that discussion without blocking the rest of the changes here.

@alandefreitas
Copy link
Collaborator Author

Please move the clang formatting changes

Well, I said “There are no pure clang format changes” in the very message before. So I’ll assume you mean “the commit that introduces the clang format configuration file” by “clang formatting changes” and cherry pick that into another PR. Let me know if you meant something different.

Use mrdocs::Optional throughout the public API for consistency and extensibility, including the optional reference return types.
Info objects contain Source information rather than _being_ source information.
Support Optional for references and replace usages of Optional with reference wrappers in the codebase.
Support Expected for references and replace usages of Expected with reference wrappers in the codebase. As with Optional references, expected references provide a safer interface that always rebinds values. Syntax is also more convenient both to declare variables without std::reference_wrapper and to access values without requiring the get function.
This leads to segmentation faults when the string contains reserved sequences.
Warnings now include a snippet of the source file similar to GCC and Clang diagnostics. Each warning prints the file path, line, and column, followed by the line of source text
and a caret marker pointing to the column. This makes it easier to identify and fix issues directly in context without having to open the file manually.
Ensure optional polymorphic types are only used for members that are really optional in terms of application logic. Otherwise, we only make them polymorphic objects that cannot be empty and give them reasonable defaults among the derived types in that type family.
Only use MRDOCS_ASSERT for valueless_after_move(). Since polymorphic objects are no longer nullable, and using moved-from objects is invalid, we should never check valueless_after_move() to determine conditional application logic.
This ensures there are no disengaged versions of value-types
This pattern defines a single source of truth for all derived types in a family of classes. Thus, conversions with the `as_` and `is_` functions are limited to valid derived classes.
Apply a consistent pattern for the class families in the metadata. Class names match their name in all places (such as Symbol instead of Info, and Type instead of TypeInfo), and the clang namespace is described explicitly to avoid name conflicts. The Javadoc hierarchy for blocks and inlines is also well defined.
@alandefreitas alandefreitas force-pushed the develop branch 5 times, most recently from daa526e to 294fc0c Compare October 9, 2025 22:57
@alandefreitas
Copy link
Collaborator Author

@mizvekov The Lua as dependency commit is now here. However, I'll split this PR into smaller PRs over the following days, and the Lua commit will be on top of it all. If you need it for your own PR before I've done that, you can still cherry-pick it on top of your branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants