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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
