[PATCH] D115936: [Clang] Add isInNamespace() to check if a Decl in a specific namespace

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 3 07:55:25 PST 2022


aaron.ballman added a comment.

These changes lack test coverage or uses of the new API, so it's a bit hard to tell whether it's being used correctly or not.



================
Comment at: clang/include/clang/AST/DeclBase.h:466
   bool isInStdNamespace() const;
+  bool isInNamespace(llvm::StringRef Namespace) const;
 
----------------
I think this API needs some comments and explanation. Is the given namespace expected to be fully qualified or will this match partial namespaces? Is it checking whether the declaration is lexically in the namespace or semantically? How does it handle inline namespaces? That sort of thing. e.g.,
```
namespace foo {
void decl();

namespace bar {
int x; // Does isInNamespace("foo") return true for this declaration?
}

inline namespace baz {
int y; // Does isInNamespace("foo") return true for this declaration?
}
}

void foo::decl() {} // Does isInNamespace("foo") return true for this specific declaration?
```


================
Comment at: clang/lib/AST/DeclBase.cpp:396
+
+bool Decl::isInNamespace(llvm::StringRef Namespace) const {
   const DeclContext *DC = getDeclContext();
----------------
Same concerns here as above. It's not at all clear what "in" means for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115936



More information about the cfe-commits mailing list