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

cmd/compile: a[1.0 << s] should be accepted #21693

Closed
griesemer opened this issue Aug 30, 2017 · 5 comments

Comments

Projects
None yet
5 participants
@griesemer
Copy link
Contributor

commented Aug 30, 2017

Per the fix for #14844, https://play.golang.org/p/fWbHLgkJ7Z should now be accepted by the compiler.

@griesemer griesemer added this to the Go1.10 milestone Aug 30, 2017

@griesemer griesemer self-assigned this Aug 30, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Aug 31, 2017

Change https://golang.org/cl/60230 mentions this issue: spec: clarify context type for certain non-constant shifts

gopherbot pushed a commit that referenced this issue Sep 1, 2017

spec: clarify context type for certain non-constant shifts
The spec is not conclusive about whether a non-constant shift of
certain untyped constant left operands is valid when the shift
expression appears as an index in an index or slice expression,
or as a size in a `make` function call.

Despite identical spec rules in all these cases, cmd/compile accepts

	make([]byte, 1.0 << s)

but pronounces an error for

	a[1.0 << s]

(go/types accepts both).

This change clarifies the spec by explicitly stating that an
untyped constant left operand in a non-constant shift (1.0 in
the above examples) will be given type `int` in these contexts.

A separate issue #21693 addresses the cmd/compile bug.

Fixes #14844.

Change-Id: I4b52125e487a607fae377fcbed55463cdce9836c
Reviewed-on: https://go-review.googlesource.com/60230
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@bradfitz

This comment has been minimized.

Copy link
Member

commented Nov 29, 2017

Ping Robert.

I assume this is a release-blocker, given the spec update?

@griesemer

This comment has been minimized.

Copy link
Contributor Author

commented Nov 29, 2017

Will look into this tomorrow. It's not spec compliant anymore but I don't think it's a release blocker - after all there's a trivial work around (just write a[1 << s] instead). Leaving as is for now in case there's an easy fix.

@odeke-em

This comment has been minimized.

Copy link
Member

commented Nov 29, 2017

I had started working on this at some ungodly hours of Saturday morning but I couldn't get a proper cast/conversion to int in

if t != nil && !t.IsInteger() {

so I'll defer to you @griesemer :)

@mdempsky mdempsky modified the milestones: Go1.10, Go1.11 Nov 29, 2017

@gopherbot

This comment has been minimized.

Copy link

commented Nov 30, 2017

Change https://golang.org/cl/81277 mentions this issue: cmd/compile: permit indices of certain non-constant shifts

@gopherbot gopherbot closed this in 088a9ad Dec 1, 2017

@golang golang locked and limited conversation to collaborators Dec 1, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.