Add support for parseable-output straight from the frontend #34417

artemcm · 2020-10-23T23:22:25Z

This PR adds initial support for emitting parseable-output JSON directly from swift-frontend.
"began" and "finished" messages are currently supported.

The existing parseable-output library used by the Driver is sunk into Basic and extended with entry-points that accept a CompilerInvocation of a swift-frontend, in addition to existing entry-points used by the Driver that accept a Job.
A new diagnostic consumer is added in order to capture all produced diagnostics during the frontend's execution, mirroring what the printing consumer would do, except simply accumulating diagnostics, on a per-primary-input basis.
For batch jobs, "began" and "finished" messages are emitted on a per-primary-input basis, with quasi-PIDs assigned to the messages of jobs we are "faking". The same scheme as used by the driver is employed to generate quasi-PIDs: start at -1000 and decrement for each next primary input. Non-batch jobs get a real OS PID.

Note: The code in ParseableOutput.h and ParseableOutput.cpp with a Job-based interface is not new. I tried moving these files carefully to preserve history, but enough stuff seems to have changed to not get nice diffs here.

include/swift/Basic/ParseableOutput.h

artemcm · 2020-10-29T16:35:33Z

@swift-ci Please Test Source Compatibility

artemcm · 2020-10-30T15:18:05Z

@swift-ci Please Smoke Test

artemcm · 2020-10-30T17:16:10Z

@swift-ci Please Smoke Test

artemcm · 2020-11-02T16:27:29Z

@swift-ci Please Smoke Test

artemcm · 2020-11-02T18:09:04Z

@swift-ci Please Smoke Test Windows platform

artemcm · 2020-11-02T19:41:48Z

@swift-ci Please Smoke Test Windows platform

artemcm · 2020-11-02T21:02:24Z

@swift-ci Please Smoke Test Windows platform

artemcm · 2020-11-03T17:30:24Z

@swift-ci Please Smoke Test

artemcm · 2020-11-03T19:24:32Z

@swift-ci Please Smoke Test

artemcm · 2020-11-04T00:00:09Z

@swift-ci Please Smoke Test Linux Platform

artemcm · 2020-11-04T16:46:05Z

@swift-ci Please Smoke Test

artemcm · 2020-11-04T18:38:08Z

@swift-ci Please smoke Test

artemcm · 2020-11-04T20:57:44Z

@swift-ci Please Test

artemcm · 2020-11-04T23:03:44Z

@swift-ci Please Test Source Compatibility

include/swift/Basic/SupplementaryOutputPaths.h

include/swift/Frontend/FrontendOptions.h

beccadax · 2020-11-04T23:48:30Z

include/swift/Basic/ParseableOutput.h

+#include "swift/Basic/LLVM.h"
+#include "swift/Basic/TaskQueue.h"
+#include "swift/Driver/Job.h"
+#include "swift/Frontend/Frontend.h"


Including Driver and Frontend headers from Basic is a layering violation. When we have to violate layering, we usually try to use forward declarations in the header and only import in the implementation, but it'd be better to avoid it entirely.

It looks like what's happening here is that you're trying to create equivalent entry points that take a driver::Job and a CompilerInvocation and share their implementation behind the scenes. Instead, I would create a common-currency type that captures the information about a job that parseable_output cares about, and then write conversion functions in Driver and Frontend respectively to construct this type from driver::Job and CompilerInvocation. (Alternatively, have a superclass in Basic that both of these classes subclass.) That will separate the components in Basic more cleanly from the details of the driver and frontend.

Thank you for pointing this out.
The implementation's Message types already lended themselves pretty nicely to being separated in this fashion with some core basics being kept in a Basic library. The specific portions have now been split into their new homes in Driver and Frontend.

include/swift/Frontend/FrontendInputsAndOutputs.h

beccadax · 2020-11-05T02:00:20Z

lib/Basic/ParseableOutput.cpp

+      Outputs.push_back(OutputPair(file_types::lookupTypeForExtension(
+                                       llvm::sys::path::extension(output)),
+                                   output));


I'm not sure it's a good idea to use lookupTypeForExtension() like this. A build system could pass filenames with no extension or the wrong extension; if they do, we should still treat these correctly, using a file type based on which field in the supplementary output path field we found the path in. (This probably means forEachSetOutput would pass the file type to the lambda.)

This is a great observation, thank you. We do indeed have all the information we need to determines this based on action and known file type instead of inferring it. I modified forAllOutputPaths to pass along the filetype.

lib/Basic/ParseableOutput.cpp

beccadax · 2020-11-05T02:07:02Z

lib/FrontendTool/FrontendTool.cpp

+    // If we got this far, we need to suppress the output of the
+    // PrintingDiagnosticConsumer to ensure that only the parseable-output
+    // is emitted
+    PDC.setSuppressOutput(true);


Just checking: We're disabling the PrintingDiagnosticConsumer instead of never adding it in the first place because we need the PDC while we're parsing arguments, including the argument that sets this option?

Exactly. In fact, it seems that this is why setSuppressOutput exists in the first place.

lib/Frontend/FrontendOptions.cpp

lib/Frontend/FrontendInputsAndOutputs.cpp

artemcm · 2020-11-06T00:00:25Z

@brentdax Thank you for such thorough and insightful feedback.

artemcm · 2020-11-06T17:01:39Z

@swift-ci please test

swift-ci · 2020-11-06T17:46:44Z

Build failed
Swift Test Linux Platform
Git Sha - ba2f677daed0578ad9dd2efda70199f21294cfdf

swift-ci · 2020-11-06T18:47:09Z

Build failed
Swift Test OS X Platform
Git Sha - ba2f677daed0578ad9dd2efda70199f21294cfdf

artemcm · 2020-11-30T21:29:50Z

@swift-ci please test

artemcm · 2020-11-30T22:27:24Z

@swift-ci please test Windows platform

artemcm · 2020-12-01T17:26:53Z

@swift-ci please test

swift-ci · 2020-12-01T18:46:54Z

Build failed
Swift Test Linux Platform
Git Sha - 94e56953b3ccf3c5c9fc52a33706ec5291bfad81

artemcm · 2020-12-02T18:02:53Z

@swift-ci please test

swift-ci · 2020-12-02T19:19:30Z

Build failed
Swift Test Linux Platform
Git Sha - ed242439584a6ecd6f1f3be2839024e43fbab437

artemcm · 2020-12-02T19:32:14Z

@swift-ci please test

artemcm · 2020-12-02T23:04:35Z

@brentdax ping. The feedback above has been addressed, I believe.

…-frontend

…primary messages for batched jobs Inside swift-frontend

Starting at a crude -1000, each invocation primary input will get its own unique quasi-Pid. Invocations with only one primary (non-batch) will get a real OS Pid. The selection of the constant starting point matches what the driver does when outputting its parseable output.

…d Parseable-Output mode

artemcm · 2020-12-12T00:01:40Z

@swift-ci please test

swift-ci · 2020-12-12T02:47:42Z

Build failed
Swift Test OS X Platform
Git Sha - 234c49ce0d3a86f1d4042b0c8f6374268bedb81c

artemcm · 2020-12-14T17:42:16Z

@swift-ci please test macOS platform

beccadax

Much better! I have a couple nits to pick, but this has the right shape now.

beccadax · 2020-12-14T19:25:06Z

include/swift/Basic/ParseableOutput.h

+struct DetailedMessagePayload {
+  std::string Executable;
+  SmallVector<std::string, 16> Arguments;
+  std::string CommandLine;
+  SmallVector<CommandInput, 4> Inputs;
+  SmallVector<OutputPair, 8> Outputs;
+};


This type ought to have a doc comment.

I'd also like you to think carefully about its name. Yes, from the perspective of the emit*Message() functions, this is the payload of the DetailedMessage you're emitting. But that's an implementation detail of this subsystem.

A better type name would come from asking, "what is this type to the clients of this header"? I might call it something like a "job description" or "process info" or maybe a "quasi-invocation" or something. I'll leave the exact decision to you.

Thank you for the insight, I fully agree.
I have renamed the type and it now has a doc to describe its purpose.

…tend Introducing new entry-points that can be used from both Driver and Frontend clients, using an intermediary new type: `DetailedMessagePayload`, when needed.

artemcm · 2020-12-14T19:42:16Z

@swift-ci please test

artemcm · 2020-12-16T17:30:39Z

@swift-ci please test

artemcm requested review from beccadax and DougGregor October 23, 2020 23:50

beccadax reviewed Oct 24, 2020

View reviewed changes

include/swift/Basic/ParseableOutput.h Show resolved Hide resolved

artemcm force-pushed the FrontendParseableOutput branch from b6a53c7 to 63c51dd Compare November 3, 2020 17:30

artemcm force-pushed the FrontendParseableOutput branch from 63c51dd to 24ca496 Compare November 3, 2020 19:24

artemcm force-pushed the FrontendParseableOutput branch from 24ca496 to e9a5203 Compare November 4, 2020 16:17

artemcm force-pushed the FrontendParseableOutput branch from e9a5203 to 477c0a8 Compare November 4, 2020 20:52

beccadax requested changes Nov 5, 2020

View reviewed changes

artemcm force-pushed the FrontendParseableOutput branch from 5d34735 to c9d21e0 Compare November 5, 2020 19:49

beccadax reviewed Nov 5, 2020

View reviewed changes

lib/Frontend/FrontendOptions.cpp Outdated Show resolved Hide resolved

beccadax reviewed Nov 5, 2020

View reviewed changes

lib/Frontend/FrontendInputsAndOutputs.cpp Outdated Show resolved Hide resolved

artemcm force-pushed the FrontendParseableOutput branch from ba2f677 to 789504e Compare November 6, 2020 19:25

artemcm force-pushed the FrontendParseableOutput branch from ed24243 to 709b55c Compare December 2, 2020 19:31

artemcm added 10 commits December 11, 2020 16:01

Sink the parseable-output library into Basic from Driver

b827875

Add support for emitting parseable-outpuit "began" message from swift…

f09ab5c

…-frontend

Add support for emitting parseable-output "finished" message and per-…

23e374c

…primary messages for batched jobs Inside swift-frontend

Suppress the output of the PrintingDiagnosticConsumer when in Fronten…

86e039f

…d Parseable-Output mode

Add frontend parseable output tests

bc58ed4

Match diagnostic output formatting from the PrintDiagnosticConsumer

e5d86d4

Add host-specific include for getpid() in FrontendTool.

269f1b9

Remove unnecessary platform-specific checks from tests

21c4e3a

Remove unnecessary XFAILs

222b153

artemcm force-pushed the FrontendParseableOutput branch from 709b55c to 234c49c Compare December 12, 2020 00:01

beccadax requested changes Dec 14, 2020

View reviewed changes

Refactor Basic/Parseable-Output to remove dependency on Driver & Fron…

915186a

…tend Introducing new entry-points that can be used from both Driver and Frontend clients, using an intermediary new type: `DetailedMessagePayload`, when needed.

artemcm force-pushed the FrontendParseableOutput branch from 234c49c to 915186a Compare December 14, 2020 19:36

artemcm merged commit 47da04f into swiftlang:main Dec 16, 2020

artemcm mentioned this pull request Jan 22, 2021

[SR-13351] Support parsable output from the Swift frontend #55791

Closed

Add support for parseable-output straight from the frontend #34417

Add support for parseable-output straight from the frontend #34417

Uh oh!

Conversation

artemcm commented Oct 23, 2020

Uh oh!

Uh oh!

artemcm commented Oct 29, 2020

Uh oh!

artemcm commented Oct 30, 2020

Uh oh!

artemcm commented Oct 30, 2020

Uh oh!

artemcm commented Nov 2, 2020

Uh oh!

artemcm commented Nov 2, 2020

Uh oh!

artemcm commented Nov 2, 2020

Uh oh!

artemcm commented Nov 2, 2020

Uh oh!

artemcm commented Nov 3, 2020

Uh oh!

artemcm commented Nov 3, 2020

Uh oh!

artemcm commented Nov 4, 2020

Uh oh!

artemcm commented Nov 4, 2020

Uh oh!

artemcm commented Nov 4, 2020

Uh oh!

artemcm commented Nov 4, 2020

Uh oh!

artemcm commented Nov 4, 2020

Uh oh!

Uh oh!

Uh oh!

beccadax Nov 4, 2020

Choose a reason for hiding this comment

Uh oh!

artemcm Nov 5, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

beccadax Nov 5, 2020

Choose a reason for hiding this comment

Uh oh!

artemcm Nov 5, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

beccadax Nov 5, 2020

Choose a reason for hiding this comment

Uh oh!

artemcm Nov 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

artemcm commented Nov 6, 2020

Uh oh!

artemcm commented Nov 6, 2020

Uh oh!

swift-ci commented Nov 6, 2020

Uh oh!

swift-ci commented Nov 6, 2020

Uh oh!

artemcm commented Nov 30, 2020

Uh oh!

artemcm commented Nov 30, 2020

Uh oh!

artemcm commented Dec 1, 2020

Uh oh!

swift-ci commented Dec 1, 2020

Uh oh!

artemcm commented Dec 2, 2020

Uh oh!

swift-ci commented Dec 2, 2020

Uh oh!

artemcm Nov 5, 2020 •

edited

Loading