Skip to content

Make our use of isspace more robust#3508

Merged
roystgnr merged 2 commits intolibMesh:develfrom
roystgnr:isspace_robust
Apr 3, 2023
Merged

Make our use of isspace more robust#3508
roystgnr merged 2 commits intolibMesh:develfrom
roystgnr:isspace_robust

Conversation

@roystgnr
Copy link
Copy Markdown
Member

We had some users of a modified libMesh who saw the existing code break here - their fault for bringing in a 3rd party header with using namespace std (thereby making isspace ambiguous), but the workaround is easy enough, and I'm not a big fan of us including C headers when there's a C++ version around anyway.

Pinging @rainiscold and @LabrosV ; hopefully I haven't typo'd either.

We had some users of a modified libMesh who saw the existing code break
here - their fault for bringing in a 3rd party header with
`using namespace std` and thereby making `isspace` ambiguous, but the
workaround is easy enough.
@jwpeterson
Copy link
Copy Markdown
Member

Any chance you'd want to make the same update in AbaqusIO and GMVIO?

@roystgnr
Copy link
Copy Markdown
Member Author

Huh; definitely. It wasn't a problem there because there's no std algorithm use and so no ambiguous template argument, but let me get them all (and getpot.h) while I'm at it.

@roystgnr
Copy link
Copy Markdown
Member Author

Wait, GMV is also using it in remove_if. I guess the polluting header just didn't end up in the dependencies there.

@moosebuild
Copy link
Copy Markdown

Job Coverage on 5b40c68 wanted to post the following:

Coverage

eda2b5 #3508 5b40c6
Total Total +/- New
Rate 60.02% 60.02% -0.00% 11.11%
Hits 48868 48868 - 1
Misses 32548 32550 +2 8

Diff coverage report

Full coverage report

Warnings

  • New new line coverage rate 11.11% is less than the suggested 90.0%

This comment will be updated on new commits.

@roystgnr roystgnr merged commit 1b2de0c into libMesh:devel Apr 3, 2023
@roystgnr roystgnr deleted the isspace_robust branch April 3, 2023 12:30
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