[PATCH] Consolidate several functions for checking the std namespace into one function in Decl and one function in DeclContext

Richard Smith richard at metafoo.co.uk
Sun Apr 13 18:13:15 PDT 2014


  I think you can remove `isStdNamespace` entirely and keep just `isInStdNamespace`, with a little tweaking. Maybe also add a flag to it to indicate whether we should consider (non-`inline`) namespaces within namespace `std` as "in" `std`.


================
Comment at: lib/AST/DeclBase.cpp:798
@@ +797,3 @@
+bool DeclContext::isStdNamespace() const {
+  const DeclContext *DC = getRedeclContext();
+
----------------
I think the `getRedeclContext` check should be in the callers, not here. Given:

namespace std {
  extern "C" { ... }
}

... I would not expect the `extern "C"` `DeclContext` to claim to be the `std` namespace.

================
Comment at: lib/AST/DeclBase.cpp:803-805
@@ +802,5 @@
+
+  const NamespaceDecl *ND = cast<NamespaceDecl>(DC);
+  if (ND->isInline())
+    return DC->getParent()->isStdNamespace();
+
----------------
Likewise, I'd expect this to be in `isInStdNamespace`. I would not expect libc++'s `std::__1` to claim to be the `std` namespace; it's a namespace within that one, sure, but it's not `std`.

================
Comment at: lib/Sema/SemaExceptionSpec.cpp:472-474
@@ -471,10 +471,5 @@
           // It's called bad_alloc, but is it in std?
           DeclContext* DC = ExRecord->getDeclContext();
           DC = DC->getEnclosingNamespaceContext();
-          if (NamespaceDecl* NS = dyn_cast<NamespaceDecl>(DC)) {
-            IdentifierInfo* NSName = NS->getIdentifier();
-            DC = DC->getParent();
-            if (NSName && NSName->getName() == "std" &&
-                DC->getEnclosingNamespaceContext()->isTranslationUnit()) {
-              return false;
-            }
+          if (DC->isStdNamespace()) {
+            return false;
----------------
Maybe just `ExRecord->isInStdNamespace`?

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:883
@@ -882,6 +882,3 @@
           DeclContext *DCParent2 = DCParent->getParent();
-          if (DCParent2->isNamespace() &&
-              cast<NamespaceDecl>(DCParent2)->getIdentifier() &&
-              cast<NamespaceDecl>(DCParent2)->getIdentifier()->isStr("std") &&
-              DCParent2->getParent()->isTranslationUnit())
+          if (DCParent2->isStdNamespace())
             Complain = false;
----------------
`DCParent->isInStdNamespace`?

================
Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1516
@@ -1530,3 +1515,3 @@
 
-  if (isInStdNamespace(D)) {
+  if (D->getDeclContext()->getEnclosingNamespaceContext()->isStdNamespace()) {
     // Skip reports within the 'std' namespace. Although these can sometimes be
----------------
This doesn't look right; this was checking whether the declaration is transitively within namespace `std` and now returns `false` if it's within a (non-`inline`) namespace within `std` (such as `std::tr1` or `std::experimental::file_system`).

================
Comment at: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:748
@@ -762,3 +747,3 @@
         if (Ctx.getSourceManager().isInSystemHeader(FD->getLocation()))
-          if (IsInStdNamespace(FD))
+          if (FD->getEnclosingNamespaceContext()->isStdNamespace())
             return false;
----------------
Likewise.


http://reviews.llvm.org/D3333






More information about the cfe-commits mailing list