proper-testing / proper

PropEr: a QuickCheck-inspired property-based testing tool for Erlang
http://proper-testing.github.io
GNU General Public License v3.0
882 stars 167 forks source link

`proper_transformer` has unfavorable behavior for fine-grained compile dependencies #288

Open TheGeorge opened 2 years ago

TheGeorge commented 2 years ago

Problem

When applying the parse_transform, PropEr accesses the abstract code of referenced modules, to determine if calls lead to exported types or not (and branches based on that decision).

This creates additional build dependencies on modules using proper_transformer. Typically, calls don't need to be considered when building beams, as they are resolved dynamically during run time.

Proposed Solution

PropEr should be lazy here when accessing the abstract code of referenced modules.

Why is this important?

With a build system that carefully tracks compile dependencies per-file, it is important to have a robust and quick way to determine these dependencies, and to keep dependencies as flat as possible.

Unfortunately, parse_transforms can dynamically pull in additional dependencies during compile time.

kostis commented 2 years ago

I do not understand the issue at all, let alone why it is important or a problem in some particular context.

In my mind, at least, a parse transform is, as its name implies, by definition something that happens at compile time. What does it mean that a parse transform should be lazy? What does it mean to not do parse transform stuff at compile time? What sort of build system carefully tracking compile dependencies are you referring to?

Confused...

TheGeorge commented 2 years ago

A parse_transform shoudn't be lazy, I wouldn't know what that should be, since they are invoked by the compiler.

The problem with the way PropEr handles external types is that it's essentially not possible to determine compile time dependencies correctly with it (at least not without encoding specifics of proper_transformer into the build system).

Let's look at an example:

-module(example).

-include_lib("proper/include/proper.hrl").

prop_example() ->
    ?FORALL(X, my_module:my_type(),  ...).

Just looking at the file, I know that I need to essentially compile proper before to build this file. But that's not the entirety that I need to build. Because proper_transformer will while compiling the file, also try to access the object code. This only works however, if the file has been compiled before to beam. So the build system needs to know this and build the file before.

Since this all happens while executing the parse_transform, it's essentially not possible to extract this information statically (at least not within the constraints of a build system)

For completeness, there is also a path, where this information is tried to be extracted, by compiling the sources, but that path is more "best effort" and easily fails due to missing compiler options, that the build system would provide.

When I mentioned lazy here, I meant, that PropEr should do this lookup during runtime only, not compile time. Because we can assume that during runtime all code that we care about has been built and is in path.

kostis commented 2 years ago

Note that what is being discussed/requested in this issue is not PropEr-specific in any way. You are effectively asking for the functionality of Erlang's parse_transform compiler option to limit itself to whatever functionality a (fictional at this point) build system is able to (easily) provide or accommodate. Even if this happens for PropEr in some way (e.g. by a user-provided pull request that "fixes" this), note that -in my opinion, at least- this is not a viable solution (for the Erlang ecosystem) in the long run.

Even if PropEr is the only code base doing weird things with a parse_transform at this point in time, sooner or later, another programmer may decide to do something cute with a parse_transform that breaks whatever implicit assumptions a build system may be depending on. Note that parse transformations allow programmers the use of Turing complete language (Erlang) where all sorts of weird things can be coded, not just accessing object files.

In my opinion, proper(TM) build systems for Erlang should take this into account and not (arbitrarily?) constrain what it is possible to express currently. Anyway, these are my thoughts on this.

If you (or anybody else) are interested in fixing this issue, you are welcome to submit a PR that changes the current way PropEr currently disambiguates whether a call mod:foo() in some "known" position (e.g., in the second argument of a ?FORALL) refers to an exported type (and therefore should be treated specially when expanding the ?FORALL macro) or to a function in module mod (and should be left unchanged). That PR will be of course reviewed and if it does not have any drawbacks it will be merged. But note that this will not solve the general issue with parse_transforms in Erlang.

TheGeorge commented 2 years ago

In my opinion, proper(TM) build systems for Erlang should take this into account and not (arbitrarily?) constrain what it is possible to express currently. Anyway, these are my thoughts on this.

In the general case, this is not possible for the build system to decide though. Think about the case where the parse_transform is opening a TCP port and receiving which modules/files to read from. I agree that this is an issue of parse_transforms in itself.

The way how build systems solve this is to put it on the user to provide this information.

I'm planning on making a PR, and am happy to chat about the issue more :-)