[PATCH] D124749: [clang-format] Handle Verilog preprocessor directives

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D124749#3514224 <https://reviews.llvm.org/D124749#3514224>, @sstwcw wrote:

> The two parents of this revision change the same file, so the build bot says patch does not apply.  Does that mean I have to submit the parent patches with less context?

Maybe just wait until one of the parents has landed. We can make a review without the bot. :)
Or put the parents in relation.

In D124749#3490834 <https://reviews.llvm.org/D124749#3490834>, @MyDeveloperDay wrote:

> You add significant number of keywords here but I don't see any of them being tested? can you add a unit tests to cover what you are adding

I think this is still open?



================
Comment at: clang/lib/Format/FormatTokenLexer.cpp:1137
+  // normal lexer if there isn't one.
+  if (!(Style.isVerilog() && readRawTokenVerilogSpecific(Tok.Tok)))
+    Lex->LexFromRawLexer(Tok.Tok);
----------------
As demonstrated by me, the parens can go under in a quick read.


================
Comment at: clang/lib/Format/FormatTokenLexer.cpp:1105
+  static const llvm::Regex VerilogToken(
+      "^(\'|``?|\\\\(\\\\(\r?\n|\r)|[^[:space:]])*)");
+
----------------
sstwcw wrote:
> HazardyKnusperkeks wrote:
> > Consider a raw string, for a better reading.
> You mean like this?
> 
>     static const llvm::Regex VerilogToken(R"re(^('|``?|\\(\\)re"
>                                           "(\r?\n|\r)|[^[:space:]])*)");
> 
I'd put it in one line, but basically yeah.


================
Comment at: clang/lib/Format/FormatTokenLexer.cpp:1129
 void FormatTokenLexer::readRawToken(FormatToken &Tok) {
-  Lex->LexFromRawLexer(Tok.Tok);
+  if (!(Style.isVerilog() && readRawTokenVerilogSpecific(Tok.Tok)))
+    Lex->LexFromRawLexer(Tok.Tok);
----------------
sstwcw wrote:
> HazardyKnusperkeks wrote:
> > Otherwise I don't see why you change that.
> The function is only supposed to be used for Verilog, so `&&` is correct. If you mean why this part is different from D121758, I changed the name from `readRawTokenLanguageSpecific` to `readRawTokenVerilogSpecific` as suggested by a reviewer. Then I moved the check for language out of that function like what `tryParseJSRegexLiteral` does.
I think I missed the parens.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124749