[PATCH] D25139: [CUDA] Add Sema::CUDADiagBuilder and Sema::CUDADiagIfDeviceCode().

[Reid Kleckner via cfe-commits]

rnk added inline comments.


> Sema.h:9210
> +      /// Emit no diagnostics.
> +      NOP,
> +      /// Emit the diagnostic immediately (i.e., behave like Sema::Diag()).

LLVM has a different enum naming convention: http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

> Sema.h:9238
> +    ///
> +    ///   if (CUDADiagBuilder(...) << foo << bar)
> +    ///     return ExprError();

I'm concerned that this usage pattern isn't going to be efficient because you build the complete diagnostic before calling the bool conversion operator to determine that it doesn't need to be emitted. I think you want to construct something more like:

  if (isCUDADeviceCode())
    CUDADiag(...) << ...;

Otherwise you are going to construct and destruct a large number of diagnostics about language features that are forbidden in device code, but are legal in host code, and 99% of the TU is going to be host code that uses these illegal features.

> Sema.h:9258
> +          : Loc(Loc), PD(PartialDiagnostic::NullDiagnostic()), Fn(Fn) {
> +        // We have to do this odd dance to create our PartialDiagnostic (first
> +        // creating a NullDiagnostic(), then calling Reset()) because we want

Remind me why we need to do this? Which arena is this stuff allocated in and where would I go to read more about it? My thought is that, if we don't construct very many of these, we should just allocate them in the usual ASTContext arena and let them live forever. It would be more consistent with our usual way of doing things.

https://reviews.llvm.org/D25139