Skip to content

Commit

Permalink
feat(cli)!: Add --unstable-features to gate unstable features (#7449)
Browse files Browse the repository at this point in the history
  • Loading branch information
jfecher authored Feb 21, 2025
1 parent c254c3c commit fd213f6
Show file tree
Hide file tree
Showing 18 changed files with 219 additions and 100 deletions.
23 changes: 17 additions & 6 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use noirc_evaluator::create_program;
use noirc_evaluator::errors::RuntimeError;
use noirc_evaluator::ssa::{SsaLogging, SsaProgramArtifact};
use noirc_frontend::debug::build_debug_crate_file;
use noirc_frontend::elaborator::{FrontendOptions, UnstableFeature};
use noirc_frontend::hir::def_map::{Contract, CrateDefMap};
use noirc_frontend::hir::Context;
use noirc_frontend::monomorphization::{
Expand Down Expand Up @@ -175,6 +176,11 @@ pub struct CompileOptions {
/// 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
#[arg(value_parser = clap::value_parser!(UnstableFeature))]
#[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 @@ -193,6 +199,16 @@ pub fn parse_expression_width(input: &str) -> Result<ExpressionWidth, std::io::E
}
}

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 @@ -332,12 +348,7 @@ pub fn check_crate(
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
1 change: 0 additions & 1 deletion compiler/noirc_frontend/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ strum.workspace = true
strum_macros.workspace = true
fxhash.workspace = true


[dev-dependencies]
base64.workspace = true
proptest.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,
self.elaborate_reasons.clone(),
);

Expand Down Expand Up @@ -496,15 +495,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);
let error: CompilationError =
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 @@ -1107,6 +1107,8 @@ impl<'context> Elaborator<'context> {
location: Location,
) -> (HirExpression, Type) {
let span = location.span;
self.use_unstable_feature(super::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: 37 additions & 30 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,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 @@ -52,6 +53,7 @@ mod comptime;
mod enums;
mod expressions;
mod lints;
mod options;
mod path_resolution;
mod patterns;
mod scope;
Expand All @@ -63,7 +65,9 @@ mod unquote;

use fm::FileId;
use iter_extended::vecmap;
use noirc_errors::{Located, Location};
use noirc_errors::{Located, Location, Span};
pub(crate) use options::ElaboratorOptions;
pub use options::{FrontendOptions, UnstableFeature};
pub use path_resolution::Turbofish;
use path_resolution::{PathResolution, PathResolutionItem};
use types::bind_ordered_generics;
Expand Down Expand Up @@ -179,9 +183,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 @@ -195,8 +196,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>,

/// Sometimes items are elaborated because a function attribute ran and generated items.
/// The Elaborator keeps track of these reasons so that when an error is produced it will
Expand All @@ -211,6 +212,7 @@ pub enum ElaborateReason {
/// Evaluating `Module::add_item`
AddingItemToModule,
}

impl ElaborateReason {
fn to_macro_error(self, error: CompilationError, location: Location) -> ComptimeError {
match self {
Expand Down Expand Up @@ -250,9 +252,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>,
elaborate_reasons: im::Vector<(ElaborateReason, Location)>,
) -> Self {
Self {
Expand All @@ -275,32 +276,29 @@ 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,
elaborate_reasons,
}
}

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,
im::Vector::new(),
)
}
Expand All @@ -309,28 +307,18 @@ impl<'context> Elaborator<'context> {
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 @@ -414,6 +402,11 @@ impl<'context> Elaborator<'context> {
self.push_errors(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 @@ -1941,8 +1934,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(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 @@ -2227,4 +2224,14 @@ 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);
let location = Location::new(span, self.file);
self.push_err(ParserError::with_reason(reason, location), self.file);
}
}
}
62 changes: 62 additions & 0 deletions compiler/noirc_frontend/src/elaborator/options.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
use std::str::FromStr;

#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum UnstableFeature {
Enums,
ArrayOwnership,
}

impl std::fmt::Display for UnstableFeature {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
Self::Enums => write!(f, "enums"),
Self::ArrayOwnership => write!(f, "array-ownership"),
}
}
}

impl FromStr for UnstableFeature {
type Err = String;

fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"enums" => Ok(Self::Enums),
"array-ownership" => Ok(Self::ArrayOwnership),
other => Err(format!("Unknown unstable feature '{other}'")),
}
}
}

/// Generic options struct meant to resolve to ElaboratorOptions below when
/// we can resolve a file path to a file id later. This generic struct is used
/// so that FrontendOptions doesn't need to duplicate fields and methods with ElaboratorOptions.
#[derive(Copy, Clone)]
pub struct GenericOptions<'a, T> {
/// The scope of --debug-comptime, or None if unset
pub debug_comptime_in_file: Option<T>,

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

/// Unstable compiler features that were explicitly enabled. Any unstable features
/// that are not in this list result in an error when used.
pub enabled_unstable_features: &'a [UnstableFeature],
}

/// Options from nargo_cli that need to be passed down to the elaborator
pub(crate) type ElaboratorOptions<'a> = GenericOptions<'a, fm::FileId>;

/// This is the unresolved version of `ElaboratorOptions`
/// CLI options that need to be passed to the compiler frontend (the elaborator).
pub type FrontendOptions<'a> = GenericOptions<'a, &'a str>;

impl<'a, T> GenericOptions<'a, T> {
/// A sane default of frontend options for running tests
pub fn test_default() -> GenericOptions<'static, T> {
GenericOptions {
debug_comptime_in_file: None,
pedantic_solving: true,
enabled_unstable_features: &[UnstableFeature::Enums],
}
}
}
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 @@ pub struct Interpreter<'local, 'interner> {

/// Stateful bigint calculator.
bigint_solver: BigIntSolverWithId,

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

#[allow(unused)]
Expand All @@ -78,8 +75,8 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
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 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
bound_generics: Vec::new(),
in_loop: false,
bigint_solver,
pedantic_solving,
}
}

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 @@ -10,7 +10,7 @@ use noirc_errors::Location;
use super::errors::InterpreterError;
use super::value::Value;
use super::Interpreter;
use crate::elaborator::Elaborator;
use crate::elaborator::{Elaborator, ElaboratorOptions};
use crate::hir::def_collector::dc_crate::{CompilationError, DefCollector};
use crate::hir::def_collector::dc_mod::collect_defs;
use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleData};
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

0 comments on commit fd213f6

Please sign in to comment.