Skip to content

THRIFT-6445: Add ability to mark enums as flags - #4040

Open
SLAVONchick wants to merge 1 commit into
apache:masterfrom
SLAVONchick:feature/6445-enum-flags
Open

SLAVONchick wants to merge 1 commit into
apache:masterfrom
SLAVONchick:feature/6445-enum-flags

Conversation

@SLAVONchick

Copy link
Copy Markdown
Contributor

Client: py,netstd

Generate Flags attributes for C# and Flag or IntFlag classes for Python. Preserve unknown flag bits across schema versions and avoid collisions between generated enum names and imported flag base classes.

Handle flag serialization in containers and immutable structures, using generated codecs where the dynamic and accelerated codecs lose flag types. Add compiler regression tests and document the annotation.

Validation: compiler build and all 20 compiler CTest tests passed; git diff --check passed. make style is unavailable in this CMake setup.

  • Did you create an Apache Jira ticket? (Request account here, not required for trivial changes) - Yes, here
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"? - Yes
  • Did you squash your changes to a single commit? (not required, but preferred) - Yes
  • Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"? - Yes, no breaking changes introduced.

Compatibility note: Existing schemas without enum.flags retain their current behavior. Adding the annotation changes the generated Python API from integer values to enum.Flag (enum.IntFlag with py:enum,type_hints) and requires Python 3.6 or later. Python callers adopting the annotation should assign flag members or explicitly construct flags from integers, use flag members in bitwise checks, and access .value when an integer is required. With the default generator, flag members no longer compare equal to integers, and assigning raw integers can fail during serialization. The wire representation remains i32, and unknown flag bits are preserved during serialization.

  • If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources. - N/A

Client: py,netstd

Generate Flags attributes for C# and Flag or IntFlag classes for Python.
Preserve unknown flag bits across schema versions and avoid collisions
between generated enum names and imported flag base classes.

Handle flag serialization in containers and immutable structures, using
generated codecs where the dynamic and accelerated codecs lose flag types.
Add compiler regression tests and document the annotation.

Validation: compiler build and all 20 compiler CTest tests passed;
git diff --check passed. make style is unavailable in this CMake setup.

Co-Authored-By: OpenAI Codex (GPT-6) <noreply@openai.com>
@mergeable mergeable Bot added c# Pull requests that update C# code Pull requests that update .NET code python compiler build and general CI cmake, automake and build system changes labels Oct 22, 2026
@SLAVONchick

Copy link
Copy Markdown
Contributor Author

The AppVeyor failure appears unrelated to this change: EnumFlagsTest passed. The failing C++ integration test appears to miss connections rejected during send on Windows, because it only records sockets after a successful send. The affected code predates this PR (ecaa4054f5).

@Jens-G

Jens-G commented Oct 22, 2026

Copy link
Copy Markdown
Member

Code review

No blocking issues found. Checked for bugs and CLAUDE.md compliance.

Three suggestions and a note, below the bar for an issue but verified with code generated by this PR's compiler (Python 3.10, 3.12 and 3.14):

  • Every member that the generated _missing_ creates for an undeclared value has _name_ = None, and Enum.__hash__ hashes the name, so with the enum.Flag base (--gen py and --gen py:enum) all of them share one hash value. Reading a set<...> or map keys holding N distinct undeclared values is therefore quadratic in N: 20,000 values (80 KB on the wire) take about 5 s to read on Python 3.12 and 4.6 s on 3.10. With def __hash__(self): return hash(self._value_) in the generated class it is 0.15 s on 3.12. The IntFlag base hashes as an int and is not affected.

if (is_flags(tenum)) {
// Preserve unknown bits on older Python versions as well as signed I32 values.
indent(f_types_) << "@classmethod" << '\n';
indent(f_types_) << "def _missing_(cls, value):" << '\n';
indent_up();
indent(f_types_) << "if not isinstance(value, int):" << '\n';
indent(f_types_) << indent_str() << "return None" << '\n';
indent(f_types_) << "if value >= 0:" << '\n';
indent_up();
indent(f_types_) << "try:" << '\n';
indent(f_types_) << indent_str() << "return super()._missing_(value)" << '\n';
indent(f_types_) << "except ValueError:" << '\n';
indent(f_types_) << indent_str() << "pass" << '\n';
indent_down();
indent(f_types_) << "member = "
<< (gen_type_hints_ ? "int.__new__(cls, value)" : "object.__new__(cls)") << '\n';
indent(f_types_) << "member._name_ = None" << '\n';
indent(f_types_) << "member._value_ = value" << '\n';
indent(f_types_) << "return cls._value2member_map_.setdefault(value, member)" << '\n';
indent_down();
f_types_ << '\n';

  • With the enum.Flag base, clearing a flag the usual way, p & ~Permissions.Read, also drops every undeclared bit, because Flag.__invert__ only complements the declared bits. A value 17 received from a newer peer (Read plus an undeclared 16) is forwarded as 0. p ^ Read and p | Write keep the bit, and so does the IntFlag base (py:enum,type_hints). The new idl.md paragraph says peers can "exchange new flags without losing them", which holds for reading and writing only. Either that sentence could say so, or the generated class could define def __invert__(self): return self.__class__(~self._value_). With that, p & ~Read gives 16 and ~Read & ReadWrite still gives Write, though repr(~Read) then shows a negative value.

thrift/doc/specs/idl.md

Lines 76 to 80 in 7b0033d

assign values or check that they are powers of two; zero and composite values are
allowed. The wire representation remains an `i32`. Python preserves unknown flag
bits when reading and writing values, allowing peers with different versions of
an enum to exchange new flags without losing them.

  • Under --gen py:enum, a raw int assigned to a flag field is converted in mutable structs by the generated __setattr__, but stored unchanged in python.immutable structs and in exceptions. Writing such a struct, or an exception raised with p=5, then fails with AttributeError: 'int' object has no attribute 'value'. The immutable branch could use the expression __setattr__ already uses, x if hasattr(x, 'value') or x is None else T(x). The plain-enum half of that branch, where an int becomes None, is THRIFT-6369.

if (is_immutable(tstruct)) {
if (gen_enum_ && type->is_enum() && !is_flags(type)) {
indent(out) << "super(" << maybe_escape_identifier(tstruct->get_name()) << ", self).__setattr__('"
<< (*m_iter)->get_name() << "', " << maybe_escape_identifier((*m_iter)->get_name())
<< " if hasattr(" << maybe_escape_identifier((*m_iter)->get_name()) << ", 'value') else "
<< type_name(type) << ".__members__.get(" << maybe_escape_identifier((*m_iter)->get_name()) << "))" << '\n';
} else if (gen_newstyle_ || gen_dynamic_) {

  • A note: _missing_ also keeps every distinct undeclared value in the class's _value2member_map_ for the life of the process. 100,000 distinct values left about 18 MB (3.12) to 36 MB (3.10) allocated with the Flag base. The standard IntFlag does the same for non-negative values, while the stock Flag rejects undeclared values, so its cache stays bounded. Dropping that cache would need value-based __eq__ and __hash__, since enum equality is identity.

🤖 Generated with Claude Code

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build and general CI cmake, automake and build system changes c# Pull requests that update C# code Pull requests that update .NET code compiler python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants