Alternatively, to avoid unnecessarily long lines a common prefix could be stripped:

panic: interface conversion: interface is p1/foo.Bar, not p2/foo.Bar

I could come up with a patch for this, but first I want input on whether this makes sense at all. @quentinmit I assume that's what NeedsFix means?

It's fine to send a patch for this (see https://golang.org/doc/contribute.html). Thanks.

Note that the compiler is supposed to already do this. See the code in Sym.symfmt in cmd/compile/internal/gc/fmt.go. I don't know why it is not working for your test case.

@ianlancetaylor thanks for the pointer. I've been playing around with debug prints. I assume you were referring to:

// If the name was used by multiple packages, display the full path,
if s.Pkg.Name != "" && numImport[s.Pkg.Name] > 1 {
        return fmt.Sprintf("%q.%s", s.Pkg.Path, s.Name)
}

But that code will not run when my error is being prepared - when the two _type.string() calls are made in the runtime - as Sym.symfmt sees fmtmode == FTypeId. To my understanding, it should be FErr, as the switch case clearly says case FErr: // This is for the user and the code that we want is in its body.

The way in which fmtmode is set is very confusing, and I haven't been able to track this down. This is also a dance between runtime and gc, two packages I've never touched before. Trying to tinker with this function at all mostly just results in cmd/compile breaking and me having to redo a full make.bash.

Do you have any pointers? Any idea why fmtmode could be wrong? Any way to debug this better than searching via prints?

Thanks!

@mvdan I'm sorry, I don't know the answers to your questions. You could try asking on golang-dev.

@rsc you seem to have touched this code more than anyone else, any idea? I'd rather keep golang-dev as a last resource.

@mvdan I'm happy to help you look into fixing this.

Thanks @mdempsky! See #17283 (comment). If that's going in the wrong direction, how would you go about figuring this out without breaking the compiler at every step?

Okay, familiarized myself a little with the code.

So to summarize the problem, we're storing short package-name-qualified names for types like "foo.Bar", which we use in runtime.TypeAssertionError panics (and a couple other places). The problem is if we have two different package paths "x/foo" and "y/foo" that both use the package name "foo", the resulting error message is ambiguous.

I don't think simply using package path for ambiguous package names will work for a couple reasons:

1. We currently emit the "foo.Bar" strings when compiling the "x/foo" and "y/foo" packages, at which time we don't know whether "foo" will be ambiguous in the resulting program.
2. Because package names and package paths are separate namespaces (though in practice related), mixing the two can introduces their own ambiguities.

I suspect to really fix this, we would need to change TypeAssertionError to always use package path qualification, rather than package name qualification.

Sounds fair. If the path up to the package name ends up being the same, perhaps TypeAssertionError.Error() could omit that part from the output string, to keep the current behaviour when the types are not ambiguous.

That may work, but to reiterate: package name is not guaranteed to be a suffix of package path (or necessarily related at all). For example https://godoc.org/google.golang.org/api/plus/v1 has package path "google.golang.org/api/plus/v1" but package name "plus".

Ah, right. Feel free to take over on the patch, as this seems non-trivial to fix.

I'm having a second crack at this, and finally made a bit of progress. Will send a work-in-progress CL soon, and will ask for pointers from there.

Change https://golang.org/cl/77210 mentions this issue: runtime: use full package paths for

The Go compiler/runtime team discussed this and decided that we should change

interface is foo.Bar, not foo.Baz

to instead say

interface is foo.Bar (x/y/z), not foo.Baz (a/b/c)

I.e., if the package names are the same (even if the type name is different), then we include the package path afterwards.

Also that this isn't a blocking issue for 1.10.

@mdempsky thank you - that seems like a good decision. I'll update my CL shortly before the 1.11 tree opens to implement this.

I just realised that this issue is a dup of #11634. I wasn't able to find that previous issue when filing this one, and just stumbled across it now. Which should we keep? I'm leaning towards this newer one, as it has more discussion towards a solution, and it is already milestoned and assigned.

Unfortunately, #11634 points out this problem is more general than we'd been considering here: we might need to distinguish complex types like func(chan *foo.Bar). Moreover, because of map, struct, and interface type literals, there can be an arbitrary number of named types within a type descriptor.

I think let's close this in favor of the older issue.

@mdempsky makes sense, thank you. I'm glad that at least we noticed early, before we sunk more time into a flawed solution and/or the wrong problem.