[PATCH] D33013: Driver must return non-zero code on errors in command line

[Serge Pavlov via Phabricator via cfe-commits]

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

Moved tooling related code into separate change https://reviews.llvm.org/D33272


https://reviews.llvm.org/D33013

Files:
  include/clang/Tooling/CompilationDatabase.h
  lib/Driver/Driver.cpp
  lib/Frontend/CreateInvocationFromCommandLine.cpp
  lib/Tooling/CommonOptionsParser.cpp
  lib/Tooling/CompilationDatabase.cpp
  lib/Tooling/Tooling.cpp
  test/Driver/aarch64-cpus.c
  test/Driver/amdgpu-features.c
  test/Driver/arm-darwin-builtin.c
  test/Driver/arm-default-build-attributes.s
  test/Driver/cl-outputs.c
  test/Driver/clang_f_opts.c
  test/Driver/cuda-external-tools.cu
  test/Driver/debug-options.c
  test/Driver/gfortran.f90
  test/Driver/split-debug.h
  test/Driver/unknown-arg.c
  test/Index/index-attrs.c
  test/Index/index-attrs.cpp
  tools/driver/driver.cpp
  unittests/Driver/ToolChainTest.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp

Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -504,29 +504,35 @@
 
 TEST(ParseFixedCompilationDatabase, ReturnsNullOnEmptyArgumentList) {
   int Argc = 0;
+  std::string ErrorMsg;
   std::unique_ptr Database(
-  FixedCompilationDatabase::loadFromCommandLine(Argc, nullptr));
+  FixedCompilationDatabase::loadFromCommandLine(Argc, nullptr, ErrorMsg));
   EXPECT_FALSE(Database);
+  EXPECT_EQ(ErrorMsg, "error: no arguments specified\n");
   EXPECT_EQ(0, Argc);
 }
 
 TEST(ParseFixedCompilationDatabase, ReturnsNullWithoutDoubleDash) {
   int Argc = 2;
   const char *Argv[] = { "1", "2" };
+  std::string ErrorMsg;
   std::unique_ptr Database(
-  FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
+  FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg));
   EXPECT_FALSE(Database);
+  EXPECT_EQ(ErrorMsg, "error: double dash not found\n");
   EXPECT_EQ(2, Argc);
 }
 
 TEST(ParseFixedCompilationDatabase, ReturnsArgumentsAfterDoubleDash) {
   int Argc = 5;
   const char *Argv[] = {
 "1", "2", "--\0no-constant-folding", "-DDEF3", "-DDEF4"
   };
+  std::string ErrorMsg;
   std::unique_ptr Database(
-  FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
+  FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg));
   ASSERT_TRUE((bool)Database);
+  ASSERT_TRUE(ErrorMsg.empty());
   std::vector Result =
 Database->getCompileCommands("source");
   ASSERT_EQ(1ul, Result.size());
@@ -543,9 +549,11 @@
 TEST(ParseFixedCompilationDatabase, ReturnsEmptyCommandLine) {
   int Argc = 3;
   const char *Argv[] = { "1", "2", "--\0no-constant-folding" };
+  std::string ErrorMsg;
   std::unique_ptr Database(
   FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
   ASSERT_TRUE((bool)Database);
+  ASSERT_TRUE(ErrorMsg.empty());
   std::vector Result =
 Database->getCompileCommands("source");
   ASSERT_EQ(1ul, Result.size());
@@ -560,9 +568,11 @@
 TEST(ParseFixedCompilationDatabase, HandlesPositionalArgs) {
   const char *Argv[] = {"1", "2", "--", "-c", "somefile.cpp", "-DDEF3"};
   int Argc = sizeof(Argv) / sizeof(char*);
+  std::string ErrorMsg;
   std::unique_ptr Database(
   FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
   ASSERT_TRUE((bool)Database);
+  ASSERT_TRUE(ErrorMsg.empty());
   std::vector Result =
 Database->getCompileCommands("source");
   ASSERT_EQ(1ul, Result.size());
@@ -579,9 +589,11 @@
 TEST(ParseFixedCompilationDatabase, HandlesArgv0) {
   const char *Argv[] = {"1", "2", "--", "mytool", "somefile.cpp"};
   int Argc = sizeof(Argv) / sizeof(char*);
+  std::string ErrorMsg;
   std::unique_ptr Database(
   FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
   ASSERT_TRUE((bool)Database);
+  ASSERT_TRUE(ErrorMsg.empty());
   std::vector Result =
 Database->getCompileCommands("source");
   ASSERT_EQ(1ul, Result.size());
Index: unittests/Driver/ToolChainTest.cpp
===
--- unittests/Driver/ToolChainTest.cpp
+++ unittests/Driver/ToolChainTest.cpp
@@ -60,6 +60,7 @@
 
   std::unique_ptr C(TheDriver.BuildCompilation(
   {"-fsyntax-only", "--gcc-toolchain=", "foo.cpp"}));
+  EXPECT_TRUE(C);
 
   std::string S;
   {
@@ -99,6 +100,7 @@
 
   std::unique_ptr C(TheDriver.BuildCompilation(
   {"-fsyntax-only", "--gcc-toolchain=", "foo.cpp"}));
+  EXPECT_TRUE(C);
 
   std::string S;
   {
@@ -128,15 +130,24 @@
 
   Driver CCDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags,
   InMemoryFileSystem);
+  CCDriver.setCheckInputsExist(false);
   Driver CXXDriver("/home/test/bin/clang++", "arm-linux-gnueabi", Diags,
InMemoryFileSystem);
+  CXXDriver.setCheckInputsExist(false);
   Driver CLDriver("/home/test/bin/clang-cl", "arm-linux-gnueabi", Diags,
   InMemoryFileSystem);
-
-  std::unique_ptr 

[PATCH] D33013: Driver must return non-zero code on errors in command line

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: lib/Tooling/CompilationDatabase.cpp:208
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
-  UnusedInputDiagConsumer DiagClient;
+  TextDiagnosticPrinter DiagnosticPrinter(llvm::errs(), &*DiagOpts);
+  UnusedInputDiagConsumer DiagClient(DiagnosticPrinter);

alexfh wrote:
> This code is used as a library not only for command-line tools. Directly 
> using stderr is wrong in many use cases of the Tooling library. It should 
> instead somehow let the user of the library get these errors via a provided 
> DiagnosticConsumer. Not sure how to do this here without a more careful 
> reading of the code, but wanted to let you know that this change causes a 
> regression at least for clang-tidy (and likely for many other Clang tools).
This function does not have a way to report error, so its interface needs to be 
changed first. The change D33272 implements such modification.



https://reviews.llvm.org/D33013



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

[PATCH] D33013: Driver must return non-zero code on errors in command line

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: lib/Tooling/CompilationDatabase.cpp:208
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
-  UnusedInputDiagConsumer DiagClient;
+  TextDiagnosticPrinter DiagnosticPrinter(llvm::errs(), &*DiagOpts);
+  UnusedInputDiagConsumer DiagClient(DiagnosticPrinter);

This code is used as a library not only for command-line tools. Directly using 
stderr is wrong in many use cases of the Tooling library. It should instead 
somehow let the user of the library get these errors via a provided 
DiagnosticConsumer. Not sure how to do this here without a more careful reading 
of the code, but wanted to let you know that this change causes a regression at 
least for clang-tidy (and likely for many other Clang tools).


https://reviews.llvm.org/D33013



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

[PATCH] D33013: Driver must return non-zero code on errors in command line

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 98930.
sepavloff added a comment.
Herald added a subscriber: klimek.

Added missed case

The patch missed a case when Compilation object is created during work of
clang based tool, it resulted in fail of the test 'clang-tidy/diagnostic.cpp'.
This addition to the patch add proper checks, hopefully, to all invocations of
'Driver::BuildCompilation'. Small changes are required in clang-tools-extra,
they are tracked by https://reviews.llvm.org/D33173.


https://reviews.llvm.org/D33013

Files:
  lib/Driver/Driver.cpp
  lib/Frontend/CreateInvocationFromCommandLine.cpp
  lib/Tooling/CompilationDatabase.cpp
  lib/Tooling/Tooling.cpp
  test/Driver/aarch64-cpus.c
  test/Driver/amdgpu-features.c
  test/Driver/arm-darwin-builtin.c
  test/Driver/arm-default-build-attributes.s
  test/Driver/cl-outputs.c
  test/Driver/clang_f_opts.c
  test/Driver/cuda-external-tools.cu
  test/Driver/debug-options.c
  test/Driver/gfortran.f90
  test/Driver/split-debug.h
  test/Driver/unknown-arg.c
  test/Index/index-attrs.c
  test/Index/index-attrs.cpp
  tools/driver/driver.cpp
  unittests/Driver/ToolChainTest.cpp

Index: unittests/Driver/ToolChainTest.cpp
===
--- unittests/Driver/ToolChainTest.cpp
+++ unittests/Driver/ToolChainTest.cpp
@@ -60,6 +60,7 @@
 
   std::unique_ptr C(TheDriver.BuildCompilation(
   {"-fsyntax-only", "--gcc-toolchain=", "foo.cpp"}));
+  EXPECT_TRUE(C);
 
   std::string S;
   {
@@ -99,6 +100,7 @@
 
   std::unique_ptr C(TheDriver.BuildCompilation(
   {"-fsyntax-only", "--gcc-toolchain=", "foo.cpp"}));
+  EXPECT_TRUE(C);
 
   std::string S;
   {
@@ -128,15 +130,24 @@
 
   Driver CCDriver("/home/test/bin/clang", "arm-linux-gnueabi", Diags,
   InMemoryFileSystem);
+  CCDriver.setCheckInputsExist(false);
   Driver CXXDriver("/home/test/bin/clang++", "arm-linux-gnueabi", Diags,
InMemoryFileSystem);
+  CXXDriver.setCheckInputsExist(false);
   Driver CLDriver("/home/test/bin/clang-cl", "arm-linux-gnueabi", Diags,
   InMemoryFileSystem);
-
-  std::unique_ptr CC(CCDriver.BuildCompilation({"foo.cpp"}));
-  std::unique_ptr CXX(CXXDriver.BuildCompilation({"foo.cpp"}));
-  std::unique_ptr CL(CLDriver.BuildCompilation({"foo.cpp"}));
-
+  CLDriver.setCheckInputsExist(false);
+
+  std::unique_ptr CC(CCDriver.BuildCompilation(
+  { "/home/test/bin/clang", "foo.cpp"}));
+  std::unique_ptr CXX(CXXDriver.BuildCompilation(
+  { "/home/test/bin/clang++", "foo.cpp"}));
+  std::unique_ptr CL(CLDriver.BuildCompilation(
+  { "/home/test/bin/clang-cl", "foo.cpp"}));
+
+  EXPECT_TRUE(CC);
+  EXPECT_TRUE(CXX);
+  EXPECT_TRUE(CL);
   EXPECT_TRUE(CCDriver.CCCIsCC());
   EXPECT_TRUE(CXXDriver.CCCIsCXX());
   EXPECT_TRUE(CLDriver.IsCLMode());
Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -454,40 +454,41 @@
   SetBackdoorDriverOutputsFromEnvVars(TheDriver);
 
   std::unique_ptr C(TheDriver.BuildCompilation(argv));
-  int Res = 0;
-  SmallVector, 4> FailingCommands;
-  if (C.get())
+  int Res = 1;
+  if (C.get()) {
+SmallVector, 4> FailingCommands;
 Res = TheDriver.ExecuteCompilation(*C, FailingCommands);
 
-  // Force a crash to test the diagnostics.
-  if (TheDriver.GenReproducer) {
-Diags.Report(diag::err_drv_force_crash)
+// Force a crash to test the diagnostics.
+if (TheDriver.GenReproducer) {
+  Diags.Report(diag::err_drv_force_crash)
 << !::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH");
 
-// Pretend that every command failed.
-FailingCommands.clear();
-for (const auto  : C->getJobs())
-  if (const Command *C = dyn_cast())
-FailingCommands.push_back(std::make_pair(-1, C));
-  }
+  // Pretend that every command failed.
+  FailingCommands.clear();
+  for (const auto  : C->getJobs())
+if (const Command *C = dyn_cast())
+  FailingCommands.push_back(std::make_pair(-1, C));
+}
 
-  for (const auto  : FailingCommands) {
-int CommandRes = P.first;
-const Command *FailingCommand = P.second;
-if (!Res)
-  Res = CommandRes;
-
-// If result status is < 0, then the driver command signalled an error.
-// If result status is 70, then the driver command reported a fatal error.
-// On Windows, abort will return an exit code of 3.  In these cases,
-// generate additional diagnostic information if possible.
-bool DiagnoseCrash = CommandRes < 0 || CommandRes == 70;
+for (const auto  : FailingCommands) {
+  int CommandRes = P.first;
+  const Command *FailingCommand = P.second;
+  if (!Res)
+Res = CommandRes;
+
+  // If result status is < 0, then the driver command signalled an error.
+  // If result status is 70, then the driver command reported a fatal error.
+  // 

[PATCH] D33013: Driver must return non-zero code on errors in command line

[Serge Pavlov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL302775: Driver must return non-zero code on errors in 
command line (authored by sepavloff).

Changed prior to commit:
  https://reviews.llvm.org/D33013?vs=98395=98592#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33013

Files:
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/test/Driver/aarch64-cpus.c
  cfe/trunk/test/Driver/amdgpu-features.c
  cfe/trunk/test/Driver/arm-darwin-builtin.c
  cfe/trunk/test/Driver/arm-default-build-attributes.s
  cfe/trunk/test/Driver/cl-outputs.c
  cfe/trunk/test/Driver/clang_f_opts.c
  cfe/trunk/test/Driver/cuda-external-tools.cu
  cfe/trunk/test/Driver/debug-options.c
  cfe/trunk/test/Driver/gfortran.f90
  cfe/trunk/test/Driver/split-debug.h
  cfe/trunk/test/Driver/unknown-arg.c
  cfe/trunk/test/Index/index-attrs.c
  cfe/trunk/test/Index/index-attrs.cpp
  cfe/trunk/tools/driver/driver.cpp

Index: cfe/trunk/test/Driver/clang_f_opts.c
===
--- cfe/trunk/test/Driver/clang_f_opts.c
+++ cfe/trunk/test/Driver/clang_f_opts.c
@@ -186,7 +186,7 @@
 // CHECK-NO-SLP-VECTORIZE-AGG-NOT: "-vectorize-slp-aggressive"
 
 // RUN: %clang -### -S -fextended-identifiers %s 2>&1 | FileCheck -check-prefix=CHECK-EXTENDED-IDENTIFIERS %s
-// RUN: %clang -### -S -fno-extended-identifiers %s 2>&1 | FileCheck -check-prefix=CHECK-NO-EXTENDED-IDENTIFIERS %s
+// RUN: not %clang -### -S -fno-extended-identifiers %s 2>&1 | FileCheck -check-prefix=CHECK-NO-EXTENDED-IDENTIFIERS %s
 // CHECK-EXTENDED-IDENTIFIERS: "-cc1"
 // CHECK-EXTENDED-IDENTIFIERS-NOT: "-fextended-identifiers"
 // CHECK-NO-EXTENDED-IDENTIFIERS: error: unsupported option '-fno-extended-identifiers'
Index: cfe/trunk/test/Driver/debug-options.c
===
--- cfe/trunk/test/Driver/debug-options.c
+++ cfe/trunk/test/Driver/debug-options.c
@@ -80,7 +80,7 @@
 // RUN: %clang -### -c -gdwarf-2 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
 //
-// RUN: %clang -### -c -gfoo %s 2>&1 | FileCheck -check-prefix=G_NO %s
+// RUN: not %clang -### -c -gfoo %s 2>&1 | FileCheck -check-prefix=G_ERR %s
 // RUN: %clang -### -c -g -g0 %s 2>&1 | FileCheck -check-prefix=G_NO %s
 // RUN: %clang -### -c -ggdb0 %s 2>&1 | FileCheck -check-prefix=G_NO %s
 // RUN: %clang -### -c -glldb -g0 %s 2>&1 | FileCheck -check-prefix=G_NO %s
@@ -171,6 +171,8 @@
 // G_PS4: "-dwarf-version=
 // G_PS4: "-generate-arange-section"
 //
+// G_ERR: error: unknown argument:
+//
 // G_NO: "-cc1"
 // G_NO-NOT: -debug-info-kind=
 //
Index: cfe/trunk/test/Driver/arm-default-build-attributes.s
===
--- cfe/trunk/test/Driver/arm-default-build-attributes.s
+++ cfe/trunk/test/Driver/arm-default-build-attributes.s
@@ -10,9 +10,9 @@
 
 // Option ignored C/C++ (since we always emit hardware and ABI build attributes
 // during codegen).
-// RUN: %clang -target armv7--none-eabi -### -x c %s -mdefault-build-attributes -verify 2>&1 \
+// RUN: %clang -target armv7--none-eabi -### -x c %s -mdefault-build-attributes 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-DISABLED
-// RUN: %clang -target armv7--none-eabi -### -x c++ %s -mdefault-build-attributes -verify 2>&1 \
+// RUN: %clang -target armv7--none-eabi -### -x c++ %s -mdefault-build-attributes 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-DISABLED
 
 // CHECK-DISABLED-NOT: "-arm-add-build-attributes"
Index: cfe/trunk/test/Driver/split-debug.h
===
--- cfe/trunk/test/Driver/split-debug.h
+++ cfe/trunk/test/Driver/split-debug.h
@@ -3,13 +3,4 @@
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -c -fmodules -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
 //
-// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -c -fmodules -emit-module -fmodules-embed-all-files -fno-implicit-modules -fno-implicit-module-maps -### %s 2> %t
-// RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
-//
-// FIXME: This should fail using clang, except that the type of the output for
-// an object output with modules is given as clang::driver::types::TY_PCH
-// rather than TY_Object.
-// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -c -fmodules -fmodule-format=obj -### %s 2> %t
-// RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
-//
 // CHECK-NO-ACTIONS-NOT: objcopy
Index: cfe/trunk/test/Driver/cl-outputs.c
===
--- cfe/trunk/test/Driver/cl-outputs.c
+++ cfe/trunk/test/Driver/cl-outputs.c
@@ -73,7 +73,7 @@
 // RUN: %clang_cl /c /o .. -### -- %s 2>&1 | FileCheck -check-prefix=oCRAZY2 %s
 // oCRAZY2:  "-o" "..obj"
 
-// RUN: %clang_cl /c %s -### /o 2>&1 | FileCheck -check-prefix=oMISSINGARG %s
+// RUN: not %clang_cl /c %s -### /o 2>&1 | FileCheck -check-prefix=oMISSINGARG 

[PATCH] D33013: Driver must return non-zero code on errors in command line

[Richard Smith via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D33013



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

[PATCH] D33013: Driver must return non-zero code on errors in command line

[Serge Pavlov via Phabricator via cfe-commits]

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

Addressed reviewer's notes.


https://reviews.llvm.org/D33013

Files:
  lib/Driver/Driver.cpp
  test/Driver/aarch64-cpus.c
  test/Driver/amdgpu-features.c
  test/Driver/arm-darwin-builtin.c
  test/Driver/arm-default-build-attributes.s
  test/Driver/cl-outputs.c
  test/Driver/clang_f_opts.c
  test/Driver/debug-options.c
  test/Driver/gfortran.f90
  test/Driver/split-debug.h
  test/Driver/unknown-arg.c
  test/Index/index-attrs.c
  test/Index/index-attrs.cpp
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -454,40 +454,41 @@
   SetBackdoorDriverOutputsFromEnvVars(TheDriver);
 
   std::unique_ptr C(TheDriver.BuildCompilation(argv));
-  int Res = 0;
-  SmallVector, 4> FailingCommands;
-  if (C.get())
+  int Res = 1;
+  if (C.get()) {
+SmallVector, 4> FailingCommands;
 Res = TheDriver.ExecuteCompilation(*C, FailingCommands);
 
-  // Force a crash to test the diagnostics.
-  if (TheDriver.GenReproducer) {
-Diags.Report(diag::err_drv_force_crash)
+// Force a crash to test the diagnostics.
+if (TheDriver.GenReproducer) {
+  Diags.Report(diag::err_drv_force_crash)
 << !::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH");
 
-// Pretend that every command failed.
-FailingCommands.clear();
-for (const auto  : C->getJobs())
-  if (const Command *C = dyn_cast())
-FailingCommands.push_back(std::make_pair(-1, C));
-  }
+  // Pretend that every command failed.
+  FailingCommands.clear();
+  for (const auto  : C->getJobs())
+if (const Command *C = dyn_cast())
+  FailingCommands.push_back(std::make_pair(-1, C));
+}
 
-  for (const auto  : FailingCommands) {
-int CommandRes = P.first;
-const Command *FailingCommand = P.second;
-if (!Res)
-  Res = CommandRes;
-
-// If result status is < 0, then the driver command signalled an error.
-// If result status is 70, then the driver command reported a fatal error.
-// On Windows, abort will return an exit code of 3.  In these cases,
-// generate additional diagnostic information if possible.
-bool DiagnoseCrash = CommandRes < 0 || CommandRes == 70;
+for (const auto  : FailingCommands) {
+  int CommandRes = P.first;
+  const Command *FailingCommand = P.second;
+  if (!Res)
+Res = CommandRes;
+
+  // If result status is < 0, then the driver command signalled an error.
+  // If result status is 70, then the driver command reported a fatal error.
+  // On Windows, abort will return an exit code of 3.  In these cases,
+  // generate additional diagnostic information if possible.
+  bool DiagnoseCrash = CommandRes < 0 || CommandRes == 70;
 #ifdef LLVM_ON_WIN32
-DiagnoseCrash |= CommandRes == 3;
+  DiagnoseCrash |= CommandRes == 3;
 #endif
-if (DiagnoseCrash) {
-  TheDriver.generateCompilationDiagnostics(*C, *FailingCommand);
-  break;
+  if (DiagnoseCrash) {
+TheDriver.generateCompilationDiagnostics(*C, *FailingCommand);
+break;
+  }
 }
   }
 
Index: test/Index/index-attrs.cpp
===
--- test/Index/index-attrs.cpp
+++ test/Index/index-attrs.cpp
@@ -1,4 +1,4 @@
-// RUN: c-index-test -index-file -check-prefix CHECK %s -target armv7-windows-gnu -fdeclspec
+// RUN: c-index-test -index-file %s -target armv7-windows-gnu -fdeclspec | FileCheck %s
 
 struct __declspec(dllexport) export_s {
   void m();
@@ -19,32 +19,32 @@
 class __attribute__((dllexport)) export_gnu_s {
   void m();
 };
-// CHECK: [indexDeclaration]: kind: struct | name: export_gnu_s | {{.*}} | lang: C++
+// CHECK: [indexDeclaration]: kind: c++-class | name: export_gnu_s | {{.*}} | lang: C++
 // CHECK: : attribute(dllexport)
 // CHECK: [indexDeclaration]: kind: c++-instance-method | name: m | {{.*}} | lang: C++
 // CHECK: : attribute(dllexport)
 
 class __attribute__((dllimport)) import_gnu_s {
   void m();
 };
-// CHECK: [indexDeclaration]: kind: struct | name: import_gnu_s | {{.*}} | lang: C++
+// CHECK: [indexDeclaration]: kind: c++-class | name: import_gnu_s | {{.*}} | lang: C++
 // CHECK: : attribute(dllimport)
 // CHECK: [indexDeclaration]: kind: c++-instance-method | name: m | {{.*}} | lang: C++
 // CHECK: : attribute(dllimport)
 
 extern "C" void __declspec(dllexport) export_function(void) {}
-// CHECK: [indexDeclaraton]: kind: function | name: export_function | {{.*}} | lang: C
+// CHECK: [indexDeclaration]: kind: function | name: export_function | {{.*}} | lang: C
 // CHECK: : attribute(dllexport)
 extern "C" void __attribute__((dllexport)) export_gnu_function(void) {}
-// CHECK: [indexDeclaraton]: kind: function | name: export_gnu_function | {{.*}} | lang: C
+// CHECK: [indexDeclaration]: 

[PATCH] D33013: Driver must return non-zero code on errors in command line

[Richard Smith via Phabricator via cfe-commits]

rsmith added a comment.

Thank you, some of these test typos are ... alarming. =)

A couple of the test updates don't look quite right, but this mostly looks 
great.




Comment at: test/Driver/amdgpu-features.c:1
-// RUN: %clang -### -target amdgcn -x cl -S -emit-llvm -mcpu=kaveri 
-mamdgpu-debugger-abi=0.0 %s -o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-MAMDGPU-DEBUGGER-ABI-0-0 %s

This should say `-o -`



Comment at: test/Driver/amdgpu-features.c:5
 
-// RUN: %clang -### -target amdgcn -x cl -S -emit-llvm -mcpu=kaveri 
-mamdgpu-debugger-abi=1.0 %s -o 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-MAMDGPU-DEBUGGER-ABI-1-0 %s

Likewise.



Comment at: test/Driver/split-debug.h:6-13
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -c -fmodules 
-emit-module -fno-implicit-modules -fno-implicit-module-maps -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
 //
 // FIXME: This should fail using clang, except that the type of the output for
 // an object output with modules is given as clang::driver::types::TY_PCH
 // rather than TY_Object.
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -c -fmodules 
-### %s 2> %t

These parts of the test don't make sense: the `-fmodule-format=obj` and 
`-emit-module` are `-cc1` options, so testing how the driver deals with them 
doesn't really make a lot of sense. I would suggest deleting the highlighted 
region of this test rather than making it test the same thing three times.


https://reviews.llvm.org/D33013



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

[PATCH] D33013: Driver must return non-zero code on errors in command line

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff created this revision.
Herald added subscribers: krytarowski, javed.absar, nhaehnle, rengolin, 
aemerson.

Now if clang driver is given wrong arguments, in some cases it
continues execution and returns zero code. This change fixes this
behavior.

The fix revealed some errors in clang test set.

File test/Driver/gfortran.f90 added in r118203 checks forwarding
gfortran flags to GCC. Now driver reports error on this file, because
the option -working-directory implemented in clang differs from the
option with the same name implemented in gfortran, in clang the option
requires argument, in gfortran does not.

In the file test/Driver/arm-darwin-builtin.c clang is called with
options -fbuiltin-strcat and -fbuiltin-strcpy. These option were removed
in r191435 and now clang reports error on this test.

File arm-default-build-attributes.s uses option -verify, which is not
supported by driver, it is cc1 option.

Similarly, the file split-debug.h uses options -fmodules-embed-all-files
and -fmodule-format=obj, which are not supported by driver.

Other revealed errors are mainly mistypes.


https://reviews.llvm.org/D33013

Files:
  lib/Driver/Driver.cpp
  test/Driver/aarch64-cpus.c
  test/Driver/amdgpu-features.c
  test/Driver/arm-darwin-builtin.c
  test/Driver/arm-default-build-attributes.s
  test/Driver/cl-outputs.c
  test/Driver/clang_f_opts.c
  test/Driver/debug-options.c
  test/Driver/gfortran.f90
  test/Driver/split-debug.h
  test/Driver/unknown-arg.c
  test/Index/index-attrs.c
  test/Index/index-attrs.cpp
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -454,40 +454,41 @@
   SetBackdoorDriverOutputsFromEnvVars(TheDriver);
 
   std::unique_ptr C(TheDriver.BuildCompilation(argv));
-  int Res = 0;
-  SmallVector, 4> FailingCommands;
-  if (C.get())
+  int Res = 1;
+  if (C.get()) {
+SmallVector, 4> FailingCommands;
 Res = TheDriver.ExecuteCompilation(*C, FailingCommands);
 
-  // Force a crash to test the diagnostics.
-  if (TheDriver.GenReproducer) {
-Diags.Report(diag::err_drv_force_crash)
+// Force a crash to test the diagnostics.
+if (TheDriver.GenReproducer) {
+  Diags.Report(diag::err_drv_force_crash)
 << !::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH");
 
-// Pretend that every command failed.
-FailingCommands.clear();
-for (const auto  : C->getJobs())
-  if (const Command *C = dyn_cast())
-FailingCommands.push_back(std::make_pair(-1, C));
-  }
+  // Pretend that every command failed.
+  FailingCommands.clear();
+  for (const auto  : C->getJobs())
+if (const Command *C = dyn_cast())
+  FailingCommands.push_back(std::make_pair(-1, C));
+}
 
-  for (const auto  : FailingCommands) {
-int CommandRes = P.first;
-const Command *FailingCommand = P.second;
-if (!Res)
-  Res = CommandRes;
-
-// If result status is < 0, then the driver command signalled an error.
-// If result status is 70, then the driver command reported a fatal error.
-// On Windows, abort will return an exit code of 3.  In these cases,
-// generate additional diagnostic information if possible.
-bool DiagnoseCrash = CommandRes < 0 || CommandRes == 70;
+for (const auto  : FailingCommands) {
+  int CommandRes = P.first;
+  const Command *FailingCommand = P.second;
+  if (!Res)
+Res = CommandRes;
+
+  // If result status is < 0, then the driver command signalled an error.
+  // If result status is 70, then the driver command reported a fatal error.
+  // On Windows, abort will return an exit code of 3.  In these cases,
+  // generate additional diagnostic information if possible.
+  bool DiagnoseCrash = CommandRes < 0 || CommandRes == 70;
 #ifdef LLVM_ON_WIN32
-DiagnoseCrash |= CommandRes == 3;
+  DiagnoseCrash |= CommandRes == 3;
 #endif
-if (DiagnoseCrash) {
-  TheDriver.generateCompilationDiagnostics(*C, *FailingCommand);
-  break;
+  if (DiagnoseCrash) {
+TheDriver.generateCompilationDiagnostics(*C, *FailingCommand);
+break;
+  }
 }
   }
 
Index: test/Index/index-attrs.cpp
===
--- test/Index/index-attrs.cpp
+++ test/Index/index-attrs.cpp
@@ -1,4 +1,4 @@
-// RUN: c-index-test -index-file -check-prefix CHECK %s -target armv7-windows-gnu -fdeclspec
+// RUN: c-index-test -index-file %s -target armv7-windows-gnu -fdeclspec | FileCheck %s
 
 struct __declspec(dllexport) export_s {
   void m();
@@ -19,32 +19,32 @@
 class __attribute__((dllexport)) export_gnu_s {
   void m();
 };
-// CHECK: [indexDeclaration]: kind: struct | name: export_gnu_s | {{.*}} | lang: C++
+// CHECK: [indexDeclaration]: kind: c++-class | name: export_gnu_s | {{.*}} | lang: C++
 // CHECK: :