[PATCH] Refactoring of Driver and TargetInfo

kledzik at apple.com kledzik at apple.com
Wed Apr 3 14:20:56 PDT 2013



================
Comment at: lib/Core/Resolver.cpp:368-371
@@ -364,3 +367,6 @@
   this->deadStripOptimize();
-  this->checkUndefines(false);
+  if (this->checkUndefines(false)) {
+    if (!_targetInfo.allowRemainingUndefines())
+      return true;
+  }
   this->removeCoalescedAwayAtoms();
----------------
Shankar Kalpathi Easwaran wrote:
> Should the returns be vice versa. true on success / false on failure. Might be easier to read if (!resolve()) for failure cases.
>From what I've seen in LLVM, it is common to return true if there is an error.  That way the client does:
  if (xxx()) {
     // handle error
  }
But I can go either way on this.

================
Comment at: lib/Driver/CoreOptions.td:6-11
@@ -13,5 +5,8 @@
 
-def OCTOTHORPE_OCTOTHORPE_OCTOTHORPE : Flag<["-"], "###">;
+def dead_strip              : Flag<["--"], "dead-strip">;
+def undefines_are_errors    : Flag<["--"], "undefines-are-errors">;
+def keep_globals            : Flag<["--"], "keep-globals">;
+def commons_search_archives : Flag<["--"], "commons-search-archives">;
 
-def emit_yaml : Flag<["-"], "emit-yaml">;
+def add_pass       : Separate<["--"], "add-pass">;
 
----------------
Shankar Kalpathi Easwaran wrote:
> Why extra spaces ?
It is easier to read when the columns are aligned.

================
Comment at: lib/Driver/DarwinOptions.td:21-35
@@ +20,16 @@
+                          HelpText<"min iOS version">;
+def ios_simulator_version_min : Separate<["-"], "ios_simulator_version_min">, 
+                          HelpText<"min iOS simulator version">;
+
+
+
+def output : Separate<["-"], "o">, HelpText<"output file name">;
+
+
+def dump : Flag<["-"], "###">;
+
+def emit_yaml : Flag<["-"], "emit_yaml">;
+
+
+def L : Joined<["-"], "L">;
+
----------------
Shankar Kalpathi Easwaran wrote:
> lot of new lines ? not sure if it exists in the file though.
I was grouping by functionality.  I've cleaned it up a bit more

================
Comment at: lib/Driver/LDOptions.td:21-23
@@ -16,8 +20,5 @@
 
 def dynamic_linker : Separate<["-"], "dynamic-linker">;
 
-def OCTOTHORPE_OCTOTHORPE_OCTOTHORPE : Flag<["-"], "###">;
-
-def emit_yaml : Flag<["-"], "emit-yaml">;
 
 def m : Separate<["-"], "m">;
----------------
Shankar Kalpathi Easwaran wrote:
> Same here, lot of new lines. Not sure if its in the file though.
Again grouping by functionality.  Now that the driver organization is in place, someone who knows binutils ld driver option well can flush out this file and add help strings and group names.

================
Comment at: lib/ReaderWriter/ELF/ELFTargetInfo.cpp:66-68
@@ -80,1 +65,5 @@
+  if (_outputFileType == elf::ET_EXEC) {
+    if (_entrySymbolName.empty()) {
+      _entrySymbolName = "start";
+    }
   }
----------------
Shankar Kalpathi Easwaran wrote:
> _start
Fixed


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



More information about the llvm-commits mailing list