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

[rtrieu]

Summary

Create Decl::isInStdNamespace() and DeclContext::isStdNamespace() for namespace checking. This will replace six implementations of std namespace checkers.

The std namespace checker in the Itanium mangler has been left since it performs additional checks with its checker.

[rsmith]

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.

lib/AST/DeclBase.cpp

798

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.

803–805

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.

lib/Sema/SemaExceptionSpec.cpp

472–474

Maybe just ExRecord->isInStdNamespace?

lib/Sema/SemaTemplateInstantiateDecl.cpp

883

DCParent->isInStdNamespace?

lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

1516

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).

lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

748

Likewise.

[rtrieu]

Add parameter for checking within inline namespaces. Reworked some of the callsites to keep original behavior.

[rtrieu]

Take two, with proper diff context this time.

[rsmith]

+bool DeclContext::isStdNamespace(bool IncludeInlineNamespace) const {
[...]
+ if (!getParent()->getRedeclContext()->isTranslationUnit())
+ return false;

This returns the wrong result if 'std' is itself in an inline namespace,
but I guess we don't need to care about that.

I think the default of IncludeInlineNamespace should be true, not false.
The call in SemaExceptionSpec should set it to 'true', as should all the
calls in the static analyzer, and that only leaves the
SemaTemplateInstantiateDecl call, where we don't care. Can the parameter be
removed altogether?

[rtrieu]

Remove parameter for checking inside inline namespaces and just do it by default. No tests affected.

[rsmith]

LGTM, thanks!

[rtrieu]

Committed at r209708.

include/clang/AST/DeclBase.h

 
   bool isInAnonymousNamespace() const;
 
+  bool isInStdNamespace(bool IncludeInlineNamespaces = false) const;
+
   ASTContext &getASTContext() const LLVM_READONLY;
 
   void setAccess(AccessSpecifier AS) {

     return DeclKind == Decl::Namespace;
   }
 
+  bool isStdNamespace(bool IncludeInlineNamespace = false) const;
+
   bool isInlineNamespace() const;
 
   /// \brief Determines whether this context is dependent on a

lib/AST/Decl.cpp

   return (proto->getParamType(1).getCanonicalType() == Context.VoidPtrTy);
 }
 
-static bool isNamespaceStd(const DeclContext *DC) {
-  const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC->getRedeclContext());
-  return ND && isNamed(ND, "std") &&
-         ND->getParent()->getRedeclContext()->isTranslationUnit();
-}
-
 bool FunctionDecl::isReplaceableGlobalAllocationFunction() const {
   if (getDeclName().getNameKind() != DeclarationName::CXXOperatorName)
     return false;

   Ty = Ty->getPointeeType();
   if (Ty.getCVRQualifiers() != Qualifiers::Const)
     return false;
-  // FIXME: Recognise nothrow_t in an inline namespace inside std?
   const CXXRecordDecl *RD = Ty->getAsCXXRecordDecl();
-  return RD && isNamed(RD, "nothrow_t") && isNamespaceStd(RD->getDeclContext());
+  return RD && isNamed(RD, "nothrow_t") &&
+         RD->isInStdNamespace(true /*IncludeInlineNamespace*/);
 }
 
 FunctionDecl *

lib/AST/DeclBase.cpp

   return false;
 }
 
+bool Decl::isInStdNamespace(bool IncludeInlineNamespace) const {
+  return getDeclContext()->isStdNamespace(IncludeInlineNamespace);
+}
+
 TranslationUnitDecl *Decl::getTranslationUnitDecl() {
   if (TranslationUnitDecl *TUD = dyn_cast<TranslationUnitDecl>(this))
     return TUD;

          cast<NamespaceDecl>(this)->isInline();
 }
 
+bool DeclContext::isStdNamespace(bool IncludeInlineNamespace) const {
+  if (!isNamespace())
+    return false;
+
+  const NamespaceDecl *ND = cast<NamespaceDecl>(this);
+  if (ND->isInline()) {
+    if (!IncludeInlineNamespace)
+      return false;
+    return ND->getParent()->isStdNamespace();
+  }
+
+  if (!getParent()->getRedeclContext()->isTranslationUnit())
+    return false;
+
+  const IdentifierInfo *II = ND->getIdentifier();
+  return II && II->isStr("std");
+}
+
 bool DeclContext::isDependentContext() const {
   if (isFileContext())
     return false;

lib/Analysis/Consumed.cpp

   }
 }
 
-static bool isStdNamespace(const DeclContext *DC) {
-  if (!DC->isNamespace()) return false;
-  while (DC->getParent()->isNamespace())
-    DC = DC->getParent();
-  const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC);
-
-  return ND && ND->getName() == "std" &&
-         ND->getDeclContext()->isTranslationUnit();
-}
-
 void ConsumedStmtVisitor::VisitCallExpr(const CallExpr *Call) {
   const FunctionDecl *FunDecl = Call->getDirectCallee();
   if (!FunDecl)

   // TODO: Make this more specific. (Deferred)
   if (Call->getNumArgs() == 1 &&
       FunDecl->getNameAsString() == "move" &&
-      isStdNamespace(FunDecl->getDeclContext())) {
+      FunDecl->isInStdNamespace(true /*IncludeInlineNamespace*/)) {
     copyInfo(Call->getArg(0), Call, CS_Consumed);
     return;
   }

lib/Sema/SemaExceptionSpec.cpp

         IdentifierInfo* Name = ExRecord->getIdentifier();
         if (Name && Name->getName() == "bad_alloc") {
           // 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 (ExRecord->isInStdNamespace()) {
+            return false;
           }
         }
       }

lib/Sema/SemaTemplateInstantiateDecl.cpp

         if (DCParent->isNamespace() &&
             cast<NamespaceDecl>(DCParent)->getIdentifier() &&
             cast<NamespaceDecl>(DCParent)->getIdentifier()->isStr("tr1")) {
-          DeclContext *DCParent2 = DCParent->getParent();
-          if (DCParent2->isNamespace() &&
-              cast<NamespaceDecl>(DCParent2)->getIdentifier() &&
-              cast<NamespaceDecl>(DCParent2)->getIdentifier()->isStr("std") &&
-              DCParent2->getParent()->isTranslationUnit())
+          if (cast<Decl>(DCParent)->isInStdNamespace())
             Complain = false;
         }
       }

lib/StaticAnalyzer/Checkers/LLVMConventionsChecker.cpp

 
   const TypedefNameDecl *TD = TT->getDecl();
 
-  if (!InNamespace(TD, "std"))
+  if (!TD->isInStdNamespace())
     return false;
 
   return TD->getName() == "string";

lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

   while (const NamespaceDecl *Parent = dyn_cast<NamespaceDecl>(ND->getParent()))
     ND = Parent;
 
-  return ND->getName() == "std";
+  return ND->isStdNamespace();
 }
 
 

lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

   const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC);
   if (!ND)
     return false;
-  
   while (const DeclContext *Parent = ND->getParent()) {
     if (!isa<NamespaceDecl>(Parent))
       break;
     ND = cast<NamespaceDecl>(Parent);
   }
 
-  return ND->getName() == "std";
+  return ND->isStdNamespace();
 }
 
 // The GDM component containing the dynamic dispatch bifurcation info. When