namecoin / namecoin-core

Namecoin full node + wallet based on the current Bitcoin Core codebase.
https://www.namecoin.org/
MIT License
455 stars 147 forks source link

Namecoin Core 23.0 fails compiling Qt #515

Open ghost opened 2 years ago

ghost commented 2 years ago
In file included from qt/buynamespage.cpp:1:
./qt/buynamespage.h:33:57: error: ‘optional’ in namespace ‘std’ does not name a template type
   33 |     QString firstupdate(const QString &name, const std::optional<QString> &value, const std::optional<QString> &transferTo) const;
      |                                                         ^~~~~~~~
./qt/buynamespage.h:7:1: note: ‘std::optional’ is defined in header ‘<optional>’; did you forget to ‘#include <optional>’?
    6 | #include <QWidget>
  +++ |+#include <optional>
    7 | 
./qt/buynamespage.h:33:65: error: expected ‘,’ or ‘...’ before ‘<’ token
   33 |     QString firstupdate(const QString &name, const std::optional<QString> &value, const std::optional<QString> &transferTo) const;
      |                                                                 ^
In file included from qt/buynamespage.cpp:1:
./qt/buynamespage.h:36:10: warning: ‘virtual bool BuyNamesPage::eventFilter(QObject*, QEvent*)’ can be marked override [-Wsuggest-override]
   36 |     bool eventFilter(QObject *object, QEvent *event);
      |          ^~~~~~~~~~~
qt/buynamespage.cpp: In member function ‘void BuyNamesPage::onRegisterNameAction()’:
qt/buynamespage.cpp:71:46: error: no matching function for call to ‘BuyNamesPage::firstupdate(QString&, const QString&, const std::optional<QString>&)’
   71 |     const QString err_msg = this->firstupdate(name, newValue, transferToAddress);
      |                             ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from qt/buynamespage.cpp:1:
./qt/buynamespage.h:33:13: note: candidate: ‘QString BuyNamesPage::firstupdate(const QString&, int) const’
   33 |     QString firstupdate(const QString &name, const std::optional<QString> &value, const std::optional<QString> &transferTo) const;
      |             ^~~~~~~~~~~
./qt/buynamespage.h:33:13: note:   candidate expects 2 arguments, 3 provided
qt/buynamespage.cpp: At global scope:
qt/buynamespage.cpp:83:9: error: no declaration matches ‘QString BuyNamesPage::firstupdate(const QString&, const std::optional<QString>&, const std::optional<QString>&) const’
   83 | QString BuyNamesPage::firstupdate(const QString &name, const std::optional<QString> &value, const std::optional<QString> &transferTo) const
      |         ^~~~~~~~~~~~
In file included from qt/buynamespage.cpp:1:
./qt/buynamespage.h:33:13: note: candidate is: ‘QString BuyNamesPage::firstupdate(const QString&, int) const’
   33 |     QString firstupdate(const QString &name, const std::optional<QString> &value, const std::optional<QString> &transferTo) const;
      |             ^~~~~~~~~~~
./qt/buynamespage.h:18:7: note: ‘class BuyNamesPage’ defined here
   18 | class BuyNamesPage : public QWidget
      |       ^~~~~~~~~~~~
make[2]: *** [Makefile:12469: qt/libbitcoinqt_a-buynamespage.o] Error 1
make[2]: Leaving directory '/home/hacker/namecoin-core-nc23.0/src'
make[1]: *** [Makefile:17862: all-recursive] Error 1
make[1]: Leaving directory '/home/hacker/namecoin-core-nc23.0/src'
make: *** [Makefile:815: all-recursive] Error 1
domob1812 commented 2 years ago

Probably a missing #include <optional> (which is fixed in master).

ncopa commented 1 year ago

I don't think its fixed in master. This fixes it:

diff --git a/src/qt/buynamespage.h b/src/qt/buynamespage.h                                                      
index 03a565c..1aa3097 100644                                                                                   
--- a/src/qt/buynamespage.h                                                                                     
+++ b/src/qt/buynamespage.h                                                                                     
@@ -5,6 +5,8 @@                                                                                                 

 #include <QWidget>                                                                                             

+#include <optional>                                                                                            
+                                                                                                               
 class WalletModel;                                                                                             

 namespace Ui {                                                                                                 
JeremyRand commented 1 year ago

Hi @ncopa , thanks for the patch. Would you mind creating a PR for that, so that we can review it properly? It definitely looks like this bug only gets tickled in certain cases, and for whatever reason it doesn't get tickled in either my personal build VM or Guix, but as long as the PR is obviously correct, I'm not too worried about figuring out how to reproduce the bug on my end -- adding an #include doesn't need that much review.

EDIT: Also apologies for missing your patch when you posted it; your comment was posted while I had COVID. :(