obfuscated marked 3 inline comments as done. obfuscated added inline comments.
================ Comment at: lib/Format/FormatTokenLexer.cpp:371 FormatToken *Macro = Tokens[Tokens.size() - 4]; - if (Macro->TokenText != "_T") + if (std::find(Style.TMacros.begin(), Style.TMacros.end(), Macro->TokenText) == + Style.TMacros.end()) ---------------- alexfh wrote: > Consider using llvm::find, which is a range adaptor for std::find. No idea what is range adaptor, but I can probably switch to it... :) ================ Comment at: lib/Format/FormatTokenLexer.cpp:386 String->HasUnescapedNewline = Macro->HasUnescapedNewline; + String->TMacroStringLiteral = true; ---------------- alexfh wrote: > obfuscated wrote: > > krasimir wrote: > > > In the original code, TMacro detection was done as: > > > ``` > > > (Text.startswith(Prefix = "_T(\"") && Text.endswith(Postfix = "\")")) > > > ``` > > > In the new code the left part is saved in TMacroStringLiteral, and the > > > right part is checked in ContinuationIndenter. Could you keep them > > > together in `FormatTokenLexer`. > > > @alexfh, why are we checking for the right check at all? What would be an > > > example where this is needed to disambiguate? > > > > > Are you talking about the code in > > ContinuationIndenter::createBreakableToken? > > > > I don't think I understand how the hole thing works. > > Using the debugger I can see that this function is executed first and then > > createBreakableToken. > > So we first detect the tmacro in FormatTokenLexer::tryMerge_TMacro and then > > use this information in the createBreakableToken to do something with it. > > > > So when I get a TMacroStringLiteral==true in createBreakableToken I know > > that the token starts with a TMacro and there is no need to use some king > > of startswith calls. Probably the endswith call is redundant and I can do > > just a string search backwards... > > In the new code the left part is saved in TMacroStringLiteral, and the > > right part is checked in ContinuationIndenter. Could you keep them together > > in FormatTokenLexer. > > +1 to keeping these conditions together. > > > @alexfh, why are we checking for the right check at all? What would be an > > example where this is needed to disambiguate? > > I don't know whether there's reasonable code that would be handled > incorrectly without checking for the `")`, but I'm sure some test case can be > generated, that would break clang-format badly without this condition. It > also serves as a documentation (similar to asserts). >> In the new code the left part is saved in TMacroStringLiteral, and the >> right part is checked in ContinuationIndenter. Could you keep them together >> in FormatTokenLexer. > > +1 to keeping these conditions together. Can you please explain what you mean here? And what am I supposed to do, because I don't know how to put these conditions together. Do you want me to search the tmacro vector in ContinuationIndenter::createBreakableToken instead of checking the bool flag? https://reviews.llvm.org/D44765 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
