make interface reserved keyword as same as union

[mateeeeeee]

remove existing interface related tests, add new reserved keywords tests

Fixes #6117

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[mateeeeeee]

@mateeeeeee please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

• (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.

@microsoft-github-policy-service agree

• (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.

@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

[llvm-beanz]

One of the two existing tests you modified runs against HLSL 2015 and the other against 2018. The 2015 one shouldn't be changed because that's the expected behavior which we need to preserve.

The 2018 test changes look good.

We should also apply a patch like this:

diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp
index d09669e5512..f4d4b1e389e 100644
--- a/lib/DxcSupport/HLSLOptions.cpp
+++ b/lib/DxcSupport/HLSLOptions.cpp
@@ -467,6 +467,8 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
   // through an argument. The value should default to 'main', but we let the
   // caller apply this policy.
 
+  opts.VerifyDiagnostics = Args.hasFlag(OPT_verify, OPT_INVALID, false);
+
   if (opts.TargetProfile.empty()) {
     opts.TargetProfile = Args.getLastArgValue(OPT_target_profile);
   }
@@ -508,7 +510,8 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
   }
 
   if (opts.HLSLVersion == hlsl::LangStd::v2015 &&
-      !(flagsToInclude & HlslFlags::ISenseOption)) {
+      !(flagsToInclude & HlslFlags::ISenseOption != 0 ||
+        opts.VerifyDiagnostics)) {
     errors << "HLSL Version 2015 is only supported for language services";
     return 1;
   }
@@ -821,7 +824,6 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
       Args.hasFlag(OPT_fnew_inlining_behavior, OPT_INVALID, false);
   opts.TimeReport = Args.hasFlag(OPT_ftime_report, OPT_INVALID, false);
   opts.TimeTrace = Args.hasFlag(OPT_ftime_trace, OPT_INVALID, false) ? "-" : "";
-  opts.VerifyDiagnostics = Args.hasFlag(OPT_verify, OPT_INVALID, false);
   if (Args.hasArg(OPT_ftime_trace_EQ))
     opts.TimeTrace = Args.getLastArgValue(OPT_ftime_trace_EQ);
   if (Arg *A = Args.getLastArg(OPT_ftime_trace_granularity_EQ)) {

This will allow the -verify flag to work on the command line for -HV 2015. With that you can also add this test case under SemaHLSL to verify the HV 2015 behavior:

// RUN: %dxc -Tlib_6_3  -verify -HV 2015 %s

interface my_interface { };

class my_class { };
class my_class_3 : my_interface { };
struct my_struct { };
struct my_struct_4 : my_interface { };
struct my_struct_5 : my_class, my_interface { };

struct my_struct_6 : my_class, my_interface, my_struct { }; // expected-error {{multiple concrete base types specified}}

interface my_interface_2 : my_interface { }; // expected-error {{interfaces cannot inherit from other types}}

[llvm-beanz]

We do actually need to support this keyword in HLSL language version 2015. That's what some of the tests you had to modify below verify.

We could instead add:

if (getLangOpts().HLSLVersion != hlsl::LangStd::v2015 && Tok.is(tok::kw_interface))
  goto HLSLReservedKeyword;

This would allow the 2015 rewriters to continue working as expected.

[mateeeeeee]

I've made changes you suggested, thanks. There was one assert I removed that seems unecessary at this point. I didn't understand if I need to revert any rewriter files.

[llvm-beanz]

Ideally you should be able to revert the rewriter test cases because the rewriter should still properly handle the HLSL 2015 interface keyword.

[mateeeeeee]

Ideally you should be able to revert the rewriter test cases because the rewriter should still properly handle the HLSL 2015 interface keyword.

Reverting the rewriter files (cpp-errors_noerr.hlsl, cpp-errors_gold.hlsl) causes "error: 'interface' is a reserved keyword in HLSL" in cpp-errors_noerr.hlsl. Are these rewriters really 2015?

[llvm-beanz]

Oh, you may need to add -HV 2015 to the RUN line. The rewriters need to work with 2015 inputs, but they do some odd things.

[mateeeeeee]

Strange but even with adding -HV 2015 it still shows the same error. I used the same RUN line as cpp-errors-hv2015.hlsl which passes just fine:
// RUN: %clang_cc1 -HV 2015 -fsyntax-only -Wno-unused-value -ffreestanding -verify %s

[llvm-beanz]

Bleh. That's because our testing infrastructure is oddly inconsistent and has multiple different ways of parsing those lines.

Looks like the rewriter tests hard codes a language version:
https://github.com/microsoft/DirectXShaderCompiler/blob/main/tools/clang/unittests/HLSL/RewriterTest.cpp#L221

[llvm-beanz]

Oh. Even cooler... The rewriter tests don't read the run line at all, they just use hard coded arguments.