google / xls

XLS: Accelerated HW Synthesis
http://google.github.io/xls/
Apache License 2.0
1.21k stars 179 forks source link

dslx-fmt::off in for loop header deletes the parent proc #1735

Open mikex-oss opened 2 days ago

mikex-oss commented 2 days ago

Describe the bug I tried to disable formatting of a long for loop accumulator type annotation, due to https://github.com/google/xls/issues/1685 making the loop hard to read.

It deleted my entire proc.

To Reproduce Run the following through the formatter:

struct FooType { a: u32, b: u32 }

struct BarType { c: u32, d: u32 }

const NUM_ELEMS = u32:8;
const NUM_BLOCKS = u32:2;

proc A {
    config() {  }

    init {  }

    next(_: ()) {
        // some comment
        let _some_import_code_here = true;

        let (_foo, _bar, _baz) =
            for (i, (foo, bar, baz)): (
                // dslx-fmt::off
                u32, (FooType[NUM_ELEMS][NUM_BLOCKS], BarType[NUM_ELEMS][NUM_BLOCKS], 
                      bool[NUM_ELEMS][NUM_BLOCKS])
                ) in range(u32:0, 8) {
                // dslx-fmt::on

                // this is another cool comment
                (foo, bar, baz)
            }((zero!<FooType[8][2]>(), zero!<BarType[8][2]>(), all_ones!<bool[8][2]>()));

        // the end
        let _my_grand_finale_here = true;
    }
}

Output:

struct FooType { a: u32, b: u32 }

struct BarType { c: u32, d: u32 }

const NUM_ELEMS = u32:8;
const NUM_BLOCKS = u32:2;

// some comment

// dslx-fmt::off

Expected behavior Source outside the disabled formatting section should be untouched. All the code should still be present.

dplassgit commented 1 day ago

This will no longer be allowed (and the user will be notified!)

It's still broken, but after I fix it, you will be able to write:

let (_foo, _bar, _baz) =
            for (i, (foo, bar, baz)): (
                // dslx-fmt::off
                u32, (FooType[NUM_ELEMS][NUM_BLOCKS], BarType[NUM_ELEMS][NUM_BLOCKS], 
                      bool[NUM_ELEMS][NUM_BLOCKS])
                // dslx-fmt::on
                ) in range(u32:0, 8) {

                // this is another cool comment
                (foo, bar, baz)
            }((zero!<FooType[8][2]>(), zero!<BarType[8][2]>(), all_ones!<bool[8][2]>()));