unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#50331: 28.0.50; Propose to obsolete cwarn.el
@ 2021-09-02  6:30 Zhiwei Chen
  2021-09-02  7:27 ` Eli Zaretskii
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Zhiwei Chen @ 2021-09-02  6:30 UTC (permalink / raw)
  To: 50331; +Cc: andlind


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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#50331: 28.0.50; Propose to obsolete cwarn.el
  2021-09-02  6:30 bug#50331: 28.0.50; Propose to obsolete cwarn.el Zhiwei Chen
@ 2021-09-02  7:27 ` Eli Zaretskii
  2021-09-02  8:12   ` Zhiwei Chen
  2021-09-02 11:10 ` Anders Lindgren
  2021-09-03  6:18 ` bug#50331: [Anders Lindgren] " Zhiwei Chen
  2 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2021-09-02  7:27 UTC (permalink / raw)
  To: Zhiwei Chen; +Cc: 50331, andlind

> From: Zhiwei Chen <condy0919@gmail.com>
> Date: Thu, 02 Sep 2021 14:30:44 +0800
> Cc: andlind@gmail.com
> 
> # conclusion
> 
> Since the first two cases are not existent, and the last one is false
> positive (IMO), I propose to obsolete cwarn.

I don't understand how you arrived at that conclusion.  cwarn provides
the diagnostics while you write the code, without any need to invoke
the compiler, and potentially long before you submit the code to the
compiler.  Some people might prefer that, so why inconvenience them?





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#50331: 28.0.50; Propose to obsolete cwarn.el
  2021-09-02  7:27 ` Eli Zaretskii
@ 2021-09-02  8:12   ` Zhiwei Chen
  2021-09-02  8:28     ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Zhiwei Chen @ 2021-09-02  8:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 50331, andlind

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Zhiwei Chen <condy0919@gmail.com>
>> Date: Thu, 02 Sep 2021 14:30:44 +0800
>> Cc: andlind@gmail.com
>> 
>> # conclusion
>> 
>> Since the first two cases are not existent, and the last one is false
>> positive (IMO), I propose to obsolete cwarn.
>
> I don't understand how you arrived at that conclusion.  cwarn provides
> the diagnostics while you write the code, without any need to invoke
> the compiler, and potentially long before you submit the code to the
> compiler.  Some people might prefer that, so why inconvenience them?

Why would I like to obsolete cwarn?

Actually, I'm making an introduction to Emacs builtin modes (in Chinese)
to see how powerful the vanilla Emacs is. But I found, Emacs is not so
orthogonal in functionality. e.g., `whitespace-mode' provides
`whitespace-cleanup' while simple.el has `delete-trailing-whitespace'.
Is it time to do subtraction for Emacs? There are too many packages
merged into Emacs, but a few are removed from Emacs (to elpa maybe).

The main reason for obsoleting cwarn is that its functionality can be
superseded by flycheck/flymake. The result of
https://grep.app/search?q=cwarn-mode shows that there are few users of
cwarn. Of course this result may be wrong, after all https://grep.app/
only searches the git repo and all data on the Internet. The safe way to
do this is to raise a poll and let the community decide if cwarn should
be removed. Since Emacs is such a monster of 40yrs, the expected result
is that most users never know there is a package named cwarn.

[1]: https://emacs-china.org/t/emacs-builtin-mode/11937

-- 
Zhiwei Chen





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#50331: 28.0.50; Propose to obsolete cwarn.el
  2021-09-02  8:12   ` Zhiwei Chen
@ 2021-09-02  8:28     ` Eli Zaretskii
  2021-09-02  8:51       ` Zhiwei Chen
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2021-09-02  8:28 UTC (permalink / raw)
  To: Zhiwei Chen; +Cc: 50331, andlind

> From: Zhiwei Chen <condy0919@gmail.com>
> Cc: 50331@debbugs.gnu.org,  andlind@gmail.com
> Date: Thu, 02 Sep 2021 16:12:03 +0800
> 
> Why would I like to obsolete cwarn?
> 
> Actually, I'm making an introduction to Emacs builtin modes (in Chinese)
> to see how powerful the vanilla Emacs is. But I found, Emacs is not so
> orthogonal in functionality. e.g., `whitespace-mode' provides
> `whitespace-cleanup' while simple.el has `delete-trailing-whitespace'.
> Is it time to do subtraction for Emacs? There are too many packages
> merged into Emacs, but a few are removed from Emacs (to elpa maybe).

That's true, and one way we have for doing that is moving some stuff
to GNU ELPA.  But that is WIP, and we didn't yet figure out how to
move stuff to ELPA from core without punishing our users.  So we need
to wait for that work to be completed before we can talk about making
Emacs core thinner.

In any case, obsoleting packages is not the right way of doing that,
IMO: there's nothing obsolete in a package that offers some
functionality which doesn't have a 1:1 replacement.

> The main reason for obsoleting cwarn is that its functionality can be
> superseded by flycheck/flymake.

Those packages require a compiler to be installed, which is one
prerequisite cwarn doesn't have.  And I personally don't yet feel
flymake is functional enough and stable enough to be a complete
replacement for specialized packages like cwarn; if nothing else,
flymake takes much more resources than cwarn.

(flycheck is not relevant, since it is not part of Emacs, and probably
never will be.)

> The result of https://grep.app/search?q=cwarn-mode shows that there
> are few users of cwarn.

That's not enough, because if you are one of those few users, removing
the package will deliver a blow, and knowing that you are a member of
a small group doesn't help.

> Of course this result may be wrong, after all https://grep.app/
> only searches the git repo and all data on the Internet. The safe way to
> do this is to raise a poll and let the community decide if cwarn should
> be removed. Since Emacs is such a monster of 40yrs, the expected result
> is that most users never know there is a package named cwarn.

In a recent discussion on Reddit, it turned out many don't know about
dabbrev.el as well, but I hope no one will propose to obsolete or
remove it on those grounds.

So I think we should not obsolete cwarn for now.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#50331: 28.0.50; Propose to obsolete cwarn.el
  2021-09-02  8:28     ` Eli Zaretskii
@ 2021-09-02  8:51       ` Zhiwei Chen
  0 siblings, 0 replies; 8+ messages in thread
From: Zhiwei Chen @ 2021-09-02  8:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 50331, andlind

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Zhiwei Chen <condy0919@gmail.com>
>> Cc: 50331@debbugs.gnu.org,  andlind@gmail.com
>> Date: Thu, 02 Sep 2021 16:12:03 +0800
>> 
>> Why would I like to obsolete cwarn?
>> 
>> Actually, I'm making an introduction to Emacs builtin modes (in Chinese)
>> to see how powerful the vanilla Emacs is. But I found, Emacs is not so
>> orthogonal in functionality. e.g., `whitespace-mode' provides
>> `whitespace-cleanup' while simple.el has `delete-trailing-whitespace'.
>> Is it time to do subtraction for Emacs? There are too many packages
>> merged into Emacs, but a few are removed from Emacs (to elpa maybe).
>
> That's true, and one way we have for doing that is moving some stuff
> to GNU ELPA.  But that is WIP, and we didn't yet figure out how to
> move stuff to ELPA from core without punishing our users.  So we need
> to wait for that work to be completed before we can talk about making
> Emacs core thinner.
>
> In any case, obsoleting packages is not the right way of doing that,
> IMO: there's nothing obsolete in a package that offers some
> functionality which doesn't have a 1:1 replacement.

Thanks for the effort in Emacs.

> In a recent discussion on Reddit, it turned out many don't know about
> dabbrev.el as well, but I hope no one will propose to obsolete or
> remove it on those grounds.
>
> So I think we should not obsolete cwarn for now.

Got it, Emacs developers have already been working on that.

-- 
Zhiwei Chen





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#50331: 28.0.50; Propose to obsolete cwarn.el
  2021-09-02  6:30 bug#50331: 28.0.50; Propose to obsolete cwarn.el Zhiwei Chen
  2021-09-02  7:27 ` Eli Zaretskii
@ 2021-09-02 11:10 ` Anders Lindgren
  2021-09-02 16:12   ` Lars Ingebrigtsen
  2021-09-03  6:18 ` bug#50331: [Anders Lindgren] " Zhiwei Chen
  2 siblings, 1 reply; 8+ messages in thread
From: Anders Lindgren @ 2021-09-02 11:10 UTC (permalink / raw)
  To: Zhiwei Chen; +Cc: 50331

[-- Attachment #1: Type: text/plain, Size: 2978 bytes --]

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
>

[-- Attachment #2: Type: text/html, Size: 3776 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#50331: 28.0.50; Propose to obsolete cwarn.el
  2021-09-02 11:10 ` Anders Lindgren
@ 2021-09-02 16:12   ` Lars Ingebrigtsen
  0 siblings, 0 replies; 8+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-02 16:12 UTC (permalink / raw)
  To: Anders Lindgren; +Cc: Zhiwei Chen, 50331

The conclusion here was that we're not going to obsolete cwarn.el, so
I'm closing this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#50331: [Anders Lindgren] Re: 28.0.50; Propose to obsolete cwarn.el
  2021-09-02  6:30 bug#50331: 28.0.50; Propose to obsolete cwarn.el Zhiwei Chen
  2021-09-02  7:27 ` Eli Zaretskii
  2021-09-02 11:10 ` Anders Lindgren
@ 2021-09-03  6:18 ` Zhiwei Chen
  2 siblings, 0 replies; 8+ messages in thread
From: Zhiwei Chen @ 2021-09-03  6:18 UTC (permalink / raw)
  To: 50331

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]


-------------------- 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


[-- Attachment #2.1: Type: text/plain, Size: 2978 bytes --]

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
>

[-- Attachment #2.2: Type: text/html, Size: 3776 bytes --]

[-- Attachment #3: Type: text/plain, Size: 84 bytes --]

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

-- 
Zhiwei Chen

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-09-03  6:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-02  6:30 bug#50331: 28.0.50; Propose to obsolete cwarn.el Zhiwei Chen
2021-09-02  7:27 ` Eli Zaretskii
2021-09-02  8:12   ` Zhiwei Chen
2021-09-02  8:28     ` Eli Zaretskii
2021-09-02  8:51       ` Zhiwei Chen
2021-09-02 11:10 ` Anders Lindgren
2021-09-02 16:12   ` Lars Ingebrigtsen
2021-09-03  6:18 ` bug#50331: [Anders Lindgren] " Zhiwei Chen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).