Delayed Diagnostics broken for some diagnostic types that don't check dependence.

[erichkeane]

After #998 as noticed in #971, some of the diagnostics are not diagnosing in kernels. See this message here:

`The problem is that in SOME cases, we actually visit a diagnostic up to 3x (in the case of kernels).

1: When doing Phase 1 of templates.

2: when doing Phase 2. This is only supposed to happen when something has 'changed' via tree-transform. GCCAsm seems to skip the rebuild in tree-transform if nothing has changed, but apparently CallExpr does NOT. The result is something that has already diagnosed, diagnoses again.

3: When constructing the OpenCL kernel. We likely shouldn't do ANY diagnostics here, since we should have done it in either 1 or 2 above.

Right now, we intentionally skip 3 (thanks to my last change). We are also (perhaps improperly) skipping 1. Due to a bug (IMO) in Tree-transform for CallExpr , the variadic function call is getting diagnosed in all 3. GCC ASM is only getting diagnosed in 1 and 3.

I believe the fix is to repair the CallExpr bug in TreeTransform if I can, and remove the skip of 1.`

I now am not sure that the CallExpr fix is the right thing to do, or that the CallExpr bug is not intended. I suspect the right fix is to simply suppress the attempt to re-diagnose in step 2 above, but I'm still evaluating. Normal diagnostics do this by marking the template invalid when it diagnoses an error. Unfortunately, we cannot do so, because it would cause a number of other errors.

I checked a handful of warnings, and realized we don't prevent them from happening 2x either:
https://godbolt.org/z/LesWP4

[erichkeane]

The immediate problem is fixed here: #1019

As for the more general solution (only diagnosing templates 1x), we likely need to solve this in community for warnings, so we likely want to figure out what happens there.

[erichkeane]

See

// let's have regular "sycl_kernel" function
template <typename name, typename Func>
__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {	
  kernelFunc();
}

// and template function
template<typename T> void some_foo(T t) {
  // Just do some bad things
 throw t; // or throw 3 if it's wrong, whatever
}

int main() {
// And call our template function with bag things inside only from a kernel:
kernel_single_task<class fake_kernel>([]() { some_foo(3); });}

This diagnoses 2x, because the 'throw' doesn't check for dependence (and probably shouldn't?). OpenMP and CUDA have similar throw checks immediately above this, so we need to figure out how it handles this issue.

[erichkeane]

CUDA and OpenMP seem to have the same problem with 'throw'. So I guess this is something we need to solve for everyone?

[bader]

Tagging @alexey-bataev to get an opinion on OpenMP.
Don't know who owns CUDA in clang.

[erichkeane]

Note @alexey-bataev that I didn't come up with a reproducer for openmp, but I didn't see anything in the code to do anything about it, we all (SYCL/CUDA/OpenMP) seem to be using the same infrastructure (creating a delayed diagnostic, then calling emitDeferredDiags).

[erichkeane]

I made a repro for CUDA, humorously they DO skip templates (and I think OpenMP does too), but they end up double diagnosing anyway. Not sure this is the same problem then.

[erichkeane]

HAH! Like the nodiscard, we actually double diagnose this too even without delayed diagnostics: https://godbolt.org/z/aGLQak

[alexey-bataev]

Known problem, sometimes messages may be emitted several times for template instantion case. Plus, seems to me, some of errors does not mark an outer declaration as invalid even without delayed diagnostics, this must be fixed of course.

[erichkeane]

@alexey-bataev Thanks! I hadn't noticed any cases where non-delayed diagnostics (other than the ones that are specifically trying to exclude themselves for delayed diagnostics) don't do this for errors, but we definitely have the problem with warnings in some places.

IMO, this part of the problem is likely a global Clang issue rather than specific to SYCL/OpenMP/etc.

[alexey-bataev]

Hi Erich, yes, it seems to me so. Plus, delayed diagnostics may have their own issues in some corner cases, especially with templates.