-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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 exprtk to submodule #3965
Conversation
Travis-CI looks good. I cancelled Apple after waiting for 20 minutes for it to start. Merging. |
Well, Apple is failing, I guess I merged this too quickly. I narrowed the build failure down to this line: inline bool string_to_real(const std::string& s, T& t)
{
- const typename numeric::details::number_type<T>::type num_type;
-
const char_t* begin = s.data();
const char_t* end = s.data() + s.size();
-
+ typename numeric::details::number_type<T>::type num_type;
return string_to_real(begin, end, t, num_type);
} A PR with this exact fix was closed here: ArashPartow/exprtk#9 I can reproduce this build error on my machine as well, which is identical to what Travis uses (OS X 10.11.6, Apple LLVM version 8.0.0 (clang-800.0.42.1)) A few people asking similar questions: |
Build seems to fail on struct tp {};
int main() {
const tp t;
return 0;
} |
@PhysSong thanks, that's what I found as well. Unfortunately, this is the default version for both Travis-CI as well as the Mac I use to create our installers from. Should we write a patch? |
@tresf I'm planning on fixing this particular 'issue' over the weekend. The error older clang versions provide turns out to be partially true and false. If the struct has one or more members, the error diagnostic would be correct. However according to DR-253, the error diagnostic should NOT be made if the struct is empty. The struct in question is used in tag-dispatch and is by design meant to be empty (no members) That all being said in order to be compatible with both pre/post C++11 versions, the solution can't use the default initialiser "{}", so the only viable option will be to implement a default constructor. |
I've updated the library. Let me know if the compilation errors still persist. |
@ArashPartow thanks kindly. Testing it out here #4013. |
Looks good, merged via 67231cb. Thanks @ArashPartow! |
Moves the Xpressive plugin's exprtk library to a submodule to pull directly from @ArashPartow, removing 38,000+ lines of code from our github repository. :)
For consistency with other plugins, uses
TitleCase
for the class names (e.g.XpressiveFoo
)and renamed folders and classes to match accordingly.@gnudles, FYI.