Skip to content

Commit 6a2da44

Browse files
author
Vladimir Radosavljevic
committed
[v2] Error on unresolvable or invalid inheritance bases
Add a diagnostic when encountering unresolvable or invalid inheritance bases. Adds two diagnostic groups: Resolution (IdentifierNotFound) for bases that don't resolve, and Type System (InvalidBase) for bases that resolve to a non-contract/interface (e.g. a library, struct, or free function). Signed-off-by: Vladimir Radosavljevic <vr@matterlabs.dev>
1 parent 616a5bf commit 6a2da44

20 files changed

Lines changed: 391 additions & 30 deletions

File tree

crates/solidity-v2/outputs/cargo/common/generated/public_api.txt

Lines changed: 106 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/solidity-v2/outputs/cargo/common/src/diagnostics/kinds/mod.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
mod utils;
22

33
pub mod compilation;
4+
pub mod resolution;
45
pub mod structure;
56
pub mod syntax;
7+
pub mod type_system;
68

79
use compilation::CompilationDiagnosticKind;
10+
use resolution::ResolutionDiagnosticKind;
811
use serde::Serialize;
912
use structure::StructureDiagnosticKind;
1013
use syntax::SyntaxDiagnosticKind;
14+
use type_system::TypeSystemDiagnosticKind;
1115

1216
use crate::diagnostics::kinds::utils::define_diagnostic_kind;
1317

@@ -23,5 +27,11 @@ define_diagnostic_kind! {
2327
Syntax(SyntaxDiagnosticKind),
2428
/// A diagnostic about structural shape.
2529
Structure(StructureDiagnosticKind),
30+
/// A diagnostic produced for undeclared identifiers, duplicate
31+
/// definitions, import failures, shadowing, ambiguous references
32+
/// and scope errors.
33+
Resolution(ResolutionDiagnosticKind),
34+
/// A diagnostic about the type system.
35+
TypeSystem(TypeSystemDiagnosticKind),
2636
}
2737
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
use serde::Serialize;
2+
3+
use crate::diagnostics::extensions::DiagnosticExtensions;
4+
use crate::diagnostics::severity::DiagnosticSeverity;
5+
6+
/// Diagnostic emitted when an identifier could not be resolved — it was not
7+
/// found, or was not unique.
8+
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
9+
pub struct IdentifierNotFound;
10+
11+
impl DiagnosticExtensions for IdentifierNotFound {
12+
fn severity(&self) -> DiagnosticSeverity {
13+
DiagnosticSeverity::Error
14+
}
15+
16+
fn code(&self) -> &'static str {
17+
"resolution/identifier-not-found"
18+
}
19+
20+
fn message(&self) -> String {
21+
"Identifier not found or not unique.".to_owned()
22+
}
23+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
mod identifier_not_found;
2+
3+
pub use identifier_not_found::IdentifierNotFound;
4+
use serde::Serialize;
5+
6+
use crate::diagnostics::kinds::utils::define_diagnostic_kind;
7+
use crate::diagnostics::kinds::DiagnosticKind;
8+
9+
define_diagnostic_kind! {
10+
parent_kind = DiagnosticKind::Resolution;
11+
12+
/// Group of diagnostics for undeclared identifiers, duplicate
13+
/// definitions, import failures, shadowing, ambiguous references
14+
/// and scope errors.
15+
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
16+
pub enum ResolutionDiagnosticKind {
17+
/// An identifier could not be resolved.
18+
IdentifierNotFound(IdentifierNotFound),
19+
}
20+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
use serde::Serialize;
2+
3+
use crate::diagnostics::extensions::DiagnosticExtensions;
4+
use crate::diagnostics::severity::DiagnosticSeverity;
5+
6+
/// Diagnostic emitted when a base in an inheritance list resolves to something
7+
/// that is not a contract or interface (for example a library, struct, or free
8+
/// function), and therefore cannot be used as a base.
9+
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
10+
pub struct InvalidBase;
11+
12+
impl DiagnosticExtensions for InvalidBase {
13+
fn severity(&self) -> DiagnosticSeverity {
14+
DiagnosticSeverity::Error
15+
}
16+
17+
fn code(&self) -> &'static str {
18+
"type-system/invalid-base"
19+
}
20+
21+
fn message(&self) -> String {
22+
"Only a contract or interface can be used as a base.".to_owned()
23+
}
24+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
mod invalid_base;
2+
3+
pub use invalid_base::InvalidBase;
4+
use serde::Serialize;
5+
6+
use crate::diagnostics::kinds::utils::define_diagnostic_kind;
7+
use crate::diagnostics::kinds::DiagnosticKind;
8+
9+
define_diagnostic_kind! {
10+
parent_kind = DiagnosticKind::TypeSystem;
11+
12+
/// Group of diagnostics about the type system — type mismatches, invalid
13+
/// conversions, operator type errors, function-call type mismatches, and ABI
14+
/// encoding constraints that are properties of a type.
15+
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
16+
pub enum TypeSystemDiagnosticKind {
17+
/// A base in an inheritance list is not a contract or interface.
18+
InvalidBase(InvalidBase),
19+
}
20+
}

crates/solidity-v2/outputs/cargo/semantic/src/context/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ impl SemanticContext {
7272
let mut types = TypeRegistry::default();
7373

7474
p1_collect_definitions::run(files, &mut binder, diagnostics);
75-
p2_linearise_contracts::run(files, &mut binder);
75+
p2_linearise_contracts::run(files, &mut binder, diagnostics);
7676
p3_type_definitions::run(files, &mut binder, &mut types, language_version);
7777
p4_resolve_references::run(files, &mut binder, &mut types, language_version);
7878

crates/solidity-v2/outputs/cargo/semantic/src/passes/p2_linearise_contracts/mod.rs

Lines changed: 64 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
use std::collections::HashMap;
22

3+
use slang_solidity_v2_common::diagnostics::kinds::resolution::IdentifierNotFound;
4+
use slang_solidity_v2_common::diagnostics::kinds::type_system::InvalidBase;
5+
use slang_solidity_v2_common::diagnostics::DiagnosticCollection;
36
use slang_solidity_v2_common::nodes::NodeId;
47
use slang_solidity_v2_ir::ir;
58

@@ -11,27 +14,49 @@ mod c3;
1114

1215
/// In this pass we collect all bases of contracts and interfaces and then
1316
/// compute the linearisation for each of them.
14-
pub fn run(files: &[impl SemanticFile], binder: &mut Binder) {
17+
pub fn run(
18+
files: &[impl SemanticFile],
19+
binder: &mut Binder,
20+
diagnostics: &mut DiagnosticCollection,
21+
) {
1522
for file in files {
16-
Pass::visit_file_collect_bases(file, binder);
23+
Pass::visit_file_collect_bases(file, binder, diagnostics);
1724
}
1825
for file in files {
19-
Pass::visit_file_linearise_contracts(file, binder);
26+
Pass::visit_file_linearise_contracts(file, binder, diagnostics);
2027
}
2128
}
2229

2330
struct Pass<'a> {
2431
binder: &'a mut Binder,
32+
diagnostics: &'a mut DiagnosticCollection,
33+
file_id: String,
2534
}
2635

2736
impl<'a> Pass<'a> {
28-
fn visit_file_collect_bases(file: &impl SemanticFile, binder: &'a mut Binder) {
29-
let mut pass = Self { binder };
37+
fn visit_file_collect_bases(
38+
file: &impl SemanticFile,
39+
binder: &'a mut Binder,
40+
diagnostics: &'a mut DiagnosticCollection,
41+
) {
42+
let mut pass = Self {
43+
binder,
44+
diagnostics,
45+
file_id: file.id().to_owned(),
46+
};
3047
pass.collect_bases_from(file.ir_root());
3148
}
3249

33-
fn visit_file_linearise_contracts(file: &impl SemanticFile, binder: &'a mut Binder) {
34-
let mut pass = Self { binder };
50+
fn visit_file_linearise_contracts(
51+
file: &impl SemanticFile,
52+
binder: &'a mut Binder,
53+
diagnostics: &'a mut DiagnosticCollection,
54+
) {
55+
let mut pass = Self {
56+
binder,
57+
diagnostics,
58+
file_id: file.id().to_owned(),
59+
};
3560
pass.linearise_contracts_from(file.ir_root());
3661
}
3762

@@ -80,22 +105,38 @@ impl<'a> Pass<'a> {
80105
types: &ir::InheritanceTypes,
81106
scope_id: ScopeId,
82107
) -> Vec<NodeId> {
83-
// TODO(validation) SDR[23]: check that we are able to resolve all bases as
84-
// otherwise we end up with a short list saved in the definition
85-
types
86-
.iter()
87-
.filter_map(|inheritance_type| {
88-
resolve_identifier_path_in_scope(self.binder, &inheritance_type.type_name, scope_id)
89-
.as_definition_id()
90-
.filter(|definition_id| {
91-
// bases are either contracts or interfaces, discard anything else
92-
matches!(
93-
self.binder.find_definition_by_id(*definition_id).unwrap(),
94-
Definition::Contract(_) | Definition::Interface(_)
95-
)
96-
})
97-
})
98-
.collect()
108+
let mut resolved_bases = Vec::new();
109+
for inheritance_type in types {
110+
let resolution = resolve_identifier_path_in_scope(
111+
self.binder,
112+
&inheritance_type.type_name,
113+
scope_id,
114+
);
115+
match resolution.as_definition_id() {
116+
None => {
117+
self.diagnostics.push(
118+
self.file_id.clone(),
119+
inheritance_type.range.clone(),
120+
IdentifierNotFound,
121+
);
122+
}
123+
Some(definition_id) => {
124+
match self.binder.find_definition_by_id(definition_id).unwrap() {
125+
Definition::Contract(_) | Definition::Interface(_) => {
126+
resolved_bases.push(definition_id);
127+
}
128+
_ => {
129+
self.diagnostics.push(
130+
self.file_id.clone(),
131+
inheritance_type.range.clone(),
132+
InvalidBase,
133+
);
134+
}
135+
}
136+
}
137+
}
138+
}
139+
resolved_bases
99140
}
100141

101142
fn linearise_contracts_from(&mut self, source_unit: &ir::SourceUnit) {

crates/solidity-v2/outputs/cargo/semantic/src/passes/tests/binder.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ contract Test is Base layout at 0 {}
3030
let mut binder = Binder::default();
3131
let mut diagnostics = DiagnosticCollection::default();
3232
p1_collect_definitions::run(&files, &mut binder, &mut diagnostics);
33+
p2_linearise_contracts::run(&files, &mut binder, &mut diagnostics);
3334
assert!(
3435
diagnostics.is_empty(),
3536
"Semantic diagnostics: {diagnostics:?}"
3637
);
37-
p2_linearise_contracts::run(&files, &mut binder);
3838

3939
// Verify definitions were collected
4040
assert_eq!(2, binder.definitions().len());
@@ -93,11 +93,11 @@ interface A is C {}
9393
let mut binder = Binder::default();
9494
let mut diagnostics = DiagnosticCollection::default();
9595
p1_collect_definitions::run(&files, &mut binder, &mut diagnostics);
96+
p2_linearise_contracts::run(&files, &mut binder, &mut diagnostics);
9697
assert!(
9798
diagnostics.is_empty(),
9899
"Semantic diagnostics: {diagnostics:?}"
99100
);
100-
p2_linearise_contracts::run(&files, &mut binder);
101101

102102
let contract_to_bases = get_contract_to_bases_map(&binder);
103103

@@ -147,7 +147,11 @@ contract Test is Base, Foo { // Base should resolve to the contract, not the var
147147
diagnostics.is_empty(),
148148
"Semantic diagnostics: {diagnostics:?}"
149149
);
150-
p2_linearise_contracts::run(&files, &mut binder);
150+
p2_linearise_contracts::run(&files, &mut binder, &mut diagnostics);
151+
assert!(
152+
!diagnostics.is_empty(),
153+
"Expected diagnostics for invalid input"
154+
);
151155

152156
let contract_to_bases = get_contract_to_bases_map(&binder);
153157

@@ -198,11 +202,11 @@ contract Test is Base {
198202

199203
let mut diagnostics = DiagnosticCollection::default();
200204
p1_collect_definitions::run(&files, &mut binder, &mut diagnostics);
205+
p2_linearise_contracts::run(&files, &mut binder, &mut diagnostics);
201206
assert!(
202207
diagnostics.is_empty(),
203208
"Semantic diagnostics: {diagnostics:?}"
204209
);
205-
p2_linearise_contracts::run(&files, &mut binder);
206210

207211
let types_before = types.iter_types().count();
208212
p3_type_definitions::run(&files, &mut binder, &mut types, language_version);
@@ -257,11 +261,11 @@ contract Test is Base {
257261

258262
let mut diagnostics = DiagnosticCollection::default();
259263
p1_collect_definitions::run(&files, &mut binder, &mut diagnostics);
264+
p2_linearise_contracts::run(&files, &mut binder, &mut diagnostics);
260265
assert!(
261266
diagnostics.is_empty(),
262267
"Semantic diagnostics: {diagnostics:?}"
263268
);
264-
p2_linearise_contracts::run(&files, &mut binder);
265269
p3_type_definitions::run(&files, &mut binder, &mut types, language_version);
266270
p4_resolve_references::run(&files, &mut binder, &mut types, language_version);
267271

crates/solidity-v2/outputs/cargo/semantic/src/passes/tests/typing.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@ fn analyze(language_version: LanguageVersion, source: &str) -> TypeAnalysis {
3232
let mut types = TypeRegistry::default();
3333
let mut diagnostics = DiagnosticCollection::default();
3434
p1_collect_definitions::run(&files, &mut binder, &mut diagnostics);
35+
p2_linearise_contracts::run(&files, &mut binder, &mut diagnostics);
3536
assert!(
3637
diagnostics.is_empty(),
3738
"Semantic diagnostics: {diagnostics:?}"
3839
);
39-
p2_linearise_contracts::run(&files, &mut binder);
4040
p3_type_definitions::run(&files, &mut binder, &mut types, language_version);
4141
p4_resolve_references::run(&files, &mut binder, &mut types, language_version);
4242

0 commit comments

Comments
 (0)