[PATCH] D50088: [Sema] Fix an error with C++17 auto non-type template parameters
erik.pilkington updated this revision to Diff 159449.
erik.pilkington retitled this revision from "[Sema] Dig through AutoTypes that
have been deduced to an undeduced AutoType in Type::isUndeducedType" to "[Sema]
Fix an error with C++17 auto non-type template parameters".
erik.pilkington edited the summary of this revision.
erik.pilkington added a comment.
New patch uses a different approach to fix this. I edited the summary/title to
explain. Sorry for the flip-flop!
https://reviews.llvm.org/D50088
Files:
clang/lib/Sema/SemaTemplate.cpp
clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -335,3 +335,13 @@
void g(int, int);
using Int = A::B<&g>::param2;
}
+
+namespace rdar41852459 {
+template struct G {};
+
+template struct S {
+ template void f() {
+G x;
+ }
+};
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -974,7 +974,7 @@
QualType Sema::CheckNonTypeTemplateParameterType(TypeSourceInfo *&TSI,
SourceLocation Loc) {
if (TSI->getType()->isUndeducedType()) {
-// C++1z [temp.dep.expr]p3:
+// C++17 [temp.dep.expr]p3:
// An id-expression is type-dependent if it contains
//- an identifier associated by name lookup with a non-type
// template-parameter declared with a type that contains a
@@ -9866,6 +9866,15 @@
if (!NewTSI)
return true;
+if (NewTSI->getType()->isUndeducedType()) {
+ // C++17 [temp.dep.expr]p3:
+ // An id-expression is type-dependent if it contains
+ //- an identifier associated by name lookup with a non-type
+ // template-parameter declared with a type that contains a
+ // placeholder type (7.1.7.4),
+ NewTSI = SubstAutoTypeSourceInfo(NewTSI, Context.DependentTy);
+}
+
if (NewTSI != NTTP->getTypeSourceInfo()) {
NTTP->setTypeSourceInfo(NewTSI);
NTTP->setType(NewTSI->getType());
Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -335,3 +335,13 @@
void g(int, int);
using Int = A::B<&g>::param2;
}
+
+namespace rdar41852459 {
+template struct G {};
+
+template struct S {
+ template void f() {
+G x;
+ }
+};
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -974,7 +974,7 @@
QualType Sema::CheckNonTypeTemplateParameterType(TypeSourceInfo *&TSI,
SourceLocation Loc) {
if (TSI->getType()->isUndeducedType()) {
-// C++1z [temp.dep.expr]p3:
+// C++17 [temp.dep.expr]p3:
// An id-expression is type-dependent if it contains
//- an identifier associated by name lookup with a non-type
// template-parameter declared with a type that contains a
@@ -9866,6 +9866,15 @@
if (!NewTSI)
return true;
+if (NewTSI->getType()->isUndeducedType()) {
+ // C++17 [temp.dep.expr]p3:
+ // An id-expression is type-dependent if it contains
+ //- an identifier associated by name lookup with a non-type
+ // template-parameter declared with a type that contains a
+ // placeholder type (7.1.7.4),
+ NewTSI = SubstAutoTypeSourceInfo(NewTSI, Context.DependentTy);
+}
+
if (NewTSI != NTTP->getTypeSourceInfo()) {
NTTP->setTypeSourceInfo(NewTSI);
NTTP->setType(NewTSI->getType());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48896: [libcxx][c++17] P0083R5: Splicing Maps and Sets Part 2: merge
erik.pilkington added a comment. Ping! https://reviews.llvm.org/D48896 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50418: [Sema] Support for P0961R1: Relaxing the structured bindings customization point finding rules
erik.pilkington created this revision.
erik.pilkington added a reviewer: rsmith.
Herald added a subscriber: dexonsmith.
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0961r1.html
I don't believe an actual defect report was filed for this, but the paper (in
the "wording" section) claims that we should back-port this to C++17 as if it
were a defect report. Besides that, this is pretty straightforward.
Thanks for taking a look!
Erik
Repository:
rC Clang
https://reviews.llvm.org/D50418
Files:
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
clang/www/cxx_status.html
Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -755,7 +755,7 @@
http://wg21.link/p0961r1";>P0961R1 (DR)
-No
+SVN
Index: clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
===
--- clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
+++ clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
@@ -36,7 +36,7 @@
auto [a0, a1, a2] = A(); // expected-error {{undeclared identifier 'get'}} expected-note {{in implicit initialization of binding declaration 'a0'}}
}
-template float &get(A);
+template float &get(A); // expected-note 2 {{no known conversion}}
void no_tuple_element_1() {
auto [a0, a1, a2] = A(); // expected-error-re {{'std::tuple_element<0U{{L*}}, A>::type' does not name a type}} expected-note {{in implicit}}
@@ -57,7 +57,7 @@
template<> struct std::tuple_element<1, A> { typedef float &type; };
template<> struct std::tuple_element<2, A> { typedef const float &type; };
-template auto get(B) -> int (&)[N + 1];
+template auto get(B) -> int (&)[N + 1]; // expected-note 2 {{no known conversion}}
template struct std::tuple_element { typedef int type[N +1 ]; };
template struct std::tuple_size : std::tuple_size {};
@@ -138,19 +138,25 @@
return c;
}
-struct D { template struct get {}; }; // expected-note {{declared here}}
+struct D {
+ // FIXME: Emit a note here explaining why this was ignored.
+ template struct get {};
+};
template<> struct std::tuple_size { static const int value = 1; };
template<> struct std::tuple_element<0, D> { typedef D::get<0> type; };
void member_get_class_template() {
- auto [d] = D(); // expected-error {{cannot refer to member 'get' in 'D' with '.'}} expected-note {{in implicit init}}
+ auto [d] = D(); // expected-error {{no matching function for call to 'get'}} expected-note {{in implicit init}}
}
-struct E { int get(); };
+struct E {
+ // FIXME: Emit a note here explaining why this was ignored.
+ int get();
+};
template<> struct std::tuple_size { static const int value = 1; };
template<> struct std::tuple_element<0, E> { typedef int type; };
void member_get_non_template() {
// FIXME: This diagnostic is not very good.
- auto [e] = E(); // expected-error {{no member named 'get'}} expected-note {{in implicit init}}
+ auto [e] = E(); // expected-error {{no matching function for call to 'get'}} expected-note {{in implicit init}}
}
namespace ADL {
@@ -230,3 +236,35 @@
}
static_assert(g() == 4); // expected-error {{constant}} expected-note {{in call to 'g()'}}
}
+
+// P0961R1
+struct InvalidMemberGet {
+ int get();
+ template int get();
+ struct get {};
+};
+template <> struct std::tuple_size { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, InvalidMemberGet> { typedef float type; };
+template float get(InvalidMemberGet) { return 0; }
+int f() {
+ InvalidMemberGet img;
+ auto [x] = img;
+ typedef decltype(x) same_as_float;
+ typedef float same_as_float;
+}
+
+struct ValidMemberGet {
+ int get();
+ template int get() { return 0; }
+ template float get() { return 0; }
+};
+template <> struct std::tuple_size { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, ValidMemberGet> { typedef float type; };
+// Don't use this one; we should use the member get.
+template int get(ValidMemberGet) { static_assert(N && false, ""); }
+int f2() {
+ ValidMemberGet img;
+ auto [x] = img;
+ typedef decltype(x) same_as_float;
+ typedef float same_as_float;
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -1108,14 +1108,29 @@
// [dcl.decomp]p3:
// The unqualified-id get is looked up in the scope of E by class member
- // access lookup
+ // access lookup ...
LookupResult MemberGet(S, GetDN, Src->getLocation(), Sema::LookupMemberName);
bool UseMemberGet = false;
if (S.isCompleteType(Src->getLocation(), DecompType)) {
if (auto *RD = DecompType->getAsCXXRecordDecl())
S.LookupQualifiedName(MemberGet, RD);
+
+// ... and if that finds at least one declaration that is a function
+// te
[PATCH] D50418: [Sema] Support for P0961R1: Relaxing the structured bindings customization point finding rules
erik.pilkington added inline comments.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1118-1130
+// ... and if that finds at least one declaration that is a function
+// template whose first template parameter is a non-type parameter ...
+LookupResult::Filter Filter = MemberGet.makeFilter();
+while (Filter.hasNext()) {
+ NamedDecl *ND = Filter.next();
+ if (auto *FTD = dyn_cast(ND)) {
+TemplateParameterList *TPL = FTD->getTemplateParameters();
rsmith wrote:
> This should be done by walking the lookup results, not by filtering them.
> Testcase:
>
> ```
> struct A {
> int get();
> };
> struct B {
> template int get();
> };
> struct C : A, B {};
> // plus specializations of tuple_size and tuple_element
> auto [x] = C();
> ```
>
> This should be ill-formed due to ambiguity when looking up `C::get`. We
> should not resolve the ambiguity by selecting `B::get`.
Ah, I see. In the new patch we finish the class member access lookup, then we
check this condition separately. I added this test to the patch. Thanks!
https://reviews.llvm.org/D50418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50418: [Sema] Support for P0961R1: Relaxing the structured bindings customization point finding rules
erik.pilkington updated this revision to Diff 159629.
erik.pilkington added a comment.
Address review comments.
https://reviews.llvm.org/D50418
Files:
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
clang/www/cxx_status.html
Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -755,7 +755,7 @@
http://wg21.link/p0961r1";>P0961R1 (DR)
-No
+SVN
Index: clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
===
--- clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
+++ clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
@@ -36,7 +36,7 @@
auto [a0, a1, a2] = A(); // expected-error {{undeclared identifier 'get'}} expected-note {{in implicit initialization of binding declaration 'a0'}}
}
-template float &get(A);
+template float &get(A); // expected-note 2 {{no known conversion}}
void no_tuple_element_1() {
auto [a0, a1, a2] = A(); // expected-error-re {{'std::tuple_element<0U{{L*}}, A>::type' does not name a type}} expected-note {{in implicit}}
@@ -57,7 +57,7 @@
template<> struct std::tuple_element<1, A> { typedef float &type; };
template<> struct std::tuple_element<2, A> { typedef const float &type; };
-template auto get(B) -> int (&)[N + 1];
+template auto get(B) -> int (&)[N + 1]; // expected-note 2 {{no known conversion}}
template struct std::tuple_element { typedef int type[N +1 ]; };
template struct std::tuple_size : std::tuple_size {};
@@ -138,19 +138,25 @@
return c;
}
-struct D { template struct get {}; }; // expected-note {{declared here}}
+struct D {
+ // FIXME: Emit a note here explaining why this was ignored.
+ template struct get {};
+};
template<> struct std::tuple_size { static const int value = 1; };
template<> struct std::tuple_element<0, D> { typedef D::get<0> type; };
void member_get_class_template() {
- auto [d] = D(); // expected-error {{cannot refer to member 'get' in 'D' with '.'}} expected-note {{in implicit init}}
+ auto [d] = D(); // expected-error {{no matching function for call to 'get'}} expected-note {{in implicit init}}
}
-struct E { int get(); };
+struct E {
+ // FIXME: Emit a note here explaining why this was ignored.
+ int get();
+};
template<> struct std::tuple_size { static const int value = 1; };
template<> struct std::tuple_element<0, E> { typedef int type; };
void member_get_non_template() {
// FIXME: This diagnostic is not very good.
- auto [e] = E(); // expected-error {{no member named 'get'}} expected-note {{in implicit init}}
+ auto [e] = E(); // expected-error {{no matching function for call to 'get'}} expected-note {{in implicit init}}
}
namespace ADL {
@@ -230,3 +236,48 @@
}
static_assert(g() == 4); // expected-error {{constant}} expected-note {{in call to 'g()'}}
}
+
+// P0961R1
+struct InvalidMemberGet {
+ int get();
+ template int get();
+ struct get {};
+};
+template <> struct std::tuple_size { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, InvalidMemberGet> { typedef float type; };
+template float get(InvalidMemberGet) { return 0; }
+int f() {
+ InvalidMemberGet img;
+ auto [x] = img;
+ typedef decltype(x) same_as_float;
+ typedef float same_as_float;
+}
+
+struct ValidMemberGet {
+ int get();
+ template int get() { return 0; }
+ template float get() { return 0; }
+};
+template <> struct std::tuple_size { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, ValidMemberGet> { typedef float type; };
+// Don't use this one; we should use the member get.
+template int get(ValidMemberGet) { static_assert(N && false, ""); }
+int f2() {
+ ValidMemberGet img;
+ auto [x] = img;
+ typedef decltype(x) same_as_float;
+ typedef float same_as_float;
+}
+
+struct Base1 {
+ int get(); // expected-note{{member found by ambiguous name lookup}}
+};
+struct Base2 {
+ template int get(); // expected-note{{member found by ambiguous name lookup}}
+};
+struct Derived : Base1, Base2 {};
+
+template <> struct std::tuple_size { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, Derived> { typedef int type; };
+
+auto [x] = Derived(); // expected-error{{member 'get' found in multiple base classes of different types}}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -1108,15 +1108,26 @@
// [dcl.decomp]p3:
// The unqualified-id get is looked up in the scope of E by class member
- // access lookup
+ // access lookup ...
LookupResult MemberGet(S, GetDN, Src->getLocation(), Sema::LookupMemberName);
bool UseMemberGet = false;
if (S.isCompleteType(Src->getLocation(), DecompType)) {
if (auto *RD = DecompType->getAsCXXRecordDecl())
[PATCH] D50122: Complex Variable defined in InitCapture Crash fix
erik.pilkington added inline comments. Comment at: lib/Sema/SemaTemplateInstantiate.cpp:2916-2918 + if (const VarDecl *VD = dyn_cast(D)) +if (VD->isInitCapture()) + return nullptr; Why are we failing to find the instantiation of the VarDecl in this case? It seems to me like we should of picked it up in the `for` loop above. Is it not getting added to the LocalInstantiationScope? Or are we looking in the wrong LocalInstantiationScope? Repository: rC Clang https://reviews.llvm.org/D50122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50527: [Parser] Support alternative operator token keyword args in Objective-C++
erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, arphaman.
Herald added a subscriber: dexonsmith.
This fixes rdar://30741878
Thanks!
Erik
Repository:
rC Clang
https://reviews.llvm.org/D50527
Files:
clang/lib/Parse/ParseExpr.cpp
clang/test/Parser/message-expr-alt-op.mm
Index: clang/test/Parser/message-expr-alt-op.mm
===
--- /dev/null
+++ clang/test/Parser/message-expr-alt-op.mm
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+@interface WeirdInterface
+-(void)allOfThem:(int)a
+ and:(int)b
+ and_eq:(int)c
+ bitand:(int)d
+ bitor:(int)e
+ compl:(int)f
+ not:(int)g
+ not_eq:(int)h
+ or:(int)i
+ or_eq:(int)j
+ xor:(int)k
+ xor_eq:(int)l;
+
+-(void)justAnd:(int)x and:(int)y;
+-(void)and;
+-(void)and:(int)x;
+@end
+
+void call_it(WeirdInterface *x) {
+ [x allOfThem:0
+ and:0
+and_eq:0
+bitand:0
+ bitor:0
+ compl:0
+ not:0
+not_eq:0
+or:0
+ or_eq:0
+ xor:0
+xor_eq:0];
+
+ [x and];
+ [x and:0];
+ [x &&:0]; // expected-error{{expected expression}};
+ [x justAnd:0 and:1];
+ [x and: 0 ? : 1];
+}
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -263,6 +263,24 @@
return isFoldOperator(getBinOpPrecedence(Kind, GreaterThanIsOperator, true));
}
+static bool isBinaryCXXAlternativeOperatorToken(Preprocessor &PP,
+const Token &Tok) {
+ switch (Tok.getKind()) {
+ case tok::ampamp:
+ case tok::ampequal:
+ case tok::amp:
+ case tok::pipe:
+ case tok::exclaimequal:
+ case tok::pipepipe:
+ case tok::pipeequal:
+ case tok::caret:
+ case tok::caretequal:
+return isLetter(PP.getSpelling(Tok).front());
+ default:
+return false;
+ }
+}
+
/// Parse a binary expression that starts with \p LHS and has a
/// precedence of at least \p MinPrec.
ExprResult
@@ -315,6 +333,19 @@
return LHS;
}
+// In Objective-C++, alternative operator tokens can be used as keyword
args
+// in message expressions. Unconsume the token so that it can reinterpreted
+// as an identifier in ParseObjCMessageExpressionBody. i.e., we support:
+// [foo meth:0 and:0];
+// [foo not_eq];
+if (getLangOpts().ObjC1 && getLangOpts().CPlusPlus &&
+Tok.isOneOf(tok::colon, tok::r_square) &&
+isBinaryCXXAlternativeOperatorToken(PP, OpToken)) {
+ PP.EnterToken(Tok);
+ Tok = OpToken;
+ return LHS;
+}
+
// Special case handling for the ternary operator.
ExprResult TernaryMiddle(true);
if (NextTokPrec == prec::Conditional) {
Index: clang/test/Parser/message-expr-alt-op.mm
===
--- /dev/null
+++ clang/test/Parser/message-expr-alt-op.mm
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+@interface WeirdInterface
+-(void)allOfThem:(int)a
+ and:(int)b
+ and_eq:(int)c
+ bitand:(int)d
+ bitor:(int)e
+ compl:(int)f
+ not:(int)g
+ not_eq:(int)h
+ or:(int)i
+ or_eq:(int)j
+ xor:(int)k
+ xor_eq:(int)l;
+
+-(void)justAnd:(int)x and:(int)y;
+-(void)and;
+-(void)and:(int)x;
+@end
+
+void call_it(WeirdInterface *x) {
+ [x allOfThem:0
+ and:0
+and_eq:0
+bitand:0
+ bitor:0
+ compl:0
+ not:0
+not_eq:0
+or:0
+ or_eq:0
+ xor:0
+xor_eq:0];
+
+ [x and];
+ [x and:0];
+ [x &&:0]; // expected-error{{expected expression}};
+ [x justAnd:0 and:1];
+ [x and: 0 ? : 1];
+}
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -263,6 +263,24 @@
return isFoldOperator(getBinOpPrecedence(Kind, GreaterThanIsOperator, true));
}
+static bool isBinaryCXXAlternativeOperatorToken(Preprocessor &PP,
+const Token &Tok) {
+ switch (Tok.getKind()) {
+ case tok::ampamp:
+ case tok::ampequal:
+ case tok::amp:
+ case tok::pipe:
+ case tok::exclaimequal:
+ case tok::pipepipe:
+ case tok::pipeequal:
+ case tok::caret:
+ case tok::caretequal:
+return isLetter(PP.getSpelling(Tok).front());
+ default:
+return false;
+ }
+}
+
/// Parse a binary expression that starts with \p LHS and has a
/// precedence of at least \p MinPrec.
ExprResult
@@ -315,6 +333,19 @@
return LHS;
}
+// In Objective-C++, alternative operator tokens can be used as keyword args
+// in message expressions. Unconsume the token so
[PATCH] D50527: [Parser] Support alternative operator token keyword args in Objective-C++
erik.pilkington updated this revision to Diff 159995.
erik.pilkington added a comment.
Remove `isBinaryCXXAlternativeOperatorToken`, this check can be done using
`OpToken.getAsIdentifierInfo()`. Thanks!
FWIW, I can't imagine this being anything but an oversight, we go out of our
way to support these tokens in `ParseObjCSelectorPiece`, and we already happen
to support unary alternative operators, such as `[foo not]`.
https://reviews.llvm.org/D50527
Files:
clang/lib/Parse/ParseExpr.cpp
clang/test/Parser/message-expr-alt-op.mm
Index: clang/test/Parser/message-expr-alt-op.mm
===
--- /dev/null
+++ clang/test/Parser/message-expr-alt-op.mm
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+@interface WeirdInterface
+-(void)allOfThem:(int)a
+ and:(int)b
+ and_eq:(int)c
+ bitand:(int)d
+ bitor:(int)e
+ compl:(int)f
+ not:(int)g
+ not_eq:(int)h
+ or:(int)i
+ or_eq:(int)j
+ xor:(int)k
+ xor_eq:(int)l;
+
+-(void)justAnd:(int)x and:(int)y;
+-(void)and;
+-(void)and:(int)x;
+@end
+
+void call_it(WeirdInterface *x) {
+ [x allOfThem:0
+ and:0
+and_eq:0
+bitand:0
+ bitor:0
+ compl:0
+ not:0
+not_eq:0
+or:0
+ or_eq:0
+ xor:0
+xor_eq:0];
+
+ [x and];
+ [x and:0];
+ [x &&:0]; // expected-error{{expected expression}};
+ [x justAnd:0 and:1];
+ [x and: 0 ? : 1];
+}
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -315,6 +315,19 @@
return LHS;
}
+// In Objective-C++, alternative operator tokens can be used as keyword
args
+// in message expressions. Unconsume the token so that it can reinterpreted
+// as an identifier in ParseObjCMessageExpressionBody. i.e., we support:
+// [foo meth:0 and:0];
+// [foo not_eq];
+if (getLangOpts().ObjC1 && getLangOpts().CPlusPlus &&
+Tok.isOneOf(tok::colon, tok::r_square) &&
+OpToken.getIdentifierInfo() != nullptr) {
+ PP.EnterToken(Tok);
+ Tok = OpToken;
+ return LHS;
+}
+
// Special case handling for the ternary operator.
ExprResult TernaryMiddle(true);
if (NextTokPrec == prec::Conditional) {
Index: clang/test/Parser/message-expr-alt-op.mm
===
--- /dev/null
+++ clang/test/Parser/message-expr-alt-op.mm
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+@interface WeirdInterface
+-(void)allOfThem:(int)a
+ and:(int)b
+ and_eq:(int)c
+ bitand:(int)d
+ bitor:(int)e
+ compl:(int)f
+ not:(int)g
+ not_eq:(int)h
+ or:(int)i
+ or_eq:(int)j
+ xor:(int)k
+ xor_eq:(int)l;
+
+-(void)justAnd:(int)x and:(int)y;
+-(void)and;
+-(void)and:(int)x;
+@end
+
+void call_it(WeirdInterface *x) {
+ [x allOfThem:0
+ and:0
+and_eq:0
+bitand:0
+ bitor:0
+ compl:0
+ not:0
+not_eq:0
+or:0
+ or_eq:0
+ xor:0
+xor_eq:0];
+
+ [x and];
+ [x and:0];
+ [x &&:0]; // expected-error{{expected expression}};
+ [x justAnd:0 and:1];
+ [x and: 0 ? : 1];
+}
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -315,6 +315,19 @@
return LHS;
}
+// In Objective-C++, alternative operator tokens can be used as keyword args
+// in message expressions. Unconsume the token so that it can reinterpreted
+// as an identifier in ParseObjCMessageExpressionBody. i.e., we support:
+// [foo meth:0 and:0];
+// [foo not_eq];
+if (getLangOpts().ObjC1 && getLangOpts().CPlusPlus &&
+Tok.isOneOf(tok::colon, tok::r_square) &&
+OpToken.getIdentifierInfo() != nullptr) {
+ PP.EnterToken(Tok);
+ Tok = OpToken;
+ return LHS;
+}
+
// Special case handling for the ternary operator.
ExprResult TernaryMiddle(true);
if (NextTokPrec == prec::Conditional) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50527: [Parser] Support alternative operator token keyword args in Objective-C++
erik.pilkington added a comment. In https://reviews.llvm.org/D50527#1194660, @rjmccall wrote: > Assuming you've done enough source-compatibility testing to say with > reasonable confidence that this won't break anything, I think this is fine. > It's a core goal of Objective-C/C++ to allow the base language as a complete > subset if at all possible. I don't really see how this could break source-compatibility, I don't believe there is a way to start a binary operator's RHS with a colon[1] or a r_square, so this change can only affect programs that were previously ill-formed. I'll run some internal builds and report back though, just to be paranoid. [1] excluding the GNU conditional expr, which is handled because getIdentifierInfo will always return nullptr for '?'. https://reviews.llvm.org/D50527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50527: [Parser] Support alternative operator token keyword args in Objective-C++
erik.pilkington added a comment. The build came back clean! https://reviews.llvm.org/D50527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48896: [libcxx][c++17] P0083R5: Splicing Maps and Sets Part 2: merge
erik.pilkington added a comment. Ping! https://reviews.llvm.org/D48896 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50994: Add a new flag and attributes to control static destructor registration
erik.pilkington created this revision.
erik.pilkington added reviewers: jfb, rsmith, aaron.ballman, rjmccall, bruno.
Herald added a subscriber: dexonsmith.
See the recent thread here:
http://lists.llvm.org/pipermail/cfe-dev/2018-July/058494.html
This patch adds the flag -fno-c++-static-destructors, which disables
registration of exit-time destructors of static or thread storage duration
variables. This patch also adds [[clang::no_destroy]] and
[[clang::always_destroy]] attributes for disabling dtors in
-fc++-static-destructors mode, and enabling dtors in
-fno-c++-static-destructors mode, respectively. This patch was based on
@bruno's https://reviews.llvm.org/D22474.
rdar://21734598
Thanks for taking a look!
Erik
Repository:
rC Clang
https://reviews.llvm.org/D50994
Files:
clang/include/clang/AST/Decl.h
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/Options.td
clang/lib/AST/Decl.cpp
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/always_destroy.cpp
clang/test/CodeGenCXX/no_destroy.cpp
clang/test/Misc/pragma-attribute-supported-attributes-list.test
clang/test/SemaCXX/no_destroy.cpp
Index: clang/test/SemaCXX/no_destroy.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/no_destroy.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -DNO_DTORS -fno-c++-static-destructors -verify %s
+// RUN: %clang_cc1 -verify %s
+
+struct SecretDestructor {
+#ifndef NO_DTORS
+ // expected-note@+2 4 {{private}}
+#endif
+private: ~SecretDestructor(); // expected-note 2 {{private}}
+};
+
+SecretDestructor sd1;
+thread_local SecretDestructor sd2;
+void locals() {
+ static SecretDestructor sd3;
+ thread_local SecretDestructor sd4;
+}
+
+#ifndef NO_DTORS
+// SecretDestructor sd1; // expected-error@-8 {{private}}
+// thread_local SecretDestructor sd2; // expected-error@-8 {{private}}
+// void locals() {
+// static SecretDestructor sd3; // expected-error@-8 {{private}}
+// thread_local SecretDestructor sd4; // expected-error@-8 {{private}}
+// }
+#endif
+
+[[clang::always_destroy]] SecretDestructor sd6; // expected-error{{private}}
+[[clang::always_destroy]] thread_local SecretDestructor sd7; // expected-error{{private}}
+
+[[clang::no_destroy]] SecretDestructor sd8;
+
+int main() {
+ [[clang::no_destroy]] int p; // expected-error{{no_destroy attribute must be applied to a variable with static or thread storage duration}}
+ [[clang::always_destroy]] int p2; // expected-error{{always_destroy attribute must be applied to a variable with static or thread storage duration}}
+ [[clang::no_destroy]] static int p3;
+ [[clang::always_destroy]] static int p4;
+}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -2,15 +2,16 @@
// The number of supported attributes should never go down!
-// CHECK: #pragma clang attribute supports 72 attributes:
+// CHECK: #pragma clang attribute supports 74 attributes:
// CHECK-NEXT: AMDGPUFlatWorkGroupSize (SubjectMatchRule_function)
// CHECK-NEXT: AMDGPUNumSGPR (SubjectMatchRule_function)
// CHECK-NEXT: AMDGPUNumVGPR (SubjectMatchRule_function)
// CHECK-NEXT: AMDGPUWavesPerEU (SubjectMatchRule_function)
// CHECK-NEXT: AVRSignal (SubjectMatchRule_function)
// CHECK-NEXT: AbiTag (SubjectMatchRule_record_not_is_union, SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_namespace)
// CHECK-NEXT: AlignValue (SubjectMatchRule_variable, SubjectMatchRule_type_alias)
// CHECK-NEXT: AllocSize (SubjectMatchRule_function)
+// CHECK-NEXT: AlwaysDestroy (SubjectMatchRule_variable)
// CHECK-NEXT: Annotate ()
// CHECK-NEXT: AnyX86NoCfCheck (SubjectMatchRule_hasType_functionType)
// CHECK-NEXT: AssumeAligned (SubjectMatchRule_objc_method, SubjectMatchRule_function)
@@ -38,6 +39,7 @@
// CHECK-NEXT: MipsLongCall (SubjectMatchRule_function)
// CHECK-NEXT: MipsShortCall (SubjectMatchRule_function)
// CHECK-NEXT: NoDebug (SubjectMatchRule_hasType_functionType, SubjectMatchRule_objc_method, SubjectMatchRule_variable_not_is_parameter)
+// CHECK-NEXT: NoDestroy (SubjectMatchRule_variable)
// CHECK-NEXT: NoDuplicate (SubjectMatchRule_function)
// CHECK-NEXT: NoEscape (SubjectMatchRule_variable_is_parameter)
// CHECK-NEXT: NoMicroMips (SubjectMatchRule_function)
Index: clang/test/CodeGenCXX/no_destroy.cpp
===
--- /dev/null
+++ clang/test/CodeGenC
[PATCH] D50527: [Parser] Support alternative operator token keyword args in Objective-C++
erik.pilkington added a comment. Ping! https://reviews.llvm.org/D50527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50994: Add a new flag and attributes to control static destructor registration
erik.pilkington updated this revision to Diff 161594.
erik.pilkington marked 7 inline comments as done.
erik.pilkington added a comment.
In this new patch:
- Fix some grammar errors in the documentation.
- Add a testcase for both attributes appearing on a variable.
- Just use hasArg() in the driver.
- Improve the diagnostic.
Thanks for the feedback!
https://reviews.llvm.org/D50994
Files:
clang/include/clang/AST/Decl.h
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/Options.td
clang/lib/AST/Decl.cpp
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/always_destroy.cpp
clang/test/CodeGenCXX/no_destroy.cpp
clang/test/Misc/pragma-attribute-supported-attributes-list.test
clang/test/SemaCXX/no_destroy.cpp
Index: clang/test/SemaCXX/no_destroy.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/no_destroy.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -DNO_DTORS -fno-c++-static-destructors -verify %s
+// RUN: %clang_cc1 -verify %s
+
+struct SecretDestructor {
+#ifndef NO_DTORS
+ // expected-note@+2 4 {{private}}
+#endif
+private: ~SecretDestructor(); // expected-note 2 {{private}}
+};
+
+SecretDestructor sd1;
+thread_local SecretDestructor sd2;
+void locals() {
+ static SecretDestructor sd3;
+ thread_local SecretDestructor sd4;
+}
+
+#ifndef NO_DTORS
+// SecretDestructor sd1; // expected-error@-8 {{private}}
+// thread_local SecretDestructor sd2; // expected-error@-8 {{private}}
+// void locals() {
+// static SecretDestructor sd3; // expected-error@-8 {{private}}
+// thread_local SecretDestructor sd4; // expected-error@-8 {{private}}
+// }
+#endif
+
+[[clang::always_destroy]] SecretDestructor sd6; // expected-error{{private}}
+[[clang::always_destroy]] thread_local SecretDestructor sd7; // expected-error{{private}}
+
+[[clang::no_destroy]] SecretDestructor sd8;
+
+int main() {
+ [[clang::no_destroy]] int p; // expected-error{{no_destroy attribute can only be applied to a variable with static or thread storage duration}}
+ [[clang::always_destroy]] int p2; // expected-error{{always_destroy attribute can only be applied to a variable with static or thread storage duration}}
+ [[clang::no_destroy]] static int p3;
+ [[clang::always_destroy]] static int p4;
+}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -2,15 +2,16 @@
// The number of supported attributes should never go down!
-// CHECK: #pragma clang attribute supports 72 attributes:
+// CHECK: #pragma clang attribute supports 74 attributes:
// CHECK-NEXT: AMDGPUFlatWorkGroupSize (SubjectMatchRule_function)
// CHECK-NEXT: AMDGPUNumSGPR (SubjectMatchRule_function)
// CHECK-NEXT: AMDGPUNumVGPR (SubjectMatchRule_function)
// CHECK-NEXT: AMDGPUWavesPerEU (SubjectMatchRule_function)
// CHECK-NEXT: AVRSignal (SubjectMatchRule_function)
// CHECK-NEXT: AbiTag (SubjectMatchRule_record_not_is_union, SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_namespace)
// CHECK-NEXT: AlignValue (SubjectMatchRule_variable, SubjectMatchRule_type_alias)
// CHECK-NEXT: AllocSize (SubjectMatchRule_function)
+// CHECK-NEXT: AlwaysDestroy (SubjectMatchRule_variable)
// CHECK-NEXT: Annotate ()
// CHECK-NEXT: AnyX86NoCfCheck (SubjectMatchRule_hasType_functionType)
// CHECK-NEXT: AssumeAligned (SubjectMatchRule_objc_method, SubjectMatchRule_function)
@@ -38,6 +39,7 @@
// CHECK-NEXT: MipsLongCall (SubjectMatchRule_function)
// CHECK-NEXT: MipsShortCall (SubjectMatchRule_function)
// CHECK-NEXT: NoDebug (SubjectMatchRule_hasType_functionType, SubjectMatchRule_objc_method, SubjectMatchRule_variable_not_is_parameter)
+// CHECK-NEXT: NoDestroy (SubjectMatchRule_variable)
// CHECK-NEXT: NoDuplicate (SubjectMatchRule_function)
// CHECK-NEXT: NoEscape (SubjectMatchRule_variable_is_parameter)
// CHECK-NEXT: NoMicroMips (SubjectMatchRule_function)
Index: clang/test/CodeGenCXX/no_destroy.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/no_destroy.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 %s -emit-llvm -triple x86_64-apple-macosx10.13.0 -o - | FileCheck %s
+
+struct NonTrivial {
+ ~NonTrivial();
+};
+
+// CHECK-NOT: __cxa_atexit{{.*}}_ZN10NonTrivialD1Ev
+[[clang::no_destroy]] NonTrivial nt1;
+// CHECK-NOT: _tlv_atexit{{.*}}_ZN10NonTrivialD1Ev
+[[clang::no_destroy]] thread_local NonTrivial nt2;
+
+struct NonTrivial2 {
+
[PATCH] D50994: Add a new flag and attributes to control static destructor registration
erik.pilkington added inline comments. Comment at: clang/include/clang/AST/Decl.h:1472 + /// Do we need to emit an exit-time destructor for this variable? + bool isNoDestroy(const ASTContext &) const; jfb wrote: > rsmith wrote: > > jfb wrote: > > > This is only valid for static variables, right? It's probably better to > > > document the function name that way, e.g. `isStaticWithNoDestroy`. > > I think the question (and hence current function name) is meaningful for > > any variable, but it just happens that the answer will always be "no" for > > non-static variables (for now, at least). Are you concerned that people > > will think calls to this function are missing from codepaths that only deal > > with automatic storage duration variables with the current name? > Yeah it seems like "this variable has no destructor". I guess even trivial > automatic variables have a (trivial) destructor, so maybe it's not ambiguous? > Up to y'all :) If anyone has a strong preference I'd be happy either way! Comment at: clang/include/clang/Basic/LangOptions.def:311 +LANGOPT(RegisterStaticDestructors, 1, 1, "Register C++ static destructors") + rsmith wrote: > Should be a `BENIGN_LANGOPT` because it doesn't affect AST construction, only > the generated code. It does affect AST construction though, we don't mark a static VarDecl's type's destructor referenced in this mode, or check it's access (because we aren't actually using it). Do you think this is the wrong choice of semantics? Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5931 +handleSimpleAttributeWithExclusions(S, D, A); + } +} jfb wrote: > This allows a variable with both attributes :) > Looking at the code above it seems like having both attributes is equivalent > to no-destroy. `handleSimpleAttributeWithExclusions` handles that automagically, i.e.: ``` $ cat t.cpp [[clang::no_destroy]] [[clang::always_destroy]] int x; $ ~/dbg-bld/bin/clang t.cpp t.cpp:1:25: error: 'always_destroy' and 'no_destroy' attributes are not compatible [[clang::no_destroy]] [[clang::always_destroy]] int x; ^ t.cpp:1:3: note: conflicting attribute is here [[clang::no_destroy]] [[clang::always_destroy]] int x; ^ 1 error generated. ``` I'll add a test to prove this though... https://reviews.llvm.org/D50994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50994: Add a new flag and attributes to control static destructor registration
erik.pilkington added a comment. Sure, done in 340311! Repository: rC Clang https://reviews.llvm.org/D50994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48896: [libcxx][c++17] P0083R5: Splicing Maps and Sets Part 2: merge
erik.pilkington added a comment. Ping! https://reviews.llvm.org/D48896 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46747: [Sema] Use dotted form of macOS version for unguarded availability FixIts
erik.pilkington added a comment. That test passes for me without this patch applied, why is UsesUnderscores actually getting set in the radar? Repository: rC Clang https://reviews.llvm.org/D46747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46747: [Sema] Use dotted form of macOS version for unguarded availability FixIts
erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land. Thanks for the explanation, LGTM. https://reviews.llvm.org/D46747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets
erik.pilkington added a comment. > One way we could deal with this is by adding an attribute to the compiler to > indicate "the const is a lie", that we can apply to `std::pair::first`, with > the semantics being that a top-level `const` is ignored when determining the > "real" type of the member (and so mutating the member after a `const_cast` > has defined behavior). This would apply to //all// `std::pair`s, not just > the `node_handle` case, but in practice we don't optimize on the basis of a > member being declared `const` *anyway*, so this isn't such a big deal right > now. I'm a fan of this idea, this would also have the bonus of fixing some pre-existing type-punning between `pair` and `pair` (see `__hash_value_type` and `__value_type` in `` and ``). > Another possibility would be a compiler intrinsic to change the type of the > pair, in-place, from a `std::pair` to a `std::pair`, > without changing anything about the members -- except that the `first` member > changes type. How could this work? It would still be possible for TBAA to assume that `void m(std::pair*, std::pair*);`'s parameters don't alias when optimizing `m`, regardless of how we form the non-const pair, right? > Yet another possibility would be to only ever access the `key` field through > a type annotated with `__attribute__((may_alias))`, and then launder the node > pointer when we're ready to insert it back into the container (to "pick up" > the new value of the const member). That's formally breaking the rules (a > `launder` isn't enough, because we didn't actually create a new object, and > in any case we still mutated the value of a `const` subobject), but it'll > probably work fine across compilers that support the attribute. I think we could fall back to this in cases where the attribute from idea (1) isn't available. There, we could mark std::pair as may_alias and still fix `__hash_value_type` and `__value_type`. This would pessimize std::pair, but only for "older" compilers. So unless I've gotten this all wrong, I'll write a patch to support that new attribute and come back to this patch if/when that lands. Sound good? Thanks for the feedback! Erik https://reviews.llvm.org/D46845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46747: [Sema] Use dotted form of macOS version for unguarded availability FixIts
erik.pilkington added a comment. > I am now thinking about just switching to dot format in > ParseAvailabilityAttribute() because it's much simpler to maintain > consistency that way. Okay, but if we're going to treat underscores as dots while printing diagnostics and fixits (which I think we should), then we shouldn't track the spelling in VersionTuple: we no longer have any use for it. The only reason that VersionTuple supports this is because of r219124, which was written so that diagnostic printing would be consistent with how the attribute was spelled. Maybe there was a good reason for this, but I can't see the radar that was linked in the commit message. Can you look up that radar to see what r219124 was actually trying to fix? I don't think we should move forward with this until we have that context. Thanks! Erik https://reviews.llvm.org/D46747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46747: [Sema] Use dotted form of macOS version for unguarded availability FixIts
erik.pilkington accepted this revision. erik.pilkington added a comment. Great, LGTM! https://reviews.llvm.org/D46747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets
erik.pilkington added a comment. In https://reviews.llvm.org/D46845#1098634, @rsmith wrote: > One way we could deal with this is by adding an attribute to the compiler to > indicate "the const is a lie", that we can apply to `std::pair::first`, with > the semantics being that a top-level `const` is ignored when determining the > "real" type of the member (and so mutating the member after a `const_cast` > has defined behavior). This would apply to //all// `std::pair`s, not just > the `node_handle` case, but in practice we don't optimize on the basis of a > member being declared `const` *anyway*, so this isn't such a big deal right > now. Would you mind elaborating on this? If we don't optimize on a member being const, then what should this attribute actually do? The only thing I can think of is making a field with type `const T` with the attribute claim it's a `T` to TBAA, but that would be a no-op because loads and stores through those types are compatible anyways. In https://reviews.llvm.org/D46845#1098634, @rsmith wrote: > Yet another possibility would be to only ever access the `key` field through > a type annotated with `__attribute__((may_alias))`, and then launder the node > pointer when we're ready to insert it back into the container (to "pick up" > the new value of the const member). That's formally breaking the rules (a > `launder` isn't enough, because we didn't actually create a new object, and > in any case we still mutated the value of a `const` subobject), but it'll > probably work fine across compilers that support the attribute. What about instead of type-punning between pair and pair (with may_alias) we just did a const_cast in key, and launder the pair when finished? Because loads and stores to `const K` and `K` are already assumed to alias each other, we should be able to omit the may_alias attribute (right?). Would that be enough to placate the compiler? If it is, then I think we should just do this and avoid adding another attribute to clang. I'll upload a patch with this to show what I mean. Thanks! Erik https://reviews.llvm.org/D46845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38483: [ExprConstant] Allow constexpr ctor to modify non static data members in body
erik.pilkington created this revision.
Previously, clang rejected the following (copied from PR19741):
struct A {
int a = 0;
constexpr A() { a = 1; }
};
constexpr bool f() {
constexpr A a;
static_assert(a.a == 1, "");
return a.a == 1;
}
static_assert(f(), "");
Clang didn't like the assignment to `a` in `A()` because it doesn't know that A
is being constructed and therefore considers the subobject A.a to be const. The
fix in this patch (which @rsmith suggested in the PR) is just to keep track of
the set of objects that are currently being constructed, and strip away the
const whenever we encounter a subobject of an object under construction.
https://bugs.llvm.org/show_bug.cgi?id=19741
Thanks for taking a look!
Erik
https://reviews.llvm.org/D38483
Files:
lib/AST/ExprConstant.cpp
test/SemaCXX/constant-expression-cxx1y.cpp
Index: test/SemaCXX/constant-expression-cxx1y.cpp
===
--- test/SemaCXX/constant-expression-cxx1y.cpp
+++ test/SemaCXX/constant-expression-cxx1y.cpp
@@ -988,3 +988,36 @@
void();
}
constexpr int void_test = (Void(0), 1);
+
+namespace PR19741 {
+constexpr void addone(int &m) { m++; }
+
+struct S {
+ int m = 0;
+ constexpr S() { addone(m); }
+};
+constexpr bool evalS() {
+ constexpr S s;
+ return s.m == 1;
+}
+static_assert(evalS(), "");
+
+struct Nested {
+ struct First { int x = 42; };
+ union {
+First first;
+int second;
+ };
+ int x;
+ constexpr Nested(int x) : first(), x(x) { x = 4; }
+ constexpr Nested() : Nested(42) {
+addone(first.x);
+x = 3;
+ }
+};
+constexpr bool evalNested() {
+ constexpr Nested N;
+ return N.first.x == 43;
+}
+static_assert(evalNested(), "");
+} // namespace PR19741
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -573,6 +573,30 @@
/// declaration whose initializer is being evaluated, if any.
APValue *EvaluatingDeclValue;
+typedef std::pair EvaluatingObject;
+
+/// EvaluatingConstructors - Set of objects that are currently being
+/// constructed.
+llvm::DenseSet EvaluatingConstructors;
+
+struct EvaluatingConstructorRAII {
+ EvalInfo &EI;
+ EvaluatingObject Object;
+ bool DidInsert;
+ EvaluatingConstructorRAII(EvalInfo &EI, EvaluatingObject Object)
+ : EI(EI), Object(Object) {
+DidInsert = EI.EvaluatingConstructors.insert(Object).second;
+ }
+ ~EvaluatingConstructorRAII() {
+if (DidInsert) EI.EvaluatingConstructors.erase(Object);
+ }
+};
+
+bool isEvaluatingConstructor(APValue::LValueBase Decl, unsigned CallIndex)
{
+ return Decl == EvaluatingDecl ||
+EvaluatingConstructors.count(EvaluatingObject(Decl, CallIndex));
+}
+
/// The current array initialization index, if we're performing array
/// initialization.
uint64_t ArrayInitIndex = -1;
@@ -3101,7 +3125,7 @@
// FIXME: We don't set up EvaluatingDecl for local variables or temporaries,
// and this doesn't do quite the right thing for const subobjects of the
// object under construction.
- if (LVal.getLValueBase() == Info.EvaluatingDecl) {
+ if (Info.isEvaluatingConstructor(LVal.getLValueBase(), LVal.CallIndex)) {
BaseType = Info.Ctx.getCanonicalType(BaseType);
BaseType.removeLocalConst();
}
@@ -4254,6 +4278,8 @@
return false;
}
+ EvalInfo::EvaluatingConstructorRAII EvalObj(
+ Info, {This.getLValueBase(), This.CallIndex});
CallStackFrame Frame(Info, CallLoc, Definition, &This, ArgValues);
// FIXME: Creating an APValue just to hold a nonexistent return value is
Index: test/SemaCXX/constant-expression-cxx1y.cpp
===
--- test/SemaCXX/constant-expression-cxx1y.cpp
+++ test/SemaCXX/constant-expression-cxx1y.cpp
@@ -988,3 +988,36 @@
void();
}
constexpr int void_test = (Void(0), 1);
+
+namespace PR19741 {
+constexpr void addone(int &m) { m++; }
+
+struct S {
+ int m = 0;
+ constexpr S() { addone(m); }
+};
+constexpr bool evalS() {
+ constexpr S s;
+ return s.m == 1;
+}
+static_assert(evalS(), "");
+
+struct Nested {
+ struct First { int x = 42; };
+ union {
+First first;
+int second;
+ };
+ int x;
+ constexpr Nested(int x) : first(), x(x) { x = 4; }
+ constexpr Nested() : Nested(42) {
+addone(first.x);
+x = 3;
+ }
+};
+constexpr bool evalNested() {
+ constexpr Nested N;
+ return N.first.x == 43;
+}
+static_assert(evalNested(), "");
+} // namespace PR19741
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -573,6 +573,30 @@
/// declaration whose initializer is being evaluated, if any.
APValue *EvaluatingDeclValue;
+typedef std::pair EvaluatingObject;
+
+/// EvaluatingCon
[PATCH] D38483: [ExprConstant] Allow constexpr ctor to modify non static data members in body
erik.pilkington updated this revision to Diff 117529.
erik.pilkington marked 2 inline comments as done.
erik.pilkington added a comment.
Thanks for the feedback, in this new patch:
- insert EvaluatingDecl into EvaluatingConstructors instead of checking it in
isEvaluatingDecl()
- Add a comment to the typedef
https://reviews.llvm.org/D38483
Files:
lib/AST/ExprConstant.cpp
test/SemaCXX/constant-expression-cxx1y.cpp
Index: test/SemaCXX/constant-expression-cxx1y.cpp
===
--- test/SemaCXX/constant-expression-cxx1y.cpp
+++ test/SemaCXX/constant-expression-cxx1y.cpp
@@ -988,3 +988,36 @@
void();
}
constexpr int void_test = (Void(0), 1);
+
+namespace PR19741 {
+constexpr void addone(int &m) { m++; }
+
+struct S {
+ int m = 0;
+ constexpr S() { addone(m); }
+};
+constexpr bool evalS() {
+ constexpr S s;
+ return s.m == 1;
+}
+static_assert(evalS(), "");
+
+struct Nested {
+ struct First { int x = 42; };
+ union {
+First first;
+int second;
+ };
+ int x;
+ constexpr Nested(int x) : first(), x(x) { x = 4; }
+ constexpr Nested() : Nested(42) {
+addone(first.x);
+x = 3;
+ }
+};
+constexpr bool evalNested() {
+ constexpr Nested N;
+ return N.first.x == 43;
+}
+static_assert(evalNested(), "");
+} // namespace PR19741
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -573,6 +573,31 @@
/// declaration whose initializer is being evaluated, if any.
APValue *EvaluatingDeclValue;
+/// EvaluatingObject - Pair of the AST node that an lvalue represents and
+/// the call index that that lvalue was allocated in.
+typedef std::pair EvaluatingObject;
+
+/// EvaluatingConstructors - Set of objects that are currently being
+/// constructed.
+llvm::DenseSet EvaluatingConstructors;
+
+struct EvaluatingConstructorRAII {
+ EvalInfo &EI;
+ EvaluatingObject Object;
+ bool DidInsert;
+ EvaluatingConstructorRAII(EvalInfo &EI, EvaluatingObject Object)
+ : EI(EI), Object(Object) {
+DidInsert = EI.EvaluatingConstructors.insert(Object).second;
+ }
+ ~EvaluatingConstructorRAII() {
+if (DidInsert) EI.EvaluatingConstructors.erase(Object);
+ }
+};
+
+bool isEvaluatingConstructor(APValue::LValueBase Decl, unsigned CallIndex) {
+ return EvaluatingConstructors.count(EvaluatingObject(Decl, CallIndex));
+}
+
/// The current array initialization index, if we're performing array
/// initialization.
uint64_t ArrayInitIndex = -1;
@@ -666,6 +691,7 @@
void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value) {
EvaluatingDecl = Base;
EvaluatingDeclValue = &Value;
+ EvaluatingConstructors.insert({Base, 0});
}
const LangOptions &getLangOpts() const { return Ctx.getLangOpts(); }
@@ -3101,7 +3127,7 @@
// FIXME: We don't set up EvaluatingDecl for local variables or temporaries,
// and this doesn't do quite the right thing for const subobjects of the
// object under construction.
- if (LVal.getLValueBase() == Info.EvaluatingDecl) {
+ if (Info.isEvaluatingConstructor(LVal.getLValueBase(), LVal.CallIndex)) {
BaseType = Info.Ctx.getCanonicalType(BaseType);
BaseType.removeLocalConst();
}
@@ -4254,6 +4280,8 @@
return false;
}
+ EvalInfo::EvaluatingConstructorRAII EvalObj(
+ Info, {This.getLValueBase(), This.CallIndex});
CallStackFrame Frame(Info, CallLoc, Definition, &This, ArgValues);
// FIXME: Creating an APValue just to hold a nonexistent return value is
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38483: [ExprConstant] Allow constexpr ctor to modify non static data members in body
erik.pilkington added inline comments.
Comment at: lib/AST/ExprConstant.cpp:588
+ : EI(EI), Object(Object) {
+DidInsert = EI.EvaluatingConstructors.insert(Object).second;
+ }
rsmith wrote:
> Can the `DidInsert == false` case actually happen?
Yep, if you have the following:
```
struct Outer {
struct Inner {
constexpr Inner() {}
};
Inner i;
constexpr Outer() {}
};
constexpr Outer o;
```
There are two ctor calls for the LValueBase `constexpr Outer o;` at call index
0.
https://reviews.llvm.org/D38483
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51172: [libcxx] Comment out #define __cpp_lib_node_extract, we only support half of that functionality
erik.pilkington created this revision. erik.pilkington added reviewers: ldionne, mclow.lists, hans. Herald added a reviewer: EricWF. Herald added subscribers: dexonsmith, christof. The other half of this is in https://reviews.llvm.org/D48896, so we shouldn't claim that we support this feature. This should be cherry-picked into the 7.0 release, since I believe the first half landed before the branch point. Repository: rCXX libc++ https://reviews.llvm.org/D51172 Files: libcxx/include/__node_handle Index: libcxx/include/__node_handle === --- libcxx/include/__node_handle +++ libcxx/include/__node_handle @@ -26,7 +26,8 @@ #if _LIBCPP_STD_VER > 14 -#define __cpp_lib_node_extract 201606L +// FIXME: Uncomment this when we support the 'merge' functionality. +// #define __cpp_lib_node_extract 201606L // Specialized in __tree & __hash_table for their _NodeType. template Index: libcxx/include/__node_handle === --- libcxx/include/__node_handle +++ libcxx/include/__node_handle @@ -26,7 +26,8 @@ #if _LIBCPP_STD_VER > 14 -#define __cpp_lib_node_extract 201606L +// FIXME: Uncomment this when we support the 'merge' functionality. +// #define __cpp_lib_node_extract 201606L // Specialized in __tree & __hash_table for their _NodeType. template ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51172: [libcxx] Comment out #define __cpp_lib_node_extract, we only support half of that functionality
erik.pilkington added a comment. Landed as r340544. @hans: Can you cherry-pick? Repository: rCXX libc++ https://reviews.llvm.org/D51172 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init
erik.pilkington created this revision.
erik.pilkington added a reviewer: arphaman.
Herald added a subscriber: dexonsmith.
rdar://18335828
Thanks!
Erik
Repository:
rC Clang
https://reviews.llvm.org/D51189
Files:
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/DeclObjC.h
clang/include/clang/AST/NSAPI.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/DeclObjC.cpp
clang/lib/AST/NSAPI.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprObjC.cpp
clang/test/SemaObjC/infer-availability-from-init.m
Index: clang/test/SemaObjC/infer-availability-from-init.m
===
--- /dev/null
+++ clang/test/SemaObjC/infer-availability-from-init.m
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx-10.9 -Wunguarded-availability -fblocks -fsyntax-only -verify %s
+
+__attribute__((objc_root_class))
+@interface NSObject
++(instancetype)new;
+-(instancetype)init;
+@end
+
+@interface MyObject : NSObject
+-(instancetype)init __attribute__((unavailable)); // expected-note{{'init' has been explicitly marked unavailable here}}
+@end
+
+void usemyobject() {
+ [MyObject new]; // expected-error{{'new' is unavailable}}
+}
+
+@interface MyOtherObject : NSObject
++(instancetype)init __attribute__((unavailable));
++(instancetype)new;
+@end
+
+void usemyotherobject() {
+ [MyOtherObject new]; // no error; new is overrideen.
+}
+
+@interface NotGoodOverride : NSObject
++(instancetype)init __attribute__((unavailable));
+-(instancetype)new;
++(instancetype)new: (int)x;
+@end
+
+void usenotgoodoverride() {
+ [NotGoodOverride new]; // no error
+}
+
+@interface NotNSObject
++(instancetype)new;
+-(instancetype)init;
+@end
+
+@interface NotMyObject : NotNSObject
+-(instancetype)init __attribute__((unavailable));
+@end
+
+void usenotmyobject() {
+ [NotMyObject new]; // no error; this isn't NSObject
+}
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2471,7 +2471,8 @@
if (!Method)
Method = Class->lookupPrivateClassMethod(Sel);
-if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs,
+nullptr, false, false, Class))
return ExprError();
}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -206,7 +206,8 @@
bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef Locs,
const ObjCInterfaceDecl *UnknownObjCClass,
bool ObjCPropertyAccess,
- bool AvoidPartialAvailabilityChecks) {
+ bool AvoidPartialAvailabilityChecks,
+ ObjCInterfaceDecl *CD) {
SourceLocation Loc = Locs.front();
if (getLangOpts().CPlusPlus && isa(D)) {
// If there were any diagnostics suppressed by template argument deduction,
@@ -292,7 +293,7 @@
}
DiagnoseAvailabilityOfDecl(D, Locs, UnknownObjCClass, ObjCPropertyAccess,
- AvoidPartialAvailabilityChecks);
+ AvoidPartialAvailabilityChecks, CD);
DiagnoseUnusedOfDecl(*this, D, Loc);
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6943,8 +6943,12 @@
/// \param D The declaration to check.
/// \param Message If non-null, this will be populated with the message from
/// the availability attribute that is selected.
+/// \param ClassMessageReceiver If we're checking the the method of a class
+/// message send, the class. Otherwise nullptr.
static std::pair
-ShouldDiagnoseAvailabilityOfDecl(const NamedDecl *D, std::string *Message) {
+ShouldDiagnoseAvailabilityOfDecl(Sema &S, const NamedDecl *D,
+ std::string *Message,
+ ObjCInterfaceDecl *ClassMessageReceiver) {
AvailabilityResult Result = D->getAvailability(Message);
// For typedefs, if the typedef declaration appears available look
@@ -6977,6 +6981,20 @@
}
}
+ // For +new, infer availability from -init.
+ if (const auto *MD = dyn_cast(D)) {
+if (S.NSAPIObj != nullptr && ClassMessageReceiver != nullptr) {
+ ObjCMethodDecl *Init = ClassMessageReceiver->lookupInstanceMethod(
+ S.NSAPIObj->getInitSelector());
+ if (Init != nullptr && Result == AR_Available && MD->isClassMethod() &&
+ MD->getSelector() == S.NSAPIObj->getNewSelector() &&
+ MD->definedInNSObject(S.getASTContext())) {
+Result = Init->getAvailability(Message);
+D = Init;
+ }
+}
+ }
+
return {Result, D};
}
@@ -75
[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init
erik.pilkington added a comment. In https://reviews.llvm.org/D51189#1211754, @arphaman wrote: > Hmm, I don't think this solution is ideal, we'd rather have an attribute > somewhere for other consumers of availability annotations. Does MyObject have > an implicit decl of `new`, or are we referring to `NSObject`s `new`? Ideally > we would an attribute on a particular `new` instead, but that might not work. We're referring to NSObject's new. I don't think it's unreasonable to ask users who override init to be unavailable also override new with the same annotation, but it seems like extra boilerplate for something that we can easily infer in clang. What other consumers are you concerned about? Repository: rC Clang https://reviews.llvm.org/D51189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51649: [Sema] Don't warn about omitting unavailable enum constants in a switch
erik.pilkington created this revision.
erik.pilkington added a reviewer: arphaman.
Herald added a subscriber: dexonsmith.
rdar://42717026
Thanks for taking a look!
Repository:
rC Clang
https://reviews.llvm.org/D51649
Files:
clang/lib/Sema/SemaStmt.cpp
clang/test/Sema/switch-availability.c
Index: clang/test/Sema/switch-availability.c
===
--- /dev/null
+++ clang/test/Sema/switch-availability.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -verify -Wswitch -triple x86_64-apple-macosx10.12 %s
+
+enum SwitchOne {
+ Unavail __attribute__((availability(macos, unavailable))),
+};
+
+void testSwitchOne(enum SwitchOne so) {
+ switch (so) {} // no warning
+}
+
+enum SwitchTwo {
+ Ed __attribute__((availability(macos, deprecated=10.12))),
+ Vim __attribute__((availability(macos, deprecated=10.13))),
+ Emacs,
+};
+
+void testSwitchTwo(enum SwitchTwo st) {
+ switch (st) {} // expected-warning{{enumeration values 'Vim' and 'Emacs' not
handled in switch}}
+}
+
+enum SwitchThree {
+ New __attribute__((availability(macos, introduced=1000))),
+};
+
+void testSwitchThree(enum SwitchThree st) {
+ switch (st) {} // expected-warning{{enumeration value 'New' not handled in
switch}}
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -1164,7 +1164,21 @@
SmallVector UnhandledNames;
- for (EI = EnumVals.begin(); EI != EIEnd; EI++){
+ for (EI = EnumVals.begin(); EI != EIEnd; EI++) {
+// Don't warn about omitted unavailable EnumConstantDecls.
+switch (EI->second->getAvailability()) {
+case AR_Deprecated:
+ // Omitting a deprecated constant is ok; it should never materialize.
+case AR_Unavailable:
+ continue;
+
+case AR_NotYetIntroduced:
+ // Partially available enum constants should be present. Note that we
+ // suppress -Wunguarded-availability diagnostics for such uses.
+case AR_Available:
+ break;
+}
+
// Drop unneeded case values
while (CI != CaseVals.end() && CI->first < EI->first)
CI++;
Index: clang/test/Sema/switch-availability.c
===
--- /dev/null
+++ clang/test/Sema/switch-availability.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -verify -Wswitch -triple x86_64-apple-macosx10.12 %s
+
+enum SwitchOne {
+ Unavail __attribute__((availability(macos, unavailable))),
+};
+
+void testSwitchOne(enum SwitchOne so) {
+ switch (so) {} // no warning
+}
+
+enum SwitchTwo {
+ Ed __attribute__((availability(macos, deprecated=10.12))),
+ Vim __attribute__((availability(macos, deprecated=10.13))),
+ Emacs,
+};
+
+void testSwitchTwo(enum SwitchTwo st) {
+ switch (st) {} // expected-warning{{enumeration values 'Vim' and 'Emacs' not handled in switch}}
+}
+
+enum SwitchThree {
+ New __attribute__((availability(macos, introduced=1000))),
+};
+
+void testSwitchThree(enum SwitchThree st) {
+ switch (st) {} // expected-warning{{enumeration value 'New' not handled in switch}}
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -1164,7 +1164,21 @@
SmallVector UnhandledNames;
- for (EI = EnumVals.begin(); EI != EIEnd; EI++){
+ for (EI = EnumVals.begin(); EI != EIEnd; EI++) {
+// Don't warn about omitted unavailable EnumConstantDecls.
+switch (EI->second->getAvailability()) {
+case AR_Deprecated:
+ // Omitting a deprecated constant is ok; it should never materialize.
+case AR_Unavailable:
+ continue;
+
+case AR_NotYetIntroduced:
+ // Partially available enum constants should be present. Note that we
+ // suppress -Wunguarded-availability diagnostics for such uses.
+case AR_Available:
+ break;
+}
+
// Drop unneeded case values
while (CI != CaseVals.end() && CI->first < EI->first)
CI++;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48896: [libcxx][c++17] P0083R5: Splicing Maps and Sets Part 2: merge
erik.pilkington updated this revision to Diff 164064.
erik.pilkington added a comment.
Ping! In the new patch:
- Uncomment __cpp_lib_node_extract, reverting r340544
- Rebase/Clean up the diff a bit
https://reviews.llvm.org/D48896
Files:
libcxx/include/__hash_table
libcxx/include/__node_handle
libcxx/include/__tree
libcxx/include/map
libcxx/include/set
libcxx/include/unordered_map
libcxx/include/unordered_set
libcxx/test/std/containers/associative/map/map.modifiers/merge.pass.cpp
libcxx/test/std/containers/associative/multimap/multimap.modifiers/merge.pass.cpp
libcxx/test/std/containers/associative/multiset/merge.pass.cpp
libcxx/test/std/containers/associative/set/merge.pass.cpp
libcxx/test/std/containers/unord/unord.map/unord.map.modifiers/merge.pass.cpp
libcxx/test/std/containers/unord/unord.multimap/unord.multimap.modifiers/merge.pass.cpp
libcxx/test/std/containers/unord/unord.multiset/merge.pass.cpp
libcxx/test/std/containers/unord/unord.set/merge.pass.cpp
Index: libcxx/test/std/containers/unord/unord.set/merge.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/containers/unord/unord.set/merge.pass.cpp
@@ -0,0 +1,135 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+//
+
+// class unordered_set
+
+// template
+// void merge(unordered_set& source);
+
+#include
+#include "test_macros.h"
+#include "Counter.h"
+
+template
+bool set_equal(const Set& set, Set other)
+{
+return set == other;
+}
+
+#ifndef TEST_HAS_NO_EXCEPTIONS
+template
+struct throw_hasher
+{
+bool& should_throw_;
+
+throw_hasher(bool& should_throw) : should_throw_(should_throw) {}
+
+typedef size_t result_type;
+typedef T argument_type;
+
+size_t operator()(const T& p) const
+{
+if (should_throw_)
+throw 0;
+return std::hash()(p);
+}
+};
+#endif
+
+int main()
+{
+{
+std::unordered_set src{1, 3, 5};
+std::unordered_set dst{2, 4, 5};
+dst.merge(src);
+assert(set_equal(src, {5}));
+assert(set_equal(dst, {1, 2, 3, 4, 5}));
+}
+
+#ifndef TEST_HAS_NO_EXCEPTIONS
+{
+bool do_throw = false;
+typedef std::unordered_set, throw_hasher>> set_type;
+set_type src({1, 3, 5}, 0, throw_hasher>(do_throw));
+set_type dst({2, 4, 5}, 0, throw_hasher>(do_throw));
+
+assert(Counter_base::gConstructed == 6);
+
+do_throw = true;
+try
+{
+dst.merge(src);
+}
+catch (int)
+{
+do_throw = false;
+}
+assert(!do_throw);
+assert(set_equal(src, set_type({1, 3, 5}, 0, throw_hasher>(do_throw;
+assert(set_equal(dst, set_type({2, 4, 5}, 0, throw_hasher>(do_throw;
+}
+#endif
+assert(Counter_base::gConstructed == 0);
+struct equal
+{
+equal() = default;
+
+bool operator()(const Counter& lhs, const Counter& rhs) const
+{
+return lhs == rhs;
+}
+};
+struct hasher
+{
+hasher() = default;
+typedef Counter argument_type;
+typedef size_t result_type;
+size_t operator()(const Counter& p) const { return std::hash>()(p); }
+};
+{
+typedef std::unordered_set, std::hash>, std::equal_to>> first_set_type;
+typedef std::unordered_set, hasher, equal> second_set_type;
+typedef std::unordered_multiset, hasher, equal> third_set_type;
+
+first_set_type first{1, 2, 3};
+second_set_type second{2, 3, 4};
+third_set_type third{1, 3};
+
+assert(Counter_base::gConstructed == 8);
+
+first.merge(second);
+first.merge(std::move(second));
+first.merge(third);
+first.merge(std::move(third));
+
+assert(set_equal(first, {1, 2, 3, 4}));
+assert(set_equal(second, {2, 3}));
+assert(set_equal(third, {1, 3}));
+
+assert(Counter_base::gConstructed == 8);
+}
+assert(Counter_base::gConstructed == 0);
+{
+std::unordered_set first;
+{
+std::unordered_set second;
+first.merge(second);
+first.merge(std::move(second));
+}
+{
+std::unordered_multiset second;
+first.merge(second);
+first.merge(std::move(second));
+}
+}
+}
Index: libcxx/test/std/containers/unord/unord.multiset/merge.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/containers/unord/unord.multiset/merge.pass.cpp
@@ -0,0 +1,135 @
[PATCH] D51697: [Sema] Clean up some __builtin_*_chk diagnostics
erik.pilkington created this revision.
erik.pilkington added reviewers: aaron.ballman, rsmith.
Herald added subscribers: kristina, dexonsmith.
Namely, print the likely macro name when it's used, and include the actual
computed sizes in the diagnostic message, which are sometimes not obvious.
rdar://43909200
Thanks for taking a look!
Repository:
rC Clang
https://reviews.llvm.org/D51697
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/builtin-object-size.c
clang/test/Sema/builtins.c
Index: clang/test/Sema/builtins.c
===
--- clang/test/Sema/builtins.c
+++ clang/test/Sema/builtins.c
@@ -221,30 +221,30 @@
// expected-note {{change size argument to be the size of the destination}}
__builtin___strlcpy_chk(buf, b, sizeof(b), __builtin_object_size(buf, 0)); // expected-warning {{size argument in '__builtin___strlcpy_chk' call appears to be size of the source; expected the size of the destination}} \
// expected-note {{change size argument to be the size of the destination}} \
-// expected-warning {{'__builtin___strlcpy_chk' will always overflow destination buffer}}
+// expected-warning {{'__builtin___strlcpy_chk' will always overflow; destination buffer has size 20, but size argument is 40}}
strlcat(buf, b, sizeof(b)); // expected-warning {{size argument in 'strlcat' call appears to be size of the source; expected the size of the destination}} \
// expected-note {{change size argument to be the size of the destination}}
__builtin___strlcat_chk(buf, b, sizeof(b), __builtin_object_size(buf, 0)); // expected-warning {{size argument in '__builtin___strlcat_chk' call appears to be size of the source; expected the size of the destination}} \
// expected-note {{change size argument to be the size of the destination}} \
- // expected-warning {{'__builtin___strlcat_chk' will always overflow destination buffer}}
+ // expected-warning {{'__builtin___strlcat_chk' will always overflow; destination buffer has size 20, but size argument is 40}}
}
// rdar://11076881
char * Test20(char *p, const char *in, unsigned n)
{
static char buf[10];
-__builtin___memcpy_chk (&buf[6], in, 5, __builtin_object_size (&buf[6], 0)); // expected-warning {{'__builtin___memcpy_chk' will always overflow destination buffer}}
+__builtin___memcpy_chk (&buf[6], in, 5, __builtin_object_size (&buf[6], 0)); // expected-warning {{'__builtin___memcpy_chk' will always overflow; destination buffer has size 4, but size argument is 5}}
__builtin___memcpy_chk (p, "abcde", n, __builtin_object_size (p, 0));
__builtin___memcpy_chk (&buf[5], "abcde", 5, __builtin_object_size (&buf[5], 0));
__builtin___memcpy_chk (&buf[5], "abcde", n, __builtin_object_size (&buf[5], 0));
-__builtin___memcpy_chk (&buf[6], "abcde", 5, __builtin_object_size (&buf[6], 0)); // expected-warning {{'__builtin___memcpy_chk' will always overflow destination buffer}}
+__builtin___memcpy_chk (&buf[6], "abcde", 5, __builtin_object_size (&buf[6], 0)); // expected-warning {{'__builtin___memcpy_chk' will always overflow; destination buffer has size 4, but size argument is 5}}
return buf;
}
@@ -276,3 +276,14 @@
(void)__builtin_signbitl(1.0f);
(void)__builtin_signbitl(1.0L);
}
+
+// rdar://43909200
+#define memcpy(x,y,z) __builtin___memcpy_chk(x,y,z, __builtin_object_size(x,0))
+#define my_memcpy(x,y,z) __builtin___memcpy_chk(x,y,z, __builtin_object_size(x,0))
+
+void test23() {
+ char src[1024];
+ char buf[10];
+ memcpy(buf, src, 11); // expected-warning{{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
+ my_memcpy(buf, src, 11); // expected-warning{{'__builtin___memcpy_chk' will always overflow; destination buffer has size 10, but size argument is 11}}
+}
Index: clang/test/Sema/builtin-object-size.c
===
--- clang/test/Sema/builtin-object-size.c
+++ clang/test/Sema/builtin-object-size.c
@@ -23,7 +23,7 @@
// rdar://6252231 - cannot call vsnprintf with va_list on x86_64
void f4(const char *fmt, ...) {
__builtin_va_list args;
- __builtin___vsnprintf_chk (0, 42, 0, 11, fmt, args); // expected-warning {{'__builtin___vsnprintf_chk' will always overflow destination buffer}}
+ __builtin___vsnprintf_chk (0, 42, 0, 11, fmt, args); // expected-warning {{'__builtin___vsnprintf_chk' will always overflow; destination buffer has size 11, but size argument is 42}}
}
// rdar://18334276
@@ -50,7 +50,7 @@
char b[5];
char buf[10];
__builtin___memccp
[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init
erik.pilkington added a comment.
In https://reviews.llvm.org/D51189#1211910, @steven_wu wrote:
> I feel like this is a much tricky situation than just new and init. Following
> example is the same situation.
>
> __attribute__((objc_root_class))
> @interface NSObject
> - (void) foo;
> - (void) bar;
> @end
>
> @implementation NSObject
> - (void) foo {}
> - (void) bar { [self foo]; }
> @end
>
> @interface MyObject : NSObject
> - (void) foo __attribute__((unavailable));
> @end
>
> void test(MyObject *obj) {
> [obj bar];
> }
>
>
> We can do something about [NSObject new] because we know it's implementation
> but we have to live with more general cases.
I agree that the general case is impossible to properly diagnose, but I think
its totally reasonable for us to special case this pattern with NSObject.
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6951
+ std::string *Message,
+ ObjCInterfaceDecl *ClassMessageReceiver) {
AvailabilityResult Result = D->getAvailability(Message);
arphaman wrote:
> Please be consistent with the name, you are using `ClassMessageReceiver`,
> `ClassReceiver` and `Receiver` in different arguments in this patch.
Sure, sorry. I canonicalized on ClassReceiver.
https://reviews.llvm.org/D51189
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init
erik.pilkington updated this revision to Diff 164316.
erik.pilkington marked 4 inline comments as done.
erik.pilkington added a comment.
Address @arphaman's review comments.
https://reviews.llvm.org/D51189
Files:
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/DeclObjC.h
clang/include/clang/AST/NSAPI.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/DeclObjC.cpp
clang/lib/AST/NSAPI.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprObjC.cpp
clang/test/SemaObjC/infer-availability-from-init.m
Index: clang/test/SemaObjC/infer-availability-from-init.m
===
--- /dev/null
+++ clang/test/SemaObjC/infer-availability-from-init.m
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx-10.9 -Wunguarded-availability -fblocks -fsyntax-only -verify %s
+
+__attribute__((objc_root_class))
+@interface NSObject
++(instancetype)new;
+-(instancetype)init;
+@end
+
+@interface MyObject : NSObject
+-(instancetype)init __attribute__((unavailable)); // expected-note{{'init' has been explicitly marked unavailable here}}
+@end
+
+void usemyobject() {
+ [MyObject new]; // expected-error{{'new' is unavailable}}
+}
+
+@interface MyOtherObject : NSObject
++(instancetype)init __attribute__((unavailable));
++(instancetype)new;
+@end
+
+void usemyotherobject() {
+ [MyOtherObject new]; // no error; new is overrideen.
+}
+
+@interface NotGoodOverride : NSObject
++(instancetype)init __attribute__((unavailable));
+-(instancetype)new;
++(instancetype)new: (int)x;
+@end
+
+void usenotgoodoverride() {
+ [NotGoodOverride new]; // no error
+}
+
+@interface NotNSObject
++(instancetype)new;
+-(instancetype)init;
+@end
+
+@interface NotMyObject : NotNSObject
+-(instancetype)init __attribute__((unavailable));
+@end
+
+void usenotmyobject() {
+ [NotMyObject new]; // no error; this isn't NSObject
+}
+
+@interface FromSelf : NSObject
+-(instancetype)init __attribute__((unavailable)); // expected-note {{'init' has been explicitly marked unavailable here}}
++(FromSelf*)another_one;
+@end
+
+@implementation FromSelf
++(FromSelf*)another_one {
+ [self new]; // expected-error{{'new' is unavailable}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2471,7 +2471,8 @@
if (!Method)
Method = Class->lookupPrivateClassMethod(Sel);
-if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs,
+nullptr, false, false, Class))
return ExprError();
}
@@ -2784,14 +2785,19 @@
} else {
if (ObjCMethodDecl *CurMeth = getCurMethodDecl()) {
if (ObjCInterfaceDecl *ClassDecl = CurMeth->getClassInterface()) {
+// FIXME: Is this correct? Why are we assuming that a message to
+// Class will call a method in the current interface?
+
// First check the public methods in the class interface.
Method = ClassDecl->lookupClassMethod(Sel);
if (!Method)
Method = ClassDecl->lookupPrivateClassMethod(Sel);
+
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs, nullptr,
+false, false, ClassDecl))
+ return ExprError();
}
- if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
-return ExprError();
}
if (!Method) {
// If not messaging 'self', look for any factory method named 'Sel'.
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -206,7 +206,8 @@
bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef Locs,
const ObjCInterfaceDecl *UnknownObjCClass,
bool ObjCPropertyAccess,
- bool AvoidPartialAvailabilityChecks) {
+ bool AvoidPartialAvailabilityChecks,
+ ObjCInterfaceDecl *ClassReceiver) {
SourceLocation Loc = Locs.front();
if (getLangOpts().CPlusPlus && isa(D)) {
// If there were any diagnostics suppressed by template argument deduction,
@@ -292,7 +293,7 @@
}
DiagnoseAvailabilityOfDecl(D, Locs, UnknownObjCClass, ObjCPropertyAccess,
- AvoidPartialAvailabilityChecks);
+ AvoidPartialAvailabilityChecks, ClassReceiver);
DiagnoseUnusedOfDecl(*this, D, Loc);
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6959,8 +6959,12 @@
/// \param D The declaration to check.
/// \param Messag
[PATCH] D48896: [libcxx][c++17] P0083R5: Splicing Maps and Sets Part 2: merge
erik.pilkington added a comment. Ping! https://reviews.llvm.org/D48896 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51997: [clang] Make sure attributes on member classes are applied properly
erik.pilkington added inline comments.
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1267-1269
+ // Instantiate the attributes on both the template declaration and the
associated record declaration.
+ SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, Inst, LateAttrs,
StartingScope);
+ SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, RecordInst,
LateAttrs, StartingScope);
nit: 80 chars.
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1268
+ // Instantiate the attributes on both the template declaration and the
associated record declaration.
+ SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, Inst, LateAttrs,
StartingScope);
+ SemaRef.InstantiateAttrsForDecl(TemplateArgs, Pattern, RecordInst,
LateAttrs, StartingScope);
Why are you instantiating the attributes onto the ClassTemplateDecl? At the
very least it seems wrong to instantiate attributes from the pattern to the
ClassTemplateDecl.
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1498
+ SemaRef.InstantiateAttrsForDecl(TemplateArgs, D, Record, LateAttrs,
StartingScope);
+
This line looks too long too.
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:3247
InstPartialSpec->setTypeAsWritten(WrittenTy);
// Check the completed partial specialization.
ldionne wrote:
> I tried adding
>
> ```
> SemaRef.InstantiateAttrsForDecl(TemplateArgs,
> ClassTemplate->getTemplatedDecl(), InstPartialSpec, LateAttrs, StartingScope);
> ```
>
> here, but just like for explicit specializations, that doesn't instantiate
> the attributes on the `CXXRecordDecl` used to determine mangling.
>
You mean `SemaRef.InstantiateAttrsForDecl(TemplateArgs, PartialSpec,
InstPartialSpec, LateAttrs, StartingScope);`?
Comment at: clang/test/SemaCXX/PR38913.cpp:19
+};
+C::X* c() { return 0; } // CHECK-DAG: @_Z1cB4CTAGv
+
ldionne wrote:
> This test is failing in the current iteration of the patch.
I think the problem here is that we don't instantiate X because it's
behind the pointer, so we're just considering the primary template. Looks like
we do add the attribute (with the fix above) here:
```
template struct C {
template struct X { };
template struct __attribute__((abi_tag("CTAG"))) X { };
};
C::X c() { return 0; }
```
But we don't get the right mangling, for some reason. Note that this problem is
present even in the non-member case.
I don't think attributes on specializations have particularly good support
anyways, so I wouldn't really lose any sleep if we "left this to a follow-up".
Maybe @rsmith has some thoughts here.
Repository:
rC Clang
https://reviews.llvm.org/D51997
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51997: [clang] Make sure attributes on member classes are applied properly
erik.pilkington added a comment. Looks good to me! @rsmith should probably have the final word though. Thanks for fixing this. Repository: rC Clang https://reviews.llvm.org/D51997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49085: [Sema] Emit a diagnostic for an invalid dependent function template specialization
erik.pilkington updated this revision to Diff 156317.
erik.pilkington added a comment.
Improve the diagnostics. Thanks!
https://reviews.llvm.org/D49085
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaTemplate.cpp
clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
Index: clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
+++ clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
@@ -359,3 +359,20 @@
template void f2(X *);
template void f2(X *); // expected-note{{in instantiation of
function template specialization 'PR10913::f2' requested here}}
}
+
+namespace test16 {
+template struct foo {}; // expected-note{{candidate template
ignored: template is not a function template}}
+template class A {
+ friend void foo(); // expected-error{{no candidate function template was
found for dependent friend function template specialization}}
+};
+}
+
+namespace test17 {
+namespace ns {
+template void foo() {} // expected-note{{candidate template ignored:
is not a member of the enclosing namespace}}
+}
+using ns::foo;
+template struct A {
+ friend void foo() {} // expected-error{{no candidate function template
was found for dependent friend function template specialization}}
+};
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -8008,17 +8008,34 @@
// the correct context.
DeclContext *FDLookupContext = FD->getDeclContext()->getRedeclContext();
LookupResult::Filter F = Previous.makeFilter();
+ enum DiscardReason { NotAFunctionTemplate, NotAMemberOfEnclosing };
+ SmallVector, 8> DiscardedCandidates;
while (F.hasNext()) {
NamedDecl *D = F.next()->getUnderlyingDecl();
-if (!isa(D) ||
-!FDLookupContext->InEnclosingNamespaceSetOf(
- D->getDeclContext()->getRedeclContext()))
+if (!isa(D)) {
F.erase();
+ DiscardedCandidates.push_back(std::make_pair(NotAFunctionTemplate, D));
+ continue;
+}
+
+if (!FDLookupContext->InEnclosingNamespaceSetOf(
+D->getDeclContext()->getRedeclContext())) {
+ F.erase();
+ DiscardedCandidates.push_back(std::make_pair(NotAMemberOfEnclosing, D));
+ continue;
+}
}
F.done();
- // Should this be diagnosed here?
- if (Previous.empty()) return true;
+ if (Previous.empty()) {
+Diag(FD->getLocation(),
+ diag::err_dependent_function_template_spec_no_match);
+for (auto &P : DiscardedCandidates)
+ Diag(P.second->getLocation(),
+ diag::note_dependent_function_template_spec_discard_reason)
+ << P.first;
+return true;
+ }
FD->setDependentTemplateSpecialization(Context, Previous.asUnresolvedSet(),
ExplicitTemplateArgs);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4103,6 +4103,12 @@
def err_explicit_specialization_inconsistent_storage_class : Error<
"explicit specialization has extraneous, inconsistent storage class "
"'%select{none|extern|static|__private_extern__|auto|register}0'">;
+def err_dependent_function_template_spec_no_match : Error<
+ "no candidate function template was found for dependent"
+ " friend function template specialization">;
+def note_dependent_function_template_spec_discard_reason : Note<
+ "candidate template ignored: %select{template is not a function template"
+ "|is not a member of the enclosing namespace}0">;
// C++ class template specializations and out-of-line definitions
def err_template_spec_needs_header : Error<
Index: clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
+++ clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
@@ -359,3 +359,20 @@
template void f2(X *);
template void f2(X *); // expected-note{{in instantiation of function template specialization 'PR10913::f2' requested here}}
}
+
+namespace test16 {
+template struct foo {}; // expected-note{{candidate template ignored: template is not a function template}}
+template class A {
+ friend void foo(); // expected-error{{no candidate function template was found for dependent friend function template specialization}}
+};
+}
+
+namespace test17 {
+namespace ns {
+template void foo() {} // expected-note{{candidate template ignored: is not a member of the enclosing namespace}}
+}
+using ns::foo;
+template struct A {
+ friend void foo() {} // expected-error{{no candidate function template was found for dependent friend function template specialization}}
+};
+}
Index: clang/lib/Sem
[PATCH] D49085: [Sema] Emit a diagnostic for an invalid dependent function template specialization
erik.pilkington updated this revision to Diff 156338.
erik.pilkington added a comment.
Improve the diagnostics further.
https://reviews.llvm.org/D49085
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaTemplate.cpp
clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
Index: clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
+++ clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
@@ -359,3 +359,31 @@
template void f2(X *);
template void f2(X *); // expected-note{{in instantiation of
function template specialization 'PR10913::f2' requested here}}
}
+
+namespace test16 {
+template struct foo {}; // expected-note{{candidate ignored: not a
function template}}
+template class A {
+ friend void foo(); // expected-error{{no candidate function template was
found for dependent friend function template specialization}}
+};
+}
+
+namespace test17 {
+namespace ns {
+template void foo() {} // expected-note{{candidate ignored: not a
member of the enclosing namespace; did you mean to explicitly qualify the
specialization?}}
+}
+using ns::foo;
+template struct A {
+ friend void foo() {} // expected-error{{no candidate function template
was found for dependent friend function template specialization}}
+};
+}
+
+namespace test18 {
+namespace ns1 { template struct foo {}; } //
expected-note{{candidate ignored: not a function template}}
+namespace ns2 { void foo() {} } // expected-note{{candidate ignored: not a
function template}}
+using ns1::foo;
+using ns2::foo;
+
+template class A {
+ friend void foo() {} // expected-error{{no candidate function template
was found for dependent friend function template specialization}}
+};
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -8008,17 +8008,34 @@
// the correct context.
DeclContext *FDLookupContext = FD->getDeclContext()->getRedeclContext();
LookupResult::Filter F = Previous.makeFilter();
+ enum DiscardReason { NotAFunctionTemplate, NotAMemberOfEnclosing };
+ SmallVector, 8> DiscardedCandidates;
while (F.hasNext()) {
NamedDecl *D = F.next()->getUnderlyingDecl();
-if (!isa(D) ||
-!FDLookupContext->InEnclosingNamespaceSetOf(
- D->getDeclContext()->getRedeclContext()))
+if (!isa(D)) {
F.erase();
+ DiscardedCandidates.push_back(std::make_pair(NotAFunctionTemplate, D));
+ continue;
+}
+
+if (!FDLookupContext->InEnclosingNamespaceSetOf(
+D->getDeclContext()->getRedeclContext())) {
+ F.erase();
+ DiscardedCandidates.push_back(std::make_pair(NotAMemberOfEnclosing, D));
+ continue;
+}
}
F.done();
- // Should this be diagnosed here?
- if (Previous.empty()) return true;
+ if (Previous.empty()) {
+Diag(FD->getLocation(),
+ diag::err_dependent_function_template_spec_no_match);
+for (auto &P : DiscardedCandidates)
+ Diag(P.second->getLocation(),
+ diag::note_dependent_function_template_spec_discard_reason)
+ << P.first;
+return true;
+ }
FD->setDependentTemplateSpecialization(Context, Previous.asUnresolvedSet(),
ExplicitTemplateArgs);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4103,6 +4103,12 @@
def err_explicit_specialization_inconsistent_storage_class : Error<
"explicit specialization has extraneous, inconsistent storage class "
"'%select{none|extern|static|__private_extern__|auto|register}0'">;
+def err_dependent_function_template_spec_no_match : Error<
+ "no candidate function template was found for dependent"
+ " friend function template specialization">;
+def note_dependent_function_template_spec_discard_reason : Note<
+ "candidate ignored: %select{not a function template"
+ "|not a member of the enclosing namespace; did you mean to explicitly
qualify the specialization?}0">;
// C++ class template specializations and out-of-line definitions
def err_template_spec_needs_header : Error<
Index: clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
+++ clang/test/CXX/temp/temp.decls/temp.friend/p1.cpp
@@ -359,3 +359,31 @@
template void f2(X *);
template void f2(X *); // expected-note{{in instantiation of function template specialization 'PR10913::f2' requested here}}
}
+
+namespace test16 {
+template struct foo {}; // expected-note{{candidate ignored: not a function template}}
+template class A {
+ friend void foo(); // expected-error
[PATCH] D49085: [Sema] Emit a diagnostic for an invalid dependent function template specialization
erik.pilkington added inline comments.
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4111
+ "candidate template ignored: %select{template is not a function template"
+ "|is not a member of the enclosing namespace}0">;
rjmccall wrote:
> Your first explanation has a subject, but the second doesn't. And I think it
> would be nice to suggest adding explicit scope qualification in the second
> case.
>
> I assume non-templates have previously been filtered out?
Oh, no, good point. A non-template can still get into here:
```
namespace ns1 { template struct foo {}; }
namespace ns2 { int foo() {} } // bad diag: template is not a function template
using ns1::foo;
using ns2::foo;
template class A {
friend void foo() {}
};
```
I added this to the testcases.
https://reviews.llvm.org/D49085
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49511: [Sema/Attribute] Check for noderef attribute
erik.pilkington added a reviewer: aaron.ballman.
erik.pilkington added a comment.
Thanks for working on this! CCing Aaron, who is code owner for attributes.
Comment at: include/clang/Basic/AttrDocs.td:3355
+ let Content = [{
+The ``noderef`` attribute allows for showing a warning whenever a pointer
marked with
+this attribute is dereferenced. This is ideally used with pointers that point
to special
'allows for showing' sounds a bit strange... maybe: "The ``noderef`` attribute
causes clang to diagnose dereferences of annotated pointer types" or something?
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9372
+def warn_dereference_of_noderef_type : Warning<
+ "dereference of noderef expression">, InGroup;
} // end of sema component.
I think -Wignored-attributes is a group for cases where clang ignores an
attribute, you should probably add you own diagnostic group, say
-Wdereferenced-noderef or something for this diagnostic. You should add this to
DiagnosticGroups.td.
Repository:
rC Clang
https://reviews.llvm.org/D49511
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets
erik.pilkington added a comment. Ping! https://reviews.llvm.org/D46845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49688: [Sema] Fix a crash when a BlockExpr appears in a default-member-initializer of a class template
erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, rsmith.
Herald added a subscriber: dexonsmith.
Previously, clang would crash here:
template struct S {
int (^p)() = ^{ return 0; };
};
S x;
The problem was that InstantiateClass() was iterating over all the decls(),
which included the BlockDecl. The template instantiator doesn't know how to
instantiate BlockDecls though, it knows about BlockExprs. This is typically
fine because BlockExprs must be the parent of the BlockDecl. To accommodate
this, delay instantiating the BlockDecl until we're instantiating the
default-member-initializer.
Thanks for taking a look!
Erik
Repository:
rC Clang
https://reviews.llvm.org/D49688
Files:
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/SemaCXX/instantiate-blocks.cpp
Index: clang/test/SemaCXX/instantiate-blocks.cpp
===
--- clang/test/SemaCXX/instantiate-blocks.cpp
+++ clang/test/SemaCXX/instantiate-blocks.cpp
@@ -30,3 +30,12 @@
noret((float)0.0, double(0.0)); // expected-note {{in instantiation of
function template specialization 'noret' requested here}}
}
+namespace rdar41200624 {
+template
+struct S {
+ int (^p)() = ^{ return 0; };
+ T (^t)() = ^{ return T{}; };
+ T s = ^{ return T{}; }();
+};
+S x;
+}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2083,6 +2083,11 @@
if (Member->getDeclContext() != Pattern)
continue;
+// BlockDecls can appear in a default-member-initializer. They must be the
+// child of a BlockExpr, so we only know how to instantiate them from
there.
+if (isa(Member))
+ continue;
+
if (Member->isInvalidDecl()) {
Instantiation->setInvalidDecl();
continue;
Index: clang/test/SemaCXX/instantiate-blocks.cpp
===
--- clang/test/SemaCXX/instantiate-blocks.cpp
+++ clang/test/SemaCXX/instantiate-blocks.cpp
@@ -30,3 +30,12 @@
noret((float)0.0, double(0.0)); // expected-note {{in instantiation of function template specialization 'noret' requested here}}
}
+namespace rdar41200624 {
+template
+struct S {
+ int (^p)() = ^{ return 0; };
+ T (^t)() = ^{ return T{}; };
+ T s = ^{ return T{}; }();
+};
+S x;
+}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2083,6 +2083,11 @@
if (Member->getDeclContext() != Pattern)
continue;
+// BlockDecls can appear in a default-member-initializer. They must be the
+// child of a BlockExpr, so we only know how to instantiate them from there.
+if (isa(Member))
+ continue;
+
if (Member->isInvalidDecl()) {
Instantiation->setInvalidDecl();
continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49688: [Sema] Fix a crash when a BlockExpr appears in a default-member-initializer of a class template
erik.pilkington added a comment. (Forgot to add rdar://41200624) Repository: rC Clang https://reviews.llvm.org/D49688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49688: [Sema] Fix a crash when a BlockExpr appears in a default-member-initializer of a class template
erik.pilkington added a comment. In https://reviews.llvm.org/D49688#1172269, @rjmccall wrote: > I honestly don't know why the `BlockDecl` is in the members list in the first > place; that seems wrong, for the same reason that we wouldn't (I assume?) > consider a lambda's implicit record to be a member. Looks like we do actually treat a lambda's implicit record as a member. This loop is only iterating over decls that are in the pattern decl's context, and I think it makes sense that the BlockDecl is there, right? Repository: rC Clang https://reviews.llvm.org/D49688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48896: [libcxx][c++17] P0083R5: Splicing Maps and Sets Part 2: merge
erik.pilkington updated this revision to Diff 156912.
erik.pilkington added a comment.
In this new patch:
- Rebase on top of changes in https://reviews.llvm.org/D46845
- Move in some forward decls that I accidentally left in
https://reviews.llvm.org/D46845
- Add some missing visibility attributes
- Add `merge` to header summaries
https://reviews.llvm.org/D48896
Files:
libcxx/include/__hash_table
libcxx/include/__tree
libcxx/include/map
libcxx/include/set
libcxx/include/unordered_map
libcxx/include/unordered_set
libcxx/test/std/containers/associative/map/map.modifiers/merge.pass.cpp
libcxx/test/std/containers/associative/multimap/multimap.modifiers/merge.pass.cpp
libcxx/test/std/containers/associative/multiset/merge.pass.cpp
libcxx/test/std/containers/associative/set/merge.pass.cpp
libcxx/test/std/containers/unord/unord.map/unord.map.modifiers/merge.pass.cpp
libcxx/test/std/containers/unord/unord.multimap/unord.multimap.modifiers/merge.pass.cpp
libcxx/test/std/containers/unord/unord.multiset/merge.pass.cpp
libcxx/test/std/containers/unord/unord.set/merge.pass.cpp
Index: libcxx/test/std/containers/unord/unord.set/merge.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/containers/unord/unord.set/merge.pass.cpp
@@ -0,0 +1,135 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+//
+
+// class unordered_set
+
+// template
+// void merge(unordered_set& source);
+
+#include
+#include "test_macros.h"
+#include "Counter.h"
+
+template
+bool set_equal(const Set& set, Set other)
+{
+return set == other;
+}
+
+#ifndef TEST_HAS_NO_EXCEPTIONS
+template
+struct throw_hasher
+{
+bool& should_throw_;
+
+throw_hasher(bool& should_throw) : should_throw_(should_throw) {}
+
+typedef size_t result_type;
+typedef T argument_type;
+
+size_t operator()(const T& p) const
+{
+if (should_throw_)
+throw 0;
+return std::hash()(p);
+}
+};
+#endif
+
+int main()
+{
+{
+std::unordered_set src{1, 3, 5};
+std::unordered_set dst{2, 4, 5};
+dst.merge(src);
+assert(set_equal(src, {5}));
+assert(set_equal(dst, {1, 2, 3, 4, 5}));
+}
+
+#ifndef TEST_HAS_NO_EXCEPTIONS
+{
+bool do_throw = false;
+typedef std::unordered_set, throw_hasher>> set_type;
+set_type src({1, 3, 5}, 0, throw_hasher>(do_throw));
+set_type dst({2, 4, 5}, 0, throw_hasher>(do_throw));
+
+assert(Counter_base::gConstructed == 6);
+
+do_throw = true;
+try
+{
+dst.merge(src);
+}
+catch (int)
+{
+do_throw = false;
+}
+assert(!do_throw);
+assert(set_equal(src, set_type({1, 3, 5}, 0, throw_hasher>(do_throw;
+assert(set_equal(dst, set_type({2, 4, 5}, 0, throw_hasher>(do_throw;
+}
+#endif
+assert(Counter_base::gConstructed == 0);
+struct equal
+{
+equal() = default;
+
+bool operator()(const Counter& lhs, const Counter& rhs) const
+{
+return lhs == rhs;
+}
+};
+struct hasher
+{
+hasher() = default;
+typedef Counter argument_type;
+typedef size_t result_type;
+size_t operator()(const Counter& p) const { return std::hash>()(p); }
+};
+{
+typedef std::unordered_set, std::hash>, std::equal_to>> first_set_type;
+typedef std::unordered_set, hasher, equal> second_set_type;
+typedef std::unordered_multiset, hasher, equal> third_set_type;
+
+first_set_type first{1, 2, 3};
+second_set_type second{2, 3, 4};
+third_set_type third{1, 3};
+
+assert(Counter_base::gConstructed == 8);
+
+first.merge(second);
+first.merge(std::move(second));
+first.merge(third);
+first.merge(std::move(third));
+
+assert(set_equal(first, {1, 2, 3, 4}));
+assert(set_equal(second, {2, 3}));
+assert(set_equal(third, {1, 3}));
+
+assert(Counter_base::gConstructed == 8);
+}
+assert(Counter_base::gConstructed == 0);
+{
+std::unordered_set first;
+{
+std::unordered_set second;
+first.merge(second);
+first.merge(std::move(second));
+}
+{
+std::unordered_multiset second;
+first.merge(second);
+first.merge(std::move(second));
+}
+}
+}
Index: libcxx/test/std/containers/unord/unord.multiset/merge.pass.cpp
=
[PATCH] D49439: [Sema] Fix a crash while converting constructors to deduction guides
erik.pilkington updated this revision to Diff 157309.
erik.pilkington added a comment.
Implement @rsmith's second suggestion. Thanks!
https://reviews.llvm.org/D49439
Files:
clang/lib/Sema/SemaTemplate.cpp
clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -348,6 +348,22 @@
};
}
+namespace rdar41330135 {
+template struct A {};
+template
+struct S {
+ template
+ S(T a, U t, A);
+};
+template struct D {
+ D(T t, A);
+};
+int f() {
+ S s(0, 0, A());
+ D d(0, A());
+}
+}
+
#else
// expected-no-diagnostics
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1659,6 +1659,23 @@
}
namespace {
+/// Tree transform to "extract" a transformed type from a class template's
+/// constructor to a deduction guide.
+class ExtractTypeForDeductionGuide
+ : public TreeTransform {
+public:
+ typedef TreeTransform Base;
+ ExtractTypeForDeductionGuide(Sema &SemaRef) : Base(SemaRef) {}
+
+ TypeSourceInfo *transform(TypeSourceInfo *TSI) { return TransformType(TSI); }
+
+ QualType TransformTypedefType(TypeLocBuilder &TLB, TypedefTypeLoc TL) {
+return TransformType(
+TLB,
+TL.getTypedefNameDecl()->getTypeSourceInfo()->getTypeLoc());
+ }
+};
+
/// Transform to convert portions of a constructor declaration into the
/// corresponding deduction guide, per C++1z [over.match.class.deduct]p1.
struct ConvertConstructorToDeductionGuideTransform {
@@ -1880,9 +1897,7 @@
MultiLevelTemplateArgumentList &Args) {
TypeSourceInfo *OldDI = OldParam->getTypeSourceInfo();
TypeSourceInfo *NewDI;
-if (!Args.getNumLevels())
- NewDI = OldDI;
-else if (auto PackTL = OldDI->getTypeLoc().getAs()) {
+if (auto PackTL = OldDI->getTypeLoc().getAs()) {
// Expand out the one and only element in each inner pack.
Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(SemaRef, 0);
NewDI =
@@ -1898,23 +1913,17 @@
if (!NewDI)
return nullptr;
-// Canonicalize the type. This (for instance) replaces references to
-// typedef members of the current instantiations with the definitions of
-// those typedefs, avoiding triggering instantiation of the deduced type
-// during deduction.
-// FIXME: It would be preferable to retain type sugar and source
-// information here (and handle this in substitution instead).
-NewDI = SemaRef.Context.getTrivialTypeSourceInfo(
-SemaRef.Context.getCanonicalType(NewDI->getType()),
-OldParam->getLocation());
+// Extract the type. This (for instance) replaces references to typedef
+// members of the current instantiations with the definitions of those
+// typedefs, avoiding triggering instantiation of the deduced type during
+// deduction.
+NewDI = ExtractTypeForDeductionGuide(SemaRef).transform(NewDI);
// Resolving a wording defect, we also inherit default arguments from the
// constructor.
ExprResult NewDefArg;
if (OldParam->hasDefaultArg()) {
- NewDefArg = Args.getNumLevels()
- ? SemaRef.SubstExpr(OldParam->getDefaultArg(), Args)
- : OldParam->getDefaultArg();
+ NewDefArg = SemaRef.SubstExpr(OldParam->getDefaultArg(), Args);
if (NewDefArg.isInvalid())
return nullptr;
}
@@ -1929,6 +1938,7 @@
NewDefArg.get());
NewParam->setScopeInfo(OldParam->getFunctionScopeDepth(),
OldParam->getFunctionScopeIndex());
+SemaRef.CurrentInstantiationScope->InstantiatedLocal(OldParam, NewParam);
return NewParam;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49439: [Sema] Fix a crash while converting constructors to deduction guides
erik.pilkington added inline comments.
Comment at: clang/lib/Sema/SemaTemplate.cpp:1899-1907
// Canonicalize the type. This (for instance) replaces references to
// typedef members of the current instantiations with the definitions of
// those typedefs, avoiding triggering instantiation of the deduced type
// during deduction.
// FIXME: It would be preferable to retain type sugar and source
// information here (and handle this in substitution instead).
NewDI = SemaRef.Context.getTrivialTypeSourceInfo(
rsmith wrote:
> This is really the problem: we shouldn't be doing a full canonicalization
> step here. I expect that even with your patch applied we'll still see
> problems for cases like:
> ```
> template struct X {};
> template struct A {
> A(T t, X);
> };
> A a(0, {});
> template struct B {
> B(U u, X);
> };
> B b(0, {});
> ```
> ... because we'll canonicalize the second parameter of `B`'s deduction guide
> to have type `X` (where that's the `t` from `A`'s deduction guide).
>
> So I think we should look at fixing the FIXME here properly. There seem to be
> at least two viable options:
>
> 1) don't canonicalize the type; instead, extend template instantiation to be
> able to cope with one template referring to members of another template
> directly, without instantiating the class containing those members, or
> 2) add a custom `TreeTransform` to do the canonicalization we want to do
> here, and to avoid the canonicalization we don't want to do
>
> Both of these seem pretty tricky to get right, though, which is why we
> currently use the canonicalization hack :(
Yep, that still crashes :/
I started to implement 2 in the new patch. This implementation just unwraps
typedefs into the deduction guide, but that is already enough to pass libcxx's
test suite. This doesn't handle everything that we could, such as expression in
a decltype [1]. This is fine for now though, because the canonicalization hack
doesn't either. In fact, I couldn't find any cases where this patch fails but
the canonicalization succeeds. I'm inclined to fix the crash now, address any
extra cases in a follow-up if we actually want to support them. Does this seem
reasonable to you?
It also seems we're reading pretty far between the lines of the standard here,
do you think a DR should be filed?
[1]: I think we could support this is we wanted by stubbing out DeclRefExprs to
members for a new (or existing?) opaque reference AST node that acted like a
declval() analog. This would allow us to do sfinae using the member's type
without relying on it's context.
https://reviews.llvm.org/D49439
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49766: Fix a crash when an error occurs in Template and the initializer is a nullptr for C++17
erik.pilkington added a comment.
Hi Balaji,
I reduced your testcase a bit more, this looks like the can be a crash on
valid. Can you use the more minimal version in the testcase?
template
struct e {
e(a) {}
};
e c(0);
Also: you should add cfe-commits as a subscriber when creating new phab
revisions. IIRC there was some issue with adding cfe-commits after the fact.
Comment at: include/clang/AST/TemplateBase.h:469
+
+/* When an error occurs in a template value that is defaulted to
+ nullptr then the nullptr is left as-is and this function will
Please use `//` comments in C++ files!
I think this comment is pretty superfluous though. If a future reader came
across this then they would probably not care too much about the details of
this bug.
Comment at: include/clang/AST/TemplateBase.h:474-475
+ and proceeds through. */
+assert(Argument.getKind() == TemplateArgument::NullPtr ||
+ Argument.getKind() == TemplateArgument::Expression);
}
I think we should let this constructor work with any non type template
argument. If you agree, could you add the extra cases? (Should just be
TemplateArgument::Declaration and TemplateArgument::Integral)
Repository:
rC Clang
https://reviews.llvm.org/D49766
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49868: [Sema] Fix a crash by completing a type before using it
erik.pilkington created this revision.
erik.pilkington added reviewers: rsmith, rjmccall.
Herald added a subscriber: dexonsmith.
Only apply this exception on a type that we're able to check.
Thanks!
Erik
Repository:
rC Clang
https://reviews.llvm.org/D49868
Files:
clang/lib/Sema/SemaInit.cpp
clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -348,6 +348,20 @@
};
}
+namespace rdar41903969 {
+template struct A {};
+template struct B;
+template struct C {
+ C(A&);
+ C(B&);
+};
+
+void foo(A &a, B &b) {
+ (void)C{b};
+ (void)C{a};
+}
+}
+
#else
// expected-no-diagnostics
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -9130,6 +9130,7 @@
Expr *E = ListInit->getInit(0);
auto *RD = E->getType()->getAsCXXRecordDecl();
if (!isa(E) && RD &&
+ isCompleteType(Kind.getLocation(), E->getType()) &&
isOrIsDerivedFromSpecializationOf(RD, Template))
TryListConstructors = false;
}
Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -348,6 +348,20 @@
};
}
+namespace rdar41903969 {
+template struct A {};
+template struct B;
+template struct C {
+ C(A&);
+ C(B&);
+};
+
+void foo(A &a, B &b) {
+ (void)C{b};
+ (void)C{a};
+}
+}
+
#else
// expected-no-diagnostics
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -9130,6 +9130,7 @@
Expr *E = ListInit->getInit(0);
auto *RD = E->getType()->getAsCXXRecordDecl();
if (!isa(E) && RD &&
+ isCompleteType(Kind.getLocation(), E->getType()) &&
isOrIsDerivedFromSpecializationOf(RD, Template))
TryListConstructors = false;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49868: [Sema] Fix a crash by completing a type before using it
erik.pilkington updated this revision to Diff 157575.
erik.pilkington added a comment.
Add the testcase @rjmccall requested.
https://reviews.llvm.org/D49868
Files:
clang/lib/Sema/SemaInit.cpp
clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -348,6 +348,31 @@
};
}
+namespace rdar41903969 {
+template struct A {};
+template struct B;
+template struct C {
+ C(A&);
+ C(B&);
+};
+
+void foo(A &a, B &b) {
+ (void)C{b};
+ (void)C{a};
+}
+
+template struct X {
+ X(std::initializer_list) = delete;
+ X(const X&);
+};
+
+template struct D : X {};
+
+void bar(D& d) {
+ (void)X{d};
+}
+}
+
#else
// expected-no-diagnostics
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -9130,6 +9130,7 @@
Expr *E = ListInit->getInit(0);
auto *RD = E->getType()->getAsCXXRecordDecl();
if (!isa(E) && RD &&
+ isCompleteType(Kind.getLocation(), E->getType()) &&
isOrIsDerivedFromSpecializationOf(RD, Template))
TryListConstructors = false;
}
Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -348,6 +348,31 @@
};
}
+namespace rdar41903969 {
+template struct A {};
+template struct B;
+template struct C {
+ C(A&);
+ C(B&);
+};
+
+void foo(A &a, B &b) {
+ (void)C{b};
+ (void)C{a};
+}
+
+template struct X {
+ X(std::initializer_list) = delete;
+ X(const X&);
+};
+
+template struct D : X {};
+
+void bar(D& d) {
+ (void)X{d};
+}
+}
+
#else
// expected-no-diagnostics
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -9130,6 +9130,7 @@
Expr *E = ListInit->getInit(0);
auto *RD = E->getType()->getAsCXXRecordDecl();
if (!isa(E) && RD &&
+ isCompleteType(Kind.getLocation(), E->getType()) &&
isOrIsDerivedFromSpecializationOf(RD, Template))
TryListConstructors = false;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49766: Fix a crash when an error occurs in Template and the initializer is a nullptr for C++17
erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land. LGTM, thanks! Do you have your commit rights yet? Comment at: include/clang/AST/TemplateBase.h:469 + +// Allow more additional argument kinds for the constructor. +assert(Argument.getKind() == TemplateArgument::NullPtr || This comment sounds pretty strange, how about "Permit any kind of template argument that can be represented with an expression"? Repository: rC Clang https://reviews.llvm.org/D49766 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49952: Check for NULL Destination-Type when creating ArrayConstant
erik.pilkington added inline comments. Comment at: test/CodeGenCXX/empty-struct-init-list.cpp:1-4 +// RUN: %clang_cc1 -std=c++11 %s +// RUN: %clang_cc1 -std=c++14 %s +// RUN: %clang_cc1 -std=c++17 %s +// expected-no-diagnostics You should add -emit-llvm, or else CodeGen won't even run. Also, expected-no-diagnostics only means anything when -verify is included on the run line. Repository: rC Clang https://reviews.llvm.org/D49952 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50088: [Sema] Dig through AutoTypes that have been deduced to an undeduced AutoType in Type::isUndeducedType
erik.pilkington created this revision.
erik.pilkington added a reviewer: rsmith.
Herald added a subscriber: dexonsmith.
Clang used to error out on the attached test case because we deduced an auto
type to an undeduced auto type in the template-id G. This caused
Sema::CheckNonTypeTemplateParameterType to error out because isUndeducedType()
returned false. It seems like we want to retain this type sugar, so this patch
teaches isUndeducedType to dig through any deduced-to-undeduced types.
rdar://41852459
Thanks for taking a look!
Erik
Repository:
rC Clang
https://reviews.llvm.org/D50088
Files:
clang/include/clang/AST/Type.h
clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -335,3 +335,13 @@
void g(int, int);
using Int = A::B<&g>::param2;
}
+
+namespace rdar41852459 {
+template struct G {};
+
+template struct S {
+ template void f() {
+G x;
+ }
+};
+}
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -6456,8 +6456,15 @@
}
inline bool Type::isUndeducedType() const {
- auto *DT = getContainedDeducedType();
- return DT && !DT->isDeduced();
+ QualType Ty(this, 0);
+ while (auto *DT = Ty->getContainedDeducedType()) {
+if (!DT->isDeduced())
+ return true;
+Ty = DT->getDeducedType();
+if (Ty.isNull())
+ return false;
+ }
+ return false;
}
/// Determines whether this is a type for which one can define
Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -335,3 +335,13 @@
void g(int, int);
using Int = A::B<&g>::param2;
}
+
+namespace rdar41852459 {
+template struct G {};
+
+template struct S {
+ template void f() {
+G x;
+ }
+};
+}
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -6456,8 +6456,15 @@
}
inline bool Type::isUndeducedType() const {
- auto *DT = getContainedDeducedType();
- return DT && !DT->isDeduced();
+ QualType Ty(this, 0);
+ while (auto *DT = Ty->getContainedDeducedType()) {
+if (!DT->isDeduced())
+ return true;
+Ty = DT->getDeducedType();
+if (Ty.isNull())
+ return false;
+ }
+ return false;
}
/// Determines whether this is a type for which one can define
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets
erik.pilkington added a comment. Ping! https://reviews.llvm.org/D46845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets
erik.pilkington added a comment. In https://reviews.llvm.org/D46845#1183435, @mclow.lists wrote: > One more thing - > When you add a new header file, you need to update > `include/module.modulemap`, `test/libcxx/double_include.sh.cpp`, and > `include/CMakeLists.txt`. > Take a look at https://reviews.llvm.org/D49338 for an example. Are you sure we have to update test/libcxx/double_include.sh.cpp? Looks like that file only has public headers, but this patch only adds `__node_handle`, which is internal. The new patch adds this header to include/CMakeLists.txt though (`__node_handle` already appeared in module.modulemap) https://reviews.llvm.org/D46845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50122: Complex Variable defined in InitCapture Crash fix
erik.pilkington added a comment. Hi Balaji, Can you try to manually reduce the attached testcase? Its really difficult to understand exactly what is actually happening here, and why your fix is correct, without having a minimal reproducer. Ideally there would just be a few decls that succinctly demonstrate what the problem is. Thanks! Repository: rC Clang https://reviews.llvm.org/D50122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50269: [compiler-rt][builtins] Don't #include CoreFoundation in os_version_check.c
erik.pilkington created this revision. erik.pilkington added a reviewer: arphaman. Herald added subscribers: Sanitizers, llvm-commits, dexonsmith, dberris. This breaks some configurations, so just forward declare everything that we need. rdar://35943793 Thanks for taking a look! Erik Repository: rCRT Compiler Runtime https://reviews.llvm.org/D50269 Files: compiler-rt/lib/builtins/os_version_check.c Index: compiler-rt/lib/builtins/os_version_check.c === --- compiler-rt/lib/builtins/os_version_check.c +++ compiler-rt/lib/builtins/os_version_check.c @@ -15,7 +15,6 @@ #ifdef __APPLE__ -#include #include #include #include @@ -28,6 +27,33 @@ static int32_t GlobalMajor, GlobalMinor, GlobalSubminor; static dispatch_once_t DispatchOnceCounter; +/* We can't include directly from here, so + * just forward declare everything that we need from it. */ + +typedef const void *CFDataRef, *CFAllocatorRef, *CFPropertyListRef, + *CFStringRef, *CFDictionaryRef, *CFTypeRef, *CFErrorRef; + +#if __LLP64__ +typedef unsigned long long CFTypeID; +typedef unsigned long long CFOptionFlags; +typedef signed long long CFIndex; +#else +typedef unsigned long CFTypeID; +typedef unsigned long CFOptionFlags; +typedef signed long CFIndex; +#endif + +typedef unsigned char UInt8; +typedef _Bool Boolean; +typedef CFIndex CFPropertyListFormat; +typedef uint32_t CFStringEncoding; + +/* kCFStringEncodingASCII analog. */ +#define CF_STRING_ENCODING_ASCII 0x0600 +/* kCFStringEncodingUTF8 analog. */ +#define CF_STRING_ENCODING_UTF8 0x08000100 +#define CF_PROPERTY_LIST_IMMUTABLE 0 + typedef CFDataRef (*CFDataCreateWithBytesNoCopyFuncTy)(CFAllocatorRef, const UInt8 *, CFIndex, CFAllocatorRef); @@ -146,15 +172,15 @@ if (CFPropertyListCreateWithDataFunc) PListRef = (*CFPropertyListCreateWithDataFunc)( -NULL, FileContentsRef, kCFPropertyListImmutable, NULL, NULL); +NULL, FileContentsRef, CF_PROPERTY_LIST_IMMUTABLE, NULL, NULL); else PListRef = (*CFPropertyListCreateFromXMLDataFunc)( -NULL, FileContentsRef, kCFPropertyListImmutable, NULL); +NULL, FileContentsRef, CF_PROPERTY_LIST_IMMUTABLE, NULL); if (!PListRef) goto Fail; CFStringRef ProductVersion = (*CFStringCreateWithCStringNoCopyFunc)( - NULL, "ProductVersion", kCFStringEncodingASCII, kCFAllocatorNull); + NULL, "ProductVersion", CF_STRING_ENCODING_ASCII, kCFAllocatorNull); if (!ProductVersion) goto Fail; CFTypeRef OpaqueValue = (*CFDictionaryGetValueFunc)(PListRef, ProductVersion); @@ -165,7 +191,7 @@ char VersionStr[32]; if (!(*CFStringGetCStringFunc)((CFStringRef)OpaqueValue, VersionStr, - sizeof(VersionStr), kCFStringEncodingUTF8)) + sizeof(VersionStr), CF_STRING_ENCODING_UTF8)) goto Fail; sscanf(VersionStr, "%d.%d.%d", &GlobalMajor, &GlobalMinor, &GlobalSubminor); @@ -182,14 +208,10 @@ /* Populate the global version variables, if they haven't already. */ dispatch_once_f(&DispatchOnceCounter, NULL, parseSystemVersionPList); - if (Major < GlobalMajor) -return 1; - if (Major > GlobalMajor) -return 0; - if (Minor < GlobalMinor) -return 1; - if (Minor > GlobalMinor) -return 0; + if (Major < GlobalMajor) return 1; + if (Major > GlobalMajor) return 0; + if (Minor < GlobalMinor) return 1; + if (Minor > GlobalMinor) return 0; return Subminor <= GlobalSubminor; } Index: compiler-rt/lib/builtins/os_version_check.c === --- compiler-rt/lib/builtins/os_version_check.c +++ compiler-rt/lib/builtins/os_version_check.c @@ -15,7 +15,6 @@ #ifdef __APPLE__ -#include #include #include #include @@ -28,6 +27,33 @@ static int32_t GlobalMajor, GlobalMinor, GlobalSubminor; static dispatch_once_t DispatchOnceCounter; +/* We can't include directly from here, so + * just forward declare everything that we need from it. */ + +typedef const void *CFDataRef, *CFAllocatorRef, *CFPropertyListRef, + *CFStringRef, *CFDictionaryRef, *CFTypeRef, *CFErrorRef; + +#if __LLP64__ +typedef unsigned long long CFTypeID; +typedef unsigned long long CFOptionFlags; +typedef signed long long CFIndex; +#else +typedef unsigned long CFTypeID; +typedef unsigned long CFOptionFlags; +typedef signed long CFIndex; +#endif + +typedef unsigned char UInt8; +typedef _Bool Boolean; +typedef CFIndex CFPropertyListFormat; +typedef uint32_t CFStringEncoding; + +/* kCFStringEncodingASCII analog. */ +#define CF_STRING_ENCODING_ASCII 0x0600 +/* kCFStringEncodingUTF8 analog. */ +#define CF_STRING_ENCODING_UTF8 0x08000100 +#define CF_PROPERTY_LIST_IMMUTABLE 0 + typedef CFDataRef (*CFDataCreateWithBytesNoCopyFuncTy)(CFAllocatorRef,
[PATCH] D33450: Warn about uses of `@available` that can't suppress the -Wunguarded-availability warnings
erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: lib/Sema/SemaExpr.cpp:15747 + // warn when it's used inappropriately (i.e. not if(@available)). + if (getCurFunctionOrMethodDecl()) +getEnclosingFunction()->HasPotentialAvailabilityViolations = true; erik.pilkington wrote: > I believe this will not be set if we're rebuilding a templated function, we > should also flag this from `TransformObjCAvailabilityCheckExpr` in > TreeTransform.h. On second thought, there is no point in doing that because we'd have already emitted a diagnostic from the pattern. This is correct as is. Repository: rL LLVM https://reviews.llvm.org/D33450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler
erik.pilkington added inline comments. Comment at: src/cxa_demangle.cpp:3036 break; -if (db.names.size() < 2) +assert(k0 <= k1 && "parse_type() mutated the name stack"); +if (k1 == k0) dexonsmith wrote: > There's no `#include ` in this file. Depending on the standard > library you're building against, you made need that here. Sure, I'll commit this with that change. https://reviews.llvm.org/D33368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler
erik.pilkington added a comment. r303806 removes the assertion (instead just returning first). I though this should never happen, I'm looking into this testcase to see if there is another bug here. Thanks, Erik Repository: rL LLVM https://reviews.llvm.org/D33368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler
erik.pilkington added a comment. > Also, are you now maintaining this code? > I am trying to find someone who wants to be CC-ed to other demangler bugs > automatically reported by oss-fuzz. I don’t think I’ll accept the title of maintainer, (I only have one commit in this file!) but I have some plans to work on it and would be interested in hearing about any bugs found in the fuzzer. Can you add erik.pilking...@gmail.com to your CC list? Repository: rL LLVM https://reviews.llvm.org/D33368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33661: [Sema][ObjC] Don't emit availability diagnostics for categories extending unavailable interfaces
erik.pilkington created this revision.
Previously, @implementations of categories that extended
unavailable/deprecated/partial @interfaces triggered availability diagnostics.
There was no way to turn this off, as we check the use of the unavailable decl
outside of the context of the @implementation, so attaching an attribute to the
category @interface doesn't do anything. Moreover, this diagnostic can't
actually catch any bugs even if it worked correctly, there is no way to use
stuff declared in a category without referencing the extended @interface
(AFAIK, I'm not fluent in Objective-C), which will trigger the diagnostic.
rdar://32427296
Thanks for taking a look,
Erik
https://reviews.llvm.org/D33661
Files:
lib/Sema/SemaDeclObjC.cpp
test/SemaObjC/attr-deprecated.m
test/SemaObjC/class-unavail-warning.m
test/SemaObjC/warn-deprecated-implementations.m
Index: test/SemaObjC/warn-deprecated-implementations.m
===
--- test/SemaObjC/warn-deprecated-implementations.m
+++ test/SemaObjC/warn-deprecated-implementations.m
@@ -28,15 +28,14 @@
- (void) G {} // No warning, implementing its own deprecated method
@end
-__attribute__((deprecated)) // expected-note 2 {{'CL' has been explicitly
marked deprecated here}}
+__attribute__((deprecated)) // expected-note {{'CL' has been explicitly marked
deprecated here}}
@interface CL // expected-note 2 {{class declared here}}
@end
@implementation CL // expected-warning {{Implementing deprecated class}}
@end
-@implementation CL ( SomeCategory ) // expected-warning {{'CL' is deprecated}}
\
-// expected-warning {{Implementing
deprecated category}}
+@implementation CL (SomeCategory) // expected-warning {{Implementing
deprecated category}}
@end
@interface CL_SUB : CL // expected-warning {{'CL' is deprecated}}
Index: test/SemaObjC/class-unavail-warning.m
===
--- test/SemaObjC/class-unavail-warning.m
+++ test/SemaObjC/class-unavail-warning.m
@@ -2,7 +2,7 @@
// rdar://9092208
__attribute__((unavailable("not available")))
-@interface MyClass { // expected-note 8 {{'MyClass' has been explicitly marked
unavailable here}}
+@interface MyClass { // expected-note 7 {{'MyClass' has been explicitly marked
unavailable here}}
@public
void *_test;
MyClass *ivar; // no error.
@@ -28,7 +28,7 @@
@interface MyClass (Cat2) // no error.
@end
-@implementation MyClass (Cat2) // expected-error {{unavailable}}
+@implementation MyClass (Cat2) // no error.
@end
int main() {
Index: test/SemaObjC/attr-deprecated.m
===
--- test/SemaObjC/attr-deprecated.m
+++ test/SemaObjC/attr-deprecated.m
@@ -83,7 +83,7 @@
}
-__attribute ((deprecated)) // expected-note 2 {{'DEPRECATED' has been
explicitly marked deprecated here}}
+__attribute ((deprecated)) // expected-note {{'DEPRECATED' has been explicitly
marked deprecated here}}
@interface DEPRECATED {
@public int ivar;
DEPRECATED *ivar2; // no warning.
@@ -98,9 +98,17 @@
@end
@interface DEPRECATED (Category2) // no warning.
+- (id)meth;
@end
-@implementation DEPRECATED (Category2) // expected-warning {{'DEPRECATED' is
deprecated}}
+__attribute__((deprecated))
+void depr_function();
+
+@implementation DEPRECATED (Category2) // no warning
+- (id)meth {
+ depr_function(); // no warning.
+ return 0;
+}
@end
@interface NS : DEPRECATED // expected-warning {{'DEPRECATED' is deprecated}}
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -1851,10 +1851,6 @@
// FIXME: PushOnScopeChains?
CurContext->addDecl(CDecl);
- // If the interface is deprecated/unavailable, warn/error about it.
- if (IDecl)
-DiagnoseUseOfDecl(IDecl, ClassLoc);
-
// If the interface has the objc_runtime_visible attribute, we
// cannot implement a category for it.
if (IDecl && IDecl->hasAttr()) {
Index: test/SemaObjC/warn-deprecated-implementations.m
===
--- test/SemaObjC/warn-deprecated-implementations.m
+++ test/SemaObjC/warn-deprecated-implementations.m
@@ -28,15 +28,14 @@
- (void) G {} // No warning, implementing its own deprecated method
@end
-__attribute__((deprecated)) // expected-note 2 {{'CL' has been explicitly marked deprecated here}}
+__attribute__((deprecated)) // expected-note {{'CL' has been explicitly marked deprecated here}}
@interface CL // expected-note 2 {{class declared here}}
@end
@implementation CL // expected-warning {{Implementing deprecated class}}
@end
-@implementation CL ( SomeCategory ) // expected-warning {{'CL' is deprecated}} \
-// expected-warning {{Implementing deprecated category}}
+@implementation CL (SomeCateg
[PATCH] D33816: [Sema][ObjC] Don't allow -Wunguarded-availability to be silenced with redeclarations
erik.pilkington created this revision.
This patch drops support for suppressing -Wunguarded-availability with
redeclarations. This was behavior left over from the -Wpartial-availability
days, where it was the only way of silencing the diagnostic. Now that we have
@available and better support for annotating contexts with availability
attributes, this behavior should be dropped.
This would emit warnings on code that used the old way of silencing the
diagnostic, but this method:
- Didn't have many users; it was clunky to use
- Has a simple upgrade path that is mentioned in the diagnostics we emit
- Allows for availability violations to go undetected
@thakis: I know the chromium project uses this warning, is this OK with you?
rdar://32388777
Thanks for taking a look,
Erik
https://reviews.llvm.org/D33816
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaExpr.cpp
test/Sema/attr-availability.c
test/Sema/attr-deprecated.c
test/Sema/attr-unavailable-message.c
test/SemaCXX/attr-deprecated.cpp
test/SemaObjC/attr-availability.m
test/SemaObjC/unguarded-availability.m
Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -5,6 +5,8 @@
#define AVAILABLE_10_11 __attribute__((availability(macos, introduced = 10.11)))
#define AVAILABLE_10_12 __attribute__((availability(macos, introduced = 10.12)))
+typedef int AVAILABLE_10_12 new_int; // expected-note + {{marked partial here}}
+
int func_10_11() AVAILABLE_10_11; // expected-note 4 {{'func_10_11' has been explicitly marked partial here}}
#ifdef OBJCPP
@@ -70,9 +72,9 @@
}
__attribute__((objc_root_class))
-AVAILABLE_10_11 @interface Class_10_11 {
+AVAILABLE_10_11 @interface Class_10_11 { // expected-note{{annotate 'Class_10_11' with an availability attribute to silence}}
int_10_11 foo;
- int_10_12 bar; // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}} expected-note{{redeclare}}
+ int_10_12 bar; // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}}
}
- (void)method1;
- (void)method2;
@@ -125,7 +127,7 @@
};
}
-void test_params(int_10_12 x); // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}} expected-note{{redeclare}}
+void test_params(int_10_12 x); // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}} expected-note{{annotate 'test_params' with an availability attribute to silence}}
void test_params2(int_10_12 x) AVAILABLE_10_12; // no warn
@@ -234,3 +236,23 @@
}
#endif
+
+struct InStruct { // expected-note{{annotate 'InStruct' with an availability attribute to silence}}
+ new_int mem; // expected-warning{{'new_int' is partial}}
+
+ struct { new_int mem; } anon; // expected-warning{{'new_int' is partial}} expected-note{{annotate '' with an availability attribute}}
+};
+
+@interface InInterface
+-(new_int)meth; // expected-warning{{'new_int' is partial}} expected-note{{annotate 'meth' with an availability attribute}}
+@end
+
+@interface Proper // expected-note{{annotate 'Proper' with an availability attribute}}
+@property (class) new_int x; // expected-warning{{'new_int' is partial}}
+@end
+
+void with_local_struct() {
+ struct local { // expected-note{{annotate 'local' with an availability attribute}}
+new_int x; // expected-warning{{'new_int' is partial}}
+ };
+}
Index: test/SemaObjC/attr-availability.m
===
--- test/SemaObjC/attr-availability.m
+++ test/SemaObjC/attr-availability.m
@@ -13,7 +13,7 @@
@interface A
- (void)method __attribute__((availability(macosx,introduced=10.1,deprecated=10.2))); // expected-note {{'method' has been explicitly marked deprecated here}}
#if defined(WARN_PARTIAL)
- // expected-note@+2 {{'partialMethod' has been explicitly marked partial here}}
+ // expected-note@+2 2 {{'partialMethod' has been explicitly marked partial here}}
#endif
- (void)partialMethod __attribute__((availability(macosx,introduced=10.8)));
@@ -66,7 +66,10 @@
@end
void f_after_redecl(A *a, B *b) {
- [a partialMethod]; // no warning
+#ifdef WARN_PARTIAL
+ // expected-warning@+2{{'partialMethod' is only available on macOS 10.8 or newer}} expected-note@+2 {{@available}}
+#endif
+ [a partialMethod];
[b partialMethod]; // no warning
[a partial_proto_method]; // no warning
[b partial_proto_method]; // no warning
@@ -133,6 +136,10 @@
@end
@interface PartialI
+#ifdef WARN_PARTIAL
+// expected-note@+3{{marked partial here}}
+// expected-note@+3{{marked partial here}}
+#endif
- (void)partialMethod __attribute__((availability(macosx,introduced=10.8)));
+ (void)partialMethod __attribute__((availability(macosx,introduced=10.8)));
@end
@@ -160,13 +167,19 @@
@end
void partialfun(PartialI* a) {
- [a partialMethod]; // no warning
+#ifdef WARN_PARTIAL
+
[PATCH] D33977: [libcxx][WIP] Experimental support for a scheduler for use in the parallel stl algorithms
erik.pilkington created this revision.
Herald added a reviewer: EricWF.
This patch adds a simple work-stealing scheduler meant for use as a fallback
implementation for the C++17 parallel stl algorithms. This scheme follows a
very simple fork/join API that should be easy to map onto different underlying
concurrency implementations if they are available. This should be suitable for
implementing par & par_unseq on top of.
I also tried this out with a lock-free deque[0]. This provides a modest
performance improvement, and might be worth implementing. For the sake of doing
this incrementally this patch just contains a locking deque.
Please see the recent thread on cfe-dev for context:
http://lists.llvm.org/pipermail/cfe-dev/2017-May/053841.html
I'm still pretty new to libc++ and to parallel stuff, so please feel free to
tear this patch apart!
Thanks for taking a look,
Erik
[0]:
https://pdfs.semanticscholar.org/3771/77bb82105c35e6e26ebad1698a20688473bd.pdf
https://reviews.llvm.org/D33977
Files:
include/experimental/execution
src/experimental/execution.cpp
test/libcxx/experimental/execution/fork_join.pass.cpp
Index: test/libcxx/experimental/execution/fork_join.pass.cpp
===
--- /dev/null
+++ test/libcxx/experimental/execution/fork_join.pass.cpp
@@ -0,0 +1,58 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include
+#include
+#include
+
+// UNSUPPORTED: libcpp-has-no-threads
+// UNSUPPORTED: c++98, c++03
+
+using namespace std;
+using namespace experimental;
+
+int parallel_fibb(int n, __task t) {
+ if (n < 2)
+return 1;
+
+ int lhs;
+ t.__fork([&](__task t) { lhs = parallel_fibb(n - 2, t); });
+ int rhs = parallel_fibb(n - 1, t);
+ t.__join();
+ return rhs + lhs;
+}
+
+void fork_many(int n, __task t) {
+ atomic count(0);
+ for (int i = 0; i < n; ++i)
+t.__fork([&](__task) { ++count; });
+
+ t.__join();
+ assert(count.load() == n);
+}
+
+int main() {
+ {
+int res;
+__evaluate_parallel_task([&](__task t) { res = parallel_fibb(10, t); });
+assert(res == 89);
+ }
+
+ {
+__evaluate_parallel_task([&](__task t) { fork_many(100, t); });
+ }
+
+ {
+auto f = async(launch::async, [] {
+ __evaluate_parallel_task([&](__task t) { fork_many(100, t); });
+});
+__evaluate_parallel_task([&](__task t) { fork_many(100, t); });
+f.get();
+ }
+}
Index: src/experimental/execution.cpp
===
--- /dev/null
+++ src/experimental/execution.cpp
@@ -0,0 +1,243 @@
+// -*- C++ -*-
+//=== execution.cpp ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "experimental/__config"
+#ifndef _LIBCPP_HAS_NO_THREADS
+
+#include "experimental/execution"
+#include "atomic"
+#include "deque"
+#include "mutex"
+#include "vector"
+#include "thread"
+
+_LIBCPP_BEGIN_NAMESPACE_EXPERIMENTAL
+
+namespace {
+class task_group;
+class worker;
+class bound_task;
+
+// FIXME: replace this with a lock free version.
+template
+class stealing_deque {
+ deque queue_;
+ mutex lock_;
+
+public:
+ stealing_deque() = default;
+ stealing_deque(const stealing_deque& other) : queue_(other.queue_) {}
+
+ void emplace_back(T&& input) {
+lock_guard ul(lock_);
+queue_.emplace_back(move(input));
+ }
+
+ bool pop_back(T& output) {
+lock_guard ul(lock_);
+if (queue_.empty())
+ return false;
+output = queue_.back();
+queue_.pop_back();
+return true;
+ }
+
+ bool steal(T& output) {
+lock_guard ul(lock_);
+if (queue_.empty())
+ return false;
+output = queue_.front();
+queue_.pop_front();
+return true;
+ }
+};
+
+// Task that has been fork()'d, but is free to be stolen by another worker.
+struct unbound_task {
+ bound_task* parent_;
+ __callable_task task_;
+
+ unbound_task() = default;
+ unbound_task(const __callable_task& task) : parent_(nullptr), task_(task) {}
+ unbound_task(bound_task* parent, __callable_task&& task)
+ : parent_(parent), task_(move(task)) {}
+};
+
+class worker {
+ stealing_deque queue_;
+ task_group& group_;
+ atomic dead_;
+
+ friend class task_group;
+
+ unsigned get_worker_id();
+
+public:
+ worker(task_group& group) : group_(group), dead_(true) {}
+ worker(const worker& other)
+ : queue_(other.queue_), group_(other.group_), dead_(true) {}
+
+ void def
[PATCH] D34096: [Sema][C++1z] Ensure structured binding's bindings in dependent foreach have non-null type
erik.pilkington created this revision.
A DecompositionDecls' bindings have a null type until the initializer is
attached, if the initializer is dependent, then the bindings should be set to
have dependent type. For non-foreach bindings, this is done in
Sema::CheckCompleteDecompositionDeclaration(), this patch ensures that this is
done to bindings in a foreach as well. Fixes PR32172.
Thanks for taking a look!
Erik
https://reviews.llvm.org/D34096
Files:
lib/Sema/SemaStmt.cpp
test/SemaCXX/cxx1z-decomposition.cpp
Index: test/SemaCXX/cxx1z-decomposition.cpp
===
--- test/SemaCXX/cxx1z-decomposition.cpp
+++ test/SemaCXX/cxx1z-decomposition.cpp
@@ -70,4 +70,10 @@
return foobar_; // expected-error {{undeclared identifier 'foobar_'}}
}
+// PR32172
+template void dependent_foreach(T t) {
+ for (auto [a,b,c] : t)
+a,b,c;
+}
+
// FIXME: by-value array copies
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -2206,8 +2206,12 @@
// Deduce any 'auto's in the loop variable as 'DependentTy'. We'll fill
// them in properly when we instantiate the loop.
-if (!LoopVar->isInvalidDecl() && Kind != BFRK_Check)
+if (!LoopVar->isInvalidDecl() && Kind != BFRK_Check) {
+ if (auto *DD = dyn_cast(LoopVar))
+for (auto *Binding : DD->bindings())
+ Binding->setType(Context.DependentTy);
LoopVar->setType(SubstAutoType(LoopVar->getType(), Context.DependentTy));
+}
} else if (!BeginDeclStmt.get()) {
SourceLocation RangeLoc = RangeVar->getLocation();
Index: test/SemaCXX/cxx1z-decomposition.cpp
===
--- test/SemaCXX/cxx1z-decomposition.cpp
+++ test/SemaCXX/cxx1z-decomposition.cpp
@@ -70,4 +70,10 @@
return foobar_; // expected-error {{undeclared identifier 'foobar_'}}
}
+// PR32172
+template void dependent_foreach(T t) {
+ for (auto [a,b,c] : t)
+a,b,c;
+}
+
// FIXME: by-value array copies
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -2206,8 +2206,12 @@
// Deduce any 'auto's in the loop variable as 'DependentTy'. We'll fill
// them in properly when we instantiate the loop.
-if (!LoopVar->isInvalidDecl() && Kind != BFRK_Check)
+if (!LoopVar->isInvalidDecl() && Kind != BFRK_Check) {
+ if (auto *DD = dyn_cast(LoopVar))
+for (auto *Binding : DD->bindings())
+ Binding->setType(Context.DependentTy);
LoopVar->setType(SubstAutoType(LoopVar->getType(), Context.DependentTy));
+}
} else if (!BeginDeclStmt.get()) {
SourceLocation RangeLoc = RangeVar->getLocation();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33977: [libcxx][WIP] Experimental support for a scheduler for use in the parallel stl algorithms
erik.pilkington added a comment. Ping! https://reviews.llvm.org/D33977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets
erik.pilkington added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:7315
+ default:
+assert(!Triple.isMacOSX() && "MacOS should be handled in the switch");
+// New targets should always warn about availability.
This assert seems a bit redundant, no?
Comment at: lib/Sema/SemaDeclAttr.cpp:7350
+unsigned DiagKind =
+!SemaRef.Diags.isIgnored(diag::warn_unguarded_availability_new,
+ Range.getBegin()) &&
There is a version of this for decls that aren't referenced in a function, such
as:
```
typedef int __attribute__((availability(macos, introducced=1000))) new_int;
new_int x; // warn
// Also:
struct S {
new_int x; // warn
};
```
-Wunguarded-availability-new should also work with this, right? If we do want
to support that, maybe we should add this check to
ShouldDiagnoseAvailabilityOfDecl(), which is called by both paths.
Repository:
rL LLVM
https://reviews.llvm.org/D34264
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33816: [Sema][ObjC] Don't allow -Wunguarded-availability to be silenced with redeclarations
erik.pilkington added a comment. Ping! https://reviews.llvm.org/D33816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets
erik.pilkington added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:6944 -diag = !ObjCPropertyAccess ? diag::err_unavailable - : diag::err_property_method_unavailable; -diag_message = diag::err_unavailable_message; Why did you remove the AR_Unavailable checking? Comment at: lib/Sema/SemaDeclAttr.cpp:7021 +bool NewWarning = +!S.Diags.isIgnored(diag::warn_unguarded_availability_new, Loc) && +shouldDiagnoseAvailabilityByDefault( I think checking Diags::isIgnored is fairly expensive, maybe we should swap the operands to the '&&' so that it is only done if necessary? Repository: rL LLVM https://reviews.llvm.org/D34264 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets
erik.pilkington added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:7031 +Introduced) && +!S.Diags.isIgnored(diag::warn_unguarded_availability_new, Loc); +diag = NewWarning ? diag::warn_partial_availability_new Sorry to keep this going so long, but why are we even checking isIgnored? The only difference it could make in whether we emit a diagnostic is if both: -Wunguarded-availability and -Wno-unguarded-availability-new are passed in, which seems like it would never happen, right? Even if somebody did pass that in, it seems reasonable to warn on old stuff but not new stuff. Maybe I'm missing something here? Repository: rL LLVM https://reviews.llvm.org/D34264 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets
erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land. LGTM, thanks for working on this! Repository: rL LLVM https://reviews.llvm.org/D34264 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33606: [Sema] Fix a crash-on-invalid when a template parameter list has a class definition or non-reference class type
erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D33606 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34556: [libcxx] Annotate c++17 aligned new/delete operators with availability attribute
erik.pilkington added inline comments.
Comment at:
test/std/language.support/support.dynamic/new.delete/new.delete.placement/new_deployment.fail.cpp:35
+ // expected-error@-10 {{call to unavailable function 'operator new[]':
introduced in macOS 10.13}}
+ // expected-note@new:204 {{candidate function has been explicitly made
unavailable}}
+ // expected-note@new:188 {{candidate function not viable: no known
conversion from 'std::align_val_t' to 'const std::nothrow_t' for 2nd argument}}
You probably shouldn't include the line numbers here, it makes the test
fragile. Ie, do `expected-note@new:*`
https://reviews.llvm.org/D34556
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33816: [Sema][ObjC] Don't allow -Wunguarded-availability to be silenced with redeclarations
erik.pilkington updated this revision to Diff 103852.
erik.pilkington added a comment.
Improve enum diagnostics, as @arphaman suggested. This causes a bit of churn
throughout the availability diagnostic machinery, if it would make it at all
easier to review, I would be happy to separate out these changes.
Thanks,
Erik
https://reviews.llvm.org/D33816
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/DelayedDiagnostic.h
include/clang/Sema/Sema.h
lib/Sema/DelayedDiagnostic.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaExpr.cpp
test/Sema/attr-availability.c
test/Sema/attr-deprecated.c
test/Sema/attr-unavailable-message.c
test/SemaCXX/attr-deprecated.cpp
test/SemaObjC/attr-availability.m
test/SemaObjC/unguarded-availability-new.m
test/SemaObjC/unguarded-availability.m
Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -5,6 +5,8 @@
#define AVAILABLE_10_11 __attribute__((availability(macos, introduced = 10.11)))
#define AVAILABLE_10_12 __attribute__((availability(macos, introduced = 10.12)))
+typedef int AVAILABLE_10_12 new_int; // expected-note + {{marked partial here}}
+
int func_10_11() AVAILABLE_10_11; // expected-note 4 {{'func_10_11' has been explicitly marked partial here}}
#ifdef OBJCPP
@@ -70,9 +72,9 @@
}
__attribute__((objc_root_class))
-AVAILABLE_10_11 @interface Class_10_11 {
+AVAILABLE_10_11 @interface Class_10_11 { // expected-note{{annotate 'Class_10_11' with an availability attribute to silence}}
int_10_11 foo;
- int_10_12 bar; // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}} expected-note{{redeclare}}
+ int_10_12 bar; // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}}
}
- (void)method1;
- (void)method2;
@@ -125,7 +127,7 @@
};
}
-void test_params(int_10_12 x); // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}} expected-note{{redeclare}}
+void test_params(int_10_12 x); // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}} expected-note{{annotate 'test_params' with an availability attribute to silence}}
void test_params2(int_10_12 x) AVAILABLE_10_12; // no warn
@@ -234,3 +236,23 @@
}
#endif
+
+struct InStruct { // expected-note{{annotate 'InStruct' with an availability attribute to silence}}
+ new_int mem; // expected-warning{{'new_int' is partial}}
+
+ struct { new_int mem; } anon; // expected-warning{{'new_int' is partial}} expected-note{{annotate '' with an availability attribute}}
+};
+
+@interface InInterface
+-(new_int)meth; // expected-warning{{'new_int' is partial}} expected-note{{annotate 'meth' with an availability attribute}}
+@end
+
+@interface Proper // expected-note{{annotate 'Proper' with an availability attribute}}
+@property (class) new_int x; // expected-warning{{'new_int' is partial}}
+@end
+
+void with_local_struct() {
+ struct local { // expected-note{{annotate 'local' with an availability attribute}}
+new_int x; // expected-warning{{'new_int' is partial}}
+ };
+}
Index: test/SemaObjC/unguarded-availability-new.m
===
--- test/SemaObjC/unguarded-availability-new.m
+++ test/SemaObjC/unguarded-availability-new.m
@@ -96,16 +96,16 @@
FUNC_AVAILABLE new_int x;
#ifndef NO_WARNING
#ifdef MAC
- // expected-warning@-3 {{'new_int' is partial: introduced in macOS 10.14}} expected-note@-3 {{explicitly redeclare 'new_int' to silence this warning}}
+ // expected-warning@-3 {{'new_int' is partial: introduced in macOS 10.14}} expected-note@-3 {{annotate 'new_int'}}
#endif
#ifdef IOS
- // expected-warning@-6 {{'new_int' is partial: introduced in iOS 12}} expected-note@-6 {{explicitly redeclare 'new_int' to silence this warning}}
+ // expected-warning@-6 {{'new_int' is partial: introduced in iOS 12}} expected-note@-6 {{annotate 'new_int'}}
#endif
#ifdef TVOS
- // expected-warning@-9 {{'new_int' is partial: introduced in tvOS 13}} expected-note@-9 {{explicitly redeclare 'new_int' to silence this warning}}
+ // expected-warning@-9 {{'new_int' is partial: introduced in tvOS 13}} expected-note@-9 {{annotate 'new_int'}}
#endif
#ifdef WATCHOS
- // expected-warning@-12 {{'new_int' is partial: introduced in watchOS 5}} expected-note@-12 {{explicitly redeclare 'new_int' to silence this warning}}
+ // expected-warning@-12 {{'new_int' is partial: introduced in watchOS 5}} expected-note@-12 {{annotate 'new_int'}}
#endif
#endif
Index: test/SemaObjC/attr-availability.m
===
--- test/SemaObjC/attr-availability.m
+++ test/SemaObjC/attr-availability.m
@@ -13,7 +13,7 @@
@interface A
- (void)method __attribute__((availability(macosx,introduced=10.1,deprecated=10.2))); // expected-note {{'method' has been explicitly marked deprecated here}}
[PATCH] D33816: [Sema][ObjC] Don't allow -Wunguarded-availability to be silenced with redeclarations
erik.pilkington updated this revision to Diff 103958.
erik.pilkington added a comment.
Make the diagnostic reference the context declaration instead of the offending
decl if no enclosing decl is found, fixing the diagnostic bug @arphaman pointed
out.
https://reviews.llvm.org/D33816
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/DelayedDiagnostic.h
include/clang/Sema/Sema.h
lib/Sema/DelayedDiagnostic.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaExpr.cpp
test/Sema/attr-availability.c
test/Sema/attr-deprecated.c
test/Sema/attr-unavailable-message.c
test/SemaCXX/attr-deprecated.cpp
test/SemaObjC/attr-availability.m
test/SemaObjC/unguarded-availability-new.m
test/SemaObjC/unguarded-availability.m
Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -5,6 +5,8 @@
#define AVAILABLE_10_11 __attribute__((availability(macos, introduced = 10.11)))
#define AVAILABLE_10_12 __attribute__((availability(macos, introduced = 10.12)))
+typedef int AVAILABLE_10_12 new_int; // expected-note + {{marked partial here}}
+
int func_10_11() AVAILABLE_10_11; // expected-note 4 {{'func_10_11' has been explicitly marked partial here}}
#ifdef OBJCPP
@@ -70,9 +72,9 @@
}
__attribute__((objc_root_class))
-AVAILABLE_10_11 @interface Class_10_11 {
+AVAILABLE_10_11 @interface Class_10_11 { // expected-note{{annotate 'Class_10_11' with an availability attribute to silence}}
int_10_11 foo;
- int_10_12 bar; // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}} expected-note{{redeclare}}
+ int_10_12 bar; // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}}
}
- (void)method1;
- (void)method2;
@@ -125,7 +127,7 @@
};
}
-void test_params(int_10_12 x); // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}} expected-note{{redeclare}}
+void test_params(int_10_12 x); // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}} expected-note{{annotate 'test_params' with an availability attribute to silence}}
void test_params2(int_10_12 x) AVAILABLE_10_12; // no warn
@@ -234,3 +236,23 @@
}
#endif
+
+struct InStruct { // expected-note{{annotate 'InStruct' with an availability attribute to silence}}
+ new_int mem; // expected-warning{{'new_int' is partial}}
+
+ struct { new_int mem; } anon; // expected-warning{{'new_int' is partial}} expected-note{{annotate '' with an availability attribute}}
+};
+
+@interface InInterface
+-(new_int)meth; // expected-warning{{'new_int' is partial}} expected-note{{annotate 'meth' with an availability attribute}}
+@end
+
+@interface Proper // expected-note{{annotate 'Proper' with an availability attribute}}
+@property (class) new_int x; // expected-warning{{'new_int' is partial}}
+@end
+
+void with_local_struct() {
+ struct local { // expected-note{{annotate 'local' with an availability attribute}}
+new_int x; // expected-warning{{'new_int' is partial}}
+ };
+}
Index: test/SemaObjC/unguarded-availability-new.m
===
--- test/SemaObjC/unguarded-availability-new.m
+++ test/SemaObjC/unguarded-availability-new.m
@@ -96,16 +96,16 @@
FUNC_AVAILABLE new_int x;
#ifndef NO_WARNING
#ifdef MAC
- // expected-warning@-3 {{'new_int' is partial: introduced in macOS 10.14}} expected-note@-3 {{explicitly redeclare 'new_int' to silence this warning}}
+ // expected-warning@-3 {{'new_int' is partial: introduced in macOS 10.14}} expected-note@-3 {{annotate 'x' with an availability attribute to silence}}
#endif
#ifdef IOS
- // expected-warning@-6 {{'new_int' is partial: introduced in iOS 12}} expected-note@-6 {{explicitly redeclare 'new_int' to silence this warning}}
+ // expected-warning@-6 {{'new_int' is partial: introduced in iOS 12}} expected-note@-6 {{annotate 'x' with an availability attribute to silence}}
#endif
#ifdef TVOS
- // expected-warning@-9 {{'new_int' is partial: introduced in tvOS 13}} expected-note@-9 {{explicitly redeclare 'new_int' to silence this warning}}
+ // expected-warning@-9 {{'new_int' is partial: introduced in tvOS 13}} expected-note@-9 {{annotate 'x' with an availability attribute to silence}}
#endif
#ifdef WATCHOS
- // expected-warning@-12 {{'new_int' is partial: introduced in watchOS 5}} expected-note@-12 {{explicitly redeclare 'new_int' to silence this warning}}
+ // expected-warning@-12 {{'new_int' is partial: introduced in watchOS 5}} expected-note@-12 {{annotate 'x' with an availability attribute to silence}}
#endif
#endif
Index: test/SemaObjC/attr-availability.m
===
--- test/SemaObjC/attr-availability.m
+++ test/SemaObjC/attr-availability.m
@@ -13,7 +13,7 @@
@interface A
- (void)method __attribute__((availability(macosx,introduced=10.1,deprecated=10.2))
[PATCH] D41261: [libcxxabi][demangler] Special case demangling for pass_object_size attribute
erik.pilkington created this revision.
erik.pilkington added reviewers: EricWF, george.burgess.iv.
This patch adds demangling for pass_object_size attribute
(https://clang.llvm.org/docs/AttributeReference.html#pass-object-size). This
attribute applies to function parameters. This attribute is mangled as if it
was meant to match the production
(https://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangle.qualified-type), but
clang emits it after the type it refers to instead of before. I think this was
a mistake, but its probably too late to fix it because of the ABI break.
Thanks for taking a look!
Erik
Repository:
rCXXA libc++abi
https://reviews.llvm.org/D41261
Files:
src/cxa_demangle.cpp
test/test_demangle.pass.cpp
Index: test/test_demangle.pass.cpp
===
--- test/test_demangle.pass.cpp
+++ test/test_demangle.pass.cpp
@@ -29611,6 +29611,11 @@
{"_ZN1SB5outer1fB5innerEv", "S[abi:outer]::f[abi:inner]()"},
{"_ZN1SC2B8ctor_tagEv", "S::S[abi:ctor_tag]()"},
{"_ZplB4MERP1SS_", "operator+[abi:MERP](S, S)"},
+// attribute pass_object_size
+{"_Z27NoViableOverloadObjectSize0PvU17pass_object_size0",
"NoViableOverloadObjectSize0(pass_object_size0 void*)"},
+{"_ZN14noninline_virt1AC2EiO1QPvU17pass_object_size0",
"noninline_virt::A::A(int, Q&&, pass_object_size0 void*)"},
+{"_ZZN7lambdas7LambdasEPcENK3$_0clEPvU17pass_object_size0",
"lambdas::Lambdas(char*)::$_0::operator()(pass_object_size0 void*) const"},
+{"_ZN8variadic6AsCtorC1EPKcU17pass_object_size0dz",
"variadic::AsCtor::AsCtor(pass_object_size0 char const*, double, ...)"},
};
const unsigned N = sizeof(cases) / sizeof(cases[0]);
Index: src/cxa_demangle.cpp
===
--- src/cxa_demangle.cpp
+++ src/cxa_demangle.cpp
@@ -12,6 +12,7 @@
// - enable_if attribute
// - decomposition declarations
// - C++ modules TS
+// - inheriting constructors
#define _LIBCPP_NO_EXCEPTIONS
@@ -3128,11 +3129,33 @@
return first;
}
+// Parse a type in the context of a function parameter. Here, the type can be
+// optionally followed by a pass_object_size attribute. Unlike other vendor
+// extension attributes, clang emits this attribute after the mangling for the
+// type it applies to, so we have to special-case it.
+//
+// ::= U17pass_object_size [0-9] # extension
+const char*
+parse_function_param_type(const char* first, const char* last, Db& db)
+{
+const char* t = parse_type(first, last, db);
+if (t != first && StringView(t, last).startsWith("U17pass_object_size"))
+{
+const char* t2 = t + std::strlen("U17pass_object_size");
+if (t2 != last && *t2 >= '0' && *t2 <= '9' && !db.Names.empty())
+{
+db.Names.back() = db.make(
+db.make(StringView(t+3, t2+1)), db.Names.back());
+t = t2+1;
+}
+}
+return t;
+}
+
// ::= R # & ref-qualifier
// ::= O # && ref-qualifier
-
-// ::= F [Y] [] E
-
+//
+// ::= F [Y] [] E
const char*
parse_function_type(const char* first, const char* last, Db& db)
{
@@ -3186,7 +3209,7 @@
continue;
}
size_t k0 = db.Names.size();
-t1 = parse_type(t, last, db);
+t1 = parse_function_param_type(t, last, db);
size_t k1 = db.Names.size();
if (t1 == t || t1 == last || k1 < k0)
return first;
@@ -4454,7 +4477,7 @@
{
while (true)
{
-const char* t1 = parse_type(t0, last, db);
+const char* t1 = parse_function_param_type(t0, last, db);
if (t1 == t0)
break;
t0 = t1;
@@ -6021,7 +6044,7 @@
size_t params_begin = db.Names.size();
while (true)
{
-t2 = parse_type(t, last, db);
+t2 = parse_function_param_type(t, last, db);
if (t2 == t)
break;
t = t2;
Index: test/test_demangle.pass.cpp
===
--- test/test_demangle.pass.cpp
+++ test/test_demangle.pass.cpp
@@ -29611,6 +29611,11 @@
{"_ZN1SB5outer1fB5innerEv", "S[abi:outer]::f[abi:inner]()"},
{"_ZN1SC2B8ctor_tagEv", "S::S[abi:ctor_tag]()"},
{"_ZplB4MERP1SS_", "operator+[abi:MERP](S, S)"},
+// attribute pass_object_size
+{"_Z27NoViableOverloadObjectSize0PvU17pass_object_size0", "NoViableOverloadObjectSize0(pass_object_size0 void*)"},
+{"_ZN14noninline_virt1AC2EiO1QPvU17pass_object_size0", "noninline_virt::A::A(int, Q&&, pass_object_size0 void*)"},
+{"_ZZN7lambdas7LambdasEPcE
[PATCH] D41261: [libcxxabi][demangler] Special case demangling for pass_object_size attribute
erik.pilkington updated this revision to Diff 127067.
erik.pilkington added a comment.
Add a testcase with pass_object_size[0-3].
https://reviews.llvm.org/D41261
Files:
src/cxa_demangle.cpp
test/test_demangle.pass.cpp
Index: test/test_demangle.pass.cpp
===
--- test/test_demangle.pass.cpp
+++ test/test_demangle.pass.cpp
@@ -29611,6 +29611,12 @@
{"_ZN1SB5outer1fB5innerEv", "S[abi:outer]::f[abi:inner]()"},
{"_ZN1SC2B8ctor_tagEv", "S::S[abi:ctor_tag]()"},
{"_ZplB4MERP1SS_", "operator+[abi:MERP](S, S)"},
+// attribute pass_object_size
+{"_Z27NoViableOverloadObjectSize0PvU17pass_object_size0",
"NoViableOverloadObjectSize0(pass_object_size0 void*)"},
+{"_ZN14noninline_virt1AC2EiO1QPvU17pass_object_size0",
"noninline_virt::A::A(int, Q&&, pass_object_size0 void*)"},
+{"_ZZN7lambdas7LambdasEPcENK3$_0clEPvU17pass_object_size0",
"lambdas::Lambdas(char*)::$_0::operator()(pass_object_size0 void*) const"},
+{"_ZN8variadic6AsCtorC1EPKcU17pass_object_size0dz",
"variadic::AsCtor::AsCtor(pass_object_size0 char const*, double, ...)"},
+
{"_Z1fPvU17pass_object_size0S_U17pass_object_size1S_U17pass_object_size2S_U17pass_object_size3",
"f(pass_object_size0 void*, pass_object_size1 void*, pass_object_size2 void*,
pass_object_size3 void*)"},
};
const unsigned N = sizeof(cases) / sizeof(cases[0]);
Index: src/cxa_demangle.cpp
===
--- src/cxa_demangle.cpp
+++ src/cxa_demangle.cpp
@@ -12,6 +12,7 @@
// - enable_if attribute
// - decomposition declarations
// - C++ modules TS
+// - inheriting constructors
#define _LIBCPP_NO_EXCEPTIONS
@@ -3128,11 +3129,33 @@
return first;
}
+// Parse a type in the context of a function parameter. Here, the type can be
+// optionally followed by a pass_object_size attribute. Unlike other vendor
+// extension attributes, clang emits this attribute after the mangling for the
+// type it applies to, so we have to special-case it.
+//
+// ::= U17pass_object_size [0-9] # extension
+const char*
+parse_function_param_type(const char* first, const char* last, Db& db)
+{
+const char* t = parse_type(first, last, db);
+if (t != first && StringView(t, last).startsWith("U17pass_object_size"))
+{
+const char* t2 = t + std::strlen("U17pass_object_size");
+if (t2 != last && *t2 >= '0' && *t2 <= '9' && !db.Names.empty())
+{
+db.Names.back() = db.make(
+db.make(StringView(t+3, t2+1)), db.Names.back());
+t = t2+1;
+}
+}
+return t;
+}
+
// ::= R # & ref-qualifier
// ::= O # && ref-qualifier
-
-// ::= F [Y] [] E
-
+//
+// ::= F [Y] [] E
const char*
parse_function_type(const char* first, const char* last, Db& db)
{
@@ -3186,7 +3209,7 @@
continue;
}
size_t k0 = db.Names.size();
-t1 = parse_type(t, last, db);
+t1 = parse_function_param_type(t, last, db);
size_t k1 = db.Names.size();
if (t1 == t || t1 == last || k1 < k0)
return first;
@@ -4454,7 +4477,7 @@
{
while (true)
{
-const char* t1 = parse_type(t0, last, db);
+const char* t1 = parse_function_param_type(t0, last, db);
if (t1 == t0)
break;
t0 = t1;
@@ -6021,7 +6044,7 @@
size_t params_begin = db.Names.size();
while (true)
{
-t2 = parse_type(t, last, db);
+t2 = parse_function_param_type(t, last, db);
if (t2 == t)
break;
t = t2;
Index: test/test_demangle.pass.cpp
===
--- test/test_demangle.pass.cpp
+++ test/test_demangle.pass.cpp
@@ -29611,6 +29611,12 @@
{"_ZN1SB5outer1fB5innerEv", "S[abi:outer]::f[abi:inner]()"},
{"_ZN1SC2B8ctor_tagEv", "S::S[abi:ctor_tag]()"},
{"_ZplB4MERP1SS_", "operator+[abi:MERP](S, S)"},
+// attribute pass_object_size
+{"_Z27NoViableOverloadObjectSize0PvU17pass_object_size0", "NoViableOverloadObjectSize0(pass_object_size0 void*)"},
+{"_ZN14noninline_virt1AC2EiO1QPvU17pass_object_size0", "noninline_virt::A::A(int, Q&&, pass_object_size0 void*)"},
+{"_ZZN7lambdas7LambdasEPcENK3$_0clEPvU17pass_object_size0", "lambdas::Lambdas(char*)::$_0::operator()(pass_object_size0 void*) const"},
+{"_ZN8variadic6AsCtorC1EPKcU17pass_object_size0dz", "variadic::AsCtor::AsCtor(pass_object_size0 char const*, double, ...)"},
+{"_Z1fPvU17pass_object_size0S_U17pass_object_size1S_U17pass_o
[PATCH] D41885: [libcxxabi][demangler] Improve handling of variadic templates
erik.pilkington created this revision.
erik.pilkington added reviewers: EricWF, mclow.lists, dexonsmith.
This patch fixes some longstanding bugs in the demangler to do with it's
handling of variadic templates.
Currently the demangler handles variadic templates by expanding them into the
Names stack. A parse_* function can succeed and add 0 or more Node*s to the
stack. Each production that can consume a variadic template was responsible for
looping over all the names it parsed, and transforming the stack. Although its
possible for a pack to appear in almost every production, most productions
didn't implement this loop (all but pointer and reference types). The current
demangler also doesn't respect "Dp" and "sp" productions, which correspond to
pack expansions in type and expression contexts. It ignores these pack
expansions, and just eagerly expands the pack in the nearest context where it
would be possible. For example, we misdemangle the following:
template struct unary {};
template void f(unary...) {}
template void f(unary, unary, unary);
// clang mangles to _Z1fIJicfEEvDp5unaryIT_E
The demangler misdemangles this symbol into:
void f(unary)
I think the best way to handle the Dp & sp productions is to parse an
unexpanded AST, then expand it whenever we encounter the Dp/sp. One way of
implementing this might be to do a recursive TreeTransform-like traversal of
the AST, but we would have to do a bunch of allocations for all the expanded
nodes and it would be pretty complex. This patch does the expanding during
printing, by reusing the unexpanded AST. This also automatically gives us
support for PackExpansions everywhere in the grammar. Most of the churn in this
patch has to do with propagating up the the pack size and some other metadata
as the AST is being constructed.
This patch also introduces a nice feature: Every parse_* function now either
fails, or returns exactly 1 Node. I'm going to use this to clean up the parser
a bit.
Thanks for taking a look!
Erik
Repository:
rCXXA libc++abi
https://reviews.llvm.org/D41885
Files:
src/cxa_demangle.cpp
test/test_demangle.pass.cpp
test/unittest_demangle.pass.cpp
Index: test/unittest_demangle.pass.cpp
===
--- test/unittest_demangle.pass.cpp
+++ test/unittest_demangle.pass.cpp
@@ -82,44 +82,6 @@
}
}
-void testSubstitutionTable() {
- {
-SubstitutionTable<2> Tab;
-
-NameType Names[] = {{"MERP"}, {"MARP"}, {"MAMP"}};
-Tab.pushPack();
-Tab.pushSubstitutionIntoPack(&Names[0]);
-Tab.pushSubstitutionIntoPack(&Names[1]);
-Tab.pushSubstitutionIntoPack(&Names[2]);
-
-int Index = 0;
-for (Node* N : Tab.nthSubstitution(0)) {
- assert(static_cast(N)->getName() == Names[Index].getName());
- ++Index;
-}
-assert(Index == 3);
-
-Tab.popPack();
-assert(Tab.empty() && Tab.size() == 0);
-Tab.pushSubstitution(&Names[0]);
-Tab.pushSubstitution(&Names[1]);
-assert(!Tab.empty() && Tab.size() == 2);
-
-int I = 0;
-for (Node* N : Tab.nthSubstitution(0)) {
- assert(static_cast(N)->getName() == "MERP");
- assert(I == 0);
- ++I;
-}
-for (Node* N : Tab.nthSubstitution(1)) {
- assert(static_cast(N)->getName() == "MARP");
- assert(I == 1);
- ++I;
-}
- }
-}
-
int main() {
testPODSmallVector();
- testSubstitutionTable();
}
Index: test/test_demangle.pass.cpp
===
--- test/test_demangle.pass.cpp
+++ test/test_demangle.pass.cpp
@@ -29585,7 +29585,7 @@
{"_Z1fPU11objcproto1A11objc_object", "f(id)"},
{"_Z1fPKU11objcproto1A7NSArray", "f(NSArray const*)"},
{"_ZNK1AIJ1Z1Y1XEEcv1BIJDpPT_EEIJS2_S1_S0_EEEv", "A::operator B() const"},
-{"_ZNK3Ncr6Silver7Utility6detail12CallOnThreadIZ53-[DeploymentSetupController handleManualServerEntry:]E3$_5EclIJEEEDTclclL_ZNS2_4getTIS4_EERT_vEEspclsr3stdE7forwardIT_Efp_EEEDpOSA_", "decltype(-[DeploymentSetupController handleManualServerEntry:]::$_5& Ncr::Silver::Utility::detail::getT<-[DeploymentSetupController handleManualServerEntry:]::$_5>()()(std::forward<-[DeploymentSetupController handleManualServerEntry:]::$_5>(fp))) Ncr::Silver::Utility::detail::CallOnThread<-[DeploymentSetupController handleManualServerEntry:]::$_5>::operator()<>(-[DeploymentSetupController handleManualServerEntry:]::$_5&&) const"},
+{"_ZNK3Ncr6Silver7Utility6detail12CallOnThreadIZ53-[DeploymentSetupController handleManualServerEntry:]E3$_5EclIJEEEDTclclL_ZNS2_4getTIS4_EERT_vEEspclsr3stdE7forwardIT_Efp_EEEDpOSA_", "decltype(-[DeploymentSetupController handleManualServerEntry:]::$_5& Ncr::Silver::Utility::detail::getT<-[DeploymentSetupController handleManualServerEntry:]::$_5>()()(std::forward<-[DeploymentSetupController handleManualServerEntry:]::$_5>(fp)...)) Ncr::Silver::Utility::detail::CallOnThread<-[DeploymentSetupController handleManualServerEntry:]::$_5>::operato
[PATCH] D41885: [libcxxabi][demangler] Improve handling of variadic templates
erik.pilkington added inline comments.
Comment at: src/cxa_demangle.cpp:130
- // Offset of position in buffer, used for building stream_string_view.
- typedef unsigned StreamPosition;
-
This caching API is being removed because it is no longer valid: if a node has
an unexpanded parameter pack it can generate multiple different strings.
Comment at: src/cxa_demangle.cpp:1599
-template
-class SubstitutionTable {
- // Substitutions hold the actual entries in the table, and PackIndices tells
This type was an optimization that flattened the previously 2d vector of
substitutions (index * pack) into two 1d vectors. Now that packs are
represented with a single Node*, we can replace it with a simple vector.
Comment at: test/test_demangle.pass.cpp:29607
{"_ZTHN3fooE", "thread-local initialization routine for foo"},
-{"_Z4algoIJiiiEEvZ1gEUlT_E_", "void algo(g::'lambda'(int,
int, int))"},
+{"_Z4algoIJiiiEEvZ1gEUlDpT_E_", "void algo(g::'lambda'(int,
int, int))"},
// attribute abi_tag
I just generated this symbol by hand because I couldn't get clang to do it
without crashing. Turns out I forgot the Dp!
Repository:
rCXXA libc++abi
https://reviews.llvm.org/D41885
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41887: [libcxxabi][demangler] Clean up and llvm-ify the expression parser
erik.pilkington created this revision.
erik.pilkington added reviewers: EricWF, mclow.lists, dexonsmith.
As of https://reviews.llvm.org/D41885, every parse_* function now either
returns a single Node* or fails. I'm using this new rule to clean up the
parsing for the demangler, and also update it to use LLVM coding conventions.
This patch updates the expression parser. The expression grammer is pretty
simple, so this patch is a pretty mechanical transformation.
In the new parser:
- The position in the mangled name is held in Db, and is accessed by Db::look()
and Db::consume() functions
- Parse functions now return a Node* directly instead of pushing a node onto
the Name stack and having their caller immediately pop it off.
- Failed to parse mean the parse functions returns nullptr.
- LLVM style.
Thanks for taking a look!
Erik
Repository:
rCXXA libc++abi
https://reviews.llvm.org/D41887
Files:
src/cxa_demangle.cpp
Index: src/cxa_demangle.cpp
===
--- src/cxa_demangle.cpp
+++ src/cxa_demangle.cpp
@@ -2062,65 +2062,850 @@
struct Db
{
-// Name stack, this is used by the parser to hold temporary names that were
-// parsed. The parser colapses multiple names into new nodes to construct
-// the AST. Once the parser is finished, names.size() == 1.
-PODSmallVector Names;
-
-// Substitution table. Itanium supports name substitutions as a means of
-// compression. The string "S42_" refers to the 44nd entry (base-36) in this
-// table.
-PODSmallVector Subs;
-
-// Template parameter table. Like the above, but referenced like "T42_".
-// This has a smaller size compared to Subs and Names because it can be
-// stored on the stack.
-PODSmallVector TemplateParams;
-
-Qualifiers CV = QualNone;
-FunctionRefQual RefQuals = FrefQualNone;
-unsigned EncodingDepth = 0;
-bool ParsedCtorDtorCV = false;
-bool TagTemplates = true;
-bool FixForwardReferences = false;
-bool TryToParseTemplateArgs = true;
-
-BumpPointerAllocator ASTAllocator;
-
-template T* make(Args&& ...args)
-{
-return new (ASTAllocator.allocate(sizeof(T)))
-T(std::forward(args)...);
-}
+ const char *First;
+ const char *Last;
-template NodeArray makeNodeArray(It begin, It end)
-{
-size_t sz = static_cast(end - begin);
-void* mem = ASTAllocator.allocate(sizeof(Node*) * sz);
-Node** data = new (mem) Node*[sz];
-std::copy(begin, end, data);
-return NodeArray(data, sz);
+ // Name stack, this is used by the parser to hold temporary names that were
+ // parsed. The parser colapses multiple names into new nodes to construct
+ // the AST. Once the parser is finished, names.size() == 1.
+ PODSmallVector Names;
+
+ // Substitution table. Itanium supports name substitutions as a means of
+ // compression. The string "S42_" refers to the 44nd entry (base-36) in this
+ // table.
+ PODSmallVector Subs;
+
+ // Template parameter table. Like the above, but referenced like "T42_".
+ // This has a smaller size compared to Subs and Names because it can be
+ // stored on the stack.
+ PODSmallVector TemplateParams;
+
+ Qualifiers CV = QualNone;
+ FunctionRefQual RefQuals = FrefQualNone;
+ unsigned EncodingDepth = 0;
+ bool ParsedCtorDtorCV = false;
+ bool TagTemplates = true;
+ bool FixForwardReferences = false;
+ bool TryToParseTemplateArgs = true;
+
+ BumpPointerAllocator ASTAllocator;
+
+ template T* make(Args&& ...args)
+ {
+ return new (ASTAllocator.allocate(sizeof(T)))
+ T(std::forward(args)...);
+ }
+
+ template NodeArray makeNodeArray(It begin, It end)
+ {
+ size_t sz = static_cast(end - begin);
+ void* mem = ASTAllocator.allocate(sizeof(Node*) * sz);
+ Node** data = new (mem) Node*[sz];
+ std::copy(begin, end, data);
+ return NodeArray(data, sz);
+ }
+
+ NodeArray popTrailingNodeArray(size_t FromPosition)
+ {
+ assert(FromPosition <= Names.size());
+ NodeArray res = makeNodeArray(
+ Names.begin() + (long)FromPosition, Names.end());
+ Names.dropBack(FromPosition);
+ return res;
+ }
+
+ bool consumeIf(StringView S) {
+if (StringView(First, Last).startsWith(S)) {
+ First += S.size();
+ return true;
}
+return false;
+ }
-NodeArray popTrailingNodeArray(size_t FromPosition)
-{
-assert(FromPosition <= Names.size());
-NodeArray res = makeNodeArray(
-Names.begin() + (long)FromPosition, Names.end());
-Names.dropBack(FromPosition);
-return res;
+ bool consumeIf(char C) {
+if (First != Last && *First == C) {
+ ++First;
+ return true;
}
+return false;
+ }
+
+ char consume() { return First != Last ? *First++ : '\0'; }
+
+ char look(unsigned Lookahead = 0) {
+if (static_cast(Last - First) <= Lookahead)
+ return '\0';
+return First[Lo
[PATCH] D41889: [libcxxabi][demangler] Clean up and llvm-ify the type parser
erik.pilkington created this revision.
erik.pilkington added reviewers: EricWF, mclow.lists, dexonsmith.
As of https://reviews.llvm.org/D41885, every parse_* function now either
returns a single Node* or fails. I'm using this new rule to clean up the
parsing for the demangler, and also update it to use LLVM coding conventions.
This patch updates the type parser. This patch also fixes a bug I noticed where
multiple qualifiers on a type get distinct entries in the substitution table.
Thanks!
Erik
Repository:
rCXXA libc++abi
https://reviews.llvm.org/D41889
Files:
src/cxa_demangle.cpp
test/test_demangle.pass.cpp
Index: test/test_demangle.pass.cpp
===
--- test/test_demangle.pass.cpp
+++ test/test_demangle.pass.cpp
@@ -29617,6 +29617,9 @@
{"_Z1fIJicEEvDp7MuncherIAstT__S1_E", "void f(Muncher, Muncher)"},
{"_ZN1SIJifcEE1fIJdjEEEiDp4MerpIJifcT_EE", "int S::f(Merp, Merp)"},
{"_Z1pIJicEEiDp4MerpIXsZT_EJT_EE", "int p(Merp, Merp)"},
+
+// Multiple qualifiers on the same type should all get the same entry in the substitution table.
+{"_Z1fPU3AS1KiS0_", "f(int const AS1*, int const AS1*)"}
};
const unsigned N = sizeof(cases) / sizeof(cases[0]);
Index: src/cxa_demangle.cpp
===
--- src/cxa_demangle.cpp
+++ src/cxa_demangle.cpp
@@ -323,6 +323,23 @@
}
};
+class NameType final : public Node {
+ const StringView Name;
+
+public:
+ NameType(StringView Name_) : Node(KNameType), Name(Name_) {
+setCachedRHSComponent(false);
+setCachedArray(false);
+setCachedFunction(false);
+ParameterPackSize = NoParameterPack;
+ }
+
+ StringView getName() const { return Name; }
+ StringView getBaseName() const override { return Name; }
+
+ void printLeft(OutputStream &s) const override { s += Name; }
+};
+
class VendorExtQualType final : public Node {
const Node *Ty;
const Node *Ext;
@@ -339,11 +356,18 @@
setCachedFunction(Ty->CachedFunction);
}
+ bool isObjCObject() const {
+return Ext->getKind() == KObjCProtoName &&
+ Ty->getKind() == KNameType &&
+ static_cast(Ty)->getName() == "objc_object";
+ }
+
const Node* getQual() const { return Ext; }
void printLeft(OutputStream &S) const override {
Ty->print(S);
-S += " ";
+if (Ext->getKind() != KObjCProtoName)
+ S += " ";
Ext->printLeft(S);
}
};
@@ -442,23 +466,6 @@
void printRight(OutputStream &S) const override { Ty->printRight(S); }
};
-class NameType final : public Node {
- const StringView Name;
-
-public:
- NameType(StringView Name_) : Node(KNameType), Name(Name_) {
-setCachedRHSComponent(false);
-setCachedArray(false);
-setCachedFunction(false);
-ParameterPackSize = NoParameterPack;
- }
-
- StringView getName() const { return Name; }
- StringView getBaseName() const override { return Name; }
-
- void printLeft(OutputStream &s) const override { s += Name; }
-};
-
class AbiTagAttr final : public Node {
const Node* Base;
StringView Tag;
@@ -482,29 +489,22 @@
};
class ObjCProtoName : public Node {
- Node *Ty;
- Node *Protocol;
+ StringView Protocol;
friend class PointerType;
public:
- ObjCProtoName(Node *Ty_, Node *Protocol_)
- : Node(KObjCProtoName), Ty(Ty_), Protocol(Protocol_) {
+ ObjCProtoName(StringView Protocol_)
+ : Node(KObjCProtoName), Protocol(Protocol_) {
setCachedRHSComponent(false);
setCachedArray(false);
setCachedFunction(false);
ParameterPackSize = NoParameterPack;
}
- bool isObjCObject() const {
-return Ty->getKind() == KNameType &&
- static_cast(Ty)->getName() == "objc_object";
- }
-
void printLeft(OutputStream &S) const override {
-Ty->print(S);
S += "<";
-Protocol->print(S);
+S += Protocol;
S += ">";
}
};
@@ -525,30 +525,29 @@
return Pointee->hasRHSComponent(S);
}
- void printLeft(OutputStream &s) const override {
+ void printLeft(OutputStream &S) const override {
// We rewrite objc_object* into id.
-if (Pointee->getKind() != KObjCProtoName ||
-!static_cast(Pointee)->isObjCObject()) {
- Pointee->printLeft(s);
- if (Pointee->hasArray(s))
-s += " ";
- if (Pointee->hasArray(s) || Pointee->hasFunction(s))
-s += "(";
- s += "*";
+if (Pointee->getKind() == KVendorExtQualType &&
+static_cast(Pointee)->isObjCObject()) {
+ const auto *ExtQual = static_cast(Pointee);
+ S += "id";
+ ExtQual->getQual()->print(S);
} else {
- const auto *objcProto = static_cast(Pointee);
- s += "id<";
- objcProto->Protocol->print(s);
- s += ">";
+ Pointee->printLeft(S);
+ if (Pointee->hasArray(S))
+S += " ";
+ if (Pointee->hasArray(S) || Pointee->hasFunction(S))
+S += "(";
+ S += "*";
}
}
- void printRight(OutputStream &s) const override {
-if (Pointe
[PATCH] D47607: [libcxx] Almost fix some UB in and
erik.pilkington created this revision.
erik.pilkington added reviewers: rsmith, EricWF, mclow.lists.
Herald added subscribers: christof, kosarev.
and define `__value_type` as a union between pair
and pair so that various operations can move into/from these pairs
[1]. This is a pretty blatant strict aliasing violation, and has the potential
break code that was optimized by TBAA. We can't remove this, not only would
that be a significant pessimization, but we are also required to provide a
non-const reference to the key_type to implement splicing maps & sets. This
patch fixes `__value_type` by downgrading it from "accidentally launch the
missiles"-level UB to theoretical UB, namely, instead of type-punning between
pairs we just const_cast the key. It's still undefined to mutate a const
object, but clang doesn't actually optimize on this rule. Just to be paranoid
this patch also `launder()`s to "pick-up" the new value of key whenever we read
from it, though this isn't launder's actual use-case. Thanks to @rsmith for the
suggestions!
To do this, this patch:
- removes the __nc_value_type data member of the `__value_type` union
- provides access to the pair with `pair`, which is
used to assign through to the pair
- changes `__value_type.__cc` to `__value_type.__get_value()`, the latter calls
std::launder in C++1z mode.
https://reviews.llvm.org/D46845 depends on this patch.
[1]: https://stackoverflow.com/a/31667355
Thanks for taking a look!
Erik
Repository:
rCXX libc++
https://reviews.llvm.org/D47607
Files:
libcxx/include/__hash_table
libcxx/include/__tree
libcxx/include/map
libcxx/include/unordered_map
Index: libcxx/include/unordered_map
===
--- libcxx/include/unordered_map
+++ libcxx/include/unordered_map
@@ -396,7 +396,7 @@
const _Hash& hash_function() const _NOEXCEPT {return *this;}
_LIBCPP_INLINE_VISIBILITY
size_t operator()(const _Cp& __x) const
-{return static_cast(*this)(__x.__cc.first);}
+{return static_cast(*this)(__x.__get_value().first);}
_LIBCPP_INLINE_VISIBILITY
size_t operator()(const _Key& __x) const
{return static_cast(*this)(__x);}
@@ -425,7 +425,7 @@
const _Hash& hash_function() const _NOEXCEPT {return __hash_;}
_LIBCPP_INLINE_VISIBILITY
size_t operator()(const _Cp& __x) const
-{return __hash_(__x.__cc.first);}
+{return __hash_(__x.__get_value().first);}
_LIBCPP_INLINE_VISIBILITY
size_t operator()(const _Key& __x) const
{return __hash_(__x);}
@@ -464,13 +464,13 @@
const _Pred& key_eq() const _NOEXCEPT {return *this;}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Cp& __x, const _Cp& __y) const
-{return static_cast(*this)(__x.__cc.first, __y.__cc.first);}
+{return static_cast(*this)(__x.__get_value().first, __y.__get_value().first);}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Cp& __x, const _Key& __y) const
-{return static_cast(*this)(__x.__cc.first, __y);}
+{return static_cast(*this)(__x.__get_value().first, __y);}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Key& __x, const _Cp& __y) const
-{return static_cast(*this)(__x, __y.__cc.first);}
+{return static_cast(*this)(__x, __y.__get_value().first);}
void swap(__unordered_map_equal&__y)
_NOEXCEPT_(__is_nothrow_swappable<_Pred>::value)
{
@@ -496,13 +496,13 @@
const _Pred& key_eq() const _NOEXCEPT {return __pred_;}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Cp& __x, const _Cp& __y) const
-{return __pred_(__x.__cc.first, __y.__cc.first);}
+{return __pred_(__x.__get_value().first, __y.__get_value().first);}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Cp& __x, const _Key& __y) const
-{return __pred_(__x.__cc.first, __y);}
+{return __pred_(__x.__get_value().first, __y);}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Key& __x, const _Cp& __y) const
-{return __pred_(__x, __y.__cc.first);}
+{return __pred_(__x, __y.__get_value().first);}
void swap(__unordered_map_equal&__y)
_NOEXCEPT_(__is_nothrow_swappable<_Pred>::value)
{
@@ -572,9 +572,9 @@
void operator()(pointer __p) _NOEXCEPT
{
if (__second_constructed)
-__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__cc.second));
+__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__get_value().second));
if (__first_constructed)
-__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__cc.first));
+__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__get_value().first));
if (__p)
__alloc_traits::deallocate(__na_, __p, 1);
}
@@ -587,27 +587,69 @@
typedef _Key key_type;
typedef _Tp
[PATCH] D47607: [libcxx] Almost fix some UB in and
erik.pilkington updated this revision to Diff 149449.
erik.pilkington marked 10 inline comments as done.
erik.pilkington added a comment.
Address review comments. Thanks!
https://reviews.llvm.org/D47607
Files:
libcxx/include/__hash_table
libcxx/include/__tree
libcxx/include/map
libcxx/include/unordered_map
Index: libcxx/include/unordered_map
===
--- libcxx/include/unordered_map
+++ libcxx/include/unordered_map
@@ -396,7 +396,7 @@
const _Hash& hash_function() const _NOEXCEPT {return *this;}
_LIBCPP_INLINE_VISIBILITY
size_t operator()(const _Cp& __x) const
-{return static_cast(*this)(__x.__cc.first);}
+{return static_cast(*this)(__x.__get_value().first);}
_LIBCPP_INLINE_VISIBILITY
size_t operator()(const _Key& __x) const
{return static_cast(*this)(__x);}
@@ -425,7 +425,7 @@
const _Hash& hash_function() const _NOEXCEPT {return __hash_;}
_LIBCPP_INLINE_VISIBILITY
size_t operator()(const _Cp& __x) const
-{return __hash_(__x.__cc.first);}
+{return __hash_(__x.__get_value().first);}
_LIBCPP_INLINE_VISIBILITY
size_t operator()(const _Key& __x) const
{return __hash_(__x);}
@@ -464,13 +464,13 @@
const _Pred& key_eq() const _NOEXCEPT {return *this;}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Cp& __x, const _Cp& __y) const
-{return static_cast(*this)(__x.__cc.first, __y.__cc.first);}
+{return static_cast(*this)(__x.__get_value().first, __y.__get_value().first);}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Cp& __x, const _Key& __y) const
-{return static_cast(*this)(__x.__cc.first, __y);}
+{return static_cast(*this)(__x.__get_value().first, __y);}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Key& __x, const _Cp& __y) const
-{return static_cast(*this)(__x, __y.__cc.first);}
+{return static_cast(*this)(__x, __y.__get_value().first);}
void swap(__unordered_map_equal&__y)
_NOEXCEPT_(__is_nothrow_swappable<_Pred>::value)
{
@@ -496,13 +496,13 @@
const _Pred& key_eq() const _NOEXCEPT {return __pred_;}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Cp& __x, const _Cp& __y) const
-{return __pred_(__x.__cc.first, __y.__cc.first);}
+{return __pred_(__x.__get_value().first, __y.__get_value().first);}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Cp& __x, const _Key& __y) const
-{return __pred_(__x.__cc.first, __y);}
+{return __pred_(__x.__get_value().first, __y);}
_LIBCPP_INLINE_VISIBILITY
bool operator()(const _Key& __x, const _Cp& __y) const
-{return __pred_(__x, __y.__cc.first);}
+{return __pred_(__x, __y.__get_value().first);}
void swap(__unordered_map_equal&__y)
_NOEXCEPT_(__is_nothrow_swappable<_Pred>::value)
{
@@ -572,42 +572,88 @@
void operator()(pointer __p) _NOEXCEPT
{
if (__second_constructed)
-__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__cc.second));
+__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__get_value().second));
if (__first_constructed)
-__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__cc.first));
+__alloc_traits::destroy(__na_, _VSTD::addressof(__p->__value_.__get_value().first));
if (__p)
__alloc_traits::deallocate(__na_, __p, 1);
}
};
#ifndef _LIBCPP_CXX03_LANG
template
-union __hash_value_type
+struct __hash_value_type
{
typedef _Key key_type;
typedef _Tp mapped_type;
typedef pairvalue_type;
-typedef pair __nc_value_type;
+typedef pair__nc_ref_pair_type;
+typedef pair __nc_rref_pair_type;
+private:
value_type __cc;
-__nc_value_type __nc;
+
+public:
+_LIBCPP_INLINE_VISIBILITY
+value_type& __get_value()
+{
+#if _LIBCPP_STD_VER > 14
+return *_VSTD::launder(_VSTD::addressof(__cc));
+#else
+return __cc;
+#endif
+}
+
+_LIBCPP_INLINE_VISIBILITY
+const value_type& __get_value() const
+{
+#if _LIBCPP_STD_VER > 14
+return *_VSTD::launder(_VSTD::addressof(__cc));
+#else
+return __cc;
+#endif
+}
+
+_LIBCPP_INLINE_VISIBILITY
+__nc_ref_pair_type __ref()
+{
+value_type& __v = __get_value();
+return __nc_ref_pair_type(const_cast(__v.first), __v.second);
+}
+
+_LIBCPP_INLINE_VISIBILITY
+__nc_rref_pair_type __move()
+{
+value_type& __v = __get_value();
+return __nc_rref_pair_type(
+_VSTD::move(const_cast(__v.first)),
+_VSTD::move(__v.second));
+}
_LIBCPP_INLINE_VISIBILITY
__hash_value_type& operator=(const __hash_value_type& __v)
-{__nc = __v.__cc; return *
[PATCH] D47607: [libcxx] Almost fix some UB in and
erik.pilkington added a comment.
In https://reviews.llvm.org/D47607#1118547, @EricWF wrote:
> I should have asked, have we actually seen a midcompile caused by this? Is
> there a reproducer? Or is this purerly speculative?
Nope, pure speculation. I still think we should still fix this though.
Comment at: libcxx/include/map:648
+_LIBCPP_INLINE_VISIBILITY
+__nc_ref_pair_type __ref()
+{
EricWF wrote:
> I think `__ref` can be private.
That's true for this patch, but I'm planning on using `__ref` in
`__node_handle` to implement key() and mapped() so we don't have to duplicate
the const cast.
https://reviews.llvm.org/D47607
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets
erik.pilkington added inline comments. Comment at: libcxx/include/__hash_table:2261 +_NodeHandle +__hash_table<_Tp, _Hash, _Equal, _Alloc>::__node_handle_extract_unique( +key_type const& __key) EricWF wrote: > If I'm not mistaken, `__node_handle_extract_unique` and > `__node_handle_extract_multi` have the exact same implementation. This is > intentional, no? If so can't we just use one implementation? Yep, good point. The new patch just has `__node_handle_extract`. https://reviews.llvm.org/D46845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47887: Move VersionTuple from clang/Basic to llvm/Support, llvm part
erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land. LGTM, thanks for adding the test! Repository: rL LLVM https://reviews.llvm.org/D47887 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47886: Move VersionTuple from clang/Basic to llvm/Support, clang part
erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land. LGTM. Looks like LLDB has some uses of VersionTuple, you should fix those to #include the LLVM version before removing the header here. (Or, better yet, do it all atomically with the monorepo as @zturner suggested) Repository: rC Clang https://reviews.llvm.org/D47886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets
erik.pilkington added a comment. Ping! https://reviews.llvm.org/D46845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48322: [Sema] Discarded statment should be an evaluatable context
erik.pilkington created this revision.
erik.pilkington added a reviewer: rsmith.
The constexpr evaluator was erroring out because these templates weren't
defined. Despite being used in a discarded statement, we still need to
constexpr evaluate them, which means that we need to instantiate them.
Fixes https://llvm.org/PR37585
Thanks!
Repository:
rC Clang
https://reviews.llvm.org/D48322
Files:
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/constant-expression-cxx1z.cpp
Index: clang/test/SemaCXX/constant-expression-cxx1z.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1z.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1z.cpp
@@ -46,3 +46,16 @@
const int &r = 0;
constexpr int n = r;
}
+
+namespace PR37585 {
+template struct S { static constexpr bool value = true; };
+template constexpr bool f() { return true; }
+template constexpr bool v = true;
+
+void test() {
+ if constexpr (true) {}
+ else if constexpr (f()) {}
+ else if constexpr (S::value) {}
+ else if constexpr (v) {}
+}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14143,13 +14143,13 @@
switch (SemaRef.ExprEvalContexts.back().Context) {
case Sema::ExpressionEvaluationContext::Unevaluated:
case Sema::ExpressionEvaluationContext::UnevaluatedAbstract:
-case Sema::ExpressionEvaluationContext::DiscardedStatement:
// Expressions in this context are never evaluated.
return false;
case Sema::ExpressionEvaluationContext::UnevaluatedList:
case Sema::ExpressionEvaluationContext::ConstantEvaluated:
case Sema::ExpressionEvaluationContext::PotentiallyEvaluated:
+case Sema::ExpressionEvaluationContext::DiscardedStatement:
// Expressions in this context could be evaluated.
return true;
Index: clang/test/SemaCXX/constant-expression-cxx1z.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1z.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1z.cpp
@@ -46,3 +46,16 @@
const int &r = 0;
constexpr int n = r;
}
+
+namespace PR37585 {
+template struct S { static constexpr bool value = true; };
+template constexpr bool f() { return true; }
+template constexpr bool v = true;
+
+void test() {
+ if constexpr (true) {}
+ else if constexpr (f()) {}
+ else if constexpr (S::value) {}
+ else if constexpr (v) {}
+}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14143,13 +14143,13 @@
switch (SemaRef.ExprEvalContexts.back().Context) {
case Sema::ExpressionEvaluationContext::Unevaluated:
case Sema::ExpressionEvaluationContext::UnevaluatedAbstract:
-case Sema::ExpressionEvaluationContext::DiscardedStatement:
// Expressions in this context are never evaluated.
return false;
case Sema::ExpressionEvaluationContext::UnevaluatedList:
case Sema::ExpressionEvaluationContext::ConstantEvaluated:
case Sema::ExpressionEvaluationContext::PotentiallyEvaluated:
+case Sema::ExpressionEvaluationContext::DiscardedStatement:
// Expressions in this context could be evaluated.
return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53046: [Sema] Fix an error-on-valid with friends and templates
erik.pilkington created this revision.
erik.pilkington added reviewers: rsmith, rjmccall.
Herald added a subscriber: dexonsmith.
Clang used to error out on the attached testcase, due to multiple definitions
of `foo`. The problem is that multiple FunctionTemplateDecl::Common
pointers are created for the two FunctionTemplateDeclarations `foo` in the
redeclaration chain, each with their own copy of the instantiation of `f`.
This patch fixes the problem by using the previous function template in a call
to `FuntionTemplateDecl::getInjectedTemplateArgs()` (this was the call that
caused the Common pointer to be allocated in the redeclaration). Most of this
patch is just boilerplate to get the function template declaration down the
stack to where it's needed.
Fixes rdar://44810129
Thanks for taking a look!
Erik
Repository:
rC Clang
https://reviews.llvm.org/D53046
Files:
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExceptionSpec.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/SemaCXX/friend-template-redecl.cpp
Index: clang/test/SemaCXX/friend-template-redecl.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/friend-template-redecl.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -std=c++17 -verify -emit-llvm-only %s
+
+// expected-no-diagnostics
+
+template void bar(const T &t) { foo(t); }
+
+template
+struct HasFriend {
+ template
+ friend void foo(const HasFriend &m) noexcept(false);
+};
+
+template
+void foo(const HasFriend &m) noexcept(false) {}
+
+void f() {
+ HasFriend x;
+ foo(x);
+ bar(x);
+}
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3575,7 +3575,8 @@
}
void Sema::InstantiateExceptionSpec(SourceLocation PointOfInstantiation,
-FunctionDecl *Decl) {
+FunctionDecl *Decl,
+FunctionDecl *OldDecl) {
const FunctionProtoType *Proto = Decl->getType()->castAs();
if (Proto->getExceptionSpecType() != EST_Uninstantiated)
return;
@@ -3601,8 +3602,9 @@
Sema::ContextRAII savedContext(*this, Decl);
LocalInstantiationScope Scope(*this);
- MultiLevelTemplateArgumentList TemplateArgs =
-getTemplateInstantiationArgs(Decl, nullptr, /*RelativeToPrimary*/true);
+ MultiLevelTemplateArgumentList TemplateArgs = getTemplateInstantiationArgs(
+ Decl, nullptr, /*RelativeToPrimary*/ true, nullptr,
+ !OldDecl ? nullptr : OldDecl->getDescribedFunctionTemplate());
FunctionDecl *Template = Proto->getExceptionSpecTemplate();
if (addInstantiatedParametersToScope(*this, Decl, Template, Scope,
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -51,11 +51,16 @@
/// instantiating the definition of the given declaration, \p D. This is
/// used to determine the proper set of template instantiation arguments for
/// friend function template specializations.
+///
+/// \param PrevFunTmpl If non-NULL, D is a pattern declaration of a function
+/// template that hasn't yet been attached to the redeclaration chain of
+/// PrevFunTmpl's pattern.
MultiLevelTemplateArgumentList
Sema::getTemplateInstantiationArgs(NamedDecl *D,
const TemplateArgumentList *Innermost,
bool RelativeToPrimary,
- const FunctionDecl *Pattern) {
+ const FunctionDecl *Pattern,
+ FunctionTemplateDecl *PrevFunTmpl) {
// Accumulate the set of template argument lists in this structure.
MultiLevelTemplateArgumentList Result;
@@ -151,6 +156,16 @@
} else if (FunctionTemplateDecl *FunTmpl
= Function->getDescribedFunctionTemplate()) {
+if (PrevFunTmpl) {
+ // FunTmpl hasn't been linked into the same redecl chain as
+ // PrevFunTmpl yet, so we can't retreive it's injected template
+ // arguments just yet; use PrevFunTmpl's instead.
+ //
+ // Note that we don't need to verify that FunTmpl is the function
+ // template that is a redeclaration of PrevFunTmpl because only one
+ // function template decl can appear in this search.
+ FunTmpl = PrevFunTmpl;
+}
// Add the "injected" template arguments.
Result.addOuterTemplateArguments(FunTmpl->getInjectedTemplateArgs());
}
Index: clang/lib/Sema/SemaExceptionSpec.cpp
===
--- clang/
[PATCH] D53046: [Sema] Fix an error-on-valid with friends and templates
erik.pilkington added inline comments.
Comment at: clang/lib/Sema/SemaDecl.cpp:10015
// merged.
if (MergeFunctionDecl(NewFD, OldDecl, S, MergeTypeWithPrevious)) {
NewFD->setInvalidDecl();
The problem is here, MergeFunctionDecl() needs the injected template args,
but...
Comment at: clang/lib/Sema/SemaDecl.cpp:10026
auto *OldFD = OldTemplateDecl->getTemplatedDecl();
NewFD->setPreviousDeclaration(OldFD);
adjustDeclContextForDeclaratorDecl(NewFD, OldFD);
... we don't link the two declarations until here!
Repository:
rC Clang
https://reviews.llvm.org/D53046
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53048: [Basic] Split out -Wimplicit-int-conversion and -Wimplicit-float-conversion from -Wconversion
erik.pilkington created this revision.
erik.pilkington added reviewers: rsmith, aaron.ballman.
Herald added a subscriber: dexonsmith.
These two diagnostics are noisy, so its reasonable for users to opt-out of them
when -Wconversion is enabled.
Fixes rdar://45058981
Thanks for taking a look!
Repository:
rC Clang
https://reviews.llvm.org/D53048
Files:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/test/Sema/implicit-int-conversion.c
Index: clang/test/Sema/implicit-int-conversion.c
===
--- /dev/null
+++ clang/test/Sema/implicit-int-conversion.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -Wconversion -Wno-implicit-int-conversion -DSMALL=char
-DBIG=int -DNO_DIAG
+// RUN: %clang_cc1 -Wno-conversion -Wimplicit-int-conversion -DSMALL=char
-DBIG=int
+// RUN: %clang_cc1 -Wconversion -Wno-implicit-float-conversion -DSMALL=float
-DBIG=double -DNO_DIAG
+// RUN: %clang_cc1 -Wno-conversion -Wimplicit-float-conversion -DSMALL=float
-DBIG=double
+
+void f() {
+ SMALL a;
+ BIG b = 0;
+ a = b;
+#ifndef NO_DIAG
+ // expected-warning@-2 {{implicit conversion}}
+#else
+ // expected-no-diagnostics
+#endif
+}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3183,10 +3183,10 @@
"implicit conversion from %0 to %1 is not permitted in C++">;
def warn_impcast_float_precision : Warning<
"implicit conversion loses floating-point precision: %0 to %1">,
- InGroup, DefaultIgnore;
+ InGroup, DefaultIgnore;
def warn_impcast_float_result_precision : Warning<
"implicit conversion when assigning computation result loses floating-point
precision: %0 to %1">,
- InGroup, DefaultIgnore;
+ InGroup, DefaultIgnore;
def warn_impcast_double_promotion : Warning<
"implicit conversion increases floating-point precision: %0 to %1">,
InGroup, DefaultIgnore;
@@ -3198,10 +3198,10 @@
InGroup, DefaultIgnore;
def warn_impcast_integer_precision : Warning<
"implicit conversion loses integer precision: %0 to %1">,
- InGroup, DefaultIgnore;
+ InGroup, DefaultIgnore;
def warn_impcast_high_order_zero_bits : Warning<
"higher order bits are zeroes after implicit conversion">,
- InGroup, DefaultIgnore;
+ InGroup, DefaultIgnore;
def warn_impcast_nonnegative_result : Warning<
"the resulting value is always non-negative after implicit conversion">,
InGroup, DefaultIgnore;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -59,6 +59,8 @@
UndefinedBoolConversion]>;
def IntConversion : DiagGroup<"int-conversion">;
def EnumConversion : DiagGroup<"enum-conversion">;
+def ImplicitIntConversion : DiagGroup<"implicit-int-conversion">;
+def ImplicitFloatConversion : DiagGroup<"implicit-float-conversion">;
def FloatOverflowConversion : DiagGroup<"float-overflow-conversion">;
def FloatZeroConversion : DiagGroup<"float-zero-conversion">;
@@ -709,6 +711,8 @@
FloatConversion,
Shorten64To32,
IntConversion,
+ImplicitIntConversion,
+ImplicitFloatConversion,
LiteralConversion,
NonLiteralNullConversion, // (1-1)->pointer (etc)
NullConversion, // NULL->non-pointer
Index: clang/test/Sema/implicit-int-conversion.c
===
--- /dev/null
+++ clang/test/Sema/implicit-int-conversion.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -Wconversion -Wno-implicit-int-conversion -DSMALL=char -DBIG=int -DNO_DIAG
+// RUN: %clang_cc1 -Wno-conversion -Wimplicit-int-conversion -DSMALL=char -DBIG=int
+// RUN: %clang_cc1 -Wconversion -Wno-implicit-float-conversion -DSMALL=float -DBIG=double -DNO_DIAG
+// RUN: %clang_cc1 -Wno-conversion -Wimplicit-float-conversion -DSMALL=float -DBIG=double
+
+void f() {
+ SMALL a;
+ BIG b = 0;
+ a = b;
+#ifndef NO_DIAG
+ // expected-warning@-2 {{implicit conversion}}
+#else
+ // expected-no-diagnostics
+#endif
+}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3183,10 +3183,10 @@
"implicit conversion from %0 to %1 is not permitted in C++">;
def warn_impcast_float_precision : Warning<
"implicit conversion loses floating-point precision: %0 to %1">,
- InGroup, DefaultIgnore;
+ InGroup, DefaultIgnore;
def warn_impcast_flo
[PATCH] D53048: [Basic] Split out -Wimplicit-int-conversion and -Wimplicit-float-conversion from -Wconversion
erik.pilkington added inline comments. Comment at: clang/test/Sema/implicit-int-conversion.c:1-4 +// RUN: %clang_cc1 -Wconversion -Wno-implicit-int-conversion -DSMALL=char -DBIG=int -DNO_DIAG +// RUN: %clang_cc1 -Wno-conversion -Wimplicit-int-conversion -DSMALL=char -DBIG=int +// RUN: %clang_cc1 -Wconversion -Wno-implicit-float-conversion -DSMALL=float -DBIG=double -DNO_DIAG +// RUN: %clang_cc1 -Wno-conversion -Wimplicit-float-conversion -DSMALL=float -DBIG=double dexonsmith wrote: > Are these RUN lines missing `-verify`? Yep, also %s! Good catch. https://reviews.llvm.org/D53048 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53048: [Basic] Split out -Wimplicit-int-conversion and -Wimplicit-float-conversion from -Wconversion
erik.pilkington updated this revision to Diff 168912.
erik.pilkington added a comment.
Actually run the tests!
https://reviews.llvm.org/D53048
Files:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/test/Sema/implicit-int-conversion.c
Index: clang/test/Sema/implicit-int-conversion.c
===
--- /dev/null
+++ clang/test/Sema/implicit-int-conversion.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 %s -verify -Wconversion -Wno-implicit-int-conversion
-DSMALL=char -DBIG=int -DNO_DIAG
+// RUN: %clang_cc1 %s -verify -Wno-conversion -Wimplicit-int-conversion
-DSMALL=char -DBIG=int
+// RUN: %clang_cc1 %s -verify -Wconversion -Wno-implicit-float-conversion
-DSMALL=float -DBIG=double -DNO_DIAG
+// RUN: %clang_cc1 %s -verify -Wno-conversion -Wimplicit-float-conversion
-DSMALL=float -DBIG=double
+
+void f() {
+ SMALL a;
+ BIG b = 0;
+ a = b;
+#ifndef NO_DIAG
+ // expected-warning@-2 {{implicit conversion}}
+#else
+ // expected-no-diagnostics
+#endif
+}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3183,10 +3183,10 @@
"implicit conversion from %0 to %1 is not permitted in C++">;
def warn_impcast_float_precision : Warning<
"implicit conversion loses floating-point precision: %0 to %1">,
- InGroup, DefaultIgnore;
+ InGroup, DefaultIgnore;
def warn_impcast_float_result_precision : Warning<
"implicit conversion when assigning computation result loses floating-point
precision: %0 to %1">,
- InGroup, DefaultIgnore;
+ InGroup, DefaultIgnore;
def warn_impcast_double_promotion : Warning<
"implicit conversion increases floating-point precision: %0 to %1">,
InGroup, DefaultIgnore;
@@ -3198,10 +3198,10 @@
InGroup, DefaultIgnore;
def warn_impcast_integer_precision : Warning<
"implicit conversion loses integer precision: %0 to %1">,
- InGroup, DefaultIgnore;
+ InGroup, DefaultIgnore;
def warn_impcast_high_order_zero_bits : Warning<
"higher order bits are zeroes after implicit conversion">,
- InGroup, DefaultIgnore;
+ InGroup, DefaultIgnore;
def warn_impcast_nonnegative_result : Warning<
"the resulting value is always non-negative after implicit conversion">,
InGroup, DefaultIgnore;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -59,6 +59,8 @@
UndefinedBoolConversion]>;
def IntConversion : DiagGroup<"int-conversion">;
def EnumConversion : DiagGroup<"enum-conversion">;
+def ImplicitIntConversion : DiagGroup<"implicit-int-conversion">;
+def ImplicitFloatConversion : DiagGroup<"implicit-float-conversion">;
def FloatOverflowConversion : DiagGroup<"float-overflow-conversion">;
def FloatZeroConversion : DiagGroup<"float-zero-conversion">;
@@ -709,6 +711,8 @@
FloatConversion,
Shorten64To32,
IntConversion,
+ImplicitIntConversion,
+ImplicitFloatConversion,
LiteralConversion,
NonLiteralNullConversion, // (1-1)->pointer (etc)
NullConversion, // NULL->non-pointer
Index: clang/test/Sema/implicit-int-conversion.c
===
--- /dev/null
+++ clang/test/Sema/implicit-int-conversion.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 %s -verify -Wconversion -Wno-implicit-int-conversion -DSMALL=char -DBIG=int -DNO_DIAG
+// RUN: %clang_cc1 %s -verify -Wno-conversion -Wimplicit-int-conversion -DSMALL=char -DBIG=int
+// RUN: %clang_cc1 %s -verify -Wconversion -Wno-implicit-float-conversion -DSMALL=float -DBIG=double -DNO_DIAG
+// RUN: %clang_cc1 %s -verify -Wno-conversion -Wimplicit-float-conversion -DSMALL=float -DBIG=double
+
+void f() {
+ SMALL a;
+ BIG b = 0;
+ a = b;
+#ifndef NO_DIAG
+ // expected-warning@-2 {{implicit conversion}}
+#else
+ // expected-no-diagnostics
+#endif
+}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3183,10 +3183,10 @@
"implicit conversion from %0 to %1 is not permitted in C++">;
def warn_impcast_float_precision : Warning<
"implicit conversion loses floating-point precision: %0 to %1">,
- InGroup, DefaultIgnore;
+ InGroup, DefaultIgnore;
def warn_impcast_float_result_precision : Warning<
"implicit conversion when assigning computation result loses floating-point precision:
[PATCH] D53046: [Sema] Fix an error-on-valid with friends and templates
erik.pilkington added a comment. In https://reviews.llvm.org/D53046#1259933, @rjmccall wrote: > The linking does actually happen in this test case, right? Can we just do > something when linking them to unify their `Common` structures? Yep, that would work too I think. We can't properly merge in the general case, because we could have to decide between more than one identical instantiations of different template redecls, but in this case the only ordering problem is with the injected template arguments, which I believe we can merge (by just discarding the new arguments, if a previous redeclaration already has some). It would definitely be less intrusive, but its a more complicated invariant. Do you think that is a better solution? I could go either way. Repository: rC Clang https://reviews.llvm.org/D53046 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53046: [Sema] Fix an error-on-valid with friends and templates
erik.pilkington updated this revision to Diff 168939.
erik.pilkington added a comment.
Merge the common pointers rather than trying to use the previous one. Thanks!
https://reviews.llvm.org/D53046
Files:
clang/include/clang/AST/DeclTemplate.h
clang/lib/AST/DeclTemplate.cpp
clang/lib/Sema/SemaDecl.cpp
clang/test/SemaCXX/friend-template-redecl.cpp
Index: clang/test/SemaCXX/friend-template-redecl.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/friend-template-redecl.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -std=c++17 -verify -emit-llvm-only %s
+
+// expected-no-diagnostics
+
+template void bar(const T &t) { foo(t); }
+
+template
+struct HasFriend {
+ template
+ friend void foo(const HasFriend &m) noexcept(false);
+};
+
+template
+void foo(const HasFriend &m) noexcept(false) {}
+
+void f() {
+ HasFriend x;
+ foo(x);
+ bar(x);
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10022,11 +10022,17 @@
if (FunctionTemplateDecl *OldTemplateDecl =
dyn_cast(OldDecl)) {
auto *OldFD = OldTemplateDecl->getTemplatedDecl();
- NewFD->setPreviousDeclaration(OldFD);
- adjustDeclContextForDeclaratorDecl(NewFD, OldFD);
FunctionTemplateDecl *NewTemplateDecl
= NewFD->getDescribedFunctionTemplate();
assert(NewTemplateDecl && "Template/non-template mismatch");
+
+ // The call to MergeFunctionDecl above may have created some state in
+ // NewTemplateDecl that needs to be merged with OldTemplateDecl before we
+ // can add it as a redeclaration.
+ NewTemplateDecl->mergePrevDecl(OldTemplateDecl);
+
+ NewFD->setPreviousDeclaration(OldFD);
+ adjustDeclContextForDeclaratorDecl(NewFD, OldFD);
if (NewFD->isCXXClassMember()) {
NewFD->setAccess(OldTemplateDecl->getAccess());
NewTemplateDecl->setAccess(OldTemplateDecl->getAccess());
Index: clang/lib/AST/DeclTemplate.cpp
===
--- clang/lib/AST/DeclTemplate.cpp
+++ clang/lib/AST/DeclTemplate.cpp
@@ -300,6 +300,42 @@
return llvm::makeArrayRef(CommonPtr->InjectedArgs, Params->size());
}
+void FunctionTemplateDecl::mergePrevDecl(FunctionTemplateDecl *Prev) {
+ using Base = RedeclarableTemplateDecl;
+
+ // If we haven't created a common pointer yet, then it can just be created
+ // with the usual method.
+ if (!Base::Common)
+return;
+
+ Common *ThisCommon = static_cast(Base::Common);
+ Common *PrevCommon = nullptr;
+ SmallVector PreviousDecls;
+ for (; Prev; Prev = Prev->getPreviousDecl()) {
+if (Prev->Base::Common) {
+ PrevCommon = static_cast(Prev->Base::Common);
+ break;
+}
+PreviousDecls.push_back(Prev);
+ }
+
+ // If the previous redecl chain hasn't created a common pointer yet, then just
+ // use this common pointer.
+ if (!PrevCommon) {
+for (auto *D : PreviousDecls)
+ D->Base::Common = ThisCommon;
+return;
+ }
+
+ // Ensure we don't leak any important state.
+ assert(ThisCommon->Specializations.size() == 0 &&
+ !ThisCommon->InstantiatedFromMember.getPointer() &&
+ !ThisCommon->InstantiatedFromMember.getInt() &&
+ "Can't merge incompatible declarations!");
+
+ Base::Common = PrevCommon;
+}
+
//===--===//
// ClassTemplateDecl Implementation
//===--===//
Index: clang/include/clang/AST/DeclTemplate.h
===
--- clang/include/clang/AST/DeclTemplate.h
+++ clang/include/clang/AST/DeclTemplate.h
@@ -1093,6 +1093,9 @@
/// template.
ArrayRef getInjectedTemplateArgs();
+ /// Merge our RedeclarableTemplateDecl::Common with \param Prev.
+ void mergePrevDecl(FunctionTemplateDecl *Prev);
+
/// Create a function template node.
static FunctionTemplateDecl *Create(ASTContext &C, DeclContext *DC,
SourceLocation L,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53154: [CodeGen] Handle extern references to OBJC_CLASS_$_*
erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, ahatanak, jfb.
Herald added a subscriber: dexonsmith.
Some ObjC users declare a `extern` variable named `OBJC_CLASS_$_Foo`, then use
it's address as a `Class`. I.e., one could define `isInstanceOfF`:
BOOL isInstanceOfF(id c) {
extern void OBJC_CLASS_$_F;
return [c class] == (Class)&OBJC_CLASS_$_F;
}
This leads to asserts in clang CodeGen if there is an `@implementation` of `F`
in the same TU as an instance of this pattern, because CodeGen assumes that a
variable named OBJC_CLASS_$_* has the right type. This patch fixes the problem
by RAUWing the old (incorrectly typed) global with a new global, then removing
the old global.
I know almost nothing about Objective-C runtime stuff, so take this patch with
a grain of salt!
Fixes rdar://45077269
Repository:
rC Clang
https://reviews.llvm.org/D53154
Files:
clang/lib/CodeGen/CGObjCMac.cpp
clang/test/CodeGenObjC/extern-void-class-decl.m
Index: clang/test/CodeGenObjC/extern-void-class-decl.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/extern-void-class-decl.m
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -emit-llvm -o - |
FileCheck %s
+
+// rdar://45077269
+
+extern void OBJC_CLASS_$_f;
+Class c = (Class)&OBJC_CLASS_$_f;
+
+@implementation f @end
+
+// Check that we override the initializer for c, and that OBJC_CLASS_$_f gets
+// the right definition.
+
+// CHECK: @c = global i8* bitcast (%struct._class_t* @"OBJC_CLASS_$_f" to i8*)
+// CHECK: @"OBJC_CLASS_$_f" = global %struct._class_t
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -7188,15 +7188,21 @@
Weak ? llvm::GlobalValue::ExternalWeakLinkage
: llvm::GlobalValue::ExternalLinkage;
-
-
llvm::GlobalVariable *GV = CGM.getModule().getGlobalVariable(Name);
- if (!GV) {
-GV = new llvm::GlobalVariable(CGM.getModule(), ObjCTypes.ClassnfABITy,
- false, L, nullptr, Name);
+ if (!GV || GV->getType() != ObjCTypes.ClassnfABITy->getPointerTo()) {
+auto *NewGV = new llvm::GlobalVariable(ObjCTypes.ClassnfABITy, false, L,
+ nullptr, Name);
if (DLLImport)
- GV->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
+ NewGV->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
+
+if (GV) {
+ GV->replaceAllUsesWith(
+ llvm::ConstantExpr::getBitCast(NewGV, GV->getType()));
+ GV->eraseFromParent();
+}
+GV = NewGV;
+CGM.getModule().getGlobalList().push_back(GV);
}
assert(GV->getLinkage() == L);
Index: clang/test/CodeGenObjC/extern-void-class-decl.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/extern-void-class-decl.m
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -emit-llvm -o - | FileCheck %s
+
+// rdar://45077269
+
+extern void OBJC_CLASS_$_f;
+Class c = (Class)&OBJC_CLASS_$_f;
+
+@implementation f @end
+
+// Check that we override the initializer for c, and that OBJC_CLASS_$_f gets
+// the right definition.
+
+// CHECK: @c = global i8* bitcast (%struct._class_t* @"OBJC_CLASS_$_f" to i8*)
+// CHECK: @"OBJC_CLASS_$_f" = global %struct._class_t
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -7188,15 +7188,21 @@
Weak ? llvm::GlobalValue::ExternalWeakLinkage
: llvm::GlobalValue::ExternalLinkage;
-
-
llvm::GlobalVariable *GV = CGM.getModule().getGlobalVariable(Name);
- if (!GV) {
-GV = new llvm::GlobalVariable(CGM.getModule(), ObjCTypes.ClassnfABITy,
- false, L, nullptr, Name);
+ if (!GV || GV->getType() != ObjCTypes.ClassnfABITy->getPointerTo()) {
+auto *NewGV = new llvm::GlobalVariable(ObjCTypes.ClassnfABITy, false, L,
+ nullptr, Name);
if (DLLImport)
- GV->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
+ NewGV->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
+
+if (GV) {
+ GV->replaceAllUsesWith(
+ llvm::ConstantExpr::getBitCast(NewGV, GV->getType()));
+ GV->eraseFromParent();
+}
+GV = NewGV;
+CGM.getModule().getGlobalList().push_back(GV);
}
assert(GV->getLinkage() == L);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output
erik.pilkington created this revision.
erik.pilkington added reviewers: bruno, rsmith.
Herald added a subscriber: dexonsmith.
This patch makes clang include headers found in modulemap files in the `.d`
file. This is necessary to capture symlinked headers, which previously were
ignored, since only the canonical version of the file makes it into the pcm.
This is a corollary to the -module-dependency-dir version from r264971.
rdar://44604913
Thanks for taking a look!
Erik
Repository:
rC Clang
https://reviews.llvm.org/D53522
Files:
clang/include/clang/Lex/ModuleMap.h
clang/lib/Frontend/DependencyFile.cpp
clang/lib/Frontend/ModuleDependencyCollector.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Serialization/ASTReader.cpp
clang/test/Modules/Inputs/dfg-symlinks.modulemap
clang/test/Modules/dependency-file-symlinks.c
clang/test/Modules/dependency-gen-pch.m
clang/test/Modules/dependency-gen.m
clang/test/Modules/dependency-gen.modulemap
Index: clang/test/Modules/dependency-gen.modulemap
===
--- clang/test/Modules/dependency-gen.modulemap
+++ clang/test/Modules/dependency-gen.modulemap
@@ -27,6 +27,7 @@
// For an explicit use of a module via -fmodule-file=, the other module maps
// and included headers are not dependencies of this target (they are instead
// dependencies of the explicitly-specified .pcm input).
+// FIXME: Shouldn't we include all transitive dependencies here?
//
// EXPLICIT: {{^}}explicit.pcm:
// EXPLICIT-NOT: dependency-gen-
Index: clang/test/Modules/dependency-gen.m
===
--- clang/test/Modules/dependency-gen.m
+++ clang/test/Modules/dependency-gen.m
@@ -4,19 +4,19 @@
// RUN: %clang_cc1 -x objective-c -isystem %S/Inputs/System/usr/include -dependency-file %t.d.1 -MT %s.o -I %S/Inputs -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t-mcp %s
// RUN: FileCheck %s < %t.d.1
// CHECK: dependency-gen.m
-// CHECK: Inputs{{.}}module.map
// CHECK: Inputs{{.}}diamond_top.h
+// CHECK: Inputs{{.}}module.map
// CHECK-NOT: usr{{.}}include{{.}}module.map
// CHECK-NOT: stdint.h
// RUN: %clang_cc1 -x objective-c -isystem %S/Inputs/System/usr/include -dependency-file %t.d.2 -MT %s.o -I %S/Inputs -sys-header-deps -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t-mcp %s
// RUN: FileCheck %s -check-prefix=CHECK-SYS < %t.d.2
// CHECK-SYS: dependency-gen.m
-// CHECK-SYS: Inputs{{.}}module.map
// CHECK-SYS: Inputs{{.}}diamond_top.h
-// CHECK-SYS: usr{{.}}include{{.}}module.map
+// CHECK-SYS: Inputs{{.}}module.map
// CHECK-SYS: stdint.h
+// CHECK-SYS: usr{{.}}include{{.}}module.map
#import "diamond_top.h"
#import "stdint.h" // inside sysroot
Index: clang/test/Modules/dependency-gen-pch.m
===
--- clang/test/Modules/dependency-gen-pch.m
+++ clang/test/Modules/dependency-gen-pch.m
@@ -5,9 +5,9 @@
// RUN: %clang_cc1 -isysroot %S/Inputs/System -triple x86_64-apple-darwin10 -module-file-deps -dependency-file %t.d -MT %s.o -I %S/Inputs -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t-mcp -emit-pch -o %t.pch %s
// RUN: FileCheck %s < %t.d
// CHECK: dependency-gen-pch.m.o
-// CHECK-NEXT: dependency-gen-pch.m
-// CHECK-NEXT: Inputs{{.}}module.map
-// CHECK-NEXT: diamond_top.pcm
-// CHECK-NEXT: Inputs{{.}}diamond_top.h
+// CHECK: dependency-gen-pch.m
+// CHECK: Inputs{{.}}diamond_top.h
+// CHECK: Inputs{{.}}module.map
+// CHECK: diamond_top.pcm
#import "diamond_top.h"
Index: clang/test/Modules/dependency-file-symlinks.c
===
--- /dev/null
+++ clang/test/Modules/dependency-file-symlinks.c
@@ -0,0 +1,37 @@
+// REQUIRES: shell
+
+// RUN: rm -fr %t
+// RUN: mkdir -p %t/cache
+
+// Set up %t as follows:
+// * misc.h #includes target.h
+// * target.h is empty
+// * link.h is a symlink to target.h
+
+// RUN: cp %S/Inputs/dfg-symlinks.modulemap %t/module.modulemap
+// RUN: echo "int foo();" > %t/target.h
+// RUN: ln -s %t/target.h %t/link.h
+// RUN: echo '#include "target.h"' >> %t/misc.h
+// RUN: echo 'int foo();' >> %t/misc.h
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache %s \
+// RUN: -I %t -MT dependency-file-symlinks.o -dependency-file - | FileCheck %s
+
+// Run clang again, to make sure that we get identical output with the cached
+// modules.
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache %s \
+// RUN: -I %t -MT dependency-file-symlinks.o -dependency-file - | FileCheck %s
+
+#include "misc.h"
+
+int main() {
+ void *p = foo;
+}
+
+// CHECK: dependency-file-symlinks.o:
+// CHECK-NEXT: {{.*}}dependency-file-symlinks.c
+// CHECK-NEXT: {{.*}}link.h
+// CHECK-NEXT: {{.*}}misc.h
+// CHECK-NEXT: {{.*}}module.modulemap
+// CHECK-NEXT: {{.*}}target.h
Index: cl
