fix: inherit fallback/receive (#99)

paradigmxyz · Oct 31, 2024 · f837010 · f837010
1 parent 4c3e4bf
commit f837010
Show file tree

Hide file tree

Showing 9 changed files with 239 additions and 40 deletions.
diff --git a/crates/interface/src/diagnostics/emitter/human.rs b/crates/interface/src/diagnostics/emitter/human.rs
@@ -34,7 +34,7 @@ pub struct HumanEmitter {
     renderer: Renderer,
 }
 
-// SAFETY: `real_writer` always points to the `dyn Writer` in `writer`.
+// SAFETY: `real_writer` always points to the `Writer` in `writer`.
 unsafe impl Send for HumanEmitter {}
 
 impl Emitter for HumanEmitter {
@@ -71,7 +71,6 @@ impl HumanEmitter {
         Self {
             writer_type_id,
             real_writer: &mut *real_writer,
-            // NOTE: Intentionally erases the `+ Send` bound, see `writer` above.
             writer: AutoStream::new(real_writer, color),
             source_map: None,
             renderer: DEFAULT_RENDERER,

diff --git a/crates/sema/src/ast_lowering/lower.rs b/crates/sema/src/ast_lowering/lower.rs
@@ -65,45 +65,13 @@ impl<'ast> super::LoweringContext<'_, 'ast, '_> {
         let prev_contract_id = std::mem::replace(&mut self.current_contract_id, Some(id));
         debug_assert_eq!(prev_contract_id, None);
 
-        let mut ctor = None;
-        let mut fallback = None;
-        let mut receive = None;
         let mut items = SmallVec::<[_; 16]>::new();
         for item in contract.body.iter() {
             let id = match &item.kind {
                 ast::ItemKind::Pragma(_)
                 | ast::ItemKind::Import(_)
                 | ast::ItemKind::Contract(_) => unreachable!("illegal item in contract body"),
                 ast::ItemKind::Using(_) => continue,
-                ast::ItemKind::Function(func) => {
-                    let hir::ItemId::Function(id) = self.lower_item(item) else { unreachable!() };
-                    match func.kind {
-                        ast::FunctionKind::Constructor
-                        | ast::FunctionKind::Fallback
-                        | ast::FunctionKind::Receive => {
-                            let slot = match func.kind {
-                                ast::FunctionKind::Constructor => &mut ctor,
-                                ast::FunctionKind::Fallback => &mut fallback,
-                                ast::FunctionKind::Receive => &mut receive,
-                                _ => unreachable!(),
-                            };
-                            if let Some(prev) = *slot {
-                                let msg = format!("{} function already declared", func.kind);
-                                let note = "previous declaration here";
-                                let prev_span = self.hir.function(prev).span;
-                                self.dcx()
-                                    .err(msg)
-                                    .span(item.span)
-                                    .span_note(prev_span, note)
-                                    .emit();
-                            } else {
-                                *slot = Some(id);
-                            }
-                        }
-                        ast::FunctionKind::Function | ast::FunctionKind::Modifier => {}
-                    }
-                    hir::ItemId::Function(id)
-                }
                 ast::ItemKind::Variable(_) => {
                     let hir::ItemId::Variable(id) = self.lower_item(item) else { unreachable!() };
                     items.push(hir::ItemId::Variable(id));
@@ -112,19 +80,16 @@ impl<'ast> super::LoweringContext<'_, 'ast, '_> {
                     }
                     continue;
                 }
-                ast::ItemKind::Struct(_)
+                ast::ItemKind::Function(_)
+                | ast::ItemKind::Struct(_)
                 | ast::ItemKind::Enum(_)
                 | ast::ItemKind::Udvt(_)
                 | ast::ItemKind::Error(_)
                 | ast::ItemKind::Event(_) => self.lower_item(item),
             };
             items.push(id);
         }
-        let contract = &mut self.hir.contracts[id];
-        contract.ctor = ctor;
-        contract.fallback = fallback;
-        contract.receive = receive;
-        contract.items = self.arena.alloc_slice_copy(&items);
+        self.hir.contracts[id].items = self.arena.alloc_slice_copy(&items);
 
         self.current_contract_id = prev_contract_id;
 

diff --git a/crates/sema/src/ast_lowering/mod.rs b/crates/sema/src/ast_lowering/mod.rs
@@ -38,6 +38,7 @@ pub(crate) fn lower<'sess, 'hir>(
     lcx.collect_contract_declarations();
     lcx.resolve_base_contracts();
     lcx.linearize_contracts();
+    lcx.assign_constructors();
 
     // Resolve declarations and top-level symbols, and finish lowering to HIR.
     lcx.resolve_symbols();

diff --git a/crates/sema/src/ast_lowering/resolve.rs b/crates/sema/src/ast_lowering/resolve.rs
@@ -195,6 +195,45 @@ impl super::LoweringContext<'_, '_, '_> {
             self.hir.contracts[contract_id].bases = self.arena.alloc_slice_copy(&bases);
         }
     }
+
+    #[instrument(level = "debug", skip_all)]
+    pub(super) fn assign_constructors(&mut self) {
+        for contract_id in self.hir.contract_ids() {
+            let mut ctor = None;
+            let mut fallback = None;
+            let mut receive = None;
+
+            for &base_id in self.hir.contract(contract_id).linearized_bases {
+                for function_id in self.hir.contract(base_id).functions() {
+                    let func = self.hir.function(function_id);
+                    let slot = match func.kind {
+                        // Ignore inherited constructors.
+                        ast::FunctionKind::Constructor if base_id == contract_id => &mut ctor,
+                        ast::FunctionKind::Fallback => &mut fallback,
+                        ast::FunctionKind::Receive => &mut receive,
+                        _ => continue,
+                    };
+                    if let Some(prev) = *slot {
+                        // Don't report an error if the function is overridden.
+                        if base_id != contract_id {
+                            continue;
+                        }
+                        let msg = format!("{} function already declared", func.kind);
+                        let note = "previous declaration here";
+                        let prev_span = self.hir.function(prev).span;
+                        self.dcx().err(msg).span(func.span).span_note(prev_span, note).emit();
+                    } else {
+                        *slot = Some(function_id);
+                    }
+                }
+            }
+
+            let c = &mut self.hir.contracts[contract_id];
+            c.ctor = ctor;
+            c.fallback = fallback;
+            c.receive = receive;
+        }
+    }
 }
 
 impl super::LoweringContext<'_, '_, '_> {

diff --git a/tests/ui/abi/basic.stdout b/tests/ui/abi/basic.stdout
@@ -553,6 +553,10 @@
           ],
           "anonymous": false
         },
+        {
+          "type": "fallback",
+          "stateMutability": "nonpayable"
+        },
         {
           "type": "function",
           "name": "f1",
@@ -868,6 +872,10 @@
             }
           ],
           "stateMutability": "nonpayable"
+        },
+        {
+          "type": "receive",
+          "stateMutability": "payable"
         }
       ],
       "hashes": {

diff --git a/tests/ui/abi/contract_special_functions.sol b/tests/ui/abi/contract_special_functions.sol
@@ -0,0 +1,18 @@
+//@ignore-host: windows
+//@compile-flags: --emit=abi,hashes --pretty-json
+
+contract C {
+    constructor() {}
+    fallback() external {}
+}
+
+contract D is C {
+    constructor() payable {}
+    receive() external payable {}
+}
+
+// Inherits `C.fallback`, but not the constructor.
+contract E is C {}
+
+// Inherits `C.fallback` and `D.receive`, but not any of the constructors.
+contract F is D {}
diff --git a/tests/ui/abi/contract_special_functions.stdout b/tests/ui/abi/contract_special_functions.stdout
@@ -0,0 +1,59 @@
+{
+  "contracts": {
+    "ROOT/tests/ui/abi/contract_special_functions.sol:C": {
+      "abi": [
+        {
+          "type": "constructor",
+          "inputs": [],
+          "stateMutability": "nonpayable"
+        },
+        {
+          "type": "fallback",
+          "stateMutability": "nonpayable"
+        }
+      ],
+      "hashes": {}
+    },
+    "ROOT/tests/ui/abi/contract_special_functions.sol:D": {
+      "abi": [
+        {
+          "type": "constructor",
+          "inputs": [],
+          "stateMutability": "payable"
+        },
+        {
+          "type": "fallback",
+          "stateMutability": "nonpayable"
+        },
+        {
+          "type": "receive",
+          "stateMutability": "payable"
+        }
+      ],
+      "hashes": {}
+    },
+    "ROOT/tests/ui/abi/contract_special_functions.sol:E": {
+      "abi": [
+        {
+          "type": "fallback",
+          "stateMutability": "nonpayable"
+        }
+      ],
+      "hashes": {}
+    },
+    "ROOT/tests/ui/abi/contract_special_functions.sol:F": {
+      "abi": [
+        {
+          "type": "fallback",
+          "stateMutability": "nonpayable"
+        },
+        {
+          "type": "receive",
+          "stateMutability": "payable"
+        }
+      ],
+      "hashes": {}
+    }
+  },
+  "version": "VERSION"
+}
diff --git a/tests/ui/resolve/contract_special_functions.sol b/tests/ui/resolve/contract_special_functions.sol
@@ -0,0 +1,24 @@
+contract C {
+    constructor() {}
+    constructor() {} //~ ERROR: constructor function already declared
+
+    fallback() external {}
+    fallback() external {} //~ ERROR: fallback function already declared
+
+    receive() external payable {}
+    receive() external payable {} //~ ERROR: receive function already declared
+}
+
+contract D {
+    constructor() {}
+    constructor() {} //~ ERROR: constructor function already declared
+    constructor() {} //~ ERROR: constructor function already declared
+
+    fallback() external {}
+    fallback() external {} //~ ERROR: fallback function already declared
+    fallback() external {} //~ ERROR: fallback function already declared
+
+    receive() external payable {}
+    receive() external payable {} //~ ERROR: receive function already declared
+    receive() external payable {} //~ ERROR: receive function already declared
+}
diff --git a/tests/ui/resolve/contract_special_functions.stderr b/tests/ui/resolve/contract_special_functions.stderr
@@ -0,0 +1,86 @@
+error: constructor function already declared
+  --> ROOT/tests/ui/resolve/contract_special_functions.sol:LL:CC
+   |
+LL |     constructor() {}
+   |     ---------------- note: previous declaration here
+LL |     constructor() {}
+   |     ^^^^^^^^^^^^^^^^
+   |
+
+error: fallback function already declared
+  --> ROOT/tests/ui/resolve/contract_special_functions.sol:LL:CC
+   |
+LL |     fallback() external {}
+   |     ---------------------- note: previous declaration here
+LL |     fallback() external {}
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+   |
+
+error: receive function already declared
+  --> ROOT/tests/ui/resolve/contract_special_functions.sol:LL:CC
+   |
+LL |     receive() external payable {}
+   |     ----------------------------- note: previous declaration here
+LL |     receive() external payable {}
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+
+error: constructor function already declared
+  --> ROOT/tests/ui/resolve/contract_special_functions.sol:LL:CC
+   |
+LL |     constructor() {}
+   |     ---------------- note: previous declaration here
+LL |     constructor() {}
+   |     ^^^^^^^^^^^^^^^^
+   |
+
+error: constructor function already declared
+  --> ROOT/tests/ui/resolve/contract_special_functions.sol:LL:CC
+   |
+LL |     constructor() {}
+   |     ---------------- note: previous declaration here
+LL |     constructor() {}
+LL |     constructor() {}
+   |     ^^^^^^^^^^^^^^^^
+   |
+
+error: fallback function already declared
+  --> ROOT/tests/ui/resolve/contract_special_functions.sol:LL:CC
+   |
+LL |     fallback() external {}
+   |     ---------------------- note: previous declaration here
+LL |     fallback() external {}
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+   |
+
+error: fallback function already declared
+  --> ROOT/tests/ui/resolve/contract_special_functions.sol:LL:CC
+   |
+LL |     fallback() external {}
+   |     ---------------------- note: previous declaration here
+LL |     fallback() external {}
+LL |     fallback() external {}
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+   |
+
+error: receive function already declared
+  --> ROOT/tests/ui/resolve/contract_special_functions.sol:LL:CC
+   |
+LL |     receive() external payable {}
+   |     ----------------------------- note: previous declaration here
+LL |     receive() external payable {}
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+
+error: receive function already declared
+  --> ROOT/tests/ui/resolve/contract_special_functions.sol:LL:CC
+   |
+LL |     receive() external payable {}
+   |     ----------------------------- note: previous declaration here
+LL |     receive() external payable {}
+LL |     receive() external payable {}
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+
+error: aborting due to 9 previous errors
+