Skip to content

Conversation

fmease
Copy link
Member

@fmease fmease commented Dec 13, 2024

&raw denotes a normal/non-raw borrow of the path raw, not the start of raw borrow expr since it's not followed by either const or mut. Ensure this (and variants) will never regress!

When I saw the open diagnostic issue #133231 (better parse error (recovery) on &raw <expr>), it made me think that we have to make sure that we will never commit too early/overzealously(†) when encountering the sequence &raw, even during parse error recovery!

Modifying the parser to eagerly treat &raw as the start of a raw borrow expr only lead to a single UI test failing, namely tests/ui/enum-discriminant/ptr_niche.rs. However, this is just coincidental — it doesn't intentionally test this edge case of the grammar.


†: With "eager" I mean something like:

diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs
index 0904a42d8a4..68d690fd602 100644
--- a/compiler/rustc_parse/src/parser/expr.rs
+++ b/compiler/rustc_parse/src/parser/expr.rs
@@ -873,11 +873,16 @@ fn error_remove_borrow_lifetime(&self, span: Span, lt_span: Span) {

     /// Parse `mut?` or `raw [ const | mut ]`.
     fn parse_borrow_modifiers(&mut self) -> (ast::BorrowKind, ast::Mutability) {
-        if self.check_keyword(kw::Raw) && self.look_ahead(1, Token::is_mutability) {
+        if self.eat_keyword(kw::Raw) {
             // `raw [ const | mut ]`.
-            let found_raw = self.eat_keyword(kw::Raw);
-            assert!(found_raw);
-            let mutability = self.parse_const_or_mut().unwrap();
+            let mutability = self.parse_const_or_mut().unwrap_or_else(|| {
+                let span = self.prev_token.span;
+                self.dcx().emit_err(ExpectedMutOrConstInRawBorrowExpr {
+                    span,
+                    after_ampersand: span.shrink_to_hi(),
+                });
+                ast::Mutability::Not
+            });
             (ast::BorrowKind::Raw, mutability)
         } else {
             // `mut?`

r? compiler

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 13, 2024
@fmease fmease force-pushed the amp-raw-is-a-normal-borrow branch from 6f81522 to f1d2a6a Compare December 13, 2024 17:36
@Noratrieb
Copy link
Member

image
heh, GitHub's syntax highlighting does not like it

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Dec 13, 2024

📌 Commit f1d2a6a has been approved by Noratrieb

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 13, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 14, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#134252 (Fix `Path::is_absolute` on Hermit)
 - rust-lang#134254 (Fix building `std` for Hermit after `c_char` change)
 - rust-lang#134255 (Update includes in `/library/core/src/error.rs`.)
 - rust-lang#134261 (Document the symbol Visibility enum)
 - rust-lang#134262 (Arbitrary self types v2: adjust diagnostic.)
 - rust-lang#134265 (Rename `ty_def_id` so people will stop using it by accident)
 - rust-lang#134271 (Arbitrary self types v2: better feature gate test)
 - rust-lang#134274 (Add check-pass test for `&raw`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4efa98c into rust-lang:master Dec 14, 2024
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 14, 2024
Rollup merge of rust-lang#134274 - fmease:amp-raw-is-a-normal-borrow, r=Noratrieb

Add check-pass test for `&raw`

`&raw` denotes a normal/non-raw borrow of the path `raw`, not the start of raw borrow since it's not followed by either `const` or `mut`. Ensure this (and variants) will never regress!

When I saw the open diagnostic issue rust-lang#133231 (better parse error (recovery) on `&raw <expr>`), it made me think that we have to make sure that we will never commit too early/overzealously(†) when encountering the sequence `&raw`, even during parse error recovery!

Modifying the parser to eagerly treat `&raw` as the start of a raw borrow expr only lead to a single UI test failing, namely [tests/ui/enum-discriminant/ptr_niche.rs](https://github.com/rust-lang/rust/blob/4847d6a9d07d4be9ba3196f6ad444af2d7bdde72/tests/ui/enum-discriminant/ptr_niche.rs). However, this is just coincidental — it didn't *intentionally* test this edge case of the grammar.

---

†: With "eager" I mean something like:

```patch
diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs
index 0904a42..68d690fd602 100644
--- a/compiler/rustc_parse/src/parser/expr.rs
+++ b/compiler/rustc_parse/src/parser/expr.rs
`@@` -873,11 +873,16 `@@` fn error_remove_borrow_lifetime(&self, span: Span, lt_span: Span) {

     /// Parse `mut?` or `raw [ const | mut ]`.
     fn parse_borrow_modifiers(&mut self) -> (ast::BorrowKind, ast::Mutability) {
-        if self.check_keyword(kw::Raw) && self.look_ahead(1, Token::is_mutability) {
+        if self.eat_keyword(kw::Raw) {
             // `raw [ const | mut ]`.
-            let found_raw = self.eat_keyword(kw::Raw);
-            assert!(found_raw);
-            let mutability = self.parse_const_or_mut().unwrap();
+            let mutability = self.parse_const_or_mut().unwrap_or_else(|| {
+                let span = self.prev_token.span;
+                self.dcx().emit_err(ExpectedMutOrConstInRawBorrowExpr {
+                    span,
+                    after_ampersand: span.shrink_to_hi(),
+                });
+                ast::Mutability::Not
+            });
             (ast::BorrowKind::Raw, mutability)
         } else {
             // `mut?`
```

---

r? compiler
@rustbot rustbot added this to the 1.85.0 milestone Dec 14, 2024
@fmease fmease deleted the amp-raw-is-a-normal-borrow branch December 14, 2024 10:04
flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 15, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#134252 (Fix `Path::is_absolute` on Hermit)
 - rust-lang#134254 (Fix building `std` for Hermit after `c_char` change)
 - rust-lang#134255 (Update includes in `/library/core/src/error.rs`.)
 - rust-lang#134261 (Document the symbol Visibility enum)
 - rust-lang#134262 (Arbitrary self types v2: adjust diagnostic.)
 - rust-lang#134265 (Rename `ty_def_id` so people will stop using it by accident)
 - rust-lang#134271 (Arbitrary self types v2: better feature gate test)
 - rust-lang#134274 (Add check-pass test for `&raw`)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants