[PATCH] D16843: [Sema] Fix bug in TypeLocBuilder::pushImpl

[Manman Ren via cfe-commits]

manmanren added inline comments.

================
Comment at: lib/Sema/TypeLocBuilder.cpp:130
@@ +129,3 @@
+      else if (CurrentlyHasPadding) {
+        // Remove 4-byte padding.
+        memmove(&Buffer[Index + 4], &Buffer[Index], NumBytesAtAlign4);
----------------
Expand this comment a little?

================
Comment at: lib/Sema/TypeLocBuilder.cpp:137
@@ -122,1 +136,3 @@
+        Index -= 4;
+      }
     }
----------------
The logic looks correct.

The above if, else if, else can be simplified by combining the first condition with the last else condition, because memmove(xx, xx, 0) will just do nothing.

if (CurrentlyHasPadding) {
  // remove padding
} else {
  // insert padding
}

If you write this part (LocalAlignment  == 8) in the same form as how we handle LocalAlignment == 4, it may be better to understand. i.e depending on the following conditions (NumBytesAtAlign8 == 0) (Padding == 0) (LocalSize % 8 == 0)

To help understanding, we can add some high-level comments to the original code of handling "LocalAlignment == 4":
When NumBytesAtAlign8 is not zero, we make sure Index is 8-byte aligned at function exit by adding a 4-byte padding if necessary.


http://reviews.llvm.org/D16843