Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated most of Kernel module methods #1963

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sampersand
Copy link
Contributor

@sampersand sampersand commented Aug 13, 2024

This PR updates most of Kernel's module methods. Here's some things to note:

  1. Only Complex, Rational, open, rand, select, exec, spawn, and system remain. These methods will be addressed in later PRs, but are somewhat complicated, and I wanted to get this PR merged.
  2. There is one TODO that's still outstanding that will be completed by the time I finish the other Kernelmethods: The new assert_send_raises doesn't actually validate the signature it's passed (instead, it only validates the raised error).
  3. The def self?.XXX syntax is used to indicate module_functions. However, it doesn't mark the instance methods as private (it should be equal to def self.XXX and private def XXX).
  4. An issue I ran into while doing this is that loop with no arguments returns an Enumerator, except the each method doesn't actually pass in any arguments (ie loop.each{|*x,**y,&z| break [x, y, z] } gives you [[], {}, nil). This means the Enumerator[nil, untyped] interface is technically incorrect, as that implies it is given an argument. I've omitted the test, but I'm not sure what to do about it.

@sampersand sampersand force-pushed the swesterman/24-08-03/Kernel_module_methods branch 4 times, most recently from 52248e2 to c695200 Compare August 25, 2024 19:34
@@ -796,7 +796,7 @@ module Kernel : BasicObject
# If *const* is defined as autoload, the file name to be loaded is replaced with
# *filename*. If *const* is defined but not as autoload, does nothing.
#
def self?.autoload: (interned _module, String filename) -> NilClass
def self?.autoload: (interned const, path filename) -> nil
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we ever decide to use const as a keyword we might want to change this to constant?

@@ -970,7 +969,9 @@ module Kernel : BasicObject
# For details on `format_string`, see [Format
# Specifications](rdoc-ref:format_specifications.rdoc).
#
def self?.format: (String format, *untyped args) -> String
def self?.sprintf: (string format, hash[Symbol, untyped] keywords) -> String
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oddly enough, sprintf only allows positional or keyword arguments, but not both.

def self?.loop: () { () -> void } -> bot
| () -> ::Enumerator[nil, bot]
def self?.loop: () -> Enumerator[nil, bot]
| () { () -> void } -> untyped
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

untyped instead of bot because you can break out

core/kernel.rbs Outdated
Comment on lines 1575 to 1583
def self?.select: ([]?, ?[]?, ?[]?, ?nil) -> bot
| [R < _ToIO] (Array[R] read, ?[]?, ?[]?, ?nil ) -> [Array[R], [], []]
| [R < _ToIO] (Array[R] read, []?, []?, Time::_Timeout timeout) -> [Array[R], [], []]?
| [R < _ToIO, W < _ToIO] (Array[R] read, ?Array[W] write, ?[]?, ?nil ) -> [Array[R], Array[W], []]
| [R < _ToIO, W < _ToIO] (Array[R] read, Array[W] write, []?, Time::_Timeout timeout) -> [Array[R], Array[W], []]?
| [R < _ToIO, W < _ToIO, E < _ToIO] (Array[R] read, ?Array[W] write, ?Array[E] error, ?nil ) -> [Array[R], Array[W], Array[E]]
| [R < _ToIO, W < _ToIO, E < _ToIO] (Array[R] read, Array[W] write, Array[E] error, Time::_Timeout timeout) -> [Array[R], Array[W], Array[E]]?
| (Array[_ToIO]? read, ?Array[_ToIO]? write, ?Array[_ToIO]? error, ?Time_Timeout? timeout) -> [Array[_ToIO], Array[_ToIO], Array[_ToIO]]?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ?[]? is because you can have empty arrays or nil as options for those arguments; the timeout can only benil, or unspecified though.

| ('s' | 115, IO | path filename) -> Integer?
| ('M' | 'A' | 'C' | 77 | 65 | 67, IO | path file) -> Time
| ('-' | '=' | '<' | '>' | 45 | 60 | 61 | 62, IO | path file1, IO | path file2) -> bool
| (String | int cmd, IO | path file1, ?IO | path file2) -> (bool | Integer? | Time)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the "catch all" case, which covers all the other variants.

@@ -1728,7 +1748,7 @@ module Kernel : BasicObject
# :experimental
# : Used for experimental features that may change in future releases.
#
def self?.warn: (*_ToS msg, ?uplevel: int?, ?category: Warning::category?) -> nil
def self?.warn: (*_ToS msgs, ?uplevel: int?, ?category: Warning::category | _ToSym | nil) -> nil
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the few places in the stdlib that uses _ToSym and not interend; the return value should be a Warning::category

@@ -331,7 +331,7 @@ def value(val, type)
when Types::Variable
true
when Types::Literal
type.literal == val
defined?(val.==) and type.literal == val
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed for the Kernel#test, as the int variants of the "catch all" variants don't define ==.

@@ -336,6 +336,27 @@ def break_from_block(value = nil)
def pass(message = nil)
assert true, message
end

def assert_send_raises(sig, errclass, object, method, ...)
# TODO: ensure args match with signature
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: The sig is actually entirely unused currently, and needs to be updated.

@sampersand sampersand force-pushed the swesterman/24-08-03/Kernel_module_methods branch 8 times, most recently from 51a4c8c to 8bff31f Compare August 25, 2024 20:18
@sampersand sampersand force-pushed the swesterman/24-08-03/Kernel_module_methods branch from 8bff31f to e4a11f4 Compare August 26, 2024 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant