On Tue, 28 Jul 2020, David Malcolm wrote:
> On Tue, 2020-07-28 at 15:50 -0400, Patrick Palka wrote:
> > Currently the -Wmisleading-indentation warning doesn't do any
> > analysis
> > when the guarded statement or the statement after it is produced by a
> > macro, as the mentioned PR illustrates. This means that we warn for:
> >
> > if (flag)
> > foo ();
> > bar ();
> >
> > and yet we don't warn for:
> >
> > #define BAR bar
> > if (flag)
> > foo ();
> > BAR ();
> >
> > which seems like a surprising limitation.
>
> IIRC we were trying to keep things simple in the initial
> implementation.
>
> > This patch extends the -Wmisleading-indentation implementation to
> > support analyzing such statements and their tokens. This is done in
> > the
> > "natural" way by resolving the location of each of the three tokens
> > to
> > the token's macro expansion point. (Additionally, if the tokens all
> > resolve to the same macro expansion point then we instead use their
> > locations within the macro definition.) When these resolved
> > locations
> > are all different, then we can proceed with applying the warning
> > heuristics to them as if no macros were involved.
>
> Thanks for working on this. I've only looked at the patch briefly, but
> so far it looks reasonable.
>
> Can you post some examples of what the output looks like for some of
> these cases? The diagnostics code has some logic for printing how
> macros are unwound, so I'm wondering what we actually print for these
> cases, and if it looks intelligble to an end-user.
For the code fragment mentioned above:
int
main()
{
#define BAR
if (flag)
foo ();
BAR ();
}
we emit:
test.cc: In function ‘int main()’:
test.cc:7:3: warning: this ‘if’ clause does not guard...
[-Wmisleading-indentation]
9 | if (flag)
| ^~
test.cc:8:15: note: ...this statement, but the latter is misleadingly indented
as if it were guarded by the ‘if’
8 | #define BAR bar
| ^~~
test.cc:10:5: note: in expansion of macro ‘BAR’
11 | BAR ();
| ^~~
And when all tokens are generated from the same macro, e.g. for:
int
main()
{
#define BAR bar
#define DO_STUFF \
if (flag) \
foo (); \
BAR ();
DO_STUFF;
}
we emit:
test.cc: In function ‘int main()’:
test.cc:11:5: warning: this ‘if’ clause does not guard...
[-Wmisleading-indentation]
11 | if (flag) \
| ^~
test.cc:14:3: note: in expansion of macro ‘DO_STUFF’
14 | DO_STUFF;
| ^~~~~~~~
test.cc:9:15: note: ...this statement, but the latter is misleadingly indented
as if it were guarded by the ‘if’
9 | #define BAR bar
| ^~~
test.cc:13:7: note: in expansion of macro ‘BAR’
13 | BAR ();
| ^~~
test.cc:14:3: note: in expansion of macro ‘DO_STUFF’
14 | DO_STUFF;
| ^~~~~~~~
The automatic macro unwinding logic saves the day here and yields mostly
legible output "for free". What do you think?
