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

Fixed export package-definitions not including cached modules #3669

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
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
540 changes: 533 additions & 7 deletions compiler-core/generated/schema_capnp.rs

Large diffs are not rendered by default.

23 changes: 21 additions & 2 deletions compiler-core/schema.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ struct Option(Value) {

struct Module {
name @0 :Text;
documentation @10 :List(Text);
types @1 :List(Property(TypeConstructor));
typeAliases @11 :List(Property(TypeAliasConstructor));
values @2 :List(Property(ValueConstructor));
accessors @3 :List(Property(AccessorsMap));
package @4 :Text;
typesConstructors @5 :List(Property(TypesVariantConstructors));
typesConstructors @5 :List(Property(TypesVariantConstructors));
lineNumbers @6 :LineNumbers;
srcPath @7 :Text;
isInternal @8 :Bool;
Expand All @@ -50,6 +52,7 @@ struct TypeValueConstructor {

struct TypeValueConstructorParameter {
type @0 :Type;
label @1 :Text;
}

struct TypeConstructor {
Expand All @@ -60,11 +63,22 @@ struct TypeConstructor {
parameters @1 :List(Type);
module @2 :Text;
publicity @3 :Publicity;
opaque @7 :Bool;
deprecated @4 :Text;
origin @5 :SrcSpan;
documentation @6 :Text;
}

struct TypeAliasConstructor {
publicity @0 :Publicity;
module @1 :Text;
origin @2 :SrcSpan;
type @3 :Type;
arity @4 :UInt16;
deprecated @5 :Text;
documentation @6 :Text;
}

struct AccessorsMap {
type @0 :Type;
accessors @1 :List(Property(RecordAccessor));
Expand All @@ -76,6 +90,11 @@ struct RecordAccessor {
label @2 :Text;
}

struct FunctionArgument {
name @0 :Text;
Copy link
Member

Choose a reason for hiding this comment

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

Function types don't contain names! Please remove this.

It would also be called a parameter. Things have parameters, and for those parameters they accept arguments.

Copy link
Author

Choose a reason for hiding this comment

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

So you mean that a parameter has a name/label, while an argument doesn't? In that case, I'd reuse the TypeValueConstructorParameter struct?

Copy link
Author

Choose a reason for hiding this comment

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

To be fair I wasn't happy with the approach I took 😅 . Tomorrow I'll try tackling this again.

Copy link
Member

Choose a reason for hiding this comment

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

Neither has names at the type level, but parameters can have names locally within a definition.

type @1 :Type;
}

struct Type {
union {
app :group {
Expand All @@ -86,7 +105,7 @@ struct Type {
}

fn :group {
arguments @3 :List(Type);
arguments @3 :List(FunctionArgument);
return @4 :Type;
}

Expand Down
70 changes: 57 additions & 13 deletions compiler-core/src/analyse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ use crate::{
fields::{FieldMap, FieldMapBuilder},
hydrator::Hydrator,
prelude::*,
AccessorsMap, Deprecation, ModuleInterface, PatternConstructor, RecordAccessor, Type,
TypeConstructor, TypeValueConstructor, TypeValueConstructorField, TypeVariantConstructors,
ValueConstructor, ValueConstructorVariant, Warning,
AccessorsMap, Deprecation, FunctionArgument, ModuleInterface, PatternConstructor,
RecordAccessor, Type, TypeAliasConstructor, TypeConstructor, TypeValueConstructor,
TypeValueConstructorField, TypeVariantConstructors, ValueConstructor,
ValueConstructorVariant, Warning,
},
uid::UniqueIdGenerator,
warning::TypeWarningEmitter,
Expand Down Expand Up @@ -296,6 +297,7 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
let Environment {
module_types: types,
module_types_constructors: types_constructors,
module_type_aliases: type_aliases,
module_values: values,
accessors,
names: type_names,
Expand All @@ -318,13 +320,15 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
}

let module = ast::Module {
documentation,
documentation: documentation.clone(),
name: self.module_name.clone(),
definitions: typed_statements,
type_info: ModuleInterface {
name: self.module_name,
documentation,
types,
types_value_constructors: types_constructors,
type_aliases,
values,
accessors,
origin: self.origin,
Expand Down Expand Up @@ -523,7 +527,13 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
body,
Some(prereg_return_type.clone()),
)?;
let args_types = args.iter().map(|a| a.type_.clone()).collect();
let args_types = args
.iter()
.map(|a| FunctionArgument {
name: a.get_variable_name().cloned(),
type_: a.type_.clone(),
})
.collect();
let type_ = fn_(args_types, body.last().type_());
Ok((
type_,
Expand Down Expand Up @@ -965,10 +975,18 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
}
};

fields.push(TypeValueConstructorField { type_: t.clone() });
fields.push(TypeValueConstructorField {
label: label
.as_ref()
.map_or(None, |(_, label)| Some(label.clone())),
type_: t.clone(),
});

// Register the type for this parameter
args_types.push(t);
args_types.push(FunctionArgument {
name: None,
type_: t,
});

// Register the label for this parameter, if there is one
if let Some((_, label)) = label {
Expand All @@ -984,7 +1002,7 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
// Insert constructor function into module scope
let type_ = match constructor.arguments.len() {
0 => type_.clone(),
_ => fn_(args_types.clone(), type_.clone()),
_ => fn_(args_types, type_.clone()),
};
let constructor_info = ValueConstructorVariant::Record {
documentation: constructor
Expand Down Expand Up @@ -1121,6 +1139,7 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
deprecation: deprecation.clone(),
parameters,
publicity,
opaque: *opaque,
type_,
documentation: documentation.as_ref().map(|(_, doc)| doc.clone()),
},
Expand Down Expand Up @@ -1177,13 +1196,16 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
// in some fashion.
let mut hydrator = Hydrator::new();
let parameters = self.make_type_vars(args, &mut hydrator, environment);
let tryblock = || {
let mut tryblock = || {
hydrator.disallow_new_type_variables();
let type_ = hydrator.type_from_ast(resolved_type, environment, &mut self.problems)?;
let type_ref =
hydrator.type_from_ast(resolved_type, environment, &mut self.problems)?;
let type_ = type_ref.as_ref().clone();
let arity = parameters.len();

environment
.names
.type_in_scope(name.clone(), type_.as_ref());
.type_in_scope(name.clone(), type_ref.as_ref());

// Insert the alias so that it can be used by other code.
environment.insert_type_constructor(
Expand All @@ -1192,9 +1214,24 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
origin: *location,
module: self.module_name.clone(),
parameters,
type_,
type_: type_ref,
deprecation: deprecation.clone(),
publicity: *publicity,
// TODO: Find if the type is opaque
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean? Is it unfinished?

Copy link
Author

Choose a reason for hiding this comment

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

Forgot about this todo 😅. Is there a problem for the code to register the type alias as a non-opaque type, just for type analysis?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not aware of any problems, but I'm not sure what you mean here.

opaque: false,
documentation: documentation.as_ref().map(|(_, doc)| doc.clone()),
},
)?;

environment.insert_type_alias(
name.clone(),
TypeAliasConstructor {
origin: *location,
module: self.module_name.clone(),
type_,
arity,
publicity: *publicity,
deprecation: deprecation.clone(),
documentation: documentation.as_ref().map(|(_, doc)| doc.clone()),
},
)?;
Expand Down Expand Up @@ -1294,7 +1331,14 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
let arg_types = args
.iter()
.map(|arg| {
hydrator.type_from_option_ast(&arg.annotation, environment, &mut self.problems)
Ok(FunctionArgument {
name: None,
type_: hydrator.type_from_option_ast(
&arg.annotation,
environment,
&mut self.problems,
)?,
})
})
.try_collect()?;
let return_type =
Expand Down
18 changes: 15 additions & 3 deletions compiler-core/src/ast/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ use crate::{
ast::{SrcSpan, TypedExpr},
build::Located,
type_::{
self, AccessorsMap, Environment, ExprTyper, FieldMap, ModuleValueConstructor,
RecordAccessor, Type, ValueConstructor, ValueConstructorVariant,
self, AccessorsMap, Environment, ExprTyper, FieldMap, FunctionArgument,
ModuleValueConstructor, RecordAccessor, Type, ValueConstructor, ValueConstructorVariant,
},
uid::UniqueIdGenerator,
warning::TypeWarningEmitter,
Expand Down Expand Up @@ -108,7 +108,19 @@ fn compile_expression(src: &str) -> TypedStatement {
environment.insert_variable(
"Cat".into(),
variant,
type_::fn_(vec![type_::string(), type_::int()], cat_type.clone()),
type_::fn_(
vec![
FunctionArgument {
name: None,
type_: type_::string(),
},
FunctionArgument {
name: None,
type_: type_::int(),
},
],
cat_type.clone(),
),
Publicity::Public,
Deprecation::NotDeprecated,
);
Expand Down
4 changes: 4 additions & 0 deletions compiler-core/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use crate::ast::{
CallArg, CustomType, DefinitionLocation, Pattern, TypeAst, TypedArg, TypedDefinition,
TypedExpr, TypedFunction, TypedPattern, TypedStatement,
};
use crate::package_interface;
use crate::type_::Type;
use crate::{
ast::{Definition, SrcSpan, TypedModule},
Expand Down Expand Up @@ -208,6 +209,7 @@ fn mode_includes_tests() {
pub struct Package {
pub config: PackageConfig,
pub modules: Vec<Module>,
pub cached_modules: Vec<type_::ModuleInterface>,
}

impl Package {
Expand Down Expand Up @@ -261,6 +263,8 @@ impl Module {
.map(|span| Comment::from((span, self.code.as_str())).content.into())
.collect();

self.ast.type_info.documentation = self.ast.documentation.clone();

// Order statements to avoid misassociating doc comments after the
// order has changed during compilation.
let mut statements: Vec<_> = self.ast.definitions.iter_mut().collect();
Expand Down
1 change: 1 addition & 0 deletions compiler-core/src/build/module_loader/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::{
build::SourceFingerprint,
io::{memory::InMemoryFileSystem, FileSystemWriter},
line_numbers::LineNumbers,
package_interface,
};
use std::time::Duration;

Expand Down
Loading