[PATCH] D38538: Avoid printing some redundant name qualifiers in completion
ilya-biryukov created this revision.
Herald added a subscriber: eraman.
Adjusted PrintingPolicy inside code completion to avoid printing some
redundant name qualifiers.
Before this change, typedefs that were written unqualified in source
code were printed with qualifiers in completion. For example, in the
following code
struct foo {
typedef int type;
type method();
};
completion item for `method` had return type of `foo::type`, even
though the original code used `type` without qualifiers.
After this change, the completion item has return type `type`, as
originally written in the source code.
Note that this change does not suppress qualifiers written by the
user. For example, in the following code
typedef int type;
struct foo {
typedef int type;
::type method(foo::type);
};
completion item for `method` has return type of `::type` and
parameter type of `foo::type`, as originally written in the source
code.
https://reviews.llvm.org/D38538
Files:
lib/Sema/SemaCodeComplete.cpp
test/CodeCompletion/call.cpp
test/CodeCompletion/enum-switch-case-qualified.cpp
test/CodeCompletion/enum-switch-case.cpp
test/CodeCompletion/qualifiers-as-written.cpp
test/CodeCompletion/uninstantiated_params.cpp
test/Index/code-completion.cpp
test/Index/complete-cxx-inline-methods.cpp
test/Index/complete-documentation-templates.cpp
Index: test/Index/complete-documentation-templates.cpp
===
--- test/Index/complete-documentation-templates.cpp
+++ test/Index/complete-documentation-templates.cpp
@@ -119,7 +119,7 @@
// CHECK-CC3: FieldDecl:{ResultType int}{TypedText T7}{{.*}}(brief comment: This is T7.)
// RUN: env CINDEXTEST_COMPLETION_BRIEF_COMMENTS=1 c-index-test -code-completion-at=%s:59:12 %s | FileCheck -check-prefix=CHECK-CC4 %s
-// CHECK-CC4: EnumConstantDecl:{ResultType T3::T9}{TypedText T10}{{.*}}(brief comment: This is T10.)
+// CHECK-CC4: EnumConstantDecl:{ResultType T9}{TypedText T10}{{.*}}(brief comment: This is T10.)
// FIXME: after we implement propagating comments through typedefs, this
// typedef for implicit instantiation should pick up the documentation
// comment from class template.
@@ -140,7 +140,7 @@
// RUN: env CINDEXTEST_COMPLETION_BRIEF_COMMENTS=1 c-index-test -code-completion-at=%s:105:20 %s | FileCheck -check-prefix=CHECK-CC7 %s
// CHECK-CC7: ClassDecl:{TypedText T105}{{.*}}(brief comment: This is T105.)
// CHECK-CC7: EnumDecl:{TypedText T106}{{.*}}(brief comment: This is T106.)
-// CHECK-CC7: EnumConstantDecl:{ResultType T100::T106}{TypedText T107}{{.*}}(brief comment: This is T107.)
+// CHECK-CC7: EnumConstantDecl:{ResultType T106}{TypedText T107}{{.*}}(brief comment: This is T107.)
// FIXME: after we implement propagating comments through typedefs, these two
// typedefs for implicit instantiations should pick up the documentation
// comment from class template.
Index: test/Index/complete-cxx-inline-methods.cpp
===
--- test/Index/complete-cxx-inline-methods.cpp
+++ test/Index/complete-cxx-inline-methods.cpp
@@ -25,7 +25,7 @@
// RUN: c-index-test -code-completion-at=%s:4:9 -std=c++98 %s | FileCheck %s
// RUN: c-index-test -code-completion-at=%s:13:7 -std=c++98 %s | FileCheck %s
-// CHECK: CXXMethod:{ResultType MyCls::Vec &}{TypedText operator=}{LeftParen (}{Placeholder const MyCls::Vec &}{RightParen )} (79)
+// CHECK: CXXMethod:{ResultType Vec &}{TypedText operator=}{LeftParen (}{Placeholder const Vec &}{RightParen )} (79)
// CHECK-NEXT: StructDecl:{TypedText Vec}{Text ::} (75)
// CHECK-NEXT: FieldDecl:{ResultType int}{TypedText x} (35)
// CHECK-NEXT: FieldDecl:{ResultType int}{TypedText y} (35)
Index: test/Index/code-completion.cpp
===
--- test/Index/code-completion.cpp
+++ test/Index/code-completion.cpp
@@ -55,7 +55,7 @@
// CHECK-MEMBER: CXXMethod:{ResultType Z &}{TypedText operator=}{LeftParen (}{Placeholder const Z &}{RightParen )}
// CHECK-MEMBER: CXXMethod:{ResultType X &}{Text X::}{TypedText operator=}{LeftParen (}{Placeholder const X &}{RightParen )}
// CHECK-MEMBER: CXXMethod:{ResultType Y &}{Text Y::}{TypedText operator=}{LeftParen (}{Placeholder const Y &}{RightParen )}
-// CHECK-MEMBER: EnumConstantDecl:{ResultType X::E}{Informative E::}{TypedText Val1}
+// CHECK-MEMBER: EnumConstantDecl:{ResultType E}{Informative E::}{TypedText Val1}
// CHECK-MEMBER: StructDecl:{TypedText X}{Text ::}
// CHECK-MEMBER: StructDecl:{TypedText Y}{Text ::}
// CHECK-MEMBER: StructDecl:{TypedText Z}{Text ::}
Index: test/CodeCompletion/uninstantiated_params.cpp
===
--- test/CodeCompletion/uninstantiated_params.cpp
+++ test/CodeCompletion/uninstantiated_params.cpp
@@ -9,5 +9,5 @@
unique_ptr x;
x.
// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:5 %s -o - | Fil
[PATCH] D38538: Avoid printing some redundant name qualifiers in completion
ilya-biryukov added inline comments. Comment at: test/CodeCompletion/enum-switch-case-qualified.cpp:25 // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:23:8 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s -// CHECK-CC1: Blue : [#M::N::C::Color#]N::C::Blue -// CHECK-CC1-NEXT: Green : [#M::N::C::Color#]N::C::Green -// CHECK-CC1-NEXT: Indigo : [#M::N::C::Color#]N::C::Indigo -// CHECK-CC1-NEXT: Orange : [#M::N::C::Color#]N::C::Orange -// CHECK-CC1-NEXT: Red : [#M::N::C::Color#]N::C::Red -// CHECK-CC1-NEXT: Violet : [#M::N::C::Color#]N::C::Violet -// CHECK-CC1: Yellow : [#M::N::C::Color#]N::C::Yellow +// CHECK-CC1: Blue : [#Color#]N::C::Blue +// CHECK-CC1-NEXT: Green : [#Color#]N::C::Green This may be a somewhat unwanted part of this change. Enum type is now written without qualifier here. I would argue that's ok, since the actual enum values are always properly qualified (they have to be, as they are actually inserted by completion) and those qualifiers provide all the necessary context for the user. https://reviews.llvm.org/D38538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38538: Avoid printing some redundant name qualifiers in completion
arphaman added inline comments. Comment at: test/CodeCompletion/call.cpp:22 // CHECK-CC1: COMPLETION: Pattern : dynamic_cast<<#type#>>(<#expression#>) - // CHECK-CC1: f(N::Y y, <#int ZZ#>) + // CHECK-CC1: f(Y y, <#int ZZ#>) // CHECK-CC1-NEXT: f(int i, <#int j#>, int k) Could we also test a similar class to `Y` that's not typedefed, so you'd see `f(N::Y)`? Comment at: test/CodeCompletion/enum-switch-case-qualified.cpp:25 // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:23:8 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s -// CHECK-CC1: Blue : [#M::N::C::Color#]N::C::Blue -// CHECK-CC1-NEXT: Green : [#M::N::C::Color#]N::C::Green -// CHECK-CC1-NEXT: Indigo : [#M::N::C::Color#]N::C::Indigo -// CHECK-CC1-NEXT: Orange : [#M::N::C::Color#]N::C::Orange -// CHECK-CC1-NEXT: Red : [#M::N::C::Color#]N::C::Red -// CHECK-CC1-NEXT: Violet : [#M::N::C::Color#]N::C::Violet -// CHECK-CC1: Yellow : [#M::N::C::Color#]N::C::Yellow +// CHECK-CC1: Blue : [#Color#]N::C::Blue +// CHECK-CC1-NEXT: Green : [#Color#]N::C::Green ilya-biryukov wrote: > This may be a somewhat unwanted part of this change. > Enum type is now written without qualifier here. I would argue that's ok, > since the actual enum values are always properly qualified (they have to be, > as they are actually inserted by completion) and those qualifiers provide all > the necessary context for the user. I'm not 100% comfortable with making this kind of change right now. I'll try to investigate what's best for our users. https://reviews.llvm.org/D38538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38538: Avoid printing some redundant name qualifiers in completion
ilya-biryukov updated this revision to Diff 118205.
ilya-biryukov added a comment.
- Included more cases into a qualifiers-as-written.cpp test.
https://reviews.llvm.org/D38538
Files:
include/clang/AST/QualTypeNames.h
include/clang/Tooling/Core/QualTypeNames.h
lib/AST/CMakeLists.txt
lib/AST/QualTypeNames.cpp
lib/Sema/SemaCodeComplete.cpp
lib/Tooling/Core/CMakeLists.txt
lib/Tooling/Core/QualTypeNames.cpp
test/CodeCompletion/call.cpp
test/CodeCompletion/qualifiers-as-written.cpp
test/CodeCompletion/uninstantiated_params.cpp
test/Index/complete-cxx-inline-methods.cpp
unittests/Tooling/QualTypeNamesTest.cpp
Index: unittests/Tooling/QualTypeNamesTest.cpp
===
--- unittests/Tooling/QualTypeNamesTest.cpp
+++ unittests/Tooling/QualTypeNamesTest.cpp
@@ -7,7 +7,7 @@
//
//===--===//
-#include "clang/Tooling/Core/QualTypeNames.h"
+#include "clang/AST/QualTypeNames.h"
#include "TestVisitor.h"
using namespace clang;
Index: test/Index/complete-cxx-inline-methods.cpp
===
--- test/Index/complete-cxx-inline-methods.cpp
+++ test/Index/complete-cxx-inline-methods.cpp
@@ -25,7 +25,7 @@
// RUN: c-index-test -code-completion-at=%s:4:9 -std=c++98 %s | FileCheck %s
// RUN: c-index-test -code-completion-at=%s:13:7 -std=c++98 %s | FileCheck %s
-// CHECK: CXXMethod:{ResultType MyCls::Vec &}{TypedText operator=}{LeftParen (}{Placeholder const MyCls::Vec &}{RightParen )} (79)
+// CHECK: CXXMethod:{ResultType Vec &}{TypedText operator=}{LeftParen (}{Placeholder const Vec &}{RightParen )} (79)
// CHECK-NEXT: StructDecl:{TypedText Vec}{Text ::} (75)
// CHECK-NEXT: FieldDecl:{ResultType int}{TypedText x} (35)
// CHECK-NEXT: FieldDecl:{ResultType int}{TypedText y} (35)
Index: test/CodeCompletion/uninstantiated_params.cpp
===
--- test/CodeCompletion/uninstantiated_params.cpp
+++ test/CodeCompletion/uninstantiated_params.cpp
@@ -9,5 +9,5 @@
unique_ptr x;
x.
// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:5 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
- // CHECK-CC1: [#void#]reset({#<#unique_ptr::pointer ptr = pointer()#>#})
+ // CHECK-CC1: [#void#]reset({#<#pointer ptr = pointer()#>#})
}
Index: test/CodeCompletion/qualifiers-as-written.cpp
===
--- /dev/null
+++ test/CodeCompletion/qualifiers-as-written.cpp
@@ -0,0 +1,30 @@
+struct foo {
+ typedef int type;
+
+ type method(type, foo::type, ::foo::type, ::foo::foo::type);
+};
+
+namespace ns {
+ struct bar {
+ };
+
+ struct baz {
+ };
+
+ int func(foo::type a, bar b, baz c);
+}
+
+typedef ns::bar bar;
+
+int func(foo a, bar b, ns::bar c, ns::baz d);
+using ns::func;
+
+void test() {
+ foo().method(0, 0, 0, 0);
+ // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:23:9 %s -o - | FileCheck %s --check-prefix=CHECK-1
+ // CHECK-1: COMPLETION: method : [#type#]method(<#type#>, <#foo::type#>, <#::foo::type#>, <#::foo::foo::type#>)
+ f
+ // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:26:3 %s -o - | FileCheck %s --check-prefix=CHECK-2
+ // CHECK-2: COMPLETION: func : [#int#]func(<#foo a#>, <#bar b#>, <#ns::bar c#>, <#ns::baz d#>
+ // CHECK-2: COMPLETION: func : [#int#]func(<#foo::type a#>, <#bar b#>, <#baz c#>
+}
Index: test/CodeCompletion/call.cpp
===
--- test/CodeCompletion/call.cpp
+++ test/CodeCompletion/call.cpp
@@ -19,10 +19,10 @@
f(Y(), 0, 0);
// RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:19:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
// CHECK-CC1: COMPLETION: Pattern : dynamic_cast<<#type#>>(<#expression#>)
- // CHECK-CC1: f(N::Y y, <#int ZZ#>)
+ // CHECK-CC1: f(Y y, <#int ZZ#>)
// CHECK-CC1-NEXT: f(int i, <#int j#>, int k)
// CHECK-CC1-NEXT: f(float x, <#float y#>)
// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:19:13 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
- // CHECK-CC2-NOT: f(N::Y y, int ZZ)
+ // CHECK-CC2-NOT: f(Y y, int ZZ)
// CHECK-CC2: f(int i, int j, <#int k#>)
}
Index: lib/Tooling/Core/CMakeLists.txt
===
--- lib/Tooling/Core/CMakeLists.txt
+++ lib/Tooling/Core/CMakeLists.txt
@@ -3,7 +3,6 @@
add_clang_library(clangToolingCore
Lookup.cpp
Replacement.cpp
- QualTypeNames.cpp
Diagnostic.cpp
LINK_LIBS
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -14,6 +14,7 @@
#include "clang/AST/DeclObjC.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/ExprObjC.h"
+#include "clang/AST/QualTypeNames.h"
#include "clang/Basic/Char
[PATCH] D38538: Avoid printing some redundant name qualifiers in completion
ilya-biryukov added inline comments. Comment at: test/CodeCompletion/call.cpp:22 // CHECK-CC1: COMPLETION: Pattern : dynamic_cast<<#type#>>(<#expression#>) - // CHECK-CC1: f(N::Y y, <#int ZZ#>) + // CHECK-CC1: f(Y y, <#int ZZ#>) // CHECK-CC1-NEXT: f(int i, <#int j#>, int k) arphaman wrote: > Could we also test a similar class to `Y` that's not typedefed, so you'd see > `f(N::Y)`? Not sure if you expect this to behave differently, but I've updated `qualifiers-as-written.cpp` to include a similar case. You could take a look and validate whether it matches your expected behavior. The general idea is to not add extra qualification in completion items, where possible, and prefer to show what was written in the source code. This is what currently happens for all names that had name-qualifiers. However, before this patch, unqualified names would printed with an added name qualifiers. This sometimes led to really bad results. One example is vector's `push_back` that roughly gets printed as (libstdc++ version) `push_back(std::vector>::value_type _Value)`. Whereas in the source code it's just `push_back(value_type _Value)`. https://reviews.llvm.org/D38538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38538: Avoid printing some redundant name qualifiers in completion
ilya-biryukov updated this revision to Diff 118185.
ilya-biryukov added a comment.
Herald added a subscriber: mgorny.
- Restore qualifiers for types of EnumConstantDecl.
https://reviews.llvm.org/D38538
Files:
include/clang/AST/QualTypeNames.h
include/clang/Tooling/Core/QualTypeNames.h
lib/AST/CMakeLists.txt
lib/AST/QualTypeNames.cpp
lib/Sema/SemaCodeComplete.cpp
lib/Tooling/Core/CMakeLists.txt
lib/Tooling/Core/QualTypeNames.cpp
test/CodeCompletion/call.cpp
test/CodeCompletion/qualifiers-as-written.cpp
test/CodeCompletion/uninstantiated_params.cpp
test/Index/complete-cxx-inline-methods.cpp
unittests/Tooling/QualTypeNamesTest.cpp
Index: unittests/Tooling/QualTypeNamesTest.cpp
===
--- unittests/Tooling/QualTypeNamesTest.cpp
+++ unittests/Tooling/QualTypeNamesTest.cpp
@@ -7,7 +7,7 @@
//
//===--===//
-#include "clang/Tooling/Core/QualTypeNames.h"
+#include "clang/AST/QualTypeNames.h"
#include "TestVisitor.h"
using namespace clang;
Index: test/Index/complete-cxx-inline-methods.cpp
===
--- test/Index/complete-cxx-inline-methods.cpp
+++ test/Index/complete-cxx-inline-methods.cpp
@@ -25,7 +25,7 @@
// RUN: c-index-test -code-completion-at=%s:4:9 -std=c++98 %s | FileCheck %s
// RUN: c-index-test -code-completion-at=%s:13:7 -std=c++98 %s | FileCheck %s
-// CHECK: CXXMethod:{ResultType MyCls::Vec &}{TypedText operator=}{LeftParen (}{Placeholder const MyCls::Vec &}{RightParen )} (79)
+// CHECK: CXXMethod:{ResultType Vec &}{TypedText operator=}{LeftParen (}{Placeholder const Vec &}{RightParen )} (79)
// CHECK-NEXT: StructDecl:{TypedText Vec}{Text ::} (75)
// CHECK-NEXT: FieldDecl:{ResultType int}{TypedText x} (35)
// CHECK-NEXT: FieldDecl:{ResultType int}{TypedText y} (35)
Index: test/CodeCompletion/uninstantiated_params.cpp
===
--- test/CodeCompletion/uninstantiated_params.cpp
+++ test/CodeCompletion/uninstantiated_params.cpp
@@ -9,5 +9,5 @@
unique_ptr x;
x.
// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:5 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
- // CHECK-CC1: [#void#]reset({#<#unique_ptr::pointer ptr = pointer()#>#})
+ // CHECK-CC1: [#void#]reset({#<#pointer ptr = pointer()#>#})
}
Index: test/CodeCompletion/qualifiers-as-written.cpp
===
--- /dev/null
+++ test/CodeCompletion/qualifiers-as-written.cpp
@@ -0,0 +1,11 @@
+struct foo {
+ typedef int type;
+
+ type method(type, foo::type, ::foo::type, ::foo::foo::type);
+};
+
+void test() {
+ foo().
+ // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:8:9 %s -o - | FileCheck %s
+ // CHECK: COMPLETION: method : [#type#]method(<#type#>, <#foo::type#>, <#::foo::type#>, <#::foo::foo::type#>)
+}
Index: test/CodeCompletion/call.cpp
===
--- test/CodeCompletion/call.cpp
+++ test/CodeCompletion/call.cpp
@@ -19,10 +19,10 @@
f(Y(), 0, 0);
// RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:19:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
// CHECK-CC1: COMPLETION: Pattern : dynamic_cast<<#type#>>(<#expression#>)
- // CHECK-CC1: f(N::Y y, <#int ZZ#>)
+ // CHECK-CC1: f(Y y, <#int ZZ#>)
// CHECK-CC1-NEXT: f(int i, <#int j#>, int k)
// CHECK-CC1-NEXT: f(float x, <#float y#>)
// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:19:13 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
- // CHECK-CC2-NOT: f(N::Y y, int ZZ)
+ // CHECK-CC2-NOT: f(Y y, int ZZ)
// CHECK-CC2: f(int i, int j, <#int k#>)
}
Index: lib/Tooling/Core/CMakeLists.txt
===
--- lib/Tooling/Core/CMakeLists.txt
+++ lib/Tooling/Core/CMakeLists.txt
@@ -3,7 +3,6 @@
add_clang_library(clangToolingCore
Lookup.cpp
Replacement.cpp
- QualTypeNames.cpp
Diagnostic.cpp
LINK_LIBS
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -14,6 +14,7 @@
#include "clang/AST/DeclObjC.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/ExprObjC.h"
+#include "clang/AST/QualTypeNames.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/MacroInfo.h"
@@ -1495,6 +1496,7 @@
Policy.AnonymousTagLocations = false;
Policy.SuppressStrongLifetime = true;
Policy.SuppressUnwrittenScope = true;
+ Policy.SuppressScope = true;
return Policy;
}
@@ -2137,9 +2139,10 @@
T = Method->getSendResultType(BaseType);
else
T = Method->getReturnType();
- } else if (const EnumConstantDecl *Enumerator = dyn_cast(ND))
+ } else if (const EnumConstantDecl *Enumerator = dy
[PATCH] D38538: Avoid printing some redundant name qualifiers in completion
ilya-biryukov added inline comments. Comment at: test/CodeCompletion/enum-switch-case-qualified.cpp:25 // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:23:8 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s -// CHECK-CC1: Blue : [#M::N::C::Color#]N::C::Blue -// CHECK-CC1-NEXT: Green : [#M::N::C::Color#]N::C::Green -// CHECK-CC1-NEXT: Indigo : [#M::N::C::Color#]N::C::Indigo -// CHECK-CC1-NEXT: Orange : [#M::N::C::Color#]N::C::Orange -// CHECK-CC1-NEXT: Red : [#M::N::C::Color#]N::C::Red -// CHECK-CC1-NEXT: Violet : [#M::N::C::Color#]N::C::Violet -// CHECK-CC1: Yellow : [#M::N::C::Color#]N::C::Yellow +// CHECK-CC1: Blue : [#Color#]N::C::Blue +// CHECK-CC1-NEXT: Green : [#Color#]N::C::Green arphaman wrote: > ilya-biryukov wrote: > > This may be a somewhat unwanted part of this change. > > Enum type is now written without qualifier here. I would argue that's ok, > > since the actual enum values are always properly qualified (they have to > > be, as they are actually inserted by completion) and those qualifiers > > provide all the necessary context for the user. > I'm not 100% comfortable with making this kind of change right now. I'll try > to investigate what's best for our users. I changed code to keep qualifiers for enums as is. (This required moving some code from `libTooling` to `AST` in order to reuse it, I'll create a separate review for that if you're ok with the overall change). https://reviews.llvm.org/D38538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38538: Avoid printing some redundant name qualifiers in completion
arphaman added inline comments. Comment at: test/CodeCompletion/qualifiers-as-written.cpp:29 + // CHECK-2: COMPLETION: func : [#int#]func(<#foo a#>, <#bar b#>, <#ns::bar c#>, <#ns::baz d#> + // CHECK-2: COMPLETION: func : [#int#]func(<#foo::type a#>, <#bar b#>, <#baz c#> +} Sorry, I see the issue now. However, I don't think that we'd like to change the signature for a function like this, as we'd still prefer to show `func (foo::type, ns::bar, ns::baz);` on this line. In Xcode we actually avoid the problem with `std::vector<>`s that you've pointed out entirely by using `value_type`. I'll check what our solution does. Btw, maybe using things like `value_type` is completely wrong (with or without the qualifier)? If we have `std::vector` shouldn't we rather show `push_back(int _Value)`, rather than the `value_type`? Perhaps this kind of change should be discussed with a wider community somehow to find out what's best for all users. https://reviews.llvm.org/D38538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38538: Avoid printing some redundant name qualifiers in completion
ilya-biryukov added inline comments.
Comment at: test/CodeCompletion/qualifiers-as-written.cpp:29
+ // CHECK-2: COMPLETION: func : [#int#]func(<#foo a#>, <#bar b#>, <#ns::bar
c#>, <#ns::baz d#>
+ // CHECK-2: COMPLETION: func : [#int#]func(<#foo::type a#>, <#bar b#>, <#baz
c#>
+}
arphaman wrote:
> Sorry, I see the issue now. However, I don't think that we'd like to change
> the signature for a function like this, as we'd still prefer to show `func
> (foo::type, ns::bar, ns::baz);` on this line.
>
> In Xcode we actually avoid the problem with `std::vector<>`s that you've
> pointed out entirely by using `value_type`. I'll check what our solution does.
>
> Btw, maybe using things like `value_type` is completely wrong (with or
> without the qualifier)? If we have `std::vector` shouldn't we rather
> show `push_back(int _Value)`, rather than the `value_type`? Perhaps this kind
> of change should be discussed with a wider community somehow to find out
> what's best for all users.
Using `int _Value` may be good in case of `vector`, but it would probably
loose typedefs, i.e. `vector` would still have `int`, which may be
undesirable.
Also, for `vector>`, the type inside `push_back` would probably end
up being `vector>`.
As for the current vs new behavior, I can totally see why you want to see more
context in completion items.
I would argue that the current code does not do a great job there, as it only
adds qualifiers to unqualified references. But the context may be missing for
qualified references as well.
For example, in the following code:
```
template >
struct vector {
typedef T value_type;
typename value_type::some_type foo();
value_type bar()
};
```
Completion item for `vector::bar` will have return type `vector>::value_type`. However, completion item for `vector>::foo` will have return type `value_type::some_type` (note
that no `vector>` qualifier was added).
I would also question the intent of the current implementation. The following
line suggest that adding those qualifers was not intended in the first place:
```
Policy.SuppressUnwrittenScope = true;
```
But that's just a wild guess, I may be completely wrong.
That being said, I don't think there is one right preference, different people
may prefer different options. We can surely discuss that in a wider community,
though.
Would you be open to adding an option for that in completion and keeping the
current behavior as a default?
https://reviews.llvm.org/D38538
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38538: Avoid printing some redundant name qualifiers in completion
ilya-biryukov added a comment. Friendly ping. Would it be okay to make the new behaviour optional? (Leaving old behaviour to be the default). https://reviews.llvm.org/D38538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38538: Avoid printing some redundant name qualifiers in completion
arphaman accepted this revision.
arphaman added inline comments.
This revision is now accepted and ready to land.
Comment at: test/CodeCompletion/qualifiers-as-written.cpp:29
+ // CHECK-2: COMPLETION: func : [#int#]func(<#foo a#>, <#bar b#>, <#ns::bar
c#>, <#ns::baz d#>
+ // CHECK-2: COMPLETION: func : [#int#]func(<#foo::type a#>, <#bar b#>, <#baz
c#>
+}
ilya-biryukov wrote:
> arphaman wrote:
> > Sorry, I see the issue now. However, I don't think that we'd like to change
> > the signature for a function like this, as we'd still prefer to show `func
> > (foo::type, ns::bar, ns::baz);` on this line.
> >
> > In Xcode we actually avoid the problem with `std::vector<>`s that you've
> > pointed out entirely by using `value_type`. I'll check what our solution
> > does.
> >
> > Btw, maybe using things like `value_type` is completely wrong (with or
> > without the qualifier)? If we have `std::vector` shouldn't we rather
> > show `push_back(int _Value)`, rather than the `value_type`? Perhaps this
> > kind of change should be discussed with a wider community somehow to find
> > out what's best for all users.
> Using `int _Value` may be good in case of `vector`, but it would
> probably loose typedefs, i.e. `vector` would still have `int`, which
> may be undesirable.
> Also, for `vector>`, the type inside `push_back` would probably
> end up being `vector>`.
>
>
> As for the current vs new behavior, I can totally see why you want to see
> more context in completion items.
>
> I would argue that the current code does not do a great job there, as it only
> adds qualifiers to unqualified references. But the context may be missing for
> qualified references as well.
> For example, in the following code:
> ```
> template >
> struct vector {
> typedef T value_type;
>
> typename value_type::some_type foo();
> value_type bar()
> };
> ```
>
> Completion item for `vector::bar` will have return type `vector std::allocator>::value_type`. However, completion item for `vector std::allocator>::foo` will have return type `value_type::some_type` (note
> that no `vector>` qualifier was added).
>
> I would also question the intent of the current implementation. The following
> line suggest that adding those qualifers was not intended in the first place:
> ```
> Policy.SuppressUnwrittenScope = true;
> ```
> But that's just a wild guess, I may be completely wrong.
>
>
> That being said, I don't think there is one right preference, different
> people may prefer different options. We can surely discuss that in a wider
> community, though.
>
> Would you be open to adding an option for that in completion and keeping the
> current behavior as a default?
>
>
Sorry about the delay.
I clarified Xcode's behavior - we actually didn't show the qualifiers for
typedefs and typealiases because of an older Clang, with ToT the behavior is as
you describe.
So this patch will remove qualifiers for structs and classes, like in the
`func` example here. I'm willing to accept it right now. We might have to
revisit this change at some point though. Let's also keep one behavior right
now, I don't think we need to diverge yet.
https://reviews.llvm.org/D38538
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38538: Avoid printing some redundant name qualifiers in completion
ilya-biryukov updated this revision to Diff 120016.
ilya-biryukov added a comment.
- Extracted the move of QualTypeNames to AST into a separate revision.
https://reviews.llvm.org/D38538
Files:
lib/Sema/SemaCodeComplete.cpp
test/CodeCompletion/call.cpp
test/CodeCompletion/qualifiers-as-written.cpp
test/CodeCompletion/uninstantiated_params.cpp
test/Index/complete-cxx-inline-methods.cpp
Index: test/Index/complete-cxx-inline-methods.cpp
===
--- test/Index/complete-cxx-inline-methods.cpp
+++ test/Index/complete-cxx-inline-methods.cpp
@@ -25,7 +25,7 @@
// RUN: c-index-test -code-completion-at=%s:4:9 -std=c++98 %s | FileCheck %s
// RUN: c-index-test -code-completion-at=%s:13:7 -std=c++98 %s | FileCheck %s
-// CHECK: CXXMethod:{ResultType MyCls::Vec &}{TypedText operator=}{LeftParen (}{Placeholder const MyCls::Vec &}{RightParen )} (79)
+// CHECK: CXXMethod:{ResultType Vec &}{TypedText operator=}{LeftParen (}{Placeholder const Vec &}{RightParen )} (79)
// CHECK-NEXT: StructDecl:{TypedText Vec}{Text ::} (75)
// CHECK-NEXT: FieldDecl:{ResultType int}{TypedText x} (35)
// CHECK-NEXT: FieldDecl:{ResultType int}{TypedText y} (35)
Index: test/CodeCompletion/uninstantiated_params.cpp
===
--- test/CodeCompletion/uninstantiated_params.cpp
+++ test/CodeCompletion/uninstantiated_params.cpp
@@ -9,5 +9,5 @@
unique_ptr x;
x.
// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:5 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
- // CHECK-CC1: [#void#]reset({#<#unique_ptr::pointer ptr = pointer()#>#})
+ // CHECK-CC1: [#void#]reset({#<#pointer ptr = pointer()#>#})
}
Index: test/CodeCompletion/qualifiers-as-written.cpp
===
--- /dev/null
+++ test/CodeCompletion/qualifiers-as-written.cpp
@@ -0,0 +1,30 @@
+struct foo {
+ typedef int type;
+
+ type method(type, foo::type, ::foo::type, ::foo::foo::type);
+};
+
+namespace ns {
+ struct bar {
+ };
+
+ struct baz {
+ };
+
+ int func(foo::type a, bar b, baz c);
+}
+
+typedef ns::bar bar;
+
+int func(foo a, bar b, ns::bar c, ns::baz d);
+using ns::func;
+
+void test() {
+ foo().method(0, 0, 0, 0);
+ // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:23:9 %s -o - | FileCheck %s --check-prefix=CHECK-1
+ // CHECK-1: COMPLETION: method : [#type#]method(<#type#>, <#foo::type#>, <#::foo::type#>, <#::foo::foo::type#>)
+ f
+ // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:26:3 %s -o - | FileCheck %s --check-prefix=CHECK-2
+ // CHECK-2: COMPLETION: func : [#int#]func(<#foo a#>, <#bar b#>, <#ns::bar c#>, <#ns::baz d#>
+ // CHECK-2: COMPLETION: func : [#int#]func(<#foo::type a#>, <#bar b#>, <#baz c#>
+}
Index: test/CodeCompletion/call.cpp
===
--- test/CodeCompletion/call.cpp
+++ test/CodeCompletion/call.cpp
@@ -19,10 +19,10 @@
f(Y(), 0, 0);
// RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:19:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
// CHECK-CC1: COMPLETION: Pattern : dynamic_cast<<#type#>>(<#expression#>)
- // CHECK-CC1: f(N::Y y, <#int ZZ#>)
+ // CHECK-CC1: f(Y y, <#int ZZ#>)
// CHECK-CC1-NEXT: f(int i, <#int j#>, int k)
// CHECK-CC1-NEXT: f(float x, <#float y#>)
// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:19:13 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
- // CHECK-CC2-NOT: f(N::Y y, int ZZ)
+ // CHECK-CC2-NOT: f(Y y, int ZZ)
// CHECK-CC2: f(int i, int j, <#int k#>)
}
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -14,6 +14,7 @@
#include "clang/AST/DeclObjC.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/ExprObjC.h"
+#include "clang/AST/QualTypeNames.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/MacroInfo.h"
@@ -1495,6 +1496,7 @@
Policy.AnonymousTagLocations = false;
Policy.SuppressStrongLifetime = true;
Policy.SuppressUnwrittenScope = true;
+ Policy.SuppressScope = true;
return Policy;
}
@@ -2137,9 +2139,10 @@
T = Method->getSendResultType(BaseType);
else
T = Method->getReturnType();
- } else if (const EnumConstantDecl *Enumerator = dyn_cast(ND))
+ } else if (const EnumConstantDecl *Enumerator = dyn_cast(ND)) {
T = Context.getTypeDeclType(cast(Enumerator->getDeclContext()));
- else if (isa(ND)) {
+T = clang::TypeName::getFullyQualifiedType(T, Context);
+ } else if (isa(ND)) {
/* Do nothing: ignore unresolved using declarations*/
} else if (const ObjCIvarDecl *Ivar = dyn_cast(ND)) {
if (!BaseType.isNull())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
[PATCH] D38538: Avoid printing some redundant name qualifiers in completion
This revision was automatically updated to reflect the committed changes.
Closed by commit rL317677: Avoid printing some redundant name qualifiers in
completion (authored by ibiryukov).
Repository:
rL LLVM
https://reviews.llvm.org/D38538
Files:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/CodeCompletion/call.cpp
cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp
cfe/trunk/test/CodeCompletion/uninstantiated_params.cpp
cfe/trunk/test/Index/complete-cxx-inline-methods.cpp
Index: cfe/trunk/test/Index/complete-cxx-inline-methods.cpp
===
--- cfe/trunk/test/Index/complete-cxx-inline-methods.cpp
+++ cfe/trunk/test/Index/complete-cxx-inline-methods.cpp
@@ -25,7 +25,7 @@
// RUN: c-index-test -code-completion-at=%s:4:9 -std=c++98 %s | FileCheck %s
// RUN: c-index-test -code-completion-at=%s:13:7 -std=c++98 %s | FileCheck %s
-// CHECK: CXXMethod:{ResultType MyCls::Vec &}{TypedText operator=}{LeftParen (}{Placeholder const MyCls::Vec &}{RightParen )} (79)
+// CHECK: CXXMethod:{ResultType Vec &}{TypedText operator=}{LeftParen (}{Placeholder const Vec &}{RightParen )} (79)
// CHECK-NEXT: StructDecl:{TypedText Vec}{Text ::} (75)
// CHECK-NEXT: FieldDecl:{ResultType int}{TypedText x} (35)
// CHECK-NEXT: FieldDecl:{ResultType int}{TypedText y} (35)
Index: cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp
===
--- cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp
+++ cfe/trunk/test/CodeCompletion/qualifiers-as-written.cpp
@@ -0,0 +1,30 @@
+struct foo {
+ typedef int type;
+
+ type method(type, foo::type, ::foo::type, ::foo::foo::type);
+};
+
+namespace ns {
+ struct bar {
+ };
+
+ struct baz {
+ };
+
+ int func(foo::type a, bar b, baz c);
+}
+
+typedef ns::bar bar;
+
+int func(foo a, bar b, ns::bar c, ns::baz d);
+using ns::func;
+
+void test() {
+ foo().method(0, 0, 0, 0);
+ // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:23:9 %s -o - | FileCheck %s --check-prefix=CHECK-1
+ // CHECK-1: COMPLETION: method : [#type#]method(<#type#>, <#foo::type#>, <#::foo::type#>, <#::foo::foo::type#>)
+ f
+ // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:26:3 %s -o - | FileCheck %s --check-prefix=CHECK-2
+ // CHECK-2: COMPLETION: func : [#int#]func(<#foo a#>, <#bar b#>, <#ns::bar c#>, <#ns::baz d#>
+ // CHECK-2: COMPLETION: func : [#int#]func(<#foo::type a#>, <#bar b#>, <#baz c#>
+}
Index: cfe/trunk/test/CodeCompletion/call.cpp
===
--- cfe/trunk/test/CodeCompletion/call.cpp
+++ cfe/trunk/test/CodeCompletion/call.cpp
@@ -19,10 +19,10 @@
f(Y(), 0, 0);
// RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:19:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
// CHECK-CC1: COMPLETION: Pattern : dynamic_cast<<#type#>>(<#expression#>)
- // CHECK-CC1: f(N::Y y, <#int ZZ#>)
+ // CHECK-CC1: f(Y y, <#int ZZ#>)
// CHECK-CC1-NEXT: f(int i, <#int j#>, int k)
// CHECK-CC1-NEXT: f(float x, <#float y#>)
// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:19:13 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
- // CHECK-CC2-NOT: f(N::Y y, int ZZ)
+ // CHECK-CC2-NOT: f(Y y, int ZZ)
// CHECK-CC2: f(int i, int j, <#int k#>)
}
Index: cfe/trunk/test/CodeCompletion/uninstantiated_params.cpp
===
--- cfe/trunk/test/CodeCompletion/uninstantiated_params.cpp
+++ cfe/trunk/test/CodeCompletion/uninstantiated_params.cpp
@@ -9,5 +9,5 @@
unique_ptr x;
x.
// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:5 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
- // CHECK-CC1: [#void#]reset({#<#unique_ptr::pointer ptr = pointer()#>#})
+ // CHECK-CC1: [#void#]reset({#<#pointer ptr = pointer()#>#})
}
Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -14,6 +14,7 @@
#include "clang/AST/DeclObjC.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/ExprObjC.h"
+#include "clang/AST/QualTypeNames.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/MacroInfo.h"
@@ -1495,6 +1496,7 @@
Policy.AnonymousTagLocations = false;
Policy.SuppressStrongLifetime = true;
Policy.SuppressUnwrittenScope = true;
+ Policy.SuppressScope = true;
return Policy;
}
@@ -2139,9 +2141,10 @@
T = Method->getSendResultType(BaseType);
else
T = Method->getReturnType();
- } else if (const EnumConstantDecl *Enumerator = dyn_cast(ND))
+ } else if (const EnumConstantDecl *Enumerator = dyn_cast(ND)) {
T = Context.getTypeDeclType(cast(Enumerator->getDeclContext()));
- else if (isa(ND)) {
+T = clang::TypeName::getFullyQualifiedType(T, Context);
+ } else if (isa(ND)) {
