Page MenuHomePhabricator

Add -fapply-global-visibility-to-externs for -cc1
ClosedPublic

Authored by scott.linder on Jan 17 2019, 11:31 AM.

Details

Summary

Introduce an option to request global visibility settings be applied to declarations without a definition or an explicit visibility, rather than the existing behavior of giving these default visibility. When the visibility of all or most extern definitions are known this allows for the same optimisations -fvisibility permits without updating source code to annotate all declarations.

Diff Detail

Repository
rL LLVM

Event Timeline

scott.linder created this revision.Jan 17 2019, 11:31 AM
scott.linder changed the repository for this revision from rL LLVM to rC Clang.Jan 17 2019, 11:35 AM
scott.linder retitled this revision from Add -f[no-]set-visibility-for-decls to Add -fset-visibility-for-decls for -cc1.

Remove driver options

rjmccall added inline comments.Jan 17 2019, 2:29 PM
include/clang/Driver/CC1Options.td
707 ↗(On Diff #182390)

"Declaration" doesn't unambiguously mean "non-definition" in language parlance;
maybe something like "external declaration" in both help text and the option name?
So maybe -fextern-visibility?

scott.linder retitled this revision from Add -fset-visibility-for-decls for -cc1 to Add -fextern-visibility for -cc1.

Update the -cc1 option name and docs; also update the LangOpt and docs.

I'm sorry to keep jerking you around, but let's spell out the flag a bit more: -fapply-global-visibility-to-externs. No reason not to be totally clear here. Thank you for updating the fields and descriptions, those all look good now.

scott.linder retitled this revision from Add -fextern-visibility for -cc1 to Add -fapply-global-visibility-to-externs for -cc1.

No worries, I agree that we don't gain much with a shorter flag here; explicit seems preferable.

rjmccall accepted this revision.Jan 18 2019, 11:01 AM

Thanks, I appreciate you bearing with me on this. LGTM.

This revision is now accepted and ready to land.Jan 18 2019, 11:01 AM
This revision was automatically updated to reflect the committed changes.