[PATCH] D30465: [mips] Set the Int64Type / IntMaxType types correctly for OpenBSD/mips64
brad added a comment. ping? Repository: rL LLVM https://reviews.llvm.org/D30465 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check
fgross marked 6 inline comments as done. fgross added inline comments. Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:110 +ClassWithSpecialMembers[ID]; +if (find(Members, Kind) == Members.end()) + Members.push_back(Kind); aaron.ballman wrote: > Please qualify the find with std::, here and elsewhere. It's actually llvm::; added namespace and switched to is_contained. https://reviews.llvm.org/D30610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check
fgross updated this revision to Diff 90593. fgross added a comment. Updated documentation, got rid of code duplication. https://reviews.llvm.org/D30610 Files: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp test/clang-tidy/cppcoreguidelines-special-member-functions.cpp Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp === --- test/clang-tidy/cppcoreguidelines-special-member-functions.cpp +++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp @@ -3,7 +3,12 @@ class DefinesDestructor { ~DefinesDestructor(); }; -// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions] +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions] + +class DefinesDefaultedDestructor { + ~DefinesDefaultedDestructor() = default; +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDefaultedDestructor' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions] class DefinesCopyConstructor { DefinesCopyConstructor(const DefinesCopyConstructor &); Index: test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp === --- test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp +++ test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp @@ -0,0 +1,71 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: [{key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctions, value: 1}, {key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor, value: 1}]}" -- + +class DefinesDestructor { + ~DefinesDestructor(); +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions] + +class DefinesDefaultedDestructor { + ~DefinesDefaultedDestructor() = default; +}; + +class DefinesCopyConstructor { + DefinesCopyConstructor(const DefinesCopyConstructor &); +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor or a copy assignment operator [cppcoreguidelines-special-member-functions] + +class DefinesCopyAssignment { + DefinesCopyAssignment &operator=(const DefinesCopyAssignment &); +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor or a copy constructor [cppcoreguidelines-special-member-functions] + +class DefinesMoveConstructor { + DefinesMoveConstructor(DefinesMoveConstructor &&); +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveConstructor' defines a move constructor but does not define a destructor, a copy constructor, a copy assignment operator or a move assignment operator [cppcoreguidelines-special-member-functions] + +class DefinesMoveAssignment { + DefinesMoveAssignment &operator=(DefinesMoveAssignment &&); +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveAssignment' defines a move assignment operator but does not define a destructor, a copy constructor, a copy assignment operator or a move constructor [cppcoreguidelines-special-member-functions] +class DefinesNothing { +}; + +class DefinesEverything { + DefinesEverything(const DefinesEverything &); + DefinesEverything &operator=(const DefinesEverything &); + DefinesEverything(DefinesEverything &&); + DefinesEverything &operator=(DefinesEverything &&); + ~DefinesEverything(); +}; + +class DeletesEverything { + DeletesEverything(const DeletesEverything &) = delete; + DeletesEverything &operator=(const DeletesEverything &) = delete; + DeletesEverything(DeletesEverything &&) = delete; + DeletesEverything &operator=(DeletesEverything &&) = delete; + ~DeletesEverything() = delete; +}; + +class DeletesCopyDefaultsMove { + DeletesCopyDefaultsMove(const DeletesCopyDefaultsMove &) = delete; + DeletesCopyDefaultsMove &operator=(const DeletesCopyDefaultsMove &) = delete; + DeletesCopyDefaultsMove(DeletesCopyDefaults
[PATCH] D22476: [AST] Make MemberExpr non-dependent according to core issue 224
mgehre updated this revision to Diff 90592. mgehre added a comment. Updated fix for the crash in SemaChecking::RefersToMemberWithReducedAlignment, so it also works for C code. https://reviews.llvm.org/D22476 Files: lib/AST/Expr.cpp lib/Sema/SemaChecking.cpp lib/Sema/SemaOverload.cpp test/Analysis/stack-addr-ps.cpp test/CXX/class.access/class.access.dcl/p1.cpp test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp test/CXX/drs/dr3xx.cpp test/CXX/temp/temp.res/temp.dep/temp.dep.expr/p5.cpp test/SemaTemplate/dependent-names.cpp test/SemaTemplate/enum-argument.cpp test/SemaTemplate/instantiate-self.cpp test/SemaTemplate/member-access-expr.cpp Index: test/SemaTemplate/member-access-expr.cpp === --- test/SemaTemplate/member-access-expr.cpp +++ test/SemaTemplate/member-access-expr.cpp @@ -154,9 +154,7 @@ template class Derived : public Base { A *field; void get(B **ptr) { - // It's okay if at some point we figure out how to diagnose this - // at instantiation time. - *ptr = field; + *ptr = field; // expected-error{{assigning to 'test6::B *' from incompatible type 'test6::A *'}} } }; } Index: test/SemaTemplate/instantiate-self.cpp === --- test/SemaTemplate/instantiate-self.cpp +++ test/SemaTemplate/instantiate-self.cpp @@ -99,7 +99,7 @@ namespace test10 { template struct A { -void f() noexcept(noexcept(f())); // expected-error {{exception specification of 'f' uses itself}} expected-note {{instantiation of}} +void f() noexcept(noexcept(f())); // / expected-error {{exception specification is not available until end of class definition}} expected-error {{exception specification of 'f' uses itself}} expected-note {{instantiation of}} }; bool b = noexcept(A().f()); // expected-note {{instantiation of}} } Index: test/SemaTemplate/enum-argument.cpp === --- test/SemaTemplate/enum-argument.cpp +++ test/SemaTemplate/enum-argument.cpp @@ -31,7 +31,7 @@ unsigned long long bitfield : e0; void f(int j) { - bitfield + j; + (void)(bitfield + j); } }; } Index: test/SemaTemplate/dependent-names.cpp === --- test/SemaTemplate/dependent-names.cpp +++ test/SemaTemplate/dependent-names.cpp @@ -274,7 +274,7 @@ int e[10]; }; void g() { - S().f(); // expected-note {{here}} + S().f(); } } Index: test/CXX/temp/temp.res/temp.dep/temp.dep.expr/p5.cpp === --- /dev/null +++ test/CXX/temp/temp.res/temp.dep/temp.dep.expr/p5.cpp @@ -0,0 +1,39 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s + +namespace CWG224 { +template +struct A { + int *i; + typedef T newT; + void f() { +char *j = nullptr; +i = j; // expected-error {{incompatible type}} +i = nullptr; +(*this).i = j; // FIXME {{incompatible type}} +this->i = j; // FIXME {{incompatible type}} +A::i = j;// expected-error {{incompatible type}} +A::i = j; // expected-error {{incompatible type}} +A::i = j;// not current instantiation +A::i = j; // expected-error {{incompatible type}} +::CWG224::A::i = j; // expected-error {{incompatible type}} + } + struct B { +int *b; +void f() { + char *c = nullptr; + b = c; // expected-error {{incompatible type}} + B::b = c;// expected-error {{incompatible type}} + A::B::b = c; // expected-error {{incompatible type}} + A::B::b = c; // not current instantiation +} + }; +}; + +template +class A { + void f(A *a1, A *a2) { +a1->i = 1; // FIXME {{incompatible type}} +a2->i = 1; // not current instantiation + } +}; +} // namespace CWG224 Index: test/CXX/drs/dr3xx.cpp === --- test/CXX/drs/dr3xx.cpp +++ test/CXX/drs/dr3xx.cpp @@ -1167,8 +1167,8 @@ namespace dr390 { // dr390: yes template struct A { -A() { f(); } // expected-warning {{call to pure virt}} -virtual void f() = 0; // expected-note {{here}} +A() { f(); } // expected-warning {{call to pure virt}} expected-warning {{call to pure virt}} +virtual void f() = 0; // expected-note {{here}} expected-note {{here}} virtual ~A() = 0; }; template A::~A() { T::error; } // expected-error {{cannot be used prior to}} Index: test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp === --- test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp +++ test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp @@ -29,7 +29,10 @@ }; } -struct Opaque0 {}; +struct Opaque0 {}; // e
[PATCH] D22476: [AST] Make MemberExpr non-dependent according to core issue 224
mgehre added inline comments. Comment at: lib/AST/Expr.cpp:1413 +for (auto *EnableIf : Method->specific_attrs()) { + if (EnableIf->getCond()->isTypeDependent()) +return false; rsmith wrote: > You should presumably be checking `isValueDependent` here. > > I also don't see any test changes covering the handling of this attribute, > and it's not completely clear to me why a dependent enable_if attribute would > cause a member to be treated as not being a member of the current > instantiation. This should probably be grouped with the check for a member > with a dependent type down on line 1458, not as part of the determination of > whether we have a member of the current instantiation. Thanks, I will move the check down. A test for this is in test/SemaCXX/enable_if.cpp: ``` template class C { void f() __attribute__((enable_if(T::expr == 0, ""))) {} void g() { f(); } }; ``` If I would not check for value dependent attributes, then the call to f would be considered non-type dependent and produce the warnings ``` error: 'error' diagnostics seen but not expected: File test/SemaCXX/enable_if.cpp Line 116: no matching member function for call to 'f' error: 'note' diagnostics seen but not expected: File test/SemaCXX/enable_if.cpp Line 115: candidate disabled: 2 errors generated. ``` We need to keep f() type-dependent until the template is instantiated. Comment at: lib/AST/Expr.cpp:1456 + // the field is type dependent. (14.6.2.1 [temp.dep.type]) + if (E->isTypeDependent() + && IsMemberOfCurrentInstantiation(base, memberdecl) rsmith wrote: > rsmith wrote: > > Please clang-format this. > The `E->isTypeDependent()` test should be in `isMemberOfCurrentInstantiation` > -- right now, that function is determining whether we have a member of the > current instantiation or a member of a non-dependent type. I will move it there. Comment at: lib/AST/Expr.cpp:1456-1458 + if (E->isTypeDependent() + && IsMemberOfCurrentInstantiation(base, memberdecl) + && !memberdecl->getType()->isDependentType()) { mgehre wrote: > rsmith wrote: > > rsmith wrote: > > > Please clang-format this. > > The `E->isTypeDependent()` test should be in > > `isMemberOfCurrentInstantiation` -- right now, that function is determining > > whether we have a member of the current instantiation or a member of a > > non-dependent type. > I will move it there. Sorry, done. Comment at: test/CXX/temp/temp.res/temp.dep/temp.dep.expr/p5.cpp:14-15 + i = nullptr; + (*this).i = j; // FIXME {{incompatible type}} + this->i = j; // FIXME {{incompatible type}} + A::i = j; // expected-error {{incompatible type}} rsmith wrote: > Do you know why we don't diagnose this? This patch adds support for the MemberExpr, e.g. ``` i = j; // expected-error {{incompatible type}} ``` which before this patch had the AST ``` | | |-BinaryOperator 0x3449f50 '' '=' | | | |-MemberExpr 0x3449ef0 'int *' lvalue ->i 0x3449950 | | | | `-CXXThisExpr 0x3449ed8 'A *' this | | | `-DeclRefExpr 0x3449f28 'char *' lvalue Var 0x3449e30 'j' 'char *' ``` Not yet fixed are CXXDependentScopeMemberExpr, i.e. ``` (*this).i = j; // FIXME {{incompatible type}} | | | - BinaryOperator 0x8cfbd0 '' '=' | | | |-CXXDependentScopeMemberExpr 0x8cfb50 '' lvalue .i | | | | `-ParenExpr 0x8cfb30 '' | | | | `-UnaryOperator 0x8cfb10 '' prefix '*' | | | | `-CXXThisExpr 0x8cfaf8 'A *' this | | | `-DeclRefExpr 0x8cfba8 'char *' lvalue Var 0x8a5a08 'j' 'char *' ``` ``` this->i = j; // FIXME {{incompatible type}} | | |-BinaryOperator 0x8cfc90 '' '=' | | | |-CXXDependentScopeMemberExpr 0x8cfc10 '' lvalue ->i | | | | `-CXXThisExpr 0x8cfbf8 'A *' this | | | `-DeclRefExpr 0x8cfc68 'char *' lvalue Var 0x8a5a08 'j' 'char *' ``` https://reviews.llvm.org/D22476 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D22476: [AST] Make MemberExpr non-dependent according to core issue 224
mgehre updated this revision to Diff 90591. mgehre marked 4 inline comments as done. mgehre added a comment. Thanks for your comments! I'm very sorry for the huge delay. I have now more time to work on this. I added all your comments to the commit and fixed a crash in SemaChecking::RefersToMemberWithReducedAlignment, when the base is an InjectedClassNameType (because it assumed that it can only be a RecordType). https://reviews.llvm.org/D22476 Files: lib/AST/Expr.cpp lib/Sema/SemaChecking.cpp lib/Sema/SemaOverload.cpp test/Analysis/stack-addr-ps.cpp test/CXX/class.access/class.access.dcl/p1.cpp test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp test/CXX/drs/dr3xx.cpp test/CXX/temp/temp.res/temp.dep/temp.dep.expr/p5.cpp test/SemaTemplate/dependent-names.cpp test/SemaTemplate/enum-argument.cpp test/SemaTemplate/instantiate-self.cpp test/SemaTemplate/member-access-expr.cpp Index: test/SemaTemplate/member-access-expr.cpp === --- test/SemaTemplate/member-access-expr.cpp +++ test/SemaTemplate/member-access-expr.cpp @@ -154,9 +154,7 @@ template class Derived : public Base { A *field; void get(B **ptr) { - // It's okay if at some point we figure out how to diagnose this - // at instantiation time. - *ptr = field; + *ptr = field; // expected-error{{assigning to 'test6::B *' from incompatible type 'test6::A *'}} } }; } Index: test/SemaTemplate/instantiate-self.cpp === --- test/SemaTemplate/instantiate-self.cpp +++ test/SemaTemplate/instantiate-self.cpp @@ -99,7 +99,7 @@ namespace test10 { template struct A { -void f() noexcept(noexcept(f())); // expected-error {{exception specification of 'f' uses itself}} expected-note {{instantiation of}} +void f() noexcept(noexcept(f())); // / expected-error {{exception specification is not available until end of class definition}} expected-error {{exception specification of 'f' uses itself}} expected-note {{instantiation of}} }; bool b = noexcept(A().f()); // expected-note {{instantiation of}} } Index: test/SemaTemplate/enum-argument.cpp === --- test/SemaTemplate/enum-argument.cpp +++ test/SemaTemplate/enum-argument.cpp @@ -31,7 +31,7 @@ unsigned long long bitfield : e0; void f(int j) { - bitfield + j; + (void)(bitfield + j); } }; } Index: test/SemaTemplate/dependent-names.cpp === --- test/SemaTemplate/dependent-names.cpp +++ test/SemaTemplate/dependent-names.cpp @@ -274,7 +274,7 @@ int e[10]; }; void g() { - S().f(); // expected-note {{here}} + S().f(); } } Index: test/CXX/temp/temp.res/temp.dep/temp.dep.expr/p5.cpp === --- /dev/null +++ test/CXX/temp/temp.res/temp.dep/temp.dep.expr/p5.cpp @@ -0,0 +1,39 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s + +namespace CWG224 { +template +struct A { + int *i; + typedef T newT; + void f() { +char *j = nullptr; +i = j; // expected-error {{incompatible type}} +i = nullptr; +(*this).i = j; // FIXME {{incompatible type}} +this->i = j; // FIXME {{incompatible type}} +A::i = j;// expected-error {{incompatible type}} +A::i = j; // expected-error {{incompatible type}} +A::i = j;// not current instantiation +A::i = j; // expected-error {{incompatible type}} +::CWG224::A::i = j; // expected-error {{incompatible type}} + } + struct B { +int *b; +void f() { + char *c = nullptr; + b = c; // expected-error {{incompatible type}} + B::b = c;// expected-error {{incompatible type}} + A::B::b = c; // expected-error {{incompatible type}} + A::B::b = c; // not current instantiation +} + }; +}; + +template +class A { + void f(A *a1, A *a2) { +a1->i = 1; // FIXME {{incompatible type}} +a2->i = 1; // not current instantiation + } +}; +} // namespace CWG224 Index: test/CXX/drs/dr3xx.cpp === --- test/CXX/drs/dr3xx.cpp +++ test/CXX/drs/dr3xx.cpp @@ -1167,8 +1167,8 @@ namespace dr390 { // dr390: yes template struct A { -A() { f(); } // expected-warning {{call to pure virt}} -virtual void f() = 0; // expected-note {{here}} +A() { f(); } // expected-warning {{call to pure virt}} expected-warning {{call to pure virt}} +virtual void f() = 0; // expected-note {{here}} expected-note {{here}} virtual ~A() = 0; }; template A::~A() { T::error; } // expected-error {{cannot be used prior to}} Index: test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
r296980 - GCC workaround: use explicit qualification to avoid injected class name.
Author: rjmccall Date: Sat Mar 4 15:46:14 2017 New Revision: 296980 URL: http://llvm.org/viewvc/llvm-project?rev=296980&view=rev Log: GCC workaround: use explicit qualification to avoid injected class name. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52625 Modified: cfe/trunk/include/clang/CodeGen/ConstantInitBuilder.h Modified: cfe/trunk/include/clang/CodeGen/ConstantInitBuilder.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CodeGen/ConstantInitBuilder.h?rev=296980&r1=296979&r2=296980&view=diff == --- cfe/trunk/include/clang/CodeGen/ConstantInitBuilder.h (original) +++ cfe/trunk/include/clang/CodeGen/ConstantInitBuilder.h Sat Mar 4 15:46:14 2017 @@ -485,8 +485,10 @@ class ConstantArrayBuilder : public ConstantArrayBuilderTemplateBase { template friend class ConstantInitBuilderTemplateBase; + + // The use of explicit qualification is a GCC workaround. template - friend class ConstantAggregateBuilderTemplateBase; + friend class CodeGen::ConstantAggregateBuilderTemplateBase; ConstantArrayBuilder(ConstantInitBuilder &builder, ConstantAggregateBuilderBase *parent, @@ -500,8 +502,10 @@ class ConstantStructBuilder : public ConstantStructBuilderTemplateBase { template friend class ConstantInitBuilderTemplateBase; + + // The use of explicit qualification is a GCC workaround. template - friend class ConstantAggregateBuilderTemplateBase; + friend class CodeGen::ConstantAggregateBuilderTemplateBase; ConstantStructBuilder(ConstantInitBuilder &builder, ConstantAggregateBuilderBase *parent, ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r296979 - Refactor ConstantInitBuilder to allow other frontends to more
Author: rjmccall Date: Sat Mar 4 15:26:29 2017 New Revision: 296979 URL: http://llvm.org/viewvc/llvm-project?rev=296979&view=rev Log: Refactor ConstantInitBuilder to allow other frontends to more easily extend the aggregate-builder API. Stupid missing language features. Also add APIs for constructing a relative reference and computing the offset of a position from the start of the initializer. Modified: cfe/trunk/include/clang/CodeGen/ConstantInitBuilder.h cfe/trunk/lib/CodeGen/ConstantInitBuilder.cpp Modified: cfe/trunk/include/clang/CodeGen/ConstantInitBuilder.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CodeGen/ConstantInitBuilder.h?rev=296979&r1=296978&r2=296979&view=diff == --- cfe/trunk/include/clang/CodeGen/ConstantInitBuilder.h (original) +++ cfe/trunk/include/clang/CodeGen/ConstantInitBuilder.h Sat Mar 4 15:26:29 2017 @@ -28,8 +28,6 @@ namespace clang { namespace CodeGen { class CodeGenModule; -class ConstantStructBuilder; -class ConstantArrayBuilder; /// A convenience builder class for complex constant initializers, /// especially for anonymous global structures used by various language @@ -45,12 +43,12 @@ class ConstantArrayBuilder; /// widgetDesc.addInt(CGM.SizeTy, widget.getPower()); /// widgetDesc.add(CGM.GetAddrOfConstantString(widget.getName())); /// widgetDesc.add(CGM.GetAddrOfGlobal(widget.getInitializerDecl())); -/// widgetArray.add(widgetDesc.finish()); +/// widgetDesc.finishAndAddTo(widgetArray); ///} -///toplevel.add(widgetArray.finish()); +///widgetArray.finishAndAddTo(toplevel); ///auto global = toplevel.finishAndCreateGlobal("WIDGET_LIST", Align, /// /*constant*/ true); -class ConstantInitBuilder { +class ConstantInitBuilderBase { struct SelfReference { llvm::GlobalVariable *Dummy; llvm::SmallVector Indices; @@ -62,254 +60,331 @@ class ConstantInitBuilder { std::vector SelfReferences; bool Frozen = false; -public: - explicit ConstantInitBuilder(CodeGenModule &CGM) : CGM(CGM) {} + friend class ConstantAggregateBuilderBase; + template + friend class ConstantAggregateBuilderTemplateBase; + + // The rule for CachedOffset is that anything which removes elements + // from the Buffer + +protected: + explicit ConstantInitBuilderBase(CodeGenModule &CGM) : CGM(CGM) {} - ~ConstantInitBuilder() { + ~ConstantInitBuilderBase() { assert(Buffer.empty() && "didn't claim all values out of buffer"); } - class AggregateBuilderBase { - protected: -ConstantInitBuilder &Builder; -AggregateBuilderBase *Parent; -size_t Begin; -bool Finished = false; -bool Frozen = false; +private: + llvm::GlobalVariable *createGlobal(llvm::Constant *initializer, + const llvm::Twine &name, + CharUnits alignment, + bool constant = false, + llvm::GlobalValue::LinkageTypes linkage + = llvm::GlobalValue::InternalLinkage, + unsigned addressSpace = 0); -llvm::SmallVectorImpl &getBuffer() { - return Builder.Buffer; -} + void setGlobalInitializer(llvm::GlobalVariable *GV, +llvm::Constant *initializer); -const llvm::SmallVectorImpl &getBuffer() const { - return Builder.Buffer; -} + void resolveSelfReferences(llvm::GlobalVariable *GV); +}; -AggregateBuilderBase(ConstantInitBuilder &builder, - AggregateBuilderBase *parent) -: Builder(builder), Parent(parent), Begin(builder.Buffer.size()) { - if (parent) { -assert(!parent->Frozen && "parent already has child builder active"); -parent->Frozen = true; - } else { -assert(!builder.Frozen && "builder already has child builder active"); -builder.Frozen = true; - } -} +/// A concrete base class for struct and array aggregate +/// initializer builders. +class ConstantAggregateBuilderBase { +protected: + ConstantInitBuilderBase &Builder; + ConstantAggregateBuilderBase *Parent; + size_t Begin; + mutable size_t CachedOffsetEnd = 0; + bool Finished = false; + bool Frozen = false; + mutable CharUnits CachedOffsetFromGlobal; -~AggregateBuilderBase() { - assert(Finished && "didn't finish aggregate builder"); -} + llvm::SmallVectorImpl &getBuffer() { +return Builder.Buffer; + } -void markFinished() { - assert(!Frozen && "child builder still active"); - assert(!Finished && "builder already finished"); - Finished = true; - if (Parent) { -assert(Parent->Frozen && - "parent not frozen while child builder active"); -Parent->Frozen = false; - } else { -assert(Builder.Frozen
[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check
aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. Missing documentation changes for the new options. Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:110 +ClassWithSpecialMembers[ID]; +if (find(Members, Kind) == Members.end()) + Members.push_back(Kind); Please qualify the find with std::, here and elsewhere. Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:138 - for (const auto &C : ClassWithSpecialMembers) { -const auto &DefinedSpecialMembers = C.second; +void SpecialMemberFunctionsCheck::checkForMissingMembers(const ClassDefId& ID, +llvm::ArrayRef DefinedSpecialMembers) Formatting Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:140 +llvm::ArrayRef DefinedSpecialMembers) +{ + llvm::SmallVector MissingSpecialMembers; { should go up a line. Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:153 + + auto RequireDtor = [&]() { +if (!HasMember(SpecialMemberFunctionKind::DefaultDestructor) && Can elide the parens. Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:167 +RequireMember(SpecialMemberFunctionKind::CopyAssignment); +if (!AllowMissingMoveFunctions && getLangOpts().CPlusPlus11) { + RequireMember(SpecialMemberFunctionKind::MoveConstructor); This feels like a lot of code duplication. Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h:51 private: + void checkForMissingMembers(const ClassDefId& ID, +llvm::ArrayRef DefinedSpecialMembers); You should run clang-format over the patch; the & should bind to ID. https://reviews.llvm.org/D30610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30592: [clang-tidy] Fix diag message for catch-by-value
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you! https://reviews.llvm.org/D30592 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30599: [ubsan] Extend the nonnull argument check to ObjC
aprantl added inline comments. Comment at: lib/CodeGen/CodeGenFunction.h:308 + /// \brief An abstract representation of regular/ObjC call/message targets. + class AbstractCallee { The \brief is redundant (we use autobrief) and shouldn't be used any more. https://reviews.llvm.org/D30599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r296975 - [index] C++: Improve handling of typedefs as base names in C++ class declarations
Author: akirtzidis Date: Sat Mar 4 11:54:56 2017 New Revision: 296975 URL: http://llvm.org/viewvc/llvm-project?rev=296975&view=rev Log: [index] C++: Improve handling of typedefs as base names in C++ class declarations Report the typedef as reference, and desugar it to report the underlying class as an implicit 'base' reference. Reporting the underlying base class for 'base' relations matches the ObjC handling and leads to a simpler model. Modified: cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp cfe/trunk/test/Index/Core/index-source.cpp Modified: cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp?rev=296975&r1=296974&r2=296975&view=diff == --- cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp (original) +++ cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp Sat Mar 4 11:54:56 2017 @@ -40,18 +40,30 @@ public: bool shouldWalkTypesOfTypeLocs() const { return false; } - bool VisitTypedefTypeLoc(TypedefTypeLoc TL) { -return IndexCtx.handleReference(TL.getTypedefNameDecl(), TL.getNameLoc(), -Parent, ParentDC, SymbolRoleSet(), -Relations); - } - #define TRY_TO(CALL_EXPR) \ do { \ if (!CALL_EXPR) \ return false; \ } while (0) + bool VisitTypedefTypeLoc(TypedefTypeLoc TL) { +if (IsBase) { + SourceLocation Loc = TL.getNameLoc(); + TRY_TO(IndexCtx.handleReference(TL.getTypedefNameDecl(), Loc, + Parent, ParentDC, SymbolRoleSet())); + if (auto *CD = TL.getType()->getAsCXXRecordDecl()) { +TRY_TO(IndexCtx.handleReference(CD, Loc, Parent, ParentDC, +(unsigned)SymbolRole::Implicit, +Relations)); + } +} else { + TRY_TO(IndexCtx.handleReference(TL.getTypedefNameDecl(), TL.getNameLoc(), + Parent, ParentDC, SymbolRoleSet(), + Relations)); +} +return true; + } + bool traverseParamVarHelper(ParmVarDecl *D) { TRY_TO(TraverseNestedNameSpecifierLoc(D->getQualifierLoc())); if (D->getTypeSourceInfo()) Modified: cfe/trunk/test/Index/Core/index-source.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.cpp?rev=296975&r1=296974&r2=296975&view=diff == --- cfe/trunk/test/Index/Core/index-source.cpp (original) +++ cfe/trunk/test/Index/Core/index-source.cpp Sat Mar 4 11:54:56 2017 @@ -1,6 +1,6 @@ // RUN: c-index-test core -print-source-symbols -- %s -std=c++14 -target x86_64-apple-macosx10.7 | FileCheck %s -// CHECK: [[@LINE+1]]:7 | class/C++ | Cls | c:@S@Cls | | Def | rel: 0 +// CHECK: [[@LINE+1]]:7 | class/C++ | Cls | [[Cls_USR:.*]] | | Def | rel: 0 class Cls { // CHECK: [[@LINE+2]]:3 | constructor/C++ | Cls | c:@S@Cls@F@Cls#I# | __ZN3ClsC1Ei | Decl,RelChild | rel: 1 // CHECK-NEXT: RelChild | Cls | c:@S@Cls @@ -11,6 +11,19 @@ class Cls { Cls(Cls &&); }; +// CHECK: [[@LINE+3]]:7 | class/C++ | SubCls1 | [[SubCls1_USR:.*]] | | Def | rel: 0 +// CHECK: [[@LINE+2]]:24 | class/C++ | Cls | [[Cls_USR]] | | Ref,RelBase,RelCont | rel: 1 +// CHECK-NEXT: RelBase,RelCont | SubCls1 | [[SubCls1_USR]] +class SubCls1 : public Cls {}; +// CHECK: [[@LINE+1]]:13 | type-alias/C | ClsAlias | [[ClsAlias_USR:.*]] | | Def | rel: 0 +typedef Cls ClsAlias; +// CHECK: [[@LINE+5]]:7 | class/C++ | SubCls2 | [[SubCls2_USR:.*]] | | Def | rel: 0 +// CHECK: [[@LINE+4]]:24 | type-alias/C | ClsAlias | [[ClsAlias_USR]] | | Ref,RelCont | rel: 1 +// CHECK-NEXT: RelCont | SubCls2 | [[SubCls2_USR]] +// CHECK: [[@LINE+2]]:24 | class/C++ | Cls | [[Cls_USR]] | | Ref,Impl,RelBase,RelCont | rel: 1 +// CHECK-NEXT: RelBase,RelCont | SubCls2 | [[SubCls2_USR]] +class SubCls2 : public ClsAlias {}; + template class TemplCls { // CHECK: [[@LINE-1]]:7 | class(Gen)/C++ | TemplCls | c:@ST>1#T@TemplCls | | Def | rel: 0 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r296974 - [index] ObjC: Improve handling of typedefs as base names in ObjC interface declarations
Author: akirtzidis Date: Sat Mar 4 11:54:53 2017 New Revision: 296974 URL: http://llvm.org/viewvc/llvm-project?rev=296974&view=rev Log: [index] ObjC: Improve handling of typedefs as base names in ObjC interface declarations - Report the typedef reference occurrence - Mark super or protocol references as 'implicit' when they come from a typedef. Modified: cfe/trunk/lib/Index/IndexDecl.cpp cfe/trunk/test/Index/Core/index-source.m Modified: cfe/trunk/lib/Index/IndexDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=296974&r1=296973&r2=296974&view=diff == --- cfe/trunk/lib/Index/IndexDecl.cpp (original) +++ cfe/trunk/lib/Index/IndexDecl.cpp Sat Mar 4 11:54:53 2017 @@ -225,14 +225,17 @@ public: } bool handleReferencedProtocols(const ObjCProtocolList &ProtList, - const ObjCContainerDecl *ContD) { + const ObjCContainerDecl *ContD, + SourceLocation SuperLoc) { ObjCInterfaceDecl::protocol_loc_iterator LI = ProtList.loc_begin(); for (ObjCInterfaceDecl::protocol_iterator I = ProtList.begin(), E = ProtList.end(); I != E; ++I, ++LI) { SourceLocation Loc = *LI; ObjCProtocolDecl *PD = *I; - TRY_TO(IndexCtx.handleReference(PD, Loc, ContD, ContD, - SymbolRoleSet(), + SymbolRoleSet roles{}; + if (Loc == SuperLoc) +roles |= (SymbolRoleSet)SymbolRole::Implicit; + TRY_TO(IndexCtx.handleReference(PD, Loc, ContD, ContD, roles, SymbolRelation{(unsigned)SymbolRole::RelationBaseOf, ContD})); } return true; @@ -241,12 +244,26 @@ public: bool VisitObjCInterfaceDecl(const ObjCInterfaceDecl *D) { if (D->isThisDeclarationADefinition()) { TRY_TO(IndexCtx.handleDecl(D)); + SourceLocation SuperLoc = D->getSuperClassLoc(); if (auto *SuperD = D->getSuperClass()) { -TRY_TO(IndexCtx.handleReference(SuperD, D->getSuperClassLoc(), D, D, -SymbolRoleSet(), +bool hasSuperTypedef = false; +if (auto *TInfo = D->getSuperClassTInfo()) { + if (auto *TT = TInfo->getType()->getAs()) { +if (auto *TD = TT->getDecl()) { + hasSuperTypedef = true; + TRY_TO(IndexCtx.handleReference(TD, SuperLoc, D, D, + SymbolRoleSet())); +} + } +} +SymbolRoleSet superRoles{}; +if (hasSuperTypedef) + superRoles |= (SymbolRoleSet)SymbolRole::Implicit; +TRY_TO(IndexCtx.handleReference(SuperD, SuperLoc, D, D, superRoles, SymbolRelation{(unsigned)SymbolRole::RelationBaseOf, D})); } - TRY_TO(handleReferencedProtocols(D->getReferencedProtocols(), D)); + TRY_TO(handleReferencedProtocols(D->getReferencedProtocols(), D, + SuperLoc)); TRY_TO(IndexCtx.indexDeclContext(D)); } else { return IndexCtx.handleReference(D, D->getLocation(), nullptr, @@ -258,7 +275,8 @@ public: bool VisitObjCProtocolDecl(const ObjCProtocolDecl *D) { if (D->isThisDeclarationADefinition()) { TRY_TO(IndexCtx.handleDecl(D)); - TRY_TO(handleReferencedProtocols(D->getReferencedProtocols(), D)); + TRY_TO(handleReferencedProtocols(D->getReferencedProtocols(), D, + /*superLoc=*/SourceLocation())); TRY_TO(IndexCtx.indexDeclContext(D)); } else { return IndexCtx.handleReference(D, D->getLocation(), nullptr, @@ -306,7 +324,8 @@ public: if (!CategoryLoc.isValid()) CategoryLoc = D->getLocation(); TRY_TO(IndexCtx.handleDecl(D, CategoryLoc)); -TRY_TO(handleReferencedProtocols(D->getReferencedProtocols(), D)); +TRY_TO(handleReferencedProtocols(D->getReferencedProtocols(), D, + /*superLoc=*/SourceLocation())); TRY_TO(IndexCtx.indexDeclContext(D)); return true; } Modified: cfe/trunk/test/Index/Core/index-source.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.m?rev=296974&r1=296973&r2=296974&view=diff == --- cfe/trunk/test/Index/Core/index-source.m (original) +++ cfe/trunk/test/Index/Core/index-source.m Sat Mar 4 11:54:53 2017 @@ -202,20 +202,27 @@ extern int setjmp(jmp_buf); @protocol MyEnumerating @end -// CHECK: [[@LINE+4]]:41 | type-alias/C | MyEnumerator | c:index-source.m@T@MyEnumerator | | Def | rel: 0 +// CHECK: [[@LINE+4]]:41 | type-alias/C | MyEnumerator | [[MyEnumerator_USR:.*]] | | Def | rel: 0 // CHECK: [[@LINE+3]]:26 | protocol/ObjC | MyEnumerating | c:objc(pl)MyEnumerating | | Ref,RelCont | rel: 1 // CHECK: [[@LINE+2]]:9 | class/ObjC | MyGenCls | c:objc(cs)MyGenCls | _OBJC_CLASS_$_MyGenCls | Ref,RelCont | rel: 1 // CHECK: [[
[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check
fgross created this revision. Herald added subscribers: JDevlieghere, nemanjai. Added two options to cppcoreguidelines-special-member-functions to allow a more relaxed checking. With 'AllowSoleDefaultDtor' set to 1, classes with explicitly defaulted destructors but no other special members are not flagged anymore: class foo { // typical scenario: defaulted virtual destructor in base classes virtual ~foo() = default; }; With 'AllowMissingMoveFunctions' set to 1, classes with no move functions at all are not flagged anymore: class bar{ ~bar(); bar(const bar&); bar& operator=(const bar&); }; https://reviews.llvm.org/D30610 Files: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp test/clang-tidy/cppcoreguidelines-special-member-functions.cpp Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp === --- test/clang-tidy/cppcoreguidelines-special-member-functions.cpp +++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp @@ -3,7 +3,12 @@ class DefinesDestructor { ~DefinesDestructor(); }; -// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions] +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions] + +class DefinesDefaultedDestructor { + ~DefinesDefaultedDestructor() = default; +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDefaultedDestructor' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions] class DefinesCopyConstructor { DefinesCopyConstructor(const DefinesCopyConstructor &); Index: test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp === --- test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp +++ test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp @@ -0,0 +1,71 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: [{key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctions, value: 1}, {key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor, value: 1}]}" -- + +class DefinesDestructor { + ~DefinesDestructor(); +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions] + +class DefinesDefaultedDestructor { + ~DefinesDefaultedDestructor() = default; +}; + +class DefinesCopyConstructor { + DefinesCopyConstructor(const DefinesCopyConstructor &); +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor or a copy assignment operator [cppcoreguidelines-special-member-functions] + +class DefinesCopyAssignment { + DefinesCopyAssignment &operator=(const DefinesCopyAssignment &); +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor or a copy constructor [cppcoreguidelines-special-member-functions] + +class DefinesMoveConstructor { + DefinesMoveConstructor(DefinesMoveConstructor &&); +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveConstructor' defines a move constructor but does not define a destructor, a copy constructor, a copy assignment operator or a move assignment operator [cppcoreguidelines-special-member-functions] + +class DefinesMoveAssignment { + DefinesMoveAssignment &operator=(DefinesMoveAssignment &&); +}; +// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveAssignment' defines a move assignment operator but does not define a destructor, a copy constructor, a copy assignment operator or a move constructor [cppcoreguidelines-special-member-functions] +class DefinesNothing { +}; + +class DefinesEverything { + DefinesEverything(const DefinesEverything &); + DefinesEverything &operator=(const DefinesEverything &); + DefinesEverything(DefinesEverything &&); + DefinesEverything &operator=(DefinesEverything &&); + ~DefinesEverything(); +}; + +class DeletesEverything { + DeletesEverything(const DeletesEverything &) = delete; + DeletesEverything
[PATCH] D30607: Replace re module by regex module in run-clang-tidy script
sylvestre.ledru created this revision. Herald added a subscriber: JDevlieghere. Avoid limitation of 100 groups for regular expressions Patch by Manuel Grizonnet reported here: https://github.com/llvm-mirror/clang-tools-extra/pull/9 https://reviews.llvm.org/D30607 Files: clang-tidy/tool/run-clang-tidy.py Index: clang-tidy/tool/run-clang-tidy.py === --- clang-tidy/tool/run-clang-tidy.py +++ clang-tidy/tool/run-clang-tidy.py @@ -39,7 +39,7 @@ import multiprocessing import os import Queue -import re +import regex as re import shutil import subprocess import sys Index: clang-tidy/tool/run-clang-tidy.py === --- clang-tidy/tool/run-clang-tidy.py +++ clang-tidy/tool/run-clang-tidy.py @@ -39,7 +39,7 @@ import multiprocessing import os import Queue -import re +import regex as re import shutil import subprocess import sys ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r296970 - Replaced UserNullMacros with NullMacros in modernize-use-nullptr check docs
Author: sylvestre Date: Sat Mar 4 08:06:26 2017 New Revision: 296970 URL: http://llvm.org/viewvc/llvm-project?rev=296970&view=rev Log: Replaced UserNullMacros with NullMacros in modernize-use-nullptr check docs By patch Peter Goldsborough on https://github.com/llvm-mirror/clang-tools-extra/pull/14 See https://github.com/llvm-mirror/clang-tools-extra/blob/2cd835ee5bcd6c4944d7e30826668ec38db40b38/clang-tidy/modernize/UseNullptrCheck.cpp#L466 Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-nullptr.rst Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-nullptr.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-nullptr.rst?rev=296970&r1=296969&r2=296970&view=diff == --- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-nullptr.rst (original) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-nullptr.rst Sat Mar 4 08:06:26 2017 @@ -39,7 +39,7 @@ transforms to: Options --- -.. option:: UserNullMacros +.. option:: NullMacros Comma-separated list of macro names that will be transformed along with ``NULL``. By default this check will only replace the ``NULL`` macro and will @@ -64,4 +64,4 @@ transforms to: int *p = nullptr; } -if the :option:`UserNullMacros` option is set to ``MY_NULL``. +if the :option:`NullMacros` option is set to ``MY_NULL``. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30592: [clang-tidy] Fix diag message for catch-by-value
fgross updated this revision to Diff 90572. fgross added a comment. Updated test case. https://reviews.llvm.org/D30592 Files: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp Index: test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp === --- test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp +++ test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp @@ -62,7 +62,7 @@ try { testThrowFunc(); } catch (logic_error e) { -// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [misc-throw-by-value-catch-by-reference] +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches by value; should catch by reference instead [misc-throw-by-value-catch-by-reference] } } Index: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp === --- clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp +++ clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp @@ -131,22 +131,24 @@ void ThrowByValueCatchByReferenceCheck::diagnoseCatchLocations( const CXXCatchStmt *catchStmt, ASTContext &context) { - const char *diagMsgCatchReference = "catch handler catches a pointer value; " - "should throw a non-pointer value and " - "catch by reference instead"; if (!catchStmt) return; auto caughtType = catchStmt->getCaughtType(); if (caughtType.isNull()) return; auto *varDecl = catchStmt->getExceptionDecl(); if (const auto *PT = caughtType.getCanonicalType()->getAs()) { +const char *diagMsgCatchReference = "catch handler catches a pointer value; " +"should throw a non-pointer value and " +"catch by reference instead"; // We do not diagnose when catching pointer to strings since we also allow // throwing string literals. if (!PT->getPointeeType()->isAnyCharacterType()) diag(varDecl->getLocStart(), diagMsgCatchReference); } else if (!caughtType->isReferenceType()) { -// If it's not a pointer and not a reference then it must be thrown "by +const char *diagMsgCatchReference = "catch handler catches by value; " +"should catch by reference instead"; +// If it's not a pointer and not a reference then it must be caught "by // value". In this case we should emit a diagnosis message unless the type // is trivial. if (!caughtType.isTrivialType(context)) Index: test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp === --- test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp +++ test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp @@ -62,7 +62,7 @@ try { testThrowFunc(); } catch (logic_error e) { -// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [misc-throw-by-value-catch-by-reference] +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches by value; should catch by reference instead [misc-throw-by-value-catch-by-reference] } } Index: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp === --- clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp +++ clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp @@ -131,22 +131,24 @@ void ThrowByValueCatchByReferenceCheck::diagnoseCatchLocations( const CXXCatchStmt *catchStmt, ASTContext &context) { - const char *diagMsgCatchReference = "catch handler catches a pointer value; " - "should throw a non-pointer value and " - "catch by reference instead"; if (!catchStmt) return; auto caughtType = catchStmt->getCaughtType(); if (caughtType.isNull()) return; auto *varDecl = catchStmt->getExceptionDecl(); if (const auto *PT = caughtType.getCanonicalType()->getAs()) { +const char *diagMsgCatchReference = "catch handler catches a pointer value; " +"should throw a non-pointer value and " +"catch by reference instead"; // We do not diagnose when catching pointer to strings since we also allow // throwing string literals. if (!PT->getPointeeType()->isAnyCharacterType()) diag(varDecl->getLocStart(), diagMsgCatchReference); } else if (!caughtType->isReferenceType()) { -// If it's not a pointer and not a reference then it must be thrown "by +const char *diagMsgCatchReference = "catch handler catches by value;
[libcxx] r296965 - Fix nonsense comment
Author: ericwf Date: Sat Mar 4 06:28:12 2017 New Revision: 296965 URL: http://llvm.org/viewvc/llvm-project?rev=296965&view=rev Log: Fix nonsense comment Modified: libcxx/trunk/utils/libcxx/test/config.py Modified: libcxx/trunk/utils/libcxx/test/config.py URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/utils/libcxx/test/config.py?rev=296965&r1=296964&r2=296965&view=diff == --- libcxx/trunk/utils/libcxx/test/config.py (original) +++ libcxx/trunk/utils/libcxx/test/config.py Sat Mar 4 06:28:12 2017 @@ -401,7 +401,8 @@ class Configuration(object): if self.is_windows: self.config.available_features.add('windows') -# Attempt to detect the glibc version by querying +# Attempt to detect the glibc version by querying for __GLIBC__ +# in 'features.h'. macros = self.cxx.dumpMacros(flags=['-include', 'features.h']) if macros is not None and '__GLIBC__' in macros: maj_v, min_v = (macros['__GLIBC__'], macros['__GLIBC_MINOR__']) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30547: [clang-tidy] Forwarding reference overload in constructors
JonasToth added inline comments. Comment at: docs/clang-tidy/checks/misc-forwarding-reference-overload.rst:38 +constructors. We suppress warnings if the copy and the move constructors are both +disabled (deleted or private), because there is nothing the prefect forwarding +constructor could hide in this case. We also suppress warnings for constructors typo: prefect -> perfect Comment at: test/clang-tidy/misc-forwarding-reference-overload.cpp:21 + Person(T &&n); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'Person' can hide copy and move constructors [misc-forwarding-reference-overload] + leanil wrote: > JonasToth wrote: > > could the check output a note when there was a userdefined constructor and > > point to that one if it gets hidden? that would make it clearer, what and > > how the problem occurs. > What do you mean by "if it gets hidden"? Should I look for specific calls > that are hijacked by the perfect forwarding ctor? Or just make a note on the > user defined copy/move ctors any time I produce a warning (without looking at > actual calls)? > I think the former would be quite tricky to do. > Also, what if the perfect forwarding ctor hides the compiler generated > copy/move? Should I still make a note (maybe pointing to the class itself)? ``` class Person { public: // perfect forwarding ctor template explicit Person(T&& n) {} // (possibly compiler generated) copy ctor Person(const Person& rhs); }; ``` a note pointing to a user defined ctor would be good i think. with compiler generated operations the warning is clear. emitting such a note would be a minor feature and if too complicated not worth it, i guess. Repository: rL LLVM https://reviews.llvm.org/D30547 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30547: [clang-tidy] Forwarding reference overload in constructors
leanil marked 6 inline comments as done. leanil added inline comments. Comment at: test/clang-tidy/misc-forwarding-reference-overload.cpp:21 + Person(T &&n); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'Person' can hide copy and move constructors [misc-forwarding-reference-overload] + JonasToth wrote: > could the check output a note when there was a userdefined constructor and > point to that one if it gets hidden? that would make it clearer, what and how > the problem occurs. What do you mean by "if it gets hidden"? Should I look for specific calls that are hijacked by the perfect forwarding ctor? Or just make a note on the user defined copy/move ctors any time I produce a warning (without looking at actual calls)? I think the former would be quite tricky to do. Also, what if the perfect forwarding ctor hides the compiler generated copy/move? Should I still make a note (maybe pointing to the class itself)? Repository: rL LLVM https://reviews.llvm.org/D30547 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30547: [clang-tidy] Forwarding reference overload in constructors
leanil updated this revision to Diff 90569. leanil added a comment. Added changes according to the comments. Repository: rL LLVM https://reviews.llvm.org/D30547 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp clang-tidy/misc/ForwardingReferenceOverloadCheck.h clang-tidy/misc/MiscTidyModule.cpp docs/ReleaseNotes.rst docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/misc-forwarding-reference-overload.rst test/clang-tidy/misc-forwarding-reference-overload.cpp Index: test/clang-tidy/misc-forwarding-reference-overload.cpp === --- /dev/null +++ test/clang-tidy/misc-forwarding-reference-overload.cpp @@ -0,0 +1,80 @@ +// RUN: %check_clang_tidy %s misc-forwarding-reference-overload %t + +namespace std { +template +struct enable_if {}; + +template +struct enable_if { typedef T type; }; + +template +using enable_if_t = typename enable_if::type; +} + +template +constexpr bool just_true = true; + +class Person { +public: + template + Person(T &&n); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'Person' can hide copy and move constructors [misc-forwarding-reference-overload] + + template + Person(T &&n, int I = 5, ...); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'Person' can hide copy and move constructors [misc-forwarding-reference-overload] +}; + +template +class Person2 { +public: + // Two parameters without default value, can't act as copy / move constructor. + template + Person2(T &&n, V &&m, int i = 5, ...); + + // Guarded with enable_if. + template + Person2(T &&n, int i = 5, std::enable_if_t a = 5, ...); + + // Guarded with enable_if. + template ::type> + Person2(T &&n); + + // Guarded with enable_if. + template + Person2(T &&n, std::enable_if_t, int> a = 1); + + // Guarded with enable_if. + template , int>::type *> + Person2(T &&n, double d = 0.0); + + // Not a forwarding reference parameter. + template + Person2(const T &&n); + + // Not a forwarding reference parameter. + Person2(int &&x); + + // Two parameters without default value, can't act as copy / move constructor. + template + Person2(T &&n, int x); + + // Not a forwarding reference parameter. + template + Person2(U &&n); +}; + +// Copy and move constructors are both disabled. +class Person3 { +public: + template + Person3(T &&n); + + template + Person3(T &&n, int I = 5, ...); + + Person3(const Person3 &rhs) = delete; + +private: + Person3(Person3 &&rhs); +}; Index: docs/clang-tidy/checks/misc-forwarding-reference-overload.rst === --- /dev/null +++ docs/clang-tidy/checks/misc-forwarding-reference-overload.rst @@ -0,0 +1,52 @@ +.. title:: clang-tidy - misc-forwarding-reference-overload + +misc-forwarding-reference-overload +== + +The check looks for perfect forwarding constructors that can hide copy or move +constructors. If a non const lvalue reference is passed to the constructor, the +forwarding reference parameter will be a better match than the const reference +parameter of the copy constructor, so the perfect forwarding constructor will be +called, which can be confusing. +For detailed description of this issue see: Scott Meyers, Effective Modern C++, +Item 26. + +Consider the following example: + + .. code-block:: c++ + +class Person { +public: + // C1: perfect forwarding ctor + template + explicit Person(T&& n) {} + + // C2: perfect forwarding ctor with parameter default value + template + explicit Person(T&& n, int x = 1) {} + + // C3: perfect forwarding ctor guarded with enable_if + template,void>> + explicit Person(T&& n) {} + + // (possibly compiler generated) copy ctor + Person(const Person& rhs); +}; + +The check warns for constructors C1 and C2, because those can hide copy and move +constructors. We suppress warnings if the copy and the move constructors are both +disabled (deleted or private), because there is nothing the prefect forwarding +constructor could hide in this case. We also suppress warnings for constructors +like C3 that are guarded with an enable_if, assuming the programmer was aware of +the possible hiding. + +Background +-- + +For deciding whether a constructor is guarded with enable_if, we consider the +default values of the type parameters and the types of the contructor +parameters. If any part of these types contains "enable_if" in its name, we +assume the constructor is guarded. + + + Index: docs/clang-tidy/checks/list.rst === --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -64,6 +64,7 @@ misc-definitions-in-headers misc-fold-init-type misc-forward-declaration-namespace + misc-forwarding-reference-overload misc-inacc