Skip to content
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

[DSLX:TS] Don't crash on "member[0] of array is a type ref" scenario. #1822

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions xls/dslx/type_system/deduce_expr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ static absl::StatusOr<std::unique_ptr<ArrayType>> DeduceArrayTypeAnnotation(
// numbers. This will become more generalized with type inference 2.0.
static absl::StatusOr<std::unique_ptr<ArrayType>> DeduceArrayInternal(
const Array* node, DeduceCtx* ctx) {
VLOG(5) << "DeduceArray; node: " << node->ToString();
VLOG(5) << "DeduceArrayInternal; node: " << node->ToString();

if (node->members().empty()) {
return DeduceEmptyArray(node, ctx);
Expand All @@ -175,6 +175,7 @@ static absl::StatusOr<std::unique_ptr<ArrayType>> DeduceArrayInternal(
DeduceArrayTypeAnnotation(node, ctx));

if (annotated != nullptr) {
VLOG(5) << "DeduceArrayInternal; annotated: " << annotated->ToString();
// If the array type is annotated, as a user convenience, we propagate the
// element type to bare (unannotated) literal numbers contained in the
// array.
Expand Down Expand Up @@ -206,11 +207,21 @@ static absl::StatusOr<std::unique_ptr<ArrayType>> DeduceArrayInternal(
}
}

std::unique_ptr<Type> inferred_element_type =
member_types[0]->CloneToUnique();
// Check that we're not making an array of types, as arrays cannot hold types.
if (inferred_element_type->IsMeta()) {
return TypeInferenceErrorStatus(
node->span(), inferred_element_type.get(),
"Array element cannot be a metatype because arrays cannot hold types.",
ctx->file_table());
}

// Check that we're not making a token array, as tokens must not alias, and
// arrays obscure their provenance as they are aggregate types.
if (member_types[0]->HasToken()) {
if (inferred_element_type->HasToken()) {
return TypeInferenceErrorStatus(
node->span(), member_types[0].get(),
node->span(), inferred_element_type.get(),
"Types with tokens cannot be placed in arrays.", ctx->file_table());
}

Expand All @@ -219,7 +230,7 @@ static absl::StatusOr<std::unique_ptr<ArrayType>> DeduceArrayInternal(

// Try to infer the array type from the first member.
std::unique_ptr<ArrayType> inferred = std::make_unique<ArrayType>(
member_types[0]->CloneToUnique(), member_types_dim);
std::move(inferred_element_type), member_types_dim);

if (annotated == nullptr) {
if (node->has_ellipsis()) {
Expand Down
8 changes: 8 additions & 0 deletions xls/dslx/type_system/type.cc
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,14 @@ absl::StatusOr<TypeDim> TupleType::GetTotalBitCount() const {

// -- ArrayType

ArrayType::ArrayType(std::unique_ptr<Type> element_type, const TypeDim& size)
: element_type_(std::move(element_type)), size_(size) {
CHECK(!element_type_->IsMeta())
<< "Array element cannot be a metatype because arrays cannot hold types; "
"got: "
<< element_type_->ToStringInternal(FullyQualify::kNo, nullptr);
}

absl::StatusOr<std::unique_ptr<Type>> ArrayType::MapSize(
const std::function<absl::StatusOr<TypeDim>(TypeDim)>& f) const {
XLS_ASSIGN_OR_RETURN(std::unique_ptr<Type> new_element_type,
Expand Down
6 changes: 1 addition & 5 deletions xls/dslx/type_system/type.h
Original file line number Diff line number Diff line change
Expand Up @@ -751,11 +751,7 @@ class TupleType : public Type {
// These will nest in the case of multidimensional arrays.
class ArrayType : public Type {
public:
ArrayType(std::unique_ptr<Type> element_type, const TypeDim& size)
: element_type_(std::move(element_type)), size_(size) {
CHECK(!element_type_->IsMeta())
<< element_type_->ToStringInternal(FullyQualify::kNo, nullptr);
}
ArrayType(std::unique_ptr<Type> element_type, const TypeDim& size);

absl::Status Accept(TypeVisitor& v) const override {
return v.HandleArray(*this);
Expand Down
25 changes: 25 additions & 0 deletions xls/dslx/type_system/typecheck_module_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1878,6 +1878,31 @@ fn main() -> u32 {
HasSubstr("Cannot pass a type as a function argument.")));
}

TEST(TypecheckTest, InvalidParametricTypeConstantArrayContents) {
constexpr std::string_view kImported = R"(
pub struct MyStruct<A: u32, B: u32> {
a: bits[A],
b: bits[B],
}
)";
constexpr std::string_view kProgram = R"(
import imported;

const MY_ARRAY = imported::MyStruct<u32:4, u32:8>[2]:[
imported::MyStruct
];
)";
auto import_data = CreateImportDataForTest();
XLS_ASSERT_OK_AND_ASSIGN(
TypecheckedModule module,
ParseAndTypecheck(kImported, "imported.x", "imported", &import_data));
absl::StatusOr<TypecheckedModule> result =
ParseAndTypecheck(kProgram, "fake_main_path.x", "main", &import_data);
EXPECT_THAT(result, StatusIs(absl::StatusCode::kInvalidArgument,
HasSubstr("Array element cannot be a metatype")))
<< result.status();
}

TEST(TypecheckErrorTest, ArraySizeOfBitsType) {
EXPECT_THAT(
Typecheck(R"(
Expand Down