Skip to content

Commit 932d7d0

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. This applies to struct generators and tuple/array emitters whose temporary variable names were not only colliding with user-generated fields but also the temporary variable names in nested scopes of the generated code for functions like ToValue and FromValue.
PiperOrigin-RevId: 862334059
1 parent f5259af commit 932d7d0

File tree

9 files changed

+228
-101
lines changed

9 files changed

+228
-101
lines changed

‎xls/dslx/cpp_transpiler/BUILD‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,15 @@ cc_library(
6767
"//xls/dslx/type_system:type_info",
6868
"//xls/dslx/type_system_v2:import_utils",
6969
"//xls/dslx/type_system_v2:type_annotation_utils",
70+
"@com_google_absl//absl/algorithm:container",
7071
"@com_google_absl//absl/base:no_destructor",
7172
"@com_google_absl//absl/container:flat_hash_map",
7273
"@com_google_absl//absl/container:flat_hash_set",
7374
"@com_google_absl//absl/status",
7475
"@com_google_absl//absl/status:statusor",
7576
"@com_google_absl//absl/strings",
7677
"@com_google_absl//absl/strings:str_format",
78+
"@com_google_absl//absl/types:span",
7779
],
7880
)
7981

@@ -98,6 +100,7 @@ cc_library(
98100
"@com_google_absl//absl/status:statusor",
99101
"@com_google_absl//absl/strings",
100102
"@com_google_absl//absl/strings:str_format",
103+
"@com_google_absl//absl/types:span",
101104
"@com_google_absl//absl/types:variant",
102105
],
103106
)

‎xls/dslx/cpp_transpiler/cpp_emitter.cc‎

Lines changed: 74 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@
1414

1515
#include "xls/dslx/cpp_transpiler/cpp_emitter.h"
1616

17+
#include <algorithm>
1718
#include <cstdint>
1819
#include <functional>
20+
#include <iterator>
1921
#include <memory>
2022
#include <optional>
2123
#include <string>
@@ -24,14 +26,17 @@
2426
#include <variant>
2527
#include <vector>
2628

29+
#include "absl/algorithm/container.h"
2730
#include "absl/base/no_destructor.h"
2831
#include "absl/container/flat_hash_map.h"
2932
#include "absl/container/flat_hash_set.h"
3033
#include "absl/status/status.h"
3134
#include "absl/status/statusor.h"
35+
#include "absl/strings/match.h"
3236
#include "absl/strings/str_cat.h"
3337
#include "absl/strings/str_format.h"
3438
#include "absl/strings/str_join.h"
39+
#include "absl/types/span.h"
3540
#include "xls/common/case_converters.h"
3641
#include "xls/common/indent.h"
3742
#include "xls/common/status/ret_check.h"
@@ -418,6 +423,10 @@ class TypeRefCppEmitter : public CppEmitter {
418423
// std::array.
419424
class ArrayCppEmitter : public CppEmitter {
420425
public:
426+
// NOTE: until we implement variable name tracking across generated scopes,
427+
// prefixes must be kept unique across all emitters and generators
428+
std::string kArrayVarPrefix = "elements";
429+
421430
explicit ArrayCppEmitter(const CppType& cpp_type, std::string_view dslx_type,
422431
int64_t array_size,
423432
std::unique_ptr<CppEmitter> element_emitter)
@@ -455,23 +464,27 @@ class ArrayCppEmitter : public CppEmitter {
455464

456465
std::string AssignToValue(std::string_view lhs, std::string_view rhs,
457466
int64_t nesting) const override {
467+
std::string tmp_var_name =
468+
UniqueVarName({std::string(rhs)}, kArrayVarPrefix);
458469
std::string ind_var = absl::StrCat("i", nesting);
459470

460471
std::vector<std::string> loop_body;
461472
loop_body.push_back("::xls::Value element;");
462473
std::string element_assignment = element_emitter_->AssignToValue(
463474
"element", absl::StrFormat("%s[%s]", rhs, ind_var), nesting + 1);
464475
loop_body.push_back(element_assignment);
465-
loop_body.push_back("elements.push_back(element);");
476+
loop_body.push_back(
477+
absl::StrFormat("%s.push_back(element);", tmp_var_name));
466478

467479
std::vector<std::string> pieces;
468-
pieces.push_back("std::vector<::xls::Value> elements;");
480+
pieces.push_back(
481+
absl::StrFormat("std::vector<::xls::Value> %s;", tmp_var_name));
469482
pieces.push_back(absl::StrFormat("for (int64_t %s = 0; %s < %d; ++%s) {",
470483
ind_var, ind_var, array_size(), ind_var));
471484
pieces.push_back(Indent(absl::StrJoin(loop_body, "\n"), 2));
472485
pieces.push_back("}");
473-
pieces.push_back(
474-
absl::StrFormat("%s = ::xls::Value::ArrayOrDie(elements);", lhs));
486+
pieces.push_back(absl::StrFormat("%s = ::xls::Value::ArrayOrDie(%s);", lhs,
487+
tmp_var_name));
475488
std::string lines = absl::StrJoin(pieces, "\n");
476489

477490
return absl::StrCat("{\n", Indent(lines, 2), "\n}");
@@ -575,6 +588,10 @@ class ArrayCppEmitter : public CppEmitter {
575588
// std::tuple.
576589
class TupleCppEmitter : public CppEmitter {
577590
public:
591+
// NOTE: until we implement variable name tracking across generated scopes,
592+
// prefixes must be kept unique across all emitters and generators
593+
std::string kTupleVarPrefix = "entries";
594+
578595
explicit TupleCppEmitter(
579596
const CppType& cpp_type, std::string_view dslx_type,
580597
std::vector<std::unique_ptr<CppEmitter>> element_emitters)
@@ -606,16 +623,20 @@ class TupleCppEmitter : public CppEmitter {
606623
std::string AssignToValue(std::string_view lhs, std::string_view rhs,
607624
int64_t nesting) const override {
608625
std::vector<std::string> pieces;
609-
pieces.push_back("std::vector<::xls::Value> members;");
610-
pieces.push_back(absl::StrFormat("members.resize(%d);", size()));
626+
std::vector<std::string> user_defined_names{std::string(rhs)};
627+
std::string tmp_var_name =
628+
UniqueVarName(user_defined_names, kTupleVarPrefix);
629+
pieces.push_back(
630+
absl::StrFormat("std::vector<::xls::Value> %s;", tmp_var_name));
631+
pieces.push_back(absl::StrFormat("%s.resize(%d);", tmp_var_name, size()));
611632
for (int64_t i = 0; i < size(); ++i) {
612633
std::string assignment = element_emitters_[i]->AssignToValue(
613-
absl::StrFormat("members[%d]", i),
634+
absl::StrFormat("%s[%d]", tmp_var_name, i),
614635
absl::StrFormat("std::get<%d>(%s)", i, rhs), nesting + 1);
615636
pieces.push_back(assignment);
616637
}
617638
pieces.push_back(
618-
absl::StrFormat("%s = ::xls::Value::Tuple(members);", lhs));
639+
absl::StrFormat("%s = ::xls::Value::Tuple(%s);", lhs, tmp_var_name));
619640
return absl::StrFormat("{\n%s\n}", Indent(absl::StrJoin(pieces, "\n"), 2));
620641
}
621642

@@ -803,8 +824,29 @@ std::string SanitizeCppName(std::string_view name) {
803824
return std::string{name};
804825
}
805826

827+
std::string PreserveTrimmedCharacter(std::string_view original,
828+
std::string_view sanitized,
829+
char c_to_preserve) {
830+
bool all_match_so_far = true;
831+
auto still_underscore = [&](char c) -> bool {
832+
all_match_so_far &= (c == c_to_preserve);
833+
return all_match_so_far;
834+
};
835+
int64_t leading_underscores =
836+
std::count_if(original.begin(), original.end(), still_underscore);
837+
all_match_so_far = true;
838+
int64_t trailing_underscores =
839+
std::count_if(original.rbegin(), original.rend(), still_underscore);
840+
return absl::StrCat(std::string(leading_underscores, c_to_preserve),
841+
sanitized,
842+
std::string(trailing_underscores, c_to_preserve));
843+
}
844+
806845
std::string DslxTypeNameToCpp(std::string_view dslx_type) {
807-
return SanitizeCppName(Camelize(dslx_type));
846+
std::string cpp_name = SanitizeCppName(Camelize(dslx_type));
847+
// Retain trimmed underscores to ensure names are unique. This matters for
848+
// user defined names only differing by underscores.
849+
return PreserveTrimmedCharacter(dslx_type, cpp_name, '_');
808850
}
809851

810852
std::string DslxModuleNameToCppNamespace(std::string_view dslx_module) {
@@ -856,4 +898,27 @@ std::string DslxModuleNameToCppNamespace(std::string_view dslx_module) {
856898
"Unsupported TypeAnnotation kind: ", type_annotation->ToString()));
857899
}
858900

901+
std::string UniqueVarName(absl::Span<const std::string> user_names,
902+
std::string_view prefix) {
903+
std::string unique_name = std::string(prefix);
904+
if (!absl::c_contains(user_names, unique_name)) {
905+
return unique_name;
906+
}
907+
std::vector<std::string> existing_user_names;
908+
absl::c_copy_if(user_names, std::back_inserter(existing_user_names),
909+
[prefix](std::string_view name) {
910+
return absl::StartsWith(name, prefix);
911+
});
912+
absl::c_sort(existing_user_names);
913+
// Appending a character avoids collision with previous names because the
914+
// potentially colliding names are sorted. Names [x, x_, x__, ...] will be
915+
// caught by subsequent iterations and enough "_" will be appended.
916+
for (int64_t i = 0; i < existing_user_names.size(); ++i) {
917+
if (existing_user_names[i] == unique_name) {
918+
unique_name += "_";
919+
}
920+
}
921+
return unique_name;
922+
}
923+
859924
} // namespace xls::dslx

‎xls/dslx/cpp_transpiler/cpp_emitter.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
#include "absl/status/statusor.h"
2525
#include "absl/strings/str_cat.h"
26+
#include "absl/types/span.h"
2627
#include "xls/dslx/frontend/ast.h"
2728
#include "xls/dslx/import_data.h"
2829
#include "xls/dslx/type_system/type_info.h"
@@ -40,6 +41,9 @@ std::string DslxTypeNameToCpp(std::string_view dslx_type);
4041
// (https://abseil.io/tips/130).
4142
std::string DslxModuleNameToCppNamespace(std::string_view dslx_module);
4243

44+
std::string UniqueVarName(absl::Span<const std::string> user_names,
45+
std::string_view prefix);
46+
4347
struct CppType {
4448
std::string name;
4549
std::string namespace_prefix = "";

‎xls/dslx/cpp_transpiler/cpp_transpiler_test.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,9 @@ struct MyStruct {
179179
y: u15,
180180
z: u8,
181181
w: s63,
182-
v: u1,
182+
result: u1,
183+
result__: u1,
184+
result_: u1,
183185
})";
184186

185187
auto import_data = CreateImportDataForTest();

‎xls/dslx/cpp_transpiler/cpp_type_generator.cc‎

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include "absl/strings/str_cat.h"
2929
#include "absl/strings/str_format.h"
3030
#include "absl/strings/str_join.h"
31+
#include "absl/types/span.h"
3132
#include "absl/types/variant.h"
3233
#include "xls/common/indent.h"
3334
#include "xls/common/status/status_macros.h"
@@ -465,6 +466,11 @@ class TypeAliasCppTypeGenerator : public CppTypeGenerator {
465466
// A type generator for emitting a C++ struct for representing a DSLX struct.
466467
class StructCppTypeGenerator : public CppTypeGenerator {
467468
public:
469+
// NOTE: until we implement variable name tracking across generated scopes,
470+
// prefixes must be kept unique across all emitters and generators
471+
std::string kResultVarPrefix = "result";
472+
std::string kListVarPrefix = "members";
473+
468474
explicit StructCppTypeGenerator(
469475
std::string_view cpp_type, std::string_view dslx_type,
470476
const StructDef* struct_def,
@@ -558,6 +564,8 @@ class StructCppTypeGenerator : public CppTypeGenerator {
558564

559565
protected:
560566
CppSource FromValueMethod() const {
567+
std::string tmp_var_name =
568+
UniqueVarName(cpp_member_names_, kResultVarPrefix);
561569
std::vector<std::string> pieces;
562570
pieces.push_back(absl::StrFormat(
563571
"if (!value.IsTuple() || value.size() != %d) {", struct_def_->size()));
@@ -566,14 +574,14 @@ class StructCppTypeGenerator : public CppTypeGenerator {
566574
"tuple of %d elements.\");",
567575
struct_def_->size()));
568576
pieces.push_back("}");
569-
pieces.push_back(absl::StrFormat("%s result;", cpp_type()));
577+
pieces.push_back(absl::StrFormat("%s %s;", cpp_type(), tmp_var_name));
570578
for (int i = 0; i < struct_def_->members().size(); i++) {
571579
std::string assignment = member_emitters_[i]->AssignFromValue(
572-
/*lhs=*/absl::StrFormat("result.%s", cpp_member_names_[i]),
580+
/*lhs=*/absl::StrFormat("%s.%s", tmp_var_name, cpp_member_names_[i]),
573581
/*rhs=*/absl::StrFormat("value.element(%d)", i), /*nesting=*/0);
574582
pieces.push_back(assignment);
575583
}
576-
pieces.push_back("return result;");
584+
pieces.push_back(absl::StrFormat("return %s;", tmp_var_name));
577585
std::string body = absl::StrJoin(pieces, "\n");
578586

579587
return CppSource{
@@ -587,17 +595,20 @@ class StructCppTypeGenerator : public CppTypeGenerator {
587595
}
588596

589597
CppSource ToValueMethod() const {
598+
std::string tmp_var_name = UniqueVarName(cpp_member_names_, kListVarPrefix);
590599
std::vector<std::string> pieces;
591-
pieces.push_back("std::vector<::xls::Value> members;");
592600
pieces.push_back(
593-
absl::StrFormat("members.resize(%d);", struct_def_->members().size()));
601+
absl::StrFormat("std::vector<::xls::Value> %s;", tmp_var_name));
602+
pieces.push_back(absl::StrFormat("%s.resize(%d);", tmp_var_name,
603+
struct_def_->members().size()));
594604
for (int i = 0; i < struct_def_->members().size(); i++) {
595605
std::string assignment = member_emitters_[i]->AssignToValue(
596-
/*lhs=*/absl::StrFormat("members[%d]", i),
606+
/*lhs=*/absl::StrFormat("%s[%d]", tmp_var_name, i),
597607
/*rhs=*/cpp_member_names_[i], /*nesting=*/0);
598608
pieces.push_back(assignment);
599609
}
600-
pieces.push_back("return ::xls::Value::Tuple(members);");
610+
pieces.push_back(
611+
absl::StrFormat("return ::xls::Value::Tuple(%s);", tmp_var_name));
601612
std::string body = absl::StrJoin(pieces, "\n");
602613

603614
return CppSource{
@@ -626,19 +637,21 @@ class StructCppTypeGenerator : public CppTypeGenerator {
626637
}
627638

628639
CppSource ToStringMethod() const {
640+
std::string tmp_var_name =
641+
UniqueVarName(cpp_member_names_, kResultVarPrefix);
629642
std::vector<std::string> pieces;
630-
pieces.push_back(
631-
absl::StrFormat("std::string result = \"%s {\\n\";", cpp_type()));
643+
pieces.push_back(absl::StrFormat("std::string %s = \"%s {\\n\";",
644+
tmp_var_name, cpp_type()));
632645
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]));
646+
pieces.push_back(absl::StrFormat("%s += __indent(indent + 1) + \"%s: \";",
647+
tmp_var_name, cpp_member_names_[i]));
635648
std::string to_string = member_emitters_[i]->ToString(
636-
"result", "indent + 2", cpp_member_names_[i], /*nesting=*/0);
649+
tmp_var_name, "indent + 2", cpp_member_names_[i], /*nesting=*/0);
637650
pieces.push_back(to_string);
638-
pieces.push_back("result += \",\\n\";");
651+
pieces.push_back(tmp_var_name + " += \",\\n\";");
639652
}
640-
pieces.push_back("result += __indent(indent) + \"}\";");
641-
pieces.push_back("return result;");
653+
pieces.push_back(tmp_var_name + " += __indent(indent) + \"}\";");
654+
pieces.push_back(absl::StrFormat("return %s;", tmp_var_name));
642655
std::string body = absl::StrJoin(pieces, "\n");
643656

644657
return CppSource{.header = "std::string ToString(int indent = 0) const;",
@@ -648,19 +661,21 @@ class StructCppTypeGenerator : public CppTypeGenerator {
648661
}
649662

650663
CppSource ToDslxStringMethod() const {
664+
std::string tmp_var_name =
665+
UniqueVarName(cpp_member_names_, kResultVarPrefix);
651666
std::vector<std::string> pieces;
652-
pieces.push_back(
653-
absl::StrFormat("std::string result = \"%s {\\n\";", dslx_type()));
667+
pieces.push_back(absl::StrFormat("std::string %s = \"%s {\\n\";",
668+
tmp_var_name, dslx_type()));
654669
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]));
670+
pieces.push_back(absl::StrFormat("%s += __indent(indent + 1) + \"%s: \";",
671+
tmp_var_name, cpp_member_names_[i]));
657672
std::string to_string = member_emitters_[i]->ToDslxString(
658-
"result", "indent + 2", cpp_member_names_[i], /*nesting=*/0);
673+
tmp_var_name, "indent + 2", cpp_member_names_[i], /*nesting=*/0);
659674
pieces.push_back(to_string);
660-
pieces.push_back("result += \",\\n\";");
675+
pieces.push_back(tmp_var_name + " += \",\\n\";");
661676
}
662-
pieces.push_back("result += __indent(indent) + \"}\";");
663-
pieces.push_back("return result;");
677+
pieces.push_back(tmp_var_name + " += __indent(indent) + \"}\";");
678+
pieces.push_back(absl::StrFormat("return %s;", tmp_var_name));
664679
std::string body = absl::StrJoin(pieces, "\n");
665680

666681
return CppSource{

0 commit comments

Comments
 (0)