| Message ID | 1450805631-15455-1-git-send-email-patrick@parcs.ath.cx |
|---|---|
| State | New |
| Headers | show |
On Tue, 2015-12-22 at 12:33 -0500, Patrick Palka wrote: > We are emitting a bogus warning for the code > > do foo (0); while (flagA); > ^--- NEXT > ^------------ BODY > ^--------------- GUARD > > In general I don't think we can get any useful indentation warning out > of a do-while statement, so this patch makes it so that we just skip > them. > > Is this OK to commit after testing? It looks like this is PR c++/69029. FWIW, I agree with your rationale - though perhaps we should instead update the frontends to not even call into c-indentation.c for do-while statements? > gcc/c-family/ChangeLog: > > * c-indentation.c (should_warn_for_misleading_indentation): > Don't warn about do-while statements. > > gcc/testsuite/ChangeLog: > > * c-c++-common/Wisleading-indentation.c: Augment test. > --- > gcc/c-family/c-indentation.c | 6 ++++++ > gcc/testsuite/c-c++-common/Wmisleading-indentation.c | 2 ++ > 2 files changed, 8 insertions(+) > > diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c > index 638a9cf..1cbaab7 100644 > --- a/gcc/c-family/c-indentation.c > +++ b/gcc/c-family/c-indentation.c > @@ -202,6 +202,12 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo, > if (line_table->seen_line_directive) > return false; > > + /* We can't usefully warn about do-while statements since the bodies of these > + statements are always explicitly delimited at both ends, so control flow is > + quite obvious. */ > + if (guard_tinfo.keyword == RID_DO) > + return false; > + > /* If the token following the body is a close brace or an "else" > then while indentation may be sloppy, there is not much ambiguity > about control flow, e.g. > diff --git a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c > index a3f5acd..72c21a0 100644 > --- a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c > +++ b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c > @@ -890,4 +890,6 @@ fn_39 (void) > i < 10; > i++); > foo (i); > + > + do foo (0); while (flagA); > }
On 01/05/2016 10:58 PM, David Malcolm wrote: >> >> In general I don't think we can get any useful indentation warning out >> of a do-while statement, so this patch makes it so that we just skip >> them. >> >> Is this OK to commit after testing? > > It looks like this is PR c++/69029. FWIW, I agree with your rationale - > though perhaps we should instead update the frontends to not even call > into c-indentation.c for do-while statements? Ok with the PR mentioned in the ChangeLog as usual. I thought about David's idea, but I think it's reasonable to keep all knowledge inside c-indentation.c as in Patrick's patch. Bernd
On Tue, Dec 22, 2015 at 12:33:51PM -0500, Patrick Palka wrote: > We are emitting a bogus warning for the code > > do foo (0); while (flagA); > ^--- NEXT > ^------------ BODY > ^--------------- GUARD > > In general I don't think we can get any useful indentation warning out > of a do-while statement, so this patch makes it so that we just skip > them. Not even say for do foo (0); while (flagA); etc.? Though, not sure if c-indentation.c wants to warn about those. Jakub
On 01/08/2016 03:43 AM, Jakub Jelinek wrote: > On Tue, Dec 22, 2015 at 12:33:51PM -0500, Patrick Palka wrote: >> We are emitting a bogus warning for the code >> >> do foo (0); while (flagA); >> ^--- NEXT >> ^------------ BODY >> ^--------------- GUARD >> >> In general I don't think we can get any useful indentation warning out >> of a do-while statement, so this patch makes it so that we just skip >> them. > > Not even say for > do > foo (0); > while (flagA); > etc.? Though, not sure if c-indentation.c wants to warn about those. That's considered "bad" indentation, but not misleading indentation because it's not implying something is being guarded that isn't actually being guarded. jeff
diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c index 638a9cf..1cbaab7 100644 --- a/gcc/c-family/c-indentation.c +++ b/gcc/c-family/c-indentation.c @@ -202,6 +202,12 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo, if (line_table->seen_line_directive) return false; + /* We can't usefully warn about do-while statements since the bodies of these + statements are always explicitly delimited at both ends, so control flow is + quite obvious. */ + if (guard_tinfo.keyword == RID_DO) + return false; + /* If the token following the body is a close brace or an "else" then while indentation may be sloppy, there is not much ambiguity about control flow, e.g. diff --git a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c index a3f5acd..72c21a0 100644 --- a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c +++ b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c @@ -890,4 +890,6 @@ fn_39 (void) i < 10; i++); foo (i); + + do foo (0); while (flagA); }
We are emitting a bogus warning for the code do foo (0); while (flagA); ^--- NEXT ^------------ BODY ^--------------- GUARD In general I don't think we can get any useful indentation warning out of a do-while statement, so this patch makes it so that we just skip them. Is this OK to commit after testing? gcc/c-family/ChangeLog: * c-indentation.c (should_warn_for_misleading_indentation): Don't warn about do-while statements. gcc/testsuite/ChangeLog: * c-c++-common/Wisleading-indentation.c: Augment test. --- gcc/c-family/c-indentation.c | 6 ++++++ gcc/testsuite/c-c++-common/Wmisleading-indentation.c | 2 ++ 2 files changed, 8 insertions(+)