yaml-cpp/node/impl.h:206 error 'operator=' should return a reference to *this [-Werror=effc++]

[tomeravi]

on Linux build

[btbouwens]

Indeed, I also see this:

In file included from /workspace/software/lib/3rdparty/yaml-cpp/src/parse.cpp:7:
/workspace/software/lib/3rdparty/yaml-cpp/include/yaml-cpp/node/impl.h: In member function 'YAML::Node& YAML::Node::operator=(const T&)':
/workspace/software/lib/3rdparty/yaml-cpp/include/yaml-cpp/node/impl.h:206:10: warning: 'operator=' should return a reference to '*this' [-Weffc++]
  206 |   return *this;
      |          ^~~~~

[tomeravi]

Do u have any suggestion how to solve it ? T בתאריך יום ה׳, 7 בינו׳ 2021, 12:39, מאת btbouwens ‏<notifications@github.com

…

: Indeed, I also see this: In file included from /workspace/software/lib/3rdparty/yaml-cpp/src/parse.cpp:7: /workspace/software/lib/3rdparty/yaml-cpp/include/yaml-cpp/node/impl.h: In member function 'YAML::Node& YAML::Node::operator=(const T&)': /workspace/software/lib/3rdparty/yaml-cpp/include/yaml-cpp/node/impl.h:206:10: warning: 'operator=' should return a reference to '*this' [-Weffc++] 206 | return *this; | ^~~~~ — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#970 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJQIFLK3TDKGD2JX5NMZJ4LSYWFPRANCNFSM4VTIXVVQ> .

[btbouwens]

No, I don't understand the problem. It looks OK to me actually.
I've seen comments that the -Weffc++ is actually questionable, but the arguments are outside my competence ...

[olafmandel]

I opened a GCC bug #98841 about this.

[olafmandel]

Jakub Jelinek wrote a fix for GCC for this, which "just" has to make it into a future release of GCC.

If I understand the explanation of the bug correctly, there is nothing that can be done to the yaml-cpp code to workaround the warning for older compilers, short of disabling the warning:

diff --git a/include/yaml-cpp/node/impl.h b/include/yaml-cpp/node/impl.h
--- a/include/yaml-cpp/node/impl.h
+++ b/include/yaml-cpp/node/impl.h
@@ -209,7 +209,15 @@ inline Node& Node::operator=(const T& rhs) {
   if (!m_isValid)
     throw InvalidNode(m_invalidKey);
   Assign(rhs);
+// work around GCC PR c++/98841
+#if defined __GNUC__ && __GNUC__ < 11
+#  pragma GCC diagnostic push
+#  pragma GCC diagnostic ignored "-Weffc++"
+#endif
   return *this;
+#if defined __GNUC__ && __GNUC__ < 11
+#  pragma GCC diagnostic pop
+#endif
 }

 inline Node& Node::operator=(const Node& rhs) {

Of course, that distracts a lot when reading the code. Including this or not may also depend on when the fix gets released and how long it takes for the developer-base of yaml-cpp to upgrade...

[christianparpart]

Jakub Jelinek wrote a fix for GCC for this, which "just" has to make it into a future release of GCC.

If I understand the explanation of the bug correctly, there is nothing that can be done to the yaml-cpp code to workaround the warning for older compilers, short of disabling the warning:

diff --git a/include/yaml-cpp/node/impl.h b/include/yaml-cpp/node/impl.h
--- a/include/yaml-cpp/node/impl.h
+++ b/include/yaml-cpp/node/impl.h
@@ -209,7 +209,15 @@ inline Node& Node::operator=(const T& rhs) {
   if (!m_isValid)
     throw InvalidNode(m_invalidKey);
   Assign(rhs);
+// work around GCC PR c++/98841
+#if defined __GNUC__ && __GNUC__ < 11
+#  pragma GCC diagnostic push
+#  pragma GCC diagnostic ignored "-Weffc++"
+#endif
   return *this;
+#if defined __GNUC__ && __GNUC__ < 11
+#  pragma GCC diagnostic pop
+#endif
 }

 inline Node& Node::operator=(const Node& rhs) {

Of course, that distracts a lot when reading the code. Including this or not may also depend on when the fix gets released and how long it takes for the developer-base of yaml-cpp to upgrade...

Can we get this patch applied to the yaml-cpp repo, please? I'd like to be able to compile my own code without any warnings (-Werror) and can't because of this. :)
Maintaining custom patches per dependency is also cumbersome.

[jbeder]

Is there a PR to fix this? If so, I'll review it.

But it's totally not worth it to spam with a bunch of compiler-specific declarations just to fix a warning.

[jasonbeach]

maybe one option is to remove the WeffC++ option when the project is not the top level project.
Added that to my PR.

[BMBurstein]

Is there a solution for this? Our project compiles with -Wall -Wextra -Werror, and we were looking into adding yaml-cpp as a dependency, but this broke our build

[olafmandel]

Yes and no:

• the GCC bug #98841 is long since closed and at least the current GCC 11 and GCC 12 compilers don't have the problem
• the pull request give YAML_CPP_INSTALL a default value that can be overridden #1011 also contained the -Weffc++ fix mentioned in a previous comment, but was since closed unmerged because its main point was obviated by a different change
• there is no active pull request as far as I know

So the question for the project maintainers is:

• do they want to fill the code with such preprocessor-macros
• or do they want to remove the -Weffc++ compiler flag (in some situations)
• or do they want to declare this wont-fix
• or can this issue (eventually) be closed as no longer relevant

[BMBurstein]

Our project is compiling on Ubuntu 20.04 (among others), which has GCC 9

[BMBurstein]

I think a solution would be to disable all these extra warnings if this is being compiled as part of another project instead of as the main project. No need to block a project from using this library because of warnings

[christianparpart]

It may be just warnings (for whatever reason), but some projects build with all warnings enabled and then use -Werror error to force the dev to fix them. Now, when you use yaml-cpp, it's kinda hard, either you live in a strict world, or you can use yaml-cpp. Is there a solution to this?

[jbeder]

To answer questions from @olafmandel above:

do they want to fill the code with such preprocessor-macros

Ideally, no.

or do they want to remove the -Weffc++ compiler flag (in some situations)

Maybe? Depending on what that looks like.

or do they want to declare this wont-fix
or can this issue (eventually) be closed as no longer relevant

Fine with me. I don't particularly care about warnings, personally.