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

fix!: Change tag attributes to require a ' prefix #6235

Merged
merged 10 commits into from
Oct 8, 2024
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: 1 addition & 1 deletion compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ fn compile_contract_inner(
.secondary
.iter()
.filter_map(|attr| {
if let SecondaryAttribute::Custom(attribute) = attr {
if let SecondaryAttribute::Tag(attribute) = attr {
Some(&attribute.contents)
} else {
None
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/ast/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1296,8 +1296,8 @@
UnresolvedTypeData::Unspecified => visitor.visit_unspecified_type(self.span),
UnresolvedTypeData::Quoted(typ) => visitor.visit_quoted_type(typ, self.span),
UnresolvedTypeData::FieldElement => visitor.visit_field_element_type(self.span),
UnresolvedTypeData::Integer(signdness, size) => {

Check warning on line 1299 in compiler/noirc_frontend/src/ast/visitor.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (signdness)
visitor.visit_integer_type(*signdness, *size, self.span);

Check warning on line 1300 in compiler/noirc_frontend/src/ast/visitor.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (signdness)
}
UnresolvedTypeData::Bool => visitor.visit_bool_type(self.span),
UnresolvedTypeData::Unit => visitor.visit_unit_type(self.span),
Expand Down Expand Up @@ -1391,7 +1391,7 @@
}

pub fn accept_children(&self, target: AttributeTarget, visitor: &mut impl Visitor) {
if let SecondaryAttribute::Custom(custom) = self {
if let SecondaryAttribute::Meta(custom) = self {
custom.accept(target, visitor);
}
}
Expand Down
36 changes: 28 additions & 8 deletions compiler/noirc_frontend/src/elaborator/comptime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ impl<'context> Elaborator<'context> {
attribute_context: AttributeContext,
generated_items: &mut CollectedItems,
) {
if let SecondaryAttribute::Custom(attribute) = attribute {
if let SecondaryAttribute::Meta(attribute) = attribute {
self.elaborate_in_comptime_context(|this| {
if let Err(error) = this.run_comptime_attribute_name_on_item(
&attribute.contents,
Expand Down Expand Up @@ -188,25 +188,45 @@ impl<'context> Elaborator<'context> {

let location = Location::new(attribute_span, self.file);
let Some((function, arguments)) = Self::parse_attribute(attribute, location)? else {
// Do not issue an error if the attribute is unknown
return Ok(());
return Err((
ResolverError::UnableToParseAttribute {
attribute: attribute.to_string(),
span: attribute_span,
}
.into(),
self.file,
));
};

// Elaborate the function, rolling back any errors generated in case it is unknown
let error_count = self.errors.len();
let function_string = function.to_string();
let function = self.elaborate_expression(function).0;
self.errors.truncate(error_count);

let definition_id = match self.interner.expression(&function) {
HirExpression::Ident(ident, _) => ident.id,
_ => return Ok(()),
_ => {
return Err((
ResolverError::AttributeFunctionIsNotAPath {
function: function_string,
span: attribute_span,
}
.into(),
self.file,
))
}
};

let Some(definition) = self.interner.try_definition(definition_id) else {
// If there's no such function, don't return an error.
// This preserves backwards compatibility in allowing custom attributes that
// do not refer to comptime functions.
return Ok(());
return Err((
ResolverError::AttributeFunctionNotInScope {
name: function_string,
span: attribute_span,
}
.into(),
self.file,
));
};

let DefinitionKind::Function(function) = definition.kind else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2242,7 +2242,7 @@ fn function_def_add_attribute(
}
}

if let Attribute::Secondary(SecondaryAttribute::Custom(attribute)) = attribute {
if let Attribute::Secondary(SecondaryAttribute::Tag(attribute)) = attribute {
let func_meta = interpreter.elaborator.interner.function_meta_mut(&func_id);
func_meta.custom_attributes.push(attribute);
}
Expand Down
27 changes: 27 additions & 0 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,12 @@ pub enum ResolverError {
UnsupportedNumericGenericType(#[from] UnsupportedNumericGenericType),
#[error("Type `{typ}` is more private than item `{item}`")]
TypeIsMorePrivateThenItem { typ: String, item: String, span: Span },
#[error("Unable to parse attribute `{attribute}`")]
UnableToParseAttribute { attribute: String, span: Span },
#[error("Attribute function `{function}` is not a path")]
AttributeFunctionIsNotAPath { function: String, span: Span },
#[error("Attribute function `{name}` is not in scope")]
AttributeFunctionNotInScope { name: String, span: Span },
}

impl ResolverError {
Expand Down Expand Up @@ -546,6 +552,27 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
*span,
)
},
ResolverError::UnableToParseAttribute { attribute, span } => {
Diagnostic::simple_error(
format!("Unable to parse attribute `{attribute}`"),
"Attribute should be a function or function call".into(),
*span,
)
},
ResolverError::AttributeFunctionIsNotAPath { function, span } => {
Diagnostic::simple_error(
format!("Attribute function `{function}` is not a path"),
"An attribute's function should be a single identifier or a path".into(),
*span,
)
},
ResolverError::AttributeFunctionNotInScope { name, span } => {
Diagnostic::simple_error(
format!("Attribute function `{name}` is not in scope"),
String::new(),
*span,
)
},
}
}
}
19 changes: 12 additions & 7 deletions compiler/noirc_frontend/src/lexer/lexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,11 @@ impl<'a> Lexer<'a> {
}
self.next_char();

let is_tag = self.peek_char_is('\'');
if is_tag {
self.next_char();
}

let contents_start = self.position + 1;

let word = self.eat_while(None, |ch| ch != ']');
Expand All @@ -321,7 +326,7 @@ impl<'a> Lexer<'a> {
let span = Span::inclusive(start, end);
let contents_span = Span::inclusive(contents_start, contents_end);

let attribute = Attribute::lookup_attribute(&word, span, contents_span)?;
let attribute = Attribute::lookup_attribute(&word, span, contents_span, is_tag)?;
if is_inner {
match attribute {
Attribute::Function(attribute) => Err(LexerErrorKind::InvalidInnerAttribute {
Expand Down Expand Up @@ -838,17 +843,17 @@ mod tests {
}

#[test]
fn custom_attribute() {
let input = r#"#[custom(hello)]"#;
fn tag_attribute() {
let input = r#"#['custom(hello)]"#;
let mut lexer = Lexer::new(input);

let token = lexer.next_token().unwrap();
assert_eq!(
token.token(),
&Token::Attribute(Attribute::Secondary(SecondaryAttribute::Custom(CustomAttribute {
&Token::Attribute(Attribute::Secondary(SecondaryAttribute::Tag(CustomAttribute {
contents: "custom(hello)".to_string(),
span: Span::from(0..16),
contents_span: Span::from(2..15)
span: Span::from(0..17),
contents_span: Span::from(3..16)
})))
);
}
Expand Down Expand Up @@ -943,7 +948,7 @@ mod tests {
let token = lexer.next_token().unwrap();
assert_eq!(
token.token(),
&Token::InnerAttribute(SecondaryAttribute::Custom(CustomAttribute {
&Token::InnerAttribute(SecondaryAttribute::Meta(CustomAttribute {
contents: "something".to_string(),
span: Span::from(0..13),
contents_span: Span::from(3..12),
Expand Down
30 changes: 24 additions & 6 deletions compiler/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,7 @@ impl Attribute {
word: &str,
span: Span,
contents_span: Span,
is_tag: bool,
) -> Result<Attribute, LexerErrorKind> {
let word_segments: Vec<&str> = word
.split(|c| c == '(' || c == ')')
Expand All @@ -758,6 +759,14 @@ impl Attribute {
is_valid.ok_or(LexerErrorKind::MalformedFuncAttribute { span, found: word.to_owned() })
};

if is_tag {
return Ok(Attribute::Secondary(SecondaryAttribute::Tag(CustomAttribute {
contents: word.to_owned(),
span,
contents_span,
})));
}

let attribute = match &word_segments[..] {
// Primary Attributes
["foreign", name] => {
Expand Down Expand Up @@ -813,7 +822,7 @@ impl Attribute {
["allow", tag] => Attribute::Secondary(SecondaryAttribute::Allow(tag.to_string())),
tokens => {
tokens.iter().try_for_each(|token| validate(token))?;
Attribute::Secondary(SecondaryAttribute::Custom(CustomAttribute {
Attribute::Secondary(SecondaryAttribute::Meta(CustomAttribute {
contents: word.to_owned(),
span,
contents_span,
Expand Down Expand Up @@ -922,7 +931,13 @@ pub enum SecondaryAttribute {
ContractLibraryMethod,
Export,
Field(String),
Custom(CustomAttribute),

/// A custom tag attribute: #['foo]
Tag(CustomAttribute),

/// An attribute expected to run a comptime function of the same name: #[foo]
Meta(CustomAttribute),

Abi(String),

/// A variable-argument comptime function.
Expand All @@ -939,7 +954,7 @@ pub enum SecondaryAttribute {

impl SecondaryAttribute {
pub(crate) fn as_custom(&self) -> Option<&CustomAttribute> {
if let Self::Custom(attribute) = self {
if let Self::Tag(attribute) = self {
Some(attribute)
} else {
None
Expand All @@ -954,7 +969,8 @@ impl SecondaryAttribute {
}
SecondaryAttribute::Export => Some("export".to_string()),
SecondaryAttribute::Field(_) => Some("field".to_string()),
SecondaryAttribute::Custom(custom) => custom.name(),
SecondaryAttribute::Tag(custom) => custom.name(),
SecondaryAttribute::Meta(custom) => custom.name(),
SecondaryAttribute::Abi(_) => Some("abi".to_string()),
SecondaryAttribute::Varargs => Some("varargs".to_string()),
SecondaryAttribute::UseCallersScope => Some("use_callers_scope".to_string()),
Expand All @@ -977,7 +993,8 @@ impl fmt::Display for SecondaryAttribute {
SecondaryAttribute::Deprecated(Some(ref note)) => {
write!(f, r#"#[deprecated("{note}")]"#)
}
SecondaryAttribute::Custom(ref attribute) => write!(f, "#[{}]", attribute.contents),
SecondaryAttribute::Tag(ref attribute) => write!(f, "#['{}]", attribute.contents),
SecondaryAttribute::Meta(ref attribute) => write!(f, "#[{}]", attribute.contents),
SecondaryAttribute::ContractLibraryMethod => write!(f, "#[contract_library_method]"),
SecondaryAttribute::Export => write!(f, "#[export]"),
SecondaryAttribute::Field(ref k) => write!(f, "#[field({k})]"),
Expand Down Expand Up @@ -1029,7 +1046,8 @@ impl AsRef<str> for SecondaryAttribute {
match self {
SecondaryAttribute::Deprecated(Some(string)) => string,
SecondaryAttribute::Deprecated(None) => "",
SecondaryAttribute::Custom(attribute) => &attribute.contents,
SecondaryAttribute::Tag(attribute) => &attribute.contents,
SecondaryAttribute::Meta(attribute) => &attribute.contents,
SecondaryAttribute::Field(string)
| SecondaryAttribute::Abi(string)
| SecondaryAttribute::Allow(string) => string,
Expand Down
6 changes: 3 additions & 3 deletions compiler/noirc_frontend/src/parser/parser/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ mod tests {

#[test]
fn parses_inner_attribute() {
let src = "#![hello]";
let src = "#!['hello]";
let mut parser = Parser::for_str(src);
let Some(SecondaryAttribute::Custom(custom)) = parser.parse_inner_attribute() else {
panic!("Expected inner custom attribute");
let Some(SecondaryAttribute::Tag(custom)) = parser.parse_inner_attribute() else {
panic!("Expected inner tag attribute");
};
expect_no_errors(&parser.errors);
assert_eq!(custom.contents, "hello");
Expand Down
16 changes: 16 additions & 0 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3203,3 +3203,19 @@ fn trait_unconstrained_methods_typechecked_correctly() {
println!("{errors:?}");
assert_eq!(errors.len(), 0);
}

#[test]
fn error_if_attribute_not_in_scope() {
let src = r#"
#[not_in_scope]
fn main() {}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);

assert!(matches!(
errors[0].0,
CompilationError::ResolverError(ResolverError::AttributeFunctionNotInScope { .. })
));
}
3 changes: 2 additions & 1 deletion noir_stdlib/src/ec/consts/te.nr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ pub struct BabyJubjub {
}

#[field(bn254)]
#[deprecated = "It's recommmended to use the external noir-edwards library (https://github.com/noir-lang/noir-edwards)"]
// Uncommenting this results in deprecated warnings in the stdlib
// #[deprecated]
pub fn baby_jubjub() -> BabyJubjub {
BabyJubjub {
// Baby Jubjub (ERC-2494) parameters in affine representation
Expand Down
6 changes: 4 additions & 2 deletions noir_stdlib/src/hash/mimc.nr
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ use crate::default::Default;
// You must use constants generated for the native field
// Rounds number should be ~ log(p)/log(exp)
// For 254 bit primes, exponent 7 and 91 rounds seems to be recommended
#[deprecated = "It's recommmended to use the external MiMC library (https://github.com/noir-lang/mimc)"]
//
// Uncommenting this results in deprecated warnings in the stdlib
// #[deprecated]
fn mimc<let N: u32>(x: Field, k: Field, constants: [Field; N], exp: Field) -> Field {
//round 0
let mut t = x + k;
Expand Down Expand Up @@ -117,7 +119,7 @@ global MIMC_BN254_CONSTANTS: [Field; MIMC_BN254_ROUNDS] = [

//mimc implementation with hardcoded parameters for BN254 curve.
#[field(bn254)]
#[deprecated = "It's recommmended to use the external MiMC library (https://github.com/noir-lang/mimc)"]
#[deprecated]
pub fn mimc_bn254<let N: u32>(array: [Field; N]) -> Field {
let exponent = 7;
let mut r = 0;
Expand Down
2 changes: 2 additions & 0 deletions noir_stdlib/src/meta/mod.nr
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ pub comptime fn make_trait_impl<Env1, Env2>(
}

mod tests {
use crate::meta::derive_via;

// docs:start:quote-example
comptime fn quote_one() -> Quoted {
quote { 1 }
Expand Down
29 changes: 27 additions & 2 deletions noir_stdlib/src/meta/op.nr
Original file line number Diff line number Diff line change
@@ -1,8 +1,22 @@
#[derive(Eq, Hash)]
pub struct UnaryOp {
op: Field
}

// Cannot derive Eq or Hash since they internally use paths
// starting with std:: which is invalid within the same crate.
// We'd need to use crate:: instead.
impl crate::cmp::Eq for UnaryOp {
fn eq(self, other: Self) -> bool {
self.op == other.op
}
}

impl crate::hash::Hash for UnaryOp {
fn hash<H>(self, h: &mut H) where H: crate::hash::Hasher {
self.op.hash(h);
}
}

impl UnaryOp {
// docs:start:is_minus
pub fn is_minus(self) -> bool {
Expand Down Expand Up @@ -46,11 +60,22 @@ impl UnaryOp {
}
}

#[derive(Eq, Hash)]
pub struct BinaryOp {
op: Field
}

impl crate::cmp::Eq for BinaryOp {
fn eq(self, other: Self) -> bool {
self.op == other.op
}
}

impl crate::hash::Hash for BinaryOp {
fn hash<H>(self, h: &mut H) where H: crate::hash::Hasher {
self.op.hash(h);
}
}

impl BinaryOp {
// docs:start:is_add
pub fn is_add(self) -> bool {
Expand Down
Loading
Loading