Skip to content

DNM: Try using memchr without inlining on windows#121671

Closed
flip1995 wants to merge 2 commits intorust-lang:masterfrom
flip1995:memchr-no-inline-bump
Closed

DNM: Try using memchr without inlining on windows#121671
flip1995 wants to merge 2 commits intorust-lang:masterfrom
flip1995:memchr-no-inline-bump

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

flip1995 commented Feb 27, 2024

@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
Collaborator

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
Collaborator

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

flip1995 commented Feb 27, 2024

@bors try

@bors
Copy link
Collaborator

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
Collaborator

bors commented Feb 27, 2024

💔 Test failed - checks-actions

@flip1995
Copy link
Member Author

flip1995 commented Feb 27, 2024

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 flip1995 force-pushed the memchr-no-inline-bump branch from 182164a to e3ea0db Compare February 27, 2024 13:35
@flip1995
Copy link
Member Author

flip1995 commented Feb 27, 2024

@bors try

@bors
Copy link
Collaborator

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

clubby789 commented Feb 27, 2024

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

flip1995 commented Feb 27, 2024

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

@flip1995 flip1995 force-pushed the memchr-no-inline-bump branch from e3ea0db to f58177a Compare February 27, 2024 14:34
@flip1995
Copy link
Member Author

flip1995 commented Feb 27, 2024

@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
Collaborator

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
Collaborator

bors commented Feb 27, 2024

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

@flip1995
Copy link
Member Author

flip1995 commented Feb 27, 2024

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

@flip1995 flip1995 force-pushed the memchr-no-inline-bump branch from f58177a to d6c1e1d Compare February 27, 2024 17:06
@flip1995
Copy link
Member Author

flip1995 commented Feb 27, 2024

@bors try

@bors
Copy link
Collaborator

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
Collaborator

bors commented Feb 27, 2024

💔 Test failed - checks-actions

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@flip1995 flip1995 force-pushed the memchr-no-inline-bump branch from d6c1e1d to 7dc5996 Compare February 27, 2024 18:12
@flip1995
Copy link
Member Author

flip1995 commented Feb 27, 2024

@bors try

@bors
Copy link
Collaborator

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
Collaborator

bors commented Feb 27, 2024

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

@flip1995
Copy link
Member Author

flip1995 commented Feb 28, 2024

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