[PATCH] D24799: [XRay] Check in Clang whether XRay supports the target when -fxray-instrument is passed

[Dean Michael Berris via cfe-commits]

dberris requested changes to this revision.
dberris added a comment.
This revision now requires changes to proceed.

Are we sure this will not fail on Windows? i.e. have you built/run the tests on Windws ARM or X86_64?



> Tools.cpp:4787
>                     options::OPT_fnoxray_instrument, false)) {
> -    CmdArgs.push_back("-fxray-instrument");
> +    const char* const XRayInstrumentOption = "-fxray-instrument";
> +    switch(getToolChain().getArch()) {

Why can't this just be a `const string`, or a `const StringRef`?

> Tools.cpp:4796
> +      Feature += " on ";
> +      Feature += Triple.getArchName().data();
> +      D.Diag(diag::err_drv_clang_unsupported) << Feature;

I'm not a fan of calling `.data()` here -- if this returns a `StringRef` I'd much rather turn it into a string directly. Either that or you can even string these together. Say something like:

  default:
    D.Diag(...) << (XRayInstrumentOption + " on " + Triple.getArchName().str());
    break;

> Tools.cpp:4799
> +      break;
> +    } }
> +    CmdArgs.push_back(XRayInstrumentOption);

As part of my submitting this change upstream, I had to format it accordingly with `clang-format` -- you might want to do the same so that the formatting is in-line with the LLVM developers style guidelines.

https://reviews.llvm.org/D24799