ksyx 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 ---------------- 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. ================ Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:135 + // UnwrappedLineParser's recognition of free-standing macro like + // Q_OBJECT may also recognize some uppercased type names that may be + // used as return type as that kind of macros, which is a bit hard to ---------------- HazardyKnusperkeks wrote: > As a Qt user who also wants to use your patch, please add a test for that > case. ;) For `Q_OBJECT` if used as pattern like this: ``` class X : ... { Q_OBJECT public: // ... } ``` I think this patch has no effect on this? The comment here is just a repetition of that in unwrapped parser ================ Comment at: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp:143 + "\n" + " LONGTYPENAME\n" + " Foobar(int t, int p) {\n" ---------------- 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 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