Jump to page: 1 2
Thread overview
[dmd-beta] Releasing 2.066.1
Oct 08, 2014
Martin Nowak
Oct 15, 2014
Martin Nowak
Oct 15, 2014
Martin Nowak
Oct 15, 2014
Walter Bright
Oct 15, 2014
Martin Nowak
Oct 15, 2014
Walter Bright
Oct 16, 2014
Martin Nowak
Oct 22, 2014
Martin Nowak
October 08, 2014
I want to release 2.066.1 pretty soon (by the end of this week).
Are there any know issues that still need to be addressed?

One thing that I'd like to get in is readding std.metastrings, because it's removal in 2.066 broke every vibe.d program.
https://github.com/D-Programming-Language/phobos/pull/2596
_______________________________________________
dmd-beta mailing list
dmd-beta@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-beta
October 08, 2014
Martin,

If you recall, in 2.066, the AA code was changed to use typeinfo.equals instead of typeinfo.compare.

This caused a so called "regression", which prompted Walter to undo the work that disallows AA keys with compare override but not equals override (a knee jerk reaction IMO to the term "regression"). I believe you had written the code for and pushed for that error. I haven't seen your voice on that change, but I thought it was not a good idea to remove that error.

I don't think it has been undone for classes either, so at least current git head is inconsistent.

Your thoughts? Releasing 2.066.1 as is will allow code built to expect opCmp to work as an AA key will silently fail as we discussed (I tested this a few weeks ago).

Relevant discussion: https://issues.dlang.org/show_bug.cgi?id=13179

On Oct 8, 2014, at 11:01 AM, Martin Nowak via dmd-beta <dmd-beta@puremagic.com> wrote:

> I want to release 2.066.1 pretty soon (by the end of this week). Are there any know issues that still need to be addressed?
> 
> One thing that I'd like to get in is readding std.metastrings, because it's removal in 2.066 broke every vibe.d program.
> https://github.com/D-Programming-Language/phobos/pull/2596
> _______________________________________________
> dmd-beta mailing list
> dmd-beta@puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-beta


_______________________________________________
dmd-beta mailing list
dmd-beta@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-beta
October 15, 2014
On 10/08/2014 06:28 PM, Steven Schveighoffer via dmd-beta wrote:
> Martin,
>
> If you recall, in 2.066, the AA code was changed to use typeinfo.equals instead of typeinfo.compare.
>
> This caused a so called "regression", which prompted Walter to undo the work that disallows AA keys with compare override but not equals override (a knee jerk reaction IMO to the term "regression"). I believe you had written the code for and pushed for that error. I haven't seen your voice on that change, but I thought it was not a good idea to remove that error.
>
Indeed fairly suboptimal, why hasn't anyone stopped him from doing it?

> I don't think it has been undone for classes either, so at least current git head is inconsistent.
>
> Your thoughts? Releasing 2.066.1 as is will allow code built to expect opCmp to work as an AA key will silently fail as we discussed (I tested this a few weeks ago).
>
> Relevant discussion:https://issues.dlang.org/show_bug.cgi?id=13179

Yes, it might silently break certain code, but apparently erroring on code that would work with the compiler generated xOpEquals wasn't acceptable either.
At least it's noted in the changelog.
http://dlang.org/changelog.html#aa-key-requirement
_______________________________________________
dmd-beta mailing list
dmd-beta@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-beta
October 15, 2014
On 10/15/2014 01:19 PM, Martin Nowak via dmd-beta wrote:
> Yes, it might silently break certain code, but apparently erroring on
> code that would work with the compiler generated xOpEquals wasn't
> acceptable either.

Let's add it back as a warning and I'll rebuild 2.066.1 afterwards.

_______________________________________________
dmd-beta mailing list
dmd-beta@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-beta
October 15, 2014
On 10/15/2014 4:32 AM, Martin Nowak via dmd-beta wrote:
> On 10/15/2014 01:19 PM, Martin Nowak via dmd-beta wrote:
>> Yes, it might silently break certain code, but apparently erroring on
>> code that would work with the compiler generated xOpEquals wasn't
>> acceptable either.
>
> Let's add it back as a warning and I'll rebuild 2.066.1 afterwards.

Please keep this in mind:

http://forum.dlang.org/post/lqv16p$1cr0$1@digitalmars.com

_______________________________________________
dmd-beta mailing list
dmd-beta@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-beta
October 15, 2014
On 10/15/2014 08:49 PM, Walter Bright via dmd-beta wrote:
>
> http://forum.dlang.org/post/lqv16p$1cr0$1@digitalmars.com
>

Thanks for the link, we left things as they were.
_______________________________________________
dmd-beta mailing list
dmd-beta@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-beta
October 15, 2014
On 10/15/2014 1:24 PM, Martin Nowak via dmd-beta wrote:
> On 10/15/2014 08:49 PM, Walter Bright via dmd-beta wrote:
>>
>> http://forum.dlang.org/post/lqv16p$1cr0$1@digitalmars.com
>>
>
> Thanks for the link, we left things as they were.

Thanks for the clarification. I was a little confused about just what course of action was proposed.
_______________________________________________
dmd-beta mailing list
dmd-beta@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-beta
October 16, 2014
On Oct 15, 2014, at 4:38 PM, Walter Bright via dmd-beta <dmd-beta@puremagic.com> wrote:

> 
> On 10/15/2014 1:24 PM, Martin Nowak via dmd-beta wrote:
>> On 10/15/2014 08:49 PM, Walter Bright via dmd-beta wrote:
>>> 
>>> http://forum.dlang.org/post/lqv16p$1cr0$1@digitalmars.com
>>> 
>> 
>> Thanks for the link, we left things as they were.
> 
> Thanks for the clarification. I was a little confused about just what course of action was proposed.

I am too actually. From your earlier messages, Martin, I thought you said we would add it as a warning? Has that been done for 2.066.1? I didn't see it in my github traffic.

FWIW, I think a warning is sufficient, even if it only lasts a few versions. As long as silent behavior changes do not happen.

Note, this is kind of crappy because in 2.065 I think, we required opCmp for AA keys, in a way that just made people re-implement the default opCmp (not a good requirement). Then we added another error that said "you can't just implement opCmp, dummy!" and so it's like we can't make up our mind. Unfortunately, the two changes were coincidental and not done by the same group, so the apparent flip flopping was not so apparent to us doing the latter changes.

I so cannot wait until AA are fully library-defined with compiler syntax sugar.

-Steve
_______________________________________________
dmd-beta mailing list
dmd-beta@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-beta
October 16, 2014
On 10/16/2014 01:32 PM, Steven Schveighoffer via dmd-beta wrote:
> I am too actually. From your earlier messages, Martin, I thought you said we would add it as a warning? Has that been done for 2.066.1? I didn't see it in my github traffic.
>
> FWIW, I think a warning is sufficient, even if it only lasts a few versions. As long as silent behavior changes do not happen.

Well I made a pull and we eventually agreed that even a warning would do more harm than good, because one cannot explicitly use the compiler generated toHash and opEquals. A opCmp that wouldn't comply with default equality also required a custom toHash, which makes it even more unlikely that we will break code.

https://github.com/D-Programming-Language/dmd/pull/4070
_______________________________________________
dmd-beta mailing list
dmd-beta@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-beta
October 16, 2014
On Oct 16, 2014, at 3:42 PM, Martin Nowak via dmd-beta <dmd-beta@puremagic.com> wrote:

> On 10/16/2014 01:32 PM, Steven Schveighoffer via dmd-beta wrote:
>> I am too actually. From your earlier messages, Martin, I thought you said we would add it as a warning? Has that been done for 2.066.1? I didn't see it in my github traffic.
>> 
>> FWIW, I think a warning is sufficient, even if it only lasts a few versions. As long as silent behavior changes do not happen.
> 
> Well I made a pull and we eventually agreed that even a warning would do more harm than good, because one cannot explicitly use the compiler generated toHash and opEquals. A opCmp that wouldn't comply with default equality also required a custom toHash, which makes it even more unlikely that we will break code.

I think there is a misunderstanding, the issue is not that opCmp and opEquals are not consistent. The problem is this:

1. As of 2.065, AAs used opCmp and NOT opEquals.
2. Despite the spec's assertion that BOTH opEquals and opCmp should be defined for AA to work properly, the compiler allowed any combination, and let the implementation tell the story.
3. All that was required was to override opCmp and toHash to make AAs work.
4. In 2.066, we no longer use opCmp and use opEquals.
5. Without an error, any code that ONLY defined opCmp (and defined it differently than the default) despite the warnings from the spec will now silently compile, and do the incorrect thing.

Basically, without an error, this is an on-purpose regression for code that has worked as long as AAs have been around (D1 included), and a silent one.

I don't know why this critical error was removed. Yes, any code that relied on the AA implementation details was broken. But it will be viewed as a regression.

The only problem with this change is that at some point prior, an ill-advised requirement for opCmp to be implemented, *even if the default was correct*, was added. This caused people who had valid AA keys with default opCmp to add an opCmp that re-implemented the same thing. Such a thing would automatically trigger the new error!

So it is a messy situation. But I think the gravity of having a silent breaking regression on something that worked for about a decade should outweigh the PR problem of forcing people to add opCmp and then forcing them to remove it. I think we owe those developers the courtesy of identifying the problem before they spend lots of time trying to debug this.

Now, if we wanted to add a warning, that is OK, at least it's not silent. But there should be some indication, IMO, that the implementation and requirements for AA keys have changed.

-Steve
_______________________________________________
dmd-beta mailing list
dmd-beta@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-beta
« First   ‹ Prev
1 2