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

Add positions to events (issue #37) #69

Merged
merged 15 commits into from
Nov 13, 2023
Merged
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: 3 additions & 3 deletions checker/specification/specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ function func(): string {
}
```

- Function is expected to return string but returned 2
- Cannot return 2 because the function is expected to return string

#### *Inferred* return type

Expand Down Expand Up @@ -201,7 +201,7 @@ function getSecond2<T, U>(p1: T, p2: U): U {
}
```

- Function is expected to return U but returned T
- Cannot return T because the function is expected to return U

#### Use of generics in function body

Expand Down Expand Up @@ -229,7 +229,7 @@ function createObject2<T, U>(a: T, b: U): { a: U, b: U } {
}
```

- Function is expected to return {"a": U, "b": U, } but returned {"a": T, "b": U, }
- Cannot return {"a": T, "b": U, } because the function is expected to return {"a": U, "b": U, }

### Function calling

Expand Down
18 changes: 16 additions & 2 deletions checker/src/behavior/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,13 @@ impl<M: crate::ASTImplementation> FunctionRegisterBehavior<M> for RegisterAsType
let id = func_ty.id;
types.functions.insert(id, func_ty);
let ty = types.register_type(crate::Type::Function(id, ThisValue::UseParent));

// TODO: Needs a way to get position (perhaps from a method in `SynthesisableFunction`).
// Can `return_type_annotation` be used to get the position?
environment.facts.events.push(Event::CreateObject {
prototype: crate::events::PrototypeArgument::Function(id),
referenced_in_scope_as: ty,
position: None,
});
ty
}
Expand All @@ -83,9 +87,11 @@ impl<M: crate::ASTImplementation> FunctionRegisterBehavior<M> for RegisterOnExis
types.functions.insert(id, func_ty);
let ty = types.register_type(crate::Type::Function(id, Default::default()));
context.facts.variable_current_value.insert(self.0, ty);
// TODO Needs a position
context.facts.events.push(Event::CreateObject {
prototype: crate::events::PrototypeArgument::Function(id),
referenced_in_scope_as: ty,
position: None,
});
}
}
Expand All @@ -111,9 +117,12 @@ impl<M: crate::ASTImplementation> FunctionRegisterBehavior<M> for RegisterOnExis
let id = func_ty.id;
types.functions.insert(id, func_ty);
let ty = types.register_type(Type::Function(id, Default::default()));

// TODO Needs a position
environment.facts.events.push(Event::CreateObject {
prototype: crate::events::PrototypeArgument::Function(id),
referenced_in_scope_as: ty,
position: None,
});
Property::Value(ty)
}
Expand Down Expand Up @@ -199,10 +208,15 @@ pub enum ThisValue {
}

impl ThisValue {
pub(crate) fn get(&self, environment: &mut Environment, types: &mut TypeStore) -> TypeId {
pub(crate) fn get(
&self,
environment: &mut Environment,
types: &mut TypeStore,
position: SpanWithSource,
) -> TypeId {
match self {
ThisValue::Passed(value) => *value,
ThisValue::UseParent => environment.get_value_of_this(types),
ThisValue::UseParent => environment.get_value_of_this(types, position),
}
}

Expand Down
5 changes: 4 additions & 1 deletion checker/src/behavior/objects.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use source_map::SpanWithSource;

use crate::{
context::{
facts::{Facts, PublicityKind},
Expand Down Expand Up @@ -25,8 +27,9 @@ impl ObjectBuilder {
under: TypeId,
value: Property,
property: PublicityKind,
position: Option<SpanWithSource>,
) {
environment.facts.register_property(self.object, under, value, true, property)
environment.facts.register_property(self.object, under, value, true, property, position)
}

pub fn build_object(self) -> TypeId {
Expand Down
42 changes: 34 additions & 8 deletions checker/src/context/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,14 @@ impl<'a> Environment<'a> {
checking_data,
)),
Reference::Property { on, with, publicity, span } => Ok(env
.set_property(on, with, publicity, new, &mut checking_data.types)?
.set_property(
on,
with,
publicity,
new,
&mut checking_data.types,
Some(span),
)?
.unwrap_or(new)),
}
}
Expand Down Expand Up @@ -419,7 +426,11 @@ impl<'a> Environment<'a> {

let variable_id = variable.get_id();

self.facts.events.push(Event::SetsVariable(variable_id, new_type));
self.facts.events.push(Event::SetsVariable(
variable_id,
new_type,
assignment_position,
));
self.facts.variable_current_value.insert(variable_id, new_type);

Ok(new_type)
Expand Down Expand Up @@ -484,6 +495,7 @@ impl<'a> Environment<'a> {
kind: PublicityKind,
types: &mut TypeStore,
with: Option<TypeId>,
position: SpanWithSource,
) -> Option<(PropertyKind, TypeId)> {
crate::types::properties::get_property(
on,
Expand All @@ -493,6 +505,7 @@ impl<'a> Environment<'a> {
self,
&mut CheckThings,
types,
position,
)
}

Expand All @@ -504,7 +517,7 @@ impl<'a> Environment<'a> {
checking_data: &mut CheckingData<U, M>,
site: SpanWithSource,
) -> TypeId {
match self.get_property(on, property, kind, &mut checking_data.types, None) {
match self.get_property(on, property, kind, &mut checking_data.types, None, site.clone()) {
Some((_, ty)) => ty,
None => {
checking_data.diagnostics_container.add_error(
Expand Down Expand Up @@ -609,11 +622,13 @@ impl<'a> Environment<'a> {

// TODO temp position
let mut value = None;

for event in self.facts.events.iter() {
// TODO explain why don't need to detect sets
if let Event::ReadsReference {
reference: other_reference,
reflects_dependency: Some(dep),
position,
} = event
{
if reference == *other_reference {
Expand Down Expand Up @@ -643,6 +658,7 @@ impl<'a> Environment<'a> {
self.facts.events.push(Event::ReadsReference {
reference: RootReference::Variable(og_var.get_id()),
reflects_dependency: Some(ty),
position,
});

ty
Expand Down Expand Up @@ -717,11 +733,12 @@ impl<'a> Environment<'a> {
&mut checking_data.types,
);
let falsy_events = falsy_environment.facts.events;
// TODO
// TODO It might be possible to get position from one of the SynthesisableConditional but its `get_position` is not implemented yet
self.facts.events.push(Event::Conditionally {
condition,
events_if_truthy: truthy_events.into_boxed_slice(),
else_events: falsy_events.into_boxed_slice(),
position: None,
});

// TODO all things that are
Expand All @@ -734,6 +751,7 @@ impl<'a> Environment<'a> {
condition,
events_if_truthy: truthy_events.into_boxed_slice(),
else_events: Default::default(),
position: None,
});

// TODO above
Expand All @@ -742,12 +760,12 @@ impl<'a> Environment<'a> {
}
}

pub fn throw_value(&mut self, value: TypeId) {
self.facts.events.push(Event::Throw(value));
pub fn throw_value(&mut self, value: TypeId, position: SpanWithSource) {
self.facts.events.push(Event::Throw(value, position));
}

pub fn return_value(&mut self, returned: TypeId) {
self.facts.events.push(Event::Return { returned })
pub fn return_value(&mut self, returned: TypeId, returned_position: SpanWithSource) {
self.facts.events.push(Event::Return { returned, returned_position })
}

/// Updates **a existing property**
Expand All @@ -760,6 +778,7 @@ impl<'a> Environment<'a> {
kind: PublicityKind,
new: TypeId,
types: &mut TypeStore,
setter_position: Option<SpanWithSource>,
) -> Result<Option<TypeId>, SetPropertyError> {
crate::types::properties::set_property(
on,
Expand All @@ -769,6 +788,7 @@ impl<'a> Environment<'a> {
self,
&mut CheckThings,
types,
setter_position,
)
}

Expand All @@ -787,13 +807,16 @@ impl<'a> Environment<'a> {
));

let prototype = crate::events::PrototypeArgument::Yeah(over);
// TODO Unknown position
let event = Event::Conditionally {
condition,
events_if_truthy: Box::new([Event::CreateObject {
referenced_in_scope_as: ty,
prototype,
position: None,
}]),
else_events: Box::new([]),
position: None,
};

self.facts.events.push(event);
Expand Down Expand Up @@ -999,6 +1022,7 @@ pub(crate) fn get_this_type_from_constraint(
facts: &mut Facts,
this_ty: TypeId,
types: &mut TypeStore,
position: SpanWithSource,
) -> TypeId {
// TODO `this_ty` can be error here..?
if this_ty == TypeId::ERROR_TYPE {
Expand Down Expand Up @@ -1034,6 +1058,7 @@ pub(crate) fn get_this_type_from_constraint(
if let Event::ReadsReference {
reference: other_reference,
reflects_dependency: Some(dep),
position,
} = event
{
if reference == RootReference::This {
Expand All @@ -1049,6 +1074,7 @@ pub(crate) fn get_this_type_from_constraint(
facts.events.push(Event::ReadsReference {
reference: RootReference::This,
reflects_dependency: Some(ty),
position,
});

ty
Expand Down
12 changes: 9 additions & 3 deletions checker/src/context/facts.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::{borrow::Cow, collections::HashMap};

use source_map::SpanWithSource;

use crate::{
behavior::functions::ClosureId,
events::{Event, RootReference},
Expand Down Expand Up @@ -42,6 +44,7 @@ impl Facts {
to: Property,
register_setter_event: bool,
publicity: PublicityKind,
position: Option<SpanWithSource>,
) {
// crate::utils::notify!("Registering {:?} {:?} {:?}", on, under, to);
self.current_properties.entry(on).or_default().push((under, publicity, to.clone()));
Expand All @@ -53,12 +56,13 @@ impl Facts {
reflects_dependency: None,
initialization: true,
publicity,
position,
});
}
}

pub(crate) fn throw_value(&mut self, value: TypeId) {
self.events.push(Event::Throw(value));
pub(crate) fn throw_value(&mut self, value: TypeId, position: SpanWithSource) {
self.events.push(Event::Throw(value, position));
}

pub fn get_events(&self) -> &[Event] {
Expand All @@ -85,7 +89,9 @@ impl Facts {
};
// TODO maybe register the environment if function ...
// TODO register properties
let value = Event::CreateObject { referenced_in_scope_as: ty, prototype };
// TODO Needs a position (or not?)
let value =
Event::CreateObject { referenced_in_scope_as: ty, prototype, position: None };
self.events.push(value);
}

Expand Down
8 changes: 6 additions & 2 deletions checker/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1400,12 +1400,16 @@ impl<T: ContextType> Context<T> {
self.parents_iter().map(|env| get_on_ctx!(&env.facts))
}

pub(crate) fn get_value_of_this(&mut self, types: &mut TypeStore) -> TypeId {
pub(crate) fn get_value_of_this(
&mut self,
types: &mut TypeStore,
position: SpanWithSource,
) -> TypeId {
match self.can_use_this {
CanUseThis::NotYetSuperToBeCalled { .. } => todo!("Cannot use super before call"),
CanUseThis::ConstructorCalled { this_ty } => this_ty,
CanUseThis::Yeah { this_ty } => {
get_this_type_from_constraint(&mut self.facts, this_ty, types)
get_this_type_from_constraint(&mut self.facts, this_ty, types, position)
}
}
}
Expand Down
16 changes: 11 additions & 5 deletions checker/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ mod defined_errors_and_warnings {
ReturnedTypeDoesNotMatch {
expected_return_type: TypeStringRepresentation,
returned_type: TypeStringRepresentation,
position: SpanWithSource,
annotation_position: SpanWithSource,
returned_position: SpanWithSource,
},
// TODO are these the same errors?
TypeIsNotIndexable(TypeStringRepresentation),
Expand Down Expand Up @@ -439,14 +440,19 @@ mod defined_errors_and_warnings {
kind: super::DiagnosticKind::Error,
},
TypeCheckError::ReturnedTypeDoesNotMatch {
position,
annotation_position,
returned_position,
expected_return_type,
returned_type,
} => Diagnostic::Position {
} => Diagnostic::PositionWithAdditionLabels {
reason: format!(
"Function is expected to return {expected_return_type} but returned {returned_type}",
"Cannot return {returned_type} because the function is expected to return {expected_return_type}",
),
position,
labels: vec![(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about this, I think returned_position should be the main diagnostic position. Could we swap the values for the main position and label?

Then the main reason should be "Cannot return {returned type} from function expected to return {expected_return_type}" and the label one should be "function annotated to return {expected_return_type} here"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On it, let me swap them and see how the new message looks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this look?

Screenshot 2023-11-04 150607

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

format!("Function annotated to return {expected_return_type} here"),
Some(annotation_position.clone()),
)],
position: returned_position,
kind: super::DiagnosticKind::Error,
},
TypeCheckError::TypeHasNoGenericParameters(name, position) => {
Expand Down
Loading