Skip to content

Conversation

denis-sh
Copy link
Contributor

@denis-sh denis-sh commented Apr 3, 2013

Issue URL: http://d.puremagic.com/issues/show_bug.cgi?id=7648

IMHO, this note must be added as the issue is unexpected and may result in big troubles for someone.

@denis-sh
Copy link
Contributor Author

denis-sh commented Apr 3, 2013

An apology: I did my best to use _w* C functions but underlying bug in C runtime prevents it from work correct. I also tried to rewrite the module using WinAPI but had to abandon the work (temporary I hope) because of lack of time.

@denis-sh
Copy link
Contributor Author

Two weeks ping.

@ghost
Copy link

ghost commented Apr 17, 2013

If there's an issue with Unicode in File, shouldn't we try to fix the bug?

@denis-sh
Copy link
Contributor Author

shouldn't we try to fix the bug?

As I told, I did my best.

12 years of having silently incorrectly working in some case which are relatively hard to predict in debugging main file IO functionality is inexcusable.

The main issue it that we use C runtime which is full of undocumented implementation defined features and even experienced coders often can't do very simple things with C runtime (see Phobos for examples of how one mustn't use C runtime and see druntime for examples of how one must use C runtime).

@dnadlinger
Copy link
Contributor

Pinging @andralex.

@denis-sh
Copy link
Contributor Author

Pinging Andrei The Destructor

Circle dependency detected! Andrei doesn't code for Windows and asks others to revise such pulls.

@andralex
Copy link
Member

Yes, I wouldn't have much to comment. On the face of it, if it's a documentation note that makes technical sense, I'm all for it. I'll merge now, please amend as necessary.

andralex added a commit that referenced this pull request Apr 17, 2013
Add note about Issue 7648 to `std.stdio.File`.
@andralex andralex merged commit 176976a into dlang:master Apr 17, 2013
@dnadlinger
Copy link
Contributor

I was just worried about basically telling people not to use File on Windows.

@DmitryOlshansky
Copy link
Member

Better then letting them shoot their feet off I guess.
All in all I expect this issue to be specific for DMC run-time, isn't it?
MSVC 64-bit run-time might work then.

@denis-sh
Copy link
Contributor Author

MSVC 64-bit run-time might work then.

AFAIK, MSVC doesn't have such issues. So it will work if my (never tested) fix is correct.

@DmitryOlshansky
Copy link
Member

What I believe that means is that we basically need a proper (modern and open-source) run-time for Win32 then.
I've been toying with one myself but aside from basics I haven't gone far. The plan was to make a feather-weight run-time by doing a very thing wrapping of WinAPI. Thus malloc was just mapped to HeapAlloc and so on.

In the long run I think 32-bit Windows systems are here to stay for a few years at least (with Windows on ARM devices if not x86) even if the most of the "market" is going to 64 bit.

@ghost
Copy link

ghost commented Apr 18, 2013

Isn't it better to use e.g. transcode from std.encoding internally in File on Windows rather than saying "don't use File"?

@denis-sh
Copy link
Contributor Author

Isn't it better to use e.g. transcode from std.encoding internally in File on Windows rather than saying "don't use File"?

You are proposing to make failures even less debugable as it will just fix some random cases where it is possible to represent a UTF8 string via CP_ACP.

@ghost
Copy link

ghost commented Apr 18, 2013

You are proposing to make failures even less debugable as it will just fix some random cases where it is possible to represent a UTF8 string via CP_ACP.

And how is your comment on the function debuggable? And what is the alternative the user should look into if they want to use files?

AFAIK transcode should throw an exception if a string can't be represented in some encoding.

@denis-sh
Copy link
Contributor Author

And how is your comment on the function debuggable?

One have to read documentation about stuff he use. So he will not hit the issue.

And what is the alternative the user should look into if they want to use files?

Written in the note.

AFAIK transcode should throw an exception if a string can't be represented in some encoding.

I mean you propose to increase amount of cases the function will work correct by accident. It is unacceptable. If the function is fundamentally broken it must be documented appropriately and not used.

@ghost
Copy link

ghost commented Apr 18, 2013

I mean you propose to increase amount of cases the function will work correct by accident.

I proposed we call transcode internally, which (if I'm not mistaken) will throw an exception if a string can't be converted to another encoding. Where's the accident?

Written in the note.

Thanks, I didn't notice. Note however that the generated doc line is a bit broken, this is what I'm getting:

One can use std.file.read/std.file.write/File">std.stream.File instead. 

If the function is fundamentally broken it must be documented appropriately and not used.

It should be fixed, people have been using File for ages. Something as fundamental as this File structure should work in a standard library.

@denis-sh
Copy link
Contributor Author

proposed we call transcode internally, which (if I'm not mistaken) will throw an exception if a string can't be converted to another encoding. Where's the accident?

I mean the case passed string is convertible is an accident.

Note however that the generated doc line is a bit broken, this is what I'm getting:

Works for me.

It should be fixed,

You are welcome!

people have been using File for ages.

Doesn't matter. std.stdio.File was broken for ages in different ways. If one used it, he didn't care about his program stability.

Something as fundamental as this File structure should work in a standard library.

Being a constant and happy D user, I can tell that it isn't an issue at all in current language state as there are more serious problems (like the fact you program can accidentally become unable to build because of any change or wrong code bugs).

@ghost
Copy link

ghost commented Apr 19, 2013

I mean the case passed string is convertible is an accident.

Using transcode internally will not change this. It will only ensure an exception is thrown if it can't be converted.

The real accident is passing a string which isn't converted and ends up being interpreted as something else by the OS, possibly leading to things like opening the wrong file (or worse, deleting a file if the File was opened with the w switch).

@alexrp
Copy link
Contributor

alexrp commented Apr 19, 2013

In the long run I think 32-bit Windows systems are here to stay for a few years at least (with Windows on ARM devices if not x86) even if the most of the "market" is going to 64 bit.

Right, but Windows on ARM will not use DMC's runtime.

@DmitryOlshansky
Copy link
Member

In the long run I think 32-bit Windows systems are here to stay for a few years at least (with Windows on ARM >>devices if not x86) even if the most of the "market" is going to 64 bit.
Right, but Windows on ARM will not use DMC's runtime.

Hence what I told above all was:

What I believe that means is that we basically need a proper (modern and open-source) run-time for Win32 then.

I know there are some shortcomings with using M$ run-time such as not being able to control how they change it (nor easily update towards newer versions). Even current Win64 version could cause us some grief later on.

I'd be crazy enough to suggest basically taking Musl (UTF-8 only smallish and neat C run-time for Linux) and porting the "syscall" part over to WinAPI. And I'd gladly lend a hand anyone trying this ;)

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.

5 participants