[PATCH] D74144: [OPENMP50]Add basic support for array-shaping operation.
John McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 25 14:06:05 PDT 2020
rjmccall added a comment.
> We cannot build an array type since this expression does not represent a supported array type. Sizes may be non-constant anywhere in the operation, variable array type does not support it.
I don't think that's true; the element type of a C99 VAT can still be another VAT. The only restriction is that we don't allow a CAT of VATs — we require the outer array to still be represented as a VAT despite have a constant immediate bound — but that's just a representational restriction and doesn't restrict what we can express. But it doesn't matter because, as you say, we can and should use a placeholder here in order to enforce the use-restriction.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:4751
+ LParenLoc, RParenLoc, Dims, Brackets);
+ if (!BaseTy->isAnyPointerType())
+ return ExprError(Diag(Base->getExprLoc(),
----------------
ABataev wrote:
> ABataev wrote:
> > rjmccall wrote:
> > > I think you should perform DefaultFunctionArrayLvalueConversion here so that e.g. arrays will decay to pointers, you load pointers from l-values, and so on. If you do so, it'll handle placeholders for you.
> > >
> > > Do you really want to allow this to operate on non-C pointer types?
> > 1. Standard clearly states that the type of the base expression must be a pointer. I don't think that we should perform implicit type casting here, like decay to pointers, etc.
> > 2. It is just a simple form of checking that this is a pointer type. Since this expression is not allowed in other languages (and I filter it out at the parsing stage), I think it is ok to use the generic form of type checking.
> Forgot to mention, that I thought about possible conversion here. But it may lead to some unpredictable results, like:
> ```
> int a[3];
> ...([3][4][n])a...
> ```
> Better to do not allow this kind of operation, I think.
> Standard clearly states that the type of the base expression must be a pointer. I don't think that we should perform implicit type casting here, like decay to pointers, etc.
The wording of the C/C++ standard is that expressions of array type decay except in certain syntactic positions. Such expressions then *do* have pointer type for this purpose. If precedent is to forbid array references, so be it, but that's not consistent with the normal language behavior.
> It is just a simple form of checking that this is a pointer type. Since this expression is not allowed in other languages (and I filter it out at the parsing stage), I think it is ok to use the generic form of type checking.
If your intent is that this only applies only to C pointers, you should just check `isPointerType()` instead of `isAnyPointerType()`, which exists for the sole purpose of also including Objective-C pointers.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:4751
+ LParenLoc, RParenLoc, Dims, Brackets);
+ if (!BaseTy->isAnyPointerType())
+ return ExprError(Diag(Base->getExprLoc(),
----------------
rjmccall wrote:
> ABataev wrote:
> > ABataev wrote:
> > > rjmccall wrote:
> > > > I think you should perform DefaultFunctionArrayLvalueConversion here so that e.g. arrays will decay to pointers, you load pointers from l-values, and so on. If you do so, it'll handle placeholders for you.
> > > >
> > > > Do you really want to allow this to operate on non-C pointer types?
> > > 1. Standard clearly states that the type of the base expression must be a pointer. I don't think that we should perform implicit type casting here, like decay to pointers, etc.
> > > 2. It is just a simple form of checking that this is a pointer type. Since this expression is not allowed in other languages (and I filter it out at the parsing stage), I think it is ok to use the generic form of type checking.
> > Forgot to mention, that I thought about possible conversion here. But it may lead to some unpredictable results, like:
> > ```
> > int a[3];
> > ...([3][4][n])a...
> > ```
> > Better to do not allow this kind of operation, I think.
> > Standard clearly states that the type of the base expression must be a pointer. I don't think that we should perform implicit type casting here, like decay to pointers, etc.
>
> The wording of the C/C++ standard is that expressions of array type decay except in certain syntactic positions. Such expressions then *do* have pointer type for this purpose. If precedent is to forbid array references, so be it, but that's not consistent with the normal language behavior.
>
> > It is just a simple form of checking that this is a pointer type. Since this expression is not allowed in other languages (and I filter it out at the parsing stage), I think it is ok to use the generic form of type checking.
>
> If your intent is that this only applies only to C pointers, you should just check `isPointerType()` instead of `isAnyPointerType()`, which exists for the sole purpose of also including Objective-C pointers.
You can still have pointers to arrays without suppressing array decay.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:4771
+ }
+ if (!Dim->isValueDependent() && !Dim->isTypeDependent()) {
+ ExprResult Result =
----------------
ABataev wrote:
> rjmccall wrote:
> > You don't really care about value-dependence here, just type-dependence. You can check value-dependence before doing the constant-evaluation check below.
> Yes, just a double check to be realy-really sure :)
Type-dependence implies value-dependence anyway.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74144/new/
https://reviews.llvm.org/D74144
More information about the cfe-commits
mailing list