[PATCH] ms-inline-asm: Scope inline asm labels to functions

[Ehsan Akhgari]

================
Comment at: include/clang/AST/Decl.h:312
@@ -310,1 +311,3 @@
   LabelStmt *TheStmt;
+  SmallString<16> MSAsmName;
+  bool MSAsmNameResolved;
----------------
rnk wrote:
> ehsan wrote:
> > rnk wrote:
> > > This will cause a memory leak for names > 16 chars. When the ASTContext is destroyed, it doesn't actually call destructors of individual AST nodes, it merely frees the memory. This should be a StringRef that points to memory from 'new (Context, 1) char[Len]', probably.
> > Oh, I didn't know that!  What's this 'new (Contex, 1) char[Len]' trick?  I'm not familiar with it, I don't think...
> It's an operator new overload declared in ASTContext.h. Basically, it's equivalent to 'new char[Len]', except it goes calls the 'void* operator new(size_t Bytes, ASTContext &Context, size_t Alignment)' overload.
Hmm, who is responsible for freeing this memory?  The comments in ASTContext.h seem to suggest that I should call ASTContext::Deallocate on the pointer, but I'm not sure when I should do that if the destructor is not invoked.  Looking at other parts of the code base, it seems like nobody worries about deleting memory allocated this way...

================
Comment at: lib/Sema/SemaStmt.cpp:2357-2358
@@ +2356,4 @@
+  if (TheDecl->isMSAsmLabel()) {
+    // TODO Move this diagnostic elsewhere so that we get to examine the goto
+    // statements after we've seen all of the body of the current scope.
+    StmtResult error(StmtError(Diag(GotoLoc, diag::err_goto_ms_asm_label)
----------------
rnk wrote:
> ehsan wrote:
> > rnk wrote:
> > > Does it help if you issue the same diagnostic when processing 'foo:' in this example?
> > >   void f() {
> > >     goto foo;
> > >     __asm {
> > >     foo:
> > >       nop
> > >     }
> > >   }
> > > 
> > > After issuing the diagnostic, you can avoid the "undeclared label" knock-on errors by pretending that you did in fact define the label.
> > I cannot issue the diagnostic there because I will not have a GotoStmt pointer when processing `foo:`.  That's really the main issue.
> > 
> > What I would ideally do would be to not issue any diagnostics in ActOnGotoStmt, and wait until the scope is fully processed, then visit all of the GotoStmts in the scope, check their LabelDecls to see if they're asm labels and issue the diagnostic there.  But I don't know where I would hook into in order to be able to visit all of the GotoStmts at the end of a scope.
> Looks like we have JumpDiagnostics.cpp for this purpose... See if you can work it in there?
Sure.

http://reviews.llvm.org/D4589