Skip to content

[Framework] Fix more warnings (clang, macos)#6083

Open
fredroy wants to merge 3 commits intosofa-framework:masterfrom
fredroy:fix_more_warnings_framework
Open

[Framework] Fix more warnings (clang, macos)#6083
fredroy wants to merge 3 commits intosofa-framework:masterfrom
fredroy:fix_more_warnings_framework

Conversation

@fredroy
Copy link
Copy Markdown
Contributor

@fredroy fredroy commented Apr 16, 2026 •
edited
Loading

Mostly warning about implicit conversions (real to bool, signed/unsigned, to a smaller type)

The not-trivial one is Sofa/framework/Type/src/sofa/type/Mat.h

  template<typename real2>
    constexpr Mat(const Mat<L,C,real2>& m) noexcept
    {
        std::copy(m.begin(), m.begin()+L, this->begin());
    }

used with CRS templated on blocks (i.e Vec), it can lead conversion (copy) from Vec3f to Vec3d, thus triggering the deprecations set in #5675

NOTE: the AI was a local-deployed one (Qwen3.5)

[with-all-tests]

By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).

Reviewers will merge this pull-request only if

  • it builds with SUCCESS for all platforms on the CI.
  • it does not generate new warnings.
  • it does not generate new unit test failures.
  • it does not generate new scene test failures.
  • it does not break API compatibility.
  • it is more than 1 week old (or has fast-merge label).

@fredroy fredroy added pr: fast merge Minor change that can be merged without waiting for the 7 review days pr: status wip Development in the pull-request is still in progress pr: clean Cleaning the code pr: AI-aided Label notifying the reviewers that part or all of the PR has been generated with the help of an AI pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels Apr 16, 2026
@fredroy
Copy link
Copy Markdown
Contributor Author

fredroy commented Apr 16, 2026

[ci-build][with-all-tests]

hugtalbot
hugtalbot reviewed Apr 24, 2026
View reviewed changes
Comment thread Sofa/framework/Helper/src/sofa/helper/MarchingCubeUtility.cpp
Comment on lines +855 to +862
if ( cell.val[0] != 0 ) voxels.push_back ( ( coord[0]+0 ) + ( coord[1]+0 ) *m_dataResolution[0] + ( coord[2]+0 ) *m_dataResolution[0]*m_dataResolution[1] ); //les voxels occupes ds ce cube
if ( cell.val[1] != 0 ) voxels.push_back ( ( coord[0]+1 ) + ( coord[1]+0 ) *m_dataResolution[0] + ( coord[2]+0 ) *m_dataResolution[0]*m_dataResolution[1] ); //les voxels occupes ds ce cube
if ( cell.val[3] != 0 ) voxels.push_back ( ( coord[0]+0 ) + ( coord[1]+1 ) *m_dataResolution[0] + ( coord[2]+0 ) *m_dataResolution[0]*m_dataResolution[1] ); //les voxels occupes ds ce cube
if ( cell.val[2] != 0 ) voxels.push_back ( ( coord[0]+1 ) + ( coord[1]+1 ) *m_dataResolution[0] + ( coord[2]+0 ) *m_dataResolution[0]*m_dataResolution[1] ); //les voxels occupes ds ce cube
if ( cell.val[4] != 0 ) voxels.push_back ( ( coord[0]+0 ) + ( coord[1]+0 ) *m_dataResolution[0] + ( coord[2]+1 ) *m_dataResolution[0]*m_dataResolution[1] ); //les voxels occupes ds ce cube
if ( cell.val[5] != 0 ) voxels.push_back ( ( coord[0]+1 ) + ( coord[1]+0 ) *m_dataResolution[0] + ( coord[2]+1 ) *m_dataResolution[0]*m_dataResolution[1] ); //les voxels occupes ds ce cube
if ( cell.val[7] != 0 ) voxels.push_back ( ( coord[0]+0 ) + ( coord[1]+1 ) *m_dataResolution[0] + ( coord[2]+1 ) *m_dataResolution[0]*m_dataResolution[1] ); //les voxels occupes ds ce cube
if ( cell.val[6] != 0 ) voxels.push_back ( ( coord[0]+1 ) + ( coord[1]+1 ) *m_dataResolution[0] + ( coord[2]+1 ) *m_dataResolution[0]*m_dataResolution[1] ); //les voxels occupes ds ce cube
Copy link
Copy Markdown
Contributor

@hugtalbot hugtalbot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This clearly changes the check, is the check = 0 the one appropriate before the push_back ?

Copy link
Copy Markdown
Contributor

@alxbilger alxbilger Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any problem here.

Comment thread Sofa/framework/Helper/src/sofa/helper/system/FileMonitor_macosx.cpp
kFSEventStreamCreateFlagFileEvents // FSEventStreamCreateFlags flags
);
FSEventStreamScheduleWithRunLoop(m_eventStream,CFRunLoopGetCurrent(), kCFRunLoopDefaultMode);
FSEventStreamSetDispatchQueue(m_eventStream, dispatch_get_main_queue());
Copy link
Copy Markdown
Contributor

@hugtalbot hugtalbot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc says "Schedules the stream on the specified dispatch queue." where dispatch_get_main_queue() is the dispatch queue to use to receive events

The documentation also states that FSEventStreamSetDispatchQueue is now deprecated and the new function is advised to be used

So 👍

Comment thread Sofa/framework/Type/src/sofa/type/DualQuat.inl
real q0dqe = (real) ( qn.orientation[0]*dq.dual[0] + qn.orientation[1]*dq.dual[1] + qn.orientation[2]*dq.dual[2] + qn.orientation[3]*dq.dual[3]);

if(dq.orientation[0] || dq.orientation[1] || dq.orientation[2] || dq.orientation[3])
if(dq.orientation[0] != 0 || dq.orientation[1] != 0 || dq.orientation[2] != 0 || dq.orientation[3] != 0)
Copy link
Copy Markdown
Contributor

@hugtalbot hugtalbot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we just want to check whether the quat exists?

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

Reviewers

@alxbilger alxbilger alxbilger left review comments

@hugtalbot hugtalbot hugtalbot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

pr: AI-aided Label notifying the reviewers that part or all of the PR has been generated with the help of an AI pr: clean Cleaning the code pr: fast merge Minor change that can be merged without waiting for the 7 review days pr: status to review To notify reviewers to review this pull-request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fredroy @alxbilger @hugtalbot