[PATCH] D31233: [XRay][compiler-rt] Remove dependency on <system_error>

Dean Michael Berris via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 22 16:42:22 PDT 2017


dberris added inline comments.


================
Comment at: compiler-rt/trunk/lib/xray/tests/unit/buffer_queue_test.cc:84-86
+    while (true) {
+      auto EC = Buffers.getBuffer(B);
+      if (EC != BufferQueue::ErrorCode::Ok)
----------------
kpw wrote:
> No action required here. Just a general style question. 
> 
> enum class is sometimes annoying by removing the implicit cast to numeric.
> For something like error codes, having the implicit cast is so natural.
> 
> What do you think of constructs where you still get scoping and implicit cast like: 
> 
> class Errorcodes {   enum Error { OK = 0, ... }; };
> 
> or similar with a namespace instead of a class?
I like that suggestion. I think it's probably something worth doing (inventing our own version of system error) later on, if it becomes something more useful not only in XRay. We can iterate on this and I'm definitely happy to make that easier to use going forward. But for now the only places where this is actually used is well contained and well defined enough that if it starts becoming a problem we can definitely go the way of a class that has a bit more semantics (explicit operator bool, error string accessor, etc).


Repository:
  rL LLVM

https://reviews.llvm.org/D31233





More information about the llvm-commits mailing list