[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

Fri Dec 6 17:47:36 PST 2019

ABataev marked an inline comment as done.
ABataev added inline comments.

================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1230
-  "unknown '%0' device kind trait in the 'device' context selector set, expected"
-  " one of 'host', 'nohost', 'cpu', 'gpu' or 'fpga'">;

----------------
jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > jdoerfert wrote:
> > > > > ABataev wrote:
> > > > > > jdoerfert wrote:
> > > > > > > I would have expected this error to be still accurate, maybe with the addition ", or quoted versions thereof".
> > > > > > Currently, we could emit it only in codegen phase to avoid double converting from expression to string. Will emit it there.
> > > > > Why can't we emit this error if the user writes `device={kind(gggggpu)}` anymore? Even `device={kind("gggggpu")}` should be diagnosable early.
> > > > The main problem here is the conversion and expression evaluation. We convert the data from the expression to strings at the codegen phase. I don't want to do the same thing for the second time in Sema just for diagnostic.
> > > We have to convert it during sema already, actually during parsing. I'm working on declare variant begin/end support right now (part of TR8) which needs the information during parsing.
> > Hmmm, it is where we need to parse the code only if the context matches the trait? Ok, will rework to convert it in sema too. Seems to me, we'll need to match the context in both, codegen and sema. Will try to move the matcher somewhere to avoid duplication in sema and codege.
> I put my patch up later tonight or tomorrow. It moves stuff around, we need to coordinate somehow.
I still won't work on it until Monday. So, if you have a good solution for the matcher, go ahead.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70973/new/

https://reviews.llvm.org/D70973