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

Use c"lit" for CStrings without unwrap #133701

Merged
merged 1 commit into from
Dec 3, 2024
Merged

Use c"lit" for CStrings without unwrap #133701

merged 1 commit into from
Dec 3, 2024

Conversation

kornelski
Copy link
Contributor

I've reviewed uses of CString::new("lit").

Some could be changed to c"lit". Some could be changed to c"lit".to_owned(), avoiding an unwrap().

Many CString documentation examples could be simplified. I deliberately haven't changed all the examples to use the exact same expression, so that they can demonstrate many ways of creating CStrings.

I've left UI tests mostly unchanged, because c"" requires edition 2021, but most UI tests use 2015, and I didn't want to accidentally change what the tests are testing.

@rustbot
Copy link
Collaborator

rustbot commented Dec 1, 2024

r? @workingjubilee

rustbot has assigned @workingjubilee.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs O-unix Operating system: Unix-like 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-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 1, 2024
@rustbot
Copy link
Collaborator

rustbot commented Dec 1, 2024

The Miri subtree was changed

cc @rust-lang/miri

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

lnicola
lnicola approved these changes Dec 1, 2024
View reviewed changes
Copy link
Member

@lnicola lnicola left a comment

Choose a reason for hiding this comment

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

👍 from the rust-analyzer side

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@workingjubilee
Copy link
Member

seems good. r=me with the double-doublequote fixed and tidy appeased.

@bors delegate+

@bors
Copy link
Contributor

bors commented Dec 1, 2024

✌️ @kornelski, you can now approve this pull request!

If @workingjubilee told you to "r=me" after making some further change, please make that change, then do @bors r=@workingjubilee

@rustbot
Copy link
Collaborator

rustbot commented Dec 1, 2024

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

The Miri subtree was changed

cc @rust-lang/miri

@rust-log-analyzer

This comment has been minimized.

Sign in to view
This was referenced Dec 2, 2024
Closed
Open
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@workingjubilee
Copy link
Member

Ah, didn't notice the paren was off.

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Dec 2, 2024

📌 Commit eadea77 has been approved by workingjubilee

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 2, 2024
@GuillaumeGomez GuillaumeGomez mentioned this pull request Dec 2, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 3, 2024
@bors
Auto merge of rust-lang#133770 - GuillaumeGomez:rollup-l62iyyx, r=Gui…
41cbe3e 
…llaumeGomez

Rollup of 10 pull requests

Successful merges:

 - rust-lang#131713 (Stabilize `const_maybe_uninit_write`)
 - rust-lang#133535 (show forbidden_lint_groups in future-compat reports)
 - rust-lang#133610 (Move `Const::{from_anon_const,try_from_lit}` to hir_ty_lowering)
 - rust-lang#133701 (Use c"lit" for CStrings without unwrap)
 - rust-lang#133704 (fix ICE when promoted has layout size overflow)
 - rust-lang#133705 (add "profiler" and "optimized-compiler-builtins" option coverage for ci-rustc)
 - rust-lang#133710 (Reducing `target_feature` check-cfg merge conflicts)
 - rust-lang#133732 (Fix `-Zdump-mir-dataflow`)
 - rust-lang#133746 (Change `AttrArgs::Eq` to a struct variant)
 - rust-lang#133763 (Fix `f16::midpoint` const feature gate)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 65b0dad into rust-lang:master Dec 3, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 3, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 3, 2024
@rust-timer
Unrolled build for rust-lang#133701
d03ec32 
Rollup merge of rust-lang#133701 - kornelski:c-str, r=workingjubilee

Use c"lit" for CStrings without unwrap

I've reviewed uses of `CString::new("lit")`.

Some could be changed to `c"lit"`. Some could be changed to `c"lit".to_owned()`, avoiding an `unwrap()`.

Many `CString` documentation examples could be simplified. I deliberately haven't changed all the examples to use the exact same expression, so that they can demonstrate many ways of creating `CString`s.

I've left UI tests mostly unchanged, because `c""` requires edition 2021, but most UI tests use 2015, and I didn't want to accidentally change what the tests are testing.
@kornelski kornelski deleted the c-str branch December 3, 2024 12:03
@uweigand uweigand mentioned this pull request Dec 4, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@workingjubilee workingjubilee workingjubilee left review comments

@jieyouxu jieyouxu jieyouxu left review comments

@lnicola lnicola lnicola approved these changes

Assignees

@workingjubilee workingjubilee

Labels
A-run-make Area: port run-make Makefiles to rmake.rs O-unix Operating system: Unix-like S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.85.0
Development

Successfully merging this pull request may close these issues.
7 participants
@kornelski @rustbot @rust-log-analyzer @workingjubilee @bors @lnicola @jieyouxu