Skip to content

[6.0🍒] NCGenerics: restrict conditional Copyable reqs #72961

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
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
6 changes: 5 additions & 1 deletion include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -2791,7 +2791,7 @@ ERROR(protocol_has_missing_requirements_versioned,none,
(Type, Type, llvm::VersionTuple, llvm::VersionTuple))
ERROR(requirement_restricts_self,none,
"%kindonly0 requirement %0 cannot add constraint "
"'%1%select{:|:| ==|:}2 %3' on 'Self'",
"'%1%select{:|:| ==|:| has same shape as}2 %3' on 'Self'",
(const ValueDecl *, StringRef, unsigned, StringRef))
ERROR(witness_argument_name_mismatch,none,
"%kind0 has different argument labels "
Expand Down Expand Up @@ -7634,6 +7634,10 @@ ERROR(inverse_extension, none,
ERROR(copyable_illegal_deinit, none,
"deinitializer cannot be declared in %kind0 that conforms to 'Copyable'",
(const ValueDecl *))
ERROR(inverse_cannot_be_conditional_on_requirement, none,
"conditional conformance to suppressible %kind0 cannot depend on "
"'%noformat1%select{:|:| ==|:| has same shape as}2 %noformat3'",
(const ProtocolDecl *, Type, unsigned, Type))
ERROR(inverse_type_member_in_conforming_type,none,
"%select{stored property %2|associated value %2}1 of "
"'%4'-conforming %kind3 has non-%4 type %0",
Expand Down
3 changes: 2 additions & 1 deletion include/swift/AST/RequirementKind.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,11 @@ enum class RequirementKind : unsigned {
Layout,
/// A same-shape requirement shape(T) == shape(U), where T and U are pack
/// parameters.
SameShape
SameShape,

// Note: there is code that packs this enum in a 3-bit bitfield. Audit users
// when adding enumerators.
LAST_KIND=SameShape
};

} // namespace swift
Expand Down
16 changes: 11 additions & 5 deletions lib/AST/DiagnosticEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,13 @@ static void formatDiagnosticArgument(StringRef Modifier,
case DiagnosticArgumentKind::FullyQualifiedType:
case DiagnosticArgumentKind::Type:
case DiagnosticArgumentKind::WitnessType: {
assert(Modifier.empty() && "Improper modifier for Type argument");
std::optional<DiagnosticFormatOptions> TypeFormatOpts;
if (Modifier == "noformat") {
TypeFormatOpts.emplace(DiagnosticFormatOptions::formatForFixIts());
} else {
assert(Modifier.empty() && "Improper modifier for Type argument");
TypeFormatOpts.emplace(FormatOpts);
}

// Strip extraneous parentheses; they add no value.
Type type;
Expand Down Expand Up @@ -829,7 +835,7 @@ static void formatDiagnosticArgument(StringRef Modifier,

auto descriptiveKind = opaqueTypeDecl->getDescriptiveKind();

Out << llvm::format(FormatOpts.OpaqueResultFormatString.c_str(),
Out << llvm::format(TypeFormatOpts->OpaqueResultFormatString.c_str(),
type->getString(printOptions).c_str(),
Decl::getDescriptiveKindName(descriptiveKind).data(),
NamingDeclText.c_str());
Expand All @@ -843,11 +849,11 @@ static void formatDiagnosticArgument(StringRef Modifier,
llvm::raw_svector_ostream OutAka(AkaText);

getAkaTypeForDisplay(type)->print(OutAka, printOptions);
Out << llvm::format(FormatOpts.AKAFormatString.c_str(),
Out << llvm::format(TypeFormatOpts->AKAFormatString.c_str(),
typeName.c_str(), AkaText.c_str());
} else {
Out << FormatOpts.OpeningQuotationMark << typeName
<< FormatOpts.ClosingQuotationMark;
Out << TypeFormatOpts->OpeningQuotationMark << typeName
<< TypeFormatOpts->ClosingQuotationMark;
}
}
break;
Expand Down
2 changes: 2 additions & 0 deletions lib/Sema/TypeCheckGeneric.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ void TypeChecker::checkProtocolSelfRequirements(ValueDecl *decl) {
req.getFirstType()->is<GenericTypeParamType>())
continue;

static_assert((unsigned)RequirementKind::LAST_KIND == 4,
"update %select in diagnostic!");
ctx.Diags.diagnose(decl, diag::requirement_restricts_self, decl,
req.getFirstType().getString(),
static_cast<unsigned>(req.getKind()),
Expand Down
45 changes: 43 additions & 2 deletions lib/Sema/TypeCheckInvertible.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,51 @@ static void checkInvertibleConformanceCommon(DeclContext *dc,
if (conformance.isConcrete()) {
auto concrete = conformance.getConcrete();
if (auto *normalConf = dyn_cast<NormalProtocolConformance>(concrete)) {
hasUnconditionalConformance =
normalConf->getConditionalRequirements().empty();
conformanceLoc = normalConf->getLoc();
assert(conformanceLoc);

auto condReqs = normalConf->getConditionalRequirements();
hasUnconditionalConformance = condReqs.empty();
auto *thisProto = normalConf->getProtocol();

// Ensure that conditional conformance to an invertible protocol IP only
// depends conformance requirements involving IP, and its subject is not
// a dependent member type.
//
// In theory, it could depend on any invertible protocol, but it may be
// confusing if we permitted that and this simplifies the model a bit.
for (auto req : condReqs) {
Type illegalSecondType;

// If we are diagnosing, fill-in the second-type string of this req.
switch (req.getKind()) {
case RequirementKind::Layout:
assert(req.getLayoutConstraint()->isClass());
illegalSecondType = ctx.getAnyObjectType();
break;
case RequirementKind::Conformance:
if (req.getProtocolDecl() == thisProto
&& !req.getFirstType()->is<DependentMemberType>())
break; // permitted, don't fill-in.
LLVM_FALLTHROUGH;
case RequirementKind::Superclass:
case RequirementKind::SameType:
case RequirementKind::SameShape:
illegalSecondType = req.getSecondType();
break;
}

static_assert((unsigned)RequirementKind::LAST_KIND == 4,
"update %select in diagnostic!");
if (illegalSecondType) {
auto t = ctx.Diags.diagnose(conformanceLoc,
diag::inverse_cannot_be_conditional_on_requirement,
thisProto,
req.getFirstType(),
static_cast<unsigned>(req.getKind()),
illegalSecondType);
}
}
}
}
assert(!conformance.isPack() && "not handled");
Expand Down
39 changes: 39 additions & 0 deletions test/Generics/inverse_conditional_conformance_restriction.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// RUN: %target-typecheck-verify-swift \
// RUN: -enable-experimental-feature NoncopyableGenerics \
// RUN: -enable-experimental-feature NonescapableTypes \
// RUN: -enable-experimental-feature SuppressedAssociatedTypes

protocol P {}
protocol Q {}
class DoggoClass {}

struct Blah<T: ~Copyable & ~Escapable>: ~Copyable, ~Escapable {}
extension Blah: Copyable {} // expected-error {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'T: Escapable'}}
extension Blah: Escapable {} // expected-error {{conditional conformance to suppressible protocol 'Escapable' cannot depend on 'T: Copyable'}}

struct Yuck<T: ~Copyable & ~Escapable>: ~Copyable, ~Escapable {}
extension Yuck: Copyable where T: ~Escapable {}
extension Yuck: Escapable where T: ~Copyable {}

struct TryConformance<Whatever: ~Copyable>: ~Copyable {}
extension TryConformance: Copyable
where Whatever: P, Whatever: Q, Whatever: Sendable {}
// expected-error@-2 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Whatever: P'}}
// expected-error@-3 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Whatever: Q'}}
// expected-error@-4 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Whatever: Sendable'}}

struct TrySameType<Whatever: ~Copyable>: ~Copyable {}
extension TrySameType: Copyable
where Whatever == Int {}
// expected-error@-2 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Whatever == Int'}}

struct TryClassAndLayoutConstraints<Whatever: ~Copyable, Heckin>: ~Copyable {}
extension TryClassAndLayoutConstraints: Copyable
where Heckin: DoggoClass, Whatever: AnyObject {}
// expected-error@-2 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Whatever: AnyObject'}}
// expected-error@-3 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Heckin: DoggoClass'}}

protocol Queue: ~Copyable { associatedtype Job: ~Copyable }
struct Scheduler<Q: Queue>: ~Copyable {}
extension Scheduler: Copyable where Q.Job: Copyable {}
// expected-error@-1 {{conditional conformance to suppressible protocol 'Copyable' cannot depend on 'Q.Job: Copyable'}}
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ extension Outer: Copyable {}
extension Outer.InnerStruct: Copyable {}

extension Outer.InnerVariation1: Copyable {}
extension Outer.InnerVariation2: Escapable {}
extension Outer.InnerVariation2: Escapable where A: ~Copyable {}

extension Outer.InnerStruct {
public func hello<T: ~Escapable>(_ t: T) {}
Expand Down
2 changes: 1 addition & 1 deletion test/ModuleInterface/noncopyable_generics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ import NoncopyableGenerics_Misc
// CHECK-MISC: #endif

// CHECK-MISC: #if compiler(>=5.3) && $NoncopyableGenerics
// CHECK-MISC-NEXT: extension {{.*}}.Outer.InnerVariation2 : Swift.Escapable {
// CHECK-MISC-NEXT: extension {{.*}}.Outer.InnerVariation2 : Swift.Escapable where A : ~Copyable {
// CHECK-MISC: #endif

// CHECK-MISC: #if compiler(>=5.3) && $NoncopyableGenerics
Expand Down