Page MenuHomePhabricator

Add an argumentsAre matcher
Needs RevisionPublic

Authored by fowles on Jan 3 2017, 3:36 PM.

Details

Summary

Add an argumentsAre matcher

Event Timeline

fowles updated this revision to Diff 82964.Jan 3 2017, 3:36 PM
fowles retitled this revision from to Add an argumentsAre matcher.
fowles updated this object.
fowles added a reviewer: klimek.
fowles added a subscriber: cfe-commits.
aaron.ballman requested changes to this revision.Jan 10 2017, 8:20 AM
aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a subscriber: aaron.ballman.

You should also regenerate the HTML matcher documentation as part of this patch.

include/clang/ASTMatchers/ASTMatchers.h
3075

This is a neat matcher, but I'm not certain it will work with the dynamic matchers, which is an unfortunate divergence for clang-query.

Another concern is that this is leaking implementation details into ASTMatchers.h rather than keeping them in ASTMatchersInternal.h.

unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
383

How does this matcher work in the presence of default arguments? e.g.,

void f(int a, int b = 12);
f(1);

Will callExpr(argumentsAre(integerLiteral())) match?

A similar question applies for variadic functions and functions without a prototype (from C).

I suspect it all works fine, but some test cases would be nice.

This revision now requires changes to proceed.Jan 10 2017, 8:20 AM

Given that this can't be expressed as a dynamic matcher, I'm wondering what the use case is for the matcher. Do you have a specific need for this functionality, or do you see this being generally useful?

@jdennett wanted this matcher for something he is working on and I had some free cycles to write it up. Unfortunately, I am about to leave on an extended vacation, so I will not be able to follow up with this patch for 2 months at the earliest.

I suspect the best solution is for someone to take over this patch and work with James to clarify any corner cases.