Bug 338376

Comment 1 Axel Uhl 2011-03-06 04:48:19 EST

Created attachment 190485 [details]
Patch introducing :: as global type name prefix

Contains the parser patches as well as the analyzer changes necessary

Comment 2 Axel Uhl 2011-03-06 05:07:54 EST

Created attachment 190488 [details]
Updated patch with more deprecations where useful

I added more deprecations to the methods without the "global" argument which only delegate to the new flavors with the "global" parameter.

Comment 3 Axel Uhl 2011-03-06 05:15:27 EST

Created attachment 190489 [details]
Handles global flag properly, not doing relative lookup for global==true

I've change the behavior in lookupClassifier such that when global==true, only a global lookup is performed

Comment 4 Ed Willink 2011-03-06 06:37:41 EST

There is no way that I will time to look at this till after EclipseCon, and even then I'm not sure that it's worth spending time on.

I can see now that a new API that is only ever invoked for global might appear safe, since it can only be entered for global. That suggests that the new API should be distinct, so a new API with an isGlobal flag is dangerous, since isGlobal = false is unsafe; not clear which API is being used.

However the current API guarantees that any derived nested Environment gets to intercept every lookup; hypothetically it might convert all names to lowercase. A new API makes global irregular, so even if API tooling is satisfied that there is API compatibility, there is a semantic change that needs discussion and notification.

:: prefix is not part of OCL, so is not part of QVTo or MOFM2T or ...

You need to build and test QVTo and MOFM2T and cinsult with their teams to see whether this grammar externsion is appropriate. I'm pretty dubious.

OCL has a single modelspace. M2M has multiple modelspaces, one per direction, and consequently introduces a modelname:: prefix to disambiguate. A ::modelname would
be wierd. I would need to consult language specs to see to what extent M::A::B::x
can hide M::A::x.

If you want to pursue this in EssentialOCL.gi you'll need to do the language research and the client discussions.

-1 until the compatibility issues have been researched and there is buyin from at least one of Acceleo or QVTo. A ParsingOption may be necessary to placate the other.

Comment 5 Ed Willink 2011-03-07 02:09:18 EST

-1

EssentialOCL.gi changes have impacts on QVTo and Acceleo for which a :: prefix is very questionable. An OCL-specific grammar change can be in OCLParser.g.

The change to invoke createPathNameCS with a false argument is an 'API' change that will impact any derived code that overrides. Preserve the existing 'API' and do a setGlobal afterwards or introduce a createGlobalPathNameCS.

...

---------------
From email discussion
---------------
>> Looking at EcoreEnvironment, EPackage.Registry is a Map of String to
>> .... There is no reason why you shouldn't install a mapping from
>> 'OclEcore' and 'EmfEcore' aliases in addition to the defaults. The
>> problem is then solveable. Any application with ambiguous package names
>> should install aliases to facilitate unambiguous access.
>
> EPackage.Registry class comment: A map from namespace URI to EPackage. So you can't just put any name as key as these names are not used for package resolution when you give a type expression. Even worse, package name resolution from the registry is implemented asking the EPackage for its name, so there is no way to alias.
Ahah! So that's why it's always been so unhelpful.

An nsURI is a fairly arbitrary string that applications may find useful. "OclEcore" is a valid URI. So there is no reason why, from the Javadoc API perspective, an EPackage.Registry should not have multiple nsURIs for a package.

Currently EcoreEnvironment.findPackage searches on registry.values.getName(). It could also/alternatively search on registry.keys. A key search would realise the import and alias 'syntax'.

API compatibility.

The registry.values search must be the default. We cannot rely on the nsURI being a complex http: string that is guaranteed not to match, so I think the  additional keys search can only occur if specifically enabled by changing e.g. EnvironmentOption.LOOKUP_PACKAGE_BY_ALIAS from its default of false to true in the Environment. This is perhaps a sufficiently useful area of control to also provide EnvironmentOption.LOOKUP_PACKAGE_BY_NAME with a default of true.

Unfortunatately findPackage is static so the last line of lookupPackage probably needs to test the option(s) and divert to an appropriate handler.

Existing clients see no change, just a minor overhead as the option is tested.

You can
- add the alias to the registry
- set the LOOKUP_PACKAGE_BY_ALIAS to true
- perhaps set the LOOKUP_PACKAGE_BY_NAME to false
You may need to ensure that the Registry passed to the Environment is not shared by other applications that may be confused by the non-unique values associated with aliases.

> I'm somehow unable to follow the QVT problem you outline. 
Maybe that was a confusing red herring. The point was that there are specification problems in this area, so we can look fro a problem-driven solution rather than an implementation-driven one.
> It doesn't force anyone in QVT to use a :: prefix.
It doesn't force the :: syntax, but it makes it available, so QVT programs can use it without syntax violations. Even though the OCL grammar has no shift reduce conflicts from the :: prefix, that may not be true for derived grammars. My recollection now is that the PathNameCS shift reduce conflict came from the QVTr grammar where TemplateExpressions are specified to occur in too many contexts.
> Which languages extend EssentialOCL.gi? To what degree does it provide contractual guarantees to such extenders?
I think that both QVTo and Acceleo use EssentialOCL.gi. I know that QVTo takes an unmodified copy to protect itself.

There is no formal specification of the role EssentialOCL.gi; just the intuitive goal of providing as close an approximation to the re-useable Essential OCL OMG component as possible. That is why we split it off from OCLParser.g that has OCL-specific clutter.

You could provide the extended pathNameCS in OCLParser.g to avoid downstream ripple. But I prefer aliases.
> It'd be great to get this into Indigo because a larger language workbench development of ours could probably not use OCL reasonably if we get stuck not being able to resolve Ecore types in expressions with a context type/package from ocl::ecore.
There is no way this will make it through in the week before M6, and anything that changes API is difficult for M7.

The alias solution involves no grammar change, no parser API change (extra createPathNameCS argument), no analyzer API change, 'no' environment API change and it provides a generally useful capability. There are just potentially private new findPackage methods and one or two new Options. These clearly have no API impact so could happen in M7.

The same approach to aliases in UMLEnvironment might finally make the two Environments have the same behaviour.

Comment 6 Axel Uhl 2011-03-07 03:13:52 EST

Instead of introducing a package prefix marking package references as global, project lead decided to go for optional package lookup by their URIs in EEPackage.Registry if a special option is set accordingly on OCL environment. See https://bugs.eclipse.org/bugs/show_bug.cgi?id=339052