You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@thrift.apache.org by "slachiewicz (via GitHub)" <gi...@apache.org> on 2026/09/08 08:42:20 UTC
[PR] THRIFT-6200: Use the declaring package for enum arguments in the Go -remote stub [thrift]
slachiewicz opened a new pull request, #3817:
URL: https://github.com/apache/thrift/pull/3817
A service method that takes an enum declared in an included file generates a `-remote` stub that does not compile. The stub qualifies the enum with the service's own package:
```go
argvalue1 := includestest.Numberz(tmp1)
```
`Numberz` is declared in `ThriftTest.thrift` and generated into `thrifttest`, which the stub already imports.
```
gopath/src/includestest/test_service-remote/test_service-remote.go:179:29: undefined: includestest.Numberz
gopath/src/includestest/extended_service-remote/extended_service-remote.go:197:29: undefined: includestest.Numberz
```
The enum branch of `generate_service_remote` wrote `package_name_aliased` unconditionally. The struct branch a few lines below already resolves the package with `module_name()` and falls back to `package_name_aliased` only when that is empty; the enum branch now does the same, so a typedef of an enum in the local package still resolves locally.
`lib/go/test` builds `includestest` but not its two `-remote` packages, which is why this never showed up. Both are added to the `check` list.
`IncludesTest.thrift` already has the shape that reproduces it, so no new IDL is needed:
```thrift
service testService extends ThriftTest.SecondService {
ThriftTest.CrazyNesting getCrazyNesting(
1: ThriftTest.StructA a,
2: ThriftTest.Numberz numbers
) throws(1: ThriftTest.Xception err1),
}
```
Regenerating every IDL under `test/`, `lib/go/test/` and `tutorial/` with and without the change touches 2 of 325 generated files — the two stubs above, one line each.
Independent of #3812; different code path, no shared lines.
*This change 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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6200: Use the declaring package for enum arguments in the Go -remote stub [thrift]
Posted by "Jens-G (via GitHub)" <gi...@apache.org>.
Jens-G commented on PR #3817:
URL: https://github.com/apache/thrift/pull/3817#issuecomment-5635255115
### Code review
No issues found. Checked for bugs and CLAUDE.md compliance.
One related case that fits THRIFT-6200 but is not covered by this change: the container/exception branch of `generate_service_remote` still qualifies the args constructor with `package_name_aliased`. Since THRIFT-5002, `func_to_service` records which service declares an inherited function, but only its name, not its package. If that service comes from an included file, the stub does not compile:
```thrift
// ParentA.thrift
namespace go parenta
service ParentService {
void takeList(1: list<i32> xs),
}
// ChildB.thrift
include "ParentA.thrift"
namespace go childb
service ChildService extends ParentA.ParentService {
}
```
```
child_service-remote.go:163:30: undefined: childb.NewParentServiceTakeListArgs
```
That is with the compiler built from this branch. `map`, `list<struct>` and exception arguments fail the same way, while enum and struct arguments inherited this way do compile. The services in `IncludesTest.thrift` do not reach this branch, because the functions they inherit from other files take no container arguments. Qualifying `containerStruct` with the package of the declaring service would fix it.
https://github.com/apache/thrift/blob/ca1d1487b6dc281cd47f7f45c05c84acf234ccdf/compiler/cpp/src/thrift/generate/t_go_generator.cc#L3027-L3030
🤖 Generated with [Claude Code](https://claude.ai/code)
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6200: Use the declaring package for arguments in the Go -remote stub [thrift]
Posted by "slachiewicz (via GitHub)" <gi...@apache.org>.
slachiewicz commented on PR #3817:
URL: https://github.com/apache/thrift/pull/3817#issuecomment-5656174274
Folded into the amended commit. `func_to_service` now keeps the `t_service*` and the container/exception branch qualifies the args constructor with `module_name()` of that service, the same way the struct and enum branches do. The stub also imports the package of every ancestor service, since a function inherited from a service two include levels away resolved to the local package for the same reason.
`NamespacedService` in `lib/go/test` gains `list`, `map` and exception arguments and extends a service in a new `NamespacedBaseTest.thrift`, so `Extended2Service` inherits across one and two include levels. `extended2_service-remote` joins the check list. Your ParentA/ChildB example builds with this branch.
*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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6200: Use the declaring package for arguments in the Go -remote stub [thrift]
Posted by "Jens-G (via GitHub)" <gi...@apache.org>.
Jens-G merged PR #3817:
URL: https://github.com/apache/thrift/pull/3817
--
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: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org