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

[Guillaume Papin]

  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.
       Compilations.reset(CompilationDatabase::autoDetectFromDirectory(
           BuildPath, ErrorMessage));
-    } else {
+      if (!Compilations)
+        llvm::report_fatal_error(ErrorMessage);
+    } else if (!SourcePaths.empty()) {
+      // Detect compilation database from the first source path.
       Compilations.reset(CompilationDatabase::autoDetectFromSource(
           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.


http://llvm-reviews.chandlerc.com/D1517