slachiewicz commented on PR #3812: URL: https://github.com/apache/thrift/pull/3812#issuecomment-5597400263
Four IDL corpora from outside this repository, generated with the parent commit and with this branch, to see what the change moves in the wild. One of them reproduces the defect on its own IDL, and the other three are byte-identical. | Corpus | Files that parse | Output difference | Build | |---|---|---|---| | [uber/thriftrw-go](https://github.com/thriftrw/thriftrw-go) test IDL | 18 of 23 | 3 files | fails on the parent commit, builds on this branch | | [jaeger-idl](https://github.com/jaegertracing/jaeger-idl) | 4 of 4 | none | builds on both | | [colbygk/evernote-sdk-golang](https://github.com/colbygk/evernote-sdk-golang) | 5 of 5 | none | builds on both | | [facebook/fbthrift](https://github.com/facebook/fbthrift) compiler fixtures | 8 of 217 | none | builds on both | ## uber/thriftrw-go reproduces it independently The test IDL under `gen/internal/tests/thrift` was written for a different Thrift implementation by a different team, and it contains the shapes these tickets describe. Three generated files differ, and the difference is a package that doesn't build: | Generated file | The IDL behind it | Parent commit | This branch | |---|---|---|---| | `structs.go` | `typedef Node List` | `type List *Node`, then `cannot use &Node{} (value of type *Node) as *List value in assignment`, `p.Tail.Read/Write/Equals undefined` | compiles | | `typedefs.go` | `typedef i128 UUID`, where `struct i128` is declared after the typedef, plus `typedef UUID MyUUID` | the same failures, on a required field and through the alias of an alias | compiles | | `nozap.go` | `typedef PrimitiveRequiredStruct Primitives`, never used in a field | compiles | compiles, and only the declaration line changes | That is THRIFT-4901's error text arriving from a third party's IDL, and `nozap.go` is the unused-alias shape from the compatibility section of the description, in the wild. Two caveats on method. The `typedefs` and `structs` packages sit behind an unrelated pre-existing defect, a duplicate case in the `String()` method of `EnumWithDuplicateValues`, which masks them; I removed that case identically in both trees before rebuilding. The `exceptions`, `enum-text-marshal-strict`, and `hyphenated-file` packages fail on both compilers for unrelated reasons: an `Error` field that collides with the `Error()` method, and hyphens in package names. Five of the 23 inputs fail to generate on both, on thriftrw dialect the Apache grammar doesn't accept. ## facebook/fbthrift fixtures confirm parity, not much more Of 217 fixture sources, 8 parse under the Apache grammar and 209 don't, because they use the modern dialect: `package` statements and `@`-prefixed structured annotations. The generate status is identical on both compilers for all 217, and all 209 failure diagnostics are byte-identical. The 8 that parse produce identical output and build with both. The failures are clean and correctly positioned, which is worth asserting even though this change doesn't touch that path. For the `line-numbers` fixture, both compilers report `module.thrift:19`, which is exactly the `@thrift.AllowLegacyMissingUris` line, with `Unexpected token in input: "@"` and a warning naming the include that could not be found. The `java-typedef` fixture turns out to cover base types only, `i16`, `string`, and a `map` of the two, so it never reaches this change. ## Jaeger and the Evernote SDK don't move Both generate byte-identical Go, and every generated package builds, including the `-remote` stubs each project deletes. Neither exercises the change: jaeger-idl has no typedefs at HEAD, and the only two in its history, `typedef string BaggageKey` and `typedef i32 MaxValueLength`, are base types. The Evernote IDL has seven typedefs across five files, all base types, and none is used before its declaration. ## What this adds The in-repo measurements show that the corpus doesn't move. These show the same thing on four projects that don't share this repository's test IDL, and one of them turns the defect into something other than a synthetic reproduction: an independent project's checked-in IDL that produces a Go package which doesn't compile. *This comment was created with AI assistance.* -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
