https://github.com/al45tair updated https://github.com/llvm/llvm-project/pull/97597
>From 02f7c5ef71b382866e02037ce82c85fa6fcbb394 Mon Sep 17 00:00:00 2001 From: Alastair Houghton <ahough...@apple.com> Date: Wed, 3 Jul 2024 17:00:51 +0100 Subject: [PATCH 1/2] [Parser][ObjC] Add -Wobjc-prefix-length warning option. This lets clang generate warnings automatically if it sees Objective-C names that don't have the correct prefix length. rdar://131055157 --- clang/include/clang/Basic/DiagnosticGroups.td | 1 + .../clang/Basic/DiagnosticParseKinds.td | 7 ++ clang/include/clang/Basic/LangOptions.def | 2 + clang/include/clang/Driver/Options.td | 7 ++ clang/include/clang/Parse/Parser.h | 1 + clang/lib/Driver/ToolChains/Clang.cpp | 1 + clang/lib/Parse/ParseObjc.cpp | 65 +++++++++++++++++++ clang/test/Parser/objc-prefixes.m | 62 ++++++++++++++++++ 8 files changed, 146 insertions(+) create mode 100644 clang/test/Parser/objc-prefixes.m diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 9431eea1f6be2..0d944434cda38 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -598,6 +598,7 @@ def ObjCPointerIntrospect : DiagGroup<"deprecated-objc-pointer-introspection", [ def ObjCMultipleMethodNames : DiagGroup<"objc-multiple-method-names">; def ObjCFlexibleArray : DiagGroup<"objc-flexible-array">; def ObjCBoxing : DiagGroup<"objc-boxing">; +def ObjCPrefixLength : DiagGroup<"objc-prefix-length">; def CompletionHandler : DiagGroup<"completion-handler">; def CalledOnceParameter : DiagGroup<"called-once-parameter", [CompletionHandler]>; def OpenCLUnsupportedRGBA: DiagGroup<"opencl-unsupported-rgba">; diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td index 12aab09f28556..7e26bd13f9c7b 100644 --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -563,6 +563,13 @@ def err_declaration_does_not_declare_param : Error< "declaration does not declare a parameter">; def err_no_matching_param : Error<"parameter named %0 is missing">; +def warn_objc_unprefixed_class_name : Warning< + "un-prefixed Objective-C class name">, + InGroup<ObjCPrefixLength>; +def warn_objc_unprefixed_protocol_name : Warning< + "un-prefixed Objective-C protocol name">, + InGroup<ObjCPrefixLength>; + /// Objective-C++ parser diagnostics def err_expected_token_instead_of_objcxx_keyword : Error< "expected %0; %1 is a keyword in Objective-C++">; diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def index 491759e2fcdbb..9df19b01f0b22 100644 --- a/clang/include/clang/Basic/LangOptions.def +++ b/clang/include/clang/Basic/LangOptions.def @@ -351,6 +351,8 @@ LANGOPT(ObjCAutoRefCount , 1, 0, "Objective-C automated reference counting") LANGOPT(ObjCWeakRuntime , 1, 0, "__weak support in the ARC runtime") LANGOPT(ObjCWeak , 1, 0, "Objective-C __weak in ARC and MRC files") LANGOPT(ObjCSubscriptingLegacyRuntime , 1, 0, "Subscripting support in legacy ObjectiveC runtime") +BENIGN_LANGOPT(ObjCPrefixLength, 32, 0, + "if non-zero, warn about Objective-C classes or protocols with names that do not have a prefix of the specified length.") BENIGN_LANGOPT(CompatibilityQualifiedIdBlockParamTypeChecking, 1, 0, "compatibility mode for type checking block parameters " "involving qualified id types") diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 1ede75d3782cd..829efafd03a2c 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3418,6 +3418,13 @@ defm objc_exceptions : BoolFOption<"objc-exceptions", PosFlag<SetTrue, [], [ClangOption, CC1Option], "Enable Objective-C exceptions">, NegFlag<SetFalse>>; +def Wobjc_prefix_length_EQ: + Joined<["-"], "Wobjc-prefix-length=">, + Visibility<[ClangOption, CC1Option]>, + MetaVarName<"<N>">, + HelpText<"Warn if Objective-C class or protocol names do not start with a prefix of the specified length">, + MarshallingInfoInt<LangOpts<"ObjCPrefixLength">>; + defm application_extension : BoolFOption<"application-extension", LangOpts<"AppExt">, DefaultFalse, PosFlag<SetTrue, [], [ClangOption, CC1Option], diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index 6880fa4bb0b03..0e2fdc44321b5 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -1704,6 +1704,7 @@ class Parser : public CodeCompletionHandler { // Objective-C External Declarations void MaybeSkipAttributes(tok::ObjCKeywordKind Kind); + bool isValidObjCPublicName(StringRef name); DeclGroupPtrTy ParseObjCAtDirectives(ParsedAttributes &DeclAttrs, ParsedAttributes &DeclSpecAttrs); DeclGroupPtrTy ParseObjCAtClassDeclaration(SourceLocation atLoc); diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 27c451c565f2b..c66914b0686c8 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -6291,6 +6291,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, } Args.AddAllArgs(CmdArgs, options::OPT_Wsystem_headers_in_module_EQ); + Args.AddLastArg(CmdArgs, options::OPT_Wobjc_prefix_length_EQ); if (Args.hasFlag(options::OPT_pedantic, options::OPT_no_pedantic, false)) CmdArgs.push_back("-pedantic"); diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp index 6a2088a73c55b..006328140d0eb 100644 --- a/clang/lib/Parse/ParseObjc.cpp +++ b/clang/lib/Parse/ParseObjc.cpp @@ -204,6 +204,57 @@ void Parser::CheckNestedObjCContexts(SourceLocation AtLoc) Diag(Decl->getBeginLoc(), diag::note_objc_container_start) << (int)ock; } +/// An Objective-C public name (a class name or protocol name) is +/// expected to have a prefix as all names are in a single global +/// namespace. +/// +/// If the -Wobjc-prefix-length is set to N, the name must start +/// with N+1 capital letters, which must be followed by a character +/// that is not a capital letter. +/// +/// For instance, for N set to 2, the following are valid: +/// +/// NSString +/// NSArray +/// +/// but these are not: +/// +/// MyString +/// NSKString +/// NSnotAString +/// +/// We make a special exception for NSCF things when the prefix is set +/// to length 2, because that's an unusual special case in the implementation +/// of the Cocoa frameworks. +/// +/// Names that start with underscores are exempt from this check, but +/// are reserved for the system and should not be used by user code. +bool Parser::isValidObjCPublicName(StringRef name) { + size_t nameLen = name.size(); + size_t requiredUpperCase = getLangOpts().ObjCPrefixLength + 1; + + if (name.starts_with('_')) + return true; + + // Special case for NSCF when prefix length is 2 + if (requiredUpperCase == 3 && nameLen > 4 && name.starts_with("NSCF") && + isUppercase(name[4]) && (nameLen == 5 || !isUppercase(name[5]))) + return true; + + if (nameLen < requiredUpperCase) + return false; + + for (size_t n = 0; n < requiredUpperCase; ++n) { + if (!isUppercase(name[n])) + return false; + } + + if (nameLen > requiredUpperCase && isUppercase(name[requiredUpperCase])) + return false; + + return true; +} + /// /// objc-interface: /// objc-class-interface-attributes[opt] objc-class-interface @@ -319,6 +370,14 @@ Decl *Parser::ParseObjCAtInterfaceDeclaration(SourceLocation AtLoc, return CategoryType; } + + // Not a category - we are declaring a class + if (getLangOpts().ObjCPrefixLength && + !PP.getSourceManager().isInSystemHeader(nameLoc) && + !isValidObjCPublicName(nameId->getName())) { + Diag(nameLoc, diag::warn_objc_unprefixed_class_name); + } + // Parse a class interface. IdentifierInfo *superClassId = nullptr; SourceLocation superClassLoc; @@ -2081,6 +2140,12 @@ Parser::ParseObjCAtProtocolDeclaration(SourceLocation AtLoc, IdentifierInfo *protocolName = Tok.getIdentifierInfo(); SourceLocation nameLoc = ConsumeToken(); + if (getLangOpts().ObjCPrefixLength && + !PP.getSourceManager().isInSystemHeader(nameLoc) && + !isValidObjCPublicName(protocolName->getName())) { + Diag(nameLoc, diag::warn_objc_unprefixed_protocol_name); + } + if (TryConsumeToken(tok::semi)) { // forward declaration of one protocol. IdentifierLocPair ProtoInfo(protocolName, nameLoc); return Actions.ObjC().ActOnForwardProtocolDeclaration(AtLoc, ProtoInfo, diff --git a/clang/test/Parser/objc-prefixes.m b/clang/test/Parser/objc-prefixes.m new file mode 100644 index 0000000000000..32c8448f93fbe --- /dev/null +++ b/clang/test/Parser/objc-prefixes.m @@ -0,0 +1,62 @@ +// RUN: %clang_cc1 %s -Wobjc-prefix-length=2 -fsyntax-only -verify + +// Test prefix length rules for ObjC interfaces and protocols + +// -- Plain interfaces -------------------------------------------------------- + +@interface _Foo +@end + +@interface Foo // expected-warning {{un-prefixed Objective-C class name}} +@end + +@interface NSFoo +@end + +// Special case for prefix-length 2 +@interface NSCFFoo +@end + +@interface NSCFXFoo // expected-warning {{un-prefixed Objective-C class name}} +@end + +@interface NSXFoo // expected-warning {{un-prefixed Objective-C class name}} +@end + +// -- Categories -------------------------------------------------------------- + +// Categories don't trigger these warnings + +@interface Foo (Bar) +@end + +@interface NSFoo (Bar) +@end + +@interface NSCFFoo (Bar) +@end + +@interface NSXFoo (Bar) +@end + +// -- Protocols --------------------------------------------------------------- + +@protocol _FooProtocol +@end + +@protocol FooProtocol // expected-warning {{un-prefixed Objective-C protocol name}} +@end + +@protocol NSFooProtocol +@end + +// Special case for prefix-length 2 +@protocol NSCFFooProtocol +@end + +@protocol NSCFXFooProtocol // expected-warning {{un-prefixed Objective-C protocol name}} +@end + +@protocol NSXFooProtocol // expected-warning {{un-prefixed Objective-C protocol name}} +@end + >From a0331bb896bc3d0a58b5bd4bdb81b7c31f0a3446 Mon Sep 17 00:00:00 2001 From: Alastair Houghton <ahough...@apple.com> Date: Wed, 3 Jul 2024 18:14:34 +0100 Subject: [PATCH 2/2] [Parser][ObjC] Add -Wobjc-prefix warning option. This allows you to specify a comma separated list of allowed prefixes, as an alternative to the -Wobjc-prefix-length option. rdar://131055157 --- clang/include/clang/Basic/DiagnosticGroups.td | 1 + .../clang/Basic/DiagnosticParseKinds.td | 7 +++ clang/include/clang/Basic/LangOptions.def | 2 +- clang/include/clang/Basic/LangOptions.h | 3 ++ clang/include/clang/Driver/Options.td | 10 +++- clang/include/clang/Parse/Parser.h | 1 + clang/lib/Driver/ToolChains/Clang.cpp | 2 + clang/lib/Parse/ParseObjc.cpp | 50 ++++++++++++++++--- clang/test/Parser/objc-prefixes2.m | 27 ++++++++++ 9 files changed, 92 insertions(+), 11 deletions(-) create mode 100644 clang/test/Parser/objc-prefixes2.m diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 0d944434cda38..d78a042955e94 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -598,6 +598,7 @@ def ObjCPointerIntrospect : DiagGroup<"deprecated-objc-pointer-introspection", [ def ObjCMultipleMethodNames : DiagGroup<"objc-multiple-method-names">; def ObjCFlexibleArray : DiagGroup<"objc-flexible-array">; def ObjCBoxing : DiagGroup<"objc-boxing">; +def ObjCPrefix : DiagGroup<"objc-prefix">; def ObjCPrefixLength : DiagGroup<"objc-prefix-length">; def CompletionHandler : DiagGroup<"completion-handler">; def CalledOnceParameter : DiagGroup<"called-once-parameter", [CompletionHandler]>; diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td index 7e26bd13f9c7b..ae82bdbff7870 100644 --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -570,6 +570,13 @@ def warn_objc_unprefixed_protocol_name : Warning< "un-prefixed Objective-C protocol name">, InGroup<ObjCPrefixLength>; +def warn_objc_bad_class_name_prefix : Warning< + "Objective-C class name prefix not in permitted list">, + InGroup<ObjCPrefix>; +def warn_objc_bad_protocol_name_prefix : Warning< + "Objective-C protocol name prefix not in permitted list">, + InGroup<ObjCPrefix>; + /// Objective-C++ parser diagnostics def err_expected_token_instead_of_objcxx_keyword : Error< "expected %0; %1 is a keyword in Objective-C++">; diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def index 9df19b01f0b22..ea5752363f58c 100644 --- a/clang/include/clang/Basic/LangOptions.def +++ b/clang/include/clang/Basic/LangOptions.def @@ -351,7 +351,7 @@ LANGOPT(ObjCAutoRefCount , 1, 0, "Objective-C automated reference counting") LANGOPT(ObjCWeakRuntime , 1, 0, "__weak support in the ARC runtime") LANGOPT(ObjCWeak , 1, 0, "Objective-C __weak in ARC and MRC files") LANGOPT(ObjCSubscriptingLegacyRuntime , 1, 0, "Subscripting support in legacy ObjectiveC runtime") -BENIGN_LANGOPT(ObjCPrefixLength, 32, 0, +BENIGN_VALUE_LANGOPT(ObjCPrefixLength, 32, 0, "if non-zero, warn about Objective-C classes or protocols with names that do not have a prefix of the specified length.") BENIGN_LANGOPT(CompatibilityQualifiedIdBlockParamTypeChecking, 1, 0, "compatibility mode for type checking block parameters " diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h index 91f1c2f2e6239..2a525228597bb 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/clang/include/clang/Basic/LangOptions.h @@ -499,6 +499,9 @@ class LangOptions : public LangOptionsBase { std::string ObjCConstantStringClass; + /// Allowed prefixes for ObjC classes and protocols + std::vector<std::string> ObjCAllowedPrefixes; + /// The name of the handler function to be called when -ftrapv is /// specified. /// diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 829efafd03a2c..7a7e21c59501b 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3418,11 +3418,17 @@ defm objc_exceptions : BoolFOption<"objc-exceptions", PosFlag<SetTrue, [], [ClangOption, CC1Option], "Enable Objective-C exceptions">, NegFlag<SetFalse>>; +def Wobjc_prefix_EQ: + CommaJoined<["-"], "Wobjc-prefix=">, Group<W_value_Group>, + Visibility<[ClangOption, CC1Option]>, + MetaVarName<"<prefixes>">, + HelpText<"Warn if Objective-C class or protocol names do not start with any prefix in a comma separated list <prefixes>. Takes precedence over -Wobjc-prefix-length">, + MarshallingInfoStringVector<LangOpts<"ObjCAllowedPrefixes">>; def Wobjc_prefix_length_EQ: - Joined<["-"], "Wobjc-prefix-length=">, + Joined<["-"], "Wobjc-prefix-length=">, Group<W_value_Group>, Visibility<[ClangOption, CC1Option]>, MetaVarName<"<N>">, - HelpText<"Warn if Objective-C class or protocol names do not start with a prefix of the specified length">, + HelpText<"Warn if Objective-C class or protocol names do not start with a prefix of the specified length. -Wobjc-prefix takes precedence if set">, MarshallingInfoInt<LangOpts<"ObjCPrefixLength">>; defm application_extension : BoolFOption<"application-extension", diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index 0e2fdc44321b5..61f7dad5680e7 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -1704,6 +1704,7 @@ class Parser : public CodeCompletionHandler { // Objective-C External Declarations void MaybeSkipAttributes(tok::ObjCKeywordKind Kind); + bool isObjCPublicNamePrefixAllowed(StringRef name); bool isValidObjCPublicName(StringRef name); DeclGroupPtrTy ParseObjCAtDirectives(ParsedAttributes &DeclAttrs, ParsedAttributes &DeclSpecAttrs); diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index c66914b0686c8..0163c641de78c 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -6291,6 +6291,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, } Args.AddAllArgs(CmdArgs, options::OPT_Wsystem_headers_in_module_EQ); + + Args.AddAllArgs(CmdArgs, options::OPT_Wobjc_prefix_EQ); Args.AddLastArg(CmdArgs, options::OPT_Wobjc_prefix_length_EQ); if (Args.hasFlag(options::OPT_pedantic, options::OPT_no_pedantic, false)) diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp index 006328140d0eb..226b058add0ad 100644 --- a/clang/lib/Parse/ParseObjc.cpp +++ b/clang/lib/Parse/ParseObjc.cpp @@ -208,6 +208,13 @@ void Parser::CheckNestedObjCContexts(SourceLocation AtLoc) /// expected to have a prefix as all names are in a single global /// namespace. /// +/// If the `-Wobjc-prefix=<list>` option is active, it specifies a list +/// of permitted prefixes; classes and protocols must start with a +/// prefix from that list. Note that in this case, we check that +/// the name matches <prefix>(<upper-case><not-upper-case>?)?, so e.g. +/// if you specify `-Wobjc-prefix=NS`, then `NSURL` would not be valid; +/// you would want to specify `-Wobjc-prefix=NS,NSURL` in that case. +/// /// If the -Wobjc-prefix-length is set to N, the name must start /// with N+1 capital letters, which must be followed by a character /// that is not a capital letter. @@ -229,10 +236,35 @@ void Parser::CheckNestedObjCContexts(SourceLocation AtLoc) /// /// Names that start with underscores are exempt from this check, but /// are reserved for the system and should not be used by user code. +bool Parser::isObjCPublicNamePrefixAllowed(StringRef name) { + size_t nameLen = name.size(); + + // If there's nothing in the list, it's allowed + if (getLangOpts().ObjCAllowedPrefixes.empty()) + return true; + + // Otherwise it must start with a list entry, optionally followed by + // an uppercase letter and then optionally by something that isn't an + // uppercase letter. + for (StringRef prefix : getLangOpts().ObjCAllowedPrefixes) { + size_t prefixLen = prefix.size(); + if (nameLen >= prefixLen && name.starts_with(prefix) && + (nameLen == prefixLen || isUppercase(name[prefixLen])) && + (nameLen == prefixLen + 1 || !isUppercase(name[prefixLen + 1]))) + return true; + } + + return false; +} + bool Parser::isValidObjCPublicName(StringRef name) { size_t nameLen = name.size(); size_t requiredUpperCase = getLangOpts().ObjCPrefixLength + 1; + // If ObjCPrefixLength is zero, we do no further checking + if (requiredUpperCase == 1) + return true; + if (name.starts_with('_')) return true; @@ -372,10 +404,11 @@ Decl *Parser::ParseObjCAtInterfaceDeclaration(SourceLocation AtLoc, } // Not a category - we are declaring a class - if (getLangOpts().ObjCPrefixLength && - !PP.getSourceManager().isInSystemHeader(nameLoc) && - !isValidObjCPublicName(nameId->getName())) { - Diag(nameLoc, diag::warn_objc_unprefixed_class_name); + if (!PP.getSourceManager().isInSystemHeader(nameLoc)) { + if (!isObjCPublicNamePrefixAllowed(nameId->getName())) + Diag(nameLoc, diag::warn_objc_bad_class_name_prefix); + else if (!isValidObjCPublicName(nameId->getName())) + Diag(nameLoc, diag::warn_objc_unprefixed_class_name); } // Parse a class interface. @@ -2140,10 +2173,11 @@ Parser::ParseObjCAtProtocolDeclaration(SourceLocation AtLoc, IdentifierInfo *protocolName = Tok.getIdentifierInfo(); SourceLocation nameLoc = ConsumeToken(); - if (getLangOpts().ObjCPrefixLength && - !PP.getSourceManager().isInSystemHeader(nameLoc) && - !isValidObjCPublicName(protocolName->getName())) { - Diag(nameLoc, diag::warn_objc_unprefixed_protocol_name); + if (!PP.getSourceManager().isInSystemHeader(nameLoc)) { + if (!isObjCPublicNamePrefixAllowed(protocolName->getName())) + Diag(nameLoc, diag::warn_objc_bad_protocol_name_prefix); + else if (!isValidObjCPublicName(protocolName->getName())) + Diag(nameLoc, diag::warn_objc_unprefixed_protocol_name); } if (TryConsumeToken(tok::semi)) { // forward declaration of one protocol. diff --git a/clang/test/Parser/objc-prefixes2.m b/clang/test/Parser/objc-prefixes2.m new file mode 100644 index 0000000000000..2d9ba88f65d43 --- /dev/null +++ b/clang/test/Parser/objc-prefixes2.m @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 %s -Wobjc-prefixes=NS,NSCF,NSURL -fsyntax-only -verify + +// Test prefix list rules + +@interface Foo // expected-warning {{Objective-C class name prefix not in permitted list}} +@end + +@interface NSFoo +@end + +@interface NSfoo // expected-warning {{Objective-C class name prefix not in permitted list}} +@end + +@interface NSFFoo // expected-warning {{Objective-C class name prefix not in permitted list}} +@end + +@interface NSCFFoo +@end + +@interface NSURL +@end + +@interface NSURLFoo +@end + +@interface NSRGBColor // expected-warning {{Objective-C class name prefix not in permitted list}} +@end _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits