This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming
ClosedPublic

Authored by berenm on Oct 27 2017, 12:31 AM.

Event Timeline

berenm created this revision.Oct 27 2017, 12:31 AM
alexfh edited edge metadata.Dec 7 2017, 7:09 AM
alexfh added a subscriber: cfe-commits.

Sorry for the delay. I missed this revision somehow. Please add cfe-commits to the subscribers list so that others can chime in.

alexfh added a comment.Dec 7 2017, 7:15 AM

One nit. Otherwise seems fine.

clang-tidy/readability/IdentifierNamingCheck.cpp
502

Can we check for !Type.isNull() && Type.isConstQualified() once here?

if (!Type.isNull() && Type.isConstQualified()) {
  if (Decl->isStaticDataMember() && NamingStyles[SK_ClassConstant])
    return SK_ClassConstant;
  if (Decl->isFileVarDecl() && NamingStyles[SK_GlobalConstant])
    return SK_GlobalConstant;
  ...
}
berenm updated this revision to Diff 126017.Dec 7 2017, 12:14 PM

Factor !Type.isNull() && Type.isConstQualified() condition

berenm marked an inline comment as done.Dec 7 2017, 12:14 PM

Sorry for the delay. I missed this revision somehow. Please add cfe-commits to the subscribers list so that others can chime in.

No worries, I'll add it in the future.

Thanks!

alexfh accepted this revision.Dec 8 2017, 9:54 AM

Looks good!

Thank you for the fix! Do you need someone to commit it for you? If yes, please rebase the fix on top of the current HEAD and let me know.

This revision is now accepted and ready to land.Dec 8 2017, 9:54 AM
berenm updated this revision to Diff 126177.Dec 8 2017, 10:24 AM

Rebase patch on upstream HEAD

Yes, please could you commit it for me? Thanks!

This revision was automatically updated to reflect the committed changes.