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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
