Repository navigation
Go: Conflicts between identifiers #1703
Description
Activity
You seem to be the maintainer of the Go bindgen?
Thanks for tagging me @QuantumSegfault! Here's what I found:
Scenario 1: Enum variant named
tagConfirmed bug. Generated getter method name collides with built-in
Tagmethod. Here's what I did to reproduce:This is the WIT I used:
package foo:bar; world example { import test; } interface test { variant my-variant { tag(string), } }
These are the resulting bindings:
package foo_bar_test const ( MyVariantTag uint8 = 0 ) type MyVariant struct { tag uint8 value any } func (self MyVariant) Tag() uint8 { return self.tag } // COMPILE ERROR: method MyVariant.Tag already declared func (self MyVariant) Tag() string { if self.tag != MyVariantTag { panic("tag mismatch") } return self.value.(string) } func MakeMyVariantTag(value string) MyVariant { return MyVariant{MyVariantTag, value} }
Scenario 2: Conflicting records and enums
Confirmed bug. Generated struct conflicts with generated enum variant. See reproduction below for more details. I think this will be a pretty straightforward fix, just a matter of deciding the best way to prevent name collisions between types. I'm going to allocate some cycles this week to think about this and report back with some possible solutions.
Here's the WIT I used:
package foo:bar; world example { import test; } interface test { record my-variant-item {} variant my-variant { item(my-variant-item), } }
Resulting bindings:
package foo_bar_test // COMPILE ERROR: MyVariantItem redeclared in this block type MyVariantItem struct { } const ( // COMPILE ERROR: MyVariantItem redeclared in this block MyVariantItem uint8 = 0 ) type MyVariant struct { tag uint8 value any } func (self MyVariant) Tag() uint8 { return self.tag } func (self MyVariant) Item() MyVariantItem { if self.tag != MyVariantItem { panic("tag mismatch") } return self.value.(MyVariantItem) } func MakeMyVariantItem(value MyVariantItem) MyVariant { return MyVariant{MyVariantItem, value} }
Scenario 1: Enum variant named tag
Try giving
taga payload, so it generates a getter for that member's payload.Ah, yep that's definitely a bug. I'll update the summary above and take a look this week.
Reacted by Demetrius KaniosOverview
It looks like this issue is part of a larger pattern that needs to be addressed. I've summarized my findings below, and I made a comprehensive reproduction. @dicej @jfleitz if you have the time, I'd be grateful to have more eyes on this, as my proposed fixes will result in some larger breaking changes.
Scenario 1: Reserved function name collisions with bindings generated from user-defined WIT types.
The issue is that
variantsandresourceshave reserved function names that have the potential to collide with user-defined WIT types.To avoid unecessary breaking changes, I propose that we fix this by suffixing each colliding user-defined function with an underscore (
_). I think it would also be smart to add a doc comment to the colliding user-defined function and print a message to the console to notify the user that their function has been mangled.These are the current reserved functions that have a risk of collision:
variant.Tag()resource.TakeHandle()resource.SetHandleresource.Handle()resource.Drop()resource.OnDrop
Scenario 2: Collisions between bindings generated from enum-like types and other user-defined types
The issue is specific to the enum-like WIT types (
enum,variant, andflags) and any user-defined types that can be defined in a world or interface (excluding WITfunctions). The WIT parser doesn't catch this because the generated bindings for these types create Goconsts that are a concatenation of the name of the type and the type's variants, which then have the potential to collide with another user-defined type.I propose we fix this by formatting the
consts generated for the enum-like types as"{TypeName}_{Case}"as opposed to"{TypeName}{Case}". If we can't think of a better way to handle the collisions, this will be a pretty sizeable breaking changeReacted by Jeremy FleitzTo avoid unecessary breaking changes, I propose that we fix this by suffixing each colliding user-defined function with an underscore (
_). I think it would also be smart to add a doc comment to the colliding user-defined function and print a message to the console to notify the user that their function has been mangled.👍 Sounds good to me.
Scenario 2: Collisions between bindings generated from enum-like types and other user-defined types
I'm not sure I understand the problem here. It's always possible to add definitions to a Go package which conflict with names in other Go packages, but that's one of the reasons the languages has packages: to provide multiple, separated name spaces instead of just a single global name space. If a developer needs to use two packages that declare different types with the same name, they can import the packages with different names and qualify the identifiers using those names, avoiding any conflict or ambiguity.
The only difference in this case as that the package is generated from WIT input, but we can still rely on the name spacing provided by the Go package system just as if the code was hand-written. Am I missing something?
@dicej these collisions actually happen inside of the same package. Take this interface for example:
interface example { record a-b-c {} variant a-b { c, } }
This generates a package that has a type collision due to the const created for the variant
a-b'sccase:package example; // COMPILE ERROR: ABC redeclared in this block // This is the record struct type ABC struct { } // This is the variant struct type AB struct { tag uint8 value any } // This is the variant case `c` const ( ABC uint8 = 0 )
To avoid unecessary breaking changes, I propose that we fix this by suffixing each colliding user-defined function with an underscore (
_). I think it would also be smart to add a doc comment to the colliding user-defined function and print a message to the console to notify the user that their function has been mangled.👍 this sounds good to me too.
On the second scenario, thank you @asteurer ! I was just going to ask if it was the collision within the generated struct prior to where implements/go package naming would come into play. So the generated code already has a breaking change. The
_between type and name seems consistent for this too.@dicej these collisions actually happen inside of the same package.
Ah, I see now. Yeah, I hit the same issue with
componentize-pyand used the{TypeName}_{Case}approach you described. It was a breaking change there just as it would be here, but seems like the right thing to do.Sounds good. Thank you both for your input!
@QuantumSegfault The fix for this just got merged. If this resolves your issue, you're welcome to close it; otherwise, feel free to share what I might have missed!
I'll take a look when I get the chance
While trying to compile Go bindings generated by
wit-bindgen0.61.1for https://github.com/Pumpkin-MC/pumpkin-plugin-wit, I've run into issues with symbol conflicts. I've noticed two major categories:variant my-variant { tag }A variant with member
tagwill generate a getterTagconflicting with the internalTag(discriminator) of the variant itself.record my-variant-item { // ... } variant my-variant { item(my-variant-item), }Having a record
XYconflicts with the enum member produced for the tag for a variantXwith caseY.