[PATCH] D99978: Fixed CodeView GuidAdapter::format to handle GUID bytes in the right order.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/lib/DebugInfo/CodeView/Formatters.cpp:28
   static const char *Lookup = "0123456789ABCDEF";
+  static const uint8_t Order[16] = {3, 2, 1,  0,  5,  4,  7,  6,
+                                    8, 9, 10, 11, 12, 13, 14, 15};
----------------
aganea wrote:
> I know the patch you're proposing is more optimal in terms of runtime performance, but it adds an extra layer of complexity for a reader who was to figure out why memory is read in that way. Since GUIDs are not that used in binary files, performance is maybe less of a concern. We could maybe rewrite the code to be more self-documenting, something along those lines?
> ```
> void GuidAdapter::format(raw_ostream &Stream, StringRef Style) {
>   assert(Item.size() == 16 && "Expected 16-byte GUID");
>   struct _GUID {
>     support::ulittle32_t Data1;
>     support::ulittle16_t Data2;
>     support::ulittle16_t Data3;
>     support::ubig64_t Data4;
>   };
>   const _GUID *G = reinterpret_cast<const _GUID *>(Item.data());
>   Stream << "{"
>          << format_hex_no_prefix(G->Data1, sizeof(G->Data1), /*Upper=*/true)
>          << '-'
>          << format_hex_no_prefix(G->Data2, sizeof(G->Data2), /*Upper=*/true)
>          << '-'
>          << format_hex_no_prefix(G->Data3, sizeof(G->Data3), /*Upper=*/true)
>          << '-'
>          << format_hex_no_prefix(G->Data4, sizeof(G->Data4), /*Upper=*/true)
>          << "}";
> }
> ```
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99978