[llvm-commits] [PATCH] lld driver

João Matos triton at vapor3d.org
Wed Dec 5 06:12:34 PST 2012



================
Comment at: lib/Driver/CoreOptions.td:3
@@ +2,3 @@
+
+def target : Separate<["-"], "target">, HelpText<"Target tripple to link for">;
+
----------------
Small typo on "triple" here.

================
Comment at: lib/Driver/Drivers.cpp:34
@@ +33,3 @@
+#include "CoreOptions.inc"
+  LastOption
+#undef OPTION
----------------
Maybe this should be OPT_LAST, to be consistent with OPT_INVALID?

================
Comment at: lib/Driver/Drivers.cpp:63
@@ +62,3 @@
+#include "LDOptions.inc"
+  LastOption
+#undef OPTION
----------------
Same here.

================
Comment at: lib/Driver/LDOptions.td:4
@@ +3,3 @@
+def flavor : Separate<["-"], "flavor">;
+def target : Separate<["-"], "target">, HelpText<"Target tripple to link for">;
+
----------------
Small typo on "triple" here.

================
Comment at: lib/Driver/Targets.cpp:11-13
@@ +10,5 @@
+/// \file
+///
+/// Concrete instances of the Target interface.
+///
+//===----------------------------------------------------------------------===//
----------------
Sean Silva wrote:
> Maybe call the file "ConcreteTargets.cpp". It's kind of annoying to see "Target.cpp" and "Targets.cpp", where nothing besides the "plural" relationship is apparent. "ConcreteTargets.cpp" makes the relationship obvious.
Since Clang (Basic/Targets.cpp) and LLVM (llvm/Config/Targets.def) already use Targets for this, I'd prefer to keep the same terminology.

================
Comment at: tools/lld/lld.cpp:121
@@ +120,3 @@
+
+  Driver::Flavor iHazAFlavor = selectFlavor(argc, argv);
+  if (iHazAFlavor == Driver::Flavor::invalid)
----------------
Sean Silva wrote:
> no comment...
While I don't have a problem with funny variable naming, some people might find it unprofessional.


http://llvm-reviews.chandlerc.com/D169



More information about the llvm-commits mailing list