[PATCH] D136090: Handle errors in expansion of response files

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

In D136090#3905845 , @sepavloff wrote:

> Thank you for reporting the issue. It was fixed in fdab9f1203ee 
> .

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136090: Handle errors in expansion of response files

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

Thank you for reporting the issue. It was fixed in fdab9f1203ee 
.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136090: Handle errors in expansion of response files

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

Hello,

I'm wondering if this is really working as expected/intended.

With this patch, if I do

  clang empty.c @atfileesc.txt -dry-run

on an empty file empty.c and atfileesc.txt containing just

  -I @name-with-at/

then I get the following error

  cannot open file: /repo/uabelho/main-github/llvm/atfileesc.txt
  Program aborted due to an unhandled Error:
  cannot open file: /repo/uabelho/main-github/llvm/atfileesc.txt
  PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
and include the crash backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.Program arguments: build-all/bin/clang empty.c @atfileesc.txt -dry-run
   #0 0x02fc9873 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(build-all/bin/clang+0x2fc9873)
   #1 0x02fc758e llvm::sys::RunSignalHandlers() 
(build-all/bin/clang+0x2fc758e)
   #2 0x02fc9bf6 SignalHandler(int) (build-all/bin/clang+0x2fc9bf6)
   #3 0x7f678e7ae630 __restore_rt (/lib64/libpthread.so.0+0xf630)
   #4 0x7f678bef5387 raise (/lib64/libc.so.6+0x36387)
   #5 0x7f678bef6a78 abort (/lib64/libc.so.6+0x37a78)
   #6 0x02f39d00 llvm::Error::fatalUncheckedError() const 
(build-all/bin/clang+0x2f39d00)
   #7 0x009d5e3a clang_main(int, char**) (build-all/bin/clang+0x9d5e3a)
   #8 0x7f678bee1555 __libc_start_main (/lib64/libc.so.6+0x22555)
   #9 0x009d233b _start (build-all/bin/clang+0x9d233b)
  Abort (core dumped)

Note that the file it is complaining about indeed exists:

  -> ls -l /repo/uabelho/main-github/llvm/atfileesc.txt
  -rw-r--r-- 1 uabelho eusers 18 Nov  1 11:15 
/repo/uabelho/main-github/llvm/atfileesc.txt

and as I wrote above it contains

  -> cat /repo/uabelho/main-github/llvm/atfileesc.txt
  -I @name-with-at/


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136090: Handle errors in expansion of response files

[Serge Pavlov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG17eb198de934: Handle errors in expansion of response files 
(authored by sepavloff).

Changed prior to commit:
  https://reviews.llvm.org/D136090?vs=469978=471753#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
  clang/test/Driver/config-file-errs.c
  clang/tools/driver/driver.cpp
  clang/unittests/Driver/ToolChainTest.cpp
  flang/tools/flang-driver/driver.cpp
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -871,7 +871,7 @@
   llvm::BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, llvm::cl::TokenizeGNUCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot).setRelativeNames(true);
-  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
   EXPECT_THAT(Argv, testing::Pointwise(
 StringEquality(),
 {"test/test", "-flag_1", "-option_1", "-option_2",
@@ -933,7 +933,14 @@
 #endif
   llvm::cl::ExpansionContext ECtx(A, Tokenizer);
   ECtx.setVFS().setCurrentDir(TestRoot);
-  ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
+  llvm::Error Err = ECtx.expandResponseFiles(Argv);
+  ASSERT_TRUE((bool)Err);
+  SmallString<128> FilePath = SelfFilePath;
+  std::error_code EC = FS.makeAbsolute(FilePath);
+  ASSERT_FALSE((bool)EC);
+  std::string ExpectedMessage =
+  std::string("recursive expansion of: '") + std::string(FilePath) + "'";
+  ASSERT_TRUE(toString(std::move(Err)) == ExpectedMessage);
 
   EXPECT_THAT(Argv,
   testing::Pointwise(StringEquality(),
@@ -971,7 +978,7 @@
   BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot);
-  ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
 
   // ASSERT instead of EXPECT to prevent potential out-of-bounds access.
   ASSERT_EQ(Argv.size(), 1 + NON_RSP_AT_ARGS + 2);
@@ -1005,7 +1012,7 @@
   BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot).setRelativeNames(true);
-  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
   EXPECT_THAT(Argv,
   testing::Pointwise(StringEquality(), {"test/test", "-flag"}));
 }
@@ -1025,7 +1032,7 @@
   llvm::cl::ExpansionContext ECtx(A, cl::TokenizeWindowsCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot).setMarkEOLs(true).setRelativeNames(
   true);
-  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
   const char *Expected[] = {"clang", "-Xclang", "-Wno-whatever", nullptr,
 "input.cpp"};
   ASSERT_EQ(std::size(Expected), Argv.size());
@@ -1038,6 +1045,30 @@
   }
 }
 
+TEST(CommandLineTest, BadResponseFile) {
+  BumpPtrAllocator A;
+  StringSaver Saver(A);
+  TempDir ADir("dir", /*Unique*/ true);
+  SmallString<128> AFilePath = ADir.path();
+  llvm::sys::path::append(AFilePath, "file.rsp");
+  std::string AFileExp = std::string("@") + std::string(AFilePath.str());
+  SmallVector Argv = {"clang", AFileExp.c_str()};
+
+  bool Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
+  ASSERT_TRUE(Res);
+  ASSERT_EQ(2U, Argv.size());
+  ASSERT_STREQ(Argv[0], "clang");
+  ASSERT_STREQ(Argv[1], AFileExp.c_str());
+
+  std::string ADirExp = std::string("@") + std::string(ADir.path());
+  Argv = {"clang", ADirExp.c_str()};
+  Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
+  ASSERT_FALSE(Res);
+  ASSERT_EQ(2U, Argv.size());
+  ASSERT_STREQ(Argv[0], "clang");
+  ASSERT_STREQ(Argv[1], ADirExp.c_str());
+}
+
 TEST(CommandLineTest, SetDefaultValue) {
   cl::ResetCommandLineParser();
 
@@ -1145,9 +1176,9 @@
 
   llvm::BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, cl::tokenizeConfigFile);
-  bool Result = ECtx.readConfigFile(ConfigFile.path(), Argv);
+  llvm::Error Result = ECtx.readConfigFile(ConfigFile.path(), Argv);
 
-  EXPECT_TRUE(Result);
+  EXPECT_FALSE((bool)Result);
   EXPECT_EQ(Argv.size(), 13U);
   EXPECT_STREQ(Argv[0], "-option_1");
   EXPECT_STREQ(Argv[1],
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -1153,14 +1153,14 @@
 }
 
 // FName must be an absolute path.
-llvm::Error

[PATCH] D136090: Handle errors in expansion of response files

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136090: Handle errors in expansion of response files

[Michał Górny via Phabricator via cfe-commits]

mgorny accepted this revision.
mgorny added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang/unittests/Driver/ToolChainTest.cpp:604-606
+  char *StrBuff = (char *)Alloc.Allocate(6, 2);
+  std::memcpy(StrBuff, "\xFF\xFE\x00\xD8\x00\x00", 6);
+  StringRef BadUTF(StrBuff, 6);

sepavloff wrote:
> mgorny wrote:
> > Wouldn't it be possible to use `std::string`, or maybe even 
> > `std::array` here? I think it'd be less error-prone.
> Nor `std::string` neither `std::array` provide custom alignment.
> 
> UTF-16 string is constructed here from array of chars to avoid problems with 
> endianness. It consists of 2-byte elements and is expected to be aligned on 
> 2-byte boundary. Array of chars is aligned on byte and sometimes the test 
> failed due to invalid alignment.
> 
> So we have to use bare pointer to have guaranteed alignment.
> 
Ah. Could you leave a comment to make the alignment requirements clear? I'm a 
bit surprised that we have to meet alignment requirements when "reading" a file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136090: Handle errors in expansion of response files

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/unittests/Driver/ToolChainTest.cpp:604-606
+  char *StrBuff = (char *)Alloc.Allocate(6, 2);
+  std::memcpy(StrBuff, "\xFF\xFE\x00\xD8\x00\x00", 6);
+  StringRef BadUTF(StrBuff, 6);

mgorny wrote:
> Wouldn't it be possible to use `std::string`, or maybe even `std::array ...>` here? I think it'd be less error-prone.
Nor `std::string` neither `std::array` provide custom alignment.

UTF-16 string is constructed here from array of chars to avoid problems with 
endianness. It consists of 2-byte elements and is expected to be aligned on 
2-byte boundary. Array of chars is aligned on byte and sometimes the test 
failed due to invalid alignment.

So we have to use bare pointer to have guaranteed alignment.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136090: Handle errors in expansion of response files

[Michał Górny via Phabricator via cfe-commits]

mgorny added inline comments.



Comment at: clang/unittests/Driver/ToolChainTest.cpp:604-606
+  char *StrBuff = (char *)Alloc.Allocate(6, 2);
+  std::memcpy(StrBuff, "\xFF\xFE\x00\xD8\x00\x00", 6);
+  StringRef BadUTF(StrBuff, 6);

Wouldn't it be possible to use `std::string`, or maybe even `std::array` here? I think it'd be less error-prone.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136090: Handle errors in expansion of response files

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136090: Handle errors in expansion of response files

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:1029
 if (llvm::sys::path::is_relative(CfgFilePath)) {
-  if (getVFS().makeAbsolute(CfgFilePath))
-return true;
-  auto Status = getVFS().status(CfgFilePath);
-  if (!Status ||
-  Status->getType() != llvm::sys::fs::file_type::regular_file) {
-Diag(diag::err_drv_config_file_not_exist) << CfgFilePath;
+  if (std::error_code EC = getVFS().makeAbsolute(CfgFilePath)) {
+Diag(diag::err_drv_cannot_open_config_file)

mgorny wrote:
> Either I'm missing something or you're not using `EC`.
You are right, this is remnants of previous implementation. Thanks for the 
catch.



Comment at: clang/lib/Driver/Driver.cpp:1041
+}
+if (Status->getType() != llvm::sys::fs::file_type::regular_file) {
+  Diag(diag::err_drv_cannot_open_config_file)

mgorny wrote:
> Not necessarily a goal for this patch but wouldn't this check fit better in 
> `readConfigFile()`?
Yes, it makes sense. Moved.



Comment at: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp:67-69
+  if (Err) {
+llvm::errs() << Err;
+  }

mgorny wrote:
> The braces seem unnecessary.
Fixed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136090: Handle errors in expansion of response files

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 469978.
sepavloff added a comment.

Updated patch


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
  clang/test/Driver/config-file-errs.c
  clang/tools/driver/driver.cpp
  clang/unittests/Driver/ToolChainTest.cpp
  flang/tools/flang-driver/driver.cpp
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -871,7 +871,7 @@
   llvm::BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, llvm::cl::TokenizeGNUCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot).setRelativeNames(true);
-  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
   EXPECT_THAT(Argv, testing::Pointwise(
 StringEquality(),
 {"test/test", "-flag_1", "-option_1", "-option_2",
@@ -933,7 +933,14 @@
 #endif
   llvm::cl::ExpansionContext ECtx(A, Tokenizer);
   ECtx.setVFS().setCurrentDir(TestRoot);
-  ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
+  llvm::Error Err = ECtx.expandResponseFiles(Argv);
+  ASSERT_TRUE((bool)Err);
+  SmallString<128> FilePath = SelfFilePath;
+  std::error_code EC = FS.makeAbsolute(FilePath);
+  ASSERT_FALSE((bool)EC);
+  std::string ExpectedMessage =
+  std::string("recursive expansion of: '") + std::string(FilePath) + "'";
+  ASSERT_TRUE(toString(std::move(Err)) == ExpectedMessage);
 
   EXPECT_THAT(Argv,
   testing::Pointwise(StringEquality(),
@@ -971,7 +978,7 @@
   BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot);
-  ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
 
   // ASSERT instead of EXPECT to prevent potential out-of-bounds access.
   ASSERT_EQ(Argv.size(), 1 + NON_RSP_AT_ARGS + 2);
@@ -1005,7 +1012,7 @@
   BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot).setRelativeNames(true);
-  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
   EXPECT_THAT(Argv,
   testing::Pointwise(StringEquality(), {"test/test", "-flag"}));
 }
@@ -1025,7 +1032,7 @@
   llvm::cl::ExpansionContext ECtx(A, cl::TokenizeWindowsCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot).setMarkEOLs(true).setRelativeNames(
   true);
-  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
   const char *Expected[] = {"clang", "-Xclang", "-Wno-whatever", nullptr,
 "input.cpp"};
   ASSERT_EQ(std::size(Expected), Argv.size());
@@ -1038,6 +1045,30 @@
   }
 }
 
+TEST(CommandLineTest, BadResponseFile) {
+  BumpPtrAllocator A;
+  StringSaver Saver(A);
+  TempDir ADir("dir", /*Unique*/ true);
+  SmallString<128> AFilePath = ADir.path();
+  llvm::sys::path::append(AFilePath, "file.rsp");
+  std::string AFileExp = std::string("@") + std::string(AFilePath.str());
+  SmallVector Argv = {"clang", AFileExp.c_str()};
+
+  bool Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
+  ASSERT_TRUE(Res);
+  ASSERT_EQ(2U, Argv.size());
+  ASSERT_STREQ(Argv[0], "clang");
+  ASSERT_STREQ(Argv[1], AFileExp.c_str());
+
+  std::string ADirExp = std::string("@") + std::string(ADir.path());
+  Argv = {"clang", ADirExp.c_str()};
+  Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
+  ASSERT_FALSE(Res);
+  ASSERT_EQ(2U, Argv.size());
+  ASSERT_STREQ(Argv[0], "clang");
+  ASSERT_STREQ(Argv[1], ADirExp.c_str());
+}
+
 TEST(CommandLineTest, SetDefaultValue) {
   cl::ResetCommandLineParser();
 
@@ -1145,9 +1176,9 @@
 
   llvm::BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, cl::tokenizeConfigFile);
-  bool Result = ECtx.readConfigFile(ConfigFile.path(), Argv);
+  llvm::Error Result = ECtx.readConfigFile(ConfigFile.path(), Argv);
 
-  EXPECT_TRUE(Result);
+  EXPECT_FALSE((bool)Result);
   EXPECT_EQ(Argv.size(), 13U);
   EXPECT_STREQ(Argv[0], "-option_1");
   EXPECT_STREQ(Argv[1],
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -1153,14 +1153,14 @@
 }
 
 // FName must be an absolute path.
-llvm::Error
-ExpansionContext::expandResponseFile(StringRef FName,
- SmallVectorImpl ) {
+Error ExpansionContext::expandResponseFile(
+StringRef 

[PATCH] D136090: Handle errors in expansion of response files

[Michał Górny via Phabricator via cfe-commits]

mgorny added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:1029
 if (llvm::sys::path::is_relative(CfgFilePath)) {
-  if (getVFS().makeAbsolute(CfgFilePath))
-return true;
-  auto Status = getVFS().status(CfgFilePath);
-  if (!Status ||
-  Status->getType() != llvm::sys::fs::file_type::regular_file) {
-Diag(diag::err_drv_config_file_not_exist) << CfgFilePath;
+  if (std::error_code EC = getVFS().makeAbsolute(CfgFilePath)) {
+Diag(diag::err_drv_cannot_open_config_file)

Either I'm missing something or you're not using `EC`.



Comment at: clang/lib/Driver/Driver.cpp:1041
+}
+if (Status->getType() != llvm::sys::fs::file_type::regular_file) {
+  Diag(diag::err_drv_cannot_open_config_file)

Not necessarily a goal for this patch but wouldn't this check fit better in 
`readConfigFile()`?



Comment at: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp:67-69
+  if (Err) {
+llvm::errs() << Err;
+  }

The braces seem unnecessary.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136090: Handle errors in expansion of response files

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: llvm/lib/Support/CommandLine.cpp:1188
+  // macros.
+  if (!RelativeNames && !InConfigFile)
 return Error::success();

rovka wrote:
> Why do you need to add `!InConfigFile` here and below? 
It is not necessary, because now configuration file is expanded with the flag 
`RelativeNames` set. It however seems more correct to check for `InConfigFile` 
also, hopefully it make code a bit clearly.



Comment at: llvm/lib/Support/CommandLine.cpp:1204
+StringRef FileName;
+if (ArgStr[0] == '@') {
+  FileName = ArgStr.drop_front(1);

rovka wrote:
> Nit: Why reverse this branch? The code around it `continue`s early, so it's 
> easier to read if this `continue`s early too.
You are right. Initially this patch was combined with D136354, where additional 
checks was added. I changed this code to be more natural.



Comment at: llvm/lib/Support/CommandLine.cpp:1205
+if (ArgStr[0] == '@') {
+  FileName = ArgStr.drop_front(1);
+  if (!llvm::sys::path::is_relative(FileName))

mgorny wrote:
> Also, I think you can use `consume_front()` here.
Yes, changed code accordingly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136090: Handle errors in expansion of response files

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 469957.
sepavloff added a comment.

Small changes proposed by reviewers


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
  clang/test/Driver/config-file-errs.c
  clang/tools/driver/driver.cpp
  clang/unittests/Driver/ToolChainTest.cpp
  flang/tools/flang-driver/driver.cpp
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -871,7 +871,7 @@
   llvm::BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, llvm::cl::TokenizeGNUCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot).setRelativeNames(true);
-  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
   EXPECT_THAT(Argv, testing::Pointwise(
 StringEquality(),
 {"test/test", "-flag_1", "-option_1", "-option_2",
@@ -933,7 +933,14 @@
 #endif
   llvm::cl::ExpansionContext ECtx(A, Tokenizer);
   ECtx.setVFS().setCurrentDir(TestRoot);
-  ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
+  llvm::Error Err = ECtx.expandResponseFiles(Argv);
+  ASSERT_TRUE((bool)Err);
+  SmallString<128> FilePath = SelfFilePath;
+  std::error_code EC = FS.makeAbsolute(FilePath);
+  ASSERT_FALSE((bool)EC);
+  std::string ExpectedMessage =
+  std::string("recursive expansion of: '") + std::string(FilePath) + "'";
+  ASSERT_TRUE(toString(std::move(Err)) == ExpectedMessage);
 
   EXPECT_THAT(Argv,
   testing::Pointwise(StringEquality(),
@@ -971,7 +978,7 @@
   BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot);
-  ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
 
   // ASSERT instead of EXPECT to prevent potential out-of-bounds access.
   ASSERT_EQ(Argv.size(), 1 + NON_RSP_AT_ARGS + 2);
@@ -1005,7 +1012,7 @@
   BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot).setRelativeNames(true);
-  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
   EXPECT_THAT(Argv,
   testing::Pointwise(StringEquality(), {"test/test", "-flag"}));
 }
@@ -1025,7 +1032,7 @@
   llvm::cl::ExpansionContext ECtx(A, cl::TokenizeWindowsCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot).setMarkEOLs(true).setRelativeNames(
   true);
-  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
   const char *Expected[] = {"clang", "-Xclang", "-Wno-whatever", nullptr,
 "input.cpp"};
   ASSERT_EQ(std::size(Expected), Argv.size());
@@ -1038,6 +1045,30 @@
   }
 }
 
+TEST(CommandLineTest, BadResponseFile) {
+  BumpPtrAllocator A;
+  StringSaver Saver(A);
+  TempDir ADir("dir", /*Unique*/ true);
+  SmallString<128> AFilePath = ADir.path();
+  llvm::sys::path::append(AFilePath, "file.rsp");
+  std::string AFileExp = std::string("@") + std::string(AFilePath.str());
+  SmallVector Argv = {"clang", AFileExp.c_str()};
+
+  bool Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
+  ASSERT_TRUE(Res);
+  ASSERT_EQ(2U, Argv.size());
+  ASSERT_STREQ(Argv[0], "clang");
+  ASSERT_STREQ(Argv[1], AFileExp.c_str());
+
+  std::string ADirExp = std::string("@") + std::string(ADir.path());
+  Argv = {"clang", ADirExp.c_str()};
+  Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
+  ASSERT_FALSE(Res);
+  ASSERT_EQ(2U, Argv.size());
+  ASSERT_STREQ(Argv[0], "clang");
+  ASSERT_STREQ(Argv[1], ADirExp.c_str());
+}
+
 TEST(CommandLineTest, SetDefaultValue) {
   cl::ResetCommandLineParser();
 
@@ -1145,9 +1176,9 @@
 
   llvm::BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, cl::tokenizeConfigFile);
-  bool Result = ECtx.readConfigFile(ConfigFile.path(), Argv);
+  llvm::Error Result = ECtx.readConfigFile(ConfigFile.path(), Argv);
 
-  EXPECT_TRUE(Result);
+  EXPECT_FALSE((bool)Result);
   EXPECT_EQ(Argv.size(), 13U);
   EXPECT_STREQ(Argv[0], "-option_1");
   EXPECT_STREQ(Argv[1],
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -1153,14 +1153,14 @@
 }
 
 // FName must be an absolute path.
-llvm::Error
-ExpansionContext::expandResponseFile(StringRef FName,
- SmallVectorImpl ) {
+Error 

[PATCH] D136090: Handle errors in expansion of response files

[Michał Górny via Phabricator via cfe-commits]

mgorny added inline comments.



Comment at: llvm/lib/Support/CommandLine.cpp:1205
+if (ArgStr[0] == '@') {
+  FileName = ArgStr.drop_front(1);
+  if (!llvm::sys::path::is_relative(FileName))

Also, I think you can use `consume_front()` here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136090: Handle errors in expansion of response files

[Diana Picus via Phabricator via cfe-commits]

rovka added a comment.

Nice tests! :) I have one microscopic nit and one question (because I don't 
know an awful lot about config files). Thanks for working on this.




Comment at: llvm/lib/Support/CommandLine.cpp:1188
+  // macros.
+  if (!RelativeNames && !InConfigFile)
 return Error::success();

Why do you need to add `!InConfigFile` here and below? 



Comment at: llvm/lib/Support/CommandLine.cpp:1204
+StringRef FileName;
+if (ArgStr[0] == '@') {
+  FileName = ArgStr.drop_front(1);

Nit: Why reverse this branch? The code around it `continue`s early, so it's 
easier to read if this `continue`s early too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136090: Handle errors in expansion of response files

[Diana Picus via Phabricator via cfe-commits]

rovka added inline comments.



Comment at: clang/test/Driver/config-file-errs.c:16
 // RUN: not %clang --config somewhere/nonexistent-config-file 2>&1 | FileCheck 
%s -check-prefix CHECK-NONEXISTENT
-// CHECK-NONEXISTENT: configuration file 
'{{.*}}somewhere/nonexistent-config-file' does not exist
+// CHECK-NONEXISTENT: configuration file 
'{{.*}}somewhere/nonexistent-config-file' cannot be opened: No such file or 
directory
 

I think you might need to use the same trick as [[ 
https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/output-paths.f90
 | this test ]]  to get the test to pass on both Windows and Linux (since they 
use different capitalization).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136090: Handle errors in expansion of response files

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 469286.
sepavloff added a comment.

Make UTF-16 string properly aligned


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136090/new/

https://reviews.llvm.org/D136090

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
  clang/test/Driver/config-file-errs.c
  clang/tools/driver/driver.cpp
  clang/unittests/Driver/ToolChainTest.cpp
  flang/tools/flang-driver/driver.cpp
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -871,7 +871,7 @@
   llvm::BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, llvm::cl::TokenizeGNUCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot).setRelativeNames(true);
-  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
   EXPECT_THAT(Argv, testing::Pointwise(
 StringEquality(),
 {"test/test", "-flag_1", "-option_1", "-option_2",
@@ -933,7 +933,14 @@
 #endif
   llvm::cl::ExpansionContext ECtx(A, Tokenizer);
   ECtx.setVFS().setCurrentDir(TestRoot);
-  ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
+  llvm::Error Err = ECtx.expandResponseFiles(Argv);
+  ASSERT_TRUE((bool)Err);
+  SmallString<128> FilePath = SelfFilePath;
+  std::error_code EC = FS.makeAbsolute(FilePath);
+  ASSERT_FALSE((bool)EC);
+  std::string ExpectedMessage =
+  std::string("recursive expansion of: '") + std::string(FilePath) + "'";
+  ASSERT_TRUE(toString(std::move(Err)) == ExpectedMessage);
 
   EXPECT_THAT(Argv,
   testing::Pointwise(StringEquality(),
@@ -971,7 +978,7 @@
   BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot);
-  ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
 
   // ASSERT instead of EXPECT to prevent potential out-of-bounds access.
   ASSERT_EQ(Argv.size(), 1 + NON_RSP_AT_ARGS + 2);
@@ -1005,7 +1012,7 @@
   BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot).setRelativeNames(true);
-  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
   EXPECT_THAT(Argv,
   testing::Pointwise(StringEquality(), {"test/test", "-flag"}));
 }
@@ -1025,7 +1032,7 @@
   llvm::cl::ExpansionContext ECtx(A, cl::TokenizeWindowsCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot).setMarkEOLs(true).setRelativeNames(
   true);
-  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
   const char *Expected[] = {"clang", "-Xclang", "-Wno-whatever", nullptr,
 "input.cpp"};
   ASSERT_EQ(std::size(Expected), Argv.size());
@@ -1038,6 +1045,30 @@
   }
 }
 
+TEST(CommandLineTest, BadResponseFile) {
+  BumpPtrAllocator A;
+  StringSaver Saver(A);
+  TempDir ADir("dir", /*Unique*/ true);
+  SmallString<128> AFilePath = ADir.path();
+  llvm::sys::path::append(AFilePath, "file.rsp");
+  std::string AFileExp = std::string("@") + std::string(AFilePath.str());
+  SmallVector Argv = {"clang", AFileExp.c_str()};
+
+  bool Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
+  ASSERT_TRUE(Res);
+  ASSERT_EQ(2U, Argv.size());
+  ASSERT_STREQ(Argv[0], "clang");
+  ASSERT_STREQ(Argv[1], AFileExp.c_str());
+
+  std::string ADirExp = std::string("@") + std::string(ADir.path());
+  Argv = {"clang", ADirExp.c_str()};
+  Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
+  ASSERT_FALSE(Res);
+  ASSERT_EQ(2U, Argv.size());
+  ASSERT_STREQ(Argv[0], "clang");
+  ASSERT_STREQ(Argv[1], ADirExp.c_str());
+}
+
 TEST(CommandLineTest, SetDefaultValue) {
   cl::ResetCommandLineParser();
 
@@ -1145,9 +1176,9 @@
 
   llvm::BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, cl::tokenizeConfigFile);
-  bool Result = ECtx.readConfigFile(ConfigFile.path(), Argv);
+  llvm::Error Result = ECtx.readConfigFile(ConfigFile.path(), Argv);
 
-  EXPECT_TRUE(Result);
+  EXPECT_FALSE((bool)Result);
   EXPECT_EQ(Argv.size(), 13U);
   EXPECT_STREQ(Argv[0], "-option_1");
   EXPECT_STREQ(Argv[1],
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -1153,14 +1153,14 @@
 }
 
 // FName must be an absolute path.
-llvm::Error
-ExpansionContext::expandResponseFile(StringRef FName,
- SmallVectorImpl ) {
+Error 

[PATCH] D136090: Handle errors in expansion of response files

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff created this revision.
sepavloff added reviewers: kadircet, mgorny, MaskRay, smeenai.
Herald added subscribers: StephenFan, hiraditya.
Herald added a reviewer: awarzynski.
Herald added projects: Flang, All.
sepavloff requested review of this revision.
Herald added subscribers: llvm-commits, jdoerfert.
Herald added projects: clang, LLVM.

Previously an error raised during an expansion of response files (including
configuration files) was ignored and only the fact of its presence was
reported to the user with generic error messages. This made it difficult to
analyze problems. For example, if a configuration file tried to read an
inexistent file, the error message said that 'configuration file cannot
be found', which is wrong and misleading.

This change enhances handling errors in the expansion so that users
could get more informative error messages.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136090

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
  clang/test/Driver/config-file-errs.c
  clang/tools/driver/driver.cpp
  clang/unittests/Driver/ToolChainTest.cpp
  flang/tools/flang-driver/driver.cpp
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -871,7 +871,7 @@
   llvm::BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, llvm::cl::TokenizeGNUCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot).setRelativeNames(true);
-  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
   EXPECT_THAT(Argv, testing::Pointwise(
 StringEquality(),
 {"test/test", "-flag_1", "-option_1", "-option_2",
@@ -933,7 +933,14 @@
 #endif
   llvm::cl::ExpansionContext ECtx(A, Tokenizer);
   ECtx.setVFS().setCurrentDir(TestRoot);
-  ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
+  llvm::Error Err = ECtx.expandResponseFiles(Argv);
+  ASSERT_TRUE((bool)Err);
+  SmallString<128> FilePath = SelfFilePath;
+  std::error_code EC = FS.makeAbsolute(FilePath);
+  ASSERT_FALSE((bool)EC);
+  std::string ExpectedMessage =
+  std::string("recursive expansion of: '") + std::string(FilePath) + "'";
+  ASSERT_TRUE(toString(std::move(Err)) == ExpectedMessage);
 
   EXPECT_THAT(Argv,
   testing::Pointwise(StringEquality(),
@@ -971,7 +978,7 @@
   BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot);
-  ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
 
   // ASSERT instead of EXPECT to prevent potential out-of-bounds access.
   ASSERT_EQ(Argv.size(), 1 + NON_RSP_AT_ARGS + 2);
@@ -1005,7 +1012,7 @@
   BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, cl::TokenizeGNUCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot).setRelativeNames(true);
-  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
   EXPECT_THAT(Argv,
   testing::Pointwise(StringEquality(), {"test/test", "-flag"}));
 }
@@ -1025,7 +1032,7 @@
   llvm::cl::ExpansionContext ECtx(A, cl::TokenizeWindowsCommandLine);
   ECtx.setVFS().setCurrentDir(TestRoot).setMarkEOLs(true).setRelativeNames(
   true);
-  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
+  ASSERT_FALSE((bool)ECtx.expandResponseFiles(Argv));
   const char *Expected[] = {"clang", "-Xclang", "-Wno-whatever", nullptr,
 "input.cpp"};
   ASSERT_EQ(std::size(Expected), Argv.size());
@@ -1038,6 +1045,30 @@
   }
 }
 
+TEST(CommandLineTest, BadResponseFile) {
+  BumpPtrAllocator A;
+  StringSaver Saver(A);
+  TempDir ADir("dir", /*Unique*/ true);
+  SmallString<128> AFilePath = ADir.path();
+  llvm::sys::path::append(AFilePath, "file.rsp");
+  std::string AFileExp = std::string("@") + std::string(AFilePath.str());
+  SmallVector Argv = {"clang", AFileExp.c_str()};
+
+  bool Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
+  ASSERT_TRUE(Res);
+  ASSERT_EQ(2U, Argv.size());
+  ASSERT_STREQ(Argv[0], "clang");
+  ASSERT_STREQ(Argv[1], AFileExp.c_str());
+
+  std::string ADirExp = std::string("@") + std::string(ADir.path());
+  Argv = {"clang", ADirExp.c_str()};
+  Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
+  ASSERT_FALSE(Res);
+  ASSERT_EQ(2U, Argv.size());
+  ASSERT_STREQ(Argv[0], "clang");
+  ASSERT_STREQ(Argv[1], ADirExp.c_str());
+}
+
 TEST(CommandLineTest, SetDefaultValue) {
   cl::ResetCommandLineParser();
 
@@ -1145,9 +1176,9 @@
 
   llvm::BumpPtrAllocator A;
   llvm::cl::ExpansionContext ECtx(A, cl::tokenizeConfigFile);
-  bool