Skip to content
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

Teach concat!() to concatenate byte str literals #52932

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/libsyntax/ext/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use rustc_data_structures::sync::Lrc;
use rustc_target::spec::abi::Abi;
use ast::{self, Ident, Generics, Expr, BlockCheckMode, UnOp, PatKind};
use attr;
Expand Down Expand Up @@ -145,6 +146,7 @@ pub trait AstBuilder {
fn expr_vec_ng(&self, sp: Span) -> P<ast::Expr>;
fn expr_vec_slice(&self, sp: Span, exprs: Vec<P<ast::Expr>>) -> P<ast::Expr>;
fn expr_str(&self, sp: Span, s: Symbol) -> P<ast::Expr>;
fn expr_byte_str(&self, sp: Span, byte_str: Vec<u8>) -> P<ast::Expr>;

fn expr_some(&self, sp: Span, expr: P<ast::Expr>) -> P<ast::Expr>;
fn expr_none(&self, sp: Span) -> P<ast::Expr>;
Expand Down Expand Up @@ -736,6 +738,10 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
self.expr_lit(sp, ast::LitKind::Str(s, ast::StrStyle::Cooked))
}

fn expr_byte_str(&self, sp: Span, byte_str: Vec<u8>) -> P<ast::Expr> {
self.expr_lit(sp, ast::LitKind::ByteStr(Lrc::new(byte_str)))
}

fn expr_cast(&self, sp: Span, expr: P<ast::Expr>, ty: P<ast::Ty>) -> P<ast::Expr> {
self.expr(sp, ast::ExprKind::Cast(expr, ty))
}
Expand Down
110 changes: 99 additions & 11 deletions src/libsyntax_ext/concat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,45 +13,78 @@ use syntax::ext::base;
use syntax::ext::build::AstBuilder;
use syntax::symbol::Symbol;
use syntax::tokenstream;
use syntax_pos;
use syntax_pos::Span;

use std::string::String;

pub fn expand_syntax_ext(
cx: &mut base::ExtCtxt,
sp: syntax_pos::Span,
sp: Span,
tts: &[tokenstream::TokenTree],
) -> Box<dyn base::MacResult + 'static> {
let es = match base::get_exprs_from_tts(cx, sp, tts) {
Some(e) => e,
None => return base::DummyResult::expr(sp),
};
let mut accumulator = String::new();
let mut string_accumulator = String::new();
let mut string_pos = vec![];
let mut int_pos = vec![];
let mut bool_pos = vec![];
let mut b_accumulator: Vec<u8> = vec![];
let mut b_pos: Vec<Span> = vec![];
// We don't support mixing things with byte str literals, but do a best effort to fill in a
// reasonable byte str output to avoid further errors down the line.
let mut unified_accumulator: Vec<u8> = vec![];
let mut missing_literal = vec![];
for e in es {
match e.node {
ast::ExprKind::Lit(ref lit) => match lit.node {
ast::LitKind::Str(ref s, _)
| ast::LitKind::Float(ref s, _)
| ast::LitKind::FloatUnsuffixed(ref s) => {
accumulator.push_str(&s.as_str());
string_accumulator.push_str(&s.as_str());
string_pos.push(e.span);
// If we ever allow `concat!("", b"")`, we should probably add a warn by default
// lint to this code.
unified_accumulator.extend(s.to_string().into_bytes());
}
ast::LitKind::Char(c) => {
accumulator.push(c);
string_accumulator.push(c);
string_pos.push(e.span);
unified_accumulator.extend(c.to_string().into_bytes());
}
ast::LitKind::Int(i, ast::LitIntType::Unsigned(_))
| ast::LitKind::Int(i, ast::LitIntType::Signed(_))
| ast::LitKind::Int(i, ast::LitIntType::Unsuffixed) => {
accumulator.push_str(&i.to_string());
string_accumulator.push_str(&i.to_string());
int_pos.push(e.span);
// If we ever allow `concat!()` mixing byte literals with integers, we need to
// define the appropriate behavior for it. Consistently considering them as
// "machine width" would be bug-prone. Taking the smallest possible size for the
// literal is probably what people _that don't think about it_ would expect, but
// would be inconsistent. Another option is only to accept the literals if they
// would fit in a `u8`.
unified_accumulator.extend(i.to_bytes().iter());
}
ast::LitKind::Bool(b) => {
accumulator.push_str(&b.to_string());
string_accumulator.push_str(&b.to_string());
bool_pos.push(e.span);
// would `concat!(true, b"asdf")` ever make sense?
}
ast::LitKind::Byte(..) | ast::LitKind::ByteStr(..) => {
cx.span_err(e.span, "cannot concatenate a byte string literal");
ast::LitKind::Byte(byte) => {
b_accumulator.push(byte);
b_pos.push(e.span);
unified_accumulator.push(byte);
}
},
ast::LitKind::ByteStr(ref b_str) => {
b_accumulator.extend(b_str.iter());
b_pos.push(e.span);
unified_accumulator.extend(b_str.iter());
}
}
_ => {
// Consider the possibility of allowing `concat!(b"asdf", [1, 2, 3, 4])`, given
// that every single element of the array is a valid `u8`.
missing_literal.push(e.span);
}
}
Expand All @@ -62,5 +95,60 @@ pub fn expand_syntax_ext(
err.emit();
}
let sp = sp.apply_mark(cx.current_expansion.mark);
base::MacEager::expr(cx.expr_str(sp, Symbol::intern(&accumulator)))
// Do not allow mixing "" and b"", but return the joint b"" to avoid further errors
if b_pos.len() > 0 && (string_pos.len() > 0 || int_pos.len() > 0 || bool_pos.len() > 0) {
let mut mixings = vec![];
if string_pos.len() > 0 {
mixings.push("string");
}
if int_pos.len() > 0 {
mixings.push("numeric");
}
if bool_pos.len() > 0 {
mixings.push("boolean");
}
let mut err = cx.struct_span_err(
b_pos.clone(),
"cannot concatenate a byte string literal with other literals",
);
if mixings.len() > 0 && (int_pos.len() > 0 || bool_pos.len() > 0) {
let msg = if mixings.len() >= 2 {
format!(
"{} or {}",
mixings[0..mixings.len() - 1].join(", "),
mixings.last().unwrap(),
)
} else {
mixings[0].to_string()
};
err.note(&format!("we don't support mixing {} literals and byte strings", msg));
}
if string_pos.len() > 0 && int_pos.len() == 0 && bool_pos.len() == 0 {
err.multipart_suggestion(
"we don't support mixing string and byte string literals, use only byte strings",
string_pos
.iter()
.map(|pos| (pos.shrink_to_lo(), "b".to_string()))
.collect(),
);
}
for pos in &b_pos {
err.span_label(*pos, "byte string literal");
}
for pos in &string_pos {
err.span_label(*pos, "string literal");
}
for pos in &int_pos {
err.span_label(*pos, "numeric literal");
}
for pos in &bool_pos {
err.span_label(*pos, "boolean literal");
}
err.emit();
base::MacEager::expr(cx.expr_byte_str(sp, unified_accumulator))
} else if b_accumulator.len() > 0 {
base::MacEager::expr(cx.expr_byte_str(sp, b_accumulator))
} else {
base::MacEager::expr(cx.expr_str(sp, Symbol::intern(&string_accumulator)))
}
}
3 changes: 2 additions & 1 deletion src/libsyntax_ext/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
html_favicon_url = "https://doc.rust-lang.org/favicon.ico",
html_root_url = "https://doc.rust-lang.org/nightly/")]

#![feature(proc_macro_internals)]
#![feature(decl_macro)]
#![feature(int_to_from_bytes)]
#![feature(proc_macro_internals)]
#![feature(str_escape)]

#![feature(rustc_diagnostic_macros)]
Expand Down
16 changes: 0 additions & 16 deletions src/test/compile-fail/concat.rs

This file was deleted.

34 changes: 34 additions & 0 deletions src/test/ui/concat.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright 2013 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn main() {
concat!(b'f'); // b'' concat is supported
concat!(b"foo"); // b"" concat is supported
concat!(foo);
//~^ ERROR: expected a literal
concat!(foo());
//~^ ERROR: expected a literal
concat!(b'a', b"bc", b"def");
concat!("abc", b"def", 'g', "hi", b"jkl");
//~^ ERROR cannot concatenate a byte string literal with other literals
// `concat!()` cannot mix "" and b"" literals (it might allow it in the future)
concat!(1, b"def");
//~^ ERROR cannot concatenate a byte string literal with other literals
concat!(true, b"def");
//~^ ERROR cannot concatenate a byte string literal with other literals
concat!(1, true, b"def");
//~^ ERROR cannot concatenate a byte string literal with other literals
concat!(1, true, "abc", b"def");
//~^ ERROR cannot concatenate a byte string literal with other literals
concat!(true, "abc", b"def");
//~^ ERROR cannot concatenate a byte string literal with other literals
concat!(1, "abc", b"def");
//~^ ERROR cannot concatenate a byte string literal with other literals
}
98 changes: 98 additions & 0 deletions src/test/ui/concat.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
error: expected a literal
--> $DIR/concat.rs:14:13
|
LL | concat!(foo);
| ^^^
|
= note: only literals (like `"foo"`, `42` and `3.14`) can be passed to `concat!()`

error: expected a literal
--> $DIR/concat.rs:16:13
|
LL | concat!(foo());
| ^^^^^
|
= note: only literals (like `"foo"`, `42` and `3.14`) can be passed to `concat!()`

error: cannot concatenate a byte string literal with other literals
--> $DIR/concat.rs:19:20
|
LL | concat!("abc", b"def", 'g', "hi", b"jkl");
| ----- ^^^^^^ --- ---- ^^^^^^ byte string literal
| | | | |
| | | | string literal
| | | string literal
| | byte string literal
| string literal
help: we don't support mixing string and byte string literals, use only byte strings
|
LL | concat!(b"abc", b"def", b'g', b"hi", b"jkl");
| ^ ^ ^

error: cannot concatenate a byte string literal with other literals
--> $DIR/concat.rs:22:16
|
LL | concat!(1, b"def");
| - ^^^^^^ byte string literal
| |
| numeric literal
|
= note: we don't support mixing numeric literals and byte strings

error: cannot concatenate a byte string literal with other literals
--> $DIR/concat.rs:24:19
|
LL | concat!(true, b"def");
| ---- ^^^^^^ byte string literal
| |
| boolean literal
|
= note: we don't support mixing boolean literals and byte strings

error: cannot concatenate a byte string literal with other literals
--> $DIR/concat.rs:26:22
|
LL | concat!(1, true, b"def");
| - ---- ^^^^^^ byte string literal
| | |
| | boolean literal
| numeric literal
|
= note: we don't support mixing numeric or boolean literals and byte strings

error: cannot concatenate a byte string literal with other literals
--> $DIR/concat.rs:28:29
|
LL | concat!(1, true, "abc", b"def");
| - ---- ----- ^^^^^^ byte string literal
| | | |
| | | string literal
| | boolean literal
| numeric literal
|
= note: we don't support mixing string, numeric or boolean literals and byte strings

error: cannot concatenate a byte string literal with other literals
--> $DIR/concat.rs:30:26
|
LL | concat!(true, "abc", b"def");
| ---- ----- ^^^^^^ byte string literal
| | |
| | string literal
| boolean literal
|
= note: we don't support mixing string or boolean literals and byte strings

error: cannot concatenate a byte string literal with other literals
--> $DIR/concat.rs:32:23
|
LL | concat!(1, "abc", b"def");
| - ----- ^^^^^^ byte string literal
| | |
| | string literal
| numeric literal
|
= note: we don't support mixing string or numeric literals and byte strings

error: aborting due to 9 previous errors