rSerge added a comment.

I have extended this feature to check for OS support too (currently Linux 
only). I can't commit it so far because I don't know how to implement a test. 
XFAIL cannot check for both CPU and OS: it can only check for one of them. I 
tried to implement 2 tests instead like these:

1. ```// RUN: not %clang -v -fxray-instrument -c %s

// XFAIL: Linux
// REQUIRES-ANY: amd64-, x86_64, x86_64h, arm
typedef int a;

  2) ```// RUN: not %clang -v -fxray-instrument -c %s
  // XFAIL: amd64-, x86_64, x86_64h, arm
  // REQUIRES: Linux
  typedef int a;

However the problem with REQUIRES / REQUIRES-ANY is that they only check in LIT 
features, but not in the target triple. So everything becomes unsupported.

Does anyone have any ideas on how to implement the tests for Clang checking for 
both OS and CPU? I have 2 options in mind:

1. extend LIT, putting OS and CPU into the feature list
2. implement the test via GTest, rather than LIT



================
Comment at: lib/Driver/Tools.cpp:4796
+      Feature += " on ";
+      Feature += Triple.getArchName().data();
+      D.Diag(diag::err_drv_clang_unsupported) << Feature;
----------------
dberris wrote:
> rSerge wrote:
> > dberris wrote:
> > > 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;
> > > ```
> > It returns `StringRef`. `.str()` would construct a `std::string`, which 
> > seems unnecessary.  Of course this piece is not performance-critical, but 
> > why not to just use `operator+=` taking as the argument `const char* const`?
> .data() doesn't necessarily have a null terminator.
> 
> Constructing a string out of successive +'s invoke move constructors on 
> std::string, which makes it as efficient if not more efficient than growing a 
> single string this way. At any rate it's much more readable if you did it in 
> a single line.
I see now, changing.


https://reviews.llvm.org/D24799



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to