Page MenuHomePhabricator

[hip] Fix ambiguity from `>>>` of CUDA.
ClosedPublic

Authored by hliao on May 1 2019, 1:12 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

hliao created this revision.May 1 2019, 1:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 1 2019, 1:12 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
tra added a reviewer: rsmith.May 1 2019, 1:14 PM
tra added a comment.May 1 2019, 1:22 PM

LGTM, but I've added @rsmith who is way more familiar with this code.

hliao added a comment.May 1 2019, 1:24 PM
In D61396#1486706, @tra wrote:

LGTM, but I've added @rsmith who is way more familiar with this code.

sure, no rush, let's wait for comments from @rsmith

yaxunl added a comment.May 1 2019, 2:11 PM

LGTM too. Thanks Michael for fixing this.

hliao added a comment.May 2 2019, 7:33 AM

@rsmith do you have the chance to review this simple fix?

hliao added a comment.May 6 2019, 9:54 AM

@rsmith Do you have the chance to review this patch?

hliao added a comment.May 7 2019, 5:23 AM

@rsmith Do you have the chance to review this patch?

rsmith added inline comments.May 7 2019, 12:40 PM
clang/lib/Parse/ParseTentative.cpp
597 ↗(On Diff #197624)

No need to check for CUDA here; that's a prerequisite for forming a greatergreatergreater token (and if we formed that token in any other language mode, we'd still want to split it here). But this should be gated by CPlusPlus11 for clarity, since that's where we get the "split >> tokens" rule from. So I think this should be:

} else if (Context == TypeIdAsTemplateArgument &&
           (Tok.isOneOf(tok::greater, tok::comma) ||
            (getLangOpts().CPlusPlus11 &&
             (Tok.isOneOf(tok::greatergreater, tok::greatergreatergreater) ||
              (Tok.is(tok::ellipsis) &&
               NextToken().isOneOf(tok::greater, tok::greatergreater,
                                   tok::greatergreatergreater, tok::comma)))))) {
hliao updated this revision to Diff 198529.May 7 2019, 2:20 PM

revise following reviewer's suggestion.

hliao marked an inline comment as done.May 7 2019, 2:22 PM

done

rsmith accepted this revision.May 7 2019, 2:29 PM
rsmith added inline comments.
clang/lib/Parse/ParseTentative.cpp
597 ↗(On Diff #198529)

A test for this case would be nice. Eg:

template<typename ...T> void bar() {
  S<S<S<T()...>>> s;
}
This revision is now accepted and ready to land.May 7 2019, 2:29 PM
hliao updated this revision to Diff 198545.May 7 2019, 3:13 PM

Add one unrealistic case for test purpose only.

hliao marked an inline comment as done.May 7 2019, 3:14 PM

done

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 7 2019, 5:52 PM