robot planned changes to this revision.
robot marked 3 inline comments as done.
robot added a comment.
I'm working on the comments, but it will take a while.
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp:58
+
+ const llvm::ArrayRef<syntax::Token> Tokens =
+ TokBuf.expandedTokens(MethodDeclRange);
----------------
sammccall wrote:
> copying the tokens directly seems brittle, for example we shouldn't be
> emitting "virtual" but its's hard to avoid doing so.
> (Also, surprising that getEndLoc() doesn't cover the `= 0`!)
>
> D122827 has a reprint() function that seems to work
My original idea was to take the AST node, clone it, remove the pure-specifier,
and print that. To me, that seemed like the obvious (object-oriented) way how a
tweak would manipulate or generate text.
Alas, I was rather stumped that there didn't seem to be support for this
already, and it was rather hard to do. I gave up when default arguments came
into play.
I've got the idea of manipulating the tokens from the "extract function" tweak
(DefineOutline.cpp / getFunctionSourceCode).
I've tried the reprint function and it does not copy `noexcept`. That is simple
to fix but my guess is there are more issues. The idea of token manipulation
was to rather copy too much than too little. I have no intuition how many
issues could arise from that :)
When I try to naively remove the `virtual` I currently print (`virtual void
foo() override;`) in the token manipulation approach by skipping the first
tokens, I end up skipping any attributes that precede the `virtual` as well. So
I wrote a test for that, and reprint also doesn't copy attributes at the
moment. It is not obvious to me if attributes should be copied, e.g.
`[[noreturn]]`.
I have a slight preference for my original approach but it's probably too
complicated if this is the only use case. Therefore I'll just implement
whatever looks most promising to you.
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DeclarePureVirtuals.cpp:153
+ llvm::StringLiteral kind() const override {
+ return CodeAction::QUICKFIX_KIND;
+ }
----------------
sammccall wrote:
> I don't think this is a quick-fix, which should address a diagnostic or so.
>
> Really there doesn't seem to be a standard kind that fits here. Maybe we
> should add a new constant "generate"?
For reference:
rust-analyser shows a "Quick Fix" in VSCode for implementing the functions in a
trait.
The go extension shows a "Rewrite" (which is an entirely different action?) in
VSCode for filling a structure object (inserting code which explicitly
initializes each member with its null value).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153152/new/
https://reviews.llvm.org/D153152
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
