On 5/31/19 12:20 PM, Jeff Law wrote:
On 5/31/19 9:56 AM, Martin Sebor wrote:
On 5/30/19 5:49 PM, Jeff Law wrote:
So in several places there's a comment which indicates that debugging
dumps and the like do not follow conventions. Presumably you've tried
to keep a narrow scope on the diagnostic push/pops. I'm also concerned
that the comments you mention that we trigger an ICE.
So while I'll ack this patch, I would like to know more about the ICE
that's triggered in the checker and what the plans are for fixing it.
Sorry, I didn't word the comment (copied below) very clearly.
What I meant to say is that the calls to error() in these files
that don't follow the convention are ultimately followed by
an ICE triggered either by an assert (as in cfgloop.c) or a call
to internal_error (cgraph.h). The diagnostics themselves don't
cause an ICE.
OK. Thanks for the clarification.
In a comment on one of the i18n bugs raised for these strings
Richard suggests these error calls should probably replaced by
direct calls to the pretty printer. That would let us avoid
suppressing the warnings and also presumably make it clear to
translators the format strings aren't meant to be translated.
It seemed like too big of a change for this patch so I simply
suppressed the warnings but I agree it's worth considering at
some point.
Agreed.
I'll adjust the comment before I check in the patch (I'm hoping
to commit it at the same time as the checker itself once it's
approved).
Your call on when to commit :-)
I just committed it in r271971 with a few minor tweaks. As before
I expect some minor fallout in the test suite, and more fixes to
follow once the checker itself is approved and committed.
Martin
