THRIFT-6333: fix(compiler): Resolve every typedef before any generator runs

[slachiewicz]

The compiler resolved a forward typedef the first time a generator asked for it. An undeclared target printed Type "X" not defined to stdout and exited, leaving whatever the earlier generators had already written (thrift --gen go --gen java left GoUnusedProtection__.go). A generator that never asked reported nothing. A typedef that reached itself (typedef T T, typedef list<L> L, or typedef B A with typedef A B) looped forever in whichever generator touched it. https://issues.apache.org/jira/browse/THRIFT-6333

parse() now calls t_program::resolve_types(), which walks every type the program refers to (typedefs, constants, struct fields, service signatures, through containers) one step at a time. Before any generator runs, it reports [FAILURE:file:line] Type "X" not defined or Type "X" refers to itself on stderr. The line is where the parser stands at the end of the file, because the parse tree keeps no line per declaration. t_typedef::get_type() throws instead of exiting, and generate() already catches that exception. Structs are not followed, so a recursive struct still compiles.

Behaviour changes, all on inputs that are already broken:

• --audit rejects an undefined type it used to ignore; test/audit/thrift_audit_test.pl passes.
• A generator that never resolved every type now fails on an unresolved one. thrift --gen html -r on lib/go/test/NamespacedTest.thrift, and on LightRail.thrift and Streetcars.thrift in lib/rs/test_recursive/src/transit/light/, whose includes are not found (only a warning), exited 0 and now exit 1 with Type "…" not defined.

Verified: every .thrift in the repository with --gen cpp -r and --gen html -r, master against this branch → 480 of 484 runs give the same exit status and output. The three html runs above now fail, and --gen cpp on NamespacedTest.thrift exits 1 as before but writes no files (master left 6).

Tests: compiler/cpp/tests/cpp/t_cpp_parser_typedef_tests.cc covers an undeclared target, a typedef declared after its use, the three self-referencing forms and a struct that contains itself through a typedef.

• Did you create an Apache Jira ticket? THRIFT-6333
• If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
• Did you squash your changes to a single commit?
• Did you do your best to avoid breaking changes? No Breaking-Change label: only inputs with an undefined or self-referencing type behave differently, as listed above.
• Code change, so the commit carries no CI-skip marker.
• Tests added (t_cpp_parser_typedef_tests.cc); codespell reports nothing on the changed lines.
• AI-assisted: the commit carries a Co-Authored-By: trailer. No new dependency or third-party code.

[Jens-G]

Code review

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

Two suggestions, below the bar for an issue but verified:

• The new pass in parse() does not finish when a typedef refers back to itself, and the compiler then hangs at 100% CPU without a message:

  • typedef list<L> L loops in the container branch of resolve_forward_types().
  • typedef T T, or typedef B A together with typedef A B, loops in get_true_type(), which the pass now calls on every declared typedef, even one that nothing uses.

  The base compiler exits 0 on each of these three files with --gen cpp (also java, py, html and netstd). With this PR it hangs for every generator and for --audit. When a struct field uses such a typedef, the base compiler already crashed or hung, except with --gen html.

  A fix that I tried: follow typedefs and container elements one step at a time, and keep a set of the types on the current path. Each of the three files then fails at once with Type "<name>" refers to itself, and on all 240 .thrift files in the repository, --gen cpp and --gen html give the same exit status and the same output as this PR. A set in the container branch alone is not enough, because typedef T T never leaves get_true_type().

thrift/compiler/cpp/src/thrift/main.cc

Lines 1050 to 1054 in b5a7d62

	// written a file.
	resolve_forward_types(program);
	} catch (string &x) {
	failure(x.c_str());
	}

thrift/compiler/cpp/src/thrift/main.cc

Lines 937 to 959 in b5a7d62

	*/
	static void resolve_forward_types(t_type* type) {
	type = type->get_true_type();
	if (type->is_list()) {
	resolve_forward_types(((t_list*)type)->get_elem_type());
	} else if (type->is_set()) {
	resolve_forward_types(((t_set*)type)->get_elem_type());
	} else if (type->is_map()) {
	resolve_forward_types(((t_map*)type)->get_key_type());
	resolve_forward_types(((t_map*)type)->get_val_type());
	}
	}
	static void resolve_forward_types(t_struct* tstruct) {
	for (t_field* field : tstruct->get_members()) {
	resolve_forward_types(field->get_type());
	}
	}
	static void resolve_forward_types(t_program* program) {
	for (t_typedef* td : program->get_typedefs()) {
	resolve_forward_types(td);
	}

thrift/compiler/cpp/src/thrift/parse/parse.cc

Lines 29 to 35 in b5a7d62

	const t_type* t_type::get_true_type() const {
	const t_type* type = this;
	while (type->is_typedef()) {
	type = ((t_typedef*)type)->get_type();
	}
	return type;
	}

• The new rejections are right, and the description already names the --audit case. Generators that never resolved every type change the same way: thrift --gen html -r lib/go/test/NamespacedTest.thrift, where the include ThriftTest.thrift is not found (only a warning), exits 0 on base and writes 4 files; with this PR it exits 1 with Type "ThriftTest.UserId" not defined. LightRail.thrift and Streetcars.thrift in lib/rs/test_recursive/src/transit/light/ behave the same. The sweep in the description (--gen go --gen java --gen json) could not show this, because those generators already failed on base, and no build target compiles these files this way. It could go next to the --audit note.

🤖 Generated with Claude Code

[slachiewicz]

Thanks. Both are in 35eee1f7a: t_program::resolve_types() follows typedefs and container elements one step at a time with a path set, so all three files fail at once with Type "…" refers to itself, and the html cases are listed in the description.

[Jens-G]

Code review

Re-reviewed at 35eee1f7a. No blocking issues found. Checked for bugs and CLAUDE.md compliance.

Both points from the last round are addressed. With compilers built from this head and from master, typedef T T, typedef list<L> L and typedef B T with typedef T B now fail at once with Type "..." refers to itself, also when a struct field uses them. With such a field, master hangs (typedef T T, --gen cpp) or crashes (typedef list<L> L, --gen py, java, netstd). On all 242 .thrift files in the repository, --gen cpp and --gen html differ only in the cases the description lists. A cycle used in a const value, a field default or a throws clause still hangs while parsing, on master too; that is THRIFT-6337.

One suggestion, below the bar for an issue but verified:

• Two new comments say that a typedef which reaches itself through a container "would send get_true_type() round forever". That is right for typedef T T, but for typedef list<L> L get_true_type() stops at the list after one step. On master that input is harmless while nothing uses it; once a struct field uses it, py, java and netstd crash with SIGSEGV and cpp dumps core, from the generators' recursion into the element type. Naming that for the container case would make the comments match what happens.

thrift/compiler/cpp/src/thrift/parse/parse.cc

Lines 43 to 49 in 35eee1f

	/**
	* Follows typedefs and container elements one step at a time. path holds the
	* types on the way here, so that a typedef reached again through itself,
	* which would send get_true_type() round forever, is reported instead.
	* done holds the types already walked in full.
	*/
	void resolve_type(const t_type* type,

thrift/compiler/cpp/tests/cpp/t_cpp_parser_typedef_tests.cc

Lines 81 to 89 in 35eee1f

	// A typedef that reaches itself, directly or through a container, would send
	// get_true_type() round forever; resolve_types() reports it instead.
	TEST_CASE("typedef that refers to itself throws when resolved", "[parser]")
	{
	const char* sources[] = {
	"typedef T T\n",
	"typedef list<T> T\n",
	"typedef B T\ntypedef T B\n",

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

XSD attribute types bypass preflight resolution, and the generator-ordering regression lacks integration coverage.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Recursively resolve types in anonymous XSD attribute structs · New
• Add integration test ensuring invalid typedefs generate no files · New

What changed in this PR

Pre-validates typedef resolution before code generation to prevent partial output and cyclic-resolution hangs.

Changes:

• Adds whole-program type resolution.
• Replaces process exit with throwable errors.
• Adds typedef-resolution tests.

File	Description
compiler/​cpp/​tests/​cpp/​t_cpp_parser_typedef_tests.cc	Tests valid, undefined, cyclic, and recursive typedefs.
compiler/​cpp/​src/​thrift/​parse/​t_typedef.cc	Throws on undefined typedef targets.
compiler/​cpp/​src/​thrift/​parse/​t_program.h	Declares program-wide resolution.
compiler/​cpp/​src/​thrift/​parse/​parse.cc	Traverses referenced types and detects cycles.
compiler/​cpp/​src/​thrift/​main.cc	Runs resolution before generators.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.