Ticket #728 (closed bug: fixed)

Opened 13 years ago

Last modified 13 years ago

r39273 breaks 'make test' for tcl

Reported by: coke Owned by: bacek
Priority: normal Milestone: 1.3
Component: none Version: trunk
Severity: medium Keywords: tcl blocker
Cc: Language:
Patch status: Platform:

Description

As of r39273, partcl's 'prove t/tcl_misc.t' fails some tests.

the transformations in src/grammar/expr/past2pir.tg begins to generate invalid code.

Change History

  Changed 13 years ago by coke

  • keywords tcl blocker added
  • milestone set to 1.3

  Changed 13 years ago by bacek

  • owner set to bacek

in reply to: ↑ description   Changed 13 years ago by bacek

Replying to coke:

As of r39273, partcl's 'prove t/tcl_misc.t' fails some tests. the transformations in src/grammar/expr/past2pir.tg begins to generate invalid code.

Ok... There is two versions which fix this ticket:

diff --git a/src/pmc.c b/src/pmc.c
index 506aa05..a81559a 100644
--- a/src/pmc.c
+++ b/src/pmc.c
@@ -258,9 +258,13 @@ Parrot_pmc_try_reuse(PARROT_INTERP, ARGIN(PMC *self), ARGIN
     if (dest->vtable->flags & VTABLE_IS_READONLY_FLAG)
         return pmc_new(interp, VTABLE_type(interp, self));
 
-    /* It's safe in this case. Caller want to replace us anyway */
+    /*
+       It's NOT safe in this case.
+       Caller want to replace us but calls to C<self> and C<dest> can be mixed.
+       See TT#728 for reference.
+    */
     if (dest == self)
-        return dest;
+        return pmc_new(interp, VTABLE_type(interp, self));
 
     /* Morph to self type is required */
     {

and

bacek@icering:~/src/parrot$ git diff src/pmc/
diff --git a/src/pmc/scalar.pmc b/src/pmc/scalar.pmc
index 70b591a..600b9f5 100644
--- a/src/pmc/scalar.pmc
+++ b/src/pmc/scalar.pmc
@@ -1069,7 +1069,7 @@ Concatenate the string C<value> in place.
         STRING * const s = Parrot_str_concat(INTERP,
             SELF.get_string(), value, 0);
 
-        dest = Parrot_pmc_try_reuse(INTERP, SELF, NULL, dest);
+        dest = pmc_new(INTERP, VTABLE_type(INTERP, SELF));
 
         VTABLE_set_string_native(INTERP, dest, s);
         return dest;

I can understand why former is better, but epically fail to understand why latter fix problem too...

  Changed 13 years ago by coke

While we try to figure out the best way to do what the original revision intended, backed it out in r39309

  Changed 13 years ago by bacek

  • status changed from new to closed
  • resolution set to fixed

Idea of reusing pmcs was discarded, related commits reverted. Resolving ticket.

Note: See TracTickets for help on using tickets.