[SILVerifier] Add a flag to not abort upon verification failure. #12519
Conversation
lib/SIL/SILVerifier.cpp
Outdated
| @@ -139,7 +142,10 @@ class SILVerifier : public SILVerifierBase<SILVerifier> { | |||
| F.print(llvm::dbgs()); | |||
| } | |||
|
|
|||
| abort(); | |||
| if (AbortOnFailure) | |||
There was a problem hiding this comment.
Just change this to use exit(1).
There was a problem hiding this comment.
Terrific, I'll do it early tomorrow and update the pull request.
There was a problem hiding this comment.
Do you happen to know why the behaviour was to abort (probably historical reasons)?
There was a problem hiding this comment.
I looked before I wrote that ; ). It is really really old (from revision < 10k IIRC). I think it was just someone working quickly.
|
@swift-ci please test |
|
@dcci I was talking with some other people and they pointed out that abort is important for crashing in the debugger. So can you add the option back (sorry!) |
|
Oh, sure, no worries. Mind if I add a comment to explain the behaviour? |
|
@swift-ci please test |
|
Build failed |
|
Build failed |
This matches what LLVM does, and makes testing/fuzzing easier. It's also slightly more friendly in case the user violates an assumption. It can't be turned on by default (yet) because it's important to crash on verification failures, when running swiftc in a debugger.
|
The build was failing yesterday for unrelated reasons, rebased. |
|
@swift-ci please smoke test |
|
@eeckstein what do you think about this one? I can put on hold and have this patch locally for now if you want to discuss further the whole abort VS exit in the verifier. |
Exit somewhat carefully instead. This matches what LLVM does,
and makes testing/fuzzing easier. It's also slightly more friendly
in case the user violates an assumption.
I'd prefer to have the compiler never aborting on user input (in particular, something text based), but this seems an OK middle-ground. I'll be happy to change the default altogether if people like it.