mirror of
https://github.com/servo/servo.git
synced 2025-08-06 06:00:15 +01:00
style: Reduce the amount of code generated by UnparsedValues::substitute_variables.
This reduces the amount of assembly instructions generated by this function from 18k+ to ~800. This should make reasoning about its stack space usage sane, and should fix the ASAN stack overflows, but also we should take this regardless, because it's saner and makes reading it simpler. I also think that the writing_mode shenanigans is fixing a bug (I think before this, we'd pick the first physical value which mapped to any of the properties, which is wrong), but I haven't bothered looking for a test-case that fails before my patch. The relevant WPTs (css/css-logical/animation*) still pass. Differential Revision: https://phabricator.services.mozilla.com/D105342
This commit is contained in:
parent
b0d05d1a5d
commit
490db1e2bd
5 changed files with 57 additions and 59 deletions
|
@ -547,6 +547,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
|
|||
|
||||
declaration.value.substitute_variables(
|
||||
declaration.id,
|
||||
self.context.builder.writing_mode,
|
||||
self.context.builder.custom_properties.as_ref(),
|
||||
self.context.quirks_mode,
|
||||
self.context.device(),
|
||||
|
|
|
@ -829,11 +829,12 @@ impl PropertyDeclarationBlock {
|
|||
// getKeyframes() implementation for CSS animations, if
|
||||
// |computed_values| is supplied, we use it to expand such variable
|
||||
// declarations. This will be fixed properly in Gecko bug 1391537.
|
||||
(&PropertyDeclaration::WithVariables(ref declaration), Some(ref _computed_values)) => {
|
||||
(&PropertyDeclaration::WithVariables(ref declaration), Some(ref computed_values)) => {
|
||||
declaration
|
||||
.value
|
||||
.substitute_variables(
|
||||
declaration.id,
|
||||
computed_values.writing_mode,
|
||||
custom_properties.as_ref(),
|
||||
QuirksMode::NoQuirks,
|
||||
device,
|
||||
|
|
|
@ -948,7 +948,7 @@
|
|||
input.parse_entirely(|input| parse_value(context, input)).map(|longhands| {
|
||||
% for sub_property in shorthand.sub_properties:
|
||||
% if sub_property.may_be_disabled_in(shorthand, engine):
|
||||
if NonCustomPropertyId::from(LonghandId::${sub_property.camel_case}).allowed_in(context) {
|
||||
if NonCustomPropertyId::from(LonghandId::${sub_property.camel_case}).allowed_in_ignoring_rule_type(context) {
|
||||
% endif
|
||||
declarations.push(PropertyDeclaration::${sub_property.camel_case}(
|
||||
longhands.${sub_property.ident}
|
||||
|
@ -980,7 +980,7 @@
|
|||
#[allow(unused_imports)]
|
||||
use crate::values::specified;
|
||||
|
||||
pub fn parse_value<'i, 't>(
|
||||
fn parse_value<'i, 't>(
|
||||
context: &ParserContext,
|
||||
input: &mut Parser<'i, 't>,
|
||||
) -> Result<Longhands, ParseError<'i>> {
|
||||
|
@ -1024,7 +1024,7 @@
|
|||
#[allow(unused_imports)]
|
||||
use crate::values::specified;
|
||||
|
||||
pub fn parse_value<'i, 't>(
|
||||
fn parse_value<'i, 't>(
|
||||
context: &ParserContext,
|
||||
input: &mut Parser<'i, 't>,
|
||||
) -> Result<Longhands, ParseError<'i>> {
|
||||
|
|
|
@ -248,7 +248,7 @@ impl AnimationValue {
|
|||
decl: &PropertyDeclaration,
|
||||
context: &mut Context,
|
||||
extra_custom_properties: Option<<&Arc<crate::custom_properties::CustomPropertiesMap>>,
|
||||
initial: &ComputedValues
|
||||
initial: &ComputedValues,
|
||||
) -> Option<Self> {
|
||||
use super::PropertyDeclarationVariantRepr;
|
||||
|
||||
|
@ -374,6 +374,7 @@ impl AnimationValue {
|
|||
|
||||
declaration.value.substitute_variables(
|
||||
declaration.id,
|
||||
context.builder.writing_mode,
|
||||
custom_properties,
|
||||
context.quirks_mode,
|
||||
context.device(),
|
||||
|
|
|
@ -1194,20 +1194,18 @@ impl LonghandId {
|
|||
}
|
||||
}
|
||||
|
||||
// TODO(emilio): Should we use a function table like CASCADE_PROPERTY does
|
||||
// to avoid blowing up code-size here?
|
||||
fn parse_value<'i, 't>(
|
||||
&self,
|
||||
context: &ParserContext,
|
||||
input: &mut Parser<'i, 't>,
|
||||
) -> Result<PropertyDeclaration, ParseError<'i>> {
|
||||
match *self {
|
||||
% for property in data.longhands:
|
||||
LonghandId::${property.camel_case} => {
|
||||
longhands::${property.ident}::parse_declared(context, input)
|
||||
}
|
||||
% endfor
|
||||
}
|
||||
type ParsePropertyFn = for<'i, 't> fn(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<PropertyDeclaration, ParseError<'i>>;
|
||||
static PARSE_PROPERTY: [ParsePropertyFn; ${len(data.longhands)}] = [
|
||||
% for property in data.longhands:
|
||||
longhands::${property.ident}::parse_declared,
|
||||
% endfor
|
||||
];
|
||||
(PARSE_PROPERTY[*self as usize])(context, input)
|
||||
}
|
||||
|
||||
/// Returns whether this property is animatable.
|
||||
|
@ -1596,15 +1594,36 @@ impl ShorthandId {
|
|||
context: &ParserContext,
|
||||
input: &mut Parser<'i, 't>,
|
||||
) -> Result<(), ParseError<'i>> {
|
||||
match *self {
|
||||
% for shorthand in data.shorthands_except_all():
|
||||
ShorthandId::${shorthand.camel_case} => {
|
||||
shorthands::${shorthand.ident}::parse_into(declarations, context, input)
|
||||
}
|
||||
% endfor
|
||||
// 'all' accepts no value other than CSS-wide keywords
|
||||
ShorthandId::All => Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
|
||||
type ParseIntoFn = for<'i, 't> fn(
|
||||
declarations: &mut SourcePropertyDeclaration,
|
||||
context: &ParserContext,
|
||||
input: &mut Parser<'i, 't>,
|
||||
) -> Result<(), ParseError<'i>>;
|
||||
|
||||
fn unreachable<'i, 't>(
|
||||
_: &mut SourcePropertyDeclaration,
|
||||
_: &ParserContext,
|
||||
_: &mut Parser<'i, 't>
|
||||
) -> Result<(), ParseError<'i>> {
|
||||
unreachable!()
|
||||
}
|
||||
|
||||
// 'all' accepts no value other than CSS-wide keywords
|
||||
if *self == ShorthandId::All {
|
||||
return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
|
||||
}
|
||||
|
||||
static PARSE_INTO: [ParseIntoFn; ${len(data.shorthands)}] = [
|
||||
% for shorthand in data.shorthands:
|
||||
% if shorthand.ident == "all":
|
||||
unreachable,
|
||||
% else:
|
||||
shorthands::${shorthand.ident}::parse_into,
|
||||
% endif
|
||||
% endfor
|
||||
];
|
||||
|
||||
(PARSE_INTO[*self as usize])(declarations, context, input)
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -1647,6 +1666,7 @@ impl UnparsedValue {
|
|||
fn substitute_variables<'cache>(
|
||||
&self,
|
||||
longhand_id: LonghandId,
|
||||
writing_mode: WritingMode,
|
||||
custom_properties: Option<<&Arc<crate::custom_properties::CustomPropertiesMap>>,
|
||||
quirks_mode: QuirksMode,
|
||||
device: &Device,
|
||||
|
@ -1719,43 +1739,18 @@ impl UnparsedValue {
|
|||
Some(shorthand) => shorthand,
|
||||
};
|
||||
|
||||
match shorthand {
|
||||
ShorthandId::All => {
|
||||
// No need to parse the 'all' shorthand as anything other
|
||||
// than a CSS-wide keyword, after variable substitution.
|
||||
return invalid_at_computed_value_time();
|
||||
}
|
||||
% for shorthand in data.shorthands_except_all():
|
||||
ShorthandId::${shorthand.camel_case} => {
|
||||
let longhands = match input.parse_entirely(|input| shorthands::${shorthand.ident}::parse_value(&context, input)) {
|
||||
Ok(l) => l,
|
||||
_ => return invalid_at_computed_value_time(),
|
||||
};
|
||||
let mut decls = SourcePropertyDeclaration::new();
|
||||
// parse_into takes care of doing `parse_entirely` for us.
|
||||
if shorthand.parse_into(&mut decls, &context, &mut input).is_err() {
|
||||
return invalid_at_computed_value_time();
|
||||
}
|
||||
|
||||
<% seen = set() %>
|
||||
% for property in shorthand.sub_properties:
|
||||
// When animating logical properties, we end up
|
||||
// physicalizing the value during the animation, but the
|
||||
// value still comes from the logical shorthand.
|
||||
//
|
||||
// So we need to handle the physical properties too.
|
||||
% for prop in property.all_physical_mapped_properties(data) + [property]:
|
||||
% if prop.ident not in seen:
|
||||
shorthand_cache.insert(
|
||||
(shorthand, LonghandId::${prop.camel_case}),
|
||||
PropertyDeclaration::${prop.camel_case}(
|
||||
longhands.${property.ident}
|
||||
% if prop.ident != property.ident:
|
||||
.clone()
|
||||
% endif
|
||||
)
|
||||
);
|
||||
% endif
|
||||
<% seen.add(prop.ident) %>
|
||||
% endfor
|
||||
% endfor
|
||||
for declaration in decls.declarations.drain(..) {
|
||||
let longhand = declaration.id().as_longhand().unwrap();
|
||||
if longhand.is_logical() {
|
||||
shorthand_cache.insert((shorthand, longhand.to_physical(writing_mode)), declaration.clone());
|
||||
}
|
||||
% endfor
|
||||
shorthand_cache.insert((shorthand, longhand), declaration);
|
||||
}
|
||||
|
||||
Cow::Borrowed(&shorthand_cache[&(shorthand, longhand_id)])
|
||||
|
@ -2469,7 +2464,7 @@ impl PropertyDeclaration {
|
|||
id,
|
||||
value: Arc::new(UnparsedValue {
|
||||
css: css.into_owned(),
|
||||
first_token_type: first_token_type,
|
||||
first_token_type,
|
||||
url_data: context.url_data.clone(),
|
||||
from_shorthand: None,
|
||||
}),
|
||||
|
@ -2506,7 +2501,7 @@ impl PropertyDeclaration {
|
|||
crate::custom_properties::parse_non_custom_with_var(input)?;
|
||||
let unparsed = Arc::new(UnparsedValue {
|
||||
css: css.into_owned(),
|
||||
first_token_type: first_token_type,
|
||||
first_token_type,
|
||||
url_data: context.url_data.clone(),
|
||||
from_shorthand: Some(id),
|
||||
});
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue