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