From 0e652f0ceedf15b7cbbbd6518719d43fb326cd93 Mon Sep 17 00:00:00 2001 From: Sam Goldman Date: Mon, 13 Jan 2025 13:22:04 -0800 Subject: [PATCH] Store Quantified instead of Unique in Args/Kwargs Summary: In D64797380 I made these store the Unique id of the relevant Quantified, because the QuantifiedKind would always be ParamSpec, so that bit wasn't necessary. This diff just uses the Quantified instead. For code that visits over the structure of code looking for Quantifieds, we can avoid adding extra cases. For example: * Deterministic printing * Quantified -> Name lookups (part of printing, see note) * Substitution Note that quantified name printing is currently incomplete. The display code walks types collecting bound tparams in types, but this doesn't account for bound tparams in _scope_. As a result, we still print out lots of types with `?_` instead of the name. Reviewed By: rchen152 Differential Revision: D67998105 fbshipit-source-id: b1008bf86470c89727082e9b1204b1326187fae9 --- pyre2/conformance/third_party/conformance.exp | 64 +++++++++---------- .../third_party/conformance.result | 14 ++-- pyre2/pyre2/bin/alt/attr.rs | 4 +- pyre2/pyre2/bin/test/callable.rs | 2 +- pyre2/pyre2/bin/types/display.rs | 12 +++- pyre2/pyre2/bin/types/types.rs | 11 +--- 6 files changed, 54 insertions(+), 53 deletions(-) diff --git a/pyre2/conformance/third_party/conformance.exp b/pyre2/conformance/third_party/conformance.exp index e3a21766e63..6173a234141 100644 --- a/pyre2/conformance/third_party/conformance.exp +++ b/pyre2/conformance/third_party/conformance.exp @@ -10025,8 +10025,8 @@ { "code": -2, "column": 12, - "concise_description": "EXPECTED Callable[[Var[Args[_]], KwArgs[Kwargs[_]]], None] <: Callable[ParamSpec(?_), None]", - "description": "EXPECTED Callable[[Var[Args[_]], KwArgs[Kwargs[_]]], None] <: Callable[ParamSpec(?_), None]", + "concise_description": "EXPECTED Callable[[Var[Args[?_]], KwArgs[Kwargs[?_]]], None] <: Callable[ParamSpec(?_), None]", + "description": "EXPECTED Callable[[Var[Args[?_]], KwArgs[Kwargs[?_]]], None] <: Callable[ParamSpec(?_), None]", "line": 53, "name": "PyreError", "stop_column": 15, @@ -10055,8 +10055,8 @@ { "code": -2, "column": 14, - "concise_description": "`Args[_]` is not iterable", - "description": "`Args[_]` is not iterable", + "concise_description": "`Args[?_]` is not iterable", + "description": "`Args[?_]` is not iterable", "line": 68, "name": "PyreError", "stop_column": 19, @@ -10065,8 +10065,8 @@ { "code": -2, "column": 11, - "concise_description": "`Args[_]` is not iterable", - "description": "`Args[_]` is not iterable", + "concise_description": "`Args[?_]` is not iterable", + "description": "`Args[?_]` is not iterable", "line": 70, "name": "PyreError", "stop_column": 16, @@ -10075,8 +10075,8 @@ { "code": -2, "column": 11, - "concise_description": "`Args[_]` is not iterable", - "description": "`Args[_]` is not iterable", + "concise_description": "`Args[?_]` is not iterable", + "description": "`Args[?_]` is not iterable", "line": 72, "name": "PyreError", "stop_column": 16, @@ -10085,8 +10085,8 @@ { "code": -2, "column": 12, - "concise_description": "EXPECTED Callable[[Var[Args[_]], KwArgs[Kwargs[_]]], None] <: Callable[ParamSpec(?_), None]", - "description": "EXPECTED Callable[[Var[Args[_]], KwArgs[Kwargs[_]]], None] <: Callable[ParamSpec(?_), None]", + "concise_description": "EXPECTED Callable[[Var[Args[?_]], KwArgs[Kwargs[?_]]], None] <: Callable[ParamSpec(?_), None]", + "description": "EXPECTED Callable[[Var[Args[?_]], KwArgs[Kwargs[?_]]], None] <: Callable[ParamSpec(?_), None]", "line": 74, "name": "PyreError", "stop_column": 15, @@ -10105,8 +10105,8 @@ { "code": -2, "column": 16, - "concise_description": "`Args[_]` is not iterable", - "description": "`Args[_]` is not iterable", + "concise_description": "`Args[?_]` is not iterable", + "description": "`Args[?_]` is not iterable", "line": 82, "name": "PyreError", "stop_column": 21, @@ -10115,8 +10115,8 @@ { "code": -2, "column": 25, - "concise_description": "Expected argument after ** to be a mapping, got: Kwargs[_]", - "description": "Expected argument after ** to be a mapping, got: Kwargs[_]", + "concise_description": "Expected argument after ** to be a mapping, got: Kwargs[?_]", + "description": "Expected argument after ** to be a mapping, got: Kwargs[?_]", "line": 82, "name": "PyreError", "stop_column": 31, @@ -10135,8 +10135,8 @@ { "code": -2, "column": 18, - "concise_description": "`Args[_]` is not iterable", - "description": "`Args[_]` is not iterable", + "concise_description": "`Args[?_]` is not iterable", + "description": "`Args[?_]` is not iterable", "line": 83, "name": "PyreError", "stop_column": 23, @@ -10145,8 +10145,8 @@ { "code": -2, "column": 27, - "concise_description": "Expected argument after ** to be a mapping, got: Kwargs[_]", - "description": "Expected argument after ** to be a mapping, got: Kwargs[_]", + "concise_description": "Expected argument after ** to be a mapping, got: Kwargs[?_]", + "description": "Expected argument after ** to be a mapping, got: Kwargs[?_]", "line": 83, "name": "PyreError", "stop_column": 33, @@ -10155,8 +10155,8 @@ { "code": -2, "column": 12, - "concise_description": "EXPECTED Callable[[Var[Args[_]], KwArgs[Kwargs[_]]], None] <: Callable[ParamSpec(?_), None]", - "description": "EXPECTED Callable[[Var[Args[_]], KwArgs[Kwargs[_]]], None] <: Callable[ParamSpec(?_), None]", + "concise_description": "EXPECTED Callable[[Var[Args[?_]], KwArgs[Kwargs[?_]]], None] <: Callable[ParamSpec(?_), None]", + "description": "EXPECTED Callable[[Var[Args[?_]], KwArgs[Kwargs[?_]]], None] <: Callable[ParamSpec(?_), None]", "line": 85, "name": "PyreError", "stop_column": 15, @@ -10195,8 +10195,8 @@ { "code": -2, "column": 20, - "concise_description": "EXPECTED Literal[1] <: Args[_]", - "description": "EXPECTED Literal[1] <: Args[_]", + "concise_description": "EXPECTED Literal[1] <: Args[?_]", + "description": "EXPECTED Literal[1] <: Args[?_]", "line": 96, "name": "PyreError", "stop_column": 21, @@ -10205,8 +10205,8 @@ { "code": -2, "column": 23, - "concise_description": "EXPECTED Literal['A'] <: Args[_]", - "description": "EXPECTED Literal['A'] <: Args[_]", + "concise_description": "EXPECTED Literal['A'] <: Args[?_]", + "description": "EXPECTED Literal['A'] <: Args[?_]", "line": 96, "name": "PyreError", "stop_column": 26, @@ -10225,8 +10225,8 @@ { "code": -2, "column": 22, - "concise_description": "EXPECTED Literal['A'] <: Kwargs[_]", - "description": "EXPECTED Literal['A'] <: Kwargs[_]", + "concise_description": "EXPECTED Literal['A'] <: Kwargs[?_]", + "description": "EXPECTED Literal['A'] <: Kwargs[?_]", "line": 97, "name": "PyreError", "stop_column": 25, @@ -10235,8 +10235,8 @@ { "code": -2, "column": 29, - "concise_description": "EXPECTED Literal[1] <: Kwargs[_]", - "description": "EXPECTED Literal[1] <: Kwargs[_]", + "concise_description": "EXPECTED Literal[1] <: Kwargs[?_]", + "description": "EXPECTED Literal[1] <: Kwargs[?_]", "line": 97, "name": "PyreError", "stop_column": 30, @@ -10255,8 +10255,8 @@ { "code": -2, "column": 20, - "concise_description": "EXPECTED Literal['A'] <: Args[_]", - "description": "EXPECTED Literal['A'] <: Args[_]", + "concise_description": "EXPECTED Literal['A'] <: Args[?_]", + "description": "EXPECTED Literal['A'] <: Args[?_]", "line": 98, "name": "PyreError", "stop_column": 23, @@ -10265,8 +10265,8 @@ { "code": -2, "column": 25, - "concise_description": "EXPECTED Literal[1] <: Args[_]", - "description": "EXPECTED Literal[1] <: Args[_]", + "concise_description": "EXPECTED Literal[1] <: Args[?_]", + "description": "EXPECTED Literal[1] <: Args[?_]", "line": 98, "name": "PyreError", "stop_column": 26, diff --git a/pyre2/conformance/third_party/conformance.result b/pyre2/conformance/third_party/conformance.result index 429c125cba2..de4c81c72bc 100644 --- a/pyre2/conformance/third_party/conformance.result +++ b/pyre2/conformance/third_party/conformance.result @@ -724,17 +724,17 @@ "Line 41: Expected 1 errors", "Line 60: Expected 1 errors", "Line 47: Unexpected errors ['assert_type(Any, int) failed', 'Answers::expr_infer wrong number of arguments to call']", - "Line 53: Unexpected errors ['EXPECTED Callable[[Var[Args[_]], KwArgs[Kwargs[_]]], None] <: Callable[ParamSpec(?_), None]']", + "Line 53: Unexpected errors ['EXPECTED Callable[[Var[Args[?_]], KwArgs[Kwargs[?_]]], None] <: Callable[ParamSpec(?_), None]']", "Line 56: Unexpected errors ['TODO: Subscript(ExprSubscript - expr_infer, Callable type']", "Line 66: Unexpected errors ['TODO: Subscript(ExprSubscript - expr_infer, Callable type']", - "Line 68: Unexpected errors ['`Args[_]` is not iterable']", - "Line 74: Unexpected errors ['EXPECTED Callable[[Var[Args[_]], KwArgs[Kwargs[_]]], None] <: Callable[ParamSpec(?_), None]']", + "Line 68: Unexpected errors ['`Args[?_]` is not iterable']", + "Line 74: Unexpected errors ['EXPECTED Callable[[Var[Args[?_]], KwArgs[Kwargs[?_]]], None] <: Callable[ParamSpec(?_), None]']", "Line 79: Unexpected errors ['Answers::expr_infer wrong number of arguments to call']", - "Line 82: Unexpected errors ['`Args[_]` is not iterable', 'Expected argument after ** to be a mapping, got: Kwargs[_]']", - "Line 85: Unexpected errors ['EXPECTED Callable[[Var[Args[_]], KwArgs[Kwargs[_]]], None] <: Callable[ParamSpec(?_), None]']", + "Line 82: Unexpected errors ['`Args[?_]` is not iterable', 'Expected argument after ** to be a mapping, got: Kwargs[?_]']", + "Line 85: Unexpected errors ['EXPECTED Callable[[Var[Args[?_]], KwArgs[Kwargs[?_]]], None] <: Callable[ParamSpec(?_), None]']", "Line 89: Unexpected errors ['Answers::expr_infer wrong number of arguments to call', 'Answers::expr_infer wrong number of arguments to call']", - "Line 96: Unexpected errors ['EXPECTED Callable[[int, str], int] <: Callable[ParamSpec(@_), int]', 'EXPECTED Literal[1] <: Args[_]', \"EXPECTED Literal['A'] <: Args[_]\"]", - "Line 97: Unexpected errors ['EXPECTED Callable[[int, str], int] <: Callable[ParamSpec(@_), int]', \"EXPECTED Literal['A'] <: Kwargs[_]\", 'EXPECTED Literal[1] <: Kwargs[_]']" + "Line 96: Unexpected errors ['EXPECTED Callable[[int, str], int] <: Callable[ParamSpec(@_), int]', 'EXPECTED Literal[1] <: Args[?_]', \"EXPECTED Literal['A'] <: Args[?_]\"]", + "Line 97: Unexpected errors ['EXPECTED Callable[[int, str], int] <: Callable[ParamSpec(@_), int]', \"EXPECTED Literal['A'] <: Kwargs[?_]\", 'EXPECTED Literal[1] <: Kwargs[?_]']" ], "generics_paramspec_semantics.py": [ "Line 98: Expected 1 errors", diff --git a/pyre2/pyre2/bin/alt/attr.rs b/pyre2/pyre2/bin/alt/attr.rs index c39289e86ad..7619e617ef9 100644 --- a/pyre2/pyre2/bin/alt/attr.rs +++ b/pyre2/pyre2/bin/alt/attr.rs @@ -160,9 +160,9 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { }, Some(AttributeBase::Quantified(q)) => { if q.is_param_spec() && attr_name == "args" { - LookupResult::Found(Type::type_form(Type::Args(q.id()))) + LookupResult::Found(Type::type_form(Type::Args(q))) } else if q.is_param_spec() && attr_name == "kwargs" { - LookupResult::Found(Type::type_form(Type::Kwargs(q.id()))) + LookupResult::Found(Type::type_form(Type::Kwargs(q))) } else { let class = q.as_value(self.stdlib); match self.get_instance_attribute(&class, attr_name) { diff --git a/pyre2/pyre2/bin/test/callable.rs b/pyre2/pyre2/bin/test/callable.rs index 30977e9ab28..21d2b500c9c 100644 --- a/pyre2/pyre2/bin/test/callable.rs +++ b/pyre2/pyre2/bin/test/callable.rs @@ -351,6 +351,6 @@ P = ParamSpec("P") def test(f: Callable[P, None]) -> Callable[P, None]: def inner(*args: P.args, **kwargs: P.kwargs) -> None: f(*args, **kwargs) # E: Answers::expr_infer wrong number of arguments to call - return inner # E: EXPECTED Callable[[Var[Args[_]], KwArgs[Kwargs[_]]], None] <: Callable[ParamSpec(?_), None] + return inner # E: EXPECTED Callable[[Var[Args[?_]], KwArgs[Kwargs[?_]]], None] <: Callable[ParamSpec(?_), None] "#, ); diff --git a/pyre2/pyre2/bin/types/display.rs b/pyre2/pyre2/bin/types/display.rs index 79214448198..ed814a4bba3 100644 --- a/pyre2/pyre2/bin/types/display.rs +++ b/pyre2/pyre2/bin/types/display.rs @@ -232,8 +232,16 @@ impl<'a> TypeDisplayContext<'a> { Type::Module(m) => write!(f, "Module[{m}]"), Type::Var(var) => write!(f, "{var}"), Type::Quantified(var) => self.fmt_quantified(var, f), - Type::Args(id) => write!(f, "Args[{id}]"), - Type::Kwargs(id) => write!(f, "Kwargs[{id}]"), + Type::Args(q) => { + write!(f, "Args[")?; + self.fmt_quantified(q, f)?; + write!(f, "]") + } + Type::Kwargs(q) => { + write!(f, "Kwargs[")?; + self.fmt_quantified(q, f)?; + write!(f, "]") + } Type::SpecialForm(x) => write!(f, "{x}"), Type::Ellipsis => write!(f, "Ellipsis"), Type::Any(style) => match style { diff --git a/pyre2/pyre2/bin/types/types.rs b/pyre2/pyre2/bin/types/types.rs index 0a8e9b602c2..04f445ba0a6 100644 --- a/pyre2/pyre2/bin/types/types.rs +++ b/pyre2/pyre2/bin/types/types.rs @@ -114,10 +114,6 @@ impl Quantified { pub fn is_param_spec(&self) -> bool { matches!(self.kind, QuantifiedKind::ParamSpec) } - - pub fn id(&self) -> Unique { - self.unique - } } /// Bundles together type param info for passing around while building TParams. @@ -358,8 +354,8 @@ pub enum Type { SpecialForm(SpecialForm), /// Used to represent `P.args`. The spec describes it as an annotation, /// but it's easier to think of it as a type that can't occur in nested positions. - Args(Unique), - Kwargs(Unique), + Args(Quantified), + Kwargs(Quantified), /// Used to represent a type that has a value representation, e.g. a class Type(Box), Ellipsis, @@ -586,9 +582,6 @@ impl Type { // FIXME: Should mostly be forcing these before printing v.zero(); } - Type::Args(id) | Type::Kwargs(id) => { - *id = Unique::zero(); - } _ => {} } })