Skip to content

Switch exprtk to submodule#3965

Merged
tresf merged 2 commits into
LMMS:masterfrom
tresf:exprtk
Nov 11, 2017
Merged

Switch exprtk to submodule#3965
tresf merged 2 commits into
LMMS:masterfrom
tresf:exprtk

Conversation

@tresf

@tresf tresf commented Nov 10, 2017

Copy link
Copy Markdown
Member

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.

@tresf

tresf commented Nov 11, 2017

Copy link
Copy Markdown
Member Author

Travis-CI looks good. I cancelled Apple after waiting for 20 minutes for it to start. Merging.

@tresf tresf merged commit 231cc82 into LMMS:master Nov 11, 2017
@tresf

tresf commented Nov 11, 2017

Copy link
Copy Markdown
Member Author

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:
https://stackoverflow.com/questions/27743264
https://stackoverflow.com/questions/7411515

This was referenced Nov 11, 2017
@PhysSong

PhysSong commented Nov 14, 2017

Copy link
Copy Markdown
Member

Build seems to fail on clang < 3.9(I used Clang shipped with Ubuntu) only. I don't know about Apple Clang, but it seems to generate errors only for old Clang.
You may try to compile this:

struct tp {};

int main() {
	const tp t;
	return 0;
}

@tresf

tresf commented Nov 14, 2017

Copy link
Copy Markdown
Member Author

@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 tresf mentioned this pull request Nov 14, 2017
@tresf tresf deleted the exprtk branch November 14, 2017 02:24
@ArashPartow

Copy link
Copy Markdown
Contributor

@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.

@ArashPartow

Copy link
Copy Markdown
Contributor

I've updated the library. Let me know if the compilation errors still persist.

@tresf

tresf commented Nov 27, 2017

Copy link
Copy Markdown
Member Author

@ArashPartow thanks kindly. Testing it out here #4013.

@tresf

tresf commented Nov 27, 2017

Copy link
Copy Markdown
Member Author

Looks good, merged via 67231cb. Thanks @ArashPartow!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants