From 6ef8f4565959bbfc6ac31e76ce2a308b6b5f326d Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Thu, 11 Sep 2025 16:54:08 +0100 Subject: [PATCH 1/3] NFC: Move `AccessNotesPath` to LangOptions --- include/swift/Basic/LangOptions.h | 3 +++ include/swift/Frontend/FrontendOptions.h | 3 --- lib/Frontend/ArgsToFrontendOptionsConverter.cpp | 3 --- lib/Frontend/CompilerInvocation.cpp | 3 +++ lib/Frontend/Frontend.cpp | 4 ++-- tools/swift-ide-test/swift-ide-test.cpp | 4 ++-- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h index 2e8e49ad79113..cfd593e18112f 100644 --- a/include/swift/Basic/LangOptions.h +++ b/include/swift/Basic/LangOptions.h @@ -493,6 +493,9 @@ namespace swift { std::shared_ptr OptimizationRemarkPassedPattern; std::shared_ptr OptimizationRemarkMissedPattern; + /// The path to load access notes from. + std::string AccessNotesPath; + /// How should we emit diagnostics about access notes? AccessNoteDiagnosticBehavior AccessNoteBehavior = AccessNoteDiagnosticBehavior::RemarkOnFailureOrSuccess; diff --git a/include/swift/Frontend/FrontendOptions.h b/include/swift/Frontend/FrontendOptions.h index a120e58903263..54e3ff8339807 100644 --- a/include/swift/Frontend/FrontendOptions.h +++ b/include/swift/Frontend/FrontendOptions.h @@ -107,9 +107,6 @@ class FrontendOptions { /// The path to which we should store indexing data, if any. std::string IndexStorePath; - /// The path to load access notes from. - std::string AccessNotesPath; - /// The path to look in when loading a module interface file, to see if a /// binary module has already been built for use by the compiler. std::string PrebuiltModuleCachePath; diff --git a/lib/Frontend/ArgsToFrontendOptionsConverter.cpp b/lib/Frontend/ArgsToFrontendOptionsConverter.cpp index ee8a94f3a0beb..f0fca0398e097 100644 --- a/lib/Frontend/ArgsToFrontendOptionsConverter.cpp +++ b/lib/Frontend/ArgsToFrontendOptionsConverter.cpp @@ -372,9 +372,6 @@ bool ArgsToFrontendOptionsConverter::convert( if (!computeModuleAliases()) return true; - if (const Arg *A = Args.getLastArg(OPT_access_notes_path)) - Opts.AccessNotesPath = A->getValue(); - if (const Arg *A = Args.getLastArg(OPT_serialize_debugging_options, OPT_no_serialize_debugging_options)) { Opts.SerializeOptionsForDebugging = diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index c977a5e2ceff5..8c1a69d7da9d1 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -1423,6 +1423,9 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args, Opts.OptimizationRemarkMissedPattern = generateOptimizationRemarkRegex(Diags, Args, A); + if (const Arg *A = Args.getLastArg(OPT_access_notes_path)) + Opts.AccessNotesPath = A->getValue(); + if (Arg *A = Args.getLastArg(OPT_Raccess_note)) { auto value = llvm::StringSwitch>( diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp index 682b50623ab15..d2f38ad2e738a 100644 --- a/lib/Frontend/Frontend.cpp +++ b/lib/Frontend/Frontend.cpp @@ -1554,12 +1554,12 @@ void CompilerInstance::setMainModule(ModuleDecl *newMod) { } void CompilerInstance::loadAccessNotesIfNeeded() { - if (Invocation.getFrontendOptions().AccessNotesPath.empty()) + if (Invocation.getLangOptions().AccessNotesPath.empty()) return; auto *mainModule = getMainModule(); - auto accessNotesPath = Invocation.getFrontendOptions().AccessNotesPath; + auto accessNotesPath = Invocation.getLangOptions().AccessNotesPath; auto bufferOrError = swift::vfs::getFileOrSTDIN(getFileSystem(), accessNotesPath); diff --git a/tools/swift-ide-test/swift-ide-test.cpp b/tools/swift-ide-test/swift-ide-test.cpp index 704aeb452bb88..fb5af648f676e 100644 --- a/tools/swift-ide-test/swift-ide-test.cpp +++ b/tools/swift-ide-test/swift-ide-test.cpp @@ -4571,8 +4571,8 @@ int main(int argc, char *argv[]) { options::ModuleCachePath[options::ModuleCachePath.size()-1]; } if (!options::AccessNotesPath.empty()) { - InitInvok.getFrontendOptions().AccessNotesPath = - options::AccessNotesPath[options::AccessNotesPath.size()-1]; + InitInvok.getLangOptions().AccessNotesPath = + options::AccessNotesPath[options::AccessNotesPath.size() - 1]; } if (options::ParseAsLibrary) { InitInvok.getFrontendOptions().InputMode = From 033638e5c76cce652cf4b7618b29e8d357158b4b Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Thu, 11 Sep 2025 16:54:08 +0100 Subject: [PATCH 2/3] NFC: Split out `TypeCheckAccessNotes.cpp` --- lib/Sema/CMakeLists.txt | 1 + lib/Sema/TypeCheckAccessNotes.cpp | 222 ++++++++++++++++++++++++++++++ lib/Sema/TypeCheckDeclPrimary.cpp | 190 ------------------------- 3 files changed, 223 insertions(+), 190 deletions(-) create mode 100644 lib/Sema/TypeCheckAccessNotes.cpp diff --git a/lib/Sema/CMakeLists.txt b/lib/Sema/CMakeLists.txt index 96d8839268e29..196d1291e2cd0 100644 --- a/lib/Sema/CMakeLists.txt +++ b/lib/Sema/CMakeLists.txt @@ -51,6 +51,7 @@ add_swift_host_library(swiftSema STATIC SyntacticElementTarget.cpp TypeOfReference.cpp TypeCheckAccess.cpp + TypeCheckAccessNotes.cpp TypeCheckAttr.cpp TypeCheckAttrABI.cpp TypeCheckAvailability.cpp diff --git a/lib/Sema/TypeCheckAccessNotes.cpp b/lib/Sema/TypeCheckAccessNotes.cpp new file mode 100644 index 0000000000000..c9454817792c4 --- /dev/null +++ b/lib/Sema/TypeCheckAccessNotes.cpp @@ -0,0 +1,222 @@ +//===--- TypeCheckAccessNotes.cpp - Type Checking for Access Notes --------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2025 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// +// +// This file implements the implicit attribute transform caused by access notes. +// +//===----------------------------------------------------------------------===// + +#include "TypeCheckDecl.h" +#include "TypeCheckObjC.h" +#include "TypeChecker.h" +#include "swift/AST/ASTContext.h" +#include "swift/AST/ASTPrinter.h" +#include "swift/AST/Decl.h" +#include "swift/AST/Module.h" +#include "swift/AST/SourceFile.h" +#include "swift/AST/TypeCheckRequests.h" +#include "swift/Basic/LLVM.h" + +using namespace swift; + +static StringRef prettyPrintAttrs(const ValueDecl *VD, + ArrayRef attrs, + SmallVectorImpl &out) { + llvm::raw_svector_ostream os(out); + StreamPrinter printer(os); + + PrintOptions opts = PrintOptions::printDeclarations(); + VD->getAttrs().print(printer, opts, attrs, VD); + return StringRef(out.begin(), out.size()).drop_back(); +} + +static void diagnoseChangesByAccessNote( + ValueDecl *VD, ArrayRef attrs, + Diag diagID, + Diag fixItID, + llvm::function_ref addFixIts) { + if (!VD->getASTContext().LangOpts.shouldRemarkOnAccessNoteSuccess() || + attrs.empty()) + return; + + // Generate string containing all attributes. + SmallString<64> attrString; + auto attrText = prettyPrintAttrs(VD, attrs, attrString); + + SourceLoc fixItLoc; + + auto reason = VD->getModuleContext()->getAccessNotes().Reason; + auto diag = VD->diagnose(diagID, reason, attrText, VD); + for (auto attr : attrs) { + diag.highlight(attr->getRangeWithAt()); + if (fixItLoc.isInvalid()) + fixItLoc = attr->getRangeWithAt().Start; + } + + if (!fixItLoc) + fixItLoc = VD->getAttributeInsertionLoc(true); + + addFixIts(VD->getASTContext().Diags.diagnose(fixItLoc, fixItID, attrText), + attrString); +} + +template +static void addOrRemoveAttr(ValueDecl *VD, const AccessNotesFile ¬es, + std::optional expected, + SmallVectorImpl &removedAttrs, + llvm::function_ref willCreate) { + if (!expected) + return; + + auto attr = VD->getAttrs().getAttribute(); + if (*expected == (attr != nullptr)) + return; + + if (*expected) { + attr = willCreate(); + attr->setAddedByAccessNote(); + VD->getAttrs().add(attr); + + // Arrange for us to emit a remark about this attribute after type checking + // has ensured it's valid. + if (auto SF = VD->getDeclContext()->getParentSourceFile()) + SF->AttrsAddedByAccessNotes[VD].push_back(attr); + } else { + removedAttrs.push_back(attr); + VD->getAttrs().removeAttribute(attr); + } +} + +InFlightDiagnostic swift::softenIfAccessNote(const Decl *D, + const DeclAttribute *attr, + InFlightDiagnostic &diag) { + const ValueDecl *VD = dyn_cast(D); + if (!VD || !attr || !attr->getAddedByAccessNote()) + return std::move(diag); + + SmallString<32> attrString; + auto attrText = prettyPrintAttrs(VD, llvm::ArrayRef(attr), attrString); + + ASTContext &ctx = D->getASTContext(); + auto behavior = ctx.LangOpts.getAccessNoteFailureLimit(); + return std::move(diag.wrapIn(diag::wrap_invalid_attr_added_by_access_note, + D->getModuleContext()->getAccessNotes().Reason, + ctx.AllocateCopy(attrText), VD) + .limitBehavior(behavior)); +} + +static void applyAccessNote(ValueDecl *VD, const AccessNote ¬e, + const AccessNotesFile ¬es) { + ASTContext &ctx = VD->getASTContext(); + SmallVector removedAttrs; + + addOrRemoveAttr(VD, notes, note.ObjC, removedAttrs, [&] { + return ObjCAttr::create(ctx, note.ObjCName, false); + }); + + addOrRemoveAttr(VD, notes, note.Dynamic, removedAttrs, + [&] { return new (ctx) DynamicAttr(true); }); + + // FIXME: If we ever have more attributes, we'll need to sort removedAttrs by + // SourceLoc. As it is, attrs are always before modifiers, so we're okay now. + + diagnoseChangesByAccessNote(VD, removedAttrs, + diag::attr_removed_by_access_note, + diag::fixit_attr_removed_by_access_note, + [&](InFlightDiagnostic diag, StringRef code) { + for (auto attr : llvm::reverse(removedAttrs)) + diag.fixItRemove(attr->getRangeWithAt()); + }); + + if (note.ObjCName) { + auto newName = note.ObjCName.value(); + + // addOrRemoveAttr above guarantees there's an ObjCAttr on this decl. + auto attr = VD->getAttrs().getAttribute(); + assert(attr && "ObjCName set, but ObjCAttr not true or did not apply???"); + + if (!attr->hasName()) { + // There was already an @objc attribute with no selector. Set it. + attr->setName(newName, true); + + if (!ctx.LangOpts.shouldRemarkOnAccessNoteSuccess()) + return; + + VD->diagnose(diag::attr_objc_name_changed_by_access_note, notes.Reason, + VD, newName); + + auto fixIt = + VD->diagnose(diag::fixit_attr_objc_name_changed_by_access_note); + fixDeclarationObjCName(fixIt, VD, ObjCSelector(), newName); + } else if (attr->getName() != newName) { + // There was already an @objc + auto behavior = ctx.LangOpts.getAccessNoteFailureLimit(); + + VD->diagnose(diag::attr_objc_name_conflicts_with_access_note, + notes.Reason, VD, attr->getName().value(), newName) + .highlight(attr->getRangeWithAt()) + .limitBehavior(behavior); + } + } +} + +void TypeChecker::applyAccessNote(ValueDecl *VD) { + (void)evaluateOrDefault(VD->getASTContext().evaluator, + ApplyAccessNoteRequest{VD}, {}); +} + +void swift::diagnoseAttrsAddedByAccessNote(SourceFile &SF) { + if (!SF.getASTContext().LangOpts.shouldRemarkOnAccessNoteSuccess()) + return; + + for (auto declAndAttrs : SF.AttrsAddedByAccessNotes) { + auto D = declAndAttrs.getFirst(); + SmallVector sortedAttrs; + llvm::append_range(sortedAttrs, declAndAttrs.getSecond()); + + // Filter out invalid attributes. + sortedAttrs.erase(llvm::remove_if(sortedAttrs, + [](DeclAttribute *attr) { + assert(attr->getAddedByAccessNote()); + return attr->isInvalid(); + }), + sortedAttrs.end()); + if (sortedAttrs.empty()) + continue; + + // Sort attributes by name. + llvm::sort(sortedAttrs, [](DeclAttribute *first, DeclAttribute *second) { + return first->getAttrName() < second->getAttrName(); + }); + sortedAttrs.erase(std::unique(sortedAttrs.begin(), sortedAttrs.end()), + sortedAttrs.end()); + + diagnoseChangesByAccessNote( + D, sortedAttrs, diag::attr_added_by_access_note, + diag::fixit_attr_added_by_access_note, + [=](InFlightDiagnostic diag, StringRef code) { + diag.fixItInsert(D->getAttributeInsertionLoc(/*isModifier=*/true), + code); + }); + } +} + +evaluator::SideEffect ApplyAccessNoteRequest::evaluate(Evaluator &evaluator, + ValueDecl *VD) const { + // Access notes don't apply to ABI-only attributes. + if (!ABIRoleInfo(VD).providesAPI()) + return {}; + + AccessNotesFile ¬es = VD->getModuleContext()->getAccessNotes(); + if (auto note = notes.lookup(VD)) + applyAccessNote(VD, *note.get(), notes); + return {}; +} diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index b7d47438bf9ea..a719d67f280de 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -2003,196 +2003,6 @@ void TypeChecker::diagnoseDuplicateCaptureVars(CaptureListExpr *expr) { diagnoseDuplicateDecls(captureListVars); } -static StringRef prettyPrintAttrs(const ValueDecl *VD, - ArrayRef attrs, - SmallVectorImpl &out) { - llvm::raw_svector_ostream os(out); - StreamPrinter printer(os); - - PrintOptions opts = PrintOptions::printDeclarations(); - VD->getAttrs().print(printer, opts, attrs, VD); - return StringRef(out.begin(), out.size()).drop_back(); -} - -static void diagnoseChangesByAccessNote( - ValueDecl *VD, ArrayRef attrs, - Diag diagID, - Diag fixItID, - llvm::function_ref addFixIts) { - if (!VD->getASTContext().LangOpts.shouldRemarkOnAccessNoteSuccess() || - attrs.empty()) - return; - - // Generate string containing all attributes. - SmallString<64> attrString; - auto attrText = prettyPrintAttrs(VD, attrs, attrString); - - SourceLoc fixItLoc; - - auto reason = VD->getModuleContext()->getAccessNotes().Reason; - auto diag = VD->diagnose(diagID, reason, attrText, VD); - for (auto attr : attrs) { - diag.highlight(attr->getRangeWithAt()); - if (fixItLoc.isInvalid()) - fixItLoc = attr->getRangeWithAt().Start; - } - - if (!fixItLoc) - fixItLoc = VD->getAttributeInsertionLoc(true); - - addFixIts(VD->getASTContext().Diags.diagnose(fixItLoc, fixItID, attrText), - attrString); -} - -template -static void addOrRemoveAttr(ValueDecl *VD, const AccessNotesFile ¬es, - std::optional expected, - SmallVectorImpl &removedAttrs, - llvm::function_ref willCreate) { - if (!expected) return; - - auto attr = VD->getAttrs().getAttribute(); - if (*expected == (attr != nullptr)) return; - - if (*expected) { - attr = willCreate(); - attr->setAddedByAccessNote(); - VD->getAttrs().add(attr); - - // Arrange for us to emit a remark about this attribute after type checking - // has ensured it's valid. - if (auto SF = VD->getDeclContext()->getParentSourceFile()) - SF->AttrsAddedByAccessNotes[VD].push_back(attr); - } else { - removedAttrs.push_back(attr); - VD->getAttrs().removeAttribute(attr); - } -} - -InFlightDiagnostic -swift::softenIfAccessNote(const Decl *D, const DeclAttribute *attr, - InFlightDiagnostic &diag) { - const ValueDecl *VD = dyn_cast(D); - if (!VD || !attr || !attr->getAddedByAccessNote()) - return std::move(diag); - - SmallString<32> attrString; - auto attrText = prettyPrintAttrs(VD, llvm::ArrayRef(attr), attrString); - - ASTContext &ctx = D->getASTContext(); - auto behavior = ctx.LangOpts.getAccessNoteFailureLimit(); - return std::move(diag.wrapIn(diag::wrap_invalid_attr_added_by_access_note, - D->getModuleContext()->getAccessNotes().Reason, - ctx.AllocateCopy(attrText), VD) - .limitBehavior(behavior)); -} - -static void applyAccessNote(ValueDecl *VD, const AccessNote ¬e, - const AccessNotesFile ¬es) { - ASTContext &ctx = VD->getASTContext(); - SmallVector removedAttrs; - - addOrRemoveAttr(VD, notes, note.ObjC, removedAttrs, [&]{ - return ObjCAttr::create(ctx, note.ObjCName, false); - }); - - addOrRemoveAttr(VD, notes, note.Dynamic, removedAttrs, [&]{ - return new (ctx) DynamicAttr(true); - }); - - // FIXME: If we ever have more attributes, we'll need to sort removedAttrs by - // SourceLoc. As it is, attrs are always before modifiers, so we're okay now. - - diagnoseChangesByAccessNote(VD, removedAttrs, - diag::attr_removed_by_access_note, - diag::fixit_attr_removed_by_access_note, - [&](InFlightDiagnostic diag, StringRef code) { - for (auto attr : llvm::reverse(removedAttrs)) - diag.fixItRemove(attr->getRangeWithAt()); - }); - - if (note.ObjCName) { - auto newName = note.ObjCName.value(); - - // addOrRemoveAttr above guarantees there's an ObjCAttr on this decl. - auto attr = VD->getAttrs().getAttribute(); - assert(attr && "ObjCName set, but ObjCAttr not true or did not apply???"); - - if (!attr->hasName()) { - // There was already an @objc attribute with no selector. Set it. - attr->setName(newName, true); - - if (!ctx.LangOpts.shouldRemarkOnAccessNoteSuccess()) - return; - - VD->diagnose(diag::attr_objc_name_changed_by_access_note, notes.Reason, - VD, newName); - - auto fixIt = - VD->diagnose(diag::fixit_attr_objc_name_changed_by_access_note); - fixDeclarationObjCName(fixIt, VD, ObjCSelector(), newName); - } - else if (attr->getName() != newName) { - // There was already an @objc - auto behavior = ctx.LangOpts.getAccessNoteFailureLimit(); - - VD->diagnose(diag::attr_objc_name_conflicts_with_access_note, - notes.Reason, VD, attr->getName().value(), newName) - .highlight(attr->getRangeWithAt()) - .limitBehavior(behavior); - } - } -} - -void TypeChecker::applyAccessNote(ValueDecl *VD) { - (void)evaluateOrDefault(VD->getASTContext().evaluator, - ApplyAccessNoteRequest{VD}, {}); -} - -void swift::diagnoseAttrsAddedByAccessNote(SourceFile &SF) { - if (!SF.getASTContext().LangOpts.shouldRemarkOnAccessNoteSuccess()) - return; - - for (auto declAndAttrs : SF.AttrsAddedByAccessNotes) { - auto D = declAndAttrs.getFirst(); - SmallVector sortedAttrs; - llvm::append_range(sortedAttrs, declAndAttrs.getSecond()); - - // Filter out invalid attributes. - sortedAttrs.erase( - llvm::remove_if(sortedAttrs, [](DeclAttribute *attr) { - assert(attr->getAddedByAccessNote()); - return attr->isInvalid(); - }), sortedAttrs.end()); - if (sortedAttrs.empty()) continue; - - // Sort attributes by name. - llvm::sort(sortedAttrs, [](DeclAttribute * first, DeclAttribute * second) { - return first->getAttrName() < second->getAttrName(); - }); - sortedAttrs.erase(std::unique(sortedAttrs.begin(), sortedAttrs.end()), - sortedAttrs.end()); - - diagnoseChangesByAccessNote(D, sortedAttrs, diag::attr_added_by_access_note, - diag::fixit_attr_added_by_access_note, - [=](InFlightDiagnostic diag, StringRef code) { - diag.fixItInsert(D->getAttributeInsertionLoc(/*isModifier=*/true), code); - }); - } -} - -evaluator::SideEffect -ApplyAccessNoteRequest::evaluate(Evaluator &evaluator, ValueDecl *VD) const { - // Access notes don't apply to ABI-only attributes. - if (!ABIRoleInfo(VD).providesAPI()) - return {}; - - AccessNotesFile ¬es = VD->getModuleContext()->getAccessNotes(); - if (auto note = notes.lookup(VD)) - applyAccessNote(VD, *note.get(), notes); - return {}; -} - static void diagnoseWrittenPlaceholderTypes(ASTContext &Ctx, const Pattern *P, Expr *init) { From 5c334c5f218b0d1ab693a8e33b760a5995702f7f Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Thu, 11 Sep 2025 16:54:08 +0100 Subject: [PATCH 3/3] Requestify the loading of access notes Replace `loadAccessNotesIfNeeded` with a request that loads the access notes on-demand. --- include/swift/AST/ASTContext.h | 4 ++ include/swift/AST/DiagnosticsFrontend.def | 3 -- include/swift/AST/DiagnosticsSema.def | 4 ++ include/swift/AST/Module.h | 7 ++-- include/swift/AST/TypeCheckRequests.h | 25 ++++++++++++ include/swift/AST/TypeCheckerTypeIDZone.def | 2 + include/swift/Frontend/Frontend.h | 5 --- lib/AST/Module.cpp | 10 +++++ lib/Frontend/Frontend.cpp | 25 ------------ lib/IDETool/IDEInspectionInstance.cpp | 1 - lib/Sema/TypeCheckAccessNotes.cpp | 42 ++++++++++++++++++--- 11 files changed, 85 insertions(+), 43 deletions(-) diff --git a/include/swift/AST/ASTContext.h b/include/swift/AST/ASTContext.h index 8e582043fad25..6de9ab2c47e90 100644 --- a/include/swift/AST/ASTContext.h +++ b/include/swift/AST/ASTContext.h @@ -1626,6 +1626,10 @@ class ASTContext final { AvailabilityDomain getTargetAvailabilityDomain() const; }; +inline SourceLoc extractNearestSourceLoc(const ASTContext *ctx) { + return SourceLoc(); +} + } // end namespace swift #endif diff --git a/include/swift/AST/DiagnosticsFrontend.def b/include/swift/AST/DiagnosticsFrontend.def index b9b2625c79a4c..71d808d191e7a 100644 --- a/include/swift/AST/DiagnosticsFrontend.def +++ b/include/swift/AST/DiagnosticsFrontend.def @@ -565,9 +565,6 @@ WARNING(module_incompatible_with_skip_function_bodies,none, "-experimental-skip-*-function-bodies* flags; they have " "been automatically disabled", (StringRef)) -WARNING(access_notes_file_io_error,none, - "ignored access notes file at '%0' because it cannot be read: %1", - (StringRef, StringRef)) WARNING(error_in_access_notes_file,none, "ignored access notes file because it cannot be parsed: %0", (StringRef)) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index a332ca7fbca4e..e38f26921d06a 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -7853,6 +7853,10 @@ ERROR(atomics_ordering_must_be_constant, none, #define WHICH_ACCESS_NOTE(reason) "specified by access note for %" #reason +WARNING(access_notes_file_io_error,none, + "ignored access notes file at '%0' because it cannot be read: %1", + (StringRef, StringRef)) + REMARK(attr_added_by_access_note, none, "implicitly added '%1' to this %kindonly2, as " WHICH_ACCESS_NOTE(0), (StringRef, StringRef, const ValueDecl *)) diff --git a/include/swift/AST/Module.h b/include/swift/AST/Module.h index bbdf9acc4c691..9c78b062eb3f5 100644 --- a/include/swift/AST/Module.h +++ b/include/swift/AST/Module.h @@ -345,8 +345,6 @@ class ModuleDecl /// \see EntryPointInfoTy EntryPointInfoTy EntryPointInfo; - AccessNotesFile accessNotes; - /// Used by the debugger to bypass resilient access to fields. bool BypassResilience = false; @@ -415,8 +413,9 @@ class ModuleDecl /// imports. ImplicitImportList getImplicitImports() const; - AccessNotesFile &getAccessNotes() { return accessNotes; } - const AccessNotesFile &getAccessNotes() const { return accessNotes; } + /// Retrieve the access notes to apply for the module, or \c nullptr if there + /// are no access notes. + const AccessNotesFile *getAccessNotes() const; /// Return whether the module was imported with resilience disabled. The /// debugger does this to access private fields. diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index c4c47aa5a5333..00c3fcdfc3325 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -4089,6 +4089,31 @@ class PrimarySourceFilesRequest bool isCached() const { return true; } }; +/// Load the access notes to apply for the main module. +/// +/// Note this is keyed on the ASTContext instead of the ModuleDecl to avoid +/// needing to re-load the access notes in cases where we have multiple main +/// modules, e.g when doing cached top-level code completion. +/// +/// FIXME: This isn't really a type-checking request, if we ever split off a +/// zone for more general requests, this should be moved there. +class LoadAccessNotesRequest + : public SimpleRequest { +public: + using SimpleRequest::SimpleRequest; + +private: + friend SimpleRequest; + + const AccessNotesFile *evaluate(Evaluator &evaluator, ASTContext *ctx) const; + +public: + // Cached. + bool isCached() const { return true; } +}; + /// Kinds of types for CustomAttr. enum class CustomAttrTypeKind { /// The type is required to not be expressed in terms of diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def index 090b6a9bd79e6..dbb8e382756a0 100644 --- a/include/swift/AST/TypeCheckerTypeIDZone.def +++ b/include/swift/AST/TypeCheckerTypeIDZone.def @@ -275,6 +275,8 @@ SWIFT_REQUEST(TypeChecker, PatternBindingCheckedAndContextualizedInitRequest, SeparatelyCached, NoLocationInfo) SWIFT_REQUEST(TypeChecker, PrimarySourceFilesRequest, ArrayRef(ModuleDecl *), Cached, NoLocationInfo) +SWIFT_REQUEST(TypeChecker, LoadAccessNotesRequest, + const AccessNotesFile *(ASTContext *), Cached, NoLocationInfo) SWIFT_REQUEST(TypeChecker, PropertyWrapperAuxiliaryVariablesRequest, PropertyWrapperAuxiliaryVariables(VarDecl *), SeparatelyCached | SplitCached, diff --git a/include/swift/Frontend/Frontend.h b/include/swift/Frontend/Frontend.h index 00c348ea070b2..9459134f6b8f5 100644 --- a/include/swift/Frontend/Frontend.h +++ b/include/swift/Frontend/Frontend.h @@ -790,11 +790,6 @@ class CompilerInstance { /// Parses and type-checks all input files. void performSema(); - /// Loads any access notes for the main module. - /// - /// FIXME: This should be requestified. - void loadAccessNotesIfNeeded(); - /// Parses and performs import resolution on all input files. /// /// This is similar to a parse-only invocation, but module imports will also diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index ed35eca070ffe..d4098f394a284 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -823,6 +823,16 @@ ImplicitImportList ModuleDecl::getImplicitImports() const { {}); } +const AccessNotesFile *ModuleDecl::getAccessNotes() const { + // Only the main module has access notes. + if (!isMainModule()) + return nullptr; + + auto &ctx = getASTContext(); + return evaluateOrDefault(ctx.evaluator, LoadAccessNotesRequest{&ctx}, + nullptr); +} + SourceFile *ModuleDecl::getSourceFileContainingLocation(SourceLoc loc) { if (loc.isInvalid()) return nullptr; diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp index d2f38ad2e738a..352f55ece5091 100644 --- a/lib/Frontend/Frontend.cpp +++ b/lib/Frontend/Frontend.cpp @@ -1553,28 +1553,6 @@ void CompilerInstance::setMainModule(ModuleDecl *newMod) { Context->MainModule = newMod; } -void CompilerInstance::loadAccessNotesIfNeeded() { - if (Invocation.getLangOptions().AccessNotesPath.empty()) - return; - - auto *mainModule = getMainModule(); - - auto accessNotesPath = Invocation.getLangOptions().AccessNotesPath; - - auto bufferOrError = - swift::vfs::getFileOrSTDIN(getFileSystem(), accessNotesPath); - if (bufferOrError) { - int sourceID = SourceMgr.addNewSourceBuffer(std::move(bufferOrError.get())); - auto buffer = SourceMgr.getLLVMSourceMgr().getMemoryBuffer(sourceID); - - if (auto accessNotesFile = AccessNotesFile::load(*Context, buffer)) - mainModule->getAccessNotes() = *accessNotesFile; - } else { - Diagnostics.diagnose(SourceLoc(), diag::access_notes_file_io_error, - accessNotesPath, bufferOrError.getError().message()); - } -} - bool CompilerInstance::performParseAndResolveImportsOnly() { FrontendStatsTracer tracer(getStatsReporter(), "parse-and-resolve-imports"); @@ -1582,9 +1560,6 @@ bool CompilerInstance::performParseAndResolveImportsOnly() { // lazily evaluate instead. Once the below computations are requestified we // ought to be able to remove this function. - // Load access notes. - loadAccessNotesIfNeeded(); - // Resolve imports for all the source files in the module. auto *mainModule = getMainModule(); performImportResolution(mainModule); diff --git a/lib/IDETool/IDEInspectionInstance.cpp b/lib/IDETool/IDEInspectionInstance.cpp index 107f7974f4d55..d54070998be36 100644 --- a/lib/IDETool/IDEInspectionInstance.cpp +++ b/lib/IDETool/IDEInspectionInstance.cpp @@ -517,7 +517,6 @@ void IDEInspectionInstance::performNewOperation( CI->getASTContext().CancellationFlag = CancellationFlag; registerIDERequestFunctions(CI->getASTContext().evaluator); - CI->loadAccessNotesIfNeeded(); performImportResolution(CI->getMainModule()); bool DidFindIDEInspectionTarget = CI->getIDEInspectionFile() diff --git a/lib/Sema/TypeCheckAccessNotes.cpp b/lib/Sema/TypeCheckAccessNotes.cpp index c9454817792c4..07b35cc17209c 100644 --- a/lib/Sema/TypeCheckAccessNotes.cpp +++ b/lib/Sema/TypeCheckAccessNotes.cpp @@ -53,7 +53,7 @@ static void diagnoseChangesByAccessNote( SourceLoc fixItLoc; - auto reason = VD->getModuleContext()->getAccessNotes().Reason; + auto reason = VD->getModuleContext()->getAccessNotes()->Reason; auto diag = VD->diagnose(diagID, reason, attrText, VD); for (auto attr : attrs) { diag.highlight(attr->getRangeWithAt()); @@ -108,7 +108,7 @@ InFlightDiagnostic swift::softenIfAccessNote(const Decl *D, ASTContext &ctx = D->getASTContext(); auto behavior = ctx.LangOpts.getAccessNoteFailureLimit(); return std::move(diag.wrapIn(diag::wrap_invalid_attr_added_by_access_note, - D->getModuleContext()->getAccessNotes().Reason, + D->getModuleContext()->getAccessNotes()->Reason, ctx.AllocateCopy(attrText), VD) .limitBehavior(behavior)); } @@ -209,14 +209,46 @@ void swift::diagnoseAttrsAddedByAccessNote(SourceFile &SF) { } } +const AccessNotesFile * +LoadAccessNotesRequest::evaluate(Evaluator &evaluator, ASTContext *_ctx) const { + auto &ctx = *_ctx; + auto &accessNotesPath = ctx.LangOpts.AccessNotesPath; + if (accessNotesPath.empty()) + return nullptr; + + auto &SM = ctx.SourceMgr; + auto &FS = *SM.getFileSystem(); + auto bufferOrError = swift::vfs::getFileOrSTDIN(FS, accessNotesPath); + if (!bufferOrError) { + ctx.Diags.diagnose(SourceLoc(), diag::access_notes_file_io_error, + accessNotesPath, bufferOrError.getError().message()); + return nullptr; + } + + int sourceID = SM.addNewSourceBuffer(std::move(bufferOrError.get())); + auto buffer = SM.getLLVMSourceMgr().getMemoryBuffer(sourceID); + + auto accessNotesFile = AccessNotesFile::load(ctx, buffer); + if (!accessNotesFile) + return nullptr; + + auto *result = + ctx.AllocateObjectCopy(std::move(*accessNotesFile)); + ctx.addDestructorCleanup(*result); + return result; +} + evaluator::SideEffect ApplyAccessNoteRequest::evaluate(Evaluator &evaluator, ValueDecl *VD) const { // Access notes don't apply to ABI-only attributes. if (!ABIRoleInfo(VD).providesAPI()) return {}; - AccessNotesFile ¬es = VD->getModuleContext()->getAccessNotes(); - if (auto note = notes.lookup(VD)) - applyAccessNote(VD, *note.get(), notes); + auto *notes = VD->getModuleContext()->getAccessNotes(); + if (!notes) + return {}; + + if (auto note = notes->lookup(VD)) + applyAccessNote(VD, *note.get(), *notes); return {}; }