ab added a comment. Please check the embedded thing (the other comments are minor). Otherwise, LGTM; if you have found this flag to be necessary, this looks like a reasonable way to implement it
================ Comment at: clang/include/clang/Driver/Options.td:1252 +def fapple_link_rtlib : Flag<["-"], "fapple-link-rtlib">, Group<f_Group>, + HelpText<"Apple specific option to force linking the clang builtins runtime library">; def flto_EQ : Joined<["-"], "flto=">, Flags<[CoreOption, CC1Option]>, Group<f_Group>, ---------------- Hmm, maybe move "Apple specific" to a parenthesis at the end? Also, I'm guessing this belonged here when it was "-flink-rtlib", it doesn't anymore. ================ Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:573 + bool NoStdOrDefaultLibs = + Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs); + bool ForceLinkBuiltins = Args.hasArg(options::OPT_fapple_link_rtlib); ---------------- Maybe sink this to the AddLinkRuntimeLibArgs implementations (as an extra parameter or directly checking the args)? The flags don't bypass the whole thing anymore, might as well treat it like -mkernel and whatnot. ================ Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2084 AddLinkRuntimeLib(Args, CmdArgs, CompilerRT, RLO_IsEmbedded); } ---------------- This is different from 'builtins'. Are you OK with the difference? Otherwise maybe this should be an error for now; I wouldn't be surprised if we never hit this path (this part, I'm not familiar with) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58320/new/ https://reviews.llvm.org/D58320 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
