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

Closed
wants to merge 12 commits into from
Closed
Show file tree
Hide file tree
Changes from 10 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
368 changes: 365 additions & 3 deletions compiler-core/generated/schema_capnp.rs

Large diffs are not rendered by default.

16 changes: 15 additions & 1 deletion 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 Down
43 changes: 35 additions & 8 deletions compiler-core/src/analyse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ use crate::{
hydrator::Hydrator,
prelude::*,
AccessorsMap, Deprecation, ModuleInterface, PatternConstructor, RecordAccessor, Type,
TypeConstructor, TypeValueConstructor, TypeValueConstructorField, TypeVariantConstructors,
ValueConstructor, ValueConstructorVariant, Warning,
TypeAliasConstructor, TypeConstructor, TypeValueConstructor, TypeValueConstructorField,
TypeVariantConstructors, ValueConstructor, ValueConstructorVariant, Warning,
},
uid::UniqueIdGenerator,
warning::TypeWarningEmitter,
Expand Down Expand Up @@ -296,6 +296,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 +319,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 @@ -965,7 +968,12 @@ 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);
Expand Down Expand Up @@ -1121,6 +1129,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 +1186,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 +1204,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
6 changes: 5 additions & 1 deletion 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 All @@ -225,7 +227,7 @@ impl Package {
}
}

#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct Module {
pub name: EcoString,
pub code: EcoString,
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
47 changes: 38 additions & 9 deletions compiler-core/src/build/package_compiler.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::analyse::{ModuleAnalyzerConstructor, TargetSupport};
use crate::line_numbers::{self, LineNumbers};
use crate::package_interface::{self, ModuleInterface};
use crate::type_::PRELUDE_MODULE_NAME;
use crate::{
ast::{SrcSpan, TypedModule, UntypedModule},
Expand All @@ -22,6 +23,7 @@ use crate::{
};
use askama::Template;
use ecow::EcoString;
use std::borrow::BorrowMut;
use std::collections::HashSet;
use std::{collections::HashMap, fmt::write, time::SystemTime};
use vec1::Vec1;
Expand All @@ -30,6 +32,11 @@ use camino::{Utf8Path, Utf8PathBuf};

use super::{ErlangAppCodegenConfiguration, TargetCodegenConfiguration, Telemetry};

pub struct Compiled {
pub modules: Vec<Module>,
pub cached_modules: Vec<type_::ModuleInterface>,
}

#[derive(Debug)]
pub struct PackageCompiler<'a, IO> {
pub io: IO,
Expand Down Expand Up @@ -104,7 +111,7 @@ where
stale_modules: &mut StaleTracker,
incomplete_modules: &mut HashSet<EcoString>,
telemetry: &dyn Telemetry,
) -> Outcome<Vec<Module>, Error> {
) -> Outcome<Compiled, Error> {
let span = tracing::info_span!("compile", package = %self.config.name.as_str());
let _enter = span.enter();

Expand Down Expand Up @@ -146,7 +153,7 @@ where
};

// Load the cached modules that have previously been compiled
for module in loaded.cached.into_iter() {
for module in loaded.cached.clone().into_iter() {
// Emit any cached warnings.
// Note that `self.cached_warnings` is set to `Ignore` (such as for
// dependency packages) then this field will not be populated.
Expand Down Expand Up @@ -182,9 +189,18 @@ where
incomplete_modules,
);

let modules = match outcome {
let mut modules = match outcome {
Outcome::Ok(modules) => modules,
Outcome::PartialFailure(_, _) | Outcome::TotalFailure(_) => return outcome,
Outcome::PartialFailure(modules, err) => {
return Outcome::PartialFailure(
Compiled {
modules,
cached_modules: loaded.cached,
},
err,
)
}
Outcome::TotalFailure(err) => return Outcome::TotalFailure(err),
};

tracing::debug!("performing_code_generation");
Expand All @@ -193,11 +209,18 @@ where
return error.into();
}

for mut module in modules.iter_mut() {
module.ast.type_info.remove_duplicated_type_aliases();
module.attach_doc_and_module_comments();
}

if let Err(error) = self.encode_and_write_metadata(&modules) {
return error.into();
}

Outcome::Ok(modules)
Outcome::Ok(Compiled {
modules,
cached_modules: loaded.cached,
})
}

fn compile_erlang_to_beam(&mut self, modules: &HashSet<Utf8PathBuf>) -> Result<(), Error> {
Expand Down Expand Up @@ -279,7 +302,7 @@ where
Ok(())
}

fn encode_and_write_metadata(&mut self, modules: &[Module]) -> Result<()> {
fn encode_and_write_metadata(&mut self, modules: &Vec<Module>) -> Result<()> {
if !self.write_metadata {
tracing::debug!("package_metadata_writing_disabled");
return Ok(());
Expand Down Expand Up @@ -513,12 +536,15 @@ fn analyse(
.infer_module(ast, line_numbers, path.clone());

match analysis {
Outcome::Ok(ast) => {
Outcome::Ok(mut ast) => {
// Module has compiled successfully. Make sure it isn't marked as incomplete.
let _ = incomplete_modules.remove(&name.clone());
// Register the types from this module so they can be imported into
// other modules.
let _ = module_types.insert(name.clone(), ast.type_info.clone());

ast.type_info.remove_duplicated_type_aliases();

// Register the successfully type checked module data so that it can be
// used for code generation and in the language server.
modules.push(Module {
Expand All @@ -533,7 +559,7 @@ fn analyse(
});
}

Outcome::PartialFailure(ast, errors) => {
Outcome::PartialFailure(mut ast, errors) => {
let error = Error::Type {
names: ast.names.clone(),
path: path.clone(),
Expand All @@ -542,6 +568,9 @@ fn analyse(
};
// Mark as incomplete so that this module isn't reloaded from cache.
let _ = incomplete_modules.insert(name.clone());

ast.type_info.remove_duplicated_type_aliases();

// Register the partially type checked module data so that it can be
// used in the language server.
modules.push(Module {
Expand Down
2 changes: 2 additions & 0 deletions compiler-core/src/build/package_loader/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,10 @@ fn write_cache(
name: name.into(),
origin: Origin::Src,
package: "my_package".into(),
documentation: vec![],
types: Default::default(),
types_value_constructors: Default::default(),
type_aliases: Default::default(),
values: Default::default(),
accessors: Default::default(),
line_numbers: line_numbers.clone(),
Expand Down
Loading