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.

Sign up for GitHub

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

Jump to bottom

Concerning warning when building #58

Closed
JBorrow opened this issue Feb 1, 2021 · 3 comments
Closed

Concerning warning when building #58

JBorrow opened this issue Feb 1, 2021 · 3 comments

Comments

@JBorrow
Copy link

JBorrow commented Feb 1, 2021

Describe the bug
I get the following warning when building on my laptop:

/Users/mphf18/Documents/swift/VELOCIraptor-STF/src/ui.cxx:811:22: note: use '=='
      to turn this assignment into an equality comparison
                if (j=line.find(sep)){
                     ^
                     ==

Using

Apple clang version 11.0.0 (clang-1100.0.33.8)
Target: x86_64-apple-darwin20.2.0

cmake -DVR_USE_SWIFT_INTERFACE=ON -DVR_USE_HYDRO=OND -DCMAKE_CXX_FLAGS="-fPIC" -DCMAKE_BUILD_TYPE=Release -DOpenMP_CXX_FLAGS="-Xpreprocessor -fopenmp -I/usr/local/opt/libomp/include" -DOpenMP_CXX_LIB_NAMES="omp" -DOpenMP_omp_LIBRARY=/usr/local/opt/libomp/lib/libomp.dylib ..

Might be worth checking out.
rtobar added a commit that referenced this issue Feb 2, 2021
@rtobar
Fix incorrect parsing logic
1ef9252 
This code, as previously written, entered into the "if" body even when
no separator ("=") was found. By good chance this didn't cause any
issues: because of how std::string::substr works the two calls issued
using the position value "j" ended up not throwing exceptions, although
both the parameter name and value would end up being equal to the full
line. This in turn means none of the subsequent "if" statements that set
different options are invoked.

Although this was probably never seen in the wild because configuration
files are seldom incorrectly written, a warning was generated by
different compilers (depending on version and warning flags). Since the
code is quite warning-full we never paid much attention to it until now.

The new code now properly checks that the result of the find call is
valid before using it.

This addresses #58.

Signed-off-by: Rodrigo Tobar <[email protected]>
@rtobar rtobar mentioned this issue Feb 2, 2021
Merged
@rtobar
Copy link

rtobar commented Feb 2, 2021

A fix has been added in the issue-58 branch, which after quick testing will be merged back to the master branch (and then this issue can be closed). Please note that although the warning is true, and has been addressed, the code didn't lend itself to a crash, and had the expected behavior anyway.

@rtobar
Copy link

rtobar commented Feb 2, 2021

Merged to the master, warning should be gone now. Thanks @JBorrow for reporting!

@rtobar rtobar closed this as completed Feb 2, 2021
@JBorrow
Copy link
Author

JBorrow commented Feb 2, 2021

Thanks very much for fixing this so quickly!

rtobar added a commit that referenced this issue Jun 24, 2021
@rtobar
Fix warning in configuration file parsing
c2202da 
This is very similar to the warning reported in #58, but for some reason
I didn't fix it in 1ef9252. Here it goes.

Signed-off-by: Rodrigo Tobar <[email protected]>
@rtobar rtobar mentioned this issue Jun 24, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@rtobar @JBorrow