update lingo 2 #6819

berniev · 2022-05-02T11:10:51Z

Another round of fixing simple warnings and other minor improvements.
Apologies for having to have multiple prs :(

Thank you for creating a pull request to contribute to FreeCAD! To ease integration, we ask you to conform to the following items. Pull requests which don't satisfy all the items below might be rejected. If you are in doubt with any of the items below, don't hesitate to ask for help in the FreeCAD forum!

Your pull request is confined strictly to a single module. That is, all the files changed by your pull request are either in App, Base, Gui or one of the Mod subfolders. If you need to make changes in several locations, make several pull requests and wait for the first one to be merged before submitting the next ones
In case your pull request does more than just fixing small bugs, make sure you discussed your ideas with other developers on the FreeCAD forum
Your branch is rebased on latest master git pull --rebase upstream master
All FreeCAD unit tests are confirmed to pass by running ./bin/FreeCAD --run-test 0
All commit messages are well-written ex: Fixes typo in Draft Move command text
Your pull request is well written and has a good description, and its title starts with the module name, ex: Draft: Fixed typos
Commit messages include issue #<id> or fixes #<id> where <id> is the issue ID number from our Issues database in case a particular commit solves or is related to an existing issue. Ex: Draft: fix typos - fixes #4805

And please remember to update the Wiki with the features added or changed once this PR is merged.
Note: If you don't have wiki access, then please mention your contribution on the 0.20 Changelog Forum Thread.

freecadci · 2022-05-02T11:17:07Z

for feature branch PR_6819. Pipeline 529256602 was triggered at 5287905. All CI branches and pipelines.

wwmayer · 2022-05-02T16:40:14Z

src/App/Document.cpp

@@ -494,7 +495,7 @@ void Document::exportGraphviz(std::ostream& out) const
                }
                if(!sgraph) {
                    if(docObj->isDerivedFrom(OriginFeature::getClassTypeId()))
-                        sgraph = GraphList[static_cast<OriginFeature*>(docObj)->getOrigin()];
+                        sgraph = GraphList[dynamic_cast<OriginFeature*>(docObj)->getOrigin()];


See forum discussion

Way above my pay grade. Will reverse and leave alone until I get my head around all that.

wwmayer · 2022-05-02T16:43:35Z

src/App/Document.cpp

-            linked = o->getLinkedObject(false);
-        else {
+        if (options & GetLinkArrayElement) {
+            linked = o->getLinkedObject(true, nullptr, Prop_Output | Prop_Hidden, 0);


This changes the semantic. Before the change the first parameter was false and now it's true.

Oops. I tried to fix not having defaults in virtual, and messed up, then obviously messed up further trying to restore. Will restore.

wwmayer · 2022-05-02T16:49:25Z

src/App/DocumentObjectPyImp.cpp

@@ -712,7 +718,7 @@ PyObject*  DocumentObjectPy::getPathsByOutList(PyObject *args)
    }
 }

-PyObject *DocumentObjectPy::getCustomAttributes(const char* attr) const
+PyObject *DocumentObjectPy::getCustomAttributes(const char* attr)


You cannot simply remove the const here. The change causes a build failure.

Cool. It did build here before I lodged, but not there! But yes, will reverse. And be more careful.

wwmayer · 2022-05-02T16:59:43Z

src/App/DocumentObjectPyImp.cpp

@@ -880,6 +886,6 @@ Py::Boolean DocumentObjectPy::getNoTouch() const {
    return Py::Boolean(getDocumentObjectPtr()->testStatus(ObjectStatus::NoTouch));
 }

-void DocumentObjectPy::setNoTouch(Py::Boolean value) {
+void DocumentObjectPy::setNoTouch(const Py::Boolean& value) {


The header DocumentObjectPy.h is a generated file and the value is passed by value.The Py:: classes are a lightweight wrapper for PyObject similar to a smart pointer.

If this should be changed then the generation script must be changed and with it a lot of source files. If we decide to do it then this has to happen in a separate commit.

OK! Will reverse. 'generated file' answers a couple of questions. Will be more cautious in future.

freecadci · 2022-05-03T08:39:37Z

for feature branch PR_6819. Pipeline 529947210 was triggered at ae100ca. All CI branches and pipelines.

yorikvanhavre · 2022-05-09T09:09:23Z

I guess in any case this is better to handle after the 0.20 release?

wwmayer · 2022-05-10T11:35:33Z

I guess in any case this is better to handle after the 0.20 release?

Yes. I haven't carefully checked every single line and it would be bad to introduce a possible regression because of an overlooked unintentional change.

Also this PR tries to improve too many different things at the same time. IMO it should be split into smaller commits but lets do it after the release.

freecadci · 2022-05-26T03:17:06Z

for feature branch PR_6819. Pipeline 548457336 was triggered at 0429615. All CI branches and pipelines.

berniev · 2022-07-30T06:18:38Z

@wwmayer I acknowledge this commit (and its mates) may have been over-confident for a newbie, and they are getting stale. Am thinking to delete them and start again, a bit wiser, and grateful for your assistance. Would you give some guidance re the better approach?

wwmayer · 2022-07-30T11:13:36Z

Would you give some guidance re the better approach?

I had a quick look at the changes and they fall into the following categories:

constructor/destructor => default, override, explicit, ...
virtual functions => override
static methods
{} initialization
remove redundant void
remove redundant == or != in conditions
replace complex iterator declarations with auto
change of URLs
others (cosmetic changes, ...)

It would be good to split the changes and make one commit for each category.

berniev · 2022-08-09T07:54:55Z

Re: explicit on constructors. What is the chance that there is some code in FreeCAD that relies on implicit conversion and will break? Is there any way to test?

…

On 30 Jul 2022, at 9:13 pm, wwmayer ***@***.***> wrote: Would you give some guidance re the better approach? I had a quick look at the changes and they fall into the following categories: constructor/destructor => default, override, explicit, ... virtual functions => override static methods {} initialization remove redundant void remove redundant == or != in conditions replace complex iterator declarations with auto change of URLs others (cosmetic changes, ...) It would be good to split the changes and make one commit for each category. — Reply to this email directly, view it on GitHub <#6819 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEXAYND6HQGDVS4UT6Y743VWUE6ZANCNFSM5U3VOTYA>. You are receiving this because you authored the thread.

wwmayer · 2022-08-09T10:23:36Z

If you add the keyword explicit to some constructors the worst what can happen is that the build fails -- but that's actually the desired effect, then.

berniev · 2022-08-09T22:30:58Z

Like this you mean?

/Users/bernie/CLionProjects/FreeCAD/src/App/Application.cpp:766:28: error: no viable overloaded operator[] for type 'std::map<DocumentT, DocTiming>'
timings[doc].d1 += timing.d1;
~~~~~~~^~~~
/usr/local/opt/llvm@11/bin/../include/c++/v1/map:1131:18: note: candidate function not viable: no known conversion from 'App::Document *' to 'const std::__1::map<App::DocumentT, DocTiming, std::__1::lessApp::DocumentT, std::__1::allocator<std::__1::pair<const App::DocumentT, DocTiming>>>::key_type' (aka 'const App::DocumentT') for 1st argument
mapped_type& operator[](const key_type& __k);
^
/usr/local/opt/llvm@11/bin/../include/c++/v1/map:1133:18: note: candidate function not viable: no known conversion from 'App::Document *' to 'std::__1::map<App::DocumentT, DocTiming, std::__1::lessApp::DocumentT, std::__1::allocator<std::__1::pair<const App::DocumentT, DocTiming>>>::key_type' (aka 'App::DocumentT') for 1st argument
mapped_type& operator[](key_type&& __k);
^
/Users/bernie/CLionProjects/FreeCAD/src/App/Application.cpp:847:25: error: no matching member function for call to 'count'
if(!newDocs.count(doc)) {
~~~~~~~~^~~~~
/usr/local/opt/llvm@11/bin/../include/c++/v1/set:786:20: note: candidate function not viable: no known conversion from 'App::Document *' to 'const std::__1::set<App::DocumentT, std::__1::lessApp::DocumentT, std::__1::allocatorApp::DocumentT>::key_type' (aka 'const App::DocumentT') for 1st argument
size_type count(const key_type& __k) const
^
/usr/local/opt/llvm@11/bin/../include/c++/v1/set:792:5: note: candidate template ignored: requirement '__is_transparent<std::__1::lessApp::DocumentT, App::Document *, void>::value' was not satisfied [with _K2 = App::Document *]
count(const _K2& __k) const {return _tree.__count_multi(__k);}
^
/Users/bernie/CLionProjects/FreeCAD/src/App/Application.cpp:852:35: error: no viable overloaded operator[] for type 'std::map<DocumentT, DocTiming>'
auto &timing = timings[doc];
~~~~~~~^~~~
/usr/local/opt/llvm@11/bin/../include/c++/v1/map:1131:18: note: candidate function not viable: no known conversion from 'App::Document *' to 'const std::__1::map<App::DocumentT, DocTiming, std::__1::lessApp::DocumentT, std::__1::allocator<std::__1::pair<const App::DocumentT, DocTiming>>>::key_type' (aka 'const App::DocumentT') for 1st argument
mapped_type& operator[](const key_type& __k);
^
/usr/local/opt/llvm@11/bin/../include/c++/v1/map:1133:18: note: candidate function not viable: no known conversion from 'App::Document *' to 'std::__1::map<App::DocumentT, DocTiming, std::__1::lessApp::DocumentT, std::__1::allocator<std::__1::pair<const App::DocumentT, DocTiming>>>::key_type' (aka 'App::DocumentT') for 1st argument
mapped_type& operator[](key_type&& __k);
^

wwmayer · 2022-08-12T05:41:57Z

Like this you mean?

No, this must be something else. There is a problem with the STL implementation of clang under macOS where the key of a map cannot be const. We already had such a case in PropertyExpressionEngine.h where depending on the platform the class member ExpressionMap is declared differently.

berniev · 2022-08-13T00:52:37Z

Actually, this is caused by the addition of explicit where implicit relied upon.

github-actions bot added Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD WB Mesh Related to the Mesh Workbench labels May 2, 2022

wwmayer reviewed May 2, 2022

View reviewed changes

yorikvanhavre added the For 0.21 label May 9, 2022

berniev force-pushed the UpdateLingo2 branch from ae100ca to 38f8a38 Compare May 26, 2022 03:10

berniev added 2 commits May 26, 2022 13:11

update lingo 2

9a1f6b3

update lingo 2

0429615

berniev force-pushed the UpdateLingo2 branch from 38f8a38 to 0429615 Compare May 26, 2022 03:12

chennes removed the For 0.21 label Jun 24, 2022

berniev closed this Jul 31, 2022

berniev deleted the UpdateLingo2 branch July 31, 2022 10:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update lingo 2 #6819

update lingo 2 #6819

berniev commented May 2, 2022

freecadci commented May 2, 2022

wwmayer May 2, 2022

berniev May 3, 2022

wwmayer May 2, 2022

berniev May 3, 2022

wwmayer May 2, 2022

berniev May 3, 2022

wwmayer May 2, 2022

berniev May 3, 2022

freecadci commented May 3, 2022

yorikvanhavre commented May 9, 2022

wwmayer commented May 10, 2022

freecadci commented May 26, 2022

berniev commented Jul 30, 2022

wwmayer commented Jul 30, 2022

berniev commented Aug 9, 2022 via email

wwmayer commented Aug 9, 2022

berniev commented Aug 9, 2022

wwmayer commented Aug 12, 2022

berniev commented Aug 13, 2022

update lingo 2 #6819

update lingo 2 #6819

Conversation

berniev commented May 2, 2022

freecadci commented May 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freecadci commented May 3, 2022

yorikvanhavre commented May 9, 2022

wwmayer commented May 10, 2022

freecadci commented May 26, 2022

berniev commented Jul 30, 2022

wwmayer commented Jul 30, 2022

berniev commented Aug 9, 2022 via email

wwmayer commented Aug 9, 2022

berniev commented Aug 9, 2022

wwmayer commented Aug 12, 2022

berniev commented Aug 13, 2022