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

Teodor Petrov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 27 15:28:49 PDT 2018


obfuscated marked an inline comment as done.
obfuscated added inline comments.


================
Comment at: lib/Format/FormatToken.h:138
+  /// \brief Whether this is a string literal similar to _T("sdfsdf").
+  bool TMacroStringLiteral = false;
+
----------------
krasimir wrote:
> We don't strictly need this new field. We could do as in the old implementation and check if the string prefix is from a T macro in ContinuationIndenter.
Using a flag is more reliable and faster - the checks are done once, so they don't have to be repeated.

>From the field layout in the struct and from the usage of bools I could only conclude that this is not a memory nor performance critical part of the code.



================
Comment at: unittests/Format/FormatTest.cpp:10693
+
+  // FIXME: This is required because parsing a configuration simply overwrites
+  // the first N elements of the list instead of resetting it.
----------------
krasimir wrote:
> Why is the `FIXME` here? I suggest just use the pattern similar to the other cases here and just keep the test with 2 elements:
> ```
> Style.TMacros.clear();
> std::vector<std::string> ExpectedTMacros = {"_T", "myT"};
> CHECK_PARSE("TMacros: [_T, myT]", TMacros, ExpectedTMacros);
> ```
I've copy pasted this from the foreach macro option.
I've not investigate why this fixme is there.


Repository:
  rC Clang

https://reviews.llvm.org/D44765





More information about the cfe-commits mailing list