fix(semantic): Address remaining cases of undeclared identifier diagnostics by ggiraldez · Pull Request #2300 · NomicFoundation/slang

ggiraldez · 2026-10-20T00:02:20Z

Closes most of the gaps left for NomicFoundation/slang-diagnostics-research#1473, NomicFoundation/slang-diagnostics-research#1686, NomicFoundation/slang-diagnostics-research#1750 and NomicFoundation/slang-diagnostics-research#1751, found by running each issue's listed solc tests through slang. The issues can be closed after merging both this PR and #2326.

Closes #2338

A library's external functions are no longer visible by bare name inside the library. super skips unimplemented functions, so it reaches the next implemented base or reports the member as missing. Through the name of a contract or interface the code doesn't derive from, only its types, errors and externally visible functions are reachable (and events from 0.8.21). This applies to member access, emit paths and storage layout expressions, and emit/revert targets are now resolved in p5 to keep that logic in one pass. A function attached with using only as an operator is no longer callable as a member. Binding a receiver to an attached function now checks data locations: by the internal or ABI rules depending on the function's visibility, and a storage parameter always needs a storage receiver.

There's a remaining case for .selector on a ternary of library functions, eg. (cond ? L.f : L.f).selector, which needs a separate function-type redesign.

changeset-bot · 2026-10-20T00:02:25Z

⚠️ No Changeset found

Latest commit: 476b5cf

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

github-actions · 2026-10-20T00:16:01Z

Bencher Report

Branch	ggiraldez/sdr-resolution-gaps
Testbed	ci

⚠️ WARNING: Truncated view!
The full continuous benchmarking report exceeds the maximum length allowed on this platform.

🚨 3 Alerts

🐰 View full continuous benchmarking report in Bencher

ggiraldez · 2026-10-29T22:24:35Z

Flipping back to draft. I found additional cases of the mentioned SDR issues that make sense to fix here.

teofr

Did a first pass, will continue later

teofr · 2026-11-02T13:18:52Z

+    │        ╰───── ref: 2
+    │           │  
+    │           ╰── ref: 4
+ 12 │     a + a;


Should we add a ref: entry to the +? And other operators?

Operators don't generate References so they cannot appear in binder snapshots.

Makes sense, not for this PR, but I wonder if we should? In situations like this one the operator works almost like any othr function call, and refers to the add method, rather than a builtin.

We can't do that without a sizeable refactor. References can currently only point to Identifiers. What we can do is use the AST's resolve_operator_to_function and only report the resolved operator in the binder report. That will still require some work, but it's doable.

teofr · 2026-11-02T14:05:30Z

                    ir::StateVariableVisibility::Public
                )
            }
            _ => true,


I was randomly asking questions to claude about the pr, and it brought up that this may be wrong (I know, not part of this PR, but I think is relevant to these diagnostics). It suggests changing this to _ => false, this doesn't play nicely with other parts of our codebase, but may be worth a check (even in another PR).

This test fails in solc with error 9582 but it's accepted on slang

// SPDX-License-Identifier: MIT pragma solidity *; contract C { struct S { uint256 a; } function g() public view { this.S; } }

That's probably true for p5 (ie. in an expression resolution context), but this function is also used for general qualified scope lookup (eg. for types/events/errors/etc) and in that case, it's not cut and dried. In fact, your previous question about event visibility from 0.8.21 onwards is one such case. I'll add it to my TODO list for later.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

teofr

Looks good, just some questions and nits, but up to you

teofr · 2026-11-03T09:13:58Z

+    /// Whether a member of a contract or interface can be reached through its
+    /// name from outside the contract's hierarchy: types, errors and
+    /// externally visible functions can, events only from 0.8.21, and
+    /// variables and internal functions never.


Suggested change

/// variables and internal functions never.

/// variables, modifiers, and internal functions never.

teofr · 2026-11-03T09:33:19Z

+        } else if let Some(member) = self.unreachable_qualified_member(identifier_path) {
+            self.binder
+                .fixup_reference(member.id(), Resolution::Unresolved);
+            let kind = namespace_member_not_found(identifier_path, identifier_path.len() - 1);
+            self.push_diagnostic(member, kind);


I'm not the biggest fan of this fixup, I wonder if we could make resolve_identifier_path_as_expression_in_scope do all the work. However, I don't have a stron idea on how to approach it, so I'd leave it for now. If you have any ideas or a strong reason why this should behave this way I'm interested in hearing it.

Agreed it's not ideal. I've been thinking that we've outgrown the resolution capabilities of the binder and we're spilling binding policy all over the passes. I would defer it to another PR though.

ggiraldez added this pull request to stack #2287 October 20, 2026 00:02

ggiraldez marked this pull request as ready for review October 20, 2026 00:06

ggiraldez requested review from a team as code owners October 20, 2026 00:06

ggiraldez force-pushed the ggiraldez/sdr-resolution-gaps branch 2 times, most recently from 8cef9da to 6174cb8 Compare October 27, 2026 23:56

ggiraldez force-pushed the ggiraldez/sdr-resolution-gaps branch from 6174cb8 to 40c10d5 Compare October 28, 2026 00:57

ggiraldez force-pushed the ggiraldez/sdr-resolution-gaps branch from 40c10d5 to c4a390b Compare October 28, 2026 22:46

ggiraldez force-pushed the ggiraldez/sdr-resolution-gaps branch from c4a390b to 3827d9c Compare October 29, 2026 17:14

ggiraldez marked this pull request as draft October 29, 2026 22:23

ggiraldez mentioned this pull request Oct 30, 2026

feat(diagnostics): Report undeclared identifiers in a type position #2288

Open

ggiraldez force-pushed the ggiraldez/sdr-resolution-gaps branch from 3827d9c to e235a85 Compare October 30, 2026 21:53

ggiraldez mentioned this pull request Oct 30, 2026

refactor(semantic): Replace FunctionType visibility with a CallMode #2326

Draft

ggiraldez marked this pull request as ready for review October 30, 2026 21:56

teofr reviewed Nov 2, 2026

View reviewed changes

ggiraldez force-pushed the ggiraldez/sdr-resolution-gaps branch from e235a85 to c8d44d2 Compare November 2, 2026 21:10

ggiraldez added 5 commits November 2, 2026 22:22

External library functions are not visible by name

370ef0e

Skip unimplemented functions when resolving super

bfdef47

Hide unreachable members of foreign contracts/interfaces

3039cba

Attached functions as operators are not resolvable by name

5ae44cb

Check receiver locations for attached functions

3630051

ggiraldez and others added 5 commits November 2, 2026 22:24

perf(semantic): Skip visibility filtering when all members are visible

b943e3c

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Avoid leaking private members through member access expressions

4317424

String values have no length member

b8ebb65

Also filter out modifiers when resolving member accesses

0f30ab7

Report unreachable emit/revert members as namespace members not found

3519d47

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ggiraldez force-pushed the ggiraldez/sdr-resolution-gaps branch from c8d44d2 to 375332b Compare November 2, 2026 23:10

teofr approved these changes Nov 3, 2026

View reviewed changes

Some suggestions from code review

476b5cf

ggiraldez force-pushed the ggiraldez/sdr-resolution-gaps branch from 375332b to 476b5cf Compare November 3, 2026 22:38

	/// variables and internal functions never.
	/// variables, modifiers, and internal functions never.

Conversation

ggiraldez commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot Bot commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

github-actions Bot commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ WARNING: Truncated view!

🚨 3 Alerts

Uh oh!

ggiraldez commented Oct 29, 2026

Uh oh!

teofr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

teofr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ggiraldez commented Oct 20, 2026 •

edited

Loading

changeset-bot Bot commented Oct 20, 2026 •

edited

Loading

github-actions Bot commented Oct 20, 2026 •

edited

Loading