21.strings.cpp bug

[Farid Zaripov <Fa...@kyiv.vdiweb.com>]

I found the following code in 21.strings.cpp line 1504:

                 if (StringIds::None == iter_types [l]) {
                     // skip a non-sensical template specialization
 >                   break;
                 }

Here the "break" clause should be replaced with "continue".

Suppose that in the StringTest array some template method is situated 
before any non-template method(s).
The current version will skip these non-template methods exercising 
because of this "break".

Example:

21.string.cons.cpp

     static const StringTest
     tests [] = {

#undef TEST
#define TEST(sig) {                                             \
         Cons (sig), sig ## _test_cases,                         \
         sizeof sig ## _test_cases / sizeof *sig ## _test_cases  \
     }

	  [...]

         TEST (range),

#undef TEST
#define TEST(sig) {                             \
         OpSet (sig), sig ## _op_set_test_cases, \
           sizeof sig ## _op_set_test_cases      \
         / sizeof *sig ## _op_set_test_cases     \
     }

         TEST (cptr),
         TEST (cstr),
         TEST (val)

     };


Test results (OpSet(cptr), OpSet(cstr), OpSet(val) were not tested):
# +-----------------------+--------+--------+--------+
# | DIAGNOSTIC            | ACTIVE |  TOTAL |INACTIVE|
# +-----------------------+--------+--------+--------+
# | (S1) INFO             |    171 |    171 |     0% |
# | (S2) NOTE             |      0 |    170 |   100% |
# | (S7) ASSERTION        |      0 |  17910 |   100% |
# +-----------------------+--------+--------+--------+


If "break" is replaced to "continue" (all overloads were tested):
# +-----------------------+--------+--------+--------+
# | DIAGNOSTIC            | ACTIVE |  TOTAL |INACTIVE|
# +-----------------------+--------+--------+--------+
# | (S1) INFO             |    201 |    201 |     0% |
# | (S2) NOTE             |      0 |    200 |   100% |
# | (S7) ASSERTION        |      0 |  21843 |   100% |
# +-----------------------+--------+--------+--------+


Farid.

Re: 21.strings.cpp bug

[Martin Sebor <se...@roguewave.com>]

Farid Zaripov wrote:
> I found the following code in 21.strings.cpp line 1504:
> 
>                 if (StringIds::None == iter_types [l]) {
>                     // skip a non-sensical template specialization
>  >                   break;
>                 }
> 
> Here the "break" clause should be replaced with "continue".
> 
> Suppose that in the StringTest array some template method is situated 
> before any non-template method(s).
> The current version will skip these non-template methods exercising 
> because of this "break".

I think you are correct. IIRC, this was supposed to be a simple
optimization but it looks like it was ill-conceived. But now that
I'm looking at the code more closely I'm beginning to wonder if
it isn't backwards. I.e., it seems to me that the loop that
iterates over the different iterator categories and types should
be nested in the one that loops over the tests, and should only
be entered when 1 < _rw_argno (test.which, StringIds::arg_range))
is true. What do you think about the attached patch?

Martin