Enabled boost features and whatnot on TAO for Linux scoreboard build: "RHEL4-NoInline". Results visible at http://tao.prismtech.com/doc-scoreboard/rhel4/gcc344/standard/index.html

unit tests now don't depend on boost anymore. 

Info about what is failing:

-------- Original Message --------
Subject: Re: Sequence_Unit_Tests
Date: Tue, 04 Nov 2008 09:33:50 -0600
From: Adam Mitz <mitza@ociweb.com>
To: Simon McQueen <sm@prismtech.com>
CC: Johnny Willemsen <jwillemsen@remedy.nl>
References: <10265823.14051225712304396.JavaMail.root@newthor> <490EE75D.8020007@prismtech.com> <490EF91D.5050307@prismtech.com>

Simon McQueen wrote:
> Hi Adam,
>
> I've been taking a look at the Sequence_Unit_Tests. It seems we have
> some errors in them. They look like they originate way back with the
> commit you made:
>
> Fri Jun  8 19:23:45 UTC 2007  Adam Mitz  <mitza@ociweb.com>
>
>         * tao/Generic_Sequence_T.h:
>
>           A default-constructed unbounded sequence has no buffer
> allocated, so
>           "release" should be false.  This restores the behavior of
> sequences
>           before the recent re-write.
>
> The change you made in the tests looks like it checks whether the
> sequence is bounded or not. If the sequence is unbounded you expect that
> release will be false, as you say above.
>
> I'm not 100% sure but isn't the file Generic_Sequence_T.h used for both
> bounded and unbounded sequences ? We certainly seem to be seeing that a
> default constructed sequence with bounded_ == true seems to have release
> () false (see below).
>
>   

My change was correct at the time I made it.  I even ran the regression 
tests. :)  Later the bounded sequences were changed to not allocate 
their buffers at construction time.

Wed Aug 15 11:21:12 UTC 2007  Johnny Willemsen  <jwillemsen@remedy.nl>

       * tao/Bounded_Array_Allocation_Traits_T.h:
       * tao/Bounded_Reference_Allocation_Traits_T.h:
       * tao/Bounded_Value_Allocation_Traits_T.h:
       * tao/Valuetype/Bounded_Valuetype_Allocation_Traits_T.h:
         Don't preallocate all sequence members when using bound sequences,
         this could lead to a stack overflow when using recursive types
         as described in bugzilla 3042. Thanks to Stanislaw Trytek
         <tryteks at pit dot edu dot pl> for reporting this.

       * tests/Bug_3042_Regression/*:
         New regression test for bug 3042, thanks to Stanislaw Trytek
         <tryteks at pit dot edu dot pl> for creating this one

Now I'm not sure what needs to be done to make it work again.  Maybe 
only the tests have to change?  What should the release flag be for a 
default-constructed bounded sequence?  The buffer is null so "true" 
probably doesn't make sense.

Thanks,
Adam

-------- Original Message --------
Subject: RE: Sequence_Unit_Tests
Date: Tue, 4 Nov 2008 16:56:14 +0100
From: Johnny Willemsen <jwillemsen@remedy.nl>
Reply-To: <jwillemsen@remedy.nl>
Organisation: Remedy IT Expertise BV
To: 'Simon McQueen' <sm@prismtech.com>, 'Adam Mitz' <mitza@ociweb.com>
References: <10265823.14051225712304396.JavaMail.root@newthor> <490EE75D.8020007@prismtech.com> <490EF91D.5050307@prismtech.com> <49106B5E.6020108@ociweb.com> <49106DCF.1000601@prismtech.com>

Hi,

> > What should the release flag be for a
> > default-constructed bounded sequence?  The buffer is null so "true"
> > probably doesn't make sense.
> 
> Probably false if the default buffer allocation thing is zero then I
> guess. Perhaps Johnny could say for definite ?

I think the test needs an update.

Johnny

From Adam the reply below, Simon, can you have a look at Adam's reply

I looked at the bounded object ref test and committed some changes, but
there are definitely some problems.  I think all of the remaining
failures that are being logged are due to this issue:

The default_initializer_calls value doesn't match the expected value. 
Was there a deliberate change to TAO in this area (I seem to remember
Simon from PrismTech working on this)?  It doesn't make much sense for
default_initializer_calls to be > 0 when allocbuf_calls is 0.

I only had a quick look at this test but my recollection is that as unit tests they were just verifying the 'status quo' in terms of the implementation rather than some tangible spec (or other) requirement for sequences. It seems plausible that they would therefore start returning errors whenever the implementation changes. Even if that change is to change from 'wrong' to 'right'.

Vladimir is our sequence expert however - I'll ask him if he wouldn't mind reviewing this ticket and the current state of the tests and giving us his opinion.

It sounds probably just like the expected value needs to change but he's best qualified to tell us if so.

Carlos wrote originally the tests before the implementation. he used the spec and wrote tests what the implementation should do. When I merged in his work things did run without errors, my biggest mistake was to keep the boost stuff at that moment. The implementation seems to have changed, but the tests haven't been checked for that. That should happen now, the tests should run fine, maybe there are bugs in the current implementation, maybe the expected calls have changed?

I want to take this but what I've seen until now is as Simon mentioned the test checks the implementation.

mine

(In reply to comment #7)
> I want to take this but what I've seen until now is as Simon mentioned the test
> checks the implementation.
> 

Here's something in particular to look at:
In file bounded_object_reference_sequence_ut.cpp, test_buffer_constructor_release_false() contains these three checks:
      FAIL_RETURN_IF_NOT(a.expect(0), a);
      FAIL_RETURN_IF_NOT(f.expect(0), f);
      FAIL_RETURN_IF_NOT(i.expect(0), i);
The third one is failing.  Does it make sense for a sequence that did 0 allocations and 0 frees to do initialization?  If it those initializations are by-design, then change the test.  Is this related to 3524?

the tests where written before the implementation, the idea was to have explicit tests to determine if the correct allocations, initialize, etc was done by the implementation. at the moment they got added to the repo they all ran without errors. the most tricky part seems to determine whether the sequence code is wrong or the test. I did remove the boost usage using find/replace, so also check the original boost code in case of real doubts. for the example of Adam, I think the test is correct, so this seems to be a bug in the code

(In reply to comment #9)
> Here's something in particular to look at:
> In file bounded_object_reference_sequence_ut.cpp,
> test_buffer_constructor_release_false() contains these three checks:
>       FAIL_RETURN_IF_NOT(a.expect(0), a);
>       FAIL_RETURN_IF_NOT(f.expect(0), f);
>       FAIL_RETURN_IF_NOT(i.expect(0), i);
> The third one is failing.  Does it make sense for a sequence that did 0
> allocations and 0 frees to do initialization?  If it those initializations are
> by-design, then change the test.  Is this related to 3524?
> 

No. I'd rather say it's related to 3574.

However, the test you mentioned has to be changed. A snippet from the questionable function is below.

....
      CORBA::ULong maximum = 64;
      tested_sequence::value_type * data = tested_sequence::allocbuf(maximum);
      a.reset();
      {
        tested_sequence x(maximum / 2, data, false);
....

Since it uses allocbuf and the later changed according to the C++ mapping (see bug#3473) then the above call will lead to 64 default initializer calls. There are a lot of similar checks in the those tests. Currently I'm trying to track them down. Some of them has to be changed and some just removed as things that are not required by the spec.

(In reply to comment #11)
> (In reply to comment #9)
> Some of them has to be changed and some just removed as things that
> are not required by the spec.

Some parts we check in the tests is as you say not in the spec, but we want to make sure number of allocs/inits/etc is as expected. Not testing that explicitly could do unwanted behaviour or a performance drop

I think I found a bug in the generic_sequence. I don't want to report it separately because it's related to how this unit test will work but I need a feedback for this.

C++ mapping says that if release flag of a sequence is FALSE then "...old storage will not be freed for either the elements or for the buffer...". In a number of places TAO fits this requirement (sequence destructor, replace, get_buffer, etc) bun in length() no check for release flag is done. Thus it can happen that sequence can release elements that it doesn't own when it's asked to shrink. Also the spec doesn't provide (at least I couldn't find) a clear description of what has to be done in this case. What shall we do? Maybe it's worth to discuss this in the devo-group?

raise it in devo

The summary from devo-group discussion.

> At the same time there is such
> a statement in the specification. "Setting the length to a smaller value
> than the current length does not affect how the storage associated with
> the sequence is manipulated. Note, however, that the elements orphaned
> by this reduction are no longer accessible and that their values cannot
> be recovered by increasing the sequence length to its original value."
> The later is difficult to achieve when we only update length without
> changing the buffer. Thus we allocate a new buffer. Or can we ignore
> the above statement from the spec?  

    The statement from the spec is a restriction on the *users* of the
sequence, not on the implementation.

    Break it down in two cases:  if the sequence owns the storage, a user of
sequences cannot make any assumptions about the elements after the sequence
length is reduced.  The sequence may or may not release the storage
immediately.  It might have some sophisticated mechanism to clean them up
when the sequence size is increased again, ours don't but that is besides
the point.

    If the user owns the storage then the sequence is not supposed to touch
the elements.  The user is basically saying: "I will manage the storage for
this buffer and its elements, don't worry your pretty little head about it."
As an implementor of the sequences we have no responsibilities beyond that
contract.  We can re-allocate when the user calls length() or we can simply
change the length() and let the user worry about keeping his own length
field.  If the user calls operator[] and re-assigns an element it is not our
problem to release it, it is the user of the sequence that has to make sure
he uses the buffer correctly.

Created attachment 1108 [details]
Changes required to the sequence unit test in order to make it work. All the changes are commented in the patch.

Created attachment 1109 [details]
Fixes for problems that I found in tao when I worked on the sequence unit test.

There are several changes.

1) In copy constructor. When a new sequence is created from a sequence that has buffer_ == 0 (which is only possible when length=0) there is no need to allocate a buffer in a new sequence. Also if the original sequence has length_ < maximum_ then we have to initialize elements length_ through maximum_.

2) There were several places in Unbounded_Octet_Sequence_T.h where release_ was not properly set when buffer_ was updated.

3) There is no need for initialize_range in Unbounded_Octet_Sequence_T.h since this is a sequence of primitive types and C++ mapping says that "Sequence elements of a basic type, such as ULong, have undefined default values."

4) I added allocbuf() with no arguments to bounded sequences as it's required by the C++ mapping (ver 1.2).

In revision 85009.

Wed Apr  1 14:28:11 UTC 2009  Vladimir Zykov  <vz@prismtech.com>

        * tests/Sequence_Unit_Tests/value_sequence_tester.hpp:
        * tests/Sequence_Unit_Tests/unbounded_sequence_cdr_ut.cpp:
        * tests/Sequence_Unit_Tests/bounded_value_sequence_ut.cpp:
        * tests/Sequence_Unit_Tests/bounded_object_reference_sequence_ut.cpp:
        * tests/Sequence_Unit_Tests/unbounded_object_reference_sequence_ut.cpp:
        * tests/Sequence_Unit_Tests/string_sequence_tester.hpp:
        * tests/Sequence_Unit_Tests/run_test.pl:
        * tests/Sequence_Unit_Tests/object_reference_sequence_element_ut.cpp:
        * tests/Sequence_Unit_Tests/testing_counters.hpp:
        * tests/Sequence_Unit_Tests/bounded_string_sequence_ut.cpp:
        * tests/Sequence_Unit_Tests/unbounded_octet_sequence_nocopy_ut.cpp:
        * tests/Sequence_Unit_Tests/bounded_sequence_cdr_ut.cpp:
        * tests/Sequence_Unit_Tests/unbounded_string_sequence_ut.cpp:
        * tests/Sequence_Unit_Tests/testing_allocation_traits.hpp:
        * tests/Sequence_Unit_Tests/string_sequence_element_ut.cpp:
        * tests/Sequence_Unit_Tests/unbounded_value_sequence_ut.cpp:
        * tests/Sequence_Unit_Tests/testing_allocation_traits_ut.cpp:
        * tests/Sequence_Unit_Tests/unbounded_octet_sequence_ut.cpp:
        * tests/Sequence_Unit_Tests/Sequence_Unit_Tests.mpc:
        * tests/Sequence_Unit_Tests/unbounded_fwd_object_reference_sequence_ut.cpp:
        * tests/Sequence_Unit_Tests/Makefile.am:

          Updated the test with respect to all recent changes in the
          implementation of sequences in TAO. This fixes bug#3488.

        * tao/Unbounded_Octet_Sequence_T.h:
        * tao/Generic_Sequence_T.h:

          Fixed several things. 1) In copy constructor. When a new
          sequence is created from a sequence that has buffer_==0
          there is no need to allocate a buffer in a new sequence.
          Also if the original sequence has length_<maximum_ then we
          have to initialize elements length_ through maximum_.
          2) There were several places in Unbounded_Octet_Sequence_T.h
          where release_ was not properly set when buffer_ was updated.
          3) There is no need for initialize_range in
          Unbounded_Octet_Sequence_T.h since this is a sequence of
          primitive types and C++ mapping says that "Sequence
          elements of a basic type, such as ULong, have undefined default
          values."

        * tao/Bounded_Array_Sequence_T.h:
        * tao/Valuetype/Bounded_Valuetype_Sequence_T.h:
        * tao/Bounded_Object_Reference_Sequence_T.h:

          Added allocbuf() with no arguments to bounded sequences as it's
          required by the C++ mapping (ver 1.2).

In 85044.

Tue Apr  7 09:01:37 UTC 2009  Vladimir Zykov  <vz@prismtech.com>

        * tests/Sequence_Unit_Tests/clean_tao_export.pl:
        * tests/Sequence_Unit_Tests/unbounded_octet_sequence_nocopy_ut.cpp:
        * tests/Sequence_Unit_Tests/Sequence_Unit_Tests.mpc:

          Fixed a test that uses specialization of unbounded_value_sequence
          for CORBA::Octet on win32. This test requires that the above
          specialization is compiled into tests code and not imported from
          TAO.dll as it was done before.

In 85063.

Thu Apr  9 11:01:26 UTC 2009  Vladimir Zykov  <vz@prismtech.com>

        * tests/Sequence_Unit_Tests/unbounded_octet_sequence_nocopy_ut.cpp:
        * tests/Sequence_Unit_Tests/Sequence_Unit_Tests.mpc:

          There were compile errors on Linux and Solaris builds with
          previous attempt to fix octet sequence test on Windows. Now
          those custom build steps are done only on windows builds.

In 85079.

Fri Apr 10 12:44:12 UTC 2009  Vladimir Zykov  <vz@prismtech.com>

        * tests/Sequence_Unit_Tests/clean_tao_export.pl:
        * tests/Sequence_Unit_Tests/run_test.pl:
        * tests/Sequence_Unit_Tests/unbounded_octet_sequence_nocopy_ut.cpp:
        * tests/Sequence_Unit_Tests/Sequence_Unit_Tests.mpc:

          Reverted my previous attempts to fix this test on windows
          and disabled unbounded octet sequence test on this platform.