Skip to content

Compiler fixes#27

Merged
hverhelst merged 3 commits into
mainfrom
hotfix
Jul 11, 2025
Merged

Compiler fixes#27
hverhelst merged 3 commits into
mainfrom
hotfix

Conversation

@hverhelst

Copy link
Copy Markdown
Member

FIX:

  • gsExprHelper path in gsAlmostC1
  • Reference of gsMultiPatch::basis in MPBES code

fix MPBES with multi-patch.basis reference
@hverhelst hverhelst requested a review from Copilot July 11, 2025 09:16

This comment was marked as outdated.

@hverhelst hverhelst requested a review from Copilot July 11, 2025 09:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses compiler errors by updating basis cloning logic, include paths, and type qualifiers.

  • Switched to clone().release() for multi-patch basis references with explicit freeAll cleanup.
  • Fixed incorrect include paths for gsExprHelper in DPatchBase and AlmostC1.
  • Added const to dynamic_cast targets and adjusted m_patches storage in DPatchBase.

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/gsMPBESUtils.h Use clone().release() and freeAll to fix basis references in MPBES.
src/gsDPatchBase.hpp Updated include path for gsExprHelper from gsAssembler to gsExpressions.
src/gsAlmostC1.hpp Updated include path for gsExprHelper from gsAssembler to gsExpressions.
src/gsDPatchBase.h Changed m_patches from reference to value to resolve lifetime/ownership.
src/gsMPBESHSplineBasis.hpp Added const qualifier on dynamic_cast target in constructor assertion.
src/gsMPBESBSplineBasis.hpp Added const qualifier on dynamic_cast target in constructor assertion.
Comments suppressed due to low confidence (4)

src/gsMPBESHSplineBasis.hpp:88

  • [nitpick] The assertion message uses plural "Bases" and is awkward. Consider changing it to something like "Basis at patch index %zu is not of type gsHTensorBasis<2>" for clarity.
        GISMO_ASSERT(dynamic_cast<const gsHTensorBasis<2> * >(& mp.basis(i))!=NULL,"Bases is not of type gsHTensorBasis<2>");

src/gsMPBESBSplineBasis.hpp:73

  • [nitpick] Similar to the HSpline assertion, the message here is misleadingly plural. Consider updating to "Basis at patch index %zu is not of type gsTensorBSplineBasis<2>".
        GISMO_ASSERT(dynamic_cast<const gsTensorBSplineBasis<2> * >(& mp.basis(i))!=NULL,"Bases is not of type gsTensorBSplineBasis<2>");

src/gsDPatchBase.h:719

  • Storing m_patches by value may introduce expensive copies of the multi-patch object. Consider reverting to a const& if copying is unintended or profiling shows a performance impact.
    const gsMultiPatch<T> m_patches;

src/gsMPBESUtils.h:40

  • [nitpick] Manual clone().release() and freeAll calls risk memory leaks or double-deletes on exceptions. Consider using smart pointers (e.g., std::unique_ptr) or RAII containers to manage basis lifetimes safely.
        freeAll(hBases);

@hverhelst hverhelst merged commit ab7db37 into main Jul 11, 2025
3 checks passed
@hverhelst hverhelst deleted the hotfix branch July 11, 2025 10:34
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.

2 participants