-
Notifications
You must be signed in to change notification settings - Fork 1.8k
internal: tighten up parser API #11134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|| seg.generic_arg_list().is_none() | ||
}); | ||
if !valid_use_path { | ||
let use_path = ast::SourceFile::parse(&format!("use {};", path)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @Veykril
This looks iffy at first, but I think that's actually the right way to check things like "is this a valid import path?" -- just construct a file with import and check this directly!
Also, looking at this, what ||
intended to be &&
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ye that was supposed to be &&
.
And I agree, this check is certainly nicer, it makes it more clear as well what is expected to be valid here.
It's tempting to expose things like
Expr::parse
,but they'll necessary have somewhat ad-hoc semantics.
Instead, we narrow down the parser's interface strictly
to what's needed for MBE. For everything else (eg, parsing
imports), the proper way is enclose the input string into
some context, parse the whole as a file, and then verify
that the input was parsed as intended.
bors r+
🤖