klauspost/asmfmt

Formatting fails with mixed comments

Kunde21 opened this issue · 3 comments

Kunde21 commented

When debugging I will mix block and line comments to quickly comment out sections of code, so I can move from this:

#include "textflag.h"

TEXT ·FailsFormatting(SB), NOSPLIT, $0
        /*/
        MOVQ fail_spectacularly+0(FP), SP //*/
        RET

to this, by deleting a single *:

#include "textflag.h"

TEXT ·FailsFormatting(SB), NOSPLIT, $0
        //
        MOVQ fail_spectacularly+0(FP), SP //*/
        RET

I'm getting a panic from asmfmt when it sees this pattern. I'm not sure the preferred action for the majority here, but I would like it to be left unchanged when it's within a line comment like this.

Minimal test:

#include "textflag.h"

TEXT ·FailsFormatting(SB), NOSPLIT, $0
        RET //*/

Panic output:

panic: runtime error: slice bounds out of range

goroutine 1 [running]:
panic(0x520100, 0xc82000e080)
       /src/runtime/panic.go:481 +0x3e6
github.com/klauspost/asmfmt.(*fstate).addLine(0xc82003baf8, 0xc82008b8c7, 0x14, 0x739, 0x0, 0x0)
        /src/github.com/klauspost/asmfmt/asmfmt.go:153 +0x1c1c
github.com/klauspost/asmfmt.Format(0x7f98752d5410, 0xc82004c070, 0x0, 0x0, 0x0, 0x0, 0x0)
        /src/github.com/klauspost/asmfmt/asmfmt.go:32 +0x327
main.processFile(0x7ffcf3bd2700, 0x13, 0x7f98752d53e8, 0xc820028028, 0x7f98752d53c0, 0xc820028010, 0x0, 0x0, 0x0)
        /src/github.com/klauspost/asmfmt/cmd/asmfmt/main.go:79 +0x248
main.gofmtMain()
        /src/github.com/klauspost/asmfmt/cmd/asmfmt/main.go:170 +0x763
main.main()
        /src/github.com/klauspost/asmfmt/cmd/asmfmt/main.go:130 +0x18
klauspost commented

Thanks for the report. I will investigate.

klauspost commented

Thanks again for the report. The fix is in #28, which will be merged shortly.

To facilitate you usage, I also made an adjustment to how block comments are handled - indentation is not longer changed for block comments. This should make it so your code will no longer jump around when you comment it out.

It should have no impact on existing code.

Kunde21 commented

Perfect. Thank you for your work on this, it's on my default-install list.