New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a new EXTRACT_PRIVATE_VIRTUAL option #6729

Merged
merged 1 commit into from Jan 8, 2019

Conversation

Projects
None yet
3 participants
@mosra
Copy link
Contributor

mosra commented Jan 2, 2019

The classic article about Virtuality from Herb Sutter suggests that virtual functions are always made private and public class interface is never virtual. Until now, it was not really possible to document these functions in Doxygen:

  • Enabling EXTRACT_PRIVATE would show all internals, not just virtual functions, which is not wanted.
  • Enabling HIDE_UNDOC_MEMBERS and HIDE_UNDOC_CLASSES would effectively disable warnings about all undocumented members, which is not wanted.

The usual workaround was to put the members into protected scope just for Doxygen:

    #ifdef DOXYGEN_GENERATING_OUTPUT
    protected:
    #else
    private:
    #endif
        /** @brief Documented private virtual function */
        virtual doStuff();

The new EXTRACT_PRIVATE_VIRTUAL option makes these visible (and able to be linked to), but shows them only if they are documented. Real-world use of this feature can be seen for example in the Magnum docs:

image

I'm mainly using the XML output, for which private functions were already included (even though unconditionally, see #2405 and #4504), but until now it was not possible to @ref them. That's now fixed and a new test is added to verify that. I numbered it 81, assuming you'd want to merge this after my previous PR, #6722 (which has a test no. 80). Besides the XML output I verified that the private virtuals are shown also in the HTML output (using the same test file), but since I'm not primarily using it, it may happen that I missed some cases.

This feature was requested back in 2005 in #1910, I assume merging this would make that feature request fullfilled :) It's maybe also related to #2187.

Thanks in advance for merging!

@mosra mosra force-pushed the mosra:extract-private-virtual branch from 07bc9d8 to 354b6be Jan 2, 2019

@albert-github

This comment has been minimized.

Copy link
Collaborator

albert-github commented Jan 3, 2019

In the config.xml you use *documented*, his is the first place (to the best of my knowledge) of the usage of *...* in the config.xml file. A consequence is that it is shown in the documentation in italic. In the Doxyfile and in the help of the Doxywizard it is shown as *...* so I think we should abstain from it or otherwise adjust the script (configgen.py) so it can also handle the *.

@mosra

This comment has been minimized.

Copy link
Contributor

mosra commented Jan 3, 2019

@albert-github Ah, thanks for pointing that out, Didn't know about this restriction, sorry.

I wanted to emphasise the word, yes, because I think it's important. If I use \em instead, will it behave correctly also in Doxywizard? Or should I rather just not use it at all?

@albert-github

This comment has been minimized.

Copy link
Collaborator

albert-github commented Jan 3, 2019

I didn't test but looked quickly in the configgen.py and I thin the \em will not be handled correctly, the \e will though (and should give the same effect. I would not use the effect at all.

Implement a new EXTRACT_PRIVATE_VIRTUAL option.
The classic article about virtuality from Herb Sutter [1] suggests that
virtual functions are always private and public class interface is never
virtual. Until now, it was not really possible to document these
functions in Doxygen:

 * Enabling EXTRACT_PRIVATE would show all internals, not just virtual
   functions, which is not wanted.
 * Enabling HIDE_UNDOC_MEMBERS and HIDE_UNDOC_CLASSES would effectively
   disable warnings about *all* undocumented members, which is not wanted.

The usual workaround was to put the members into protected scope just
for Doxygen:

    #ifdef DOXYGEN_GENERATING_OUTPUT
    protected:
    #else
    private:
    #endif
        /** @brief Documented private virtual function */
        virtual doStuff();

The new EXTRACT_PRIVATE_VIRTUAL option makes these visible (and able to
be linked to), but shows them *only* if they are documented.

[1] http://www.gotw.ca/publications/mill18.htm

@mosra mosra force-pushed the mosra:extract-private-virtual branch from 354b6be to d18b3ea Jan 3, 2019

@mosra

This comment has been minimized.

Copy link
Contributor

mosra commented Jan 3, 2019

Okay, removed it, then :)

@doxygen doxygen merged commit d18b3ea into doxygen:master Jan 8, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@doxygen

This comment has been minimized.

Copy link
Owner

doxygen commented Jan 8, 2019

Note that I shortened the option name to EXTRACT_PRIV_VIRTUAL to make doxygen -s -g produce a nicely aligned list of options with at least one space before the =.

@mosra

This comment has been minimized.

Copy link
Contributor

mosra commented Jan 8, 2019

Thank you!

@mosra mosra deleted the mosra:extract-private-virtual branch Jan 8, 2019

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