Re: [PATCH 1/2] Add unit test to aarch64 prologue analyzer

[Yao Qi <qiyaoltc at gmail dot com>]

On Tue, Nov 29, 2016 at 09:57:46AM -0500, Antoine Tremblay wrote:
> 
> Yao Qi writes:
> 
> > @@ -436,6 +461,89 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
> >    return start;
> >  }
> >  
> > +static CORE_ADDR
> > +aarch64_analyze_prologue (struct gdbarch *gdbarch,
> > +			  CORE_ADDR start, CORE_ADDR limit,
> > +			  struct aarch64_prologue_cache *cache)
> > +{
> > +  instruction_reader reader { };
> > +
> 
> Could we use the default constructor here? If it's kept.
> 

It uses the default constructor.  instruction_reader has the default
constructor.

+ public:
+  instruction_reader () = default;

Do you mean I need to remove it?

> > +  return aarch64_analyze_prologue (gdbarch, start, limit, cache,
> > +				   reader);
> > +}
> > +
> > +#if GDB_SELF_TEST
> > +
> > +namespace selftests {
> > +
> > +  /* Instruction reader from manually cooked instruction sequences.  */
> > +  class instruction_reader_test : public abstract_instruction_reader
> > +  {
> > +  public:
> > +    instruction_reader_test() = default ;
> 
> Very nit, but there's a space before ';'
> 

Oh, yes, it should be "instruction_reader_test () = default;"

> Also I wonder if we need to specify the default constructor explicitly ?
> Is there a rationale for it?
> 
> It's never used too, unless you apply my previous comment.

The instruction_reader_test default constructor is never used.  How
about using "= delete"?

    instruction_reader_test () = delete;
    instruction_reader_test (std::initializer_list<uint32_t> init)
      : insns{init} {}

-- 
Yao (齐尧)