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

STYLE: VC9 compatibility does not seem necessary #202

Merged

Conversation

hjmjohnson
Copy link
Contributor

The code comments in the VC9 alternate files
were comments referencing VC8 and VC7 that are
no longer supported. Once the VC8 and VC7
specific comments were removed, there was
no longer any specialization for VC9, so
the alternate files for VC9 were no longer
relevant and indicate that VC9 will work
even without these files.

@hjmjohnson
Copy link
Contributor Author

Thank you Brad.

#202

I have a hope that vnl_math::abs(long double) can just be std::abs(long double).

In fact I am hoping that we can remove as many special cases as possible so that vnl_math:: is really just an alias for std::.

The only place where there is still conditional logic for the simpler case is with VC9 support.

Thank you for reporting the failures that are going to occur with the proposed pull request.

Hans

On 2/26/16, 9:26 AM, "Brad King" brad.king@kitware.com wrote:

On 02/25/2016 10:01 PM, Johnson, Hans J wrote:
_MSC_VER = 1500, Visual Studio 2008 = Version 9.x
There is a few remnants of code that claim it is needed for Version8
and Version7, but are in the Version9 files. If I can confirm that
Version9 does not need these work arounds, we can significantly
simplify some code to make it easier to maintain and easier to
review in the future.
Please let me know if you could build the current version of VXL under VS9?

I have it. Is there a PR to be tested?

-Brad

@bradking
Copy link
Contributor

This hunk in commit 005d4c7:

+  const long double pinf_q =   HUGE_VALL; //vcl_numeric_limits<long double>::infinity();
+  const long double ninf_q = - HUGE_VALL; //vcl_numeric_limits<long double>::infinity();

does not compile in VS 9 because it has no HUGE_VALL macro. After fixing this locally I can compile vnl_test_all, vnl_algo_test_all, and vcl_test_all with VS 9 as of the version in this PR.

@hjmjohnson
Copy link
Contributor Author

Thank you Brad. Could you paste a "git diff" of what you needed to do here?

I'm off to SPIE conference, and will have some time on my airplane rides to look into this.

@bradking
Copy link
Contributor

I made this change locally:

-  const long double pinf_q =   HUGE_VALL; //vcl_numeric_limits<long double>::infinity();
-  const long double ninf_q = - HUGE_VALL; //vcl_numeric_limits<long double>::infinity();
+  const long double pinf_q =   vcl_numeric_limits<long double>::infinity();
+  const long double ninf_q = - vcl_numeric_limits<long double>::infinity();

Of course this likely breaks whatever was fixed by switching to HUGE_VALL in the first place.

The code comments in the VC9 alternate files
were comments referencing VC8 and VC7 that are
no longer supported.  Once the VC8 and VC7
specific comments were removed, there was
no longer any specialization for VC9, so
the alternate files for VC9 were no longer
relevant and indicate that VC9 will work
even without these files.
hjmjohnson added a commit that referenced this pull request Feb 27, 2016
…esToVC9

STYLE: VC9 compatibility does not seem necessary
@hjmjohnson hjmjohnson merged commit d18fdc7 into vxl:master Feb 27, 2016
@hjmjohnson hjmjohnson deleted the RemoveReferencesToVC9 branch February 27, 2016 22:26
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