incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Black <abl...@roguewave.com>
Subject Re: [PATCH] stdcxx target warning counting
Date Thu, 12 Oct 2006 23:06:06 GMT
Take 3 attached, altered some, slightly altered log at end.

Martin Sebor wrote:
> Andrew Black wrote:
>> Revised patch, attached.  I believe the same change log can be used.
>>
> [...]
>> One version of the patch did do a case insensitive comparison, moving 
>> the rw_strcasecmp (and rw_charcasecmp) functions from exec.cpp to 
>> util.cpp.  The reason I chose to do things this way was so that 
>> different warning parsing modes could be specified for the compile and 
>> link stages.  An example where this could be useful would be when 
>> building with GCC on Solaris, but linking with Sun's linker.
> 
> That's a good point. But I'm afraid I'm not sure I see how
> capitalization can help with which linker is used.

I was using the same capitalization trick that was used for setting 
hard/soft limits.  I've reworked the parse_warn_opts method, simplifying 
the logic and accepting only a single alias code (in lower case).  A 
future patch would likely be to accept case insensitive alias names.

[...]

> 
> [...]
>> This is a math overflow check.  If we assume ~5 assertions take 1 k of 
>> disk space in a result log (each assertion ~200 characters (bytes) 
>> long), the resulting file would be ~800,000,000 kb, or ~800 Gb.  A 
>> file this long is theoretically within the capabilities of a modern 
>> OS, but you are likely to run out of disk space prior to writing the 
>> entire log.
>>
>> This check was added for consistency with the assertion counting code, 
>> and it would be outside the scope of this patch to remove that check 
>> (and the ST_OVERFLOW item).
> 
> Maybe. It doesn't mean that we need to introduce unnecessary logic
> into new code just for consistency with existing code. I don't
> suppose you would introduce a bug into new code just because we
> had the same bug in a similar piece of existing code, would you? :)
> Incidentally, if you insist on handling this exceedingly unlikely
> case, it would be better to indicate the overflow in the column
> that overflowed (since there are more than one) and not in the
> status column.

It makes sense.  In this revised patch, I set the value of t_warn to -1 
(Max unsigned int) if we hit overflow.  It would be a simple patch 
(later) to alter the existing logic to behave the same way, remove the 
ST_OVERFLOW item, and display an overflow message when the count reaches 
this value.

--Andrew Black

Log:
     * makefile.rules (%.o: %(AS_EXT), %.o: %.cpp, %: %.o, %: %.cpp): 
Add $(TEEOPTS) to compile/link line so that output is routed to log files.
     * makefile.common (TEEOPTS): Always tee output to $@.log.
     * target.h (target_opts): Add c_warn and l_warn fields.
       (target_status): Split warn field into c_warn, l_warn, t_warn.
     * output.cpp (check_test): Count warnings, store in status->t_warn.
       (check_compat_test): Expand tail horizon, capture warnings into 
status->t_warn.
     * display.cpp (print_header_plain): Add WARN column, rename WALL 
column to REAL.
       (print_status_plain): Print the total number of warnings in the 
WARN column.
     * runall.cpp (count_warnings): Add static method to count the 
number of warnings in a .log file
       (check_target_ok): Always calculate object file name, call 
count_warnings for object and executable.
     * cmdopt.cpp (usage_text): Document new --warn switch
       (parse_warn_opts): Define new helper function to parse --warn switch.
       (eval_options): Used here in parsing and to initialize default 
c_warn and l_warn values.

Mime
View raw message