[vtk-developers] STL in VTK Header Files

[David Thompson]

> > Or maybe it is time to revise the coding standards? I don't know what the
> > correct process is for doing this though.
> 
> I don't think that we need to revise this particular rule. It would be
> confusing if it said "don't include STL headers UNLESS you have leaf
> class or if it is a private header". I think that it is enough to
> allow for exceptions. The same way we allow for exceptions to
> including only the superclass' header in a .h file. Having rules that
> are enforced blindly is never a good idea - there is almost always a
> time when you have to break a rule.

I agree that there shouldn't be a hard and fast rule, but I think that
the coding guidelines should include the reasons the decision was made.
Knowing the intent helps people who weren't involved in developing the
guidelines decide when to use "// STL include" and when not to. The
current guidelines (which actually aren't in the coding standards but
instead in the FAQ) do call out compile time as one reason for the
restriction but they do not mention the fact that Windows doesn't allow
STL objects to be passed across library boundaries. I would hope that
gets added along with a note on how to create an exception for leaf or
private header files -- it's crazy to hide the rules thus guaranteeing
the frustration of someone trying to check in a private header file. I'd
also suggest moving the notes into the coding guidelines and having the
FAQ point to the guidelines instead of the other way around. If no
objects, I would be glad to make the change and have pasted a modified
version of the FAQ entry below.

	David


=== Can I use STL with VTK? ===

As of VTK version 4.2, you can use STL. However, the following policy applies.

# STL is for implementation, not interface. All STL references should be contained in a .cxx class or the private section of the .h header file. Microsoft operating systems do not allow STL objects to be passed across library boundaries due to memory allocation issues; this prevents their use in programming interfaces.
# Use the PIMPL idiom to forward reference/contain STL classes in heavily used superclasses. STL is big, fat, and slow to compile so we do not want to include STL headers in any .h files that are included by most of VTK, e.g. vtkObject.h vtkSource.h etc.
# Include the VTK wrapper header files: vtkstd/map instead of map.
# Use the vtkstd:: namespace to refer to STL classes and functions.
# For private classes whose declaration must be shared (e.g., so that subclasses may access them) or leaf classes whose header files will only be included by one implementation file, you may include STL headers by adding the comment "// STL include" to the end of each STL header include directive. Without this comment, you will not be allowed to commit the file to the repository. This should be avoided whenever possible.

Here's an example (from vtkInterpolateVelocityField):

In the .h file (the PIMPL) forward declare

 class vtkInterpolatedVelocityFieldDataSetsType;
 //
 class VTK_COMMON_EXPORT vtkInterpolatedVelocityField : public vtkFunctionSet
 {
 private:
   vtkInterpolatedVelocityFieldDataSetsType* DataSets;
 };

In the .cxx file define the class (here deriving from the STL vector
container)

 # include <vtkstd/vector>
 typedef vtkstd::vector< vtkSmartPointer<vtkDataSet> > DataSetsTypeBase;
 class vtkInterpolatedVelocityFieldDataSetsType: public DataSetsTypeBase
 {};

In the .cxx file construct and destruct the class:

 vtkInterpolatedVelocityField::vtkInterpolatedVelocityField()
 {
   this->DataSets = new vtkInterpolatedVelocityFieldDataSetsType;
 }
 vtkInterpolatedVelocityField::~vtkInterpolatedVelocityField()
 {
   delete this->DataSets;
 }

And in the .cxx file use the container as you would any STL container:

 for ( DataSetsTypeBase::iterator i = this->DataSets->begin();
       i != this->DataSets->end(); ++i)
   {
   ds = i->GetPointer();
   ....
   }



> 
> I looked at the Python code that enforces the STL header rule and it
> would be easy to change it to ignore cases like
> 
>  #include<vtkstd/vector> // STL include
> 
> -berk
> 
> On Mon, Jun 8, 2009 at 8:51 AM, Dave Partyka<dave.partyka at kitware.com> wrote:
> > Well one thing we can do is come up with a file naming convention or some
> > string in the header file to parse that would cause the commit check to
> > permit the commit even if there are STL headers. But that is rather dubious
> > as then anyone can just use whatever we come up with rather than just
> > following the convention in the first place. Dave Thompson example (files
> > named vtk*Private.h") can be easily implemented, but that won't help all
> > cases.
> > Or maybe it is time to revise the coding standards? I don't know what the
> > correct process is for doing this though.
> >
> > On Fri, Jun 5, 2009 at 9:47 PM, Berk Geveci <berk.geveci at kitware.com> wrote:
> >>
> >> Is there a way of by-passing this commit check? There are a rare
> >> occasions where it may be necessary to commit code which include STL
> >> headers in .h files.
> >>
> >> To remind everyone why the decision to keep STL headers from include
> >> files was made: performance. STL headers are bulky and may slow down
> >> compilation significantly if they are included in header files that
> >> are included by a lot of other files. Including STL headers in the
> >> include files of leaf classes (such as concrete algorithm classes)
> >> does not have any effect on compilation time. There are no other good
> >> reasons why STL headers cannot be included in header files.
> >>
> >> Here is the catch: we occasionally receive contributions that include
> >> STL header in .h. These tend to be super big reader classes. Fixing
> >> the whole darn thing to use PIMPL is a waste of time which we don't
> >> have. So, if we do not provide a way of having exception to this rule,
> >> those contributions would never make it into VTK.
> >>
> >> So we should either provide a way of by-passing this commit check in
> >> these rare occasions or we should enforce this through a header test
> >> which can support such exceptions.
> >>
> >> -berk
> >>
> >> On Fri, Jun 5, 2009 at 5:13 PM, Francois
> >> Bertel<francois.bertel at kitware.com> wrote:
> >> > Isn't already enforced by HeaderTesting-* tests ?
> >> >
> >> > PS: I continue the discussion on vtk-developers only as this is the
> >> > canonic place to discuss about this topic.
> >> >
> >> > On Fri, Jun 5, 2009 at 4:42 PM, Dave Partyka<dave.partyka at kitware.com>
> >> > wrote:
> >> >> Hi everyone,
> >> >> A new CVS commit check has been added to prevent commits of VTK header
> >> >> files
> >> >> that contain STL includes. Exceptions can be added by contacting myself
> >> >> or
> >> >> Brad King.
> >> >> I also want to give a friendly reminder that the VTK coding standards
> >> >> (VTK
> >> >> has coding standards???) provide guidelines for using STL in VTK and
> >> >> require
> >> >> that STL not be used in header files. See the following for more
> >> >> information
> >> >> or feel free to ask me if you have any questions.
> >> >> VTK Coding Standards:
> >> >> http://www.vtk.org/Wiki/VTK_Coding_Standards
> >> >> VTK FAQ regarding STL
> >> >> http://www.vtk.org/Wiki/VTK_FAQ#Can_I_use_STL_with_VTK.3F
> >> >> Also please report any problems/bugs with the new commit check to
> >> >> myself or
> >> >> Brad King.
> >> >> Thanks very much!
> >> >
> >> >
> >> >
> >> > --
> >> > François Bertel, PhD  | Kitware Inc. Suite 204
> >> > 1 (518) 371 3971 x113 | 28 Corporate Drive
> >> >                      | Clifton Park NY 12065, USA
> >> > _______________________________________________
> >> > 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
> >> >
> >> >
> >
> >
> _______________________________________________
> 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
> 
>