- Removed CLAUDE.md guidance file for Claude Code usage - Removed code reuse improvement plan documentation - Removed encapsulation review documentation - Updated composer.json to specify phpstan version constraint - Fixed namespace issues in Extractor.php exception handling - Renamed main function in bin/extractor.php to avoid conflicts - Updated LICENSE to standard GPL formatmaster
parent
208541d00b
commit
422bdc4ce6
17 changed files with 128 additions and 732 deletions
@ -1,274 +0,0 @@ |
|||||||
# Code Reuse Improvement Plan |
|
||||||
|
|
||||||
## Analysis Summary |
|
||||||
|
|
||||||
| Metric | Value | |
|
||||||
|--------|-------| |
|
||||||
| Total source lines | ~11,000 (PHP only) | |
|
||||||
| CompilerBase | 5,917 lines, 269 methods, 20 traits | |
|
||||||
| Gcc↔Clang duplication | ~70-80% of methods | |
|
||||||
| Linux↔Macos duplication | ~80% of methods | |
|
||||||
| `fatalError()` call sites | 174+ across codebase | |
|
||||||
| Test setUp/tearDown dup | 4+ test classes | |
|
||||||
|
|
||||||
--- |
|
||||||
|
|
||||||
## Phase 1: High-Impact Backend/Platform Deduplication (P0) |
|
||||||
|
|
||||||
### 1.1 Extract `UnixPlatform` base class |
|
||||||
|
|
||||||
**Files**: `Platform/Linux.php` (249 lines), `Platform/Macos.php` (278 lines) |
|
||||||
|
|
||||||
These 13 methods are 100% identical between Linux and Macos: |
|
||||||
- `getIncludeFlags()`, `getLibraryPathFlags()`, `getObjectExtension()`, `getExecutableExtension()`, `getPathSeparator()`, `getPhpDir()`, `getRpathOptions()`, `getPicFlag()`, `buildPhpIncludePaths()`, `findPhpConfig()`, `buildPhpLibPaths()` |
|
||||||
|
|
||||||
Near-identical with minor parameterization: |
|
||||||
- `getLibraryFlags()` — only the regex differs (`.a|.so` vs `.a|.dylib`) |
|
||||||
- `detectPhpLibs()` — only the lib name differs (`libphp.so` vs `libphp.dylib`) |
|
||||||
|
|
||||||
**Plan**: Create `UnixPlatform extends PlatformBase` between `PlatformBase` and `Linux`/`Macos`. Move all identical methods up. Add abstract `getSharedLibraryExtension()` (already exists) and a protected `getSharedLibName()` for the single differing method. |
|
||||||
|
|
||||||
**Expected savings**: ~180 lines removed, ~150 lines added = net ~30 lines but massive maintainability gain. |
|
||||||
|
|
||||||
### 1.2 Extract `GccLikeBackend` base class |
|
||||||
|
|
||||||
**Files**: `Backend/Gcc.php` (379 lines), `Backend/Clang.php` (515 lines) |
|
||||||
|
|
||||||
These methods are structurally identical with only Windows-specific branching: |
|
||||||
- `compileFile()`, `linkObjects()`, `buildCompileCommand()`, `buildCCompileCommand()`, `buildNativeCompileCommand()`, `buildLinkCommand()`, `buildCompileOptions()`, `buildLinkOptions()`, `buildFullCompileOptions()`, `buildFullLinkOptions()` |
|
||||||
|
|
||||||
**Plan**: Create `GccLikeBackend extends CompilerBackend` with all shared logic. Define template-method hooks for the differences: |
|
||||||
- `getCompilerSpecificFlags()` — empty for Gcc, MSVC compat flags for Clang/Windows |
|
||||||
- `getOutputFlag($isWindows)` — `-o` vs `/OUT:` |
|
||||||
- `getSanitizerFlag($type)` — handle the `address`/`addr` aliasing difference |
|
||||||
- `getPICHandling($config)` — Gcc always adds `-fPIC`, Clang skips on Windows |
|
||||||
|
|
||||||
**Expected savings**: ~250+ lines removed from Gcc.php and Clang.php. Msvc.php is sufficiently different (different flag syntax) to remain standalone. |
|
||||||
|
|
||||||
--- |
|
||||||
|
|
||||||
## Phase 2: CompilerBase Internal Deduplication (P1) |
|
||||||
|
|
||||||
### 2.1 Consolidate Big* type dispatch in BinaryOpTrait |
|
||||||
|
|
||||||
**File**: `Parser/BinaryOpTrait.php` (lines 31-107) |
|
||||||
|
|
||||||
The three blocks for BigFloat (lines 31-54), Decimal (lines 56-76), and BigInt (lines 78-107) in `parseBinaryOp()` share identical structure: |
|
||||||
1. Check if either operand is the big type |
|
||||||
2. Guard against incompatible mixing |
|
||||||
3. Convert the non-matching operand |
|
||||||
4. Dispatch to arithmetic or comparison operator |
|
||||||
|
|
||||||
**Plan**: Extract `parseBigNumBinaryOp(string $type, string $left, string $right, ...)` parameterized by type name, conversion function, and operator maps. Same refactoring applies to `genBigNumericCmp()` (lines 315-355). |
|
||||||
|
|
||||||
**Expected savings**: ~40 lines. |
|
||||||
|
|
||||||
### 2.2 Data-driven operator dispatch tables |
|
||||||
|
|
||||||
**Files**: `Parser/BinaryOpTrait.php` (16 wrapper methods), `Parser/AssignOpTrait.php` (14 wrapper methods) |
|
||||||
|
|
||||||
30+ thin methods that are just `parseBinaryOp($left, $right, '+')` / `parseAssignOp($node, '+=')`. |
|
||||||
|
|
||||||
**Plan**: Replace with a static map in `parseExpr()`: |
|
||||||
```php |
|
||||||
private const BINARY_OP_MAP = [ |
|
||||||
'Expr_BinaryOp_Plus' => '+', |
|
||||||
'Expr_BinaryOp_Minus' => '-', |
|
||||||
// ... |
|
||||||
]; |
|
||||||
private const ASSIGN_OP_MAP = [ |
|
||||||
'Expr_AssignOp_Plus' => '+=', |
|
||||||
// ... |
|
||||||
]; |
|
||||||
``` |
|
||||||
|
|
||||||
**Expected savings**: ~200 lines removed (boilerplate method bodies). |
|
||||||
|
|
||||||
### 2.3 Deduplicate call dispatch patterns |
|
||||||
|
|
||||||
**Files**: `CompilerBase.php` (`parseFuncCall`, `parseMethodCall`, `parseStaticCall` — ~300 lines combined), `UniversalMethodCall.php` (`tryOptimizePhpFn` vs `dispatchFuncCall`) |
|
||||||
|
|
||||||
These share the same overall flow: resolve callable → try native/optimized path → on `PlaceHolder` fall back to placeholder → parse args → wrap in `php::call()`. Additionally, `tryOptimizePhpFn()` (UniversalMethodCall lines 720-772) duplicates the argument type conversion logic already present in `dispatchFuncCall()` (FuncCallOptimizer lines 234-269). |
|
||||||
|
|
||||||
**Plan**: Extract a shared `resolveCall(CallLike $expr, ...)` method. Unify arg conversion so `tryOptimizePhpFn` delegates to `dispatchFuncCall` instead of reimplementing it. |
|
||||||
|
|
||||||
**Expected savings**: ~40 lines, fixes double-calculation of arg conversions. |
|
||||||
|
|
||||||
### 2.4 Deduplicate UNIVERSAL_METHODS math entries |
|
||||||
|
|
||||||
**File**: `UniversalMethodCall.php` (lines 12-69) |
|
||||||
|
|
||||||
The INT block (lines 12-41) and FLOAT block (lines 42-69) contain 20 identical math method entries (`abs`, `ceil`, `floor`, `sqrt`, `sin`, `cos`, etc.) differing only in `return_type`. Also the `calc_op` entries (add/sub/mul/div) are duplicated. |
|
||||||
|
|
||||||
**Plan**: Define math method names once in a shared array, generate both INT and FLOAT entries in the constructor with the appropriate `return_type`. |
|
||||||
|
|
||||||
**Expected savings**: ~25 lines of config data. |
|
||||||
|
|
||||||
### 2.5 Deduplicate constant folding methods |
|
||||||
|
|
||||||
**File**: `Optimizer/FuncCallOptimizer.php` (lines 515-593) |
|
||||||
|
|
||||||
8 methods (`doFoldStringLen`, `doFoldStringCase`, `doFoldCmp2`, `doFoldCmp3`, `doFoldCountLiteral`, `doFoldKnownClass`, `doFoldKnownConstant`, `doFoldSsaType`) follow the identical pattern: extract args → check types → compute → return literal or false. |
|
||||||
|
|
||||||
**Plan**: Create a generic `tryFold(callable $check, callable $compute)` that handles the arg extraction and short-circuit boilerplate. Each folder becomes a one-liner. |
|
||||||
|
|
||||||
**Expected savings**: ~50 lines. |
|
||||||
|
|
||||||
### 2.6 Remove MSVC compat flag duplication in Clang |
|
||||||
|
|
||||||
**File**: `Backend/Clang.php` |
|
||||||
|
|
||||||
The 4-line MSVC compatibility block (`-fms-compatibility`, `-fms-compatibility-version=19.40`, `-fdelayed-template-parsing`, `-fms-extensions`) appears 7 times (compileFile, buildCompileCommand, buildCCompileCommand, buildNativeCompileCommand, buildFullCompileOptions, buildCompileOptions, buildLinkOptions). |
|
||||||
|
|
||||||
**Plan**: Extract `private function getMsvcCompatFlags(): string` method. Called once per method that needs it instead of repeated inline. |
|
||||||
|
|
||||||
**Expected savings**: ~24 lines, single point of change if MSVC compat flags need updating. |
|
||||||
|
|
||||||
### 2.7 Merge return-check blocks |
|
||||||
|
|
||||||
**File**: `CompilerBase.php`, `parseReturn()` (line 1621) and `genReturnCode()` (line 5741) |
|
||||||
|
|
||||||
Identical 7-line union type check blocks. |
|
||||||
|
|
||||||
**Plan**: Extract `genUnionReturnWrapper(string $exprVar)` method. |
|
||||||
|
|
||||||
**Expected savings**: ~10 lines, eliminates drift risk. |
|
||||||
|
|
||||||
### 2.8 Fix `buildCCompileCommand()` inconsistency between Gcc and Clang |
|
||||||
|
|
||||||
**Files**: `Backend/Gcc.php` (lines 131-137), `Backend/Clang.php` (lines 198-206) |
|
||||||
|
|
||||||
Gcc unconditionally appends `-O$level` then conditionally appends `-g`. Clang treats debug and optimization as mutually exclusive (`if debug: -O0 -g` else `-O$level`). This is a behavioral inconsistency between backends implementing the same abstract method. |
|
||||||
|
|
||||||
**Plan**: Standardize on one behavior (the Clang pattern of `-O0 -g` for debug is the correct one — debug builds should not optimize). This will be automatically resolved by Phase 1.2 (GccLikeBackend). |
|
||||||
|
|
||||||
--- |
|
||||||
|
|
||||||
## Phase 3: Structural Improvements (P2) |
|
||||||
|
|
||||||
### 3.1 Entity flag-check consistency |
|
||||||
|
|
||||||
**File**: `Entity/PropertyDef.php` has `isPrivate()`, `isProtected()`, `isPublic()`, `isStatic()`. `Entity/MethodDef.php` has none — flag checks are done inline in CompilerBase. |
|
||||||
|
|
||||||
**Plan**: Add a `HasFlags` trait used by both `PropertyDef` and `MethodDef`: |
|
||||||
```php |
|
||||||
trait HasFlags { |
|
||||||
public function isPrivate(): bool { return $this->flags & Modifiers::PRIVATE; } |
|
||||||
public function isProtected(): bool { return $this->flags & Modifiers::PROTECTED; } |
|
||||||
public function isPublic(): bool { return !$this->isPrivate() && !$this->isProtected(); } |
|
||||||
public function isStatic(): bool { return $this->flags & Modifiers::STATIC; } |
|
||||||
public function isAbstract(): bool { return $this->flags & Modifiers::ABSTRACT; } |
|
||||||
} |
|
||||||
``` |
|
||||||
|
|
||||||
**Expected savings**: Removes inline flag checks from CompilerBase, adds clarity. |
|
||||||
|
|
||||||
### 3.2 Test infrastructure base class |
|
||||||
|
|
||||||
**Files**: `phpunit/src/AstNodeTypeTest.php`, `CompilerBaseAdapterTest.php`, `TraitsTest.php`, `PreprocessorTest.php` |
|
||||||
|
|
||||||
All 4 duplicate the same setUp/tearDown pattern: create temp dir, `CompilerTest::create()`, recursive cleanup. |
|
||||||
|
|
||||||
**Plan**: Add `CompilerTestCase extends \PHPUnit\Framework\TestCase` to `phpunit/bootstrap.php`: |
|
||||||
```php |
|
||||||
abstract class CompilerTestCase extends TestCase { |
|
||||||
protected string $tmpDir; |
|
||||||
protected CompilerTest $compiler; |
|
||||||
|
|
||||||
protected function setUp(): void { |
|
||||||
parent::setUp(); |
|
||||||
$this->tmpDir = sys_get_temp_dir() . '/compiler_test_' . uniqid(); |
|
||||||
mkdir($this->tmpDir, 0777, true); |
|
||||||
$this->compiler = CompilerTest::create($this->tmpDir); |
|
||||||
} |
|
||||||
|
|
||||||
protected function tearDown(): void { |
|
||||||
parent::tearDown(); |
|
||||||
// recursive cleanup |
|
||||||
} |
|
||||||
} |
|
||||||
``` |
|
||||||
|
|
||||||
### 3.3 Eliminate `buildFull*Options` / `build*Options` duality |
|
||||||
|
|
||||||
**Files**: `Backend/Gcc.php`, `Backend/Clang.php`, `Backend/Msvc.php` |
|
||||||
|
|
||||||
All three backends implement both `buildFullCompileOptions()` / `buildCompileOptions()` and `buildFullLinkOptions()` / `buildLinkOptions()`. The "full" variants are subsets of the "standard" variants working from differently-keyed option arrays. They have drifted independently (e.g., RPATH handling differs between the two in Gcc/Clang). |
|
||||||
|
|
||||||
**Plan**: Make the "full" variants delegate to the "standard" variants by normalizing their option keys once at the call site. Keep only one code path for each (compile/link). |
|
||||||
|
|
||||||
**Expected savings**: ~100+ lines, eliminates drift between the two variants. |
|
||||||
|
|
||||||
### 3.4 Deduplicate Preprocessor AST switch |
|
||||||
|
|
||||||
**File**: `Preprocessor.php` |
|
||||||
|
|
||||||
`prepareFile()` (lines 115-151) and `prepareNamespace()` (lines 196-223) both switch over the same set of AST `Stmt_*` types with nearly identical case bodies. |
|
||||||
|
|
||||||
**Plan**: Extract `processStmt(Node $v)` method that both callers share. |
|
||||||
|
|
||||||
**Expected savings**: ~25 lines. |
|
||||||
|
|
||||||
### 3.5 Remove dead code: `ScopeContext` |
|
||||||
|
|
||||||
**File**: `Context/ScopeContext.php` (7 lines) |
|
||||||
|
|
||||||
An empty class with no properties or methods. Used only as a type annotation in `FunctionContext`. Either populate it with scope-relevant state, or remove it and use plain `\stdClass` / array / null. |
|
||||||
|
|
||||||
### 3.6 Reduce StdContainerTrait coupling |
|
||||||
|
|
||||||
**File**: `Parser/StdContainerTrait.php` (823 lines, 48 methods) |
|
||||||
|
|
||||||
This is effectively a standalone subsystem for std container handling. As a trait, it has unrestricted access to CompilerBase's internals. |
|
||||||
|
|
||||||
**Plan**: Extract core logic into `StdContainerHandler` service class. The trait becomes a thin facade that delegates to the handler. |
|
||||||
|
|
||||||
**Expected savings**: Better testability, clearer boundaries, easier to understand. |
|
||||||
|
|
||||||
--- |
|
||||||
|
|
||||||
## Phase 4: Longer-Term Architectural (P3) |
|
||||||
|
|
||||||
### 4.1 Break CompilerBase into domain-specific classes |
|
||||||
|
|
||||||
Currently CompilerBase is a 5,917-line god class using 20 traits as a workaround for PHP's single inheritance. Consider: |
|
||||||
|
|
||||||
- `ExpressionCompiler` — all parseExpr sub-dispatch (~500 lines) |
|
||||||
- `StatementCompiler` — parseStmts, parseIf, parseWhile, parseFor, parseSwitch, etc. |
|
||||||
- `TypeResolver` — parseTypeDecl, detectClassOfExpr, type checking |
|
||||||
- `CallResolver` — parseFuncCall, parseMethodCall, parseStaticCall, parseNew |
|
||||||
|
|
||||||
These would be injected services rather than traits, making CompilerBase a coordinator. |
|
||||||
|
|
||||||
### 4.2 Shared AST walker pattern with Python Translator |
|
||||||
|
|
||||||
Both PHP and Python translators implement the same "walk-collect-indent-emit" pipeline independently. `Core\Translator` could define a standard `walkAst($nodes, callable $visitor)` that handles indentation and line collection. |
|
||||||
|
|
||||||
--- |
|
||||||
|
|
||||||
## Implementation Order & Impact Matrix |
|
||||||
|
|
||||||
| # | Item | Savings | Risk | Effort | |
|
||||||
|---|------|---------|------|--------| |
|
||||||
| 1.1 | UnixPlatform base class | ~180 dup lines | Low | 2-3h | |
|
||||||
| 1.2 | GccLikeBackend base class | ~250 dup lines | Medium | 3-4h | |
|
||||||
| 2.1 | BigNum dispatch consolidation | ~40 lines | Low | 1h | |
|
||||||
| 2.2 | Data-driven op dispatch | ~200 lines | Low | 1-2h | |
|
||||||
| 2.3 | Unify call dispatch patterns | ~40 lines | Low | 1-2h | |
|
||||||
| 2.4 | UNIVERSAL_METHODS math dedup | ~25 lines | Low | 30m | |
|
||||||
| 2.5 | Fold method template | ~50 lines | Low | 1h | |
|
||||||
| 2.6 | MSVC compat flags in Clang | ~24 lines | Low | 30m | |
|
||||||
| 2.7 | Merge return-check blocks | ~10 lines | Low | 30m | |
|
||||||
| 2.8 | Fix buildCCompileCommand drift | bug fix | Low | 30m | |
|
||||||
| 3.1 | HasFlags trait | clarity | Low | 1h | |
|
||||||
| 3.2 | CompilerTestCase base class | boilerplate | Low | 1h | |
|
||||||
| 3.3 | Eliminate Full*Options duality | ~100 lines | Medium | 2h | |
|
||||||
| 3.4 | Preprocessor AST switch dedup | ~25 lines | Low | 1h | |
|
||||||
| 3.5 | Remove dead ScopeContext | 7 lines | Low | 15m | |
|
||||||
| 3.6 | StdContainer service class | boundary | Medium | 3-4h | |
|
||||||
| 4.1 | Domain classes | architecture | High | 1-2 weeks | |
|
||||||
| 4.2 | AST walker pattern | architecture | Medium | 3-5h | |
|
||||||
|
|
||||||
**Total estimated savings**: ~950+ lines of duplicated / dead code. |
|
||||||
|
|
||||||
**Recommended execution**: Phase 1 → Phase 2 → Phase 3. Items within each phase are independent and can be parallelized. |
|
||||||
@ -1,325 +0,0 @@ |
|||||||
# Encapsulation Review |
|
||||||
|
|
||||||
## Summary |
|
||||||
|
|
||||||
| Metric | Value | |
|
||||||
|--------|-------| |
|
||||||
| Entity classes with all-public fields | 9/9 (100%) | |
|
||||||
| FunctionContext public properties | 26 (all mutable) | |
|
||||||
| CompilerBase private methods | 5/269 (1.9%) | |
|
||||||
| Traits with direct `$this->context->` access | 6 traits, 60+ sites | |
|
||||||
| ScopeContext (dead code) | 7 lines, empty class | |
|
||||||
|
|
||||||
--- |
|
||||||
|
|
||||||
## Issue 1: Entity classes — all-public mutable fields |
|
||||||
|
|
||||||
**Severity**: High. Every entity class exposes all internal state as public writable properties. External code in Preprocessor/CompilerBase directly mutates them. |
|
||||||
|
|
||||||
### 1.1 ClassDef (18 public properties) |
|
||||||
|
|
||||||
`src/Php/Entity/ClassDef.php` |
|
||||||
|
|
||||||
```php |
|
||||||
public array $methods = []; // externally populated: $classDef->properties[$name] = ... |
|
||||||
public array $properties = []; // externally populated |
|
||||||
public array $constants = []; // externally populated |
|
||||||
public array $implements = []; // externally populated |
|
||||||
public string $extends = ''; // externally set: $this->classDef->extends = ... |
|
||||||
public bool $requireCtor = false; |
|
||||||
public bool $enum = false; |
|
||||||
public ?string $enumBackingType = null; |
|
||||||
public array $enumCases = []; |
|
||||||
public array $abstractMethods = []; |
|
||||||
public ?Trait_ $trait = null; |
|
||||||
public array $traitAliases = []; |
|
||||||
public array $traitIgnored = []; |
|
||||||
public int $flags; // no visibility checks, raw bitmask |
|
||||||
public bool $inheritedFromInternalClass = false; |
|
||||||
public string $ctorInit = ''; // mutated during code generation |
|
||||||
public string $ctorClean = ''; // mutated during code generation |
|
||||||
public FunctionContext $propertyContext; // set after construction |
|
||||||
``` |
|
||||||
|
|
||||||
**Issues**: |
|
||||||
- `$properties`, `$methods`, `$constants` — exposed as raw arrays. External code does `$classDef->properties[$name] = $propDef`. No validation that the key matches `$propDef->name`, no type enforcement. |
|
||||||
- `$flags` — raw int, no guarantee it's a valid Modifiers bitmask. |
|
||||||
- `$ctorInit` / `$ctorClean` — mutated by CompilerBase during code generation, not initialization. |
|
||||||
- Property additions use `addMethod()`, `addAbstractMethod()` but array properties are also set directly via `[] =`. |
|
||||||
- `$extends` — set directly as raw string, bypasses `parent::__construct()` which also sets it on ClassLikeDef. |
|
||||||
|
|
||||||
**Recommendation**: |
|
||||||
- Make `$methods`, `$properties`, `$constants` private, expose via `addMethod()`/`getMethod()` (already exists) |
|
||||||
- Make `$flags` private, expose `isAbstract()` (already exists), add `isFinal()`, `isReadonly()` |
|
||||||
- Make `$extends` write-once via `setExtends(string)` with validation |
|
||||||
- Add `appendCtorInit(string)` and `appendCtorClean(string)` methods instead of direct string mutation |
|
||||||
|
|
||||||
### 1.2 FunctionDef (12 public properties) |
|
||||||
|
|
||||||
`src/Php/Entity/FunctionDef.php` |
|
||||||
|
|
||||||
```php |
|
||||||
public string $name; |
|
||||||
public string $returnType; |
|
||||||
public array $argInfoList = []; // externally populated: $functionDef->argInfoList[] = $argInfo |
|
||||||
public int $argCountRequired = 0; |
|
||||||
public string $params = ''; // generated C++ param string, mutated during compilation |
|
||||||
public string $namespace; |
|
||||||
public bool $method = false; |
|
||||||
public bool $stub = false; |
|
||||||
public bool $returnTypeUndeclared = false; |
|
||||||
public string $returnClass = ''; |
|
||||||
public ?array $returnTypeCheck = null; |
|
||||||
public string $returnTypeStr = ''; |
|
||||||
public ?NodeAbstract $returnTypeNode = null; |
|
||||||
``` |
|
||||||
|
|
||||||
**Issues**: |
|
||||||
- `$name` and `$namespace` are set in constructor but still publicly writable — should be readonly |
|
||||||
- `$argInfoList[]` is directly appended to by Preprocessor (line 329) |
|
||||||
- `$params` is a codegen artifact stored on the entity — belongs in a separate compilation context |
|
||||||
|
|
||||||
**Recommendation**: |
|
||||||
- Make constructor-set properties readonly (`$name`, `$namespace`, `$returnType`) |
|
||||||
- Add `addArg(ArgInfo $arg)` method instead of direct array mutation |
|
||||||
- Extract `$params` to a compilation context separate from the definition entity |
|
||||||
|
|
||||||
### 1.3 PropertyDef (7 public properties) |
|
||||||
|
|
||||||
`src/Php/Entity/PropertyDef.php` |
|
||||||
|
|
||||||
```php |
|
||||||
public string $name; |
|
||||||
public string $type; |
|
||||||
public int $flags; |
|
||||||
public ?string $default = null; |
|
||||||
public ?ArrayInitPlan $arrayInitPlan = null; |
|
||||||
public bool $nullable = false; |
|
||||||
public string $class = ''; // set after construction |
|
||||||
``` |
|
||||||
|
|
||||||
**Issues**: |
|
||||||
- `$class` is set after construction externally (`$propDef->class = $fullClassName`) |
|
||||||
- `$flags` is raw int — already has `isPrivate()`/`isProtected()`/`isPublic()`/`isStatic()` methods, good |
|
||||||
- Constructor already sets all core fields — `$class` should be added to the constructor |
|
||||||
|
|
||||||
**Recommendation**: |
|
||||||
- Add `$class` to the constructor (it's always known at construction time) |
|
||||||
- Make constructor-set fields readonly or private |
|
||||||
|
|
||||||
### 1.4 MethodDef (4 public properties) |
|
||||||
|
|
||||||
`src/Php/Entity/MethodDef.php` |
|
||||||
|
|
||||||
```php |
|
||||||
public int $flags; |
|
||||||
public string $name; |
|
||||||
public ?FunctionDef $functionDef = null; // set after construction |
|
||||||
public bool $hasDynamicCall = false; |
|
||||||
``` |
|
||||||
|
|
||||||
**Issues**: |
|
||||||
- No flag-check methods — inline checks in CompilerBase should use `$methodDef->isPrivate()` instead |
|
||||||
- `$functionDef` is set externally: `$this->methodDef->functionDef = $functionDef` (Preprocessor line 411) |
|
||||||
|
|
||||||
**Recommendation**: |
|
||||||
- Add `HasFlags` trait (from Phase 3.1 of reuse plan) |
|
||||||
- Add `setFunctionDef(FunctionDef $fd)` method with validation |
|
||||||
|
|
||||||
### 1.5 ConstantDef (8 public properties) |
|
||||||
|
|
||||||
```php |
|
||||||
public string $name; |
|
||||||
public string $type; |
|
||||||
public int $flags; |
|
||||||
public string $value; |
|
||||||
public string $arrayExpr = ''; |
|
||||||
public string $class = ''; |
|
||||||
public ?NodeAbstract $valueExpr = null; |
|
||||||
``` |
|
||||||
|
|
||||||
**Issues**: Same pattern — constructor sets core fields, but `$class` is set externally afterward. |
|
||||||
|
|
||||||
**Recommendation**: Add `$class` to constructor. |
|
||||||
|
|
||||||
--- |
|
||||||
|
|
||||||
## Issue 2: FunctionContext — public mutable grab-bag |
|
||||||
|
|
||||||
**Severity**: High. 26 public properties, all writable by any code with access to the context object. |
|
||||||
|
|
||||||
`src/Php/Context/FunctionContext.php` |
|
||||||
|
|
||||||
```php |
|
||||||
public ?SsaBuilder $ssaBuilder = null; // transient analysis state |
|
||||||
public array $stableObjects = []; // SSA optimizer state |
|
||||||
public array $hoistedProps = []; // SSA optimizer state |
|
||||||
public array $unsafeObjectProps = []; // SSA optimizer state |
|
||||||
public array $objects = []; // object variable tracking |
|
||||||
public array $stdArrays = []; // std container tracking |
|
||||||
public array $stdContainers = []; // std container tracking |
|
||||||
public array $localVars = []; // local variable table |
|
||||||
public array $staticVars = []; // static variable table |
|
||||||
public array $globalVars = []; // global variable table |
|
||||||
public array $ceWrappers = []; // class entry wrappers |
|
||||||
public int $tmpVarIndex = 0; // auto-increment counter |
|
||||||
public array $arguments = []; // function arguments |
|
||||||
public bool $inLoop = false; // control-flow state |
|
||||||
public bool $inClosure = false; // control-flow state |
|
||||||
public bool $hasMultiLevelBreak = false; |
|
||||||
public bool $hasMultiLevelContinue = false; |
|
||||||
public bool $inAssignExpr = false; // expression context |
|
||||||
public array $beforeStmtLines = []; // deferred code (flushed before stmts) |
|
||||||
public array $afterStmtLines = []; // deferred code (flushed after stmts) |
|
||||||
public array $objectProps; // (uninitialized!) |
|
||||||
public array $staticPropRefs = []; // static property references |
|
||||||
public int $scopeLevel = 0; // lexical scope depth |
|
||||||
/** @var array<int, ScopeContext> */ |
|
||||||
public array $scopeLayouts = []; // per-scope data |
|
||||||
``` |
|
||||||
|
|
||||||
**Issues**: |
|
||||||
- Traits directly mutate deeply nested state: `$this->context->stdArrays[$var] = ...`, `$this->context->localVars[$name] = ...` |
|
||||||
- No semantic grouping — analysis state, variable tracking, control-flow flags all mixed |
|
||||||
- `$objectProps` is declared but never initialized (could be null at runtime) |
|
||||||
- `$scopeLayouts` is managed through `enterScope()`/`leaveScope()` — but can be bypassed |
|
||||||
- `$tmpVarIndex` auto-increment — should use a method instead of direct `++` |
|
||||||
|
|
||||||
**Recommendation**: |
|
||||||
- Group related properties into sub-objects: `VariableTable`, `ControlFlowState`, `ScopeManager` |
|
||||||
- Make properties that should only be read by the compiler layer private/protected with getters |
|
||||||
- Add `incrementTmpVar(): int`, `addLocalVar()`, `addBeforeStmt()` methods |
|
||||||
- Initialize `$objectProps = []` |
|
||||||
|
|
||||||
--- |
|
||||||
|
|
||||||
## Issue 3: CompilerBase — only 1.9% private methods |
|
||||||
|
|
||||||
**Severity**: Medium. Virtually everything is public or protected. |
|
||||||
|
|
||||||
`src/Php/CompilerBase.php` — 269 methods total: |
|
||||||
- ~25 public methods (many should be protected or internal) |
|
||||||
- ~239 protected methods (most should be private — internal helpers) |
|
||||||
- **5 private methods** (1.9%) |
|
||||||
|
|
||||||
### 3.1 Methods that should be private |
|
||||||
|
|
||||||
The following methods are internal helpers only called from within CompilerBase (not from Preprocessor, Translator, or traits). They are unnecessarily `protected`: |
|
||||||
|
|
||||||
| Method | Line | Called from | |
|
||||||
|--------|------|-------------| |
|
||||||
| `resetFunction()` | 833 | Internal only | |
|
||||||
| `resetMethod()` | 840 | Internal only | |
|
||||||
| `resetClass()` | 846 | Internal only | |
|
||||||
| `resolveObjectClassDef()` | 816 | Already private ✓ | |
|
||||||
| `getBigIntLiteralString()` | 1183 | Already private ✓ | |
|
||||||
| `getDecimalLiteralString()` | 1188 | Already private ✓ | |
|
||||||
| `parseBeforeStmtLines()` | 1321 | Internal, but accessed by traits | |
|
||||||
| `parseAfterStmtLines()` | 1331 | Internal, but accessed by traits | |
|
||||||
| `genTmpVarName()` | 734 | Public — should at least be protected | |
|
||||||
|
|
||||||
### 3.2 Public methods that are internal concern |
|
||||||
|
|
||||||
| Method | Current visibility | Issue | |
|
||||||
|--------|-------------------|-------| |
|
||||||
| `genTmpVarName()` | public | Only used internally for variable name generation | |
|
||||||
| `writeFile()` | public | File I/O — should be a separate service | |
|
||||||
| `stop()` | public | Error helper — could be internal | |
|
||||||
| `isScalarInt()` | public | AST helper, only used internally | |
|
||||||
| `getType()` | public | AST helper, only used internally | |
|
||||||
| `getObjectType()` | public | Type mapping, used internally | |
|
||||||
| `getTypeFromZendType()` | public | Type mapping, used internally | |
|
||||||
| `getIncludeDir()` | public | Config getter — should be on a Config object | |
|
||||||
| `getBuildDir()` | public | Config getter — should be on a Config object | |
|
||||||
|
|
||||||
### 3.3 Public constants leaked as API |
|
||||||
|
|
||||||
30 public constants for internal type names, literal values, etc. These are needed by traits but expose internal naming conventions. |
|
||||||
|
|
||||||
--- |
|
||||||
|
|
||||||
## Issue 4: Trait → Context coupling |
|
||||||
|
|
||||||
**Severity**: Medium. Traits bypass any encapsulation boundary and directly mutate `$this->context`. |
|
||||||
|
|
||||||
| Trait | `$this->context->` accesses | |
|
||||||
|-------|---------------------------| |
|
||||||
| `StdContainerTrait` | 40+ accesses to `stdArrays`, `stdContainers`, `objects`, `localVars` | |
|
||||||
| `LoopVarOptimizer` | accesses to `localVars`, `arguments`, `scopeLevel`, `inLoop` | |
|
||||||
| `SsaPropOptimizer` | accesses to `stableObjects`, `hoistedProps`, `unsafeObjectProps`, `objects` | |
|
||||||
| `FuncCallOptimizer` | accesses to `beforeStmtLines`, `arguments`, `localVars` | |
|
||||||
| `SsaTypeOptimizer` | accesses to `localVars` | |
|
||||||
| `BinaryOpTrait` | accesses to `objects`, `localVars` | |
|
||||||
|
|
||||||
**Issues**: |
|
||||||
- Traits have no declared contract — they assume `$this->context` exists and has specific properties |
|
||||||
- If a property name changes in FunctionContext, all 6 traits break silently |
|
||||||
- No type safety — arrays are indexed by string but accessed with arbitrary keys |
|
||||||
|
|
||||||
**Recommendation**: |
|
||||||
- Define a `ContextAccess` interface that traits must use instead of direct property access |
|
||||||
- Or: inject context into trait methods as a parameter instead of reading from `$this` |
|
||||||
- Short-term: add `@property-read` annotations to document the contract |
|
||||||
|
|
||||||
--- |
|
||||||
|
|
||||||
## Issue 5: Preprocessor directly mutates entity state |
|
||||||
|
|
||||||
**Severity**: Medium. Preprocessor bypasses entity boundaries. |
|
||||||
|
|
||||||
`src/Php/Preprocessor.php`: |
|
||||||
```php |
|
||||||
line 279: $argInfo->name = $name; // direct property set |
|
||||||
line 329: $functionDef->argInfoList[] = $argInfo; // direct array append |
|
||||||
line 411: $this->methodDef->functionDef = $functionDef; // direct property set |
|
||||||
line 447: $this->classDef->extends = $this->parentClass; // direct property set |
|
||||||
line 585: $this->classDef->constants[$constInfo->name] = ...; // direct array set |
|
||||||
line 618: $this->classDef->properties[$name] = $propDef; // direct array set |
|
||||||
``` |
|
||||||
|
|
||||||
**Recommendation**: Use entity methods: `$functionDef->addArg($argInfo)`, `$this->methodDef->setFunctionDef($functionDef)`, `$classDef->addProperty($propDef)`, etc. |
|
||||||
|
|
||||||
--- |
|
||||||
|
|
||||||
## Issue 6: CompilerBase protected state leaked to inheritance chain |
|
||||||
|
|
||||||
**Severity**: Low-Medium. The chain CompilerBase → Preprocessor → Translator means any protected property in CompilerBase is accessible from Translator. |
|
||||||
|
|
||||||
CompilerBase has ~50 protected properties. Translator is 3301 lines and accesses many of them. There's no way to know which properties are "safe to use" vs "internal to CompilerBase." |
|
||||||
|
|
||||||
**Recommendation**: Migrate internal-only properties to `private` over time, with explicit getter methods where needed. |
|
||||||
|
|
||||||
--- |
|
||||||
|
|
||||||
## Issue 7: Platform/Backend — well encapsulated |
|
||||||
|
|
||||||
**Severity**: None. The Platform and Backend layers are well-encapsulated: |
|
||||||
- All state is private (e.g., `$compilerCommand`, `$linkerCommand` in GccLikeBackend) |
|
||||||
- Only methods are public |
|
||||||
- Abstract contracts are clear |
|
||||||
- Factory pattern is used consistently |
|
||||||
|
|
||||||
**No changes needed in this layer.** |
|
||||||
|
|
||||||
--- |
|
||||||
|
|
||||||
## Issue 8: ScopeContext is dead code |
|
||||||
|
|
||||||
**Severity**: Low. `src/Php/Context/ScopeContext.php` — 7 lines, empty class body. Used as a placeholder type in FunctionContext's `$scopeLayouts` array. Either populate it or remove it. |
|
||||||
|
|
||||||
--- |
|
||||||
|
|
||||||
## Implementation Priority |
|
||||||
|
|
||||||
| # | Issue | Impact | Effort | Risk | |
|
||||||
|---|-------|--------|--------|------| |
|
||||||
| 1.1 | Entity: readonly for constructor fields | Data integrity | 2h | Low | |
|
||||||
| 1.2 | Entity: add mutation methods (addArg, addProperty, etc.) | Safe mutation | 3h | Medium | |
|
||||||
| 2.1 | FunctionContext: group properties into sub-objects | Clarity | 4h | Medium | |
|
||||||
| 2.2 | FunctionContext: add accessor methods | Controlled mutation | 3h | Medium | |
|
||||||
| 3 | CompilerBase: demote public→protected, protected→private | Boundary clarity | 4h | Medium | |
|
||||||
| 4 | Define trait context contract | Safe coupling | 3h | Medium | |
|
||||||
| 5 | Preprocessor: use entity methods | Consistent mutation | 2h | Low | |
|
||||||
| 8 | Remove ScopeContext dead code | Cleanup | 15m | None | |
|
||||||
|
|
||||||
**Recommended order**: Start with 8 (quick win), then 1.1 + 1.2 (entity cleanup), then 2.1 + 2.2 (context cleanup), then 3 + 5 + 4 (CompilerBase boundary). |
|
||||||
@ -1,103 +0,0 @@ |
|||||||
# CLAUDE.md |
|
||||||
|
|
||||||
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. |
|
||||||
|
|
||||||
## Project Overview |
|
||||||
|
|
||||||
TypePHP is a PHP native compilation project. Its `tpc` command is TypePHP Compiler (AOT), which translates PHP source code to C++, then compiles it with GCC/Clang/MSVC into native binaries. It supports Linux (primary), macOS, and Windows. |
|
||||||
|
|
||||||
**Prerequisites**: PHP 8.2+, GCC 9+ (C++17), CMake 3.24+. The `swoole/phpx` extension must be compiled (see README.md). |
|
||||||
|
|
||||||
## TypePHP Language Design Principles |
|
||||||
|
|
||||||
TypePHP should not blindly mirror every PHP language behavior. Most PHP syntax and semantics should remain compatible with ZendPHP, but some legal PHP constructs are historical baggage or language-design mistakes that conflict with static compilation, clear semantics, or robust generated C++ code. |
|
||||||
|
|
||||||
When reviewing or changing compiler behavior: |
|
||||||
|
|
||||||
- Prefer PHP compatibility for common, well-defined syntax that does not weaken the TypePHP static model. |
|
||||||
- Reject PHP historical baggage when the syntax is ambiguous, surprising, or only preserved for legacy compatibility. |
|
||||||
- Diagnose such cases as early as possible during preprocessing/static compilation, instead of deferring to runtime TypeCheck or ZendVM errors. |
|
||||||
- Provide precise errors that include the relevant function/method name, parameter/property name, and type information where applicable. |
|
||||||
- Compare with other statically compiled languages such as C/C++, Java, C#, Go, Rust, Kotlin, and TypeScript before deciding whether TypePHP should preserve or reject a PHP behavior. |
|
||||||
|
|
||||||
Example: `function test($a = 1, $b, $c) {}` is legal in PHP, but the default value for `$a` is effectively ignored and all parameters become required. This is a PHP historical compatibility artifact. TypePHP should reject it during preprocessing instead of preserving the behavior. |
|
||||||
|
|
||||||
Example: PHP permits `return $value;` inside `__construct()` and lets callers consume `parent::__construct()` as a value, even though constructors cannot declare a return type. TypePHP treats constructors consistently with C++/Java-style semantics: constructors initialize objects and must not return values. `return;` is allowed, but `return $value;` or using a constructor call as a value must be rejected during static compilation. |
|
||||||
|
|
||||||
## Build & Test Commands |
|
||||||
|
|
||||||
```bash |
|
||||||
# Install PHP dependencies |
|
||||||
composer install |
|
||||||
|
|
||||||
# Compile a PHP project to a native binary |
|
||||||
./tpc <path-to-project-or-file> |
|
||||||
|
|
||||||
# Run all PHPUnit tests |
|
||||||
./vendor/bin/phpunit |
|
||||||
|
|
||||||
# Run a single PHPUnit test class |
|
||||||
./vendor/bin/phpunit phpunit/src/AstNodeTypeTest.php |
|
||||||
|
|
||||||
# Run PHPT integration tests (all) |
|
||||||
php run-tests.php tests/compiler/ |
|
||||||
|
|
||||||
# Run a single PHPT test |
|
||||||
php run-tests.php tests/compiler/arrays.phpt |
|
||||||
``` |
|
||||||
|
|
||||||
## Architecture |
|
||||||
|
|
||||||
### Translation Pipeline |
|
||||||
|
|
||||||
The compiler follows a 4-stage pipeline, orchestrated by `src/Translator.php` (the main entry point): |
|
||||||
|
|
||||||
1. **prepare()** — Scan PHP files, collect symbol declarations and dependencies, topological-sort for compilation order |
|
||||||
2. **convert()** — Parse PHP AST via `nikic/php-parser`, translate each node to C++ source code |
|
||||||
3. **compile()** — Invoke the platform C++ compiler (GCC/Clang/MSVC) on generated `.cc` files |
|
||||||
4. **build()** — Link object files into a native binary executable |
|
||||||
|
|
||||||
### Class Hierarchy |
|
||||||
|
|
||||||
``` |
|
||||||
src/CompilerBase.php (core PHP→C++ translation logic, indent/output/mode helpers) |
|
||||||
├─ uses traits: AstNodeType, FuncCallOptimizer, ClosureGenerator, |
|
||||||
│ PlaceHolderGenerator, PropertyPromotion, MagicMethodDetector |
|
||||||
└─ src/Preprocessor.php (scanning, symbol tables, dependency sort, YAML config) |
|
||||||
└─ src/Translator.php (full pipeline: prepare→convert→compile→build) |
|
||||||
└─ src/CompilerTest.php (test-only subclass, used by PHPUnit tests) |
|
||||||
``` |
|
||||||
|
|
||||||
### Key Components |
|
||||||
|
|
||||||
| Directory | Purpose | |
|
||||||
|-----------|---------| |
|
||||||
| `src/Entity/` | Data classes: `ClassDef`, `FunctionDef`, `MethodDef`, `PropertyDef`, `ConstantDef`, `InterfaceDef` | |
|
||||||
| `src/Generator/` | Codegen helpers: `ClosureGenerator`, `PlaceHolderGenerator`, `PropertyPromotion`, `Utils` | |
|
||||||
| `src/Backend/` | Compiler abstraction: `CompilerBackend` (abstract) → `Gcc`, `Clang`, `Msvc`. Factory pattern via `CompilerFactory` | |
|
||||||
| `src/Platform/` | OS abstraction: `PlatformBase` → `Linux`, `Macos`, `Windows`. Factory via `PlatformFactory` | |
|
||||||
| `src/Context/` | `ScopeContext` and `FunctionContext` for variable scoping and type tracking | |
|
||||||
| `src/Exception/` | `SyntaxError`, `Unsupported`, `DynamicCall`, `PlaceHolder`, `Skip`, `Redo`, `TestError` | |
|
||||||
| `src/Parser/` | Special-purpose parsers like `StdContainerParser` (C++ std container foreach support) | |
|
||||||
| `src/Resolver/Reflection.php` | Static helpers wrapping PHP reflection (internal class/function detection) | |
|
||||||
| `src/Generator/Symbol.php` | Maps PHP operations to `phpx` C++ API symbol names | |
|
||||||
| `src/Build/FileScanner.php` | Recursive file discovery with extension filtering (supports `.php`, `.cpp`, `.c`, `.s`, `.m`, `.mm`) | |
|
||||||
| `src/Entity/ArgInfo.php` | Generates C function argument info structures for internal function registration | |
|
||||||
| `src/Extractor.php` | Extracts interfaces from PHP classes | |
|
||||||
| `src/Transform/Visitor.php` | Base `NodeVisitorAbstract` extension (skeleton for custom AST visitors) | |
|
||||||
|
|
||||||
### Configuration |
|
||||||
|
|
||||||
- `project.yml` — per-project build config (name, build-mode, C++ standard, compiler flags, sources, resources/icon) |
|
||||||
- Command-line arguments and YAML config are merged in `Preprocessor`, with CLI taking highest priority |
|
||||||
|
|
||||||
### Generated Output |
|
||||||
|
|
||||||
Generated `.cc` and `.o` files land in `build/` directory. The compiled binary is named from `project.yml`'s `name` field (default: `app`). |
|
||||||
|
|
||||||
### Test Infrastructure |
|
||||||
|
|
||||||
- **PHPUnit tests** (`phpunit/src/`) — unit/integration tests for compiler internals. Bootstrap at `phpunit/bootstrap.php` defines a `BaseTest` class with an `exec()` helper that runs the compiler and expects a `TestError` exception containing a given string |
|
||||||
- **PHPT tests** (`tests/compiler/`) — end-to-end tests using the standard PHPT format (`run-tests.php`). Each `.phpt` contains PHP source and expected output sections |
|
||||||
|
|
||||||
When writing new compiler tests, use `CompilerTest::create(ROOT_PATH)` (in `src/CompilerTest.php`) which sets `forTest = true` to enable test-specific behavior without writing files to disk. |
|
||||||
Loading…
Reference in new issue