xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land.
In D116370#3213191 <https://reviews.llvm.org/D116370#3213191>, @ymandel wrote: > In D116370#3213120 <https://reviews.llvm.org/D116370#3213120>, @xazax.hun > wrote: > >> Does it make sense to have both the single and multi variable tracking >> tests? Or do we want to have these as some sort of steps in a tutorial? > > Right -- the intent in keeping the original one was for an (eventual) > tutorial. For that matter, if you think there's changes I should make to the > single variable version to bring them closer together, I'd be happy to do > that. I think from the tutorial's point of view that would be awesome! But I'm also happy with doing that in a follow-up PR some later time. ================ Comment at: clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp:53 + }; + // `State` determines the meaning of the lattice when `Value` is `None`: + // * `Undefined` -> bottom, ---------------- If `ValueSate` only makes sense when `Value` is none, this almost looks like a `std::variant<ValueState, int64_t>`. Unfortunately, as far as I understand LLVM is not at C++17 yet. I did not find an equivalent structure in the LLVM support library, but in case you agree, I think we could leave a TODO to change this after the C++ standard requirement was raised. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116370/new/ https://reviews.llvm.org/D116370 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
