Skip to content

Commit

Permalink
Do not panic when there is auto impl type check (#6094)
Browse files Browse the repository at this point in the history
## Description

This PR tries to fix #6093.

The cause seems to be a problem with our error reporting. What is
happening is, the source code has other errors unrelated to auto impl of
AbiEncode. For example, some private fields are being accessed from
other modules.

This is currently panicking because when type-checking the auto impl,
some of these errors "leak" into the auto impl `type_check` and make it
fail completely; I was expecting the type check would always finish,
even if with some errors.

This was being used as a security guard against implementation bugs. But
now that the auto impl is more stable we can remove it, and when a type
check completely fails, we do not panic anymore, just failing that
particular auto impl.

## Checklist

- [ ] I have linked to any relevant issues.
- [ ] 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).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [ ] 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
xunilrj authored Jun 5, 2024
1 parent 0e56728 commit e1fcf98
Showing 1 changed file with 19 additions and 21 deletions.
40 changes: 19 additions & 21 deletions sway-core/src/semantic_analysis/ast_node/declaration/auto_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,8 @@ where

use std::fmt::Write;
let mut code = String::new();
writeln!(&mut code, "let variant: u64 = buffer.decode::<u64>();").unwrap();
writeln!(&mut code, "match variant {{ {arms} _ => __revert(0), }}").unwrap();
let _ = writeln!(&mut code, "let variant: u64 = buffer.decode::<u64>();");
let _ = writeln!(&mut code, "match variant {{ {arms} _ => __revert(0), }}");

Some(code)
}
Expand Down Expand Up @@ -306,7 +306,7 @@ where

let decl = match nodes[0].content {
AstNodeContent::Declaration(Declaration::FunctionDeclaration(f)) => f,
_ => todo!(),
_ => unreachable!("unexpected node"),
};

if handler.has_errors() {
Expand All @@ -324,21 +324,19 @@ where
assert!(!handler.has_warnings(), "{:?}", handler);

let ctx = self.ctx.by_ref();
let (decl, namespace) = ctx
.scoped_and_namespace(|ctx| {
TyDecl::type_check(
&handler,
ctx,
parsed::Declaration::FunctionDeclaration(decl),
)
})
.unwrap();

assert!(!handler.has_warnings(), "{:?}", handler);
let r = ctx.scoped_and_namespace(|ctx| {
TyDecl::type_check(
&handler,
ctx,
parsed::Declaration::FunctionDeclaration(decl),
)
});

// Uncomment this to understand why an entry function was not generated
// println!("{:#?}", handler);

let (decl, namespace) = r.map_err(|_| handler.clone())?;

if handler.has_errors() || matches!(decl, TyDecl::ErrorRecovery(_, _)) {
Err(handler)
} else {
Expand Down Expand Up @@ -371,21 +369,21 @@ where

let decl = match nodes[0].content {
AstNodeContent::Declaration(Declaration::ImplTrait(f)) => f,
_ => todo!(),
_ => unreachable!("unexpected item"),
};

assert!(!handler.has_errors(), "{:?}", handler);

let ctx = self.ctx.by_ref();
let (decl, namespace) = ctx
.scoped_and_namespace(|ctx| {
TyDecl::type_check(&handler, ctx, Declaration::ImplTrait(decl))
})
.unwrap();
let r = ctx.scoped_and_namespace(|ctx| {
TyDecl::type_check(&handler, ctx, Declaration::ImplTrait(decl))
});

// Uncomment this to understand why auto impl failed for a type.
// println!("{:#?}", handler);

let (decl, namespace) = r.map_err(|_| handler.clone())?;

if handler.has_errors() || matches!(decl, TyDecl::ErrorRecovery(_, _)) {
Err(handler)
} else {
Expand Down Expand Up @@ -785,7 +783,7 @@ where
if let Some(constraint_not_satisfied) = constraint_not_satisfied {
let ty = match constraint_not_satisfied {
CompileError::TraitConstraintNotSatisfied { ty, .. } => ty,
_ => unreachable!(),
_ => unreachable!("unexpected error"),
};
handler.emit_err(CompileError::CouldNotGenerateEntryMissingImpl {
ty,
Expand Down

0 comments on commit e1fcf98

Please sign in to comment.