[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added a comment.

In D147669#4251441 <https://reviews.llvm.org/D147669#4251441>, @compnerd wrote:

> In D147669#4249968 <https://reviews.llvm.org/D147669#4249968>, @sgraenitz wrote:
>
>> First of all, yes the existing code is incorrect. Thanks for looking into this. However, I am afraid this isn't the right solution.
>
>
>
>> You probably mean `LLVM_ENABLE_ABI_BREAKING_CHECKS`? For `Error` this enables logic to make sure the error was checked: https://github.com/llvm/llvm-project/blob/release/16.x/llvm/include/llvm/Support/Error.h#L724 I think your observation is a side effect of the issue you look at and has nothing to do with compiler optimizations like NRVO. If logging is off, the error isn't consumed.
>
> Yes, I did indeed mean checks and not changes.  I had originally tried with only the consumption in the non-logging case, but that still seemed to abort similarly.  Adding the `std::move` on the assignment for the conditional was also required for some reason.

Hmm, now I'm wondering if I had run the wrong binary, because I do remember that I somehow ended up running the wrong binary once or twice.  Let's go with removing the confusing `std::move` in the `if` because that _shouldn't_ be needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147669