Hi Reid. Nice to hear from you.
On 2007-12-08, at 14:43, Reid Spencer wrote:
> Hi Gordon,
>
> Nice addition.
Thanks.
> Some minor feedback for you below ...
>
> On Sat, 2007-12-08 at 17:07 +0000, Gordon Henriksen wrote:
>
>> + struct PooledString {
>> + StringPool *Pool; ///< So the string can remove itself.
>> + unsigned Refcount; ///< Number of referencing
>> PooledStringPtrs.
>> +
>> + public:
>> + PooledString() : Pool(0), Refcount(0) { }
>> + };
>
> Since you have added doxygen comments for the structure's data
> members,
> why not document the structure itself as well? This will lead to less
> confusing documentation.
Okay.
>> +
>> + friend class PooledStringPtr;
>> +
>> + typedef StringMap<PooledString> table_t;
>> + typedef StringMapEntry<PooledString> entry_t;
>> + table_t InternTable;
>> +
>> + public:
>> + StringPool();
>> + ~StringPool();
>> +
>> + PooledStringPtr intern(const char *Begin, const char *End);
>> + inline PooledStringPtr intern(const char *Str);
>
> These two methods are primary interfaces to the StringPool, aren't
> they?
> Shouldn't they be documented with doxygen comments?
Okay.
>> + PooledStringPtr StringPool::intern(const char *Str) {
>> + return intern(Str, Str + strlen(Str));
>
> Maybe use strnlen(3)
That function is sufficiently nonstandard to not exist on Darwin.
> here to guard against Str not being null terminated ?
This is a convenience specifically for null-terminated strings, so I'm
not sure how defensive programming here is useful.
— Gordon
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
