zygoloid wrote: > I'm not opposed to the changes, though I do find it somewhat strange to add > them to UBSan given that they're not undefined behavior (even though there's > precedent).
This is adding checks to `-fsanitize=implicit-conversion`, which is not part of `-fsanitize=undefined`. It's sort of ambiguous whether UBSan means `-fsanitize=undefined` (that is, checks for UB that can be done without a big runtime), or whether it means any of the checks that the UBSan infrastructure and runtime power, but the established precedent is we also use the UBSan infrastructure to do non-UB data-loss checks like this, and this patch seems like a logical extension of the existing behavior. I'd be happier if we drew a brighter distinction between the UB checks and the non-UB checks that our sanitizers perform. For example, perhaps the non-UB sanitizers shouldn't be listed in https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html but in a different page, or perhaps at least in a different section from the other checks. But I think that's independent of this PR, and in the context of this PR, I don't think we should be re-litigating whether this kind of data-loss-through-conversion check is in scope. https://github.com/llvm/llvm-project/pull/75481 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
