On 07/10/2012 03:15 PM, Manuel Klimek wrote: > On Tue, Jul 10, 2012 at 3:10 PM, Arnaud A. de Grandmaison > <[email protected]> wrote: >> Is it necessary for 'CompilationDatabase::autoDetectFromSource' to take >> a sourcefile to seed the search, instead of using a directory ? >> >> Using a directory would make this function more generic. > I really want this to "just work" for the 99% case.
That's also what I want :) But I did not express well what I meant then. What I proposed in my earlier mail, using the original implementation of autoDetectFromSource, was to have it check : - if the argument is a directory : do not modify it with get_parent. We do not want to skip the deepest directory level, - otherwise, do a get_parent which acts essentially as 'dirname' (the current behaviour) In the attached patch, I add support for the autodetect feature starting from directories. With this implementation, it is made clear whether the starting point for the search is a file or a directory. > >> For example, in a plugin like clang_complete for vim, to find a >> compilation database, you would do something like : >> 1 - autoDetectFromSource( basename(sourceFile) ) >> 2 - if (no database for sourceFile or sourceFile not found in database) >> then autoDetectFromSource( getCwd() ) > We can add this behavior to autoDetectFromSource if you think it's important. > > I exactly *don't* want every tool to have to write their own magic > lookup-oh-where's-the-database implementation. I think that'd be hard > to support. What I meant is that we should provide the low level functionality that tools can use to build their own magic upon. The example I gave is supposed to take place in the vim plugin, not in clang :). With the current implementation of autoDetectFromSource, either the tool author has to rewrite its own version for looking into dirs (we do not want that), or do something dirty like append a dummy file to the directory (we do not want to do that either). I think we should factorize what can easily be factorized, and not have each tool re-implement the low-level logic. High level logic is clearly theirs. > Also, this version is the "80% case". We'll add the other 20% incrementally. > For example, chandler wants to allow .clangrc files (which we'll > probably want when we have clangd anyway) to specify source to build > mappings and other stuff. I think we have lots of ways we can improve > this, but in general, I want tools to write autoDetectFromSource and > be done with it. With those 3 functions loadFromDirectory / autoDetectFromSource / autoDetectFromDir we cover most --- if not all --- low-level use cases, and higher level use can be built above. > Cheers, > /Manuel > > Cheers, -- Arnaud de Grandmaison
>From 56d0708bc28d821f1c5d8cbd572c3cd16613e9cb Mon Sep 17 00:00:00 2001 From: "Arnaud A. de Grandmaison" <[email protected]> Date: Tue, 10 Jul 2012 16:50:58 +0200 Subject: [PATCH] Adds support for auto-detection of compilation databases, looking in a directory and all its parents. --- include/clang/Tooling/CompilationDatabase.h | 11 ++++++- lib/Tooling/CompilationDatabase.cpp | 42 +++++++++++++++++++++------ test/Tooling/clang-check-autodetect-dir.cpp | 11 +++++++ tools/clang-check/ClangCheck.cpp | 4 ++ 4 files changed, 57 insertions(+), 11 deletions(-) create mode 100644 test/Tooling/clang-check-autodetect-dir.cpp diff --git a/include/clang/Tooling/CompilationDatabase.h b/include/clang/Tooling/CompilationDatabase.h index 0dec6f8..a92e5d5 100644 --- a/include/clang/Tooling/CompilationDatabase.h +++ b/include/clang/Tooling/CompilationDatabase.h @@ -83,11 +83,18 @@ public: /// \brief Tries to detect a compilation database location and load it. /// - /// Looks for a compilation database in all parent paths by calling - /// loadFromDirectory. + /// Looks for a compilation database in all parent paths of file 'SourceFile' + /// by calling loadFromDirectory. static CompilationDatabase *autoDetectFromSource(StringRef SourceFile, std::string &ErrorMessage); + /// \brief Tries to detect a compilation database location and load it. + /// + /// Looks for a compilation database in directory 'SourceDir' and all + /// its parent paths by calling loadFromDirectory. + static CompilationDatabase *autoDetectFromDir(StringRef SourceDir, + std::string &ErrorMessage); + /// \brief Returns all compile commands in which the specified file was /// compiled. /// diff --git a/lib/Tooling/CompilationDatabase.cpp b/lib/Tooling/CompilationDatabase.cpp index 8e91112..103b16f 100644 --- a/lib/Tooling/CompilationDatabase.cpp +++ b/lib/Tooling/CompilationDatabase.cpp @@ -122,23 +122,47 @@ CompilationDatabase::loadFromDirectory(StringRef BuildDirectory, return Database.take(); } -CompilationDatabase * -CompilationDatabase::autoDetectFromSource(StringRef SourceFile, - std::string &ErrorMessage) { - llvm::SmallString<1024> AbsolutePath(getAbsolutePath(SourceFile)); - StringRef Directory = llvm::sys::path::parent_path(AbsolutePath); +static CompilationDatabase * +findCompilationDatabaseFromDirectory(StringRef Directory) { + CompilationDatabase *DB; while (!Directory.empty()) { std::string LoadErrorMessage; - if (CompilationDatabase *DB = loadFromDirectory(Directory, - LoadErrorMessage)) + DB = CompilationDatabase::loadFromDirectory(Directory, LoadErrorMessage); + + if (DB) return DB; Directory = llvm::sys::path::parent_path(Directory); } - ErrorMessage = ("Could not auto-detect compilation database for file \"" + - SourceFile + "\"").str(); return NULL; } +CompilationDatabase * +CompilationDatabase::autoDetectFromSource(StringRef SourceFile, + std::string &ErrorMessage) { + llvm::SmallString<1024> AbsolutePath(getAbsolutePath(SourceFile)); + StringRef Directory = llvm::sys::path::parent_path(AbsolutePath); + + CompilationDatabase *DB = findCompilationDatabaseFromDirectory(Directory); + + if (!DB) + ErrorMessage = ("Could not auto-detect compilation database for file \"" + + SourceFile + "\"").str(); + return DB; +} + +CompilationDatabase * +CompilationDatabase::autoDetectFromDir(StringRef SourceDir, + std::string &ErrorMessage) { + llvm::SmallString<1024> AbsolutePath(getAbsolutePath(SourceDir)); + + CompilationDatabase *DB = findCompilationDatabaseFromDirectory(AbsolutePath); + + if (!DB) + ErrorMessage = ("Could not auto-detect compilation database from directory \"" + + SourceDir + "\"").str(); + return DB; +} + FixedCompilationDatabase * FixedCompilationDatabase::loadFromCommandLine(int &Argc, const char **Argv, diff --git a/test/Tooling/clang-check-autodetect-dir.cpp b/test/Tooling/clang-check-autodetect-dir.cpp new file mode 100644 index 0000000..2c39504 --- /dev/null +++ b/test/Tooling/clang-check-autodetect-dir.cpp @@ -0,0 +1,11 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t/abc/def +// RUN: echo "[{\"directory\":\".\",\"command\":\"clang++ -c %t/test.cpp\",\"file\":\"%t/test.cpp\"}]" | sed -e 's/\\/\\\\/g' > %t/compile_commands.json +// RUN: cp "%s" "%t/test.cpp" +// RUN: clang-check -p "%t/abc/def" "%t/test.cpp" 2>&1|FileCheck %s +// FIXME: Make the above easier. + +// CHECK: C++ requires +invalid; + +// REQUIRES: shell diff --git a/tools/clang-check/ClangCheck.cpp b/tools/clang-check/ClangCheck.cpp index ef4a3ac..4a4d223 100644 --- a/tools/clang-check/ClangCheck.cpp +++ b/tools/clang-check/ClangCheck.cpp @@ -60,6 +60,10 @@ int main(int argc, const char **argv) { if (!BuildPath.empty()) { Compilations.reset(CompilationDatabase::loadFromDirectory(BuildPath, ErrorMessage)); + if (!Compilations) + Compilations.reset(CompilationDatabase::autoDetectFromDir(BuildPath, + ErrorMessage)); + } else { Compilations.reset(CompilationDatabase::autoDetectFromSource( SourcePaths[0], ErrorMessage)); -- 1.7.8.6
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
