Page MenuHomePhabricator

[XRay][compiler-rt] Remove dependency on <system_error>
ClosedPublic

Authored by dberris on Mar 21 2017, 9:40 PM.

Details

Summary

Depending on C++11 <system_error> introduces a link-time requirement to
C++11 symbols. Removing it allows us to depend on header-only C++11 and
up libraries.

Partially fixes http://llvm.org/PR32274 -- we know there's more invasive work
to be done, but we're doing it incrementally.

Diff Detail

Repository
rL LLVM

Event Timeline

dberris created this revision.Mar 21 2017, 9:40 PM
dblaikie accepted this revision.Mar 21 2017, 9:45 PM

Looks plausible/good to me.

This revision is now accepted and ready to land.Mar 21 2017, 9:45 PM
pelikan accepted this revision.Mar 21 2017, 9:46 PM

Code like from the good ol' days! It looks actually better than before ;-)

This revision was automatically updated to reflect the committed changes.
kpw edited edge metadata.Mar 22 2017, 4:09 PM

I'm a bit late to the party, but LGTM. I agree with Martin that this is preferable to the old revision in terms of clarity. "return BufferQueue::ErrorCode::Ok" beats "return {}" for reading in my book.

compiler-rt/trunk/lib/xray/tests/unit/buffer_queue_test.cc
84–86

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?

dberris added inline comments.Mar 22 2017, 4:42 PM
compiler-rt/trunk/lib/xray/tests/unit/buffer_queue_test.cc
84–86

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).