Skip to content

Change the debug! macro ast expansion for temporaries #12239

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

Closed
alexcrichton opened this issue Feb 13, 2014 · 3 comments · Fixed by #12349
Closed

Change the debug! macro ast expansion for temporaries #12239

alexcrichton opened this issue Feb 13, 2014 · 3 comments · Fixed by #12349
Labels
A-frontend Area: Compiler frontend (errors, parsing and HIR)

Comments

@alexcrichton
Copy link
Member

Currently, expanding debug!("{}", foo) looks something like:

static PRECOMPILED_FORMAT_STRING = ...;
let foo_addr = &foo;
let arguments: &fmt::Arguments = unsafe { /* do things with foo_addr */ };
fmt::writeln(logger, arguments)

This is a problem with the new temporary rules because you can't debug!("{}", some_ref_cell.borrow().get()). This is because the borrow() ends after the enclosing statement which in this case is let foo_addr = &foo;

In order to allow debugging these values, we should change the ast expansion to:

static PRECOMPILED_FORMAT_STRING = ...;
match foo {
    ref foo_addr => {
        let arguments: &fmt::Arguments = unsafe { /* do things with foo_addr */ };
        fmt::writeln(logger, arguments)
    }
}

Note that with a match the enclosing statement encompasses the entire formatting call, so debugging foo.borrow().get() should be valid. One other portion of this bug would be ensuring that codegen doesn't suffer in terms of compile-time or size as a result of this change. It is currently unknown how much a match scope would affect codegen.

cc @nikomatsakis

@edwardw
Copy link
Contributor

edwardw commented Feb 17, 2014

This may break some code. For example, format!("expected constant: {}", (*err)); won't compile any more because (*err) will end up in the match head and deny(unnecessary_parens) is turned on by default.

@edwardw
Copy link
Contributor

edwardw commented Feb 17, 2014

I saw several such compiling errors in librustc itself already.

@nikomatsakis
Copy link
Contributor

Argh. This seems to be more of a bug with the linter, or at least a
very unfortunate interaction! I feel like linters (or at least this
linter) should ignore code generated by macro expansion.

bors added a commit that referenced this issue Feb 19, 2014
Currently, the format_args! macro and its downstream macros in turn
expand to series of let statements, one for each of its arguments, and
then the invocation of the macro function. If one or more of the
arguments are RefCell's, the enclosing statement for the temporary of
the let is the let itself, which leads to scope problem. This patch
changes let's to a match expression.

Closes #12239.
@bors bors closed this as completed in 111e092 Feb 19, 2014
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 25, 2022
flip1995 pushed a commit to flip1995/rust that referenced this issue Apr 4, 2024
…tation, r=y21

Add `missing_transmute_annotations` lint

Fixes rust-lang/rust-clippy#715.

r? `@blyxyas`

changelog: Add `missing_transmute_annotations` lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-frontend Area: Compiler frontend (errors, parsing and HIR)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants