[vtk-developers] VTK coding conventions clarification

[David Lonie]

Sounds like removing the restriction on std::string in Core has the most
support. Any objections? I'll add std::string support to vtkOStreamWrapper
and update the conventions document later this week if I don't hear any
reasons why this shouldn't be done.

Thanks!
Dave


On Tue, Mar 18, 2014 at 11:56 AM, David Gobbi <david.gobbi at gmail.com> wrote:

> No worries about the wrappers, they have all supported std::string
> just fine ever since VTK 5.8.  And std::string should definitely be
> preferred over vtkStdString as an interface type.
>
>   David
>
>
> On Tue, Mar 18, 2014 at 9:46 AM, Sebastien Jourdain
> <sebastien.jourdain at kitware.com> wrote:
> > I'm not sure for Java. When vtkStdString was used, we were getting a
> > vtkStdString object and not a regular Java String object.
> > But don't know for std::string...
> >
> > Seb
> >
> >
> > On Tue, Mar 18, 2014 at 9:38 AM, David Lonie <david.lonie at kitware.com>
> > wrote:
> >>
> >> On Tue, Mar 18, 2014 at 11:33 AM, Berk Geveci <berk.geveci at kitware.com>
> >> wrote:
> >>>
> >>> Not an answer to your question but a concern. Did you verify that
> >>> public methods that use std::string are wrapped properly in all
> >>> languages that we support? If not, you will have provide public
> >>> methods that use char* in addition. Or add support for std::string to
> >>> all wrapper generators. I suspect that Python works but I don't know
> >>> about others.
> >>
> >>
> >> I have not, I assumed that the rule in the guidelines implies that they
> >> are supported. I can test this out if we decide to keep std::strings
> around
> >> in our API.
> >>
> >> Does anyone who works on the wrappings know off the top of their heads?
> >>
> >> Dave
> >>
> >>>
> >>> On Tue, Mar 18, 2014 at 11:09 AM, David Lonie <david.lonie at kitware.com
> >
> >>> wrote:
> >>> > Hi list,
> >>> >
> >>> > Referring to the document here:
> >>> >
> >>> > http://public.kitware.com/Wiki/VTK_Coding_Standards
> >>> >
> >>> > there are two conventions that appear to be in conflict:
> >>> >
> >>> > 27. STL usage is disallowed in the Common modules' public API, and
> >>> > should
> >>> > only be used in implementation files there. The other modules may use
> >>> > STL,
> >>> > but should do so only when necessary if there is not an appropriate
> VTK
> >>> > class. Care should be taken when using the STL in public API,
> >>> > especially in
> >>> > the context of what can be wrapped.
> >>> >
> >>> > Rationale: limits header inclusion bloat, wrappers are not capable of
> >>> > handling many non-vtkObject derived classes.
> >>> >
> >>> >
> >>> > and
> >>> >
> >>> > 36. Do not use vtkStdString in new API; prefer std::string.
> >>> >
> >>> > Rationale: vtkStdString was introduced as a workaround for compilers
> >>> > that
> >>> > couldn't handle the long symbol name for the expanded std::string
> type.
> >>> > It
> >>> > is no longer needed on modern platforms.
> >>> >
> >>> >
> >>> > I've added some new API that uses std::string to set some character
> >>> > string
> >>> > attributes, but vtkSetMacro and friends all try to pass the value to
> >>> > the
> >>> > debug stream, which uses the vtkOStreamWrapper from Common/Core.
> Since
> >>> > compilers and standard library implementation have matured since many
> >>> > of
> >>> > these rules were put in place, I wonder if it may be time to relax
> some
> >>> > of
> >>> > these restrictions?
> >>> >
> >>> > To specifically address my issue of having new API using std::string
> >>> > that
> >>> > can't be used with the VTK macros, I see a few different solutions:
> >>> >
> >>> > 1) Don't use the macros. This doesn't really address the issue, but
> >>> > will let
> >>> > us avoid it for a while.
> >>> >
> >>> > 2) Use the vtkSetStringMacro etc that operate char* member variables.
> >>> > The
> >>> > explicit memory handling of this approach is annoying, and it's nice
> to
> >>> > be
> >>> > able to use non-C-array string representations that clean up after
> >>> > themselves, copy/compare as objects, etc.
> >>> >
> >>> > 3) Remove rule #27 and allow STL containers everywhere, or at least
> >>> > make an
> >>> > exception for std::string if it is to be our preferred string class.
> >>> >
> >>> > 4) Remove rule #36 and make vtkStdString the defacto string class for
> >>> > the
> >>> > VTK library.
> >>> >
> >>> > Any insight would be appreciated :-)
> >>> >
> >>> > Dave
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20140319/e848257e/attachment-0002.html>