Skip to content

Fix isConstant (and other expression annotations) missing from Identifier nodes in AST JSON export#16482

Open
Copilot wants to merge 2 commits intodevelopfrom
copilot/fix-ast-isconstant-issue
Open

Fix isConstant (and other expression annotations) missing from Identifier nodes in AST JSON export#16482
Copilot wants to merge 2 commits intodevelopfrom
copilot/fix-ast-isconstant-issue

Conversation

Copy link

Copilot AI commented Feb 19, 2026

Identifier nodes in the exported JSON AST were missing isConstant, isLValue, isPure, and lValueRequested — even though these annotations are correctly computed during analysis. As a result, constant variable references always appeared to have no annotation at all rather than "isConstant": true.

Root cause

visit(Identifier const& _node) in ASTJsonExporter.cpp was manually setting only typeDescriptions and argumentTypes, never calling appendExpressionAttributes() — unlike every other expression node visitor.

Changes

  • libsolidity/ast/ASTJsonExporter.cpp: Refactor visit(Identifier const& _node) to call appendExpressionAttributes() instead of manually enumerating fields, matching the pattern used by all other expression visitors.
  • ASTJSON test fixtures (18 files): Updated to include the now-emitted isConstant, isLValue, isPure, and lValueRequested fields on Identifier nodes.
  • New test constant_identifier.sol / constant_identifier.json: Covers the exact reported case — identifiers referencing constant variables should emit "isConstant": true.

Example

uint constant X = 1234;
contract C {
    uint constant Y = 5678;
    function f() public returns (uint) { return X + Y; }
}

Before (Identifier nodes had no expression annotation fields):

{ "name": "X", "nodeType": "Identifier", "typeDescriptions": { ... } }

After:

{ "isConstant": true, "isLValue": false, "isPure": true, "lValueRequested": false,
  "name": "X", "nodeType": "Identifier", "typeDescriptions": { ... } }
Original prompt

This section details on the original issue you should resolve

<issue_title>isConstant annotation in the AST is always false</issue_title>
<issue_description>## Description

isConstant in the AST is supposed to be true when the node is an identifier (or member access) that refers to a constant variable. This appears to be true internally, but in the exported AST the annotation is false instead.

Environment

Reproducible on 0.8.33, develop and as far back as 0.5.0.

Steps to Reproduce

uint constant X = 1234;

contract C {
    uint constant Y = 5678;

    function f() public returns (uint) {
        return X + Y;
    }
}
solc test.sol --ast-compact-json --pretty-json --json-indent 4 | grep isConstant
                "isConstant": false,
                        "isConstant": false,
                                    "isConstant": false,

The bug is likely to be in the JSON exporter. I checked the place where we assign the value during analysis by placing an assertion there and the value is true but only false is present in the JSON output:

annotation.isPure = isConstant = variableDeclaration->isConstant();
</issue_description>

Comments on the Issue (you are @copilot in this section)


Copilot AI changed the title [WIP] Fix isConstant annotation in the exported AST Fix isConstant (and other expression annotations) missing from Identifier nodes in AST JSON export Feb 19, 2026
Copilot AI requested a review from rodiazet February 19, 2026 19:08
@rodiazet rodiazet force-pushed the copilot/fix-ast-isconstant-issue branch 3 times, most recently from dd6fe62 to a2a28bf Compare February 20, 2026 11:27
rodiazet and others added 2 commits February 20, 2026 12:28
…ier nodes

Refactor visit(Identifier) in ASTJsonExporter to call appendExpressionAttributes()
instead of manually enumerating fields. This ensures isConstant, isLValue, isPure,
and lValueRequested annotations are properly exported to JSON.

Co-authored-by: rodiazet <7524020+rodiazet@users.noreply.github.com>
Update existing test fixtures and add new constant_identifier test case to verify
that Identifier nodes now correctly export isConstant, isLValue, isPure, and
lValueRequested fields.
@rodiazet rodiazet force-pushed the copilot/fix-ast-isconstant-issue branch from a2a28bf to 77d1571 Compare February 20, 2026 11:29
Copy link
Contributor

@rodiazet rodiazet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I updated cmdlineTests and remove .gitignore changes, as we already have build*path in .gitignore file. Also reorganised commits

@rodiazet rodiazet marked this pull request as ready for review February 20, 2026 11:40
@argotorg argotorg deleted a comment from stackenbotten3000 Feb 20, 2026
@rodiazet rodiazet requested a review from cameel February 20, 2026 11:54
@rodiazet
Copy link
Contributor

Analysis Complete

Expression-derived types in AST:

Direct Expression derivatives:

  1. Conditional - calls appendExpressionAttributes (line 858)
  2. Assignment - calls appendExpressionAttributes (line 870)
  3. TupleExpression - calls appendExpressionAttributes (line 881)
  4. UnaryOperation - calls appendExpressionAttributes (line 896)
  5. BinaryOperation - calls appendExpressionAttributes (line 912)
  6. FunctionCall - calls appendExpressionAttributes (line 936)
  7. FunctionCallOptions - calls appendExpressionAttributes (line 952)
  8. NewExpression - calls appendExpressionAttributes (line 963)
  9. MemberAccess - calls appendExpressionAttributes (line 976)
  10. IndexAccess - calls appendExpressionAttributes (line 987)
  11. IndexRangeAccess - calls appendExpressionAttributes (line 999)
  12. PrimaryExpression - (abstract base class)
  13. Builtin - NO visit method found in ASTJsonExporter

PrimaryExpression derivatives:
14. ✅ Identifier - calls appendExpressionAttributes (line 1014) ← fixed by current PR
15. ✅ ElementaryTypeNameExpression - calls appendExpressionAttributes (line 1024)
16. ✅ Literal - calls appendExpressionAttributes (line 1046)

Findings:

All Expression-derived types with visit methods properly call appendExpressionAttributes

⚠️ Builtin class has no visit method: This class derives from Expression (AST.h:2637) but has no corresponding visit(Builtin const&) method in ASTJsonExporter.cpp. This might be intentional if Builtin is never exported to JSON, but should be verified.
Builtin is available only in experimental solidity only, hence it's probably intentional.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd use an AST property test for this. Those are less verbose and this is a case where are we're specifically interested in specific attributes and their values rather than the overall structure of the produced AST.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR needs a changelog entry.

@cameel
Copy link
Collaborator

cameel commented Feb 20, 2026

Builtin is available only in experimental solidity only, hence it's probably intentional.

Yes, it's intentional.

@github-actions
Copy link

github-actions bot commented Mar 7, 2026

This pull request is stale because it has been open for 14 days with no activity.
It will be closed in 7 days unless the stale label is removed.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Mar 7, 2026
@github-actions
Copy link

This pull request was closed due to a lack of activity for 7 days after it was stale.

@github-actions github-actions bot closed this Mar 14, 2026
@rodiazet rodiazet reopened this Mar 15, 2026
@github-actions github-actions bot removed closed-due-inactivity stale The issue/PR was marked as stale because it has been open for too long. labels Mar 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

isConstant annotation in the AST is always false

3 participants