Anastasia marked 2 inline comments as done.
Anastasia added inline comments.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:6721
+ if (getLangOpts().OpenCL) {
+
----------------
rjmccall wrote:
> Since you're moving this code anyway, can this be split into its own
> function? I'm not sure if it's actually important that some of these
> failures return immediately and some of them fall through to later checks.
> I'm not sure if it's actually important that some of these failures return
> immediately and some of them fall through to later checks.
Yes, it looks a bit random. Do we have any guideline whether we should return
or keep diagnosing as long as possible?
================
Comment at: clang/lib/Sema/SemaType.cpp:7460
+ // the initializing expression type during the type deduction.
+ (T->isUndeducedAutoType() && IsPointee) ||
// OpenCL spec v2.0 s6.9.b:
----------------
rjmccall wrote:
> Anastasia wrote:
> > rjmccall wrote:
> > > Okay, I understand why you're doing this now, and it makes sense. I
> > > would like to propose changing the entire way
> > > `deduceOpenCLImplicitAddrSpace` works. Why don't we do it more like how
> > > ObjC ARC infers its implicit ownership qualifiers:
> > >
> > > - In SemaType, we add the default address space to non-qualified,
> > > non-dependent, non-undeduced-`auto` pointees when parsing a pointer or
> > > reference type.
> > > - In SemaType, we add the default address space to non-qualified pointees
> > > when building a pointer or reference type.
> > > - We add the default address space at the top level when when building a
> > > variable.
> > >
> > > Then all of this context-specific logic where we're looking at different
> > > declarator chunks and trying to infer the relationship of the current
> > > chunk to the overall type being parsed can just go away or get pushed
> > > into a more appropriate position.
> > Ok, it mainly works, however I still need a bit of parsing horribleness
> > when deducing addr space of declarations with parenthesis in
> > `GetFullTypeForDeclarator`. This is the case for block pointers or
> > pointers/references to arrays. It is incorrect to add address spaces on
> > ParenType while building a pointer or references so I have to detect this
> > as special case.
> You can't add an address space outside a `ParenType`? That seems odd; what
> problems are you seeing exactly?
>
> If it's really just specific to `ParenType`, you could simply drill through
> them and then rebuild the `ParenType`s.
> You can't add an address space outside a `ParenType`? That seems odd; what
> problems are you seeing exactly?
When we add addr space for a pointee and it's a `ParenType` the addr space
should actually be added to a first non-`ParenType` but not `ParenType` itself.
For example is we declare a reference to an array we want the array type to
have an address space not `ParenType`.
> If it's really just specific to ParenType, you could simply drill through
> them and then rebuild the ParenTypes.
Ok, in the case I explained above we would have to add address space to the
first non-`ParenTypes` and then rebuild all `ParenType`s. I will try that.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65744/new/
https://reviews.llvm.org/D65744
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
