This moves the toString(Error) function, and the failure-handling case of
consumeError, out-of-line. This enables calls to consumeError, toString,
errorToBool, and expectedToOptional from code compiled with RTTI turned on,
even if LLVM is built with RTTI turned off.
Details
- Reviewers
dblaikie v.g.vassilev
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Vassil -- This is a generalization of the error fix that we discussed. Could you double check that it fixes the linker error that you saw?
After thinking about it for a while I'm on the fence about this approach -- on the one hand it's a reasonable cleanup, on the other hand I don't think we generally support linking -no-rtti LLVM libraries from -rtti code, and support for doing that (even partial support) might be an explicit non-goal for the project.
Another approach to fixing the example might just be to split it into two files. Pros of that approach: it deals with no-rtti/rtti crossover code The Right Way, Con: it complicates the example for everyone else.
Going via C also works for me:
diff diff --git a/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp b/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp index 744a98bcdc87..fbdeddf0300b 100644 --- a/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp +++ b/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp @@ -25,6 +25,7 @@ #include "llvm/ExecutionEngine/Orc/LLJIT.h" #include "llvm/Support/ManagedStatic.h" #include "llvm/Support/TargetSelect.h" +#include "llvm-c/Error.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -52,9 +53,7 @@ TEST(InterpreterTest, CatchException) { auto J = llvm::orc::LLJITBuilder().create(); if (!J) { // The platform does not support JITs. - // We can't use llvm::consumeError as it needs typeinfo for ErrorInfoBase. - auto E = J.takeError(); - (void)E; + LLVMConsumeError(llvm::wrap(J.takeError())); return; } }
I think that would be the least intrusive way to go...
Ok. I'm going to abandon this review -- we can revisit in the future if we find more instances of this issue.