> -----Original Message-----
> From: Martin Sebor [mailto:[EMAIL PROTECTED]
> Sent: Tuesday, April 15, 2008 4:26 PM
> To: [email protected]
> 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.
