[HLSL] Pointers are unsupported in HLSL

[beanz]

Summary

HLSL does not support pointers or references. This change generates errors in sema for generating pointer, and reference types as well as common operators (address-of, dereference, arrow), which are used with pointers and are unsupported in HLSL.

[aaron.ballman]

I think you're going to need semantic restrictions as well as parsing restrictions because of type deduction. Even if you don't support templates (yet), there's still auto, decltype, __typeof__, and __auto_type (I probably missed some, lol) to worry about.

Also... can HLSL be combined with Objective-C mode (do we have to worry about their pointers as well)?

clang/test/ParserHLSL/parse_pointer.hlsl

10 ↗

(On Diff #420670)

12 ↗

(On Diff #420670)

I am proud of you for this test case, because I was going to ask for it. :-D

23 ↗

(On Diff #420670)

Here are a few more fun test cases (which may require semantic tests instead of parsing tests):

void func();

void devilish_language(auto look_ma_no_pointers); // templates get you the same effects as well

void test() {
  int x;
  devilish_language(&x); // I presume we want this to be diagnosed
  devilish_language(func); // I think you wanted this to not be diagnosed?
  devilish_language("oh no, not array decay!"); // Same here as function decay?

  auto but_but_but = "please, stop Aaron!" + 0; // Uhhh... is the addition okay but the declaration bad?

  int array[2];
  *(&array[0] + 1) = 12; // Maaaayybbbe this is fine?
}

(Thing for you to double-check: do any targets predefine macros that expand to a pointer? I checked InitPreprocessor.cpp and things looked fine there, but I didn't check targets.)

clang/test/ParserHLSL/parse_reference.hlsl

4 ↗

(On Diff #420670)

18 ↗

(On Diff #420670)

Similar extra tests involving type deduction as the pointer tests.

[beanz]

In D123167#3434023, @aaron.ballman wrote:

I think you're going to need semantic restrictions as well as parsing restrictions because of type deduction. Even if you don't support templates (yet), there's still auto, decltype, __typeof__, and __auto_type (I probably missed some, lol) to worry about.

Thankfully HLSL is like C++98-based... I don't think we support any of those cases.

Also... can HLSL be combined with Objective-C mode (do we have to worry about their pointers as well)?

Oh god... Now you're giving me nightmares. I feel like implementing objc_msgsend on a GPU might be a bridge (or ten) too far...

clang/test/ParserHLSL/parse_pointer.hlsl

23 ↗

(On Diff #420670)

I am in awe of your gift for finding ways to trip things up :)

I think most of these we can't hit because of auto. Eyeballing that lat case we _might_ handle it. I'll double check.

[aaron.ballman]

In D123167#3434289, @beanz wrote:

In D123167#3434023, @aaron.ballman wrote:

I think you're going to need semantic restrictions as well as parsing restrictions because of type deduction. Even if you don't support templates (yet), there's still auto, decltype, __typeof__, and __auto_type (I probably missed some, lol) to worry about.

Thankfully HLSL is like C++98-based... I don't think we support any of those cases.

Oh, were compilers that easy... https://godbolt.org/z/EMasKrdh3

Also... can HLSL be combined with Objective-C mode (do we have to worry about their pointers as well)?

Oh god... Now you're giving me nightmares. I feel like implementing objc_msgsend on a GPU might be a bridge (or ten) too far...

Heh, good. Can you make sure we reject that situation loudly in the frontend (in another patch, on another day, no rush)?

clang/test/ParserHLSL/parse_pointer.hlsl

23 ↗

(On Diff #420670)

I am broken for writing decent code after spending so many years in Core thinking about edge cases. :-D

Btw, here's another one to try:

int i = *(int *)0; // Check that pointer types used in expressions are caught, same for reference

[tschuett]

Do you need a SemaHLSL.cpp file similar to SemaSYCL.cpp?

[beanz]

In D123167#3434346, @tschuett wrote:

Do you need a SemaHLSL.cpp file similar to SemaSYCL.cpp?

Yes absolutely. I haven't yet added any Sema-specific validations yet that are complex enough to break out, but we will 100% get there soon.

[tschuett]

In D123167#3434349, @beanz wrote:

In D123167#3434346, @tschuett wrote:

Do you need a SemaHLSL.cpp file similar to SemaSYCL.cpp?

Yes absolutely. I haven't yet added any Sema-specific validations yet that are complex enough to break out, but we will 100% get there soon.

Is the downstream compiler also relying on the parser?

[beanz]

In D123167#3434350, @tschuett wrote:

Is the downstream compiler also relying on the parser?

Not sure what you mean by downstream compiler. The current official HLSL compiler is a fork of clang-3.7. We don't intend to merge code in either direction. The fork does have modifications throughout clang including the parser, sema, and codegen layers.

[tschuett]

Relying on the parser to find semantic misbehavior sounds strange ...

[beanz]

In D123167#3434364, @tschuett wrote:

Relying on the parser to find semantic misbehavior sounds strange ...

I don't disagree. An alternative point: HLSL has no _syntax_ for pointers.

[tschuett]

In D123167#3434372, @beanz wrote:

In D123167#3434364, @tschuett wrote:

Relying on the parser to find semantic misbehavior sounds strange ...

I don't disagree. An alternative point: HLSL has no _syntax_ for pointers.

I got your point.

[aaron.ballman]

In D123167#3434364, @tschuett wrote:

Relying on the parser to find semantic misbehavior sounds strange ...

I had a meeting with @beanz today because I was also pretty surprised. Chris, it might make sense to type up a bit of what we talked about as an FAQ and maybe stick it in Clang's docs directory; WDYT?

[beanz]

In D123167#3434375, @aaron.ballman wrote:

I had a meeting with @beanz today because I was also pretty surprised. Chris, it might make sense to type up a bit of what we talked about as an FAQ and maybe stick it in Clang's docs directory; WDYT?

Totally agree. I had actually started working on something to help bring new contributors into the effort. I'll try to get something up this week.

[aaron.ballman]

In D123167#3434383, @beanz wrote:

In D123167#3434375, @aaron.ballman wrote:

I had a meeting with @beanz today because I was also pretty surprised. Chris, it might make sense to type up a bit of what we talked about as an FAQ and maybe stick it in Clang's docs directory; WDYT?

Totally agree. I had actually started working on something to help bring new contributors into the effort. I'll try to get something up this week.

Huttah! Thank you. :-)

[beanz]

Updating based on PR feedback.

I've moved the error reporting into Sema, and added additional errors for unsupported operators.

I've also expanded the test coverage with some of @aaron.ballman's cursed code, which improves coverage substantailly.

[aaron.ballman]

clang/include/clang/Basic/DiagnosticSemaKinds.td

11593–11598

Should we be adding .* and ->* to the list of unsupported operators? (Hmmm, or can we never get to those operators because we can't form the base expression anyway?)

clang/lib/Sema/SemaExpr.cpp

15372–15377

How should this interplay with overloaded operators on user-defined types? Is that case allowed so long as it doesn't form a pointer or a reference? (We should add test coverage for that.)

clang/lib/Sema/SemaExprMember.cpp

1739–1740

Same question here for overloaded operators.

clang/test/SemaHLSL/prohibit_reference.hlsl

5

void chirp(int &&); // error

[beanz]

One comment inline. Update coming in a few minutes.

clang/lib/Sema/SemaExpr.cpp

15372–15377

So, the "correct" end goal is that we don't support overloading those operators. We have a few limitations on which operators we allow overloading for, generally any operators that in normal use return references or pointers, we don't allow you to overload.

That said, I wrote some test code to see where this falls over with my current change:

struct Fish {
  struct Fins {
    int Left;
    int Right;
  };
  int X;
  int operator *() {
    return X;
  }

  Fins operator ->() {
    return Fins();
  }
};

int gone_fishing() {
  Fish F;
  int Result = *F;       // works... and is consistent with DXC
  Result += F->Left; // error: member reference type 'Fish::Fins' is not a pointer
  return Result;
}

In the existing compiler we produce an error on definition of operators that aren't supported. I'd like to handle improving the diagnostics for those operators that way in a later patch if that is okay.

The .* and .-> operators I couldn't get errors to actually trigger because I couldn't think of a way to write them that wasn't dependent on getting a member pointer, which errors and causes the remaining expression to not be checked.

[beanz]

Updates to test cases to increase coverage.

[aaron.ballman]

LGTM aside from the formatting nit.

clang/lib/Sema/SemaExpr.cpp

15372–15377

In the existing compiler we produce an error on definition of operators that aren't supported. I'd like to handle improving the diagnostics for those operators that way in a later patch if that is okay.

That's fine by me.

The .* and .-> operators I couldn't get errors to actually trigger because I couldn't think of a way to write them that wasn't dependent on getting a member pointer, which errors and causes the remaining expression to not be checked.

Perfect, I couldn't find a way either, we can punt on that until we find a concrete issue.

clang/include/clang/Basic/DiagnosticSemaKinds.td

 
 // HLSL Diagnostics
 def err_hlsl_attr_unsupported_in_stage : Error<"attribute %0 is unsupported in %select{Pixel|Vertex|Geometry|Hull|Domain|Compute|Library|RayGeneration|Intersection|AnyHit|ClosestHit|Miss|Callable|Mesh|Amplification|Invalid}1 shaders, requires %2">;
 
 def err_hlsl_numthreads_argument_oor : Error<"argument '%select{X|Y|Z}0' to numthreads attribute cannot exceed %1">;
 def err_hlsl_numthreads_invalid : Error<"total number of threads cannot exceed %0">;
 def err_hlsl_attribute_param_mismatch : Error<"%0 attribute parameters do not match the previous declaration">;
 
+def err_hlsl_pointers_unsupported : Error<
+  "%select{pointers|references}0 are unsupported in HLSL">;
+
+def err_hlsl_operator_unsupported : Error<
+  "the '%select{&|*|->}0' operator is unsupported in HLSL">;
+
 // Layout randomization warning.
 def err_cast_from_randomized_struct : Error<
   "casting from randomized structure pointer type %0 to %1">;
 } // end of sema component.

clang/lib/Sema/SemaExpr.cpp

// only with a builtin functions and therefore should be disallowed here.
         (Ty->isImageType() || Ty->isSamplerT() || Ty->isPipeType()
         || Ty->isBlockPointerType())) {
       return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
                        << InputExpr->getType()
                        << Input.get()->getSourceRange());
     }
   }
 
+  if (getLangOpts().HLSL) {
+    if (Opc == UO_AddrOf)
+      return ExprError(Diag(OpLoc, diag::err_hlsl_operator_unsupported) << 0);
+    if (Opc == UO_Deref)
+      return ExprError(Diag(OpLoc, diag::err_hlsl_operator_unsupported) << 1);
+  }
+
   switch (Opc) {
   case UO_PreInc:
   case UO_PreDec:
   case UO_PostInc:
   case UO_PostDec:
     resultType = CheckIncrementDecrementOperand(*this, Input.get(), VK, OK,
                                                 OpLoc,
                                                 Opc == UO_PreInc ||

clang/lib/Sema/SemaExprMember.cpp

ExprResult Sema::ActOnMemberAccessExpr(Scope *S, Expr *Base,
   DeclarationNameInfo NameInfo;
   const TemplateArgumentListInfo *TemplateArgs;
   DecomposeUnqualifiedId(Id, TemplateArgsBuffer,
                          NameInfo, TemplateArgs);
 
   DeclarationName Name = NameInfo.getName();
   bool IsArrow = (OpKind == tok::arrow);
 
+  if (getLangOpts().HLSL && IsArrow)
+    return ExprError(Diag(OpLoc, diag::err_hlsl_operator_unsupported) << 2);
+
   NamedDecl *FirstQualifierInScope
     = (!SS.isSet() ? nullptr : FindFirstQualifierInScope(S, SS.getScopeRep()));
 
   // This is a postfix expression, so get rid of ParenListExprs.
   ExprResult Result = MaybeConvertParenListExprToParenExpr(S, Base);
   if (Result.isInvalid()) return ExprError();
   Base = Result.get();
 

clang/lib/Sema/SemaType.cpp

QualType Sema::BuildPointerType(QualType T,
 
   if (T->isFunctionType() && getLangOpts().OpenCL &&
       !getOpenCLOptions().isAvailableOption("__cl_clang_function_pointers",
                                             getLangOpts())) {
     Diag(Loc, diag::err_opencl_function_pointer) << /*pointer*/ 0;
     return QualType();
   }
 
+  if (getLangOpts().HLSL) {
+    Diag(Loc, diag::err_hlsl_pointers_unsupported) << 0;
+    return QualType();
+  }
+
   if (checkQualifiedFunction(*this, T, Loc, QFK_Pointer))
     return QualType();
 
   assert(!T->isObjCObjectType() && "Should build ObjCObjectPointerType");
 
   // In ARC, it is forbidden to build pointers to unqualified pointers.
   if (getLangOpts().ObjCAutoRefCount)
     T = inferARCLifetimeForPointee(*this, T, Loc, /*reference*/ false);
QualType Sema::BuildReferenceType(QualType T, bool SpelledAsLValue,
   // C++ [dcl.ref]p1:
   //   A declarator that specifies the type "reference to cv void"
   //   is ill-formed.
   if (T->isVoidType()) {
     Diag(Loc, diag::err_reference_to_void);
     return QualType();
   }
 
+  if (getLangOpts().HLSL) {
+    Diag(Loc, diag::err_hlsl_pointers_unsupported) << 1;
+    return QualType();
+  }
+
   if (checkQualifiedFunction(*this, T, Loc, QFK_Reference))
     return QualType();
 
   if (T->isFunctionType() && getLangOpts().OpenCL &&
       !getOpenCLOptions().isAvailableOption("__cl_clang_function_pointers",
                                             getLangOpts())) {
     Diag(Loc, diag::err_opencl_function_pointer) << /*reference*/ 1;
     return QualType();
QualType Sema::BuildMemberPointerType(QualType T, QualType Class,
 
   if (T->isFunctionType() && getLangOpts().OpenCL &&
       !getOpenCLOptions().isAvailableOption("__cl_clang_function_pointers",
                                             getLangOpts())) {
     Diag(Loc, diag::err_opencl_function_pointer) << /*pointer*/ 0;
     return QualType();
   }
 
+  if (getLangOpts().HLSL) {
+    Diag(Loc, diag::err_hlsl_pointers_unsupported) << 0;
+    return QualType();
+  }
+
   // Adjust the default free function calling convention to the default method
   // calling convention.
   bool IsCtorOrDtor =
       (Entity.getNameKind() == DeclarationName::CXXConstructorName) ||
       (Entity.getNameKind() == DeclarationName::CXXDestructorName);
   if (T->isFunctionType())
     adjustMemberFunctionCC(T, /*IsStatic=*/false, IsCtorOrDtor, Loc);
 

clang/test/SemaHLSL/prohibit_pointer.hlsl

+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - -fsyntax-only %s -verify
+
+// expected-error@+1 {{pointers are unsupported in HLSL}}
+typedef int (*fn_int)(int);
+void* bark(int); // expected-error {{pointers are unsupported in HLSL}}
+void meow(int*); // expected-error {{pointers are unsupported in HLSL}}
+
+struct Foo {
+  int X;
+  int Y;
+} *bad; // expected-error {{pointers are unsupported in HLSL}}
+
+// expected-error@+1 {{pointers are unsupported in HLSL}}
+void woof(int Foo::*Member);
+
+int entry() {
+  int X;
+  Foo F;
+  
+  // expected-error@+2 {{the '&' operator is unsupported in HLSL}}
+  // expected-error@+1 {{pointers are unsupported in HLSL}}
+  int Foo::*Member = &F.X;
+
+  
+  // expected-error@+1 {{the '&' operator is unsupported in HLSL}}
+  int *Y = &X; // expected-error {{pointers are unsupported in HLSL}}
+  
+  // expected-error@+2 {{the '->' operator is unsupported in HLSL}}
+  // expected-error@+1 {{the '&' operator is unsupported in HLSL}}
+  int W = (&F)->X;
+
+  int Array[2];
+  // expected-error@+1 {{the '&' operator is unsupported in HLSL}}
+  *(&Array[0] + 1) = 12;
+  // expected-error@+1 {{the '*' operator is unsupported in HLSL}}
+  *Array = 12;
+}
+
+int roar(Foo *F) { // expected-error {{pointers are unsupported in HLSL}}
+  // expected-error@+1 {{the '->' operator is unsupported in HLSL}}
+  return F->X;
+}
+
+template <typename T>
+void devilish_language(T look_ma_no_pointers);
+
+void make_me_cry() {
+  int X;
+  // expected-error@+1 {{the '&' operator is unsupported in HLSL}}
+  devilish_language(&X);
+
+  // not-expected-error@+1 {{pointers are unsupported in HLSL}}
+  devilish_language(roar); // allow function pointer decay
+
+  // not-expected-error@+1 {{pointers are unsupported in HLSL}}
+  devilish_language("roar"); // allow array pointer decay
+}
+
+struct Fish {
+  struct Fins {
+    int Left;
+    int Right;
+  };
+  int X;
+  int operator *() {
+    return X;
+  }
+
+  // expected-note@+1 {{'->' applied to return value of the operator->() declared here}}
+  Fins operator ->() {
+    return Fins();
+  }
+};
+
+int gone_fishing() {
+  Fish F;
+  int Result = *F; // user-defined dereference operators work
+  // expected-error@+1 {{member reference type 'Fish::Fins' is not a pointer}}
+  Result += F->Left;
+  return Result;
+}

clang/test/SemaHLSL/prohibit_reference.hlsl

+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - -fsyntax-only %s -verify
+
+int& bark(int); // expected-error {{references are unsupported in HLSL}}
+void meow(int&); // expected-error {{references are unsupported in HLSL}}
+void chirp(int &&); // expected-error {{references are unsupported in HLSL}}
+// expected-warning@-1 {{rvalue references are a C++11 extension}}
+
+struct Foo {
+  int X;
+  int Y;
+};
+
+int entry() {
+  int X;
+  int &Y = X; // expected-error {{references are unsupported in HLSL}}
+}
+
+int roar(Foo &F) { // expected-error {{references are unsupported in HLSL}}
+  return F.X;
+}