diff mbox series

Rust: libformat_parser: Lower minimum Rust version to 1.49 (was: Rust: Work around 'error[E0658]: `let...else` statements are unstable')

Message ID 874j3d18ir.fsf@euler.schwinge.ddns.net
State New
Headers show
Series Rust: libformat_parser: Lower minimum Rust version to 1.49 (was: Rust: Work around 'error[E0658]: `let...else` statements are unstable') | expand

Commit Message

Thomas Schwinge Dec. 9, 2024, 9:59 a.m. UTC
Hi!

On 2024-12-05T13:37:13+0100, Arthur Cohen <arthur.cohen@embecosm.com> wrote:
> On 12/4/24 13:35, Thomas Schwinge wrote:
>> On 2024-11-25T11:24:08+0100, Arthur Cohen <arthur.cohen@embecosm.com> wrote:
>>> [...] We had previously done something similar to
>>> adapt to Rust 1.72 when we originally reused the format_args parser:
>>>
>>> https://github.com/Rust-GCC/gccrs/pull/2964

(By the way: I'm confused why I didn't see that one/this existing
GCC/Rust commit, before attempting to fix this myself...  Anyway -- at
least I did a little bit of Rust programming this way.)  ;-)

Anyway:

> In that case, could you instead add the commit mentioned in the PR so we 
> don't need to revert it or change it? But instead we'd just be 
> upstreaming this commit slightly earlier. My thinking is we do the 
> following:
>
> 1. Push commit 039624951f9 upstream, which lowers the required Rust 
> version to 1.49

Pushed to trunk branch commit 67a164eb1da6dba9fb789ae768beebbaa3be37de
"Rust: libformat_parser: Lower minimum Rust version to 1.49", see
attached.

> 2. Send a PR to our github repo with that commit so we merge it in our 
> dev repo as well

That's not actually necessary, I think?  Because:

> 3. This way once we rebase our branch to send it upstream, this commit 
> will just be skipped

This will happen "automatically"?


Grüße
 Thomas


> I think the commit should apply on both our dev repo and our version of 
> gccrs which is upstream.
>
> Feel free to take ownership of it - the changes are extremely similar, 
> the only difference is the use of a trait extension for String::leak() 
> instead of a dedicated leak_string() function.
>
> How do you feel about that? Would that be okay?
>
> Best,
>
> Arthur

Comments

Arthur Cohen Dec. 9, 2024, 11:58 a.m. UTC | #1
Hi Thomas,

On 12/9/24 10:59 AM, Thomas Schwinge wrote:
> Hi!
> 
> On 2024-12-05T13:37:13+0100, Arthur Cohen <arthur.cohen@embecosm.com> wrote:
>> On 12/4/24 13:35, Thomas Schwinge wrote:
>>> On 2024-11-25T11:24:08+0100, Arthur Cohen <arthur.cohen@embecosm.com> wrote:
>>>> [...] We had previously done something similar to
>>>> adapt to Rust 1.72 when we originally reused the format_args parser:
>>>>
>>>> https://github.com/Rust-GCC/gccrs/pull/2964
> 
> (By the way: I'm confused why I didn't see that one/this existing
> GCC/Rust commit, before attempting to fix this myself...  Anyway -- at
> least I did a little bit of Rust programming this way.)  ;-)
> 
> Anyway:
> 
>> In that case, could you instead add the commit mentioned in the PR so we
>> don't need to revert it or change it? But instead we'd just be
>> upstreaming this commit slightly earlier. My thinking is we do the
>> following:
>>
>> 1. Push commit 039624951f9 upstream, which lowers the required Rust
>> version to 1.49
> 
> Pushed to trunk branch commit 67a164eb1da6dba9fb789ae768beebbaa3be37de
> "Rust: libformat_parser: Lower minimum Rust version to 1.49", see
> attached.

Thanks!

>> 2. Send a PR to our github repo with that commit so we merge it in our
>> dev repo as well
> 
> That's not actually necessary, I think?  Because:
> 
>> 3. This way once we rebase our branch to send it upstream, this commit
>> will just be skipped
> 
> This will happen "automatically"?
> 

Right, thank you! That completely skipped my mind.

Best,

Arthur
> 
> Grüße
>   Thomas
> 
> 
>> I think the commit should apply on both our dev repo and our version of
>> gccrs which is upstream.
>>
>> Feel free to take ownership of it - the changes are extremely similar,
>> the only difference is the use of a trait extension for String::leak()
>> instead of a dedicated leak_string() function.
>>
>> How do you feel about that? Would that be okay?
>>
>> Best,
>>
>> Arthur
> 
>
diff mbox series

Patch

From 67a164eb1da6dba9fb789ae768beebbaa3be37de Mon Sep 17 00:00:00 2001
From: Arthur Cohen <arthur.cohen@embecosm.com>
Date: Tue, 23 Apr 2024 14:13:21 +0200
Subject: [PATCH] Rust: libformat_parser: Lower minimum Rust version to 1.49

libgrust/ChangeLog:

	* libformat_parser/Cargo.toml: Change Rust edition from 2021 to 2018.
	* libformat_parser/generic_format_parser/Cargo.toml: Likewise.
	* libformat_parser/generic_format_parser/src/lib.rs: Remove usage of
	then-unstable std features and language constructs.
	* libformat_parser/src/lib.rs: Likewise, plus provide extension trait
	for String::leak.
---
 libgrust/libformat_parser/Cargo.toml          |  2 +-
 .../generic_format_parser/Cargo.toml          |  2 +-
 .../generic_format_parser/src/lib.rs          | 41 ++++++++++++++-----
 libgrust/libformat_parser/src/lib.rs          | 10 +++++
 4 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/libgrust/libformat_parser/Cargo.toml b/libgrust/libformat_parser/Cargo.toml
index 0fcfa3e89a4c..3c214915d31e 100644
--- a/libgrust/libformat_parser/Cargo.toml
+++ b/libgrust/libformat_parser/Cargo.toml
@@ -1,7 +1,7 @@ 
 [package]
 name = "libformat_parser"
 version = "0.1.0"
-edition = "2021"
+edition = "2018"
 
 [workspace]
 
diff --git a/libgrust/libformat_parser/generic_format_parser/Cargo.toml b/libgrust/libformat_parser/generic_format_parser/Cargo.toml
index 34577038cbed..224ad6826ec8 100644
--- a/libgrust/libformat_parser/generic_format_parser/Cargo.toml
+++ b/libgrust/libformat_parser/generic_format_parser/Cargo.toml
@@ -1,7 +1,7 @@ 
 [package]
 name = "generic_format_parser"
 version = "0.1.0"
-edition = "2021"
+edition = "2018"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
diff --git a/libgrust/libformat_parser/generic_format_parser/src/lib.rs b/libgrust/libformat_parser/generic_format_parser/src/lib.rs
index 8062bf9e5cec..e255bf16908f 100644
--- a/libgrust/libformat_parser/generic_format_parser/src/lib.rs
+++ b/libgrust/libformat_parser/generic_format_parser/src/lib.rs
@@ -505,7 +505,7 @@  impl<'a> Parser<'a> {
             }
 
             pos = peek_pos;
-            description = format!("expected `'}}'`, found `{maybe:?}`");
+            description = format!("expected `'}}'`, found `{:?}`", maybe);
         } else {
             description = "expected `'}'` but string was terminated".to_owned();
             // point at closing `"`
@@ -690,12 +690,16 @@  impl<'a> Parser<'a> {
 
         // fill character
         if let Some(&(idx, c)) = self.cur.peek() {
-            if let Some((_, '>' | '<' | '^')) = self.cur.clone().nth(1) {
-                spec.fill = Some(c);
-                spec.fill_span = Some(self.span(idx, idx + 1));
-                self.cur.next();
+            match self.cur.clone().nth(1) {
+                Some((_, '>')) | Some((_, '<')) | Some((_, '^')) => {
+                    spec.fill = Some(c);
+                    spec.fill_span = Some(self.span(idx, idx + 1));
+                    self.cur.next();
+                }
+                _ => {}
             }
         }
+
         // Alignment
         if self.consume('<') {
             spec.align = AlignLeft;
@@ -908,7 +912,11 @@  impl<'a> Parser<'a> {
             );
         }
 
-        found.then_some(cur)
+        if found {
+            Some(cur)
+        } else {
+            None
+        }
     }
 
     fn suggest_format(&mut self) {
@@ -991,8 +999,9 @@  fn find_width_map_from_snippet(
     // If we only trimmed it off the input, `format!("\n")` would cause a mismatch as here we they actually match up.
     // Alternatively, we could just count the trailing newlines and only trim one from the input if they don't match up.
     let input_no_nl = input.trim_end_matches('\n');
-    let Some(unescaped) = unescape_string(snippet) else {
-        return InputStringKind::NotALiteral;
+    let unescaped = match unescape_string(snippet) {
+        Some(u) => u,
+        None => return InputStringKind::NotALiteral,
     };
 
     let unescaped_no_nl = unescaped.trim_end_matches('\n');
@@ -1023,7 +1032,13 @@  fn find_width_map_from_snippet(
 
                 width_mappings.push(InnerWidthMapping::new(pos, width, 0));
             }
-            ('\\', Some((_, 'n' | 't' | 'r' | '0' | '\\' | '\'' | '\"'))) => {
+            ('\\', Some((_, 'n')))
+            | ('\\', Some((_, 't')))
+            | ('\\', Some((_, 'r')))
+            | ('\\', Some((_, '0')))
+            | ('\\', Some((_, '\\')))
+            | ('\\', Some((_, '\'')))
+            | ('\\', Some((_, '\"'))) => {
                 width_mappings.push(InnerWidthMapping::new(pos, 2, 1));
                 let _ = s.next();
             }
@@ -1049,7 +1064,7 @@  fn find_width_map_from_snippet(
                             .as_str()
                             .get(..digits_len)
                             .and_then(|digits| u32::from_str_radix(digits, 16).ok())
-                            .and_then(char::from_u32)
+                            .and_then(std::char::from_u32)
                             .map_or(1, char::len_utf8);
 
                         // Skip the digits, for chars that encode to more than 1 utf-8 byte
@@ -1105,7 +1120,11 @@  fn unescape_string(string: &str) -> Option<string::String> {
     let buf = string::String::from(string);
     let ok = true;
 
-    ok.then_some(buf)
+    if ok {
+        Some(buf)
+    } else {
+        None
+    }
 }
 
 // Assert a reasonable size for `Piece`
diff --git a/libgrust/libformat_parser/src/lib.rs b/libgrust/libformat_parser/src/lib.rs
index 84fac38e224d..0769577740f4 100644
--- a/libgrust/libformat_parser/src/lib.rs
+++ b/libgrust/libformat_parser/src/lib.rs
@@ -5,6 +5,16 @@ 
 
 use std::ffi::CStr;
 
+trait StringLeakExt {
+    fn leak<'a>(self) -> &'a mut str;
+}
+
+impl StringLeakExt for String {
+    fn leak<'a>(self) -> &'a mut str {
+        Box::leak(self.into_boxed_str())
+    }
+}
+
 trait IntoFFI<T> {
     fn into_ffi(self) -> T;
 }
-- 
2.45.2