This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Fixed false-negative in readability-identifier-naming
ClosedPublic

Authored by PiotrZSL on Aug 5 2023, 5:04 AM.

Details

Summary

Issue were cased by checking if canonical declaration of record is a
definition. Unfortunately when forward declarations were used, it were
not a definition.

Fixes: #64463

Diff Detail

Event Timeline

PiotrZSL created this revision.Aug 5 2023, 5:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 5 2023, 5:04 AM
Herald added a subscriber: xazax.hun. · View Herald Transcript
PiotrZSL requested review of this revision.Aug 5 2023, 5:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 5 2023, 5:04 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
PiotrZSL updated this revision to Diff 547475.Aug 5 2023, 5:24 AM
PiotrZSL edited the summary of this revision. (Show Details)

Properly handle situation when class is forward-declared as struct.

carlosgalvezp accepted this revision.Aug 5 2023, 8:14 AM

LGTM, minor question that can be fixed post-review unless you want to discuss further!

clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp
716

Nit: shouldn't we use also struct here?

This revision is now accepted and ready to land.Aug 5 2023, 8:14 AM
PiotrZSL added inline comments.Aug 5 2023, 8:18 AM
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp
716

No, thats on prupose. I wanted to test also in same place that style is taken from definition, not forward declaration, can add some comment, and "public:"

PiotrZSL updated this revision to Diff 547503.Aug 5 2023, 11:08 AM

Rebase + make tests more simple

PiotrZSL marked an inline comment as done.Aug 6 2023, 1:12 AM
carlosgalvezp accepted this revision.Aug 6 2023, 1:44 AM