Coding Standard Questions
Kevin Tew
I'll post patches to the list, but here are some things that are not
defined by the coding standards, but of which I'm a fan.
So the question is are they acceptable in the parrot code base.
1) *s should go right next to the type in function declarations and
definitions
/* incorrect */
do_thaw(Parrot_Interp interpreter, PMC * pmc, visit_info *info)
/* correct */
do_thaw(Parrot_Interp interpreter, PMC* pmc, visit_info* info)
2) All structs should be typedefed
3) Single line if and elses are acceptable
/*old*/
if (!info->thaw_result)
info->thaw_result = pmc;
else
*info->thaw_ptr = pmc;
/*new*/
if (!info->thaw_result) info->thaw_result = pmc;
else *info->thaw_ptr = pmc;
4) c89 allows declaration and initialization on the same line right?
INTVAL counter = 0;
Kevin
Mark J. Reed
> 1) *s should go right next to the type in function declarations and
> definitions
I disagree; they should go right next to the name being declared,
since C declarations are designed to reflect the way the declared item
is later used: PMC *pmc, visit_info *info.
> 2) All structs should be typedefed
That seems reasonable. What about struct names? Around here we
require that all structs have both a struct name and a typedef, and
the two names match:
typedef struct foo {...} foo;
It's redundant except when you need to declare a self-referential
pointer, of course, but consistency for consistency's sake isn't
always bad. :)
> 3) Single line if and elses are acceptable
I personally don't like that. I'd rather go tthe other way and
require curlies always.
> 4) c89 allows declaration and initialization on the same line right?
> INTVAL counter = 0;
Sure. Even pre-ANSI C allows that.
--
Mark J. Reed <mark...@mail.com>
Andy Lester
On Oct 17, 2006, at 3:33 PM, Kevin Tew wrote:
> if (!info->thaw_result) info->thaw_result = pmc;
> else *info->thaw_ptr = pmc;
No, definitely not.
if ( foo ) {
bar();
}
else {
bat();
}
--
Andy Lester => an...@petdance.com => www.petdance.com => AIM:petdance
Jerry Gay
>
> On Oct 17, 2006, at 3:33 PM, Kevin Tew wrote:
>
> > if (!info->thaw_result) info->thaw_result = pmc;
> > else *info->thaw_ptr = pmc;
>
> No, definitely not.
>
> if ( foo ) {
> bar();
> }
> else {
> bat();
> }
>
if (foo)
bar();
else
bat();
is specced in the code formatting section of pdd07. specifically, "Do
not routinely put single statements in statement blocks" and
"parentheses should not have space immediately after the opening
parenthesis nor immediately before the closing parenthesis" apply
here.
~jerry
Chromatic
> On Oct 17, 2006, at 3:33 PM, Kevin Tew wrote:
> > if (!info->thaw_result) info->thaw_result = pmc;
> > else *info->thaw_ptr = pmc;
>
> No, definitely not.
>
> if ( foo ) {
> bar();
> }
> else {
> bat();
> }
Chip already weighed in the other way.
-- c
Kevin Tew
single statements bodies of ifs
if ( foo )
bar();
else
bat();
I view lines as a valuable resource. I like to fit whole functions on
the screen when possible so I'm more of a fan of
if (foo) bar();
else bat();
Curlies are ok with me too, but I think a decision against curlies was made.
if (foo) { bar(); }
else { bat(); }
Kevin Tew
Jerry thanks for finding the reference in pdd07.
Note I'm not trying to start a preference war here, I would just like
Chip to rule on some things that are not in the coding spec yet.
Thanks,
Kevin
Prvious mail should have read:
Leopold Toetsch
> While exploring Parrot internals I started cleaning up some code.
> I'll post patches to the list, but here are some things that are not
> defined by the coding standards, but of which I'm a fan.
> So the question is are they acceptable in the parrot code base.
>
> 1) *s should go right next to the type in function declarations and
> definitions
>
> /* incorrect */
> do_thaw(Parrot_Interp interpreter, PMC * pmc, visit_info *info)
> /* correct */
> do_thaw(Parrot_Interp interpreter, PMC* pmc, visit_info* info)
Both look wrong to me, my POV is:
do_thaw(Interp *interpreter, PMC *pmc, visit_info *info)
a) Parrot_Interp was removed for some reason long time ago in almost all of
the code (it's usage ought to be in embed land). Please lookup p6i history.
b) Re PMC* a vs. PMC *a:
the former might make one think of PMC* is declaring some pointers:
PMC* a, b; # oops wrong b isn't a pointer at all
PMC *a, *b; # all fine
> 2) All structs should be typedefed
No. Commonly used structs might be typedefed, but that should never be
enforced.
> 3) Single line if and elses are acceptable
Nevva.
> 4) c89 allows declaration and initialization on the same line right?
> INTVAL counter = 0;
of course.
> Kevin
leo
Mr. Shawn H. Corey
> 1) *s should go right next to the type in function declarations and
> definitions
>
> /* incorrect */
> do_thaw(Parrot_Interp interpreter, PMC * pmc, visit_info *info)
> /* correct */
> do_thaw(Parrot_Interp interpreter, PMC* pmc, visit_info* info)
Disagree. Consider:
char* foo ()
{
char* str1, str2;
...
}
As compared to:
char *foo()
{
char *str1, *str2;
...
}
I seen this error many times. Because of the way C associates the * with
the variable, not the type, you should write the code that way.
>
> 2) All structs should be typedefed
>
> 3) Single line if and elses are acceptable
>
> /*old*/
> if (!info->thaw_result)
> info->thaw_result = pmc;
> else
> *info->thaw_ptr = pmc;
>
> /*new*/
> if (!info->thaw_result) info->thaw_result = pmc;
> else *info->thaw_ptr = pmc;
Disagree. Subsequences should always be blocked.
if( ! info->thaw_result ){
info->thaw_result = pmc;
}else{
info->thaw_result = pmc;
}
Reasons:
1. It clear what is associated with what.
2. It's closer to what Perl does.
3. Statements can be added eaiser.
>
>
> 4) c89 allows declaration and initialization on the same line right?
> INTVAL counter = 0;
>
> Kevin
>
--
__END__
Just my 0.00000002 million dollars worth,
Shawn
"For the things we have to learn before we can do them, we learn by
doing them."
Aristotle
"Where you find the greatest fear, you find the greatest courage."
BRIAN: You're all individuals!
CROWD: We're all individuals!
LONE VOICE IN THE BACK: I'm not!
Leopold Toetsch
> if ( foo )
> bar();
> else
> bat();
> if ( foo )
no spaces in the parens. No example in pdd07 is looking like this. There where
some ident rules in that pdd some time ago regarding that.
Above vs.:
if (foo) {
bar();
}
else {
bat();
}
Anything that has the smallest smell of ever needing an extra statement after
if or else shall use braces in the first place (IMHO).
if (ending)
return 1; /* ok for me */
if (foo) {
do_something(); /* might need something else too */
}
leo
Andy Lester
Coding standards discussions = much heat, little light.
I'm sorry I responded to anything in this thread in the first place.
Please.
Leopold Toetsch
> > the screen when possible so I'm more of a fan of
If you are out of lines:
$ perl -e'++$lines for 1..1000'
helps for some time, then pleae restart.
> > if (foo) bar();
> > else bat();
That's an unreadable mess.
EOT (this one)
leo
Chip Salzenberg
> While exploring Parrot internals I started cleaning up some code.
> I'll post patches to the list, but here are some things that are not
> defined by the coding standards, but of which I'm a fan.
> So the question is are they acceptable in the parrot code base.
Thanks for making these explicit proposals. Answers below.
> 1) *s should go right next to the type in function declarations and
> definitions
No, in fact the opposite will be added to the coding standards as a rule.
Sadly the "*" next to the type is a common error propagated recently by
Bjarne Stroustrup (he makes the same mistake with "&"). The grammars of C
and C++ are what make this a misleading mistake. Consider:
char* p, q; /* bad */
Are p and q of the same type? No. Is q a char*? No. In contrast,
consider this:
char *p, q; /* not misleading, at least */
Here we see clearly expressed that both *p and q are of type char.
> 2) All structs should be typedefed
I'm hoping eventually to make Parrot compilable with C++, if only to support
making the Interpreter a COM object. This alters my usual ideal for
structure declarations. So the structure rules are:
a. All structures must have tags ("struct foo").
b. All structures should have typedefs.
c. The typedef and the tag should be the same.
(Structures inside functions can break all these rules.)
Here's my ideal structure definition:
typedef struct Foo {
...
} Foo;
Or, if forward declaration is important, the typedef and the struct can be
separated, e.g.:
typedef struct foo foo;
...
struct foo {
...
};
> 3) Single line if and elses are acceptable
No, sorry. I do that in my own Perl code, but in C it's too cutesie.
> 4) c89 allows declaration and initialization on the same line right?
> INTVAL counter = 0;
That's entirely legal & OK. But beware, it can be overdone. If you're not
careful, can end up doing a lot of work in your initializations, work that
may be entirely pointless if the given function takes an early return for
some reason.
--
Chip Salzenberg <ch...@pobox.com>
Chip Salzenberg
> if ( foo ) {
> bar();
> }
> else {
> bat();
> }
Well, that's not correct either: Our coding standards already say to omit
needless braces, and don't space inside the parens of if/while/etc. Thus,
this is the preferred format:
if (foo)
bar();
else
baz();
--
Chip Salzenberg <ch...@pobox.com>
Andy Lester
On Nov 12, 2006, at 6:46 PM, Chip Salzenberg wrote:
> char *p, q; /* not misleading, at least */
>
> Here we see clearly expressed that both *p and q are of type char.
I think it IS misleading. I would do this as:
char *p;
char q;
As MJD says, it's better to look than to think.
Chip Salzenberg
> Anything that has the smallest smell of ever needing an extra statement after
> if or else shall use braces in the first place (IMHO).
Predicting the future is something humans are bad at, sadly.
--
Chip Salzenberg <ch...@pobox.com>
bjarne
Chip Salzenberg wrote:
> Sadly the "*" next to the type is a common error propagated recently
For about 25 years
> by
> Bjarne Stroustrup (he makes the same mistake with "&"). The grammars of C
> and C++ are what make this a misleading mistake.
It's a taste issue:
http://www.research.att.com/~bs/bs_faq2.html#whitespace
Calling it a "mistake" is taking sides in a religious war.
-- Bjarne Stroustrup; http://www.research.att.com/~bs