HazardyKnusperkeks added inline comments.
================
Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:134
+ OperateIndex + 1 < Lines.size()) {
+ // UnwrappedLineParser's recognition of free-standing macro like
+ // Q_OBJECT may also recognize some uppercased type names that may be
----------------
curdeius wrote:
> ksyx wrote:
> > HazardyKnusperkeks wrote:
> > > Shouldn't we set a type for such cases instead of repeating the detection
> > > code here?
> > Here I actually did a few more checks to limit the impact to the minimum
> > but I am happy to do that if that's necessary.
> > Shouldn't we set a type for such cases instead of repeating the detection
> > code here?
>
> :+1:
Thanks! Shouldn't we drop are radically change this comment?
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1686
+
+ FormatToken *LastToken = FormatTok;
nextToken();
----------------
This is more consistent with the rest of the code base.
================
Comment at: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp:47
llvm::StringRef ExpectedCode = "") {
::testing::ScopedTrace t(File, Line, ::testing::Message() << Code.str());
bool HasOriginalCode = true;
----------------
This is from Debugging?
================
Comment at: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp:143
+ "\n"
+ " LONGTYPENAME\n"
+ " Foobar(int t, int p) {\n"
----------------
ksyx wrote:
> HazardyKnusperkeks wrote:
> > Maybe really use HRESULT? People will know what that should be.
> It actually does not matter what type name it is as long as it is an
> identifier with >=5 characters and all uppercased
That I know, but HRESULT would look familiar to at least some people.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117520/new/
https://reviews.llvm.org/D117520
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
