[PATCH] Transform all files in a compilation database if no source files are provided.

Guillaume Papin guillaume.papin at epitech.eu
Tue Sep 10 10:37:39 PDT 2013

  Looking at the documentation of `llvm::report_fatal_error()` it looks like to me we shouldn't use it here for command line argument errors.
  We should just print the messages with `llvm::errs()` and return 1.

  I think we are almost there, you should probably add the user documentation in your next patch.

Comment at: clang-modernize/tool/ClangModernize.cpp:295-301
@@ -294,2 +294,9 @@
+// Predicate definition for determining whether a file is not included.
+struct isFileNotIncludedPredicate {
+  bool operator()(llvm::StringRef FilePath) {
+    return !GlobalOptions.ModifiableFiles.isFileIncluded(FilePath);
+  }
 int main(int argc, const char **argv) {
A static function is enough here.

  static bool isFileNotIncludedPredicate(llvm::StringRef FilePath) {
    return !GlobalOptions.ModifiableFiles.isFileIncluded(FilePath);

Comment at: clang-modernize/tool/ClangModernize.cpp:318-340
@@ -306,19 +317,25 @@
+  // If no fixed compilation database is provided then we try to autodetect a
+  // compilation database from a directory (-p) or from the first source.
   if (!Compilations) {
     std::string ErrorMessage;
     if (BuildPath.getNumOccurrences() > 0) {
+      // Detect compilation database from a Directory.
           BuildPath, ErrorMessage));
-    } else {
+      if (!Compilations)
+        llvm::report_fatal_error(ErrorMessage);
+    } else if (!SourcePaths.empty()) {
+      // Detect compilation database from the first source path.
           SourcePaths[0], ErrorMessage));
       // If no compilation database can be detected from source then we create
       // a new FixedCompilationDatabase with c++11 support.
       if (!Compilations) {
-        std::string CommandLine[] = {"-std=c++11"};
+        std::string CommandLine[] = { "-std=c++11" };
         Compilations.reset(new FixedCompilationDatabase(".", CommandLine));
-    }
-    if (!Compilations)
-      llvm::report_fatal_error(ErrorMessage);
+    } else
+      llvm::report_fatal_error("Could not determine sources to transform.");
I think we should just have a function that load the compilation database.
A function with a comment that states the behavior clearly and with the following prototype but a better name:

  CompilationDatabase *loadCompileDB(std::string &ErrorMessage);

The function doesn't need to use imbricated if-else-if-else then, just use early returns

* if -p is provided, return autodetect, return 0 on error with ErrorMessage set
* if !SourcePath.empty() try to detect from file, if detection worked return
* return fixed db with `-std=c++11`

The function doesn't need to take care of SourcePaths being empty.

Comment at: clang-modernize/tool/ClangModernize.cpp:342-365
@@ -324,1 +341,26 @@
+  if (!SourcePaths.empty()) {
+    // Remove explicitly provided sources that are in the exclude list.
+    std::vector<std::string>::iterator I = SourcePaths.begin();
+    while (I != SourcePaths.end()) {
+      if (GlobalOptions.ModifiableFiles.isFileExplicitlyExcluded(*I)) {
+        llvm::errs() << "Warning \"" << *I << "\" will not be transformed "
+                     << "because it's in the excluded list.\n";
+        I = SourcePaths.erase(I);
+      } else
+        ++I;
+    }
+  } else if (!GlobalOptions.ModifiableFiles.isIncludeListEmpty()) {
+    // Use source paths from the compilation database.
+    // We only transform files that are explicitly included.
+    std::vector<std::string> Files = Compilations->getAllFiles();
+    std::remove_copy_if(Files.begin(), Files.end(),
+       std::back_inserter(SourcePaths), isFileNotIncludedPredicate());
+  } else
+       llvm::report_fatal_error("Use -include to indicate which files of the "
+                                "compilation database to transform.");
+  // There is no source to transform.
+  if (SourcePaths.empty())
+    llvm::report_fatal_error("Could not determine sources to transform.");
I think for the list of file we can do two things.
Let's start by creating the final list of files:

  std::vector<std::string> Files;

Then the algorithm stays quite similar to what we have now:

* If `SourcePaths` isn't empty then use `std::remove_copy_if`, to copy `SourcePaths` files to `Files` but removing the ones explicitly excluded. The functor passed to `std::remove_copy_if` displays the warning.
* Otherwise
** First check if include list is empty, if so print the error about '-include' and exit
** Set `Files` to the result of `Compilations->getAllFiles()` and use `std::remove_if` to remove files that aren't included. I believe it's easier to understand than the "doubly negated" logic of `remove_copy_if` / file_not_included.

Now just need to print an error and exit if there is no source files.
Also replace occurences of SourcePaths by Files in the remaining code.


More information about the cfe-commits mailing list