[PATCH] D152813: [clang][dataflow] Create `Value`s for integer literals.

[Martin Böhme via Phabricator via cfe-commits]

mboehme marked 2 inline comments as done.
mboehme added inline comments.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/Arena.h:89
+  /// `Value`. These literals are the same every time.
+  IntegerValue &makeIntLiteral(llvm::APInt Value);
+
----------------
xazax.hun wrote:
> gribozavr2 wrote:
> > Should we be taking the type into account? If not, why not? Please add the type or document why the type isn't tracked.
> What would happen if we try to create the same value (like 5) but with different bit widths? (E.g., 64 bit integer vs 32 bit integer). I suspect we might end up having the same value twice in our constant pool, which might be fine. Just wanted to double check what is the desired behavior.
> Should we be taking the type into account? If not, why not? Please add the type or document why the type isn't tracked.

I've clarified that the type isn't tracked but is instead determined by the `Expr` that the integer literal is associated with. (This is consistent with our treatment of `IntegerValue`s more generally.)

If we want to do constant propagation through integral conversions, this should be done when processing the `IntegerCast` node.  This would, if necessary, produce an integer literal with a different value.


================
Comment at: clang/include/clang/Analysis/FlowSensitive/Arena.h:89
+  /// `Value`. These literals are the same every time.
+  IntegerValue &makeIntLiteral(llvm::APInt Value);
+
----------------
mboehme wrote:
> xazax.hun wrote:
> > gribozavr2 wrote:
> > > Should we be taking the type into account? If not, why not? Please add the type or document why the type isn't tracked.
> > What would happen if we try to create the same value (like 5) but with different bit widths? (E.g., 64 bit integer vs 32 bit integer). I suspect we might end up having the same value twice in our constant pool, which might be fine. Just wanted to double check what is the desired behavior.
> > Should we be taking the type into account? If not, why not? Please add the type or document why the type isn't tracked.
> 
> I've clarified that the type isn't tracked but is instead determined by the `Expr` that the integer literal is associated with. (This is consistent with our treatment of `IntegerValue`s more generally.)
> 
> If we want to do constant propagation through integral conversions, this should be done when processing the `IntegerCast` node.  This would, if necessary, produce an integer literal with a different value.
> What would happen if we try to create the same value (like 5) but with different bit widths? (E.g., 64 bit integer vs 32 bit integer). I suspect we might end up having the same value twice in our constant pool, which might be fine. Just wanted to double check what is the desired behavior.

This isn't a consideration, as integer literals aren't typed (see reply to comment above).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152813/new/

https://reviews.llvm.org/D152813