[flang-commits] [flang] [flang][nfc] make size and offset optional so you can check if they have been set (PR #170931)

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-semantics

Author: Andre Kuhlenschmidt (akuhlens)

<details>
<summary>Changes</summary>

This PR makes the size field optional in symbols so that you can tell when it has been set. A later PR checks if a size is set before running some routines that will fail if it wasn't reasonable.

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


4 Files Affected:

- (modified) flang/include/flang/Semantics/symbol.h (+18-4) 
- (modified) flang/lib/Semantics/compute-offsets.cpp (+6) 
- (modified) flang/lib/Semantics/data-to-inits.cpp (+1-1) 
- (modified) flang/lib/Semantics/symbol.cpp (+2-2) 


``````````diff
diff --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h
index 95efe1ae2bd5e..b2bfc6102349a 100644
--- a/flang/include/flang/Semantics/symbol.h
+++ b/flang/include/flang/Semantics/symbol.h
@@ -874,10 +874,24 @@ class Symbol {
   Scope *scope() { return scope_; }
   const Scope *scope() const { return scope_; }
   void set_scope(Scope *scope) { scope_ = scope; }
-  std::size_t size() const { return size_; }
+  std::optional<std::size_t> sizeOpt() const { return size_; }
+  std::size_t size() const {
+    // NOTE: I tried to insert the following check, but there are places where
+    // we are relying on the fact that the size is 0 if it is not set.
+    // CHECK(size_.has_value());
+    return size_.value_or(0);
+  }
   void set_size(std::size_t size) { size_ = size; }
-  std::size_t offset() const { return offset_; }
+  void set_size(std::optional<std::size_t> size) { size_ = size; }
+  std::optional<std::size_t> offsetOpt() const { return offset_; }
+  std::size_t offset() const {
+    // NOTE: I tried to insert the following check, but there are places where
+    // we are relying on the fact that the offset is 0 if it is not set.
+    // CHECK(offset_.has_value());
+    return offset_.value_or(0);
+  }
   void set_offset(std::size_t offset) { offset_ = offset; }
+  void set_offset(std::optional<std::size_t> offset) { offset_ = offset; }
   // Give the symbol a name with a different source location but same chars.
   void ReplaceName(const SourceName &);
   static std::string OmpFlagToClauseName(Flag ompFlag);
@@ -986,8 +1000,8 @@ class Symbol {
   Attrs implicitAttrs_; // subset of attrs_ that were not explicit
   Flags flags_;
   Scope *scope_{nullptr};
-  std::size_t size_{0}; // size in bytes
-  std::size_t offset_{0}; // byte offset in scope or common block
+  std::optional<std::size_t> size_; // size in bytes
+  std::optional<std::size_t> offset_; // byte offset in scope or common block
   Details details_;
 
   Symbol() {} // only created in class Symbols
diff --git a/flang/lib/Semantics/compute-offsets.cpp b/flang/lib/Semantics/compute-offsets.cpp
index 1c48d33549a2e..1e557121de434 100644
--- a/flang/lib/Semantics/compute-offsets.cpp
+++ b/flang/lib/Semantics/compute-offsets.cpp
@@ -211,6 +211,8 @@ void ComputeOffsetsHelper::Compute(Scope &scope) {
     }
   }
   for (auto &[symbol, dep] : dependents_) {
+    // We are occassionally relying offset here to return zero when
+    // it has not been set yet.
     symbol->set_offset(dep.symbol->offset() + dep.offset);
     if (const auto *block{FindCommonBlockContaining(*dep.symbol)}) {
       symbol->get<ObjectEntityDetails>().set_commonBlock(*block);
@@ -281,6 +283,7 @@ void ComputeOffsetsHelper::DoCommonBlock(Symbol &commonBlock) {
       } else {
         eqIter = equivalenceBlock_.find(base);
         base.get<ObjectEntityDetails>().set_commonBlock(commonBlock);
+        CHECK(symbol.offsetOpt().has_value());
         base.set_offset(symbol.offset() - dep.offset);
         previous.emplace(base);
       }
@@ -393,11 +396,14 @@ std::size_t ComputeOffsetsHelper::ComputeOffset(
 
 std::size_t ComputeOffsetsHelper::DoSymbol(
     Symbol &symbol, std::optional<const size_t> newAlign) {
+  // Symbols such as the main program and modules.
   if (!symbol.has<ObjectEntityDetails>() && !symbol.has<ProcEntityDetails>()) {
     return 0;
   }
   SizeAndAlignment s{GetSizeAndAlignment(symbol, true)};
   if (s.size == 0) {
+    // FIXME: Errors that prevent us from computing the symbols size or
+    // variables that really require no storage should be set here.
     return 0;
   }
   std::size_t previousOffset{offset_};
diff --git a/flang/lib/Semantics/data-to-inits.cpp b/flang/lib/Semantics/data-to-inits.cpp
index bbf3b28fe03e6..831d2cc77eb81 100644
--- a/flang/lib/Semantics/data-to-inits.cpp
+++ b/flang/lib/Semantics/data-to-inits.cpp
@@ -786,7 +786,7 @@ static bool CombineEquivalencedInitialization(
     combinedSymbol.set(Symbol::Flag::CompilerCreated);
     inits.emplace(&combinedSymbol, std::move(combined));
     auto &details{combinedSymbol.get<ObjectEntityDetails>()};
-    combinedSymbol.set_offset(first.offset());
+    combinedSymbol.set_offset(first.offsetOpt());
     combinedSymbol.set_size(bytes);
     std::size_t minElementBytes{
         ComputeMinElementBytes(associated, foldingContext)};
diff --git a/flang/lib/Semantics/symbol.cpp b/flang/lib/Semantics/symbol.cpp
index ed0715a422e78..8289e5afa48f7 100644
--- a/flang/lib/Semantics/symbol.cpp
+++ b/flang/lib/Semantics/symbol.cpp
@@ -723,8 +723,8 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &os, const Symbol &symbol) {
   if (!symbol.flags().empty()) {
     os << " (" << symbol.flags() << ')';
   }
-  if (symbol.size_) {
-    os << " size=" << symbol.size_ << " offset=" << symbol.offset_;
+  if (symbol.size_ && *symbol.size_) {
+    os << " size=" << symbol.size_ << " offset=" << symbol.offset();
   }
   os << ": " << symbol.details_;
   return os;

``````````

</details>


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