[clang] [Parser][ObjC] Add -Wobjc-prefix-length warning option. (PR #97597)
Alastair Houghton via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 4 09:48:06 PDT 2024
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 <ahoughton at 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 --------------------------------------------------------
+
+ at interface _Foo
+ at end
+
+ at interface Foo // expected-warning {{un-prefixed Objective-C class name}}
+ at end
+
+ at interface NSFoo
+ at end
+
+// Special case for prefix-length 2
+ at interface NSCFFoo
+ at end
+
+ at interface NSCFXFoo // expected-warning {{un-prefixed Objective-C class name}}
+ at end
+
+ at interface NSXFoo // expected-warning {{un-prefixed Objective-C class name}}
+ at end
+
+// -- Categories --------------------------------------------------------------
+
+// Categories don't trigger these warnings
+
+ at interface Foo (Bar)
+ at end
+
+ at interface NSFoo (Bar)
+ at end
+
+ at interface NSCFFoo (Bar)
+ at end
+
+ at interface NSXFoo (Bar)
+ at end
+
+// -- Protocols ---------------------------------------------------------------
+
+ at protocol _FooProtocol
+ at end
+
+ at protocol FooProtocol // expected-warning {{un-prefixed Objective-C protocol name}}
+ at end
+
+ at protocol NSFooProtocol
+ at end
+
+// Special case for prefix-length 2
+ at protocol NSCFFooProtocol
+ at end
+
+ at protocol NSCFXFooProtocol // expected-warning {{un-prefixed Objective-C protocol name}}
+ at end
+
+ at protocol NSXFooProtocol // expected-warning {{un-prefixed Objective-C protocol name}}
+ at end
+
>From a0331bb896bc3d0a58b5bd4bdb81b7c31f0a3446 Mon Sep 17 00:00:00 2001
From: Alastair Houghton <ahoughton at 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
+
+ at interface Foo // expected-warning {{Objective-C class name prefix not in permitted list}}
+ at end
+
+ at interface NSFoo
+ at end
+
+ at interface NSfoo // expected-warning {{Objective-C class name prefix not in permitted list}}
+ at end
+
+ at interface NSFFoo // expected-warning {{Objective-C class name prefix not in permitted list}}
+ at end
+
+ at interface NSCFFoo
+ at end
+
+ at interface NSURL
+ at end
+
+ at interface NSURLFoo
+ at end
+
+ at interface NSRGBColor // expected-warning {{Objective-C class name prefix not in permitted list}}
+ at end
>From b6aa05d9d16cc5eb71d25bac41cd8fe4bb1f1bc2 Mon Sep 17 00:00:00 2001
From: Alastair Houghton <ahoughton at 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
+
+ at protocol NSRGBColorProtocol // expected-warning {{Objective-C protocol name prefix not in permitted list}}
+ at end
+
+ at interface XXFoo // expected-warning {{Objective-C class name prefix in forbidden list}}
+ at end
+
+ at protocol XXFooProtocol // expected-warning {{Objective-C protocol name prefix in forbidden list}}
+ at end
>From a4aa2565573ce7f3b02faf85caca9ab5ed4cfe3a Mon Sep 17 00:00:00 2001
From: Alastair Houghton <ahoughton at 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 <ahoughton at 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 <ahoughton at 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
More information about the cfe-commits
mailing list