Index: clangd/TUScheduler.cpp =================================================================== --- clangd/TUScheduler.cpp +++ clangd/TUScheduler.cpp @@ -370,8 +370,8 @@ bool CanReuseAST = InputsAreTheSame && (OldPreamble == NewPreamble); { std::lock_guard Lock(Mutex); - if (NewPreamble) - LastBuiltPreamble = NewPreamble; + // Always use the NewPreamble. + LastBuiltPreamble = NewPreamble; } // Before doing the expensive AST reparse, we want to release our reference // to the old preamble, so it can be freed if there are no other references Index: unittests/clangd/XRefsTests.cpp =================================================================== --- unittests/clangd/XRefsTests.cpp +++ unittests/clangd/XRefsTests.cpp @@ -1021,6 +1021,65 @@ EXPECT_THAT(*Locations, IsEmpty()); } +TEST(GoToDefinitoin, WithPreamble) { + // Test stragety: AST should always use the latest preamble instead of last + // good preamble. + MockFSProvider FS; + IgnoreDiagnostics DiagConsumer; + MockCompilationDatabase CDB; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + + auto FooCpp = testPath("foo.cpp"); + auto FooCppUri = URIForFile{FooCpp}; + // The trigger locations must be the same. + Annotations FooWithHeader(R"cpp(#include "fo^o.h")cpp"); + Annotations FooWithoutHeader(R"cpp(double [[fo^o]]();)cpp"); + + FS.Files[FooCpp] = FooWithHeader.code(); + + auto FooH = testPath("foo.h"); + auto FooHUri = URIForFile{FooH}; + Annotations FooHeader(R"cpp([[]])cpp"); + FS.Files[FooH] = FooHeader.code(); + + runAddDocument(Server, FooCpp, FooWithHeader.code()); + // GoToDefinition goes to a #include file: the result comes from the preamble. + Server.findDefinitions( + FooCpp, FooWithHeader.point(), + [&](llvm::Expected> Loc) { + ASSERT_TRUE(bool(Loc)); + EXPECT_THAT(*Loc, ElementsAre(Location{FooHUri, FooHeader.range()})); + }); + + // Only preamble is built, and no AST is built in this request. + Server.addDocument(FooCpp, FooWithoutHeader.code(), WantDiagnostics::No); + // We build AST here, and it should use the latest preamble rather than the + // stale one. + Server.findDefinitions( + FooCpp, FooWithoutHeader.point(), + [&](llvm::Expected> Loc) { + ASSERT_TRUE(bool(Loc)); + EXPECT_THAT(*Loc, + ElementsAre(Location{FooCppUri, FooWithoutHeader.range()})); + }); + EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Wait for GoToDefinition"; + + + // Reset test environment. + runAddDocument(Server, FooCpp, FooWithHeader.code()); + // Both preamble and AST are built in this request. + Server.addDocument(FooCpp, FooWithoutHeader.code(), WantDiagnostics::Yes); + // Use the AST being built in above request. + Server.findDefinitions( + FooCpp, FooWithoutHeader.point(), + [&](llvm::Expected> Loc) { + ASSERT_TRUE(bool(Loc)); + EXPECT_THAT(*Loc, + ElementsAre(Location{FooCppUri, FooWithoutHeader.range()})); + }); + EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Wait for GoToDefinition"; +} + } // namespace } // namespace clangd } // namespace clang