[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC336995: [analyzer][UninitializedObjectChecker] Fixed
captured lambda variable name (authored by Szelethus, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D48291
Files:
lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
test/Analysis/cxx-uninitialized-object.cpp
Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
@@ -232,6 +232,10 @@
static void printNoteMessage(llvm::raw_ostream ,
const FieldChainInfo );
+/// Returns with Field's name. This is a helper function to get the correct name
+/// even if Field is a captured lambda variable.
+static StringRef getVariableName(const FieldDecl *Field);
+
//===--===//
// Methods for UninitializedObjectChecker.
//===--===//
@@ -581,30 +585,14 @@
// "uninitialized field 'this->x'", but we can't refer to 'x' directly,
// we need an explicit namespace resolution whether the uninit field was
// 'D1::x' or 'D2::x'.
-//
-// TODO: If a field in the fieldchain is a captured lambda parameter, this
-// function constructs an empty string for it:
-//
-// template struct A {
-// Callable c;
-// A(const Callable , int) : c(c) {}
-// };
-//
-// int b; // say that this isn't zero initialized
-// auto alwaysTrue = [](int a) { return true; };
-//
-// A call with these parameters: A::A(alwaysTrue, int())
-// will emit a note with the message "uninitialized field: 'this->c.'". If
-// possible, the lambda parameter name should be retrieved or be replaced with a
-// "" or something similar.
void FieldChainInfo::print(llvm::raw_ostream ) const {
if (Chain.isEmpty())
return;
const llvm::ImmutableListImpl *L =
Chain.getInternalPointer();
printTail(Out, L->getTail());
- Out << L->getHead()->getDecl()->getNameAsString();
+ Out << getVariableName(L->getHead()->getDecl());
}
void FieldChainInfo::printTail(
@@ -615,7 +603,7 @@
printTail(Out, L->getTail());
const FieldDecl *Field = L->getHead()->getDecl();
- Out << Field->getNameAsString();
+ Out << getVariableName(Field);
Out << (Field->getType()->isPointerType() ? "->" : ".");
}
@@ -676,6 +664,21 @@
Out << "'";
}
+static StringRef getVariableName(const FieldDecl *Field) {
+ // If Field is a captured lambda variable, Field->getName() will return with
+ // an empty string. We can however acquire it's name from the lambda's
+ // captures.
+ const auto *CXXParent = dyn_cast(Field->getParent());
+
+ if (CXXParent && CXXParent->isLambda()) {
+assert(CXXParent->captures_begin());
+auto It = CXXParent->captures_begin() + Field->getFieldIndex();
+return It->getCapturedVar()->getName();
+ }
+
+ return Field->getName();
+}
+
void ento::registerUninitializedObjectChecker(CheckerManager ) {
auto Chk = Mgr.registerChecker();
Chk->IsPedantic = Mgr.getAnalyzerOptions().getBooleanOption(
Index: test/Analysis/cxx-uninitialized-object.cpp
===
--- test/Analysis/cxx-uninitialized-object.cpp
+++ test/Analysis/cxx-uninitialized-object.cpp
@@ -737,6 +737,22 @@
//===--===//
template
+struct LambdaThisTest {
+ Callable functor;
+
+ LambdaThisTest(const Callable , int) : functor(functor) {
+// All good!
+ }
+};
+
+struct HasCapturableThis {
+ void fLambdaThisTest() {
+auto isEven = [this](int a) { return a % 2 == 0; }; // no-crash
+LambdaThisTest(isEven, int());
+ }
+};
+
+template
struct LambdaTest1 {
Callable functor;
@@ -760,7 +776,7 @@
void fLambdaTest2() {
int b;
- auto equals = [](int a) { return a == b; }; // expected-note{{uninitialized field 'this->functor.'}}
+ auto equals = [](int a) { return a == b; }; // expected-note{{uninitialized field 'this->functor.b'}}
LambdaTest2(equals, int());
}
#else
@@ -782,8 +798,8 @@
namespace LT3Detail {
struct RecordType {
- int x; // expected-note{{uninitialized field 'this->functor..x'}}
- int y; // expected-note{{uninitialized field 'this->functor..y'}}
+ int x; // expected-note{{uninitialized field 'this->functor.rec1.x'}}
+ int y; // expected-note{{uninitialized field 'this->functor.rec1.y'}}
};
} // namespace LT3Detail
@@ -826,6 +842,35 @@
}
#endif //PEDANTIC
+template
+struct MultipleLambdaCapturesTest1 {
+ Callable functor;
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ MultipleLambdaCapturesTest1(const Callable , int) : functor(functor)
[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name
Szelethus updated this revision to Diff 155361.
Szelethus marked an inline comment as done.
Szelethus added a comment.
Thanks! :)
Rebased to https://reviews.llvm.org/rC336994.
https://reviews.llvm.org/D48291
Files:
lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
test/Analysis/cxx-uninitialized-object.cpp
Index: test/Analysis/cxx-uninitialized-object.cpp
===
--- test/Analysis/cxx-uninitialized-object.cpp
+++ test/Analysis/cxx-uninitialized-object.cpp
@@ -737,6 +737,22 @@
//===--===//
template
+struct LambdaThisTest {
+ Callable functor;
+
+ LambdaThisTest(const Callable , int) : functor(functor) {
+// All good!
+ }
+};
+
+struct HasCapturableThis {
+ void fLambdaThisTest() {
+auto isEven = [this](int a) { return a % 2 == 0; }; // no-crash
+LambdaThisTest(isEven, int());
+ }
+};
+
+template
struct LambdaTest1 {
Callable functor;
@@ -760,7 +776,7 @@
void fLambdaTest2() {
int b;
- auto equals = [](int a) { return a == b; }; // expected-note{{uninitialized field 'this->functor.'}}
+ auto equals = [](int a) { return a == b; }; // expected-note{{uninitialized field 'this->functor.b'}}
LambdaTest2(equals, int());
}
#else
@@ -782,8 +798,8 @@
namespace LT3Detail {
struct RecordType {
- int x; // expected-note{{uninitialized field 'this->functor..x'}}
- int y; // expected-note{{uninitialized field 'this->functor..y'}}
+ int x; // expected-note{{uninitialized field 'this->functor.rec1.x'}}
+ int y; // expected-note{{uninitialized field 'this->functor.rec1.y'}}
};
} // namespace LT3Detail
@@ -826,6 +842,35 @@
}
#endif //PEDANTIC
+template
+struct MultipleLambdaCapturesTest1 {
+ Callable functor;
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ MultipleLambdaCapturesTest1(const Callable , int) : functor(functor) {} // expected-warning{{2 uninitialized field}}
+};
+
+void fMultipleLambdaCapturesTest1() {
+ int b1, b2 = 3, b3;
+ auto equals = [, , ](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized field 'this->functor.b1'}}
+ // expected-note@-1{{uninitialized field 'this->functor.b3'}}
+ MultipleLambdaCapturesTest1(equals, int());
+}
+
+template
+struct MultipleLambdaCapturesTest2 {
+ Callable functor;
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ MultipleLambdaCapturesTest2(const Callable , int) : functor(functor) {} // expected-warning{{1 uninitialized field}}
+};
+
+void fMultipleLambdaCapturesTest2() {
+ int b1, b2 = 3, b3;
+ auto equals = [b1, , ](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized field 'this->functor.b3'}}
+ MultipleLambdaCapturesTest2(equals, int());
+}
+
//===--===//
// System header tests.
//===--===//
Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
@@ -232,6 +232,10 @@
static void printNoteMessage(llvm::raw_ostream ,
const FieldChainInfo );
+/// Returns with Field's name. This is a helper function to get the correct name
+/// even if Field is a captured lambda variable.
+static StringRef getVariableName(const FieldDecl *Field);
+
//===--===//
// Methods for UninitializedObjectChecker.
//===--===//
@@ -581,30 +585,14 @@
// "uninitialized field 'this->x'", but we can't refer to 'x' directly,
// we need an explicit namespace resolution whether the uninit field was
// 'D1::x' or 'D2::x'.
-//
-// TODO: If a field in the fieldchain is a captured lambda parameter, this
-// function constructs an empty string for it:
-//
-// template struct A {
-// Callable c;
-// A(const Callable , int) : c(c) {}
-// };
-//
-// int b; // say that this isn't zero initialized
-// auto alwaysTrue = [](int a) { return true; };
-//
-// A call with these parameters: A::A(alwaysTrue, int())
-// will emit a note with the message "uninitialized field: 'this->c.'". If
-// possible, the lambda parameter name should be retrieved or be replaced with a
-// "" or something similar.
void FieldChainInfo::print(llvm::raw_ostream ) const {
if (Chain.isEmpty())
return;
const llvm::ImmutableListImpl *L =
Chain.getInternalPointer();
printTail(Out, L->getTail());
- Out << L->getHead()->getDecl()->getNameAsString();
+ Out << getVariableName(L->getHead()->getDecl());
}
void FieldChainInfo::printTail(
@@ -615,7 +603,7 @@
printTail(Out, L->getTail());
const FieldDecl *Field =
[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name
NoQ accepted this revision.
NoQ added a comment.
Looks good!
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:691
+StringRef getVariableName(const FieldDecl *Field) {
+ // If \p Field is a captured lambda variable, Field->getName() will return
Szelethus wrote:
> george.karpenkov wrote:
> > Is this a member method? Then it should be prefixed with a class name.
> > Currently it looks like you have a member method without an implementation
> > and a separate C-style function?
> It is a statically defined method, and you can see its forward declaration in
> the same diff.
>
> Back when I started writing this checker, the amount of function laying in
> the anonymous namespace was very few, but this has changed. I'll fix it
> sometime because it's getting somewhat annoying (and is against the [[
> https://llvm.org/docs/CodingStandards.html | coding standards ]]).
Yeah, just make them `static`, it's easier to read
(https://llvm.org/docs/CodingStandards.html#anonymous-namespaces).
https://reviews.llvm.org/D48291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name
Szelethus marked 3 inline comments as done.
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:691
+StringRef getVariableName(const FieldDecl *Field) {
+ // If \p Field is a captured lambda variable, Field->getName() will return
george.karpenkov wrote:
> Is this a member method? Then it should be prefixed with a class name.
> Currently it looks like you have a member method without an implementation
> and a separate C-style function?
It is a statically defined method, and you can see its forward declaration in
the same diff.
Back when I started writing this checker, the amount of function laying in the
anonymous namespace was very few, but this has changed. I'll fix it sometime
because it's getting somewhat annoying (and is against the [[
https://llvm.org/docs/CodingStandards.html | coding standards ]]).
https://reviews.llvm.org/D48291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name
Szelethus updated this revision to Diff 154420.
Szelethus added a comment.
Finding the correct captured variable now works with
`FieldDecl::getFieldIndex()`.
https://reviews.llvm.org/D48291
Files:
lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
test/Analysis/cxx-uninitialized-object.cpp
Index: test/Analysis/cxx-uninitialized-object.cpp
===
--- test/Analysis/cxx-uninitialized-object.cpp
+++ test/Analysis/cxx-uninitialized-object.cpp
@@ -737,6 +737,22 @@
//===--===//
template
+struct LambdaThisTest {
+ Callable functor;
+
+ LambdaThisTest(const Callable , int) : functor(functor) {
+// All good!
+ }
+};
+
+struct HasCapturableThis {
+ void fLambdaThisTest() {
+auto isEven = [this](int a) { return a % 2 == 0; }; // no-crash
+LambdaThisTest(isEven, int());
+ }
+};
+
+template
struct LambdaTest1 {
Callable functor;
@@ -760,7 +776,7 @@
void fLambdaTest2() {
int b;
- auto equals = [](int a) { return a == b; }; // expected-note{{uninitialized field 'this->functor.'}}
+ auto equals = [](int a) { return a == b; }; // expected-note{{uninitialized field 'this->functor.b'}}
LambdaTest2(equals, int());
}
#else
@@ -782,8 +798,8 @@
namespace LT3Detail {
struct RecordType {
- int x; // expected-note{{uninitialized field 'this->functor..x'}}
- int y; // expected-note{{uninitialized field 'this->functor..y'}}
+ int x; // expected-note{{uninitialized field 'this->functor.rec1.x'}}
+ int y; // expected-note{{uninitialized field 'this->functor.rec1.y'}}
};
} // namespace LT3Detail
@@ -826,6 +842,35 @@
}
#endif //PEDANTIC
+template
+struct MultipleLambdaCapturesTest1 {
+ Callable functor;
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ MultipleLambdaCapturesTest1(const Callable , int) : functor(functor) {} // expected-warning{{2 uninitialized field}}
+};
+
+void fMultipleLambdaCapturesTest1() {
+ int b1, b2 = 3, b3;
+ auto equals = [, , ](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized field 'this->functor.b1'}}
+ // expected-note@-1{{uninitialized field 'this->functor.b3'}}
+ MultipleLambdaCapturesTest1(equals, int());
+}
+
+template
+struct MultipleLambdaCapturesTest2 {
+ Callable functor;
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ MultipleLambdaCapturesTest2(const Callable , int) : functor(functor) {} // expected-warning{{1 uninitialized field}}
+};
+
+void fMultipleLambdaCapturesTest2() {
+ int b1, b2 = 3, b3;
+ auto equals = [b1, , ](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized field 'this->functor.b3'}}
+ MultipleLambdaCapturesTest2(equals, int());
+}
+
//===--===//
// System header tests.
//===--===//
Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
@@ -230,6 +230,10 @@
/// Constructs a note message for a given FieldChainInfo object.
void printNoteMessage(llvm::raw_ostream , const FieldChainInfo );
+/// Returns with Field's name. This is a helper function to get the correct name
+/// even if Field is a captured lambda variable.
+StringRef getVariableName(const FieldDecl *Field);
+
} // end of anonymous namespace
//===--===//
@@ -596,30 +600,14 @@
// "uninitialized field 'this->x'", but we can't refer to 'x' directly,
// we need an explicit namespace resolution whether the uninit field was
// 'D1::x' or 'D2::x'.
-//
-// TODO: If a field in the fieldchain is a captured lambda parameter, this
-// function constructs an empty string for it:
-//
-// template struct A {
-// Callable c;
-// A(const Callable , int) : c(c) {}
-// };
-//
-// int b; // say that this isn't zero initialized
-// auto alwaysTrue = [](int a) { return true; };
-//
-// A call with these parameters: A::A(alwaysTrue, int())
-// will emit a note with the message "uninitialized field: 'this->c.'". If
-// possible, the lambda parameter name should be retrieved or be replaced with a
-// "" or something similar.
void FieldChainInfo::print(llvm::raw_ostream ) const {
if (Chain.isEmpty())
return;
const llvm::ImmutableListImpl *L =
Chain.getInternalPointer();
printTail(Out, L->getTail());
- Out << L->getHead()->getDecl()->getNameAsString();
+ Out << getVariableName(L->getHead()->getDecl());
}
void FieldChainInfo::printTail(
@@ -630,7 +618,7 @@
printTail(Out, L->getTail());
const FieldDecl *Field = L->getHead()->getDecl();
- Out << Field->getNameAsString();
+ Out << getVariableName(Field);
Out <<
[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Looks like @NoQ had some valid comments as well. https://reviews.llvm.org/D48291 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name
george.karpenkov added a comment.
Few more nits.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:691
+StringRef getVariableName(const FieldDecl *Field) {
+ // If \p Field is a captured lambda variable, Field->getName() will return
Is this a member method? Then it should be prefixed with a class name.
Currently it looks like you have a member method without an implementation and
a separate C-style function?
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:692
+StringRef getVariableName(const FieldDecl *Field) {
+ // If \p Field is a captured lambda variable, Field->getName() will return
+ // with an empty string. We can however acquire it's name by iterating over
Doxygen-style comments in LLVM start with a triple slash, and are located above
the function. Then the tools for autocompletion (e.g. clangd) can pick them up.
https://reviews.llvm.org/D48291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:699
+for (const LambdaCapture : CXXParent->captures()) {
+ if (L.getLocation() == Field->getLocation())
+return L.getCapturedVar()->getName();
Uhm, this source location comparison looks really flaky. Ideally we would have
looked up by field and get the captured variable directly, but it seems that we
don't have a ready-made method for this.
The second ideal thing would probably be a reverse variant of
`CXXRecordDecl::getCaptureFields()`. Looking at how it's implemented, it seems
that capture indix is in sync with field index. Because we have
`FieldDecl::getFieldIndex()`, probably we could just lookup the captures array
by that index?
https://reviews.llvm.org/D48291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name
Szelethus updated this revision to Diff 152105.
Szelethus added a comment.
Fixes according to inline comments.
https://reviews.llvm.org/D48291
Files:
lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
test/Analysis/cxx-uninitialized-object.cpp
Index: test/Analysis/cxx-uninitialized-object.cpp
===
--- test/Analysis/cxx-uninitialized-object.cpp
+++ test/Analysis/cxx-uninitialized-object.cpp
@@ -737,6 +737,22 @@
//===--===//
template
+struct LambdaThisTest {
+ Callable functor;
+
+ LambdaThisTest(const Callable , int) : functor(functor) {
+// All good!
+ }
+};
+
+struct HasCapturableThis {
+ void fLambdaThisTest() {
+auto isEven = [this](int a) { return a % 2 == 0; }; // no-crash
+LambdaThisTest(isEven, int());
+ }
+};
+
+template
struct LambdaTest1 {
Callable functor;
@@ -760,7 +776,7 @@
void fLambdaTest2() {
int b;
- auto equals = [](int a) { return a == b; }; // expected-note{{uninitialized field 'this->functor.'}}
+ auto equals = [](int a) { return a == b; }; // expected-note{{uninitialized field 'this->functor.b'}}
LambdaTest2(equals, int());
}
#else
@@ -782,8 +798,8 @@
namespace LT3Detail {
struct RecordType {
- int x; // expected-note{{uninitialized field 'this->functor..x'}}
- int y; // expected-note{{uninitialized field 'this->functor..y'}}
+ int x; // expected-note{{uninitialized field 'this->functor.rec1.x'}}
+ int y; // expected-note{{uninitialized field 'this->functor.rec1.y'}}
};
} // namespace LT3Detail
@@ -826,6 +842,35 @@
}
#endif //PEDANTIC
+template
+struct MultipleLambdaCapturesTest1 {
+ Callable functor;
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ MultipleLambdaCapturesTest1(const Callable , int) : functor(functor) {} // expected-warning{{2 uninitialized field}}
+};
+
+void fMultipleLambdaCapturesTest1() {
+ int b1, b2 = 3, b3;
+ auto equals = [, , ](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized field 'this->functor.b1'}}
+ // expected-note@-1{{uninitialized field 'this->functor.b3'}}
+ MultipleLambdaCapturesTest1(equals, int());
+}
+
+template
+struct MultipleLambdaCapturesTest2 {
+ Callable functor;
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ MultipleLambdaCapturesTest2(const Callable , int) : functor(functor) {} // expected-warning{{1 uninitialized field}}
+};
+
+void fMultipleLambdaCapturesTest2() {
+ int b1, b2 = 3, b3;
+ auto equals = [b1, , ](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized field 'this->functor.b3'}}
+ MultipleLambdaCapturesTest2(equals, int());
+}
+
//===--===//
// System header tests.
//===--===//
Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
@@ -230,6 +230,10 @@
/// Constructs a note message for a given FieldChainInfo object.
void printNoteMessage(llvm::raw_ostream , const FieldChainInfo );
+/// Returns with Field's name. This is a helper function to get the correct name
+/// even if Field is a captured lambda variable.
+StringRef getVariableName(const FieldDecl *Field);
+
} // end of anonymous namespace
//===--===//
@@ -604,30 +608,14 @@
// "uninitialized field 'this->x'", but we can't refer to 'x' directly,
// we need an explicit namespace resolution whether the uninit field was
// 'D1::x' or 'D2::x'.
-//
-// TODO: If a field in the fieldchain is a captured lambda parameter, this
-// function constructs an empty string for it:
-//
-// template struct A {
-// Callable c;
-// A(const Callable , int) : c(c) {}
-// };
-//
-// int b; // say that this isn't zero initialized
-// auto alwaysTrue = [](int a) { return true; };
-//
-// A call with these parameters: A::A(alwaysTrue, int())
-// will emit a note with the message "uninitialized field: 'this->c.'". If
-// possible, the lambda parameter name should be retrieved or be replaced with a
-// "" or something similar.
void FieldChainInfo::print(llvm::raw_ostream ) const {
if (Chain.isEmpty())
return;
const llvm::ImmutableListImpl *L =
Chain.getInternalPointer();
printTail(Out, L->getTail());
- Out << L->getHead()->getDecl()->getNameAsString();
+ Out << getVariableName(L->getHead()->getDecl());
}
void FieldChainInfo::printTail(
@@ -638,7 +626,7 @@
printTail(Out, L->getTail());
const FieldDecl *Field = L->getHead()->getDecl();
- Out << Field->getNameAsString();
+ Out << getVariableName(Field);
Out << (Field->getType()->isPointerType() ? "->" : ".");
}
@@
[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name
george.karpenkov added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:685
+
+ if (CXXParent && CXXParent->isLambda()) {
+CXXRecordDecl::capture_const_iterator CapturedVar =
Szelethus wrote:
> george.karpenkov wrote:
> > CXXParent is guaranteed to be non-null at this stage, otherwise dyn_cast
> > fails
> I found this on
> http://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates:
>
> >dyn_cast<>:
> >
> >The dyn_cast<> operator is a “checking cast” operation. It checks to see
> > if the operand is of the specified type, and if so, returns a pointer to it
> > (this operator does not work with references). If the operand is not of the
> > correct type, a null pointer is returned.
>
> So I guess this should be alright.
Oops sorry my bad.
Repository:
rC Clang
https://reviews.llvm.org/D48291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:685
+
+ if (CXXParent && CXXParent->isLambda()) {
+CXXRecordDecl::capture_const_iterator CapturedVar =
george.karpenkov wrote:
> CXXParent is guaranteed to be non-null at this stage, otherwise dyn_cast fails
I found this on
http://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates:
>dyn_cast<>:
>
>The dyn_cast<> operator is a “checking cast” operation. It checks to see
> if the operand is of the specified type, and if so, returns a pointer to it
> (this operator does not work with references). If the operand is not of the
> correct type, a null pointer is returned.
So I guess this should be alright.
Repository:
rC Clang
https://reviews.llvm.org/D48291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name
Szelethus added a comment.
Thanks for the review!
I spend some time thinking about the support for lambda functions, and I start
to think that checking for lambda misuse shouldn't be the responsibility of
this checker. I created a new revision for that discussion, I wouldn't like to
abandon this one until I heard what you think of this! :)
https://reviews.llvm.org/D48318
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:687
+CXXRecordDecl::capture_const_iterator CapturedVar =
+std::find_if(CXXParent->captures_begin(), CXXParent->captures_end(),
+ [](const LambdaCapture ) {
george.karpenkov wrote:
> Could we just use a for-each here? Sorry for being opinionated, I'm pretty
> sure it would be both shorter and more readable.
Great idea! :) I completely agree, to be honest I scratched my head too while I
wrote it down.
Repository:
rC Clang
https://reviews.llvm.org/D48291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name
george.karpenkov requested changes to this revision.
george.karpenkov added a comment.
This revision now requires changes to proceed.
Looks good overall, but some nits inline.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:682
+StringRef getVariableName(const FieldDecl *Field) {
+ const auto *CXXParent = dyn_cast(Field->getParent());
Can we have a comment explaining the semantics? From my understanding, you find
a corresponding capture using a source location, and get a name from there.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:685
+
+ if (CXXParent && CXXParent->isLambda()) {
+CXXRecordDecl::capture_const_iterator CapturedVar =
CXXParent is guaranteed to be non-null at this stage, otherwise dyn_cast fails
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:687
+CXXRecordDecl::capture_const_iterator CapturedVar =
+std::find_if(CXXParent->captures_begin(), CXXParent->captures_end(),
+ [](const LambdaCapture ) {
Could we just use a for-each here? Sorry for being opinionated, I'm pretty sure
it would be both shorter and more readable.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:689
+ [](const LambdaCapture ) {
+ return Field->getLocation().getRawEncoding() ==
+ L.getLocation().getRawEncoding();
That's exactly the semantics of the equality operator on source locations, so
why not `Field->getLocation() == L.getLocation()` ?
Repository:
rC Clang
https://reviews.llvm.org/D48291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name
Szelethus created this revision.
Szelethus added reviewers: xazax.hun, george.karpenkov, NoQ, rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet,
whisperity.
Repository:
rC Clang
https://reviews.llvm.org/D48291
Files:
lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
test/Analysis/cxx-uninitialized-object.cpp
Index: test/Analysis/cxx-uninitialized-object.cpp
===
--- test/Analysis/cxx-uninitialized-object.cpp
+++ test/Analysis/cxx-uninitialized-object.cpp
@@ -737,6 +737,22 @@
//===--===//
template
+struct LambdaThisTest {
+ Callable functor;
+
+ LambdaThisTest(const Callable , int) : functor(functor) {
+// All good!
+ }
+};
+
+struct HasCapturableThis {
+ void fLambdaThisTest() {
+auto isEven = [this](int a) { return a % 2 == 0; }; // no-crash
+LambdaThisTest(isEven, int());
+ }
+};
+
+template
struct LambdaTest1 {
Callable functor;
@@ -760,7 +776,7 @@
void fLambdaTest2() {
int b;
- auto equals = [](int a) { return a == b; }; // expected-note{{uninitialized field 'this->functor.'}}
+ auto equals = [](int a) { return a == b; }; // expected-note{{uninitialized field 'this->functor.b'}}
LambdaTest2(equals, int());
}
#else
@@ -782,8 +798,8 @@
namespace LT3Detail {
struct RecordType {
- int x; // expected-note{{uninitialized field 'this->functor..x'}}
- int y; // expected-note{{uninitialized field 'this->functor..y'}}
+ int x; // expected-note{{uninitialized field 'this->functor.rec1.x'}}
+ int y; // expected-note{{uninitialized field 'this->functor.rec1.y'}}
};
} // namespace LT3Detail
@@ -826,6 +842,35 @@
}
#endif //PEDANTIC
+template
+struct MultipleLambdaCapturesTest1 {
+ Callable functor;
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ MultipleLambdaCapturesTest1(const Callable , int) : functor(functor) {} // expected-warning{{2 uninitialized field}}
+};
+
+void fMultipleLambdaCapturesTest1() {
+ int b1, b2 = 3, b3;
+ auto equals = [, , ](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized field 'this->functor.b1'}}
+ // expected-note@-1{{uninitialized field 'this->functor.b3'}}
+ MultipleLambdaCapturesTest1(equals, int());
+}
+
+template
+struct MultipleLambdaCapturesTest2 {
+ Callable functor;
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ MultipleLambdaCapturesTest2(const Callable , int) : functor(functor) {} // expected-warning{{1 uninitialized field}}
+};
+
+void fMultipleLambdaCapturesTest2() {
+ int b1, b2 = 3, b3;
+ auto equals = [b1, , ](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized field 'this->functor.b3'}}
+ MultipleLambdaCapturesTest2(equals, int());
+}
+
//===--===//
// System header tests.
//===--===//
Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
@@ -230,6 +230,10 @@
/// Constructs a note message for a given FieldChainInfo object.
void printNoteMessage(llvm::raw_ostream , const FieldChainInfo );
+/// Returns with Field's name. This is a helper function to get the correct name
+/// even if Field is a captured lambda variable.
+StringRef getVariableName(const FieldDecl *Field);
+
} // end of anonymous namespace
//===--===//
@@ -594,30 +598,15 @@
// "uninitialized field 'this->x'", but we can't refer to 'x' directly,
// we need an explicit namespace resolution whether the uninit field was
// 'D1::x' or 'D2::x'.
-//
-// TODO: If a field in the fieldchain is a captured lambda parameter, this
-// function constructs an empty string for it:
-//
-// template struct A {
-// Callable c;
-// A(const Callable , int) : c(c) {}
-// };
-//
-// int b; // say that this isn't zero initialized
-// auto alwaysTrue = [](int a) { return true; };
-//
-// A call with these parameters: A::A(alwaysTrue, int())
-// will emit a note with the message "uninitialized field: 'this->c.'". If
-// possible, the lambda parameter name should be retrieved or be replaced with a
-// "" or something similar.
void FieldChainInfo::print(llvm::raw_ostream ) const {
if (Chain.isEmpty())
return;
const llvm::ImmutableListImpl *L =
Chain.getInternalPointer();
printTail(Out, L->getTail());
- Out << L->getHead()->getDecl()->getNameAsString();
+
+ Out << getVariableName(L->getHead()->getDecl());
}
void FieldChainInfo::printTail(
@@ -628,7 +617,7 @@
printTail(Out, L->getTail());
const FieldDecl *Field = L->getHead()->getDecl();
- Out << Field->getNameAsString();
+
