Page MenuHomePhabricator

[clang][cli] Fix generation of '-fvisibility' with regards to '-mignore-xcoff-visibility'
ClosedPublic

Authored by jansvoboda11 on Feb 26 2021, 6:21 AM.

Details

Summary

This patch fixes failure of the CodeGen/aix-ignore-xcoff-visibility.cpp test with command line round-trip.

The absence of '-fvisibility' implies '-mignore-xcoff-visibility'.

The problem is that when '-fvisibility default' is passed to -cc1, it isn't being generated. (This adheres to the principle that generation doesn't produce arguments with default values.)

However, that caused '-mignore-xcoff-visibility' to be implied in the generated command line (without '-fvisibility'), while it wasn't implied in the original command line (with '-fvisibility').

This patch fixes that by always generating '-fvisibility' and explains the situation in comment.

(The '-mginore-xcoff-visibility' option was added in D87451).

Diff Detail

Event Timeline

jansvoboda11 created this revision.Feb 26 2021, 6:21 AM
jansvoboda11 requested review of this revision.Feb 26 2021, 6:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 26 2021, 6:21 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
jansvoboda11 edited the summary of this revision. (Show Details)Mar 4 2021, 8:59 AM
This revision is now accepted and ready to land.Mar 4 2021, 9:11 AM
DiggerLin added a comment.EditedMar 5 2021, 2:08 PM
  1. in your summary , it looks clang will emit "-fvisibility default" to cc1 even if there is no -fvisibility in the clang command ? I compiled the clang with your patch, it looks do not always emit "-fvisibility default" to cc1. I understand wrong on your summary ?
  2. If clang will emit "-fvisibility default" to cc1 when there is no -fvisibility in the clang command, it is a new functionality , there should be a test case for it?
  1. in your summary , it looks clang will emit "-fvisibility default" to cc1 even if there is no -fvisibility in the clang command ? I compiled the clang with your patch, it looks do not always emit "-fvisibility default" to cc1. I understand wrong on your summary ?
  2. If clang will emit "-fvisibility default" to cc1 when there is no -fvisibility in the clang command, it is a new functionality , there should be a test case for it?
  1. -fvisibility default will be generated by CompilerInvocation::generateCC1CommandLine, which will be used during Clang modules build. Not by the Clang driver when constructing -cc1 invocation on normal (non-modular) build.
  2. This code is tested by a round-trip mechanism when -DCLANG_ROUND_TRIP_CC1_ARGS is turned on: https://lists.llvm.org/pipermail/cfe-dev/2021-February/067714.html