Bugzilla – Full Text Bug Listing
| Summary: | [otjld][compiler] consider changing LiftingFailedException to a checked exception | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] Objectteams | Reporter: | Stephan Herrmann <stephan.herrmann> | ||||||||||
| Component: | OTJ | Assignee: | Stephan Herrmann <stephan.herrmann> | ||||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||||
| Severity: | enhancement | ||||||||||||
| Priority: | P3 | ||||||||||||
| Version: | 0.8 | ||||||||||||
| Target Milestone: | 2.0 RC1 | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Stephan Herrmann
Want to decide either way before the upcoming release to enable inclusion in the freeze of OTJLD 1.3. Created attachment 195287 [details]
Current experiment
This patch changes LiftingFailedException into a checked exception.
+ Lift methods that can raise the exception now declare it.
+ Callin wrappers propagate this declaration if needed.
+ OTRE catches LiftingFailedException in chaining wrapper
(just like it was already catching LiftingVetoException).
This brings one subtlety: when a binding ambiguity is introduced somewhere in
a team hierarchy, existing liftTo methods may have to be redefined to throwing
this exception. The patch experiments with hiding this fact, instead of reporting
an incompatible redefinition, we report
"Team introduces binding ambiguity for role {0}, which may break clients of the super team"
This problem is classified as a "hidden lifting problem", because lifting may fail
in a situation other than an explicit declared lifting.
The existing diagnostic
"Unsafe callin mapping, because lifting to role {0} may fail due to a reported binding ambiguity (OTJLD 4.1(b))."
Has been extended to apply to problems in parameter lifting, too, and has been
reassigned to the new class of "hidden lifting problems".
The new diagnostics are configurable as follows:
By default hidden lifting problems have severity ERROR.
+ The batch compiler understands the new warning option "hiddenLiftingProblem".
+ Inside the code a new @SuppressWarnings("hidden-lifting-problem") can be used.
TODO: the corresponding paragraph in the OTJLD is yet to be written
and linked to error messages.
TODO: check how lifting problems due to abstract relevant role blend in.
TODO: the patch uses getRealClass() in two locations, check if this is OK.
Note that the new option is not configurable via the UI but that might be OK.
Created attachment 195496 [details]
Second part of the implementation
Meanwhile two minor commits have fixed issues:
r1523:
- if a team has >0 abstract relevant roles we still need to check, which!
r1524:
- if triggered by parameter mapping flag hidden-lifting-problem against the callin mapping
This patch addresses the following:
Split IProblem into CallinDespiteBindingAmbiguity & CallinDespiteAbstractRole
Consistently generate throw LFE only when a problem has been detected and
during reporting use the problemId specific to the detected problem.
Remember the problemId also in CallinMappingDeclaration.rolesWithLiftingProblem
(renamed from rolesWithLiftingProblem).
Declare LiftingFailedException also in _OT$lift_dynamic methods, if problem
exists, otherwise use new SoftLiftingFailedException, which would probably
indicate an incompatible class change.
Existing OT/J code has been adjusted to the change using different strategies: (A) Avoid abstractness for bound roles. (A.1) Add method bodies which log the problem. Replaces LFE in no-man's land with logging inside the role (see ResourceProjectAdaptor, PresentationAdaptor) (A.2) Remove abstract methods and require clients to cast to a specific role type. Replaces LFE in no-man's land with CCE at client side. (see OTTypeHierarchies) (B) Declare LFE (team method with declared lifting) Makes formerly hidden LFE visible (see tests in Reflection, PlayedByRelation) (C) Suppress "hidden-lifting-problem" at callin binding Makes formerly hidden LFE visible (see CallHierarchyAdaptor, OTQuickFixes) All required changes make sense. Let's go for it. Created attachment 195552 [details] 3rd part As implemented by previous patches this diagnostic overlaps with the irritant DefiniteBindingAmbiguity as introduced in bug 316200. This new patch obsoletes that irritant (and warning token "def-bind-ambiguity"): - explicit locations (declared lifting and callout) are signaled by the need to declare LiftingFailedException (added an OTJLD link to that specific error message). No more reporting required. - implicit locations (callin) now use the HiddenLiftingProblem irritant. Patch also reverts some refactoring from the previous patch (argument to TeamModel.getRoleToLiftTo()) Created attachment 195642 [details] part 4 More advances - discriminate errors re LiftingFailedException, mention OTJLD only when reporting against a lift call - re-introduce special ambiguity analysis for <B base R> (was wrongly removed in 3rd part attachment 195552 [details]) Also keep tests up-to-date. Patches have been committed - r1511-4 - r1526-8 - r1560-2 - r1565-6 plus minor corrections/polish in - r1523 - r1524 - r1530-1 Existing code has been adjusted - r1516-21 - r1525 That closes this issue. Note, that we also have a new quickfix in bug 345844. Verified for 2.0 RC1 using build 201105161939. |