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.

Sign up for GitHub

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

Jump to bottom

DNM: Try using memchr without inlining on windows #121671

Closed
wants to merge 2 commits into from
Closed

DNM: Try using memchr without inlining on windows #121671

wants to merge 2 commits into from

Conversation

flip1995
Copy link
Member

@flip1995 flip1995 commented Feb 27, 2024 •
edited
Loading

Another attempt at #121465, trying to fix the issue on windows to unblock the Clippy sync.

(Does specifying a git dependency even work or will the tidy tool complain? 🤔)

memchr patch

r? @ghost

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Feb 27, 2024
@rustbot
Copy link
Collaborator

rustbot commented Feb 27, 2024

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@flip1995
Copy link
Member Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 27, 2024
@bors
Auto merge of rust-lang#121671 - flip1995:memchr-no-inline-bump, r=<try>
e7b9ada 
DNM: Try using memchr without inlining on windows

Another attempt at rust-lang#121465, trying to fix the issue on windows to unblock the Clippy sync.

(Does specifying a git dependency even work or will the `tidy` tool complain? 🤔)

r? `@ghost`
@bors
Copy link
Contributor

bors commented Feb 27, 2024

⌛ Trying commit c4b7bdb with merge e7b9ada...

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Feb 27, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 27, 2024
@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Feb 27, 2024
@flip1995
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Feb 27, 2024

⌛ Trying commit 182164a with merge 807e524...

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 27, 2024
@bors
Auto merge of rust-lang#121671 - flip1995:memchr-no-inline-bump, r=<try>
807e524 
DNM: Try using memchr without inlining on windows

Another attempt at rust-lang#121465, trying to fix the issue on windows to unblock the Clippy sync.

(Does specifying a git dependency even work or will the `tidy` tool complain? 🤔)

r? `@ghost`
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Feb 27, 2024

💔 Test failed - checks-actions

@flip1995
Copy link
Member Author

That also didn't work...

@flip1995 flip1995 closed this Feb 27, 2024
@flip1995 flip1995 deleted the memchr-no-inline-bump branch February 27, 2024 13:28
@flip1995 flip1995 restored the memchr-no-inline-bump branch February 27, 2024 13:31
@flip1995 flip1995 reopened this Feb 27, 2024
@flip1995
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Feb 27, 2024

⌛ Trying commit e3ea0db with merge 29627ed...

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 27, 2024
@bors
Auto merge of rust-lang#121671 - flip1995:memchr-no-inline-bump, r=<try>
29627ed 
DNM: Try using memchr without inlining on windows

Another attempt at rust-lang#121465, trying to fix the issue on windows to unblock the Clippy sync.

(Does specifying a git dependency even work or will the `tidy` tool complain? 🤔)

[memchr patch](BurntSushi/memchr@0310038)

r? `@ghost`
@clubby789
Copy link
Contributor

I think this is only patching the version used by rustc_ast (which I locked since it was the only in-tree usage of memchr), which probably wouldn't affect the linker issue. Using a patch dependency and unlocking it might work?

@flip1995 flip1995 mentioned this pull request Feb 27, 2024
Merged
@flip1995
Copy link
Member Author

Yeah, I think you're right. Good idea with the patch version. Didn't know of this cargo feature TIL, thanks!

@flip1995
Copy link
Member Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 27, 2024
@bors
Auto merge of rust-lang#121671 - flip1995:memchr-no-inline-bump, r=<try>
0abcecb 
DNM: Try using memchr without inlining on windows

Another attempt at rust-lang#121465, trying to fix the issue on windows to unblock the Clippy sync.

(Does specifying a git dependency even work or will the `tidy` tool complain? 🤔)

[memchr patch](BurntSushi/memchr@0310038)

r? `@ghost`
@bors
Copy link
Contributor

bors commented Feb 27, 2024

⌛ Trying commit f58177a with merge 0abcecb...

@rust-log-analyzer

This comment was marked as outdated.

Sign in to view
@bors
Copy link
Contributor

bors commented Feb 27, 2024

☀️ Try build successful - checks-actions
Build commit: 0abcecb (0abcecb65f184320ecda8191e77371c288f1d101)

@flip1995
Copy link
Member Author

Ok, this worked. Let's find out whether the cfg_attr for inlining are necessary or if the #[used] attribute is sufficient.

@flip1995
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Feb 27, 2024

⌛ Trying commit d6c1e1d with merge 711253e...

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 27, 2024
@bors
Auto merge of rust-lang#121671 - flip1995:memchr-no-inline-bump, r=<try>
711253e 
DNM: Try using memchr without inlining on windows

Another attempt at rust-lang#121465, trying to fix the issue on windows to unblock the Clippy sync.

(Does specifying a git dependency even work or will the `tidy` tool complain? 🤔)

[memchr patch](BurntSushi/memchr@master...flip1995:memchr:master)

r? `@ghost`
@bors
Copy link
Contributor

bors commented Feb 27, 2024

💔 Test failed - checks-actions

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@flip1995
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Feb 27, 2024

⌛ Trying commit 7dc5996 with merge 72e5ada...

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 27, 2024
@bors
Auto merge of rust-lang#121671 - flip1995:memchr-no-inline-bump, r=<try>
72e5ada 
DNM: Try using memchr without inlining on windows

Another attempt at rust-lang#121465, trying to fix the issue on windows to unblock the Clippy sync.

(Does specifying a git dependency even work or will the `tidy` tool complain? 🤔)

[memchr patch](BurntSushi/memchr@master...flip1995:memchr:master)

r? `@ghost`
@bors
Copy link
Contributor

bors commented Feb 27, 2024

☀️ Try build successful - checks-actions
Build commit: 72e5ada (72e5ada2fbce1e5a401c7bc05e6a0de711c65fce)

@flip1995
Copy link
Member Author

BurntSushi/memchr@f2e2936

This change to memchr would "fix" the issue. But it is definitely not desirable in any way. For more details and discussions, refer to this Zulip thread

@flip1995 flip1995 closed this Feb 28, 2024
@flip1995 flip1995 deleted the memchr-no-inline-bump branch February 28, 2024 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@flip1995 @rustbot @bors @rust-log-analyzer @clubby789