rnk added inline comments.

================
Comment at: lib/Sema/SemaCUDA.cpp:474
@@ +473,3 @@
+  SourceLocation Loc = FD.getLocation();
+  if (!SM.isInSystemHeader(Loc))
+    return false;
----------------
jlebar wrote:
> tra wrote:
> > Can C++ library headers ever be non-system? I.e. can someone use libc++ via 
> > -I ?
> > 
> Good question, I have no idea if that's supposed to work.  Reid, do you know?
libc++ complex has this pragma in it:
  #pragma GCC system_header
So we should be safe regardless of the flags used to find it.

================
Comment at: lib/Sema/SemaCUDA.cpp:483-488
@@ +482,8 @@
+
+  bool IsInStd = FD.isInStdNamespace();
+  if (const auto *Method = dyn_cast<CXXMethodDecl>(&FD))
+    if (const auto *Parent = Method->getParent())
+      IsInStd |= Parent->isInStdNamespace();
+  if (!IsInStd)
+    return false;
+
----------------
I'd do this check after the system header test and before the "complex" test, 
since it's probably faster.

================
Comment at: lib/Sema/SemaCUDA.cpp:485
@@ +484,3 @@
+  if (const auto *Method = dyn_cast<CXXMethodDecl>(&FD))
+    if (const auto *Parent = Method->getParent())
+      IsInStd |= Parent->isInStdNamespace();
----------------
There's no cast on the RHS, so I'd spell out `CXXRecordDecl` here to make 
things more obvious.

================
Comment at: lib/Sema/SemaDecl.cpp:8344
@@ +8343,3 @@
+  // CUDA: Some decls in system headers get an implicit __host__ __device__.
+  if (getLangOpts().CUDA && declShouldBeCUDAHostDevice(*NewFD)) {
+    NewFD->addAttr(CUDADeviceAttr::CreateImplicit(Context));
----------------
Do you want this to apply to declarations as well as definitions? Your test 
uses that functionality.


http://reviews.llvm.org/D18328



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

Reply via email to