JonasToth added a comment.

In https://reviews.llvm.org/D41648#1047799, @JonasToth wrote:

> 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.


@aaron.ballman Did you have time to look at my analysis result?


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