Page MenuHomePhabricator

[clang-format] Fix lambdas returning template specialization that contains operator in parameter
ClosedPublic

Authored by jkorous on Mar 4 2019, 3:43 PM.

Details

Summary

Inspired by https://reviews.llvm.org/D58922

Since this code compiles I assume we should add those tokens to switch:

template<int> struct foo {};

int main() {
  { auto lambda = []() -> foo<5+2> { return {}; }; }
  { auto lambda = []() -> foo<5-2> { return {}; }; }
  { auto lambda = []() -> foo<5/3> { return {}; }; }
  { auto lambda = []() -> foo<5%2> { return {}; }; }
  { auto lambda = []() -> foo<5<<2> { return {}; }; }
  { auto lambda = []() -> foo<!false> { return {}; }; }
  { auto lambda = []() -> foo<~5> { return {}; }; }
  { auto lambda = []() -> foo<5|2> { return {}; }; }
  { auto lambda = []() -> foo<5||2> { return {}; }; }
  { auto lambda = []() -> foo<5&2> { return {}; }; }
  { auto lambda = []() -> foo<5&&2> { return {}; }; }
  { auto lambda = []() -> foo<5==2> { return {}; }; }
  { auto lambda = []() -> foo<5!=2> { return {}; }; }
  { auto lambda = []() -> foo<5>=2> { return {}; }; }
  { auto lambda = []() -> foo<5<2> { return {}; }; }
  { auto lambda = []() -> foo<5<=2> { return {}; }; }
  { auto lambda = []() -> foo<2 ? 1 : 0> { return {}; }; }
}

Diff Detail

Event Timeline

jkorous created this revision.Mar 4 2019, 3:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2019, 3:43 PM
MyDeveloperDay accepted this revision.Mar 4 2019, 11:51 PM

I'm not sure I personally would ever write code like that ;-) , but if its legal C++ and it compiles we should handle it the same as foo<1>,foo<true>,foo<!true>

As there are a number of reviews out there for formatting Lambdas I think its a good idea for us to add corner cases like this to the unit tests, but it does get me thinking if this shouldn't be handled by a piece of code which knows about trailing return types (template or otherwise) and not be in the general Lambda parsing code

I suspect that given that the switch statement handles

tok::identifier, tok::less, tok::numeric_constant, tok::greater
foo            <                          1              >

We are effectively just consuming the return type tokens.

But to handle what you have here it LGTM and handles more use cases that https://bugs.llvm.org/show_bug.cgi?id=40910 would throw up.

Thanks for helping out

This revision is now accepted and ready to land.Mar 4 2019, 11:51 PM

I'm not sure I personally would ever write code like that ;-) , but if its legal C++ and it compiles we should handle it the same as foo<1>,foo<true>,foo<!true>

I hear you :D

As there are a number of reviews out there for formatting Lambdas I think its a good idea for us to add corner cases like this to the unit tests, but it does get me thinking if this shouldn't be handled by a piece of code which knows about trailing return types (template or otherwise) and not be in the general Lambda parsing code

I suspect that given that the switch statement handles

tok::identifier, tok::less, tok::numeric_constant, tok::greater
foo            <                          1              >

We are effectively just consuming the return type tokens.

But to handle what you have here it LGTM and handles more use cases that https://bugs.llvm.org/show_bug.cgi?id=40910 would throw up.

Thanks for helping out

I had the same exact idea but since I wasn't able to find any existing function I just followed your lead for now. But I assume we can just factor out this later.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 5 2019, 11:26 AM