Skip to content

Commit

Permalink
[DSLX:TS] Use type zipper to print mismatches in types more nicely.
Browse files Browse the repository at this point in the history
Remaining to be done: we should identify when tuples have "substring matches";
e.g. they won't structurally match but some elements inside of them will. This
will help make messages better for cases e.g. where we leave off a tuple
element by accident.

Towards #1379

PiperOrigin-RevId: 626218553
  • Loading branch information
cdleary authored and copybara-github committed Apr 19, 2024
1 parent 66fd600 commit 3a37270
Show file tree
Hide file tree
Showing 14 changed files with 533 additions and 61 deletions.
1 change: 1 addition & 0 deletions xls/dslx/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ cc_library(
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/strings:str_format",
"@com_googlesource_code_re2//:re2",
],
)

Expand Down
16 changes: 14 additions & 2 deletions xls/dslx/error_printer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "xls/common/status/status_macros.h"
#include "xls/dslx/frontend/pos.h"
#include "xls/dslx/warning_collector.h"
#include "re2/re2.h"

namespace xls::dslx {

Expand Down Expand Up @@ -78,6 +79,17 @@ absl::Status PrintPositionalError(
int64_t last_line_printed =
std::min(limit.lineno() + line_count_each_side, file_limit.lineno());

// Strip ANSI escape codes from the error message opportunistically if we know
// we shouldn't be emitting colors.
//
// This is a bit of a layering violation but makes life easier as it allows
// "downstream" error message creation to insert ANSI codes without needing to
// be told explicitly if color error messages are ok.
std::string msg{error_message};
if (!isatty(fileno(stderr)) || color == PositionalErrorColor::kNoColor) {
RE2::GlobalReplace(&msg, "\33\\[\\d+m", "");
}

std::string_view pos_color_leader;
std::string_view msg_color_leader;
std::string_view color_reset;
Expand Down Expand Up @@ -134,14 +146,14 @@ absl::Status PrintPositionalError(
dashes_and_arrow = std::string(width - 1, '-') + "^";
}
os << absl::StreamFormat("%s%s^%s %s%s\n", msg_color_leader, squiggles,
dashes_and_arrow, error_message, color_reset);
dashes_and_arrow, msg, color_reset);
}
} else if (i == limit.lineno()) {
// Emit arrow pointing to the end of the multi-line error.
std::string spaces(std::string_view("0000: ").size(), ' ');
std::string underscores(std::max(int64_t{0}, limit.colno()), '_');
os << absl::StreamFormat("%s%s|%s^ %s%s\n", msg_color_leader, spaces,
underscores, error_message, color_reset);
underscores, msg, color_reset);
// We're done drawing the multiline arrows; put down the crayon.
bar = bar_off;
}
Expand Down
1 change: 1 addition & 0 deletions xls/dslx/interpreter_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@ def test_cast_array_to_wrong_bit_count(self):
0003: let x = u2[2]:[2, 3];
0004: assert_eq(u3:0, x as u3)
~~~~~~~~~~~~~~~~~~~~~~~~^-----^ XlsTypeError: Cannot cast from expression type uN[2][2] to uN[3].
Type mismatch:
uN[2][2]
vs uN[3]
0005: }
Expand Down
7 changes: 6 additions & 1 deletion xls/dslx/tests/errors/error_modules_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1097,7 +1097,12 @@ def test_deeply_nested_type_mismatch(self):
)
self.assertIn('XlsTypeError:', stderr)
self.assertIn(
'(uN[8], uN[16], uN[32], uN[64])\nvs (uN[8], uN[16], uN[33], uN[64])',
'Mismatched elements within type:\n'
' uN[32]\n'
'vs uN[33]\n'
'Overall type mismatch:\n'
' (uN[8], uN[16], uN[32], uN[64])\n'
'vs (uN[8], uN[16], uN[33], uN[64])',
stderr,
)
self.assertIn(
Expand Down
33 changes: 32 additions & 1 deletion xls/dslx/type_system/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -261,12 +261,13 @@ cc_library(
srcs = ["maybe_explain_error.cc"],
hdrs = ["maybe_explain_error.h"],
deps = [
":format_type_mismatch",
":type",
":type_mismatch_error_data",
"@com_google_absl//absl/log",
"@com_google_absl//absl/status",
"@com_google_absl//absl/strings:str_format",
"//xls/dslx:errors",
"//xls/common/status:status_macros",
"//xls/dslx/frontend:ast",
"//xls/dslx/frontend:ast_node",
"//xls/dslx/frontend:pos",
Expand Down Expand Up @@ -865,3 +866,33 @@ cc_test(
"//xls/common/status:matchers",
],
)

cc_library(
name = "format_type_mismatch",
srcs = ["format_type_mismatch.cc"],
hdrs = ["format_type_mismatch.h"],
deps = [
":type",
":zip_types",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/strings:str_format",
"@com_google_absl//absl/types:variant",
"//xls/common:visitor",
"//xls/common/status:status_macros",
],
)

cc_test(
name = "format_type_mismatch_test",
srcs = ["format_type_mismatch_test.cc"],
deps = [
":format_type_mismatch",
":type",
"//xls/common:xls_gunit",
"//xls/common:xls_gunit_main",
"//xls/common/status:matchers",
"//xls/common/status:status_macros",
],
)
229 changes: 229 additions & 0 deletions xls/dslx/type_system/format_type_mismatch.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,229 @@
// Copyright 2024 The XLS Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#include "xls/dslx/type_system/format_type_mismatch.h"

#include <cstdint>
#include <string>
#include <string_view>
#include <utility>
#include <vector>

#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/str_format.h"
#include "absl/strings/str_join.h"
#include "absl/types/variant.h"
#include "xls/common/status/status_macros.h"
#include "xls/common/visitor.h"
#include "xls/dslx/type_system/type.h"
#include "xls/dslx/type_system/zip_types.h"

namespace xls::dslx {
namespace {

constexpr std::string_view kAnsiReset = "\33[0m";
constexpr std::string_view kAnsiRed = "\33[31m";
constexpr std::string_view kAnsiBoldOn = "\33[1m";
constexpr std::string_view kAnsiBoldOff = "\33[22m";

// Populates the ref given as `mismatches` with the mismatches.
//
// Note: we could have this use the auto-formatting pretty printer to get more
// readable line wrapping for very long types, but we hope that highlighting the
// subtype mismatches inside the broader type might suffice for now.
class Callbacks : public ZipTypesCallbacks {
public:
explicit Callbacks(
std::vector<std::pair<std::string, std::string>>& mismatches)
: mismatches_(mismatches) {}

absl::Status NoteAggregateStart(const AggregatePair& aggregates) override {
return absl::visit(
Visitor{
[&](std::pair<const TupleType*, const TupleType*>) {
AddMatchedBoth("(");
return absl::OkStatus();
},
[&](std::pair<const StructType*, const StructType*> p) {
AddMatchedBoth(
absl::StrCat(p.first->nominal_type().identifier(), "{"));
return absl::OkStatus();
},
[&](std::pair<const ArrayType*, const ArrayType*> p) {
/* goes at the end */
return absl::OkStatus();
},
[&](std::pair<const ChannelType*, const ChannelType*> p) {
AddMatchedBoth("chan(");
return absl::OkStatus();
},
[&](std::pair<const FunctionType*, const FunctionType*> p) {
return absl::UnimplementedError(
"Cannot print diffs of function types.");
},
[&](std::pair<const MetaType*, const MetaType*> p) {
AddMatchedBoth("typeof(");
return absl::OkStatus();
},
},
aggregates);
}

absl::Status NoteAggregateEnd(const AggregatePair& aggregates) override {
return absl::visit(
Visitor{
[&](std::pair<const TupleType*, const TupleType*>) {
AddMatchedBoth(")");
return absl::OkStatus();
},
[&](std::pair<const StructType*, const StructType*>) {
AddMatchedBoth("}");
return absl::OkStatus();
},
[&](std::pair<const ArrayType*, const ArrayType*> p) {
AddMatched(absl::StrCat("[", p.first->size().ToString(), "]"),
&colorized_lhs_);
AddMatched(absl::StrCat("[", p.second->size().ToString(), "]"),
&colorized_rhs_);
return absl::OkStatus();
},
[&](std::pair<const ChannelType*, const ChannelType*> p) {
AddMatchedBoth(")");
return absl::OkStatus();
},
[&](std::pair<const FunctionType*, const FunctionType*> p) {
return absl::UnimplementedError(
"Cannot print diffs of function types.");
},
[&](std::pair<const MetaType*, const MetaType*> p) {
AddMatchedBoth(")");
return absl::OkStatus();
},
},
aggregates);
}

absl::Status NoteMatchedLeafType(const Type& lhs, const Type* lhs_parent,
const Type& rhs,
const Type* rhs_parent) override {
match_count_++;
BeforeType(lhs, lhs_parent, rhs, rhs_parent);
AddMatched(lhs.ToString(), &colorized_lhs_);
AddMatched(rhs.ToString(), &colorized_rhs_);
AfterType(lhs, lhs_parent, rhs, rhs_parent);
return absl::OkStatus();
}

absl::Status NoteTypeMismatch(const Type& lhs, const Type* lhs_parent,
const Type& rhs,
const Type* rhs_parent) override {
mismatches_.push_back({lhs.ToString(), rhs.ToString()});
BeforeType(lhs, lhs_parent, rhs, rhs_parent);
AddMismatched(lhs.ToString(), rhs.ToString());
AfterType(lhs, lhs_parent, rhs, rhs_parent);
return absl::OkStatus();
}

std::string_view colorized_lhs() const { return colorized_lhs_; }
std::string_view colorized_rhs() const { return colorized_rhs_; }

int64_t match_count() const { return match_count_; }

private:
// Adds a struct field before the RHS.
void BeforeType(const Type& lhs, const Type* lhs_parent, const Type& rhs,
const Type* rhs_parent) {
if (lhs_parent == nullptr) {
return;
}
if (auto* parent_struct = dynamic_cast<const StructType*>(lhs_parent);
parent_struct != nullptr) {
int64_t index = parent_struct->IndexOf(lhs).value();
AddMatchedBoth(absl::StrCat(parent_struct->GetMemberName(index), ": "));
}
}

void AfterType(const Type& lhs, const Type* lhs_parent, const Type& rhs,
const Type* rhs_parent) {
if (lhs_parent == nullptr) {
return;
}
if (auto* parent_struct = dynamic_cast<const StructType*>(lhs_parent);
parent_struct != nullptr &&
parent_struct->IndexOf(lhs).value() + 1 != parent_struct->size()) {
AddMatchedBoth(", ");
}
if (auto* parent_tuple = dynamic_cast<const TupleType*>(lhs_parent);
parent_tuple != nullptr &&
parent_tuple->IndexOf(lhs).value() + 1 != parent_tuple->size()) {
AddMatchedBoth(", ");
}
}

void AddMismatched(std::string_view lhs, std::string_view rhs) {
absl::StrAppend(&colorized_lhs_, kAnsiRed, lhs, kAnsiReset);
absl::StrAppend(&colorized_rhs_, kAnsiRed, rhs, kAnsiReset);
}

void AddMatched(std::string_view matched_text, std::string* out) {
absl::StrAppend(out, matched_text);
}
// Helper that adds the matched text to both the LHS and RHS.
void AddMatchedBoth(std::string_view matched_text) {
AddMatched(matched_text, &colorized_lhs_);
AddMatched(matched_text, &colorized_rhs_);
}

// We start the string off with an ANSI reset since we have our own coloring
// we do inside.
std::string colorized_lhs_;
std::string colorized_rhs_;
std::vector<std::pair<std::string, std::string>>& mismatches_;
int64_t match_count_ = 0;
};

} // namespace

absl::StatusOr<std::string> FormatTypeMismatch(const Type& lhs,
const Type& rhs) {
std::vector<std::pair<std::string, std::string>> mismatches;

Callbacks callbacks(mismatches);

XLS_RETURN_IF_ERROR(ZipTypes(lhs, rhs, callbacks));

std::vector<std::string> lines;
if (callbacks.match_count() == 0) {
lines.push_back("Type mismatch:");
lines.push_back(absl::StrFormat(" %s", lhs.ToString()));
lines.push_back(absl::StrFormat("vs %s", rhs.ToString()));
} else {
lines.push_back(absl::StrFormat("%sMismatched elements %swithin%s type:",
kAnsiReset, kAnsiBoldOn, kAnsiBoldOff));
for (const auto& [lhs_mismatch, rhs_mismatch] : mismatches) {
lines.push_back(absl::StrFormat(" %s", lhs_mismatch));
lines.push_back(absl::StrFormat("vs %s", rhs_mismatch));
}
lines.push_back(absl::StrFormat("%sOverall%s type mismatch:", kAnsiBoldOn,
kAnsiBoldOff));
lines.push_back(
absl::StrFormat("%s %s", kAnsiReset, callbacks.colorized_lhs()));
lines.push_back(absl::StrFormat("vs %s", callbacks.colorized_rhs()));
}
return absl::StrJoin(lines, "\n");
}

} // namespace xls::dslx
44 changes: 44 additions & 0 deletions xls/dslx/type_system/format_type_mismatch.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright 2024 The XLS Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#ifndef XLS_DSLX_TYPE_SYSTEM_FORMAT_TYPE_MISMATCH_H_
#define XLS_DSLX_TYPE_SYSTEM_FORMAT_TYPE_MISMATCH_H_

#include <string>

#include "absl/status/statusor.h"
#include "xls/dslx/type_system/type.h"

namespace xls::dslx {

// Returns a string that displays the mismatch between the "lhs" and "rhs" types
// in more detail in an attempt to be helpful to a DSLX user.
//
// This may include lines that highlight specific differences between the types
// where the structure was the same between lhs and rhs; e.g. for
//
// lhs: (u32, u64)
// rhs: (u32, s64)
//
// The returned string will highlight that the discrepancy is between the u64 /
// s64 within the tuples.
//
// The returned string should be assumed to be multi-line but not
// newline-terminated.
absl::StatusOr<std::string> FormatTypeMismatch(const Type& lhs,
const Type& rhs);

} // namespace xls::dslx

#endif // XLS_DSLX_TYPE_SYSTEM_FORMAT_TYPE_MISMATCH_H_
Loading

0 comments on commit 3a37270

Please sign in to comment.