FWD » Core Development » Base Language

To understand the limitations of the P2J PROGRAM-NAME implementation, please see the JavaDoc for EnvironmentOps.getSourceName().

This suggests that looking into the java stack trace is not correct. Instead, ControlFlowOps should keep a stack for the legacy code calls and interogate that.

Other cases to check:
- are user-defined function calls reported too?
- what about triggers?
- how are super procedure calls handled? Does this report the original name (at the SOURCE-PROCEDURE where the procedure or user-defined function is invoked)?
- how are RUN SUPER and SUPER calls reported?

This suggests that looking into the java stack trace is not correct.

Yes. At the time it was originally coded, the converted business logic for RUN was directly invoking the execute() method on instances created from hard coded class names. Since then, you have fully implemented the RUN support properly (i.e. completely indirect calling mechanism, managed by the runtime).

Instead, ControlFlowOps should keep a stack for the legacy code calls and interogate that.

Agreed. This will allow a 100% correct program-name implementation.

Yes, your other points are also valid. Some more investigation is needed.

This one proves a little more challenging: the procedure names in error messages and the procedure's FILE-NAME and NAME attributes follow the PROCEDURE-NAME function. This means that we need to report the relative name (used at the RUN statement) on error messages and whenever the FILE-NAME and NAME attributes are checked for a procedure. Internally (i.e. when interrogating SourceNameMapper APIs) we need to use absolute names (this can be done from the referent's class name, using SourceNameMapper.convertJavaProg): my concern is that, if the PROPATH gets modified, we might resolve the relative name to a completely different program.

my concern is that, if the PROPATH gets modified, we might resolve the relative name to a completely different program.

I don't fully understand your concern. Isn't PROPATH assignment supposed to cause this behavior in the 4GL?

Greg Shah wrote:

my concern is that, if the PROPATH gets modified, we might resolve the relative name to a completely different program.

I don't fully understand your concern. Isn't PROPATH assignment supposed to cause this behavior in the 4GL?

The idea was that internally I can't use the relative name and call i.e. SourceNameMapper.getParameterModes, each time I need it. For this reason, I need to keep two names: for internal usage, the absolute name (resolved and saved when the RUN statement is called for that program). For legacy usage (i.e. error messages, PROGRAM-NAME, etc), the name used at RUN statement (relative name, in my terms).

Code Review 0220d

I am OK with the changes. As always with the procedure processing, this stuff is pretty nasty. Great work!

We don't have much choice in coding so much dependence on reflection and the old approach was also dependent (but broken in 4GL terms). So this is a good improvement, but we need to keep that dependence in mind. I doubt the inner class naming mechanism will change however when we implement the new Java 8 lambda expressions, this code will possibly need revisions.

The only other thing I wonder about is why we need to use findRootEnclosingInstance() in TriggerDefinition. Doesn't outer always properly reference the root enclosing instance?

The only dependence on reflection is the "java-style calls of legacy user-def functions", when we need to determine what function is being invoked.

For example, the code is very dependent on the the naming scheme of inner classes ("this$" and so forth). Such hard coded references may need to be changed when we move to lambda expressions.

I was thinking if someone manages to pass the anon inner block instance instead of ExternalClass.this;

OK, it makes sense.

Option 3 is the best solution, IF it is possible.

A modified option 1 may be the next best approach. The idea is to provide 3 new ControlFlowOps APIs for "direct" calls to converted code. There would be the following:

• invokeDirectExternalProcedure
• invokeDirectInternalProcedure
• invokeDirectFunction

The idea of "direct" is to suggest that the calls are made with a specific Java class (and method names for the last 2) instead of being determined at runtime. I prefer not to do this because it kills compile-time type safety, makes things more indirect and is generally nasty. But it is better than forcing hand-written code to use legacy names and the fully dynamic CFO approach.

Or perhaps option 2 would be not as bad if the CFO code was refactored such that it could be reused for this case and thus there was common code.

Greg Shah wrote:

Option 3 is the best solution, IF it is possible.

1. the "superCall" state
2. the "relative name" used to invoke the procedure/user-def function
3. the target procedure, i.e. which external program was assumed the internal entry was part of, not the actual location of the resolved internal entry.

If I push this to some ProcedureManager stack, ProcedureManager's scopeStart will just need to interogate the top of this stack, before pushing the data to the other stacks (i.e. target procedure, source procedure, etc). Beside this, BlockManager will be changed to push correct superCall/relative name/target procedure, in case of java-style calls (what we are doing with BlockManager.functionBlock will be done for internal and external procedures, too).

Failures during testing of 0222b.zip: there are cases like this (MAJIC uses them):

public static void something()
{
   externalProcedure(...);
}

The 0222b.zip and 0222e.zip (from #2248) have passed regression testing, after checking the cumulative regression testing results.

Code Review 0222b/e

I am fine with the changes. This is another nice improvement/refactoring of the code such that the logic is better centralized.

Have you retested the "standalone" super proc/persistent proc testcases? The only dangerous part of the change is really whether it properly pushes/pops state properly for all cases.

Otherwise, I am fine with you checking this in.

Greg Shah wrote:

Have you retested the "standalone" super proc/persistent proc testcases?

Yes, the tests are OK.

0222b.zip was committed to bzr rev 10477
0222c.zip from #2228 (note 71 has a typo, 0222c.zip is the correct one) was committed to bzr rev 10478