- 16 Nov, 2017 19 commits
-
-
Russ Cox authored
If we're running coverage on a package using cgo, we need to apply both cmd/cover and cmd/cgo as source transformers. To date we've applied cgo, then cover. Cover is very sensitive to the exact character position of expressions in its input, though, and cgo is not, so swap them, applying first cover and then cgo. The only drawback here is that coverage formerly applied to SWIG-generated cgo files, and now it does not. I am not convinced anyone depended critically on that, and probably the later analysis with go tool cover would have tried to parse the original .swig file as a Go file and gotten very confused. Fixes #8726. Fixes #9212. Fixes #9479. Change-Id: I777c8b64f7726cb117d59e03073954abc6dfa34d Reviewed-on: https://go-review.googlesource.com/77155Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Russ Cox authored
Passing the absolute path to cgo puts the absolute path in the generated file's //line directives, which then shows that path in the compiler output, which the go command can then make relative to the current directory, same as it does for other compiler output. Change-Id: Ia2064fea40078c46fd97e3a3b8c9fa1488f913e3 Reviewed-on: https://go-review.googlesource.com/77154Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Russ Cox authored
Cgo has always operated by rewriting the AST and invoking go/printer. This CL converts it to use the AST to make decisions but then apply its edits directly to the underlying source text. This approach worked better in rsc.io/grind (used during the C to Go conversion) and also more recently in cmd/cover. It guarantees that all comments and line numbers are preserved exactly. This eliminates a lot of special concern about comments and problems with cgo not preserving meaningful comments. Combined with the CL changing cmd/cover to use the same approach, it means that the combination of applying cgo and applying cover still guarantees all comments and line numbers are preserved exactly. This sets us up to fix some cgo vs cover bugs by swapping the order in which they run during the go command. This also sets up #16623 a bit: the edit list being accumulated here is nearly exactly what you'd want to pass to the compiler for that issue. Change-Id: I7611815be22e7c5c0d4fc3fa11832c42b32c4eb3 Reviewed-on: https://go-review.googlesource.com/77153Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Russ Cox authored
CL 52810 changed Reader to interpret a quoted \r\n as a raw \r\n when reading fields. This seems likely to break existing users, and discussion on both #21201 (the original issue that triggered the change) and #22746 (discussing whether to revert the change) failed to identify a single motivating example for this change. To avoid breaking existing users for no clear reason, revert the change. The Reader has been rewritten in the interim so this is not a git revert but instead and adjustment (and slight simplification) of the new Reader. Fixes #22746. Change-Id: Ie857b2f4b1359a207d085b6d3c3a6d440a997d12 Reviewed-on: https://go-review.googlesource.com/78295Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
-
Russ Cox authored
CL 56950 correctly identified code with checks that were impossible. But instead of correcting the checks it deleted them. This CL corrects the code to check what was meant. Change-Id: Ic89222184ee4fa5cacccae12d750601a9438ac8d Reviewed-on: https://go-review.googlesource.com/78113 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Ian Lance Taylor authored
This fixes the misc/swig tests. Change-Id: I60c87bbd361fe8b4f69e4507b25dc99a226da3d7 Reviewed-on: https://go-review.googlesource.com/76610 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
-
Russ Cox authored
This reverts CL 55972. Reason for revert: this changes Perm's behavior unnecessarily. I asked for this change originally but I now regret it. Reverting so that I don't have to justify it in Go 1.10 release notes. Edited to keep the change to rand_test.go, which seems to have been mostly unrelated. Fixes #22744. Change-Id: If8bb1bcde3ced0db2fdcd0aa65ab128613686c66 Reviewed-on: https://go-review.googlesource.com/78195 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Russ Cox authored
This reverts commit 630d176e. Reason for revert: the CL moves a parser for what appears to be an Android-specific file format into the main code and makes it available on all platforms. Android-specific file formats should be limited to Android. Change-Id: I3f19fe03673d65ed1446a0dcf95e5986053e10c0 Reviewed-on: https://go-review.googlesource.com/77950Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Than McIntosh authored
Fix typo in DWARF register config for GOOARCH=x86; was picking up the AMD64 set, should have been selecting x86 set. Change-Id: I9a4c6f1378baf3cb2f0ad8d60f3ee2f24cd5dc91 Reviewed-on: https://go-review.googlesource.com/77990 Run-TryBot: Than McIntosh <thanm@google.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
-
Russ Cox authored
CL 60630 claimed to and did “improve performance of CopyN” but in doing so introduced a second copy of the I/O copying loop. This code is subtle and easy to get wrong and the last thing we need is of two copies that can drift out of sync. Even the newly introduced copy contains various subtle changes that are not obviously semantically equivalent to the original. (They probably are, but it's not obvious.) Although the CL description does not explain further what the important optimization was, it appears that the most critical one was not allocating a 32kB buffer for CopyN(w, r, 512). This CL deletes the forked copy of copy and instead applies the buffer size restriction optimization directly to copy itself. CL 60630 reported: name old time/op new time/op delta CopyNSmall-4 5.09µs ± 1% 2.25µs ±86% -55.91% (p=0.000 n=11+14) CopyNLarge-4 114µs ±73% 121µs ±72% ~ (p=0.701 n=14+14) Starting with that CL as the baseline, this CL does not change a ton: name old time/op new time/op delta CopyNSmall-8 370ns ± 1% 411ns ± 1% +11.18% (p=0.000 n=16+14) CopyNLarge-8 18.2µs ± 1% 18.3µs ± 1% +0.63% (p=0.000 n=19+20) It does give up a small amount of the win of 60630 but preserves the bulk of it, with the benefit that we will not need to debug these two copies drifting out of sync in the future. Change-Id: I05b1a5a7115390c5867847cba606b75d513eb2e2 Reviewed-on: https://go-review.googlesource.com/78122 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
-
Russ Cox authored
cover -func mode was reporting a coverage for function declarations without bodies - assembly functions. Since we are not annotating their code, we have no data for those functions and should not report them at all. Fixes #6880. Change-Id: I4b8cd90805accf61f54e3ee167f54f4dc10c7c59 Reviewed-on: https://go-review.googlesource.com/77152Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Russ Cox authored
Now that cover does not modify the formatting of the original file or add any newline characters, we can make it print a //line comment pointing back at the original, and compiler errors and panics will report accurate line numbers. Fixes #6329. Fixes #15757. Change-Id: I7b0e386112c69beafe69e0d47c5f9e9abc87c0f5 Reviewed-on: https://go-review.googlesource.com/77151 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
Joe Kyo authored
Change-Id: I491c5ddd1a5d8e55f8e6bb9377bc3811e42773f8 Reviewed-on: https://go-review.googlesource.com/77870Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
Even after disabling on 1-CPU systems, builders are still flaking too often. Unless there are at least 4 CPUs, don't require test interlacing at all. Fixes #22665 (again). Change-Id: Ief792c496c1ee70939532e6ca8bef012fe78178e Reviewed-on: https://go-review.googlesource.com/77310 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
-
Russ Cox authored
CL 21663 allowed drivers to implement ExecerContext without also implementing Execer, and similarly QueryerContext without Queryer, but it did not make that clear in the documentation. This CL updates the documentation. Change-Id: I9a4accaac32edfe255fe7c0b0907d4c1014322b4 Reviewed-on: https://go-review.googlesource.com/78129Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
-
Russ Cox authored
CL 70210 added Decoder for #21590, and in doing so it changed the existing func Decode to return partial results for decoding errors. That seems like a good change to make to Decode, but it was untested (except as used by Decoder), inconsistent with DecodeString in all error cases, and inconsistent with Decoder in not returning partial results for odd-length input strings. This CL makes Decode, DecodeString, and Decoder all agree about the handling of partial results (they are returned) and error precedence (the error earliest in the input is reported), and it documents and tests this. Change-Id: Ifb7d1e100ecb66fe2ed5ba34a621084d480f16db Reviewed-on: https://go-review.googlesource.com/78120 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Russ Cox authored
Change-Id: I112d0164df6abd9ca1df287376cf3605268385df Reviewed-on: https://go-review.googlesource.com/78116 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Rob Pike <r@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Kevin Burke authored
Using ASCII values for keys is a bad idea since it makes them vastly easier to guess. Instead, use the same method as the examples in the golang.org/x/crypto/nacl package to load keys from a hex value. Changing the key required updating the ciphertext in many of the examples. I am still worried about the fact the examples ask the user to authenticate messages; authentication isn't trivial, and to be honest it may be better to steer people to a higher level primitive like secretbox, unless people really need AES. Fixes #21012. Change-Id: I8d918cf194694cd380b06c2d561178167ca61adb Reviewed-on: https://go-review.googlesource.com/48596Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Hiroshi Ioka authored
Fixes #18873 Change-Id: Idb9750f739f91ebca34efcbc177254d412b4d90d Reviewed-on: https://go-review.googlesource.com/44111Reviewed-by: Adam Langley <agl@golang.org> Run-TryBot: Adam Langley <agl@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
- 15 Nov, 2017 21 commits
-
-
Troels Thomsen authored
A sequential lookup using any non-canceled context has a risk of returning the result of the previous lookup for a canceled context (i.e. an error). This is already prevented for timed out context by forgetting the host immediately and extending this to also compare the error to `context.Canceled` resolves this issue. Fixes #22724 Change-Id: I7aafa1459a0de4dc5c4332988fbea23cbf4dba07 Reviewed-on: https://go-review.googlesource.com/77670 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
The replacement rune is a valid rune and can appear as itself in valid UTF8 (it encodes as three bytes). To check for invalid UTF8 it is necessary to look for utf8.DecodeRune returning the replacement rune and size==1. Change-Id: I169be8d1fe61605c921ac13cc2fde94f80f3463c Reviewed-on: https://go-review.googlesource.com/78126 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
-
Russ Cox authored
Change-Id: I540bdabe8ffda4697315fa6f09ad710c05b4a94d Reviewed-on: https://go-review.googlesource.com/78134 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
ctx.Done() == ctx.Background().Done() is just a long way to write ctx.Done() == nil. Use the short way. Change-Id: I7b3198b5dc46b8b40086243aa61882bc8c268eac Reviewed-on: https://go-review.googlesource.com/78128 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
Originally we tried the strict -er suffix as the rule in this case but eventually we decided it was too awkward: io.WriteByter became io.ByteWriter. By analogy, here the interface should be named SessionResetter instead of the awkward ResetSessioner. This change should not affect any drivers that have already implemented the interface, because the method name is not changing. (This was added during the Go 1.10 cycle and has not been released yet, so we can change it.) Change-Id: Ie50e4e090d3811f85965da9da37d966e9f45e79d Reviewed-on: https://go-review.googlesource.com/78127 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
-
Russ Cox authored
Rewrite the text added in CL 50911, which I did not understand. Change-Id: Id6271ffe2f7c8833dd7733fe0254fa4927fac150 Reviewed-on: https://go-review.googlesource.com/78124 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
MultiWriter(w1, w2) only writes to w2 if w1.Write succeeds. I did not know this, and it was not documented. Document and test. Change-Id: Idec2e8444d5a7aca0b95d07814a28daa454eb1d3 Reviewed-on: https://go-review.googlesource.com/78123 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
CL 58210 introduced this constant for reasons I don't understand. It should not be in the exported const block, which will pollute godoc output with a "... unexported" notice. Also since we already have a constant named xmlnsPrefix for "xmlns", it is very confusing to also have xmlNamespacePrefix for "xml". If we must have the constant at all, rename it to xmlPrefix. Change-Id: I15f937454d730005816fcd32b1acca703acf1e51 Reviewed-on: https://go-review.googlesource.com/78121 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
A record can span multiple lines (the whole reason for the extra field), so the important fact is that it's the _start_ of the record. Make that clear in the name. (This API was added during the Go 1.10 cycle so it can still be cleaned up.) Change-Id: Id95b3ceb7cdfc4aa0ed5a053cb84da8945fa5496 Reviewed-on: https://go-review.googlesource.com/78119 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
-
Russ Cox authored
Mainly get rid of the weird zero-value struct literal, but while we're here also group and order things a bit better: first the reader, then the data, then the call (which takes reader then data). Change-Id: I901b0661d85d8eaa0807e4482aac66500ca996c7 Reviewed-on: https://go-review.googlesource.com/78118 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
-
Russ Cox authored
Apparently we maintain this list by hand (for example, CL 52351). Change-Id: I0a0b346cf2b7b547729cb1d0fa1642de447f7bba Reviewed-on: https://go-review.googlesource.com/78117 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
Mainly capitalize the first letter. (Followup to CL 54351.) Change-Id: I2d5c3d72c53d3468de7a9d4af8bd009182ff3d38 Reviewed-on: https://go-review.googlesource.com/78114 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
CL 65851 (bytes) and CL 65910 (strings) “improve[d] readability” by removing the special case that bypassed the whole function body when chars == "". In doing so, yes, the function was unindented a level, which is nice, but the runtime of that case went from O(1) to O(n) where n = len(s). I don't know if anyone's code depends on the O(1) behavior in this case, but quite possibly someone's does. This CL adds the special case back, with a comment to prevent future deletions, and without reindenting each function body in full. Change-Id: I5aba33922b304dd1b8657e6d51d6c937a7f95c81 Reviewed-on: https://go-review.googlesource.com/78112 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Russ Cox authored
ExampleTrimLeft was inexplicably complex. Change-Id: I13ca81bdeba728bdd632acf82e3a1101d29b9f39 Reviewed-on: https://go-review.googlesource.com/78111 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
-
Russ Cox authored
This should help make clear that Len is not counting runes. Also delete empty string, which doesn't add much. Change-Id: I1602352df1897fef6e855e9db0bababb8ab788ca Reviewed-on: https://go-review.googlesource.com/78110 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
-
Russ Cox authored
Change-Id: If7ec07be4ecb0c1d6a1eb5c0740f150473aea6fa Reviewed-on: https://go-review.googlesource.com/78130 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
-
Joe Tsai authored
Change error message prefix from "tar:" to "archive/tar:" to maintain backwards compatibility with Go1.9 and earlier in the unfortunate event that someone is relying on string parsing of errors. Fixes #22740 Change-Id: I59039c59818a0599e9d3b06bb5a531aa22a389b8 Reviewed-on: https://go-review.googlesource.com/77933Reviewed-by: roger peppe <rogpeppe@gmail.com>
-
Daniel Martí authored
If a composite literal contains any comments on their own lines without any elements, the printer would unindent the comments. The comments in this edge case are written when the closing '}' is written. Indent and outdent first so that the indentation is interspersed before the comment is written. Also note that the go/printer golden tests don't show the exact same behaviour that gofmt does. Added a TODO to figure this out in a separate CL. While at it, ensure that the tree conforms to gofmt. The changes are unrelated to this indentation fix, however. Fixes #22355. Change-Id: I5ac25ac6de95a236f1e123479127cc4dd71e93fe Reviewed-on: https://go-review.googlesource.com/74232 Run-TryBot: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Robert Griesemer <gri@golang.org>
-
Austin Clements authored
The CPU time reported in the gctrace for STW phases is simply work.stwprocs times the wall-clock duration of these phases. However, work.stwprocs is set to gcprocs(), which is wrong for multiple reasons: 1. gcprocs is intended to limit the number of Ms used for mark termination based on how well the garbage collector actually scales, but the gctrace wants to report how much CPU time is being stolen from the application. During STW, that's *all* of the CPU, regardless of how many the garbage collector can actually use. 2. gcprocs assumes it's being called during STW, so it limits its result to sched.nmidle+1. However, we're not calling it during STW, so sched.nmidle is typically quite small, even if GOMAXPROCS is quite large. Fix this by setting work.stwprocs to min(ncpu, GOMAXPROCS). This also fixes the overall GC CPU fraction, which is based on the computed CPU times. Fixes #22725. Change-Id: I64b5ce87e28dbec6870aa068ce7aecdd28c058d1 Reviewed-on: https://go-review.googlesource.com/77710 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rick Hudson <rlh@golang.org>
-
Keith Randall authored
Use IndexByte first, as it allows us to skip lots of bytes quickly. If IndexByte is generating a lot of false positives, switch over to Rabin-Karp. Experiments for ppc64le bytes: name old time/op new time/op delta IndexPeriodic/IndexPeriodic2-2 1.12ms ± 0% 0.18ms ± 0% -83.54% (p=0.000 n=10+9) IndexPeriodic/IndexPeriodic4-2 635µs ± 0% 184µs ± 0% -71.06% (p=0.000 n=9+9) IndexPeriodic/IndexPeriodic8-2 289µs ± 0% 184µs ± 0% -36.51% (p=0.000 n=10+9) IndexPeriodic/IndexPeriodic16-2 133µs ± 0% 183µs ± 0% +37.68% (p=0.000 n=10+9) IndexPeriodic/IndexPeriodic32-2 68.3µs ± 0% 70.2µs ± 0% +2.76% (p=0.000 n=10+10) IndexPeriodic/IndexPeriodic64-2 35.8µs ± 0% 36.6µs ± 0% +2.17% (p=0.000 n=8+10) strings: name old time/op new time/op delta IndexPeriodic/IndexPeriodic2-2 184µs ± 0% 184µs ± 0% +0.11% (p=0.029 n=4+4) IndexPeriodic/IndexPeriodic4-2 184µs ± 0% 184µs ± 0% ~ (p=0.886 n=4+4) IndexPeriodic/IndexPeriodic8-2 184µs ± 0% 184µs ± 0% ~ (p=0.486 n=4+4) IndexPeriodic/IndexPeriodic16-2 185µs ± 1% 184µs ± 0% ~ (p=0.343 n=4+4) IndexPeriodic/IndexPeriodic32-2 184µs ± 0% 69µs ± 0% -62.37% (p=0.029 n=4+4) IndexPeriodic/IndexPeriodic64-2 184µs ± 0% 37µs ± 0% -80.17% (p=0.029 n=4+4) Fixes #22578 Change-Id: If2a4d8554cb96bfd699b58149d13ac294615f8b8 Reviewed-on: https://go-review.googlesource.com/76070Reviewed-by: Alberto Donizetti <alb.donizetti@gmail.com>
-
Brad Fitzpatrick authored
Additions to: https://go-review.googlesource.com/c/go/+/61570 https://go-review.googlesource.com/c/go/+/61550 Change-Id: Id89e1119333a8721cb9720a04a01dab1f2705fa9 Reviewed-on: https://go-review.googlesource.com/77591 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
-