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

Type aliases, take #N #357

Merged
merged 16 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
30d277f
fix: Pavex correctly handles type alises with generic parameters that…
LukeMathWalker Oct 27, 2024
4fddb00
chore: Provide an easy-to-examine representation for the set of const…
LukeMathWalker Oct 29, 2024
192a292
fix: Panic eagerly if internal invariants are not upheld when binding…
LukeMathWalker Oct 29, 2024
7315892
feat: Support lifetime parameters in type aliases
LukeMathWalker Oct 29, 2024
a657e0a
fix: Bind generic parameters correctly in all instances
LukeMathWalker Oct 29, 2024
62b8932
chore: Display the cyclic dependency graph when PAVEX_DEBUG is set
LukeMathWalker Oct 29, 2024
8b60796
chore: Display the available constructibles when PAVEX_DEBUG is set a…
LukeMathWalker Oct 29, 2024
288c6d5
fix: Don't complain about missing constructors when looking at a nake…
LukeMathWalker Oct 29, 2024
5162d33
chore: Improve test coverage for type aliases
LukeMathWalker Oct 29, 2024
a49729a
chore: Improve panic message with details about the item we couldn't …
LukeMathWalker Oct 29, 2024
0018b57
fix: Improve error message when we fail to find a method item in the …
LukeMathWalker Oct 29, 2024
e4305c6
fix: Look for the 'impl' block in the crate that define the type, rat…
LukeMathWalker Oct 29, 2024
8161f57
fix: Improve error message
LukeMathWalker Oct 29, 2024
a146063
chore: Punctuation in error messages.
LukeMathWalker Oct 29, 2024
1befce5
chore: Cover foreign traits in tests
LukeMathWalker Oct 29, 2024
2293bc8
chore: Update doc examples
LukeMathWalker Oct 30, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
× I can't find a constructor for `biscotti::ProcessorConfig`.
│ I need an instance of `biscotti::ProcessorConfig` to
│ invoke your constructor, `<pavex::cookie::Processor as
│ std::convert::From::<pavex::cookie::ProcessorConfig>>::from`.
│ core::convert::From::<pavex::cookie::ProcessorConfig>>::from`.
│
│ ╭─[../../../../../libs/pavex/src/cookie/kit.rs:80:1]
│ 80 │ .error_handler(f!(super::errors::InjectResponseCookiesError::into_response));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,13 @@ fn cycle_error(
.unwrap();
}

let diagnostic_builder = CompilerDiagnostic::builder(anyhow::anyhow!(error_msg));
let error = anyhow::anyhow!(
"There is a cycle in this dependency graph. Graph:\n{}",
// TODO: Use a PadAdapter to indent the graph, avoiding the need for an intermediate stringa allocation.
textwrap::indent(&graph.debug_dot(component_db, computation_db), " ")
)
.context(error_msg);
let diagnostic_builder = CompilerDiagnostic::builder(error);

diagnostic_builder.help(
"Break the cycle! Remove one of the 'depends-on' relationship by changing the signature of \
Expand Down
12 changes: 10 additions & 2 deletions libs/pavexc/src/compiler/analyses/components/db/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ impl ComponentDb {
package_graph: &PackageGraph,
) -> Option<AnnotatedSnippet> {
let global_item_id = callable.source_coordinates.as_ref()?;
let item = krate_collection.get_type_by_global_type_id(global_item_id);
let item = krate_collection.get_item_by_global_type_id(global_item_id);
let definition_span = item.span.as_ref()?;
let source_contents = diagnostic::read_source_file(
&definition_span.filename,
Expand All @@ -177,6 +177,10 @@ impl ComponentDb {
syn::parse_str::<syn::ImplItemFn>(&span_contents)
{
item.sig.output
} else if let Ok(item) =
syn::parse_str::<syn::TraitItemFn>(&span_contents)
{
item.sig.output
} else {
panic!("Could not parse as a function or method:\n{span_contents}")
}
Expand Down Expand Up @@ -271,7 +275,7 @@ impl ComponentDb {
package_graph: &PackageGraph,
) -> Option<AnnotatedSnippet> {
let global_item_id = callable.source_coordinates.as_ref()?;
let item = krate_collection.get_type_by_global_type_id(global_item_id);
let item = krate_collection.get_item_by_global_type_id(global_item_id);
let definition_span = item.span.as_ref()?;
let source_contents = diagnostic::read_source_file(
&definition_span.filename,
Expand All @@ -289,6 +293,10 @@ impl ComponentDb {
syn::parse_str::<syn::ImplItemFn>(&span_contents)
{
item.sig.generics.params
} else if let Ok(item) =
syn::parse_str::<syn::TraitItemFn>(&span_contents)
{
item.sig.generics.params
} else {
panic!("Could not parse as a function or method:\n{span_contents}")
}
Expand Down
65 changes: 56 additions & 9 deletions libs/pavexc/src/compiler/analyses/constructibles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,26 @@ use crate::try_source;

use super::framework_items::FrameworkItemDb;

#[derive(Debug)]
/// The set of types that can be injected into request handlers, error handlers and (other) constructors.
pub(crate) struct ConstructibleDb {
scope_id2constructibles: IndexMap<ScopeId, ConstructiblesInScope>,
}

impl std::fmt::Debug for ConstructibleDb {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
writeln!(f, "Available constructibles:\n")?;
for (scope_id, constructibles) in &self.scope_id2constructibles {
writeln!(
f,
"- {scope_id}:\n{}",
// TODO: Use a PadAdapter down here to avoid allocating an intermediate string
textwrap::indent(&format!("{:?}", constructibles), " ")
)?;
}
Ok(())
}
}

impl ConstructibleDb {
/// Compute the set of types that can be injected into request handlers, error handlers and
/// (other) constructors.
Expand Down Expand Up @@ -147,6 +161,20 @@ impl ConstructibleDb {
}
}

// Both `T` and `&T` are always constructibles, when looking at generic constructors that take them
// as input. The actual check will happen when they are bound to a specific type.
match input {
ResolvedType::Reference(ref_) => {
if let ResolvedType::Generic(_) = ref_.inner.as_ref() {
continue;
}
}
ResolvedType::Generic(_) => {
continue;
}
_ => {}
}

if let Some((input_component_id, mode)) = self.get_or_try_bind(
scope_id,
input,
Expand Down Expand Up @@ -232,7 +260,7 @@ impl ConstructibleDb {
continue;
}
if let Some(user_component_id) = component_db.user_component_id(component_id) {
Self::missing_constructor(
self.missing_constructor(
user_component_id,
component_db.user_component_db(),
input,
Expand Down Expand Up @@ -605,6 +633,7 @@ impl ConstructibleDb {
}

fn missing_constructor(
&self,
user_component_id: UserComponentId,
user_component_db: &UserComponentDb,
unconstructible_type: &ResolvedType,
Expand Down Expand Up @@ -640,10 +669,12 @@ impl ConstructibleDb {
.flatten();

let callable = &computation_db[user_component_id];
let e = anyhow::anyhow!(
"I can't find a constructor for `{unconstructible_type:?}`.\n\
I need an instance of `{unconstructible_type:?}` to invoke your {component_kind}, `{}`.",
callable.path
let e = anyhow::anyhow!("I can't find a constructor for `{}`.\n{self:?}", unconstructible_type.display_for_error()).context(
format!(
"I can't find a constructor for `{unconstructible_type:?}`.\n\
I need an instance of `{unconstructible_type:?}` to invoke your {component_kind}, `{}`.",
callable.path
)
);
let definition_info = get_definition_info(
callable,
Expand Down Expand Up @@ -935,7 +966,6 @@ impl ConstructibleDb {
}
}

#[derive(Debug)]
/// The set of constructibles that have been registered in a given scope.
///
/// Be careful! This is not the set of all types that can be constructed in the given scope!
Expand Down Expand Up @@ -1006,15 +1036,22 @@ impl ConstructiblesInScope {
}
for templated_constructible_type in &self.templated_constructors {
if let Some(bindings) = templated_constructible_type.is_a_template_for(type_) {
let (templated_component_id, _) = self.get(templated_constructible_type).unwrap();
let template = templated_constructible_type.clone();
let (templated_component_id, _) = self.get(&template).unwrap();
self.bind_and_register_constructor(
templated_component_id,
component_db,
computation_db,
framework_item_db,
&bindings,
);
return self.get(type_);
let bound = self.get(type_);
assert!(bound.is_some(), "I used {} as a templated constructor to build {} but the binding process didn't succeed as expected.\nBindings:\n{}",
template.display_for_error(),
type_.display_for_error(),
bindings.into_iter().map(|(k, v)| format!("- {k} -> {}", v.display_for_error())).collect::<Vec<_>>().join("\n")
);
return bound;
}
}

Expand Down Expand Up @@ -1090,3 +1127,13 @@ impl ConstructiblesInScope {
}
}
}

impl std::fmt::Debug for ConstructiblesInScope {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
writeln!(f, "Constructibles:")?;
for (type_, component_id) in &self.type2constructor_id {
writeln!(f, "- {} -> {:?}", type_.display_for_error(), component_id)?;
}
Ok(())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ pub struct ScopeGraphBuilder {
/// See [`ScopeGraph`] for more information.
pub struct ScopeId(usize);

impl std::fmt::Display for ScopeId {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "Scope {}", self.0)
}
}

impl PartialEq<ScopeId> for &ScopeId {
fn eq(&self, other: &ScopeId) -> bool {
self.0 == other.0
Expand Down
2 changes: 1 addition & 1 deletion libs/pavexc/src/compiler/component/prebuilt_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::language::ResolvedType;
pub struct PrebuiltType(ResolvedType);

impl PrebuiltType {
pub fn new(ty: ResolvedType) -> Result<Self, PrebuiltTypeValidationError> {
pub(crate) fn new(ty: ResolvedType) -> Result<Self, PrebuiltTypeValidationError> {
if ty.has_implicit_lifetime_parameters() || !ty.named_lifetime_parameters().is_empty() {
return Err(PrebuiltTypeValidationError::CannotHaveLifetimeParameters { ty });
}
Expand Down
4 changes: 2 additions & 2 deletions libs/pavexc/src/compiler/path_parameter_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ pub(crate) fn verify_path_parameters(
unreachable!()
};
for field_id in field_ids {
let field_item = krate_collection.get_type_by_global_type_id(&GlobalItemId {
let field_item = krate_collection.get_item_by_global_type_id(&GlobalItemId {
rustdoc_item_id: field_id.clone(),
package_id: extracted_path_type.package_id.clone(),
});
Expand Down Expand Up @@ -298,7 +298,7 @@ fn must_be_a_plain_struct(
let Some(item_id) = t.rustdoc_id.clone() else {
unreachable!()
};
let item = krate_collection.get_type_by_global_type_id(&GlobalItemId {
let item = krate_collection.get_item_by_global_type_id(&GlobalItemId {
rustdoc_item_id: item_id,
package_id: t.package_id.clone(),
});
Expand Down
Loading