mrdocs icon indicating copy to clipboard operation
mrdocs copied to clipboard

Inconsistent handling of macro calls in SFINAE conversion to requirements.

Open jll63 opened this issue 5 months ago • 6 comments

In:

template<
        class Other,
        typename = std::enable_if_t<
            BOOST_OPENMETHOD_DETAIL_UNLESS_MRDOCS(detail::)
                SameSmartPtr<SmartPtr, Other, Registry> &&
            BOOST_OPENMETHOD_DETAIL_UNLESS_MRDOCS(detail::)
                IsPolymorphic<element_type, Registry> &&
            std::is_constructible_v<SmartPtr, Other&&>>>
    virtual_ptr(Other&& other)

In this context, BOOST_OPENMETHOD_DETAIL_UNLESS_MRDOCS(detail::) expands to nothing.

Out:

template<class Other>
requires SameSmartPtr<SmartPtr, Other, Registry> &&
            BOOST_OPENMETHOD_DETAIL_UNLESS_MRDOCS(detail::)
                IsPolymorphic<element_type, Registry> &&
            std::is_constructible_v<SmartPtr, Other&&>
virtual_ptr(Other&& other);

jll63 avatar Sep 14 '25 13:09 jll63

I tried changing the macro function to a symbol, same problem:

#define BOOST_OPENMETHOD_DETAIL_UNLESS_MRDOCS

    template<
        class Other,
        typename = std::enable_if_t<
            BOOST_OPENMETHOD_DETAIL_UNLESS_MRDOCS
                SameSmartPtr<SmartPtr, Other, Registry> &&
            BOOST_OPENMETHOD_DETAIL_UNLESS_MRDOCS
                IsPolymorphic<typename Other::element_type, Registry> &&
            std::is_constructible_v<SmartPtr, const Other&>>>
    virtual_ptr(const Other& other)

Out:

template<class Other>
requires SameSmartPtr<SmartPtr, Other, Registry> &&
            BOOST_OPENMETHOD_DETAIL_UNLESS_MRDOCS
                IsPolymorphic<typename Other::element_type, Registry> &&
            std::is_constructible_v<SmartPtr, const Other&>
virtual_ptr(Other const& other);

jll63 avatar Sep 14 '25 13:09 jll63

Any chance this can be addressed before the deadline for new libraries?

jll63 avatar Sep 27 '25 15:09 jll63

MrDocs can't extract and manipulate expression trees yet. This would be an extremely valuable feature that we're currently unable to implement. The best solution now would be to come up with prettier types to use there. For instance, you can hide everything behind a "see-below" concept or trait or a regular trait that uses some other implementation-defined concepts or traits. This is useful because the code will still be correct, you can hide details, and there's a name to refer to this concept in the public API instead of just hiding part of what's being evaluated.

alandefreitas avatar Sep 28 '25 21:09 alandefreitas

It does "process" one of the two occurrences of the macro though...That's what I find strange.

jll63 avatar Sep 28 '25 21:09 jll63

For instance, you can hide everything behind a "see-below" concept or trait or a regular trait that uses some other implementation-defined concepts or traits. This is useful because the code will still be correct, you can hide details, and there's a name to refer to this concept in the public API instead of just hiding part of what's being evaluated.

IsPolymorphic used to be detail::is_polymorphic. However, I really like how MrDocs translates C++17 SFINAE to C++20 requirements. I feel it makes it easier for me to write the doc in those areas.

I can see other options...

I could change IsPolymorphic back to is_polymorphic, but not in namespace detail, as a public concept. During review, I got some comments about (maybe) exposing to many internals. That's why I made it exposition-only. But MrDocs gives me away :-D as it shows which header defines it.

Or I could put the detail:: inside an #ifdef __MRDOCS__, probably that would to the trick.

[later]

Well it doesn't :-O

    template<
        class Other,
        typename = std::enable_if_t<
#ifndef __MRDOCS__
            detail::
#endif
                SameSmartPtr<SmartPtr, Other, Registry> &&
#ifndef __MRDOCS__
            detail::
#endif
                IsPolymorphic<typename Other::element_type, Registry> &&
            std::is_constructible_v<SmartPtr, const Other&>>>
    virtual_ptr(const Other& other)
template<class Other>
requires SameSmartPtr<SmartPtr, Other, Registry> &&
#ifndef __MRDOCS__
            detail::
#endif
                IsPolymorphic<typename Other::element_type, Registry> &&
            std::is_constructible_v<SmartPtr, const Other&>
virtual_ptr(Other const& other);

jll63 avatar Sep 28 '25 21:09 jll63

I agree with everything you just said.

exposing to many internals

On this point, you don't necessarily need to include it in the public API. For instance, it could be part of some "exposition_only" namespace, or an implementation detail in another public concept, a "see below" symbol, or something similar. These things are not public, so it should be considered as exposing too many internals.

Even when MrDocs is able to evaluate expressions, this problems wouldn't go away because it's very hard to come up with something decent. For instance, if mrdocs just replaced all subexpressions with the typical algorithm, we could get something like:

template<class Other>
requires /* implementation-defined */ &&
               /* implementation-defined */ &&
               std::is_constructible_v<SmartPtr, Other&&>
virtual_ptr(Other&& other);

which is much worse than constraints in an exposition only namespace (which is what cppreference does, by the way).

Of course, we could say what we need is to evaluate these implementation defined expressions, but at this point we would be implementing a whole interpreter for a superset of C++ just to solve this problem.

alandefreitas avatar Sep 29 '25 07:09 alandefreitas