Skip to content

Issue 13537: forbid modifying overlapping mutable/immutable fields in @safe code#5467

Closed
quickfur wants to merge 2 commits intodlang:masterfrom
quickfur:issue13537
Closed

Issue 13537: forbid modifying overlapping mutable/immutable fields in @safe code#5467
quickfur wants to merge 2 commits intodlang:masterfrom
quickfur:issue13537

Conversation

@quickfur
Copy link
Member

This implementation is a compromise; it only prevents immutable breakage in @safe code, but still allows this in @system code, because forbidding it in @system code would break existing code such as std.typecons.Rebindable that use a union to reinterpret type modifiers.

…lds in @safe code

This implementation is a compromise; it only prevents immutable breakage
in @safe code, but still allows this in @System code, because forbidding
it in @System code would break existing code such as
std.typecons.Rebindable that use a union to reinterpret type modifiers.
@dlang-bot
Copy link
Contributor

Fix Bugzilla Description
13537 Unions may break immutability

@JakobOvrum
Copy link

Does this help with memory safety?

@quickfur
Copy link
Member Author

Not directly, no. But potentially it might prevent an illegal attempt to modify read-only memory.

@MetaLang
Copy link
Member

Does this also work with nested anonymous unions?

@quickfur
Copy link
Member Author

It seems to work:

struct S
{
    union
    {   
        struct
        {   
            int x;
            immutable int y;
        }
        struct
        {   
            int z;
            int w;
        }
    }
}
void hun() @safe
{
    S s;
    s.w = 1;
}

Compiler output:

test/fail_compilation/fail13537.d(49): Error: field s.w cannot be modified in @safe code because it overlaps mutable and immutable

Changing the last line to s.z = 1; makes the compile error go away.

Did you have a specific test case in mind?

@MetaLang
Copy link
Member

No, I just tried to fix this problem before and I couldn't figure out how to make it work for anonymous unions.

@quickfur
Copy link
Member Author

ping @WalterBright @yebblies
Does this PR make sense, or the approach is wrong and we should just close this?

@yebblies
Copy link
Contributor

Is overlapping const and immutable allowed?

@quickfur
Copy link
Member Author

Yes.

@yebblies
Copy link
Contributor

It looks to me like it isn't with this patch... Could I get a test case added for it?

@quickfur
Copy link
Member Author

Added a test case... though I'm not sure how exactly to test this, since const/immutable by definition means you can't modify anything, so it's not as if you could test that assigning to the const field is allowed.

@WalterBright
Copy link
Member

This should be extended so that using unions to bypass qualifiers shared, const, immutable, and inout should not be allowed in safe code.

@quickfur
Copy link
Member Author

Then what should we do about std.typecons.Rebindable? Make it @trusted?

@WalterBright
Copy link
Member

Since it cannot be mechanically verified as safe, then yes, it has to be trusted (if it is actually safe).

@WalterBright
Copy link
Member

Are you planning on moving forward with this or should I take it over?

@quickfur
Copy link
Member Author

quickfur commented Jul 6, 2016

I'm short on time to work on this right now, please take this over so that it doesn't get delayed any longer. Thanks!

@WalterBright
Copy link
Member

Rebooted as #5940

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