Skip to content

Conversation

@maximilianruesch
Copy link
Collaborator

@maximilianruesch maximilianruesch commented Nov 12, 2025

The field assignability analysis has gone through many amends since its initial introduction, reducing code clarity, maintainability, and extensibility to what could be considered a negative. A recent example of this is the review in #190 (review). It became clear that many of the amends done were symptomatic fixes done to satisfy some edge case, but not fix the underlying issue.

This PR aims to do several things (in no particular order):

  • Reduce the analysis to the core issue of determining "whether there potentially is an execution path from a read to a write or not" (which would correspond to Assignable / NonAssignable)
  • Introduce clear and dedicated support for lazy initialization and clone patterns
  • Improve the readability, clarity and maintainability of the analysis
  • Make the analysis extendable again while keeping sanity
  • Increase soundness to the maximum attainable with the available information

Besides the titular complete overhaul of the analysis, the PR achieves this by:

  1. Introducing a "part" system, where each supported "pattern" can determine whether it thinks it is applicable, and respond to how this influences the assignability of the field under analysis
    i. Parts are traits that register themselves with in the abstract analysis when they are included in the analysis inheritance hierarchy
    ii. The order of parts is fixed through Scala's linearisation of traits
  2. Abstracting read / write information handling away and only handling particular accesses and their interaction in the actual analysis
  3. Removing the old notion of L0 because no one understood it anyway, and we should move forward with a unified notion of an assignability analysis

This PR acknowledges the additional test cases ("advanced counter examples") developed in #190 and integrates them.

This PR does not (yet):

  • Overhaul the tests, such that:
    • Each level gets an assigned ground truth
    • Through an override system, allow definition of current expectations against the ground truth
    • Basic interleavings of read / write discovery is tested (currently, reads are discovered before writes, swapping them manually results in test failures when it should not)
  • Offer more sophisticated read-write-path analysis in situations where arbitrary calls are made before writing a field (currently leading to a sound abort) even for common cases such as:
    • Recognizing that there is no path from a reading constructor to a writing constructor when they do not call any constructors
    • Handling simple methods such as setters that do not read any fields, or more generally offering a framework for marking methods as "dangerous, if we discover reads or other suspicious calls on them later"
  • Overhaul the lazy initialization part, which may benefit from code clarification

Support pickup / drop

The overhaul changes results in integration tests, which are explained here (currently an incomplete list):

Now supported

  • Extremely convoluted factory patterns (like clones, except that the value is not read from the current instance), especially with non-private fields

Now Unsupported

  • Arbitrary instance calls in constructors before writes: Could contain reads of the field -> (sophisticated) Interprocedural path analysis is out of scope here
    • This change causes by far the most integration test failures
  • Lazy init patterns that do not take place on the this instance: Examples include inline lazy initializers for .class values generated by compilers before JDK 5 (the relevant integration test file batik-util 1.7.jar was produced with JDK 1.1). See GeneratedClassConstantLazyInit test for further information.

# Conflicts:
#	DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/lazyinitialization/primitive_types/LazyInitializationPrimitiveTypes.java
#	DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/stringelements/SimpleStringModel.java
#	OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala
#	OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants