JonasToth added a comment. In D54222#2294123 <https://reviews.llvm.org/D54222#2294123>, @jranieri-grammatech wrote:
> In D54222#1290789 <https://reviews.llvm.org/D54222#1290789>, @JonasToth wrote: > >> I agree with @Eugene.Zelenko that this check should rather live in >> `bugprone-` as the issue its diagnosing is not LLVM specific. It is ok to >> add an alias into the llvm module. > > Here are the warnings it issues with the patch's matcher: > > clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:625:43: warning: > address of static local variable 'tag' may not be identical across library > boundaries [llvm-problematic-statics] > > clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:219:1: > warning: address of static local variable 'Index' may not be identical > across library boundaries [llvm-problematic-statics] > > clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h:25:1: > warning: address of static local variable 'Index' may not be identical > across library boundaries [llvm-problematic-statics] > > And here are the additional warnings it issues when relaxing the matcher (to > be `unless(isExpansionInMainFile())`): > > clang/lib/StaticAnalyzer/Checkers/Iterator.h:130:47: warning: address of > static local variable 'Index' may not be identical across library boundaries > [llvm-problematic-statics] > clang/lib/StaticAnalyzer/Checkers/Iterator.h:136:47: warning: address of > static local variable 'Index' may not be identical across library boundaries > [llvm-problematic-statics] > clang/lib/StaticAnalyzer/Checkers/Iterator.h:142:47: warning: address of > static local variable 'Index' may not be identical across library boundaries > [llvm-problematic-statics] > clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h:60:5: warning: > address of static local variable 'index' may not be identical across library > boundaries [llvm-problematic-statics] > llvm/utils/unittest/googletest/src/gtest.cc:3989:3: warning: address of > static local variable 'instance' may not be identical across library > boundaries [llvm-problematic-statics] > > And also this one, which was hidden by default but made running tidy noisier > by always saying there was a hidden warning: > > /usr/bin/../include/c++/v1/functional:1960:9: warning: address of static > local variable '__policy_' may not be identical across library boundaries > [llvm-problematic-statics] > > All of the first set of warnings might be actual problems, but the second set > definitely aren't problems because they aren't going to be used across shared > library boundaries. By making this be a LLVM-specific check, we can avoid the > noise of these false positives. Also, I looked through the C++ Core > Guidelines for any rules that might address this but didn't see anything > relevant. > > Thoughts? Do you have data for other projects? As this is not a very common thing and probably different for code-bases with plugins and so on, the "chattiness" of the check would be interesting to know. If the check is usually quiet, then i would think its ok to have it as a general check together with proper documentation explaining why these statics can be a problem. I would still like to have it in `bugprone-`, because this is a real problem that can arise and the check is more likely to be ignored if considered to be "just for llvm". The decision for true vs false positive is not possible to decide within clang-tidy, is it? I think this should then be left to the developer (as it is probably not that much?). Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54222/new/ https://reviews.llvm.org/D54222 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
