Adding support of +define+ in FileList.

verilog/analysis/verilog_project.cc

@@ -24,6 +24,7 @@
 #include "absl/strings/match.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/str_join.h"
+#include "absl/strings/string_view.h"
 #include "absl/strings/strip.h"
 #include "absl/time/time.h"
 #include "absl/types/optional.h"
@@ -327,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 {
@@ -345,4 +347,57 @@ absl::StatusOr<FileList> ParseSourceFileListFromFile(
   return ParseSourceFileList(file_list_file, content);
 }
 
+absl::StatusOr<FileList> ParseSourceFileListFromCommandline(
+    const std::vector<absl::string_view>& cmdline) {
+  FileList result;
+  for (absl::string_view argument : cmdline) {
+    if (argument.empty()) continue;
+    if (argument[0] != '+') {
+      // Then "argument" is a SV file name.
+      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") {
+      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");
+        }
+        // add the define argument.
+        result.defines.emplace_back(macro_pair.first, macro_pair.second);
+      }
+    } else if (plus_argument_type == "incdir") {
+      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 result;
+}
+
 }  // namespace verilog

verilog/analysis/verilog_project.h

@@ -319,6 +319,17 @@ class VerilogProject {
       buffer_to_analyzer_map_;
 };
 
+// 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 {
+    return name == macro_definition.name && value == macro_definition.value;
+  }
+};
+
 // File list for compiling a System Verilog project.
 struct FileList {
   // Ordered list of files to compile.
@@ -327,6 +338,8 @@ struct FileList {
   std::vector<std::string> include_dirs;
   // Path to the file list.
   std::string file_list_path;
+  // Defined macros.
+  std::vector<TextMacroDefinition> defines;
 };
 
 // Reads in a list of files line-by-line from 'file_list_file'. The include
@@ -339,6 +352,11 @@ absl::StatusOr<FileList> ParseSourceFileListFromFile(
 FileList ParseSourceFileList(absl::string_view file_list_path,
                              const std::string& file_list_content);
 
+// Parse positional parameters from command line and extract files, +incdir+ and
+// +define+.
+absl::StatusOr<FileList> ParseSourceFileListFromCommandline(
+    const std::vector<absl::string_view>& cmdline);
+
 }  // namespace verilog
 
 #endif  // VERIBLE_VERILOG_ANALYSIS_VERILOG_PROJECT_H_

verilog/analysis/verilog_project_test.cc

@@ -592,5 +592,42 @@ 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);
+    EXPECT_FALSE(parsed_file_list.ok());
+  }
+}
+
+TEST(VerilogProjectTest, ParseSourceFileListFromCommandline) {
+  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",
+      "+define+macro6=a=b",
+      "+incdir+../path/to/file4+./path/to/file5"};
+  auto parsed_file_list = ParseSourceFileListFromCommandline(cmdline);
+  ASSERT_TRUE(parsed_file_list.ok());
+
+  EXPECT_THAT(parsed_file_list->file_paths,
+              ElementsAre("file1", "file2", "file3"));
+  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 = {
+      {"macro1", "text1"}, {"macro2", ""}, {"macro3", "text3"},
+      {"macro4", ""},      {"macro5", ""}, {"macro6", "a=b"}};
+  EXPECT_THAT(parsed_file_list->defines,
+              ElementsAre(macros[0], macros[1], macros[2], macros[3], macros[4],
+                          macros[5]));
+}
+
 }  // namespace
 }  // namespace verilog

verilog/tools/preprocessor/BUILD

@@ -17,6 +17,7 @@ cc_binary(
         "//verilog/preprocessor:verilog_preprocess",
         "//verilog/parser:verilog_lexer",
         "//verilog/analysis:verilog_analyzer",
+        "//verilog/analysis:verilog_project",
         "//verilog/analysis:flow_tree",
         "@com_google_absl//absl/flags:flag",
         "@com_google_absl//absl/flags:usage",

verilog/tools/preprocessor/verilog_preprocessor.cc

@@ -15,6 +15,7 @@
 #include <functional>
 #include <iostream>
 #include <string>
+#include <vector>
 
 #include "absl/flags/flag.h"
 #include "absl/flags/usage.h"
@@ -26,6 +27,7 @@
 #include "common/util/subcommand.h"
 #include "verilog/analysis/flow_tree.h"
 #include "verilog/analysis/verilog_analyzer.h"
+#include "verilog/analysis/verilog_project.h"
 #include "verilog/parser/verilog_lexer.h"
 #include "verilog/preprocessor/verilog_preprocess.h"
 #include "verilog/transform/strip_comments.h"
@@ -38,11 +40,20 @@ ABSL_FLAG(int, limit_variants, 20, "Maximum number of variants printed");
 static absl::Status StripComments(const SubcommandArgsRange& args,
                                   std::istream&, std::ostream& outs,
                                   std::ostream&) {
-  if (args.empty()) {
+  // 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);
+  if (!parsed_file_list.ok()) {
+    return parsed_file_list.status();
+  }
+  const auto& files = parsed_file_list->file_paths;
+
+  if (files.empty()) {
     return absl::InvalidArgumentError(
         "Missing file argument.  Use '-' for stdin.");
   }
-  const absl::string_view source_file = args[0];
+  const absl::string_view source_file = files[0];
   std::string source_contents;
   if (auto status = verible::file::GetContents(source_file, &source_contents);
       !status.ok()) {
@@ -80,7 +91,7 @@ static absl::Status PreprocessSingleFile(absl::string_view source_file,
     return status;
   }
   verilog::VerilogPreprocess::Config config;
-  config.filter_branches = 1;
+  config.filter_branches = true;
   // config.expand_macros=1;
   verilog::VerilogPreprocess preprocessor(config);
   verilog::VerilogLexer lexer(source_contents);
@@ -90,8 +101,9 @@ static absl::Status PreprocessSingleFile(absl::string_view source_file,
     // For now we will store the syntax tree tokens only, ignoring all the
     // white-space characters. however that should be stored to output the
     // source code just like it was, but with conditionals filtered.
-    if (verilog::VerilogLexer::KeepSyntaxTreeTokens(lexer.GetLastToken()))
+    if (verilog::VerilogLexer::KeepSyntaxTreeTokens(lexer.GetLastToken())) {
       lexed_sequence.push_back(lexer.GetLastToken());
+    }
   }
   verible::TokenStreamView lexed_streamview;
   // Initializing the lexed token stream view.
@@ -108,10 +120,20 @@ static absl::Status PreprocessSingleFile(absl::string_view source_file,
 static absl::Status MultipleCU(const SubcommandArgsRange& args, std::istream&,
                                std::ostream& outs,
                                std::ostream& message_stream) {
-  if (args.empty()) {
-    return absl::InvalidArgumentError("Missing file arguments.");
+  // 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);
+  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()".
+
+  if (files.empty()) {
+    return absl::InvalidArgumentError("ERROR: Missing file argument.");
   }
-  for (absl::string_view source_file : args) {
+  for (const absl::string_view source_file : files) {
     message_stream << source_file << ":\n";
     auto status = PreprocessSingleFile(source_file, outs, message_stream);
     if (!status.ok()) return status;
@@ -123,12 +145,27 @@ static absl::Status MultipleCU(const SubcommandArgsRange& args, std::istream&,
 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);
+  if (!parsed_file_list.ok()) {
+    return parsed_file_list.status();
+  }
+  const auto& files = parsed_file_list->file_paths;
+  // TODO(karimtera): Pass the +define's to the preprocessor, and only
+  // generate variants with theses defines fixed.
+
   const int limit_variants = absl::GetFlag(FLAGS_limit_variants);
-  if (args.size() > 1) {
+
+  if (files.empty()) {
+    return absl::InvalidArgumentError("ERROR: Missing file argument.");
+  }
+  if (files.size() > 1) {
     return absl::InvalidArgumentError(
         "ERROR: generate-variants only works on one file.");
   }
-  const absl::string_view source_file = args[0];
+  const auto& source_file = files[0];
   std::string source_contents;
   if (auto status = verible::file::GetContents(source_file, &source_contents);
       !status.ok()) {