Skip to content
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

Classes split over multiple files #19

Open
mrzv opened this issue Oct 23, 2018 · 4 comments
Open

Classes split over multiple files #19

mrzv opened this issue Oct 23, 2018 · 4 comments

Comments

@mrzv
Copy link

mrzv commented Oct 23, 2018

I have the following situation (in many different situations):

// a.hpp
class A
{
    class B;

    void g(const B& b);
};

#include "b.hpp"
// b.hpp
class A::B
{
    void g();
};

If I run hyde on a.hpp alone, I don't get any output for B. If I run hyde with with b.hpp alone or passing both a.hpp and b.hpp, I get:

.../hyde/build/in/b.hpp:1:7: error: use of undeclared identifier 'A'
class A::B
      ^
1 error generated.

Is there a way to support this use case?

@fosterbrereton
Copy link
Contributor

fosterbrereton commented Oct 30, 2018

The tool currently works on a per-file basis. The main reason for this is to accurately detect the existence of docs that shouldn't be around anymore (because an API was either moved or deleted.)

If you were to compile a program, b.hpp requires a.hpp be included in order to pass compilation. Since hyde is a compiler, this requirement still holds here.

A couple fixes could address the situation. hyde could be improved to better handle multi-file situations like this. You could also modify b.hpp to properly include its dependencies. Given that the latter has to happen in order for b.hpp to compile as a bona fide program, I'd try that route first.

@mrzv
Copy link
Author

mrzv commented Oct 30, 2018

b.hpp is an implementation detail. It should never be included on its own. I suppose adding #include "a.hpp" to it is harmless, but redundant.

To me this is a bug in hyde. It's a pretty common practice in header-only libraries to split definition and implementation. This is admittedly a somewhat extreme case of that, but it's valid C++. It seems hyde should support it.

As for deleted API, shouldn't it be the other way around? Shouldn't hyde ingest everything in the library and based on that decide whether something has been removed. If a class is simply moved to a different file, the documentation shouldn't change (or be lost).

@X-Ryl669
Copy link

In the PImpl idiom, you want the PImpl to be hidden. As such, B should not be documented (except as an opaque struct/class). If you want your user to actually use B, then make its declaration visible as @fosterbrereton says, it's no more PImpl. Usually, in such idiom, you'll define B in a CPP file (not a header file), since no-one should be able to use/instantiate it except A.

@mrzv
Copy link
Author

mrzv commented Oct 28, 2019

I'm not sure what PImpl idiom has to do with anything, but I'm sure that a documentation tool should not dictate how a user is to organize their code. That would be the proverbial tail wagging the dog.

I very much hope this issue gets fixed somehow. I think hyde's out-of-source design is brilliant, and I'd love to use, but with the current per-file processing, I simply can't.

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

No branches or pull requests

3 participants