Skip to content

Commit

Permalink
Merge pull request #7468 from joshuawarner32/fuzzing-bugs-9
Browse files Browse the repository at this point in the history 
And... more fuzzing bugs!
  • Loading branch information
@lukewilliamboswell
lukewilliamboswell authored Jan 6, 2025
2 parents 01da957 + 9fcefb3 commit 89ef225
Show file tree
Hide file tree
Showing 24 changed files with 498 additions and 278 deletions.
25 changes: 8 additions & 17 deletions crates/compiler/can/src/def.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ enum PendingTypeDef<'a> {
name: Loc<Symbol>,
vars: Vec<Loc<Lowercase>>,
ann: &'a Loc<ast::TypeAnnotation<'a>>,
derived: Option<&'a Loc<ast::ImplementsAbilities<'a>>>,
derived: Option<&'a ast::ImplementsAbilities<'a>>,
},

Ability {
Expand Down Expand Up @@ -319,7 +319,7 @@ impl PendingTypeDef<'_> {
ann,
derived,
} => {
let end = derived.map(|d| d.region).unwrap_or(ann.region);
let end = derived.map(|d| d.item.region).unwrap_or(ann.region);
let region = Region::span_across(&name.region, &end);

Some((name.value, region))
Expand Down Expand Up @@ -761,12 +761,11 @@ fn canonicalize_opaque<'a>(
var_store: &mut VarStore,
scope: &mut Scope,
pending_abilities_in_scope: &PendingAbilitiesInScope,

name: Loc<Symbol>,
name_str: &'a str,
ann: &'a Loc<ast::TypeAnnotation<'a>>,
vars: &[Loc<Lowercase>],
has_abilities: Option<&'a Loc<ast::ImplementsAbilities<'a>>>,
has_abilities: Option<&'a ast::ImplementsAbilities<'a>>,
) -> Result<CanonicalizedOpaque<'a>, ()> {
let alias = canonicalize_alias(
env,
Expand All @@ -784,11 +783,11 @@ fn canonicalize_opaque<'a>(

let mut derived_defs = Vec::new();
if let Some(has_abilities) = has_abilities {
let has_abilities = has_abilities.value.collection();
let has_abilities = has_abilities.item;

let mut derived_abilities = vec![];

for has_ability in has_abilities.items {
for has_ability in has_abilities.value.items {
let region = has_ability.region;
let (ability, opt_impls) = match has_ability.value.extract_spaces().item {
ast::ImplementsAbility::ImplementsAbility { ability, impls } => (ability, impls),
Expand Down Expand Up @@ -1303,7 +1302,7 @@ fn canonicalize_type_defs<'a>(
Loc<Symbol>,
Vec<Loc<Lowercase>>,
&'a Loc<ast::TypeAnnotation<'a>>,
Option<&'a Loc<ast::ImplementsAbilities<'a>>>,
Option<&'a ast::ImplementsAbilities<'a>>,
),
Ability(Loc<Symbol>, Vec<PendingAbilityMember<'a>>),
}
Expand Down Expand Up @@ -2805,7 +2804,7 @@ fn to_pending_alias_or_opaque<'a>(
name: &'a Loc<&'a str>,
vars: &'a [Loc<ast::Pattern<'a>>],
ann: &'a Loc<ast::TypeAnnotation<'a>>,
opt_derived: Option<&'a Loc<ast::ImplementsAbilities<'a>>>,
opt_derived: Option<&'a ast::ImplementsAbilities<'a>>,
kind: AliasKind,
) -> PendingTypeDef<'a> {
let region = Region::span_across(&name.region, &ann.region);
Expand Down Expand Up @@ -2898,15 +2897,7 @@ fn to_pending_type_def<'a>(
header: TypeHeader { name, vars },
typ: ann,
derived,
} => to_pending_alias_or_opaque(
env,
scope,
name,
vars,
ann,
derived.as_ref(),
AliasKind::Opaque,
),
} => to_pending_alias_or_opaque(env, scope, name, vars, ann, *derived, AliasKind::Opaque),

Ability {
header, members, ..
Expand Down
57 changes: 29 additions & 28 deletions crates/compiler/fmt/src/annotation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -703,36 +703,37 @@ impl<'a> Formattable for ImplementsAbility<'a> {

impl<'a> Formattable for ImplementsAbilities<'a> {
fn is_multiline(&self) -> bool {
match self {
ImplementsAbilities::SpaceAfter(..) | ImplementsAbilities::SpaceBefore(..) => true,
ImplementsAbilities::Implements(has_abilities) => {
is_collection_multiline(has_abilities)
}
}
self.before_implements_kw.iter().any(|s| s.is_comment())
|| self.after_implements_kw.iter().any(|s| s.is_comment())
|| is_collection_multiline(&self.item.value)
}

fn format_with_options(&self, buf: &mut Buf, parens: Parens, newlines: Newlines, indent: u16) {
match self {
ImplementsAbilities::Implements(has_abilities) => {
if newlines == Newlines::Yes {
buf.newline();
}
buf.indent(indent);
buf.push_str(roc_parse::keyword::IMPLEMENTS);
buf.spaces(1);
fmt_collection(buf, indent, Braces::Square, *has_abilities, Newlines::No);
}
ImplementsAbilities::SpaceBefore(has_abilities, spaces) => {
buf.newline();
buf.indent(indent);
fmt_comments_only(buf, spaces.iter(), NewlineAt::Bottom, indent);
has_abilities.format_with_options(buf, parens, Newlines::No, indent)
}
ImplementsAbilities::SpaceAfter(has_abilities, spaces) => {
has_abilities.format_with_options(buf, parens, newlines, indent);
fmt_comments_only(buf, spaces.iter(), NewlineAt::Bottom, indent);
}
fn format_with_options(&self, buf: &mut Buf, _parens: Parens, newlines: Newlines, indent: u16) {
if !self.before_implements_kw.is_empty() {
buf.newline();
buf.indent(indent);
fmt_comments_only(
buf,
self.before_implements_kw.iter(),
NewlineAt::Bottom,
indent,
);
}
if newlines == Newlines::Yes {
buf.ensure_ends_with_newline();
}
buf.indent(indent);
buf.push_str(roc_parse::keyword::IMPLEMENTS);
if !self.after_implements_kw.is_empty() {
fmt_comments_only(
buf,
self.after_implements_kw.iter(),
NewlineAt::Bottom,
indent,
);
}
buf.ensure_ends_with_whitespace();
fmt_collection(buf, indent, Braces::Square, self.item.value, Newlines::No);
}
}

Expand Down Expand Up @@ -1217,7 +1218,7 @@ impl<'a> Nodify<'a> for TypeAnnotation<'a> {
},
after: last_after,
needs_indent,
prec: Prec::AsType,
prec: Prec::FunctionType,
}
}
}
Expand Down
36 changes: 8 additions & 28 deletions crates/compiler/fmt/src/def.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ use roc_error_macros::internal_error;
use roc_parse::ast::{
AbilityMember, Defs, Expr, ExtractSpaces, ImportAlias, ImportAsKeyword, ImportExposingKeyword,
ImportedModuleName, IngestedFileAnnotation, IngestedFileImport, ModuleImport,
ModuleImportParams, Pattern, PatternApplyStyle, Spaceable, Spaces, SpacesAfter, SpacesBefore,
StrLiteral, TypeAnnotation, TypeDef, TypeHeader, ValueDef,
ModuleImportParams, Pattern, PatternApplyStyle, Spaces, SpacesBefore, StrLiteral,
TypeAnnotation, TypeDef, TypeHeader, ValueDef,
};
use roc_parse::expr::merge_spaces;
use roc_parse::header::Keyword;
Expand Down Expand Up @@ -94,21 +94,6 @@ pub fn def_lift_spaces<'a, 'b: 'a>(
}
}

fn lift_spaces_after<'a, 'b: 'a, T: 'b + ExtractSpaces<'a> + Spaceable<'a>>(
arena: &'a Bump,
item: T,
) -> SpacesAfter<'a, <T as ExtractSpaces<'a>>::Item>
where
<T as ExtractSpaces<'a>>::Item: Spaceable<'a>,
{
let spaces = item.extract_spaces();

SpacesAfter {
item: spaces.item.maybe_before(arena, spaces.before),
after: spaces.after,
}
}

pub fn tydef_lift_spaces<'a, 'b: 'a>(arena: &'a Bump, def: TypeDef<'b>) -> Spaces<'a, TypeDef<'a>> {
match def {
TypeDef::Alias { header, ann } => {
Expand All @@ -128,17 +113,12 @@ pub fn tydef_lift_spaces<'a, 'b: 'a>(arena: &'a Bump, def: TypeDef<'b>) -> Space
typ,
derived,
} => {
if let Some(derived) = derived {
let derived_lifted = lift_spaces_after(arena, derived.value);

if derived.is_some() {
// It's structurally impossible for a derived clause to have spaces after
Spaces {
before: &[],
item: TypeDef::Opaque {
header,
typ,
derived: Some(Loc::at(derived.region, derived_lifted.item)),
},
after: derived_lifted.after,
item: def,
after: &[],
}
} else {
let typ_lifted = ann_lift_spaces_after(arena, &typ.value);
Expand Down Expand Up @@ -461,7 +441,7 @@ impl<'a> Formattable for TypeDef<'a> {
// Always put the has-abilities clause on a newline if the opaque annotation
// contains a where-has clause.
let has_abilities_multiline = if let Some(has_abilities) = has_abilities {
!has_abilities.value.is_empty() && ann_is_where_clause
!has_abilities.item.value.is_empty() && ann_is_where_clause
} else {
false
};
Expand All @@ -481,7 +461,7 @@ impl<'a> Formattable for TypeDef<'a> {
if let Some(has_abilities) = has_abilities {
buf.spaces(1);

has_abilities.format_with_options(
(*has_abilities).format_with_options(
buf,
Parens::NotNeeded,
Newlines::from_bool(make_multiline),
Expand Down
23 changes: 14 additions & 9 deletions crates/compiler/fmt/src/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1814,14 +1814,17 @@ fn fmt_return<'a>(
buf.indent(indent);
buf.push_str(keyword::RETURN);

let return_indent = if return_value.is_multiline() {
let value = expr_lift_spaces(parens, buf.text.bump(), &return_value.value);

let return_indent = if value.item.is_multiline()
|| (newlines == Newlines::Yes && !value.before.is_empty())
|| value.before.iter().any(|s| s.is_comment())
{
indent + INDENT
} else {
indent
};

let value = expr_lift_spaces(parens, buf.text.bump(), &return_value.value);

if !value.before.is_empty() {
format_spaces(buf, value.before, newlines, return_indent);
}
Expand Down Expand Up @@ -2057,15 +2060,17 @@ fn fmt_record_like<'a, 'b: 'a, Field, ToSpacesAround>(
// doesnt make sense.
Some(RecordPrefix::Update(record_var)) => {
buf.spaces(1);
record_var.format(buf, indent);
buf.indent(indent);
buf.push_str(" &");
record_var.format(buf, indent + INDENT);
buf.indent(indent + INDENT);
buf.ensure_ends_with_whitespace();
buf.push_str("&");
}
Some(RecordPrefix::Mapper(mapper_var)) => {
buf.spaces(1);
mapper_var.format(buf, indent);
buf.indent(indent);
buf.push_str(" <-");
mapper_var.format(buf, indent + INDENT);
buf.indent(indent + INDENT);
buf.ensure_ends_with_whitespace();
buf.push_str("<-");
}
}

Expand Down
61 changes: 11 additions & 50 deletions crates/compiler/parse/src/ast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ pub enum TypeDef<'a> {
Opaque {
header: TypeHeader<'a>,
typ: Loc<TypeAnnotation<'a>>,
derived: Option<Loc<ImplementsAbilities<'a>>>,
derived: Option<&'a ImplementsAbilities<'a>>,
},

/// An ability definition. E.g.
Expand Down Expand Up @@ -1552,31 +1552,11 @@ pub enum ImplementsAbility<'a> {
}

#[derive(Debug, Copy, Clone, PartialEq)]
pub enum ImplementsAbilities<'a> {
/// `implements [Eq { eq: myEq }, Hash]`
Implements(Collection<'a, Loc<ImplementsAbility<'a>>>),

// We preserve this for the formatter; canonicalization ignores it.
SpaceBefore(&'a ImplementsAbilities<'a>, &'a [CommentOrNewline<'a>]),
SpaceAfter(&'a ImplementsAbilities<'a>, &'a [CommentOrNewline<'a>]),
}

impl ImplementsAbilities<'_> {
pub fn collection(&self) -> &Collection<Loc<ImplementsAbility>> {
let mut it = self;
loop {
match it {
Self::SpaceBefore(inner, _) | Self::SpaceAfter(inner, _) => {
it = inner;
}
Self::Implements(collection) => return collection,
}
}
}

pub fn is_empty(&self) -> bool {
self.collection().is_empty()
}
pub struct ImplementsAbilities<'a> {
pub before_implements_kw: &'a [CommentOrNewline<'a>],
pub implements: Region,
pub after_implements_kw: &'a [CommentOrNewline<'a>],
pub item: Loc<Collection<'a, Loc<ImplementsAbility<'a>>>>,
}

#[derive(Debug, Copy, Clone, PartialEq)]
Expand Down Expand Up @@ -2278,15 +2258,6 @@ impl<'a> Spaceable<'a> for ImplementsAbility<'a> {
}
}

impl<'a> Spaceable<'a> for ImplementsAbilities<'a> {
fn before(&'a self, spaces: &'a [CommentOrNewline<'a>]) -> Self {
ImplementsAbilities::SpaceBefore(self, spaces)
}
fn after(&'a self, spaces: &'a [CommentOrNewline<'a>]) -> Self {
ImplementsAbilities::SpaceAfter(self, spaces)
}
}

impl<'a> Expr<'a> {
pub const REPL_OPAQUE_FUNCTION: Self = Expr::Var {
module_name: "",
Expand Down Expand Up @@ -2396,7 +2367,6 @@ impl_extract_spaces!(Tag);
impl_extract_spaces!(AssignedField<T>);
impl_extract_spaces!(TypeAnnotation);
impl_extract_spaces!(ImplementsAbility);
impl_extract_spaces!(ImplementsAbilities);
impl_extract_spaces!(Implements);

impl<'a, T: Copy> ExtractSpaces<'a> for Spaced<'a, T> {
Expand Down Expand Up @@ -2720,7 +2690,11 @@ impl<'a> Malformed for TypeDef<'a> {
header,
typ,
derived,
} => header.is_malformed() || typ.is_malformed() || derived.is_malformed(),
} => {
header.is_malformed()
|| typ.is_malformed()
|| derived.map(|d| d.item.is_malformed()).unwrap_or_default()
}
TypeDef::Ability {
header,
loc_implements,
Expand Down Expand Up @@ -2762,19 +2736,6 @@ impl<'a> Malformed for ImplementsAbility<'a> {
}
}

impl<'a> Malformed for ImplementsAbilities<'a> {
fn is_malformed(&self) -> bool {
match self {
ImplementsAbilities::Implements(abilities) => {
abilities.iter().any(|ability| ability.is_malformed())
}
ImplementsAbilities::SpaceBefore(has, _) | ImplementsAbilities::SpaceAfter(has, _) => {
has.is_malformed()
}
}
}
}

impl<'a> Malformed for AbilityImpls<'a> {
fn is_malformed(&self) -> bool {
match self {
Expand Down
Loading

0 comments on commit 89ef225

Please sign in to comment.