Skip to content

Commit

Permalink
Merge pull request #78280 from swiftlang/revert-77140-swift-lexical-l…
Browse files Browse the repository at this point in the history
…ookup-validation

Revert "[SwiftLexicalLookup] New unqualified lookup implementation validation"
  • Loading branch information
plemarquand authored Dec 19, 2024
2 parents b0123bc + e06d0b9 commit ae88aac
Show file tree
Hide file tree
Showing 12 changed files with 1 addition and 885 deletions.
15 changes: 0 additions & 15 deletions include/swift/AST/ASTBridging.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,21 +101,6 @@ struct BridgedLocatedIdentifier {
BridgedSourceLoc NameLoc;
};

struct BridgedConsumedLookupResult {
SWIFT_NAME("name")
BridgedIdentifier Name;

SWIFT_NAME("nameLoc")
BridgedSourceLoc NameLoc;

SWIFT_NAME("flag")
SwiftInt Flag;

BRIDGED_INLINE BridgedConsumedLookupResult(swift::Identifier name,
swift::SourceLoc sourceLoc,
SwiftInt flag);
};

class BridgedDeclBaseName {
BridgedIdentifier Ident;

Expand Down
9 changes: 0 additions & 9 deletions include/swift/AST/ASTBridgingImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,6 @@ swift::DeclBaseName BridgedDeclBaseName::unbridged() const {
return swift::DeclBaseName(Ident.unbridged());
}

//===----------------------------------------------------------------------===//
// MARK: BridgedDeclBaseName
//===----------------------------------------------------------------------===//

BridgedConsumedLookupResult::BridgedConsumedLookupResult(
swift::Identifier name, swift::SourceLoc sourceLoc, SwiftInt flag)
: Name(BridgedIdentifier(name)), NameLoc(BridgedSourceLoc(sourceLoc)),
Flag(flag) {}

//===----------------------------------------------------------------------===//
// MARK: BridgedDeclNameRef
//===----------------------------------------------------------------------===//
Expand Down
7 changes: 0 additions & 7 deletions include/swift/AST/DiagnosticsCommon.def
Original file line number Diff line number Diff line change
Expand Up @@ -236,13 +236,6 @@ NOTE(in_macro_expansion,none,
ERROR(macro_experimental,none,
"%0 macros are an experimental feature that is not enabled %select{|(%1)}1",
(StringRef, StringRef))

//------------------------------------------------------------------------------
// MARK: lexical lookup diagnostics
//------------------------------------------------------------------------------

ERROR(lookup_outputs_dont_match,none,
"Unqualified lookup output from ASTScope and SwiftLexicalLookup don't match", ())

//------------------------------------------------------------------------------
// MARK: bridged diagnostics
Expand Down
4 changes: 0 additions & 4 deletions include/swift/Basic/Features.def
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,6 @@ EXPERIMENTAL_FEATURE(ParserRoundTrip, false)
/// Swift parser.
EXPERIMENTAL_FEATURE(ParserValidation, false)

/// Whether to perform validation of the unqualified lookup produced by
/// ASTScope and SwiftLexicalLookup
EXPERIMENTAL_FEATURE(UnqualifiedLookupValidation, false)

/// Enables implicit some while also enabling existential `any`
EXPERIMENTAL_FEATURE(ImplicitSome, false)

Expand Down
7 changes: 0 additions & 7 deletions include/swift/Bridging/ASTGen.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,6 @@ intptr_t swift_ASTGen_configuredRegions(
void swift_ASTGen_freeConfiguredRegions(
BridgedIfConfigClauseRangeInfo *_Nullable regions, intptr_t numRegions);

bool swift_ASTGen_validateUnqualifiedLookup(
void *_Nonnull sourceFile,
BridgedASTContext astContext,
BridgedSourceLoc sourceLoc,
bool finishInSequentialScope,
BridgedArrayRef astScopeResultRef);

size_t
swift_ASTGen_virtualFiles(void *_Nonnull sourceFile,
BridgedVirtualFile *_Nullable *_Nonnull virtualFiles);
Expand Down
124 changes: 1 addition & 123 deletions lib/AST/ASTScope.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

#include "swift/AST/ASTContext.h"
#include "swift/AST/ASTWalker.h"
#include "swift/Bridging/ASTGen.h"
#include "swift/AST/Decl.h"
#include "swift/AST/Expr.h"
#include "swift/AST/Initializer.h"
Expand All @@ -40,104 +39,6 @@ using namespace ast_scope;

#pragma mark ASTScope

class LoggingASTScopeDeclConsumer
: public namelookup::AbstractASTScopeDeclConsumer {
private:
const int shouldLookInMembers = 0b10;
namelookup::AbstractASTScopeDeclConsumer *originalConsumer;

public:
mutable SmallVector<BridgedConsumedLookupResult> recordedElements;

LoggingASTScopeDeclConsumer(
namelookup::AbstractASTScopeDeclConsumer *consumer)
: originalConsumer(consumer) {}

~LoggingASTScopeDeclConsumer() = default;

/// Called for every ValueDecl visible from the lookup.
///
/// Takes an array in order to batch the consumption before setting
/// IndexOfFirstOuterResult when necessary.
///
/// Additionally, each name is logged to `recordedElements` and
/// can be later used in validation of `SwiftLexicalLookup` result.
///
/// \param baseDC either a type context or the local context of a
/// `self` parameter declaration. See LookupResult for a discussion
/// of type -vs- instance lookup results.
///
/// \return true if the lookup should be stopped at this point.
bool consume(ArrayRef<ValueDecl *> values,
NullablePtr<DeclContext> baseDC = nullptr) override {
bool endOfLookup = originalConsumer->consume(values, baseDC);

for (auto value : values) {
if (auto sourceLoc = value->getLoc()) {
recordedElements.push_back(BridgedConsumedLookupResult(
value->getBaseIdentifier(), sourceLoc, endOfLookup));
} else {
// If sourceLoc is unavailable, use location of it's parent.
recordedElements.push_back(BridgedConsumedLookupResult(
value->getBaseIdentifier(),
value->getDeclContext()->getAsDecl()->getLoc(), endOfLookup));
}
}

return endOfLookup;
};

/// Look for members of a nominal type or extension scope.
///
/// Each call is recorded in `recordedElements` with a special flag set.
/// It can be later used in validation of `SwiftLexicalLookup` result.
///
/// \return true if the lookup should be stopped at this point.
bool lookInMembers(const DeclContext *scopeDC) const override {
bool endOfLookup = originalConsumer->lookInMembers(scopeDC);

if (auto *extDecl = dyn_cast<ExtensionDecl>(scopeDC)) {
recordedElements.push_back(BridgedConsumedLookupResult(
Identifier(), extDecl->getExtendedTypeRepr()->getLoc(),
shouldLookInMembers + endOfLookup));
} else {
recordedElements.push_back(BridgedConsumedLookupResult(
scopeDC->getSelfNominalTypeDecl()->getBaseIdentifier(),
scopeDC->getAsDecl()->getLoc(), shouldLookInMembers + endOfLookup));
}

return endOfLookup;
};

/// Called for local VarDecls that might not yet be in scope.
///
/// Note that the set of VarDecls visited here are going to be a
/// superset of those visited in consume().
bool consumePossiblyNotInScope(ArrayRef<VarDecl *> values) override {
bool result = originalConsumer->consumePossiblyNotInScope(values);
return result;
}

/// Called right before looking at the parent scope of a BraceStmt.
///
/// \return true if the lookup should be stopped at this point.
bool finishLookupInBraceStmt(BraceStmt *stmt) override {
return originalConsumer->finishLookupInBraceStmt(stmt);
}

#ifndef NDEBUG
void startingNextLookupStep() override {
originalConsumer->startingNextLookupStep();
}
void finishingLookup(std::string input) const override {
originalConsumer->finishingLookup(input);
}
bool isTargetLookup() const override {
return originalConsumer->isTargetLookup();
}
#endif
};

void ASTScope::unqualifiedLookup(
SourceFile *SF, SourceLoc loc,
namelookup::AbstractASTScopeDeclConsumer &consumer) {
Expand All @@ -147,30 +48,7 @@ void ASTScope::unqualifiedLookup(

if (auto *s = SF->getASTContext().Stats)
++s->getFrontendCounters().NumASTScopeLookups;

// Perform validation of SwiftLexicalLookup if option
// Feature::UnqualifiedLookupValidation is enabled and lookup was not
// performed in a macro.
if (SF->getASTContext().LangOpts.hasFeature(
Feature::UnqualifiedLookupValidation) &&
!SF->getEnclosingSourceFile()) {
LoggingASTScopeDeclConsumer loggingASTScopeDeclConsumer =
LoggingASTScopeDeclConsumer(&consumer);

ASTScopeImpl::unqualifiedLookup(SF, loc, loggingASTScopeDeclConsumer);

bool passed = swift_ASTGen_validateUnqualifiedLookup(
SF->getExportedSourceFile(), SF->getASTContext(), loc,
loggingASTScopeDeclConsumer.finishLookupInBraceStmt(nullptr),
BridgedArrayRef(loggingASTScopeDeclConsumer.recordedElements.data(),
loggingASTScopeDeclConsumer.recordedElements.size()));

if (!passed) {
SF->getASTContext().Diags.diagnose(loc, diag::lookup_outputs_dont_match);
}
} else {
ASTScopeImpl::unqualifiedLookup(SF, loc, consumer);
}
ASTScopeImpl::unqualifiedLookup(SF, loc, consumer);
}

llvm::SmallVector<LabeledStmt *, 4> ASTScope::lookupLabeledStmts(
Expand Down
1 change: 0 additions & 1 deletion lib/AST/FeatureSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ UNINTERESTING_FEATURE(OpaqueTypeErasure)
UNINTERESTING_FEATURE(PackageCMO)
UNINTERESTING_FEATURE(ParserRoundTrip)
UNINTERESTING_FEATURE(ParserValidation)
UNINTERESTING_FEATURE(UnqualifiedLookupValidation)
UNINTERESTING_FEATURE(ImplicitSome)
UNINTERESTING_FEATURE(ParserASTGen)
UNINTERESTING_FEATURE(BuiltinMacros)
Expand Down
1 change: 0 additions & 1 deletion lib/ASTGen/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ let package = Package(
dependencies: [
.product(name: "SwiftDiagnostics", package: "swift-syntax"),
.product(name: "SwiftIfConfig", package: "swift-syntax"),
.product(name: "SwiftLexicalLookup", package: "swift-syntax"),
.product(name: "SwiftOperators", package: "swift-syntax"),
.product(name: "SwiftParser", package: "swift-syntax"),
.product(name: "SwiftParserDiagnostics", package: "swift-syntax"),
Expand Down
2 changes: 0 additions & 2 deletions lib/ASTGen/Sources/ASTGen/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ add_pure_swift_host_library(swiftASTGen STATIC CXX_INTEROP
Exprs.swift
Fingerprint.swift
Generics.swift
LexicalLookup.swift
Literals.swift
ParameterClause.swift
Patterns.swift
Expand All @@ -27,7 +26,6 @@ add_pure_swift_host_library(swiftASTGen STATIC CXX_INTEROP
_CompilerRegexParser
_CompilerSwiftSyntax
_CompilerSwiftIfConfig
_CompilerSwiftLexicalLookup
_CompilerSwiftOperators
_CompilerSwiftSyntaxBuilder
_CompilerSwiftParser
Expand Down
Loading

0 comments on commit ae88aac

Please sign in to comment.