[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

bjope added a comment. In D80525#2215778 , @adamcz wrote: > Looks like it's an issue with not clearing the module cache. I'll revert this > change to fix the build, then re-submit with proper fix. Ok, thanks. Let me know if you want me to investigate

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

adamcz added a comment. Looks like it's an issue with not clearing the module cache. I'll revert this change to fix the build, then re-submit with proper fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80525/new/

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

bjope added a comment. Might be that it is some other commit that cause the problem. Looking a bit closer at the jenkins history it seems like the tests passed once after this patch was added. But the next time it failed. That second time the "[Diagnoostics] Reworked -Wstring-concatenation"

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

adamcz added a comment. In D80525#2215755 , @bjope wrote: > In D80525#2215641 , @adamcz wrote: > >> This is quite strange. I can't reproduce this with gcc 7.5.0, 9.3.0 or >> 10.0.1. Which version are you using? >>

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

bjope added a comment. In D80525#2215641 , @adamcz wrote: > This is quite strange. I can't reproduce this with gcc 7.5.0, 9.3.0 or > 10.0.1. Which version are you using? > > There are bugs around clangd + modules + diagnostics. I'm fixing one in >

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

adamcz added a comment. This is quite strange. I can't reproduce this with gcc 7.5.0, 9.3.0 or 10.0.1. Which version are you using? There are bugs around clangd + modules + diagnostics. I'm fixing one in https://reviews.llvm.org/D85753 (not in it's final form yet), but it doesn't seem to be

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

JesperAntonsson added a comment. Hi, In our local Jenkins machinery, the UndefOfModuleMacro test fails on master when built with gcc. Pasting the crash dump below. Any ideas? BR /Jesper 12:24:39 FAIL: Clangd Unit Tests :: ./ClangdTests/SymbolCollectorTest.UndefOfModuleMacro (31492 of 65755)

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

This revision was automatically updated to reflect the committed changes. Closed by commit rG4061d9e42cff: [clangd] Fix crash-bug in preamble indexing when using modules. (authored by adamcz). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80525/new/

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

adamcz updated this revision to Diff 284376. adamcz marked 2 inline comments as done. adamcz added a comment. var rename, comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80525/new/ https://reviews.llvm.org/D80525 Files:

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

adamcz added inline comments. Comment at: clang-tools-extra/clangd/unittests/TestTU.h:71 + // required for use of modules. + // FIXME: Intercept the write of implicitly build module to disk and update + // TestFS with that content to avoid using real file system in tests.

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

sammccall accepted this revision. sammccall added inline comments. Comment at: clang-tools-extra/clangd/unittests/TestFS.h:51 + // This is useful for testing module support. + bool OverlayRealFileSystem = false; }; also ForModules here?

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

adamcz added inline comments. Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:1523 + TU.ExtraArgs.push_back("-fmodule-map-file=/module.map"); + TU.OverlayRealFileSystem = true; + sammccall wrote: > I'm a bit torn on this - this is

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

adamcz updated this revision to Diff 284360. adamcz marked 2 inline comments as done. adamcz added a comment. final(?) review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80525/new/ https://reviews.llvm.org/D80525 Files:

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:1509 +TEST_F(SymbolCollectorTest, UndefOfModuleMacro) { + auto TU =

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

adamcz added a comment. Resurrecting this, with the ignore #undef when #define is in a module approach. Still worth having a test, especially that it's the first test we have for modules. Note that the test code changed since last version due to introduction of ThreasafeFS since my original

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

adamcz updated this revision to Diff 274482. adamcz added a comment. Changed to just ignore undefs without matching def Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80525/new/ https://reviews.llvm.org/D80525 Files:

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

sammccall added a comment. So this is late in the game but... maybe we should just not report this case as a reference? `#undef foo` is valid if `foo` was never defined, and doesn't refer to anything. If we similarly don't resolve the reference in this case as we only import macros that were

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

adamcz updated this revision to Diff 266240. 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/D80525/new/ https://reviews.llvm.org/D80525 Files:

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

sammccall added inline comments. Comment at: clang-tools-extra/clangd/test/symbols-modules.test:1 +# Verify that we do not crash and correctly find the definition of a macro that +# was imported from a module and then #undef-ed in preamble. Generally, we prefer

[PATCH] D80525: [clangd] Fix crash-bug in preamble indexing when using modules.

adamcz created this revision. adamcz added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. When preamble contains #undef, indexing code finds the matching #define and uses that during