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

[VeriblePreProcessor][2]: Positional arguments to pass defines and inclusion directories. #1373

Conversation

karimtera
Copy link
Contributor

NOTE: This is a part of the sequentially splitted PRs from PR #1360.

Description:
This PR implements a class that handles positional arguments for the preprocessor tool.
Any of the following positional argument can be passed one or more times.

Argument Description
<file.sv> Specifies a SV file to be preprocessed.
+define+<foo>[=<text>] Defines one or more macros same as `define foo text
+incdir+<dir> Specifies the directory to search for files included with `include "file_name"

@tgorochowik tgorochowik requested a review from hzeller August 4, 2022 15:00
Copy link
Collaborator

@hzeller hzeller left a comment

Choose a reason for hiding this comment

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

I think if we use your code to parse things into a struct FileList (and add a container in there for the defines), we have a good starting point to use it for all tools.

- Using FileList in the preprocessor tool instead of CmdPositionalArguments.
Copy link
Collaborator

@hzeller hzeller left a comment

Choose a reason for hiding this comment

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

Nice. Make sure to add a unit test. Also, use absl::string_view where possible, avoid std::string unless you actually need a copy of the data.

ParseSourceFileListFromCommandline:
- Always uses absl::string_view unless we really need to copy the data.
- Added unit test containing defines, include directories, and source files.
@hzeller
Copy link
Collaborator

hzeller commented Sep 2, 2022

Here the first start would of course to rebase to current head.

But since there are a lot of changes that you essentially have to integrate, and this one is a somewhat independent (just the commandline changes), it might be easier if you start a fresh pull request: you just copy over the verilog_project.h, verilog_project.cc and verilog_project_test.cc as well as relevant BUILD rules over from here into a fresh fork from head and create a fresh pull request from that (and abandon this)

Up to you.

@karimtera
Copy link
Contributor Author

That would be easier, I am on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants