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

[NoQ]

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 :)

[dcoughlin]

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]

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.

[dcoughlin]

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]

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]

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

[NoQ]

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.

cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

if (Loc.isValid()) {
             llvm::errs() << " Inline_Regular";
             break;
         }
         llvm::errs() << ")";
       }
       else
         assert(Mode == (AM_Syntax | AM_Path) && "Unexpected mode!");
 
-      llvm::errs() << ": " << Loc.getFilename();
-      if (isa<FunctionDecl>(D) || isa<ObjCMethodDecl>(D)) {
-        const NamedDecl *ND = cast<NamedDecl>(D);
-        llvm::errs() << ' ' << ND->getQualifiedNameAsString() << '\n';
-      }
-      else if (isa<BlockDecl>(D)) {
-        llvm::errs() << ' ' << "block(line:" << Loc.getLine() << ",col:"
-                     << Loc.getColumn() << '\n';
-      }
-      else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
-        Selector S = MD->getSelector();
-        llvm::errs() << ' ' << S.getAsString();
-      }
+      llvm::errs() << ": " << Loc.getFilename() << ' '
+                           << getFunctionName(D) << '\n';
     }
   }
 
   void Initialize(ASTContext &Context) override {
     Ctx = &Context;
     checkerMgr = createCheckerManager(*Opts, PP.getLangOpts(), Plugins,
                                       PP.getDiagnostics());
 
#include "clang/StaticAnalyzer/Core/Analyses.def"
   }
 
   void AddDiagnosticConsumer(PathDiagnosticConsumer *Consumer) override {
     PathConsumers.push_back(Consumer);
   }
 
 private:
   void storeTopLevelDecls(DeclGroupRef DG);
+  std::string getFunctionName(const Decl *D);
 
   /// \brief Check if we should skip (not analyze) the given function.
   AnalysisMode getModeForDecl(Decl *D, AnalysisMode Mode);
 
 };
 } // end anonymous namespace
 
 
void AnalysisConsumer::HandleTranslationUnit(ASTContext &C) {
   NumBlocksInAnalyzedFunctions = FunctionSummaries.getTotalNumBasicBlocks();
   if (NumBlocksInAnalyzedFunctions > 0)
     PercentReachableBlocks =
       (FunctionSummaries.getTotalNumVisitedBasicBlocks() * 100) /
         NumBlocksInAnalyzedFunctions;
 
 }
 
-static std::string getFunctionName(const Decl *D) {
-  if (const ObjCMethodDecl *ID = dyn_cast<ObjCMethodDecl>(D)) {
-    return ID->getSelector().getAsString();
-  }
-  if (const FunctionDecl *ND = dyn_cast<FunctionDecl>(D)) {
-    IdentifierInfo *II = ND->getIdentifier();
-    if (II)
-      return II->getName();
+std::string AnalysisConsumer::getFunctionName(const Decl *D) {
+  std::string Str;
+  llvm::raw_string_ostream OS(Str);
+
+  if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
+    OS << FD->getQualifiedNameAsString();
+
+    // In C++, there are overloads.
+    if (Ctx->getLangOpts().CPlusPlus) {
+      OS << '(';
+      for (const auto &P : FD->parameters()) {
+        if (P != *FD->param_begin())
+          OS << ", ";
+        OS << P->getType().getAsString();
+      }
+      OS << ')';
     }
-  return "";
+
+  } else if (isa<BlockDecl>(D)) {
+    PresumedLoc Loc = Ctx->getSourceManager().getPresumedLoc(D->getLocation());
+
+    if (Loc.isValid()) {
+      OS << "block (line: " << Loc.getLine() << ", col: " << Loc.getColumn()
+         << ')';
+    }
+
+  } else if (const ObjCMethodDecl *OMD = dyn_cast<ObjCMethodDecl>(D)) {
+
+    // FIXME: copy-pasted from CGDebugInfo.cpp.
+    OS << (OMD->isInstanceMethod() ? '-' : '+') << '[';
+    const DeclContext *DC = OMD->getDeclContext();
+    if (const auto *OID = dyn_cast<ObjCImplementationDecl>(DC)) {
+      OS << OID->getName();
+    } else if (const auto *OID = dyn_cast<ObjCInterfaceDecl>(DC)) {
+      OS << OID->getName();
+    } else if (const auto *OC = dyn_cast<ObjCCategoryDecl>(DC)) {
+      if (OC->IsClassExtension()) {
+        OS << OC->getClassInterface()->getName();
+      } else {
+        OS << OC->getIdentifier()->getNameStart() << '('
+           << OC->getIdentifier()->getNameStart() << ')';
+      }
+    } else if (const auto *OCD = dyn_cast<ObjCCategoryImplDecl>(DC)) {
+      OS << ((const NamedDecl *)OCD)->getIdentifier()->getNameStart() << '('
+         << OCD->getIdentifier()->getNameStart() << ')';
+    } else if (isa<ObjCProtocolDecl>(DC)) {
+      // We can extract the type of the class from the self pointer.
+      if (ImplicitParamDecl *SelfDecl = OMD->getSelfDecl()) {
+        QualType ClassTy =
+            cast<ObjCObjectPointerType>(SelfDecl->getType())->getPointeeType();
+        ClassTy.print(OS, PrintingPolicy(LangOptions()));
+      }
+    }
+    OS << ' ' << OMD->getSelector().getAsString() << ']';
+
+  }
+
+  return OS.str();
 }
 
 AnalysisConsumer::AnalysisMode
 AnalysisConsumer::getModeForDecl(Decl *D, AnalysisMode Mode) {
   if (!Opts->AnalyzeSpecificFunction.empty() &&
       getFunctionName(D) != Opts->AnalyzeSpecificFunction)
     return AM_None;
 

cfe/trunk/test/Analysis/analyzeOneFunction.m

-// RUN: %clang_cc1 -analyze -analyze-function="myMethodWithY:withX:" -analyzer-checker=core,osx.cocoa.RetainCount -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -analyze -analyze-function="-[Test1 myMethodWithY:withX:]" -analyzer-checker=core,osx.cocoa.RetainCount -analyzer-store=region -verify %s
 
 typedef signed char BOOL;
 typedef unsigned int NSUInteger;
 typedef struct _NSZone NSZone;
 @class NSInvocation, NSMethodSignature, NSCoder, NSString, NSEnumerator;
 @protocol NSObject  - (BOOL)isEqual:(id)object; @end
 @protocol NSCopying  - (id)copyWithZone:(NSZone *)zone; @end
 @protocol NSCoding  - (void)encodeWithCoder:(NSCoder *)aCoder; @end

cfe/trunk/test/Analysis/analyze_display_progress.cpp

-// RUN: %clang_cc1 -analyze -analyzer-display-progress %s 2>&1 | FileCheck %s
-
-void f() {};
-void g() {};
-void h() {}
-
-struct SomeStruct {
-  void f() {}
-};
-
-struct SomeOtherStruct {
-  void f() {}
-};
-
-namespace ns {
-  struct SomeStruct {
-    void f() {}
-  };
-}
-
-// CHECK: analyze_display_progress.cpp f
-// CHECK: analyze_display_progress.cpp g
-// CHECK: analyze_display_progress.cpp h
-// CHECK: analyze_display_progress.cpp SomeStruct::f
-// CHECK: analyze_display_progress.cpp SomeOtherStruct::f
-// CHECK: analyze_display_progress.cpp ns::SomeStruct::f

cfe/trunk/test/Analysis/analyzer-display-progress.cpp

+// RUN: %clang_cc1 -analyze -analyzer-display-progress %s 2>&1 | FileCheck %s
+
+void f() {};
+void g() {};
+void h() {}
+
+struct SomeStruct {
+  void f() {}
+};
+
+struct SomeOtherStruct {
+  void f() {}
+};
+
+namespace ns {
+  struct SomeStruct {
+    void f(int) {}
+    void f(float, ::SomeStruct) {}
+    void f(float, SomeStruct) {}
+  };
+}
+
+// CHECK: analyzer-display-progress.cpp f()
+// CHECK: analyzer-display-progress.cpp g()
+// CHECK: analyzer-display-progress.cpp h()
+// CHECK: analyzer-display-progress.cpp SomeStruct::f()
+// CHECK: analyzer-display-progress.cpp SomeOtherStruct::f()
+// CHECK: analyzer-display-progress.cpp ns::SomeStruct::f(int)
+// CHECK: analyzer-display-progress.cpp ns::SomeStruct::f(float, ::SomeStruct)
+// CHECK: analyzer-display-progress.cpp ns::SomeStruct::f(float, struct ns::SomeStruct)

cfe/trunk/test/Analysis/analyzer-display-progress.m

+// RUN: %clang_cc1 -fblocks -analyze -analyzer-display-progress %s 2>&1 | FileCheck %s
+
+#include "Inputs/system-header-simulator-objc.h"
+
+static void f() {}
+
+@interface I: NSObject
+-(void)instanceMethod:(int)arg1 with:(int)arg2;
++(void)classMethod;
+@end
+
+@implementation I
+-(void)instanceMethod:(int)arg1 with:(int)arg2 {}
++(void)classMethod {}
+@end
+
+void g(I *i, int x, int y) {
+  [I classMethod];
+  [i instanceMethod: x with: y];
+
+  void (^block)(void);
+  block = ^{};
+  block();
+}
+
+// CHECK: analyzer-display-progress.m f
+// CHECK: analyzer-display-progress.m -[I instanceMethod:with:]
+// CHECK: analyzer-display-progress.m +[I classMethod]
+// CHECK: analyzer-display-progress.m g
+// CHECK: analyzer-display-progress.m block (line: 22, col: 11)