[clang][NFC] Declare CXXBasePaths::isAmbiguous as const

[davidstone]

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.

[llvmbot]

@llvm/pr-subscribers-clang

Author: David Stone (davidstone)

Changes

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 --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; }