This is an archive of the discontinued LLVM Phabricator instance.

[clang][driver] Add -fno-visibility-inlines-hidden option
ClosedPublic

Authored by kongyi on Jun 2 2021, 10:13 AM.

Details

Summary

This allows overriding -fvisibility-inlines-hidden.

Original change by andrewjcg, rebased to top of trunk.

Diff Detail

Event Timeline

kongyi created this revision.Jun 2 2021, 10:13 AM
kongyi requested review of this revision.Jun 2 2021, 10:13 AM
MaskRay added inline comments.
clang/include/clang/Driver/Options.td
2681

Use NegFlag. Just add a driver option, avoid the CC1 option.

MaskRay added inline comments.Jun 2 2021, 10:26 AM
clang/lib/Driver/ToolChains/Clang.cpp
5647

AddLastArg is not recommended for fno- if `fno- doesn't need to be a CC1 option.
The case below OPT_fvisibility_inlines_hidden_static_local_var is a workaround (will be deleted in few weeks) and should not be used as a reference.

kongyi updated this revision to Diff 349463.Jun 3 2021, 12:06 AM
MaskRay added inline comments.Jun 3 2021, 12:07 AM
clang/test/Driver/visibility-inlines-hidden.cpp
12

The two invocations are sufficient.

kongyi updated this revision to Diff 349465.Jun 3 2021, 12:12 AM
MaskRay added inline comments.Jun 3 2021, 12:16 AM
clang/test/Driver/visibility-inlines-hidden.cpp
10

I think there is little additional value testing both -fno- and -fpos- -fneg-. Testing the latter should be sufficient.

Other tests just use 2>&1 | FileCheck - it is more convenient though providing a bit less of guarantee (combined stdout/stderr instead of stderr) but we have dedicated tests for stderr testing. It's not this test file's responsibility for that.

16

This can be shared with CHECK-1.

kongyi updated this revision to Diff 349470.Jun 3 2021, 12:36 AM
kongyi marked 5 inline comments as done.
MaskRay accepted this revision.Jun 3 2021, 12:37 AM
MaskRay added inline comments.
clang/test/Driver/visibility-inlines-hidden.cpp
3

A common pattern is to place | either at the beginning of the continuation line or the end of the last line.
The continuation needs 2-column indentation.

This revision is now accepted and ready to land.Jun 3 2021, 12:37 AM
This revision was landed with ongoing or failed builds.Jun 3 2021, 2:09 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 3 2021, 2:09 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript