Eliminate WIN3264 macro instead of MSWIN #3932

h-east · 2019-02-11T01:16:25Z

~~We use the existing WIN3264 macro instead.~~

Define FEAT_MSWIN and FEAT_GUI_MSWIN.

Use ~~FEAT_MSWIN~~ MSWIN instead of _WIN32, WIN32 and WIN3264.
Use FEAT_GUI_MSWIN instead of FEAT_GUI_W32.

We use the existing WIN3264 macro instead.

codecov-io · 2019-02-11T01:37:02Z

Codecov Report

Merging #3932 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3932      +/-   ##
==========================================
- Coverage   79.13%   79.13%   -0.01%     
==========================================
  Files         105      105              
  Lines      141110   141110              
==========================================
- Hits       111670   111667       -3     
- Misses      29440    29443       +3

Impacted Files	Coverage Δ
src/if_cscope.c	`76.42% <ø> (ø)`	⬆️
src/gui_gtk_x11.c	`48.23% <ø> (-0.1%)`	⬇️
src/if_ruby.c	`90.29% <ø> (ø)`	⬆️
src/normal.c	`75.56% <ø> (ø)`	⬆️
src/fileio.c	`75.44% <ø> (ø)`	⬆️
src/if_tcl.c	`86.42% <ø> (ø)`	⬆️
src/evalfunc.c	`88.44% <ø> (ø)`	⬆️
src/ex_cmds2.c	`84.89% <ø> (ø)`	⬆️
src/if_lua.c	`85.33% <ø> (ø)`	⬆️
src/ex_getln.c	`76.01% <ø> (ø)`	⬆️
... and 43 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78d21da...df88908. Read the comment docs.

brammool · 2019-02-11T16:56:34Z

We use the existing WIN3264 macro instead.

We could do it the other way around, use MSWIN instead of WIN3264. It's shorter and easier to read. We can probably also use MSWIN instead of WIN32. There is an OR operation on WIN32 and _WIN64, but I guess that when _WIN64 is defined WIN32 also is defined. So I think we can just use MSWIN nearly everywhere, and only use _WIN64 where it really has to be the 64 bit version. All the rest works for both 32 and 64 bit MS-Windows.

…

-- The MS-Windows registry is no more hostile than any other bunch of state information... that is held in a binary format... a format that nobody understands... and is replicated and cached in a complex and largely undocumented way... and contains large amounts of duplicate and obfuscated information... (Ben Peterson) /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

Eliminate WIN3264 instead of MSWIN. We use MSWIN macro.

mattn · 2019-02-12T00:58:08Z

WIN32 is undocumented at least. Actually _WIN32 and _WIN64 is officially.

mattn · 2019-02-12T01:00:51Z

See https://docs.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=vs-2017

h-east · 2019-02-12T04:52:58Z

@mattn Thanks!

This microsoft site is written as follows.

_WIN32 Defined as 1 when the compilation target is 32-bit ARM, 64-bit ARM, x86, or x64. Otherwise, undefined.

This means that the definition below is fine, right?

#ifdef _WIN32
# define MSWIN
#endif

Then, we can replace the part directly referencing WIN32 or _WIN32 with MSWIN, right?

k-takata · 2019-02-12T06:14:40Z

I thought that defining WIN3264 was redundant, because it is exactly the same as _WIN32.
However, one merit for using WIN3264 is that it can avoid misunderstanding that _WIN32 doesn't includes _WIN64 (which is incorrect).
MSWIN was used for both Win16 and Win32, but we have already dropped the support for Win16.
So now MSWIN, WIN3264 and _WIN32 (and also WIN32) are all the same.

Using _WIN32 is a standard way in the Windows programming. So it is straightforward.
However, still there's a risk that someone misunderstands that it doesn't include _WIN64. Do we need to care such a case? If so, MSWIN might be better.

k-takata · 2019-02-13T00:44:37Z

Bram's comment:
https://groups.google.com/d/msg/vim_dev/XsEQZnQr8Ck/StHpy92xAwAJ

It is best to separate the dependency on the compiler and Makefile from
what we use throughout Vim code. "MSWIN" is the easiest to understand
for all builds on MS-Windows. Then we can define it in one place in
vim.h, depending on _WIN32 or WIN32 (WIN32 is defined in all the
MS-Windows makefiles).

We can keep using _WIN64 for 64-bit specific code.

Okay, let's use MSWIN and _WIN64.
There is a part which uses WIN64 in evalfunc.c. It can be removed then.

BTW, we have also FEAT_GUI_W32 and FEAT_GUI_MSWIN.
FEAT_GUI_W32 is defined in the makefiles, and FEAT_GUI_MSWIN is defined in vim.h.
What do you think of this? Using FEAT_GUI_MSWIN is better?

brammool · 2019-02-13T12:34:51Z

Bram's comment: https://groups.google.com/d/msg/vim_dev/XsEQZnQr8Ck/StHpy92xAwAJ > It is best to separate the dependency on the compiler and Makefile from > what we use throughout Vim code. "MSWIN" is the easiest to understand > for all builds on MS-Windows. Then we can define it in one place in > vim.h, depending on _WIN32 or WIN32 (WIN32 is defined in all the > MS-Windows makefiles). > > We can keep using _WIN64 for 64-bit specific code. Okay, let's use `MSWIN` and `_WIN64`. There is a part which uses `WIN64` in `evalfunc.c`. It can be removed then. BTW, we have also `FEAT_GUI_W32` and `FEAT_GUI_MSWIN`. `FEAT_GUI_W32` is defined in the makefiles, and `FEAT_GUI_MSWIN` is defined in `vim.h`. What do you think of this? Using `FEAT_GUI_MSWIN` is better?

Yes, since it's also used for 64-bit Windows, thus the "32" part is confusing. I suppose FEAT_GUI is defined, so we can make it: #if defined(FEAT_GUI) && defined(MSWIN) # define FEAT_GUI_MSWIN #endif And remove other places where FEAT_GUI_MSWIN is defined.

…

-- System administrators are just like women: You can't live with them and you can't live without them. /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

h-east · 2019-02-13T12:42:12Z

All right. Thank you!
I have been busy for a few days, so I will work on weekends.
(Unofficial 30% rule can not be used! 😁)

--
Best regards,
Hirohito Higashi (h_east)

h-east · 2019-02-17T01:29:24Z

I have a question.
Do we still need to support Make_ivc.mak?

k-takata · 2019-02-17T01:45:06Z

Recent versions of Visual Studio cannot open Make_ivc.mak (and also Make_dvc.mak).
I don't think we still need to support them. But is it related to this PR?

h-east · 2019-02-17T11:51:22Z

Yes. We need s/FEAT_GUI_W32/FEAT_GUI/g in Make_ivc.mak

Use FEAT_MSWIN instead of _WIN32, WIN32 and WIN3264. Use FEAT_GUI_MSWIN instead of FEAT_GUI_W32.

brammool · 2019-02-17T13:06:53Z

Let me know when this is ready to include.

h-east · 2019-02-17T13:10:53Z

My work has been completed, but the impact range is large,
I'm happy if you can have reviews by various people.

h-east · 2019-02-17T15:18:20Z

@mattn, @ntak and Windows developers:
JFYI, Use ~~FEAT_MSWIN~~ MSWIN instead of WIN3264, WIN32 and _WIN32.
And also use FEAT_GUI_MSWIN instead of FEAT_GUI_W32.

Reason:
#3932 (comment)
#3932 (comment)

k-takata · 2019-02-17T19:11:37Z

Hmm, you defined FEAT_GUI_MSWIN and FEAT_GUI differently with other platforms.
In other platforms, FEAT_GUI_GTK or other platform-specific macros are defined in each makefile, and FEAT_GUI is defined in vim.h. However you defined FEAT_GUI in Windows makefiles, and FEAT_GUI_MSWIN in vim.h. They are reversed.

k-takata · 2019-02-17T19:16:05Z

JFYI, Use FEAT_MSWIN instead of WIN3264, WIN32 and _WIN32.

s/FEAT_MSWIN/MSWIN/

h-east · 2019-02-18T02:40:27Z

They are reversed.

Thank you for your advice.
Indeed, the opposite is true.
I will PR later.

Problem: Macros for MS-Windows are inconsistent, using "32", "3264 and others. Solution: Use MSWIN for all MS-Windows builds. Use FEAT_GUI_MSWIN for the GUI build. (Hirohito Higashi, closes vim/vim#3932) vim/vim@4f97475

) Problem: Macros for MS-Windows are inconsistent, using "32", "3264 and others. Solution: Use MSWIN for all MS-Windows builds. Use FEAT_GUI_MSWIN for the GUI build. (Hirohito Higashi, closes vim/vim#3932) vim/vim@4f97475

Eliminate MSWIN macro.

0331328

We use the existing WIN3264 macro instead.

k-takata added the platform-windows label Feb 11, 2019

h-east added 2 commits February 12, 2019 09:20

Merge branch 'master' into eliminate_MSWIN_macro

ded3b19

Fixed the review pointed out from Bram.

05d918e

Eliminate WIN3264 instead of MSWIN. We use MSWIN macro.

h-east changed the title ~~Eliminate MSWIN macro.~~ Eliminate WIN3264 macro instead of MSWIN Feb 12, 2019

Merge branch 'master' into eliminate_MSWIN_macro

9478cbd

h-east added 2 commits February 17, 2019 00:06

Merge branch 'master' into eliminate_MSWIN_macro

2efe17d

Merge branch 'master' into eliminate_MSWIN_macro

8717dc2

Define FEAT_MSWIN and FEAT_GUI_MSWIN.

15e837f

Use FEAT_MSWIN instead of _WIN32, WIN32 and WIN3264. Use FEAT_GUI_MSWIN instead of FEAT_GUI_W32.

Merge branch 'master' into eliminate_MSWIN_macro

df88908

brammool closed this in 4f97475 Feb 17, 2019

h-east deleted the eliminate_MSWIN_macro branch February 17, 2019 23:27

h-east mentioned this pull request Feb 18, 2019

Define FEAT_GUI_MSWIN instead of FEAT_GUI in Windows makefiles. #3996

Closed

dundargoc mentioned this pull request Sep 16, 2022

vim-patch:8.1.0941: macros for MS-Windows are inconsistent neovim/neovim#20215

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Eliminate WIN3264 macro instead of MSWIN #3932

Eliminate WIN3264 macro instead of MSWIN #3932

h-east commented Feb 11, 2019 •

edited

codecov-io commented Feb 11, 2019 •

edited

brammool commented Feb 11, 2019 via email

mattn commented Feb 12, 2019

mattn commented Feb 12, 2019

h-east commented Feb 12, 2019

k-takata commented Feb 12, 2019

k-takata commented Feb 13, 2019

brammool commented Feb 13, 2019 via email

h-east commented Feb 13, 2019

h-east commented Feb 17, 2019

k-takata commented Feb 17, 2019

h-east commented Feb 17, 2019

brammool commented Feb 17, 2019

h-east commented Feb 17, 2019

h-east commented Feb 17, 2019 •

edited

k-takata commented Feb 17, 2019

k-takata commented Feb 17, 2019

h-east commented Feb 18, 2019

Eliminate WIN3264 macro instead of MSWIN #3932

Eliminate WIN3264 macro instead of MSWIN #3932

Conversation

h-east commented Feb 11, 2019 • edited

codecov-io commented Feb 11, 2019 • edited

Codecov Report

brammool commented Feb 11, 2019 via email

mattn commented Feb 12, 2019

mattn commented Feb 12, 2019

h-east commented Feb 12, 2019

k-takata commented Feb 12, 2019

k-takata commented Feb 13, 2019

brammool commented Feb 13, 2019 via email

h-east commented Feb 13, 2019

h-east commented Feb 17, 2019

k-takata commented Feb 17, 2019

h-east commented Feb 17, 2019

brammool commented Feb 17, 2019

h-east commented Feb 17, 2019

h-east commented Feb 17, 2019 • edited

k-takata commented Feb 17, 2019

k-takata commented Feb 17, 2019

h-east commented Feb 18, 2019

h-east commented Feb 11, 2019 •

edited

codecov-io commented Feb 11, 2019 •

edited

h-east commented Feb 17, 2019 •

edited