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

proposal: Go 2: language: make slices of constant strings constant if the indexes are constant #28591

Open
ainar-g opened this issue Nov 4, 2018 · 17 comments
Labels
LanguageChange Proposal Proposal-Hold v2 A language change or incompatible library change
Milestone

Comments

@ainar-g
Copy link
Contributor

ainar-g commented Nov 4, 2018

This will probably be assigned a Go2 label, although I think that it could be done during Go 1.

Consider multiline string constants:

const multiline = `
This is a long text
that spans multiple lines
and then ends.
`

If you feed this text to a parser and it finds a problem on the first line ("This is a long text"), it will actually report that the error is on the second line because of the leading newline. One might think that this is easily solved by slicing the string:

const multiline = `
This is a long text
that spans multiple lines
and then ends.
`[1:]

But this won't compile. The Spec currently says:

(…) For untyped string operands the result is a non-constant value of type string. (…)

Why not make it constant? After all, len(multiline) is constant.


There are other solutions to this. One could just use multiline[1:] everywhere where one doesn't want the leading newline. Or start "This is a long text" on the first line, but that hurts readability and "editability". Another solution is to make multiline a variable. The use case is pretty obscure and there isn't an immediate need for this, so this proposal is more about a question, why are slices (and indexing!) of constant strings not constant in the first place?

@gopherbot gopherbot added this to the Proposal milestone Nov 4, 2018
@mvdan mvdan changed the title proposal: make slices of constant strings constant if the indexes are constant proposal: spec: make slices of constant strings constant if the indexes are constant Nov 5, 2018
@ianlancetaylor ianlancetaylor changed the title proposal: spec: make slices of constant strings constant if the indexes are constant proposal: Go 2: language: make slices of constant strings constant if the indexes are constant Nov 5, 2018
@ianlancetaylor ianlancetaylor added the v2 A language change or incompatible library change label Nov 5, 2018
@ianlancetaylor
Copy link
Contributor

Marking as Go 2 because we've decided that all language changes are Go 2.

I know that this has been discussed in the past but I don't remember why it wasn't changed. Maybe simply because it rarely comes up in practice.

@ianlancetaylor
Copy link
Contributor

We should also consider accepting

const c = "abc"[1]

@ianlancetaylor ianlancetaylor added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Dec 11, 2018
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 16, 2019
@gopherbot gopherbot added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 3, 2019
@griesemer
Copy link
Contributor

griesemer commented Jan 29, 2020

It turns out that this proposed change is not backward-compatible after all ...

Consider "A"[0] : With the proposed change, this expression evaluates to the untyped byte constant 65. But if we try to negate this value, as in:

var _ = -"A"[0]

we will get an error: -"A"[0] (constant -65 of type byte) overflows byte . This is currently (without the change) valid code. Oops!

For similar reasons, constant index expressions are not backwards-compatible either. The length of a constant slice expression len("abc"[0:2]) is an untyped integer constant (3), but if we negate it and try to assign it to a variable of uint type, we will get an error.

var _ uint = -uint(len("abc"[0:2]))

(Without the change, these values are not constant, and the compiler won't complain.)

Fun times!

@griesemer
Copy link
Contributor

Related: #11370, #11368.

@jimmyfrasche
Copy link
Member

Would

const (
  a = "abc"[iota]
  b
  c
)

be legal?

@griesemer
Copy link
Contributor

@jimmyfrasche Very nice! Indeed it would.

@thepudds
Copy link
Contributor

thepudds commented Jan 29, 2020

It turns out that this proposed change is not backward-compatible after all ...

@griesemer FWIW, my most immediate reaction would be to suggest postponing this from Go 1.15 if that is indeed that case (for most of the reasons discussed in the related golang-dev thread, including your comment here). This change does not seem worth triggering the first use of the go version directive (if that is what would need to happen).

edit: sorry, mangled link.

@gopherbot
Copy link

Change https://golang.org/cl/216977 mentions this issue: go/types: constant string index and slice expressions are constant

@gopherbot
Copy link

Change https://golang.org/cl/217119 mentions this issue: spec: constant string index and slice expressions return a constant

@bcmills
Copy link
Contributor

bcmills commented Feb 3, 2020

@thepudds

This change does not seem worth triggering the first use of the go version directive (if that is what would need to happen).

To the contrary! A minor, mostly-backward-compatible change seems like a great way to try out the mechanism without causing too much churn if it doesn't go as well as we expect.

@griesemer
Copy link
Contributor

We discussed this issue in the proposal review committee. Since this proposal is not backward-compatible and we put a premium on language stability, we decided to not proceed with this for Go 1.15.

We discussed introducing a vet check instead, but it doesn't seem warranted since this change is essentially language fine-tuning - a "nice to have" if it were backward-compatible, but not important.

I will leave this open for future consideration, when non-backward compatible changes become viable due to universal use of modules.

@griesemer griesemer modified the milestones: Proposal, Backlog Feb 6, 2020
@griesemer griesemer added Proposal-Hold and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Feb 6, 2020
@alandonovan
Copy link
Contributor

The degree to which a strict reading of backward compatibility leaves little room to maneuver is constantly surprising.

@TapirLiu
Copy link
Contributor

TapirLiu commented Mar 22, 2020

One more case:

_ = 1/len(""[:])

Currently it compiles, but if len(""[:]) returns constant 0, it will not.

@rsc
Copy link
Contributor

rsc commented May 6, 2020

This was proposed for Go 1.15 (https://blog.golang.org/go1.15-proposals) but declined because it is not backwards compatible. Leaving open for some future breaking change (maybe).

@rsc rsc added this to Accepted in Proposals (old) May 6, 2020
@rsc rsc moved this from Accepted to Declined in Proposals (old) May 6, 2020
@go101
Copy link

go101 commented Sep 23, 2020

Another incompatible case:

package main

const s = "Go101.org"
// len(s) == 9
// 1 << 9 == 512
// 512 / 128 == 4

var a byte = 1 << len(s) / 128
var b byte = 1 << len(s[:]) / 128

func main() {
	println(a, b) // 4 0
}

It will print 4 4 if this proposal is adpoted.

@alandonovan
Copy link
Contributor

@go101 That's diabolical.

Spoiler: the difference arises from the top-down effect of the enclosing type (byte) on the shift operator when its right operand is non-constant. Thus 1 << len(s[:]) is truncated to byte. The type rules for shifts are the most complex of all...

@adonovan
Copy link
Member

adonovan commented Jan 25, 2024

Given the new extended forward compatibility mechanism, this breaking change could be implemented in files that set a sufficiently high minimum Go version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LanguageChange Proposal Proposal-Hold v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests