[PATCH] D51289: [XRay] FDR Record Producer/Consumer Implementation

[Dean Michael Berris via Phabricator via llvm-commits]

dberris added inline comments.


================
Comment at: llvm/trunk/lib/XRay/FDRRecordProducer.cpp:50
+      MetadataRecordKinds::Pid,
+  };
+  switch (Mapping[T]) {
----------------
RKSimon wrote:
> @dberris  Why are you mapping an integer to an enum with the same value?!? Replace the uint8_t T arg with an MetadataRecordKinds enum arg, or just get rid of the enum (AFAICT its only used in this function) and use the uint8_t directly.
So, there are two reasons:

- The data we're getting is a byte, and we need to decide whether the byte maps to the number we're expecting. The caller of the function (below) only has access to the byte.

- I could just static cast the byte into a `MetadataRecordKinds` value, but that's undefined behaviour (not all the values of T will be valid values for the enum).

If I get rid of the enum, I have to keep the enum names and values as comments (previous version of this patch).

Ultimately the enum is a convenience. This mapping though is a little suspect.

This is where being able to just share the enum definition between compiler-rt and llvm would make things a lot easier. :(


Repository:
  rL LLVM

https://reviews.llvm.org/D51289