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

Remove char_u, long_u, short_u #459

Open
tarruda opened this Issue Apr 3, 2014 · 6 comments

Comments

Projects
None yet
7 participants
@tarruda
Member

tarruda commented Apr 3, 2014

We should replace most occurrences of char_u by uint8_t(where only characters are used it's ok to use plain char) and use standard C99 fixed-size types such as uint16_t/uint_64t instead of long_u/short_u

@ashleyh

This comment has been minimized.

Show comment
Hide comment
@ashleyh

ashleyh Apr 3, 2014

Contributor

👍, also C99 true/false instead of TRUE/FALSE

Contributor

ashleyh commented Apr 3, 2014

👍, also C99 true/false instead of TRUE/FALSE

@philix

This comment has been minimized.

Show comment
Hide comment
@philix

philix Apr 4, 2014

Member

We should keep in mind the problems discussed in #296 (comment)

ref #296

Member

philix commented Apr 4, 2014

We should keep in mind the problems discussed in #296 (comment)

ref #296

@aktau

This comment has been minimized.

Show comment
Hide comment
@aktau

aktau Apr 9, 2014

Member

In a related note: I think it's absolutely essential for maintainability to get rid of the casting madness in legacy vim. All these explicit casts everywhere seriously decrease the readability of the code. I believe Bram himself realized this, hence the usage of macros like STRLEN that automatically cast (yet also look ugly). A beautiful codebase is a codebase people want to work on.

Member

aktau commented Apr 9, 2014

In a related note: I think it's absolutely essential for maintainability to get rid of the casting madness in legacy vim. All these explicit casts everywhere seriously decrease the readability of the code. I believe Bram himself realized this, hence the usage of macros like STRLEN that automatically cast (yet also look ugly). A beautiful codebase is a codebase people want to work on.

elmart added a commit to elmart/neovim that referenced this issue Apr 30, 2014

Remove project int types: Case short_u: Replace with uint16_t.
- Replace short_u with uint16_t (same size, should give no problems).
- When possible, remove explicit downcasts so that they are found when
  flag -Wconversion enabled.
- Remove short_u typedef.

Requested in neovim#459.

elmart added a commit to elmart/neovim that referenced this issue May 1, 2014

Remove project int types: Case int_u: Replace with uint32_t.
- Replace int_u with uint32_t (same size, should give no problems).
  In fact, only usage found seems to be dead code (some functions in
  ui.h seem not to be used anymore).
- Remove int_u typedef.

Requested in neovim#459.

elmart added a commit to elmart/neovim that referenced this issue May 1, 2014

Remove project int types: Case long_i: Replace with plain long.
Replace long_i with plain long.

long_i was just plain long, adding marker __w64, to be used by
Microsoft's compilers only, as an aid when transitioning from 32 bits to
64 bits. Purpose of this marker was, in fact, to make a 32 bit compiler
emit the same warnings that a 64 bit compiler would.

This __w64 marker is nowadays deprecated by said compilers, and use of a
real 64 bit compiler is recommended instead. See
http://msdn.microsoft.com/en-us/library/s04b5w00.aspx for details.

So, there's no reason to maintain this anymore, and thus is removed.

Refactoring long into int64_t is not attempted, as doing that in a bulk
way is too much complicated. That is left to be done later, on a
file-by-file basis, probably intermixed with file-by-file -Wconversion
activation.

Requested in neovim#459.

elmart added a commit to elmart/neovim that referenced this issue May 1, 2014

Remove project int types: Case short_u: Replace with uint16_t.
- Replace short_u with uint16_t (same size, should give no problems).
- When possible, remove explicit downcasts so that they are found when
  flag -Wconversion enabled.
- Remove short_u typedef.

Requested in neovim#459.

elmart added a commit to elmart/neovim that referenced this issue May 1, 2014

Remove project int types: Case int_u: Replace with uint32_t.
- Replace int_u with uint32_t (same size, should give no problems).
  In fact, only usage found seems to be dead code (some functions in
  ui.h seem not to be used anymore).
- Remove int_u typedef.

Requested in neovim#459.

elmart added a commit to elmart/neovim that referenced this issue May 1, 2014

Remove project int types: Case long_i: Replace with plain long.
Replace long_i with plain long.

long_i was just plain long, adding marker __w64, to be used by
Microsoft's compilers only, as an aid when transitioning from 32 bits to
64 bits. Purpose of this marker was, in fact, to make a 32 bit compiler
emit the same warnings that a 64 bit compiler would.

This __w64 marker is nowadays deprecated by said compilers, and use of a
real 64 bit compiler is recommended instead. See
http://msdn.microsoft.com/en-us/library/s04b5w00.aspx for details.

So, there's no reason to maintain this anymore, and thus is removed.

Refactoring long into int64_t is not attempted, as doing that in a bulk
way is too much complicated. That is left to be done later, on a
file-by-file basis, probably intermixed with file-by-file -Wconversion
activation.

Requested in neovim#459.

elmart added a commit to elmart/neovim that referenced this issue May 1, 2014

Remove project int types: Case long_i: Replace with plain long.
Replace long_i with plain long.

long_i was just plain long, adding marker __w64, to be used by
Microsoft's compilers only, as an aid when transitioning from 32 bits to
64 bits. Purpose of this marker was, in fact, to make a 32 bit compiler
emit the same warnings that a 64 bit compiler would.

This __w64 marker is nowadays deprecated by said compilers, and use of a
real 64 bit compiler is recommended instead. See
http://msdn.microsoft.com/en-us/library/s04b5w00.aspx for details.

So, there's no reason to maintain this anymore, and thus is removed.

Refactoring long into int64_t is not attempted, as doing that in a bulk
way is too much complicated. That is left to be done later, on a
file-by-file basis, probably intermixed with file-by-file -Wconversion
activation.

Requested in neovim#459.

elmart added a commit to elmart/neovim that referenced this issue May 3, 2014

Remove project int types: Case short_u: Replace with uint16_t.
- Replace short_u with uint16_t (same size, should give no problems).
- When possible, remove explicit downcasts so that they are found when
  flag -Wconversion enabled.
- Remove short_u typedef.

Requested in neovim#459.

elmart added a commit to elmart/neovim that referenced this issue May 3, 2014

Remove project int types: Case int_u: Replace with uint32_t.
- Replace int_u with uint32_t (same size, should give no problems).
  In fact, only usage found seems to be dead code (some functions in
  ui.h seem not to be used anymore).
- Remove int_u typedef.

Requested in neovim#459.

elmart added a commit to elmart/neovim that referenced this issue May 3, 2014

Remove project int types: Case long_i: Replace with plain long.
Replace long_i with plain long.

long_i was just plain long, adding marker __w64, to be used by
Microsoft's compilers only, as an aid when transitioning from 32 bits to
64 bits. Purpose of this marker was, in fact, to make a 32 bit compiler
emit the same warnings that a 64 bit compiler would.

This __w64 marker is nowadays deprecated by said compilers, and use of a
real 64 bit compiler is recommended instead. See
http://msdn.microsoft.com/en-us/library/s04b5w00.aspx for details.

So, there's no reason to maintain this anymore, and thus is removed.

Refactoring long into int64_t is not attempted, as doing that in a bulk
way is too much complicated. That is left to be done later, on a
file-by-file basis, probably intermixed with file-by-file -Wconversion
activation.

Requested in neovim#459.

justinmk added a commit that referenced this issue May 3, 2014

Remove project int types: Case short_u: Replace with uint16_t.
- Replace short_u with uint16_t (same size, should give no problems).
- When possible, remove explicit downcasts so that they are found when
  flag -Wconversion enabled.
- Remove short_u typedef.

Requested in #459.

justinmk added a commit that referenced this issue May 3, 2014

Remove project int types: Case int_u: Replace with uint32_t.
- Replace int_u with uint32_t (same size, should give no problems).
  In fact, only usage found seems to be dead code (some functions in
  ui.h seem not to be used anymore).
- Remove int_u typedef.

Requested in #459.

justinmk added a commit that referenced this issue May 3, 2014

Remove project int types: Case long_i: Replace with plain long.
Replace long_i with plain long.

long_i was just plain long, adding marker __w64, to be used by
Microsoft's compilers only, as an aid when transitioning from 32 bits to
64 bits. Purpose of this marker was, in fact, to make a 32 bit compiler
emit the same warnings that a 64 bit compiler would.

This __w64 marker is nowadays deprecated by said compilers, and use of a
real 64 bit compiler is recommended instead. See
http://msdn.microsoft.com/en-us/library/s04b5w00.aspx for details.

So, there's no reason to maintain this anymore, and thus is removed.

Refactoring long into int64_t is not attempted, as doing that in a bulk
way is too much complicated. That is left to be done later, on a
file-by-file basis, probably intermixed with file-by-file -Wconversion
activation.

Requested in #459.

@justinmk justinmk added the refactor label Jul 11, 2014

@justinmk justinmk removed the ready label Nov 14, 2014

@cklein

This comment has been minimized.

Show comment
Hide comment
@cklein

cklein Nov 30, 2014

I started working on do_cmdline and do_cmdline_cmd with signatures:

int do_cmdline_cmd(const char *cmd);
int do_cmdline(const char *cmdline, LineGetter fgetline, void *cookie, int flags);

I try to change as little as possible, also by adding a few casts where the type of cmd is determined by another function.

cklein commented Nov 30, 2014

I started working on do_cmdline and do_cmdline_cmd with signatures:

int do_cmdline_cmd(const char *cmd);
int do_cmdline(const char *cmdline, LineGetter fgetline, void *cookie, int flags);

I try to change as little as possible, also by adding a few casts where the type of cmd is determined by another function.

@mikeyjk

This comment has been minimized.

Show comment
Hide comment
@mikeyjk

mikeyjk Mar 26, 2015

"also C99 true/false instead of TRUE/FALSE"

Just a straight string replace?

mikeyjk commented Mar 26, 2015

"also C99 true/false instead of TRUE/FALSE"

Just a straight string replace?

@philix

This comment has been minimized.

Show comment
Hide comment
@philix

philix Mar 31, 2015

Member

@mikeyjk no, the variable types should be changed from int to bool

Member

philix commented Mar 31, 2015

@mikeyjk no, the variable types should be changed from int to bool

mbainter added a commit to mbainter/neovim that referenced this issue Apr 8, 2015

Refactoring default_vim_dir and default_vimruntime_dir to char type n…
…eovim#459

This is an initial pull just to verify I'm on the right track. In
light of that, in vim_getenv() I setup casts to keep things working.
If it looks good I'll use it as an entry point, and start working the
calls that vim_getenv() uses that still have char_u (like remove_tail
and path_tail).

mbainter added a commit to mbainter/neovim that referenced this issue Apr 11, 2015

elmart added a commit that referenced this issue Apr 12, 2015

Refactor default_vim{,runtime}_dir to use char type. #2375
See: #459
Reviewed-by: Justin M. Keyes <justinkz@gmail.com>
Reviewed-by: Eliseo Martínez <eliseomarmol@gmail.com>

mbainter added a commit to mbainter/neovim that referenced this issue Apr 12, 2015

mbainter added a commit to mbainter/neovim that referenced this issue Apr 12, 2015

mbainter added a commit to mbainter/neovim that referenced this issue Apr 12, 2015

mbainter added a commit to mbainter/neovim that referenced this issue Apr 12, 2015

mbainter added a commit to mbainter/neovim that referenced this issue Apr 12, 2015

mbainter added a commit to mbainter/neovim that referenced this issue Apr 12, 2015

mbainter added a commit to mbainter/neovim that referenced this issue Apr 12, 2015

justinmk added a commit that referenced this issue Apr 12, 2015

mbainter added a commit to mbainter/neovim that referenced this issue Apr 12, 2015

mbainter added a commit to mbainter/neovim that referenced this issue Apr 13, 2015

mbainter added a commit to mbainter/neovim that referenced this issue Apr 17, 2015

Shougo added a commit to Shougo/neovim that referenced this issue Apr 20, 2015

Refactor default_vim{,runtime}_dir to use char type. neovim#2375
See: neovim#459
Reviewed-by: Justin M. Keyes <justinkz@gmail.com>
Reviewed-by: Eliseo Martínez <eliseomarmol@gmail.com>

Shougo added a commit to Shougo/neovim that referenced this issue Apr 20, 2015

Shougo added a commit to Shougo/neovim that referenced this issue Apr 20, 2015

mhinz added a commit to mhinz/neovim that referenced this issue Apr 23, 2015

Refactor default_vim{,runtime}_dir to use char type. neovim#2375
See: neovim#459
Reviewed-by: Justin M. Keyes <justinkz@gmail.com>
Reviewed-by: Eliseo Martínez <eliseomarmol@gmail.com>

mhinz added a commit to mhinz/neovim that referenced this issue Apr 23, 2015

mhinz added a commit to mhinz/neovim that referenced this issue Apr 23, 2015

chrisosaurus added a commit to chrisosaurus/neovim that referenced this issue May 8, 2015

chrisosaurus added a commit to chrisosaurus/neovim that referenced this issue May 8, 2015

dwb pushed a commit to dwb/neovim that referenced this issue Feb 21, 2017

Fix sh maker: recognize output format without 'line' from dash (neovi…
…m#459)

This is apparently used on Travis, but makes sense in general - e.g.
probably still for Ubuntu systems?!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment