[PATCH] D87451: add new clang option -mignore-xcoff-visibility

[Jason Liu via Phabricator via cfe-commits]

jasonliu added inline comments.


================
Comment at: clang/docs/ClangCommandLineReference.rst:2310
 
+.. option:: -mignore-xcoff-visibility
+
----------------
We should move this option to where all the other -m options resides.


================
Comment at: clang/docs/ClangCommandLineReference.rst:2312
+
+Ignore all the visibility pragmas and attributes in source code for aix xcoff.
+
----------------



================
Comment at: clang/include/clang/Basic/LangOptions.def:300
 ENUM_LANGOPT(GC, GCMode, 2, NonGC, "Objective-C Garbage Collection mode")
+BENIGN_LANGOPT(IgnoreXCOFFVisibility, 1, 0, "All the visibility pragmas and attributes that are specified in the source are ignored in aix.")
 ENUM_LANGOPT(ValueVisibilityMode, Visibility, 3, DefaultVisibility,
----------------
nit: AIX XCOFF target


================
Comment at: clang/include/clang/Driver/Options.td:1939
 def dA : Flag<["-"], "dA">, Alias<fverbose_asm>;
+def mignore_xcoff_visibility : Flag<["-"], "mignore-xcoff-visibility">, Group<m_Group>,
+  HelpText<"Ignore all the visibility pragmas and attributes in source code">,
----------------
We should move this option to where the m groups belong.


================
Comment at: clang/lib/AST/Decl.cpp:1487
+                             ? CK.forLinkageOnly()
+                             : CK);
 }
----------------
Question: When I look for `visibility` in CodeGen, I could find some of the visibility settings there that does not depend on the query of existing visibility settings. For example, some of the vtable generation, which means we could still get visibility settings out from IR in some situation?
Does those visibility settings in the CodeGen matters?


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2768
   // The type-visibility mode defaults to the value-visibility mode.
   if (Arg *typeVisOpt = Args.getLastArg(OPT_ftype_visibility)) {
     Opts.setTypeVisibilityMode(parseVisibility(typeVisOpt, Args, Diags));
----------------
Question: how should -mignore-xcoff-visiblity interact with -ftype-visibility?


================
Comment at: clang/test/Driver/ignore-xcoff-visibility.cpp:24
+// RUN: FileCheck -check-prefix=ERROR %s
+
+__attribute__((visibility("hidden"))) void foo_h(int *p) {
----------------
I don't feel like this test belongs in the Driver directory. 
There is driver test needed, which is to pass in `-###` and specify -mignore-xcoff-visibility to see if it gets passed by the driver.
But the majority of what this test tests seems belong to CodeGen directory.


================
Comment at: clang/test/Driver/ignore-xcoff-visibility.cpp:62
+#pragma GCC visibility pop
+
+// IGNOREVISIBILITY:    @b = global i32 0
----------------
Do we also want to test attribute visibility on a type?
For example:
```
class __attribute__((__visibility__("hidden"))) TestClass {
   int foo();
   int bar();
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87451/new/

https://reviews.llvm.org/D87451