davidcole1340 / ext-php-rs

Bindings for the Zend API to build PHP extensions natively in Rust.
Apache License 2.0
585 stars 63 forks source link

Spurious macro errors during development #327

Open faassen opened 5 days ago

faassen commented 5 days ago

During development in VS Code with Rust analyzer, even if the build succeeds, I still get a lot of spurious errors in my editor:

"The#[php_module]macro must be called last to ensure functions and classes are registered.

Impls must be declared before you declare your startup function and module function

These go away once I restart Rust analyzer, but appear again as soon as I make any edits. This makes for an uncomfortable development experience, especially given that the build options ext-php-rs requires also seem to make cargo recompile a lot more than usual.

Does anyone else see these errors? Is there a way to fix this? I assume something is going wrong with the order in which macros are executed, and that somehow the #[php_module] macro is executed too early under some conditions.

Xenira commented 5 days ago

Guess it has to do with this:

These macros do abuse the fact that (at the moment) proc macro expansion seems to happen orderly, on one single thread. It has been stated many times that this order is undefined behaviour (see here), so these macros could break at any time with a rustc update (let's just keep our fingers crossed).

The macros abuse this fact by storing a global state, which stores information about all the constants, functions, methods and classes you have registered throughout your crate. It is then read out of the state in the function tagged with the #[php_module] attribute. This is why this function must be the last function in your crate.

In the case the ordering does change (or we find out that it already was not in order), the most likely solution will be having to register your PHP exports manually inside the #[php_module] function.

https://davidcole1340.github.io/ext-php-rs/macros/index.html

Have the same error, but only sometimes. A little luck is involved getting stuff to execute in the correct order (or thats my guess at least).

faassen commented 5 days ago

While the magic is nice when it works, for me my editor is full of errors basically most of the time, unless I restart rust analyzer. Do you use rust analyzer?

I looked into the API to see where there's a "no magic" way to do this, but I don't see yet how to register things by hand. I think it would be helpful to have a "less magic" mode where there's a high level way to register classes and functions.

Xenira commented 5 days ago

I am using rust analyzer as well. Also seems like it has gotten worse.

Both fixing the magic and a no magic way would be great. Will have a look after work.

faassen commented 4 days ago

Besides analyzer errors I am now also getting errors like this:

PHP Fatal error: Xee\SequenceIterator must be registered before Xee\Sequence in Unknown on line 0

I'm not sure whether it's related. Yesterday when I got it I thought I fixed it by moving the SequenceIterator registration above the Sequence definition, but today with additional changes, I'm getting this error more consistently. If I can't fix this reliably and this is indeed due to auto-registration order issues then this is a bigger problem than just the analyzer being flaky. I'm going to do some digging.

[edit] Changing the order of the Rust code wildly some more and recompiling again fixed it. For now. So I think this is due to the order of registration somehow. But obviously this isn't great.

[edit 2] Now spun off into #328

faassen commented 4 days ago

An interesting note from the docs:

Classes and constants are not registered in the get_module function. These are registered inside the extension startup function.

It's odd then that we are getting these interactions.