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

x/sys/unix: consolidate duplicate information #33059

Open
ianlancetaylor opened this issue Jul 11, 2019 · 3 comments

Comments

@ianlancetaylor
Copy link
Contributor

commented Jul 11, 2019

In the current golang.org/x/sys/unix package, there is a lot of duplication, especially in the GNU/Linux files. We currently support 13 different GOARCH values, and more are possible. If I diff, say, zerrors_linux_amd64.go and zerrors_linux_mips.go, I see that a bit more than 500 lines are different, but the files are just over 3000 lines long. That suggests that we are repeating 2500 lines of information 13 times.

This doesn't really matter all that much, but it does make the package somewhat larger than necessary and makes it slightly harder to review CLs. Since we generate the information automatically anyhow, I propose that we add another step: run a merge step on all the files for a given GOOS. For all constants, types, and functions that are defined precisely identically for each GOARCH, move them into a single unified file named simply zerrors_linux.go. Similarly for zsyscall, zsysnum, and ztypes, of course. It should be straightforward to write the merge program based using go/ast. It doesn't have to be smart: just merge definitions that are exactly identical. If in the future the values are different on some specific GOARCH, the automatic generation should automatically leave the definitions back in the GOARCH-specific files.

CC @tklauser @paulzhol @mdlayher @josephlr

@tklauser

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

This sounds like a very good idea to me!

While adding this post-processing step using go/ast we could also port the regexp-based post-processing steps in mkpost.go to use go/ast (as has been proposed multiple times in CL reviews already).

@pierrec

This comment has been minimized.

Copy link

commented Jul 13, 2019

Hello,

I am having a "go" at this, since I have been using the go/ast recently and would like to finally contribute to the project.

I have a few questions as I have started looking into this.

  1. You suggest using .go as the file name holding the factored out consts/types/funcs but there already are such files (e.g. syscall_linux.go exists as well as syscall_linux.go). Or should only the z.go files be considered?

  2. Is preserving the blocks of consts important or could they be all merged?
    Merging them make the implementation (much?) easier.

Thanks.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor Author

commented Jul 13, 2019

We should only be looking at merging information in the generated files, whose names all start with 'z'. The merged file names should also start with 'z'. We currently have 13 files zerrors_linux_GOARCH.go. I'm suggesting that the common aspects be merged into zerrors_linux.go.

We shouldn't touch the non-generated files, at least not with an automated process.

In the generated files it is not important to preserve blocks of constants.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.