[PATCH] D101702: [clang-format] Add more support for C# 8 nullables

Tue May 4 00:14:58 PDT 2021

curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM. That's great. Thanks for working on this and for the cleanup.

================
Comment at: clang/lib/Format/FormatTokenLexer.cpp:102
+
+    // FIXME: Investigate what token type gives the correct operator priority.
+    if (tryMergeTokens(FatArrow, TT_JsFatArrow))
----------------
Could you explain in what case the operator precedence may be wrong here?

================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3195
+    // No space before null forgiving '!'.
+    if (Right.is(TT_JsNonNullAssertion))
+      return false;
----------------
exv wrote:
> curdeius wrote:
> > Should it be renamed to include C#?
> Now that you've mentioned it, I do see that JS also defines an almost duplicate set of null-related operators, including null-propagating, null-coalescing and null-coalescing assignment operators, and clang-format handles them with a different code path than C# 😅. They do have slightly different names, but the way they should be parsed and interpreted is the same, and I think it makes sense to use the same logic and token definitions for both of these languages. This resulted in a lot of special-handling for C# tokens that wasn't necessary being removed too.
> 
> However, renaming them would introduce a lot of noise, so I think I would prefer to do that in a separate commit.
Fair enough.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101702/new/

https://reviews.llvm.org/D101702