C++ types v2 #122
C++ types v2 #122
Conversation
Storage classes were arguably already useful in C, but they will become even more so in C++ once we get to classes and we want to be as consistent as possible between the two domains.
We will want to annotate static methods (among other qualifiers) so, for consistency, we will treat C the same way.
C++ syntax requires certain method qualifiers _after_ the argument list. E.g. 'const' methods. We cannot bundle those as part of the return type nor any of the existing fields, so we need to change the Docstring API. Methods are otherwise handled like functions, so we don't need any new Docstring specialization.
These work similar to TYPE_REF, which we also skip so that the comments apply to the FIELD_DECL cursors instead of what precedes them. These cover cases like: /** comment */ A::B<...> foo; /** comment */ T<...> bar;
|
At a glance, looks nicer! And indeed even I should've noticed the access specifier omission for member variables! |
|
I might have some nitpicks here and there, but I'm not sure it's the best use of either of our time to respin this. It's probably easier to continue working on this after it's been merged, and I trust you to stick around. 😉 I found the coverage report in #123 to be useful for reference. In fact, this merge increases the coverage. I also wrote a tool that takes a testcase Anyway, are you happy with this yourself? Good to go? |
|
Oh, on the nitpicks, some of the methods in the tests could use some more parameters. There are very few. But can be added later. |
Btw one of the things I noted was that Sphinx does not display public access specifier even though it's present in rst. Only protected/private. |
I'm planning on it! Though, heads up, I will probably slow down a bit going forward. Everyone liked it and would ultimately prefer it to breathe, but with everything else that I have to do and without a clear decision from the team to make it work, I'm not comfortable enough taking on the risk myself. Besides nuclear fusion is delayed enough as it is xD Instead, I'm planning on wiring up breathe and focus on other things. Once this is a bit more mature, I can attempt a conversion. Especially if breathe / Doxygen become annoying (I give ourselves good chances on that one). Either way, I expect C++ will likely attract a lot more users and contributions than C did. However sad that is to me.
Well, if I mostly added code with ~100% coverage, relative coverage should increase even if uncovered lines didn't change much. Some sort of cognitive bias... :P But it's a cool thing to put a number on. Also, really nice output, I can totally see a few missed cases, though I'd rather do a separate round to trim redundant code paths and add new tests after merging #123. Besides, it's not too bad right now!
That's a neat idea. I often end up using our own docs to demo that, but it's a bit cumbersome.
Yeah, I'm pretty stoked with how it turned out so far and the previous round had bigger holes than this one for sure. That said, I actually found one specifier I didn't take care of:
Yeah, I may have been taking a bit of a white box approach to tests so far.
Yeah, I noticed it. I think putting them there is the right behaviour though in case Sphinx changes. Also not sure if this can be affected by themes, haven't looked. |
Too bad, though not completely unexpected. All thing considered, it would have been a bit of a jump into the unknown, while Doxygen+Breathe is the safe choice. IMO it's over-complicated as a whole, especially for small projects, but then you can actually find support and solutions to the problems you hit.
Fingers crossed! :)
On this one, up to you really!
Agreed. |
Let's merge it! :) |
We already support access specifiers in methods and the C++ specific `constexpr` in functions / methods. Now we add support for the same specifiers and 'mutable' as well to variables / member variables.
Classes and structures share nearly all the semantics, so it's only natural that we can fix both with mostly common code paths.
Scoped enumerators come in two forms: `enum class` and `enum struct`, but they are exactly the same, unlike in the classes vs structs case. Still, Sphinx has a `cpp:enum-class` and a `cpp:enum-struct` directives. Since referencing is not broken in any way --- all enums are referenced with `:cpp:enum:`, scoped or not ---, the only point in looking at the tokens to distinguish between the two forms in the source code is for Sphinx to render `class` or `struct`. That is hardly a good reason to descend at the token level when both mean literally the same, so we use `cpp:enum-class` for all cases of scoped enums.
The `cpp:autoclass` directive is easy to implement and allows one to specify a single method with the `:members:` option. However, that implies documenting the class itself. At some point we'll probably want companion `cpp:method` and `cpp:member` directives that can properly document a single member as (e.g.): .. cpp:function:: A::B(int c) These require handling of namespaces first though.
BrunoMSantos
force-pushed
the
cpp-types-v2
branch
from
6589938
to
a7ee4ca
Compare
This supersedes #120.
Other than the proposed changes, I noticed and fixed some small issues:
through the cracks before, but it's now fixed and reflected in the test cases.
imagine how I missed it, but it was even more flagrant than the c/dtors. Fixed
now. This is by far the biggest difference and is pretty much limited to the commit parser: improve support for C++ specifiers
thingy which made the improvements obvious.
performance: if one, avoid the other.