Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 8 additions & 0 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -5855,6 +5855,7 @@ enum class PropertyWrapperSynthesizedPropertyKind {
class VarDecl : public AbstractStorageDecl {
friend class NamingPatternRequest;
NamedPattern *NamingPattern = nullptr;
GenericEnvironment *OpenedElementEnvironment = nullptr;

public:
enum class Introducer : uint8_t {
Expand Down Expand Up @@ -5982,6 +5983,13 @@ class VarDecl : public AbstractStorageDecl {
NamedPattern *getNamingPattern() const;
void setNamingPattern(NamedPattern *Pat);

GenericEnvironment *getOpenedElementEnvironment() const {
return OpenedElementEnvironment;
}
void setOpenedElementEnvironment(GenericEnvironment *Env) {
OpenedElementEnvironment = Env;
}

/// If this is a VarDecl that does not belong to a CaseLabelItem's pattern,
/// return this. Otherwise, this VarDecl must belong to a CaseStmt's
/// CaseLabelItem. In that case, return the first case label item of the first
Expand Down
20 changes: 17 additions & 3 deletions include/swift/Sema/SyntacticElementTarget.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,16 @@
#include "swift/AST/Pattern.h"
#include "swift/AST/Stmt.h"
#include "swift/AST/TypeLoc.h"
#include "swift/Basic/TaggedUnion.h"
#include "swift/Sema/ConstraintLocator.h"
#include "swift/Sema/ContextualTypeInfo.h"

namespace swift {

namespace constraints {
/// Describes information about a for-each loop that needs to be tracked
/// within the constraint system.
struct ForEachStmtInfo {
/// Describes information specific to a sequence
/// in a for-each loop.
struct SequenceIterationInfo {
/// The type of the sequence.
Type sequenceType;

Expand All @@ -47,6 +48,19 @@ struct ForEachStmtInfo {
Expr *nextCall;
};

/// Describes information specific to a pack expansion expression
/// in a for-each loop.
struct PackIterationInfo {
/// The type of the pattern that matches the elements.
Type patternType;
};

/// Describes information about a for-each loop that needs to be tracked
/// within the constraint system.
struct ForEachStmtInfo : TaggedUnion<SequenceIterationInfo, PackIterationInfo> {
using TaggedUnion::TaggedUnion;
};

/// Describes the target to which a constraint system's solution can be
/// applied.
class SyntacticElementTarget {
Expand Down
22 changes: 22 additions & 0 deletions lib/AST/ASTVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,13 @@ class Verifier : public ASTWalker {
if (!shouldVerify(cast<Stmt>(S)))
return false;

if (auto *expansion =
dyn_cast<PackExpansionExpr>(S->getParsedSequence())) {
if (!shouldVerify(expansion)) {
return false;
}
}

if (!S->getElementExpr())
return true;

Expand All @@ -804,6 +811,11 @@ class Verifier : public ASTWalker {
}

void cleanup(ForEachStmt *S) {
if (auto *expansion =
dyn_cast<PackExpansionExpr>(S->getParsedSequence())) {
cleanup(expansion);
}

if (!S->getElementExpr())
return;

Expand Down Expand Up @@ -2605,6 +2617,16 @@ class Verifier : public ASTWalker {
abort();
}

// If we are performing pack iteration, variables have to carry the
// generic environment. Catching the missing environment here will prevent
// the code from being lowered.
if (var->getTypeInContext()->is<ErrorType>()) {
Out << "VarDecl is missing a Generic Environment: ";
var->getInterfaceType().print(Out);
Out << "\n";
abort();
}

// The fact that this is *directly* be a reference storage type
// cuts the code down quite a bit in getTypeOfReference.
if (var->getAttrs().hasAttribute<ReferenceOwnershipAttr>() !=
Expand Down
5 changes: 5 additions & 0 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7135,6 +7135,11 @@ VarDecl::VarDecl(DeclKind kind, bool isStatic, VarDecl::Introducer introducer,
}

Type VarDecl::getTypeInContext() const {
// If we are performing pack iteration, use the generic environment of the
// pack expansion expression to get the right context of a local variable.
if (auto *env = getOpenedElementEnvironment())
return GenericEnvironment::mapTypeIntoContext(env, getInterfaceType());

return getDeclContext()->mapTypeIntoContext(getInterfaceType());
}

Expand Down
3 changes: 3 additions & 0 deletions lib/AST/Stmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,9 @@ void ForEachStmt::setPattern(Pattern *p) {
}

Expr *ForEachStmt::getTypeCheckedSequence() const {
if (auto *expansion = dyn_cast<PackExpansionExpr>(getParsedSequence()))
return expansion;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to do more checking here or add a bit to the statement to make sure that we are only returning non-null pack expansion expressions iff type-checking was successful. For regular for-in statements type-checker sets iteratorVar, for pack iteration I think we just need to tag the expression itself (no harm if that applies for both packs and regular sequences too!).

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that we might need to rethink the purpose of this method with this feature in mind. The bug where pack iteration was not working in closures had to partially deal with the fact that whoever calls it assumes that the type-checking was successful and we are dealing with a sequence if the method is non-null: 90ca95c#diff-f7b20ead68204a38f1ecf3cd2202f98fbcbfc193e117458b8ce6e612cb8855c7R1893. When we are dealing with a pack expansion, the assumed "sequence" will be non-null (but containing an expansion expr) and we will go into the sequence code path, which is undesired. So the only way to use this method "right" is to check if casting the result into an expansion expression is non-null to make sure that the sequence is null... Which is not obvious at all and we need to fix this. I think I'll put a TODO on it for now


return iteratorVar ? iteratorVar->getInit(/*index=*/0) : nullptr;
}

Expand Down
Loading