Page MenuHomePhabricator

[analyzer] Change -analyze-function to accept qualified names.
ClosedPublic

Authored by NoQ on Jul 27 2016, 5:22 AM.

Details

Summary

The -analyze-function option is very useful when debugging test failures, especially when printing or viewing exploded graphs - with this option you no longer need to delete or comment out other test cases.

But because this option accepts unqualified identifiers, it is hard to use this option in C++ tests (in which you often need to test a particular method of a particular class, but there's no way to specify the class, eg. D22374).

This patch changes -analyze-function to accept qualified names. Because -analyzer-display-progress has recently been changed to display qualified names, you can use -analyzer-display-progress to quickly lookup the exact name you need to put into -analyze-function.

Yeah, i agree it's a good idea to update the debugging tricks doc, just as soon as i get to it :)

Diff Detail

Repository
rL LLVM

Event Timeline

NoQ updated this revision to Diff 65710.Jul 27 2016, 5:22 AM
NoQ retitled this revision from to [analyzer] Change -analyze-function to accept qualified names..
NoQ updated this object.
NoQ added a reviewer: zaks.anna.

I think it would be better for fully-qualified Objective-C methods to be specified with their Objective-C-style names. For example: "-[Test1 myMethodWithY:withX:]". This would also remove the ambiguity when there are class and instance methods with the same selector.

Does this approach work for C++ when there are multiple overloads of methods with the same name?

NoQ added a comment.Jul 27 2016, 8:45 AM

I think it would be better for fully-qualified Objective-C methods to be specified with their Objective-C-style names. For example: "-[Test1 myMethodWithY:withX:]".

Uhm, need to know more about those. So i just print "-[", then fully-qualified class name, then selector identifier, then "]"?

Does this approach work for C++ when there are multiple overloads of methods with the same name?

Nope, unfortunately not. I'm open to suggestions on how to make this work. We may either dump types of arguments, or do something like "foo~1", "foo~2", etc.

In D22856#497796, @NoQ wrote:

I think it would be better for fully-qualified Objective-C methods to be specified with their Objective-C-style names. For example: "-[Test1 myMethodWithY:withX:]".

Uhm, need to know more about those. So i just print "-[", then fully-qualified class name, then selector identifier, then "]"?

Instance methods get "-" and class methods (which are similar to static methods in Java) get the "+" prefix. See CGDebugInfo::getObjCMethodName() in CGDebugInfo.cpp for the gory details.

zaks.anna edited edge metadata.Jul 27 2016, 10:21 PM

We'd definitely want the same routine in both: printing the name in -analyzer-display-progress and be accepted by -analyze-function.

Looks like what ND->getQualifiedNameAsString() returns is not proper ObjC syntax, but maybe it's fine for the debug feature? Even though it is not very readable...

zaks.anna accepted this revision.Jul 29 2016, 5:48 PM
zaks.anna edited edge metadata.

Since this is an improvement, it looks good to me. (Improving printing of ObjC methods is a good add on, but not blocking..)

This revision is now accepted and ready to land.Jul 29 2016, 5:48 PM
NoQ updated this revision to Diff 66360.Aug 1 2016, 12:58 PM
NoQ edited edge metadata.

Copy-paste the code from CGDebugInfo.cpp to support Objective-C instance and class methods. Add more tests. Move the analyze_display_progress.cpp because most tests use dashes in names.

This revision was automatically updated to reflect the committed changes.