EricWF marked an inline comment as done.
EricWF added a comment.
@alexfh Thanks for correcting the reviewers. I had no idea who to add so I used
something like `git-suggest-reviewers`.
================
Comment at: lib/Format/Format.cpp:1958
LangOpts.DeclSpecKeyword = 1; // To get __declspec.
+ LangOpts.CoroutinesTS = 1;
return LangOpts;
----------------
djasper wrote:
> Can we change this only if Style.isCpp()? We should probably do that for
> other things, too, but as we are adding more keywords here, lets not do that
> for Java/JS/Proto.
SGTM. I'll make the change.
================
Comment at: lib/Format/TokenAnnotator.cpp:589
}
+ if (Style.isCpp())
+ if (CurrentToken && CurrentToken->is(tok::kw_co_await))
----------------
djasper wrote:
> if (Style.isCpp() && CurrentToken && ..)
> next();
The `Style.isCpp()` should be unneeded now that the Coroutines TS keywords only
parse when the style is Cpp.
================
Comment at: lib/Format/TokenAnnotator.cpp:2267
+ return true;
+ if (Right.is(tok::l_paren) && Left.is(tok::kw_co_await) &&
+ Left.Previous && Left.Previous->is(tok::kw_operator))
----------------
djasper wrote:
> I know that the split between spaceRrequiredBefore and spaceRequiredBetween
> is generally bad. However, here you seem to be testing the exact same thing
> that is then retested in spaceRequiredBetween. I'd prefer the logic to be in
> one place only. Which tests break if you remove the changes to
> spaceRequiredBetween?
> Which tests break if you remove the changes to spaceRequiredBetween?
Not sure I have a test, I was probably mistaken adding this.
@djasper Assuming only one set of changes is needed, would they be better in
`spaceRequiredBetween` or `spaceRequiredBefore`?
================
Comment at: lib/Format/UnwrappedLineParser.cpp:433
case tok::kw___try:
+ assert(!Tok->is(tok::kw_co_await));
if (!LBraceStack.empty() && LBraceStack.back()->BlockKind == BK_Unknown)
----------------
djasper wrote:
> EricWF wrote:
> > This change is accidental.
> What does that mean? Remove it?
Sorry, I meant to remove it, I just couldn't when I left the comment as a
reminder.
https://reviews.llvm.org/D34225
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
