Skip to content

Commit c13655b

Browse files
angelomatni1copybara-github
angelomatni1
authored and
copybara-github
committed
fix: DSLX C++ transpiler must unique-ify generated temp variable names to not collide with DSLX struct member field names
PiperOrigin-RevId: 861393002
1 parent 4201434 commit c13655b

File tree

10 files changed

+161
-135
lines changed

10 files changed

+161
-135
lines changed

‎xls/dslx/cpp_transpiler/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ cc_library(
9393
"//xls/dslx/bytecode:bytecode_interpreter",
9494
"//xls/dslx/frontend:ast",
9595
"//xls/dslx/type_system:type_info",
96+
"@com_google_absl//absl/algorithm:container",
9697
"@com_google_absl//absl/container:flat_hash_map",
9798
"@com_google_absl//absl/status",
9899
"@com_google_absl//absl/status:statusor",

‎xls/dslx/cpp_transpiler/cpp_transpiler_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ struct MyStruct {
179179
y: u15,
180180
z: u8,
181181
w: s63,
182-
v: u1,
182+
result: u1,
183183
})";
184184

185185
auto import_data = CreateImportDataForTest();

‎xls/dslx/cpp_transpiler/cpp_type_generator.cc‎

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <utility>
2323
#include <vector>
2424

25+
#include "absl/algorithm/container.h"
2526
#include "absl/container/flat_hash_map.h"
2627
#include "absl/status/status.h"
2728
#include "absl/status/statusor.h"
@@ -44,6 +45,24 @@
4445
namespace xls::dslx {
4546
namespace {
4647

48+
constexpr std::string_view kUniqueVarNamePrefix = "result";
49+
50+
std::string UniqueVarName(std::vector<std::string>& user_names) {
51+
std::string unique_name = std::string(kUniqueVarNamePrefix);
52+
if (!absl::c_contains(user_names, unique_name)) {
53+
return unique_name;
54+
}
55+
std::vector<std::string> existing_user_names = user_names;
56+
absl::c_sort(existing_user_names);
57+
// Appending a character avoids collision with previous names due to sorting.
58+
for (int64_t i = 0; i < existing_user_names.size(); ++i) {
59+
if (existing_user_names[i] == unique_name) {
60+
unique_name += "_";
61+
}
62+
}
63+
return unique_name;
64+
}
65+
4766
absl::StatusOr<InterpValue> InterpretExpr(
4867
ImportData* import_data, TypeInfo* type_info, Expr* expr,
4968
const absl::flat_hash_map<std::string, InterpValue>& env) {
@@ -479,7 +498,8 @@ class StructCppTypeGenerator : public CppTypeGenerator {
479498
names.push_back(SanitizeCppName(struct_def->GetMemberName(i)));
480499
}
481500
return names;
482-
}()) {}
501+
}()),
502+
tmp_var_name_(UniqueVarName(cpp_member_names_)) {}
483503
~StructCppTypeGenerator() override = default;
484504

485505
static absl::StatusOr<std::unique_ptr<StructCppTypeGenerator>> Create(
@@ -566,14 +586,14 @@ class StructCppTypeGenerator : public CppTypeGenerator {
566586
"tuple of %d elements.\");",
567587
struct_def_->size()));
568588
pieces.push_back("}");
569-
pieces.push_back(absl::StrFormat("%s result;", cpp_type()));
589+
pieces.push_back(absl::StrFormat("%s %s;", cpp_type(), tmp_var_name_));
570590
for (int i = 0; i < struct_def_->members().size(); i++) {
571591
std::string assignment = member_emitters_[i]->AssignFromValue(
572-
/*lhs=*/absl::StrFormat("result.%s", cpp_member_names_[i]),
592+
/*lhs=*/absl::StrFormat("%s.%s", tmp_var_name_, cpp_member_names_[i]),
573593
/*rhs=*/absl::StrFormat("value.element(%d)", i), /*nesting=*/0);
574594
pieces.push_back(assignment);
575595
}
576-
pieces.push_back("return result;");
596+
pieces.push_back(absl::StrFormat("return %s;", tmp_var_name_));
577597
std::string body = absl::StrJoin(pieces, "\n");
578598

579599
return CppSource{
@@ -588,16 +608,18 @@ class StructCppTypeGenerator : public CppTypeGenerator {
588608

589609
CppSource ToValueMethod() const {
590610
std::vector<std::string> pieces;
591-
pieces.push_back("std::vector<::xls::Value> members;");
592611
pieces.push_back(
593-
absl::StrFormat("members.resize(%d);", struct_def_->members().size()));
612+
absl::StrFormat("std::vector<::xls::Value> %s;", tmp_var_name_));
613+
pieces.push_back(absl::StrFormat("%s.resize(%d);", tmp_var_name_,
614+
struct_def_->members().size()));
594615
for (int i = 0; i < struct_def_->members().size(); i++) {
595616
std::string assignment = member_emitters_[i]->AssignToValue(
596-
/*lhs=*/absl::StrFormat("members[%d]", i),
617+
/*lhs=*/absl::StrFormat("%s[%d]", tmp_var_name_, i),
597618
/*rhs=*/cpp_member_names_[i], /*nesting=*/0);
598619
pieces.push_back(assignment);
599620
}
600-
pieces.push_back("return ::xls::Value::Tuple(members);");
621+
pieces.push_back(
622+
absl::StrFormat("return ::xls::Value::Tuple(%s);", tmp_var_name_));
601623
std::string body = absl::StrJoin(pieces, "\n");
602624

603625
return CppSource{
@@ -627,18 +649,18 @@ class StructCppTypeGenerator : public CppTypeGenerator {
627649

628650
CppSource ToStringMethod() const {
629651
std::vector<std::string> pieces;
630-
pieces.push_back(
631-
absl::StrFormat("std::string result = \"%s {\\n\";", cpp_type()));
652+
pieces.push_back(absl::StrFormat("std::string %s = \"%s {\\n\";",
653+
tmp_var_name_, cpp_type()));
632654
for (int i = 0; i < struct_def_->members().size(); i++) {
633-
pieces.push_back(absl::StrFormat(
634-
"result += __indent(indent + 1) + \"%s: \";", cpp_member_names_[i]));
655+
pieces.push_back(absl::StrFormat("%s += __indent(indent + 1) + \"%s: \";",
656+
tmp_var_name_, cpp_member_names_[i]));
635657
std::string to_string = member_emitters_[i]->ToString(
636-
"result", "indent + 2", cpp_member_names_[i], /*nesting=*/0);
658+
tmp_var_name_, "indent + 2", cpp_member_names_[i], /*nesting=*/0);
637659
pieces.push_back(to_string);
638-
pieces.push_back("result += \",\\n\";");
660+
pieces.push_back(tmp_var_name_ + " += \",\\n\";");
639661
}
640-
pieces.push_back("result += __indent(indent) + \"}\";");
641-
pieces.push_back("return result;");
662+
pieces.push_back(tmp_var_name_ + " += __indent(indent) + \"}\";");
663+
pieces.push_back(absl::StrFormat("return %s;", tmp_var_name_));
642664
std::string body = absl::StrJoin(pieces, "\n");
643665

644666
return CppSource{.header = "std::string ToString(int indent = 0) const;",
@@ -649,18 +671,18 @@ class StructCppTypeGenerator : public CppTypeGenerator {
649671

650672
CppSource ToDslxStringMethod() const {
651673
std::vector<std::string> pieces;
652-
pieces.push_back(
653-
absl::StrFormat("std::string result = \"%s {\\n\";", dslx_type()));
674+
pieces.push_back(absl::StrFormat("std::string %s = \"%s {\\n\";",
675+
tmp_var_name_, dslx_type()));
654676
for (int i = 0; i < struct_def_->members().size(); i++) {
655-
pieces.push_back(absl::StrFormat(
656-
"result += __indent(indent + 1) + \"%s: \";", cpp_member_names_[i]));
677+
pieces.push_back(absl::StrFormat("%s += __indent(indent + 1) + \"%s: \";",
678+
tmp_var_name_, cpp_member_names_[i]));
657679
std::string to_string = member_emitters_[i]->ToDslxString(
658-
"result", "indent + 2", cpp_member_names_[i], /*nesting=*/0);
680+
tmp_var_name_, "indent + 2", cpp_member_names_[i], /*nesting=*/0);
659681
pieces.push_back(to_string);
660-
pieces.push_back("result += \",\\n\";");
682+
pieces.push_back(tmp_var_name_ + " += \",\\n\";");
661683
}
662-
pieces.push_back("result += __indent(indent) + \"}\";");
663-
pieces.push_back("return result;");
684+
pieces.push_back(tmp_var_name_ + " += __indent(indent) + \"}\";");
685+
pieces.push_back(absl::StrFormat("return %s;", tmp_var_name_));
664686
std::string body = absl::StrJoin(pieces, "\n");
665687

666688
return CppSource{
@@ -712,6 +734,9 @@ class StructCppTypeGenerator : public CppTypeGenerator {
712734
const StructDef* struct_def_;
713735
std::vector<std::unique_ptr<CppEmitter>> member_emitters_;
714736
std::vector<std::string> cpp_member_names_;
737+
// The name of temporary variables in generated code that do not collide with
738+
// user defined member names.
739+
std::string tmp_var_name_;
715740
};
716741

717742
} // namespace

‎xls/dslx/cpp_transpiler/testdata/ArrayOfTyperefs.cctxt‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,17 @@ absl::StatusOr<Foo> Foo::FromValue(const ::xls::Value& value) {
5555
}
5656

5757
absl::StatusOr<::xls::Value> Foo::ToValue() const {
58-
std::vector<::xls::Value> members;
59-
members.resize(2);
58+
std::vector<::xls::Value> result;
59+
result.resize(2);
6060
if (!FitsInNBitsUnsigned(a, 32)) {
6161
return absl::InvalidArgumentError(absl::StrFormat("Unsigned value %#x does not fit in 32 bits", a));
6262
}
63-
members[0] = ::xls::Value(::xls::UBits(a, 32));
63+
result[0] = ::xls::Value(::xls::UBits(a, 32));
6464
if (!FitsInNBitsUnsigned(b, 64)) {
6565
return absl::InvalidArgumentError(absl::StrFormat("Unsigned value %#x does not fit in 64 bits", b));
6666
}
67-
members[1] = ::xls::Value(::xls::UBits(b, 64));
68-
return ::xls::Value::Tuple(members);
67+
result[1] = ::xls::Value(::xls::UBits(b, 64));
68+
return ::xls::Value::Tuple(result);
6969
}
7070

7171
std::string Foo::ToString(int indent) const {
@@ -126,18 +126,18 @@ absl::StatusOr<Bar> Bar::FromValue(const ::xls::Value& value) {
126126
}
127127

128128
absl::StatusOr<::xls::Value> Bar::ToValue() const {
129-
std::vector<::xls::Value> members;
130-
members.resize(1);
129+
std::vector<::xls::Value> result;
130+
result.resize(1);
131131
{
132132
std::vector<::xls::Value> elements;
133133
for (int64_t i0 = 0; i0 < 2; ++i0) {
134134
::xls::Value element;
135135
XLS_ASSIGN_OR_RETURN(element, c[i0].ToValue());
136136
elements.push_back(element);
137137
}
138-
members[0] = ::xls::Value::ArrayOrDie(elements);
138+
result[0] = ::xls::Value::ArrayOrDie(elements);
139139
}
140-
return ::xls::Value::Tuple(members);
140+
return ::xls::Value::Tuple(result);
141141
}
142142

143143
std::string Bar::ToString(int indent) const {

‎xls/dslx/cpp_transpiler/testdata/BasicArray.cctxt‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ absl::StatusOr<MyStruct> MyStruct::FromValue(const ::xls::Value& value) {
7474
}
7575

7676
absl::StatusOr<::xls::Value> MyStruct::ToValue() const {
77-
std::vector<::xls::Value> members;
78-
members.resize(3);
77+
std::vector<::xls::Value> result;
78+
result.resize(3);
7979
{
8080
std::vector<::xls::Value> elements;
8181
for (int64_t i0 = 0; i0 < 32; ++i0) {
@@ -86,7 +86,7 @@ absl::StatusOr<::xls::Value> MyStruct::ToValue() const {
8686
element = ::xls::Value(::xls::UBits(x[i0], 32));
8787
elements.push_back(element);
8888
}
89-
members[0] = ::xls::Value::ArrayOrDie(elements);
89+
result[0] = ::xls::Value::ArrayOrDie(elements);
9090
}
9191
{
9292
std::vector<::xls::Value> elements;
@@ -98,7 +98,7 @@ absl::StatusOr<::xls::Value> MyStruct::ToValue() const {
9898
element = ::xls::Value(::xls::SBits(y[i0], 7));
9999
elements.push_back(element);
100100
}
101-
members[1] = ::xls::Value::ArrayOrDie(elements);
101+
result[1] = ::xls::Value::ArrayOrDie(elements);
102102
}
103103
{
104104
std::vector<::xls::Value> elements;
@@ -110,9 +110,9 @@ absl::StatusOr<::xls::Value> MyStruct::ToValue() const {
110110
element = ::xls::Value(::xls::UBits(z[i0], 8));
111111
elements.push_back(element);
112112
}
113-
members[2] = ::xls::Value::ArrayOrDie(elements);
113+
result[2] = ::xls::Value::ArrayOrDie(elements);
114114
}
115-
return ::xls::Value::Tuple(members);
115+
return ::xls::Value::Tuple(result);
116116
}
117117

118118
std::string MyStruct::ToString(int indent) const {

0 commit comments

Comments
 (0)