[PATCH] D44765: PR36643 Make clang-format support more generic TMarcos

[Teodor Petrov via Phabricator via cfe-commits]

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