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

Clear eval_libc errors from unix shims #3984

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

noahmbright
Copy link
Contributor

@noahmbright noahmbright commented Oct 21, 2024

Each instance of eval_libc("E is scrubbed out. Currently, this has the idealistic function calls, but this won't compile yet. Marking as a draft and making comments with the representative compilation errors. Whether we want to hold off on making these erring changes or address the root problem now is up to the reviewers.

See comments for the issues concretely, but it seems like everything stems from differences between the return types of this.eval_libc and LibcError.

@noahmbright noahmbright marked this pull request as draft October 21, 2024 18:12
src/shims/alloc.rs Outdated Show resolved Hide resolved
src/shims/unix/foreign_items.rs Outdated Show resolved Hide resolved
src/shims/unix/foreign_items.rs Outdated Show resolved Hide resolved
src/shims/unix/linux/foreign_items.rs Outdated Show resolved Hide resolved
src/shims/unix/macos/foreign_items.rs Outdated Show resolved Hide resolved
src/shims/alloc.rs Outdated Show resolved Hide resolved
src/shims/unix/foreign_items.rs Outdated Show resolved Hide resolved
src/shims/unix/foreign_items.rs Outdated Show resolved Hide resolved
src/shims/unix/linux/foreign_items.rs Outdated Show resolved Hide resolved
src/shims/unix/linux/sync.rs Outdated Show resolved Hide resolved
src/shims/unix/linux/sync.rs Outdated Show resolved Hide resolved
src/shims/unix/macos/foreign_items.rs Outdated Show resolved Hide resolved
src/shims/unix/solarish/foreign_items.rs Outdated Show resolved Hide resolved
src/shims/unix/sync.rs Outdated Show resolved Hide resolved
src/concurrency/sync.rs Outdated Show resolved Hide resolved
@noahmbright noahmbright marked this pull request as ready for review October 24, 2024 23:03
@RalfJung
Copy link
Member

marked this pull request as ready for review

Note that this does not trigger a notification, so it is easy to be missed. Please do this instead:
@rustbot ready

@rustbot rustbot added the S-waiting-on-review Status: Waiting for a review to complete label Oct 25, 2024
src/concurrency/sync.rs Outdated Show resolved Hide resolved
@RalfJung
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels Oct 25, 2024
@noahmbright
Copy link
Contributor Author

Reverted that change in sync.rs and rebased a merge conflict, hence the force push.
@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Oct 25, 2024
@RalfJung
Copy link
Member

Thanks! Please squash this into a single commit.
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels Oct 25, 2024
@noahmbright
Copy link
Contributor Author

Done - again thanks for your patience and willingness to help. I was definitely too excited about mindlessly changing every eval_libc I could find. This was a good lesson for me. @rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Oct 25, 2024
@RalfJung RalfJung added this pull request to the merge queue Oct 25, 2024
Merged via the queue into rust-lang:master with commit 0164e7d Oct 25, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Waiting for a review to complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants