facebook / hhvm

A virtual machine for executing programs written in Hack.
https://hhvm.com
Other
18.18k stars 2.99k forks source link

strict mode inconsistent: needs typehints for global constants, not for class constants #4873

Open fredemmott opened 9 years ago

fredemmott commented 9 years ago

This seems weird:

<?hh // strict

class Foo {
  const int TYPED_CONST = 567;
  // This is fine...
  const UNTYPED_CONST = 123;
}

const int TYPED_CONST = 123;
// test.php:11:7,19: Please add a type hint (Naming[2001])
const UNTYPED_CONST = 456;
jwatzman commented 9 years ago

I think we'll automatically infer UNTYPED_CONST as int (which we can do due to the limited number of things you can put in the RHS of a const). We don't do this for toplevel consts to support untyped consts in partial mode (this is basically exclusively for FB MUST_PREPARE and friends).

Agree this is inconsistent and we should try to fix it, but removing "probably easy" since it's a little more subtle than that.

lexidor commented 1 year ago

Partial mode is not a thing anymore blog post. The argument for inferring const X = 5; at top-level scope to be TAny is no longer applicable.

A requirement for a explicit type on a class constant makes a lot of sense. The type of a class constant had to be compatible with any trait or super classes implementations. I'd prefer to be explicit here, since inferring a more specific type could break other (compatible) declarations.

I don't mind the requirement on top-level constants, but the argument could be made this requirement is moot in almost all cases. Constant at top-level scopes are semantically equivalent to a variable you can assign only once. Those don't require a type to be explicitly declared.

The most specific type can be inferred from the RHS, except when declaring a newtype constant.

newtype FilePermission = int;
const FilePermission NO_RESTRICTIONS = 0o777;

Looking at the past, having two different behaviors between strict and partial mode would have been confusing. In strict mode, the type would be inferred, but in partial mode, it would have been TAny. 4.136 has lost support. All constants at top-level have a type specified now. Allowing the new "infer just like a variable" behavior would not be a bc break.