[lld] r248873 - Remove `explicit` from constructors that take more than one parameter.

[Aaron Ballman via llvm-commits]

On Tue, Sep 29, 2015 at 10:23 PM, Rui Ueyama <ruiu at google.com> wrote:
> On Tue, Sep 29, 2015 at 7:20 PM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> On Tue, Sep 29, 2015 at 10:06 PM, Rui Ueyama via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>> > Author: ruiu
>> > Date: Tue Sep 29 21:06:17 2015
>> > New Revision: 248873
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=248873&view=rev
>> > Log:
>> > Remove `explicit` from constructors that take more than one parameter.
>>
>> Why? This still has effect for braced initialization, for instance.
>
>
> For consistency with other code in LLD. These classes or the other classes
> that don't have `explicit` needed to be changed.

I think it depends on the intent of the API, but my preference is to
mark the constructors as "explicit" if you don't want them to be
implicitly created, even if there is more than a single parameter to
the constructor. It's less likely to happen, but it captures the
intent clearly.

~Aaron

>
>>
>>
>> struct S {
>>   explicit S(int, float) {}
>> };
>>
>> S foo() {
>>   return { 1, 2 }; // ill-formed because of the explicit constructor
>> }
>>
>> int main() {
>>   S s = foo();
>> }
>>
>> ~Aaron
>>
>> >
>> > Modified:
>> >     lld/trunk/ELF/InputFiles.h
>> >     lld/trunk/ELF/Symbols.h
>> >
>> > Modified: lld/trunk/ELF/InputFiles.h
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputFiles.h?rev=248873&r1=248872&r2=248873&view=diff
>> >
>> > ==============================================================================
>> > --- lld/trunk/ELF/InputFiles.h (original)
>> > +++ lld/trunk/ELF/InputFiles.h Tue Sep 29 21:06:17 2015
>> > @@ -41,7 +41,7 @@ public:
>> >    StringRef getName() const { return MB.getBufferIdentifier(); }
>> >
>> >  protected:
>> > -  explicit InputFile(Kind K, MemoryBufferRef M) : MB(M), FileKind(K) {}
>> > +  InputFile(Kind K, MemoryBufferRef M) : MB(M), FileKind(K) {}
>> >    MemoryBufferRef MB;
>> >
>> >  private:
>> > @@ -52,7 +52,7 @@ enum ELFKind { ELF32LEKind, ELF32BEKind,
>> >
>> >  class ELFFileBase : public InputFile {
>> >  public:
>> > -  explicit ELFFileBase(Kind K, ELFKind EKind, MemoryBufferRef M)
>> > +  ELFFileBase(Kind K, ELFKind EKind, MemoryBufferRef M)
>> >        : InputFile(K, M), EKind(EKind) {}
>> >    static bool classof(const InputFile *F) {
>> >      Kind K = F->kind();
>> > @@ -71,7 +71,7 @@ protected:
>> >  // .o file.
>> >  class ObjectFileBase : public ELFFileBase {
>> >  public:
>> > -  explicit ObjectFileBase(ELFKind EKind, MemoryBufferRef M)
>> > +  ObjectFileBase(ELFKind EKind, MemoryBufferRef M)
>> >        : ELFFileBase(ObjectKind, EKind, M) {}
>> >    static bool classof(const InputFile *F) { return F->kind() ==
>> > ObjectKind; }
>> >
>> > @@ -185,7 +185,7 @@ private:
>> >  // .so file.
>> >  class SharedFileBase : public ELFFileBase {
>> >  public:
>> > -  explicit SharedFileBase(ELFKind EKind, MemoryBufferRef M)
>> > +  SharedFileBase(ELFKind EKind, MemoryBufferRef M)
>> >        : ELFFileBase(SharedKind, EKind, M) {}
>> >    static bool classof(const InputFile *F) { return F->kind() ==
>> > SharedKind; }
>> >  };
>> >
>> > Modified: lld/trunk/ELF/Symbols.h
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Symbols.h?rev=248873&r1=248872&r2=248873&view=diff
>> >
>> > ==============================================================================
>> > --- lld/trunk/ELF/Symbols.h (original)
>> > +++ lld/trunk/ELF/Symbols.h Tue Sep 29 21:06:17 2015
>> > @@ -149,7 +149,7 @@ protected:
>> >    typedef typename Base::Elf_Sym Elf_Sym;
>> >
>> >  public:
>> > -  explicit Defined(Kind K, StringRef N, const Elf_Sym &Sym)
>> > +  Defined(Kind K, StringRef N, const Elf_Sym &Sym)
>> >        : ELFSymbolBody<ELFT>(K, N, Sym) {}
>> >
>> >    static bool classof(const SymbolBody *S) { return S->isDefined(); }
>> > @@ -162,7 +162,7 @@ template <class ELFT> class DefinedAbsol
>> >  public:
>> >    static Elf_Sym IgnoreUndef;
>> >
>> > -  explicit DefinedAbsolute(StringRef N, const Elf_Sym &Sym)
>> > +  DefinedAbsolute(StringRef N, const Elf_Sym &Sym)
>> >        : Defined<ELFT>(Base::DefinedAbsoluteKind, N, Sym) {}
>> >
>> >    static bool classof(const SymbolBody *S) {
>> > @@ -180,7 +180,7 @@ template <class ELFT> class DefinedCommo
>> >  public:
>> >    typedef typename std::conditional<ELFT::Is64Bits, uint64_t,
>> > uint32_t>::type
>> >        uintX_t;
>> > -  explicit DefinedCommon(StringRef N, const Elf_Sym &Sym)
>> > +  DefinedCommon(StringRef N, const Elf_Sym &Sym)
>> >        : Defined<ELFT>(Base::DefinedCommonKind, N, Sym) {
>> >      MaxAlignment = Sym.st_value;
>> >    }
>> > @@ -203,8 +203,7 @@ template <class ELFT> class DefinedRegul
>> >    typedef typename Base::Elf_Sym Elf_Sym;
>> >
>> >  public:
>> > -  explicit DefinedRegular(StringRef N, const Elf_Sym &Sym,
>> > -                          InputSection<ELFT> &Section)
>> > +  DefinedRegular(StringRef N, const Elf_Sym &Sym, InputSection<ELFT>
>> > &Section)
>> >        : Defined<ELFT>(Base::DefinedRegularKind, N, Sym),
>> > Section(Section) {}
>> >
>> >    static bool classof(const SymbolBody *S) {
>> > @@ -219,8 +218,8 @@ template <class ELFT> class DefinedSynth
>> >
>> >  public:
>> >    typedef typename Base::Elf_Sym Elf_Sym;
>> > -  explicit DefinedSynthetic(StringRef N, const Elf_Sym &Sym,
>> > -                            OutputSection<ELFT> &Section)
>> > +  DefinedSynthetic(StringRef N, const Elf_Sym &Sym,
>> > +                   OutputSection<ELFT> &Section)
>> >        : Defined<ELFT>(Base::DefinedSyntheticKind, N, Sym),
>> > Section(Section) {}
>> >
>> >    static bool classof(const SymbolBody *S) {
>> > @@ -238,7 +237,7 @@ template <class ELFT> class Undefined :
>> >  public:
>> >    static Elf_Sym Synthetic;
>> >
>> > -  explicit Undefined(StringRef N, const Elf_Sym &Sym)
>> > +  Undefined(StringRef N, const Elf_Sym &Sym)
>> >        : ELFSymbolBody<ELFT>(Base::UndefinedKind, N, Sym) {}
>> >
>> >    static bool classof(const SymbolBody *S) {
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>