Opened 6 years ago

Last modified 2 years ago

#56114 new defect

audacity: issues with C++11 in MemoryX.h

Reported by: mojca (Mojca Miklavec) Owned by: RJVB (René Bertin)
Priority: Normal Milestone:
Component: ports Version:
Keywords: Cc: cooljeanius (Eric Gallager)
Port: audacity

Description

On < 10.9:

In file included from BlockFile.cpp:43:
In file included from ./BlockFile.h:15:
./MemoryX.h:21:22: error: declaration conflicts with target of using declaration already in scope
inline long long int llrint(float __x) { return __builtin_llrintf(__x); }
                     ^
/opt/local/include/gcc/c++/cmath:1530:3: note: target of using declaration
  llrint(float __x)
  ^
/opt/local/include/gcc/c++/math.h:94:12: note: using declaration
using std::llrint;
           ^
In file included from BlockFile.cpp:43:
In file included from ./BlockFile.h:15:
./MemoryX.h:23:22: error: declaration conflicts with target of using declaration already in scope
inline long long int llrint(long double __x) { return __builtin_llrintl(__x); }
                     ^
/opt/local/include/gcc/c++/cmath:1534:3: note: target of using declaration
  llrint(long double __x)
  ^
/opt/local/include/gcc/c++/math.h:94:12: note: using declaration
using std::llrint;
           ^

See ticket:56097#comment:10

Attachments (1)

patch-port-for-vsteffect.diff (6.1 KB) - added by RJVB (René Bertin) 6 years ago.

Download all attachments as: .zip

Change History (11)

comment:2 Changed 6 years ago by mojca (Mojca Miklavec)

The problem seems to be with

#if defined(__MAC_OS_X_VERSION_MIN_REQUIRED) && __MAC_OS_X_VERSION_MIN_REQUIRED <= __MAC_10_6

which is wrong, i.e. assuming that one cannot run C++11 code on 10.6 and this confuses everything.

Is forum the only place to file tickets for Audacity?

Version 0, edited 6 years ago by mojca (Mojca Miklavec) (next)

comment:3 Changed 6 years ago by mojca (Mojca Miklavec)

Additionally I needed to do

  • src/WaveTrack.cpp

    a b void WaveTrack::SplitAt(double t) 
    23682368         newClip->Clear(c->GetStartTime(), t);
    23692369
    23702370         //offset the NEW clip by the splitpoint (noting that it is already offset to c->GetStartTime())
    2371          sampleCount here = llrint(floor(((t - c->GetStartTime()) * mRate) + 0.5));
     2371         sampleCount here = llrint((long double)floor(((t - c->GetStartTime()) * mRate) + 0.5));
    23722372         newClip->Offset(here.as_double()/(double)mRate);
    23732373         // This could invalidate the iterators for the loop!  But we return
    23742374         // at once so it's okay

due to

WaveTrack.cpp:2371:29: error: call to 'llrint' is ambiguous
         sampleCount here = llrint(floor(((t - c->GetStartTime()) * mRate) + 0.5));
                            ^~~~~~
/opt/local/include/gcc/c++/cmath:1530:3: note: candidate function
  llrint(float __x)
  ^
/opt/local/include/gcc/c++/cmath:1534:3: note: candidate function
  llrint(long double __x)
  ^

comment:4 Changed 6 years ago by mojca (Mojca Miklavec)

effects/VST/VSTEffect.cpp:1116:12: error: constructor for 'VSTEffect' must explicitly initialize the member 'mResource' which does not have a default constructor
VSTEffect::VSTEffect(const wxString & path, VSTEffect *master)
           ^
effects/VST/VSTEffect.h:313:19: note: member is declared here
   ResourceHandle mResource;
                  ^

comment:5 Changed 6 years ago by RJVB (René Bertin)

So if I understand correctly, we should disable the fallback to non-C++11 mode in MemoryX.h ? I've wondered about that myself, actually. Did you already check if that's the only location where this fallback exists?

The later error in VSTEffect is strange, it's something I would have expected to see on Mac or Linux myself. The need for initialising explicitly shouldn't depend on what compiler and C++ runtime are being used, but there may be something to fix in the ResourceHandle definitions.

comment:6 Changed 6 years ago by mojca (Mojca Miklavec)

You can now see links to all builds next to the commit in github interface. Basically the builds fail on < 10.9 and succeed on all machines >= 10.9. On those machines where it fails, 10.5 can be ignored (failed dependency), 10.7 and 10.8 fail with this VSTEffect thingie and 10.6 fails with the above message. Once I disable that "if 10.6" fallback and fix that minor "unable to decide which function to use", 10.6 basically fails at the same point as on 10.7/10.8 (VSTEffect). So while I'm not 100% sure that these are all the places that needed a fix, it looks pretty optimistic. But I'm not sure what exactly is the problem with missing constructor, I didn't check.

comment:7 in reply to:  6 Changed 6 years ago by RJVB (René Bertin)

Replying to mojca:

You can now see links to all builds next to the commit in github interface.

I'll have to see if those are more useful than the raw buildbot reports, where I could rarely find the actual error.

On those machines where it fails, 10.5 can be ignored (failed dependency)

That was also a PPC build, right? I haven't checked, but wouldn't be amazed if the current Audacity is tricky to build on that platform anyway; will the port be ignored by the bots if I mark the architecture as x86 only?

But I'm not sure what exactly is the problem with missing constructor, I didn't check.

I'll have a look at that, and double-check if there are any other locations where 10.6 is being detected.

comment:8 Changed 6 years ago by mojca (Mojca Miklavec)

No, the links on github just point to buildbot. The main problem we are/were facing is that it's nearly impossible to find the buildbot logs a few days after the build has finished, but you won't get any added benefit otherwise.

Yes, 10.5 is a PPC. Whether or not the buildbot will ignore it is something I don't know, but I would not worry about that for now. We need to implement some new features in the base that will make it easier to declare and recognise that something cannot be built on a particular version of Darwin.

In the meantime upstream "fixed" the error (they now check if the OS version is lower than 10.6 instead of lower or equal), but I don't consider the fix to be a correct one.

Last edited 6 years ago by mojca (Mojca Miklavec) (previous) (diff)

comment:9 Changed 6 years ago by RJVB (René Bertin)

Seems they reverted the fix again...

I'm attaching a patch (to be applied to the portdir) which renames the patch that touched VSTEffect.cpp and improves it. The code is now 64-bit safer and handles initialisation of the mResource member in a way that should be compatible with libstdc++ . This is apparently where the error was.

The modified code compiles with clang++5/libc++ on Mac, and doesn't get flagged when parsing against libstdc++ on Linux (as far as I can check this, of course). I don't have VST plugins so I have not been able to test runtime behaviour.

Changed 6 years ago by RJVB (René Bertin)

comment:10 Changed 2 years ago by cooljeanius (Eric Gallager)

Cc: cooljeanius added
Note: See TracTickets for help on using tickets.