Le 03/07/2016 20:52, Richard Heck a écrit :

My own view is that our code is undercommented---and I know I'm as bad
about this as anyone. That said, I often add comments to code I have to read
to fix various bugs, if only for my own benefit later.

I agree. I noticed myself doing that as well sometimes.


One might wonder in such cases why things are being copied or passed
by reference, for example.

Unfortunately, while producing equivalent code is easy (or so it seems
so far), fully understanding the old code is hard. For instance, I trust
the old code that references are still valid when the callback is called.

Actually, in some cases such as GuiAlert, where the call happens in a
different thread's temporality, the safety benefit of storing by copy
would (barring a better understanding of the assumptions that I do not
have) far outweigh the cost of copying the strings. I might even change
the code in this case for that reason.

So while I agree that comments regarding the old code would help, I
cannot make up this missing information.

No doubt the std::bind versions are write-only, but that only illustrates
my point: Our code is undercommented. In this case, for example:

@@ -741,7 +740,9 @@ void PreviewLoader::Impl::startLoading(bool wait)
      // Initiate the conversion from LaTeX to bitmap images files.
      ForkedCall::SignalTypePtr
          convert_ptr(new ForkedCall::SignalType);
-    convert_ptr->connect(bind(&Impl::finishedGenerating, this, _1, _2));
+    convert_ptr->connect([this](pid_t p, int retval) {
+            finishedGenerating(p, retval);
+        });

      ForkedCall call(buffer_.filePath());
      int ret = call.startScript(command, convert_ptr);

I'm not sure that the original author's decision not to add a comment is
a reason we shouldn't add one.

Do you mean a comment explaining the purpose of the code? Unfortunately
I cannot invent this information. Do you mean a comment describing the
anonymous function? Then adding the types and the names of the
arguments, which I had to look up, can already count as added comments.


I don't have a problem with the patch's being committed. But it's worth
remembering that most of us are amateurs. We don't all keep up with the
latest developments in compiler technology. As a result, discussions
about ways to make our codebase accessible to amateurs aren't at all
uncommon. I knew almost no C++ when I started working on LyX. The
way I learned, and especially the way I learned STL, was by doing.

Keeping the code accessible is something I have in mind. I believe that
my recent modifications involving C++11 features let us write simpler
and safer code.

(Now, for the cultural minute, I would really not call lambda
expressions the "latest development in compiler technology" given that
the lambda calculus is older than computer science, and its efficient
compilation older than C++.)

On a related matter, I have been thinking hard about how to take
advantage of the new move semantics while remaining simple (one issue
being that calling std::move can misleadingly incur a copy). I will
probably write another message about this soon, since I currently have
code the relies on it.


What was the issue with gcc 4.6?

A segfault, that I have since been unable to reproduce, but which I
could not make sense of despite having a backtrace.

Guillaume

Reply via email to