Skip to content

Commit

Permalink
Add error message when nominal type of returned struct mismatches ret…
Browse files Browse the repository at this point in the history 
…urn type.

#247

PiperOrigin-RevId: 658643913
  • Loading branch information
@richmckeever @copybara-github
richmckeever authored and copybara-github committed Aug 2, 2024
1 parent d97dc04 commit ee7624e
Show file tree
Hide file tree
Showing 11 changed files with 445 additions and 218 deletions.
1 change: 1 addition & 0 deletions xls/dslx/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ cc_library(
deps = [
":import_record",
":interp_value",
"//xls/dslx/frontend:ast",
"//xls/dslx/frontend:ast_node",
"//xls/dslx/frontend:pos",
"//xls/dslx/type_system:type",
Expand Down
29 changes: 23 additions & 6 deletions xls/dslx/errors.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,29 @@
#include "absl/strings/str_format.h"
#include "absl/strings/str_join.h"
#include "absl/types/span.h"
#include "xls/dslx/frontend/ast.h"
#include "xls/dslx/frontend/ast_node.h"
#include "xls/dslx/frontend/pos.h"
#include "xls/dslx/import_record.h"
#include "xls/dslx/interp_value.h"
#include "xls/dslx/type_system/type.h"

namespace xls::dslx {
namespace {

template <typename TypeOrAnnotation>
absl::Status TypeInferenceErrorStatusInternal(
const Span& span, const TypeOrAnnotation* type_or_annotation,
std::string_view message) {
std::string type_str;
if (type_or_annotation != nullptr) {
type_str = type_or_annotation->ToString() + " ";
}
return absl::InvalidArgumentError(absl::StrFormat(
"TypeInferenceError: %s %s%s", span.ToString(), type_str, message));
}

} // namespace

absl::Status ArgCountMismatchErrorStatus(const Span& span,
std::string_view message) {
Expand Down Expand Up @@ -57,12 +73,13 @@ absl::Status InvalidIdentifierErrorStatus(const Span& span,

absl::Status TypeInferenceErrorStatus(const Span& span, const Type* type,
std::string_view message) {
std::string type_str;
if (type != nullptr) {
type_str = type->ToString() + " ";
}
return absl::InvalidArgumentError(absl::StrFormat(
"TypeInferenceError: %s %s%s", span.ToString(), type_str, message));
return TypeInferenceErrorStatusInternal(span, type, message);
}

absl::Status TypeInferenceErrorStatusForAnnotation(
const Span& span, const TypeAnnotation* type_annotation,
std::string_view message) {
return TypeInferenceErrorStatusInternal(span, type_annotation, message);
}

absl::Status TypeMissingErrorStatus(const AstNode& node, const AstNode* user) {
Expand Down
8 changes: 8 additions & 0 deletions xls/dslx/errors.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include "absl/status/status.h"
#include "absl/types/span.h"
#include "xls/dslx/frontend/ast.h"
#include "xls/dslx/frontend/ast_node.h"
#include "xls/dslx/frontend/pos.h"
#include "xls/dslx/import_record.h"
Expand Down Expand Up @@ -48,6 +49,13 @@ absl::Status InvalidIdentifierErrorStatus(const Span& span,
absl::Status TypeInferenceErrorStatus(const Span& span, const Type* type,
std::string_view message);

// Variant of `TypeInferenceErrorStatus` for when there is only a
// `type_annotation` available, or its readability is better than that of the
// corresponding `Type` (e.g. due to erasure of parametric bindings).
absl::Status TypeInferenceErrorStatusForAnnotation(
const Span& span, const TypeAnnotation* type_annotation,
std::string_view message);

// Creates a TypeMissingError status value referencing the given node (which has
// its type missing) and user (which found that its type was missing).
absl::Status TypeMissingErrorStatus(const AstNode& node, const AstNode* user);
Expand Down
3 changes: 3 additions & 0 deletions xls/dslx/type_system/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -828,11 +828,14 @@ cc_library(
":typecheck_invocation",
":unwrap_meta_type",
":warn_on_defined_but_unused",
"@com_google_absl//absl/container:flat_hash_map",
"@com_google_absl//absl/log",
"@com_google_absl//absl/log:check",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/strings:str_format",
"//xls/common:casts",
"//xls/common/status:ret_check",
"//xls/common/status:status_macros",
"//xls/dslx:constexpr_evaluator",
Expand Down
194 changes: 93 additions & 101 deletions xls/dslx/type_system/deduce.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1362,107 +1362,6 @@ absl::StatusOr<std::unique_ptr<Type>> DeduceBuiltinTypeAnnotation(
return std::make_unique<MetaType>(std::move(t));
}

// Converts an AST expression in "dimension position" (e.g. in an array type
// annotation's size) and converts it into a TypeDim value that can be
// used in (e.g.) a ConcreteStruct. The result is either a constexpr-evaluated
// value or a ParametricSymbol (for a parametric binding that has not yet been
// defined).
//
// Note: this is not capable of expressing more complex ASTs -- it assumes
// something is either fully constexpr-evaluatable, or symbolic.
static absl::StatusOr<TypeDim> DimToConcreteUsize(const Expr* dim_expr,
DeduceCtx* ctx) {
std::unique_ptr<BitsType> u32 = BitsType::MakeU32();
auto validate_high_bit = [&u32](const Span& span, uint32_t value) {
if ((value >> 31) == 0) {
return absl::OkStatus();
}
return TypeInferenceErrorStatus(
span, u32.get(),
absl::StrFormat("Dimension value is too large, high bit is set: %#x; "
"was a negative number accidentally cast to a size?",
value));
};

// We allow numbers in dimension position to go without type annotations -- we
// implicitly make the type of the dimension u32, as we generally do with
// dimension values.
if (auto* number = dynamic_cast<const Number*>(dim_expr)) {
if (number->type_annotation() == nullptr) {
XLS_RETURN_IF_ERROR(TryEnsureFitsInBitsType(*number, *u32));
ctx->type_info()->SetItem(number, *u32);
} else {
XLS_ASSIGN_OR_RETURN(auto dim_type, ctx->Deduce(number));
if (*dim_type != *u32) {
return ctx->TypeMismatchError(
dim_expr->span(), nullptr, *dim_type, nullptr, *u32,
absl::StrFormat(
"Dimension %s must be a `u32` (soon to be `usize`, see "
"https://github.com/google/xls/issues/450 for details).",
dim_expr->ToString()));
}
}

XLS_ASSIGN_OR_RETURN(int64_t value, number->GetAsUint64());
const uint32_t value_u32 = static_cast<uint32_t>(value);
XLS_RET_CHECK_EQ(value, value_u32);

XLS_RETURN_IF_ERROR(validate_high_bit(number->span(), value_u32));

// No need to use the ConstexprEvaluator here. We've already got the goods.
// It'd have trouble anyway, since this number isn't type-decorated.
ctx->type_info()->NoteConstExpr(dim_expr, InterpValue::MakeU32(value_u32));
return TypeDim::CreateU32(value_u32);
}

// First we check that it's a u32 (in the future we'll want it to be a usize).
XLS_ASSIGN_OR_RETURN(std::unique_ptr<Type> dim_type, ctx->Deduce(dim_expr));
if (*dim_type != *u32) {
return ctx->TypeMismatchError(
dim_expr->span(), nullptr, *dim_type, nullptr, *u32,
absl::StrFormat(
"Dimension %s must be a `u32` (soon to be `usize`, see "
"https://github.com/google/xls/issues/450 for details).",
dim_expr->ToString()));
}

// Now we try to constexpr evaluate it.
const ParametricEnv parametric_env = ctx->GetCurrentParametricEnv();
VLOG(5) << "Attempting to evaluate dimension expression: `"
<< dim_expr->ToString() << "` via parametric env: " << parametric_env;
XLS_RETURN_IF_ERROR(ConstexprEvaluator::Evaluate(
ctx->import_data(), ctx->type_info(), ctx->warnings(), parametric_env,
dim_expr, dim_type.get()));
if (ctx->type_info()->IsKnownConstExpr(dim_expr)) {
XLS_ASSIGN_OR_RETURN(InterpValue constexpr_value,
ctx->type_info()->GetConstExpr(dim_expr));
XLS_ASSIGN_OR_RETURN(uint64_t int_value,
constexpr_value.GetBitValueViaSign());
uint32_t u32_value = static_cast<uint32_t>(int_value);
XLS_RETURN_IF_ERROR(validate_high_bit(dim_expr->span(), u32_value));
XLS_RET_CHECK_EQ(u32_value, int_value);
return TypeDim::CreateU32(u32_value);
}

// If there wasn't a known constexpr we could evaluate it to at this point, we
// attempt to turn it into a parametric expression.
absl::StatusOr<std::unique_ptr<ParametricExpression>> parametric_expr_or =
ExprToParametric(dim_expr, ctx);
if (parametric_expr_or.ok()) {
return TypeDim(std::move(parametric_expr_or).value());
}

VLOG(3) << "Could not convert dim expr to parametric expr; status: "
<< parametric_expr_or.status();

// If we can't evaluate it to a parametric expression we give an error.
return TypeInferenceErrorStatus(
dim_expr->span(), nullptr,
absl::StrFormat(
"Could not evaluate dimension expression `%s` to a constant value.",
dim_expr->ToString()));
}

// As above, but converts to a TypeDim value that is boolean.
static absl::StatusOr<TypeDim> DimToConcreteBool(const Expr* dim_expr,
DeduceCtx* ctx) {
Expand Down Expand Up @@ -2047,4 +1946,97 @@ absl::StatusOr<std::unique_ptr<Type>> DeduceAndResolve(const AstNode* node,
return Resolve(*deduced, ctx);
}

absl::StatusOr<TypeDim> DimToConcreteUsize(const Expr* dim_expr,
DeduceCtx* ctx) {
std::unique_ptr<BitsType> u32 = BitsType::MakeU32();
auto validate_high_bit = [&u32](const Span& span, uint32_t value) {
if ((value >> 31) == 0) {
return absl::OkStatus();
}
return TypeInferenceErrorStatus(
span, u32.get(),
absl::StrFormat("Dimension value is too large, high bit is set: %#x; "
"was a negative number accidentally cast to a size?",
value));
};

// We allow numbers in dimension position to go without type annotations -- we
// implicitly make the type of the dimension u32, as we generally do with
// dimension values.
if (auto* number = dynamic_cast<const Number*>(dim_expr)) {
if (number->type_annotation() == nullptr) {
XLS_RETURN_IF_ERROR(TryEnsureFitsInBitsType(*number, *u32));
ctx->type_info()->SetItem(number, *u32);
} else {
XLS_ASSIGN_OR_RETURN(auto dim_type, ctx->Deduce(number));
if (*dim_type != *u32) {
return ctx->TypeMismatchError(
dim_expr->span(), nullptr, *dim_type, nullptr, *u32,
absl::StrFormat(
"Dimension %s must be a `u32` (soon to be `usize`, see "
"https://github.com/google/xls/issues/450 for details).",
dim_expr->ToString()));
}
}

XLS_ASSIGN_OR_RETURN(int64_t value, number->GetAsUint64());
const uint32_t value_u32 = static_cast<uint32_t>(value);
XLS_RET_CHECK_EQ(value, value_u32);

XLS_RETURN_IF_ERROR(validate_high_bit(number->span(), value_u32));

// No need to use the ConstexprEvaluator here. We've already got the goods.
// It'd have trouble anyway, since this number isn't type-decorated.
ctx->type_info()->NoteConstExpr(dim_expr, InterpValue::MakeU32(value_u32));
return TypeDim::CreateU32(value_u32);
}

// First we check that it's a u32 (in the future we'll want it to be a usize).
XLS_ASSIGN_OR_RETURN(std::unique_ptr<Type> dim_type, ctx->Deduce(dim_expr));
if (*dim_type != *u32) {
return ctx->TypeMismatchError(
dim_expr->span(), nullptr, *dim_type, nullptr, *u32,
absl::StrFormat(
"Dimension %s must be a `u32` (soon to be `usize`, see "
"https://github.com/google/xls/issues/450 for details).",
dim_expr->ToString()));
}

// Now we try to constexpr evaluate it.
const ParametricEnv parametric_env = ctx->GetCurrentParametricEnv();
VLOG(5) << "Attempting to evaluate dimension expression: `"
<< dim_expr->ToString() << "` via parametric env: " << parametric_env;
XLS_RETURN_IF_ERROR(ConstexprEvaluator::Evaluate(
ctx->import_data(), ctx->type_info(), ctx->warnings(), parametric_env,
dim_expr, dim_type.get()));
if (ctx->type_info()->IsKnownConstExpr(dim_expr)) {
XLS_ASSIGN_OR_RETURN(InterpValue constexpr_value,
ctx->type_info()->GetConstExpr(dim_expr));
XLS_ASSIGN_OR_RETURN(uint64_t int_value,
constexpr_value.GetBitValueViaSign());
uint32_t u32_value = static_cast<uint32_t>(int_value);
XLS_RETURN_IF_ERROR(validate_high_bit(dim_expr->span(), u32_value));
XLS_RET_CHECK_EQ(u32_value, int_value);
return TypeDim::CreateU32(u32_value);
}

// If there wasn't a known constexpr we could evaluate it to at this point, we
// attempt to turn it into a parametric expression.
absl::StatusOr<std::unique_ptr<ParametricExpression>> parametric_expr_or =
ExprToParametric(dim_expr, ctx);
if (parametric_expr_or.ok()) {
return TypeDim(std::move(parametric_expr_or).value());
}

VLOG(3) << "Could not convert dim expr to parametric expr; status: "
<< parametric_expr_or.status();

// If we can't evaluate it to a parametric expression we give an error.
return TypeInferenceErrorStatus(
dim_expr->span(), nullptr,
absl::StrFormat(
"Could not evaluate dimension expression `%s` to a constant value.",
dim_expr->ToString()));
}

} // namespace xls::dslx
11 changes: 11 additions & 0 deletions xls/dslx/type_system/deduce.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,17 @@ absl::StatusOr<std::unique_ptr<Type>> Resolve(const Type& type, DeduceCtx* ctx);
absl::StatusOr<std::unique_ptr<Type>> DeduceAndResolve(const AstNode* node,
DeduceCtx* ctx);

// Converts an AST expression in "dimension position" (e.g. in an array type
// annotation's size) and converts it into a `TypeDim` value that can be used
// in, e.g., an `ArrayType`. The result is either a constexpr-evaluated value or
// a `ParametricExpression` (for a parametric binding that has not yet been
// defined).
//
// Note: this is not capable of evaluating more complex ASTs; it assumes
// something is either fully constexpr-evaluatable, or symbolic.
absl::StatusOr<TypeDim> DimToConcreteUsize(const Expr* dim_expr,
DeduceCtx* ctx);

} // namespace xls::dslx

#endif // XLS_DSLX_TYPE_SYSTEM_DEDUCE_H_
26 changes: 20 additions & 6 deletions xls/dslx/type_system/parametric_instantiator_internal.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ namespace internal {
namespace {

// Resolves possibly-parametric type 'annotated' via 'parametric_env_map'.
absl::StatusOr<std::unique_ptr<Type>> Resolve(
absl::StatusOr<std::unique_ptr<Type>> ResolveInternal(
const Type& annotated,
const absl::flat_hash_map<std::string, InterpValue>& parametric_env_map) {
XLS_ASSIGN_OR_RETURN(
Expand All @@ -78,6 +78,19 @@ absl::StatusOr<std::unique_ptr<Type>> Resolve(
ToParametricEnv(ParametricEnv(parametric_env_map)));
return TypeDim(std::move(evaluated));
}));
if (!parametric_env_map.empty()) {
// We need to add nominal dims upon parametric instantiation, in order for
// e.g. the return type of `bar` to get 8 rather than a meaningless `N` as
// the nominal dim in:
// `struct S<N:u32> { ... }
// fn foo<N: u32>() -> S<N> { ... }
// fn bar() -> S<u32:8> { foo<u32:8>() }`
absl::flat_hash_map<std::string, TypeDim> nominal_dims;
for (const auto& [key, interp_value] : parametric_env_map) {
nominal_dims.emplace(key, TypeDim(interp_value));
}
resolved = resolved->AddNominalTypeDims(std::move(nominal_dims));
}
VLOG(5) << "Resolved " << annotated.ToString() << " to "
<< resolved->ToString();
return resolved;
Expand Down Expand Up @@ -178,7 +191,8 @@ absl::Status EagerlyPopulateParametricEnvMap(
// it, and ensure it's set as the resolved type in the type info before
// interpretation.
XLS_ASSIGN_OR_RETURN(std::unique_ptr<Type> expr_type, ctx->Deduce(expr));
XLS_ASSIGN_OR_RETURN(expr_type, Resolve(*expr_type, parametric_env_map));
XLS_ASSIGN_OR_RETURN(expr_type,
ResolveInternal(*expr_type, parametric_env_map));
XLS_RET_CHECK(!expr_type->HasParametricDims()) << expr_type->ToString();
ctx->type_info()->SetItem(expr, *expr_type);

Expand Down Expand Up @@ -406,7 +420,7 @@ absl::StatusOr<TypeAndParametricEnv> FunctionInstantiator::Instantiate() {
const Type& param_type = *param_types_[i];
const Type& arg_type = *args()[i].type();
XLS_ASSIGN_OR_RETURN(std::unique_ptr<Type> instantiated_param_type,
Resolve(param_type, parametric_env_map()));
ResolveInternal(param_type, parametric_env_map()));
if (*instantiated_param_type != arg_type) {
// Although it's not the *original* parameter (which could be a little
// confusing to the user) we want to show what the mismatch was directly,
Expand All @@ -422,7 +436,7 @@ absl::StatusOr<TypeAndParametricEnv> FunctionInstantiator::Instantiate() {
// Resolve the return type according to the bindings we collected.
const Type& orig = function_type_->return_type();
XLS_ASSIGN_OR_RETURN(std::unique_ptr<Type> resolved,
Resolve(orig, parametric_env_map()));
ResolveInternal(orig, parametric_env_map()));
VLOG(5) << "Resolved return type from " << orig.ToString() << " to "
<< resolved->ToString();

Expand Down Expand Up @@ -470,7 +484,7 @@ absl::StatusOr<TypeAndParametricEnv> StructInstantiator::Instantiate() {
const Type& member_type = *member_types_[i];
const Type& arg_type = *args()[i].type();
XLS_ASSIGN_OR_RETURN(std::unique_ptr<Type> instantiated_member_type,
Resolve(member_type, parametric_env_map()));
ResolveInternal(member_type, parametric_env_map()));
if (*instantiated_member_type != arg_type) {
return ctx().TypeMismatchError(
args()[i].span(), nullptr, *instantiated_member_type, nullptr,
Expand All @@ -479,7 +493,7 @@ absl::StatusOr<TypeAndParametricEnv> StructInstantiator::Instantiate() {
}

XLS_ASSIGN_OR_RETURN(std::unique_ptr<Type> resolved,
Resolve(*struct_type_, parametric_env_map()));
ResolveInternal(*struct_type_, parametric_env_map()));
return TypeAndParametricEnv{
.type = std::move(resolved),
.parametric_env = ParametricEnv(parametric_env_map())};
Expand Down
Loading

0 comments on commit ee7624e

Please sign in to comment.