[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb updated this revision to Diff 558132. jaredgrubb marked 3 inline comments as done. jaredgrubb added a comment. Update to address all the review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/ https://reviews.llvm.org/D150083 Files: clang/docs/ClangForma

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb marked 7 inline comments as done. jaredgrubb added inline comments. Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:113 + + // Deduplicate the attributes. + Indices.erase(std::unique(Indices.begin(), Indices.end(), owenpan wrote: > j

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb added a comment. In D150083#4656832 , @owenpan wrote: > Thank you for your patience! I appreciate the help :) I'm excited to get this in! > In D150083#4655528 , @owenpan wrote: > >> See also D153228 <

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb added a comment. From my perspective, this patch is ready to go! Please let me know if there's anything more I can do to improve the patch! (@owenpan thanks so much for your help so far!) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/ https://reviews.llvm.org/D15

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb updated this revision to Diff 558010. jaredgrubb added a comment. Adjusting the Style comments, since the behavior changed and were not updated to reflect that leading/trailing comments no longer have special handling. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb added a comment. I opened https://github.com/llvm/llvm-project/issues/71323 for this patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/ https://reviews.llvm.org/D150083 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb updated this revision to Diff 558009. jaredgrubb marked 28 inline comments as done. jaredgrubb added a comment. Addressing all review comments to date. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/ https://reviews.llvm.org/D150083 Files: clang/docs/ClangFormatS

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb added inline comments. Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:55 +const FormatToken *LParenTok, const FormatToken *RParenTok) const { + // Skip past any leading comments. + const FormatToken *const BeginTok = LParenTok->getNextNonComment

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb added a comment. Should I open a GitHub issue for this patch? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/ https://reviews.llvm.org/D150083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb updated this revision to Diff 557922. jaredgrubb added a comment. Address review feedback from @owenpan on Oct23. Thanks for helping me tune this patch! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/ https://reviews.llvm.org/D150083 Files: clang/docs/ClangForma

[PATCH] D145262: [clang-format] Treat AttributeMacros more like __attribute__

jaredgrubb added a comment. In D145262#4653670 , @owenpan wrote: > @jaredgrubb please provide your authorship for `git commit --author` so that > we can land the patch on your behalf. The best to use would be: Jared Grubb Thanks so much for your he

[PATCH] D145262: [clang-format] Treat AttributeMacros more like __attribute__

jaredgrubb updated this revision to Diff 557674. jaredgrubb added a comment. Addressed review feedback from Oct2 (@owenpan). A new issue on Github was opened for this enhancement (https://github.com/llvm/llvm-project/issues/68722) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 557542. jaredgrubb added a comment. Address review comments, and adjusting the patch to address other merges since this patch was started. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new/ https://reviews.llvm.org/D145262 Files: clang

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb added a comment. @MyDeveloperDay addressed your comments. Thanks! Would love to get this in before Phabricator closes. Anything else I can do to help? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/ https://reviews.llvm.org/D150083 ___

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb added a comment. @MyDeveloperDay addressed your comments. Thanks! Would love to get this in before Phabricator closes. Anything else I can do to help? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/ https://reviews.llvm.org/D150083 ___

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb updated this revision to Diff 557275. jaredgrubb added a comment. Rebased and adjusted docs to reflect that this patch would appear in clang-format 18 (not 17 now). Removed extraneous comment change (will do NFC later for that) Removed other build-system changes (as requested). CHANG

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb marked an inline comment as done. jaredgrubb added inline comments. Comment at: clang/unittests/Format/FormatTestObjC.cpp:1619 + // Reflow after first macro. + // FIXME: these should indent but don't. + verifyFormat("- (id)init ATTRIBUTE_MACRO(X)\n"

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 557274. jaredgrubb added a comment. Hopefully this will satisfy and we can merge! I didn't notice the Phabricator timeline and I'm hoping we can finish this before Oct 1. Rebased. Removed extraneous doc comment not related to patch (as requested). Removed

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb updated this revision to Diff 540492. jaredgrubb added a comment. Rebased (no other updates) to re-run build and hopefully get some eyes on this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/ https://reviews.llvm.org/D150083 Files: clang/docs/ClangFormatStyleOp

[PATCH] D146434: [clang-format] Fix support for ObjC blocks with pointer return types

jaredgrubb updated this revision to Diff 540488. jaredgrubb marked an inline comment as done. jaredgrubb added a comment. Address review comment and rebase to re-run tests. Intend to merge if everything is green! (If there are further comments, I will commit to address them!) CHANGES SINCE LAS

[PATCH] D146434: [clang-format] Fix support for ObjC blocks with pointer return types

jaredgrubb marked 2 inline comments as done. jaredgrubb added inline comments. Herald added a subscriber: wangpc. Herald added a reviewer: rymiel. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1779-1782 nextToken(); +// Return types: pointers are ok too. +

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 540477. jaredgrubb added a comment. Rebased and added a line to Release Notes, no other changes. Have requested this to be merged if all builds are green (since this patch has been approved). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/n

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb added a comment. Are there any other comments on this patch? I would love to make this into clang-17! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/ https://reviews.llvm.org/D150083 ___ cfe-commits mailing list cfe-comm

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb updated this revision to Diff 524351. jaredgrubb marked an inline comment as done. jaredgrubb added a comment. Address review comments: unroll a loop in unit tests to explicitly test all the property attributes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/ https

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 521953. jaredgrubb added a comment. Address review comments: - remove redundant `&&` - remove part of patch that was not tested by any test and should be its own patch on its own merits CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new/

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb marked an inline comment as done. jaredgrubb added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:5473 + if (Right.isOneOf(tok::kw___attribute, TT_AttributeMacro)) +return true; + HazardyKnusperkeks wrote: > jaredgrubb wrote: > >

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb updated this revision to Diff 521952. jaredgrubb added a comment. Address review comments: - fix some style - add unit test for each ObjC attribute recognized by the compiler - adjust the docs for the style-option to show a YAML example with all of them in a sane order (something peop

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:5473 + if (Right.isOneOf(tok::kw___attribute, TT_AttributeMacro)) +return true; + HazardyKnusperkeks wrote: > Does changing this return value make no difference? In other words

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb updated this revision to Diff 520247. jaredgrubb added a comment. Update `ClangFormatStyleOptions.rst` as requested by the auto-comment (thanks auto-bot!). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/ https://reviews.llvm.org/D150083 Files: clang/docs/ClangFo

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb added a comment. Some implementation notes: - The implementation was modeled after `QualifierAlignmentFixer` and `sortCppIncludes`. - the additions to the `.bazel`/`.gn` build files was done naively based on searching for where "QualifierFixerTest" appeared in the repo; I can't reall

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

jaredgrubb created this revision. jaredgrubb added reviewers: benhamilton, egorzhdan, owenpan, HazardyKnusperkeks. jaredgrubb added a project: clang-format. Herald added projects: All, clang. Herald added a subscriber: cfe-commits. Herald added reviewers: rymiel, MyDeveloperDay. Herald added a comm

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 520195. jaredgrubb added a comment. Address review feedback about `code/endcode`. Otherwise the patch is the same and I hope ready for merge? I'd love to get a green check :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new/ https://revi

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb added a comment. I have uploaded patch for all the points you called out. (Sorry for delay; I missed the suggestions earlier!) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new/ https://reviews.llvm.org/D145262 ___ cfe-comm

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 516211. jaredgrubb marked 6 inline comments as done. jaredgrubb added a comment. Address review comments from @owenpan CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new/ https://reviews.llvm.org/D145262 Files: clang/lib/Format/Continuat

[PATCH] D146310: [clang-format] Fix dropped 'else' in 398cddf6acec

jaredgrubb added a comment. Manuel, if you're happy with the change, do you mind committing it? I don't have commit access (at least I've never requested it, so I assume I can't, I've never tried) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1463

[PATCH] D146310: [clang-format] Fix dropped 'else' in 398cddf6acec

jaredgrubb added a comment. Yeh, I considered trying to craft one as courtesy but this seemed like a very far edge case and didn't seem really worth it. So glad you agree :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146310/new/ https://reviews

[PATCH] D146434: [clang-format] Fix support for ObjC blocks with pointer return types

jaredgrubb added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:22178 "});"); - verifyFormat("Block b = ^int *(A *a, B *b) {}"); + verifyFormat("Block b = ^int *(A *a, B *b) {\n" + "};"); MyDeveloperDay wrote: >

[PATCH] D146434: [clang-format] Fix support for ObjC blocks with pointer return types

jaredgrubb created this revision. jaredgrubb added reviewers: djasper, egorzhdan, benhamilton. jaredgrubb added a project: clang-format. Herald added a project: All. jaredgrubb requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The ObjC-block

[PATCH] D146310: [clang-format] Fix dropped 'else' in 398cddf6acec

jaredgrubb added a comment. The difference doesn't appear to affect any unit tests (which is unfortunate), but I think you didn't mean to remove this else, based both on the logic of the original commit and the format of the patched line. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D146310: [clang-format] Fix dropped 'else' in 398cddf6acec

jaredgrubb created this revision. jaredgrubb added a reviewer: klimek. jaredgrubb added a project: clang-format. Herald added a project: All. jaredgrubb requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. A patch (398cddf6acec

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb added inline comments. Comment at: clang/unittests/Format/FormatTestObjC.cpp:1619 + // Reflow after first macro. + // FIXME: these should indent but don't. + verifyFormat("- (id)init ATTRIBUTE_MACRO(X)\n" I don't love this FIXME, but I was afraid to

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:5329 + if (Right.is(tok::l_square) && Right.is(TT_AttributeSquare)) return !Left.is(TT_AttributeSquare); I broke this `if` into two pieces and restored an original `return t

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 504486. jaredgrubb added a comment. - Fixed an issue in `TokenAnnotator` about it not breaking between macros properly (it was catching in an ObjC selector-check too early) - Add more ObjC tests, covering method and property declarations too. There are st

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb updated this revision to Diff 502390. jaredgrubb added a comment. Create unit-tests for the patch (and remove the proposed non-unit test "test"). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145262/new/ https://reviews.llvm.org/D145262 Files: clang/lib/Format/Continuation

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb added a comment. I wasn't sure about testing (this is my first patch!) and the test-case I did was in `clang/test/Format` .. I can look at `clang/unittests/Format` and see how to model something like it. If I do that, would that be in-addition-to or instead-of the test-case I includ

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb added a comment. For background, the current clang-format results in the following (`-style="{BasedOnStyle: LLVM, ColumnLimit: 0, AttributeMacros: [MACRO]}`): MACRO MACRO(A) @interface Foo @end MACRO(A) MACRO @interface Foo @end This patch improves it (remove

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

jaredgrubb created this revision. jaredgrubb added reviewers: HazardyKnusperkeks, djasper, egorzhdan. jaredgrubb added a project: clang-format. Herald added a project: All. jaredgrubb requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. I notice