Skip to content

Commit

Permalink
Rollup merge of rust-lang#75857 - dtolnay:unsafe, r=nagisa
Browse files Browse the repository at this point in the history 
Syntactically permit unsafety on mods

Similar to rust-lang#66183; we will accept these constructs syntactically but reject with a semantic check after macro expansion if a proc macro hasn't replaced it with something else meaningful to Rust.

```rust
#[mymacro]
unsafe mod m {
    ...
}

#[mymacro]
unsafe extern "C++" {
    ...
}
```

The intention is that this might be used as a kind of "item-level unsafe" in attribute macro DSLs -- holding things which are unsafe to declare but potentially safe to use. For example I look forward to using this in https://github.com/dtolnay/cxx.

In the absence of a procedural macro rewriting them to something else, they'll continue to be rejected at compile time though with a better error message than before.

### Before:

```console
error: expected item, found keyword `unsafe`
 --> src/main.rs:1:1
  |
1 | unsafe mod m {
  | ^^^^^^ expected item
```

### After:

```console
error: module cannot be declared unsafe
 --> src/main.rs:1:1
  |
1 | unsafe mod m {
  | ^^^^^^

error: extern block cannot be declared unsafe
 --> src/main.rs:4:1
  |
4 | unsafe extern "C++" {
  | ^^^^^^
```

Closes rust-lang#68048.
  • Loading branch information
@tmandry
tmandry authored Sep 10, 2020
2 parents 8c35a92 + fd4dd00 commit 5aed495
Show file tree
Hide file tree
Showing 17 changed files with 284 additions and 37 deletions.
10 changes: 8 additions & 2 deletions compiler/rustc_ast/src/ast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2289,22 +2289,28 @@ impl FnRetTy {
/// Module declaration.
///
/// E.g., `mod foo;` or `mod foo { .. }`.
#[derive(Clone, Encodable, Decodable, Debug, Default)]
#[derive(Clone, Encodable, Decodable, Debug)]
pub struct Mod {
/// A span from the first token past `{` to the last token until `}`.
/// For `mod foo;`, the inner span ranges from the first token
/// to the last token in the external file.
pub inner: Span,
/// `unsafe` keyword accepted syntactically for macro DSLs, but not
/// semantically by Rust.
pub unsafety: Unsafe,
pub items: Vec<P<Item>>,
/// `true` for `mod foo { .. }`; `false` for `mod foo;`.
pub inline: bool,
}

/// Foreign module declaration.
///
/// E.g., `extern { .. }` or `extern C { .. }`.
/// E.g., `extern { .. }` or `extern "C" { .. }`.
#[derive(Clone, Encodable, Decodable, Debug)]
pub struct ForeignMod {
/// `unsafe` keyword accepted syntactically for macro DSLs, but not
/// semantically by Rust.
pub unsafety: Unsafe,
pub abi: Option<StrLit>,
pub items: Vec<P<ForeignItem>>,
}
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_ast/src/mut_visit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ pub fn noop_visit_ty<T: MutVisitor>(ty: &mut P<Ty>, vis: &mut T) {
}

pub fn noop_visit_foreign_mod<T: MutVisitor>(foreign_mod: &mut ForeignMod, vis: &mut T) {
let ForeignMod { abi: _, items } = foreign_mod;
let ForeignMod { unsafety: _, abi: _, items } = foreign_mod;
items.flat_map_in_place(|item| vis.flat_map_foreign_item(item));
}

Expand Down Expand Up @@ -970,7 +970,8 @@ pub fn noop_visit_fn_header<T: MutVisitor>(header: &mut FnHeader, vis: &mut T) {
vis.visit_asyncness(asyncness);
}

pub fn noop_visit_mod<T: MutVisitor>(Mod { inner, items, inline: _ }: &mut Mod, vis: &mut T) {
pub fn noop_visit_mod<T: MutVisitor>(module: &mut Mod, vis: &mut T) {
let Mod { inner, unsafety: _, items, inline: _ } = module;
vis.visit_span(inner);
items.flat_map_in_place(|item| vis.flat_map_item(item));
}
Expand All @@ -990,7 +991,7 @@ pub fn noop_visit_crate<T: MutVisitor>(krate: &mut Crate, vis: &mut T) {

let len = items.len();
if len == 0 {
let module = Mod { inner: span, items: vec![], inline: true };
let module = Mod { inner: span, unsafety: Unsafe::No, items: vec![], inline: true };
Crate { module, attrs: vec![], span, proc_macros }
} else if len == 1 {
let Item { attrs, span, kind, .. } = items.into_iter().next().unwrap().into_inner();
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_ast_passes/src/ast_validation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -990,12 +990,15 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
self.error_item_without_body(item.span, "function", msg, " { <body> }");
}
}
ItemKind::ForeignMod(_) => {
ItemKind::ForeignMod(ForeignMod { unsafety, .. }) => {
let old_item = mem::replace(&mut self.extern_mod, Some(item));
self.invalid_visibility(
&item.vis,
Some("place qualifiers on individual foreign items instead"),
);
if let Unsafe::Yes(span) = unsafety {
self.err_handler().span_err(span, "extern block cannot be declared unsafe");
}
visit::walk_item(self, item);
self.extern_mod = old_item;
return; // Avoid visiting again.
Expand Down Expand Up @@ -1029,7 +1032,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
walk_list!(self, visit_attribute, &item.attrs);
return;
}
ItemKind::Mod(Mod { inline, .. }) => {
ItemKind::Mod(Mod { inline, unsafety, .. }) => {
if let Unsafe::Yes(span) = unsafety {
self.err_handler().span_err(span, "module cannot be declared unsafe");
}
// Ensure that `path` attributes on modules are recorded as used (cf. issue #35584).
if !inline && !self.session.contains_name(&item.attrs, sym::path) {
self.check_mod_file_item_asciionly(item.ident);
Expand Down
11 changes: 9 additions & 2 deletions compiler/rustc_ast_pretty/src/pprust.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1139,7 +1139,11 @@ impl<'a> State<'a> {
self.print_fn_full(sig, item.ident, gen, &item.vis, def, body, &item.attrs);
}
ast::ItemKind::Mod(ref _mod) => {
self.head(visibility_qualified(&item.vis, "mod"));
self.head(to_string(|s| {
s.print_visibility(&item.vis);
s.print_unsafety(_mod.unsafety);
s.word("mod");
}));
self.print_ident(item.ident);

if _mod.inline || self.is_expanded {
Expand All @@ -1154,7 +1158,10 @@ impl<'a> State<'a> {
}
}
ast::ItemKind::ForeignMod(ref nmod) => {
self.head("extern");
self.head(to_string(|s| {
s.print_unsafety(nmod.unsafety);
s.word("extern");
}));
if let Some(abi) = nmod.abi {
self.print_literal(&abi.as_lit());
self.nbsp();
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ impl<'a> StripUnconfigured<'a> {
}

pub fn configure_foreign_mod(&mut self, foreign_mod: &mut ast::ForeignMod) {
let ast::ForeignMod { abi: _, items } = foreign_mod;
let ast::ForeignMod { unsafety: _, abi: _, items } = foreign_mod;
items.flat_map_in_place(|item| self.configure(item));
}

Expand Down
27 changes: 22 additions & 5 deletions compiler/rustc_expand/src/expand.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_ast::token;
use rustc_ast::tokenstream::TokenStream;
use rustc_ast::visit::{self, AssocCtxt, Visitor};
use rustc_ast::{self as ast, AttrItem, Block, LitKind, NodeId, PatKind, Path};
use rustc_ast::{ItemKind, MacArgs, MacCallStmt, MacStmtStyle, StmtKind};
use rustc_ast::{ItemKind, MacArgs, MacCallStmt, MacStmtStyle, StmtKind, Unsafe};
use rustc_ast_pretty::pprust;
use rustc_attr::{self as attr, is_builtin_attr, HasAttrs};
use rustc_data_structures::map_in_place::MapInPlace;
Expand Down Expand Up @@ -370,11 +370,21 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
None => {
// Resolution failed so we return an empty expansion
krate.attrs = vec![];
krate.module = ast::Mod { inner: orig_mod_span, items: vec![], inline: true };
krate.module = ast::Mod {
inner: orig_mod_span,
unsafety: Unsafe::No,
items: vec![],
inline: true,
};
}
Some(ast::Item { span, kind, .. }) => {
krate.attrs = vec![];
krate.module = ast::Mod { inner: orig_mod_span, items: vec![], inline: true };
krate.module = ast::Mod {
inner: orig_mod_span,
unsafety: Unsafe::No,
items: vec![],
inline: true,
};
self.cx.span_err(
span,
&format!(
Expand Down Expand Up @@ -1441,8 +1451,15 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
push_directory(&self.cx.sess, ident, &item.attrs, dir)
} else {
// We have an outline `mod foo;` so we need to parse the file.
let (new_mod, dir) =
parse_external_mod(&self.cx.sess, ident, span, dir, &mut attrs, pushed);
let (new_mod, dir) = parse_external_mod(
&self.cx.sess,
ident,
span,
old_mod.unsafety,
dir,
&mut attrs,
pushed,
);

let krate = ast::Crate {
span: new_mod.inner,
Expand Down
12 changes: 8 additions & 4 deletions compiler/rustc_expand/src/module.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rustc_ast::{token, Attribute, Mod};
use rustc_ast::{token, Attribute, Mod, Unsafe};
use rustc_errors::{struct_span_err, PResult};
use rustc_parse::new_parser_from_file;
use rustc_session::parse::ParseSess;
Expand Down Expand Up @@ -42,6 +42,7 @@ crate fn parse_external_mod(
sess: &Session,
id: Ident,
span: Span, // The span to blame on errors.
unsafety: Unsafe,
Directory { mut ownership, path }: Directory,
attrs: &mut Vec<Attribute>,
pop_mod_stack: &mut bool,
Expand All @@ -60,13 +61,16 @@ crate fn parse_external_mod(
drop(included_mod_stack);

// Actually parse the external file as a module.
let mut module =
new_parser_from_file(&sess.parse_sess, &mp.path, Some(span)).parse_mod(&token::Eof)?;
let mut parser = new_parser_from_file(&sess.parse_sess, &mp.path, Some(span));
let mut module = parser.parse_mod(&token::Eof, unsafety)?;
module.0.inline = false;
module
};
// (1) ...instead, we return a dummy module.
let (module, mut new_attrs) = result.map_err(|mut err| err.emit()).unwrap_or_default();
let (module, mut new_attrs) = result.map_err(|mut err| err.emit()).unwrap_or_else(|_| {
let module = Mod { inner: Span::default(), unsafety, items: Vec::new(), inline: false };
(module, Vec::new())
});
attrs.append(&mut new_attrs);

// Extract the directory path for submodules of `module`.
Expand Down
67 changes: 51 additions & 16 deletions compiler/rustc_parse/src/parser/item.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,35 +28,46 @@ impl<'a> Parser<'a> {
/// Parses a source module as a crate. This is the main entry point for the parser.
pub fn parse_crate_mod(&mut self) -> PResult<'a, ast::Crate> {
let lo = self.token.span;
let (module, attrs) = self.parse_mod(&token::Eof)?;
let (module, attrs) = self.parse_mod(&token::Eof, Unsafe::No)?;
let span = lo.to(self.token.span);
let proc_macros = Vec::new(); // Filled in by `proc_macro_harness::inject()`.
Ok(ast::Crate { attrs, module, span, proc_macros })
}

/// Parses a `mod <foo> { ... }` or `mod <foo>;` item.
fn parse_item_mod(&mut self, attrs: &mut Vec<Attribute>) -> PResult<'a, ItemInfo> {
let unsafety = self.parse_unsafety();
self.expect_keyword(kw::Mod)?;
let id = self.parse_ident()?;
let (module, mut inner_attrs) = if self.eat(&token::Semi) {
Default::default()
(Mod { inner: Span::default(), unsafety, items: Vec::new(), inline: false }, Vec::new())
} else {
self.expect(&token::OpenDelim(token::Brace))?;
self.parse_mod(&token::CloseDelim(token::Brace))?
self.parse_mod(&token::CloseDelim(token::Brace), unsafety)?
};
attrs.append(&mut inner_attrs);
Ok((id, ItemKind::Mod(module)))
}

/// Parses the contents of a module (inner attributes followed by module items).
pub fn parse_mod(&mut self, term: &TokenKind) -> PResult<'a, (Mod, Vec<Attribute>)> {
pub fn parse_mod(
&mut self,
term: &TokenKind,
unsafety: Unsafe,
) -> PResult<'a, (Mod, Vec<Attribute>)> {
let lo = self.token.span;
let attrs = self.parse_inner_attributes()?;
let module = self.parse_mod_items(term, lo)?;
let module = self.parse_mod_items(term, lo, unsafety)?;
Ok((module, attrs))
}

/// Given a termination token, parses all of the items in a module.
fn parse_mod_items(&mut self, term: &TokenKind, inner_lo: Span) -> PResult<'a, Mod> {
fn parse_mod_items(
&mut self,
term: &TokenKind,
inner_lo: Span,
unsafety: Unsafe,
) -> PResult<'a, Mod> {
let mut items = vec![];
while let Some(item) = self.parse_item()? {
items.push(item);
Expand All @@ -75,7 +86,7 @@ impl<'a> Parser<'a> {

let hi = if self.token.span.is_dummy() { inner_lo } else { self.prev_token.span };

Ok(Mod { inner: inner_lo.to(hi), items, inline: true })
Ok(Mod { inner: inner_lo.to(hi), unsafety, items, inline: true })
}
}

Expand Down Expand Up @@ -235,8 +246,13 @@ impl<'a> Parser<'a> {
self.parse_item_extern_crate()?
} else {
// EXTERN BLOCK
self.parse_item_foreign_mod(attrs)?
self.parse_item_foreign_mod(attrs, Unsafe::No)?
}
} else if self.is_unsafe_foreign_mod() {
// EXTERN BLOCK
let unsafety = self.parse_unsafety();
self.expect_keyword(kw::Extern)?;
self.parse_item_foreign_mod(attrs, unsafety)?
} else if self.is_static_global() {
// STATIC ITEM
self.bump(); // `static`
Expand All @@ -256,7 +272,9 @@ impl<'a> Parser<'a> {
{
// IMPL ITEM
self.parse_item_impl(attrs, def())?
} else if self.eat_keyword(kw::Mod) {
} else if self.check_keyword(kw::Mod)
|| self.check_keyword(kw::Unsafe) && self.is_keyword_ahead(1, &[kw::Mod])
{
// MODULE ITEM
self.parse_item_mod(attrs)?
} else if self.eat_keyword(kw::Type) {
Expand Down Expand Up @@ -893,10 +911,14 @@ impl<'a> Parser<'a> {
/// extern "C" {}
/// extern {}
/// ```
fn parse_item_foreign_mod(&mut self, attrs: &mut Vec<Attribute>) -> PResult<'a, ItemInfo> {
fn parse_item_foreign_mod(
&mut self,
attrs: &mut Vec<Attribute>,
unsafety: Unsafe,
) -> PResult<'a, ItemInfo> {
let abi = self.parse_abi(); // ABI?
let items = self.parse_item_list(attrs, |p| p.parse_foreign_item())?;
let module = ast::ForeignMod { abi, items };
let module = ast::ForeignMod { unsafety, abi, items };
Ok((Ident::invalid(), ItemKind::ForeignMod(module)))
}

Expand Down Expand Up @@ -938,6 +960,15 @@ impl<'a> Parser<'a> {
.emit();
}

fn is_unsafe_foreign_mod(&self) -> bool {
self.token.is_keyword(kw::Unsafe)
&& self.is_keyword_ahead(1, &[kw::Extern])
&& self.look_ahead(
2 + self.look_ahead(2, |t| t.can_begin_literal_maybe_minus() as usize),
|t| t.kind == token::OpenDelim(token::Brace),
)
}

fn is_static_global(&mut self) -> bool {
if self.check_keyword(kw::Static) {
// Check if this could be a closure.
Expand Down Expand Up @@ -1552,10 +1583,14 @@ impl<'a> Parser<'a> {
// `$qual fn` or `$qual $qual`:
|| QUALS.iter().any(|&kw| self.check_keyword(kw))
&& self.look_ahead(1, |t| {
// ...qualified and then `fn`, e.g. `const fn`.
// `$qual fn`, e.g. `const fn` or `async fn`.
t.is_keyword(kw::Fn)
// Two qualifiers. This is enough. Due `async` we need to check that it's reserved.
|| t.is_non_raw_ident_where(|i| QUALS.contains(&i.name) && i.is_reserved())
// Two qualifiers `$qual $qual` is enough, e.g. `async unsafe`.
|| t.is_non_raw_ident_where(|i| QUALS.contains(&i.name)
// Rule out 2015 `const async: T = val`.
&& i.is_reserved()
// Rule out unsafe extern block.
&& !self.is_unsafe_foreign_mod())
})
// `extern ABI fn`
|| self.check_keyword(kw::Extern)
Expand All @@ -1567,9 +1602,9 @@ impl<'a> Parser<'a> {
/// up to and including the `fn` keyword. The formal grammar is:
///
/// ```
/// Extern = "extern" StringLit ;
/// Extern = "extern" StringLit? ;
/// FnQual = "const"? "async"? "unsafe"? Extern? ;
/// FnFrontMatter = FnQual? "fn" ;
/// FnFrontMatter = FnQual "fn" ;
/// ```
pub(super) fn parse_fn_front_matter(&mut self) -> PResult<'a, FnHeader> {
let constness = self.parse_constness();
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/ast-json/ast-json-noexpand-output.stdout
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"module":{"inner":{"lo":0,"hi":0},"items":[{"attrs":[],"id":0,"span":{"lo":0,"hi":0},"vis":{"node":"Inherited","span":{"lo":0,"hi":0}},"ident":{"name":"core","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":null}],"inline":true},"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"crate_type","span":{"lo":0,"hi":0}},"id":0,"args":null}]},"args":{"variant":"Eq","fields":[{"lo":0,"hi":0},{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}]}}]},"id":null,"style":"Inner","span":{"lo":0,"hi":0}}],"span":{"lo":0,"hi":0},"proc_macros":[]}
{"module":{"inner":{"lo":0,"hi":0},"unsafety":"No","items":[{"attrs":[],"id":0,"span":{"lo":0,"hi":0},"vis":{"node":"Inherited","span":{"lo":0,"hi":0}},"ident":{"name":"core","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":null}],"inline":true},"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"crate_type","span":{"lo":0,"hi":0}},"id":0,"args":null}]},"args":{"variant":"Eq","fields":[{"lo":0,"hi":0},{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}]}}]},"id":null,"style":"Inner","span":{"lo":0,"hi":0}}],"span":{"lo":0,"hi":0},"proc_macros":[]}
Loading

0 comments on commit 5aed495

Please sign in to comment.