From 59dee75ce6637aa9363fa9b50c38f0cd5abe0c8d Mon Sep 17 00:00:00 2001 From: Jens Geyer Date: Tue, 30 Jun 2026 22:10:06 +0200 Subject: [PATCH 1/4] THRIFT-6075: Generate Equal method for Delphi Thrift structs Client: delphi Patch: Jens Geyer Co-Authored-By: Claude Sonnet 4.6 --- .../src/thrift/generate/t_delphi_generator.cc | 336 ++++++++++++++++++ .../test/serializer/TestSerializer.Tests.pas | 233 ++++++++++++ 2 files changed, 569 insertions(+) diff --git a/compiler/cpp/src/thrift/generate/t_delphi_generator.cc b/compiler/cpp/src/thrift/generate/t_delphi_generator.cc index 384c9e1f621..cc92dcde2ad 100644 --- a/compiler/cpp/src/thrift/generate/t_delphi_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_delphi_generator.cc @@ -236,6 +236,15 @@ class t_delphi_generator : public t_oop_generator { t_struct* tstruct); void generate_delphi_exception_definition(std::ostream& out, t_struct* tstruct); + void generate_delphi_struct_equality_impl(std::ostream& out, + std::string cls_prefix, + t_struct* tstruct, + bool is_exception); + std::string generate_equal_container(std::ostream& code, + std::ostream& vars, + t_type* ttype, + const std::string& lhs, + const std::string& rhs); void generate_function_helpers(t_function* tfunction); void generate_service_interface(t_service* tservice); @@ -1486,6 +1495,7 @@ void t_delphi_generator::generate_delphi_struct_impl(ostream& out, generate_delphi_struct_reader_impl(out, cls_prefix, tstruct, false); generate_delphi_struct_writer_impl(out, cls_prefix, tstruct, false); generate_delphi_struct_tostring_impl(out, cls_prefix, tstruct, false); + generate_delphi_struct_equality_impl(out, cls_prefix, tstruct, false); } void t_delphi_generator::generate_delphi_exception_impl(ostream& out, @@ -1590,6 +1600,16 @@ void t_delphi_generator::generate_delphi_exception_impl(ostream& out, generate_delphi_struct_reader_impl(out, cls_prefix, tstruct, true); generate_delphi_struct_writer_impl(out, cls_prefix, tstruct, true); generate_delphi_struct_tostring_impl(out, cls_prefix, tstruct, true); + + // Equals override for exception wrapper class + indent_impl(out) << "function " << cls_prefix << cls_nm << ".Equals(Obj: TObject): System.Boolean;" << '\n'; + indent_impl(out) << "begin" << '\n'; + indent_up_impl(); + indent_impl(out) << "if Obj is " << cls_nm << '\n'; + indent_impl(out) << "then Result := FData.Equal(" << cls_nm << "(Obj).FData)" << '\n'; + indent_impl(out) << "else Result := inherited Equals(Obj);" << '\n'; + indent_down_impl(); + indent_impl(out) << "end;" << '\n' << '\n'; } void t_delphi_generator::print_delphi_struct_type_factory_func(ostream& out, t_struct* tstruct) { @@ -1688,6 +1708,9 @@ void t_delphi_generator::generate_delphi_struct_definition(ostream& out, } } + out << '\n'; + indent(out) << "function Equal(const other: IInterface): System.Boolean;" << '\n'; + indent_down(); indent(out) << "end;" << render_deprecation_attribute(tstruct->annotations_, " {", "}") @@ -1753,6 +1776,8 @@ void t_delphi_generator::generate_delphi_struct_definition(ostream& out, out << '\n'; indent(out) << "function ToString: string; override;" << '\n'; + indent(out) << "function Equal(const other: IInterface): System.Boolean;" << '\n'; + indent(out) << "function Equals(Obj: TObject): System.Boolean; override;" << '\n'; out << '\n'; indent(out) << "// IBase" << '\n'; @@ -1848,6 +1873,7 @@ void t_delphi_generator::generate_delphi_exception_definition(ostream& out, out << '\n'; indent(out) << "property ExceptionData : " << struct_intf_name << " read FData;" << '\n'; indent(out) << "function ToString: string; override;" << '\n'; + indent(out) << "function Equals(Obj: TObject): System.Boolean; override;" << '\n'; out << '\n'; indent(out) << "// IBase" << '\n'; @@ -4070,6 +4096,316 @@ void t_delphi_generator::generate_delphi_struct_tostring_impl(ostream& out, indent_impl(out) << "end;" << '\n' << '\n'; } +// Emit comparison code for a container or binary field into `code`, with any new locals +// accumulated into `vars` (2-space prefix, level-independent). Returns the name of the Boolean +// result variable that is set by the generated code. +std::string t_delphi_generator::generate_equal_container(ostream& code, + ostream& vars, + t_type* ttype, + const string& lhs, + const string& rhs) { + ttype = ttype->get_true_type(); + string eq_var = tmp("_eq"); + vars << " " << eq_var << " : System.Boolean;" << '\n'; + indent_impl(code) << eq_var << " := True;" << '\n'; + + if (ttype->is_struct() || ttype->is_xception()) { + indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then " << eq_var << " := False" << '\n'; + indent_impl(code) << "else if " << lhs << " <> nil then " << eq_var << " := " << lhs << ".Equal(" << rhs << ");" << '\n'; + + } else if (ttype->is_binary()) { + if (com_types_) { + // IThriftBytes — nullable interface + indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then " << eq_var << " := False" << '\n'; + indent_impl(code) << "else if " << lhs << " <> nil then begin" << '\n'; + indent_up_impl(); + indent_impl(code) << "if " << lhs << ".Count <> " << rhs << ".Count then " << eq_var << " := False" << '\n'; + indent_impl(code) << "else if " << lhs << ".Count > 0 then" << '\n'; + indent_impl(code) << " " << eq_var << " := SysUtils.CompareMem(" + << lhs << ".QueryRawDataPtr, " << rhs << ".QueryRawDataPtr, " << lhs << ".Count);" << '\n'; + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; + } else if (rtti_) { + // TThriftBytes — packed record with .data and .Length + indent_impl(code) << "if " << lhs << ".Length <> " << rhs << ".Length then " << eq_var << " := False" << '\n'; + indent_impl(code) << "else if " << lhs << ".Length > 0 then" << '\n'; + indent_impl(code) << " " << eq_var << " := SysUtils.CompareMem(" + << "@" << lhs << ".data[0], @" << rhs << ".data[0], " << lhs << ".Length);" << '\n'; + } else { + // SysUtils.TBytes — dynamic array + indent_impl(code) << "if System.Length(" << lhs << ") <> System.Length(" << rhs << ") then " << eq_var << " := False" << '\n'; + indent_impl(code) << "else if System.Length(" << lhs << ") > 0 then" << '\n'; + indent_impl(code) << " " << eq_var << " := SysUtils.CompareMem(" + << "PByte(" << lhs << "), PByte(" << rhs << "), System.Length(" << lhs << "));" << '\n'; + } + + } else if (ttype->is_list()) { + t_list* tlist = (t_list*)ttype; + t_type* elem_type = tlist->get_elem_type()->get_true_type(); + string i_var = tmp("_i"); + vars << " " << i_var << " : System.Integer;" << '\n'; + + indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then " << eq_var << " := False" << '\n'; + indent_impl(code) << "else if " << lhs << " <> nil then begin" << '\n'; + indent_up_impl(); + indent_impl(code) << "if " << lhs << ".Count <> " << rhs << ".Count then " << eq_var << " := False" << '\n'; + indent_impl(code) << "else begin" << '\n'; + indent_up_impl(); + indent_impl(code) << "for " << i_var << " := 0 to " << lhs << ".Count - 1 do begin" << '\n'; + indent_up_impl(); + + bool elem_needs_helper = type_can_be_null(elem_type) || elem_type->is_binary() || elem_type->is_container(); + if (!elem_needs_helper) { + if (elem_type->is_base_type() && ((t_base_type*)elem_type)->get_base() == t_base_type::TYPE_UUID) { + indent_impl(code) << "if not SysUtils.IsEqualGUID(" << lhs << "[" << i_var << "], " + << rhs << "[" << i_var << "]) then begin " << eq_var << " := False; break; end;" << '\n'; + } else { + indent_impl(code) << "if " << lhs << "[" << i_var << "] <> " << rhs << "[" << i_var + << "] then begin " << eq_var << " := False; break; end;" << '\n'; + } + } else { + string inner = generate_equal_container(code, vars, elem_type, + lhs + "[" + i_var + "]", rhs + "[" + i_var + "]"); + indent_impl(code) << "if not " << inner << " then begin " << eq_var << " := False; break; end;" << '\n'; + } + + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end for + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end else (counts equal) + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end else if lhs <> nil + + } else if (ttype->is_set()) { + t_set* tset = (t_set*)ttype; + t_type* elem_type = tset->get_elem_type()->get_true_type(); + bool elem_needs_scan = type_can_be_null(elem_type) || elem_type->is_binary() || elem_type->is_container(); + + indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then " << eq_var << " := False" << '\n'; + indent_impl(code) << "else if " << lhs << " <> nil then begin" << '\n'; + indent_up_impl(); + indent_impl(code) << "if " << lhs << ".Count <> " << rhs << ".Count then " << eq_var << " := False" << '\n'; + indent_impl(code) << "else begin" << '\n'; + indent_up_impl(); + + string e_var = tmp("_e"); + vars << " " << e_var << " : " << type_name(elem_type) << ";" << '\n'; + + if (!elem_needs_scan) { + // Scalar elements: value-based hash equality in Contains is correct + indent_impl(code) << "for " << e_var << " in " << lhs << " do" << '\n'; + indent_impl(code) << " if not " << rhs << ".Contains(" << e_var + << ") then begin " << eq_var << " := False; break; end;" << '\n'; + } else { + // Interface-typed elements: O(n²) scan + string e2_var = tmp("_e"); + string found_var = tmp("_found"); + vars << " " << e2_var << " : " << type_name(elem_type) << ";" << '\n'; + vars << " " << found_var << " : System.Boolean;" << '\n'; + + indent_impl(code) << "for " << e_var << " in " << lhs << " do begin" << '\n'; + indent_up_impl(); + indent_impl(code) << found_var << " := False;" << '\n'; + indent_impl(code) << "for " << e2_var << " in " << rhs << " do begin" << '\n'; + indent_up_impl(); + string inner_eq = generate_equal_container(code, vars, elem_type, e_var, e2_var); + indent_impl(code) << "if " << inner_eq << " then begin " << found_var << " := True; break; end;" << '\n'; + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end inner for + indent_impl(code) << "if not " << found_var << " then begin " << eq_var << " := False; break; end;" << '\n'; + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end outer for + } + + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end else (counts equal) + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end else if lhs <> nil + + } else if (ttype->is_map()) { + t_map* tmap = (t_map*)ttype; + t_type* ktype = tmap->get_key_type()->get_true_type(); + t_type* vtype = tmap->get_val_type()->get_true_type(); + bool key_needs_scan = type_can_be_null(ktype) || ktype->is_binary() || ktype->is_container(); + bool val_needs_helper = type_can_be_null(vtype) || vtype->is_binary() || vtype->is_container(); + + string k_var = tmp("_k"); + vars << " " << k_var << " : " << type_name(ktype) << ";" << '\n'; + + indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then " << eq_var << " := False" << '\n'; + indent_impl(code) << "else if " << lhs << " <> nil then begin" << '\n'; + indent_up_impl(); + indent_impl(code) << "if " << lhs << ".Count <> " << rhs << ".Count then " << eq_var << " := False" << '\n'; + indent_impl(code) << "else begin" << '\n'; + indent_up_impl(); + + if (!key_needs_scan) { + // Scalar key: use ContainsKey then compare values + indent_impl(code) << "for " << k_var << " in " << lhs << ".Keys do begin" << '\n'; + indent_up_impl(); + indent_impl(code) << "if not " << rhs << ".ContainsKey(" << k_var << ") then begin " + << eq_var << " := False; break; end;" << '\n'; + + string vl = lhs + "[" + k_var + "]"; + string vr = rhs + "[" + k_var + "]"; + + if (!val_needs_helper) { + if (vtype->is_base_type() && ((t_base_type*)vtype)->get_base() == t_base_type::TYPE_UUID) { + indent_impl(code) << "if not SysUtils.IsEqualGUID(" << vl << ", " << vr + << ") then begin " << eq_var << " := False; break; end;" << '\n'; + } else { + indent_impl(code) << "if " << vl << " <> " << vr << " then begin " + << eq_var << " := False; break; end;" << '\n'; + } + } else { + string val_inner = generate_equal_container(code, vars, vtype, vl, vr); + indent_impl(code) << "if not " << val_inner << " then begin " << eq_var << " := False; break; end;" << '\n'; + } + + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end for keys + + } else { + // Interface-typed key: O(n²) scan over key pairs + string k2_var = tmp("_k"); + string found_var = tmp("_found"); + vars << " " << k2_var << " : " << type_name(ktype) << ";" << '\n'; + vars << " " << found_var << " : System.Boolean;" << '\n'; + + indent_impl(code) << "for " << k_var << " in " << lhs << ".Keys do begin" << '\n'; + indent_up_impl(); + indent_impl(code) << found_var << " := False;" << '\n'; + indent_impl(code) << "for " << k2_var << " in " << rhs << ".Keys do begin" << '\n'; + indent_up_impl(); + string key_inner = generate_equal_container(code, vars, ktype, k_var, k2_var); + indent_impl(code) << "if " << key_inner << " then begin" << '\n'; + indent_up_impl(); + // Keys match: compare values + string vl = lhs + "[" + k_var + "]"; + string vr = rhs + "[" + k2_var + "]"; + if (!val_needs_helper) { + if (vtype->is_base_type() && ((t_base_type*)vtype)->get_base() == t_base_type::TYPE_UUID) { + indent_impl(code) << "if SysUtils.IsEqualGUID(" << vl << ", " << vr + << ") then begin " << found_var << " := True; break; end;" << '\n'; + } else { + indent_impl(code) << "if " << vl << " = " << vr << " then begin " + << found_var << " := True; break; end;" << '\n'; + } + } else { + string val_inner = generate_equal_container(code, vars, vtype, vl, vr); + indent_impl(code) << "if " << val_inner << " then begin " << found_var << " := True; break; end;" << '\n'; + } + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end if key_inner + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end for k2_var + indent_impl(code) << "if not " << found_var << " then begin " << eq_var << " := False; break; end;" << '\n'; + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end for k_var + } + + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end else (counts equal) + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end else if lhs <> nil + } + + return eq_var; +} + +void t_delphi_generator::generate_delphi_struct_equality_impl(ostream& out, + string cls_prefix, + t_struct* tstruct, + bool is_exception) { + ostringstream local_vars; + ostringstream code_block; + + const vector& fields = tstruct->get_members(); + vector::const_iterator f_iter; + + string cls_nm; + string intf_nm = type_name(tstruct, false, false); + if (is_exception) { + cls_nm = type_name(tstruct, true, true); + } else { + cls_nm = type_name(tstruct, true, false); + } + + indent_impl(code_block) << "begin" << '\n'; + indent_up_impl(); + + indent_impl(code_block) << "if not Supports(other, " << intf_nm << ", _eq_other) then Exit(False);" << '\n'; + + for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) { + t_field* field = *f_iter; + t_type* ftype = field->get_type()->get_true_type(); + bool is_optional = (field->get_req() != t_field::T_REQUIRED); + bool null_allowed = type_can_be_null(ftype); + bool is_bin = ftype->is_base_type() && ((t_base_type*)ftype)->is_binary(); + bool needs_helper = null_allowed || is_bin || ftype->is_container(); + + string self_prop = "Self." + prop_name(field, is_exception); + string other_prop = "_eq_other." + prop_name(field, false); + string isset_self = prop_name(field, is_exception, "__isset_"); + string isset_other = "_eq_other." + prop_name(field, false, "__isset_"); + + if (is_optional) { + indent_impl(code_block) << "if " << isset_self << " <> " << isset_other + << " then Exit(False);" << '\n'; + indent_impl(code_block) << "if " << isset_self << " then begin" << '\n'; + indent_up_impl(); + } + + if (!needs_helper) { + if (ftype->is_base_type() && ((t_base_type*)ftype)->get_base() == t_base_type::TYPE_UUID) { + indent_impl(code_block) << "if not SysUtils.IsEqualGUID(" << self_prop << ", " << other_prop + << ") then Exit(False);" << '\n'; + } else { + indent_impl(code_block) << "if " << self_prop << " <> " << other_prop + << " then Exit(False);" << '\n'; + } + } else { + string eq_var = generate_equal_container(code_block, local_vars, ftype, self_prop, other_prop); + indent_impl(code_block) << "if not " << eq_var << " then Exit(False);" << '\n'; + } + + if (is_optional) { + indent_down_impl(); + indent_impl(code_block) << "end;" << '\n'; + } + } + + indent_impl(code_block) << "Result := True;" << '\n'; + indent_down_impl(); + indent_impl(code_block) << "end;" << '\n' << '\n'; + + // Equal(other: IInterface): Boolean + indent_impl(out) << "function " << cls_prefix << cls_nm << ".Equal(const other: IInterface): System.Boolean;" << '\n'; + indent_impl(out) << "var" << '\n'; + indent_up_impl(); + indent_impl(out) << "_eq_other : " << intf_nm << ";" << '\n'; + indent_down_impl(); + if (!local_vars.str().empty()) { + out << local_vars.str(); + } + out << code_block.str(); + + // Equals(Obj: TObject): Boolean override + string intf_var = tmp("_intf"); + indent_impl(out) << "function " << cls_prefix << cls_nm << ".Equals(Obj: TObject): System.Boolean;" << '\n'; + indent_impl(out) << "var" << '\n'; + indent_up_impl(); + indent_impl(out) << intf_var << " : IInterface;" << '\n'; + indent_down_impl(); + indent_impl(out) << "begin" << '\n'; + indent_up_impl(); + indent_impl(out) << "if Supports(Obj, IInterface, " << intf_var << ")" << '\n'; + indent_impl(out) << "then Result := Equal(" << intf_var << ")" << '\n'; + indent_impl(out) << "else Result := inherited Equals(Obj);" << '\n'; + indent_down_impl(); + indent_impl(out) << "end;" << '\n' << '\n'; +} + bool t_delphi_generator::is_void(t_type* type) { type = type->get_true_type(); diff --git a/lib/delphi/test/serializer/TestSerializer.Tests.pas b/lib/delphi/test/serializer/TestSerializer.Tests.pas index 7f5b829ec51..adc39e917cd 100644 --- a/lib/delphi/test/serializer/TestSerializer.Tests.pas +++ b/lib/delphi/test/serializer/TestSerializer.Tests.pas @@ -85,6 +85,7 @@ TTestSerializer = class //extends TestCase { procedure Test_Serializer_Deserializer; procedure Test_COM_Types; procedure Test_ThriftBytesCTORs; + procedure Test_Equal; procedure Test_OneOfEach( const method : TMethod; const factory : TFactoryPair; const stream : TFileStream); procedure Test_CompactStruct( const method : TMethod; const factory : TFactoryPair; const stream : TFileStream); @@ -506,12 +507,244 @@ procedure TTestSerializer.Test_ThriftBytesCTORs; end; +procedure TTestSerializer.Test_Equal; +var + bonk1, bonk2 : IBonk; + innerBonk1, + innerBonk2 : IBonk; + nesting1, nesting2 : INesting; + tuple1, tuple2 : ITupleProtocolTestStruct; + ooe1, ooe2 : IOneOfEach; + byteList1, + byteList2 : IThriftList; + implA, implB : TBonkImpl; + intfA, intfB : IBonk; + cpst1, cpst2 : ICompactProtoTestStruct; + i32Set1, + i32Set2 : IThriftHashSet; + structSet1, + structSet2 : IThriftHashSet; + map1, map2 : IThriftDictionary; + b64a, b64b : IBase64; + exc1, exc2 : TMutableException; + dummyObj : TObject; +begin + // --- Bonk: required scalars with implicit isset --- + bonk1 := TBonkImpl.Create; + bonk1.&Type := 42; + bonk1.Message := 'hello'; + + ASSERT( not bonk1.Equal(nil)); // nil other -> False + ASSERT( bonk1.Equal(bonk1)); // self -> True + + bonk2 := TBonkImpl.Create; + bonk2.&Type := 42; + bonk2.Message := 'hello'; + ASSERT( bonk1.Equal(bonk2)); // equal values -> True + + bonk2.Message := 'world'; + ASSERT( not bonk1.Equal(bonk2)); // message mismatch -> False + + bonk2.Message := 'hello'; + bonk2.&Type := 99; + ASSERT( not bonk1.Equal(bonk2)); // type mismatch -> False + + // --- TupleProtocolTestStruct: optional fields and isset semantics --- + tuple1 := TTupleProtocolTestStructImpl.Create; + tuple2 := TTupleProtocolTestStructImpl.Create; + ASSERT( tuple1.Equal(tuple2)); // both Field1 unset -> True + + tuple1.Field1 := 1; // sets Field1 and its isset flag + ASSERT( not tuple1.Equal(tuple2)); // isset mismatch -> False + + tuple2.Field1 := 1; + ASSERT( tuple1.Equal(tuple2)); // both set, same value -> True + + tuple2.Field1 := 99; + ASSERT( not tuple1.Equal(tuple2)); // both set, different value -> False + + // --- Nesting: nested struct field --- + innerBonk1 := TBonkImpl.Create; + innerBonk1.&Type := 7; + innerBonk1.Message := 'inner'; + innerBonk2 := TBonkImpl.Create; + innerBonk2.&Type := 7; + innerBonk2.Message := 'inner'; + + nesting1 := TNestingImpl.Create; + nesting1.My_bonk := innerBonk1; + nesting2 := TNestingImpl.Create; + nesting2.My_bonk := innerBonk2; + ASSERT( nesting1.Equal(nesting2)); // equal nested structs -> True + + innerBonk2.Message := 'different'; + ASSERT( not nesting1.Equal(nesting2)); // nested struct mismatch -> False + + nesting2.My_bonk := nil; + ASSERT( not nesting1.Equal(nesting2)); // nil vs non-nil nested struct -> False + + // --- OneOfEach: list field --- + byteList1 := TThriftListImpl.Create; + byteList1.Add(1); + byteList1.Add(2); + byteList2 := TThriftListImpl.Create; + byteList2.Add(1); + byteList2.Add(2); + + ooe1 := TOneOfEachImpl.Create; + ooe1.Byte_list := byteList1; + ooe2 := TOneOfEachImpl.Create; + ooe2.Byte_list := byteList2; + ASSERT( ooe1.Equal(ooe2)); // same list -> True + + byteList2.Add(3); + ASSERT( not ooe1.Equal(ooe2)); // count mismatch -> False + + byteList2 := TThriftListImpl.Create; + byteList2.Add(1); + byteList2.Add(9); + ooe2.Byte_list := byteList2; + ASSERT( not ooe1.Equal(ooe2)); // element mismatch -> False + + // --- CompactProtoTestStruct: set (scalar elements use Contains) --- + cpst1 := TCompactProtoTestStructImpl.Create; + cpst2 := TCompactProtoTestStructImpl.Create; + i32Set1 := TThriftHashSetImpl.Create; + i32Set1.Add(10); + i32Set1.Add(20); + cpst1.I32_set := i32Set1; + i32Set2 := TThriftHashSetImpl.Create; + i32Set2.Add(10); + i32Set2.Add(20); + cpst2.I32_set := i32Set2; + ASSERT( cpst1.Equal(cpst2)); // same set -> True + + i32Set2 := TThriftHashSetImpl.Create; + i32Set2.Add(10); + cpst2.I32_set := i32Set2; + ASSERT( not cpst1.Equal(cpst2)); // count mismatch -> False + + i32Set2 := TThriftHashSetImpl.Create; + i32Set2.Add(10); + i32Set2.Add(99); + cpst2.I32_set := i32Set2; + ASSERT( not cpst1.Equal(cpst2)); // element mismatch -> False + + // --- CompactProtoTestStruct: set (interface elements use O(n^2) scan) --- + cpst1 := TCompactProtoTestStructImpl.Create; + cpst2 := TCompactProtoTestStructImpl.Create; + structSet1 := TThriftHashSetImpl.Create; + structSet1.Add(TEmptyImpl.Create); + cpst1.Struct_set := structSet1; + structSet2 := TThriftHashSetImpl.Create; + structSet2.Add(TEmptyImpl.Create); + cpst2.Struct_set := structSet2; + ASSERT( cpst1.Equal(cpst2)); // both contain one Empty -> True + + cpst2.Struct_set := nil; + ASSERT( not cpst1.Equal(cpst2)); // nil vs non-nil set -> False + + // --- CompactProtoTestStruct: map (scalar keys use ContainsKey) --- + cpst1 := TCompactProtoTestStructImpl.Create; + cpst2 := TCompactProtoTestStructImpl.Create; + map1 := TThriftDictionaryImpl.Create; + map1.AddOrSetValue(1, 10); + map1.AddOrSetValue(2, 20); + cpst1.Byte_byte_map := map1; + map2 := TThriftDictionaryImpl.Create; + map2.AddOrSetValue(1, 10); + map2.AddOrSetValue(2, 20); + cpst2.Byte_byte_map := map2; + ASSERT( cpst1.Equal(cpst2)); // same map -> True + + map2 := TThriftDictionaryImpl.Create; + map2.AddOrSetValue(1, 10); + cpst2.Byte_byte_map := map2; + ASSERT( not cpst1.Equal(cpst2)); // missing key -> False + + map2 := TThriftDictionaryImpl.Create; + map2.AddOrSetValue(1, 10); + map2.AddOrSetValue(2, 99); + cpst2.Byte_byte_map := map2; + ASSERT( not cpst1.Equal(cpst2)); // value mismatch -> False + + // --- Base64 struct: binary field --- + b64a := TBase64Impl.Create; + b64b := TBase64Impl.Create; + {$IF cDebugProtoTest_Option_COM_types} + b64a.B1 := TThriftBytesImpl.Create( TBytes.Create(1, 2, 3)); + b64b.B1 := TThriftBytesImpl.Create( TBytes.Create(1, 2, 3)); + ASSERT( b64a.Equal(b64b)); // same binary -> True + b64b.B1 := TThriftBytesImpl.Create( TBytes.Create(1, 2, 4)); + ASSERT( not b64a.Equal(b64b)); // content mismatch -> False + b64b.B1 := TThriftBytesImpl.Create( TBytes.Create(1, 2)); + ASSERT( not b64a.Equal(b64b)); // length mismatch -> False + {$ELSE} + b64a.B1 := TBytes.Create(1, 2, 3); + b64b.B1 := TBytes.Create(1, 2, 3); + ASSERT( b64a.Equal(b64b)); // same binary -> True + b64b.B1 := TBytes.Create(1, 2, 4); + ASSERT( not b64a.Equal(b64b)); // content mismatch -> False + b64b.B1 := TBytes.Create(1, 2); + ASSERT( not b64a.Equal(b64b)); // length mismatch -> False + {$IFEND} + + // --- Equals(TObject) --- + implA := TBonkImpl.Create; + intfA := implA; // ref count -> 1: implA managed by intfA + implA.&Type := 5; + implA.Message := 'eq_test'; + + implB := TBonkImpl.Create; + intfB := implB; // ref count -> 1: implB managed by intfB + implB.&Type := 5; + implB.Message := 'eq_test'; + ASSERT( implA.Equals(implB)); // same values, compatible type -> True + + implB.Message := 'other'; + ASSERT( not implA.Equals(implB)); // value mismatch -> False + + dummyObj := TObject.Create; + try + ASSERT( not implA.Equals(dummyObj)); // incompatible type -> False + finally + dummyObj.Free; + end; + + intfA := nil; // releases implA (ref count -> 0) + intfB := nil; // releases implB + + // --- Exception wrapper: TMutableException.Equals --- + exc1 := TMutableException.Create; + exc2 := TMutableException.Create; + try + exc1.Msg := 'same'; + exc2.Msg := 'same'; + ASSERT( exc1.Equals(exc2)); // same field values -> True + + exc2.Msg := 'different'; + ASSERT( not exc1.Equals(exc2)); // field value mismatch -> False + + dummyObj := TObject.Create; + try + ASSERT( not exc1.Equals(dummyObj)); // incompatible type -> False + finally + dummyObj.Free; + end; + finally + FreeAndNil(exc1); + FreeAndNil(exc2); + end; +end; + + procedure TTestSerializer.RunTests; begin try Test_Serializer_Deserializer; Test_COM_Types; Test_ThriftBytesCTORs; + Test_Equal; except on e:Exception do begin Writeln( e.ClassName+': '+ e.Message); From ae19c8de5030152f0ee6d07d430b0319caf6a69f Mon Sep 17 00:00:00 2001 From: Jens Geyer Date: Wed, 1 Jul 2026 02:41:45 +0200 Subject: [PATCH 2/4] THRIFT-6075: Fix set/map Equal to consume matched elements, add regression tests Client: delphi The set/map equality scan in generate_equal_container checked only that every lhs element had some match in rhs, without marking matched rhs entries as consumed. One rhs element could satisfy multiple lhs elements, producing a false Equal=True when one side held reference-distinct but value-equal elements (e.g. two struct-typed set elements with the same field values, reachable because the generated collections use no custom IEqualityComparer and Delphi's default comparer for interface-typed elements is reference-based). Both the set and map branches now match against an indexed copy of rhs plus a parallel used-flags array, marking entries consumed on match. Adds EqualityItem/EqualityItemContainers fixtures to SimpleException.thrift (Empty has no fields and can't express two distinct-but-equal instances) and set/map duplicate-value and duplicate-key regression cases to Test_Equal, alongside a sanity check for the legitimate-equal case. Co-Authored-By: Claude Sonnet 5 --- .../src/thrift/generate/t_delphi_generator.cc | 105 ++++++++++++++---- .../test/serializer/SimpleException.thrift | 12 ++ .../test/serializer/TestSerializer.Tests.pas | 65 +++++++++++ 3 files changed, 162 insertions(+), 20 deletions(-) diff --git a/compiler/cpp/src/thrift/generate/t_delphi_generator.cc b/compiler/cpp/src/thrift/generate/t_delphi_generator.cc index cc92dcde2ad..63b7185e470 100644 --- a/compiler/cpp/src/thrift/generate/t_delphi_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_delphi_generator.cc @@ -4197,21 +4197,52 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, indent_impl(code) << " if not " << rhs << ".Contains(" << e_var << ") then begin " << eq_var << " := False; break; end;" << '\n'; } else { - // Interface-typed elements: O(n²) scan - string e2_var = tmp("_e"); + // Struct/binary/container elements: O(n²) scan that marks matched rhs elements + // as consumed, so a single rhs element cannot satisfy more than one lhs element. + // This matters because the underlying set may legitimately hold multiple elements + // that are .Equal() to one another but distinct by reference (the collection's own + // uniqueness check is reference-based for non-scalar element types). + string arr_var = tmp("_arr"); + string used_var = tmp("_used"); + string idx_var = tmp("_i"); string found_var = tmp("_found"); - vars << " " << e2_var << " : " << type_name(elem_type) << ";" << '\n'; + string e2_var = tmp("_e"); + vars << " " << arr_var << " : array of " << type_name(elem_type) << ";" << '\n'; + vars << " " << used_var << " : array of System.Boolean;" << '\n'; + vars << " " << idx_var << " : System.Integer;" << '\n'; vars << " " << found_var << " : System.Boolean;" << '\n'; + vars << " " << e2_var << " : " << type_name(elem_type) << ";" << '\n'; + + indent_impl(code) << "System.SetLength(" << arr_var << ", " << rhs << ".Count);" << '\n'; + indent_impl(code) << "System.SetLength(" << used_var << ", " << rhs << ".Count);" << '\n'; + indent_impl(code) << idx_var << " := 0;" << '\n'; + indent_impl(code) << "for " << e2_var << " in " << rhs << " do begin" << '\n'; + indent_up_impl(); + indent_impl(code) << arr_var << "[" << idx_var << "] := " << e2_var << ";" << '\n'; + indent_impl(code) << "System.Inc(" << idx_var << ");" << '\n'; + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; indent_impl(code) << "for " << e_var << " in " << lhs << " do begin" << '\n'; indent_up_impl(); indent_impl(code) << found_var << " := False;" << '\n'; - indent_impl(code) << "for " << e2_var << " in " << rhs << " do begin" << '\n'; + indent_impl(code) << "for " << idx_var << " := 0 to System.Length(" << arr_var << ") - 1 do begin" << '\n'; + indent_up_impl(); + indent_impl(code) << "if not " << used_var << "[" << idx_var << "] then begin" << '\n'; + indent_up_impl(); + string inner_eq = generate_equal_container(code, vars, elem_type, e_var, + arr_var + "[" + idx_var + "]"); + indent_impl(code) << "if " << inner_eq << " then begin" << '\n'; indent_up_impl(); - string inner_eq = generate_equal_container(code, vars, elem_type, e_var, e2_var); - indent_impl(code) << "if " << inner_eq << " then begin " << found_var << " := True; break; end;" << '\n'; + indent_impl(code) << used_var << "[" << idx_var << "] := True;" << '\n'; + indent_impl(code) << found_var << " := True;" << '\n'; + indent_impl(code) << "break;" << '\n'; indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end inner for + indent_impl(code) << "end;" << '\n'; // end if matched + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end if not yet used + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end scan for indent_impl(code) << "if not " << found_var << " then begin " << eq_var << " := False; break; end;" << '\n'; indent_down_impl(); indent_impl(code) << "end;" << '\n'; // end outer for @@ -4266,39 +4297,73 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, indent_impl(code) << "end;" << '\n'; // end for keys } else { - // Interface-typed key: O(n²) scan over key pairs - string k2_var = tmp("_k"); + // Struct/binary/container key: O(n²) scan over key pairs that marks matched rhs + // keys as consumed, so a single rhs key cannot satisfy more than one lhs key + // (same rationale as the analogous set case above). + string karr_var = tmp("_karr"); + string used_var = tmp("_used"); + string idx_var = tmp("_i"); string found_var = tmp("_found"); - vars << " " << k2_var << " : " << type_name(ktype) << ";" << '\n'; + string k2_var = tmp("_k"); + vars << " " << karr_var << " : array of " << type_name(ktype) << ";" << '\n'; + vars << " " << used_var << " : array of System.Boolean;" << '\n'; + vars << " " << idx_var << " : System.Integer;" << '\n'; vars << " " << found_var << " : System.Boolean;" << '\n'; + vars << " " << k2_var << " : " << type_name(ktype) << ";" << '\n'; + + indent_impl(code) << "System.SetLength(" << karr_var << ", " << rhs << ".Count);" << '\n'; + indent_impl(code) << "System.SetLength(" << used_var << ", " << rhs << ".Count);" << '\n'; + indent_impl(code) << idx_var << " := 0;" << '\n'; + indent_impl(code) << "for " << k2_var << " in " << rhs << ".Keys do begin" << '\n'; + indent_up_impl(); + indent_impl(code) << karr_var << "[" << idx_var << "] := " << k2_var << ";" << '\n'; + indent_impl(code) << "System.Inc(" << idx_var << ");" << '\n'; + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; indent_impl(code) << "for " << k_var << " in " << lhs << ".Keys do begin" << '\n'; indent_up_impl(); indent_impl(code) << found_var << " := False;" << '\n'; - indent_impl(code) << "for " << k2_var << " in " << rhs << ".Keys do begin" << '\n'; + indent_impl(code) << "for " << idx_var << " := 0 to System.Length(" << karr_var << ") - 1 do begin" << '\n'; + indent_up_impl(); + indent_impl(code) << "if not " << used_var << "[" << idx_var << "] then begin" << '\n'; indent_up_impl(); - string key_inner = generate_equal_container(code, vars, ktype, k_var, k2_var); + string key_inner = generate_equal_container(code, vars, ktype, k_var, + karr_var + "[" + idx_var + "]"); indent_impl(code) << "if " << key_inner << " then begin" << '\n'; indent_up_impl(); - // Keys match: compare values + // Keys match: compare values, and only consume the rhs key/mark found if the + // value matches too (a key-only match with a differing value must keep scanning). string vl = lhs + "[" + k_var + "]"; - string vr = rhs + "[" + k2_var + "]"; + string vr = rhs + "[" + karr_var + "[" + idx_var + "]]"; if (!val_needs_helper) { if (vtype->is_base_type() && ((t_base_type*)vtype)->get_base() == t_base_type::TYPE_UUID) { - indent_impl(code) << "if SysUtils.IsEqualGUID(" << vl << ", " << vr - << ") then begin " << found_var << " := True; break; end;" << '\n'; + indent_impl(code) << "if SysUtils.IsEqualGUID(" << vl << ", " << vr << ") then begin" << '\n'; } else { - indent_impl(code) << "if " << vl << " = " << vr << " then begin " - << found_var << " := True; break; end;" << '\n'; + indent_impl(code) << "if " << vl << " = " << vr << " then begin" << '\n'; } + indent_up_impl(); + indent_impl(code) << used_var << "[" << idx_var << "] := True;" << '\n'; + indent_impl(code) << found_var << " := True;" << '\n'; + indent_impl(code) << "break;" << '\n'; + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end if value matches } else { string val_inner = generate_equal_container(code, vars, vtype, vl, vr); - indent_impl(code) << "if " << val_inner << " then begin " << found_var << " := True; break; end;" << '\n'; + indent_impl(code) << "if " << val_inner << " then begin" << '\n'; + indent_up_impl(); + indent_impl(code) << used_var << "[" << idx_var << "] := True;" << '\n'; + indent_impl(code) << found_var << " := True;" << '\n'; + indent_impl(code) << "break;" << '\n'; + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end if val_inner } indent_down_impl(); indent_impl(code) << "end;" << '\n'; // end if key_inner indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end for k2_var + indent_impl(code) << "end;" << '\n'; // end if not yet used + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end scan for indent_impl(code) << "if not " << found_var << " then begin " << eq_var << " := False; break; end;" << '\n'; indent_down_impl(); indent_impl(code) << "end;" << '\n'; // end for k_var diff --git a/lib/delphi/test/serializer/SimpleException.thrift b/lib/delphi/test/serializer/SimpleException.thrift index c13876c8d74..adfad149678 100644 --- a/lib/delphi/test/serializer/SimpleException.thrift +++ b/lib/delphi/test/serializer/SimpleException.thrift @@ -27,6 +27,18 @@ exception Error { 3: uuid ExceptionData = '00000000-4444-CCCC-ffff-0123456789ab' } +// Fixtures for TTestSerializer.Test_Equal: a struct with a non-trivial field, +// used as a set element / map key so tests can construct reference-distinct +// but value-equal instances (Empty, used elsewhere, cannot express this). +struct EqualityItem { + 1: required i32 Value; +} + +struct EqualityItemContainers { + 1: required set Items; + 2: required map ItemMap; +} + // EOF diff --git a/lib/delphi/test/serializer/TestSerializer.Tests.pas b/lib/delphi/test/serializer/TestSerializer.Tests.pas index adc39e917cd..6342c223233 100644 --- a/lib/delphi/test/serializer/TestSerializer.Tests.pas +++ b/lib/delphi/test/serializer/TestSerializer.Tests.pas @@ -525,6 +525,9 @@ procedure TTestSerializer.Test_Equal; structSet1, structSet2 : IThriftHashSet; map1, map2 : IThriftDictionary; + eqc1, eqc2 : IEqualityItemContainers; + eqItemA, eqItemB, + eqItemC : IEqualityItem; b64a, b64b : IBase64; exc1, exc2 : TMutableException; dummyObj : TObject; @@ -668,6 +671,68 @@ procedure TTestSerializer.Test_Equal; cpst2.Byte_byte_map := map2; ASSERT( not cpst1.Equal(cpst2)); // value mismatch -> False + // --- EqualityItemContainers.Items (set): duplicate-value regression --- + // Two distinct EqualityItem instances with the same value can legitimately both + // land in one set (TThriftHashSetImpl's default comparer is reference-based for + // interface-typed T, so it does not dedup by value). A set-equality scan that does + // not mark matched rhs entries as consumed could let one rhs entry satisfy two + // different lhs entries, incorrectly reporting equality when the sets differ + // (THRIFT-6075 review finding). + eqc1 := TEqualityItemContainersImpl.Create; + eqc1.Items := TThriftHashSetImpl.Create; + eqItemA := TEqualityItemImpl.Create; + eqItemA.Value := 5; + eqItemB := TEqualityItemImpl.Create; + eqItemB.Value := 5; // distinct instance, same value as eqItemA + eqc1.Items.Add(eqItemA); + eqc1.Items.Add(eqItemB); + eqc1.ItemMap := TThriftDictionaryImpl.Create; + + eqc2 := TEqualityItemContainersImpl.Create; + eqc2.Items := TThriftHashSetImpl.Create; + eqItemC := TEqualityItemImpl.Create; + eqItemC.Value := 5; // matches eqItemA/eqItemB by value + eqc2.Items.Add(eqItemC); + eqItemC := TEqualityItemImpl.Create; + eqItemC.Value := 7; // no counterpart in eqc1 -> sets differ + eqc2.Items.Add(eqItemC); + eqc2.ItemMap := TThriftDictionaryImpl.Create; + + ASSERT( not eqc1.Equal(eqc2)); // counts match (2=2) but eqc2 holds a value (7) eqc1 lacks + + // Sanity check: a genuinely equal pair (no extra/duplicate values) still passes. + eqc1.Items := TThriftHashSetImpl.Create; + eqItemA := TEqualityItemImpl.Create; + eqItemA.Value := 9; + eqc1.Items.Add(eqItemA); + + eqc2.Items := TThriftHashSetImpl.Create; + eqItemC := TEqualityItemImpl.Create; + eqItemC.Value := 9; + eqc2.Items.Add(eqItemC); + ASSERT( eqc1.Equal(eqc2)); // single matching value on both sides -> True + + // --- EqualityItemContainers.ItemMap (map): same duplicate-key scenario --- + eqc1.ItemMap := TThriftDictionaryImpl.Create; + eqItemA := TEqualityItemImpl.Create; + eqItemA.Value := 5; + eqItemB := TEqualityItemImpl.Create; + eqItemB.Value := 5; // distinct key instance, same value as eqItemA + eqc1.ItemMap.AddOrSetValue(eqItemA, 100); + eqc1.ItemMap.AddOrSetValue(eqItemB, 100); + + eqc2.ItemMap := TThriftDictionaryImpl.Create; + eqItemC := TEqualityItemImpl.Create; + eqItemC.Value := 5; + eqc2.ItemMap.AddOrSetValue(eqItemC, 100); + eqItemC := TEqualityItemImpl.Create; + eqItemC.Value := 7; + eqc2.ItemMap.AddOrSetValue(eqItemC, 999); + + // eqc1.Items/eqc2.Items are still equal from the sanity check above (both single + // value 9), so this isolates the mismatch to ItemMap. + ASSERT( not eqc1.Equal(eqc2)); // counts match (2=2) but eqc2 holds a key/value (7->999) eqc1 lacks + // --- Base64 struct: binary field --- b64a := TBase64Impl.Create; b64b := TBase64Impl.Create; From 3e462a756cf467cd27104af1e58c717f15b64165 Mon Sep 17 00:00:00 2001 From: Jens Geyer Date: Wed, 1 Jul 2026 02:49:23 +0200 Subject: [PATCH 3/4] THRIFT-6075: Remove dead is_exception branch, apply clang-format to new code Client: delphi generate_delphi_struct_equality_impl took an is_exception parameter whose only call site always passed false; the true branch was unreachable (exceptions get their equality via a separate hand-written Equals(TObject) wrapper). Removed the parameter and the dead branch. Ran git clang-format scoped to the lines changed since 289a1c1e, rather than a full-file reformat, since this file was not clang-format-conformant before this PR and a full pass would have produced an unrelated diff. Verified both changes are no-ops: rebuilt the compiler and regenerated test/DebugProtoTest.thrift (default and com_types) before and after, output is byte-identical. Co-Authored-By: Claude Sonnet 5 --- .../src/thrift/generate/t_delphi_generator.cc | 52 +++++++++---------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/compiler/cpp/src/thrift/generate/t_delphi_generator.cc b/compiler/cpp/src/thrift/generate/t_delphi_generator.cc index 63b7185e470..b4eb2d9dbb3 100644 --- a/compiler/cpp/src/thrift/generate/t_delphi_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_delphi_generator.cc @@ -238,8 +238,7 @@ class t_delphi_generator : public t_oop_generator { t_struct* tstruct); void generate_delphi_struct_equality_impl(std::ostream& out, std::string cls_prefix, - t_struct* tstruct, - bool is_exception); + t_struct* tstruct); std::string generate_equal_container(std::ostream& code, std::ostream& vars, t_type* ttype, @@ -1495,7 +1494,7 @@ void t_delphi_generator::generate_delphi_struct_impl(ostream& out, generate_delphi_struct_reader_impl(out, cls_prefix, tstruct, false); generate_delphi_struct_writer_impl(out, cls_prefix, tstruct, false); generate_delphi_struct_tostring_impl(out, cls_prefix, tstruct, false); - generate_delphi_struct_equality_impl(out, cls_prefix, tstruct, false); + generate_delphi_struct_equality_impl(out, cls_prefix, tstruct); } void t_delphi_generator::generate_delphi_exception_impl(ostream& out, @@ -4226,23 +4225,24 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, indent_impl(code) << "for " << e_var << " in " << lhs << " do begin" << '\n'; indent_up_impl(); indent_impl(code) << found_var << " := False;" << '\n'; - indent_impl(code) << "for " << idx_var << " := 0 to System.Length(" << arr_var << ") - 1 do begin" << '\n'; + indent_impl(code) << "for " << idx_var << " := 0 to System.Length(" << arr_var + << ") - 1 do begin" << '\n'; indent_up_impl(); indent_impl(code) << "if not " << used_var << "[" << idx_var << "] then begin" << '\n'; indent_up_impl(); - string inner_eq = generate_equal_container(code, vars, elem_type, e_var, - arr_var + "[" + idx_var + "]"); + string inner_eq + = generate_equal_container(code, vars, elem_type, e_var, arr_var + "[" + idx_var + "]"); indent_impl(code) << "if " << inner_eq << " then begin" << '\n'; indent_up_impl(); indent_impl(code) << used_var << "[" << idx_var << "] := True;" << '\n'; indent_impl(code) << found_var << " := True;" << '\n'; indent_impl(code) << "break;" << '\n'; indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end if matched + indent_impl(code) << "end;" << '\n'; // end if matched indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end if not yet used + indent_impl(code) << "end;" << '\n'; // end if not yet used indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end scan for + indent_impl(code) << "end;" << '\n'; // end scan for indent_impl(code) << "if not " << found_var << " then begin " << eq_var << " := False; break; end;" << '\n'; indent_down_impl(); indent_impl(code) << "end;" << '\n'; // end outer for @@ -4324,12 +4324,13 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, indent_impl(code) << "for " << k_var << " in " << lhs << ".Keys do begin" << '\n'; indent_up_impl(); indent_impl(code) << found_var << " := False;" << '\n'; - indent_impl(code) << "for " << idx_var << " := 0 to System.Length(" << karr_var << ") - 1 do begin" << '\n'; + indent_impl(code) << "for " << idx_var << " := 0 to System.Length(" << karr_var + << ") - 1 do begin" << '\n'; indent_up_impl(); indent_impl(code) << "if not " << used_var << "[" << idx_var << "] then begin" << '\n'; indent_up_impl(); - string key_inner = generate_equal_container(code, vars, ktype, k_var, - karr_var + "[" + idx_var + "]"); + string key_inner + = generate_equal_container(code, vars, ktype, k_var, karr_var + "[" + idx_var + "]"); indent_impl(code) << "if " << key_inner << " then begin" << '\n'; indent_up_impl(); // Keys match: compare values, and only consume the rhs key/mark found if the @@ -4338,7 +4339,8 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, string vr = rhs + "[" + karr_var + "[" + idx_var + "]]"; if (!val_needs_helper) { if (vtype->is_base_type() && ((t_base_type*)vtype)->get_base() == t_base_type::TYPE_UUID) { - indent_impl(code) << "if SysUtils.IsEqualGUID(" << vl << ", " << vr << ") then begin" << '\n'; + indent_impl(code) << "if SysUtils.IsEqualGUID(" << vl << ", " << vr << ") then begin" + << '\n'; } else { indent_impl(code) << "if " << vl << " = " << vr << " then begin" << '\n'; } @@ -4347,7 +4349,7 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, indent_impl(code) << found_var << " := True;" << '\n'; indent_impl(code) << "break;" << '\n'; indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end if value matches + indent_impl(code) << "end;" << '\n'; // end if value matches } else { string val_inner = generate_equal_container(code, vars, vtype, vl, vr); indent_impl(code) << "if " << val_inner << " then begin" << '\n'; @@ -4356,14 +4358,14 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, indent_impl(code) << found_var << " := True;" << '\n'; indent_impl(code) << "break;" << '\n'; indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end if val_inner + indent_impl(code) << "end;" << '\n'; // end if val_inner } indent_down_impl(); indent_impl(code) << "end;" << '\n'; // end if key_inner indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end if not yet used + indent_impl(code) << "end;" << '\n'; // end if not yet used indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end scan for + indent_impl(code) << "end;" << '\n'; // end scan for indent_impl(code) << "if not " << found_var << " then begin " << eq_var << " := False; break; end;" << '\n'; indent_down_impl(); indent_impl(code) << "end;" << '\n'; // end for k_var @@ -4379,22 +4381,16 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, } void t_delphi_generator::generate_delphi_struct_equality_impl(ostream& out, - string cls_prefix, - t_struct* tstruct, - bool is_exception) { + string cls_prefix, + t_struct* tstruct) { ostringstream local_vars; ostringstream code_block; const vector& fields = tstruct->get_members(); vector::const_iterator f_iter; - string cls_nm; + string cls_nm = type_name(tstruct, true, false); string intf_nm = type_name(tstruct, false, false); - if (is_exception) { - cls_nm = type_name(tstruct, true, true); - } else { - cls_nm = type_name(tstruct, true, false); - } indent_impl(code_block) << "begin" << '\n'; indent_up_impl(); @@ -4409,9 +4405,9 @@ void t_delphi_generator::generate_delphi_struct_equality_impl(ostream& out, bool is_bin = ftype->is_base_type() && ((t_base_type*)ftype)->is_binary(); bool needs_helper = null_allowed || is_bin || ftype->is_container(); - string self_prop = "Self." + prop_name(field, is_exception); + string self_prop = "Self." + prop_name(field, false); string other_prop = "_eq_other." + prop_name(field, false); - string isset_self = prop_name(field, is_exception, "__isset_"); + string isset_self = prop_name(field, false, "__isset_"); string isset_other = "_eq_other." + prop_name(field, false, "__isset_"); if (is_optional) { From c4eda463ecb14c05ccaafb5b43a773298a9d88d4 Mon Sep 17 00:00:00 2001 From: Jens Geyer Date: Wed, 1 Jul 2026 23:48:41 +0200 Subject: [PATCH 4/4] THRIFT-6075: Simplify generated Equal code and reduce its runtime cost Client: delphi Three changes to generate_equal_container, none of which alter the comparison semantics: 1. Capture operands into locals: the recursion previously passed composed access-path strings down, so expressions like Self.List_field[_i][_k][_k2] (property getter + GetItem + two hash lookups) were re-evaluated at every mention, up to ~8x per loop iteration in deeply nested containers. Operands are now captured into fresh locals once per level (already_local skips the copy where the operand is a loop variable, pair field or snapshot array slot). 2. Compare maps via pair enumeration and TryGetValue: the scalar-key path iterates lhs pairs and fetches the rhs value with a single lookup (was three: ContainsKey + lhs[k] + rhs[k], plus repeated getters). The non-scalar-key consume-scan snapshots rhs keys and values into parallel arrays, removing hash lookups from that path entirely. 3. Exit(False) directly on mismatch: a new exit_on_fail mode lets field-level, list-element and scalar-key-map comparisons leave the generated function immediately instead of threading _eq flags and break statements through every nesting level. The flag-and-fall- through form remains only for the candidate probes inside the set/map consume-scans, where a mismatch means "try the next candidate" rather than "structs differ". Verified by rebuilding the compiler and regenerating ThriftTest, DebugProtoTest and SimpleException in default, com_types and rtti modes: all 358 generated Equal/Equals functions are begin/end-balanced, structs without containers/binaries generate byte-identical code, and the duplicate-element consume-scan regression shape is preserved. Co-Authored-By: Claude Fable 5 --- .../src/thrift/generate/t_delphi_generator.cc | 329 ++++++++++++------ 1 file changed, 216 insertions(+), 113 deletions(-) diff --git a/compiler/cpp/src/thrift/generate/t_delphi_generator.cc b/compiler/cpp/src/thrift/generate/t_delphi_generator.cc index b4eb2d9dbb3..37e7dfba661 100644 --- a/compiler/cpp/src/thrift/generate/t_delphi_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_delphi_generator.cc @@ -242,8 +242,10 @@ class t_delphi_generator : public t_oop_generator { std::string generate_equal_container(std::ostream& code, std::ostream& vars, t_type* ttype, - const std::string& lhs, - const std::string& rhs); + const std::string& lhs_in, + const std::string& rhs_in, + bool exit_on_fail, + bool already_local); void generate_function_helpers(t_function* tfunction); void generate_service_interface(t_service* tservice); @@ -4095,47 +4097,150 @@ void t_delphi_generator::generate_delphi_struct_tostring_impl(ostream& out, indent_impl(out) << "end;" << '\n' << '\n'; } -// Emit comparison code for a container or binary field into `code`, with any new locals -// accumulated into `vars` (2-space prefix, level-independent). Returns the name of the Boolean -// result variable that is set by the generated code. +// Emit comparison code for a struct, binary or container value into `code`, with any new +// locals accumulated into `vars` (2-space prefix, level-independent). +// +// In exit mode (exit_on_fail = true) a mismatch leaves the generated Equal function directly +// via Exit(False) and an empty string is returned. In trial mode a mismatch only records the +// outcome: a fresh Boolean variable, whose name is returned, is set to False and control falls +// through, so the caller can keep probing other candidates. The set/map consume-scans below +// rely on that to test rhs candidates without aborting the whole comparison. +// +// Unless already_local is set, both operands are captured into fresh locals first, so property +// getters, GetItem calls and hash lookups are evaluated once instead of once per mention. std::string t_delphi_generator::generate_equal_container(ostream& code, - ostream& vars, - t_type* ttype, - const string& lhs, - const string& rhs) { + ostream& vars, + t_type* ttype, + const string& lhs_in, + const string& rhs_in, + bool exit_on_fail, + bool already_local) { ttype = ttype->get_true_type(); - string eq_var = tmp("_eq"); - vars << " " << eq_var << " : System.Boolean;" << '\n'; - indent_impl(code) << eq_var << " := True;" << '\n'; + + string lhs = lhs_in; + string rhs = rhs_in; + if (!already_local) { + lhs = tmp("_l"); + rhs = tmp("_r"); + vars << " " << lhs << " : " << type_name(ttype) << ";" << '\n'; + vars << " " << rhs << " : " << type_name(ttype) << ";" << '\n'; + indent_impl(code) << lhs << " := " << lhs_in << ";" << '\n'; + indent_impl(code) << rhs << " := " << rhs_in << ";" << '\n'; + } + + string eq_var; + if (!exit_on_fail) { + eq_var = tmp("_eq"); + vars << " " << eq_var << " : System.Boolean;" << '\n'; + indent_impl(code) << eq_var << " := True;" << '\n'; + } + + // Mismatch action for statements inside the per-element loops below. + const string fail_stmt + = exit_on_fail ? "Exit(False);" : "begin " + eq_var + " := False; break; end;"; + + // Shared nil/Count guards around the per-element loops of list/set/map. + auto container_prolog = [&]() { + if (exit_on_fail) { + indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then Exit(False);" + << '\n'; + indent_impl(code) << "if " << lhs << " <> nil then begin" << '\n'; + indent_up_impl(); + indent_impl(code) << "if " << lhs << ".Count <> " << rhs << ".Count then Exit(False);" + << '\n'; + } else { + indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then " << eq_var + << " := False" << '\n'; + indent_impl(code) << "else if " << lhs << " <> nil then begin" << '\n'; + indent_up_impl(); + indent_impl(code) << "if " << lhs << ".Count <> " << rhs << ".Count then " << eq_var + << " := False" << '\n'; + indent_impl(code) << "else begin" << '\n'; + indent_up_impl(); + } + }; + auto container_epilog = [&]() { + if (!exit_on_fail) { + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end else (counts equal) + } + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; // end if lhs <> nil + }; if (ttype->is_struct() || ttype->is_xception()) { - indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then " << eq_var << " := False" << '\n'; - indent_impl(code) << "else if " << lhs << " <> nil then " << eq_var << " := " << lhs << ".Equal(" << rhs << ");" << '\n'; + if (exit_on_fail) { + indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then Exit(False);" + << '\n'; + indent_impl(code) << "if (" << lhs << " <> nil) and not " << lhs << ".Equal(" << rhs + << ") then Exit(False);" << '\n'; + } else { + indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then " << eq_var + << " := False" << '\n'; + indent_impl(code) << "else if " << lhs << " <> nil then " << eq_var << " := " << lhs + << ".Equal(" << rhs << ");" << '\n'; + } } else if (ttype->is_binary()) { if (com_types_) { // IThriftBytes — nullable interface - indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then " << eq_var << " := False" << '\n'; - indent_impl(code) << "else if " << lhs << " <> nil then begin" << '\n'; - indent_up_impl(); - indent_impl(code) << "if " << lhs << ".Count <> " << rhs << ".Count then " << eq_var << " := False" << '\n'; - indent_impl(code) << "else if " << lhs << ".Count > 0 then" << '\n'; - indent_impl(code) << " " << eq_var << " := SysUtils.CompareMem(" - << lhs << ".QueryRawDataPtr, " << rhs << ".QueryRawDataPtr, " << lhs << ".Count);" << '\n'; - indent_down_impl(); - indent_impl(code) << "end;" << '\n'; + if (exit_on_fail) { + indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then Exit(False);" + << '\n'; + indent_impl(code) << "if " << lhs << " <> nil then begin" << '\n'; + indent_up_impl(); + indent_impl(code) << "if " << lhs << ".Count <> " << rhs << ".Count then Exit(False);" + << '\n'; + indent_impl(code) << "if (" << lhs << ".Count > 0) and not SysUtils.CompareMem(" << lhs + << ".QueryRawDataPtr, " << rhs << ".QueryRawDataPtr, " << lhs + << ".Count) then Exit(False);" << '\n'; + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; + } else { + indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then " << eq_var + << " := False" << '\n'; + indent_impl(code) << "else if " << lhs << " <> nil then begin" << '\n'; + indent_up_impl(); + indent_impl(code) << "if " << lhs << ".Count <> " << rhs << ".Count then " << eq_var + << " := False" << '\n'; + indent_impl(code) << "else if " << lhs << ".Count > 0 then" << '\n'; + indent_impl(code) << " " << eq_var << " := SysUtils.CompareMem(" << lhs + << ".QueryRawDataPtr, " << rhs << ".QueryRawDataPtr, " << lhs + << ".Count);" << '\n'; + indent_down_impl(); + indent_impl(code) << "end;" << '\n'; + } } else if (rtti_) { // TThriftBytes — packed record with .data and .Length - indent_impl(code) << "if " << lhs << ".Length <> " << rhs << ".Length then " << eq_var << " := False" << '\n'; - indent_impl(code) << "else if " << lhs << ".Length > 0 then" << '\n'; - indent_impl(code) << " " << eq_var << " := SysUtils.CompareMem(" - << "@" << lhs << ".data[0], @" << rhs << ".data[0], " << lhs << ".Length);" << '\n'; + if (exit_on_fail) { + indent_impl(code) << "if " << lhs << ".Length <> " << rhs << ".Length then Exit(False);" + << '\n'; + indent_impl(code) << "if (" << lhs << ".Length > 0) and not SysUtils.CompareMem(@" << lhs + << ".data[0], @" << rhs << ".data[0], " << lhs + << ".Length) then Exit(False);" << '\n'; + } else { + indent_impl(code) << "if " << lhs << ".Length <> " << rhs << ".Length then " << eq_var + << " := False" << '\n'; + indent_impl(code) << "else if " << lhs << ".Length > 0 then" << '\n'; + indent_impl(code) << " " << eq_var << " := SysUtils.CompareMem(@" << lhs << ".data[0], @" + << rhs << ".data[0], " << lhs << ".Length);" << '\n'; + } } else { // SysUtils.TBytes — dynamic array - indent_impl(code) << "if System.Length(" << lhs << ") <> System.Length(" << rhs << ") then " << eq_var << " := False" << '\n'; - indent_impl(code) << "else if System.Length(" << lhs << ") > 0 then" << '\n'; - indent_impl(code) << " " << eq_var << " := SysUtils.CompareMem(" - << "PByte(" << lhs << "), PByte(" << rhs << "), System.Length(" << lhs << "));" << '\n'; + if (exit_on_fail) { + indent_impl(code) << "if System.Length(" << lhs << ") <> System.Length(" << rhs + << ") then Exit(False);" << '\n'; + indent_impl(code) << "if (System.Length(" << lhs << ") > 0) and not SysUtils.CompareMem(" + << "PByte(" << lhs << "), PByte(" << rhs << "), System.Length(" << lhs + << ")) then Exit(False);" << '\n'; + } else { + indent_impl(code) << "if System.Length(" << lhs << ") <> System.Length(" << rhs << ") then " + << eq_var << " := False" << '\n'; + indent_impl(code) << "else if System.Length(" << lhs << ") > 0 then" << '\n'; + indent_impl(code) << " " << eq_var << " := SysUtils.CompareMem(" + << "PByte(" << lhs << "), PByte(" << rhs << "), System.Length(" << lhs + << "));" << '\n'; + } } } else if (ttype->is_list()) { @@ -4143,49 +4248,45 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, t_type* elem_type = tlist->get_elem_type()->get_true_type(); string i_var = tmp("_i"); vars << " " << i_var << " : System.Integer;" << '\n'; + bool elem_needs_helper + = type_can_be_null(elem_type) || elem_type->is_binary() || elem_type->is_container(); - indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then " << eq_var << " := False" << '\n'; - indent_impl(code) << "else if " << lhs << " <> nil then begin" << '\n'; - indent_up_impl(); - indent_impl(code) << "if " << lhs << ".Count <> " << rhs << ".Count then " << eq_var << " := False" << '\n'; - indent_impl(code) << "else begin" << '\n'; - indent_up_impl(); + container_prolog(); indent_impl(code) << "for " << i_var << " := 0 to " << lhs << ".Count - 1 do begin" << '\n'; indent_up_impl(); - bool elem_needs_helper = type_can_be_null(elem_type) || elem_type->is_binary() || elem_type->is_container(); if (!elem_needs_helper) { - if (elem_type->is_base_type() && ((t_base_type*)elem_type)->get_base() == t_base_type::TYPE_UUID) { - indent_impl(code) << "if not SysUtils.IsEqualGUID(" << lhs << "[" << i_var << "], " - << rhs << "[" << i_var << "]) then begin " << eq_var << " := False; break; end;" << '\n'; + if (elem_type->is_base_type() + && ((t_base_type*)elem_type)->get_base() == t_base_type::TYPE_UUID) { + indent_impl(code) << "if not SysUtils.IsEqualGUID(" << lhs << "[" << i_var << "], " << rhs + << "[" << i_var << "]) then " << fail_stmt << '\n'; } else { indent_impl(code) << "if " << lhs << "[" << i_var << "] <> " << rhs << "[" << i_var - << "] then begin " << eq_var << " := False; break; end;" << '\n'; + << "] then " << fail_stmt << '\n'; } } else { - string inner = generate_equal_container(code, vars, elem_type, - lhs + "[" + i_var + "]", rhs + "[" + i_var + "]"); - indent_impl(code) << "if not " << inner << " then begin " << eq_var << " := False; break; end;" << '\n'; + // recursion captures the elements into locals (already_local = false) + if (exit_on_fail) { + generate_equal_container(code, vars, elem_type, lhs + "[" + i_var + "]", + rhs + "[" + i_var + "]", true, false); + } else { + string inner = generate_equal_container(code, vars, elem_type, lhs + "[" + i_var + "]", + rhs + "[" + i_var + "]", false, false); + indent_impl(code) << "if not " << inner << " then " << fail_stmt << '\n'; + } } indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end for - indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end else (counts equal) - indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end else if lhs <> nil + indent_impl(code) << "end;" << '\n'; // end for + container_epilog(); } else if (ttype->is_set()) { t_set* tset = (t_set*)ttype; t_type* elem_type = tset->get_elem_type()->get_true_type(); - bool elem_needs_scan = type_can_be_null(elem_type) || elem_type->is_binary() || elem_type->is_container(); + bool elem_needs_scan + = type_can_be_null(elem_type) || elem_type->is_binary() || elem_type->is_container(); - indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then " << eq_var << " := False" << '\n'; - indent_impl(code) << "else if " << lhs << " <> nil then begin" << '\n'; - indent_up_impl(); - indent_impl(code) << "if " << lhs << ".Count <> " << rhs << ".Count then " << eq_var << " := False" << '\n'; - indent_impl(code) << "else begin" << '\n'; - indent_up_impl(); + container_prolog(); string e_var = tmp("_e"); vars << " " << e_var << " : " << type_name(elem_type) << ";" << '\n'; @@ -4193,8 +4294,8 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, if (!elem_needs_scan) { // Scalar elements: value-based hash equality in Contains is correct indent_impl(code) << "for " << e_var << " in " << lhs << " do" << '\n'; - indent_impl(code) << " if not " << rhs << ".Contains(" << e_var - << ") then begin " << eq_var << " := False; break; end;" << '\n'; + indent_impl(code) << " if not " << rhs << ".Contains(" << e_var << ") then " << fail_stmt + << '\n'; } else { // Struct/binary/container elements: O(n²) scan that marks matched rhs elements // as consumed, so a single rhs element cannot satisfy more than one lhs element. @@ -4230,8 +4331,9 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, indent_up_impl(); indent_impl(code) << "if not " << used_var << "[" << idx_var << "] then begin" << '\n'; indent_up_impl(); - string inner_eq - = generate_equal_container(code, vars, elem_type, e_var, arr_var + "[" + idx_var + "]"); + // the candidate probe must not abort the function -> always trial mode + string inner_eq = generate_equal_container(code, vars, elem_type, e_var, + arr_var + "[" + idx_var + "]", false, true); indent_impl(code) << "if " << inner_eq << " then begin" << '\n'; indent_up_impl(); indent_impl(code) << used_var << "[" << idx_var << "] := True;" << '\n'; @@ -4243,15 +4345,12 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, indent_impl(code) << "end;" << '\n'; // end if not yet used indent_down_impl(); indent_impl(code) << "end;" << '\n'; // end scan for - indent_impl(code) << "if not " << found_var << " then begin " << eq_var << " := False; break; end;" << '\n'; + indent_impl(code) << "if not " << found_var << " then " << fail_stmt << '\n'; indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end outer for + indent_impl(code) << "end;" << '\n'; // end outer for } - indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end else (counts equal) - indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end else if lhs <> nil + container_epilog(); } else if (ttype->is_map()) { t_map* tmap = (t_map*)ttype; @@ -4259,69 +4358,76 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, t_type* vtype = tmap->get_val_type()->get_true_type(); bool key_needs_scan = type_can_be_null(ktype) || ktype->is_binary() || ktype->is_container(); bool val_needs_helper = type_can_be_null(vtype) || vtype->is_binary() || vtype->is_container(); + bool val_is_uuid + = vtype->is_base_type() && ((t_base_type*)vtype)->get_base() == t_base_type::TYPE_UUID; - string k_var = tmp("_k"); - vars << " " << k_var << " : " << type_name(ktype) << ";" << '\n'; + string pair_type + = "Generics.Collections.TPair<" + type_name(ktype) + ", " + type_name(vtype) + ">"; + string pair_var = tmp("_pair"); + vars << " " << pair_var << " : " << pair_type << ";" << '\n'; - indent_impl(code) << "if (" << lhs << " = nil) <> (" << rhs << " = nil) then " << eq_var << " := False" << '\n'; - indent_impl(code) << "else if " << lhs << " <> nil then begin" << '\n'; - indent_up_impl(); - indent_impl(code) << "if " << lhs << ".Count <> " << rhs << ".Count then " << eq_var << " := False" << '\n'; - indent_impl(code) << "else begin" << '\n'; - indent_up_impl(); + container_prolog(); if (!key_needs_scan) { - // Scalar key: use ContainsKey then compare values - indent_impl(code) << "for " << k_var << " in " << lhs << ".Keys do begin" << '\n'; - indent_up_impl(); - indent_impl(code) << "if not " << rhs << ".ContainsKey(" << k_var << ") then begin " - << eq_var << " := False; break; end;" << '\n'; + // Scalar key: iterate the lhs pairs, fetch the rhs value with a single lookup + string v_var = tmp("_v"); + vars << " " << v_var << " : " << type_name(vtype) << ";" << '\n'; - string vl = lhs + "[" + k_var + "]"; - string vr = rhs + "[" + k_var + "]"; + indent_impl(code) << "for " << pair_var << " in " << lhs << " do begin" << '\n'; + indent_up_impl(); + indent_impl(code) << "if not " << rhs << ".TryGetValue(" << pair_var << ".Key, " << v_var + << ") then " << fail_stmt << '\n'; + string vl = pair_var + ".Value"; if (!val_needs_helper) { - if (vtype->is_base_type() && ((t_base_type*)vtype)->get_base() == t_base_type::TYPE_UUID) { - indent_impl(code) << "if not SysUtils.IsEqualGUID(" << vl << ", " << vr - << ") then begin " << eq_var << " := False; break; end;" << '\n'; + if (val_is_uuid) { + indent_impl(code) << "if not SysUtils.IsEqualGUID(" << vl << ", " << v_var << ") then " + << fail_stmt << '\n'; } else { - indent_impl(code) << "if " << vl << " <> " << vr << " then begin " - << eq_var << " := False; break; end;" << '\n'; + indent_impl(code) << "if " << vl << " <> " << v_var << " then " << fail_stmt << '\n'; } } else { - string val_inner = generate_equal_container(code, vars, vtype, vl, vr); - indent_impl(code) << "if not " << val_inner << " then begin " << eq_var << " := False; break; end;" << '\n'; + if (exit_on_fail) { + generate_equal_container(code, vars, vtype, vl, v_var, true, true); + } else { + string val_inner = generate_equal_container(code, vars, vtype, vl, v_var, false, true); + indent_impl(code) << "if not " << val_inner << " then " << fail_stmt << '\n'; + } } indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end for keys + indent_impl(code) << "end;" << '\n'; // end for pairs } else { - // Struct/binary/container key: O(n²) scan over key pairs that marks matched rhs - // keys as consumed, so a single rhs key cannot satisfy more than one lhs key - // (same rationale as the analogous set case above). + // Struct/binary/container key: O(n²) scan over a rhs key/value snapshot that marks + // matched rhs keys as consumed, so a single rhs key cannot satisfy more than one + // lhs key (same rationale as the analogous set case above). + string rpair_var = tmp("_pair"); string karr_var = tmp("_karr"); + string varr_var = tmp("_varr"); string used_var = tmp("_used"); string idx_var = tmp("_i"); string found_var = tmp("_found"); - string k2_var = tmp("_k"); + vars << " " << rpair_var << " : " << pair_type << ";" << '\n'; vars << " " << karr_var << " : array of " << type_name(ktype) << ";" << '\n'; + vars << " " << varr_var << " : array of " << type_name(vtype) << ";" << '\n'; vars << " " << used_var << " : array of System.Boolean;" << '\n'; vars << " " << idx_var << " : System.Integer;" << '\n'; vars << " " << found_var << " : System.Boolean;" << '\n'; - vars << " " << k2_var << " : " << type_name(ktype) << ";" << '\n'; indent_impl(code) << "System.SetLength(" << karr_var << ", " << rhs << ".Count);" << '\n'; + indent_impl(code) << "System.SetLength(" << varr_var << ", " << rhs << ".Count);" << '\n'; indent_impl(code) << "System.SetLength(" << used_var << ", " << rhs << ".Count);" << '\n'; indent_impl(code) << idx_var << " := 0;" << '\n'; - indent_impl(code) << "for " << k2_var << " in " << rhs << ".Keys do begin" << '\n'; + indent_impl(code) << "for " << rpair_var << " in " << rhs << " do begin" << '\n'; indent_up_impl(); - indent_impl(code) << karr_var << "[" << idx_var << "] := " << k2_var << ";" << '\n'; + indent_impl(code) << karr_var << "[" << idx_var << "] := " << rpair_var << ".Key;" << '\n'; + indent_impl(code) << varr_var << "[" << idx_var << "] := " << rpair_var << ".Value;" << '\n'; indent_impl(code) << "System.Inc(" << idx_var << ");" << '\n'; indent_down_impl(); indent_impl(code) << "end;" << '\n'; - indent_impl(code) << "for " << k_var << " in " << lhs << ".Keys do begin" << '\n'; + indent_impl(code) << "for " << pair_var << " in " << lhs << " do begin" << '\n'; indent_up_impl(); indent_impl(code) << found_var << " := False;" << '\n'; indent_impl(code) << "for " << idx_var << " := 0 to System.Length(" << karr_var @@ -4329,16 +4435,17 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, indent_up_impl(); indent_impl(code) << "if not " << used_var << "[" << idx_var << "] then begin" << '\n'; indent_up_impl(); - string key_inner - = generate_equal_container(code, vars, ktype, k_var, karr_var + "[" + idx_var + "]"); + // the candidate probe must not abort the function -> always trial mode + string key_inner = generate_equal_container(code, vars, ktype, pair_var + ".Key", + karr_var + "[" + idx_var + "]", false, true); indent_impl(code) << "if " << key_inner << " then begin" << '\n'; indent_up_impl(); // Keys match: compare values, and only consume the rhs key/mark found if the // value matches too (a key-only match with a differing value must keep scanning). - string vl = lhs + "[" + k_var + "]"; - string vr = rhs + "[" + karr_var + "[" + idx_var + "]]"; + string vl = pair_var + ".Value"; + string vr = varr_var + "[" + idx_var + "]"; if (!val_needs_helper) { - if (vtype->is_base_type() && ((t_base_type*)vtype)->get_base() == t_base_type::TYPE_UUID) { + if (val_is_uuid) { indent_impl(code) << "if SysUtils.IsEqualGUID(" << vl << ", " << vr << ") then begin" << '\n'; } else { @@ -4351,7 +4458,7 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, indent_down_impl(); indent_impl(code) << "end;" << '\n'; // end if value matches } else { - string val_inner = generate_equal_container(code, vars, vtype, vl, vr); + string val_inner = generate_equal_container(code, vars, vtype, vl, vr, false, true); indent_impl(code) << "if " << val_inner << " then begin" << '\n'; indent_up_impl(); indent_impl(code) << used_var << "[" << idx_var << "] := True;" << '\n'; @@ -4361,20 +4468,17 @@ std::string t_delphi_generator::generate_equal_container(ostream& code, indent_impl(code) << "end;" << '\n'; // end if val_inner } indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end if key_inner + indent_impl(code) << "end;" << '\n'; // end if key_inner indent_down_impl(); indent_impl(code) << "end;" << '\n'; // end if not yet used indent_down_impl(); indent_impl(code) << "end;" << '\n'; // end scan for - indent_impl(code) << "if not " << found_var << " then begin " << eq_var << " := False; break; end;" << '\n'; + indent_impl(code) << "if not " << found_var << " then " << fail_stmt << '\n'; indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end for k_var + indent_impl(code) << "end;" << '\n'; // end for pair_var } - indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end else (counts equal) - indent_down_impl(); - indent_impl(code) << "end;" << '\n'; // end else if lhs <> nil + container_epilog(); } return eq_var; @@ -4426,8 +4530,7 @@ void t_delphi_generator::generate_delphi_struct_equality_impl(ostream& out, << " then Exit(False);" << '\n'; } } else { - string eq_var = generate_equal_container(code_block, local_vars, ftype, self_prop, other_prop); - indent_impl(code_block) << "if not " << eq_var << " then Exit(False);" << '\n'; + generate_equal_container(code_block, local_vars, ftype, self_prop, other_prop, true, false); } if (is_optional) {