{"action":"created","issue":{"url":"https://api.github.com/repos/domokit/mojo/issues/765","repository_url":"https://api.github.com/repos/domokit/mojo","labels_url":"https://api.github.com/repos/domokit/mojo/issues/765/labels{/name}","comments_url":"https://api.github.com/repos/domokit/mojo/issues/765/comments","events_url":"https://api.github.com/repos/domokit/mojo/issues/765/events","html_url":"https://github.com/domokit/mojo/issues/765","id":152594287,"number":765,"title":"enums in go bindings should have the enum type","user":{"login":"jynnantonix","id":455457,"avatar_url":"https://avatars.githubusercontent.com/u/455457?v=3","gravatar_id":"","url":"https://api.github.com/users/jynnantonix","html_url":"https://github.com/jynnantonix","followers_url":"https://api.github.com/users/jynnantonix/followers","following_url":"https://api.github.com/users/jynnantonix/following{/other_user}","gists_url":"https://api.github.com/users/jynnantonix/gists{/gist_id}","starred_url":"https://api.github.com/users/jynnantonix/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jynnantonix/subscriptions","organizations_url":"https://api.github.com/users/jynnantonix/orgs","repos_url":"https://api.github.com/users/jynnantonix/repos","events_url":"https://api.github.com/users/jynnantonix/events{/privacy}","received_events_url":"https://api.github.com/users/jynnantonix/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/domokit/mojo/labels/bindings","name":"bindings","color":"207de5"}],"state":"open","locked":false,"assignee":{"login":"MitchRudominer","id":10732743,"avatar_url":"https://avatars.githubusercontent.com/u/10732743?v=3","gravatar_id":"","url":"https://api.github.com/users/MitchRudominer","html_url":"https://github.com/MitchRudominer","followers_url":"https://api.github.com/users/MitchRudominer/followers","following_url":"https://api.github.com/users/MitchRudominer/following{/other_user}","gists_url":"https://api.github.com/users/MitchRudominer/gists{/gist_id}","starred_url":"https://api.github.com/users/MitchRudominer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MitchRudominer/subscriptions","organizations_url":"https://api.github.com/users/MitchRudominer/orgs","repos_url":"https://api.github.com/users/MitchRudominer/repos","events_url":"https://api.github.com/users/MitchRudominer/events{/privacy}","received_events_url":"https://api.github.com/users/MitchRudominer/received_events","type":"User","site_admin":false},"milestone":null,"comments":5,"created_at":"2016-05-02T16:54:58Z","updated_at":"2016-05-02T20:02:32Z","closed_at":null,"body":"Consider the following enum in a mojom:\r\n```\r\nenum Error {\r\n  OK = 0,\r\n  INVALID_ARGUMENT,\r\n  PERMISSION_DENIED,\r\n  BUSY,\r\n  OUT_OF_BOUNDS,\r\n  UNIMPLEMENTED,\r\n  INTERNAL,\r\n};\r\n```\r\n\r\nThe generated go bindings are:\r\n\r\n```go\r\ntype Error int32\r\n\r\nconst (\r\n\tError_Ok = 0\r\n\tError_InvalidArgument = Error_Ok + 1;\r\n\tError_PermissionDenied = Error_InvalidArgument + 1;\r\n\tError_Busy = Error_PermissionDenied + 1;\r\n\tError_OutOfBounds = Error_Busy + 1;\r\n\tError_Unimplemented = Error_OutOfBounds + 1;\r\n\tError_Internal = Error_Unimplemented + 1;\r\n)\r\n```\r\n\r\nWhile it's good that the bindings define the enum to be its own type, they don't actually define the values of the enum to have the same type.  This means that code like the following:\r\n\r\n```go\r\nfunc DoSomething() Error {\r\n\terr := Error_Ok\r\n\r\n\t// Do some work, maybe change err.\r\n\r\n\treturn err\r\n}\r\n```\r\n\r\nfails to compile with the error `cannot use err (type int) as type Error in return argument`.\r\n\r\nThis can be fixed by giving the enum values the enum type in the `const()`.  This would also make the 'Error_' prefix unnecessary since the compiler would check the actual types of the values.  The proper generated bindings should look like:\r\n\r\n```go\r\ntype Error int32\r\n\r\nconst (\r\n\tOk Error = 0\r\n\tInvalidArgument = Ok + 1;\r\n\tPermissionDenied = InvalidArgument + 1;\r\n\tBusy = PermissionDenied + 1;\r\n\tOutOfBounds = Busy + 1;\r\n\tUnimplemented = OutOfBounds + 1;\r\n\tInternal = Unimplemented + 1;\r\n)\r\n```\r\n\r\n@vardhan-cr @crawshaw @shinykitten @MitchRudominer "},"comment":{"url":"https://api.github.com/repos/domokit/mojo/issues/comments/216346056","html_url":"https://github.com/domokit/mojo/issues/765#issuecomment-216346056","issue_url":"https://api.github.com/repos/domokit/mojo/issues/765","id":216346056,"user":{"login":"vardhan-cr","id":13354180,"avatar_url":"https://avatars.githubusercontent.com/u/13354180?v=3","gravatar_id":"","url":"https://api.github.com/users/vardhan-cr","html_url":"https://github.com/vardhan-cr","followers_url":"https://api.github.com/users/vardhan-cr/followers","following_url":"https://api.github.com/users/vardhan-cr/following{/other_user}","gists_url":"https://api.github.com/users/vardhan-cr/gists{/gist_id}","starred_url":"https://api.github.com/users/vardhan-cr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/vardhan-cr/subscriptions","organizations_url":"https://api.github.com/users/vardhan-cr/orgs","repos_url":"https://api.github.com/users/vardhan-cr/repos","events_url":"https://api.github.com/users/vardhan-cr/events{/privacy}","received_events_url":"https://api.github.com/users/vardhan-cr/received_events","type":"User","site_admin":false},"created_at":"2016-05-02T20:02:31Z","updated_at":"2016-05-02T20:02:31Z","body":"the const typing you're proposing lgtm, but I have reservations about removing the enum name from the enum values in the generated go bindings:\r\n\r\nI think we should prefix the enum values with the enum's name /because/ of the collision; enum value should always be enum-scoped, not namespace/module-scoped.  Retrofitting enum values to avoid collision with a different enum value (or any other identifier actually) in the same mojom scope (or module scope) is not a good solution. C++ and Dart bindings already deliberately scope their enum values with the enum name for this reason (i.e., in C++, you /have/ to do Error::Ok since it's an 'enum class').\r\n\r\nRegards to the '_' in Go names; i think this is a general problem of trying to map mojom types (and features) to a language's types and features. AFAIK, its not possible to scope constants inside other types in Go, so I see this as a sacrifice. We can choose to name-mangle however we want (we currently managle mojom names of 'a_bucket' to Go names 'aBucket'), though I think we should keep the '_' in enum names."}}