search

0xPolygonZero / plonky2

Apache License 2.0
757 stars 283 forks source link

`PolynomialBatch::from_values` takes refs #1392

Closed dvdplm closed 9 months ago

dvdplm commented 9 months ago

Fixes a TODO in the code to make PolynomialBatch::from_values take PolynomialValues by ref instead of by value. Doing this doesn't really fix anything given that the subsequent FFT operations require ownership of the values. Still, I think it's better to have the clone happen in a single place than spread all over.

sonarcloud[bot] commented 9 months ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

dvdplm commented 9 months ago

I think that after this investigation it's clear that it'd require a pretty big re-architecting to fix this so if anything there could be a ticket to look into what the best way to go about that is. And measure if it's actually a problem. To me, a TODO: should be about small things and local to the place in the code where they appear (and preferably always be matched with a ticket nr), otherwise it just becomes misleading. It's like having a TODO: Make plonky2 fast, which is a true todo but also not something that belong to a single line of code or even single source file.