Use a proc macro to declare preallocated symbols #59655

Zoxc · 2019-04-03T00:54:51Z

rust-highfive · 2019-04-03T01:03:14Z

The job x86_64-gnu-llvm-6.0 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:01504910:start=1554252951726268766,finish=1554252952603197523,duration=876928757
$ 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
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
Setting environment variables from .travis.yml
---
[00:07:07]    Compiling rustc_macros v0.1.0 (/checkout/src/librustc_macros)
[00:07:07] error[E0583]: file not found for module `symbols`
[00:07:07]   --> src/librustc_macros/src/lib.rs:12:5
[00:07:07]    |
[00:07:07] 12 | mod symbols;
[00:07:07]    |
[00:07:07]    |
[00:07:07]    = help: name the file either symbols.rs or symbols/mod.rs inside the directory "src/librustc_macros/src"
[00:07:07] error: aborting due to previous error
[00:07:07] 
[00:07:07] For more information about this error, try `rustc --explain E0583`.
[00:07:07] error: Could not compile `rustc_macros`.
---
travis_time:end:017f782b:start=1554253391329039133,finish=1554253391337608513,duration=8569380
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:06d40b80
$ 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:00116596
travis_time:start:00116596
$ 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/

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)

Mark-Simulacrum · 2019-04-03T01:21:14Z

@bors try for perf

bors · 2019-04-03T01:21:25Z

⌛ Trying commit e76ebf77a364229542ea4a97bd76f0f1e74ad8d1 with merge 61412ac6c97f93955672e1bafe3f9cd5e1edbab8...

bors · 2019-04-03T03:36:09Z

☀️ Try build successful - checks-travis
Build commit: 61412ac6c97f93955672e1bafe3f9cd5e1edbab8

Zoxc · 2019-04-03T03:44:53Z

@rust-timer build 61412ac6c97f93955672e1bafe3f9cd5e1edbab8

rust-timer · 2019-04-03T03:44:55Z

Success: Queued 61412ac6c97f93955672e1bafe3f9cd5e1edbab8 with parent 7641873, comparison URL.

rust-timer · 2019-04-03T10:10:45Z

Finished benchmarking try commit 61412ac6c97f93955672e1bafe3f9cd5e1edbab8

petrochenkov · 2019-04-07T15:12:06Z

Random thoughts:

Proc macros are a heavy hammer, I'd really prefer to use them only if macro_rules cannot do the job.
In this case the only thing that macro_rules cannot do is counting (or it can, but with higher complexity than a proc macro).
For the symbols pre-interned so far just writing the numbers manually would be preferable to introducing a proc macro, but I can imagine how this can become an issue if more symbols is pre-interned (e.g. all built-in types u8, u16, etc).
(FWIW, "Go To Definition" on the keywords doesn't work even with declarative macros in my editor, and mod keywords is still searchable as a text, so using a proc macro doesn't regress at least this part.)
Perf results are mostly noise, so not doing this and continuing working with strs is a real alternative.
In some sense this gives libsyntax_pos the knowledge about entities from entirely unrelated crates, that's probably not too bad given that they are are parts of the language (grammars of various attributes), like keywords.
Names of built-in attributes are one of the primary sources of strs right now, but they cannot be pre-interned without either duplicating that huge list of attributes or moving all the knowledge about attributes into syntax_pos (both variants don't look especially good).
I actually wanted to get rid of struct Keyword and move keywords to the scheme that's used for symbols in this PR - just a const Match: Symbol = .... That's probably a material for another PR.

cc @rust-lang/compiler
Any opinions on casual use of proc macros in the compiler?

src/libsyntax/ast.rs

src/libsyntax_pos/symbol.rs

Zoxc · 2019-04-09T07:46:56Z

Proc macros are a heavy hammer, I'd really prefer to use them only if macro_rules cannot do the job.
In this case the only thing that macro_rules cannot do is counting (or it can, but with higher complexity than a proc macro).
For the symbols pre-interned so far just writing the numbers manually would be preferable to introducing a proc macro, but I can imagine how this can become an issue if more symbols is pre-interned (e.g. all built-in types u8, u16, etc).

Yeah, counting with macro_rules is annoying and could quite possibly be slow if this list of symbols is extended. Also I wouldn't want to do this with manual counting.

(FWIW, "Go To Definition" on the keywords doesn't work even with declarative macros in my editor, and mod keywords is still searchable as a text, so using a proc macro doesn't regress at least this part.)

I made the keywords and symbols modules regular non-macro modules, Racer is still confused though.

Perf results are mostly noise, so not doing this and continuing working with strs is a real alternative.

Perf results are pretty irrelevant since this isn't applied to anything yet.

petrochenkov · 2019-04-09T19:16:01Z

src/libsyntax_pos/symbol.rs

+        }
+    }
+
+    keywords!();


petrochenkov · 2019-04-09T19:18:26Z

@bors r+

bors · 2019-04-09T19:18:27Z

📌 Commit 30a86cf has been approved by petrochenkov

bors · 2019-04-09T19:18:28Z

🌲 The tree is currently closed for pull requests below priority 15, this pull request will be tested once the tree is reopened

@petrochenkov

Use a proc macro to declare preallocated symbols r? @petrochenkov

Centril · 2019-04-14T23:30:45Z

@bors r-

Zoxc · 2019-04-15T05:24:45Z

@bors r=petrochenkov

bors · 2019-04-15T05:24:46Z

📌 Commit f598091 has been approved by petrochenkov

bors · 2019-04-15T05:24:56Z

⌛ Testing commit f598091 with merge 8a09e37289afb0f45e088bcb70465070a68a3c5c...

bors · 2019-04-15T07:12:10Z

💔 Test failed - checks-travis

rust-highfive · 2019-04-15T07:12:11Z

The job arm-android 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.

[01:44:21] test string::test_str_clear ... ok
[01:44:21] test string::test_str_truncate ... ok
[01:44:21] test string::test_str_truncate_invalid_len ... ok
[01:44:21] test string::test_str_truncate_split_codepoint ... ok
[01:44:21] died due to signal 11
[01:44:21] 
[01:44:21] 
[01:44:21] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "arm-linux-androideabi" "-j" "4" "--release" "--locked" "--color" "always" "--features" "panic-unwind backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "-p" "alloc" "--"
[01:44:21] expected success, got: exit code: 3
---
travis_time:end:0900cbc0:start=1555312243973300663,finish=1555312243980997977,duration=7697314
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:01bf6408
$ 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:1099af60
travis_time:start:1099af60
$ 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:18db65b4
$ 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 · 2019-04-15T09:00:13Z

@rust-lang/infra Is this a spurious segfault?

kennytm · 2019-04-15T09:07:33Z

Yes.

@bors retry #55861

bors · 2019-04-15T10:03:46Z

⌛ Testing commit f598091 with merge fcf850f...

@petrochenkov

Use a proc macro to declare preallocated symbols r? @petrochenkov

bors · 2019-04-15T12:57:29Z

☀️ Test successful - checks-travis, status-appveyor
Approved by: petrochenkov
Pushing fcf850f to master...

@petrochenkov

Preallocate BUILTIN_ATTRIBUTES symbols Builds on #59655 r? @petrochenkov

rust-highfive assigned petrochenkov Apr 3, 2019

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

Zoxc force-pushed the symbols branch from 3a931fc to e76ebf7 Compare April 3, 2019 01:10

Zoxc force-pushed the symbols branch from e76ebf7 to 907ad61 Compare April 5, 2019 01:06

Zoxc mentioned this pull request Apr 5, 2019

TLS lookups in libsyntax_pos are expensive #59718

Closed

petrochenkov reviewed Apr 7, 2019

View reviewed changes

src/libsyntax/ast.rs Outdated Show resolved Hide resolved

petrochenkov reviewed Apr 7, 2019

View reviewed changes

src/libsyntax_pos/symbol.rs Outdated Show resolved Hide resolved

petrochenkov reviewed Apr 9, 2019

View reviewed changes

src/libsyntax_pos/symbol.rs

}

}

keywords!();

Copy link

Contributor

petrochenkov Apr 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 9, 2019

Zoxc mentioned this pull request Apr 10, 2019

Preallocate BUILTIN_ATTRIBUTES symbols #59850

Merged

cramertj added a commit to cramertj/rust that referenced this pull request Apr 11, 2019

Rollup merge of rust-lang#59655 - Zoxc:symbols, r=petrochenkov

e4950c7

Use a proc macro to declare preallocated symbols r? @petrochenkov

cramertj mentioned this pull request Apr 11, 2019

Rollup of 16 pull requests #59885

Closed

This comment has been minimized.

Sign in to view

bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 12, 2019

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 14, 2019

Zoxc added 5 commits April 15, 2019 07:23

Use a proc macro to declare preallocated symbols

10855a3

Make check_name generic

61a7a60

Ensure the symbols are pure strings

afeda72

Move modules outside the proc macro

baebf79

Use colon for keyword defs

f598091

Zoxc force-pushed the symbols branch from e1b1e61 to f598091 Compare April 15, 2019 05:24

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 15, 2019

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 15, 2019

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 15, 2019

bors added a commit that referenced this pull request Apr 15, 2019

Auto merge of #59655 - Zoxc:symbols, r=petrochenkov

fcf850f

Use a proc macro to declare preallocated symbols r? @petrochenkov

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 15, 2019

bors merged commit f598091 into rust-lang:master Apr 15, 2019

Zoxc deleted the symbols branch April 15, 2019 13:18

bors added a commit that referenced this pull request Apr 15, 2019

Auto merge of #59850 - Zoxc:symbols-attrs, r=petrochenkov

9217fe0

Preallocate BUILTIN_ATTRIBUTES symbols Builds on #59655 r? @petrochenkov

fee1-dead mentioned this pull request Apr 7, 2022

Migrate symbols to use declarative macro. #95726

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a proc macro to declare preallocated symbols #59655

Use a proc macro to declare preallocated symbols #59655

Zoxc commented Apr 3, 2019

rust-highfive commented Apr 3, 2019

Mark-Simulacrum commented Apr 3, 2019

bors commented Apr 3, 2019

bors commented Apr 3, 2019

Zoxc commented Apr 3, 2019

rust-timer commented Apr 3, 2019

rust-timer commented Apr 3, 2019

petrochenkov commented Apr 7, 2019 •

edited

Loading

Zoxc commented Apr 9, 2019

petrochenkov Apr 9, 2019

petrochenkov commented Apr 9, 2019

bors commented Apr 9, 2019

bors commented Apr 9, 2019

This comment has been minimized.

Centril commented Apr 14, 2019

Zoxc commented Apr 15, 2019

bors commented Apr 15, 2019

bors commented Apr 15, 2019

bors commented Apr 15, 2019

rust-highfive commented Apr 15, 2019

Zoxc commented Apr 15, 2019

kennytm commented Apr 15, 2019

bors commented Apr 15, 2019

bors commented Apr 15, 2019

Use a proc macro to declare preallocated symbols #59655

Use a proc macro to declare preallocated symbols #59655

Conversation

Zoxc commented Apr 3, 2019

rust-highfive commented Apr 3, 2019

Mark-Simulacrum commented Apr 3, 2019

bors commented Apr 3, 2019

bors commented Apr 3, 2019

Zoxc commented Apr 3, 2019

rust-timer commented Apr 3, 2019

rust-timer commented Apr 3, 2019

petrochenkov commented Apr 7, 2019 • edited Loading

Zoxc commented Apr 9, 2019

petrochenkov Apr 9, 2019

Choose a reason for hiding this comment

petrochenkov commented Apr 9, 2019

bors commented Apr 9, 2019

bors commented Apr 9, 2019

This comment has been minimized.

Centril commented Apr 14, 2019

Zoxc commented Apr 15, 2019

bors commented Apr 15, 2019

bors commented Apr 15, 2019

bors commented Apr 15, 2019

rust-highfive commented Apr 15, 2019

Zoxc commented Apr 15, 2019

kennytm commented Apr 15, 2019

bors commented Apr 15, 2019

bors commented Apr 15, 2019

petrochenkov commented Apr 7, 2019 •

edited

Loading