Sync modifiableLvalue from D -> C++

[ibuclaw]

@WalterBright - please confirm. The addition of IntervalExp::modifiableLvalue was a mistake.

My assumption is yes because it is identical to DelegatePtrExp::modifiableLvalue.

[wilzbach]

@ibuclaw DAutoTest doesn't like this:

../phobos/std/experimental/ndslice/slice.d(26): Deprecation: module std.experimental.ndslice.internal is deprecated - Please use mir-algorithm DUB package: http://github.com/libmir/mir-algorithm
../phobos/std/experimental/ndslice/internal.d(8): Deprecation: module std.experimental.ndslice.slice is deprecated - Please use mir-algorithm DUB package: http://github.com/libmir/mir-algorithm
../phobos/std/experimental/ndslice/selection.d(63): Deprecation: module std.experimental.ndslice.internal is deprecated - Please use mir-algorithm DUB package: http://github.com/libmir/mir-algorithm
../phobos/std/experimental/ndslice/selection.d(64): Deprecation: module std.experimental.ndslice.slice is deprecated - Please use mir-algorithm DUB package: http://github.com/libmir/mir-algorithm
../phobos/std/experimental/ndslice/package.d(315): Deprecation: module std.experimental.ndslice.slice is deprecated - Please use mir-algorithm DUB package: http://github.com/libmir/mir-algorithm
../phobos/std/experimental/ndslice/package.d(316): Deprecation: module std.experimental.ndslice.iteration is deprecated - Please use mir-algorithm DUB package: http://github.com/libmir/mir-algorithm
../phobos/std/experimental/ndslice/iteration.d(106): Deprecation: module std.experimental.ndslice.internal is deprecated - Please use mir-algorithm DUB package: http://github.com/libmir/mir-algorithm
../phobos/std/experimental/ndslice/iteration.d(107): Deprecation: module std.experimental.ndslice.slice is deprecated - Please use mir-algorithm DUB package: http://github.com/libmir/mir-algorithm
../phobos/std/experimental/ndslice/package.d(317): Deprecation: module std.experimental.ndslice.selection is deprecated - Please use mir-algorithm DUB package: http://github.com/libmir/mir-algorithm
../phobos/std/c/linux/pthread.d(17): Deprecation: module std.c.linux.linux is deprecated - Import the appropriate core.sys.posix.* modules instead
../phobos/std/random.d(446): Error: template instance std.random.LinearCongruentialEngine!(uint, 16807, 0, 2147483647) does not match template declaration LinearCongruentialEngine(UIntType, UIntType a, UIntType c, UIntType m) if (isUnsigned!UIntType)
../phobos/std/random.d(448): Error: template instance std.random.LinearCongruentialEngine!(uint, 48271, 0, 2147483647) does not match template declaration LinearCongruentialEngine(UIntType, UIntType a, UIntType c, UIntType m) if (isUnsigned!UIntType)
../phobos/std/random.d(730): Error: template instance std.random.MersenneTwisterEngine!(uint, 32, 624, 397, 31, 2567483615u, 11, 7, 2636928640u, 15, 4022730752u, 18) does not match template declaration MersenneTwisterEngine(UIntType, ulong w, ulong n, ulong m, ulong r, UIntType a, ulong u, ulong s, UIntType b, ulong t, UIntType c, ulong l) if (isUnsigned!UIntType)
../phobos/std/random.d(1024): Error: template instance std.random.XorshiftEngine!(uint, 32, 13, 17, 15) does not match template declaration XorshiftEngine(UIntType, UIntType bits, UIntType a, UIntType b, UIntType c) if (isUnsigned!UIntType)
../phobos/std/random.d(1025): Error: template instance std.random.XorshiftEngine!(uint, 64, 10, 13, 10) does not match template declaration XorshiftEngine(UIntType, UIntType bits, UIntType a, UIntType b, UIntType c) if (isUnsigned!UIntType)
../phobos/std/random.d(1026): Error: template instance std.random.XorshiftEngine!(uint, 96, 10, 5, 26) does not match template declaration XorshiftEngine(UIntType, UIntType bits, UIntType a, UIntType b, UIntType c) if (isUnsigned!UIntType)
../phobos/std/random.d(1027): Error: template instance std.random.XorshiftEngine!(uint, 128, 11, 8, 19) does not match template declaration XorshiftEngine(UIntType, UIntType bits, UIntType a, UIntType b, UIntType c) if (isUnsigned!UIntType)
../phobos/std/random.d(1028): Error: template instance std.random.XorshiftEngine!(uint, 160, 2, 1, 4) does not match template declaration XorshiftEngine(UIntType, UIntType bits, UIntType a, UIntType b, UIntType c) if (isUnsigned!UIntType)
../phobos/std/random.d(1029): Error: template instance std.random.XorshiftEngine!(uint, 192, 2, 1, 4) does not match template declaration XorshiftEngine(UIntType, UIntType bits, UIntType a, UIntType b, UIntType c) if (isUnsigned!UIntType)
../phobos/std/traits.d(6620): Error: static assert  "Type uint does not have an Unsigned counterpart"
../phobos/std/traits.d(6386):        instantiated from here: Modifier!uint
../phobos/std/traits.d(6624):        instantiated from here: ModifyTypePreservingTQ!(Impl, uint)
../phobos/std/format.d(1480):        instantiated from here: Unsigned!uint
../phobos/std/format.d(1940):        ... (9 instantiations, -v to show) ...
../phobos/std/format.d(977):        instantiated from here: parse!(int, const(char)[])
../phobos/std/bitmanip.d(2005):        instantiated from here: FormatSpec!char
posix.mak:461: recipe for target 'docs-prerelease.json' failed
make: *** [docs-prerelease.json] Error 1

FYI: ndslice has been removed in master: dlang/phobos#5187

[ibuclaw]

@ibuclaw DAutoTest doesn't like this:

That has nothing to do with this patch though.

[ibuclaw]

Forced new commit hash, no changes done to code.

[ibuclaw]

Confirmed, the autotest is broken, not this change (#6599 has the same failed message) - @CyberShadow know what's wrong? I'm running this on stable phobos+druntime branch, and it passes.

find ../druntime/src -name '*.d' | sed -e '/gcstub/d'   -e /unittest/d > .prerelease-files.txt
find ../phobos -name '*.d' | sed -e /unittest.d/d   -e /windows/d >> .prerelease-files.txt
../dmd/src/dmd -c -o- -version=CoreDdoc -version=StdDdoc -Df.prerelease-dummy.html   -Xfdocs-prerelease.json -I../phobos @.prerelease-files.txt

[ibuclaw]

@WalterBright - any reply?

[CyberShadow]

That reminds me of dlang/dlang.org#1568... but, looking at the branch history, it started failing again well after that was fixed.

It looks like it got broken by either dlang/tools#219 or #6566, and I see that @WalterBright didn't use auto-merge for the latter (the documentation tester didn't even report in).

[CyberShadow]

That reminds me of dlang/dlang.org#1568...

It's definitely that. Compare the build log from stable today and February 8 - same errors in std.random.

[CyberShadow]

So, I guess I could stoop even lower and add an even dirtier workaround - pass the files in exactly the ext4 order - or someone could actually sit down and figure out WTF is wrong with the compiler this time :)

[CyberShadow]

Steps to reproduce:

1. Ensure your /tmp/ is mounted as a tmpfs
2. mkdir /tmp/digger-test
3. Set local.workDir in digger.ini to /tmp/digger-test
4. Run digger build --with=website stable@2017-03-08

[CyberShadow]

OK, so I just needed to apply the same workaround to the prerelease stuff this time: dlang/dlang.org#1604

[WalterBright]

The addition of IntervalExp::modifiableLvalue was a mistake.

To be clear on if the mistake is it existing, or the mistake is in its implementation, one would need to examine the PR that inserted it.

[ibuclaw]

You rebased the pr, I'm pretty sure you would remember if you explicitly added a modifiableLvalue method to IntervalExp with an error message about delegates inside (this class is used for slices, and only internally).

[ibuclaw]

The PR number is in the commit message, or just visit #5852

[ibuclaw]

Further proof...

https://codecov.io/gh/dlang/dmd/src/master/src/expression.d#L12093...12101

Codecov says that this is dead code.

[WalterBright]

I misread it. I thought it was IntegralExp, but it's IntervalExp. From examining the use of it, I don't think modifiableLvalue() should ever be called on it. But leaving it out means the default one would be called. I suggest replacing the body of the function with assert(0); to confirm that it never gets called.

[ibuclaw]

I suggest replacing the body of the function with assert(0); to confirm that it never gets called.

And I suggest you don't. If you wanted to restrict what kinds of expression are permitted to call this, then assert false should be the default case.

[ibuclaw]

Ping?

[WalterBright]

Please replace the function body with assert(0); and I'll pull it.

[yebblies]

I suggest:

assert(0, "Just delete this function to fix the ICE");

so that the fix is clear when this inevitably shows up again.

[ibuclaw]

Or just error gracefully, which is the default...

https://github.com/ibuclaw/dmd/blob/173e671221aab5df65b3c352c50af583225da542/src/expression.d#L2778

[yebblies]

Yeah I know. If you make it mergeable I'll merge it and Walter can yell at me in person in a few days.

[ibuclaw]

Yeah I know. If you make it mergeable I'll merge it and Walter can yell at me in person in a few days.

You mean, we'll yell at Walter in person, for not properly reviewing his own PRs.

[ibuclaw]

Or I could always add this to #2194.