-
Notifications
You must be signed in to change notification settings - Fork 13.4k
debuginfo: Ignore optimized enum tests for GDB versions that can't handle them. #39039
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
Note that only the gdbr tests should be ignored. It would be better if we directly annotated the tests, too (not the entire file) |
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.
r = me with the minor comments addressed
@@ -94,6 +99,30 @@ impl EarlyProps { | |||
} | |||
} | |||
|
|||
fn extract_gdb_version_range(line: &str) -> (u32, u32) { |
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.
Could you add a comment explaining the return type please?
.collect::<Vec<&str>>(); | ||
|
||
match range_components.len() { | ||
0 => panic!(ERROR_MESSAGE), |
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.
Could be removed
.split(' ') | ||
.last() | ||
.expect("Malformed GDB version directive"); | ||
let min_version = extract_gdb_version_range(line); |
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.
I think the name is misleading because it is both the min and max version? Rather, than renaming, using a destructuring let
rather than tuple indexing would probably be more readable.
actual_version < extract_gdb_version(min_version).unwrap() | ||
actual_version < min_version.0 | ||
} else if line.contains("ignore-gdb-version") { | ||
let version_range = extract_gdb_version_range(line); |
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.
Again, I'd prefer to destructure here.
@bors r=nrc Thanks for the review, I think the comments are all addressed now. |
📌 Commit 30ba990 has been approved by |
@bors rollup |
Sorry, I didn't find the time to implement that. Maybe I'll come back to this later but feel free to implement it (e.g. by adding a "ignore-gdbr-in-version-range" directive). |
…, r=nrc debuginfo: Ignore optimized enum tests for GDB versions that can't handle them. Fixes rust-lang#38948. r? @nrc cc @Manishearth
ignore more gdb versions with buggy rust support This extends the versions of gdb which were ignored in rust-lang#39039. While just ignoring gdb versions up to 7.12.1 would have been sufficient for now, I believe (after consulting https://sourceware.org/gdb/wiki/Internals%20Versions) that ignoring versions up to 7.12.9 will prevent the tests failing again for 7.12.2, etc. while still running all tests for the development versions of gdb (which will be >= 7.12.10 as far as I can tell). This should fix rust-lang#39522. cc @Manishearth, @michaelwoerister, rust-lang#38948
ignore more gdb versions with buggy rust support This extends the versions of gdb which were ignored in rust-lang#39039. While just ignoring gdb versions up to 7.12.1 would have been sufficient for now, I believe (after consulting https://sourceware.org/gdb/wiki/Internals%20Versions) that ignoring versions up to 7.12.9 will prevent the tests failing again for 7.12.2, etc. while still running all tests for the development versions of gdb (which will be >= 7.12.10 as far as I can tell). This should fix rust-lang#39522. cc @Manishearth, @michaelwoerister, rust-lang#38948
ignore more gdb versions with buggy rust support This extends the versions of gdb which were ignored in rust-lang#39039. While just ignoring gdb versions up to 7.12.1 would have been sufficient for now, I believe (after consulting https://sourceware.org/gdb/wiki/Internals%20Versions) that ignoring versions up to 7.12.9 will prevent the tests failing again for 7.12.2, etc. while still running all tests for the development versions of gdb (which will be >= 7.12.10 as far as I can tell). This should fix rust-lang#39522. cc @Manishearth, @michaelwoerister, rust-lang#38948
Fixes #38948.
r? @nrc
cc @Manishearth