[cfe-commits] TableGen backend API refactoring review request

[Sean Silva]

The two attached patches change the TableGen backend API (of LLVM and clang
respectively). They are git diff'd against current ToT. They must both be
applied "atomically" because the Clang changes depend on LLVM header
changes (is there a way to avoid this problem without leaving in the old
code in with a "don't touch, will be removed soon" comment?).


### Description of the changes

These patches change the TableGen backend API as described in <
http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-May/049530.html> (I'm
working on documentation component, but I'm planning on having that be a
separate patch, submitted after the one that actually changes the API is
committed). The new backend API is just:

EmitFoo(RecordKeeper &, raw_ostream & /*, anything else you need */);

which is simpler and more flexible than the current one. Another benefit is
that backends can be in their own .cpp file, without needing a pointless
stub header; additionally, everything inside the .cpp file can be put in an
anonymous namespace except EmitFoo.

Although the patches are >2K lines each, it is mostly mechanical
refactoring churn. Generally, the following changes were made:
* remove header files whose only use was to declare classes inheriting from
TableGenBackend. if the class was literally just a stub around the
FooEmitter::run method, I just made the `run` method become an `EmitFoo`
function and deleted the stub class. if there was a more substantial class
that holds state for the emission process, I brought the declaration down
into the .cpp file and then made EmitFoo just be a stub calling
FooEmitter(Records).run(OS).
* centralize the declaration of all of the TableGen backends into
{llvm,clang}/utils/TableGen/TableGenBackends.h
* clean out include/llvm/TableGen/TableGenBackend.h. remove class
TableGenBackend, with accompanying changes to TableGenBackend.cpp.
EmitSourceFileHeader was preserved, but made a freestanding function with
name changed to start with a lowercase letter.
* made appropriate changes to {llvm,clang} TableGen.cpp files to use the
new API.
* within the backends, I wrapped stuff in an anonymous namespace as I saw
opportunities to do so that the new API afforded and I was "in the area".

I built the "check" and "clang-test" targets with these changes applied and
they both pass (are there other test targets i should run?).


### Specific feedback requests

I'm in the air about where to declare the backends namespacewise. In the
LLVM TableGen (which I converted first), I decided to put all the backends
inside a namespace "tblgen_backends" inside namespace llvm, which turns out
to be a pain in the ass, since you have to write

namespace llvm {
namespace tblgen_backends{
EmitFoo(RecordKeeper &RK, raw_ostream &OS) {
  /* emit stuff */
}
} // End llvm namespace
} // End tblgen_backends namespace

inside the backend's .cpp file. In the clang TableGen, I decided to go
leaner and just have the backends in namespace "tblgen_backends". I'm
considering also just having the backends be in the global namespace, since
it doesn't really matter that much and is more convenient to write.

Another area that I have questions is whether the filenames should be
changed, since they are all called FooEmitter.cpp, but sometimes a class
FooEmitter doesn't exist anymore. My general feeling is that it doesn't
really matter, so best to just leave them. This is easily changed in a
future patch if it is deemed desirable.

These patches are hopefully pretty much ready to go into trunk besides the
issues I just mentioned. Some general nitpicking would also be appreciated.

--Sean Silva
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120604/385bbeca/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm.patch
Type: application/octet-stream
Size: 88587 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120604/385bbeca/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang.patch
Type: application/octet-stream
Size: 88595 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120604/385bbeca/attachment-0001.obj>