[PATCH] D53787: [Sema] Provide -fvisibility-global-new-delete-hidden option

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Looks good with minor changes, thanks.



================
Comment at: clang/include/clang/Basic/LangOptions.def:228
 BENIGN_LANGOPT(InlineVisibilityHidden , 1, 0, "hidden default visibility for inline C++ methods")
+BENIGN_LANGOPT(GlobalAllocationFunctionVisibilityHidden , 1, 0, "hidden default visibility for global operator new and delete declaration")
 BENIGN_LANGOPT(ParseUnknownAnytype, 1, 0, "__unknown_anytype")
----------------
This should just be `LANGOPT`, not `BENIGN_LANGOPT`, because it changes how we construct the AST (in an incompatible way).

Also, could you please remove the "default" from the description of this LANGOPT (and the previous one)? "default visibility" means something else, so this is confusing as written.


================
Comment at: clang/include/clang/Driver/Options.td:1728
+def fvisibility_global_new_delete_hidden : Flag<["-"], "fvisibility-global-new-delete-hidden">, Group<f_Group>,
+  HelpText<"Give global C++ operator new and delete declarations hidden visibility by default">, Flags<[CC1Option]>;
 def fwhole_program_vtables : Flag<["-"], "fwhole-program-vtables">, Group<f_Group>,
----------------
Remove the "by default" here: the program can't change the visibility to anything else via an attribute.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53787