Navigation Menu

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

cmd/go2go: adding strings together is not defined #39623

Closed
aykevl opened this issue Jun 16, 2020 · 5 comments
Closed

cmd/go2go: adding strings together is not defined #39623

aykevl opened this issue Jun 16, 2020 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@aykevl
Copy link

aykevl commented Jun 16, 2020

I've been experimenting a bit with generics at https://go2goplay.golang.org/ and I think I found a bug.

What did you do?

See: https://go2goplay.golang.org/p/efS6x6s-9NI
I tried concatenating a string:

type IntString interface {
	type int, string
}

func Sum(type T IntString)(s []T) (result T) {
	for _, v := range s {
		result += v // this fails even though it's possible to add strings together
	}
	return
}

What did you expect to see?

Strings can be added (concatenated) together, so I would expect it to also work when using generics.
Specialized variant of the same function, which does compile:

func SumString(s []string) (result string) {
	for _, v := range s {
		result += v
	}
	return
}

What did you see instead?

It reports the following error:

type checking failed for main
prog.go2:13:3: invalid operation: operator + not defined for result (variable of type T)

Both integers and strings support the + operator so I would expect this to work.

@ChrisHines
Copy link
Contributor

There seems to be some interaction with the int type in the constraint here. This works https://go2goplay.golang.org/p/nYbMg_uv31l:

package main

import "fmt"

func main() {
	fmt.Println(Add("Hello", ", world!"))
}

func Add(type T addable)(x, y T) T {
	return x + y
}

type addable interface {
	type string
}
Hello, world!

But this does not https://go2goplay.golang.org/p/Ksywhb4RF6x:

package main

import "fmt"

func main() {
	fmt.Println(Add("Hello", ", world!"))
}

func Add(type T addable)(x, y T) T {
	return x + y
}

type addable interface {
	type string, int
}
type checking failed for main
prog.go2:10:9: invalid operation: operator + not defined for x (variable of type T)

@griesemer griesemer self-assigned this Jun 17, 2020
@griesemer
Copy link
Contributor

Yes, this is a bug in the type checker; thanks for reporting.

One of the more complicated things to get right is computing the subset of operations that are valid for all types of a type list. In this case it should have been easy, but obviously there's a bug somewhere.

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 17, 2020
@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Jun 17, 2020
@ianlancetaylor ianlancetaylor changed the title go2go: adding strings together is not defined cmd/go2go: adding strings together is not defined Jun 17, 2020
@gopherbot
Copy link

Change https://golang.org/cl/238359 mentions this issue: [dev.go2go] go/types: use correct predicate when checking binary addition

gopherbot pushed a commit that referenced this issue Jun 17, 2020
…tion

See also https://team-review.git.corp.google.com/c/golang/go2-dev/+/759562
for the analogous problem for another predicate.

Fixes #39623.

Change-Id: I39efcb07c337691eb202de166e2cb8d6b5d5ee18
Reviewed-on: https://go-review.googlesource.com/c/go/+/238359
Reviewed-by: Robert Griesemer <gri@golang.org>
@griesemer
Copy link
Contributor

This is now fixed in dev.go2go. Will be fixed in the playground as soon as it's updated.

@james-antill
Copy link

FWIW I can confirm that it works in playground now.

@golang golang locked and limited conversation to collaborators Jun 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants