rZhBoYao marked an inline comment as done.
rZhBoYao 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:
> > 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'.
> > do that parsing in this function BEFORE the call to ActOnStartOfFunctionDef?
> But we need Decl *Res returned by ActOnStartOfFunctionDef.
>
> I did try to factor out the diagnostic right after `ActOnFunctionDefinition`
> and 27 tests were failing.
> Furthermore, we are not the only caller of `ActOnFunctionDefinition`.
I meant to say `ActOnStartOfFunctionDef`.
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
