Bug 453635 - [compiler][null] Update NullReferenceImplTests and friends
Summary: [compiler][null] Update NullReferenceImplTests and friends
Status: CLOSED WONTFIX
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 4.4   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: JDT-Core-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: test
Depends on: 453648
Blocks:
  Show dependency tree
 
Reported: 2014-11-30 08:28 EST by Stephan Herrmann CLA
Modified: 2020-06-05 18:25 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Herrmann CLA 2014-11-30 08:28:14 EST
Bug 453483 requires changes to the bit operations in UnconditionalFlowInfo. This routinely requires verification using NullReferenceImplTests (which are not part of regular builds & tests).

The test harness given there and in NullReferenceImplTransformations supports several strategies for updating and comparing these elements:
 - a human readably description of all transitions in each of the major bit operations (as comments)
 - a hex-encoded tabular definition of those transitions
 - the current implementation of those operations
Additionally, karnaugh maps can be created for developing more compact implementations.

It turns out, that tests and implementation have slightly diverged, thus reducing the feasibility of comparisons:

- tabular definitions are incomplete. Based on a notion of transitive closure one state (0x1C) was considered unreachable, but previous bugs have proven this to be wrong - perhaps due to added UFI-operations that do not participate in the implementation tests. Operation mergedWith was already amended, the others still have to follow suite.

- tabular definitions as well as textual description (comments) are not ordered in the same way as the generator output, making it difficult to compare whether definitions re-generated from the current implementation match to the previously captured definitions.

- re-generating definitions marks all textual descriptions of ThreeDimensionalTransformation (incl. subclass) as "CHECK", because it is not correctly detected when a transition actually matches the previous tabular form.
Comment 1 Stephan Herrmann CLA 2014-11-30 08:33:58 EST
Additionally, bug 453483 will introduce two more bits to be considered during addInitializationsFrom(). I'll check feasibility of including these bits in the tests.

This is a problem both for defining the transformation as well as for printing the karnaugh maps. In both cases an increase in size by factor 4 stretches the limits of what can be manually handled.

Minimally, tests must ensure a reasonable value (1,1) for these bits to provide for comparability with previous results.
Comment 2 Stephan Herrmann CLA 2014-11-30 12:32:16 EST
First test updates have been pushed:

http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=b077f1ec27f6f9c95317db4e5e225319a96f87d5
(A) generator changes to solve these issues:
 - comparison of generator output with previous test file was encumbered by different sorting orders. For each section of NullReferenceImplTransformations a well defined order is now used (selected as to require minimal changes in this file).
 - state 0x1C was not considered by the generator because it was not found to be reachable. Considering, however, that these tests don't represent all possible operations on flowInfos, and given that this state has been observed in real life, it should be included by the generator. This is ensured by included all named states ("isSymbolic").
 - the generator created bogus "CHECK" marks because for three-dimensional transformations it could not retrieve the defined result.


http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=9c5059700205577ee2d000823b77569787384ecb
(B) re-order definitions of transformations to align with generator output. In addition to re-ordering lines, this also includes for the symmetric operation "mergedWith" that left & right operands were swapped for a number of late added transitions.


http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=79774393baaba7f9e5f3461242a5b822bf662647
(C) accept output of the generator as improved in (A) into files prepared by (B). All newly added transitions were scanned, some are commented as bogus or not ideal. No transitions were changed, however.

At this point we're close to a fix point, where the generator would create an exact copy of its input. Only remaining difference: comments on individual transitions (some old, some newly added).

Note, that NullReferenceImplTests - which are not part of our regular test suite - show one failure. During addInitializationsFrom the start value 0000 creates different outcomes depending on the number of slots used (UFI.extra bits, happens only in combination with state 0x2C). I assume, the bug depends on whether or not the flow info is recognized as not having any (interesting) null info.
While this is a definite bug, it's not a regression recently introduced - it just wasn't noticed before. I'm waiting with a fix until all dust in the related bugs has settled.

Also, this bug remains open to accommodate more changes necessitated by bug 453483.
Comment 3 Stephan Herrmann CLA 2014-12-04 14:20:34 EST
let's see if bug 453648 will come to life... (not for M4, though)...
Comment 4 Stephan Herrmann CLA 2016-03-25 10:28:59 EDT
Too much on my plate for 4.6. Bulk deferral to 4.7
Comment 5 Manoj N Palat CLA 2018-05-16 12:51:22 EDT
bulk move out of 4.8
Comment 6 Manoj N Palat CLA 2018-05-16 12:56:40 EDT
bulk move out of 4.8