On Thu, Sep 5, 2013 at 4:26 PM, Eli Friedman <[email protected]> wrote:
> On Wed, Sep 4, 2013 at 10:10 AM, Reid Kleckner <[email protected]> wrote: > >> On Fri, Aug 30, 2013 at 10:45 AM, Reid Kleckner <[email protected]> wrote: >> >>> >>> >>> ================ >>> Comment at: lib/Sema/SemaLambda.cpp:101 >>> @@ -98,1 +100,3 @@ >>> + (isa<FunctionDecl>(CurContext) && >>> + Context.getTargetInfo().getCXXABI().isMicrosoft())) { >>> ManglingContextDecl = 0; >>> ---------------- >>> Eli Friedman wrote: >>> > Why do you need to number anything in a function which isn't inline? >>> CodeGen can mangle static variables etc. however it wants because they >>> aren't externally visible. >>> I'm using these to number bits in a bitfield, not to mangle. I have to >>> use this mechanism to number the bits of inline functions, so it makes >>> sense to use it to number bits of non inline functions too. Otherwise I >>> have to add back the CXXABIFunctionState code I had in the first version of >>> this patch. >>> >>> I'm having a very hard time extracting this function from Sema and >>> moving it over to AST/ItaniumCXXABI.cpp as John suggested. It accesses a >>> bunch of Sema fields. Do you have any thoughts on how this should look in >>> the long run? >>> >> >> Any thoughts on this? This is needed to avoid double initialization of >> llvm::outs/errs(), and factoring this one check out will require a massive >> migration of Eli's Itanium logic from Sema to AST. >> >> > The Itanium ABI code actually uses two numbering schemes: one for > externally-visible stuff, and one for non-exernally visible stuff. Not > sure if you want to go that route. See the beginning of the definition of > "class ItaniumMangleContext" for how it numbers non-externally-visible > stuff. The idea is that we can avoid tracking the numbering in the AST for > declarations where the numbers don't matter. (Also, it involved changing > less code when I implemented it.) > That makes sense. I'll number non-externally visible things in CodeGen and commit without the check in SemaLambda. Thanks John for the review! > If you want to, I'm fine with leaving that check as-is. > > (I haven't actually reviewed the rest of the patch.) > > -Eli >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
