MaskRay wrote: > > LGTM. > > Thanks for the review. As we discussed previously, I believe this patch's > effect will be highly visible throughout the LLVM community. Unless there are > bugs we missed, I do not know how anyone might argue it worsens their use > cases. But I have been surprised before. Do you think we should ask for > additional reviews of at least the behavioral change (e.g., by looking > through test changes)? Or should we post a PSA before landing?
Yes, it's a good idea to invite suggestions on the syntax... > > The patch's new code is fully exercised in a coverage build. > > I do not have much experience with LLVM's coverage tools. Is there a public > report? Or can you give me a brief description of what you did? It's not a public report. I did a `LLVM_BUILD_INSTRUMENTED_COVERAGE=ON `LLVM_PROFILE_FILE_PATTERN=/tmp/out/profraw/%4m.profraw -DCMAKE_CXX_FLAGS=-fprofile-continuous` build to check whether all new conditions are covered by tests. https://github.com/llvm/llvm-project/pull/198138 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
