[LLVMdev] tblgen internals

[Chris Lattner]

On Dec 12, 2010, at 10:54 AM, Garrison Venn wrote:

> 
> Hey Chris,
> 
> The following patch removes all global references to a RecordKeeper instance for the tblgen
> utility. This effort was motivated by the FIXME remark, in TableGen.cpp. Although a few files 
> were touched, the main change was to Record.h.
> 
> The patch takes the simple approach of adding a RecordKeeper reference to TGParser, and 
> any needed emitter helper classes. In addition, since some of the classes, and methods in Record 
> reference the previously global RecordKeeper instance, I added another RecordKeeper 
> reference to Record, along with such a typed argument to Record's constructor, and a 
> corresponding factory method to RecordKeeper.

This looks like great progress to me, applied in r121659.

That said, it is suboptimal for Record to have to have a RecordKeeper& in it.  I haven't looked at the code in a long time, is it feasible to detangle that out of record, or is it not worth it?

Another random question: why is createRecord better than using "new Record"?  If createRecord is better, it would be good to make the Record ctor private so the code doesn't evolve into sometimes using one and sometimes using the other.

Some minor coding style things:

+  RecordKeeper &getRecords() const {
+    return(TrackedRecords);
+  }
+  Record *createRecord(const std::string &N, SMLoc loc) {
+    return(new Record(N, loc, *this));
+  }

No need for the parens around the return value.

+  RecordKeeper& getRecords() {
+    return(Records);
+  }
+
+  RecordKeeper& getRecords() const {
+    return(Records);
+  }

Ditto.  Also, if you have the later, you don't need the former version of this.  Also, please put a space before the & instead of after it, giving:

+  RecordKeeper &getRecords() const {
+    return Records;
+  }


+  /// Tracked Records
+  RecordKeeper& Records;

Same thing with &/space spacement.

Otherwise the patch looks great, thanks for working on this Garrison!

-Chris