some (bundled) minor changes (take 2) #176

esohns · 2015-12-02T13:56:14Z

second submission of pull request #172 (see description and commit labels).
This addresses some issues with ACE, mainly template instantiation problems with MSVC.
Has been compiled and (briefly) tested on Win32 (Windows 8 and 10, MSVC 2015) and Linux (g++ 5.2.1)

…mplate instantiation issues on Win32)

# Conflicts: # ACE/ace/Condition_Recursive_Thread_Mutex.h

jwillemsen · 2015-12-04T08:40:21Z

ACE/ace/Condition_Recursive_Thread_Mutex.h

@@ -101,11 +108,22 @@ class ACE_Export ACE_Condition<ACE_Recursive_Thread_Mutex>
  ACE_Recursive_Thread_Mutex &mutex_;

 };
+// *NOTE*: prevent implicit instantiations by includees to relieve the linker
+#if defined (__GNUG__)


Shouldn't this have a version check on g++ version, maybe add a new ACE_HAS... macro because probably this is then needed on more places?

yes, please see commit 4155a8c

jwillemsen · 2015-12-04T17:30:54Z

ACE/ace/Condition_Recursive_Thread_Mutex.h

+#if defined (ACE_HAS_CPP11_EXTERN_TEMPLATES)
+// suppress a warning, g++ 5.2.1 does not support attributes on template
+// instantiation declarations. *TODO*: this may go back further
+# if defined (__GNUG__) && (__GNUC__ == 5 && __GNUC_MINOR__ == 2 && __GNUC_PATCHLEVEL__ == 1)


Also use a new define for this, that makes it easier when this is needed in more places and for example gcc 5.2.2 has the same issue

agreed, please see commit b695f27

…mplate instantiation issues on Win32) (update) this addresses complaints by the integration build system (#include ace/OS.h)

jwillemsen · 2015-12-08T15:23:14Z

Looks the travis-ci build fails, have you compiled also the complete TAO codebase with these changes?

esohns · 2015-12-13T18:07:46Z

Hi. As you can see the test builds now complete successfully.

I tried compiling TAO with MSVC. However, there appears to be an issue with some headers missing from the (MPC-generated) project file(s) (e.g. $TAO_ROOT/tao/IOPC.h), so some of the projects do not compile with this approach at the moment. I have not pursued this any further.
The build completes fine otherwise.

# Conflicts: # ACE/protocols/ace/HTBP/HTBP_Channel.cpp # ACE/protocols/ace/HTBP/HTBP_Inside_Squid_Filter.cpp # ACE/protocols/ace/HTBP/HTBP_Session.cpp

jwillemsen · 2016-09-12T06:54:21Z

All checks have failed

jwillemsen · 2016-10-24T14:27:13Z

Could you provide some more background on the "mainly template instantiation problems with MSVC." We haven't seen this problem until now and I am not sure what you are fixing.

jwillemsen · 2016-10-24T14:31:02Z

ACE/tests/Recursive_Condition_Bug_Test.cpp

+#include "test_config.h"
+
+// *NOTE*: explicit template instantiation required here...
+template class ACE_Condition<ACE_Recursive_Thread_Mutex>;


Why is this required?

fixed with 53bbf4b

… commit

esohns · 2016-10-24T16:44:11Z

I am using ACE in a project that has had trouble linking with MSVC 2015. The error was that of multiple defined symbols in conjunction with (explicit) template instantiations scattered over the ACE code (specifically, the ACE_Condition<ACE_Recursive_Thread_Mutex> specialization). Recent versions of MSVC support extern template declarations (the equivalent of extern variables) to address this issue, so I have introduced the notion to ACE

jwillemsen · 2016-10-24T18:44:38Z

I am concerned about breaking user applications given the significant amount of changes you made outside of the ACE core, are they all really needed? Could you extend one of the unit tests with an use case that doesn't work before your changes? Also please make a note about these changes in the ACE/NEWS file

jwillemsen · 2016-10-24T18:45:25Z

ACE/ace/Condition_Recursive_Thread_Mutex.h

 #include "ace/Recursive_Thread_Mutex.h"
 #include "ace/Condition_Attributes.h"
 #include "ace/Condition_T.h"

 ACE_BEGIN_VERSIONED_NAMESPACE_DECL

+class ACE_Time_Value;


Is this one needed?

this is a 'forward declaration'; the header does not #include ace/Time_Value.h, even though it is referencing ACE_Time_Value*. Depending on what the other #includes pull in, this may not be needed (but the header is more consistent this way). As I was changing the header, I cleaned it up a little bit.

jwillemsen · 2016-10-24T18:48:02Z

ACE/ace/config-g++-common.h

+// instantiation declarations
+// *TODO*: this probably goes back further than 5.2
+# if (__GNUC__ >= 6 || (__GNUC__ == 5 && __GNUC_MINOR__ >= 2))
+#  define ACE_LACKS_CPP11_EXTERN_TEMPLATE_ATTRIBUTES


What about gcc 6.x, still needed? Your remark is just about 5.2.1

this needs to be tested against newer versions of gcc and adjusted when it integrates the C++11 extern template attributes feature in the future. Also, the targeted version range needs tuning, as the patch applies to earlier versions of gcc as well (see TODO: this probably goes back as far as gcc 4.3 when ACE_HAS_CPP11_EXTERN_TEMPLATES was introduced (not verified)).
I don't have access to any other versions of gcc, so I cannot test this exhaustively. However, as this feature merely introduces/suppresses a compiler warning, the impact ought to be minimal, while timely feedback from other users on different platforms is likely.

esohns · 2016-10-30T21:34:25Z

Updated NEWS. The only changes that are really features is the support for extern templates, and the timer queue code rework. The latter already has a unit test which covers the functionality. At this stage I am not sure how to come up with a minimal example that could serve as a unit/regression test for the former; I will add this as a Compiler_Features_xx_Test.cpp in /tests when I find time.

jwillemsen · 2016-10-31T11:14:42Z

Merged to check with the full scoreboard

jwillemsen · 2016-10-31T15:34:57Z

Had to revert, looks builds started to break

jwillemsen · 2016-10-31T15:46:46Z

Working on some changes as part of issue #320 so that travis-ci shows the face configuration issues, please make a new pull request when you see #320 integrated in master

jwillemsen · 2016-10-31T18:25:44Z

@esohns Issue #320 got merged to master, can you get that and look at the errors in the FACE configuration. Also check http://www.dre.vanderbilt.edu/scoreboard/integrated.html, could be that some other builds also compile the changes.

esohns · 2016-11-01T14:15:33Z

I had a look at the build failures on the Linux platform [http://scoreboard.ociweb.com/doc_master_ant_linux-sb_gcc_d0o1/2016_10_31_14_39_Totals.html](build failures). The errors are related to the removal of ace/Synch.h from ace/Thread_Manager.h. I have created a new pull request #322 that addresses this (and corelated) issues. Please refer to the explanation in the pull request for more information.

Other than that, I found nothing unusual on the scoreboard so far

Erik Sohns added 11 commits December 1, 2015 19:35

improved solaris 11 support

3bff6f8

fix problem with std io when using ACE_OSTREAM_TYPE

6cec721

added support for CancelIoEx() (Win32)

502c6fc

fixed an issue with (exported) template specializations (Win32)

aeb68c0

fix potential memory leak when using the Win32 Proactor (part1)

de13b79

repaired include strategy for ace/Synch.h (prerequisite to solving te…

7acfc9e

…mplate instantiation issues on Win32)

fixed an issue with (exported) template specializations (Win32) (part2)

6d66024

fixed an issue with (exported) template specializations (Win32) (part2)

b1d83ea

Merge branch 'master' of github.com:esohns/ATCD

6dba799

# Conflicts: # ACE/ace/Condition_Recursive_Thread_Mutex.h

fix potential memory leak when using the Win32 Proactor (part2a)

c717bec

fixed an issue with (exported) template specializations (Win32) (part3)

733efc7

jwillemsen reviewed Dec 4, 2015
View reviewed changes

Erik Sohns added 3 commits December 4, 2015 18:12

fixed an issue with (exported) template specializations (Win32)

4155a8c

fixed an issue with (exported) template specializations (Win32)

9ca4697

Merge branch 'master' of github.com:esohns/ATCD

2609c79

jwillemsen reviewed Dec 4, 2015
View reviewed changes

Erik Sohns added 5 commits December 4, 2015 18:34

fix potential memory leak when using the Win32 Proactor (part2a)

1280574

fixed an issue with (exported) template specializations (Win32)

b695f27

fixed an issue with (exported) template specializations (Win32)

39aeb7f

fixed an issue with (exported) template specializations (Win32)

5a757dd

repaired include strategy for ace/Synch.h (prerequisite to solving te…

a255354

…mplate instantiation issues on Win32) (update) this addresses complaints by the integration build system (#include ace/OS.h)

added support for CancelIoEx() (Win32) (update passes travis-ci build)

dee50e8

Erik Sohns added 2 commits May 30, 2016 00:18

reuse current timestamp to compute wait delay

219ae58

Merge branch 'master' of https://github.com/DOCGroup/ACE_TAO

7ad5360

# Conflicts: # ACE/protocols/ace/HTBP/HTBP_Channel.cpp # ACE/protocols/ace/HTBP/HTBP_Inside_Squid_Filter.cpp # ACE/protocols/ace/HTBP/HTBP_Session.cpp

Erik Sohns added 2 commits October 23, 2016 18:46

Merge branch 'master' of https://github.com/DOCGroup/ACE_TAO

d5fbc0d

repaired integration build failure

8dbd724

jwillemsen reviewed Oct 24, 2016

View reviewed changes

removed now-obsolete template instantiation left over from an earlier…

53bbf4b

… commit

jwillemsen reviewed Oct 24, 2016

View reviewed changes

updated NEWS

4bb4047

jwillemsen merged commit bd5aaab into DOCGroup:master Oct 31, 2016

jwillemsen mentioned this pull request Oct 31, 2016

Revert "some (bundled) minor changes (take 2)" #318

Merged

esohns mentioned this pull request Nov 1, 2016

ace/Synch.h sanitizations #322

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

some (bundled) minor changes (take 2) #176

some (bundled) minor changes (take 2) #176

esohns commented Dec 2, 2015 •

edited

Loading

jwillemsen Dec 4, 2015

esohns Dec 4, 2015

jwillemsen Dec 4, 2015

esohns Dec 4, 2015

jwillemsen commented Dec 8, 2015

esohns commented Dec 13, 2015

jwillemsen commented Sep 12, 2016

jwillemsen commented Oct 24, 2016

jwillemsen Oct 24, 2016

esohns Oct 24, 2016 •

edited

Loading

esohns commented Oct 24, 2016 •

edited

Loading

jwillemsen commented Oct 24, 2016

jwillemsen Oct 24, 2016

esohns Oct 30, 2016 •

edited

Loading

jwillemsen Oct 24, 2016

esohns Oct 30, 2016 •

edited

Loading

esohns commented Oct 30, 2016 •

edited

Loading

jwillemsen commented Oct 31, 2016

jwillemsen commented Oct 31, 2016

jwillemsen commented Oct 31, 2016

jwillemsen commented Oct 31, 2016

esohns commented Nov 1, 2016 •

edited

Loading

some (bundled) minor changes (take 2) #176

some (bundled) minor changes (take 2) #176

Conversation

esohns commented Dec 2, 2015 • edited Loading

jwillemsen Dec 4, 2015

Choose a reason for hiding this comment

esohns Dec 4, 2015

Choose a reason for hiding this comment

jwillemsen Dec 4, 2015

Choose a reason for hiding this comment

esohns Dec 4, 2015

Choose a reason for hiding this comment

jwillemsen commented Dec 8, 2015

esohns commented Dec 13, 2015

jwillemsen commented Sep 12, 2016

jwillemsen commented Oct 24, 2016

jwillemsen Oct 24, 2016

Choose a reason for hiding this comment

esohns Oct 24, 2016 • edited Loading

Choose a reason for hiding this comment

esohns commented Oct 24, 2016 • edited Loading

jwillemsen commented Oct 24, 2016

jwillemsen Oct 24, 2016

Choose a reason for hiding this comment

esohns Oct 30, 2016 • edited Loading

Choose a reason for hiding this comment

jwillemsen Oct 24, 2016

Choose a reason for hiding this comment

esohns Oct 30, 2016 • edited Loading

Choose a reason for hiding this comment

esohns commented Oct 30, 2016 • edited Loading

jwillemsen commented Oct 31, 2016

jwillemsen commented Oct 31, 2016

jwillemsen commented Oct 31, 2016

jwillemsen commented Oct 31, 2016

esohns commented Nov 1, 2016 • edited Loading

esohns commented Dec 2, 2015 •

edited

Loading

esohns Oct 24, 2016 •

edited

Loading

esohns commented Oct 24, 2016 •

edited

Loading

esohns Oct 30, 2016 •

edited

Loading

esohns Oct 30, 2016 •

edited

Loading

esohns commented Oct 30, 2016 •

edited

Loading

esohns commented Nov 1, 2016 •

edited

Loading