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

analyze: script for automatically fixing some compile errors #1174

Merged
merged 5 commits into from
Dec 3, 2024

Conversation

spernsteiner
Copy link
Collaborator

This branch adds a Python script that automatically fixes some kinds of compiler errors by processing the output of rustc --error-format json. It applies all MachineApplicable suggestions, plus two other kinds of fixes that don't come with a MachineApplicable suggestion (in our current nightly) but are important for lighttpd buffer:

  • "Lifetime may not live long enough": For each function, c2rust-analyze adds a lifetime parameter for each reference in the arguments and return type, but doesn't add any outlives bounds (e.g. 'a: 'b) to relate those lifetimes. On functions that return a borrowed reference into one of their arguments, this results in a borrow checker error. The error comes with a human-readable suggestion like "consider adding the following bound: 'a: 'b", and also marks the declaration of the first lifetime ('a); the auto-fix script parses this output and inserts the suggested : 'b bound after the declaration.
  • "The trait Copy may not be implemented for this type": When c2rust-analyze converts a field from raw pointer type to &mut T or Box<T>, it doesn't remove derive(Copy) from the struct. This causes a compile error because &mut T and Box<T> don't implement Copy. The error marks the location of the Copy token in #[derive(Copy)]; the auto-fix script removes that token (and the subsequent comma, if necessary). Note this can produce a trailing comma (#[derive(Clone, Copy)] -> #[derive(Clone, )]) or an empty derive() (#[derive(Copy)] -> #[derive()]), both of which are legal but unidiomatic.

@spernsteiner spernsteiner requested a review from ahomescu December 2, 2024 19:11
c2rust-analyze/scripts/auto_fix_errors.py Outdated Show resolved Hide resolved
c2rust-analyze/scripts/auto_fix_errors.py Show resolved Hide resolved
Comment on lines +16 to +52
Fix = namedtuple('Fix', (
# Path to the file to modify.
'file_path',
# Line number where the fix will be applied. This is used for debug output
# only.
'line_number',
# Start of the byte range to replace.
'start_byte',
# End (exclusive) of the byte range to replace.
'end_byte',
# Replacement text (as a `str`, not `bytes`).
'new_text',
# The original error message. This is used for debug output only.
'message',
))

LifetimeBound = namedtuple('LifetimeBound', (
'file_path',
'line_number',
# Byte offset of the start of the lifetime parameter declaration.
'start_byte',
# Byte offset of the end of the lifetime parameter declaration.
'end_byte',
# The lifetime to use in the new bound. If `'a: 'b` is the suggested
# bound, then `start/end_byte` points to the declaration of `'a`, and
# `bound_lifetime` is the string `"'b"`.
'bound_lifetime',
))

RemoveDeriveCopy = namedtuple('RemoveDeriveCopy', (
'file_path',
'line_number',
# Byte offset of the start of the token `Copy`.
'start_byte',
# Byte offset of the end of the token `Copy`.
'end_byte',
))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think it's a lot more idiomatic to use @dataclasses instead of namedtuple.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I originally used @dataclass here, but switched to namedtuple because CI was failing - the Ubuntu 18 runner has Python 3.6, but dataclasses were added in 3.7

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh that's really annoying. Python 3.6 was EOL 3 years ago.

@spernsteiner spernsteiner force-pushed the analyze-auto-fix-script-base branch from 7fdeddc to ecd1cbc Compare December 3, 2024 19:29
@spernsteiner spernsteiner force-pushed the analyze-auto-fix-script branch from 60b828d to ba9bad9 Compare December 3, 2024 19:29
@spernsteiner spernsteiner changed the base branch from analyze-auto-fix-script-base to master December 3, 2024 19:29
@spernsteiner spernsteiner merged commit ff0fe1a into master Dec 3, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants