bug#50331: [Anders Lindgren] Re: 28.0.50; Propose to obsolete cwarn.el

[Zhiwei Chen]

-------------------- Start of forwarded message --------------------
From: Anders Lindgren <andlind@gmail.com>
Date: Thu, 2 Sep 2021 13:10:56 +0200
Subject: Re: 28.0.50; Propose to obsolete cwarn.el
To: Zhiwei Chen <condy0919@gmail.com>
Cc: bug-gnu-emacs@gnu.org

Hi!

I strongly object to deprecating CWarn!

I can agree that highlighting reference parameters might not make much sense today.

However, highlighting trailing semicolons and assignments inside expressions are things that often make code easier to read and understand, and it prevents you from writing broken code.

In the assignment case, you only get warnings for trivial cases like "if (x = 0)". However, in more complex cases, where you have an intentional assignment deep inside an _expression_, the compiler doesn't (and shouldn't) issue an error or warning. However, having the operator highlighted makes a world of difference.

For example:

    if (   x->SomeTest()

        && p = x->ThisMightPointToSomething()          // By highlighting the "=", this code is easier to read.

        && score += p->Score())                                   // Another one, which is hard to miss.

    {

      ..

    }

Again, trailing semicolons, especially for "for", is a valid code pattern and few compilers I know of would issue a warning for it. In your example, gcc only issues a warning if the next line is indented. Highlighting the semicolon, on the other hand, still makes sense to me.

Instead of deprecating CWarn, why don't we try to modernize it? If I had written the package today (rather than in 1999) I would have defined a new face that would inherit from font-lock-warning-face. By doing it that way, the new face would work in all color themes, at the same time a user can redefine it if there is a need for it.

There was a poll that was posted to the Emacs community a couple of years ago about minor modes that should be enabled by default. If I remember correctly, CWarn mode was very high in that list (it might even have topped it). Unfortunately I no longer remember where the poll took place.

    -- Anders Lindgren

On Thu, Sep 2, 2021 at 8:30 AM Zhiwei Chen <condy0919@gmail.com> wrote:

Today, modern compilers can recognize the three cases described in cwarn file.

# semicolon after if/for/while

if (value);
  foo();

g++ -Wall produces helpful diagnostic information:

a.cpp:11:5: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’
   11 |     foo();
      |     ^~~


# assignment in _expression_

if (x = 0)
  foo();

a.cpp:10:9: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
   10 |   if (x = 0);
      |       ~~^~~

g++ -Wall does a great job.


# reference parameter uses

int bar(int& a) {
  return a;
}

I don't think it should be displayed in warning face, it's widely used today.

# conclusion

Since the first two cases are not existent, and the last one is false
positive (IMO), I propose to obsolete cwarn.

CCed Anders Lindgren, the author of cwarn.

--
Zhiwei Chen

-------------------- End of forwarded message --------------------

-- 
Zhiwei Chen