Skip to content

Requestify ExportedSourceFile parsing #66029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions include/swift/AST/ParseRequests.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,24 @@ class ParseSourceFileRequest
readDependencySource(const evaluator::DependencyRecorder &) const;
};

/// Parse the ExportedSourceFile for a given SourceFile.
class ExportedSourceFileRequest
: public SimpleRequest<ExportedSourceFileRequest,
void *(const SourceFile *),
RequestFlags::Cached> {
public:
using SimpleRequest::SimpleRequest;

private:
friend SimpleRequest;

void *evaluate(Evaluator &evaluator, const SourceFile *SF) const;

public:
// Cached.
bool isCached() const { return true; }
};

/// Parse the top-level items of a SourceFile.
class ParseTopLevelDeclsRequest
: public SimpleRequest<
Expand Down
3 changes: 3 additions & 0 deletions include/swift/AST/ParseTypeIDZone.def
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,6 @@ SWIFT_REQUEST(Parse, ParseSourceFileRequest,
SWIFT_REQUEST(Parse, ParseTopLevelDeclsRequest,
ArrayRef<Decl *>(SourceFile *), Cached,
NoLocationInfo)
SWIFT_REQUEST(Parse, ExportedSourceFileRequest,
void *(const SourceFile *), Cached,
NoLocationInfo)
16 changes: 9 additions & 7 deletions include/swift/AST/SourceFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,11 @@ class SourceFile final : public FileUnit {
/// files, as they get parsed multiple times.
SuppressWarnings = 1 << 4,

/// Whether to disable the Swift Parser ASTGen
/// e.g. in dependency scanning, where an AST is not needed.
DisableSwiftParserASTGen = 1 << 5,
/// Ensure that the SwiftSyntax tree round trips correctly.
RoundTrip = 1 << 5,

/// Validate the new SwiftSyntax parser diagnostics.
ValidateNewParserDiagnostics = 1 << 6,
};
using ParsingOptions = OptionSet<ParsingFlags>;

Expand Down Expand Up @@ -262,6 +264,10 @@ class SourceFile final : public FileUnit {
/// code for it. Note this method returns \c false in WMO.
bool isPrimary() const { return IsPrimary; }

/// Retrieve the \c ExportedSourceFile instance produced by ASTGen, which
/// includes the SourceFileSyntax node corresponding to this source file.
void *getExportedSourceFile() const;

/// The list of local type declarations in the source file.
llvm::SetVector<TypeDecl *> LocalTypeDecls;

Expand Down Expand Up @@ -334,10 +340,6 @@ class SourceFile final : public FileUnit {
/// this source file.
llvm::SmallVector<Located<StringRef>, 0> VirtualFilePaths;

/// The \c ExportedSourceFile instance produced by ASTGen, which includes
/// the SourceFileSyntax node corresponding to this source file.
void *exportedSourceFile = nullptr;

/// Returns information about the file paths used for diagnostics and magic
/// identifiers in this source file, including virtual filenames introduced by
/// \c #sourceLocation(file:) declarations.
Expand Down
9 changes: 9 additions & 0 deletions lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3687,6 +3687,10 @@ SourceFile::getDefaultParsingOptions(const LangOptions &langOpts) {
opts |= ParsingFlags::DisablePoundIfEvaluation;
if (langOpts.CollectParsedToken)
opts |= ParsingFlags::CollectParsedTokens;
if (langOpts.hasFeature(Feature::ParserRoundTrip))
opts |= ParsingFlags::RoundTrip;
if (langOpts.hasFeature(Feature::ParserValidation))
opts |= ParsingFlags::ValidateNewParserDiagnostics;
return opts;
}

Expand Down Expand Up @@ -3762,6 +3766,11 @@ ArrayRef<Decl *> SourceFile::getHoistedDecls() const {
return Hoisted;
}

void *SourceFile::getExportedSourceFile() const {
auto &eval = getASTContext().evaluator;
return evaluateOrDefault(eval, ExportedSourceFileRequest{this}, nullptr);
}

void SourceFile::addDeclWithRuntimeDiscoverableAttrs(ValueDecl *decl) {
assert(!decl->getRuntimeDiscoverableAttrs().empty());
DeclsWithRuntimeDiscoverableAttrs.insert(decl);
Expand Down
4 changes: 4 additions & 0 deletions lib/Basic/Statistic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -419,11 +419,14 @@ UnifiedStatsReporter::noteCurrentProcessExitStatus(int status) {

void
UnifiedStatsReporter::publishAlwaysOnStatsToLLVM() {
// NOTE: We do `Stat = 0` below to force LLVM to register the statistic,
// ensuring we print counters, even if 0.
if (FrontendCounters) {
auto &C = getFrontendCounters();
#define FRONTEND_STATISTIC(TY, NAME) \
do { \
static Statistic Stat = {#TY, #NAME, #NAME}; \
Stat = 0; \
Stat += (C).NAME; \
} while (0);
#include "swift/Basic/Statistics.def"
Expand All @@ -434,6 +437,7 @@ UnifiedStatsReporter::publishAlwaysOnStatsToLLVM() {
#define DRIVER_STATISTIC(NAME) \
do { \
static Statistic Stat = {"Driver", #NAME, #NAME}; \
Stat = 0; \
Stat += (C).NAME; \
} while (0);
#include "swift/Basic/Statistics.def"
Expand Down
36 changes: 21 additions & 15 deletions lib/Frontend/Frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1513,6 +1513,9 @@ void CompilerInstance::finishTypeChecking() {

SourceFile::ParsingOptions
CompilerInstance::getSourceFileParsingOptions(bool forPrimary) const {
using ActionType = FrontendOptions::ActionType;
using ParsingFlags = SourceFile::ParsingFlags;

const auto &frontendOpts = Invocation.getFrontendOptions();
const auto action = frontendOpts.RequestedAction;

Expand All @@ -1521,42 +1524,45 @@ CompilerInstance::getSourceFileParsingOptions(bool forPrimary) const {
// Generally in a parse-only invocation, we want to disable #if evaluation.
// However, there are a couple of modes where we need to know which clauses
// are active.
if (action != FrontendOptions::ActionType::EmitImportedModules &&
action != FrontendOptions::ActionType::ScanDependencies) {
opts |= SourceFile::ParsingFlags::DisablePoundIfEvaluation;
if (action != ActionType::EmitImportedModules &&
action != ActionType::ScanDependencies) {
opts |= ParsingFlags::DisablePoundIfEvaluation;
}

// If we need to dump the parse tree, disable delayed bodies as we want to
// show everything.
if (action == FrontendOptions::ActionType::DumpParse)
opts |= SourceFile::ParsingFlags::DisableDelayedBodies;
if (action == ActionType::DumpParse)
opts |= ParsingFlags::DisableDelayedBodies;
}

auto typeOpts = getASTContext().TypeCheckerOpts;
if (forPrimary || isWholeModuleCompilation()) {
const auto &typeOpts = getASTContext().TypeCheckerOpts;
const auto isEffectivelyPrimary = forPrimary || isWholeModuleCompilation();
if (isEffectivelyPrimary) {
// Disable delayed body parsing for primaries and in WMO, unless
// forcefully skipping function bodies
if (typeOpts.SkipFunctionBodies == FunctionBodySkipping::None)
opts |= SourceFile::ParsingFlags::DisableDelayedBodies;
opts |= ParsingFlags::DisableDelayedBodies;
} else {
// Suppress parse warnings for non-primaries, as they'll get parsed multiple
// times.
opts |= SourceFile::ParsingFlags::SuppressWarnings;
opts |= ParsingFlags::SuppressWarnings;
}

// Dependency scanning does not require an AST, so disable Swift Parser
// ASTGen parsing completely.
if (frontendOpts.RequestedAction ==
FrontendOptions::ActionType::ScanDependencies)
opts |= SourceFile::ParsingFlags::DisableSwiftParserASTGen;
// Turn off round-trip checking for secondary files, and for dependency
// scanning and IDE inspection.
if (!isEffectivelyPrimary || SourceMgr.hasIDEInspectionTargetBuffer() ||
frontendOpts.RequestedAction == ActionType::ScanDependencies) {
opts -= ParsingFlags::RoundTrip;
opts -= ParsingFlags::ValidateNewParserDiagnostics;
}

// Enable interface hash computation for primaries or emit-module-separately,
// but not in WMO, as it's only currently needed for incremental mode.
if (forPrimary ||
typeOpts.SkipFunctionBodies ==
FunctionBodySkipping::NonInlinableWithoutTypes ||
frontendOpts.ReuseFrontendForMultipleCompilations) {
opts |= SourceFile::ParsingFlags::EnableInterfaceHash;
opts |= ParsingFlags::EnableInterfaceHash;
}
return opts;
}
Expand Down
181 changes: 111 additions & 70 deletions lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,7 @@ extern "C" void swift_ASTGen_buildTopLevelASTNodes(void *sourceFile,
void Parser::parseTopLevelItems(SmallVectorImpl<ASTNode> &items) {
#if SWIFT_SWIFT_PARSER
Optional<DiagnosticTransaction> existingParsingTransaction;
if (!SF.getParsingOptions()
.contains(SourceFile::ParsingFlags::DisableSwiftParserASTGen))
parseSourceFileViaASTGen(items, existingParsingTransaction);
parseSourceFileViaASTGen(items, existingParsingTransaction);
#endif

// Prime the lexer.
Expand Down Expand Up @@ -261,92 +259,135 @@ void Parser::parseTopLevelItems(SmallVectorImpl<ASTNode> &items) {
}

#if SWIFT_SWIFT_PARSER
if (!SF.getParsingOptions().contains(
SourceFile::ParsingFlags::DisableSwiftParserASTGen)) {
if (existingParsingTransaction)
existingParsingTransaction->abort();

// Perform round-trip and/or validation checking.
if ((Context.LangOpts.hasFeature(Feature::ParserRoundTrip) ||
Context.LangOpts.hasFeature(Feature::ParserValidation)) &&
SF.exportedSourceFile &&
!SourceMgr.hasIDEInspectionTargetBuffer()) {
if (Context.LangOpts.hasFeature(Feature::ParserRoundTrip) &&
swift_ASTGen_roundTripCheck(SF.exportedSourceFile)) {
SourceLoc loc;
if (auto bufferID = SF.getBufferID()) {
loc = Context.SourceMgr.getLocForBufferStart(*bufferID);
}
diagnose(loc, diag::parser_round_trip_error);
} else if (Context.LangOpts.hasFeature(Feature::ParserValidation) &&
!Context.Diags.hadAnyError() &&
swift_ASTGen_emitParserDiagnostics(
&Context.Diags, SF.exportedSourceFile,
/*emitOnlyErrors=*/true,
/*downgradePlaceholderErrorsToWarnings=*/
Context.LangOpts.Playground ||
Context.LangOpts.WarnOnEditorPlaceholder)) {
// We might have emitted warnings in the C++ parser but no errors, in
// which case we still have `hadAnyError() == false`. To avoid emitting
// the same warnings from SwiftParser, only emit errors from SwiftParser
SourceLoc loc;
if (auto bufferID = SF.getBufferID()) {
if (existingParsingTransaction)
existingParsingTransaction->abort();

using ParsingFlags = SourceFile::ParsingFlags;
const auto parsingOpts = SF.getParsingOptions();

// If we don't need to validate anything, we're done.
if (!parsingOpts.contains(ParsingFlags::RoundTrip) &&
!parsingOpts.contains(ParsingFlags::ValidateNewParserDiagnostics)) {
return;
}

auto *exportedSourceFile = SF.getExportedSourceFile();
if (!exportedSourceFile)
return;

// Perform round-trip and/or validation checking.
if (parsingOpts.contains(ParsingFlags::RoundTrip) &&
swift_ASTGen_roundTripCheck(exportedSourceFile)) {
SourceLoc loc;
if (auto bufferID = SF.getBufferID()) {
loc = Context.SourceMgr.getLocForBufferStart(*bufferID);
}
diagnose(loc, diag::parser_round_trip_error);
return;
}
if (parsingOpts.contains(ParsingFlags::ValidateNewParserDiagnostics) &&
!Context.Diags.hadAnyError()) {
auto hadSyntaxError = swift_ASTGen_emitParserDiagnostics(
&Context.Diags, exportedSourceFile,
/*emitOnlyErrors=*/true,
/*downgradePlaceholderErrorsToWarnings=*/
Context.LangOpts.Playground ||
Context.LangOpts.WarnOnEditorPlaceholder);
if (hadSyntaxError) {
// We might have emitted warnings in the C++ parser but no errors, in
// which case we still have `hadAnyError() == false`. To avoid
// emitting the same warnings from SwiftParser, only emit errors from
// SwiftParser
SourceLoc loc;
if (auto bufferID = SF.getBufferID()) {
loc = Context.SourceMgr.getLocForBufferStart(*bufferID);
}
diagnose(loc, diag::parser_new_parser_errors);
}
diagnose(loc, diag::parser_new_parser_errors);
}
}
#endif
}

void *ExportedSourceFileRequest::evaluate(Evaluator &evaluator,
const SourceFile *SF) const {
#if SWIFT_SWIFT_PARSER
// The SwiftSyntax parser doesn't (yet?) handle SIL.
if (SF->Kind == SourceFileKind::SIL)
return nullptr;

auto &ctx = SF->getASTContext();
auto &SM = ctx.SourceMgr;

auto bufferID = SF->getBufferID();
if (!bufferID)
return nullptr;

StringRef contents = SM.extractText(SM.getRangeForBuffer(*bufferID));

// Parse the source file.
auto exportedSourceFile = swift_ASTGen_parseSourceFile(
contents.begin(), contents.size(),
SF->getParentModule()->getName().str().str().c_str(),
SF->getFilename().str().c_str());

ctx.addCleanup([exportedSourceFile] {
swift_ASTGen_destroySourceFile(exportedSourceFile);
});
return exportedSourceFile;
#else
return nullptr;
#endif
}

void
Parser::parseSourceFileViaASTGen(SmallVectorImpl<ASTNode> &items,
Optional<DiagnosticTransaction> &transaction,
bool suppressDiagnostics) {
#if SWIFT_SWIFT_PARSER
Optional<DiagnosticTransaction> existingParsingTransaction;
if (SF.Kind != SourceFileKind::SIL) {
StringRef contents =
SourceMgr.extractText(SourceMgr.getRangeForBuffer(L->getBufferID()));

// Parse the source file.
auto exportedSourceFile = swift_ASTGen_parseSourceFile(
contents.begin(), contents.size(),
SF.getParentModule()->getName().str().str().c_str(),
SF.getFilename().str().c_str());
SF.exportedSourceFile = exportedSourceFile;
Context.addCleanup([exportedSourceFile] {
swift_ASTGen_destroySourceFile(exportedSourceFile);
});
using ParsingFlags = SourceFile::ParsingFlags;
const auto parsingOpts = SF.getParsingOptions();
const auto &langOpts = Context.LangOpts;

// We only need to do parsing if we either have ASTGen enabled, or want the
// new parser diagnostics.
auto needToParse = [&]() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this to avoid negating the if's here 😅?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I originally wrote it as:

    if (!parsingOpts.contains(ParsingFlags::EnableASTGen) &&
        !(parsingOpts.contains(ParsingFlags::EnableASTGenDiagnostics) && !suppressDiagnostics))
      return;

and then I said to myself: "I'd hate to read that code if I ever saw it in the wild", and so rewrote it as this 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could split it across two if and just negate with an or instead:

if (!parsingOpts.contains(ParsingFlags::EnableASTGen)) {
  if (!parsingOpts.contains(ParsingFlags::EnableASTGenDiagnostics) || suppressDiagnostics)
    return
}

I don't have a strong preference. I think yours is probably clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I kind of also dislike that too

if (langOpts.hasFeature(Feature::ParserASTGen))
return true;
if (!suppressDiagnostics &&
langOpts.hasFeature(Feature::ParserDiagnostics)) {
return true;
}
return false;
}();
if (!needToParse)
return;

// If we're supposed to emit diagnostics from the parser, do so now.
if ((Context.LangOpts.hasFeature(Feature::ParserDiagnostics) ||
Context.LangOpts.hasFeature(Feature::ParserASTGen)) &&
!suppressDiagnostics &&
swift_ASTGen_emitParserDiagnostics(
&Context.Diags, SF.exportedSourceFile, /*emitOnlyErrors=*/false,
/*downgradePlaceholderErrorsToWarnings=*/
Context.LangOpts.Playground ||
Context.LangOpts.WarnOnEditorPlaceholder) &&
Context.Diags.hadAnyError() &&
!Context.LangOpts.hasFeature(Feature::ParserASTGen)) {
auto *exportedSourceFile = SF.getExportedSourceFile();
if (!exportedSourceFile)
return;

// If we're supposed to emit diagnostics from the parser, do so now.
if (!suppressDiagnostics) {
auto hadSyntaxError = swift_ASTGen_emitParserDiagnostics(
&Context.Diags, exportedSourceFile, /*emitOnlyErrors=*/false,
/*downgradePlaceholderErrorsToWarnings=*/langOpts.Playground ||
langOpts.WarnOnEditorPlaceholder);
if (hadSyntaxError && Context.Diags.hadAnyError() &&
!langOpts.hasFeature(Feature::ParserASTGen)) {
// Errors were emitted, and we're still using the C++ parser, so
// disable diagnostics from the C++ parser.
transaction.emplace(Context.Diags);
}
}

// If we want to do ASTGen, do so now.
if (Context.LangOpts.hasFeature(Feature::ParserASTGen)) {
swift_ASTGen_buildTopLevelASTNodes(
exportedSourceFile, CurDeclContext, &Context, &items, appendToVector);

// Spin the C++ parser to the end; we won't be using it.
while (!Tok.is(tok::eof)) {
consumeToken();
}
// If we want to do ASTGen, do so now.
if (langOpts.hasFeature(Feature::ParserASTGen)) {
swift_ASTGen_buildTopLevelASTNodes(exportedSourceFile, CurDeclContext,
&Context, &items, appendToVector);

return;
// Spin the C++ parser to the end; we won't be using it.
while (!Tok.is(tok::eof)) {
consumeToken();
}
}
#endif
Expand Down
Loading