Skip to content

Commit

Permalink
Rollup merge of #66271 - petrochenkov:abism, r=Centril
Browse files Browse the repository at this point in the history
syntax: Keep string literals in ABIs and `asm!` more precisely

As a result we don't lose spans when `extern` functions or blocks are passed to proc macros, and also escape all string literals consistently.
Continuation of #60679, which did a similar thing with all literals besides those in ABIs and `asm!`.

TODO: Add tests.

Fixes #60493
Fixes #64561
r? @Centril
  • Loading branch information
JohnTitor authored Nov 17, 2019
2 parents d8f2526 + 28aec1b commit b83d50d
Show file tree
Hide file tree
Showing 18 changed files with 271 additions and 177 deletions.
2 changes: 1 addition & 1 deletion src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1219,7 +1219,7 @@ impl<'a> LoweringContext<'a> {
ImplTraitContext::disallowed(),
),
unsafety: f.unsafety,
abi: this.lower_abi(f.abi),
abi: this.lower_extern(f.ext),
decl: this.lower_fn_decl(&f.decl, None, false, None),
param_names: this.lower_fn_params_to_names(&f.decl),
}))
Expand Down
18 changes: 13 additions & 5 deletions src/librustc/hir/lowering/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,7 @@ impl LoweringContext<'_> {

fn lower_foreign_mod(&mut self, fm: &ForeignMod) -> hir::ForeignMod {
hir::ForeignMod {
abi: self.lower_abi(fm.abi),
abi: fm.abi.map_or(abi::Abi::C, |abi| self.lower_abi(abi)),
items: fm.items
.iter()
.map(|x| self.lower_foreign_item(x))
Expand Down Expand Up @@ -1283,18 +1283,26 @@ impl LoweringContext<'_> {
unsafety: h.unsafety,
asyncness: self.lower_asyncness(h.asyncness.node),
constness: h.constness.node,
abi: self.lower_abi(h.abi),
abi: self.lower_extern(h.ext),
}
}

pub(super) fn lower_abi(&mut self, abi: Abi) -> abi::Abi {
abi::lookup(&abi.symbol.as_str()).unwrap_or_else(|| {
pub(super) fn lower_abi(&mut self, abi: StrLit) -> abi::Abi {
abi::lookup(&abi.symbol_unescaped.as_str()).unwrap_or_else(|| {
self.error_on_invalid_abi(abi);
abi::Abi::Rust
})
}

fn error_on_invalid_abi(&self, abi: Abi) {
pub(super) fn lower_extern(&mut self, ext: Extern) -> abi::Abi {
match ext {
Extern::None => abi::Abi::Rust,
Extern::Implicit => abi::Abi::C,
Extern::Explicit(abi) => self.lower_abi(abi),
}
}

fn error_on_invalid_abi(&self, abi: StrLit) {
struct_span_err!(
self.sess,
abi.span,
Expand Down
50 changes: 38 additions & 12 deletions src/librustc_parse/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -778,13 +778,12 @@ impl<'a> Parser<'a> {

macro_rules! parse_lit {
() => {
match self.parse_lit() {
Ok(literal) => {
match self.parse_opt_lit() {
Some(literal) => {
hi = self.prev_span;
ex = ExprKind::Lit(literal);
}
Err(mut err) => {
err.cancel();
None => {
return Err(self.expected_expression_found());
}
}
Expand Down Expand Up @@ -1074,11 +1073,39 @@ impl<'a> Parser<'a> {
self.maybe_recover_from_bad_qpath(expr, true)
}

/// Matches `lit = true | false | token_lit`.
/// Returns a string literal if the next token is a string literal.
/// In case of error returns `Some(lit)` if the next token is a literal with a wrong kind,
/// and returns `None` if the next token is not literal at all.
pub fn parse_str_lit(&mut self) -> Result<ast::StrLit, Option<Lit>> {
match self.parse_opt_lit() {
Some(lit) => match lit.kind {
ast::LitKind::Str(symbol_unescaped, style) => Ok(ast::StrLit {
style,
symbol: lit.token.symbol,
suffix: lit.token.suffix,
span: lit.span,
symbol_unescaped,
}),
_ => Err(Some(lit)),
}
None => Err(None),
}
}

pub(super) fn parse_lit(&mut self) -> PResult<'a, Lit> {
self.parse_opt_lit().ok_or_else(|| {
let msg = format!("unexpected token: {}", self.this_token_descr());
self.span_fatal(self.token.span, &msg)
})
}

/// Matches `lit = true | false | token_lit`.
/// Returns `None` if the next token is not a literal.
pub(super) fn parse_opt_lit(&mut self) -> Option<Lit> {
let mut recovered = None;
if self.token == token::Dot {
// Attempt to recover `.4` as `0.4`.
// Attempt to recover `.4` as `0.4`. We don't currently have any syntax where
// dot would follow an optional literal, so we do this unconditionally.
recovered = self.look_ahead(1, |next_token| {
if let token::Literal(token::Lit { kind: token::Integer, symbol, suffix })
= next_token.kind {
Expand Down Expand Up @@ -1107,11 +1134,10 @@ impl<'a> Parser<'a> {
match Lit::from_token(token) {
Ok(lit) => {
self.bump();
Ok(lit)
Some(lit)
}
Err(LitError::NotLiteral) => {
let msg = format!("unexpected token: {}", self.this_token_descr());
Err(self.span_fatal(token.span, &msg))
None
}
Err(err) => {
let span = token.span;
Expand All @@ -1120,18 +1146,18 @@ impl<'a> Parser<'a> {
_ => unreachable!(),
};
self.bump();
self.error_literal_from_token(err, lit, span);
self.report_lit_error(err, lit, span);
// Pack possible quotes and prefixes from the original literal into
// the error literal's symbol so they can be pretty-printed faithfully.
let suffixless_lit = token::Lit::new(lit.kind, lit.symbol, None);
let symbol = Symbol::intern(&suffixless_lit.to_string());
let lit = token::Lit::new(token::Err, symbol, lit.suffix);
Lit::from_lit_token(lit, span).map_err(|_| unreachable!())
Some(Lit::from_lit_token(lit, span).unwrap_or_else(|_| unreachable!()))
}
}
}

fn error_literal_from_token(&self, err: LitError, lit: token::Lit, span: Span) {
fn report_lit_error(&self, err: LitError, lit: token::Lit, span: Span) {
// Checks if `s` looks like i32 or u1234 etc.
fn looks_like_width_suffix(first_chars: &[char], s: &str) -> bool {
s.len() > 1
Expand Down
41 changes: 24 additions & 17 deletions src/librustc_parse/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ use super::diagnostics::{Error, dummy_arg, ConsumeClosingDelim};

use crate::maybe_whole;

use syntax::ast::{self, Abi, DUMMY_NODE_ID, Ident, Attribute, AttrKind, AttrStyle, AnonConst, Item};
use syntax::ast::{self, DUMMY_NODE_ID, Ident, Attribute, AttrKind, AttrStyle, AnonConst, Item};
use syntax::ast::{ItemKind, ImplItem, ImplItemKind, TraitItem, TraitItemKind, UseTree, UseTreeKind};
use syntax::ast::{PathSegment, IsAuto, Constness, IsAsync, Unsafety, Defaultness};
use syntax::ast::{PathSegment, IsAuto, Constness, IsAsync, Unsafety, Defaultness, Extern, StrLit};
use syntax::ast::{Visibility, VisibilityKind, Mutability, FnHeader, ForeignItem, ForeignItemKind};
use syntax::ast::{Ty, TyKind, Generics, TraitRef, EnumDef, VariantData, StructField};
use syntax::ast::{Mac, MacDelimiter, Block, BindingMode, FnDecl, FnSig, SelfKind, Param};
Expand Down Expand Up @@ -105,7 +105,7 @@ impl<'a> Parser<'a> {
return Ok(Some(self.parse_item_extern_crate(lo, vis, attrs)?));
}

let abi = self.parse_opt_abi()?;
let abi = self.parse_abi();

if self.eat_keyword(kw::Fn) {
// EXTERN FUNCTION ITEM
Expand All @@ -114,7 +114,7 @@ impl<'a> Parser<'a> {
unsafety: Unsafety::Normal,
asyncness: respan(fn_span, IsAsync::NotAsync),
constness: respan(fn_span, Constness::NotConst),
abi,
ext: Extern::from_abi(abi),
};
return self.parse_item_fn(lo, vis, attrs, header);
} else if self.check(&token::OpenDelim(token::Brace)) {
Expand Down Expand Up @@ -143,14 +143,14 @@ impl<'a> Parser<'a> {
if self.check_keyword(kw::Extern) {
self.sess.gated_spans.gate(sym::const_extern_fn, lo.to(self.token.span));
}
let abi = self.parse_extern_abi()?;
let ext = self.parse_extern()?;
self.bump(); // `fn`

let header = FnHeader {
unsafety,
asyncness: respan(const_span, IsAsync::NotAsync),
constness: respan(const_span, Constness::Const),
abi,
ext,
};
return self.parse_item_fn(lo, vis, attrs, header);
}
Expand Down Expand Up @@ -193,7 +193,7 @@ impl<'a> Parser<'a> {
unsafety,
asyncness,
constness: respan(fn_span, Constness::NotConst),
abi: Abi::new(sym::Rust, fn_span),
ext: Extern::None,
};
return self.parse_item_fn(lo, vis, attrs, header);
}
Expand Down Expand Up @@ -230,7 +230,7 @@ impl<'a> Parser<'a> {
unsafety: Unsafety::Normal,
asyncness: respan(fn_span, IsAsync::NotAsync),
constness: respan(fn_span, Constness::NotConst),
abi: Abi::new(sym::Rust, fn_span),
ext: Extern::None,
};
return self.parse_item_fn(lo, vis, attrs, header);
}
Expand All @@ -242,14 +242,14 @@ impl<'a> Parser<'a> {
self.bump(); // `unsafe`
// `{` is also expected after `unsafe`; in case of error, include it in the diagnostic.
self.check(&token::OpenDelim(token::Brace));
let abi = self.parse_extern_abi()?;
let ext = self.parse_extern()?;
self.expect_keyword(kw::Fn)?;
let fn_span = self.prev_span;
let header = FnHeader {
unsafety: Unsafety::Unsafe,
asyncness: respan(fn_span, IsAsync::NotAsync),
constness: respan(fn_span, Constness::NotConst),
abi,
ext,
};
return self.parse_item_fn(lo, vis, attrs, header);
}
Expand Down Expand Up @@ -1100,7 +1100,7 @@ impl<'a> Parser<'a> {
fn parse_item_foreign_mod(
&mut self,
lo: Span,
abi: Abi,
abi: Option<StrLit>,
visibility: Visibility,
mut attrs: Vec<Attribute>,
extern_sp: Span,
Expand Down Expand Up @@ -1775,9 +1775,16 @@ impl<'a> Parser<'a> {
attrs: Vec<Attribute>,
header: FnHeader,
) -> PResult<'a, Option<P<Item>>> {
let is_c_abi = match header.ext {
ast::Extern::None => false,
ast::Extern::Implicit => true,
ast::Extern::Explicit(abi) => abi.symbol_unescaped == sym::C,
};
let (ident, decl, generics) = self.parse_fn_sig(ParamCfg {
is_self_allowed: false,
allow_c_variadic: header.abi.symbol == sym::C && header.unsafety == Unsafety::Unsafe,
// FIXME: Parsing should not depend on ABI or unsafety and
// the variadic parameter should always be parsed.
allow_c_variadic: is_c_abi && header.unsafety == Unsafety::Unsafe,
is_name_required: |_| true,
})?;
let (inner_attrs, body) = self.parse_inner_attrs_and_block()?;
Expand Down Expand Up @@ -1905,19 +1912,19 @@ impl<'a> Parser<'a> {
}
let asyncness = respan(self.prev_span, asyncness);
let unsafety = self.parse_unsafety();
let (constness, unsafety, abi) = if is_const_fn {
(respan(const_span, Constness::Const), unsafety, Abi::default())
let (constness, unsafety, ext) = if is_const_fn {
(respan(const_span, Constness::Const), unsafety, Extern::None)
} else {
let abi = self.parse_extern_abi()?;
(respan(self.prev_span, Constness::NotConst), unsafety, abi)
let ext = self.parse_extern()?;
(respan(self.prev_span, Constness::NotConst), unsafety, ext)
};
if !self.eat_keyword(kw::Fn) {
// It is possible for `expect_one_of` to recover given the contents of
// `self.expected_tokens`, therefore, do not use `self.unexpected()` which doesn't
// account for this.
if !self.expect_one_of(&[], &[])? { unreachable!() }
}
Ok(FnHeader { constness, unsafety, asyncness, abi })
Ok(FnHeader { constness, unsafety, asyncness, ext })
}

/// Parse the "signature", including the identifier, parameters, and generics of a function.
Expand Down
64 changes: 15 additions & 49 deletions src/librustc_parse/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ use crate::{Directory, DirectoryOwnership};
use crate::lexer::UnmatchedBrace;

use syntax::ast::{
self, Abi, DUMMY_NODE_ID, AttrStyle, Attribute, CrateSugar, Ident,
IsAsync, MacDelimiter, Mutability, StrStyle, Visibility, VisibilityKind, Unsafety,
self, DUMMY_NODE_ID, AttrStyle, Attribute, CrateSugar, Extern, Ident, StrLit,
IsAsync, MacDelimiter, Mutability, Visibility, VisibilityKind, Unsafety,
};

use syntax::print::pprust;
Expand Down Expand Up @@ -1212,40 +1212,34 @@ impl<'a> Parser<'a> {
}

/// Parses `extern string_literal?`.
/// If `extern` is not found, the Rust ABI is used.
/// If `extern` is found and a `string_literal` does not follow, the C ABI is used.
fn parse_extern_abi(&mut self) -> PResult<'a, Abi> {
fn parse_extern(&mut self) -> PResult<'a, Extern> {
Ok(if self.eat_keyword(kw::Extern) {
self.parse_opt_abi()?
Extern::from_abi(self.parse_abi())
} else {
Abi::default()
Extern::None
})
}

/// Parses a string literal as an ABI spec.
/// If one is not found, the "C" ABI is used.
fn parse_opt_abi(&mut self) -> PResult<'a, Abi> {
let span = if self.token.can_begin_literal_or_bool() {
let ast::Lit { span, kind, .. } = self.parse_lit()?;
match kind {
ast::LitKind::Str(symbol, _) => return Ok(Abi::new(symbol, span)),
ast::LitKind::Err(_) => {}
fn parse_abi(&mut self) -> Option<StrLit> {
match self.parse_str_lit() {
Ok(str_lit) => Some(str_lit),
Err(Some(lit)) => match lit.kind {
ast::LitKind::Err(_) => None,
_ => {
self.struct_span_err(span, "non-string ABI literal")
self.struct_span_err(lit.span, "non-string ABI literal")
.span_suggestion(
span,
lit.span,
"specify the ABI with a string literal",
"\"C\"".to_string(),
Applicability::MaybeIncorrect,
)
.emit();
None
}
}
span
} else {
self.prev_span
};
Ok(Abi::new(sym::C, span))
Err(None) => None,
}
}

/// We are parsing `async fn`. If we are on Rust 2015, emit an error.
Expand Down Expand Up @@ -1337,34 +1331,6 @@ impl<'a> Parser<'a> {
self.look_ahead(1, |t| *t == token::OpenDelim(token::Brace) ||
*t == token::BinOp(token::Star))
}

fn parse_optional_str(&mut self) -> Option<(Symbol, ast::StrStyle, Option<ast::Name>)> {
let ret = match self.token.kind {
token::Literal(token::Lit { kind: token::Str, symbol, suffix }) =>
(symbol, ast::StrStyle::Cooked, suffix),
token::Literal(token::Lit { kind: token::StrRaw(n), symbol, suffix }) =>
(symbol, ast::StrStyle::Raw(n), suffix),
_ => return None
};
self.bump();
Some(ret)
}

pub fn parse_str(&mut self) -> PResult<'a, (Symbol, StrStyle)> {
match self.parse_optional_str() {
Some((s, style, suf)) => {
let sp = self.prev_span;
self.expect_no_suffix(sp, "a string literal", suf);
Ok((s, style))
}
_ => {
let msg = "expected string literal";
let mut err = self.fatal(msg);
err.span_label(self.token.span, msg);
Err(err)
}
}
}
}

crate fn make_unclosed_delims_error(
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_parse/parser/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ impl<'a> Parser<'a> {
*/

let unsafety = self.parse_unsafety();
let abi = self.parse_extern_abi()?;
let ext = self.parse_extern()?;
self.expect_keyword(kw::Fn)?;
let cfg = ParamCfg {
is_self_allowed: false,
Expand All @@ -296,7 +296,7 @@ impl<'a> Parser<'a> {
};
let decl = self.parse_fn_decl(cfg, false)?;
Ok(TyKind::BareFn(P(BareFnTy {
abi,
ext,
unsafety,
generic_params,
decl,
Expand Down
Loading

0 comments on commit b83d50d

Please sign in to comment.