cmd/go: return more errors from ReadModFile, loadModFile

Return more errors instead of base.Fatalf, so we can handle them in the callers.

For #57001.

Change-Id: If3e63d3f64188148f5d750991f9cb1175790d89d Reviewed-on: https://go-review.googlesource.com/c/go/+/499983 Reviewed-by: Bryan Mills bcmills@google.com Auto-Submit: Russ Cox rsc@golang.org TryBot-Result: Gopher Robot gobot@golang.org Run-TryBot: Russ Cox rsc@golang.org

That sounds like a detail worthy of being in the api/docs

What version of Go, VS Code & VS Code Go extension are you using?

• Run go version to get version of Go from the VS Code integrated terminal.
  • go version go1.18.10 linux/amd64
• Run gopls -v version to get version of Gopls from the VS Code integrated terminal.
  • v0.12.0
• Run code -v or code-insiders -v to get version of VS Code or VS Code Insiders.
  • 1.78.2
• Check your installed extensions to get the version of the VS Code Go extension
  • v0.38.0
• Run Ctrl+Shift+P (Cmd+Shift+P on Mac OS) > Go: Locate Configured Go Tools command.
  • I can't provide all these information but if you need a specific detail, don't hesitate to ask me.

Describe the bug

The breakpoint of my debugger doesn't work with my Remote-SSH session.

At first, the error was "could not find file" then I added a "substiturePath". Now my error is "breakpoint 1 can not be enabled".

You can see below my launch.json.

Steps to reproduce the behavior:

1. Add a random breakpoint reachable
2. Press F5 or click on Start debugging
3. The debug session runs normally but the breakpoint is not working

git-codereview: recognize sso://go/ and rpc://go/

Google engineers are now required to use these access methods instead of https://go.googlesource.com/, so recognize them as aliases.

(It is possible for every engineer to configure their Git clients with insteadOf clauses to hide these from the codereview plugin, but it's far less error-prone to just handle it here.)

Change-Id: Ic1c9a6b45aa61b11ff25ce21bca6e49344974b04 Reviewed-on: https://go-review.googlesource.com/c/review/+/499923 TryBot-Result: Gopher Robot gobot@golang.org Reviewed-by: Peter Weinberger pjw@google.com Run-TryBot: Russ Cox rsc@golang.org

Hi @prattmic, if I try your CL 497175 (ps 1, fa555c12) on my original uncommented example:

$ gotip version
go version devel go1.21-fa555c12 Mon May 22 16:58:07 2023 -0400

$ gotip build -pgo=cpu.out -gcflags='-m=99 -d=pgodebug=99 -d=pgodevirtualize=2'

I still get the no function body message in the log and no devirtualization:

./main.go:42:16: PGO devirtualize considering call s.Speak()
./main.go:42:16: edge main.f:6 -> example/speak.(*Speaker1).Speak (weight 186): hottest so far
./main.go:42:16: edge main.f:6 -> example/speak.(*Speaker2).Speak (weight 34): too cold (hottest 186)
./main.go:42:16 call main.f:6: hottest callee example/speak.(*Speaker1).Speak (weight 186)
./speak/speak.go:9:6: should not PGO devirtualize (*Speaker1).Speak: no function body

I also sent CL 500155, which was a quick exploratory hack. That does seem to successfully allow devirtualization:

./main.go:42:16: PGO devirtualize considering call s.Speak()
./main.go:42:16: edge main.f:6 -> example/speak.(*Speaker1).Speak (weight 186): hottest so far
./main.go:42:16: edge main.f:6 -> example/speak.(*Speaker2).Speak (weight 34): too cold (hottest 186)
./main.go:42:16 call main.f:6: hottest callee example/speak.(*Speaker1).Speak (weight 186)
./main.go:42:16: PGO devirtualizing call to speak.(*Speaker1).Speak

Is your CL 497175 something that might be merged for 1.21, or is that more likely 1.22 at this point?

If a more targeted fix is in order for 1.21, I'd be interested in trying to mature my CL 500155 into a real fix and add tests. (It is likely possible to do better than a string compare 😅). That said, I'd also of course understand if you'd prefer to address this yourself via CL 497175 or another CL.

Thanks, and the PGO devirtualization work seems very promising!

tools fieldalignment -fix Can cause comments to be lost

$ go version 1.19.5

Found new dashboard test flakes for:

#!watchflakes
default <- pkg == "cmd/go" && test == "TestScript" && builder != "illumos-amd64"

vcs-test.golang.org rerouted to http://127.0.0.1:55017
https://vcs-test.golang.org rerouted to https://127.0.0.1:55018
go test proxy running at GOPROXY=http://127.0.0.1:55019/mod
--- FAIL: TestScript (1.29s)
    --- FAIL: TestScript/test_race_install_cgo (4.72s)
        script_test.go:134: 2023-06-01T23:47:41Z
        script_test.go:136: $WORK=/tmp/buildlet/tmp/cmd-go-test-1610893047/tmpdir3780688752/test_race_install_cgo4160555547
        script_test.go:158: 
            # Tests Issue #10500 (4.550s)
            > [!race] skip
...
    --- FAIL: TestScript/mod_sumdb_golang (3.40s)
        script_test.go:134: 2023-06-01T23:47:43Z
        script_test.go:136: $WORK=/tmp/buildlet/tmp/cmd-go-test-1610893047/tmpdir3780688752/mod_sumdb_golang2870893587
        script_test.go:158: 
            # Test default GOPROXY and GOSUMDB (3.264s)
            > env GOPROXY=
            > env GOSUMDB=
            > go env GOPROXY
            [stdout]
            https://proxy.golang.org,direct
...
            r12    0xc000126db0
            r13    0x21c8240
            r14    0x111757e
            r15    0x10
            rip    0x10053f1
            rflags 0x10206
            cs     0x2b
            fs     0x0
            gs     0x0
        script_test.go:158: FAIL: testdata/script/build_GOTMPDIR.txt:9: go build -x hello.go: exit status 2

— watchflakes

I am OK with this.

@rolandshoemaker Great, thanks.

Change https://go.dev/cl/500195 mentions this issue: go/types, types2: handle all type parameters before big unify switch

Yup.

Found new dashboard test flakes for:

#!watchflakes
default <- pkg == "cmd/go" && test == "TestScript" && builder != "illumos-amd64"

vcs-test.golang.org rerouted to http://127.0.0.1:53914
https://vcs-test.golang.org rerouted to https://127.0.0.1:53915
go test proxy running at GOPROXY=http://127.0.0.1:53916/mod
--- FAIL: TestScript (1.43s)
    --- FAIL: TestScript/gotoolchain_local (78.74s)
        script_test.go:134: 2023-06-01T21:55:45Z
        script_test.go:136: $WORK=/tmp/buildlet/tmp/cmd-go-test-2361753261/tmpdir925950850/gotoolchain_local2668578490
        script_test.go:158: 
            # This test uses the fake toolchain switch support in cmd/go/internal/toolchain.Switch
            # to exercise all the version selection logic without needing actual toolchains.
...
    --- FAIL: TestScript/work_init_path (0.05s)
        script_test.go:134: 2023-06-01T21:57:04Z
        script_test.go:136: $WORK=/tmp/buildlet/tmp/cmd-go-test-2361753261/tmpdir925950850/work_init_path1920241360
        script_test.go:158: 
        script_test.go:158: FAIL: testdata/script/work_init_path.txt:0: context deadline exceeded
    --- FAIL: TestScript/build_import_comment (5.67s)
        script_test.go:134: 2023-06-01T21:56:58Z
        script_test.go:136: $WORK=/tmp/buildlet/tmp/cmd-go-test-2361753261/tmpdir925950850/build_import_comment1140798218
        script_test.go:158: 
            # Test in GOPATH mode first. (0.000s)
...
    --- FAIL: TestScript/bug (0.11s)
        script_test.go:134: 2023-06-01T21:57:04Z
        script_test.go:136: $WORK=/tmp/buildlet/tmp/cmd-go-test-2361753261/tmpdir925950850/bug1264553030
        script_test.go:158: 
        script_test.go:158: FAIL: testdata/script/bug.txt:0: context deadline exceeded
    --- FAIL: TestScript/badgo (0.09s)
        script_test.go:134: 2023-06-01T21:57:04Z
        script_test.go:136: $WORK=/tmp/buildlet/tmp/cmd-go-test-2361753261/tmpdir925950850/badgo1349235223
        script_test.go:158: 
        script_test.go:158: FAIL: testdata/script/badgo.txt:0: context deadline exceeded

— watchflakes

400 lines and a call to unsafe is a sort of a lot of work. 😆

Change https://go.dev/cl/500155 mentions this issue: cmd/compile/internal/devirtualize: devirtualize methods in other packages if current package has a concrete reference

Change https://go.dev/cl/500155 mentions this issue: cmd/compile/internal/devirtualize: devirtualize methods in other packages if current package has a concrete reference

I hacked up a CL which does the right thing when calling Concat on input slices that alias each other. It isn't too horrible, see the CL for details.

doc/go1.21: crypto release notes

Updates #58645

Change-Id: Ib7e2baba41bb327d8fc466afb1e117fe2f22e1c9 Reviewed-on: https://go-review.googlesource.com/c/go/+/499637 Reviewed-by: Damien Neil dneil@google.com Auto-Submit: Roland Shoemaker roland@golang.org TryBot-Result: Gopher Robot gobot@golang.org Run-TryBot: Roland Shoemaker roland@golang.org

Change https://go.dev/cl/499997 mentions this issue: go/types, types2: remove version check for more lenient constant handling in inference

Change https://go.dev/cl/499997 mentions this issue: go/types, types2: remove version check for more lenient constant handling in inference

cc: @findleyr @ianlancetaylor for feedback

The Go 1.21 compiler implements significantly more powerful and precise type inference than Go 1.20. One of the improvements made is for untyped constants (#58671). That change was implemented based on the language version: -lang must be >= go1.21.

func f[P any](...P) {}

func main() {
	f(1, 2.0)
}

produces an error (default type float64 of 2.0 does not match inferred type int for P) with -lang=go1.20 but runs fine with -lang >= go1.21.

However, there are plenty of other inference improvements that that were made for 1.21 which are accessible even if -lang < go1.21. For instance:

package main

func f[T any](interface{ m(T) }) {}

type S struct{}

func (S) m(int) {}

func main() {
	f(S{})
}

In this case the change and error is not fundamentally different from above: in Go 1.20 we couldn't infer a type argument and now we can. In both these cases one can provide explicit type arguments are make the code compile. All inference improvements are fully backwards-compatible.

I propose that we remove the -lang check for the more lenient constant handling for consistency with the other inference improvements. (Making all improvements -lang dependent is not practical.)

Change https://go.dev/cl/500175 mentions this issue: slices: implement Concat