-
Notifications
You must be signed in to change notification settings - Fork 216
Conversation
library/init/data/int/basic.lean
Outdated
@@ -382,4 +417,10 @@ by rw -int.sub_nat_nat_eq_coe; exact sub_nat_nat_elim m n | |||
(λi n, by rw [nat.add_sub_cancel_left]; refl) | |||
(λi n, by rw [add_assoc, nat.sub_eq_zero_of_le (nat.le_add_right _ _)]; refl) | |||
|
|||
#eval do m ← list.range 5, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debugging code should not be in the library files.
library/init/data/int/bitwise.lean
Outdated
| (m : ℕ) n := nat.test_bit m n | ||
| -[1+ m] n := bnot (nat.test_bit m n) | ||
|
||
def nat_bitwise (f : bool → bool → bool) (m n : ℕ) : ℤ := |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function (and bitwise
) doesn't seem to be used anywhere. Is there a reason we can't use it for lor
, land
, etc.?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm torn on whether the definitional equality should be the reduction into positive/negative cases as I've done here, or the theorems bitwise or = lor
etc. Either way we will have both, but definitional reduction can be important here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, they are used in the theorems at the end of the file.
library/init/data/nat/bitwise.lean
Outdated
|
||
def div2 (n : ℕ) : ℕ := (bodd_div2 n).2 | ||
|
||
def bodd (n : ℕ) : bool := (bodd_div2 n).1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was wrong with n % 2 = 1
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dependence on WF definitions makes it more complicated for the kernel to reduce the definition. I didn't think it would be important, but it kept breaking proofs in weird ways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't think it would be important, but it kept breaking proofs in weird ways.
Do you still remember what it broke? As far as I can tell, bodd_div2 n
only reduces if n
is a numeral. But in this case I would have expected the WF definition to reduce as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not that it can't reduce; it reduces but only when unfold_lemmas
is enabled. This causes a problem for binary_rec_eq
, because the type of the theorem includes f ff 0 z = z
(where the left side has type C (bit ff 0)
and the right side has type C 0
), so that any users of this theorem must also have unfold_lemmas
enabled in order to reduce bit ff 0 := 0
. If they don't, we are in the strange circumstance of having equalities embedded in constants whose type-correctness is not verifiable, and I have no idea what kind of guarantees lean can give in this situation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oooh, I see. Yes, this is really, really ugly. And we will probably see this again in the future, since it happens with every WF definition...
I see two potential solutions:
- Use equational rfl-lemmas for reduction in the type_context. This is a bit of work, and Leo has mentioned that there are lots of issues here.
- As a quick hack, we could special-case
acc.rec
(and maybeeq.rec
andheq.rec
?) in the type_context by reducing the major premise usingtransparency_mode::All
.
BTW, we have exactly the same issue with attribute [irreducible]
.
library/init/data/nat/gcd.lean
Outdated
|
||
/- gcd -/ | ||
|
||
def gcd.F : Π (y : ℕ), (Π (y' : ℕ), y' < y → nat → nat) → nat → nat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we define gcd
using the equation compiler now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. We should probably also define div
and mod
using the equation compiler.
BTW, this PR doesn't build. |
Errors in the last commit were fixed, and it has been build-tested this time. |
library/init/data/nat/bitwise.lean
Outdated
|
||
def div2 (n : ℕ) : ℕ := (bodd_div2 n).2 | ||
|
||
def bodd (n : ℕ) : bool := (bodd_div2 n).1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't think it would be important, but it kept breaking proofs in weird ways.
Do you still remember what it broke? As far as I can tell, bodd_div2 n
only reduces if n
is a numeral. But in this case I would have expected the WF definition to reduce as well.
library/init/data/nat/bitwise.lean
Outdated
@[simp] lemma bodd_one : bodd 1 = tt := rfl | ||
@[simp] lemma bodd_two : bodd 2 = ff := rfl | ||
|
||
@[simp] def bodd_succ (n : ℕ) : bodd (succ n) = bnot (bodd n) := |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/def/lemma/g
library/init/data/int/bitwise.lean
Outdated
| (m : ℕ) n := nat.test_bit m n | ||
| -[1+ m] n := bnot (nat.test_bit m n) | ||
|
||
def nat_bitwise (f : bool → bool → bool) (m n : ℕ) : ℤ := |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, they are used in the theorems at the end of the file.
@digama0 This PR still has build errors. |
I think this PR is ready for merge (or at least ready for others to try to find issues with it) now. |
517a8c2
to
c93b2c7
Compare
@gebner Is the "request changes" finished? |
The type-correctness of binary_rec_eq (the statement, not the proof) depends on unfolding the embedded well-founded definition of mod. This definition avoids it by using two simpler functions bodd and div2 that reduce well in the kernel.
I still think that bodd_div2 is hideous, but I get why we need it. We should define |
@gebner Actually the latest version of this PR does define The other reason for |
Merged based on @gebner approval. |
Expansion of the previous commit to include VM implementations of integer bitwise operations.