-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ZIR-232: Add Assert
and Abort
externs
#69
Conversation
Assert
and Abort
externsAssert
and Abort
externs
@@ -44,7 +44,8 @@ template <typename T> void formatFieldElem(const InterpVal* interpVal, llvm::raw | |||
std::vector<uint64_t> ExternHandler::doExtern(llvm::StringRef name, | |||
llvm::StringRef extra, | |||
llvm::ArrayRef<const InterpVal*> args, | |||
size_t outCount) { | |||
size_t outCount, | |||
bool* failed) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of pointer-to-bool in this API leaves me feeling sketchy, both because of the vagueness about lifetimes associated with bare pointers, and because the caller can easily (perhaps inadvertently) ignore the failure of a call. Would it work to return std::optional<std::vector<uint64_t>>
instead, using the absence of a result to indicate failure? Or, if that's too heavy, perhaps use a bool&
instead?
Perhaps these ideas have been considered and rejected - happy to accept this if there's reason to do it this way - but it'd be good to know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was reluctant because there were a lot of places where existing extern handlers use return {};
, which would then mean std::nullopt
instead of an empty array. I got over it and fixed all of these, because you're right that it's better to do it that way.
6fba7e7
to
26fb714
Compare
Resolves ZIR-185
extern Assert(x: Val, message: String);
which stops proving ifx
is not zeroextern Abort();
which unconditionally stops proving