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



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

Reply via email to