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

Verilog preprocessor: implement multi-line defines #77

Merged
merged 1 commit into from
Sep 26, 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
14 changes: 14 additions & 0 deletions regression/verilog/preprocessor/multi-line-define1.desc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
CORE
multi-line-define1.v
--preprocess
// Enable multi-line checking
activate-multi-line-match
`line 1 "multi-line-define1.v" 0

A
B
C
^EXIT=0$
^SIGNAL=0$
--
^PREPROCESSING FAILED$
4 changes: 4 additions & 0 deletions regression/verilog/preprocessor/multi-line-define1.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
`define foo A \
B \
C
`foo
14 changes: 12 additions & 2 deletions src/verilog/verilog_preprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -362,13 +362,23 @@ void verilog_preprocessort::directive()
// skip whitespace
tokenizer().skip_ws();

// Read any tokens until end of line.
// Read any tokens until end of line,
// but note that \n can be escaped with a backslash.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't verilog_preprocessor_token_sourcet::skip_until_eol also learn about this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately the standard is very unclear under which circumstances this is supposed to happen. I'll need to experiment with some standard tools.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is global, it might be preferable to move the escaping into the flex tokenizer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Note that any defines in this sequence
// are not expanded at this point.
while(!tokenizer().eof() && tokenizer().peek() != '\n')
{
auto token = tokenizer().next_token();
define.tokens.push_back(std::move(token));
if(token == '\\' && tokenizer().peek() == '\n')
{
// Eat the newline, which is escaped.
// Not clear whether the newline is meant to show
// in the expansion.
auto nl = tokenizer().next_token();
define.tokens.push_back(std::move(nl));
}
else
define.tokens.push_back(std::move(token));
Comment on lines +377 to +381
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a matter of taste:

...
  token = tokenizer().next_token();
}
define.tokens.push_back(std::move(token));

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed a matter of taste; I do feel that the longer version is quicker to understand.

}

#ifdef DEBUG
Expand Down