Skip to content

Commit

Permalink
Fix incorrect block scoping rules for legacy code (#165)
Browse files Browse the repository at this point in the history
What title said. Thanks @d1ll0n
  • Loading branch information
d1ll0n authored Nov 9, 2022
1 parent 745e34c commit c4feb1a
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 13 deletions.
39 changes: 26 additions & 13 deletions src/ast/definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -289,16 +289,28 @@ function* lookupInFunctionDefinition(
/**
* Lookup the definition corresponding to `name` in the `Block|UncheckedBlock` `scope`. Yield all matches.
*/
function* lookupInBlock(name: string, scope: Block | UncheckedBlock): Iterable<AnyResolvable> {
for (const node of scope.children) {
if (!(node instanceof VariableDeclarationStatement)) {
continue;
}

for (const decl of node.vDeclarations) {
if (decl.name === name) {
yield decl;
}
function* lookupInBlock(
name: string,
scope: Block | UncheckedBlock,
version: string
): Iterable<AnyResolvable> {
let declarations: VariableDeclaration[];
if (version && lt(version, "0.5.0")) {
declarations = scope.getChildrenByType(VariableDeclaration);
} else {
declarations = scope.children
.filter((node) => node instanceof VariableDeclarationStatement)
.reduce(
(declarations: VariableDeclaration[], statement) => [
...declarations,
...(statement as VariableDeclarationStatement).vDeclarations
],
[]
);
}
for (const declaration of declarations) {
if (declaration.name === name) {
yield declaration;
}
}
}
Expand All @@ -313,7 +325,8 @@ function lookupInScope(
name: string,
scope: ScopeNode,
visitedUnits = new Set<SourceUnit>(),
ignoreVisiblity: boolean
ignoreVisiblity: boolean,
version = ""
): Set<AnyResolvable> {
let results: Iterable<AnyResolvable>;

Expand All @@ -328,7 +341,7 @@ function lookupInScope(
} else if (scope instanceof VariableDeclarationStatement) {
results = scope.vDeclarations.filter((decl) => decl.name === name);
} else if (scope instanceof Block || scope instanceof UncheckedBlock) {
results = lookupInBlock(name, scope);
results = lookupInBlock(name, scope, version);
} else if (scope instanceof TryCatchClause) {
results = scope.vParameters
? scope.vParameters.vParameters.filter((param) => param.name === name)
Expand Down Expand Up @@ -376,7 +389,7 @@ export function resolveAny(
// If this is the first element (e.g. `A` in `A.B.C`), walk up the
// stack of scopes starting from the current context, looking for `A`
while (scope !== undefined) {
res = lookupInScope(element, scope, undefined, ignoreVisiblity);
res = lookupInScope(element, scope, undefined, ignoreVisiblity, version);

if (res.size > 0) {
// Sanity check - when multiple results are found, they must either be overloaded events
Expand Down
22 changes: 22 additions & 0 deletions test/samples/solidity/resolving/block_04.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,27 @@ contract Foo {
foo memory m = foo(bar);

uint bar = m.x;

uint m1 = k;
{uint k;}
}

uint256 x1;
uint256 x2;
uint256 x3;
uint256 x4;

function second() internal {
{
uint256 x1 = 1;
if (x1 > 0) {
uint256 x2 = 2;
}
{
uint256 x3 = 3;
}
}
for (uint256 x4; x4 < 10; x4++) {}
uint256 y = x1 + x2 + x3 + x4;
}
}
19 changes: 19 additions & 0 deletions test/samples/solidity/resolving/block_05.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,23 @@ contract Foo {

foo = 2;
}

uint256 x1;
uint256 x2;
uint256 x3;
uint256 x4;

function second() internal {
{
uint256 x1 = 1;
if (x1 > 0) {
uint256 x2 = 2;
}
{
uint256 x3 = 3;
}
}
for (uint256 x4; x4 < 10; x4++) {}
uint256 y = x1 + x2 + x3 + x4;
}
}

0 comments on commit c4feb1a

Please sign in to comment.