bugGNU Octave - Bugs: bug #41178, clang: ov-classdef.h: constructing...

 
 

bug #41178: clang: ov-classdef.h: constructing a map of an incomplete type

Submitted by:  Michael C. Grant <mcgrant>
Submitted on:  Thu 09 Jan 2014 09:00:10 PM UTC  
 
Category: InterpreterSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Build Failure
Status: FixedAssigned to: None
Originator Name: Michael C. GrantOpen/Closed: Closed
Release: devOperating System: Any

Discussion locked!

 

(Jump to the original submission Jump to the original submission)

 Spam posted by shania
 Spam posted by magalie
Fri 11 Dec 2015 04:36:08 PM UTC, comment #30:

This change was so trivial that I decided to graft it to stable for the 4.0.1 release.

John W. Eaton <jwe>
Project Administrator
Sat 12 Sep 2015 09:35:44 PM UTC, comment #29:

Ok, finally pushed to the default branch:

http://hg.savannah.gnu.org/hgweb/octave/rev/a142b62527ef

Thanks for everyone's work and patience in resolving this one.

Mike Miller <mtmiller>
Project Administrator
Fri 21 Aug 2015 11:47:15 PM UTC, comment #28:

Mike M, the patch works for me on Mac OS X.

Ben Abbott <bpabbott>
Project Member
Fri 21 Aug 2015 10:01:30 PM UTC, comment #27:

My reading of the C++ standard is that (as I said in my very first post) "C++ requires that all template parameters in the standard library containers be complete types, so this is non-compliant."

The behavior when this condition is not met is undefined. Clang is choosing to be more strict than GCC, but the standard mandates neither approach.

Michael C. Grant <mcgrant>
Fri 21 Aug 2015 08:55:27 PM UTC, comment #26:

Otherwise, I have no problem with the patch, so I'd say go ahead and add a comment and push it.

John W. Eaton <jwe>
Project Administrator
Fri 21 Aug 2015 08:54:30 PM UTC, comment #25:

Is this a bug in clang or something that is not guaranteed to work but that GCC just happens to allow? In any case, it might be a good idea to put a comment above the function definition explaining why it is not defined in the header file even though it is apparently a trivial function. Otherwise, someone might have the urge to move it back there.

John W. Eaton <jwe>
Project Administrator
Fri 21 Aug 2015 05:27:24 PM UTC, comment #24:

Attached, ready to push. Works for me with clang 3.5 on Debian. Almost identical to the previously attached patch, but please test anyway replacing the earlier ov-classdef.patch.

(file #34691)

Mike Miller <mtmiller>
Project Administrator
Mon 17 Aug 2015 07:38:51 PM UTC, comment #23:

Mike,

When you have a changeset ready, I'll be quick to test it on Mac OS 10.10.

Ben Abbott <bpabbott>
Project Member
Mon 17 Aug 2015 04:03:49 PM UTC, comment #22:

I think the ov-classdef patch should definitely be applied, I planned to do this weeks ago but have been overworked. I will try to take care of it soon.

I've reopened bug #43298, waiting for the patch to be refined to apply properly only when needed for broken versions of llvm libc++.

Mike Miller <mtmiller>
Project Administrator
Mon 27 Jul 2015 12:43:25 PM UTC, comment #21:

I think that the clang3.5-fix patch originates from https://savannah.gnu.org/bugs/?43298. Sadly the decision there was to not apply it because it's considered as a clang 3.5 bug.

I guess the ov-patch should not be applied for Mac OS only but rather whenever somebody uses clang (or better: always).

Sebastian <sschoeps>
Mon 27 Jul 2015 12:45:37 AM UTC, comment #20:

Who should get credit for the clang3.5-fix.patch and include-fix.patch?

Mike (Miller) do you see any reason these two can't be applied to default?

Also, since we can't build Octave 4.0 on Mac OS X without the ov-classdef changes, can some pre-processor magic be used to only apply that patch to Mac OS X systems? Would that avoid breaking Octave's API on other OSs?

Ben Abbott <bpabbott>
Project Member
Thu 23 Jul 2015 08:45:42 PM UTC, comment #19:

>akira noda, do you want to be credited for this fix?


Yes,please.
Here is my full name and e-mail address "akira noda<you.akira.noda@gmail.com>"
Thank you.

Anonymous
Thu 23 Jul 2015 05:24:14 AM UTC, comment #18:

Dear Ben,

I use a modified homebrew script based on the work of Michael Grant (link posted already). I compile without MESA (see below).

I suggest to move the GUI discussion to the mailing list. However, before I like to mention that I tweaked my compile settings and I can run [https://www.dropbox.com/s/c61z7b11bb229q3/octave4_yosemite.png?dl=0 Octave 4.0.0 with GUI on Yosemite. Honestly, I don't know why it works since I did not fix the main thread issue...

Sebastian <sschoeps>
Thu 23 Jul 2015 03:23:25 AM UTC, comment #17:

The one change to ov-classdef.* fixes the build for me with clang 3.6, the equivalent of the attached ov-classdef.patch. I tested building with clang 3.6 on Debian both with gcc's libstdc++ and with llvm's libc++, both build successfully.

So I think this is suitable for applying to default, probably not the stable branch since it changes the ov-classdef.h header file.

akira noda, do you want to be credited for this fix? We typically give attribution to any contributor who provides a patch for Octave with full name and email address.

Mike Miller <mtmiller>
Project Administrator
Thu 23 Jul 2015 12:43:17 AM UTC, comment #16:

Sebastian,

I tried the three patches you attached. Below is the result. Perhaps my configure process is different?

My configure process ended with the below

Ben Abbott <bpabbott>
Project Member
Wed 22 Jul 2015 10:24:19 PM UTC, comment #15:

Hi, I'm the anonymoous.(akira noda )

I don't know much about other OS.
But I can run octave 4.0.0 (GUI and CLI) with fllowing ports
https://bugs.freebsd.org/bugzilla/attachment.cgi?id=159040

This zip file contains all patches for FreeBSD.
(Please see "files" directory.)

Anonymous
Wed 22 Jul 2015 01:35:51 PM UTC, comment #14:

I am also able to test this on Debian with clang 3.6 (which had the same error as seen on OSX).

Can the anonymous who submitted the diff in comment #11 provide a name and email for attribution assuming this patch is suitable and fixes the error for everyone?

Mike Miller <mtmiller>
Project Administrator
Wed 22 Jul 2015 01:12:39 PM UTC, comment #13:

If someone can attach a complete patch (or set of patches), I can handle the initial testing and push the result to the default branch (assuming I'm successful with building on Yosemite).

The GUI problem is due to another problem. On Mac OS X it is necessary that the gui thread is the main thread. When running the gui, the fltk and qt toolkits are not part of the main thread. This problem cannot be addressed until building on Yosemite works.

Ben Abbott <bpabbott>
Project Member
Wed 22 Jul 2015 09:41:50 AM UTC, comment #12:

Thanks for sharing. I can confirm that this patch allows to compile octave 4.0 also on Yosemite with clang. Homebrew users may find a first formula at https://raw.githubusercontent.com/schoeps/homebrew-science/octave/octave.rb.

However, the GUI does not start but I think that this is a known problem (does it work on free FreeeBSD).

Sebastian <sschoeps>
Wed 22 Jul 2015 01:48:31 AM UTC, comment #11:

I can build with FreeBSD/clang3.4.1 with fllowing patch
(ofcause with other patch from ports)
Just move mark_as_constructed function to .cc file.

--- libinterp/octave-value/ov-classdef.h.orig 2015-05-23 23:21:53.000000000 +0900
+++ libinterp/octave-value/ov-classdef.h 2015-07-22 10:39:22.000000000 +0900
@@ -438,7 +438,7 @@

void mark_as_constructed (void) { ctor_list.clear (); }

- void mark_as_constructed (const cdef_class& cls) { ctor_list.erase (cls); }
+ void mark_as_constructed (const cdef_class& cls);

bool is_constructed (void) const { return ctor_list.empty (); }

--- libinterp/octave-value/ov-classdef.cc.orig 2015-05-23 23:21:53.000000000 +0900
+++ libinterp/octave-value/ov-classdef.cc 2015-07-22 10:39:22.000000000 +0900
@@ -1937,6 +1937,11 @@
return true;
}

+void cdef_object_scalar::mark_as_constructed (const cdef_class& cls)
+{
+ ctor_list.erase (cls);
+}
+
handle_cdef_object::~handle_cdef_object (void)
{
#if DEBUG_TRACE

Anonymous
Sat 06 Jun 2015 11:57:33 PM UTC, comment #10:

I still see this same error under MacPorts with octave 4.0.0 release vs. Apple clang (Apple LLVM version 6.0 (clang-600.0.54) (based on LLVM 3.5svn)) and MacPorts clang-3.6 (clang version 3.6.0 (tags/RELEASE_360/final)).

This is a show stopper for the Mac platform, as no version of gcc can be used to build a stable gui port due to the C11++ library mismatch.

Marius Schamschula <mschamschula>
Sun 12 Apr 2015 04:11:07 AM UTC, comment #9:

I can confirm this bug is still present with clang 3.6 on Debian.

Mike Miller <mtmiller>
Project Administrator
Tue 03 Feb 2015 11:42:13 PM UTC, comment #8:

As octave 4.0 is coming near, I am in the process of building a octave-devel Portfile under MacPorts. As the current Portfile for octave 3.8.2 blacklists clang-3.5 and -3.6, I left this unchanged. So I tried building tip under OS X 10.9.5, with Xcode 6.1.1 using clang-3.4 and ran into the same issue.

Marius Schamschula <mschamschula>
Mon 19 Jan 2015 08:44:18 AM UTC, comment #7:

I tried to compile octave on OSX 10.10 with clang 3.5 yesterday. The bug seems to be still present. Has anyone looked at it in the mean time? Is there a fix?

Sebastian <sschoeps>
Sat 25 Jan 2014 04:39:58 AM UTC, comment #6:

Confirmed on Debian GNU/Linux with clang 3.3 and libc++, roughly the same error messages.

Mike Miller <mtmiller>
Project Administrator
Sat 11 Jan 2014 05:28:28 PM UTC, comment #5:

By the way I realized why I thought this was auto generated: because it's a new file not present in all the branches! Anyway, I'm focused on gui-release now, so this is not urgent from my perspective. Only those who need to compile the tip using clang will be affected by this.

Michael C. Grant <mcgrant>
Fri 10 Jan 2014 03:55:38 AM UTC, comment #4:

I'm sorry for the mistake. Thank you for looking at this!

Some references:
- StackOverflow: "Why C++ containers don't allow incomplete types?" (http://stackoverflow.com/questions/18672135/why-c-containers-dont-allow-incomplete-types)
- Standard Librarian: Containers of incomplete Types (http://stackoverflow.com/questions/18672135/why-c-containers-dont-allow-incomplete-types)
- Clang: incomplete types in templates (http://clang.llvm.org/compatibility.html#undep_incomplete)

Michael C. Grant <mcgrant>
Fri 10 Jan 2014 12:33:44 AM UTC, comment #3:

For the record, ov-classdef.h is not generated, it's manually hand-written.

If the construct is not supported by clang++ (although it works ok with g++ and MSVC), some code re-organization will be required. I'll try to look into it in the coming days.

Michael Goffioul <goffioul>
Project Member
Thu 09 Jan 2014 09:15:32 PM UTC, comment #2:

I should add that line 743 is also non-compliant, because it's declared within cdef_class itself. I know it sucks...

Michael C. Grant <mcgrant>
Thu 09 Jan 2014 09:06:16 PM UTC, comment #1:

FYI, here's the clang++ output for one example compile:

Michael C. Grant <mcgrant>
Thu 09 Jan 2014 09:00:10 PM UTC, original submission:

I've been struggling to compile the dev sources on Mac OSX with the latest Xcode, and it has proven quite frustrating. But I think I've figured out why: you've got some code in there that technically is not standards compliant, and clang++ is deciding to be particularly strict about it.

Specifically: on line 448 of libinterp/octave-value/ov-classdef.h, which looks to be autogenerated, you have this:

cdef_class is declared on line 611. But C++ requires that all template parameters in the standard library containers be complete types, so this is non-compliant.

The standard says that compiler behavior is undefined in this case: compilers are allowed to reject it completely, or they can compile it, their choice. Apparently clang is not making the choice I'd like here. Why it is more permissive other times I am frankly not sure.

Michael C. Grant <mcgrant>

 

Attached Files
file #34489:  clang3.5-fix.patch added by sschoeps (4KiB - application/octet-stream - patches (as applied in the homebrew formula))
file #34490:  ov-classdef.patch added by sschoeps (997B - application/octet-stream - patches (as applied in the homebrew formula))
file #34491:  include-fix.patch added by sschoeps (777B - application/octet-stream - patches (as applied in the homebrew formula))

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by shania (Posted a comment)
  • -unavailable- added by magalie (Posted a comment)
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by bpabbott (Posted a comment)
  • -unavailable- added by joetastic
  • -unavailable- added by bpabbott
  • -unavailable- added by mschamschula (Posted a comment)
  • -unavailable- added by sschoeps (Posted a comment)
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by goffioul (Posted a comment)
  • -unavailable- added by mcgrant (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 12 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 13 Feb 2017 07:42:35 PM UTCmtmillerDiscussion LockUnlocked=>Locked
    Sat 12 Sep 2015 09:35:44 PM UTCmtmillerStatusPatch Submitted=>Fixed
      Open/ClosedOpen=>Closed
    Fri 21 Aug 2015 05:27:24 PM UTCmtmillerAttached File-=>Added octave-clang-classdef.patch, #34691
    Thu 23 Jul 2015 03:23:25 AM UTCmtmillerStatusConfirmed=>Patch Submitted
    Wed 22 Jul 2015 02:56:16 PM UTCsschoepsAttached File-=>Added clang3.5-fix.patch, #34489
      Attached File-=>Added ov-classdef.patch, #34490
      Attached File-=>Added include-fix.patch, #34491
    Thu 18 Jun 2015 04:31:13 PM UTCjoetasticCarbon-Copy-=>Added joetastic
    Sun 12 Apr 2015 02:38:14 PM UTCmtmillerSummaryov-classdef.h: constructing a map of an incomplete type=>clang: ov-classdef.h: constructing a map of an incomplete type
    Mon 09 Feb 2015 02:40:24 AM UTCbpabbottCarbon-Copy-=>Added -unavailable-
    Sat 25 Jan 2014 04:39:58 AM UTCmtmillerStatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup1