File.close invalidates

[marler8997]

This change is the polar opposite of #10213 but one that Andrew has expressed he would like. Instead of providing 2 ways to cleanup a file (one that invalidates and one that doesn't), this PR modifies File.close to always invalidate. Note this is a breaking change that now requires all instances of File that you want to close to be mutable. This means no more const file = , you must always use var file =. I've argued this reduces code readability, but that's my opinion, you can see for yourselves whether or not you agree and whether it would justify a second way to cleanup files (i.e. adding a closeConst for const files).

Also note that if we start requiring files to be mutable, we will be introducing "ownership" semantics to File. Namely, passing a File by value vs by reference has a new meaning, and having a File field as a value or a reference has new meaning. This adds complexity to how File is interpreted that doesn't exist today with immutable instances of File.

[andrewrk]

I recommend to run the full test suite locally for a big breaking change such as this to make sure it will pass the CI.

[andrewrk]

Looks like this was never passing the CI, and now it has a bunch of merge conflicts. I'm closing this since it's an old PR. I think for this particular change it's a bit of a tricky one - we're going to want to pick a strategic time to make it.

Personally this is not a priority to me until we start focusing on the standard library, which I expect to be in a couple release cycles from now, after we ship self-hosted (#89), and after we have the package manager (#943).

[marler8997]

I was waiting for a response from you on #10213 (comment) before putting more work into this change.

As far as I understand, the reasons for requiring File to be mutable is so we can invalidate its contents on deinit/close. However, this reason would also apply to the allocator interface:

// instead of this
const slice = try allocator.alloc(u8, 100);
defer allocator.free(slice);

// users must do this
var slice = try allocator.alloc(u8, 100);
defer allocator.free(&slice);

Can you confirm whether or not this is also the case, or is there a difference between these two cases that makes you think we should treat them differently?

[andrewrk]

I see the similarities between these cases. I don't want to make this change to the allocator interface. I see your point. I don't have a conclusion or any guidance at this time. I'm focused on other parts of the project and am postponing these API consistency considerations. I think we have other priorities currently and the resolution to this problem will reveal itself once we focus on it.