From e3db6f0c3a4932389adac0760343f2f067ed595e Mon Sep 17 00:00:00 2001 From: Andrew Dunham Date: Wed, 26 Jun 2024 12:21:19 -0400 Subject: [PATCH] tools/checklocks: support field comments for guard specifications This allows specifying guards in the format: type foo struct { mu sync.Mutex bar int // +checklocks:mu } This syntax, while more limited, makes writing lock guards less verbose for structures where field documentation isn't attached to each field. For example, being able to write something like the following with field comments reduces the number of lines in the struct by >25% and is, in my opinion, easier to read. type widget struct { mu sync.Mutex // guards following // The following counters track [...] success int // +checklocks:mu failure int // +checklocks:mu retries int // +checklocks:mu timeouts int // +checklocks:mu } Signed-off-by: Andrew Dunham --- tools/checklocks/facts.go | 6 +++++- tools/checklocks/test/basics.go | 10 ++++++++++ tools/checklocks/test/test.go | 7 +++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/tools/checklocks/facts.go b/tools/checklocks/facts.go index d978a2bee..3ea106378 100644 --- a/tools/checklocks/facts.go +++ b/tools/checklocks/facts.go @@ -670,7 +670,11 @@ func (pc *passContext) structLockGuardFacts(structType *types.Struct, ss *ast.St for i, field := range ss.Fields.List { var lgf lockGuardFacts fieldObj = structType.Field(i) // N.B. Captured above. - pc.fillLockGuardFacts(fieldObj, field.Doc, findLocal, &lgf) + if field.Doc != nil { + pc.fillLockGuardFacts(fieldObj, field.Doc, findLocal, &lgf) + } else if field.Comment != nil { + pc.fillLockGuardFacts(fieldObj, field.Comment, findLocal, &lgf) + } // See above, for anonymous structure fields. if ss, ok := field.Type.(*ast.StructType); ok { diff --git a/tools/checklocks/test/basics.go b/tools/checklocks/test/basics.go index e941fba5b..554b88608 100644 --- a/tools/checklocks/test/basics.go +++ b/tools/checklocks/test/basics.go @@ -143,3 +143,13 @@ func testTwoLocksDoubleGuardStructOnlyOne(tc *twoLocksDoubleGuardStruct) { func testTwoLocksDoubleGuardStructInvalid(tc *twoLocksDoubleGuardStruct) { tc.doubleGuardedField = 3 // +checklocksfail:2 } + +func testFieldCommentValid(tc *fieldCommentStruct) { + tc.mu.Lock() + tc.guardedField = 1 + tc.mu.Unlock() +} + +func testFieldCommentInvalid(tc *fieldCommentStruct) { + tc.guardedField = 2 // +checklocksfail +} diff --git a/tools/checklocks/test/test.go b/tools/checklocks/test/test.go index fd2a92162..61d6b3a2b 100644 --- a/tools/checklocks/test/test.go +++ b/tools/checklocks/test/test.go @@ -64,3 +64,10 @@ type nestedGuardStruct struct { val oneGuardStruct ptr *oneGuardStruct } + +// fieldCommentStruct has one lock and a single field, but uses the +// field.Comment instead of field.Doc for the guard. +type fieldCommentStruct struct { + mu sync.Mutex + guardedField int // +checklocks:mu +}