Bug 13084 – ModuleInfo.opApply delegate expects immutable parameter

The following code works fine in DMD 2.065.0: void main () { ModuleInfo* minfo; foreach (m ; ModuleInfo) minfo = m; } But in DMD 2.066.0-b1 the delegate parameter in ModuleInfo.opApply expects an immutable ModuleInfo*. This is a breaking change and a regression. Is it possible to fix this by overloading opApply with a mutable parameter?

It is intentional to change all ModuleInfo objects immutable. https://github.com/D-Programming-Language/druntime/pull/790

I assumed that, but i still breaks code. What about the standard deprecation cycle?

(In reply to Jacob Carlborg from comment #2) > I assumed that, but i still breaks code. What about the standard deprecation > cycle? Unfortunately, it's impossible. We cannot allow iterating both mutable and immutable objects by ModuleInfo.opApply, because it will cause type system violation. We should change the opApply signature all at once. And compiler already place all ModuleInfo in read-only section. So modifying ModuleInfo will cause segfault in some platforms. It's more worse.

Hmm. Suppose we did this: alias immutable(_ModuleInfo) ModuleInfo; ?

(In reply to Walter Bright from comment #4) > Hmm. Suppose we did this: > > alias immutable(_ModuleInfo) ModuleInfo; > > ? It would still break the API.

(In reply to Kenji Hara from comment #3) > And compiler already place all ModuleInfo in read-only section. So modifying > ModuleInfo will cause segfault in some platforms. It's more worse. How can you modify ModuleInfo when all its members (methods and fields) are const?

(In reply to Jacob Carlborg from comment #5) > (In reply to Walter Bright from comment #4) > > Hmm. Suppose we did this: > > > > alias immutable(_ModuleInfo) ModuleInfo; > > > > ? > > It would still break the API. How? (I know it will break people modifying ModuleInfo at runtime, but anything else?)

(In reply to Walter Bright from comment #7) > How? (I know it will break people modifying ModuleInfo at runtime, but > anything else?) If I understand you correctly the current ModuleInfo would be renamed to _ModuleInfo and a new immutable alias would be added, named ModuleInfo? The ModuleInfo I have declared would be immutable? That could work. I will work for the function where I found the problem but it returns the module info and I have not looked yet how the returned value is used.

(In reply to Jacob Carlborg from comment #8) > If I understand you correctly the current ModuleInfo would be renamed to > _ModuleInfo and a new immutable alias would be added, named ModuleInfo? Yes. > The ModuleInfo I have declared would be immutable? Yes. > That could work. I will > work for the function where I found the problem but it returns the module > info and I have not looked yet how the returned value is used.

Did the alias thang: https://github.com/D-Programming-Language/druntime/pull/886 https://github.com/D-Programming-Language/dmd/pull/3754

(In reply to Walter Bright from comment #9) > (In reply to Jacob Carlborg from comment #8) > > If I understand you correctly the current ModuleInfo would be renamed to > > _ModuleInfo and a new immutable alias would be added, named ModuleInfo? > > Yes. > > > The ModuleInfo I have declared would be immutable? > > Yes. > > > That could work. I will > > work for the function where I found the problem but it returns the module > > info and I have not looked yet how the returned value is used. I don't think it's good idea. It would introduce confusing than benefits.

(In reply to Kenji Hara from comment #11) > I don't think it's good idea. It would introduce confusing than benefits. Please explain. What confusion?

(In reply to Walter Bright from comment #12) > (In reply to Kenji Hara from comment #11) > > I don't think it's good idea. It would introduce confusing than benefits. > > Please explain. What confusion? All D programmers would assume that unqualified type name T is a mutable type, and const(T) is const, immutable(T) is immutable. But, the proposed idea will break the assumption - all ModuleInfo, const(ModuleInfo), immutable(ModuleInfo) will be same immutable. it's very confusing situation.

(In reply to Kenji Hara from comment #13) > All D programmers would assume that unqualified type name T is a mutable > type, and const(T) is const, immutable(T) is immutable. > > But, the proposed idea will break the assumption - all ModuleInfo, > const(ModuleInfo), immutable(ModuleInfo) will be same immutable. it's very > confusing situation. Hmm, that's a good point.

(In reply to Kenji Hara from comment #13) > All D programmers would assume that unqualified type name T is a mutable > type, There were some bumps in transitioning to the immutable alias 'string', but it all turned out for the best. The compiler complains if you do anything wrong, and this proposal fixes the breaking code issue.

Find another project with the same error.

(In reply to Jacob Carlborg from comment #16) > Find another project with the same error. ??

(In reply to Walter Bright from comment #17) > (In reply to Jacob Carlborg from comment #16) > > Find another project with the same error. > > ?? I'm just indicating that there are more than one project that is affected by this.

(In reply to Jacob Carlborg from comment #18) > I'm just indicating that there are more than one project that is affected by > this. Affected by what? If you know one, why not name it?

(In reply to David Nadlinger from comment #19) > Affected by what? If you know one, why not name it? Affected by the issue I reported here. Or rather the change that causes this issue. The projects I've found this in so far is DWT and Tango.

(In reply to Walter Bright from comment #15) > (In reply to Kenji Hara from comment #13) > > All D programmers would assume that unqualified type name T is a mutable > > type, > > There were some bumps in transitioning to the immutable alias 'string', Note that, string == immutable(char)[], it is _mutable_ array of immutable elements. It's different.

I posted a little overview of the situation at the druntime pull request page.

> All D programmers would assume that unqualified type name T is a mutable type, and const(T) is const, immutable(T) is immutable. What? Sounds like total nonsense. I use aliases of qualified types all the time, it is a very useful tool for enforcing certain qualified semantics. Programmers that expect that must have never read the docs. I think it is a reasonable solution.

(In reply to Dicebot from comment #23) > > All D programmers would assume that unqualified type name T is a mutable type, and const(T) is const, immutable(T) is immutable. > > What? Sounds like total nonsense. I use aliases of qualified types all the > time, it is a very useful tool for enforcing certain qualified semantics. > Programmers that expect that must have never read the docs. > > I think it is a reasonable solution. In your code, you can have such aliases. But don't add such alias in standard library code. It will introduce much limited situation than benefits. For example, in the proposal original mutable name _ModuleInfo is private and inaccessible. So if a programmer want to use const ModuleInfo, he should write it like const(Unqual!ModuleInfo). Why Unqual is necessary? In other words, you assumes that most D programmers won't use non-immutable ModuleInfo in their code. I think it's not reasonable assumption.

Let's list a few objective facts: 1. The current situation (this bug report) occurs in the wild in real code in more than one project. Existing code is broken. 2. https://issues.dlang.org/show_bug.cgi?id=13148 has not been shown to occur anywhere in existing code, it is conjecture. A reason to write such code has not been demonstrated. 3. If there is a reason to write 13148 code, it can be fixed with the Unqual!ModuleInfo change. 4. In either solution, using ModuleInfo incorrectly will result in compiler errors, not silent corruption. Therefore it seems highly likely that there will be far more instances of requiring users to write immutable(ModuleInfo) than Unqual!ModuleInfo. Making ModuleInfo an immutable alias thus breaks far less existing code. (Inferring that an unadorned type name would necessarily be mutable comes as a surprise to me. This assumption is not stated anywhere in the documentation nor in common usage. A grep of phobos/std shows immutable aliases occurring. It's one of the things alias is for.)

> In your code, you can have such aliases. But don't add such alias in standard library code. It will introduce much limited situation than benefits. This is just your preference, there is nothing standard about it. Such solution is introduced exactly to impose limits and make using unqualified type harder. Forcing certain type to be always used as immutable is not unheard of which also means that you don't normally need const version of same type - because, well, it is always meant to be immutable. On actual topic: is there any reason why ModuleInfo itself needs to be immutable and marking all its data members is not enough? That should still make all the the options from PR #790 valid or am I missing something?

(In reply to Dicebot from comment #26) > On actual topic: is there any reason why ModuleInfo itself needs to be > immutable and marking all its data members is not enough? That should still > make all the the options from PR #790 valid or am I missing something? Exactly, I've already asked that but didn't get a reply on that question. Is it possible to write to the pointer that will cause problems?

(In reply to Jacob Carlborg from comment #27) > (In reply to Dicebot from comment #26) > > > On actual topic: is there any reason why ModuleInfo itself needs to be > > immutable and marking all its data members is not enough? That should still > > make all the the options from PR #790 valid or am I missing something? > > Exactly, I've already asked that but didn't get a reply on that question. Is > it possible to write to the pointer that will cause problems? ModuleInfo is a variable sized struct that is decoded at runtime. The two explicit fields are only the start of it. Making the whole thing immutable makes the intent clear.

Is it possible to apply usual C idiom for variadic length structs? Something like this: struct ModuleInfo { immutable: uint _flags; uint _index; void[0] payload; /* methods */ } I am not sure it is legal application of immutable qualifier though.

(In reply to Dicebot from comment #26) > On actual topic: is there any reason why ModuleInfo itself needs to be > immutable and marking all its data members is not enough? That should still > make all the the options from PR #790 valid or am I missing something? Well, without breaking the type system, the runtime only has access to an array of immutable pointers to ModuleInfos.

Commits pushed to master at https://github.com/D-Programming-Language/druntime https://github.com/D-Programming-Language/druntime/commit/d9df5c6418a3ed0d826aa9ad92eda01013670616 fix Issue 13084 - ModuleInfo.opApply delegate expects immutable parameter - cast to mutable https://github.com/D-Programming-Language/druntime/commit/4acdfcbbae02987f995d722daea17da78ccbd155 Merge pull request #909 from MartinNowak/fix13084 fix Issue 13084 - ModuleInfo.opApply delegate expects immutable parameter

Commit pushed to 2.066 at https://github.com/D-Programming-Language/druntime https://github.com/D-Programming-Language/druntime/commit/1aabbb24e7813c101bdb1c17c4ee17d09eb093c8 Merge pull request #909 from MartinNowak/fix13084 fix Issue 13084 - ModuleInfo.opApply delegate expects immutable parameter

Thanks

Commit pushed to master at https://github.com/D-Programming-Language/druntime https://github.com/D-Programming-Language/druntime/commit/1aabbb24e7813c101bdb1c17c4ee17d09eb093c8 Merge pull request #909 from MartinNowak/fix13084