Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:112775 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 96747 invoked from network); 6 Jan 2021 09:45:08 -0000 Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5) by pb1.pair.com with SMTP; 6 Jan 2021 09:45:08 -0000 Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 9DF901804E2 for ; Wed, 6 Jan 2021 01:21:33 -0800 (PST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,HTML_MESSAGE, RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.2 X-Spam-Virus: No X-Envelope-From: Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com [209.85.167.44]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Wed, 6 Jan 2021 01:21:33 -0800 (PST) Received: by mail-lf1-f44.google.com with SMTP id h22so5133078lfu.2 for ; Wed, 06 Jan 2021 01:21:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xiP0O5jW3hcfG4xVM4oRJ7sOrXueeO61xIfm8BSqWns=; b=ofRR+9xtegeV+MrsgYIpYdLiLUek+2T7u0iq4EqIKbr1IK347HN/KhKz27Y6N5UqJO qZH6A5EwCHdZW0uti4HTx65nLtz2LpJELjVJod5omX4Oik+WJAX6MQTcTqNi6cBYQPma gNcazL7Bj7tKURKfu+skCo0euH3x5r/Mph2va8/bbMl4bJxbtbJz+AiTlDFwdcw9bDla 1dLd1hBgRwcX0Q0+kph6hScbuABs7Amc8P2+LwVmK3VD04zeeC0p0h9JeNdvrIkp7Bti Pv2clWcg3xe8iV3AAUZrJV+wRJxJUCAC3av3cnPR0yoKv478j1OSDIpkHTlRWpafHFf1 Ry4Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=xiP0O5jW3hcfG4xVM4oRJ7sOrXueeO61xIfm8BSqWns=; b=OFtf2BkjitVRu5t8nM4sqGPmd0Pi6Dfg1WvegTfZpxn9jXTcbLMnWSsq1Q7rcP4faj LOl/K3NHyA+5gyxfSaPUI5GpxErtMbn8UB2ACEILVz7cWStP5M+AXLTapEESDdZUzebE b63oETuY/riONeAMjxUygM/jCs4XfGgNUfUJPhH+C5E8zird+icGQbFLG+aVh2BD7Oz4 +QBG/joZNig1TfTG6p2h31T+UB9Xg603yumxEwf6WIzzxQHorrHPug4unaic5EYGd57q c750Pjw2Y26+vKR+ypFwO6wZK275mufUo1jsiwgj+EQhA0VW6pKqFX4h9kBTN+xKBMGo HKSg== X-Gm-Message-State: AOAM531Kz99qynNqfo3xBcx92Z/DvTXtt3q9v6FfANIT/xFLVwTyhP6S fz5Orkwsj8eV9YZiwzHvjdGNuHNwKKOQSEqkkS8yV20L9QMo8g== X-Google-Smtp-Source: ABdhPJx2P3JSZNIra94yKh1R7PLNSwr7ryuZJH/10gRquMCIX8VFefFbPvSC46jQ5ihGAxRUlqtZIq45ipsby9YZtbI= X-Received: by 2002:a2e:9f14:: with SMTP id u20mr1792748ljk.244.1609924890132; Wed, 06 Jan 2021 01:21:30 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: Date: Wed, 6 Jan 2021 10:21:14 +0100 Message-ID: To: Larry Garfield Cc: php internals Content-Type: multipart/alternative; boundary="000000000000311c9b05b837d90f" Subject: Re: [PHP-DEV] [RFC] Enumerations, Round 2 From: nikita.ppv@gmail.com (Nikita Popov) --000000000000311c9b05b837d90f Content-Type: text/plain; charset="UTF-8" On Tue, Jan 5, 2021 at 6:56 PM Larry Garfield wrote: > On Tue, Jan 5, 2021, at 8:26 AM, Nikita Popov wrote: > > > Nice work, I like the updated proposal. Some notes: > > > > > Similarly, enum names and case names are both case insensitive. > > > > I agree that enum names should be case insensitive (like class names), > but > > why should case names be case insensitive? The closest analogon to a case > > would be a class constant, and those are case sensitive. > > This is a left over from the earlier draft when Cases were also classes. > I've corrected it. Thanks for the catch. > > > > All Cases have a read-only property, case, that is the case-sensitive > > name of the case itself. > > > > I can see how this makes sense, but it wouldn't be my first guess as to > how > > you access the case name. I'm wondering if using $enumValue->name or > > $enumValue->caseName might be preferable. > > We'll change it to ->name. > > > > Scalar equivalent values must be literals. Constants and constant > > expressions are not supported. > > > > Why? This seems inconsistent with the overall language. If I can use a > > constant expression as a class constant value, why can't I use it as an > > enum value? > > Mostly because it was more challenging to do. We're fine with including > it if Ilija can make it work. (Suggestions for him on how to do so most > efficiently are welcome.) > It may be okay to retain the limitation, but in that case the RFC should include some technical justification for that. It would also be good to explicitly state whether something like "1 + 1" is allowed, i.e. whether constant expressions are forbidden entirely, or only those that cannot be evaluated at compile-time. > > > ScalarEnum exposes an additional static method from() that is > > automatically generated. > > > > I think it would be good to be slightly more explicit here and call it > > fromValue(). (We could have fromName() to construct it from a case name, > > and any number of custom from* named constructors.) > > > > > The from() method will up-cast from a scalar to its corresponding > > Enumerated Case. Invalid scalars with no matching Case will throw a > > ValueError. There is also a has() method, which will return boolean true > if > > a case with that value exists and false otherwise > > > > Just a thought, but rather than having has() and from(), it may make > sense > > to have from() (throws if invalid) and tryFrom() (returns null if > invalid). > > I think a has() method is pretty much useless in isolation, it will > always > > be used in a combination of has() and from(), in which case it is better > to > > combine them rather than have an implicit contract between them. > > Hm, maybe. My concern is that "try" to me implies "exceptions are > involved somewhere", since PHP doesn't have the "TryX means nullable > return" convention that Rust or C# have. > > Anyone else want to weigh in? > > > > Manually defining a static from() or has() method on a Scalar Enum will > > result in a fatal error. > > > > Or a non-static one :) You can't define a static and a non-static method > > with the same name in PHP. > > Fixed. > > > > Enums and cases may have attributes attached to them, like any other > > language construct. The TARGET_CLASS target filter will include Enums > > themselves. The TARGET_CONST target filter will include Enum Cases > > > > TARGET_CONST should presumably be TARGET_CLASS_CONST. More generally > > though, I wonder if there should be a separate TARGET_CASE... > > We had that in the spec until like 40 hours ago. :-) We removed it mainly > because beberlei was pushing back on it. (And the corresponding reflection > classes, which are still under discussion.) > > Personally I'd rather have the flexibility of targeting enums and cases > specifically. Ilija and Benjamin feel it's better to "embrace the > object-y-ness" of enums and just use the existing targets. It's not a hill > I'm going to die on either way, so if you can convince them I'm happy to > add them back in. > Can't say I have a particularly strong argument either way here :) > > > Returns the scalar equivalent type of the Enum, if any. If it doesn't > > have one, it returns a ReflectionType on null. > > > > Possibly I'm misunderstanding the sentence, but why does it return a > > "ReflectionType on null" rather than just "null"? It's not like ::$value > > will have a null value in this case, it will not exist at all. > > More likely I'm misunderstanding the nuances of the reflection API. The > ideal here is to avoid a nullable return if at all possible. As noted > above reflection is the shakiest part left right now, so if you have > specific suggestions for how it would work better, please share. > Well, why do you want to avoid a nullable return here? We have many other getType/getReturnType style APIs that all return null if no type is specified, so I would expect this one to work the same way. Returning ReflectionType("null") is disingenuous, as the enum was neither declared as "enum Foo: null {}", nor does it behave as if it were declared as such. > > > Additionally, a new function is_enum(mixed): bool returns true if the > > value passed is an enum or case object. > > > > Could you please clarify what "an enum or case object" means? Is this > > function both for checking whether an object is a case object and a > > (string) class an enum class? > > It would return true if given an enum case, a variable that is assigned to > an enum case, or a class name string that refers to a class that is an > enum. Other than replicating the test for it I;m not sure how better to > describe it. (Or, just use that sentence?) > Including an example would be good. I just checked the implementation, and apparently the actual signature is "is_enum(mixed $value, bool $autoload = true): bool", which brings me to the concern I have about this API: Mixing checking for classes, which requires autoloading, and for other values is generally a bad idea -- been there, done that. The problem is that if used to check whether a value is an enum instance, then just naively writing is_enum($value) may trigger autoloading when $value happens to be a string. This is not just inefficient, but also a possible security issue. The $autoload flag can mitigate that, but that's just a workaround for bad function design. This could be addressed in different ways, e.g. one could have separate is_enum_class() and is_enum_value() functions, though the first one would probably more typically be called enum_exists(). I'm not sure if that is the right way to address it though, as the intended use case for the is_enum() function is not entirely clear to me. Regards, Nikita --000000000000311c9b05b837d90f--