RFC: Disable Visual Studio warning about

[Greg Ercolano]

I'm working on issue #109 to solve the many warnings Visual Studio throws up,
in particular the long standing warnings about open()/_open(), write()/_write(), etc, e.g.

Warning C4996   'open': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _open.
Warning C4996   'read': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _read.
Warning C4996   'close': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _close.
Warning C4996   'write': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _write.

We know things like "#define open _open" is bad (due to how it affects variable and class methods of the same name),
and I don't think wrappers like fl_open(), fl_write() etc. are the right solution.

Looking into solving this specific warning, we have this recommendation from Microsoft:
From https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996?view=msvc-160

POSIX function names

[..]

Microsoft renamed some POSIX and Microsoft-specific library functions in the CRT to conform with C99 and C++03 constraints on reserved and global implementation-defined names. Only the names are deprecated, not the functions themselves. In most cases, a leading underscore was added to the function name to create a conforming name. The compiler issues a deprecation warning for the original function name, and suggests the preferred name.

To fix this issue, we usually recommend you change your code to use the suggested function names instead. However, the updated names are Microsoft-specific. If you need to use the existing function names for portability reasons, you can turn off these warnings. The functions are still available in the library under their original names.

To turn off deprecation warnings for these functions, define the preprocessor macro _CRT_NONSTDC_NO_WARNINGS. You can define this macro at the command line by including the option /D_CRT_NONSTDC_NO_WARNINGS.

Curious which should be the best approach for us:

    1) A #define for _CRT_NONSTDC_NO_WARNINGS in one of our .H files specific to Visual Studio builds
    2) Build system via cmake/configure (via compiler flags, or Visual Studio project creation)
    3) Other solution?
    4) Do nothing, leaving the warnings as a reminder Microsoft is incredibly annoying

Votes? Comments?

[Albrecht Schlosser]

On 2/16/21 10:48 PM Greg Ercolano wrote:
> I'm working on issue #109 to solve the many warnings Visual Studio
> throws up,

> in particular the long standing warnings about *open()/**_open()*,
> *write()*/*_write()*, etc, e.g.

>
> Warning C4996   'open': The POSIX name for this item is deprecated.
> Instead, use the ISO C and C++ conformant name: _open.

> [...]
>
> We know things like *"**#define open _open**"* is bad (due to how it

> affects variable and class methods of the same name),
> and I don't think wrappers like fl_open(), fl_write() etc. are the right
> solution.

There's an important point regarding open(): it takes a filename and as
such it is subject to UTF-8 conversion to Windows "Wide Character"
(UTF-16). This means, to be correct for international applications
(non-ASCII), we *need* a wrapper for open().

For the other calls (read, write, close) we could use any applicable
solution because no paths are involved.

> Looking into solving this specific warning, we have this recommendation
> from Microsoft:

> [...]
> //To turn off deprecation warnings for these functions, define the

> preprocessor macro _CRT_NONSTDC_NO_WARNINGS. You can define this
> macro at the command line by including the option

> /D_CRT_NONSTDC_NO_WARNINGS.//

This would be an almost secure way, as long as we only use it at build
time inside the library: such a definition must not be inherited by the
user build system. This is something can probably do with CMake.

Side note: if you're asking "WTH does he mean with inherit in this
context"? Yes, the library build system can make such definitions
"public" in the sense that the user program should use them as well.
Typically used for build flags that must be the same in the lib and the
user program that calls the lib.

> Curious which should be the best approach for us:
>
>     1) A #define for _CRT_NONSTDC_NO_WARNINGS in one of our .H files
> specific to Visual Studio builds

As I wrote above, this would need to be inside the library code and
never in a public header file.

>     2) Build system via cmake/configure (via compiler flags, or Visual
> Studio project creation)

That would be my preferred solution among the suggested solutions.

>     3) Other solution?

Suppress warnings by number in our build system (CMake) and maybe make
this selectable by the user, for instance:

warning C4996: 'open': The POSIX name for this item...

This would be warning no 4996 and can be suppressed with compiler switch
'/wd4996' or something like that. This would, however, suppress all
these POSIX name related warnings, not only open or read/write/close.

>     4) Do nothing, leaving the warnings as a reminder Microsoft is
> incredibly annoying

Good idea. ;-)

> Votes? Comments?

Not really. Comments yes (above), but not really a vote.

It would probably be wise to create a CMake build option to disable all
of these annoying warnings together.

The attached file shows the annoying VS warnings I'm usually
suppressing. Note this is an old list with edits for different versions
and additional comments.

In cmake-gui I'm editing "OPTION_OPTIM" or I'm running CMake with:

-D OPTION_OPTIM="/wd4244 /wd4267 /wd4305 /wd4996 /wd4312 /wd4334 /wd4838"

Silence is golden... ;-) ;-)

I could imagine a CMake option "SUPPRESS_COMMON_VS_WARNINGS" that does
exactly that and can be selected by the user or developer.

--------

PS: (unrelated) My new Apple MacBook Air with M1 (ARM) chip is up and
running and I was able to `git clone` FLTK, run CMake (Unix Makefiles)
and build FLTK (i.e. the ARM version) w/o issues! For a total macOS
newbie this went surprisingly smooth and quick.

[Bill Spitzak]

I believe in Windows 10 they finally fixed UTF-8. You can set the locale so that filenames for open(), etc, are interpreted as UTF-8 strings. I strongly recommend this as the correct fix, and that the wrapper functions be removed.

--
You received this message because you are subscribed to the Google Groups "fltk.coredev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkcoredev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/fltkcoredev/4f599104-501b-9cfb-c789-36f7a9d356c5%40online.de.

[Ian MacArthur]

On 17 Feb 2021, at 00:19, Bill Spitzak wrote:
>
> I believe in Windows 10 they finally fixed UTF-8. You can set the locale so that filenames for open(), etc, are interpreted as UTF-8 strings. I strongly recommend this as the correct fix, and that the wrapper functions be removed.

But does that not leave us having to distinguish between WIn10 and Win-(older) at runtime then do something different...?
There’s still an lot (a majority?) of older systems out there in the wild...

[Greg Ercolano]

    Perhaps the right thing is to make a wrapper fl_open() (we don't have one presently I don't think)
    and have it detect use the UTF8 -> WC conversion only when needed, so that we can open files
    with utf8 strings.

    Do we really know if it's an OS thing (Windows 10?) in which case it'd be a runtime check,
    or is it a C library version thing, in which case it'd be compile time?

    One thing seems sure; open() doesn't handle utf8 pathnames on Windows.
    I did a short test with a utf8 encoded subdirectory on my file server, the open() operation fails
    on Windows 8 for both mingw and VS2017, even though the command line cat/type can show the file just fine.
    And of course on Mac and Linux the program works fine; test program attached as utf8test.cxx.

    Results below for the filename "./UTF8-作藩于外/foo/foo.txt" (hoping that posts OK).

    I was able to get _wopen() to work in mingw by modifying that program to use a wchar_t string,
    so it does seem to be an issue with the windows implementation of open()/_open().

[Albrecht Schlosser]

On 2/17/21 10:43 PM Greg Ercolano wrote:
>
>> On 17 Feb 2021, at 00:19, Bill Spitzak wrote:
>>> I believe in Windows 10 they finally fixed UTF-8. You can set the locale so that filenames for open(), etc, are interpreted as UTF-8 strings. I strongly recommend this as the correct fix, and that the wrapper functions be removed.

Even if it worked for newer systems we couldn't rely on it because FLTK
also supports older Windows versions and, of course, we can't rely on
any specific locale setting on any Windows user system.

The *W (wide character) functions, however, are (well-)defined and if we
use them correctly this should always work correctly.

>     Perhaps the right thing is to make a wrapper fl_open() (we don't
> have one presently I don't think)

Yep, that and only that, if you ask me. I wonder why we didn't detect
the missing fl_open() wrapper yet. We have wrappers for "everything",
even fl_access(), fl_getenv(), fl_putenv(), ... And yes, environment
variables are also concerned.

The Windows API requires either an "Ansi" character string (i.e. current
codepage) or a "Unicode" aka "Wide Character" string (UTF-16). This is
implemented by either the 'A' suffix or the 'W' suffix of function names
whereas the function name w/o the suffix uses a trick with a definition
of Unicode or similar macro or whatever.

We're *always* using the *W function names explicitly where applicable
to avoid ambiguities.

>     and have it detect use the UTF8 -> WC conversion only when needed,
> so that we can open files with utf8 strings.

In that case conversion == detection. If it's not necessary (7-bit ASCII
only) the converted string is identical to the original string.

A separate detection pass would only add complexity.

>     Do we really know if it's an OS thing (Windows 10?) in which case
> it'd be a runtime check,
>     or is it a C library version thing, in which case it'd be compile time?

It's definitely runtime, you must call the *W function and provide a
UTF-16 string. UTF-8 with non-ASCII contents will always fail.

>     One thing seems sure; open() doesn't handle utf8 pathnames on Windows.

>     I did a short test with a utf8 encoded subdirectory ...
> ...

>     I was able to get _wopen() to work in mingw by modifying that
> program to use a wchar_t string,

How? Did you convert the UTF-8 string or how did you provide a wchar_t
string? Which encoding? Maybe with L"string" syntax? Yes, that would be
UTF-16 encoded and work with _wopen().

>     so it does seem to be an issue with the windows implementation of
> open()/_open().

Absolutely correct. I'm not sure what the real difference is (I think
only MS would know) but in our context both would do the wrong thing if
provided with UTF-8 (non-ASCII) strings.

[Greg Ercolano]

    I used this:

    // Assumes cwd contains the UTF8-???? subdirectory
    // UTF8 -> WIDECHAR
    wchar_t wcfilename[] = L"./UTF8-作藩于外/foo/foo.txt";
    if ( (fd = _wopen(wcfilename, O_RDONLY)) == -1 ) {  // Visual Studio wide char open()
        printf("_wopen() failed\n");
        return 1;
    }

    While that worked with mingw, it didn't with VS for some reason, not sure why.
    Might have been a missing compiler flag; I just used "CL /TP foo.cxx" to build it,
    but perhaps VS needs some coercion to comprehend inline utf8.

    I don't think I'm going to take on creating the wrapper, as I don't know enough
    about the ins and outs of all the details to make it do the right thing at the right time..

[Albrecht Schlosser]

On 2/17/21 11:29 PM Greg Ercolano wrote:
>
> On 2/17/21 2:12 PM, Albrecht Schlosser wrote:
>> On 2/17/21 10:43 PM Greg Ercolano wrote:
>>>      I was able to get _wopen() to work in mingw by modifying that
>>> program to use a wchar_t string,
>>
>> How? Did you convert the UTF-8 string or how did you provide a wchar_t
>> string? Which encoding?
>> Maybe with L"string" syntax? Yes, that would be UTF-16 encoded and
>> work with _wopen().
>
>     I used this:
>
>     // Assumes cwd contains the UTF8-???? subdirectory
>     // UTF8 -> WIDECHAR
>     wchar_t wcfilename[] = L"./UTF8-作藩于外/foo/foo.txt";
>     if ( (fd = _wopen(wcfilename, O_RDONLY)) == -1 ) {  // Visual
> Studio wide char open()
>         printf("_wopen() failed\n");
>         return 1;
>     }

Ah, yes, L"./UTF8-作藩于外/foo/foo.txt" is what I expected.

>     While that worked with mingw, it didn't with VS for some reason,
> not sure why.

Neither do I. And I won't try!

>     Might have been a missing compiler flag; I just used "CL /TP
> foo.cxx" to build it,
>     but perhaps VS needs some coercion to comprehend inline utf8.

Maybe you need to define "Unicode" (how exactly I do not know). It's
always a mystery...

>     I don't think I'm going to take on creating the wrapper, as I don't
> know enough
>     about the ins and outs of all the details to make it do the right
> thing at the right time..

I can do it and I can also test it on my systems. There are enough
examples like fl_access() to show how to do it (assuming fl_access() is
doing it right).

[Bill Spitzak]

It looks like fl_open and fl_fopen already exist in fltk

--
You received this message because you are subscribed to the Google Groups "fltk.coredev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkcoredev...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/fltkcoredev/08dfb608-7a77-98aa-5e5b-ffc3eb4bda53%40online.de.

[Greg Ercolano]

On 2/17/21 2:56 PM, Bill Spitzak wrote:

It looks like fl_open and fl_fopen already exist in fltk

    Nice, looks like we inherited this from OksiD's utf8 port.
    Well that should be easy to fix the warnings in fluid then..!

    It occurs to me we had quite a few fl_xxx()'s in place, but didn't wholesale switch to using them yet,
    so the opportunity is nigh..

[Greg Ercolano]

On 2/17/21 3:30 PM, Greg Ercolano wrote:

On 2/17/21 2:56 PM, Bill Spitzak wrote:

It looks like fl_open and fl_fopen already exist in fltk

    Nice, looks like we inherited this from OksiD's utf8 port.
    Well that should be easy to fix the warnings in fluid then..!

    Actually, looks like the warning for open() is only in the zlib project ('gzlib.c'),
    and that use is already in a wrapper function called gz_open() which /seems/
    to handle wide chars conversion already.

    So in that case just silencing the warning would help when building that project.

    Leaving warnings on for at least open() would be good so that if any fltk dev
    adds a call to open() in the library code, we'll get a warning about it, and (eventually)
    come to the conclusion to use fl_open() instead.

    It's interesting to me there's no warning about our use of open() in the Fl_WinAPI_System_Driver.cxx
    code:

---

int Fl_WinAPI_System_Driver::open_ext(const char *fnam, int binary, int oflags, int pmode) {
  if (oflags == 0) oflags = _O_RDONLY;
  oflags |= (binary ? _O_BINARY : _O_TEXT);
  return open(fnam, oflags, pmode);
}

---

    Which must mean we're somehow disabling warnings there.
    Perhaps whatever that is just needs to be added to the zlib project..?

[Manolo]

On Thursday, February 18, 2021 at 1:20:29 AM UTC+1 er...@seriss.com wrote:
    It's interesting to me there's no warning about our use of open() in the Fl_WinAPI_System_Driver.cxx
    code:

---

int Fl_WinAPI_System_Driver::open_ext(const char *fnam, int binary, int oflags, int pmode) {
  if (oflags == 0) oflags = _O_RDONLY;
  oflags |= (binary ? _O_BINARY : _O_TEXT);
  return open(fnam, oflags, pmode);
}

---

Caution: the red statement above calls another member function of class Fl_WinAPI_System_Driver :

int Fl_WinAPI_System_Driver::open(const char *fnam, int oflags, int pmode) {
  utf8_to_wchar(fnam, wbuf);
  if (pmode == -1) return _wopen(wbuf, oflags);
  else return _wopen(wbuf, oflags, pmode);
}
which goes into the OS with _wopen().

[Manolo]

On Thursday, February 18, 2021 at 8:15:21 AM UTC+1 Manolo wrote:

---

int Fl_WinAPI_System_Driver::open_ext(const char *fnam, int binary, int oflags, int pmode) {
  if (oflags == 0) oflags = _O_RDONLY;
  oflags |= (binary ? _O_BINARY : _O_TEXT);
  return open(fnam, oflags, pmode);
}

---

I've just committed a change to that statement:

 return this->open(fnam, oflags, pmode);

[Manolo]

There remain a few VisualStudio warnings coming from file nanosvg.h which

gets included by Fl_SVG_Image.cxx when building libfltk_images :

1>…\fltk\src\../nanosvg/nanosvg.h(1433,12): warning C4244: 'initialisation' : conversion of 'double' to 'float', possible data loss
1>…\fltk\src\../nanosvg/nanosvg.h(1441,12): warning C4244: 'initialisation' : conversion of 'double' to 'float', possible data loss
1>…\fltk\src\../nanosvg/nanosvg.h(1474,26): warning C4244: '=' : conversion of 'double' to 'float', possible data loss
1>…\fltk\src\../nanosvg/nanosvg.h(2517,29): warning C4244: '=' : conversion of 'double' to 'float', possible data loss
1>…\fltk\src\../nanosvg/nanosvg.h(2521,29): warning C4244: '=' : conversion of 'double' to 'float', possible data loss
1>…\fltk\src\../nanosvg/nanosvg.h(2525,30): warning C4244: '=' : conversion of 'double' to 'float', possible data loss
1>…\fltk\src\../nanosvg/nanosvg.h(2529,31): warning C4244: '=' : conversion of 'double' to 'float', possible data loss

What about adding in our own nanosvg branch an extra "pragma warning" to the 2 already present in nanosvg.h and obtain

#ifdef _MSC_VER
    #pragma warning (disable: 4996) // Switch off security warnings
    #pragma warning (disable: 4100) // Switch off unreferenced formal parameter warnings

    #pragma warning (disable: 4244) // Switch off implicit type conversion warnings  <---- this would be added

[imm]

Alternatively, would adding explicit casts work to silence the warning?

I think an explicit cast (if it works!) would be "better" than turning off the warning with the pragma. Maybe...

-- 
Ian
From my Fairphone FP3

[Albrecht Schlosser]

Yes this would work but ...

> I think an explicit cast (if it works!) would be "better" than turning
> off the warning with the pragma. Maybe...

... (IMO) we don't want to change much of the code of bundled libs
unless /necessary/, otherwise updating the libs becomes more difficult.
We have our FLTK specific scaling and one MSVC specific patch and Greg
worked on another patch recently.

For the same reason I added compiler options to suppress such warnings
in the bundled jpeg and zlib code (png didn't exhibit such warnings).

In the case of nanosvg we can't do the same because it's included in our
code (unless we use option (2) below). I think we have these options:

(1) Enable the warning (pragma) in Fl_SVG_Image.cxx just before we
include the nanosvg headers and (maybe [1]) disable it afterwards.

(2) Use a new file just for the nanosvg implementation (maybe
fl_nanosvg.cxx?) and use the #pragma before the inclusion of nanosvg
headers (no restore needed [1]).

(3) Would be to change the nanosvg files in our branch as proposed by
Manolo.

(4) Would be to leave the warnings as-is.

Option (2) would result in a very short file but provide a clean
separation, not only for the "new" pragma but also for the existing ones.

I'd opt for (2), (1), (3), (4) in this order of precedence (highest first).


--------

[1] Regarding #pragma (scope):

Does anybody know what scope such #pragma's have? Would they only be
valid inside the (included) file or would they extend to our FLTK code
*after* the inclusion of the nanosvg headers? Anyway, it looks like we
could do something like this:

#ifdef __MSC_VER
#pragma warning (push)
#pragma warning (disable: 4244)
#endif

{ include nanosvg stuff here }

#ifdef _MSC_VER
#pragma warning (pop)
#endif

The "easier" option w/o #pragma push/pop would probably be a separate
file for nanosvg implementation.

[Manolo]

On Monday, February 22, 2021 at 12:55:45 PM UTC+1 Albrecht Schlosser wrote:

(2) Use a new file just for the nanosvg implementation (maybe
fl_nanosvg.cxx?) and use the #pragma before the inclusion of nanosvg
headers (no restore needed [1]).

That is a convenient solution but it would require some glue code

between FLTK and nanosvg to be put in practice because FLTK

uses one struct and 4 functions (struct NSVGimage, nsvgDelete(),

nsvgParse(), nsvgRasterizeXY() and nsvgCreateRasterizer()) defined

by nanosvg.h and nanosvgrast.h. Moreover, this would require to duplicate

in FLTK the declaration of "struct NSVGimage" instead of using that

found in nanosvg.h. Thus, option (1) may be more convenient.

[Bill Spitzak]

I don't thinki double->float warnings are at all useful. They might be useful if both VC++ and GCC stopped them from happening when you are simply assigning a constant to a float, or check the number of digits in the constant to make a guess as to whether the user really is assuming higher accuracy than a float.

--
You received this message because you are subscribed to the Google Groups "fltk.coredev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkcoredev...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/fltkcoredev/5d026293-5e04-406f-8ced-ecaec6ca42f2n%40googlegroups.com.

[Albrecht Schlosser]

On 2/22/21 3:59 PM Manolo wrote:
> On Monday, February 22, 2021 at 12:55:45 PM UTC+1 Albrecht Schlosser wrote:
>
>
> (2) Use a new file just for the nanosvg implementation (maybe
> fl_nanosvg.cxx?) and use the #pragma before the inclusion of nanosvg
> headers (no restore needed [1]).
>
>
> That is a convenient solution but it would require some glue code
> between FLTK and nanosvg to be put in practice because FLTK

> uses one struct and 4 functions ...

This glue code is to #include one or both of the nanosvg header files
w/o defining NANOSVG_IMPLEMENTATION and NANOSVGRAST_IMPLEMENTATION,
resp., as with every other "normal" header file.

[Albrecht Schlosser]

On 2/22/21 9:54 PM Albrecht Schlosser wrote:
>
> This glue code is to #include one or both of the nanosvg header files
> w/o defining NANOSVG_IMPLEMENTATION and NANOSVGRAST_IMPLEMENTATION,
> resp., as with every other "normal" header file.

I forgot the link to the docs:
https://github.com/fltk/nanosvg#using-nanosvg-in-your-project

[Manolo]

Good. I hadn't realized that . In that case, I vote for option (2).

[Albrecht Schlosser]

On 2/22/21 12:55 PM Albrecht Schlosser wrote:
> On 2/22/21 12:22 PM imm wrote:
>> On Mon, 22 Feb 2021, 10:59 Manolo wrote:
>>

> In the case of nanosvg ... I think we have these options:

>
> (1) Enable the warning (pragma) in Fl_SVG_Image.cxx just before we
> include the nanosvg headers and (maybe [1]) disable it afterwards.
>
> (2) Use a new file just for the nanosvg implementation (maybe
> fl_nanosvg.cxx?) and use the #pragma before the inclusion of nanosvg
> headers (no restore needed [1]).
>

> ...

>
> [1] Regarding #pragma (scope):
>
> Does anybody know what scope such #pragma's have? Would they only be
> valid inside the (included) file or would they extend to our FLTK code
> *after* the inclusion of the nanosvg headers?

After a little research it's clear that the scope of #pragma is the
compilation unit regardless of #include files (include files are parsed
by the preprocessor and are not known to the compiler). This means that
it's bad style to set a #pragma inside a header file (nanosvg) w/o
restoring its value at the end of the header file (which includes the
implementation in this case).

That said, I tried a "clean" solution with a /minimal/ patch that just
adds the (hopefully) correct #pragma statements for VS and leaves the
rest as-is. See attached 'nanosvg-warnings.diff'.

Meanwhile I'm more for option (1) than (2) because it doesn't change
much of the existing code and provides a clean solution, i.e. (a)
suppresses warnings only in nanosvg code and (b) restores the #pragma so
it doesn't affect the FLTK code in src/Fl_SVG_Image.cxx.

This patch compiled in my tests w/o warnings in Fl_SVG_Image.cxx and/or
nanosvg.h with VS 2019. The GitHub build log of my own fork with this
patch can be found here:
https://github.com/Albrecht-S/fltk/runs/1965066789

So after all I think both options (1: extra implementation file) and (2:
see patch) are fine and I'm undecided.

Votes, anybody?

[Manolo]

On Tuesday, February 23, 2021 at 10:50:52 PM UTC+1 Albrecht Schlosser wrote: 

So after all I think both options (1: extra implementation file) and (2:
see patch) are fine and I'm undecided.

Votes, anybody?

Options 1 and 2 were  defined as follows :

> (1) Enable the warning (pragma) in Fl_SVG_Image.cxx just before we
> include the nanosvg headers and (maybe [1]) disable it afterwards.
>
> (2) Use a new file just for the nanosvg implementation (maybe
> fl_nanosvg.cxx?) and use the #pragma before the inclusion of nanosvg
> headers (no restore needed [1]).

Therefore I'm a little confused when I read

  "option[s] (1: extra implementation file) "

My vote would be for

"Enable the warning (pragma) in Fl_SVG_Image.cxx and disable it afterwards"

with the advantage of not introducing a new file with very little added value.

But, is it possible that some older versions of Visual Studio don't support

#pragma warning (push) / #pragma warning (pop)

In that case, option

"Use a new file just for the nanosvg implementation"

would be my choice.

[Albrecht Schlosser]

On 2/24/21 7:45 AM Manolo wrote:
>
> On Tuesday, February 23, 2021 at 10:50:52 PM UTC+1 Albrecht Schlosser
> wrote:
>
> So after all I think both options (1: extra implementation file) and
> (2:
> see patch) are fine and I'm undecided.
>
> Votes, anybody?
>
> Options 1 and 2 were  defined as follows :
> > (1) Enable the warning (pragma) in Fl_SVG_Image.cxx just before we
> > include the nanosvg headers and (maybe [1]) disable it afterwards.
> >
> > (2) Use a new file just for the nanosvg implementation (maybe
> > fl_nanosvg.cxx?) and use the #pragma before the inclusion of nanosvg
> > headers (no restore needed [1]).
>
> Therefore I'm a little confused when I read
>   "option[s] (1: extra implementation file) "

Sorry, I mixed that up. It's as you cited above.

> My vote would be for
> "Enable the warning (pragma) in Fl_SVG_Image.cxx and disable it afterwards"
> with the advantage of not introducing a new file with very little added
> value.

Yes, that's my vote now too.

> But, is it possible that some older versions of Visual Studio don't support
> #pragma warning (push) / #pragma warning (pop)
> In that case, option
> "Use a new file just for the nanosvg implementation"
> would be my choice.

Good question, I don't know. I can test VS 2008 (my oldest version) but
I think Greg has still much older versions available.

Greg, could you (or anybody else, of course) please test on your oldest
VS version and report back? Or does anybody know?

The patch can be found here (attached file nanosvg-warnings.diff):
https://groups.google.com/g/fltkcoredev/c/UB1E09RUguk/m/zbZbzVr_AQAJ