[PATCH] D103089: [Debug-Info][CodeView] Fix GUID string generation for MSVC generated objects.

[Adrian McCarthy via Phabricator via llvm-commits]

amccarth added a comment.

I'm fine focusing on this part to get the bug fixed.  We can consider looking at accepting upper and lower case letters in the future.



================
Comment at: llvm/unittests/DebugInfo/CodeView/GUIDFormatTest.cpp:37
+void checkData(GuidData &Data) {
+  struct MSGuid {
+    support::ulittle32_t Data0;
----------------
I see why it's done, but it's a bit confusing that this definition of MSGuid is different from the one in the formatting code.  In fact, this one has 18 bytes of data and may contain padding.

If you were to move this definition down to the one place it's used, it might not be as confusing.


================
Comment at: llvm/unittests/DebugInfo/CodeView/GUIDFormatTest.cpp:82
+
+TEST(GUIDFormatTest, Data_00) {
+  GuidData Data = {
----------------
It's not clear to my why we need 16 of these tests.  Is this overkill?

The names Data_xx don't really clue me in as to what's going on in each test set.  I see the "Shifting xx" comments, but that still doesn't quite do it for me.

>From the comments, I expected a set of tests for Variant 1 (UUIDs) and one for Variant 2 (MS/COM-style GUIDs).


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

https://reviews.llvm.org/D103089