This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Remove special case for kw_operator when aligning decls
ClosedPublic

Authored by rymiel on Nov 1 2022, 8:15 PM.

Details

Summary

This change breaks no existing tests but does fix the linked issue.
Declarations of operator overloads are annotated with
TT_FunctionDeclarationName on the operator keyword, which is already
being checked for when aligning, so the extra kw_operator doesn't seem
to be necessary. (just for reference, it was added in
rG92b397fb9d55ccdf4632c2b1b15b4a0ee417cf74 / 92b397fb9d55ccdf4632c2b1b15b4a0ee417cf74)

Fixes https://github.com/llvm/llvm-project/issues/55733

Diff Detail

Event Timeline

rymiel created this revision.Nov 1 2022, 8:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 1 2022, 8:15 PM
rymiel requested review of this revision.Nov 1 2022, 8:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 1 2022, 8:15 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
rymiel added a project: Restricted Project.Nov 1 2022, 8:16 PM
owenpan added inline comments.Nov 2 2022, 2:16 AM
clang/unittests/Format/FormatTest.cpp
17939

Add a full stop.

17939–17946

Consider using a test case similar to the examples in https://github.com/llvm/llvm-project/issues/55733 or not adding a test case at all as the added annotator tests will cover it.

17945

Remove the newline.

clang/unittests/Format/TokenAnnotatorTest.cpp
441–456

Instead of adding new tests, you can add checks for kw_operator and TT_FunctionDeclarationName to the existing ones.

What Owen says.
Apart from that looks good.

rymiel marked 3 inline comments as done.Nov 4 2022, 5:29 PM
rymiel added inline comments.
clang/unittests/Format/TokenAnnotatorTest.cpp
441–456

I wanted to have tests that are declaring operator functions, not just calling them

rymiel updated this revision to Diff 473380.Nov 4 2022, 5:29 PM

Remove format test, relying only on annotator tests

owenpan accepted this revision.Nov 4 2022, 11:54 PM

LGTM

This revision is now accepted and ready to land.Nov 4 2022, 11:54 PM
This revision was landed with ongoing or failed builds.Nov 17 2022, 1:39 AM
This revision was automatically updated to reflect the committed changes.