Skip to content

Commit

Permalink
Dereferencing operator index [] (#5530)
Browse files Browse the repository at this point in the history
## Description

This PR implements index operator `[]` for
[references](https://github.com/FuelLabs/sway-rfcs/blob/ironcev/amend-references/files/0010-references.sw).
The overall effort related to references is tracked in #5063.

`[]` is defined for references using this recursive definition:
`<reference>[<index>] := (*<reference>)[<index>]`.

This eliminates the need for the dereferencing operator `*` when working
with references to arrays:

```Sway
let array = [1, 2, 3];

let r = &&&array;

assert((***r)[0] == r[0]);
```
```Sway
let r = &&&[ &&[1, 2, 3], &&[4, 5, 6] ];
assert(r[0][2] == 3);
assert(r[1][0] == 4);
```

Additionally, the PR fixes two previously existing issues (see below
screenshots):
- the misleading error message on type not implementing the "index"
method when indexing a non-indexable type.
- the broken error span and expression text when indexing a
non-indexable type in reassignments.

Before:
![No method named index found for
type](https://github.com/FuelLabs/sway/assets/4142833/4693510d-bcb4-45ca-8f41-7988a8d87b3e)

![Not an indexable expression -
Before](https://github.com/FuelLabs/sway/assets/4142833/6b8f28f0-25db-4ba8-b6a5-3d5468c70cdc)

After:
![Type is not
indexable](https://github.com/FuelLabs/sway/assets/4142833/df160b3b-312e-40dc-b32f-71786ee382ea)

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
  • Loading branch information
ironcev authored Jan 31, 2024
1 parent ed82a34 commit 6f9aab8
Show file tree
Hide file tree
Showing 21 changed files with 778 additions and 96 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1799,65 +1799,70 @@ impl ty::TyExpression {
let type_engine = ctx.engines.te();
let engines = ctx.engines();

let prefix_te = {
let mut current_prefix_te = Box::new({
let ctx = ctx
.by_ref()
.with_help_text("")
.with_type_annotation(type_engine.insert(engines, TypeInfo::Unknown, None));
ty::TyExpression::type_check(handler, ctx, prefix.clone())?
};

fn get_array_type(ty: TypeId, type_engine: &TypeEngine) -> Option<TypeInfo> {
match &*type_engine.get(ty) {
TypeInfo::Array(..) => Some((*type_engine.get(ty)).clone()),
TypeInfo::Alias { ty, .. } => get_array_type(ty.type_id, type_engine),
_ => None,
}
ty::TyExpression::type_check(handler, ctx, prefix)?
});

let mut current_type = type_engine.get_unaliased(current_prefix_te.return_type);

let prefix_type_id = current_prefix_te.return_type;
let prefix_span = current_prefix_te.span.clone();

// Create the prefix part of the final array index expression.
// This might be an expression that directly evaluates to an array type,
// or an arbitrary number of dereferencing expressions where the last one
// dereference to an array type.
//
// We will either hit an array at the end or return an error, so the
// loop cannot be endless.
while !current_type.is_array() {
match &*current_type {
TypeInfo::Ref(referenced_type) => {
let referenced_type_id = referenced_type.type_id;

current_prefix_te = Box::new(ty::TyExpression {
expression: ty::TyExpressionVariant::Deref(current_prefix_te),
return_type: referenced_type_id,
span: prefix_span.clone(),
});

current_type = type_engine.get_unaliased(referenced_type_id);
}
_ => {
return Err(handler.emit_err(CompileError::NotIndexable {
actually: engines.help_out(prefix_type_id).to_string(),
span: prefix_span.clone(),
}))
}
};
}

// If the return type is a static array then create a `ty::TyExpressionVariant::ArrayIndex`.
if let Some(TypeInfo::Array(elem_type, _)) =
get_array_type(prefix_te.return_type, type_engine)
{
let TypeInfo::Array(array_type_argument, _) = &*current_type else {
panic!("The current type must be an array.");
};

let index_te = {
let type_info_u64 = TypeInfo::UnsignedInteger(IntegerBits::SixtyFour);
let ctx = ctx
.with_help_text("")
.with_type_annotation(type_engine.insert(engines, type_info_u64, None));
let index_te = ty::TyExpression::type_check(handler, ctx, index)?;

Ok(ty::TyExpression {
expression: ty::TyExpressionVariant::ArrayIndex {
prefix: Box::new(prefix_te),
index: Box::new(index_te),
},
return_type: elem_type.type_id,
span,
})
} else {
// Otherwise convert into a method call 'index(self, index)' via the std::ops::Index trait.
let method_name = TypeBinding {
inner: MethodName::FromTrait {
call_path: CallPath {
prefixes: vec![
Ident::new_with_override("core".into(), span.clone()),
Ident::new_with_override("ops".into(), span.clone()),
],
suffix: Ident::new_with_override("index".into(), span.clone()),
is_absolute: true,
},
},
type_arguments: TypeArgs::Regular(vec![]),
span: span.clone(),
};
type_check_method_application(
handler,
ctx,
method_name,
vec![],
vec![prefix, index],
span,
)
}
ty::TyExpression::type_check(handler, ctx, index)?
};

Ok(ty::TyExpression {
expression: ty::TyExpressionVariant::ArrayIndex {
prefix: current_prefix_te,
index: Box::new(index_te),
},
return_type: array_type_argument.type_id,
span,
})
}

fn type_check_intrinsic_function(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub(crate) fn instantiate_tuple_index_access(
let type_engine = engines.te();
let mut tuple_type_arg_to_access = None;
let type_info = type_engine.get(parent.return_type);
let type_args = type_info.expect_tuple(handler, engines, parent.span.as_str(), &parent.span)?;
let type_args = type_info.expect_tuple(handler, engines, &parent.span)?;
for (pos, type_arg) in type_args.iter().enumerate() {
if pos == index {
tuple_type_arg_to_access = Some(type_arg.clone());
Expand Down
18 changes: 10 additions & 8 deletions sway-core/src/semantic_analysis/namespace/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,6 @@ impl Items {
let mut symbol = symbol.return_type(handler, engines)?;
let mut symbol_span = base_name.span();
let mut parent_rover = symbol;
let mut full_name_for_error = base_name.to_string();
let mut full_span_for_error = base_name.span();
for projection in projections {
let resolved_type = match type_engine.to_typeinfo(symbol, &symbol_span) {
Expand Down Expand Up @@ -412,7 +411,6 @@ impl Items {
parent_rover = symbol;
symbol = field_type_id;
symbol_span = field_name.span().clone();
full_name_for_error.push_str(field_name.as_str());
full_span_for_error =
Span::join(full_span_for_error, field_name.span().clone());
}
Expand All @@ -435,7 +433,6 @@ impl Items {
parent_rover = symbol;
symbol = *field_type;
symbol_span = index_span.clone();
full_name_for_error.push_str(&index.to_string());
full_span_for_error = Span::join(full_span_for_error, index_span.clone());
}
(
Expand All @@ -445,7 +442,14 @@ impl Items {
parent_rover = symbol;
symbol = elem_ty.type_id;
symbol_span = index_span.clone();
full_span_for_error = index_span.clone();
// `index_span` does not contain the enclosing square brackets.
// Which means, if this array index access is the last one before the
// erroneous expression, the `full_span_for_error` will be missing the
// closing `]`. We can live with this small glitch so far. To fix it,
// we would need to bring the full span of the index all the way from
// the parsing stage. An effort that doesn't pay off at the moment.
// TODO: Include the closing square bracket into the error span.
full_span_for_error = Span::join(full_span_for_error, index_span.clone());
}
(actually, ty::ProjectionKind::StructField { .. }) => {
return Err(handler.emit_err(CompileError::FieldAccessOnNonStruct {
Expand All @@ -455,16 +459,14 @@ impl Items {
}
(actually, ty::ProjectionKind::TupleField { .. }) => {
return Err(handler.emit_err(CompileError::NotATuple {
name: full_name_for_error,
span: full_span_for_error,
actually: engines.help_out(actually).to_string(),
span: full_span_for_error,
}));
}
(actually, ty::ProjectionKind::ArrayIndex { .. }) => {
return Err(handler.emit_err(CompileError::NotIndexable {
name: full_name_for_error,
span: full_span_for_error,
actually: engines.help_out(actually).to_string(),
span: full_span_for_error,
}));
}
}
Expand Down
18 changes: 9 additions & 9 deletions sway-core/src/type_system/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1092,6 +1092,10 @@ impl TypeInfo {
matches!(self, TypeInfo::Ref(_))
}

pub fn is_array(&self) -> bool {
matches!(self, TypeInfo::Array(_, _))
}

pub(crate) fn apply_type_arguments(
self,
handler: &Handler,
Expand Down Expand Up @@ -1409,25 +1413,21 @@ impl TypeInfo {
&self,
handler: &Handler,
engines: &Engines,
debug_string: impl Into<String>,
debug_span: &Span,
) -> Result<Vec<TypeArgument>, ErrorEmitted> {
match self {
TypeInfo::Tuple(elems) => Ok(elems.to_vec()),
TypeInfo::Alias {
ty: TypeArgument { type_id, .. },
..
} => {
engines
.te()
.get(*type_id)
.expect_tuple(handler, engines, debug_string, debug_span)
}
} => engines
.te()
.get(*type_id)
.expect_tuple(handler, engines, debug_span),
TypeInfo::ErrorRecovery(err) => Err(*err),
a => Err(handler.emit_err(CompileError::NotATuple {
name: debug_string.into(),
span: debug_span.clone(),
actually: engines.help_out(a).to_string(),
span: debug_span.clone(),
})),
}
}
Expand Down
31 changes: 19 additions & 12 deletions sway-error/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,18 +287,10 @@ pub enum CompileError {
ModuleNotFound { span: Span, name: String },
#[error("This is a {actually}, not a struct. Fields can only be accessed on structs.")]
FieldAccessOnNonStruct { actually: String, span: Span },
#[error("\"{name}\" is a {actually}, not a tuple. Elements can only be access on tuples.")]
NotATuple {
name: String,
span: Span,
actually: String,
},
#[error("\"{name}\" is a {actually}, which is not an indexable expression.")]
NotIndexable {
name: String,
span: Span,
actually: String,
},
#[error("This is a {actually}, not a tuple. Elements can only be access on tuples.")]
NotATuple { actually: String, span: Span },
#[error("This expression has type \"{actually}\", which is not an indexable type.")]
NotIndexable { actually: String, span: Span },
#[error("\"{name}\" is a {actually}, not an enum.")]
NotAnEnum {
name: String,
Expand Down Expand Up @@ -1713,6 +1705,21 @@ impl ToDiagnostic for CompileError {
},
help: vec![],
},
NotIndexable { actually, span } => Diagnostic {
reason: Some(Reason::new(code(1), "Type is not indexable".to_string())),
issue: Issue::error(
source_engine,
span.clone(),
format!("This expression has type \"{actually}\", which is not an indexable type.")
),
hints: vec![],
help: vec![
"Index operator `[]` can be used only on indexable types.".to_string(),
"In Sway, indexable types are:".to_string(),
format!("{}- arrays. E.g., `[u64;3]`.", Indent::Single),
format!("{}- references, direct or indirect, to arrays. E.g., `&[u64;3]` or `&&&[u64;3]`.", Indent::Single),
],
},
_ => Diagnostic {
// TODO: Temporary we use self here to achieve backward compatibility.
// In general, self must not be used and will not be used once we
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
category = "fail"

# check: $()Type is not indexable
# check: $()}[0];
# nextln: $()No method named "index" found for type "Never".
# nextln: $()This expression has type "Never", which is not an indexable type.
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
category = "fail"

# check: $()"b" is a bool, which is not an indexable expression.
# check: $()Type is not indexable
# check: $()b[0] = true;
# nextln: $()This expression has type "bool", which is not an indexable type.

# check: $()Assignment to immutable variable. Variable my_array is not declared as mutable.

# check: $()Cannot pass immutable argument to mutable parameter.

# check: $()Mismatched types.

# check: $()"my_array_2" is a u64, which is not an indexable expression.
# check: $()Type is not indexable
# check: $()my_array_2[0][1] = false;
# nextln: $()This expression has type "u64", which is not an indexable type.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[[package]]
name = "non_indexable_types"
source = "member"
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[project]
authors = ["Fuel Labs <[email protected]>"]
license = "Apache-2.0"
name = "non_indexable_types"
entry = "main.sw"
implicit-std = false
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
script;

struct S {
x: u8,
u8_field: u8,
}

fn main() {
let mut not_array = 0;
let _ = not_array[0];
not_array[0] = 1;

let mut s = S { x: 0, u8_field: 0 };
let _ = s[0];
s[0] = 1;

let _ = s.x[0];
s.x[0] = 1;

let mut array = [s, s];
let _ = array[0].x[0];
array[0].x[0] = 1;

let _= array[0].u8_field[0];
array[0].u8_field[0] = 1;

let _ = array[0][0];
array[0][0] = 1;

let mut tuple = (1, 2);
let _ = tuple[0];
tuple[0] = 1;

let _ = tuple.1[0];
tuple.1[0] = 1;
}
Loading

0 comments on commit 6f9aab8

Please sign in to comment.