Move syntax::ext to a syntax_expand and refactor some attribute logic #65465
Conversation
This comment has been minimized.
This comment has been minimized.
JohnTitor
added
the
S-waiting-on-review
| @@ -10,13 +10,14 @@ | |||
| #![feature(plugin_registrar, rustc_private)] | |||
There was a problem hiding this comment.
Maybe let's remove these tests? They are pretty annoying...
There was a problem hiding this comment.
These are smoke tests for syntactic plugins, the only thing making sure that they don't break.
I think they should exist until we remove the plugins themselves.
| @@ -18,7 +18,7 @@ use syntax::parse::new_parser_from_source_str; | |||
| use syntax::parse::parser::Parser; | |||
There was a problem hiding this comment.
Shouldn't this be a normal ui test?
There was a problem hiding this comment.
AFAIK, anything depending on compiler crates has to be a fulldeps test.
There was a problem hiding this comment.
Yeah, what I wonder tho if this test could not be expressed as a series of UI tests instead -- the structure seems to be just ordinary compile-pass / compile-fail wrt. "reject" and "check". Using cfg(FALSE) it seems to me we can migrate this test to use that instead and privatize some stuff in the process.
There was a problem hiding this comment.
Ah, you mean rewriting the test entirely to test snippets inside fn run() using the usual UI approach instead of libsyntax APIs.
If it's possible to do, then it would be great.
There was a problem hiding this comment.
Also, the test can be made a unit test inside libsyntax, if privacy of libsyntax APIs is an issue.
This comment has been minimized.
This comment has been minimized.
|
r=me with fulldeps tests updated. |
petrochenkov
added
S-waiting-on-author
Centril
force-pushed
the
split-syntax-1
branch
from
5aa5011 to
1a26fdf
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Centril
force-pushed
the
split-syntax-1
branch
from
69e2b43 to
8ca16dd
Compare
|
@bors r=petrochenkov |
|
📌 Commit 8ca16dd has been approved by |
bors
added
S-waiting-on-bors
Move syntax::ext to a syntax_expand and refactor some attribute logic Part of rust-lang#65324. r? @petrochenkov
Move syntax::ext to a syntax_expand and refactor some attribute logic Part of rust-lang#65324. r? @petrochenkov
Rollup of 8 pull requests Successful merges: - #65094 (Prefer statx on linux if available) - #65316 (make File::try_clone produce non-inheritable handles on Windows) - #65319 (InterpCx: make memory field public) - #65461 (Don't recommend ONCE_INIT in std::sync::Once) - #65465 (Move syntax::ext to a syntax_expand and refactor some attribute logic) - #65469 (Update libc to 0.2.64) - #65475 (add example for type_name) - #65478 (fmt::Write is about string slices, not byte slices) Failed merges: r? @ghost
Move syntax::ext to a syntax_expand and refactor some attribute logic Part of rust-lang#65324. r? @petrochenkov
Rollup of 8 pull requests Successful merges: - #65237 (Move debug_map assertions after check for err) - #65316 (make File::try_clone produce non-inheritable handles on Windows) - #65319 (InterpCx: make memory field public) - #65461 (Don't recommend ONCE_INIT in std::sync::Once) - #65465 (Move syntax::ext to a syntax_expand and refactor some attribute logic) - #65475 (add example for type_name) - #65478 (fmt::Write is about string slices, not byte slices) - #65486 (doc: fix typo in OsStrExt and OsStringExt) Failed merges: r? @ghost
| @@ -522,7 +520,7 @@ impl<'a> Parser<'a> { | |||
|
|
|||
| /// If the next token is the given keyword, eats it and returns `true`. | |||
| /// Otherwise, returns `false`. An expectation is also added for diagnostics purposes. | |||
| pub fn eat_keyword(&mut self, kw: Symbol) -> bool { | |||
| fn eat_keyword(&mut self, kw: Symbol) -> bool { | |||
There was a problem hiding this comment.
@Centril Please reconsider before making these kinds of 'refactorings.' Tools like rustfmt which rely on the libsyntax suffer severely from reduced visibility. Now I have to iterate through the changes, re-publish hidden fields and methods, make a PR and wait until it gets merged.
I understand that the libsyntax is primarily for the rustc itself, and its interface is unstable. But please be conservative about the visibility of its interface.
There was a problem hiding this comment.
@topecongiro If you rely on something then there should be a comment to that effect in the rustc sources. Reducing visibility is an important tool for cleaning up the rustc internals and should be used.
Part of #65324.
r? @petrochenkov