> 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] > <mailto:[email protected]>> wrote: > >> On Aug 4, 2014, at 5:26 PM, Richard Smith <[email protected] >> <mailto:[email protected]>> wrote: >> >> On Mon, Aug 4, 2014 at 3:03 PM, Ben Langmuir <[email protected] >> <mailto:[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. > > 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. > > 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
