Review Request 124427: kdevplatform: Make ctors explicit to avoid implicit construction (krazy2)
Boris Egorov
egorov at linux.com
Fri Jul 24 09:49:53 UTC 2015
> On July 24, 2015, 9:16 a.m., Milian Wolff wrote:
> > LGTM, but what did you mean by "these changes break code with std::move" - can you please clarifiy?
>
> Boris Egorov wrote:
> Tests fail to pass. I removed failing lines, you can see them in diff. They are working with 'moved-from' objects. For example, test them for emptiness.
>
> Milian Wolff wrote:
> OK, I just looked at the diff in more depth (it was too long, sorry for my tl;dr; fail!)
>
> How did the change to add explicit ctors fail these tests? Do you know? I would assume it should be unrelated? Also, some parts of these tests I added deliberately afaik. And testing for emptiness of an moved-from object sounds perfectly fine to me. Why did you remove that?
I don't know how exactly my changes broke tests. But I ran them before and after changes, and they definitely started failing after.
Like I said in commit message, I read in some[1] resources[2] that move-from object remains in valid, but not unspecified state. So we can delete them or assign to them, but we should not do any assumptions about their state or contents. Moved strings can be not empty, for example (I saw exactly this behaviour in failed tests).
I can be wrong, I haven't worked with std::move much.
1: http://en.cppreference.com/w/cpp/utility/move
2: http://stackoverflow.com/questions/20850196/what-lasts-after-using-stdmove-c11/20850223#20850223
- Boris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124427/#review82861
-----------------------------------------------------------
On July 23, 2015, 9:38 a.m., Boris Egorov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124427/
> -----------------------------------------------------------
>
> (Updated July 23, 2015, 9:38 a.m.)
>
>
> Review request for KDevelop.
>
>
> Repository: kdevplatform
>
>
> Description
> -------
>
> Some constructors cannot be made explicit without further code
> modifications (project fails to build). Examples:
>
> IndexedDeclaration
> IndexedQualifiedIdentifier
> ReferencedTopDUContext
> IndexedDUContext
> IndexedTopDUContext
> LocalIndexedDeclaration
>
> Somehow these changes break tests with std::move. Some sources said
> that "moved-from" variables is in unspecified state after moving, so
> let's disable assertions working with such variables.
>
>
> Diffs
> -----
>
> debugger/breakpoint/breakpointdetails.h dff8cf2
> debugger/breakpoint/breakpointmodel.h c12e8fa
> debugger/framestack/framestackmodel.h fa1cbab
> debugger/framestack/framestackwidget.h 9ee5a83
> debugger/interfaces/ibreakpointcontroller.h c39f497
> debugger/interfaces/iframestackmodel.h 01b2545
> debugger/interfaces/ivariablecontroller.h d501a04
> debugger/util/pathmappings.h ffe58f2
> debugger/util/treemodel.h 71704c0
> debugger/variable/variablecollection.h af732c3
> debugger/variable/variablewidget.h 6a47497
> documentation/documentationview.h 6c8cd50
> interfaces/context.h 92f0448
> interfaces/iassistant.h 61b6046
> interfaces/idebugcontroller.h 72e52a4
> interfaces/idocumentcontroller.h 3036cae
> interfaces/ilanguagecontroller.h 04eb259
> interfaces/ipartcontroller.h 4fed10e
> interfaces/iplugincontroller.h b77a8aa
> interfaces/iproject.h f0f38d6
> interfaces/iprojectcontroller.h 86c2dfd
> interfaces/iprojectprovider.h 1e76dbd
> interfaces/iruncontroller.h e342f6c
> interfaces/iselectioncontroller.h fb65377
> interfaces/isession.h 1c1ee0a
> interfaces/isourceformatter.h f35748f
> interfaces/isourceformattercontroller.h efe03c1
> interfaces/launchconfigurationpage.h 2589325
> language/assistant/renameassistant.h cfd3d67
> language/assistant/staticassistant.h 4ca18c3
> language/assistant/staticassistantsmanager.h ab5430e
> language/backgroundparser/backgroundparser.h 4a79651
> language/backgroundparser/documentchangetracker.h bacd0b5
> language/backgroundparser/parseprojectjob.h 82e8b02
> language/backgroundparser/urlparselock.h 6005f9f
> language/codecompletion/codecompletionmodel.h b22cd97
> language/codecompletion/codecompletionworker.h e2fb39b
> language/codecompletion/normaldeclarationcompletionitem.h c19f4b2
> language/codegen/applychangeswidget.h 12fe48d
> language/codegen/archivetemplateloader.h 886c7ec
> language/codegen/astchangeset.h 000e5ae
> language/codegen/basicrefactoring.h 72ae54a
> language/codegen/duchainchangeset.h 0465da3
> language/codegen/sourcefiletemplate.h 4810443
> language/codegen/utilities.h d8e00e6
> language/duchain/aliasdeclaration.h 251c2e5
> language/duchain/builders/dynamiclanguageexpressionvisitor.h f06607a
> language/duchain/classdeclaration.h 128cf05
> language/duchain/classfunctiondeclaration.h fe32111
> language/duchain/classmemberdeclaration.h a9c9487
> language/duchain/declaration.h c5f2450
> language/duchain/declarationid.h dd3c050
> language/duchain/duchainbase.h 3c33e71
> language/duchain/duchaindumper.h 87e91b0
> language/duchain/duchainlock.h 0a16df1
> language/duchain/duchainpointer.h 3fd906f
> language/duchain/ducontextdynamicdata.h c0a8472
> language/duchain/forwarddeclaration.h 3deae92
> language/duchain/functiondeclaration.h e206c54
> language/duchain/functiondefinition.h 237bddb
> language/duchain/identifier.h 37fef57
> language/duchain/localindexedducontext.h a8eae2a
> language/duchain/namespacealiasdeclaration.h 817631a
> language/duchain/navigation/abstractnavigationcontext.h 29d5784
> language/duchain/navigation/problemnavigationcontext.h f19aa18
> language/duchain/navigation/usescollector.h b18f769
> language/duchain/navigation/usesnavigationcontext.h 61ff1fe
> language/duchain/navigation/useswidget.h 94087d7
> language/duchain/parsingenvironment.h f5524ce
> language/duchain/problem.h b8454fb
> language/duchain/tests/test_identifier.cpp 1799946
> language/duchain/topducontextdata.h b9de0df
> language/duchain/topducontextdynamicdata.h 1bdc465
> language/duchain/types/abstracttype.h 7613168
> language/duchain/types/arraytype.h 7a50ec6
> language/duchain/types/constantintegraltype.h 445af1f
> language/duchain/types/containertypes.h b69e335
> language/duchain/types/delayedtype.h 3954009
> language/duchain/types/enumerationtype.h 61b8cfa
> language/duchain/types/enumeratortype.h 3ba420f
> language/duchain/types/functiontype.h f1b15ea
> language/duchain/types/integraltype.h a93820e
> language/duchain/types/pointertype.h 4f336cb
> language/duchain/types/referencetype.h 38c91ad
> language/duchain/types/structuretype.h ccc533f
> language/duchain/types/typealiastype.h 7eed573
> language/duchain/types/unsuretype.h 5d504b4
> language/duchain/use.h 5eff1f4
> language/editor/modificationrevision.h 1fdff87
> language/editor/modificationrevisionset.h 4093780
> language/highlighting/codehighlighting.h 76f7f01
> language/highlighting/configurablecolors.h 4a65172
> language/interfaces/codecontext.h 72b5a4a
> language/util/basicsetrepository.h 19faedb
> language/util/kdevhash.h 4ac8c0d
> outputview/outputdelegate.h adbc275
> outputview/outputmodel.h 8ec09e7
> plugins/appwizard/appwizardpagewidget.h 1d6e105
> plugins/appwizard/appwizardplugin.h a59c187
> plugins/appwizard/projecttemplatesmodel.h e9837fc
> plugins/appwizard/projectvcspage.h f587c86
> plugins/classbrowser/allclassesfolder.h 94b16da
> plugins/classbrowser/classbrowserplugin.h 0bcb8ff
> plugins/classbrowser/classmodelnode.h b1e7308
> plugins/classbrowser/projectfolder.h b721ee1
> plugins/codeutils/codeutilsplugin.h ca12e2a
> plugins/contextbrowser/browsemanager.h 51166cd
> plugins/contextbrowser/contextbrowser.h 09066cd
> plugins/cvs/commitdialog.h cba07c0
> plugins/cvs/cvsannotatejob.h ed6a3d9
> plugins/cvs/cvsdiffjob.h 7e9feae
> plugins/cvs/cvsjob.h 474381e
> plugins/cvs/cvslogjob.h ad32982
> plugins/cvs/cvsproxy.h 4cab04c
> plugins/cvs/cvsstatusjob.h 4eafff5
> plugins/cvs/importmetadatawidget.h 9c776f1
> plugins/documentswitcher/documentswitcherplugin.h e85a54d
> plugins/documentswitcher/documentswitchertreeview.h cba65c8
> plugins/documentview/kdevdocumentmodel.h d851f5a
> plugins/documentview/kdevdocumentselection.h 832484c
> plugins/execute/executeplugin.h 3e379b1
> plugins/execute/nativeappconfig.h 4eacf1e
> plugins/execute/projecttargetscombobox.h b29a7df
> plugins/executescript/executescriptplugin.h f4b2e60
> plugins/executescript/scriptappconfig.h 66dd716
> plugins/externalscript/editexternalscript.h 2b26ca7
> plugins/externalscript/externalscriptplugin.h 706c55b
> plugins/externalscript/externalscriptview.h f54f062
> plugins/filemanager/kdevfilemanagerplugin.h e543106
> plugins/filetemplates/classidentifierpage.h 8ce3da8
> plugins/filetemplates/licensepage.h 0134afb
> plugins/filetemplates/outputpage.h f01d392
> plugins/filetemplates/overridespage.h db4863a
> plugins/filetemplates/templateclassassistant.h 76e60f1
> plugins/filetemplates/templatepreview.h 0324fb5
> plugins/git/gitjob.h 8d0a8f2
> plugins/git/gitplugin.h 6bafc59
> plugins/grepview/grepjob.h cb4e81f
> plugins/grepview/grepoutputview.h 95c83dc
> plugins/grepview/grepviewplugin.h 68dfdb2
> plugins/outlineview/outlinenode.h 0e8d3f5
> plugins/patchreview/patchreview.h d7127b0
> plugins/problemreporter/problemhighlighter.h 8d566ab
> plugins/problemreporter/problemnavigationcontext.h e761d14
> plugins/problemreporter/problemreporterplugin.h 1014429
> plugins/problemreporter/problemsview.h ced44f2
> plugins/projectmanagerview/projectbuildsetwidget.h e843d3f
> plugins/projectmanagerview/projectmanagerviewplugin.h b180d25
> plugins/projectmanagerview/projecttreeview.h 6512157
> plugins/quickopen/duchainitemquickopen.h af28afe
> plugins/quickopen/expandingtree/expandingtree.h 17daa39
> plugins/quickopen/expandingtree/expandingwidgetmodel.h 6d9618e
> plugins/quickopen/projectfilequickopen.h 06c483d
> plugins/quickopen/projectitemquickopen.h 5651375
> plugins/quickopen/quickopenmodel.h 32e3e98
> plugins/quickopen/quickopenplugin.h 8449f07
> plugins/standardoutputview/toolviewdata.h a082f9a
> plugins/subversion/svnaddjob.h 03a1902
> plugins/subversion/svnaddjob_p.h 456ed96
> plugins/subversion/svnblamejob.h 562e0e8
> plugins/subversion/svnblamejob_p.h ebb77ff
> plugins/subversion/svncatjob.h 6fab39b
> plugins/subversion/svncatjob_p.h ee488fe
> plugins/subversion/svncheckoutjob.h a5b8f02
> plugins/subversion/svncheckoutjob_p.h 836e641
> plugins/subversion/svncheckoutmetadatawidget.h aec9cef
> plugins/subversion/svnclient.h 56fe837
> plugins/subversion/svncommitjob_p.h ae00109
> plugins/subversion/svncopyjob.h d146c77
> plugins/subversion/svncopyjob_p.h 8dd1d2c
> plugins/subversion/svndiffjob.h 2d9b099
> plugins/subversion/svndiffjob_p.h fed5385
> plugins/subversion/svnimportjob.h 1b4efee
> plugins/subversion/svnimportmetadatawidget.h 25d178b
> plugins/subversion/svninfojob.h 436bc3f
> plugins/subversion/svninfojob_p.h cb61e75
> plugins/subversion/svninternaljobbase.h 4f2940e
> plugins/subversion/svnjobbase.h 5f622cf
> plugins/subversion/svnlocationwidget.h 9c927ce
> plugins/subversion/svnlogjob.h dbd7c07
> plugins/subversion/svnlogjob_p.h c198c43
> plugins/subversion/svnmovejob.h b52c1ad
> plugins/subversion/svnmovejob_p.h 9055033
> plugins/subversion/svnremovejob.h d5dbc3d
> plugins/subversion/svnremovejob_p.h b0e8c41
> plugins/subversion/svnrevertjob.h 0c9d585
> plugins/subversion/svnrevertjob_p.h a385aa6
> plugins/subversion/svnssldialog.h 6ccc9b8
> plugins/subversion/svnstatusjob_p.h 2477ed1
> plugins/subversion/svnupdatejob.h 5e00b77
> plugins/subversion/svnupdatejob_p.h 31fcece
> plugins/switchtobuddy/switchtobuddyplugin.h a7b566c
> plugins/testview/testviewplugin.h 0fe8df1
> plugins/welcomepage/declarative/icoreobject.h 1244b56
> plugins/welcomepage/uihelper.h 78b1df6
> plugins/welcomepage/welcomepageview.h 1aee0e7
> project/projectbuildsetmodel.h 920ac1d
> project/projectchangesmodel.h 7fd84c9
> project/projectconfigskeleton.h 80e118b
> project/projectitemlineedit.h cda1807
> project/projectmodel.h 22ac4fa
> project/projectproxymodel.h 6feba14
> project/projectutils.h 03fce30
> serialization/indexedstring.h 7e06cab
> serialization/itemrepositoryregistry.h 7185555
> serialization/tests/test_indexedstring.cpp f0c7958
> shell/core_p.h 3caed06
> shell/debugcontroller.h a609e33
> shell/documentationcontroller.h 0be4b2e
> shell/documentcontroller.h 4c74860
> shell/editorconfigpage.h 593523b
> shell/filteredproblemstore.h 25c29d3
> shell/languagecontroller.h 28d1e7e
> shell/launchconfiguration.h 692bf5c
> shell/launchconfigurationdialog.h 799c844
> shell/loadedpluginsdialog.h 699475a
> shell/mainwindow_p.h fd23597
> shell/openprojectpage.h 5602b74
> shell/plugincontroller.h d4f48a6
> shell/problemmodel.h 48b7f1f
> shell/problemmodelset.h 5aa2836
> shell/problemstore.h c1426c0
> shell/problemstorenode.h 05844c2
> shell/project.h 49d69a4
> shell/projectcontroller.h 30db285
> shell/projectinfopage.h c58004b
> shell/projectsourcepage.h 80c2cfb
> shell/runcontroller.h 9e5ee2a
> shell/selectioncontroller.h ad77f85
> shell/session.h 2b5a09f
> shell/sessioncontroller.h db5c7d0
> shell/sessiondialog.h 8e97dfb
> shell/settings/sessionconfigskeleton.h 8c76493
> shell/sourceformattercontroller.h fcf181f
> shell/statusbar.h 295cc92
> shell/textdocument.h 64952e0
> shell/uicontroller.h 677ff3f
> shell/workingsets/workingset.h dd301c2
> shell/workingsets/workingsetwidget.h fdc1db9
> sublime/aggregatemodel.h c9d0caf
> sublime/container.h fecd865
> sublime/controller.h 61883d2
> sublime/holdupdates.h ca0ee37
> sublime/idealcontroller.h 8541191
> sublime/ideallayout.h 4baac33
> sublime/idealtoolbutton.h e8b2baf
> sublime/mainwindow_p.h 4a18f4c
> util/duchainify/main.h c0304ef
> util/environmentgrouplist.h 0412c5e
> util/executecompositejob.h 26d5617
> util/focusedtreeview.h f3bf065
> util/foregroundlock.h 3d994da
> util/objectlist.h c59b487
> util/processlinemaker.h 6a53922
> vcs/dvcs/dvcsjob.h 81c6f33
> vcs/dvcs/ui/dvcsimportmetadatawidget.h c037be2
> vcs/dvcs/ui/revhistory/commitView.h 046cd5f
> vcs/interfaces/icontentawareversioncontrol.h 2ef5882
> vcs/models/brancheslistmodel.h f96255d
> vcs/models/vcsfilechangesmodel.h e77aa1a
> vcs/models/vcsitemeventmodel.h 61966e4
> vcs/vcsjob.h 95af667
> vcs/vcslocation.h 3a270e8
> vcs/widgets/standardvcslocationwidget.h b1efd40
> vcs/widgets/vcscommitdialog.h 85f8d59
> vcs/widgets/vcsdiffpatchsources.h 6ca06f8
> vcs/widgets/vcsdiffwidget.h 088b073
> vcs/widgets/vcsimportmetadatawidget.h c088e39
> vcs/widgets/vcslocationwidget.h 1d921a0
>
> Diff: https://git.reviewboard.kde.org/r/124427/diff/
>
>
> Testing
> -------
>
> Builds, passes tests
>
>
> Thanks,
>
> Boris Egorov
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20150724/dae1683b/attachment-0001.html>
More information about the KDevelop-devel
mailing list