AmpersandTarski / Ampersand

Build database applications faster than anyone else, and keep your data pollution free as a bonus.
http://ampersandtarski.github.io/
GNU General Public License v3.0
40 stars 8 forks source link

Test Fspec in regression for bug prevention #1385

Open stefjoosten opened 1 year ago

stefjoosten commented 1 year ago

In issue #1380 we discovered that it would be useful to do an equality check on the data structure FSpec in the regression, to detect a large category of possible deep mistakes. For example, this would have saved us issue #1380 in the first place.

A complication is that FSpec contains functions. So this is not entirely straightforward. Hence this issue, so we won't forget.

hanjoosten commented 1 year ago

fDeriveProofs is the culprit

Looking back to the original issue, the solution is not to make FSpec an instance of Eq. However, we need all fields of FSpec to be strict. Currently, all of these fields, except for fDeriveProofs are strict. fDeriveProofs is problematic. It cannot be made strict, because of a bug in the normalizer.

If this last field is made strict, the ampersand generator will not terminate any longer. I am not sure if this holds for all scripts, but there certainly are scripts that trigger a loop (non-termination). I have done some investigation on this in the past, and concluded that we had nontermination in the normalizer. (see commit https://github.com/AmpersandTarski/Ampersand/commit/e361594abeee1dbbad6468345375a8104bdcebfc for details)

@sjcjoosten , I remember you saying that you wanted to look into this issue? If so, please have a look.

sjcjoosten commented 1 year ago

I don't think making all FSpec fields strict is a solution for the problem of testing. A downside of making FSpec strict is that it could potentially make the compiler much slower for tasks that are fast and easy (such as only type checking). FSpec is meant to be calculated lazily.

I would also agree with Han that making FSpec an instance of Eq would be using the wrong tool to achieve testability.

I'd prefer making FSpec an instance of NFData (https://hackage.haskell.org/package/deepseq-1.4.1.1/docs/Control-DeepSeq.html#t:NFData ) by deriving it. This would also allow FSpec to contain functions (a -> b is an instance of NFData too)

Regardless, we all agree that fDeriveProofs should terminate, and before we can go with any of the now three proposed solutions, we need to fix that. I'd be happy to take a look some time next week.