Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compiler,runtime: fix multiple definitions of a single function #352

Merged
merged 2 commits into from
May 24, 2019

Conversation

aykevl
Copy link
Member

@aykevl aykevl commented May 14, 2019

strings.IndexByte used to be implemented in the runtime. Now it is implemented using a direct call to internal/bytealg.IndexByte. Fixes #351.

The real problem is of course golang/go#31330.

@aykevl
Copy link
Member Author

aykevl commented May 14, 2019

This fails on Go 1.11 only, because the change was introduced in Go 1.12. @deadprogram maybe we should drop support for Go 1.11? One reason for keeping it would be that the next Debian stable release will ship with Go 1.11 so supporting Go 1.11 would ease the installation for people using that. Alternatives would be to allow multiple definitions and just pick the first (gross, hard to maintain in the future) or to check the version of the used Go standard library (more work). Any preferences?

@deadprogram
Copy link
Member

I hate to say it, but checking the version of the Go standard library sounds like the way. The Debian thing is reason enough.

@aykevl aykevl added this to the Version 0.6 milestone May 21, 2019
@deadprogram deadprogram added the bug Something isn't working label May 21, 2019
aykevl added 2 commits May 24, 2019 14:21
strings.IndexByte was implemented in the runtime up to Go 1.11. It is
implemented using a direct call to internal/bytealg.IndexByte since Go
1.12.

Make sure we remain compatible with both.
@aykevl aykevl force-pushed the fix-multiple-definitions branch from af6870c to 7217610 Compare May 24, 2019 12:38
@aykevl aykevl changed the base branch from master to dev May 24, 2019 12:38
@aykevl
Copy link
Member Author

aykevl commented May 24, 2019

This is hopefully fixed now.

@deadprogram
Copy link
Member

It fixes it for me. Now merging, thank you very much!

@deadprogram deadprogram merged commit f2cd4d1 into dev May 24, 2019
@deadprogram deadprogram deleted the fix-multiple-definitions branch May 24, 2019 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants