[HLSL] Adjust access specifier behavior

[beanz]

Summary

HLSL doesn't support access specifiers. This change has two components:

1. Make default access for classes public
2. Diagnose the use of access specifiers as a clang HLSL extension

As long as the default behavior for access specifiers matches HLSL,
allowing them to be used doesn't cause sourece incompatability with
valid code. As such enabling them as a clang extension seems like a
reasonable approach.

Fixes #55124

[MaskRay]

clang/test/ParserHLSL/access_specifiers.hlsl

9

May need tests for protected: and public:, and possibly for struct.

[aaron.ballman]

clang/include/clang/Basic/DiagnosticParseKinds.td

1603

This also needs to go into a warning group.

clang/lib/Parse/ParseDeclCXX.cpp

3517–3519

Gross. 🤮

clang/lib/Sema/SemaDeclCXX.cpp

2504–2507

🤮

clang/test/ParserHLSL/access_specifiers.hlsl

5–6

(It's generally easier to maintain tests when the expected check lines are directly on the impacted line rather than using offsets to it. Same applies through the rest of the test. NB: we don't care about 80 col limits in tests.)

[beanz]

Updates based on PR feedback.

[aaron.ballman]

LGTM aside from whitespace changes.

clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp

1 ↗

(On Diff #425678)

It looks like these are unintentional whitespace changes? (Same below)

[beanz]

clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp

1 ↗

(On Diff #425678)

Ha! I didn't notice this came along for the ride. This is from me trying to get the git file attributes setup correctly. This file should be crlf in the index (hence the title and comment) but is lf.

clang/include/clang/Basic/DiagnosticGroups.td

def PedanticMacros : DiagGroup<"pedantic-macros",
                     [DeprecatedPragma,
                      MacroRedefined,
                      BuiltinMacroRedefined,
                      RestrictExpansionMacro,
                      FinalMacro]>;
 
 def BranchProtection : DiagGroup<"branch-protection">;
 
+// HLSL diagnostic groups
+// Warnings for HLSL Clang extensions
+def HLSLExtension : DiagGroup<"hlsl-extensions">;

clang/include/clang/Basic/DiagnosticParseKinds.td

 
 def note_max_tokens_total_override : Note<"total token limit set here">;
 
 // HLSL Parser Diagnostics
 
 def err_expected_semantic_identifier : Error<
   "expected HLSL Semantic identifier">;
 def err_unknown_hlsl_semantic : Error<"unknown HLSL semantic %0">;
+def ext_hlsl_access_specifiers : ExtWarn<
+  "access specifiers are a clang HLSL extension">,
+  InGroup<HLSLExtension>;
 
 } // end of Parser diagnostics

clang/lib/Parse/ParseDeclCXX.cpp

BaseResult Parser::ParseBaseSpecifier(Decl *ClassDecl) {
   // Parse the 'virtual' keyword.
   if (TryConsumeToken(tok::kw_virtual))
     IsVirtual = true;
 
   CheckMisplacedCXX11Attribute(Attributes, StartLoc);
 
   // Parse an (optional) access specifier.
   AccessSpecifier Access = getAccessSpecifierIfPresent();
-  if (Access != AS_none)
+  if (Access != AS_none) {
     ConsumeToken();
+    if (getLangOpts().HLSL)
+      Diag(Tok.getLocation(), diag::ext_hlsl_access_specifiers);
+  }
 
   CheckMisplacedCXX11Attribute(Attributes, StartLoc);
 
   // Parse the 'virtual' keyword (again!), in case it came after the
   // access specifier.
   if (Tok.is(tok::kw_virtual))  {
     SourceLocation VirtualLoc = ConsumeToken();
     if (IsVirtual) {
Parser::DeclGroupPtrTy Parser::ParseCXXClassMemberDeclarationWithPragmas(
   case tok::kw_private:
     // FIXME: We don't accept GNU attributes on access specifiers in OpenCL mode
     // yet.
     if (getLangOpts().OpenCL && !NextToken().is(tok::colon))
       return ParseCXXClassMemberDeclaration(AS, AccessAttrs);
     LLVM_FALLTHROUGH;
   case tok::kw_public:
   case tok::kw_protected: {
+    if (getLangOpts().HLSL)
+      Diag(Tok.getLocation(), diag::ext_hlsl_access_specifiers);
     AccessSpecifier NewAS = getAccessSpecifierIfPresent();
     assert(NewAS != AS_none);
     // Current token is a C++ access specifier.
     AS = NewAS;
     SourceLocation ASLoc = Tok.getLocation();
     unsigned TokLength = Tok.getLength();
     ConsumeToken();
     AccessAttrs.clear();
void Parser::ParseCXXMemberSpecification(SourceLocation RecordLoc,
   if (TagDecl)
     Actions.ActOnStartCXXMemberDeclarations(getCurScope(), TagDecl, FinalLoc,
                                             IsFinalSpelledSealed, IsAbstract,
                                             T.getOpenLocation());
 
   // C++ 11p3: Members of a class defined with the keyword class are private
   // by default. Members of a class defined with the keywords struct or union
   // are public by default.
+  // HLSL: In HLSL members of a class are public by default.
   AccessSpecifier CurAS;
-  if (TagType == DeclSpec::TST_class)
+  if (TagType == DeclSpec::TST_class && !getLangOpts().HLSL)
     CurAS = AS_private;
   else
     CurAS = AS_public;
   ParsedAttributes AccessAttrs(AttrFactory);
 
   if (TagDecl) {
     // While we still have something to read, read the member-declarations.
     while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&

clang/lib/Sema/SemaDeclCXX.cpp

 /// \returns a new CXXBaseSpecifier if well-formed, emits diagnostics
 /// and returns NULL otherwise.
 CXXBaseSpecifier *
 Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
                          SourceRange SpecifierRange,
                          bool Virtual, AccessSpecifier Access,
                          TypeSourceInfo *TInfo,
                          SourceLocation EllipsisLoc) {
+  // In HLSL, unspecified class access is public rather than private.
+  if (getLangOpts().HLSL && Class->getTagKind() == TTK_Class &&
+      Access == AS_none)
+    Access = AS_public;
+
   QualType BaseType = TInfo->getType();
   if (BaseType->containsErrors()) {
     // Already emitted a diagnostic when parsing the error type.
     return nullptr;
   }
   // C++ [class.union]p1:
   //   A union shall not have base classes.
   if (Class->isUnion()) {

clang/test/ParserHLSL/access_specifiers.hlsl

+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s -verify
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -DSTRUCT -o - %s -verify
+
+#ifdef STRUCT
+#define KEYWORD struct
+#else
+#define KEYWORD class
+#endif
+
+KEYWORD Doggo {
+  int legs;    // expected-note {{member is declared here}} expected-note {{member is declared here}}
+protected:     // expected-warning {{access specifiers are a clang HLSL extension}}
+  int ears[2]; // expected-note {{declared protected here}}
+private:       // expected-warning {{access specifiers are a clang HLSL extension}}
+  int tail;    // expected-note {{declared private here}} expected-note {{declared private here}}
+};
+
+KEYWORD Shiba : public Doggo { // expected-warning {{access specifiers are a clang HLSL extension}}
+  int undercoat;
+};
+
+KEYWORD Akita : Doggo {
+  int floof;
+};
+
+KEYWORD Chow : private Doggo {  // expected-warning {{access specifiers are a clang HLSL extension}} expected-note {{constrained by private inheritance here}}
+  int megafloof;
+};
+
+KEYWORD Dachshund : protected Doggo {  // expected-warning {{access specifiers are a clang HLSL extension}} expected-note {{constrained by protected inheritance here}}
+  int wiry;
+};
+
+void Puppers() {
+  Shiba Shibe;
+  Shibe.undercoat = 0xFFFF;
+  Shibe.legs = 4;
+
+  Shibe.tail = 1;    // expected-error {{'tail' is a private member of 'Doggo'}}
+  Shibe.ears[0] = 1; // expected-error {{'ears' is a protected member of 'Doggo'}}
+
+  Akita Aki;
+  Aki.floof = 0xFFFF;
+  Aki.legs = 4;
+
+  Aki.tail = 1; // expected-error {{'tail' is a private member of 'Doggo'}}
+
+  Chow Ch;
+  Ch.megafloof = 0xFFFF;
+
+  Ch.legs = 4; // expected-error {{'legs' is a private member of 'Doggo'}}
+
+  Dachshund DH;
+  DH.legs = 4; // expected-error {{'legs' is a protected member of 'Doggo'}}
+}