[PATCH] D34600: [Error] add expectSuccess()

[Lang Hames via Phabricator via llvm-commits]

lhames added a comment.

> I was only aiming for one that wasn't even behind NDEBUG as it seems pointless to "optimize" this code path rather than crashing quickly.

Yep. We tossed this question around a few times during the development of Error. It fell in the same basket as "should we track error propagation even in optimized builds" (which I would expect to add more overhead than this). In the end we settled on minimizing the performance overhead for now. It's a discussion I'm happy to revisit though: practice production code is likely to be exposed to environments we haven't tested, and having it fail loudly and clearly (rather than invoking UB) might be worth a little extra overhead? (But I guess you could say that about any assertion).

As a half-way point we could consider putting it under its own flag, something like CHECK_ERRORS_IN_RELEASE.

> But no big deal anyways. I think the interesting thing is finding a good name. I'll let you two figure this out...

Sure. Zachary - do you have a preference?


https://reviews.llvm.org/D34600