[cfe-dev] clang-refactor usage and development

[Jonas Toth via cfe-dev]

Hi Kirill,


Am 25.08.2018 um 12:25 schrieb Kirill Bobyrev:
> Hi Jonas,
>
> I believe that your use-case is slightly different from what
> Clang-Refactor tries (or, to be honest, is supposed to try) to
> support. Basically, Clang-Refactor should be able to process
> refactoring queries from the user coming from the editor, command line
> or any other way.
I thought this could be done from clang-tidy as well. E.g. making all
locals const workflow:

1. Do the analysis and find all possible const variables
2. Isolate their declaration (unfolding the decl stmts) (just call the
refactor framework maybe with the DeclStmt as argument)
3. Change Qualifier -> adding const (another call to the refactoring
framework)

Clang-tidy would then just manage the actual workhorses beeing analysis
facilities (the const-thing lives in clang-tidy repo at the moment) and
the high level refactorings.

I guess it would be an issue to have these steps separated, because the
source-to-source transformation probably requires a rebuild of the AST
to reliably do the next refactoring etc.

And an editor integration automatically benefits from the refactorings
clang-tidy can do. But here 'clangd' is probably the key.
> From my perspective, your use-case perfectly fits into the scope of
> Clang-Tidy: what you propose seems more like linting than a
> refactoring (in case of adding const I can totally see how that can
> fit into the general refactoring pipeline with the “change type of the
> variable” request, but it has many pitfalls const-ness change
> shouldn’t invoke). Both cases look very much like Clang-Tidy checks to me:
>
> * Variable is not mutated anywhere -> change it’s type to `const T`
My issue while implementing and thinking about it was, that many values
will not benefit from the clang-tidy transformations because
declarations like `int i, j, k = 0;` and other value-like classes are
common enough.
If only one of these variables can be a constant(e.g. `k`) you can only
emit the warning and can't do the transformation without a prior
'IsolateDecl'.

And 'change type' is a common thing for clang-tidy (introducing smart
pointers, auto-usage, gsl::owner, gsl::not_null, ...). All of them rely
to some extend on an 'IsolateDecl'-Refactoring to do their
transformations correct or in more places.

So I think my main point would be code-reuse. It is possible to
implement these refactoring primitives in the clang-tidy/utils-space but
then other clang-tools could not benefit from it.

> Also, I believe that clang-rename Vim and Emacs integrations were
> changed for a long long time (although I can see that you recently
> added Python 3 support, thank you!). I would advise you to use Clangd,
> which utilises the same infrastructure Clang-Rename does for
> renamings, but has way better support. I understand that setting up
> Clangd might be not trivial and I might share my experience soon-ish,
> so that more people could try it.
I honestly did not try it out so far, I have read somewhere that only
Visual Studio is (was?) supported and as a vim on Linux guy I did not
investigate further. I would love to have even more IDE features and if
there is support for vim, I will definitely try it out (and maybe even
help making it better ;)).
> To sum up, I would suggest implementing both cases as Clang-Tidy
> checks. Right now, Clangd exposes compile warnings and Fix-Its to the
> client, so that user can see potentially problematic places and apply
> these Fix-Its. AFAIK there is an ongoing effort to integrate
> Clang-Tidy into Clangd pipeline so that Clang-Tidy warnings and
> Fix-Its would be also exposed to the user, I am personally very
> excited about that. Thus said, I believe you might have way better
> user experience by going this way, because you wouldn’t have to
> implement stand-alone tools and deal with integration.
Totally agree that one tool serving all use-cases is best for the user
experience and reducing boilerplate.

Best Jonas
>
> Let me know if it works for you!
>
> Kind regards,
> Kirill Bobyrev
>
> P.S. You can take a look at the original
> design: https://lists.llvm.org/pipermail/cfe-dev/2016-September/050641.html.
> Unfortunately, nothing major happened ever since, but the idea is
> still out there, especially now that we have Clangd.
>
>> On 25 Aug 2018, at 11:48, Jonas Toth via cfe-dev
>> <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
>>
>> Hello everyone,
>>
>> I am currently looking into adding new refactorings for clang to utilize
>> them both in clang-tidy and clang-refactor.
>>
>> While exploring clang-refactor it seemed, that the tool itself does not
>> work yet on standalone files, but only for the existing test cases. But
>> the code for the tool itself does contain logic to actually apply
>> refactorings inplace or to print the new file to stdout.
>>
>> Given that clang-refactor still seems to be in an unstable stage, could
>> someone point me to the right places to make it work as standalone tool?
>> I would be very interested to get it running and maybe write a
>> vim-integration similar to clang-rename as well.
>>
>>
>> Background:
>>
>> For me it would be very valuable to add the refactoring to a central
>> place within clang, as I want to extend some of the clang-tidy checks to
>> do more code transformations which relies sometimes on prior
>> refactorings. For example to change the const-ness of a variable it is
>> necessary to isolate the variable declaration first and change the type
>> second.
>>
>> Transforming `int a, b, c = 0;` into `int a; int b; int c = 0;` is the
>> refactoring i am implementing at the moment as well. But my code runs
>> within clang-tidy as I was able to experiment better with it.
>>
>>
>> Thank you for the help.
>>
>> All the best
>>
>> Jonas
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180825/d66a13e2/attachment.html>