JonasToth added a comment.

I checked several codebases and implemented a little helper script to see which 
kind of macros exist. Then I added some filter regex as configuration and tried 
again, here are the results:

For https://github.com/nlohmann/json which is a very high quality codebase, the 
results were as expected: very clean, and good macro usage: (except a `#define 
private public` for tests)

Filter: `JSON*|NLOHMANN*`
F5913499: all_macros.txt <https://reviews.llvm.org/F5913499>

F5913498: filtered_macros.txt <https://reviews.llvm.org/F5913498>

CMake on the other hand uses many macros and don't obey CAPS_ONLY rules for all 
macros.

Filter:  
`_FILE_OFFSET_BITS|AE_*|ARCHIVE_*|cal_*|CMAKE_*|cm*|CM_*|CPP_*|CURL*|E_*|exp_*|F90*|jp*|JSON_*|KW*|kw*|O_*|REQ_*|UV_*|YY*|yy*|Z_*`

F5913502: warned_macros.txt <https://reviews.llvm.org/F5913502>

F5913501: filtered_macros.txt <https://reviews.llvm.org/F5913501>

OpenCV isn't clean either, here the results:

Filter: 
`ASSERT*|ALL*|CAL*|CC*|CALC*|calc*|CL*|cl*|CUDA*|CV*|cv*|EXPECT*|GTEST*|FUNCTOR*|HAVE*|ICV*|IPL*|IPP*|ipp*|__itt*|ITT*|JAS*|jas*|MESSAGE*|MAX*|OCL*|opengl*|OPENCV*|TYP*`

F5913504: all_macros.txt <https://reviews.llvm.org/F5913504>

F5913503: filtered_modules_macros.txt <https://reviews.llvm.org/F5913503>

libtorrent https://github.com/arvidn/libtorrent has reasonable macro usage

Filter: `DEBUG*|LIBTORRENT*|TORRENT*|UNI*`

F5913519: filtered_macros.txt <https://reviews.llvm.org/F5913519>

F5913518: all_macros.txt <https://reviews.llvm.org/F5913518>

LLVM lib/ defines many macros, almost all of them are ALL_CAPS

Filter: 
`AARCH64*|X86*|ARM*|AMDGPU*|ASAN*|ATTR*|CHECK*|COMMON*|CV*|CXX*|DEBUG*|DEC*|DEFINE*|ESAN*|ENUM*|FMA*|FUZZER*|HANDLE*|INTERCEPTOR*|LSAN*|MATH*|MSAN*|OPENMP*|TSAN*|TYPE*|UBSAN*`

F5913505: all_lib_macros.txt <https://reviews.llvm.org/F5913505>
F5913528: filtered_lib_macros.txt <https://reviews.llvm.org/F5913528>

LLVM tools/ is a similar story
Filter: 
`ANALYSIS*|ASSERT*|CHECK*|DEBUG*|DECL*|DEPENDENT*|DIAG*|END*|ENUM*|GENERIC*|IMAGE*|KEYWORD*|LANG*|LLVM*|NON*|OBJC*|OPENMP*|OVERLOAD*|PROC*|REGISTER*|SANITIZER*|STMT*|TYPE*|X86*`

F5913566: all_tools_macros.txt <https://reviews.llvm.org/F5913566>

F5913565: filtered_tools_macros.txt <https://reviews.llvm.org/F5913565>

@aaron.ballman Would you like to see other projects checked? I think this is a 
starting point for now.

My opinion based on what i see is

- maybe two modes for this check make sense, having one requiring CAPS_ONLY and 
the currently implemented stricter check. This allows easier migration to safer 
macro usage
- it is possible to reduce macro usage to a minimal amount, and the complex 
macros like AST stuff can be filtered with the regex. Furthermore, restricting 
all macros to a "macro namespace" is possible for sure.

Things I would like to add to the check:

- my little filtering script is valuable for developers, that want to address 
the macro issue. It should be added to the docs and everyone can make something 
based on that. It will be linux centered.
- enforcing ALL_CAPS, including its usage
- (adding a prefix to all macro, passing the filter, including its usage)

Code transformation has the problems of scope and potentially breaking code 
badly, because clang-tidy wasn't run over all of the code.

The check is chatty, as expected, because macros in header files, especially 
central ones, pollute everything. That is the reason for the check, too. 
Overall I am still in favor of this approach, seeing some obscure macros that 
should be replaced with actual language features (like overloading, .inline 
functions, constants, ...) in the checked codebases.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to