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

String recursion error after upgrading from 0.26.0 to 0.27.1 #540

Closed
JenChampagne opened this issue Jan 7, 2023 · 8 comments · Fixed by #557
Closed

String recursion error after upgrading from 0.26.0 to 0.27.1 #540

JenChampagne opened this issue Jan 7, 2023 · 8 comments · Fixed by #557
Labels
bug serde Issues related to mapping from Rust types to XML

Comments

@JenChampagne
Copy link

I'm getting a really weird error after upgrading from 0.26.0 to 0.27.1. I've tried to put together a reproduce case, but I'm having difficulty doing so, but I'll give as many details as I can.

The error I get is as follows:

error[E0275]: overflow evaluating the requirement `&mut std::string::String: std::fmt::Write`
  |
  = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`lib`)
  = note: required for `&mut &mut std::string::String` to implement `std::fmt::Write`
  = note: 127 redundant requirements hidden
  = note: required for `&mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut std::string::String` to implement `std::fmt::Write`
  = note: required for `se::element::ElementSerializer<'_, &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut std::string::String>` to implement `serde::Serializer`

For more information about this error, try `rustc --explain E0275`.
error: could not compile `project` due to previous error

I make use of AsRef<T> and Cow<'_, str> in several cases, but I'm not sure that it is the core of the issue since I'm not able to make a reproduce case. I don't understand where the recursion of &mut is coming from. Nothing in my code is explicitly recursive as far as I am aware, and cargo expand does not highlight anything that stands out.

Does the above provide any clues that might point me in the direction of where to look in my code? I will try to again to reproduce, but the repository having the issue is a private one.

@JenChampagne JenChampagne changed the title Upgrading from String recursion error after upgrading from 0.26.0 to 0.27.1 Jan 7, 2023
@Mingun
Copy link
Collaborator

Mingun commented Jan 7, 2023

This is something about serializer. Serializer was rewritten in 0.27.0 and now writes to fmt::Write instead of io::Write. My speculation: probably you try to serialize to a string and there somewhere the unnecessary &mut.

@Clazex
Copy link

Clazex commented Feb 8, 2023

Met similar problem in both 0.27.0 & 0.27.1.
The problem does not persist with quick-xml removed or downgraded to 0.26.0.

Build log with -Z macro-backtrace:

error: reached the recursion limit while instantiating `some_type::_::_serde::ser::impls::<impl Serialize for &some_type::_::_serde::__private::ser::content::Content>::serialize::<se::element::ElementSerializer<'_, &mut &mut ...>>`
   --> <REDACTED>\quick-xml-0.27.1\src\se\element.rs:211:9
    |
211 | /         value.serialize(ElementSerializer {
212 | |             ser: self.ser.new_seq_element_serializer(),
213 | |             key: self.key,
214 | |         })?;
    | |__________^
    |
note: `some_type::_::_serde::ser::impls::<impl Serialize for &'a T>::serialize` defined here
   --> <REDACTED>\serde-1.0.152\src\ser\impls.rs:423:13
    |
415 | /  macro_rules! deref_impl {
416 | |      (
417 | |          $(#[doc = $doc:tt])*
418 | |          <$($desc:tt)+
...   |
423 | |/             fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
424 | ||             where
425 | ||                 S: Serializer,
    | ||______________________________^
...   |
430 | |      };
431 | |  }
    | |__- in this expansion of `deref_impl!`
432 |
433 |    deref_impl!(<'a, T: ?Sized> Serialize for &'a T where T: Serialize);
    |    ------------------------------------------------------------------- in this macro invocation
    = note: the full type name has been written to '<REDACTED>.long-type.txt'

long-type.txt:

some_type::_::_serde::ser::impls::<impl Serialize for &some_type::_::_serde::__private::ser::content::Content>::serialize::<se::element::ElementSerializer<'_, &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut std::string::String>>

There is no &mut in all types in the crate. And increasing recursion limit does not help.

@Mingun
Copy link
Collaborator

Mingun commented Feb 8, 2023

Could you provide the minimal reproducible example of this? What version of serde you are using? cargo tree should show it

@Clazex
Copy link

Clazex commented Feb 8, 2023

here

@Mingun
Copy link
Collaborator

Mingun commented Feb 9, 2023

I shortened the example to:

use serde::Serialize;

#[derive(Serialize)]
pub enum Enum {
    Variant {},
}

#[derive(Serialize)]
pub struct Struct {
    #[serde(flatten)]
    flatten: Enum,
}

pub fn to_xml(data: &Struct) -> Result<String, quick_xml::DeError> {
    quick_xml::se::to_string(&data)
}

Interesting, that removing pub from to_xml fixes the problem, so probably this is a bug in rustc

@Clazex
Copy link

Clazex commented Feb 9, 2023

I guess &mut come from here because if I do quick_xml::se::Serializer::new(std::ffi::OsString::new()), the long-type still ends with String

@Clazex
Copy link

Clazex commented Feb 9, 2023

Update: This panics for Unsupported("cannot serialize map without defined root tag")

fn main() {
    use serde::ser::Serializer;
    quick_xml::se::Serializer::new(String::new())
        .serialize_map(None).unwrap();
}

However the derived Serialize impl for struct or variant that has flattened struct/enum fields always starts with serialize_map call (checked with cargo-expand)
So the new serializer cannot deal with flattening at all 😢

@Mingun
Copy link
Collaborator

Mingun commented Feb 9, 2023

So the new serializer cannot deal with flattening at all cry

No, it is because it was created with new instead of with_root. Probably I should include correct method to the error message. But you're right, flattening works poorly for XML, it is better to avoid it, especially for deserialization. In many cases it will not work due to its design.

@Mingun Mingun added bug serde Issues related to mapping from Rust types to XML labels Feb 9, 2023
Mingun added a commit to Mingun/quick-xml that referenced this issue Feb 18, 2023
@Mingun Mingun linked a pull request Feb 18, 2023 that will close this issue
dralley pushed a commit that referenced this issue Feb 20, 2023
crapStone added a commit to Calciumdibromid/CaBr2 that referenced this issue Mar 16, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [quick-xml](https://github.com/tafia/quick-xml) | dependencies | minor | `0.27.1` -> `0.28.0` |

---

### Release Notes

<details>
<summary>tafia/quick-xml</summary>

### [`v0.28.0`](https://github.com/tafia/quick-xml/blob/HEAD/Changelog.md#&#8203;0280----2023-03-13)

[Compare Source](tafia/quick-xml@v0.27.1...v0.28.0)

##### New Features

-   [#&#8203;541]: (De)serialize specially named `$text` enum variant in [externally tagged]
    enums to / from textual content
-   [#&#8203;556]: `to_writer` and `to_string` now accept `?Sized` types
-   [#&#8203;556]: Add new `to_writer_with_root` and `to_string_with_root` helper functions
-   [#&#8203;520]: Add methods `BytesText::inplace_trim_start` and `BytesText::inplace_trim_end`
    to trim leading and trailing spaces from text events
-   [#&#8203;565]: Allow deserialize special field names `$value` and `$text` into borrowed
    fields when use serde deserializer
-   [#&#8203;568]: Rename `Writter::inner` into `Writter::get_mut`
-   [#&#8203;568]: Add method `Writter::get_ref`
-   [#&#8203;569]: Rewrite the `Reader::read_event_into_async` as an async fn, making the future `Send` if possible.
-   [#&#8203;571]: Borrow element names (`<element>`) when deserialize with serde.
    This change allow to deserialize into `HashMap<&str, T>`, for example
-   [#&#8203;573]: Add basic support for async byte writers via tokio's `AsyncWrite`.

##### Bug Fixes

-   [#&#8203;537]: Restore ability to deserialize attributes that represents XML namespace
    mappings (`xmlns:xxx`) that was broken since [#&#8203;490]
-   [#&#8203;510]: Fix an error of deserialization of `Option<T>` fields where `T` is some
    sequence type (for example, `Vec` or tuple)
-   [#&#8203;540]: Fix a compilation error (probably a rustc bug) in some circumstances.
    `Serializer::new` and `Serializer::with_root` now accepts only references to `Write`r.
-   [#&#8203;520]: Merge consequent (delimited only by comments and processing instructions)
    texts and CDATA when deserialize using serde deserializer. `DeEvent::Text` and
    `DeEvent::CData` events was replaced by `DeEvent::Text` with merged content.
    The same behavior for the `Reader` does not implemented (yet?) and should be
    implemented manually
-   [#&#8203;562]: Correctly set minimum required version of memchr dependency to 2.1
-   [#&#8203;565]: Correctly set minimum required version of tokio dependency to 1.10
-   [#&#8203;565]: Fix compilation error when build with serde <1.0.139

[externally tagged]: https://serde.rs/enum-representations.html#externally-tagged

[#&#8203;490]: tafia/quick-xml#490

[#&#8203;510]: tafia/quick-xml#510

[#&#8203;520]: tafia/quick-xml#520

[#&#8203;537]: tafia/quick-xml#537

[#&#8203;540]: tafia/quick-xml#540

[#&#8203;541]: tafia/quick-xml#541

[#&#8203;556]: tafia/quick-xml#556

[#&#8203;562]: tafia/quick-xml#562

[#&#8203;565]: tafia/quick-xml#565

[#&#8203;568]: tafia/quick-xml#568

[#&#8203;569]: tafia/quick-xml#569

[#&#8203;571]: tafia/quick-xml#571

[#&#8203;573]: tafia/quick-xml#573

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS42LjAiLCJ1cGRhdGVkSW5WZXIiOiIzNS42LjAifQ==-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Co-authored-by: crapStone <crapstone01@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1818
Reviewed-by: crapStone <crapstone@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug serde Issues related to mapping from Rust types to XML
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants