[PATCH] D115936: [ASTMatcher] Add a new matcher called isInTopNamespace

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/AST/DeclBase.h:465-467
+  /// Helper to check if a Decl in a specific top level namespace, while inline
+  /// namespaces will be skipped.
+  bool isInTopNamespace(llvm::StringRef) const;
----------------
I'm not entirely sold on this API as it's super specific to just one use case of checking the initial namespace component -- that can already be done trivially by calling `getQualifiedNameAsString()` and doing a string compare up to the first `::`, basically (at least for named declarations).

Also, it's still not clear whether this is talking about a lexical or semantic lookup. e.g.,
```
namespace foo {
void bar(); // This decl is lexically and semantically in foo
}
void foo::bar() {} // This decl is lexically in the global namespace but is semantically within foo.
```
Does `isInTopNamespace("foo")` return true for both declarations, or just the first?

I think it'd be nice to generalize this a bit more, into something like `bool isInNamespace(llvm::StringRef FullyQualifiedNamespace, LookupContext Kind)` (and make a new enum for `LookupContext` [feel free to pick a better name, too] that specifies semantic vs lexical so the user can pick which behavior they want).

However, even this is a bit of a tough API because of namespace aliases. e.g.,
```
namespace foo {
  void bar();
}

namespace frobble = foo;

void frobble::bar() {} // What should isInNamespace("foo") do here? My inclination is to return true, maybe?
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115936/new/

https://reviews.llvm.org/D115936