[llvm] r231254 - Revert r231221, "Devirtualize ~parser<T> by making it protected in base classes and making derived classes final"

David Blaikie dblaikie at gmail.com
Wed Mar 4 10:34:18 PST 2015


On Wed, Mar 4, 2015 at 10:33 AM, Rafael EspĂ­ndola <
rafael.espindola at gmail.com> wrote:

> I have delta running. I will upload a reduced testcase in one sec.
>

No worries - if it's the same assertion text on the same file I'm happy
enough to assume it's the same thing.

Hand reducing the test case provided in PR22791 got me to this:

template <typename>
struct foo {
protected:
  ~foo() = default;
};

struct x : foo<int> {
  x() {}
};

template struct foo<int>;

I'll file a clang bug with this shortly & can workaround the issue by using
{} instead of "= default" in this particular context.


>
> On 4 March 2015 at 10:15, David Blaikie <dblaikie at gmail.com> wrote:
> > Your bug report doesn't actually mention the crash, but a related (I
> believe
> > duplicate) bug report did
> >
> > http://llvm.org/bugs/show_bug.cgi?id=22791
> >
> > clang::CodeGen::CodeGenFunction::EnterDtorCleanups(const
> > clang::CXXDestructorDecl *, clang::CXXDtorType): Assertion
> > `(!DD->isTrivial() || DD->hasAttr<DLLExportAttr>()) && "Should not emit
> dtor
> > epilogue for non-exported trivial dtor!"' failed.
> >
> > & yes, I believe my commit (that Takumi reverted - thanks!) did cause
> this -
> > looks like I tickled a clang bug. I'll work on a reproduction/reduction &
> > figure out how to workaround it so I can resubmit this change..
> >
> >
> > On Wed, Mar 4, 2015 at 8:39 AM, Rafael EspĂ­ndola
> > <rafael.espindola at gmail.com> wrote:
> >>
> >> Was the error the one in:
> >>
> >> http://llvm.org/bugs/show_bug.cgi?id=22789
> >>
> >> ?
> >>
> >> On 4 March 2015 at 08:24, NAKAMURA Takumi <geek4civic at gmail.com> wrote:
> >> > Author: chapuni
> >> > Date: Wed Mar  4 10:24:40 2015
> >> > New Revision: 231254
> >> >
> >> > URL: http://llvm.org/viewvc/llvm-project?rev=231254&view=rev
> >> > Log:
> >> > Revert r231221, "Devirtualize ~parser<T> by making it protected in
> base
> >> > classes and making derived classes final"
> >> >
> >> > It broke seflhosting.
> >> >
> >> > Modified:
> >> >     llvm/trunk/include/llvm/Support/CommandLine.h
> >> >
> >> > Modified: llvm/trunk/include/llvm/Support/CommandLine.h
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/CommandLine.h?rev=231254&r1=231253&r2=231254&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/include/llvm/Support/CommandLine.h (original)
> >> > +++ llvm/trunk/include/llvm/Support/CommandLine.h Wed Mar  4 10:24:40
> >> > 2015
> >> > @@ -714,6 +714,7 @@ class basic_parser_impl { // non-templat
> >> >  public:
> >> >    basic_parser_impl(Option &O) {}
> >> >
> >> > +  virtual ~basic_parser_impl() {}
> >> >
> >> >    enum ValueExpected getValueExpectedFlagDefault() const {
> >> >      return ValueRequired;
> >> > @@ -742,7 +743,6 @@ public:
> >> >    virtual void anchor();
> >> >
> >> >  protected:
> >> > -  ~basic_parser_impl() = default;
> >> >    // A helper for basic_parser::printOptionDiff.
> >> >    void printOptionName(const Option &O, size_t GlobalWidth) const;
> >> >  };
> >> > @@ -755,15 +755,12 @@ public:
> >> >    basic_parser(Option &O) : basic_parser_impl(O) {}
> >> >    typedef DataType parser_data_type;
> >> >    typedef OptionValue<DataType> OptVal;
> >> > -
> >> > -protected:
> >> > -  ~basic_parser() = default;
> >> >  };
> >> >
> >> >  //--------------------------------------------------
> >> >  // parser<bool>
> >> >  //
> >> > -template <> class parser<bool> final : public basic_parser<bool> {
> >> > +template <> class parser<bool> : public basic_parser<bool> {
> >> >  public:
> >> >    parser(Option &O) : basic_parser(O) {}
> >> >
> >> > @@ -790,8 +787,7 @@ EXTERN_TEMPLATE_INSTANTIATION(class basi
> >> >
> >> >  //--------------------------------------------------
> >> >  // parser<boolOrDefault>
> >> > -template <>
> >> > -class parser<boolOrDefault> final : public
> basic_parser<boolOrDefault>
> >> > {
> >> > +template <> class parser<boolOrDefault> : public
> >> > basic_parser<boolOrDefault> {
> >> >  public:
> >> >    parser(Option &O) : basic_parser(O) {}
> >> >
> >> > @@ -817,7 +813,7 @@ EXTERN_TEMPLATE_INSTANTIATION(class basi
> >> >  //--------------------------------------------------
> >> >  // parser<int>
> >> >  //
> >> > -template <> class parser<int> final : public basic_parser<int> {
> >> > +template <> class parser<int> : public basic_parser<int> {
> >> >  public:
> >> >    parser(Option &O) : basic_parser(O) {}
> >> >
> >> > @@ -839,7 +835,7 @@ EXTERN_TEMPLATE_INSTANTIATION(class basi
> >> >  //--------------------------------------------------
> >> >  // parser<unsigned>
> >> >  //
> >> > -template <> class parser<unsigned> final : public
> >> > basic_parser<unsigned> {
> >> > +template <> class parser<unsigned> : public basic_parser<unsigned> {
> >> >  public:
> >> >    parser(Option &O) : basic_parser(O) {}
> >> >
> >> > @@ -862,8 +858,7 @@ EXTERN_TEMPLATE_INSTANTIATION(class basi
> >> >  // parser<unsigned long long>
> >> >  //
> >> >  template <>
> >> > -class parser<unsigned long long> final
> >> > -    : public basic_parser<unsigned long long> {
> >> > +class parser<unsigned long long> : public basic_parser<unsigned long
> >> > long> {
> >> >  public:
> >> >    parser(Option &O) : basic_parser(O) {}
> >> >
> >> > @@ -886,7 +881,7 @@ EXTERN_TEMPLATE_INSTANTIATION(class basi
> >> >  //--------------------------------------------------
> >> >  // parser<double>
> >> >  //
> >> > -template <> class parser<double> final : public basic_parser<double>
> {
> >> > +template <> class parser<double> : public basic_parser<double> {
> >> >  public:
> >> >    parser(Option &O) : basic_parser(O) {}
> >> >
> >> > @@ -908,7 +903,7 @@ EXTERN_TEMPLATE_INSTANTIATION(class basi
> >> >  //--------------------------------------------------
> >> >  // parser<float>
> >> >  //
> >> > -template <> class parser<float> final : public basic_parser<float> {
> >> > +template <> class parser<float> : public basic_parser<float> {
> >> >  public:
> >> >    parser(Option &O) : basic_parser(O) {}
> >> >
> >> > @@ -930,7 +925,7 @@ EXTERN_TEMPLATE_INSTANTIATION(class basi
> >> >  //--------------------------------------------------
> >> >  // parser<std::string>
> >> >  //
> >> > -template <> class parser<std::string> final : public
> >> > basic_parser<std::string> {
> >> > +template <> class parser<std::string> : public
> >> > basic_parser<std::string> {
> >> >  public:
> >> >    parser(Option &O) : basic_parser(O) {}
> >> >
> >> > @@ -955,7 +950,7 @@ EXTERN_TEMPLATE_INSTANTIATION(class basi
> >> >  //--------------------------------------------------
> >> >  // parser<char>
> >> >  //
> >> > -template <> class parser<char> final : public basic_parser<char> {
> >> > +template <> class parser<char> : public basic_parser<char> {
> >> >  public:
> >> >    parser(Option &O) : basic_parser(O) {}
> >> >
> >> >
> >> >
> >> > _______________________________________________
> >> > 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
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150304/c8b59c68/attachment.html>


More information about the llvm-commits mailing list