This is an archive of the discontinued LLVM Phabricator instance.

missing-namespace-std check for clang-tidy
AbandonedPublic

Authored by jbcoe on Feb 17 2015, 7:53 PM.

Details

Reviewers
eliben
Summary

Sanity check would be good along with suggestions for how to proceed.

missing-namespace-std works but only in very limited scope:

It can't handle references.
It can't handle pointers.
It can't handle nested template types like MyTemplateClass<std::string>.

I'd be grateful for input on how to do any of these. I presume there's a standard way to get the referenced type or ultimate pointee (so that std::string**** works). There's no need for the check to dig into typedefs as it's only the written type that it's concerned with. My best efforts resulted in odd hangs when parsing the <vector> header.

Diff Detail

Event Timeline

jbcoe updated this revision to Diff 20137.Feb 17 2015, 7:53 PM
jbcoe retitled this revision from to missing-namespace-std check for clang-tidy.
jbcoe updated this object.
jbcoe edited the test plan for this revision. (Show Details)
jbcoe added reviewers: klimek, eliben.
jbcoe set the repository for this revision to rL LLVM.
jbcoe added a subscriber: Unknown Object (MLST).
klimek edited edge metadata.Feb 18 2015, 4:50 AM

Why is this not something we want to directly match the TypeLocs via loc(type(..)).bind(...)? That would also help with the nested type problems (std::vector****)

jbcoe added a comment.Feb 18 2015, 5:12 AM

Can you elaborate?

I've not written checks before and this was the first thing I could get to work (for a ver limited definition of 'work'), if there's a neater or smarter way to do it I'd be keen to learn.

klimek added inline comments.Feb 18 2015, 7:42 AM
clang-tidy/misc/MissingNamespaceStdCheck.cpp
21

I imagine you'll want to replace this matcher with something like:
loc(qualType(hasDeclaration(decl(hasAncestor(decl(namespaceDecl(hasName("std")))))), unless(elaboratedType(hasQualifier(anyOf(specifiesNamespace(hasName("std")), hasPrefix(specifiesNamespace(hasName("std")))))))))
(you can pull out duplicate submatchers with auto matcher = ... and reuse it)
The problem is that we don't have hasParent for typeLoc matches, so for
std::X
this will still match "X" (the inner typeloc).
For that you'll need to make sure to have a matcher like:
loc(elaboratedType(namesType(type().bind("root"))))
store the bound types as "visited",
and then make sure when you hit the callback for the first matcher that the type was not visited via an outer elaboratedType already.

klimek resigned from this revision.Jul 3 2015, 6:44 AM
klimek removed a reviewer: klimek.
jbcoe added a comment.Jul 11 2015, 2:27 PM

This work is stalled but not abandoned. I plan to resume work mid-August.

jbcoe abandoned this revision.Sep 16 2015, 4:31 PM

This work needs such extensive revision that I'll submit a new version when I start work on it again.