clang-format: better detection for multiplication operator

[Daniel Jasper]

I think this makes more sense for now. If we stumble upon bugs requiring
the fix for the other operators, we still have your other patch.


On Thu, Apr 3, 2014 at 10:58 PM, Johannes Kapfhammer <jkapfham at ethz.ch>wrote:

> On Sun, Mar 30, 2014 at 07:45:51PM +0200, Daniel Jasper wrote:
> > All in all, I think a much easier solution for the mentioned bug would be
> > to treat "<<" similar to how we treat assignments, i.e. if we find a <<,
> we
> > assume the RHS to be an expression (that's the very first "if" in
> > determineTokenType()).
> >
> Not sure if that's the right place.  The "if" branch you are talking
> about will set all preceding stars (on the same level) to
> TT_PointerOrReference:
>
> Before: cout << a *a << b *b << c *c;
> After:  cout << a *a << b *b << c * b;
> My Fix: cout << a * a << b * b << c * c;
>
> So we need the special care for operator<< at least in a new
> "else if".  I've added a "light" version of this patch below, note
> that it only handles operator<< and doesn't work with other operators.
> Doing it the way I did in the last mail has the additional benefit
> that it works for all operators and not just "<<" (in particular
> operator+ is hard to get otherwise).  But we probably won't need this.
>
> --
>
> From 16d81e7e3a0dedd5d51f963a134b26eb9c63b777 Mon Sep 17 00:00:00 2001
> From: Kapfhammer <kapf at student.ethz.ch>
> Date: Sun, 30 Mar 2014 12:04:17 +0200
> Subject: [PATCH] Use binary operators as an indicator of for an expression
> and
>  extend the test suite.
>
> This solves the issue where the star was too often interpreted as a
> pointer, e.g "cout<<a*b;" was formatted to "cout << a *b;" instead of
> "cout << a * b;".  By marking statements more often an expression, the
> function determineStarAmpUsage() is more reliable.
>
> The test suite was extended.
> ---
>  lib/Format/TokenAnnotator.cpp   |  5 +++++
>  unittests/Format/FormatTest.cpp | 15 +++++++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp
> index 0034235..7fbbdae 100644
> --- a/lib/Format/TokenAnnotator.cpp
> +++ b/lib/Format/TokenAnnotator.cpp
> @@ -681,6 +681,11 @@ private:
>            Previous->Type = TT_PointerOrReference;
>          }
>        }
> +    } else if (Current.is(tok::lessless) &&
> +               !Line.First->isOneOf(tok::kw_template, tok::kw_using) &&
>

How does this change the behavior in any way?


> +               (!Current.Previous ||
> +                Current.Previous->isNot(tok::kw_operator))) {
>

I wonder whether we can just use:

} else if (Current.is(tok::lessless) && !Line.MustBeDeclaration) { ..



> +      Contexts.back().IsExpression = true;
>      } else if (Current.isOneOf(tok::kw_return, tok::kw_throw)) {
>        Contexts.back().IsExpression = true;
>      } else if (Current.is(tok::l_paren) && !Line.MustBeDeclaration &&
> diff --git a/unittests/Format/FormatTest.cpp
> b/unittests/Format/FormatTest.cpp
> index 5395fd9..7478a23 100644
> --- a/unittests/Format/FormatTest.cpp
> +++ b/unittests/Format/FormatTest.cpp
> @@ -4625,6 +4625,21 @@ TEST_F(FormatTest, UnderstandsRvalueReferences) {
>    verifyGoogleFormat("#define WHILE(a, b, c) while (a && (b == c))");
>  }
>
> +TEST_F(FormatTest, FormatsMultiplicationOperator) {
> +  verifyFormat("operator*(type *a)");
> +  verifyFormat("operator<<(type *a)");
> +  verifyFormat("{ cout << (a * b); }");
> +  verifyFormat("{ cout << a * b; }");
> +  verifyFormat("{ cout << a * b << c * d; }");
> +  verifyFormat("type (*f)(type *a)");
> +  verifyFormat("type (&f)(type *a)");
> +  verifyFormat("void f(type *a)");
> +  verifyFormat("using f = void (*)(a *b)");
> +  verifyFormat("template <typename T = void (*)(a *b)");
> +  verifyFormat("using f = void (c::*)(a *b)");
> +  verifyFormat("template <typename T = void (c::*)(a *b)>");
>

There are many tests here that don't contain a "<<". What are they testing?

+}
>
+
>  TEST_F(FormatTest, FormatsBinaryOperatorsPrecedingEquals) {
>    verifyFormat("void f() {\n"
>                 "  x[aaaaaaaaa -\n"
> --
> 1.9.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140404/0959d76a/attachment.html>