macro variables -> constexpr

[eoan-ermine]

No description provided.

[benapetr]

Hello, what exactly is a benefit in using constexpr over macros we had before? Does this even compile? I think some of the definitions we had in definitions file were used by preprocessor macros as well, so I don't think all build paths will be functioning right now.

[eoan-ermine]

Hello, what exactly is a benefit in using constexpr over macros we had before? Does this even compile? I think some of the definitions we had in definitions file were used by preprocessor macros as well, so I don't think all build paths will be functioning right now.

I parsed a list of macro variables, i replaced with constexpr (here it is) and ran the next script:

import subprocess

f = open("cat.txt") # File with content of https://pastebin.com/kKWTLPiw
names = [name.strip() for name in f.readlines()]
f.close()

patterns = [f"#ifdef {name}" for name in names]
patterns.extend(
	[f"#ifndef {name}" for name in names]
)
grep_target = "\\|".join(patterns)

result = subprocess.Popen(
	f"grep -n '{grep_target}' -r .",
	 shell=True, stdout=subprocess.PIPE
)

print(result.stdout.read().decode('utf-8'))

And got the following output:

./src/huggle_core/definitions_prod.hpp:151:#ifndef HUGGLE_TIMER
./src/huggle_core/deprecated/pythonengine.cpp:707:#ifdef HUGGLE_GLOBAL_EXTENSION_PATH
./src/huggle_core/gc.cpp:28:#ifdef HUGGLE_USE_MT_GC
./src/huggle_core/gc.cpp:114:#ifdef HUGGLE_USE_MT_GC
./src/huggle_core/gc.cpp:129:#ifdef HUGGLE_USE_MT_GC
./src/huggle_core/gc.cpp:142:#ifdef HUGGLE_USE_MT_GC
./src/huggle_core/definitions.hpp:149:#ifndef HUGGLE_TIMER
./src/huggle_core/exception.cpp:38:#ifdef HUGGLE_BREAKPAD
./src/huggle_core/exception.cpp:171:#ifdef HUGGLE_BREAKPAD
./src/huggle_core/exception.cpp:189:#ifdef HUGGLE_BREAKPAD
./src/huggle_core/exception.hpp:73:#ifdef HUGGLE_BREAKPAD
./src/huggle_core/core.cpp:64:#ifdef HUGGLE_BREAKPAD
./src/huggle_core/core.cpp:107:#ifdef HUGGLE_GLOBAL_EXTENSION_PATH
./src/huggle_core/core.cpp:217:#ifdef HUGGLE_GLOBAL_EXTENSION_PATH
./src/huggle_ui/scripting/uiscript.cpp:53:#ifdef HUGGLE_GLOBAL_EXTENSION_PATH
./src/huggle_ui/mainwindow.cpp:1490:#ifndef HUGGLE_USE_MT_GC

I will cancel replacing these macro variables with constexp, but the rest of the macro variables can be replaced with constexp without problems

[eoan-ermine]

Hello, what exactly is a benefit in using constexpr over macros we had before? Does this even compile? I think some of the definitions we had in definitions file were used by preprocessor macros as well, so I don't think all build paths will be functioning right now.

It's common practice to use constant variables instead of macro variables, they provides more type safety and can't generate hard-to-find name conflicts.
They have no disadvantages, but the advantages mentioned above.

[benapetr]

they actually do have quite a number of disadvantages:

• they can't be processed by preprocessor logic (#ifdef etc)
• they can't be overriden by compiler directives
• they can't be defined by compiler (./configure switches for example)

I will go through this PR later and check if it breaks some other logic, but as I said, there is many compile paths, some of them platform-dependent, so this needs intensive testing on all 3 supported platforms, right now unit tests are wonky @phuzion what is the current situation with travis conversion to github? Where can I find build results for Windows, MacOS and Linux? Only appveyor is working it seems?

[phuzion]

@phuzion what is the current situation with travis conversion to github? Where can I find build results for Windows, MacOS and Linux? Only appveyor is working it seems?

I'll need to sit down with you at some point this week to get the nightlies back up and going. Long story short, we'll need to replicate the rest of the functionality in AppVeyor onto GitHub Actions, which should just be a matter of either getting me a dump of the config, or getting me access into AppVeyor.

I believe you mentioned something about talking to someone at WMF to get some storage for the nightlies rather than hosting it on your own. However, if you're ok with it, I would be happy to migrate the entire AppVeyor config to GHA this week.

[benapetr]

but veyor is doing fine, what I am missing are old travis builds, what happened to them? I was thinking you migrated them, but I don't see any builds now for MacOS / Linux

[phuzion]

When I began the migration process to GHA, (which I admittedly haven't completed, apologies), I made a change to the Travis script where we now build with QtWebEngine. The current Travis environment can't build with QtWebEngine. My intention was to migrate to GHA using the most modern technologies we had available, and abandon AppVeyor and Travis CI in favor of a centralized, single place for CI.

You're not seeing GHA builds on this Pull Request, because we currently don't build on PRs, only pushes. Changing this would be a trivial change, it's literally a 30 second pull request.

As I mentioned previously, I'd like to sit down with you soon and hash out exactly what needs to be done for us to consider GHA a complete CI solution for Huggle, and finally wrap up the project that I started months ago. If you have some time this week, let's chat about this on IRC.