[Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter

[Dimitar Vlahovski via lldb-commits]

dvlahovski added inline comments.

================
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:47-48
@@ +46,4 @@
+
+  if (*context_flags & uint32_t(MinidumpContext_x86_64_Flags::Control)) {
+    writeRegister(source_data, result_base, &reg_info[lldb_cs_x86_64], 2);
+  }
----------------
amccarth wrote:
> dvlahovski wrote:
> > If it is then when I do a `&` the result is an enum class of type `MinidumpContext_x86_64_Flags`. And the compiler complains that this is not convertible to bool
> I think what Zach means is that you could locally define a uint32_t const, initialized with the value from the enum.  Then each if statement could use that constant without a cast.
> 
> Also, is this right?  `MinidumpContext_x86_x64_Flags::Control` has two bits set, so the condition will be true if either of them is set.  Is that the intended behavior?  Or should you be ensuring that they're both set like this:
> 
>     const utin32_t ControlFlags = MinidumpContext_x86_64::Control;
>     if ((*context_flags & ControlFlags) == ControlFlags) {
>       ...
>     }
> 
> ?
Now that I think about I should check the arch flag bit at the beggining of this.
But then this if's are OK I think - in them I only want to check if that specific bit is set.


https://reviews.llvm.org/D24919