Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build opm-core with C++0x support if available #82

Merged
merged 6 commits into from
Oct 24, 2012
Merged

Conversation

rolk
Copy link
Member

@rolk rolk commented Oct 24, 2012

The externally visible header files of DUNE seems to be using two features if available: HAVE_STATIC_ASSERT and HAVE_NULLPTR, so I have added the macros that test for these.

@blattms
I think commit 056bce2 may be interesting for DUNE itself; you may want to cherry-pick that one.

These files are shamelessly ripped from dune-common/m4 (e28ca3)
Surely this must have been a copy&paste error.
Since DUNE does this, it might a good idea to track the same ABI.
@rolk
Copy link
Member Author

rolk commented Oct 24, 2012

@bska
I agree; I just pulled the same checks as in DUNE.

@bska
Copy link
Member

bska commented Oct 24, 2012

@rolk
I'm not sure I understand what you are saying. Do you think that --disable-gxx0xcheck should also disable any (and all) checks for -std=c++11? If so, then commit 056bce2 is a step back in my opinion. I don't really have an alternative implementation, though, so I'll merge the request.

@rolk
Copy link
Member Author

rolk commented Oct 24, 2012

@bska
I'll update the commit, please wait...

By adding an extra test we can avoid the somehow illogical possibility
of building with '--enable-gxx11check --disable-gxx0xcheck'.
@rolk
Copy link
Member Author

rolk commented Oct 24, 2012

@bska
The way it is now supposed to work: If C++11 is available, then skip check for C++0x. If C++0x is disabled, then don't check for C++11 either.

056bce2 is supposed to fix something which I believe is just a typo; they use the same option to check for both C++11 and C++0x (you can't disable C++11 and keep C++0x, then).

@rolk
Copy link
Member Author

rolk commented Oct 24, 2012

@bska
One moment...

@rolk
Copy link
Member Author

rolk commented Oct 24, 2012

@bska
Admittedly gold-plating, but then "later" will probably always be later :-)

@blattms
Only 6d2f7e1 is OPM-specific; the rest of the patches in the changeset could apply to DUNE also.

@bska
Copy link
Member

bska commented Oct 24, 2012

Brilliant!

Thank you so much.

@bska
Copy link
Member

bska commented Oct 24, 2012

Hmm. That's a bit strange.

When I apply this change locally, I too manage to reproduce the issue of #79. This is using both GCC 4.4 and GCC 4.7. I don't know what prevented me from seeing the issue earlier, but now I'm leaning towards #79 being a genuine (C++11) bug in opm/core/wells/WellsGroup.cpp.

GCC 4.7 gives a rather helpful diagnostic

[...]
../../opm/core/wells/WellsGroup.cpp: In member function 'virtual std::pair<Opm::WellNode*, double> Opm::WellNode::getWorstOffending(const std::__debug::vector<double>&, const std::__debug::vector<double>&, Opm::ProductionSpecification::ControlMode)':
../../opm/core/wells/WellsGroup.cpp:785:66: error: no matching function for call to 'make_pair(Opm::WellNode* const, double)'
[... snip ...]
/work/encap/gcc-4.7.0+0/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/stl_pair.h:274:5: note: template<class _T1, class _T2> constexpr std::pair<typename std::__decay_and_strip<_T1>::__type, typename std::__decay_and_strip<_T2>::__type> std::make_pair(_T1&&, _T2&&)
/work/encap/gcc-4.7.0+0/lib/gcc/x86_64-unknown-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/stl_pair.h:274:5: note:   template argument deduction/substitution failed:
../../opm/core/wells/WellsGroup.cpp:785:66: note:   cannot convert 'this' (type 'Opm::WellNode* const') to type 'Opm::WellNode*&&'
[...]

Especially, a Opm::WellNode* const is spectacularly different from a const Opm::WellNode*.

@rolk
Copy link
Member Author

rolk commented Oct 24, 2012

Especially, a Opm::WellNode* const is spectacularly different from a const Opm::WellNode*

Indeed. It still looks like a compiler/library bug; the standard says (or used to say at least :-)) that 'this' was either 'T_' or 'const T_', depending. Anyway, the following assignment compiles fine: pair<int*,double> p = make_pair(nullptr,0.);, and nullptr is even "more const" than 'this' ;-)

Also, this example compiles with g++ -c -std=c++0x

#include <utility>
struct Foo {
    std::pair<Foo*,int> bar () {
        return std::make_pair (this, 0);
    }
};

At this point, I'll just be a coward and tip over the game table by declaring that we can just create the std::pair directly since we already know the components' types -- commited as 09f458 ;-)

@bska
Copy link
Member

bska commented Oct 24, 2012

@rolk
Following commit 09f458f everything works perfectly upon including this PR. I'll merge shortly.

Thank you very much!

bska added a commit that referenced this pull request Oct 24, 2012
Build opm-core with C++0x support if available
@bska bska merged commit eec1552 into OPM:master Oct 24, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants