google / google/gvisor

[checklocks] allow //+checklocks:s.mu directives to name multiple protected fields (mutexHat-style)

Open
#12,648 1 comment 1 reaction 0 assignees View on GitHub
type: enhancement
Dominant language
Go
Stars
19.3k
Forks
2k
Avg merge
3d 5h
Merged PRs (30d)
264

Description

### Description

`checklocks` already provides annotations that let developers express which locks must be held when accessing a field. This works well when annotating fields directly. However, in the “mutex-hat” style (annotating the mutex with what it protects), a single mutex often guards *several* fields, and the current `+checklocks:s.mu` directive only supports naming one field at a time. This creates repetitive boilerplate.

This issue requests support for listing multiple protected fields in a single `+checklocks:s.mu` directive.

#### Motivation / Problem

In many codebases, the documentation pattern looks like:

- A mutex `mu` protects several fields
- The mutex (or its comment) is where that protection relationship is documented

This keeps “what the mutex protects” next to the mutex and avoids scattering lock info across many fields. It is similar to the go-critic’s `mutexHat` request:
- https://github.com/go-critic/go-critic/issues/465

Today, with `checklocks`, expressing that `s.mu` protects multiple fields requires repeating the directive multiple times, or moving to field-level annotations everywhere. The repeated directive approach is noisy and scales poorly for structs with many guarded fields.

### Is this feature related to a specific bug?

_No_

### Do you have a specific solution in mind?

Allow `// +checklocks:s.mu` directives to name *multiple fields* in a single directive.

#### Proposed syntax options

Any of these would work; the main goal is “one directive → many fields”.

```go

// Option A: comma-separated list

type s struct {
mu sync.Mutex
// +checklocks:s.mu=rateLimits,mostRecent,foo,bar
rateLimits Rate
mostRecent rateLimitCategory
foo int
bar string
}

// Option B: space-separated list

type s struct {
mu sync.Mutex
// +checklocks:s.mu rateLimits mostRecent foo bar
rateLimits Rate
mostRecent rateLimitCategory
foo int
bar string
}

// Option C: bracketed list

type s struct {
mu sync.Mutex
// +checklocks:s.mu=[rateLimits mostRecent foo bar]
rateLimits Rate
mostRecent rateLimitCategory
foo int
bar string
}

```

_(Whatever best matches existing directive parsing conventions.)_

Expected behavior
• checklocks should treat each listed field as guarded by s.mu exactly as if each field had its own “field is guarded by mu” annotation.
• Multiple +checklocks:s.mu directives should merge (union) rather than override.
• Existing single-field usage must continue to work unchanged.

**Example of desired semantics**

Given:

```go
type s struct {
mu sync.Mutex
// +checklocks:s.mu=rateLimits,mostRecent
rateLimits Rate
mostRecent rateLimitCategory
}
```

Then accesses to rateLimits or mostRecent without holding mu should be flagged the same way as if the fields were individually annotated.

**Why this helps**
• Greatly reduces annotation noise when one mutex protects many fields.
• Encourages the “mutex-hat” style where the mutex declares what it guards.
• Improves readability for large structs by keeping protection relationships centralized.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.