Remove uses of the comma operator #1908
Conversation
| @@ -3322,7 +3322,7 @@ version(unittest) | |||
| import std.conv, std.range, std.algorithm; | |||
| //ensure constructor handles bad ordering and overlap | |||
| auto c1 = CodepointSet('а', 'я'+1, 'А','Я'+1); | |||
| foreach(ch; chain(iota('a', 'я'+1)), iota('А','Я'+1)) | |||
| foreach(ch; chain(iota('a', 'я'+1), iota('А','Я'+1))) | |||
There was a problem hiding this comment.
It's a good thing we have @blackwhale to write bad code to help us prove the dangers of comma operator or implicit string concatenation :)
End joke, he writes good code, abd also a lot of code :)
There was a problem hiding this comment.
It's a good thing we have @blackwhale to write bad code to help us prove the dangers of comma operator or implicit string concatenation :)
LOL. Much obliged ;)
|
The failures in the tester are bizarre. |
|
@yebblies: Any idea what's causing the failure? |
|
Nope. I guess I'll binary-search it. |
| : fmt.trailing; | ||
| if (sep) | ||
| fmt.readUpToNextSpec(input); |
There was a problem hiding this comment.
This change is causing the failures...
There was a problem hiding this comment.
Maybe readUpToNextSpec can change sep? IDK, but it seems to work with the above the condexp.
|
@yebblies: Seems like maybe std.uni's unittest was bad to begin with? You could just comment that out and file a separate bug so this pull request isn't stalled. |
|
This isn't very high on my priority list right now, I'll get around to it eventually. |
|
@blackwhale: Ping. Any idea about the failure? |
|
@klickverbot I'll investigate. |
|
This code passes with or without this pull: auto c1 = CodepointSet('А','Я'+1, 'а', 'я'+1);
assert(c1.byCodepoint.equal(chain(
iota('А','Я'+1), iota('а', 'я'+1))));However I get a cascade of failures including bad seed in std.random when unittesting phobos on Win32. The havoc ends with a segfault.somewhere. |
The issue is that the test should use the 'CYRILLIC SMALL LETTER A' (1072; 0x0430). http://www.fileformat.info/info/unicode/char/0430/index.htm But currently, the code has 'LATIN SMALL LETTER A' (97; 0x61). That's the issue. |
|
@monarchdodra Good catch :) |
@blackwhale : Thanks. I would have caught it sooner had I known 'a' was the first Cyrillic letter. I was observing the unittest going haywire, but couldn't make any sense out of what I was observing, or even what you were trying to test. Maybe hiragana would make a better candidate for testing? Less ambiguity. Well, I figure you just used whatever was in the bug report. In any case, while tracking down the issue, I wished for this feature: |
|
@blackwhale @monarchdodra : Could one of you make a pull with the fix so #1908 isn't blocked? |
@AndrejMitrovic : Done : #1951 . @yebblies will still have to rebase, but at least it's fixed. Hum... Or, when I get back home, I can push the fix straight into your branch instead? |
'LATIN SMALL LETTER A' => 'CYRILLIC SMALL LETTER A'
|
Auto-merge toggled on |
Remove uses of the comma operator
No description provided.