[clangd] Refine recoveryAST flags in clangd

[hokein]

Summary

so that we could start experiment for C.

Previously, these flags in clangd were only meaningful for C++. We need
to flip them for C, this patch repurpose these flags.

• if true, just set it.
• if false, just respect the value in clang.

this would allow us to keep flags on for C++, and optionally flip them on for C.

[sammccall]

This is fine as is, but could consider retiring the flags instead.

clang-tools-extra/clangd/ClangdServer.h

132

nit: respect to -> respect

This could just be "If true, force -frecovery-ast-type"

clang-tools-extra/clangd/Compiler.cpp

84–88

I don't have any problem with the changes to the flags, but we can also do experiments just with config now:

CompileFlags:
  Add: [-Xclang, -frecovery-ast]

in ~/.config/clangd/config.yaml

[hokein]

clang-tools-extra/clangd/Compiler.cpp

84–88

this sounds a new idea, but it looks like our internal client doesn't fully support config (yet?).

[hokein]

address comments.

clang-tools-extra/clangd/ClangdServer.h

struct Options {
 
     /// If set, enable clang-tidy in clangd and use to it get clang-tidy
     /// configurations for a particular file.
     /// Clangd supports only a small subset of ClangTidyOptions, these options
     /// (Checks, CheckOptions) are about which clang-tidy checks will be
     /// enabled.
     ClangTidyOptionsBuilder GetClangTidyOptions;
 
-    /// If true, turn on the `-frecovery-ast` clang flag.
-    bool BuildRecoveryAST = true;
+    /// If true, force -frecovery-ast flag.
+    /// If false, respect the value in clang.
+    bool BuildRecoveryAST = false;
 
-    /// If true, turn on the `-frecovery-ast-type` clang flag.
-    bool PreserveRecoveryASTType = true;
+    /// If true, force -frecovery-ast-type flag.
+    /// If false, respect the value in clang.
+    bool PreserveRecoveryASTType = false;
 
     /// Clangd's workspace root. Relevant for "workspace" operations not bound
     /// to a particular file.
     /// FIXME: If not set, should use the current working directory.
     llvm::Optional<std::string> WorkspaceRoot;
 
     /// The resource directory is used to find internal headers, overriding
     /// defaults and -resource-dir compiler flag).

clang-tools-extra/clangd/Compiler.cpp

buildCompilerInvocation(const ParseInputs &Inputs, clang::DiagnosticConsumer &D,
   CI->getPreprocessorOpts().ImplicitPCHInclude.clear();
   CI->getPreprocessorOpts().PrecompiledPreambleBytes = {0, false};
   CI->getPreprocessorOpts().PCHThroughHeader.clear();
   CI->getPreprocessorOpts().PCHWithHdrStop = false;
   CI->getPreprocessorOpts().PCHWithHdrStopCreate = false;
   // Don't crash on `#pragma clang __debug parser_crash`
   CI->getPreprocessorOpts().DisablePragmaDebugCrash = true;
 
-  // Recovery expression currently only works for C++.
-  if (CI->getLangOpts()->CPlusPlus) {
-    CI->getLangOpts()->RecoveryAST = Inputs.Opts.BuildRecoveryAST;
-    CI->getLangOpts()->RecoveryASTType = Inputs.Opts.PreserveRecoveryASTType;
-  }
+  if (Inputs.Opts.BuildRecoveryAST)
+    CI->getLangOpts()->RecoveryAST = true;
+  if (Inputs.Opts.PreserveRecoveryASTType)
+    CI->getLangOpts()->RecoveryASTType = true;
+
   return CI;
 }
 
 std::unique_ptr<CompilerInstance>
 prepareCompilerInstance(std::unique_ptr<clang::CompilerInvocation> CI,
                         const PrecompiledPreamble *Preamble,
                         std::unique_ptr<llvm::MemoryBuffer> Buffer,
                         llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,

clang-tools-extra/clangd/tool/ClangdMain.cpp

opt<bool> CrossFileRename{
     cat(Features),
     desc("Enable cross-file rename feature."),
     init(true),
 };
 
 opt<bool> RecoveryAST{
     "recovery-ast",
     cat(Features),
-    desc("Preserve expressions in AST for broken code (C++ only)."),
+    desc("Preserve expressions in AST for broken code."),
     init(ClangdServer::Options().BuildRecoveryAST),
 };
 
 opt<bool> RecoveryASTType{
     "recovery-ast-type",
     cat(Features),
     desc("Preserve the type for recovery AST."),
     init(ClangdServer::Options().PreserveRecoveryASTType),

clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

CodeCompleteResult completions(const TestTU &TU, Position Point,
   if (!IndexSymbols.empty()) {
     assert(!Opts.Index && "both Index and IndexSymbols given!");
     OverrideIndex = memIndex(std::move(IndexSymbols));
     Opts.Index = OverrideIndex.get();
   }
 
   MockFS FS;
   auto Inputs = TU.inputs(FS);
-  Inputs.Opts.BuildRecoveryAST = true;
-  Inputs.Opts.PreserveRecoveryASTType = true;
   IgnoreDiagnostics Diags;
   auto CI = buildCompilerInvocation(Inputs, Diags);
   if (!CI) {
     ADD_FAILURE() << "Couldn't build CompilerInvocation";
     return {};
   }
   auto Preamble = buildPreamble(testPath(TU.Filename), *CI, Inputs,
                                 /*InMemory=*/true, /*Callback=*/nullptr);
SignatureHelp signatures(llvm::StringRef Text, Position Point,
   std::unique_ptr<SymbolIndex> Index;
   if (!IndexSymbols.empty())
     Index = memIndex(IndexSymbols);
 
   auto TU = TestTU::withCode(Text);
   MockFS FS;
   auto Inputs = TU.inputs(FS);
   Inputs.Index = Index.get();
-  Inputs.Opts.BuildRecoveryAST = true;
-  Inputs.Opts.PreserveRecoveryASTType = true;
   IgnoreDiagnostics Diags;
   auto CI = buildCompilerInvocation(Inputs, Diags);
   if (!CI) {
     ADD_FAILURE() << "Couldn't build CompilerInvocation";
     return {};
   }
   auto Preamble = buildPreamble(testPath(TU.Filename), *CI, Inputs,
                                 /*InMemory=*/true, /*Callback=*/nullptr);

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Code = R"cpp(
     void test() {
     [[label]]:
       return;
     }
   )cpp";
   EXPECT_DECLS("LabelStmt", "label:");
 }
 
+TEST_F(TargetDeclTest, RecoveryForC) {
+  Flags = {"-xc", "-Xclang", "-frecovery-ast"};
+  Code = R"cpp(
+    // error-ok: testing behavior on broken code
+    // int f();
+    int f(int);
+    int x = [[f]]();
+  )cpp";
+  EXPECT_DECLS("DeclRefExpr", "int f(int)");
+}
+
 TEST_F(TargetDeclTest, Recovery) {
   Code = R"cpp(
     // error-ok: testing behavior on broken code
     int f();
     int f(int, int);
     int x = [[f]](42);
   )cpp";
   EXPECT_DECLS("UnresolvedLookupExpr", "int f()", "int f(int, int)");

clang-tools-extra/clangd/unittests/TestTU.cpp

ParseInputs TestTU::inputs(MockFS &FS) const {
   Argv.push_back(FullFilename);
   Inputs.CompileCommand.Filename = FullFilename;
   Inputs.CompileCommand.Directory = testRoot();
   Inputs.Contents = Code;
   if (OverlayRealFileSystemForModules)
     FS.OverlayRealFileSystemForModules = true;
   Inputs.TFS = &FS;
   Inputs.Opts = ParseOptions();
-  Inputs.Opts.BuildRecoveryAST = true;
-  Inputs.Opts.PreserveRecoveryASTType = true;
   Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;
   Inputs.Opts.ClangTidyOpts.WarningsAsErrors = ClangTidyWarningsAsErrors;
   Inputs.Index = ExternalIndex;
   if (Inputs.Index)
     Inputs.Opts.SuggestMissingIncludes = true;
   return Inputs;
 }