Skip to content

Commit

Permalink
Update xls/dslx subpackage to follow updated StatusOr style guidance at
Browse files Browse the repository at this point in the history
  • Loading branch information
mikex-oss authored and copybara-github committed Dec 13, 2024
1 parent d9bc80f commit 186af57
Show file tree
Hide file tree
Showing 12 changed files with 266 additions and 271 deletions.
16 changes: 9 additions & 7 deletions xls/dslx/bytecode/bytecode_emitter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -233,12 +233,12 @@ BytecodeEmitter::EmitExpression(

absl::Status BytecodeEmitter::HandleArray(const Array* node) {
if (type_info_->IsKnownConstExpr(node)) {
auto const_expr_or = type_info_->GetConstExpr(node);
XLS_RET_CHECK_OK(const_expr_or.status());
absl::StatusOr<InterpValue> const_expr = type_info_->GetConstExpr(node);
XLS_RET_CHECK_OK(const_expr.status());
VLOG(5) << absl::StreamFormat(
"BytecodeEmitter::HandleArray; node %s is known constexpr: %s",
node->ToString(), const_expr_or.value().ToString());
Add(Bytecode::MakeLiteral(node->span(), const_expr_or.value()));
node->ToString(), const_expr->ToString());
Add(Bytecode::MakeLiteral(node->span(), *const_expr));
return absl::OkStatus();
}

Expand Down Expand Up @@ -998,8 +998,10 @@ absl::Status BytecodeEmitter::HandleIndex(const Index* node) {
}
Type* type = maybe_type.value();
// These will never fail.
absl::StatusOr<TypeDim> dim_or = type->GetTotalBitCount();
absl::StatusOr<int64_t> width_or = dim_or.value().GetAsInt64();
absl::StatusOr<TypeDim> dim = type->GetTotalBitCount();
CHECK_OK(dim);
absl::StatusOr<int64_t> width = dim->GetAsInt64();
CHECK_OK(width);

int64_t limit_width;
if (slice->start() == nullptr) {
Expand All @@ -1013,7 +1015,7 @@ absl::Status BytecodeEmitter::HandleIndex(const Index* node) {
}
bytecode_.push_back(
Bytecode(node->span(), Bytecode::Op::kLiteral,
InterpValue::MakeSBits(limit_width, width_or.value())));
InterpValue::MakeSBits(limit_width, *width)));
} else {
XLS_RETURN_IF_ERROR(slice->limit()->AcceptExpr(this));
}
Expand Down
7 changes: 3 additions & 4 deletions xls/dslx/dslx_builtins_signatures.cc
Original file line number Diff line number Diff line change
Expand Up @@ -350,16 +350,15 @@ class Checker {
return *this;
}

absl::StatusOr<bool> is_signed_or = bits_like->is_signed.GetAsBool();
if (!is_signed_or.ok()) {
absl::StatusOr<bool> is_signed = bits_like->is_signed.GetAsBool();
if (!is_signed.ok()) {
status_ = TypeInferenceErrorStatus(
span_, &t,
absl::StrCat("Could not determine signedness of argument ", argno,
"; got ", t.ToString()));
return *this;
}
bool is_signed = is_signed_or.value();
if (!is_signed) {
if (!*is_signed) {
status_ = TypeInferenceErrorStatus(
span_, &t,
absl::StrFormat("Want argument %d to be signed bits; got %s", argno,
Expand Down
12 changes: 6 additions & 6 deletions xls/dslx/frontend/ast_cloner_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1522,15 +1522,15 @@ proc MyProc {
})";

FileTable file_table;
absl::StatusOr<std::unique_ptr<Module>> module_or =
absl::StatusOr<std::unique_ptr<Module>> module =
ParseModule(kProgram, "fake_path.x", "the_module", file_table);
if (!module_or.ok()) {
if (!module.ok()) {
UniformContentFilesystem vfs(kProgram);
TryPrintError(module_or.status(), file_table, vfs);
TryPrintError(module.status(), file_table, vfs);
}
XLS_ASSERT_OK(module_or.status());
Module* module = module_or.value().get();
XLS_ASSERT_OK_AND_ASSIGN(std::unique_ptr<Module> clone, CloneModule(*module));
XLS_ASSERT_OK(module.status());
XLS_ASSERT_OK_AND_ASSIGN(std::unique_ptr<Module> clone,
CloneModule(*module->get()));
EXPECT_EQ(kExpected, clone->ToString());
}

Expand Down
51 changes: 26 additions & 25 deletions xls/dslx/frontend/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2097,9 +2097,9 @@ absl::StatusOr<Expr*> Parser::ParseTermRhs(Expr* lhs, Bindings& outer_bindings,
// this colon ----------^
if (peek->kind() == TokenKind::kColon) {
DropTokenOrDie();
absl::StatusOr<TypeDefinition> type_definition_or =
absl::StatusOr<TypeDefinition> type_definition =
ToTypeDefinition(lhs);
if (!type_definition_or.ok()) {
if (!type_definition.ok()) {
const Span error_span(lhs->span().start(), index->span().limit());
return ParseErrorStatus(
error_span,
Expand All @@ -2110,8 +2110,7 @@ absl::StatusOr<Expr*> Parser::ParseTermRhs(Expr* lhs, Bindings& outer_bindings,
}
// TODO(rspringer): We can't currently support parameterized
// ColonRef-to-types with this function structure.
auto* type_ref =
module_->Make<TypeRef>(span, type_definition_or.value());
auto* type_ref = module_->Make<TypeRef>(span, *type_definition);
auto* type_ref_type = module_->Make<TypeRefTypeAnnotation>(
span, type_ref, /*parametrics=*/std::vector<ExprOrType>());
auto* array_type =
Expand Down Expand Up @@ -2714,33 +2713,35 @@ absl::StatusOr<ModuleMember> Parser::ParseProcLike(const Pos& start_pos,
} else if (peek->IsIdentifier("config")) {
XLS_RETURN_IF_ERROR(check_not_yet_specified(proc_like_body.config, peek));

// We make a more specific/helpful error message when you try to refer to
// a name that the config function is supposed to define from within the
// config function.
auto specialize_config_name_error =
[proc_like_body](const absl::Status& status) {
xabsl::StatusBuilder builder(status);
std::optional<std::string_view> bad_name =
MaybeExtractParseNameError(status);
if (bad_name.has_value() &&
HasMemberNamed(proc_like_body, bad_name.value())) {
builder << absl::StreamFormat(
"\"%s\" is a proc member, but those cannot be referenced "
"from within a proc config function.",
bad_name.value());
}
return builder;
};

// Note: the config function does not have access to the proc members,
// because that's what it is defining. It does, however, have access to
// type aliases and similar. As a result, we use `proc_bindings` instead
// of `member_bindings` as the base bindings here.
Bindings this_bindings(&proc_bindings);
absl::StatusOr<Function*> config_or = ParseProcConfig(
this_bindings, parametric_bindings, proc_like_body.members,
name_def->identifier(), is_public);

// We make a more specific/helpful error message when you try to refer to
// a name that the config function is supposed to define from within the
// config function.
if (std::optional<std::string_view> bad_name =
MaybeExtractParseNameError(config_or.status());
bad_name.has_value() &&
HasMemberNamed(proc_like_body, bad_name.value())) {
xabsl::StatusBuilder builder(config_or.status());
builder << absl::StreamFormat(
"\"%s\" is a proc member, "
"but those cannot be referenced "
"from within a proc config function.",
bad_name.value());
return builder;
}
XLS_ASSIGN_OR_RETURN(Function * config,
ParseProcConfig(this_bindings, parametric_bindings,
proc_like_body.members,
name_def->identifier(), is_public),
_.With(specialize_config_name_error));

XLS_RETURN_IF_ERROR(config_or.status());
Function* config = config_or.value();
proc_like_body.config = config;

// TODO(https://github.com/google/xls/issues/1029): 2024-03-25 this is a
Expand Down
Loading

0 comments on commit 186af57

Please sign in to comment.