[WIP] Treat crates specially when expanding #50101

abonander · 2018-04-20T04:20:41Z

Give the base compilation unit of Rust source code the special treatment it deserves during macro expansion.

Pretty-prints crates as they should appear, doesn't pretend they're just modules without names (which breaks syn).

I think this should also require proc-macro-attributes on the crate to be invoked with absolute paths which should alleviate resolution issues without doing anything too crazy.

TODO:

implement tests
require absolute paths in non-built-in attributes

I branched this from #50092, will rebase when that one is merged so only relevant commits appear (if that isn't fixed automatically)
closes #41430

r? @petrochenkov
cc @Rantanen @jseyfried

closes rust-lang#49934

abonander · 2018-04-20T04:23:20Z

src/libsyntax/print/pprust.rs

@@ -286,6 +287,10 @@ pub fn token_to_string(tok: &Token) -> String {
    }
 }

+pub fn crate_to_string(krate: &ast::Crate) -> String {


print_crate requires more stuff to work and prints the crate as if it's going to be compiled from the printed source, adding stuff like #![no_std] and the std prelude injection.

Question: do we want to show the prelude injections and stuff that print_crate() adds to its output?

abonander · 2018-04-20T04:23:41Z

src/test/compile-fail/issue-49934.rs

@@ -0,0 +1,23 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT


this file is from #50092, should disappear when that's merged

abonander · 2018-04-20T04:24:03Z

src/libsyntax/ext/expand.rs

+            ExpansionKind::OptExpr =>
+                Expansion::OptExpr(items.next().map(Annotatable::expect_expr)),
+            ExpansionKind::Pat | ExpansionKind::Ty =>
+                panic!("patterns and types aren't annotatable"),


these changes are from #50092

abonander · 2018-04-20T04:25:09Z

cc @alexcrichton for the absolute paths thing (see OP)

rust-highfive · 2018-04-20T05:09:01Z

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.

[00:44:34] .........................i..........................................................................
[00:44:38] ....................................................................................................
[00:44:42] ....................................................................................................
[00:44:46] ....................................................................................................
[00:44:50] ...........................................................F...FF.F.................................
[00:45:02] ....................................................................................................
[00:45:08] ....................................................................................................
[00:45:15] .......................i...........................................................................i
[00:45:21] ....................................................................................................
[00:45:21] ....................................................................................................
[00:45:27] .............ii...................................................................................F.
[00:45:35] ..............................................................................................i.....
   mod inner { #![derive(Debug)] }
[00:45:38] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:488:22
[00:45:38] -    |                 ^^^^^^^^^^^^^^^^^ help: try an outer attribute: `#[derive(Debug)]`
[00:45:38] - 
[00:45:38] - error: `derive` may only be applied to structs, enums and unions
[00:45:38] -   --> $DIR/issue-43106-gating-of-derive.rs:23:5
[00:45:38] -    |
[00:45:38] - LL |     #[derive(Debug)]
[00:45:38] - 
[00:45:38] - 
[00:45:38] - error: `derive` may only be applied to structs, enums and unions
[00:45:38] -   --> $DIR/issue-43106-gating-of-derive.rs:36:5
[00:45:38] -    |
[00:45:38] - LL |     #[derive(Debug)]
[00:45:38] - 
[00:45:38] - 
[00:45:38] - error: `derive` may only be applied to structs, enums and unions
[00:45:38] -   --> $DIR/issue-43106-gating-of-derive.rs:40:5
[00:45:38] -    |
[00:45:38] - LL |     #[derive(Debug)]
[00:45:38] - 
[00:45:38] - error: aborting due to 6 previous errors
[00:45:38] - 
[00:45:38] - 
[00:45:38] - 
[00:45:38] 
[00:45:38] 
[00:45:38] error: failed to write stderr to `/checkout/src/test/ui/feature-gate/issue-43106-gating-of-derive.stderr`: Read-only file system (os error 30)
[00:45:38] thread '[ui] ui/feature-gate/issue-43106-gating-of-derive.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:1889:9
[00:45:38] 
[00:45:38] ---- [ui] ui/feature-gate/issue-43106-gating-of-macro_use.rs stdout ----
[00:45:38]  diff of stderr:
[00:45:38] 
[00:45:38] 
[00:45:38] 1 error: argumeest/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--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"
[00:45:38] 
[00:45:38] 
[00:45:38] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:45:38] Build completed unsuccessfully in 0:02:27
[00:45:38] Build completed unsuccessfully in 0:02:27
[00:45:38] Makefile:58: recipe for target 'check' failed
[00:45:38] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0bdf48a0
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
56092 ./obj/build/x86_64-unknown-linux-gnu/stage0/lib/rustlib/x86_64-unknown-linux-gnu/bin
55340 ./obj/build/x86_64-unknown-linux-gnu/stage0-rustc/release
53564 ./obj/build/x86_64-unknown-linux-gnu/stage0-rustc/release/build
52060 ./obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/incremental/syntax-33oa6nnkk1g08
52056 ./obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/incremental/syntax-33oa6nnkk1g08/s-f0a85r4gez-1iutfhd-2h2c6e0haira
47824 ./obj/build/x86_64-unknown-linux-gnu/stage0-std
47496 ./obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps
46652 ./obj/build/x86_64-unknown-linux-gnu/stage0-std/release
46648 ./obj/build/x86_64-unknown-linux-gnu/stage1-std/release

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)

alexcrichton · 2018-04-20T15:01:14Z

I'd personally probably recommend against this, I'd find it hard to imagine at least us ever stabilizing such functionality :(

abonander · 2018-04-21T08:55:13Z

There's a lot of interest in being able to transform/walk an entire crate, so if we're not going to allow proc-macros to do it we should start thinking about something else.

abonander · 2018-04-21T08:59:00Z

@alexcrichton if you prefer I can pivot this to forbid or feature-gate proc-macro attribute invocations on the crate.

Rantanen · 2018-04-21T09:23:12Z

I'd personally probably recommend against this, I'd find it hard to imagine at least us ever stabilizing such functionality :(

Is this in reference to macro attributes on the crate level or the special handling of the crate expansion?

if you prefer I can pivot this to forbid or feature-gate proc-macro attribute invocations on the crate.

Proc-macro attributes on the crate is, from my perspective, the only use that would even need this change. If proc-macro attributes on the crate are not supported, then what other need is there for the crate token expansion working?

Pretty-prints crates as they should appear, doesn't pretend they're just modules without names (which breaks syn).

If @alexcrichton's comment was related to the specific implementation, then we still have the other option: Keep the crate token expansion as it is (or change it slightly), but also make sure that the crate can be parsed back from the tokens. Currently the big issue is that while a crate can be tokenized - there is no token stream that can be turned back into a crate.

abonander · 2018-04-21T12:55:36Z

Proc-macro attributes on the crate is, from my perspective, the only use that would even need this change. If proc-macro attributes on the crate are not supported, then what other need is there for the crate token expansion working?

I meant that instead of fixing crate expansion I would either rewrite this PR to or open a new one that forbids proc macro attribute invocations on the crate, or feature-gate the current usage so proc macro attributes can be stabilized without having to address this stuff first.

Otherwise, yeah @alexcrichton your comment was rather vague. I assumed you meant you don't see proc macro attribute invocations on the crate ever being stabilized.

alexcrichton · 2018-04-21T15:48:01Z

Er sorry yeah, let me clarify. Right now for "Macros 1.2" were very unlikely to stabilize attribute invocations on modules. It's a weird question of what does #[foo] mod foo; receive? Does it receive mod foo ;? The contents of the module foo?

Something like entire crate expansion also has huge repercussions on hygiene as well as whether it's feasible/quick. Entire crate expansion runs a risk of being extremely slow (we have to serialize back and forth with the procedural macro) and highly non-incremental (it's a complete black box to the compiler). I'd imagine that we're very far away from stabilizing this functionality, if at all.

In that sense I'd personally be wary of landing this functionality in the compiler, even on nightly. I think it'd be best to take other avenues of attack for use cases that would otherwise require entire-crate expansion.

abonander · 2018-04-21T23:04:38Z

@alexcrichton It's already allowed but it's very buggy (#41430). So, feature-gate or forbid?

alexcrichton · 2018-04-22T00:02:32Z

I don't personally feel too strongly, although if it were exclusively up to me I'd err on the side of forbidding this rather than feature-gating a fixed implementaiton.

abonander · 2018-04-22T05:06:06Z

@petrochenkov what is your opinion on this? (see above discussion)

abonander · 2018-04-27T04:38:27Z

Closing this PR as this appears to need more discussion.

petrochenkov · 2018-04-28T09:11:25Z

Oh, this is closed now.
Nice, the less reviewing the better 😆

abonander added 2 commits April 19, 2018 16:58

don't panic on #[derive] placed on statements/expressions

7656945

closes rust-lang#49934

prototype: treat crates as special when expanding

51211e5

abonander commented Apr 20, 2018

View reviewed changes

pietroalbini assigned petrochenkov Apr 20, 2018

pietroalbini added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 20, 2018

abonander closed this Apr 27, 2018

abonander mentioned this pull request Apr 27, 2018

Attribute macros invoked at crate root have issues #41430

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Treat crates specially when expanding #50101

[WIP] Treat crates specially when expanding #50101

abonander commented Apr 20, 2018 •

edited

Loading

abonander Apr 20, 2018 •

edited

Loading

abonander Apr 20, 2018

abonander Apr 20, 2018

abonander commented Apr 20, 2018

rust-highfive commented Apr 20, 2018

alexcrichton commented Apr 20, 2018

abonander commented Apr 21, 2018

abonander commented Apr 21, 2018

Rantanen commented Apr 21, 2018

abonander commented Apr 21, 2018

alexcrichton commented Apr 21, 2018

abonander commented Apr 21, 2018

alexcrichton commented Apr 22, 2018

abonander commented Apr 22, 2018

abonander commented Apr 27, 2018

petrochenkov commented Apr 28, 2018

		@@ -0,0 +1,23 @@
		// Copyright 2018 The Rust Project Developers. See the COPYRIGHT

[WIP] Treat crates specially when expanding #50101

[WIP] Treat crates specially when expanding #50101

Conversation

abonander commented Apr 20, 2018 • edited Loading

abonander Apr 20, 2018 • edited Loading

Choose a reason for hiding this comment

abonander Apr 20, 2018

Choose a reason for hiding this comment

abonander Apr 20, 2018

Choose a reason for hiding this comment

abonander commented Apr 20, 2018

rust-highfive commented Apr 20, 2018

alexcrichton commented Apr 20, 2018

abonander commented Apr 21, 2018

abonander commented Apr 21, 2018

Rantanen commented Apr 21, 2018

abonander commented Apr 21, 2018

alexcrichton commented Apr 21, 2018

abonander commented Apr 21, 2018

alexcrichton commented Apr 22, 2018

abonander commented Apr 22, 2018

abonander commented Apr 27, 2018

petrochenkov commented Apr 28, 2018

abonander commented Apr 20, 2018 •

edited

Loading

abonander Apr 20, 2018 •

edited

Loading