blink_gc_plugin: Make RecordInfo::Get{Fields,Bases} return deterministic order

tools/clang/blink_gc_plugin/RecordInfo.h

Index: tools/clang/blink_gc_plugin/RecordInfo.h
diff --git a/tools/clang/blink_gc_plugin/RecordInfo.h b/tools/clang/blink_gc_plugin/RecordInfo.h
index 1a25d8342b8a22b5c605079e4a1637be09ecaf5a..c8a4d466d226690dbee5a8a9f5f2e35aded91a51 100644
--- a/tools/clang/blink_gc_plugin/RecordInfo.h
+++ b/tools/clang/blink_gc_plugin/RecordInfo.h
@@ -69,16 +69,22 @@ class FieldPoint : public GraphPoint {
 // Wrapper class to lazily collect information about a C++ record.
 class RecordInfo {
  public:
-  typedef std::map<clang::CXXRecordDecl*, BasePoint> Bases;

[Nico, 2016/01/29 03:10:08]

would giving the map a custom comparator that compares on something more
deterministic that the pointer address help as well?

[hans, 2016/01/29 04:49:14]

For Bases, a map doesn't seem to buy as anything at all, since the callers of
GetBases() just want to iterate over all of them anyway. A vector is more
efficient for that.

For Fields, a map with a comparator that compares the Decls' SourceLocations
would work. I suppose the best solution depends on what the primary use is of
the data structure. Is it to iterate over all the fields, or to look up a
specific one? The code does both, but I don't know it well enough to say what's
more important.

Using LLVM's MapVector could also be a good alternative.

I don't care much how we solve this, as long as the iteration order is
deterministic. I suppose it's up to sof :-)

[sof, 2016/01/29 09:00:19]

What you propose makes the most sense to me, use a std::vector for bases, but a
map with a Compare that orders on getLocStart(), or something. Not a very
expensive node comparison nor will the underlying tree come out the worse for
it.

-  typedef std::map<clang::FieldDecl*, FieldPoint> Fields;
+  typedef std::pair<clang::CXXRecordDecl*, BasePoint> Base;
+  typedef std::vector<Base> Bases;
+  typedef std::pair<clang::FieldDecl*, FieldPoint> Field;
+  typedef std::vector<Field> Fields;
   typedef std::vector<const clang::Type*> TemplateArgs;
 
   ~RecordInfo();
 
   clang::CXXRecordDecl* record() const { return record_; }
   const std::string& name() const { return name_; }
+
   Fields& GetFields();
+  bool HasField(clang::FieldDecl*);
+  Field& GetField(clang::FieldDecl*);
   Bases& GetBases();
+
   clang::CXXMethodDecl* GetTraceMethod();
   clang::CXXMethodDecl* GetTraceDispatchMethod();
   clang::CXXMethodDecl* GetFinalizeDispatchMethod();
@@ -115,8 +121,8 @@ class RecordInfo {
 
   void walkBases();
 
-  Fields* CollectFields();
-  Bases* CollectBases();
+  void CollectFields();
+  void CollectBases();
   void DetermineTracingMethods();
   bool InheritsTrace();
 
@@ -128,6 +134,7 @@ class RecordInfo {
   TracingStatus fields_need_tracing_;
   Bases* bases_;
   Fields* fields_;
+  std::map<clang::FieldDecl*, size_t>* field_map_;
 
   enum CachedBool { kFalse = 0, kTrue = 1, kNotComputed = 2 };
   CachedBool is_stack_allocated_;