llvm / llvm/llvm-project

ClangTidy check *-magic-numbers does not report magic macro arguments

Open
#163,426 4 comments 0 reactions 0 assignees View on GitHub
clang-tidy false-negative
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

*This was originally reported in clangd/clangd#2509.*

*Reproduced with clang-tidy alone on Debian.*

`*-magic-numbers` catches function arguments fine, but does not report anything for magic macro arguments.

Basic reproduction:

```yaml
# .clangd

Diagnostics:
ClangTidy:
Add: "*-magic-numbers"
```

```c
// test.c

#define ADD_ONE_MACRO(x) ((x) + 1)

int AddOneFunction (int x) {
return x + 1;
}

int main (void) {
int magicNumber = 5; // Diagnosed as "5 is a magic number"
int magicFuncArg = AddOneFunction(7); // Diagnosed as "7 is a magic number"
int magicMacroArg = ADD_ONE_MACRO(9); // NOT diagnosed
return 0;
}
```

**Logs**

*Log generated from a clean output, then pasting in full block of code.*

Log

```
V[01:53:35.837] <<< {"jsonrpc":"2.0","method":"textDocument/didChange","params":{"contentChanges":[{"range":{"end":{"character":0,"line":0},"start":{"character":0,"line":0}},"rangeLength":0,"text":"#define ADD_ONE_MACRO(x) ((x) + 1)\n\nint AddOneFunction (int x) {\n return x + 1;\n}\n\nint main (void) {\n int magicNumber = 5; // Diagnosed as \"5 is a magic number\"\n int magicFuncArg = AddOneFunction(7); // Diagnosed as \"7 is a magic number\"\n int magicMacroArg = ADD_ONE_MACRO(9); // NOT diagnosed\n return 0;\n}\n"}],"textDocument":{"uri":"file:///f%3A/Creative/test.c","version":448}}}

I[01:53:35.837] <-- textDocument/didChange
I[01:53:35.837] --> textDocument/clangd.fileStatus
V[01:53:35.837] >>> {"jsonrpc":"2.0","method":"textDocument/clangd.fileStatus","params":{"state":"file is queued","uri":"file:///f:/Creative/test.c"}}

V[01:53:35.873] <<< {"id":143,"jsonrpc":"2.0","method":"textDocument/inlayHint","params":{"range":{"end":{"character":0,"line":12},"start":{"character":0,"line":0}},"textDocument":{"uri":"file:///f%3A/Creative/test.c"}}}

I[01:53:35.873] <-- textDocument/inlayHint(143)
I[01:53:35.874] Failed to find compilation database for f:\Creative\test.c
I[01:53:35.874] ASTWorker building file f:\Creative\test.c version 448 with command clangd fallback
[f:\Creative]
"C:\\tools\\mingw32\\bin\\clang" "-resource-dir=C:\\tools\\clangd\\lib\\clang\\20" -- "f:\\Creative\\test.c"
V[01:53:35.879] Driver produced command: cc1 -cc1 -triple x86_64-pc-windows-msvc19.33.0 -fsyntax-only -disable-free -clear-ast-before-backend -disable-llvm-verifier -discard-value-names -main-file-name test.c -mrelocation-model pic -pic-level 2 -mframe-pointer=none -relaxed-aliasing -fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases -fms-volatile -funwind-tables=2 -target-cpu x86-64 -tune-cpu generic "-fdebug-compilation-dir=f:\\Creative" "-fcoverage-compilation-dir=f:\\Creative" -resource-dir "C:\\tools\\clangd\\lib\\clang\\20" -internal-isystem "C:\\tools\\clangd\\lib\\clang\\20\\include" -internal-isystem "C:/Program Files/Microsoft Visual Studio 10.0/VC/include" -internal-isystem "C:/Program Files/Microsoft Visual Studio 9.0/VC/include" -internal-isystem "C:/Program Files/Microsoft Visual Studio 9.0/VC/PlatformSDK/Include" -internal-isystem "C:/Program Files/Microsoft Visual Studio 8/VC/include" -internal-isystem "C:/Program Files/Microsoft Visual Studio 8/VC/PlatformSDK/Include" -ferror-limit 19 -fno-use-cxa-atexit -fms-extensions -fms-compatibility -fms-compatibility-version=19.33 -fskip-odr-check-in-gmf -fdelayed-template-parsing -no-round-trip-args -faddrsig -x c "f:\\Creative\\test.c"
I[01:53:35.879] --> textDocument/clangd.fileStatus
V[01:53:35.879] >>> {"jsonrpc":"2.0","method":"textDocument/clangd.fileStatus","params":{"state":"parsing includes, parsing main file","uri":"file:///f:/Creative/test.c"}}

V[01:53:35.879] Rebuilding invalidated preamble for f:\Creative\test.c version 448 (previous was version 447)
I[01:53:35.886] Built preamble of size 270956 for file f:\Creative\test.c version 448 in 0.01 seconds
I[01:53:35.886] --> workspace/semanticTokens/refresh(8)
V[01:53:35.886] >>> {"id":8,"jsonrpc":"2.0","method":"workspace/semanticTokens/refresh","params":null}

V[01:53:35.887] indexed preamble AST for f:\Creative\test.c version 448:
symbol slab: 1 symbols, 4456 bytes
ref slab: 0 symbols, 0 refs, 136 bytes
relations slab: 0 relations, 24 bytes
V[01:53:35.887] Build dynamic index for header symbols with estimated memory usage of 10588 bytes
V[01:53:35.887] <<< {"id":8,"jsonrpc":"2.0","result":null}

I[01:53:35.887] <-- reply(8)
V[01:53:35.887] <<< {"id":144,"jsonrpc":"2.0","method":"textDocument/semanticTokens/full/delta","params":{"previousResultId":"14","textDocument":{"uri":"file:///f%3A/Creative/test.c"}}}

I[01:53:35.887] <-- textDocument/semanticTokens/full/delta(144)
V[01:53:35.890] indexed file AST for f:\Creative\test.c version 448:
symbol slab: 3 symbols, 4920 bytes
ref slab: 4 symbols, 5 refs, 4328 bytes
relations slab: 0 relations, 24 bytes
V[01:53:35.890] Build dynamic index for main-file symbols with estimated memory usage of 12664 bytes
I[01:53:35.890] --> textDocument/publishDiagnostics
V[01:53:35.890] >>> {"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"diagnostics":[{"code":"cppcoreguidelines-avoid-magic-numbers","codeDescription":{"href":"https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-magic-numbers.html"},"message":"5 is a magic number; consider replacing it with a named constant","range":{"end":{"character":23,"line":7},"start":{"character":22,"line":7}},"relatedInformation":[],"severity":2,"source":"clang-tidy"},{"code":"cppcoreguidelines-avoid-magic-numbers","codeDescription":{"href":"https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-magic-numbers.html"},"message":"7 is a magic number; consider replacing it with a named constant","range":{"end":{"character":39,"line":8},"start":{"character":38,"line":8}},"relatedInformation":[],"severity":2,"source":"clang-tidy"}],"uri":"file:///f:/Creative/test.c","version":448}}

I[01:53:35.890] --> textDocument/inactiveRegions
V[01:53:35.890] >>> {"jsonrpc":"2.0","method":"textDocument/inactiveRegions","params":{"regions":[],"textDocument":{"uri":"file:///f:/Creative/test.c"}}}

V[01:53:35.902] indexed file AST for f:\Creative\test.c version 448:
symbol slab: 3 symbols, 4920 bytes
ref slab: 3 symbols, 5 refs, 4304 bytes
relations slab: 0 relations, 24 bytes
V[01:53:35.902] Build dynamic index for main-file symbols with estimated memory usage of 12640 bytes
I[01:53:35.902] --> textDocument/publishDiagnostics
V[01:53:35.902] >>> {"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"diagnostics":[{"code":"cppcoreguidelines-avoid-magic-numbers","codeDescription":{"href":"https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-magic-numbers.html"},"message":"5 is a magic number; consider replacing it with a named constant","range":{"end":{"character":23,"line":7},"start":{"character":22,"line":7}},"relatedInformation":[],"severity":2,"source":"clang-tidy"},{"code":"cppcoreguidelines-avoid-magic-numbers","codeDescription":{"href":"https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-magic-numbers.html"},"message":"7 is a magic number; consider replacing it with a named constant","range":{"end":{"character":39,"line":8},"start":{"character":38,"line":8}},"relatedInformation":[],"severity":2,"source":"clang-tidy"}],"uri":"file:///f:/Creative/test.c","version":448}}

I[01:53:35.902] --> textDocument/inactiveRegions
V[01:53:35.902] >>> {"jsonrpc":"2.0","method":"textDocument/inactiveRegions","params":{"regions":[],"textDocument":{"uri":"file:///f:/Creative/test.c"}}}

V[01:53:35.902] ASTWorker running InlayHints on version 448 of f:\Creative\test.c
I[01:53:35.902] --> reply:textDocument/inlayHint(143) 28 ms
V[01:53:35.902] >>> {"id":143,"jsonrpc":"2.0","result":[{"kind":2,"label":[{"value":"x:"}],"paddingLeft":false,"paddingRight":true,"position":{"character":38,"line":8}}]}

V[01:53:35.902] ASTWorker running SemanticHighlights on version 448 of f:\Creative\test.c
I[01:53:35.902] --> reply:textDocument/semanticTokens/full/delta(144) 14 ms
V[01:53:35.902] >>> {"id":144,"jsonrpc":"2.0","result":{"edits":[{"data":[0,8,13,19,131073,2,4,14,3,131075,0,20,1,2,16387,1,11,1,2,16384,0,2,1,21,0,3,4,4,3,131075,1,8,11,1,16387,1,8,12,1,16387,0,15,14,3,131072,1,8,13,1,16387,0,16,13,19,131072],"deleteCount":0,"start":0}],"resultId":"15"}}

I[01:53:35.902] --> textDocument/clangd.fileStatus
V[01:53:35.902] >>> {"jsonrpc":"2.0","method":"textDocument/clangd.fileStatus","params":{"state":"idle","uri":"file:///f:/Creative/test.c"}}

V[01:53:36.051] <<< {"id":145,"jsonrpc":"2.0","method":"textDocument/foldingRange","params":{"textDocument":{"uri":"file:///f%3A/Creative/test.c"}}}

I[01:53:36.051] <-- textDocument/foldingRange(145)
I[01:53:36.051] --> reply:textDocument/foldingRange(145) 0 ms
V[01:53:36.051] >>> {"id":145,"jsonrpc":"2.0","result":[{"endLine":3,"kind":"region","startCharacter":28,"startLine":2},{"endLine":10,"kind":"region","startCharacter":17,"startLine":6}]}

V[01:53:36.187] <<< {"id":146,"jsonrpc":"2.0","method":"textDocument/documentSymbol","params":{"textDocument":{"uri":"file:///f%3A/Creative/test.c"}}}

I[01:53:36.187] <-- textDocument/documentSymbol(146)
V[01:53:36.187] ASTWorker running DocumentSymbols on version 448 of f:\Creative\test.c
I[01:53:36.188] --> reply:textDocument/documentSymbol(146) 0 ms
V[01:53:36.188] >>> {"id":146,"jsonrpc":"2.0","result":[{"detail":"int (int)","kind":12,"name":"AddOneFunction","range":{"end":{"character":1,"line":4},"start":{"character":0,"line":2}},"selectionRange":{"end":{"character":18,"line":2},"start":{"character":4,"line":2}}},{"detail":"int (void)","kind":12,"name":"main","range":{"end":{"character":1,"line":11},"start":{"character":0,"line":6}},"selectionRange":{"end":{"character":8,"line":6},"start":{"character":4,"line":6}}}]}

I[01:53:36.188] --> textDocument/clangd.fileStatus
V[01:53:36.188] >>> {"jsonrpc":"2.0","method":"textDocument/clangd.fileStatus","params":{"state":"idle","uri":"file:///f:/Creative/test.c"}}

V[01:53:36.188] <<< {"id":147,"jsonrpc":"2.0","method":"textDocument/documentSymbol","params":{"textDocument":{"uri":"file:///f%3A/Creative/test.c"}}}

I[01:53:36.188] <-- textDocument/documentSymbol(147)
V[01:53:36.188] ASTWorker running DocumentSymbols on version 448 of f:\Creative\test.c
I[01:53:36.188] --> reply:textDocument/documentSymbol(147) 0 ms
V[01:53:36.188] >>> {"id":147,"jsonrpc":"2.0","result":[{"detail":"int (int)","kind":12,"name":"AddOneFunction","range":{"end":{"character":1,"line":4},"start":{"character":0,"line":2}},"selectionRange":{"end":{"character":18,"line":2},"start":{"character":4,"line":2}}},{"detail":"int (void)","kind":12,"name":"main","range":{"end":{"character":1,"line":11},"start":{"character":0,"line":6}},"selectionRange":{"end":{"character":8,"line":6},"start":{"character":4,"line":6}}}]}

I[01:53:36.188] --> textDocument/clangd.fileStatus
V[01:53:36.188] >>> {"jsonrpc":"2.0","method":"textDocument/clangd.fileStatus","params":{"state":"idle","uri":"file:///f:/Creative/test.c"}}

V[01:53:36.638] <<< {"id":148,"jsonrpc":"2.0","method":"textDocument/inlayHint","params":{"range":{"end":{"character":0,"line":12},"start":{"character":0,"line":0}},"textDocument":{"uri":"file:///f%3A/Creative/test.c"}}}

I[01:53:36.638] <-- textDocument/inlayHint(148)
V[01:53:36.638] ASTWorker running InlayHints on version 448 of f:\Creative\test.c
I[01:53:36.638] --> reply:textDocument/inlayHint(148) 0 ms
V[01:53:36.638] >>> {"id":148,"jsonrpc":"2.0","result":[{"kind":2,"label":[{"value":"x:"}],"paddingLeft":false,"paddingRight":true,"position":{"character":38,"line":8}}]}

I[01:53:36.638] --> textDocument/clangd.fileStatus
V[01:53:36.638] >>> {"jsonrpc":"2.0","method":"textDocument/clangd.fileStatus","params":{"state":"idle","uri":"file:///f:/Creative/test.c"}}

V[01:53:36.838] <<< {"id":149,"jsonrpc":"2.0","method":"textDocument/documentLink","params":{"textDocument":{"uri":"file:///f%3A/Creative/test.c"}}}

I[01:53:36.838] <-- textDocument/documentLink(149)
V[01:53:36.838] ASTWorker running DocumentLinks on version 448 of f:\Creative\test.c
I[01:53:36.838] --> reply:textDocument/documentLink(149) 0 ms
V[01:53:36.838] >>> {"id":149,"jsonrpc":"2.0","result":[]}

I[01:53:36.838] --> textDocument/clangd.fileStatus
V[01:53:36.838] >>> {"jsonrpc":"2.0","method":"textDocument/clangd.fileStatus","params":{"state":"idle","uri":"file:///f:/Creative/test.c"}}
```

**System information**

Output of `clangd --version`:

```
clangd version 20.1.0 (https://github.com/llvm/llvm-project 24a30daaa559829ad079f2ff7f73eb4e18095f88)
Features: windows+grpc
Platform: x86_64-pc-windows-msvc
```

Editor/LSP plugin: VSCodium/VSCode

Operating system: Windows 10

Contributor guide

Open the contributing guide

Research direction

Start with the .clangd configuration and the test.c reproduction, enabling the *-magic-numbers clang-tidy check. Compare diagnostics for the literal, function argument, and macro argument cases. Done means the macro argument 9 is diagnosed consistently with the other magic numbers, with regression coverage for this example.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers, devtools
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.