[all-commits] [llvm/llvm-project] 0cb7e7: Make iteration over the DeclContext::lookup_result...

[Vassil Vassilev via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 0cb7e7ca0c864e052bf49978f3bcd667c9e16930
      https://github.com/llvm/llvm-project/commit/0cb7e7ca0c864e052bf49978f3bcd667c9e16930
  Author: Vassil Vassilev <v.g.vassilev at gmail.com>
  Date:   2021-03-17 (Wed, 17 Mar 2021)

  Changed paths:
    M clang-tools-extra/clangd/unittests/TestTU.cpp
    M clang/include/clang/AST/ASTContext.h
    M clang/include/clang/AST/CXXInheritance.h
    M clang/include/clang/AST/Decl.h
    M clang/include/clang/AST/DeclBase.h
    M clang/include/clang/AST/DeclContextInternals.h
    M clang/include/clang/Serialization/ASTWriter.h
    M clang/lib/ARCMigrate/ObjCMT.cpp
    M clang/lib/AST/ASTContext.cpp
    M clang/lib/AST/CXXInheritance.cpp
    M clang/lib/AST/Decl.cpp
    M clang/lib/AST/DeclBase.cpp
    M clang/lib/AST/ExternalASTMerger.cpp
    M clang/lib/AST/TypePrinter.cpp
    M clang/lib/CodeGen/CodeGenFunction.h
    M clang/lib/Sema/MultiplexExternalSemaSource.cpp
    M clang/lib/Sema/SemaDeclCXX.cpp
    M clang/lib/Sema/SemaLookup.cpp
    M clang/lib/Sema/SemaObjCProperty.cpp
    M clang/lib/Sema/SemaTemplateInstantiate.cpp
    M clang/lib/Serialization/ASTReader.cpp
    M clang/lib/Serialization/ASTWriterDecl.cpp
    M clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
    M clang/test/PCH/cxx-explicit-specifier.cpp
    M clang/tools/libclang/CXType.cpp
    M clang/unittests/AST/ASTImporterTest.cpp
    M lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
    M lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
    M lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
    M lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

  Log Message:
  -----------
  Make iteration over the DeclContext::lookup_result safe.

The idiom:
```
DeclContext::lookup_result R = DeclContext::lookup(Name);
for (auto *D : R) {...}
```

is not safe when in the loop body we trigger deserialization from an AST file.
The deserialization can insert new declarations in the StoredDeclsList whose
underlying type is a vector. When the vector decides to reallocate its storage
the pointer we hold becomes invalid.

This patch replaces a SmallVector with an singly-linked list. The current
approach stores a SmallVector<NamedDecl*, 4> which is around 8 pointers.
The linked list is 3, 5, or 7. We do better in terms of memory usage for small
cases (and worse in terms of locality -- the linked list entries won't be near
each other, but will be near their corresponding declarations, and we were going
to fetch those memory pages anyway). For larger cases: the vector uses a
doubling strategy for reallocation, so will generally be between half-full and
full. Let's say it's 75% full on average, so there's N * 4/3 + 4 pointers' worth
of space allocated currently and will be 2N pointers with the linked list. So we
break even when there are N=6 entries and slightly lose in terms of memory usage
after that. We suspect that's still a win on average.

Thanks to @rsmith!

Differential revision: https://reviews.llvm.org/D91524