llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: David Stone (davidstone) <details> <summary>Changes</summary> To make this change, we have to use `lookup` instead of `operator[]` on a map. They both return the same thing: a default constructed value. The difference is that `lookup` default constructs a value and then returns it, whereas `operator[]` default constructs a value, inserts it into the map, and then returns a reference to that. Given that we are using a by-value return, the only way this is different is if a later use of the map depends on a value being at that key. The map is a private variable of the class, so the only possible users are are other member functions. The only other use of the map that cares about the contents of the map is in `lookupInBases`, and it accesses the map with `operator[]`. This means that attempting to access the same element in this function will default construct the value before doing anything with it, which means it would do the exact thing it needs to do in the case where we are looking up a non-existent key, therefore no behavior has changed. In terms of performance, this would either be a win or neutral. The benefit is that in some cases, we can avoid a memory allocation just read the contents of a 32-bit `0`. If a call to `isAmbiguous` is always followed up with a call to `lookupInBases`, then we allocate the memory just a little bit later for no difference in performance. --- Full diff: https://github.com/llvm/llvm-project/pull/169944.diff 2 Files Affected: - (modified) clang/include/clang/AST/CXXInheritance.h (+1-1) - (modified) clang/lib/AST/CXXInheritance.cpp (+2-2) ``````````diff diff --git a/clang/include/clang/AST/CXXInheritance.h b/clang/include/clang/AST/CXXInheritance.h index e89326081a180..72d365bfbc1f3 100644 --- a/clang/include/clang/AST/CXXInheritance.h +++ b/clang/include/clang/AST/CXXInheritance.h @@ -192,7 +192,7 @@ class CXXBasePaths { /// Determine whether the path from the most-derived type to the /// given base type is ambiguous (i.e., it refers to multiple subobjects of /// the same base type). - bool isAmbiguous(CanQualType BaseType); + bool isAmbiguous(CanQualType BaseType) const; /// Whether we are finding multiple paths to detect ambiguities. bool isFindingAmbiguities() const { return FindAmbiguities; } diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp index 7a3e7ea4e5b8f..29f5916284ebb 100644 --- a/clang/lib/AST/CXXInheritance.cpp +++ b/clang/lib/AST/CXXInheritance.cpp @@ -34,9 +34,9 @@ using namespace clang; /// ambiguous, i.e., there are two or more paths that refer to /// different base class subobjects of the same type. BaseType must be /// an unqualified, canonical class type. -bool CXXBasePaths::isAmbiguous(CanQualType BaseType) { +bool CXXBasePaths::isAmbiguous(CanQualType BaseType) const { BaseType = BaseType.getUnqualifiedType(); - IsVirtBaseAndNumberNonVirtBases Subobjects = ClassSubobjects[BaseType]; + IsVirtBaseAndNumberNonVirtBases Subobjects = ClassSubobjects.lookup(BaseType); return Subobjects.NumberOfNonVirtBases + (Subobjects.IsVirtBase ? 1 : 0) > 1; } `````````` </details> https://github.com/llvm/llvm-project/pull/169944 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
