[PATCH] D25264: Implement MS _BitScan intrinsics

Albert Gutowski via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 5 09:55:12 PDT 2016


agutowski added inline comments.


> majnemer wrote in CGBuiltin.cpp:2640-2647
> This should be in an anonymous namespace. Also, consider using an `enum class` instead of an `enum` nested inside a namespace.

I can see three options:
 (1) put the existing code inside an anonymous namespace;
 (2) create a private enum class in CodeGenFunction;
 (3) pull EmitMSVCBuiltinExpr outside of CodeGenFunction and take CGF object as an argument (and then make enum class inside an anonymous namespace);
I don't really like any of them. Enum class sounds nice as I can imagine someone trying to pass the global builtin ID (like X86::BI_BitScanForward) instead of the one from MSVCIntrin namespace (although it should fail on any test; still, it would compile without enum class). But builtins use CGF methods all of the time, so pulling it out will make the code of every bulitin uglier. So I guess I'll try to go with the private enum class inside the CodeGenFunction, but if you have any better ideas, I'm eager to listen.

https://reviews.llvm.org/D25264





More information about the cfe-commits mailing list