Skip to content

fix: inherited members not shown in derived classes#1203

Open
gennaroprota wants to merge 1 commit into
cppalliance:developfrom
gennaroprota:fix/inherited_members_not_shown_in_derived_classes
Open

fix: inherited members not shown in derived classes#1203
gennaroprota wants to merge 1 commit into
cppalliance:developfrom
gennaroprota:fix/inherited_members_not_shown_in_derived_classes

Conversation

@gennaroprota
Copy link
Copy Markdown
Collaborator

Fixes issue #1176.

Members defined in a class template base were silently dropped from derived class templates that inherited via parameter passthrough (e.g., template<T> class derived : public base<T>), even with inherit-base-members: copy-all. Concrete bases (derived : public base<int>) worked, dependent ones didn't, and the difference was invisible: no warning, no diagnostic (the base just got skipped).

The root cause was in BaseMembersFinalizer.cpp: when extract-implicit-specializations is on (the default), the code unconditionally replaced Name::id (primary template's ID) with SpecializationName::specializationID. The latter is set by ASTVisitor only for ClassTemplateSpecializationDecls, i.e., concrete instantiations. For a dependent base, Clang's TemplateSpecializationType::getAsCXXRecordDecl() returns the primary template, the dyn_cast to CTSD fails, and specializationID stays SymbolID::invalid. The subsequent MRDOCS_CHECK_OR_CONTINUE(baseID) then quietly bailed out. The fix is to fall back to the primary's ID when specializationID is invalid, so dependent template-template inheritance picks up the primary's members with the primary's template parameter intact. Concrete cases still inherit from the implicit specialization as before.

Changes

  • Source: BaseMembersFinalizer now only switches from the primary template's ID to SpecializationName::specializationID when the latter is valid; dependent bases keep the primary's ID and inherit its members.
  • Tests: new template-base.cpp fixture under test-files/golden-tests/config/inherit-base-members/, covering both the template-template passthrough case (derived : public base<T>) and a non-template-derived-from-concrete-specialization control (concrete_derived : public base<int>).
  • Golden tests: generated template-base.{xml,adoc,html} to reflect the cure: derived now lists the inherited method and operator[] alongside the existing entries on concrete_derived. No other fixtures changed.

Testing

The new template-base fixture is picked up automatically by the existing mrdocs-golden-tests-{xml,adoc,html} CTest targets, which already run on every CI build via the Build & Test matrix. The control case (concrete_derived) verifies that the previously-working concrete-specialization path stays unchanged, and the new case (derived) verifies the fix going forward. No CI workflow changes needed.

Documentation

Not needed: the public behavior of inherit-base-members already documents what users expected; this PR makes it match its description.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 15, 2026

✨ Highlights

  • 🧪 New golden tests added

🧾 Changes by Scope

Scope Lines Δ% Lines Δ Lines + Lines - Files Δ Files + Files ~ Files ↔ Files -
🥇 Golden Tests 99% 993 993 - 5 5 - - -
🛠️ Source 1% 15 14 1 1 - 1 - -
Total 100% 1008 1007 1 6 5 1 - -

Legend: Files + (added), Files ~ (modified), Files ↔ (renamed), Files - (removed)

🔝 Top Files

  • test-files/golden-tests/config/inherit-base-members/template-base.xml (Golden Tests): 407 lines Δ (+407 / -0)
  • test-files/golden-tests/config/inherit-base-members/template-base.html (Golden Tests): 320 lines Δ (+320 / -0)
  • test-files/golden-tests/config/inherit-base-members/template-base.adoc (Golden Tests): 236 lines Δ (+236 / -0)

Generated by 🚫 dangerJS against 5b5af96

@codecov
Copy link
Copy Markdown

codecov Bot commented May 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.12%. Comparing base (18818c9) to head (5b5af96).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1203   +/-   ##
========================================
  Coverage    82.12%   82.12%           
========================================
  Files           33       33           
  Lines         3149     3149           
  Branches       734      734           
========================================
  Hits          2586     2586           
  Misses         387      387           
  Partials       176      176           
Flag Coverage Δ
bootstrap 82.12% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gennaroprota gennaroprota linked an issue May 15, 2026 that may be closed by this pull request
@gennaroprota gennaroprota changed the title fix: inherited members are missing in derived classes (issue #1176) fix: inherited members not shown in derived classes (issue #1176) May 15, 2026
…e#1176)

When the base is a dependent template specialization (e.g., `base<T>` in
`template<T> class derived : public base<T>`), `BaseMembersFinalizer`
unconditionally read `SpecializationName::specializationID`, which is
only set for concrete `ClassTemplateSpecializationDecl`s. For dependent
bases, it stays invalid, so the `MRDOCS_CHECK_OR_CONTINUE(baseID)` guard
silently skipped the base and no members were inherited.

The fix falls back to the primary template's ID (`Name::id`) when
`specializationID` is invalid, so dependent template-template
inheritance now picks up the primary's members. Concrete cases like
`derived : public base<int>` still inherit from the implicit
specialization as before.

Closes issue cppalliance#1176.
@gennaroprota gennaroprota force-pushed the fix/inherited_members_not_shown_in_derived_classes branch from d8f3df1 to 5b5af96 Compare May 15, 2026 09:18
@cppalliance-bot
Copy link
Copy Markdown

cppalliance-bot commented May 15, 2026

An automated preview of the documentation is available at https://1203.mrdocs.prtest2.cppalliance.org/index.html

If more commits are pushed to the pull request, the docs will rebuild at the same URL.

2026-05-15 09:29:39 UTC

Copy link
Copy Markdown
Collaborator

@alandefreitas alandefreitas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. My comment is that this title makes sense for the issue, but not for the commit. After the PR, we know exactly the case we're fixing. Issue numbers in the title are uncommon, too.

@gennaroprota
Copy link
Copy Markdown
Collaborator Author

I'll remove the issue number from the title. As to the commit message subject mentioning the issue and not the fix, that was intentional: we report the subject in our release notes, and people may be scanning them for known problems. Would you like to mention the fix instead?

@gennaroprota gennaroprota changed the title fix: inherited members not shown in derived classes (issue #1176) fix: inherited members not shown in derived classes May 15, 2026
@alandefreitas
Copy link
Copy Markdown
Collaborator

Oh, I see. I didn’t know it was intentional. Well, I think this would break the pattern used here for years. Changing the pattern is significant enough to warrant internal discussion. The impact I see in the kind of changelog we get:

  • The changelog extension has an expectation of where the issues we reference go. This issue wouldn’t appear on the list of issues in the changelog or the corresponding issues for a change because the extension will look for the issue number in the body, as in conventional commit messages, and for the GitHub issue-tagging pattern.
  • The changelog would also become more redundant. Right now, the person knows what has been fixed from the change log (if their case is covered by the fix), and the issue provides a reference to the issue that led us to discover the problem (someone else’s case). But after this change to the changelog, the person scanning it doesn’t know what has been fixed, only the case that led us to find out about the necessity for the fix. The new format would remove the initial information about how it affects their case (for instance, cases involving specializations) and would include information about someone else’s case twice (when the person didn’t yet know the full category of fixtures affected).

We could change the system. But I don't think the internal discussion about weather we should change this is worth blocking this PR. I think just changing the title until we have the discussion makes more sense even if we decide to change it.

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

Successfully merging this pull request may close these issues.

Inherited members are not shown in derived classes

3 participants