Interpolated Strings #7988
Interpolated Strings #7988
Conversation
|
Thanks for your pull request and interest in making D better, @marler8997! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + dmd#7988" |
|
See also: #6703 |
|
Regarding syntax. I had a short look in this a couple of months ago and here's what other languages use:
More details: https://en.wikipedia.org/wiki/String_interpolation and https://rosettacode.org/wiki/String_interpolation_(included) |
marler8997
force-pushed
the
interpolatedStrings
branch
2 times, most recently
from
3a8e157 to
0678dc5
Compare
|
I think a big use case for interpolated strings in D is code generation for use with mixins. This makes me lean towards starting with an uncommon character like |
|
I really like this, but it seems a little strange to me that 'i' is a prefix while 'c', 'w', and 'd' are all suffixes (now that 'x' has been deprecated, I think 'i' being a prefix is a special case). EDIT: never mind, I forgot about 'r' for WYSIWYG strings. |
|
This needs to be done with a DIP. |
| { | ||
| $(body) | ||
| } | ||
| }); |
There was a problem hiding this comment.
Perhaps add tests with other string literal syntaxes as well.
There was a problem hiding this comment.
and prolly the suffixes. i"foo $(bar)"w should translate to ("foo "w, bar) or something; just repeat the suffixes an the generated string literals inside.
For Swift it's I like the following syntax:
|
marler8997
force-pushed
the
interpolatedStrings
branch
3 times, most recently
from
37a6f8f to
6aed8ce
Compare
src/dmd/astbase.d
Outdated
| @@ -4470,6 +4470,7 @@ struct ASTBase | |||
| size_t len; // number of code units | |||
| ubyte sz = 1; // 1: char, 2: wchar, 4: dchar | |||
| char postfix = 0; // 'c', 'w', 'd' | |||
| bool interpolate = false; // true if prefixed with 'i' | |||
There was a problem hiding this comment.
Please make this a Ddoc comment. I know the other ones are not, but we're trying to improve. I would also flesh out the comment a bit, something like: true if an interpolated string, i.e. prefixed with 'i'. Please also use backticks for code in comments.
There was a problem hiding this comment.
Should I also add ddoc comments to expression.d, or is that supposed to go away at some point?
There was a problem hiding this comment.
Hmm, that's a good question. I see that I added the comment to astbase.d. But I would say aim to add the comments in other files, i.e. not in astbase.d.
|
I can see arguments for and against supporting a shorthand escape syntax like The good thing is that we can always delay this decision as it is a simple addition that doesn't affect the mechanics of interpolated strings without it. |
marler8997
force-pushed
the
interpolatedStrings
branch
from
6aed8ce to
68c161a
Compare
test/d_do_test.d
Outdated
| @@ -497,8 +497,7 @@ string envGetRequired(in char[] name) | |||
| auto value = environment.get(name); | |||
| if(value is null) | |||
| { | |||
| writefln("Error: missing environment variable '%s', was this called this through the Makefile?", | |||
| name); | |||
| writeln(i"Error: missing environment variable '$(name)', was this called this through the Makefile?"); | |||
There was a problem hiding this comment.
NOTE TO REVIEWERS:
All changes in this file are just "dog fooding" interpolated strings.
Please, let's stop the syntax discussion and move this proposal into a DIP. Firstly, I really don't want you to do a ton on work on this and then have to change a lot or throw it away due to feedback. The DIP review fleshes-out potential pitfalls in design and implementation, and also involves more community opinions. Secondly, I'd like to see some justification for this addition, because I'm really not sold that this provides enough usefulness over [1] dlang/phobos#4318 if it's moved to the suggested alias parameters approach it can be merged. |
What do template instantiations use, the shorter syntax, i.e. |
marler8997
force-pushed
the
interpolatedStrings
branch
2 times, most recently
from
2b93840 to
c7588ce
Compare
A single token. Which would mean |
DIPs is where good work goes to die. This is a simple addition and if we can get it in without the red-tape/formality then we would be saving alot of people's time/effort to work on other things.
Telling me you don't want me to do a ton of work, and then saying you want me to do a DIP are two completely contradictory statements :)
Interpolated strings are pretty nice in some use cases. Code generation is a great example which makes it a perfect complement with D's mixin. Most other modern languages have also added support for string interpolation and being able to say that D has it as well is a huge plus, along with the fact that D's version has NO RUNTIME OVERHEAD making it better than everything else currently out there! I should also add that this addition was extremely simple and straightfoward. There wasn't any hard decisions to make. It uses existing feature like tuples/string literals making it a natural extension of what's already in D. If the change was not so straightforward I would want alot more discussion but this is not the case here. |
marler8997
force-pushed
the
interpolatedStrings
branch
from
c7588ce to
b84e71a
Compare
|
inTUPLEated strings!! :) |
That's what many people thought when they added #6589. Turns out even "simple" changes need more eyes on them. Let's not do that again. I understand why some people have lost faith in the DIP process, but Mike has been working with Walter and Andrei on the DIPs behind the scenes, I assure you. And it does help your case to already have an implementation ready. I'm not saying that this should be closed or anything. What I'm saying is we have a process in place to approve language additions in order to weigh the benefits and the costs and to work out any issues. Let's use that system to our advantage. |
|
If Walter/Andrei think a DIP is required then so be it. However, you're muddying the waters and getting in the way of developers by asserting they need to do more work than is currently being asked for. Work that arguably has little to no value and IMO overkill for this particular change. I don't mind you suggesting that a DIP could be useful, but just because you think something MUST have a DIP doesn't mean that it should. Your opinion is encouraged and welcome, but asserting your opinions on others derails the conversation and distracts from getting work done. Leave that decision up to Walter/Andrei. |
|
@WalterBright and @andralex, Can you please explicitly state if this requires a DIP or not? Marking this as "Need Approval" and assigning to @WalterBright and @andralex in an effort to invoke a decision. |
|
@Jash11 I hope to get progress on this at dconf. |
Codecov Report
@@ Coverage Diff @@
## master #7988 +/- ##
==========================================
- Coverage 85.56% 85.49% -0.08%
==========================================
Files 142 142
Lines 73624 73719 +95
==========================================
+ Hits 62996 63025 +29
- Misses 10628 10694 +66
Continue to review full report at Codecov.
|
2 similar comments
Codecov Report
@@ Coverage Diff @@
## master #7988 +/- ##
==========================================
- Coverage 85.56% 85.49% -0.08%
==========================================
Files 142 142
Lines 73624 73719 +95
==========================================
+ Hits 62996 63025 +29
- Misses 10628 10694 +66
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #7988 +/- ##
==========================================
- Coverage 85.56% 85.49% -0.08%
==========================================
Files 142 142
Lines 73624 73719 +95
==========================================
+ Hits 62996 63025 +29
- Misses 10628 10694 +66
Continue to review full report at Codecov.
|
|
@thewilsonator any progress? |
From DConf? Not that I heard of. |
|
According to Andrei @marler8997 was working on a DIP for this? Is this true @marler8997? Anyhow, I fully agree that we should just merge it under the new |
| @@ -684,6 +684,8 @@ dmd -cov -unittest myprog.d | |||
| "list all variables going into thread local storage"), | |||
| Feature("vmarkdown", "vmarkdown", | |||
| "list instances of Markdown replacements in Ddoc"), | |||
| Feature("interpolate", "interpolateStrings", | |||
| "enable interpolated string support via the 'i' prefix on string literals"), | |||
There was a problem hiding this comment.
Should be preview - not a transition - as this allows earlier merging.
| PERMUTE_ARGS: | ||
| REQUIRED_ARGS: -transition=interpolate | ||
| */ | ||
| import std.conv : text; |
There was a problem hiding this comment.
We should try to avoid using Phobos in the DMD testsuite. Maybe we can add a very reduced and minimalistic version of text?
|
This does need a DIP. Issues that need covering:
|
I listed a number of design issues what would not be helped along with the current state as a preview. They're not difficult to resolve, they just need attention. |
dlang/DIPs#140 the author seems to be missing but the DIP is there. |
A DIP would need a point of contact (usually but not necessarily the first author) stated in the header. If none is specified then it's the first author. |
Totally, I'm just linking for those who might not know it is there. |
Are you asking how this changes the language grammar? If so, it doesn't change it. It only changes the tokenization (i.e., allows the
My current solution is rather than inventing a syntax to do this inside interpolated strings is to use the existing features of the language. Here's an example of how that could be done: // put me in a library
auto fmt(T)(T value, string spec)
{
static struct Formatter
{
T value;
string spec;
void toString(scope void delegate(const(char)[]) sink)
{
import std.format : formattedWrite;
formattedWrite(sink, spec, value);
}
}
return Formatter(value, f);
}
writeln(`this is a formatted value $(myvalue.fmt("%-3x"))`);Maybe it would be a good idea to add a syntax, but I'm not sure. So I opted to leave it out for now since this is a feature that could always be added on later without breaking anything.
Outside the expressions, the interpolation parser ignores everything except the "... ", a, " ... %s ... ", c , b
This makes string interpolation less versatile and more difficult to use. Now it has to be parsed by a formatting function. Note that interpolation doesn't have to just be used to create strings, it's a pretty syntax to interleave string literals and expressions. This solution doesn't assume what the author will do with those pieces, it just breaks them down into their parts and let's the user decide what to do with them. This solution is more general while still allowing a string to be created with a single function call, and allowing them to be printed with a single function call (i.e. Furthermore, the // using this solution
writeln(i" ... $(a) ... ", i" ... $(b) ... ");
// becomes
writeln(" ... ", a, " ... ", " ... ", b, " .. ");
// using the '%s' solution
writefln(i" ... $(a) ... ", i" ... $(b) ... ");
// becomes
writefln(" ... %s ... ", a, " ... %s ... ", b);
// that doesn't work, so you'd have to do this
writeln(i" ... $(a) ... ".format, i" ... $(b) ... ".format);
I'm happy to add support so long as you can give me the grammar node you would like this to be ( |
|
@marler8997 thanks for the detailed response. I think this makes it clear exactly why a DIP is needed - the design needs to be complete and all in one document. You have a great start on the implementation, people can play with it as they like (I do suggest at least rebasing it so it's easier for people to try out), but I think it is more productive at the moment to work on the DIP. |
That needs to be updated here: https://dlang.org/spec/lex.html. |
|
I will write a DIP so long as @WalterBright and/or @andralex are willing to provide guidance and feedback at reasonable times or when requested. I'm am very glad that I have received such feedback on my current DIP (DIP1011) which gives me some confidence that this will be the case. However, I do worry because this goes against the guidelines of the DIP process. I have a pretty good idea of how I'd like interpolated strings to be implemented but there are pieces that I will need to know your preferences on (i.e. which grammar node to use for non-parenthetical expressions). So long as we are on the same page, I'll find some time soon to write the DIP and rebase/refresh this PR for experimentation. |
@marler8997 One key announcement from DConf AGM which I think Mike Parker will publicly announce on the D Blog soon, is that Andrei passing down his top-level leadership functions to Atila. You can watch this here for the full annoucement: https://www.youtube.com/watch?v=cpTAtiboIDs&t=3015s tl;dr: there will be a few big changes to the DIP process announced very soon. The biggest one is that you can ask Atila for guidance too. I don't know more than that, but I think more timely reviews and shorter DIP turnaround times are under discussion ;-) |
|
@marler8997 any progress on the DIP? |
|
No sorry. I'm still working on my current DIP, DIP 1011. Trying to finish my new changes to it before I take on another. |
|
Cleaning my old PRs that aren't going to be integrated to clean up my Pull Request list. |
Added interpolated strings. Syntax is to prefix any kind of string literal with the letter
i, i.e.Note that an interpolated string does not generate code to build a string at runtime, instead, lowers to a tuple.
A good way to think about an interpolated string is that it's just another way of creating a tuple. It's a nice way to write a tuple if the tuple is mostly made of raw characters (i.e. mixin code, HTML generation, etc). Generating a tuple gives full control to the application for determining what to do with it. To build a runtime string from an interpolated string, you can use the
textfunction, i.e.Here's an example where you can generate code using an interpolated
iq{ ... }string:This example demonstrates how interpolated strings enable using the
q{...}string literal for code generation even if the string itself contains curly braces. This wasn't possible before.A Note on Performance
Theoretically (though I haven't measured yet) this form of string should have very good compile-time and run-time performance when it comes to creating strings. Since it lowers to a tuple, it allows a print or string building function to instantiate the minimal code needed to build/write the string. It should have the same run-time performance as
format!"..."(...)but should also have MUCH better compile-time performance.To add a bit more detail, this PR performs string interpolation in the "parse stage". This means it is done after the string literal has been processed/escaped. This also means it is done before the semantic stage which makes it more performant than any library solution could be allowing it to be used with mixins or low-level code without having to give up performance or sacrifice more overhead. It also allows error messages to point directly to source code instead of pointing to code that is "mixed in" in the semantic stage.
Previous attempt: #6703