Discussion:
Not quoted option names in errors and warnings
Martin Liška
2018-11-15 10:12:22 UTC
Permalink
Hi.

I've done a quick grep of gcc/po/gcc.pot and I see quite a lot of missing quotations
of option names (~400). Is it something we should fix? How important is that?

Martin
Martin Sebor
2018-11-15 16:54:20 UTC
Permalink
Post by Martin Liška
Hi.
I've done a quick grep of gcc/po/gcc.pot and I see quite a lot of missing quotations
of option names (~400). Is it something we should fix? How important is that?
That's quite a few... I've been fixing these as I notice them,
usually as part of related changes. The most onerous part of
the cleanup is adjusting tests, especially under the target-
specific ones. It's (obviously) not critical but I think it
would be nice to make the quoting consistent throughout over
time (if not in one go) and then put in place a -Wformat
checker to detect the missing quoting as new diagnostics are
introduced. Do you think it might be scriptable?

Martin
Martin Liška
2018-11-20 10:10:44 UTC
Permalink
Post by Martin Sebor
Post by Martin Liška
Hi.
I've done a quick grep of gcc/po/gcc.pot and I see quite a lot of missing quotations
of option names (~400). Is it something we should fix? How important is that?
That's quite a few... I've been fixing these as I notice them,
usually as part of related changes.  The most onerous part of
the cleanup is adjusting tests, especially under the target-
specific ones.  It's (obviously) not critical but I think it
would be nice to make the quoting consistent throughout over
time (if not in one go) and then put in place a -Wformat
checker to detect the missing quoting as new diagnostics are
introduced.  Do you think it might be scriptable?
Hi.

Are you talking about a proper GCC warning that will be triggered once a warning
message is missing quotations?

I can definitely cook a patch in next stage1 and the testsuite fall out should
be easy to come with.

Martin
Post by Martin Sebor
Martin
Martin Sebor
2018-11-20 15:24:15 UTC
Permalink
Post by Martin Liška
Post by Martin Sebor
Post by Martin Liška
Hi.
I've done a quick grep of gcc/po/gcc.pot and I see quite a lot of missing quotations
of option names (~400). Is it something we should fix? How important is that?
That's quite a few... I've been fixing these as I notice them,
usually as part of related changes. The most onerous part of
the cleanup is adjusting tests, especially under the target-
specific ones. It's (obviously) not critical but I think it
would be nice to make the quoting consistent throughout over
time (if not in one go) and then put in place a -Wformat
checker to detect the missing quoting as new diagnostics are
introduced. Do you think it might be scriptable?
Hi.
Are you talking about a proper GCC warning that will be triggered once a warning
message is missing quotations?
I can definitely cook a patch in next stage1 and the testsuite fall out should
be easy to come with.
Yes, issuing a -Wformat warning for __gcc_diag__ functions is what
I'm thinking of. A checker that would look for substrings within
the format string that match the "-[Wfm][a-z][a-z_1-9]*" patterns
(or anything else that matches an option) and point them out if
they're not enclosed in a pair of %< %> (or suggest to use %qs).

Martin
Martin Liška
2018-11-20 19:54:52 UTC
Permalink
Post by Martin Sebor
Post by Martin Liška
Post by Martin Sebor
Post by Martin Liška
Hi.
I've done a quick grep of gcc/po/gcc.pot and I see quite a lot of missing quotations
of option names (~400). Is it something we should fix? How important is that?
That's quite a few... I've been fixing these as I notice them,
usually as part of related changes.  The most onerous part of
the cleanup is adjusting tests, especially under the target-
specific ones.  It's (obviously) not critical but I think it
would be nice to make the quoting consistent throughout over
time (if not in one go) and then put in place a -Wformat
checker to detect the missing quoting as new diagnostics are
introduced.  Do you think it might be scriptable?
Hi.
Are you talking about a proper GCC warning that will be triggered once a warning
message is missing quotations?
I can definitely cook a patch in next stage1 and the testsuite fall out should
be easy to come with.
Yes, issuing a -Wformat warning for __gcc_diag__ functions is what
I'm thinking of.  A checker that would look for substrings within
the format string that match the "-[Wfm][a-z][a-z_1-9]*" patterns
(or anything else that matches an option) and point them out if
they're not enclosed in a pair of %< %> (or suggest to use %qs).
Martin
Sounds good to me. Well, I can imagine doing that for GCC 9 release.
When will you find spare cycles for the warning? In can prepare
the warning/error messages patch.

Thanks,
Martin
Martin Sebor
2018-11-21 00:48:28 UTC
Permalink
Post by Martin Liška
Post by Martin Sebor
Post by Martin Liška
Post by Martin Sebor
Post by Martin Liška
Hi.
I've done a quick grep of gcc/po/gcc.pot and I see quite a lot of
missing quotations
of option names (~400). Is it something we should fix? How
important is that?
That's quite a few... I've been fixing these as I notice them,
usually as part of related changes. The most onerous part of
the cleanup is adjusting tests, especially under the target-
specific ones. It's (obviously) not critical but I think it
would be nice to make the quoting consistent throughout over
time (if not in one go) and then put in place a -Wformat
checker to detect the missing quoting as new diagnostics are
introduced. Do you think it might be scriptable?
Hi.
Are you talking about a proper GCC warning that will be triggered once a warning
message is missing quotations?
I can definitely cook a patch in next stage1 and the testsuite fall out should
be easy to come with.
Yes, issuing a -Wformat warning for __gcc_diag__ functions is what
I'm thinking of. A checker that would look for substrings within
the format string that match the "-[Wfm][a-z][a-z_1-9]*" patterns
(or anything else that matches an option) and point them out if
they're not enclosed in a pair of %< %> (or suggest to use %qs).
Martin
Sounds good to me. Well, I can imagine doing that for GCC 9 release.
When will you find spare cycles for the warning? In can prepare
the warning/error messages patch.
I don't expect to have the time to work on the warning until
sometime in stage 1 (as an enhancement I also wouldn't expect
it to be approved, but maybe you can get away with it ;-)

Martin
Martin Liška
2018-11-27 16:36:47 UTC
Permalink
Post by Martin Sebor
Post by Martin Liška
Post by Martin Sebor
Post by Martin Liška
Post by Martin Sebor
Post by Martin Liška
Hi.
I've done a quick grep of gcc/po/gcc.pot and I see quite a lot of
missing quotations
of option names (~400). Is it something we should fix? How important is that?
That's quite a few... I've been fixing these as I notice them,
usually as part of related changes.  The most onerous part of
the cleanup is adjusting tests, especially under the target-
specific ones.  It's (obviously) not critical but I think it
would be nice to make the quoting consistent throughout over
time (if not in one go) and then put in place a -Wformat
checker to detect the missing quoting as new diagnostics are
introduced.  Do you think it might be scriptable?
Hi.
Are you talking about a proper GCC warning that will be triggered once a warning
message is missing quotations?
I can definitely cook a patch in next stage1 and the testsuite fall out should
be easy to come with.
Yes, issuing a -Wformat warning for __gcc_diag__ functions is what
I'm thinking of.  A checker that would look for substrings within
the format string that match the "-[Wfm][a-z][a-z_1-9]*" patterns
(or anything else that matches an option) and point them out if
they're not enclosed in a pair of %< %> (or suggest to use %qs).
Martin
Sounds good to me. Well, I can imagine doing that for GCC 9 release.
When will you find spare cycles for the warning? In can prepare
the warning/error messages patch.
I don't expect to have the time to work on the warning until
sometime in stage 1 (as an enhancement I also wouldn't expect
it to be approved, but maybe you can get away with it ;-)
Martin
That's fine, I've just noticed that to my TODO list for next stage1.

One related question: Is it fine to use apostrophes in dg-error/dg-warning patterns.
E.g.
gcc/testsuite/g++.dg/cpp1z/decomp48.C: return s; // { dg-warning "reference to local variable 's' returned" }

shouldn't that be { dg-warning "reference to local variable .s. returned" }?

Martin
Joseph Myers
2018-11-27 17:02:25 UTC
Permalink
Post by Martin Liška
One related question: Is it fine to use apostrophes in
dg-error/dg-warning patterns.
In general the testsuite sets LC_ALL to C (and to C.ASCII on platforms
where C means C.UTF-8), so yes.
--
Joseph S. Myers
***@codesourcery.com
Loading...