[vtk-developers] Use of std::cout in tests
Bill Lorensen
bill.lorensen at gmail.com
Thu Sep 20 11:41:41 EDT 2012
But, it could segfault for have valgrind defects... It is a form of
testing to check for memory access and leaks.
On Thu, Sep 20, 2012 at 11:13 AM, Marcus D. Hanwell <
marcus.hanwell at kitware.com> wrote:
> This sounds reasonable to me, I would like to point out that being
> able to selectively pull symbols into the current namespace is a
> feature of namespaces, using std::cout to bring in a symbol you will
> use a lot is pretty common.
>
> We should be careful to use the new C++ headers consistently, and if
> we are going to relax this language then I still think that changing
> all occurrences to your preferred style should be discouraged in
> existing code (this will add noise to changes, and confuse functional
> changes over style changes), instead preferring the style currently
> used in that file.
>
> Even if the style is not applied everywhere we should still have an up
> to date style guide, and aim to stick with it automated tools or not.
> As a new contributor to a project that is one of the things I look
> for, or simply try to stick to the existing style in a given source
> file.
>
> My major objection to the patch that started all this was that whilst
> it increased coverage it did not actually verify any of the results
> and would always pass whether the output was correct or not (assuming
> it didn't segfault). I want to increase coverage, but would like to
> ensure that the tests will fail if someone breaks the code, and that
> is also why I am not sure I see the value in outputting lots of data
> (it appears I misunderstood duplicate CDash data, and that is far less
> of an issue than I thought it was).
>
> Marcus
>
> On Wed, Sep 19, 2012 at 5:44 PM, David Gobbi <david.gobbi at gmail.com>
> wrote:
> > The only reason that VTK didn't use std:: historically was so that it
> > could support pre-standard or broken compilers. The last badly broken
> > compiler we had to support was MSVC 2002. But we don't support it
> > anymore, do we?
> >
> > Therefore there is no longer any reason to avoid std::cout, and the
> > style guidelines should be amended.
> >
> > So vtkstd:: has become std:: as of VTK 6
> > likewise std::cout should be preferred over cout
> > and std::string over vtkStdString.
> >
> > I don't like to see style guidelines being broken (even in tests) so
> > the sooner the guidelines can be amended, the better. Just remove the
> > sentence that insists on using "cout". And please do not add a line
> > that insists on "std::cout".
> >
> > - David
> >
> >
> > On Wed, Sep 19, 2012 at 3:27 PM, Bill Lorensen <bill.lorensen at gmail.com>
> wrote:
> >> Julien,
> >>
> >> In the early days of VTK, at GE, coding style was a high priority. At
> that
> >> time we had a paid-for utility that enforced it. Later, before KWStyle,
> VTK
> >> stopped enforcing the style.
> >>
> >> Someday, I hope, we can use KWStyle to bring the style back to what is
> used
> >> to be. But this will be a big effort. First to define the style via
> KWStyle,
> >> then to bring VTK into line with the style, and finally to maintain it.
> >>
> >> BTW, I still feel that code coverage should be the high priority. But,
> every
> >> time we bring this up, people raise issues about process, style, output,
> >> etc. I'd rather have code covered with tests that produce too much
> output,
> >> or use varying styles versus no test coverage at all.
> >>
> >> Bill
> >>
> >>
> >>
> >> On Wed, Sep 19, 2012 at 5:13 PM, Julien Finet <julien.finet at kitware.com
> >
> >> wrote:
> >>>
> >>> Actually, the coding style should have been defined before (aka is the
> top
> >>> priority) the first unit test was written. And the latter should have
> been
> >>> written before the first code was written :-P
> >>> Defining/changing/applying coding style might be a good thing to do in
> VTK
> >>> 6.0 (higher priority) as it might change the API. Code coverage (and
> fixing
> >>> bugs) can always be done in succeeding minor versions.
> >>>
> >>> Concerning your 2nd question (in your first email), if it comes to a
> vote,
> >>> I would vote in favor of always prefixing with the namespace of the
> external
> >>> library (here std::), as long as it is the standard namespace name
> (std::
> >>> and not vtkstd::). It can't hurt to be specific.
> >>>
> >>> Julien.
> >>>
> >>> On Wed, Sep 19, 2012 at 4:56 PM, Bill Lorensen <
> bill.lorensen at gmail.com>
> >>> wrote:
> >>>>
> >>>> David,
> >>>>
> >>>> There is not style checknig in VTK, nor is there an consistent style
> in
> >>>> VTK. Years ago we had a style and enforced through a third party
> package.
> >>>>
> >>>> ITK enforces style with KWStyle and we have nightly tests that check
> the
> >>>> style.
> >>>>
> >>>> To be honest, style is pretty low in priority versus code coverage and
> >>>> valgrind.
> >>>>
> >>>> Bill
> >>>>
> >>>>
> >>>> On Wed, Sep 19, 2012 at 4:52 PM, David Thompson
> >>>> <david.thompson at kitware.com> wrote:
> >>>>>
> >>>>> I don't want to be so strict about formatting that we start using
> >>>>>
> >>>>> http://uncrustify.sourceforge.net/
> >>>>>
> >>>>> as another pass before commits can be pushed to Gerrit, but it would
> be
> >>>>> nice to have a "style file" for uncrustify (or a similar tool) that
> embodies
> >>>>> the VTK code style so less code review time is spent on formatting
> and more
> >>>>> on the logic, documentation, tests, and organization.
> >>>>>
> >>>>> David
> >>>>>
> >>>>> On Sep 19, 2012, at 4:40 PM, "Marcus D. Hanwell"
> >>>>> <marcus.hanwell at kitware.com> wrote:
> >>>>>
> >>>>> > On Wed, Sep 19, 2012 at 4:06 PM, Bill Lorensen
> >>>>> > <bill.lorensen at gmail.com> wrote:
> >>>>> >> Folks,
> >>>>> >>
> >>>>> >> There has been some recent turmoil regarding the use of std::cout
> in
> >>>>> >> tests
> >>>>> >> versus cout. The current style guidelines recommend the latter.
> >>>>> >> First, why is this even an issue. Second, can we amend the style
> to
> >>>>> >> allow
> >>>>> >> std::cout? Much c++ code I see uses std::cout.
> >>>>> >>
> >>>>> >> I'd much rather spend time improving VTK'sl ow coverage that argue
> >>>>> >> over what
> >>>>> >> seems to be a non-issue.
> >>>>> >>
> >>>>> > I guess I was the source of the aforementioned turmoil. I think it
> is
> >>>>> > important to stick to a common style, and actively changing test
> code
> >>>>> > to not match the style seems bad to me. I don't see the point in
> fully
> >>>>> > specifying std::cout when we have brought it into the global
> >>>>> > namespace, but if there is a strong feeling we should then my point
> >>>>> > was simply that we should specify that is the preferred style from
> >>>>> > this point forward (or that either is acceptable).
> >>>>> >
> >>>>> > If we all start modifying the VTK coding standards then our code
> will
> >>>>> > be far less consistent, and harder to understand due to using
> several
> >>>>> > styles. If there is general agreement on relaxing some parts of the
> >>>>> > style I really don't feel very strongly, but I think it should be
> done
> >>>>> > deliberately rather than on an ad-hoc basis.
> >>>>> >
> >>>>> > Where would you draw the line if this should be relaxed? Could
> >>>>> > standard indentation be used instead of the special VTK
> indentation in
> >>>>> > classes I author? I am sure we don't want to allow that, but it
> does
> >>>>> > require some definition on what is strictly required and what is
> >>>>> > advised/optional.
> >>>>> >
> >>>>> > Marcus
> >>>>> > _______________________________________________
> >>>>> > Powered by www.kitware.com
> >>>>> >
> >>>>> > Visit other Kitware open-source projects at
> >>>>> > http://www.kitware.com/opensource/opensource.html
> >>>>> >
> >>>>> > Follow this link to subscribe/unsubscribe:
> >>>>> > http://www.vtk.org/mailman/listinfo/vtk-developers
> >>>>> >
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>> --
> >>>> Unpaid intern in BillsBasement at noware dot com
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> Powered by www.kitware.com
> >>>>
> >>>> Visit other Kitware open-source projects at
> >>>> http://www.kitware.com/opensource/opensource.html
> >>>>
> >>>> Follow this link to subscribe/unsubscribe:
> >>>> http://www.vtk.org/mailman/listinfo/vtk-developers
> >>>>
> >>>>
> >>>
> >>
> >>
> >>
> >> --
> >> Unpaid intern in BillsBasement at noware dot com
> >>
> >>
> >> _______________________________________________
> >> Powered by www.kitware.com
> >>
> >> Visit other Kitware open-source projects at
> >> http://www.kitware.com/opensource/opensource.html
> >>
> >> Follow this link to subscribe/unsubscribe:
> >> http://www.vtk.org/mailman/listinfo/vtk-developers
> >>
> >>
>
--
Unpaid intern in BillsBasement at noware dot com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20120920/cb9430de/attachment.html>
More information about the vtk-developers
mailing list