Chrono IO state savers inconsistent with Boost.IO state savers

[rkawulak]

The constructors of Boost.IO state savers taking the second new_value argument set the state to the new_value immediately and restore the previous state upon destruction. OTOH, the Boost.Chrono IO state savers do something different (and arguably unintuitive): they don't alter the state upon construction at all and restore the state to new_value rather than the previous state upon destruction. The behaviour should be aligned with that of Boost.IO to avoid confusion and make it more useful.

[viboes]

Please, could you be more explicit?

Are you referring to

    struct duration_style_io_saver
    {

      //! the type of the state to restore
      typedef std::ios_base state_type;
      //! the type of aspect to save
      typedef duration_style aspect_type;

      /**
       * Explicit construction from an i/o stream.
       *
       * Store a reference to the i/o stream and the value of the associated @c duration_style.
       */
      explicit duration_style_io_saver(state_type &s) :
        s_save_(s)
      {
        a_save_ = get_duration_style(s_save_);
      }

      /**
       * Construction from an i/o stream and a @c duration_style to restore.
       *
       * Stores a reference to the i/o stream and the value @c duration_style to restore given as parameter.
       */
      duration_style_io_saver(state_type &s, aspect_type new_value) :
        s_save_(s), a_save_(new_value)
      {
      }

      /**
       * Destructor.
       *
       * Restores the i/o stream with the duration_style to be restored.
       */
      ~duration_style_io_saver()
      {
        this->restore();
      }

      /**
       * Restores the i/o stream with the duration_style to be restored.
       */
      void restore()
      {
        set_duration_style(s_save_, a_save_);
      }

    private:
      duration_style_io_saver& operator=(duration_style_io_saver const& rhs) ;

      state_type& s_save_;
      aspect_type a_save_;
    };

[rkawulak]

Yes. The second constructor should be implemented the following way to be consistent with IO state saver semantics (see for example ​basic_ios_iostate_saver):

duration_style_io_saver(state_type &s, aspect_type new_value) :
  s_save_(s), a_save_(get_duration_style(s))
{
  set_duration_style(s, new_value);
}

The same for all the remaining Chrono IO state savers.

[viboes]

Thanks for catching this point. I missed it completly. I should stop adding code that I don't test.

[viboes]

Have you find other cases?

[rkawulak]

IIRC, all IO state savers in Chrono library are implemented this way.

[viboes]

​https://github.com/boostorg/chrono/commit/d08ef2352a68a8b616c7e14cf30066204907f019

[rkawulak]

Seems that not all documentation comments have been updated accordingly.

[viboes]

Ok, I will take a look.

[viboes]

​https://github.com/boostorg/chrono/commit/ec29de7bf0ea3a83ee54f449438624f8c15c8139