sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
There are a few alternative ways to expose this.
The most obvious is a RK_Pattern, but it seems to have some triggering problems.
Consider a completion "case Color::Red"
- we want the user input to match Red
- we *don't* want the user input to match Color::
- if "case" is displayed, users will expect to be able to match it.
It's not possible to satisfy all of these:
- [TypedText:case] [Text:Color::][TypedText:Red] has a bad getTypedText()
- [Text:case] [Text:Color::][TypedText:Red] shows "case" but you can't type it
- [TypedText:case Color::TypedText:Red] will match against the qualifier
(I suspect the third is the least bad here)
The option explored here is to provide a regular Declaration completion, but
with "case" inserted as a fix-it. This results in "case" being neither displayed
or matched, but inserted. (If the user types "case", the existing completions
kick in.)
Curious what you think. I'm biased as I'm working on an integration that
supports fixits but not patterns :-)
Repository:
rC Clang
https://reviews.llvm.org/D63098
Files:
lib/Sema/SemaCodeComplete.cpp
test/CodeCompletion/enum-switch-case.cpp
Index: test/CodeCompletion/enum-switch-case.cpp
===================================================================
--- test/CodeCompletion/enum-switch-case.cpp
+++ test/CodeCompletion/enum-switch-case.cpp
@@ -18,11 +18,13 @@
case N::Yellow:
break;
- case
+ case N::Green:
// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:21:8 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
// CHECK-CC1: Blue : [#N::Color#]N::Blue
// CHECK-CC1-NEXT: Green : [#N::Color#]N::Green
// CHECK-CC1-NEXT: Indigo : [#N::Color#]N::Indigo
// CHECK-CC1-NEXT: Orange : [#N::Color#]N::Orange
// CHECK-CC1-NEXT: Violet : [#N::Color#]N::Violet
-
+ V
+ // RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits -code-completion-at=%s:28:4 %s -o - | FileCheck -check-prefix=CHECK-UNQUAL %s
+ // CHECK-UNQUAL: Violet : [#N::Color#]N::Violet (requires fix-it: {28:3-28:3} to "case ")
Index: lib/Sema/SemaCodeComplete.cpp
===================================================================
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -18,6 +18,7 @@
#include "clang/AST/QualTypeNames.h"
#include "clang/AST/Type.h"
#include "clang/Basic/CharInfo.h"
+#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/Specifiers.h"
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/MacroInfo.h"
@@ -1984,6 +1985,81 @@
}
}
+namespace {
+/// Information that allows to avoid completing redundant enumerators.
+struct CoveredEnumerators {
+ llvm::SmallPtrSet<EnumConstantDecl *, 8> Seen;
+ NestedNameSpecifier *SuggestedQualifier = nullptr;
+
+ // Determine which enumerators we have already seen in the switch statement.
+ // FIXME: Ideally, we would also be able to look *past* the code-completion
+ // token, in case we are code-completing in the middle of the switch and not
+ // at the end. However, we aren't able to do so at the moment.
+ static CoveredEnumerators gather(SwitchStmt *Switch) {
+ CoveredEnumerators Enumerators;
+ for (SwitchCase *SC = Switch->getSwitchCaseList(); SC;
+ SC = SC->getNextSwitchCase()) {
+ CaseStmt *Case = dyn_cast<CaseStmt>(SC);
+ if (!Case)
+ continue;
+
+ Expr *CaseVal = Case->getLHS()->IgnoreParenCasts();
+ if (auto *DRE = dyn_cast<DeclRefExpr>(CaseVal))
+ if (auto *Enumerator = dyn_cast<EnumConstantDecl>(DRE->getDecl())) {
+ // We look into the AST of the case statement to determine which
+ // enumerator was named. Alternatively, we could compute the value of
+ // the integral constant expression, then compare it against the
+ // values of each enumerator. However, value-based approach would not
+ // work as well with C++ templates where enumerators declared within a
+ // template are type- and value-dependent.
+ Enumerators.Seen.insert(Enumerator);
+
+ // If this is a qualified-id, keep track of the nested-name-specifier
+ // so that we can reproduce it as part of code completion, e.g.,
+ //
+ // switch (TagD.getKind()) {
+ // case TagDecl::TK_enum:
+ // break;
+ // case XXX
+ //
+ // At the XXX, our completions are TagDecl::TK_union,
+ // TagDecl::TK_struct, and TagDecl::TK_class, rather than TK_union,
+ // TK_struct, and TK_class.
+ Enumerators.SuggestedQualifier = DRE->getQualifier();
+ }
+ }
+ return Enumerators;
+ }
+};
+} // namespace
+
+static void AddEnumerators(ResultBuilder &Results, ASTContext &Context,
+ EnumDecl *Enum, DeclContext *CurContext,
+ const CoveredEnumerators &Enumerators,
+ llvm::Optional<FixItHint> FixIt = llvm::None) {
+ NestedNameSpecifier *Qualifier = Enumerators.SuggestedQualifier;
+ if (Context.getLangOpts().CPlusPlus && !Qualifier && Enumerators.Seen.empty()) {
+ // If there are no prior enumerators in C++, check whether we have to
+ // qualify the names of the enumerators that we suggest, because they
+ // may not be visible in this scope.
+ Qualifier = getRequiredQualification(Context, CurContext, Enum);
+ }
+
+ CodeCompletionBuilder Builder(Results.getAllocator(),
+ Results.getCodeCompletionTUInfo());
+ Results.EnterNewScope();
+ for (auto *E : Enum->enumerators()) {
+ if (Enumerators.Seen.count(E))
+ continue;
+
+ CodeCompletionResult R(E, CCP_EnumInCase, Qualifier);
+ if (FixIt)
+ R.FixIts.push_back(*FixIt);
+ Results.AddResult(R, CurContext, nullptr, false);
+ }
+ Results.ExitScope();
+}
+
/// Add language constructs that show up for "ordinary" names.
static void AddOrdinaryNameResults(Sema::ParserCompletionContext CCC, Scope *S,
Sema &SemaRef, ResultBuilder &Results) {
@@ -2206,6 +2282,24 @@
// Switch-specific statements.
if (SemaRef.getCurFunction() &&
!SemaRef.getCurFunction()->SwitchStack.empty()) {
+ EnumDecl *EnumDef = nullptr;
+ SwitchStmt *Switch =
+ SemaRef.getCurFunction()->SwitchStack.back().getPointer();
+ if (Switch->getCond()) {
+ QualType SwitchType = Switch->getCond()->IgnoreImplicit()->getType();
+ if (SwitchType->isEnumeralType())
+ EnumDef = SwitchType->castAs<EnumType>()->getDecl()->getDefinition();
+ }
+ if (EnumDef && SemaRef.CodeCompleter->includeFixIts()) {
+ // case <value>
+ AddEnumerators(
+ Results, SemaRef.Context, EnumDef, SemaRef.CurContext,
+ CoveredEnumerators::gather(Switch),
+ FixItHint::CreateInsertion(SemaRef.getPreprocessor()
+ .getCodeCompletionTokenRange()
+ .getBegin(),
+ "case "));
+ }
// case expression:
Builder.AddTypedTextChunk("case");
Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
@@ -4225,36 +4319,6 @@
SmallVector<Decl *, 4> IgnoreDecls;
};
-namespace {
-/// Information that allows to avoid completing redundant enumerators.
-struct CoveredEnumerators {
- llvm::SmallPtrSet<EnumConstantDecl *, 8> Seen;
- NestedNameSpecifier *SuggestedQualifier = nullptr;
-};
-} // namespace
-
-static void AddEnumerators(ResultBuilder &Results, ASTContext &Context,
- EnumDecl *Enum, DeclContext *CurContext,
- const CoveredEnumerators &Enumerators) {
- NestedNameSpecifier *Qualifier = Enumerators.SuggestedQualifier;
- if (Context.getLangOpts().CPlusPlus && !Qualifier && Enumerators.Seen.empty()) {
- // If there are no prior enumerators in C++, check whether we have to
- // qualify the names of the enumerators that we suggest, because they
- // may not be visible in this scope.
- Qualifier = getRequiredQualification(Context, CurContext, Enum);
- }
-
- Results.EnterNewScope();
- for (auto *E : Enum->enumerators()) {
- if (Enumerators.Seen.count(E))
- continue;
-
- CodeCompletionResult R(E, CCP_EnumInCase, Qualifier);
- Results.AddResult(R, CurContext, nullptr, false);
- }
- Results.ExitScope();
-}
-
/// Try to find a corresponding FunctionProtoType for function-like types (e.g.
/// function pointers, std::function, etc).
static const FunctionProtoType *TryDeconstructFunctionLike(QualType T) {
@@ -4978,49 +5042,12 @@
if (EnumDecl *Def = Enum->getDefinition())
Enum = Def;
- // Determine which enumerators we have already seen in the switch statement.
- // FIXME: Ideally, we would also be able to look *past* the code-completion
- // token, in case we are code-completing in the middle of the switch and not
- // at the end. However, we aren't able to do so at the moment.
- CoveredEnumerators Enumerators;
- for (SwitchCase *SC = Switch->getSwitchCaseList(); SC;
- SC = SC->getNextSwitchCase()) {
- CaseStmt *Case = dyn_cast<CaseStmt>(SC);
- if (!Case)
- continue;
-
- Expr *CaseVal = Case->getLHS()->IgnoreParenCasts();
- if (auto *DRE = dyn_cast<DeclRefExpr>(CaseVal))
- if (auto *Enumerator =
- dyn_cast<EnumConstantDecl>(DRE->getDecl())) {
- // We look into the AST of the case statement to determine which
- // enumerator was named. Alternatively, we could compute the value of
- // the integral constant expression, then compare it against the
- // values of each enumerator. However, value-based approach would not
- // work as well with C++ templates where enumerators declared within a
- // template are type- and value-dependent.
- Enumerators.Seen.insert(Enumerator);
-
- // If this is a qualified-id, keep track of the nested-name-specifier
- // so that we can reproduce it as part of code completion, e.g.,
- //
- // switch (TagD.getKind()) {
- // case TagDecl::TK_enum:
- // break;
- // case XXX
- //
- // At the XXX, our completions are TagDecl::TK_union,
- // TagDecl::TK_struct, and TagDecl::TK_class, rather than TK_union,
- // TK_struct, and TK_class.
- Enumerators.SuggestedQualifier = DRE->getQualifier();
- }
- }
-
// Add any enumerators that have not yet been mentioned.
ResultBuilder Results(*this, CodeCompleter->getAllocator(),
CodeCompleter->getCodeCompletionTUInfo(),
CodeCompletionContext::CCC_Expression);
- AddEnumerators(Results, Context, Enum, CurContext, Enumerators);
+ AddEnumerators(Results, Context, Enum, CurContext,
+ CoveredEnumerators::gather(Switch));
if (CodeCompleter->includeMacros()) {
AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
