Skip to content

Conversation

Copy link

Copilot AI commented Nov 10, 2025

Addresses 44 review comments on PR #20, focusing on P0/P1 bugs and C++ modernization.

Critical Fixes (P0)

ContextParser compilation failures:

  • Added missing <memory> include for std::unique_ptr
  • Fixed IState.hpp include path (states/base/IState.hpp)
  • Fixed TopNodeAs() to cast NodeEntry::MutableNode() instead of NodeEntry&
// Before: doesn't compile
return dynamic_cast<T*>(node_stack_.back());

// After: properly casts the AstNode pointer
return dynamic_cast<T*>(node_stack_.back().MutableNode());

Missing Accept() implementations:

  • Implemented visitor dispatch for all 40+ AST node types

Type Safety & Modern C++

Primitive types aligned with VM:

  • CharLit: char32_tchar (UTF-8 encoding)
  • FloatLit: long doubledouble (64-bit consistency)
  • IntLit: long longint64_t (explicit width)
  • StringLit: std::u32stringstd::string

Eliminated raw pointers:

  • Operator resolvers: const T*std::optional<std::reference_wrapper<const T>>
  • Diagnostic severity: raw pointer → std::shared_ptr<const ISeverity>
  • DiagnosticCollector limits: 0 = unlimitedstd::optional<std::size_t>

API improvements:

  • Operator tag Name(): const char*std::string_view
  • OpTags: static class → namespace
  • ReturnStmt::value_: nullable pointer → std::optional<std::unique_ptr<Expr>>

Structural Improvements

  • Added AstNodeDerived concept for TopNodeAs<T>() template constraint
  • Enhanced Diagnostic with proper methods, copy/move semantics, and [[nodiscard]]
  • Restructured IfStmt/Branch to support else-if chains cleanly
  • Specialized InterfaceDecl::members_ to std::vector<std::unique_ptr<InterfaceMethod>>
  • Added namespace closures to all parser files with updated header guards

Not Addressed

Deferred design questions requiring broader discussion:

  • Renaming StmtStatement (230+ files)
  • Specializing Call::callee_ to hypothetical CallableExpr base
  • Making Unary::op_ a const reference (constructor impact)

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI mentioned this pull request Nov 10, 2025
Copilot AI changed the title [WIP] Address feedback from review on Parser PR Address code review feedback: fix critical bugs and modernize C++ idioms Nov 10, 2025
Copilot AI requested a review from bialger November 10, 2025 23:46
Copilot finished work on behalf of bialger November 10, 2025 23:46
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.

2 participants