New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 690114 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS:
Pri: 2
Type: Bug



Sign in to add a comment

clang-format quality problem: argument packing is hard to read with arity >4

Project Member Reported by rsesek@chromium.org, Feb 8 2017

Issue description

clang-format produced code that (choose all that apply): 
- Doesn't match Chromium style
- Doesn't match blink style
[x] Riles my finely honed stylistic dander
[x] No sane human would ever choose

Here's the code before formatting:

  kern_return_t result =
      mach_vm_region(mach_task_self(),
                     reprotection_start,
                     reprotection_length,
                     VM_REGION_BASIC_INFO_64,
                     reinterpret_cast<vm_region_info_t>(&info),
                     &count,
                     &unused);


Here's the code after formatting:

  kern_return_t result = mach_vm_region(
      mach_task_self(), reprotection_start, reprotection_length,
      VM_REGION_BASIC_INFO_64, reinterpret_cast<vm_region_info_t>(&info),
      &count, &unused);

Here's how it ought to look:

  kern_return_t result =
      mach_vm_region(mach_task_self(),
                     reprotection_start,
                     reprotection_length,
                     VM_REGION_BASIC_INFO_64,
                     reinterpret_cast<vm_region_info_t>(&info),
                     &count,
                     &unused);

Code review link for full files/context:
The reviewer also thought this was a change for the worse: https://codereview.chromium.org/2649973003/diff/20001/base/allocator/allocator_interception_mac.mm#newcode66

The argument list is objectively harder to read when the arguments are smooshed together like that.
 
Filed upstream as https://llvm.org/bugs/show_bug.cgi?id=31907
Components: Tools
Labels: clang-format
Labels: Pri-2
Issue has a component, but no priority. Updating to have default priority (Pri-2)

Sign in to add a comment