xp-framework / compiler

Compiles future PHP to today's PHP.
19 stars 0 forks source link

Add support for omitting property types at compile time #158

Closed thekid closed 1 year ago

thekid commented 1 year ago

This pull request makes it possible to omit property types at compile time.

Usage

Given the following source code:

class Types {
  private string $name;
}

Typical compilation:

$ xp compile -q Types.php
<?php

 class Types{
private string $name;static function __init() {}} Types::__init();;

Compile without property types:

$ xp compile -q -a php:without-property-types Types.php
<?php

 class Types{
private  $name;static function __init() {}} Types::__init();;

Originally requested in https://github.com/xp-framework/compiler/issues/119

danon commented 1 year ago

Is the Types::__init(); still necessary if we ommitting type checks in runtime?

To answer your question:

and only decide whether we want to add additional runtime checking at the stage of compilation

@Danon - have a look at #158, would that meet your needs?

I was looking for a quick solution, to simply remove from code any structure that isn't supported in earlier versions, without adding anything new into the code (like polyfils of runtime checks). Basically I wanted to remove usage of new features, even if it means loosing some of the checks.

thekid commented 1 year ago

Is the Types::__init(); still necessary if we ommitting type checks in runtime?

No, this code is always generated when emitting any compiled type. The __init() function initializes static properties to non-scalar values, e.g.:

// What we write
class Types {
  private static $parser= new Parser();
}

// What gets emitted
class Types {
  private static $parser;

  static function __init() {
    self::$parser= new Parser();
  }
}
Types::__init();

We could optimize the emitted code by removing the declaration as well as the invocation if no such initializations exist, yes.

danon commented 1 year ago

Is the Types::__init(); still necessary if we ommitting type checks in runtime?

No, this code is always generated when emitting any compiled type. The __init() function initializes static properties to non-scalar values, e.g.:

// What we write
class Types {
  private static $parser= new Parser();
}

// What gets emitted
class Types {
  private static $parser;

  static function __init() {
    self::$parser= new Parser();
  }
}
Types::__init();

We could optimize the emitted code by removing the declaration as well as the invocation if no such initializations exist, yes.

Ahh, I see. Very good. The application you're creating is indeed very well thought.

However, this doesn't meet my needs. My needs would require that the code that is transpiled can always be transpiled to a lower version, so for example if there is such a code:

class Types {
  private static $parser= new Parser(); // can't be easily transpiled
}

then I would need that the transpilation fails and says something "Cannot transpile"; that's what I was initially looking for.

That is not to say this is how your application must work, you will do what is best for the library; but that's just not what I needed right now.

I'm writing an application in a very low/old version. I would like to write in a higher one, but I can't upgrade the code, because of the continued support of the old version. I don't need type checks and runtime verification, because I have a very well designed suite of tests that I trust.

I would like to write the code in higher versions (like properties and what not), and then simply remove all the features that aren't present in the lower version (like properties), but I'd hate to add any additional logic like runtime checking or static initializer. If I add such code, I need the transpiler to end cricically and tell me "you can't use static initializer, because then we won't be able to transpile to lower version without additional runtime checks". This is my need. I don't expect you to implement it, if it isn't in the spirit of the library.

thekid commented 1 year ago

I was looking for a quick solution, to simply remove from code any structure that isn't supported in earlier versions

Hrm, OK, isn't this already pretty much addressed by the target version (supplied using -t)?

# Uses current PHP version (8.2 at the time of writing):
$ xp compile -q Types.php
<?php

 class Types{
private string $name;static function __init() {}} Types::__init();;

# Targets PHP 7.0
$ xp compile -q -t php:7.0 Types.php
<?php

 class Types{
private  $name;static function __init() {}} Types::__init();;

Everything that cannot be simulated in the specified PHP target version is removed1️⃣ - while features easily transpileable to lower versions are supported seamlessly. Some examples:

In the code you write, you do not need to know about any of this. You can go ahead and use any of the newer features, and then ship a library which will run in - say - PHP 7.1 (minus some of the extra security of e.g. runtime property type checks). For details, see https://github.com/xp-framework/compiler/wiki/Implementation-status


1️⃣ Quoting our design principles:

  • If in question, offer a degraded experience for older PHP versions
  • If in question, defer error handling to the PHP runtime instead of performing extensive checks
thekid commented 1 year ago

I'm writing an application in a very low/old version. I would like to write in a higher one, but I can't upgrade the code, because of the continued support of the old version. I don't need type checks and runtime verification, because I have a very well designed suite of tests that I trust.

Let me show you what I mean using your library https://github.com/t-regx/T-Regx/:

First steps

Change the autoloader to load classes from the dist directory and include the compiler as development dependency:

diff --git a/composer.json b/composer.json
index a2c8afc0..b87e2254 100644
--- a/composer.json
+++ b/composer.json
@@ -37,7 +37,7 @@
       "helper/helper.php"
     ],
     "psr-4": {
-      "TRegx\\": "src/"
+      "TRegx\\": "dist/"
     }
   },
   "autoload-dev": {
@@ -65,7 +65,8 @@
     "ext-json": "*",
     "phpunit/phpunit": "^9.5.4 || ^8.0 || ^7.5",
     "rawr/cross-data-providers": "^2.3.0",
-    "rawr/fqn-check": "^1.3.0"
+    "rawr/fqn-check": "^1.3.0",
+    "xp-framework/compiler": "dev-master as 8.9.0"
   },
   "archive": {
     "exclude": [

Run composer up, then transpile all sources to there, supplying the minimum PHP version you want to support (in this example, I'll use PHP 7.3, but you can go all the way down to PHP 7.0 if you wish):

$ xp compile -t php:7.3 -e .php -o dist/ src/
# ...
♥ Compiled 339 file(s) to dist using lang.ast.emit.PHP71, 0 error(s) occurred
Memory used: 2937.78 kB (3109.29 kB peak)
Time taken: 0.910 seconds

Verification

Now we want to see whether the transpiled code still works as expected:

$ php73 ./vendor/bin/phpunit test/
# ...
Time: 00:00.978, Memory: 36.00 MB

OK (3839 tests, 6029 assertions)

$ php82 ./vendor/bin/phpunit test/
# ...
Time: 00:00.903, Memory: 34.00 MB

OK (3867 tests, 6079 assertions)

Start rewriting your code

For example, by using first-class callable syntax available as of PHP 8.1:

diff --git a/src/SafeRegex/Internal/Bug.php b/src/SafeRegex/Internal/Bug.php
index fae86786..c1515273 100644
--- a/src/SafeRegex/Internal/Bug.php
+++ b/src/SafeRegex/Internal/Bug.php
@@ -21,7 +21,7 @@ class Bug

     private static function mapArray(array $pattern): array
     {
-        return \array_map([Bug::class, 'map'], $pattern);
+        return \array_map(self::map(...), $pattern);
     }

     public static function fixArrayKeys(array $patterns): array

After transpiling, the test suite will continue to run seamlessly with older PHP versions. This way, you can modernize your library step by step.


There are some downsides:

  1. You need to add to your workflow is to invoke the transpiler after every source edit, hurting the edit-save-rerun paradigm of the PHP world.
  2. Because composer clones from GIT, your library's codebase will always have to include both src and dist, doubling the installed package size and causing double the diff for every commit.

One idea for no. 1 could be to use xp -watch . compile ..., but we could look into something like the JIT class loading implemented in the XP Framework integration for regular composer-based projects.

If we would run a postinstall hook to run the compiler, we could get around no. 2 - this could work with Composer Plugins but we would always need to bundle the complete XP Compiler in this case. See also https://github.com/phpstan/extension-installer#usage

thekid commented 1 year ago

we could look into something like the JIT class loading

Proof of concept

We need to replace the PSR-4 class loader with our own JIT, and change xp-framework/compiler from a dev-dependency to a runtime dependency.

diff --git a/composer.json b/composer.json
index a2c8afc0..a9f2bd72 100644
--- a/composer.json
+++ b/composer.json
@@ -34,11 +34,9 @@
   ],
   "autoload": {
     "files": [
-      "helper/helper.php"
-    ],
-    "psr-4": {
-      "TRegx\\": "src/"
-    }
+      "helper/helper.php",
+      "helper/jit.php"
+    ]
   },
   "autoload-dev": {
     "classmap": [
@@ -59,7 +57,8 @@
   },
   "require": {
     "php": ">=7.1.0",
-    "ext-mbstring": "*"
+    "ext-mbstring": "*",
+    "xp-framework/compiler": "dev-master as 8.9.0"
   },
   "require-dev": {
     "ext-json": "*",

The helper/jit.php file contains the following:

<?php

use io\File;
use lang\ast\{Language, Emitter, Tokens};

class JIT {
  private $sources, $target, $version, $lang, $emit;

  public function __construct($sources, $target, $version) {
    $this->sources= $sources;
    $this->target= $target;
    $this->version= $version;

    // Ensure target directory exists
    is_dir($target) || mkdir($target, 0755);
  }

  public function compile($source, $target) {

    // Lazy-init compiler
    if (null === $this->emit) {
      $this->lang= Language::named('PHP');
      $this->emit= Emitter::forRuntime("php:$this->version")->newInstance();
      foreach ($this->lang->extensions() as $extension) {
        $extension->setup($this->lang, $this->emit);
      }
    }

    $this->emit->write(
      $this->lang->parse(new Tokens($source))->stream(),
      $target->out()
    );
  }

  public function load($class) {
    foreach ($this->sources as $prefix => $source) {
      $l= strlen($prefix);
      if (0 !== strncmp($prefix, $class, $l)) continue;

      // Use flat filesystem structure inside target directory
      $fname= strtr(substr($class, $l), '\\', DIRECTORY_SEPARATOR);
      $source= $source.DIRECTORY_SEPARATOR.$fname.'.php';
      $target= $this->target.DIRECTORY_SEPARATOR.crc32(dirname($fname)).'-'.basename($fname).'.php';

      $m= filemtime($target);
      if (false === $m || $m < filemtime($source)) {
        $this->compile(new File($source), new File($target));
      }

      return include($target);
    }

    // Name does not match any prefix, delegate to next loader
    return false;
  }
}

spl_autoload_register([
  new JIT(
    ['TRegx\\' => 'src'],
    'dist',
    PHP_MAJOR_VERSION.'.'.PHP_MINOR_VERSION.'.'.PHP_RELEASE_VERSION,
  ),
  'load'
]);

It compiles classes just in time and saves them to the dist/ directory if their target file does not exist or is older than the source file. The good thing: It uses the current PHP runtime, so if a user is using an old PHP version, more code will be transpiled, for newer versions, less heavy lifting will be done.

Performance

Running the test suite with PSR-4:

Time: 00:00.953, Memory: 34.00 MB

OK (3867 tests, 6079 assertions)

real    0m1.389s
user    0m0.000s
sys     0m0.015s

Running the test suite with JIT class loader, first run:

Time: 00:01.466, Memory: 36.00 MB

OK (3867 tests, 6079 assertions)

real    0m2.642s
user    0m0.000s
sys     0m0.015s

Subsequent runs:

Time: 00:00.978, Memory: 34.00 MB

OK (3867 tests, 6079 assertions)

real    0m1.458s
user    0m0.000s
sys     0m0.000s

👉 Performance is almost identical after all files have been compiled.


I don't like the hardcoded prefixes and the target directory in the jit.php file, it would be great if we could somehow use values from composer.json for this. Also, the JIT class could come bundled with XP Compiler if we find it generic enough.

Integrating with composer would also give us the benefit of being able to compile the code as part of the library's installation, and not during runtime. While it's probably acceptable to write to the disk during composer install, users may not find trustworthy later on, and it may outright fail if we're inside a read-only filesystem (e.g., a :ro docker volume).

thekid commented 1 year ago

Integrating with composer would also give us the benefit of being able to compile the code as part of the library's installation, and not during runtime

Here's how that could work:

diff --git a/composer.json b/composer.json
index a2c8afc0..cd64dc59 100644
--- a/composer.json
+++ b/composer.json
@@ -36,7 +36,7 @@
     "files": [
       "helper/helper.php"
     ],
-    "psr-4": {
+    "jit": {
       "TRegx\\": "src/"
     }
   },
@@ -59,7 +59,8 @@
   },
   "require": {
     "php": ">=7.1.0",
-    "ext-mbstring": "*"
+    "ext-mbstring": "*",
+    "xp-framework/jit": "dev-main as 1.0.0"
   },
   "require-dev": {
     "ext-json": "*",

The compiler plugin creates a file vendor/jit.php with the contents from above and adds it to the autoloader. Its code is relatively straight forward and hooks into Composer's ScriptEvents::PRE_AUTOLOAD_DUMP phase. For details, see https://github.com/xp-framework/jit

When first run, composer asks whether to trust and allow xp-framework/jit, and remembers the user's choice to composer.json:

Do you trust "xp-framework/jit" to execute code and wish to enable it now?
(writes "allow-plugins" to composer.json) [y,n,d,?] y

The temporary directory looks as follows:

$ ls -al /tmp/jit.php-3791947204/ | head
total 1701
drwxr-xr-x+ 1 timmf timmf     0 Feb 19 21:18 .
drwxr-xr-x+ 1 timmf timmf     0 Feb 19 21:18 ..
-rwxr-xr-x  1 timmf timmf   847 Feb 19 21:18 1055188484-Candidates.php
-rwxr-xr-x  1 timmf timmf   253 Feb 19 21:18 1055188484-Condition.php
-rwxr-xr-x  1 timmf timmf  1014 Feb 19 21:18 1055188484-Cut.php
-rwxr-xr-x  1 timmf timmf   548 Feb 19 21:18 1055188484-Definition.php
-rwxr-xr-x  1 timmf timmf   691 Feb 19 21:18 1055188484-EmptyOptional.php
-rwxr-xr-x  1 timmf timmf  3861 Feb 19 21:18 1055188484-EntryPoints.php
-rwxr-xr-x  1 timmf timmf   835 Feb 19 21:18 1055188484-Filter.php

Also, while this works fine for the library itself, I assume it wouldn't work for a library or project requiring this library - I need to research this -> fixed in xp-framework/jit@1a13a1e


⚠️ Using autoload: jit makes this an easy-enough change from a usability point of view, and while all composer interactions will work, composer validate will not, spitting out the error message: autoload : invalid value (jit), must be one of psr-0, psr-4, classmap, files, exclude-from-classmap.

🔢 As we now use XP Compiler as a runtime dependency, we bring quite a bit of library code along: 3.0M (382K ast, 508K compiler, 2.1M core, 6K jit).