From 4d2bea739a829e7ed2b2d7caa11e7577a1fa2d12 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Thu, 16 Jun 2022 23:29:20 +0800 Subject: [PATCH 1/2] kafka/protocol/schemata: drop unused `import functools` functools was imported by 89a8b55dce51bc554d28850c8fea2529a00da5a6, but that commit does not reference this python module, so let's just drop it Signed-off-by: Kefu Chai --- src/v/kafka/protocol/schemata/generator.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/v/kafka/protocol/schemata/generator.py b/src/v/kafka/protocol/schemata/generator.py index e4f67509e319..a284207c1e80 100644 --- a/src/v/kafka/protocol/schemata/generator.py +++ b/src/v/kafka/protocol/schemata/generator.py @@ -33,7 +33,6 @@ # make it easier to audit where sensitive information is used. import io import json -import functools import pathlib import re import sys From 82fc206816c25144a840a2dc6656d8bf531f2748 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Thu, 16 Jun 2022 08:11:22 +0800 Subject: [PATCH 2/2] kafka/protocol/schemata: define default operator==() if possible * generator.py: add default-generated operator==() to types which can have default-generated operator==(). for those who contains non-generated types which cannot have default-generated opereator==(), they are listed in WITHOUT_DEFAULT_EQUALITY_OPERATOR. so far, only `produce_request_record_data` and `batch_reader` are in the list. because: - they are not generated using the generator.py script, so the generator.py cannot deduce if it is eligible to have the default-generated operator==(). - they have iobuf as its indirect member variable via `kafka_batch_adapter`. and iobuf is not default-comparable. - we have dedicated rule in generator.py to synthesis a type definition which contains either of these types. this helps to eliminate the handcrafted 'operator==()' implementations, - better maintanability - easier to compiler to find the used operator==() if it needs them. * use default generated operator==() when appropriate. simpler this way. also, this helps the compiler to see the comparison operators before they are used. this change helps to address the FTBFS with clang++-15 + libstd++-12: /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algobase.h:1161:22: error: invalid operands to binary expression ('const kafka::api_versions_response_key' and 'const kafka::api_versions_response_key') if (!(*__first1 == *__first2)) ~~~~~~~~~ ^ ~~~~~~~~~ /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algobase.h:1210:38: note: in instantiation of function template specialization 'std::__equal::equal' requested here return std::__equal<__simple>::equal(__first1, __last1, __first2); ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algobase.h:1218:19: note: in instantiation of function template specialization 'std::__equal_aux1' requested here return std::__equal_aux1(std::__niter_base(__first1), ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algobase.h:1555:19: note: in instantiation of function template specialization 'std::__equal_aux<__gnu_cxx::__normal_iterator>, __gnu_cxx::__normal_iterator>>' requested here return std::__equal_aux(__first1, __last1, __first2); ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_vector.h:2037:16: note: in instantiation of function template specialization 'std::equal<__gnu_cxx::__normal_iterator>, __gnu_cxx::__normal_iterator>>' requested here && std::equal(__x.begin(), __x.end(), __y.begin())); } ^ /home/kefu/dev/redpanda/src/v/kafka/protocol/api_versions.h:67:35: note: in instantiation of function template specialization 'std::operator==>' requested here && a.data.api_keys == b.data.api_keys ^ /home/kefu/dev/redpanda/src/v/kafka/protocol/join_group.h:137:13: note: candidate function not viable: no known conversion from 'const kafka::api_versions_response_key' to 'const std::vector' for 1st argument inline bool operator==( ^ Signed-off-by: Kefu Chai --- src/v/kafka/protocol/api_versions.h | 32 ++-------------------- src/v/kafka/protocol/schemata/generator.py | 19 +++++++++++++ 2 files changed, 21 insertions(+), 30 deletions(-) diff --git a/src/v/kafka/protocol/api_versions.h b/src/v/kafka/protocol/api_versions.h index c67516c12de0..f6d53a5e67ae 100644 --- a/src/v/kafka/protocol/api_versions.h +++ b/src/v/kafka/protocol/api_versions.h @@ -58,36 +58,8 @@ struct api_versions_response final { private: friend bool operator==( - const kafka::api_versions_response& a, - const kafka::api_versions_response& b) { - return a.data.error_code == b.data.error_code - && a.data.throttle_time_ms == b.data.throttle_time_ms - && a.data.finalized_features_epoch - == b.data.finalized_features_epoch - && a.data.api_keys == b.data.api_keys - && a.data.supported_features == b.data.supported_features - && a.data.finalized_features == b.data.finalized_features; - } + const kafka::api_versions_response&, const kafka::api_versions_response&) + = default; }; -inline bool operator==( - const api_versions_response_key& a, const api_versions_response_key& b) { - return a.api_key == b.api_key && a.min_version == b.min_version - && a.max_version == b.max_version; -} - -inline bool operator==( - const kafka::supported_feature_key& a, - const kafka::supported_feature_key& b) { - return a.name == b.name && a.min_version == b.min_version - && a.max_version == b.max_version; -} - -inline bool operator==( - const kafka::finalized_feature_key& a, - const kafka::finalized_feature_key& b) { - return a.name == b.name && a.max_version_level == b.max_version_level - && a.min_version_level == b.min_version_level; -} - } // namespace kafka diff --git a/src/v/kafka/protocol/schemata/generator.py b/src/v/kafka/protocol/schemata/generator.py index a284207c1e80..bd17bc9aa899 100644 --- a/src/v/kafka/protocol/schemata/generator.py +++ b/src/v/kafka/protocol/schemata/generator.py @@ -436,6 +436,13 @@ def make_context_field(path): "FinalizedFeatureKey", ] +# a list of struct types which are ineligible to have default-generated +# `operator==()`, because one or more of its member variables are not +# comparable +WITHOUT_DEFAULT_EQUALITY_OPERATOR = { + 'kafka::batch_reader', 'kafka::produce_request_record_data' +} + # The following is a list of tag types which contain fields where their # respective types are not prefixed with []. The generator special cases these # as ArrayTypes @@ -633,6 +640,10 @@ def structs(self): res.append(t) return res + @property + def is_default_comparable(self): + return all(field.is_default_comparable for field in self.fields) + class ArrayType(FieldType): def __init__(self, value_type): @@ -844,6 +855,11 @@ def value_type(self): def name(self): return snake_case(self._field["name"]) + @property + def is_default_comparable(self): + type_name, _ = self._redpanda_type() + return type_name not in WITHOUT_DEFAULT_EQUALITY_OPERATOR + HEADER_TEMPLATE = """ #pragma once @@ -893,6 +909,9 @@ def name(self): // added by redpanda. see generator.py:make_context_field. {{ struct.context_field[0] }} {{ struct.context_field[1] -}}; {%- endif %} +{%- if struct.is_default_comparable %} + friend bool operator==(const {{ struct.name }}&, const {{ struct.name }}&) = default; +{%- endif %} {% endmacro %} namespace kafka {