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

cmd/compile: ambiguous TODO in ssa/html.go #41098

Closed
xiyichan opened this issue Aug 28, 2020 · 11 comments
Closed

cmd/compile: ambiguous TODO in ssa/html.go #41098

xiyichan opened this issue Aug 28, 2020 · 11 comments

Comments

@xiyichan
Copy link
Contributor

@xiyichan xiyichan commented Aug 28, 2020

What version of Go are you using (go version)?

$ go version
master

What did you do?

cmd/compile/internal/ssa/html.go:122

image

Use GOSSAFUNC to generate ssa.html.
This is the original state.
image

Is this hoping to be optimized like this?
image

Or other styles?

@randall77
Copy link
Contributor

@randall77 randall77 commented Aug 28, 2020

@ALTree
Copy link
Member

@ALTree ALTree commented Aug 28, 2020

The comment was introduced in afc480b.

@ALTree ALTree changed the title cmd/compile/:The TODO in the source code is ambiguous cmd/compile: ambiguous TODO in ssa/html.go Aug 28, 2020
@ALTree ALTree added this to the Unplanned milestone Aug 28, 2020
@xiyichan
Copy link
Contributor Author

@xiyichan xiyichan commented Aug 28, 2020

Maybe not the letters are rotated 90 degrees. Could it be that the title is rotated 90 degrees?

@josharian
Copy link
Contributor

@josharian josharian commented Aug 28, 2020

See https://go-review.googlesource.com/c/go/+/226209/comment/d6b6d7d5_a01a22b1/. TL;DR: It'd better to use sideways-lr instead of vertical-lr, but it's not widely enough available: https://caniuse.com/#search=sideways-lr

@xiyichan
Copy link
Contributor Author

@xiyichan xiyichan commented Aug 29, 2020

See https://go-review.googlesource.com/c/go/+/226209/comment/d6b6d7d5_a01a22b1/. TL;DR: It'd better to use sideways-lr instead of vertical-lr, but it's not widely enough available: https://caniuse.com/#search=sideways-lr

ok,I try to use sideways-lr.but only firefox support this.

@bradford-hamilton
Copy link
Contributor

@bradford-hamilton bradford-hamilton commented Aug 29, 2020

See https://go-review.googlesource.com/c/go/+/226209/comment/d6b6d7d5_a01a22b1/. TL;DR: It'd better to use sideways-lr instead of vertical-lr, but it's not widely enough available: https://caniuse.com/#search=sideways-lr

ok,I try to use sideways-lr.but only firefox support this.

Hey, I think I'm seeing the confusion and sorry this is my fault... The comment should actually say 180 degrees. The original look that we wanted with sideways-lr reads from bottom to top & left to right (if I'm remembering correctly). @xiyichan does that help some?

@xiyichan
Copy link
Contributor Author

@xiyichan xiyichan commented Aug 29, 2020

See https://go-review.googlesource.com/c/go/+/226209/comment/d6b6d7d5_a01a22b1/. TL;DR: It'd better to use sideways-lr instead of vertical-lr, but it's not widely enough available: https://caniuse.com/#search=sideways-lr

ok,I try to use sideways-lr.but only firefox support this.

Hey, I think I'm seeing the confusion and sorry this is my fault... The comment should actually say 180 degrees. The original look that we wanted with sideways-lr reads from bottom to top & left to right (if I'm remembering correctly). @xiyichan does that help some?

yes, It should be rotated 180 degrees.I try to use other css to achieve.

@xiyichan
Copy link
Contributor Author

@xiyichan xiyichan commented Aug 29, 2020

See https://go-review.googlesource.com/c/go/+/226209/comment/d6b6d7d5_a01a22b1/. TL;DR: It'd better to use sideways-lr instead of vertical-lr, but it's not widely enough available: https://caniuse.com/#search=sideways-lr

ok,I try to use sideways-lr.but only firefox support this.

Hey, I think I'm seeing the confusion and sorry this is my fault... The comment should actually say 180 degrees. The original look that we wanted with sideways-lr reads from bottom to top & left to right (if I'm remembering correctly). @xiyichan does that help some?

yes, It should be rotated 180 degrees.I try to use other css to achieve.

like this?
image

@bradford-hamilton
Copy link
Contributor

@bradford-hamilton bradford-hamilton commented Aug 29, 2020

See https://go-review.googlesource.com/c/go/+/226209/comment/d6b6d7d5_a01a22b1/. TL;DR: It'd better to use sideways-lr instead of vertical-lr, but it's not widely enough available: https://caniuse.com/#search=sideways-lr

ok,I try to use sideways-lr.but only firefox support this.

Hey, I think I'm seeing the confusion and sorry this is my fault... The comment should actually say 180 degrees. The original look that we wanted with sideways-lr reads from bottom to top & left to right (if I'm remembering correctly). @xiyichan does that help some?

yes, It should be rotated 180 degrees.I try to use other css to achieve.

like this?
image

@xiyichan yes actually I think this is exactly what we wanted!

@xiyichan
Copy link
Contributor Author

@xiyichan xiyichan commented Aug 30, 2020

See https://go-review.googlesource.com/c/go/+/226209/comment/d6b6d7d5_a01a22b1/. TL;DR: It'd better to use sideways-lr instead of vertical-lr, but it's not widely enough available: https://caniuse.com/#search=sideways-lr

ok,I try to use sideways-lr.but only firefox support this.

Hey, I think I'm seeing the confusion and sorry this is my fault... The comment should actually say 180 degrees. The original look that we wanted with sideways-lr reads from bottom to top & left to right (if I'm remembering correctly). @xiyichan does that help some?

yes, It should be rotated 180 degrees.I try to use other css to achieve.

like this?
image

@xiyichan yes actually I think this is exactly what we wanted!

@bradford-hamilton ok,I will submit a CL.

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 30, 2020

Change https://golang.org/cl/251657 mentions this issue: cmd/compile:rotate phase's title 180 degrees in ssa/html.go

@gopherbot gopherbot closed this in 0b1cec7 Sep 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.