[PATCH] D45467: COFF: Friendlier undefined symbol errors.

[Zachary Turner via llvm-commits]

Is there a particular reason to use a pointer? Inside the function it isn’t
null checked so we’re already assuming non null.
On Mon, Apr 16, 2018 at 7:49 PM Peter Collingbourne via Phabricator <
reviews at reviews.llvm.org> wrote:

> pcc added inline comments.
>
>
> ================
> Comment at: lld/COFF/Chunks.h:195-197
> +  ArrayRef<coff_relocation> relocs() const {
> +    return {Relocs.begin(), Relocs.end()};
> +  }
> ----------------
> zturner wrote:
> > ruiu wrote:
> > > If we can always do this, should we store Relocs as an ArrayRef
> instead of iterator_range in the first place?
> > Alternatively, why not just return an `iterator_range`?
> Switching to ArrayRef made the code a little simpler and was done in
> D45714.
>
>
> ================
> Comment at: lld/COFF/PDB.cpp:1210
> +
> +    for (const DebugSubsectionRecord &SS : Subsections) {
> +      switch (SS.kind()) {
> ----------------
> zturner wrote:
> > Don't we already iterate the subsection list for other reasons in this
> file?  Would it be worth piggybacking off of that to save the relevant info
> from the various subsections into each `SectionChunk` so that we can just
> do something like `DbgC->Lines`
> I don't think that would work. We read CV subsections elsewhere to create
> a PDB, but that needs to happen after we issue undefined symbol errors
> because it requires section contents to be relocated, which requires all
> sections to be laid out and all symbols to be defined. But if we're issuing
> an undefined symbol error then obviously not all symbols will be defined.
>
>
> ================
> Comment at: lld/COFF/PDB.h:33-34
> +
> +std::pair<llvm::StringRef, uint32_t> getFileLine(SectionChunk *C,
> +                                                 uint32_t Addr);
>  }
> ----------------
> zturner wrote:
> > Can you take this argument by `const&` instead of by non-const pointer?
> Const seems fine but we usually pass pointers around in this part of the
> code for things like chunks.
>
>
> https://reviews.llvm.org/D45467
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180417/8205a01e/attachment.html>