argotorg / argotorg/solidity

Only the first duplicate-definition group is reported per name: `reported.count(i)` is in the loop condition instead of the body

Open
#16,945 2 comments 0 reactions 1 assignee Claimed by @msooseth View on GitHub
bug :bug: low impact
Dominant language
C++
Stars
25.7k
Forks
6.2k
Avg merge
1d 11h
Merged PRs (30d)
21

Description

## Description

When one name has more than one group of duplicate definitions, only the first group is reported.
The remaining duplicates are silently dropped from the diagnostics.

`libsolidity/analysis/ContractLevelChecker.cpp:239`, in `findDuplicateDefinitions`:

```cpp
for (size_t i = 0; i < overloads.size() && !reported.count(i); ++i)
```

`reported` holds the indices already covered by an earlier error (populated at `:260`). The test
for it sits in the **loop condition**, so reaching an already-reported index terminates the entire
loop rather than skipping that one index and continuing.

The bookkeeping itself suggests this isn't intended: if the goal were "stop after the first
report", a `bool` would be enough. Maintaining a `std::set` of indices only makes sense if
those indices are meant to be skipped when they come round as `i` — i.e. this looks like a
`continue` written as a loop exit.

**Expected:** one error per duplicate group.

**Actual:** one error total, per name.

This is a diagnostics-completeness issue rather than a soundness one, and I'd suggest triaging it
as such. Index 0 can never be in `reported`, so the first group always reports and a contract
containing duplicates always fails to compile — nothing invalid is accepted. The cost is that a
developer fixes one duplicate, recompiles, and discovers the next.

## Environment

- Compiler version: 0.8.36+commit.8a079791 (source byte-identical on current `develop`; the line
dates to `d054a3b85`, 2018-11-29, which moved it in already written this way)
- Compilation pipeline (legacy, IR, SSA CFG): all — this is in the analysis phase, before codegen
- Target EVM version (as per compiler settings): default (`osaka`); not EVM-version dependent
- Framework/IDE (e.g. Foundry, Hardhat, Remix): `solc` command line directly
- EVM execution environment / backend / blockchain client: n/a — compile-time only
- Operating system: macOS 26.5.2

## Steps to Reproduce

`poc.sol` — one name, two independent duplicate groups:

```solidity
// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8.0;

contract C {
function f(uint) public {}
function f(uint) public {}
function f(string memory) public {}
function f(string memory) public {}
}
```

```
solc poc.sol
```

**Expected:** two errors — `f(uint)` is defined twice and `f(string)` is defined twice, at separate
source locations.

**Actual:** one error.

```
Error: Function with same name and parameter types defined twice.
```

**Control** — identical duplicate structure under two different names reports both, because each
name gets its own `overloads` vector:

```solidity
contract C {
function f(uint) public {}
function f(uint) public {}
function g(string memory) public {}
function g(string memory) public {}
}
```

```
Error: Function with same name and parameter types defined twice.
Error: Function with same name and parameter types defined twice.
```

Extent:

| source | errors emitted | expected |
|---|---|---|
| one name, 2 duplicate groups | 1 | 2 |
| one name, 3 duplicate groups | 1 | 3 |
| two names, 1 group each (control) | 2 | 2 |
| one name, 1 group (control) | 1 | 1 |
| events, one name, 2 groups | 1 | 2 |

The last row reaches `EventDefinition` (error 5883), since `findDuplicateDefinitions` is a template
used by both `checkDuplicateFunctions` (`:188`) and `checkDuplicateEvents` (`:200`).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.