Skip to content

Commit 660a2ea

Browse files
authored
Merge pull request #78335 from rintaro/revert-revert-77140-swift-lexical-lookup-validation
Reapply "[SwiftLexicalLookup] New unqualified lookup implementation validation"
2 parents f7fb099 + bfb49da commit 660a2ea

File tree

13 files changed

+890
-1
lines changed

13 files changed

+890
-1
lines changed

include/swift/AST/ASTBridging.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,21 @@ struct BridgedLocatedIdentifier {
101101
BridgedSourceLoc NameLoc;
102102
};
103103

104+
struct BridgedConsumedLookupResult {
105+
SWIFT_NAME("name")
106+
BridgedIdentifier Name;
107+
108+
SWIFT_NAME("nameLoc")
109+
BridgedSourceLoc NameLoc;
110+
111+
SWIFT_NAME("flag")
112+
SwiftInt Flag;
113+
114+
BRIDGED_INLINE BridgedConsumedLookupResult(swift::Identifier name,
115+
swift::SourceLoc sourceLoc,
116+
SwiftInt flag);
117+
};
118+
104119
class BridgedDeclBaseName {
105120
BridgedIdentifier Ident;
106121

include/swift/AST/ASTBridgingImpl.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,15 @@ swift::DeclBaseName BridgedDeclBaseName::unbridged() const {
5555
return swift::DeclBaseName(Ident.unbridged());
5656
}
5757

58+
//===----------------------------------------------------------------------===//
59+
// MARK: BridgedDeclBaseName
60+
//===----------------------------------------------------------------------===//
61+
62+
BridgedConsumedLookupResult::BridgedConsumedLookupResult(
63+
swift::Identifier name, swift::SourceLoc sourceLoc, SwiftInt flag)
64+
: Name(BridgedIdentifier(name)), NameLoc(BridgedSourceLoc(sourceLoc)),
65+
Flag(flag) {}
66+
5867
//===----------------------------------------------------------------------===//
5968
// MARK: BridgedDeclNameRef
6069
//===----------------------------------------------------------------------===//

include/swift/AST/DiagnosticsCommon.def

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,13 @@ NOTE(in_macro_expansion,none,
236236
ERROR(macro_experimental,none,
237237
"%0 macros are an experimental feature that is not enabled %select{|(%1)}1",
238238
(StringRef, StringRef))
239+
240+
//------------------------------------------------------------------------------
241+
// MARK: lexical lookup diagnostics
242+
//------------------------------------------------------------------------------
243+
244+
ERROR(lookup_outputs_dont_match,none,
245+
"Unqualified lookup output from ASTScope and SwiftLexicalLookup don't match", ())
239246

240247
//------------------------------------------------------------------------------
241248
// MARK: bridged diagnostics

include/swift/Basic/Features.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,10 @@ EXPERIMENTAL_FEATURE(ParserRoundTrip, false)
300300
/// Swift parser.
301301
EXPERIMENTAL_FEATURE(ParserValidation, false)
302302

303+
/// Whether to perform validation of the unqualified lookup produced by
304+
/// ASTScope and SwiftLexicalLookup
305+
EXPERIMENTAL_FEATURE(UnqualifiedLookupValidation, false)
306+
303307
/// Enables implicit some while also enabling existential `any`
304308
EXPERIMENTAL_FEATURE(ImplicitSome, false)
305309

include/swift/Bridging/ASTGen.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,13 @@ intptr_t swift_ASTGen_configuredRegions(
9898
void swift_ASTGen_freeConfiguredRegions(
9999
BridgedIfConfigClauseRangeInfo *_Nullable regions, intptr_t numRegions);
100100

101+
bool swift_ASTGen_validateUnqualifiedLookup(
102+
void *_Nonnull sourceFile,
103+
BridgedASTContext astContext,
104+
BridgedSourceLoc sourceLoc,
105+
bool finishInSequentialScope,
106+
BridgedArrayRef astScopeResultRef);
107+
101108
size_t
102109
swift_ASTGen_virtualFiles(void *_Nonnull sourceFile,
103110
BridgedVirtualFile *_Nullable *_Nonnull virtualFiles);

lib/AST/ASTScope.cpp

Lines changed: 123 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#include "swift/AST/ASTContext.h"
1919
#include "swift/AST/ASTWalker.h"
20+
#include "swift/Bridging/ASTGen.h"
2021
#include "swift/AST/Decl.h"
2122
#include "swift/AST/Expr.h"
2223
#include "swift/AST/Initializer.h"
@@ -39,6 +40,104 @@ using namespace ast_scope;
3940

4041
#pragma mark ASTScope
4142

43+
class LoggingASTScopeDeclConsumer
44+
: public namelookup::AbstractASTScopeDeclConsumer {
45+
private:
46+
const int shouldLookInMembers = 0b10;
47+
namelookup::AbstractASTScopeDeclConsumer *originalConsumer;
48+
49+
public:
50+
mutable SmallVector<BridgedConsumedLookupResult> recordedElements;
51+
52+
LoggingASTScopeDeclConsumer(
53+
namelookup::AbstractASTScopeDeclConsumer *consumer)
54+
: originalConsumer(consumer) {}
55+
56+
~LoggingASTScopeDeclConsumer() = default;
57+
58+
/// Called for every ValueDecl visible from the lookup.
59+
///
60+
/// Takes an array in order to batch the consumption before setting
61+
/// IndexOfFirstOuterResult when necessary.
62+
///
63+
/// Additionally, each name is logged to `recordedElements` and
64+
/// can be later used in validation of `SwiftLexicalLookup` result.
65+
///
66+
/// \param baseDC either a type context or the local context of a
67+
/// `self` parameter declaration. See LookupResult for a discussion
68+
/// of type -vs- instance lookup results.
69+
///
70+
/// \return true if the lookup should be stopped at this point.
71+
bool consume(ArrayRef<ValueDecl *> values,
72+
NullablePtr<DeclContext> baseDC = nullptr) override {
73+
bool endOfLookup = originalConsumer->consume(values, baseDC);
74+
75+
for (auto value : values) {
76+
if (auto sourceLoc = value->getLoc()) {
77+
recordedElements.push_back(BridgedConsumedLookupResult(
78+
value->getBaseIdentifier(), sourceLoc, endOfLookup));
79+
} else {
80+
// If sourceLoc is unavailable, use location of it's parent.
81+
recordedElements.push_back(BridgedConsumedLookupResult(
82+
value->getBaseIdentifier(),
83+
value->getDeclContext()->getAsDecl()->getLoc(), endOfLookup));
84+
}
85+
}
86+
87+
return endOfLookup;
88+
};
89+
90+
/// Look for members of a nominal type or extension scope.
91+
///
92+
/// Each call is recorded in `recordedElements` with a special flag set.
93+
/// It can be later used in validation of `SwiftLexicalLookup` result.
94+
///
95+
/// \return true if the lookup should be stopped at this point.
96+
bool lookInMembers(const DeclContext *scopeDC) const override {
97+
bool endOfLookup = originalConsumer->lookInMembers(scopeDC);
98+
99+
if (auto *extDecl = dyn_cast<ExtensionDecl>(scopeDC)) {
100+
recordedElements.push_back(BridgedConsumedLookupResult(
101+
Identifier(), extDecl->getExtendedTypeRepr()->getLoc(),
102+
shouldLookInMembers + endOfLookup));
103+
} else {
104+
recordedElements.push_back(BridgedConsumedLookupResult(
105+
scopeDC->getSelfNominalTypeDecl()->getBaseIdentifier(),
106+
scopeDC->getAsDecl()->getLoc(), shouldLookInMembers + endOfLookup));
107+
}
108+
109+
return endOfLookup;
110+
};
111+
112+
/// Called for local VarDecls that might not yet be in scope.
113+
///
114+
/// Note that the set of VarDecls visited here are going to be a
115+
/// superset of those visited in consume().
116+
bool consumePossiblyNotInScope(ArrayRef<VarDecl *> values) override {
117+
bool result = originalConsumer->consumePossiblyNotInScope(values);
118+
return result;
119+
}
120+
121+
/// Called right before looking at the parent scope of a BraceStmt.
122+
///
123+
/// \return true if the lookup should be stopped at this point.
124+
bool finishLookupInBraceStmt(BraceStmt *stmt) override {
125+
return originalConsumer->finishLookupInBraceStmt(stmt);
126+
}
127+
128+
#ifndef NDEBUG
129+
void startingNextLookupStep() override {
130+
originalConsumer->startingNextLookupStep();
131+
}
132+
void finishingLookup(std::string input) const override {
133+
originalConsumer->finishingLookup(input);
134+
}
135+
bool isTargetLookup() const override {
136+
return originalConsumer->isTargetLookup();
137+
}
138+
#endif
139+
};
140+
42141
void ASTScope::unqualifiedLookup(
43142
SourceFile *SF, SourceLoc loc,
44143
namelookup::AbstractASTScopeDeclConsumer &consumer) {
@@ -48,7 +147,30 @@ void ASTScope::unqualifiedLookup(
48147

49148
if (auto *s = SF->getASTContext().Stats)
50149
++s->getFrontendCounters().NumASTScopeLookups;
51-
ASTScopeImpl::unqualifiedLookup(SF, loc, consumer);
150+
151+
// Perform validation of SwiftLexicalLookup if option
152+
// Feature::UnqualifiedLookupValidation is enabled and lookup was not
153+
// performed in a macro.
154+
if (SF->getASTContext().LangOpts.hasFeature(
155+
Feature::UnqualifiedLookupValidation) &&
156+
!SF->getEnclosingSourceFile()) {
157+
LoggingASTScopeDeclConsumer loggingASTScopeDeclConsumer =
158+
LoggingASTScopeDeclConsumer(&consumer);
159+
160+
ASTScopeImpl::unqualifiedLookup(SF, loc, loggingASTScopeDeclConsumer);
161+
162+
bool passed = swift_ASTGen_validateUnqualifiedLookup(
163+
SF->getExportedSourceFile(), SF->getASTContext(), loc,
164+
loggingASTScopeDeclConsumer.finishLookupInBraceStmt(nullptr),
165+
BridgedArrayRef(loggingASTScopeDeclConsumer.recordedElements.data(),
166+
loggingASTScopeDeclConsumer.recordedElements.size()));
167+
168+
if (!passed) {
169+
SF->getASTContext().Diags.diagnose(loc, diag::lookup_outputs_dont_match);
170+
}
171+
} else {
172+
ASTScopeImpl::unqualifiedLookup(SF, loc, consumer);
173+
}
52174
}
53175

54176
llvm::SmallVector<LabeledStmt *, 4> ASTScope::lookupLabeledStmts(

lib/AST/CMakeLists.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,4 +171,9 @@ target_link_libraries(swiftAST
171171
PUBLIC swiftBasic
172172
PRIVATE swiftMarkup)
173173

174+
if (SWIFT_BUILD_SWIFT_SYNTAX)
175+
target_link_libraries(swiftAST
176+
PRIVATE swiftASTGen)
177+
endif()
178+
174179
set_swift_llvm_is_available(swiftAST)

lib/AST/FeatureSet.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ UNINTERESTING_FEATURE(OpaqueTypeErasure)
102102
UNINTERESTING_FEATURE(PackageCMO)
103103
UNINTERESTING_FEATURE(ParserRoundTrip)
104104
UNINTERESTING_FEATURE(ParserValidation)
105+
UNINTERESTING_FEATURE(UnqualifiedLookupValidation)
105106
UNINTERESTING_FEATURE(ImplicitSome)
106107
UNINTERESTING_FEATURE(ParserASTGen)
107108
UNINTERESTING_FEATURE(BuiltinMacros)

lib/ASTGen/Package.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ let package = Package(
5656
dependencies: [
5757
.product(name: "SwiftDiagnostics", package: "swift-syntax"),
5858
.product(name: "SwiftIfConfig", package: "swift-syntax"),
59+
.product(name: "SwiftLexicalLookup", package: "swift-syntax"),
5960
.product(name: "SwiftOperators", package: "swift-syntax"),
6061
.product(name: "SwiftParser", package: "swift-syntax"),
6162
.product(name: "SwiftParserDiagnostics", package: "swift-syntax"),

lib/ASTGen/Sources/ASTGen/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ add_pure_swift_host_library(swiftASTGen STATIC CXX_INTEROP
1010
Exprs.swift
1111
Fingerprint.swift
1212
Generics.swift
13+
LexicalLookup.swift
1314
Literals.swift
1415
ParameterClause.swift
1516
Patterns.swift
@@ -26,6 +27,7 @@ add_pure_swift_host_library(swiftASTGen STATIC CXX_INTEROP
2627
_CompilerRegexParser
2728
_CompilerSwiftSyntax
2829
_CompilerSwiftIfConfig
30+
_CompilerSwiftLexicalLookup
2931
_CompilerSwiftOperators
3032
_CompilerSwiftSyntaxBuilder
3133
_CompilerSwiftParser

0 commit comments

Comments
 (0)