AdamMagierFOSS wrote: > > One thing I'll preemptively address is I didn't know where to put the new > > unit testing - creating a separate file seems a little heavy handed but I > > see that there's a test for UBSan shift generation > > (`clang/test/CodeGen/ubsan-shift.c`) and one for UBSan + _BitInt > > (`clang/test/CodeGen/ext-int-sanitizer.cpp`). Both seem equally "valid" but > > neither seem to test in the same way that I'm trying to test these changes. > > Advice on this would be appreciated. > > Either one is fine as long as you aren't having to narrow the portability of > a test case in order to use `_BitInt` in it. > > Patch generally LGTM.
After thinking about it some more maybe it would be best to keep this test its own file. The other two tests check the optimized code but in this test I explicitly want to test the unoptimized code to make sure the base code generation is going as anticipated. The way I understand it, either I include these test cases in the existing tests and there's a disconnect in how the "first part" of the test gets performed vs the "second part" of the test or I keep the test separate. Unless there's a strong opinion otherwise I think this would be the best way forward. https://github.com/llvm/llvm-project/pull/80515 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
