[PATCH] D62156: [Sema][PR41730] Diagnose addr space mismatch while constructing objects

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:8219
+          // to allow skipping it. However we are not visiting all qual
+          // currently and therefore there is no condition yet.
           Diag(SL, diag::err_invalid_qualified_constructor)
----------------
I don't think this is a FIXME; it's just a notice to future maintainers.  How about something like: "We want to emit a diagnostic on any qualifier except an address-space qualifier.  However, forEachQualifier currently doesn't visit address-space qualifiers, so there's no way to write this condition right now; we just diagnose on everything."


================
Comment at: lib/Sema/SemaOverload.cpp:6095
+    // Check that addr space of an object being constructed is convertible to
+    // the one ctor qualified with.
+    if (!Qualifiers::isAddressSpaceSupersetOf(
----------------
Anastasia wrote:
> rjmccall wrote:
> > "Check that the constructor is capable of constructing an object in the destination address space."
> > 
> > Should there be an exception here for trivial default/copy/move constructors?
> Good point. Current implementation is generating one overload of each default/copy/move in generic address space only. But we could potentially look at changing this. If we could add extra overloads once we encounter each new ctor invocation it would be the easiest approach and this code would still be applicable. However, I would need to understand the feasibility in more details. May be this is something for the future work? Or do you have other suggestions?     
It's fine to leave it as future work.  I think synthesizing new overloads after the fact will probably be problematic for the AST, though, and it'll be easier to just treat trivial copy/move constructors (and assignment operators, for that matter) as implicitly being able to construct objects in all possible address spaces (as well as *from* all possible address spaces).


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

https://reviews.llvm.org/D62156