nonempty broken for strings?

[CeylonMigrationBot]

[@FroMage] This could be a language module bug.

I just tried:

print("path part: " pathPart ", nonempty? " (nonempty pathPart) "");

And got:

path part: /path/to;param1;param2=foo/file.html, nonempty? false

Which is not normal is it? It's breaking the SDK tests at any rate.

[Migrated from ceylon/ceylon-compiler#887] [Closed at 2013-01-31 16:01:28]

[CeylonMigrationBot]

[@chochos] IIRC the change of Sequential means you can't use nonempty on String anymore. @gavinking xan clarify this.

[CeylonMigrationBot]

[@gavinking] @chochos Right, but I don't think there is any good reason why String can't be a subtype of Sequential. Is there?

[CeylonMigrationBot]

[@FroMage] If it can't work it should be a compile error then.

[CeylonMigrationBot]

[@gavinking] Well for now FixedLength and friends are still there, so I have not changed anything in the typechecker. For now I guess this should work. But we're proposing to remove FixedLength/None/Some.

[CeylonMigrationBot]

[@chochos] I guess String could/shpuld be a Sequential. And I think you mean FixedSized.

[CeylonMigrationBot]

[@chochos] And looking and this more closely now, I think that actually I do not remember correctly... I thought the typechecker gave an error when doing nonempty on a String, since it's supposed to work on Sequentials which String is not. So I don't know how the code in the issue description even compiled...

[CeylonMigrationBot]

[@FroMage] It does, it's in the SDK

[CeylonMigrationBot]

[@tombentley] This highlights that our tests for the nonempty operator must be completely inadequate, for us not to have noticed before we merged the branches.

[CeylonMigrationBot]

[@FroMage] So what are we going to do about that one?

[CeylonMigrationBot]

[@tombentley] Write some more tests, I suppose. And then fix the underlying problem.

[CeylonMigrationBot]

[@tombentley] The code compiled because a nonempty currently gets transformed to a instanceof Sequence.

That's incorrect given the old definition of nonempty in terms of FixedSized, but would appear to be right for the soon-to-be definition. So assuming we are actually going to get rid of FixedSized soon, then the fix would be to make SequenceString satisfy Sequence.

[CeylonMigrationBot]

[@tombentley] The above commit fixes this issue, but I think we should keep this open until the definition of nonempty catches up with the implementation.

[CeylonMigrationBot]

[@FroMage] Definition? Then this is now a typechecker spec bug no?

[CeylonMigrationBot]

[@quintesse] This seems to work fine now (provided the code is updated to the latest syntax of course), closing.