Skip to content

[Sema/SIL] Improve diagnostics related to init accessors #66513

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 10 commits into from
Jun 14, 2023
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
13 changes: 13 additions & 0 deletions include/swift/AST/DeclContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ namespace swift {
class SerializedDefaultArgumentInitializer;
class SerializedTopLevelCodeDecl;
class StructDecl;
class AccessorDecl;

namespace serialization {
using DeclID = llvm::PointerEmbeddedInt<unsigned, 31>;
Expand Down Expand Up @@ -457,6 +458,18 @@ class alignas(1 << DeclContextAlignInBits) DeclContext
return const_cast<DeclContext*>(this)->getInnermostMethodContext();
}

/// Returns the innermost accessor context that belongs to a property.
///
/// This routine looks through closure, initializer, and local function
/// contexts to find the innermost accessor declaration.
///
/// \returns the innermost accessor, or null if there is no such context.
LLVM_READONLY
AccessorDecl *getInnermostPropertyAccessorContext();
const AccessorDecl *getInnermostPropertyAccessorContext() const {
return const_cast<DeclContext*>(this)->getInnermostPropertyAccessorContext();
}

/// Returns the innermost type context.
///
/// This routine looks through closure, initializer, and local function
Expand Down
17 changes: 16 additions & 1 deletion include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -7342,7 +7342,22 @@ ERROR(init_accessor_accesses_attribute_on_other_declaration,none,
ERROR(init_accessor_property_both_init_and_accessed,none,
"property %0 cannot be both initialized and accessed",
(DeclName))

ERROR(invalid_use_of_self_in_init_accessor,none,
"'self' within init accessors can only be used to reference "
"properties listed in 'initializes' and 'accesses'; "
"init accessors are run before 'self' is fully available", ())
ERROR(init_accessor_invalid_member_ref,none,
"cannot reference instance member %0; init accessors can only "
"refer to instance properties listed in 'initializes' and "
"'accesses' attributes",
(DeclNameRef))
ERROR(cannot_synthesize_memberwise_due_to_property_init_order,none,
"cannot synthesize memberwise initializer",
())
NOTE(out_of_order_access_in_init_accessor,none,
"init accessor for %0 cannot access stored property %1 because it "
"is called before %1 is initialized",
(Identifier, Identifier))

#define UNDEFINE_DIAGNOSTIC_MACROS
#include "DefineDiagnosticMacros.h"
37 changes: 37 additions & 0 deletions include/swift/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,10 @@ enum class FixKind : uint8_t {

/// Ignore missing 'each' keyword before value pack reference.
IgnoreMissingEachKeyword,

/// Ignore the fact that member couldn't be referenced within init accessor
/// because its name doesn't appear in 'initializes' or 'accesses' attributes.
AllowInvalidMemberReferenceInInitAccessor,
};

class ConstraintFix {
Expand Down Expand Up @@ -3536,6 +3540,39 @@ class IgnoreMissingEachKeyword final : public ConstraintFix {
}
};

class AllowInvalidMemberReferenceInInitAccessor final : public ConstraintFix {
DeclNameRef MemberName;

AllowInvalidMemberReferenceInInitAccessor(ConstraintSystem &cs,
DeclNameRef memberName,
ConstraintLocator *locator)
: ConstraintFix(cs, FixKind::AllowInvalidMemberReferenceInInitAccessor,
locator),
MemberName(memberName) {}

public:
std::string getName() const override {
llvm::SmallVector<char, 16> scratch;
auto memberName = MemberName.getString(scratch);
return "allow reference to member '" + memberName.str() +
"' in init accessor";
}

bool diagnose(const Solution &solution, bool asNote = false) const override;

bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
return diagnose(*commonFixes.front().first);
}

static AllowInvalidMemberReferenceInInitAccessor *
create(ConstraintSystem &cs, DeclNameRef memberName,
ConstraintLocator *locator);

static bool classof(const ConstraintFix *fix) {
return fix->getKind() == FixKind::AllowInvalidMemberReferenceInInitAccessor;
}
};

} // end namespace constraints
} // end namespace swift

Expand Down
5 changes: 5 additions & 0 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -1879,6 +1879,11 @@ struct MemberLookupResult {
/// This is a static member being access through a protocol metatype
/// but its result type doesn't conform to this protocol.
UR_InvalidStaticMemberOnProtocolMetatype,

/// This is a member that doesn't appear in 'initializes' and/or
/// 'accesses' attributes of the init accessor and therefore canno
/// t be referenced in its body.
UR_UnavailableWithinInitAccessor,
};

/// This is a list of considered (but rejected) candidates, along with a
Expand Down
6 changes: 6 additions & 0 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7132,6 +7132,12 @@ bool VarDecl::isMemberwiseInitialized(bool preferDeclaredProperties) const {
isBackingStorageForDeclaredProperty(this))
return false;

// If this is a computed property with `init` accessor, it's
// memberwise initializable when it could be used to initialize
// other stored properties.
if (auto *init = getAccessor(AccessorKind::Init))
return init->getAttrs().hasAttribute<InitializesAttr>();

// If this is a computed property, it's not memberwise initialized unless
// the caller has asked for the declared properties and it is either a
// `lazy` property or a property with an attached wrapper.
Expand Down
18 changes: 18 additions & 0 deletions lib/AST/DeclContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,24 @@ AbstractFunctionDecl *DeclContext::getInnermostMethodContext() {
return nullptr;
}

AccessorDecl *DeclContext::getInnermostPropertyAccessorContext() {
auto dc = this;
do {
if (auto decl = dc->getAsDecl()) {
auto accessor = dyn_cast<AccessorDecl>(decl);
// If we found a non-accessor decl, we're done.
if (accessor == nullptr)
return nullptr;

auto *storage = accessor->getStorage();
if (isa<VarDecl>(storage) && storage->getDeclContext()->isTypeContext())
return accessor;
}
} while ((dc = dc->getParent()));

return nullptr;
}

bool DeclContext::isTypeContext() const {
if (auto decl = getAsDecl())
return isa<NominalTypeDecl>(decl) || isa<ExtensionDecl>(decl);
Expand Down
53 changes: 32 additions & 21 deletions lib/SILGen/SILGenConstructor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -272,12 +272,13 @@ static RValue maybeEmitPropertyWrapperInitFromValue(
subs, std::move(arg));
}

static void emitApplyOfInitAccessor(SILGenFunction &SGF, SILLocation loc,
AccessorDecl *accessor, SILValue selfValue,
SILType selfTy, RValue &&initialValue) {
static void
emitApplyOfInitAccessor(SILGenFunction &SGF, SILLocation loc,
AccessorDecl *accessor, SILValue selfValue,
SILType selfTy, RValue &&initialValue) {
SmallVector<SILValue> arguments;

auto emitFieldReference = [&](VarDecl *field) {
auto emitFieldReference = [&](VarDecl *field, bool forInit = false) {
auto fieldTy =
selfTy.getFieldType(field, SGF.SGM.M, SGF.getTypeExpansionContext());
return SGF.B.createStructElementAddr(loc, selfValue, field,
Expand All @@ -287,7 +288,7 @@ static void emitApplyOfInitAccessor(SILGenFunction &SGF, SILLocation loc,
// First, let's emit all of the indirect results.
if (auto *initAttr = accessor->getAttrs().getAttribute<InitializesAttr>()) {
for (auto *property : initAttr->getPropertyDecls(accessor)) {
arguments.push_back(emitFieldReference(property));
arguments.push_back(emitFieldReference(property, /*forInit=*/true));
}
}

Expand Down Expand Up @@ -399,29 +400,39 @@ static void emitImplicitValueConstructor(SILGenFunction &SGF,

// If we have an indirect return slot, initialize it in-place.
if (resultSlot) {
// Tracks all the init accessors we have emitted
// because they can initialize more than one property.
llvm::SmallPtrSet<AccessorDecl *, 2> emittedInitAccessors;

auto elti = elements.begin(), eltEnd = elements.end();
for (VarDecl *field : decl->getStoredProperties()) {

llvm::SmallPtrSet<VarDecl *, 4> storedProperties;
{
auto properties = decl->getStoredProperties();
storedProperties.insert(properties.begin(), properties.end());
}

for (auto *member : decl->getMembers()) {
auto *field = dyn_cast<VarDecl>(member);
if (!field)
continue;

if (initializedViaAccessor.count(field))
continue;

// Handle situations where this stored propery is initialized
// via a call to an init accessor on some other property.
if (initializedViaAccessor.count(field)) {
auto *initProperty = initializedViaAccessor.find(field)->second;
auto *initAccessor = initProperty->getAccessor(AccessorKind::Init);

if (emittedInitAccessors.count(initAccessor))
if (auto *initAccessor = field->getAccessor(AccessorKind::Init)) {
if (field->isMemberwiseInitialized(/*preferDeclaredProperties=*/true)) {
assert(elti != eltEnd &&
"number of args does not match number of fields");

emitApplyOfInitAccessor(SGF, Loc, initAccessor, resultSlot, selfTy,
std::move(*elti));
++elti;
continue;
}
}

emitApplyOfInitAccessor(SGF, Loc, initAccessor, resultSlot, selfTy,
std::move(*elti));

emittedInitAccessors.insert(initAccessor);
++elti;
// If this is not one of the stored properties, let's move on.
if (!storedProperties.count(field))
continue;
}

auto fieldTy =
selfTy.getFieldType(field, SGF.SGM.M, SGF.getTypeExpansionContext());
Expand Down
40 changes: 32 additions & 8 deletions lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1154,14 +1154,38 @@ void LifetimeChecker::doIt() {

// All of the indirect results marked as "out" have to be fully initialized
// before their lifetime ends.
if (TheMemory.isOut() && Uses.empty()) {
auto loc = TheMemory.getLoc();

std::string propertyName;
auto *property = TheMemory.getPathStringToElement(0, propertyName);
diagnose(Module, F.getLocation(),
diag::ivar_not_initialized_by_init_accessor, property->getName());
EmittedErrorLocs.push_back(loc);
if (TheMemory.isOut()) {
auto diagnoseMissingInit = [&]() {
std::string propertyName;
auto *property = TheMemory.getPathStringToElement(0, propertyName);
diagnose(Module, F.getLocation(),
diag::ivar_not_initialized_by_init_accessor,
property->getName());
EmittedErrorLocs.push_back(TheMemory.getLoc());
};

// No uses means that there was no initialization.
if (Uses.empty()) {
diagnoseMissingInit();
return;
}

// Go over every return block and check whether member is fully initialized
// because it's possible that there is branch that doesn't have any use of
// the memory and nothing else is going to diagnose that. This is different
// from `self`, for example, because it would always have either `copy_addr`
// or `load` before return.

auto returnBB = F.findReturnBB();

while (returnBB != F.end()) {
auto *terminator = returnBB->getTerminator();

if (!isInitializedAtUse(DIMemoryUse(terminator, DIUseKind::Load, 0, 1)))
diagnoseMissingInit();

++returnBB;
}
}

// If the memory object has nontrivial type, then any destroy/release of the
Expand Down
5 changes: 5 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9058,3 +9058,8 @@ bool MissingEachForValuePackReference::diagnoseAsError() {

return true;
}

bool InvalidMemberReferenceWithinInitAccessor::diagnoseAsError() {
emitDiagnostic(diag::init_accessor_invalid_member_ref, MemberName);
return true;
}
13 changes: 13 additions & 0 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -3015,6 +3015,19 @@ class MissingEachForValuePackReference final : public FailureDiagnostic {
bool diagnoseAsError() override;
};

class InvalidMemberReferenceWithinInitAccessor final
: public FailureDiagnostic {
DeclNameRef MemberName;

public:
InvalidMemberReferenceWithinInitAccessor(const Solution &solution,
DeclNameRef memberName,
ConstraintLocator *locator)
: FailureDiagnostic(solution, locator), MemberName(memberName) {}

bool diagnoseAsError() override;
};

} // end namespace constraints
} // end namespace swift

Expand Down
15 changes: 15 additions & 0 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2796,3 +2796,18 @@ IgnoreMissingEachKeyword::create(ConstraintSystem &cs, Type valuePackTy,
return new (cs.getAllocator())
IgnoreMissingEachKeyword(cs, valuePackTy, locator);
}

bool AllowInvalidMemberReferenceInInitAccessor::diagnose(
const Solution &solution, bool asNote) const {
InvalidMemberReferenceWithinInitAccessor failure(solution, MemberName,
getLocator());
return failure.diagnose(asNote);
}

AllowInvalidMemberReferenceInInitAccessor *
AllowInvalidMemberReferenceInInitAccessor::create(ConstraintSystem &cs,
DeclNameRef memberName,
ConstraintLocator *locator) {
return new (cs.getAllocator())
AllowInvalidMemberReferenceInInitAccessor(cs, memberName, locator);
}
46 changes: 46 additions & 0 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9632,6 +9632,44 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
}
}

if (auto *UDE =
getAsExpr<UnresolvedDotExpr>(memberLocator->getAnchor())) {
auto *base = UDE->getBase();
if (auto *accessor = DC->getInnermostPropertyAccessorContext()) {
if (accessor->isInitAccessor() && isa<DeclRefExpr>(base) &&
accessor->getImplicitSelfDecl() ==
cast<DeclRefExpr>(base)->getDecl()) {
bool isValidReference = false;

// If name doesn't appear in either `initializes` or `accesses`
// then it's invalid instance member.

if (auto *initializesAttr =
accessor->getAttrs().getAttribute<InitializesAttr>()) {
isValidReference |= llvm::any_of(
initializesAttr->getProperties(), [&](Identifier name) {
return DeclNameRef(name) == memberName;
});
}

if (auto *accessesAttr =
accessor->getAttrs().getAttribute<AccessesAttr>()) {
isValidReference |= llvm::any_of(
accessesAttr->getProperties(), [&](Identifier name) {
return DeclNameRef(name) == memberName;
});
}

if (!isValidReference) {
result.addUnviable(
candidate,
MemberLookupResult::UR_UnavailableWithinInitAccessor);
return;
}
}
}
}

// If the underlying type of a typealias is fully concrete, it is legal
// to access the type with a protocol metatype base.
} else if (instanceTy->isExistentialType() &&
Expand Down Expand Up @@ -10208,6 +10246,10 @@ fixMemberRef(ConstraintSystem &cs, Type baseTy,

case MemberLookupResult::UR_InvalidStaticMemberOnProtocolMetatype:
return AllowInvalidStaticMemberRefOnProtocolMetatype::create(cs, locator);

case MemberLookupResult::UR_UnavailableWithinInitAccessor:
return AllowInvalidMemberReferenceInInitAccessor::create(cs, memberName,
locator);
}
}

Expand Down Expand Up @@ -14633,6 +14675,10 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
return recordFix(fix, 100) ? SolutionKind::Error : SolutionKind::Solved;
}

case FixKind::AllowInvalidMemberReferenceInInitAccessor: {
return recordFix(fix, 5) ? SolutionKind::Error : SolutionKind::Solved;
}

case FixKind::ExplicitlyConstructRawRepresentable: {
// Let's increase impact of this fix for binary operators because
// it's possible to get both `.rawValue` and construction fixes for
Expand Down
Loading