[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.
shuaiwang updated this revision to Diff 166595.
shuaiwang marked 5 inline comments as done.
shuaiwang added a comment.
Addresses review comments.
Repository:
rC Clang
https://reviews.llvm.org/D52219
Files:
include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
lib/Analysis/ExprMutationAnalyzer.cpp
unittests/Analysis/ExprMutationAnalyzerTest.cpp
Index: unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -52,11 +52,21 @@
bool isMutated(const SmallVectorImpl &Results, ASTUnit *AST) {
const auto *const S = selectFirst("stmt", Results);
const auto *const E = selectFirst("expr", Results);
+ assert(S && E);
return ExprMutationAnalyzer(*S, AST->getASTContext()).isMutated(E);
}
+bool isPointeeMutated(const SmallVectorImpl &Results,
+ ASTUnit *AST) {
+ const auto *const S = selectFirst("stmt", Results);
+ const auto *const E = selectFirst("expr", Results);
+ assert(S && E);
+ return ExprMutationAnalyzer(*S, AST->getASTContext()).isPointeeMutated(E);
+}
+
SmallVector
mutatedBy(const SmallVectorImpl &Results, ASTUnit *AST) {
+ EXPECT_TRUE(isMutated(Results, AST));
const auto *const S = selectFirst("stmt", Results);
SmallVector Chain;
ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
@@ -71,6 +81,19 @@
return Chain;
}
+std::string pointeeMutatedBy(const SmallVectorImpl &Results,
+ ASTUnit *AST) {
+ EXPECT_TRUE(isPointeeMutated(Results, AST));
+ const auto *const S = selectFirst("stmt", Results);
+ const auto *const E = selectFirst("expr", Results);
+ ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
+ const Stmt *By = Analyzer.findPointeeMutation(E);
+ std::string buffer;
+ llvm::raw_string_ostream stream(buffer);
+ By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy());
+ return StringRef(stream.str()).trim().str();
+}
+
std::string removeSpace(std::string s) {
s.erase(std::remove_if(s.begin(), s.end(),
[](char c) { return std::isspace(c); }),
@@ -100,10 +123,14 @@
} // namespace
TEST(ExprMutationAnalyzerTest, Trivial) {
- const auto AST = buildASTFromCode("void f() { int x; x; }");
- const auto Results =
+ auto AST = buildASTFromCode("void f() { int x; x; }");
+ auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_FALSE(isMutated(Results, AST.get()));
+
+ AST = buildASTFromCode("void f() { const int x = 0; x; }");
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
}
class AssignmentTest : public ::testing::TestWithParam {};
@@ -134,41 +161,111 @@
Values("++x", "--x", "x++", "x--"), );
TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) {
- const auto AST = buildASTFromCode(
+ auto AST = buildASTFromCode(
"void f() { struct Foo { void mf(); }; Foo x; x.mf(); }");
- const auto Results =
+ auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+ EXPECT_FALSE(isPointeeMutated(Results, AST.get()));
+
+ AST = buildASTFromCode(
+ "void f() { struct Foo { void mf(); }; Foo *p; p->mf(); }");
+ Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_EQ(pointeeMutatedBy(Results, AST.get()), "p->mf()");
+
+ AST = buildASTFromCode(
+ "void f() { struct Foo { void mf(); }; Foo *x; Foo *&p = x; p->mf(); }");
+ Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_EQ(pointeeMutatedBy(Results, AST.get()), "p->mf()");
}
TEST(ExprMutationAnalyzerTest, AssumedNonConstMemberFunc) {
auto AST = buildASTFromCodeWithArgs(
"struct X { template void mf(); };"
- "template void f() { X x; x.mf(); }",
+ "template void f() { X x; x.mf(); }"
+ "template void g() { X *p; p->mf(); }",
{"-fno-delayed-template-parsing"});
auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+ Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_EQ(pointeeMutatedBy(Results, AST.get()), "p->mf()");
- AST = buildASTFromCodeWithArgs("template void f() { T x; x.mf(); }",
- {"-fno-delayed-template-parsing"});
+ AST =
+ buildASTFromCodeWithArgs("template void f() { T x; x.mf(); }"
+ "template void g() { T *p; p->mf(); }",
+ {"-fno-delayed-template-parsing"});
[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.
shuaiwang marked 10 inline comments as done.
shuaiwang added inline comments.
Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:86
+// - Pointer to non-const
+// - Pointer-like type with `operator*` returning non-const reference
+bool isPointeeMutable(const Expr *Exp, const ASTContext &Context) {
Szelethus wrote:
> Didn't you mean these to be doxygen comments?
I feel these don't need to be doxygen comments.
Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:156
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+
+ AST = tooling::buildASTFromCode(
JonasToth wrote:
> JonasToth wrote:
> > I feel that there a multiple tests missing:
> >
> > - multiple levels of pointers `int ***`, `int * const *`
> > - pointers to references `int &*`
> > - references to pointers `int *&`
> > - ensure that having a const pointer does no influence the pointee analysis
> > `int * const p = &i; *p = 42;`
> > - a class with `operator*` + `operator->` const/non-const and the analysis
> > for pointers to that class
> > - pointer returned from a function
> > - non-const reference returned
> > ```
> > int& foo(int *p) {
> > return *p;
> > }
> > ```
> >
> for the multi-level pointer mutation: it would be enough to test, that the
> second layer is analyzed properly, and that the `int * >const< *` would be
> detected.
Added except for:
- Anything that requires following a dereference, we need
`findPointeeDerefMutation` for that.
- Pointer to a class with `operator*` + `operator->`, I think those two
operators doesn't matter, there's no way to accidentally invoke them from a
pointer.
Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:686
+ AST = tooling::buildASTFromCode(
+ "void g(const int*); void f() { const int x[2] = {}; g(x); }");
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
JonasToth wrote:
> Please add a mutating example for array access through a pointer (as its very
> common in C-style code).
This also need `findPointeeDerefMutation`.
Repository:
rC Clang
https://reviews.llvm.org/D52219
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342794 - Update smart pointer detection for thread safety analysis.
Author: rtrieu
Date: Fri Sep 21 18:50:52 2018
New Revision: 342794
URL: http://llvm.org/viewvc/llvm-project?rev=342794&view=rev
Log:
Update smart pointer detection for thread safety analysis.
Objects are determined to be smart pointers if they have both a star and arrow
operator. Some implementations of smart pointers have these overloaded
operators in a base class, while the check only searched the derived class.
This fix will also look for the operators in the base class.
Modified:
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=342794&r1=342793&r2=342794&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Sep 21 18:50:52 2018
@@ -425,17 +425,36 @@ static bool isIntOrBool(Expr *Exp) {
// Check to see if the type is a smart pointer of some kind. We assume
// it's a smart pointer if it defines both operator-> and operator*.
static bool threadSafetyCheckIsSmartPointer(Sema &S, const RecordType* RT) {
- DeclContextLookupResult Res1 = RT->getDecl()->lookup(
- S.Context.DeclarationNames.getCXXOperatorName(OO_Star));
- if (Res1.empty())
-return false;
+ auto IsOverloadedOperatorPresent = [&S](const RecordDecl *Record,
+ OverloadedOperatorKind Op) {
+DeclContextLookupResult Result =
+Record->lookup(S.Context.DeclarationNames.getCXXOperatorName(Op));
+return !Result.empty();
+ };
+
+ const RecordDecl *Record = RT->getDecl();
+ bool foundStarOperator = IsOverloadedOperatorPresent(Record, OO_Star);
+ bool foundArrowOperator = IsOverloadedOperatorPresent(Record, OO_Arrow);
+ if (foundStarOperator && foundArrowOperator)
+return true;
- DeclContextLookupResult Res2 = RT->getDecl()->lookup(
- S.Context.DeclarationNames.getCXXOperatorName(OO_Arrow));
- if (Res2.empty())
+ const CXXRecordDecl *CXXRecord = dyn_cast(Record);
+ if (!CXXRecord)
return false;
- return true;
+ for (auto BaseSpecifier : CXXRecord->bases()) {
+if (!foundStarOperator)
+ foundStarOperator = IsOverloadedOperatorPresent(
+ BaseSpecifier.getType()->getAsRecordDecl(), OO_Star);
+if (!foundArrowOperator)
+ foundArrowOperator = IsOverloadedOperatorPresent(
+ BaseSpecifier.getType()->getAsRecordDecl(), OO_Arrow);
+ }
+
+ if (foundStarOperator && foundArrowOperator)
+return true;
+
+ return false;
}
/// Check if passed in Decl is a pointer type.
Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=342794&r1=342793&r2=342794&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Fri Sep 21 18:50:52
2018
@@ -5481,3 +5481,98 @@ void f() {
int &i = i; // expected-warning {{reference 'i' is not yet bound to a value
when used within its own initialization}}
}
}
+
+namespace Derived_Smart_Pointer {
+template
+class SmartPtr_Derived : public SmartPtr {};
+
+class Foo {
+public:
+ SmartPtr_Derived mu_;
+ int a GUARDED_BY(mu_);
+ int b GUARDED_BY(mu_.get());
+ int c GUARDED_BY(*mu_);
+
+ void Lock() EXCLUSIVE_LOCK_FUNCTION(mu_);
+ void Unlock() UNLOCK_FUNCTION(mu_);
+
+ void test0() {
+a = 1; // expected-warning {{writing variable 'a' requires holding mutex
'mu_' exclusively}}
+b = 1; // expected-warning {{writing variable 'b' requires holding mutex
'mu_' exclusively}}
+c = 1; // expected-warning {{writing variable 'c' requires holding mutex
'mu_' exclusively}}
+ }
+
+ void test1() {
+Lock();
+a = 1;
+b = 1;
+c = 1;
+Unlock();
+ }
+};
+
+class Bar {
+ SmartPtr_Derived foo;
+
+ void test0() {
+foo->a = 1;// expected-warning {{writing variable 'a' requires
holding mutex 'foo->mu_' exclusively}}
+(*foo).b = 1; // expected-warning {{writing variable 'b' requires
holding mutex 'foo->mu_' exclusively}}
+foo.get()->c = 1; // expected-warning {{writing variable 'c' requires
holding mutex 'foo->mu_' exclusively}}
+ }
+
+ void test1() {
+foo->Lock();
+foo->a = 1;
+foo->Unlock();
+
+foo->mu_->Lock();
+foo->b = 1;
+foo->mu_->Unlock();
+
+MutexLock lock(foo->mu_.get());
+foo->c = 1;
+ }
+};
+
+class PointerGuard {
+ Mutex mu1;
+ Mutex mu2;
+ SmartPtr_Derived i GUARDED_BY(mu1) PT_GUARDED_BY(mu2);
+
+ void test0() {
+i.get(); // expected-warning {{reading variable 'i' requires holding
mutex 'mu1'}}
+*i = 2; // expected-warning {{reading variable 'i' requires holding
mutex 'mu1'}} \
+ // expected-warni
[PATCH] D51867: [Diagnostics] Add error handling to FormatDiagnostic()
vsapsai added a comment. It seems like there are too many asserts and they are too specific, they seem to be aimed at specific potential bugs. What about asserts that make sure we maintain some invariants? For example, check `DiagStr < DiagEnd` once in a loop instead of every place we increment `DiagStr`. Do you think it should catch the same problems but maybe a little bit later? My suggestion is based on assumption that `FormatDiagnostic` works only with predefined format strings and not with strings provided by compiler users (arguments can be provided by users but not format strings). I won't be able for some time to check the review again, so it's OK not to wait for my approval. Repository: rC Clang https://reviews.llvm.org/D51867 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51789: [clang] Add the exclude_from_explicit_instantiation attribute
ldionne updated this revision to Diff 166590.
ldionne added a comment.
Fix warnings on undefined entities
Repository:
rC Clang
https://reviews.llvm.org/D51789
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/CodeGenCXX/attr-exclude_from_explicit_instantiation.dont_assume_extern_instantiation.cpp
clang/test/Misc/pragma-attribute-supported-attributes-list.test
clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.diagnose_on_undefined_entity.cpp
clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.explicit_instantiation.cpp
clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.extern_declaration.cpp
clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.merge_redeclarations.cpp
Index: clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.merge_redeclarations.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.merge_redeclarations.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// Test that we properly merge the exclude_from_explicit_instantiation
+// attribute on redeclarations.
+
+#define EXCLUDE_FROM_EXPLICIT_INSTANTIATION __attribute__((exclude_from_explicit_instantiation))
+
+template
+struct Foo {
+ // Declaration without the attribute, definition with the attribute.
+ void func1();
+
+ // Declaration with the attribute, definition without the attribute.
+ EXCLUDE_FROM_EXPLICIT_INSTANTIATION void func2();
+
+ // Declaration with the attribute, definition with the attribute.
+ EXCLUDE_FROM_EXPLICIT_INSTANTIATION void func3();
+};
+
+template
+EXCLUDE_FROM_EXPLICIT_INSTANTIATION void Foo::func1() {
+ using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template
+void Foo::func2() {
+ using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template
+EXCLUDE_FROM_EXPLICIT_INSTANTIATION void Foo::func3() {
+ using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+struct Empty { };
+extern template struct Foo;
+
+int main() {
+ Foo foo;
+ foo.func1(); // expected-note{{in instantiation of}}
+ foo.func2(); // expected-note{{in instantiation of}}
+ foo.func3(); // expected-note{{in instantiation of}}
+}
Index: clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.extern_declaration.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-exclude_from_explicit_instantiation.extern_declaration.cpp
@@ -0,0 +1,69 @@
+// RUN: %clang_cc1 -Wno-unused-local-typedef -fsyntax-only -verify %s
+
+// Test that extern instantiation declarations cause members marked with
+// the exclude_from_explicit_instantiation attribute to be instantiated in
+// the current TU.
+
+#define EXCLUDE_FROM_EXPLICIT_INSTANTIATION __attribute__((exclude_from_explicit_instantiation))
+
+template
+struct Foo {
+ EXCLUDE_FROM_EXPLICIT_INSTANTIATION inline void non_static_member_function1();
+
+ EXCLUDE_FROM_EXPLICIT_INSTANTIATION void non_static_member_function2();
+
+ EXCLUDE_FROM_EXPLICIT_INSTANTIATION static inline void static_member_function1();
+
+ EXCLUDE_FROM_EXPLICIT_INSTANTIATION static void static_member_function2();
+
+ EXCLUDE_FROM_EXPLICIT_INSTANTIATION static int static_data_member;
+
+ struct EXCLUDE_FROM_EXPLICIT_INSTANTIATION member_class1 {
+static void static_member_function() {
+ using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+ };
+
+ struct member_class2 {
+EXCLUDE_FROM_EXPLICIT_INSTANTIATION static void static_member_function() {
+ using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+ };
+};
+
+template
+inline void Foo::non_static_member_function1() {
+ using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template
+void Foo::non_static_member_function2() {
+ using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template
+inline void Foo::static_member_function1() {
+ using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template
+void Foo::static_member_function2() {
+ using Fail = typename T::invalid; // expected-error{{no type named 'invalid' in 'Empty'}}
+}
+
+template
+int Foo::static_data_member = T::invalid; // expected-error{{no member named 'invalid' in 'Empty'}}
+
+struct Empty { };
+extern template struct Foo;
+
+int main() {
+ Foo foo;
+ foo.non_static_member_function1(); // expected-note{{in instantiation of}}
+ foo.non_static_member_function2(); // expected-note{{in instantiation of}}
+ Foo::static_memb
Re: r342793 - [Lexer] Add udefined_behavior_sanitizer feature
My bad. I saw that Vitaly accepted it and thought it would be ok since I got 2 LGTMs. On Fri, Sep 21, 2018 at 6:21 PM Aaron Ballman wrote: > > The reviewer asked you to wait a day so that others might have a > chance to review it, so this commit seems premature. I have no > technical concerns with the patch, but the sanitizer owners should > have had a chance to weigh in. That said, I don't see value in > reverting and recommitting later, so if there are concerns, they can > be dealt with post commit. > > ~Aaron > > On Fri, Sep 21, 2018 at 9:03 PM, Leonard Chan via cfe-commits > wrote: > > Author: leonardchan > > Date: Fri Sep 21 18:03:16 2018 > > New Revision: 342793 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=342793&view=rev > > Log: > > [Lexer] Add udefined_behavior_sanitizer feature > > > > This can be used to detect whether the code is being built with UBSan using > > the __has_feature(undefined_behavior_sanitizer) predicate. > > > > Differential Revision: https://reviews.llvm.org/D52386 > > > > Added: > > cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp > > Modified: > > cfe/trunk/include/clang/Basic/Features.def > > > > Modified: cfe/trunk/include/clang/Basic/Features.def > > URL: > > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Features.def?rev=342793&r1=342792&r2=342793&view=diff > > == > > --- cfe/trunk/include/clang/Basic/Features.def (original) > > +++ cfe/trunk/include/clang/Basic/Features.def Fri Sep 21 18:03:16 2018 > > @@ -38,6 +38,8 @@ FEATURE(hwaddress_sanitizer, > > LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress | > > SanitizerKind::KernelHWAddress)) > > FEATURE(xray_instrument, LangOpts.XRayInstrument) > > +FEATURE(undefined_behavior_sanitizer, > > +LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined)) > > FEATURE(assume_nonnull, true) > > FEATURE(attribute_analyzer_noreturn, true) > > FEATURE(attribute_availability, true) > > > > Added: cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp > > URL: > > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp?rev=342793&view=auto > > == > > --- cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp > > (added) > > +++ cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp Fri > > Sep 21 18:03:16 2018 > > @@ -0,0 +1,13 @@ > > +// RUN: %clang -E -fsanitize=undefined %s -o - | FileCheck > > --check-prefix=CHECK-UBSAN %s > > +// RUN: %clang -E -fsanitize=alignment %s -o - | FileCheck > > --check-prefix=CHECK-ALIGNMENT %s > > +// RUN: %clang -E %s -o - | FileCheck --check-prefix=CHECK-NO-UBSAN %s > > + > > +#if __has_feature(undefined_behavior_sanitizer) > > +int UBSanEnabled(); > > +#else > > +int UBSanDisabled(); > > +#endif > > + > > +// CHECK-UBSAN: UBSanEnabled > > +// CHECK-ALIGNMENT: UBSanEnabled > > +// CHECK-NO-UBSAN: UBSanDisabled > > > > > > ___ > > cfe-commits mailing list > > cfe-commits@lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r342793 - [Lexer] Add udefined_behavior_sanitizer feature
The reviewer asked you to wait a day so that others might have a chance to review it, so this commit seems premature. I have no technical concerns with the patch, but the sanitizer owners should have had a chance to weigh in. That said, I don't see value in reverting and recommitting later, so if there are concerns, they can be dealt with post commit. ~Aaron On Fri, Sep 21, 2018 at 9:03 PM, Leonard Chan via cfe-commits wrote: > Author: leonardchan > Date: Fri Sep 21 18:03:16 2018 > New Revision: 342793 > > URL: http://llvm.org/viewvc/llvm-project?rev=342793&view=rev > Log: > [Lexer] Add udefined_behavior_sanitizer feature > > This can be used to detect whether the code is being built with UBSan using > the __has_feature(undefined_behavior_sanitizer) predicate. > > Differential Revision: https://reviews.llvm.org/D52386 > > Added: > cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp > Modified: > cfe/trunk/include/clang/Basic/Features.def > > Modified: cfe/trunk/include/clang/Basic/Features.def > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Features.def?rev=342793&r1=342792&r2=342793&view=diff > == > --- cfe/trunk/include/clang/Basic/Features.def (original) > +++ cfe/trunk/include/clang/Basic/Features.def Fri Sep 21 18:03:16 2018 > @@ -38,6 +38,8 @@ FEATURE(hwaddress_sanitizer, > LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress | > SanitizerKind::KernelHWAddress)) > FEATURE(xray_instrument, LangOpts.XRayInstrument) > +FEATURE(undefined_behavior_sanitizer, > +LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined)) > FEATURE(assume_nonnull, true) > FEATURE(attribute_analyzer_noreturn, true) > FEATURE(attribute_availability, true) > > Added: cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp?rev=342793&view=auto > == > --- cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp (added) > +++ cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp Fri Sep > 21 18:03:16 2018 > @@ -0,0 +1,13 @@ > +// RUN: %clang -E -fsanitize=undefined %s -o - | FileCheck > --check-prefix=CHECK-UBSAN %s > +// RUN: %clang -E -fsanitize=alignment %s -o - | FileCheck > --check-prefix=CHECK-ALIGNMENT %s > +// RUN: %clang -E %s -o - | FileCheck --check-prefix=CHECK-NO-UBSAN %s > + > +#if __has_feature(undefined_behavior_sanitizer) > +int UBSanEnabled(); > +#else > +int UBSanDisabled(); > +#endif > + > +// CHECK-UBSAN: UBSanEnabled > +// CHECK-ALIGNMENT: UBSanEnabled > +// CHECK-NO-UBSAN: UBSanDisabled > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature
This revision was automatically updated to reflect the committed changes. Closed by commit rC342793: [Lexer] Add udefined_behavior_sanitizer feature (authored by leonardchan, committed by ). Repository: rL LLVM https://reviews.llvm.org/D52386 Files: include/clang/Basic/Features.def test/Lexer/has_feature_undefined_behavior_sanitizer.cpp Index: include/clang/Basic/Features.def === --- include/clang/Basic/Features.def +++ include/clang/Basic/Features.def @@ -38,6 +38,8 @@ LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress | SanitizerKind::KernelHWAddress)) FEATURE(xray_instrument, LangOpts.XRayInstrument) +FEATURE(undefined_behavior_sanitizer, +LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined)) FEATURE(assume_nonnull, true) FEATURE(attribute_analyzer_noreturn, true) FEATURE(attribute_availability, true) Index: test/Lexer/has_feature_undefined_behavior_sanitizer.cpp === --- test/Lexer/has_feature_undefined_behavior_sanitizer.cpp +++ test/Lexer/has_feature_undefined_behavior_sanitizer.cpp @@ -0,0 +1,13 @@ +// RUN: %clang -E -fsanitize=undefined %s -o - | FileCheck --check-prefix=CHECK-UBSAN %s +// RUN: %clang -E -fsanitize=alignment %s -o - | FileCheck --check-prefix=CHECK-ALIGNMENT %s +// RUN: %clang -E %s -o - | FileCheck --check-prefix=CHECK-NO-UBSAN %s + +#if __has_feature(undefined_behavior_sanitizer) +int UBSanEnabled(); +#else +int UBSanDisabled(); +#endif + +// CHECK-UBSAN: UBSanEnabled +// CHECK-ALIGNMENT: UBSanEnabled +// CHECK-NO-UBSAN: UBSanDisabled Index: include/clang/Basic/Features.def === --- include/clang/Basic/Features.def +++ include/clang/Basic/Features.def @@ -38,6 +38,8 @@ LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress | SanitizerKind::KernelHWAddress)) FEATURE(xray_instrument, LangOpts.XRayInstrument) +FEATURE(undefined_behavior_sanitizer, +LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined)) FEATURE(assume_nonnull, true) FEATURE(attribute_analyzer_noreturn, true) FEATURE(attribute_availability, true) Index: test/Lexer/has_feature_undefined_behavior_sanitizer.cpp === --- test/Lexer/has_feature_undefined_behavior_sanitizer.cpp +++ test/Lexer/has_feature_undefined_behavior_sanitizer.cpp @@ -0,0 +1,13 @@ +// RUN: %clang -E -fsanitize=undefined %s -o - | FileCheck --check-prefix=CHECK-UBSAN %s +// RUN: %clang -E -fsanitize=alignment %s -o - | FileCheck --check-prefix=CHECK-ALIGNMENT %s +// RUN: %clang -E %s -o - | FileCheck --check-prefix=CHECK-NO-UBSAN %s + +#if __has_feature(undefined_behavior_sanitizer) +int UBSanEnabled(); +#else +int UBSanDisabled(); +#endif + +// CHECK-UBSAN: UBSanEnabled +// CHECK-ALIGNMENT: UBSanEnabled +// CHECK-NO-UBSAN: UBSanDisabled ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342793 - [Lexer] Add udefined_behavior_sanitizer feature
Author: leonardchan Date: Fri Sep 21 18:03:16 2018 New Revision: 342793 URL: http://llvm.org/viewvc/llvm-project?rev=342793&view=rev Log: [Lexer] Add udefined_behavior_sanitizer feature This can be used to detect whether the code is being built with UBSan using the __has_feature(undefined_behavior_sanitizer) predicate. Differential Revision: https://reviews.llvm.org/D52386 Added: cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp Modified: cfe/trunk/include/clang/Basic/Features.def Modified: cfe/trunk/include/clang/Basic/Features.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Features.def?rev=342793&r1=342792&r2=342793&view=diff == --- cfe/trunk/include/clang/Basic/Features.def (original) +++ cfe/trunk/include/clang/Basic/Features.def Fri Sep 21 18:03:16 2018 @@ -38,6 +38,8 @@ FEATURE(hwaddress_sanitizer, LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress | SanitizerKind::KernelHWAddress)) FEATURE(xray_instrument, LangOpts.XRayInstrument) +FEATURE(undefined_behavior_sanitizer, +LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined)) FEATURE(assume_nonnull, true) FEATURE(attribute_analyzer_noreturn, true) FEATURE(attribute_availability, true) Added: cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp?rev=342793&view=auto == --- cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp (added) +++ cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp Fri Sep 21 18:03:16 2018 @@ -0,0 +1,13 @@ +// RUN: %clang -E -fsanitize=undefined %s -o - | FileCheck --check-prefix=CHECK-UBSAN %s +// RUN: %clang -E -fsanitize=alignment %s -o - | FileCheck --check-prefix=CHECK-ALIGNMENT %s +// RUN: %clang -E %s -o - | FileCheck --check-prefix=CHECK-NO-UBSAN %s + +#if __has_feature(undefined_behavior_sanitizer) +int UBSanEnabled(); +#else +int UBSanDisabled(); +#endif + +// CHECK-UBSAN: UBSanEnabled +// CHECK-ALIGNMENT: UBSanEnabled +// CHECK-NO-UBSAN: UBSanDisabled ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature
This revision was automatically updated to reflect the committed changes. Closed by commit rL342793: [Lexer] Add udefined_behavior_sanitizer feature (authored by leonardchan, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52386?vs=166585&id=166587#toc Repository: rL LLVM https://reviews.llvm.org/D52386 Files: cfe/trunk/include/clang/Basic/Features.def cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp Index: cfe/trunk/include/clang/Basic/Features.def === --- cfe/trunk/include/clang/Basic/Features.def +++ cfe/trunk/include/clang/Basic/Features.def @@ -38,6 +38,8 @@ LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress | SanitizerKind::KernelHWAddress)) FEATURE(xray_instrument, LangOpts.XRayInstrument) +FEATURE(undefined_behavior_sanitizer, +LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined)) FEATURE(assume_nonnull, true) FEATURE(attribute_analyzer_noreturn, true) FEATURE(attribute_availability, true) Index: cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp === --- cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp +++ cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp @@ -0,0 +1,13 @@ +// RUN: %clang -E -fsanitize=undefined %s -o - | FileCheck --check-prefix=CHECK-UBSAN %s +// RUN: %clang -E -fsanitize=alignment %s -o - | FileCheck --check-prefix=CHECK-ALIGNMENT %s +// RUN: %clang -E %s -o - | FileCheck --check-prefix=CHECK-NO-UBSAN %s + +#if __has_feature(undefined_behavior_sanitizer) +int UBSanEnabled(); +#else +int UBSanDisabled(); +#endif + +// CHECK-UBSAN: UBSanEnabled +// CHECK-ALIGNMENT: UBSanEnabled +// CHECK-NO-UBSAN: UBSanDisabled Index: cfe/trunk/include/clang/Basic/Features.def === --- cfe/trunk/include/clang/Basic/Features.def +++ cfe/trunk/include/clang/Basic/Features.def @@ -38,6 +38,8 @@ LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress | SanitizerKind::KernelHWAddress)) FEATURE(xray_instrument, LangOpts.XRayInstrument) +FEATURE(undefined_behavior_sanitizer, +LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined)) FEATURE(assume_nonnull, true) FEATURE(attribute_analyzer_noreturn, true) FEATURE(attribute_availability, true) Index: cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp === --- cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp +++ cfe/trunk/test/Lexer/has_feature_undefined_behavior_sanitizer.cpp @@ -0,0 +1,13 @@ +// RUN: %clang -E -fsanitize=undefined %s -o - | FileCheck --check-prefix=CHECK-UBSAN %s +// RUN: %clang -E -fsanitize=alignment %s -o - | FileCheck --check-prefix=CHECK-ALIGNMENT %s +// RUN: %clang -E %s -o - | FileCheck --check-prefix=CHECK-NO-UBSAN %s + +#if __has_feature(undefined_behavior_sanitizer) +int UBSanEnabled(); +#else +int UBSanDisabled(); +#endif + +// CHECK-UBSAN: UBSanEnabled +// CHECK-ALIGNMENT: UBSanEnabled +// CHECK-NO-UBSAN: UBSanDisabled ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature
phosek accepted this revision. phosek added a comment. This revision is now accepted and ready to land. LGTM but you should probably a wait a day to see if sanitizer owners are fine with with this as well. Repository: rC Clang https://reviews.llvm.org/D52386 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52386: [Lexer] Add udefined_behavior_sanitizer feature
leonardchan created this revision. leonardchan added reviewers: phosek, kcc, echristo, vitalybuka, morehouse. leonardchan added a project: clang. This can be used to detect whether the code is being built with UBSan using the __has_feature(undefined_behavior_sanitizer) predicate. Repository: rC Clang https://reviews.llvm.org/D52386 Files: include/clang/Basic/Features.def test/Lexer/has_feature_undefined_behavior_sanitizer.cpp Index: test/Lexer/has_feature_undefined_behavior_sanitizer.cpp === --- /dev/null +++ test/Lexer/has_feature_undefined_behavior_sanitizer.cpp @@ -0,0 +1,13 @@ +// RUN: %clang -E -fsanitize=undefined %s -o - | FileCheck --check-prefix=CHECK-UBSAN %s +// RUN: %clang -E -fsanitize=alignment %s -o - | FileCheck --check-prefix=CHECK-ALIGNMENT %s +// RUN: %clang -E %s -o - | FileCheck --check-prefix=CHECK-NO-UBSAN %s + +#if __has_feature(undefined_behavior_sanitizer) +int UBSanEnabled(); +#else +int UBSanDisabled(); +#endif + +// CHECK-UBSAN: UBSanEnabled +// CHECK-ALIGNMENT: UBSanEnabled +// CHECK-NO-UBSAN: UBSanDisabled Index: include/clang/Basic/Features.def === --- include/clang/Basic/Features.def +++ include/clang/Basic/Features.def @@ -38,6 +38,8 @@ LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress | SanitizerKind::KernelHWAddress)) FEATURE(xray_instrument, LangOpts.XRayInstrument) +FEATURE(undefined_behavior_sanitizer, +LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined)) FEATURE(assume_nonnull, true) FEATURE(attribute_analyzer_noreturn, true) FEATURE(attribute_availability, true) Index: test/Lexer/has_feature_undefined_behavior_sanitizer.cpp === --- /dev/null +++ test/Lexer/has_feature_undefined_behavior_sanitizer.cpp @@ -0,0 +1,13 @@ +// RUN: %clang -E -fsanitize=undefined %s -o - | FileCheck --check-prefix=CHECK-UBSAN %s +// RUN: %clang -E -fsanitize=alignment %s -o - | FileCheck --check-prefix=CHECK-ALIGNMENT %s +// RUN: %clang -E %s -o - | FileCheck --check-prefix=CHECK-NO-UBSAN %s + +#if __has_feature(undefined_behavior_sanitizer) +int UBSanEnabled(); +#else +int UBSanDisabled(); +#endif + +// CHECK-UBSAN: UBSanEnabled +// CHECK-ALIGNMENT: UBSanEnabled +// CHECK-NO-UBSAN: UBSanDisabled Index: include/clang/Basic/Features.def === --- include/clang/Basic/Features.def +++ include/clang/Basic/Features.def @@ -38,6 +38,8 @@ LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress | SanitizerKind::KernelHWAddress)) FEATURE(xray_instrument, LangOpts.XRayInstrument) +FEATURE(undefined_behavior_sanitizer, +LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined)) FEATURE(assume_nonnull, true) FEATURE(attribute_analyzer_noreturn, true) FEATURE(attribute_availability, true) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51809: [CUDA][HIP] Fix assertion in LookupSpecialMember
jlebar requested changes to this revision.
jlebar added subscribers: timshen, rsmith.
jlebar added a comment.
This revision now requires changes to proceed.
Sorry for missing tra's ping earlier, I get a lot of HIP email traffic that's
99% inactionable by me, so I didn't notice my username in tra's earlier email.
@tra, @timshen, and I debugged this IRL for a few hours this afternoon. The
result of this is that we don't think the fix in this patch is correct.
Here's what we think is happening.
When clang sees `using A::A` inside of `B`, it has to check whether this
constructor is legal in `B`. An example of where this constructor would *not*
be legal is something like:
struct NoDefaultConstructor { NoDefaultConstructor() = delete; };
struct A { A(const int& x) {} }
struct B {
using A::A;
NoDefaultConstructor c;
};
The reason this `using A::A` is not legal here is because the `using` statement
is equivalent to writing
B(const int& x) : A(x) {}
but this constructor is not legal, because `NoDefaultConstructor` is not
default-constructible, and a constructor for `B` must explicitly initialize all
non-default-initializable members.
Here is the code that checks whether the `using` statement is legal:
https://github.com/llvm-project/llvm-project-20170507/blob/51b65eeeab0d24268783d6246fd949d9a16e10e8/clang/lib/Sema/SemaDeclCXX.cpp#L11018
This code is kind of a lie! `DerivedCtor` is the constructor `B(const int& x)
: A(x) {}` that we've created in response to the `using` declaration. Notice
that it's not a default constructor! In fact, it's not even a special member
(i.e. it's not a default constructor, copy constructor, move constructor,
etc.). But notice that we pass `CXXDefaultConstructor`, and we call the
function `ShouldDeleteSpecialMember`!
The reason we don't tell the truth here seems to be out of convenience. To
determine whether we should delete the new constructor on `B`, it seems like we
are trying to ask: Would a default constructor on `B` be legal, ignoring the
fact that `A` has to be explicitly initialized? That is, the new constructor
we're creating is just like a default constructor on `B`, except that first it
initializes `A`. So we're trying to reuse the default constructor logic.
But eventually our tricks and dishonesty catch up to us, here in CUDA code.
This patch fixes one instance where we do the wrong thing and hit an assertion,
but who knows if the code is right in general; simply adding on another layer
of hack does not seem like the right approach to us.
cc @rsmith
https://reviews.llvm.org/D51809
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52136: [clang-tidy] Add modernize-concat-nested-namespaces check
aaron.ballman added inline comments.
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:31
+static bool singleNamedNamespaceChild(const NamespaceDecl &ND) {
+ const NamespaceDecl::decl_range Decls = ND.decls();
+ if (std::distance(Decls.begin(), Decls.end()) != 1)
We usually only const-qualify local declarations when they're
pointers/references, so you can drop the `const` here (and in several other
places). It's not a hard and fast rule, but the clutter is not useful in such
small functions.
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:50
+
+auto ConcatNestedNamespacesCheck::concatNamespaces() -> NamespaceString {
+ NamespaceString Result("namespace ");
I'm not keen on the trailing return type used here. It's reasonable, but clever
in ways we don't use elsewhere.
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:52
+const NamespaceContextVec &Namespaces) {
+ std::ostringstream Result;
+ bool First = true;
wgml wrote:
> aaron.ballman wrote:
> > Can this be rewritten with `llvm::for_each()` and a `Twine` so that we
> > don't have to use `ostringstream` (which is a big hammer for this).
> The main advantage of `stringstream` was that it is concise.
>
> I don't think I can effectively use `Twine` to build a result in a loop. If
> I'm wrong, correct me, please.
>
> I reworked `concatNamespaces` to use `SmallString` with another educated
> guess of `40` for capacity.
The `Twine` idea I was thinking of is not too far off from what you have with
`SmallString`, but perhaps is too clever:
```
return std::accumulate(Namespaces.begin(), Namespaces.end(), llvm::Twine(),
[](llvm::Twine Ret, const NamespaceDecl *ND) {
return Ret + "::" + ND->getName();
}).str();
```
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.h:29
+private:
+ using NamespaceContextVec = llvm::SmallVector;
+
wgml wrote:
> aaron.ballman wrote:
> > Why 6?
> Educated guess. I considered the codebases I usually work with and it seemed
> a sane value in that context.
Okay, I can live with that. Thanks!
https://reviews.llvm.org/D52136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47687: [Sema] Missing -Wlogical-op-parentheses warnings in macros (PR18971)
Higuoxing updated this revision to Diff 166583.
Higuoxing added a comment.
I update the *SuggestParentheses* function, now, it could deal with parentheses
inside macros
https://reviews.llvm.org/D47687
Files:
lib/Sema/SemaExpr.cpp
test/Sema/parentheses.c
Index: test/Sema/parentheses.c
===
--- test/Sema/parentheses.c
+++ test/Sema/parentheses.c
@@ -14,7 +14,11 @@
if ((i = 4)) {}
}
-void bitwise_rel(unsigned i) {
+#define VOID_CAST(cond) ( (void)(cond) )
+#define APPLY_OPS(op0, op1, x, y, z) ( VOID_CAST(x op0 y op1 z) )
+#define APPLY_OPS_DIRECTLY(op0, op1, x, y, z) ( (void)(x op0 y op1 z) )
+
+void bitwise_rel(unsigned i, unsigned ) {
(void)(i & 0x2 == 0); // expected-warning {{& has lower precedence than ==}} \
// expected-note{{place parentheses around the '==' expression to silence this warning}} \
// expected-note{{place parentheses around the & expression to evaluate it first}}
@@ -96,6 +100,31 @@
(void)(i && i || 0); // no warning.
(void)(0 || i && i); // no warning.
+
+ VOID_CAST(i && i || i); // expected-warning {{'&&' within '||'}} \
+// expected-note {{place parentheses around the '&&' expression to silence this warning}}
+
+ APPLY_OPS_DIRECTLY(&&, ||, i, i, i); // no warning.
+ APPLY_OPS(&&, ||, i, i, i); // no warning.
+
+ APPLY_OPS_DIRECTLY(&&, ||, i, i, i || i && i); // expected-warning {{'&&' within '||'}} \
+ // expected-note {{place parentheses around the '&&' expression to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:41-[[@LINE-2]]:41}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:47-[[@LINE-3]]:47}:")"
+
+ APPLY_OPS_DIRECTLY(&&, ||, i, i, i || i && ); // expected-warning {{'&&' within '||'}} \
+ // expected-note {{place parentheses around the '&&' expression to silence this warning}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:41-[[@LINE-2]]:41}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:50-[[@LINE-3]]:50}:")"
+
+ APPLY_OPS(&&, ||, i, i, i || i && i);// no warning.
+
+ APPLY_OPS_DIRECTLY(&&, ||, i, i, i && i); // no warning.
+ APPLY_OPS_DIRECTLY(&&, ||, i, i, i || i); // no warning.
+
+ APPLY_OPS(&&, ||, i, i, i && i); // no warning.
+ APPLY_OPS(&&, ||, i, i, i || i); // no warning.
+
}
_Bool someConditionFunc();
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -7103,9 +7103,11 @@
static void SuggestParentheses(Sema &Self, SourceLocation Loc,
const PartialDiagnostic &Note,
SourceRange ParenRange) {
- SourceLocation EndLoc = Self.getLocForEndOfToken(ParenRange.getEnd());
- if (ParenRange.getBegin().isFileID() && ParenRange.getEnd().isFileID() &&
- EndLoc.isValid()) {
+ SourceManager &SM = Self.getSourceManager();
+ SourceLocation spellLoc = SM.getSpellingLoc(ParenRange.getEnd());
+ unsigned tokLen = Lexer::MeasureTokenLength(spellLoc, SM, Self.getLangOpts());
+ SourceLocation EndLoc = spellLoc.getLocWithOffset(tokLen);
+ if (EndLoc.isValid()) {
Self.Diag(Loc, Note)
<< FixItHint::CreateInsertion(ParenRange.getBegin(), "(")
<< FixItHint::CreateInsertion(EndLoc, ")");
@@ -12331,6 +12333,34 @@
}
}
+static void DiagnoseLogicalAndInLogicalOr(Sema &Self, SourceLocation OpLoc,
+ Expr *LHSExpr, Expr *RHSExpr) {
+ SourceManager &SM = Self.getSourceManager();
+ if (!OpLoc.isMacroID()) {
+DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr);
+DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr);
+return;
+ }
+
+ // Diagnose parentheses, if and only if operator and its LHS, RHS
+ // are from the same argument position of first level macros
+ SourceLocation OpExpansionLoc;
+ if (!SM.isMacroArgExpansion(OpLoc, &OpExpansionLoc) ||
+ SM.getImmediateMacroCallerLoc(OpLoc).isMacroID())
+return;
+
+ SourceLocation ExprExpansionLoc;
+ if (SM.isMacroArgExpansion(LHSExpr->getExprLoc(), &ExprExpansionLoc) &&
+ OpExpansionLoc == ExprExpansionLoc &&
+ !SM.getImmediateMacroCallerLoc(LHSExpr->getExprLoc()).isMacroID())
+DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr);
+
+ if (SM.isMacroArgExpansion(RHSExpr->getExprLoc(), &ExprExpansionLoc) &&
+ OpExpansionLoc == ExprExpansionLoc &&
+ !SM.getImmediateMacroCallerLoc(RHSExpr->getExprLoc()).isMacroID())
+DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr);
+}
+
/// Look for bitwise op in the left or right hand of a bitwise op with
/// lower precedence and emit a diagnostic together with a fixit hint that wraps
/// the '&' expression in parentheses.
@@ -12407,10 +12437,8 @@
// War
[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C
aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, dblaikie.
In C, enumerators are not hoisted into, say, a struct decl context when the
enumeration is declared inside of a struct. Instead, the enumerators are
hoisted into the translation unit decl context. This patch fixes
`getRedeclContext()` to skip records as well as transparent contexts when the
original context is an enumeration. This allows us to catch enumerator
redeclarations as well as silent name hiding + miscompiles.
This patch address PR15071.
https://reviews.llvm.org/D52384
Files:
lib/AST/DeclBase.cpp
test/Sema/enum.c
Index: test/Sema/enum.c
===
--- test/Sema/enum.c
+++ test/Sema/enum.c
@@ -135,3 +135,26 @@
};
int makeStructNonEmpty;
};
+
+static int EnumRedecl; // expected-note 2 {{previous definition is here}}
+struct S {
+ enum {
+EnumRedecl = 4 // expected-error {{redefinition of 'EnumRedecl'}}
+ } e;
+};
+
+union U {
+ enum {
+EnumRedecl = 5 // expected-error {{redefinition of 'EnumRedecl'}}
+ } e;
+};
+
+enum PR15071 {
+ PR15071_One // expected-note {{previous definition is here}}
+};
+
+struct EnumRedeclStruct {
+ enum {
+PR15071_One // expected-error {{redefinition of enumerator 'PR15071_One'}}
+ } e;
+};
Index: lib/AST/DeclBase.cpp
===
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -1700,8 +1700,16 @@
DeclContext *DeclContext::getRedeclContext() {
DeclContext *Ctx = this;
- // Skip through transparent contexts.
- while (Ctx->isTransparentContext())
+
+ // In C, the redeclaration context for enumerators is the translation unit,
+ // so we skip through transparent contexts as well as struct/union contexts.
+ bool SkipRecords = getDeclKind() == Decl::Kind::Enum &&
+ !getParentASTContext().getLangOpts().CPlusPlus;
+
+ // Skip through contexts to get to the redeclaration context. Transparent
+ // contexts are always skipped.
+ while (SkipRecords ? Ctx->isTransparentContext() || Ctx->isRecord()
+ : Ctx->isTransparentContext())
Ctx = Ctx->getParent();
return Ctx;
}
Index: test/Sema/enum.c
===
--- test/Sema/enum.c
+++ test/Sema/enum.c
@@ -135,3 +135,26 @@
};
int makeStructNonEmpty;
};
+
+static int EnumRedecl; // expected-note 2 {{previous definition is here}}
+struct S {
+ enum {
+EnumRedecl = 4 // expected-error {{redefinition of 'EnumRedecl'}}
+ } e;
+};
+
+union U {
+ enum {
+EnumRedecl = 5 // expected-error {{redefinition of 'EnumRedecl'}}
+ } e;
+};
+
+enum PR15071 {
+ PR15071_One // expected-note {{previous definition is here}}
+};
+
+struct EnumRedeclStruct {
+ enum {
+PR15071_One // expected-error {{redefinition of enumerator 'PR15071_One'}}
+ } e;
+};
Index: lib/AST/DeclBase.cpp
===
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -1700,8 +1700,16 @@
DeclContext *DeclContext::getRedeclContext() {
DeclContext *Ctx = this;
- // Skip through transparent contexts.
- while (Ctx->isTransparentContext())
+
+ // In C, the redeclaration context for enumerators is the translation unit,
+ // so we skip through transparent contexts as well as struct/union contexts.
+ bool SkipRecords = getDeclKind() == Decl::Kind::Enum &&
+ !getParentASTContext().getLangOpts().CPlusPlus;
+
+ // Skip through contexts to get to the redeclaration context. Transparent
+ // contexts are always skipped.
+ while (SkipRecords ? Ctx->isTransparentContext() || Ctx->isRecord()
+ : Ctx->isTransparentContext())
Ctx = Ctx->getParent();
return Ctx;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51789: [clang] Add the exclude_from_explicit_instantiation attribute
rsmith added inline comments.
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4683-4686
+ "Member '%0' marked with 'exclude_from_explicit_instantiation' attribute is "
+ "not defined but an explicit template instantiation declaration exists. "
+ "Reliance on this member being defined by an explicit template instantiation
"
+ "will lead to link errors.">;
ldionne wrote:
> rsmith wrote:
> > Diagnostics should start with a lowercase letter and not end with a period.
> >
> > That said, I'm not sure I see why this diagnostic is correct / useful. If
> > the entity is never used, then there's no link error. And if it is ever
> > used, then you should get an implicit instantiation like normal, and we
> > already have a diagnostic for the case where an entity is implicitly
> > instantiated and no definition is available.
> > Diagnostics should start with a lowercase letter and not end with a period.
>
> Done.
>
> > That said, I'm not sure I see why this diagnostic is correct / useful. If
> > the entity is never used, then there's no link error. And if it is ever
> > used, then you should get an implicit instantiation like normal, and we
> > already have a diagnostic for the case where an entity is implicitly
> > instantiated and no definition is available.
>
> This is not what happens right now. If you don't provide a definition but you
> try to call the function, an extern call will be produced (and that will
> result in a link error because any potential explicit instantiation won't
> provide the function). For example:
>
> ```
> cat < template
> struct Foo {
> __attribute__((exclude_from_explicit_instantiation)) static void
> static_member_function();
> };
>
> extern template struct Foo;
>
> int main() {
> Foo::static_member_function();
> }
> EOF
> ```
>
> Results in the following LLVM IR:
>
> ```
> ; Function Attrs: noinline norecurse nounwind optnone
> define i32 @main() #0 {
> entry:
> call void @_ZN3FooIiE22static_member_functionEv()
> ret i32 0
> }
>
> declare void @_ZN3FooIiE22static_member_functionEv() #1
> ```
>
> I guess we should be getting a warning or an error on the point of implicit
> instantiation instead, or is this behavior acceptable?
>
I don't think your example is fundamentally any different from:
```
template
struct Foo {
static void static_member_function();
};
int main() {
Foo::static_member_function();
}
```
which likewise produces a declaration of
`_ZN3FooIiE22static_member_functionEv`. That case produces a
`-Wundefined-func-template` warning; your example should do the same.
Repository:
rC Clang
https://reviews.llvm.org/D51789
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs
nickdesaulniers added inline comments.
Comment at: lib/Sema/SemaType.cpp:1682
// or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
srhines wrote:
> This is broken for C11 and C17 (even before you touch anything). As we just
> talked about, let's have a helper function to detect the oldest version (and
> maybe even that should get promoted as a LANGOPT).
From the declarations in `include/clang/Frontend/LangStandards.def`, newer
versions of C bitwise OR together flags of previous versions. So C99 sets C99
flag, C11 sets C99 and C11 flags, and C17 sets C99, C11, and C17 flags.
So this should be correct (though even to me, this looks wrong on first
glance). Thanks to @george.burgess.iv and you for help in looking into this.
Repository:
rC Clang
https://reviews.llvm.org/D52248
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs
nickdesaulniers updated this revision to Diff 166578.
nickdesaulniers marked an inline comment as done.
nickdesaulniers added a comment.
- also run test on gnu99+
Repository:
rC Clang
https://reviews.llvm.org/D52248
Files:
lib/Sema/SemaType.cpp
test/Sema/gnu89-const.c
Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-GNU89-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu99 -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu99 -pedantic -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu11 -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu11 -pedantic -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu17 -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu17 -pedantic -fsyntax-only 2>&1 | FileCheck
-check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89, unless -pedantic was specified. Do not warn in gnu99+, even
+// with -pedantic.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-GNU89-PEDANTIC: 14:7: warning: extension used
+// CHECK-GNU89-PEDANTIC: 14:1: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: 14:1: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: 19:7: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: 23:1: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
// C90 6.5.3 constraints: "The same type qualifier shall not appear more
// than once in the same specifier-list or qualifier-list, either directly
// or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic
&&
+ DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
<< "const";
Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU89-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu99 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu99 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu11 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu11 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu17 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu17 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89, unless -pedantic was specified. Do not warn in gnu99+, even
+// with -pedantic.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-GNU89-PEDANTIC: 14:7: warning: extension used
+// CHECK-GNU89-PEDANTIC: 14:1: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: 14:1: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: 19:7: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: 23:1: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
// C90 6.5.3 constraints: "The same type qualifier shall not appear more
// than once in the same specifier-list or qualifier-list, either directly
// or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getC
[PATCH] D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`.
kristina marked 2 inline comments as done. kristina added a comment. Binary layout also looks sane (compiled with `-fPIC -faddrsig -Wl,--icf=safe`), just digging it through it in a dissasembler: [/q/src/clwn]$ llvm-readobj -elf-output-style=GNU -sections /q/org.llvm.caches/clownschool/clwn-sysroot/System/Library/Frameworks/CoreFoundation.framework/libCoreFoundation.so There are 36 section headers, starting at offset 0x3f57d8: Section Headers: [Nr] Name TypeAddress OffSize ES Flg Lk Inf Al [ 0] NULL 00 00 00 0 0 0 [ 1] .dynsym DYNSYM 0200 000200 00fd08 18 A 5 1 8 [ 2] .gnu.version VERSYM ff08 00ff08 001516 02 A 1 0 2 [ 3] .gnu.version_rVERNEED 00011420 011420 0001a0 00 A 5 5 4 [ 4] .gnu.hash GNU_HASH000115c0 0115c0 003bf8 00 A 1 0 8 [ 5] .dynstr STRTAB 000151b8 0151b8 011677 00 A 0 0 1 [ 6] .rela.dyn RELA00026830 026830 01f860 18 A 1 0 8 [ 7] .rela.plt RELA00046090 046090 006408 18 A 1 0 8 [ 8] .rodata PROGBITS0004c4a0 04c4a0 00fdd0 00 AMS 0 0 32 [ 9] .gcc_except_table PROGBITS0005c270 05c270 cc 00 A 0 0 4 [10] .eh_frame_hdr PROGBITS0005c33c 05c33c 005934 00 A 0 0 4 [11] .eh_frame PROGBITS00061c70 061c70 017fac 00 A 0 0 8 [12] .text PROGBITS0007a000 07a000 228e1f 00 AX 0 0 16 [13] .init PROGBITS002a2e20 2a2e20 1a 00 AX 0 0 4 [14] .fini PROGBITS002a2e3c 2a2e3c 09 00 AX 0 0 4 [15] .plt PROGBITS002a2e50 2a2e50 0042c0 00 AX 0 0 16 [16] .data PROGBITS002a8000 2a8000 001580 00 WA 0 0 16 [17] .tm_clone_table PROGBITS002a9580 2a9580 00 00 WA 0 0 8 [18] cfstring PROGBITS002a9580 2a9580 007740 00 WA 0 0 8 [19] .got.plt PROGBITS002b0cc0 2b0cc0 002170 00 WA 0 0 8 [20] .fini_array FINI_ARRAY 002b3000 2b3000 08 00 WA 0 0 8 [21] .init_array INIT_ARRAY 002b3008 2b3008 10 00 WA 0 0 8 [22] .data.rel.ro PROGBITS002b3020 2b3020 007a18 00 WA 0 0 16 [23] .dynamic DYNAMIC 002baa38 2baa38 000200 10 WA 5 0 8 [24] .got PROGBITS002bac38 2bac38 000718 00 WA 0 0 8 [25] .bss NOBITS 002bc000 2bb350 0050a8 00 WA 0 0 16 [26] .comment PROGBITS 2bb350 0002b3 01 MS 0 0 1 [27] .debug_strPROGBITS 2bb603 00b6a0 01 MS 0 0 1 [28] .debug_abbrev PROGBITS 2c6ca3 00162d 00 0 0 1 [29] .debug_info PROGBITS 2c82d0 0261bc 00 0 0 1 [30] .debug_macinfoPROGBITS 2ee48c 53 00 0 0 1 [31] .debug_line PROGBITS 2ee4df 0cb0f5 00 0 0 1 [32] .debug_ranges PROGBITS 3b95d4 30 00 0 0 1 [33] .symtab SYMTAB 3b9608 01d370 18 35 2288 8 [34] .shstrtab STRTAB 3d6978 000165 00 0 0 1 [35] .strtab STRTAB 3d6add 01ecf4 00 0 0 1 Repository: rC Clang https://reviews.llvm.org/D52344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342790 - Thread safety analysis: Make printSCFG compile again [NFC]
Author: aaronpuchert
Date: Fri Sep 21 16:46:35 2018
New Revision: 342790
URL: http://llvm.org/viewvc/llvm-project?rev=342790&view=rev
Log:
Thread safety analysis: Make printSCFG compile again [NFC]
Not used productively, so no observable functional change.
Note that printSCFG doesn't yet work reliably, it seems to crash
sometimes.
Modified:
cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h?rev=342790&r1=342789&r2=342790&view=diff
==
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h Fri Sep 21
16:46:35 2018
@@ -785,7 +785,26 @@ protected:
void printCast(const Cast *E, StreamType &SS) {
if (!CStyle) {
SS << "cast[";
- SS << E->castOpcode();
+ switch (E->castOpcode()) {
+ case CAST_none:
+SS << "none";
+break;
+ case CAST_extendNum:
+SS << "extendNum";
+break;
+ case CAST_truncNum:
+SS << "truncNum";
+break;
+ case CAST_toFloat:
+SS << "toFloat";
+break;
+ case CAST_toInt:
+SS << "toInt";
+break;
+ case CAST_objToPtr:
+SS << "objToPtr";
+break;
+ }
SS << "](";
self()->printSExpr(E->expr(), SS, Prec_Unary);
SS << ")";
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=342790&r1=342789&r2=342790&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Fri Sep 21 16:46:35 2018
@@ -64,13 +64,6 @@ using namespace threadSafety;
// Key method definition
ThreadSafetyHandler::~ThreadSafetyHandler() = default;
-namespace {
-
-class TILPrinter :
-public til::PrettyPrinter {};
-
-} // namespace
-
/// Issue a warning about an invalid lock expression
static void warnInvalidLock(ThreadSafetyHandler &Handler,
const Expr *MutexExp, const NamedDecl *D,
Modified: cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp?rev=342790&r1=342789&r2=342790&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp Fri Sep 21 16:46:35 2018
@@ -944,6 +944,16 @@ void SExprBuilder::exitCFG(const CFGBloc
}
/*
+namespace {
+
+class TILPrinter :
+public til::PrettyPrinter {};
+
+} // namespace
+
+namespace clang {
+namespace threadSafety {
+
void printSCFG(CFGWalker &Walker) {
llvm::BumpPtrAllocator Bpa;
til::MemRegionRef Arena(&Bpa);
@@ -951,4 +961,7 @@ void printSCFG(CFGWalker &Walker) {
til::SCFG *Scfg = SxBuilder.buildCFG(Walker);
TILPrinter::print(Scfg, llvm::errs());
}
+
+} // namespace threadSafety
+} // namespace clang
*/
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`.
kristina updated this revision to Diff 166576.
kristina added a comment.
Does this look about right? I'm not sure what's up with DSO local attribute, it
doesn't seem to apply, I think this test makes sense though. `ninja
check-clang-codegen` succeeded this time:
Testing Time: 12.77s
Expected Passes: 1242
Expected Failures : 2
Unsupported Tests : 120
Bitcode looks like this (for the 3rd case, which is a fairly odd one since it's
a variable sized array without extern):
/q/org.llvm.caches/llvm-8.0/4132/bin/clang -cc1 -internal-isystem
/q/org.llvm.caches/llvm-8.0/4132/lib/clang/8.0.4132/include -nostdsysteminc
-triple x86_64-elf -S -emit-llvm
/SourceCache/llvm-trunk-8.0.4132/tools/clang/test/CodeGen/cfstring-elf.c -o -
; ModuleID =
'/SourceCache/llvm-trunk-8.0.4132/tools/clang/test/CodeGen/cfstring-elf.c'
source_filename =
"/SourceCache/llvm-trunk-8.0.4132/tools/clang/test/CodeGen/cfstring-elf.c"
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-unknown-elf"
%struct.__NSConstantString_tag = type { i32*, i32, i8*, i64 }
%struct.__CFString = type opaque
@.str = private unnamed_addr constant [7 x i8] c"string\00", section
".rodata", align 1
@_unnamed_cfstring_ = private global %struct.__NSConstantString_tag { i32*
getelementptr inbounds ([0 x i32], [0 x i32]* bitcast ([1 x i64]*
@__CFConstantStringClassReference to [0 x i32]*), i32 0, i32 0), i32 1992, i8*
getelementptr inbounds ([7 x i8], [7 x i8]* @.str, i32 0, i32 0), i64 6 },
section "cfstring", align 8
@string = constant %struct.__CFString* bitcast
(%struct.__NSConstantString_tag* @_unnamed_cfstring_ to %struct.__CFString*),
align 8
@__CFConstantStringClassReference = common global [1 x i64] zeroinitializer,
align 8
!llvm.module.flags = !{!0}
!llvm.ident = !{!1}
!0 = !{i32 1, !"wchar_size", i32 4}
!1 = !{!"Kristina's toolchain (8.0.4132+assert+modules+thinlto+tests+debug)
clang version 8.0.4132 (https://llvm.googlesource.com/clang
8c1a8e6be4042ef6e930f19162f5e308ac56a38e) (https://llvm.googlesource.com/llvm
bf40f1cb4e37e0046f94428f6332255ccf00e440) (based on LLVM 8.0.4132svn)"}
Repository:
rC Clang
https://reviews.llvm.org/D52344
Files:
lib/CodeGen/CodeGenModule.cpp
test/CodeGen/cfstring-elf.c
Index: test/CodeGen/cfstring-elf.c
===
--- test/CodeGen/cfstring-elf.c
+++ test/CodeGen/cfstring-elf.c
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-elf -DCF_BUILDING_CF -DDECL -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF-IN-CF-DECL
+// RUN: %clang_cc1 -triple x86_64-elf -DCF_BUILDING_CF -DDEFN -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF-IN-CF-DEFN
+// RUN: %clang_cc1 -triple x86_64-elf -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF
+// RUN: %clang_cc1 -triple x86_64-elf -DEXTERN -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF-EXTERN
+
+// RUN: %clang_cc1 -Os -triple x86_64-elf -DCF_BUILDING_CF -DDECL -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF-IN-CF-DECL
+// RUN: %clang_cc1 -Os -triple x86_64-elf -DCF_BUILDING_CF -DDEFN -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF-IN-CF-DEFN
+// RUN: %clang_cc1 -Os -triple x86_64-elf -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF
+// RUN: %clang_cc1 -Os -triple x86_64-elf -DEXTERN -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-CF-EXTERN
+
+
+#if defined(CF_BUILDING_CF)
+#if defined(DECL)
+extern long __CFConstantStringClassReference[];
+#elif defined(DEFN)
+long __CFConstantStringClassReference[32];
+#endif
+#else
+#if defined(EXTERN)
+extern long __CFConstantStringClassReference[];
+#else
+long __CFConstantStringClassReference[];
+#endif
+#endif
+
+typedef struct __CFString *CFStringRef;
+const CFStringRef string = (CFStringRef)__builtin___CFStringMakeConstantString("string");
+
+
+// CHECK-CF-IN-CF-DECL: @__CFConstantStringClassReference = external global [0 x i32]
+// CHECK-CF-IN-CF-DEFN: @__CFConstantStringClassReference = common global [32 x i64] zeroinitializer, align 16
+// CHECK-CF: @__CFConstantStringClassReference = common global [1 x i64] zeroinitializer, align 8
+// CHECK-CF-EXTERN: @__CFConstantStringClassReference = external global [0 x i32]
+// CHECK-CF-EXTERN: @.str = private unnamed_addr constant [7 x i8] c"string\00", section ".rodata", align 1
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -4109,37 +4109,48 @@
llvm::Constant *Zero = llvm::Constant::getNullValue(Int32Ty);
llvm::Constant *Zeros[] = { Zero, Zero };
-
+
// If we don't already have it, get __CFConstantStringClassReference.
if (!CFConstantStringClassRef) {
llvm::Type *Ty = getTypes().ConvertType(getContext().IntTy);
Ty = llvm::ArrayType::get(Ty, 0);
-llvm::GlobalValue *GV = cast(
-
r342789 - Fix codemodels.c test case (only test mcmodel-kernel on x86)
Author: ctice Date: Fri Sep 21 16:19:49 2018 New Revision: 342789 URL: http://llvm.org/viewvc/llvm-project?rev=342789&view=rev Log: Fix codemodels.c test case (only test mcmodel-kernel on x86) A recent commit I made broke aarch64 testing, because "kernel" apparently is not a valid code-model on aarch64, and one of my tests tested that. This fixes the problem (hopefully) by adding "-triple x86_64-unknown-linux-gnu" to the test build with "-mcodel-model kernel". Differential Revision: https://reviews.llvm.org/D52383 Modified: cfe/trunk/test/CodeGen/codemodels.c Modified: cfe/trunk/test/CodeGen/codemodels.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/codemodels.c?rev=342789&r1=342788&r2=342789&view=diff == --- cfe/trunk/test/CodeGen/codemodels.c (original) +++ cfe/trunk/test/CodeGen/codemodels.c Fri Sep 21 16:19:49 2018 @@ -1,7 +1,7 @@ // RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-NOMODEL // RUN: %clang_cc1 -emit-llvm -mcode-model tiny %s -o - | FileCheck %s -check-prefix=CHECK-TINY // RUN: %clang_cc1 -emit-llvm -mcode-model small %s -o - | FileCheck %s -check-prefix=CHECK-SMALL -// RUN: %clang_cc1 -emit-llvm -mcode-model kernel %s -o - | FileCheck %s -check-prefix=CHECK-KERNEL +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -mcode-model kernel %s -o - | FileCheck %s -check-prefix=CHECK-KERNEL // RUN: %clang_cc1 -emit-llvm -mcode-model medium %s -o - | FileCheck %s -check-prefix=CHECK-MEDIUM // RUN: %clang_cc1 -emit-llvm -mcode-model large %s -o - | FileCheck %s -check-prefix=CHECK-LARGE ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342787 - Thread safety analysis: Make sure FactEntrys stored in FactManager are immutable [NFC]
Author: aaronpuchert
Date: Fri Sep 21 16:08:30 2018
New Revision: 342787
URL: http://llvm.org/viewvc/llvm-project?rev=342787&view=rev
Log:
Thread safety analysis: Make sure FactEntrys stored in FactManager are
immutable [NFC]
Since FactEntrys are stored in the FactManager, we can't manipulate them
anymore when they are stored there.
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=342787&r1=342786&r2=342787&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Fri Sep 21 16:08:30 2018
@@ -151,7 +151,7 @@ public:
StringRef DiagKind) const = 0;
// Return true if LKind >= LK, where exclusive > shared
- bool isAtLeast(LockKind LK) {
+ bool isAtLeast(LockKind LK) const {
return (LKind == LK_Exclusive) || (LK == LK_Shared);
}
};
@@ -162,7 +162,7 @@ using FactID = unsigned short;
/// the analysis of a single routine.
class FactManager {
private:
- std::vector> Facts;
+ std::vector> Facts;
public:
FactID newFact(std::unique_ptr Entry) {
@@ -171,7 +171,6 @@ public:
}
const FactEntry &operator[](FactID F) const { return *Facts[F]; }
- FactEntry &operator[](FactID F) { return *Facts[F]; }
};
/// A FactSet is the set of facts that are known to be true at a
@@ -241,22 +240,23 @@ public:
});
}
- FactEntry *findLock(FactManager &FM, const CapabilityExpr &CapE) const {
+ const FactEntry *findLock(FactManager &FM, const CapabilityExpr &CapE) const
{
auto I = std::find_if(begin(), end(), [&](FactID ID) {
return FM[ID].matches(CapE);
});
return I != end() ? &FM[*I] : nullptr;
}
- FactEntry *findLockUniv(FactManager &FM, const CapabilityExpr &CapE) const {
+ const FactEntry *findLockUniv(FactManager &FM,
+const CapabilityExpr &CapE) const {
auto I = std::find_if(begin(), end(), [&](FactID ID) -> bool {
return FM[ID].matchesUniv(CapE);
});
return I != end() ? &FM[*I] : nullptr;
}
- FactEntry *findPartialMatch(FactManager &FM,
- const CapabilityExpr &CapE) const {
+ const FactEntry *findPartialMatch(FactManager &FM,
+const CapabilityExpr &CapE) const {
auto I = std::find_if(begin(), end(), [&](FactID ID) -> bool {
return FM[ID].partiallyMatches(CapE);
});
@@ -1258,7 +1258,7 @@ void ThreadSafetyAnalyzer::addLock(FactS
if (!ReqAttr && !Entry->negative()) {
// look for the negative capability, and remove it from the fact set.
CapabilityExpr NegC = !*Entry;
-FactEntry *Nen = FSet.findLock(FactMan, NegC);
+const FactEntry *Nen = FSet.findLock(FactMan, NegC);
if (Nen) {
FSet.removeLock(FactMan, NegC);
}
@@ -1277,7 +1277,7 @@ void ThreadSafetyAnalyzer::addLock(FactS
}
// FIXME: Don't always warn when we have support for reentrant locks.
- if (FactEntry *Cp = FSet.findLock(FactMan, *Entry)) {
+ if (const FactEntry *Cp = FSet.findLock(FactMan, *Entry)) {
if (!Entry->asserted())
Cp->handleLock(FSet, FactMan, *Entry, Handler, DiagKind);
} else {
@@ -1575,7 +1575,7 @@ void BuildLockset::warnIfMutexNotHeld(co
if (Cp.negative()) {
// Negative capabilities act like locks excluded
-FactEntry *LDat = FSet.findLock(Analyzer->FactMan, !Cp);
+const FactEntry *LDat = FSet.findLock(Analyzer->FactMan, !Cp);
if (LDat) {
Analyzer->Handler.handleFunExcludesLock(
DiagKind, D->getNameAsString(), (!Cp).toString(), Loc);
@@ -1596,7 +1596,7 @@ void BuildLockset::warnIfMutexNotHeld(co
return;
}
- FactEntry* LDat = FSet.findLockUniv(Analyzer->FactMan, Cp);
+ const FactEntry *LDat = FSet.findLockUniv(Analyzer->FactMan, Cp);
bool NoError = true;
if (!LDat) {
// No exact match found. Look for a partial match.
@@ -1632,7 +1632,7 @@ void BuildLockset::warnIfMutexHeld(const
return;
}
- FactEntry* LDat = FSet.findLock(Analyzer->FactMan, Cp);
+ const FactEntry *LDat = FSet.findLock(Analyzer->FactMan, Cp);
if (LDat) {
Analyzer->Handler.handleFunExcludesLock(
DiagKind, D->getNameAsString(), Cp.toString(), Exp->getExprLoc());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51187: [RFC] Thread safety analysis: Track status of scoped capability
delesley added a comment. In https://reviews.llvm.org/D51187#1241354, @aaronpuchert wrote: > Thanks for pointing out my error! Ignoring the implementation for a moment, > do you think this is a good idea or will it produce too many false positives? > Releasable/relockable scoped capabilities that I have seen keep track of the > status, so it makes sense, but maybe there are others out there. I think this is probably a good idea. Code which manipulates the underlying mutex while it is being managed by another object is a recipe for trouble. It's hard for me to guess how many false positives it will generate without actually running it over our code base, which I personally don't have time to do right now. It should definitely go in -Wthread-safety-beta so it won't break the build. Unfortunately, the before/after checks are still in thread-safety-beta, and they should really be moved out of beta before something else is moved in. The good news is that Matthew O'Niel just volunteered to do that. That is, unfortunately, not a trivial operation, so it may be some weeks before he's done. Repository: rC Clang https://reviews.llvm.org/D51187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342774 - Make compare function in r342648 have strict weak ordering.
Author: rtrieu Date: Fri Sep 21 14:20:33 2018 New Revision: 342774 URL: http://llvm.org/viewvc/llvm-project?rev=342774&view=rev Log: Make compare function in r342648 have strict weak ordering. Comparison functions used in sorting algorithms need to have strict weak ordering. Remove the assert and allow comparisons on all lists. Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=342774&r1=342773&r2=342774&view=diff == --- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original) +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Fri Sep 21 14:20:33 2018 @@ -7607,8 +7607,15 @@ public: SI->getAssociatedDeclaration()) break; } -assert(CI != CE && SI != SE && - "Unexpected end of the map components."); + +// Lists contain the same elements. +if (CI == CE && SI == SE) + return false; + +// List with less elements is less than list with more elements. +if (CI == CE || SI == SE) + return CI == CE; + const auto *FD1 = cast(CI->getAssociatedDeclaration()); const auto *FD2 = cast(SI->getAssociatedDeclaration()); if (FD1->getParent() == FD2->getParent()) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51812: Simplify CheckFallThroughForBody
gromer added a comment. OK, the diffs are now un-borked. Sorry for the flailing incompetence. https://reviews.llvm.org/D51812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52377: [HIP] Support early finalization of device code
tra added a comment. Overall the patch look OK. I'll take a closer look on Monday. Which mode do you expect will be most commonly used for HIP by default? With this patch we'll have two different ways to do similar things in HIP vs. CUDA. E.g. by default CUDA compiles GPU code in each TU in a complete executable and requires -fcuda-rdc to compile to GPU object file. HIP defaults to object-file compilation and requires --hip-early-finalize to match CUDA's default behavior. I wonder if it would make sense to provide a single way to control this behavior. E.g. `--fgpu-rdc` (an alias for -cuda-rdc, perhaps?) would default to true in HIP, but disabled in CUDA. `-fno-gpu-rdc` would force 'whole GPU executable per TU' mode. https://reviews.llvm.org/D52377 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51812: Simplify CheckFallThroughForBody
gromer updated this revision to Diff 166558.
https://reviews.llvm.org/D51812
Files:
b/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
Index: b/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- b/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -497,205 +497,218 @@
return AlwaysFallThrough;
}
-namespace {
+static void CheckFallThroughForBlock(Sema &S, const Stmt *Body,
+ const BlockExpr *blkExpr,
+ AnalysisDeclContext &AC) {
+ const FunctionType *FT =
+ blkExpr->getType()->getPointeeType()->getAs();
+ if (FT == nullptr) return;
-struct CheckFallThroughDiagnostics {
- unsigned diag_MaybeFallThrough_HasNoReturn;
- unsigned diag_MaybeFallThrough_ReturnsNonVoid;
- unsigned diag_AlwaysFallThrough_HasNoReturn;
- unsigned diag_AlwaysFallThrough_ReturnsNonVoid;
- unsigned diag_NeverFallThroughOrReturn;
- enum { Function, Block, Lambda, Coroutine } funMode;
- SourceLocation FuncLoc;
-
- static CheckFallThroughDiagnostics MakeForFunction(const Decl *Func) {
-CheckFallThroughDiagnostics D;
-D.FuncLoc = Func->getLocation();
-D.diag_MaybeFallThrough_HasNoReturn =
- diag::warn_falloff_noreturn_function;
-D.diag_MaybeFallThrough_ReturnsNonVoid =
- diag::warn_maybe_falloff_nonvoid_function;
-D.diag_AlwaysFallThrough_HasNoReturn =
- diag::warn_falloff_noreturn_function;
-D.diag_AlwaysFallThrough_ReturnsNonVoid =
- diag::warn_falloff_nonvoid_function;
+ bool ReturnsValue = !FT->getReturnType()->isVoidType();
+ bool HasNoReturnAttr = FT->getNoReturnAttr();
-// Don't suggest that virtual functions be marked "noreturn", since they
-// might be overridden by non-noreturn functions.
-bool isVirtualMethod = false;
-if (const CXXMethodDecl *Method = dyn_cast(Func))
- isVirtualMethod = Method->isVirtual();
+ // Short circuit for compilation speed.
+ if (!ReturnsValue && !HasNoReturnAttr) return;
-// Don't suggest that template instantiations be marked "noreturn"
-bool isTemplateInstantiation = false;
-if (const FunctionDecl *Function = dyn_cast(Func))
- isTemplateInstantiation = Function->isTemplateInstantiation();
+ SourceLocation RBrace = Body->getEndLoc();
+ switch (CheckFallThrough(AC)) {
+case MaybeFallThrough:
+ if (HasNoReturnAttr)
+S.Diag(RBrace, diag::err_noreturn_block_has_return_expr);
+ else if (ReturnsValue)
+S.Diag(RBrace, diag::err_maybe_falloff_nonvoid_block);
+ break;
-if (!isVirtualMethod && !isTemplateInstantiation)
- D.diag_NeverFallThroughOrReturn =
-diag::warn_suggest_noreturn_function;
-else
- D.diag_NeverFallThroughOrReturn = 0;
+case AlwaysFallThrough:
+ if (HasNoReturnAttr)
+S.Diag(RBrace, diag::err_noreturn_block_has_return_expr);
+ else if (ReturnsValue)
+S.Diag(RBrace, diag::err_falloff_nonvoid_block);
+ break;
-D.funMode = Function;
-return D;
+case NeverFallThroughOrReturn:
+case NeverFallThrough:
+case UnknownFallThrough:
+ break;
}
+}
- static CheckFallThroughDiagnostics MakeForCoroutine(const Decl *Func) {
-CheckFallThroughDiagnostics D;
-D.FuncLoc = Func->getLocation();
-D.diag_MaybeFallThrough_HasNoReturn = 0;
-D.diag_MaybeFallThrough_ReturnsNonVoid =
-diag::warn_maybe_falloff_nonvoid_coroutine;
-D.diag_AlwaysFallThrough_HasNoReturn = 0;
-D.diag_AlwaysFallThrough_ReturnsNonVoid =
-diag::warn_falloff_nonvoid_coroutine;
-D.funMode = Coroutine;
-return D;
- }
-
- static CheckFallThroughDiagnostics MakeForBlock() {
-CheckFallThroughDiagnostics D;
-D.diag_MaybeFallThrough_HasNoReturn =
- diag::err_noreturn_block_has_return_expr;
-D.diag_MaybeFallThrough_ReturnsNonVoid =
- diag::err_maybe_falloff_nonvoid_block;
-D.diag_AlwaysFallThrough_HasNoReturn =
- diag::err_noreturn_block_has_return_expr;
-D.diag_AlwaysFallThrough_ReturnsNonVoid =
- diag::err_falloff_nonvoid_block;
-D.diag_NeverFallThroughOrReturn = 0;
-D.funMode = Block;
-return D;
- }
-
- static CheckFallThroughDiagnostics MakeForLambda() {
-CheckFallThroughDiagnostics D;
-D.diag_MaybeFallThrough_HasNoReturn =
- diag::err_noreturn_lambda_has_return_expr;
-D.diag_MaybeFallThrough_ReturnsNonVoid =
- diag::warn_maybe_falloff_nonvoid_lambda;
-D.diag_AlwaysFallThrough_HasNoReturn =
- diag::err_noreturn_lambda_has_return_expr;
-D.diag_AlwaysFallThrough_ReturnsNonVoid =
- diag::warn_falloff_nonvoid_lambda;
-D.diag_NeverFallThroughOrReturn = 0;
-D.funMode = Lambda;
-return D;
- }
-
- bool checkDiagnostics(DiagnosticsEngine &D, bool ReturnsVoid,
-bool HasNoReturn) const {
-if (funMode == Function) {
- return (Re
[PATCH] D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`.
smeenai added a comment. In https://reviews.llvm.org/D52344#1242530, @rjmccall wrote: > In https://reviews.llvm.org/D52344#1242451, @kristina wrote: > > > Would you be okay with me renaming `cfstring` to `.cfstring` for ELF so > > it's in line with ELF section naming conventions (I'm not entirely sure if > > that could cause issues with ObjC stuff)? And yes I think it's best to > > avoid that code-path altogether if it turns out to be a constant as that's > > likely from the declaration of the type, I'll write a FileCheck test in a > > moment and check that I can apply the same logic to ELF aside from the DLL > > stuff as CoreFoundation needs to export the symbol somehow while preserving > > the implicit extern attribute for every other TU that comes from the > > builtin that `CFSTR` expands to. Is what I'm suggesting making sense? If > > not, let me know, I may be misunderstanding what's happening here. > > > Following the ELF conventions seems like the right thing to do, assuming it > doesn't cause compatibility problems. David Chisnall might know best here. We would lose the `__start_` and `__end_` markers (which @compnerd mentioned) if the section was renamed to `.cfstring`, because the linker only generates those markers for sections whose names are valid C identifiers, so I don't think that rename would be valid. Here's how LLD does it, for example: https://reviews.llvm.org/diffusion/L/browse/lld/trunk/ELF/Writer.cpp;342772$1764-1776 Repository: rC Clang https://reviews.llvm.org/D52344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52377: [HIP] Support early finalization of device code
yaxunl updated this revision to Diff 166556.
yaxunl added a comment.
Fix comments.
https://reviews.llvm.org/D52377
Files:
include/clang/Driver/Options.td
include/clang/Driver/Types.def
lib/CodeGen/CGCUDANV.cpp
lib/Driver/Driver.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Driver/ToolChains/CommonArgs.cpp
lib/Driver/ToolChains/HIP.cpp
lib/Driver/ToolChains/HIP.h
test/CodeGenCUDA/device-stub.cu
test/Driver/cuda-phases.cu
test/Driver/hip-early-finalize.hip
test/Driver/hip-toolchain.hip
Index: test/Driver/hip-toolchain.hip
===
--- test/Driver/hip-toolchain.hip
+++ test/Driver/hip-toolchain.hip
@@ -80,7 +80,7 @@
// CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
// CHECK-SAME: "-targets={{.*}},hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900"
-// CHECK-SAME: "-inputs={{.*}},[[IMG_DEV1]],[[IMG_DEV2]]" "-outputs=[[BUNDLE:.*o]]"
+// CHECK-SAME: "-inputs={{.*}},[[IMG_DEV1]],[[IMG_DEV2]]" "-outputs=[[BUNDLE:.*hipfb]]"
// CHECK: [[LD:".*ld.*"]] {{.*}} [[A_OBJ_HOST]] [[B_OBJ_HOST]]
// CHECK-SAME: {{.*}} "-T" "{{.*}}.lk"
Index: test/Driver/hip-early-finalize.hip
===
--- /dev/null
+++ test/Driver/hip-early-finalize.hip
@@ -0,0 +1,150 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu --hip-early-finalize \
+// RUN: -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
+// RUN: --hip-device-lib=lib1.bc --hip-device-lib=lib2.bc \
+// RUN: --hip-device-lib-path=%S/Inputs/hip_multiple_inputs/lib1 \
+// RUN: --hip-device-lib-path=%S/Inputs/hip_multiple_inputs/lib2 \
+// RUN: -fuse-ld=lld \
+// RUN: %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN: %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck -check-prefixes=CHECK %s
+
+//
+// Compile device code in a.cu to code object for gfx803.
+//
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx803"
+// CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
+// CHECK-SAME: {{.*}} "-o" [[A_BC_803:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
+
+// CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_803]]
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
+// CHECK-SAME: "-o" [[LINKED_BC_DEV_A_803:".*-gfx803-linked-.*bc"]]
+
+// CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_803]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx803"
+// CHECK-SAME: "-o" [[OPT_BC_DEV_A_803:".*-gfx803-optimized.*bc"]]
+
+// CHECK: [[LLC: ".*llc"]] [[OPT_BC_DEV_A_803]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-filetype=obj" "-mcpu=gfx803" "-o" [[OBJ_DEV_A_803:".*-gfx803-.*o"]]
+
+// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK-SAME: "-o" "[[IMG_DEV_A_803:.*out]]" [[OBJ_DEV_A_803]]
+
+//
+// Compile device code in a.cu to code object for gfx900.
+//
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx900"
+// CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
+// CHECK-SAME: {{.*}} "-o" [[A_BC_900:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[A_SRC]]
+
+// CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_900]]
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
+// CHECK-SAME: "-o" [[LINKED_BC_DEV_A_900:".*-gfx900-linked-.*bc"]]
+
+// CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_900]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx900"
+// CHECK-SAME: "-o" [[OPT_BC_DEV_A_900:".*-gfx900-optimized.*bc"]]
+
+// CHECK: [[LLC: ".*llc"]] [[OPT_BC_DEV_A_900]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-filetype=obj" "-mcpu=gfx900" "-o" [[OBJ_DEV_A_900:".*-gfx900-.*o"]]
+
+// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK-SAME: "-o" "[[IMG_DEV_A_900:.*out]]" [[OBJ_DEV_A_900]]
+
+//
+// Bundle and embed device code in host object for a.cu.
+//
+
+// CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
+// CHECK-SAME: "-targets={{.*}},hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900"
+// CHECK-SAME: "-inputs={{.*}},[[IMG_DEV_A_803]],[[IMG_DEV_A_900]]" "-outputs=[[BUNDLE_A:.*hipfb]]"
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "x86_64-unknown-linux-gnu"
+// CHECK-SAME: "-aux-triple" "amdgcn-amd-amdhsa" "-emit-obj"
+// CHECK-SAME: {{.*}} "-main-file-name" "a.cu"
+// CHECK-SAME: {{.*}} "-o" [[A_OBJ_HOST:".*o"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[A_SRC]]
+// CHECK-SAME: {{.*}} "-fcuda-include-gpubinary" "[[BUNDLE_A]]"
+
+//
+// Compile device code in b.hip to code object for gfx803.
+//
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-e
[PATCH] D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`.
rjmccall added a reviewer: theraven. rjmccall added a comment. In https://reviews.llvm.org/D52344#1242451, @kristina wrote: > Would you be okay with me renaming `cfstring` to `.cfstring` for ELF so it's > in line with ELF section naming conventions (I'm not entirely sure if that > could cause issues with ObjC stuff)? And yes I think it's best to avoid that > code-path altogether if it turns out to be a constant as that's likely from > the declaration of the type, I'll write a FileCheck test in a moment and > check that I can apply the same logic to ELF aside from the DLL stuff as > CoreFoundation needs to export the symbol somehow while preserving the > implicit extern attribute for every other TU that comes from the builtin that > `CFSTR` expands to. Is what I'm suggesting making sense? If not, let me know, > I may be misunderstanding what's happening here. Following the ELF conventions seems like the right thing to do, assuming it doesn't cause compatibility problems. David Chisnall might know best here. Comment at: lib/CodeGen/CodeGenModule.cpp:4108 + // consistent with old behavior for this target as it would fail + // on the cast<> instead. + assert(GV && "isa isn't a GlobalValue"); kristina wrote: > rjmccall wrote: > > compnerd wrote: > > > I think that the comment isn't particularly helpful - it basically is > > > directing you to look at the history of the file. > > I think we should just skip this step, even on COFF, if it's not a > > `GlobalValue`. > > > > Probably the most correct move would be to only apply this logic if the IR > > declaration was synthesized. Or maybe even just change this code to look > > for a global variable called `__CFConstantStringClassReference` in the > > first place and then emit a reference to it if found, and only otherwise > > try to build the synthetic variable. But just bailing out early if > > something weird is going on is also a legitimate step. > Doesn't this code practically do this since creating a runtime global is a > no-op if it already exists within the module, however there is potentially > one module which can have it which is CFString.c (assuming no bridging here > of any kind) while it makes sense for the rest to refer to it as an implicit > extern (in which case I need to add an ELF code path to do exactly the same. Notice that this logic runs regardless of whether we actually created the global, though; we're potentially overwriting stuff that was decided based on content from the actual declaration. Repository: rC Clang https://reviews.llvm.org/D52344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51812: Simplify CheckFallThroughForBody
gromer updated this revision to Diff 166545.
https://reviews.llvm.org/D51812
Files:
B/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
Index: B/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- B/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ B/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -497,205 +497,218 @@
return AlwaysFallThrough;
}
-namespace {
+static void CheckFallThroughForBlock(Sema &S, const Stmt *Body,
+ const BlockExpr *blkExpr,
+ AnalysisDeclContext &AC) {
+ const FunctionType *FT =
+ blkExpr->getType()->getPointeeType()->getAs();
+ if (FT == nullptr) return;
-struct CheckFallThroughDiagnostics {
- unsigned diag_MaybeFallThrough_HasNoReturn;
- unsigned diag_MaybeFallThrough_ReturnsNonVoid;
- unsigned diag_AlwaysFallThrough_HasNoReturn;
- unsigned diag_AlwaysFallThrough_ReturnsNonVoid;
- unsigned diag_NeverFallThroughOrReturn;
- enum { Function, Block, Lambda, Coroutine } funMode;
- SourceLocation FuncLoc;
-
- static CheckFallThroughDiagnostics MakeForFunction(const Decl *Func) {
-CheckFallThroughDiagnostics D;
-D.FuncLoc = Func->getLocation();
-D.diag_MaybeFallThrough_HasNoReturn =
- diag::warn_falloff_noreturn_function;
-D.diag_MaybeFallThrough_ReturnsNonVoid =
- diag::warn_maybe_falloff_nonvoid_function;
-D.diag_AlwaysFallThrough_HasNoReturn =
- diag::warn_falloff_noreturn_function;
-D.diag_AlwaysFallThrough_ReturnsNonVoid =
- diag::warn_falloff_nonvoid_function;
+ bool ReturnsValue = !FT->getReturnType()->isVoidType();
+ bool HasNoReturnAttr = FT->getNoReturnAttr();
-// Don't suggest that virtual functions be marked "noreturn", since they
-// might be overridden by non-noreturn functions.
-bool isVirtualMethod = false;
-if (const CXXMethodDecl *Method = dyn_cast(Func))
- isVirtualMethod = Method->isVirtual();
+ // Short circuit for compilation speed.
+ if (!ReturnsValue && !HasNoReturnAttr) return;
-// Don't suggest that template instantiations be marked "noreturn"
-bool isTemplateInstantiation = false;
-if (const FunctionDecl *Function = dyn_cast(Func))
- isTemplateInstantiation = Function->isTemplateInstantiation();
+ SourceLocation RBrace = Body->getEndLoc();
+ switch (CheckFallThrough(AC)) {
+case MaybeFallThrough:
+ if (HasNoReturnAttr)
+S.Diag(RBrace, diag::err_noreturn_block_has_return_expr);
+ else if (ReturnsValue)
+S.Diag(RBrace, diag::err_maybe_falloff_nonvoid_block);
+ break;
-if (!isVirtualMethod && !isTemplateInstantiation)
- D.diag_NeverFallThroughOrReturn =
-diag::warn_suggest_noreturn_function;
-else
- D.diag_NeverFallThroughOrReturn = 0;
+case AlwaysFallThrough:
+ if (HasNoReturnAttr)
+S.Diag(RBrace, diag::err_noreturn_block_has_return_expr);
+ else if (ReturnsValue)
+S.Diag(RBrace, diag::err_falloff_nonvoid_block);
+ break;
-D.funMode = Function;
-return D;
+case NeverFallThroughOrReturn:
+case NeverFallThrough:
+case UnknownFallThrough:
+ break;
}
+}
- static CheckFallThroughDiagnostics MakeForCoroutine(const Decl *Func) {
-CheckFallThroughDiagnostics D;
-D.FuncLoc = Func->getLocation();
-D.diag_MaybeFallThrough_HasNoReturn = 0;
-D.diag_MaybeFallThrough_ReturnsNonVoid =
-diag::warn_maybe_falloff_nonvoid_coroutine;
-D.diag_AlwaysFallThrough_HasNoReturn = 0;
-D.diag_AlwaysFallThrough_ReturnsNonVoid =
-diag::warn_falloff_nonvoid_coroutine;
-D.funMode = Coroutine;
-return D;
- }
-
- static CheckFallThroughDiagnostics MakeForBlock() {
-CheckFallThroughDiagnostics D;
-D.diag_MaybeFallThrough_HasNoReturn =
- diag::err_noreturn_block_has_return_expr;
-D.diag_MaybeFallThrough_ReturnsNonVoid =
- diag::err_maybe_falloff_nonvoid_block;
-D.diag_AlwaysFallThrough_HasNoReturn =
- diag::err_noreturn_block_has_return_expr;
-D.diag_AlwaysFallThrough_ReturnsNonVoid =
- diag::err_falloff_nonvoid_block;
-D.diag_NeverFallThroughOrReturn = 0;
-D.funMode = Block;
-return D;
- }
-
- static CheckFallThroughDiagnostics MakeForLambda() {
-CheckFallThroughDiagnostics D;
-D.diag_MaybeFallThrough_HasNoReturn =
- diag::err_noreturn_lambda_has_return_expr;
-D.diag_MaybeFallThrough_ReturnsNonVoid =
- diag::warn_maybe_falloff_nonvoid_lambda;
-D.diag_AlwaysFallThrough_HasNoReturn =
- diag::err_noreturn_lambda_has_return_expr;
-D.diag_AlwaysFallThrough_ReturnsNonVoid =
- diag::warn_falloff_nonvoid_lambda;
-D.diag_NeverFallThroughOrReturn = 0;
-D.funMode = Lambda;
-return D;
- }
-
- bool checkDiagnostics(DiagnosticsEngine &D, bool ReturnsVoid,
-bool HasNoReturn) const {
-if (funMode == Function) {
- return (Re
[PATCH] D52338: [analyzer] Process state in checkEndFunction in RetainCountChecker
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342770: [analyzer] Process state in checkEndFunction in
RetainCountChecker (authored by george.karpenkov, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52338?vs=166401&id=166555#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52338
Files:
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
cfe/trunk/test/Analysis/Inputs/expected-plists/retain-release-path-notes.m.plist
cfe/trunk/test/Analysis/retain-release-arc.m
cfe/trunk/test/Analysis/retain-release-path-notes.m
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
@@ -252,7 +252,6 @@
check::PostStmt,
check::PostStmt,
check::PostCall,
-check::PreStmt,
check::RegionChanges,
eval::Assume,
eval::Call > {
@@ -388,8 +387,7 @@
const LocationContext* LCtx,
const CallEvent *Call) const;
- void checkPreStmt(const ReturnStmt *S, CheckerContext &C) const;
- void checkReturnWithRetEffect(const ReturnStmt *S, CheckerContext &C,
+ ExplodedNode* checkReturnWithRetEffect(const ReturnStmt *S, CheckerContext &C,
ExplodedNode *Pred, RetEffect RE, RefVal X,
SymbolRef Sym, ProgramStateRef state) const;
@@ -416,12 +414,20 @@
ProgramStateRef
handleAutoreleaseCounts(ProgramStateRef state, ExplodedNode *Pred,
const ProgramPointTag *Tag, CheckerContext &Ctx,
- SymbolRef Sym, RefVal V) const;
+ SymbolRef Sym,
+ RefVal V,
+ const ReturnStmt *S=nullptr) const;
ExplodedNode *processLeaks(ProgramStateRef state,
SmallVectorImpl &Leaked,
CheckerContext &Ctx,
ExplodedNode *Pred = nullptr) const;
+
+private:
+ /// Perform the necessary checks and state adjustments at the end of the
+ /// function.
+ /// \p S Return statement, may be null.
+ ExplodedNode * processReturn(const ReturnStmt *S, CheckerContext &C) const;
};
//===--===//
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -814,35 +814,35 @@
return true;
}
-//===--===//
-// Handle return statements.
-//===--===//
-
-void RetainCountChecker::checkPreStmt(const ReturnStmt *S,
- CheckerContext &C) const {
+ExplodedNode * RetainCountChecker::processReturn(const ReturnStmt *S,
+ CheckerContext &C) const {
+ ExplodedNode *Pred = C.getPredecessor();
// Only adjust the reference count if this is the top-level call frame,
// and not the result of inlining. In the future, we should do
// better checking even for inlined calls, and see if they match
// with their expected semantics (e.g., the method should return a retained
// object, etc.).
if (!C.inTopFrame())
-return;
+return Pred;
+
+ if (!S)
+return Pred;
const Expr *RetE = S->getRetValue();
if (!RetE)
-return;
+return Pred;
ProgramStateRef state = C.getState();
SymbolRef Sym =
state->getSValAsScalarOrLoc(RetE, C.getLocationContext()).getAsLocSymbol();
if (!Sym)
-return;
+return Pred;
// Get the reference count binding (if any).
const RefVal *T = getRefBinding(state, Sym);
if (!T)
-return;
+return Pred;
// Change the reference count.
RefVal X = *T;
@@ -861,36 +861,35 @@
if (cnt) {
X.setCount(cnt - 1);
X = X ^ RefVal::ReturnedOwned;
- }
- else {
+ } else {
X = X ^ RefVal::ReturnedNotOwned;
}
break;
}
default:
- return;
+ return Pred;
}
// Update the binding.
state = setRefBinding(state, Sym, X);
- ExplodedNode *Pred = C.addTransition(state);
+ Pred = C.addTransition(state);
// At this point we have up
[PATCH] D52337: [analyzer] Highlight sink nodes in red in exploded graph
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342769: [analyzer] Highlight sink nodes in red (authored by
george.karpenkov, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52337?vs=166400&id=166553#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52337
Files:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2957,6 +2957,8 @@
// work.
static std::string getNodeAttributes(const ExplodedNode *N,
ExplodedGraph *G) {
+if (N->isSink())
+ return "color=red";
return {};
}
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2957,6 +2957,8 @@
// work.
static std::string getNodeAttributes(const ExplodedNode *N,
ExplodedGraph *G) {
+if (N->isSink())
+ return "color=red";
return {};
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52337: [analyzer] Highlight sink nodes in red in exploded graph
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342769: [analyzer] Highlight sink nodes in red (authored by
george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D52337?vs=166400&id=166552#toc
Repository:
rC Clang
https://reviews.llvm.org/D52337
Files:
lib/StaticAnalyzer/Core/ExprEngine.cpp
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2957,6 +2957,8 @@
// work.
static std::string getNodeAttributes(const ExplodedNode *N,
ExplodedGraph *G) {
+if (N->isSink())
+ return "color=red";
return {};
}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2957,6 +2957,8 @@
// work.
static std::string getNodeAttributes(const ExplodedNode *N,
ExplodedGraph *G) {
+if (N->isSink())
+ return "color=red";
return {};
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52326: [analyzer] Associate diagnostics created in checkEndFunction with a return statement, if possible
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342768: [analyzer] Associate diagnostics created in
checkEndFunction with a return… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D52326?vs=166542&id=166551#toc
Repository:
rC Clang
https://reviews.llvm.org/D52326
Files:
include/clang/Analysis/ProgramPoint.h
lib/StaticAnalyzer/Core/CheckerManager.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/PathDiagnostic.cpp
test/Analysis/inner-pointer.cpp
test/Analysis/malloc-free-after-return.cpp
Index: include/clang/Analysis/ProgramPoint.h
===
--- include/clang/Analysis/ProgramPoint.h
+++ include/clang/Analysis/ProgramPoint.h
@@ -80,6 +80,7 @@
CallEnterKind,
CallExitBeginKind,
CallExitEndKind,
+ FunctionExitKind,
PreImplicitCallKind,
PostImplicitCallKind,
MinImplicitCallKind = PreImplicitCallKind,
@@ -329,6 +330,29 @@
}
};
+class FunctionExitPoint : public ProgramPoint {
+public:
+ explicit FunctionExitPoint(const ReturnStmt *S,
+ const LocationContext *LC,
+ const ProgramPointTag *tag = nullptr)
+ : ProgramPoint(S, FunctionExitKind, LC, tag) {}
+
+ const CFGBlock *getBlock() const {
+return &getLocationContext()->getCFG()->getExit();
+ }
+
+ const ReturnStmt *getStmt() const {
+return reinterpret_cast(getData1());
+ }
+
+private:
+ friend class ProgramPoint;
+ FunctionExitPoint() = default;
+ static bool isKind(const ProgramPoint &Location) {
+return Location.getKind() == FunctionExitKind;
+ }
+};
+
// PostCondition represents the post program point of a branch condition.
class PostCondition : public PostStmt {
public:
Index: test/Analysis/malloc-free-after-return.cpp
===
--- test/Analysis/malloc-free-after-return.cpp
+++ test/Analysis/malloc-free-after-return.cpp
@@ -17,5 +17,5 @@
int *freeAfterReturnLocal() {
S X;
- return X.getData();
-} // expected-warning {{Use of memory after it is freed}}
+ return X.getData(); // expected-warning {{Use of memory after it is freed}}
+}
Index: test/Analysis/inner-pointer.cpp
===
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -412,8 +412,9 @@
std::string s;
return s.c_str(); // expected-note {{Pointer to inner buffer of 'std::string' obtained here}}
// expected-note@-1 {{Inner buffer of 'std::string' deallocated by call to destructor}}
-} // expected-warning {{Inner pointer of container used after re/deallocation}}
-// expected-note@-1 {{Inner pointer of container used after re/deallocation}}
+// expected-warning@-2 {{Inner pointer of container used after re/deallocation}}
+// expected-note@-3 {{Inner pointer of container used after re/deallocation}}
+}
char *c();
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -774,18 +774,20 @@
}
// Otherwise, see if the node's program point directly points to a statement.
ProgramPoint P = N->getLocation();
- if (Optional SP = P.getAs())
+ if (auto SP = P.getAs())
return SP->getStmt();
- if (Optional BE = P.getAs())
+ if (auto BE = P.getAs())
return BE->getSrc()->getTerminator();
- if (Optional CE = P.getAs())
+ if (auto CE = P.getAs())
return CE->getCallExpr();
- if (Optional CEE = P.getAs())
+ if (auto CEE = P.getAs())
return CEE->getCalleeContext()->getCallSite();
- if (Optional PIPP = P.getAs())
+ if (auto PIPP = P.getAs())
return PIPP->getInitializer()->getInit();
- if (Optional CEB = P.getAs())
+ if (auto CEB = P.getAs())
return CEB->getReturnStmt();
+ if (auto FEP = P.getAs())
+return FEP->getStmt();
return nullptr;
}
@@ -822,17 +824,21 @@
const SourceManager &SM) {
assert(N && "Cannot create a location with a null node.");
const Stmt *S = getStmt(N);
+ const LocationContext *LC = N->getLocationContext();
if (!S) {
// If this is an implicit call, return the implicit call point location.
if (Optional PIE = N->getLocationAs())
return PathDiagnosticLocation(PIE->getLocation(), SM);
+if (auto FE = N->getLocationAs()) {
+ if (const ReturnStmt *RS = FE->getStmt())
+return PathDiagnosticLocation::createBegin(RS, SM, LC);
+}
S = getNextStmt(N);
}
if (S) {
ProgramPoint P = N->getLocation();
-const LocationContext *LC = N->getLocationContext();
/
[PATCH] D52338: [analyzer] Process state in checkEndFunction in RetainCountChecker
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342770: [analyzer] Process state in checkEndFunction in
RetainCountChecker (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D52338?vs=166401&id=166554#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52338
Files:
lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
test/Analysis/Inputs/expected-plists/retain-release-path-notes.m.plist
test/Analysis/retain-release-arc.m
test/Analysis/retain-release-path-notes.m
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -814,35 +814,35 @@
return true;
}
-//===--===//
-// Handle return statements.
-//===--===//
-
-void RetainCountChecker::checkPreStmt(const ReturnStmt *S,
- CheckerContext &C) const {
+ExplodedNode * RetainCountChecker::processReturn(const ReturnStmt *S,
+ CheckerContext &C) const {
+ ExplodedNode *Pred = C.getPredecessor();
// Only adjust the reference count if this is the top-level call frame,
// and not the result of inlining. In the future, we should do
// better checking even for inlined calls, and see if they match
// with their expected semantics (e.g., the method should return a retained
// object, etc.).
if (!C.inTopFrame())
-return;
+return Pred;
+
+ if (!S)
+return Pred;
const Expr *RetE = S->getRetValue();
if (!RetE)
-return;
+return Pred;
ProgramStateRef state = C.getState();
SymbolRef Sym =
state->getSValAsScalarOrLoc(RetE, C.getLocationContext()).getAsLocSymbol();
if (!Sym)
-return;
+return Pred;
// Get the reference count binding (if any).
const RefVal *T = getRefBinding(state, Sym);
if (!T)
-return;
+return Pred;
// Change the reference count.
RefVal X = *T;
@@ -861,36 +861,35 @@
if (cnt) {
X.setCount(cnt - 1);
X = X ^ RefVal::ReturnedOwned;
- }
- else {
+ } else {
X = X ^ RefVal::ReturnedNotOwned;
}
break;
}
default:
- return;
+ return Pred;
}
// Update the binding.
state = setRefBinding(state, Sym, X);
- ExplodedNode *Pred = C.addTransition(state);
+ Pred = C.addTransition(state);
// At this point we have updated the state properly.
// Everything after this is merely checking to see if the return value has
// been over- or under-retained.
// Did we cache out?
if (!Pred)
-return;
+return nullptr;
// Update the autorelease counts.
static CheckerProgramPointTag AutoreleaseTag(this, "Autorelease");
- state = handleAutoreleaseCounts(state, Pred, &AutoreleaseTag, C, Sym, X);
+ state = handleAutoreleaseCounts(state, Pred, &AutoreleaseTag, C, Sym, X, S);
- // Did we cache out?
+ // Have we generated a sink node?
if (!state)
-return;
+return nullptr;
// Get the updated binding.
T = getRefBinding(state, Sym);
@@ -913,10 +912,10 @@
}
}
- checkReturnWithRetEffect(S, C, Pred, RE, X, Sym, state);
+ return checkReturnWithRetEffect(S, C, Pred, RE, X, Sym, state);
}
-void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S,
+ExplodedNode * RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S,
CheckerContext &C,
ExplodedNode *Pred,
RetEffect RE, RefVal X,
@@ -929,31 +928,30 @@
// [self addSubview:_contentView]; // invalidates 'self'
// [_contentView release];
if (X.getIvarAccessHistory() != RefVal::IvarAccessHistory::None)
-return;
+return Pred;
// Any leaks or other errors?
if (X.isReturnedOwned() && X.getCount() == 0) {
if (RE.getKind() != RetEffect::NoRet) {
- bool hasError = false;
if (!RE.isOwned()) {
+
// The returning type is a CF, we expect the enclosing method should
// return ownership.
-hasError = true;
X = X ^ RefVal::ErrorLeakReturned;
- }
- if (hasError) {
// Generate an error node.
state = setRefBinding(state, Sym, X);
static CheckerProgramPointTag ReturnOwnLeakTag(this, "ReturnsOwnLeak");
ExplodedNode *N = C.addTransition(state, Pred, &ReturnOwnLeakTag);
if (N) {
const LangOptions &LOpts = C.getAST
[PATCH] D52269: [analyzer] [NFC] Dead code removal
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342765: [analyzer] [NFC] Dead code removal (authored by
george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D52269?vs=166143&id=166546#toc
Repository:
rC Clang
https://reviews.llvm.org/D52269
Files:
include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/BugReporter.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
Index: lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- lib/StaticAnalyzer/Core/BugReporter.cpp
+++ lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2023,8 +2023,6 @@
void BugType::anchor() {}
-void BugType::FlushReports(BugReporter &BR) {}
-
void BuiltinBug::anchor() {}
//===--===//
@@ -2253,14 +2251,6 @@
if (BugTypes.isEmpty())
return;
- // First flush the warnings for each BugType. This may end up creating new
- // warnings and new BugTypes.
- // FIXME: Only NSErrorChecker needs BugType's FlushReports.
- // Turn NSErrorChecker into a proper checker and remove this.
- SmallVector bugTypes(BugTypes.begin(), BugTypes.end());
- for (const auto I : bugTypes)
-const_cast(I)->FlushReports(*this);
-
// We need to flush reports in deterministic order to ensure the order
// of the reports is consistent between runs.
for (const auto EQ : EQClassesVector)
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -189,7 +189,7 @@
this),
SymMgr(StateMgr.getSymbolManager()),
svalBuilder(StateMgr.getSValBuilder()), ObjCNoRet(mgr.getASTContext()),
- ObjCGCEnabled(gcEnabled), BR(mgr, *this),
+ BR(mgr, *this),
VisitedCallees(VisitedCalleesIn), HowToInline(HowToInlineIn) {
unsigned TrimInterval = mgr.options.getGraphTrimInterval();
if (TrimInterval != 0) {
@@ -3184,11 +3184,6 @@
if (trim) {
std::vector Src;
-// Flush any outstanding reports to make sure we cover all the nodes.
-// This does not cause them to get displayed.
-for (const auto I : BR)
- const_cast(I)->FlushReports(BR);
-
// Iterate through the reports and get their nodes.
for (BugReporter::EQClasses_iterator
EI = BR.EQClasses_begin(), EE = BR.EQClasses_end(); EI != EE; ++EI) {
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -141,9 +141,6 @@
/// implicitly never returns.
ObjCNoReturn ObjCNoRet;
- /// Whether or not GC is enabled in this analysis.
- bool ObjCGCEnabled;
-
/// The BugReporter associated with this engine. It is important that
/// this object be placed at the very end of member variables so that its
/// destructor is called before the rest of the ExprEngine is destroyed.
@@ -201,8 +198,6 @@
return *currBldrCtx;
}
- bool isObjCGCEnabled() { return ObjCGCEnabled; }
-
const Stmt *getStmt() const;
void GenerateAutoTransition(ExplodedNode *N);
Index: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
@@ -162,10 +162,6 @@
return getSValBuilder().getSymbolManager();
}
- bool isObjCGCEnabled() const {
-return Eng.isObjCGCEnabled();
- }
-
ProgramStateManager &getStateManager() {
return Eng.getStateManager();
}
Index: include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
===
--- include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
+++ include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
@@ -65,8 +65,6 @@
/// by a sink node.
bool isSuppressOnSink() const { return SuppressOnSink; }
void setSuppressOnSink(bool x) { SuppressOnSink = x; }
-
- virtual void FlushReports(BugReporter& BR);
};
class BuiltinBug : public BugType {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342770 - [analyzer] Process state in checkEndFunction in RetainCountChecker
Author: george.karpenkov
Date: Fri Sep 21 13:37:20 2018
New Revision: 342770
URL: http://llvm.org/viewvc/llvm-project?rev=342770&view=rev
Log:
[analyzer] Process state in checkEndFunction in RetainCountChecker
Modify the RetainCountChecker to perform state "adjustments" in
checkEndFunction, as performing work in PreStmt does not
work with destructors.
The previous version made an implicit assumption that no code runs
after the return statement is executed.
rdar://43945028
Differential Revision: https://reviews.llvm.org/D52338
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
cfe/trunk/test/Analysis/Inputs/expected-plists/retain-release-path-notes.m.plist
cfe/trunk/test/Analysis/retain-release-arc.m
cfe/trunk/test/Analysis/retain-release-path-notes.m
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp?rev=342770&r1=342769&r2=342770&view=diff
==
---
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
(original)
+++
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
Fri Sep 21 13:37:20 2018
@@ -814,12 +814,9 @@ bool RetainCountChecker::evalCall(const
return true;
}
-//===--===//
-// Handle return statements.
-//===--===//
-
-void RetainCountChecker::checkPreStmt(const ReturnStmt *S,
- CheckerContext &C) const {
+ExplodedNode * RetainCountChecker::processReturn(const ReturnStmt *S,
+ CheckerContext &C) const {
+ ExplodedNode *Pred = C.getPredecessor();
// Only adjust the reference count if this is the top-level call frame,
// and not the result of inlining. In the future, we should do
@@ -827,22 +824,25 @@ void RetainCountChecker::checkPreStmt(co
// with their expected semantics (e.g., the method should return a retained
// object, etc.).
if (!C.inTopFrame())
-return;
+return Pred;
+
+ if (!S)
+return Pred;
const Expr *RetE = S->getRetValue();
if (!RetE)
-return;
+return Pred;
ProgramStateRef state = C.getState();
SymbolRef Sym =
state->getSValAsScalarOrLoc(RetE, C.getLocationContext()).getAsLocSymbol();
if (!Sym)
-return;
+return Pred;
// Get the reference count binding (if any).
const RefVal *T = getRefBinding(state, Sym);
if (!T)
-return;
+return Pred;
// Change the reference count.
RefVal X = *T;
@@ -861,20 +861,19 @@ void RetainCountChecker::checkPreStmt(co
if (cnt) {
X.setCount(cnt - 1);
X = X ^ RefVal::ReturnedOwned;
- }
- else {
+ } else {
X = X ^ RefVal::ReturnedNotOwned;
}
break;
}
default:
- return;
+ return Pred;
}
// Update the binding.
state = setRefBinding(state, Sym, X);
- ExplodedNode *Pred = C.addTransition(state);
+ Pred = C.addTransition(state);
// At this point we have updated the state properly.
// Everything after this is merely checking to see if the return value has
@@ -882,15 +881,15 @@ void RetainCountChecker::checkPreStmt(co
// Did we cache out?
if (!Pred)
-return;
+return nullptr;
// Update the autorelease counts.
static CheckerProgramPointTag AutoreleaseTag(this, "Autorelease");
- state = handleAutoreleaseCounts(state, Pred, &AutoreleaseTag, C, Sym, X);
+ state = handleAutoreleaseCounts(state, Pred, &AutoreleaseTag, C, Sym, X, S);
- // Did we cache out?
+ // Have we generated a sink node?
if (!state)
-return;
+return nullptr;
// Get the updated binding.
T = getRefBinding(state, Sym);
@@ -913,10 +912,10 @@ void RetainCountChecker::checkPreStmt(co
}
}
- checkReturnWithRetEffect(S, C, Pred, RE, X, Sym, state);
+ return checkReturnWithRetEffect(S, C, Pred, RE, X, Sym, state);
}
-void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S,
+ExplodedNode * RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt
*S,
CheckerContext &C,
ExplodedNode *Pred,
RetEffect RE, RefVal X,
@@ -929,20 +928,17 @@ void RetainCountChecker::checkReturnWith
// [self addSubview:_contentView]; // invalidates 'self'
// [_contentView release];
if (X.getIvarAccessHistory() != RefVal::IvarAccessHistory::None)
-return;
+return Pred;
// Any leaks or other errors?
if (X.isReturnedOwned() && X.get
r342768 - [analyzer] Associate diagnostics created in checkEndFunction with a return statement, if possible
Author: george.karpenkov
Date: Fri Sep 21 13:36:41 2018
New Revision: 342768
URL: http://llvm.org/viewvc/llvm-project?rev=342768&view=rev
Log:
[analyzer] Associate diagnostics created in checkEndFunction with a return
statement, if possible
If not possible, use the last line of the declaration, as before.
Differential Revision: https://reviews.llvm.org/D52326
Modified:
cfe/trunk/include/clang/Analysis/ProgramPoint.h
cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
cfe/trunk/test/Analysis/inner-pointer.cpp
cfe/trunk/test/Analysis/malloc-free-after-return.cpp
Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=342768&r1=342767&r2=342768&view=diff
==
--- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original)
+++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Fri Sep 21 13:36:41 2018
@@ -80,6 +80,7 @@ public:
CallEnterKind,
CallExitBeginKind,
CallExitEndKind,
+ FunctionExitKind,
PreImplicitCallKind,
PostImplicitCallKind,
MinImplicitCallKind = PreImplicitCallKind,
@@ -329,6 +330,29 @@ private:
}
};
+class FunctionExitPoint : public ProgramPoint {
+public:
+ explicit FunctionExitPoint(const ReturnStmt *S,
+ const LocationContext *LC,
+ const ProgramPointTag *tag = nullptr)
+ : ProgramPoint(S, FunctionExitKind, LC, tag) {}
+
+ const CFGBlock *getBlock() const {
+return &getLocationContext()->getCFG()->getExit();
+ }
+
+ const ReturnStmt *getStmt() const {
+return reinterpret_cast(getData1());
+ }
+
+private:
+ friend class ProgramPoint;
+ FunctionExitPoint() = default;
+ static bool isKind(const ProgramPoint &Location) {
+return Location.getKind() == FunctionExitKind;
+ }
+};
+
// PostCondition represents the post program point of a branch condition.
class PostCondition : public PostStmt {
public:
Modified: cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp?rev=342768&r1=342767&r2=342768&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp Fri Sep 21 13:36:41
2018
@@ -446,9 +446,8 @@ void CheckerManager::runCheckersForEndFu
// autotransition for it.
NodeBuilder Bldr(Pred, Dst, BC);
for (const auto checkFn : EndFunctionCheckers) {
-const ProgramPoint &L = BlockEntrance(BC.Block,
- Pred->getLocationContext(),
- checkFn.Checker);
+const ProgramPoint &L =
+FunctionExitPoint(RS, Pred->getLocationContext(), checkFn.Checker);
CheckerContext C(Bldr, Eng, Pred, L);
checkFn(RS, C);
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=342768&r1=342767&r2=342768&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri Sep 21 13:36:41 2018
@@ -2984,6 +2984,17 @@ struct DOTGraphTraits :
<< Loc.castAs().getBlock()->getBlockID();
break;
+case ProgramPoint::FunctionExitKind: {
+ auto FEP = Loc.getAs();
+ Out << "Function Exit: B"
+ << FEP->getBlock()->getBlockID();
+ if (const ReturnStmt *RS = FEP->getStmt()) {
+Out << "\\l Return: S" << RS->getID(Context) << "\\l";
+RS->printPretty(Out, /*helper=*/nullptr, Context.getPrintingPolicy(),
+ /*Indentation=*/2, /*NewlineSymbol=*/"\\l");
+ }
+ break;
+}
case ProgramPoint::BlockExitKind:
assert(false);
break;
Modified: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp?rev=342768&r1=342767&r2=342768&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp Fri Sep 21 13:36:41
2018
@@ -774,18 +774,20 @@ const Stmt *PathDiagnosticLocation::getS
}
// Otherwise, see if the node's program point directly points to a statement.
ProgramPoint P = N->getLocation();
- if (Optional SP = P.getAs())
+ if (auto SP = P.getAs())
return SP->getStmt();
- if (Optional BE = P.getAs())
+ if (auto B
r342769 - [analyzer] Highlight sink nodes in red
Author: george.karpenkov
Date: Fri Sep 21 13:37:01 2018
New Revision: 342769
URL: http://llvm.org/viewvc/llvm-project?rev=342769&view=rev
Log:
[analyzer] Highlight sink nodes in red
Differential Revision: https://reviews.llvm.org/D52337
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=342769&r1=342768&r2=342769&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri Sep 21 13:37:01 2018
@@ -2957,6 +2957,8 @@ struct DOTGraphTraits :
// work.
static std::string getNodeAttributes(const ExplodedNode *N,
ExplodedGraph *G) {
+if (N->isSink())
+ return "color=red";
return {};
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342767 - [analyzer] [NFC] Prefer make_unique over "new"
Author: george.karpenkov
Date: Fri Sep 21 13:36:21 2018
New Revision: 342767
URL: http://llvm.org/viewvc/llvm-project?rev=342767&view=rev
Log:
[analyzer] [NFC] Prefer make_unique over "new"
Differential Revision: https://reviews.llvm.org/D52336
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp?rev=342767&r1=342766&r2=342767&view=diff
==
---
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
(original)
+++
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
Fri Sep 21 13:36:21 2018
@@ -750,9 +750,8 @@ void RetainCountChecker::processNonLeakE
}
assert(BT);
- auto report = std::unique_ptr(
- new CFRefReport(*BT, C.getASTContext().getLangOpts(),
- SummaryLog, N, Sym));
+ auto report = llvm::make_unique(
+ *BT, C.getASTContext().getLangOpts(), SummaryLog, N, Sym);
report->addRange(ErrorRange);
C.emitReport(std::move(report));
}
@@ -951,9 +950,9 @@ void RetainCountChecker::checkReturnWith
ExplodedNode *N = C.addTransition(state, Pred, &ReturnOwnLeakTag);
if (N) {
const LangOptions &LOpts = C.getASTContext().getLangOpts();
- C.emitReport(std::unique_ptr(new CFRefLeakReport(
- *getLeakAtReturnBug(LOpts), LOpts,
- SummaryLog, N, Sym, C, IncludeAllocationLine)));
+ C.emitReport(llvm::make_unique(
+ *getLeakAtReturnBug(LOpts), LOpts, SummaryLog, N, Sym, C,
+ IncludeAllocationLine));
}
}
}
@@ -978,9 +977,9 @@ void RetainCountChecker::checkReturnWith
if (!returnNotOwnedForOwned)
returnNotOwnedForOwned.reset(new ReturnedNotOwnedForOwned(this));
- C.emitReport(std::unique_ptr(new CFRefReport(
+ C.emitReport(llvm::make_unique(
*returnNotOwnedForOwned, C.getASTContext().getLangOpts(),
- SummaryLog, N, Sym)));
+ SummaryLog, N, Sym));
}
}
}
@@ -1182,9 +1181,8 @@ RetainCountChecker::handleAutoreleaseCou
overAutorelease.reset(new OverAutorelease(this));
const LangOptions &LOpts = Ctx.getASTContext().getLangOpts();
-Ctx.emitReport(std::unique_ptr(
-new CFRefReport(*overAutorelease, LOpts,
-SummaryLog, N, Sym, os.str(;
+Ctx.emitReport(llvm::make_unique(
+*overAutorelease, LOpts, SummaryLog, N, Sym, os.str()));
}
return nullptr;
@@ -1235,9 +1233,8 @@ RetainCountChecker::processLeaks(Program
: getLeakAtReturnBug(LOpts);
assert(BT && "BugType not initialized.");
- Ctx.emitReport(std::unique_ptr(
- new CFRefLeakReport(*BT, LOpts, SummaryLog, N, *I, Ctx,
- IncludeAllocationLine)));
+ Ctx.emitReport(llvm::make_unique(
+ *BT, LOpts, SummaryLog, N, *I, Ctx, IncludeAllocationLine));
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342766 - [analyzer] Fix bug in isInevitablySinking
Author: george.karpenkov
Date: Fri Sep 21 13:36:01 2018
New Revision: 342766
URL: http://llvm.org/viewvc/llvm-project?rev=342766&view=rev
Log:
[analyzer] Fix bug in isInevitablySinking
If the non-sink report is generated at the exit node, it will be
suppressed by the current functionality in isInevitablySinking, as it
only checks the successors of the block, but not the block itself.
The bug shows up in RetainCountChecker checks.
Differential Revision: https://reviews.llvm.org/D52284
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=342766&r1=342765&r2=342766&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Fri Sep 21 13:36:01 2018
@@ -2812,16 +2812,15 @@ static bool isInevitablySinking(const Ex
DFSWorkList.pop_back();
Visited.insert(Blk);
+// If at least one path reaches the CFG exit, it means that control is
+// returned to the caller. For now, say that we are not sure what
+// happens next. If necessary, this can be improved to analyze
+// the parent StackFrameContext's call site in a similar manner.
+if (Blk == &Cfg.getExit())
+ return false;
+
for (const auto &Succ : Blk->succs()) {
if (const CFGBlock *SuccBlk = Succ.getReachableBlock()) {
-if (SuccBlk == &Cfg.getExit()) {
- // If at least one path reaches the CFG exit, it means that control
is
- // returned to the caller. For now, say that we are not sure what
- // happens next. If necessary, this can be improved to analyze
- // the parent StackFrameContext's call site in a similar manner.
- return false;
-}
-
if (!isImmediateSinkBlock(SuccBlk) && !Visited.count(SuccBlk)) {
// If the block has reachable child blocks that aren't no-return,
// add them to the worklist.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342765 - [analyzer] [NFC] Dead code removal
Author: george.karpenkov
Date: Fri Sep 21 13:35:39 2018
New Revision: 342765
URL: http://llvm.org/viewvc/llvm-project?rev=342765&view=rev
Log:
[analyzer] [NFC] Dead code removal
Differential Revision: https://reviews.llvm.org/D52269
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h?rev=342765&r1=342764&r2=342765&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h Fri Sep
21 13:35:39 2018
@@ -65,8 +65,6 @@ public:
/// by a sink node.
bool isSuppressOnSink() const { return SuppressOnSink; }
void setSuppressOnSink(bool x) { SuppressOnSink = x; }
-
- virtual void FlushReports(BugReporter& BR);
};
class BuiltinBug : public BugType {
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h?rev=342765&r1=342764&r2=342765&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
Fri Sep 21 13:35:39 2018
@@ -162,10 +162,6 @@ public:
return getSValBuilder().getSymbolManager();
}
- bool isObjCGCEnabled() const {
-return Eng.isObjCGCEnabled();
- }
-
ProgramStateManager &getStateManager() {
return Eng.getStateManager();
}
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=342765&r1=342764&r2=342765&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Fri
Sep 21 13:35:39 2018
@@ -141,9 +141,6 @@ private:
/// implicitly never returns.
ObjCNoReturn ObjCNoRet;
- /// Whether or not GC is enabled in this analysis.
- bool ObjCGCEnabled;
-
/// The BugReporter associated with this engine. It is important that
/// this object be placed at the very end of member variables so that its
/// destructor is called before the rest of the ExprEngine is destroyed.
@@ -201,8 +198,6 @@ public:
return *currBldrCtx;
}
- bool isObjCGCEnabled() { return ObjCGCEnabled; }
-
const Stmt *getStmt() const;
void GenerateAutoTransition(ExplodedNode *N);
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=342765&r1=342764&r2=342765&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Fri Sep 21 13:35:39 2018
@@ -2023,8 +2023,6 @@ static std::unique_ptr g
void BugType::anchor() {}
-void BugType::FlushReports(BugReporter &BR) {}
-
void BuiltinBug::anchor() {}
//===--===//
@@ -2253,14 +2251,6 @@ void BugReporter::FlushReports() {
if (BugTypes.isEmpty())
return;
- // First flush the warnings for each BugType. This may end up creating new
- // warnings and new BugTypes.
- // FIXME: Only NSErrorChecker needs BugType's FlushReports.
- // Turn NSErrorChecker into a proper checker and remove this.
- SmallVector bugTypes(BugTypes.begin(), BugTypes.end());
- for (const auto I : bugTypes)
-const_cast(I)->FlushReports(*this);
-
// We need to flush reports in deterministic order to ensure the order
// of the reports is consistent between runs.
for (const auto EQ : EQClassesVector)
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=342765&r1=342764&r2=342765&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri Sep 21 13:35:39 2018
@@ -189,7 +189,7 @@ ExprEngine::ExprEngine(cross_tu::CrossTr
this),
SymMgr(State
[PATCH] D52377: [HIP] Support early finalization of device code
yaxunl created this revision.
yaxunl added a reviewer: tra.
This patch introduced a driver option `--hip-early-finalize`. When enabled,
clang will assume the device code in each translation unit does not call
external functions except those in the device library, therefore it is possible
to compile the device code in each translation unit to self-contained kernels
and embed them in the host object, so that the host object behaves like
usual host object which can be linked by lld.
The benefits of this feature is: 1. allow users to create static libraries which
can be linked by host linker; 2. amortized device code linking time.
This patch modifies HIP action builder to insert actions for linking device
code and generating HIP fatbin, and pass HIP fatbin to host backend action.
It extracts code for constructing command for generating HIP fatbin as
a function so that it can be reused by early finalization. It also modifies
codegen of HIP host constructor functions to embed the device fatbin
when it is available.
https://reviews.llvm.org/D52377
Files:
include/clang/Driver/Options.td
include/clang/Driver/Types.def
lib/CodeGen/CGCUDANV.cpp
lib/Driver/Driver.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Driver/ToolChains/CommonArgs.cpp
lib/Driver/ToolChains/HIP.cpp
lib/Driver/ToolChains/HIP.h
test/CodeGenCUDA/device-stub.cu
test/Driver/cuda-phases.cu
test/Driver/hip-early-finalize.hip
test/Driver/hip-toolchain.hip
Index: test/Driver/hip-toolchain.hip
===
--- test/Driver/hip-toolchain.hip
+++ test/Driver/hip-toolchain.hip
@@ -80,7 +80,7 @@
// CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
// CHECK-SAME: "-targets={{.*}},hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900"
-// CHECK-SAME: "-inputs={{.*}},[[IMG_DEV1]],[[IMG_DEV2]]" "-outputs=[[BUNDLE:.*o]]"
+// CHECK-SAME: "-inputs={{.*}},[[IMG_DEV1]],[[IMG_DEV2]]" "-outputs=[[BUNDLE:.*hipfb]]"
// CHECK: [[LD:".*ld.*"]] {{.*}} [[A_OBJ_HOST]] [[B_OBJ_HOST]]
// CHECK-SAME: {{.*}} "-T" "{{.*}}.lk"
Index: test/Driver/hip-early-finalize.hip
===
--- /dev/null
+++ test/Driver/hip-early-finalize.hip
@@ -0,0 +1,150 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu --hip-early-finalize \
+// RUN: -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
+// RUN: --hip-device-lib=lib1.bc --hip-device-lib=lib2.bc \
+// RUN: --hip-device-lib-path=%S/Inputs/hip_multiple_inputs/lib1 \
+// RUN: --hip-device-lib-path=%S/Inputs/hip_multiple_inputs/lib2 \
+// RUN: -fuse-ld=lld \
+// RUN: %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN: %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck -check-prefixes=CHECK %s
+
+//
+// Compile device code in a.cu to code object for gfx803.
+//
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx803"
+// CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
+// CHECK-SAME: {{.*}} "-o" [[A_BC_803:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
+
+// CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_803]]
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
+// CHECK-SAME: "-o" [[LINKED_BC_DEV_A_803:".*-gfx803-linked-.*bc"]]
+
+// CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_803]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx803"
+// CHECK-SAME: "-o" [[OPT_BC_DEV_A_803:".*-gfx803-optimized.*bc"]]
+
+// CHECK: [[LLC: ".*llc"]] [[OPT_BC_DEV_A_803]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-filetype=obj" "-mcpu=gfx803" "-o" [[OBJ_DEV_A_803:".*-gfx803-.*o"]]
+
+// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK-SAME: "-o" "[[IMG_DEV_A_803:.*out]]" [[OBJ_DEV_A_803]]
+
+//
+// Compile device code in a.cu to code object for gfx900.
+//
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx900"
+// CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
+// CHECK-SAME: {{.*}} "-o" [[A_BC_900:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[A_SRC]]
+
+// CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_900]]
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
+// CHECK-SAME: "-o" [[LINKED_BC_DEV_A_900:".*-gfx900-linked-.*bc"]]
+
+// CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_900]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx900"
+// CHECK-SAME: "-o" [[OPT_BC_DEV_A_900:".*-gfx900-optimized.*bc"]]
+
+// CHECK: [[LLC: ".*llc"]] [[OPT_BC_DEV_A_900]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-filetype=obj" "-mcpu=gfx900" "-o" [[OBJ_DEV_A_900:".*-gfx900-.*o"]]
+
+// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "--no-u
[PATCH] D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`.
kristina added a comment. Would you be okay with me renaming `cfstring` to `.cfstring` for ELF so it's in line with ELF section naming conventions (I'm not entirely sure if that could cause issues with ObjC stuff)? And yes I think it's best to avoid that code-path altogether if it turns out to be a constant as that's likely from the declaration of the type, I'll write a FileCheck test in a moment and check that I can apply the same logic to ELF aside from the DLL stuff as CoreFoundation needs to export the symbol somehow while preserving the implicit extern attribute for every other TU that comes from the builtin that `CFSTR` expands to. Is what I'm suggesting making sense? If not, let me know, I may be misunderstanding what's happening here. I'll add the test and another revision in a second, maybe that will clear it up a bit. Comment at: lib/CodeGen/CodeGenModule.cpp:4108 + // consistent with old behavior for this target as it would fail + // on the cast<> instead. + assert(GV && "isa isn't a GlobalValue"); rjmccall wrote: > compnerd wrote: > > I think that the comment isn't particularly helpful - it basically is > > directing you to look at the history of the file. > I think we should just skip this step, even on COFF, if it's not a > `GlobalValue`. > > Probably the most correct move would be to only apply this logic if the IR > declaration was synthesized. Or maybe even just change this code to look for > a global variable called `__CFConstantStringClassReference` in the first > place and then emit a reference to it if found, and only otherwise try to > build the synthetic variable. But just bailing out early if something weird > is going on is also a legitimate step. Doesn't this code practically do this since creating a runtime global is a no-op if it already exists within the module, however there is potentially one module which can have it which is CFString.c (assuming no bridging here of any kind) while it makes sense for the rest to refer to it as an implicit extern (in which case I need to add an ELF code path to do exactly the same. Repository: rC Clang https://reviews.llvm.org/D52344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45639: [Driver] Support default libc++ library location on Darwin
phosek added a comment. In https://reviews.llvm.org/D45639#1193112, @ldionne wrote: > @phosek I don't understand how you can expect code compiled with new headers > to link against an old dylib, unless you're setting the target platform, in > which case anything that would fail to link will instead be a compiler error. > I mean, we're adding stuff to the dylib from one version to the next, so _of > course_ it won't always work. @ldionne it's the other way round. Today, Clang driver on macOS automatically uses headers that are part of the compiler distribution (i.e. `-I../include/c++/v1`) but it always links against the system library (`/usr/lib/libc++.dylib`) because the library path to libraries that are shipped with the compiler (`-L../lib`) is never added to the link-line. Those two may not necessarily be the same version, and almost certainly won't be if you use Clang build from trunk. We hit this case again recently where developers are trying to use libc++ filesystem library. Using `#include ` just works but `-lc++fs` fails because that library is not shipped as part of macOS at the moment, so developers need to add explicit `-L/../lib` on macOS. This is not the case on other platforms such as Linux. The purpose of this patch is to make the driver behave more consistently across platforms so developers who build their own Clang don't need to explicitly pass `-L/../lib` on macOS. Repository: rC Clang https://reviews.llvm.org/D45639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52368: [libc++abi] is_strcmp parameter to is_equal is unused for WIN32
This revision was automatically updated to reflect the committed changes. Closed by commit rL342764: [libc++abi] is_strcmp parameter to is_equal is unused for WIN32 (authored by pirama, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D52368 Files: libcxxabi/trunk/src/private_typeinfo.cpp Index: libcxxabi/trunk/src/private_typeinfo.cpp === --- libcxxabi/trunk/src/private_typeinfo.cpp +++ libcxxabi/trunk/src/private_typeinfo.cpp @@ -64,6 +64,7 @@ return x == y; return strcmp(x->name(), y->name()) == 0; #else +(void) use_strcmp; return (x == y) || (strcmp(x->name(), y->name()) == 0); #endif } Index: libcxxabi/trunk/src/private_typeinfo.cpp === --- libcxxabi/trunk/src/private_typeinfo.cpp +++ libcxxabi/trunk/src/private_typeinfo.cpp @@ -64,6 +64,7 @@ return x == y; return strcmp(x->name(), y->name()) == 0; #else +(void) use_strcmp; return (x == y) || (strcmp(x->name(), y->name()) == 0); #endif } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`.
rjmccall added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:4108 + // consistent with old behavior for this target as it would fail + // on the cast<> instead. + assert(GV && "isa isn't a GlobalValue"); compnerd wrote: > I think that the comment isn't particularly helpful - it basically is > directing you to look at the history of the file. I think we should just skip this step, even on COFF, if it's not a `GlobalValue`. Probably the most correct move would be to only apply this logic if the IR declaration was synthesized. Or maybe even just change this code to look for a global variable called `__CFConstantStringClassReference` in the first place and then emit a reference to it if found, and only otherwise try to build the synthetic variable. But just bailing out early if something weird is going on is also a legitimate step. Repository: rC Clang https://reviews.llvm.org/D52344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52368: [libc++abi] is_strcmp parameter to is_equal is unused for WIN32
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM. Repository: rCXXA libc++abi https://reviews.llvm.org/D52368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r342734 - NFC: deduplicate isRepeatedBytePattern from clang to LLVM's isBytewiseValue
Hello, please add linebreaks to your commit messages. This commit makes
`git log` look pretty ugly.
On Fri, Sep 21, 2018 at 9:55 AM JF Bastien via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: jfb
> Date: Fri Sep 21 06:54:09 2018
> New Revision: 342734
>
> URL: http://llvm.org/viewvc/llvm-project?rev=342734&view=rev
> Log:
> NFC: deduplicate isRepeatedBytePattern from clang to LLVM's isBytewiseValue
>
> Summary:
> This code was in CGDecl.cpp and really belongs in LLVM. It happened to
> have isBytewiseValue which served a very similar purpose but wasn't as
> powerful as clang's version. Remove the clang version, and augment
> isBytewiseValue to be as powerful so that clang does the same thing it used
> to.
>
> LLVM part of this patch: D51751
>
> Subscribers: dexonsmith, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D51752
>
> Modified:
> cfe/trunk/lib/CodeGen/CGDecl.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=342734&r1=342733&r2=342734&view=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDecl.cpp Fri Sep 21 06:54:09 2018
> @@ -30,6 +30,7 @@
> #include "clang/Basic/TargetInfo.h"
> #include "clang/CodeGen/CGFunctionInfo.h"
> #include "clang/Frontend/CodeGenOptions.h"
> +#include "llvm/Analysis/ValueTracking.h"
> #include "llvm/IR/DataLayout.h"
> #include "llvm/IR/GlobalVariable.h"
> #include "llvm/IR/Intrinsics.h"
> @@ -948,111 +949,17 @@ static bool shouldUseBZeroPlusStoresToIn
> canEmitInitWithFewStoresAfterBZero(Init, StoreBudget);
> }
>
> -/// A byte pattern.
> -///
> -/// Can be "any" pattern if the value was padding or known to be undef.
> -/// Can be "none" pattern if a sequence doesn't exist.
> -class BytePattern {
> - uint8_t Val;
> - enum class ValueType : uint8_t { Specific, Any, None } Type;
> - BytePattern(ValueType Type) : Type(Type) {}
> -
> -public:
> - BytePattern(uint8_t Value) : Val(Value), Type(ValueType::Specific) {}
> - static BytePattern Any() { return BytePattern(ValueType::Any); }
> - static BytePattern None() { return BytePattern(ValueType::None); }
> - bool isAny() const { return Type == ValueType::Any; }
> - bool isNone() const { return Type == ValueType::None; }
> - bool isValued() const { return Type == ValueType::Specific; }
> - uint8_t getValue() const {
> -assert(isValued());
> -return Val;
> - }
> - BytePattern merge(const BytePattern Other) const {
> -if (isNone() || Other.isNone())
> - return None();
> -if (isAny())
> - return Other;
> -if (Other.isAny())
> - return *this;
> -if (getValue() == Other.getValue())
> - return *this;
> -return None();
> - }
> -};
> -
> -/// Figures out whether the constant can be initialized with memset.
> -static BytePattern constantIsRepeatedBytePattern(llvm::Constant *C) {
> - if (isa(C) ||
> isa(C))
> -return BytePattern(0x00);
> - if (isa(C))
> -return BytePattern::Any();
> -
> - if (isa(C)) {
> -auto *Int = cast(C);
> -if (Int->getBitWidth() % 8 != 0)
> - return BytePattern::None();
> -const llvm::APInt &Value = Int->getValue();
> -if (Value.isSplat(8))
> - return BytePattern(Value.getLoBits(8).getLimitedValue());
> -return BytePattern::None();
> - }
> -
> - if (isa(C)) {
> -auto *FP = cast(C);
> -llvm::APInt Bits = FP->getValueAPF().bitcastToAPInt();
> -if (Bits.getBitWidth() % 8 != 0)
> - return BytePattern::None();
> -if (!Bits.isSplat(8))
> - return BytePattern::None();
> -return BytePattern(Bits.getLimitedValue() & 0xFF);
> - }
> -
> - if (isa(C)) {
> -llvm::Constant *Splat =
> cast(C)->getSplatValue();
> -if (Splat)
> - return constantIsRepeatedBytePattern(Splat);
> -return BytePattern::None();
> - }
> -
> - if (isa(C) || isa(C)) {
> -BytePattern Pattern(BytePattern::Any());
> -for (unsigned I = 0, E = C->getNumOperands(); I != E; ++I) {
> - llvm::Constant *Elt = cast(C->getOperand(I));
> - Pattern = Pattern.merge(constantIsRepeatedBytePattern(Elt));
> - if (Pattern.isNone())
> -return Pattern;
> -}
> -return Pattern;
> - }
> -
> - if (llvm::ConstantDataSequential *CDS =
> - dyn_cast(C)) {
> -BytePattern Pattern(BytePattern::Any());
> -for (unsigned I = 0, E = CDS->getNumElements(); I != E; ++I) {
> - llvm::Constant *Elt = CDS->getElementAsConstant(I);
> - Pattern = Pattern.merge(constantIsRepeatedBytePattern(Elt));
> - if (Pattern.isNone())
> -return Pattern;
> -}
> -return Pattern;
> - }
> -
> - // BlockAddress, ConstantExpr, and everything else is scary.
> - return BytePattern::None();
> -}
> -
> /// Decide whether we should use memset to initialize a local variable
> instead
> /// of using a memcpy from a constant global. Assum
[PATCH] D52136: [clang-tidy] Add modernize-concat-nested-namespaces check
wgml marked 2 inline comments as done.
wgml added inline comments.
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:52
+const NamespaceContextVec &Namespaces) {
+ std::ostringstream Result;
+ bool First = true;
aaron.ballman wrote:
> Can this be rewritten with `llvm::for_each()` and a `Twine` so that we don't
> have to use `ostringstream` (which is a big hammer for this).
The main advantage of `stringstream` was that it is concise.
I don't think I can effectively use `Twine` to build a result in a loop. If I'm
wrong, correct me, please.
I reworked `concatNamespaces` to use `SmallString` with another educated guess
of `40` for capacity.
Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.h:29
+private:
+ using NamespaceContextVec = llvm::SmallVector;
+
aaron.ballman wrote:
> Why 6?
Educated guess. I considered the codebases I usually work with and it seemed a
sane value in that context.
Comment at: docs/clang-tidy/checks/list.rst:13
abseil-str-cat-append
+ abseil-string-find-startswith
android-cloexec-accept
aaron.ballman wrote:
> This is an unrelated change -- feel free to make a separate commit that fixes
> this (no review needed).
Setup script did that I guess. I reverted the change.
https://reviews.llvm.org/D52136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52136: [clang-tidy] Add modernize-concat-nested-namespaces check
wgml updated this revision to Diff 166529.
wgml marked 4 inline comments as done.
wgml edited the summary of this revision.
https://reviews.llvm.org/D52136
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
clang-tidy/modernize/ConcatNestedNamespacesCheck.h
clang-tidy/modernize/ModernizeTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
test/clang-tidy/modernize-concat-nested-namespaces.cpp
Index: test/clang-tidy/modernize-concat-nested-namespaces.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-concat-nested-namespaces.cpp
@@ -0,0 +1,161 @@
+// RUN: %check_clang_tidy %s modernize-concat-nested-namespaces %t -- -- -std=c++17
+
+namespace n1 {}
+
+namespace n2 {
+namespace n3 {
+void t();
+}
+namespace n4 {
+void t();
+}
+} // namespace n2
+
+namespace n5 {
+inline namespace n6 {
+void t();
+}
+} // namespace n5
+
+namespace n7 {
+void t();
+
+namespace n8 {
+void t();
+}
+} // namespace n7
+
+namespace n9 {
+namespace n10 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n9::n10
+void t();
+} // namespace n10
+} // namespace n9
+// CHECK-FIXES: }
+
+namespace n11 {
+namespace n12 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n11::n12
+namespace n13 {
+void t();
+}
+namespace n14 {
+void t();
+}
+} // namespace n12
+} // namespace n11
+// CHECK-FIXES: }
+
+namespace n15 {
+namespace n16 {
+void t();
+}
+
+inline namespace n17 {
+void t();
+}
+
+namespace n18 {
+namespace n19 {
+namespace n20 {
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n18::n19::n20
+void t();
+} // namespace n20
+} // namespace n19
+} // namespace n18
+// CHECK-FIXES: }
+
+namespace n21 {
+void t();
+}
+} // namespace n15
+
+namespace n22 {
+namespace {
+void t();
+}
+} // namespace n22
+
+namespace n23 {
+namespace {
+namespace n24 {
+namespace n25 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n24::n25
+void t();
+} // namespace n25
+} // namespace n24
+// CHECK-FIXES: }
+} // namespace
+} // namespace n23
+
+namespace n26::n27 {
+namespace n28 {
+namespace n29::n30 {
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n26::n27::n28::n29::n30
+void t() {}
+} // namespace n29::n30
+} // namespace n28
+} // namespace n26::n27
+// CHECK-FIXES: }
+
+namespace n31 {
+namespace n32 {}
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+} // namespace n31
+// CHECK-FIXES-EMPTY
+
+namespace n33 {
+namespace n34 {
+namespace n35 {}
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+} // namespace n34
+// CHECK-FIXES-EMPTY
+namespace n36 {
+void t();
+}
+} // namespace n33
+
+namespace n37::n38 {
+void t();
+}
+
+#define IEXIST
+namespace n39 {
+namespace n40 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n39::n40
+#ifdef IEXIST
+void t() {}
+#endif
+} // namespace n40
+} // namespace n39
+// CHECK-FIXES: }
+
+namespace n41 {
+namespace n42 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n41::n42
+#ifdef IDONTEXIST
+void t() {}
+#endif
+} // namespace n42
+} // namespace n41
+// CHECK-FIXES: }
+
+int main() {
+ n26::n27::n28::n29::n30::t();
+#ifdef IEXIST
+ n39::n40::t();
+#endif
+
+#ifdef IDONTEXIST
+ n41::n42::t();
+#endif
+
+ return 0;
+}
Index: docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
@@ -0,0 +1,49 @@
+.. title:: clang-tidy - modernize-concat-nested-namespaces
+
+modernize-concat-nested-namespaces
+==
+
+Checks for use of nested namespaces in a form of ``namespace a { namespace b { ... } }``
+and offers change to syntax introduced in C++17: ``namespace a::b { ... }``.
+Inlined namespaces are not modified.
+
+For example:
+
+.. code-block:: c++
+
+ namespace n1 {
+ namespace n2 {
+ void t();
+ }
+ }
+
+ namespace n3 {
+ namespace n4 {
+ namespace n5 {
+ void t();
+ }
+ }
+ namespace n6 {
+ namespace n7 {
+ void t();
+ }
+ }
+ }
+
+Will be modified to:
+
+.. code-block:: c++
+
+ namespace n1
[PATCH] D52313: [clang-doc] Avoid parsing undefined base classes
juliehockett updated this revision to Diff 166526.
juliehockett added a comment.
Adding test case
https://reviews.llvm.org/D52313
Files:
clang-tools-extra/clang-doc/Serialize.cpp
clang-tools-extra/test/clang-doc/bc-record.cpp
clang-tools-extra/test/clang-doc/mapper-record.cpp
clang-tools-extra/test/clang-doc/md-record.cpp
clang-tools-extra/test/clang-doc/public-record.cpp
clang-tools-extra/test/clang-doc/test_cases/record.cpp
clang-tools-extra/test/clang-doc/yaml-record.cpp
Index: clang-tools-extra/test/clang-doc/yaml-record.cpp
===
--- clang-tools-extra/test/clang-doc/yaml-record.cpp
+++ clang-tools-extra/test/clang-doc/yaml-record.cpp
@@ -39,6 +39,8 @@
class Y {};
};
+class G;
+
// RUN: clang-doc --format=yaml --doxygen --extra-arg=-fmodules-ts -p %t %t/test.cpp -output=%t/docs
@@ -90,133 +92,143 @@
// CHECK-2-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
// CHECK-2-NEXT: ...
-// RUN: cat %t/docs/./E.yaml | FileCheck %s --check-prefix CHECK-3
+// RUN: cat %t/docs/./G.yaml | FileCheck %s --check-prefix CHECK-3
// CHECK-3: ---
// CHECK-3-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
-// CHECK-3-NEXT: Name:'E'
-// CHECK-3-NEXT: DefLocation:
-// CHECK-3-NEXT: LineNumber: 25
-// CHECK-3-NEXT: Filename:'test'
+// CHECK-3-NEXT: Name:'G'
+// CHECK-3-NEXT: Location:
+// CHECK-3-NEXT: - LineNumber: 42
+// CHECK-3-NEXT: Filename:'test'
// CHECK-3-NEXT: TagType: Class
-// CHECK-3-NEXT: ChildFunctions:
-// CHECK-3-NEXT: - USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
-// CHECK-3-NEXT: Name:'E'
-// CHECK-3-NEXT: Namespace:
-// CHECK-3-NEXT: - Type:Record
-// CHECK-3-NEXT: Name:'E'
-// CHECK-3-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
-// CHECK-3-NEXT: DefLocation:
-// CHECK-3-NEXT: LineNumber: 27
-// CHECK-3-NEXT: Filename:'test'
-// CHECK-3-NEXT: IsMethod:true
-// CHECK-3-NEXT: Parent:
-// CHECK-3-NEXT: Type:Record
-// CHECK-3-NEXT: Name:'E'
-// CHECK-3-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
-// CHECK-3-NEXT: ReturnType:
-// CHECK-3-NEXT: Type:
-// CHECK-3-NEXT: Name:'void'
-// CHECK-3-NEXT: - USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
-// CHECK-3-NEXT: Name:'~E'
-// CHECK-3-NEXT: Namespace:
-// CHECK-3-NEXT: - Type:Record
-// CHECK-3-NEXT: Name:'E'
-// CHECK-3-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
-// CHECK-3-NEXT: DefLocation:
-// CHECK-3-NEXT: LineNumber: 28
-// CHECK-3-NEXT: Filename:'test'
-// CHECK-3-NE
[PATCH] D52368: [libc++abi] is_strcmp parameter to is_equal is unused for WIN32
pirama updated this revision to Diff 166527. pirama added a comment. Simplify patch. Repository: rCXXA libc++abi https://reviews.llvm.org/D52368 Files: src/private_typeinfo.cpp Index: src/private_typeinfo.cpp === --- src/private_typeinfo.cpp +++ src/private_typeinfo.cpp @@ -64,6 +64,7 @@ return x == y; return strcmp(x->name(), y->name()) == 0; #else +(void) use_strcmp; return (x == y) || (strcmp(x->name(), y->name()) == 0); #endif } Index: src/private_typeinfo.cpp === --- src/private_typeinfo.cpp +++ src/private_typeinfo.cpp @@ -64,6 +64,7 @@ return x == y; return strcmp(x->name(), y->name()) == 0; #else +(void) use_strcmp; return (x == y) || (strcmp(x->name(), y->name()) == 0); #endif } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`.
compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. This looks fine to me, but this definitely should have an accompanying test. As to the other items you mention, the current section naming actually is important as it enables the CFString structures to be located (the linker will create `__start_cfstring` and `__stop_cfstring` synthetic symbols to mark the beginning and ending of the section), allowing the runtime to get to the section (the section is meant to be an array of objects). Additionally, the section is not marked as constant because the runtime can actually change the contents (`CFConstantString` is *not* constant as you would think, the underlying value can be changed as can the `isa` pointer). Comment at: lib/CodeGen/CodeGenModule.cpp:4101 Ty = llvm::ArrayType::get(Ty, 0); -llvm::GlobalValue *GV = cast( -CreateRuntimeVariable(Ty, "__CFConstantStringClassReference")); - +llvm::Constant *CGV = +CreateRuntimeVariable(Ty, "__CFConstantStringClassReference"); `CGV` doesn't really follow the expectation (`GV` was short for `GlobalValue`, this is a `Constant` so `C` seems more appropriate). Comment at: lib/CodeGen/CodeGenModule.cpp:4108 + // consistent with old behavior for this target as it would fail + // on the cast<> instead. + assert(GV && "isa isn't a GlobalValue"); I think that the comment isn't particularly helpful - it basically is directing you to look at the history of the file. Repository: rC Clang https://reviews.llvm.org/D52344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52323: Add necessary support for storing code-model to module IR.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342758: Add necessary support for storing code-model to
module IR. (authored by ctice, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52323?vs=166492&id=166523#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52323
Files:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/test/CodeGen/codemodels.c
Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp
@@ -44,6 +44,7 @@
#include "clang/CodeGen/ConstantInitBuilder.h"
#include "clang/Frontend/CodeGenOptions.h"
#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/ADT/Triple.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/IR/CallSite.h"
@@ -53,6 +54,7 @@
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Module.h"
#include "llvm/ProfileData/InstrProfReader.h"
+#include "llvm/Support/CodeGen.h"
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/MD5.h"
@@ -556,6 +558,20 @@
getModule().setPIELevel(static_cast(PLevel));
}
+ if (getCodeGenOpts().CodeModel.size() > 0) {
+unsigned CM = llvm::StringSwitch(getCodeGenOpts().CodeModel)
+ .Case("tiny", llvm::CodeModel::Tiny)
+ .Case("small", llvm::CodeModel::Small)
+ .Case("kernel", llvm::CodeModel::Kernel)
+ .Case("medium", llvm::CodeModel::Medium)
+ .Case("large", llvm::CodeModel::Large)
+ .Default(~0u);
+if (CM != ~0u) {
+ llvm::CodeModel::Model codeModel =
static_cast(CM);
+ getModule().setCodeModel(codeModel);
+}
+ }
+
if (CodeGenOpts.NoPLT)
getModule().setRtLibUseGOT();
Index: cfe/trunk/test/CodeGen/codemodels.c
===
--- cfe/trunk/test/CodeGen/codemodels.c
+++ cfe/trunk/test/CodeGen/codemodels.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
-check-prefix=CHECK-NOMODEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model tiny %s -o - | FileCheck %s
-check-prefix=CHECK-TINY
+// RUN: %clang_cc1 -emit-llvm -mcode-model small %s -o - | FileCheck %s
-check-prefix=CHECK-SMALL
+// RUN: %clang_cc1 -emit-llvm -mcode-model kernel %s -o - | FileCheck %s
-check-prefix=CHECK-KERNEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model medium %s -o - | FileCheck %s
-check-prefix=CHECK-MEDIUM
+// RUN: %clang_cc1 -emit-llvm -mcode-model large %s -o - | FileCheck %s
-check-prefix=CHECK-LARGE
+
+// CHECK-TINY: !llvm.module.flags = !{{{.*}}}
+// CHECK-TINY: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 0}
+// CHECK-SMALL: !llvm.module.flags = !{{{.*}}}
+// CHECK-SMALL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 1}
+// CHECK-KERNEL: !llvm.module.flags = !{{{.*}}}
+// CHECK-KERNEL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 2}
+// CHECK-MEDIUM: !llvm.module.flags = !{{{.*}}}
+// CHECK-MEDIUM: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 3}
+// CHECK-LARGE: !llvm.module.flags = !{{{.*}}}
+// CHECK-LARGE: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 4}
+// CHECK-NOMODEL-NOT: Code Model
Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp
@@ -44,6 +44,7 @@
#include "clang/CodeGen/ConstantInitBuilder.h"
#include "clang/Frontend/CodeGenOptions.h"
#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/ADT/Triple.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/IR/CallSite.h"
@@ -53,6 +54,7 @@
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Module.h"
#include "llvm/ProfileData/InstrProfReader.h"
+#include "llvm/Support/CodeGen.h"
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/MD5.h"
@@ -556,6 +558,20 @@
getModule().setPIELevel(static_cast(PLevel));
}
+ if (getCodeGenOpts().CodeModel.size() > 0) {
+unsigned CM = llvm::StringSwitch(getCodeGenOpts().CodeModel)
+ .Case("tiny", llvm::CodeModel::Tiny)
+ .Case("small", llvm::CodeModel::Small)
+ .Case("kernel", llvm::CodeModel::Kernel)
+ .Case("medium", llvm::CodeModel::Medium)
+ .Case("large", llvm::CodeModel::Large)
+ .Default(~0u);
+if (CM != ~0u) {
+ llvm::CodeModel::Model codeModel = static_cast(CM);
+ getModule().setCodeModel(codeModel);
+}
+ }
+
if (CodeGenOpts.NoPLT)
getModule().setRtLibUseGOT();
Index: cfe/trunk/test/CodeGen/codemodels.c
===
--- cfe/trunk/test/CodeGen/codemodels.c
+++ cfe/tru
r342758 - Add necessary support for storing code-model to module IR.
Author: ctice
Date: Fri Sep 21 11:34:59 2018
New Revision: 342758
URL: http://llvm.org/viewvc/llvm-project?rev=342758&view=rev
Log:
Add necessary support for storing code-model to module IR.
Currently the code-model does not get saved in the module IR, so if a
code model is specified when compiling with LTO, it gets lost and is
not propagated properly to LTO. This patch does what is necessary in
the front end to pass the code-model to the module, so that the back
end can store it in the Module .
Differential Revision: https://reviews.llvm.org/D52323
Added:
cfe/trunk/test/CodeGen/codemodels.c
Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=342758&r1=342757&r2=342758&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Fri Sep 21 11:34:59 2018
@@ -44,6 +44,7 @@
#include "clang/CodeGen/ConstantInitBuilder.h"
#include "clang/Frontend/CodeGenOptions.h"
#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/ADT/Triple.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/IR/CallSite.h"
@@ -53,6 +54,7 @@
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Module.h"
#include "llvm/ProfileData/InstrProfReader.h"
+#include "llvm/Support/CodeGen.h"
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/MD5.h"
@@ -556,6 +558,20 @@ void CodeGenModule::Release() {
getModule().setPIELevel(static_cast(PLevel));
}
+ if (getCodeGenOpts().CodeModel.size() > 0) {
+unsigned CM = llvm::StringSwitch(getCodeGenOpts().CodeModel)
+ .Case("tiny", llvm::CodeModel::Tiny)
+ .Case("small", llvm::CodeModel::Small)
+ .Case("kernel", llvm::CodeModel::Kernel)
+ .Case("medium", llvm::CodeModel::Medium)
+ .Case("large", llvm::CodeModel::Large)
+ .Default(~0u);
+if (CM != ~0u) {
+ llvm::CodeModel::Model codeModel =
static_cast(CM);
+ getModule().setCodeModel(codeModel);
+}
+ }
+
if (CodeGenOpts.NoPLT)
getModule().setRtLibUseGOT();
Added: cfe/trunk/test/CodeGen/codemodels.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/codemodels.c?rev=342758&view=auto
==
--- cfe/trunk/test/CodeGen/codemodels.c (added)
+++ cfe/trunk/test/CodeGen/codemodels.c Fri Sep 21 11:34:59 2018
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
-check-prefix=CHECK-NOMODEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model tiny %s -o - | FileCheck %s
-check-prefix=CHECK-TINY
+// RUN: %clang_cc1 -emit-llvm -mcode-model small %s -o - | FileCheck %s
-check-prefix=CHECK-SMALL
+// RUN: %clang_cc1 -emit-llvm -mcode-model kernel %s -o - | FileCheck %s
-check-prefix=CHECK-KERNEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model medium %s -o - | FileCheck %s
-check-prefix=CHECK-MEDIUM
+// RUN: %clang_cc1 -emit-llvm -mcode-model large %s -o - | FileCheck %s
-check-prefix=CHECK-LARGE
+
+// CHECK-TINY: !llvm.module.flags = !{{{.*}}}
+// CHECK-TINY: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 0}
+// CHECK-SMALL: !llvm.module.flags = !{{{.*}}}
+// CHECK-SMALL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 1}
+// CHECK-KERNEL: !llvm.module.flags = !{{{.*}}}
+// CHECK-KERNEL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 2}
+// CHECK-MEDIUM: !llvm.module.flags = !{{{.*}}}
+// CHECK-MEDIUM: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 3}
+// CHECK-LARGE: !llvm.module.flags = !{{{.*}}}
+// CHECK-LARGE: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 4}
+// CHECK-NOMODEL-NOT: Code Model
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51812: Simplify CheckFallThroughForBody
gromer added inline comments. Comment at: b/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp:601 +case MaybeFallThrough: + if (ReturnsValue) +S.Diag(RBrace, diag::warn_maybe_falloff_nonvoid_coroutine) rsmith wrote: > This `if` and the one below are redundant now. Leaving aside the mismatch between the locations passed to `isIgnored` and `Diag`, I believe this `if` still has an observable effect (namely suppressing the diagnostic) in the case where `ReturnsValue` is false, `HasNoReturnAttr` is true, and neither diagnostic is ignored. Even if I'm wrong about that, it's not clear to me whether the error is here or in the early return condition (as noted in the FIXME above, the two do not appear to match up). https://reviews.llvm.org/D51812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52339: Support enums with a fixed underlying type in all language modes
erik.pilkington added reviewers: jlebar, Anastasia. erik.pilkington added subscribers: jlebar, Anastasia. erik.pilkington added inline comments. Comment at: clang/include/clang/Basic/Features.def:233 EXTENSION(cxx_variadic_templates, LangOpts.CPlusPlus) +EXTENSION(cxx_fixed_enum, true) // C++14 features supported by other languages as extensions. aaron.ballman wrote: > erik.pilkington wrote: > > aaron.ballman wrote: > > > Are we sure we want to make this decision for things like OpenCL, Cuda, > > > etc? > > I can't think of any reason why not, seems there are a handful of other > > EXTENSION(*, true) features. Do you have a preference? > I think we should probably ask folks from the projects to see if they're okay > with the extension or not, but I'd guess this won't be contentious. Sure, @jlebar and @Anastasia: Any thoughts here? Repository: rC Clang https://reviews.llvm.org/D52339 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51812: Simplify CheckFallThroughForBody
gromer updated this revision to Diff 166521.
gromer marked 2 inline comments as done.
https://reviews.llvm.org/D51812
Files:
B/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
Index: B/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- B/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ B/llvm/tools/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -509,8 +509,7 @@
bool HasNoReturnAttr = FT->getNoReturnAttr();
// Short circuit for compilation speed.
- if (!ReturnsValue && !HasNoReturnAttr)
- return;
+ if (!ReturnsValue && !HasNoReturnAttr) return;
SourceLocation RBrace = Body->getEndLoc();
switch (CheckFallThrough(AC)) {
@@ -535,25 +534,20 @@
}
}
-static void CheckFallThroughForLambda(
-Sema &S, const Decl *D, const Stmt *Body, AnalysisDeclContext &AC,
-sema::FunctionScopeInfo *FSI) {
+static void CheckFallThroughForLambda(Sema &S, const Decl *D, const Stmt *Body,
+ AnalysisDeclContext &AC,
+ sema::FunctionScopeInfo *FSI) {
const auto *FD = dyn_cast(D);
if (FD == nullptr) return;
- // cpu_dispatch functions permit empty function bodies for ICC compatibility.
- if (FD->isCPUDispatchMultiVersion())
-return;
-
auto *LSI = cast(FSI);
bool ReturnsValue = LSI->WrappedReturnType.isNull()
- ? !FD->getReturnType()->isVoidType()
- : LSI->CoroutineHasNonVoidReturn;
+ ? !FD->getReturnType()->isVoidType()
+ : LSI->CoroutineHasNonVoidReturn;
bool HasNoReturnAttr = FD->isNoReturn();
// Short circuit for compilation speed.
- if (!ReturnsValue && !HasNoReturnAttr)
- return;
+ if (!ReturnsValue && !HasNoReturnAttr) return;
SourceLocation RBrace = Body->getEndLoc();
switch (CheckFallThrough(AC)) {
@@ -576,9 +570,10 @@
}
}
-static void CheckFallThroughForCoroutine(
-Sema &S, const Decl *D, const Stmt *Body,
-AnalysisDeclContext &AC, sema::FunctionScopeInfo *FSI) {
+static void CheckFallThroughForCoroutine(Sema &S, const Decl *D,
+ const Stmt *Body,
+ AnalysisDeclContext &AC,
+ sema::FunctionScopeInfo *FSI) {
const auto *FD = dyn_cast(D);
if (FD == nullptr) return;
@@ -597,8 +592,8 @@
D->getLocation()) ||
Diags.isIgnored(diag::warn_maybe_falloff_nonvoid_coroutine,
D->getLocation())) &&
- (!HasNoReturnAttr))
- return;
+ !HasNoReturnAttr)
+return;
SourceLocation RBrace = Body->getEndLoc();
switch (CheckFallThrough(AC)) {
@@ -619,9 +614,9 @@
}
}
-static void CheckFallThroughForFunction(
-Sema &S, const Decl *D, const Stmt *Body,
-AnalysisDeclContext &AC) {
+static void CheckFallThroughForFunction(Sema &S, const Decl *D,
+const Stmt *Body,
+AnalysisDeclContext &AC) {
bool ReturnsVoid = false;
bool HasNoReturnAttr = false;
bool SuggestNoReturn = true;
@@ -642,8 +637,7 @@
isTemplateInstantiation = Function->isTemplateInstantiation();
SuggestNoReturn = !isVirtualMethod && !isTemplateInstantiation;
- }
- else if (const auto *MD = dyn_cast(D)) {
+ } else if (const auto *MD = dyn_cast(D)) {
ReturnsVoid = MD->getReturnType()->isVoidType();
HasNoReturnAttr = MD->hasAttr();
}
@@ -654,9 +648,8 @@
// Short circuit for compilation speed.
DiagnosticsEngine &Diags = S.getDiagnostics();
- if ((ReturnsVoid ||
- Diags.isIgnored(diag::warn_maybe_falloff_nonvoid_function,
- D->getLocation())) &&
+ if ((ReturnsVoid || Diags.isIgnored(diag::warn_maybe_falloff_nonvoid_function,
+ D->getLocation())) &&
(!HasNoReturnAttr ||
Diags.isIgnored(diag::warn_noreturn_function_has_return_expr,
D->getLocation())) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer
george.burgess.iv accepted this revision. george.burgess.iv added a comment. LGTM too. Thanks again! Repository: rC Clang https://reviews.llvm.org/D52268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52339: Support enums with a fixed underlying type in all language modes
erik.pilkington added a comment. > There's no reason to make this an `ObjC2`-only feature; we should probably > eliminate that distinction throughout the compiler. Sure, I was just writing a proposal to do that: http://lists.llvm.org/pipermail/cfe-dev/2018-September/059468.html Repository: rC Clang https://reviews.llvm.org/D52339 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52368: [libc++abi] is_strcmp parameter to is_equal is unused for WIN32
mclow.lists added a comment. This seems like overkill to me; why not add `(void) use_strcmp` right before line 67 instead? Repository: rCXXA libc++abi https://reviews.llvm.org/D52368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50229: [ARM][AArch64] Add feature +fp16fml
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D50229 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52339: Support enums with a fixed underlying type in all language modes
rjmccall added a comment.
In https://reviews.llvm.org/D52339#1242241, @aaron.ballman wrote:
> In https://reviews.llvm.org/D52339#1242202, @erik.pilkington wrote:
>
> > From the last line in the paper, it seems that C++ compatibility is a goal
> > of the paper (or at least a consideration). We should probably think about
> > this if/when the final wording gets accepted though.
>
>
> Agreed. WG14's charter explicitly prefers compatibility with C++ when
> possible.
>
> The part that I wasn't quite sure on were the type constraints in the
> proposal. C++ allows any integral type and ignores cv qualifiers, but the
> proposal lists specific types and doesn't discuss qualifiers. By my reading,
> this is code allowed in C++ but prohibited in the proposed wording:
>
> enum E : const int32_t {
> One
> };
>
>
> (Because the type is int32_t and is cv-qualified.) However, it's possible
> that's an oversight rather than an intentional design. I'll bring it up with
> Clive to see, but perhaps we can spot other divergences and can provide him
> with a list of concerns on the implementation side.
Not accepting `typedef`s would be negligence on behalf of the committee, since
generally people use this feature specifically to control the size of the
`enum`, which you can't do portably without a `typedef`. Not accepting
qualified types is a justifiable decision.
There's no reason to make this an `ObjC2`-only feature; we should probably
eliminate that distinction throughout the compiler.
Repository:
rC Clang
https://reviews.llvm.org/D52339
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52268: [AST] Squeeze some bits in LinkageComputer
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D52268#1241793, @riccibruno wrote: > In https://reviews.llvm.org/D52268#1241033, @rjmccall wrote: > > > `LinkageComputer` isn't actually persisted anywhere, right? And there's > > maybe one computer active at once? So this compression is theoretically > > saving one pointer of stack space but forcing a bunch of bit-manipulation > > every time these fields are accessed. > > > It is not persisted but this saves one pointer per entry in the map. Another > factor is that hashing a pair involves hashing > each component and then combining the result, which is comparatively much > more expansive than just hashing a `PointerIntPair`, > which involves only a shift and a xor. The field storing the > `LVComputationKind` is never directly read but only used to differentiate > various kinds of computations in the map. I went back and instrumented the > lookup function `LinkageComputer::lookup` with `rdtsc`, > and (with all the usual caveats about microbenchmarks and `rdtsc`) I get > that this cuts the number of ticks spent inside `lookup` > from about 8e6 to 3.5e6. Now of course taking a step back this represent > only milliseconds and is firmly in the category of > "way to small to bother", but now we might as well do it. Oh, I see, the commit summary is wrong. You're not compressing `LinkageComputer`, you're compressing a lookup key type. LGTM. Repository: rC Clang https://reviews.llvm.org/D52268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52321: [CUDA] Fixed parsing of optional template-argument-list.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342752: [CUDA] Fixed parsing of optional
template-argument-list. (authored by tra, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52321?vs=166509&id=166512#toc
Repository:
rC Clang
https://reviews.llvm.org/D52321
Files:
lib/Parse/ParseTemplate.cpp
test/Parser/cuda-kernel-call-c++11.cu
test/Parser/cuda-kernel-call.cu
Index: test/Parser/cuda-kernel-call-c++11.cu
===
--- test/Parser/cuda-kernel-call-c++11.cu
+++ test/Parser/cuda-kernel-call-c++11.cu
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
-template struct S {};
+template struct S {};
template void f();
@@ -11,10 +11,14 @@
// expected-no-diagnostics
S>> s3;
+ S>> s30;
S>>> s4;
+ S>>> s40;
S s5;
+ S s50;
(void)(&f>>==0);
+ (void)(&f>>==0);
}
Index: test/Parser/cuda-kernel-call.cu
===
--- test/Parser/cuda-kernel-call.cu
+++ test/Parser/cuda-kernel-call.cu
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
-template struct S {};
+template struct S {};
template void f();
void foo(void) {
@@ -13,5 +13,7 @@
// The following two are parse errors because -std=c++11 is not enabled.
S>> s; // expected-error 2{{use '> >'}}
+ S>> s1; // expected-error 2{{use '> >'}}
(void)(&f>>==0); // expected-error 2{{use '> >'}}
+ (void)(&f>>==0); // expected-error 2{{use '> >'}}
}
Index: lib/Parse/ParseTemplate.cpp
===
--- lib/Parse/ParseTemplate.cpp
+++ lib/Parse/ParseTemplate.cpp
@@ -946,7 +946,9 @@
bool Invalid = false;
{
GreaterThanIsOperatorScope G(GreaterThanIsOperator, false);
-if (Tok.isNot(tok::greater) && Tok.isNot(tok::greatergreater))
+if (!Tok.isOneOf(tok::greater, tok::greatergreater,
+ tok::greatergreatergreater, tok::greaterequal,
+ tok::greatergreaterequal))
Invalid = ParseTemplateArgumentList(TemplateArgs);
if (Invalid) {
Index: test/Parser/cuda-kernel-call-c++11.cu
===
--- test/Parser/cuda-kernel-call-c++11.cu
+++ test/Parser/cuda-kernel-call-c++11.cu
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
-template struct S {};
+template struct S {};
template void f();
@@ -11,10 +11,14 @@
// expected-no-diagnostics
S>> s3;
+ S>> s30;
S>>> s4;
+ S>>> s40;
S s5;
+ S s50;
(void)(&f>>==0);
+ (void)(&f>>==0);
}
Index: test/Parser/cuda-kernel-call.cu
===
--- test/Parser/cuda-kernel-call.cu
+++ test/Parser/cuda-kernel-call.cu
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
-template struct S {};
+template struct S {};
template void f();
void foo(void) {
@@ -13,5 +13,7 @@
// The following two are parse errors because -std=c++11 is not enabled.
S>> s; // expected-error 2{{use '> >'}}
+ S>> s1; // expected-error 2{{use '> >'}}
(void)(&f>>==0); // expected-error 2{{use '> >'}}
+ (void)(&f>>==0); // expected-error 2{{use '> >'}}
}
Index: lib/Parse/ParseTemplate.cpp
===
--- lib/Parse/ParseTemplate.cpp
+++ lib/Parse/ParseTemplate.cpp
@@ -946,7 +946,9 @@
bool Invalid = false;
{
GreaterThanIsOperatorScope G(GreaterThanIsOperator, false);
-if (Tok.isNot(tok::greater) && Tok.isNot(tok::greatergreater))
+if (!Tok.isOneOf(tok::greater, tok::greatergreater,
+ tok::greatergreatergreater, tok::greaterequal,
+ tok::greatergreaterequal))
Invalid = ParseTemplateArgumentList(TemplateArgs);
if (Invalid) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342752 - [CUDA] Fixed parsing of optional template-argument-list.
Author: tra
Date: Fri Sep 21 10:46:28 2018
New Revision: 342752
URL: http://llvm.org/viewvc/llvm-project?rev=342752&view=rev
Log:
[CUDA] Fixed parsing of optional template-argument-list.
We need to consider all tokens that start with '>' when
we're checking for the end of an empty template argument list.
Differential Revision: https://reviews.llvm.org/D52321
Modified:
cfe/trunk/lib/Parse/ParseTemplate.cpp
cfe/trunk/test/Parser/cuda-kernel-call-c++11.cu
cfe/trunk/test/Parser/cuda-kernel-call.cu
Modified: cfe/trunk/lib/Parse/ParseTemplate.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTemplate.cpp?rev=342752&r1=342751&r2=342752&view=diff
==
--- cfe/trunk/lib/Parse/ParseTemplate.cpp (original)
+++ cfe/trunk/lib/Parse/ParseTemplate.cpp Fri Sep 21 10:46:28 2018
@@ -946,7 +946,9 @@ Parser::ParseTemplateIdAfterTemplateName
bool Invalid = false;
{
GreaterThanIsOperatorScope G(GreaterThanIsOperator, false);
-if (Tok.isNot(tok::greater) && Tok.isNot(tok::greatergreater))
+if (!Tok.isOneOf(tok::greater, tok::greatergreater,
+ tok::greatergreatergreater, tok::greaterequal,
+ tok::greatergreaterequal))
Invalid = ParseTemplateArgumentList(TemplateArgs);
if (Invalid) {
Modified: cfe/trunk/test/Parser/cuda-kernel-call-c++11.cu
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cuda-kernel-call-c%2B%2B11.cu?rev=342752&r1=342751&r2=342752&view=diff
==
--- cfe/trunk/test/Parser/cuda-kernel-call-c++11.cu (original)
+++ cfe/trunk/test/Parser/cuda-kernel-call-c++11.cu Fri Sep 21 10:46:28 2018
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
-template struct S {};
+template struct S {};
template void f();
@@ -11,10 +11,14 @@ void foo(void) {
// expected-no-diagnostics
S>> s3;
+ S>> s30;
S>>> s4;
+ S>>> s40;
S s5;
+ S s50;
(void)(&f>>==0);
+ (void)(&f>>==0);
}
Modified: cfe/trunk/test/Parser/cuda-kernel-call.cu
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cuda-kernel-call.cu?rev=342752&r1=342751&r2=342752&view=diff
==
--- cfe/trunk/test/Parser/cuda-kernel-call.cu (original)
+++ cfe/trunk/test/Parser/cuda-kernel-call.cu Fri Sep 21 10:46:28 2018
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
-template struct S {};
+template struct S {};
template void f();
void foo(void) {
@@ -13,5 +13,7 @@ void foo(void) {
// The following two are parse errors because -std=c++11 is not enabled.
S>> s; // expected-error 2{{use '> >'}}
+ S>> s1; // expected-error 2{{use '> >'}}
(void)(&f>>==0); // expected-error 2{{use '> >'}}
+ (void)(&f>>==0); // expected-error 2{{use '> >'}}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52323: Add necessary support for storing code-model to module IR.
tejohnson accepted this revision. tejohnson added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D52323 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52339: Support enums with a fixed underlying type in all language modes
aaron.ballman added a comment.
In https://reviews.llvm.org/D52339#1242202, @erik.pilkington wrote:
> From the last line in the paper, it seems that C++ compatibility is a goal of
> the paper (or at least a consideration). We should probably think about this
> if/when the final wording gets accepted though.
Agreed. WG14's charter explicitly prefers compatibility with C++ when possible.
The part that I wasn't quite sure on were the type constraints in the proposal.
C++ allows any integral type and ignores cv qualifiers, but the proposal lists
specific types and doesn't discuss qualifiers. By my reading, this is code
allowed in C++ but prohibited in the proposed wording:
enum E : const int32_t {
One
};
(Because the type is int32_t and is cv-qualified.) However, it's possible
that's an oversight rather than an intentional design. I'll bring it up with
Clive to see, but perhaps we can spot other divergences and can provide him
with a list of concerns on the implementation side.
Comment at: clang/include/clang/Basic/Features.def:233
EXTENSION(cxx_variadic_templates, LangOpts.CPlusPlus)
+EXTENSION(cxx_fixed_enum, true)
// C++14 features supported by other languages as extensions.
erik.pilkington wrote:
> aaron.ballman wrote:
> > Are we sure we want to make this decision for things like OpenCL, Cuda, etc?
> I can't think of any reason why not, seems there are a handful of other
> EXTENSION(*, true) features. Do you have a preference?
I think we should probably ask folks from the projects to see if they're okay
with the extension or not, but I'd guess this won't be contentious.
Repository:
rC Clang
https://reviews.llvm.org/D52339
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52339: Support enums with a fixed underlying type in all language modes
erik.pilkington added inline comments. Comment at: clang/include/clang/Basic/Features.def:90 FEATURE(objc_default_synthesize_properties, LangOpts.ObjC2) -FEATURE(objc_fixed_enum, LangOpts.ObjC2) +FEATURE(objc_fixed_enum, true) FEATURE(objc_instancetype, LangOpts.ObjC2) erik.pilkington wrote: > aaron.ballman wrote: > > Is this really supported in ObjC1, or is there no longer a distinction to > > be made there? > I suppose it's a first class feature in ObjC2, but an extension in ObjC1. On > second thought I should probably not change this, because ObjC1 doesn't have > this as a feature, although it does have it as an extension. On third thought, it looks like we never even try to compile with ObjC1 = 1 and ObjC2 = 0, so I guess this is fine as-is. Repository: rC Clang https://reviews.llvm.org/D52339 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52321: [CUDA] Fixed parsing of optional template-argument-list.
tra updated this revision to Diff 166509.
tra added a comment.
Added '>=' and '>>=' to the list of tokens that may indicate the end of the
empty template argument list.
https://reviews.llvm.org/D52321
Files:
clang/lib/Parse/ParseTemplate.cpp
clang/test/Parser/cuda-kernel-call-c++11.cu
clang/test/Parser/cuda-kernel-call.cu
Index: clang/test/Parser/cuda-kernel-call.cu
===
--- clang/test/Parser/cuda-kernel-call.cu
+++ clang/test/Parser/cuda-kernel-call.cu
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
-template struct S {};
+template struct S {};
template void f();
void foo(void) {
@@ -13,5 +13,7 @@
// The following two are parse errors because -std=c++11 is not enabled.
S>> s; // expected-error 2{{use '> >'}}
+ S>> s1; // expected-error 2{{use '> >'}}
(void)(&f>>==0); // expected-error 2{{use '> >'}}
+ (void)(&f>>==0); // expected-error 2{{use '> >'}}
}
Index: clang/test/Parser/cuda-kernel-call-c++11.cu
===
--- clang/test/Parser/cuda-kernel-call-c++11.cu
+++ clang/test/Parser/cuda-kernel-call-c++11.cu
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
-template struct S {};
+template struct S {};
template void f();
@@ -11,10 +11,14 @@
// expected-no-diagnostics
S>> s3;
+ S>> s30;
S>>> s4;
+ S>>> s40;
S s5;
+ S s50;
(void)(&f>>==0);
+ (void)(&f>>==0);
}
Index: clang/lib/Parse/ParseTemplate.cpp
===
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -946,7 +946,9 @@
bool Invalid = false;
{
GreaterThanIsOperatorScope G(GreaterThanIsOperator, false);
-if (Tok.isNot(tok::greater) && Tok.isNot(tok::greatergreater))
+if (!Tok.isOneOf(tok::greater, tok::greatergreater,
+ tok::greatergreatergreater, tok::greaterequal,
+ tok::greatergreaterequal))
Invalid = ParseTemplateArgumentList(TemplateArgs);
if (Invalid) {
Index: clang/test/Parser/cuda-kernel-call.cu
===
--- clang/test/Parser/cuda-kernel-call.cu
+++ clang/test/Parser/cuda-kernel-call.cu
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
-template struct S {};
+template struct S {};
template void f();
void foo(void) {
@@ -13,5 +13,7 @@
// The following two are parse errors because -std=c++11 is not enabled.
S>> s; // expected-error 2{{use '> >'}}
+ S>> s1; // expected-error 2{{use '> >'}}
(void)(&f>>==0); // expected-error 2{{use '> >'}}
+ (void)(&f>>==0); // expected-error 2{{use '> >'}}
}
Index: clang/test/Parser/cuda-kernel-call-c++11.cu
===
--- clang/test/Parser/cuda-kernel-call-c++11.cu
+++ clang/test/Parser/cuda-kernel-call-c++11.cu
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
-template struct S {};
+template struct S {};
template void f();
@@ -11,10 +11,14 @@
// expected-no-diagnostics
S>> s3;
+ S>> s30;
S>>> s4;
+ S>>> s40;
S s5;
+ S s50;
(void)(&f>>==0);
+ (void)(&f>>==0);
}
Index: clang/lib/Parse/ParseTemplate.cpp
===
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -946,7 +946,9 @@
bool Invalid = false;
{
GreaterThanIsOperatorScope G(GreaterThanIsOperator, false);
-if (Tok.isNot(tok::greater) && Tok.isNot(tok::greatergreater))
+if (!Tok.isOneOf(tok::greater, tok::greatergreater,
+ tok::greatergreatergreater, tok::greaterequal,
+ tok::greatergreaterequal))
Invalid = ParseTemplateArgumentList(TemplateArgs);
if (Invalid) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342750 - [NFC] remove unused variable
Author: jfb Date: Fri Sep 21 10:38:41 2018 New Revision: 342750 URL: http://llvm.org/viewvc/llvm-project?rev=342750&view=rev Log: [NFC] remove unused variable It was causing a warning. Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=342750&r1=342749&r2=342750&view=diff == --- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original) +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Fri Sep 21 10:38:41 2018 @@ -7571,7 +7571,6 @@ public: const MapData &BaseData = CI == CE ? L : L1; OMPClauseMappableExprCommon::MappableExprComponentListRef SubData = SI == SE ? Components : Components1; - auto It = CI == CE ? SI : CI; auto &OverlappedElements = OverlappedData.FindAndConstruct(&BaseData); OverlappedElements.getSecond().push_back(SubData); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52368: [libc++abi] is_strcmp parameter to is_equal is unused for WIN32
pirama created this revision.
pirama added reviewers: EricWF, srhines, mstorsjo.
Herald added subscribers: libcxx-commits, ldionne, christof.
Mark it as unused to avoid -Wunused-parameter.
Repository:
rCXXA libc++abi
https://reviews.llvm.org/D52368
Files:
src/private_typeinfo.cpp
Index: src/private_typeinfo.cpp
===
--- src/private_typeinfo.cpp
+++ src/private_typeinfo.cpp
@@ -51,13 +51,17 @@
// Defining _LIBCXX_DYNAMIC_FALLBACK does not help since can_catch() calls
// is_equal() with use_strcmp=false so the string names are not compared.
+#define _UNUSED_IN_WIN32
#ifdef _WIN32
#include
+#define _UNUSED_IN_WIN32 __attribute__((unused))
#endif
static inline
bool
-is_equal(const std::type_info* x, const std::type_info* y, bool use_strcmp)
+is_equal(const std::type_info* x,
+ const std::type_info* y,
+ _UNUSED_IN_WIN32 bool use_strcmp)
{
#ifndef _WIN32
if (!use_strcmp)
Index: src/private_typeinfo.cpp
===
--- src/private_typeinfo.cpp
+++ src/private_typeinfo.cpp
@@ -51,13 +51,17 @@
// Defining _LIBCXX_DYNAMIC_FALLBACK does not help since can_catch() calls
// is_equal() with use_strcmp=false so the string names are not compared.
+#define _UNUSED_IN_WIN32
#ifdef _WIN32
#include
+#define _UNUSED_IN_WIN32 __attribute__((unused))
#endif
static inline
bool
-is_equal(const std::type_info* x, const std::type_info* y, bool use_strcmp)
+is_equal(const std::type_info* x,
+ const std::type_info* y,
+ _UNUSED_IN_WIN32 bool use_strcmp)
{
#ifndef _WIN32
if (!use_strcmp)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51808: [CUDA] Ignore uncallable functions when we check for usual deallocators.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342749: [CUDA] Ignore uncallable functions when we check for
usual deallocators. (authored by tra, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D51808?vs=166053&id=166504#toc
Repository:
rL LLVM
https://reviews.llvm.org/D51808
Files:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/CodeGenCUDA/usual-deallocators.cu
cfe/trunk/test/SemaCUDA/call-host-fn-from-device.cu
cfe/trunk/test/SemaCUDA/usual-deallocators.cu
Index: cfe/trunk/include/clang/AST/DeclCXX.h
===
--- cfe/trunk/include/clang/AST/DeclCXX.h
+++ cfe/trunk/include/clang/AST/DeclCXX.h
@@ -2109,10 +2109,15 @@
Base, IsAppleKext);
}
- /// Determine whether this is a usual deallocation function
- /// (C++ [basic.stc.dynamic.deallocation]p2), which is an overloaded
- /// delete or delete[] operator with a particular signature.
- bool isUsualDeallocationFunction() const;
+ /// Determine whether this is a usual deallocation function (C++
+ /// [basic.stc.dynamic.deallocation]p2), which is an overloaded delete or
+ /// delete[] operator with a particular signature. Populates \p PreventedBy
+ /// with the declarations of the functions of the same kind if they were the
+ /// reason for this function returning false. This is used by
+ /// Sema::isUsualDeallocationFunction to reconsider the answer based on the
+ /// context.
+ bool isUsualDeallocationFunction(
+ SmallVectorImpl &PreventedBy) const;
/// Determine whether this is a copy-assignment operator, regardless
/// of whether it was declared implicitly or explicitly.
Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -1619,6 +1619,8 @@
SourceLocation Loc, const NamedDecl *D,
ArrayRef Equiv);
+ bool isUsualDeallocationFunction(const CXXMethodDecl *FD);
+
bool isCompleteType(SourceLocation Loc, QualType T) {
return !RequireCompleteTypeImpl(Loc, T, nullptr);
}
Index: cfe/trunk/test/CodeGenCUDA/usual-deallocators.cu
===
--- cfe/trunk/test/CodeGenCUDA/usual-deallocators.cu
+++ cfe/trunk/test/CodeGenCUDA/usual-deallocators.cu
@@ -0,0 +1,133 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple nvptx-unknown-unknown -fcuda-is-device \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefixes=COMMON,DEVICE
+// RUN: %clang_cc1 %s --std=c++11 -triple nvptx-unknown-unknown \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefixes=COMMON,HOST
+// RUN: %clang_cc1 %s --std=c++17 -triple nvptx-unknown-unknown -fcuda-is-device \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefixes=COMMON,DEVICE
+// RUN: %clang_cc1 %s --std=c++17 -triple nvptx-unknown-unknown \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefixes=COMMON,HOST
+
+#include "Inputs/cuda.h"
+extern "C" __host__ void host_fn();
+extern "C" __device__ void dev_fn();
+extern "C" __host__ __device__ void hd_fn();
+
+struct H1D1 {
+ __host__ void operator delete(void *) { host_fn(); };
+ __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct H1D2 {
+ __host__ void operator delete(void *) { host_fn(); };
+ __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H2D1 {
+ __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+ __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct H2D2 {
+ __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+ __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H1D1D2 {
+ __host__ void operator delete(void *) { host_fn(); };
+ __device__ void operator delete(void *) { dev_fn(); };
+ __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H1H2D1 {
+ __host__ void operator delete(void *) { host_fn(); };
+ __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+ __device__ void operator delete(void *) { dev_fn(); };
+};
+
+struct H1H2D2 {
+ __host__ void operator delete(void *) { host_fn(); };
+ __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+ __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+struct H1H2D1D2 {
+ __host__ void operator delete(void *) { host_fn(); };
+ __host__ void operator delete(void *, __SIZE_TYPE__) { host_fn(); };
+ __device__ void operator delete(void *) { dev_fn(); };
+ __device__ void operator delete(void *, __SIZE_TYPE__) { dev_fn(); };
+};
+
+
+template
+__host__ __device__ void test_hd(void *p) {
+ T *t = (T *)p;
+ delete t;
+}
[PATCH] D52339: Support enums with a fixed underlying type in all language modes
erik.pilkington added a comment. In https://reviews.llvm.org/D52339#1242126, @aaron.ballman wrote: > In https://reviews.llvm.org/D52339#1242118, @jfb wrote: > > > In https://reviews.llvm.org/D52339#1242103, @aaron.ballman wrote: > > > > > In https://reviews.llvm.org/D52339#1242086, @jfb wrote: > > > > > > > I think we should consider proposing this to the C committee. > > > > @aaron.ballman I can help Erik write the paper, would you be able to > > > > present it? Too tight for the upcoming meeting, but I'm guessing we > > > > have *plenty* of time for ++C17. > > > > > > > > > It's already been proposed to WG14 and is currently on the SD-3 list of > > > features to consider for C2x. See > > > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2008.pdf. I know Clive > > > and am happy to point him towards this patch (when it lands) as > > > demonstration of industry desire for the feature, in case he needs to > > > provide updated papers. > > > > > > Wonderful! Does this match he proposed C2x semantics? Once voted in we'll > > want to change this from just an extension to also be a `-std=c2x` thing, > > better have them match now. > > > I have to validate that still, but from a quick look, I think we're in the > ballpark if not outright compatible. From the last line in the paper, it seems that C++ compatibility is a goal of the paper (or at least a consideration). We should probably think about this if/when the final wording gets accepted though. Comment at: clang/include/clang/Basic/Features.def:90 FEATURE(objc_default_synthesize_properties, LangOpts.ObjC2) -FEATURE(objc_fixed_enum, LangOpts.ObjC2) +FEATURE(objc_fixed_enum, true) FEATURE(objc_instancetype, LangOpts.ObjC2) aaron.ballman wrote: > Is this really supported in ObjC1, or is there no longer a distinction to be > made there? I suppose it's a first class feature in ObjC2, but an extension in ObjC1. On second thought I should probably not change this, because ObjC1 doesn't have this as a feature, although it does have it as an extension. Comment at: clang/include/clang/Basic/Features.def:233 EXTENSION(cxx_variadic_templates, LangOpts.CPlusPlus) +EXTENSION(cxx_fixed_enum, true) // C++14 features supported by other languages as extensions. aaron.ballman wrote: > Are we sure we want to make this decision for things like OpenCL, Cuda, etc? I can't think of any reason why not, seems there are a handful of other EXTENSION(*, true) features. Do you have a preference? Repository: rC Clang https://reviews.llvm.org/D52339 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342749 - [CUDA] Ignore uncallable functions when we check for usual deallocators.
Author: tra
Date: Fri Sep 21 10:29:33 2018
New Revision: 342749
URL: http://llvm.org/viewvc/llvm-project?rev=342749&view=rev
Log:
[CUDA] Ignore uncallable functions when we check for usual deallocators.
Previously clang considered function variants from both sides of
compilation and that resulted in picking up wrong deallocation function.
Differential Revision: https://reviews.llvm.org/D51808
Added:
cfe/trunk/test/CodeGenCUDA/usual-deallocators.cu
cfe/trunk/test/SemaCUDA/usual-deallocators.cu
Modified:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/SemaCUDA/call-host-fn-from-device.cu
Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=342749&r1=342748&r2=342749&view=diff
==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Fri Sep 21 10:29:33 2018
@@ -2109,10 +2109,15 @@ public:
Base, IsAppleKext);
}
- /// Determine whether this is a usual deallocation function
- /// (C++ [basic.stc.dynamic.deallocation]p2), which is an overloaded
- /// delete or delete[] operator with a particular signature.
- bool isUsualDeallocationFunction() const;
+ /// Determine whether this is a usual deallocation function (C++
+ /// [basic.stc.dynamic.deallocation]p2), which is an overloaded delete or
+ /// delete[] operator with a particular signature. Populates \p PreventedBy
+ /// with the declarations of the functions of the same kind if they were the
+ /// reason for this function returning false. This is used by
+ /// Sema::isUsualDeallocationFunction to reconsider the answer based on the
+ /// context.
+ bool isUsualDeallocationFunction(
+ SmallVectorImpl &PreventedBy) const;
/// Determine whether this is a copy-assignment operator, regardless
/// of whether it was declared implicitly or explicitly.
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=342749&r1=342748&r2=342749&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Sep 21 10:29:33 2018
@@ -1619,6 +1619,8 @@ public:
SourceLocation Loc, const NamedDecl *D,
ArrayRef Equiv);
+ bool isUsualDeallocationFunction(const CXXMethodDecl *FD);
+
bool isCompleteType(SourceLocation Loc, QualType T) {
return !RequireCompleteTypeImpl(Loc, T, nullptr);
}
Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=342749&r1=342748&r2=342749&view=diff
==
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Fri Sep 21 10:29:33 2018
@@ -2005,7 +2005,9 @@ CXXMethodDecl *CXXMethodDecl::getDevirtu
return nullptr;
}
-bool CXXMethodDecl::isUsualDeallocationFunction() const {
+bool CXXMethodDecl::isUsualDeallocationFunction(
+SmallVectorImpl &PreventedBy) const {
+ assert(PreventedBy.empty() && "PreventedBy is expected to be empty");
if (getOverloadedOperator() != OO_Delete &&
getOverloadedOperator() != OO_Array_Delete)
return false;
@@ -2063,14 +2065,16 @@ bool CXXMethodDecl::isUsualDeallocationF
// This function is a usual deallocation function if there are no
// single-parameter deallocation functions of the same kind.
DeclContext::lookup_result R = getDeclContext()->lookup(getDeclName());
- for (DeclContext::lookup_result::iterator I = R.begin(), E = R.end();
- I != E; ++I) {
-if (const auto *FD = dyn_cast(*I))
- if (FD->getNumParams() == 1)
-return false;
+ bool Result = true;
+ for (const auto *D : R) {
+if (const auto *FD = dyn_cast(D)) {
+ if (FD->getNumParams() == 1) {
+PreventedBy.push_back(FD);
+Result = false;
+ }
+}
}
-
- return true;
+ return Result;
}
bool CXXMethodDecl::isCopyAssignmentOperator() const {
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=342749&r1=342748&r2=342749&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Sep 21 10:29:33 2018
@@ -13183,7 +13183,8 @@ CheckOperatorDeleteDeclaration(Sema &Sem
// C++ P0722:
// A destroying operator delete shall be a usual deallocation function.
if (MD && !MD->getParent()->isDependentContext() &&
- MD->isDestroyingOperatorDelete() && !MD->isUsualDeallocationFunction()) {
+ MD->isDestroyingOpera
[PATCH] D52281: [clang-tidy] Add modernize check to use std::invoke in generic code
aaron.ballman added inline comments.
Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:27-32
+ // template
+ // void f(T func) {
+ // func();
+ // ^~~
+ // ::std::invoke(func, 1)
+ Finder->addMatcher(callExpr(has(declRefExpr())).bind("functor"), this);
lebedev.ri wrote:
> aaron.ballman wrote:
> > lebedev.ri wrote:
> > > Bikeshedding: i do not understand the idea behind `std::invoke`.
> > > Why is the new version better/preferred?
> > It generically handles all the various kinds of "callable" objects
> > (lambdas, functors, function pointers, pointer to member functions, etc).
> Thank you for answering, but i still do not understand.
> E.g. on that example, what benefits does it give?
> Even after looking at cppreference, i'm not sure..
> Just curious.
Genericity is the primary benefit. Here's the original paper with its
motivation:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3727.html
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.
grimar added a comment. To summarise: - It shares most of the benefits with the .dwo solution. - Unlike .dwo, there is no need to split the file and it is friendlier to other tools (ccache, distcc, etc) - But unlike .dwo a distributed build system has to copy larger .o files. https://reviews.llvm.org/D52296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52321: [CUDA] Fixed parsing of optional template-argument-list.
rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Parse/ParseTemplate.cpp:949-950 GreaterThanIsOperatorScope G(GreaterThanIsOperator, false); -if (Tok.isNot(tok::greater) && Tok.isNot(tok::greatergreater)) +if (!Tok.isOneOf(tok::greater, tok::greatergreater, + tok::greatergreatergreater)) Invalid = ParseTemplateArgumentList(TemplateArgs); It'd be good to include the other tokens that start with `>` here (which we also split as an extension): `tok::greaterequal` and `tok::greatergreaterequal`. https://reviews.llvm.org/D52321 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52339: Support enums with a fixed underlying type in all language modes
aaron.ballman added a comment. In https://reviews.llvm.org/D52339#1242118, @jfb wrote: > In https://reviews.llvm.org/D52339#1242103, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D52339#1242086, @jfb wrote: > > > > > I think we should consider proposing this to the C committee. > > > @aaron.ballman I can help Erik write the paper, would you be able to > > > present it? Too tight for the upcoming meeting, but I'm guessing we have > > > *plenty* of time for ++C17. > > > > > > It's already been proposed to WG14 and is currently on the SD-3 list of > > features to consider for C2x. See > > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2008.pdf. I know Clive and > > am happy to point him towards this patch (when it lands) as demonstration > > of industry desire for the feature, in case he needs to provide updated > > papers. > > > Wonderful! Does this match he proposed C2x semantics? Once voted in we'll > want to change this from just an extension to also be a `-std=c2x` thing, > better have them match now. I have to validate that still, but from a quick look, I think we're in the ballpark if not outright compatible. Repository: rC Clang https://reviews.llvm.org/D52339 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52339: Support enums with a fixed underlying type in all language modes
jfb added a comment. In https://reviews.llvm.org/D52339#1242103, @aaron.ballman wrote: > In https://reviews.llvm.org/D52339#1242086, @jfb wrote: > > > I think we should consider proposing this to the C committee. > > @aaron.ballman I can help Erik write the paper, would you be able to > > present it? Too tight for the upcoming meeting, but I'm guessing we have > > *plenty* of time for ++C17. > > > It's already been proposed to WG14 and is currently on the SD-3 list of > features to consider for C2x. See > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2008.pdf. I know Clive and > am happy to point him towards this patch (when it lands) as demonstration of > industry desire for the feature, in case he needs to provide updated papers. Wonderful! Does this match he proposed C2x semantics? Once voted in we'll want to change this from just an extension to also be a `-std=c2x` thing, better have them match now. Repository: rC Clang https://reviews.llvm.org/D52339 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52323: Add necessary support for storing code-model to module IR.
cmtice updated this revision to Diff 166492.
cmtice edited the summary of this revision.
cmtice added a comment.
Move include statement from CodeGenModule.h to CodeGenModule.cpp
Remove incorrect "default" case statement.
Add test for "tiny" code model.
https://reviews.llvm.org/D52323
Files:
lib/CodeGen/CodeGenModule.cpp
test/CodeGen/codemodels.c
Index: test/CodeGen/codemodels.c
===
--- test/CodeGen/codemodels.c
+++ test/CodeGen/codemodels.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
-check-prefix=CHECK-NOMODEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model tiny %s -o - | FileCheck %s
-check-prefix=CHECK-TINY
+// RUN: %clang_cc1 -emit-llvm -mcode-model small %s -o - | FileCheck %s
-check-prefix=CHECK-SMALL
+// RUN: %clang_cc1 -emit-llvm -mcode-model kernel %s -o - | FileCheck %s
-check-prefix=CHECK-KERNEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model medium %s -o - | FileCheck %s
-check-prefix=CHECK-MEDIUM
+// RUN: %clang_cc1 -emit-llvm -mcode-model large %s -o - | FileCheck %s
-check-prefix=CHECK-LARGE
+
+// CHECK-TINY: !llvm.module.flags = !{{{.*}}}
+// CHECK-TINY: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 0}
+// CHECK-SMALL: !llvm.module.flags = !{{{.*}}}
+// CHECK-SMALL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 1}
+// CHECK-KERNEL: !llvm.module.flags = !{{{.*}}}
+// CHECK-KERNEL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 2}
+// CHECK-MEDIUM: !llvm.module.flags = !{{{.*}}}
+// CHECK-MEDIUM: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 3}
+// CHECK-LARGE: !llvm.module.flags = !{{{.*}}}
+// CHECK-LARGE: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 4}
+// CHECK-NOMODEL-NOT: Code Model
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -44,6 +44,7 @@
#include "clang/CodeGen/ConstantInitBuilder.h"
#include "clang/Frontend/CodeGenOptions.h"
#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/ADT/Triple.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/IR/CallSite.h"
@@ -53,6 +54,7 @@
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Module.h"
#include "llvm/ProfileData/InstrProfReader.h"
+#include "llvm/Support/CodeGen.h"
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/MD5.h"
@@ -556,6 +558,20 @@
getModule().setPIELevel(static_cast(PLevel));
}
+ if (getCodeGenOpts().CodeModel.size() > 0) {
+unsigned CM = llvm::StringSwitch(getCodeGenOpts().CodeModel)
+ .Case("tiny", llvm::CodeModel::Tiny)
+ .Case("small", llvm::CodeModel::Small)
+ .Case("kernel", llvm::CodeModel::Kernel)
+ .Case("medium", llvm::CodeModel::Medium)
+ .Case("large", llvm::CodeModel::Large)
+ .Default(~0u);
+if (CM != ~0u) {
+ llvm::CodeModel::Model codeModel =
static_cast(CM);
+ getModule().setCodeModel(codeModel);
+}
+ }
+
if (CodeGenOpts.NoPLT)
getModule().setRtLibUseGOT();
Index: test/CodeGen/codemodels.c
===
--- test/CodeGen/codemodels.c
+++ test/CodeGen/codemodels.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-NOMODEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model tiny %s -o - | FileCheck %s -check-prefix=CHECK-TINY
+// RUN: %clang_cc1 -emit-llvm -mcode-model small %s -o - | FileCheck %s -check-prefix=CHECK-SMALL
+// RUN: %clang_cc1 -emit-llvm -mcode-model kernel %s -o - | FileCheck %s -check-prefix=CHECK-KERNEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model medium %s -o - | FileCheck %s -check-prefix=CHECK-MEDIUM
+// RUN: %clang_cc1 -emit-llvm -mcode-model large %s -o - | FileCheck %s -check-prefix=CHECK-LARGE
+
+// CHECK-TINY: !llvm.module.flags = !{{{.*}}}
+// CHECK-TINY: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 0}
+// CHECK-SMALL: !llvm.module.flags = !{{{.*}}}
+// CHECK-SMALL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 1}
+// CHECK-KERNEL: !llvm.module.flags = !{{{.*}}}
+// CHECK-KERNEL: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 2}
+// CHECK-MEDIUM: !llvm.module.flags = !{{{.*}}}
+// CHECK-MEDIUM: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 3}
+// CHECK-LARGE: !llvm.module.flags = !{{{.*}}}
+// CHECK-LARGE: !{{[0-9]+}} = !{i32 1, !"Code Model", i32 4}
+// CHECK-NOMODEL-NOT: Code Model
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -44,6 +44,7 @@
#include "clang/CodeGen/ConstantInitBuilder.h"
#include "clang/Frontend/CodeGenOptions.h"
#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/ADT/Triple.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/IR/CallSite.h"
@@ -53,6 +5
[PATCH] D52323: Add necessary support for storing code-model to module IR.
cmtice marked an inline comment as done.
cmtice added inline comments.
Comment at: lib/CodeGen/CodeGenModule.cpp:569
+ .Default(~0u);
+if ((CM != ~0u) && (CM != ~1u)) {
+ llvm::CodeModel::Model codeModel =
static_cast(CM);
tejohnson wrote:
> Can you simplify by using a single constant for both of these (since handling
> the same)?
I just realized .Case("default") is not a valid case, so I will remove that and
there will only be 1 constant.
Comment at: test/CodeGen/codemodels.c:2
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
-check-prefix=CHECK-NOMODEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model small %s -o - | FileCheck %s
-check-prefix=CHECK-SMALL
+// RUN: %clang_cc1 -emit-llvm -mcode-model kernel %s -o - | FileCheck %s
-check-prefix=CHECK-KERNEL
tejohnson wrote:
> Might as well check "tiny" and "default" as well for completeness.
I will add a check for "tiny". "default" turns out not to be a valid option.
Repository:
rC Clang
https://reviews.llvm.org/D52323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52339: Support enums with a fixed underlying type in all language modes
aaron.ballman added a comment. In https://reviews.llvm.org/D52339#1242086, @jfb wrote: > I think we should consider proposing this to the C committee. @aaron.ballman > I can help Erik write the paper, would you be able to present it? Too tight > for the upcoming meeting, but I'm guessing we have *plenty* of time for ++C17. It's already been proposed to WG14 and is currently on the SD-3 list of features to consider for C2x. See http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2008.pdf. I know Clive and am happy to point him towards this patch (when it lands) as demonstration of industry desire for the feature, in case he needs to provide updated papers. Repository: rC Clang https://reviews.llvm.org/D52339 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52364: [clangd] Initial supoprt for cross-namespace global code completion.
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
When no scope qualifier is specified, allow completing index symbols
from any scope and insert proper automatically. This is still experimental and
hidden behind a flag.
Things missing:
- Scope proximity based scoring.
- FuzzyFind supports weighted scopes.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52364
Files:
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/index/Index.h
clangd/index/MemIndex.cpp
clangd/index/dex/Dex.cpp
clangd/tool/ClangdMain.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/DexTests.cpp
Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -565,6 +565,16 @@
EXPECT_THAT(match(*I, Req), UnorderedElementsAre("a::y1"));
}
+TEST(DexTest, WildcardScope) {
+ auto I =
+ Dex::build(generateSymbols({"a::y1", "a::b::y2", "c::y3"}), URISchemes);
+ FuzzyFindRequest Req;
+ Req.Query = "y";
+ Req.Scopes = {"a::", "*"};
+ EXPECT_THAT(match(*I, Req),
+ UnorderedElementsAre("a::y1", "a::b::y2", "c::y3"));
+}
+
TEST(DexTest, IgnoreCases) {
auto I = Dex::build(generateSymbols({"ns::ABC", "ns::abc"}), URISchemes);
FuzzyFindRequest Req;
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2040,6 +2040,41 @@
}
}
+TEST(CompletionTest, CrossNamespaceCompletion) {
+ clangd::CodeCompleteOptions Opts = {};
+ Opts.IncludeIndexSymbolsFromAllScopes = true;
+
+ auto Results = completions(
+ R"cpp(
+namespace na {
+void f() { Clangd^ }
+}
+ )cpp",
+ {cls("nx::Clangd1"), cls("ny::Clangd2"), cls("Clangd3"),
+ cls("na::nb::Clangd4")},
+ Opts);
+ EXPECT_THAT(Results.Completions,
+ UnorderedElementsAre(AllOf(Qualifier("nx::"), Named("Clangd1")),
+ AllOf(Qualifier("ny::"), Named("Clangd2")),
+ AllOf(Qualifier(""), Named("Clangd3")),
+ AllOf(Qualifier("nb::"), Named("Clangd4";
+}
+
+TEST(CompletionTest, NoQualifierIfShadowed) {
+ clangd::CodeCompleteOptions Opts = {};
+ Opts.IncludeIndexSymbolsFromAllScopes = true;
+
+ auto Results = completions(R"cpp(
+namespace nx { class Clangd1 {}; }
+using nx::Clangd1;
+void f() { Clangd^ }
+ )cpp",
+ {cls("nx::Clangd1"), cls("nx::Clangd2")}, Opts);
+ EXPECT_THAT(Results.Completions,
+ UnorderedElementsAre(AllOf(Qualifier(""), Named("Clangd1")),
+ AllOf(Qualifier("nx::"), Named("Clangd2";
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -136,6 +136,15 @@
"enabled separatedly."),
llvm::cl::init(true), llvm::cl::Hidden);
+static llvm::cl::opt CrossNamespaceCompletion(
+"cross-namespace-completion",
+llvm::cl::desc(
+"This is an experimental feature. If set to true, code completion will "
+"include index symbols that are not defined in the scopes (e.g. "
+"namespaces) visible from the code completion point. Such completions "
+"can insert scope qualifiers."),
+llvm::cl::init(false), llvm::cl::Hidden);
+
static llvm::cl::opt
ShowOrigins("debug-origin",
llvm::cl::desc("Show origins of completion items"),
@@ -304,6 +313,7 @@
}
CCOpts.SpeculativeIndexRequest = Opts.StaticIndex;
CCOpts.EnableFunctionArgSnippets = EnableFunctionArgSnippets;
+ CCOpts.IncludeIndexSymbolsFromAllScopes = CrossNamespaceCompletion;
// Initialize and run ClangdLSPServer.
ClangdLSPServer LSPServer(
Index: clangd/index/dex/Dex.cpp
===
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -12,6 +12,7 @@
#include "FuzzyMatch.h"
#include "Logger.h"
#include "Quality.h"
+#include "index/Index.h"
#include "llvm/ADT/StringSet.h"
#include
#include
@@ -156,6 +157,10 @@
// Generate scope tokens for search query.
std::vector> ScopeIterators;
for (const auto &Scope : Req.Scopes) {
+if (Scope == "*") {
+ ScopeIterators.push_back(createTrue(Symbols.size()));
+ continue;
+}
const auto It = InvertedIndex.find(Token(Token::Kind::Scope, Scope));
if (It != InvertedIndex.end())
ScopeIterators.push_back(It->second.iterator());
Index: clangd/index/MemIndex.cpp
===
--- clangd/index/MemIndex.cpp
+++ clangd/inde
[PATCH] D52339: Support enums with a fixed underlying type in all language modes
jfb added a comment. I think we should consider proposing this to the C committee. @aaron.ballman I can help Erik write the paper, would you be able to present it? Too tight for the upcoming meeting, but I'm guessing we have *plenty* of time for ++C17. Repository: rC Clang https://reviews.llvm.org/D52339 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52047: [clangd] Add building benchmark and memory consumption tracking
sammccall requested changes to this revision.
sammccall added a comment.
This revision now requires changes to proceed.
This change does several things, and I think most of them need further thought.
Can we discuss Monday?
Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:96
-static void DexQueries(benchmark::State &State) {
+// This is not a *real* benchmark: it shows size of built MemIndex (in bytes).
+// Same for the next "benchmark".
This looks like it'll be really stable, why does it need to be a benchmark vs
say a dexp subcommand?
https://reviews.llvm.org/D52047
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52323: Add necessary support for storing code-model to module IR.
tejohnson added a comment.
Note that if you add a line like:
"Depends on https://reviews.llvm.org/D52322"; in the summary that Phabricator
will automatically link the two in the right way.
Comment at: lib/CodeGen/CodeGenModule.cpp:569
+ .Default(~0u);
+if ((CM != ~0u) && (CM != ~1u)) {
+ llvm::CodeModel::Model codeModel =
static_cast(CM);
Can you simplify by using a single constant for both of these (since handling
the same)?
Comment at: lib/CodeGen/CodeGenModule.h:38
#include "llvm/IR/ValueHandle.h"
+#include "llvm/Support/CodeGen.h"
#include "llvm/Transforms/Utils/SanitizerStats.h"
Since nothing changed in this header, should this new include be moved to
CodeGenModule.cpp?
Comment at: test/CodeGen/codemodels.c:2
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
-check-prefix=CHECK-NOMODEL
+// RUN: %clang_cc1 -emit-llvm -mcode-model small %s -o - | FileCheck %s
-check-prefix=CHECK-SMALL
+// RUN: %clang_cc1 -emit-llvm -mcode-model kernel %s -o - | FileCheck %s
-check-prefix=CHECK-KERNEL
Might as well check "tiny" and "default" as well for completeness.
Repository:
rC Clang
https://reviews.llvm.org/D52323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52300: [clangd] Implement VByte PostingList compression
sammccall added a comment. In https://reviews.llvm.org/D52300#1241754, @kbobyrev wrote: > I addressed the easiest issues. I'll try to implement separate storage > structure for `Head`s and `Payload`s which would potentially make the > implementation cleaner and easier to understand (and also more maintainable > since that would be way easier to go for SIMD instructions speedups and other > encoding schemes if we do that). That doesn't sound more maintainable, that sounds like a performance hack that will hurt the layering. Which is ok :-) but please don't do that until you measure a nontrivial performance improvement from it. > Also, I'll refine https://reviews.llvm.org/D52047 a little bit and I believe > that is should be way easier to understand performance + memory consumption > once we have these benchmarks in. Both @ioeric and @ilya-biryukov expressed > their concern with regard to the memory consumption "benchmark" and suggested > a separate binary. While this seems fine to me, I think it's important to > keep performance + memory tracking infrastructure easy to use (in this sense > scattering different metrics across multiple binaries makes it less > accessible and probably introduce some code duplication) and therefore using > this "trick" is OK to me, but I don't have a strong opinion about this. What > do you think, @sammccall? I may be missing some context, but you're talking about index idle memory usage right? Can't this just be a dexp command? No objection to annotating benchmark runs with memory usage too, but I wouldn't jump through hoops unless there's a strong reason. Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:121 +/// Single-byte masks used for VByte compression bit manipulations. +constexpr uint8_t SevenBytesMask = 0x7f; // 0b0111 kbobyrev wrote: > sammccall wrote: > > move to the function where they're used > But they're used both in `encodeStream()` and `decompress()`. I tried to move > as much static constants to functions where they're used, but these masks are > useful for both encoding and decoding. Is there something I should do instead > (e.g. make them members of `PostingList`)? You're right. My real objection here is that these decls are hard to understand here, and the code that uses them is also hard to understand. I think this is because they aren't very powerful abstractions (the detail abstracted is limited, and it's not strongly abstracted) and the names aren't sufficiently good. (I don't have great ones, though not confusing bits and bytes would be a start :-) My top suggestion is to inline these everywhere they're used. This is bit twiddling code, not knowing what the bits are can obscure understanding hard, and encourage you to write the code in a falsely general way. Failing that, SevenBytesMask -> LowBits and ContinuationBit -> More or HighBit? FourBytesMask isn't needed, you won't be decoding any invalid data. https://reviews.llvm.org/D52300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52047: [clangd] Add building benchmark and memory consumption tracking
ioeric added inline comments.
Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:78
+static void DexQueries(benchmark::State &State) {
+ const auto Dex = buildDex();
Why did we move this benchmark (`DexQueries`)? It seems unnecessary.
Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:106
+// benchmark report (possible options for time units are ms, ns and us).
+State.SetIterationTime(/*double Seconds=*/Mem->estimateMemoryUsage() /
+ 1000);
nit: maybe divide by `1000.0` to avoid precision loss?
Comment at: clang-tools-extra/clangd/index/SymbolYAML.cpp:187
+llvm::Expected symbolsFromFile(llvm::StringRef SymbolFilename) {
+ auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
Could you put this near the old `loadIndex` to preserve the revision history?
https://reviews.llvm.org/D52047
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52339: Support enums with a fixed underlying type in all language modes
aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. Whoops, that acceptance was accidental. Pretending I want changes just to make it clear in Phab. :-) Repository: rC Clang https://reviews.llvm.org/D52339 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52339: Support enums with a fixed underlying type in all language modes
aaron.ballman added a reviewer: aaron.ballman. aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I really like this idea in general, thank you for proposing this patch! Comment at: clang/include/clang/Basic/Features.def:90 FEATURE(objc_default_synthesize_properties, LangOpts.ObjC2) -FEATURE(objc_fixed_enum, LangOpts.ObjC2) +FEATURE(objc_fixed_enum, true) FEATURE(objc_instancetype, LangOpts.ObjC2) Is this really supported in ObjC1, or is there no longer a distinction to be made there? Comment at: clang/include/clang/Basic/Features.def:233 EXTENSION(cxx_variadic_templates, LangOpts.CPlusPlus) +EXTENSION(cxx_fixed_enum, true) // C++14 features supported by other languages as extensions. Are we sure we want to make this decision for things like OpenCL, Cuda, etc? Repository: rC Clang https://reviews.llvm.org/D52339 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342741 - [OPENMP] Disable emission of the class with vptr if they are not used in
Author: abataev
Date: Fri Sep 21 07:55:26 2018
New Revision: 342741
URL: http://llvm.org/viewvc/llvm-project?rev=342741&view=rev
Log:
[OPENMP] Disable emission of the class with vptr if they are not used in
target constructs.
Prevent compilation of the classes with the virtual tables when
compiling for the device.
Modified:
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/OpenMP/declare_target_codegen.cpp
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=342741&r1=342740&r2=342741&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Sep 21 07:55:26 2018
@@ -14918,7 +14918,8 @@ void Sema::MarkVTableUsed(SourceLocation
// Do not mark as used if compiling for the device outside of the target
// region.
if (LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
- !isInOpenMPDeclareTargetContext() && !getCurFunctionDecl())
+ !isInOpenMPDeclareTargetContext() &&
+ !isInOpenMPTargetExecutionDirective())
return;
// Try to insert this class into the map.
Modified: cfe/trunk/test/OpenMP/declare_target_codegen.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_target_codegen.cpp?rev=342741&r1=342740&r2=342741&view=diff
==
--- cfe/trunk/test/OpenMP/declare_target_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_target_codegen.cpp Fri Sep 21 07:55:26 2018
@@ -157,6 +157,16 @@ struct Bake {
template class Bake;
#pragma omp end declare target
+struct BaseNonT {
+ virtual ~BaseNonT() {}
+};
+
+#pragma omp declare target
+struct BakeNonT {
+ virtual ~BakeNonT() {}
+};
+#pragma omp end declare target
+
// CHECK-DAG: declare extern_weak signext i32 @__create()
// CHECK-NOT: define {{.*}}{{baz1|baz4|maini1|Base}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52300: [clangd] Implement VByte PostingList compression
kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:29 + DecompressedChunk = ChunkIndex->decompress(); + InnerIndex = begin(DecompressedChunk); +} sammccall wrote: > nit: we generally use members (DecompressedChunk.begin()) unless actually > dealing with arrays or templates, since lookup rules are simpler > I thought using `std::begin(Container)`, `std::end(Container)` is way more robust because the API is essentially the same if the code changes, so I used it everywhere in Dex. Do you think I should change this patch or keep it to keep the codebase more consistent? Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:121 +/// Single-byte masks used for VByte compression bit manipulations. +constexpr uint8_t SevenBytesMask = 0x7f; // 0b0111 sammccall wrote: > move to the function where they're used But they're used both in `encodeStream()` and `decompress()`. I tried to move as much static constants to functions where they're used, but these masks are useful for both encoding and decoding. Is there something I should do instead (e.g. make them members of `PostingList`)? https://reviews.llvm.org/D52300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52084: [clangd] Improve PostingList iterator string representation
kbobyrev closed this revision. kbobyrev added a comment. I think this one is addressed in the VByte patch, so I'm closing this revision in order to prevent conflicts between these two. https://reviews.llvm.org/D52084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52357: [clangd] Force Dex to respect symbol collector flags
kbobyrev updated this revision to Diff 166482.
kbobyrev marked 2 inline comments as done.
https://reviews.llvm.org/D52357
Files:
clang-tools-extra/clangd/index/dex/Dex.cpp
clang-tools-extra/unittests/clangd/DexTests.cpp
Index: clang-tools-extra/unittests/clangd/DexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexTests.cpp
@@ -583,6 +583,20 @@
EXPECT_THAT(lookup(*I, SymbolID("ns::nonono")), UnorderedElementsAre());
}
+TEST(DexTest, SymbolIndexOptionsFilter) {
+ auto CodeCompletionSymbol = symbol("Completion");
+ auto NonCodeCompletionSymbol = symbol("NoCompletion");
+ CodeCompletionSymbol.Flags = Symbol::SymbolFlag::IndexedForCodeCompletion;
+ NonCodeCompletionSymbol.Flags = Symbol::SymbolFlag::None;
+ std::vector Symbols{CodeCompletionSymbol, NonCodeCompletionSymbol};
+ Dex I(Symbols, URISchemes);
+ FuzzyFindRequest Req;
+ Req.RestrictForCodeCompletion = false;
+ EXPECT_THAT(match(I, Req), ElementsAre("Completion", "NoCompletion"));
+ Req.RestrictForCodeCompletion = true;
+ EXPECT_THAT(match(I, Req), ElementsAre("Completion"));
+}
+
TEST(DexTest, ProximityPathsBoosting) {
auto RootSymbol = symbol("root::abc");
RootSymbol.CanonicalDeclaration.FileURI = "unittest:///file.h";
Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -22,6 +22,10 @@
namespace {
+// Mark symbols which are can be used for code completion.
+static const Token RestrictedForCodeCompletion =
+Token(Token::Kind::Sentinel, "Restricted For Code Completion");
+
// Returns the tokens which are given symbol's characteristics. Currently, the
// generated tokens only contain fuzzy matching trigrams and symbol's scope,
// but in the future this will also return path proximity tokens and other
@@ -39,6 +43,8 @@
for (const auto &ProximityURI :
generateProximityURIs(Sym.CanonicalDeclaration.FileURI))
Result.emplace_back(Token::Kind::ProximityURI, ProximityURI);
+ if (Sym.Flags & Symbol::IndexedForCodeCompletion)
+Result.emplace_back(RestrictedForCodeCompletion);
return Result;
}
@@ -175,6 +181,10 @@
TopLevelChildren.push_back(createOr(move(BoostingIterators)));
}
+ if (Req.RestrictForCodeCompletion)
+TopLevelChildren.push_back(
+InvertedIndex.find(RestrictedForCodeCompletion)->second.iterator());
+
// Use TRUE iterator if both trigrams and scopes from the query are not
// present in the symbol index.
auto QueryIterator = TopLevelChildren.empty()
Index: clang-tools-extra/unittests/clangd/DexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexTests.cpp
@@ -583,6 +583,20 @@
EXPECT_THAT(lookup(*I, SymbolID("ns::nonono")), UnorderedElementsAre());
}
+TEST(DexTest, SymbolIndexOptionsFilter) {
+ auto CodeCompletionSymbol = symbol("Completion");
+ auto NonCodeCompletionSymbol = symbol("NoCompletion");
+ CodeCompletionSymbol.Flags = Symbol::SymbolFlag::IndexedForCodeCompletion;
+ NonCodeCompletionSymbol.Flags = Symbol::SymbolFlag::None;
+ std::vector Symbols{CodeCompletionSymbol, NonCodeCompletionSymbol};
+ Dex I(Symbols, URISchemes);
+ FuzzyFindRequest Req;
+ Req.RestrictForCodeCompletion = false;
+ EXPECT_THAT(match(I, Req), ElementsAre("Completion", "NoCompletion"));
+ Req.RestrictForCodeCompletion = true;
+ EXPECT_THAT(match(I, Req), ElementsAre("Completion"));
+}
+
TEST(DexTest, ProximityPathsBoosting) {
auto RootSymbol = symbol("root::abc");
RootSymbol.CanonicalDeclaration.FileURI = "unittest:///file.h";
Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -22,6 +22,10 @@
namespace {
+// Mark symbols which are can be used for code completion.
+static const Token RestrictedForCodeCompletion =
+Token(Token::Kind::Sentinel, "Restricted For Code Completion");
+
// Returns the tokens which are given symbol's characteristics. Currently, the
// generated tokens only contain fuzzy matching trigrams and symbol's scope,
// but in the future this will also return path proximity tokens and other
@@ -39,6 +43,8 @@
for (const auto &ProximityURI :
generateProximityURIs(Sym.CanonicalDeclaration.FileURI))
Result.emplace_back(Token::Kind::ProximityURI, ProximityURI);
+ if (Sym.Flags & Symbol::IndexedForCodeCompletion)
+Result.emplace_back(RestrictedForCodeCompletion);
return Result;
}
@@ -175,6 +181,10 @@
TopLevelChildren.push_back(createOr(move(BoostingIterators)));
}
+ if (Req.RestrictForCodeCompletion)
+TopLevelChildren
[PATCH] D52047: [clangd] Add building benchmark and memory consumption tracking
kbobyrev updated this revision to Diff 166481.
kbobyrev marked 3 inline comments as done.
kbobyrev added a comment.
- Be more explicit about the nature of "benchmarks" with memory tracking logic
via `State::SetLabel(...)`.
- Force single iteration for "benchmarks" with memory usage tracking
- Add another "benchmark" with `Dex` memory overhead over plain `SymbolSlab`
Huge thanks to @lebedev.ri for helping! This looks much better to me now.
https://reviews.llvm.org/D52047
Files:
clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
clang-tools-extra/clangd/index/Serialization.cpp
clang-tools-extra/clangd/index/SymbolYAML.cpp
clang-tools-extra/clangd/index/SymbolYAML.h
clang-tools-extra/clangd/index/dex/Dex.cpp
Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -124,9 +124,6 @@
for (const auto &TokenToPostingList : TempInvertedIndex)
InvertedIndex.insert({TokenToPostingList.first,
PostingList(move(TokenToPostingList.second))});
-
- vlog("Built Dex with estimated memory usage {0} bytes.",
- estimateMemoryUsage());
}
/// Constructs iterators over tokens extracted from the query and exhausts it
@@ -238,8 +235,9 @@
Bytes += SymbolQuality.size() * sizeof(float);
Bytes += LookupTable.getMemorySize();
Bytes += InvertedIndex.getMemorySize();
- for (const auto &P : InvertedIndex)
-Bytes += P.second.bytes();
+ for (const auto &TokenAndPostingList : InvertedIndex)
+Bytes += TokenAndPostingList.first.Data.size() +
+ TokenAndPostingList.second.bytes();
return Bytes + BackingDataSize;
}
Index: clang-tools-extra/clangd/index/SymbolYAML.h
===
--- clang-tools-extra/clangd/index/SymbolYAML.h
+++ clang-tools-extra/clangd/index/SymbolYAML.h
@@ -29,6 +29,9 @@
// Read symbols from a YAML-format string.
SymbolSlab symbolsFromYAML(llvm::StringRef YAMLContent);
+// Read symbols from YAML or RIFF file.
+llvm::Expected symbolsFromFile(llvm::StringRef SymbolFilename);
+
// Read one symbol from a YAML-stream.
// The returned symbol is backed by Input.
Symbol SymbolFromYAML(llvm::yaml::Input &Input);
Index: clang-tools-extra/clangd/index/SymbolYAML.cpp
===
--- clang-tools-extra/clangd/index/SymbolYAML.cpp
+++ clang-tools-extra/clangd/index/SymbolYAML.cpp
@@ -9,6 +9,7 @@
#include "SymbolYAML.h"
#include "Index.h"
+#include "Logger.h"
#include "Serialization.h"
#include "Trace.h"
#include "dex/Dex.h"
@@ -172,6 +173,7 @@
namespace clangd {
SymbolSlab symbolsFromYAML(llvm::StringRef YAMLContent) {
+ trace::Span Tracer("ParseYAML");
llvm::yaml::Input Yin(YAMLContent);
std::vector S;
Yin >> S;
@@ -182,6 +184,24 @@
return std::move(Syms).build();
}
+llvm::Expected symbolsFromFile(llvm::StringRef SymbolFilename) {
+ auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
+ if (!Buffer)
+return llvm::make_error(
+("Can't open " + SymbolFilename).str(), llvm::errc::invalid_argument);
+
+ StringRef Data = Buffer->get()->getBuffer();
+
+ if (!Data.startswith("RIFF")) { // Magic for binary index file.
+return symbolsFromYAML(Data);
+ }
+ auto RIFF = readIndexFile(Data);
+ return RIFF ? llvm::Expected(std::move(*RIFF->Symbols))
+ : llvm::make_error(
+("Can't open " + SymbolFilename).str(),
+llvm::errc::invalid_argument);
+}
+
Symbol SymbolFromYAML(llvm::yaml::Input &Input) {
Symbol S;
Input >> S;
@@ -206,30 +226,16 @@
llvm::ArrayRef URISchemes,
bool UseDex) {
trace::Span OverallTracer("LoadIndex");
- auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
- if (!Buffer) {
-llvm::errs() << "Can't open " << SymbolFilename << "\n";
-return nullptr;
- }
- StringRef Data = Buffer->get()->getBuffer();
-
- llvm::Optional Slab;
- if (Data.startswith("RIFF")) { // Magic for binary index file.
-trace::Span Tracer("ParseRIFF");
-if (auto RIFF = readIndexFile(Data))
- Slab = std::move(RIFF->Symbols);
-else
- llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
- } else {
-trace::Span Tracer("ParseYAML");
-Slab = symbolsFromYAML(Data);
- }
-
+ auto Slab = symbolsFromFile(SymbolFilename);
if (!Slab)
return nullptr;
trace::Span Tracer("BuildIndex");
- return UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
-: MemIndex::build(std::move(*Slab), RefSlab());
+ auto Index = UseDex ? dex::Dex::build(std::move(*Slab), URISchemes)
+ : MemIndex::build(std::move(*Slab), RefSlab());
+ vlog("Loaded {0} from {1} with estimated memory usage {2}",
+ UseDex ? "Dex" :
