Skip to content

Commit f4fa361

Browse files
Michael137git-crd
authored andcommitted
[lldb][ObjC][NFC] Rewrite IsPossibleObjCMethodName in terms of llvm::StringRef (llvm#167660)
We've seen some crashes around this area (particularly around checking/handling raw C-strings). Dealing with `StringRef`s makes it a bit easier to reason about. This doesn't fix anything per se, but is an improvement in readability. rdar://164519648
1 parent 1aed1c9 commit f4fa361

File tree

3 files changed

+51
-7
lines changed

3 files changed

+51
-7
lines changed

lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,3 +1065,10 @@ ObjCLanguage::GetBooleanFromString(llvm::StringRef str) const {
10651065
.Case("NO", {false})
10661066
.Default({});
10671067
}
1068+
1069+
bool ObjCLanguage::IsPossibleObjCMethodName(llvm::StringRef name) {
1070+
if (!name.starts_with("-[") && !name.starts_with("+["))
1071+
return false;
1072+
1073+
return name.ends_with("]");
1074+
}

lldb/source/Plugins/Language/ObjC/ObjCLanguage.h

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -175,13 +175,7 @@ class ObjCLanguage : public Language {
175175

176176
static llvm::StringRef GetPluginNameStatic() { return "objc"; }
177177

178-
static bool IsPossibleObjCMethodName(const char *name) {
179-
if (!name)
180-
return false;
181-
bool starts_right = (name[0] == '+' || name[0] == '-') && name[1] == '[';
182-
bool ends_right = (name[strlen(name) - 1] == ']');
183-
return (starts_right && ends_right);
184-
}
178+
static bool IsPossibleObjCMethodName(llvm::StringRef name);
185179

186180
static bool IsPossibleObjCSelector(const char *name) {
187181
if (!name)

lldb/unittests/Language/ObjC/ObjCLanguageTest.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,3 +112,46 @@ TEST(ObjCLanguage, InvalidMethodNameParsing) {
112112
EXPECT_FALSE(lax_method.has_value());
113113
}
114114
}
115+
116+
struct ObjCMethodTestCase {
117+
llvm::StringRef name;
118+
bool is_valid;
119+
};
120+
121+
struct ObjCMethodNameTextFiture
122+
: public testing::TestWithParam<ObjCMethodTestCase> {};
123+
124+
static ObjCMethodTestCase g_objc_method_name_test_cases[] = {
125+
{"", false},
126+
{"+[Uh oh!", false},
127+
{"-[Definitely not...", false},
128+
{"[Nice try ] :)", false},
129+
{"+MaybeIfYouSquintYourEyes]", false},
130+
{"?[Tricky]", false},
131+
{"[]", false},
132+
{"-[a", false},
133+
{"+[a", false},
134+
{"-]a]", false},
135+
{"+]a]", false},
136+
137+
// FIXME: should these count as valid?
138+
{"+[]", true},
139+
{"-[]", true},
140+
{"-[[]", true},
141+
{"+[[]", true},
142+
{"+[a ]", true},
143+
{"-[a ]", true},
144+
145+
// Valid names
146+
{"+[a a]", true},
147+
{"-[a a]", true},
148+
};
149+
150+
TEST_P(ObjCMethodNameTextFiture, TestIsPossibleObjCMethodName) {
151+
// Tests ObjCLanguage::IsPossibleObjCMethodName
152+
auto [name, expect_valid] = GetParam();
153+
EXPECT_EQ(ObjCLanguage::IsPossibleObjCMethodName(name), expect_valid);
154+
}
155+
156+
INSTANTIATE_TEST_SUITE_P(ObjCMethodNameTests, ObjCMethodNameTextFiture,
157+
testing::ValuesIn(g_objc_method_name_test_cases));

0 commit comments

Comments
 (0)