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

Correctly process flatten fields in enum variants #2567

Merged
merged 4 commits into from
Aug 11, 2024

Conversation

Mingun
Copy link
Contributor

@Mingun Mingun commented Aug 10, 2023

Store has_flatten mark in the Variant attributes and use it instead of attr::Container::has_flatten() in methods that can be called for variants.

Fixes #1904, fixes #2565, fixes #2792

@Mingun
Copy link
Contributor Author

Mingun commented Sep 17, 2023

@dtolnay , when you could look at this? This bugs becomes annoying...

@Mingun
Copy link
Contributor Author

Mingun commented Mar 3, 2024

@dtolnay, @oli-obk, can you give some feedback?

@Mingun Mingun force-pushed the fix-2565 branch 2 times, most recently from cfde011 to 2b88ec0 Compare July 23, 2024 17:56
Mingun and others added 3 commits August 9, 2024 19:53
failures (1):
    regression::issue2565::simple_variant
Currently panics in derive:

error: proc-macro derive panicked
    --> test_suite\tests\test_annotations.rs:2386:25
     |
2386 |     #[derive(Serialize, Deserialize, PartialEq, Debug)]
     |                         ^^^^^^^^^^^
     |
     = help: message: assertion failed: !cattrs.has_flatten()

error: proc-macro derive panicked
  --> test_suite\tests\regression\issue1904.rs:57:10
   |
57 | #[derive(Deserialize)]
   |          ^^^^^^^^^^^
   |
   = help: message: assertion failed: !cattrs.has_flatten()

error: proc-macro derive panicked
  --> test_suite\tests\regression\issue1904.rs:47:10
   |
47 | #[derive(Deserialize)]
   |          ^^^^^^^^^^^
   |
   = help: message: assertion failed: !cattrs.has_flatten()

error: proc-macro derive panicked
  --> test_suite\tests\regression\issue1904.rs:37:10
   |
37 | #[derive(Deserialize)]
   |          ^^^^^^^^^^^
   |
   = help: message: assertion failed: !cattrs.has_flatten()

error: proc-macro derive panicked
  --> test_suite\tests\regression\issue1904.rs:27:10
   |
27 | #[derive(Deserialize)]
   |          ^^^^^^^^^^^
   |
   = help: message: assertion failed: !cattrs.has_flatten()

error: proc-macro derive panicked
  --> test_suite\tests\regression\issue1904.rs:16:10
   |
16 | #[derive(Deserialize)]
   |          ^^^^^^^^^^^
   |
   = help: message: assertion failed: !cattrs.has_flatten()

error: proc-macro derive panicked
 --> test_suite\tests\regression\issue1904.rs:7:10
  |
7 | #[derive(Deserialize)]
  |          ^^^^^^^^^^^
  |
  = help: message: assertion failed: !cattrs.has_flatten()
- Fix incorrect deserialization of variants that doesn't contain flatten field when other contains
- Fix a panic when deriving `Deserialize` for an enum with tuple and struct with flatten field

Fixes (2):
    regression::issue2565::simple_variant
    regression::issue1904 (compilation)
@Mingun
Copy link
Contributor Author

Mingun commented Aug 9, 2024

@dtolnay, @oli-obk, can this bugfix be merged?

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thank you!

@dtolnay dtolnay merged commit fc55ac7 into serde-rs:master Aug 11, 2024
15 checks passed
@Mingun Mingun deleted the fix-2565 branch August 11, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants