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.

Sign up for GitHub

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

Jump to bottom

cmd/internal/obj/arm64: reduce verbosity of code in /obj/arm64/asm7.go #22576

Closed
xyproto opened this issue Nov 4, 2017 · 4 comments
Closed

cmd/internal/obj/arm64: reduce verbosity of code in /obj/arm64/asm7.go #22576

xyproto opened this issue Nov 4, 2017 · 4 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Unplanned

Comments

@xyproto
Copy link
Contributor

xyproto commented Nov 4, 2017 •
edited

Hi,

This is my first attempt at contributing to the Go source code, I am very open to suggestions to improving my process.

I submitted a patch to gerrit and was asked by Brad Fitzpatrick (who kindly reviewed the code) to create a corresponding bug.

This is the issue that I believed can be improved by my patch:

  • asm7.go can be reduced with 38 lines of code, with increased clarity of code and the exact same behavior.

Here's the change: 75690
@ALTree ALTree changed the title cmd/compile: reduce verbosity of code in src/cmd/internal/obj/arm64/asm7.go cmd/internal/obj/arm64: reduce verbosity of code in /obj/arm64/asm7.go Nov 4, 2017
@ALTree
Copy link
Member

ALTree commented Nov 4, 2017

I've edited the title of the issue because based on your comments on the CL

the intention is to increase the readability [of the code in asm7.go]

this is just about deciding whether it's worth changing the code to make it more readable, and you are not reporting compiler bug(?)

@ALTree ALTree added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Nov 4, 2017
@xyproto
Copy link
Contributor Author

xyproto commented Nov 4, 2017

@ALTree Thank you. That is correct, I do not consider this to be a compiler bug, only an improvement in readability and line count (every line of code has a certain percentagewise chance for bugs, so in theory, fewer lines = fewer bugs, right?).

@bradfitz
Copy link
Contributor

bradfitz commented Nov 6, 2017

so in theory, fewer lines = fewer bugs, right?

No, many of us would argue strongly that that assumption is not true. For example, 5 tricky lines are more likely to hide a bug than 10 boring lines.

That said, your change might be an improvement, but I'll let others decide. I just wanted to clarify that reducing the number of lines is not a goal in and of itself.

@bradfitz bradfitz added this to the Go1.11 milestone Nov 6, 2017
@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Unplanned Jun 14, 2018
@ALTree
Copy link
Member

ALTree commented Feb 3, 2020

Decision seems to be: let's leave that code alone; OP has abandoned the proposed CL, so we can close here.

@ALTree ALTree closed this as completed Feb 3, 2020
@golang golang locked and limited conversation to collaborators Feb 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@bradfitz @xyproto @ianlancetaylor @ALTree @gopherbot