adobe / hyde

A front-end to Jekyll that parses C++ sources to produce and enforce out-of-line documentation
http://opensource.adobe.com/hyde/
MIT License
323 stars 41 forks source link

Classes split over multiple files #19

Open mrzv opened 5 years ago

mrzv commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 4 years ago

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 commented 4 years ago

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.