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.

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

Infinite loop when compiling nested macros #11692

Closed
bnoordhuis opened this issue Jan 20, 2014 · 9 comments · Fixed by #12370
Closed

Infinite loop when compiling nested macros #11692

bnoordhuis opened this issue Jan 20, 2014 · 9 comments · Fixed by #12370
Labels
P-medium Medium priority
Milestone

Comments

@bnoordhuis
Copy link
Contributor

Apologies if this has been reported before. A search didn't turn up anything but you know how GH's search is.

$ cat t.rs
fn main() { print!(format!("BAM")); }

$ rustc t.rs 2>&1 | head -20
t.rs:1:20: 1:26 error: macro undefined: 'format'
t.rs:1 fn main() { print!(format!("BAM")); }
                          ^~~~~~
t.rs:1:20: 1:26 error: macro undefined: 'format'
t.rs:1 fn main() { print!(format!("BAM")); }
                          ^~~~~~
t.rs:1:20: 1:26 error: macro undefined: 'format'
t.rs:1 fn main() { print!(format!("BAM")); }
                          ^~~~~~
t.rs:1:20: 1:26 error: macro undefined: 'format'
t.rs:1 fn main() { print!(format!("BAM")); }
                          ^~~~~~
t.rs:1:20: 1:26 error: macro undefined: 'format'
t.rs:1 fn main() { print!(format!("BAM")); }
                          ^~~~~~
t.rs:1:20: 1:26 error: macro undefined: 'format'
t.rs:1 fn main() { print!(format!("BAM")); }
                          ^~~~~~
t.rs:1:20: 1:26 error: macro undefined: 'format'
t.rs:1 fn main() { print!(format!("BAM")); }

Tested with today's x86_64 lucid nightly from https://launchpad.net/~hansjorg/+archive/rust

@alexcrichton
Copy link
Member

That sounds pretty bad!

Nominating.

@pnkfelix
Copy link
Member

Accepted for 1.0, P-high

@edwardw
Copy link
Contributor

edwardw commented Feb 11, 2014

The code path seems to be:
https://github.com/mozilla/rust/blob/master/src/libsyntax/ext/format.rs#L799
https://github.com/mozilla/rust/blob/master/src/libsyntax/ext/base.rs#L302

The syntax::ext::base::ExtCtxt::expand_expr only looks for the built-in macros returned by syntax_expander_table, so it fails to recognize the second macro format! which is not a built-in one.

But how to fix it?

alexcrichton added a commit to alexcrichton/rust that referenced this issue Feb 11, 2014
This led to infinite recursion when compiling a macro which inside used an
undefined macro. Instead some dummy expression is returned for the result of
expansion (rather than the macro itself).

Closes rust-lang#11692
@sfackler
Copy link
Member

You'd probably want to move the SyntaxEnv from MacroExpander to inside the ExtCtxt.

@edwardw
Copy link
Contributor

edwardw commented Feb 12, 2014

Moving SyntaxEnv from MacroExpander to inside ExtCtxt is a great idea! I tried it but there's a practical problem. There are several important match expressions in which a instance of SyntaxEnv is borrowed immutably and ExtCtxt mutably. Moving SyntaxEnv to inside ExtCtxt would make that illegal.

In the same vein, changing MacroExpanderFn from
fn(ecx: &mut ExtCtxt, ...) -> MacResult
to
fn(ecx: &mut ExtCtxt, env: &SyntaxEnv, ...) -> MacResult
or
fn(expander: &mut MacroExpander, ...) -> MacResult
will have the same effect. But that approach involves about 170+ changes to the libsyntax.

Is there any other way to do it?

@sfackler
Copy link
Member

I think the best thing is to probably restructure the matches to avoid the double borrow. I'm not sure how painful that'll be though. Changing the signature of MacroExpanderFn seems a bit unfortunate.

@edwardw
Copy link
Contributor

edwardw commented Feb 13, 2014

I couldn't find a way to untangle the double borrow. Here's what I found so far for whoever may pick up this issue in the future.

The issue reported in this ticket is twofold. One is that unless it is one of the hardcoded macros defined in syntax::ext::base::syntax_expander_table, a nested macro will be deemed as undefined; the other is that a nested undefined macro will cause a infinite loop in the phase 2 of crate compilation.

The reason that a nested legit macro deemed as undefined is that the inner macro gets expanded in the expander function of outer macro. But the expander function has no syntax context information right now, in that context there are macros being injected from std and syntax extensions. Moving SyntaxEnv to inside ExtCtxt will do with minimum modification to the current macro API.

The #11803 seems to be remotely related.

bors added a commit that referenced this issue Feb 19, 2014
Closes #11692. Instead of returning the original expression, a dummy expression
(with identical span) is returned. This prevents infinite loops of failed
expansions as well as odd double error messages in certain situations.

This is a slightly better fix than #12197, because it does not produce a double error and also fixes a few other cases where an infinite loop could happen.

This does not fix the other issue in #11692 (non-builtin macros not being recognised when expanded inside macros), which I think should be moved into a separate issue.
@bors bors closed this as completed in 0bdfd0f Feb 19, 2014
@edwardw
Copy link
Contributor

edwardw commented Feb 19, 2014

Do we plan to support nested macro invocations? #9323 is also relevant.

@sfackler
Copy link
Member

Nested macro invocations should be (?) supported. Things are expanded outside-in.

flip1995 pushed a commit to flip1995/rust that referenced this issue Nov 2, 2023
…, r=xFrednet

[`iter_without_into_iter`]: fix papercuts in suggestion and restrict linting to exported types

See rust-lang#11692 for more context.

tldr: the lint `iter_without_into_iter` has suggestions that don't compile, which imo isn't that problematic because it does have the appropriate `Applicability` that tells external tools that it shouldn't be auto-applied.
However there were some obvious "errors" in the suggestion that really should've been included in my initial PR adding the lint, which is fixed by this PR:
- `IntoIterator::into_iter` needs a `self` argument.
- `IntoIterator::Iter` associated type doesn't exist. This should've just been `Item`.

This still doesn't make it machine applicable, and the remaining things are imho quite non-trivial to implement, as I've explained in rust-lang/rust-clippy#11692 (comment).
I personally think it's fine to leave it there and let the user change the remaining errors when copy-pasting the suggestion (e.g. errors caused by lifetimes that were permitted in fn return-position but are not in associated types).
This is how many of our other lint suggestions already work.

Also, we now restrict linting to only exported types. This required moving basically all of the tests around since they were previously in the `main` function. Same for `into_iter_without_iter`. The git diff is a bit useless here...

changelog: [`iter_without_into_iter`]: fix papercuts in suggestion and restrict linting to exported types

(cc `@lopopolo,` figured I should mention you since you created the issue)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-medium Medium priority
Projects
None yet
5 participants