Skip to content

Commit

Permalink
COMMON: Silence MSVC warning for Common::gcd calls with an unsigned type
Browse files Browse the repository at this point in the history
  • Loading branch information
@Templier
Templier committed May 24, 2011
1 parent 488759c commit 89e954c
Showing 1 changed file with 7 additions and 0 deletions.
7 changes: 7 additions & 0 deletions common/algorithm.h
View file
Expand Up @@ -234,6 +234,11 @@ void sort(T first, T last) {
sort(first, last, Common::Less<typename T::ValueType>());
}

// MSVC is complaining about the minus operator being applied to an unsigned type
// We disable this warning for the affected section of code
#pragma warning(push)
#pragma warning(disable: 4146)

/**
* Euclid's algorithm to compute the greatest common divisor.
*/
Expand All @@ -256,6 +261,8 @@ T gcd(T a, T b) {
return b;
}

#pragma warning(pop)

} // End of namespace Common
#endif

7 comments on commit 89e954c

@lordhoto
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't these pragmas only be used for MSVC, it seems a bit risky to enable them for all compilers.

@Templier
Copy link
Member Author

Choose a reason for hiding this comment

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

We already disable unknown pragmas warnings for both GCC and Visual Studio (we have GCC specific pragmas in the code).

I don't mind marking those as MSVC specific, but then we should really update the scumm engine code too and add back the warnings.

@lordhoto
Copy link
Contributor

Choose a reason for hiding this comment

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

What gcc specific pragmas do we use for all compilers?

@Templier
Copy link
Member Author

Choose a reason for hiding this comment

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

mark is used quite a lot in the scumm engine code (and I also use region quite a bit in ring, which is sort of the MSVC equivalent, but it's not a concern as it's not in master)

MSVC will happily ignore all those when silencing warning 4068 (the same way GCC does with -Wno-unknown-pragmas).

@clone2727
Copy link
Contributor

@clone2727 clone2727 commented on 89e954c May 24, 2011 via email

Choose a reason for hiding this comment

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

@lordhoto
Copy link
Contributor

Choose a reason for hiding this comment

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

Reading through it yes, it looks like pragma mark is used by Xcode to add labels.

Anyway unlike "mark" (which is probably a no-op for gcc+darwin) "warning" is also used as a pragma in icc for example. Since I don't think icc and msvc have the same warning numbers it seems not that good to have such code enabled by default. In this case it might be harmless, but to avoid other people copying the same code I am strongly against having this without guards in our common code.

@fingolfin
Copy link
Contributor

Choose a reason for hiding this comment

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

The "#pragma mark" are probably all by me. This pragma still seems from the day of MacOS classic, where many editors and IDEs used it to enrich the "function popups" they usually offered. That is, at the top of the window, you had a button that when pressed shows a menu with all global symbols defined in the file, in order, intermixed with any "#pragma mark" values.

On Mac OS X, many editors support this till this day, e.g. BBEdit, my primary editor.

For me, it just helped to structure various files and make them more "accessible". Alas, if they bother people a lot, we can remove them, although I am pretty sure they are 100% harmless.

Please sign in to comment.