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

WIP: Gate generating mod items in procedural macros #50587

Closed
wants to merge 1 commit into from

Conversation

alexcrichton
Copy link
Member

Discovered in #50504 the hygiene here is super questionable, so we'll likely want to gate this by default for procedural macros and procedural attributes for sure. This is a PR to evaluate the impact for procedural derive as well.

@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 9, 2018
@alexcrichton
Copy link
Member Author

@bors: try

@alexcrichton
Copy link
Member Author

@rust-lang/infra, could this be queued up for a check-only crater run?

@bors
Copy link
Contributor

bors commented May 9, 2018

⌛ Trying commit bf3a1f146ad648bca69083a52dea192ba7a65e1f with merge d8c55432067be68f5e33e1472daddd580de55bca...

@alexcrichton
Copy link
Member Author

@bors: try

@bors
Copy link
Contributor

bors commented May 9, 2018

⌛ Trying commit 37b3c5e with merge d5fe9545473461c4b00b487ae07543b8d8646f3c...

@kennytm kennytm added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label May 9, 2018
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:start:test_run-pass-fulldeps
Check compiletest suite=run-pass-fulldeps mode=run-pass (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[00:58:09] 
[00:58:09] running 88 tests
[00:59:50] ................................................F........F..................test [run-pass] run-pass-fulldeps/myriad-closures.rs has been running for over 60 seconds
[01:02:15] failures:
[01:02:15] 
[01:02:15] ---- [run-pass] run-pass-fulldeps/proc-macro/add-impl.rs stdout ----
[01:02:15]  
[01:02:15]  
[01:02:15] error: compilation failed!
[01:02:15] status: exit code: 101
[01:02:15] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass-fulldeps/proc-macro/add-impl.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-fulldeps" "--target=x86_64-unknown-linux-gnu" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-fulldeps/proc-macro/add-impl.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-fulldeps/proc-macro/add-impl.stage2-x86_64-unknown-linux-gnu.aux"
[01:02:15] ------------------------------------------
[01:02:15] 
[01:02:15] ------------------------------------------
[01:02:15] stderr:
[01:02:15] stderr:
[01:02:15] ------------------------------------------
[01:02:15] error[E0658]: procedural macros cannot expand to modules
[01:02:15]   --> /checkout/src/test/run-pass-fulldeps/proc-macro/add-impl.rs:17:10
[01:02:15]    |
[01:02:15] 17 | #[derive(AddImpl)]
[01:02:15]    |
[01:02:15]    = help: add #![feature(proc_macro_mod)] to the crate attributes to enable
[01:02:15] 
[01:02:15] error: aborting due to previous error
---
[01:02:15] ---- [run-pass] run-pass-fulldeps/proc-macro/crate-var.rs stdout ----
[01:02:15]  
[01:02:15] error: compilation failed!
[01:02:15] status: exit code: 101
[01:02:15] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass-fulldeps/proc-macro/crate-var.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-fulldeps" "--target=x86_64-unknown-linux-gnu" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-fulldeps/proc-macro/crate-var.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-fulldeps/proc-macro/crate-var.stage2-x86_64-unknown-linux-gnu.aux"
[01:02:15] ------------------------------------------
[01:02:15] 
[01:02:15] ------------------------------------------
[01:02:15] stderr:
[01:02:15] stderr:
[01:02:15] ------------------------------------------
[01:02:15] error[E0658]: procedural macros cannot expand to modules
[01:02:15]   --> /checkout/src/test/run-pass-fulldeps/proc-macro/crate-var.rs:22:14
[01:02:15]    |
[01:02:15] 22 |     #[derive(Double)]
[01:02:15] ...
[01:02:15] ...
[01:02:15] 25 | m!();
[01:02:15]    | ----- in this macro invocation
[01:02:15]    = help: add #![feature(proc_macro_mod)] to the crate attributes to enable
[01:02:15] 
[01:02:15] error: aborting due to previous error
[01:02:15] 
---
[01:02:15] 
[01:02:15] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:492:22
[01:02:15] 
[01:02:15] 
[01:02:15] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/run-pass-fulldeps" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass-fulldeps" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "run-pass" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[01:02:15] 
[01:02:15] 
[01:02:15] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:02:15] Build completed unsuccessfully in 0:21:36
[01:02:15] Build completed unsuccessfully in 0:21:36
[01:02:15] Makefile:58: recipe for target 'check' failed
[01:02:15] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:00ac8030
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented May 10, 2018

☀️ Test successful - status-travis
State: approved= try=True

@aidanhs
Copy link
Member

aidanhs commented May 12, 2018

Crater run started.

@petrochenkov
Copy link
Contributor

@alexcrichton
We probably should apply the same restriction to macros generating macro_rules! items.

@Mark-Simulacrum
Copy link
Member

@alexcrichton
Copy link
Member Author

Alrighty-roo, thanks @Mark-Simulacrum! Definitely seems like we can't land this as-is, I'll file a follow-up.

@alexcrichton alexcrichton deleted the no-modules branch May 17, 2018 03:36
alexcrichton added a commit to alexcrichton/rust that referenced this pull request May 18, 2018
This commit feature gates generating modules and macro definitions in procedural
macro expansions. Custom derive is exempt from this check as it would be a large
retroactive breaking change (rust-lang#50587). It's hoped that we can hopefully stem the
bleeding to figure out a better solution here before opening up the floodgates.

The restriction here is specifically targeted at surprising hygiene results [1]
that result in non-"copy/paste" behavior. Hygiene and procedural macros is
intended to be avoided as much as possible for Macros 1.2 by saying everything
is "as if you copy/pasted the code", but modules and macros are sort of weird
exceptions to this rule that aren't fully fleshed out.

[1]: rust-lang#50504 (comment)

cc rust-lang#50504
bors added a commit that referenced this pull request May 20, 2018
rustc: Disallow modules and macros in expansions

This commit feature gates generating modules and macro definitions in procedural
macro expansions. Custom derive is exempt from this check as it would be a large
retroactive breaking change (#50587). It's hoped that we can hopefully stem the
bleeding to figure out a better solution here before opening up the floodgates.

The restriction here is specifically targeted at surprising hygiene results [1]
that result in non-"copy/paste" behavior. Hygiene and procedural macros is
intended to be avoided as much as possible for Macros 1.2 by saying everything
is "as if you copy/pasted the code", but modules and macros are sort of weird
exceptions to this rule that aren't fully fleshed out.

[1]: #50504 (comment)

cc #50504
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-crater Status: Waiting on a crater run to be completed. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants