Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

win32: retire visual studio 2013 support #21624

Merged
merged 1 commit into from
Nov 13, 2023
Merged

Conversation

tonycoz
Copy link
Contributor

@tonycoz tonycoz commented Nov 9, 2023

I recently (tried) to test VS 2013 builds, but while vs2013 in theory supports mixed declarations and code, in practice that support is broken, and failed to build toke.obj:

The one case I looked at in detail, VS 2013 required an extra ; after a conditional statement to accept the following declaration, for example:

C:\Users\Tony\dev\perl\git>cl /c mixed.c
Microsoft (R) C/C++ Optimizing Compiler Version 18.00.21005.1 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

mixed.c
mixed.c(8) : error C2143: syntax error : missing ';' before 'const'
mixed.c(9) : error C2065: 'pend' : undeclared identifier
mixed.c(9) : error C2100: illegal indirection

C:\Users\Tony\dev\perl\git>type mixed.c
/* adapted from S_intuit_more() in toke.c.
   don't run this code
*/
int g(const char *p) {
  if (!p)
    return (0);

  const char * const pend = p+1;
  return *pend;
}

regcomp.obj also failed to build due to the use of the "inline" keyword in regcomp.h. This is fairly trivially fixable but inline is also C99.

Despite these build issues we haven't received any error reports for VS2013, so I don't think it's worth re-working any mixed declarations to support it.

It might be worth updating regcomp.h to use PERL_STATIC_INLINE but that's out of scope for this commit.

README.win32 Outdated Show resolved Hide resolved
@xenu
Copy link
Member

xenu commented Nov 9, 2023

Yeah, if the build is broken and nobody noticed, it's a no-brainer. VS2013 was the odd one out of the supported versions, the only one without UCRT.

@khwilliamson
Copy link
Contributor

What then is the earliest supported version after this is merged?

@sisyphus
Copy link
Contributor

What then is the earliest supported version after this is merged?

Earliest supported will then be version 14.0, also referred to as "MSVC140", and "Visual C++ 2015" in the GNUmakefile and Makefile.

I recently (tried) to test VS 2013 builds, but while vs2013 in theory
supports mixed declarations and code, in practice that support is
broken, and failed to build toke.obj:

The one case I looked at in detail, VS 2013 required an extra ; after
a conditional statement to accept the following declaration, for
example:

```
C:\Users\Tony\dev\perl\git>cl /c mixed.c
Microsoft (R) C/C++ Optimizing Compiler Version 18.00.21005.1 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

mixed.c
mixed.c(8) : error C2143: syntax error : missing ';' before 'const'
mixed.c(9) : error C2065: 'pend' : undeclared identifier
mixed.c(9) : error C2100: illegal indirection

C:\Users\Tony\dev\perl\git>type mixed.c
/* adapted from S_intuit_more() in toke.c.
   don't run this code
*/
int g(const char *p) {
  if (!p)
    return (0);

  const char * const pend = p+1;
  return *pend;
}
```

regcomp.obj also failed to build due to the use of the "inline"
keyword in regcomp.h.  This is fairly trivially fixable but
inline is also C99.

Despite these build issues we haven't received any error reports for
VS2013, so I don't think it's worth re-working any mixed declarations
to support it.

It might be worth updating regcomp.h to use PERL_STATIC_INLINE
but that's out of scope for this commit.
@tonycoz tonycoz force-pushed the smoke-me/tonyc/retire-vs-2013 branch from fef5fef to e8a4ced Compare November 12, 2023 20:38
@tonycoz tonycoz merged commit 6be4da5 into blead Nov 13, 2023
57 checks passed
@tonycoz tonycoz deleted the smoke-me/tonyc/retire-vs-2013 branch November 13, 2023 23:35
@khwilliamson
Copy link
Contributor

I believe we can also remove anything that special cases any _MSC_VER < 1900

@mauke
Copy link
Contributor

mauke commented Nov 15, 2023

Not a lot of those around:

cpan/Compress-Raw-Zlib/zlib-src/zutil.c:#if (!defined(_MSC_VER) || (_MSC_VER <= 600))
cpan/Compress-Raw-Zlib/zlib-src/zutil.h:#if (defined(_MSC_VER) && (_MSC_VER > 600)) && !defined __INTERIX
cpan/Time-Piece/Piece.xs:#if !(_MSC_VER >= 1500)
dist/Time-HiRes/HiRes.xs:#  if((defined(_MSC_VER) && _MSC_VER < 1900) || \
perl.h:#      elif _MSC_VER < 1900

@jkeenan
Copy link
Contributor

jkeenan commented Nov 15, 2023 via email

@sisyphus
Copy link
Contributor

dist/Time-HiRes/HiRes.xs:# if((defined(_MSC_VER) && _MSC_VER < 1900) ||
\ perl.h:# elif _MSC_VER < 1900 |

We can remove more than just that from HiRes.xs.
That quoted line is part of a section that pertains solely to unsupported Visual Studio and unsupported mingw.org compilers.
AFAICS,, the following piece from HiRes.xs can (and should) be removed in its entirety:

/* Visual C++ 2013 and older don't have the timespec structure.
 * Neither do mingw.org compilers with MinGW runtimes older than 3.22. */
#  if((defined(_MSC_VER) && _MSC_VER < 1900) || \
      (defined(__MINGW32__) && !defined(__MINGW64_VERSION_MAJOR) && \
      defined(__MINGW32_MAJOR_VERSION) && (__MINGW32_MAJOR_VERSION < 3 || \
      (__MINGW32_MAJOR_VERSION == 3 && __MINGW32_MINOR_VERSION < 22))))
struct timespec {
    time_t tv_sec;
    long   tv_nsec;
};
#  endif

@khwilliamson
Copy link
Contributor

Also, now that MingW can be compiled with UCRT, things that are under WIN32 and #ifndef _MSC_VER should be checked to see if _UCRT is defined, and then we can assume a modern run time library, but I don't know how modern

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants