[flang] Fix crash in HLFIR generation

[klausler]

Structure constructors with multiple components would crash when components were from parent types. The search for the right parent component must be done anew for each component.

Fixes #118270, #96994, and #105848.

[llvmbot]

@llvm/pr-subscribers-flang-fir-hlfir

Author: Peter Klausler (klausler)

Changes

Structure constructors with multiple components would crash when components were from parent types. The search for the right parent component must be done anew for each component.

Fixes #118270.

---

Full diff: https://github.com/llvm/llvm-project/pull/118399.diff

1 Files Affected:

• (modified) flang/lib/Lower/ConvertExprToHLFIR.cpp (+5-6)

diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp
index e93fbc562f9b13..4ab319b016caf7 100644
--- a/flang/lib/Lower/ConvertExprToHLFIR.cpp
+++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -1696,18 +1696,17 @@ class HlfirBuilder {
     // required chains of hlfir.designate to address the parent components so
     // that the StructureConstructor can later be lowered by addressing these
     // parent components if needed. Note: the front-end orders the components in
-    // structure constructors. The code below relies on the component to appear
-    // in order.
+    // structure constructors.
     using ValueAndParent = std::tuple<const Fortran::lower::SomeExpr &,
                                       const Fortran::semantics::Symbol &,
                                       hlfir::EntityWithAttributes>;
     llvm::SmallVector<ValueAndParent> valuesAndParents;
-    Fortran::lower::ComponentReverseIterator compIterator(
-        ctor.result().derivedTypeSpec());
-    hlfir::EntityWithAttributes currentParent = varOp;
     for (const auto &value : llvm::reverse(ctor.values())) {
       const Fortran::semantics::Symbol &compSym = *value.first;
-      while (!compIterator.lookup(compSym.name())) {
+      hlfir::EntityWithAttributes currentParent = varOp;
+      for (Fortran::lower::ComponentReverseIterator compIterator(
+               ctor.result().derivedTypeSpec());
+           !compIterator.lookup(compSym.name());) {
         const auto &parentType = compIterator.advanceToParentType();
         llvm::StringRef parentName = toStringRef(parentType.name());
         auto baseRecTy = mlir::cast<fir::RecordType>(

[jeanPerier]

Thanks for the fix. I liked the "sorted after semantic analysis" invariant to avoid the quadratic aspect, and I do not really get why it does not hold here. But given most structure constructors will likely have under a dozen components, the compiler performance point is likely moot, so if you think semantic cannot easily/cheaply guarantee the ordering, I am fine with this patch.

[jeanPerier]

Why was that assumption broken by the fact that the parent type was from a module in a different source file? It is true if the module is defined in the same source file.