go-capnp icon indicating copy to clipboard operation
go-capnp copied to clipboard

capnpc-go should not generate name collisions

Open zombiezen opened this issue 9 years ago • 7 comments

One of the notable examples is in persistent.capnp that the annotation and the type collide. However, there are subtler cases, such as the struct field in schema.capnp should not collide with the Struct field of the wrapper struct.

zombiezen avatar Aug 22 '16 05:08 zombiezen

Hope it's cool if I think out loud on this thread.

One obvious strategy: we need an injective function from legal capnproto idenfiers to legal go identifiers that don't collide with names that go-capnpc uses for helper functions (e.g. *_ServerToClient).

Facts we can potentially take advantage of:

  • capnproto has different case rules for different kinds of identifiers, e.g. I think annotations are the only thing that can appear at top-level in a schema file that can start with a lower case letter. This might be be the only case where we have to worry about upper-casing a capnproto name causing a collision with another capnproto name on its own.
  • underscores are legal in go names, but not in capnproto names. We already seem to use underscores as namespace seperators.
    • Idea: if a name would otherwise collide with a go or go-capnpc name, append a single underscore. e.g. Struct becomes Struct_
  • Filesystems on MacOS and Windows are case-insensitive by default, so if someone tries to name two files e.g. web-session.capnp and webSession.capnp, they're going to hit other problems anyway. So maybe we can just punt on this and say "don't do that." $Go.name still exists if someone gets screwed over by someone else's schema.
  • There are some conventions that show up in sandstorm and the main repo re: what chars get used in filenames, so we can probably optimize at those, and for stuff that tends not to show up risk generating something ugly.

zenhack avatar Sep 17 '16 21:09 zenhack

For characters in filenames that don't constitute legal go package names, we can replace them with strings that start with a double underscore, e.g. c++.capnp generates a package named c__plus__plus. Since dash (-) shows up often in idiomatic filenames, we can maybe special case this to camelCasing, taking advantage of the filesystem point I made above (so websession.capnp generates package websession, and web-session.capnp generates package webSession).

zenhack avatar Sep 17 '16 21:09 zenhack

Obviously, a lot of these techniques are going to cause source-level breakage, don't know how you want to handle that.

zenhack avatar Sep 17 '16 21:09 zenhack

We could also just make go-capnpc use names with a trailing underscore, that don't otherwise conflict with go keywords. This would let us introduce new ones without worrying about disturbing the names of exising schema.

zenhack avatar Sep 17 '16 21:09 zenhack

One other thing that needs thinking about; what do we do about importing different schema with the same filename, and therefore package name? e.g. "/foo/mypackage.capnp" and "/foo/mypackage.capnp". I there's a similar question for dealing with #42, and an ideal solution would cover both.

zenhack avatar Sep 17 '16 21:09 zenhack

Maybe add this to the v3 milestone? Allowing it to be a breaking change opens up a lot of options.

zenhack avatar May 18 '17 19:05 zenhack

With pull request https://github.com/capnproto/go-capnp/pull/542 the capnproto files in std are up-to-date but they still have slight changes from upstream:

  1. They all have $Go.package() and $Go.import() at the bottom
  2. std/capnp/schema.capnp still uses $Go.name() to rename the three instances of "struct," as otherwise they would become "Struct" in the generated code which would not compile.

Thinking out loud here, it makes sense to eventually remove these duplicate copies of the capnproto files and use the vanilla unmodified copies directly from upstream. Seems like https://github.com/capnproto/go-capnp/issues/47 is where that final goal can be reached.

If we remove the $Go.name() renames, I think this issue is completely fixed.

davidhubbard avatar Aug 11 '23 02:08 davidhubbard