Define queries using a proc macro by Zoxc · Pull Request #56462 · rust-lang/rust

Zoxc · 2018-12-03T15:23:59Z

cc @rust-lang/compiler

rust-highfive · 2018-12-03T15:24:03Z

r? @zackmdavis

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

eddyb · 2018-12-03T15:52:40Z

src/librustc/ty/query/mod.rs

This is mind-bending, but cool!

src/librustc/query/mod.rs

mark-i-m · 2018-12-03T19:13:08Z

@Zoxc This is cool :)

When it merges, could you please update https://rust-lang.github.io/rustc-guide/query.html ? That would be much appreciated!

src/librustc_macros/src/query.rs

petrochenkov · 2018-12-03T21:23:56Z

Perhaps I didn't work with queries enough, but I don't understand what's the point of the change and why is this an improvement.

eddyb · 2018-12-04T03:42:55Z

@petrochenkov This is pretty much just replacing a macro_rules macro (define_queries!) with a proc macro that has nicer (and simpler) input syntax.

The PR is more confusing atm though, because it kept both macros and daisy-chained them, to allow gradually moving from the old one to the new one.

src/librustc/ty/query/README.md

Zoxc · 2018-12-07T02:52:29Z

I forgot to include syntax for QueryDescription:

impl<'tcx> QueryDescription<'tcx> for queries::type_op_prove_predicate<'tcx> {
    fn describe(_tcx: TyCtxt<'_, '_, '_>, goal: CanonicalTypeOpProvePredicateGoal<'tcx>)
                -> Cow<'static, str> {
        format!("evaluating `type_op_prove_predicate` `{:?}`", goal).into()
    }
}

Anyone have some good ideas for that?

eddyb · 2018-12-07T10:42:41Z

I'd say something like this:

description("evaluating `type_op_prove_predicate` `{:?}`", goal)

Zoxc · 2018-12-07T14:45:04Z

@eddyb That looks very unbound.

How about:

query type_of(DefId) -> Ty<'tcx> {
 	"evaluating `finding type of `{:?}`", $key
}

mark-i-m · 2018-12-07T16:24:01Z

Why not use a doc comment?

eddyb · 2018-12-07T16:49:48Z

@mark-i-m Literally this?

/// "finding type of `{:?}`", $key`

Or something neater? Maybe like this:

/// finding type of `{key:?}`

rust-highfive · 2018-12-08T00:41:45Z

The job mingw-check 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:end:05b801f8:start=1544224442596761929,finish=1544224512640402127,duration=70043640198
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=mingw-check
---
[00:07:26] configure: build.locked-deps    := True
[00:07:26] configure: llvm.ccache          := sccache
[00:07:26] configure: build.cargo-native-static := True
[00:07:26] configure: dist.missing-tools   := True
[00:07:26] configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
[00:07:26] configure: writing `config.toml` in current directory
[00:07:26] configure: 
[00:07:26] configure: run `python /checkout/x.py --help`
[00:07:26] configure: 
---
[00:12:27]     Checking rustc-demangle v0.1.9
[00:12:27]     Checking num_cpus v1.8.0
[00:12:31]     Checking memmap v0.6.2
[00:12:31]    Compiling rustc_llvm v0.0.0 (/checkout/src/librustc_llvm)
[00:12:33] error[E0463]: can't find crate for `rustc_macros` which `rustc` depends on
[00:12:33]    |
[00:12:33] 45 | #[macro_use] extern crate rustc;
[00:12:33]    |              ^^^^^^^^^^^^^^^^^^^ can't find crate
[00:12:33] 
[00:12:33] 
[00:12:33] error: aborting due to previous error
[00:12:33] 
[00:12:33] For more information about this error, try `rustc --explain E0463`.
[00:12:33] error: Could not compile `rustc_codegen_llvm`.
[00:12:33] 
[00:12:33] To learn more, run the command again with --verbose.
[00:12:33] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "--target" "i686-pc-windows-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--manifest-path" "/checkout/src/librustc_codegen_llvm/Cargo.toml" "--message-format" "json"
[00:12:33] travis_fold:start:stage0-rustc_codegen_llvm
travis_time:start:stage0-rustc_codegen_llvm
failed to run: /checkout/obj/build/bootstrap/debug/bootstrap check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu
[00:12:33] Build completed unsuccessfully in 0:03:56
---
travis_time:end:100940bc:start=1544225275217129397,finish=1544225275222627296,duration=5497899
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:04205a02
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:04627458
travis_time:start:04627458
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:099010d8
$ dmesg | grep -i kill

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)

Zoxc · 2018-12-08T14:18:00Z

I don't think using comments is a good idea. You'd want to use those to document the queries. Also it's very surprising that comments are executable.

rust-highfive · 2018-12-08T15:49:34Z

The job mingw-check 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:end:00c700f5:start=1544278669563959679,finish=1544278723777822350,duration=54213862671
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=mingw-check
---
[00:07:32] configure: build.locked-deps    := True
[00:07:32] configure: llvm.ccache          := sccache
[00:07:32] configure: build.cargo-native-static := True
[00:07:32] configure: dist.missing-tools   := True
[00:07:32] configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
[00:07:32] configure: writing `config.toml` in current directory
[00:07:32] configure: 
[00:07:32] configure: run `python /checkout/x.py --help`
[00:07:32] configure: 
---
[00:12:39]     Checking rustc_save_analysis v0.0.0 (/checkout/src/librustc_save_analysis)
[00:12:39]     Checking rustc_codegen_ssa v0.0.0 (/checkout/src/librustc_codegen_ssa)
[00:12:44]     Checking rustc-main v0.0.0 (/checkout/src/rustc)
[00:12:45]     Finished release [optimized] target(s) in 3m 21s
[00:12:45] thread 'main' panicked at 'failed to copy `/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/release/deps/librustc_macros-9f012a3c4a8560db.so` to `/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_macros-9f012a3c4a8560db.so`: No such file or directory (os error 2)', src/bootstrap/lib.rs:1180:17
[00:12:45] travis_fold:end:stage0-rustc

[00:12:45] travis_time:end:stage0-rustc:start=1544279296114616927,finish=1544279497326681879,duration=201212064952

---
travis_time:end:09b0810d:start=1544279498047769029,finish=1544279498053695603,duration=5926574
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0bee4ee4
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0914b014
travis_time:start:0914b014
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:1a96e650
$ dmesg | grep -i kill

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)

oli-obk

just a nit and a potential refactoring

r=me with both addressed

src/librustc_macros/src/query.rs

oli-obk · 2019-03-18T14:08:36Z

@bors r+

bors · 2019-03-18T14:08:37Z

📌 Commit 198dfce has been approved by oli-obk

bors · 2019-03-18T15:04:32Z

⌛ Testing commit 198dfce with merge 3a05002e245eb2c9fe06c1ab62ef48e0c7f0f69c...

bors · 2019-03-18T19:04:32Z

💥 Test timed out

Zoxc · 2019-03-18T19:05:46Z

@bors retry

bors · 2019-03-18T21:24:22Z

⌛ Testing commit 198dfce with merge 3bf064b...

Define queries using a proc macro cc @rust-lang/compiler

bors · 2019-03-19T00:47:59Z

☀️ Test successful - checks-travis, status-appveyor
Approved by: oli-obk
Pushing 3bf064b to master...

There was a big macro rewrite in these pull requests: rust-lang/rust#56462 rust-lang/rust#59517 Update the query chapter to describe the new macro usage.

rust-highfive assigned zackmdavis Dec 3, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 3, 2018