<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Sun, Dec 13, 2015 at 6:15 PM Piotr Dziwinski via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
Hello,<br>
<br>
I want to let you know that I'm still here and still working on
clang-tidy checker for localizing variables.<br>
Sorry for the long delay, the time flies so fast.<br>
<br>
Anyway, I have some good and some bad news.<br>
<br>
The good news is that I got my checker to work correctly when it
comes to handling nested scopes and control statements, including
all the corner cases that I predicted.<br>
I even made provisions for things like introducing localized for
loop counters and inserting braces where necessary in switch-case
blocks.<br>
<br>
The bad news is that I ran into two issues that I couldn't resolve
on my own, both of which are critical for the checker to be useful
in real production code.<br>
<br>
Issue #1. Handling multiple variables declared in one declaration
statement.<br>
<br>
<blockquote>The case of moving a single declaration is easy enough -
I can just cut the whole declaration as written in program source
and paste it somewhere else.<br>
But if I have to cut out only selected parts of a multiple
declaration and move them, I need some way of parsing the
declaration to separate parts of the statement.<br>
Let's take for example:<br>
<blockquote>long int a, *b, c[10];<br>
</blockquote>
I want to obtain the necessary source locations from AST so that I
can parse this declaration into four component strings:<br>
- type identifier: "long int"<br>
- first variable declaration: "a"<br>
- second variable declaration: "*b"<br>
- third variable declaration: "c[10]"<br>
<br>
Once I have that, it will be easy to independently cut out code
between identified source locations and create insertions by
putting together the strings from source code.<br>
<br>
I tried different methods of achieving that but given the
interface of DeclStmt and Decl, I cannot find a way to do that
reliably for all possible cases.<br>
I can obtain source locations for subsequent variable
declarations, but these always point to beginning of variable name
as it appears in code.<br>
What I need is actual source location separating "long int" from
"a" and further locations of separating commas between
declarations.<br>
Theoretically, I could use Lexer to find comma tokens, but there
are also other contexts where comma may appear, such list of
arguments of function pointer.<br></blockquote></div></blockquote><div><br></div><div>I believe you'll need to whip out the lexer for this currently.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF">
Issue #2. Merging multiple FixIt hint insertions in the same
location.<br>
<blockquote>For example, given:<br>
<blockquote>int a; int b;<br>
// ...<br>
call(a, b); // first use of "a" and "b"<br>
</blockquote>
My checker would generate two diagnostics:<br>
<br>
- "variable "a" declaration can be moved closer to its point of
use" with the following FixIt hint:<br>
<blockquote>int b;<br>
// ...<br>
int a; call(a, b);<br>
</blockquote>
- "variable "b" declaration can be moved closer to its point of
use" with the following FixIt hint:<br>
<blockquote>int a;<br>
// ...<br>
int b; call(a, b);<br>
</blockquote>
The end result of applying these FixIt hints is:<br>
<blockquote>// ...<br>
int b; call(a, b);<br>
</blockquote>
So the removals are handled correctly, but in case of insertions,
the second one overrides the first one. What I want is to have
both insertions present, one after the other.<br>
<br>
So far, I have worked around this problem by clumping together
such problematic FixIt hints to one of the diagnostics, so that
they are applied all at once.<br>
However, this can be quite confusing for the user who would see
seemingly unrelated FixIt hints under a diagnostic referring to
only one variable.<br>
Needless to say, this also makes the code much more complicated
than it needs to be as I have to store information first and emit
diagnostics later.<br>
<br>
I don't know how to resolve this, as this is a more a problem with
applying FixIt hints in general.<br>
Perhaps this is actually the intended behavior in some cases. Even
so, I would like to have a way to address my use case, without
impacting other use cases.<br></blockquote></div></blockquote><div><br></div><div>I think that has bitten us at some point, too - if this is still a problem, I'd consider it a bug in clang-tidy's fixit handling.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF"><blockquote>
</blockquote>
<br>
<br>
I would appreciate any pointers of how to tackle these two issues.<br>
I'm afraid that without solving them, my checker will remain in
"always-beta" state.<br>
<br>
Meanwhile, if you are interested in current version of the code, you
can check out my Github fork:<br>
<a href="https://github.com/piotrdz/clang-tools-extra" target="_blank">https://github.com/piotrdz/clang-tools-extra</a>
<br>
<br>
<br>
Best regards,<br>
Piotr Dziwinski</div><div text="#000000" bgcolor="#FFFFFF"><br>
<br>
<div>On 22/10/15 17:21, Richard via cfe-dev
wrote:<br>
</div>
<blockquote type="cite">
<pre>[Please reply *only* to the list and do not include my email directly
in the To: or Cc: of your reply; otherwise I will not see your reply.
Thanks.]
In article <a href="mailto:5626C26F.4040009@gmail.com" target="_blank"><5626C26F.4040009@gmail.com></a>,
Piotr Dziwinski via cfe-dev <a href="mailto:cfe-dev@lists.llvm.org" target="_blank"><cfe-dev@lists.llvm.org></a> writes:
</pre>
<blockquote type="cite">
<pre>It still needs work on corner cases and some hard cases like switch
statements, which I've left out for now. And of course more tests. I
will try to cover as much as possible in unit tests, but it's just
impossible to test everything. The real verification will be running it
on some production code. I will look at the project you gave as example,
too.
</pre>
</blockquote>
<pre>Areas where I have found refactoring tools to fail are with "exotic"
types like pointer to function, pointer to member data and pointer to
member function. Sometimes things freak out with unions, too.
I looked at your test file on github and it's already covering the
idea of multiple variables declared in a single statement where one of
them is localized later in the scope, so that looks good.
</pre>
</blockquote>
<br>
</div>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div></div>