This is an archive of the discontinued LLVM Phabricator instance.

[ASTMatchers] Add narrowing matcher: hasExternalFormalLinkage
ClosedPublic

Authored by thegameg on Aug 16 2016, 3:59 PM.

Details

Summary

Matches NamedDecl nodes whose linkage is external.

Diff Detail

Event Timeline

thegameg updated this revision to Diff 68273.Aug 16 2016, 3:59 PM
thegameg retitled this revision from to [ASTMatchers] Add narrowing matcher: hasExternalFormalLinkage.
thegameg updated this object.
thegameg added reviewers: bkramer, alexfh, hokein.
thegameg added a subscriber: cfe-commits.
aaron.ballman added a subscriber: aaron.ballman.

A few minor nits, but thank you for the patch!

include/clang/ASTMatchers/ASTMatchers.h
5481

It might also be good to point out the case of an anonymous namespace member which has external linkage (despite having internal semantic linkage because the member has a unique name in all translation units).

unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
1942

Should use EXPECT_TRUE with notMatches() here and below.

thegameg updated this revision to Diff 68288.Aug 16 2016, 5:03 PM

Mention UniqueExternalLinkage type, add a test according to it.
EXPECT_FALSE(matches( -> EXPECT_TRUE(notMatches(

thegameg marked an inline comment as done.Aug 16 2016, 5:04 PM
thegameg updated this revision to Diff 68290.Aug 16 2016, 5:06 PM

Sorry, sent the wrong patch.

thegameg marked an inline comment as done.Aug 16 2016, 5:07 PM
alexfh requested changes to this revision.Aug 17 2016, 1:31 AM
alexfh edited edge metadata.
alexfh added inline comments.
docs/LibASTMatchersReference.html
2774

Please regenerate the .html file.

This revision now requires changes to proceed.Aug 17 2016, 1:31 AM
thegameg updated this revision to Diff 68332.EditedAug 17 2016, 4:10 AM
thegameg edited edge metadata.
thegameg added a subscriber: alexfh.

LibASTMatchersReference.html regenerated using dump_ast_matchers.py.

aaron.ballman added inline comments.Aug 17 2016, 5:30 AM
include/clang/ASTMatchers/ASTMatchers.h
5490–5497

I wouldn't copy and paste the text from Linkage.h, merely point out below: Example matches f() because it has external formal linkage despite being unique to the translation unit as though it has internal linkage (matcher = ...).

thegameg updated this revision to Diff 68340.EditedAug 17 2016, 5:41 AM
thegameg edited edge metadata.

Update documentation according to @aaron.ballman's remarks.

thegameg marked an inline comment as done.Aug 17 2016, 5:41 AM
aaron.ballman accepted this revision.Aug 17 2016, 5:56 AM
aaron.ballman edited edge metadata.

LGTM, thank you!

Thank you for the review.

I don't have commit access, would you commit this for me, please?

Thanks!

I've commit in r278926. (I can't close because @alexfh hasn't accepted yet; Alex, can you accept & close?)

alexfh accepted this revision.Aug 17 2016, 7:27 AM
alexfh edited edge metadata.

LG

This revision is now accepted and ready to land.Aug 17 2016, 7:27 AM
alexfh closed this revision.Aug 17 2016, 7:27 AM

I've commit in r278926. (I can't close because @alexfh hasn't accepted yet; Alex, can you accept & close?)

Sure. Done.

BTW, Phabricator can close revisions automatically on commit: http://llvm.org/docs/Phabricator.html#committing-a-change