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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
