[cfe-commits] r54364 - in /cfe/trunk: Driver/ASTConsumers.cpp Driver/ASTConsumers.h Driver/clang.cpp include/clang/CodeGen/ModuleBuilder.h lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h lib/CodeGen/ModuleBuilder.cpp

[Daniel Dunbar]

My justification to Ted for the at least having the Release() style interface to 
the CodeGen interface is that it probably doesn't make sense to pass in a
module that isn't empty (since if it has things, and they matter, it can easily
break). 

Further, it is clear that at some point CodeGen needs to finish emission and
clean up its data structures, and calling Release provides a clear place to do
that (although HandleTranslationUnit would work too).

My preference would be to keep the new style of CodeGen interface and
fix the problem upstream. 


Having ParseAST not delete the Consumer may help, but it doesn't really solve
the problem if you suddenly want to layer more Consumers on top of each other.
In that case, the simplest thing is to just provide a Consumer which will stuff
the CodeGen Module* result somewhere for access later. I agree that this isn't
particularly aesthetically appealing however.

 - Daniel


----- Original Message ----
From: Matthijs Kooijman <matthijs at stdin.nl>
To: Ted Kremenek <kremenek at apple.com>
Cc: cfe-commits at cs.uiuc.edu
Sent: Thursday, August 7, 2008 4:23:30 AM
Subject: Re: [cfe-commits] r54364 - in /cfe/trunk: Driver/ASTConsumers.cpp Driver/ASTConsumers.h Driver/clang.cpp include/clang/CodeGen/ModuleBuilder.h lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h lib/CodeGen/ModuleBuilder.cpp

Hi Ted,

> Refactored driver logic for CodeGen into LLVMCodeGenWriter.  This
> ASTConsumer layers on top of LLVMCodeGen (another existing ASTConsumer) to
> emit bitcode files to disk.  This layering takes this logic out of clang.cpp
> and puts it directly into the ASTConsumer interface.  The benefit is that
> now --emit-llvm works with both serialized ASTs and regular source files.
This changes breaks clang for use in my own frontend.

What I want to do is make clang turn a C file into an llvm::Module* and then
do additional processing on that (running transformations and linking modules
together).

Previously, I would create a Module, pass it to CreateLLVMCodeGen, run
ParseAST and do my processing on my Module*. After this change, I can only
pass in a Module name and CodeGeneratorImpl will take care of creating the
Module for me.

However, to get at the module, I must now use ReleaseModule(), which I can't
call until after having run ParseAST (since ReleaseModule invalidates the
CodeGenerator). But, ParseAST deletes its consumer at the end of parsing, so I
can't call ReleaseModule after running ParseAST either (took me a while to
figure this one out, since it resulted in a very weird segfault).

So, if I want to keep using clang, I should make my own ASTConsumer, which
wraps a CodeGenerator to do its work, but doesn't destroy it. Or, as the
LLVMCodeGenWriter does, call ReleaseModule() in the destructor of my consumer
and makes the Module* available in some way or do all my postprocessing in
that destructor.

Somehow, none of these options seem like a good idea. I think the main problem
here is that ParseAST destroys the Consumer. This might be convenient in a lot
of cases, but could very well be left out. At the very least, this could be
made optional (I'll try this to at least get my stuff working again for now).

Alternatively, CodeGenerator could support both the old and new behaviour (ie,
also allow to pass in an existing Module* instead of just the module name).
However, I think the new way is somewhat nicer, so this is probably not the
best solution.

Gr.

Matthijs