[PATCH] D53347: [clangd] Simplify auto hover

2018-10-24 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL345128: [clangd] Simplify auto hover (authored by ibiryukov, 
committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53347?vs=170273=170835#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53347

Files:
  clang-tools-extra/trunk/clangd/XRefs.cpp
  clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp


Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -585,17 +585,6 @@
 
   Optional getDeducedType() { return DeducedType; }
 
-  // Remove the surrounding Reference or Pointer type of the given type T.
-  QualType UnwrapReferenceOrPointer(QualType T) {
-// "auto &" is represented as a ReferenceType containing an AutoType
-if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
-  return RT->getPointeeType();
-// "auto *" is represented as a PointerType containing an AutoType
-if (const PointerType *PT = dyn_cast(T.getTypePtr()))
-  return PT->getPointeeType();
-return T;
-  }
-
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
@@ -606,17 +595,9 @@
 D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation)
   return true;
 
-auto DeclT = UnwrapReferenceOrPointer(D->getType());
-const AutoType *AT = dyn_cast(DeclT.getTypePtr());
-if (AT && !AT->getDeducedType().isNull()) {
-  // For auto, use the underlying type because the const& would be
-  // represented twice: written in the code and in the hover.
-  // Example: "const auto I = 1", we only want "int" when hovering on auto,
-  // not "const int".
-  //
-  // For decltype(auto), take the type as is because it cannot be written
-  // with qualifiers or references but its decuded type can be const-ref.
-  DeducedType = AT->isDecltypeAuto() ? DeclT : DeclT.getUnqualifiedType();
+if (auto *AT = D->getType()->getContainedAutoType()) {
+  if (!AT->getDeducedType().isNull())
+DeducedType = AT->getDeducedType();
 }
 return true;
   }
@@ -640,12 +621,13 @@
 if (CurLoc != SearchedLocation)
   return true;
 
-auto T = UnwrapReferenceOrPointer(D->getReturnType());
-const AutoType *AT = dyn_cast(T.getTypePtr());
+const AutoType *AT = D->getReturnType()->getContainedAutoType();
 if (AT && !AT->getDeducedType().isNull()) {
-  DeducedType = T.getUnqualifiedType();
-} else { // auto in a trailing return type just points to a DecltypeType.
-  const DecltypeType *DT = dyn_cast(T.getTypePtr());
+  DeducedType = AT->getDeducedType();
+} else {
+  // auto in a trailing return type just points to a DecltypeType and
+  // getContainedAutoType does not unwrap it.
+  const DecltypeType *DT = dyn_cast(D->getReturnType());
   if (!DT->getUnderlyingType().isNull())
 DeducedType = DT->getUnderlyingType();
 }
Index: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
@@ -999,6 +999,13 @@
   )cpp",
   "",
   },
+  {
+  R"cpp(// More compilcated structured types.
+int bar();
+^auto (*foo)() = bar;
+  )cpp",
+  "int",
+  },
   };
 
   for (const OneTest  : Tests) {


Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -585,17 +585,6 @@
 
   Optional getDeducedType() { return DeducedType; }
 
-  // Remove the surrounding Reference or Pointer type of the given type T.
-  QualType UnwrapReferenceOrPointer(QualType T) {
-// "auto &" is represented as a ReferenceType containing an AutoType
-if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
-  return RT->getPointeeType();
-// "auto *" is represented as a PointerType containing an AutoType
-if (const PointerType *PT = dyn_cast(T.getTypePtr()))
-  return PT->getPointeeType();
-return T;
-  }
-
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
@@ -606,17 +595,9 @@
 D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation)
   return true;
 
-auto DeclT = UnwrapReferenceOrPointer(D->getType());
-const AutoType *AT = dyn_cast(DeclT.getTypePtr());
-if (AT && !AT->getDeducedType().isNull()) {
-  // For auto, use the underlying type because the const& would be
-  // represented twice: written in the code and in the hover.
-  // Example: "const auto I = 1", we only want "int" when hovering on auto,
- 

[PATCH] D53347: [clangd] Simplify auto hover

2018-10-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

In https://reviews.llvm.org/D53347#1267216, @kadircet wrote:

> LGTM, it bugs me that some part in the documentation says it doesn't go 
> through decltype(`This looks through declarators like pointer types, but not 
> through decltype or typedefs`) but since tests containing decltype didn't 
> break it looks OK to me. I would still wait for @hokein to have another look 
> as well.


The decltypes are handled separately in `VisitDecltypeTypeLoc` and typedefs are 
a decl, so they are handled even before the visitor runs. This is only about 
'auto'. So I think we're covered here.




Comment at: clangd/XRefs.cpp:592
 
-auto DeclT = UnwrapReferenceOrPointer(D->getType());
-const AutoType *AT = dyn_cast(DeclT.getTypePtr());
-if (AT && !AT->getDeducedType().isNull()) {
-  // For auto, use the underlying type because the const& would be
-  // represented twice: written in the code and in the hover.
-  // Example: "const auto I = 1", we only want "int" when hovering on auto,
-  // not "const int".
-  //
-  // For decltype(auto), take the type as is because it cannot be written
-  // with qualifiers or references but its decuded type can be const-ref.
-  DeducedType = AT->isDecltypeAuto() ? DeclT : DeclT.getUnqualifiedType();
-}
+auto AT = D->getType()->getContainedAutoType();
+if (AT && !AT->getDeducedType().isNull())

hokein wrote:
> Maybe just 
> 
> ```
> if (auto* DT = D->getType()->getContainedDeducedType())
>DeducedType = *DT;
> ```
> 
> ? 
It feels like explicitly unwrapping auto from the deduced type instead of 
relying the printer to do that is a bit more straightforward.

Simplified code a bit per suggestions.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53347



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53347: [clangd] Simplify auto hover

2018-10-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 170273.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.

- Addressed review comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53347

Files:
  clangd/XRefs.cpp
  unittests/clangd/XRefsTests.cpp


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -999,6 +999,13 @@
   )cpp",
   "",
   },
+  {
+  R"cpp(// More compilcated structured types.
+int bar();
+^auto (*foo)() = bar;
+  )cpp",
+  "int",
+  },
   };
 
   for (const OneTest  : Tests) {
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -585,17 +585,6 @@
 
   llvm::Optional getDeducedType() { return DeducedType; }
 
-  // Remove the surrounding Reference or Pointer type of the given type T.
-  QualType UnwrapReferenceOrPointer(QualType T) {
-// "auto &" is represented as a ReferenceType containing an AutoType
-if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
-  return RT->getPointeeType();
-// "auto *" is represented as a PointerType containing an AutoType
-if (const PointerType *PT = dyn_cast(T.getTypePtr()))
-  return PT->getPointeeType();
-return T;
-  }
-
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
@@ -606,17 +595,9 @@
 D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation)
   return true;
 
-auto DeclT = UnwrapReferenceOrPointer(D->getType());
-const AutoType *AT = dyn_cast(DeclT.getTypePtr());
-if (AT && !AT->getDeducedType().isNull()) {
-  // For auto, use the underlying type because the const& would be
-  // represented twice: written in the code and in the hover.
-  // Example: "const auto I = 1", we only want "int" when hovering on auto,
-  // not "const int".
-  //
-  // For decltype(auto), take the type as is because it cannot be written
-  // with qualifiers or references but its decuded type can be const-ref.
-  DeducedType = AT->isDecltypeAuto() ? DeclT : DeclT.getUnqualifiedType();
+if (auto *AT = D->getType()->getContainedAutoType()) {
+  if (!AT->getDeducedType().isNull())
+DeducedType = AT->getDeducedType();
 }
 return true;
   }
@@ -640,12 +621,13 @@
 if (CurLoc != SearchedLocation)
   return true;
 
-auto T = UnwrapReferenceOrPointer(D->getReturnType());
-const AutoType *AT = dyn_cast(T.getTypePtr());
+const AutoType *AT = D->getReturnType()->getContainedAutoType();
 if (AT && !AT->getDeducedType().isNull()) {
-  DeducedType = T.getUnqualifiedType();
-} else { // auto in a trailing return type just points to a DecltypeType.
-  const DecltypeType *DT = dyn_cast(T.getTypePtr());
+  DeducedType = AT->getDeducedType();
+} else {
+  // auto in a trailing return type just points to a DecltypeType and
+  // getContainedAutoType does not unwrap it.
+  const DecltypeType *DT = dyn_cast(D->getReturnType());
   if (!DT->getUnderlyingType().isNull())
 DeducedType = DT->getUnderlyingType();
 }


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -999,6 +999,13 @@
   )cpp",
   "",
   },
+  {
+  R"cpp(// More compilcated structured types.
+int bar();
+^auto (*foo)() = bar;
+  )cpp",
+  "int",
+  },
   };
 
   for (const OneTest  : Tests) {
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -585,17 +585,6 @@
 
   llvm::Optional getDeducedType() { return DeducedType; }
 
-  // Remove the surrounding Reference or Pointer type of the given type T.
-  QualType UnwrapReferenceOrPointer(QualType T) {
-// "auto &" is represented as a ReferenceType containing an AutoType
-if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
-  return RT->getPointeeType();
-// "auto *" is represented as a PointerType containing an AutoType
-if (const PointerType *PT = dyn_cast(T.getTypePtr()))
-  return PT->getPointeeType();
-return T;
-  }
-
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
@@ -606,17 +595,9 @@
 D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation)
   return true;
 
-auto DeclT = UnwrapReferenceOrPointer(D->getType());
-const AutoType *AT = dyn_cast(DeclT.getTypePtr());
-if (AT && !AT->getDeducedType().isNull()) {
-  // For auto, use the underlying type because the const& would be
-  // represented twice: written in the code and in the 

[PATCH] D53347: [clangd] Simplify auto hover

2018-10-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.

looks good! didn't know this API before.




Comment at: clangd/XRefs.cpp:592
 
-auto DeclT = UnwrapReferenceOrPointer(D->getType());
-const AutoType *AT = dyn_cast(DeclT.getTypePtr());
-if (AT && !AT->getDeducedType().isNull()) {
-  // For auto, use the underlying type because the const& would be
-  // represented twice: written in the code and in the hover.
-  // Example: "const auto I = 1", we only want "int" when hovering on auto,
-  // not "const int".
-  //
-  // For decltype(auto), take the type as is because it cannot be written
-  // with qualifiers or references but its decuded type can be const-ref.
-  DeducedType = AT->isDecltypeAuto() ? DeclT : DeclT.getUnqualifiedType();
-}
+auto AT = D->getType()->getContainedAutoType();
+if (AT && !AT->getDeducedType().isNull())

Maybe just 

```
if (auto* DT = D->getType()->getContainedDeducedType())
   DeducedType = *DT;
```

? 



Comment at: clangd/XRefs.cpp:620
+  DeducedType = AT->getDeducedType();
 } else { // auto in a trailing return type just points to a DecltypeType.
+  const DecltypeType *DT = dyn_cast(D->getReturnType());

also expand this comment mentioning that `decltype` is not handled by the 
`getContainedAutoType` or `getContainedDeducedType`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53347



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53347: [clangd] Simplify auto hover

2018-10-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

LGTM, it bugs me that some part in the documentation says it doesn't go through 
decltype(`This looks through declarators like pointer types, but not through 
decltype or typedefs`) but since tests containing decltype didn't break it 
looks OK to me. I would still wait for @hokein to have another look as well.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53347



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53347: [clangd] Simplify auto hover

2018-10-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.
ilya-biryukov added reviewers: kadircet, hokein.
Herald added subscribers: arphaman, jkorous, MaskRay, ioeric.

Use helper from clang. Also fixes some weird corner cases, e.g.

  auto (*foo)() = bar;


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53347

Files:
  clangd/XRefs.cpp
  unittests/clangd/XRefsTests.cpp


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -999,6 +999,13 @@
   )cpp",
   "",
   },
+  {
+  R"cpp(// More compilcated structured types.
+int bar();
+^auto (*foo)() = bar;
+  )cpp",
+  "int",
+  },
   };
 
   for (const OneTest  : Tests) {
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -579,17 +579,6 @@
 
   llvm::Optional getDeducedType() { return DeducedType; }
 
-  // Remove the surrounding Reference or Pointer type of the given type T.
-  QualType UnwrapReferenceOrPointer(QualType T) {
-// "auto &" is represented as a ReferenceType containing an AutoType
-if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
-  return RT->getPointeeType();
-// "auto *" is represented as a PointerType containing an AutoType
-if (const PointerType *PT = dyn_cast(T.getTypePtr()))
-  return PT->getPointeeType();
-return T;
-  }
-
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
@@ -600,18 +589,9 @@
 D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation)
   return true;
 
-auto DeclT = UnwrapReferenceOrPointer(D->getType());
-const AutoType *AT = dyn_cast(DeclT.getTypePtr());
-if (AT && !AT->getDeducedType().isNull()) {
-  // For auto, use the underlying type because the const& would be
-  // represented twice: written in the code and in the hover.
-  // Example: "const auto I = 1", we only want "int" when hovering on auto,
-  // not "const int".
-  //
-  // For decltype(auto), take the type as is because it cannot be written
-  // with qualifiers or references but its decuded type can be const-ref.
-  DeducedType = AT->isDecltypeAuto() ? DeclT : DeclT.getUnqualifiedType();
-}
+auto AT = D->getType()->getContainedAutoType();
+if (AT && !AT->getDeducedType().isNull())
+  DeducedType = AT->getDeducedType();
 return true;
   }
 
@@ -634,12 +614,11 @@
 if (CurLoc != SearchedLocation)
   return true;
 
-auto T = UnwrapReferenceOrPointer(D->getReturnType());
-const AutoType *AT = dyn_cast(T.getTypePtr());
+const AutoType *AT = D->getReturnType()->getContainedAutoType();
 if (AT && !AT->getDeducedType().isNull()) {
-  DeducedType = T.getUnqualifiedType();
+  DeducedType = AT->getDeducedType();
 } else { // auto in a trailing return type just points to a DecltypeType.
-  const DecltypeType *DT = dyn_cast(T.getTypePtr());
+  const DecltypeType *DT = dyn_cast(D->getReturnType());
   if (!DT->getUnderlyingType().isNull())
 DeducedType = DT->getUnderlyingType();
 }


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -999,6 +999,13 @@
   )cpp",
   "",
   },
+  {
+  R"cpp(// More compilcated structured types.
+int bar();
+^auto (*foo)() = bar;
+  )cpp",
+  "int",
+  },
   };
 
   for (const OneTest  : Tests) {
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -579,17 +579,6 @@
 
   llvm::Optional getDeducedType() { return DeducedType; }
 
-  // Remove the surrounding Reference or Pointer type of the given type T.
-  QualType UnwrapReferenceOrPointer(QualType T) {
-// "auto &" is represented as a ReferenceType containing an AutoType
-if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
-  return RT->getPointeeType();
-// "auto *" is represented as a PointerType containing an AutoType
-if (const PointerType *PT = dyn_cast(T.getTypePtr()))
-  return PT->getPointeeType();
-return T;
-  }
-
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
@@ -600,18 +589,9 @@
 D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation)
   return true;
 
-auto DeclT = UnwrapReferenceOrPointer(D->getType());
-const AutoType *AT = dyn_cast(DeclT.getTypePtr());
-if (AT && !AT->getDeducedType().isNull()) {
-  // For auto, use the underlying type because the const& would be
-  // represented twice: written in the code and in the hover.
-  // Example: "const auto I = 1", we only want "int" when