no limitations

[navicore]

No description provided.

[claude]

PR Review: Remove "Current Limitations" Section

Summary

This PR removes the "Current Limitations" section from docs/TYPE_SYSTEM_GUIDE.md. The section removal is partially correct but reveals a documentation accuracy issue.

---

✅ Correct Removal: ADT Syntax Limitation

The first limitation about ADTs is outdated and correct to remove:

The removed text claimed:

Seq has runtime variants (tagged unions) but not compile-time algebraic data type declarations

This is FALSE. Seq DOES support compile-time ADT syntax via union declarations:

union Option {
  Some { value: Int }
  None
}

union Message {
  Get { response-chan: Int }
  Increment { response-chan: Int }
  Report { op: Int, delta: Int, total: Int }
}

Evidence:

• docs/design/ADT_DESIGN.md documents the union type system
• 14+ .seq files use union declarations (examples/csp/actor_counters.seq, examples/lisp/parser.seq, tests/integration/src/test-union-same-type-fields.seq, etc.)
• Compiler generates typed constructors with full compile-time type checking
• Pattern matching with exhaustiveness checking is supported

---

⚠️ Problematic Removal: Stack Effects Requirement

The second limitation about stack effects is STILL TRUE and should not have been removed without clarification:

The removed text stated:

All words must have explicit stack effect declarations. The compiler verifies the body matches the declaration.

This remains accurate. Stack effects are still mandatory in Seq v0.14+.

Evidence from compiler source (crates/compiler/src/typechecker.rs:317-329):

// Second pass: collect all word signatures
// All words must have explicit stack effect declarations (v2.0 requirement)
for word in &program.words {
    if let Some(effect) = &word.effect {
        self.env.insert(word.name.clone(), effect.clone());
    } else {
        return Err(format!(
            "Word '{}' is missing a stack effect declaration.\n\
             All words must declare their stack effect, e.g.: : {} ( -- ) ... ;",
            word.name, word.name
        ));
    }
}

Documentation inconsistency:

• docs/language-guide.md:100 states: "Stack effects are required on all word definitions"
• docs/TYPE_SYSTEM_GUIDE.md:426-438 says "Even though the checker can infer types, always declare effects" but shows a discouraged example without stack effects
• The "discouraged" example would actually fail compilation with an error

---

📋 Recommendations

1. Keep ADT removal ✅

The ADT limitation is outdated and correct to remove.

2. Clarify stack effects requirement ⚠️

Option A: Add a note explaining that while the documentation previously called this a "limitation," it's actually a design feature for clarity and type safety:

## Design Philosophy: Explicit Stack Effects

All words require explicit stack effect declarations:

\`\`\`seq
: double ( Int -- Int )
  2 i.* ;
\`\`\`

This is a deliberate design choice (not a limitation):
- Ensures clarity: every word's interface is explicit
- Enables better error messages: type errors reference your declared intent
- Improves tooling: LSP can provide completions without analyzing bodies
- Catches bugs early: mismatches between declaration and implementation are caught at compile time

Option B: Update docs/TYPE_SYSTEM_GUIDE.md best practices section to clarify that stack effects are required, not just recommended.

3. Fix documentation inconsistencies

The TYPE_SYSTEM_GUIDE.md still has a "Best Practices" section (lines 426-438) with a misleading "discouraged" example:

# Discouraged - unclear what it does
: double
  2 multiply ;

This should be labeled as "Error - compilation fails" not "discouraged", since the compiler will reject this code.

---

🔍 Security & Performance

• No security concerns: Documentation-only change
• No performance impact: Documentation-only change
• No breaking changes: No code changes

---

🧪 Test Coverage

Not applicable - this is a documentation change with no code modifications.

However, consider adding a negative test case to ensure the compiler continues to reject words without stack effects:

# Should fail to compile
: no-effect-word
  42 ;

---

📝 Code Quality

Documentation quality: The removal improves accuracy by eliminating the outdated ADT limitation, but creates a gap by removing information about stack effects that's still partially true.

Suggested commit message refinement:

Current: "no limitations"

Better: "Remove outdated ADT limitation from TYPE_SYSTEM_GUIDE"

This more accurately describes what was removed and why.

---

✅ Verdict

Approve with suggestions. The PR correctly removes outdated information about ADTs, but the stack effects portion should be handled more carefully through documentation updates rather than silent removal.

The title "no limitations" is aspirational and perhaps premature - Seq still has the intentional constraint that stack effects must be declared explicitly. This is a feature, not a bug, but it's worth documenting the philosophy behind it.