[analyzer] Reject incomplete matches in CallEvent::isCalled(), also introduce wildcards

[steakhal]

Summary

Previously, if all the parts of the CallDescription were matched, we
accepted the Call.
On the other hand, it would make sense to check if the Call has any
unmatched qualified name parts, that are not inline or anonymous
namespaces.

Consequently, now we reject the Call: my::std::container::data() when
matching against CallDescription: { "std", "container", "data" }.

To get all the tests to work, I needed to introduce wildcard matching.
The wildcard is the "?" string literal.

Using wildcards, now it's possible to match llvm::MyClass::getAs() by
the { "llvm", "?", "getAs" } CallDescription.
Check the related unittest for more examples.

About the semantics of the 'wildcard':
The qualified name parts are matched as usual:
If we mismatch, and the current part was an inline/anonymous namespace,
we skip this and continue by trying to match the rest of the segments.
Else, if we mismatched AND we are not trying to match against a wildcard,
we reject.
Otherwise, we either matched OR consumed a wildcard, so advance to the
next segment.

So, a wildcard can only match against a single name part. They will
match greedily except when it's an inline/anonymous namespace, which
won't consume the wildcard.

[martong]

I think it would be really useful to copy the description and the usage of the "wildcard" you provide in the summary just above the definition of the CallDescription class or above the constructor.

clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

555

Perhaps we could have at least one test when the DeclContext is a CXXRecordDecl (and not a NamespaceDecl).

[steakhal]

In D111795#3064180, @martong wrote:

I think it would be really useful to copy the description and the usage of the "wildcard" you provide in the summary just above the definition of the CallDescription class or above the constructor.

Huh, it seems like I missed nameless structs as well. Check the WildcardNamelessStructs test case, it's sick.

Updates:

• rebased
• IsInlineOrAnonymousNamespace() -> SkippableNameless(), which detects inline/anonymous namespaces, nameless structs.
• added doc comments for wildcards
• added assertion that the function name cannot be a wildcard.
• added FIXME stating that we will use function decl for comparisons

clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

555

Sure!

[martong]

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

350

IsSkippable ? Being unnamed (the term nameless is not used) is just a detail, who knows, maybe in the future we want to have an exclusion for exotic but unnamed AST nodes.

355–359

Actually, there are two 'terms' here: unnamed and anonymous structs and they are different:

struct A { // unnamed structs
  struct { struct A *next; } entry0;
  struct { struct A *next; } entry1;
};

struct X { struct { int a; }; struct { int b; }; }; // anonymous structs

Why does it matter? Because I am not sure that e.g. FieldDecl::isAnonymousStructOrUnion() returns true for an unnamed struct, my guess it does not! (I have some vague memory that I've spent a few hours b/c of this when I worked with the ASTImporter).
Although, I think your code handles both cases, an additional test case would be beneficial. And unfortunately, there could be other anonymous members: unions.

clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

648–650

Yes, I think they would deserve another test case.

[martong]

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

355–359

...I am not sure that e.g. FieldDecl::isAnonymousStructOrUnion() returns true for an unnamed struct, my guess it does not!

Ahh, this is not a problem. Becuase it is impossible to call isAnonymousStructOrUnion on an unnamed struct because by definition there's no FieldDecl with that struct.
Still, I hope the rest of my previous comment makes sense.

[steakhal]

In D111795#3064180, @martong wrote:

I think it would be really useful to copy the description and the usage of the "wildcard" you provide in the summary just above the definition of the CallDescription class or above the constructor.

Definitely! I'll update accordingly.

---

Changes:

• SkippableNameless -> IsSkippable
• add test covering unnamed unions
• add comment why we don't test anonymous structs/unions
• simplify the rejection loop
• add const to a local variable

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

350

Okay, that should be enough.

355–359

Actually, there are two 'terms' here: unnamed and anonymous structs and they are different

Yes, exactly. This is why I decided calling the lambda as unnamed instead of anonymous xD

I have some vague memory that I've spent a few hours

Me too :D

Ahh, this is not a problem. Becuase it is impossible to call isAnonymousStructOrUnion on an unnamed struct because by definition there's no FieldDecl with that struct.

Exactly. I should have left a comment there about this!
You probably meant RecordDecl::isAnonymousStructOrUnion(), and it can actually contain FieldDecls, but no FunctionDecls.
I'm gonna also add a test for unnamed unions.

clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

648–650

One can not define an anonymous struct/union containing a method that we could match for.

[ASDenysPetrov]

Nice extension. Maybe it's worth to think about using regex in the future?

clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

69–71

As I can see this is unrelated changes. And IMO you should move this in the following patch with corresponding test coverage.

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

359

As I can see, implementation of getAsIdentifierInfo never returns nullptr. So we could get by with single Name.isIdentifier() here.

378–391

Nice :) I'm in favor of a such type of self-readable code (brought out const bools) instead of comments around.
The only suggestion here is to enclose the conditions in parentheses.

[ASDenysPetrov]

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

359

As I can see, implementation of getAsIdentifierInfo never returns nullptr. So we could get by with single Name.isIdentifier() here.

An amendment: getAsIdentifierInfo already has isIdentifier inside. So we can just use getAsIdentifierInfo only.

[steakhal]

In D111795#3121749, @ASDenysPetrov wrote:

Nice extension. Maybe it's worth to think about using regex in the future?

We could, but we would definitely pay the price. So, I'm not really a fan of doing that.

clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

69–71

This is a strongly related hunk. The cast-value-logic.cpp:145 would fail otherwise.
The clang::Shape::getAs<Circle>() function would not be matched without this hunk.

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

359

The CallDescription.WildcardUnnamedStructs unittest demonstrates exactly this.
In that case, even though Name.isIdentifier() is true, Name.getAsIdentifierInfo() returns nullptr.
To signify this fact, I'm using an explicit check against nullptr and the comment tries to explain the scenario when it might happen.

378–391

I think it's commonly known that the negation has precedence over the conjunction operator. So, I would insist.

[ASDenysPetrov]

In D111795#3121851, @steakhal wrote:

We could, but we would definitely pay the price. So, I'm not really a fan of doing that.

That's fair. Performance of regex really is a weak place.

clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

69–71

OK, I see.

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

359

getAsIdentifierInfo never returns nullptr

My fault. I was wrong. It can return nullptr. Skip this comments :).

378–391

Oh, sorry, my obscure comment. I mean here:

const bool Mismatch =
          (cast<NamedDecl>(Ctx)->getName() != *QualifierPartsIt);
const bool MatchingAgainstWildcard = (*QualifierPartsIt == Wildcard);

[steakhal]

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

378–391

I see. Makes sense.

[steakhal]

add parens

---

I also plan to update the title and the summary of this patch to emphasize that the major change is about wildcards.
Even though originally purposed as titled.

[ASDenysPetrov]

@steakhal for the changes.
Please, add to summary several examples of the feature usage.

[steakhal]

In D111795#3123899, @ASDenysPetrov wrote:

@steakhal for the changes.
Please, add to summary several examples of the feature usage.

Sure, I will!

I would encourage the rest of the community to have a look at this. I'm really curious about your opinions!
@xazax.hun @NoQ

[xazax.hun]

I like the overall idea of being strict by default and use wild cards to get the previous behavior. I am not convinced, however, about the current matching strategy.

Previously, we could skip arbitrary number of namespaces. As far as I understand, the proposed wild card will match exactly one namespace. While using the new method we can express that there is a namespace that could be anything, we cannot express that there can be any number of namespaces that could be anything.

I think people would expect {"foo", "*", "bar"} to match foo:skipped1:skipped2:bar. We could introduce a separate symbol like ? if we want to support matching a single namespace.
So I think if we want to retain the features we had before this patch we might want to change the matching logic.

But let me know if I misunderstood something.

[xazax.hun]

Alternatively, if we do not want the wildcard to match multiple names, I'd suggest using "?" which is more conventional to match a single item.

[steakhal]

In D111795#3127693, @xazax.hun wrote:

I like the overall idea of being strict by default and use wild cards to get the previous behavior. I am not convinced, however, about the current matching strategy.

Previously, we could skip arbitrary number of namespaces. As far as I understand, the proposed wild card will match exactly one namespace. While using the new method we can express that there is a namespace that could be anything, we cannot express that there can be any number of namespaces that could be anything.

It's true. But it turns out no checker explored this fact - according to the tests.
Do you have any use-case actually in the wild, that we would regress on by having this restriction?

I think people would expect {"foo", "*", "bar"} to match foo:skipped1:skipped2:bar. We could introduce a separate symbol like ? if we want to support matching a single namespace.

I think the ? is a great idea. We shall see if it won't conflict with some ObjC madness.

So I think if we want to retain the features we had before this patch we might want to change the matching logic.

I wouldn't mind, but it would make the matching even more difficult, potentially backtracking (?) which I'm not really a fan of.

[xazax.hun]

In D111795#3127849, @steakhal wrote:

I wouldn't mind, but it would make the matching even more difficult, potentially backtracking (?) which I'm not really a fan of.

I would be surprised if it would need backtracking.

[balazske]

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h

1270 ↗

(On Diff #386253)

This documentation is not the most easy to understand, probably because meaning of "skip" in this and next context seems to be different. Better: like inline namespaces are ignored, and the "*" matches one non-inline namespace (if this is correct).

namespace X { inline namespace X { void f(); } }

Here {"X", "X", "f"} will match or not? And {"X", "*", "f"}, and {"X", "f"}?

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

350

I really do not like the "lambda inside lambda" constructs.

[steakhal]

• Rebased, clang-format
• Changed wildcard sign: "*" -> "?"
• Added the InlineNamespaces test
• Elaborate/rephrase the doc comment about the semantics of the wildcards

[steakhal]

Thanks @balazske, your point is really concerning. I'll need to think about it how to proceed.

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h

1270 ↗

(On Diff #386253)

I see. This is an issue.
Inline namespaces will be consumed only if their name matches the expected qualifier name part. Otherwise, they will be skipped and the matching continues with the same expected qualifier name part.
You will have a match for {"X", "X", "f"}, but won't with {"X", "*", "f"} - which is really counter intuitive :(
Thanks for highlighting this.
You would still have a match for {"X", "f"} and {"*", "f"} though. Check the InlineNamespaces test.

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

350

Let's land D113590 first. After that, I will probably refactor this patch to use static functions instead.

[steakhal]

Let's reach a wider consensus about this. I'm really worried.

[xazax.hun]

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h

1270 ↗

(On Diff #386253)

Whooops, I missed this one. Yeah, let's make this more intuitive before merging.

[balazske]

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h

1270 ↗

(On Diff #386253)

In a code like above a call to X::f() (from not in namespace) would be ambiguous, but X::X::f() is OK. So the allowed match should be for {"X", "X", "f"}, {"X", "?", "f"}, {"?", "X", "f"}, but not {"X", "f"} or {"?", "f"}. But if the second namespace is Y instead of X the {"X", "f"} could match.

[steakhal]

I can't see how one could implement the ? and * placeholders without backtracking. Backtracking is not necessarily bad, but I think it's less than ideal. I haven't done a POC implementation nor any measurements.
So, I'm abandoning this patch.

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h

class CallDescription {
   Optional<unsigned> RequiredArgs;
   Optional<size_t> RequiredParams;
   int Flags;
 
 public:
   /// Constructs a CallDescription object.
   ///
   /// @param QualifiedName The list of the name qualifiers of the function that
-  /// will be matched. The user is allowed to skip any of the qualifiers.
-  /// For example, {"std", "basic_string", "c_str"} would match both
+  /// will be matched. The user is allowed to skip any of the inline namespace
+  /// qualifiers. For example, {"std", "basic_string", "c_str"} would match both
   /// std::basic_string<...>::c_str() and std::__1::basic_string<...>::c_str().
+  /// We also accept 'wildcards', which is a single question mark "?".
+  ///
+  /// A successful match of a qualifier part will consume it.
+  /// A failed match will consume the wildcard if available.
+  /// Inline namespaces will consume a qualifier part only if the expected part
+  /// matches with the name of the inline namespace, otherwise, they will be
+  /// skipped and the match continues without consuming any qualifier part.
+  /// If at the end of the match there are qualifier parts left out, the match
+  /// is rejected.
+  /// The function name cannot be a wildcard.
+  /// For example: {"my", "?", "foo"} would match any name at the wildcard -
+  /// except inline namespaces.
+  /// Check the unittests for more details.
   ///
   /// @param RequiredArgs The number of arguments that is expected to match a
   /// call. Omit this parameter to match every occurrence of call with a given
   /// name regardless the number of arguments.
   CallDescription(int Flags, ArrayRef<const char *> QualifiedName,
                   Optional<unsigned> RequiredArgs = None,
                   Optional<size_t> RequiredParams = None);
 

clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

const CallDescriptionMap<std::pair<CastCheck, CallKind>> CDM = {
       {{{"llvm", "cast"}, 1},
        {&CastValueChecker::evalCast, CallKind::Function}},
       {{{"llvm", "dyn_cast"}, 1},
        {&CastValueChecker::evalDynCast, CallKind::Function}},
       {{{"llvm", "cast_or_null"}, 1},
        {&CastValueChecker::evalCastOrNull, CallKind::Function}},
       {{{"llvm", "dyn_cast_or_null"}, 1},
        {&CastValueChecker::evalDynCastOrNull, CallKind::Function}},
-      {{{"clang", "castAs"}, 0},
+      {{{"clang", "?", "castAs"}, 0},
        {&CastValueChecker::evalCastAs, CallKind::Method}},
-      {{{"clang", "getAs"}, 0},
+      {{{"clang", "?", "getAs"}, 0},
        {&CastValueChecker::evalGetAs, CallKind::Method}},
       {{{"llvm", "isa"}, 1},
        {&CastValueChecker::evalIsa, CallKind::InstanceOf}},
       {{{"llvm", "isa_and_nonnull"}, 1},
        {&CastValueChecker::evalIsaAndNonNull, CallKind::InstanceOf}}};
 
   void evalCast(const CallEvent &Call, DefinedOrUnknownSVal DV,
                 CheckerContext &C) const;

clang/lib/StaticAnalyzer/Core/CallDescription.cpp

 ento::CallDescription::CallDescription(
     int Flags, ArrayRef<const char *> QualifiedName,
     Optional<unsigned> RequiredArgs /*= None*/,
     Optional<size_t> RequiredParams /*= None*/)
     : QualifiedName(QualifiedName), RequiredArgs(RequiredArgs),
       RequiredParams(readRequiredParams(RequiredArgs, RequiredParams)),
       Flags(Flags) {
   assert(!QualifiedName.empty());
+  assert(QualifiedName.back() != StringRef("?"));
 }
 
 /// Construct a CallDescription with default flags.
 ento::CallDescription::CallDescription(
     ArrayRef<const char *> QualifiedName,
     Optional<unsigned> RequiredArgs /*= None*/,
     Optional<size_t> RequiredParams /*= None*/)
     : CallDescription(0, QualifiedName, RequiredArgs, RequiredParams) {}

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

const bool ArgsMatch =
         !CD.RequiredArgs || CD.RequiredArgs == Call.getNumArgs();
     const bool ParamsMatch =
         !CD.RequiredParams || CD.RequiredParams == Call.parameters().size();
     return ArgsMatch && ParamsMatch;
   };
 
   const auto MatchQualifiedNameParts = [](const CallDescription &CD,
                                           const Decl *D) -> bool {
+    const auto IsSkippable = [](const DeclContext *Ctx) -> bool {
+      // Inline or anonymous namespaces.
+      if (const auto *NS = dyn_cast<NamespaceDecl>(Ctx))
+        return NS->isInlineNamespace() || NS->isAnonymousNamespace();
+
+      // Struct/class instances without name, such as `struct {} obj;`.
+      // This is different from the anonymous structs/unions.
+      if (const auto *RD = dyn_cast<RecordDecl>(Ctx)) {
+        const DeclarationName Name = RD->getDeclName();
+        return Name.isIdentifier() && Name.getAsIdentifierInfo() == nullptr;
+      }
+      return false;
+    };
     const auto FindNextNamespaceOrRecord =
         [](const DeclContext *Ctx) -> const DeclContext * {
       while (Ctx && !isa<NamespaceDecl, RecordDecl>(Ctx))
         Ctx = Ctx->getParent();
       return Ctx;
     };
+    constexpr StringRef Wildcard = "?";
 
     auto QualifierPartsIt = CD.begin_qualified_name_parts();
     const auto QualifierPartsEndIt = CD.end_qualified_name_parts();
 
-    // Match namespace and record names. Skip unrelated names if they don't
-    // match.
+    // Match the qualified name parts.
     const DeclContext *Ctx = FindNextNamespaceOrRecord(D->getDeclContext());
     for (; Ctx && QualifierPartsIt != QualifierPartsEndIt;
          Ctx = FindNextNamespaceOrRecord(Ctx->getParent())) {
-      // If not matched just continue and try matching for the next one.
-      if (cast<NamedDecl>(Ctx)->getName() != *QualifierPartsIt)
+      const bool Mismatch =
+          (cast<NamedDecl>(Ctx)->getName() != *QualifierPartsIt);
+      const bool MatchingAgainstWildcard = (*QualifierPartsIt == Wildcard);
+
+      if (Mismatch && IsSkippable(Ctx))
         continue;
+
+      // Otherwise, a wildcard matches anything greedily.
+      if (Mismatch && !MatchingAgainstWildcard)
+        return false;
+
+      // We advance since we either matched, or used a wildcard.
+      assert(!Mismatch || MatchingAgainstWildcard);
       ++QualifierPartsIt;
     }
 
+    // We should check if any named stuff is left, which is not skippable.
+    for (; Ctx; Ctx = Ctx->getParent())
+      if (isa<NamedDecl>(Ctx) && !IsSkippable(Ctx))
+        return false;
+
     // We matched if we consumed all expected qualifier segments.
     return QualifierPartsIt == QualifierPartsEndIt;
   };
 
   // Let's start matching...
   if (!ExactMatchArgAndParamCounts(*this, CD))
     return false;
 

clang/test/Analysis/return-value-guaranteed.cpp

 // RUN: %clang_analyze_cc1 \
 // RUN:  -analyzer-checker=core,apiModeling.llvm.ReturnValue \
-// RUN:  -analyzer-output=text -verify=class %s
+// RUN:  -analyzer-output=text %s
+//
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,apiModeling.llvm.ReturnValue \
+// RUN:  -analyzer-output=text %s -DErrorReturnsFalse
 
 struct Foo { int Field; };
 bool problem();
 void doSomething();
 
 // We predefined the return value of 'MCAsmParser::Error' as true and we cannot
 // take the false-branches which leads to a "garbage value" false positive.
-namespace test_classes {
 struct MCAsmParser {
+#ifdef ErrorReturnsFalse
+  // We predefined 'MCAsmParser::Error' as returning true, but now it returns
+  // false, which breaks our invariant. Test the notes.
+  static bool Error() {
+    return false; // expected-note {{'MCAsmParser::Error' returns false}}
+    // expected-note@-1 {{Returning zero, which participates in a condition later}}
+  }
+#else
   static bool Error();
+#endif
 };
 
+namespace test_classes {
 bool parseFoo(Foo &F) {
   if (problem()) {
-    // class-note@-1 {{Assuming the condition is false}}
-    // class-note@-2 {{Taking false branch}}
+    // expected-note@-1 {{Assuming the condition is false}}
+    // expected-note@-2 {{Taking false branch}}
     return MCAsmParser::Error();
   }
 
   F.Field = 0;
-  // class-note@-1 {{The value 0 is assigned to 'F.Field'}}
+  // expected-note@-1 {{The value 0 is assigned to 'F.Field'}}
   return !MCAsmParser::Error();
-  // class-note@-1 {{'MCAsmParser::Error' returns true}}
-  // class-note@-2 {{Returning zero, which participates in a condition later}}
+  // expected-note@-1 {{'MCAsmParser::Error' returns true}}
+  // expected-note@-2 {{Returning zero, which participates in a condition later}}
 }
 
 bool parseFile() {
   Foo F;
   if (parseFoo(F)) {
-    // class-note@-1 {{Calling 'parseFoo'}}
-    // class-note@-2 {{Returning from 'parseFoo'}}
-    // class-note@-3 {{Taking false branch}}
+    // expected-note@-1 {{Calling 'parseFoo'}}
+    // expected-note@-2 {{Returning from 'parseFoo'}}
+    // expected-note@-3 {{Taking false branch}}
     return true;
   }
 
   if (F.Field == 0) {
-    // class-note@-1 {{Field 'Field' is equal to 0}}
-    // class-note@-2 {{Taking true branch}}
+    // expected-note@-1 {{Field 'Field' is equal to 0}}
+    // expected-note@-2 {{Taking true branch}}
 
     // no-warning: "The left operand of '==' is a garbage value" was here.
     doSomething();
   }
 
   (void)(1 / F.Field);
-  // class-warning@-1 {{Division by zero}}
-  // class-note@-2 {{Division by zero}}
+  // expected-warning@-1 {{Division by zero}}
+  // expected-note@-2 {{Division by zero}}
   return false;
 }
 } // namespace test_classes
 
-
-// We predefined 'MCAsmParser::Error' as returning true, but now it returns
-// false, which breaks our invariant. Test the notes.
 namespace test_break {
-struct MCAsmParser {
-  static bool Error() {
-    return false; // class-note {{'MCAsmParser::Error' returns false}}
-    // class-note@-1 {{Returning zero, which participates in a condition later}}
-  }
-};
-
 bool parseFoo(Foo &F) {
   if (problem()) {
-    // class-note@-1 {{Assuming the condition is false}}
-    // class-note@-2 {{Taking false branch}}
+    // expected-note@-1 {{Assuming the condition is false}}
+    // expected-note@-2 {{Taking false branch}}
     return !MCAsmParser::Error();
   }
 
   F.Field = 0;
-  // class-note@-1 {{The value 0 is assigned to 'F.Field'}}
+  // expected-note@-1 {{The value 0 is assigned to 'F.Field'}}
   return MCAsmParser::Error();
-  // class-note@-1 {{Calling 'MCAsmParser::Error'}}
-  // class-note@-2 {{Returning from 'MCAsmParser::Error'}}
-  // class-note@-3 {{Returning zero, which participates in a condition later}}
+  // expected-note@-1 {{Calling 'MCAsmParser::Error'}}
+  // expected-note@-2 {{Returning from 'MCAsmParser::Error'}}
+  // expected-note@-3 {{Returning zero, which participates in a condition later}}
 }
 
 bool parseFile() {
   Foo F;
   if (parseFoo(F)) {
-    // class-note@-1 {{Calling 'parseFoo'}}
-    // class-note@-2 {{Returning from 'parseFoo'}}
-    // class-note@-3 {{Taking false branch}}
+    // expected-note@-1 {{Calling 'parseFoo'}}
+    // expected-note@-2 {{Returning from 'parseFoo'}}
+    // expected-note@-3 {{Taking false branch}}
     return true;
   }
 
   (void)(1 / F.Field);
-  // class-warning@-1 {{Division by zero}}
-  // class-note@-2 {{Division by zero}}
+  // expected-warning@-1 {{Division by zero}}
+  // expected-note@-2 {{Division by zero}}
   return false;
 }
-} // namespace test_classes
+} // namespace test_break

clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

 
 TEST(CallDescription, QualifiedNames) {
   constexpr StringRef AdditionalCode = R"code(
     void foo() {
       using namespace std;
       basic_string<char> s;
       s.c_str();
     })code";
+
   const std::string Code = (Twine{MockStdStringHeader} + AdditionalCode).str();
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
           {{{"std", "basic_string", "c_str"}}, true},
       })),
       Code));
 }
 
 TEST(CallDescription, MatchConstructor) {
   constexpr StringRef AdditionalCode = R"code(
     void foo() {
       using namespace std;
       basic_string<char> s("hello");
     })code";
+
   const std::string Code = (Twine{MockStdStringHeader} + AdditionalCode).str();
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<FrontendAction>(
           new CallDescriptionAction<CXXConstructExpr>({
               {{{"std", "basic_string", "basic_string"}, 2, 2}, true},
           })),
       Code));
 }
struct Bar {
       operator int();
     };
     } // bbb
     } // aaa
     void foo() {
       aaa::bbb::Bar x;
       int tmp = x;
     })code";
+
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
           {{{"aaa", "bbb", "Bar", "operator int"}}, true},
       })),
       Code));
 }
 
 TEST(CallDescription, RejectOverQualifiedNames) {
   constexpr auto Code = R"code(
     namespace my {
     namespace std {
       struct container {
         const char *data() const;
       };
     } // namespace std
     } // namespace my
 
     void foo() {
       using namespace my;
       std::container v;
       v.data();
     })code";
 
-  // FIXME: We should **not** match.
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-          {{{"std", "container", "data"}}, true},
+          {{{"std", "container", "data"}}, false},
       })),
       Code));
 }
 
 TEST(CallDescription, DontSkipNonInlineNamespaces) {
   constexpr auto Code = R"code(
     namespace my {
     /*not inline*/ namespace v1 {
       void bar();
     } // namespace v1
     } // namespace my
     void foo() {
       my::v1::bar();
     })code";
 
   {
     SCOPED_TRACE("my v1 bar");
     EXPECT_TRUE(tooling::runToolOnCode(
         std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
             {{{"my", "v1", "bar"}}, true},
         })),
         Code));
   }
   {
-    // FIXME: We should **not** skip non-inline namespaces.
     SCOPED_TRACE("my bar");
     EXPECT_TRUE(tooling::runToolOnCode(
         std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-            {{{"my", "bar"}}, true},
+            {{{"my", "bar"}}, false},
         })),
         Code));
   }
 }
 
 TEST(CallDescription, SkipTopInlineNamespaces) {
   constexpr auto Code = R"code(
     inline namespace my {
constexpr auto Code = R"code(
     EXPECT_TRUE(tooling::runToolOnCode(
         std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
             {{{"v1", "bar"}}, true},
         })),
         Code));
   }
 }
 
-TEST(CallDescription, SkipAnonimousNamespaces) {
+TEST(CallDescription, InlineNamespaces) {
+  constexpr auto Code = R"code(
+    namespace X { inline namespace X { void f(); } }
+    void foo() {
+      X::f();
+    })code";
+
+  // One can refer to 'f' by  'X::f()' or 'X::X::f()' in code.
+  // Hence, it would be intuitive to have matches using any of
+  // {"?", "f"}, {"?", "?", "f"}.
+  // However, wildcards won't consume //inline// namespaces,
+  // thus only {"?", "f"} wildcarded call description will match.
+  {
+    SCOPED_TRACE("X X f");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"X", "X", "f"}}, true},
+        })),
+        Code));
+  }
+  {
+    // Wildcards won't consume //inline// namespaces.
+    SCOPED_TRACE("X ? f");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"X", "?", "f"}}, false},
+        })),
+        Code));
+  }
+  {
+    // FIXME: Since the inline namespace has the name "X", it will eagerly
+    // consume the "X" from the CallDescription, but after that it will end the
+    // match and have unmatched non-skippable name parts, thus we reject the
+    // match.
+    SCOPED_TRACE("X f");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"X", "f"}}, false},
+        })),
+        Code));
+  }
+  {
+    // The wildcard consumed the outer 'X' namespace.
+    SCOPED_TRACE("? f");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"?", "f"}}, true},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("? ? f");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"?", "?", "f"}}, false},
+        })),
+        Code));
+  }
+}
+
+TEST(CallDescription, SkipAnonymousNamespaces) {
   constexpr auto Code = R"code(
     namespace {
     namespace std {
     namespace {
     inline namespace {
       struct container {
         const char *data() const { return nullptr; };
       };
constexpr StringRef Code = R"code(
     namespace ccc {
       void bar();
     }} // namespace bbb::ccc
     namespace bbb_alias = bbb;
     } // namespace aaa
     void foo() {
       aaa::bbb_alias::ccc::bar();
     })code";
+
   {
     SCOPED_TRACE("aaa bbb ccc bar");
     EXPECT_TRUE(tooling::runToolOnCode(
         std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
             {{{"aaa", "bbb", "ccc", "bar"}}, true},
         })),
         Code));
   }
constexpr StringRef Code = R"code(
     namespace ccc {
       void bar();
     }}} // namespace aaa::bbb::ccc
     namespace aaa_bbb_ccc = aaa::bbb::ccc;
     void foo() {
       using namespace aaa_bbb_ccc;
       bar();
     })code";
+
   {
     SCOPED_TRACE("aaa bbb ccc bar");
     EXPECT_TRUE(tooling::runToolOnCode(
         std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
             {{{"aaa", "bbb", "ccc", "bar"}}, true},
         })),
         Code));
   }
EXPECT_TRUE(tooling::runToolOnCode(
           {{{"memset", 3}, false}, {{CDF_MaybeBuiltin, "memset", 3}, true}})),
       "void foo() {"
       "  int x;"
       "  __builtin___memset_chk(&x, 0, sizeof(x),"
       "                         __builtin_object_size(&x, 0));"
       "}"));
 }
 
+TEST(CallDescription, WildcardNamespace) {
+  constexpr StringRef Code = R"code(
+    namespace aaa {
+    namespace bbb {
+      void bar();
+    }} // namespace aaa::bbb
+    void foo() {
+      aaa::bbb::bar();
+    })code";
+
+  {
+    SCOPED_TRACE("? bbb bar");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"?", "bbb", "bar"}}, true},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("aaa ? bar");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"aaa", "?", "bar"}}, true},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("? ? bar");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"?", "?", "bar"}}, true},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("? bar");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"?", "bar"}}, false},
+        })),
+        Code));
+  }
+
+  // Negative cases:
+  {
+    SCOPED_TRACE("? ? ? bar");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"?", "?", "?", "bar"}}, false},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("zzz ? bar");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"zzz", "?", "bar"}}, false},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("? zzz bar");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"?", "zzz", "bar"}}, false},
+        })),
+        Code));
+  }
+}
+
+/// Not testing anonymous structs/unions since they cannot have any methods that
+/// we could match for.
+
+TEST(CallDescription, WildcardUnnamedStructs) {
+  constexpr auto Code = R"code(
+    namespace {
+    namespace std {
+    inline namespace {
+      struct {
+      struct foobar {
+      struct container {
+        const char *data() const { return nullptr; };
+      }; // struct container
+      }; // struct foobar
+      } aaa;
+    } // namespace anonymous
+    } // namespace std
+    } // namespace anonymous
+
+    void foo() {
+      decltype(std::aaa)::foobar::container v;
+      v.data();
+    })code";
+
+  {
+    // This test feels odd, since one cannot refer to the nameless type of
+    // `aaa`. We can circumvent this limitation by using decltype. Frankly, one
+    // cannot declare a variable using `std::foobar::container v`. Note:
+    // anonymous structs/unions represent a different thing, when they are
+    // nameless AND there is no instance of them.
+    SCOPED_TRACE("without wildcard");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"std", "foobar", "container", "data"}}, true},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("? foobar container data");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"?", "foobar", "container", "data"}}, true},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("std ? container data");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"std", "?", "container", "data"}}, true},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("std foobar ? data");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"std", "foobar", "?", "data"}}, true},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("std ? ? data");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"std", "?", "?", "data"}}, true},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("? foobar ? data");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"?", "foobar", "?", "data"}}, true},
+        })),
+        Code));
+  }
+}
+
+// This is the same testcase as WildcardUnnamedStructs, but with unnamed union.
+TEST(CallDescription, WildcardUnnamedUnions) {
+  constexpr auto Code = R"code(
+    namespace {
+    namespace std {
+    inline namespace {
+      union {
+      struct foobar {
+      struct container {
+        const char *data() const { return nullptr; };
+      }; // struct container
+      }; // struct foobar
+      } aaa;
+    } // namespace anonymous
+    } // namespace std
+    } // namespace anonymous
+
+    void foo() {
+      decltype(std::aaa)::foobar::container v;
+      v.data();
+    })code";
+
+  {
+    // Same concerns apply that was for WildcardUnnamedStructs.
+    SCOPED_TRACE("without wildcard");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"std", "foobar", "container", "data"}}, true},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("? foobar container data");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"?", "foobar", "container", "data"}}, true},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("std ? container data");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"std", "?", "container", "data"}}, true},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("std foobar ? data");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"std", "foobar", "?", "data"}}, true},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("std ? ? data");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"std", "?", "?", "data"}}, true},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("? foobar ? data");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"?", "foobar", "?", "data"}}, true},
+        })),
+        Code));
+  }
+}
+
 } // namespace
 } // namespace ento
 } // namespace clang