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

ParsedFEMFunction: change "parsers" to a vector of unique_ptrs #2720

Merged
merged 1 commit into from
Sep 30, 2020

Conversation

jwpeterson
Copy link
Member

This is an attempt to fix the issue raised in #2710. Since
FunctionParserBase has a deleted move constructor, it is not
considered to be MoveInsertable, which means it cannot be passed to
std::vector::push_back() by value.

This is an attempt to fix the issue raised in libMesh#2710. Since
FunctionParserBase has a deleted move constructor, it is not
considered to be MoveInsertable, which means it cannot be passed to
std::vector::push_back() by value.

Adding a vector<unique_ptr> to the class means that it can no longer
be default copy constructed. We had a unit test of this but, no actual
uses within the library so hopefully it's OK to drop it.
@jwpeterson
Copy link
Member Author

The "Test No Optional" failure here is the same one reported in

1) test: MeshInputTest::testDynaReadPatch (E) 
uncaught exception of type libMesh::LogicError
- ERROR:  Unrecognized solver package: 6

so I think this change is otherwise fine when Fparser is disabled.

@roystgnr
Copy link
Member

This looks good at first glance, but I think our only really decent test coverage here was in GRINS. Give me a day or so to futz around there and try things out?

@jwpeterson
Copy link
Member Author

Sure, I'm not even trying to get this into 1.6.0 necessarily. If its proven that it works with clang-11, then I'll be backporting it to the 1.4 and 1.5 releases anyway so I can always tack on a 1.6.x at that time as well.

@jwpeterson
Copy link
Member Author

Note to self: invalidate "Test No Optional" build after #2721 is merged.

@moosebuild
Copy link

Job Test No Optional on 783091b : invalidated by @jwpeterson

Try again now that "Test No Optional" configuration was fixed in another PR

@roystgnr
Copy link
Member

Works fine with my tests, including with GRINS.

@roystgnr roystgnr merged commit 16910ef into libMesh:master Sep 30, 2020
@jwpeterson jwpeterson deleted the fix_2710 branch October 1, 2020 13:44
jwpeterson added a commit that referenced this pull request Oct 6, 2020
The master hashes of the commits cherry-picked and squashed to create this commit:

PR #2720
783091b

PR #2731
b67b3e4
0a1dbe4
73d58f2
3a61395
a6f14fe
00cd55d
1310eac

A similar "combined" commit should also be cherry-picked onto the
1.4.x and 1.5.x series and new patch-level releases should be made for
them, since this is an issue which prevents certain compilers from
being able to compile the library.

Refs #2710
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.

3 participants