Skip to content

[Sema] Remove lookupBoolType #22488

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.

Sign up for GitHub

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
Feb 12, 2019
Merged

[Sema] Remove lookupBoolType #22488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 7 additions & 1 deletion lib/Sema/CSApply.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8013,6 +8013,12 @@ Expr *Solution::convertOptionalToBool(Expr *expr,
// Match the optional value against its `Some` case.
auto &ctx = tc.Context;
auto isSomeExpr = new (ctx) EnumIsCaseExpr(expr, ctx.getOptionalSomeDecl());
cs.setType(isSomeExpr, tc.lookupBoolType(cs.DC));
auto boolDecl = ctx.getBoolDecl();

if (!boolDecl) {
tc.diagnose(SourceLoc(), diag::broken_bool);
}

cs.setType(isSomeExpr, boolDecl ? boolDecl->getDeclaredType() : Type());
return isSomeExpr;
}
9 changes: 8 additions & 1 deletion lib/Sema/CSGen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2800,7 +2800,14 @@ namespace {
CS.getConstraintLocator(expr));

// The result is Bool.
return CS.getTypeChecker().lookupBoolType(CS.DC);
auto boolDecl = tc.Context.getBoolDecl();

if (!boolDecl) {
tc.diagnose(SourceLoc(), diag::broken_bool);
return Type();
}

return boolDecl->getDeclaredType();
}

Type visitDiscardAssignmentExpr(DiscardAssignmentExpr *expr) {
Expand Down
23 changes: 0 additions & 23 deletions lib/Sema/TypeChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,29 +231,6 @@ ModuleDecl *TypeChecker::getStdlibModule(const DeclContext *dc) {
return StdlibModule;
}

Type TypeChecker::lookupBoolType(const DeclContext *dc) {
if (!boolType) {
boolType = ([&] {
SmallVector<ValueDecl *, 2> results;
getStdlibModule(dc)->lookupValue({}, Context.getIdentifier("Bool"),
NLKind::QualifiedLookup, results);
if (results.size() != 1) {
diagnose(SourceLoc(), diag::broken_bool);
return Type();
}

auto tyDecl = dyn_cast<NominalTypeDecl>(results.front());
if (!tyDecl) {
diagnose(SourceLoc(), diag::broken_bool);
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're not going to bother diagnosing this anymore, you can remove the diagnostic too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use this diagnostic elsewhere in TypeCheckDeclObjC and with Bool.init(_builtinBooleanLiteral:) in CSApply

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although arguably the last one could be its own diagnostic. These broken diagnostics are something I want to improve in the future however.

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 the 'broken stdlib' diagnostics should become fatal. Parts of the compiler don't recover well from these and assert later, so we may as well just exit early when it happens.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you suggesting something like llvm_unreachable for these 'broken stdlib' diagnostics?

Copy link
Member

Choose a reason for hiding this comment

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

I definitely have run into cases where the failures are pretty terrible and at times you just get weird things like the literal doesn't conform to equatable or some equally odd until you look through and find special look ups in the compiler.

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 think for this patch I'll go ahead and add back the diagnostic, but in a follow up I'll fix up these broken diagnostics to be fatal.

return Type();
}

return tyDecl->getDeclaredType();
})();
}
return *boolType;
}

/// Bind the given extension to the given nominal type.
static void bindExtensionToNominal(ExtensionDecl *ext,
NominalTypeDecl *nominal) {
Expand Down
3 changes: 0 additions & 3 deletions lib/Sema/TypeChecker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1848,9 +1848,6 @@ class TypeChecker final : public LazyResolver {
/// operator \c name appended to the expression.
Expr *findLHS(DeclContext *DC, Expr *E, Identifier name);

/// Look up the Bool type in the standard library.
Type lookupBoolType(const DeclContext *dc);

/// @}

/// \name Overload resolution
Expand Down