[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

hctim added a comment. Looks like this broke the ASan buildbot (and may have been missed because the bot was already red at the time): https://lab.llvm.org/buildbot/#/builders/5/builds/24588 Instructions on how to repro the bot are here: https://github.com/google/sanitizers/wiki/SanitizerBotRe

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

junaire added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:1500 + +assert(WeakRefReferences.empty() && + "Not all WeakRefRefs have been applied"); v.g.vassilev wrote: > It seems that we forgot to move the `WeakRefReferences`? Thank

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

v.g.vassilev added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:1500 + +assert(WeakRefReferences.empty() && + "Not all WeakRefRefs have been applied"); It seems that we forgot to move the `WeakRefReferences`? Repository: rG LLV

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

This revision was automatically updated to reflect the committed changes. Closed by commit rGb8f945971581: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder (authored by junaire). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126781/

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

junaire added a comment. In D126781#3570147 , @rjmccall wrote: > Okay, LGTM Thanks for the review ☺️ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126781/new/ https://reviews.llvm.org/D126781 ___

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Okay, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126781/new/ https://reviews.llvm.org/D126781 _

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

junaire updated this revision to Diff 435460. junaire added a comment. OK, then we'd better to roll the condition back. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126781/new/ https://reviews.llvm.org/D126781 Files: clang/lib/CodeGen/CodeGenMo

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

rjmccall added a comment. In D126781#3565173 , @junaire wrote: > I believe that `OldBuilder` is always valid in the normal cases, so > remove the if condition and replace it with an assertion. Is `StartModule` not called for the first time we start emitt

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

junaire updated this revision to Diff 435004. junaire added a comment. I believe that `OldBuilder` is always valid in the normal cases, so remove the if condition and replace it with an assertion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126781

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

junaire updated this revision to Diff 435000. junaire added a comment. Address @rjmccall 's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126781/new/ https://reviews.llvm.org/D126781 Files: clang/lib/CodeGen/CodeGenModule.h clang/lib/

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

rjmccall added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:1482 + + struct KeepLazyEmiitedSymRAII { +std::unique_ptr OldBuilder; rjmccall wrote: > junaire wrote: > > rjmccall wrote: > > > I think a RAII object is an odd way to express this

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

junaire added inline comments. Comment at: clang/lib/CodeGen/ModuleBuilder.cpp:163 for (auto &&Lib : CodeGenOpts.DependentLibraries) Builder->AddDependentLib(Lib); I left I may doing something wrong here, after invoking `Initialize` we set `Bui

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

junaire updated this revision to Diff 434714. junaire added a comment. Try to address @rjmccall 's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126781/new/ https://reviews.llvm.org/D126781 Files: clang/lib/CodeGen/CodeGenModule.h cla

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

rjmccall added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:1482 + + struct KeepLazyEmiitedSymRAII { +std::unique_ptr OldBuilder; junaire wrote: > rjmccall wrote: > > I think a RAII object is an odd way to express this API; there's not real

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

junaire added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:1482 + + struct KeepLazyEmiitedSymRAII { +std::unique_ptr OldBuilder; rjmccall wrote: > I think a RAII object is an odd way to express this API; there's not really a > natural reas

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

rjmccall added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:1482 + + struct KeepLazyEmiitedSymRAII { +std::unique_ptr OldBuilder; I think a RAII object is an odd way to express this API; there's not really a natural reason you would scope

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

junaire updated this revision to Diff 434479. junaire added a comment. Remove extra brackets Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126781/new/ https://reviews.llvm.org/D126781 Files: clang/lib/CodeGen/CodeGenModule.h clang/lib/CodeGen/

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

junaire added inline comments. Comment at: clang/lib/CodeGen/ModuleBuilder.cpp:138 + { +CodeGenModule::KeepLazyEmiitedSymRAII RAIIKeeper(Builder); +M.reset(new llvm::Module(ExpandModuleName(ModuleName, CodeGenOpts), C)); v.g.vassilev wrote:

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

junaire updated this revision to Diff 434474. junaire marked an inline comment as done. junaire added a comment. fix a typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126781/new/ https://reviews.llvm.org/D126781 Files: clang/lib/CodeGen/CodeG

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

junaire updated this revision to Diff 434473. junaire added a comment. friend class comes to the rescue! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126781/new/ https://reviews.llvm.org/D126781 Files: clang/lib/CodeGen/CodeGenModule.h clang/

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

v.g.vassilev added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:1506 + + struct KeepLazyEmiitedSymRAII { +std::unique_ptr OldBuilder; Would it make sense to make the RAII a friend to avoid adding the accessors of various structures?

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

junaire updated this revision to Diff 434251. junaire added a comment. I think std::swap makes the logic more clear Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126781/new/ https://reviews.llvm.org/D126781 Files: clang/lib/CodeGen/CodeGenModule

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

junaire updated this revision to Diff 434238. junaire added a comment. Fix the broken build Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126781/new/ https://reviews.llvm.org/D126781 Files: clang/lib/CodeGen/CodeGenModule.h clang/lib/CodeGen/M

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

junaire updated this revision to Diff 434236. junaire retitled this revision from " [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder" to "[CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder". junaire edited the summary of this revision. junaire added a comment. A

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

rsmith added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:1480-1482 + llvm::SmallPtrSetImpl &getEmittedDeferredDecls() { +return EmittedModuleInitializers; + } This function name doesn't match its implementation. Comment

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

v.g.vassilev added a comment. In D126781#3554845 , @rjmccall wrote: > Okay, I understand. So, first off, I wouldn't really call that a "weak" > symbol rather than, say, a lazily-emitted symbol; "weak" already has plenty > of different senses, and we sh