diff options
author | Mathieu Chartier <mathieuc@google.com> | 2013-08-14 16:14:24 -0700 |
---|---|---|
committer | Mathieu Chartier <mathieuc@google.com> | 2013-08-16 13:15:37 -0700 |
commit | 02e25119b15a6f619f17db99f5d05124a5807ff3 (patch) | |
tree | 7be4cbbf28033e5ee0621565b410fe5d8170a8fb | |
parent | 7d70a7932f0ba09eb01a93caab060aef1403d4e6 (diff) | |
download | art-02e25119b15a6f619f17db99f5d05124a5807ff3.zip art-02e25119b15a6f619f17db99f5d05124a5807ff3.tar.gz art-02e25119b15a6f619f17db99f5d05124a5807ff3.tar.bz2 |
Fix up TODO: c++0x, update cpplint.
Needed to update cpplint to handle const auto.
Fixed a few cpplint errors that were being missed before.
Replaced most of the TODO c++0x with ranged based loops. Loops which
do not have a descriptive container name have a concrete type instead
of auto.
Change-Id: Id7cc0f27030f56057c544e94277300b3f298c9c5
49 files changed, 1108 insertions, 722 deletions
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index b8727fe..0ef0428 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -664,8 +664,7 @@ void CompilerDriver::LoadImageClasses(base::TimingLogger& timings) Thread* self = Thread::Current(); ScopedObjectAccess soa(self); ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); - typedef DescriptorSet::iterator It; // TODO: C++0x auto - for (It it = image_classes_->begin(), end = image_classes_->end(); it != end;) { + for (auto it = image_classes_->begin(), end = image_classes_->end(); it != end;) { std::string descriptor(*it); SirtRef<mirror::Class> klass(self, class_linker->FindSystemClass(descriptor.c_str())); if (klass.get() == NULL) { @@ -687,12 +686,9 @@ void CompilerDriver::LoadImageClasses(base::TimingLogger& timings) unresolved_exception_types.clear(); class_linker->VisitClasses(ResolveCatchBlockExceptionsClassVisitor, &unresolved_exception_types); - typedef std::set<std::pair<uint16_t, const DexFile*> >::const_iterator It; // TODO: C++0x auto - for (It it = unresolved_exception_types.begin(), - end = unresolved_exception_types.end(); - it != end; ++it) { - uint16_t exception_type_idx = it->first; - const DexFile* dex_file = it->second; + for (const std::pair<uint16_t, const DexFile*>& exception_type : unresolved_exception_types) { + uint16_t exception_type_idx = exception_type.first; + const DexFile* dex_file = exception_type.second; mirror::DexCache* dex_cache = class_linker->FindDexCache(*dex_file); mirror:: ClassLoader* class_loader = NULL; SirtRef<mirror::Class> klass(self, class_linker->ResolveType(*dex_file, exception_type_idx, diff --git a/compiler/elf_fixup.cc b/compiler/elf_fixup.cc index 6c090fd..359c493 100644 --- a/compiler/elf_fixup.cc +++ b/compiler/elf_fixup.cc @@ -80,7 +80,6 @@ bool ElfFixup::Fixup(File* file, uintptr_t oat_data_begin) { #define DT_MIPS_RLD_MAP 0x70000016 // d_ptr bool ElfFixup::FixupDynamic(ElfFile& elf_file, uintptr_t base_address) { - // TODO: C++0x auto. for (::llvm::ELF::Elf32_Word i = 0; i < elf_file.GetDynamicNum(); i++) { ::llvm::ELF::Elf32_Dyn& elf_dyn = elf_file.GetDynamic(i); ::llvm::ELF::Elf32_Word d_tag = elf_dyn.d_tag; diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc index 4e9ae54..548ea9e 100644 --- a/compiler/image_writer.cc +++ b/compiler/image_writer.cc @@ -74,10 +74,7 @@ bool ImageWriter::Write(const std::string& image_filename, ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); const std::vector<DexCache*>& all_dex_caches = class_linker->GetDexCaches(); - for (size_t i = 0; i < all_dex_caches.size(); i++) { - DexCache* dex_cache = all_dex_caches[i]; - dex_caches_.insert(dex_cache); - } + dex_caches_.insert(all_dex_caches.begin(), all_dex_caches.end()); UniquePtr<File> oat_file(OS::OpenFileReadWrite(oat_filename.c_str())); if (oat_file.get() == NULL) { @@ -117,11 +114,7 @@ bool ImageWriter::Write(const std::string& image_filename, gc::Heap* heap = Runtime::Current()->GetHeap(); heap->CollectGarbage(false); // Remove garbage. // Trim size of alloc spaces. - const std::vector<gc::space::ContinuousSpace*>& spaces = heap->GetContinuousSpaces(); - // TODO: C++0x auto - typedef std::vector<gc::space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), end = spaces.end(); it != end; ++it) { - gc::space::ContinuousSpace* space = *it; + for (const auto& space : heap->GetContinuousSpaces()) { if (space->IsDlMallocSpace()) { space->AsDlMallocSpace()->Trim(); } @@ -163,13 +156,8 @@ bool ImageWriter::Write(const std::string& image_filename, } bool ImageWriter::AllocMemory() { - gc::Heap* heap = Runtime::Current()->GetHeap(); - const std::vector<gc::space::ContinuousSpace*>& spaces = heap->GetContinuousSpaces(); size_t size = 0; - // TODO: C++0x auto - typedef std::vector<gc::space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), end = spaces.end(); it != end; ++it) { - gc::space::ContinuousSpace* space = *it; + for (const auto& space : Runtime::Current()->GetHeap()->GetContinuousSpaces()) { if (space->IsDlMallocSpace()) { size += space->Size(); } @@ -203,9 +191,7 @@ void ImageWriter::ComputeEagerResolvedStringsCallback(Object* obj, void* arg) { String* string = obj->AsString(); const uint16_t* utf16_string = string->GetCharArray()->GetData() + string->GetOffset(); ImageWriter* writer = reinterpret_cast<ImageWriter*>(arg); - typedef Set::const_iterator CacheIt; // TODO: C++0x auto - for (CacheIt it = writer->dex_caches_.begin(), end = writer->dex_caches_.end(); it != end; ++it) { - DexCache* dex_cache = *it; + for (DexCache* dex_cache : writer->dex_caches_) { const DexFile& dex_file = *dex_cache->GetDexFile(); const DexFile::StringId* string_id = dex_file.FindStringId(utf16_string); if (string_id != NULL) { @@ -251,16 +237,13 @@ void ImageWriter::PruneNonImageClasses() { class_linker->VisitClasses(NonImageClassesVisitor, &context); // Remove the undesired classes from the class roots. - typedef std::set<std::string>::const_iterator ClassIt; // TODO: C++0x auto - for (ClassIt it = non_image_classes.begin(), end = non_image_classes.end(); it != end; ++it) { - class_linker->RemoveClass((*it).c_str(), NULL); + for (const std::string& it : non_image_classes) { + class_linker->RemoveClass(it.c_str(), NULL); } // Clear references to removed classes from the DexCaches. ArtMethod* resolution_method = runtime->GetResolutionMethod(); - typedef Set::const_iterator CacheIt; // TODO: C++0x auto - for (CacheIt it = dex_caches_.begin(), end = dex_caches_.end(); it != end; ++it) { - DexCache* dex_cache = *it; + for (DexCache* dex_cache : dex_caches_) { for (size_t i = 0; i < dex_cache->NumResolvedTypes(); i++) { Class* klass = dex_cache->GetResolvedType(i); if (klass != NULL && !IsImageClass(klass)) { @@ -324,9 +307,8 @@ void ImageWriter::CheckNonImageClassesRemovedCallback(Object* obj, void* arg) { void ImageWriter::DumpImageClasses() { CompilerDriver::DescriptorSet* image_classes = compiler_driver_.GetImageClasses(); CHECK(image_classes != NULL); - typedef std::set<std::string>::const_iterator It; // TODO: C++0x auto - for (It it = image_classes->begin(), end = image_classes->end(); it != end; ++it) { - LOG(INFO) << " " << *it; + for (const std::string& image_class : *image_classes) { + LOG(INFO) << " " << image_class; } } @@ -368,9 +350,8 @@ ObjectArray<Object>* ImageWriter::CreateImageRoots() const { ObjectArray<Object>* dex_caches = ObjectArray<Object>::Alloc(self, object_array_class, dex_caches_.size()); int i = 0; - typedef Set::const_iterator It; // TODO: C++0x auto - for (It it = dex_caches_.begin(), end = dex_caches_.end(); it != end; ++it, ++i) { - dex_caches->Set(i, *it); + for (DexCache* dex_cache : dex_caches_) { + dex_caches->Set(i++, dex_cache); } // build an Object[] of the roots needed to restore the runtime @@ -403,7 +384,7 @@ void ImageWriter::CalculateNewObjectOffsets(size_t oat_loaded_size, size_t oat_d SirtRef<ObjectArray<Object> > image_roots(self, CreateImageRoots()); gc::Heap* heap = Runtime::Current()->GetHeap(); - const std::vector<gc::space::ContinuousSpace*>& spaces = heap->GetContinuousSpaces(); + const auto& spaces = heap->GetContinuousSpaces(); DCHECK(!spaces.empty()); DCHECK_EQ(0U, image_end_); @@ -418,10 +399,7 @@ void ImageWriter::CalculateNewObjectOffsets(size_t oat_loaded_size, size_t oat_d // TODO: Add InOrderWalk to heap bitmap. const char* old = self->StartAssertNoThreadSuspension("ImageWriter"); DCHECK(heap->GetLargeObjectsSpace()->GetLiveObjects()->IsEmpty()); - // TODO: C++0x auto - typedef std::vector<gc::space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), end = spaces.end(); it != end; ++it) { - gc::space::ContinuousSpace* space = *it; + for (const auto& space : spaces) { space->GetLiveBitmap()->InOrderWalk(CalculateNewObjectOffsetsCallback, this); DCHECK_LT(image_end_, image_->Size()); } diff --git a/compiler/image_writer.h b/compiler/image_writer.h index 750109d..6a126b8 100644 --- a/compiler/image_writer.h +++ b/compiler/image_writer.h @@ -204,8 +204,7 @@ class ImageWriter { uint32_t quick_to_interpreter_bridge_offset_; // DexCaches seen while scanning for fixing up CodeAndDirectMethods - typedef std::set<mirror::DexCache*> Set; - Set dex_caches_; + std::set<mirror::DexCache*> dex_caches_; }; } // namespace art diff --git a/compiler/sea_ir/ir/regions_test.cc b/compiler/sea_ir/ir/regions_test.cc index 9813465..8ca51e4 100644 --- a/compiler/sea_ir/ir/regions_test.cc +++ b/compiler/sea_ir/ir/regions_test.cc @@ -56,4 +56,4 @@ TEST_F(RegionsTest, Basics) { EXPECT_EQ(root, preds->at(0)); } -} // namespace art +} // namespace sea_ir diff --git a/compiler/sea_ir/types/type_data_test.cc b/compiler/sea_ir/types/type_data_test.cc index a66ebce..f7a5362 100644 --- a/compiler/sea_ir/types/type_data_test.cc +++ b/compiler/sea_ir/types/type_data_test.cc @@ -38,4 +38,4 @@ TEST_F(TypeDataTest, Basics) { EXPECT_TRUE(byte_type == td.FindTypeOf(second_instruction_id)); } -} // namespace art +} // namespace sea_ir diff --git a/compiler/sea_ir/types/type_inference_visitor.cc b/compiler/sea_ir/types/type_inference_visitor.cc index 3da2fc1..81a8f4d 100644 --- a/compiler/sea_ir/types/type_inference_visitor.cc +++ b/compiler/sea_ir/types/type_inference_visitor.cc @@ -78,9 +78,9 @@ std::vector<const Type*> TypeInferenceVisitor::GetOperandTypes( const Type* TypeInferenceVisitor::MergeTypes(std::vector<const Type*>& types) const { const Type* type = NULL; - if (types.size()>0) { + if (types.size() > 0) { type = *(types.begin()); - if (types.size()>1) { + if (types.size() > 1) { for (std::vector<const Type*>::const_iterator cit = types.begin(); cit != types.end(); cit++) { if (!type->Equals(**cit)) { diff --git a/compiler/sea_ir/types/type_inference_visitor.h b/compiler/sea_ir/types/type_inference_visitor.h index 200b9f0..4bdac38 100644 --- a/compiler/sea_ir/types/type_inference_visitor.h +++ b/compiler/sea_ir/types/type_inference_visitor.h @@ -61,7 +61,7 @@ class TypeInferenceVisitor: public IRVisitor { std::vector<const Type*> GetOperandTypes(InstructionNode* instruction) const; const Type* GetType() { // TODO: Currently multiple defined types are not supported. - if (crt_type_.size()>0) { + if (!crt_type_.empty()) { const Type* single_type = crt_type_.at(0); crt_type_.clear(); return single_type; diff --git a/compiler/sea_ir/types/type_inference_visitor_test.cc b/compiler/sea_ir/types/type_inference_visitor_test.cc index 8a249eb..77acb3d 100644 --- a/compiler/sea_ir/types/type_inference_visitor_test.cc +++ b/compiler/sea_ir/types/type_inference_visitor_test.cc @@ -130,4 +130,4 @@ TEST_F(TypeInferenceVisitorTest, GetOperandTypes) { } -} // namespace art +} // namespace sea_ir diff --git a/compiler/utils/assembler.h b/compiler/utils/assembler.h index 9d79002..c9be4ed 100644 --- a/compiler/utils/assembler.h +++ b/compiler/utils/assembler.h @@ -137,6 +137,7 @@ class SlowPath { // Next in linked list of slow paths SlowPath *next_; + private: friend class AssemblerBuffer; DISALLOW_COPY_AND_ASSIGN(SlowPath); }; diff --git a/oatdump/oatdump.cc b/oatdump/oatdump.cc index fbfdfd9..8bc7877 100644 --- a/oatdump/oatdump.cc +++ b/oatdump/oatdump.cc @@ -736,13 +736,10 @@ class ImageDumper { oat_dumper_.reset(new OatDumper(host_prefix_, *oat_file)); - std::vector<const OatFile::OatDexFile*> oat_dex_files = oat_file->GetOatDexFiles(); - for (size_t i = 0; i < oat_dex_files.size(); i++) { - const OatFile::OatDexFile* oat_dex_file = oat_dex_files[i]; + for (const OatFile::OatDexFile* oat_dex_file : oat_file->GetOatDexFiles()) { CHECK(oat_dex_file != NULL); - std::pair<std::string, size_t> entry(oat_dex_file->GetDexFileLocation(), - oat_dex_file->FileSize()); - stats_.oat_dex_file_sizes.push_back(entry); + stats_.oat_dex_file_sizes.push_back(std::make_pair(oat_dex_file->GetDexFileLocation(), + oat_dex_file->FileSize())); } os << "OBJECTS:\n" << std::flush; @@ -761,10 +758,7 @@ class ImageDumper { std::ostream indent_os(&indent_filter); os_ = &indent_os; ReaderMutexLock mu(self, *Locks::heap_bitmap_lock_); - // TODO: C++0x auto - typedef std::vector<gc::space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), end = spaces.end(); it != end; ++it) { - gc::space::Space* space = *it; + for (const auto& space : spaces) { if (space->IsImageSpace()) { gc::space::ImageSpace* image_space = space->AsImageSpace(); image_space->GetLiveBitmap()->Walk(ImageDumper::Callback, this); @@ -1263,16 +1257,16 @@ class ImageDumper { os << "object_bytes breakdown:\n"; size_t object_bytes_total = 0; - typedef SizeAndCountTable::const_iterator It; // TODO: C++0x auto - for (It it = sizes_and_counts.begin(), end = sizes_and_counts.end(); it != end; ++it) { - const std::string& descriptor(it->first); - double average = static_cast<double>(it->second.bytes) / static_cast<double>(it->second.count); - double percent = PercentOfObjectBytes(it->second.bytes); + for (const auto& sizes_and_count : sizes_and_counts) { + const std::string& descriptor(sizes_and_count.first); + double average = static_cast<double>(sizes_and_count.second.bytes) / + static_cast<double>(sizes_and_count.second.count); + double percent = PercentOfObjectBytes(sizes_and_count.second.bytes); os << StringPrintf("%32s %8zd bytes %6zd instances " "(%4.0f bytes/instance) %2.0f%% of object_bytes\n", - descriptor.c_str(), it->second.bytes, it->second.count, - average, percent); - object_bytes_total += it->second.bytes; + descriptor.c_str(), sizes_and_count.second.bytes, + sizes_and_count.second.count, average, percent); + object_bytes_total += sizes_and_count.second.bytes; } os << "\n" << std::flush; CHECK_EQ(object_bytes, object_bytes_total); @@ -1292,11 +1286,10 @@ class ImageDumper { large_initializer_code_bytes, PercentOfOatBytes(large_initializer_code_bytes), large_method_code_bytes, PercentOfOatBytes(large_method_code_bytes)) << "DexFile sizes:\n"; - typedef std::vector<std::pair<std::string, size_t> >::const_iterator It2; - for (It2 it = oat_dex_file_sizes.begin(); it != oat_dex_file_sizes.end(); - ++it) { + for (const std::pair<std::string, size_t>& oat_dex_file_size : oat_dex_file_sizes) { os << StringPrintf("%s = %zd (%2.0f%% of oat file bytes)\n", - it->first.c_str(), it->second, PercentOfOatBytes(it->second)); + oat_dex_file_size.first.c_str(), oat_dex_file_size.second, + PercentOfOatBytes(oat_dex_file_size.second)); } os << "\n" << StringPrintf("gc_map_bytes = %7zd (%2.0f%% of oat file bytes)\n" diff --git a/runtime/base/histogram.h b/runtime/base/histogram.h index f508af9..2a02cf4 100644 --- a/runtime/base/histogram.h +++ b/runtime/base/histogram.h @@ -112,6 +112,6 @@ template <class Value> class Histogram { DISALLOW_COPY_AND_ASSIGN(Histogram); }; -} +} // namespace art #endif // ART_RUNTIME_BASE_HISTOGRAM_H_ diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 039e7bc..ef27321 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -1111,16 +1111,15 @@ void ClassLinker::VisitRoots(RootVisitor* visitor, void* arg, bool clean_dirty) Thread* self = Thread::Current(); { ReaderMutexLock mu(self, dex_lock_); - for (size_t i = 0; i < dex_caches_.size(); i++) { - visitor(dex_caches_[i], arg); + for (mirror::DexCache* dex_cache : dex_caches_) { + visitor(dex_cache, arg); } } { ReaderMutexLock mu(self, *Locks::classlinker_classes_lock_); - typedef Table::const_iterator It; // TODO: C++0x auto - for (It it = classes_.begin(), end = classes_.end(); it != end; ++it) { - visitor(it->second, arg); + for (const std::pair<size_t, mirror::Class*>& it : classes_) { + visitor(it.second, arg); } // We deliberately ignore the class roots in the image since we @@ -1135,14 +1134,13 @@ void ClassLinker::VisitRoots(RootVisitor* visitor, void* arg, bool clean_dirty) void ClassLinker::VisitClasses(ClassVisitor* visitor, void* arg) const { ReaderMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_); - typedef Table::const_iterator It; // TODO: C++0x auto - for (It it = classes_.begin(), end = classes_.end(); it != end; ++it) { - if (!visitor(it->second, arg)) { + for (const std::pair<size_t, mirror::Class*>& it : classes_) { + if (!visitor(it.second, arg)) { return; } } - for (It it = image_classes_.begin(), end = image_classes_.end(); it != end; ++it) { - if (!visitor(it->second, arg)) { + for (const std::pair<size_t, mirror::Class*>& it : image_classes_) { + if (!visitor(it.second, arg)) { return; } } @@ -1157,9 +1155,8 @@ static bool GetClassesVisitor(mirror::Class* c, void* arg) { void ClassLinker::VisitClassesWithoutClassesLock(ClassVisitor* visitor, void* arg) const { std::set<mirror::Class*> classes; VisitClasses(GetClassesVisitor, &classes); - typedef std::set<mirror::Class*>::const_iterator It; // TODO: C++0x auto - for (It it = classes.begin(), end = classes.end(); it != end; ++it) { - if (!visitor(*it, arg)) { + for (mirror::Class* klass : classes) { + if (!visitor(klass, arg)) { return; } } @@ -2160,10 +2157,9 @@ mirror::Class* ClassLinker::InsertClass(const StringPiece& descriptor, mirror::C bool ClassLinker::RemoveClass(const char* descriptor, const mirror::ClassLoader* class_loader) { size_t hash = Hash(descriptor); WriterMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_); - typedef Table::iterator It; // TODO: C++0x auto // TODO: determine if its better to search classes_ or image_classes_ first ClassHelper kh; - for (It it = classes_.lower_bound(hash), end = classes_.end(); it != end && it->first == hash; + for (auto it = classes_.lower_bound(hash), end = classes_.end(); it != end && it->first == hash; ++it) { mirror::Class* klass = it->second; kh.ChangeClass(klass); @@ -2172,7 +2168,7 @@ bool ClassLinker::RemoveClass(const char* descriptor, const mirror::ClassLoader* return true; } } - for (It it = image_classes_.lower_bound(hash), end = classes_.end(); + for (auto it = image_classes_.lower_bound(hash), end = classes_.end(); it != end && it->first == hash; ++it) { mirror::Class* klass = it->second; kh.ChangeClass(klass); @@ -2204,8 +2200,9 @@ mirror::Class* ClassLinker::LookupClassLocked(const char* descriptor, const mirror::ClassLoader* class_loader, size_t hash, const Table& classes) { ClassHelper kh(NULL, this); - typedef Table::const_iterator It; // TODO: C++0x auto - for (It it = classes.lower_bound(hash), end = classes_.end(); it != end && it->first == hash; ++it) { + auto end = classes_.end(); + for (auto it = classes.lower_bound(hash); it != end && it->first == hash; + ++it) { mirror::Class* klass = it->second; kh.ChangeClass(klass); if (strcmp(descriptor, kh.GetDescriptor()) == 0 && klass->GetClassLoader() == class_loader) { @@ -2228,17 +2225,18 @@ void ClassLinker::LookupClasses(const char* descriptor, std::vector<mirror::Clas classes.clear(); size_t hash = Hash(descriptor); ReaderMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_); - typedef Table::const_iterator It; // TODO: C++0x auto // TODO: determine if its better to search classes_ or image_classes_ first ClassHelper kh(NULL, this); - for (It it = classes_.lower_bound(hash), end = classes_.end(); it != end && it->first == hash; ++it) { + for (auto it = classes_.lower_bound(hash), end = classes_.end(); it != end && it->first == hash; + ++it) { mirror::Class* klass = it->second; kh.ChangeClass(klass); if (strcmp(descriptor, kh.GetDescriptor()) == 0) { classes.push_back(klass); } } - for (It it = image_classes_.lower_bound(hash), end = classes_.end(); it != end && it->first == hash; ++it) { + for (auto it = image_classes_.lower_bound(hash), end = classes_.end(); + it != end && it->first == hash; ++it) { mirror::Class* klass = it->second; kh.ChangeClass(klass); if (strcmp(descriptor, kh.GetDescriptor()) == 0) { @@ -3967,12 +3965,11 @@ void ClassLinker::DumpAllClasses(int flags) const { std::vector<mirror::Class*> all_classes; { ReaderMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_); - typedef Table::const_iterator It; // TODO: C++0x auto - for (It it = classes_.begin(), end = classes_.end(); it != end; ++it) { - all_classes.push_back(it->second); + for (const std::pair<size_t, mirror::Class*>& it : classes_) { + all_classes.push_back(it.second); } - for (It it = image_classes_.begin(), end = image_classes_.end(); it != end; ++it) { - all_classes.push_back(it->second); + for (const std::pair<size_t, mirror::Class*>& it : image_classes_) { + all_classes.push_back(it.second); } } diff --git a/runtime/debugger.cc b/runtime/debugger.cc index 569a370..a72ae22 100644 --- a/runtime/debugger.cc +++ b/runtime/debugger.cc @@ -3033,9 +3033,8 @@ void Dbg::DdmSetThreadNotification(bool enable) { } { ScopedObjectAccess soa(self); - typedef std::list<Thread*>::const_iterator It; // TODO: C++0x auto - for (It it = threads.begin(), end = threads.end(); it != end; ++it) { - Dbg::DdmSendThreadNotification(*it, CHUNK_TYPE("THCR")); + for (Thread* thread : threads) { + Dbg::DdmSendThreadNotification(thread, CHUNK_TYPE("THCR")); } } ResumeVM(); @@ -3600,8 +3599,7 @@ class StringTable { } size_t IndexOf(const char* s) const { - typedef std::set<std::string>::const_iterator It; // TODO: C++0x auto - It it = table_.find(s); + auto it = table_.find(s); if (it == table_.end()) { LOG(FATAL) << "IndexOf(\"" << s << "\") failed"; } @@ -3613,9 +3611,8 @@ class StringTable { } void WriteTo(std::vector<uint8_t>& bytes) const { - typedef std::set<std::string>::const_iterator It; // TODO: C++0x auto - for (It it = table_.begin(); it != table_.end(); ++it) { - const char* s = (*it).c_str(); + for (const std::string& str : table_) { + const char* s = str.c_str(); size_t s_len = CountModifiedUtf8Chars(s); UniquePtr<uint16_t> s_utf16(new uint16_t[s_len]); ConvertModifiedUtf8ToUtf16(s_utf16.get(), s); diff --git a/runtime/dex_file_verifier.cc b/runtime/dex_file_verifier.cc index 09e929c..5b076e0 100644 --- a/runtime/dex_file_verifier.cc +++ b/runtime/dex_file_verifier.cc @@ -1299,8 +1299,7 @@ bool DexFileVerifier::CheckIntraSection() { } bool DexFileVerifier::CheckOffsetToTypeMap(uint32_t offset, uint16_t type) { - typedef SafeMap<uint32_t, uint16_t>::iterator It; // TODO: C++0x auto - It it = offset_to_type_map_.find(offset); + auto it = offset_to_type_map_.find(offset); if (it == offset_to_type_map_.end()) { LOG(ERROR) << StringPrintf("No data map entry found @ %x; expected %x", offset, type); return false; diff --git a/runtime/gc/accounting/gc_allocator.cc b/runtime/gc/accounting/gc_allocator.cc index 0b0d3ed..11d0e67 100644 --- a/runtime/gc/accounting/gc_allocator.cc +++ b/runtime/gc/accounting/gc_allocator.cc @@ -31,6 +31,6 @@ namespace accounting { Runtime::Current()->GetHeap()->RegisterGCDeAllocation(bytes); free(p); } -} -} -} +} // namespace accounting +} // namespace gc +} // namespace art diff --git a/runtime/gc/accounting/gc_allocator.h b/runtime/gc/accounting/gc_allocator.h index d1356a7..1fba858 100644 --- a/runtime/gc/accounting/gc_allocator.h +++ b/runtime/gc/accounting/gc_allocator.h @@ -75,8 +75,8 @@ namespace accounting { GCAllocatorImpl<T>, std::allocator<T> >::value { }; -} -} -} +} // namespace accounting +} // namespace gc +} // namespace art #endif // ART_RUNTIME_GC_ACCOUNTING_GC_ALLOCATOR_H_ diff --git a/runtime/gc/accounting/heap_bitmap-inl.h b/runtime/gc/accounting/heap_bitmap-inl.h index 0524ccb..18b93d4 100644 --- a/runtime/gc/accounting/heap_bitmap-inl.h +++ b/runtime/gc/accounting/heap_bitmap-inl.h @@ -25,20 +25,12 @@ namespace accounting { template <typename Visitor> inline void HeapBitmap::Visit(const Visitor& visitor) { - // TODO: C++0x auto - typedef SpaceBitmapVector::iterator It; - for (It it = continuous_space_bitmaps_.begin(), end = continuous_space_bitmaps_.end(); - it != end; ++it) { - SpaceBitmap* bitmap = *it; + for (const auto& bitmap : continuous_space_bitmaps_) { bitmap->VisitMarkedRange(bitmap->HeapBegin(), bitmap->HeapLimit(), visitor); } - // TODO: C++0x auto - typedef SpaceSetMapVector::iterator It2; - DCHECK(discontinuous_space_sets_.begin() != discontinuous_space_sets_.end()); - for (It2 it = discontinuous_space_sets_.begin(), end = discontinuous_space_sets_.end(); - it != end; ++it) { - SpaceSetMap* set = *it; - set->Visit(visitor); + DCHECK(!discontinuous_space_sets_.empty()); + for (const auto& space_set : discontinuous_space_sets_) { + space_set->Visit(visitor); } } diff --git a/runtime/gc/accounting/heap_bitmap.cc b/runtime/gc/accounting/heap_bitmap.cc index 0462905..5589461 100644 --- a/runtime/gc/accounting/heap_bitmap.cc +++ b/runtime/gc/accounting/heap_bitmap.cc @@ -23,12 +23,9 @@ namespace gc { namespace accounting { void HeapBitmap::ReplaceBitmap(SpaceBitmap* old_bitmap, SpaceBitmap* new_bitmap) { - // TODO: C++0x auto - typedef SpaceBitmapVector::iterator It; - for (It it = continuous_space_bitmaps_.begin(), end = continuous_space_bitmaps_.end(); - it != end; ++it) { - if (*it == old_bitmap) { - *it = new_bitmap; + for (auto& bitmap : continuous_space_bitmaps_) { + if (bitmap == old_bitmap) { + bitmap = new_bitmap; return; } } @@ -36,12 +33,9 @@ void HeapBitmap::ReplaceBitmap(SpaceBitmap* old_bitmap, SpaceBitmap* new_bitmap) } void HeapBitmap::ReplaceObjectSet(SpaceSetMap* old_set, SpaceSetMap* new_set) { - // TODO: C++0x auto - typedef SpaceSetMapVector::iterator It; - for (It it = discontinuous_space_sets_.begin(), end = discontinuous_space_sets_.end(); - it != end; ++it) { - if (*it == old_set) { - *it = new_set; + for (auto& space_set : discontinuous_space_sets_) { + if (space_set == old_set) { + space_set = new_set; return; } } @@ -52,13 +46,10 @@ void HeapBitmap::AddContinuousSpaceBitmap(accounting::SpaceBitmap* bitmap) { DCHECK(bitmap != NULL); // Check for interval overlap. - typedef SpaceBitmapVector::iterator It; - for (It it = continuous_space_bitmaps_.begin(), end = continuous_space_bitmaps_.end(); - it != end; ++it) { - SpaceBitmap* bitmap = *it; - SpaceBitmap* cur_bitmap = *it; - CHECK(bitmap->HeapBegin() < cur_bitmap->HeapLimit() && - bitmap->HeapLimit() > cur_bitmap->HeapBegin()) + for (const auto& cur_bitmap : continuous_space_bitmaps_) { + CHECK(!( + bitmap->HeapBegin() < cur_bitmap->HeapLimit() && + bitmap->HeapLimit() > cur_bitmap->HeapBegin())) << "Bitmap " << bitmap->Dump() << " overlaps with existing bitmap " << cur_bitmap->Dump(); } continuous_space_bitmaps_.push_back(bitmap); @@ -70,20 +61,13 @@ void HeapBitmap::AddDiscontinuousObjectSet(SpaceSetMap* set) { } void HeapBitmap::Walk(SpaceBitmap::Callback* callback, void* arg) { - // TODO: C++0x auto - typedef SpaceBitmapVector::iterator It; - for (It it = continuous_space_bitmaps_.begin(), end = continuous_space_bitmaps_.end(); - it != end; ++it) { - SpaceBitmap* bitmap = *it; + for (const auto& bitmap : continuous_space_bitmaps_) { bitmap->Walk(callback, arg); } - // TODO: C++0x auto - typedef SpaceSetMapVector::iterator It2; - DCHECK(discontinuous_space_sets_.begin() != discontinuous_space_sets_.end()); - for (It2 it = discontinuous_space_sets_.begin(), end = discontinuous_space_sets_.end(); - it != end; ++it) { - SpaceSetMap* set = *it; - set->Walk(callback, arg); + + DCHECK(!discontinuous_space_sets_.empty()); + for (const auto& space_set : discontinuous_space_sets_) { + space_set->Walk(callback, arg); } } diff --git a/runtime/gc/accounting/heap_bitmap.h b/runtime/gc/accounting/heap_bitmap.h index ada976f..2ca8c4a 100644 --- a/runtime/gc/accounting/heap_bitmap.h +++ b/runtime/gc/accounting/heap_bitmap.h @@ -66,11 +66,7 @@ class HeapBitmap { } SpaceBitmap* GetContinuousSpaceBitmap(const mirror::Object* obj) { - // TODO: C++0x auto - typedef SpaceBitmapVector::iterator It; - for (It it = continuous_space_bitmaps_.begin(), end = continuous_space_bitmaps_.end(); - it != end; ++it) { - SpaceBitmap* bitmap = *it; + for (const auto& bitmap : continuous_space_bitmaps_) { if (bitmap->HasAddress(obj)) { return bitmap; } @@ -79,13 +75,9 @@ class HeapBitmap { } SpaceSetMap* GetDiscontinuousSpaceObjectSet(const mirror::Object* obj) { - // TODO: C++0x auto - typedef SpaceSetMapVector::iterator It; - for (It it = discontinuous_space_sets_.begin(), end = discontinuous_space_sets_.end(); - it != end; ++it) { - SpaceSetMap* set = *it; - if (set->Test(obj)) { - return set; + for (const auto& space_set : discontinuous_space_sets_) { + if (space_set->Test(obj)) { + return space_set; } } return NULL; diff --git a/runtime/gc/accounting/mod_union_table.cc b/runtime/gc/accounting/mod_union_table.cc index 3bbc381..4865219 100644 --- a/runtime/gc/accounting/mod_union_table.cc +++ b/runtime/gc/accounting/mod_union_table.cc @@ -36,54 +36,6 @@ namespace art { namespace gc { namespace accounting { -class MarkIfReachesAllocspaceVisitor { - public: - explicit MarkIfReachesAllocspaceVisitor(Heap* const heap, accounting::SpaceBitmap* bitmap) - : heap_(heap), - bitmap_(bitmap) { - } - - // Extra parameters are required since we use this same visitor signature for checking objects. - void operator()(const Object* obj, const Object* ref, const MemberOffset& /* offset */, - bool /* is_static */) const { - // TODO: Optimize? - // TODO: C++0x auto - const std::vector<space::ContinuousSpace*>& spaces = heap_->GetContinuousSpaces(); - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It cur = spaces.begin(); cur != spaces.end(); ++cur) { - if ((*cur)->IsDlMallocSpace() && (*cur)->Contains(ref)) { - bitmap_->Set(obj); - break; - } - } - } - - private: - Heap* const heap_; - accounting::SpaceBitmap* const bitmap_; -}; - -class ModUnionVisitor { - public: - explicit ModUnionVisitor(Heap* const heap, accounting::SpaceBitmap* bitmap) - : heap_(heap), - bitmap_(bitmap) { - } - - void operator()(const Object* obj) const - SHARED_LOCKS_REQUIRED(Locks::heap_bitmap_lock_, - Locks::mutator_lock_) { - DCHECK(obj != NULL); - // We don't have an early exit since we use the visitor pattern, an early exit should - // significantly speed this up. - MarkIfReachesAllocspaceVisitor visitor(heap_, bitmap_); - collector::MarkSweep::VisitObjectReferences(obj, visitor); - } - private: - Heap* const heap_; - accounting::SpaceBitmap* const bitmap_; -}; - class ModUnionClearCardSetVisitor { public: explicit ModUnionClearCardSetVisitor(ModUnionTable::CardSet* const cleared_cards) @@ -237,29 +189,23 @@ class ModUnionCheckReferences { void ModUnionTableReferenceCache::Verify() { // Start by checking that everything in the mod union table is marked. Heap* heap = GetHeap(); - typedef SafeMap<const byte*, std::vector<const Object*> >::const_iterator It; - typedef std::vector<const Object*>::const_iterator It2; - for (It it = references_.begin(), end = references_.end(); it != end; ++it) { - for (It2 it_ref = it->second.begin(), end_ref = it->second.end(); it_ref != end_ref; - ++it_ref ) { - CHECK(heap->IsLiveObjectLocked(*it_ref)); + for (const std::pair<const byte*, std::vector<const Object*> >& it : references_) { + for (const Object* ref : it.second) { + CHECK(heap->IsLiveObjectLocked(ref)); } } // Check the references of each clean card which is also in the mod union table. CardTable* card_table = heap->GetCardTable(); - for (It it = references_.begin(); it != references_.end(); ++it) { - const byte* card = &*it->first; + for (const std::pair<const byte*, std::vector<const Object*> > & it : references_) { + const byte* card = it.first; if (*card == CardTable::kCardClean) { - std::set<const Object*> reference_set; - for (It2 itr = it->second.begin(); itr != it->second.end(); ++itr) { - reference_set.insert(*itr); - } + std::set<const Object*> reference_set(it.second.begin(), it.second.end()); ModUnionCheckReferences visitor(this, reference_set); uintptr_t start = reinterpret_cast<uintptr_t>(card_table->AddrFromCard(card)); uintptr_t end = start + CardTable::kCardSize; - space::ContinuousSpace* space = - heap->FindContinuousSpaceFromObject(reinterpret_cast<Object*>(start), false); + auto* space = heap->FindContinuousSpaceFromObject(reinterpret_cast<Object*>(start), false); + DCHECK(space != nullptr); SpaceBitmap* live_bitmap = space->GetLiveBitmap(); live_bitmap->VisitMarkedRange(start, end, visitor); } @@ -268,24 +214,20 @@ void ModUnionTableReferenceCache::Verify() { void ModUnionTableReferenceCache::Dump(std::ostream& os) { CardTable* card_table = heap_->GetCardTable(); - typedef std::set<byte*>::const_iterator It; os << "ModUnionTable cleared cards: ["; - for (It it = cleared_cards_.begin(); it != cleared_cards_.end(); ++it) { - byte* card = *it; - uintptr_t start = reinterpret_cast<uintptr_t>(card_table->AddrFromCard(card)); + for (byte* card_addr : cleared_cards_) { + uintptr_t start = reinterpret_cast<uintptr_t>(card_table->AddrFromCard(card_addr)); uintptr_t end = start + CardTable::kCardSize; os << reinterpret_cast<void*>(start) << "-" << reinterpret_cast<void*>(end) << ","; } os << "]\nModUnionTable references: ["; - typedef SafeMap<const byte*, std::vector<const Object*> >::const_iterator It2; - for (It2 it = references_.begin(); it != references_.end(); ++it) { - const byte* card = &*it->first; - uintptr_t start = reinterpret_cast<uintptr_t>(card_table->AddrFromCard(card)); + for (const std::pair<const byte*, std::vector<const Object*> >& it : references_) { + const byte* card_addr = it.first; + uintptr_t start = reinterpret_cast<uintptr_t>(card_table->AddrFromCard(card_addr)); uintptr_t end = start + CardTable::kCardSize; os << reinterpret_cast<void*>(start) << "-" << reinterpret_cast<void*>(end) << "->{"; - typedef std::vector<const Object*>::const_iterator It3; - for (It3 itr = it->second.begin(); itr != it->second.end(); ++itr) { - os << reinterpret_cast<const void*>(*itr) << ","; + for (const mirror::Object* ref : it.second) { + os << reinterpret_cast<const void*>(ref) << ","; } os << "},"; } @@ -298,20 +240,18 @@ void ModUnionTableReferenceCache::Update() { std::vector<const Object*> cards_references; ModUnionReferenceVisitor visitor(this, &cards_references); - typedef std::set<byte*>::iterator It; - for (It it = cleared_cards_.begin(), cc_end = cleared_cards_.end(); it != cc_end; ++it) { - byte* card = *it; + for (const auto& card : cleared_cards_) { // Clear and re-compute alloc space references associated with this card. cards_references.clear(); uintptr_t start = reinterpret_cast<uintptr_t>(card_table->AddrFromCard(card)); uintptr_t end = start + CardTable::kCardSize; - SpaceBitmap* live_bitmap = - heap->FindContinuousSpaceFromObject(reinterpret_cast<Object*>(start), false)->GetLiveBitmap(); + auto* space = heap->FindContinuousSpaceFromObject(reinterpret_cast<Object*>(start), false); + DCHECK(space != nullptr); + SpaceBitmap* live_bitmap = space->GetLiveBitmap(); live_bitmap->VisitMarkedRange(start, end, visitor); // Update the corresponding references for the card. - // TODO: C++0x auto - SafeMap<const byte*, std::vector<const Object*> >::iterator found = references_.find(card); + auto found = references_.find(card); if (found == references_.end()) { if (cards_references.empty()) { // No reason to add empty array. @@ -326,14 +266,11 @@ void ModUnionTableReferenceCache::Update() { } void ModUnionTableReferenceCache::MarkReferences(collector::MarkSweep* mark_sweep) { - // TODO: C++0x auto size_t count = 0; - typedef SafeMap<const byte*, std::vector<const Object*> >::const_iterator It; - for (It it = references_.begin(); it != references_.end(); ++it) { - typedef std::vector<const Object*>::const_iterator It2; - for (It2 it_ref = it->second.begin(); it_ref != it->second.end(); ++it_ref) { - mark_sweep->MarkRoot(*it_ref); + for (const auto& ref : references_) { + for (const auto& obj : ref.second) { + mark_sweep->MarkRoot(obj); ++count; } } @@ -353,38 +290,28 @@ void ModUnionTableCardCache::ClearCards(space::ContinuousSpace* space) { void ModUnionTableCardCache::MarkReferences(collector::MarkSweep* mark_sweep) { CardTable* card_table = heap_->GetCardTable(); ModUnionScanImageRootVisitor visitor(mark_sweep); - typedef std::set<byte*>::const_iterator It; - It it = cleared_cards_.begin(); - It cc_end = cleared_cards_.end(); - if (it != cc_end) { - byte* card = *it; - uintptr_t start = reinterpret_cast<uintptr_t>(card_table->AddrFromCard(card)); - uintptr_t end = start + CardTable::kCardSize; - space::ContinuousSpace* cur_space = - heap_->FindContinuousSpaceFromObject(reinterpret_cast<Object*>(start), false); - accounting::SpaceBitmap* cur_live_bitmap = cur_space->GetLiveBitmap(); - cur_live_bitmap->VisitMarkedRange(start, end, visitor); - for (++it; it != cc_end; ++it) { - card = *it; - start = reinterpret_cast<uintptr_t>(card_table->AddrFromCard(card)); - end = start + CardTable::kCardSize; - if (UNLIKELY(!cur_space->Contains(reinterpret_cast<Object*>(start)))) { - cur_space = heap_->FindContinuousSpaceFromObject(reinterpret_cast<Object*>(start), false); - cur_live_bitmap = cur_space->GetLiveBitmap(); - } - cur_live_bitmap->VisitMarkedRange(start, end, visitor); + space::ContinuousSpace* space = nullptr; + SpaceBitmap* bitmap = nullptr; + for (const byte* card_addr : cleared_cards_) { + auto start = reinterpret_cast<uintptr_t>(card_table->AddrFromCard(card_addr)); + auto end = start + CardTable::kCardSize; + auto obj_start = reinterpret_cast<Object*>(start); + if (UNLIKELY(space == nullptr || !space->Contains(obj_start))) { + space = heap_->FindContinuousSpaceFromObject(obj_start, false); + DCHECK(space != nullptr); + bitmap = space->GetLiveBitmap(); + DCHECK(bitmap != nullptr); } + bitmap->VisitMarkedRange(start, end, visitor); } } void ModUnionTableCardCache::Dump(std::ostream& os) { CardTable* card_table = heap_->GetCardTable(); - typedef std::set<byte*>::const_iterator It; os << "ModUnionTable dirty cards: ["; - for (It it = cleared_cards_.begin(); it != cleared_cards_.end(); ++it) { - byte* card = *it; - uintptr_t start = reinterpret_cast<uintptr_t>(card_table->AddrFromCard(card)); - uintptr_t end = start + CardTable::kCardSize; + for (const byte* card_addr : cleared_cards_) { + auto start = reinterpret_cast<uintptr_t>(card_table->AddrFromCard(card_addr)); + auto end = start + CardTable::kCardSize; os << reinterpret_cast<void*>(start) << "-" << reinterpret_cast<void*>(end) << ","; } os << "]"; diff --git a/runtime/gc/collector/garbage_collector.cc b/runtime/gc/collector/garbage_collector.cc index 378a971..9260137 100644 --- a/runtime/gc/collector/garbage_collector.cc +++ b/runtime/gc/collector/garbage_collector.cc @@ -114,11 +114,7 @@ void GarbageCollector::SwapBitmaps() { // these bitmaps. The bitmap swapping is an optimization so that we do not need to clear the live // bits of dead objects in the live bitmap. const GcType gc_type = GetGcType(); - const std::vector<space::ContinuousSpace*>& cont_spaces = GetHeap()->GetContinuousSpaces(); - // TODO: C++0x - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = cont_spaces.begin(), end = cont_spaces.end(); it != end; ++it) { - space::ContinuousSpace* space = *it; + for (const auto& space : GetHeap()->GetContinuousSpaces()) { // We never allocate into zygote spaces. if (space->GetGcRetentionPolicy() == space::kGcRetentionPolicyAlwaysCollect || (gc_type == kGcTypeFull && @@ -132,11 +128,8 @@ void GarbageCollector::SwapBitmaps() { } } } - const std::vector<space::DiscontinuousSpace*>& disc_spaces = GetHeap()->GetDiscontinuousSpaces(); - // TODO: C++0x - typedef std::vector<space::DiscontinuousSpace*>::const_iterator It2; - for (It2 it = disc_spaces.begin(), end = disc_spaces.end(); it != end; ++it) { - space::LargeObjectSpace* space = down_cast<space::LargeObjectSpace*>(*it); + for (const auto& disc_space : GetHeap()->GetDiscontinuousSpaces()) { + space::LargeObjectSpace* space = down_cast<space::LargeObjectSpace*>(disc_space); accounting::SpaceSetMap* live_set = space->GetLiveObjects(); accounting::SpaceSetMap* mark_set = space->GetMarkObjects(); heap_->GetLiveBitmap()->ReplaceObjectSet(live_set, mark_set); diff --git a/runtime/gc/collector/mark_sweep.cc b/runtime/gc/collector/mark_sweep.cc index 61570ae..e93bcd1 100644 --- a/runtime/gc/collector/mark_sweep.cc +++ b/runtime/gc/collector/mark_sweep.cc @@ -84,16 +84,13 @@ void MarkSweep::ImmuneSpace(space::ContinuousSpace* space) { SetImmuneRange(reinterpret_cast<Object*>(space->Begin()), reinterpret_cast<Object*>(space->End())); } else { - const std::vector<space::ContinuousSpace*>& spaces = GetHeap()->GetContinuousSpaces(); - const space::ContinuousSpace* prev_space = NULL; + const space::ContinuousSpace* prev_space = nullptr; // Find out if the previous space is immune. - // TODO: C++0x - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), end = spaces.end(); it != end; ++it) { - if (*it == space) { + for (space::ContinuousSpace* cur_space : GetHeap()->GetContinuousSpaces()) { + if (cur_space == space) { break; } - prev_space = *it; + prev_space = cur_space; } // If previous space was immune, then extend the immune region. Relies on continuous spaces @@ -322,13 +319,9 @@ void MarkSweep::SetImmuneRange(Object* begin, Object* end) { void MarkSweep::FindDefaultMarkBitmap() { base::TimingLogger::ScopedSplit split("FindDefaultMarkBitmap", &timings_); - const std::vector<space::ContinuousSpace*>& spaces = GetHeap()->GetContinuousSpaces(); - // TODO: C++0x - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), end = spaces.end(); it != end; ++it) { - space::ContinuousSpace* space = *it; + for (const auto& space : GetHeap()->GetContinuousSpaces()) { if (space->GetGcRetentionPolicy() == space::kGcRetentionPolicyAlwaysCollect) { - current_mark_bitmap_ = (*it)->GetMarkBitmap(); + current_mark_bitmap_ = space->GetMarkBitmap(); CHECK(current_mark_bitmap_ != NULL); return; } @@ -344,11 +337,10 @@ void MarkSweep::ExpandMarkStack() { // Someone else acquired the lock and expanded the mark stack before us. return; } - std::vector<Object*> temp; - temp.insert(temp.begin(), mark_stack_->Begin(), mark_stack_->End()); + std::vector<Object*> temp(mark_stack_->Begin(), mark_stack_->End()); mark_stack_->Resize(mark_stack_->Capacity() * 2); - for (size_t i = 0; i < temp.size(); ++i) { - mark_stack_->PushBack(temp[i]); + for (const auto& obj : temp) { + mark_stack_->PushBack(obj); } } @@ -608,12 +600,8 @@ class ScanObjectVisitor { void MarkSweep::ScanGrayObjects(byte minimum_age) { accounting::CardTable* card_table = GetHeap()->GetCardTable(); - const std::vector<space::ContinuousSpace*>& spaces = GetHeap()->GetContinuousSpaces(); ScanObjectVisitor visitor(this); - // TODO: C++0x - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), space_end = spaces.end(); it != space_end; ++it) { - space::ContinuousSpace* space = *it; + for (const auto& space : GetHeap()->GetContinuousSpaces()) { switch (space->GetGcRetentionPolicy()) { case space::kGcRetentionPolicyNeverCollect: timings_.StartSplit("ScanGrayImageSpaceObjects"); @@ -656,15 +644,12 @@ void MarkSweep::VerifyImageRoots() { // space timings_.StartSplit("VerifyImageRoots"); CheckBitmapVisitor visitor(this); - const std::vector<space::ContinuousSpace*>& spaces = GetHeap()->GetContinuousSpaces(); - // TODO: C++0x - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), end = spaces.end(); it != end; ++it) { - if ((*it)->IsImageSpace()) { - space::ImageSpace* space = (*it)->AsImageSpace(); - uintptr_t begin = reinterpret_cast<uintptr_t>(space->Begin()); - uintptr_t end = reinterpret_cast<uintptr_t>(space->End()); - accounting::SpaceBitmap* live_bitmap = space->GetLiveBitmap(); + for (const auto& space : GetHeap()->GetContinuousSpaces()) { + if (space->IsImageSpace()) { + space::ImageSpace* image_space = space->AsImageSpace(); + uintptr_t begin = reinterpret_cast<uintptr_t>(image_space->Begin()); + uintptr_t end = reinterpret_cast<uintptr_t>(image_space->End()); + accounting::SpaceBitmap* live_bitmap = image_space->GetLiveBitmap(); DCHECK(live_bitmap != NULL); live_bitmap->VisitMarkedRange(begin, end, visitor); } @@ -687,11 +672,7 @@ void MarkSweep::RecursiveMark() { const bool partial = GetGcType() == kGcTypePartial; ScanObjectVisitor scan_visitor(this); if (!kDisableFinger) { - const std::vector<space::ContinuousSpace*>& spaces = GetHeap()->GetContinuousSpaces(); - // TODO: C++0x - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), end = spaces.end(); it != end; ++it) { - space::ContinuousSpace* space = *it; + for (const auto& space : GetHeap()->GetContinuousSpaces()) { if ((space->GetGcRetentionPolicy() == space::kGcRetentionPolicyAlwaysCollect) || (!partial && space->GetGcRetentionPolicy() == space::kGcRetentionPolicyFullCollect)) { current_mark_bitmap_ = space->GetMarkBitmap(); @@ -729,10 +710,7 @@ void MarkSweep::ReMarkRoots() { void MarkSweep::SweepJniWeakGlobals(IsMarkedTester is_marked, void* arg) { JavaVMExt* vm = Runtime::Current()->GetJavaVM(); MutexLock mu(Thread::Current(), vm->weak_globals_lock); - IndirectReferenceTable* table = &vm->weak_globals; - typedef IndirectReferenceTable::iterator It; // TODO: C++0x auto - for (It it = table->begin(), end = table->end(); it != end; ++it) { - const Object** entry = *it; + for (const Object** entry : vm->weak_globals) { if (!is_marked(*entry, arg)) { *entry = kClearedJniWeakGlobal; } @@ -815,10 +793,7 @@ void MarkSweep::VerifySystemWeaks() { JavaVMExt* vm = runtime->GetJavaVM(); MutexLock mu(Thread::Current(), vm->weak_globals_lock); - IndirectReferenceTable* table = &vm->weak_globals; - typedef IndirectReferenceTable::iterator It; // TODO: C++0x auto - for (It it = table->begin(), end = table->end(); it != end; ++it) { - const Object** entry = *it; + for (const Object** entry : vm->weak_globals) { VerifyIsLive(*entry); } } @@ -988,11 +963,7 @@ void MarkSweep::Sweep(bool swap_bitmaps) { SweepCallbackContext scc; scc.mark_sweep = this; scc.self = Thread::Current(); - const std::vector<space::ContinuousSpace*>& spaces = GetHeap()->GetContinuousSpaces(); - // TODO: C++0x - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), end = spaces.end(); it != end; ++it) { - space::ContinuousSpace* space = *it; + for (const auto& space : GetHeap()->GetContinuousSpaces()) { // We always sweep always collect spaces. bool sweep_space = (space->GetGcRetentionPolicy() == space::kGcRetentionPolicyAlwaysCollect); if (!partial && !sweep_space) { @@ -1040,11 +1011,9 @@ void MarkSweep::SweepLargeObjects(bool swap_bitmaps) { size_t freed_objects = 0; size_t freed_bytes = 0; Thread* self = Thread::Current(); - // TODO: C++0x - typedef accounting::SpaceSetMap::Objects::iterator It; - for (It it = live_objects.begin(), end = live_objects.end(); it != end; ++it) { - if (!large_mark_objects->Test(*it)) { - freed_bytes += large_object_space->Free(self, const_cast<Object*>(*it)); + for (const Object* obj : live_objects) { + if (!large_mark_objects->Test(obj)) { + freed_bytes += large_object_space->Free(self, const_cast<Object*>(obj)); ++freed_objects; } } @@ -1054,11 +1023,7 @@ void MarkSweep::SweepLargeObjects(bool swap_bitmaps) { } void MarkSweep::CheckReference(const Object* obj, const Object* ref, MemberOffset offset, bool is_static) { - const std::vector<space::ContinuousSpace*>& spaces = GetHeap()->GetContinuousSpaces(); - // TODO: C++0x - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), end = spaces.end(); it != end; ++it) { - space::ContinuousSpace* space = *it; + for (const auto& space : GetHeap()->GetContinuousSpaces()) { if (space->IsDlMallocSpace() && space->Contains(ref)) { DCHECK(IsMarked(obj)); @@ -1508,11 +1473,7 @@ void MarkSweep::ProcessReferences(Object** soft_references, bool clear_soft, void MarkSweep::UnBindBitmaps() { base::TimingLogger::ScopedSplit split("UnBindBitmaps", &timings_); - const std::vector<space::ContinuousSpace*>& spaces = GetHeap()->GetContinuousSpaces(); - // TODO: C++0x - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), end = spaces.end(); it != end; ++it) { - space::ContinuousSpace* space = *it; + for (const auto& space : GetHeap()->GetContinuousSpaces()) { if (space->IsDlMallocSpace()) { space::DlMallocSpace* alloc_space = space->AsDlMallocSpace(); if (alloc_space->temp_bitmap_.get() != NULL) { @@ -1585,11 +1546,7 @@ void MarkSweep::FinishPhase() { cumulative_timings_.End(); // Clear all of the spaces' mark bitmaps. - const std::vector<space::ContinuousSpace*>& spaces = GetHeap()->GetContinuousSpaces(); - // TODO: C++0x - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), end = spaces.end(); it != end; ++it) { - space::ContinuousSpace* space = *it; + for (const auto& space : GetHeap()->GetContinuousSpaces()) { if (space->GetGcRetentionPolicy() != space::kGcRetentionPolicyNeverCollect) { space->GetMarkBitmap()->Clear(); } diff --git a/runtime/gc/collector/mark_sweep.h b/runtime/gc/collector/mark_sweep.h index e39e2f7..8db03d3 100644 --- a/runtime/gc/collector/mark_sweep.h +++ b/runtime/gc/collector/mark_sweep.h @@ -428,6 +428,7 @@ class MarkSweep : public GarbageCollector { bool clear_soft_references_; + private: friend class AddIfReachesAllocSpaceVisitor; // Used by mod-union table. friend class CheckBitmapVisitor; friend class CheckObjectVisitor; diff --git a/runtime/gc/collector/partial_mark_sweep.cc b/runtime/gc/collector/partial_mark_sweep.cc index ef893c5..cc3cfe5 100644 --- a/runtime/gc/collector/partial_mark_sweep.cc +++ b/runtime/gc/collector/partial_mark_sweep.cc @@ -33,14 +33,10 @@ PartialMarkSweep::PartialMarkSweep(Heap* heap, bool is_concurrent, const std::st void PartialMarkSweep::BindBitmaps() { MarkSweep::BindBitmaps(); - const std::vector<space::ContinuousSpace*>& spaces = GetHeap()->GetContinuousSpaces(); WriterMutexLock mu(Thread::Current(), *Locks::heap_bitmap_lock_); // For partial GCs we need to bind the bitmap of the zygote space so that all objects in the // zygote space are viewed as marked. - // TODO: C++0x - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), end = spaces.end(); it != end; ++it) { - space::ContinuousSpace* space = *it; + for (const auto& space : GetHeap()->GetContinuousSpaces()) { if (space->GetGcRetentionPolicy() == space::kGcRetentionPolicyFullCollect) { CHECK(space->IsZygoteSpace()); ImmuneSpace(space); diff --git a/runtime/gc/collector/partial_mark_sweep.h b/runtime/gc/collector/partial_mark_sweep.h index 25304b9..3b788f4 100644 --- a/runtime/gc/collector/partial_mark_sweep.h +++ b/runtime/gc/collector/partial_mark_sweep.h @@ -38,6 +38,7 @@ class PartialMarkSweep : public MarkSweep { // collections, ie the Zygote space. Also mark this space is immune. virtual void BindBitmaps() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + private: DISALLOW_COPY_AND_ASSIGN(PartialMarkSweep); }; diff --git a/runtime/gc/collector/sticky_mark_sweep.cc b/runtime/gc/collector/sticky_mark_sweep.cc index aad7c29..008d3e0 100644 --- a/runtime/gc/collector/sticky_mark_sweep.cc +++ b/runtime/gc/collector/sticky_mark_sweep.cc @@ -33,15 +33,11 @@ StickyMarkSweep::StickyMarkSweep(Heap* heap, bool is_concurrent, const std::stri void StickyMarkSweep::BindBitmaps() { PartialMarkSweep::BindBitmaps(); - const std::vector<space::ContinuousSpace*>& spaces = GetHeap()->GetContinuousSpaces(); WriterMutexLock mu(Thread::Current(), *Locks::heap_bitmap_lock_); // For sticky GC, we want to bind the bitmaps of all spaces as the allocation stack lets us // know what was allocated since the last GC. A side-effect of binding the allocation space mark // and live bitmap is that marking the objects will place them in the live bitmap. - // TODO: C++0x - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), end = spaces.end(); it != end; ++it) { - space::ContinuousSpace* space = *it; + for (const auto& space : GetHeap()->GetContinuousSpaces()) { if (space->GetGcRetentionPolicy() == space::kGcRetentionPolicyAlwaysCollect) { BindLiveToMarkBitmap(space); } diff --git a/runtime/gc/collector/sticky_mark_sweep.h b/runtime/gc/collector/sticky_mark_sweep.h index e009b62..2099c79 100644 --- a/runtime/gc/collector/sticky_mark_sweep.h +++ b/runtime/gc/collector/sticky_mark_sweep.h @@ -45,6 +45,7 @@ class StickyMarkSweep : public PartialMarkSweep { void Sweep(bool swap_bitmaps) EXCLUSIVE_LOCKS_REQUIRED(Locks::heap_bitmap_lock_); + private: DISALLOW_COPY_AND_ASSIGN(StickyMarkSweep); }; diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc index a2453b8..d5a8d75 100644 --- a/runtime/gc/heap.cc +++ b/runtime/gc/heap.cc @@ -183,11 +183,8 @@ Heap::Heap(size_t initial_size, size_t growth_limit, size_t min_free, size_t max heap_capacity += continuous_spaces_.back()->AsDlMallocSpace()->NonGrowthLimitCapacity(); } - // Mark image objects in the live bitmap - // TODO: C++0x - typedef std::vector<space::ContinuousSpace*>::iterator It; - for (It it = continuous_spaces_.begin(); it != continuous_spaces_.end(); ++it) { - space::ContinuousSpace* space = *it; + // Mark image objects in the live bitmap. + for (const auto& space : continuous_spaces_) { if (space->IsImageSpace()) { space::ImageSpace* image_space = space->AsImageSpace(); image_space->RecordImageAllocations(image_space->GetLiveBitmap()); @@ -393,9 +390,7 @@ void Heap::AddContinuousSpace(space::ContinuousSpace* space) { // Ensure that ImageSpaces < ZygoteSpaces < AllocSpaces so that we can do address based checks to // avoid redundant marking. bool seen_zygote = false, seen_alloc = false; - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = continuous_spaces_.begin(); it != continuous_spaces_.end(); ++it) { - space::ContinuousSpace* space = *it; + for (const auto& space : continuous_spaces_) { if (space->IsImageSpace()) { DCHECK(!seen_zygote); DCHECK(!seen_alloc); @@ -436,17 +431,13 @@ void Heap::DumpGcPerformanceInfo(std::ostream& os) { uint64_t total_duration = 0; // Dump cumulative loggers for each GC type. - // TODO: C++0x uint64_t total_paused_time = 0; - typedef std::vector<collector::MarkSweep*>::const_iterator It; - for (It it = mark_sweep_collectors_.begin(); - it != mark_sweep_collectors_.end(); ++it) { - collector::MarkSweep* collector = *it; + for (const auto& collector : mark_sweep_collectors_) { CumulativeLogger& logger = collector->GetCumulativeTimings(); if (logger.GetTotalNs() != 0) { os << Dumpable<CumulativeLogger>(logger); const uint64_t total_ns = logger.GetTotalNs(); - const uint64_t total_pause_ns = (*it)->GetTotalPausedTimeNs(); + const uint64_t total_pause_ns = collector->GetTotalPausedTimeNs(); double seconds = NsToMs(logger.GetTotalNs()) / 1000.0; const uint64_t freed_bytes = collector->GetTotalFreedBytes(); const uint64_t freed_objects = collector->GetTotalFreedObjects(); @@ -507,11 +498,9 @@ Heap::~Heap() { space::ContinuousSpace* Heap::FindContinuousSpaceFromObject(const mirror::Object* obj, bool fail_ok) const { - // TODO: C++0x auto - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = continuous_spaces_.begin(), end = continuous_spaces_.end(); it != end; ++it) { - if ((*it)->Contains(obj)) { - return *it; + for (const auto& space : continuous_spaces_) { + if (space->Contains(obj)) { + return space; } } if (!fail_ok) { @@ -522,11 +511,9 @@ space::ContinuousSpace* Heap::FindContinuousSpaceFromObject(const mirror::Object space::DiscontinuousSpace* Heap::FindDiscontinuousSpaceFromObject(const mirror::Object* obj, bool fail_ok) const { - // TODO: C++0x auto - typedef std::vector<space::DiscontinuousSpace*>::const_iterator It; - for (It it = discontinuous_spaces_.begin(), end = discontinuous_spaces_.end(); it != end; ++it) { - if ((*it)->Contains(obj)) { - return *it; + for (const auto& space : discontinuous_spaces_) { + if (space->Contains(obj)) { + return space; } } if (!fail_ok) { @@ -544,11 +531,9 @@ space::Space* Heap::FindSpaceFromObject(const mirror::Object* obj, bool fail_ok) } space::ImageSpace* Heap::GetImageSpace() const { - // TODO: C++0x auto - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = continuous_spaces_.begin(), end = continuous_spaces_.end(); it != end; ++it) { - if ((*it)->IsImageSpace()) { - return (*it)->AsImageSpace(); + for (const auto& space : continuous_spaces_) { + if (space->IsImageSpace()) { + return space->AsImageSpace(); } } return NULL; @@ -627,10 +612,7 @@ mirror::Object* Heap::AllocObject(Thread* self, mirror::Class* c, size_t byte_co // If the allocation failed due to fragmentation, print out the largest continuous allocation. if (!large_object_allocation && total_bytes_free >= byte_count) { size_t max_contiguous_allocation = 0; - // TODO: C++0x auto - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = continuous_spaces_.begin(), end = continuous_spaces_.end(); it != end; ++it) { - space::ContinuousSpace* space = *it; + for (const auto& space : continuous_spaces_) { if (space->IsDlMallocSpace()) { space->AsDlMallocSpace()->Walk(MSpaceChunkCallback, &max_contiguous_allocation); } @@ -706,19 +688,14 @@ void Heap::VerifyObjectImpl(const mirror::Object* obj) { } void Heap::DumpSpaces() { - // TODO: C++0x auto - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = continuous_spaces_.begin(), end = continuous_spaces_.end(); it != end; ++it) { - space::ContinuousSpace* space = *it; + for (const auto& space : continuous_spaces_) { accounting::SpaceBitmap* live_bitmap = space->GetLiveBitmap(); accounting::SpaceBitmap* mark_bitmap = space->GetMarkBitmap(); LOG(INFO) << space << " " << *space << "\n" << live_bitmap << " " << *live_bitmap << "\n" << mark_bitmap << " " << *mark_bitmap; } - typedef std::vector<space::DiscontinuousSpace*>::const_iterator It2; - for (It2 it = discontinuous_spaces_.begin(), end = discontinuous_spaces_.end(); it != end; ++it) { - space::DiscontinuousSpace* space = *it; + for (const auto& space : discontinuous_spaces_) { LOG(INFO) << space << " " << *space << "\n"; } } @@ -1143,11 +1120,8 @@ void Heap::PreZygoteFork() { have_zygote_space_ = true; // Reset the cumulative loggers since we now have a few additional timing phases. - // TODO: C++0x - typedef std::vector<collector::MarkSweep*>::const_iterator It; - for (It it = mark_sweep_collectors_.begin(), end = mark_sweep_collectors_.end(); - it != end; ++it) { - (*it)->ResetCumulativeStatistics(); + for (const auto& collector : mark_sweep_collectors_) { + collector->ResetCumulativeStatistics(); } } @@ -1238,10 +1212,7 @@ collector::GcType Heap::CollectGarbageInternal(collector::GcType gc_type, GcCaus ATRACE_BEGIN(gc_cause_and_type_strings[gc_cause][gc_type]); collector::MarkSweep* collector = NULL; - typedef std::vector<collector::MarkSweep*>::iterator It; - for (It it = mark_sweep_collectors_.begin(), end = mark_sweep_collectors_.end(); - it != end; ++it) { - collector::MarkSweep* cur_collector = *it; + for (const auto& cur_collector : mark_sweep_collectors_) { if (cur_collector->IsConcurrent() == concurrent_gc_ && cur_collector->GetGcType() == gc_type) { collector = cur_collector; break; @@ -1596,9 +1567,7 @@ void Heap::SwapStacks() { void Heap::ProcessCards(base::TimingLogger& timings) { // Clear cards and keep track of cards cleared in the mod-union table. - typedef std::vector<space::ContinuousSpace*>::iterator It; - for (It it = continuous_spaces_.begin(), end = continuous_spaces_.end(); it != end; ++it) { - space::ContinuousSpace* space = *it; + for (const auto& space : continuous_spaces_) { if (space->IsImageSpace()) { base::TimingLogger::ScopedSplit split("ImageModUnionClearCards", &timings); image_mod_union_table_->ClearCards(space); @@ -2085,9 +2054,7 @@ void Heap::RegisterNativeFree(int bytes) { int64_t Heap::GetTotalMemory() const { int64_t ret = 0; - typedef std::vector<space::ContinuousSpace*>::const_iterator It; - for (It it = continuous_spaces_.begin(), end = continuous_spaces_.end(); it != end; ++it) { - space::ContinuousSpace* space = *it; + for (const auto& space : continuous_spaces_) { if (space->IsImageSpace()) { // Currently don't include the image space. } else if (space->IsDlMallocSpace()) { @@ -2095,9 +2062,7 @@ int64_t Heap::GetTotalMemory() const { ret += space->AsDlMallocSpace()->GetFootprint(); } } - typedef std::vector<space::DiscontinuousSpace*>::const_iterator It2; - for (It2 it = discontinuous_spaces_.begin(), end = discontinuous_spaces_.end(); it != end; ++it) { - space::DiscontinuousSpace* space = *it; + for (const auto& space : discontinuous_spaces_) { if (space->IsLargeObjectSpace()) { ret += space->AsLargeObjectSpace()->GetBytesAllocated(); } diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc index 3e75716..8af4d7e 100644 --- a/runtime/indirect_reference_table.cc +++ b/runtime/indirect_reference_table.cc @@ -309,9 +309,8 @@ bool IndirectReferenceTable::Remove(uint32_t cookie, IndirectRef iref) { } void IndirectReferenceTable::VisitRoots(RootVisitor* visitor, void* arg) { - typedef IndirectReferenceTable::iterator It; // TODO: C++0x auto - for (It it = begin(), end = this->end(); it != end; ++it) { - visitor(**it, arg); + for (auto ref : *this) { + visitor(*ref, arg); } } diff --git a/runtime/indirect_reference_table.h b/runtime/indirect_reference_table.h index 97706b8..26f53db 100644 --- a/runtime/indirect_reference_table.h +++ b/runtime/indirect_reference_table.h @@ -248,8 +248,6 @@ bool inline operator!=(const IrtIterator& lhs, const IrtIterator& rhs) { class IndirectReferenceTable { public: - typedef IrtIterator iterator; - IndirectReferenceTable(size_t initialCount, size_t maxCount, IndirectRefKind kind); ~IndirectReferenceTable(); @@ -301,12 +299,12 @@ class IndirectReferenceTable { return segment_state_.parts.topIndex; } - iterator begin() { - return iterator(table_, 0, Capacity()); + IrtIterator begin() { + return IrtIterator(table_, 0, Capacity()); } - iterator end() { - return iterator(table_, Capacity(), Capacity()); + IrtIterator end() { + return IrtIterator(table_, Capacity(), Capacity()); } void VisitRoots(RootVisitor* visitor, void* arg); diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc index ae3a165..6caad01 100644 --- a/runtime/instrumentation.cc +++ b/runtime/instrumentation.cc @@ -206,12 +206,9 @@ static void InstrumentationRestoreStack(Thread* thread, void* arg) } return true; // Ignore upcalls. } - typedef std::deque<instrumentation::InstrumentationStackFrame>::const_iterator It; // TODO: C++0x auto bool removed_stub = false; // TODO: make this search more efficient? - for (It it = instrumentation_stack_->begin(), end = instrumentation_stack_->end(); it != end; - ++it) { - InstrumentationStackFrame instrumentation_frame = *it; + for (InstrumentationStackFrame instrumentation_frame : *instrumentation_stack_) { if (instrumentation_frame.frame_id_ == GetFrameId()) { if (kVerboseInstrumentation) { LOG(INFO) << " Removing exit stub in " << DescribeLocation(); @@ -407,8 +404,7 @@ const void* Instrumentation::GetQuickCodeFor(const mirror::ArtMethod* method) co void Instrumentation::MethodEnterEventImpl(Thread* thread, mirror::Object* this_object, const mirror::ArtMethod* method, uint32_t dex_pc) const { - typedef std::list<InstrumentationListener*>::const_iterator It; // TODO: C++0x auto - It it = method_entry_listeners_.begin(); + auto it = method_entry_listeners_.begin(); bool is_end = (it == method_entry_listeners_.end()); // Implemented this way to prevent problems caused by modification of the list while iterating. while (!is_end) { @@ -422,8 +418,7 @@ void Instrumentation::MethodEnterEventImpl(Thread* thread, mirror::Object* this_ void Instrumentation::MethodExitEventImpl(Thread* thread, mirror::Object* this_object, const mirror::ArtMethod* method, uint32_t dex_pc, const JValue& return_value) const { - typedef std::list<InstrumentationListener*>::const_iterator It; // TODO: C++0x auto - It it = method_exit_listeners_.begin(); + auto it = method_exit_listeners_.begin(); bool is_end = (it == method_exit_listeners_.end()); // Implemented this way to prevent problems caused by modification of the list while iterating. while (!is_end) { @@ -438,10 +433,8 @@ void Instrumentation::MethodUnwindEvent(Thread* thread, mirror::Object* this_obj const mirror::ArtMethod* method, uint32_t dex_pc) const { if (have_method_unwind_listeners_) { - typedef std::list<InstrumentationListener*>::const_iterator It; // TODO: C++0x auto - for (It it = method_unwind_listeners_.begin(), end = method_unwind_listeners_.end(); it != end; - ++it) { - (*it)->MethodUnwind(thread, method, dex_pc); + for (InstrumentationListener* listener : method_unwind_listeners_) { + listener->MethodUnwind(thread, method, dex_pc); } } } @@ -454,9 +447,8 @@ void Instrumentation::DexPcMovedEventImpl(Thread* thread, mirror::Object* this_o // around the problem and in general we may have to move to something like reference counting to // ensure listeners are deleted correctly. std::list<InstrumentationListener*> copy(dex_pc_listeners_); - typedef std::list<InstrumentationListener*>::const_iterator It; // TODO: C++0x auto - for (It it = copy.begin(), end = copy.end(); it != end; ++it) { - (*it)->DexPcMoved(thread, this_object, method, dex_pc); + for (InstrumentationListener* listener : copy) { + listener->DexPcMoved(thread, this_object, method, dex_pc); } } @@ -467,10 +459,8 @@ void Instrumentation::ExceptionCaughtEvent(Thread* thread, const ThrowLocation& if (have_exception_caught_listeners_) { DCHECK_EQ(thread->GetException(NULL), exception_object); thread->ClearException(); - typedef std::list<InstrumentationListener*>::const_iterator It; // TODO: C++0x auto - for (It it = exception_caught_listeners_.begin(), end = exception_caught_listeners_.end(); - it != end; ++it) { - (*it)->ExceptionCaught(thread, throw_location, catch_method, catch_dex_pc, exception_object); + for (InstrumentationListener* listener : exception_caught_listeners_) { + listener->ExceptionCaught(thread, throw_location, catch_method, catch_dex_pc, exception_object); } thread->SetException(throw_location, exception_object); } diff --git a/runtime/intern_table.cc b/runtime/intern_table.cc index e2c1a64..d7398ca 100644 --- a/runtime/intern_table.cc +++ b/runtime/intern_table.cc @@ -40,9 +40,8 @@ void InternTable::DumpForSigQuit(std::ostream& os) const { void InternTable::VisitRoots(RootVisitor* visitor, void* arg, bool clean_dirty) { MutexLock mu(Thread::Current(), intern_table_lock_); - typedef Table::const_iterator It; // TODO: C++0x auto - for (It it = strong_interns_.begin(), end = strong_interns_.end(); it != end; ++it) { - visitor(it->second, arg); + for (const auto& strong_intern : strong_interns_) { + visitor(strong_intern.second, arg); } if (clean_dirty) { is_dirty_ = false; @@ -52,8 +51,7 @@ void InternTable::VisitRoots(RootVisitor* visitor, void* arg, bool clean_dirty) mirror::String* InternTable::Lookup(Table& table, mirror::String* s, uint32_t hash_code) { intern_table_lock_.AssertHeld(Thread::Current()); - typedef Table::const_iterator It; // TODO: C++0x auto - for (It it = table.find(hash_code), end = table.end(); it != end; ++it) { + for (auto it = table.find(hash_code), end = table.end(); it != end; ++it) { mirror::String* existing_string = it->second; if (existing_string->Equals(s)) { return existing_string; @@ -75,8 +73,7 @@ void InternTable::RegisterStrong(mirror::String* s) { void InternTable::Remove(Table& table, const mirror::String* s, uint32_t hash_code) { intern_table_lock_.AssertHeld(Thread::Current()); - typedef Table::iterator It; // TODO: C++0x auto - for (It it = table.find(hash_code), end = table.end(); it != end; ++it) { + for (auto it = table.find(hash_code), end = table.end(); it != end; ++it) { if (it->second == s) { table.erase(it); return; @@ -166,8 +163,8 @@ bool InternTable::ContainsWeak(mirror::String* s) { void InternTable::SweepInternTableWeaks(IsMarkedTester is_marked, void* arg) { MutexLock mu(Thread::Current(), intern_table_lock_); - typedef Table::iterator It; // TODO: C++0x auto - for (It it = weak_interns_.begin(), end = weak_interns_.end(); it != end;) { + // TODO: std::remove_if + lambda. + for (auto it = weak_interns_.begin(), end = weak_interns_.end(); it != end;) { mirror::Object* object = it->second; if (!is_marked(object, arg)) { weak_interns_.erase(it++); diff --git a/runtime/intern_table_test.cc b/runtime/intern_table_test.cc index ffb93eb..d79d2c4 100644 --- a/runtime/intern_table_test.cc +++ b/runtime/intern_table_test.cc @@ -58,8 +58,7 @@ class TestPredicate { public: bool IsMarked(const mirror::Object* s) const { bool erased = false; - typedef std::vector<const mirror::String*>::iterator It; // TODO: C++0x auto - for (It it = expected_.begin(), end = expected_.end(); it != end; ++it) { + for (auto it = expected_.begin(), end = expected_.end(); it != end; ++it) { if (*it == s) { expected_.erase(it); erased = true; diff --git a/runtime/jni_internal.cc b/runtime/jni_internal.cc index 852dd00..460e3b0 100644 --- a/runtime/jni_internal.cc +++ b/runtime/jni_internal.cc @@ -538,12 +538,12 @@ class Libraries { void Dump(std::ostream& os) const { bool first = true; - for (It it = libraries_.begin(); it != libraries_.end(); ++it) { + for (const auto& library : libraries_) { if (!first) { os << ' '; } first = false; - os << it->first; + os << library.first; } } @@ -552,7 +552,7 @@ class Libraries { } SharedLibrary* Get(const std::string& path) { - It it = libraries_.find(path); + auto it = libraries_.find(path); return (it == libraries_.end()) ? NULL : it->second; } @@ -566,8 +566,8 @@ class Libraries { std::string jni_short_name(JniShortName(m)); std::string jni_long_name(JniLongName(m)); const ClassLoader* declaring_class_loader = m->GetDeclaringClass()->GetClassLoader(); - for (It it = libraries_.begin(); it != libraries_.end(); ++it) { - SharedLibrary* library = it->second; + for (const auto& lib : libraries_) { + SharedLibrary* library = lib.second; if (library->GetClassLoader() != declaring_class_loader) { // We only search libraries loaded by the appropriate ClassLoader. continue; @@ -591,8 +591,6 @@ class Libraries { } private: - typedef SafeMap<std::string, SharedLibrary*>::const_iterator It; // TODO: C++0x auto - SafeMap<std::string, SharedLibrary*> libraries_; }; diff --git a/runtime/mirror/art_method.h b/runtime/mirror/art_method.h index 7301f23..5d4a6ea 100644 --- a/runtime/mirror/art_method.h +++ b/runtime/mirror/art_method.h @@ -442,6 +442,7 @@ class MANAGED ArtMethod : public Object { static Class* java_lang_reflect_ArtMethod_; + private: friend struct art::ArtMethodOffsets; // for verifying offset information DISALLOW_IMPLICIT_CONSTRUCTORS(ArtMethod); }; diff --git a/runtime/monitor.cc b/runtime/monitor.cc index 48c0569..ff193c9 100644 --- a/runtime/monitor.cc +++ b/runtime/monitor.cc @@ -979,9 +979,7 @@ void MonitorList::Add(Monitor* m) { void MonitorList::SweepMonitorList(IsMarkedTester is_marked, void* arg) { MutexLock mu(Thread::Current(), monitor_list_lock_); - typedef std::list<Monitor*>::iterator It; // TODO: C++0x auto - It it = list_.begin(); - while (it != list_.end()) { + for (auto it = list_.begin(); it != list_.end(); ) { Monitor* m = *it; if (!is_marked(m->GetObject(), arg)) { VLOG(monitor) << "freeing monitor " << m << " belonging to unmarked object " << m->GetObject(); diff --git a/runtime/native/dalvik_system_DexFile.cc b/runtime/native/dalvik_system_DexFile.cc index 061dfb8..4ee3533 100644 --- a/runtime/native/dalvik_system_DexFile.cc +++ b/runtime/native/dalvik_system_DexFile.cc @@ -253,14 +253,10 @@ static jboolean DexFile_isDexOptNeeded(JNIEnv* env, jclass, jstring javaFilename return JNI_TRUE; } - gc::Heap* heap = runtime->GetHeap(); - const std::vector<gc::space::ContinuousSpace*>& spaces = heap->GetContinuousSpaces(); - // TODO: C++0x auto - typedef std::vector<gc::space::ContinuousSpace*>::const_iterator It; - for (It it = spaces.begin(), end = spaces.end(); it != end; ++it) { - if ((*it)->IsImageSpace()) { + for (const auto& space : runtime->GetHeap()->GetContinuousSpaces()) { + if (space->IsImageSpace()) { // TODO: Ensure this works with multiple image spaces. - const ImageHeader& image_header = (*it)->AsImageSpace()->GetImageHeader(); + const ImageHeader& image_header = space->AsImageSpace()->GetImageHeader(); if (oat_file->GetOatHeader().GetImageFileLocationOatChecksum() != image_header.GetOatChecksum()) { ScopedObjectAccess soa(env); LOG(INFO) << "DexFile_isDexOptNeeded cache file " << cache_location diff --git a/runtime/output_stream_test.cc b/runtime/output_stream_test.cc index 8da2ac9..d5e9755 100644 --- a/runtime/output_stream_test.cc +++ b/runtime/output_stream_test.cc @@ -78,4 +78,4 @@ TEST_F(OutputStreamTest, Vector) { CheckTestOutput(output); } -} // namespace std +} // namespace art diff --git a/runtime/reference_table.cc b/runtime/reference_table.cc index de64c26..8e23cbb 100644 --- a/runtime/reference_table.cc +++ b/runtime/reference_table.cc @@ -232,9 +232,8 @@ void ReferenceTable::Dump(std::ostream& os, const Table& entries) { } void ReferenceTable::VisitRoots(RootVisitor* visitor, void* arg) { - typedef Table::const_iterator It; // TODO: C++0x auto - for (It it = entries_.begin(), end = entries_.end(); it != end; ++it) { - visitor(*it, arg); + for (const auto& ref : entries_) { + visitor(ref, arg); } } diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc index 9c28c87..671924a 100644 --- a/runtime/thread_list.cc +++ b/runtime/thread_list.cc @@ -53,8 +53,8 @@ bool ThreadList::Contains(Thread* thread) { } bool ThreadList::Contains(pid_t tid) { - for (It it = list_.begin(), end = list_.end(); it != end; ++it) { - if ((*it)->tid_ == tid) { + for (const auto& thread : list_) { + if (thread->tid_ == tid) { return true; } } @@ -113,8 +113,8 @@ void ThreadList::DumpUnattachedThreads(std::ostream& os) { void ThreadList::DumpLocked(std::ostream& os) { os << "DALVIK THREADS (" << list_.size() << "):\n"; - for (It it = list_.begin(), end = list_.end(); it != end; ++it) { - (*it)->Dump(os); + for (const auto& thread : list_) { + thread->Dump(os); os << "\n"; } } @@ -122,8 +122,7 @@ void ThreadList::DumpLocked(std::ostream& os) { void ThreadList::AssertThreadsAreSuspended(Thread* self, Thread* ignore1, Thread* ignore2) { MutexLock mu(self, *Locks::thread_list_lock_); MutexLock mu2(self, *Locks::thread_suspend_count_lock_); - for (It it = list_.begin(), end = list_.end(); it != end; ++it) { - Thread* thread = *it; + for (const auto& thread : list_) { if (thread != ignore1 && thread != ignore2) { CHECK(thread->IsSuspended()) << "\nUnsuspended thread: <<" << *thread << "\n" @@ -160,9 +159,7 @@ size_t ThreadList::RunCheckpoint(Closure* checkpoint_function) { // Call a checkpoint function for each thread, threads which are suspend get their checkpoint // manually called. MutexLock mu(self, *Locks::thread_list_lock_); - // TODO: C++0x auto. - for (It it = list_.begin(), end = list_.end(); it != end; ++it) { - Thread* thread = *it; + for (const auto& thread : list_) { if (thread != self) { for (;;) { if (thread->RequestCheckpoint(checkpoint_function)) { @@ -189,8 +186,7 @@ size_t ThreadList::RunCheckpoint(Closure* checkpoint_function) { checkpoint_function->Run(self); // Run the checkpoint on the suspended threads. - for (size_t i = 0; i < suspended_count_modified_threads.size(); ++i) { - Thread* thread = suspended_count_modified_threads[i]; + for (const auto& thread : suspended_count_modified_threads) { if (!thread->IsSuspended()) { // Wait until the thread is suspended. uint64_t start = NanoTime(); @@ -243,8 +239,7 @@ void ThreadList::SuspendAll() { // Update global suspend all state for attaching threads. ++suspend_all_count_; // Increment everybody's suspend count (except our own). - for (It it = list_.begin(), end = list_.end(); it != end; ++it) { - Thread* thread = *it; + for (const auto& thread : list_) { if (thread == self) { continue; } @@ -285,8 +280,7 @@ void ThreadList::ResumeAll() { // Update global suspend all state for attaching threads. --suspend_all_count_; // Decrement the suspend counts for all threads. - for (It it = list_.begin(), end = list_.end(); it != end; ++it) { - Thread* thread = *it; + for (const auto& thread : list_) { if (thread == self) { continue; } @@ -341,8 +335,7 @@ void ThreadList::SuspendAllForDebugger() { ++suspend_all_count_; ++debug_suspend_all_count_; // Increment everybody's suspend count (except our own). - for (It it = list_.begin(), end = list_.end(); it != end; ++it) { - Thread* thread = *it; + for (const auto& thread : list_) { if (thread == self || thread == debug_thread) { continue; } @@ -427,8 +420,7 @@ void ThreadList::UndoDebuggerSuspensions() { suspend_all_count_ -= debug_suspend_all_count_; debug_suspend_all_count_ = 0; // Update running threads. - for (It it = list_.begin(), end = list_.end(); it != end; ++it) { - Thread* thread = *it; + for (const auto& thread : list_) { if (thread == self || thread->debug_suspend_count_ == 0) { continue; } @@ -457,8 +449,7 @@ void ThreadList::WaitForOtherNonDaemonThreadsToExit() { } all_threads_are_daemons = true; MutexLock mu(self, *Locks::thread_list_lock_); - for (It it = list_.begin(), end = list_.end(); it != end; ++it) { - Thread* thread = *it; + for (const auto& thread : list_) { if (thread != self && !thread->IsDaemon()) { all_threads_are_daemons = false; break; @@ -476,8 +467,7 @@ void ThreadList::SuspendAllDaemonThreads() { MutexLock mu(self, *Locks::thread_list_lock_); { // Tell all the daemons it's time to suspend. MutexLock mu2(self, *Locks::thread_suspend_count_lock_); - for (It it = list_.begin(), end = list_.end(); it != end; ++it) { - Thread* thread = *it; + for (const auto& thread : list_) { // This is only run after all non-daemon threads have exited, so the remainder should all be // daemons. CHECK(thread->IsDaemon()) << *thread; @@ -491,8 +481,7 @@ void ThreadList::SuspendAllDaemonThreads() { for (int i = 0; i < 10; ++i) { usleep(200 * 1000); bool all_suspended = true; - for (It it = list_.begin(), end = list_.end(); it != end; ++it) { - Thread* thread = *it; + for (const auto& thread : list_) { if (thread != self && thread->GetState() == kRunnable) { if (!have_complained) { LOG(WARNING) << "daemon thread not yet suspended: " << *thread; @@ -567,22 +556,22 @@ void ThreadList::Unregister(Thread* self) { } void ThreadList::ForEach(void (*callback)(Thread*, void*), void* context) { - for (It it = list_.begin(), end = list_.end(); it != end; ++it) { - callback(*it, context); + for (const auto& thread : list_) { + callback(thread, context); } } void ThreadList::VisitRoots(RootVisitor* visitor, void* arg) const { MutexLock mu(Thread::Current(), *Locks::thread_list_lock_); - for (It it = list_.begin(), end = list_.end(); it != end; ++it) { - (*it)->VisitRoots(visitor, arg); + for (const auto& thread : list_) { + thread->VisitRoots(visitor, arg); } } void ThreadList::VerifyRoots(VerifyRootVisitor* visitor, void* arg) const { MutexLock mu(Thread::Current(), *Locks::thread_list_lock_); - for (It it = list_.begin(), end = list_.end(); it != end; ++it) { - (*it)->VerifyRoots(visitor, arg); + for (const auto& thread : list_) { + thread->VerifyRoots(visitor, arg); } } @@ -607,9 +596,9 @@ void ThreadList::ReleaseThreadId(Thread* self, uint32_t id) { Thread* ThreadList::FindThreadByThinLockId(uint32_t thin_lock_id) { MutexLock mu(Thread::Current(), *Locks::thread_list_lock_); - for (It it = list_.begin(), end = list_.end(); it != end; ++it) { - if ((*it)->GetThinLockId() == thin_lock_id) { - return *it; + for (const auto& thread : list_) { + if (thread->GetThinLockId() == thin_lock_id) { + return thread; } } return NULL; diff --git a/runtime/thread_list.h b/runtime/thread_list.h index d95f191..3df3e2c 100644 --- a/runtime/thread_list.h +++ b/runtime/thread_list.h @@ -102,8 +102,6 @@ class ThreadList { Thread* FindThreadByThinLockId(uint32_t thin_lock_id); private: - typedef std::list<Thread*>::const_iterator It; // TODO: C++0x auto - uint32_t AllocThreadId(Thread* self); void ReleaseThreadId(Thread* self, uint32_t id) LOCKS_EXCLUDED(allocated_ids_lock_); diff --git a/runtime/thread_pool.h b/runtime/thread_pool.h index c26926c..9c6d47b 100644 --- a/runtime/thread_pool.h +++ b/runtime/thread_pool.h @@ -55,6 +55,7 @@ class ThreadPoolWorker { const size_t stack_size_; pthread_t pthread_; + private: friend class ThreadPool; DISALLOW_COPY_AND_ASSIGN(ThreadPoolWorker); }; @@ -117,6 +118,7 @@ class ThreadPool { uint64_t total_wait_time_; Barrier creation_barier_; + private: friend class ThreadPoolWorker; friend class WorkStealingWorker; DISALLOW_COPY_AND_ASSIGN(ThreadPool); @@ -153,6 +155,7 @@ class WorkStealingWorker : public ThreadPoolWorker { WorkStealingWorker(ThreadPool* thread_pool, const std::string& name, size_t stack_size); virtual void Run(); + private: friend class WorkStealingThreadPool; DISALLOW_COPY_AND_ASSIGN(WorkStealingWorker); }; diff --git a/runtime/trace.cc b/runtime/trace.cc index 13e2bf6..5d6943c 100644 --- a/runtime/trace.cc +++ b/runtime/trace.cc @@ -610,12 +610,9 @@ void Trace::GetVisitedMethods(size_t buf_size, } } -void Trace::DumpMethodList(std::ostream& os, - const std::set<mirror::ArtMethod*>& visited_methods) { - typedef std::set<mirror::ArtMethod*>::const_iterator It; // TODO: C++0x auto +void Trace::DumpMethodList(std::ostream& os, const std::set<mirror::ArtMethod*>& visited_methods) { MethodHelper mh; - for (It it = visited_methods.begin(); it != visited_methods.end(); ++it) { - mirror::ArtMethod* method = *it; + for (const auto& method : visited_methods) { mh.ChangeMethod(method); os << StringPrintf("%p\t%s\t%s\t%s\t%s\n", method, PrettyDescriptor(mh.GetDeclaringClassDescriptor()).c_str(), mh.GetName(), diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h index 6171943..ff4386e 100644 --- a/runtime/verifier/method_verifier.h +++ b/runtime/verifier/method_verifier.h @@ -122,7 +122,7 @@ class PcToRegisterLineTable { uint16_t registers_size, MethodVerifier* verifier); RegisterLine* GetLine(size_t idx) { - Table::iterator result = pc_to_register_line_.find(idx); // TODO: C++0x auto + auto result = pc_to_register_line_.find(idx); if (result == pc_to_register_line_.end()) { return NULL; } else { diff --git a/runtime/verifier/reg_type.cc b/runtime/verifier/reg_type.cc index 8418928..25f840c 100644 --- a/runtime/verifier/reg_type.cc +++ b/runtime/verifier/reg_type.cc @@ -395,8 +395,7 @@ std::string UnresolvedMergedType::Dump() const { std::stringstream result; std::set<uint16_t> types = GetMergedTypes(); result << "UnresolvedMergedReferences("; - typedef std::set<uint16_t>::const_iterator It; // TODO: C++0x auto - It it = types.begin(); + auto it = types.begin(); result << reg_type_cache_->GetFromId(*it).Dump(); for (++it; it != types.end(); ++it) { result << ", "; @@ -609,9 +608,8 @@ std::set<uint16_t> UnresolvedMergedType::GetMergedTypes() const { types.insert(refs.second); } if (kIsDebugBuild) { - typedef std::set<uint16_t>::const_iterator It; // TODO: C++0x auto - for (It it = types.begin(); it != types.end(); ++it) { - CHECK(!reg_type_cache_->GetFromId(*it).IsUnresolvedMergedReference()); + for (const auto& type : types) { + CHECK(!reg_type_cache_->GetFromId(type).IsUnresolvedMergedReference()); } } return types; diff --git a/runtime/verifier/reg_type.h b/runtime/verifier/reg_type.h index 33f4195..865ba20 100644 --- a/runtime/verifier/reg_type.h +++ b/runtime/verifier/reg_type.h @@ -287,6 +287,7 @@ class RegType { friend class RegTypeCache; + private: DISALLOW_COPY_AND_ASSIGN(RegType); }; diff --git a/runtime/verifier/register_line.cc b/runtime/verifier/register_line.cc index 24a626b..5affe47 100644 --- a/runtime/verifier/register_line.cc +++ b/runtime/verifier/register_line.cc @@ -208,9 +208,8 @@ std::string RegisterLine::Dump() const { result += GetRegisterType(i).Dump(); result += "],"; } - typedef std::deque<uint32_t>::const_iterator It; // TODO: C++0x auto - for (It it = monitors_.begin(), end = monitors_.end(); it != end; ++it) { - result += StringPrintf("{%d},", *it); + for (const auto& monitor : monitors_) { + result += StringPrintf("{%d},", monitor); } return result; } diff --git a/tools/cpplint.py b/tools/cpplint.py index 0b5fb93..30b5216 100755 --- a/tools/cpplint.py +++ b/tools/cpplint.py @@ -53,12 +53,8 @@ # - Check for 0 in char context (should be '\0') # - Check for camel-case method name conventions for methods # that are not simple inline getters and setters -# - Check that base classes have virtual destructors -# put " // namespace" after } that closes a namespace, with -# namespace's name after 'namespace' if it is named. # - Do not indent namespace contents # - Avoid inlining non-trivial constructors in header files -# include base/basictypes.h if DISALLOW_EVIL_CONSTRUCTORS is used # - Check for old-school (void) cast for call-sites of functions # ignored return value # - Check gUnit usage of anonymous namespace @@ -80,6 +76,7 @@ same line, but it is far from perfect (in either direction). """ import codecs +import copy import getopt import math # for log import os @@ -139,6 +136,22 @@ Syntax: cpplint.py [--verbose=#] [--output=vs7] [--filter=-x,+y,...] the top-level categories like 'build' and 'whitespace' will also be printed. If 'detailed' is provided, then a count is provided for each category like 'build/class'. + + root=subdir + The root directory used for deriving header guard CPP variable. + By default, the header guard CPP variable is calculated as the relative + path to the directory that contains .git, .hg, or .svn. When this flag + is specified, the relative path is calculated from the specified + directory. If the specified directory does not exist, this flag is + ignored. + + Examples: + Assuing that src/.git exists, the header guard CPP variables for + src/chrome/browser/ui/browser.h are: + + No flag => CHROME_BROWSER_UI_BROWSER_H_ + --root=chrome => BROWSER_UI_BROWSER_H_ + --root=chrome/browser => UI_BROWSER_H_ """ # We categorize each error message we print. Here are the categories. @@ -161,6 +174,7 @@ _ERROR_CATEGORIES = [ 'build/printf_format', 'build/storage_class', 'legal/copyright', + 'readability/alt_tokens', 'readability/braces', 'readability/casting', 'readability/check', @@ -169,6 +183,7 @@ _ERROR_CATEGORIES = [ 'readability/function', 'readability/multiline_comment', 'readability/multiline_string', + 'readability/namespace', 'readability/nolint', 'readability/streams', 'readability/todo', @@ -189,13 +204,14 @@ _ERROR_CATEGORIES = [ 'runtime/sizeof', 'runtime/string', 'runtime/threadsafe_fn', - 'runtime/virtual', 'whitespace/blank_line', 'whitespace/braces', 'whitespace/comma', 'whitespace/comments', + 'whitespace/empty_loop_body', 'whitespace/end_of_line', 'whitespace/ending_newline', + 'whitespace/forcolon', 'whitespace/indent', 'whitespace/labels', 'whitespace/line_length', @@ -241,8 +257,9 @@ _CPP_HEADERS = frozenset([ 'numeric', 'ostream', 'ostream.h', 'parsestream.h', 'pfstream.h', 'PlotFile.h', 'procbuf.h', 'pthread_alloc.h', 'rope', 'rope.h', 'ropeimpl.h', 'SFile.h', 'slist', 'slist.h', 'stack.h', 'stdexcept', - 'stdiostream.h', 'streambuf.h', 'stream.h', 'strfile.h', 'string', - 'strstream', 'strstream.h', 'tempbuf.h', 'tree.h', 'typeinfo', 'valarray', + 'stdiostream.h', 'streambuf', 'streambuf.h', 'stream.h', 'strfile.h', + 'string', 'strstream', 'strstream.h', 'tempbuf.h', 'tree.h', 'typeinfo', + 'valarray', ]) @@ -278,6 +295,34 @@ for op, inv_replacement in [('==', 'NE'), ('!=', 'EQ'), _CHECK_REPLACEMENT['EXPECT_FALSE_M'][op] = 'EXPECT_%s_M' % inv_replacement _CHECK_REPLACEMENT['ASSERT_FALSE_M'][op] = 'ASSERT_%s_M' % inv_replacement +# Alternative tokens and their replacements. For full list, see section 2.5 +# Alternative tokens [lex.digraph] in the C++ standard. +# +# Digraphs (such as '%:') are not included here since it's a mess to +# match those on a word boundary. +_ALT_TOKEN_REPLACEMENT = { + 'and': '&&', + 'bitor': '|', + 'or': '||', + 'xor': '^', + 'compl': '~', + 'bitand': '&', + 'and_eq': '&=', + 'or_eq': '|=', + 'xor_eq': '^=', + 'not': '!', + 'not_eq': '!=' + } + +# Compile regular expression that matches all the above keywords. The "[ =()]" +# bit is meant to avoid matching these keywords outside of boolean expressions. +# +# False positives include C-style multi-line comments (http://go/nsiut ) +# and multi-line strings (http://go/beujw ), but those have always been +# troublesome for cpplint. +_ALT_TOKEN_REPLACEMENT_PATTERN = re.compile( + r'[ =()](' + ('|'.join(_ALT_TOKEN_REPLACEMENT.keys())) + r')(?=[ (]|$)') + # These constants define types of headers for use with # _IncludeState.CheckNextIncludeOrder(). @@ -287,6 +332,17 @@ _LIKELY_MY_HEADER = 3 _POSSIBLE_MY_HEADER = 4 _OTHER_HEADER = 5 +# These constants define the current inline assembly state +_NO_ASM = 0 # Outside of inline assembly block +_INSIDE_ASM = 1 # Inside inline assembly block +_END_ASM = 2 # Last line of inline assembly block +_BLOCK_ASM = 3 # The whole block is an inline assembly block + +# Match start of assembly blocks +_MATCH_ASM = re.compile(r'^\s*(?:asm|_asm|__asm|__asm__)' + r'(?:\s+(volatile|__volatile__))?' + r'\s*[{(]') + _regexp_compile_cache = {} @@ -297,6 +353,10 @@ _RE_SUPPRESSION = re.compile(r'\bNOLINT\b(\([^)]*\))?') # on which those errors are expected and should be suppressed. _error_suppressions = {} +# The root directory used for deriving header guard CPP variable. +# This is set by --root flag. +_root = None + def ParseNolintSuppressions(filename, raw_line, linenum, error): """Updates the global list of error-suppressions. @@ -649,7 +709,6 @@ class _FunctionState(object): if not self.in_a_function: return # END android-added - if Match(r'T(EST|est)', self.current_function): base_trigger = self._TEST_TRIGGER else: @@ -736,7 +795,6 @@ class FileInfo: return "art/" + fullname[len(prefix) + 1:] # END android-changed - # Don't know what to do; header guard warnings may be wrong... return fullname @@ -825,6 +883,9 @@ def Error(filename, linenum, category, confidence, message): if _cpplint_state.output_format == 'vs7': sys.stderr.write('%s(%s): %s [%s] [%d]\n' % ( filename, linenum, message, category, confidence)) + elif _cpplint_state.output_format == 'eclipse': + sys.stderr.write('%s:%s: warning: %s [%s] [%d]\n' % ( + filename, linenum, message, category, confidence)) else: sys.stderr.write('%s:%s: %s [%s] [%d]\n' % ( filename, linenum, message, category, confidence)) @@ -934,7 +995,7 @@ class CleansedLines(object): 1) elided member contains lines without strings and comments, 2) lines member contains lines without comments, and - 3) raw member contains all the lines without processing. + 3) raw_lines member contains all the lines without processing. All these three members are of <type 'list'>, and of the same length. """ @@ -974,6 +1035,29 @@ class CleansedLines(object): return elided +def FindEndOfExpressionInLine(line, startpos, depth, startchar, endchar): + """Find the position just after the matching endchar. + + Args: + line: a CleansedLines line. + startpos: start searching at this position. + depth: nesting level at startpos. + startchar: expression opening character. + endchar: expression closing character. + + Returns: + Index just after endchar. + """ + for i in xrange(startpos, len(line)): + if line[i] == startchar: + depth += 1 + elif line[i] == endchar: + depth -= 1 + if depth == 0: + return i + 1 + return -1 + + def CloseExpression(clean_lines, linenum, pos): """If input points to ( or { or [, finds the position that closes it. @@ -1000,18 +1084,23 @@ def CloseExpression(clean_lines, linenum, pos): if startchar == '[': endchar = ']' if startchar == '{': endchar = '}' - num_open = line.count(startchar) - line.count(endchar) - while linenum < clean_lines.NumLines() and num_open > 0: + # Check first line + end_pos = FindEndOfExpressionInLine(line, pos, 0, startchar, endchar) + if end_pos > -1: + return (line, linenum, end_pos) + tail = line[pos:] + num_open = tail.count(startchar) - tail.count(endchar) + while linenum < clean_lines.NumLines() - 1: linenum += 1 line = clean_lines.elided[linenum] - num_open += line.count(startchar) - line.count(endchar) - # OK, now find the endchar that actually got us back to even - endpos = len(line) - while num_open >= 0: - endpos = line.rfind(')', 0, endpos) - num_open -= 1 # chopped off another ) - return (line, linenum, endpos + 1) + delta = line.count(startchar) - line.count(endchar) + if num_open + delta <= 0: + return (line, linenum, + FindEndOfExpressionInLine(line, 0, num_open, startchar, endchar)) + num_open += delta + # Did not find endchar before end of file, give up + return (line, clean_lines.NumLines(), -1) def CheckForCopyright(filename, lines, error): """Logs an error if no Copyright message appears at the top of the file.""" @@ -1041,8 +1130,13 @@ def GetHeaderGuardCPPVariable(filename): # Restores original filename in case that cpplint is invoked from Emacs's # flymake. filename = re.sub(r'_flymake\.h$', '.h', filename) + filename = re.sub(r'/\.flymake/([^/]*)$', r'/\1', filename) + fileinfo = FileInfo(filename) - return re.sub(r'[-./\s]', '_', fileinfo.RepositoryName()).upper() + '_' + file_path_from_root = fileinfo.RepositoryName() + if _root: + file_path_from_root = re.sub('^' + _root + os.sep, '', file_path_from_root) + return re.sub(r'[-./\s]', '_', file_path_from_root).upper() + '_' def CheckForHeaderGuard(filename, lines, error): @@ -1206,6 +1300,7 @@ threading_list = ( ('gmtime(', 'gmtime_r('), ('localtime(', 'localtime_r('), ('rand(', 'rand_r('), + ('readdir(', 'readdir_r('), ('strtok(', 'strtok_r('), ('ttyname(', 'ttyname_r('), ) @@ -1266,17 +1361,55 @@ def CheckInvalidIncrement(filename, clean_lines, linenum, error): 'Changing pointer instead of value (or unused value of operator*).') -class _ClassInfo(object): +class _BlockInfo(object): + """Stores information about a generic block of code.""" + + def __init__(self, seen_open_brace): + self.seen_open_brace = seen_open_brace + self.open_parentheses = 0 + self.inline_asm = _NO_ASM + + def CheckBegin(self, filename, clean_lines, linenum, error): + """Run checks that applies to text up to the opening brace. + + This is mostly for checking the text after the class identifier + and the "{", usually where the base class is specified. For other + blocks, there isn't much to check, so we always pass. + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + error: The function to call with any errors found. + """ + pass + + def CheckEnd(self, filename, clean_lines, linenum, error): + """Run checks that applies to text after the closing brace. + + This is mostly used for checking end of namespace comments. + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + error: The function to call with any errors found. + """ + pass + + +class _ClassInfo(_BlockInfo): """Stores information about a class.""" - def __init__(self, name, clean_lines, linenum): + def __init__(self, name, class_or_struct, clean_lines, linenum): + _BlockInfo.__init__(self, False) self.name = name - self.linenum = linenum - self.seen_open_brace = False + self.starting_linenum = linenum self.is_derived = False - self.virtual_method_linenumber = None - self.has_virtual_destructor = False - self.brace_depth = 0 + if class_or_struct == 'struct': + self.access = 'public' + else: + self.access = 'private' # Try to find the end of the class. This will be confused by things like: # class A { @@ -1286,26 +1419,324 @@ class _ClassInfo(object): self.last_line = 0 depth = 0 for i in range(linenum, clean_lines.NumLines()): - line = clean_lines.lines[i] + line = clean_lines.elided[i] depth += line.count('{') - line.count('}') if not depth: self.last_line = i break + def CheckBegin(self, filename, clean_lines, linenum, error): + # Look for a bare ':' + if Search('(^|[^:]):($|[^:])', clean_lines.elided[linenum]): + self.is_derived = True -class _ClassState(object): - """Holds the current state of the parse relating to class declarations. - It maintains a stack of _ClassInfos representing the parser's guess - as to the current nesting of class declarations. The innermost class - is at the top (back) of the stack. Typically, the stack will either - be empty or have exactly one entry. - """ +class _NamespaceInfo(_BlockInfo): + """Stores information about a namespace.""" + + def __init__(self, name, linenum): + _BlockInfo.__init__(self, False) + self.name = name or '' + self.starting_linenum = linenum + + def CheckEnd(self, filename, clean_lines, linenum, error): + """Check end of namespace comments.""" + line = clean_lines.raw_lines[linenum] + + # Check how many lines is enclosed in this namespace. Don't issue + # warning for missing namespace comments if there aren't enough + # lines. However, do apply checks if there is already an end of + # namespace comment and it's incorrect. + # + # TODO(unknown): We always want to check end of namespace comments + # if a namespace is large, but sometimes we also want to apply the + # check if a short namespace contained nontrivial things (something + # other than forward declarations). There is currently no logic on + # deciding what these nontrivial things are, so this check is + # triggered by namespace size only, which works most of the time. + if (linenum - self.starting_linenum < 10 + and not Match(r'};*\s*(//|/\*).*\bnamespace\b', line)): + return + + # Look for matching comment at end of namespace. + # + # Note that we accept C style "/* */" comments for terminating + # namespaces, so that code that terminate namespaces inside + # preprocessor macros can be cpplint clean. Example: http://go/nxpiz + # + # We also accept stuff like "// end of namespace <name>." with the + # period at the end. + # + # Besides these, we don't accept anything else, otherwise we might + # get false negatives when existing comment is a substring of the + # expected namespace. Example: http://go/ldkdc, http://cl/23548205 + if self.name: + # Named namespace + if not Match((r'};*\s*(//|/\*).*\bnamespace\s+' + re.escape(self.name) + + r'[\*/\.\\\s]*$'), + line): + error(filename, linenum, 'readability/namespace', 5, + 'Namespace should be terminated with "// namespace %s"' % + self.name) + else: + # Anonymous namespace + if not Match(r'};*\s*(//|/\*).*\bnamespace[\*/\.\\\s]*$', line): + error(filename, linenum, 'readability/namespace', 5, + 'Namespace should be terminated with "// namespace"') + + +class _PreprocessorInfo(object): + """Stores checkpoints of nesting stacks when #if/#else is seen.""" + + def __init__(self, stack_before_if): + # The entire nesting stack before #if + self.stack_before_if = stack_before_if + + # The entire nesting stack up to #else + self.stack_before_else = [] + + # Whether we have already seen #else or #elif + self.seen_else = False + + +class _NestingState(object): + """Holds states related to parsing braces.""" def __init__(self): - self.classinfo_stack = [] + # Stack for tracking all braces. An object is pushed whenever we + # see a "{", and popped when we see a "}". Only 3 types of + # objects are possible: + # - _ClassInfo: a class or struct. + # - _NamespaceInfo: a namespace. + # - _BlockInfo: some other type of block. + self.stack = [] + + # Stack of _PreprocessorInfo objects. + self.pp_stack = [] + + def SeenOpenBrace(self): + """Check if we have seen the opening brace for the innermost block. + + Returns: + True if we have seen the opening brace, False if the innermost + block is still expecting an opening brace. + """ + return (not self.stack) or self.stack[-1].seen_open_brace + + def InNamespaceBody(self): + """Check if we are currently one level inside a namespace body. + + Returns: + True if top of the stack is a namespace block, False otherwise. + """ + return self.stack and isinstance(self.stack[-1], _NamespaceInfo) + + def UpdatePreprocessor(self, line): + """Update preprocessor stack. + + We need to handle preprocessors due to classes like this: + #ifdef SWIG + struct ResultDetailsPageElementExtensionPoint { + #else + struct ResultDetailsPageElementExtensionPoint : public Extension { + #endif + (see http://go/qwddn for original example) + + We make the following assumptions (good enough for most files): + - Preprocessor condition evaluates to true from #if up to first + #else/#elif/#endif. + + - Preprocessor condition evaluates to false from #else/#elif up + to #endif. We still perform lint checks on these lines, but + these do not affect nesting stack. + + Args: + line: current line to check. + """ + if Match(r'^\s*#\s*(if|ifdef|ifndef)\b', line): + # Beginning of #if block, save the nesting stack here. The saved + # stack will allow us to restore the parsing state in the #else case. + self.pp_stack.append(_PreprocessorInfo(copy.deepcopy(self.stack))) + elif Match(r'^\s*#\s*(else|elif)\b', line): + # Beginning of #else block + if self.pp_stack: + if not self.pp_stack[-1].seen_else: + # This is the first #else or #elif block. Remember the + # whole nesting stack up to this point. This is what we + # keep after the #endif. + self.pp_stack[-1].seen_else = True + self.pp_stack[-1].stack_before_else = copy.deepcopy(self.stack) + + # Restore the stack to how it was before the #if + self.stack = copy.deepcopy(self.pp_stack[-1].stack_before_if) + else: + # TODO(unknown): unexpected #else, issue warning? + pass + elif Match(r'^\s*#\s*endif\b', line): + # End of #if or #else blocks. + if self.pp_stack: + # If we saw an #else, we will need to restore the nesting + # stack to its former state before the #else, otherwise we + # will just continue from where we left off. + if self.pp_stack[-1].seen_else: + # Here we can just use a shallow copy since we are the last + # reference to it. + self.stack = self.pp_stack[-1].stack_before_else + # Drop the corresponding #if + self.pp_stack.pop() + else: + # TODO(unknown): unexpected #endif, issue warning? + pass - def CheckFinished(self, filename, error): + def Update(self, filename, clean_lines, linenum, error): + """Update nesting state with current line. + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + error: The function to call with any errors found. + """ + line = clean_lines.elided[linenum] + + # Update pp_stack first + self.UpdatePreprocessor(line) + + # Count parentheses. This is to avoid adding struct arguments to + # the nesting stack. + if self.stack: + inner_block = self.stack[-1] + depth_change = line.count('(') - line.count(')') + inner_block.open_parentheses += depth_change + + # Also check if we are starting or ending an inline assembly block. + if inner_block.inline_asm in (_NO_ASM, _END_ASM): + if (depth_change != 0 and + inner_block.open_parentheses == 1 and + _MATCH_ASM.match(line)): + # Enter assembly block + inner_block.inline_asm = _INSIDE_ASM + else: + # Not entering assembly block. If previous line was _END_ASM, + # we will now shift to _NO_ASM state. + inner_block.inline_asm = _NO_ASM + elif (inner_block.inline_asm == _INSIDE_ASM and + inner_block.open_parentheses == 0): + # Exit assembly block + inner_block.inline_asm = _END_ASM + + # Consume namespace declaration at the beginning of the line. Do + # this in a loop so that we catch same line declarations like this: + # namespace proto2 { namespace bridge { class MessageSet; } } + while True: + # Match start of namespace. The "\b\s*" below catches namespace + # declarations even if it weren't followed by a whitespace, this + # is so that we don't confuse our namespace checker. The + # missing spaces will be flagged by CheckSpacing. + namespace_decl_match = Match(r'^\s*namespace\b\s*([:\w]+)?(.*)$', line) + if not namespace_decl_match: + break + + new_namespace = _NamespaceInfo(namespace_decl_match.group(1), linenum) + self.stack.append(new_namespace) + + line = namespace_decl_match.group(2) + if line.find('{') != -1: + new_namespace.seen_open_brace = True + line = line[line.find('{') + 1:] + + # Look for a class declaration in whatever is left of the line + # after parsing namespaces. The regexp accounts for decorated classes + # such as in: + # class LOCKABLE API Object { + # }; + # + # Templates with class arguments may confuse the parser, for example: + # template <class T + # class Comparator = less<T>, + # class Vector = vector<T> > + # class HeapQueue { + # + # Because this parser has no nesting state about templates, by the + # time it saw "class Comparator", it may think that it's a new class. + # Nested templates have a similar problem: + # template < + # typename ExportedType, + # typename TupleType, + # template <typename, typename> class ImplTemplate> + # + # To avoid these cases, we ignore classes that are followed by '=' or '>' + class_decl_match = Match( + r'\s*(template\s*<[\w\s<>,:]*>\s*)?' + '(class|struct)\s+([A-Z_]+\s+)*(\w+(?:::\w+)*)' + '(([^=>]|<[^<>]*>)*)$', line) + if (class_decl_match and + (not self.stack or self.stack[-1].open_parentheses == 0)): + self.stack.append(_ClassInfo( + class_decl_match.group(4), class_decl_match.group(2), + clean_lines, linenum)) + line = class_decl_match.group(5) + + # If we have not yet seen the opening brace for the innermost block, + # run checks here. + if not self.SeenOpenBrace(): + self.stack[-1].CheckBegin(filename, clean_lines, linenum, error) + + # Update access control if we are inside a class/struct + if self.stack and isinstance(self.stack[-1], _ClassInfo): + access_match = Match(r'\s*(public|private|protected)\s*:', line) + if access_match: + self.stack[-1].access = access_match.group(1) + + # Consume braces or semicolons from what's left of the line + while True: + # Match first brace, semicolon, or closed parenthesis. + matched = Match(r'^[^{;)}]*([{;)}])(.*)$', line) + if not matched: + break + + token = matched.group(1) + if token == '{': + # If namespace or class hasn't seen a opening brace yet, mark + # namespace/class head as complete. Push a new block onto the + # stack otherwise. + if not self.SeenOpenBrace(): + self.stack[-1].seen_open_brace = True + else: + self.stack.append(_BlockInfo(True)) + if _MATCH_ASM.match(line): + self.stack[-1].inline_asm = _BLOCK_ASM + elif token == ';' or token == ')': + # If we haven't seen an opening brace yet, but we already saw + # a semicolon, this is probably a forward declaration. Pop + # the stack for these. + # + # Similarly, if we haven't seen an opening brace yet, but we + # already saw a closing parenthesis, then these are probably + # function arguments with extra "class" or "struct" keywords. + # Also pop these stack for these. + if not self.SeenOpenBrace(): + self.stack.pop() + else: # token == '}' + # Perform end of block checks and pop the stack. + if self.stack: + self.stack[-1].CheckEnd(filename, clean_lines, linenum, error) + self.stack.pop() + line = matched.group(2) + + def InnermostClass(self): + """Get class info on the top of the stack. + + Returns: + A _ClassInfo object if we are inside a class, or None otherwise. + """ + for i in range(len(self.stack), 0, -1): + classinfo = self.stack[i - 1] + if isinstance(classinfo, _ClassInfo): + return classinfo + return None + + def CheckClassFinished(self, filename, error): """Checks that all classes have been completely parsed. Call this when all lines in a file have been processed. @@ -1313,17 +1744,18 @@ class _ClassState(object): filename: The name of the current file. error: The function to call with any errors found. """ - if self.classinfo_stack: - # Note: This test can result in false positives if #ifdef constructs - # get in the way of brace matching. See the testBuildClass test in - # cpplint_unittest.py for an example of this. - error(filename, self.classinfo_stack[0].linenum, 'build/class', 5, - 'Failed to find complete declaration of class %s' % - self.classinfo_stack[0].name) + # Note: This test can result in false positives if #ifdef constructs + # get in the way of brace matching. See the testBuildClass test in + # cpplint_unittest.py for an example of this. + for obj in self.stack: + if isinstance(obj, _ClassInfo): + error(filename, obj.starting_linenum, 'build/class', 5, + 'Failed to find complete declaration of class %s' % + obj.name) def CheckForNonStandardConstructs(filename, clean_lines, linenum, - class_state, error): + nesting_state, error): """Logs an error if we see certain non-ANSI constructs ignored by gcc-2. Complain about several constructs which gcc-2 accepts, but which are @@ -1336,8 +1768,6 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, - text after #endif is not allowed. - invalid inner-style forward declaration. - >? and <? operators, and their >?= and <?= cousins. - - classes with virtual methods need virtual destructors (compiler warning - available, but not turned on yet.) Additionally, check for constructor/destructor style violations and reference members, as it is very convenient to do so while checking for @@ -1347,8 +1777,8 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, filename: The name of the current file. clean_lines: A CleansedLines instance containing the file. linenum: The number of the line to check. - class_state: A _ClassState instance which maintains information about - the current stack of nested class declarations being parsed. + nesting_state: A _NestingState instance which maintains information about + the current stack of nested blocks being parsed. error: A callable to which errors are reported, which takes 4 arguments: filename, line number, error level, and message """ @@ -1377,7 +1807,7 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, if Search(r'\b(const|volatile|void|char|short|int|long' r'|float|double|signed|unsigned' r'|schar|u?int8|u?int16|u?int32|u?int64)' - r'\s+(auto|register|static|extern|typedef)\b', + r'\s+(register|static|extern|typedef)\b', line): error(filename, linenum, 'build/storage_class', 5, 'Storage class (static, extern, typedef, etc) should be first.') @@ -1407,45 +1837,13 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, 'const string& members are dangerous. It is much better to use ' 'alternatives, such as pointers or simple constants.') - # Track class entry and exit, and attempt to find cases within the - # class declaration that don't meet the C++ style - # guidelines. Tracking is very dependent on the code matching Google - # style guidelines, but it seems to perform well enough in testing - # to be a worthwhile addition to the checks. - classinfo_stack = class_state.classinfo_stack - # Look for a class declaration. The regexp accounts for decorated classes - # such as in: - # class LOCKABLE API Object { - # }; - class_decl_match = Match( - r'\s*(template\s*<[\w\s<>,:]*>\s*)?' - '(class|struct)\s+([A-Z_]+\s+)*(\w+(::\w+)*)', line) - if class_decl_match: - classinfo_stack.append(_ClassInfo( - class_decl_match.group(4), clean_lines, linenum)) - - # Everything else in this function uses the top of the stack if it's - # not empty. - if not classinfo_stack: + # Everything else in this function operates on class declarations. + # Return early if the top of the nesting stack is not a class, or if + # the class head is not completed yet. + classinfo = nesting_state.InnermostClass() + if not classinfo or not classinfo.seen_open_brace: return - classinfo = classinfo_stack[-1] - - # If the opening brace hasn't been seen look for it and also - # parent class declarations. - if not classinfo.seen_open_brace: - # If the line has a ';' in it, assume it's a forward declaration or - # a single-line class declaration, which we won't process. - if line.find(';') != -1: - classinfo_stack.pop() - return - classinfo.seen_open_brace = (line.find('{') != -1) - # Look for a bare ':' - if Search('(^|[^:]):($|[^:])', line): - classinfo.is_derived = True - if not classinfo.seen_open_brace: - return # Everything else in this function is for after open brace - # The class may have been declared with namespace or classname qualifiers. # The constructor and destructor will not have those qualifiers. base_classname = classinfo.name.split('::')[-1] @@ -1462,35 +1860,6 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, error(filename, linenum, 'runtime/explicit', 5, 'Single-argument constructors should be marked explicit.') - # Look for methods declared virtual. - if Search(r'\bvirtual\b', line): - classinfo.virtual_method_linenumber = linenum - # Only look for a destructor declaration on the same line. It would - # be extremely unlikely for the destructor declaration to occupy - # more than one line. - if Search(r'~%s\s*\(' % base_classname, line): - classinfo.has_virtual_destructor = True - - # Look for class end. - brace_depth = classinfo.brace_depth - brace_depth = brace_depth + line.count('{') - line.count('}') - if brace_depth <= 0: - classinfo = classinfo_stack.pop() - # Try to detect missing virtual destructor declarations. - # For now, only warn if a non-derived class with virtual methods lacks - # a virtual destructor. This is to make it less likely that people will - # declare derived virtual destructors without declaring the base - # destructor virtual. - if ((classinfo.virtual_method_linenumber is not None) and - (not classinfo.has_virtual_destructor) and - (not classinfo.is_derived)): # Only warn for base classes - error(filename, classinfo.linenum, 'runtime/virtual', 4, - 'The class %s probably needs a virtual destructor due to ' - 'having virtual method(s), one declared at line %d.' - % (classinfo.name, classinfo.virtual_method_linenumber)) - else: - classinfo.brace_depth = brace_depth - def CheckSpacingForFunctionCall(filename, line, linenum, error): """Checks for the correctness of various spacing around function calls. @@ -1545,7 +1914,8 @@ def CheckSpacingForFunctionCall(filename, line, linenum, error): error(filename, linenum, 'whitespace/parens', 2, 'Extra space after (') if (Search(r'\w\s+\(', fncall) and - not Search(r'#\s*define|typedef', fncall)): + not Search(r'#\s*define|typedef', fncall) and + not Search(r'\w\s+\((\w+::)?\*\w+\)\(', fncall)): error(filename, linenum, 'whitespace/parens', 4, 'Extra space before ( in function call') # If the ) is followed only by a newline or a { + newline, assume it's @@ -1678,8 +2048,165 @@ def CheckComment(comment, filename, linenum, error): error(filename, linenum, 'whitespace/todo', 2, 'TODO(my_username) should be followed by a space') +def CheckAccess(filename, clean_lines, linenum, nesting_state, error): + """Checks for improper use of DISALLOW* macros. + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + nesting_state: A _NestingState instance which maintains information about + the current stack of nested blocks being parsed. + error: The function to call with any errors found. + """ + line = clean_lines.elided[linenum] # get rid of comments and strings + + matched = Match((r'\s*(DISALLOW_COPY_AND_ASSIGN|' + r'DISALLOW_EVIL_CONSTRUCTORS|' + r'DISALLOW_IMPLICIT_CONSTRUCTORS)'), line) + if not matched: + return + if nesting_state.stack and isinstance(nesting_state.stack[-1], _ClassInfo): + if nesting_state.stack[-1].access != 'private': + error(filename, linenum, 'readability/constructors', 3, + '%s must be in the private: section' % matched.group(1)) + + else: + # Found DISALLOW* macro outside a class declaration, or perhaps it + # was used inside a function when it should have been part of the + # class declaration. We could issue a warning here, but it + # probably resulted in a compiler error already. + pass + + +def FindNextMatchingAngleBracket(clean_lines, linenum, init_suffix): + """Find the corresponding > to close a template. + + Args: + clean_lines: A CleansedLines instance containing the file. + linenum: Current line number. + init_suffix: Remainder of the current line after the initial <. + + Returns: + True if a matching bracket exists. + """ + line = init_suffix + nesting_stack = ['<'] + while True: + # Find the next operator that can tell us whether < is used as an + # opening bracket or as a less-than operator. We only want to + # warn on the latter case. + # + # We could also check all other operators and terminate the search + # early, e.g. if we got something like this "a<b+c", the "<" is + # most likely a less-than operator, but then we will get false + # positives for default arguments (e.g. http://go/prccd) and + # other template expressions (e.g. http://go/oxcjq). + match = Search(r'^[^<>(),;\[\]]*([<>(),;\[\]])(.*)$', line) + if match: + # Found an operator, update nesting stack + operator = match.group(1) + line = match.group(2) + + if nesting_stack[-1] == '<': + # Expecting closing angle bracket + if operator in ('<', '(', '['): + nesting_stack.append(operator) + elif operator == '>': + nesting_stack.pop() + if not nesting_stack: + # Found matching angle bracket + return True + elif operator == ',': + # Got a comma after a bracket, this is most likely a template + # argument. We have not seen a closing angle bracket yet, but + # it's probably a few lines later if we look for it, so just + # return early here. + return True + else: + # Got some other operator. + return False + + else: + # Expecting closing parenthesis or closing bracket + if operator in ('<', '(', '['): + nesting_stack.append(operator) + elif operator in (')', ']'): + # We don't bother checking for matching () or []. If we got + # something like (] or [), it would have been a syntax error. + nesting_stack.pop() + + else: + # Scan the next line + linenum += 1 + if linenum >= len(clean_lines.elided): + break + line = clean_lines.elided[linenum] + + # Exhausted all remaining lines and still no matching angle bracket. + # Most likely the input was incomplete, otherwise we should have + # seen a semicolon and returned early. + return True + + +def FindPreviousMatchingAngleBracket(clean_lines, linenum, init_prefix): + """Find the corresponding < that started a template. + + Args: + clean_lines: A CleansedLines instance containing the file. + linenum: Current line number. + init_prefix: Part of the current line before the initial >. + + Returns: + True if a matching bracket exists. + """ + line = init_prefix + nesting_stack = ['>'] + while True: + # Find the previous operator + match = Search(r'^(.*)([<>(),;\[\]])[^<>(),;\[\]]*$', line) + if match: + # Found an operator, update nesting stack + operator = match.group(2) + line = match.group(1) + + if nesting_stack[-1] == '>': + # Expecting opening angle bracket + if operator in ('>', ')', ']'): + nesting_stack.append(operator) + elif operator == '<': + nesting_stack.pop() + if not nesting_stack: + # Found matching angle bracket + return True + elif operator == ',': + # Got a comma before a bracket, this is most likely a + # template argument. The opening angle bracket is probably + # there if we look for it, so just return early here. + return True + else: + # Got some other operator. + return False + + else: + # Expecting opening parenthesis or opening bracket + if operator in ('>', ')', ']'): + nesting_stack.append(operator) + elif operator in ('(', '['): + nesting_stack.pop() -def CheckSpacing(filename, clean_lines, linenum, error): + else: + # Scan the previous line + linenum -= 1 + if linenum < 0: + break + line = clean_lines.elided[linenum] + + # Exhausted all earlier lines and still no matching angle bracket. + return False + + +def CheckSpacing(filename, clean_lines, linenum, nesting_state, error): """Checks for the correctness of various spacing issues in the code. Things we check for: spaces around operators, spaces after @@ -1692,6 +2219,8 @@ def CheckSpacing(filename, clean_lines, linenum, error): filename: The name of the current file. clean_lines: A CleansedLines instance containing the file. linenum: The number of the line to check. + nesting_state: A _NestingState instance which maintains information about + the current stack of nested blocks being parsed. error: The function to call with any errors found. """ @@ -1701,7 +2230,16 @@ def CheckSpacing(filename, clean_lines, linenum, error): # Before nixing comments, check if the line is blank for no good # reason. This includes the first line after a block is opened, and # blank lines at the end of a function (ie, right before a line like '}' - if IsBlankLine(line): + # + # Skip all the blank line checks if we are immediately inside a + # namespace body. In other words, don't issue blank line warnings + # for this block: + # namespace { + # + # } + # + # A warning about missing end of namespace comments will be issued instead. + if IsBlankLine(line) and not nesting_state.InNamespaceBody(): elided = clean_lines.elided prev_line = elided[linenum - 1] prevbrace = prev_line.rfind('{') @@ -1709,8 +2247,7 @@ def CheckSpacing(filename, clean_lines, linenum, error): # both start with alnums and are indented the same amount. # This ignores whitespace at the start of a namespace block # because those are not usually indented. - if (prevbrace != -1 and prev_line[prevbrace:].find('}') == -1 - and prev_line[:prevbrace].find('namespace') == -1): + if prevbrace != -1 and prev_line[prevbrace:].find('}') == -1: # OK, we have a blank line at the start of a code block. Before we # complain, we check if it is an exception to the rule: The previous # non-empty line has the parameters of a function header that are indented @@ -1742,12 +2279,7 @@ def CheckSpacing(filename, clean_lines, linenum, error): if not exception: error(filename, linenum, 'whitespace/blank_line', 2, 'Blank line at the start of a code block. Is this needed?') - # This doesn't ignore whitespace at the end of a namespace block - # because that is too hard without pairing open/close braces; - # however, a special exception is made for namespace closing - # brackets which have a comment containing "namespace". - # - # Also, ignore blank lines at the end of a block in a long if-else + # Ignore blank lines at the end of a block in a long if-else # chain, like this: # if (condition1) { # // Something followed by a blank line @@ -1759,7 +2291,6 @@ def CheckSpacing(filename, clean_lines, linenum, error): next_line = raw[linenum + 1] if (next_line and Match(r'\s*}', next_line) - and next_line.find('namespace') == -1 and next_line.find('} else ') == -1): error(filename, linenum, 'whitespace/blank_line', 3, 'Blank line at the end of a code block. Is this needed?') @@ -1820,26 +2351,59 @@ def CheckSpacing(filename, clean_lines, linenum, error): # though, so we punt on this one for now. TODO. # You should always have whitespace around binary operators. - # Alas, we can't test < or > because they're legitimately used sans spaces - # (a->b, vector<int> a). The only time we can tell is a < with no >, and - # only if it's not template params list spilling into the next line. + # + # Check <= and >= first to avoid false positives with < and >, then + # check non-include lines for spacing around < and >. match = Search(r'[^<>=!\s](==|!=|<=|>=)[^<>=!\s]', line) - if not match: - # Note that while it seems that the '<[^<]*' term in the following - # regexp could be simplified to '<.*', which would indeed match - # the same class of strings, the [^<] means that searching for the - # regexp takes linear rather than quadratic time. - if not Search(r'<[^<]*,\s*$', line): # template params spill - match = Search(r'[^<>=!\s](<)[^<>=!\s]([^>]|->)*$', line) if match: error(filename, linenum, 'whitespace/operators', 3, 'Missing spaces around %s' % match.group(1)) - # We allow no-spaces around << and >> when used like this: 10<<20, but + # We allow no-spaces around << when used like this: 10<<20, but # not otherwise (particularly, not when used as streams) - match = Search(r'[^0-9\s](<<|>>)[^0-9\s]', line) + match = Search(r'(\S)(?:L|UL|ULL|l|ul|ull)?<<(\S)', line) + if match and not (match.group(1).isdigit() and match.group(2).isdigit()): + error(filename, linenum, 'whitespace/operators', 3, + 'Missing spaces around <<') + elif not Match(r'#.*include', line): + # Avoid false positives on -> + reduced_line = line.replace('->', '') + + # Look for < that is not surrounded by spaces. This is only + # triggered if both sides are missing spaces, even though + # technically should should flag if at least one side is missing a + # space. This is done to avoid some false positives with shifts. + match = Search(r'[^\s<]<([^\s=<].*)', reduced_line) + if (match and + not FindNextMatchingAngleBracket(clean_lines, linenum, match.group(1))): + error(filename, linenum, 'whitespace/operators', 3, + 'Missing spaces around <') + + # Look for > that is not surrounded by spaces. Similar to the + # above, we only trigger if both sides are missing spaces to avoid + # false positives with shifts. + match = Search(r'^(.*[^\s>])>[^\s=>]', reduced_line) + if (match and + not FindPreviousMatchingAngleBracket(clean_lines, linenum, + match.group(1))): + error(filename, linenum, 'whitespace/operators', 3, + 'Missing spaces around >') + + # We allow no-spaces around >> for almost anything. This is because + # C++11 allows ">>" to close nested templates, which accounts for + # most cases when ">>" is not followed by a space. + # + # We still warn on ">>" followed by alpha character, because that is + # likely due to ">>" being used for right shifts, e.g.: + # value >> alpha + # + # When ">>" is used to close templates, the alphanumeric letter that + # follows would be part of an identifier, and there should still be + # a space separating the template type and the identifier. + # type<type<type>> alpha + match = Search(r'>>[a-zA-Z_]', line) if match: error(filename, linenum, 'whitespace/operators', 3, - 'Missing spaces around %s' % match.group(1)) + 'Missing spaces around >>') # There shouldn't be space around unary operators match = Search(r'(!\s|~\s|[\s]--[\s;]|[\s]\+\+[\s;])', line) @@ -1913,16 +2477,23 @@ def CheckSpacing(filename, clean_lines, linenum, error): # the semicolon there. if Search(r':\s*;\s*$', line): error(filename, linenum, 'whitespace/semicolon', 5, - 'Semicolon defining empty statement. Use { } instead.') + 'Semicolon defining empty statement. Use {} instead.') elif Search(r'^\s*;\s*$', line): error(filename, linenum, 'whitespace/semicolon', 5, 'Line contains only semicolon. If this should be an empty statement, ' - 'use { } instead.') + 'use {} instead.') elif (Search(r'\s+;\s*$', line) and not Search(r'\bfor\b', line)): error(filename, linenum, 'whitespace/semicolon', 5, 'Extra space before last semicolon. If this should be an empty ' - 'statement, use { } instead.') + 'statement, use {} instead.') + + # In range-based for, we wanted spaces before and after the colon, but + # not around "::" tokens that might appear. + if (Search('for *\(.*[^:]:[^: ]', line) or + Search('for *\(.*[^: ]:[^:]', line)): + error(filename, linenum, 'whitespace/forcolon', 2, + 'Missing space around colon in range-based for loop') def CheckSectionSpacing(filename, clean_lines, class_info, linenum, error): @@ -1948,8 +2519,8 @@ def CheckSectionSpacing(filename, clean_lines, class_info, linenum, error): # # If we didn't find the end of the class, last_line would be zero, # and the check will be skipped by the first condition. - if (class_info.last_line - class_info.linenum <= 24 or - linenum <= class_info.linenum): + if (class_info.last_line - class_info.starting_linenum <= 24 or + linenum <= class_info.starting_linenum): return matched = Match(r'\s*(public|protected|private):', clean_lines.lines[linenum]) @@ -1960,15 +2531,18 @@ def CheckSectionSpacing(filename, clean_lines, class_info, linenum, error): # - We are at the beginning of the class. # - We are forward-declaring an inner class that is semantically # private, but needed to be public for implementation reasons. + # Also ignores cases where the previous line ends with a backslash as can be + # common when defining classes in C macros. prev_line = clean_lines.lines[linenum - 1] if (not IsBlankLine(prev_line) and - not Search(r'\b(class|struct)\b', prev_line)): + not Search(r'\b(class|struct)\b', prev_line) and + not Search(r'\\$', prev_line)): # Try a bit harder to find the beginning of the class. This is to # account for multi-line base-specifier lists, e.g.: # class Derived # : public Base { - end_class_head = class_info.linenum - for i in range(class_info.linenum, linenum): + end_class_head = class_info.starting_linenum + for i in range(class_info.starting_linenum, linenum): if Search(r'\{\s*$', clean_lines.lines[i]): end_class_head = i break @@ -2018,9 +2592,11 @@ def CheckBraces(filename, clean_lines, linenum, error): # which is commonly used to control the lifetime of # stack-allocated variables. We don't detect this perfectly: we # just don't complain if the last non-whitespace character on the - # previous non-blank line is ';', ':', '{', or '}'. + # previous non-blank line is ';', ':', '{', or '}', or if the previous + # line starts a preprocessor block. prevline = GetPreviousNonBlankLine(clean_lines, linenum)[0] - if not Search(r'[;:}{]\s*$', prevline): + if (not Search(r'[;:}{]\s*$', prevline) and + not Match(r'\s*#', prevline)): error(filename, linenum, 'whitespace/braces', 4, '{ should almost always be at the end of the previous line') @@ -2074,6 +2650,33 @@ def CheckBraces(filename, clean_lines, linenum, error): "You don't need a ; after a }") +def CheckEmptyLoopBody(filename, clean_lines, linenum, error): + """Loop for empty loop body with only a single semicolon. + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + error: The function to call with any errors found. + """ + + # Search for loop keywords at the beginning of the line. Because only + # whitespaces are allowed before the keywords, this will also ignore most + # do-while-loops, since those lines should start with closing brace. + line = clean_lines.elided[linenum] + if Match(r'\s*(for|while)\s*\(', line): + # Find the end of the conditional expression + (end_line, end_linenum, end_pos) = CloseExpression( + clean_lines, linenum, line.find('(')) + + # Output warning if what follows the condition expression is a semicolon. + # No warning for all other cases, including whitespace or newline, since we + # have a separate check for semicolons preceded by whitespace. + if end_pos >= 0 and Match(r';', end_line[end_pos:]): + error(filename, end_linenum, 'whitespace/empty_loop_body', 5, + 'Empty loop bodies should use {} or continue') + + def ReplaceableCheck(operator, macro, line): """Determine whether a basic CHECK can be replaced with a more specific one. @@ -2142,6 +2745,38 @@ def CheckCheck(filename, clean_lines, linenum, error): break +def CheckAltTokens(filename, clean_lines, linenum, error): + """Check alternative keywords being used in boolean expressions. + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + error: The function to call with any errors found. + """ + line = clean_lines.elided[linenum] + + # Avoid preprocessor lines + if Match(r'^\s*#', line): + return + + # Last ditch effort to avoid multi-line comments. This will not help + # if the comment started before the current line or ended after the + # current line, but it catches most of the false positives. At least, + # it provides a way to workaround this warning for people who use + # multi-line comments in preprocessor macros. + # + # TODO(unknown): remove this once cpplint has better support for + # multi-line comments. + if line.find('/*') >= 0 or line.find('*/') >= 0: + return + + for match in _ALT_TOKEN_REPLACEMENT_PATTERN.finditer(line): + error(filename, linenum, 'readability/alt_tokens', 2, + 'Use operator %s instead of %s' % ( + _ALT_TOKEN_REPLACEMENT[match.group(1)], match.group(1))) + + def GetLineWidth(line): """Determines the width of the line in column positions. @@ -2164,7 +2799,7 @@ def GetLineWidth(line): return len(line) -def CheckStyle(filename, clean_lines, linenum, file_extension, class_state, +def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, error): """Checks rules from the 'C++ style rules' section of cppguide.html. @@ -2177,6 +2812,8 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, class_state, clean_lines: A CleansedLines instance containing the file. linenum: The number of the line to check. file_extension: The extension (without the dot) of the filename. + nesting_state: A _NestingState instance which maintains information about + the current stack of nested blocks being parsed. error: The function to call with any errors found. """ @@ -2258,16 +2895,19 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, class_state, not ((cleansed_line.find('case ') != -1 or cleansed_line.find('default:') != -1) and cleansed_line.find('break;') != -1)): - error(filename, linenum, 'whitespace/newline', 4, + error(filename, linenum, 'whitespace/newline', 0, 'More than one command on the same line') # Some more style checks CheckBraces(filename, clean_lines, linenum, error) - CheckSpacing(filename, clean_lines, linenum, error) + CheckEmptyLoopBody(filename, clean_lines, linenum, error) + CheckAccess(filename, clean_lines, linenum, nesting_state, error) + CheckSpacing(filename, clean_lines, linenum, nesting_state, error) CheckCheck(filename, clean_lines, linenum, error) - if class_state and class_state.classinfo_stack: - CheckSectionSpacing(filename, clean_lines, - class_state.classinfo_stack[-1], linenum, error) + CheckAltTokens(filename, clean_lines, linenum, error) + classinfo = nesting_state.InnermostClass() + if classinfo: + CheckSectionSpacing(filename, clean_lines, classinfo, linenum, error) _RE_PATTERN_INCLUDE_NEW_STYLE = re.compile(r'#include +"[^/]+\.h"') @@ -2564,9 +3204,11 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, fnline))): # We allow non-const references in a few standard places, like functions - # called "swap()" or iostream operators like "<<" or ">>". + # called "swap()" or iostream operators like "<<" or ">>". We also filter + # out for loops, which lint otherwise mistakenly thinks are functions. if not Search( - r'(swap|Swap|operator[<>][<>])\s*\(\s*(?:[\w:]|<.*>)+\s*&', + r'(for|swap|Swap|operator[<>][<>])\s*\(\s*' + r'(?:(?:typename\s*)?[\w:]|<.*>)+\s*&', fnline): error(filename, linenum, 'runtime/references', 2, 'Is this a non-const reference? ' @@ -2578,7 +3220,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, # probably a member operator declaration or default constructor. match = Search( r'(\bnew\s+)?\b' # Grab 'new' operator, if it's there - r'(int|float|double|bool|char|u?int(8|16|32|64)_t)\([^)]', line) # TODO(enh): upstream change to handle all stdint types. + r'(int|float|double|bool|char|int32|uint32|int64|uint64)\([^)]', line) if match: # gMock methods are defined using some variant of MOCK_METHODx(name, type) # where type may be float(), int(string), etc. Without context they are @@ -2588,14 +3230,23 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, if (match.group(1) is None and # If new operator, then this isn't a cast not (Match(r'^\s*MOCK_(CONST_)?METHOD\d+(_T)?\(', line) or Match(r'^\s*MockCallback<.*>', line))): - error(filename, linenum, 'readability/casting', 4, - 'Using deprecated casting style. ' - 'Use static_cast<%s>(...) instead' % - match.group(2)) + # Try a bit harder to catch gmock lines: the only place where + # something looks like an old-style cast is where we declare the + # return type of the mocked method, and the only time when we + # are missing context is if MOCK_METHOD was split across + # multiple lines (for example http://go/hrfhr ), so we only need + # to check the previous line for MOCK_METHOD. + if (linenum == 0 or + not Match(r'^\s*MOCK_(CONST_)?METHOD\d+(_T)?\(\S+,\s*$', + clean_lines.elided[linenum - 1])): + error(filename, linenum, 'readability/casting', 4, + 'Using deprecated casting style. ' + 'Use static_cast<%s>(...) instead' % + match.group(2)) CheckCStyleCast(filename, linenum, line, clean_lines.raw_lines[linenum], 'static_cast', - r'\((int|float|double|bool|char|u?int(8|16|32|64))\)', error) # TODO(enh): upstream change to handle all stdint types. + r'\((int|float|double|bool|char|u?int(16|32|64))\)', error) # This doesn't catch all cases. Consider (const char * const)"hello". # @@ -2713,7 +3364,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, include_state, printf_args = _GetTextInside(line, r'(?i)\b(string)?printf\s*\(') if printf_args: match = Match(r'([\w.\->()]+)$', printf_args) - if match: + if match and match.group(1) != '__VA_ARGS__': function_name = re.search(r'\b((?:string)?printf)\s*\(', line, re.I).group(1) error(filename, linenum, 'runtime/printf', 4, @@ -2834,6 +3485,11 @@ def CheckCStyleCast(filename, linenum, line, raw_line, cast_type, pattern, 'Using sizeof(type). Use sizeof(varname) instead if possible') return True + # operator++(int) and operator--(int) + if (line[0:match.start(1) - 1].endswith(' operator++') or + line[0:match.start(1) - 1].endswith(' operator--')): + return False + remainder = line[match.end(0):] # The close paren is for function pointers as arguments to a function. @@ -3122,13 +3778,13 @@ def CheckMakePairUsesDeduction(filename, clean_lines, linenum, error): if match: error(filename, linenum, 'build/explicit_make_pair', 4, # 4 = high confidence - 'Omit template arguments from make_pair OR use pair directly OR' - ' if appropriate, construct a pair directly') + 'For C++11-compatibility, omit template arguments from make_pair' + ' OR use pair directly OR if appropriate, construct a pair directly') -def ProcessLine(filename, file_extension, - clean_lines, line, include_state, function_state, - class_state, error, extra_check_functions=[]): +def ProcessLine(filename, file_extension, clean_lines, line, + include_state, function_state, nesting_state, error, + extra_check_functions=[]): """Processes a single line in the file. Args: @@ -3139,8 +3795,8 @@ def ProcessLine(filename, file_extension, line: Number of line being processed. include_state: An _IncludeState instance in which the headers are inserted. function_state: A _FunctionState instance which counts function lines, etc. - class_state: A _ClassState instance which maintains information about - the current stack of nested class declarations being parsed. + nesting_state: A _NestingState instance which maintains information about + the current stack of nested blocks being parsed. error: A callable to which errors are reported, which takes 4 arguments: filename, line number, error level, and message extra_check_functions: An array of additional check functions that will be @@ -3149,13 +3805,16 @@ def ProcessLine(filename, file_extension, """ raw_lines = clean_lines.raw_lines ParseNolintSuppressions(filename, raw_lines[line], line, error) + nesting_state.Update(filename, clean_lines, line, error) + if nesting_state.stack and nesting_state.stack[-1].inline_asm != _NO_ASM: + return CheckForFunctionLengths(filename, clean_lines, line, function_state, error) CheckForMultilineCommentsAndStrings(filename, clean_lines, line, error) - CheckStyle(filename, clean_lines, line, file_extension, class_state, error) + CheckStyle(filename, clean_lines, line, file_extension, nesting_state, error) CheckLanguage(filename, clean_lines, line, file_extension, include_state, error) CheckForNonStandardConstructs(filename, clean_lines, line, - class_state, error) + nesting_state, error) CheckPosixThreading(filename, clean_lines, line, error) CheckInvalidIncrement(filename, clean_lines, line, error) CheckMakePairUsesDeduction(filename, clean_lines, line, error) @@ -3182,7 +3841,7 @@ def ProcessFileData(filename, file_extension, lines, error, include_state = _IncludeState() function_state = _FunctionState() - class_state = _ClassState() + nesting_state = _NestingState() ResetNolintSuppressions() @@ -3195,9 +3854,9 @@ def ProcessFileData(filename, file_extension, lines, error, clean_lines = CleansedLines(lines) for line in xrange(clean_lines.NumLines()): ProcessLine(filename, file_extension, clean_lines, line, - include_state, function_state, class_state, error, + include_state, function_state, nesting_state, error, extra_check_functions) - class_state.CheckFinished(filename, error) + nesting_state.CheckClassFinished(filename, error) CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error) @@ -3312,7 +3971,8 @@ def ParseArguments(args): (opts, filenames) = getopt.getopt(args, '', ['help', 'output=', 'verbose=', 'stdout', # TODO(enh): added --stdout 'counting=', - 'filter=']) + 'filter=', + 'root=']) except getopt.GetoptError: PrintUsage('Invalid arguments.') @@ -3328,8 +3988,8 @@ def ParseArguments(args): elif opt == '--stdout': # TODO(enh): added --stdout output_stream = sys.stdout # TODO(enh): added --stdout elif opt == '--output': - if not val in ('emacs', 'vs7'): - PrintUsage('The only allowed output formats are emacs and vs7.') + if not val in ('emacs', 'vs7', 'eclipse'): + PrintUsage('The only allowed output formats are emacs, vs7 and eclipse.') output_format = val elif opt == '--verbose': verbosity = int(val) @@ -3341,6 +4001,9 @@ def ParseArguments(args): if val not in ('total', 'toplevel', 'detailed'): PrintUsage('Valid counting options are total, toplevel, and detailed') counting_style = val + elif opt == '--root': + global _root + _root = val if not filenames: PrintUsage('No files were specified.') @@ -3349,7 +4012,6 @@ def ParseArguments(args): _SetVerboseLevel(verbosity) _SetFilters(filters) _SetCountingStyle(counting_style) - sys.stderr = output_stream # TODO(enh): added --stdout return filenames |