Immutability

[tmssngr]

IMHO Go lacks an important feature: making clear which functions are allowed to modify what. I would greatly appreciate it if the caller of a function/method would make it obvious which parameter might be modified by the callee. In my (Java) projects from time to time the last parameter is a "sink" callback to which the callee outputs some values. However, I can't express that in the language.

	public static void detectLiveness(IRInstruction instruction, boolean alsoForGlobal, Set<IRVar> uses, Set<IRVar> defines) {
		IRUtils.getVars(instruction,
		                var -> add(var, alsoForGlobal, uses),
		                var -> add(var, alsoForGlobal, defines));
	}

	private static void add(IRVar var, boolean alsoForGlobal, Set<IRVar> uses) {
		if (alsoForGlobal || var.scope() != VariableScope.global) {
			uses.add(var);
		}
	}

I suggest to use mut as keyboard for the caller and function definition where a parameter might be modified. Otherwise the compiler should reject to compile:

fn detect_var_liveness(instruction: IRInstruction, also_for_global: bool, uses: mut Set<IRVar>, defines: mut Set<IRVar>) {
	...
	add(var, also_for_global, mut uses) // would fail to compile without `mut` here or if `uses` would have defined without `mut`
	add(var, also_for_global, mut defines)
	...
}

fn add(var: IRVar, also_for_global: bool, uses: mut Set<IRVar>) {
	...
}

Though this would not make any difference in the resulting Go code, it would make it obvious while writing/reading the code which parameter can and which can't be modified by the called function. What do you think?

[tmssngr]

Currently, this code

	let mut is_hex_output = false
	let mut file_names: Slice<String> = []
	for arg in os.Args[1:] {
		if arg == "--hex" {
			is_hex_output = true
			continue
		}

		file_names.add(arg)
	}
	...

will produce the warning

  ▲ Unused mut
   ╭─[src/main.lis:8:10]
 7 │     let mut is_hex_output = false
 8 │     let mut file_names: Slice<String> = []
   ·             ─────┬────
   ·                  ╰── declared as mutable
 9 │     for arg in os.Args[1:] {
   ╰────
  help: Remove mut from the declaration if you do not need to mutate the variable · code: [lint.unnecessary_mut]

I would expect a slice to be modified if entries are added, so I'd say, mut should be required in this case.

[stormpat]

Slice has no add method? You probably wanted append.

let mut file_names: Slice<string> = []
file_names = file_names.append("file.txt")

Also the primitives (like string) need to be lowercased.

[tmssngr]

Yes, I noticed that also a couple of minutes after sending.

[ivov]

@tmssngr Catching up to this thread:

• mut consistency - The invariant is now enforced at feat: mutable bindings must own their value #951
• mut for parameters - Lis already supports this. Please see here.
• mut for callsites - If you enable inlay hints, your IDE should display mut for params at callsites.
• mut lints firing two at once - Fixed at fix: silence unused mut warning inside erroring functions #956

[tmssngr]

mut for parameters: seems not to work for references. This is no compiler error:

append(&buffer)
...
fn append(buffer: Ref<strings.Builder>) {
	buffer.WriteByte('=')
}

IMHO only this should be allowed to compile:

append(&mut buffer)
...
fn append(buffer: mut Ref<strings.Builder>) {
	buffer.WriteByte('=')
}

[ivov]

This escape hatch is documented here.

Lis enforces mutability where it owns the value, but Ref<T> is a plain Go pointer, which has no notion of (im)mutability, and there is one Ref<T> whether you take it with & or receive it via interop. To enforce mut on Ref<T> the compiler would have to classify every pointer as mutable or not. Go does not track this, and we cannot reliably determine it for third-party Go, so the rule would be either unenforceable at the boundary or inconsistent across it.

[tmssngr]

The larger the structs become, the more often one wants to pass them by reference.

IMHO, one major disadvantage of Go is that it is too simple in this aspect, either copying the whole struct (slow) or not giving guarantees of immutability. Java is not much better but usually one uses interfaces to work around.

I agree that calling Go code from Lis would be the problematic case. For safety reasons one idea could be: treat all references passed to Go functions as mutable, except those listed in a certain project file. Not elegant, but for me the benefits

• improved performance (less copying because of passing by reference) and
• improved safety (code could rely on a method not modifying the passed struct)
  would outweigh it.

[tmssngr]

IMHO, without enforcing immutable references, I don't see a serious pro for having local immutable variables at all. Usually functions are not that large and relatively easy to reason about. But invoked functions are a black box for the caller (without looking at their source code). It is like a far less important problem is solved (let = immutable, let mut = mutable) just because it is easier to achieve, but the more important use-cases that matter for large scale projects are left out. I think, with enforced immutable refs, Lis would be a much better choice for larger applications than Go - at least for me.

With the same argument (Go does not support it), we couldn't even make struct fields immutable either - which works for Java by using final since its initial release.

[ivov]

With mut for params, immutability is already enforced at Lisette callsites. The one type Lis cannot cover is Ref because the mutation can happen in code the compiler never sees across the boundary, hence Ref as a uniform escape hatch for both sides. final fields in Java are enforceable because every write is visible to its compiler. In future Lis could control mutability soundly for struct fields on the Lis side, for the same reason.

Right now I do not see a design that would solve this soundly across the boundary. Inferring mutability from Go source looks at first sight complex and unreliable. A per-project file for arbitrary third-party Go would push an unbounded and unverified maintenance burden onto the user, going stale with each dependency update. Splitting Ref into two pointer types where one is disavowed as unsafe at the boundary would be containment only.

I'll keep researching.