[PATCH] D121678: [pseudo] Split greatergreater token.

[Sam McCall via Phabricator via cfe-commits]

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

This seems like the right approach to me.

It seems a bit strange to be doing this whether we're parsing C++ or not, but I guess we're not going to want low-level grammar differences between C and C++ if we can avoid it.



================
Comment at: clang/lib/Tooling/Syntax/Pseudo/Token.cpp:97
 
+TokenStream splitGreaterGreater(const TokenStream &Input) {
+  TokenStream Out;
----------------
What do you think about combining this into cook()?
Seems a bit wasteful to do a separate pass for this. (We can always split it out again later if cook() gets too complicated)


================
Comment at: clang/lib/Tooling/Syntax/Pseudo/Token.cpp:107
+        T.Data = T.text().data() + 1;
+        // FIXME: Line is wrong if the first greater is followed by an escaped
+        // newline.
----------------
I don't think we'll ever fix this, I'd drop the FIXME and add a `!` at the end :-)


================
Comment at: clang/lib/Tooling/Syntax/Pseudo/Token.cpp:112
+      }
+      assert(false && "split an uncook token stream!");
+    }
----------------
why not assert at the top and remove the if?

This can only be caused by a really bad programmer error, I don't think we should worry about defending against it making it into production.


================
Comment at: clang/lib/Tooling/Syntax/Pseudo/cxx.bnf:102
 fold-operator := <<
-fold-operator := >>
+#! Custom modification to split >> to two greater token.
+fold-operator := > >
----------------
we should rather indirect this change through a new nonterminal:

```
greater-greater := > >
fold-operator := greater-greater
```

This lets us document the change in one place, and makes it easier to avoid the false parses if they prove important. We could restrict the first rule to only match if the second token has a `mergeable` bit or something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121678