[BatchMode] Fix interaction of bridging PCH and file specific diag consumer. #15452
Conversation
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
Good catch! Hm. I don't love the linear scan. Maybe we could check just the first file and assert that the rest are the same? |
|
@jrose-apple I guess; not terribly wedded to it but I will note that the existing code ( |
|
I'm sorry, I meant that instead of your call to |
graydon
force-pushed
the
batch-mode-pch-attach-diagnostic-multiplexing
branch
from
53cd405 to
3d76cd5
Compare
|
@jrose-apple pushed update with that change (along with fix to linux, which lacks |
lib/AST/DiagnosticConsumer.cpp
Outdated
| // can't find buffers for the inputs). | ||
| if (!(SubConsumers.empty() || | ||
| SM.getIDForBufferIdentifier(SubConsumers.begin()->first).hasValue())) { | ||
| assert(std::none_of(SubConsumers.begin(), SubConsumers.end(), |
There was a problem hiding this comment.
Nitpick: llvm::none_of makes this simpler!
lib/AST/DiagnosticConsumer.cpp
Outdated
| // than trying to build a nonsensical map (and actually crashing since we | ||
| // can't find buffers for the inputs). | ||
| if (!(SubConsumers.empty() || | ||
| SM.getIDForBufferIdentifier(SubConsumers.begin()->first).hasValue())) { |
There was a problem hiding this comment.
Honestly if SubConsumers is empty we're in trouble anyway. We may as well assert that so that we don't drop diagnostics on the floor. (This is good too because I find this if-condition hard to read.)
…nsumer. In batch mode, when using serialized diagnostics, we have a FileSpecificDiagnosticConsumer splitting diags out to separate .dia files (one per primary). This constructs a mapping from filenames to consumers when it's first given a diag. So far so good. It assumes, however, that there are source buffers to be found for each input file it knows the name of. This is true -- and useful to assert -- once the source buffers are set up. It's also a prerequisite for building the map it uses to direct diags. Unfortunately there's a small window between the consumer being built and the source manager getting buffers, and in this window we attach to a bridging PCH. If that attaching generates a diag of itself (say due to a bogus module map or such) then the diag consumer is asked to build its map without source buffers. The thing to do here is just to fall back to the "no mapping found" case; the only tricky part is identifying when we're in that window of time. I've chosen to use the approximating condition of "none of the file buffers exist at all".
graydon
force-pushed
the
batch-mode-pch-attach-diagnostic-multiplexing
branch
from
3d76cd5 to
689663d
Compare
|
@jrose-apple nits addressed (hopefully)! |
|
@swift-ci please test and merge |
|
@swift-ci please test |
|
@swift-ci please test and merge |
|
Build failed |
|
Build failed |
|
@swift-ci please smoke test OSX platform |
1 similar comment
|
@swift-ci please smoke test OSX platform |
|
@swift-ci please smoke test |
|
@swift-ci please test OSX platform |
|
@swift-ci please test |
|
Build failed |
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
In batch mode, when using serialized diagnostics, we have a
FileSpecificDiagnosticConsumer splitting diags out to separate .dia files (one
per primary). This constructs a mapping from filenames to consumers when it's
first given a diag. So far so good.
It assumes, however, that there are source buffers to be found for each input
file it knows the name of. This is true -- and useful to assert -- once the
source buffers are set up. It's also a prerequisite for building the map it
uses to direct diags.
Unfortunately there's a small window between the consumer being built and the
source manager getting buffers, and in this window we attach to a bridging PCH.
If that attaching generates a diag of itself (say due to a bogus module map or
such) then the diag consumer is asked to build its map without source buffers.
The thing to do here is just to fall back to the "no mapping found" case; the
only tricky part is identifying when we're in that window of time. I've chosen
to use the approximating condition of "none of the file buffers exist at all".