Skip to content

Commit 8d9c25b

Browse files
committed
Improve clarity of CS2::TableOf
Currently, the implementation of CS2::TableOf does a few of things that hinder the ability to reason about the code. 1) To increase the terse- ness of the code, it makes prolific use of preprocessor macro substi- tutes for class and function template parameters. 2) It uses inheri- tance rather than composition to bring in the implementation of its underlying storage. 3) When providing a function template parameter for initializing entires, it passes the parameter by non-const reference, preventing the use of r-values for initialization. The second point is particularly problematic, as it leads to scenarios like the following: ``` class Allocator; class TakesAnAllocator { TakesAnAllocator(Allocator a); } TableOf<TakesAnAllocator> myTable(someAllocator); myTable.addElement(myTable); ``` In the final line of this example, the user of TableOf takes advantage of TableOf's inheriting from its underlying container to initialize objects as if it were an Allocator. To solve 1), this change set expands the preprocessor macros, purging them from existence. To solve 2) this change set refactors TableOf to use composition to import the functionality of the underlying storage rather than inheritance, plus the required changes to the rest of the code-base to support this new model. Finally, to solve 3) we change the signature of both TableOf's AddEntry, as well as the constructor for BaseArrayOf::DerivedElement to take the initializer object by const reference. Furthermore, we update TR_BitVector's copy constructor and assignment operator such that they also take const references, to cause TR_RegisterCandidate's implicit copy constructor to use const referen- ces. Signed-off-by: Luc des Trois Maisons <lmaisons@ca.ibm.com>
1 parent 5457a70 commit 8d9c25b

10 files changed

+339
-131
lines changed

compiler/cs2/arrayof.h

+12-7
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ namespace CS2 {
6464
///
6565
// ------------------------------------------------------------------------
6666
template <class AElementType, class Allocator, size_t segmentBits = 8>
67-
class BaseArrayOf : private Allocator {
67+
struct BaseArrayOf : private Allocator {
6868
public:
6969

7070
BaseArrayOf (const Allocator &a = Allocator());
@@ -102,10 +102,12 @@ class BaseArrayOf : private Allocator {
102102
/// \param[in] s Size to grow the array.
103103
void GrowTo (size_t s);
104104

105+
/// \return Size in bytes of memory dynamically allocated for the array
106+
unsigned long DynamicMemoryUsage() const;
107+
105108
/// \return Size in bytes currently allocated for the array
106109
unsigned long MemoryUsage() const;
107110

108-
protected:
109111
/// \return Number of elements per segment.
110112
size_t ElementsPerSegment() const;
111113

@@ -132,7 +134,6 @@ class BaseArrayOf : private Allocator {
132134
return out;
133135
}
134136

135-
protected:
136137
class DerivedElement {
137138
public:
138139

@@ -142,7 +143,7 @@ class BaseArrayOf : private Allocator {
142143

143144
DerivedElement() : fElement() {}
144145
template <class Initializer>
145-
DerivedElement (Initializer &element) : fElement(element) {}
146+
DerivedElement (const Initializer &element) : fElement(element) {}
146147
~DerivedElement() {}
147148

148149
AElementType &Element() { return fElement; }
@@ -189,7 +190,8 @@ CS2_ARTEMP inline typename CS2_BASEARDECL::DerivedElement *CS2_BASEARDECL::Segme
189190
}
190191

191192
CS2_ARTEMP inline
192-
typename CS2_BASEARDECL::DerivedElement *CS2_BASEARDECL::DerivedElementAt (size_t elementIndex) const {
193+
typename CS2_BASEARDECL::DerivedElement *
194+
CS2_BASEARDECL::DerivedElementAt (size_t elementIndex) const {
193195
size_t segmentMapIndex = elementIndex >> segmentBits;
194196
CS2Assert (segmentMapIndex < fNumberOfSegments,
195197
("Index %lu does not exist", elementIndex));
@@ -385,16 +387,19 @@ CS2_ARTEMP inline void CS2_BASEARDECL::ShrinkTo (size_t newSize) {
385387
}
386388
}
387389

388-
CS2_ARTEMP inline unsigned long CS2_BASEARDECL::MemoryUsage() const {
390+
CS2_ARTEMP inline unsigned long CS2_BASEARDECL::DynamicMemoryUsage() const {
389391
unsigned long sizeInBytes;
390392

391393
sizeInBytes = fMaxSegments * sizeof(DerivedElement *);
392394
sizeInBytes += NumberOfElements() * sizeof(DerivedElement);
393-
sizeInBytes += sizeof(CS2_BASEARDECL);
394395

395396
return sizeInBytes;
396397
}
397398

399+
CS2_ARTEMP inline unsigned long CS2_BASEARDECL::MemoryUsage() const {
400+
return DynamicMemoryUsage() + sizeof(CS2_BASEARDECL);
401+
}
402+
398403

399404
template <class AElementType, class Allocator, size_t segmentBits = 8, class Initializer = AElementType>
400405
class ArrayOf : public CS2_BASEARDECL {

compiler/cs2/bitvectr.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class ABitVector : private Allocator {
6969
class BitRef;
7070
public:
7171

72-
ABitVector(const Allocator &a = Allocator()) :
72+
explicit ABitVector(const Allocator &a = Allocator()) :
7373
Allocator(a), fNumBits(0), fBitWords(NULL) {
7474
}
7575

0 commit comments

Comments
 (0)