Skip to content

Conversation

@wujingyue
Copy link
Collaborator

It was accidentally moved over by #5605.

@wujingyue
Copy link
Collaborator Author

!test

@wujingyue wujingyue requested a review from naoyam January 7, 2026 23:47
@github-actions
Copy link

github-actions bot commented Jan 7, 2026

Auto-merge Status

✅ Internal CI is finished
✅ No failed checks
✅ PR is mergeable
ℹ️ PR mergeable_state: unstable

Description

  • Move Scope class from composite_nodes to internal_nodes

  • Relocate all Scope method implementations from composite_nodes.cpp to internal_nodes.cpp

  • Update header files to reflect the class location change

  • Fix accidental placement from PR Split composite IR nodes into dedicated files #5605

Changes walkthrough

Relevant files
Enhancement
composite_nodes.cpp
Remove Scope methods from composite_nodes.cpp                       

csrc/ir/composite_nodes.cpp

  • Remove Scope class method implementations (toString, insert,
    insert_before, insert_after, erase, contains, clear)
  • Delete approximately 66 lines of Scope-related code
  • +0/-61   
    internal_nodes.cpp
    Add Scope methods to internal_nodes.cpp                                   

    csrc/ir/internal_nodes.cpp

  • Add Scope class method implementations (toString, insert,
    insert_before, insert_after, erase, contains, clear)
  • Insert approximately 66 lines of Scope-related code
  • +63/-0   
    composite_nodes.h
    Remove Scope class from composite_nodes.h                               

    csrc/ir/composite_nodes.h

  • Remove entire Scope class definition (77 lines)
  • Remove unnecessary #include directive
  • +0/-69   
    internal_nodes.h
    Add Scope class to internal_nodes.h                                           

    csrc/ir/internal_nodes.h

  • Add complete Scope class definition with all methods and members
  • Include forward declaration of Scope class
  • Add approximately 58 lines of Scope class code
  • +67/-1   

    PR Reviewer Guide

    Here are some key observations to aid the review process:

    🧪 No relevant tests
    ⚡ No major issues detected

    @greptile-apps
    Copy link
    Contributor

    greptile-apps bot commented Jan 7, 2026

    Greptile Summary

    This PR correctly moves the Scope class from composite_nodes to internal_nodes, reversing an accidental move in PR #5605. The change is a straightforward refactoring that relocates the class definition and implementation to its proper home alongside other internal IR nodes like IfThenElse.

    Key changes:

    • Moved Scope class definition from composite_nodes.h to internal_nodes.h
    • Moved all Scope method implementations from composite_nodes.cpp to internal_nodes.cpp
    • Removed unnecessary #include <list> from composite_nodes.h (already present in internal_nodes.h)
    • No logical changes to the class itself; this is a pure relocation

    Confidence Score: 5/5

    • This PR is safe to merge with no concerns. It's a clean refactoring that correctly relocates misplaced code.
    • Score of 5 reflects that this is a straightforward, well-executed code move with no functional changes. The relocation of Scope from composite_nodes to internal_nodes is logically correct based on the class's purpose and usage. All method implementations are identical to the originals, and the include structure is clean with no circular dependencies or missing headers.
    • No files require special attention

    Important Files Changed

    Filename Overview
    csrc/ir/composite_nodes.h Removed Scope class definition and <list> include header. This is correct since Scope is not used in composite nodes and belongs in internal_nodes.h.
    csrc/ir/composite_nodes.cpp Removed all Scope method implementations. The move is clean and complete with no remaining references.
    csrc/ir/internal_nodes.h Added complete Scope class definition with proper includes. The class definition is logically correct with all necessary methods and member variables.
    csrc/ir/internal_nodes.cpp Added all Scope method implementations moved from composite_nodes.cpp. Implementations are unchanged and correct.

    Sequence Diagram

    sequenceDiagram
        participant Before as Before PR
        participant After as After PR
        
        Before->>composite_nodes.h: Contains Scope class definition
        Before->>composite_nodes.cpp: Contains Scope implementations
        Before->>internal_nodes.h: Forward declares Scope (in base_nodes)
        
        After->>composite_nodes.h: Scope class removed
        After->>composite_nodes.cpp: Scope implementations removed
        After->>internal_nodes.h: Scope class definition added
        After->>internal_nodes.cpp: Scope implementations added
        
        Note over After: Scope now properly belongs in internal_nodes<br/>where IfThenElse and similar classes live
    
    Loading

    @wujingyue wujingyue requested a review from zasdfgbnm January 8, 2026 19:53
    @wujingyue wujingyue added the enable-auto-merge Auto-merge a PR when: 1) PR mergeable 2) Internal CI complete 3) No failures label Jan 8, 2026
    Copy link
    Collaborator

    @naoyam naoyam left a comment

    Choose a reason for hiding this comment

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

    LGTM

    @github-actions github-actions bot merged commit cf170f4 into main Jan 9, 2026
    63 checks passed
    @github-actions github-actions bot removed the enable-auto-merge Auto-merge a PR when: 1) PR mergeable 2) Internal CI complete 3) No failures label Jan 9, 2026
    @github-actions github-actions bot deleted the wjy/scope branch January 9, 2026 00:54
    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.

    3 participants