[PATCH] D70832: [clangd] Log cc1 args at verbose level.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 3 inline comments as done.
sammccall added a comment.

Sorry about committing this too early, had it confused with another patch. 
Fixed in 4f000824222f97c0cfd5b19951a1068132e57e79 





Comment at: clang-tools-extra/clangd/TUScheduler.cpp:412
 std::unique_ptr Invocation =
 buildCompilerInvocation(Inputs, CompilerInvocationDiagConsumer);
+// Log cc1 args even (especially!) if creating invocation failed.

kbobyrev wrote:
> `buildCompilerInvocation(Inputs, CompilerInvocationDiagConsumer, )`? 
> Otherwise, I think the vector would always be empty, right?
You're right, I changed this after testing it.

Fixed in 4f000824222f97c0cfd5b19951a1068132e57e79



Comment at: clang/include/clang/Frontend/Utils.h:231
+bool ShouldRecoverOnErrors = false,
+std::vector *CC1Args = nullptr);
 

kbobyrev wrote:
> Nit: `llvm::Optional` might look better for `CC1Args`
Doesn't really work as we want optional + pass-by-reference here - a pointer is 
the usual idiom.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70832



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

[PATCH] D70832: [clangd] Log cc1 args at verbose level.

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

Sorry, seems I was too late :(




Comment at: clang-tools-extra/clangd/TUScheduler.cpp:412
 std::unique_ptr Invocation =
 buildCompilerInvocation(Inputs, CompilerInvocationDiagConsumer);
+// Log cc1 args even (especially!) if creating invocation failed.

`buildCompilerInvocation(Inputs, CompilerInvocationDiagConsumer, )`? 
Otherwise, I think the vector would always be empty, right?



Comment at: clang/include/clang/Frontend/Utils.h:231
+bool ShouldRecoverOnErrors = false,
+std::vector *CC1Args = nullptr);
 

Nit: `llvm::Optional` might look better for `CC1Args`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70832



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

[PATCH] D70832: [clangd] Log cc1 args at verbose level.

[Sam McCall via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG407ac2eb5f13: [clangd] Log cc1 args at verbose level. 
(authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70832

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang/include/clang/Frontend/Utils.h
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp

Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -26,7 +26,8 @@
 
 std::unique_ptr clang::createInvocationFromCommandLine(
 ArrayRef ArgList, IntrusiveRefCntPtr Diags,
-IntrusiveRefCntPtr VFS, bool ShouldRecoverOnErorrs) {
+IntrusiveRefCntPtr VFS, bool ShouldRecoverOnErorrs,
+std::vector *CC1Args) {
   if (!Diags.get()) {
 // No diagnostics engine was provided, so create our own diagnostics object
 // with the default options.
@@ -89,6 +90,8 @@
   }
 
   const ArgStringList  = Cmd.getArguments();
+  if (CC1Args)
+*CC1Args = {CCArgs.begin(), CCArgs.end()};
   auto CI = std::make_unique();
   if (!CompilerInvocation::CreateFromArgs(*CI, CCArgs, *Diags) &&
   !ShouldRecoverOnErorrs)
Index: clang/include/clang/Frontend/Utils.h
===
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -217,14 +217,18 @@
 /// non-null (and possibly incorrect) CompilerInvocation if any errors were
 /// encountered. When this flag is false, always return null on errors.
 ///
-/// \return A CompilerInvocation, or 0 if none was built for the given
+/// \param CC1Args - if non-null, will be populated with the args to cc1
+/// expanded from \p Args. May be set even if nullptr is returned.
+///
+/// \return A CompilerInvocation, or nullptr if none was built for the given
 /// argument vector.
 std::unique_ptr createInvocationFromCommandLine(
 ArrayRef Args,
 IntrusiveRefCntPtr Diags =
 IntrusiveRefCntPtr(),
 IntrusiveRefCntPtr VFS = nullptr,
-bool ShouldRecoverOnErrors = false);
+bool ShouldRecoverOnErrors = false,
+std::vector *CC1Args = nullptr);
 
 /// Return the value of the last argument as an integer, or a default. If Diags
 /// is non-null, emits an error if the argument is given, but non-integral.
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -407,8 +407,12 @@
 llvm::join(Inputs.CompileCommand.CommandLine, " "));
 // Rebuild the preamble and the AST.
 StoreDiags CompilerInvocationDiagConsumer;
+std::vector CC1Args;
 std::unique_ptr Invocation =
 buildCompilerInvocation(Inputs, CompilerInvocationDiagConsumer);
+// Log cc1 args even (especially!) if creating invocation failed.
+if (!CC1Args.empty())
+  vlog("cc1 args: {0}", llvm::join(CC1Args, " "));
 std::vector CompilerInvocationDiags =
 CompilerInvocationDiagConsumer.take();
 if (!Invocation) {
Index: clang-tools-extra/clangd/Compiler.h
===
--- clang-tools-extra/clangd/Compiler.h
+++ clang-tools-extra/clangd/Compiler.h
@@ -52,8 +52,8 @@
 
 /// Builds compiler invocation that could be used to build AST or preamble.
 std::unique_ptr
-buildCompilerInvocation(const ParseInputs ,
-clang::DiagnosticConsumer );
+buildCompilerInvocation(const ParseInputs , clang::DiagnosticConsumer ,
+std::vector *CC1Args = nullptr);
 
 /// Creates a compiler instance, configured so that:
 ///   - Contents of the parsed file are remapped to \p MainFile.
Index: clang-tools-extra/clangd/Compiler.cpp
===
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -42,7 +42,8 @@
 
 std::unique_ptr
 buildCompilerInvocation(const ParseInputs ,
-clang::DiagnosticConsumer ) {
+clang::DiagnosticConsumer ,
+std::vector *CC1Args) {
   std::vector ArgStrs;
   for (const auto  : Inputs.CompileCommand.CommandLine)
 ArgStrs.push_back(S.c_str());
@@ -57,7 +58,7 @@
   CompilerInstance::createDiagnostics(new DiagnosticOptions, , false);
   std::unique_ptr CI = createInvocationFromCommandLine(
   ArgStrs, CommandLineDiagsEngine, Inputs.FS,
-  /*ShouldRecoverOnErrors=*/true);
+  /*ShouldRecoverOnErrors=*/true, CC1Args);
   if (!CI)
 return 

[PATCH] D70832: [clangd] Log cc1 args at verbose level.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 231487.
sammccall added a comment.

Fix Compiler.h changes that got mangled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70832

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang/include/clang/Frontend/Utils.h
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp

Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -26,7 +26,8 @@
 
 std::unique_ptr clang::createInvocationFromCommandLine(
 ArrayRef ArgList, IntrusiveRefCntPtr Diags,
-IntrusiveRefCntPtr VFS, bool ShouldRecoverOnErorrs) {
+IntrusiveRefCntPtr VFS, bool ShouldRecoverOnErorrs,
+std::vector *CC1Args) {
   if (!Diags.get()) {
 // No diagnostics engine was provided, so create our own diagnostics object
 // with the default options.
@@ -89,6 +90,8 @@
   }
 
   const ArgStringList  = Cmd.getArguments();
+  if (CC1Args)
+*CC1Args = {CCArgs.begin(), CCArgs.end()};
   auto CI = std::make_unique();
   if (!CompilerInvocation::CreateFromArgs(*CI, CCArgs, *Diags) &&
   !ShouldRecoverOnErorrs)
Index: clang/include/clang/Frontend/Utils.h
===
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -217,14 +217,18 @@
 /// non-null (and possibly incorrect) CompilerInvocation if any errors were
 /// encountered. When this flag is false, always return null on errors.
 ///
-/// \return A CompilerInvocation, or 0 if none was built for the given
+/// \param CC1Args - if non-null, will be populated with the args to cc1
+/// expanded from \p Args. May be set even if nullptr is returned.
+///
+/// \return A CompilerInvocation, or nullptr if none was built for the given
 /// argument vector.
 std::unique_ptr createInvocationFromCommandLine(
 ArrayRef Args,
 IntrusiveRefCntPtr Diags =
 IntrusiveRefCntPtr(),
 IntrusiveRefCntPtr VFS = nullptr,
-bool ShouldRecoverOnErrors = false);
+bool ShouldRecoverOnErrors = false,
+std::vector *CC1Args = nullptr);
 
 /// Return the value of the last argument as an integer, or a default. If Diags
 /// is non-null, emits an error if the argument is given, but non-integral.
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -407,8 +407,12 @@
 llvm::join(Inputs.CompileCommand.CommandLine, " "));
 // Rebuild the preamble and the AST.
 StoreDiags CompilerInvocationDiagConsumer;
+std::vector CC1Args;
 std::unique_ptr Invocation =
 buildCompilerInvocation(Inputs, CompilerInvocationDiagConsumer);
+// Log cc1 args even (especially!) if creating invocation failed.
+if (!CC1Args.empty())
+  vlog("cc1 args: {0}", llvm::join(CC1Args, " "));
 std::vector CompilerInvocationDiags =
 CompilerInvocationDiagConsumer.take();
 if (!Invocation) {
Index: clang-tools-extra/clangd/Compiler.h
===
--- clang-tools-extra/clangd/Compiler.h
+++ clang-tools-extra/clangd/Compiler.h
@@ -52,8 +52,8 @@
 
 /// Builds compiler invocation that could be used to build AST or preamble.
 std::unique_ptr
-buildCompilerInvocation(const ParseInputs ,
-clang::DiagnosticConsumer );
+buildCompilerInvocation(const ParseInputs , clang::DiagnosticConsumer ,
+std::vector *CC1Args = nullptr);
 
 /// Creates a compiler instance, configured so that:
 ///   - Contents of the parsed file are remapped to \p MainFile.
Index: clang-tools-extra/clangd/Compiler.cpp
===
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -42,7 +42,8 @@
 
 std::unique_ptr
 buildCompilerInvocation(const ParseInputs ,
-clang::DiagnosticConsumer ) {
+clang::DiagnosticConsumer ,
+std::vector *CC1Args) {
   std::vector ArgStrs;
   for (const auto  : Inputs.CompileCommand.CommandLine)
 ArgStrs.push_back(S.c_str());
@@ -57,7 +58,7 @@
   CompilerInstance::createDiagnostics(new DiagnosticOptions, , false);
   std::unique_ptr CI = createInvocationFromCommandLine(
   ArgStrs, CommandLineDiagsEngine, Inputs.FS,
-  /*ShouldRecoverOnErrors=*/true);
+  /*ShouldRecoverOnErrors=*/true, CC1Args);
   if (!CI)
 return nullptr;
   // createInvocationFromCommandLine sets DisableFree.
___
cfe-commits mailing list

[PATCH] D70832: [clangd] Log cc1 args at verbose level.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: FAILURE - 
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70832



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

[PATCH] D70832: [clangd] Log cc1 args at verbose level.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kbobyrev.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, javed.absar, ilya-biryukov.
Herald added a project: clang.

This will help debugging driver issues.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70832

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang/include/clang/Frontend/Utils.h
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp

Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -26,7 +26,8 @@
 
 std::unique_ptr clang::createInvocationFromCommandLine(
 ArrayRef ArgList, IntrusiveRefCntPtr Diags,
-IntrusiveRefCntPtr VFS, bool ShouldRecoverOnErorrs) {
+IntrusiveRefCntPtr VFS, bool ShouldRecoverOnErorrs,
+std::vector *CC1Args) {
   if (!Diags.get()) {
 // No diagnostics engine was provided, so create our own diagnostics object
 // with the default options.
@@ -89,6 +90,8 @@
   }
 
   const ArgStringList  = Cmd.getArguments();
+  if (CC1Args)
+*CC1Args = {CCArgs.begin(), CCArgs.end()};
   auto CI = std::make_unique();
   if (!CompilerInvocation::CreateFromArgs(*CI, CCArgs, *Diags) &&
   !ShouldRecoverOnErorrs)
Index: clang/include/clang/Frontend/Utils.h
===
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -217,14 +217,18 @@
 /// non-null (and possibly incorrect) CompilerInvocation if any errors were
 /// encountered. When this flag is false, always return null on errors.
 ///
-/// \return A CompilerInvocation, or 0 if none was built for the given
+/// \param CC1Args - if non-null, will be populated with the args to cc1
+/// expanded from \p Args. May be set even if nullptr is returned.
+///
+/// \return A CompilerInvocation, or nullptr if none was built for the given
 /// argument vector.
 std::unique_ptr createInvocationFromCommandLine(
 ArrayRef Args,
 IntrusiveRefCntPtr Diags =
 IntrusiveRefCntPtr(),
 IntrusiveRefCntPtr VFS = nullptr,
-bool ShouldRecoverOnErrors = false);
+bool ShouldRecoverOnErrors = false,
+std::vector *CC1Args = nullptr);
 
 /// Return the value of the last argument as an integer, or a default. If Diags
 /// is non-null, emits an error if the argument is given, but non-integral.
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -407,8 +407,12 @@
 llvm::join(Inputs.CompileCommand.CommandLine, " "));
 // Rebuild the preamble and the AST.
 StoreDiags CompilerInvocationDiagConsumer;
+std::vector CC1Args;
 std::unique_ptr Invocation =
 buildCompilerInvocation(Inputs, CompilerInvocationDiagConsumer);
+// Log cc1 args even (especially!) if creating invocation failed.
+if (!CC1Args.empty())
+  vlog("cc1 args: {0}", llvm::join(CC1Args, " "));
 std::vector CompilerInvocationDiags =
 CompilerInvocationDiagConsumer.take();
 if (!Invocation) {
Index: clang-tools-extra/clangd/Compiler.h
===
--- clang-tools-extra/clangd/Compiler.h
+++ clang-tools-extra/clangd/Compiler.h
@@ -68,7 +68,8 @@
 std::unique_ptr prepareCompilerInstance(
 std::unique_ptr, const PrecompiledPreamble *,
 std::unique_ptr MainFile,
-IntrusiveRefCntPtr, DiagnosticConsumer &);
+IntrusiveRefCntPtr, DiagnosticConsumer &,
+std::vector *CC1Args = nullptr);
 
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Compiler.cpp
===
--- clang-tools-extra/clangd/Compiler.cpp
+++ clang-tools-extra/clangd/Compiler.cpp
@@ -42,7 +42,8 @@
 
 std::unique_ptr
 buildCompilerInvocation(const ParseInputs ,
-clang::DiagnosticConsumer ) {
+clang::DiagnosticConsumer ,
+std::vector *CC1Args) {
   std::vector ArgStrs;
   for (const auto  : Inputs.CompileCommand.CommandLine)
 ArgStrs.push_back(S.c_str());
@@ -57,7 +58,7 @@
   CompilerInstance::createDiagnostics(new DiagnosticOptions, , false);
   std::unique_ptr CI = createInvocationFromCommandLine(
   ArgStrs, CommandLineDiagsEngine, Inputs.FS,
-  /*ShouldRecoverOnErrors=*/true);
+  /*ShouldRecoverOnErrors=*/true, CC1Args);
   if (!CI)
 return nullptr;
   // createInvocationFromCommandLine sets DisableFree.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org