[cfe-dev] [clang-tidy] Some possible contributions

[Piotr Dziwinski via cfe-dev]

On 16/12/15 18:13, Richard via cfe-dev wrote:
> [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 <56709B81.2020403 at gmail.com>,
>      Piotr Dziwinski via cfe-dev <cfe-dev at lists.llvm.org> writes:
>
>> On 15/12/15 21:10, Richard via cfe-dev wrote:
>>> I remember now that I was looking into this for a check that would
>>> split a multiple variable declaration into multiple single variable
>>> declarations. [...]
>> Well, taking the assumption that all variable declarations are single
>> declarations would simplify my localizing variables checker a great deal
>> and make issue #1 obsolete.
> OK, good we're on the same thought track.
>
>> On the other hand, I worry about the decreased usability from the user's
>> point of view.
> Agreed.
>
>> He would have to first run the checker that splits multiple
>> declarations, applying its fixes everywhere.
>> Only then, as a second step, he would be able to run the localizing
>> variables checker, and use its fixes.
> How would a user feel if the localizing check broke down multiple
> delcarations into single declarations as a by-product?
>
> In other words:
>
> int i, j, k;
> // many lines of code
> i = 10;
>
> becomes
>
> int j;
> int k;
> // many lines of code
> int i = 10;
>
> Would this be unacceptable?

Well, my checker doesn't have anything against multiple declarations per 
se. There are cases where they are not considered a problem.
For example:

   int a, b;
   useByReference(a, b);

Here, both variables are already declared just before they are used so 
there is nothing to do, and no diagnostic is generated.
I would like to keep it like that, as any change suggested here, even 
if's just splitting the declarations, would be confusing to the user.

The only problem is when variables from multiple declaration statement 
must be moved, possibly to different locations.
In such case, since we are already changing the line with declaration, 
we might as well do as you suggest and split the declaration.
Actually, it would be quite simple to implement as well: remove the 
whole declaration and then insert individual variables as single 
declarations,
either somewhere else if they are to be moved, or in the place of old 
declaration if they are to be left behind as they were.

However, if we go this way, there is a problem with generating the 
diagnostics in a sensible way.

So far, the idea was to have:
   1 diagnostic = 1 variable = 1 FixIt hint removal from old location = 
1 FixIt hint insertion at new location.

If we decide to do the splitting of multiple declarations the way I 
described, it would have to be:
  1 diagnostic = 1 (multiple) declaration statement = 1 FixIt hint 
removal from old location = 1 or more FixIt hint insertions for the 
individual variables.

I'm not sure if it is better or worse from the user's POV.

Perhaps you are right and the easiest way out would be to assume that 
all declarations are single declarations,
and let the user deal with multiple declarations by running another 
checker to split them.

Best regards,
Piotr Dziwinski