Address comments

Author: clementval

flang/include/flang/ISO_Fortran_binding.h

@@ -146,11 +146,15 @@ extern "C++" template <typename T> struct FlexibleArray : T {
   CFI_rank_t rank; /* [0 .. CFI_MAX_RANK] */ \
   CFI_type_t type; \
   CFI_attribute_t attribute; \
-  /* The lsb of this field indicates the presence of the f18Addendum. Other \
-   * bits are used to specify the index of the allocator used to managed \
-   * memory of the data hold by the descriptor. */ \
+  /* This encodes both the presence of the f18Addendum and the index of the \
+   * allocator used to managed memory of the data hold by the descriptor. */ \
   unsigned char extra;
 
+/* Number of bits used to encode the addendum presence flag */
+#define _CFI_ADDENDUM_BITS 1
+/* Value of the addendum presence flag */
+#define _CFI_ADDENDUM_FLAG 1
+
 typedef struct CFI_cdesc_t {
   _CFI_CDESC_T_HEADER_MEMBERS
 #ifdef __cplusplus
@@ -160,8 +164,15 @@ typedef struct CFI_cdesc_t {
 #endif
 
 #ifdef __cplusplus
-  RT_API_ATTRS inline bool HasAddendum() const { return extra & 1; }
-  RT_API_ATTRS inline int GetAllocIdx() const { return ((int)extra >> 1); }
+  RT_API_ATTRS inline bool HasAddendum() const {
+    return extra & _CFI_ADDENDUM_FLAG;
+  }
+  RT_API_ATTRS inline void SetHasAddendum() {
+    extra = extra | _CFI_ADDENDUM_FLAG;
+  }
+  RT_API_ATTRS inline int GetAllocIdx() const {
+    return ((int)extra >> _CFI_ADDENDUM_BITS);
+  }
 #endif
 } CFI_cdesc_t;
 

flang/include/flang/Runtime/descriptor.h

@@ -92,8 +92,8 @@ class Dimension {
 // The storage for this object follows the last used dim[] entry in a
 // Descriptor (CFI_cdesc_t) generic descriptor.  Space matters here, since
 // descriptors serve as POINTER and ALLOCATABLE components of derived type
-// instances.  The presence of this structure is implied by the lsb of
-// CFI_cdesc_t.extra set to 1, and the number of elements in the len_[]
+// instances.  The presence of this structure is encoded in the
+// CFI_cdesc_t.extra field, and the number of elements in the len_[]
 // array is determined by derivedType_->LenParameters().
 class DescriptorAddendum {
 public:

flang/runtime/descriptor.cpp

@@ -52,7 +52,7 @@ RT_API_ATTRS void Descriptor::Establish(TypeCode t, std::size_t elementBytes,
     }
   }
   if (addendum) {
-    raw_.extra = raw_.extra | 1;
+    raw_.SetHasAddendum();
   }
   DescriptorAddendum *a{Addendum()};
   RUNTIME_CHECK(terminator, addendum == (a != nullptr));
@@ -297,6 +297,8 @@ void Descriptor::Dump(FILE *f) const {
   std::fprintf(f, "  type      %d\n", static_cast<int>(raw_.type));
   std::fprintf(f, "  attribute %d\n", static_cast<int>(raw_.attribute));
   std::fprintf(f, "  extra     %d\n", static_cast<int>(raw_.extra));
+  std::fprintf(f, "    addendum  %d\n", static_cast<int>(raw_.HasAddendum()));
+  std::fprintf(f, "    alloc_idx %d\n", static_cast<int>(raw_.GetAllocIdx()));
   for (int j{0}; j < raw_.rank; ++j) {
     std::fprintf(f, "  dim[%d] lower_bound %jd\n", j,
         static_cast<std::intmax_t>(raw_.dim[j].lower_bound));

flang/unittests/Evaluate/ISO-Fortran-binding.cpp

@@ -113,7 +113,6 @@ static void check_CFI_establish(CFI_cdesc_t *dv, void *base_addr,
         }
       }
     }
-    MATCH(0, res->raw().extra);
     if (type == CFI_type_struct || type == CFI_type_char ||
         type == CFI_type_char16_t || type == CFI_type_char32_t ||
         type == CFI_type_other) {