Skip to content
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

feat(cli)!: Add --unstable-features to gate unstable features #7449

Merged
merged 17 commits into from
Feb 21, 2025
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ members = [
"acvm-repo/bn254_blackbox_solver",
# Utility crates
"utils/iter-extended",
"utils/cli-args",
]
default-members = [
"tooling/nargo_cli",
Expand Down Expand Up @@ -109,6 +110,7 @@ ark-std = { version = "^0.5.0", default-features = false }

# Misc utils crates
iter-extended = { path = "utils/iter-extended" }
cli-args = { path = "utils/cli-args" }

# LSP
async-lsp = { version = "0.1.0", default-features = false }
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_driver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ serde.workspace = true
fxhash.workspace = true
rust-embed.workspace = true
tracing.workspace = true
cli-args.workspace = true

[features]
bn254 = ["noirc_frontend/bn254", "noirc_evaluator/bn254"]
Expand Down
23 changes: 17 additions & 6 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
use acvm::acir::circuit::ExpressionWidth;
use acvm::compiler::MIN_EXPRESSION_WIDTH;
use clap::Args;
use cli_args::FrontendOptions;
use cli_args::UnstableFeature;
use fm::{FileId, FileManager};
use iter_extended::vecmap;
use noirc_abi::{AbiParameter, AbiType, AbiValue};
Expand Down Expand Up @@ -150,11 +152,11 @@
#[arg(long, hide = true)]
pub skip_brillig_constraints_check: bool,

/// Flag to turn on the lookback feature of the Brillig call constraints

Check warning on line 155 in compiler/noirc_driver/src/lib.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lookback)
/// check, allowing tracking argument values before the call happens preventing
/// certain rare false positives (leads to a slowdown on large rollout functions)
#[arg(long)]
pub enable_brillig_constraints_check_lookback: bool,

Check warning on line 159 in compiler/noirc_driver/src/lib.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lookback)

/// Setting to decide on an inlining strategy for Brillig functions.
/// A more aggressive inliner should generate larger programs but more optimized
Expand All @@ -179,6 +181,10 @@
/// Used internally to test for non-determinism in the compiler.
#[clap(long, hide = true)]
pub check_non_determinism: bool,

/// Unstable features to enable for this current build
#[clap(long, short = 'Z', value_delimiter = ',')]
pub unstable_features: Vec<UnstableFeature>,
}

pub fn parse_expression_width(input: &str) -> Result<ExpressionWidth, std::io::Error> {
Expand All @@ -197,6 +203,16 @@
}
}

impl CompileOptions {
pub fn frontend_options(&self) -> FrontendOptions {
FrontendOptions {
debug_comptime_in_file: self.debug_comptime_in_file.as_deref(),
pedantic_solving: self.pedantic_solving,
enabled_unstable_features: &self.unstable_features,
}
}
}

#[derive(Debug)]
pub enum CompileError {
MonomorphizationError(MonomorphizationError),
Expand Down Expand Up @@ -336,12 +352,7 @@
crate_id: CrateId,
options: &CompileOptions,
) -> CompilationResult<()> {
let diagnostics = CrateDefMap::collect_defs(
crate_id,
context,
options.debug_comptime_in_file.as_deref(),
options.pedantic_solving,
);
let diagnostics = CrateDefMap::collect_defs(crate_id, context, options.frontend_options());
let crate_files = context.crate_files(&crate_id);
let warnings_and_errors: Vec<FileDiagnostic> = diagnostics
.into_iter()
Expand Down Expand Up @@ -697,8 +708,8 @@
},
emit_ssa: if options.emit_ssa { Some(context.package_build_path.clone()) } else { None },
skip_underconstrained_check: options.skip_underconstrained_check,
enable_brillig_constraints_check_lookback: options

Check warning on line 711 in compiler/noirc_driver/src/lib.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lookback)
.enable_brillig_constraints_check_lookback,

Check warning on line 712 in compiler/noirc_driver/src/lib.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lookback)
enable_brillig_constraints_check: options.enable_brillig_constraints_check,
inliner_aggressiveness: options.inliner_aggressiveness,
max_bytecode_increase_percent: options.max_bytecode_increase_percent,
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ rangemap = "1.4.0"
strum.workspace = true
strum_macros.workspace = true
fxhash.workspace = true

cli-args.workspace = true

[dev-dependencies]
base64.workspace = true
Expand Down
7 changes: 3 additions & 4 deletions compiler/noirc_frontend/src/elaborator/comptime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,8 @@ impl<'context> Elaborator<'context> {
self.usage_tracker,
self.crate_graph,
self.crate_id,
self.debug_comptime_in_file,
self.interpreter_call_stack.clone(),
self.pedantic_solving,
self.options,
);

elaborator.function_context.push(FunctionContext::default());
Expand Down Expand Up @@ -488,15 +487,15 @@ impl<'context> Elaborator<'context> {
Some(DependencyId::Function(function)) => Some(function),
_ => None,
};
Interpreter::new(self, self.crate_id, current_function, self.pedantic_solving)
Interpreter::new(self, self.crate_id, current_function)
}

pub(super) fn debug_comptime<T: Display, F: FnMut(&mut NodeInterner) -> T>(
&mut self,
location: Location,
mut expr_f: F,
) {
if Some(location.file) == self.debug_comptime_in_file {
if Some(location.file) == self.options.debug_comptime_in_file {
let displayed_expr = expr_f(self.interner);
self.errors.push((
InterpreterError::debug_evaluate_comptime(displayed_expr, location).into(),
Expand Down
2 changes: 2 additions & 0 deletions compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1023,6 +1023,8 @@ impl<'context> Elaborator<'context> {
match_expr: MatchExpression,
span: Span,
) -> (HirExpression, Type) {
self.use_unstable_feature(cli_args::UnstableFeature::Enums, span);

let (expression, typ) = self.elaborate_expression(match_expr.expression);
let (let_, variable) = self.wrap_in_let(expression, typ);

Expand Down
67 changes: 35 additions & 32 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ use crate::{
hir::{
def_collector::{
dc_crate::{
filter_literal_globals, CollectedItems, CompilationError, ImplMap, UnresolvedEnum,
UnresolvedFunctions, UnresolvedGlobal, UnresolvedStruct, UnresolvedTraitImpl,
UnresolvedTypeAlias,
filter_literal_globals, CollectedItems, CompilationError, ElaboratorOptions,
ImplMap, UnresolvedEnum, UnresolvedFunctions, UnresolvedGlobal, UnresolvedStruct,
UnresolvedTraitImpl, UnresolvedTypeAlias,
},
errors::DefCollectorErrorKind,
},
Expand All @@ -35,6 +35,7 @@ use crate::{
DefinitionKind, DependencyId, ExprId, FuncId, FunctionModifiers, GlobalId, NodeInterner,
ReferenceId, TraitId, TraitImplId, TypeAliasId, TypeId,
},
parser::{ParserError, ParserErrorReason},
token::SecondaryAttribute,
EnumVariant, Shared, Type, TypeVariable,
};
Expand All @@ -60,6 +61,7 @@ mod traits;
pub mod types;
mod unquote;

use cli_args::UnstableFeature;
use fm::FileId;
use iter_extended::vecmap;
use noirc_errors::{Location, Span, Spanned};
Expand Down Expand Up @@ -178,9 +180,6 @@ pub struct Elaborator<'context> {

crate_id: CrateId,

/// The scope of --debug-comptime, or None if unset
debug_comptime_in_file: Option<FileId>,

/// These are the globals that have yet to be elaborated.
/// This map is used to lazily evaluate these globals if they're encountered before
/// they are elaborated (e.g. in a function's type or another global's RHS).
Expand All @@ -194,8 +193,8 @@ pub struct Elaborator<'context> {
/// that comptime value and any visibility errors were already reported.
silence_field_visibility_errors: usize,

/// Use pedantic ACVM solving
pedantic_solving: bool,
/// Options from the nargo cli
options: ElaboratorOptions<'context>,
}

#[derive(Default)]
Expand Down Expand Up @@ -224,9 +223,8 @@ impl<'context> Elaborator<'context> {
usage_tracker: &'context mut UsageTracker,
crate_graph: &'context CrateGraph,
crate_id: CrateId,
debug_comptime_in_file: Option<FileId>,
interpreter_call_stack: im::Vector<Location>,
pedantic_solving: bool,
options: ElaboratorOptions<'context>,
) -> Self {
Self {
scopes: ScopeForest::default(),
Expand All @@ -248,60 +246,47 @@ impl<'context> Elaborator<'context> {
trait_bounds: Vec::new(),
function_context: vec![FunctionContext::default()],
current_trait_impl: None,
debug_comptime_in_file,
unresolved_globals: BTreeMap::new(),
current_trait: None,
interpreter_call_stack,
in_comptime_context: false,
silence_field_visibility_errors: 0,
pedantic_solving,
options,
}
}

pub fn from_context(
context: &'context mut Context,
crate_id: CrateId,
debug_comptime_in_file: Option<FileId>,
pedantic_solving: bool,
options: ElaboratorOptions<'context>,
) -> Self {
Self::new(
&mut context.def_interner,
&mut context.def_maps,
&mut context.usage_tracker,
&context.crate_graph,
crate_id,
debug_comptime_in_file,
im::Vector::new(),
pedantic_solving,
options,
)
}

pub fn elaborate(
context: &'context mut Context,
crate_id: CrateId,
items: CollectedItems,
debug_comptime_in_file: Option<FileId>,
pedantic_solving: bool,
options: ElaboratorOptions<'context>,
) -> Vec<(CompilationError, FileId)> {
Self::elaborate_and_return_self(
context,
crate_id,
items,
debug_comptime_in_file,
pedantic_solving,
)
.errors
Self::elaborate_and_return_self(context, crate_id, items, options).errors
}

pub fn elaborate_and_return_self(
context: &'context mut Context,
crate_id: CrateId,
items: CollectedItems,
debug_comptime_in_file: Option<FileId>,
pedantic_solving: bool,
options: ElaboratorOptions<'context>,
) -> Self {
let mut this =
Self::from_context(context, crate_id, debug_comptime_in_file, pedantic_solving);
let mut this = Self::from_context(context, crate_id, options);
this.elaborate_items(items);
this.check_and_pop_function_context();
this
Expand Down Expand Up @@ -385,6 +370,11 @@ impl<'context> Elaborator<'context> {
self.errors.extend(self.interner.check_for_dependency_cycles());
}

/// True if we should use pedantic ACVM solving
pub fn pedantic_solving(&self) -> bool {
self.options.pedantic_solving
}

/// Runs `f` and if it modifies `self.generics`, `self.generics` is truncated
/// back to the previous length.
fn recover_generics<T>(&mut self, f: impl FnOnce(&mut Self) -> T) -> T {
Expand Down Expand Up @@ -1846,8 +1836,12 @@ impl<'context> Elaborator<'context> {
self.generics.clear();

let datatype = self.interner.get_type(*type_id);
let generics = datatype.borrow().generic_types();
self.add_existing_generics(&typ.enum_def.generics, &datatype.borrow().generics);
let datatype_ref = datatype.borrow();
let generics = datatype_ref.generic_types();
self.add_existing_generics(&typ.enum_def.generics, &datatype_ref.generics);

self.use_unstable_feature(cli_args::UnstableFeature::Enums, datatype_ref.name.span());
drop(datatype_ref);

let self_type = Type::DataType(datatype.clone(), generics);
let self_type_id = self.interner.push_quoted_type(self_type.clone());
Expand Down Expand Up @@ -2131,4 +2125,13 @@ impl<'context> Elaborator<'context> {
_ => true,
})
}

/// Register a use of the given unstable feature. Errors if the feature has not
/// been explicitly enabled in this package.
pub fn use_unstable_feature(&mut self, feature: UnstableFeature, span: Span) {
if !self.options.enabled_unstable_features.contains(&feature) {
let reason = ParserErrorReason::ExperimentalFeature(feature);
self.push_err(ParserError::with_reason(reason, span));
}
}
}
6 changes: 1 addition & 5 deletions compiler/noirc_frontend/src/hir/comptime/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@

/// Stateful bigint calculator.
bigint_solver: BigIntSolverWithId,

/// Use pedantic ACVM solving
pedantic_solving: bool,
}

#[allow(unused)]
Expand All @@ -78,8 +75,8 @@
elaborator: &'local mut Elaborator<'interner>,
crate_id: CrateId,
current_function: Option<FuncId>,
pedantic_solving: bool,
) -> Self {
let pedantic_solving = elaborator.pedantic_solving();
let bigint_solver = BigIntSolverWithId::with_pedantic_solving(pedantic_solving);
Self {
elaborator,
Expand All @@ -88,7 +85,6 @@
bound_generics: Vec::new(),
in_loop: false,
bigint_solver,
pedantic_solving,
}
}

Expand Down Expand Up @@ -258,7 +254,7 @@
}
} else {
let name = self.elaborator.interner.function_name(&function);
unreachable!("Non-builtin, lowlevel or oracle builtin fn '{name}'")

Check warning on line 257 in compiler/noirc_frontend/src/hir/comptime/interpreter.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lowlevel)
}
}

Expand Down Expand Up @@ -1058,7 +1054,7 @@
}

/// Generate matches for bit shifting, which in Noir only accepts `u8` for RHS.
macro_rules! match_bitshift {

Check warning on line 1057 in compiler/noirc_frontend/src/hir/comptime/interpreter.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bitshift)
(($lhs_value:ident as $lhs:ident $op:literal $rhs_value:ident as $rhs:ident) => $expr:expr) => {
match_values! {
($lhs_value as $lhs $op $rhs_value as $rhs) {
Expand Down Expand Up @@ -1128,7 +1124,7 @@
BinaryOpKind::Xor => match_bitwise! {
(lhs_value as lhs "^" rhs_value as rhs) => lhs ^ rhs
},
BinaryOpKind::ShiftRight => match_bitshift! {

Check warning on line 1127 in compiler/noirc_frontend/src/hir/comptime/interpreter.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bitshift)
(lhs_value as lhs ">>" rhs_value as rhs) => lhs.checked_shr(rhs.into())
},
BinaryOpKind::ShiftLeft => match_bitshift! {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl<'local, 'context> Interpreter<'local, 'context> {
arguments,
return_type,
location,
self.pedantic_solving,
self.elaborator.pedantic_solving(),
)
}
}
Expand Down
6 changes: 2 additions & 4 deletions compiler/noirc_frontend/src/hir/comptime/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use super::errors::InterpreterError;
use super::value::Value;
use super::Interpreter;
use crate::elaborator::Elaborator;
use crate::hir::def_collector::dc_crate::{CompilationError, DefCollector};
use crate::hir::def_collector::dc_crate::{CompilationError, DefCollector, ElaboratorOptions};
use crate::hir::def_collector::dc_mod::collect_defs;
use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleData};
use crate::hir::{Context, ParsedFiles};
Expand Down Expand Up @@ -60,13 +60,11 @@ pub(crate) fn with_interpreter<T>(

let main = context.get_main_function(&krate).expect("Expected 'main' function");

let pedantic_solving = true;
let mut elaborator = Elaborator::elaborate_and_return_self(
&mut context,
krate,
collector.items,
None,
pedantic_solving,
ElaboratorOptions::test_default(),
);

let errors = elaborator.errors.clone();
Expand Down
Loading
Loading