Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Command help - -fix flag is documented but doesn't do anything #6

Closed
joeycumines opened this issue May 16, 2023 · 2 comments
Closed

Comments

@joeycumines
Copy link

Cool tool. I found the fieldalignment tool unsuitable for general purpose use (e.g. as a core part of the build). So far, this resolves the qualms I had, so thanks.

I'm guessing there's a reason you made it -apply rather than -fix, but just thought I'd raise that -fix is also documented, but doesn't seem to do anything. I tried -fix first, for whatever reason.

$ betteralign -h
betteralign: find structs that would use less memory if their fields were sorted

Usage: betteralign [-flag] [package]

This analyzer find structs that can be rearranged to use less memory, and provides
a suggested edit with the most compact order.

Note that there are two different diagnostics reported. One checks struct size,
and the other reports "pointer bytes" used. Pointer bytes is how many bytes of the
object that the garbage collector has to potentially scan for pointers, for example:

        struct { uint32; string }

have 16 pointer bytes because the garbage collector has to scan up through the string's
inner pointer.

        struct { string; *uint32 }

has 24 pointer bytes because it has to scan further through the *uint32.

        struct { string; uint32 }

has 8 because it can stop immediately after the string pointer.

Be aware that the most compact order is not always the most efficient.
In rare cases it may cause two variables each updated by its own goroutine
to occupy the same CPU cache line, inducing a form of memory contention
known as "false sharing" that slows down both goroutines.


Flags:
  -V    print version and exit
  -all
        no effect (deprecated)
  -apply
        apply suggested fixes
  -c int
        display offending line with this many lines of context (default -1)
  -cpuprofile string
        write CPU profile to this file
  -debug string
        debug flags, any subset of "fpstv"
  -fix
        apply all suggested fixes
  -flags
        print analyzer flags in JSON
  -generated_files
        also check and fix generated files
  -json
        emit JSON output
  -memprofile string
        write memory profile to this file
  -source
        no effect (deprecated)
  -tags string
        no effect (deprecated)
  -test
        indicates whether test files should be analyzed, too (default true)
  -test_files
        also check and fix test files
  -trace string
        write trace log to this file
  -v    no effect (deprecated)
@dkorunic
Copy link
Owner

I'm glad that it works for you @joeycumines! Now, in regards why there is an -apply flag and not -fix, it is because Golang go/analysis/singlechecker and go/analysis/internal/checker imply by default some of the flags like -fix. However that specific functionality (as described the README) works by rewriting the specific AST nodes when SuggestedFixes is used. That behaviour cannot be overridden and since dealing with AST is not what we need due to AST shortcomings, -apply flag uses completely separate logic and deals with DST, printing the whole tree and rewriting files atomically.
Sadly -fix flag can't be easily removed from the flags.

@joeycumines
Copy link
Author

Gotcha, I'll just close this then @dkorunic, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants