tra added a comment.

Looks OK overall except for the huge `if` below.



================
Comment at: lib/Sema/SemaDecl.cpp:11923-11930
+      if (getLangOpts().CUDA &&
+          !(VD->hasAttr<CUDASharedAttr>() ||
+            (VD->getType().isConstQualified() &&
+             !VD->hasAttr<CUDADeviceAttr>() &&
+             !VD->hasAttr<CUDAConstantAttr>())) &&
           CUDADiagIfDeviceCode(VD->getLocation(),
                                diag::err_device_static_local_var)
----------------
This is rather convoluted. It would make it somewhat more readable if we could 
split CUDADiagIfDeviceCode into its own if().

Or, maybe use a lambda + early exit or, perhaps even goto to break down this 
huge if:

```
[&](){
   if (VD->hasAttr<CUDASharedAttr>()) return;
   if (VD->getType().isConstQualified() 
        && !(VD->hasAttr<CUDADeviceAttr>()||VD->hasAttr<CUDAConstantAttr>())
        return;
   if (CUDADiagIfDeviceCode(VD->getLocation(), 
diag::err_device_static_local_var)
              << CurrentCUDATarget()))
          VD->setInvalidDecl();
}()
```


https://reviews.llvm.org/D49931



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

Reply via email to