[PATCH] D123278: [Clang] [Docs] Add HLSLSupport page

Thu Apr 7 06:59:06 PDT 2022

aaron.ballman added a comment.

Thanks for getting a start on this documentation, I appreciate it!

================
Comment at: clang/docs/HLSLSupport.rst:11
+
+HLSL Support is a new initiative underway in the Clang codebase. This document
+describes the high level goals of the project, the guiding principals, as well
----------------
I can't wait to read "is a new initiative underway" four years from now. :-D (We may want to prevent that by making it a less temporal statement.)

================
Comment at: clang/docs/HLSLSupport.rst:12
+HLSL Support is a new initiative underway in the Clang codebase. This document
+describes the high level goals of the project, the guiding principals, as well
+as some of the idiosyncrasies of the HLSL language and how we intend to support
----------------

================
Comment at: clang/docs/HLSLSupport.rst:19-23
+The long term goal of this project is to enable clang to function as a
+replacement for the `DirectXShaderCompiler (DXC)
+<https://github.com/microsoft/DirectXShaderCompiler/>`_. in all its supported
+use cases. Accomplishing that goal will require a high level of source
+compatibility.
----------------

================
Comment at: clang/docs/HLSLSupport.rst:33
+
+Guiding Principals
+==================
----------------

================
Comment at: clang/docs/HLSLSupport.rst:46-48
+HLSL support in clang is expressed as C++ minus unsupported C and C++ features.
+This is different from how other clang languages are implemented. Most languages
+in Clang are additive on top of C.
----------------

================
Comment at: clang/docs/HLSLSupport.rst:59
+checks on language options where the code is simple and isolated, and prefer
+HLSL-specific implementation files for any code of reasonable complexity.
+
----------------
I think it would be good to also mention what happens when HLSL is in conflict with either C or C++, at a high level.

================
Comment at: clang/docs/HLSLSupport.rst:73
+Following the examples of other parser extensions HLSL will add a ParseHLSL.cpp
+file to contain the implementations of HLSL-specific extensions to the clang
+parser.
----------------
Probably also worth pointing out that most HLSL parsing is expected to be the usual C and C++ code paths, or is that not your expectation?

================
Comment at: clang/docs/HLSLSupport.rst:121-122
+operators (unary ``*``, and ``->``), as well as the address of operator (unary
+&). While HLSL disallows pointers and references in the syntax, HLSL does use
+reference types in the AST.
+
----------------
Presumably it also uses pointer types in the AST for things like array and function decay?

================
Comment at: clang/docs/HLSLSupport.rst:143-145
+In HLSL 2018 and earlier, HLSL supported logical operators (and the ternary
+operator) on vector types. This behavior required that operators not short
+circuit.
----------------
It's not clear whether the behavior will vary for all types or just vector types. Also, does this apply in preprocessor conditionals the same as runtime expressions?

================
Comment at: clang/docs/HLSLSupport.rst:150-152
+HLSL has a ``precise`` qualifier that behaves unlike anything else in the C
+language. The support for this qualifier in DXC is buggy, so our bar for
+compatibility is low.
----------------
Is it worth supporting at all (I know you want source compatibility...)? Type system changes are generally expensive and invasive, largely because changing the type system in C++ is complicated. For example, does this qualifier impact overload sets or template specializations? Does it get name mangled? That sort of thing.

================
Comment at: clang/docs/HLSLSupport.rst:166
+HLSL uses templates to define builtin types and methods, but disallowed
+user-defined templates until HLSL 2021. HLSL also allows omiting empty template
+parameter lists when all template parameters are defaulted. This is an ambiguous
----------------

================
Comment at: clang/docs/HLSLSupport.rst:168
+parameter lists when all template parameters are defaulted. This is an ambiguous
+syntax in C++, however clang detects the case and issues a diagnostic, so
+supporting it is minimally invasive.
----------------

================
Comment at: clang/docs/HLSLSupport.rst:175
+HLSL uses the OpenCL vector extensions, and also provides C++-style constructors
+for vectors that are not supported by clang.
+
----------------

================
Comment at: clang/docs/HLSLSupport.rst:185
+* ``union`` types `(in progress for HLSL 202x) <https://github.com/microsoft/DirectXShaderCompiler/pull/4132>`_
+* Most features C11 and later
+
----------------
>From C99:
VLAs?
_Complex/_Imaginary?

>From C11:
Threads/Atomics?
Language features like `_Generic` (or `[[]]` attributes in C2x)?

Also, it's not clear whether you expect these unsupported features to be diagnosed or supported as extensions in HLSL, etc.

================
Comment at: clang/docs/HLSLSupport.rst:189
+
+* RTTI
+* Exceptions
----------------
No `dynamic_cast` or `typeid`?

================
Comment at: clang/docs/HLSLSupport.rst:193
+* Access specifiers
+* Anonymous namespaces
+* ``new`` & ``delete`` operators
----------------
Inline namespaces?

================
Comment at: clang/docs/HLSLSupport.rst:194
+* Anonymous namespaces
+* ``new`` & ``delete`` operators
+* Constructors & destructors
----------------
I presume...

How about placement new expressions?

================
Comment at: clang/docs/HLSLSupport.rst:197
+* Any use of the ``virtual`` keyword
+* Most features C++11 and later
----------------
Same question here as above with C on whether we expect to support those as extensions or diagnose them as invalid.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123278