[clang] [compiler-rt] [libc] [libclc] [libcxxabi] [lld] [lldb] [llvm] [mlir] llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:3804: lacking () for c… (PR #90391)

Alex Langford via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 29 10:52:45 PDT 2024


bulbazord wrote:

> > Please update the PR subject as its a lot more than just X86AsmParser.cpp
> 
> Hi @RKSimon
> 
> All the issues mentioned are fixed. The title of the PR is misleading. The title is the same as the issue (#85868) it corresponds to. Got a look to other PR's and I thought that this is the usual naming convention. Please refer to the [commit](https://github.com/llvm/llvm-project/pull/90391/commits/54c6c6b7d71f5ff293412f5f91e9f880480284f8) and you will see all the modified files.

Commit titles should accurately reflect a change. Your change modifies more than just X86AsmParser.cpp, so reading the commit title alone might lead one to believe that's all that changed. But it's not, you changed many things across the code base (even if the changes are all of the same kind). A more accurate title might be something like: `Add clarifying parenthesis around non-trivial conditions in ternary expressions`.

https://github.com/llvm/llvm-project/pull/90391


More information about the llvm-commits mailing list