-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug in higher-order matching #225
Comments
The issue might come from the matching of an abstraction against an applied pattern variable. The current lambdapi rejects it as line 176 of eval.ml suggests. |
This issue suggests that even in the old syntax, LambdaPi performs syntactic matching rather than matching modulo beta. |
Does your PR @gabrielhdt fixes this issue? |
Hum for now it fails to build a tree, precisely because of this case. It should be easy to fix though. |
The decision trees branch now behave like the master branch, that is, it raises
|
Alright, but the file is well-typed, so there is still a bug. |
The error message is weird, since the line which raises the problem is the line 46 of |
@GuillaumeGen Can you detail your comment? Why do you think it depends on the definition of "matching"? |
I can try to have a look later, but you should definitely try to make the example more minimal (just inline |
As far as I understand, LambdaPi implements AFSM (or something close to it). Hence there are two kind of application, the usual one |
@rlepigre Here you go
|
Maybe I should mention also that this example comes from the encoding of a type system called
The original file I have provided in this issue aims to encode this logic. Nat is an example that this encoding seems to work. The main benefit of From my understanding, PML is not that far from |
Remark: the file is checked if we replace the first rule defining code by
|
So it is indeed a Higher-Order issue. In my case though, I don't want to use this rule since it does an eta-expansion. |
No. |
And this rule is more general since it applies even if the argument is not a lambda. |
In Cedille it does ^^ . |
How? |
Because using this rule, you maybe have two terms |
The "normal form" computed by LP for
This is not a normal form. It seems that |
This is indeed the case as shown by the following example:
|
@fblanqui you are right about that, your explanation is correct. The environment |
Will it be easy to fix? It seems that we cannot use is_closed. Instead we need to check that some variables do not occur. Is it possible to do this efficiently with Bindlib? |
But is that really a bug though? For me this looks like some variable is gonna escape its scope here. |
If you have a rule
it matches the term When you encounter the rule
You cannot assume, for the same reason that any term which matches |
I have a branch with dtrees that solves Frederic subproblem, but not Francois' |
@rlepigre , this is the difference between programming languages and proof assistants: in proof assistants, we want to compute strong normal forms, hence reduce under lambda's. |
Will be fixed by merging #172. |
Let's not close until the bug has been fixed in master (I'll take care of that now), especially since #172 is not ready to be merged in my opinion. I'll review it right after I fix master. |
Could you please explain why Gabriel's branch does not solve the problem? Do you have an example? |
In fact I'm not sure, I'm still trying to understand what is happening. There are several issues going on at the same time. Solving the issue naively may lead to free variables escaping their scope if we are not careful. For instance, consider the following example.
What used to happen here was that With the approach that I am considering now, this issue is solved. However, I now run into the problem that Does Dedukti do anything special to enforce that free variables do not escape their scope @francoisthire? |
Maybe @barras or @Gaspi will give a better answer than mine. But in Dedukti you can But here, I think we try also to solve this problem by computing the snf of the argument if the matching failed beforehand. |
In my branch, |
@gabrielhdt it is very wrong because reducing |
@francoisthire my example comes from a simplification of the |
Rodolphe, I believe that there is a misunderstanding about what Gabriel said. Gabriel, correct me if I am wrong but I think that you said that, currently, in your branch, |
That is indeed what I meant. However, this is what I thought I implemented, and in fact, I just found a bug in some auxiliary function. I'm fixing this. Regarding the conditional test, I intended to do this, but some tests failed, namely the |
solves issue mentioned in thread of Deducteam#225
So as of commit 4e458b7, the expected behaviour announced by Frederic operates. Additionally, I've made clearer what I stated above, see here and there. In both lines, one condition translates "check for free variables if some are forbidden", the other part forces the variable check in case of permutation of free variables as shown there. |
@gabrielhdt What about efficiency in your branch? Have you tested |
I made a mistake in my answer. I thought that |
@rlepigre on verine, performance does not change, but I haven't fixed the issue as you have done, using |
@gabrielhdt OK, no worries. I started to give comments on your PR, but I'll do that in several waves. It would be nice to merge it soon, and solve the problems at the same time. I don't want to spend time optimizing the version I fixed, it is probably better to finish the work on decision trees. |
Is it possible to get a behavior similar to the current one once @gabrielhdt PR will be merged? |
wdym? |
@gabrielhdt I don't understand why you have to check variable occurrences when there is a permutation. In abstractions.lp there should be no test on variables. |
@fblanqui in fact it is a trivial variable check, but I need to remap the right variables into the term, and this is done by a variable check. |
I don't understand. If you take |
I was mistaken, it has nothing to do with permutation. However, during evaluation, I unbind terms using variables created at compile time. In the end, I must gather the free variables introduced and bind them into right hand side. This is currently done by a variable check, which performs a |
Yes, it would be better to separate the two. |
Commit 2494410 separates the two, and no free var check is done in |
I am sorry, I don't have time to minimize the example. I am aware of a type checking issue in Lambdapi which is probably related to Higher-Order matching. The error can be found in the Librairies repository. There are two files in play. The first one encodes Cedille's core theory: cedille.dk . The second one encodes a basic example with it: nat.dk.
The first file type checks but not the second in Lambdapi (but it does in Dedukti and Dedukti is right on this one). I know that the error commes from either this rule or this rule which are both Higher-Order rules. One of these rules is not applied while it should.
The text was updated successfully, but these errors were encountered: