[llvm-commits] [cfe-commits] TableGen backend API refactoring review request

[Sean Silva]

Ok, these patches address all of your points.

1-llvm.patch is the initial changes to the llvm tree. It changes the
backend api for llvm/utils/TableGen/* and it makes some very minor
non-breaking changes to include/llvm/TableGen/TableGenBackend.h and
lib/TableGen/TableGenBackend.h be make emitSourceFileHeader available as a
free function (the member function still exists to not break Clang's
TableGen)
2-clang.patch changes Clang's TableGen to the new API
3-llvm.patch removes struct TableGenBackend and all remnants of the old API.

I built `check` and `clang-test` after applying each one against ToT of the
respective repos; all green.

--Sean Silva

On Wed, Jun 6, 2012 at 1:42 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk>wrote:

>
> On Jun 4, 2012, at 11:34 PM, Sean Silva <silvas at purdue.edu> wrote:
>
> > 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?).
>
> Please try to avoid revlock like that. You can just leave struct
> TableGenBackend around until it isn't used any longer.
>
> Also, split the changes to lib/TableGen and include/llvm/TableGen into a
> third patch.
>
> > 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.
>
> There's really no need for that. Backends don't go in libraries. Just
> stick everything in 'llvm'.
>
> > 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.
>
> Leave them as is.
>
> /jakob
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120608/9a4b27e6/attachment.html>