Bug 164685 - Cleanup class SfxItemSetFixed
Summary: Cleanup class SfxItemSetFixed
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium trivial
Assignee: Not Assigned
URL:
Whiteboard: target:25.8.0
Keywords: difficultyMedium, easyHack, skillCpp, topicCleanup
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2025-01-13 12:46 UTC by Armin Le Grand (allotropia)
Modified: 2025-01-30 23:58 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Armin Le Grand (allotropia) 2025-01-13 12:46:34 UTC
Description:
See comment on it in include/svl/itemset.hxx:

// Was: Allocate the items array inside the object, to reduce allocation cost.
// NOTE: No longer needed with unordered_set, but there are 560+ places to
// replace/adapt, so keep it for now.
// To adapt, there is the static makeFixedSfxItemSet member in SfxItemSet. Did
// that in one place to check functionality (easy hack...?)

Thus: all usages where a SfxItemSetFixed is constructed should be replaced using SfxItemSet::makeFixedSfxItemSet. As an example there is one place where I did that as POC: in sc/source/ui/view/cellsh3.cxx line 478. It is now

SfxItemSet aEmptySet(SfxItemSet::makeFixedSfxItemSet<ATTR_PATTERN_START, ATTR_PATTERN_END>(*pReqArgs->GetPool()));

and it originally was

SfxItemSetFixed<ATTR_PATTERN_START, ATTR_PATTERN_END>  aEmptySet( *pReqArgs->GetPool() );

thus it is mainly just a re-arrangement. Maybe someone with skills could do that even using a script or similar...

Actual Results:
class SfxItemSetFixed still exists.

Expected Results:
class SfxItemSetFixed can be completely removed from the code base.


Reproducible: Always


User Profile Reset: No

Additional Info:
This is an easy hack that needs C++ knowledge and maybe some scripting experience.
Comment 1 Armin Le Grand (allotropia) 2025-01-13 14:45:05 UTC
Note: This is not complicated, but unfortunately a whole bunch of places - when grepping for SfxItemSetFixed I get 575 results in 278 files. With some in comments and the class definition itself it still is a high number. Maybe doing this i several steps is feasible. Or - as mentioned - someone with scripting/meta language could do it in one run...
Comment 2 Rafael Lima 2025-01-14 19:27:50 UTC
Hi Armin,

I'm looking into this... I created a Python script using regex to make all the substitutions and it works around 90% of the times.

However, I came across a few lines where we have unique pointers such as:

In: sc/source/ui/drawfunc/drawsh.cxx

auto xItemSet = std::make_unique<SfxItemSetFixed<SID_ATTR_MACROITEM, SID_ATTR_MACROITEM, SID_EVENTCONFIG, SID_EVENTCONFIG>>( SfxGetpApp()->GetPool() );

What is the best approach to deal with these cases? This is how I'm doing the conversion:

std::unique_ptr<SfxItemSet> xItemSet = std::make_unique<SfxItemSet>(SfxItemSet::makeFixedSfxItemSet<SID_ATTR_MACROITEM, SID_ATTR_MACROITEM, SID_EVENTCONFIG, SID_EVENTCONFIG>(SfxGetpApp()->GetPool()));

Is this correct?
Comment 3 Commit Notification 2025-01-20 11:38:44 UTC
rafaelhlima committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/7afd95daf2224e80fe57c62f819dfb9ec4c630c9

tdf#164685 Cleanup SfxItemSetFixed

It will be available in 25.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 4 Commit Notification 2025-01-30 23:58:56 UTC
rafaelhlima committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/80c90897657896b94689513119bf05eafc21519b

tdf#164685 Cleanup SfxItemSetFixed - Part 2

It will be available in 25.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.