squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Merge #6700

Browse source 
6700: More macro diagnostics improvements r=jonas-schievink a=jonas-schievink

This threads macro expansion errors through `eager.rs` and the `AsMacroCall` trait, improving macro diagnostics emitted during body lowering.

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
This commit is contained in:
bors[bot] 2020-12-03 16:55:15 +00:00 committed by GitHub
commit d46fce88f5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 226 additions and 70 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
crates/hir_def/src/body.rs
View file

@ -120,18 +120,24 @@ impl Expander {
self.resolve_path_as_macro(db, &path)
};
let call_id = match macro_call.as_call_id(db, self.crate_def_map.krate, resolver) {
let mut err = None;
let call_id =
macro_call.as_call_id_with_errors(db, self.crate_def_map.krate, resolver, &mut |e| {
err.get_or_insert(e);
});
let call_id = match call_id {
Some(it) => it,
None => {
// FIXME: this can mean other things too, but `as_call_id` doesn't provide enough
// info.
return ExpandResult::only_err(mbe::ExpandError::Other(
"failed to parse or resolve macro invocation".into(),
));
if err.is_none() {
eprintln!("no error despite `as_call_id_with_errors` returning `None`");
}
return ExpandResult { value: None, err };
}
};
let err = db.macro_expand_error(call_id);
if err.is_none() {
err = db.macro_expand_error(call_id);
}
let file_id = call_id.as_file();

28
crates/hir_def/src/body/tests.rs
View file

@ -78,21 +78,41 @@ fn f() {
fn macro_diag_builtin() {
check_diagnostics(
r#"
#[rustc_builtin_macro]
macro_rules! env {}
#[rustc_builtin_macro]
macro_rules! include {}
#[rustc_builtin_macro]
macro_rules! compile_error {}
#[rustc_builtin_macro]
macro_rules! format_args {
() => {}
}
fn f() {
// Test a handful of built-in (eager) macros:
include!(invalid);
//^^^^^^^^^^^^^^^^^ failed to parse or resolve macro invocation
//^^^^^^^^^^^^^^^^^ could not convert tokens
include!("does not exist");
//^^^^^^^^^^^^^^^^^^^^^^^^^^ failed to parse or resolve macro invocation
//^^^^^^^^^^^^^^^^^^^^^^^^^^ could not convert tokens
env!(invalid);
//^^^^^^^^^^^^^ failed to parse or resolve macro invocation
//^^^^^^^^^^^^^ could not convert tokens
env!("OUT_DIR");
//^^^^^^^^^^^^^^^ `OUT_DIR` not set, enable "load out dirs from check" to fix
compile_error!("compile_error works");
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `compile_error!` called: compile_error works
// Lazy:
format_args!();
//^^^^^^^^^^^^^^ failed to parse or resolve macro invocation
//^^^^^^^^^^^^^^ no rule matches input tokens
}
"#,
);

57
crates/hir_def/src/lib.rs
View file

@ -465,21 +465,37 @@ pub trait AsMacroCall {
db: &dyn db::DefDatabase,
krate: CrateId,
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
) -> Option<MacroCallId>;
}
) -> Option<MacroCallId> {
self.as_call_id_with_errors(db, krate, resolver, &mut |_| ())
}
impl AsMacroCall for InFile<&ast::MacroCall> {
fn as_call_id(
fn as_call_id_with_errors(
&self,
db: &dyn db::DefDatabase,
krate: CrateId,
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
error_sink: &mut dyn FnMut(mbe::ExpandError),
) -> Option<MacroCallId>;
}
impl AsMacroCall for InFile<&ast::MacroCall> {
fn as_call_id_with_errors(
&self,
db: &dyn db::DefDatabase,
krate: CrateId,
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
error_sink: &mut dyn FnMut(mbe::ExpandError),
) -> Option<MacroCallId> {
let ast_id = AstId::new(self.file_id, db.ast_id_map(self.file_id).ast_id(self.value));
let h = Hygiene::new(db.upcast(), self.file_id);
let path = path::ModPath::from_src(self.value.path()?, &h)?;
let path = self.value.path().and_then(|path| path::ModPath::from_src(path, &h));
AstIdWithPath::new(ast_id.file_id, ast_id.value, path).as_call_id(db, krate, resolver)
if path.is_none() {
error_sink(mbe::ExpandError::Other("malformed macro invocation".into()));
}
AstIdWithPath::new(ast_id.file_id, ast_id.value, path?)
.as_call_id_with_errors(db, krate, resolver, error_sink)
}
}
@ -497,22 +513,32 @@ impl<T: ast::AstNode> AstIdWithPath<T> {
}
impl AsMacroCall for AstIdWithPath<ast::MacroCall> {
fn as_call_id(
fn as_call_id_with_errors(
&self,
db: &dyn db::DefDatabase,
krate: CrateId,
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
error_sink: &mut dyn FnMut(mbe::ExpandError),
) -> Option<MacroCallId> {
let def: MacroDefId = resolver(self.path.clone())?;
let def: MacroDefId = resolver(self.path.clone()).or_else(|| {
error_sink(mbe::ExpandError::Other("could not resolve macro".into()));
None
})?;
if let MacroDefKind::BuiltInEager(_) = def.kind {
let macro_call = InFile::new(self.ast_id.file_id, self.ast_id.to_node(db.upcast()));
let hygiene = Hygiene::new(db.upcast(), self.ast_id.file_id);
Some(
expand_eager_macro(db.upcast(), krate, macro_call, def, &|path: ast::Path| {
resolver(path::ModPath::from_src(path, &hygiene)?)
})?
expand_eager_macro(
db.upcast(),
krate,
macro_call,
def,
&|path: ast::Path| resolver(path::ModPath::from_src(path, &hygiene)?),
error_sink,
)
.ok()?
.into(),
)
} else {
@ -522,13 +548,18 @@ impl AsMacroCall for AstIdWithPath<ast::MacroCall> {
}
impl AsMacroCall for AstIdWithPath<ast::Item> {
fn as_call_id(
fn as_call_id_with_errors(
&self,
db: &dyn db::DefDatabase,
krate: CrateId,
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
error_sink: &mut dyn FnMut(mbe::ExpandError),
) -> Option<MacroCallId> {
let def = resolver(self.path.clone())?;
let def: MacroDefId = resolver(self.path.clone()).or_else(|| {
error_sink(mbe::ExpandError::Other("could not resolve macro".into()));
None
})?;
Some(
def.as_lazy_macro(
db.upcast(),

74
crates/hir_expand/src/builtin_macro.rs
View file

@ -86,7 +86,6 @@ pub fn find_builtin_macro(
register_builtin! {
LAZY:
(column, Column) => column_expand,
(compile_error, CompileError) => compile_error_expand,
(file, File) => file_expand,
(line, Line) => line_expand,
(assert, Assert) => assert_expand,
@ -97,6 +96,7 @@ register_builtin! {
(format_args_nl, FormatArgsNl) => format_args_expand,
EAGER:
(compile_error, CompileError) => compile_error_expand,
(concat, Concat) => concat_expand,
(include, Include) => include_expand,
(include_bytes, IncludeBytes) => include_bytes_expand,
@ -213,25 +213,6 @@ fn file_expand(
ExpandResult::ok(expanded)
}
fn compile_error_expand(
_db: &dyn AstDatabase,
_id: LazyMacroId,
tt: &tt::Subtree,
) -> ExpandResult<tt::Subtree> {
if tt.count() == 1 {
if let tt::TokenTree::Leaf(tt::Leaf::Literal(it)) = &tt.token_trees[0] {
let s = it.text.as_str();
if s.contains('"') {
return ExpandResult::ok(quote! { loop { #it }});
}
};
}
ExpandResult::only_err(mbe::ExpandError::BindingError(
"`compile_error!` argument be a string".into(),
))
}
fn format_args_expand(
_db: &dyn AstDatabase,
_id: LazyMacroId,
@ -280,6 +261,30 @@ fn unquote_str(lit: &tt::Literal) -> Option<String> {
token.value().map(|it| it.into_owned())
}
fn compile_error_expand(
_db: &dyn AstDatabase,
_id: EagerMacroId,
tt: &tt::Subtree,
) -> ExpandResult<Option<(tt::Subtree, FragmentKind)>> {
let err = match &*tt.token_trees {
[tt::TokenTree::Leaf(tt::Leaf::Literal(it))] => {
let text = it.text.as_str();
if text.starts_with('"') && text.ends_with('"') {
// FIXME: does not handle raw strings
mbe::ExpandError::Other(format!(
"`compile_error!` called: {}",
&text[1..text.len() - 1]
))
} else {
mbe::ExpandError::BindingError("`compile_error!` argument must be a string".into())
}
}
_ => mbe::ExpandError::BindingError("`compile_error!` argument must be a string".into()),
};
ExpandResult { value: Some((quote! {}, FragmentKind::Items)), err: Some(err) }
}
fn concat_expand(
_db: &dyn AstDatabase,
_arg_id: EagerMacroId,
@ -417,17 +422,25 @@ fn env_expand(
Err(e) => return ExpandResult::only_err(e),
};
// FIXME:
// If the environment variable is not defined int rustc, then a compilation error will be emitted.
// We might do the same if we fully support all other stuffs.
// But for now on, we should return some dummy string for better type infer purpose.
// However, we cannot use an empty string here, because for
// `include!(concat!(env!("OUT_DIR"), "/foo.rs"))` will become
// `include!("foo.rs"), which might go to infinite loop
let s = get_env_inner(db, arg_id, &key).unwrap_or_else(|| "__RA_UNIMPLEMENTED__".to_string());
let mut err = None;
let s = get_env_inner(db, arg_id, &key).unwrap_or_else(|| {
// The only variable rust-analyzer ever sets is `OUT_DIR`, so only diagnose that to avoid
// unnecessary diagnostics for eg. `CARGO_PKG_NAME`.
if key == "OUT_DIR" {
err = Some(mbe::ExpandError::Other(
r#"`OUT_DIR` not set, enable "load out dirs from check" to fix"#.into(),
));
}
// If the variable is unset, still return a dummy string to help type inference along.
// We cannot use an empty string here, because for
// `include!(concat!(env!("OUT_DIR"), "/foo.rs"))` will become
// `include!("foo.rs"), which might go to infinite loop
"__RA_UNIMPLEMENTED__".to_string()
});
let expanded = quote! { #s };
ExpandResult::ok(Some((expanded, FragmentKind::Expr)))
ExpandResult { value: Some((expanded, FragmentKind::Expr)), err }
}
fn option_env_expand(
@ -638,7 +651,8 @@ mod tests {
"#,
);
assert_eq!(expanded, r#"loop{"error!"}"#);
// This expands to nothing (since it's in item position), but emits an error.
assert_eq!(expanded, "");
}
#[test]

1
crates/hir_expand/src/db.rs
View file

@ -207,6 +207,7 @@ fn macro_expand_with_arg(
} else {
return ExpandResult {
value: Some(db.lookup_intern_eager_expansion(id).subtree),
// FIXME: There could be errors here!
err: None,
};
}

116
crates/hir_expand/src/eager.rs
View file

@ -26,19 +26,89 @@ use crate::{
};
use base_db::CrateId;
use mbe::ExpandResult;
use parser::FragmentKind;
use std::sync::Arc;
use syntax::{algo::SyntaxRewriter, SyntaxNode};
pub struct ErrorEmitted {
_private: (),
}
trait ErrorSink {
fn emit(&mut self, err: mbe::ExpandError);
fn option<T>(
&mut self,
opt: Option<T>,
error: impl FnOnce() -> mbe::ExpandError,
) -> Result<T, ErrorEmitted> {
match opt {
Some(it) => Ok(it),
None => {
self.emit(error());
Err(ErrorEmitted { _private: () })
}
}
}
fn option_with<T>(
&mut self,
opt: impl FnOnce() -> Option<T>,
error: impl FnOnce() -> mbe::ExpandError,
) -> Result<T, ErrorEmitted> {
self.option(opt(), error)
}
fn result<T>(&mut self, res: Result<T, mbe::ExpandError>) -> Result<T, ErrorEmitted> {
match res {
Ok(it) => Ok(it),
Err(e) => {
self.emit(e);
Err(ErrorEmitted { _private: () })
}
}
}
fn expand_result_option<T>(&mut self, res: ExpandResult<Option<T>>) -> Result<T, ErrorEmitted> {
match (res.value, res.err) {
(None, Some(err)) => {
self.emit(err);
Err(ErrorEmitted { _private: () })
}
(Some(value), opt_err) => {
if let Some(err) = opt_err {
self.emit(err);
}
Ok(value)
}
(None, None) => unreachable!("`ExpandResult` without value or error"),
}
}
}
impl ErrorSink for &'_ mut dyn FnMut(mbe::ExpandError) {
fn emit(&mut self, err: mbe::ExpandError) {
self(err);
}
}
fn err(msg: impl Into<String>) -> mbe::ExpandError {
mbe::ExpandError::Other(msg.into())
}
pub fn expand_eager_macro(
db: &dyn AstDatabase,
krate: CrateId,
macro_call: InFile<ast::MacroCall>,
def: MacroDefId,
resolver: &dyn Fn(ast::Path) -> Option<MacroDefId>,
) -> Option<EagerMacroId> {
let args = macro_call.value.token_tree()?;
let parsed_args = mbe::ast_to_token_tree(&args)?.0;
mut diagnostic_sink: &mut dyn FnMut(mbe::ExpandError),
) -> Result<EagerMacroId, ErrorEmitted> {
let parsed_args = diagnostic_sink.option_with(
|| Some(mbe::ast_to_token_tree(&macro_call.value.token_tree()?)?.0),
|| err("malformed macro invocation"),
)?;
// Note:
// When `lazy_expand` is called, its *parent* file must be already exists.
@ -55,17 +125,22 @@ pub fn expand_eager_macro(
});
let arg_file_id: MacroCallId = arg_id.into();
let parsed_args = mbe::token_tree_to_syntax_node(&parsed_args, FragmentKind::Expr).ok()?.0;
let parsed_args =
diagnostic_sink.result(mbe::token_tree_to_syntax_node(&parsed_args, FragmentKind::Expr))?.0;
let result = eager_macro_recur(
db,
InFile::new(arg_file_id.as_file(), parsed_args.syntax_node()),
krate,
resolver,
diagnostic_sink,
)?;
let subtree = to_subtree(&result)?;
let subtree =
diagnostic_sink.option(to_subtree(&result), || err("failed to parse macro result"))?;
if let MacroDefKind::BuiltInEager(eager) = def.kind {
let (subtree, fragment) = eager.expand(db, arg_id, &subtree).value?;
let res = eager.expand(db, arg_id, &subtree);
let (subtree, fragment) = diagnostic_sink.expand_result_option(res)?;
let eager = EagerCallLoc {
def,
fragment,
@ -74,9 +149,9 @@ pub fn expand_eager_macro(
file_id: macro_call.file_id,
};
Some(db.intern_eager_expansion(eager))
Ok(db.intern_eager_expansion(eager))
} else {
None
panic!("called `expand_eager_macro` on non-eager macro def {:?}", def);
}
}
@ -91,13 +166,16 @@ fn lazy_expand(
def: &MacroDefId,
macro_call: InFile<ast::MacroCall>,
krate: CrateId,
) -> Option<InFile<SyntaxNode>> {
) -> ExpandResult<Option<InFile<SyntaxNode>>> {
let ast_id = db.ast_id_map(macro_call.file_id).ast_id(&macro_call.value);
let id: MacroCallId =
def.as_lazy_macro(db, krate, MacroCallKind::FnLike(macro_call.with_value(ast_id))).into();
db.parse_or_expand(id.as_file()).map(|node| InFile::new(id.as_file(), node))
let err = db.macro_expand_error(id);
let value = db.parse_or_expand(id.as_file()).map(|node| InFile::new(id.as_file(), node));
ExpandResult { value, err }
}
fn eager_macro_recur(
@ -105,7 +183,8 @@ fn eager_macro_recur(
curr: InFile<SyntaxNode>,
krate: CrateId,
macro_resolver: &dyn Fn(ast::Path) -> Option<MacroDefId>,
) -> Option<SyntaxNode> {
mut diagnostic_sink: &mut dyn FnMut(mbe::ExpandError),
) -> Result<SyntaxNode, ErrorEmitted> {
let original = curr.value.clone();
let children = curr.value.descendants().filter_map(ast::MacroCall::cast);
@ -113,7 +192,8 @@ fn eager_macro_recur(
// Collect replacement
for child in children {
let def: MacroDefId = macro_resolver(child.path()?)?;
let def = diagnostic_sink
.option_with(|| macro_resolver(child.path()?), || err("failed to resolve macro"))?;
let insert = match def.kind {
MacroDefKind::BuiltInEager(_) => {
let id: MacroCallId = expand_eager_macro(
@ -122,17 +202,21 @@ fn eager_macro_recur(
curr.with_value(child.clone()),
def,
macro_resolver,
diagnostic_sink,
)?
.into();
db.parse_or_expand(id.as_file())?
db.parse_or_expand(id.as_file())
.expect("successful macro expansion should be parseable")
}
MacroDefKind::Declarative
| MacroDefKind::BuiltIn(_)
| MacroDefKind::BuiltInDerive(_)
| MacroDefKind::ProcMacro(_) => {
let expanded = lazy_expand(db, &def, curr.with_value(child.clone()), krate)?;
let res = lazy_expand(db, &def, curr.with_value(child.clone()), krate);
let val = diagnostic_sink.expand_result_option(res)?;
// replace macro inside
eager_macro_recur(db, expanded, krate, macro_resolver)?
eager_macro_recur(db, val, krate, macro_resolver, diagnostic_sink)?
}
};
@ -140,5 +224,5 @@ fn eager_macro_recur(
}
let res = rewriter.rewrite(&original);
Some(res)
Ok(res)
}