Skip to content

Fix Issue 23639 - Casting to shared not allowed with -preview=nosharedaccess #14836

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.

Sign up for GitHub

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

Merged
RazvanN7 merged 1 commit into from
Jan 20, 2023
Merged

Fix Issue 23639 - Casting to shared not allowed with -preview=nosharedaccess #14836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion compiler/src/dmd/expressionsem.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13185,7 +13185,9 @@ bool checkSharedAccess(Expression e, Scope* sc, bool returnRef = false)
// Error by default
bool visit(Expression e)
{
if (e.type.isShared())
// https://issues.dlang.org/show_bug.cgi?id=23639
// Should be able to cast(shared)
if (!e.isCastExp() && e.type.isShared())
Copy link
Contributor

@thewilsonator thewilsonator Jan 18, 2023

Choose a reason for hiding this comment

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

what does this do to const shared?

Copy link
Contributor Author

@RazvanN7 RazvanN7 Jan 19, 2023

Choose a reason for hiding this comment

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

If you cast to const shared it just performs the cast.

Copy link
Contributor

@thewilsonator thewilsonator Jan 19, 2023

Choose a reason for hiding this comment

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

please augment the test case then.

Copy link
Contributor Author

@RazvanN7 RazvanN7 Jan 19, 2023

Choose a reason for hiding this comment

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

Why? Before this patch the cast would not have been allowed, but now it is allowed. Note that the code would fail because you cannot convert a const(shared) to a shared.

return sharedError(e);
return false;
}
Expand Down
12 changes: 12 additions & 0 deletions compiler/test/compilable/test23639.d
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// https://issues.dlang.org/show_bug.cgi?id=23639

// REQUIRED_ARGS: -preview=nosharedaccess

class T {}

shared(T) allocClassMem()
{
void *p;
// assume p is allocated here
return cast(shared(T))p;
Copy link
Contributor

@thewilsonator thewilsonator Jan 18, 2023

Choose a reason for hiding this comment

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

what is the @safety of this operation?

Copy link
Contributor Author

@RazvanN7 RazvanN7 Jan 19, 2023

Choose a reason for hiding this comment

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

Error: cast from void* to shared(T) not allowed in safe code

The expected @safety level - @System.

Copy link
Contributor

@thewilsonator thewilsonator Jan 19, 2023

Choose a reason for hiding this comment

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

and if typeof(p) == T?

Copy link
Contributor Author

@RazvanN7 RazvanN7 Jan 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The modified code has nothing to do with the safety checks. Casts to differently qualified types remain unsafe, that part is not modified.

Copy link
Contributor

@thewilsonator thewilsonator Jan 19, 2023

Choose a reason for hiding this comment

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

-preview=nosharedaccess is all about safety. even if the end result is that the code must be system. Perhaps I was thinking things wrong, but it seems to me that the only reason why you would want to do this with -preview=nosharedaccess is because you have a unique reference to an object you then want to share. Its fine if that needs to be trusted, then please disregard this request for changes, but please do document this discussion in the test case.

Copy link
Contributor Author

@RazvanN7 RazvanN7 Jan 20, 2023

Choose a reason for hiding this comment

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

Yes, a prime example of this is memory allocation. You allocate heap memory which is naturally shared among threads and you want to express that. What other way of casting the mallocd memory to shared is there?

}
23 changes: 23 additions & 0 deletions compiler/test/fail_compilation/shared.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,3 +236,26 @@ struct BitRange
this.bits++;
}
}

/*
TEST_OUTPUT:
---
fail_compilation/shared.d(3004): Error: cast from `void*` to `shared(int*)` not allowed in safe code
fail_compilation/shared.d(3005): Error: cast from `void*` to `shared(const(int*))` not allowed in safe code
fail_compilation/shared.d(3008): Error: cast from `shared(void*)` to `int*` not allowed in safe code
fail_compilation/shared.d(3009): Error: cast from `shared(void*)` to `shared(const(int*))` not allowed in safe code
---
*/

#line 3000

void test_casting_safe() @safe
{
void *p;
auto t1 = cast(shared(int*))p;
auto t2 = cast(const(shared(int*)))p;

shared void* s;
auto x1 = cast(int*)s;
auto x2 = cast(const(shared(int*)))s;
}