classification
Title: Fix _tkinter compiler warnings on MSVC
Type: behavior Stage: resolved
Components: Build, Tkinter, Windows Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: zach.ware Nosy List: loewis, python-dev, serhiy.storchaka, zach.ware
Priority: normal Keywords: patch

Created on 2014-08-04 21:06 by zach.ware, last changed 2014-08-05 16:56 by zach.ware. This issue is now closed.

Files
File name Uploaded Description Edit
tkinter_MSVC_warnings.diff zach.ware, 2014-08-04 21:06 review
Messages (11)
msg224776 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-08-04 21:06
Dropping support of Tk 8.3 caused a few compiler warnings on Windows:

  ..\Modules\_tkinter.c(587): warning C4090: '=' : different 'const' qualifiers
  ..\Modules\_tkinter.c(588): warning C4090: '=' : different 'const' qualifiers
  ..\Modules\_tkinter.c(589): warning C4090: '=' : different 'const' qualifiers
  ..\Modules\_tkinter.c(590): warning C4090: '=' : different 'const' qualifiers
  ..\Modules\_tkinter.c(591): warning C4090: '=' : different 'const' qualifiers
  ..\Modules\_tkinter.c(592): warning C4090: '=' : different 'const' qualifiers
  ..\Modules\_tkinter.c(593): warning C4090: '=' : different 'const' qualifiers

Does this patch look like the correct fix?
msg224809 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-08-05 06:07
I'm surprised that this caused a warning. In man page Tcl_GetObjType() is described as

       Tcl_ObjType *
       Tcl_GetObjType(typeName)

and in header file it is declared as

EXTERN Tcl_ObjType *	Tcl_GetObjType _ANSI_ARGS_((char * typeName));
Tcl_ObjType *Tcl_GetObjType(typeName);

But this change shouldn't add any ill effect. The patch LGTM.
msg224820 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-08-05 11:23
Serhiy: See

http://svn.python.org/projects/external/tcl-8.6.1.0/generic/tclDecls.h

where it now is

EXTERN CONST86 Tcl_ObjType * Tcl_GetObjType(const char *typeName);

I think the patch is wrong as it stands, as "const" is not a reserved word in C89. Instead, you should be using something like

#ifndef CONST86
#define CONST86
#endif

Tcl_ObjType CONST86 *BooleanType;
msg224827 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-08-05 13:36
> Serhiy: See
> http://svn.python.org/projects/external/tcl-8.6.1.0/generic/tclDecls.h
> where it now is
> EXTERN CONST86 Tcl_ObjType * Tcl_GetObjType(const char *typeName);

Oh, my fault. I did not notice that my tcl workspace was switched to 8.3 
branch.

> I think the patch is wrong as it stands, as "const" is not a reserved word
> in C89.

"const" was introduced in ANSI C (aka C89) and it is reserved word. The patch 
is correct because even if Tcl_GetObjType() returns "Tcl_ObjType *", the 
result is compatible with "Tcl_ObjType const *" (or "const Tcl_ObjType *").
msg224829 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-08-05 13:57
I see. So the patch is fine indeed.
msg224840 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-08-05 14:53
Thank you both for the reviews!

Embarrassingly, I just realized that I didn't post the patch I meant to; in quickly reconstructing the patch after accidentally destroying a previous version, I made it "Tcl_ObjType const *SomeType;" instead of "const Tcl_ObjType *SomeType;".  It seems to me that "const <type>" is much more common than "<type> const"; does it actually matter which way it goes?  Either way fixes the warnings.
msg224843 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-08-05 15:32
"const T" and "T const" are the same type. This compiles, and all variables have the same type:

int main()
{
  const volatile int a;
  const int volatile b;
  int const volatile c;
  volatile int const d;
  volatile int const etc;
}

Even "volatile short const int" and "long volatile long const int" work.
msg224845 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-08-05 15:43
I see.  There doesn't appear to be anything in PEP 7 about this; do we have a style preference for this that applies here?  Or should I just shut up and commit it? :)
msg224851 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-08-05 16:08
Grepping through the source shows that currently, the const always goes before the type (for char, wchar_t, Py_UCS2, and int). So please do this style adjustment and commit.
msg224859 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-08-05 16:55
New changeset 7ed237478fcc by Zachary Ware in branch 'default':
Closes #22136: Fix MSVC compiler warnings introduced by #22085
http://hg.python.org/cpython/rev/7ed237478fcc
msg224860 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-08-05 16:56
Done and done.  Thank you very much, Martin.
History
Date User Action Args
2014-08-05 16:56:58zach.waresetmessages: + msg224860
2014-08-05 16:55:10python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg224859

resolution: fixed
stage: commit review -> resolved
2014-08-05 16:08:27loewissetmessages: + msg224851
2014-08-05 15:43:24zach.waresetmessages: + msg224845
2014-08-05 15:32:18loewissetmessages: + msg224843
2014-08-05 14:53:55zach.waresetmessages: + msg224840
2014-08-05 13:57:16loewissetmessages: + msg224829
2014-08-05 13:36:17serhiy.storchakasetmessages: + msg224827
2014-08-05 11:23:33loewissetnosy: + loewis
messages: + msg224820
2014-08-05 06:07:31serhiy.storchakasetassignee: zach.ware
messages: + msg224809
stage: patch review -> commit review
2014-08-04 21:06:58zach.warecreate