Skip to content
This repository has been archived by the owner on Sep 9, 2024. It is now read-only.

Commit

Permalink
feat(parser): throw error on macro name clash (#293)
Browse files Browse the repository at this point in the history 
* fix: restrict duplicate macros

* refactor: create seperate method to check duplicates

* fix: restrict duplicate macros

* refactor: create seperate method to check duplicates

* fix: run clippy

* chore: add tracing::error

* chore: remove comments

---------

Co-authored-by: Maddiaa0 <[email protected]>
  • Loading branch information
@PraneshASP @Maddiaa0
PraneshASP and Maddiaa0 committed Oct 1, 2023
1 parent c62ab07 commit 09d7a24
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 31 deletions.
81 changes: 50 additions & 31 deletions huff_parser/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ impl Parser {
TokenKind::Macro | TokenKind::Fn | TokenKind::Test => {
let m = self.parse_macro()?;
tracing::info!(target: "parser", "SUCCESSFULLY PARSED MACRO {}", m.name);
self.check_duplicate_macro(&contract, m.clone())?;
contract.macros.push(m);
}
TokenKind::JumpTable | TokenKind::JumpTablePacked | TokenKind::CodeTable => {
Expand Down Expand Up @@ -132,7 +133,7 @@ impl Parser {
TokenKind::Include
)),
spans: AstSpan(self.spans.clone()),
})
});
}
}

Expand All @@ -157,7 +158,7 @@ impl Parser {
kind: ParserErrorKind::InvalidName(tok.clone()),
hint: Some(format!("Expected import string. Got: \"{tok}\"")),
spans: AstSpan(new_spans),
})
});
}
};

Expand Down Expand Up @@ -185,6 +186,24 @@ impl Parser {
std::mem::discriminant(&self.current_token.kind) == std::mem::discriminant(&kind)
}

/// Checks if there is a duplicate macro name
pub fn check_duplicate_macro(
&self,
contract: &Contract,
m: MacroDefinition,
) -> Result<(), ParserError> {
if contract.macros.binary_search_by(|_macro| _macro.name.cmp(&m.name)).is_ok() {
tracing::error!(target: "parser", "DUPLICATE MACRO NAME FOUND: {}", m.name);
Err(ParserError {
kind: ParserErrorKind::DuplicateMacro(m.name),
hint: Some("MACRO names should be unique".to_string()),
spans: AstSpan(vec![self.spans[2].clone()]),
})
} else {
Ok(())
}
}

/// Consumes the next token.
pub fn consume(&mut self) {
self.spans.push(self.current_token.span.clone());
Expand All @@ -197,7 +216,7 @@ impl Parser {
loop {
let token = self.peek().unwrap();
if !kinds.contains(&token.kind) {
break
break;
}
self.current_token = token;
self.cursor += 1;
Expand Down Expand Up @@ -238,7 +257,7 @@ impl Parser {
kind: ParserErrorKind::InvalidName(tok.clone()),
hint: Some(format!("Expected function name, found: \"{tok}\"")),
spans: AstSpan(self.spans.clone()),
})
});
}
};

Expand Down Expand Up @@ -300,7 +319,7 @@ impl Parser {
kind: ParserErrorKind::InvalidName(tok.clone()),
hint: Some(format!("Expected event name, found: \"{tok}\"")),
spans: AstSpan(self.spans.clone()),
})
});
}
};

Expand Down Expand Up @@ -331,7 +350,7 @@ impl Parser {
kind: ParserErrorKind::UnexpectedType(tok),
hint: Some("Expected constant name.".to_string()),
spans: AstSpan(self.spans.clone()),
})
});
}
};

Expand All @@ -356,7 +375,7 @@ impl Parser {
.to_string(),
),
spans: AstSpan(vec![self.current_token.span.clone()]),
})
});
}
};

Expand Down Expand Up @@ -384,7 +403,7 @@ impl Parser {
kind: ParserErrorKind::UnexpectedType(tok),
hint: Some("Expected error name.".to_string()),
spans: AstSpan(self.spans.clone()),
})
});
}
};

Expand Down Expand Up @@ -431,7 +450,7 @@ impl Parser {
),
hint: Some(format!("Expected string for decorator flag: {s}")),
spans: AstSpan(vec![self.current_token.span.clone()]),
})
});
}
}
// The value flag accepts a single literal as an argument
Expand All @@ -447,7 +466,7 @@ impl Parser {
),
hint: Some(format!("Expected literal for decorator flag: {s}")),
spans: AstSpan(vec![self.current_token.span.clone()]),
})
});
}
}
Err(_) => {
Expand All @@ -456,7 +475,7 @@ impl Parser {
kind: ParserErrorKind::InvalidDecoratorFlag(s.clone()),
hint: Some(format!("Unknown decorator flag: {s}")),
spans: AstSpan(self.spans.clone()),
})
});
}
}

Expand All @@ -474,7 +493,7 @@ impl Parser {
)),
hint: Some(String::from("Unknown decorator flag")),
spans: AstSpan(self.spans.clone()),
})
});
}
}

Expand Down Expand Up @@ -683,7 +702,7 @@ impl Parser {
kind: ParserErrorKind::InvalidTokenInMacroBody(kind),
hint: None,
spans: AstSpan(vec![self.current_token.span.clone()]),
})
});
}
};
}
Expand All @@ -706,8 +725,8 @@ impl Parser {
pub fn parse_label(&mut self) -> Result<Vec<Statement>, ParserError> {
let mut statements: Vec<Statement> = Vec::new();
self.match_kind(TokenKind::Colon)?;
while !self.check(TokenKind::Label("NEXT_LABEL".to_string())) &&
!self.check(TokenKind::CloseBrace)
while !self.check(TokenKind::Label("NEXT_LABEL".to_string()))
&& !self.check(TokenKind::CloseBrace)
{
match self.current_token.kind.clone() {
TokenKind::Literal(val) => {
Expand Down Expand Up @@ -796,7 +815,7 @@ impl Parser {
kind: ParserErrorKind::InvalidTokenInLabelDefinition(kind),
hint: None,
spans: AstSpan(vec![self.current_token.span.clone()]),
})
});
}
};
}
Expand Down Expand Up @@ -848,7 +867,7 @@ impl Parser {
self.consume();
on_type = true;
}
continue
continue;
}

// Check for literals
Expand All @@ -868,7 +887,7 @@ impl Parser {
self.consume();
on_type = true;
}
continue
continue;
}
}

Expand Down Expand Up @@ -909,9 +928,9 @@ impl Parser {
}

// name comes second (is optional)
if select_name &&
(self.check(TokenKind::Ident("x".to_string())) ||
self.check(TokenKind::PrimitiveType(PrimitiveEVMType::Address)))
if select_name
&& (self.check(TokenKind::Ident("x".to_string()))
|| self.check(TokenKind::PrimitiveType(PrimitiveEVMType::Address)))
{
// We need to check if the name is a keyword - not the type
if !on_type {
Expand All @@ -927,7 +946,7 @@ impl Parser {
"Argument names cannot be EVM types: {arg_str}"
)),
spans: AstSpan(vec![self.current_token.span.clone()]),
})
});
}
}
TokenKind::PrimitiveType(ty) => {
Expand Down Expand Up @@ -960,7 +979,7 @@ impl Parser {
kind: ParserErrorKind::InvalidArgs(self.current_token.kind.clone()),
hint: None,
spans: AstSpan(vec![self.current_token.span.clone()]),
})
});
}

arg.span = AstSpan(arg_spans);
Expand Down Expand Up @@ -1038,7 +1057,7 @@ impl Parser {
.to_string(),
),
spans: AstSpan(new_spans),
})
});
}
}
if self.check(TokenKind::Comma) {
Expand Down Expand Up @@ -1085,8 +1104,8 @@ impl Parser {
0_usize
}
})
.sum::<usize>() /
2
.sum::<usize>()
/ 2
}
};

Expand Down Expand Up @@ -1126,7 +1145,7 @@ impl Parser {
),
hint: Some("Expected valid hex bytecode.".to_string()),
spans: AstSpan(new_spans),
})
});
}
} else {
StatementType::LabelCall(ident_str.to_string())
Expand All @@ -1141,7 +1160,7 @@ impl Parser {
kind: ParserErrorKind::InvalidTableBodyToken(kind.clone()),
hint: Some("Expected an identifier string.".to_string()),
spans: AstSpan(new_spans),
})
});
}
};
}
Expand Down Expand Up @@ -1246,7 +1265,7 @@ impl Parser {
kind: ParserErrorKind::InvalidUint256(size),
hint: None,
spans: AstSpan(vec![self.current_token.span.clone()]),
})
});
}
Ok(self.match_kind(self.current_token.kind.clone())?)
}
Expand All @@ -1256,7 +1275,7 @@ impl Parser {
kind: ParserErrorKind::InvalidBytes(size),
hint: None,
spans: AstSpan(vec![self.current_token.span.clone()]),
})
});
}
Ok(self.match_kind(self.current_token.kind.clone())?)
}
Expand All @@ -1270,7 +1289,7 @@ impl Parser {
kind: ParserErrorKind::InvalidInt(size),
hint: None,
spans: AstSpan(vec![self.current_token.span.clone()]),
})
});
}
let curr_token_kind = self.current_token.kind.clone();
self.consume();
Expand Down
64 changes: 64 additions & 0 deletions huff_parser/tests/macro.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1232,3 +1232,67 @@ fn empty_test_with_multi_flag_decorator() {
assert_eq!(macro_definition, expected);
assert_eq!(parser.current_token.kind, TokenKind::Eof);
}

#[test]
fn test_duplicate_macro_error() {
let source = r#"
#define macro CONSTRUCTOR() = takes(0) returns (0) {}
#define macro MINT() = takes(0) returns (0) {
0x04 calldataload // [to]
0x00 // [from (0x00), to]
0x24 calldataload // [value, from, to]
}
#define macro MINT() = takes(0) returns (0) {
0x04 calldataload // [to]
0x00 // [from (0x00), to]
0x24 calldataload // [value, from, to]
}
#define macro MAIN() = takes(0) returns (0) {
0x00 calldataload 0xE0 shr
dup1 0x40c10f19 eq mints jumpi
mints:
MINT()
}
"#;

let mut start = 0;
let target = "MINT";
let target_len = target.len();
let mut occurrences = Vec::new();

while let Some(pos) = source[start..].find(target) {
let adjusted_start = start + pos;
let adjusted_end = adjusted_start + target_len - 1;

occurrences.push((adjusted_start, adjusted_end));
start = adjusted_end + 1;
}

let full_source = FullFileSource { source, file: None, spans: vec![] };
let lexer = Lexer::new(full_source.source);
let tokens = lexer.into_iter().map(|x| x.unwrap()).collect::<Vec<Token>>();
let mut parser = Parser::new(tokens, Some("".to_string()));

// This should be caught before codegen invalid macro statement
match parser.parse() {
Ok(_) => panic!("moose"),
Err(e) => {
assert_eq!(
e,
ParserError {
kind: ParserErrorKind::DuplicateMacro("MINT".to_string()),
hint: Some("MACRO names should be unique".to_string()),
spans: AstSpan(vec![Span {
start: occurrences[1].0,
end: occurrences[1].1,
file: None
}]),
}
)
}
}
}
10 changes: 10 additions & 0 deletions huff_utils/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ pub enum ParserErrorKind {
InvalidDecoratorFlag(String),
/// Invalid decorator flag argument
InvalidDecoratorFlagArg(TokenKind),
/// Duplicate MACRO
DuplicateMacro(String),
}

/// A Lexing Error
Expand Down Expand Up @@ -488,6 +490,14 @@ impl fmt::Display for CompilerError {
pe.spans.error(pe.hint.as_ref())
)
}
ParserErrorKind::DuplicateMacro(mn) => {
write!(
f,
"\nError: Duplicate MACRO name found: \"{}\" \n{}\n",
mn,
pe.spans.error(pe.hint.as_ref())
)
}
},
CompilerError::PathBufRead(os_str) => {
write!(
Expand Down

0 comments on commit 09d7a24

Please sign in to comment.