<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Aug 5, 2014, at 1:08 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" class="">richard@metafoo.co.uk</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote">On Mon, Aug 4, 2014 at 9:03 PM, Ben Langmuir <span dir="ltr" class=""><<a href="mailto:blangmuir@apple.com" target="_blank" class="">blangmuir@apple.com</a>></span> wrote:<br class="">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""><div class="h5"><br class=""><div class=""><blockquote type="cite" class=""><div class="">On Aug 4, 2014, at 5:26 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank" class="">richard@metafoo.co.uk</a>> wrote:</div>
<br class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote">On Mon, Aug 4, 2014 at 3:03 PM, Ben Langmuir <span dir="ltr" class=""><<a href="mailto:blangmuir@apple.com" target="_blank" class="">blangmuir@apple.com</a>></span> wrote:<br class="">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Richard,<br class="">
<br class="">
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.</blockquote>
<div class=""><br class=""></div><div class="">Hmm, thinking more about the specific problem here, I wonder if even the approaches we've discussed so far are not enough. In particular, suppose we hit this diagnostic:</div><div class=""><br class=""></div><div class="">Diag(...) << D->somethingThatTriggersDeserialization()</div>
<div class=""><br class=""></div><div class="">In this case, we'll again assert when deserialization issues a diagnostic with another diagnostic in flight. Some quick grepping was unable to find such a case, but I feel uneasy about this possibility; this seems like a time bomb. Maybe ASTReader should buffer all of its diagnostics, and emit them at some known-safe point in time, such as end of TU. What do you think?</div>
</div></div></div>
</div></blockquote></div><br class=""></div></div><div class="">It looks like a lot of ASTReader is sort of doing that already by using the 1-item buffer from Diags.setDelayedDiagnostic() and just discarding any diagnostics after the first one. The “safe point” in this case is immediately when there is no in-flight diag, or whenever the in-flight diagnostic is emitted if there is.</div>
</div></blockquote><div class=""><br class=""></div><div class="">This is an incorrect approach in the general case: it may emit diagnostics between an error or warning and its notes. It's fine for fatal errors, though.</div></div></div></div></div></blockquote><div><br class=""></div>Ah, good point.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""><br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word" class=""><div class="">That being said, I don’t know that we necessarily *want* non-fatal errors to be emitted if they are encountered while a diagnostic is in flight.</div></div></blockquote><div class=""><br class=""></div>
<div class="">I'm certain we do. It doesn't make sense to suppress an error from the serialization system merely because we first detected it while assembling a warning message, say.</div></div></div></div></div></blockquote><div><br class=""></div><div>Okay.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class="">I think there are a few reasonable ways we can produce diagnostics from the serialization system:</div>
<div class=""><br class=""></div><div class="">1) Buffer the diagnostic and its notes, and produce it when we're at a point where we know we're not mid-diagnostic. (This doesn't need to be end of TU, we could also insert safe points at various locations in the parser, or immediately before we begin emission of any non-note diagnostic.)</div></div></div></div></div></blockquote><div><br class=""></div><div>It feels really messy to have to spread this outside the DiagnosticsEngine and ASTReader. That said, this feels like the best idea we have so far.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote">
<div class=""><br class=""></div><div class="">2) Set aside the current diagnostic, if there is one, produce the serialization diagnostic, then restore the set-aside data once we're done. (If the diagnostic we set aside is a note, inform the diagnostics system that it should suppress that and any future notes for this diagnostic.) This may interleave a serialization diagnostic inside another diagnostic, so we should probably only consider this for diagnostics that are so important that they should not be delayed (that is, because we know future diagnostics will be hopeless), and it should be marked as DefaultFatal so we suppress those future diagnostics.</div></div></div></div></div></blockquote><div><br class=""></div><div>Is this safe in general? We don’t know the state of the partial diagnostic we would need to move aside.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote">
<div class=""><br class=""></div><div class="">3) Use the DelayedDiagnostic system; this has the same restrictions as option 2 but gives a slightly different (arguably worse) diagnostic order.</div><div class=""><br class=""></div><div class="">Maybe there are other approaches that work out OK, but it's not acceptable to just drop diagnostics.</div></div></div></div></div></blockquote></div><br class=""><div class="">A quick-and-dirty fix would be to upgrade the deserialization diagnostic seen while a diagnostic is in-flight to fatal and use the existing 1-item buffer. That would be better than asserting for the time being. I don’t really have any strong opinion about </div></body></html>