This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Fix support for ObjC blocks with pointer return types
ClosedPublic

Authored by jaredgrubb on Mar 20 2023, 9:31 AM.

Details

Summary

The ObjC-block detection code only supports a single token as the return type. Add support to detect pointers, too (ObjC has lots of object-pointers).

For example, using BasedOnStyle: WebKit, the following is stable output:

int* p = ^int*(void)
{ //
    return nullptr;
}
();

After the patch:

int* p = ^int*(void) { //
    return nullptr;
}();

Diff Detail

Event Timeline

jaredgrubb created this revision.Mar 20 2023, 9:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 20 2023, 9:31 AM
jaredgrubb requested review of this revision.Mar 20 2023, 9:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 20 2023, 9:31 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
MyDeveloperDay added inline comments.
clang/unittests/Format/FormatTest.cpp
22622–22623

I don’t like us changing tests

jaredgrubb added inline comments.Mar 20 2023, 8:43 PM
clang/unittests/Format/FormatTest.cpp
22622–22623

Normally I would agree, but in this case, this is a bug-fix for the test as well.

If you look at the verifyFormat just below this one, its braces are broken across separate lines. The inconsistency between these two tests was due to the difference in the block's return type (pointer vs non-pointer).

My patch corrects exactly that inconsistency.

This revision is now accepted and ready to land.Mar 21 2023, 3:14 AM
owenpan added inline comments.Mar 23 2023, 9:57 PM
clang/lib/Format/UnwrappedLineParser.cpp
1804–1809
clang/unittests/Format/FormatTestObjC.cpp
1032–1035
jaredgrubb marked 2 inline comments as done.Jul 14 2023, 10:34 AM
jaredgrubb added inline comments.
clang/lib/Format/UnwrappedLineParser.cpp
1804–1809

I think this obscures the logic a bit. The first one is to consume one token (the return type) and the next is to consume a different kind of token (trailing stars). Putting them in a while loop makes it harder to reason about why it's looping in that way.

clang/unittests/Format/FormatTestObjC.cpp
1032–1035

Fixed!

jaredgrubb marked an inline comment as done.

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!)

owenpan accepted this revision.Jul 14 2023, 4:26 PM
MyDeveloperDay accepted this revision.Jul 16 2023, 7:09 AM