Skip to content

[v2] False IdentifierRedeclaration for symbols imported via two routes, and for overloads imported from different files #2003

Description

@hedgar2017

IdentifierRedeclaration is reported for two import patterns that Solidity allows: importing the same declaration through two routes, and importing overloads of one name from different files.

Reproduction

A — the same free function reached by two import routes

// a.sol
function f() pure {}
// b.sol
import "a.sol";
import {f} from "a.sol";

contract C {
    function g() public pure { f(); }
}

b.sol reports Resolution(IdentifierRedeclaration) on f. Both names denote the same declaration, so there is nothing to disambiguate.

B — overloads of one name imported from different files

// x.sol
function f(uint256 a) pure returns (uint256) { return a; }
// y.sol
function f(string memory s) pure returns (bytes memory) { return bytes(s); }
// z.sol
import {f} from "x.sol";
import {f} from "y.sol";

contract C {
    function g() public pure { f(1); }
}

z.sol reports Resolution(IdentifierRedeclaration) on f. The two targets have distinct signatures and should overload.

Both reproduce on main at 61d67fc, at LanguageVersion::LATEST / EvmTarget::LATEST.

Cause

conflicting_definition in crates/solidity-v2/outputs/cargo/semantic/src/passes/common/conflicts.rs is a pairwise comparison that delegates to Definition::overloads_with:

let existing = binder.find_definition_by_id(existing_id)?;
if new_definition.overloads_with(existing) {
    None
} else {
    Some(existing_id)
}

Both operands here are Definition::ImportedSymbol. The comparison never follows the alias to what was imported, so it can see neither that the two sides resolve to the same target (A) nor that they resolve to distinct overloadable signatures (B).

Notes for whoever picks this up

  • The fix belongs behind the alias: resolve each ImportedSymbol to its target set before comparing, then apply the existing overloads_with rule to the resolved definitions. Short-circuiting on "both sides are functions, therefore compatible" is not sufficient — it would also accept two imported functions with identical signatures, which is a real redeclaration.
  • re-use default_import_closure for identifier redefinition checking #1956 proposes re-using default_import_closure for identifier redefinition checking in this same area, and may be the natural implementation vehicle.
  • No snapshot case currently covers either pattern. Both belong in crates/solidity-v2/testing/snapshots/diagnostics_output/resolution/identifier_redeclaration/. Adding them there also records solc's verdict alongside slang's, which is the cleanest way to confirm the expected behavior rather than relying on a reading of the spec.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions