[llvm] r234200 - DebugInfo: Add MDTypeRefArray, to replace DITypeArray
Duncan P. N. Exon Smith
dexonsmith at apple.com
Tue Apr 7 09:35:37 PDT 2015
> On 2015-Apr-07, at 09:04, Tamas Berghammer <tberghammer at google.com> wrote:
>
> No problem, thanks for sending me the commit id fixed the issue. It is working now but I would still consider to change the signature of MDTypeRefArray::operator[] to take an std::ptrdiff_t if you don't see any issue what can be caused by it because that one is the standard argument for operator[] and it will eliminate any similar problem in the future.
>
I'm working on a different fix that removes the implicit conversion
to `MDTuple*()`. We use `unsigned` all the time as indexes, so
changing the index to `std::ptrdiff_t` will give ambiguities for all
the other callers (i.e., that own't fix the underlying problem).
> On Tue, Apr 7, 2015 at 4:50 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>
> > On 2015-Apr-07, at 06:59, Tamas Berghammer <tberghammer at google.com> wrote:
> >
> > This patch cause the following compile error on 32 bit android targets:
> >
> > /mnt/ssd/ll/git/llvm/tools/clang/lib/CodeGen/CGDebugInfo.cpp: In member function 'llvm::DICompositeType clang::CodeGen::CGDebugInfo::getOrCreateInstanceMethodType(clang::QualType, const clang::FunctionProtoType*, llvm::DIFile)':
> > /mnt/ssd/ll/git/llvm/tools/clang/lib/CodeGen/CGDebugInfo.cpp:1047:22: error: ambiguous overload for 'operator[]' (operand types are 'llvm::DITypeArray {aka llvm::MDTypeRefArray}' and 'int')
> > Elts.push_back(Args[0]);
> > ^
> > /mnt/ssd/ll/git/llvm/tools/clang/lib/CodeGen/CGDebugInfo.cpp:1047:22: note: candidates are:
> > /mnt/ssd/ll/git/llvm/tools/clang/lib/CodeGen/CGDebugInfo.cpp:1047:22: note: operator[](llvm::MDTuple*, int) <built-in>
> > In file included from /mnt/ssd/ll/git/llvm/include/llvm/IR/DebugInfo.h:25:0,
> > from /mnt/ssd/ll/git/llvm/include/llvm/IR/DIBuilder.h:20,
> > from /mnt/ssd/ll/git/llvm/tools/clang/lib/CodeGen/CGDebugInfo.h:23,
> > from /mnt/ssd/ll/git/llvm/tools/clang/lib/CodeGen/CGDebugInfo.cpp:14:
> > /mnt/ssd/ll/git/llvm/include/llvm/IR/DebugInfoMetadata.h:106:13: note: llvm::MDTypeRef llvm::MDTypeRefArray::operator[](unsigned int) const
> > MDTypeRef operator[](unsigned I) const { return MDTypeRef(N->getOperand(I)); }
> >
> > I see two possible fix but I am not sure which one is better:
> > * Change the constant in CGDebugInfo.cpp:1047 to unsigned (0u)
> > * Change the argument of MDTypeRefArray::operator[] to std::ptrdiff_t (or something similar)
> >
> > I prefer the second option but I am not sure which one is correct so please check it and submit a fix accordingly.
> >
>
> Looks like Timur applied a bandaid for me in r234307 already.
>
> Sorry for the trouble!
>
> > Thanks,
> > Tamas
> >
> > On Mon, Apr 6, 2015 at 9:24 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> > On Mon, Apr 6, 2015 at 1:13 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> >
> > > On 2015-Apr-06, at 12:58, David Blaikie <dblaikie at gmail.com> wrote:
> > >
> > >
> > >
> > > On Mon, Apr 6, 2015 at 12:48 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> > > Author: dexonsmith
> > > Date: Mon Apr 6 14:48:50 2015
> > > New Revision: 234200
> > >
> > > URL: http://llvm.org/viewvc/llvm-project?rev=234200&view=rev
> > > Log:
> > > DebugInfo: Add MDTypeRefArray, to replace DITypeArray
> > >
> > > This array-like wrapper adapts `MDTuple` to have elements of `MDTypeRef`
> > > (whereas `MDTypeArray` has elements of `MDType`). This is necessary to
> > > migrate code using `DITypeArray`. The only use of this is
> > > `MDSubroutineType`'s `getTypeArray()` accessor.
> > >
> > > Modified:
> > > llvm/trunk/include/llvm/IR/DebugInfoMetadata.h
> > >
> > > Modified: llvm/trunk/include/llvm/IR/DebugInfoMetadata.h
> > > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfoMetadata.h?rev=234200&r1=234199&r2=234200&view=diff
> > > ==============================================================================
> > > --- llvm/trunk/include/llvm/IR/DebugInfoMetadata.h (original)
> > > +++ llvm/trunk/include/llvm/IR/DebugInfoMetadata.h Mon Apr 6 14:48:50 2015
> > > @@ -89,6 +89,42 @@ typedef TypedDebugNodeRef<DebugNode> Deb
> > > typedef TypedDebugNodeRef<MDScope> MDScopeRef;
> > > typedef TypedDebugNodeRef<MDType> MDTypeRef;
> > >
> > > +class MDTypeRefArray {
> > > + const MDTuple *N = nullptr;
> > > +
> > > +public:
> > > + MDTypeRefArray(const MDTuple *N) : N(N) {}
> > > + operator MDTuple *() const { return const_cast<MDTuple *>(N); }
> > > + MDTuple *operator->() const { return const_cast<MDTuple *>(N); }
> > > + MDTuple &operator*() const { return *const_cast<MDTuple *>(N); }
> > > +
> > > + unsigned size() const { return N->getNumOperands(); }
> > > + MDTypeRef operator[](unsigned I) const { return MDTypeRef(N->getOperand(I)); }
> > > +
> > > + class iterator : std::iterator<std::input_iterator_tag, MDTypeRef,
> > > + std::ptrdiff_t, void, MDTypeRef> {
> > > + MDNode::op_iterator I;
> > > +
> > > + public:
> > > + explicit iterator(MDNode::op_iterator I) : I(I) {}
> > > + MDTypeRef operator*() const { return MDTypeRef(*I); }
> > > + iterator &operator++() {
> > > + ++I;
> > > + return *this;
> > > + }
> > > + iterator operator++(int) {
> > > + iterator Temp(*this);
> > > + ++I;
> > > + return Temp;
> > > + }
> > > + bool operator==(const iterator &X) const { return I == X.I; }
> > > + bool operator!=(const iterator &X) const { return I != X.I; }
> > >
> > > FWIW, ideally op overloads that can be non-members should be (to allow the same conversions on the LHS and RHS) - these could be inline friend definitions (or one a friend, and one not - since it can be defined in terms of the other). Not a big deal & we violate this all over the place, and it's not immediately apparent that any conversions should be possible on either side - just good habits.
> >
> > I'll keep that in mind. Here there are no implicit conversions, so I'm
> > tempted to leave it as is (since this is the most compact/clear way of
> > writing the code).
> >
> > Unless you think this should *always* be the way we do it?
> >
> > That's my personal preference, yes - it just hasn't risen to the level of codifying it in the coding standards (like many C++ idioms, really).
> >
> > & off-hand I can't find a great/canonical reference on this style/approach. No doubt I read it in a book somewhere...
> >
> > Anyway, I'll leave it at "just a thought" rather than make any effort to mandate it, for now.
> >
> > - Dave
> >
> > If so, you
> > might propose an addition to the coding standards. FWIW, I don't have
> > a strong opinion one way or the other; happy to switch to "the one true
> > way" if we decide there is one.
> >
> > >
> > > + };
> > > +
> > > + iterator begin() const { return iterator(N->op_begin()); }
> > > + iterator end() const { return iterator(N->op_end()); }
> > > +};
> > > +
> > > /// \brief Tagged DWARF-like metadata node.
> > > ///
> > > /// A metadata node with a DWARF tag (i.e., a constant named \c DW_TAG_*,
> > > @@ -826,7 +862,7 @@ public:
> > >
> > > TempMDSubroutineType clone() const { return cloneImpl(); }
> > >
> > > - MDTuple *getTypeArray() const { return getElements(); }
> > > + MDTypeRefArray getTypeArray() const { return getElements(); }
> > > Metadata *getRawTypeArray() const { return getRawElements(); }
> > >
> > > static bool classof(const Metadata *MD) {
> > >
> > >
> > > _______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> >
>
>
More information about the llvm-commits
mailing list