Bug 20670 – immutable template specialization pattern matches immutable struct, strips immutable

Status
NEW
Severity
regression
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2020-03-13T08:59:15Z
Last change time
2024-12-13T19:07:36Z
Keywords
industry
Assigned to
No Owner
Creator
FeepingCreature
Moved to GitHub: dmd#19676 →

Comments

Comment #0 by default_357-line — 2020-03-13T08:59:15Z
Consider an immutable struct: ``` immutable struct S { } ``` Let's define a template specialization that tries to strip away immutable from a type, like Unqual. ``` mixin template Foo(T) { static assert(is(S == T)); } mixin template Foo(T: immutable U, U) { pragma(msg, U.stringof); // even if this matches, S should still be S. There is only one S. static assert(is(S == U)); } ``` Let's apply it to our immutable struct: ``` mixin Foo!S; ``` Since 2.076.1, it takes the second template and fails with the static assert! We've somehow managed to take a struct type and strip away its inherent constness attribute. This is of course very bad. If we define S as `immutable struct S`, we should not be able by any mechanism to get the non-immutable struct S back out.
Comment #1 by default_357-line — 2020-04-07T14:28:11Z
Different instance of probably the same bug: immutable struct Struct { } struct Wrapper { void foo(U)(inout(U)) inout { // U is inferred as 'struct Struct', not 'immutable struct Struct'. // evidence: pragma(msg, isMutable!Struct.stringof ~ " - " ~ isMutable!U.stringof); static assert(is(U == Struct)); } } Wrapper().foo(Struct());
Comment #2 by default_357-line — 2022-05-05T08:38:17Z
Just ran into this issue again.
Comment #3 by snarwin+bugzilla — 2022-05-08T16:27:05Z
Worth noting that even when immutable is stripped from the struct type itself, it still applies to the struct's members: --- immutable struct S { int n; } static if (is(S : immutable U, U)) { static assert(!is(U == immutable)); static assert(is(typeof(U.n) == immutable)); } ---
Comment #4 by dkorpel — 2022-05-10T19:34:18Z
I don't think structs can have an 'inherent constness attribute'. The spec says: https://dlang.org/spec/struct.html#const-struct > A struct declaration can have a storage class of const, immutable or shared. > It has an equivalent effect as declaring each member of the struct as const, immutable or shared. So these structs are supposed to be treated the same: ``` immutable struct S0 { int n; } struct S1 { immutable int n; } ``` Now note that even for S1, `is(S1 == immutable(S1))` does not hold. I think the real issue is this: ``` pragma(msg, S0); // immutable(S0) pragma(msg, S1); // S1 ``` Referring to `S0` by name results in an `immutable(S0)` type instead of just `S0` with immutable members.
Comment #5 by dkorpel — 2022-05-12T12:56:01Z
Comment #6 by default_357-line — 2022-05-12T15:02:17Z
Well... it's ... still bad. (As well as spec-violating.) It means that there's straight up no way in the language to detect any difference between immutable struct S {} ... S and struct S { } ... immutable S. The idea is that the constness attached to the type gives a hint as to how that type should be used. For instance with template functions that return Nullable, I would expect the first to return Nullable!(immutable S), ie. Nullable!S, and the second immutable Nullable!S. This is, at the moment, impossible due to this bug.
Comment #7 by snarwin+bugzilla — 2022-05-12T16:27:37Z
D's type system currently guarantees that for all qualified types Q(T), there exists the corresponding unqualified type T. Introducing special-case exceptions to this guarantee is likely to break at least as much code as it fixes (try `git grep 'Unqual!'` in Phobos to get a rough idea). IMO if one wants to hint to users that a type should be used with a particular qualifier by default, a better way (that does not require weird corner cases in the type system) is to use a public alias to a qualified version of a private type. --- lib.d module lib; private struct StructImpl { } alias Struct = immutable(StructImpl); --- app.d import lib; Struct s; static assert(is(typeof(s) == immutable)); ---
Comment #8 by default_357-line — 2022-05-13T00:24:12Z
> D's type system currently guarantees that for all qualified types Q(T), there exists the corresponding unqualified type T. I disagree with this claim, specifically because `immutable struct S` exists. I don't see where D makes the claim that this is the case. `Unqual` says that it strips type qualifiers. But `immutable struct` is not actually a qualifier, it's an "immutable declaration." This is currently, apparently, implemented as an implicit qualifier, which is what this bug is about, and at any rate is not what the spec says. What's the point of having unqualified types anyway? It's not the same as a mutable value conversion of the type regardless. (That would require headmutable.) So since it can't be used to make "a mutable field of T", I don't see what this gives you.
Comment #9 by dkorpel — 2022-05-14T15:50:55Z
(In reply to FeepingCreature from comment #8) > I disagree with this claim, specifically because `immutable struct S` > exists. I don't see where D makes the claim that this is the case. D didn't make the claim, Paul made the claim. It follows from how immutable is defined as something that modifies a type, see for example name mangling: https://dlang.org/spec/abi.html#type_mangling The compiler uses that for type identity. Also see how Type Qualifiers are implemented as a field in the top level `Type` class: https://github.com/dlang/dmd/blob/c4cea697e8658f103a69967587e75dd130506304/src/dmd/mtype.d#L334 > But `immutable struct` is not actually > a qualifier, it's an "immutable declaration." This is currently, apparently, > implemented as an implicit qualifier, which is what this bug is about Do you want to invent a new `immutable struct` concept, with its own mangling? Because I prefer not to complicate the type system, so I'm looking for a solution without special cases. > and at any rate is not what the spec says. I can clarify the spec, but I don't think that's what you're after.
Comment #10 by snarwin+bugzilla — 2022-05-14T16:09:22Z
> But `immutable struct` is not actually a qualifier, it's an > "immutable declaration." This is currently, apparently, implemented as > an implicit qualifier, which is what this bug is about, and at any rate > is not what the spec says. Here's the relevant part of the spec: https://dlang.org/spec/attribute.html#immutable It says immutable works "the same was as const does", so let's look at the section on const: > The const attribute changes the type of the declared symbol from T to > const(T), where T is the type specified (or inferred) for the introduced > symbol in the absence of const. https://dlang.org/spec/attribute.html#const This paragraph implicitly assumes that the declared symbol has a type. But in the case of `immutable struct S`, S does not *have* a type, it *is* a type. So the spec does not give us a clear answer one way or the other to the question of what `immutable struct S` ought to mean.
Comment #11 by default_357-line — 2022-05-14T18:05:39Z
The problem is that it seems like Unqual has conflicting purposes - and this is, I think, ultimately about Unqual; about the ability to take a type - any type - and gain a mutable version. The problem is that immutable, on one hand, says "it acts like every field has been marked immutable." But the ability to mark a field immutable is already enough to completely obsolete Unqual as a concept, since a declaration of Unqual!T will not be a mutable value. Okay, so from this we take that it's just the case that we have mutable types, and immutable types, and Unqual is a specialty tool that's useless in the general case. (Hence our own librebindable.) But then the ability to strip immutable from a type is a problem. Because now you have to differentiate between "a type marked as immutable" and "a type declaration marked as immutable". Because, since you've already gained, necessarily, the ability to deal with types that cannot be turned into mutable declarations by any means, there is no point to stripping immutable from `immutable struct S` anymore. What does it buy you? The bubble of types you can create a mutable declaration of gets a bit bigger. It still doesn't cover an appreciable fraction of the D type landscape - including the types you get if you *just replace immutable struct with the thing it says it's a shortcut for!* So from that basis, I'd drawn a distinction between 'immutable struct S' and 'struct S, immutable S' - as "in one case, the user may want to see a mutable S, especially if used as an inner field in a constructed type; ie. we want to move immutable as far outward as possible, as in `immutable Nullable!S`, and in the other case, the user asserts that they never ever ever want to have to deal with a mutable S for any reason." Which makes sense, because that gives us pure value types, with working invariants. Domain types, where we don't need to hide every field behind an accessor - because *the only way* to set a field is via the constructor. As it should be for domain types. So I've always thought that this was what `immutable struct S` was for. Because it's not like it's good for anything else.
Comment #12 by snarwin+bugzilla — 2022-05-14T18:13:59Z
> Because, since you've already gained, necessarily, the ability to deal with types that cannot be turned into mutable declarations by any means, there is no point to stripping immutable from `immutable struct S` anymore. What does it buy you? As Dennis said: fewer special cases in the type system. Forget about Unqual. It was just meant to be an example (evidently not a very good one). Avoiding special cases in the type system is the point.
Comment #13 by default_357-line — 2022-05-14T18:24:39Z
To clarify where I'm coming from, please follow our (Funkwerk's) evolution of domain types, using a fictionalized example. First, consider a type: class PassengerWagon; class CargoWagon; struct Wagon { PassengerWagon passengerWagon; CargoWagon cargoWagon; } Now this is trying to approximate a sum type, but one issue is that a wagon can, for instance, be both a passenger and a cargo wagon. Wagons cannot actually do this; it is a modelling error. We solve it with an invariant: struct Wagon { PassengerWagon passengerWagon; CargoWagon cargoWagon; invariant((passengerWagon is null) != (cargoWagon is null)); } Now of course this is a very dangerous type! Because if you auto wagon = Wagon(passengerWagon, null); wagon.passengerWagon = null; and the invariant is silently violated, and as the bad value gets carried deeper into the program, it may cause mischief far away. Also, the implicit struct constructor doesn't check the invariant, so we can do angry stuff like Wagon(). So we secure it! struct Wagon { private PassengerWagon passengerWagon_; private CargoWagon cargoWagon_; invariant((passengerWagon_ is null) != (cargoWagon_ is null)); @disable this(); this(PassengerWagon passengerWagon, CargoWagon cargoWagon) { this.passengerWagon_ = passengerWagon; this.cargoWagon_ = cargoWagon; } PassengerWagon passengerWagon() { return passengerWagon_; } CargoWagon cargoWagon() { return cargoWagon_; } } You may note that this simple type is starting to become... annoying. Which is to say, terrible. Terrible to read, terrible to write, and rife with potential for typos. We had a *lot* of structs like this. With a lot of fields. The amount of programmer-hours spent on this was unreasonably high. I introduced boilerplate to fix it: struct Wagon { @ConstRead private PassengerWagon passengerWagon_; @ConstRead private CargoWagon cargoWagon_; invariant((passengerWagon_ is null) != (cargoWagon_ is null)); // also generate the constructor, while we're at it mixin(GenerateThis); mixin(GenerateFieldAccessors); } Now it's shorter, but it's still kind of annoying, and also for some reason all our machines run out of memory when we trigger builds. Could the thousand lines of templates in boilerplate have something to do with that? We may never know. But - wasn't the thing we actually wanted just to enforce that all fields were only set through the constructor? And wasn't there already a D language feature that ensured that fields couldn't change? immutable struct Wagon { PassengerWagon passengerWagon; CargoWagon cargoWagon; invariant((passengerWagon_ is null) != (cargoWagon_ is null)); mixin(GenerateThis); } Nice and simple, no accessors, basically no templateness, no unintended mutations bypassing the invariants. Of course, :points at the large list of bugs he filed with regard to immutable.: And also, :points at his Turducken technique post and librebindable just to get *what Unqual was supposed to be from the start.*: And by the way, we have an internal implementation of hashmaps! We don't have that for performance, but *just for dealing with immutable types.* It's just that that simple struct, with the simple annotation, and zero template overhead, is *so damn tempting.* This is how we want every struct in the domain layer to look. No mutation, pure data. If immutable is not for that, fine, but something should be, because this is a key component of writing safe, trustable code.
Comment #14 by snarwin+bugzilla — 2022-05-14T18:46:04Z
> But - wasn't the thing we actually wanted just to enforce that all fields > were only set through the constructor? If that's all you want, then the current behavior of `immutable struct` should be sufficient--because, as I pointed out in comment 3, stripping immutable from the type does not strip it from the members. In fact, even the behavior Dennis proposes in comment 4 would be sufficient. However, going by the original bug report, it seems like in addition to this you *also* want to be able to distinguish between `immutable struct S` and `struct S { immutable: }` in template specializations. And in order to do this, you would like a new special case to be introduced to the type system. Am I missing anything?
Comment #15 by dkorpel — 2022-05-14T18:58:04Z
(In reply to FeepingCreature from comment #13) > This is how we want every struct in the domain layer to look. Would this work? ``` struct S { immutable: // fields... } ```
Comment #16 by default_357-line — 2022-05-14T19:15:44Z
Huh. Yeah... Does that work? edit: Also, I hadn't quite realized that struct S was still, actually, immutable on every field. Now that you point it out, that obviates the worries I'd had about it. But why is there even an immutable tag on it then? Why isn't `immutable struct S` just exactly the same as `struct S { immutable: }`?
Comment #17 by snarwin+bugzilla — 2022-05-15T15:07:23Z
Introduced by the fix for issue 17582: https://github.com/dlang/dmd/pull/6958 So, it seems like the "implicit qualifier" behavior has always been intended, but was inadvertently disabled for a 5-year period, starting from version 2.056 and ending in version 2.076.
Comment #18 by robert.schadek — 2024-12-13T19:07:36Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/19676 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB