- 28 Aug, 2019 1 commit
-
-
Keith Randall authored
We never used it, might as well get rid of it. Change-Id: I5c23c93e90173bff9ac1fc1b8ae1e2025215d6eb Reviewed-on: https://go-review.googlesource.com/c/go/+/191938 Run-TryBot: Keith Randall <khr@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
-
- 27 Aug, 2019 32 commits
-
-
Filippo Valsorda authored
Fixes #30055 Change-Id: If757c43b52fc7bf62b0afb1c720615329fb5569d Reviewed-on: https://go-review.googlesource.com/c/go/+/191999 Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Filippo Valsorda authored
SSLv3 has been irreparably broken since the POODLE attack 5 years ago and RFC 7568 (f.k.a. draft-ietf-tls-sslv3-diediedie) prohibits its use in no uncertain terms. As announced in the Go 1.13 release notes, remove support for it entirely in Go 1.14. Updates #32716 Change-Id: Id653557961d8f75f484a01e6afd2e104a4ccceaf Reviewed-on: https://go-review.googlesource.com/c/go/+/191976 Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Josh Bleecher Snyder authored
While working on #30645, I noticed that many instances in which the walkinrange optimization could apply were not even being considered. This was because of extraneous blocks in the CFG, of the type that shortcircuit normally removes. The change improves the shortcircuit pass to handle most of those cases. (There are a few that can only be reasonably detected later in compilation, after other optimizations have been run, but not enough to be worth chasing.) Notable changes: * Instead of calculating live-across-blocks values, use v.Uses == 1. This is cheaper and more straightforward. v.Uses did not exist when this pass was initially written. * Incorporate a fusePlain and loop until stable. This is necessary to find many of the instances. * Allow Copy and Not wrappers around Phi values. This significantly increases effectiveness. * Allow removal of all preds, creating a dead block. The previous pass stopped unnecessarily at one pred. * Use phielimValue during cleanup instead of manually setting the op to OpCopy. The result is marginally faster compilation and smaller code. name old time/op new time/op delta Template 213ms ± 2% 212ms ± 2% -0.63% (p=0.002 n=49+48) Unicode 90.0ms ± 2% 89.8ms ± 2% ~ (p=0.122 n=48+48) GoTypes 710ms ± 3% 711ms ± 2% ~ (p=0.433 n=45+49) Compiler 3.23s ± 2% 3.22s ± 2% ~ (p=0.124 n=47+49) SSA 10.0s ± 1% 10.0s ± 1% -0.43% (p=0.000 n=48+50) Flate 135ms ± 3% 135ms ± 2% ~ (p=0.311 n=49+49) GoParser 158ms ± 2% 158ms ± 2% ~ (p=0.757 n=48+48) Reflect 447ms ± 2% 447ms ± 2% ~ (p=0.815 n=49+48) Tar 189ms ± 2% 189ms ± 3% ~ (p=0.530 n=47+49) XML 251ms ± 3% 250ms ± 1% -0.75% (p=0.002 n=49+48) [Geo mean] 427ms 426ms -0.25% name old user-time/op new user-time/op delta Template 265ms ± 2% 265ms ± 2% ~ (p=0.969 n=48+50) Unicode 119ms ± 6% 119ms ± 6% ~ (p=0.738 n=50+50) GoTypes 923ms ± 2% 925ms ± 2% ~ (p=0.057 n=43+47) Compiler 4.37s ± 2% 4.37s ± 2% ~ (p=0.691 n=50+46) SSA 13.4s ± 1% 13.4s ± 1% ~ (p=0.282 n=42+49) Flate 162ms ± 2% 162ms ± 2% ~ (p=0.774 n=48+50) GoParser 186ms ± 2% 186ms ± 3% ~ (p=0.213 n=47+47) Reflect 572ms ± 2% 573ms ± 3% ~ (p=0.303 n=50+49) Tar 240ms ± 3% 240ms ± 2% ~ (p=0.939 n=46+44) XML 302ms ± 2% 302ms ± 2% ~ (p=0.399 n=47+47) [Geo mean] 540ms 541ms +0.07% name old alloc/op new alloc/op delta Template 36.8MB ± 0% 36.7MB ± 0% -0.42% (p=0.008 n=5+5) Unicode 28.1MB ± 0% 28.1MB ± 0% ~ (p=0.151 n=5+5) GoTypes 124MB ± 0% 124MB ± 0% -0.26% (p=0.008 n=5+5) Compiler 571MB ± 0% 566MB ± 0% -0.84% (p=0.008 n=5+5) SSA 1.86GB ± 0% 1.85GB ± 0% -0.58% (p=0.008 n=5+5) Flate 22.8MB ± 0% 22.8MB ± 0% -0.17% (p=0.008 n=5+5) GoParser 27.3MB ± 0% 27.3MB ± 0% -0.20% (p=0.008 n=5+5) Reflect 79.5MB ± 0% 79.3MB ± 0% -0.20% (p=0.008 n=5+5) Tar 34.7MB ± 0% 34.6MB ± 0% -0.42% (p=0.008 n=5+5) XML 45.4MB ± 0% 45.3MB ± 0% -0.29% (p=0.008 n=5+5) [Geo mean] 80.0MB 79.7MB -0.34% name old allocs/op new allocs/op delta Template 378k ± 0% 377k ± 0% -0.22% (p=0.008 n=5+5) Unicode 339k ± 0% 339k ± 0% ~ (p=0.643 n=5+5) GoTypes 1.36M ± 0% 1.36M ± 0% -0.10% (p=0.008 n=5+5) Compiler 5.51M ± 0% 5.50M ± 0% -0.13% (p=0.008 n=5+5) SSA 17.5M ± 0% 17.5M ± 0% -0.14% (p=0.008 n=5+5) Flate 234k ± 0% 234k ± 0% -0.04% (p=0.008 n=5+5) GoParser 299k ± 0% 299k ± 0% -0.05% (p=0.008 n=5+5) Reflect 978k ± 0% 979k ± 0% +0.02% (p=0.016 n=5+5) Tar 351k ± 0% 351k ± 0% -0.04% (p=0.008 n=5+5) XML 435k ± 0% 435k ± 0% -0.11% (p=0.008 n=5+5) [Geo mean] 840k 840k -0.08% file before after Δ % go 14794788 14770212 -24576 -0.166% addr2line 4203688 4199592 -4096 -0.097% api 5954056 5941768 -12288 -0.206% asm 4862704 4846320 -16384 -0.337% cgo 4778920 4770728 -8192 -0.171% compile 24001568 23923792 -77776 -0.324% cover 5198440 5190248 -8192 -0.158% dist 3595248 3587056 -8192 -0.228% doc 4618504 4610312 -8192 -0.177% fix 3337416 3333320 -4096 -0.123% link 6120408 6116312 -4096 -0.067% nm 4149064 4140872 -8192 -0.197% objdump 4555608 4547416 -8192 -0.180% pprof 14616324 14595844 -20480 -0.140% test2json 2766328 2762232 -4096 -0.148% trace 11638844 11622460 -16384 -0.141% vet 8274936 8258552 -16384 -0.198% total 132520780 132270972 -249808 -0.189% Change-Id: Ifcd235a2a6e5f13ed5c93e62523e2ef61321fccf Reviewed-on: https://go-review.googlesource.com/c/go/+/178197 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
-
Bryan C. Mills authored
'git ls-remote' started recognizing the '--' separator at some point after 2.7.4, but git defaults to version 2.7.4 on Ubuntu 16.04 LTS, which remains supported by Ubuntu until April 2021. We added '--' tokens to most VCS commands as a defensive measure in CL 181237, but it isn't strictly necessary here because the 'scheme' argument to our template is chosen from a predefined list: we can safely drop it to retain compatibility. Fixes #33836 Updates #26746 Change-Id: Ibb53366b95f8029b587e0b7646a439330d759ac7 Reviewed-on: https://go-review.googlesource.com/c/go/+/191978 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
-
Josh Bleecher Snyder authored
CSE can make dead values live again. Running deadcode first avoids that; it also makes CSE more efficient. file before after Δ % api 5970616 5966520 -4096 -0.069% asm 4867088 4846608 -20480 -0.421% compile 23988320 23935072 -53248 -0.222% link 6084376 6080280 -4096 -0.067% nm 4165736 4161640 -4096 -0.098% objdump 4572216 4568120 -4096 -0.090% pprof 14452996 14457092 +4096 +0.028% trace 11467292 11471388 +4096 +0.036% total 132181100 132099180 -81920 -0.062% Compiler performance impact is negligible: name old alloc/op new alloc/op delta Template 38.8MB ± 0% 38.8MB ± 0% -0.04% (p=0.008 n=5+5) Unicode 28.2MB ± 0% 28.2MB ± 0% ~ (p=1.000 n=5+5) GoTypes 131MB ± 0% 131MB ± 0% -0.14% (p=0.008 n=5+5) Compiler 606MB ± 0% 606MB ± 0% -0.05% (p=0.008 n=5+5) SSA 2.14GB ± 0% 2.13GB ± 0% -0.26% (p=0.008 n=5+5) Flate 24.0MB ± 0% 24.0MB ± 0% -0.18% (p=0.008 n=5+5) GoParser 28.8MB ± 0% 28.8MB ± 0% -0.15% (p=0.008 n=5+5) Reflect 83.8MB ± 0% 83.7MB ± 0% -0.11% (p=0.008 n=5+5) Tar 36.4MB ± 0% 36.4MB ± 0% -0.09% (p=0.008 n=5+5) XML 47.9MB ± 0% 47.8MB ± 0% -0.15% (p=0.008 n=5+5) [Geo mean] 84.6MB 84.5MB -0.12% name old allocs/op new allocs/op delta Template 379k ± 0% 380k ± 0% +0.15% (p=0.008 n=5+5) Unicode 340k ± 0% 340k ± 0% ~ (p=0.738 n=5+5) GoTypes 1.36M ± 0% 1.36M ± 0% +0.05% (p=0.008 n=5+5) Compiler 5.49M ± 0% 5.49M ± 0% +0.12% (p=0.008 n=5+5) SSA 17.5M ± 0% 17.5M ± 0% -0.18% (p=0.008 n=5+5) Flate 235k ± 0% 235k ± 0% ~ (p=0.079 n=5+5) GoParser 302k ± 0% 302k ± 0% ~ (p=0.310 n=5+5) Reflect 976k ± 0% 977k ± 0% +0.08% (p=0.008 n=5+5) Tar 352k ± 0% 352k ± 0% +0.12% (p=0.008 n=5+5) XML 436k ± 0% 436k ± 0% -0.05% (p=0.008 n=5+5) [Geo mean] 842k 842k +0.03% Change-Id: I53e8faed1859885ca5c4a5d45067a50984f3eff1 Reviewed-on: https://go-review.googlesource.com/c/go/+/175879 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
-
Stefan Baebler authored
The TestParseErrors test function was not strict with unwanted errors received from url.Parse(). It was not failing in such cases, now it does Fixes #33646 Updates #29098 Change-Id: I069521093e2bff8b1fcd41ffd3f9799f3108bc61 GitHub-Last-Rev: e6844c57f979ddb8418643d9c5244a5d1b4578ba GitHub-Pull-Request: golang/go#33876 Reviewed-on: https://go-review.googlesource.com/c/go/+/191966 Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org>
-
Anderson Queiroz authored
Fixes golang/go#31249 Change-Id: I3280f8ab170ed31d4efb71106533e016d430d44c Reviewed-on: https://go-review.googlesource.com/c/go/+/191557Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Filippo Valsorda authored
It was mistakenly re-enabled in CL 146217. Fixes #33837 Change-Id: I8c0e1787114c6232df5888e51e355906622295bc Reviewed-on: https://go-review.googlesource.com/c/go/+/191877 Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
-
Bryan C. Mills authored
Updates #33879 Change-Id: Ifc91490b1cb791fdf5ffe69ef81c0ec0e6cbecc3 Reviewed-on: https://go-review.googlesource.com/c/go/+/191997 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Alexander Rakoczy <alex@golang.org>
-
Matthew Dempsky authored
Because the Node AST represents references to declared objects (e.g., variables, packages, types, constants) by directly pointing to the referred object, we don't have use-position info for these objects. For switch statements with duplicate cases, we report back where the first duplicate value appeared. However, due to the AST representation, if the value was a declared constant, we mistakenly reported the constant declaration position as the previous case position. This CL reports back against the 'case' keyword's position instead, if there's no more precise information available to us. It also refactors code to emit the same "previous at" error message for duplicate values in map literals. Thanks to Emmanuel Odeke for the test case. Fixes #33460. Change-Id: Iec69542ccd4aad594dde8df02d1b880a422c5622 Reviewed-on: https://go-review.googlesource.com/c/go/+/188901Reviewed-by: Robert Griesemer <gri@golang.org> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Matthew Dempsky authored
Use efaceOf to safely convert from *interface{} to *_eface, and to make it clearer what the pointer arithmetic is computing. Incidentally, remove a spurious unsafe.Pointer->*uint8->unsafe.Pointer round trip conversion in newproc. No behavior change. Change-Id: I2ad9d791d35d8bd008ef43b03dad1589713c5fd4 Reviewed-on: https://go-review.googlesource.com/c/go/+/190457 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
-
Bryan C. Mills authored
I'm trying to keep the code changes minimal for backporting to Go 1.13, so it is still possible for a handful of entries to leak, but the leaks are now O(1) instead of O(N) in the steady state. Longer-term, I think it would be a good idea to coalesce idleMu with connsPerHostMu and clear entries out of both queues as soon as their goroutines are done waiting. Fixes #33849 Fixes #33850 Change-Id: Ia66bc64671eb1014369f2d3a01debfc023b44281 Reviewed-on: https://go-review.googlesource.com/c/go/+/191964 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
LE Manh Cuong authored
The assembly output for x & c == c, where c is power of 2: MOVQ "".set+8(SP), AX ANDQ $8, AX CMPQ AX, $8 SETEQ "".~r2+24(SP) With optimization using bitset: MOVQ "".set+8(SP), AX BTL $3, AX SETCS "".~r2+24(SP) output less than 1 instruction. However, there is no speed improvement: name old time/op new time/op delta AllBitSet-8 0.35ns ± 0% 0.35ns ± 0% ~ (all equal) Fixes #31904 Change-Id: I5dca4e410bf45716ed2145e3473979ec997e35d4 Reviewed-on: https://go-review.googlesource.com/c/go/+/175957 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
-
Daniel Martí authored
The decoder called this function to check numbers being decoded into a json.Number. However, these can't be quoted as strings, so the tokenizer has already verified they are valid JSON numbers. Verified this by adding a test with such an input. As expected, it produces a syntax error, not the fmt.Errorf - that line could never execute. Since the only remaining non-test caller of isvalidnumber is in encode.go, move the function there. This change should slightly reduce the amount of work when decoding into json.Number, though that isn't very common nor part of any current benchmarks. Change-Id: I67a1723deb3d18d5b542d6dd35f3ae56a43f23eb Reviewed-on: https://go-review.googlesource.com/c/go/+/184817 Run-TryBot: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Dong-hee Na authored
There is an unnecessary lower operation in isJSType. Simple logic fix can improve tiny performance. name old time/op new time/op delta isJSType-8 152ns ± 0% 58ns ± 7% -61.82% (p=0.001 n=6+8) name old alloc/op new alloc/op delta isJSType-8 32.0B ± 0% 0.0B -100.00% (p=0.000 n=8+8) name old allocs/op new allocs/op delta isJSType-8 1.00 ± 0% 0.00 -100.00% (p=0.000 n=8+8) Change-Id: I281aadf1677d4377920c9649af206381189a27e6 Reviewed-on: https://go-review.googlesource.com/c/go/+/177118 Run-TryBot: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
-
Daniel Martí authored
This reverts https://golang.org/cl/185080. Reason for revert: some new changes are erroring again, so this broke the builders. Change-Id: I28da16da98b90cefbb47173d31bbbb56e43062d5 Reviewed-on: https://go-review.googlesource.com/c/go/+/191781 Run-TryBot: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
-
LE Manh Cuong authored
state and ssafn both have their own Fatalf, so use them instead of global Fatalf. Updates #19683 Change-Id: Ie02a961d4285ab0a3f3b8d889a5b498d926ed567 Reviewed-on: https://go-review.googlesource.com/c/go/+/188539 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
-
Daniel Martí authored
First, add cpu and memory profiling flags, as these are useful to see where rulegen is spending its time. It now takes many seconds to run on a recent laptop, so we have to keep an eye on what it's doing. Second, stop writing '_ = var' lines to keep imports and variables used at all times. Now that rulegen removes all such unused names, they're unnecessary. To perform the removal, lean on go/types to first detect what names are unused. We can configure it to give us all the type-checking errors in a file, so we can collect all "declared but not used" errors in a single pass. We then use astutil.Apply to remove the relevant nodes based on the line information from each unused error. This allows us to apply the changes without having to do extra parser+printer roundtrips to plaintext, which are far too expensive. We need to do multiple such passes, as removing an unused variable declaration might then make another declaration unused. Two passes are enough to clean every file at the moment, so add a limit of three passes for now to avoid eating cpu uncontrollably by accident. The resulting performance of the changes above is a ~30% loss across the table, since go/types is fairly expensive. The numbers were obtained with 'benchcmd Rulegen go run *.go', which involves compiling rulegen itself, but that seems reflective of how the program is used. name old time/op new time/op delta Rulegen 5.61s ± 0% 7.36s ± 0% +31.17% (p=0.016 n=5+4) name old user-time/op new user-time/op delta Rulegen 7.20s ± 1% 9.92s ± 1% +37.76% (p=0.016 n=5+4) name old sys-time/op new sys-time/op delta Rulegen 135ms ±19% 169ms ±17% +25.66% (p=0.032 n=5+5) name old peak-RSS-bytes new peak-RSS-bytes delta Rulegen 71.0MB ± 2% 85.6MB ± 2% +20.56% (p=0.008 n=5+5) We can live with a bit more resource usage, but the time/op getting close to 10s isn't good. To win that back, introduce concurrency in main.go. This further increases resource usage a bit, but the real time on this quad-core laptop is greatly reduced. The final benchstat is as follows: name old time/op new time/op delta Rulegen 5.61s ± 0% 3.97s ± 1% -29.26% (p=0.008 n=5+5) name old user-time/op new user-time/op delta Rulegen 7.20s ± 1% 13.91s ± 1% +93.09% (p=0.008 n=5+5) name old sys-time/op new sys-time/op delta Rulegen 135ms ±19% 269ms ± 9% +99.17% (p=0.008 n=5+5) name old peak-RSS-bytes new peak-RSS-bytes delta Rulegen 71.0MB ± 2% 226.3MB ± 1% +218.72% (p=0.008 n=5+5) It might be possible to reduce the cpu or memory usage in the future, such as configuring go/types to do less work, or taking shortcuts to avoid having to run it many times. For now, ~2x cpu and ~4x memory usage seems like a fair trade for a faster and better rulegen. Finally, we can remove the old code that tried to remove some unused variables in a hacky and unmaintainable way. Change-Id: Iff9e83e3f253babf5a1bd48cc993033b8550cee6 Reviewed-on: https://go-review.googlesource.com/c/go/+/189798 Run-TryBot: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
-
Andrew Gerrand authored
This removes a special case that was added to fix issue #10956, but that was never actually effective. The code in the test case still fails to read, so perhaps the zip64 support added in CL 6463050 inadvertently caught this particular case. It's possible that the original theorized bug still exists, but I'm not convinced it was ever fixed. Update #28700 Change-Id: I4854de616364510f64a6def30b308686563f8dbb Reviewed-on: https://go-review.googlesource.com/c/go/+/179757 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
tnclong authored
Template.New calls t.init, which allocates several items that are immediately rewritten by copy, so avoid the call to New Change-Id: I16c7cb001bbcd14cf547c1a2db2734a2f8214e7e Reviewed-on: https://go-review.googlesource.com/c/go/+/182757 Run-TryBot: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
-
Stefan Baebler authored
The TestParseErrors test function was not strict with unwanted errors received from url.Parse(). It was not failing in such cases, now it does. Change-Id: I18a26a68c1136f5c762989a76e04b47e33dd35f1 GitHub-Last-Rev: c33f9842f7908f27012859e25caa79388cc2785a GitHub-Pull-Request: golang/go#32954 Reviewed-on: https://go-review.googlesource.com/c/go/+/185080Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Alex Brainman authored
TransmitFile does not allow for number of bytes that can be transmitted to be larger than 2147483646. See https://docs.microsoft.com/en-us/windows/win32/api/mswsock/nf-mswsock-transmitfile for details. So adjust sendFile accordingly. No test added, because this would require creating large file (more than 2GB file). Fixes #33193. Change-Id: I82e0cb104d112264e4ea363bb20b6d02ac30b38e Reviewed-on: https://go-review.googlesource.com/c/go/+/187037 Run-TryBot: Alex Brainman <alex.brainman@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
-
Dong-hee Na authored
After Go.1.10+ strings.Builder is known as more efficient in concatenating and building strings than bytes.Buffer. In this CL, there is a minor logic fix for getting advantage of strings.builder. name old time/op new time/op delta DefinedTemplate-8 543ns ± 3% 512ns ± 2% -5.73% (p=0.000 n=8+8) name old alloc/op new alloc/op delta DefinedTemplate-8 192B ± 0% 160B ± 0% -16.67% (p=0.000 n=8+8) name old allocs/op new allocs/op delta DefinedTemplate-8 5.00 ± 0% 5.00 ± 0% ~ (all equal) Change-Id: Icda0054d146e6c5e32ed8a4d13221bb6850d31b4 Reviewed-on: https://go-review.googlesource.com/c/go/+/175261 Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Jonathan Amsterdam authored
Add brief descriptions of why one might implement an Is or As method. Fixes #33364. Change-Id: I81a091bf564c654ddb9ef3997e780451a01efb7a Reviewed-on: https://go-review.googlesource.com/c/go/+/191338Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Reviewed-by: Andrew Bonventre <andybons@golang.org> Run-TryBot: Jonathan Amsterdam <jba@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Daniel Martí authored
Because TestUnmarshal actually allocates a new value to decode into using ptr's pointer type, any existing data is thrown away. This was harmless in alomst all of the test cases, minus the "overwriting of data" ones added in 2015 in CL 12209. I spotted that nothing covered decoding a JSON array with few elements into a slice which already had many elements. I initially assumed that the code was buggy or that some code could be removed, when in fact there simply wasn't any code covering the edge case. Move those two tests to TestPrefilled, which already served a very similar purpose. Remove the map case, as TestPrefilled already has plenty of prefilled map cases. Moreover, we no longer reset an entire map when decoding, as per the godoc: To unmarshal a JSON object into a map, Unmarshal first establishes a map to use. If the map is nil, Unmarshal allocates a new map. Otherwise Unmarshal reuses the existing map, keeping existing entries. Finally, to ensure that ptr is used correctly in the future, make TestUnmarshal error if it's anything other than a pointer to a zero value. That is, the only correct use should be new(type). Don't rename the ptr field, as that would be extremely noisy and cause unwanted merge conflicts. Change-Id: I41e3ecfeae42d877ac5443a6bd622ac3d6c8120c Reviewed-on: https://go-review.googlesource.com/c/go/+/185738 Run-TryBot: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
-
Daniel Martí authored
rulegen.go produces plaintext Go code directly, which was fine for a while. However, that's started being a bottleneck for making code generation more complex, as we can only generate code directly one line at a time. Some workarounds were used, like multiple layers of buffers to generate chunks of code, to then use strings.Contains to see whether variables need to be defined or not. However, that's error-prone, verbose, and difficult to work with. A better approach is to generate an intermediate syntax tree in memory, which we can inspect and modify easily. For example, we could run a number of "passes" on the syntax tree before writing to disk, such as removing unused variables, simplifying logic, or moving declarations closer to their uses. This is the first step in that direction, without changing any of the generated code. We didn't use go/ast directly, as it's too complex for our needs. In particular, we only need a few kinds of simple statements, but we do want to support arbitrary expressions. As such, define a simple set of statement structs, and add thin layers for printer.Fprint and ast.Inspect. A nice side effect of this change, besides removing some buffers and string handling, is that we can now avoid passing so many parameters around. And, while we add over a hundred lines of code, the tricky pieces of code are now a bit simpler to follow. While at it, apply some cleanups, such as replacing isVariable with token.IsIdentifier, and consistently using log.Fatalf. Follow-up CLs will start improving the generated code, also simplifying the rulegen code itself. I've added some TODOs for the low-hanging fruit that I intend to work on right after. Updates #30810. Change-Id: Ic371c192b29c85dfc4a001be7fbcbeec85facc9d Reviewed-on: https://go-review.googlesource.com/c/go/+/177539 Run-TryBot: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
-
Javier Revillas authored
HTTP is an initialism, not an acronym, where you pronounce each letter as a word. It's "an H", not "a H". Running `find src/net/http -type f | xargs grep -n 'an HTTP' | wc -l` shows that the "an HTTP" form is used 67 times across the `net/http` package. Furthermore, `find src/net/http -type f | xargs grep -n 'a HTTP' | wc -l` yields only 4 results. Change-Id: I219c292a9e2c9bf7a009dbfe82ea8b15874685e9 GitHub-Last-Rev: 6ebd095023af47444b6b0fc5b6d7b26d85f4c7b7 GitHub-Pull-Request: golang/go#33810 Reviewed-on: https://go-review.googlesource.com/c/go/+/191700Reviewed-by: Toshihiro Shiino <shiino.toshihiro@gmail.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
LE Manh Cuong authored
After golang.org/cl/33895, function adjustctx can not be inlined, cost 82 exceeds budget 80 Change-Id: Ie559ed80ea2c251add940a99f11b2983f6cbddbc Reviewed-on: https://go-review.googlesource.com/c/go/+/187977 Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
-
zdjones authored
Array accesses with index types smaller than the machine word size may involve a sign or zero extension of the index value before bounds checking. Currently, this defeats prove because the facts about the original index value don't flow through the sign/zero extension. This CL fixes this by looking back through value-preserving sign/zero extensions when adding facts via Update and, where appropriate, applying the same facts using the pre-extension value. This fix is enhanced by also looking back through value-preserving extensions within ft.isNonNegative to infer whether the extended value is known to be non-negative. Without this additional isNonNegative enhancement, this logic is rendered significantly less effective by the limitation discussed in the next paragraph. In Update, the application of facts to pre-extension values is limited to cases where the domain of the new fact is consistent with the type of the pre-extension value. There may be cases where this cross-domain passing of facts is valid, but distinguishing them from the invalid cases is difficult for me to reason about and to implement. Assessing which cases to allow requires details about the context and inferences behind the fact being applied which are not available within Update. Additional difficulty arises from the fact that the SSA does not curently differentiate extensions added by the compiler for indexing operations, extensions added by the compiler for implicit conversions, or explicit extensions from the source. Examples of some cases that would need to be filtered correctly for cross-domain facts: (1) A uint8 is zero-extended to int for indexing (a value-preserving zeroExt). When, if ever, can signed domain facts learned about the int be applied to the uint8? (2) An int8 is sign-extended to int16 (value-preserving) for an equality comparison. Equality comparison facts are currently always learned in both the signed and unsigned domains. When, if ever, can the unsigned facts learned about the int16, from the int16 != int16 comparison, be applied to the original int8? This is an alternative to CL 122695 and CL 174309. Compared to CL 122695, this CL differs in that the facts added about the pre-extension value will pass through the Update method, where additional inferences are processed (e.g. fence-post implications, see #29964). CL 174309 is limited to bounds checks, so is narrower in application, and makes the code harder to read. Fixes #26292. Fixes #29964. Fixes #15074 Removes 238 bounds checks from std/cmd. Change-Id: I1f87c32ee672bfb8be397b27eab7a4c2f304893f Reviewed-on: https://go-review.googlesource.com/c/go/+/174704 Run-TryBot: Zach Jones <zachj1@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Giovanni Bajo <rasky@develer.com>
-
Sergei Zagurskii authored
directlyAssignable invoked rtype.Name() just to compare its result to empty string. We really only need to check whether rtype has name. It can be done much cheaper, by checking tflagNamed. Benchmark: https://play.golang.org/p/V2BzESPuf2w name old time/op new time/op delta DirectlyAssignable-12 32.7ns ± 6% 6.6ns ± 6% -79.80% (p=0.008 n=5+5) Fixes #32186 Change-Id: I1a2a167dbfddf319fba3015cb6a011bf010f99a8 Reviewed-on: https://go-review.googlesource.com/c/go/+/178518 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-
Robert Griesemer authored
Don't skip closing parentheses of any kind after a missing expression. They are likely part of the lexical construct enclosing the expression. Fixes #33386. Change-Id: Ic0abc2037ec339a345ec357ccc724b7ad2a64c00 Reviewed-on: https://go-review.googlesource.com/c/go/+/188502Reviewed-by: Matthew Dempsky <mdempsky@google.com>
-
Jason A. Donenfeld authored
Some constants were added above the initial copyright blurb, and then later a new copyright blurb was added on top of that. So we wound up with two header sections, one of which contained a useful comment that became obscured. This commit fixes up that mistake. Change-Id: I8b9b8c34495cdceae959e151e8ccdee3137f6ca4 Reviewed-on: https://go-review.googlesource.com/c/go/+/191841 Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
-
- 26 Aug, 2019 7 commits
-
-
Matthew Dempsky authored
Quietly drop duplicate methods inherited from embedded interfaces if they have an identical signature to existing methods. Updates #6977. Change-Id: I144151cb7d99695f12b555c0db56207993c56284 Reviewed-on: https://go-review.googlesource.com/c/go/+/187519 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
-
Matthew Dempsky authored
Move checkdupfields call into expandiface, and inline/simplify offmod. More prep work for implementing #6977. Change-Id: I958ae87f761ec25a8fa7298a2a3019eeca5b25ba Reviewed-on: https://go-review.googlesource.com/c/go/+/187518Reviewed-by: Robert Griesemer <gri@golang.org>
-
Matthew Dempsky authored
Allows avoiding the Type.Fields call, which affects prevents checkdupfields from being called at the more natural point during dowidth. Change-Id: I724789c860e7fffba1e8e876e2d74dcfba85d75c Reviewed-on: https://go-review.googlesource.com/c/go/+/187517Reviewed-by: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
obei authored
Updates #33738 Change-Id: If0856d7c57ecfde08341c1aecb5e92361fd64f2b Reviewed-on: https://go-review.googlesource.com/c/go/+/191217Reviewed-by: Andrew Bonventre <andybons@golang.org> Run-TryBot: Andrew Bonventre <andybons@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
-
Katie Hockman authored
Fixes #33796 Change-Id: I9f6837be96410e96d004523c48bef95ee1427484 Reviewed-on: https://go-review.googlesource.com/c/go/+/191746Reviewed-by: Bryan C. Mills <bcmills@google.com>
-
Robert Griesemer authored
The stack of delayed actions is grown by pushing a new action on top with Checker.later. Checker.processDelayed processes all actions above a top watermark and then resets the stack to top. Until now, pushed actions above the watermark were processed in LIFO order. This change processes them in FIFO order, which seems more natural (if an action A was delayed before an action B, A should be processed before B for that stack segment). (With this change, Checker.later could be used instead of Checker.atEnd to postpone interface method type comparison and then the specific example in issue #33656 does type-check. However, in general we want interface method type comparisons to run after all interfaces are completed. With Checker.later we may still end up mixing interface completions and interface method type comparisons in ways leading to other errors for sufficiently convoluted code.) Also, move Checker.processDelayed from resolver.go to check.go. Change-Id: Id31254605e6944c490eab410553fff907630cc64 Reviewed-on: https://go-review.googlesource.com/c/go/+/191458Reviewed-by: Matthew Dempsky <mdempsky@google.com>
-
Robert Griesemer authored
Updates #6977. Change-Id: I6eda4be550e7c7ea1e1bac3222850002d90a81a8 Reviewed-on: https://go-review.googlesource.com/c/go/+/190378Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
-