Caslyn added a comment. Thanks for the review @PiotrZSL, I’m new to this space and appreciate the comments. I have a few questions around some of them and would greatly appreciate any guidance you can give.
The intent of this patch is to upstream a generic version of the google style check (apologies for the private link) for use on Fuchsia and other projects. I’m happy to make this check a misc check that handles initialization and destruction if you wouldn’t mind advising a little bit. To handle the initialization aspect, would you suggest I combine logic from the existing fuchsia-statically-constructed-objects check? > Please also look into: https://github.com/llvm/llvm-project/issues/62334 I see there’s some potential clean up for the fuchsia module. I can take a stab at that ticket after this patch. ================ Comment at: clang-tools-extra/clang-tidy/fuchsia/GlobalVariablesCheck.cpp:31-32 + unless(allOf(hasType(references(qualType())), + unless(hasInitializer(exprWithCleanups( + has(materializeTemporaryExpr()))))))); + ---------------- PiotrZSL wrote: > there can be some implicitCasts here, and it wont match, also > CXXBindTemporaryExpr can show up, this isn't a good way... Is there a better way to implement this intent while handling those corner cases? ================ Comment at: clang-tools-extra/clang-tidy/fuchsia/GlobalVariablesCheck.cpp:38 + varDecl(is_global_or_static_candidate, + unless(hasType(cxxRecordDecl(hasAnyName(IgnoreVars)))), + // Special handling for std::array is treated below ---------------- PiotrZSL wrote: > what about typedefs ? There is the typedef scenario tested on L#90 the global-variables.cpp test and I've added an additional test for typedef to references - are there any other test cases I can add to make sure this check handles them correctly? ================ Comment at: clang-tools-extra/clang-tidy/fuchsia/GlobalVariablesCheck.cpp:96 + Result.Nodes.getNodeAs<VarDecl>("global_var_std_array")) { + // In the case of std::array, check the type of the first template argument. + // Zero-size arrays are always allowed. ---------------- PiotrZSL wrote: > if you put non trivial type into std::array, then std::array instance will > become non-trivial itself, no need to look into arguments. Done - removed special handling for `std::array`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152953/new/ https://reviews.llvm.org/D152953 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
