-
Notifications
You must be signed in to change notification settings - Fork 58
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
Unsafe ops for fq_default #1906
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1906 +/- ##
==========================================
+ Coverage 87.25% 87.27% +0.01%
==========================================
Files 97 97
Lines 35651 35640 -11
==========================================
- Hits 31109 31105 -4
+ Misses 4542 4535 -7 ☔ View full report in Codecov by Sentry. |
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.
looks mostly good. some comments to the .poly
thingy
+(x::FqFieldElem, y::$jT) = x + parent(x)(y) | ||
+(x::$jT, y::FqFieldElem) = y + x | ||
|
||
-(x::FqFieldElem, y::$jT) = x - parent(x)(y) | ||
-(x::$jT, y::FqFieldElem) = parent(y)(x) - y |
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.
shouldn't these delegate to add!
or sub!
as well?
Edit: I just noticed that there are no corresponding flint functions for these, but we have fallbacks in place in AA.
Edit: but this would be an endless loop...
maybe just add a comment here that these wait for a change in FLINT?
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.
We could implement add!
and sub!
already now without an endless loop by inserting the coercions there. But there are already tons of changes and I have to draw the line somewhere. So here I just took the code that was there, the only change is that it is now in an for ... @begin eval
construct.
Someone can make these additional changes in a future iteration. There are a gazillion other things one could do in this file alone (e.g. adding set!
methods to simplify the constructors). For now I'd like to move on
Co-authored-by: Lars Göttgens <lars.goettgens@rwth-aachen.de>
No description provided.