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

[Eliza via Phabricator via cfe-commits]

exv added inline comments.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:1593
+                tok::kw_namespace, tok::r_paren, tok::r_square, tok::r_brace,
+                tok::kw_false, tok::kw_true, Keywords.kw_type, Keywords.kw_get,
+                Keywords.kw_set) ||
----------------
curdeius wrote:
> Does adding false and true here may negatively affect JS in any way? (Just thinking out loud)
Nothing I can see: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-0.html#non-null-assertion-operator


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3195
+    // No space before null forgiving '!'.
+    if (Right.is(TT_JsNonNullAssertion))
+      return false;
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101702