diff --git a/xls/dslx/bytecode/bytecode_emitter.cc b/xls/dslx/bytecode/bytecode_emitter.cc index a4d103a47c..50fca1520d 100644 --- a/xls/dslx/bytecode/bytecode_emitter.cc +++ b/xls/dslx/bytecode/bytecode_emitter.cc @@ -881,10 +881,6 @@ absl::Status BytecodeEmitter::HandleConstAssert(const ConstAssert* node) { return absl::OkStatus(); } -absl::Status BytecodeEmitter::HandleConstantArray(const ConstantArray* node) { - return HandleArray(node); -} - absl::Status BytecodeEmitter::HandleConstRef(const ConstRef* node) { return HandleNameRef(node); } diff --git a/xls/dslx/bytecode/bytecode_emitter.h b/xls/dslx/bytecode/bytecode_emitter.h index fb1ad8aa9b..76ac6ef7d3 100644 --- a/xls/dslx/bytecode/bytecode_emitter.h +++ b/xls/dslx/bytecode/bytecode_emitter.h @@ -91,7 +91,6 @@ class BytecodeEmitter : public ExprVisitor { absl::Status HandleColonRef(const ColonRef* node) override; absl::StatusOr HandleColonRefInternal(const ColonRef* node); absl::Status HandleConstAssert(const ConstAssert* node) override; - absl::Status HandleConstantArray(const ConstantArray* node) override; absl::Status HandleConstRef(const ConstRef* node) override; absl::Status HandleFor(const For* node) override; absl::Status HandleFormatMacro(const FormatMacro* node) override; diff --git a/xls/dslx/constexpr_evaluator.cc b/xls/dslx/constexpr_evaluator.cc index 8585a6c5dd..a8684b41bf 100644 --- a/xls/dslx/constexpr_evaluator.cc +++ b/xls/dslx/constexpr_evaluator.cc @@ -397,11 +397,6 @@ absl::Status ConstexprEvaluator::HandleColonRef(const ColonRef* expr) { subject); } -absl::Status ConstexprEvaluator::HandleConstantArray( - const ConstantArray* expr) { - return HandleArray(expr); -} - absl::Status ConstexprEvaluator::HandleConstAssert( const ConstAssert* const_assert) { const FileTable& file_table = *const_assert->owner()->file_table(); diff --git a/xls/dslx/constexpr_evaluator.h b/xls/dslx/constexpr_evaluator.h index 8a4f7a106c..a58da9e2b8 100644 --- a/xls/dslx/constexpr_evaluator.h +++ b/xls/dslx/constexpr_evaluator.h @@ -73,7 +73,6 @@ class ConstexprEvaluator : public xls::dslx::ExprVisitor { absl::Status HandleChannelDecl(const ChannelDecl* expr) override; absl::Status HandleColonRef(const ColonRef* expr) override; absl::Status HandleConstAssert(const ConstAssert* const_assert) override; - absl::Status HandleConstantArray(const ConstantArray* expr) override; absl::Status HandleConstRef(const ConstRef* expr) override; absl::Status HandleFor(const For* expr) override; absl::Status HandleFunctionRef(const FunctionRef* expr) override; diff --git a/xls/dslx/frontend/ast.cc b/xls/dslx/frontend/ast.cc index 592b2abad3..f3a9fa0102 100644 --- a/xls/dslx/frontend/ast.cc +++ b/xls/dslx/frontend/ast.cc @@ -728,19 +728,6 @@ Array::Array(Module* owner, Span span, std::vector members, members_(std::move(members)), has_ellipsis_(has_ellipsis) {} -ConstantArray::ConstantArray(Module* owner, Span span, - std::vector members, bool has_ellipsis, - bool in_parens) - : Array(owner, std::move(span), std::move(members), has_ellipsis, - in_parens) { - for (Expr* expr : this->members()) { - CHECK(IsConstant(expr)) - << "non-constant in constant array: " << expr->ToString(); - } -} - -ConstantArray::~ConstantArray() = default; - // -- class TypeRef TypeRef::TypeRef(Module* owner, Span span, TypeDefinition type_definition) @@ -913,7 +900,7 @@ SelfTypeAnnotation::~SelfTypeAnnotation() = default; BuiltinNameDef::~BuiltinNameDef() = default; bool IsConstant(AstNode* node) { - if (IsOneOf(node)) { + if (IsOneOf(node)) { return true; } if (IsOneOf(node)) { diff --git a/xls/dslx/frontend/ast.h b/xls/dslx/frontend/ast.h index 40c6ead55d..3211a975a3 100644 --- a/xls/dslx/frontend/ast.h +++ b/xls/dslx/frontend/ast.h @@ -54,7 +54,6 @@ X(Cast) \ X(ChannelDecl) \ X(ColonRef) \ - X(ConstantArray) \ X(ConstRef) \ X(For) \ X(FormatMacro) \ @@ -1116,7 +1115,7 @@ class TypeAlias : public AstNode { }; // Represents an array expression; e.g. `[a, b, c]`. -class Array : public Expr { +class Array final : public Expr { public: Array(Module* owner, Span span, std::vector members, bool has_ellipsis, bool in_parens = false); @@ -1168,22 +1167,6 @@ class Array : public Expr { bool has_ellipsis_; }; -// A constant array expression is an array expression where all of the -// expressions contained within it are constant. -class ConstantArray : public Array { - public: - // Adds checking for constant-expression-ness of the members beyond - // Array::Array. - ConstantArray(Module* owner, Span span, std::vector members, - bool has_ellipsis, bool in_parens = false); - - ~ConstantArray() override; - - absl::Status Accept(AstNodeVisitor* v) const override { - return v->HandleConstantArray(this); - } -}; - // Several different AST nodes define types that can be referred to by a // TypeRef. using TypeDefinition = @@ -3406,10 +3389,8 @@ class VerbatimNode : public Expr { std::string text_; }; -// Helper for determining whether an AST node is constant (e.g., can be -// considered a constant value in a ConstantArray). In general a node -// with no children is considered constant, but there are some exceptions -// (e.g., NameRef). +// Helper for determining whether an AST node is constant (e.g., clearly can be +// considered a constant value before type checking). bool IsConstant(AstNode* n); } // namespace xls::dslx diff --git a/xls/dslx/frontend/ast_cloner.cc b/xls/dslx/frontend/ast_cloner.cc index 1377984abb..07d658c945 100644 --- a/xls/dslx/frontend/ast_cloner.cc +++ b/xls/dslx/frontend/ast_cloner.cc @@ -241,23 +241,6 @@ class AstCloner : public AstNodeVisitor { return absl::OkStatus(); } - absl::Status HandleConstantArray(const ConstantArray* n) override { - XLS_RETURN_IF_ERROR(VisitChildren(n)); - - std::vector new_members; - for (const Expr* old_member : n->members()) { - new_members.push_back(down_cast(old_to_new_.at(old_member))); - } - ConstantArray* new_array = module_->Make( - n->span(), new_members, n->has_ellipsis(), n->in_parens()); - if (n->type_annotation() != nullptr) { - new_array->set_type_annotation( - down_cast(old_to_new_.at(n->type_annotation()))); - } - old_to_new_[n] = new_array; - return absl::OkStatus(); - } - absl::Status HandleConstantDef(const ConstantDef* n) override { XLS_RETURN_IF_ERROR(VisitChildren(n)); TypeAnnotation* new_type_annotation = nullptr; diff --git a/xls/dslx/frontend/ast_cloner_test.cc b/xls/dslx/frontend/ast_cloner_test.cc index 0262671849..e753618544 100644 --- a/xls/dslx/frontend/ast_cloner_test.cc +++ b/xls/dslx/frontend/ast_cloner_test.cc @@ -655,7 +655,7 @@ fn main() -> foo::BAR { EXPECT_EQ(kProgram, clone->ToString()); } -TEST(AstClonerTest, ConstantArray) { +TEST(AstClonerTest, IotaArray) { constexpr std::string_view kProgram = R"(const ARRAY_SIZE = uN[32]:5; fn main() -> u32[ARRAY_SIZE] { ([u32:0, u32:1, u32:2, u32:3, u32:4]) diff --git a/xls/dslx/frontend/ast_test.cc b/xls/dslx/frontend/ast_test.cc index 7b5b21ad7a..2374a4895a 100644 --- a/xls/dslx/frontend/ast_test.cc +++ b/xls/dslx/frontend/ast_test.cc @@ -319,6 +319,9 @@ TEST(AstTest, IsConstantNumber) { EXPECT_TRUE(IsConstant(number)); } +// Tests the IsConstant predicate on an array with a single `name` reference. +// +// Since the name reference is not constant, the array is not constant. TEST(AstTest, IsConstantArrayOfNameRefs) { FileTable file_table; const Span fake_span; @@ -332,6 +335,8 @@ TEST(AstTest, IsConstantArrayOfNameRefs) { EXPECT_FALSE(IsConstant(array)); } +// Tests the IsConstant predicate on an array with a single `number` literal. +// Since the number literal is constant, the array is constant. TEST(AstTest, IsConstantArrayOfNumbers) { FileTable file_table; const Span fake_span; @@ -344,6 +349,8 @@ TEST(AstTest, IsConstantArrayOfNumbers) { EXPECT_TRUE(IsConstant(array)); } +// Tests the IsConstant predicate on an empty array. +// Since there are no members, the array is constant. TEST(AstTest, IsConstantEmptyArray) { FileTable file_table; const Span fake_span; diff --git a/xls/dslx/frontend/parser.cc b/xls/dslx/frontend/parser.cc index ad076f7ee0..9ed48a97e6 100644 --- a/xls/dslx/frontend/parser.cc +++ b/xls/dslx/frontend/parser.cc @@ -1217,10 +1217,6 @@ absl::StatusOr Parser::ParseArray(Bindings& bindings) { } Span span(start_tok.span().start(), cbrack_pos); - if (std::all_of(exprs.begin(), exprs.end(), IsConstant)) { - return module_->Make(span, std::move(exprs), - has_trailing_ellipsis); - } return module_->Make(span, std::move(exprs), has_trailing_ellipsis); } @@ -1272,11 +1268,18 @@ absl::StatusOr Parser::ParseCast(Bindings& bindings, return term; } + // This handles the case where we're trying to make a tuple constant that has + // an explicitly annotated type; e.g. + // + // ```dslx + // const MY_TUPLE = (u32, u64):(u32:32, u64:64); + // ``` if (auto* tuple = dynamic_cast(term); tuple != nullptr && std::all_of(tuple->members().begin(), tuple->members().end(), IsConstant)) { return term; } + return ParseErrorStatus( type->span(), "Old-style cast only permitted for constant arrays/tuples " diff --git a/xls/dslx/frontend/parser_test.cc b/xls/dslx/frontend/parser_test.cc index e0aa055048..65b55bb137 100644 --- a/xls/dslx/frontend/parser_test.cc +++ b/xls/dslx/frontend/parser_test.cc @@ -2060,7 +2060,7 @@ TEST_F(ParserTest, TupleArrayAndInt) { auto* tuple = dynamic_cast(e); EXPECT_EQ(2, tuple->members().size()); auto* array = tuple->members()[0]; - EXPECT_NE(dynamic_cast(array), nullptr); + EXPECT_NE(dynamic_cast(array), nullptr); } TEST_F(ParserTest, Cast) { RoundTripExpr("foo() as u32", {"foo"}); } @@ -2424,9 +2424,9 @@ fn f(a: MyStruct) -> u32 { })"); } -TEST_F(ParserTest, ConstantArray) { +TEST_F(ParserTest, ArrayOfConstantNumberLiterals) { Expr* e = RoundTripExpr("u32[2]:[0, 1]", {}, false, std::nullopt); - ASSERT_TRUE(dynamic_cast(e) != nullptr); + ASSERT_TRUE(dynamic_cast(e) != nullptr); } TEST_F(ParserTest, MixedArrayIsNotConstantArray) { @@ -2447,9 +2447,9 @@ const b = u32[2]:[a, a];)"); ASSERT_TRUE(dynamic_cast(array) != nullptr); } -TEST_F(ParserTest, EmptyArrayIsConstant) { +TEST_F(ParserTest, EmptyArray) { Expr* e = RoundTripExpr("u32[2]:[]", {}, false, std::nullopt); - ASSERT_TRUE(dynamic_cast(e) != nullptr); + ASSERT_TRUE(dynamic_cast(e) != nullptr); } TEST_F(ParserTest, DoubleNegation) { RoundTripExpr("!!x", {"x"}, false); } diff --git a/xls/dslx/ir_convert/extract_conversion_order.cc b/xls/dslx/ir_convert/extract_conversion_order.cc index 76059a5190..77776d72fd 100644 --- a/xls/dslx/ir_convert/extract_conversion_order.cc +++ b/xls/dslx/ir_convert/extract_conversion_order.cc @@ -236,10 +236,6 @@ class InvocationVisitor : public ExprVisitor { return absl::OkStatus(); } - absl::Status HandleConstantArray(const ConstantArray* expr) override { - return HandleArray(expr); - } - absl::Status HandleFor(const For* expr) override { XLS_RETURN_IF_ERROR(expr->init()->AcceptExpr(this)); XLS_RETURN_IF_ERROR(expr->iterable()->AcceptExpr(this)); diff --git a/xls/dslx/ir_convert/function_converter.cc b/xls/dslx/ir_convert/function_converter.cc index 1711876328..8711beb7b2 100644 --- a/xls/dslx/ir_convert/function_converter.cc +++ b/xls/dslx/ir_convert/function_converter.cc @@ -314,7 +314,6 @@ class FunctionConverterVisitor : public AstNodeVisitor { NO_TRAVERSE_DISPATCH_VISIT(Attr) NO_TRAVERSE_DISPATCH_VISIT(Array) NO_TRAVERSE_DISPATCH_VISIT(StatementBlock) - NO_TRAVERSE_DISPATCH_VISIT(ConstantArray) NO_TRAVERSE_DISPATCH_VISIT(Cast) NO_TRAVERSE_DISPATCH_VISIT(ColonRef) NO_TRAVERSE_DISPATCH_VISIT(ConstantDef) @@ -3699,14 +3698,6 @@ absl::StatusOr FunctionConverter::GetConstBits( return value.GetBitsWithStatus(); } -absl::Status FunctionConverter::HandleConstantArray(const ConstantArray* node) { - // Note: previously we would force constant evaluation here, but because all - // constexprs should be evaluated during typechecking, we shouldn't need to - // forcibly do constant evaluation at IR conversion time; therefore, we just - // build BValues and let XLS opt constant fold them. - return HandleArray(node); -} - absl::StatusOr FunctionConverter::ResolveTypeToIr( const AstNode* node) { XLS_ASSIGN_OR_RETURN(std::unique_ptr type, ResolveType(node)); diff --git a/xls/dslx/ir_convert/function_converter.h b/xls/dslx/ir_convert/function_converter.h index 8ac4baadb6..f58d2a967c 100644 --- a/xls/dslx/ir_convert/function_converter.h +++ b/xls/dslx/ir_convert/function_converter.h @@ -333,7 +333,6 @@ class FunctionConverter { absl::Status HandleStatementBlock(const StatementBlock* node); absl::Status HandleCast(const Cast* node); absl::Status HandleColonRef(const ColonRef* node); - absl::Status HandleConstantArray(const ConstantArray* node); absl::Status HandleConstantDef(const ConstantDef* node); absl::Status HandleFor(const For* node); absl::Status HandleFormatMacro(const FormatMacro* node); diff --git a/xls/dslx/type_system/deduce.cc b/xls/dslx/type_system/deduce.cc index 160750b196..5303446084 100644 --- a/xls/dslx/type_system/deduce.cc +++ b/xls/dslx/type_system/deduce.cc @@ -2030,7 +2030,6 @@ class DeduceVisitor : public AstNodeVisitor { DEDUCE_DISPATCH(Attr, DeduceAttr) DEDUCE_DISPATCH(StatementBlock, DeduceStatementBlock) DEDUCE_DISPATCH(ChannelDecl, DeduceChannelDecl) - DEDUCE_DISPATCH(ConstantArray, DeduceConstantArray) DEDUCE_DISPATCH(ColonRef, DeduceColonRef) DEDUCE_DISPATCH(Index, DeduceIndex) DEDUCE_DISPATCH(Match, DeduceMatch) diff --git a/xls/dslx/type_system/deduce_expr.cc b/xls/dslx/type_system/deduce_expr.cc index 89af50110d..b345a28645 100644 --- a/xls/dslx/type_system/deduce_expr.cc +++ b/xls/dslx/type_system/deduce_expr.cc @@ -47,9 +47,27 @@ #include "xls/ir/bits.h" namespace xls::dslx { +namespace { + +// Notes that `type` is the type of `node` in the type information and notes the +// constexpr value for the literal number in the type info. +absl::Status NoteTypeForNumber(const Number& node, const Type& type, + DeduceCtx* ctx) { + ctx->type_info()->SetItem(&node, type); + // We can't do any constexpr evaluation until parametric dims have been + // resolved. + if (type.HasParametricDims()) { + return absl::OkStatus(); + } + XLS_ASSIGN_OR_RETURN(InterpValue value, EvaluateNumber(node, type)); + ctx->type_info()->NoteConstExpr(&node, value); + return absl::OkStatus(); +} -static absl::StatusOr> DeduceEmptyArray(const Array* node, - DeduceCtx* ctx) { +} // namespace + +static absl::StatusOr> DeduceEmptyArray( + const Array* node, DeduceCtx* ctx) { // We cannot have an array that is just an ellipsis, ellipsis indicates we // should replicate the last member. if (node->has_ellipsis()) { @@ -95,17 +113,82 @@ static absl::StatusOr> DeduceEmptyArray(const Array* node, ctx->file_table()); } - return annotated; + return absl::WrapUnique(dynamic_cast(annotated.release())); } -absl::StatusOr> DeduceArray(const Array* node, - DeduceCtx* ctx) { +// Note: returns nullptr if there is no type annotation. +static absl::StatusOr> DeduceArrayTypeAnnotation( + const Array* node, DeduceCtx* ctx) { + if (node->type_annotation() == nullptr) { + return nullptr; + } + + XLS_ASSIGN_OR_RETURN(std::unique_ptr annotated, + ctx->Deduce(node->type_annotation())); + XLS_ASSIGN_OR_RETURN( + annotated, + UnwrapMetaType(std::move(annotated), node->span(), + "array type-prefix position", ctx->file_table())); + VLOG(5) << absl::StreamFormat( + "DeduceArray; inferred type annotation `%s` to be `%s`", + node->type_annotation()->ToString(), annotated->ToString()); + + // It must be an array! + auto* array_type = dynamic_cast(annotated.get()); + if (array_type == nullptr) { + return TypeInferenceErrorStatus( + node->span(), annotated.get(), + "Array was not annotated with an array type.", ctx->file_table()); + } + + if (array_type->HasParametricDims()) { + return TypeInferenceErrorStatus( + node->type_annotation()->span(), array_type, + absl::StrFormat("Annotated type for array " + "literal must be constexpr; type has dimensions that " + "cannot be resolved."), + ctx->file_table()); + } + + return absl::WrapUnique(dynamic_cast(annotated.release())); +} + +// Implemention note: there are a few cases to handle in appropriate order: +// - empty arrays we handle independently +// - if there is a type annotation that is not appropriate, we want to report +// that first instead of reporting about problems in the contents of the array +// - then we report problems with the contents of the array as normal +// +// As a user convenience, if the array type is annotated and there are bare +// numbers inside the array, we propagate the element type to the literal +// numbers. This will become more generalized with type inference 2.0. +static absl::StatusOr> DeduceArrayInternal( + const Array* node, DeduceCtx* ctx) { VLOG(5) << "DeduceArray; node: " << node->ToString(); if (node->members().empty()) { return DeduceEmptyArray(node, ctx); } + // Note: `annotated` may be nullptr. + XLS_ASSIGN_OR_RETURN(std::unique_ptr annotated, + DeduceArrayTypeAnnotation(node, ctx)); + + if (annotated != nullptr) { + // If the array type is annotated, as a user convenience, we propagate the + // element type to bare (unannotated) literal numbers contained in the + // array. + for (Expr* member : node->members()) { + if (auto* number = dynamic_cast(member); + number != nullptr && number->type_annotation() == nullptr) { + XLS_RETURN_IF_ERROR( + TryEnsureFitsInType(*number, annotated->element_type())); + XLS_RETURN_IF_ERROR( + NoteTypeForNumber(*number, annotated->element_type(), ctx)); + } + } + } + std::vector> member_types; for (Expr* member : node->members()) { XLS_ASSIGN_OR_RETURN(std::unique_ptr member_type, @@ -138,7 +221,7 @@ absl::StatusOr> DeduceArray(const Array* node, std::unique_ptr inferred = std::make_unique( member_types[0]->CloneToUnique(), member_types_dim); - if (node->type_annotation() == nullptr) { + if (annotated == nullptr) { if (node->has_ellipsis()) { const Span array_obrack_span(node->span().start(), node->span().start()); return TypeInferenceErrorStatus( @@ -154,35 +237,8 @@ absl::StatusOr> DeduceArray(const Array* node, return inferred; } - // The type annotation is present, see what it is. - XLS_ASSIGN_OR_RETURN(std::unique_ptr annotated, - ctx->Deduce(node->type_annotation())); - XLS_ASSIGN_OR_RETURN( - annotated, - UnwrapMetaType(std::move(annotated), node->span(), - "array type-prefix position", ctx->file_table())); - VLOG(5) << absl::StreamFormat( - "DeduceArray; inferred type annotation `%s` to be `%s`", - node->type_annotation()->ToString(), annotated->ToString()); - - // It must be an array! - auto* array_type = dynamic_cast(annotated.get()); - if (array_type == nullptr) { - return TypeInferenceErrorStatus( - node->span(), annotated.get(), - "Array was not annotated with an array type.", ctx->file_table()); - } - - if (array_type->HasParametricDims()) { - return TypeInferenceErrorStatus( - node->type_annotation()->span(), array_type, - absl::StrFormat("Annotated type for array " - "literal must be constexpr; type has dimensions that " - "cannot be resolved."), - ctx->file_table()); - } - - const TypeDim& annotation_size = array_type->size(); + XLS_RET_CHECK(annotated != nullptr); + const TypeDim& annotation_size = annotated->size(); // If we were presented with the wrong number of elements (vs what the // annotated type expected), flag an error. @@ -194,22 +250,22 @@ absl::StatusOr> DeduceArray(const Array* node, std::string message = absl::StrFormat( "Annotated array size %s is too small for observed array member " "count %d", - array_type->size().ToString(), member_types.size()); - return ctx->TypeMismatchError(node->span(), nullptr, *array_type, nullptr, + annotated->size().ToString(), member_types.size()); + return ctx->TypeMismatchError(node->span(), nullptr, *annotated, nullptr, *inferred, message); } } else { // No ellipsis. if (annotation_size != member_types_dim) { std::string message = absl::StrFormat( "Annotated array size %s does not match inferred array size %d.", - array_type->size().ToString(), member_types.size()); + annotated->size().ToString(), member_types.size()); if (inferred == nullptr) { // No type to compare our expectation to, as there was no member to // infer the type from. - return TypeInferenceErrorStatus(node->span(), array_type, message, + return TypeInferenceErrorStatus(node->span(), annotated.get(), message, ctx->file_table()); } - return ctx->TypeMismatchError(node->span(), nullptr, *array_type, nullptr, + return ctx->TypeMismatchError(node->span(), nullptr, *annotated, nullptr, *inferred, message); } } @@ -217,7 +273,7 @@ absl::StatusOr> DeduceArray(const Array* node, // Check the element type of the annotation is the same as the inferred type // from the element(s). XLS_ASSIGN_OR_RETURN(std::unique_ptr resolved_element_type, - ctx->Resolve(array_type->element_type())); + ctx->Resolve(annotated->element_type())); if (*resolved_element_type != *member_types[0]) { return ctx->TypeMismatchError( node->members().at(0)->span(), nullptr, *resolved_element_type, nullptr, @@ -227,69 +283,58 @@ absl::StatusOr> DeduceArray(const Array* node, } // In case of an ellipsis, the annotated type wins out. - return annotated; + return absl::WrapUnique(dynamic_cast(annotated.release())); } -absl::StatusOr> DeduceConstantArray( - const ConstantArray* node, DeduceCtx* ctx) { - if (node->type_annotation() == nullptr) { - return DeduceArray(node, ctx); +// Implementation note: we wrap up DeduceArrayInternal with a check to see the +// members are all constexpr, in which case the array itself is constexpr. +absl::StatusOr> DeduceArray(const Array* node, + DeduceCtx* ctx) { + XLS_ASSIGN_OR_RETURN(std::unique_ptr type, + DeduceArrayInternal(node, ctx)); + + VLOG(5) << "DeduceArray; node: `" << node->ToString() << "`; type: `" + << type->ToString() << "`"; + + std::vector constexpr_values; + constexpr_values.reserve(node->members().size()); + for (const Expr* member : node->members()) { + std::optional value = + ctx->type_info()->GetConstExprOption(member); + if (!value.has_value()) { + VLOG(5) << "DeduceArray; member: `" << member->ToString() + << "` is not constexpr"; + return type; + } + constexpr_values.push_back(std::move(value.value())); } - XLS_ASSIGN_OR_RETURN(std::unique_ptr type, - ctx->Deduce(node->type_annotation())); - XLS_ASSIGN_OR_RETURN( - type, UnwrapMetaType(std::move(type), node->type_annotation()->span(), - "array type-prefix position", ctx->file_table())); + VLOG(5) << "DeduceArray; constexpr_values: [" + << absl::StrJoin(constexpr_values, ", ") << "]"; - auto* array_type = dynamic_cast(type.get()); - if (array_type == nullptr) { - return TypeInferenceErrorStatus( - node->type_annotation()->span(), type.get(), - absl::StrFormat("Annotated type for array " - "literal must be an array type; got %s %s", - type->GetDebugTypeName(), - node->type_annotation()->ToString()), - ctx->file_table()); - } - - const Type& element_type = array_type->element_type(); - for (Expr* member : node->members()) { - XLS_RET_CHECK(IsConstant(member)); - if (Number* number = dynamic_cast(member); - number != nullptr && number->type_annotation() == nullptr) { - ctx->type_info()->SetItem(member, element_type); - - if (std::optional bits_like = - GetBitsLike(element_type); - bits_like.has_value()) { - XLS_RETURN_IF_ERROR( - TryEnsureFitsInType(*number, bits_like.value(), element_type)); - } else { - return TypeInferenceErrorStatus( - number->span(), &element_type, - absl::StrFormat("Annotated element type for array cannot be " - "applied to a literal number"), - ctx->file_table()); - } + // If there is an ellipsis at the end we need to fill with the last value + // until we reach the array size. + if (node->has_ellipsis()) { + XLS_ASSIGN_OR_RETURN(int64_t array_size, type->size().GetAsInt64()); + XLS_RET_CHECK(constexpr_values.size() > 0) + << "Cannot have an array with ellipsis but no given member to repeat"; + InterpValue last_value = constexpr_values.back(); + while (constexpr_values.size() < array_size) { + constexpr_values.push_back(last_value); } } - XLS_RETURN_IF_ERROR(DeduceArray(node, ctx).status()); + // We made it through all the elements observing they are constexpr -- note in + // the type information that the array is constexpr. + XLS_ASSIGN_OR_RETURN(InterpValue array_value, + InterpValue::MakeArray(std::move(constexpr_values))); + ctx->type_info()->NoteConstExpr(node, array_value); return type; } absl::StatusOr> DeduceNumber(const Number* node, DeduceCtx* ctx) { VLOG(5) << "DeduceNumber: " << node->ToString(); - auto note_constexpr_value = [&](const Type& type) -> absl::Status { - if (type.HasParametricDims()) { - return absl::OkStatus(); - } - XLS_ASSIGN_OR_RETURN(InterpValue value, EvaluateNumber(*node, type)); - ctx->type_info()->NoteConstExpr(node, value); - return absl::OkStatus(); - }; std::unique_ptr type; @@ -299,13 +344,13 @@ absl::StatusOr> DeduceNumber(const Number* node, case NumberKind::kBool: { auto type = BitsType::MakeU1(); ctx->type_info()->SetItem(node, *type); - XLS_RETURN_IF_ERROR(note_constexpr_value(*type)); + XLS_RETURN_IF_ERROR(NoteTypeForNumber(*node, *type, ctx)); return type; } case NumberKind::kCharacter: { auto type = BitsType::MakeU8(); ctx->type_info()->SetItem(node, *type); - XLS_RETURN_IF_ERROR(note_constexpr_value(*type)); + XLS_RETURN_IF_ERROR(NoteTypeForNumber(*node, *type, ctx)); return type; } default: @@ -339,8 +384,8 @@ absl::StatusOr> DeduceNumber(const Number* node, node->span(), type.get(), "Non-bits type used to define a numeric literal.", ctx->file_table()); } - ctx->type_info()->SetItem(node, *type); - XLS_RETURN_IF_ERROR(note_constexpr_value(*type)); + + XLS_RETURN_IF_ERROR(NoteTypeForNumber(*node, *type, ctx)); return type; } diff --git a/xls/dslx/type_system/deduce_expr.h b/xls/dslx/type_system/deduce_expr.h index 99f684eedc..bf338b8175 100644 --- a/xls/dslx/type_system/deduce_expr.h +++ b/xls/dslx/type_system/deduce_expr.h @@ -31,9 +31,6 @@ namespace xls::dslx { absl::StatusOr> DeduceArray(const Array* node, DeduceCtx* ctx); -absl::StatusOr> DeduceConstantArray( - const ConstantArray* node, DeduceCtx* ctx); - absl::StatusOr> DeduceNumber(const Number* node, DeduceCtx* ctx); diff --git a/xls/dslx/type_system/deduce_utils.cc b/xls/dslx/type_system/deduce_utils.cc index 859705d89c..7ddc3ca69a 100644 --- a/xls/dslx/type_system/deduce_utils.cc +++ b/xls/dslx/type_system/deduce_utils.cc @@ -205,6 +205,18 @@ absl::Status TryEnsureFitsInType(const Number& number, return absl::OkStatus(); } +absl::Status TryEnsureFitsInType(const Number& number, const Type& type) { + if (std::optional bits_like = GetBitsLike(type); + bits_like.has_value()) { + return TryEnsureFitsInType(number, bits_like.value(), type); + } + return TypeInferenceErrorStatus( + number.span(), &type, + absl::StrFormat("Non-bits type (%s) used to define a numeric literal.", + type.GetDebugTypeName()), + *number.owner()->file_table()); +} + absl::Status TryEnsureFitsInBitsType(const Number& number, const BitsType& type) { std::optional bits_like = GetBitsLike(type); diff --git a/xls/dslx/type_system/deduce_utils.h b/xls/dslx/type_system/deduce_utils.h index 7ed1b3b12f..d88222709e 100644 --- a/xls/dslx/type_system/deduce_utils.h +++ b/xls/dslx/type_system/deduce_utils.h @@ -42,11 +42,17 @@ using ColonRefSubjectT = std::variant; -// If the width is known for "type", checks that "number" fits in that type. +// Validates that "number" fits in `bits_like` if the size is known. `bits_like` +// should have been derived from `type` -- this routine uses `type` for error +// reporting. absl::Status TryEnsureFitsInType(const Number& number, const BitsLikeProperties& bits_like, const Type& type); +// Validates whether the given `type` is bits-like and, if so, delegates to +// `TryEnsureFitsInType(number, bits_like, type)`. +absl::Status TryEnsureFitsInType(const Number& number, const Type& type); + // Shorthand for the above where we know a prori the type is a `BitsType` (note // that there are types that are bits-like that are not exactly `BitsType`, see // `IsBitsLike`). diff --git a/xls/dslx/type_system/typecheck_module_test.cc b/xls/dslx/type_system/typecheck_module_test.cc index 641bc3c8e0..8cb41da5c6 100644 --- a/xls/dslx/type_system/typecheck_module_test.cc +++ b/xls/dslx/type_system/typecheck_module_test.cc @@ -282,6 +282,13 @@ fn f() -> u32 { p() } XLS_EXPECT_OK(Typecheck(kProgram)); } +TEST(TypecheckTest, TupleWithExplicitlyAnnotatedType) { + constexpr std::string_view kProgram = R"( +const MY_TUPLE = (u32, u64):(u32:32, u64:64); +)"; + XLS_EXPECT_OK(Typecheck(kProgram)); +} + TEST(TypecheckTest, ProcWithImplEmpty) { constexpr std::string_view kProgram = R"( proc Foo {} @@ -1910,10 +1917,11 @@ TEST(TypecheckTest, OutOfRangeNumberInConstantArray) { } TEST(TypecheckErrorTest, BadTypeForConstantArrayOfNumbers) { - EXPECT_THAT(Typecheck("const A = u8[3][4]:[1, 2, 3, 4];"), - StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("Annotated element type for array cannot be " - "applied to a literal number"))); + EXPECT_THAT( + Typecheck("const A = u8[3][4]:[1, 2, 3, 4];"), + StatusIs( + absl::StatusCode::kInvalidArgument, + HasSubstr("Non-bits type (array) used to define a numeric literal"))); } TEST(TypecheckErrorTest, ConstantArrayEmptyMembersWrongCountVsDecl) { @@ -2263,14 +2271,14 @@ fn main() -> () { } TEST(TypecheckTest, BadArrayLiteralType) { - EXPECT_THAT(Typecheck(R"( + EXPECT_THAT( + Typecheck(R"( fn main() -> s32[2] { s32:[1, 2] } )"), - StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("Annotated type for array literal must be an " - "array type; got sbits s32"))); + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("Array was not annotated with an array type"))); } TEST(TypecheckTest, CharLiteralArray) { diff --git a/xls/tools/proto_to_dslx.cc b/xls/tools/proto_to_dslx.cc index 29a509ccfd..12b7d0f7c3 100644 --- a/xls/tools/proto_to_dslx.cc +++ b/xls/tools/proto_to_dslx.cc @@ -342,7 +342,7 @@ absl::StatusOr MakeZeroValuedElement( const dslx::FileTable& file_table = *module->file_table(); XLS_ASSIGN_OR_RETURN(uint64_t real_size, array_size->GetAsUint64(file_table)); - return module->Make( + return module->Make( span, std::vector(real_size, member), /*has_ellipsis=*/false); } @@ -708,8 +708,7 @@ absl::Status EmitArray( } } - auto* array = - module->Make(span, *array_elements, has_ellipsis); + auto* array = module->Make(span, *array_elements, has_ellipsis); elements->push_back(std::make_pair(std::string(field_name), array)); auto* u32_type = module->Make(