[mycpp] translate % to StrFormat() #1430
Conversation
|
I don't understand the logic in |
mycpp/cppgen_pass.py
Outdated
| if isinstance(args[0], StrExpr): | ||
| fmt = 'StrFormat(%s' % escape_format_str(args[0].value) | ||
| else: | ||
| fmt = 'dynamic_fmt_dummy(' |
There was a problem hiding this comment.
Is this still necessary? I thought this is what we want to get rid of
mycpp/cppgen_pass.py
Outdated
| fmt_types = [self.types[arg] for arg in rest] | ||
| temp_name = self._WriteFmtFunc(fmt, fmt_types) | ||
| self.fmt_ids[o] = temp_name | ||
| fmt = escape_format_str(args[0].value) |
There was a problem hiding this comment.
Maybe pedantic, but how about naming this:
quoted_fmt = PythonStringLiteral(args[0].value)
? Logically that is the operation we want -- create a python string literal from a string
Digression (probably thinking about my latest Zulip posts :) --
For some reason I think of "escaping" as using an "escape character", although I realize using such prefixes is a common method of "quoting" (turning special chars / operator chars into literals)
e.g.
\nin C/JSON is an escape sequence, but I wouldn't call it quoting\\in C/JSON is technically quoting, equivalent to'\'in shell
I also think of & in HTML also as escaping
Or maybe this is a bogus distinction and they are synonyms :)
| @@ -47,6 +47,9 @@ def run_tests(): | |||
| x = 'x' | |||
| print("%s\tb\n%s\td\n" % (x, x)) | |||
|
|
|||
| fmt = "%d" | |||
| print(fmt % 10) | |||
There was a problem hiding this comment.
StrFormat() already has good coverage, but I would add a few more cases here because it's very easy and the assertions get checked "for free"
The test harness compares stdout of C++ and Python and then diffs them
that's ninja mycpp-logs-equal, which is part of mycpp/TEST.sh test-translator
Else we truncate strings whenever we encounter NUL.
|
Pushed a few changes. Have a look? Turns out adding support for arbitrary expressions as format strings was pretty easy. Just had to call |
| fputs("\n", stdout); | ||
| } | ||
|
|
||
| // Like print(..., file=sys.stderr), but Python code explicitly calls it. | ||
| void println_stderr(Str* s) { | ||
| fputs(s->data(), stderr); | ||
| for (int i = 0; i < len(s); ++i) { |
There was a problem hiding this comment.
Ah I see, I guess this was necessary to make tests pass with NUL bytes!
When we do the C I/O -> POSIX I/O changes, I think this will end up using man 2 write
write(0, s->data(), len(s))
write(0, "\n", 1)
|
Also one interesting thing is that it appears it's MyPy that's giving the compile error on Also it appears we let through i.e. there is a gap between what we support and what Python supports (and MyPy checks). I would like to tighten those things up to help contributors avoid errors in the future I started |
|
Ah right! You mentioned this on the other PR and I forgot. I'll send a PR to do some validation on the fmt strings |
|
I added Which can be run with A flaw with this testing method and mycpp is that it only allows ONE error per file It would be better if you could assert that the file has 5 errors, but unfortunately no |
|
Basically I want to eventually rewrite mycpp to get rid of MyPy, and hopefully the tests will guide us! :) |
|
Yeah I should have put in some validation in |
No description provided.