echristo added inline comments.
================ Comment at: clang/include/clang/Frontend/CodeGenOptions.h:113 + XRay_None, // Emit none of the instrumentation points. + XRay_FunctionExtents, // Only emit function entry/exit instrumentation + // points. ---------------- "function" might spell easier? :) ================ Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3248 + // custom events. + { + auto XRayBundle = CGM.getCodeGenOpts().getXRayInstrumentationBundle(); ---------------- I'd probably spell this code like the block above it rather than this. ================ Comment at: clang/lib/Driver/XRayArgs.cpp:61 D.Diag(diag::err_drv_clang_unsupported) - << (std::string(XRayInstrumentOption) + " on non-supported target OS"); + << (std::string(XRayInstrumentOption) + + " on non-supported target OS"); ---------------- Extraneous reformat. ================ Comment at: clang/lib/Driver/XRayArgs.cpp:86 + Args.getLastArg(options::OPT_fxray_instrumentation_bundle)) { + StringRef B = A->getValue(); + if (B != "all" && B != "none" && B != "function-extents" && ---------------- How about a more descriptive name here and a string switch below? ================ Comment at: clang/lib/Frontend/CompilerInvocation.cpp:452 + StringRef V = A->getValue(); + if (V == "all") + return CodeGenOptions::XRayInstrumentationPointBundle::XRay_All; ---------------- StringSwitch maybe? https://reviews.llvm.org/D44970 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits