JustinStitt wrote: > This test is meant to document the current behavior as a baseline, so that > when we implement `UBSAN` null/alignment checks for aggregate copies in a > follow-up PR, we can update the test accordingly and verify the before/after > changes.
Ok I see. I think I would have understood better if the line in your PR description was changed a bit: ```diff -Add test coverage for UBSAN null and alignment sanitizer checks on aggregate copy operations. +Add tests verifying the lack of UBSAN null and alignment sanitizer instrumentation on aggregate copy operations. ``` My main worry with the wording is if someone down the line is trying to do some git archaeology and finds a commit titled `[UBSAN] [NFC] add coverage for null and alignment checks` they may be confused to find out it doesn't add coverage for actual sanitizer instrumentation but rather tests for the lack of sanitizer instrumentation. The tests look fine and pass so LGTM but you should wait for at least one other person because they may want you to just carry your tests downstream and only submit tests with the implementation PR that covers the blindspots of the sanitizers. https://github.com/llvm/llvm-project/pull/176210 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
