[PATCH] D105328: [Frontend] Only compile modules if not already finalized

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG766a08df12c1: [Frontend] Only compile modules if not already finalized (authored by bnbarham, committed by vsapsai). Repository: rG LLVM Github Mo

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

bnbarham marked an inline comment as done. bnbarham added a comment. I don't have commit access. @vsapsai or @akyrtzi would you mind committing this? The test failures seem unrelated to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105328/new/

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

bnbarham marked an inline comment as done. bnbarham added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:2854 +bool recompileFinalized = +Result == OutOfDate && Capabilities & ARR_OutOfDate && +getModuleManager().getModuleCache(

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

bnbarham updated this revision to Diff 358826. bnbarham marked 2 inline comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105328/new/ https://reviews.llvm.org/D105328 Files: clang/include/clang/Basic/DiagnosticCommonKinds.td clang/

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

vsapsai accepted this revision. vsapsai added a comment. This revision is now accepted and ready to land. Have 1 punctuation nit (that I'm not sure about), the rest looks good. Comment at: clang/lib/Serialization/ASTReader.cpp:2854 +bool recompileFinalized = +

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

bnbarham marked 2 inline comments as done. bnbarham added inline comments. Comment at: clang/lib/Frontend/CompilerInstance.cpp:1063 +<< ModuleName; +return ImportingInstance.getFrontendOpts().AllowPCMWithCompilerErrors; + } vsapsai wrote: > bnbarham

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

bnbarham updated this revision to Diff 358798. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105328/new/ https://reviews.llvm.org/D105328 Files: clang/include/clang/Basic/DiagnosticCommonKinds.td clang/include/clang/Basic/DiagnosticSerialization

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

vsapsai added inline comments. Comment at: clang/lib/Frontend/CompilerInstance.cpp:1063 +<< ModuleName; +return ImportingInstance.getFrontendOpts().AllowPCMWithCompilerErrors; + } bnbarham wrote: > bnbarham wrote: > > vsapsai wrote: > > > Can we get

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

bnbarham updated this revision to Diff 358465. bnbarham edited the summary of this revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105328/new/ https://reviews.llvm.org/D105328 Files: clang/include/clang/Basic/DiagnosticCommonKinds.td cla

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

bnbarham marked 5 inline comments as done. bnbarham added inline comments. Comment at: clang/lib/Frontend/CompilerInstance.cpp:1063 +<< ModuleName; +return ImportingInstance.getFrontendOpts().AllowPCMWithCompilerErrors; + } bnbarham wrote: > vsapsai

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

bnbarham added inline comments. Comment at: clang/lib/Frontend/CompilerInstance.cpp:1063 +<< ModuleName; +return ImportingInstance.getFrontendOpts().AllowPCMWithCompilerErrors; + } vsapsai wrote: > Can we get in infinite loop with `AllowPCMWithCompil

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

vsapsai added inline comments. Comment at: clang/lib/Frontend/CompilerInstance.cpp:1063 +<< ModuleName; +return ImportingInstance.getFrontendOpts().AllowPCMWithCompilerErrors; + } Can we get in infinite loop with `AllowPCMWithCompilerErrors = true`?

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

bnbarham added inline comments. Comment at: clang/unittests/Serialization/ModuleCacheTest.cpp:125-126 + ASSERT_TRUE(Invocation2); + CompilerInstance Instance2(Instance.getPCHContainerOperations(), + &Instance.getModuleCache()); + Instance2.setDiagno

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

vsapsai added inline comments. Comment at: clang/unittests/Serialization/ModuleCacheTest.cpp:125-126 + ASSERT_TRUE(Invocation2); + CompilerInstance Instance2(Instance.getPCHContainerOperations(), + &Instance.getModuleCache()); + Instance2.setDiagnos

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

bnbarham updated this revision to Diff 356861. bnbarham added a comment. Updated the test comment with the actual cause (ie. `Top` being recompiled and inserted, not `M`). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105328/new/ https://reviews.l

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

bnbarham added a comment. In D105328#2861028 , @vsapsai wrote: > This problem shouldn't happen based on module-level name hashing but for some > reason it is happening. If I'm not mistaken, InMemoryModuleCache uses .pcm > file name as the key. But that

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

vsapsai added a comment. This problem shouldn't happen based on module-level name hashing but for some reason it is happening. If I'm not mistaken, InMemoryModuleCache uses .pcm file name as the key. But that file name should contain a hash part that is based on the modulemap path. But framewor

[PATCH] D105328: [Frontend] Only compile modules if not already finalized

bnbarham created this revision. bnbarham added reviewers: akyrtzi, arphaman. Herald added a subscriber: mgorny. bnbarham requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. It was possible to re-add a module to a shared in-memory module cache w