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

Empty map can be assigned to #4

Open
johnsiilver opened this issue Jul 3, 2017 · 3 comments
Open

Empty map can be assigned to #4

johnsiilver opened this issue Jul 3, 2017 · 3 comments

Comments

@johnsiilver
Copy link

Looks like an empty map that is frozen does not get protection. You can alter it as much as you want.

Code showing incorrect behavior:

package main

import "github.com/lukechampine/freeze"

func main() {
        tr := map[string]int{}
        tr = freeze.Map(tr).(map[string]int)
        tr["yo"] = 3
}

Code showing correct behavior:

package main

import "github.com/lukechampine/freeze"

func main() {
        tr := map[string]int{"hello": 1}
        tr = freeze.Map(tr).(map[string]int)
        tr["yo"] = 3
}
@lukechampine
Copy link
Owner

good catch. I believe this is due to the same resizing behavior that affects slices. From the README:

Appending to a frozen slice will trigger a panic iff len(slice) < cap(slice). This is because appending to a full slice will allocate new memory.

In theory, then, this behavior should be observed whenever inserting into a map causes it to resize. So it's more insidious than simply breaking on empty maps.

There's probably some other field of the map that can be frozen to restore the desired behavior. But maps are annoying because their memory layout isn't nearly as stable as slices -- it changes in nearly every minor release. Worse, this code assumes that you're using the standard Go compiler (instead of, e.g. gccgo). The same issue affects my randmap package.

The best we can probably do is update the implementation for each new Go release, and guard the various implementations with build tags. Unfortunately I am rather busy right now so I'm not sure when I'll get around to it.

@johnsiilver
Copy link
Author

johnsiilver commented Jul 3, 2017 via email

@lukechampine
Copy link
Owner

lukechampine commented Jul 3, 2017

Ah, I'm glad to hear that someone found a real use for this package. And that it's being used properly (for testing only) rather than running in production code or something...

There have been proposals to add a "const"-like keyword to Go, but they haven't gone anywhere. For example, there was a read-only slices proposal, but it was found to bifurcate the language APIs too much; see this response.

As for static analysis, you can likely do a decent job, but not a perfect one. For example, whenever you use an interface:

func writeData(w io.Writer, data []byte) {
    w.Write(data)
}

It's difficult (maybe impossible) for a static analysis tool to determine whether w.Write mutates data. In fact, this particular example frequently impedes optimization: since the compiler can't determine whether w retains a pointer to data, it is forced to keep data on the heap. Whereas if you call a method directly, e.g.:

f, _ := os.Open("foo")
f.Write(data)

The compiler knows that the *os.File implementation of Write doesn't retain a pointer, so it can allocate on the heap and skip garbage collection. You can see all this by compiling a simple program with -gcflags -m.

Anyway, I think such a tool would be simple in theory. You just need a list of mutation syntaxes, like slice[x] = y, map[x] = y, *x = y, etc. Then you check each function (and the functions it calls, recursively) for occurrences of those syntax patterns. For the reason given above, you can't always get a definite answer, but your tool could perhaps output something like:

- func foo: does not mutate any arguments
- func bar:
    - mutates argument x
    - does not mutate argument y
- func quux:
    - cannot prove that argument x is not mutated
        - reason: interface call `w.Write` (line 455)

Then the user could optionally modify quux to not use an interface or something.
Sounds like a fun project!

btw, you may be interested in ply, my attempt at creating a superset of Go that supports "generics" (not really).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants