On Tue, Aug 5, 2014 at 2:51 PM, Ben Langmuir <[email protected]> wrote:
> > On Aug 5, 2014, at 1:08 PM, Richard Smith <[email protected]> wrote: > > On Mon, Aug 4, 2014 at 9:03 PM, Ben Langmuir <[email protected]> wrote: > >> >> On Aug 4, 2014, at 5:26 PM, Richard Smith <[email protected]> wrote: >> >> On Mon, Aug 4, 2014 at 3:03 PM, Ben Langmuir <[email protected]> wrote: >> >>> Hi Richard, >>> >>> 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. >> >> >> 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: >> >> Diag(...) << D->somethingThatTriggersDeserialization() >> >> 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? >> >> >> 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. >> > > 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. > > > Ah, good point. > > > 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. >> > > 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. > > > Okay. > > I think there are a few reasonable ways we can produce diagnostics from > the serialization system: > > 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.) > > > 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. > We can probably limit the safe points to just before a non-note diagnostic and end-of-TU, and avoid touching code outside DiagnosticsEngine and ASTReader that way. > 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. > > > Is this safe in general? We don’t know the state of the partial > diagnostic we would need to move aside. > I think the most risky part would be re-entering the diagnostic consumer when it's half way through rendering a diagnostic (in the case where rendering a type is what triggers deserialization). > 3) Use the DelayedDiagnostic system; this has the same restrictions as > option 2 but gives a slightly different (arguably worse) diagnostic order. > > Maybe there are other approaches that work out OK, but it's not acceptable > to just drop diagnostics. > > > 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 >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
