[pmc] deprecate StringHandle auto-reopen and auto-read

deprecate PIO_VF_AWAYS_READABLE. (yes, a typo)
make StringHandle read/write mode, and auto-reopen strict.
StringHandle needs to be consistent with the FileHandle API.
I.e. reading from a handle open for writing needs to fail,
ditto writing to a handle open for reading. (already tested
and handled correctly)

Add testcase for read/write mode errors.

Add a seek(0,0) to various readall places on closed
write-only handles, and change those handles to "rw".

Closes GH #245.

api.yaml

@@ -714,7 +714,7 @@
   note:
     Only imageiofreeze still uses it.
     Set #define PARROT_BUFFERLIKE_LIST to use it
-    Will be removed with Release 8.0.0.
+    Will be removed with Release 8.3.0.
 
     Deprecated:
     * C<gc_sys->allocate_bufferlike_header>
@@ -726,3 +726,14 @@
     - deprecated
   ticket:  'https://github.com/parrot/parrot/issues/1179'
 -
+  name :  'stricter StringHandle PMC, no auto-reopen, auto-read'
+  note:
+    StringHandle auto-read and auto-reopen semantics is deprecated. It behaves
+    now as the FileHandle PMC.
+    Reading from a handle open for writing fails, readall() for a closed handle fails.
+    Either use the "rw" mode or the new set_mode() method, and the new seek(0, 0) method.
+    Will fail with Release 8.3.0.
+  tags:
+    - api
+    - deprecated
+  ticket : 'https://github.com/parrot/parrot/issues/245'

compilers/opsc/src/Ops/Emitter.pm

@@ -152,9 +152,9 @@ method emit_opsenum_h_body($fh) {
 method print_c_source_file() {
     # Build file in memeory
     my $fh := pir::new__Ps('StringHandle');
-    $fh.open('dummy.c', 'w');
+    $fh.open('dummy.c', 'rw');
     self.emit_c_source_file($fh);
-    $fh.close();
+    $fh.seek(0, 0);
 
     # ... and write it to disk
     my $final := pir::new__Ps('FileHandle');

ext/winxed/compiler.pir

@@ -25691,7 +25691,7 @@
     null $P1
     unless_null __ARG_3, __label_2
     new $P1, [ 'StringHandle' ]
-    $P1.'open'("__eval__", "w")
+    $P1.'open'("__eval__", "rw")
     goto __label_3
   __label_2: # else
     set $P1, __ARG_3
@@ -25711,7 +25711,7 @@
     .return(__ARG_3)
     goto __label_7
   __label_6: # else
-    $P1.'close'()
+    $P1.'seek'(0, 0)
     $P5 = $P1.'readall'()
     null $S1
     if_null $P5, __label_8
@@ -25749,7 +25749,7 @@
     null $P1
     unless_null __ARG_2, __label_1
     new $P1, [ 'StringHandle' ]
-    $P1.'open'("__eval__", "w")
+    $P1.'open'("__eval__", "rw")
     goto __label_2
   __label_1: # else
     set $P1, __ARG_2
@@ -25763,7 +25763,7 @@
     .return(__ARG_2)
     goto __label_4
   __label_3: # else
-    $P1.'close'()
+    $P1.'seek'(0, 0)
     .tailcall $P1.'readall'()
   __label_4: # endif
 

src/io/api.c

@@ -1,5 +1,5 @@
 /*
-Copyright (C) 2001-2014, Parrot Foundation.
+Copyright (C) 2001-2016, Parrot Foundation.
 
 =head1 NAME
 
@@ -1225,7 +1225,9 @@ Parrot_io_peek(PARROT_INTERP, ARGMOD(PMC *handle))
     {
         const IO_VTABLE * const vtable = IO_GET_VTABLE(interp, handle);
         IO_BUFFER * const read_buffer = IO_GET_READ_BUFFER(interp, handle);
-        const INTVAL c = Parrot_io_buffer_peek(interp, read_buffer, handle, vtable);
+        INTVAL c;
+        io_verify_is_open_for(interp, handle, vtable, PIO_F_READ);
+        c = Parrot_io_buffer_peek(interp, read_buffer, handle, vtable);
 
         if (c == -1)
             return STRINGNULL;

src/io/utilities.c

@@ -1,5 +1,5 @@
 /*
-Copyright (C) 2001-2012, Parrot Foundation.
+Copyright (C) 2001-2016, Parrot Foundation.
 
 =head1 NAME
 
@@ -159,8 +159,9 @@ C<flags>. C<flags> can be one of C<PIO_F_READ> or C<PIO_F_WRITE>. This
 function throws an exception if this handle is closed or if it is not open
 for the specified mode.
 
-There is an exception that certain types are flagged C<PIO_VF_ALWAYS_READABLE>
+There is an exception that certain types are flagged C<PIO_VF_AWAYS_READABLE>
 on the vtable. Those types are always readable, so take that into account.
+But this behavior for the StringHandle PMC was deprecated with 8.2.0.
 
 =cut
 
@@ -172,22 +173,32 @@ io_verify_is_open_for(PARROT_INTERP, ARGIN(PMC *handle),
 {
     ASSERT_ARGS(io_verify_is_open_for)
 
+    /* Deprecated with 8.2.0, removed with 8.3.0 */
     /* Some types like StringHandle are always readable, even if only opened
        in 'w' mode or when closed. Several parts of the build, test suite and
        libraries depend on this. */
-    if (vtable->flags & PIO_VF_AWAYS_READABLE) {
-        if (flags == PIO_F_READ)
-            return;
-        else
-            flags &= ~PIO_F_READ;
-    }
+    /* DONE: fixed those places: opsc, winxed, ... */
 
-    if (Parrot_io_is_closed(interp, handle))
+    if (Parrot_io_is_closed(interp, handle)) {
+#if PARROT_MAJOR_VERSION <= 8 || (PARROT_MAJOR_VERSION == 8 && PARROT_MINOR_VERSION < 3)
+        if (vtable->flags & PIO_VF_AWAYS_READABLE) {
+            Parrot_warn_deprecated(interp, "StringHandle auto-reopen is deprecated with 8.2.0");
+            return;
+        }
+#endif
         Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_PIO_ERROR,
                 "IO PMC %s is not open", vtable->name);
-    if ((vtable->get_flags(interp, handle) & flags) == 0)
+    }
+    if ((vtable->get_flags(interp, handle) & flags) == 0) {
+#if PARROT_MAJOR_VERSION <= 8 || (PARROT_MAJOR_VERSION == 8 && PARROT_MINOR_VERSION < 3)
+        if (vtable->flags & PIO_VF_AWAYS_READABLE) {
+            Parrot_warn_deprecated(interp, "StringHandle auto-read is deprecated with 8.2.0");
+            return;
+        }
         Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_PIO_ERROR,
                 "IO PMC %s is not in mode %d", vtable->name, flags);
+#endif
+    }
 }
 
 /*

src/pmc/stringhandle.pmc

@@ -182,6 +182,8 @@ Returns whether the StringHandle has reached the end of the buffer.
 Opens a string handle with the given mode. The filename is not used, but is
 stored for mocking.
 
+The mode defaults to "r".
+
 =cut
 
 */
@@ -203,6 +205,27 @@ stored for mocking.
         RETURN(PMC *handle);
     }
 
+/*
+
+=item C<METHOD set_mode(STRING *mode)>
+
+Changes the direction of the StringHandle, e.g. switching from
+read to write or vice-versa.
+
+This is faster then close and re-open, esp. since open ignores
+the filename argument.
+
+=cut
+
+*/
+
+    METHOD set_mode(STRING *mode) {
+        STRING *open_mode;
+        PMC * handle;
+
+        if (!STRING_IS_NULL(mode))
+            SET_ATTR_mode(INTERP, SELF, mode);
+    }
 
 /*
 

t/pmc/stringhandle.t

@@ -6,7 +6,7 @@ use warnings;
 use lib qw( . lib ../lib ../../lib );
 
 use Test::More;
-use Parrot::Test tests => 26;
+use Parrot::Test tests => 27;
 
 =head1 NAME
 
@@ -249,7 +249,7 @@ ok 5 - read integer back from file
 ok 6 - read string back from file
 OUT
 
-pir_output_is( <<'CODE', <<'OUT', 'print' );
+pir_output_is( <<'CODE', <<'OUT', 'print errors', todo => 'no errors until 8.3.0' );
 .include 'except_types.pasm'
 .sub 'test' :main
     .local pmc sh, eh
@@ -270,7 +270,7 @@ handle1:
 done1:
     say result
 
-    # print to SH opened for reading
+    # print to SH opened for reading throws an error
     result = 0
     set_label eh, handle2
     sh.'open'('mockname', 'r')
@@ -287,6 +287,44 @@ CODE
 0
 OUT
 
+pir_output_is( <<'CODE', <<'OUT', 'read/write mode errors', todo => 'no errors until 8.3.0' );
+.include 'except_types.pasm'
+.sub 'test' :main
+    .local pmc sh, eh
+    .local int result
+    sh = new ['StringHandle']
+    eh = new ['ExceptionHandler']
+    eh.'handle_types'(.EXCEPTION_PIO_ERROR)
+    push_eh eh
+
+    # read from SH not yet opened
+    result = 0
+    set_label eh, handle1
+    $S0 = sh.'read'(1)
+    result = 1
+    goto done1
+handle1:
+    finalize eh
+done1:
+    say result
+
+    # read from SH opened for writing throws an error
+    result = 0
+    set_label eh, handle2
+    sh.'open'('mockname', 'w')
+    result = sh.'peek'()
+    result = 1
+    goto done2
+handle2:
+    finalize eh
+done2:
+    say result
+.end
+CODE
+0
+0
+OUT
+
 # L<PDD22/I\/O PMC API/=item print.*=item readline>
 pir_output_is( <<"CODE", <<'OUT', 'readline - synchronous' );
 .sub 'test' :main
@@ -580,7 +618,7 @@ CODE
 ok 1 - $S0 = $P0.mode() # get read mode
 OUT
 
-pir_output_is( <<"CODE", <<"OUTPUT", "readall - closed stringhandle" );
+pir_output_is( <<"CODE", <<"OUTPUT", "readall an reopened handle" );
 .sub main :main
     \$S0 = <<"EOS"
 line 1
@@ -603,7 +641,7 @@ CODE
 ok
 OUTPUT
 
-pir_output_is( <<"CODE", <<"OUTPUT", "readall - stringhandle with null content" );
+pir_output_is( <<"CODE", <<"OUTPUT", "readall - flushed stringhandle errors", todo => 'no errors until 8.3.0' );
 .sub main :main
     .local pmc sh
     .local string s
@@ -613,16 +651,33 @@ pir_output_is( <<"CODE", <<"OUTPUT", "readall - stringhandle with null content"
     # and that is the case we are testing here.
     # Also, ensures coverage of the flush method.
     sh.'flush'()
+    push_eh E1
+    sh.'readall'()
+    s = "error1"
+E1:
+    pop_eh
+    print '['
+    print s
+    say ']'
+
+    sh.'open'('mockname', 'r')
+    sh.'seek'(0, 0)
+    push_eh E2
     s = sh.'readall'()
+    s = "error2"
+E2:
+    pop_eh
+
     print '['
     print s
     say ']'
 .end
 CODE
+[error1]
 []
 OUTPUT
 
-pir_output_is( <<"CODE", <<"OUTPUT", "readall() - opened stringhandle" );
+pir_output_is( <<"CODE", <<"OUTPUT", "readall() - rw stringhandle" );
 .sub main :main
     \$S0 = <<"EOS"
 line 1
@@ -631,11 +686,9 @@ line 3
 EOS
     .local pmc pio, pio2
     pio = new ['StringHandle']
-    pio.'open'("temp_file", "w")
+    pio.'open'("temp_file", "rw")
     pio.'print'(\$S0)
-    pio.'close'()
-
-    pio.'open'("temp_file", "r")
+    pio.'seek'(0, 0)
     \$S1 = pio.'readall'()
     if \$S0 == \$S1 goto ok
     print "not "
@@ -675,21 +728,30 @@ CODE
 0
 OUTPUT
 
-pir_output_is( <<"CODE", <<"OUTPUT", "readall() - utf8 on closed stringhandle" );
+pir_output_is( <<"CODE", <<"OUTPUT", "readall() - on closed stringhandle", todo => 'no errors until 8.3.0' );
+.include 'except_types.pasm'
 .sub 'main' :main
-    .local pmc ifh
+    .local pmc ifh, eh
+    .local int result
     ifh = new ['StringHandle']
     ifh.'encoding'('utf8')
 
-    \$S0 = ifh.'readall'('temp_file')
-
-    \$I0 = encoding \$S0
-    \$S1 = encodingname \$I0
+    eh = new ['ExceptionHandler']
+    eh.'handle_types'(.EXCEPTION_PIO_ERROR)
+    push_eh eh
+    result = 0
+    set_label eh, handle1
+    \$S0 = ifh.'readall'()
+    result = 1
+    goto done1
+handle1:
+    finalize eh
+done1:
 
-    say \$S1
+    say result
 .end
 CODE
-utf8
+0
 OUTPUT
 
 pir_output_is( <<"CODE", <<"OUTPUT", "readall() - utf8 on opened stringhandle" );