[lld] r208365 - [PECOFF] Split LocallyImportedSymbolFile into two classes.

Rui Ueyama ruiu at google.com
Tue May 20 10:43:13 PDT 2014


On Tue, May 20, 2014 at 8:06 AM, David Blaikie <dblaikie at gmail.com> wrote:

> Yep - we shouldn't be putting anonymous namespaces in headers,
> generally. If you have some elements of the header that aren't meant
> to be exposed to users, but need to be in the header for whatever
> reason, the usual approach is to put them in an "impl" or "private"
> namespace (you could look around in the LLVM project to see what the
> most common name/convention is there - I'm not sure off-hand)


That is true. The anonymous namespace in the header doesn't make sense.
I'll rename it.


>  On Tue, May 20, 2014 at 3:56 AM, Simon Atanasyan <simon at atanasyan.com>
> wrote:
> > Hi Rui,
> >
> > On Fri, May 9, 2014 at 2:30 AM, Rui Ueyama <ruiu at google.com> wrote:
> >> URL: http://llvm.org/viewvc/llvm-project?rev=208365&view=rev
> >
> > [...]
> >
> >> +class VirtualArchiveLibraryFile : public ArchiveLibraryFile {
> >> +public:
> >> +  VirtualArchiveLibraryFile(StringRef filename)
> >> +      : ArchiveLibraryFile(filename) {}
> >
> > Just curious, what intention is to put VirtualArchiveLibraryFile into
> > the anonymous namespace and later use it as a base class for globally
> > visible class LocallyImportedSymbolFile? By the way this triggers the
> > following gcc warning:
> >
> > [[
> > LinkerGeneratedSymbolFile.h:143:7: warning:
> > ‘lld::pecoff::LocallyImportedSymbolFile’ has a field
> > ‘lld::pecoff::LocallyImportedSymbolFile::<anonymous>’ whose type uses
> > the anonymous namespace ...
> > ]]
> >
> > [...]
> >
> >> -class LocallyImportedSymbolFile : public ArchiveLibraryFile {
> >> +class LocallyImportedSymbolFile : public VirtualArchiveLibraryFile {
> >
> > --
> > Simon
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140520/464260d6/attachment.html>


More information about the llvm-commits mailing list