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

Throw error on macro name clash #293

Merged
merged 8 commits into from
Oct 1, 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
81 changes: 50 additions & 31 deletions huff_parser/src/lib.rs
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()]),
Maddiaa0 marked this conversation as resolved.
Show resolved Hide resolved
})
} 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
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
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