Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Struct std.mmfile.MmFile? #9951

Open
dlangBugzillaToGithub opened this issue Feb 12, 2013 · 3 comments
Open

Struct std.mmfile.MmFile? #9951

dlangBugzillaToGithub opened this issue Feb 12, 2013 · 3 comments

Comments

@dlangBugzillaToGithub
Copy link

bearophile_hugs reported this on 2013-02-12T17:45:01Z

Transfered from https://issues.dlang.org/show_bug.cgi?id=9501

CC List

  • issues.dlang (@jmdavis)
  • qs.il.paperinik

Description

Maybe it's a good idea for std.mmfile.MmFile to become a struct like std.stdio.File, so the file is closed when this struct goes out of scope and the reference count it keeps is zero.
@dlangBugzillaToGithub
Copy link
Author

issues.dlang (@jmdavis) commented on 2013-02-12T17:54:43Z

Yes, std.mmfile.MmFile should be a struct. It doesn't use polymorphism and has no need to be a class. But making it a struct would break all code that uses it, so we're arguably stuck with it being the way that it is.

@dlangBugzillaToGithub
Copy link
Author

bearophile_hugs commented on 2013-02-12T18:19:08Z

(In reply to comment #1)
> Yes, std.mmfile.MmFile should be a struct. It doesn't use polymorphism and has
> no need to be a class.

And maybe it doesn't have an explicit close() method, I don't know why, maybe for safety in accessing the memory mapped bytes.


> But making it a struct would break all code that uses
> it, so we're arguably stuck with it being the way that it is.

There are always acceptable deprecation paths, if we want them. Like adding the struct to std.mmfile with another name ("RcMmFile"?) and deprecating std.mmfile.MmFile for two years.

@dlangBugzillaToGithub
Copy link
Author

bearophile_hugs commented on 2013-02-12T20:05:43Z

Mostly unrelated, about std.mmfile: I think in the std.mmfile module this line (line 270) should offer a string message too when the file is not found:

errnoEnforce(false);


And I think switches like this one should become "final":

            switch (mode)
            {
            case Mode.read:
...
            default:
                assert(0);
            }

@LightBender LightBender removed the P4 label Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants