[PATCH] D85532: Correctly set CompilingPCH in PrecompilePreambleAction.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe2d61ae57333: Correctly set CompilingPCH in PrecompilePreambleAction. (authored by adamcz). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D85532: Correctly set CompilingPCH in PrecompilePreambleAction.

adamcz updated this revision to Diff 284390. adamcz added a comment. assert() -> ASSERT_TRUE Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85532/new/ https://reviews.llvm.org/D85532 Files: clang-tools-extra/clangd/unittests/CMakeLists.txt clan

[PATCH] D85532: Correctly set CompilingPCH in PrecompilePreambleAction.

adamcz added inline comments. Comment at: clang/unittests/Frontend/ASTUnitTest.cpp:146 + auto File = AU->getFileManager().getFileRef("Textual.h", false, false); + assert(File); + // Verify that we do not crash here. adamcz wrote: > kadircet wrote: > > ASSERT_T

[PATCH] D85532: Correctly set CompilingPCH in PrecompilePreambleAction.

adamcz added inline comments. Comment at: clang/unittests/Frontend/ASTUnitTest.cpp:146 + auto File = AU->getFileManager().getFileRef("Textual.h", false, false); + assert(File); + // Verify that we do not crash here. kadircet wrote: > ASSERT_TRUE again You can

[PATCH] D85532: Correctly set CompilingPCH in PrecompilePreambleAction.

adamcz updated this revision to Diff 284365. adamcz marked an inline comment as done. adamcz added a comment. more review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85532/new/ https://reviews.llvm.org/D85532 Files: clang-tools-extra/

[PATCH] D85532: Correctly set CompilingPCH in PrecompilePreambleAction.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang/unittests/Frontend/ASTUnitTest.cpp:144 + CInvok, PCHContainerOps, Diags, FileMgr, false, CaptureDiagsKind::None, 1, + TU_Comple

[PATCH] D85532: Correctly set CompilingPCH in PrecompilePreambleAction.

adamcz added inline comments. Comment at: clang-tools-extra/clangd/unittests/ModulesTests.cpp:28 +)cpp"); + TU.ExtraArgs.push_back("-fmodules"); + TU.ExtraArgs.push_back("-fmodule-name=M"); kadircet wrote: > this one is not needed right? I suppose not. ==

[PATCH] D85532: Correctly set CompilingPCH in PrecompilePreambleAction.

adamcz updated this revision to Diff 284329. adamcz marked 3 inline comments as done. adamcz added a comment. addressed review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85532/new/ https://reviews.llvm.org/D85532 Files: clang-tools-e

[PATCH] D85532: Correctly set CompilingPCH in PrecompilePreambleAction.

kadircet added a comment. Regarding tests, it feels like we can also test this in ASTUnitTests which is directly in clang, as it is also using PrecompiledPreamble::Build. What about moving the test there instead? Comment at: clang-tools-extra/clangd/unittests/ModulesTests.cpp

[PATCH] D85532: Correctly set CompilingPCH in PrecompilePreambleAction.

adamcz updated this revision to Diff 283922. adamcz added a comment. This time the fix is included ;-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85532/new/ https://reviews.llvm.org/D85532 Files: clang-tools-extra/clangd/unittests/CMakeLists.

[PATCH] D85532: Correctly set CompilingPCH in PrecompilePreambleAction.

adamcz created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, mgorny. Herald added a project: clang. adamcz requested review of this revision. Herald added a subscriber: ilya-biryukov. This fixes a crash bug in clangd when used with modules. ASTWrite