[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#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.

I do wonder if we should keep the consume to be unconditional or not as @alvinhochun points out that `llvm::Error` does guarantee this behaviour.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147669