compile commands header to source heuristic lower-cases filenames before inferring file types

[ishaangandhi]

Summary

This leads to ".C" files being rewritten as ".c" files and being inferred to be "c" files as opposed to "c++" files.

See https://github.com/clangd/clangd/issues/1108 for the original bug report.

[sammccall]

Change looks good, thank you!

Can you add a test demonstrating it, and update the diff here?
Take a look at the InterpolateTest cases in clang/unittests/Tooling/CompilationDatabaseTest.cpp.

(If you can, please create diffs with context using -U99999 - this lets us see more context around the changed lines in phabricator)

clang/lib/Tooling/InterpolatingCompilationDatabase.cpp

332

nit: StringRef() isn't needed here, conversion will happen implicitly.

(The one on 329 is needed because the overload set of StringSaver::save is ambiguous)

[ishaangandhi]

• Added a test case
• Removed redundant "StringRef" constructor

[ishaangandhi]

Test case added, re-diffed with -U9999, and removed redundant constructor. Thanks for the quick feedback, @sammccall!

(I didn't wait for the tests to run locally, I am hoping to use your CI systems to do that. It seems like a simple enough of a test that I hope I can get it right on the first try.)

[sammccall]

In D124262#3471841, @ishaangandhi wrote:

Test case added, re-diffed with -U9999, and removed redundant constructor. Thanks for the quick feedback, @sammccall!

(I didn't wait for the tests to run locally, I am hoping to use your CI systems to do that. It seems like a simple enough of a test that I hope I can get it right on the first try.)

Unfortunately it looks like you've created a patch that phabricator can render, but the precommit CI can't apply (it's using git apply).
The usual way to upload is with arc, but the output of git diff -U9999 should also work.

clang/unittests/Tooling/CompilationDatabaseTest.cpp

843

Is this really what you intend to test?

I think our goal here is to treat .H and .C as C++ rather than C, not to link them together in some way. If the current file is foo.H, then foo.cpp and foo.C should be equally compelling candidates, but both beat foo.c.

[ishaangandhi]

clang/unittests/Tooling/CompilationDatabaseTest.cpp

843

Bleh brain fart. You're right. "exact.C" should win over "exact.c", not necessarily "exact.cpp".

[ishaangandhi]

Made test case reflect that proxies for ".H" files are ".C" files, and not ".c files".

[sammccall]

Thanks!
I can land this for you if you don't have commit access - can you provide the name/email to use for the commit?

[ishaangandhi]

In D124262#3472079, @sammccall wrote:

Thanks!
I can land this for you if you don't have commit access - can you provide the name/email to use for the commit?

Thank you!

Ishaan Gandhi
[e-mail]

clang/lib/Tooling/InterpolatingCompilationDatabase.cpp

FileIndex(std::vector<std::string> Files)
     llvm::sort(OriginalPaths);
     Paths.reserve(OriginalPaths.size());
     Types.reserve(OriginalPaths.size());
     Stems.reserve(OriginalPaths.size());
     for (size_t I = 0; I < OriginalPaths.size(); ++I) {
       StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
       Paths.emplace_back(Path, I);
-      Types.push_back(foldType(guessType(Path)));
+      Types.push_back(foldType(guessType(OriginalPaths[I])));
       Stems.emplace_back(sys::path::stem(Path), I);
       auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
       for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, ++Dir)
         if (Dir->size() > ShortDirectorySegment) // not trivial ones
           Components.emplace_back(*Dir, I);
     }
     llvm::sort(Paths);
     llvm::sort(Stems);
     llvm::sort(Components);

clang/unittests/Tooling/CompilationDatabaseTest.cpp

 
 TEST_F(InterpolateTest, Case) {
   add("FOO/BAR/BAZ/SHOUT.cc");
   add("foo/bar/baz/quiet.cc");
   // Case mismatches are completely ignored, so we choose the name match.
   EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST_F(InterpolateTest, LanguagePreference) {
+  add("foo/bar/baz/exact.C");
+  add("foo/bar/baz/exact.c");
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".c files".
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");
+}
+
 TEST_F(InterpolateTest, Aliasing) {
   add("foo.cpp", "-faligned-new");
 
   // The interpolated command should keep the given flag as written, even though
   // the flag is internally represented as an alias.
   EXPECT_EQ(getCommand("foo.hpp"), "clang -D foo.cpp -faligned-new");
 }