[PATCH] Correctly record "used" markings in AST files which reference other AST files

Eli Friedman eli.friedman at gmail.com
Tue Sep 3 16:19:12 PDT 2013


Patch attached.  I think this is mostly straightforward, but I'l like to
get this reviewed by someone more familiar with the serialization code.

Fixes PR16635.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130903/324b58a6/attachment.html>
-------------- next part --------------
diff --git a/include/clang/AST/ASTMutationListener.h b/include/clang/AST/ASTMutationListener.h
index ab0d79a..6d12a92 100644
--- a/include/clang/AST/ASTMutationListener.h
+++ b/include/clang/AST/ASTMutationListener.h
@@ -90,6 +90,11 @@ public:
                                             const ObjCPropertyDecl *OrigProp,
                                             const ObjCCategoryDecl *ClassExt) {}
 
+  /// \brief A declaration is marked used which was not previously marked used.
+  ///
+  /// \param D the declaration marked used
+  virtual void DeclarationMarkedUsed(const Decl *D) {}
+
   // NOTE: If new methods are added they should also be added to
   // MultiplexASTMutationListener.
 };
diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h
index 71b22ab..2dbfa26 100644
--- a/include/clang/AST/DeclBase.h
+++ b/include/clang/AST/DeclBase.h
@@ -500,7 +500,16 @@ public:
   /// whether the function is used.
   bool isUsed(bool CheckUsedAttr = true) const;
 
-  void setUsed(bool U = true) { Used = U; }
+  /// \brief Set whether the declaration is used, in the sense of odr-use.
+  ///
+  /// This should only be used immediately after creating a declaration.
+  void setIsUsed(bool U) { Used = U; }
+
+  /// \brief Mark the declaration used, in the sense of odr-use.
+  ///
+  /// This notifies any mutation listeners in addition to setting a bit
+  /// indicating the declaration is used.
+  void markUsed(ASTContext &C);
 
   /// \brief Whether this declaration was referenced.
   bool isReferenced() const;
diff --git a/include/clang/Serialization/ASTWriter.h b/include/clang/Serialization/ASTWriter.h
index 9aa0dd7..07fdd06 100644
--- a/include/clang/Serialization/ASTWriter.h
+++ b/include/clang/Serialization/ASTWriter.h
@@ -743,6 +743,7 @@ public:
   virtual void AddedObjCPropertyInClassExtension(const ObjCPropertyDecl *Prop,
                                             const ObjCPropertyDecl *OrigProp,
                                             const ObjCCategoryDecl *ClassExt);
+  void DeclarationMarkedUsed(const Decl *D) LLVM_OVERRIDE;
 };
 
 /// \brief AST and semantic-analysis consumer that generates a
diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp
index e46d671..e4d4e77 100644
--- a/lib/AST/DeclBase.cpp
+++ b/lib/AST/DeclBase.cpp
@@ -291,6 +291,16 @@ bool Decl::isUsed(bool CheckUsedAttr) const {
   return false; 
 }
 
+void Decl::markUsed(ASTContext &C) {
+  if (Used)
+    return;
+
+  if (C.getASTMutationListener())
+    C.getASTMutationListener()->DeclarationMarkedUsed(this);
+
+  Used = true;
+}
+
 bool Decl::isReferenced() const { 
   if (Referenced)
     return true;
diff --git a/lib/Frontend/MultiplexConsumer.cpp b/lib/Frontend/MultiplexConsumer.cpp
index f8ee36a..9cf68a5 100644
--- a/lib/Frontend/MultiplexConsumer.cpp
+++ b/lib/Frontend/MultiplexConsumer.cpp
@@ -107,6 +107,8 @@ public:
   virtual void AddedObjCPropertyInClassExtension(const ObjCPropertyDecl *Prop,
                                             const ObjCPropertyDecl *OrigProp,
                                             const ObjCCategoryDecl *ClassExt);
+  void DeclarationMarkedUsed(const Decl *D) LLVM_OVERRIDE;
+
 private:
   std::vector<ASTMutationListener*> Listeners;
 };
@@ -175,6 +177,10 @@ void MultiplexASTMutationListener::AddedObjCPropertyInClassExtension(
   for (size_t i = 0, e = Listeners.size(); i != e; ++i)
     Listeners[i]->AddedObjCPropertyInClassExtension(Prop, OrigProp, ClassExt);
 }
+void MultiplexASTMutationListener::DeclarationMarkedUsed(const Decl *D) {
+  for (size_t i = 0, e = Listeners.size(); i != e; ++i)
+    Listeners[i]->DeclarationMarkedUsed(D);
+}
 
 }  // end namespace clang
 
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 78942af..68df00d 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -2739,8 +2739,7 @@ bool Sema::MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old,
     New->setPure();
 
   // Merge "used" flag.
-  if (Old->isUsed(false))
-    New->setUsed();
+  New->setIsUsed(Old->isUsed(false));
 
   // Merge attributes from the parameters.  These can mismatch with K&R
   // declarations.
@@ -3050,8 +3049,7 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) {
   }
 
   // Merge "used" flag.
-  if (Old->isUsed(false))
-    New->setUsed();
+  New->setIsUsed(Old->isUsed(false));
 
   // Keep a chain of previous declarations.
   New->setPreviousDeclaration(Old);
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 71b32e5..01f3b7c 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -7951,7 +7951,7 @@ void Sema::DefineImplicitDefaultConstructor(SourceLocation CurrentLocation,
   SourceLocation Loc = Constructor->getLocation();
   Constructor->setBody(new (Context) CompoundStmt(Loc));
 
-  Constructor->setUsed();
+  Constructor->markUsed(Context);
   MarkVTableUsed(CurrentLocation, ClassDecl);
 
   if (ASTMutationListener *L = getASTMutationListener()) {
@@ -8289,7 +8289,7 @@ void Sema::DefineInheritingConstructor(SourceLocation CurrentLocation,
   SourceLocation Loc = Constructor->getLocation();
   Constructor->setBody(new (Context) CompoundStmt(Loc));
 
-  Constructor->setUsed();
+  Constructor->markUsed(Context);
   MarkVTableUsed(CurrentLocation, ClassDecl);
 
   if (ASTMutationListener *L = getASTMutationListener()) {
@@ -8421,7 +8421,7 @@ void Sema::DefineImplicitDestructor(SourceLocation CurrentLocation,
 
   SourceLocation Loc = Destructor->getLocation();
   Destructor->setBody(new (Context) CompoundStmt(Loc));
-  Destructor->setUsed();
+  Destructor->markUsed(Context);
   MarkVTableUsed(CurrentLocation, ClassDecl);
 
   if (ASTMutationListener *L = getASTMutationListener()) {
@@ -9117,7 +9117,7 @@ void Sema::DefineImplicitCopyAssignment(SourceLocation CurrentLocation,
   if (getLangOpts().CPlusPlus11 && CopyAssignOperator->isImplicit())
     diagnoseDeprecatedCopyOperation(*this, CopyAssignOperator, CurrentLocation);
 
-  CopyAssignOperator->setUsed();
+  CopyAssignOperator->markUsed(Context);
 
   SynthesizedFunctionScope Scope(*this, CopyAssignOperator);
   DiagnosticErrorTrap Trap(Diags);
@@ -9562,7 +9562,7 @@ void Sema::DefineImplicitMoveAssignment(SourceLocation CurrentLocation,
     return;
   }
   
-  MoveAssignOperator->setUsed();
+  MoveAssignOperator->markUsed(Context);
 
   SynthesizedFunctionScope Scope(*this, MoveAssignOperator);
   DiagnosticErrorTrap Trap(Diags);
@@ -9915,7 +9915,7 @@ void Sema::DefineImplicitCopyConstructor(SourceLocation CurrentLocation,
         /*isStmtExpr=*/ false).takeAs<Stmt>());
   }
 
-  CopyConstructor->setUsed();
+  CopyConstructor->markUsed(Context);
   if (ASTMutationListener *L = getASTMutationListener()) {
     L->CompletedImplicitDefinition(CopyConstructor);
   }
@@ -10101,7 +10101,7 @@ void Sema::DefineImplicitMoveConstructor(SourceLocation CurrentLocation,
         /*isStmtExpr=*/ false).takeAs<Stmt>());
   }
 
-  MoveConstructor->setUsed();
+  MoveConstructor->markUsed(Context);
 
   if (ASTMutationListener *L = getASTMutationListener()) {
     L->CompletedImplicitDefinition(MoveConstructor);
@@ -10119,7 +10119,7 @@ static void markLambdaCallOperatorUsed(Sema &S, CXXRecordDecl *Lambda) {
         Lambda->lookup(
           S.Context.DeclarationNames.getCXXOperatorName(OO_Call)).front());
   CallOperator->setReferenced();
-  CallOperator->setUsed();
+  CallOperator->markUsed(S.Context);
 }
 
 void Sema::DefineImplicitLambdaToFunctionPointerConversion(
@@ -10131,7 +10131,7 @@ void Sema::DefineImplicitLambdaToFunctionPointerConversion(
   // Make sure that the lambda call operator is marked used.
   markLambdaCallOperatorUsed(*this, Lambda);
   
-  Conv->setUsed();
+  Conv->markUsed(Context);
   
   SynthesizedFunctionScope Scope(*this, Conv);
   DiagnosticErrorTrap Trap(Diags);
@@ -10150,7 +10150,7 @@ void Sema::DefineImplicitLambdaToFunctionPointerConversion(
     
   // Fill in the __invoke function with a dummy implementation. IR generation
   // will fill in the actual details.
-  Invoke->setUsed();
+  Invoke->markUsed(Context);
   Invoke->setReferenced();
   Invoke->setBody(new (Context) CompoundStmt(Conv->getLocation()));
   
@@ -10164,7 +10164,7 @@ void Sema::DefineImplicitLambdaToBlockPointerConversion(
        SourceLocation CurrentLocation,
        CXXConversionDecl *Conv) 
 {
-  Conv->setUsed();
+  Conv->markUsed(Context);
   
   SynthesizedFunctionScope Scope(*this, Conv);
   DiagnosticErrorTrap Trap(Diags);
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 4fc54bd..464d587 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -9623,7 +9623,7 @@ ExprResult Sema::ActOnUnaryOp(Scope *S, SourceLocation OpLoc,
 /// ActOnAddrLabel - Parse the GNU address of label extension: "&&foo".
 ExprResult Sema::ActOnAddrLabel(SourceLocation OpLoc, SourceLocation LabLoc,
                                 LabelDecl *TheDecl) {
-  TheDecl->setUsed();
+  TheDecl->markUsed(Context);
   // Create the AST node.  The address of a label always has type 'void*'.
   return Owned(new (Context) AddrLabelExpr(OpLoc, LabLoc, TheDecl,
                                        Context.getPointerType(Context.VoidTy)));
@@ -11097,7 +11097,7 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, FunctionDecl *Func) {
   // decl in the middle of a decl chain. We loop to maintain the invariant
   // that once a decl is used, all decls after it are also used.
   for (FunctionDecl *F = Func->getMostRecentDecl();; F = F->getPreviousDecl()) {
-    F->setUsed(true);
+    F->markUsed(Context);
     if (F == Func)
       break;
   }
@@ -11172,7 +11172,7 @@ static ExprResult captureInCapturedRegion(Sema &S, CapturedRegionScopeInfo *RSI,
   Expr *Ref = new (S.Context) DeclRefExpr(Var, RefersToEnclosingLocal,
                                           DeclRefType, VK_LValue, Loc);
   Var->setReferenced(true);
-  Var->setUsed(true);
+  Var->markUsed(S.Context);
 
   return Ref;
 }
@@ -11214,7 +11214,7 @@ static ExprResult captureInLambda(Sema &S, LambdaScopeInfo *LSI,
   Expr *Ref = new (S.Context) DeclRefExpr(Var, RefersToEnclosingLocal, 
                                           DeclRefType, VK_LValue, Loc);
   Var->setReferenced(true);
-  Var->setUsed(true);
+  Var->markUsed(S.Context);
 
   // When the field has array type, create index variables for each
   // dimension of the array. We use these index variables to subscript
@@ -11660,7 +11660,7 @@ static void MarkVarDeclODRUsed(Sema &SemaRef, VarDecl *Var,
 
   SemaRef.tryCaptureVariable(Var, Loc);
 
-  Var->setUsed(true);
+  Var->markUsed(SemaRef.Context);
 }
 
 void Sema::UpdateMarkingForLValueToRValue(Expr *E) {
diff --git a/lib/Sema/SemaLambda.cpp b/lib/Sema/SemaLambda.cpp
index 242105f..3a796ad 100644
--- a/lib/Sema/SemaLambda.cpp
+++ b/lib/Sema/SemaLambda.cpp
@@ -1105,7 +1105,7 @@ ExprResult Sema::BuildBlockForLambdaConversion(SourceLocation CurrentLocation,
         Lambda->lookup(
           Context.DeclarationNames.getCXXOperatorName(OO_Call)).front());
   CallOperator->setReferenced();
-  CallOperator->setUsed();
+  CallOperator->markUsed(Context);
 
   ExprResult Init = PerformCopyInitialization(
                       InitializedEntity::InitializeBlock(ConvLocation, 
diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
index 861c1c8..ef2202f 100644
--- a/lib/Sema/SemaStmt.cpp
+++ b/lib/Sema/SemaStmt.cpp
@@ -2041,7 +2041,7 @@ Sema::BuildCXXForRangeStmt(SourceLocation ForLoc, SourceLocation ColonLoc,
 
   if (RangeVarType->isDependentType()) {
     // The range is implicitly used as a placeholder when it is dependent.
-    RangeVar->setUsed();
+    RangeVar->markUsed(Context);
 
     // Deduce any 'auto's in the loop variable as 'DependentTy'. We'll fill
     // them in properly when we instantiate the loop.
@@ -2284,7 +2284,7 @@ StmtResult Sema::ActOnGotoStmt(SourceLocation GotoLoc,
                                SourceLocation LabelLoc,
                                LabelDecl *TheDecl) {
   getCurFunction()->setHasBranchIntoScope();
-  TheDecl->setUsed();
+  TheDecl->markUsed(Context);
   return Owned(new (Context) GotoStmt(TheDecl, GotoLoc, LabelLoc));
 }
 
diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 5d6847f..cc9cb63 100644
--- a/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3352,7 +3352,7 @@ void Sema::BuildVariableInstantiation(
   NewVar->setAccess(OldVar->getAccess());
 
   if (!OldVar->isStaticDataMember()) {
-    NewVar->setUsed(OldVar->isUsed(false));
+    NewVar->setIsUsed(OldVar->isUsed(false));
     NewVar->setReferenced(OldVar->isReferenced());
   }
 
diff --git a/lib/Serialization/ASTCommon.h b/lib/Serialization/ASTCommon.h
index fa3ef58..ef81e69 100644
--- a/lib/Serialization/ASTCommon.h
+++ b/lib/Serialization/ASTCommon.h
@@ -26,7 +26,8 @@ enum DeclUpdateKind {
   UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION,
   UPD_CXX_ADDED_ANONYMOUS_NAMESPACE,
   UPD_CXX_INSTANTIATED_STATIC_DATA_MEMBER,
-  UPD_CXX_DEDUCED_RETURN_TYPE
+  UPD_CXX_DEDUCED_RETURN_TYPE,
+  UPD_DECL_MARKED_USED
 };
 
 TypeIdx TypeIdxFromBuiltin(const BuiltinType *BT);
diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index 67ca94b..b169665 100644
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -377,7 +377,7 @@ void ASTDeclReader::VisitDecl(Decl *D) {
     D->setAttrsImpl(Attrs, Reader.getContext());
   }
   D->setImplicit(Record[Idx++]);
-  D->setUsed(Record[Idx++]);
+  D->setIsUsed(Record[Idx++]);
   D->setReferenced(Record[Idx++]);
   D->setTopLevelDeclInObjCContainer(Record[Idx++]);
   D->setAccess((AccessSpecifier)Record[Idx++]);
@@ -2838,6 +2838,11 @@ void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile,
           FD, Reader.readType(ModuleFile, Record, Idx));
       break;
     }
+
+    case UPD_DECL_MARKED_USED: {
+      D->markUsed(Reader.getContext());
+      break;
+    }
     }
   }
 }
diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp
index f8e8b5d..28a5a42 100644
--- a/lib/Serialization/ASTWriter.cpp
+++ b/lib/Serialization/ASTWriter.cpp
@@ -4314,6 +4314,7 @@ void ASTWriter::ResolveDeclUpdatesBlocks() {
         break;
 
       case UPD_CXX_INSTANTIATED_STATIC_DATA_MEMBER:
+      case UPD_DECL_MARKED_USED:
         ++Idx;
         break;
 
@@ -5370,3 +5371,12 @@ void ASTWriter::AddedObjCPropertyInClassExtension(const ObjCPropertyDecl *Prop,
 
   RewriteDecl(D);
 }
+
+void ASTWriter::DeclarationMarkedUsed(const Decl *D) {
+  assert(!WritingAST && "Already writing the AST!");
+  if (!D->isFromASTFile())
+    return;
+
+  UpdateRecord &Record = DeclUpdates[D];
+  Record.push_back(UPD_DECL_MARKED_USED);
+}
diff --git a/test/Modules/Inputs/System/usr/include/stdio.h b/test/Modules/Inputs/System/usr/include/stdio.h
index 9a7b106..f41e09c 100644
--- a/test/Modules/Inputs/System/usr/include/stdio.h
+++ b/test/Modules/Inputs/System/usr/include/stdio.h
@@ -1,3 +1,3 @@
 typedef struct { int id; } FILE;
 int fprintf(FILE*restrict, const char* restrict format, ...);
-
+extern FILE *__stderrp;
diff --git a/test/Modules/Inputs/pch-used.h b/test/Modules/Inputs/pch-used.h
new file mode 100644
index 0000000..60e0097
--- /dev/null
+++ b/test/Modules/Inputs/pch-used.h
@@ -0,0 +1,2 @@
+ at import cstd.stdio;
+static inline void SPXTrace() { fprintf(__stderrp, ""); }
diff --git a/test/Modules/pch-used.m b/test/Modules/pch-used.m
new file mode 100644
index 0000000..9bdcac2
--- /dev/null
+++ b/test/Modules/pch-used.m
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang_cc1 -x objective-c-header -emit-pch %S/Inputs/pch-used.h -o %t/pch-used.h.pch -fmodules -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include
+// RUN: %clang_cc1 %s -include-pch %t/pch-used.h.pch -fmodules -fmodules-cache-path=%t/cache -O0 -isystem %S/Inputs/System/usr/include -emit-llvm-only
+
+void f() { SPXTrace(); }


More information about the cfe-commits mailing list