[clang][Tooling] Prefer -x over -std when interpolating

[kadircet]

Summary

Currently interpolation logic prefers -std over -x. But the latter is a
more strong signal, so this patch inverts the order and only makes use of -std
if -x didn't exist.

Fixes https://github.com/clangd/clangd/issues/185

Thanks @sammccall for tracking this down!

[merge_guards_bot]

Unit tests: pass. 61035 tests passed, 0 failed and 728 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[kbobyrev]

LGTM

[sammccall]

Thanks for taking care of this!

clang/lib/Tooling/InterpolatingCompilationDatabase.cpp

194

Note that type is initialized from guessType, not only -x, so this comment isn't true in general, just for *.h.

Example: if CDB contains clang -std=c++14 foo.cc, then we'll treat foo.mm as obj-c++ rather than c++. (The right thing in that case and probably in general)

clang/lib/Tooling/InterpolatingCompilationDatabase.cpp

for (unsigned Pos = 1; Pos < OldArgs.size();) {
           Std = *GivenStd;
         continue;
       }
 
       Cmd.CommandLine.insert(Cmd.CommandLine.end(),
                              OldArgs.data() + OldPos, OldArgs.data() + Pos);
     }
 
-    if (Std != LangStandard::lang_unspecified) // -std take precedence over -x
+    // Make use of -std iff -x was missing.
+    if (Type == types::TY_INVALID && Std != LangStandard::lang_unspecified)
       Type = toType(LangStandard::getLangStandardForKind(Std).getLanguage());
     Type = foldType(*Type);
     // The contract is to store None instead of TY_INVALID.
     if (Type == types::TY_INVALID)
       Type = llvm::None;
   }
 
   // Produce a CompileCommand for \p filename, based on this one.

clang/unittests/Tooling/CompilationDatabaseTest.cpp

TEST_F(InterpolateTest, Nearby) {
   // if nothing matches at all, we still get the closest alpha match
   EXPECT_EQ(getProxy("below/some/obscure/path.cpp"), "an/other/foo.cpp");
 }
 
 TEST_F(InterpolateTest, Language) {
   add("dir/foo.cpp", "-std=c++17");
   add("dir/bar.c", "");
   add("dir/baz.cee", "-x c");
+  add("dir/aux.cpp", "-std=c++17 -x objective-c++");
 
   // .h is ambiguous, so we add explicit language flags
   EXPECT_EQ(getCommand("foo.h"),
             "clang -D dir/foo.cpp -x c++-header -std=c++17");
   // Same thing if we have no extension. (again, we treat as header).
   EXPECT_EQ(getCommand("foo"), "clang -D dir/foo.cpp -x c++-header -std=c++17");
   // and invalid extensions.
   EXPECT_EQ(getCommand("foo.cce"),
             "clang -D dir/foo.cpp -x c++-header -std=c++17");
   // and don't add -x if the inferred language is correct.
   EXPECT_EQ(getCommand("foo.hpp"), "clang -D dir/foo.cpp -std=c++17");
   // respect -x if it's already there.
   EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c-header");
   // prefer a worse match with the right extension.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/bar.c");
   Entries.erase(path(StringRef("dir/bar.c")));
   // Now we transfer across languages, so drop -std too.
   EXPECT_EQ(getCommand("foo.c"), "clang -D dir/foo.cpp");
+  // Prefer -x over -std when overriding language.
+  EXPECT_EQ(getCommand("aux.h"),
+            "clang -D dir/aux.cpp -x objective-c++-header -std=c++17");
 }
 
 TEST_F(InterpolateTest, Strip) {
   add("dir/foo.cpp", "-o foo.o -Wall");
   // the -o option and the input file are removed, but -Wall is preserved.
   EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
 }