Skip to content

Special-case Pattern Binding Decls Created by LLDB #37742

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 1 commit into from
Jun 4, 2021
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
4 changes: 3 additions & 1 deletion include/swift/AST/ASTScope.h
Original file line number Diff line number Diff line change
Expand Up @@ -957,7 +957,9 @@ class PatternEntryInitializerScope final : public AbstractPatternEntryScope {
public:
PatternEntryInitializerScope(PatternBindingDecl *pbDecl, unsigned entryIndex)
: AbstractPatternEntryScope(pbDecl, entryIndex),
initAsWrittenWhenCreated(pbDecl->getOriginalInit(entryIndex)) {}
initAsWrittenWhenCreated(pbDecl->isDebuggerBinding() ?
pbDecl->getInit(entryIndex) :
pbDecl->getOriginalInit(entryIndex)) {}
virtual ~PatternEntryInitializerScope() {}

protected:
Expand Down
31 changes: 29 additions & 2 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -318,10 +318,13 @@ class alignas(1 << DeclAlignInBits) Decl {
Hoisted : 1
);

SWIFT_INLINE_BITFIELD_FULL(PatternBindingDecl, Decl, 1+2+16,
SWIFT_INLINE_BITFIELD_FULL(PatternBindingDecl, Decl, 1+1+2+16,
/// Whether this pattern binding declares static variables.
IsStatic : 1,

/// Whether this pattern binding is synthesized by the debugger.
IsDebugger : 1,

/// Whether 'static' or 'class' was used.
StaticSpelling : 2,

Expand Down Expand Up @@ -1471,9 +1474,10 @@ class PatternBindingEntry {
enum class PatternFlags {
IsText = 1 << 0,
IsFullyValidated = 1 << 1,
IsFromDebugger = 1 << 2,
};
/// The initializer context used for this pattern binding entry.
llvm::PointerIntPair<DeclContext *, 2, OptionSet<PatternFlags>>
llvm::PointerIntPair<DeclContext *, 3, OptionSet<PatternFlags>>
InitContextAndFlags;

/// Values captured by this initializer.
Expand Down Expand Up @@ -1501,6 +1505,14 @@ class PatternBindingEntry {
PatternFlags::IsFullyValidated);
}

/// Set if this pattern binding came from the debugger.
///
/// Stay away unless you are \c PatternBindingDecl::createForDebugger
void setFromDebugger() {
InitContextAndFlags.setInt(InitContextAndFlags.getInt() |
PatternFlags::IsFromDebugger);
}

public:
/// \p E is the initializer as parsed.
PatternBindingEntry(Pattern *P, SourceLoc EqualLoc, Expr *E,
Expand Down Expand Up @@ -1583,6 +1595,11 @@ class PatternBindingEntry {
PatternAndFlags.setInt(PatternAndFlags.getInt() | Flags::Subsumed);
}

/// Returns \c true if the debugger created this pattern binding entry.
bool isFromDebugger() const {
return InitContextAndFlags.getInt().contains(PatternFlags::IsFromDebugger);
}

// Return the first variable initialized by this pattern.
VarDecl *getAnchoringVarDecl() const;

Expand Down Expand Up @@ -1668,6 +1685,13 @@ class PatternBindingDecl final : public Decl,
unsigned NumPatternEntries,
DeclContext *Parent);

// A dedicated entrypoint that allows LLDB to create pattern bindings
// that look implicit to the compiler but contain user code.
static PatternBindingDecl *createForDebugger(ASTContext &Ctx,
StaticSpellingKind Spelling,
Pattern *Pat, Expr *E,
DeclContext *Parent);

SourceLoc getStartLoc() const {
return StaticLoc.isValid() ? StaticLoc : VarLoc;
}
Expand Down Expand Up @@ -1869,6 +1893,9 @@ class PatternBindingDecl final : public Decl,
return getPatternList()[i].getInitStringRepresentation(scratch);
}

/// Returns \c true if this pattern binding was created by the debugger.
bool isDebuggerBinding() const { return Bits.PatternBindingDecl.IsDebugger; }

static bool classof(const Decl *D) {
return D->getKind() == DeclKind::PatternBinding;
}
Expand Down
28 changes: 25 additions & 3 deletions lib/AST/ASTScopeCreation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -480,9 +480,15 @@ ScopeCreator::addToScopeTreeAndReturnInsertionPoint(ASTNode n,
if (!n)
return parent;

// HACK: LLDB creates implicit pattern bindings that... contain user
// expressions. We need to actually honor lookups through those bindings
// in case they contain closures that bind additional variables in further
// scopes.
if (auto *d = n.dyn_cast<Decl *>())
if (d->isImplicit())
return parent;
if (!isa<PatternBindingDecl>(d)
|| !cast<PatternBindingDecl>(d)->isDebuggerBinding())
return parent;

NodeAdder adder(endLoc);
if (auto *p = n.dyn_cast<Decl *>())
Expand Down Expand Up @@ -733,6 +739,23 @@ PatternEntryDeclScope::expandAScopeThatCreatesANewInsertionPoint(
this, decl, patternEntryIndex);
}

// If this pattern binding entry was created by the debugger, it will always
// have a synthesized init that is created from user code. We special-case
// lookups into these scopes to look through the debugger's chicanery to the
// underlying user-defined scopes, if any.
if (patternEntry.isFromDebugger() && patternEntry.getInit()) {
ASTScopeAssert(
patternEntry.getInit()->getSourceRange().isValid(),
"pattern initializer has invalid source range");
ASTScopeAssert(
!getSourceManager().isBeforeInBuffer(
patternEntry.getInit()->getStartLoc(), decl->getStartLoc()),
"inits are always after the '='");
scopeCreator
.constructExpandAndInsert<PatternEntryInitializerScope>(
this, decl, patternEntryIndex);
}

// Add accessors for the variables in this pattern.
patternEntry.getPattern()->forEachVariable([&](VarDecl *var) {
scopeCreator.addChildrenForParsedAccessors(var, this);
Expand All @@ -751,8 +774,7 @@ void
PatternEntryInitializerScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
ScopeCreator &scopeCreator) {
// Create a child for the initializer expression.
scopeCreator.addToScopeTree(ASTNode(getPatternEntry().getOriginalInit()),
this);
scopeCreator.addToScopeTree(ASTNode(initAsWrittenWhenCreated), this);
}


Expand Down
7 changes: 7 additions & 0 deletions lib/AST/ASTScopeSourceRange.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ static SourceLoc getLocAfterExtendedNominal(const ExtensionDecl *);

void ASTScopeImpl::checkSourceRangeBeforeAddingChild(ASTScopeImpl *child,
const ASTContext &ctx) const {
// Ignore debugger bindings - they're a special mix of user code and implicit
// wrapper code that is too difficult to check for consistency.
if (auto d = getDeclIfAny().getPtrOrNull())
Copy link
Contributor

Choose a reason for hiding this comment

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

We actually need source ranges to be in order for lookups to work correctly, though. Can you ignore the lldb-written part of the binding? It shouldn't have source locations anyway.

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 believe that’s what this does. When we expand the scopes for the binding’s initializer we’ll still run these checks.

if (auto *PBD = dyn_cast<PatternBindingDecl>(d))
if (PBD->isDebuggerBinding())
return;

auto &sourceMgr = ctx.SourceMgr;

auto range = getCharSourceRangeOfScope(sourceMgr);
Expand Down
11 changes: 11 additions & 0 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1341,6 +1341,17 @@ PatternBindingDecl *PatternBindingDecl::createImplicit(
return Result;
}

PatternBindingDecl *PatternBindingDecl::createForDebugger(
ASTContext &Ctx, StaticSpellingKind StaticSpelling, Pattern *Pat, Expr *E,
DeclContext *Parent) {
auto *Result = createImplicit(Ctx, StaticSpelling, Pat, E, Parent);
Result->Bits.PatternBindingDecl.IsDebugger = true;
for (auto &entry : Result->getMutablePatternList()) {
entry.setFromDebugger();
}
return Result;
}

PatternBindingDecl *
PatternBindingDecl::create(ASTContext &Ctx, SourceLoc StaticLoc,
StaticSpellingKind StaticSpelling,
Expand Down