-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
||
|
@@ -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; | ||
} | ||
|
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() {} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 thefindBuffer
services on wide uses of availability macros, we could look into deregistering these small buffers after parsing and copy all these strings.