[PATCH] [lld][LayoutPass] Split buildFollowOnTable for readability.

Shankar Kalpathi Easwaran shankarke at gmail.com
Mon May 13 17:13:27 PDT 2013



================
Comment at: lib/Passes/LayoutPass.cpp:136
@@ +135,3 @@
+      return prevAtom;
+  }
+}
----------------
This should return a nullptr if nothing is found.

================
Comment at: lib/Passes/LayoutPass.cpp:153-157
@@ +152,7 @@
+      break;
+    if ((*atom).size() == 0) {
+      // TODO: print warning that an impossible layout is being desired by the
+      // user.
+      return false;
+    }
+  }
----------------
remove the extra braces

================
Comment at: lib/Passes/LayoutPass.cpp:140
@@ +139,3 @@
+// Check if all the atoms followed by the given target atom are of size zero.
+//
+// When this method is called, an atom being added is not of size zero and
----------------
empty comment ?

================
Comment at: include/lld/Passes/LayoutPass.h:76-77
@@ +75,4 @@
+  // Helper methods for buildFollowOnTable().
+  const DefinedAtom *findAtomFollowedBy(const DefinedAtom *targetAtom);
+  bool checkAllPrevAtomsZeroSize(const DefinedAtom *targetAtom);
+
----------------
These should be const functions as well.

================
Comment at: lib/Passes/LayoutPass.cpp:170-171
@@ +169,4 @@
+        _followOnNexts.find(targetAtom);
+    if (targetFollowOnAtomsIter == _followOnNexts.end())
+      break;
+    targetAtom = targetFollowOnAtomsIter->second;
----------------
could just return here.

================
Comment at: lib/Passes/LayoutPass.cpp:391-393
@@ -420,4 +390,5 @@
     if (start != _followOnRoots.end()) {
-      for (const DefinedAtom *nextAtom = start->second; nextAtom != NULL;
+      for (const DefinedAtom *nextAtom = start->second;
+           nextAtom;
            nextAtom = _followOnNexts[nextAtom]) {
         AtomToOrdinalT::iterator pos = _ordinalOverrideMap.find(nextAtom);
----------------
you might want to run the changes with clang-format, if it has not been done already. 


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



More information about the llvm-commits mailing list