Project

General

Profile

Bug #17590

`M.prepend M` has hidden side effect

Added by znz (Kazuhiro NISHIYAMA) about 1 month ago. Updated 12 days ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [x86_64-darwin19]
[ruby-core:102283]

Description

M.prepend M raises ArgumentError, but it has hidden side effect.

module M; end
class C; end
C.prepend M
C.include M
M.prepend M rescue nil
module M2; end
M2.prepend M
C.include M2
p C.ancestors # => [M, C, M2, M, M2, Object, Kernel, BasicObject]
module M; end
class C; end
C.prepend M
C.include M
module M2; end
M2.prepend M
C.include M2
p C.ancestors # => [M, C, M2, Object, Kernel, BasicObject]
#1

Updated by znz (Kazuhiro NISHIYAMA) about 1 month ago

  • Backport changed from 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN to 2.5: DONTNEED, 2.6: DONTNEED, 2.7: DONTNEED, 3.0: UNKNOWN

Updated by jeremyevans0 (Jeremy Evans) 25 days ago

There are two issues I noted here. One is that cyclic prepends modify the ancestor chain before raising an exception. A simple fix for that is to scan the module ancestor chain to look for a cyclic prepend before making a modification. I've submitted a pull request to fix that: https://github.com/ruby/ruby/pull/4165

That doesn't fix the issue in this example. The problem in this example is due to the optimization in 37e6c83609ac9d4c30ca4660ee16701e53cf82a3. I've tested reverting that commit and you get the expected result after doing so. However, that commit contains an important optimization. I don't see Alan Wu as an assignee on Redmine, so I'll commit on the related GitHub commit and see if he can look into it.

#3

Updated by alanwu (Alan Wu) 12 days ago

  • Status changed from Open to Closed

Applied in changeset git|58e82206057f2a1644b69ac3631016009ae48590.


Check for cyclic prepend before making origin

It's important to only make the origin when the prepend goes
through, as the precense of the origin informs whether to do an
origin backfill.

This plus 2d877327e fix [Bug #17590].

Also available in: Atom PDF