Skip to content

Commit fda3cfb

Browse files
coadometa-codesync[bot]
coado
authored and
meta-codesync[bot]
committed
Fix Doxygen merging base classes from primary templates into specializations (#56239)
Summary: Pull Request resolved: #56239 Doxygen incorrectly merges base classes from primary templates into their partial specializations. In C++, a specialization's inheritance list completely replaces the primary template's, but Doxygen combines both into a single basecompoundref list. - **Contradictory type traits** — is_optional<std::optional<T>> and is_variant_of_data_types<std::variant<Ts...>> showed inheritance from both std::false_type (primary) and std::true_type (specialization) - **Duplicate base classes** — Converter<jsi::Object> listed ConverterBase<jsi::Object> twice (once from the primary template after substitution, once from the specialization) This diff fixes both issues with two complementary mechanisms: - **Dedup-by-name (Extendable._deduplicate_base_classes)**: removes exact duplicate base classes, keeping the last occurrence. Handles cases where Doxygen's template argument substitution produces identical names. - **Primary template base subtraction (StructLikeScopeKind._remove_merged_primary_bases)**: for partial specializations, looks up the primary template among sibling scopes and performs count-based subtraction of its bases. Count-based (rather than set-based) subtraction correctly preserves bases that a specialization explicitly re-inherits from the same class as the primary. Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D98291360
1 parent 8fa7cb8 commit fda3cfb

File tree

10 files changed

+123
-18
lines changed

10 files changed

+123
-18
lines changed

‎scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10875,7 +10875,7 @@ struct facebook::react::detail::merge_data_types<std::variant<AllowedTypesT...>>
1087510875
}
1087610876

1087710877
template <CSSDataType... Ts>
10878-
struct facebook::react::detail::is_variant_of_data_types<std::variant<Ts...>> : public std::false_type, public std::true_type {
10878+
struct facebook::react::detail::is_variant_of_data_types<std::variant<Ts...>> : public std::true_type {
1087910879
}
1088010880

1088110881
template <CSSDataType... AlllowedTypes1T, CSSDataType... AlllowedTypes2T, CSSMaybeCompoundDataType... RestT>
@@ -12472,13 +12472,13 @@ struct facebook::react::bridging::is_optional : public std::false_type {
1247212472
}
1247312473

1247412474
template <typename T>
12475-
struct facebook::react::bridging::Converter<std::optional<T>> : public facebook::react::bridging::ConverterBase<T>, public facebook::react::bridging::ConverterBase<facebook::jsi::Value> {
12475+
struct facebook::react::bridging::Converter<std::optional<T>> : public facebook::react::bridging::ConverterBase<facebook::jsi::Value> {
1247612476
public Converter(facebook::jsi::Runtime& rt, std::optional<T> value);
1247712477
public operator std::optional<T>();
1247812478
}
1247912479

1248012480
template <typename T>
12481-
struct facebook::react::bridging::is_optional<std::optional<T>> : public std::false_type, public std::true_type {
12481+
struct facebook::react::bridging::is_optional<std::optional<T>> : public std::true_type {
1248212482
}
1248312483

1248412484
struct facebook::react::bridging::Converter<facebook::jsi::Object> : public facebook::react::bridging::ConverterBase<facebook::jsi::Object> {

‎scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10702,7 +10702,7 @@ struct facebook::react::detail::merge_data_types<std::variant<AllowedTypesT...>>
1070210702
}
1070310703

1070410704
template <CSSDataType... Ts>
10705-
struct facebook::react::detail::is_variant_of_data_types<std::variant<Ts...>> : public std::false_type, public std::true_type {
10705+
struct facebook::react::detail::is_variant_of_data_types<std::variant<Ts...>> : public std::true_type {
1070610706
}
1070710707

1070810708
template <CSSDataType... AlllowedTypes1T, CSSDataType... AlllowedTypes2T, CSSMaybeCompoundDataType... RestT>
@@ -12299,13 +12299,13 @@ struct facebook::react::bridging::is_optional : public std::false_type {
1229912299
}
1230012300

1230112301
template <typename T>
12302-
struct facebook::react::bridging::Converter<std::optional<T>> : public facebook::react::bridging::ConverterBase<T>, public facebook::react::bridging::ConverterBase<facebook::jsi::Value> {
12302+
struct facebook::react::bridging::Converter<std::optional<T>> : public facebook::react::bridging::ConverterBase<facebook::jsi::Value> {
1230312303
public Converter(facebook::jsi::Runtime& rt, std::optional<T> value);
1230412304
public operator std::optional<T>();
1230512305
}
1230612306

1230712307
template <typename T>
12308-
struct facebook::react::bridging::is_optional<std::optional<T>> : public std::false_type, public std::true_type {
12308+
struct facebook::react::bridging::is_optional<std::optional<T>> : public std::true_type {
1230912309
}
1231012310

1231112311
struct facebook::react::bridging::Converter<facebook::jsi::Object> : public facebook::react::bridging::ConverterBase<facebook::jsi::Object> {

‎scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13328,7 +13328,7 @@ struct facebook::react::detail::merge_data_types<std::variant<AllowedTypesT...>>
1332813328
}
1332913329

1333013330
template <CSSDataType... Ts>
13331-
struct facebook::react::detail::is_variant_of_data_types<std::variant<Ts...>> : public std::false_type, public std::true_type {
13331+
struct facebook::react::detail::is_variant_of_data_types<std::variant<Ts...>> : public std::true_type {
1333213332
}
1333313333

1333413334
template <CSSDataType... AlllowedTypes1T, CSSDataType... AlllowedTypes2T, CSSMaybeCompoundDataType... RestT>
@@ -14860,13 +14860,13 @@ struct facebook::react::bridging::is_optional : public std::false_type {
1486014860
}
1486114861

1486214862
template <typename T>
14863-
struct facebook::react::bridging::Converter<std::optional<T>> : public facebook::react::bridging::ConverterBase<T>, public facebook::react::bridging::ConverterBase<facebook::jsi::Value> {
14863+
struct facebook::react::bridging::Converter<std::optional<T>> : public facebook::react::bridging::ConverterBase<facebook::jsi::Value> {
1486414864
public Converter(facebook::jsi::Runtime& rt, std::optional<T> value);
1486514865
public operator std::optional<T>();
1486614866
}
1486714867

1486814868
template <typename T>
14869-
struct facebook::react::bridging::is_optional<std::optional<T>> : public std::false_type, public std::true_type {
14869+
struct facebook::react::bridging::is_optional<std::optional<T>> : public std::true_type {
1487014870
}
1487114871

1487214872
struct facebook::react::bridging::Converter<facebook::jsi::Object> : public facebook::react::bridging::ConverterBase<facebook::jsi::Object> {

‎scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13155,7 +13155,7 @@ struct facebook::react::detail::merge_data_types<std::variant<AllowedTypesT...>>
1315513155
}
1315613156

1315713157
template <CSSDataType... Ts>
13158-
struct facebook::react::detail::is_variant_of_data_types<std::variant<Ts...>> : public std::false_type, public std::true_type {
13158+
struct facebook::react::detail::is_variant_of_data_types<std::variant<Ts...>> : public std::true_type {
1315913159
}
1316013160

1316113161
template <CSSDataType... AlllowedTypes1T, CSSDataType... AlllowedTypes2T, CSSMaybeCompoundDataType... RestT>
@@ -14687,13 +14687,13 @@ struct facebook::react::bridging::is_optional : public std::false_type {
1468714687
}
1468814688

1468914689
template <typename T>
14690-
struct facebook::react::bridging::Converter<std::optional<T>> : public facebook::react::bridging::ConverterBase<T>, public facebook::react::bridging::ConverterBase<facebook::jsi::Value> {
14690+
struct facebook::react::bridging::Converter<std::optional<T>> : public facebook::react::bridging::ConverterBase<facebook::jsi::Value> {
1469114691
public Converter(facebook::jsi::Runtime& rt, std::optional<T> value);
1469214692
public operator std::optional<T>();
1469314693
}
1469414694

1469514695
template <typename T>
14696-
struct facebook::react::bridging::is_optional<std::optional<T>> : public std::false_type, public std::true_type {
14696+
struct facebook::react::bridging::is_optional<std::optional<T>> : public std::true_type {
1469714697
}
1469814698

1469914699
struct facebook::react::bridging::Converter<facebook::jsi::Object> : public facebook::react::bridging::ConverterBase<facebook::jsi::Object> {

‎scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7628,7 +7628,7 @@ struct facebook::react::detail::merge_data_types<std::variant<AllowedTypesT...>>
76287628
}
76297629

76307630
template <CSSDataType... Ts>
7631-
struct facebook::react::detail::is_variant_of_data_types<std::variant<Ts...>> : public std::false_type, public std::true_type {
7631+
struct facebook::react::detail::is_variant_of_data_types<std::variant<Ts...>> : public std::true_type {
76327632
}
76337633

76347634
template <CSSDataType... AlllowedTypes1T, CSSDataType... AlllowedTypes2T, CSSMaybeCompoundDataType... RestT>
@@ -9153,13 +9153,13 @@ struct facebook::react::bridging::is_optional : public std::false_type {
91539153
}
91549154

91559155
template <typename T>
9156-
struct facebook::react::bridging::Converter<std::optional<T>> : public facebook::react::bridging::ConverterBase<T>, public facebook::react::bridging::ConverterBase<facebook::jsi::Value> {
9156+
struct facebook::react::bridging::Converter<std::optional<T>> : public facebook::react::bridging::ConverterBase<facebook::jsi::Value> {
91579157
public Converter(facebook::jsi::Runtime& rt, std::optional<T> value);
91589158
public operator std::optional<T>();
91599159
}
91609160

91619161
template <typename T>
9162-
struct facebook::react::bridging::is_optional<std::optional<T>> : public std::false_type, public std::true_type {
9162+
struct facebook::react::bridging::is_optional<std::optional<T>> : public std::true_type {
91639163
}
91649164

91659165
struct facebook::react::bridging::Converter<facebook::jsi::Object> : public facebook::react::bridging::ConverterBase<facebook::jsi::Object> {

‎scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7619,7 +7619,7 @@ struct facebook::react::detail::merge_data_types<std::variant<AllowedTypesT...>>
76197619
}
76207620

76217621
template <CSSDataType... Ts>
7622-
struct facebook::react::detail::is_variant_of_data_types<std::variant<Ts...>> : public std::false_type, public std::true_type {
7622+
struct facebook::react::detail::is_variant_of_data_types<std::variant<Ts...>> : public std::true_type {
76237623
}
76247624

76257625
template <CSSDataType... AlllowedTypes1T, CSSDataType... AlllowedTypes2T, CSSMaybeCompoundDataType... RestT>
@@ -9144,13 +9144,13 @@ struct facebook::react::bridging::is_optional : public std::false_type {
91449144
}
91459145

91469146
template <typename T>
9147-
struct facebook::react::bridging::Converter<std::optional<T>> : public facebook::react::bridging::ConverterBase<T>, public facebook::react::bridging::ConverterBase<facebook::jsi::Value> {
9147+
struct facebook::react::bridging::Converter<std::optional<T>> : public facebook::react::bridging::ConverterBase<facebook::jsi::Value> {
91489148
public Converter(facebook::jsi::Runtime& rt, std::optional<T> value);
91499149
public operator std::optional<T>();
91509150
}
91519151

91529152
template <typename T>
9153-
struct facebook::react::bridging::is_optional<std::optional<T>> : public std::false_type, public std::true_type {
9153+
struct facebook::react::bridging::is_optional<std::optional<T>> : public std::true_type {
91549154
}
91559155

91569156
struct facebook::react::bridging::Converter<facebook::jsi::Object> : public facebook::react::bridging::ConverterBase<facebook::jsi::Object> {

‎scripts/cxx-api/parser/scope/extendable.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,20 @@ def add_base(self, base: Base | list[Base]) -> None:
2525
self.base_classes.append(b)
2626
else:
2727
self.base_classes.append(base)
28+
self._deduplicate_base_classes()
29+
30+
def _deduplicate_base_classes(self) -> None:
31+
"""Remove duplicate base classes.
32+
33+
Doxygen sometimes reports the same base class multiple times (e.g.
34+
when template argument substitution produces identical names for
35+
a primary template and its specialization). This keeps only the
36+
last occurrence of each name.
37+
"""
38+
seen: dict[str, int] = {}
39+
for i, base in enumerate(self.base_classes):
40+
seen[base.name] = i
41+
self.base_classes = [self.base_classes[i] for i in sorted(seen.values())]
2842

2943
def qualify_base_classes(self, scope) -> None:
3044
"""Qualify base class names and their template arguments."""

‎scripts/cxx-api/parser/scope/struct_like_scope_kind.py‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,48 @@ def add_template(self, template: Template | [Template]) -> None:
4242
self.template_list.add(template)
4343

4444
def close(self, scope: Scope) -> None:
45+
self._remove_merged_primary_bases(scope)
4546
self.qualify_base_classes(scope)
4647

48+
def _remove_merged_primary_bases(self, scope: Scope) -> None:
49+
"""Remove base classes that Doxygen incorrectly merged from the
50+
primary template into a partial specialization.
51+
52+
In C++ a partial specialization's inheritance list completely
53+
replaces the primary template's, but Doxygen merges both lists
54+
into the specialization's ``basecompoundref`` elements.
55+
56+
This method performs count-based subtraction: for each base in
57+
the primary template, one matching occurrence (by name) is removed
58+
from this specialization's base list. Count-based subtraction
59+
correctly preserves bases that the specialization explicitly
60+
re-inherits from the same class as the primary template.
61+
"""
62+
if self.specialization_args is None:
63+
return
64+
if scope.parent_scope is None:
65+
return
66+
67+
for sibling in scope.parent_scope.inner_scopes.values():
68+
if (
69+
sibling is not scope
70+
and sibling.name == scope.name
71+
and isinstance(sibling.kind, StructLikeScopeKind)
72+
and sibling.kind.specialization_args is None
73+
):
74+
primary_base_counts: dict[str, int] = {}
75+
for b in sibling.kind.base_classes:
76+
primary_base_counts[b.name] = primary_base_counts.get(b.name, 0) + 1
77+
78+
result = []
79+
for b in self.base_classes:
80+
if primary_base_counts.get(b.name, 0) > 0:
81+
primary_base_counts[b.name] -= 1
82+
else:
83+
result.append(b)
84+
self.base_classes = result
85+
break
86+
4787
def to_string(self, scope: Scope) -> str:
4888
result = ""
4989

‎scripts/cxx-api/tests/snapshots/should_deduplicate_base_classes/snapshot.api‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
struct test::BaseA {
2+
}
3+
4+
struct test::Derived : public test::BaseA {
5+
}
6+
7+
struct test::false_type {
8+
}
9+
10+
struct test::true_type {
11+
}
12+
13+
template <typename T>
14+
struct test::is_special<T *> : public test::true_type {
15+
}
16+
17+
template <typename>
18+
struct test::is_special : public test::false_type {
19+
}

‎scripts/cxx-api/tests/snapshots/should_deduplicate_base_classes/test.h‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
#pragma once
9+
10+
namespace test {
11+
12+
struct false_type {};
13+
struct true_type {};
14+
15+
// Primary template inherits from false_type.
16+
// Partial specialization inherits from true_type.
17+
// Doxygen may incorrectly merge both base class lists into the
18+
// specialization, producing : false_type, true_type. The parser
19+
// should subtract the primary template's bases and keep only true_type.
20+
21+
template <typename>
22+
struct is_special : public false_type {};
23+
24+
template <typename T>
25+
struct is_special<T *> : public true_type {};
26+
27+
// Duplicate base class via Doxygen merging template-substituted bases.
28+
struct BaseA {};
29+
30+
struct Derived : public BaseA, public BaseA {};
31+
32+
} // namespace test

0 commit comments

Comments
 (0)