jkorous added a comment.

Hi @kadircet!
I am investigating a failure of `PatchesAdditionalIncludes` test that we got 
internally. It's a failed assert in `ReplayPreamble::replay`.
Our clangd source code is for all practical purposes identical to upstream 
version but not so with clang source. Specifically what we do is that in 
`CompilerInstance::createPreprocessor` we always add one particular callback.
This means that when in the test we are calling `buildPreamble` for `TestTU TU` 
with empty buffer we never hit the early return in `ReplayPreamble::attach()` 
(ParsedAST.cpp:124) like upstream version does and proceed to create a new 
`ReplayPreamble` object with `PreambleBounds` of `size() == 0` which leads to 
`ReplayPreamble::MainFileTokens` to be empty and later we hit failed assert in 
`ReplayPreamble::replay` about `assert(HashTok != MainFileTokens.end() && ...)`.

Now, while I can just tweak either `ReplayPreamble::attach()` or remove the PP 
callback in the test internally I am wondering whether you consider empty 
preamble & PP with callbacks valid use-case of `ReplayPreamble` and worth a fix.

This is where we are creating the empty `MainFileTokens`:

  * frame #0: 0x0000000103337649 ClangdTests` clang::clangd::(anonymous 
namespace)::ReplayPreamble::ReplayPreamble(this=0x0000000114f45da0, 
Includes=0x00007ffeefbfc678, Delegate=0x0000000114f3bd80, 
SM=0x0000000114f3dd80, PP=0x0000000115850218, LangOpts=0x0000000114f38eb0, 
PB=0x00007ffeefbfc658)  + 169 at ParsedAST.cpp:142
    frame #1: 0x000000010333756d ClangdTests` clang::clangd::(anonymous 
namespace)::ReplayPreamble::ReplayPreamble(this=0x0000000114f45da0, 
Includes=0x00007ffeefbfc678, Delegate=0x0000000114f3bd80, 
SM=0x0000000114f3dd80, PP=0x0000000115850218, LangOpts=0x0000000114f38eb0, 
PB=0x00007ffeefbfc658)  + 77 at ParsedAST.cpp:139
    frame #2: 0x0000000103334fa7 ClangdTests` clang::clangd::(anonymous 
namespace)::ReplayPreamble::attach(Includes=0x00007ffeefbfc678, 
Clang=0x0000000114f33ea0, PB=0x00007ffeefbfc658)  + 183 at ParsedAST.cpp:126
    frame #3: 0x0000000103334189 ClangdTests` 
clang::clangd::ParsedAST::build(Filename=(Data = "/clangd-test/foo.cpp", Length 
= 20), Inputs=0x00007ffeefbfdf98, CI=nullptr, 
CompilerInvocationDiags=ArrayRef<clang::clangd::Diag> @ 0x00007ffeefbfd830, 
Preamble=std::__1::shared_ptr<const clang::clangd::PreambleData>::element_type 
@ 0x0000000114f3e728 strong=2 weak=1)  + 3897 at ParsedAST.cpp:385
    frame #4: 0x00000001006f1032 ClangdTests` clang::clangd::(anonymous 
namespace)::ParsedASTTest_PatchesAdditionalIncludes_Test::TestBody(this=0x0000000114f04090)
  + 1778 at ParsedASTTests.cpp:477

This is the failed assert:

  * frame #4: 0x0000000103337a0c ClangdTests` clang::clangd::(anonymous 
namespace)::ReplayPreamble::replay(this=0x0000000114f45da0)  + 556 at 
ParsedAST.cpp:182
    frame #5: 0x000000010333777c ClangdTests` clang::clangd::(anonymous 
namespace)::ReplayPreamble::FileChanged(this=0x0000000114f45da0, Loc=(ID = 74), 
Reason=ExitFile, Kind=C_User, PrevFID=(ID = 2))  + 156 at ParsedAST.cpp:166
    frame #6: 0x0000000101b7900a ClangdTests` 
clang::PPChainedCallbacks::FileChanged(this=0x0000000116204080, Loc=(ID = 74), 
Reason=ExitFile, FileType=C_User, PrevFID=(ID = 2))  + 90 at PPCallbacks.h:390
    frame #7: 0x0000000101b79046 ClangdTests` 
clang::PPChainedCallbacks::FileChanged(this=0x00000001162040c0, Loc=(ID = 74), 
Reason=ExitFile, FileType=C_User, PrevFID=(ID = 2))  + 150 at PPCallbacks.h:391
    frame #8: 0x0000000101b79046 ClangdTests` 
clang::PPChainedCallbacks::FileChanged(this=0x0000000116204100, Loc=(ID = 74), 
Reason=ExitFile, FileType=C_User, PrevFID=(ID = 2))  + 150 at PPCallbacks.h:391
    frame #9: 0x0000000101b79046 ClangdTests` 
clang::PPChainedCallbacks::FileChanged(this=0x0000000116204160, Loc=(ID = 74), 
Reason=ExitFile, FileType=C_User, PrevFID=(ID = 2))  + 150 at PPCallbacks.h:391
    frame #10: 0x0000000101b79046 ClangdTests` 
clang::PPChainedCallbacks::FileChanged(this=0x0000000116205480, Loc=(ID = 74), 
Reason=ExitFile, FileType=C_User, PrevFID=(ID = 2))  + 150 at PPCallbacks.h:391
    frame #11: 0x0000000101b9a590 ClangdTests` 
clang::Preprocessor::HandleEndOfFile(this=0x0000000115850218, 
Result=0x0000000116808210, isEndOfMacro=false)  + 3904 at PPLexerChange.cpp:469
    frame #12: 0x0000000101b38713 ClangdTests` 
clang::Lexer::LexEndOfFile(this=0x0000000116206330, Result=0x0000000116808210, 
CurPtr="")  + 931 at Lexer.cpp:2833
    frame #13: 0x0000000101b39c44 ClangdTests` 
clang::Lexer::LexTokenInternal(this=0x0000000116206330, 
Result=0x0000000116808210, TokAtPhysicalStartOfLine=true)  + 420 at 
Lexer.cpp:3265
    frame #14: 0x0000000101b382f8 ClangdTests` 
clang::Lexer::Lex(this=0x0000000116206330, Result=0x0000000116808210)  + 216 at 
Lexer.cpp:3216
    frame #15: 0x0000000101bd7396 ClangdTests` 
clang::Preprocessor::Lex(this=0x0000000115850218, Result=0x0000000116808210)  + 
118 at Preprocessor.cpp:900
    frame #16: 0x0000000101b75ef1 ClangdTests` 
clang::Preprocessor::CachingLex(this=0x0000000115850218, 
Result=0x0000000116808210)  + 289 at PPCaching.cpp:63
    frame #17: 0x0000000101bd73d5 ClangdTests` 
clang::Preprocessor::Lex(this=0x0000000115850218, Result=0x0000000116808210)  + 
181 at Preprocessor.cpp:906
    frame #18: 0x0000000104e3c21c ClangdTests` 
clang::Parser::TryConsumeToken(this=0x0000000116808200, Expected=semi)  + 204 
at Parser.h:489
    frame #19: 0x0000000104e3bf1a ClangdTests` 
clang::Parser::ExpectAndConsumeSemi(this=0x0000000116808200, DiagID=1526)  + 42 
at Parser.cpp:162
    frame #20: 0x0000000104d5f0e0 ClangdTests` 
clang::Parser::ParseDeclGroup(this=0x0000000116808200, DS=0x00007ffeefbfb6e8, 
Context=FileContext, DeclEnd=0x0000000000000000, FRI=0x0000000000000000)  + 
3168 at ParseDecl.cpp:2243
    frame #21: 0x0000000104e42c48 ClangdTests` 
clang::Parser::ParseDeclOrFunctionDefInternal(this=0x0000000116808200, 
attrs=0x00007ffeefbfbc80, DS=0x00007ffeefbfb6e8, AS=AS_none)  + 1704 at 
Parser.cpp:1109
    frame #22: 0x0000000104e42170 ClangdTests` 
clang::Parser::ParseDeclarationOrFunctionDefinition(this=0x0000000116808200, 
attrs=0x00007ffeefbfbc80, DS=0x0000000000000000, AS=AS_none)  + 144 at 
Parser.cpp:1125
    frame #23: 0x0000000104e411fa ClangdTests` 
clang::Parser::ParseExternalDeclaration(this=0x0000000116808200, 
attrs=0x00007ffeefbfbc80, DS=0x0000000000000000)  + 3642 at Parser.cpp:945
    frame #24: 0x0000000104e3f254 ClangdTests` 
clang::Parser::ParseTopLevelDecl(this=0x0000000116808200, 
Result=0x00007ffeefbfbde8, IsFirstDecl=false)  + 1828 at Parser.cpp:693
    frame #25: 0x0000000104d465e3 ClangdTests` 
clang::ParseAST(S=0x000000011680c200, PrintStats=false, 
SkipFunctionBodies=false)  + 595 at ParseAST.cpp:158
    frame #26: 0x0000000101992662 ClangdTests` 
clang::ASTFrontendAction::ExecuteAction(this=0x0000000114f349a0)  + 322 at 
FrontendAction.cpp:1066
    frame #27: 0x0000000101991b78 ClangdTests` 
clang::FrontendAction::Execute(this=0x0000000114f349a0)  + 136 at 
FrontendAction.cpp:959
    frame #28: 0x00000001033343ab ClangdTests` 
clang::clangd::ParsedAST::build(Filename=(Data = "/clangd-test/foo.cpp", Length 
= 20), Inputs=0x00007ffeefbfdf98, CI=nullptr, 
CompilerInvocationDiags=ArrayRef<clang::clangd::Diag> @ 0x00007ffeefbfd830, 
Preamble=std::__1::shared_ptr<const clang::clangd::PreambleData>::element_type 
@ 0x0000000114f3e728 strong=2 weak=1)  + 4443 at ParsedAST.cpp:415
    frame #29: 0x00000001006f1032 ClangdTests` clang::clangd::(anonymous 
namespace)::ParsedASTTest_PatchesAdditionalIncludes_Test::TestBody(this=0x0000000114f04090)
  + 1778 at ParsedASTTests.cpp:477


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77644/new/

https://reviews.llvm.org/D77644



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to