# Regjit Code Review > Use when reviewing RegJIT code changes for quality, correctness, and PCRE/RE2 compatibility - Skill: `tomevault-io/regjit-code-review` (Agent Skill, multi-file: 2 files) - Install (CLI): `npx skillmds@latest add tomevault-io/regjit-code-review` - Raw SKILL.md: https://api.skillmd.com/api/skills/tomevault-io/regjit-code-review/raw - Safety review: pending (external: skill-scanner PASS, skillspector PASS) - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: Coding & Dev Tools - Author: tomevault-io (https://skillmd.com/u/tomevault-io) - Updated: 2026-09-17 - Page: https://skillmd.com/skills/tomevault-io/regjit-code-review --- # RegJIT Code Review ## Overview Systematically review code changes in RegJIT for quality, correctness, and adherence to regex engine compatibility standards (PCRE, std::regex, RE2). ## When to Use - After writing significant code changes - Before committing new features or fixes - When reviewing anchor/quantifier behavior changes - When modifying LLVM IR generation - Before merging branches ## Review Checklist ### 1. Scope & Context (5 min) - [ ] Identify all modified files - [ ] Understand change purpose (feature/fix/refactor) - [ ] Check alignment with commit message - [ ] Verify no unintended files modified ### 2. Functionality (15 min) - [ ] Logic is correct and matches intent - [ ] Edge cases handled (empty input, single char, boundaries) - [ ] Search loop working correctly for anchors with quantifiers - [ ] PCRE/RE2 behavior verified **Key checks for RegJIT:** - Anchor semantics: `^` and `$` match at correct positions only - Zero-width assertions: `^`, `$`, `\b`, `\B` behave as zero-width - Quantifier precedence: applied to correct elements - Search loop: attempts match at every offset (0 to strlen inclusive) ### 3. Code Quality (10 min) - [ ] Consistent style (naming, indentation) - [ ] Comments explain "why" not just "what" - [ ] No magic numbers without explanation - [ ] LLVM IR generation follows patterns **RegJIT specific:** - BasicBlock naming is descriptive - PHI node usage correct for loops - No dead code blocks - SSA form maintained ### 4. Architecture (10 min) - [ ] Follows AST node pattern - [ ] Proper separation between Lexer/Parser/CodeGen - [ ] LLVM integration correct - [ ] No circular dependencies ### 5. Testing (5 min) - [ ] Tests added for new functionality - [ ] Existing tests still pass - [ ] Edge cases covered - [ ] PCRE/RE2 compatibility verified ### 6. Documentation (5 min) - [ ] Code comments clear and accurate - [ ] AGENTS.md updated if adding features - [ ] Complex algorithms explained - [ ] Search loop note present if modifying Func::CodeGen() ## Common RegJIT Issues to Watch ### LLVM-Specific - [ ] Using wrong BasicBlock for branching - [ ] All code paths lead to return statement - [ ] Memory leaks in LLVM objects (use smart pointers) - [ ] SSA form violations (redefining values) ### Anchor/Quantifier - [ ] Search loop preserved (critical for compatibility) - [ ] Zero-width detection working (isZeroWidth()) - [ ] Anchor attempts at correct offsets - [ ] Quantifier properly applied to expressions ### C++ Quality - [ ] No use-after-free bugs - [ ] Proper RAII usage - [ ] Smart pointers for memory safety - [ ] Const correctness maintained ## Code Review Process 1. **Read the diff** - understand changes at high level 2. **Check logic** - trace through critical paths 3. **Verify tests** - ensure coverage is adequate 4. **Look for issues** - use common issues checklist 5. **Document findings** - note specific file:line references 6. **Provide feedback** - be specific and actionable ## Example Review ``` ## Code Review: anchor quantifier search loop ### Files ✅ src/regjit.cpp - Core matcher with search loop ✅ tests/test_anchor_quant_edge.cpp - Comprehensive test coverage ✅ AGENTS.md - Documentation updated ✅ Makefile - Test target added ### Functionality ✅ Search loop attempts match at every offset ✅ Zero-width anchor semantics preserved ✅ PCRE/RE2 compatible for ^*, $+, \b*, etc ✅ Edge cases handled (empty string, end of string) ### Code Quality ✅ Variable names descriptive (LoopCheckBB, TrySuccess) ✅ Comments explain search loop purpose ✅ IR structure clean and correct ✅ No compiler warnings ### Architecture ✅ Follows existing patterns ✅ LLVM integration proper ✅ No regressions in other features ### Testing ✅ 15 edge case tests added ✅ All existing tests pass ✅ Verified against PCRE behavior ### Result ✅ APPROVED - High quality implementation with excellent compatibility ``` ## Red Flags | Flag | Severity | Action | |------|----------|--------| | Search loop removed/modified | 🔴 Critical | Ask why, verify PCRE compat | | Anchor behavior changed | 🔴 Critical | Must verify with PCRE/RE2 | | New zero-width code | 🟡 High | Check isZeroWidth() logic | | LLVM IR dead code | 🟡 High | Remove or explain | | Quantifier modification | 🟡 High | Verify greedy/non-greedy | | Missing test for feature | 🟠 Medium | Request tests added | | AGENTS.md not updated | 🟠 Medium | Minor, can update later | | Code comments unclear | 🟢 Low | Ask for clarification | ## Questions to Ask 1. **Why did you choose this approach?** - Understand design intent 2. **What edge cases could break this?** - Uncover missing handling 3. **How does this compare to PCRE?** - Verify compatibility 4. **What tests verify this works?** - Ensure coverage 5. **Will this impact performance?** - Check optimization 6. **Is this the simplest solution?** - Prefer clarity over cleverness ## Success Criteria - [ ] Code compiles without warnings - [ ] All tests pass (make test_all) - [ ] No memory leaks - [ ] PCRE/RE2 compatible - [ ] Clear comments and documentation - [ ] Follows project patterns - [ ] Ready to commit ## Quick Reference ```bash # Build and test make clean && make test_all # Check specific tests ./test_anchor_quant_edge ./test_anchor ./test_charclass # View changes git diff src/regjit.cpp # Check for warnings make 2>&1 | grep -i warning ``` --- > Converted and distributed by [TomeVault](https://tomevault.io/claim/acekingke) — claim your Tome and manage your conversions.