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

471–473

Maybe just ExRecord->isInStdNamespace?

lib/Sema/SemaTemplateInstantiateDecl.cpp

882

DCParent->isInStdNamespace?

lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

1531

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

763

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

public:
 
   TranslationUnitDecl *getTranslationUnitDecl();
   const TranslationUnitDecl *getTranslationUnitDecl() const {
     return const_cast<Decl*>(this)->getTranslationUnitDecl();
   }
 
   bool isInAnonymousNamespace() const;
 
+  bool isInStdNamespace() const;
+
   ASTContext &getASTContext() const LLVM_READONLY;
 
   void setAccess(AccessSpecifier AS) {
     Access = AS;
     assert(AccessDeclContextSanity());
   }
 
   AccessSpecifier getAccess() const {
public:
   bool isRecord() const {
     return DeclKind >= Decl::firstRecord && DeclKind <= Decl::lastRecord;
   }
 
   bool isNamespace() const {
     return DeclKind == Decl::Namespace;
   }
 
+  bool isStdNamespace() const;
+
   bool isInlineNamespace() const;
 
   /// \brief Determines whether this context is dependent on a
   /// template parameter.
   bool isDependentContext() const;
 
   /// isTransparentContext - Determines whether this context is a
   /// "transparent" context, meaning that the members declared in this

lib/AST/Decl.cpp

ASTContext &Context =
     cast<TranslationUnitDecl>(getDeclContext()->getRedeclContext())
       ->getASTContext();
 
   // The result type and first argument type are constant across all
   // these operators.  The second argument must be exactly void*.
   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;
   if (getDeclName().getCXXOverloadedOperator() != OO_New &&
       getDeclName().getCXXOverloadedOperator() != OO_Delete &&
       getDeclName().getCXXOverloadedOperator() != OO_Array_New &&
       getDeclName().getCXXOverloadedOperator() != OO_Array_Delete)
     return false;
bool FunctionDecl::isReplaceableGlobalAllocationFunction() const {
   if (Ctx.getLangOpts().SizedDeallocation &&
       Ctx.hasSameType(Ty, Ctx.getSizeType()))
     return true;
   if (!Ty->isReferenceType())
     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();
 }
 
 FunctionDecl *
 FunctionDecl::getCorrespondingUnsizedGlobalDeallocationFunction() const {
   ASTContext &Ctx = getASTContext();
   if (!Ctx.getLangOpts().SizedDeallocation)
     return 0;
 

lib/AST/DeclBase.cpp

do {
     if (const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC))
       if (ND->isAnonymousNamespace())
         return true;
   } while ((DC = DC->getParent()));
 
   return false;
 }
 
+bool Decl::isInStdNamespace() const {
+  return getDeclContext()->isStdNamespace();
+}
+
 TranslationUnitDecl *Decl::getTranslationUnitDecl() {
   if (TranslationUnitDecl *TUD = dyn_cast<TranslationUnitDecl>(this))
     return TUD;
 
   DeclContext *DC = getDeclContext();
   assert(DC && "This decl is not contained in a translation unit!");
 
   while (!DC->isTranslationUnit()) {
if (isa<FunctionDecl>(this))
     if (getParent()->getRedeclContext()->isFileContext() &&
         getLexicalParent()->getRedeclContext()->isRecord())
       return getLexicalParent();
   
   return getParent();
 }
 
 bool DeclContext::isInlineNamespace() const {
   return isNamespace() &&
          cast<NamespaceDecl>(this)->isInline();
 }
 
+bool DeclContext::isStdNamespace() const {
+  if (!isNamespace())
+    return false;
+
+  const NamespaceDecl *ND = cast<NamespaceDecl>(this);
+  if (ND->isInline()) {
+    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;
 
   if (isa<ClassTemplatePartialSpecializationDecl>(this))
     return true;
 
   if (const CXXRecordDecl *Record = dyn_cast<CXXRecordDecl>(this)) {

lib/Analysis/Consumed.cpp

case BO_PtrMemI:
     forwardInfo(BinOp->getLHS(), BinOp);
     break;
     
   default:
     break;
   }
 }
 
-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)
     return;
 
   // Special case for the std::move function.
   // TODO: Make this more specific. (Deferred)
-  if (Call->getNumArgs() == 1 &&
-      FunDecl->getNameAsString() == "move" &&
-      isStdNamespace(FunDecl->getDeclContext())) {
+  if (Call->getNumArgs() == 1 && FunDecl->getNameAsString() == "move" &&
+      FunDecl->isInStdNamespace()) {
     copyInfo(Call->getArg(0), Call, CS_Consumed);
     return;
   }
 
   handleCall(Call, 0, FunDecl);
   propagateReturnType(Call, FunDecl);
 }
 

lib/Sema/SemaExceptionSpec.cpp

else if (OldEST == EST_Dynamic && NewEST == EST_None)
       WithExceptions = Old;
     if (WithExceptions && WithExceptions->getNumExceptions() == 1) {
       // One has no spec, the other throw(something). If that something is
       // std::bad_alloc, all conditions are met.
       QualType Exception = *WithExceptions->exception_begin();
       if (CXXRecordDecl *ExRecord = Exception->getAsCXXRecordDecl()) {
         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()) {
 
   // At this point, the only remaining valid case is two matching dynamic
   // specifications. We return here unless both specifications are dynamic.
   if (OldEST != EST_Dynamic || NewEST != EST_Dynamic) {
     if (MissingExceptionSpecification && Old->hasExceptionSpec() &&
         !New->hasExceptionSpec()) {
       // The old type has an exception specification of some sort, but
       // the new type does not.

lib/Sema/SemaTemplateInstantiateDecl.cpp

if (PrevClassTemplate) {
           Pattern->getIdentifier()->isStr("_Map_base") &&
           DC->isNamespace() &&
           cast<NamespaceDecl>(DC)->getIdentifier() &&
           cast<NamespaceDecl>(DC)->getIdentifier()->isStr("__detail")) {
         DeclContext *DCParent = DC->getParent();
         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;
         }
       }
 
       TemplateParameterList *PrevParams
         = PrevClassTemplate->getTemplateParameters();
 
       // Make sure the parameter lists match.

lib/StaticAnalyzer/Checkers/LLVMConventionsChecker.cpp

if (const ElaboratedType *QT = T->getAs<ElaboratedType>())
     T = QT->getNamedType();
 
   const TypedefType *TT = T->getAs<TypedefType>();
   if (!TT)
     return false;
 
   const TypedefNameDecl *TD = TT->getDecl();
 
-  if (!InNamespace(TD, "std"))
+  if (!TD->isInStdNamespace())
     return false;
 
   return TD->getName() == "string";
 }
 
 static bool IsClangType(const RecordDecl *RD) {
   return RD->getName() == "Type" && InNamespace(RD, "clang");
 }

lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

static bool isInStdNamespace(const Decl *D) {
   const DeclContext *DC = D->getDeclContext()->getEnclosingNamespaceContext();
   const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC);
   if (!ND)
     return false;
 
   while (const NamespaceDecl *Parent = dyn_cast<NamespaceDecl>(ND->getParent()))
     ND = Parent;
 
-  return ND->getName() == "std";
+  return ND->isStdNamespace();
 }
 
 
 PathDiagnosticPiece *
 LikelyFalsePositiveSuppressionBRVisitor::getEndPath(BugReporterContext &BRC,
                                                     const ExplodedNode *N,
                                                     BugReport &BR) {
   // Here we suppress false positives coming from system headers. This list is
   // based on known issues.
   ExprEngine &Eng = BRC.getBugReporter().getEngine();
   AnalyzerOptions &Options = Eng.getAnalysisManager().options;
   const Decl *D = N->getLocationContext()->getDecl();
 
   if (isInStdNamespace(D)) {
     // Skip reports within the 'std' namespace. Although these can sometimes be
     // the user's fault, we currently don't report them very well, and
     // Note that this will not help for any other data structure libraries, like
     // TR1, Boost, or llvm/ADT.
     if (Options.shouldSuppressFromCXXStandardLibrary()) {
       BR.markInvalid(getTag(), 0);
       return 0;
 

lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

 
 }
 
 static bool IsInStdNamespace(const FunctionDecl *FD) {
   const DeclContext *DC = FD->getEnclosingNamespaceContext();
   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
 // the exact type of the receiver is not known, we want to explore both paths -
 // one on which we do inline it and the other one on which we don't. This is
 // done to ensure we do not drop coverage.
 // This is the map from the receiver region to a bool, specifying either we
 // consider this region's information precise or not along the given path.
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(CalleeADC->getDecl())) {
       // Conditionally control the inlining of template functions.
       if (!Opts.mayInlineTemplateFunctions())
         if (FD->getTemplatedKind() != FunctionDecl::TK_NonTemplate)
           return false;
 
       // Conditionally control the inlining of C++ standard library functions.
       if (!Opts.mayInlineCXXStandardLibrary())
         if (Ctx.getSourceManager().isInSystemHeader(FD->getLocation()))
           if (IsInStdNamespace(FD))
             return false;
 
       // Conditionally control the inlining of methods on objects that look
       // like C++ containers.
       if (!Opts.mayInlineCXXContainerCtorsAndDtors())
         if (!Ctx.getSourceManager().isInMainFile(FD->getLocation()))
           if (isContainerCtorOrDtor(Ctx, FD))
             return false;