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

Unable to Utilize unsafe {} Blocks as an Expression #98093

Open
HTGAzureX1212 opened this issue Jun 14, 2022 · 10 comments
Open

Unable to Utilize unsafe {} Blocks as an Expression #98093

HTGAzureX1212 opened this issue Jun 14, 2022 · 10 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-grammar Area: The grammar of Rust A-parser Area: The parsing of Rust source code to an AST. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@HTGAzureX1212
Copy link
Contributor

HTGAzureX1212 commented Jun 14, 2022

I tried this code:

fn predicate() -> bool {
    unsafe { std::str::from_utf8_unchecked(&[65]) } == "A"
}

fn main() {}

I expected to see this happen: compilation succeeds.

Instead, this happened: two errors are emitted.

error: expected expression, found `==`
 --> src/main.rs:2:49
  |
2 | unsafe { std::str::from_utf8_unchecked(&[65]) } == "A"
  |                                                 ^^ expected expression

error[E0308]: mismatched types
 --> src/main.rs:2:10
  |
2 | unsafe { std::str::from_utf8_unchecked(&[65]) } == "A"
  |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- help: consider using a semicolon here: `;`
  |          |
  |          expected `()`, found `&str`

Meta

rustc --version --verbose:

rustc 1.63.0-nightly (ca122c7eb 2022-06-13)
binary: rustc
commit-hash: ca122c7ebb3ab50149c9d3d24ddb59c252b32272
commit-date: 2022-06-13
host: x86_64-pc-windows-msvc
release: 1.63.0-nightly
LLVM version: 14.0.5

Even though I know that I can simply wrap the entire unsafe block with parentheses, but this still doesn't seem right when an if block can be combined with boolean expressions in a similar fashion:

if true && if true { true } else { false } { }

I know that the if case seem to be very spaghetti, but I feel like unsafe blocks should exhibit this same behaviour during compilation.

@HTGAzureX1212 HTGAzureX1212 added the C-bug Category: This is a bug. label Jun 14, 2022
@eggyal
Copy link
Contributor

eggyal commented Jun 14, 2022

Works on playground. Could you share the full MCVE?

@HTGAzureX1212
Copy link
Contributor Author

Here is the error I am currently running into in my codebase:
https://github.com/IronVM/IronJVM/blob/a59f3f516e6f1beb2abb0c8d53d7bacb1db81b9b/ironjvm/ironjvm_javautil/src/cf.rs

image

Not sure if it is my problem or not.

@forslund
Copy link

The issue seems to occur in the playground as well if the statement is by itself on a line as in the original example.

But saved or used in an if statement seems to be ok.

@eggyal
Copy link
Contributor

eggyal commented Jun 14, 2022

That issue has nothing to do with unsafe. { 0 } == 0; also fails (playground).

@eggyal
Copy link
Contributor

eggyal commented Jun 14, 2022

As stated in The Rust Reference, under Expression statements (emphasis added):

An expression that consists of only a block expression or control flow expression, if used in a context where a statement is permitted, can omit the trailing semicolon. This can cause an ambiguity between it being parsed as a standalone statement and as a part of another expression; in this case, it is parsed as a statement. The type of ExpressionWithBlock expressions when used as statements must be the unit type.

Accordingly, the block is parsed as a statement and must have unit type—but it does not (giving rise to the first error). The == is then parsed as the start of the following statement, which is a syntax error (giving rise to the second error).

I therefore think this is by design and not an issue.

@rustbot label +invalid

@rustbot
Copy link
Collaborator

rustbot commented Jun 14, 2022

Error: Label invalid can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

@HTGAzureX1212
Copy link
Contributor Author

HTGAzureX1212 commented Jun 15, 2022

Consider that

let foo = unsafe { /* some stuff here */ };
foo == /* something */

is allowed, is it wrong to consider that the unsafe {} block is in fact an expression, given that it is "assigned to a let binding"?

Additionally, I am quite sure that it is common for programmers to flatten the condition, writing something like

unsafe { /* some stuff here */ } == /* something */

for the return value of a function, etc.

Is my interpretation incorrect or am I misunderstanding something? If so I am more than happy to be corrected in this scenario.

@eggyal
Copy link
Contributor

eggyal commented Jun 15, 2022

The block can be an expression, but in some contexts it's ambiguous whether it's an expression or a statement and in those cases Rust opts for it to be a statement. Basically, Rust parses the code as if you had written a ; immediately after the block.

If the rule in such ambiguous situations was changed so that blocks were expressions rather than statements, then a lot existing Rust code would no longer compile. Consider for example:

unsafe {
    foo()
}

bar();

Even worse, this same ambiguity would also then be resolved differently for control flow blocks such as if, where it is extremely common to omit the ; statement terminator:

if test {
   foo()
}

bar();

Perhaps there is room to argue that where such blocks are followed by an infix operator the ambiguity should be resolved in favour of the block being an expression, but that could introduce quite a bit more complexity... it may well have been discussed before elsewhere, I'll have a hunt around. On reflection, that won't work because & in such cases would itself be ambiguous: is it the infix "and" operator or the prefix "borrow" operator? Likewise - (infix subtraction vs prefix negation) and * (infix multiplication vs prefix dereference): resolving these ambiguities in favour of being infix operators would break existing code; whereas resolving them in favour of being prefix operators would perhaps be even more surprising to users than this issue.

@eggyal
Copy link
Contributor

eggyal commented Jun 15, 2022

resolving them in favour of being prefix operators would perhaps be even more surprising to users than this issue.

I've opened https://internals.rust-lang.org/t/expression-vs-statement-ambiguities/16823 to discuss this, potentially with a view to a language RFC if the idea is well received.

@ChrisDenton ChrisDenton added the needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. label Jul 16, 2023
@fmease fmease added A-grammar Area: The grammar of Rust T-lang Relevant to the language team, which will review and decide on the PR/issue. C-discussion Category: Discussion or questions that doesn't represent real issues. A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. C-bug Category: This is a bug. C-discussion Category: Discussion or questions that doesn't represent real issues. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jan 25, 2024
@fmease
Copy link
Member

fmease commented Jan 25, 2024

Marking this as a diagnostics issue. From the perspective of the Rust grammar, there's nothing wrong here but ideally the parser would provide actionable hints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-grammar Area: The grammar of Rust A-parser Area: The parsing of Rust source code to an AST. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants