[llvm-commits] [llvm] r76759 - in /llvm/trunk: include/llvm/Bitcode/LLVMBitCodes.h include/llvm/Constants.h include/llvm/MDNode.h include/llvm/Value.h lib/AsmParser/LLParser.cpp lib/AsmParser/LLParser.h lib/Bitcode/Reader/BitcodeReader.cpp lib/Bitcode/Reader/BitcodeReader.h lib/Bitcode/Writer/BitcodeWriter.cpp lib/Bitcode/Writer/ValueEnumerator.cpp lib/VMCore/AsmWriter.cpp lib/VMCore/Constants.cpp lib/VMCore/Value.cpp test/Feature/embeddedmetadata.ll test/Feature/mdnode.ll tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp

[Chris Lattner]

On Jul 22, 2009, at 10:43 AM, Devang Patel wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=76759&view=rev
> Log:
> Introduce MetadataBase, a base class for MDString and MDNode.
> Derive MDString directly from MetadataBase.
> Introduce new bitcode block to hold metadata.

Nice metadata shouldn't be "isa<Constant>".

> +++ llvm/trunk/include/llvm/MDNode.h Wed Jul 22 12:43:22 2009
> @@ -31,6 +31,65 @@
> namespace llvm {
>
> // 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> +// MetadataBase  - A base class for MDNode and MDString.
> +class MetadataBase : public Value {

Please add a significant doxygen comment here.

> +public:
> +  MetadataBase(const Type *Ty, unsigned scid)
> +    : Value(Ty, scid) {}

This shouldn't have a public ctor, it should be protected because it  
is an abstract class.

> +// 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> +/// MDString - a single uniqued string.
> +/// These are used to efficiently contain a byte sequence for  
> metadata.
> +///
> +class MDString : public MetadataBase {
> +  MDString(const MDString &);            // DO NOT IMPLEMENT
> +
> +  const char *StrBegin, *StrEnd;

Storing start+end is somewhat unconventional.  Why not start+length  
instead?

> +  friend class LLVMContextImpl;
> +
> +public:
> +  MDString(const char *begin, const char *end)
> +    : MetadataBase(Type::MetadataTy, Value::MDStringVal),
> +      StrBegin(begin), StrEnd(end) {}

MDStrings should be uniqued, right?  If so, this ctor should be private.

> +
> +  intptr_t size() const { return StrEnd - StrBegin; }

Is size() the best name for this?  Maybe length would be better?  It  
should return unsigned, not intptr_t.  'intptr_t' is an implementation  
detail.

-Chris