Skip to content

Commit

Permalink
'FileList' usage in the preprocessor.
Browse files Browse the repository at this point in the history
'ParseSourceFileListFromCommandline()':
- Fix parsing issue.
- Add better error messages.
- Add more tests.

The preprocessor tool:
- Use 'ParseSourceFileListFromCommandline()' in subcommands.
  • Loading branch information
karimtera committed Sep 2, 2022
1 parent 5d18978 commit b1dcd93
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 60 deletions.
77 changes: 44 additions & 33 deletions verilog/analysis/verilog_project.cc
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ FileList ParseSourceFileList(absl::string_view file_list_path,

if (absl::StartsWith(file_path, kIncludeDirPrefix)) {
// Handle includes
// TODO(karimtera): split directories by comma, to allow multiple dirs.
file_list_out.include_dirs.emplace_back(
absl::StripPrefix(file_path, kIncludeDirPrefix));
} else {
Expand All @@ -348,48 +349,58 @@ absl::StatusOr<FileList> ParseSourceFileListFromFile(

absl::StatusOr<FileList> ParseSourceFileListFromCommandline(
const std::vector<absl::string_view>& cmdline) {
FileList file_list_out;
FileList result;
for (absl::string_view argument :
verible::make_range(cmdline.begin(), cmdline.end())) {
if (argument.empty()) continue;
if (argument[0] != '+') {
// Then "argument" is a SV file name.
file_list_out.file_paths.push_back(std::string(argument));
} else {
// it should be either a define or incdir.
std::vector<absl::string_view> argument_plus_splitted =
absl::StrSplit(argument, absl::ByChar('+'), absl::SkipEmpty());
if (argument_plus_splitted.size() < 2) {
return absl::InvalidArgumentError("Unkown argument.");
}
absl::string_view plus_argument_type = argument_plus_splitted[0];
if (plus_argument_type == "define") {
// define argument.
for (const absl::string_view define_argument :
verible::make_range(argument_plus_splitted.begin() + 1,
argument_plus_splitted.end())) {
// argument_plus_splitted[0] is 'define' so it is safe to skip it.
// define_argument is something like <macro1>=<value>.
std::pair<std::string, std::string> macro_pair = absl::StrSplit(
define_argument, absl::ByChar('='), absl::SkipEmpty());
TextMacroDefinition macro{macro_pair.first, macro_pair.second};
// add the define argument.
file_list_out.defines.push_back(macro);
}
} else if (plus_argument_type == "incdir") {
// incdir argument.
for (const absl::string_view incdir_argument :
verible::make_range(argument_plus_splitted.begin() + 1,
argument_plus_splitted.end())) {
// argument_plus_splitted[0] is 'incdir' so it is safe to skip it.
file_list_out.include_dirs.push_back(std::string(incdir_argument));
result.file_paths.push_back(std::string(argument));
continue;
}
// It should be either a define or incdir.
std::vector<absl::string_view> argument_plus_splitted =
absl::StrSplit(argument, absl::ByChar('+'), absl::SkipEmpty());
if (argument_plus_splitted.size() < 2) {
return absl::InvalidArgumentError(
absl::StrCat("ERROR: Expected either '+define+' or '+incdir+' "
"followed by the parameter but got '",
argument, "'"));
}
absl::string_view plus_argument_type = argument_plus_splitted[0];
if (plus_argument_type == "define") {
// define argument.
for (const absl::string_view define_argument :
verible::make_range(argument_plus_splitted.begin() + 1,
argument_plus_splitted.end())) {
// argument_plus_splitted[0] is 'define' so it is safe to skip it.
// define_argument is something like <macro1>=<value>.
std::pair<std::string, std::string> macro_pair = absl::StrSplit(
define_argument, absl::MaxSplits('=', 1), absl::SkipEmpty());
if (absl::StrContains(define_argument, '=') &&
macro_pair.second.empty()) {
return absl::InvalidArgumentError(
"ERROR: Expected '+define+<macro>[=<value>]', but '<value>' "
"after '=' is missing");
}
} else {
return absl::InvalidArgumentError("Unkown argument.");
// add the define argument.
result.defines.emplace_back(macro_pair.first, macro_pair.second);
}
} else if (plus_argument_type == "incdir") {
// incdir argument.
for (const absl::string_view incdir_argument :
verible::make_range(argument_plus_splitted.begin() + 1,
argument_plus_splitted.end())) {
// argument_plus_splitted[0] is 'incdir' so it is safe to skip it.
result.include_dirs.emplace_back(std::string(incdir_argument));
}
} else {
return absl::InvalidArgumentError(absl::StrCat(
"ERROR: Expected either '+define+' or '+incdir+' but got '",
plus_argument_type, "'"));
}
}
return file_list_out;
return result;
}

} // namespace verilog
2 changes: 2 additions & 0 deletions verilog/analysis/verilog_project.h
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,8 @@ class VerilogProject {

// TODO(karimtera): Using "MacroDefiniton" struct might be better.
struct TextMacroDefinition {
TextMacroDefinition(std::string name, std::string value)
: name(std::move(name)), value(std::move(value)){};
std::string name;
std::string value;
bool operator==(const TextMacroDefinition& macro_definition) const {
Expand Down
40 changes: 24 additions & 16 deletions verilog/analysis/verilog_project_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -592,17 +592,26 @@ TEST(VerilogProjectTest, ParseSourceFileList) {
ElementsAre(".", "/an/include_dir1", "/an/include_dir2"));
}

TEST(VerilogProjectTest, ParseInvalidSourceFileListFromCommandline) {
std::vector<std::vector<absl::string_view>> test_cases = {
{"+define+macro1="}, {"+define+"}, {"+not_valid_define+"}};
for (const auto& cmdline : test_cases) {
auto parsed_file_list = ParseSourceFileListFromCommandline(cmdline);
ASSERT_FALSE(parsed_file_list.ok());
}
}

TEST(VerilogProjectTest, ParseSourceFileListFromCommandline) {
std::vector<absl::string_view> cmdline;
cmdline.push_back("+define+macro1=text1+macro2+macro3=text3");
cmdline.push_back("file1");
cmdline.push_back("+define+macro4");
cmdline.push_back("file2");
cmdline.push_back("+incdir+~/path/to/file1+path/to/file2");
cmdline.push_back("+incdir+./path/to/file3");
cmdline.push_back("+define+macro5");
cmdline.push_back("file3");
cmdline.push_back("+incdir+../path/to/file4+./path/to/file5");
std::vector<absl::string_view> cmdline = {
"+define+macro1=text1+macro2+macro3=text3",
"file1",
"+define+macro4",
"file2",
"+incdir+~/path/to/file1+path/to/file2",
"+incdir+./path/to/file3",
"+define+macro5",
"file3",
"+incdir+../path/to/file4+./path/to/file5"};
auto parsed_file_list = ParseSourceFileListFromCommandline(cmdline);
ASSERT_TRUE(parsed_file_list.ok());

Expand All @@ -611,12 +620,11 @@ TEST(VerilogProjectTest, ParseSourceFileListFromCommandline) {
EXPECT_THAT(parsed_file_list->include_dirs,
ElementsAre("~/path/to/file1", "path/to/file2", "./path/to/file3",
"../path/to/file4", "./path/to/file5"));
std::vector<TextMacroDefinition> macros;
macros.push_back(TextMacroDefinition{"macro1", "text1"});
macros.push_back(TextMacroDefinition{"macro2", ""});
macros.push_back(TextMacroDefinition{"macro3", "text3"});
macros.push_back(TextMacroDefinition{"macro4", ""});
macros.push_back(TextMacroDefinition{"macro5", ""});
std::vector<TextMacroDefinition> macros = {{"macro1", "text1"},
{"macro2", ""},
{"macro3", "text3"},
{"macro4", ""},
{"macro5", ""}};
EXPECT_THAT(
parsed_file_list->defines,
ElementsAre(macros[0], macros[1], macros[2], macros[3], macros[4]));
Expand Down
23 changes: 12 additions & 11 deletions verilog/tools/preprocessor/verilog_preprocessor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ static absl::Status StripComments(const SubcommandArgsRange& args,
std::istream&, std::ostream& outs,
std::ostream&) {
// Parse the arguments into a FileList.
std::vector<absl::string_view> cmdline_args(args.begin(),args.end());
auto parsed_file_list = verilog::ParseSourceFileListFromCommandline(cmdline_args);
std::vector<absl::string_view> cmdline_args(args.begin(), args.end());
auto parsed_file_list =
verilog::ParseSourceFileListFromCommandline(cmdline_args);
if (!parsed_file_list.ok()) {
return parsed_file_list.status();
}
Expand Down Expand Up @@ -120,17 +121,17 @@ static absl::Status MultipleCU(const SubcommandArgsRange& args, std::istream&,
std::ostream& outs,
std::ostream& message_stream) {
// Parse the arguments into a FileList.
std::vector<absl::string_view> cmdline_args(args.begin(),args.end());
auto parsed_file_list = verilog::ParseSourceFileListFromCommandline(cmdline_args);
std::vector<absl::string_view> cmdline_args(args.begin(), args.end());
auto parsed_file_list =
verilog::ParseSourceFileListFromCommandline(cmdline_args);
if (!parsed_file_list.ok()) {
return parsed_file_list.status();
}
const auto& files = parsed_file_list->file_paths;
//TODO(karimtera): Pass defines and incdirs to "PreprocessSingleFile()".
// TODO(karimtera): Pass defines and incdirs to "PreprocessSingleFile()".

if (files.empty()) {
return absl::InvalidArgumentError(
"ERROR: Missing file argument.");
return absl::InvalidArgumentError("ERROR: Missing file argument.");
}
for (const absl::string_view source_file : files) {
message_stream << source_file << ":\n";
Expand All @@ -145,8 +146,9 @@ static absl::Status GenerateVariants(const SubcommandArgsRange& args,
std::istream&, std::ostream& outs,
std::ostream& message_stream) {
// Parse the arguments into a FileList.
std::vector<absl::string_view> cmdline_args(args.begin(),args.end());
auto parsed_file_list = verilog::ParseSourceFileListFromCommandline(cmdline_args);
std::vector<absl::string_view> cmdline_args(args.begin(), args.end());
auto parsed_file_list =
verilog::ParseSourceFileListFromCommandline(cmdline_args);
if (!parsed_file_list.ok()) {
return parsed_file_list.status();
}
Expand All @@ -155,8 +157,7 @@ static absl::Status GenerateVariants(const SubcommandArgsRange& args,
const int limit_variants = absl::GetFlag(FLAGS_limit_variants);

if (files.empty()) {
return absl::InvalidArgumentError(
"ERROR: Missing file argument.");
return absl::InvalidArgumentError("ERROR: Missing file argument.");
}
if (files.size() > 1) {
return absl::InvalidArgumentError(
Expand Down

0 comments on commit b1dcd93

Please sign in to comment.