<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Aug 4, 2014 at 3:03 PM, Ben Langmuir <span dir="ltr"><<a href="mailto:blangmuir@apple.com" target="_blank">blangmuir@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Richard,<br>
<br>
We have several lazy builtin Decls (for ObjC decls, va_list, etc.) that might get filled in when we desugar a type for the ODR diagnostics, and these may deserialize more content from a module when they lookup an IdentifierInfo, leading to trying to emit a diagnostic while a diagnostic is already in flight.  My patch fixes the specific issue I ran into with the ODR diagnostics, but it seems like there may be a more general problem that filling in these lazy builtins may not be safe during diagnostic printing.  Any thoughts?</blockquote>
<div><br></div><div>Your patch LGTM.</div><div><br></div><div>I don't think this is specific to filling in lazy builtins: any AST deserialization triggered by diagnoseOdrViolations seems to hit this issue. This used to be protected from this particular issue by occurring within a 'Deserializing' RAII region; I moved it out of that region because it depends indirectly on AST invariants being maintained, so it seemed better to keep it outside the 'danger zone' of finishPendingActions.</div>
<div><br></div><div>We could perhaps make this more robust by adding a flag indicating whether we're already doing diagnoseOdrViolations, and skipping it if it reentrantly triggers itself, but your change looks like it would have the same effect in practice.</div>
<div><br></div><div>FWIW, the PassInterestingDeclsToConsumer call in FinishedDeserializing has a similar, but worse, issue: it calls back into CodeGen, and may be triggered by CodeGen inspecting the AST.</div></div></div>
</div>