Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

core.stdc.stdlib: annotate with 'scope'#1749

Merged
andralex merged 1 commit intodlang:masterfrom
WalterBright:scope-stdlib
Feb 13, 2017
Merged

core.stdc.stdlib: annotate with 'scope'#1749
andralex merged 1 commit intodlang:masterfrom
WalterBright:scope-stdlib

Conversation

@WalterBright
Copy link
Member

For -dip1000. Also corrects problem where strtold can be used to convert pointer to immutable to pointer to mutable.

@andralex
Copy link
Member

Auto-merge toggled on

// strtold exists starting from VS2013, so we make this a template to avoid link errors
///
real strtold()(in char* nptr, char** endptr)
real strtold()(scope inout(char)* nptr, inout(char)** endptr)
Copy link
Member

@MartinNowak MartinNowak Feb 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fails on Win32_64 w/

std\conv.d(3163): Error: template core.stdc.stdlib.strtold cannot deduce function from argument types !()(string, typeof(null)), candidates are:
..\druntime\import\core\stdc\stdlib.d(109):        core.stdc.stdlib.strtold()(scope inout(char)* nptr, inout(char)** endptr)

c_long atol(scope const char* nptr);
///
long atoll(in char* nptr);
long atoll(scope const char* nptr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was circulated for quite a while that in is supposed to mean scope const, in the compiler it was always just const but lots of people have used it as scope const. Have you thought about actually changing the semantic of in?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andralex andralex merged commit 9be31c4 into dlang:master Feb 13, 2017
float strtof(scope inout(char)* nptr, scope inout(char)** endptr);
///
c_long strtol(in char* nptr, char** endptr, int base);
c_long strtol(scope inout(char)* nptr, scope inout(char)** endptr, int base);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to have broken the travis builds for dmd, e.g. https://travis-ci.org/dlang/dmd/jobs/201062823#L5271

The change seems ok, but it breaks code that tried to work around the wrong annotation. The problem with fixing dmd is that it is built with both dmd 2.073 and git-master...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message:

mars.d(420): Error: function core.stdc.stdlib.strtol (scope inout(char)* nptr, scope inout(char)** endptr, int base) is not callable using argument types (const(char)*, char**, int)

This is the problem I was trying to correct. The old declaration allowed a const pointer to be converted to a mutable one. Any code that relies on such needs to be fixed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. dmd doesn't rely on the old signature, it had to workaround it with a cast. See dlang/dmd#6539

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect we need to readd the old signatures as deprecated, otherwise every other project will fail to compile.

@WalterBright WalterBright deleted the scope-stdlib branch February 13, 2017 22:48
@MartinNowak
Copy link
Member

It says @system at the top of the module.
https://github.com/WalterBright/druntime/blob/fe160cb22d035f21f5bb39ec86553728bec45f91/src/core/stdc/stdlib.d#L30
So all of those APIs are marked as unsafe and the compiler won't check scope for them anyhow :o.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants