mosra
Regression with class template specialization matching in Doxygen 1.9.1
Following assertion triggers in recent builds of DARMA-tasking/vt when using the latest version of m.css (699abdd5):
https://github.com/mosra/m.css/blob/699abdd57242d33291a0ea4ee333c7d4de11a30c/documentation/doxygen.py#L2600
Traceback with additional details:
diff --git a/documentation/doxygen.py b/documentation/doxygen.py
index d3ef69eb..ae214cc6 100755
--- a/documentation/doxygen.py
+++ b/documentation/doxygen.py
@@ -2597,7 +2597,7 @@ def postprocess_state(state: State):
prefix = state.compounds[compound.parent].name + '::'
- assert compound.name.startswith(prefix)
+ assert compound.name.startswith(prefix), f"compound.name: {compound.name}\nprefix: {prefix}"
compound.leaf_name = compound.name[len(prefix):]
Traceback (most recent call last):
File "/build/vt/./m.css/documentation/doxygen.py", line 4305, in <module>
run(state, templates=os.path.abspath(args.templates), wildcard=args.wildcard, index_pages=args.index_pages, search_merge_subtrees=not args.search_no_subtree_merging, search_add_lookahead_barriers=not args.search_no_lookahead_barriers, search_merge_prefixes=not args.search_no_prefix_merging)
File "/build/vt/./m.css/documentation/doxygen.py", line 4128, in run
postprocess_state(state)
File "/build/vt/./m.css/documentation/doxygen.py", line 2600, in postprocess_state
assert compound.name.startswith(prefix), f"compound.name: {compound.name}\nprefix: {prefix}"
AssertionError: compound.name: vt::rdma::Handle<T, E, IndexT, typename std::enable_if_t<not std::is_same<IndexT, vt::NodeType>::value>>::IndexTagType
prefix: vt::rdma::Handle::
Proposed fix:
diff --git a/documentation/doxygen.py b/documentation/doxygen.py
index d3ef69eb..42378197 100755
--- a/documentation/doxygen.py
+++ b/documentation/doxygen.py
@@ -2597,7 +2597,7 @@ def postprocess_state(state: State):
prefix = state.compounds[compound.parent].name + '::'
- assert compound.name.startswith(prefix)
+ assert compound.name.startswith(state.compounds[compound.parent].name)
compound.leaf_name = compound.name[len(prefix):]
This seems to be enough to get our build passing, not sure if any caveats apply.
Hi, this actually seems like a regression in Doxygen 1.9.1 (or possibly other versions around that one). I managed to strip it down to just the following:
template<class T> struct Outer;
template<class T> struct Outer<
std::enable_if_t<std::is_same<T, int>::value>
> {
struct FirstInner {};
};
template<class T> struct Outer<
std::enable_if_t<!std::is_same<T, int>::value>
> {
struct SecondInner {};
};
On both 1.8.16 and 1.12, this produces a correct output, while on 1.9.1 this leads to the assertion you're encountering, and additionally Doxygen itself complains that
File.h:28: warning: Internal inconsistency: scope for class Outer< std::enable_if_t< std::is_same< T, int >::value >>::FirstInner not found!
File.h:35: warning: Internal inconsistency: scope for class Outer< std::enable_if_t<!std::is_same< T, int >::value >>::SecondInner not found!
Those warnings then lead to an unspecialized Outer base class being created, to which both FirstInner and SecondInner get inserted as members. Which is obviously incorrect. The same can be seen in your generated docs, screenshot for archival purposes:
Both the IndexTagType member from handle.index.h and the NodeTagType member from handle.node.h get listed, as if both specializations were a single class, and their docs get merged as well. The sanity check assertion is then just an inevitable result from this -- it gets a member listed in a scope in which it shouldn't be at all.
Your fix attempt then makes the build passing, yes, but the parent name doesn't get correctly stripped -- it should list just IndexTagType there, not <T, E, IndexT, typename std::enable_if_t<not std::is_same<IndexT, vt::NodeType>::value>>::IndexTagType, and the two types should be in two different specializations anyway. Not sure what to do here, besides expanding the assertion with some more reasonable message, or ignoring the type that got inserted to a wrong place -- there's not really anything it could do to "unbreak" the input.
Funnily enough, if I change my stripped-down case to the following (i.e., using the C++11-style enable_if, not C++17-style enable_if_t), it works on 1.9.1. So maybe that's what could be a workaround on your end, apart from trying to use some other Doxygen version?
template<class T> struct Outer;
template<class T> struct Outer<
typename std::enable_if<std::is_same<T, int>::value>::type
> {
struct FirstInner {};
};
template<class T> struct Outer<
typename std::enable_if<!std::is_same<T, int>::value>::type
> {
/** @brief Foo */
struct SecondInner {};
};
Hi, this actually seems like a regression in Doxygen 1.9.1 (or possibly other versions around that one). I managed to strip it down to just the following (...)
Thanks a lot for the detailed (!) response. I was suspecting Doxygen to be the culprit, but I didn't have the patience to track it down properly.
On both 1.8.16 and 1.12, this produces a correct output, while on 1.9.1 this leads to the assertion you're encountering (...)
Alright, if it works with 1.12 then I guess there's no point in reporting this in Doxygen. I will try to adapt our documentation workflow to use the latest version (maybe the binary release will work, since Ubuntu is using 1.9.8).
Please see #215 -- I personally had to revert back to 1.8.16 because it's far from great. On the m.css side all the workarounds for 1.12 are in place so it shouldn't wildly assert and such, but there are a few serious regressions for which I so far wasn't able to figure out a workaround nor a patch. I hope to get back to that before the maintainers publish a new release (which they often do during summer holidays or christmas), but it's an uphill battle. #239 might also affect you.
If the above-suggested workaround with typename std::enable_if works, then I think that's the best way forward. If not, then I'd suggest trying with 1.9.4, because 1.9.5+ forward contains the massive regressions listed in #215. Doxygen binaries from the official website should work as long as you aren't on a distro that's too old, IIRC 18.04 / 20.04 cannot run the new ones because a glibc mismatch. (This is what I do here on the CI.)
In any case, I'll add a better assertion message for this, just wanted to do some bisecting to know what all versions are affected.
Please see #215 -- I personally had to revert back to 1.8.16 because it's far from great. On the m.css side all the workarounds for 1.12 are in place so it shouldn't wildly assert and such, but there are a few serious regressions for which I so far wasn't able to figure out a workaround nor a patch. I hope to get back to that before the maintainers publish a new release (which they often do during summer holidays or christmas), but it's an uphill battle. #239 might also affect you.
If the above-suggested workaround with
typename std::enable_ifworks, then I think that's the best way forward. If not, then I'd suggest trying with 1.9.4, because 1.9.5+ forward contains the massive regressions listed in #215. Doxygen binaries from the official website should work as long as you aren't on a distro that's too old, IIRC 18.04 / 20.04 cannot run the new ones because a glibc mismatch. (This is what I do here on the CI.)In any case, I'll add a better assertion message for this, just wanted to do some bisecting to know what all versions are affected.
Just FYI I have opted to downgrade Doxygen to 1.8.16 to keep in sync with the CI here. The generated docs seem to be correct now. +1 for improving the assertion message.