ebevhan added a comment.
Sorry for the delay! I rebased the patch onto latest and it mostly worked, with
a few hitches.
================
Comment at: clang/lib/Sema/SemaCast.cpp:2617-2619
+ (Self.getLangOpts().OpenCL &&
+ (DestPPointee->isFunctionType() ||
+ SrcPPointee->isFunctionType())))) {
----------------
Without this, `SemaOpenCL/func.cl` fails since `foo((void*)foo)` is no longer
an error. Since the function pointer is Default-qualified,
`isExplicitAddrSpaceConversionLegal` delegates the check to the target, which
will always return true.
To be honest, the clause in that method for letting targets allow conversions
where `LangAS::Default` is involved only works so long as every type in OpenCL
has the "right" semantic AS. When any of them are Default, things like this
happen.
Maybe we should only ask the target about explicit conversions if both ASes are
Default or a target AS?
================
Comment at: clang/lib/Sema/SemaOverload.cpp:5289
+ // FIXME: hasAddressSpace is wrong; this check will be skipped if FromType is
+ // not qualified with an address space, but if there's no implicit conversion
----------------
Anastasia wrote:
> ebevhan wrote:
> > Anastasia wrote:
> > > Do you have a failing test case, if so feel free to create a bug?
> > Unsure how I'd make one. I suspect this can't be triggered in OpenCL++,
> > because you can't really have LangAS::Default on FromType there, can you?
> > It would always be some AS.
> >
> > Doing it in another way would require a target that has configurable ASes,
> > which doesn't exist yet. Also, it would require methods qualified with
> > target ASes, and that doesn't work yet either.
> Ok, that's right in OpenCL almost every type gets an address space early in
> parsing.
>
> But if we already know it's a bug and the fix for it we could change this
> code? Although I think the bug fix should better be done on a separate review.
I made the fix here just to try, and it caused a difference in
address-space-lambda.clcpp.
I'm not sure why that lambda would be valid just because `mutable` is added...
So this seems like the correct behavior to me?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62574/new/
https://reviews.llvm.org/D62574
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
