RE: Tentative patch to catch a[n]=b[n++]...

RE: Tentative patch to catch a[n]=b[n++]...

[Kaz Kylheku]

On Wednesday, March 25, 1998 1:04 PM, Jim Wilson [SMTP:wilson@cygnus.com] 
wrote:
> 	I have written a patch for gcc-2.8.1 / egcs-1.0.2 that flags
> 	warnings on the more simple cases of modifying a variable between sequence
> 	points.
>
> It would be interesting to have such a warning option in gcc.

Interesting, but probably not useful. Only boneheaded programmers write the
simple cases; and the not-so-simple cases are difficult to detect.
I would find it a waste of compile time for my own code, and annoying
if it warns on correct code.

Perhaps a run-time diagnosis based on the bounds checking code would
be more fruitful, in that it would actually detect when a modification has
happened to the same object more than once in arbitrarily complex
cases.

Re: Tentative patch to catch a[n]=b[n++]...

[Jeffrey A Law]

  In message < 01BD58BB.84FD66D0.kaz@cafe.net >you write:
  > On Wednesday, March 25, 1998 1:04 PM, Jim Wilson [SMTP:wilson@cygnus.com] 
  > wrote:
  > > 	I have written a patch for gcc-2.8.1 / egcs-1.0.2 that flags
  > > 	warnings on the more simple cases of modifying a variable between seque
  > nce
  > > 	points.
  > >
  > > It would be interesting to have such a warning option in gcc.
  > 
  > Interesting, but probably not useful. Only boneheaded programmers write the
  > simple cases; and the not-so-simple cases are difficult to detect.
  > I would find it a waste of compile time for my own code, and annoying
  > if it warns on correct code.
It doesn't have to be on by default.  And I would find it useful.

It's not uncommon for me to take suspicious bug reports and run
them through a "warn about everything" compile to see if any source
issues need to be looked at *before* spending the time analyzing
the generated code to look for a potential bug.

It's worth noting that several warning options will give false
positive warnings.  The question at had is how often do we get
false positives :-)

jeff

Re: Tentative patch to catch a[n]=b[n++]...

[Joe Buck]

>> > 	I have written a patch for gcc-2.8.1 / egcs-1.0.2 that flags
>> > 	warnings on the more simple cases of modifying a variable between 
>> >     sequence points.

>> Interesting, but probably not useful. Only boneheaded programmers write the
>> simple cases; and the not-so-simple cases are difficult to detect.
>> I would find it a waste of compile time for my own code, and annoying
>> if it warns on correct code.

> It doesn't have to be on by default.  And I would find it useful.

If the warning does not produce false positives (or only does in rare
cases), it may still be an advantage because it will decrease the number
of bug reports received.  So you might want to turn it on by default, as
it would be likely to save Cygnus money, as your support customers will
fix their own boneheaded problems!

Re: Tentative patch to catch a[n]=b[n++]...

[Jeffrey A Law]

  In message <Pine.LNX.3.95.980425220411.13755A-100000@atlas.imaginator.com>you write:
  > 	These cases are just hacked from my regression test code, does
  > this look like something that is sufficiently useful to include ?
I believe so.

  > What
  > worries me is wrongly flagging correct cases. There are always cases that
  > are not possible to trap eg. 
Many of gcc's warnings give false positives.  Some warnings are for
problems that are common and harmless in most sources.

The biggest downside to lots of false positives is that the warning
probably won't be all that useful to include in -Wall and thus won't
get used as often as it might if it was implied by -Wall.

  > 	As I haven't grasped the process of Changelog entries / posting
  > patches etc. can someone help ?
First, have you reviewed the egcs web page on contributing to the
project?

http://egcs.cygnus.com/contribute.html

If you've got specific questions after reading that page and some
of the pages it links to let us know.

jeff

RE: Tentative patch to catch a[n]=b[n++]...

[Kaz Kylheku]

On Saturday, April 25, 1998 2:10 PM, Michael Meeks 
[SMTP:michael@imaginator.com] wrote:
>
> Hi,
>
> 	I have written a patch for gcc-2.8.1 / egcs-1.0.2 that flags
> warnings on the more simple cases of modifying a variable between sequence
> points.

However, the example given in your Subject: line isn't a case of modifying
a variable more than once between sequence points.

It is a violation of the second law: the value of a modified object shall be 
accessed
only for the purpose of computing the new value.

Suppose that a[] and b[] do not overlap. What is wrong with a[n] = b[n++]?
Clearly, no object is modified more than once. But the prior value of 'n' is
used in a subexpression which is not subordinate to the computation
of the new value, namely the subexpression which supplies the offset
into a[]. This use of n does not demonstrably precede its modification,
so it is ambiguous whether the prior value or the new value is used---or
even whether indeed some half-baked value is used. The behavior is
undefined.

I hate to ask this, but are you sure you know what you are doing? :) :)

Tentative patch to catch a[n]=b[n++]...

[Michael Meeks]

Hi,

	I have written a patch for gcc-2.8.1 / egcs-1.0.2 that flags
warnings on the more simple cases of modifying a variable between sequence
points.

	Since the code doesn't locate sequence points in a formal manner,
it will warn on some code that is correct but unadvisable :-) Here is a
list of results on my test code:

  a = ++a + b ; /* Fail */
  a = a-- + b ; /* Fail */
  a = (a++ && 4) ; /* shouldn't Fail but does */

  a[n]=b[n++] ; /* Fail */
  a[--n]=b[n] ; /* Fail */
  a[++n]=b[--n] ; /* Fail */
  c[n][n]=c[n][n]++ ; /* should fail but doesn't */
  c[n][p]=c[n][n++] ; /* Fail */ /* if lvalue [n][n] then fails twice */

  *ptr++ = (int)ptr++ ; /* Fail */
  ptr->a = ptr->a++ ; /* should fail but doesn't */
  ptr->a = (int)(ptr++) ; /* Fail */

a = b = a++ ;   /* Fail */
b = a = --b ;   /* Fail */
a = 1 + (a=1) ; /* Fail */
a = (a=b) ;     /* Fail */
a = (a=b) + 1 ; /* Fail */

	These cases are just hacked from my regression test code, does
this look like something that is sufficiently useful to include ? What
worries me is wrongly flagging correct cases. There are always cases that
are not possible to trap eg. 

int a ;
int *b = &a, *c = &a ;
*b = *c++ ;

	etc.

	As I haven't grasped the process of Changelog entries / posting
patches etc. can someone help ?

	Thanks for your time,

		Michael Meeks.

-- 
 michael@imaginator.com  <><, Pseudo Engineer, itinerant idiot

Re: Tentative patch to catch a[n]=b[n++]...

[Jim Wilson]

	I have written a patch for gcc-2.8.1 / egcs-1.0.2 that flags
	warnings on the more simple cases of modifying a variable between sequence
	points.

It would be interesting to have such a warning option in gcc.

Since this patch may warn for some code that is OK (if I understood you
corectly), it should be a separate -W option that is not enabled by default by
any other -W options.

Presumably this is a non-trivial change, which means we will need a copyright
assignment and/or disclaimer (depending on the size) before we can accept it.

Jim

Tentative patch to catch a[n]=b[n++]...

[Michael Meeks]

Hi,

	I have written a patch for gcc-2.8.1 / egcs-1.0.2 that flags
warnings on the more simple cases of modifying a variable between sequence
points.

	Since the code doesn't locate sequence points in a formal manner,
it will warn on some code that is correct but unadvisable :-) eg.

  a = ++a + b ; /* Fail */
  a = a-- + b ; /* Fail */
  a = (a++ && 4) ; /* shouldn't Fail but does */

  a[n]=b[n++] ; /* Fail */
  a[--n]=b[n] ; /* Fail */
  a[++n]=b[--n] ; /* Fail */
  c[n][n]=c[n][n]++ ; /* should fail but doesn't */
  c[n][p]=c[n][n++] ; /* Fail */ /* if [n][n] then fails twice */

  *ptr++ = (int)ptr++ ; /* Fail */
  ptr->a = ptr->a++ ; /* should fail but doesn't */
  ptr->a = (int)(ptr++) ; /* Fail */

a = b = a++ ;   /* Fail */
b = a = --b ;   /* Fail */
a = 1 + (a=1) ; /* Fail */
a = (a=b) ;     /* Fail */
a = (a=b) + 1 ; /* Fail */

	These cases are just hacked from my regression test code, does
this look like something that is sufficiently useful to include ? What
worries me is wrongly flagging correct cases, as I know there are always
cases that are not possible to trap eg.

int a ;
int *b = &a, *c = &a ;
*b = *c++ ;

	etc.

	The other thing is that my internet connection disappears on 21st
until the summer term. I suppose that means I shouldn't post a patch as I
can't support it until then ? 

	Thanks for your advice,

		Michael Meeks.

-- 
 michael@imaginator.com  <><, Pseudo Engineer, itinerant idiot