diff --git a/verilog/analysis/verilog_project.cc b/verilog/analysis/verilog_project.cc index 29819dad9..6afad9d7d 100644 --- a/verilog/analysis/verilog_project.cc +++ b/verilog/analysis/verilog_project.cc @@ -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 { @@ -348,48 +349,58 @@ absl::StatusOr ParseSourceFileListFromFile( absl::StatusOr ParseSourceFileListFromCommandline( const std::vector& 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 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 =. - std::pair 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 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 =. + std::pair 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+[=]', but '' " + "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 diff --git a/verilog/analysis/verilog_project.h b/verilog/analysis/verilog_project.h index f08ecdf86..8df20018e 100644 --- a/verilog/analysis/verilog_project.h +++ b/verilog/analysis/verilog_project.h @@ -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 { diff --git a/verilog/analysis/verilog_project_test.cc b/verilog/analysis/verilog_project_test.cc index 89b6768f2..a0c9fda9e 100644 --- a/verilog/analysis/verilog_project_test.cc +++ b/verilog/analysis/verilog_project_test.cc @@ -592,17 +592,26 @@ TEST(VerilogProjectTest, ParseSourceFileList) { ElementsAre(".", "/an/include_dir1", "/an/include_dir2")); } +TEST(VerilogProjectTest, ParseInvalidSourceFileListFromCommandline) { + std::vector> 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 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 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()); @@ -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 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 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])); diff --git a/verilog/tools/preprocessor/verilog_preprocessor.cc b/verilog/tools/preprocessor/verilog_preprocessor.cc index 3a866eb52..8015ee431 100644 --- a/verilog/tools/preprocessor/verilog_preprocessor.cc +++ b/verilog/tools/preprocessor/verilog_preprocessor.cc @@ -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 cmdline_args(args.begin(),args.end()); - auto parsed_file_list = verilog::ParseSourceFileListFromCommandline(cmdline_args); + std::vector cmdline_args(args.begin(), args.end()); + auto parsed_file_list = + verilog::ParseSourceFileListFromCommandline(cmdline_args); if (!parsed_file_list.ok()) { return parsed_file_list.status(); } @@ -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 cmdline_args(args.begin(),args.end()); - auto parsed_file_list = verilog::ParseSourceFileListFromCommandline(cmdline_args); + std::vector 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"; @@ -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 cmdline_args(args.begin(),args.end()); - auto parsed_file_list = verilog::ParseSourceFileListFromCommandline(cmdline_args); + std::vector cmdline_args(args.begin(), args.end()); + auto parsed_file_list = + verilog::ParseSourceFileListFromCommandline(cmdline_args); if (!parsed_file_list.ok()) { return parsed_file_list.status(); } @@ -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(