Use hygiene for AST passes #63919

matthewjasper · 2019-08-26T17:07:28Z

AST passes are now able to have resolve consider their expansions as if they were opaque macros defined either in some module in the current crate, or a fake empty module with #[no_implicit_prelude].

Add an ExpnKind for AST passes.
Remove gensyms in AST passes.
Remove gensyms in#[test], #[bench] and #[test_case].
Allow opaque macros to define tests.
Move tests for unit tests to their own directory.
Remove Ident::{gensym, is_gensymed} - Ident::gensym_if_underscore still exists.

cc #60869, #61019

r? @petrochenkov

petrochenkov · 2019-08-27T21:44:58Z

Nice, nice. The end of gensyms is closer with every day.

src/libsyntax_ext/test_harness.rs

src/libsyntax_ext/standard_library_imports.rs

petrochenkov · 2019-08-27T22:08:11Z

src/libsyntax_ext/test_harness.rs

-    }
-    visible_path.extend_from_slice(path);
-    visible_path
-}


I see the test harness is rewritten significantly.
Could you document what is generated now?

If identifiers with carefully located def-site spans are used instead of reexports, then we should be able to support #[test] attributes everywhere, including inside functions, blocks etc.
This also should obviate the need in #[test_case], the whole purpose of which is to add pub which is no longer necessary if the privacy check is performed from the new def-site span's point of view.

Ok, looks like with the new hygienic ident approach the test infrastructure can indeed be simplified and improved further, but nothing prevents landing it in the current intermediate state.
(I think I'll going to investigate this myself a bit later, I still don't like that apply_mark has to be used.)

src/libsyntax_pos/hygiene.rs

petrochenkov · 2019-08-27T22:13:25Z

I've left some high-level comments, but didn't read the code in detail, will do that somewhere this week.

src/librustc_resolve/macros.rs

src/libsyntax_pos/hygiene.rs

src/librustc_privacy/lib.rs

src/libsyntax_ext/proc_macro_harness.rs

petrochenkov · 2019-09-02T23:03:34Z

(Still need to review test harness and stdlib injection.)

petrochenkov · 2019-09-04T23:59:03Z

src/libsyntax_ext/test_harness.rs

-    }
-    visible_path.extend_from_slice(path);
-    visible_path
-}


Ok, looks like with the new hygienic ident approach the test infrastructure can indeed be simplified and improved further, but nothing prevents landing it in the current intermediate state.
(I think I'll going to investigate this myself a bit later, I still don't like that apply_mark has to be used.)

src/libsyntax_ext/standard_library_imports.rs

petrochenkov · 2019-09-05T00:26:12Z

src/test/ui/hygiene/prelude-import-hygiene.rs

+// Should not be used for the prelude import - not a concern in the 2015 edition
+// because `std` is already declared in the crate root.
+#[cfg(rust2018)]
+extern crate not_libstd as std;


Well, this is kinda arguable.
If we can reroute standard library (including prelude imports) to somewhere else with --extern std=/path/to/my_std, then why can't we do the same thing in source code with extern crate my_std as std.
cc #63687 (comment)

I see it the other way --extern std=/path/to/my_std means that there doesn't need to be a way to override std in source code, especially one that only works on the 2018 edition.

Looking at the linked issue, the crate in question isn't #![no_std] and uses the 2015 edition, so it's not overriding anything there.

src/libsyntax_ext/standard_library_imports.rs

`gensym_if_underscore` still exists. The symbol interner can still create arbitray gensyms, this is just not exposed publicly.

Use these to create call-site spans for AST passes when needed.

Also ensure that we're consistently using the def-site span when appropriate.

petrochenkov · 2019-09-06T17:04:39Z

@bors r+

bors · 2019-09-06T17:04:40Z

📌 Commit 3f3fc52 has been approved by petrochenkov

@petrochenkov

…etrochenkov Use hygiene for AST passes AST passes are now able to have resolve consider their expansions as if they were opaque macros defined either in some module in the current crate, or a fake empty module with `#[no_implicit_prelude]`. * Add an ExpnKind for AST passes. * Remove gensyms in AST passes. * Remove gensyms in`#[test]`, `#[bench]` and `#[test_case]`. * Allow opaque macros to define tests. * Move tests for unit tests to their own directory. * Remove `Ident::{gensym, is_gensymed}` - `Ident::gensym_if_underscore` still exists. cc rust-lang#60869, rust-lang#61019 r? @petrochenkov

bors · 2019-09-07T06:06:46Z

⌛ Testing commit 3f3fc52 with merge bf5f7249fbcdf5ff7d91909fee0b4dc09ff5326a...

Centril · 2019-09-07T06:07:24Z

@bors retry rolled up.

rust-highfive · 2019-09-07T06:08:35Z

Your PR failed (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.

2019-09-07T06:07:34.0822965Z ##[command]git config gc.auto 0
2019-09-07T06:07:34.1479006Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-09-07T06:07:34.1926917Z ##[command]git config --get-all http.proxy
2019-09-07T06:07:34.2427780Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin
2019-09-07T06:07:37.1476362Z fatal: remote error: upload-pack: not our ref bf5f7249fbcdf5ff7d91909fee0b4dc09ff5326ac_lib
2019-09-07T06:07:37.1566861Z fatal: the remote end hung up unexpectedly
2019-09-07T06:07:37.2696929Z ##[warning]Git fetch failed with exit code 128, back off 1.954 seconds before retry.

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)

@ghost

Rollup of 10 pull requests Successful merges: - #63919 (Use hygiene for AST passes) - #63927 (Filter linkcheck spurious failure) - #64149 (rustc_codegen_llvm: give names to non-alloca variable values.) - #64192 (Bail out when encountering likely missing turbofish in parser) - #64231 (Move the HIR CFG to `rustc_ast_borrowck`) - #64233 (Correct pluralisation of various diagnostic messages) - #64236 (reduce visibility) - #64240 (Include compiler-rt in the source tarball) - #64241 ([doc] Added more prereqs and note about default directory) - #64243 (Move injection of attributes from command line to `libsyntax_ext`) Failed merges: r? @ghost

eddyb · 2020-03-21T11:11:45Z

src/libsyntax_ext/test_harness.rs

        // We don't want to recurse into anything other than mods, since
        // mods or tests inside of functions will break things


Doesn't this PR solve this problem entirely?

I think that it should work for modules inside expressions of we visited them. Is a bit more work for tests that are directly in blocks.

Mostly, but it looks like some bits are still missing, see https://internals.rust-lang.org/t/allowing-test-on-inner-functions/11765.

Ah that's what @matthewjasper was mentioning, i.e. ast::Block's NodeId not being usable as the parent.

rust-highfive assigned petrochenkov Aug 26, 2019

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

This comment has been minimized.

Sign in to view

petrochenkov reviewed Aug 27, 2019

View reviewed changes

src/libsyntax_ext/test_harness.rs Outdated Show resolved Hide resolved

petrochenkov reviewed Aug 27, 2019

View reviewed changes

src/libsyntax_ext/standard_library_imports.rs Outdated Show resolved Hide resolved

petrochenkov reviewed Aug 27, 2019

View reviewed changes

src/libsyntax_pos/hygiene.rs Show resolved Hide resolved

petrochenkov mentioned this pull request Aug 28, 2019

syntax_pos::symbol::Symbol::gensym() is incompatible with stable hashing. #49300

Closed

This comment has been minimized.

Sign in to view

matthewjasper force-pushed the remove-gensymmed branch from a93b68f to b992aec Compare August 30, 2019 22:35

matthewjasper added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 31, 2019

petrochenkov reviewed Sep 2, 2019

View reviewed changes

petrochenkov reviewed Sep 5, 2019

View reviewed changes

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 5, 2019

matthewjasper added 5 commits September 5, 2019 15:07

Add an ExpnKind for AST passes

0133941

Allow ast passes to create hygienic spans

4082cd9

Make use of hygiene in AST passes

6fcdb36

Move tests for unit tests to their own directory

2a82aec

Remove Ident::{gensym, is_gensymed}

beb2f5b

`gensym_if_underscore` still exists. The symbol interner can still create arbitray gensyms, this is just not exposed publicly.

matthewjasper and others added 4 commits September 5, 2019 15:07

Fix 2018 edition expanded pretty printing

846df20

Don't call diag_span_note_once for suppressed lints

0b86782

Add with_{def_site,call_site,legacy}_ctxt, methods to Span

c8cf9f5

Use these to create call-site spans for AST passes when needed.

Document test harness generation

e552840

Also ensure that we're consistently using the def-site span when appropriate.

matthewjasper force-pushed the remove-gensymmed branch from b992aec to ff45e72 Compare September 5, 2019 15:19

This comment has been minimized.

Sign in to view

matthewjasper force-pushed the remove-gensymmed branch from ff45e72 to b299c4d Compare September 5, 2019 17:09

This comment has been minimized.

Sign in to view

Simplify std lib injection

3f3fc52

matthewjasper added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 6, 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 Sep 6, 2019

Centril mentioned this pull request Sep 7, 2019

Rollup of 10 pull requests #64246

Merged

bors merged commit 3f3fc52 into rust-lang:master Sep 7, 2019

matthewjasper deleted the remove-gensymmed branch September 7, 2019 20:28

petrochenkov mentioned this pull request Sep 21, 2019

Remove last uses of gensyms #64623

Merged

eddyb reviewed Mar 21, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use hygiene for AST passes #63919

Use hygiene for AST passes #63919

matthewjasper commented Aug 26, 2019

This comment has been minimized.

This comment has been minimized.

petrochenkov commented Aug 27, 2019

petrochenkov Aug 27, 2019

petrochenkov Sep 4, 2019

petrochenkov commented Aug 27, 2019

This comment has been minimized.

petrochenkov commented Sep 2, 2019

petrochenkov Sep 4, 2019

petrochenkov Sep 5, 2019

matthewjasper Sep 5, 2019

This comment has been minimized.

This comment has been minimized.

petrochenkov commented Sep 6, 2019

bors commented Sep 6, 2019

bors commented Sep 7, 2019

Centril commented Sep 7, 2019

rust-highfive commented Sep 7, 2019

eddyb Mar 21, 2020

matthewjasper Mar 21, 2020

petrochenkov Mar 21, 2020

eddyb Mar 21, 2020

		// We don't want to recurse into anything other than mods, since
		// mods or tests inside of functions will break things

Use hygiene for AST passes #63919

Use hygiene for AST passes #63919

Conversation

matthewjasper commented Aug 26, 2019

This comment has been minimized.

This comment has been minimized.

petrochenkov commented Aug 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petrochenkov commented Aug 27, 2019

This comment has been minimized.

petrochenkov commented Sep 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

petrochenkov commented Sep 6, 2019

bors commented Sep 6, 2019

bors commented Sep 7, 2019

Centril commented Sep 7, 2019

rust-highfive commented Sep 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment