This is an archive of the discontinued LLVM Phabricator instance.

Make google-build-using-namespace skip std::.*literals
ClosedPublic

Authored by marejde on May 9 2017, 11:06 AM.

Details

Summary

C++14 added a couple of user-defined literals in the standard library. E.g.
std::chrono_literals and std::literals::chrono_literals . Using them
requires a using directive so do not warn in google-build-using-namespace
if namespace name starts with "std::" and ends with "literals".

Diff Detail

Repository
rL LLVM

Event Timeline

marejde created this revision.May 9 2017, 11:06 AM
marejde added inline comments.May 9 2017, 11:32 AM
test/clang-tidy/google-namespaces.cpp
33 ↗(On Diff #98322)

s/Use/use/ . Same below. Fixing.

marejde updated this revision to Diff 98326.May 9 2017, 11:36 AM

Fixed wrong capitalization in added CHECK strings.

marejde marked an inline comment as done.May 9 2017, 11:36 AM
alexfh requested changes to this revision.May 11 2017, 8:31 AM
alexfh added inline comments.
clang-tidy/google/UsingNamespaceDirectiveCheck.cpp
43 ↗(On Diff #98326)

getQualifiedNameAsString() is pretty expensive, since it allocates a string and reconstructs the qualified name by going up all declaration contexts. Instead, consider checking the names directly, something like this:

const NamespaceDecl *NS = U->getNominatedNamespace();
if (NS->getName().ends_with("literals")) {
  const auto *Parent = dyn_cast_or_null<NamespaceDecl>(NS->getParent());
  if (Parent && (Parent->isStdNamespace() || (Parent->getName() == "literals" && Parent->getParent() && Parent->getParent()->isStdNamespace())))
    return;
}
This revision now requires changes to proceed.May 11 2017, 8:31 AM
marejde updated this revision to Diff 98893.May 13 2017, 7:02 AM
marejde edited edge metadata.

Removed use of getQualifiedNameAsString().

marejde updated this revision to Diff 98914.May 14 2017, 2:54 AM

Removed use of nested namespace definition in test (C++17) and added a couple of more namespaces with "literals" in name that check should warn for.

alexfh accepted this revision.May 15 2017, 6:24 AM

LG with one nit. Thank you for addressing this! Do you need me to commit the patch for you?

clang-tidy/google/UsingNamespaceDirectiveCheck.cpp
48 ↗(On Diff #98914)

Please make this a static free function instead, since it doesn't need access to the check's members.

This revision is now accepted and ready to land.May 15 2017, 6:24 AM
marejde updated this revision to Diff 99011.May 15 2017, 8:51 AM

Modified isStdLiteralsNamespace() to be a static method.

Do you need me to commit the patch for you?

Yes please. I do not have commit access.

clang-tidy/google/UsingNamespaceDirectiveCheck.cpp
48 ↗(On Diff #98914)

Done.

This revision was automatically updated to reflect the committed changes.