-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Diagnostics][Sema] warn inconstructible enum #16987
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -169,6 +169,8 @@ class CircularityChecker { | |
bool diagnoseInfiniteRecursion(CanType parentType, ValueDecl *member, | ||
CanType memberType); | ||
|
||
void diagnoseNonWellFoundedEnum(EnumDecl *E); | ||
|
||
void addPathElementsTo(Path &path, CanType type); | ||
void addPathElement(Path &path, ValueDecl *member, CanType memberType); | ||
|
||
|
@@ -272,7 +274,11 @@ bool CircularityChecker::expandStruct(CanType type, StructDecl *S, | |
bool CircularityChecker::expandEnum(CanType type, EnumDecl *E, | ||
unsigned depth) { | ||
// Indirect enums are representational leaves. | ||
if (E->isIndirect()) return false; | ||
if (E->isIndirect()) { | ||
// Diagnose whether the enum is non-well-founded before bailing | ||
diagnoseNonWellFoundedEnum(E); | ||
return false; | ||
} | ||
|
||
startExpandingType(type); | ||
|
||
|
@@ -298,6 +304,7 @@ bool CircularityChecker::expandEnum(CanType type, EnumDecl *E, | |
if (addMember(type, elt, eltType, depth)) | ||
return true; | ||
} | ||
diagnoseNonWellFoundedEnum(E); | ||
|
||
return false; | ||
} | ||
|
@@ -601,3 +608,44 @@ bool CircularityChecker::diagnoseInfiniteRecursion(CanType parentType, | |
|
||
return true; | ||
} | ||
|
||
/// Show a warning if all cases of the given enum are recursive, | ||
/// making it impossible to be instantiated. Such an enum is 'non-well-founded'. | ||
/// The outcome of this method is irrelevant. | ||
void CircularityChecker::diagnoseNonWellFoundedEnum(EnumDecl *E) { | ||
|
||
auto containsType = [](TupleType *tuple, Type E) -> bool { | ||
for (auto type: tuple->getElementTypes()) { | ||
if (type->isEqual(E)) | ||
return true; | ||
} | ||
return false; | ||
}; | ||
auto isNonWellFounded = [containsType, E]() -> bool { | ||
auto elts = E->getAllElements(); | ||
if (elts.empty()) | ||
return false; | ||
|
||
for (auto elt: elts) { | ||
if (!elt->hasInterfaceType() || | ||
!(elt->isIndirect() || E->isIndirect())) | ||
return false; | ||
|
||
auto argTy = elt->getArgumentInterfaceType(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can simplify this and handle nested uninhabited tuples by using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bit unexpected for me, can you explain why a tuple containing an uninhabited type is uninhabited? I also don't understand how this can help to simplify. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
enum Foo {
case bar(Never)
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I understand correctly now, uninhabited means 'not constructible'. So you can't construct a tuple that has an empty enum, because you can't instantiate an empty enum. The first thing that comes to mind when I see uninhabited is a type with no explicit members. Maybe implicit as well. Well, actually, if a type has neither explicit nor implicit members, it is inconstructible. But it's still a somewhat confusing way to put it for me. Do I correctly understand that you want me to firstly check if the argument is uninhabited so that such cases are also counted in favor of inconstructability? The diagnostic would then be misleading the way it is now. It can be changed then to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @CodaFi But, I'm don't think it's a good idea to warn about 'uninhabited' cases. Similar to how we don't warn about empty enums. WDYT? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @CodaFi Using the terminology Slava gave, I don't think we should warn about uninhabited types (The issue faces non-well-founded types, or unconstructibly infinite types – the way @jckarter describes them in the report). enum Foo1 { case A(Never), case B(Foo1) } // uninhabited
enum Foo2 { case A(Int, Foo2), case B(Foo2) // non-well-founded |
||
if (!argTy) | ||
return false; | ||
|
||
if (auto tuple = argTy->getAs<TupleType>()) { | ||
if (!containsType(tuple, E->getSelfInterfaceType())) | ||
return false; | ||
} else if (auto paren = dyn_cast<ParenType>(argTy.getPointer())) { | ||
if (!E->getSelfInterfaceType()->isEqual(paren->getUnderlyingType())) | ||
return false; | ||
} | ||
} | ||
return true; | ||
}; | ||
|
||
if (isNonWellFounded()) | ||
TC.diagnose(E, diag::enum_non_well_founded); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you assert that
RequireDelayedChecking
got tripped if the element has no interface type?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might not get tripped because I have a call to the method in the fast path before the loop in
expandEnum
where it is set.