Box type params and arguments fields on the class definition node (#6275)

## Summary

This PR boxes the `TypeParams` and `Arguments` fields on the class
definition node. These fields are optional and often emitted, and given
that class definition is our largest enum variant, we pay the cost of
including them for every statement in the AST. Boxing these types
reduces the statement size by 40 bytes, which seems like a good tradeoff
given how infrequently these are accessed.

## Test Plan

Need to benchmark, but no behavior changes.
This commit is contained in:
Charlie Marsh 2023-08-02 12:47:06 -04:00 committed by GitHub
parent 8c40886f87
commit 8a0f844642
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
23 changed files with 87 additions and 102 deletions

View file

@ -218,7 +218,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
checker,
expr,
id,
arguments.as_ref(),
arguments.as_deref(),
);
}
}

View file

@ -374,15 +374,17 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
flake8_django::rules::nullable_model_string_field(checker, body);
}
if checker.enabled(Rule::DjangoExcludeWithModelForm) {
if let Some(diagnostic) =
flake8_django::rules::exclude_with_model_form(checker, arguments.as_ref(), body)
{
if let Some(diagnostic) = flake8_django::rules::exclude_with_model_form(
checker,
arguments.as_deref(),
body,
) {
checker.diagnostics.push(diagnostic);
}
}
if checker.enabled(Rule::DjangoAllWithModelForm) {
if let Some(diagnostic) =
flake8_django::rules::all_with_model_form(checker, arguments.as_ref(), body)
flake8_django::rules::all_with_model_form(checker, arguments.as_deref(), body)
{
checker.diagnostics.push(diagnostic);
}
@ -390,7 +392,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::DjangoUnorderedBodyContentInModel) {
flake8_django::rules::unordered_body_content_in_model(
checker,
arguments.as_ref(),
arguments.as_deref(),
body,
);
}
@ -428,7 +430,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::ErrorSuffixOnExceptionName) {
if let Some(diagnostic) = pep8_naming::rules::error_suffix_on_exception_name(
stmt,
arguments.as_ref(),
arguments.as_deref(),
name,
&checker.settings.pep8_naming.ignore_names,
) {
@ -444,7 +446,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
checker,
stmt,
name,
arguments.as_ref(),
arguments.as_deref(),
body,
);
}
@ -485,7 +487,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
flake8_builtins::rules::builtin_variable_shadowing(checker, name, name.range());
}
if checker.enabled(Rule::DuplicateBases) {
pylint::rules::duplicate_bases(checker, name, arguments.as_ref());
pylint::rules::duplicate_bases(checker, name, arguments.as_deref());
}
if checker.enabled(Rule::NoSlotsInStrSubclass) {
flake8_slots::rules::no_slots_in_str_subclass(checker, stmt, class_def);

View file

@ -132,7 +132,7 @@ fn is_standard_library_override(
class_def: &ast::StmtClassDef,
model: &SemanticModel,
) -> bool {
let Some(Arguments { args: bases, .. }) = class_def.arguments.as_ref() else {
let Some(Arguments { args: bases, .. }) = class_def.arguments.as_deref() else {
return false;
};
match name {

View file

@ -60,7 +60,7 @@ pub(crate) fn model_without_dunder_str(
..
}: &ast::StmtClassDef,
) {
if !is_non_abstract_model(arguments.as_ref(), body, checker.semantic()) {
if !is_non_abstract_model(arguments.as_deref(), body, checker.semantic()) {
return;
}
if has_dunder_method(body) {

View file

@ -1,10 +1,10 @@
use ruff_python_ast::{self as ast, Arguments, Expr, Ranged, Stmt};
use rustc_hash::FxHashSet;
use ruff_diagnostics::Diagnostic;
use ruff_diagnostics::Violation;
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::comparable::ComparableExpr;
use ruff_python_ast::{self as ast, Expr, Ranged, Stmt};
use crate::checkers::ast::Checker;
@ -54,15 +54,11 @@ impl Violation for NonUniqueEnums {
/// PIE796
pub(crate) fn non_unique_enums(checker: &mut Checker, parent: &Stmt, body: &[Stmt]) {
let Stmt::ClassDef(ast::StmtClassDef {
arguments: Some(Arguments { args: bases, .. }),
..
}) = parent
else {
let Stmt::ClassDef(parent) = parent else {
return;
};
if !bases.iter().any(|expr| {
if !parent.bases().any(|expr| {
checker
.semantic()
.resolve_call_path(expr)

View file

@ -186,7 +186,7 @@ pub(crate) fn non_self_return_type(
match name {
"__iter__" => {
if is_iterable(returns, checker.semantic())
&& is_iterator(class_def.arguments.as_ref(), checker.semantic())
&& is_iterator(class_def.arguments.as_deref(), checker.semantic())
{
checker.diagnostics.push(Diagnostic::new(
NonSelfReturnType {
@ -199,7 +199,7 @@ pub(crate) fn non_self_return_type(
}
"__aiter__" => {
if is_async_iterable(returns, checker.semantic())
&& is_async_iterator(class_def.arguments.as_ref(), checker.semantic())
&& is_async_iterator(class_def.arguments.as_deref(), checker.semantic())
{
checker.diagnostics.push(Diagnostic::new(
NonSelfReturnType {

View file

@ -570,7 +570,7 @@ pub(crate) fn unannotated_assignment_in_stub(
}
if let ScopeKind::Class(ast::StmtClassDef { arguments, .. }) = checker.semantic().scope().kind {
if is_enum(arguments.as_ref(), checker.semantic()) {
if is_enum(arguments.as_deref(), checker.semantic()) {
return;
}
}

View file

@ -1,6 +1,6 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Arguments, Expr, Stmt};
use ruff_python_ast::{self as ast, Expr, Stmt};
use ruff_python_semantic::Scope;
use crate::checkers::ast::Checker;
@ -217,17 +217,12 @@ pub(crate) fn unused_private_protocol(
continue;
};
let Stmt::ClassDef(ast::StmtClassDef {
name,
arguments: Some(Arguments { args: bases, .. }),
..
}) = checker.semantic().stmts[source]
else {
let Stmt::ClassDef(class_def) = checker.semantic().stmts[source] else {
continue;
};
if !bases
.iter()
if !class_def
.bases()
.any(|base| checker.semantic().match_typing_expr(base, "Protocol"))
{
continue;
@ -235,7 +230,7 @@ pub(crate) fn unused_private_protocol(
diagnostics.push(Diagnostic::new(
UnusedPrivateProtocol {
name: name.to_string(),
name: class_def.name.to_string(),
},
binding.range,
));
@ -308,17 +303,12 @@ pub(crate) fn unused_private_typed_dict(
let Some(source) = binding.source else {
continue;
};
let Stmt::ClassDef(ast::StmtClassDef {
name,
arguments: Some(Arguments { args: bases, .. }),
..
}) = checker.semantic().stmts[source]
else {
let Stmt::ClassDef(class_def) = checker.semantic().stmts[source] else {
continue;
};
if !bases
.iter()
if !class_def
.bases()
.any(|base| checker.semantic().match_typing_expr(base, "TypedDict"))
{
continue;
@ -326,7 +316,7 @@ pub(crate) fn unused_private_typed_dict(
diagnostics.push(Diagnostic::new(
UnusedPrivateTypedDict {
name: name.to_string(),
name: class_def.name.to_string(),
},
binding.range,
));

View file

@ -63,7 +63,7 @@ pub(crate) fn no_slots_in_namedtuple_subclass(
stmt: &Stmt,
class: &StmtClassDef,
) {
let Some(Arguments { args: bases, .. }) = class.arguments.as_ref() else {
let Some(Arguments { args: bases, .. }) = class.arguments.as_deref() else {
return;
};

View file

@ -51,7 +51,7 @@ impl Violation for NoSlotsInStrSubclass {
/// SLOT000
pub(crate) fn no_slots_in_str_subclass(checker: &mut Checker, stmt: &Stmt, class: &StmtClassDef) {
let Some(Arguments { args: bases, .. }) = class.arguments.as_ref() else {
let Some(Arguments { args: bases, .. }) = class.arguments.as_deref() else {
return;
};

View file

@ -51,7 +51,7 @@ impl Violation for NoSlotsInTupleSubclass {
/// SLOT001
pub(crate) fn no_slots_in_tuple_subclass(checker: &mut Checker, stmt: &Stmt, class: &StmtClassDef) {
let Some(Arguments { args: bases, .. }) = class.arguments.as_ref() else {
let Some(Arguments { args: bases, .. }) = class.arguments.as_deref() else {
return;
};

View file

@ -1,6 +1,3 @@
use ruff_python_ast as ast;
use ruff_python_ast::Arguments;
use ruff_python_ast::call_path::from_qualified_name;
use ruff_python_ast::helpers::map_callable;
use ruff_python_semantic::{Binding, BindingKind, ScopeKind, SemanticModel};
@ -38,38 +35,31 @@ pub(crate) fn runtime_evaluated(
}
fn runtime_evaluated_base_class(base_classes: &[String], semantic: &SemanticModel) -> bool {
if let ScopeKind::Class(ast::StmtClassDef {
arguments: Some(Arguments { args: bases, .. }),
..
}) = &semantic.scope().kind
{
for base in bases {
if let Some(call_path) = semantic.resolve_call_path(base) {
if base_classes
.iter()
.any(|base_class| from_qualified_name(base_class) == call_path)
{
return true;
}
}
}
}
false
let ScopeKind::Class(class_def) = &semantic.scope().kind else {
return false;
};
class_def.bases().any(|base| {
semantic.resolve_call_path(base).is_some_and(|call_path| {
base_classes
.iter()
.any(|base_class| from_qualified_name(base_class) == call_path)
})
})
}
fn runtime_evaluated_decorators(decorators: &[String], semantic: &SemanticModel) -> bool {
if let ScopeKind::Class(ast::StmtClassDef { decorator_list, .. }) = &semantic.scope().kind {
for decorator in decorator_list {
if let Some(call_path) = semantic.resolve_call_path(map_callable(&decorator.expression))
{
if decorators
let ScopeKind::Class(class_def) = &semantic.scope().kind else {
return false;
};
class_def.decorator_list.iter().any(|decorator| {
semantic
.resolve_call_path(map_callable(&decorator.expression))
.is_some_and(|call_path| {
decorators
.iter()
.any(|decorator| from_qualified_name(decorator) == call_path)
{
return true;
}
}
}
}
false
.any(|base_class| from_qualified_name(base_class) == call_path)
})
})
}

View file

@ -166,11 +166,11 @@ fn create_properties_from_keywords(keywords: &[Keyword]) -> Result<Vec<Stmt>> {
fn create_class_def_stmt(typename: &str, body: Vec<Stmt>, base_class: &Expr) -> Stmt {
ast::StmtClassDef {
name: Identifier::new(typename.to_string(), TextRange::default()),
arguments: Some(Arguments {
arguments: Some(Box::new(Arguments {
args: vec![base_class.clone()],
keywords: vec![],
range: TextRange::default(),
}),
})),
body,
type_params: None,
decorator_list: vec![],

View file

@ -119,14 +119,14 @@ fn create_class_def_stmt(
) -> Stmt {
ast::StmtClassDef {
name: Identifier::new(class_name.to_string(), TextRange::default()),
arguments: Some(Arguments {
arguments: Some(Box::new(Arguments {
args: vec![base_class.clone()],
keywords: match total_keyword {
Some(keyword) => vec![keyword.clone()],
None => vec![],
},
range: TextRange::default(),
}),
})),
body,
type_params: None,
decorator_list: vec![],

View file

@ -40,7 +40,7 @@ impl AlwaysAutofixableViolation for UnnecessaryClassParentheses {
/// UP039
pub(crate) fn unnecessary_class_parentheses(checker: &mut Checker, class_def: &ast::StmtClassDef) {
let Some(arguments) = class_def.arguments.as_ref() else {
let Some(arguments) = class_def.arguments.as_deref() else {
return;
};

View file

@ -46,7 +46,7 @@ impl AlwaysAutofixableViolation for UselessObjectInheritance {
/// UP004
pub(crate) fn useless_object_inheritance(checker: &mut Checker, class_def: &ast::StmtClassDef) {
let Some(arguments) = class_def.arguments.as_ref() else {
let Some(arguments) = class_def.arguments.as_deref() else {
return;
};

View file

@ -51,7 +51,7 @@ pub(super) fn is_dataclass(class_def: &ast::StmtClassDef, semantic: &SemanticMod
/// Returns `true` if the given class is a Pydantic `BaseModel` or `BaseSettings` subclass.
pub(super) fn is_pydantic_model(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool {
let Some(Arguments { args: bases, .. }) = class_def.arguments.as_ref() else {
let Some(Arguments { args: bases, .. }) = class_def.arguments.as_deref() else {
return false;
};

View file

@ -965,31 +965,31 @@ impl<'a> From<&'a ast::Expr> for ComparableExpr<'a> {
#[derive(Debug, PartialEq, Eq, Hash)]
pub struct StmtFunctionDef<'a> {
name: &'a str,
parameters: ComparableParameters<'a>,
body: Vec<ComparableStmt<'a>>,
decorator_list: Vec<ComparableDecorator<'a>>,
name: &'a str,
type_params: Option<ComparableTypeParams<'a>>,
parameters: ComparableParameters<'a>,
returns: Option<ComparableExpr<'a>>,
body: Vec<ComparableStmt<'a>>,
}
#[derive(Debug, PartialEq, Eq, Hash)]
pub struct StmtAsyncFunctionDef<'a> {
name: &'a str,
parameters: ComparableParameters<'a>,
body: Vec<ComparableStmt<'a>>,
decorator_list: Vec<ComparableDecorator<'a>>,
name: &'a str,
type_params: Option<ComparableTypeParams<'a>>,
parameters: ComparableParameters<'a>,
returns: Option<ComparableExpr<'a>>,
body: Vec<ComparableStmt<'a>>,
}
#[derive(Debug, PartialEq, Eq, Hash)]
pub struct StmtClassDef<'a> {
decorator_list: Vec<ComparableDecorator<'a>>,
name: &'a str,
type_params: Option<ComparableTypeParams<'a>>,
arguments: Option<ComparableArguments<'a>>,
body: Vec<ComparableStmt<'a>>,
decorator_list: Vec<ComparableDecorator<'a>>,
type_params: Option<ComparableTypeParams<'a>>,
}
#[derive(Debug, PartialEq, Eq, Hash)]
@ -1022,6 +1022,12 @@ impl<'a> From<&'a ast::TypeParams> for ComparableTypeParams<'a> {
}
}
impl<'a> From<&'a Box<ast::TypeParams>> for ComparableTypeParams<'a> {
fn from(type_params: &'a Box<ast::TypeParams>) -> Self {
type_params.as_ref().into()
}
}
#[derive(Debug, PartialEq, Eq, Hash)]
pub enum ComparableTypeParam<'a> {
TypeVar(TypeParamTypeVar<'a>),

View file

@ -409,7 +409,7 @@ where
..
}) => {
arguments
.as_ref()
.as_deref()
.is_some_and(|Arguments { args, keywords, .. }| {
args.iter().any(|expr| any_over_expr(expr, func))
|| keywords

View file

@ -160,8 +160,8 @@ pub struct StmtClassDef {
pub range: TextRange,
pub decorator_list: Vec<Decorator>,
pub name: Identifier,
pub type_params: Option<TypeParams>,
pub arguments: Option<Arguments>,
pub type_params: Option<Box<TypeParams>>,
pub arguments: Option<Box<Arguments>>,
pub body: Vec<Stmt>,
}
@ -3032,9 +3032,10 @@ mod size_assertions {
use super::*;
use static_assertions::assert_eq_size;
assert_eq_size!(Stmt, [u8; 184]);
assert_eq_size!(Stmt, [u8; 144]);
assert_eq_size!(StmtFunctionDef, [u8; 136]);
assert_eq_size!(StmtClassDef, [u8; 176]);
assert_eq_size!(StmtAsyncFunctionDef, [u8; 136]);
assert_eq_size!(StmtClassDef, [u8; 104]);
assert_eq_size!(StmtTry, [u8; 104]);
assert_eq_size!(Expr, [u8; 80]);
assert_eq_size!(Constant, [u8; 32]);

View file

@ -58,7 +58,7 @@ impl FormatNodeRule<StmtClassDef> for FormatStmtClassDef {
write!(f, [text("class"), space(), name.format()])?;
if let Some(arguments) = arguments {
if let Some(arguments) = arguments.as_deref() {
// Drop empty parentheses, e.g., in:
// ```python
// class A():

View file

@ -1199,10 +1199,10 @@ ClassDef: ast::Stmt = {
ast::Stmt::ClassDef(
ast::StmtClassDef {
name,
arguments,
arguments: arguments.map(Box::new),
body,
decorator_list,
type_params,
type_params: type_params.map(Box::new),
range: (location..end_location).into()
},
)

View file

@ -1,5 +1,5 @@
// auto-generated: "lalrpop 0.20.0"
// sha3: aadf067e37a9f39d450f1403b759a9659c60e697758ddd2b8c2b5fa2d0d73672
// sha3: f99d8cb29227bfbe1fa07719f655304a9a93fd4715726687ef40c091adbdbad5
use num_bigint::BigInt;
use ruff_text_size::TextSize;
use ruff_python_ast::{self as ast, Ranged, MagicKind};
@ -33583,10 +33583,10 @@ fn __action170<
ast::Stmt::ClassDef(
ast::StmtClassDef {
name,
arguments,
arguments: arguments.map(Box::new),
body,
decorator_list,
type_params,
type_params: type_params.map(Box::new),
range: (location..end_location).into()
},
)