[PATCH] D97201: [flang] Detect circularly defined interfaces of procedures

[Pete Steinfeld via Phabricator via llvm-commits]

PeteSteinfeld added inline comments.


================
Comment at: flang/include/flang/Semantics/symbol.h:597
   bool operator!=(const Symbol &that) const { return !(*this == that); }
-  bool operator<(const Symbol &that) const {
-    // For sets of symbols: collate them by source location
-    return name_.begin() < that.name_.begin();
-  }
+  // For sets of symbols (SymbolSet): collate them by address since their
+  // source location can change
----------------
PeteSteinfeld wrote:
> tskeith wrote:
> > PeteSteinfeld wrote:
> > > klausler wrote:
> > > > Heap addresses are arbitrary and cannot be expected to have relationships that are portable.  This change is going to lead to spurious test failures esp. across platforms.  You're going to have to order the symbols via the actual characters in their names instead.
> > > Good catch.  Will do.
> > > Heap addresses are arbitrary and cannot be expected to have relationships that are portable.  This change is going to lead to spurious test failures esp. across platforms.  You're going to have to order the symbols via the actual characters in their names instead.
> > 
> > An alternative is to make `SymbolSet` be an `std::unordered_set` and explicitly sort the symbols when the order matters, e.g. in .mod files.
> > 
> I tried going down the path of making `SymbolSet` be unordered but the resulting changes were pretty messy, and I gave up in favor of another suggestion from Tim -- to record the initial value of the symbol's name and use that as the basis for the "<" operator.
After examining several alternatives, I've concluded that making SymbolSet unordered is the least disruptive solution.  More information in an update soon to follow.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97201/new/

https://reviews.llvm.org/D97201