-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Create a LazyString type, for places (like errors) which we would prefer to defer the actual work #33711
Create a LazyString type, for places (like errors) which we would prefer to defer the actual work #33711
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
""" | ||
LazyString <: AbstractString | ||
|
||
A lazy representation of string interpolation. This is useful when a string | ||
needs to be constructed in a context where performing the actual interpolation | ||
and string construction is unnecessary or undesirable (e.g. in error paths | ||
of functions). | ||
|
||
This type is designed to be cheap to construct at runtime, trying to offload | ||
as much work as possible to either the macro or later printing operations. | ||
|
||
!!! compat "Julia 1.8" | ||
`LazyString` requires Julia 1.8 or later. | ||
""" | ||
mutable struct LazyString <: AbstractString | ||
parts::Tuple | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be better to allocate an Array here, so we do not need to create a whole new Tuple type and can examine and iterate it more easily later? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It can be be an immutable array once that's merged, but currently the analysis in #43852 is not strong enough to model the mutation of the mutable array at the moment. I think for the current use cases, a tuple is mostly fine, since the types will be known at compile time and are stable. Plus it's the error path anyway, which we heavily pessimize and in particular, don't infer, so it'll allocate tons of tuple types anyway. |
||
# Created on first access | ||
str::String | ||
LazyString(args...) = new(args) | ||
end | ||
|
||
""" | ||
lazy"str" | ||
|
||
Create a [`LazyString`](@ref) using regular string interpolation syntax. | ||
Note that interpolations are *evaluated* at LazyString construction time, | ||
but *printing* is delayed until the first access to the string. | ||
|
||
!!! compat "Julia 1.8" | ||
`lazy"str"` requires Julia 1.8 or later. | ||
""" | ||
macro lazy_str(text) | ||
parts = Any[] | ||
lastidx = idx = 1 | ||
while (idx = findnext('$', text, idx)) !== nothing | ||
lastidx < idx && push!(parts, text[lastidx:idx-1]) | ||
idx += 1 | ||
expr, idx = Meta.parseatom(text, idx; filename=string(__source__.file)) | ||
push!(parts, esc(expr)) | ||
lastidx = idx | ||
end | ||
lastidx <= lastindex(text) && push!(parts, text[lastidx:end]) | ||
:(LazyString($(parts...))) | ||
end | ||
|
||
function String(l::LazyString) | ||
if !isdefined(l, :str) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any worry about race conditions for code like this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. We can use an atomic acquire and cmpswap release here to fix that. (Or perhaps create an "assign_once" type, that manages this pattern for us?) |
||
l.str = sprint() do io | ||
for p in l.parts | ||
print(io, p) | ||
end | ||
end | ||
end | ||
return l.str | ||
end | ||
|
||
hash(s::LazyString, h::UInt64) = hash(String(s), h) | ||
lastindex(s::LazyString) = lastindex(String(s)) | ||
iterate(s::LazyString) = iterate(String(s)) | ||
iterate(s::LazyString, i::Integer) = iterate(String(s), i) | ||
isequal(a::LazyString, b::LazyString) = isequal(String(a), String(b)) | ||
==(a::LazyString, b::LazyString) = (String(a) == String(b)) | ||
ncodeunits(s::LazyString) = ncodeunits(String(s)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these cases be using the string macro for consistency? It seems you've included
strings/lazy.jl
before abstractarray.jl during bootstrap so I guess it should work?[edit] ah, I see that some uses of
string()
are just for keeping indentation under control. It sure would be nice if it was easier to reuse the parsing logic from #40753 so we had consistency of escaping rules for these kind of "almost but not quite a normal string" type of custom string literals. Rather than needing the raw string escaping rules.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just did the straightforward replacement. We can change these for consistency in a followup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Certainly fine by me.