dblaikie added a comment. (still a fair few unhandled comments from the last round - guess work to address those is still ongoing)
================ Comment at: llvm/include/llvm/Support/Compression.h:95-98 + static CompressionSpecRef Unknown; + static CompressionSpecRef None; + static CompressionSpecRef Zlib; + static CompressionSpecRef ZStd; ---------------- ckissane wrote: > dblaikie wrote: > > Generally we don't want more variables that need global constructors in > > LLVM - so these should probably be function-local statics in functions > > instead. > > (I don't think we need a CompressionSpecRef for `Unknown` or `None`, though) > these are just shortcuts to the function local statics of `CompressionSpecRef > getCompressionSpec(uint8_t Kind)` Yeah, though they're still globals with non-trivial construction, which is to be avoided in LLVM ( https://llvm.org/docs/CodingStandards.html#do-not-use-static-constructors ) So they should either be removed (I think that's probably the right tradeoff, and the one I showed in my proposal - callers that want only one specific compression algorithm can pay the small runtime overhead of going through the switch unnecessarily) or replaced with global functions that use function local statics so the initialization is only paid when the functions are called, and not by every program that links in LLVM for any reason. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131992/new/ https://reviews.llvm.org/D131992 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
