[Lldb-commits] [clang] [lldb] [HLSL] Implement intangible AST type (PR #97362)
@@ -8001,6 +8001,12 @@ NamedDecl *Sema::ActOnVariableDeclarator( } } + if (getLangOpts().HLSL) { +if (R->isHLSLSpecificType() && !NewVD->isImplicit()) { + Diag(D.getBeginLoc(), diag::err_hlsl_intangible_type_cannot_be_declared); llvm-beanz wrote: HLSL's sizzles types will need to be allowed in struct fields, otherwise the property can't propagate correctly up to the resource objects or structures that contain resources which HLSL allows. https://github.com/llvm/llvm-project/pull/97362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [HLSL] Implement intangible AST type (PR #97362)
@@ -8001,6 +8001,12 @@ NamedDecl *Sema::ActOnVariableDeclarator( } } + if (getLangOpts().HLSL) { +if (R->isHLSLSpecificType() && !NewVD->isImplicit()) { + Diag(D.getBeginLoc(), diag::err_hlsl_intangible_type_cannot_be_declared); llvm-beanz wrote: `NewVD->isImplicit()` means the declaration is implicit and doesn't have any source, that prevents us from being able to use these in `hlsl.h` or directly in tests. I think that restriction may be more than we need. https://github.com/llvm/llvm-project/pull/97362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [HLSL] Implement intangible AST type (PR #97362)
@@ -0,0 +1,33 @@ +//===-- HLSLIntangibleTypes.def - HLSL standard intangible types *- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// This file defines HLSL standard intangible types. These are implementation- +// defined types such as handle types that have no defined object +// representation or value representation and their size is unknown at compile +// time. +// +// The macro is: +// +//HLSL_INTANGIBLE_TYPE(Name, Id, SingletonId) +// +// where: +// +// - Name is the name of the builtin type. +// +// - BuiltinType::Id is the enumerator defining the type. +// +// - Context.SingletonId is the global singleton of this type. +// +// To include this file, define HLSL_INTANGIBLE_TYPE. +// The macro will be undefined after inclusion. +// +//===--===// + +HLSL_INTANGIBLE_TYPE(__builtin_hlsl_resource_t, HLSLResource, HLSLResourceTy) llvm-beanz wrote: I don't know if we have any cases where a type is prefixed by `__builtin`, usually types are `__clang` or `__` (e.g. `__arm`, `__amdgpu`). Maybe we should just do `__hlsl`? https://github.com/llvm/llvm-project/pull/97362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [HLSL] Implement intangible AST type (PR #97362)
@@ -12335,6 +12335,10 @@ def warn_hlsl_availability_unavailable : Warning, InGroup, DefaultError; +def err_hlsl_intangible_type_cannot_be_declared : Error< +"HLSL intangible type cannot be declared here">; +def err_hlsl_intangible_type_as_function_arg_or_return : Error< +"HLSL intangible type cannot be used as function %select{argument|return value}0">; llvm-beanz wrote: I don't think this is correct. We do allow resources as argument and return types. https://github.com/llvm/llvm-project/pull/97362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [HLSL] Implement intangible AST type (PR #97362)
@@ -3428,6 +3428,12 @@ void CXXNameMangler::mangleType(const BuiltinType *T) { Out << 'u' << type_name.size() << type_name; \ break; #include "clang/Basic/AMDGPUTypes.def" +#define HLSL_INTANGIBLE_TYPE(Name, Id, SingletonId) \ + case BuiltinType::Id: \ +type_name = #Name; \ +Out << type_name.size() << type_name; \ llvm-beanz wrote: ```suggestion Out << 'u' << type_name.size() << type_name; \ ``` The `u` is significant here because it denotes a vendor-specific builtin type (see: https://itanium-cxx-abi.github.io/cxx-abi/abi-mangling.html). https://github.com/llvm/llvm-project/pull/97362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [HLSL] Implement intangible AST type (PR #97362)
@@ -8001,6 +8001,12 @@ NamedDecl *Sema::ActOnVariableDeclarator( } } + if (getLangOpts().HLSL) { +if (R->isHLSLSpecificType() && !NewVD->isImplicit()) { + Diag(D.getBeginLoc(), diag::err_hlsl_intangible_type_cannot_be_declared); llvm-beanz wrote: Is the intent here to basically say that you can't create these types unless the declaration is implicit? I know we don't expose handle types in DXC, but do we really need to force these to be invalid in source? https://github.com/llvm/llvm-project/pull/97362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [HLSL] Implement intangible AST type (PR #97362)
https://github.com/llvm-beanz commented: Few comments, but mostly I really like this direction! https://github.com/llvm/llvm-project/pull/97362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [HLSL] Implement intangible AST type (PR #97362)
https://github.com/llvm-beanz edited https://github.com/llvm/llvm-project/pull/97362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)
llvm-beanz wrote: > I don't know if the pre-commit testing guarantees that. Configuration > settings will permit the files to be checked out in either Unix (`\n`) or > Windows (`\r\n`) line-endings. Today on Windows you basically have to check out LLVM as unix line endings. There are a bunch of tests that fail if you don't. Hopefully this is a step toward fixing that (which is why I'm in favor of this). > If the builders are all configured to run in Unix line endings we lose that > coverage. I suspect the Windows bots are basically all configured that way today. Our instructions tell people they need to disable `autocrlf` (see: https://llvm.org/docs/GettingStarted.html#getting-the-source-code-and-building-llvm), and not disabling it causes problems for users still (see this message from today: https://discord.com/channels/636084430946959380/745925509971705877/1235913415902629958). >While I understand that this change only changes how the LLVM sources are >stored, the issue is that the LLVM sources include the _tests_ directory. >These need to be tested in various manners to ensure that we test how we >handle the different inputs (in clang). One option might be to exclude the >tests directory from the line ending conversion if we cannot verify that we >have tests in both formats. I see two issues with this: 1) because the repo doesn't work with autocrlf today, we actually can't rely on testing on different platforms to provide this coverage. 2) We shouldn't be relying on git settings to get this coverage anyways. We should have tests that require specific line endings, and we should use gitattributes to ensure that we run those tests regardless of the user's git settings. > While, yes, it is possible to get the wrong behaviour currently, if the user > configures git explicitly for a line-ending, I would expect them to be aware > of that. The use of `.gitattributes` means that their defaults are not > necessarily honoured and thus this can catch them by surprise. I think today not only is it possible to get the wrong behavior, if you're developing on Windows Git's default behavior causes the wrong behavior for LLVM. Which often means that contributors (especially new contributors) have no idea that they're doing something wrong. I don't know that this PR is perfect, but IMO it is a huge step in the right direction to allow Windows contributors to build, test and contribute to LLVM using the default configurations of the OS and tools. https://github.com/llvm/llvm-project/pull/86318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)
https://github.com/llvm-beanz approved this pull request. I'm happy with this approach. We can adjust and iteratively address issues if any appear. https://github.com/llvm/llvm-project/pull/86318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)
llvm-beanz wrote: > I'm a bit confused. Are you saying that as I've expressed it, `autocrlf=true` > won't work correctly? I _think_ you're saying you're in favour of this patch > _in principal_, but I need to fix the mixed line endings case? Sorry for being unclear. I meant to express that I'm so strongly in favor of what you're accomplishing here that I don't want any of my feedback to stand in the way. https://github.com/llvm/llvm-project/pull/86318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)
llvm-beanz wrote: > This ^ seems a bit problematic to me, though (where the contents of the repo > isn't representative of the bits we want - but perhaps it's schrodinger's > line endings & it doesn't matter if it's always checked out as crlf - though > if we one day converted to another source control system, would that be a > problem? are there some things that examine the underlying/"real" repo > contents?) - what would be the cost to using `eol=input` as you've done for > the mixed line ending case? below \= The argument I would have in favor of an explicit value rather than `eol=input` is that it at least gives us a source of truth in the `.gitattributes` as to what type of line ending the file is supposed to have. I don't have a strong feeling one way or another. I just want `autocrlf=true` to work reliably on all platforms. https://github.com/llvm/llvm-project/pull/86318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)
llvm-beanz wrote: > I am not saying this isn't a problem at all, but how often has anyone done a > one line change and caused a 50k diff, and submitted it without noticing? Way more often than you would hope. I don't have numbers but if you search through the git history for `crlf` or `dos2unix` those phrases hit on a lot of commits (more than just my abolishcrlf.org email address). https://github.com/llvm/llvm-project/pull/86318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)
llvm-beanz wrote: Philosophically I agree with this change, but I think some subprojects may need to adopt specific policies about how they handle files that are expected to be specific line endings. Specifically, Clang needs testing that verifies correct behavior on both CRLF and LF files in some cases, as does LLVM's split-file. I did a pass a few years ago fixing a bunch of tests that had hard-specified file offsets in them that broke if you had CRLF line endings, and fixing how the correct line ending type was detected (some of the changes are 13fa17db3a720d149bcd0783856347a4f09cf634, 9d3437fbf3419502351d41ff9e28f06b0c3f06e8, ea836c880abcc74d3983fc35de407d647f0a002d, 1c1b0027e86fab2b0877488abc1625a457ca70b3). Should we come up with an expected filename format that we can use with gitattributes and rename the existing files rather than just listing each file that has special needs by itself? https://github.com/llvm/llvm-project/pull/86318 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits