[cg] Lookup Visitors generation fails when calling external OCL Operations

[eclipse-ocl-bot]

| --- | --- | | Bugzilla Link | 500817 | | Status | NEW | | Importance | P3 normal | | Reported | Sep 04, 2016 06:17 EDT | | Modified | Sep 06, 2016 13:10 EDT | | Reporter | Adolfo Sanchez-Barbudo Herrera |

Description

A rework that replaces an OCL closure with a couple of OCL operations in the companies example makes the lookup visitor generation fail.

Branches:

• ocl-master
• qvtd-master
• cs2as-master

Repro:

• Run org.xtext.example.companies.build\src\org\xtext\example\companies\build\mwe\GenerateCompanyModels.mwe2
• Open org.xtext.example.companies\src-gen\org\xtext\example\company\util\CompanyUnqualifiedEmployeeLookupVisitor.java

Observed error at lines (108, 1090:

final /@Thrown/ SetValue OP_company_c_c_Department_c_c_getEmployees_o_e_32_c_32_Set_o_company_c_c_Employee_e_0 = idResolver.getOperation(OPid_getEmployees);\ final /@Thrown/ SetValue getEmployees = (SetValue)OP_company_c_c_Department_c_c_getEmployees_o_e_32_c_32_Set_o_company_c_c_Employee_e_0.evaluate(executor, SET_CLSSid_Employee, _1);

---

NB For several reasons I'm considering (pending on performance analysis) to deprecate Visitors generation in favour of CGed Cached OCL Operations.

Therefore a complete solution (including dynamic dispatch) of Bug 500519 (with respect to operations) should make this bugzilla obsolete.

[eclipse-ocl-bot]

By Ed Willink on Sep 04, 2016 06:57

Finally, a repro of a bad call to Complete OCL. Bug 500519 would fix it all, but I think that a partial simpler solution will work since you don't seem to need overloads. Since CompanyUnqualifiedEmployeeLookupVisitor is not constrained by an outer JET template, we can easily add local cached calls.

[eclipse-ocl-bot]

By Ed Willink on Sep 04, 2016 07:18

In principle it is sufficient to replace the bad generation:

        final /*@Thrown*/ SetValue OP_company_c_c_Department_c_c_getEmployees_o_e_32_c_32_Set_o_company_c_c_Employee_e_0 = idResolver.getOperation(OPid_getEmployees);\
        final /*@Thrown*/ SetValue getEmployees = (SetValue)OP_company_c_c_Department_c_c_getEmployees_o_e_32_c_32_Set_o_company_c_c_Employee_e_0.evaluate(executor, SET_CLSSid_Employee, _1);

by something like:

        final /*@Thrown*/ SetValue getEmployees = (SetValue)((ExecutorInternalExtension)executor).getCachedEvaluationResult(Department_GetEmployees.INSTANCE, dummyCaller, new Object[]{_1});

and the appropriate Department_GetEmployees class.

dummyCaller is barely used. It should really return SET_CLSSid_Employee as its getTypeId(). If debugging ever gets enthusiastic dummyCaller should identify the caller better.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Sep 05, 2016 06:33

Since modifying generated code, I've created a Company_GetEmployees rather than Department_GetEmployees (so that any inlining is removed)

For the first 6 models (Last one taking too long)\ Original experiment: 3, 9, 47, 54, 1119, 25713\ New experiment : 6, 12, 19, 21, 251 , 6139

Clearly, escalating much better with only this improvement.

[eclipse-ocl-bot]

By Ed Willink on Sep 05, 2016 06:57

(In reply to Adolfo Sanchez-Barbudo Herrera from comment #3)

For the first 6 models (Last one taking too long) Original experiment: 3, 9, 47, 54, 1119, 25713 New experiment : 6, 12, 19, 21, 251 , 6139

Disappointing but not entirely surprising that the small models get slower.

I'm expecting that the current bad operation calls should detect that CompaniesHelpers.ocl is the source of missing operations. Probably simplest to treat them as 'global constants' so they get emitted once as nested classes. A separate CompleteOCL code generation specifically for CompaniesHelpers.ocl has problems co-ordinating the base package prefixes.

Other operations are probably OCLinEcore-like and so subject to Ecore compatibility. If you produce a call tree we can see how/whether we should cache OCLinEcore.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Sep 05, 2016 07:40

• I've committed the code (cs2as repo), in case you wanted to have a look.
• qvtd-asanchez/master also includes a workaround to address the fact that QVTiTransforamtionExecutor is not an ExecutorInternalExtension

(In reply to comment #4)\

Other operations are probably OCLinEcore-like and so subject to Ecore compatibility. If you produce a call tree we can see how/whether we should cache OCLinEcore.

Is this for me ? If that is the case, I'm unsure what you a referring/what you want me have a look and/or produce

Regards,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Sep 05, 2016 16:39

(In reply to Ed Willink from comment #1)

Finally, a repro of a bad call to Complete OCL.

The current code uses a CGNativeOperation for lookupPackage in ExampleV2_CG. This was deep within a recursion, so something happened right.

It may be sufficient to just revise CGNativeOperation as cached and invoke CGNativeOperation more widely.

[eclipse-ocl-bot]

By Ed Willink on Sep 06, 2016 06:54

(In reply to Ed Willink from comment #6)

The current code uses a CGNativeOperation for lookupPackage in ExampleV2_CG. This was deep within a recursion, so something happened right.

AS2CGVisitor.inlineOperationCall computes all transitively referenced final operations and inhibits inlining if the called operation is recursed.

[eclipse-ocl-bot]

By Ed Willink on Sep 06, 2016 13:10

(In reply to Ed Willink from comment #7)

AS2CGVisitor.inlineOperationCall computes all transitively referenced final operations and inhibits inlining if the called operation is recursed.

Avoiding inlining when calling a non-final operation avoids the infinite recursion and is a simple heuristic for where a cache might help.