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

Peter Klausler via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 3 15:34:31 PST 2021


klausler added inline comments.


================
Comment at: flang/include/flang/Semantics/symbol.h:600
+    // For maps of symbols: collate them by source location
     return name_.begin() < that.name_.begin();
   }
----------------
tskeith wrote:
> I don't like keeping an `operator<` that is only valid in some cases. It took a long time to find the bug with `SymbolSet` and `errorSymbols_` and the presence of this function makes it easy to introduce another similar bug.
> 
> I think it would be better not to provide a default ordering of symbols, or to provide one that is always valid. Ordering by symbol address is valid, though may result in an unexpected order. Or we could add a serial number to symbols and sort them using that, i.e. in order of creation.
I concur, and will soon propose replacing SymbolSet with multiple types whose collating order (if any) is more clear.

Most SymbolSet use cases really just need to be an unordered collection of Symbol pointers.  The other cases might be better served as maps and multimaps from name positions or name values to Symbols.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97774



More information about the llvm-commits mailing list