ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
From: "mame (Yusuke Endoh) via ruby-core" <ruby-core@ml.ruby-lang.org>
To: ruby-core@ml.ruby-lang.org
Cc: "mame (Yusuke Endoh)" <noreply@ruby-lang.org>
Subject: [ruby-core:117195] [Ruby master Feature#20331] Should parser warn hash duplication and when clause?
Date: Fri, 15 Mar 2024 05:53:07 +0000 (UTC)	[thread overview]
Message-ID: <redmine.journal-107284.20240315055306.7872@ruby-lang.org> (raw)
In-Reply-To: redmine.issue-20331.20240312131542.7872@ruby-lang.org

Issue #20331 has been updated by mame (Yusuke Endoh).


> But we don't want to remove the warnings, so the compiler will need to warn for those cases?

At the dev meeting, Matz said that the compiler doesn't need to warn those cases. He said that just remove the warning except literally conflict cases.

> To be clear, prism already runs on PicoRuby with this warning enabled, and it works just fine. Can you clarify what concerns you have here?

Matz said he wants to make the footprint as small as possible.

Subtle cases like `{ 1.0 => a, 1.00 => b }` and `{ "\x00" => a, "\0" => b }` were also discussed, and some committers wants to keep the warning for the cases, but Matz said no need to warn such cases.

In my personal opinion: the cases that I remember this warning useful are when I copy-pasted a key-value pair of Hash literal or `when` clause of `case` statement and forgot to edit them. I don't remember any real-world conflicts like `when 10; when 0xA`. So I also think that warning a literally conflict makes sense. Still a little nervous about something like `when 1.0; when 1.00`, though.

----------------------------------------
Feature #20331: Should parser warn hash duplication and when clause?
https://bugs.ruby-lang.org/issues/20331#change-107284

* Author: yui-knk (Kaneko Yuichiro)
* Status: Open
----------------------------------------
# Background

Right now, parser warns duplicated hash keys (#1) and when clause (#2).
For example,

```ruby
{1 => :a, 1 => :b}

# => warning: key 1 is duplicated and overwritten on line 1
```

```ruby
case 2
when 1, 1
else
end

# => test.rb:2: warning: duplicated `when' clause with line 2 is ignored
```

The parser compares different cardinality numbers.

```ruby
{
    1 => :a,
  0x1 => :b,
  0b1 => :b,
  0d1 => :b,
  0o1 => :b,
}

# => test.rb:2: warning: key 1 is duplicated and overwritten on line 3
# => test.rb:3: warning: key 1 is duplicated and overwritten on line 4
# => test.rb:4: warning: key 1 is duplicated and overwritten on line 5
# => test.rb:5: warning: key 1 is duplicated and overwritten on line 6
```

# Problem

Currently this is implemeted by converting string like `"123"` to Ruby Object and compare them.
It's needed to remove Ruby Object from parse.y for Universal Parser.
I created PR https://github.com/ruby/ruby/pull/10079 which implements bignum for parse.y without dependency on Ruby Object, however nobu and mame express concern about the cost and benefit of implmenting bignum for parser.
I want to discuss which is the best approach for this problem.

By the way, it's needed to calculate irreducible fraction for Rational key if we will keep warning messages.

```ruby
$ ruby -wc -e '{10.2r => :a, 10.2r => :b}'
-e:1: warning: key (51/5) is duplicated and overwritten on line 1
-e:1: warning: unused literal ignored
Syntax OK
```

# Options

## 1. Warnings on parser

Pros: 
* Users of Universal Parser don't need to implement warnings by themselves. I guess developers of other Ruby implementation may get benefit of reducing their effort.
* Warnings are shown by `ruby -wc`.

Cons:
* We need to maintain bignum implementation for parser.

There are two approaches for this option.

### 1-1. Implement bignum for parser

The PR is this approach, implementing sub set of Ruby bignum for parser.

### 1-2. Extract existing bignum implementation then use it

Make existing bignum implementation to be independent of Ruby Object and use it from both bignum.c and parse.y.

## 2. Moving warnings logic into compile phase

We can use Ruby Object in compile.c. Then moving the logic into compile.c solves this problem.

Pros:
* No need to implement bignum for parser.

Cons:
* Users of Universal Parser need to implement warnings by themselves.
* Warnings are not shown by `ruby -wc`.




-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

  parent reply	other threads:[~2024-03-15  5:53 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-12 13:15 [ruby-core:117115] [Ruby master Feature#20331] Should parser warn hash duplication and when clause? yui-knk (Kaneko Yuichiro) via ruby-core
2024-03-12 13:20 ` [ruby-core:117117] " byroot (Jean Boussier) via ruby-core
2024-03-12 14:10 ` [ruby-core:117118] " kddnewton (Kevin Newton) via ruby-core
2024-03-13  0:55 ` [ruby-core:117124] " tenderlovemaking (Aaron Patterson) via ruby-core
2024-03-13  8:21 ` [ruby-core:117126] " zverok (Victor Shepelev) via ruby-core
2024-03-13  8:55 ` [ruby-core:117127] " byroot (Jean Boussier) via ruby-core
2024-03-14  1:50 ` [ruby-core:117135] " kddnewton (Kevin Newton) via ruby-core
2024-03-14  8:07 ` [ruby-core:117143] " matz (Yukihiro Matsumoto) via ruby-core
2024-03-14 13:12 ` [ruby-core:117168] " kddnewton (Kevin Newton) via ruby-core
2024-03-15  5:53 ` mame (Yusuke Endoh) via ruby-core [this message]
2024-03-15 12:46 ` [ruby-core:117196] " kddnewton (Kevin Newton) via ruby-core

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.ruby-lang.org/en/community/mailing-lists/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=redmine.journal-107284.20240315055306.7872@ruby-lang.org \
    --to=ruby-core@ruby-lang.org \
    --cc=noreply@ruby-lang.org \
    --cc=ruby-core@ml.ruby-lang.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).