Skip to content

std.math: deprecate backwards compatibility with Phobos1#2636

Merged
quickfur merged 2 commits intodlang:masterfrom
9il:part1
Oct 25, 2014
Merged

std.math: deprecate backwards compatibility with Phobos1#2636
quickfur merged 2 commits intodlang:masterfrom
9il:part1

Conversation

@9il
Copy link
Member

@9il 9il commented Oct 24, 2014

is____ functions in std.math

Depends on dlang/dmd#4090

@quickfur
Copy link
Member

Looks like there's a merge conflict; please rebase.

@9il 9il mentioned this pull request Oct 24, 2014
@quickfur
Copy link
Member

Auto-merge toggled on

@9il
Copy link
Member Author

9il commented Oct 24, 2014

Test failed: expected rc == 0, exited with rc == 1

What does it mean?

@IgorStepanov
Copy link
Contributor

@9il
rc == return code. You need to scroll above to see the actual error.
Win32 fail

==============================

Test failed: 

expected:

----

fail_compilation\fail80_m64.d(28): Error: cannot implicitly convert expression ("progress_rem") of type string to ulong

fail_compilation\fail80_m64.d(29): Error: cannot implicitly convert expression ("redo") of type string to ulong

----

actual:

----

C:\cygwin\home\Administrator\sandbox\d\d-tester\client\pull-1230416-Win_32\dmd\src\..\..\phobos\std\format.d(33): Deprecation: alias std.math.isnan is deprecated - Phobos1 math functions are deprecated, use isNaN

fail_compilation\fail80_m64.d(28): Error: cannot implicitly convert expression ("progress_rem") of type string to ulong

fail_compilation\fail80_m64.d(29): Error: cannot implicitly convert expression ("redo") of type string to ulong

----

Win64 fail

Z:\Administrator\sandbox\d\d-tester\client\pull-1230429-Win_64\dmd\src\..\..\phobos\std\format.d(33): Deprecation: alias std.math.isnan is deprecated - Phobos1 math functions are deprecated, use isNaN



compilable\diag10768.d(36): Deprecation: implicitly overriding base class method diag10768.Frop.frop with diag10768.Foo.frop deprecated; add 'override' attribute







==============================

Test failed: 

expected:

----

compilable\diag10768.d(36): Deprecation: implicitly overriding base class method diag10768.Frop.frop with diag10768.Foo.frop deprecated; add 'override' attribute

----

actual:

----

Z:\Administrator\sandbox\d\d-tester\client\pull-1230429-Win_64\dmd\src\..\..\phobos\std\format.d(33): Deprecation: alias std.math.isnan is deprecated - Phobos1 math functions are deprecated, use isNaN

compilable\diag10768.d(36): Deprecation: implicitly overriding base class method diag10768.Frop.frop with diag10768.Foo.frop deprecated; add 'override' attribute

----

@quickfur
Copy link
Member

Looks like one of the tests is failing, probably the output doesn't exactly match the expected output.

@quickfur
Copy link
Member

P.S. Ah, looks like you have to update some of the dmd test files to either use the new function names, or update the expected output to include the new deprecation messages.

@quickfur
Copy link
Member

Auto-merge toggled off

@9il
Copy link
Member Author

9il commented Oct 24, 2014

Ok, probably I understand. I need to rewrite Phobos1 tests =)

@quickfur
Copy link
Member

Hmm, the autotester is passing now. Was there a corresponding dmd PR that got merged?

@9il
Copy link
Member Author

9il commented Oct 25, 2014

isnan was called from std.format, not from test: See 9il@cb8d75e .
dlang/dmd#4090 it is compiles or not compiles test, and it is compiles with deprecated feature.

@quickfur
Copy link
Member

Ah, makes sense. Thanks! LGTM.

@quickfur
Copy link
Member

Auto-merge toggled on

quickfur pushed a commit that referenced this pull request Oct 25, 2014
std.math: deprecate backwards compatibility with Phobos1
@quickfur quickfur merged commit 3ee0f00 into dlang:master Oct 25, 2014
@9il 9il deleted the part1 branch October 25, 2014 20:27
@WalterBright
Copy link
Member

These name changes have to stop. They're the poster boy for churn giving an "illusion of progress".

@mihails-strasuns
Copy link

Never underestimate learning curve damage of inconsistent naming.

@WalterBright
Copy link
Member

Never underestimate learning curve damage of inconsistent naming.

I don't buy that. There's nothing "intuitive" or "consistent" with isNaN instead of isnan. (Googling it shows that isnan, isNaN, ISNAN, is-nan are common spellings in various languages.) There is, however, considerable ongoing damage with "my working code from 1 year ago won't compile anymore because of gratuitous incompatible changes."

We ought to be working on things that matter, like making the documentation for isnan better, than just more wretched bikeshedding and breaking.

@yebblies
Copy link
Contributor

I don't buy that. There's nothing "intuitive" or "consistent" with isNaN instead of isnan.

Of course there is, because we have a naming style and isnan doesn't follow it.

@WalterBright
Copy link
Member

Of course there is, because we have a naming style and isnan doesn't follow it.

Then it should have been isNan. In any case, it's been isnan for 14 years. Breaking long standing, working, correct code like this for no good reason just annoys people. It wastes our time and every user's time. It must stop. I'd be shocked if isnan confused anybody.

@mihails-strasuns
Copy link

In any case, it's been isnan for 14 years

It was isNaN for many years. isnan as simply an alias hanging around undocumented and frustrating newcomers when they notice is used (is it different from "official" isNaN? why it is used?). All this PR does is finally marking it as deprecated to give clear message that it is legacy. It breaks no code at all and clarifies potential confusion. We need more cleanup like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants