<div dir="ltr">I've talked to Richard (Smith). He's not certain as to why PCH causes there to be an external source for layouts. It seems that PCH would take one of two strategies, either don't store layouts in the PCH and recompute them at compile time (which I don't see why would impose an external layout) or serialize the layout in the PCH, in which case there's no need to call layout at all. To my understanding ExternalSource is related to DWARF. It's entirely possible that we shouldn't get getting the ExternalSource flag during PCH. Further investigation is required to figure out exactly what's wrong. Both Richard and I are reasonably busy, so I wouldn't expect to get to figuring it out for a while. If you know more about how PCH works (you've been using it) I'm more than happy to have a discussion about why ExternalSource is firing and if it should and if we should ignore it.<div>
<br></div><div>-Warren</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Feb 11, 2014 at 11:28 AM, Warren Hunt <span dir="ltr"><<a href="mailto:whunt@google.com" target="_blank">whunt@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I have concerns about why PCH uses external layout and how PCH and pragmas (such as pack) interact. Richard Smith is our local expert on clang's PCH but it out at a C++ committee meeting this week so he's not around to question. It may well be the case that PCH should not use external layouts and that should be fixed. If it's working for you w/o the guard you can continue doing so but I don't guarantee anything until I've had a detailed discussion about how external layouts and PCH are supposed to work. I'll get back to you as soon as I have a plan.<div>
<br></div><div>thanks and good luck,</div><div>-Warren</div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Feb 11, 2014 at 12:34 AM, Will Wilson <span dir="ltr"><<a href="mailto:will@indefiant.com" target="_blank">will@indefiant.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Warren,<div><br></div><div>I've been using PCH with clang on Windows quite heavily (although for custom AST analysis only) and have a bit of experience with clang serialization code. As far as I can tell everything you rely on should already be serialized - at least local tests on large Windows codebases don't reveal any issues with the !D->getASTContext().getExternalSource() conditional removed. Probably the best bet would be to add a few basic tests using a PCH and try removing the conditional. YMMV but I think you might be pleasantly surprised and find you have no work to do.</div>
<div><br></div><div>Thanks for the great work on it!</div><div>Will.</div></div><div class="gmail_extra"><div><div><br><br><div class="gmail_quote">On 11 February 2014 09:03, Warren Hunt <span dir="ltr"><<a href="mailto:whunt@google.com" target="_blank">whunt@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">There is no code in the MS Record Layout builder to handle external sources so the behavior is unlikely to be correct in the case of an external source (hence the guard). I don't know anything about how PCH works or interacts with clang on windows. I'll try to get a better answer for you but in the time being I'd suggest not using PCH and clang on windows in combination.<span><font color="#888888"><div>
<br></div><div>-Warren</div></font></span></div><div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Feb 6, 2014 at 4:23 PM, Will Wilson <span dir="ltr"><<a href="mailto:will@indefiant.com" target="_blank">will@indefiant.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Warren,<div><br></div><div>I've just been hunting a bug in MS mode where records were being given different layouts depending on if the TU was built with a PCH or not. In the end I tracked it down to <span>ASTContext</span><span>::</span>getASTRecordLayout() introduced in r192494:</div>
<div><br></div><div><div><div> if (isMsLayout(D) && !D->getASTContext().getExternalSource()) {</div></div><div> NewEntry = BuildMicrosoftASTRecordLayout(D);</div></div><div><br></div><div>Removing the "&& !D->getASTContext().getExternalSource()" restored the expected behaviour. Is this check still needed?</div>
<div><br></div><div>Cheers,</div><div>Will.</div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br><br clear="all"><div><br></div></div></div><span><font color="#888888">-- <br><div dir="ltr"><div><span style="color:rgb(68,68,68);font-family:Arial,Helvetica,sans-serif"><b>Indefiant Ltd.</b></span></div>
</div>
</font></span></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>