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: Adding internal keyword #1873

Merged
merged 8 commits into from
Jul 8, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions crates/nargo/src/artifacts/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ pub struct PreprocessedContractFunction {

pub function_type: ContractFunctionType,

pub is_internal: bool,

pub abi: Abi,

#[serde(
Expand Down
1 change: 1 addition & 0 deletions crates/nargo/src/ops/preprocess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ pub fn preprocess_contract_function<B: ProofSystemCompiler>(
Ok(PreprocessedContractFunction {
name: func.name,
function_type: func.function_type,
is_internal: func.is_internal,
abi: func.abi,

bytecode: optimized_bytecode,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,6 @@ fn main(x : Field, y : pub Field) {
contract Foo {
fn double(x: Field) -> pub Field { x * 2 }
fn triple(x: Field) -> pub Field { x * 3 }
internal fn quadruple(x: Field) -> pub Field { x * 4 }
open internal fn skibbidy(x: Field) -> pub Field { x * 5 }
}
2 changes: 2 additions & 0 deletions crates/noirc_driver/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ pub struct ContractFunction {

pub function_type: ContractFunctionType,

pub is_internal: bool,

pub abi: Abi,

#[serde(serialize_with = "serialize_circuit", deserialize_with = "deserialize_circuit")]
Expand Down
1 change: 1 addition & 0 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ fn compile_contract(
functions.push(ContractFunction {
name,
function_type,
is_internal: func_meta.is_internal.unwrap_or(false),
abi: function.abi,
bytecode: function.circuit,
});
Expand Down
2 changes: 2 additions & 0 deletions crates/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,8 @@ pub struct FunctionDefinition {
/// True if this function was defined with the 'open' keyword
pub is_open: bool,

pub is_internal: bool,

/// True if this function was defined with the 'unconstrained' keyword
pub is_unconstrained: bool,

Expand Down
7 changes: 7 additions & 0 deletions crates/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ pub enum ResolverError {
MutableReferenceToImmutableVariable { variable: String, span: Span },
#[error("Mutable references to array indices are unsupported")]
MutableReferenceToArrayElement { span: Span },
#[error("Function is not defined in a contract yet sets is_internal")]
ContractFunctionInternalInNormalFunction { span: Span },
}

impl ResolverError {
Expand Down Expand Up @@ -268,6 +270,11 @@ impl From<ResolverError> for Diagnostic {
ResolverError::MutableReferenceToArrayElement { span } => {
Diagnostic::simple_error("Mutable references to array elements are currently unsupported".into(), "Try storing the element in a fresh variable first".into(), span)
},
ResolverError::ContractFunctionInternalInNormalFunction { span } => Diagnostic::simple_error(
"Only functions defined within contracts can set their functions to be internal".into(),
"Non-contract functions cannot be 'internal'".into(),
span,
),
}
}
}
14 changes: 14 additions & 0 deletions crates/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,7 @@ impl<'a> Resolver<'a> {
kind: func.kind,
attributes,
contract_function_type: self.handle_function_type(func),
is_internal: self.handle_is_function_internal(func),
is_unconstrained: func.def.is_unconstrained,
location,
typ,
Expand Down Expand Up @@ -708,6 +709,19 @@ impl<'a> Resolver<'a> {
}
}

fn handle_is_function_internal(&mut self, func: &NoirFunction) -> Option<bool> {
if self.in_contract() {
Some(func.def.is_internal)
} else {
if func.def.is_internal {
self.push_err(ResolverError::ContractFunctionInternalInNormalFunction {
span: func.name_ident().span(),
});
}
None
}
}

fn declare_numeric_generics(&mut self, params: &[Type], return_type: &Type) {
if self.generics.is_empty() {
return;
Expand Down
1 change: 1 addition & 0 deletions crates/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ mod test {
attributes: None,
location,
contract_function_type: None,
is_internal: None,
is_unconstrained: false,
typ: Type::Function(vec![Type::field(None), Type::field(None)], Box::new(Type::Unit)),
parameters: vec![
Expand Down
7 changes: 6 additions & 1 deletion crates/noirc_frontend/src/hir_def/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,15 @@ pub struct FuncMeta {
/// Attribute per function.
pub attributes: Option<Attribute>,

/// This function's visibility in its contract.
/// This function's type in its contract.
/// If this function is not in a contract, this is always 'Secret'.
pub contract_function_type: Option<ContractFunctionType>,

/// This function's visibility.
/// If this function is internal can only be called by itself.
/// Will be None if not in contract.
pub is_internal: Option<bool>,

pub is_unconstrained: bool,

pub parameters: Parameters,
Expand Down
3 changes: 3 additions & 0 deletions crates/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,7 @@ pub enum Keyword {
Impl,
If,
In,
Internal,
Let,
Mod,
Mut,
Expand Down Expand Up @@ -466,6 +467,7 @@ impl fmt::Display for Keyword {
Keyword::Impl => write!(f, "impl"),
Keyword::If => write!(f, "if"),
Keyword::In => write!(f, "in"),
Keyword::Internal => write!(f, "internal"),
Keyword::Let => write!(f, "let"),
Keyword::Mod => write!(f, "mod"),
Keyword::Mut => write!(f, "mut"),
Expand Down Expand Up @@ -504,6 +506,7 @@ impl Keyword {
"impl" => Keyword::Impl,
"if" => Keyword::If,
"in" => Keyword::In,
"internal" => Keyword::Internal,
"let" => Keyword::Let,
"mod" => Keyword::Mod,
"mut" => Keyword::Mut,
Expand Down
18 changes: 12 additions & 6 deletions crates/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,10 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
.map(
|(
(
((((attribute, (is_unconstrained, is_open)), name), generics), parameters),
(
(((attribute, (is_unconstrained, is_open, is_internal)), name), generics),
parameters,
),
((return_distinctness, return_visibility), return_type),
),
body,
Expand All @@ -172,6 +175,7 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
name,
attribute, // XXX: Currently we only have one attribute defined. If more attributes are needed per function, we can make this a vector and make attribute definition more expressive
is_open,
is_internal,
is_unconstrained,
generics,
parameters,
Expand All @@ -185,14 +189,16 @@ fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunction> {
)
}

/// function_modifiers: 'unconstrained' 'open' | 'unconstrained' | 'open' | %empty
///
/// returns (is_unconstrained, is_open) for whether each keyword was present
fn function_modifiers() -> impl NoirParser<(bool, bool)> {
/// function_modifiers: 'open internal' | 'internal' | 'unconstrained' | 'open' | %empty
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
/// returns (is_unconstrained, is_open, is_internal) for whether each keyword was present
fn function_modifiers() -> impl NoirParser<(bool, bool, bool)> {
keyword(Keyword::Unconstrained)
.or_not()
.then(keyword(Keyword::Open).or_not())
.map(|(unconstrained, open)| (unconstrained.is_some(), open.is_some()))
.then(keyword(Keyword::Internal).or_not())
.map(|((unconstrained, open), internal)| {
(unconstrained.is_some(), open.is_some(), internal.is_some())
})
}

/// non_empty_ident_list: ident ',' non_empty_ident_list
Expand Down