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 <[EMAIL PROTECTED]> To: Ted Kremenek <[EMAIL PROTECTED]> Cc: [email protected] 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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
