Skip to content

Commit

Permalink
feat: merge and sort imports (#6322)
Browse files Browse the repository at this point in the history
# Description

## Problem

Manually arranging imports is time consuming.

## Summary

Added two new configurations, `reorder_imports` (true by default) and
`imports_granularity` (`Preserve` by default), similar to the ones found
in rustfmt. With the default configuration imports are reorganized
alphabetically but not merged.

While implementing this I bumped into one code in the test programs like
this:

```noir
use foo;
use foo::bar;
```

Merging those in Rust ends up like this:

```noir
use foo::{bar, self};
```

We don't have `self` in imports, so I had two choices:
1. Try to leave the above unmodified.
2. Implement `self` in imports.

It turned out that implement `self` in imports is very easy: when we
desugar imports, if the last segment is "self" we just remove it. Then
`use foo::self` is desugared into `use foo`.

## Additional Context

None.

## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Tom French <[email protected]>
  • Loading branch information
asterite and TomAFrench authored Oct 23, 2024
1 parent 5a6dae9 commit 07ab515
Show file tree
Hide file tree
Showing 41 changed files with 953 additions and 79 deletions.
4 changes: 3 additions & 1 deletion compiler/noirc_frontend/src/ast/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,9 @@ impl UseTree {

match self.kind {
UseTreeKind::Path(name, alias) => {
vec![ImportStatement { visibility, path: prefix.join(name), alias }]
// Desugar `use foo::{self}` to `use foo`
let path = if name.0.contents == "self" { prefix } else { prefix.join(name) };
vec![ImportStatement { visibility, path, alias }]
}
UseTreeKind::List(trees) => {
let trees = trees.into_iter();
Expand Down
15 changes: 15 additions & 0 deletions compiler/noirc_frontend/src/parser/parser/use_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,14 @@ impl<'a> Parser<'a> {
fn parse_use_tree_in_list(&mut self) -> Option<UseTree> {
let start_span = self.current_token_span;

// Special case: "self" cannot be followed by anything else
if self.eat_self() {
return Some(UseTree {
prefix: Path { segments: Vec::new(), kind: PathKind::Plain, span: start_span },
kind: UseTreeKind::Path(Ident::new("self".to_string(), start_span), None),
});
}

let use_tree = self.parse_use_tree_without_kind(
start_span,
PathKind::Plain,
Expand Down Expand Up @@ -250,4 +258,11 @@ mod tests {
let (_, errors) = parse_program(src);
assert!(!errors.is_empty());
}

#[test]
fn errors_on_double_colon_after_self() {
let src = "use foo::{self::bar};";
let (_, errors) = parse_program(src);
assert!(!errors.is_empty());
}
}
22 changes: 22 additions & 0 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3389,3 +3389,25 @@ fn arithmetic_generics_rounding_fail() {
let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);
}

#[test]
fn uses_self_in_import() {
let src = r#"
mod moo {
pub mod bar {
pub fn foo() -> i32 {
1
}
}
}
use moo::bar::{self};
pub fn baz() -> i32 {
bar::foo()
}
fn main() {}
"#;
assert_no_errors(src);
}
2 changes: 1 addition & 1 deletion noir_stdlib/src/array/check_shuffle.nr
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ where
}

mod test {
use super::check_shuffle;
use crate::cmp::Eq;
use super::check_shuffle;

struct CompoundStruct {
a: bool,
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/bigint.nr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::ops::{Add, Sub, Mul, Div};
use crate::cmp::Eq;
use crate::ops::{Add, Div, Mul, Sub};

global bn254_fq = &[
0x47, 0xFD, 0x7C, 0xD8, 0x16, 0x8C, 0x20, 0x3C, 0x8d, 0xca, 0x71, 0x68, 0x91, 0x6a, 0x81, 0x97,
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/cmp.nr
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ where
}

mod cmp_tests {
use crate::cmp::{min, max};
use crate::cmp::{max, min};

#[test]
fn sanity_check_min() {
Expand Down
6 changes: 3 additions & 3 deletions noir_stdlib/src/collections/map.nr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::cmp::Eq;
use crate::option::Option;
use crate::default::Default;
use crate::hash::{Hash, Hasher, BuildHasher};
use crate::collections::bounded_vec::BoundedVec;
use crate::default::Default;
use crate::hash::{BuildHasher, Hash, Hasher};
use crate::option::Option;

// We use load factor alpha_max = 0.75.
// Upon exceeding it, assert will fail in order to inform the user
Expand Down
4 changes: 2 additions & 2 deletions noir_stdlib/src/collections/umap.nr
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::cmp::Eq;
use crate::option::Option;
use crate::default::Default;
use crate::hash::{Hash, Hasher, BuildHasher};
use crate::hash::{BuildHasher, Hash, Hasher};
use crate::option::Option;

// An unconstrained hash table with open addressing and quadratic probing.
// Note that "unconstrained" here means that almost all operations on this
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/ec/consts/te.nr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::ec::tecurve::affine::Point as TEPoint;
use crate::ec::tecurve::affine::Curve as TECurve;
use crate::ec::tecurve::affine::Point as TEPoint;

pub struct BabyJubjub {
pub curve: TECurve,
Expand Down
10 changes: 5 additions & 5 deletions noir_stdlib/src/ec/montcurve.nr
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@ pub mod affine {
// Points are represented by two-dimensional Cartesian coordinates.
// All group operations are induced by those of the corresponding Twisted Edwards curve.
// See e.g. <https://eprint.iacr.org/2017/212.pdf> for details on the correspondences.
use crate::cmp::Eq;
use crate::ec::is_square;
use crate::ec::montcurve::curvegroup;
use crate::ec::safe_inverse;
use crate::ec::sqrt;
use crate::ec::swcurve::affine::Curve as SWCurve;
use crate::ec::swcurve::affine::Point as SWPoint;
use crate::ec::tecurve::affine::Curve as TECurve;
use crate::ec::tecurve::affine::Point as TEPoint;
use crate::ec::is_square;
use crate::ec::safe_inverse;
use crate::ec::sqrt;
use crate::ec::ZETA;
use crate::cmp::Eq;

// Curve specification
pub struct Curve { // Montgomery Curve configuration (ky^2 = x^3 + j*x^2 + x)
Expand Down Expand Up @@ -222,12 +222,12 @@ pub mod curvegroup {
// Points are represented by three-dimensional projective (homogeneous) coordinates.
// All group operations are induced by those of the corresponding Twisted Edwards curve.
// See e.g. <https://eprint.iacr.org/2017/212.pdf> for details on the correspondences.
use crate::cmp::Eq;
use crate::ec::montcurve::affine;
use crate::ec::swcurve::curvegroup::Curve as SWCurve;
use crate::ec::swcurve::curvegroup::Point as SWPoint;
use crate::ec::tecurve::curvegroup::Curve as TECurve;
use crate::ec::tecurve::curvegroup::Point as TEPoint;
use crate::cmp::Eq;

pub struct Curve { // Montgomery Curve configuration (ky^2 z = x*(x^2 + j*x*z + z*z))
pub j: Field,
Expand Down
8 changes: 4 additions & 4 deletions noir_stdlib/src/ec/swcurve.nr
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ pub mod affine {
// Points are represented by two-dimensional Cartesian coordinates.
// Group operations are implemented in terms of those in CurveGroup (in this case, extended Twisted Edwards) coordinates
// for reasons of efficiency, cf. <https://en.wikibooks.org/wiki/Cryptography/Prime_Curve/Jacobian_Coordinates>.
use crate::ec::swcurve::curvegroup;
use crate::ec::safe_inverse;
use crate::cmp::Eq;
use crate::ec::is_square;
use crate::ec::safe_inverse;
use crate::ec::sqrt;
use crate::cmp::Eq;
use crate::ec::swcurve::curvegroup;

// Curve specification
pub struct Curve { // Short Weierstrass curve
Expand Down Expand Up @@ -190,8 +190,8 @@ pub mod curvegroup {
// CurveGroup representation of Weierstrass curves
// Points are represented by three-dimensional Jacobian coordinates.
// See <https://en.wikibooks.org/wiki/Cryptography/Prime_Curve/Jacobian_Coordinates> for details.
use crate::ec::swcurve::affine;
use crate::cmp::Eq;
use crate::ec::swcurve::affine;

// Curve specification
pub struct Curve { // Short Weierstrass curve
Expand Down
8 changes: 4 additions & 4 deletions noir_stdlib/src/ec/tecurve.nr
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ pub mod affine {
// Group operations are implemented in terms of those in CurveGroup (in this case, extended Twisted Edwards) coordinates
// for reasons of efficiency.
// See <https://eprint.iacr.org/2008/522.pdf> for details.
use crate::ec::tecurve::curvegroup;
use crate::cmp::Eq;
use crate::ec::montcurve::affine::Curve as MCurve;
use crate::ec::montcurve::affine::Point as MPoint;
use crate::ec::swcurve::affine::Curve as SWCurve;
use crate::ec::swcurve::affine::Point as SWPoint;
use crate::cmp::Eq;
use crate::ec::tecurve::curvegroup;

// Curve specification
pub struct Curve { // Twisted Edwards curve
Expand Down Expand Up @@ -197,12 +197,12 @@ pub mod curvegroup {
// CurveGroup coordinate representation of Twisted Edwards curves
// Points are represented by four-dimensional projective coordinates, viz. extended Twisted Edwards coordinates.
// See section 3 of <https://eprint.iacr.org/2008/522.pdf> for details.
use crate::ec::tecurve::affine;
use crate::cmp::Eq;
use crate::ec::montcurve::curvegroup::Curve as MCurve;
use crate::ec::montcurve::curvegroup::Point as MPoint;
use crate::ec::swcurve::curvegroup::Curve as SWCurve;
use crate::ec::swcurve::curvegroup::Point as SWPoint;
use crate::cmp::Eq;
use crate::ec::tecurve::affine;

// Curve specification
pub struct Curve { // Twisted Edwards curve
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/eddsa.nr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::default::Default;
use crate::ec::consts::te::baby_jubjub;
use crate::ec::tecurve::affine::Point as TEPoint;
use crate::hash::Hasher;
use crate::hash::poseidon::PoseidonHasher;
use crate::default::Default;

// Returns true if signature is valid
pub fn eddsa_poseidon_verify(
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/embedded_curve_ops.nr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::ops::arith::{Add, Sub, Neg};
use crate::cmp::Eq;
use crate::ops::arith::{Add, Neg, Sub};

/// A point on the embedded elliptic curve
/// By definition, the base field of the embedded curve is the scalar field of the proof system curve, i.e the Noir Field.
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/field/bn254.nr
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ pub fn lt(a: Field, b: Field) -> bool {
mod tests {
// TODO: Allow imports from "super"
use crate::field::bn254::{
decompose, compute_lt, assert_gt, gt, TWO_POW_128, compute_lte, PLO, PHI,
assert_gt, compute_lt, compute_lte, decompose, gt, PHI, PLO, TWO_POW_128,
};

#[test]
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/field/mod.nr
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
pub mod bn254;
use bn254::lt as bn254_lt;
use crate::runtime::is_unconstrained;
use bn254::lt as bn254_lt;

impl Field {
/// Asserts that `self` can be represented in `bit_size` bits.
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/hash/mimc.nr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::hash::Hasher;
use crate::default::Default;
use crate::hash::Hasher;

// mimc-p/p implementation
// constants are (publicly generated) random numbers, for instance using keccak as a ROM.
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/hash/mod.nr
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ pub mod sha256;
pub mod sha512;

use crate::default::Default;
use crate::uint128::U128;
use crate::embedded_curve_ops::{
EmbeddedCurvePoint, EmbeddedCurveScalar, multi_scalar_mul, multi_scalar_mul_array_return,
};
use crate::meta::derive_via;
use crate::uint128::U128;

// Kept for backwards compatibility
pub use sha256::{digest, sha256, sha256_compression, sha256_var};
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/hash/poseidon/bn254/consts.nr
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Used like so: sage generate_parameters_grain.sage 1 0 254 2 8 56 0x30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000001
// Constants for various Poseidon instances in the case of the prime field of the same order as BN254.
// Consistent with https://github.com/iden3/circomlib/blob/master/circuits/poseidon.circom and https://github.com/iden3/circomlib/blob/master/circuits/poseidon_constants.circom
use crate::hash::poseidon::PoseidonConfig;
use crate::hash::poseidon::config;
use crate::hash::poseidon::PoseidonConfig;
// S-box power
fn alpha() -> Field {
5
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/hash/poseidon/mod.nr
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
pub mod bn254; // Instantiations of Poseidon for prime field of the same order as BN254
use crate::hash::Hasher;
use crate::default::Default;
use crate::hash::Hasher;

// A config struct defining the parameters of the Poseidon instance to use.
//
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/hash/poseidon2.nr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::hash::Hasher;
use crate::default::Default;
use crate::hash::Hasher;

comptime global RATE: u32 = 3;

Expand Down
4 changes: 2 additions & 2 deletions noir_stdlib/src/meta/expr.nr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//! Contains methods on the built-in `Expr` type for quoted, syntactically valid expressions.

use crate::option::Option;
use crate::meta::op::UnaryOp;
use crate::meta::op::BinaryOp;
use crate::meta::op::UnaryOp;
use crate::option::Option;

impl Expr {
/// If this expression is an array literal `[elem1, ..., elemN]`, this returns a slice of each element in the array.
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/meta/trait_constraint.nr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::hash::{Hash, Hasher};
use crate::cmp::Eq;
use crate::hash::{Hash, Hasher};

impl Eq for TraitConstraint {
comptime fn eq(self, other: Self) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/meta/trait_def.nr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::hash::{Hash, Hasher};
use crate::cmp::Eq;
use crate::hash::{Hash, Hasher};

impl TraitDefinition {
#[builtin(trait_def_as_trait_constraint)]
Expand Down
4 changes: 2 additions & 2 deletions noir_stdlib/src/ops/mod.nr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
pub(crate) mod arith;
pub(crate) mod bit;

pub use arith::{Add, Sub, Mul, Div, Rem, Neg};
pub use bit::{Not, BitOr, BitAnd, BitXor, Shl, Shr};
pub use arith::{Add, Div, Mul, Neg, Rem, Sub};
pub use bit::{BitAnd, BitOr, BitXor, Not, Shl, Shr};
4 changes: 2 additions & 2 deletions noir_stdlib/src/option.nr
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::hash::{Hash, Hasher};
use crate::cmp::{Ordering, Ord, Eq};
use crate::cmp::{Eq, Ord, Ordering};
use crate::default::Default;
use crate::hash::{Hash, Hasher};

pub struct Option<T> {
_is_some: bool,
Expand Down
12 changes: 6 additions & 6 deletions noir_stdlib/src/prelude.nr
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
pub use crate::collections::vec::Vec;
pub use crate::collections::bounded_vec::BoundedVec;
pub use crate::option::Option;
pub use crate::{print, println, assert_constant};
pub use crate::uint128::U128;
pub use crate::{assert_constant, print, println};
pub use crate::cmp::{Eq, Ord};
pub use crate::default::Default;
pub use crate::collections::bounded_vec::BoundedVec;
pub use crate::collections::vec::Vec;
pub use crate::convert::{From, Into};
pub use crate::default::Default;
pub use crate::meta::{derive, derive_via};
pub use crate::option::Option;
pub use crate::panic::panic;
pub use crate::uint128::U128;
4 changes: 2 additions & 2 deletions noir_stdlib/src/uint128.nr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::ops::{Add, Sub, Mul, Div, Rem, Not, BitOr, BitAnd, BitXor, Shl, Shr};
use crate::cmp::{Eq, Ord, Ordering};
use crate::ops::{Add, BitAnd, BitOr, BitXor, Div, Mul, Not, Rem, Shl, Shr, Sub};

global pow64: Field = 18446744073709551616; //2^64;
global pow63: Field = 9223372036854775808; // 2^63;
Expand Down Expand Up @@ -313,7 +313,7 @@ impl Shr for U128 {
}

mod tests {
use crate::uint128::{U128, pow64, pow63};
use crate::uint128::{pow63, pow64, U128};

#[test]
fn test_not(lo: u64, hi: u64) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ use std::ec::tecurve::curvegroup::Point as G;
use std::ec::swcurve::affine::Point as SWGaffine;
use std::ec::swcurve::curvegroup::Point as SWG;

use std::compat;
use std::ec::montcurve::affine::Point as MGaffine;
use std::ec::montcurve::curvegroup::Point as MG;
use std::compat;

fn main() {
// This test only makes sense if Field is the right prime field.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::foo::in_foo_mod;
use crate::foo::bar::in_bar_mod;
use crate::baz::in_baz_mod;
use crate::foo::bar::in_bar_mod;
use crate::foo::in_foo_mod;

mod foo;
mod baz;
Expand Down
2 changes: 1 addition & 1 deletion test_programs/compile_success_empty/reexports/src/main.nr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use reexporting_lib::{FooStruct, MyStruct, lib};
use reexporting_lib::{FooStruct, lib, MyStruct};

fn main() {
let x: FooStruct = MyStruct { inner: 0 };
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::hash::Hasher;
use std::hash::poseidon2::Poseidon2Hasher;
use std::hash::poseidon::PoseidonHasher;
use std::hash::poseidon2::Poseidon2Hasher;

fn main(x: Field, y: pub Field) {
let mut hasher = PoseidonHasher::default();
Expand Down
Loading

0 comments on commit 07ab515

Please sign in to comment.