Page MenuHomePhabricator

[analyzer][NFC] Switch to using CallDescription::matches() instead of isCalled()
ClosedPublic

Authored by steakhal on Wed, Nov 10, 10:45 AM.

Details

Summary

This patch replaces each use of the previous API with the new one.
In variadic cases, it will use the ADL matchesAny(Call, CDs...) variadic function.
Also simplifies some code involving such operations.

Diff Detail

Event Timeline

steakhal created this revision.Wed, Nov 10, 10:45 AM
steakhal updated this revision to Diff 386850.Fri, Nov 12, 7:42 AM

use the terser ADL syntax matches_any(Call, CD1, CDs...)

xazax.hun accepted this revision.Fri, Nov 12, 9:24 AM

Once I understand the motivation behind CallDescription::matches(), this patch looks good. The use of variadic matching is a nice cleanup.

This revision is now accepted and ready to land.Fri, Nov 12, 9:24 AM
martong added inline comments.Thu, Nov 18, 9:22 AM
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
2275

I think this would be more unified to the rest and would make further refactoring easier (considering my other comment https://reviews.llvm.org/D113590#inline-1089657)

steakhal updated this revision to Diff 388429.Fri, Nov 19, 2:12 AM
steakhal edited the summary of this revision. (Show Details)

rebase

steakhal added inline comments.Fri, Nov 19, 2:13 AM
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
2275

I think the member function is more readable.
I could reject single argument calls of matches_any() to have consistency across single CallDescription checking.
Or do the opposite, basically what you propose. Although, I would still stick to the member function syntax. It's probably a personal preference.

What I tried to do is to have the exact same name for both. That would play nicely with the Stroustrup's unified member function call syntax (brevzin.github.io blog post).
However, I could not make it work :/

martong accepted this revision.Fri, Nov 19, 2:56 AM

LGTM!

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
2275

Okay, let's not get stuck on nuances.

This revision was landed with ongoing or failed builds.Fri, Nov 19, 9:33 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFri, Nov 19, 9:33 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript