Sema: Late code review feedback from @jrose-apple #27016
Conversation
|
@swift-ci Please smoke test |
lib/AST/ModuleNameLookup.cpp
Outdated
| [](ValueDecl *VD) { return !isa<FuncDecl>(VD); }) | ||
| == decls.end()) | ||
| if (std::none_of(decls.begin() + initialCount, decls.end(), | ||
| [](ValueDecl *VD) { return isa<FuncDecl>(VD); }) |
There was a problem hiding this comment.
You inverted the logic here. "fail to find not-a-FuncDecl" is "none are not FuncDecls" or "all are FuncDecls".
There was a problem hiding this comment.
Yeah, oops. I think all_of() is best here.
lib/Sema/TypeCheckAttr.cpp
Outdated
| @@ -1617,7 +1617,7 @@ void AttributeChecker::checkApplicationMainAttribute(DeclAttribute *attr, | |||
| namelookup::lookupInModule(KitModule, Id_ApplicationDelegate, | |||
| decls, NLKind::QualifiedLookup, | |||
| namelookup::ResolutionKind::TypesOnly, | |||
| KitModule); | |||
| nullptr); | |||
There was a problem hiding this comment.
I thought you defaulted this argument, but if not can you give it an argument label comment?
There was a problem hiding this comment.
Actually we don't default it, and it can't be null. We assert if it is. I kept this behavior from the old implementation.
slavapestov
force-pushed
the
review-feedback-for-module-lookup
branch
from
d9e09ba to
82c0436
Compare
slavapestov
force-pushed
the
review-feedback-for-module-lookup
branch
from
82c0436 to
08e5045
Compare
|
@swift-ci Please smoke test |
| for (const auto &result : lookupString) { | ||
| TC.validateDecl(result.getValueDecl()); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we still need these so that IRGen can access them, no? Or is it requestified enough for that to not be a problem?
There was a problem hiding this comment.
I think the latter, but I think these are imported declarations so validateDecl() has no effect anyway?
Addressing the post-commit review on #26865.