Bump cppcheck from 2.3 to 2.9 #570

jmarrec · 2022-10-18T08:48:21Z

No description provided.

…oved at v1.0.0: 9ba19ca

macumber · 2022-11-20T20:39:48Z

Any idea why the cppcheck build started failing?

macumber · 2022-11-20T20:41:03Z

Want some help getting this to pass?

``` run-clang-tidy -config-file=../OpenStudioApplication/src/.clang-tidy -header-filter='OpenStudioApplication/src.*' -checks=-*,google-explicit-constructor -fix -format -style=file ```

…structor warning

jmarrec · 2022-11-22T12:13:28Z

@macumber Only one type of warning left, the virtualCallInConstructor one. What do we do about this one?

src/model_editor/InspectorGadget.hpp:274]:(style),[virtualCallInConstructor],
Virtual function 'layoutItems' is called from constructor
'InspectorGadget(openstudio::WorkspaceObject&workspaceObj,int indent,ComboHighlightBridge*bridge,bool showComments,bool showAllFields,bool recursive,bool locked)' at line 158. 
Dynamic binding is not used.

macumber · 2022-11-22T14:51:28Z

Can we just make InspectorGadget::layoutItems non-virtual? It doesn't look like it is overridden anywhere.

jmarrec · 2022-11-22T17:15:46Z

layoutItems

In this specific case it's fine. This wasn't perhaps the best of examples, the most common one is "setCategoriesAndFields", which is in the base class OSGridController and all derives classes override it and call it from the ctor.

OpenStudioApplication/src/shared_gui_components/OSGridController.hpp

Lines 528 to 531 in a2feef7

    
           // This function determines the category for 
        
           // each button, and the fields associated with 
        
           // each category 
        
           virtual void setCategoriesAndFields();

OpenStudioApplication/src/openstudio_lib/SpacesShadingGridView.cpp

Lines 111 to 130 in a2feef7

    
           SpacesShadingGridController::SpacesShadingGridController(bool isIP, const QString& headerText, IddObjectType iddObjectType, const model::Model& model, 
        
                                                                    const std::vector<model::ModelObject>& modelObjects) 
        
             : OSGridController(isIP, headerText, iddObjectType, model, modelObjects) { 
        
             setCategoriesAndFields(); 
        
           } 
        
           void SpacesShadingGridController::setCategoriesAndFields() { 
        
             { 
        
               std::vector<QString> fields; 
        
               fields.push_back(SHADEDSURFACENAME); 
        
               fields.push_back(SHADINGSURFACEGROUP); 
        
               fields.push_back(CONSTRUCTION); 
        
               fields.push_back(TRANSMITTANCESCHEDULE); 
        
               //fields.push_back(DAYLIGHTINGSHELFNAME); 
        
               std::pair<QString, std::vector<QString>> categoryAndFields = std::make_pair(QString("General"), fields); 
        
               addCategoryAndFields(categoryAndFields); 
        
             } 
        
             OSGridController::setCategoriesAndFields(); 
        
           }

The clean way would maybe be to have a private setCategoriesAndFieldInternal method that isn't virtual and that's the one both the Ctor and the setCategoriesAndFields would call. But that's an annoying code change

macumber · 2022-11-22T17:44:42Z

Ok I see, well IMHO this is an important check so we should probably try to fix these issues. If you want, I can do the remaining changes.

jmarrec · 2022-11-22T17:49:31Z

I guess setCategoriesAndFields isn't a virtual method per se actually.

The child setCategoriesAndFields explicitly calls the base OSGridController::setCategoriesAndFields
I don't see anywhere setCategoriesAndFields being called from an OSGridController base (but could be wrong)

```python from pathlib import Path import re ROOT_DIR = Path('.').resolve().parent cpp_files = list(Path(ROOT_DIR / 'src').glob('**/*.cpp')) for cpp_file in cpp_files: replacement_done = False with open(cpp_file, 'r') as f: content = f.read() lines = content.splitlines() new_lines = [] in_block = False for i, line in enumerate(lines): if 'std::vector<QString> fields;' in line: replacement_done = True in_block = True new_lines.append(' std::vector<QString> fields{') elif not in_block: new_lines.append(line) elif 'fields.push_back' in line: new_lines.append(line.replace('fields.push_back(', '').replace(');', ',')) else: new_lines.append(' };') new_lines.append(line) in_block = False if not replacement_done: continue with open(cpp_file, 'w') as f: f.write("\n".join(new_lines) + '\n') ```

jmarrec · 2022-11-22T19:10:48Z

@macumber I cleaned up the rest of the cppcheck warnings... I suppose I could have "unvirtualized" a few more functions, but I was getting pretty annoyed by the end. If you could spot check the last commits and do a quick UI test session that'd be great

macumber · 2022-11-22T20:59:58Z

Sounds good, will do!

macumber · 2022-11-23T16:09:31Z

There seems to be a crash when going to the schedule tab, I'll get a debug build of your branch going and see if I can find it.

I can look later but here is the call stack for the crash. To reproduce, load the example model and then go to the schedules tab.

openstudiolib.dll!std::_Ref_count_base::_Incref() Line 1100	C++
openstudiolib.dll!std::_Ptr_base<openstudio::detail::IdfObject_Impl>::_Incref() Line 1335	C++
openstudiolib.dll!std::_Ptr_base<openstudio::detail::IdfObject_Impl>::_Copy_construct_from<openstudio::detail::IdfObject_Impl>(const std::shared_ptr<openstudio::detail::IdfObject_Impl> & _Other) Line 1293	C++
openstudiolib.dll!std::shared_ptr<openstudio::detail::IdfObject_Impl>::shared_ptr<openstudio::detail::IdfObject_Impl>(const std::shared_ptr<openstudio::detail::IdfObject_Impl> & _Other) Line 1575	C++
openstudiolib.dll!openstudio::IdfObject::IdfObject(const openstudio::IdfObject & other) Line 2031	C++
openstudiolib.dll!openstudio::WorkspaceObject::WorkspaceObject(const openstudio::WorkspaceObject & __that)	C++
openstudiolib.dll!openstudio::model::ModelObject::ModelObject(const openstudio::model::ModelObject & __that)	C++
openstudiolib.dll!openstudio::model::ParentObject::ParentObject(const openstudio::model::ParentObject & __that)	C++
openstudiolib.dll!openstudio::model::ResourceObject::ResourceObject(const openstudio::model::ResourceObject & __that)	C++
openstudiolib.dll!openstudio::model::ScheduleBase::ScheduleBase(const openstudio::model::ScheduleBase & __that)	C++
openstudiolib.dll!openstudio::model::Schedule::Schedule(const openstudio::model::Schedule & __that)	C++
openstudiolib.dll!openstudio::model::ScheduleRuleset::ScheduleRuleset(const openstudio::model::ScheduleRuleset & __that)	C++
**OpenStudioApp.exe!openstudio::ScheduleTab::schedule() Line 670	C++**
OpenStudioApp.exe!openstudio::ScheduleTabHeader::ScheduleTabHeader(openstudio::ScheduleTab * scheduleTab, QWidget * parent) Line 783	C++
OpenStudioApp.exe!openstudio::ScheduleTab::ScheduleTab(const openstudio::model::ScheduleRuleset & schedule, openstudio::SchedulesView * schedulesView, QWidget * parent) Line 613	C++
OpenStudioApp.exe!openstudio::SchedulesView::addSchedule(const openstudio::model::ScheduleRuleset & schedule) Line 250	C++
OpenStudioApp.exe!openstudio::SchedulesView::SchedulesView(bool isIP, const openstudio::model::Model & model) Line 189	C++
OpenStudioApp.exe!openstudio::SchedulesTabController::setSubTab(int index) Line 469	C++

macumber · 2022-11-23T20:08:36Z

src/openstudio_lib/SchedulesView.cpp

@@ -613,14 +610,15 @@ ScheduleTab::ScheduleTab(const model::ScheduleRuleset& schedule, SchedulesView*
  : QWidget(parent),
    //m_mouseDown(false),
    m_selected(false),
+    m_header(new ScheduleTabHeader(this)),


@jmarrec there is an issue here because ScheduleTabHeader::ScheduleTabHeader tries to call SchedulesView::schedule before m_schedule is set. I fixed that but hit another error. I think some of these changes to move construction from ctor body to initializer list might not be ok in this file at least.

I was just running the same inside LLDB right now, came up to the same conclusion about the order of members being initialized, and the fact that the ScheduleTabHeader ctor takes this

Bump cppcheck from 2.3 to 2.9

e17c567

jmarrec added the Developer Issue 🤓 label Oct 18, 2022

jmarrec self-assigned this Oct 18, 2022

jmarrec added 3 commits October 18, 2022 15:04

Remove ScriptFolderListView, which uses ScriptsListView which was rem…

30bf3fd

…oved at v1.0.0: 9ba19ca

Start fixing cppcheck warnings (+ modernize code at the same time)

43a4eaa

More fixes + modernizing

79dfe36

jmarrec added 5 commits November 21, 2022 17:37

Merge remote-tracking branch 'origin/run-clang-tidy' into cppcheck_bump

4b80331

Merge remote-tracking branch 'origin/develop' into cppcheck_bump

eb548ff

Make ctors explicit via google-explicit-constructor (and manual fixes)

9a13125

``` run-clang-tidy -config-file=../OpenStudioApplication/src/.clang-tidy -header-filter='OpenStudioApplication/src.*' -checks=-*,google-explicit-constructor -fix -format -style=file ```

Misc cppcheck fixes

1306e6b

Random fix, while I investigate how to deal with the virtualCallInCon…

402bb6b

…structor warning

Merge remote-tracking branch 'origin/develop' into cppcheck_bump

a23fce9

jmarrec force-pushed the cppcheck_bump branch from 468a62b to a23fce9 Compare November 22, 2022 12:15

jmarrec added 4 commits November 22, 2022 13:50

Unvirtualize InspectGadget: it's not needed

33c9390

setCategoriesAndFields doesn't need to be virtual

c39646e

cleanup rest of the cppcheck 2.9 warnings

a0f9ff6

macumber reviewed Nov 23, 2022

View reviewed changes

macumber added 2 commits November 23, 2022 13:12

Fix crash in schedules tab and update translations

fb9ab00

clang-format

feb5eca

macumber added 2 commits November 23, 2022 17:13

Merge remote-tracking branch 'remotes/origin/develop' into cppcheck_bump

062754a

Fix merge issue

7acb083

macumber merged commit 2d19d49 into develop Nov 24, 2022

macumber deleted the cppcheck_bump branch November 24, 2022 16:11

github-actions bot locked and limited conversation to collaborators Nov 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump cppcheck from 2.3 to 2.9 #570

Bump cppcheck from 2.3 to 2.9 #570

jmarrec commented Oct 18, 2022

macumber commented Nov 20, 2022

macumber commented Nov 20, 2022

jmarrec commented Nov 22, 2022 •

edited

Loading

macumber commented Nov 22, 2022

jmarrec commented Nov 22, 2022 •

edited

Loading

macumber commented Nov 22, 2022

jmarrec commented Nov 22, 2022

jmarrec commented Nov 22, 2022 •

edited

Loading

macumber commented Nov 22, 2022

macumber commented Nov 23, 2022 •

edited

Loading

macumber Nov 23, 2022

jmarrec Nov 23, 2022

Bump cppcheck from 2.3 to 2.9 #570

Bump cppcheck from 2.3 to 2.9 #570

Conversation

jmarrec commented Oct 18, 2022

macumber commented Nov 20, 2022

macumber commented Nov 20, 2022

jmarrec commented Nov 22, 2022 • edited Loading

macumber commented Nov 22, 2022

jmarrec commented Nov 22, 2022 • edited Loading

macumber commented Nov 22, 2022

jmarrec commented Nov 22, 2022

jmarrec commented Nov 22, 2022 • edited Loading

macumber commented Nov 22, 2022

macumber commented Nov 23, 2022 • edited Loading

macumber Nov 23, 2022

Choose a reason for hiding this comment

jmarrec Nov 23, 2022

Choose a reason for hiding this comment

jmarrec commented Nov 22, 2022 •

edited

Loading

jmarrec commented Nov 22, 2022 •

edited

Loading

jmarrec commented Nov 22, 2022 •

edited

Loading

macumber commented Nov 23, 2022 •

edited

Loading