This is an archive of the discontinued LLVM Phabricator instance.

[Sema] Respect instantiated-from template's VisibilityAttr for two implicit/explicit instantiation cases
Needs ReviewPublic

Authored by MaskRay on Jul 8 2023, 12:50 PM.


Group Reviewers
Restricted Project

In CodeGenCXX/visibility.cpp,

Match GCC for part of test51: Change explicit instantiation of a function
template to respect the instantiated-from template's VisibilityAttr.

For the implicit instantiation zed<&z>(), our behavior remains different from
GCC, as the template parameter type 'foo' (hidden) wins
(shouldConsiderTemplateVisibility returns true). This seems an extreme corner

Match GCC for test71: For an implicit or explicit instantiation of a class
template, change its member template instantiation to respect the
instantiated-from member template's VisibilityAttr.

Diff Detail

Event Timeline

MaskRay created this revision.Jul 8 2023, 12:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 8 2023, 12:50 PM
MaskRay requested review of this revision.Jul 8 2023, 12:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 8 2023, 12:50 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
MaskRay edited the summary of this revision. (Show Details)Jul 8 2023, 1:06 PM
rjmccall added inline comments.Jul 24 2023, 10:56 AM

Okay, this change seems wrong. A visibility attribute on an explicit specialization or instantiation should definitely override everything else. A visibility attribute on a template pattern should only override other visibility information that we would derive from that pattern, like the visibility of the template parameters; it should not override visibility from the template arguments. You probably need this function to return an enum with three cases: (1) factor in both template arguments and template parameters, (2) factor in only template arguments, and (3) factor in nothing.

Also, the bug here doesn't seem to be specific to function templates, and you should fix the code for all three paths (function templates, class templates, and variable templates). Basically we're universally mishandling visibility attributes on template patterns for templates with non-type template parameters with hidden visibility.

MaskRay updated this revision to Diff 550961.Aug 16 2023, 7:26 PM

rebase. this does not address rjmccall's comment, but just to show the difference to other tests

MaskRay added inline comments.Nov 13 2023, 1:07 AM

I have added test coverage to test51 in visibility.cpp and studied GCC's behavior.

Sent to ignore visibility effects from template parameters. It will bring our behavior closer to GCC.