erichkeane added inline comments.

================
Comment at: clang/lib/Parse/Parser.cpp:1306
+  bool Delete =
+      Tok.is(tok::equal) && NextToken().is(tok::kw_delete) ? true : false;
   Decl *Res = Actions.ActOnStartOfFunctionDef(getCurScope(), D,
----------------
rZhBoYao wrote:
> erichkeane wrote:
> > I'm not sure about doing this 'look ahead' here, this feels dangerous to 
> > me.  First, does this work with comments?  Second, it seems we wouldn't 
> > normally look at 'deleted' if SkipBody.ShouldSkip (see below with the early 
> > exit)?
> > 
> > Next I'm not a fan of double-parsing these tokens with this lookahead.  I 
> > wonder, if we should move hte logic from ~1334 and 1338 up here and 
> > calculate the 'deleted'/'defaulted' 'earlier', before we 
> > 'actOnStartOfFunctionDef`.  
> > 
> > This would result in us being able to instead change the signature of 
> > ActOnStartOfFunctionDef to take some enum as to whether it is 
> > deleted/defaulted, AND create the function decl as deleted/defaulted 'in 
> > place' (or, at least, call SetDeclDeleted or SetDeclDefaulted immediately).
> > 
> > 
> > 
> > 
> > I'm not sure about doing this 'look ahead' here, this feels dangerous to 
> > me. First, does this work with comments?
> Yes, it returns a normal token after phase 5, so comments are long gone.
> 
> > Second, it seems we wouldn't normally look at 'deleted' if 
> > SkipBody.ShouldSkip (see below with the early exit)?
> SkipBody.ShouldSkip is an output parameter of `ActOnStartOfFunctionDef`. We 
> need to either look ahead or consume "delete" before entering 
> `ActOnStartOfFunctionDef` anyway.
> 
> > Next I'm not a fan of double-parsing these tokens with this lookahead.
> It does look weird. Consume them I will. Updated diff coming.
> 
> > AND create the function decl as deleted/defaulted 'in place' (or, at least, 
> > call SetDeclDeleted or SetDeclDefaulted immediately).
> SetDecl{Deleted | Defaulted} needs KWLoc tho. I haven't thought of a way of 
> doing that "in place" inside `ActOnStartOfFunctionDef`.
My point is: do that parsing in this function BEFORE the call to 
ActOnStartOfFunctionDef?

Alternatively, we could add a function to Sema to 
'ActOnFunctionDefinition(DefType)' and move this diagnostic THERE instead of 
ActOnStartofFunctionDef, and call it AFTER we have handled the '= 
delete/=default/etc'.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122981/new/

https://reviews.llvm.org/D122981

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to