[Dependency Scanning] Configure a serialized diagnostics consumer for in-memory scans #84191
Conversation
|
@swift-ci test |
artemcm
force-pushed
the
InMemoryDepScanSerializedDiagnostics
branch
from
e8da4b3 to
116d170
Compare
|
@swift-ci test |
cachemeifyoucan
left a comment
cachemeifyoucan
left a comment
There was a problem hiding this comment.
LGTM function wise. Some comments inline.
There was a problem hiding this comment.
I don't think you need to type-erase to the base type. This can just be ThreadSafeSerializedDiagnosticConsumer
There was a problem hiding this comment.
I only did so because the existing SerializedDiagnosticConsumer infrastructure, and therefore also my extension, are both defined in SerializedDiagnosticConsumer.cpp implementation file. We could hoist them up to be in a header, but I wanted to match the existing pattern.
There was a problem hiding this comment.
I feel like ThreadSafeDiagnosticConsumer can be together with DiagnosticConsumer and we can just use this type here.
There was a problem hiding this comment.
I don't understand this comment, could you please be more specific?
ThreadSafeSerializedDiagnosticConsumer inherits from SerializedDiagnosticConsumer. The latter is implementation-only (defined in a .cpp). I modeled the former to match. What does "can be together with DiagnosticConsumer " mean?
There was a problem hiding this comment.
I mean ThreadSafeDiagnosticConsumer that inherits from DiagnosticConsumer. We only need to make sure it is a ThreadSafeDiagnosticConsumer so we can add to scan instance. I don't see any upcast is needed to add here.
There was a problem hiding this comment.
Are you referring to ThreadSafeDiagnosticCollector? Are you suggesting that ThreadSafeSerializedDiagnosticConsumer inherit both ThreadSafeDiagnosticCollector and SerializedDiagnosticConsumer?
There was a problem hiding this comment.
Hmm, ok, I don't want to have a diamond here. Maybe I will model it as ThreadSafeDiagnosticConsumer with an internal SerializedDiagnosticConsumer, but you can't finish processing like that.
I will let you decide if you want to do it this way. Otherwise LGTM.
…consumer for in-memory scans This change introduces a thread-safe version of the 'SerializedDiagnosticConsumer' and refactors scanning compilation instance creation code to ensure this consumer gets added when the scanner query configuration command-line includes '-serialized-diagnostics-path' option.
artemcm
force-pushed
the
InMemoryDepScanSerializedDiagnostics
branch
from
116d170 to
b6760e7
Compare
|
@swift-ci smoke test |
|
@swift-ci smoke test |
There was a problem hiding this comment.
Hmm, ok, I don't want to have a diamond here. Maybe I will model it as ThreadSafeDiagnosticConsumer with an internal SerializedDiagnosticConsumer, but you can't finish processing like that.
I will let you decide if you want to do it this way. Otherwise LGTM.
2 participants
This change introduces a thread-safe version of the
SerializedDiagnosticConsumerand refactors scanning compilation instance creation code to ensure this consumer gets added when the scanner query configuration command-line includes '-serialized-diagnostics-path' option.This change ties creation of diagnostic consumers used with in-memory scanning to the same place where the scanner instance gets created, and ties their lifetime to the Scanner Query Context, generated in
initCompilerInstanceForScan.This change also adds a new-driver-only flag
-dependency-scan-serialize-diagnostics-pathto allow the driver to configure scanner-only serialized diagnostic output.