rimvydas added inline comments.

================
Comment at: lib/Driver/ToolChains/DragonFly.cpp:118
     }
-    
CmdArgs.push_back(Args.MakeArgString(getToolChain().GetFilePath("crti.o")));
-    if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
-      CmdArgs.push_back(
-          Args.MakeArgString(getToolChain().GetFilePath("crtbeginS.o")));
+    if (crt1)
+      CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crt1)));
----------------
mgorny wrote:
> Can't `crt1` be non-null only if `!Args.hasArg(options::OPT_shared` here? 
> i.e. is there a reason to do it like this instead of just pushing it inside 
> the above `if`?
The `crt1` can only be used if `!Args.hasArg(options::OPT_shared`, both 
declaration and `CmdArgs.push_back` could be moved inside. The only reason for 
this adjustment is to reduce stylistic differences against 
lib/Driver/ToolChains/FreeBSD.cpp driver layout.




================
Comment at: lib/Driver/ToolChains/DragonFly.cpp:123
+
+    const char *crtbegin = nullptr;
+    if (Args.hasArg(options::OPT_shared) || IsPIE)
----------------
mgorny wrote:
> This default will never be used.
Correct. Could be changed in both FreeBSD.cpp and DragonFly.cpp.


================
Comment at: lib/Driver/ToolChains/DragonFly.cpp:185
+    if (Args.hasArg(options::OPT_shared) || IsPIE)
+      
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtendS.o")));
     else
----------------
mgorny wrote:
> Inconsistency here: above you used helper variable, here you duplicate the 
> whole line.
Same as two previous. Only to match lib/Driver/ToolChains/FreeBSD.cpp layout.


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

https://reviews.llvm.org/D35169



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to