Skip to content

Sema: Move the availability macros cache to the ASTContext #76794

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 2 commits into from
Oct 2, 2024
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
7 changes: 7 additions & 0 deletions include/swift/AST/ASTContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ namespace swift {
class AbstractFunctionDecl;
class ASTContext;
enum class Associativity : unsigned char;
class AvailabilityMacroMap;
class AvailabilityRange;
class BoundGenericType;
class BuiltinTupleDecl;
Expand Down Expand Up @@ -987,6 +988,12 @@ class ASTContext final {
return getMultiPayloadEnumTagSinglePayloadAvailability();
}

/// Cache of the availability macros parsed from the command line arguments.
///
/// This is an implementation detail, access via
/// \c Parser::parseAllAvailabilityMacroArguments.
AvailabilityMacroMap &getAvailabilityMacroCache() const;

/// Test support utility for loading a platform remap file
/// in case an SDK is not specified to the compilation.
const clang::DarwinSDKInfo::RelatedTargetVersionMapping *
Expand Down
13 changes: 13 additions & 0 deletions include/swift/AST/AvailabilitySpec.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,19 @@ class OtherPlatformAvailabilitySpec : public AvailabilitySpec {
}
};

/// Maps of macro name and version to availability specifications.
/// Organized as two nested \c DenseMap keyed first on the macro name then
/// the macro version. This structure allows to peek at macro names before
/// parsing a version tuple.
class AvailabilityMacroMap {
public:
typedef llvm::DenseMap<llvm::VersionTuple,
SmallVector<AvailabilitySpec *, 4>> VersionEntry;

bool WasParsed = false;
llvm::DenseMap<StringRef, VersionEntry> Impl;
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a StringMap rather than depend on the StringRefs living forever?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's safe currently. The StringRefs should point to a buffer attached to the SourceManager of the ASTContext, so both the cache and the sources should be freed together. Does this sound right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently one of these buffers is allocated for each -define-availability argument. I suspect this doesn't help with the general performances of the findBuffer services on wide uses of availability macros, we could look into deregistering these small buffers after parsing and copy all these strings.

};

} // end namespace swift

#endif
18 changes: 1 addition & 17 deletions include/swift/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -346,22 +346,6 @@ class Parser {
/// This vector is managed by \c StructureMarkerRAII objects.
llvm::SmallVector<StructureMarker, 16> StructureMarkers;

/// Maps of macro name and version to availability specifications.
typedef llvm::DenseMap<llvm::VersionTuple,
SmallVector<AvailabilitySpec *, 4>>
AvailabilityMacroVersionMap;
typedef llvm::DenseMap<StringRef, AvailabilityMacroVersionMap>
AvailabilityMacroMap;

/// Cache of the availability macros parsed from the command line arguments.
/// Organized as two nested \c DenseMap keyed first on the macro name then
/// the macro version. This structure allows to peek at macro names before
/// parsing a version tuple.
AvailabilityMacroMap AvailabilityMacros;

/// Has \c AvailabilityMacros been computed?
bool AvailabilityMacrosComputed = false;

public:
Parser(unsigned BufferID, SourceFile &SF, DiagnosticEngine* LexerDiags,
SILParserStateBase *SIL, PersistentParserState *PersistentState);
Expand Down Expand Up @@ -2080,7 +2064,7 @@ class Parser {
parseAvailabilityMacro(SmallVectorImpl<AvailabilitySpec *> &Specs);

/// Parse the availability macros definitions passed as arguments.
void parseAllAvailabilityMacroArguments();
AvailabilityMacroMap &parseAllAvailabilityMacroArguments();

/// Result of parsing an availability macro definition.
struct AvailabilityMacroDefinition {
Expand Down
7 changes: 7 additions & 0 deletions lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,9 @@ struct ASTContext::Implementation {
/// Singleton used to cache the import graph.
swift::namelookup::ImportCache TheImportCache;

/// Cache of availability macros parsed from the command line.
AvailabilityMacroMap TheAvailabilityMacroCache;
Copy link
Member

Choose a reason for hiding this comment

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

Could we use the request-evaluator here rather than add the cache directly to the ASTContext?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The request input here would be the compiler arguments. Since it's used at the parser level we can't rely on them being attached to the module as I would usually do. Would you know what to define as inputs for a request in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Other requests seem to use the ASTContext itself. I'll see to use that.


/// The module loader used to load Clang modules.
ClangModuleLoader *TheClangModuleLoader = nullptr;

Expand Down Expand Up @@ -2281,6 +2284,10 @@ swift::namelookup::ImportCache &ASTContext::getImportCache() const {
return getImpl().TheImportCache;
}

AvailabilityMacroMap &ASTContext::getAvailabilityMacroCache() const {
return getImpl().TheAvailabilityMacroCache;
}

ClangModuleLoader *ASTContext::getClangModuleLoader() const {
return getImpl().TheClangModuleLoader;
}
Expand Down
46 changes: 25 additions & 21 deletions lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2063,22 +2063,20 @@ void Parser::parseObjCSelector(SmallVector<Identifier, 4> &Names,
}

bool Parser::peekAvailabilityMacroName() {
parseAllAvailabilityMacroArguments();
AvailabilityMacroMap Map = AvailabilityMacros;
AvailabilityMacroMap &Map = parseAllAvailabilityMacroArguments();

StringRef MacroName = Tok.getText();
return Map.find(MacroName) != Map.end();
return Map.Impl.find(MacroName) != Map.Impl.end();
}

ParserStatus
Parser::parseAvailabilityMacro(SmallVectorImpl<AvailabilitySpec *> &Specs) {
// Get the macros from the compiler arguments.
parseAllAvailabilityMacroArguments();
AvailabilityMacroMap Map = AvailabilityMacros;
AvailabilityMacroMap &Map = parseAllAvailabilityMacroArguments();

StringRef MacroName = Tok.getText();
auto NameMatch = Map.find(MacroName);
if (NameMatch == Map.end())
auto NameMatch = Map.Impl.find(MacroName);
if (NameMatch == Map.Impl.end())
return makeParserSuccess(); // No match, it could be a standard platform.

consumeToken();
Expand Down Expand Up @@ -2114,20 +2112,26 @@ Parser::parseAvailabilityMacro(SmallVectorImpl<AvailabilitySpec *> &Specs) {
return makeParserSuccess();
}

void Parser::parseAllAvailabilityMacroArguments() {

if (AvailabilityMacrosComputed) return;

AvailabilityMacroMap Map;
AvailabilityMacroMap &Parser::parseAllAvailabilityMacroArguments() {
AvailabilityMacroMap &Map = Context.getAvailabilityMacroCache();
if (Map.WasParsed)
return Map;

SourceManager &SM = Context.SourceMgr;
LangOptions LangOpts = Context.LangOpts;

// Allocate all buffers in one go to avoid repeating the sorting in
// findBufferContainingLocInternal.
llvm::SmallVector<unsigned, 4> bufferIDs;
for (StringRef macro: LangOpts.AvailabilityMacros) {
unsigned bufferID = SM.addMemBufferCopy(macro,
"-define-availability argument");
bufferIDs.push_back(bufferID);
}

// Parse each macro definition.
for (unsigned bufferID: bufferIDs) {
// Create temporary parser.
int bufferID = SM.addMemBufferCopy(macro,
"-define-availability argument");
swift::ParserUnit PU(SM, SourceFileKind::Main, bufferID, LangOpts,
TypeCheckerOptions(), SILOptions(), "unknown");

Expand Down Expand Up @@ -2156,9 +2160,9 @@ void Parser::parseAllAvailabilityMacroArguments() {
ParsedMacro.Specs = SpecsCopy;

// Find the macro info by name.
AvailabilityMacroVersionMap MacroDefinition;
auto NameMatch = Map.find(ParsedMacro.Name);
if (NameMatch != Map.end()) {
AvailabilityMacroMap::VersionEntry MacroDefinition;
auto NameMatch = Map.Impl.find(ParsedMacro.Name);
if (NameMatch != Map.Impl.end()) {
MacroDefinition = NameMatch->getSecond();
}

Expand All @@ -2171,12 +2175,12 @@ void Parser::parseAllAvailabilityMacroArguments() {
}

// Save back the macro spec.
Map.erase(ParsedMacro.Name);
Map.insert({ParsedMacro.Name, MacroDefinition});
Map.Impl.erase(ParsedMacro.Name);
Map.Impl.insert({ParsedMacro.Name, MacroDefinition});
}

AvailabilityMacros = Map;
AvailabilityMacrosComputed = true;
Map.WasParsed = true;
return Map;
}

ParserStatus Parser::parsePlatformVersionInList(StringRef AttrName,
Expand Down
23 changes: 23 additions & 0 deletions test/Sema/availability_define_multi_file.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// RUN: %empty-directory(%t)
// RUN: split-file %s %t --leading-lines

// RUN: not %target-swift-frontend -typecheck -diagnostic-style llvm \
// RUN: %t/FileA.swift %t/FileB.swift \
// RUN: -define-availability "_justAName" \
// RUN: 2>&1 | %FileCheck %s

// CHECK: -define-availability argument:1:11: error: expected ':' after '_justAName' in availability macro definition
// CHECK-NEXT: _justAName

/// It's parsed once so the diagnostic is produced once as well.
// CHECK-NOT: _justAName

//--- FileA.swift

@available(_triggerParsingMacros)
public func brokenPlatforms() {}

//--- FileB.swift

@available(_triggerParsingMacros)
public func brokenPlatforms() {}