New issue
Advanced search Search tips

Issue 919875 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

The style_builder_template.tmpl generates syntactically wrong code for properties using 'empy' as template value

Project Member Reported by jfernan...@igalia.com, Jan 8

Issue description

Since #r562436 we use a new template value 'empty' to generate empty StyleBuilder functions.

I'm not sure since when this jinja code started to produce syntactically invalid code, or whether its caused by some missing/outdated dependencies in my own building environment (Linux Ubuntu 18.04). I'll try to figure that out as soon as possible.  


 
Cc: andruud@chromium.org futhark@chromium.org
I'm far from an expert on the jinja code generation area, but my guess is that the tpl is forcing an inline definition of the empty StyleBuilder functions, conflicting with the comment added instead of valid c++ code. 

The result is that the end block symbol is part of the commented code.

  // Style builder functions
  void ApplyInitial(StyleResolverState& state) const override {
    // Intentionally empty.  }

Components: Blink>CSS
This change seems to solve the problem, at least in my own environment:

https://chromium-review.googlesource.com/c/chromium/src/+/1401062
I tried that locally and it adds an empty line for the case where we have an implementation, which is unfortunate. I'm not familiar with jinja, but the generated code without your patch is correct for me.

I admit that the proposed patch may be not correct, but I thought it could be useful to illustrate where the problem comes from. 

Additionally, it's possible that the issue with code generation is caused by some dependency in my system; it's true that none of my colleagues were able to reproduce the issue. I thought that most of the dependencies were bundle into the chromium source, but perhaps there is something that this jinja based code generation requires from the system that leads to this annoying bug. 
BTW, I've just added @andruud and @futhark because they were the ones responsible for the template change that produces now the wrong generated code. I'd appreciate other names who could have more knowledge about jinja and the chromium code generation mechanism.
Status: Available (was: Untriaged)

Sign in to add a comment