Added support of configuration files

[sepavloff]

Summary

Added support of configuration files

Configuration files store set of options and allow to apply all of them
in a simple way, just by selection of particular file. This change extends
CommandLine Library with functions that read such files.

[ABataev]

lib/Support/CommandLine.cpp

956 ↗

(On Diff #72508)

What if Dir is nullptr?

977 ↗

(On Diff #72508)

What if Argv is empty?

984 ↗

(On Diff #72508)

'cfg_option' does not follow LLVM coding standard. And 'x'

1000–1001 ↗

(On Diff #72508)

Use llvm::sys::path::replace_extension(CfgFile, ".cfg")

1036 ↗

(On Diff #72508)

Remove 'else', unconditional 'return CfgFileSearch::Ignored;'

1142 ↗

(On Diff #72508)

What if Cur == Source.begin()?

[sepavloff]

lib/Support/CommandLine.cpp

956 ↗

(On Diff #72508)

It would be programmer's error. Added assert.

977 ↗

(On Diff #72508)

It would be programmers's error - Argv must represents argv from main. Added assert.

984 ↗

(On Diff #72508)

Fixed.

1000–1001 ↗

(On Diff #72508)

That would change semantics. A user can use something like testing.64 as a config name, it would be silently converted to testing.cfg which is not the desired behavior.

1036 ↗

(On Diff #72508)

Fixed.

1142 ↗

(On Diff #72508)

This is an error. Rewrote this place, added unit tests for this function.

[sepavloff]

Updated patch according to reviewer's notes.

[sepavloff]

Updated patch

Based on feedback on mail list added three function instead of cl::findConfigFile.
It allows a user to make more flexible config file search. Also made some cosmetic
changes.

[sepavloff]

Updated documentation

[mgorny]

lib/Support/CommandLine.cpp

1047 ↗

(On Diff #73065)

As I've mentioned on D24933, I think it'd be actually better to have the default dirs stored in LLVM rather than provided by the tools. This way, we can get them consistent in all LLVM tools that are going to use the configuration files, and avoid repeating them in every one of them.

[sepavloff]

Fixed comments and small error in unit test.

[ABataev]

include/llvm/Support/CommandLine.h

1793 ↗

(On Diff #74696)

\brief tag is not required

1802 ↗

(On Diff #74696)

s/TokenizeConfigFile/tokenizeConfigFile/g

1818 ↗

(On Diff #74696)

No \brief

1833 ↗

(On Diff #74696)

No \brief

1844 ↗

(On Diff #74696)

No \brief

1857 ↗

(On Diff #74696)

No \brief

1871 ↗

(On Diff #74696)

No \brief

lib/Support/CommandLine.cpp

716–717 ↗

(On Diff #74696)

Is this sequence expected on Linux too?

1012 ↗

(On Diff #74696)

Use llvm::sys::path::extension(CfgFile) != ".cfg" instead

[sepavloff]

Addressed reviewer's notes.

[sepavloff]

lib/Support/CommandLine.cpp

716–717 ↗

(On Diff #74696)

Config file can be prepared on Windows machine and then be used on Linux. Processing Windows EOLs on Linux host saves users from unexpected behavior in this case.

[sepavloff]

Updated patch

Several small changes:

• Improved documentation,
• cl::readConfigFile got new argument, number of inserted argument. It may be useful if the option read from config file need to be validated.
• function cl::findDefaultCfgFile is renamed to cl::searchForFile. In fact it is general purpose function, not config file specific.

[sepavloff]

Updated patch

• Enhanced documentation,
• Search for config files specified by --config is allowed in binary directory as well,
• Does not add ".cfg" to config files automatically.

[sepavloff]

Updated patch

Simplified the patch:

• removed suport of reading config file name from environment variable.

It is more convenient to read options from such variable rather than the
name of config file. Config file still can be specified by environment
variable using option --config.

• API function do not return error code anymore, they set string argument

to the error text.

Also the function readConfigFile now returns boolean value to indicate
if config file was successfully read.

[sepavloff]

Simplified patch

Configuration files are no more considered a feature of CommandLine
Library. Now the patch contains only minimal set of function necessary
for operations on config files. All other functionality is moved to
clang sources.

[sepavloff]

Updated patch

Support and tests for comments and line continuation are removed from
this patch. These features are now presented in D36045. Special
function for reading config files is still required as nested @file
must be resolved relative to including file, not current directory.

[sepavloff]

Updated patch

Processing comments in config files is now made in the function
tokenizeConfigFile.

[sepavloff]

Could this patch be reviewed?
It is the only dependency for D24933 (Enable configuration files in clang), which is now accepted.

[hfinkel]

lib/Support/CommandLine.cpp

716 ↗

(On Diff #127983)

This seems like a breaking change. Can you add a parameter to control this so that this only happens when called from cl::tokenizeConfigFile.

[sepavloff]

Updated patch

Treatment of trailing backslashes is moved to tokenizeConfigFile.
Now all operations related to config file support are separated from
the rest of code.

[hfinkel]

LGTM

llvm/trunk/include/llvm/Support/CommandLine.h

void TokenizeWindowsCommandLine(StringRef Source, StringSaver &Saver,
                                 bool MarkEOLs = false);
 
 /// \brief String tokenization function type.  Should be compatible with either
 /// Windows or Unix command line tokenizers.
 using TokenizerCallback = void (*)(StringRef Source, StringSaver &Saver,
                                    SmallVectorImpl<const char *> &NewArgv,
                                    bool MarkEOLs);
 
+/// Tokenizes content of configuration file.
+///
+/// \param [in] Source The string representing content of config file.
+/// \param [in] Saver Delegates back to the caller for saving parsed strings.
+/// \param [out] NewArgv All parsed strings are appended to NewArgv.
+/// \param [in] MarkEOLs Added for compatibility with TokenizerCallback.
+///
+/// It works like TokenizeGNUCommandLine with ability to skip comment lines.
+///
+void tokenizeConfigFile(StringRef Source, StringSaver &Saver,
+                        SmallVectorImpl<const char *> &NewArgv,
+                        bool MarkEOLs = false);
+
+/// Reads command line options from the given configuration file.
+///
+/// \param [in] CfgFileName Path to configuration file.
+/// \param [in] Saver  Objects that saves allocated strings.
+/// \param [out] Argv Array to which the read options are added.
+/// \return true if the file was successfully read.
+///
+/// It reads content of the specified file, tokenizes it and expands "@file"
+/// commands resolving file names in them relative to the directory where
+/// CfgFilename resides.
+///
+bool readConfigFile(StringRef CfgFileName, StringSaver &Saver,
+                    SmallVectorImpl<const char *> &Argv);
+
 /// \brief Expand response files on a command line recursively using the given
 /// StringSaver and tokenization strategy.  Argv should contain the command line
 /// before expansion and will be modified in place. If requested, Argv will
 /// also be populated with nullptrs indicating where each response file line
 /// ends, which is useful for the "/link" argument that needs to consume all
 /// remaining arguments only until the next end of line, when in a response
 /// file.
 ///

llvm/trunk/lib/Support/CommandLine.cpp

void cl::TokenizeWindowsCommandLine(StringRef Src, StringSaver &Saver,
   // Append the last token after hitting EOF with no whitespace.
   if (!Token.empty())
     NewArgv.push_back(Saver.save(StringRef(Token)).data());
   // Mark the end of response files
   if (MarkEOLs)
     NewArgv.push_back(nullptr);
 }
 
+void cl::tokenizeConfigFile(StringRef Source, StringSaver &Saver,
+                            SmallVectorImpl<const char *> &NewArgv,
+                            bool MarkEOLs) {
+  for (const char *Cur = Source.begin(); Cur != Source.end();) {
+    SmallString<128> Line;
+    // Check for comment line.
+    if (isWhitespace(*Cur)) {
+      while (Cur != Source.end() && isWhitespace(*Cur))
+        ++Cur;
+      continue;
+    }
+    if (*Cur == '#') {
+      while (Cur != Source.end() && *Cur != '\n')
+        ++Cur;
+      continue;
+    }
+    // Find end of the current line.
+    const char *Start = Cur;
+    for (const char *End = Source.end(); Cur != End; ++Cur) {
+      if (*Cur == '\\') {
+        if (Cur + 1 != End) {
+          ++Cur;
+          if (*Cur == '\n' ||
+              (*Cur == '\r' && (Cur + 1 != End) && Cur[1] == '\n')) {
+            Line.append(Start, Cur - 1);
+            Cur += (*Cur == '\r' ? 2 : 1);
+            Start = Cur;
+          }
+        }
+      } else if (*Cur == '\n')
+        break;
+    }
+    // Tokenize line.
+    Line.append(Start, Cur);
+    cl::TokenizeGNUCommandLine(Line, Saver, NewArgv, MarkEOLs);
+  }
+}
+
 // It is called byte order marker but the UTF-8 BOM is actually not affected
 // by the host system's endianness.
 static bool hasUTF8ByteOrderMark(ArrayRef<char> S) {
   return (S.size() >= 3 && S[0] == '\xef' && S[1] == '\xbb' && S[2] == '\xbf');
 }
 
 static bool ExpandResponseFile(StringRef FName, StringSaver &Saver,
                                TokenizerCallback Tokenizer,
if (!ExpandResponseFile(Arg + 1, Saver, Tokenizer, ExpandedArgv,
       continue;
     }
     Argv.erase(Argv.begin() + I);
     Argv.insert(Argv.begin() + I, ExpandedArgv.begin(), ExpandedArgv.end());
   }
   return AllExpanded;
 }
 
+bool cl::readConfigFile(StringRef CfgFile, StringSaver &Saver,
+                        SmallVectorImpl<const char *> &Argv) {
+  if (!ExpandResponseFile(CfgFile, Saver, cl::tokenizeConfigFile, Argv,
+                          /*MarkEOLs*/ false, /*RelativeNames*/ true))
+    return false;
+  return ExpandResponseFiles(Saver, cl::tokenizeConfigFile, Argv,
+                             /*MarkEOLs*/ false, /*RelativeNames*/ true);
+}
+
 /// ParseEnvironmentOptions - An alternative entry point to the
 /// CommandLine library, which allows you to read the program's name
 /// from the caller (as PROGNAME) and its command-line arguments from
 /// an environment variable (whose name is given in ENVVAR).
 ///
 void cl::ParseEnvironmentOptions(const char *progName, const char *envVar,
                                  const char *Overview) {
   // Check args.

llvm/trunk/unittests/Support/CommandLineTest.cpp

TEST(CommandLineTest, TokenizeWindowsCommandLine) {
   const char Input[] = "a\\b c\\\\d e\\\\\"f g\" h\\\"i j\\\\\\\"k \"lmn\" o pqr "
                       "\"st \\\"u\" \\v";
   const char *const Output[] = { "a\\b", "c\\\\d", "e\\f g", "h\"i", "j\\\"k",
                                  "lmn", "o", "pqr", "st \"u", "\\v" };
   testCommandLineTokenizer(cl::TokenizeWindowsCommandLine, Input, Output,
                            array_lengthof(Output));
 }
 
+TEST(CommandLineTest, TokenizeConfigFile1) {
+  const char *Input = "\\";
+  const char *const Output[] = { "\\" };
+  testCommandLineTokenizer(cl::tokenizeConfigFile, Input, Output,
+                           array_lengthof(Output));
+}
+
+TEST(CommandLineTest, TokenizeConfigFile2) {
+  const char *Input = "\\abc";
+  const char *const Output[] = { "abc" };
+  testCommandLineTokenizer(cl::tokenizeConfigFile, Input, Output,
+                           array_lengthof(Output));
+}
+
+TEST(CommandLineTest, TokenizeConfigFile3) {
+  const char *Input = "abc\\";
+  const char *const Output[] = { "abc\\" };
+  testCommandLineTokenizer(cl::tokenizeConfigFile, Input, Output,
+                           array_lengthof(Output));
+}
+
+TEST(CommandLineTest, TokenizeConfigFile4) {
+  const char *Input = "abc\\\n123";
+  const char *const Output[] = { "abc123" };
+  testCommandLineTokenizer(cl::tokenizeConfigFile, Input, Output,
+                           array_lengthof(Output));
+}
+
+TEST(CommandLineTest, TokenizeConfigFile5) {
+  const char *Input = "abc\\\r\n123";
+  const char *const Output[] = { "abc123" };
+  testCommandLineTokenizer(cl::tokenizeConfigFile, Input, Output,
+                           array_lengthof(Output));
+}
+
+TEST(CommandLineTest, TokenizeConfigFile6) {
+  const char *Input = "abc\\\n";
+  const char *const Output[] = { "abc" };
+  testCommandLineTokenizer(cl::tokenizeConfigFile, Input, Output,
+                           array_lengthof(Output));
+}
+
+TEST(CommandLineTest, TokenizeConfigFile7) {
+  const char *Input = "abc\\\r\n";
+  const char *const Output[] = { "abc" };
+  testCommandLineTokenizer(cl::tokenizeConfigFile, Input, Output,
+                           array_lengthof(Output));
+}
+
+TEST(CommandLineTest, TokenizeConfigFile8) {
+  SmallVector<const char *, 0> Actual;
+  BumpPtrAllocator A;
+  StringSaver Saver(A);
+  cl::tokenizeConfigFile("\\\n", Saver, Actual, /*MarkEOLs=*/false);
+  EXPECT_TRUE(Actual.empty());
+}
+
+TEST(CommandLineTest, TokenizeConfigFile9) {
+  SmallVector<const char *, 0> Actual;
+  BumpPtrAllocator A;
+  StringSaver Saver(A);
+  cl::tokenizeConfigFile("\\\r\n", Saver, Actual, /*MarkEOLs=*/false);
+  EXPECT_TRUE(Actual.empty());
+}
+
+TEST(CommandLineTest, TokenizeConfigFile10) {
+  const char *Input = "\\\nabc";
+  const char *const Output[] = { "abc" };
+  testCommandLineTokenizer(cl::tokenizeConfigFile, Input, Output,
+                           array_lengthof(Output));
+}
+
+TEST(CommandLineTest, TokenizeConfigFile11) {
+  const char *Input = "\\\r\nabc";
+  const char *const Output[] = { "abc" };
+  testCommandLineTokenizer(cl::tokenizeConfigFile, Input, Output,
+                           array_lengthof(Output));
+}
+
 TEST(CommandLineTest, AliasesWithArguments) {
   static const size_t ARGC = 3;
   const char *const Inputs[][ARGC] = {
     { "-tool", "-actual=x", "-extra" },
     { "-tool", "-actual", "x" },
     { "-tool", "-alias=x", "-extra" },
     { "-tool", "-alias", "x" }
   };
for (auto &OM : cl::getRegisteredOptions(*cl::TopLevelSubCommand)) {
     O->setDefault();
   }
 
   EXPECT_TRUE(Opt1 == "true");
   EXPECT_TRUE(Opt2);
   EXPECT_TRUE(Opt3 == 3);
 }
 
+TEST(CommandLineTest, ReadConfigFile) {
+  llvm::SmallVector<const char *, 1> Argv;
+
+  llvm::SmallString<128> TestDir;
+  std::error_code EC =
+      llvm::sys::fs::createUniqueDirectory("unittest", TestDir);
+  EXPECT_TRUE(!EC);
+
+  llvm::SmallString<128> TestCfg;
+  llvm::sys::path::append(TestCfg, TestDir, "foo");
+  std::ofstream ConfigFile(TestCfg.c_str());
+  EXPECT_TRUE(ConfigFile.is_open());
+  ConfigFile << "# Comment\n"
+                "-option_1\n"
+                "@subconfig\n"
+                "-option_3=abcd\n"
+                "-option_4=\\\n"
+                "cdef\n";
+  ConfigFile.close();
+
+  llvm::SmallString<128> TestCfg2;
+  llvm::sys::path::append(TestCfg2, TestDir, "subconfig");
+  std::ofstream ConfigFile2(TestCfg2.c_str());
+  EXPECT_TRUE(ConfigFile2.is_open());
+  ConfigFile2 << "-option_2\n"
+                 "\n"
+                 "   # comment\n";
+  ConfigFile2.close();
+
+  // Make sure the current directory is not the directory where config files
+  // resides. In this case the code that expands response files will not find
+  // 'subconfig' unless it resolves nested inclusions relative to the including
+  // file.
+  llvm::SmallString<128> CurrDir;
+  EC = llvm::sys::fs::current_path(CurrDir);
+  EXPECT_TRUE(!EC);
+  EXPECT_TRUE(StringRef(CurrDir) != StringRef(TestDir));
+
+  llvm::BumpPtrAllocator A;
+  llvm::StringSaver Saver(A);
+  bool Result = llvm::cl::readConfigFile(TestCfg, Saver, Argv);
+
+  EXPECT_TRUE(Result);
+  EXPECT_EQ(Argv.size(), 4U);
+  EXPECT_STREQ(Argv[0], "-option_1");
+  EXPECT_STREQ(Argv[1], "-option_2");
+  EXPECT_STREQ(Argv[2], "-option_3=abcd");
+  EXPECT_STREQ(Argv[3], "-option_4=cdef");
+
+  llvm::sys::fs::remove(TestCfg2);
+  llvm::sys::fs::remove(TestCfg);
+  llvm::sys::fs::remove(TestDir);
+}
+
 }  // anonymous namespace