-
Notifications
You must be signed in to change notification settings - Fork 18.3k
Open
Labels
LanguageChangeSuggested changes to the Go languageSuggested changes to the Go languageLanguageChangeReviewDiscussed by language change review committeeDiscussed by language change review committeeProposal
Milestone
Description
The non-addressability requirement of map index expressions makes sense by itself since we don't want a surviving pointer to a map element. But surely it should be ok to have an invisible temporary pointer p to the respective field which is just used during the assignment (*p = x). It seems that this doesn't introduce any more issues than what we have already when assigning to a larger map element that cannot be assigned to atomically. E.g., the following program should be legal: package main import "fmt" var m = map[string]struct{x, y int} { "foo": {2, 3}, } func main() { m["foo"].x = 4 // cannot assign to m["foo"].x fmt.Println(m) }
nathany, mingrammer, ajruckman, fakegeass, binderclip and 43 more
Metadata
Metadata
Assignees
Labels
LanguageChangeSuggested changes to the Go languageSuggested changes to the Go languageLanguageChangeReviewDiscussed by language change review committeeDiscussed by language change review committeeProposal
Type
Projects
Milestone
Relationships
Development
Select code repository
Activity
gopherbot commentedon Dec 11, 2012
Comment 1 by Robert.nr1:
griesemer commentedon Dec 11, 2012
Comment 2:
gopherbot commentedon Dec 11, 2012
Comment 3 by Robert.nr1:
ianlancetaylor commentedon Dec 11, 2012
Comment 4:
extemporalgenome commentedon Dec 11, 2012
Comment 5:
rsc commentedon Mar 12, 2013
Comment 6:
rsc commentedon Nov 27, 2013
Comment 7:
Labels changed: added go1.3maybe.
rsc commentedon Dec 4, 2013
Comment 8:
Labels changed: added release-none, removed go1.3maybe.
rsc commentedon Dec 4, 2013
Comment 9:
Labels changed: added repo-main.
48 remaining items
seebs commentedon Oct 6, 2020
Wow, this is surprisingly hard to phrase cleanly.
I think the goal is: Given any valid expression that includes a map index and would otherwise be addressable, we want to be able to use it in assignments, increments, and so on, as if it were addressable. One of the challenges is that a simplistic description of this might imply that this should apply to expressions which wouldn't be addressable for other reasons not involving the map index. (Like, if you have a map of function-returning-int, we don't want
m[k]()++
to be allowed, even thoughm[k]()
is a reasonable expression.)Thought experiment: What about pointer receiver methods?
If you don't allow pointer-receiver methods, it's Annoying. If you do, you have opened the floodgates and made the value act as though it's addressable.
Possible resolution: If you do a thing to a map-index expression that would require it to be addressable, the thing happens on an addressable copy, and then the contents of that copy are assigned back into the map. If you stashed a copy of the address and then later tried to use it, this is considered Own Fault.
But... That feels like a sneaky gotcha.
So I think the right answer may be "still don't allow pointer-receiver methods". In which case... it's tricky to express the thing that is being allowed. Assignment, increment, etc, say that the thing must be addressable or a map index expression. Go doesn't really have a codified distinction between "lvalue" and "addressable", which is what I think is at issue.
Hmm. Given
m[k].sel
, wherem[k]
is a map index expression, and.sel
is any combination of selectors or index expressions, the result can be used in assignment or increment operations, and works as thoughm[k]
were extracted to a temporary variable and then stored back.Except that the map index expression doesn't get evaluated twice.
mdempsky commentedon Oct 6, 2020
No, we don't want to allow users to create pointers to the variables within a map. If we allowed that, then we might as well just make map indexing addressable. In your example, neither
m[0].Inc()
norm[0].Stash()
should be allowed.The Go spec already states "Each left-hand side operand must be addressable, a map index expression, or (for = assignments only) the blank identifier." We probably just need to extend "a map index expression" here similarly to how we already define addressability: "The operand must be addressable, that is, either a variable, pointer indirection, or slice indexing operation; or a field selector of an addressable struct operand; or an array indexing operation of an addressable array."
We would need to define somewhere what happens when assigning to just a sub-variable within a map somewhere. Probably the easiest way would be to just define that when assigning to a key not already in the map, a corresponding value is first zero-initialized just before the assignment.
There's also nothing special about
++
.m[0].X++
is defines as meaning the same thing asm[0].X += 1
, which in turn means the same asm[0].X = m[0].X + 1
. If we allow assigning tom[0].X
, thenm[0].X += 1
andm[0].X++
are implicitly allowed too (sincem[0].X
as a value expression is already valid).seebs commentedon Oct 6, 2020
I think the behavior with uninitialized values automatically falls out of the way a map yields a zero value when you access a nonexistent key.
I agree that pointer receivers are a bad idea, but they don't necessarily strictly imply making the content of the map addressable; they might imply making addressable copies of the content.
I'm not actually sure what I expect to "really" happen under the hood from something like
m[0].X++
; I would not be surprised if it resulted in exactly what currently happens when we write the code explicitly, which is that the entire contents of m[0] are copied, X is modified in the copy, and then the entire copy is written back. On the other hand, if it could arrange to only have to copy (and write back) that individual value, that would be a potentially significant performance win in some cases.theanine commentedon May 4, 2023
I think it would also be useful to improve the error messages around these situations so it's clear what the problem is.
./main.go:14:2: cannot assign to struct field f[id].bar in map
^ useful error message, understandable how to fix the issue.
./main.go:14:2: cannot assign to f[id].bar[0] (value of type time.Time)
^ confusing error message, as the issue is related to the assignment to the map value and not a type issue.
gopherbot commentedon May 5, 2023
Change https://go.dev/cl/492875 mentions this issue:
go/types, types2: better error message for bad assignment
go/types, types2: better error message for bad assignment
jieqiyue commentedon Aug 14, 2024