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

Windows.Win32.FileSystem.FILE_ACCESS_FLAGS #253

Closed
kennykerr opened this issue Feb 17, 2021 · 7 comments
Closed

Windows.Win32.FileSystem.FILE_ACCESS_FLAGS #253

kennykerr opened this issue Feb 17, 2021 · 7 comments
Labels
usability Touch-up to improve the user experience for a language projection

Comments

@kennykerr
Copy link
Contributor

The enum values are signed when they should be unsigned since they are flags.

@kennykerr kennykerr added the usability Touch-up to improve the user experience for a language projection label Feb 17, 2021
@sotteson1
Copy link
Contributor

Signed values let you do bitwise operations on them too. I'm not sure that because an enum is flags that it requires it to be unsigned. Is that a requirement for Rust?

@AArnott
Copy link
Member

AArnott commented Feb 17, 2021

Flags has no bearing on whether an enum should be signed, AFAIK. The enum should be the same type as the "constants" had that it now contains.

@kennykerr
Copy link
Contributor Author

My point is that these particular values should be unsigned, not all flag enums necessarily. The values are clearly unsigned, ilSpy even renders them that way by default since they're flags 😉, and the CreateFile parameter that these flags map to is an unsigned parameter.

@kennykerr
Copy link
Contributor Author

As an aside, I'd love to see an example of Windows flags that are intentionally signed. Not saying it doesn't exist, but I'd love to add it to my test cases.

@sotteson1
Copy link
Contributor

Here's one. This one is cppwinrt. Not sure if that matters.
D:\repos\win32metadata\artifacts\installedpackages\Microsoft.Windows.SDK.CPP.10.0.19041.5\c\Include\10.0.19041.0\cppwinrt\winrt\impl\Windows.Devices.Custom.0.h

    enum class IOControlAccessMode : int32_t
    {
        Any = 0,
        Read = 1,
        Write = 2,
        ReadWrite = 3,
    };

@sotteson1
Copy link
Contributor

Another one here:
D:\repos\win32metadata\artifacts\installedpackages\Microsoft.Windows.SDK.CPP.10.0.19041.5\c\Source\10.0.19041.0\ucrt\inc\corecrt_internal_stdio.h

enum : long
{
    // Mode bits:  These bits control the stream mode.  A stream may be in one
    // of three modes:  read mode, write mode, or update (read/write) mode.  At
    // least one of these bits will be set for any open stream.
    //
    // If the stream is open in read mode or write mode, then only the _IOREAD
    // or _IOWRITE bit will be set.
    //
    // If the stream is open in update (read/write) mode, then the _IOUPDATE bit
    // will be set.  Further state must also be tracked for update mode streams.
    // Read and write operations cannot be mixed willy-nilly:  in most cases, a
    // flush or reposition must take place in order to transition between reading
    // and writing.  So, for update mode streams, if the next operation must be
    // a read, the _IOREAD bit is set, and if the next operation must be a write,
    // the _IOWRITE bit is set.
    _IOREAD           = 0x0001,
    _IOWRITE          = 0x0002,
    _IOUPDATE         = 0x0004,

@kennykerr
Copy link
Contributor Author

Those are not flags. IOControlAccessMode is signed and does not offer bitwise operators. Specifically, WinRT enums that represent flags are always unsigned.

It is true that there are, historically, Win32 flags that are signed. Here's a fun example. But I have yet to find one that is intentionally signed. As in there was a valid conscious choice to make it signed beyond historical source compatibility.

Anyway, it doesn't matter. For the sake of originalism 😉 we can just use the flags attribute to determine which are flags in the case of Win32. It would just be simpler if there was more consistency across WinRT and Win32.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
usability Touch-up to improve the user experience for a language projection
Projects
None yet
Development

No branches or pull requests

3 participants