This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Fix annotating when deleting array of pointers
ClosedPublic

Authored by jackhong12 on Aug 29 2022, 11:15 PM.

Diff Detail

Event Timeline

jackhong12 created this revision.Aug 29 2022, 11:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2022, 11:15 PM
jackhong12 edited the summary of this revision. (Show Details)Aug 29 2022, 11:16 PM
jackhong12 published this revision for review.Aug 29 2022, 11:23 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2022, 11:23 PM
jackhong12 retitled this revision from Fix annotating when deleting array of pointers to [clang-format] Fix annotating when deleting array of pointers.Aug 29 2022, 11:42 PM

Annotate * as UnaryOperator instead of PointerOrReference

jackhong12 edited the summary of this revision. (Show Details)Aug 30 2022, 1:02 AM
MyDeveloperDay added inline comments.Aug 30 2022, 1:25 AM
clang/lib/Format/TokenAnnotator.cpp
2379

Why unary here, doesn’t that make it a multiply?

In this case, I think it's dereferencing a pointer instead of multiplying two numbers.

jackhong12 added inline comments.Aug 30 2022, 2:53 AM
clang/lib/Format/TokenAnnotator.cpp
2378–2382

The * token in delete *x; will be annotated as UnaryOperator (dereferencing a pointer).

In the case delete[] *x;, there is a ] before *. So, it will be annotated as BinaryOperator by this rule.

I think both * here should be UnaryOperator. Therefore, I add a new rule to match the delete[] pattern.

MyDeveloperDay accepted this revision.Aug 30 2022, 11:00 AM
MyDeveloperDay added inline comments.
clang/lib/Format/TokenAnnotator.cpp
2378–2382

it will be annotated as BinaryOperator by this rule. of course my bad

clang/unittests/Format/FormatTest.cpp
10541

could you cover the other PointerAlignment case for completeness

This revision is now accepted and ready to land.Aug 30 2022, 11:00 AM
HazardyKnusperkeks added a project: Restricted Project.Aug 30 2022, 12:40 PM

Please mark comments as done, when the discussion has ended.

clang/lib/Format/TokenAnnotator.cpp
2376

I don't know if this is nice. I think I would prefer PrevToken->getPreviousNonComment() and subsequently PrevToken->getPreviousNonComment()->getPreviousNonComment().

owenpan added inline comments.Aug 30 2022, 5:09 PM
clang/lib/Format/TokenAnnotator.cpp
2375–2378

endsSequence() is exactly what you want here. ;)

clang/unittests/Format/FormatTest.cpp
10538

A typo?

10541

IMO, testing only PAS_Left would suffice as we just want to ensure that no spaces are added after the *s.

jackhong12 retitled this revision from [clang-format] Fix annotating when deleting array of pointers to Fix annotating when deleting array of pointers.
jackhong12 edited the summary of this revision. (Show Details)

Use endsSequence to match the pattern

jackhong12 retitled this revision from Fix annotating when deleting array of pointers to [clang-format] Fix annotating when deleting array of pointers.
jackhong12 edited the summary of this revision. (Show Details)

Add left alignment test

jackhong12 marked 7 inline comments as done.Aug 30 2022, 6:44 PM
jackhong12 added inline comments.
clang/lib/Format/TokenAnnotator.cpp
2375–2378

Thanks! It looks more concise now.

jackhong12 marked an inline comment as done.Aug 30 2022, 10:53 PM