[PATCH] D100860: [C++4OpenCL] Add missing OpenCL specific diagnostics in templates

Anastasia Stulova via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 20 09:53:52 PDT 2021


Anastasia added a comment.

> I've left the check during parsing as well, because it provides slightly better error messages, though some are lost now that NewVD is set as invalid, instead of just setting D invalid.

Yes, when there are multiple errors we don't provide the diagnostics consistently so I think it's fine.



================
Comment at: clang/lib/Sema/SemaDecl.cpp:6825
+/// Returns true if there hasn't been any invalid type diagnosed.
+static bool diagnoseOpenCLTypeDecl(Sema &Se, Declarator &D, VarDecl *NewVD) {
+  // C++ for OpenCL does not allow the thread_local storage qualifier.
----------------
I find this function a bit confusing I would just move its body inline where it is currently called.

Although actually could this code be moved to where we do similar check for `static` or `extern`
https://clang.llvm.org/doxygen/SemaDecl_8cpp_source.html#l07911

You would then need to change this slightly i.e. to call `VarDecl::getTSCSpec` instead.


================
Comment at: clang/test/SemaOpenCLCXX/template-diagnostics.clcpp:27
+
+  static __global clk_event_t local_static;
+}
----------------
Let's move this into the OpenCL C test for `clk_event_t` diagnostic `test/SemaOpenCL/clk_event_t.cl`. I would add a `FIXME` comment next to it stating that it is not clear whether this should be diagnosed or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100860



More information about the cfe-commits mailing list