<div dir="ltr"><div dir="ltr" class="gmail_msg"><div dir="ltr" class="gmail_msg">I've spent a few days debugging/trying to understand the following, so I'm writing it all down in the hopes of getting it straight/clear and potentially getting some external perspective on what's going on, whether I've understood it correctly, and what might be a good way to solve it.<br class="gmail_msg"><br class="gmail_msg">Starting with this test case:<br class="gmail_msg"><font face="monospace" class="gmail_msg">foo.h:<br class="gmail_msg"></font><div class="gmail_msg"><font face="monospace" class="gmail_msg">  struct foo {};</font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg">  inline void e() { foo(); }<br class="gmail_msg">bar.h:<br class="gmail_msg"></font><div class="gmail_msg"><font face="monospace" class="gmail_msg">  #include "foo.h"</font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg">  template <typename T></font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg">  foo bar(foo &f) {</font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg">    return f;</font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg">  }</font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg">  void z() { (void)&bar<int>; }<br class="gmail_msg"></font><br class="gmail_msg">Build each into a separate module, run modular codegen on bar.pcm:<br class="gmail_msg"><div style="font-family:monospace" class="gmail_msg"><div class="gmail_msg">  clang-tot -cc1 -fmodules-codegen -xc++ -emit-module -fmodules \<br class="gmail_msg">    -fmodule-name=foo foo.cppmap -o foo.pcm</div><div class="gmail_msg">  clang-tot -cc1 -fmodules-codegen -xc++ -emit-module -fmodules \<br class="gmail_msg">    -fmodule-name=bar bar.cppmap -o bar.pcm -fmodule-file=foo.pcm</div><div class="gmail_msg">  clang-tot -c bar.pcm -o bar.o<br class="gmail_msg"><br class="gmail_msg"></div></div><div class="gmail_msg">(I haven't fully understood why the use of 'foo' in foo.h is necessary, nor why 'bar' needs to be a template - those might provide some further insight about how this should/could work)</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">This patch makes the failure a bit more visible/immediate:<br class="gmail_msg"><div class="gmail_msg"><font face="monospace" class="gmail_msg">  diff --git lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTReaderStmt.cpp</font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg">  index b4718367d4..1fb48e9560 100644</font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg">  @@ -3903,7 +3904,9 @@ Stmt *ASTReader::ReadStmtFromStream(ModuleFile &F) {</font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg">       ++NumStatementsRead;</font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg"> </font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg">       if (S && !IsStmtReference) {</font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg">  +      auto X = Cursor.GetCurrentBitNo();</font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg">         Reader.Visit(S);</font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg">  +      assert(X == Cursor.GetCurrentBitNo() && "Narf");</font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg">         StmtEntries[Cursor.GetCurrentBitNo()] = S;</font></div><div class="gmail_msg"><font face="monospace" class="gmail_msg">       }</font></div><div class="gmail_msg"> </div><div class="gmail_msg">This is layered on top of the modular codegen rewrite/refactor (to use a bit on function definitions, instead of to imply modular codegen from the Module object) and some other fixes. I'll include the full patch I'm working with.</div><br class="gmail_msg">The sequence of relevant steps (as best as I can figure)<br class="gmail_msg"><ol class="gmail_msg"><li class="gmail_msg">bar() body deserialization begins</li><li class="gmail_msg">bar module's DeclCursor is used, jumping to the start of the bar()'s definition</li><li class="gmail_msg">clang::ASTReader::ReadStmtFromStream iterates through Stmts in bar()<br class="gmail_msg">no SavedStreamPosition nor Deserializing object here</li><li class="gmail_msg">deserializing the EXPR_CXX_CONSTRUCT gets interesting:</li><li class="gmail_msg">eventually involves deserializing foo(const foo&)</li><li class="gmail_msg">foo module's DeclCursor is used</li><li class="gmail_msg">foo module's DeclCursor is saved/preserved with a SavedStreamPosition</li><li class="gmail_msg">a ExternalASTSource::Deserializing context is started</li><li class="gmail_msg">the definition of foo(const foo&) (in the bar module) is deserialized</li><li class="gmail_msg">this definition is 'interesting' and added to the ASTReader's InterestingDecls</li><li class="gmail_msg">The end of the Deserializing context (8) is reached</li><ol class="gmail_msg"><li class="gmail_msg">In the non-modular-codegen case, DeclMustBeEmitted is not true for foo(const foo&) and it is shelved for lazy emission, end of story</li></ol><li class="gmail_msg">In the modular-codegen case, foo(const foo&) in the bar module must be emitted (all inline functions (implicitly or explicitly) defined in a module are emitted weak, etc)<br class="gmail_msg"></li><li class="gmail_msg">So foo(const foo&) is non-lazily deserialized and emitted</li><li class="gmail_msg">The bar module's DeclCursor is used for this, unprotected</li><li class="gmail_msg">execution eventually gets back to the deserialization of bar() - and the DeclCursor it's using is out of position -> badness.</li></ol><div class="gmail_msg">Maybe there's a better way to provide this timeline, I'm not sure - hopefully it makes sense.<br class="gmail_msg"><br class="gmail_msg">Essentially the way ASTReader::ReadDeclRecord (started in (5) that contains the SavedStreamPosition and Deserializing context) seems to assume that the only things that will be non-lazily deserialized will come from the same module. Modular codegen breaks that invariant at the moment (well, with the patch provided).<br><br>I did try SavedStreamPosition-protecting the call from 3->4 (specifically the "Visit(Expr)" call in ReadStmtFromStream) though still hit some crashes. Maybe that's the right path to go down still, but need to do more?<br><br>When I tried to copy the DeclCursor in ReadStmtFromStream that actually broke things pretty significantly (compile errors on valid code even without modular codegen enabled). But I don't know much about the cursors - evidently more than only an efficiency concern, I guess the non-offset state in the copy of the cursor changes and so the original DeclCursor isn't updated, etc. (I wonder about splitting these cursors into a shared state + offset, share the state with shared_ptr and make it cheap to copy the actual offset state around so there's less reason to risk these sharing situations & all the SavedStreamPosition protection that requires).<br><br>Long & short of it: What should I do here? What're the likely goals I should be trying to move the code towards, if any?<br><br>- Dave</div></div><div style="font-family:monospace" class="gmail_msg"><span style="font-family:sans-serif" class="gmail_msg"><br class="gmail_msg"></span></div></div></div></div></div></div>