r203299 - In my tests, I'm finding that declaring iterators in terms of ranges can sometimes have dangerous side-effects where the range temporary is destroyed, taking the underlying iterators out with it.

Aaron Ballman aaron at aaronballman.com
Fri Mar 7 14:17:20 PST 2014


Author: aaronballman
Date: Fri Mar  7 16:17:20 2014
New Revision: 203299

URL: http://llvm.org/viewvc/llvm-project?rev=203299&view=rev
Log:
In my tests, I'm finding that declaring iterators in terms of ranges can sometimes have dangerous side-effects where the range temporary is destroyed, taking the underlying iterators out with it.

This changes the iterators so that they are no longer implemented in terms of ranges (so it's a very partial revert of the existing rangification efforts).

Modified:
    cfe/trunk/include/clang/AST/Decl.h
    cfe/trunk/include/clang/AST/DeclBase.h
    cfe/trunk/include/clang/AST/DeclObjC.h
    cfe/trunk/include/clang/AST/Redeclarable.h
    cfe/trunk/lib/AST/DeclBase.cpp

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=203299&r1=203298&r2=203299&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Fri Mar  7 16:17:20 2014
@@ -1846,14 +1846,20 @@ public:
   typedef llvm::iterator_range<param_iterator> param_range;
   typedef llvm::iterator_range<param_const_iterator> param_const_range;
 
-  param_iterator param_begin() { return params().begin(); }
-  param_iterator param_end()   { return params().end(); }
+  param_iterator param_begin() { return param_iterator(ParamInfo); }
+  param_iterator param_end() {
+    return param_iterator(ParamInfo + param_size());
+  }
   param_range params() {
     return param_range(ParamInfo, ParamInfo + param_size());
   }
 
-  param_const_iterator param_begin() const { return params().begin(); }
-  param_const_iterator param_end() const   { return params().end(); }
+  param_const_iterator param_begin() const {
+    return param_const_iterator(ParamInfo);
+  }
+  param_const_iterator param_end() const {
+    return param_const_iterator(ParamInfo + param_size());
+  }
   param_const_range params() const {
     return param_const_range(ParamInfo, ParamInfo + param_size());
   }

Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=203299&r1=203298&r2=203299&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Fri Mar  7 16:17:20 2014
@@ -780,8 +780,10 @@ public:
                         redecl_iterator());
   }
 
-  redecl_iterator redecls_begin() const { return redecls().begin(); }
-  redecl_iterator redecls_end() const { return redecls().end(); }
+  redecl_iterator redecls_begin() const {
+    return redecl_iterator(const_cast<Decl *>(this));
+  }
+  redecl_iterator redecls_end() const { return redecl_iterator(); }
 
   /// \brief Retrieve the previous declaration that declares the same entity
   /// as this declaration, or NULL if there is no previous declaration.
@@ -1311,16 +1313,16 @@ public:
   /// decls_begin/decls_end - Iterate over the declarations stored in
   /// this context.
   decl_range decls() const;
-  decl_iterator decls_begin() const { return decls().begin(); }
-  decl_iterator decls_end() const { return decls().end(); }
+  decl_iterator decls_begin() const;
+  decl_iterator decls_end() const { return decl_iterator(); }
   bool decls_empty() const;
 
   /// noload_decls_begin/end - Iterate over the declarations stored in this
   /// context that are currently loaded; don't attempt to retrieve anything
   /// from an external source.
   decl_range noload_decls() const;
-  decl_iterator noload_decls_begin() const { return noload_decls().begin(); }
-  decl_iterator noload_decls_end() const { return noload_decls().end(); }
+  decl_iterator noload_decls_begin() const;
+  decl_iterator noload_decls_end() const { return decl_iterator(); }
 
   /// specific_decl_iterator - Iterates over a subrange of
   /// declarations stored in a DeclContext, providing only those that

Modified: cfe/trunk/include/clang/AST/DeclObjC.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=203299&r1=203298&r2=203299&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclObjC.h (original)
+++ cfe/trunk/include/clang/AST/DeclObjC.h Fri Mar  7 16:17:20 2014
@@ -352,10 +352,14 @@ public:
     return param_const_range(getParams(), getParams() + NumParams);
   }
 
-  param_const_iterator param_begin() const { return params().begin(); }
-  param_const_iterator param_end() const { return params().end(); }
-  param_iterator param_begin() { return params().begin(); }
-  param_iterator param_end() { return params().end(); }
+  param_const_iterator param_begin() const {
+    return param_const_iterator(getParams());
+  }
+  param_const_iterator param_end() const {
+    return param_const_iterator(getParams() + NumParams);
+  }
+  param_iterator param_begin() { return param_iterator(getParams()); }
+  param_iterator param_end() { return param_iterator(getParams() + NumParams); }
 
   // This method returns and of the parameters which are part of the selector
   // name mangling requirements.

Modified: cfe/trunk/include/clang/AST/Redeclarable.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Redeclarable.h?rev=203299&r1=203298&r2=203299&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Redeclarable.h (original)
+++ cfe/trunk/include/clang/AST/Redeclarable.h Fri Mar  7 16:17:20 2014
@@ -171,8 +171,11 @@ public:
                         redecl_iterator());
   }
 
-  redecl_iterator redecls_begin() const { return redecls().begin(); }
-  redecl_iterator redecls_end() const { return redecls().end(); }
+  redecl_iterator redecls_begin() const {
+    return redecl_iterator(
+        const_cast<decl_type *>(static_cast<const decl_type *>(this)));
+  }
+  redecl_iterator redecls_end() const { return redecl_iterator(); }
 
   friend class ASTDeclReader;
   friend class ASTDeclWriter;

Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=203299&r1=203298&r2=203299&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Fri Mar  7 16:17:20 2014
@@ -1080,12 +1080,22 @@ DeclContext::decl_range DeclContext::nol
   return decl_range(decl_iterator(FirstDecl), decl_iterator());
 }
 
+DeclContext::decl_iterator DeclContext::noload_decls_begin() const {
+  return decl_iterator(FirstDecl);
+}
+
 DeclContext::decl_range DeclContext::decls() const {
   if (hasExternalLexicalStorage())
     LoadLexicalDeclsFromExternalStorage();
   return decl_range(decl_iterator(FirstDecl), decl_iterator());
 }
 
+DeclContext::decl_iterator DeclContext::decls_begin() const {
+  if (hasExternalLexicalStorage())
+    LoadLexicalDeclsFromExternalStorage();
+  return decl_iterator(FirstDecl);
+}
+
 bool DeclContext::decls_empty() const {
   if (hasExternalLexicalStorage())
     LoadLexicalDeclsFromExternalStorage();





More information about the cfe-commits mailing list