[PATCH] D126972: [clang][dataflow] Modify optional-checker to handle type aliases.
ymandel updated this revision to Diff 434048.
ymandel marked an inline comment as done.
ymandel added a comment.
address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126972/new/
https://reviews.llvm.org/D126972
Files:
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
Index:
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2214,6 +2214,23 @@
UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:10:7")));
}
+// Verifies that the model sees through aliases.
+TEST_P(UncheckedOptionalAccessTest, WithAlias) {
+ ExpectLatticeChecksFor(
+ R"(
+#include "unchecked_optional_access_test.h"
+
+template
+using MyOptional = $ns::$optional;
+
+void target(MyOptional opt) {
+ opt.value();
+ /*[[check]]*/
+}
+ )",
+ UnorderedElementsAre(Pair("check", "unsafe: input.cc:8:7")));
+}
+
// FIXME: Add support for:
// - constructors (copy, move)
// - assignment operators (default, copy, move)
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -43,13 +43,14 @@
hasTemplateArgument(0, refersToType(type().bind("T";
}
-auto hasOptionalType() { return hasType(optionalClass()); }
-
-auto hasOptionalOrAliasType() {
+auto optionalOrAliasType() {
return hasUnqualifiedDesugaredType(
recordType(hasDeclaration(optionalClass(;
}
+/// Matches any of the spellings of the optional types and sugar, aliases, etc.
+auto hasOptionalType() { return hasType(optionalOrAliasType()); }
+
auto isOptionalMemberCallWithName(
llvm::StringRef MemberName,
llvm::Optional Ignorable = llvm::None) {
@@ -164,9 +165,8 @@
}
auto isCallReturningOptional() {
- return callExpr(callee(functionDecl(
- returns(anyOf(hasOptionalOrAliasType(),
-referenceType(pointee(hasOptionalOrAliasType(;
+ return callExpr(callee(functionDecl(returns(anyOf(
+ optionalOrAliasType(),
referenceType(pointee(optionalOrAliasType(;
}
/// Creates a symbolic value for an `optional` value using `HasValueVal` as the
@@ -485,8 +485,9 @@
return MatchSwitchBuilder()
// Attach a symbolic "has_value" state to optional values that we see for
// the first time.
- .CaseOf(expr(anyOf(declRefExpr(), memberExpr()),
hasOptionalType()),
-initializeOptionalReference)
+ .CaseOf(
+ expr(anyOf(declRefExpr(), memberExpr()), hasOptionalType()),
+ initializeOptionalReference)
// make_optional
.CaseOf(isMakeOptionalCall(), transferMakeOptionalCall)
Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2214,6 +2214,23 @@
UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:10:7")));
}
+// Verifies that the model sees through aliases.
+TEST_P(UncheckedOptionalAccessTest, WithAlias) {
+ ExpectLatticeChecksFor(
+ R"(
+#include "unchecked_optional_access_test.h"
+
+template
+using MyOptional = $ns::$optional;
+
+void target(MyOptional opt) {
+ opt.value();
+ /*[[check]]*/
+}
+ )",
+ UnorderedElementsAre(Pair("check", "unsafe: input.cc:8:7")));
+}
+
// FIXME: Add support for:
// - constructors (copy, move)
// - assignment operators (default, copy, move)
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -43,13 +43,14 @@
hasTemplateArgument(0, refersToType(type().bind("T";
}
-auto hasOptionalType() { return hasType(optionalClass()); }
-
-auto hasOptionalOrAliasType() {
+auto optionalOrAliasType() {
return hasUnqualifiedDesugaredType(
recordType(hasDeclaration(optionalClass(;
}
+/// Matches any of the spellings of the optional types and sugar, aliases, etc.
+auto hasOptionalType() { return hasType(optionalOrAliasType()); }
+
auto isOptionalMemberCallWithName(
llvm::StringRef MemberName,
llvm::Optional Ignorable =
[PATCH] D126972: [clang][dataflow] Modify optional-checker to handle type aliases.
sgatev accepted this revision.
sgatev added inline comments.
This revision is now accepted and ready to land.
Comment at:
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp:2217
+// Verifies that the check sees through aliases.
+TEST_P(UncheckedOptionalAccessTest, WithAlias) {
Let's replace "check"/"checker" with "model" here and in the commit message.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126972/new/
https://reviews.llvm.org/D126972
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D126972: [clang][dataflow] Modify optional-checker to handle type aliases.
ymandel created this revision.
ymandel added reviewers: xazax.hun, sgatev.
Herald added subscribers: tschuett, steakhal, jeroen.dobbelaere, rnkovacs.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.
Previously, type aliases were not handled (and resulted in an assertion
firing). This patch generalizes the code to consider aliases everywhere (a
previous patch already considered aliases for optional-returning functions).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D126972
Files:
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
Index:
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2214,6 +2214,23 @@
UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:10:7")));
}
+// Verifies that the check sees through aliases.
+TEST_P(UncheckedOptionalAccessTest, WithAlias) {
+ ExpectLatticeChecksFor(
+ R"(
+#include "unchecked_optional_access_test.h"
+
+template
+using MyOptional = $ns::$optional;
+
+void target(MyOptional opt) {
+ opt.value();
+ /*[[check]]*/
+}
+ )",
+ UnorderedElementsAre(Pair("check", "unsafe: input.cc:8:7")));
+}
+
// FIXME: Add support for:
// - constructors (copy, move)
// - assignment operators (default, copy, move)
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -43,13 +43,14 @@
hasTemplateArgument(0, refersToType(type().bind("T";
}
-auto hasOptionalType() { return hasType(optionalClass()); }
-
-auto hasOptionalOrAliasType() {
+auto optionalOrAliasType() {
return hasUnqualifiedDesugaredType(
recordType(hasDeclaration(optionalClass(;
}
+/// Matches any of the spellings of the optional types and sugar, aliases, etc.
+auto hasOptionalType() { return hasType(optionalOrAliasType()); }
+
auto isOptionalMemberCallWithName(
llvm::StringRef MemberName,
llvm::Optional Ignorable = llvm::None) {
@@ -164,9 +165,8 @@
}
auto isCallReturningOptional() {
- return callExpr(callee(functionDecl(
- returns(anyOf(hasOptionalOrAliasType(),
-referenceType(pointee(hasOptionalOrAliasType(;
+ return callExpr(callee(functionDecl(returns(anyOf(
+ optionalOrAliasType(),
referenceType(pointee(optionalOrAliasType(;
}
/// Creates a symbolic value for an `optional` value using `HasValueVal` as the
@@ -485,8 +485,9 @@
return MatchSwitchBuilder()
// Attach a symbolic "has_value" state to optional values that we see for
// the first time.
- .CaseOf(expr(anyOf(declRefExpr(), memberExpr()),
hasOptionalType()),
-initializeOptionalReference)
+ .CaseOf(
+ expr(anyOf(declRefExpr(), memberExpr()), hasOptionalType()),
+ initializeOptionalReference)
// make_optional
.CaseOf(isMakeOptionalCall(), transferMakeOptionalCall)
Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2214,6 +2214,23 @@
UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:10:7")));
}
+// Verifies that the check sees through aliases.
+TEST_P(UncheckedOptionalAccessTest, WithAlias) {
+ ExpectLatticeChecksFor(
+ R"(
+#include "unchecked_optional_access_test.h"
+
+template
+using MyOptional = $ns::$optional;
+
+void target(MyOptional opt) {
+ opt.value();
+ /*[[check]]*/
+}
+ )",
+ UnorderedElementsAre(Pair("check", "unsafe: input.cc:8:7")));
+}
+
// FIXME: Add support for:
// - constructors (copy, move)
// - assignment operators (default, copy, move)
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -43,13 +43,14 @@
hasTemplateArgument(0, refersToType(type().bind("T";
}
-auto hasOptionalType() { return hasType(optionalClass()); }
-
-auto hasOptionalOrAliasType() {
+auto optionalOrAliasType() {
return hasUnqualifiedDesugaredType(
