Skip to content

Conversation

@bobtista
Copy link

@bobtista bobtista commented Dec 1, 2025

  • Used clang-tidy

@bobtista bobtista marked this pull request as draft December 1, 2025 21:45
@Caball009
Copy link

Caball009 commented Dec 1, 2025

Are we sure that VC6 treats inline the same way as modern compilers do? I think we've seen in the past that different function inlining can cause mismatches for seemingly obscure reasons.

I'd recommend running this change against a large number of replays.

@bobtista bobtista force-pushed the bobtista/build/clang-tidy-redundant-inline branch from 0629427 to 92878b7 Compare December 1, 2025 22:49
@bobtista
Copy link
Author

bobtista commented Dec 1, 2025

VC6 treats inline the same way as modern compilers do

Section 7.1.2: "A function defined within a class definition is an inline function."
C++98 Standard (ISO/IEC 14882:1998)

This means member functions defined inside class bodies are implicitly inline in C++98, regardless of the inline keyword. VC6 claims C++98 support, so it should follow this. How many replays does our CI check with?

@xezon
Copy link

xezon commented Dec 2, 2025

If they were not implict inline then it would compile error, because of duplicate symbol definitions.

@bobtista bobtista marked this pull request as ready for review December 4, 2025 02:04
Copy link

@xezon xezon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also removed register keyword.

@xezon xezon changed the title build: remove redundant inline specifiers from implicitly inline functions refactor: Remove superfluous inline and register keywords Dec 6, 2025
@xezon xezon added the Refactor Edits the code with insignificant behavior changes, is never user facing label Dec 6, 2025
@bobtista bobtista force-pushed the bobtista/build/clang-tidy-redundant-inline branch from 92878b7 to c67addf Compare December 6, 2025 17:54
@bobtista
Copy link
Author

bobtista commented Dec 6, 2025

It also removed register keyword.

Yeah, do you want me to keep it? Only 2 lines had the register keyword removed in this branch- both are in the same function Get_Date_From_Day, in:
Core/Tools/matchbot/wlib/xtime.cpp
Core/Tools/mangler/wlib/xtime.cpp

The register keyword is just a hint that the compiler can ignore, even in VC6. It was deprecated in C++11 and removed in C++17, our CppMacros.h defines REGISTER as empty too. It's unrelated I suppose but also harmless. Up to you.

@Caball009
Copy link

Caball009 commented Dec 6, 2025

How many replays does our CI check with?

I assume 9 replays https://github.com/TheSuperHackers/GeneralsReplays/tree/af0c61ccdccbf06750b0ecd2de99fe65842bfc1a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Refactor Edits the code with insignificant behavior changes, is never user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants