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

Switch caller and callee to suppress warnings #544

Closed
wants to merge 1 commit into from

Conversation

jslee02
Copy link
Member

@jslee02 jslee02 commented Nov 3, 2015

No description provided.

@jslee02 jslee02 added this to the DART 5.1.1 milestone Nov 3, 2015
@mkoval
Copy link
Collaborator

mkoval commented Nov 4, 2015

👍 These warnings always bothered me when building DART, but I couldn't figure out how to fix them. I'm glad you did. 😄

@mxgrey
Copy link
Member

mxgrey commented Nov 4, 2015

This pull request breaks backwards compatibility. If someone extended the Function class such that they overrided the eval(Eigen::Map<Eigen::VectorXd>) function signature (which is how it worked up until 5.1), then their derived function will no longer work correctly when passed into a Solver (it will just behave like a constant 0.0 function instead). The warning was bothersome, but it was a symptom of maintaining backwards compatibility until the next major release.

@mxgrey
Copy link
Member

mxgrey commented Nov 4, 2015

It should be noted that the Solver implementations are calling the eval(const Eigen::VectorXd&) function signature now, and not the eval(Eigen::Map<Eigen::VectorXd>) function signature.

@jslee02
Copy link
Member Author

jslee02 commented Nov 4, 2015

Yeah, this warnings were annoying. 😢 However, I missed the possibility that @mxgrey pointed out.

It seems we need to keep this as it was for the backward compatibility. Instead, I would like to suppress the warnings for this specific case. I think I found a way to suppress warnings for specific lines but it is not cross platform.

I will close this PR and create another one for it.

@jslee02 jslee02 closed this Nov 4, 2015
jslee02 added a commit that referenced this pull request Nov 4, 2015
For backward compatibility, we have to call deprecated functions in Function.cpp, and compilers complain for these calls. This commit suppresses the warnings only for those calls. See also #544.

Note that the suppression code couldn't be made as macros. It needs to use compiler dependent #pragma directives but directives is not allowed to be decleared in macros.
@psigen
Copy link
Collaborator

psigen commented Nov 4, 2015

Is this something that's worth losing backward compatibility on the next major release? It seems undesirable to accumulate warning suppression cruft over time.

@mxgrey
Copy link
Member

mxgrey commented Nov 4, 2015

Yes, my plan was to remove the old method with the next major release.

@jslee02 jslee02 deleted the function_warnings branch November 6, 2015 03:06
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.

4 participants