clang-format, recognizing alternative binary operator tokens

Hello,

clang-format does a really nice job breaking lines with boolean binary
operators, e.g., && and ||.

The C++ standard provides alternative keywords for these operators
(and, or). I think clang-format should be able to treat these
alternative keywords the same as the corresponding binary operators.
However, with these alternative keywords, clang-format makes
completely different whitespace choices. For example, using
clang-format 3.5 from svn r207601, with the default LLVM style options

void foo(void) {
  if (call_some_function() >
      0 and(some_other_function_result() ==
            0 or yet_another_result or(something_else == 1))) {
  }

  if (call_some_function() > 0 &&
      (some_other_function_result() == 0 || yet_another_result ||
       (something_else == 1))) {
  }
}

I did some investigation to see what might be causing this behavior.
It boils down to the LangOptions returned by getFormattingOptions() in
Format.cpp line 1848. If I add

LangOpts.CXXOperatorNames = 1;

to that function, then clang-format outputs the much more pleasing

void foo(void) {
  if (call_some_function() > 0 and
      (some_other_function_result() == 0 or yet_another_result or
       (something_else == 1))) {
  }

  if (call_some_function() > 0 &&
      (some_other_function_result() == 0 || yet_another_result ||
       (something_else == 1))) {
  }
}

Of course, this should probably not always be enabled.

One option would be to only enable this option if LS_Cpp03 or LS_Cpp11
were chosen. But C users #including <iso646.h> might want this option,
and not all C++ users would want it (since for example, MSVC++ does
not recognize these unless using standards compliant mode).

So another option would be to add a format style option to selectively
enable this behavior.

Would such a change to clang-format be welcome? If so, what would be
the preferred implementation?

Thanks,

Could no-operator-names be on by default?

This seems like a useful enhancement. Why don't you either a) file a bug, or b) propose a patch?

I would suggest that we should default to recognizing the keyword whenever it's legal for the language being compiled. We shouldn't need to opt-in to standard defined language features. I don't really see any downside to having it enabled. If we support a MSVC++ compatibility mode (do we?), it should be controlled by that flag.

Philip

These operators are always available in Clang's C++ mode, so IMO
clang-format should enable this LangOption by default. There doesn't
appear to be a flag for turning this off in C++.

MSVC requires that you include <iso646.h> if you want to use these:
http://msdn.microsoft.com/en-us/library/w6b8ws6z.aspx

I don't think we need a flag in clang-format to turn it off.

Thanks everyone for the feedback.

It sounds like folks are ok with enabling the keyword binary operators
by default, and not adding a new option to clang-format.

Do we want to enable this even if "Language" is not "Cpp" in the Style options?

One thing that I find confusing: in the same function,
LangOpts.CPlusPlus is always set to 1. Does this mean that
clang-format always lexes all source files using C++ keywords? Even if
they are C or ObjC files?

I'd be happy to submit a patch, but I'd like to get the exact behavior
nailed down beforehand, since I'm new to clang/llvm community.

Also, FWIW, you can also keep MSVC happy with the operator keywords by
passing /Za to the compiler.

Incidentally: that flag doesn't actually make MSVC handle these keywords
properly; they instead appear to be faked up by #defines (which in turn
breaks some versions of boost...)

Patch submitted to phabricator at http://reviews.llvm.org/D3634.