Skip to content

Commit

Permalink
fix: erlang compiler bug with shadowing sized values in bit patterns
Browse files Browse the repository at this point in the history
  • Loading branch information
Eingin authored and lpil committed Sep 30, 2024
1 parent a501575 commit 5efe8d3
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 53 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@

### Bug Fixes

- Fixed a bug in the compiler where shadowing a sized value in a bit pattern
would cause invalid erlang code to be generated.
([Antonio Iaccarino](https://github.com/eingin))

- Fixed a bug where the formatter would not format strings with big grapheme
clusters properly.
([Giacomo Cavalieri](https://github.com/giacomocavalieri))
Expand Down
64 changes: 33 additions & 31 deletions compiler-core/src/erlang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -669,21 +669,21 @@ fn const_segment<'a>(
options: &'a [TypedConstantBitArraySegmentOption],
env: &mut Env<'a>,
) -> Document<'a> {
let mut value_is_a_string_literal = false;
let document = match value {
// Skip the normal <<value/utf8>> surrounds
Constant::String { value, .. } => {
value_is_a_string_literal = true;
value.to_doc().surround("\"", "\"")
}
let value_is_a_string_literal = matches!(value, Constant::String { .. });

// As normal
Constant::Int { .. } | Constant::Float { .. } | Constant::BitArray { .. } => {
const_inline(value, env)
}
let create_document = |env: &mut Env<'a>| {
match value {
// Skip the normal <<value/utf8>> surrounds
Constant::String { value, .. } => value.to_doc().surround("\"", "\""),

// As normal
Constant::Int { .. } | Constant::Float { .. } | Constant::BitArray { .. } => {
const_inline(value, env)
}

// Wrap anything else in parentheses
value => const_inline(value, env).surround("(", ")"),
// Wrap anything else in parentheses
value => const_inline(value, env).surround("(", ")"),
}
};

let size = |value: &'a TypedConstant, env: &mut Env<'a>| match value {
Expand All @@ -697,7 +697,7 @@ fn const_segment<'a>(
let unit = |value: &'a u8| Some(eco_format!("unit:{value}").to_doc());

bit_array_segment(
document,
create_document,
options,
size,
unit,
Expand All @@ -722,23 +722,22 @@ fn expr_segment<'a>(
options: &'a [BitArrayOption<TypedExpr>],
env: &mut Env<'a>,
) -> Document<'a> {
let mut value_is_a_string_literal = false;
let value_is_a_string_literal = matches!(value, TypedExpr::String { .. });

let document = match value {
// Skip the normal <<value/utf8>> surrounds and set the string literal flag
TypedExpr::String { value, .. } => {
value_is_a_string_literal = true;
string_inner(value).surround("\"", "\"")
}
let create_document = |env: &mut Env<'a>| {
match value {
// Skip the normal <<value/utf8>> surrounds and set the string literal flag
TypedExpr::String { value, .. } => string_inner(value).surround("\"", "\""),

// As normal
TypedExpr::Int { .. }
| TypedExpr::Float { .. }
| TypedExpr::Var { .. }
| TypedExpr::BitArray { .. } => expr(value, env),
// As normal
TypedExpr::Int { .. }
| TypedExpr::Float { .. }
| TypedExpr::Var { .. }
| TypedExpr::BitArray { .. } => expr(value, env),

// Wrap anything else in parentheses
value => expr(value, env).surround("(", ")"),
// Wrap anything else in parentheses
value => expr(value, env).surround("(", ")"),
}
};

let size = |expression: &'a TypedExpr, env: &mut Env<'a>| match expression {
Expand All @@ -764,7 +763,7 @@ fn expr_segment<'a>(
let unit = |value: &'a u8| Some(eco_format!("unit:{value}").to_doc());

bit_array_segment(
document,
create_document,
options,
size,
unit,
Expand All @@ -774,8 +773,8 @@ fn expr_segment<'a>(
)
}

fn bit_array_segment<'a, Value: 'a, SizeToDoc, UnitToDoc>(
mut document: Document<'a>,
fn bit_array_segment<'a, Value: 'a, CreateDoc, SizeToDoc, UnitToDoc>(
mut create_document: CreateDoc,
options: &'a [BitArrayOption<Value>],
mut size_to_doc: SizeToDoc,
mut unit_to_doc: UnitToDoc,
Expand All @@ -784,6 +783,7 @@ fn bit_array_segment<'a, Value: 'a, SizeToDoc, UnitToDoc>(
env: &mut Env<'a>,
) -> Document<'a>
where
CreateDoc: FnMut(&mut Env<'a>) -> Document<'a>,
SizeToDoc: FnMut(&'a Value, &mut Env<'a>) -> Option<Document<'a>>,
UnitToDoc: FnMut(&'a u8) -> Option<Document<'a>>,
{
Expand Down Expand Up @@ -826,6 +826,8 @@ where
}
}

let mut document = create_document(env);

document = document.append(size);
let others_is_empty = others.is_empty();

Expand Down
48 changes: 26 additions & 22 deletions compiler-core/src/erlang/pattern.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::cell::RefCell;

use ecow::eco_format;

use crate::analyse::Inferred;
Expand Down Expand Up @@ -194,39 +196,41 @@ fn pattern_segment<'a>(
env: &mut Env<'a>,
guards: &mut Vec<Document<'a>>,
) -> Document<'a> {
let mut pattern_is_a_string_literal = false;
let mut pattern_is_a_discard = false;
let document = match value {
// Skip the normal <<value/utf8>> surrounds
Pattern::String { value, .. } => {
pattern_is_a_string_literal = true;
value.to_doc().surround("\"", "\"")
}
let pattern_is_a_string_literal = matches!(value, Pattern::String { .. });
let pattern_is_a_discard = matches!(value, Pattern::Discard { .. });

// As normal
Pattern::Discard { .. } => {
pattern_is_a_discard = true;
print(value, vars, define_variables, env, guards)
}
Pattern::Variable { .. } | Pattern::Int { .. } | Pattern::Float { .. } => {
print(value, vars, define_variables, env, guards)
}
let vars = RefCell::new(vars);
let guards = RefCell::new(guards);

// No other pattern variants are allowed in pattern bit array segments
let create_document = |env: &mut Env<'a>| match value {
Pattern::String { value, .. } => value.to_doc().surround("\"", "\""),
Pattern::Discard { .. }
| Pattern::Variable { .. }
| Pattern::Int { .. }
| Pattern::Float { .. } => print(
value,
&mut vars.borrow_mut(),
define_variables,
env,
&mut guards.borrow_mut(),
),
_ => panic!("Pattern segment match not recognised"),
};

let size = |value: &'a TypedPattern, env: &mut Env<'a>| {
Some(
":".to_doc()
.append(print(value, vars, define_variables, env, guards)),
)
Some(":".to_doc().append(print(
value,
&mut vars.borrow_mut(),
define_variables,
env,
&mut guards.borrow_mut(),
)))
};

let unit = |value: &'a u8| Some(eco_format!("unit:{value}").to_doc());

bit_array_segment(
document,
create_document,
options,
size,
unit,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
source: compiler-core/src/erlang/tests.rs
expression: "\npub fn main() {\n let code = <<\"hello world\":utf8>>\n let pre = 1\n case code {\n <<pre:bytes-size(pre), _:bytes>> -> pre\n _ -> panic\n }\n} "
---
-module(my@mod).
-compile([no_auto_import, nowarn_unused_vars, nowarn_unused_function, nowarn_nomatch]).

-export([main/0]).

-file("/root/project/test/my/mod.gleam", 2).
-spec main() -> bitstring().
main() ->
Code = <<"hello world"/utf8>>,
Pre = 1,
case Code of
<<Pre@1:Pre/binary, _/binary>> ->
Pre@1;

_ ->
erlang:error(#{gleam_error => panic,
message => <<"`panic` expression evaluated."/utf8>>,
module => <<"my/mod"/utf8>>,
function => <<"main"/utf8>>,
line => 7})
end.
16 changes: 16 additions & 0 deletions compiler-core/src/erlang/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -943,3 +943,19 @@ fn windows_file_escaping_bug() {
let output = compile_test_project(src, path, None);
insta::assert_snapshot!(insta::internals::AutoName, output, src);
}

// https://github.com/gleam-lang/gleam/issues/3315
#[test]
fn bit_pattern_shadowing() {
assert_erl!(
"
pub fn main() {
let code = <<\"hello world\":utf8>>
let pre = 1
case code {
<<pre:bytes-size(pre), _:bytes>> -> pre
_ -> panic
}
} "
);
}

0 comments on commit 5efe8d3

Please sign in to comment.