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: add const literals for reference types like structs, maps, and arrays #21130

Open
ghasemloo opened this issue Jul 23, 2017 · 17 comments
Labels
LanguageChange NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal v2 A language change or incompatible library change
Milestone

Comments

@ghasemloo
Copy link

ghasemloo commented Jul 23, 2017

Summary:
I propose adding const literals for reference types. E.g.

const c = []string{"1", "2", "3"}

Background:
Having const structs and arrays and maps is very useful to make sure the value is not changed. This is specially for tests where you want to make sure the value you are passing to functions, the only way right now is to make a copy of the object explicitly. When passing a const to a function or over a chan or assigning it to a variable Go would make a copy.

Proposal:
Allow definition of constants of reference types like structs, maps, and arrays.

Impact:
No impact on existing code.

Discussion:
This can simplifies code.

Alternatives:

Alternative 1: use var definitions, do explicitly copy
Without a built-in DeepCopy standard function the developer has to write their own copy functions for each type. It is also likely to break if the language adds new basic types in future.
But even if we had a DeepCopy standard function one cannot be sure that some other part of code is not changing the value of the variable. This gets worse when the const has to be exported or when the packages consists of multiple files.

var c = []string{"1", "2", "3"}
.
.
.
v := cCopy(c)

Alternative 2: generator function
We can write functions that create the struct and return it. E.g.

func genC() []string { return []string{"1", "2", "3"} }
.
.
.
v := genC(c)

This leads to code with lots of functions just for creating values.

@gopherbot gopherbot added this to the Proposal milestone Jul 23, 2017
@bradleyfalzon
Copy link

May be related to or dupe of #20443?

@bradfitz bradfitz added v2 A language change or incompatible library change LanguageChange labels Jul 23, 2017
@ghasemloo
Copy link
Author

ghasemloo commented Jul 23, 2017

Thanks, it is related. However the main case I have is structs specially with large nested protobufs.

@ericlagergren
Copy link
Contributor

@ghasemloo I think the difference between your proposal and #20443 is yours is a blanket, "do not modify this in any way" sort of const?

@kevinburke kevinburke marked this as a duplicate of #21131 Jul 24, 2017
@ghasemloo ghasemloo changed the title Proposal: add const types (Go2) Proposal: add const literals for reference types like structs, maps, and arrays (Go2) Jul 24, 2017
@ghasemloo
Copy link
Author

It seems to me that #21131 is about const map and slice types, not const literals.

@ianlancetaylor ianlancetaylor changed the title Proposal: add const literals for reference types like structs, maps, and arrays (Go2) proposal: Go 2: add const literals for reference types like structs, maps, and arrays Feb 27, 2018
@ianlancetaylor
Copy link
Contributor

This needs more explanation. What are you allowed to put in a const struct literal? Are the elements only permitted to be constant expressions? Can they include an address operation? Can they include a slice? Does the slice then become const? Can you use a const struct literal with fields of interface type? You mentioned only structs, maps, and arrays, but what about interface and pointer types?

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 27, 2018
@ghasemloo
Copy link
Author

For the use-cases I have seen they are mostly other const expressions. E.g. I want to define a package-level struct var that I use in tests, I don't want that to be modified between tests. But to cover this case completely it I think it needs to allow pointers, functions, slices, etc.

I am guessing the question is: is any part of this const struct mutable? For the common use case that motivated this proposal the answer is no (so it can be implemented as a typed fixed-size fixed-content block of memory that never gets modified).

@ianlancetaylor
Copy link
Contributor

If you want this proposal to be adopted it needs to be precise. Right now it is too vague.

@ghasemloo
Copy link
Author

Yep, agreed. I can expand this but after seeing #23157 I think that would cover the use cases so maybe we should just close this one?

@bcmills
Copy link
Contributor

bcmills commented Mar 3, 2018

I would propose:

  • Structs, arrays, slices, maps, pointers, and interfaces can be constants.
  • Constants must comprise only other constants.
    • Fields of constant structs, elements of constant arrays and slices, keys and elements of constant maps, concrete values of constant interfaces, and indirections of constant pointers are themselves constants.
  • Constants are not addressable, transitively.
    • Constant structs are not addressable, and neither are their fields.
    • Elements of constant arrays and slices are not addressable.
    • The method set of a constant includes only the methods with value receivers.
      • The type of the constant may define methods with pointer receivers, but in order to call them the constant must be copied to a variable.
  • Constant values of reference types (slices, maps, pointers, and interfaces, and structs with fields of any of those types) cannot be assigned to variables or passed as function or method arguments, except as the second argument to the copy and append built-ins.
    • This restriction makes it possible to use local information to determine whether a given expression refers to a constant.
    • Unfortunately, it also makes constants relatively useless for composite types with any sort of nesting.
  • range loops over constants can declare the index and element as constants:
    for const k, v = range mapConstant {
    	…
    }
    (This seems to require proposal: spec: redefine range loop variables in each iteration #20733 for consistency.)

I think that would cover a range of use-cases, such as composite keys in maps, while limiting interactions with the rest of the type system and language spec. It's enough of an expansion to be useful (for example, in #20757), but limited enough to be something we could plausibly implement in a release cycle.

@ianlancetaylor
Copy link
Contributor

Thanks for writing that up.

It's contradictory on slices, though: you say both that slices can be constants and that they cannot be constants.

It does seem possible that in many real life uses people will want to be able to write constant slices and constant pointers. But it sort of depends on what this is for and why people will use it. @ghasemloo specifically says it is for use with large nested protobufs; for that use case clearly pointers and slices are required.

That said, I don't really understand the advantage of using this with large nested protobufs. The original message says this can simplify code, but I honestly don't see why. The change from writing var to const is not a meaningful simplification.

At the moment the only purpose I can see is avoiding bugs. What kinds of bugs happen in practice that this proposal would avoid? Or is there some other use that I am not seeing?

@bcmills
Copy link
Contributor

bcmills commented Mar 7, 2018

It's contradictory on slices, though: you say both that slices can be constants and that they cannot be constants.

Oops, that's a typo. I had meant to write that slices cannot be constants (under this version of the proposal), since I'm not proposing any changes to addressability.

But on further thought, that restriction isn't necessary; instead, we need a restriction to avoid leaking constant values of reference types into variables. I've updated the proposal above accordingly.

I agree that that seems to make constants unsuitable for all but the simplest protobufs. The things I want to use it for are simpler: time.Duration, structs that implement error, and similar value-types that may be structs. (Honestly, for my use-cases, widening const to structs alone — and omitting reference types entirely — would probably suffice.)

@agnivade
Copy link
Contributor

I would like to have constant errors and slices at the very least.

Real world use cases -

  1. errors

I really wish I don't have to write this (https://github.com/agnivade/funnel/blob/master/config.go#L29) inside a var()

ErrInvalidFileRenamePolicy = errors.New(FileRenamePolicy + " can only be timestamp or serial")
ErrInvalidMaxAge = errors.New(MaxAge + " must end with either d or h and start with a number")

I have tons of errors like this in my non-open source code.

  1. slices

Every time I need a static encryption key, I have to use a var for a byte slice. Consider the gorilla/csrf package. Unless you really want to write the key shoved in the code like that. You have to use a var.

@ghasemloo
Copy link
Author

ghasemloo commented Apr 8, 2018

@ianlancetaylor it simplifies code because if we have const we would not need to make a copy when passing it to functions without worrying about their modification.

The common use-case I repeatedly see is in tests. We have a value that we use in many tests. It is a big struct. We define them as package level vars in the test file. But how can we ensure that the value of the var doesn't change between tests? Over the last year this has caused bugs in code and tests more than few times.

Most often people just ignore the issue and assume that tests will not modify these global variables. A common solution is to make a deep copy, but Go doesn't have a deep copy builtin and proto.Clone doesn't work with general structs, only with protos. Another alternative I have seen is to define a generator function that returns a copy for each of these but when you have a couple of these defining those functions becomes less appealing.

@davecheney
Copy link
Contributor

@agnivade

I really wish I don't have to write this

You don't have to.

https://dave.cheney.net/2016/04/07/constant-errors

@agnivade
Copy link
Contributor

agnivade commented Apr 8, 2018

Not bad 👍 . My request still stands though. I would want the error type to be allowed as a constant from the Go spec itself.

@networkimprov
Copy link

networkimprov commented Aug 7, 2018

@bcmills,

Constant values of reference types (slices, maps, pointers, and interfaces, and structs with fields of any of those types) cannot be assigned to variables or passed as function or method arguments

Tho not explicitly addressable, composite const literals could be passed to functions by reference implicitly (as maps are now). That probably requires #20443 (const function argument). I think passing const widgets to functions is rather essential.

Also a constant's address could initialize pointer and interface constants, as you implied.
const ( c = 1; p = &c; i interface{} = &c )

Regarding assignment, couldn't const c = T{...}; v := c or func f(v T){ }(c) invoke the same procedure as v := T{...} ? A composite literal is a template, and when needed repeatedly, it's sensible to make the template a const.

Alternatively, a first implementation could disallow const literals which don't occupy contiguous memory, allowing v := c to invoke memcpy(v, c, sizeof c). So const arrays are OK, but not non-empty slices & maps. Pointers and interface elements should be OK since in both cases the pointer refers to another constant.

@networkimprov
Copy link

Discussion of this has resumed in #6386.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LanguageChange NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests

10 participants