v1nh1shungry added a comment.
Thank you for reviewing and giving suggestions!
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExpandDeducedType.cpp:57
+std::string ExpandDeducedType::title() const {
+ return IsAutoType ? "Expand auto type" : "Expand decltype";
+}
----------------
sammccall wrote:
> Maybe "Replace with deduced type" would be clearer for both cases?
>
> Then we don't need to track IsAutoType.
> (I have no objection with spending a bit of code to provide better text, but
> given that the cursor is either on top of "auto" or "decltype" I don't think
> we're actually adding anything by echoing it back)
I think your suggestion is better. Thanks!
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExpandDeducedType.cpp:139
- // if it's a lambda expression, return an error message
- if (isa<RecordType>(*DeducedType) &&
- cast<RecordType>(*DeducedType)->getDecl()->isLambda()) {
- return error("Could not expand type of lambda expression");
+ // we shouldn't replace a dependent type, e.g.
+ // template <class T>
----------------
sammccall wrote:
> why not? replacing this with `T` seems perfectly reasonable.
> (The fact that we don't do this with `auto t = T{}` is just because we're
> hitting a limitation of clang's AST)
I'd like it too. But the `printType()` would give out a `<dependent type>`.
Though I haven't looked into this, would you mind giving some suggestions about
it?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141226/new/
https://reviews.llvm.org/D141226
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
