Files in a compilation database using relative paths are not being transformed

[arielbernal]

Summary

This patch fixes the path for the YAML files, and complements D1800 which makes replacements independent of the location.

[revane]

Could you expand "rel" to relative wherever it appears? LLVM style is shorthand-averse.

clang-modernize/Core/Transform.cpp

102–105

Wouldn't it be better to do this once per source file in ClangModernizer.cpp instead of once per source per transform here?

[arielbernal]

@revane sure, I'll rename those files files.

clang-modernize/Core/Transform.cpp

102–105

No problem, but I'm not convinced that this is the right approach, if we move the YAML files to a folder this will go away.

[arielbernal]

@revane, Sources are made absolute in ClangModernizer.cpp but Filename here is coming from the command line in the compilation database and that path is relative to the directory so in order to make this change only once we should parse the compilation database command and fix the path. I guess this could be the best solution but could involve some re-factoring since ClangTool assumes that the files are provided relative to the directory in the compilation database and changes directories so everything would be relative to the current directory.
CurrentSource is just used for YAML files to be created in the correct place since D1800 is fixing the replacements for relative paths.

[arielbernal]

Addressed comments

[arielbernal]

clang-modernize/Core/Transform.cpp

103

@revane Please disregard these Overrides lines, they got mixed up in a merge. They are not anymore there.

[Eugene.Zelenko]

Looks like patch was not committed, but may be irrelevant these days.

clang-modernize/Core/Transform.cpp

bool Transform::isFileModifiable(const SourceManager &SM,
 
   return GlobalOptions.ModifiableFiles.isFileIncluded(FE->getName());
 }
 
 bool Transform::handleBeginSource(CompilerInstance &CI, StringRef Filename) {
   assert(Overrides != 0 && "Subclass transform didn't provide InputState");
 
   Overrides->applyOverrides(CI.getSourceManager());
-  CurrentSource = Filename;
+
+  llvm::SmallString<256> FilePath(Filename);
+  llvm::error_code EC = llvm::sys::fs::make_absolute(FilePath);
+  assert(!EC && "File path can't be made absolute");
+  (void) EC;
+
+  CurrentSource = FilePath.c_str();
 
   if (Options().EnableTiming) {
     Timings.push_back(std::make_pair(Filename.str(), llvm::TimeRecord()));
     Timings.back().second -= llvm::TimeRecord::getCurrentTime(true);
   }
   return true;
 }
 

test/clang-modernize/Compilations/Inputs/compile_commands_rel.json

+[
+{
+  "directory": "$(path)/a1/",
+  "command": "clang++ -o compilations.o -c compilations.cpp -std=c++11",
+  "file": "compilations.cpp"
+},
+{
+  "directory": "$(path)/a2/",
+  "command": "clang++ -o compilations.o -c compilations.cpp -std=c++11",
+  "file": "compilations.cpp"
+},
+{
+  "directory": "$(path)/a3/",
+  "command": "clang++ -o compilations.o -c compilations.cpp -std=c++11",
+  "file": "compilations.cpp"
+}
+]

test/clang-modernize/Compilations/compilation_rel.cpp

+// The following block tests:
+//   - Relative paths are use in the compilation database.
+//   - A compilation database is detected from build path specified by -p and
+//     -include was provided.
+
+// Create directory structure
+// a1, a2  and a3 are specified paths for files in the compilation database.
+// RUN: rm -rf %T/CompilationRel
+// RUN: mkdir -p %T/CompilationRel
+// RUN: mkdir -p %T/CompilationRel/a1
+// RUN: mkdir -p %T/CompilationRel/a2
+// RUN: mkdir -p %T/CompilationRel/a3
+
+// This test uses a compilation database
+// RUN: sed -e 's#$(path)#%/T/CompilationRel#g' %S/Inputs/compile_commands_rel.json > %T/CompilationRel/compile_commands.json
+
+// Check that files are tranformed when -p and -include are specified.
+// RUN: cp %S/Inputs/compilations.cpp %T/CompilationRel/a1
+// RUN: cp %S/Inputs/compilations.cpp %T/CompilationRel/a2
+// RUN: cp %S/Inputs/compilations.cpp %T/CompilationRel/a3
+
+// RUN: clang-modernize -use-nullptr -p=%T/CompilationRel -include=%T/CompilationRel/a1,%T/CompilationRel/a3
+// RUN: diff -b %S/Inputs/compilations_expected.cpp %T/CompilationRel/a1/compilations.cpp
+// RUN: not diff -b %S/Inputs/compilations_expected.cpp %T/CompilationRel/a2/compilations.cpp
+// RUN: diff -b %S/Inputs/compilations_expected.cpp %T/CompilationRel/a3/compilations.cpp

test/clang-modernize/Compilations/no_compilation_rel.cpp

+// The following block tests that files are transformed when specified using
+// relative paths.
+
+// Create directory structure
+// a1, a2  and a3 are specified paths for files in the compilation database.
+// RUN: rm -rf %T/NoCompilationRel
+// RUN: mkdir -p %T/NoCompilationRel
+// RUN: mkdir -p %T/NoCompilationRel/a1
+// RUN: mkdir -p %T/NoCompilationRel/a2
+// RUN: mkdir -p %T/NoCompilationRel/a3
+
+// RUN: cp %S/Inputs/compilations.cpp %T/NoCompilationRel/a1
+// RUN: cp %S/Inputs/compilations.cpp %T/NoCompilationRel/a2
+// RUN: cp %S/Inputs/compilations.cpp %T/NoCompilationRel/a3
+// RUN: cd %T
+// RUN: clang-modernize -use-nullptr ./NoCompilationRel/a1/compilations.cpp ./NoCompilationRel/a3/compilations.cpp
+// RUN: diff -b %S/Inputs/compilations_expected.cpp %T/NoCompilationRel/a1/compilations.cpp
+// RUN: not diff -b %S/Inputs/compilations_expected.cpp %T/NoCompilationRel/a2/compilations.cpp
+// RUN: diff -b %S/Inputs/compilations_expected.cpp %T/NoCompilationRel/a3/compilations.cpp