Skip to content

NFC: [CodeCompletion] Use enums in bit fields as-is #40163

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 4 commits into from
Dec 21, 2021
Merged

NFC: [CodeCompletion] Use enums in bit fields as-is #40163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
94a372e
[CodeCompletion] Use enums in bit fields as-is
rintaro Nov 12, 2021
f8e10bb
[CodeCompletion] Add static_assert to limit the max values of enums
rintaro Nov 16, 2021
f17671c
Guard a static assert for MSVC
rintaro Dec 3, 2021
c1542bf
[SourceKit] Guard static assert by _MSC_VER
ahoppen Dec 21, 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
165 changes: 85 additions & 80 deletions include/swift/IDE/CodeCompletion.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ class alignas(detail::CodeCompletionStringChunk) CodeCompletionString final :
///
/// This enum is ordered from the contexts that are "nearest" to the code
/// completion point to "outside" contexts.
enum class SemanticContextKind {
enum class SemanticContextKind : uint8_t {
/// Used in cases when the concept of semantic context is not applicable.
None,

Expand Down Expand Up @@ -452,6 +452,8 @@ enum class SemanticContextKind {

/// A declaration imported from other module.
OtherModule,

MAX_VALUE = OtherModule
};

enum class CodeCompletionFlairBit: uint8_t {
Expand Down Expand Up @@ -482,7 +484,7 @@ enum class CodeCompletionFlairBit: uint8_t {
using CodeCompletionFlair = OptionSet<CodeCompletionFlairBit>;

/// The declaration kind of a code completion result, if it is a declaration.
enum class CodeCompletionDeclKind {
enum class CodeCompletionDeclKind : uint8_t {
Module,
Class,
Struct,
Expand All @@ -508,7 +510,7 @@ enum class CodeCompletionDeclKind {
PrecedenceGroup,
};

enum class CodeCompletionLiteralKind {
enum class CodeCompletionLiteralKind : uint8_t {
ArrayLiteral,
BooleanLiteral,
ColorLiteral,
Expand All @@ -520,7 +522,7 @@ enum class CodeCompletionLiteralKind {
Tuple,
};

enum class CodeCompletionOperatorKind {
enum class CodeCompletionOperatorKind : uint8_t {
None,
Unknown,
Bang, // !
Expand Down Expand Up @@ -564,16 +566,18 @@ enum class CodeCompletionOperatorKind {
PipeEq, // |=
PipePipe, // ||
TildeEq, // ~=

MAX_VALUE = TildeEq
};

enum class CodeCompletionKeywordKind {
enum class CodeCompletionKeywordKind : uint8_t {
None,
#define KEYWORD(X) kw_##X,
#define POUND_KEYWORD(X) pound_##X,
#include "swift/Syntax/TokenKinds.def"
};

enum class CompletionKind {
enum class CompletionKind : uint8_t {
None,
Import,
UnresolvedMember,
Expand Down Expand Up @@ -612,30 +616,34 @@ enum class CompletionKind {
TypeAttrBeginning,
};

enum class CodeCompletionDiagnosticSeverity: uint8_t {
enum class CodeCompletionDiagnosticSeverity : uint8_t {
None,
Error,
Warning,
Remark,
Note,

MAX_VALUE = Note
};

/// A single code completion result.
class CodeCompletionResult {
friend class CodeCompletionResultBuilder;

public:
enum class ResultKind {
enum class ResultKind : uint8_t {
Declaration,
Keyword,
Pattern,
Literal,
BuiltinOperator,

MAX_VALUE = BuiltinOperator
};

/// Describes the relationship between the type of the completion results and
/// the expected type at the code completion position.
enum class ExpectedTypeRelation {
enum class ExpectedTypeRelation : uint8_t {
/// The result does not have a type (e.g. keyword).
NotApplicable,

Expand All @@ -654,9 +662,11 @@ class CodeCompletionResult {

/// The result's type is identical to the type of the expected.
Identical,

MAX_VALUE = Identical
};

enum class NotRecommendedReason {
enum class NotRecommendedReason : uint8_t {
None = 0,
RedundantImport,
RedundantImportIndirect,
Expand All @@ -665,16 +675,18 @@ class CodeCompletionResult {
InvalidAsyncContext,
CrossActorReference,
VariableUsedInOwnDefinition,

MAX_VALUE = VariableUsedInOwnDefinition
};

private:
unsigned Kind : 3;
ResultKind Kind : 3;
unsigned AssociatedKind : 8;
unsigned KnownOperatorKind : 6;
unsigned SemanticContext : 3;
unsigned Flair: 8;
unsigned NotRecommended : 4;
unsigned IsSystem : 1;
CodeCompletionOperatorKind KnownOperatorKind : 6;
SemanticContextKind SemanticContext : 3;
unsigned char Flair : 8;
NotRecommendedReason NotRecommended : 4;
Copy link
Member

Choose a reason for hiding this comment

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

Do we have static_asserts that e.g. NotRecommendedReason doesn’t contain more then 2^4 elements?

Copy link
Member Author

@rintaro rintaro Nov 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

No. Good idea. I added a commit.

bool IsSystem : 1;
Copy link
Member

Choose a reason for hiding this comment

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

Not really related to this PR but in libSyntax’s RawSyntax, I saw measurable performance improvements when not compressing bitfields into the minimum number of bits but having them byte-aligned. I think this was because the CPU doesn’t need to do bitshifts/masks to retrieve the value.

The bits inside RawSyntax were probably accessed a lot more frequently than these, but it might be worth investigating if byte-aligning these might also be beneficial performance-wise, even though it increases the memory footprint.


/// The number of bytes to the left of the code completion point that
/// should be erased first if this completion string is inserted in the
Expand All @@ -689,10 +701,18 @@ class CodeCompletionResult {
StringRef ModuleName;
StringRef BriefDocComment;
ArrayRef<StringRef> AssociatedUSRs;
unsigned TypeDistance : 3;
unsigned DiagnosticSeverity: 3;
ExpectedTypeRelation TypeDistance : 3;
CodeCompletionDiagnosticSeverity DiagnosticSeverity : 3;
StringRef DiagnosticMessage;

// Assertions for limiting max values of enums.
static_assert(int(ResultKind::MAX_VALUE) < 1 << 3, "");
static_assert(int(CodeCompletionOperatorKind::MAX_VALUE) < 1 << 6, "");
static_assert(int(SemanticContextKind::MAX_VALUE) < 1 << 3, "");
static_assert(int(NotRecommendedReason::MAX_VALUE) < 1 << 4, "");
static_assert(int(ExpectedTypeRelation::MAX_VALUE) < 1 << 3, "");
static_assert(int(CodeCompletionDiagnosticSeverity::MAX_VALUE) < 1 << 3, "");

public:
/// Constructs a \c Pattern, \c Keyword or \c BuiltinOperator result.
///
Expand All @@ -704,22 +724,20 @@ class CodeCompletionResult {
CodeCompletionOperatorKind KnownOperatorKind =
CodeCompletionOperatorKind::None,
StringRef BriefDocComment = StringRef())
: Kind(unsigned(Kind)), KnownOperatorKind(unsigned(KnownOperatorKind)),
SemanticContext(unsigned(SemanticContext)),
Flair(unsigned(Flair.toRaw())),
NotRecommended(unsigned(NotRecommendedReason::None)),
: Kind(Kind), KnownOperatorKind(KnownOperatorKind),
SemanticContext(SemanticContext), Flair(Flair.toRaw()),
NotRecommended(NotRecommendedReason::None),
NumBytesToErase(NumBytesToErase), CompletionString(CompletionString),
BriefDocComment(BriefDocComment), TypeDistance(unsigned(TypeDistance)) {
BriefDocComment(BriefDocComment), TypeDistance(TypeDistance) {
assert(Kind != ResultKind::Declaration && "use the other constructor");
assert(CompletionString);
if (isOperator() && KnownOperatorKind == CodeCompletionOperatorKind::None)
this->KnownOperatorKind =
(unsigned)getCodeCompletionOperatorKind(CompletionString);
this->KnownOperatorKind = getCodeCompletionOperatorKind(CompletionString);
assert(!isOperator() ||
getOperatorKind() != CodeCompletionOperatorKind::None);
AssociatedKind = 0;
IsSystem = 0;
DiagnosticSeverity = 0;
IsSystem = false;
DiagnosticSeverity = CodeCompletionDiagnosticSeverity::None;
}

/// Constructs a \c Keyword result.
Expand All @@ -731,16 +749,16 @@ class CodeCompletionResult {
CodeCompletionString *CompletionString,
ExpectedTypeRelation TypeDistance,
StringRef BriefDocComment = StringRef())
: Kind(unsigned(ResultKind::Keyword)), KnownOperatorKind(0),
SemanticContext(unsigned(SemanticContext)),
Flair(unsigned(Flair.toRaw())),
NotRecommended(unsigned(NotRecommendedReason::None)),
: Kind(ResultKind::Keyword),
KnownOperatorKind(CodeCompletionOperatorKind::None),
SemanticContext(SemanticContext), Flair(Flair.toRaw()),
NotRecommended(NotRecommendedReason::None),
NumBytesToErase(NumBytesToErase), CompletionString(CompletionString),
BriefDocComment(BriefDocComment), TypeDistance(unsigned(TypeDistance)) {
BriefDocComment(BriefDocComment), TypeDistance(TypeDistance) {
assert(CompletionString);
AssociatedKind = static_cast<unsigned>(Kind);
IsSystem = 0;
DiagnosticSeverity = 0;
IsSystem = false;
DiagnosticSeverity = CodeCompletionDiagnosticSeverity::None;
}

/// Constructs a \c Literal result.
Expand All @@ -751,15 +769,15 @@ class CodeCompletionResult {
CodeCompletionFlair Flair, unsigned NumBytesToErase,
CodeCompletionString *CompletionString,
ExpectedTypeRelation TypeDistance)
: Kind(unsigned(ResultKind::Literal)), KnownOperatorKind(0),
SemanticContext(unsigned(SemanticContext)),
Flair(unsigned(Flair.toRaw())),
NotRecommended(unsigned(NotRecommendedReason::None)),
: Kind(ResultKind::Literal),
KnownOperatorKind(CodeCompletionOperatorKind::None),
SemanticContext(SemanticContext), Flair(Flair.toRaw()),
NotRecommended(NotRecommendedReason::None),
NumBytesToErase(NumBytesToErase), CompletionString(CompletionString),
TypeDistance(unsigned(TypeDistance)) {
TypeDistance(TypeDistance) {
AssociatedKind = static_cast<unsigned>(LiteralKind);
IsSystem = 0;
DiagnosticSeverity = 0;
IsSystem = false;
DiagnosticSeverity = CodeCompletionDiagnosticSeverity::None;
assert(CompletionString);
}

Expand All @@ -776,20 +794,20 @@ class CodeCompletionResult {
StringRef BriefDocComment,
ArrayRef<StringRef> AssociatedUSRs,
ExpectedTypeRelation TypeDistance)
: Kind(unsigned(ResultKind::Declaration)), KnownOperatorKind(0),
SemanticContext(unsigned(SemanticContext)),
Flair(unsigned(Flair.toRaw())), NotRecommended(unsigned(NotRecReason)),
NumBytesToErase(NumBytesToErase), CompletionString(CompletionString),
ModuleName(ModuleName), BriefDocComment(BriefDocComment),
AssociatedUSRs(AssociatedUSRs), TypeDistance(unsigned(TypeDistance)) {
: Kind(ResultKind::Declaration),
KnownOperatorKind(CodeCompletionOperatorKind::None),
SemanticContext(SemanticContext), Flair(Flair.toRaw()),
NotRecommended(NotRecReason), NumBytesToErase(NumBytesToErase),
CompletionString(CompletionString), ModuleName(ModuleName),
BriefDocComment(BriefDocComment), AssociatedUSRs(AssociatedUSRs),
TypeDistance(TypeDistance) {
assert(AssociatedDecl && "should have a decl");
AssociatedKind = unsigned(getCodeCompletionDeclKind(AssociatedDecl));
IsSystem = getDeclIsSystem(AssociatedDecl);
DiagnosticSeverity = 0;
DiagnosticSeverity = CodeCompletionDiagnosticSeverity::None;
assert(CompletionString);
if (isOperator())
KnownOperatorKind =
(unsigned)getCodeCompletionOperatorKind(CompletionString);
KnownOperatorKind = getCodeCompletionOperatorKind(CompletionString);
assert(!isOperator() ||
getOperatorKind() != CodeCompletionOperatorKind::None);
}
Expand All @@ -806,16 +824,13 @@ class CodeCompletionResult {
ArrayRef<StringRef> AssociatedUSRs,
ExpectedTypeRelation TypeDistance,
CodeCompletionOperatorKind KnownOperatorKind)
: Kind(unsigned(ResultKind::Declaration)),
KnownOperatorKind(unsigned(KnownOperatorKind)),
SemanticContext(unsigned(SemanticContext)),
Flair(unsigned(Flair.toRaw())), NotRecommended(unsigned(NotRecReason)),
IsSystem(IsSystem), NumBytesToErase(NumBytesToErase),
CompletionString(CompletionString), ModuleName(ModuleName),
BriefDocComment(BriefDocComment), AssociatedUSRs(AssociatedUSRs),
TypeDistance(unsigned(TypeDistance)),
DiagnosticSeverity(unsigned(diagSeverity)),
DiagnosticMessage(DiagnosticMessage) {
: Kind(ResultKind::Declaration), KnownOperatorKind(KnownOperatorKind),
SemanticContext(SemanticContext), Flair(Flair.toRaw()),
NotRecommended(NotRecReason), IsSystem(IsSystem),
NumBytesToErase(NumBytesToErase), CompletionString(CompletionString),
ModuleName(ModuleName), BriefDocComment(BriefDocComment),
AssociatedUSRs(AssociatedUSRs), TypeDistance(TypeDistance),
DiagnosticSeverity(diagSeverity), DiagnosticMessage(DiagnosticMessage) {
AssociatedKind = static_cast<unsigned>(DeclKind);
assert(CompletionString);
assert(!isOperator() ||
Expand All @@ -828,7 +843,7 @@ class CodeCompletionResult {
CodeCompletionResult *withFlair(CodeCompletionFlair newFlair,
CodeCompletionResultSink &Sink);

ResultKind getKind() const { return static_cast<ResultKind>(Kind); }
ResultKind getKind() const { return Kind; }

CodeCompletionDeclKind getAssociatedDeclKind() const {
assert(getKind() == ResultKind::Declaration);
Expand Down Expand Up @@ -860,33 +875,23 @@ class CodeCompletionResult {

CodeCompletionOperatorKind getOperatorKind() const {
assert(isOperator());
return static_cast<CodeCompletionOperatorKind>(KnownOperatorKind);
return KnownOperatorKind;
}

bool isSystem() const {
return static_cast<bool>(IsSystem);
}
bool isSystem() const { return IsSystem; }

ExpectedTypeRelation getExpectedTypeRelation() const {
return static_cast<ExpectedTypeRelation>(TypeDistance);
}
ExpectedTypeRelation getExpectedTypeRelation() const { return TypeDistance; }

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

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

CodeCompletionFlair getFlair() const {
return static_cast<CodeCompletionFlair>(Flair);
}
CodeCompletionFlair getFlair() const { return CodeCompletionFlair(Flair); }

/// Modify "flair" of this result *in place*.
void setFlair(CodeCompletionFlair flair) {
Flair = unsigned(flair.toRaw());
}
void setFlair(CodeCompletionFlair flair) { Flair = flair.toRaw(); }

bool isNotRecommended() const {
return getNotRecommendedReason() != NotRecommendedReason::None;
Expand All @@ -911,12 +916,12 @@ class CodeCompletionResult {
}

void setDiagnostics(CodeCompletionDiagnosticSeverity severity, StringRef message) {
DiagnosticSeverity = static_cast<unsigned>(severity);
DiagnosticSeverity = severity;
DiagnosticMessage = message;
}

CodeCompletionDiagnosticSeverity getDiagnosticSeverity() const {
return static_cast<CodeCompletionDiagnosticSeverity>(DiagnosticSeverity);
return DiagnosticSeverity;
}

StringRef getDiagnosticMessage() const {
Expand Down
2 changes: 1 addition & 1 deletion lib/IDE/CodeCompletion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ void CodeCompletionResult::printPrefix(raw_ostream &OS) const {
PRINT_FLAIR(ExpressionAtNonScriptOrMainFileScope, "ExprAtFileScope")
Prefix.append("]");
}
if (NotRecommended)
if (NotRecommended != NotRecommendedReason::None)
Prefix.append("/NotRecommended");
if (IsSystem)
Prefix.append("/IsSystem");
Expand Down
2 changes: 1 addition & 1 deletion lib/IDE/ExprContextAnalysis.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class Expr;
class ValueDecl;

namespace ide {
enum class SemanticContextKind;
enum class SemanticContextKind : uint8_t;

/// Type check parent contexts of the given decl context, and the body of the
/// given context until \c Loc if the context is a function body.
Expand Down
6 changes: 6 additions & 0 deletions tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "swift/AST/DiagnosticsParse.h"
#include "swift/AST/DiagnosticsFrontend.h"
#include "swift/AST/DiagnosticsSIL.h"
#include "swift/Basic/Compiler.h"
#include "swift/Basic/SourceManager.h"
#include "swift/Demangling/ManglingUtils.h"
#include "swift/Frontend/Frontend.h"
Expand Down Expand Up @@ -608,7 +609,12 @@ struct SwiftSemanticToken {
return SwiftLangSupport::getUIDForCodeCompletionDeclKind(Kind, getIsRef());
}
};
#if !defined(_MSC_VER)
static_assert(sizeof(SwiftSemanticToken) == 8, "Too big");
// FIXME: MSVC doesn't pack bitfields with different underlying types.
// Giving up to check this in MSVC for now, becasue static_assert is only for
// keeping low memory usage.
#endif

class SwiftDocumentSemanticInfo :
public ThreadSafeRefCountedBase<SwiftDocumentSemanticInfo> {
Expand Down
Loading