Auto Clang-format for generated code?

[Ian Clelland]

Apologies if this is a stupid question that has been hashed out to death on the lists before, but would it make sense to have clang-format be the last step of the Blink bindings code generation process?

It seems like the current templates in Source/bindings/templates are heavily weighted towards readability of generated code, at the expense of readability of the source template, which sucks for developers trying to understand or update the templates themselves. As far as I see, it makes working on those templates more difficult, with the only benefit being that code in gen/ adheres to blink style (which clang-format would also ensure)

This ends up manifesting as contortions in template code around whitespace and blank lines in loops/conditionals, and as filters like indent(), and utility functions like generate_indented_conditional_code(), all of which wouldn't be needed if we could just auto-format the template output to match blink style. We'd end up with both readable files in gen/ *and* understandable source in bindings/templates.

It might mean that changes to the formatter would require simultaneously updating bindings/tests/results, but that doesn't seem too onerous. (or else change those tests to validate the pre-formatted code instead)

Have I missed any important issues? Or is this something we can/should do?

[Daniel Cheng]

IIRC, the main reason we didn't do it is because it was easier not to when we were doing the migration to clang-format --style=Chromium.

The main con I can think of is that clang-format doesn't always format data tables like a human would--and the generated bindings use table-based registration to set up the v8 templates. As long as we told clang-format to ignore those, I think the overall readability would improve.

Daniel

 

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAK_TSX%2BqCMxzfJrF0Kn526r3dUR8xXJYY-d17R%3DBVtJfpAnBGw%40mail.gmail.com.

[Jeremy Roman]

Or change the tests to format both the expected and actual results.

 

Have I missed any important issues? Or is this something we can/should do?

--

[Kentaro Hara]

Ian's point totally makes sense. We should run the clang-format at the final phase, which will enable us to clean up the template files significantly. We should :)

On Fri, May 26, 2017 at 3:52 AM, Daniel Cheng <dch...@chromium.org> wrote:

On Thu, May 25, 2017 at 10:46 AM Ian Clelland <icle...@chromium.org> wrote:

Apologies if this is a stupid question that has been hashed out to death on the lists before, but would it make sense to have clang-format be the last step of the Blink bindings code generation process?

It seems like the current templates in Source/bindings/templates are heavily weighted towards readability of generated code, at the expense of readability of the source template, which sucks for developers trying to understand or update the templates themselves. As far as I see, it makes working on those templates more difficult, with the only benefit being that code in gen/ adheres to blink style (which clang-format would also ensure)

This ends up manifesting as contortions in template code around whitespace and blank lines in loops/conditionals, and as filters like indent(), and utility functions like generate_indented_conditional_code(), all of which wouldn't be needed if we could just auto-format the template output to match blink style. We'd end up with both readable files in gen/ *and* understandable source in bindings/templates.

It might mean that changes to the formatter would require simultaneously updating bindings/tests/results, but that doesn't seem too onerous. (or else change those tests to validate the pre-formatted code instead)

Have I missed any important issues? Or is this something we can/should do?

IIRC, the main reason we didn't do it is because it was easier not to when we were doing the migration to clang-format --style=Chromium.

Yeah, that's the only reason. 

FWIW we have a poor C++ source formatter for the generated code to reduce the mess of the template files. We should replace it with clang-format and clean up the template files!

The main con I can think of is that clang-format doesn't always format data tables like a human would--and the generated bindings use table-based registration to set up the v8 templates. As long as we told clang-format to ignore those, I think the overall readability would improve.

Daniel

 

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAK_TSX%2BqCMxzfJrF0Kn526r3dUR8xXJYY-d17R%3DBVtJfpAnBGw%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAF3XrKqsXUeZ8K2PpU%2BLy789L6_XyxHXttmXpk1-sZpKP2XVJA%40mail.gmail.com.

--

Kentaro Hara, Tokyo, Japan

[Hitoshi Yoshida]

One reason we did not it was utilities.write_file(). It outputs a file iff the newly generated code is different from

the existing file content to avoid compiling the same file again.

Thus if we want to introduce clang-format at this point, we have to call clang-format as a Subprocess.

I think it is not so difficult, because clang-format works for standard I/O.

Beside it, I have a concern about it; When we get compile errors in generated code, won't it be difficult to find

the appropriate part of template files with reading the generated code?

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABg10jz5L3-BMxxC-BCHxutubC-X5sm-qW7T5HXpXi3d9BvB1A%40mail.gmail.com.

--

Hitoshi Yoshida (Peria)

email: pe...@chromium.org

[Yuta Kitamura]

On Sun, May 28, 2017 at 2:19 AM, Hitoshi Yoshida <pe...@chromium.org> wrote:

One reason we did not it was utilities.write_file(). It outputs a file iff the newly generated code is different from

the existing file content to avoid compiling the same file again.

Thus if we want to introduce clang-format at this point, we have to call clang-format as a Subprocess.

I think it is not so difficult, because clang-format works for standard I/O.

Beside it, I have a concern about it; When we get compile errors in generated code, won't it be difficult to find

the appropriate part of template files with reading the generated code?

Is it feasible to insert #line into the generated files?

[Daniel Cheng]

FWIW, having had to debug generated code in the past, I think the most useful thing might be to emit comments in the generated code that serve as breadcrumbs: that way, you can search for the comment string and see which template file it came from.

Daniel

[Mike Lawther]

+! this has always been my advice for generated code - please please please explain where the code was generated from. Be explicit about it - output a comment saying the file was generated from content in XYZ.in, or ABC.json. Even the commandline that generated the output would be useful.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAF3XrKrwwTgmPtx%2B%3DxWOaCWbNcod-Zw70jaHCMbF%3DvL17T6wwA%40mail.gmail.com.