On Tue, Aug 25, 2009 at 3:39 AM, David Chisnall<[email protected]> wrote: > On 25 Aug 2009, at 08:20, Daniel Dunbar wrote: > >> On Thu, Aug 20, 2009 at 5:23 AM, David Chisnall<[email protected]> >> wrote: >>> >>> On 20 Aug 2009, at 07:15, Chris Lattner wrote: >>> >>>> I admit that the original check for darwin was completely hideous here, >>>> but this is making it a lot worse. Can this be factored out into >>>> targetInfo >>>> somehow? At the very least, can this use llvm::Triple to do this >>>> decoding? >>> >>> >>> Agreed - very ugly. The attached patch makes it much less so. Since we >>> were already passing the Context, I've moved all of the >>> architecture-specific tweaking inside the constructor and modified it to >>> use >>> llvm::Triple. >> >> Oops, I missed this patch. However, I since fixed this to basically >> have the same effect, although its still outside the constructor. > > Would it make more sense to move it into the constructor? It still seems > messy having IA32-specific stuff in the generic code.
I personally prefer all the policy decisions to be grouped together, and to not have constructors do real work or make descisions. For example, if we had a command line argument for this I would consider it wrong to need to pass LangOptions in to the constructor. - Daniel _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
