-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add string macros #47
Conversation
Hmmmm, I think I'd prefer lowercase? |
I do not think these test failures are related to this change. |
Thank you for contributing! Glad to have another person interested in the package 😄 I'm just pretty much against macros where avoidable so I should probably defer to others on whether we add this or not (i'd tentatively be against, but it's not much code to maintain and perhaps some users like macros so 🤷) What's the benefit of the string macro in this case? I tend to think of macros as mostly providing nicer syntax. If we do add this, i agree with Jacob on preferring the lowercase spelling :) |
I prefer lowercase spelling. However, the major question is in what context do you expect to use these macros in practice. My experience is that these strings are useful if one works with collections of them (usually large collections). In such cases the key thing for performance is having uniform type of strings in the whole collection (I raise this point as this is not guaranteed by this macro - and cannot be). In summary - could you give some practical example when you think that having such macro would be handy? |
The other benefit of macros is that you can do work at macro-expansion-time instead of at runtime, which is another important benefit of the Regex example you provided. i.e. we compile the regex expression at macro-expansion-time once instead of every time we encounter the regex at runtime. In a similar vein, but as far as I can tell not implemented as such here, you could do the InlineString construction at macro-expansion-time instead of doing it at runtime. To do that, we'd want to do a full |
6f7bcbc
to
f286c80
Compare
I updated the macros so they are now lowercase and more is done while building the expression. |
@mkitti - my question about use-case was important (at least for me). Whether the design you propose is OK will depend on the intended usage scenarios. What are the situations where you assume you would use these macros? |
Inline string literalsThe way I view this is a way to create "inline string" literals that are interpreted as raw strings. Currently, one has to first construct a julia> using InlineStrings
julia> A = String15[]
String15[]
julia> push!(A, inline"Hello world"15)
1-element Vector{String15}:
"Hello world"
julia> push!(A, inline"Bye"15)
2-element Vector{String15}:
"Hello world"
"Bye"
julia> push!(A, inline"Hasta Luego"15)
3-element Vector{String15}:
"Hello world"
"Bye"
"Hasta Luego" One practical benefit of this is avoiding unnecessary allocation during construction. julia> @allocated inline"This is a string macro"
0
julia> @allocated InlineString("This is not a string macro")
48 Consider how the two forms differ in compilation: julia> f() = inline"This is a string. Hello!"
f (generic function with 1 method)
julia> g() = String31("This is a string. Hello!")
g (generic function with 1 method)
julia> @code_llvm f()
; @ REPL[30]:1 within `f`
; Function Attrs: uwtable
define void @julia_f_731(i256* noalias nocapture sret(i256) %0) #0 {
top:
store i256 38178759162904981154304547648621834361872905465358515510743786238036441825304, i256* %0, align 8
ret void
}
julia> @code_llvm g()
; @ REPL[31]:1 within `g`
; Function Attrs: uwtable
define void @julia_g_733(i256* noalias nocapture sret(i256) %0) #0 {
top:
%1 = alloca i256, align 8
call void @j_String31_735(i256* noalias nocapture nonnull sret(i256) %1, {}* inttoptr (i64 7549763600 to {}*)) #0
%2 = load i256, i256* %1, align 8
store i256 %2, i256* %0, align 8
ret void
} This is not merely superficial cosmetics. There are real practical and tangible implications of using these macros. HomoiconicityThe second consideration, not offered in this pull request, is changing how InlineStrings are displayed (as opposed to printed). At the moment inline strings are displayed exactly like their julia> String15("Hello world")
"Hello world" It would be clearer that the result is an inline string if this were displayed as if it was a literal. julia> String15("Hello world")
inline"Hello world"15 This would make the display of the inline string types homoiconic. Regular expression strings provide a precedence for this. julia> r"Hello"
r"Hello" |
On the display change idea, i wouldn't be in favour of changing the default "pretty printing" which shows inline strings the same as regular strings, but i'd support Perhaps we could discuss that idea further in an issue or on #52 to not distract from this PR :) |
This is true. Sorry for not being precise. My question was in what cases these tangible benefits matter. I.e. do we have use cases where these savings matter? It would have to be cases where there are millions of such literals (most likely such a literal is defined in a function that is called millions of times). Do they happen in practice? Thinking of this for example using a pattern:
And here
This is something that indeed I sometimes users want. However, if we went for this then I would feel that |
A practical use case is in situations where allocations are not allowed at all. For example, in the use of Here we're trying to enable applications which may be quite difficult with the current constructors.
In the case of display, it does mean 15 exactly. The limitation in this package is that only values of julia> using InlineStrings
julia> inline"abcdefg"11
"abcdefg"
julia> typeof(ans)
String15
julia> inline"hello"11
"hello"
julia> typeof(ans)
String15 |
Ah - now I have looked at implementation. Sorry for not checking this earlier. So you guarantee type stability here when you allow passing a number. What I would discuss though if it were not better to just define macros: See even at our discussion - when I was saying "exactly" I meant that for a single number passed it is always the same |
In that case, perhaps |
We could. Let us wait for others to comment as this is a matter of style. Thank you for submitting this PR. |
yeah, I agree on preferring the |
I made individual macros for julia> inline1"a"|> typeof
String1
julia> inline3"a"|> typeof
String3
julia> inline7"a"|> typeof
String7
julia> inline15"a"|> typeof
String15
julia> inline31"a"|> typeof
String31
julia> inline63"a"|> typeof
String63
julia> inline127"a"|> typeof
String127
julia> inline255"a"|> typeof
String255 |
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.
LGTM
can we drop the |
Is it weird that the canonical type is called |
Hmmm, yeah, it doesn't seem like this form is as useful now that we have
Yeah, we could consider naming these |
i also prefer Thanks again for the PR -- now that things are done at macro-expansion time and can help avoid allocations i think this is a really nice addition! |
src/InlineStrings.jl
Outdated
@@ -277,6 +295,28 @@ end | |||
InlineString(x::InlineString) = x | |||
InlineString(x::AbstractString)::InlineStringTypes = (InlineStringType(ncodeunits(x)))(x) | |||
|
|||
""" | |||
inline"string" | |||
inline"string"N |
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 would also not add inline"string"N
. If it is really needed we can add it later, but I do not feel it is useful given the other definitions we add.
src/InlineStrings.jl
Outdated
@@ -277,6 +295,28 @@ end | |||
InlineString(x::InlineString) = x | |||
InlineString(x::AbstractString)::InlineStringTypes = (InlineStringType(ncodeunits(x)))(x) | |||
|
|||
""" | |||
inline"string" | |||
inline"string"N |
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 would also not add inline"string"N
. If it is really needed we can add it later, but I do not feel it is useful given the other definitions we add.
The julia> x = 15
15
julia> quote
@inline_str "Hello World" $x
end
quote
#= REPL[35]:2 =#
inline"Hello World"15
end
julia> eval(ans)
"Hello World"
julia> typeof(ans)
String15 The only alternative I know of is to build the macro call expression: julia> x = 15
15
julia> e = Expr(:macrocall, Symbol("@inline$(x)_str"), LineNumberNode(0), "Hello world")
:(inline15"Hello world")
julia> eval(e)
"Hello world"
julia> typeof(ans)
String15 |
Yes, but the question is when would one need them in practice (as I have commented - these macros can be added later if they are needed). However, if you indeed need then now then I think they can be added. |
I suppose we should export all of the new macro forms. edit: Already done. |
This creates a string macro to aid in inline string construction.