stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eric Lemings" <Eric.Lemi...@roguewave.com>
Subject RE: New 27.basic.ios.cpp test migrated (LONG)
Date Tue, 15 Apr 2008 23:06:03 GMT
 

> -----Original Message-----
> From: Martin Sebor [mailto:sebor@roguewave.com] 
> Sent: Tuesday, April 15, 2008 4:26 PM
> To: dev@stdcxx.apache.org
> Subject: Re: New 27.basic.ios.cpp test migrated (LONG)
> 
...
> About the test, it looks good to me. It needs only a few minor
> tweaks :)
> 
> Regarding the convention for preprocessor directives, it is
> 2 spaces and if there are places where it's inconsistent they
> should be fixed :)

Is there a reason its 2 spaces instead of 4?  If not, it should
be fixed everywhere i.e. made consistent in all contexts. :)

> Also, most test driver headers are squeaky clean WRT namespace
> pollution so they could be #included at the top of all tests,
> even those that require that no unnecessary library headers
> be #included first and so there's no need to be #including
> them in the middle like we had to do with some of the older
> tests.

Noted.

> Nice touch to add the command line option! WRT names of
> functions and variables defined in each test, there should be
> no rw_ prefix to make them easily distinguishable them names
> defined by the test driver. The test driver uses the rw_
> prefix for extern names and _rw_ (with a leading underscore)
> for names with internal linkage (static). So the name of the
> new option variable should be just opt_no_basic_ios_ctors.

Hmm.  I noticed variable names for command-line options in other
tests using the rw_ prefix so I used it also.  Funny how undesired
conventions get propagated, huh?

I corrected it though.

> One other comment about formatting: there should never be
> any code after a closing curly brace. I.e., we want this
> 
>    278     if (rw_opt_no_basic_ios_ctors) {
>    279         rw_note (0, 0, 0, "basic_ios<T> ctors disabled");
>    280     } else {
> 
> to look like this:
> 
>    278     if (rw_opt_no_basic_ios_ctors) {
>    279         rw_note (0, 0, 0, "basic_ios<T> ctors disabled");
>    280     }
>    281     else {

Good eye.  Is there a rationale for this or is just preference?
For me, this is just a holdover from K&R C.

Also, I just noticed the link to the style document on the home
page is broken.  Is it stored in Subversion or some other place
maybe?  Probably should give it a quick glance...

> Finally, every function is extern by default. There is no need
> to explicitly declare it as such (I believe there are compilers
> that warn about function definitions with the extern keyword).

Yep, and return types of functions default(ed?) to `int' type when
omitted though I'm not sure that still holds but they're still
always explictly specified nonetheless.  Same premise: never
rely on the default.

Anyways, another holdover from Whitesmiths style.

Brad.

Mime
View raw message