Re: More review of current stm branch code

2006-08-15 Thread Chip Salzenberg
On Fri, Aug 11, 2006 at 04:38:59PM -0400, Charles Reiss wrote:
 On 8/10/06, Chip Salzenberg [EMAIL PROTECTED] wrote:
  /* XXX is it okay to combine flatten/slurpy into one flag? */
 
The answer is No: flat is an output flag, slurpy_array is an input
flag, and there's no guarantee that the input and output flags won't
conflict with each other.  So I guess this means that something has to
change.
 
 I suppose trying to make '@' mean something different for signatures and
 for calls from C (as I have done) is a Bad Idea as long as the same code
 is used to parse the signatures in both cases.  The easy solution is to
 choose a character other than '@' for one of the directions though I can't
 think of what might be most natural ('F' for flat?).

To provide a general ability to wrap any call from C, then you need to add
named parameter support too; flat arrays are _so_ Perl 5.  Would '%' and 'N'
work for you?  :-)  (also see below)

 MORE QUESTIONS
  * The '@' character for native call signatures is new, and AFAICT is just
syntactic sugar, since the caller could do the array creation himself.
Could you explain what you would have to do if you didn't introduce this
feature?  (I'm not necessarily against it, mind you, I just want to know
what the deal is.)

 It is just syntactic sugar. So, not using would be as you describe, having
 the (most likely PIR) caller construct the array manually. My use of this
 feature is only to allow naturally passing an arbitrary number of
 arguments to the subroutine that is first executed in a new thread, which
 I feel is quite convenient.

Well, as it happens, we've been wanting support for variadic PMC METHODs for
a Long Time.  Since you're already introducing necessary infrastructure, are
you inclined to add some PMC METHOD syntactic sugar too, so that we can
conveniently define METHODs that accept arbitrary parameter lists?  (This
is not a merge blocker, of course; just a wishlist item.)

  * Another comment asks:
 
 # autogenerate for exotic types
 # (XXX is this appropriate or do we want them to each
 # be explicitly cleared to have the variant?)
 
Well, that depends.  Is there currently any way for a named METHOD to
specify whether it is :write, and if so, is this used?  If so, then yes,
making an automatic ro variant is OK.  If not, then I think we might 
want such a thing...?
 
 The read-only variant generation currently does not handle NCI methods
 at all.  There are number of implementation options; the best I can
 think of is to override findmethod (in the read-only type) to check
 for a property on the found method PMC that would indicates it writes
 (or vice-versa).

That's actually kind of neat; at least, it's clearly the least bad of all
the options I see.

The only minor downside IMO is that in order to get adequate speed we'll
need to dedicate an easy-to-see flag bit somewhere visible in the method PMC
to mean :write, and there are only so many of those.  But fast threading
is hardly a minor feature, so I don't mind that cost at all.

 A bigger issue for automatic read-only variant generation is that MMD
 methods currently don't do any read-onlyness detection. (Sorry!) (This is
 not quite as much as a problem as it may seem because things like String
 and Integer, being designed to allow subclassing, call vtable methods from
 their MMD methods to do any manipulation.)

Understood, agreed.

 As a stopgap solution, it would be easy to reverse the logic I have
 now and default to not generating a read-only version except when the
 .pmc file says it is okay instead of the other way around.

Well, this is still 0.4, not 1.0.  If you can just tell us on the list what
we shouldn't do, so we can include it in the release notes, then I don't
mind having only partial protection.  For obvious reasons, nobody's using
the thread support right now, so it won't break anything.  ...Or will it?
-- 
Chip Salzenberg [EMAIL PROTECTED]


Re: More review of current stm branch code

2006-08-15 Thread Chip Salzenberg
On Sat, Aug 12, 2006 at 08:14:52PM -0400, Charles Reiss wrote:
 I wrote:
 The read-only variant generation currently does not handle NCI methods
 at all. There are number of implementation options; the best I can
 think of is to override findmethod (in the read-only type) to check
 for a property on the found method PMC that would indicates it writes
 (or vice-versa).
 
 This is done (w/ s/findmethod/find_method/ of course), returning PMCNULL
 instead of a method that would write. (This prevents it from being called,
 but doesn't yield helpful errors.)

Yow, done already!  Excellent.  (Shoulda read the followup first.)

 It also does not allow .pmc files to overide the default idea of
 whether a vtable method is read-only.
 
 This remains unresolved though I'm not certain it should be allowed.
 The hard part of doing it correctly is handling inheritance.

It's an interface guarantee, and as such I think it's OK that it can't be
overridden in a derived class.  Agree?

 A bigger issue for automatic read-only variant generation is that  MMD
 methods currently don't do any read-onlyness detection. (Sorry!)
 
 This is now fixed.

Yow^2!  Nice work.
-- 
Chip Salzenberg [EMAIL PROTECTED]


Re: More review of current stm branch code

2006-08-15 Thread Charles Reiss

On 8/15/06, Chip Salzenberg [EMAIL PROTECTED] wrote:

On Sat, Aug 12, 2006 at 08:14:52PM -0400, Charles Reiss wrote:
 I wrote:

[snip]

 It also does not allow .pmc files to overide the default idea of
 whether a vtable method is read-only.

 This remains unresolved though I'm not certain it should be allowed.
 The hard part of doing it correctly is handling inheritance.

It's an interface guarantee, and as such I think it's OK that it can't be
overridden in a derived class.  Agree?


Er, when I first read that I thought you meant that with respect to
the inheritance issue, but reading again I'm not sure. But, anyways,
looking over pmc2c I found that doing it 'properly' was not too
difficult, so I've done that (read/writeness is inherited by default
and can be overridden). I provide the feature in the hope that people
will think really hard before using it to violate interface
guarantees.

-- Charles Reiss


stm ready (was Re: More review of current stm branch code)

2006-08-15 Thread Chip Salzenberg
So, given the below, looks like we've got everything sewn up and the
long-awaited day of the STM merge is upon us.

Charles, care to do the honors?

On Tue, Aug 15, 2006 at 06:31:38PM -0400, Charles Reiss wrote:
 On 8/15/06, Chip Salzenberg [EMAIL PROTECTED] wrote:
 On Sat, Aug 12, 2006 at 08:14:52PM -0400, Charles Reiss wrote:
  I wrote:
  It also does not allow .pmc files to overide the default idea of
  whether a vtable method is read-only.
 
  This remains unresolved though I'm not certain it should be allowed.
  The hard part of doing it correctly is handling inheritance.
 
 It's an interface guarantee, and as such I think it's OK that it can't be
 overridden in a derived class.  Agree?
 
 Er, when I first read that I thought you meant that with respect to
 the inheritance issue, but reading again I'm not sure.

Yes, that's what I meant.

 But, anyways, looking over pmc2c I found that doing it 'properly' was not
 too difficult, so I've done that (read/writeness is inherited by default
 and can be overridden).

Works For Me, thanks.

 I provide the feature in the hope that people will think really hard
 before using it to violate interface guarantees.

Big friendly warnings are the right first step.  If people start ignoring
the signs and wandering onto the live minefields, we'll have to start
thinking about audio warnings and fierce guards with pointed sticks.
-- 
Chip Salzenberg [EMAIL PROTECTED]


Re: More review of current stm branch code

2006-08-12 Thread Charles Reiss

I wrote:
[snip]

I suppose trying to make '@' mean something different for signatures and for
calls from C (as I have done) is a Bad Idea as long as the same code is used
to parse the signatures in both cases. The easy solution is to choose
a character
other than '@' for one of the directions though I can't think of what
might be most
natural ('F' for flat?).


I've separated it into 'F' (flatten for call from C) and '@' for now.

[snip]


The read-only variant generation currently does not handle NCI methods
at all. There are number of implementation options; the best I can
think of is to override findmethod (in the read-only type) to check
for a property on the found method PMC that would indicates it writes
(or vice-versa).


This is done (w/ s/findmethod/find_method/ of course), returning PMCNULL
instead of a method that would write. (This prevents it from being called,
but doesn't yield helpful errors.)


It also does not allow .pmc files to overide the default idea of
whether a vtable method is read-only.


This remains unresolved though I'm not certain it should be allowed.
The hard part of doing it correctly is handling inheritance.


A bigger issue for automatic read-only variant generation is that  MMD
methods currently don't do any read-onlyness detection. (Sorry!)


This is now fixed.

[snip]

  -- Charles Reiss


Re: Review of current stm branch code

2006-08-11 Thread Sam Phillips

/LURK

On 11 Aug 2006, at 06:11, Joshua Hoblitt wrote:


This is a bad joke, right?  How much of your life are you intending to
spend on chasing down hard to find missing braces bugs?


On 11 Aug 2006, at 06:52, Chip Salzenberg wrote:
Seriously: I am serious.  Many of the changes I have in mind for  
the Parrot
source code are based on reducing the visual footprint of  
constructs without
changing their meaning, and eliminating needless words^Wbraces is  
one way

to do that.


On 11 Aug 2006, at 06:28, Uri Guttman wrote:

braces RULES!!


Six years into the project the Parrot team, responsible for the  
Perl6 internals finaly get round to arguing about what style of C  
brackets and indenting they are going to use..


I can just see how that will run on Slashdot..

:sheesh!:

;-)
Sam Phillips
[EMAIL PROTECTED]
LURK


Re: Review of current stm branch code

2006-08-11 Thread Chip Salzenberg
On Fri, Aug 11, 2006 at 11:46:37AM +0100, Sam Phillips wrote:
 Six years into the project the Parrot team, responsible for the  
 Perl6 internals finaly get round to arguing about what style of C  
 brackets and indenting they are going to use..

It's not an argument.[*]  If people kept talking about it, and if I kept
trying to persuade them, back  forth...  then, it'd be an argument.

[*] It's abuse.  Arguments are down the hall.
-- 
Chip Salzenberg [EMAIL PROTECTED]


Re: More review of current stm branch code

2006-08-11 Thread Charles Reiss

On 8/10/06, Chip Salzenberg [EMAIL PROTECTED] wrote:

More on the STM branch:


ANSWERS, FOR A CHANGE

 * A comment asks:

 /* XXX is it okay to combine flatten/slurpy into one flag? */

   The answer is No: flat is an output flag, slurpy_array is an input
   flag, and there's no guarantee that the input and output flags won't
   conflict with each other.  So I guess this means that something has to
   change.



I suppose trying to make '@' mean something different for signatures and for
calls from C (as I have done) is a Bad Idea as long as the same code is used
to parse the signatures in both cases. The easy solution is to choose
a character
other than '@' for one of the directions though I can't think of what
might be most
natural ('F' for flat?).

[one note moved later in this email]


MORE QUESTIONS

 * The '@' character for native call signatures is new, and AFAICT is just
   syntactic sugar, since the caller could do the array creation himself.
   Could you explain what you would have to do if you didn't introduce this
   feature?  (I'm not necessarily against it, mind you, I just want to know
   what the deal is.)


It is just syntactic sugar. So, not using would be as you describe, having the
(most likely PIR) caller construct the array manually. My use of this feature is
only to allow naturally passing an arbitrary number of arguments to
the subroutine that
is first executed in a new thread, which I feel is quite convenient.
It is, however,
not terribly important (and perhaps I shouldn't have spent time
implementing it), and
if deemed undeseriable, I would not suffer much penalty in removing it.



 * Another comment asks:

# autogenerate for exotic types
# (XXX is this appropriate or do we want them to each
# be explicitly cleared to have the variant?)

   Well, that depends.  Is there currently any way for a named METHOD to
   specify whether it is :write, and if so, is this used?  If so, then yes,
   making an automatic ro variant is OK.  If not, then I think we might want
   such a thing...?



The read-only variant generation currently does not handle NCI methods
at all. There are number of implementation options; the best I can
think of is to override findmethod (in the read-only type) to check
for a property on the found method PMC that would indicates it writes
(or vice-versa).

It also does not allow .pmc files to overide the default idea of
whether a vtable method is read-only.

A bigger issue for automatic read-only variant generation is that  MMD
methods currently don't do any read-onlyness detection. (Sorry!) (This
is not quite as much as a problem as it may seem because things like
String and Integer, being designed to allow subclassing, call vtable
methods from their MMD methods to do any manipulation.)

As a stopgap solution, it would be easy to reverse the logic I have
now and default to not generating a read-only version except when the
.pmc file says it is okay instead of the other way around.



ANOTHER NAMING THING

 * Please rename 'ro_variant' to something ending in '_vtable',
   e.g. 'ro_variant_vtable', to make clear that it's not a class pointer
   or type number.



Done (using suggested name).

   -- Charles Reiss


Re: Review of current stm branch code

2006-08-11 Thread Charles Reiss

On 8/10/06, Chip Salzenberg [EMAIL PROTECTED] wrote:

I appreciate the quality of the stm code in general.  You're being careful,
you know what you're doing with C, and you're good at creating abstractions.
I hope when STM is done[*] you'll keep hacking on Parrot.

[*] As if it will ever be really done.
No work of art is ever finished, only abandoned.

Here are my comments.  Once these issues and/or questions are addressed
and/or diposed of, we can merge the state-of-stm onto the trunk.  Please
correct me wherever I've misunderstood what you're doing.


POSSIBLE BUGS

 * START_WRITE_HLL params vs. usage

+#define START_WRITE_HLL(interpreter, hll_info) \
+do { \
+if (PMC_sync(interpreter-HLL_info)) { \
+hll_info = interpreter-HLL_info = \
+Parrot_clone(interpreter, interpreter-HLL_info); \
+if (PMC_sync(interpreter-HLL_info)) { \
+mem_internal_free(PMC_sync(interpreter-HLL_info)); \
+} \
+} \
+} while (0)

   But then:

+START_WRITE_HLL(interpreter, interpreter-HLL_info)

   That expansion, which is in part:

  interpreter-HLL_info = interpreter-HLL_info = Parrot_clone(interpreter, 
interpreter-HLL_info);
  mem_internal_free(PMC_sync(interpreter-HLL_info));

   Looks odd or even broken, offhand.  Am I missing something?


I think it was just odd but violated what I had intended for that
macro. I've changed the use of the macro.


 * enum trailing commas are not standard

   C89 doesn't allow enum lists to end with a comma.  PITA, I know, but we
   can't require C99 yet.  So e.g. 'thread_state_enum' needs a comma removed.


Fixed.



QUESTIONS/REQUESTS

 * You're defining a heck of a lot of macros.  Is there any set of them that
   can be marked as private (to the STM implementation), e.g. with a leading
   single underscore?  Say, if the ATOMIC_INT_CAS macro is only used by other
   macros and isn't part of the supported interface, then it could be renamed
   to _ATOMIC_INT_CAS.  This will prevent the abstraction from leaking.

 * It's surely a mean trick on the poor user/programmer to have
   Parrot_atomic_int actually hold a long.  :-, Any reason not to use int?
   Or do you want Parrot_atomic_long?


I changed the name to Parrot_atomic_integer; I don't want to imply
that it's going to be the
same width as any particular built-in type or any particular
Parrot-supplied type. I haven't
changed ATOMIC_INT_* - ATOMIC_INTEGER_*, however, as I feel those
macro names are long enough (especially with PARROT_ added, see below)
and as I abbreviated 'pointer' too. (If you disagree, I'll change
those, too.)


 * Is there any need/use for multiple integral atomic types, like an
   unsigned or a UINTVAL or something?  (I expect the answer is no.)


No.


 * Could *_{READ,WRITE}_HLL be named something like *_{READ,WRITE}_HLL_INFO,
   assuming they need to be macros at all?  An HLL is an abstraction that
   currently doesn't have a single concrete representation.


Done.


 * I see that Parrot_get_HLL_namespace() is now deferring namespace creation.
   That could perhaps be problematic.  We've told users that
  get_root_namespace ['your_own_hll'; 'x']
   and
  get_hll_namespace ['x']
   have identical results.  So, what's the payoff for this delayed creation?


Mainly my coding convenience. I've undelayed the creation.


 * Does your ops.num renumber ops, or is diff just getting confused by 
whitespace?
   If the former, what's up?


I apparently renumbered ops by mistake (I guess I shouldn't have
assumed that dev/tools/ops_renum.mak would DTRT). Fixed by manually
adding my new ops to the end of the file from  trunk.



CODING STANDARD^WWHIM ISSUES

I do need to update pdd07.  In the meantime, here are some coding standard
issues I'd appreciate if you'd address.  For some of them you're going to be
setting the new standard for the rest of the code, but such is the fate of
code reviewed the guy who's writing the new spec.  :-,


 * Out of all the macros you're defining, will any of them be ever needed by
   users of the Parrot extension or embedding interfaces?  If so, their full
   official names will have to start with PARROT_.  But when compiling
   Parrot, it's OK to define non-PARROT_-prefix versions as aliases.


Full names changed along with all usages (instead of defining aliases).

[snip]


 * macro args  protection

   Most of your code is really good  careful with macro behaviors.
   Just a couple exceptions:

+#define START_WRITE_HLL(interpreter, hll_info) \
+do { \
+if (PMC_sync(interpreter-HLL_info)) { \
+hll_info = interpreter-HLL_info = \
+Parrot_clone(interpreter, interpreter-HLL_info); \
+if (PMC_sync(interpreter-HLL_info)) { \
+mem_internal_free(PMC_sync(interpreter-HLL_info)); \
+} \
+} \
+} while (0)

   Macro 

Review of current stm branch code

2006-08-10 Thread Chip Salzenberg
I appreciate the quality of the stm code in general.  You're being careful,
you know what you're doing with C, and you're good at creating abstractions.
I hope when STM is done[*] you'll keep hacking on Parrot.

[*] As if it will ever be really done.
No work of art is ever finished, only abandoned.

Here are my comments.  Once these issues and/or questions are addressed
and/or diposed of, we can merge the state-of-stm onto the trunk.  Please
correct me wherever I've misunderstood what you're doing.


POSSIBLE BUGS

 * START_WRITE_HLL params vs. usage

+#define START_WRITE_HLL(interpreter, hll_info) \
+do { \
+if (PMC_sync(interpreter-HLL_info)) { \
+hll_info = interpreter-HLL_info = \
+Parrot_clone(interpreter, interpreter-HLL_info); \
+if (PMC_sync(interpreter-HLL_info)) { \
+mem_internal_free(PMC_sync(interpreter-HLL_info)); \
+} \
+} \
+} while (0)

   But then:

+START_WRITE_HLL(interpreter, interpreter-HLL_info)

   That expansion, which is in part:

  interpreter-HLL_info = interpreter-HLL_info = Parrot_clone(interpreter, 
interpreter-HLL_info);
  mem_internal_free(PMC_sync(interpreter-HLL_info));

   Looks odd or even broken, offhand.  Am I missing something?

 * enum trailing commas are not standard

   C89 doesn't allow enum lists to end with a comma.  PITA, I know, but we
   can't require C99 yet.  So e.g. 'thread_state_enum' needs a comma removed.


QUESTIONS/REQUESTS

 * You're defining a heck of a lot of macros.  Is there any set of them that
   can be marked as private (to the STM implementation), e.g. with a leading
   single underscore?  Say, if the ATOMIC_INT_CAS macro is only used by other
   macros and isn't part of the supported interface, then it could be renamed
   to _ATOMIC_INT_CAS.  This will prevent the abstraction from leaking.

 * It's surely a mean trick on the poor user/programmer to have
   Parrot_atomic_int actually hold a long.  :-, Any reason not to use int?
   Or do you want Parrot_atomic_long?

 * Is there any need/use for multiple integral atomic types, like an
   unsigned or a UINTVAL or something?  (I expect the answer is no.)

 * Could *_{READ,WRITE}_HLL be named something like *_{READ,WRITE}_HLL_INFO,
   assuming they need to be macros at all?  An HLL is an abstraction that
   currently doesn't have a single concrete representation.

 * I see that Parrot_get_HLL_namespace() is now deferring namespace creation.
   That could perhaps be problematic.  We've told users that
  get_root_namespace ['your_own_hll'; 'x']
   and
  get_hll_namespace ['x']
   have identical results.  So, what's the payoff for this delayed creation?

 * Does your ops.num renumber ops, or is diff just getting confused by 
whitespace?
   If the former, what's up?


CODING STANDARD^WWHIM ISSUES

I do need to update pdd07.  In the meantime, here are some coding standard
issues I'd appreciate if you'd address.  For some of them you're going to be
setting the new standard for the rest of the code, but such is the fate of
code reviewed the guy who's writing the new spec.  :-,


 * Out of all the macros you're defining, will any of them be ever needed by
   users of the Parrot extension or embedding interfaces?  If so, their full
   official names will have to start with PARROT_.  But when compiling
   Parrot, it's OK to define non-PARROT_-prefix versions as aliases.

   For example, if the Parrot_atomic_int is a public type, exposed to
   embed/extend, as its name implies, then the ATOMIC_* macros will have to
   be PARROT_ATOMIC_*.  But then this is OK too:

  #ifdef PARROT_IN_CORE
  #  define ATOMIC_FOO PARROT_ATOMIC_FOO
  #  define ATOMIC_BAR PARROT_ATOMIC_BAR
  ...
  #endif


 * macro args  protection

   Most of your code is really good  careful with macro behaviors.
   Just a couple exceptions:

+#define START_WRITE_HLL(interpreter, hll_info) \
+do { \
+if (PMC_sync(interpreter-HLL_info)) { \
+hll_info = interpreter-HLL_info = \
+Parrot_clone(interpreter, interpreter-HLL_info); \
+if (PMC_sync(interpreter-HLL_info)) { \
+mem_internal_free(PMC_sync(interpreter-HLL_info)); \
+} \
+} \
+} while (0)

   Macro arguments should be parenthesized when used, and it helps to name
   them in all caps so as to distinguish them from the normal names ref'd in
   the macro body.

   Similarly, when you're not using do {} while (0), the macro expansion
   should be parenthesized if it contains an exposed operator.  For example,
   this:

+#   define ATOMIC_INT_INC(result, a) result = ++(a).val

   should be paren'd:

+#   define ATOMIC_INT_INC(result, a) ((result) = ++(a).val)


 * useless curlies

-if (PMC_IS_NULL(type_hash))
+if (PMC_IS_NULL(type_hash)) {
 return 

More review of current stm branch code

2006-08-10 Thread Chip Salzenberg
More on the STM branch:


ANSWERS, FOR A CHANGE

 * A comment asks:

 /* XXX is it okay to combine flatten/slurpy into one flag? */

   The answer is No: flat is an output flag, slurpy_array is an input
   flag, and there's no guarantee that the input and output flags won't
   conflict with each other.  So I guess this means that something has to
   change.


 * Another comment asks:

# autogenerate for exotic types
# (XXX is this appropriate or do we want them to each
# be explicitly cleared to have the variant?)

   Well, that depends.  Is there currently any way for a named METHOD to
   specify whether it is :write, and if so, is this used?  If so, then yes,
   making an automatic ro variant is OK.  If not, then I think we might want
   such a thing...?


MORE QUESTIONS

 * The '@' character for native call signatures is new, and AFAICT is just
   syntactic sugar, since the caller could do the array creation himself.
   Could you explain what you would have to do if you didn't introduce this
   feature?  (I'm not necessarily against it, mind you, I just want to know
   what the deal is.)


ANOTHER NAMING THING

 * Please rename 'ro_variant' to something ending in '_vtable',
   e.g. 'ro_variant_vtable', to make clear that it's not a class pointer
   or type number.


-- 
Chip Salzenberg [EMAIL PROTECTED]


Re: Review of current stm branch code

2006-08-10 Thread Joshua Hoblitt
On Thu, Aug 10, 2006 at 07:19:21PM -0700, Chip Salzenberg wrote:
  * enum trailing commas are not standard
 
C89 doesn't allow enum lists to end with a comma.  PITA, I know, but we
can't require C99 yet.  So e.g. 'thread_state_enum' needs a comma removed.

Nor does C++ understand the trailing commas and I've run into this issue
when trying to link C++ to C with C99 headers.

-J

--


pgpzfK9fw7gSx.pgp
Description: PGP signature


Re: Review of current stm branch code

2006-08-10 Thread Joshua Hoblitt
On Thu, Aug 10, 2006 at 07:19:21PM -0700, Chip Salzenberg wrote:
  * useless curlies
 
 -if (PMC_IS_NULL(type_hash))
 +if (PMC_IS_NULL(type_hash)) {
  return core_type;
 +}
  hash = PMC_struct_val(type_hash);
 -if (!hash-entries)
 +if (!hash-entries) {
  return core_type;
 +}
 
Something I'm hoping to stamp out is the use of curlies for all if/else
clauses, which makes code taller without making it substantially clearer.
I'd appreciate if you'd use a no-curlies-when-possible style.
 
OTOH, if you don't want to do this right away, I'd be OK with a merge
first, and fixing the curlies later.
 
OTGH, the project needs automated filters for more coding standards,
including one that that notes (and optionally kills) the excess curlies.

This is a bad joke, right?  How much of your life are you intending to
spend on chasing down hard to find missing braces bugs?

-J

--


pgpN3uSF9vUYs.pgp
Description: PGP signature


Re: Review of current stm branch code

2006-08-10 Thread Uri Guttman
 JH == Joshua Hoblitt [EMAIL PROTECTED] writes:

  JH On Thu, Aug 10, 2006 at 07:19:21PM -0700, Chip Salzenberg wrote:
   * useless curlies
   
   Something I'm hoping to stamp out is the use of curlies for all if/else
   clauses, which makes code taller without making it substantially clearer.
   I'd appreciate if you'd use a no-curlies-when-possible style.
   
   OTOH, if you don't want to do this right away, I'd be OK with a merge
   first, and fixing the curlies later.
   
   OTGH, the project needs automated filters for more coding standards,
   including one that that notes (and optionally kills) the excess curlies.

  JH This is a bad joke, right?  How much of your life are you intending to
  JH spend on chasing down hard to find missing braces bugs?

heh, this brings me back to my coding standards issues from when i did
tons of c. i ALWAYS used braces so you could visually see the
conditional or block of code. i was so glad when i switched to perl that
braces were manditory. there are so many bug issues with dangling
if/else clauses and unless you used pythonicly strict indenting, you
were going to have them unless you used braces. the extra lines used are
easily offset by better windowing editors, block hiding, and other
things and also the reduction in bugs makes that minor sacrifice well
worth it. but since i don't hack on parrot code i will just leave it at
that. braces RULES!!

back to lurking,

uri

-- 
Uri Guttman  --  [EMAIL PROTECTED]   http://www.stemsystems.com
--Perl Consulting, Stem Development, Systems Architecture, Design and Coding-
Search or Offer Perl Jobs    http://jobs.perl.org


Re: Review of current stm branch code

2006-08-10 Thread Chip Salzenberg
On Thu, Aug 10, 2006 at 07:11:15PM -1000, Joshua Hoblitt wrote:
 On Thu, Aug 10, 2006 at 07:19:21PM -0700, Chip Salzenberg wrote:
   * useless curlies
 OTGH, the project needs automated filters for more coding standards,
 including one that that notes (and optionally kills) the excess curlies.
 
 This is a bad joke, right?  How much of your life are you intending to
 spend on chasing down hard to find missing braces bugs?

If avoiding the possibility of hard-to-find bugs, were my priority, I
wouldn't enjoy writing C and C++.  Or Perl.  Or PL/I.

Seriously: I am serious.  Many of the changes I have in mind for the Parrot
source code are based on reducing the visual footprint of constructs without
changing their meaning, and eliminating needless words^Wbraces is one way
to do that.

Anyone who needs braces to avoid/detect errors is probably just not
acclimated to the C language, and/or failing to indent properly (that's what
smart editors are for; no snide Python remarks please).  And Parrot is
written in the complete C language, not some intersection of C and Perl.

I'd rather invest my extra vertical space into some well-chosen comments and
blank lines between phrases, which can convey useful meaning, rather than
braces around one-line if/else clauses, which are just noise.
-- 
Chip Salzenberg [EMAIL PROTECTED]