[PATCH] D40409: [Tooling] Acknowledge that many CompilationDatabases don't support enumeration.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL318943: [Tooling] Acknowledge that many CompilationDatabases
don't support enumeration. (authored by sammccall).
Repository:
rL LLVM
https://reviews.llvm.org/D40409
Files:
cfe/trunk/include/clang/Tooling/CompilationDatabase.h
cfe/trunk/lib/Tooling/CompilationDatabase.cpp
Index: cfe/trunk/include/clang/Tooling/CompilationDatabase.h
===
--- cfe/trunk/include/clang/Tooling/CompilationDatabase.h
+++ cfe/trunk/include/clang/Tooling/CompilationDatabase.h
@@ -64,10 +64,12 @@
/// \brief Interface for compilation databases.
///
-/// A compilation database allows the user to retrieve all compile command lines
-/// that a specified file is compiled with in a project.
-/// The retrieved compile command lines can be used to run clang tools over
-/// a subset of the files in a project.
+/// A compilation database allows the user to retrieve compile command lines
+/// for the files in a project.
+///
+/// Many implementations are enumerable, allowing all command lines to be
+/// retrieved. These can be used to run clang tools over a subset of the files
+/// in a project.
class CompilationDatabase {
public:
virtual ~CompilationDatabase();
@@ -114,15 +116,21 @@
StringRef FilePath) const = 0;
/// \brief Returns the list of all files available in the compilation database.
- virtual std::vector getAllFiles() const = 0;
+ ///
+ /// By default, returns nothing. Implementations should override this if they
+ /// can enumerate their source files.
+ virtual std::vector getAllFiles() const { return {}; }
/// \brief Returns all compile commands for all the files in the compilation
/// database.
///
/// FIXME: Add a layer in Tooling that provides an interface to run a tool
/// over all files in a compilation database. Not all build systems have the
/// ability to provide a feasible implementation for \c getAllCompileCommands.
- virtual std::vector getAllCompileCommands() const = 0;
+ ///
+ /// By default, this is implemented in terms of getAllFiles() and
+ /// getCompileCommands(). Subclasses may override this for efficiency.
+ virtual std::vector getAllCompileCommands() const;
};
/// \brief Interface for compilation database plugins.
@@ -149,6 +157,7 @@
/// \brief A compilation database that returns a single compile command line.
///
/// Useful when we want a tool to behave more like a compiler invocation.
+/// This compilation database is not enumerable: getAllFiles() returns {}.
class FixedCompilationDatabase : public CompilationDatabase {
public:
/// \brief Creates a FixedCompilationDatabase from the arguments after "--".
@@ -199,17 +208,6 @@
std::vector
getCompileCommands(StringRef FilePath) const override;
- /// \brief Returns the list of all files available in the compilation database.
- ///
- /// Note: This is always an empty list for the fixed compilation database.
- std::vector getAllFiles() const override;
-
- /// \brief Returns all compile commands for all the files in the compilation
- /// database.
- ///
- /// Note: This is always an empty list for the fixed compilation database.
- std::vector getAllCompileCommands() const override;
-
private:
/// This is built up to contain a single entry vector to be returned from
/// getCompileCommands after adding the positional argument.
Index: cfe/trunk/lib/Tooling/CompilationDatabase.cpp
===
--- cfe/trunk/lib/Tooling/CompilationDatabase.cpp
+++ cfe/trunk/lib/Tooling/CompilationDatabase.cpp
@@ -112,6 +112,15 @@
return DB;
}
+std::vector CompilationDatabase::getAllCompileCommands() const {
+ std::vector Result;
+ for (const auto &File : getAllFiles()) {
+auto C = getCompileCommands(File);
+std::move(C.begin(), C.end(), std::back_inserter(Result));
+ }
+ return Result;
+}
+
CompilationDatabasePlugin::~CompilationDatabasePlugin() {}
namespace {
@@ -342,16 +351,6 @@
return Result;
}
-std::vector
-FixedCompilationDatabase::getAllFiles() const {
- return std::vector();
-}
-
-std::vector
-FixedCompilationDatabase::getAllCompileCommands() const {
- return std::vector();
-}
-
namespace {
class FixedCompilationDatabasePlugin : public CompilationDatabasePlugin {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40409: [Tooling] Acknowledge that many CompilationDatabases don't support enumeration.
sammccall added inline comments.
Comment at: include/clang/Tooling/CompilationDatabase.h:122
+ /// can enumerate their source files.
+ virtual std::vector getAllFiles() const { return {}; }
grandinj wrote:
> I know very little about LLVM's standards, so ignore me if I'm wrong, but
> shouldn't this be returning a pair of (begin,end) iterators rather than
> potentially a copy of a very large array of strings?
>
> And shouldn't it be returning an iteration over StringRef rather then
> std::string, which will require copying the actual data?
You might well be right. But this is an existing interface designed as an
extension point for out-of-tree build systems. I'd rather not break them unless
we have evidence of an actual performance problem.
(Note my change here is backwards compatible and doesn't change the signature)
https://reviews.llvm.org/D40409
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40409: [Tooling] Acknowledge that many CompilationDatabases don't support enumeration.
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D40409 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40409: [Tooling] Acknowledge that many CompilationDatabases don't support enumeration.
grandinj added inline comments.
Comment at: include/clang/Tooling/CompilationDatabase.h:122
+ /// can enumerate their source files.
+ virtual std::vector getAllFiles() const { return {}; }
I know very little about LLVM's standards, so ignore me if I'm wrong, but
shouldn't this be returning a pair of (begin,end) iterators rather than
potentially a copy of a very large array of strings?
And shouldn't it be returning an iteration over StringRef rather then
std::string, which will require copying the actual data?
Comment at: include/clang/Tooling/CompilationDatabase.h:133
+ /// getCompileCommands(). Subclasses may override this for efficiency.
+ virtual std::vector getAllCompileCommands() const;
};
similarly here
https://reviews.llvm.org/D40409
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40409: [Tooling] Acknowledge that many CompilationDatabases don't support enumeration.
sammccall created this revision.
Herald added a subscriber: klimek.
Provide default implementations so that only getCompileCommands() is mandatory.
https://reviews.llvm.org/D40409
Files:
include/clang/Tooling/CompilationDatabase.h
lib/Tooling/CompilationDatabase.cpp
Index: lib/Tooling/CompilationDatabase.cpp
===
--- lib/Tooling/CompilationDatabase.cpp
+++ lib/Tooling/CompilationDatabase.cpp
@@ -112,6 +112,15 @@
return DB;
}
+std::vector CompilationDatabase::getAllCompileCommands() const {
+ std::vector Result;
+ for (const auto &File : getAllFiles()) {
+auto C = getCompileCommands(File);
+std::move(C.begin(), C.end(), std::back_inserter(Result));
+ }
+ return Result;
+}
+
CompilationDatabasePlugin::~CompilationDatabasePlugin() {}
namespace {
@@ -342,16 +351,6 @@
return Result;
}
-std::vector
-FixedCompilationDatabase::getAllFiles() const {
- return std::vector();
-}
-
-std::vector
-FixedCompilationDatabase::getAllCompileCommands() const {
- return std::vector();
-}
-
namespace {
class FixedCompilationDatabasePlugin : public CompilationDatabasePlugin {
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -64,10 +64,12 @@
/// \brief Interface for compilation databases.
///
-/// A compilation database allows the user to retrieve all compile command lines
-/// that a specified file is compiled with in a project.
-/// The retrieved compile command lines can be used to run clang tools over
-/// a subset of the files in a project.
+/// A compilation database allows the user to retrieve compile command lines
+/// for the files in a project.
+///
+/// Many implementations are enumerable, allowing all command lines to be
+/// retrieved. These can be used to run clang tools over a subset of the files
+/// in a project.
class CompilationDatabase {
public:
virtual ~CompilationDatabase();
@@ -114,15 +116,21 @@
StringRef FilePath) const = 0;
/// \brief Returns the list of all files available in the compilation database.
- virtual std::vector getAllFiles() const = 0;
+ ///
+ /// By default, returns nothing. Implementations should override this if they
+ /// can enumerate their source files.
+ virtual std::vector getAllFiles() const { return {}; }
/// \brief Returns all compile commands for all the files in the compilation
/// database.
///
/// FIXME: Add a layer in Tooling that provides an interface to run a tool
/// over all files in a compilation database. Not all build systems have the
/// ability to provide a feasible implementation for \c getAllCompileCommands.
- virtual std::vector getAllCompileCommands() const = 0;
+ ///
+ /// By default, this is implemented in terms of getAllFiles() and
+ /// getCompileCommands(). Subclasses may override this for efficiency.
+ virtual std::vector getAllCompileCommands() const;
};
/// \brief Interface for compilation database plugins.
@@ -149,6 +157,7 @@
/// \brief A compilation database that returns a single compile command line.
///
/// Useful when we want a tool to behave more like a compiler invocation.
+/// This compilation database is not enumerable: getAllFiles() returns {}.
class FixedCompilationDatabase : public CompilationDatabase {
public:
/// \brief Creates a FixedCompilationDatabase from the arguments after "--".
@@ -199,17 +208,6 @@
std::vector
getCompileCommands(StringRef FilePath) const override;
- /// \brief Returns the list of all files available in the compilation database.
- ///
- /// Note: This is always an empty list for the fixed compilation database.
- std::vector getAllFiles() const override;
-
- /// \brief Returns all compile commands for all the files in the compilation
- /// database.
- ///
- /// Note: This is always an empty list for the fixed compilation database.
- std::vector getAllCompileCommands() const override;
-
private:
/// This is built up to contain a single entry vector to be returned from
/// getCompileCommands after adding the positional argument.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
