Skip to content

[5.5][completion] Clarify and simplify not-recommended state #36964

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 2 commits into from
Apr 20, 2021
Merged

[5.5][completion] Clarify and simplify not-recommended state #36964

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4dca0c6
[completion] Clarify and simplify not-recommended state
benlangmuir Apr 19, 2021
dbd2dd4
[completion] Make NotRecommendedReason an enum class
benlangmuir Apr 19, 2021
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
46 changes: 22 additions & 24 deletions include/swift/IDE/CodeCompletion.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,21 +599,21 @@ class CodeCompletionResult {
Identical,
};

enum NotRecommendedReason {
Redundant,
enum class NotRecommendedReason {
None = 0,
RedundantImport,
Deprecated,
InvalidContext,
InvalidAsyncContext,
CrossActorReference,
NoReason,
VariableUsedInOwnDefinition,
};

private:
unsigned Kind : 3;
unsigned AssociatedKind : 8;
unsigned KnownOperatorKind : 6;
unsigned SemanticContext : 3;
unsigned NotRecommended : 1;
unsigned NotRecReason : 3;
unsigned NotRecommended : 4;
unsigned IsSystem : 1;

/// The number of bytes to the left of the code completion point that
Expand Down Expand Up @@ -644,11 +644,10 @@ class CodeCompletionResult {
CodeCompletionOperatorKind::None,
StringRef BriefDocComment = StringRef())
: Kind(Kind), KnownOperatorKind(unsigned(KnownOperatorKind)),
SemanticContext(unsigned(SemanticContext)), NotRecommended(false),
NotRecReason(NotRecommendedReason::NoReason),
SemanticContext(unsigned(SemanticContext)),
NotRecommended(unsigned(NotRecommendedReason::None)),
NumBytesToErase(NumBytesToErase), CompletionString(CompletionString),
BriefDocComment(BriefDocComment),
TypeDistance(TypeDistance) {
BriefDocComment(BriefDocComment), TypeDistance(TypeDistance) {
assert(Kind != Declaration && "use the other constructor");
assert(CompletionString);
if (isOperator() && KnownOperatorKind == CodeCompletionOperatorKind::None)
Expand All @@ -670,8 +669,8 @@ class CodeCompletionResult {
ExpectedTypeRelation TypeDistance,
StringRef BriefDocComment = StringRef())
: Kind(Keyword), KnownOperatorKind(0),
SemanticContext(unsigned(SemanticContext)), NotRecommended(false),
NotRecReason(NotRecommendedReason::NoReason),
SemanticContext(unsigned(SemanticContext)),
NotRecommended(unsigned(NotRecommendedReason::None)),
NumBytesToErase(NumBytesToErase), CompletionString(CompletionString),
BriefDocComment(BriefDocComment), TypeDistance(TypeDistance) {
assert(CompletionString);
Expand All @@ -688,8 +687,8 @@ class CodeCompletionResult {
CodeCompletionString *CompletionString,
ExpectedTypeRelation TypeDistance)
: Kind(Literal), KnownOperatorKind(0),
SemanticContext(unsigned(SemanticContext)), NotRecommended(false),
NotRecReason(NotRecommendedReason::NoReason),
SemanticContext(unsigned(SemanticContext)),
NotRecommended(unsigned(NotRecommendedReason::None)),
NumBytesToErase(NumBytesToErase), CompletionString(CompletionString),
TypeDistance(TypeDistance) {
AssociatedKind = static_cast<unsigned>(LiteralKind);
Expand All @@ -706,15 +705,14 @@ class CodeCompletionResult {
unsigned NumBytesToErase,
CodeCompletionString *CompletionString,
const Decl *AssociatedDecl, StringRef ModuleName,
bool NotRecommended,
CodeCompletionResult::NotRecommendedReason NotRecReason,
StringRef BriefDocComment,
ArrayRef<StringRef> AssociatedUSRs,
ArrayRef<std::pair<StringRef, StringRef>> DocWords,
enum ExpectedTypeRelation TypeDistance)
: Kind(ResultKind::Declaration), KnownOperatorKind(0),
SemanticContext(unsigned(SemanticContext)),
NotRecommended(NotRecommended), NotRecReason(NotRecReason),
NotRecommended(unsigned(NotRecReason)),
NumBytesToErase(NumBytesToErase), CompletionString(CompletionString),
ModuleName(ModuleName), BriefDocComment(BriefDocComment),
AssociatedUSRs(AssociatedUSRs), DocWords(DocWords),
Expand All @@ -735,7 +733,7 @@ class CodeCompletionResult {
unsigned NumBytesToErase,
CodeCompletionString *CompletionString,
CodeCompletionDeclKind DeclKind, bool IsSystem,
StringRef ModuleName, bool NotRecommended,
StringRef ModuleName,
CodeCompletionResult::NotRecommendedReason NotRecReason,
StringRef BriefDocComment,
ArrayRef<StringRef> AssociatedUSRs,
Expand All @@ -745,11 +743,11 @@ class CodeCompletionResult {
: Kind(ResultKind::Declaration),
KnownOperatorKind(unsigned(KnownOperatorKind)),
SemanticContext(unsigned(SemanticContext)),
NotRecommended(NotRecommended), NotRecReason(NotRecReason),
IsSystem(IsSystem), NumBytesToErase(NumBytesToErase),
CompletionString(CompletionString), ModuleName(ModuleName),
BriefDocComment(BriefDocComment), AssociatedUSRs(AssociatedUSRs),
DocWords(DocWords), TypeDistance(TypeDistance) {
NotRecommended(unsigned(NotRecReason)), IsSystem(IsSystem),
NumBytesToErase(NumBytesToErase), CompletionString(CompletionString),
ModuleName(ModuleName), BriefDocComment(BriefDocComment),
AssociatedUSRs(AssociatedUSRs), DocWords(DocWords),
TypeDistance(TypeDistance) {
AssociatedKind = static_cast<unsigned>(DeclKind);
assert(CompletionString);
assert(!isOperator() ||
Expand Down Expand Up @@ -800,15 +798,15 @@ class CodeCompletionResult {
}

NotRecommendedReason getNotRecommendedReason() const {
return static_cast<NotRecommendedReason>(NotRecReason);
return static_cast<NotRecommendedReason>(NotRecommended);
}

SemanticContextKind getSemanticContext() const {
return static_cast<SemanticContextKind>(SemanticContext);
}

bool isNotRecommended() const {
return NotRecommended;
return getNotRecommendedReason() != NotRecommendedReason::None;
}

unsigned getNumBytesToErase() const {
Expand Down
48 changes: 21 additions & 27 deletions lib/IDE/CodeCompletion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ using namespace swift;
using namespace ide;

using CommandWordsPairs = std::vector<std::pair<StringRef, StringRef>>;
using NotRecommendedReason = CodeCompletionResult::NotRecommendedReason;

enum CodeCompletionCommandKind {
none,
Expand Down Expand Up @@ -819,7 +820,7 @@ void CodeCompletionResultBuilder::setAssociatedDecl(const Decl *D) {
}

if (D->getAttrs().getDeprecated(D->getASTContext()))
setNotRecommended(CodeCompletionResult::Deprecated);
setNotRecommended(NotRecommendedReason::Deprecated);
}

namespace {
Expand Down Expand Up @@ -1294,8 +1295,7 @@ CodeCompletionResult *CodeCompletionResultBuilder::takeResult() {

return new (*Sink.Allocator) CodeCompletionResult(
SemanticContext, NumBytesToErase, CCS, AssociatedDecl, ModuleName,
/*NotRecommended=*/IsNotRecommended, NotRecReason,
copyString(*Sink.Allocator, BriefComment),
NotRecReason, copyString(*Sink.Allocator, BriefComment),
copyAssociatedUSRs(*Sink.Allocator, AssociatedDecl),
copyArray(*Sink.Allocator, CommentWords), ExpectedTypeRelation);
}
Expand Down Expand Up @@ -2090,8 +2090,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
Builder.addBaseName(MD->getNameStr());
Builder.addTypeAnnotation("Module");
if (Pair.second)
Builder.setNotRecommended(CodeCompletionResult::NotRecommendedReason::
Redundant);
Builder.setNotRecommended(NotRecommendedReason::RedundantImport);
}
}

Expand All @@ -2115,9 +2114,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
}
}

void addModuleName(
ModuleDecl *MD,
Optional<CodeCompletionResult::NotRecommendedReason> R = None) {
void addModuleName(ModuleDecl *MD, Optional<NotRecommendedReason> R = None) {

// Don't add underscored cross-import overlay modules.
if (MD->getDeclaringModuleIfCrossImportOverlay())
Expand Down Expand Up @@ -2151,11 +2148,11 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
continue;

auto MD = ModuleDecl::create(ModuleName, Ctx);
Optional<CodeCompletionResult::NotRecommendedReason> Reason = None;
Optional<NotRecommendedReason> Reason = None;

// Imported modules are not recommended.
if (ImportedModules.count(MD->getNameStr()) != 0)
Reason = CodeCompletionResult::NotRecommendedReason::Redundant;
Reason = NotRecommendedReason::RedundantImport;

addModuleName(MD, Reason);
}
Expand Down Expand Up @@ -2518,9 +2515,8 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
return Type();
}

void analyzeActorIsolation(
const ValueDecl *VD, Type T, bool &implicitlyAsync,
Optional<CodeCompletionResult::NotRecommendedReason> &NotRecommended) {
void analyzeActorIsolation(const ValueDecl *VD, Type T, bool &implicitlyAsync,
Optional<NotRecommendedReason> &NotRecommended) {
auto isolation = getActorIsolation(const_cast<ValueDecl *>(VD));

switch (isolation.getKind()) {
Expand All @@ -2544,19 +2540,19 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
if (implicitlyAsync && T) {
if (isa<VarDecl>(VD)) {
if (!isSendableType(CurrDeclContext, T)) {
NotRecommended = CodeCompletionResult::CrossActorReference;
NotRecommended = NotRecommendedReason::CrossActorReference;
}
} else {
assert(isa<FuncDecl>(VD) || isa<SubscriptDecl>(VD));
// Check if the result and the param types are all 'Sendable'.
auto *AFT = T->castTo<AnyFunctionType>();
if (!isSendableType(CurrDeclContext, AFT->getResult())) {
NotRecommended = CodeCompletionResult::CrossActorReference;
NotRecommended = NotRecommendedReason::CrossActorReference;
} else {
for (auto &param : AFT->getParams()) {
Type paramType = param.getPlainType();
if (!isSendableType(CurrDeclContext, paramType)) {
NotRecommended = CodeCompletionResult::CrossActorReference;
NotRecommended = NotRecommendedReason::CrossActorReference;
break;
}
}
Expand All @@ -2577,18 +2573,18 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
if (VD->hasInterfaceType())
VarType = getTypeOfMember(VD, dynamicLookupInfo);

Optional<CodeCompletionResult::NotRecommendedReason> NotRecommended;
Optional<NotRecommendedReason> NotRecommended;
// "not recommended" in its own getter.
if (Kind == LookupKind::ValueInDeclContext) {
if (auto accessor = dyn_cast<AccessorDecl>(CurrDeclContext)) {
if (accessor->getStorage() == VD && accessor->isGetter())
NotRecommended = CodeCompletionResult::NoReason;
NotRecommended = NotRecommendedReason::VariableUsedInOwnDefinition;
}
}
bool implicitlyAsync = false;
analyzeActorIsolation(VD, VarType, implicitlyAsync, NotRecommended);
if (!NotRecommended && implicitlyAsync && !CanCurrDeclContextHandleAsync) {
NotRecommended = CodeCompletionResult::InvalidContext;
NotRecommended = NotRecommendedReason::InvalidAsyncContext;
}

CommandWordsPairs Pairs;
Expand Down Expand Up @@ -2929,8 +2925,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
addTypeAnnotation(Builder, AFT->getResult(), genericSig);

if (AFT->isAsync() && !CanCurrDeclContextHandleAsync) {
Builder.setNotRecommended(
CodeCompletionResult::NotRecommendedReason::InvalidContext);
Builder.setNotRecommended(NotRecommendedReason::InvalidAsyncContext);
}
};

Expand Down Expand Up @@ -3039,14 +3034,14 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
if (AFT && !IsImplicitlyCurriedInstanceMethod)
trivialTrailingClosure = hasTrivialTrailingClosure(FD, AFT);

Optional<CodeCompletionResult::NotRecommendedReason> NotRecommended;
Optional<NotRecommendedReason> NotRecommended;
bool implictlyAsync = false;
analyzeActorIsolation(FD, AFT, implictlyAsync, NotRecommended);

if (!NotRecommended && !IsImplicitlyCurriedInstanceMethod &&
((AFT && AFT->isAsync()) || implictlyAsync) &&
!CanCurrDeclContextHandleAsync) {
NotRecommended = CodeCompletionResult::InvalidContext;
NotRecommended = NotRecommendedReason::InvalidAsyncContext;
}

// Add the method, possibly including any default arguments.
Expand Down Expand Up @@ -3244,8 +3239,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
}

if (ConstructorType->isAsync() && !CanCurrDeclContextHandleAsync) {
Builder.setNotRecommended(
CodeCompletionResult::NotRecommendedReason::InvalidContext);
Builder.setNotRecommended(NotRecommendedReason::InvalidAsyncContext);
}
};

Expand Down Expand Up @@ -3291,12 +3285,12 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
if (!subscriptType)
return;

Optional<CodeCompletionResult::NotRecommendedReason> NotRecommended;
Optional<NotRecommendedReason> NotRecommended;
bool implictlyAsync = false;
analyzeActorIsolation(SD, subscriptType, implictlyAsync, NotRecommended);

if (!NotRecommended && implictlyAsync && !CanCurrDeclContextHandleAsync) {
NotRecommended = CodeCompletionResult::InvalidContext;
NotRecommended = NotRecommendedReason::InvalidAsyncContext;
}

CommandWordsPairs Pairs;
Expand Down
12 changes: 7 additions & 5 deletions lib/IDE/CodeCompletionCache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ static bool readCachedModule(llvm::MemoryBuffer *in,
auto declKind = static_cast<CodeCompletionDeclKind>(*cursor++);
auto opKind = static_cast<CodeCompletionOperatorKind>(*cursor++);
auto context = static_cast<SemanticContextKind>(*cursor++);
auto notRecommended = static_cast<bool>(*cursor++);
auto notRecommended =
static_cast<CodeCompletionResult::NotRecommendedReason>(*cursor++);
auto isSystem = static_cast<bool>(*cursor++);
auto numBytesToErase = static_cast<unsigned>(*cursor++);
auto oldCursor = cursor;
Expand Down Expand Up @@ -228,9 +229,10 @@ static bool readCachedModule(llvm::MemoryBuffer *in,
if (kind == CodeCompletionResult::Declaration) {
result = new (*V.Sink.Allocator) CodeCompletionResult(
context, numBytesToErase, string, declKind, isSystem, moduleName,
notRecommended, CodeCompletionResult::NotRecommendedReason::NoReason,
briefDocComment, copyArray(*V.Sink.Allocator, ArrayRef<StringRef>(assocUSRs)),
copyArray(*V.Sink.Allocator, ArrayRef<std::pair<StringRef, StringRef>>(declKeywords)),
notRecommended, briefDocComment,
copyArray(*V.Sink.Allocator, ArrayRef<StringRef>(assocUSRs)),
copyArray(*V.Sink.Allocator,
ArrayRef<std::pair<StringRef, StringRef>>(declKeywords)),
CodeCompletionResult::Unknown, opKind);
} else {
result = new (*V.Sink.Allocator)
Expand Down Expand Up @@ -349,7 +351,7 @@ static void writeCachedModule(llvm::raw_ostream &out,
else
LE.write(static_cast<uint8_t>(CodeCompletionOperatorKind::None));
LE.write(static_cast<uint8_t>(R->getSemanticContext()));
LE.write(static_cast<uint8_t>(R->isNotRecommended()));
LE.write(static_cast<uint8_t>(R->getNotRecommendedReason()));
LE.write(static_cast<uint8_t>(R->isSystem()));
LE.write(static_cast<uint8_t>(R->getNumBytesToErase()));
LE.write(
Expand Down
4 changes: 1 addition & 3 deletions lib/IDE/CodeCompletionResultBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,8 @@ class CodeCompletionResultBuilder {
CodeCompletionResult::Unknown;
bool Cancelled = false;
ArrayRef<std::pair<StringRef, StringRef>> CommentWords;
bool IsNotRecommended = false;
CodeCompletionResult::NotRecommendedReason NotRecReason =
CodeCompletionResult::NotRecommendedReason::NoReason;
CodeCompletionResult::NotRecommendedReason::None;
StringRef BriefDocComment = StringRef();

void addChunkWithText(CodeCompletionString::Chunk::ChunkKind Kind,
Expand Down Expand Up @@ -148,7 +147,6 @@ class CodeCompletionResultBuilder {
void setLiteralKind(CodeCompletionLiteralKind kind) { LiteralKind = kind; }
void setKeywordKind(CodeCompletionKeywordKind kind) { KeywordKind = kind; }
void setNotRecommended(CodeCompletionResult::NotRecommendedReason Reason) {
IsNotRecommended = true;
NotRecReason = Reason;
}

Expand Down
7 changes: 3 additions & 4 deletions tools/SourceKit/lib/SwiftLang/CodeCompletionOrganizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1169,10 +1169,9 @@ Completion *CompletionBuilder::finish() {
base = SwiftResult(
semanticContext, current.getNumBytesToErase(), completionString,
current.getAssociatedDeclKind(), current.isSystem(),
current.getModuleName(), current.isNotRecommended(),
current.getNotRecommendedReason(), current.getBriefDocComment(),
current.getAssociatedUSRs(), current.getDeclKeywords(),
typeRelation, opKind);
current.getModuleName(), current.getNotRecommendedReason(),
current.getBriefDocComment(), current.getAssociatedUSRs(),
current.getDeclKeywords(), typeRelation, opKind);
} else {
base = SwiftResult(current.getKind(), semanticContext,
current.getNumBytesToErase(), completionString,
Expand Down