[flang-commits] [flang] [flang] Respect record type changes in FIR alias analysis access paths (PR #231655)

[Tom Eccles via flang-commits]

================
@@ -591,40 +669,43 @@ static bool noAliasBasedOnType(mlir::Value lhs, mlir::Value rhs) {
 /// source's POINTER attribute reflects the component traversed during the
 /// walk, not the root variable, so we check only TARGET on the source.
 ///
-/// One exception: if BOTH sides end with a PointerDeref, the two pointers
-/// could independently be associated with the same third-party TARGET
-/// variable, so we conservatively return false.
+/// One exception: if BOTH sides continue through a PointerDeref, the two
+/// pointers could independently be associated with the same third-party
+/// TARGET variable, so such steps do not prove disjointness.
 static bool pathsDivergeAtComponent(const fir::AliasAnalysis::Source &lhsSrc,
                                     const fir::AliasAnalysis::Source &rhsSrc) {
   using PathStep = fir::AliasAnalysis::Source::PathStep;
+  if (lhsSrc.accessPath.storageDecl != rhsSrc.accessPath.storageDecl)
+    return false;
   auto &lhsSteps = lhsSrc.accessPath.steps;
   auto &rhsSteps = rhsSrc.accessPath.steps;
-  size_t minLen = std::min(lhsSteps.size(), rhsSteps.size());
-  for (size_t i = 0; i < minLen; ++i) {
-    if (lhsSteps[i].kind == PathStep::Kind::Component &&
-        rhsSteps[i].kind == PathStep::Kind::Component &&
-        lhsSteps[i].component != rhsSteps[i].component) {
-      auto hasPtrDerefAfter = [](llvm::ArrayRef<PathStep> steps, size_t from) {
-        for (size_t j = from; j < steps.size(); ++j)
-          if (steps[j].kind == PathStep::Kind::PointerDeref)
-            return true;
-        return false;
-      };
-      bool lhsHasPtrDeref = hasPtrDerefAfter(lhsSteps, i + 1);
-      bool rhsHasPtrDeref = hasPtrDerefAfter(rhsSteps, i + 1);
+  auto hasPtrDeref = [](llvm::ArrayRef<PathStep> steps) {
+    return llvm::any_of(steps, [](const PathStep &s) {
+      return s.kind == PathStep::Kind::PointerDeref;
+    });
+  };
+  for (size_t i = 0; i < lhsSteps.size(); ++i) {
+    if (lhsSteps[i].kind != PathStep::Kind::Component)
+      continue;
+    for (size_t j = 0; j < rhsSteps.size(); ++j) {
+      if (rhsSteps[j].kind != PathStep::Kind::Component ||
+          !componentsAreDisjoint(lhsSteps[i], rhsSteps[j]))
+        continue;
+      bool lhsHasPtrDeref =
+          hasPtrDeref(llvm::ArrayRef<PathStep>(lhsSteps).drop_front(i + 1));
+      bool rhsHasPtrDeref =
+          hasPtrDeref(llvm::ArrayRef<PathStep>(rhsSteps).drop_front(j + 1));
       if (lhsHasPtrDeref && rhsHasPtrDeref)
-        return false;
+        continue;
       if (lhsHasPtrDeref || rhsHasPtrDeref) {
-        for (size_t j = 0; j < i; ++j)
-          if (lhsSteps[j].kind == PathStep::Kind::PointerDeref)
-            return false;
+        if (hasPtrDeref(llvm::ArrayRef<PathStep>(lhsSteps).take_front(i)) ||
+            hasPtrDeref(llvm::ArrayRef<PathStep>(rhsSteps).take_front(j)))
+          continue;
         if (lhsSrc.isTarget() || rhsSrc.isTarget())
----------------
tblah wrote:

My AI reviewer found that with `x%pointer => x%allocatable`, it incorrectly treats `x%allocatable%v` and `x%pointer%v` as disjoint. 

I think this missed this case where the TARGET attribute is present.

Full (AI-written) reproducer:
```
module m
  type base
    integer :: v(4)
  end type
  type, extends(base) :: child
    type(base), allocatable :: a
    type(base), pointer :: p
  end type
contains
  subroutine shift(x)
    type(child), target :: x
    x%a%v(2:4) = x%p%v(1:3)
  end subroutine
end module
program main
  use m
  type(child), target :: x
  allocate(x%a)
  x%a%v = [1, 2, 3, 4]
  x%p => x%a
  call shift(x)
  print *, x%a%v
  if (any(x%a%v /= [1, 1, 2, 3])) stop 1
end program
```

https://github.com/llvm/llvm-project/pull/231655