[assembler] Fix code comments without source position [v8/v8 : main]
0 views
Skip to first unread message
Clemens Backes (Gerrit)
Dec 16, 2025, 10:30:02 AM (12 days ago) Dec 16
to V8 LUCI CQ, Leszek Swirski, cbruni...@chromium.org, dmercadi...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Leszek Swirski
Clemens Backes added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 1 (Latest):
Clemens Backes . resolved
PTAL; found that when reading generated code and seeing lots of " - " suffixes.
Related details
Attention is currently required from:
- Leszek Swirski
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Leszek Swirski (Gerrit)
Dec 16, 2025, 10:38:43 AM (12 days ago) Dec 16
to Clemens Backes, V8 LUCI CQ, cbruni...@chromium.org, dmercadi...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Clemens Backes
Leszek Swirski voted and added 1 comment![]( "Open in Gerrit")
Votes added by Leszek Swirski
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Clemens Backes
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Clemens Backes (Gerrit)
Dec 16, 2025, 10:48:37 AM (12 days ago) Dec 16
to Leszek Swirski, V8 LUCI CQ, cbruni...@chromium.org, dmercadi...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Clemens Backes voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
Dec 16, 2025, 11:14:41 AM (12 days ago) Dec 16
to Clemens Backes, Leszek Swirski, cbruni...@chromium.org, dmercadi...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
V8 LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[assembler] Fix code comments without source position
The `FileName()` of an empty (default initialized) `SourceLocation` is
the empty string, but not nullptr.
To check if a filename is set, we would need to check if the first
character is '\0', or we check the line number instead, which is what
`SourceLocation::ToString` already did internally.
For convenience (to make it easier to use the right check the next
time), this introduces an `operator bool` on `v8::SourceLocation` to
check if the source location is initialized.
R=les...@chromium.org
Change-Id: I87286600eaa26dbec57919ea83ecefd05368b0b9
Reviewed-by: Leszek Swirski <les...@chromium.org>
Commit-Queue: Clemens Backes <clem...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#104351}
Files:
- M include/v8-source-location.h
- M src/codegen/assembler.h
- M src/codegen/code-stub-assembler.cc
- M src/compiler/code-assembler.h
- M src/compiler/turboshaft/assembler.h
- M test/unittests/heap/cppgc/source-location-unittest.cc
Change size: S
Delta: 6 files changed, 22 insertions(+), 18 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Leszek Swirski
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
Dec 18, 2025, 2:46:30 PM (10 days ago) Dec 18
to V8 LUCI CQ, Clemens Backes, Lei Zhang, Leszek Swirski, cbruni...@chromium.org, dmercadi...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Clemens Backes
Lei Zhang added 1 comment![]( "Open in Gerrit")
File include/v8-source-location.h
Line 76, Patchset 2 (Latest):
return (std::ostringstream{} << loc_.function_name() << '@'Lei Zhang . unresolved
Is it important to using ostringstream here? This CL transitively adds <sstream> to 15% of translation units when building the Linux chrome binary. Too bad there's no .cc file to stash the ToString() impl.
Related details
Attention is currently required from:
- Clemens Backes
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Leszek Swirski (Gerrit)
Dec 19, 2025, 4:30:58 AM (10 days ago) Dec 19
to V8 LUCI CQ, Clemens Backes, Lei Zhang, cbruni...@chromium.org, dmercadi...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Attention needed from Clemens Backes
Leszek Swirski added 1 comment![]( "Open in Gerrit")
File include/v8-source-location.h
Line 76, Patchset 2 (Latest):
return (std::ostringstream{} << loc_.function_name() << '@'Lei Zhang . unresolved
Is it important to using ostringstream here? This CL transitively adds <sstream> to 15% of translation units when building the Linux chrome binary. Too bad there's no .cc file to stash the ToString() impl.
Leszek Swirski
ToString could be moved to api.cc
Related details
Attention is currently required from:
- Clemens Backes
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Clemens Backes (Gerrit)
Dec 19, 2025, 4:34:30 AM (10 days ago) Dec 19
to V8 LUCI CQ, Lei Zhang, Leszek Swirski, cbruni...@chromium.org, dmercadi...@chromium.org, oilpan-r...@chromium.org, v8-re...@googlegroups.com
Clemens Backes added 1 comment![]( "Open in Gerrit")
File include/v8-source-location.h
Line 76, Patchset 2 (Latest):
return (std::ostringstream{} << loc_.function_name() << '@'Lei Zhang . resolved
Leszek SwirskiIs it important to using ostringstream here? This CL transitively adds <sstream> to 15% of translation units when building the Linux chrome binary. Too bad there's no .cc file to stash the ToString() impl.
ToString could be moved to api.cc
Clemens Backes
Ack, I just uploaded the CL: https://crrev.com/c/7277346
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages