-
Notifications
You must be signed in to change notification settings - Fork 283
refactor: avoid reordering attributes when restoring them #1433
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
base: master
Are you sure you want to change the base?
Conversation
|
I added a test but I'm not sure that these c2rust-refactor tests are actually run in CI. It seems like we're not running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this just be written as a Rust #[test] instead? cargo test is run in CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_rust_refactor.py is pretty small (unlike test_translator.py), and it makes a lot of things more seamless to keep it all in cargo test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do want to test end-to-end refactorer behavior here if possible, so if you have a suggestion on how to express that as a #[test] (e.g. setting up integrated snapshot testing for the refactorer), I'm all ears.
9a2ef27 to
1251c6c
Compare
kkysen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this why stuff like #[c2rust::src_loc = "125:8"]#[derive(Copy, Clone)] is happening? I noticed that while testing crisp.
Yeah, this PR fixes that weird formatting. |
including in the presence of multiple
#[derive(...)]attributes interleaved with other attributes. No test at present; I'll look into adding one.