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

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, DiggerLin.
Herald added a subscriber: dang.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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 was that '-fvisibility default' passed to -cc1 wasn'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 <https://reviews.llvm.org/D87451>).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97552

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1838,6 +1838,23 @@
     }
   }
 
+  // This is the reason why '-fvisibility' needs to be always generated:
+  // its absence implies '-mignore-xcoff-visibility'.
+  //
+  // Suppose the original cc1 command line does contain '-fvisibility default':
+  // '-mignore-xcoff-visibility' should not be implied.
+  // * If '-fvisibility' is not generated (as most options with default values
+  //   don't), its absence would imply '-mignore-xcoff-visibility'. This changes
+  //   the command line semantics.
+  // * If '-fvisibility' is generated regardless of its presence and value,
+  //   '-mignore-xcoff-visibility' won't be implied and the command line
+  //   semantics are kept intact.
+  //
+  // When the original cc1 command line does **not** contain '-fvisibility',
+  // '-mignore-xcoff-visibility' is implied. The generated command line will
+  // contain both '-fvisibility default' and '-mignore-xcoff-visibility' and
+  // subsequent calls to `CreateFromArgs`/`generateCC1CommandLine` will always
+  // produce the same arguments.
   if (T.isOSAIX() && (Args.hasArg(OPT_mignore_xcoff_visibility) ||
                       !Args.hasArg(OPT_fvisibility)))
     Opts.IgnoreXCOFFVisibility = 1;
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5302,7 +5302,9 @@
   MarshallingInfoStringInt<CodeGenOpts<"SSPBufferSize">, "8">;
 def fvisibility : Separate<["-"], "fvisibility">,
   HelpText<"Default type and symbol visibility">,
-  MarshallingInfoVisibility<LangOpts<"ValueVisibilityMode">, "DefaultVisibility">;
+  MarshallingInfoVisibility<LangOpts<"ValueVisibilityMode">, "DefaultVisibility">,
+  // Always emitting because of the relation to `-mignore-xcoff-visibility`.
+  AlwaysEmit;
 def ftype_visibility : Separate<["-"], "ftype-visibility">,
   HelpText<"Default type visibility">,
   MarshallingInfoVisibility<LangOpts<"TypeVisibilityMode">, fvisibility.KeyPath>;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97552.326676.patch
Type: text/x-patch
Size: 2247 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210226/32c3cd7e/attachment.bin>