Bug #10235: Wrong variable used on CATCH block - Conversion Tools - Golden Code Redmine

Bug #10235

« Previous | Next »

Wrong variable used on CATCH block

Added by Teodor Gorghe about 1 year ago. Updated about 1 year ago.

Status:

Test

Priority:

Normal

Assignee:

Teodor Gorghe

Target version:

Start date:

Due date:

% Done:

100%

billable:

vendor_id:

GCD

case_num:

version_reported:

version_resolved:

reviewer:

Constantin Asofiei

production:

env_name:

topics:

History

#2 Updated by Teodor Gorghe about 1 year ago

There is a issue when there are multiple CATCH blocks. On FWD, the variable reference for the first block is from the second block.

tests/error_handling/TestCatchVariable.cls

USING OpenEdge.Core.Assert FROM PROPATH.

BLOCK-LEVEL ON ERROR UNDO, THROW.

CLASS tests.error_handling.TestCatchVariable:
    DEFINE PUBLIC STATIC PROPERTY DummyProperty AS INTEGER INITIAL 0 NO-UNDO
    GET():
        IF DummyProperty = 0 THEN
        DO ON ERROR UNDO, THROW:
            UNDO, THROW NEW Progress.Lang.AppError().

            CATCH aError AS Progress.Lang.Error :
                Assert:IsTrue(VALID-OBJECT(aError)).
                DummyProperty = 1.
            END CATCH.
        END.
        ELSE DO ON ERROR UNDO, THROW:
            UNDO, THROW NEW Progress.Lang.AppError().

            CATCH aError AS Progress.Lang.Error :
                Assert:IsTrue(VALID-OBJECT(aError)).
                DummyProperty = 0.
            END CATCH.
        END.

        RETURN DummyProperty.
    END GET.
    PRIVATE SET.

    @Test.
    METHOD PUBLIC VOID TestPropertyCatchVarInstance ():
        Assert:Equals(1, DummyProperty).
        Assert:Equals(0, DummyProperty).
    END METHOD.
END CLASS.

Converted code:

...
public class TestCatchVariable
extends BaseObject
{
   ...
   @LegacySignature(type = Type.GETTER, name = "DummyProperty", returns = "INTEGER")
   public static integer getDummyProperty()
   {
      ObjectOps.load(TestCatchVariable.class);
      object<? extends com.goldencode.p2j.oo.lang.LegacyError> aError_top = TypeFactory.object(com.goldencode.p2j.oo.lang.LegacyError.class);
      object<? extends com.goldencode.p2j.oo.lang.LegacyError> aError_top_1 = TypeFactory.object(com.goldencode.p2j.oo.lang.LegacyError.class);

      return function(TestCatchVariable.class, "DummyProperty", integer.class, new Block((Body) () -> 
      {
         if (_isEqual(dummyProperty.get(), 0))
         {
            OnPhrase[] onPhrase0 = new OnPhrase[]
            {
               new OnPhrase(Condition.ERROR, Action.THROW, "blockLabel0")
            };

            doBlock(TransactionType.SUB, "blockLabel0", onPhrase0, new Block((Init) () -> 
            {
               catchError(com.goldencode.p2j.oo.lang.LegacyError.class, 
               aError -> 
               {
                  aError_top.assign(aError);
                  Assert.isTrue(aError_top_1.isValid()); // !!! The issue is here. Instead of "aError_top", "aError_top_1" is used.
                  setDummyProperty(new integer(1));
               });
            }, 
            (Body) () -> 
            {
               {
                  undoThrow(ObjectOps.newInstance(com.goldencode.p2j.oo.lang.AppError.class));
               }
            }));
         }
         else
         {
            OnPhrase[] onPhrase1 = new OnPhrase[]
            {
               new OnPhrase(Condition.ERROR, Action.THROW, "blockLabel1")
            };

            doBlock(TransactionType.SUB, "blockLabel1", onPhrase1, new Block((Init) () -> 
            {
               catchError(com.goldencode.p2j.oo.lang.LegacyError.class, 
               aError_1 -> 
               {
                  aError_top_1.assign(aError_1);
                  Assert.isTrue(aError_1.isValid());
                  setDummyProperty(new integer(0));
               });
            }, 
            (Body) () -> 
            {
               {
                  undoThrow(ObjectOps.newInstance(com.goldencode.p2j.oo.lang.AppError.class));
               }
            }));
         }

         storeReturnValue(dummyProperty.get());
      }));
   }
   ...
}

#3 Updated by Teodor Gorghe about 1 year ago

Status changed from New to WIP
Assignee set to Teodor Gorghe

When using a DO ON CATCH block on a method, this issue does not occur.

ABL code

    @Test.
    METHOD PUBLIC VOID TestMethodCatchVarInstance ():
        DEFINE VARIABLE iValue AS INTEGER NO-UNDO INITIAL 0.
        DO ON ERROR UNDO, THROW:
            UNDO, THROW NEW Progress.Lang.AppError().

            CATCH anotherError AS Progress.Lang.Error :
                Assert:IsTrue(VALID-OBJECT(anotherError)).
                iValue = 1.
            END CATCH.
        END.
        Assert:Equals(1, iValue).
        DO ON ERROR UNDO, THROW:
            UNDO, THROW NEW Progress.Lang.AppError().

            CATCH anotherError AS Progress.Lang.Error :
                Assert:IsTrue(VALID-OBJECT(anotherError)).
                iValue = 0.
            END CATCH.
        END.
        Assert:Equals(0, iValue).
    END METHOD.

Converted Code

   @LegacySignature(type = Type.METHOD, name = "TestMethodCatchVarInstance")
   @Test(order = 1)
   public void testMethodCatchVarInstance()
   {
      integer iValue = TypeFactory.integer((long) 0);
      object<? extends com.goldencode.p2j.oo.lang.LegacyError> anotherError_top = TypeFactory.object(com.goldencode.p2j.oo.lang.LegacyError.class);
      object<? extends com.goldencode.p2j.oo.lang.LegacyError> anotherError_top_1 = TypeFactory.object(com.goldencode.p2j.oo.lang.LegacyError.class);

      internalProcedure(TestCatchVariable.class, this, "TestMethodCatchVarInstance", new Block((Body) () -> 
      {
         OnPhrase[] onPhrase2 = new OnPhrase[]
         {
            new OnPhrase(Condition.ERROR, Action.THROW, "blockLabel2")
         };

         doBlock(TransactionType.SUB, "blockLabel2", onPhrase2, new Block((Init) () -> 
         {
            catchError(com.goldencode.p2j.oo.lang.LegacyError.class, 
            anotherError -> 
            {
               anotherError_top.assign(anotherError);
               Assert.isTrue(anotherError.isValid());
               iValue.assign(1);
            });
         }, 
         (Body) () -> 
         {
            {
               undoThrow(ObjectOps.newInstance(com.goldencode.p2j.oo.lang.AppError.class));
            }
         }));

         Assert.legacyEquals(new integer(1), iValue);

         OnPhrase[] onPhrase3 = new OnPhrase[]
         {
            new OnPhrase(Condition.ERROR, Action.THROW, "blockLabel3")
         };

         doBlock(TransactionType.SUB, "blockLabel3", onPhrase3, new Block((Init) () -> 
         {
            catchError(com.goldencode.p2j.oo.lang.LegacyError.class, 
            anotherError_1 -> 
            {
               anotherError_top_1.assign(anotherError_1);
               Assert.isTrue(anotherError_1.isValid());
               iValue.assign(0);
            });
         }, 
         (Body) () -> 
         {
            {
               undoThrow(ObjectOps.newInstance(com.goldencode.p2j.oo.lang.AppError.class));
            }
         }));

         Assert.legacyEquals(new integer(0), iValue);
      }));
   }

#4 Updated by Constantin Asofiei about 1 year ago

Please check what anotherError contains in the method. I think 4GL defines a single var per top-level block, not multiple vars.

Something like:

def var a1 as Progress.Lang.AppError.
def var a2 as Progress.Lang.AppError.
a1 = NEW Progress.Lang.AppError("err1").
a2 = NEW Progress.Lang.AppError("err2").

UNDO, THROW a1.
UNDO, THROW a2.

// check after each block what anotherError has in the message.  it should be err1 after first block and err2 after second block.

#5 Updated by Teodor Gorghe about 1 year ago

Constantin Asofiei wrote:

Please check what anotherError contains in the method. I think 4GL defines a single var per top-level block, not multiple vars.

Something like:
[...]

This test passes on 4GL and FWD:

    @Test.
    METHOD PUBLIC VOID TestMethodCatchTwoVarInstance ():
        DEFINE VARIABLE a1 AS Progress.Lang.AppError NO-UNDO.
        DEFINE VARIABLE a2 AS Progress.Lang.AppError NO-UNDO.
        a1 = NEW Progress.Lang.AppError("err1", 1).
        a2 = NEW Progress.Lang.AppError("err2", 1).
        DO ON ERROR UNDO, THROW:
            UNDO, THROW a1.
            CATCH anotherError AS Progress.Lang.Error :
                Assert:Equals("err1", anotherError:GetMessage(1)).
            END CATCH.
        END.
        DO ON ERROR UNDO, THROW:
            UNDO, THROW a2.
            CATCH anotherError AS Progress.Lang.Error :
                Assert:Equals("err2", anotherError:GetMessage(1)).
            END CATCH.
        END.
    END METHOD.

When it used on properties, on FWD this test fails (because the variable for the first catch block is wrong):

    DEFINE PUBLIC STATIC PROPERTY DummyProperty2 AS INTEGER INITIAL 0 NO-UNDO
    GET():
        DEFINE VARIABLE a1 AS Progress.Lang.AppError NO-UNDO.
        DEFINE VARIABLE a2 AS Progress.Lang.AppError NO-UNDO.
        a1 = NEW Progress.Lang.AppError("err1", 1).
        a2 = NEW Progress.Lang.AppError("err2", 1).
        DO ON ERROR UNDO, THROW:
            UNDO, THROW a1.
            CATCH anotherError AS Progress.Lang.Error :
                Assert:Equals("err1", anotherError:GetMessage(1)).
            END CATCH.
        END.
        DO ON ERROR UNDO, THROW:
            UNDO, THROW a2.
            CATCH anotherError AS Progress.Lang.Error :
                Assert:Equals("err2", anotherError:GetMessage(1)).
                RETURN DummyProperty2.
            END CATCH.
        END.
        RETURN -1.
    END GET.
    PRIVATE SET.
    @Test.
    METHOD PUBLIC VOID TestPropertyCatchTwoVarInstance ():
        Assert:Equals(0, DummyProperty2).
    END METHOD.

ABL Code

   @LegacySignature(type = Type.METHOD, name = "TestMethodCatchTwoVarInstance")
   @Test(order = 2)
   public void testMethodCatchTwoVarInstance()
   {
      object<? extends com.goldencode.p2j.oo.lang.AppError> a1 = TypeFactory.object(com.goldencode.p2j.oo.lang.AppError.class);
      object<? extends com.goldencode.p2j.oo.lang.AppError> a2 = TypeFactory.object(com.goldencode.p2j.oo.lang.AppError.class);
      object<? extends com.goldencode.p2j.oo.lang.LegacyError> anotherError_top = TypeFactory.object(com.goldencode.p2j.oo.lang.LegacyError.class);
      object<? extends com.goldencode.p2j.oo.lang.LegacyError> anotherError_top_1 = TypeFactory.object(com.goldencode.p2j.oo.lang.LegacyError.class);

      internalProcedure(TestCatchVariable.class, this, "TestMethodCatchTwoVarInstance", new Block((Body) () -> 
      {
         a1.assign(ObjectOps.newInstance(com.goldencode.p2j.oo.lang.AppError.class, "II", "err1", 1));
         a2.assign(ObjectOps.newInstance(com.goldencode.p2j.oo.lang.AppError.class, "II", "err2", 1));

         OnPhrase[] onPhrase6 = new OnPhrase[]
         {
            new OnPhrase(Condition.ERROR, Action.THROW, "blockLabel6")
         };

         doBlock(TransactionType.SUB, "blockLabel6", onPhrase6, new Block((Init) () -> 
         {
            catchError(com.goldencode.p2j.oo.lang.LegacyError.class, 
            anotherError -> 
            {
               anotherError_top.assign(anotherError);
               Assert.legacyEquals(new character("err1"), anotherError.ref().getMessage(new integer(1)));
            });
         }, 
         (Body) () -> 
         {
            {
               undoThrow(a1);
            }
         }));

         OnPhrase[] onPhrase7 = new OnPhrase[]
         {
            new OnPhrase(Condition.ERROR, Action.THROW, "blockLabel7")
         };

         doBlock(TransactionType.SUB, "blockLabel7", onPhrase7, new Block((Init) () -> 
         {
            catchError(com.goldencode.p2j.oo.lang.LegacyError.class, 
            anotherError_1 -> 
            {
               anotherError_top_1.assign(anotherError_1);
               Assert.legacyEquals(new character("err2"), anotherError_1.ref().getMessage(new integer(1)));
            });
         }, 
         (Body) () -> 
         {
            {
               undoThrow(a2);
            }
         }));
      }));
   @LegacySignature(type = Type.GETTER, name = "DummyProperty2", returns = "INTEGER")
   public static integer getDummyProperty2()
   {
      ObjectOps.load(TestCatchVariable.class);
      object<? extends com.goldencode.p2j.oo.lang.AppError> a1 = TypeFactory.object(com.goldencode.p2j.oo.lang.AppError.class);
      object<? extends com.goldencode.p2j.oo.lang.AppError> a2 = TypeFactory.object(com.goldencode.p2j.oo.lang.AppError.class);
      object<? extends com.goldencode.p2j.oo.lang.LegacyError> anotherError_top = TypeFactory.object(com.goldencode.p2j.oo.lang.LegacyError.class);
      object<? extends com.goldencode.p2j.oo.lang.LegacyError> anotherError_top_1 = TypeFactory.object(com.goldencode.p2j.oo.lang.LegacyError.class);

      return function(TestCatchVariable.class, "DummyProperty2", integer.class, new Block((Body) () -> 
      {
         a1.assign(ObjectOps.newInstance(com.goldencode.p2j.oo.lang.AppError.class, "II", "err1", 1));
         a2.assign(ObjectOps.newInstance(com.goldencode.p2j.oo.lang.AppError.class, "II", "err2", 1));

         OnPhrase[] onPhrase2 = new OnPhrase[]
         {
            new OnPhrase(Condition.ERROR, Action.THROW, "blockLabel2")
         };

         doBlock(TransactionType.SUB, "blockLabel2", onPhrase2, new Block((Init) () -> 
         {
            catchError(com.goldencode.p2j.oo.lang.LegacyError.class, 
            anotherError -> 
            {
               anotherError_top.assign(anotherError);
               Assert.legacyEquals(new character("err1"), anotherError_top_1.ref().getMessage(new integer(1)));
            });
         }, 
         (Body) () -> 
         {
            {
               undoThrow(a1);
            }
         }));

         OnPhrase[] onPhrase3 = new OnPhrase[]
         {
            new OnPhrase(Condition.ERROR, Action.THROW, "blockLabel3")
         };

         doBlock(TransactionType.SUB, "blockLabel3", onPhrase3, new Block((Init) () -> 
         {
            catchError(com.goldencode.p2j.oo.lang.LegacyError.class, 
            anotherError_1 -> 
            {
               anotherError_top_1.assign(anotherError_1);
               Assert.legacyEquals(new character("err2"), anotherError_1.ref().getMessage(new integer(1)));
               returnNormal(dummyProperty2.get());
            });
         }, 
         (Body) () -> 
         {
            {
               undoThrow(a2);
            }
         }));

         storeReturnValue(-1);
      }));
   }
   @LegacySignature(type = Type.METHOD, name = "TestPropertyCatchTwoVarInstance")
   @Test(order = 4)
   public void testPropertyCatchTwoVarInstance()
   {
      internalProcedure(TestCatchVariable.class, this, "TestPropertyCatchTwoVarInstance", new Block((Body) () -> 
      {
         Assert.legacyEquals(new integer(0), getDummyProperty2());
      }));
   }

╷
├─ JUnit Jupiter ✔
├─ JUnit Vintage ✔
├─ JUnit Platform Suite ✔
└─ FWD Test ✔
   └─ tests.error_handling.TestCatchVariable ✔
      ├─ testMethodCatchVarInstance ✔
      ├─ testMethodCatchTwoVarInstance ✔
      ├─ testPropertyCatchVarInstance ✘ Expected: TRUE but was: no
      └─ testPropertyCatchTwoVarInstance ✘ Invalid handle.  Not initialized or points to a deleted object. (3135)

#6 Updated by Constantin Asofiei about 1 year ago

Change it like this:

    @Test.
    METHOD PUBLIC VOID TestMethodCatchTwoVarInstance ():
        DEFINE VARIABLE a1 AS Progress.Lang.AppError NO-UNDO.
        DEFINE VARIABLE a2 AS Progress.Lang.AppError NO-UNDO.
        a1 = NEW Progress.Lang.AppError("err1", 1).
        a2 = NEW Progress.Lang.AppError("err2", 1).
        DO ON ERROR UNDO, THROW:
            UNDO, THROW a1.
            CATCH anotherError AS Progress.Lang.Error :
                Assert:Equals("err1", anotherError:GetMessage(1)).
            END CATCH.
        END.
        Assert:Equals("err1", anotherError:GetMessage(1)).
        DO ON ERROR UNDO, THROW:
            UNDO, THROW a2.
            CATCH anotherError AS Progress.Lang.Error :
                Assert:Equals("err2", anotherError:GetMessage(1)).
            END CATCH.
        END.
        Assert:Equals("err2", anotherError:GetMessage(1)).
    END METHOD.

#7 Updated by Teodor Gorghe about 1 year ago

Constantin Asofiei wrote:

Change it like this:
[...]

This passes on FWD and 4GL. It is used the same ~~variable~~ reference.

I think that the assign statement on the lambda function should not be there and the lambda function parameter should be used. On property, this does not pass.

#8 Updated by Teodor Gorghe about 1 year ago

Status changed from WIP to Review
% Done changed from 0 to 100

Created task branch 10235a.
Committed revision 16014 on task branch 10235a:
- CATCH variable scoping fixups for property getters/setters.

Constantin, please check if this fix is ok.
Now, all the test passes.

#9 Updated by Teodor Gorghe about 1 year ago

reviewer Constantin Asofiei added

#10 Updated by Constantin Asofiei about 1 year ago

Please move this code into a function in common-progress.rules

ancestor(prog.method_def, -1)  or 
             ancestor(prog.constructor, -1) or
             ancestor(prog.kw_get, -1)      or
             ancestor(prog.kw_set, -1)      or

similar to how hasTopLevelInternalNodeAncestor is defined. Also, include destructors, too.

#11 Updated by Teodor Gorghe about 1 year ago

Committed revision 16015 on task branch 10235a:
- moved the condition into common-progress.rules

Committed revision 16016 on task branch 10235a:
- included destructors into hasTopLevelMethodPropertyAncestor

#12 Updated by Constantin Asofiei about 1 year ago

Status changed from Review to Internal Test

Thanks, we need to conversion-test this.

#13 Updated by Constantin Asofiei about 1 year ago

Status changed from Internal Test to Merge Pending

Please merge 10235a now.

#14 Updated by Teodor Gorghe about 1 year ago

Status changed from Merge Pending to Test

Branch 10235a was merged to trunk revision 16049 and archived.

Also available in: Atom PDF

Project

General

Profile

FWD » Core Development » Conversion Tools

Issues

Custom queries