APInt API is atrocious

[lattner]

Bugzilla Link	5207
Resolution	FIXED
Resolved on	Dec 09, 2010 02:35
Version	1.0
OS	All
CC	@asl,@jayfoad

Extended Description

The APInt API is extremely inconsistent. Among other things, some methods update the APInt in place, and some return a new APInt. This leads to awful stuff like:

V = V.lshr(ByteStart*8);
V.trunc(ByteSize*8);

I think that we should fix this by changing operations that update in place to have an _eq suffix, this would give us:

V = V.lshr_eq(ByteStart*8);
V = V.trunc_eq(ByteSize*8);

or, better yet in this case:

V.lshr(ByteStart*8);
V.trunc(ByteSize*8);

The ones that update in place should return void to make it really clear that they do not return a new APInt.

[lattner]

Sorry, I got that completely backward. This is what we'd have (foo_eq corresponds to things like +=):

V = V.lshr(ByteStart*8);
V = V.trunc(ByteSize*8);

or, better yet in this case:

V.lshr_eq(ByteStart*8);
V.trunc_eq(ByteSize*8);

[llvmbot]

+1

[jayfoad]

I think that we should fix this by changing operations that update in place to
have an _eq suffix

The public non-const non-static non-operator methods in question are:

• Read(). Doesn't seem to be used or even implemented, and nor is Emit(), so I suggest removing them both. (I've sent a patch for this to llvm-commits, currently awaiting list moderator approval.)

• trunc(), sext(), zext(), sextOrTrunc(), zextOrTrunc().

• set(), clear(), flip(). Are you sure you want these to have an _eq suffix? It doesn't seem to read very nicely, especially the versions that take no argument.

• doubleToBits(), floatToBits(). I suggest changing them to return a new APInt instead. (I've sent a patch for this to llvm-commits, currently awaiting list moderator approval.)

[lattner]

Sounds great!

[jayfoad]

• Read(). Doesn't seem to be used or even implemented, and nor is Emit(), so I
  suggest removing them both.

Done:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20101122/112578.html

• trunc(), sext(), zext(), sextOrTrunc(), zextOrTrunc().

Changed them to return a new value:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20101206/113298.html
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20101206/037106.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20101206/113299.html

• set(), clear(), flip(). Are you sure you want these to have an _eq suffix? It
  doesn't seem to read very nicely, especially the versions that take no
  argument.

Made them return void:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20101129/112774.html

... and renamed them:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20101129/112909.html

• doubleToBits(), floatToBits(). I suggest changing them to return a new APInt
  instead.

Done:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20101122/112577.html

Is there anything else wrong with the APInt API, or can we close the bug?

[lattner]

It's looking great to me new Jay, please do close this. Thanks again for tackling this!

[jayfoad]

Fixed, then!