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/6] [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 9431eea1f6be22..0d944434cda387 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 12aab09f285567..7e26bd13f9c7bc 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 491759e2fcdbb9..9df19b01f0b22d 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 1ede75d3782cdc..829efafd03a2c0 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 6880fa4bb0b03a..0e2fdc44321b5a 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 27c451c565f2bd..c66914b0686c82 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 6a2088a73c55b9..006328140d0ebf 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 00000000000000..32c8448f93fbe4 --- /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/6] [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 0d944434cda387..d78a042955e948 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 7e26bd13f9c7bc..ae82bdbff78707 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 9df19b01f0b22d..ea5752363f58c3 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 91f1c2f2e6239e..2a525228597bbe 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 829efafd03a2c0..7a7e21c59501be 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 0e2fdc44321b5a..61f7dad5680e78 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 c66914b0686c82..0163c641de78c1 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 006328140d0ebf..226b058add0adc 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 00000000000000..2d9ba88f65d431 --- /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 >From b6aa05d9d16cc5eb71d25bac41cd8fe4bb1f1bc2 Mon Sep 17 00:00:00 2001 From: Alastair Houghton <ahough...@apple.com> Date: Thu, 4 Jul 2024 12:18:35 +0100 Subject: [PATCH 3/6] [Parser][ObjC] Add -Wobjc-forbidden-prefixes warning option. Also rename `-Wobjc-prefix` to `-Wobjc-prefixes`, as that makes more sense, and fix some capitalization issues. Additionally, deal with `NSURL` and `NSUUID` and other similar things in a better way (specifically, for the allow and forbidden lists, we allow a match of the entire prefix against the name, rather than requiring additional characters). rdar://131055157 --- clang/include/clang/Basic/DiagnosticGroups.td | 3 +- .../clang/Basic/DiagnosticParseKinds.td | 11 +- clang/include/clang/Basic/LangOptions.h | 3 + clang/include/clang/Driver/Options.td | 12 +- clang/include/clang/Parse/Parser.h | 9 +- clang/lib/Driver/ToolChains/Clang.cpp | 3 +- clang/lib/Parse/ParseObjc.cpp | 110 +++++++++++------- clang/test/Parser/objc-prefixes2.m | 11 +- 8 files changed, 109 insertions(+), 53 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index d78a042955e948..8972410ea9347e 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -598,7 +598,8 @@ 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 ObjCPrefixes : DiagGroup<"objc-prefixes">; +def ObjCForbiddenPrefixes : DiagGroup<"objc-forbidden-prefixes">; 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 ae82bdbff78707..ff0c33488ea53a 100644 --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -572,10 +572,17 @@ def warn_objc_unprefixed_protocol_name : Warning< def warn_objc_bad_class_name_prefix : Warning< "Objective-C class name prefix not in permitted list">, - InGroup<ObjCPrefix>; + InGroup<ObjCPrefixes>; def warn_objc_bad_protocol_name_prefix : Warning< "Objective-C protocol name prefix not in permitted list">, - InGroup<ObjCPrefix>; + InGroup<ObjCPrefixes>; + +def warn_objc_forbidden_class_name_prefix : Warning< + "Objective-C class name prefix in forbidden list">, + InGroup<ObjCForbiddenPrefixes>; +def warn_objc_forbidden_protocol_name_prefix : Warning< + "Objective-C protocol name prefix in forbidden list">, + InGroup<ObjCForbiddenPrefixes>; /// Objective-C++ parser diagnostics def err_expected_token_instead_of_objcxx_keyword : Error< diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h index 2a525228597bbe..932bacc27d8cb6 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/clang/include/clang/Basic/LangOptions.h @@ -502,6 +502,9 @@ class LangOptions : public LangOptionsBase { /// Allowed prefixes for ObjC classes and protocols std::vector<std::string> ObjCAllowedPrefixes; + /// Forbidden prefixes for ObjC classes and protocols + std::vector<std::string> ObjCForbiddenPrefixes; + /// 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 7a7e21c59501be..2b7b95861c50c0 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3418,17 +3418,23 @@ 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>, +def Wobjc_prefixes_EQ: + CommaJoined<["-"], "Wobjc-prefixes=">, 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_forbidden_prefixes_EQ: + Joined<["-"], "Wobjc-forbidden-prefixes=">, Group<W_value_Group>, + Visibility<[ClangOption, CC1Option]>, + MetaVarName<"<prefixes>">, + HelpText<"Warn if Objective-C class or protocol names start with any prefix in a comma separated list <prefixes>. Takes precedence over both -Wobjc-prefixes and -Wobjc-prefix-length">, + MarshallingInfoStringVector<LangOpts<"ObjCForbiddenPrefixes">>; def Wobjc_prefix_length_EQ: 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. -Wobjc-prefix takes precedence if set">, + 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", diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index 61f7dad5680e78..bfbd3026669e54 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -1704,8 +1704,13 @@ class Parser : public CodeCompletionHandler { // Objective-C External Declarations void MaybeSkipAttributes(tok::ObjCKeywordKind Kind); - bool isObjCPublicNamePrefixAllowed(StringRef name); - bool isValidObjCPublicName(StringRef name); + enum ObjCPublicNameValidationResult { + ObjCNameUnprefixed = 0, + ObjCNameForbidden = -2, + ObjCNameNotAllowed = -1, + ObjCNameAllowed = 1 + }; + ObjCPublicNameValidationResult ValidateObjCPublicName(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 0163c641de78c1..0714e6bd35a270 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -6292,7 +6292,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.AddAllArgs(CmdArgs, options::OPT_Wobjc_prefixes_EQ); + Args.AddAllArgs(CmdArgs, options::OPT_Wobjc_forbidden_prefixes_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 226b058add0adc..37795fbaadd598 100644 --- a/clang/lib/Parse/ParseObjc.cpp +++ b/clang/lib/Parse/ParseObjc.cpp @@ -236,55 +236,61 @@ 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; +static inline bool ObjCNameMatchesPrefix(StringRef Name, StringRef Prefix) { + size_t NameLen = Name.size(); + size_t PrefixLen = Prefix.size(); + return (NameLen >= PrefixLen && Name.starts_with(Prefix) && + (NameLen == PrefixLen || isUppercase(Name[PrefixLen])) && + (NameLen == PrefixLen + 1 || !isUppercase(Name[PrefixLen + 1]))); +} - // 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; +Parser::ObjCPublicNameValidationResult +Parser::ValidateObjCPublicName(StringRef Name) { + // Check the -Wobjc-forbidden-prefixes list + if (!getLangOpts().ObjCForbiddenPrefixes.empty()) { + for (StringRef Prefix : getLangOpts().ObjCForbiddenPrefixes) { + if (ObjCNameMatchesPrefix(Name, Prefix)) + return ObjCNameForbidden; + } } - return false; -} + // Check the -Wobjc-prefixes list + if (!getLangOpts().ObjCAllowedPrefixes.empty()) { + for (StringRef Prefix : getLangOpts().ObjCAllowedPrefixes) { + if (ObjCNameMatchesPrefix(Name, Prefix)) + return ObjCNameAllowed; + } -bool Parser::isValidObjCPublicName(StringRef name) { - size_t nameLen = name.size(); - size_t requiredUpperCase = getLangOpts().ObjCPrefixLength + 1; + return ObjCNameNotAllowed; + } - // If ObjCPrefixLength is zero, we do no further checking - if (requiredUpperCase == 1) - return true; + // Finally, check against the -Wobjc-prefix-length setting + if (getLangOpts().ObjCPrefixLength) { + size_t NameLen = Name.size(); + size_t RequiredUpperCase = getLangOpts().ObjCPrefixLength + 1; - if (name.starts_with('_')) - return true; + if (Name.starts_with('_')) + return ObjCNameAllowed; - // 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; + // 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 ObjCNameAllowed; - if (nameLen < requiredUpperCase) - return false; + if (NameLen < RequiredUpperCase) + return ObjCNameUnprefixed; - for (size_t n = 0; n < requiredUpperCase; ++n) { - if (!isUppercase(name[n])) - return false; - } + for (size_t N = 0; N < RequiredUpperCase; ++N) { + if (!isUppercase(Name[N])) + return ObjCNameUnprefixed; + } - if (nameLen > requiredUpperCase && isUppercase(name[requiredUpperCase])) - return false; + if (NameLen > RequiredUpperCase && isUppercase(Name[RequiredUpperCase])) + return ObjCNameUnprefixed; + } - return true; + return ObjCNameAllowed; } /// @@ -405,10 +411,19 @@ Decl *Parser::ParseObjCAtInterfaceDeclaration(SourceLocation AtLoc, // Not a category - we are declaring a class if (!PP.getSourceManager().isInSystemHeader(nameLoc)) { - if (!isObjCPublicNamePrefixAllowed(nameId->getName())) - Diag(nameLoc, diag::warn_objc_bad_class_name_prefix); - else if (!isValidObjCPublicName(nameId->getName())) + switch (ValidateObjCPublicName(nameId->getName())) { + case ObjCNameUnprefixed: Diag(nameLoc, diag::warn_objc_unprefixed_class_name); + break; + case ObjCNameNotAllowed: + Diag(nameLoc, diag::warn_objc_bad_class_name_prefix); + break; + case ObjCNameForbidden: + Diag(nameLoc, diag::warn_objc_forbidden_class_name_prefix); + break; + case ObjCNameAllowed: + break; + } } // Parse a class interface. @@ -2174,10 +2189,19 @@ Parser::ParseObjCAtProtocolDeclaration(SourceLocation AtLoc, SourceLocation nameLoc = ConsumeToken(); if (!PP.getSourceManager().isInSystemHeader(nameLoc)) { - if (!isObjCPublicNamePrefixAllowed(protocolName->getName())) - Diag(nameLoc, diag::warn_objc_bad_protocol_name_prefix); - else if (!isValidObjCPublicName(protocolName->getName())) + switch (ValidateObjCPublicName(protocolName->getName())) { + case ObjCNameUnprefixed: Diag(nameLoc, diag::warn_objc_unprefixed_protocol_name); + break; + case ObjCNameNotAllowed: + Diag(nameLoc, diag::warn_objc_bad_protocol_name_prefix); + break; + case ObjCNameForbidden: + Diag(nameLoc, diag::warn_objc_forbidden_protocol_name_prefix); + break; + case ObjCNameAllowed: + break; + } } 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 index 2d9ba88f65d431..fc94b15663dcf9 100644 --- a/clang/test/Parser/objc-prefixes2.m +++ b/clang/test/Parser/objc-prefixes2.m @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 %s -Wobjc-prefixes=NS,NSCF,NSURL -fsyntax-only -verify +// RUN: %clang_cc1 %s -Wobjc-prefixes=NS,NSCF,NSURL -Wobjc-forbidden-prefixes=XX -fsyntax-only -verify // Test prefix list rules @@ -25,3 +25,12 @@ @interface NSURLFoo @interface NSRGBColor // expected-warning {{Objective-C class name prefix not in permitted list}} @end + +@protocol NSRGBColorProtocol // expected-warning {{Objective-C protocol name prefix not in permitted list}} +@end + +@interface XXFoo // expected-warning {{Objective-C class name prefix in forbidden list}} +@end + +@protocol XXFooProtocol // expected-warning {{Objective-C protocol name prefix in forbidden list}} +@end >From a4aa2565573ce7f3b02faf85caca9ab5ed4cfe3a Mon Sep 17 00:00:00 2001 From: Alastair Houghton <ahough...@apple.com> Date: Thu, 4 Jul 2024 14:54:45 +0100 Subject: [PATCH 4/6] [Parser][ObjC] Just add the last copy of each argument. We probably should just add the last copy of the `-Wobjc-prefix` argument(s). rdar://131055157 --- clang/lib/Driver/ToolChains/Clang.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 0714e6bd35a270..53f02e258c1cad 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -6292,8 +6292,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_prefixes_EQ); - Args.AddAllArgs(CmdArgs, options::OPT_Wobjc_forbidden_prefixes_EQ); + Args.AddLastArg(CmdArgs, options::OPT_Wobjc_prefixes_EQ); + Args.AddLastArg(CmdArgs, options::OPT_Wobjc_forbidden_prefixes_EQ); Args.AddLastArg(CmdArgs, options::OPT_Wobjc_prefix_length_EQ); if (Args.hasFlag(options::OPT_pedantic, options::OPT_no_pedantic, false)) >From d8896aa7eb7929ce5451932e6e6e4deab23e3ebb Mon Sep 17 00:00:00 2001 From: Alastair Houghton <ahough...@apple.com> Date: Thu, 4 Jul 2024 17:03:14 +0100 Subject: [PATCH 5/6] [Parser][ObjC] Don't use the W_value_Group for -Wobjc-prefix* If we use `W_value_Group`, we end up adding extra arguments from `clang::ParseDiagnosticArgs`, which results in argument round trip failure. rdar://131055157 --- clang/include/clang/Driver/Options.td | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 2b7b95861c50c0..4aa4eb632766e1 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3419,19 +3419,19 @@ defm objc_exceptions : BoolFOption<"objc-exceptions", "Enable Objective-C exceptions">, NegFlag<SetFalse>>; def Wobjc_prefixes_EQ: - CommaJoined<["-"], "Wobjc-prefixes=">, Group<W_value_Group>, + CommaJoined<["-"], "Wobjc-prefixes=">, 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_forbidden_prefixes_EQ: - Joined<["-"], "Wobjc-forbidden-prefixes=">, Group<W_value_Group>, + Joined<["-"], "Wobjc-forbidden-prefixes=">, Visibility<[ClangOption, CC1Option]>, MetaVarName<"<prefixes>">, HelpText<"Warn if Objective-C class or protocol names start with any prefix in a comma separated list <prefixes>. Takes precedence over both -Wobjc-prefixes and -Wobjc-prefix-length">, MarshallingInfoStringVector<LangOpts<"ObjCForbiddenPrefixes">>; def Wobjc_prefix_length_EQ: - Joined<["-"], "Wobjc-prefix-length=">, Group<W_value_Group>, + 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">, >From 19ba1efd1c3babd4b4b02f310cfd81538d125be8 Mon Sep 17 00:00:00 2001 From: Alastair Houghton <ahough...@apple.com> Date: Thu, 4 Jul 2024 17:46:40 +0100 Subject: [PATCH 6/6] [Parser][ObjC] Fix an off-by-one bug. Don't try to read off the end of the `StringRef` for the name in `ObjCNameMatchesPrefix()`. rdar://131055157 --- clang/lib/Parse/ParseObjc.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp index 37795fbaadd598..4d0f6b41832efb 100644 --- a/clang/lib/Parse/ParseObjc.cpp +++ b/clang/lib/Parse/ParseObjc.cpp @@ -241,8 +241,8 @@ static inline bool ObjCNameMatchesPrefix(StringRef Name, StringRef Prefix) { size_t NameLen = Name.size(); size_t PrefixLen = Prefix.size(); return (NameLen >= PrefixLen && Name.starts_with(Prefix) && - (NameLen == PrefixLen || isUppercase(Name[PrefixLen])) && - (NameLen == PrefixLen + 1 || !isUppercase(Name[PrefixLen + 1]))); + (NameLen <= PrefixLen || isUppercase(Name[PrefixLen])) && + (NameLen <= PrefixLen + 1 || !isUppercase(Name[PrefixLen + 1]))); } Parser::ObjCPublicNameValidationResult _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits