Skip to content

Commit

Permalink
Use fmt::format for all ENFORCE and raise (sorbet#2084)
Browse files Browse the repository at this point in the history
Use fmt::format for all ENFORCE and raise (sorbet#2084)

* Make ENFORCE and Error::raise use fmt::format

* Update all ENFORCEs to use new syntax

* Update all Exception::raise's to use new syntax

(Squashed by Merge Bot)
  • Loading branch information
jez-stripe authored and CIBot committed Mar 5, 2019
1 parent dfd0b6a commit e202d91
Show file tree
Hide file tree
Showing 25 changed files with 88 additions and 85 deletions.
6 changes: 3 additions & 3 deletions ast/desugar/Desugar.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pair<MethodDef::ARGS_store, InsSeq::STATS_store> desugarArgs(DesugarContext dctx
// do nothing
} else {
auto node = argnode.get();
Exception::raise("not implemented: ", demangle(typeid(*node).name()));
Exception::raise("not implemented: {}", demangle(typeid(*node).name()));
}

return make_pair(std::move(args), std::move(destructures));
Expand Down Expand Up @@ -278,7 +278,7 @@ unique_ptr<Expression> node2TreeImpl(DesugarContext dctx, unique_ptr<parser::Nod
return MK::EmptyTree();
}
auto loc = what->loc;
ENFORCE(loc.exists(), "parse-tree node has no location: " + what->toString(dctx.ctx));
ENFORCE(loc.exists(), "parse-tree node has no location: {}", what->toString(dctx.ctx));
unique_ptr<Expression> result;
typecase(
what.get(),
Expand Down Expand Up @@ -1498,7 +1498,7 @@ unique_ptr<Expression> node2TreeImpl(DesugarContext dctx, unique_ptr<parser::Nod
},

[&](parser::BlockPass *blockPass) { Exception::raise("Send should have already handled the BlockPass"); },
[&](parser::Node *node) { Exception::raise("Unimplemented Parser Node: ", node->nodeName()); });
[&](parser::Node *node) { Exception::raise("Unimplemented Parser Node: {}", node->nodeName()); });
ENFORCE(result.get() != nullptr, "desugar result unset");
return result;
} catch (SorbetException &) {
Expand Down
2 changes: 1 addition & 1 deletion ast/treemap/treemap.h
Original file line number Diff line number Diff line change
Expand Up @@ -711,7 +711,7 @@ template <class FUNC, class CTX> class TreeMapper {
return mapCast(std::unique_ptr<Cast>(static_cast<Cast *>(what.release())), ctx);
} else {
auto *ref = what.get();
Exception::raise("should never happen. Forgot to add new tree kind? ", demangle(typeid(*ref).name()));
Exception::raise("should never happen. Forgot to add new tree kind? {}", demangle(typeid(*ref).name()));
}
} catch (SorbetException &e) {
Exception::failInFuzzer();
Expand Down
6 changes: 3 additions & 3 deletions cfg/CFG.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,16 @@ void CFG::sanityCheck(core::Context ctx) {
}

for (auto &bb : this->basicBlocks) {
ENFORCE(bb->bexit.isCondSet(), "Block exit condition left unset for block " + bb->toString(ctx));
ENFORCE(bb->bexit.isCondSet(), "Block exit condition left unset for block {}", bb->toString(ctx));

if (bb.get() == deadBlock()) {
continue;
}

auto thenCount = absl::c_count(bb->bexit.thenb->backEdges, bb.get());
auto elseCount = absl::c_count(bb->bexit.elseb->backEdges, bb.get());
ENFORCE(thenCount == 1, "bb id=", bb->id, "; then has ", thenCount, " back edges");
ENFORCE(elseCount == 1, "bb id=", bb->id, "; else has ", elseCount, " back edges");
ENFORCE(thenCount == 1, "bb id={}; then has {} back edges", bb->id, thenCount);
ENFORCE(elseCount == 1, "bb id={}; else has {} back edges", bb->id, elseCount);
if (bb->bexit.thenb == bb->bexit.elseb) {
ENFORCE(!bb->bexit.cond.variable.exists());
} else {
Expand Down
2 changes: 1 addition & 1 deletion cfg/builder/builder_finalize.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ void CFGBuilder::sanityCheck(core::Context ctx, CFG &cfg) {
continue;
}
if (bb.get() != cfg.entry()) {
ENFORCE((bb->flags & CFG::WAS_JUMP_DESTINATION) != 0, "block ", bb->id, " was never linked into cfg");
ENFORCE((bb->flags & CFG::WAS_JUMP_DESTINATION) != 0, "block {} was never linked into cfg", bb->id);
}
auto thenFnd = absl::c_find(bb->bexit.thenb->backEdges, bb.get());
auto elseFnd = absl::c_find(bb->bexit.elseb->backEdges, bb.get());
Expand Down
2 changes: 1 addition & 1 deletion cfg/builder/builder_walk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ BasicBlock *CFGBuilder::walk(CFGContext cctx, ast::Expression *what, BasicBlock
[&](ast::ClassDef *c) { Exception::raise("Should have been removed by FlattenWalk"); },
[&](ast::MethodDef *c) { Exception::raise("Should have been removed by FlattenWalk"); },

[&](ast::Expression *n) { Exception::raise("Unimplemented AST Node: ", n->nodeName()); });
[&](ast::Expression *n) { Exception::raise("Unimplemented AST Node: {}", n->nodeName()); });

// For, Rescue,
// Symbol, Array,
Expand Down
11 changes: 8 additions & 3 deletions common/Exception.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,21 @@ class Exception final {
static void print_backtrace() noexcept;
static void failInFuzzer() noexcept;

[[noreturn]] static inline bool enforce_handler(std::string check, std::string file, int line)
__attribute__((noreturn)) {
enforce_handler(check, file, line, "(no message provided)");
}
template <typename... TArgs>
[[noreturn]] static inline bool enforce_handler(std::string check, std::string file, int line,
[[noreturn]] static inline bool enforce_handler(std::string check, std::string file, int line, std::string message,
const TArgs &... args) __attribute__((noreturn)) {
raise(file + ":" + std::to_string(line), " enforced condition ", check, " has failed: ", args...);
raise(file + ":" + std::to_string(line), " enforced condition ", check,
" has failed: ", fmt::format(message, args...));
}
};

template <typename... TArgs>[[noreturn]] bool Exception::raise(const TArgs &... args) {
Exception::failInFuzzer();
std::string message = absl::StrCat("", args...);
std::string message = fmt::format(args...);

if (message.size() > 0) {
fatalLogger->error("Exception::raise(): {}\n", message);
Expand Down
2 changes: 1 addition & 1 deletion common/typecase.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ template <typename Base, typename... Subclasses> void typecase(Base *base, Subcl
if (!base) {
sorbet::Exception::raise("nullptr passed to typecase");
}
sorbet::Exception::raise("not handled typecase case: ", demangle(typeid(*base).name()));
sorbet::Exception::raise("not handled typecase case: {}", demangle(typeid(*base).name()));
}
}
} // namespace sorbet
Expand Down
6 changes: 3 additions & 3 deletions core/Context.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ class GlobalSubstitution {
if (!allowSameFromTo) {
from.sanityCheckSubstitution(*this);
}
ENFORCE(from._id < nameSubstitution.size(), "name substitution index out of bounds, got " +
std::to_string(from._id) + " where subsitution size is " +
std::to_string(nameSubstitution.size()));
ENFORCE(from._id < nameSubstitution.size(),
"name substitution index out of bounds, got {} where subsitution size is {}", std::to_string(from._id),
std::to_string(nameSubstitution.size()));
return nameSubstitution[from._id];
}

Expand Down
2 changes: 1 addition & 1 deletion core/Error.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class Error {
: loc(loc), what(what), header(move(header)), isSilenced(isSilenced), autocorrects(move(autocorrects)),
sections(sections) {
ENFORCE(this->header.empty() || this->header.back() != '.');
ENFORCE(this->header.find('\n') == std::string::npos, this->header, " has a newline in it");
ENFORCE(this->header.find('\n') == std::string::npos, "{} has a newline in it", this->header);
}
};

Expand Down
11 changes: 6 additions & 5 deletions core/GlobalState.cc
Original file line number Diff line number Diff line change
Expand Up @@ -338,13 +338,13 @@ void GlobalState::initEmpty() {
for (int arity = 0; arity <= Symbols::MAX_PROC_ARITY; ++arity) {
string name = absl::StrCat("Proc", arity);
auto id = synthesizeClass(enterNameConstant(name), Symbols::Proc()._id);
ENFORCE(id == Symbols::Proc(arity), "Proc creation failed for arity: ", arity, " got: ", id._id,
" expected: ", Symbols::Proc(arity)._id);
ENFORCE(id == Symbols::Proc(arity), "Proc creation failed for arity: {} got: {} expected: {}", arity, id._id,
Symbols::Proc(arity)._id);
id.data(*this)->singletonClass(*this);
}

ENFORCE(symbols.size() == Symbols::last_synthetic_sym()._id + 1,
"Too many synthetic symbols? have: ", symbols.size(), " expected: ", Symbols::last_synthetic_sym()._id + 1);
"Too many synthetic symbols? have: {} expected: {}", symbols.size(), Symbols::last_synthetic_sym()._id + 1);

installIntrinsics();

Expand Down Expand Up @@ -895,8 +895,9 @@ void GlobalState::sanityCheck() const {
ENFORCE(!strings.empty(), "empty string table size");
ENFORCE(!names_by_hash.empty(), "empty name hash table size");
ENFORCE((names_by_hash.size() & (names_by_hash.size() - 1)) == 0, "name hash table size is not a power of two");
ENFORCE(names.capacity() * 2 == names_by_hash.capacity(), "name table and hash name table sizes out of sync",
" names.capacity=", names.capacity(), " names_by_hash.capacity=", names_by_hash.capacity());
ENFORCE(names.capacity() * 2 == names_by_hash.capacity(),
"name table and hash name table sizes out of sync names.capacity={} names_by_hash.capacity={}",
names.capacity(), names_by_hash.capacity());
ENFORCE(names_by_hash.size() == names_by_hash.capacity(), "hash name table not at full capacity");
int i = -1;
for (auto &nm : names) {
Expand Down
4 changes: 2 additions & 2 deletions core/Loc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ Loc Loc::join(Loc other) const {
Loc::Detail Loc::offset2Pos(const File &file, u4 off) {
Loc::Detail pos;

ENFORCE(off <= file.source().size(), "file offset out of bounds in file: " + string(file.path()) + " @ " +
to_string(off) + " <= " + to_string(file.source().size()));
ENFORCE(off <= file.source().size(), "file offset out of bounds in file: {} @ {} <= {}", string(file.path()),
to_string(off), to_string(file.source().size()));
if (off >= file.source().size()) {
// parser generate positions out of file \facepalm.
off = file.source().size() - 1;
Expand Down
2 changes: 1 addition & 1 deletion core/Names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ unsigned int Name::hash(const GlobalState &gs) const {
case CONSTANT:
return _hash_mix_constant(CONSTANT, cnst.original.id());
default:
Exception::raise("Unknown name kind?", kind);
Exception::raise("Unknown name kind? {}", kind);
}
}

Expand Down
2 changes: 1 addition & 1 deletion core/Symbols.cc
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ SymbolRef Symbol::findMemberTransitiveInternal(const GlobalState &gs, NameRef na
}
}

Exception::raise("findMemberTransitive hit a loop while resolving ");
Exception::raise("findMemberTransitive hit a loop while resolving");
}

SymbolRef result = findMember(gs, name);
Expand Down
6 changes: 3 additions & 3 deletions core/proto/proto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ com::stripe::rubytyper::File::StrictLevel strictToProto(core::StrictLevel strict
case core::StrictLevel::None:
return com::stripe::rubytyper::File::StrictLevel::File_StrictLevel_None;
case core::StrictLevel::Internal:
Exception::raise(
"Should never happen"); // we should never attempt to serialize any state that had internal errors
// we should never attempt to serialize any state that had internal errors
Exception::raise("Should never happen");
case core::StrictLevel::Ignore:
return com::stripe::rubytyper::File::StrictLevel::File_StrictLevel_Ignore;
case core::StrictLevel::Stripe:
Expand All @@ -231,7 +231,7 @@ com::stripe::rubytyper::File::StrictLevel strictToProto(core::StrictLevel strict
case core::StrictLevel::Autogenerated:
return com::stripe::rubytyper::File::StrictLevel::File_StrictLevel_Autogenerated;
default:
ENFORCE(false, "bad strict level: ", (int)strict);
ENFORCE(false, "bad strict level: {}", (int)strict);
}
}

Expand Down
2 changes: 1 addition & 1 deletion core/serialize/serialize.cc
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,7 @@ void SerializerImpl::pickle(Pickler &p, FileRef file, const unique_ptr<ast::Expr
pickleTree(p, file, a->typeAlias);
},

[&](ast::Expression *n) { Exception::raise("Unimplemented AST Node: ", n->nodeName()); });
[&](ast::Expression *n) { Exception::raise("Unimplemented AST Node: {}", n->nodeName()); });
}

unique_ptr<ast::Expression> SerializerImpl::unpickleExpr(serialize::UnPickler &p, GlobalState &gs, FileRef file) {
Expand Down
22 changes: 11 additions & 11 deletions core/types/calls.cc
Original file line number Diff line number Diff line change
Expand Up @@ -648,9 +648,9 @@ DispatchResult dispatchCallSymbol(Context ctx, DispatchArgs args,
}

if (args.block != nullptr) {
ENFORCE(!data->arguments().empty(), "Every symbol must at least have a block arg: " + data->show(ctx));
ENFORCE(!data->arguments().empty(), "Every symbol must at least have a block arg: {}", data->show(ctx));
SymbolRef bspec = data->arguments().back();
ENFORCE(bspec.data(ctx)->isBlockArgument(), "The last symbol must be the block arg: " + data->show(ctx));
ENFORCE(bspec.data(ctx)->isBlockArgument(), "The last symbol must be the block arg: {}", data->show(ctx));

TypePtr blockType = Types::resultTypeAsSeenFrom(ctx, bspec, symbol, targs);
if (!blockType) {
Expand Down Expand Up @@ -731,11 +731,11 @@ TypePtr getMethodArguments(Context ctx, SymbolRef klass, NameRef name, const vec
args.reserve(data->arguments().size());
for (auto arg : data->arguments()) {
if (arg.data(ctx)->isRepeated()) {
ENFORCE(args.empty(),
"getCallArguments with positional and repeated args is not supported: ", data->toString(ctx));
ENFORCE(args.empty(), "getCallArguments with positional and repeated args is not supported: {}",
data->toString(ctx));
return Types::arrayOf(ctx, Types::resultTypeAsSeenFrom(ctx, arg, klass, targs));
}
ENFORCE(!arg.data(ctx)->isKeyword(), "getCallArguments does not support kwargs: ", data->toString(ctx));
ENFORCE(!arg.data(ctx)->isKeyword(), "getCallArguments does not support kwargs: {}", data->toString(ctx));
if (arg.data(ctx)->isBlockArgument()) {
continue;
}
Expand Down Expand Up @@ -790,7 +790,7 @@ SymbolRef unwrapSymbol(const Type *type) {

[&](const ProxyType *proxy) { type = proxy->underlying().get(); },

[&](const Type *ty) { ENFORCE(false, "Unexpected type: ", ty->typeName()); });
[&](const Type *ty) { ENFORCE(false, "Unexpected type: {}", ty->typeName()); });
}
return result;
}
Expand Down Expand Up @@ -1326,14 +1326,14 @@ class Array_flatten : public IntrinsicMethod {
} else if (auto *tuple = cast_type<TupleType>(thisType)) {
element = tuple->elementType();
} else {
ENFORCE(false, "Array#flatten on unexpected type: ", args.selfType->show(ctx));
ENFORCE(false, "Array#flatten on unexpected type: {}", args.selfType->show(ctx));
}

int64_t depth = INT64_MAX;
if (args.args.size() == 1) {
auto argTyp = args.args[0]->type;
ENFORCE(args.locs.args.size() == 1,
"Mismatch between args.size() andargs.locs.args.size(): ", args.locs.args.size());
ENFORCE(args.locs.args.size() == 1, "Mismatch between args.size() and args.locs.args.size(): {}",
args.locs.args.size());
auto argLoc = args.locs.args[0];

auto lt = cast_type<LiteralType>(argTyp.get());
Expand All @@ -1352,7 +1352,7 @@ class Array_flatten : public IntrinsicMethod {
depth = INT64_MAX;
}
} else {
ENFORCE(args.args.empty(), "Array#flatten passed too many args: ", args.args.size());
ENFORCE(args.args.empty(), "Array#flatten passed too many args: {}", args.args.size());
}

return Types::arrayOf(ctx, recursivelyFlattenArrays(ctx, element, depth));
Expand All @@ -1370,7 +1370,7 @@ class Array_compact : public IntrinsicMethod {
} else if (auto *tuple = cast_type<TupleType>(thisType)) {
element = tuple->elementType();
} else {
ENFORCE(false, "Array#compact on unexpected type: ", args.selfType->show(ctx));
ENFORCE(false, "Array#compact on unexpected type: {}", args.selfType->show(ctx));
}
auto ret = Types::approximateSubtract(ctx, element, Types::nilClass());
return Types::arrayOf(ctx, ret);
Expand Down
18 changes: 9 additions & 9 deletions core/types/subtyping.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ TypePtr lubGround(Context ctx, const TypePtr &t1, const TypePtr &t2);

TypePtr Types::any(Context ctx, const TypePtr &t1, const TypePtr &t2) {
auto ret = lub(ctx, t1, t2);
ENFORCE(Types::isSubType(ctx, t1, ret), "\n" + ret->toString(ctx) + "\n is not a super type of \n" +
t1->toString(ctx) + "\n was lubbing with " + t2->toString(ctx));
ENFORCE(Types::isSubType(ctx, t2, ret), "\n" + ret->toString(ctx) + "\n is not a super type of \n" +
t2->toString(ctx) + "\n was lubbing with \n" + t1->toString(ctx));
ENFORCE(Types::isSubType(ctx, t1, ret), "\n{}\nis not a super type of\n{}\nwas lubbing with {}", ret->toString(ctx),
t1->toString(ctx), t2->toString(ctx));
ENFORCE(Types::isSubType(ctx, t2, ret), "\n{}\nis not a super type of\n{}\nwas lubbing with {}", ret->toString(ctx),
t2->toString(ctx), t1->toString(ctx));

// TODO: @dmitry, reenable
// ENFORCE(t1->hasUntyped() || t2->hasUntyped() || ret->hasUntyped() || // check if this test makes sense
Expand Down Expand Up @@ -524,11 +524,11 @@ TypePtr Types::all(Context ctx, const TypePtr &t1, const TypePtr &t2) {
auto ret = glb(ctx, t1, t2);
ret->sanityCheck(ctx);

ENFORCE(Types::isSubType(ctx, ret, t1), "\n" + ret->toString(ctx) + "\n is not a subtype of \n" +
t1->toString(ctx) + "\n was glbbing with \n" + t2->toString(ctx));
ENFORCE(Types::isSubType(ctx, ret, t1), "\n{}\nis not a subtype of\n{}\nwas glbbing with\n{}", ret->toString(ctx),
t1->toString(ctx), t2->toString(ctx));

ENFORCE(Types::isSubType(ctx, ret, t2), "\n" + glb(ctx, t1, t2)->toString(ctx) + "\n is not a subtype of \n" +
t2->toString(ctx) + "\n was glbbing with \n" + t1->toString(ctx));
ENFORCE(Types::isSubType(ctx, ret, t2), "\n{}\n is not a subtype of\n{}\nwas glbbing with\n{}", ret->toString(ctx),
t2->toString(ctx), t1->toString(ctx));
// TODO: @dmitry, reenable
// ENFORCE(t1->hasUntyped() || t2->hasUntyped() || ret->hasUntyped() || // check if this test makes sense
// !Types::isSubTypeUnderConstraint(ctx, t1, t2) || ret == t1 || ret->isUntyped(),
Expand Down Expand Up @@ -1140,7 +1140,7 @@ bool isSubTypeUnderConstraintSingle(Context ctx, TypeConstraint &constr, const T
return classSymbolIsAsGoodAs(ctx, c1->symbol, c2->symbol);
}
}
Exception::raise("isSubTypeUnderConstraint(", t1->typeName(), ", ", t2->typeName(), "): unreachable");
Exception::raise("isSubTypeUnderConstraint({}, {}): unreachable", t1->typeName(), t2->typeName());
}
}

Expand Down
6 changes: 3 additions & 3 deletions core/types/types.cc
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,9 @@ TypePtr Types::dropSubtypesOf(Context ctx, const TypePtr &from, SymbolRef klass)
}
},
[&](Type *) { result = from; });
ENFORCE(Types::isSubType(ctx, result, from), "dropSubtypesOf(" + from->toString(ctx) + "," +
klass.data(ctx)->showFullName(ctx) + ") returned " +
result->toString(ctx) + ", which is not a subtype of the input");
ENFORCE(Types::isSubType(ctx, result, from),
"dropSubtypesOf({}, {}) returned {}, which is not a subtype of the input", from->toString(ctx),
klass.data(ctx)->showFullName(ctx), result->toString(ctx));
return result;
}

Expand Down
2 changes: 1 addition & 1 deletion infer/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,7 @@ core::TypePtr Environment::processBinding(core::Context ctx, cfg::Binding &bind,
}
});

ENFORCE(tp.type.get() != nullptr, "Inferencer did not assign type: ", bind.value->toString(ctx));
ENFORCE(tp.type.get() != nullptr, "Inferencer did not assign type: {}", bind.value->toString(ctx));
tp.type->sanityCheck(ctx);

if (checkFullyDefined && !tp.type->isFullyDefined()) {
Expand Down
3 changes: 1 addition & 2 deletions main/autogen/autogen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -525,8 +525,7 @@ class MsgpackWriter {

static int assert_valid_version(int version) {
if (version < MIN_VERSION || version > MAX_VERSION) {
Exception::raise("msgpack version ", version, " not in available range [", MIN_VERSION, ",", MAX_VERSION,
"]");
Exception::raise("msgpack version {} not in available range [{}, {}]", version, MIN_VERSION, MAX_VERSION);
}
return version;
}
Expand Down
Loading

0 comments on commit e202d91

Please sign in to comment.