IdentifierInfo is basically a uniqued string class, you can call Preprocessor::getIdentifierInfo() to look one up. The parser should have access to the preprocessor.
On Sat, Sep 6, 2014 at 2:03 PM, Ehsan Akhgari <[email protected]> wrote: > I started to look into this again, and I hit an issue that I'm not sure > how to solve. Creating a LabelDecl requires an IdentifierInfo > corresponding to the identifier token for the label name, but for these asm > labels, we use a different tokenizer on the LLVM side that doesn't know > about IdentifierInfo's at all. It looks like the IdentifierInfo argument > is used to construct a DeclarationName. I suppose one option that I have > here is to add a new NameKind to DeclarationName and make it possible to > construct a DeclarationName from that, but is there an easier option here? > > > On Thu, Jul 31, 2014 at 3:50 PM, Reid Kleckner <[email protected]> wrote: > >> On Thu, Jul 24, 2014 at 4:26 PM, Ehsan Akhgari <[email protected]> >> wrote: >> >>> On Thu, Jul 24, 2014 at 2:52 PM, Reid Kleckner <[email protected]> wrote: >>> >>>> ================ >>>> Comment at: lib/Parse/ParseStmtAsm.cpp:96 >>>> @@ -95,1 +95,3 @@ >>>> >>>> + void LookupInlineAsmLabel(StringRef &Identifier, llvm::SourceMgr >>>> &LSM, >>>> + llvm::SMLoc Location, bool Create) { >>>> ---------------- >>>> You can return a StringRef directly. >>>> >>>> ================ >>>> Comment at: lib/Sema/SemaDecl.cpp:1492 >>>> @@ -1491,1 +1491,3 @@ >>>> >>>> + HandleMSAsmLabelsOnScopePop(); >>>> + >>>> ---------------- >>>> I believe ActOnPopScope happens on any closing curly brace, so this >>>> won't do the "right" thing: >>>> void f() { >>>> bool failed; >>>> __asm retry: >>>> { >>>> failed = g(); >>>> } // The label is cleared here. >>>> if (failed) >>>> __asm jmp retry >>>> } >>>> >>>> Thinking more carefully, I don't think having the label map right on >>>> Sema works at all in the general case of nested functions. Consider this >>>> C++ example with inner classes: >>>> void f() { >>>> __asm some_label: >>>> struct A { >>>> static void g() { >>>> __asm jmp some_label ; This should jump forwards >>>> __asm some_label: >>>> __asm nop >>>> } >>>> }; >>>> } >>>> >>>> You can build similar test cases from C++11 lambdas and Obj-C blocks, >>>> and now OpenMP captured statements. >>>> >>>> We need to associate this label map with the function we're currently >>>> processing. Now that I'm considering the actual implementation >>>> difficulties, I'm going back to your "proposal #3" on PR20023, where we >>>> model these labels as regular label statements. That way, we reuse all the >>>> right name lookup machinery. We won't be able to 'goto' a label from an >>>> inline asm statement or 'jmp' to a non-asm label, so if you go this way, >>>> please diagnose both cases. You can probably add a bit to LabelDecl to >>>> track whether the label was from MS inline asm or regular source code. >>>> >>>> Apologies for the runaround, but I think it's probably better to >>>> rewrite along these lines. What do you think? >>>> >>> >>> Hmm, there are a number of problems with that approach. One is that we >>> need to break up __asm blocks and perhaps remember part of the parser state >>> until we hit the end of the function (such as the number of braces we have >>> seen, their source locations, etc.) which complicates the parsing code, and >>> actually needs us to do something at the end of the function too. It also >>> means that we won't be able to reuse LookupInlineAsmIdentifier, since >>> AFAICT labels are not Expr's. Also, it will mean that the __asm blocks >>> will be disjoint across labels, even after my __asm block joining patch. >>> >> >> I'm don't want to support going to labels in asm or jumping out: we >> shouldn't break up __asm blobs and introduce new LabelStmts. Instead, we >> should synthesize a LabelDecl with no associated LabelStmt and use the rest >> of the usual label machinery to insert it into the correct function scope. >> However, when we see 'goto label_from_asm', we'll have to add code to >> reject that. Presumably we can tell the two kinds of LabelDecl apart by >> the presence or absence of a LabelStmt. >> >> >>> It seems like your main objection to this approach is to storing the >>> label map on Sema, and that is quite fair (and I should add tests for >>> that!). But isn't a much easier solution to that to store the label map at >>> the right place? I've been looking at the code that handles LabelDecl's, >>> and I think we can get by with storing the map on Scope objects, and do >>> something similar to Sema::LookupOrCreateLabel to make sure we are >>> accessing the right function scope. I think with that change, everything >>> else in my patch will work properly. >>> >> >> That would work too, but I think you'll end up duplicating lots of the >> label processing logic if you go that way. >> >> It's probably also worth thinking about how to handle these kinds of test >> cases: >> >> void foo() { >> __asm jmp bar ; is 'bar' a function name or label? >> } >> void bar(); >> > > > > -- > Ehsan >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
