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 str::to_owned when format! has no formatting arguments #75894

Closed
wants to merge 3 commits into from
Closed

Use str::to_owned when format! has no formatting arguments #75894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
be1ea4b
Use str::to_owned for `format!` without formatting arguments
tesuji Aug 21, 2020
eb0ca61
[dnm] enable fail tests
tesuji Sep 28, 2020
4c9b5a6
[dnm] enable fail tests
tesuji Sep 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,9 @@ jobs:
- name: x86_64-gnu-llvm-8
os: ubuntu-latest-xl
env: {}
- name: x86_64-gnu-tools
env:
CI_ONLY_WHEN_SUBMODULES_CHANGED: 1
- name: x86_64-gnu
os: ubuntu-latest-xl
env: {}
timeout-minutes: 600
runs-on: "${{ matrix.os }}"
steps:
Expand Down
1 change: 1 addition & 0 deletions library/alloc/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
#![feature(exact_size_is_empty)]
#![feature(exclusive_range_pattern)]
#![feature(extend_one)]
#![feature(fmt_as_str)]
#![feature(fmt_internals)]
#![feature(fn_traits)]
#![feature(fundamental)]
Expand Down
16 changes: 14 additions & 2 deletions library/alloc/src/macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,21 @@ macro_rules! vec {
/// ```
#[macro_export]
#[stable(feature = "rust1", since = "1.0.0")]
#[allow_internal_unstable(fmt_as_str)]
macro_rules! format {
($($arg:tt)*) => {{
let res = $crate::fmt::format($crate::__export::format_args!($($arg)*));
res
// NOTE: `format_args!` borrows from temporaries. This means that
// `match` is necessary to extend the lifetime of the temporaries until after
// the `Arguments` is no longer used. The same pattern is used
// inside `format_args!` itself.
let r = match $crate::__export::format_args!($($arg)*) {
// HACK: We hope that constant propagation will make LLVM optimize out
// this match.
Copy link
Member

Choose a reason for hiding this comment

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

Is this FIXME actionable at all? Is this really a FIXME?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this is more like a HACK.

args => match args.as_str() {
Some(s) => $crate::borrow::ToOwned::to_owned(s),
None => $crate::fmt::format(args),
}
};
r
}}
}
4 changes: 1 addition & 3 deletions src/ci/github-actions/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,7 @@ jobs:
- name: x86_64-gnu-llvm-8
<<: *job-linux-xl

- name: x86_64-gnu-tools
env:
CI_ONLY_WHEN_SUBMODULES_CHANGED: 1
- name: x86_64-gnu
<<: *job-linux-xl

try:
Expand Down
4 changes: 3 additions & 1 deletion src/ci/scripts/run-build-from-ci.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ export SRC=.
# the environment
rustup self uninstall -y || true
if [ -z "${IMAGE+x}" ]; then
src/ci/run.sh
if ! src/ci/run.sh; then
cat build/x86_64-unknown-linux-gnu/test/codegen/issue-75742-format_without_fmt_args/issue-75742-format_without_fmt_args.ll
fi
else
src/ci/docker/run.sh "${IMAGE}"
fi
39 changes: 39 additions & 0 deletions src/test/codegen/issue-75742-format_without_fmt_args.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// min-llvm-version: 10.0.0
// compile-flags: -C opt-level=3
#![crate_type = "rlib"]
#![feature(format_args_capture)]

// Make sure allocation not happen when result of `format!` is unused and
// there are no formatting arguments.

// CHECK-LABEL: @format_wo_fmt_args
// CHECK-NEXT: {{"_ZN[^:]+"}}:
// CHECK-NEXT: ret
#[no_mangle]
pub fn format_wo_fmt_args() {
format!("");
tesuji marked this conversation as resolved.
Show resolved Hide resolved
format!("a long story");
format!("a long story {{");
}

// CHECK-LABEL: @format_wo_fmt_args_ret
// CHECK-NOT: Arguments
#[no_mangle]
pub fn format_wo_fmt_args_ret() -> String {
format!("a long story")
}

// CHECK-LABEL: @format_w_fmt_args_ret_1
// CHECK: alloc::fmt::format
#[no_mangle]
pub fn format_w_fmt_args_ret_1(n: usize) -> String {
format!("a long story: {}", n)
}

// CHECK-LABEL: @format_w_fmt_args_ret_2
// CHECK: core::fmt::ArgumentV1::from_usize
// CHECK: alloc::fmt::format
#[no_mangle]
pub fn format_w_fmt_args_ret_2(n: usize, width: usize) -> String {
format!("a long story {n:width$}")
}
71 changes: 42 additions & 29 deletions src/test/pretty/issue-4264.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,35 +30,48 @@


({
let res =
((::alloc::fmt::format as
for<'r> fn(Arguments<'r>) -> String {format})(((::core::fmt::Arguments::new_v1
as
fn(&[&'static str], &[ArgumentV1]) -> Arguments {Arguments::new_v1})((&([("test"
as
&str)]
as
[&str; 1])
as
&[&str; 1]),
(&(match (()
as
())
{
()
=>
([]
as
[ArgumentV1; 0]),
}
as
[ArgumentV1; 0])
as
&[ArgumentV1; 0]))
as
Arguments))
as String);
(res as String)
let r =
(match ((::core::fmt::Arguments::new_v1 as
fn(&[&'static str], &[ArgumentV1]) -> Arguments {Arguments::new_v1})((&([("test"
as
&str)]
as
[&str; 1])
as
&[&str; 1]),
(&(match (()
as
())
{
()
=>
([]
as
[ArgumentV1; 0]),
}
as
[ArgumentV1; 0])
as
&[ArgumentV1; 0]))
as Arguments) {
args =>
(match ((args as Arguments).as_str() as
Option<&str>) {
Some(s) =>
((::alloc::borrow::ToOwned::to_owned as
for<'r> fn(&'r str) -> <str as ToOwned>::Owned {<str as ToOwned>::to_owned})((s
as
&str))
as String),
None =>
((::alloc::fmt::format as
for<'r> fn(Arguments<'r>) -> String {format})((args
as
Arguments))
as String),
} as String),
} as String);
(r as String)
} as String);
} as ())
pub type Foo = [i32; (3 as usize)];
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/borrowck/issue-64453.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ struct Value;
static settings_dir: String = format!("");
//~^ ERROR calls in statics are limited to constant functions
//~| ERROR calls in statics are limited to constant functions
//~| ERROR calls in statics are limited to constant functions
//~| ERROR calls in statics are limited to constant functions

fn from_string(_: String) -> Value {
Value
Expand Down
20 changes: 18 additions & 2 deletions src/test/ui/borrowck/issue-64453.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0507]: cannot move out of static item `settings_dir`
--> $DIR/issue-64453.rs:14:37
--> $DIR/issue-64453.rs:16:37
|
LL | let settings_data = from_string(settings_dir);
| ^^^^^^^^^^^^ move occurs because `settings_dir` has type `String`, which does not implement the `Copy` trait
Expand All @@ -20,7 +20,23 @@ LL | static settings_dir: String = format!("");
|
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 3 previous errors
error[E0015]: calls in statics are limited to constant functions, tuple structs and tuple variants
--> $DIR/issue-64453.rs:4:31
|
LL | static settings_dir: String = format!("");
| ^^^^^^^^^^^
|
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0015]: calls in statics are limited to constant functions, tuple structs and tuple variants
--> $DIR/issue-64453.rs:4:31
|
LL | static settings_dir: String = format!("");
| ^^^^^^^^^^^
|
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 5 previous errors

Some errors have detailed explanations: E0015, E0507.
For more information about an error, try `rustc --explain E0015`.