[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D39799#920780, @JVApen wrote:

> How does this new file know if it should handle it's flags as  it does in 
> clang.exe or clang-cl.exe?


It doesn't set argv[0], so it's treated like `clang` by default.

I believe the flag `--driver-mode={gcc,g++,cpp,cl}` can be used to change this.
(I should add this to the brief doc)


Repository:
  rL LLVM

https://reviews.llvm.org/D39799



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[JVApen via Phabricator via cfe-commits]

JVApen added a comment.

How does this new file know if it should handle it's flags as  it does in 
clang.exe or clang-cl.exe?


Repository:
  rL LLVM

https://reviews.llvm.org/D39799



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL31: [Tooling] Use FixedCompilationDatabase when 
`compile_flags.txt` is found. (authored by sammccall).

Repository:
  rL LLVM

https://reviews.llvm.org/D39799

Files:
  cfe/trunk/docs/JSONCompilationDatabase.rst
  cfe/trunk/include/clang/Tooling/CompilationDatabase.h
  cfe/trunk/lib/Tooling/CompilationDatabase.cpp
  cfe/trunk/test/Tooling/Inputs/fixed-header.h
  cfe/trunk/test/Tooling/fixed-database.cpp

Index: cfe/trunk/lib/Tooling/CompilationDatabase.cpp
===
--- cfe/trunk/lib/Tooling/CompilationDatabase.cpp
+++ cfe/trunk/lib/Tooling/CompilationDatabase.cpp
@@ -10,6 +10,9 @@
 //  This file contains implementations of the CompilationDatabase base class
 //  and the FixedCompilationDatabase.
 //
+//  FIXME: Various functions that take a string  should be upgraded
+//  to Expected.
+//
 //===--===//
 
 #include "clang/Tooling/CompilationDatabase.h"
@@ -26,6 +29,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/LineIterator.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -302,8 +306,22 @@
   std::vector StrippedArgs;
   if (!stripPositionalArgs(CommandLine, StrippedArgs, ErrorMsg))
 return nullptr;
-  return std::unique_ptr(
-  new FixedCompilationDatabase(Directory, StrippedArgs));
+  return llvm::make_unique(Directory, StrippedArgs);
+}
+
+std::unique_ptr
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string ) {
+  ErrorMsg.clear();
+  llvm::ErrorOr File =
+  llvm::MemoryBuffer::getFile(Path);
+  if (std::error_code Result = File.getError()) {
+ErrorMsg = "Error while opening fixed database: " + Result.message();
+return nullptr;
+  }
+  std::vector Args{llvm::line_iterator(**File),
+llvm::line_iterator()};
+  return llvm::make_unique(
+  llvm::sys::path::parent_path(Path), std::move(Args));
 }
 
 FixedCompilationDatabase::
@@ -334,6 +352,22 @@
   return std::vector();
 }
 
+namespace {
+
+class FixedCompilationDatabasePlugin : public CompilationDatabasePlugin {
+  std::unique_ptr
+  loadFromDirectory(StringRef Directory, std::string ) override {
+SmallString<1024> DatabasePath(Directory);
+llvm::sys::path::append(DatabasePath, "compile_flags.txt");
+return FixedCompilationDatabase::loadFromFile(DatabasePath, ErrorMessage);
+  }
+};
+
+static CompilationDatabasePluginRegistry::Add
+X("fixed-compilation-database", "Reads plain-text flags file");
+
+} // namespace
+
 namespace clang {
 namespace tooling {
 
Index: cfe/trunk/docs/JSONCompilationDatabase.rst
===
--- cfe/trunk/docs/JSONCompilationDatabase.rst
+++ cfe/trunk/docs/JSONCompilationDatabase.rst
@@ -91,3 +91,9 @@
 the top of the build directory. Clang tools are pointed to the top of
 the build directory to detect the file and use the compilation database
 to parse C++ code in the source tree.
+
+Alternatives
+
+For simple projects, Clang tools also recognize a compile_flags.txt file.
+This should contain one flag per line. The same flags will be used to compile
+any file.
Index: cfe/trunk/include/clang/Tooling/CompilationDatabase.h
===
--- cfe/trunk/include/clang/Tooling/CompilationDatabase.h
+++ cfe/trunk/include/clang/Tooling/CompilationDatabase.h
@@ -182,6 +182,11 @@
   int , const char *const *Argv, std::string ,
   Twine Directory = ".");
 
+  /// Reads flags from the given file, one-per line.
+  /// Returns nullptr and sets ErrorMessage if we can't read the file.
+  static std::unique_ptr
+  loadFromFile(StringRef Path, std::string );
+
   /// \brief Constructs a compilation data base from a specified directory
   /// and command line.
   FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine);
Index: cfe/trunk/test/Tooling/fixed-database.cpp
===
--- cfe/trunk/test/Tooling/fixed-database.cpp
+++ cfe/trunk/test/Tooling/fixed-database.cpp
@@ -0,0 +1,18 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/Src
+// RUN: cp "%s" "%t/Src/test.cpp"
+// RUN: mkdir -p %t/Include
+// RUN: cp "%S/Inputs/fixed-header.h" "%t/Include/"
+// -I flag is relative to %t (where compile_flags is), not Src/.
+// RUN: echo '-IInclude/' >> %t/compile_flags.txt
+// RUN: echo "-Dklazz=class" >> %t/compile_flags.txt
+// RUN: echo '-std=c++11' >> %t/compile_flags.txt
+// RUN: clang-check "%t/Src/test.cpp" 2>&1
+// RUN: not clang-check "%s" 2>&1 | FileCheck "%s" -check-prefix=NODB
+
+// NODB: unknown type name 'klazz'
+klazz F{};
+
+// NODB: 'fixed-header.h' file not found
+#include "fixed-header.h"
+static_assert(SECRET_SYMBOL == 

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Sam McCall via Phabricator via cfe-commits]

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

Add fixme to upgrade out-param APIs.


https://reviews.llvm.org/D39799

Files:
  docs/JSONCompilationDatabase.rst
  include/clang/Tooling/CompilationDatabase.h
  lib/Tooling/CompilationDatabase.cpp
  test/Tooling/Inputs/fixed-header.h
  test/Tooling/fixed-database.cpp

Index: test/Tooling/fixed-database.cpp
===
--- /dev/null
+++ test/Tooling/fixed-database.cpp
@@ -0,0 +1,18 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/Src
+// RUN: cp "%s" "%t/Src/test.cpp"
+// RUN: mkdir -p %t/Include
+// RUN: cp "%S/Inputs/fixed-header.h" "%t/Include/"
+// -I flag is relative to %t (where compile_flags is), not Src/.
+// RUN: echo '-IInclude/' >> %t/compile_flags.txt
+// RUN: echo "-Dklazz=class" >> %t/compile_flags.txt
+// RUN: echo '-std=c++11' >> %t/compile_flags.txt
+// RUN: clang-check "%t/Src/test.cpp" 2>&1
+// RUN: not clang-check "%s" 2>&1 | FileCheck "%s" -check-prefix=NODB
+
+// NODB: unknown type name 'klazz'
+klazz F{};
+
+// NODB: 'fixed-header.h' file not found
+#include "fixed-header.h"
+static_assert(SECRET_SYMBOL == 1, "");
Index: test/Tooling/Inputs/fixed-header.h
===
--- /dev/null
+++ test/Tooling/Inputs/fixed-header.h
@@ -0,0 +1 @@
+#define SECRET_SYMBOL 1
Index: lib/Tooling/CompilationDatabase.cpp
===
--- lib/Tooling/CompilationDatabase.cpp
+++ lib/Tooling/CompilationDatabase.cpp
@@ -10,6 +10,9 @@
 //  This file contains implementations of the CompilationDatabase base class
 //  and the FixedCompilationDatabase.
 //
+//  FIXME: Various functions that take a string  should be upgraded
+//  to Expected.
+//
 //===--===//
 
 #include "clang/Tooling/CompilationDatabase.h"
@@ -26,6 +29,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/LineIterator.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -302,8 +306,22 @@
   std::vector StrippedArgs;
   if (!stripPositionalArgs(CommandLine, StrippedArgs, ErrorMsg))
 return nullptr;
-  return std::unique_ptr(
-  new FixedCompilationDatabase(Directory, StrippedArgs));
+  return llvm::make_unique(Directory, StrippedArgs);
+}
+
+std::unique_ptr
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string ) {
+  ErrorMsg.clear();
+  llvm::ErrorOr File =
+  llvm::MemoryBuffer::getFile(Path);
+  if (std::error_code Result = File.getError()) {
+ErrorMsg = "Error while opening fixed database: " + Result.message();
+return nullptr;
+  }
+  std::vector Args{llvm::line_iterator(**File),
+llvm::line_iterator()};
+  return llvm::make_unique(
+  llvm::sys::path::parent_path(Path), std::move(Args));
 }
 
 FixedCompilationDatabase::
@@ -334,6 +352,22 @@
   return std::vector();
 }
 
+namespace {
+
+class FixedCompilationDatabasePlugin : public CompilationDatabasePlugin {
+  std::unique_ptr
+  loadFromDirectory(StringRef Directory, std::string ) override {
+SmallString<1024> DatabasePath(Directory);
+llvm::sys::path::append(DatabasePath, "compile_flags.txt");
+return FixedCompilationDatabase::loadFromFile(DatabasePath, ErrorMessage);
+  }
+};
+
+static CompilationDatabasePluginRegistry::Add
+X("fixed-compilation-database", "Reads plain-text flags file");
+
+} // namespace
+
 namespace clang {
 namespace tooling {
 
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -182,6 +182,11 @@
   int , const char *const *Argv, std::string ,
   Twine Directory = ".");
 
+  /// Reads flags from the given file, one-per line.
+  /// Returns nullptr and sets ErrorMessage if we can't read the file.
+  static std::unique_ptr
+  loadFromFile(StringRef Path, std::string );
+
   /// \brief Constructs a compilation data base from a specified directory
   /// and command line.
   FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine);
Index: docs/JSONCompilationDatabase.rst
===
--- docs/JSONCompilationDatabase.rst
+++ docs/JSONCompilationDatabase.rst
@@ -91,3 +91,9 @@
 the top of the build directory. Clang tools are pointed to the top of
 the build directory to detect the file and use the compilation database
 to parse C++ code in the source tree.
+
+Alternatives
+
+For simple projects, Clang tools also recognize a compile_flags.txt file.
+This should contain one flag per line. The same flags will be used to compile
+any file.
___
cfe-commits mailing list

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Manuel Klimek via Phabricator via cfe-commits]

klimek accepted this revision.
klimek added a comment.

lg




Comment at: lib/Tooling/CompilationDatabase.cpp:312
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string ) {
+  ErrorMsg.clear();
+  llvm::ErrorOr File =

sammccall wrote:
> klimek wrote:
> > I assume we can't use ErrorOr as return value?
> Again this mirrors JSONCompilationDatabase. (And also 
> CompilationDatabasePlugin, which is harder to change)
> Change this one, change both, or leave as-is?
I think we want to add a FIXME to change the interface.


https://reviews.llvm.org/D39799



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D39799#920195, @sammccall wrote:

> It's doable, but then you're fighting the infrastructure here. The same 
> ambiguity can exist between any pair of CompilationDatabasePlugins (try 
> dumping a compile_commands.json in a google source tree). If we want to make 
> this an error or have well-defined priority, that should be handled by 
> autoDetectFromDirectory I think.
>
> (Happy to do that if you think it's important - personally I'm not convinced 
> it will be an issue in practice)


Yeah, I'm not sure if that's an issue either. Let's see whether anyone will be 
using both `compile_flags.txt` and `compile_commands.json` in the first place. 
We can always do it later.
Oh, and LGTM from my side!


https://reviews.llvm.org/D39799



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D39799#919508, @ilya-biryukov wrote:

> In https://reviews.llvm.org/D39799#919494, @sammccall wrote:
>
> > e.g. IIUC, things like `#include "sibling.h"` won't look for the h file 
> > next to the cc file?
>
>
> Actually, this should work as quoted includes are always searched relative to 
> the current file before looking in the include paths. No matter what the WD 
> is.


Right, thanks :-)

>> Ideally yes... i'm not sure this is so easy with the registry mechanism 
>> though :-(
>>  They'd have to be present at the same level, so I'm not sure this is a huge 
>> deal.
> 
> Yeah, looks like this would require unifying `FixedCompilationDatabase` and 
> `JSONCompilationDatabase`. This doesn't seem too crazy, though. "A class that 
> handles reading compilation arguments from the filesystem." seems like a 
> valid entity to me.

It's doable, but then you're fighting the infrastructure here. The same 
ambiguity can exist between any pair of CompilationDatabasePlugins (try dumping 
a compile_commands.json in a google source tree). If we want to make this an 
error or have well-defined priority, that should be handled by 
autoDetectFromDirectory I think.

(Happy to do that if you think it's important - personally I'm not convinced it 
will be an issue in practice)


https://reviews.llvm.org/D39799



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Sam McCall via Phabricator via cfe-commits]

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

Add test verifying that working directory is compile_flags's parent.


https://reviews.llvm.org/D39799

Files:
  docs/JSONCompilationDatabase.rst
  include/clang/Tooling/CompilationDatabase.h
  lib/Tooling/CompilationDatabase.cpp
  test/Tooling/Inputs/fixed-header.h
  test/Tooling/fixed-database.cpp

Index: test/Tooling/fixed-database.cpp
===
--- /dev/null
+++ test/Tooling/fixed-database.cpp
@@ -0,0 +1,18 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/Src
+// RUN: cp "%s" "%t/Src/test.cpp"
+// RUN: mkdir -p %t/Include
+// RUN: cp "%S/Inputs/fixed-header.h" "%t/Include/"
+// -I flag is relative to %t (where compile_flags is), not Src/.
+// RUN: echo '-IInclude/' >> %t/compile_flags.txt
+// RUN: echo "-Dklazz=class" >> %t/compile_flags.txt
+// RUN: echo '-std=c++11' >> %t/compile_flags.txt
+// RUN: clang-check "%t/Src/test.cpp" 2>&1
+// RUN: not clang-check "%s" 2>&1 | FileCheck "%s" -check-prefix=NODB
+
+// NODB: unknown type name 'klazz'
+klazz F{};
+
+// NODB: 'fixed-header.h' file not found
+#include "fixed-header.h"
+static_assert(SECRET_SYMBOL == 1, "");
Index: test/Tooling/Inputs/fixed-header.h
===
--- /dev/null
+++ test/Tooling/Inputs/fixed-header.h
@@ -0,0 +1 @@
+#define SECRET_SYMBOL 1
Index: lib/Tooling/CompilationDatabase.cpp
===
--- lib/Tooling/CompilationDatabase.cpp
+++ lib/Tooling/CompilationDatabase.cpp
@@ -26,6 +26,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/LineIterator.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -302,8 +303,22 @@
   std::vector StrippedArgs;
   if (!stripPositionalArgs(CommandLine, StrippedArgs, ErrorMsg))
 return nullptr;
-  return std::unique_ptr(
-  new FixedCompilationDatabase(Directory, StrippedArgs));
+  return llvm::make_unique(Directory, StrippedArgs);
+}
+
+std::unique_ptr
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string ) {
+  ErrorMsg.clear();
+  llvm::ErrorOr File =
+  llvm::MemoryBuffer::getFile(Path);
+  if (std::error_code Result = File.getError()) {
+ErrorMsg = "Error while opening fixed database: " + Result.message();
+return nullptr;
+  }
+  std::vector Args{llvm::line_iterator(**File),
+llvm::line_iterator()};
+  return llvm::make_unique(
+  llvm::sys::path::parent_path(Path), std::move(Args));
 }
 
 FixedCompilationDatabase::
@@ -334,6 +349,22 @@
   return std::vector();
 }
 
+namespace {
+
+class FixedCompilationDatabasePlugin : public CompilationDatabasePlugin {
+  std::unique_ptr
+  loadFromDirectory(StringRef Directory, std::string ) override {
+SmallString<1024> DatabasePath(Directory);
+llvm::sys::path::append(DatabasePath, "compile_flags.txt");
+return FixedCompilationDatabase::loadFromFile(DatabasePath, ErrorMessage);
+  }
+};
+
+static CompilationDatabasePluginRegistry::Add
+X("fixed-compilation-database", "Reads plain-text flags file");
+
+} // namespace
+
 namespace clang {
 namespace tooling {
 
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -182,6 +182,11 @@
   int , const char *const *Argv, std::string ,
   Twine Directory = ".");
 
+  /// Reads flags from the given file, one-per line.
+  /// Returns nullptr and sets ErrorMessage if we can't read the file.
+  static std::unique_ptr
+  loadFromFile(StringRef Path, std::string );
+
   /// \brief Constructs a compilation data base from a specified directory
   /// and command line.
   FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine);
Index: docs/JSONCompilationDatabase.rst
===
--- docs/JSONCompilationDatabase.rst
+++ docs/JSONCompilationDatabase.rst
@@ -91,3 +91,9 @@
 the top of the build directory. Clang tools are pointed to the top of
 the build directory to detect the file and use the compilation database
 to parse C++ code in the source tree.
+
+Alternatives
+
+For simple projects, Clang tools also recognize a compile_flags.txt file.
+This should contain one flag per line. The same flags will be used to compile
+any file.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D39799#919494, @sammccall wrote:

> e.g. IIUC, things like `#include "sibling.h"` won't look for the h file next 
> to the cc file?


Actually, this should work as quoted includes are always searched relative to 
the current file before looking in the include paths. No matter what the WD is.

> `Result` is initialized with a copy of `CompilerCommands`, whose element is 
> initialized with the directory passed to the constructor.

Missed that, thanks!

> Ideally yes... i'm not sure this is so easy with the registry mechanism 
> though :-(
>  They'd have to be present at the same level, so I'm not sure this is a huge 
> deal.

Yeah, looks like this would require unifying `FixedCompilationDatabase` and 
`JSONCompilationDatabase`. This doesn't seem too crazy, though. "A class that 
handles reading compilation arguments from the filesystem." seems like a valid 
entity to me.


https://reviews.llvm.org/D39799



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D39799#919468, @ilya-biryukov wrote:

> In https://reviews.llvm.org/D39799#919415, @sammccall wrote:
>
> > Currently the working directory is always the parent of `compile_flags.txt` 
> > as you suggest.
> >  Maybe this isn't great though - sometimes it might be important that the 
> > WD is the parent of the source file?
>
>
> Parent of the source file seems rather inconvenient most of the time. Even 
> when reading the contents of `compile_flags.txt` one would have to consider 
> it the contexts of every source file.


Understood, this is the best behavior I think. I'm just wondering whether it's 
a good enough heuristic to be silently applied.
e.g. IIUC, things like `#include "sibling.h"` won't look for the h file next to 
the cc file?

> I'm looking at CompilationDatabase.cpp:321 
> 
>  and don't see the `CompileCommand::Directory` being set at all. Maybe I'm 
> missing something.

`Result` is initialized with a copy of `CompilerCommands`, whose element is 
initialized with the directory passed to the constructor.

> A few ideas for tests:
> 
> 1. Test relative include paths (`-Irelpath/to/include`) work.

Definitely will do this.

> 2. What happens if both `compile_commands.json` and `compile_flags.txt` are 
> present? Should we test and document it? I'd expect the first one to take 
> priority and the second one to come into play only if the input file is not 
> found in the first one.

Ideally yes... i'm not sure this is so easy with the registry mechanism though 
:-(
They'd have to be present at the same level, so I'm not sure this is a huge 
deal.


https://reviews.llvm.org/D39799



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D39799#919415, @sammccall wrote:

> Currently the working directory is always the parent of `compile_flags.txt` 
> as you suggest.
>  Maybe this isn't great though - sometimes it might be important that the WD 
> is the parent of the source file?


Parent of the source file seems rather inconvenient most of the time. Even when 
reading the contents of `compile_flags.txt` one would have to consider it the 
contexts of every source file.

I'm looking at CompilationDatabase.cpp:321 

 and don't see the `CompileCommand::Directory` being set at all. Maybe I'm 
missing something.

A few ideas for tests:

1. Test relative include paths (`-Irelpath/to/include`) work.
2. What happens if both `compile_commands.json` and `compile_flags.txt` are 
present? Should we test and document it? I'd expect the first one to take 
priority and the second one to come into play only if the input file is not 
found in the first one.


https://reviews.llvm.org/D39799



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Sam McCall via Phabricator via cfe-commits]

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

llvm::make_unique


https://reviews.llvm.org/D39799

Files:
  docs/JSONCompilationDatabase.rst
  include/clang/Tooling/CompilationDatabase.h
  lib/Tooling/CompilationDatabase.cpp
  test/Tooling/fixed-database.cpp

Index: test/Tooling/fixed-database.cpp
===
--- /dev/null
+++ test/Tooling/fixed-database.cpp
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "-Dklazz=class" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-check "%t/test.cpp" 2>&1
+// RUN: not clang-check "%s" 2>&1 | FileCheck "%s" -check-prefix=NODB
+// NODB: unknown type name 'klazz'
+klazz F{};
Index: lib/Tooling/CompilationDatabase.cpp
===
--- lib/Tooling/CompilationDatabase.cpp
+++ lib/Tooling/CompilationDatabase.cpp
@@ -26,6 +26,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/LineIterator.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -302,8 +303,23 @@
   std::vector StrippedArgs;
   if (!stripPositionalArgs(CommandLine, StrippedArgs, ErrorMsg))
 return nullptr;
-  return std::unique_ptr(
-  new FixedCompilationDatabase(Directory, StrippedArgs));
+  return llvm::make_unique(Directory, StrippedArgs);
+}
+
+std::unique_ptr
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string ) {
+  ErrorMsg.clear();
+  llvm::ErrorOr File =
+  llvm::MemoryBuffer::getFile(Path);
+  if (std::error_code Result = File.getError()) {
+ErrorMsg = "Error while opening fixed database: " + Result.message();
+return nullptr;
+  }
+  std::vector Args{
+  llvm::line_iterator(**File, /*SkipBlanks=*/true, /*CommentMarker=*/'#'),
+  llvm::line_iterator()};
+  return llvm::make_unique(
+  llvm::sys::path::parent_path(Path), std::move(Args));
 }
 
 FixedCompilationDatabase::
@@ -334,6 +350,24 @@
   return std::vector();
 }
 
+namespace {
+
+class FixedCompilationDatabasePlugin : public CompilationDatabasePlugin {
+  std::unique_ptr
+  loadFromDirectory(StringRef Directory, std::string ) override {
+SmallString<1024> DatabasePath(Directory);
+llvm::sys::path::append(DatabasePath, "compile_flags.txt");
+return FixedCompilationDatabase::loadFromFile(DatabasePath, ErrorMessage);
+  }
+};
+
+// Register the JSONCompilationDatabasePlugin with the
+// CompilationDatabasePluginRegistry using this statically initialized variable.
+static CompilationDatabasePluginRegistry::Add
+X("fixed-compilation-database", "Reads plain-text flags file");
+
+} // namespace
+
 namespace clang {
 namespace tooling {
 
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -182,6 +182,11 @@
   int , const char *const *Argv, std::string ,
   Twine Directory = ".");
 
+  /// Reads flags from the given file, one-per line.
+  /// Returns nullptr and sets ErrorMessage if we can't read the file.
+  static std::unique_ptr
+  loadFromFile(StringRef Path, std::string );
+
   /// \brief Constructs a compilation data base from a specified directory
   /// and command line.
   FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine);
Index: docs/JSONCompilationDatabase.rst
===
--- docs/JSONCompilationDatabase.rst
+++ docs/JSONCompilationDatabase.rst
@@ -91,3 +91,9 @@
 the top of the build directory. Clang tools are pointed to the top of
 the build directory to detect the file and use the compilation database
 to parse C++ code in the source tree.
+
+Alternatives
+
+For simple projects, Clang tools also recognize a compile_flags.txt file.
+This should contain one flag per line. The same flags will be used to compile
+any file.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked an inline comment as done.
sammccall added a comment.

In https://reviews.llvm.org/D39799#919388, @ilya-biryukov wrote:

> I can see that `FixedCompilationDatabase` does not set a working directory. 
> Is this something we may want to have for `compile_flags.txt` or one would 
> need to resort to `compile_commands.json` to get this?
>  E.g., I'd find it useful to add includes with paths relative to 
> `compile_flags.txt` by putting `-Iproject-dep/include` there.


Currently the working directory is always the parent of `compile_flags.txt` as 
you suggest.
Maybe this isn't great though - sometimes it might be important that the WD is 
the parent of the source file?


https://reviews.llvm.org/D39799



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I can see that `FixedCompilationDatabase` does not set a working directory. Is 
this something we may want to have for `compile_flags.txt` or one would need to 
resort to `compile_commands.json` to get this?
E.g., I'd find it useful to add includes with paths relative to 
`compile_flags.txt` by putting `-Iproject-dep/include` there.




Comment at: lib/Tooling/CompilationDatabase.cpp:322
+  llvm::line_iterator()};
+  return std::unique_ptr(new 
FixedCompilationDatabase(
+  llvm::sys::path::parent_path(Path), std::move(Args)));

Maybe use `llvm:make_unique`?


https://reviews.llvm.org/D39799



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: include/clang/Tooling/CompilationDatabase.h:188
+  static std::unique_ptr
+  loadFromFile(StringRef Path, std::string );
+

klimek wrote:
> Perhaps loadFromTextFile?
This mirrors JSONCompilationDatabase for consistency.
Happy to change this one, change both, or leave as-is. WDYT?



Comment at: lib/Tooling/CompilationDatabase.cpp:312
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string ) {
+  ErrorMsg.clear();
+  llvm::ErrorOr File =

klimek wrote:
> I assume we can't use ErrorOr as return value?
Again this mirrors JSONCompilationDatabase. (And also 
CompilationDatabasePlugin, which is harder to change)
Change this one, change both, or leave as-is?


https://reviews.llvm.org/D39799



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: include/clang/Tooling/CompilationDatabase.h:188
+  static std::unique_ptr
+  loadFromFile(StringRef Path, std::string );
+

Perhaps loadFromTextFile?



Comment at: lib/Tooling/CompilationDatabase.cpp:312
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string ) {
+  ErrorMsg.clear();
+  llvm::ErrorOr File =

I assume we can't use ErrorOr as return value?


https://reviews.llvm.org/D39799



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

[PATCH] D39799: [Tooling] Use FixedCompilationDatabase when `compile_flags.txt` is found.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
Herald added a subscriber: ilya-biryukov.

This is an alternative to JSONCompilationDatabase for simple projects that
don't use a build system such as CMake.
(You can also drop one in ~, to make your tools use e.g. C++11 by default)

There's no facility for varying flags per-source-file or per-machine.
Possibly this could be accommodated backwards-compatibly using cpp, but even if
not the simplicity seems worthwhile for the cases that are addressed.

Tested with clangd, works great! (requires clangd restart)


https://reviews.llvm.org/D39799

Files:
  docs/JSONCompilationDatabase.rst
  include/clang/Tooling/CompilationDatabase.h
  lib/Tooling/CompilationDatabase.cpp
  test/Tooling/fixed-database.cpp

Index: test/Tooling/fixed-database.cpp
===
--- /dev/null
+++ test/Tooling/fixed-database.cpp
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "-Dklazz=class" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-check "%t/test.cpp" 2>&1
+// RUN: not clang-check "%s" 2>&1 | FileCheck "%s" -check-prefix=NODB
+// NODB: unknown type name 'klazz'
+klazz F{};
Index: lib/Tooling/CompilationDatabase.cpp
===
--- lib/Tooling/CompilationDatabase.cpp
+++ lib/Tooling/CompilationDatabase.cpp
@@ -26,6 +26,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/LineIterator.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -306,6 +307,22 @@
   new FixedCompilationDatabase(Directory, StrippedArgs));
 }
 
+std::unique_ptr
+FixedCompilationDatabase::loadFromFile(StringRef Path, std::string ) {
+  ErrorMsg.clear();
+  llvm::ErrorOr File =
+  llvm::MemoryBuffer::getFile(Path);
+  if (std::error_code Result = File.getError()) {
+ErrorMsg = "Error while opening fixed database: " + Result.message();
+return nullptr;
+  }
+  std::vector Args{
+  llvm::line_iterator(**File, /*SkipBlanks=*/true, /*CommentMarker=*/'#'),
+  llvm::line_iterator()};
+  return std::unique_ptr(new FixedCompilationDatabase(
+  llvm::sys::path::parent_path(Path), std::move(Args)));
+}
+
 FixedCompilationDatabase::
 FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine) {
   std::vector ToolCommandLine(1, "clang-tool");
@@ -334,6 +351,24 @@
   return std::vector();
 }
 
+namespace {
+
+class FixedCompilationDatabasePlugin : public CompilationDatabasePlugin {
+  std::unique_ptr
+  loadFromDirectory(StringRef Directory, std::string ) override {
+SmallString<1024> DatabasePath(Directory);
+llvm::sys::path::append(DatabasePath, "compile_flags.txt");
+return FixedCompilationDatabase::loadFromFile(DatabasePath, ErrorMessage);
+  }
+};
+
+// Register the JSONCompilationDatabasePlugin with the
+// CompilationDatabasePluginRegistry using this statically initialized variable.
+static CompilationDatabasePluginRegistry::Add
+X("fixed-compilation-database", "Reads plain-text flags file");
+
+} // namespace
+
 namespace clang {
 namespace tooling {
 
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -182,6 +182,11 @@
   int , const char *const *Argv, std::string ,
   Twine Directory = ".");
 
+  /// Reads flags from the given file, one-per line.
+  /// Returns nullptr and sets ErrorMessage if we can't read the file.
+  static std::unique_ptr
+  loadFromFile(StringRef Path, std::string );
+
   /// \brief Constructs a compilation data base from a specified directory
   /// and command line.
   FixedCompilationDatabase(Twine Directory, ArrayRef CommandLine);
Index: docs/JSONCompilationDatabase.rst
===
--- docs/JSONCompilationDatabase.rst
+++ docs/JSONCompilationDatabase.rst
@@ -91,3 +91,9 @@
 the top of the build directory. Clang tools are pointed to the top of
 the build directory to detect the file and use the compilation database
 to parse C++ code in the source tree.
+
+Alternatives
+
+For simple projects, Clang tools also recognize a compile_flags.txt file.
+This should contain one flag per line. The same flags will be used to compile
+any file.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits