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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
