Wasmtime GitHub notifications bot (Jan 10 2022 at 11:25): Wasmtime GitHub notifications bot (Jan 10 2022 at 11:25):bjorn3 opened issue #3674:
They make the diffs to those files much bigger than necessary as line numbers often change, thus making it harder to review the generated files. It also makes git pack files slightly bigger. As an alternative to line numbers maybe fragments of the source isle files can be included that can be searched for in the source files?
Wasmtime GitHub notifications bot (Jan 10 2022 at 19:43):fitzgen commented on issue #3674:
I think the line numbers are useful when looking at the generated code and understanding why things are there and what source they correspond to.
I think including ISLE source fragments is a could improvement, but still doesn't obviate the need for line numbers, since it doesn't help me go back to the canonical source location if I have edits to make.
We could potentially add a
-diffline to.gitattributesso that diffs for generated files are suppressed.
Wasmtime GitHub notifications bot (Jan 10 2022 at 19:47):bjorn3 commented on issue #3674:
We could potentially add a -diff line to .gitattributes so that diffs for generated files are suppressed.
I would like to still have diffs for the generated code to see what it is exactly doing. That is why I want the diffs for them to be less noisy. Github currently hides the diffs anyway due to the size.
Wasmtime GitHub notifications bot (Jan 10 2022 at 22:09):cfallin commented on issue #3674:
@bjorn3 I'm curious, are you looking at the diffs of generated code in PRs that add e.g. instruction lowerings in order to understand what new functionality is included?
In general, I would hope that one doesn't need to resort to that, in the sense that the diffs at the ISLE DSL level should be clear enough; if the only way to tell what changed is to look at the generated code, then that's an issue with language or prelude design, IMHO.
Also, we can't necessarily guarantee that the output will be "stable" with incremental updates: perhaps sometime in the future, we have a heuristic that generated lowering code, or chooses a different strategy, when something changes on the input side.
In other words, I'd prefer to optimize for understandability of the generated code at a point in time -- hence the line number comments, so e.g. someone following with a debugger can get back to ISLE source -- rather than "incremental-update diff cleanliness". Does that make sense? Understanding what you're hoping to get from the diffs could of course help us refine this.
Wasmtime GitHub notifications bot (Jan 10 2022 at 22:10):cfallin edited a comment on issue #3674:
@bjorn3 I'm curious, are you looking at the diffs of generated code in PRs that add e.g. instruction lowerings in order to understand what new functionality is included?
In general, I would hope that one doesn't need to resort to that, in the sense that the diffs at the ISLE DSL level should be clear enough; if the only way to tell what changed is to look at the generated code, then that's an issue with language or prelude design, IMHO.
Also, we can't necessarily guarantee that the output will be "stable" with incremental updates: perhaps sometime in the future, we have a heuristic that generates very lowering code by choosing a different strategy when something changes on the input side.
In other words, I'd prefer to optimize for understandability of the generated code at a point in time -- hence the line number comments, so e.g. someone following with a debugger can get back to ISLE source -- rather than "incremental-update diff cleanliness". Does that make sense? Understanding what you're hoping to get from the diffs could of course help us refine this.
Wasmtime GitHub notifications bot (Jan 10 2022 at 22:10):cfallin edited a comment on issue #3674:
@bjorn3 I'm curious, are you looking at the diffs of generated code in PRs that add e.g. instruction lowerings in order to understand what new functionality is included?
In general, I would hope that one doesn't need to resort to that, in the sense that the diffs at the ISLE DSL level should be clear enough; if the only way to tell what changed is to look at the generated code, then that's an issue with language or prelude design, IMHO.
Also, we can't necessarily guarantee that the output will be "stable" with incremental updates: perhaps sometime in the future, we have a heuristic that generates very different lowering code by choosing a different strategy when something changes on the input side.
In other words, I'd prefer to optimize for understandability of the generated code at a point in time -- hence the line number comments, so e.g. someone following with a debugger can get back to ISLE source -- rather than "incremental-update diff cleanliness". Does that make sense? Understanding what you're hoping to get from the diffs could of course help us refine this.
Wasmtime GitHub notifications bot (May 19 2022 at 16:15):alexcrichton commented on issue #3674:
ISLE-generated code is no longer checked in, so I'm going to close this.
Wasmtime GitHub notifications bot (May 19 2022 at 16:15):alexcrichton closed issue #3674:
They make the diffs to those files much bigger than necessary as line numbers often change, thus making it harder to review the generated files. It also makes git pack files slightly bigger. As an alternative to line numbers maybe fragments of the source isle files can be included that can be searched for in the source files?
Last updated: Oct 23 2024 at 20:03 UTC