alexfh 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())
obfuscated marked an inline comment as done.
obfuscated added a comment.
Ping?
https://reviews.llvm.org/D44765
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/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(),
alexfh added inline comments.
Comment at: include/clang/Format/Format.h:1676
+ /// \brief A vector of macros that should be interpreted as macros expanding
+ /// to a string literal encoding prefix instead of as function calls.
"A list of macro names"?
obfuscated added inline comments.
Comment at: lib/Format/FormatTokenLexer.cpp:386
String->HasUnescapedNewline = Macro->HasUnescapedNewline;
+ String->TMacroStringLiteral = true;
krasimir wrote:
> In the original code, TMacro detection was done as:
> ```
>
krasimir added inline comments.
Comment at: lib/Format/FormatTokenLexer.cpp:386
String->HasUnescapedNewline = Macro->HasUnescapedNewline;
+ String->TMacroStringLiteral = true;
In the original code, TMacro detection was done as:
```
(Text.startswith(Prefix
obfuscated updated this revision to Diff 140191.
obfuscated marked an inline comment as done.
obfuscated added a comment.
Fixed tests. Fixed description.
https://reviews.llvm.org/D44765
Files:
include/clang/Format/Format.h
lib/Format/ContinuationIndenter.cpp
lib/Format/Format.cpp
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
krasimir added a comment.
I don't have preferences over names, but I agree with Alex that the option
should have more detailed description.
Comment at: lib/Format/FormatToken.h:138
+ /// \brief Whether this is a string literal similar to _T("sdfsdf").
+ bool
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Thank you! One more comment from me and I'll leave the rest of the review to
Krasimir, who has a better idea of how the configuration options should be
named etc.
obfuscated updated this revision to Diff 139687.
obfuscated added a comment.
Attached both commits in a single diff...
Repository:
rC Clang
https://reviews.llvm.org/D44765
Files:
include/clang/Format/Format.h
lib/Format/ContinuationIndenter.cpp
lib/Format/Format.cpp
obfuscated updated this revision to Diff 139686.
obfuscated added a comment.
Added an option
Repository:
rC Clang
https://reviews.llvm.org/D44765
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/FormatTokenLexer.cpp
unittests/Format/FormatTest.cpp
Index:
alexfh added a comment.
In https://reviews.llvm.org/D44765#1045789, @obfuscated wrote:
> In https://reviews.llvm.org/D44765#1045373, @alexfh wrote:
>
> > We can't just treat `anything("")` like the _T macro. There should be a
> > whitelist configurable with an option. By default only _T
obfuscated added a comment.
In https://reviews.llvm.org/D44765#1045373, @alexfh wrote:
> We can't just treat `anything("")` like the _T macro. There should be a
> whitelist configurable with an option. By default only _T should be handled.
What cases could break with this version of the
Typz added a comment.
You are right, I did not mean it would help with the handling of these macros;
but it may be related on the configuration-side of things : adding an option
for listing these macros may hit the same limitation, and the same mean of
storing (in the code) the list of these
alexfh added a comment.
In https://reviews.llvm.org/D44765#1045394, @Typz wrote:
> A generic (or at least extandable) approach to specifying macro behaviors was
> introduced here: https://reviews.llvm.org/D33440
I believe, that patch solves a significantly different problem and it won't
make
Typz added a comment.
A generic (or at least extandable) approach to specifying macro behaviors was
introduced here: https://reviews.llvm.org/D33440
But this change seem to be on hold, as a configuration based on providing
"macro definitions" seem to be preferable and kind-of in the pipe,
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
We can't just treat `anything("")` like the _T macro. There should be a
whitelist configurable with an option. By default only _T should be handled.
Repository:
rC Clang
obfuscated created this revision.
obfuscated added a reviewer: krasimir.
Herald added a subscriber: klimek.
This patch changes the check for _T to detect TMarcos with a more generic check.
This makes it possible to format wxWidgets projects (where wxT is used a lot)
correctly.
Patch by Teodor
19 matches
Mail list logo
