[PATCH] D16178: [analyzer] A quick fix on top of D12901/r257464
NoQ created this revision. NoQ added reviewers: pgousseau, zaks.anna, dcoughlin, xazax.hun. NoQ added a subscriber: cfe-commits. Sorry for being a bit slow, i should have had a look at the review earlier; i noticed some stuff after the recent patch by Pierre Gousseau was committed. 1. There's an off-by-one in the comparison `evalBinOp` in `evalIntegralCast`, the patch attached to this revision fixes it, and tests it via bool-assignment checker. 2. There's also a FIXME test for the same checker, which is a regression after this patch (used to throw a warning, now it doesn't); seems not as bad as the crash, so it can probably be addressed later. 3. I noticed that it's still possible to trigger the crash with the following test, not sure what to do with it: ``` void f1(long foo) { unsigned index = -1; if (index < foo - 1) index = foo; if (index + 1 == 0) {} } ``` So my best idea is to push this quick fix for off-by-one and then think where to go further. Probably we need to teach `RangeConstraintManager` to work with `SymbolCast`'s correctly (eg. understand how their range is related to a parent symbol's range). Eventually, once it gets better, we'd probably want to change the following in `evalIntegralCast`: ``` std::tie(IsNotTruncated, IsTruncated) = state->assume(CompVal); - if (!IsNotTruncated && IsTruncated) { + if (IsTruncated) { ``` but right now it breaks tests (not only bool-assignment, but also other checkers that start failing can probably be shown to loose warnings after this patch, but i didn't have a look at this yet, so not sure, just hand-waving). Anyway, generally, overally, i think r257464 is a step in the very right direction if we want to work with casts better, and i was actually very happy to see it went that way :) http://reviews.llvm.org/D16178 Files: lib/StaticAnalyzer/Core/SValBuilder.cpp test/Analysis/bool-assignment.c Index: test/Analysis/bool-assignment.c === --- test/Analysis/bool-assignment.c +++ test/Analysis/bool-assignment.c @@ -42,6 +42,15 @@ BOOL x = y; // expected-warning {{Assignment of a non-Boolean value}} return; } + if (y > 200 && y < 250) { +// FIXME: Currently we are loosing this warning due to a SymbolCast in RHS. +BOOL x = y; // no-warning +return; + } + if (y >= 127 && y < 150) { +BOOL x = y; // expected-warning{{Assignment of a non-Boolean value}} +return; + } if (y > 1) { BOOL x = y; // expected-warning {{Assignment of a non-Boolean value}} return; Index: lib/StaticAnalyzer/Core/SValBuilder.cpp === --- lib/StaticAnalyzer/Core/SValBuilder.cpp +++ lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -451,7 +451,7 @@ NonLoc FromVal = val.castAs(); QualType CmpTy = getConditionType(); NonLoc CompVal = - evalBinOpNN(state, BO_LT, FromVal, ToTypeMaxVal, CmpTy).castAs(); + evalBinOpNN(state, BO_LE, FromVal, ToTypeMaxVal, CmpTy).castAs(); ProgramStateRef IsNotTruncated, IsTruncated; std::tie(IsNotTruncated, IsTruncated) = state->assume(CompVal); if (!IsNotTruncated && IsTruncated) { Index: test/Analysis/bool-assignment.c === --- test/Analysis/bool-assignment.c +++ test/Analysis/bool-assignment.c @@ -42,6 +42,15 @@ BOOL x = y; // expected-warning {{Assignment of a non-Boolean value}} return; } + if (y > 200 && y < 250) { +// FIXME: Currently we are loosing this warning due to a SymbolCast in RHS. +BOOL x = y; // no-warning +return; + } + if (y >= 127 && y < 150) { +BOOL x = y; // expected-warning{{Assignment of a non-Boolean value}} +return; + } if (y > 1) { BOOL x = y; // expected-warning {{Assignment of a non-Boolean value}} return; Index: lib/StaticAnalyzer/Core/SValBuilder.cpp === --- lib/StaticAnalyzer/Core/SValBuilder.cpp +++ lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -451,7 +451,7 @@ NonLoc FromVal = val.castAs(); QualType CmpTy = getConditionType(); NonLoc CompVal = - evalBinOpNN(state, BO_LT, FromVal, ToTypeMaxVal, CmpTy).castAs(); + evalBinOpNN(state, BO_LE, FromVal, ToTypeMaxVal, CmpTy).castAs(); ProgramStateRef IsNotTruncated, IsTruncated; std::tie(IsNotTruncated, IsTruncated) = state->assume(CompVal); if (!IsNotTruncated && IsTruncated) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r257754 - PR25910: clang allows two var definitions with the same mangled name
Author: asbokhan Date: Thu Jan 14 04:41:16 2016 New Revision: 257754 URL: http://llvm.org/viewvc/llvm-project?rev=257754&view=rev Log: PR25910: clang allows two var definitions with the same mangled name Proper diagnostic and resolution of mangled names' conflicts in variables. When there is a declaration and a definition using the same name but different types, we emit what is in the definition. When there are two conflicting definitions, we issue an error. Differential Revision: http://reviews.llvm.org/D15686 Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp cfe/trunk/lib/CodeGen/CodeGenModule.h cfe/trunk/test/CodeGenCXX/duplicate-mangled-name.cpp Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=257754&r1=257753&r2=257754&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu Jan 14 04:41:16 2016 @@ -1244,19 +1244,23 @@ void CodeGenModule::EmitDeferred() { for (DeferredGlobal &G : CurDeclsToEmit) { GlobalDecl D = G.GD; -llvm::GlobalValue *GV = G.GV; G.GV = nullptr; // We should call GetAddrOfGlobal with IsForDefinition set to true in order // to get GlobalValue with exactly the type we need, not something that // might had been created for another decl with the same mangled name but // different type. -// FIXME: Support for variables is not implemented yet. -if (isa(D.getDecl())) - GV = cast(GetAddrOfGlobal(D, /*IsForDefinition=*/true)); -else - if (!GV) -GV = GetGlobalValue(getMangledName(D)); +llvm::GlobalValue *GV = dyn_cast( +GetAddrOfGlobal(D, /*IsForDefinition=*/true)); + +// In case of different address spaces, we may still get a cast, even with +// IsForDefinition equal to true. Query mangled names table to get +// GlobalValue. +if (!GV) + GV = GetGlobalValue(getMangledName(D)); + +// Make sure GetGlobalValue returned non-null. +assert(GV); // Check to see if we've already emitted this. This is necessary // for a couple of reasons: first, decls can end up in the @@ -1264,7 +1268,7 @@ void CodeGenModule::EmitDeferred() { // up with definitions in unusual ways (e.g. by an extern inline // function acquiring a strong function redefinition). Just // ignore these cases. -if (GV && !GV->isDeclaration()) +if (!GV->isDeclaration()) continue; // Otherwise, emit the definition and move on to the next one. @@ -1730,7 +1734,7 @@ void CodeGenModule::EmitGlobalDefinition } if (const auto *VD = dyn_cast(D)) -return EmitGlobalVarDefinition(VD); +return EmitGlobalVarDefinition(VD, !VD->hasDefinition()); llvm_unreachable("Invalid argument to EmitGlobalDefinition()"); } @@ -1771,8 +1775,8 @@ CodeGenModule::GetOrCreateLLVMFunction(S // error. if (IsForDefinition && !Entry->isDeclaration()) { GlobalDecl OtherGD; - // Check that GD is not yet in ExplicitDefinitions is required to make - // sure that we issue an error only once. + // Check that GD is not yet in DiagnosedConflictingDefinitions is required + // to make sure that we issue an error only once. if (lookupRepresentativeDecl(MangledName, OtherGD) && (GD.getCanonicalDecl().getDecl() != OtherGD.getCanonicalDecl().getDecl()) && @@ -1982,10 +1986,15 @@ bool CodeGenModule::isTypeConstant(QualT /// /// If D is non-null, it specifies a decl that correspond to this. This is used /// to set the attributes on the global when it is first created. +/// +/// If IsForDefinition is true, it is guranteed that an actual global with +/// type Ty will be returned, not conversion of a variable with the same +/// mangled name but some other type. llvm::Constant * CodeGenModule::GetOrCreateLLVMGlobal(StringRef MangledName, llvm::PointerType *Ty, - const VarDecl *D) { + const VarDecl *D, + bool IsForDefinition) { // Lookup the entry, lazily creating it if necessary. llvm::GlobalValue *Entry = GetGlobalValue(MangledName); if (Entry) { @@ -2001,11 +2010,34 @@ CodeGenModule::GetOrCreateLLVMGlobal(Str if (Entry->getType() == Ty) return Entry; +// If there are two attempts to define the same mangled name, issue an +// error. +if (IsForDefinition && !Entry->isDeclaration()) { + GlobalDecl OtherGD; + const VarDecl *OtherD; + + // Check that D is not yet in DiagnosedConflictingDefinitions is required + // to make sure that we issue an error only once. + if (lookupRepresentativeDecl(MangledName, OtherGD) && + (D->getCanonicalDecl() != OtherGD.getCanonicalDecl().getDecl()) && +
Re: [PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name
andreybokhanko added a comment. @tra, @rnk, @rjmccall, thanks for the review! Yours, Andrey Repository: rL LLVM http://reviews.llvm.org/D15686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name
This revision was automatically updated to reflect the committed changes. Closed by commit rL257754: PR25910: clang allows two var definitions with the same mangled name (authored by asbokhan). Changed prior to commit: http://reviews.llvm.org/D15686?vs=44425&id=44845#toc Repository: rL LLVM http://reviews.llvm.org/D15686 Files: cfe/trunk/lib/CodeGen/CodeGenModule.cpp cfe/trunk/lib/CodeGen/CodeGenModule.h cfe/trunk/test/CodeGenCXX/duplicate-mangled-name.cpp Index: cfe/trunk/lib/CodeGen/CodeGenModule.h === --- cfe/trunk/lib/CodeGen/CodeGenModule.h +++ cfe/trunk/lib/CodeGen/CodeGenModule.h @@ -696,11 +696,14 @@ unsigned GetGlobalVarAddressSpace(const VarDecl *D, unsigned AddrSpace); /// Return the llvm::Constant for the address of the given global variable. - /// If Ty is non-null and if the global doesn't exist, then it will be greated + /// If Ty is non-null and if the global doesn't exist, then it will be created /// with the specified type instead of whatever the normal requested type - /// would be. + /// would be. If IsForDefinition is true, it is guranteed that an actual + /// global with type Ty will be returned, not conversion of a variable with + /// the same mangled name but some other type. llvm::Constant *GetAddrOfGlobalVar(const VarDecl *D, - llvm::Type *Ty = nullptr); + llvm::Type *Ty = nullptr, + bool IsForDefinition = false); /// Return the address of the given function. If Ty is non-null, then this /// function will use the specified type if it has to create it. @@ -1136,7 +1139,8 @@ llvm::Constant *GetOrCreateLLVMGlobal(StringRef MangledName, llvm::PointerType *PTy, -const VarDecl *D); +const VarDecl *D, +bool IsForDefinition = false); void setNonAliasAttributes(const Decl *D, llvm::GlobalObject *GO); @@ -1147,7 +1151,7 @@ void EmitGlobalDefinition(GlobalDecl D, llvm::GlobalValue *GV = nullptr); void EmitGlobalFunctionDefinition(GlobalDecl GD, llvm::GlobalValue *GV); - void EmitGlobalVarDefinition(const VarDecl *D); + void EmitGlobalVarDefinition(const VarDecl *D, bool IsTentative = false); void EmitAliasDefinition(GlobalDecl GD); void EmitObjCPropertyImplementations(const ObjCImplementationDecl *D); void EmitObjCIvarInitializations(ObjCImplementationDecl *D); Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp === --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp @@ -1244,27 +1244,31 @@ for (DeferredGlobal &G : CurDeclsToEmit) { GlobalDecl D = G.GD; -llvm::GlobalValue *GV = G.GV; G.GV = nullptr; // We should call GetAddrOfGlobal with IsForDefinition set to true in order // to get GlobalValue with exactly the type we need, not something that // might had been created for another decl with the same mangled name but // different type. -// FIXME: Support for variables is not implemented yet. -if (isa(D.getDecl())) - GV = cast(GetAddrOfGlobal(D, /*IsForDefinition=*/true)); -else - if (!GV) -GV = GetGlobalValue(getMangledName(D)); +llvm::GlobalValue *GV = dyn_cast( +GetAddrOfGlobal(D, /*IsForDefinition=*/true)); + +// In case of different address spaces, we may still get a cast, even with +// IsForDefinition equal to true. Query mangled names table to get +// GlobalValue. +if (!GV) + GV = GetGlobalValue(getMangledName(D)); + +// Make sure GetGlobalValue returned non-null. +assert(GV); // Check to see if we've already emitted this. This is necessary // for a couple of reasons: first, decls can end up in the // deferred-decls queue multiple times, and second, decls can end // up with definitions in unusual ways (e.g. by an extern inline // function acquiring a strong function redefinition). Just // ignore these cases. -if (GV && !GV->isDeclaration()) +if (!GV->isDeclaration()) continue; // Otherwise, emit the definition and move on to the next one. @@ -1730,7 +1734,7 @@ } if (const auto *VD = dyn_cast(D)) -return EmitGlobalVarDefinition(VD); +return EmitGlobalVarDefinition(VD, !VD->hasDefinition()); llvm_unreachable("Invalid argument to EmitGlobalDefinition()"); } @@ -1771,8 +1775,8 @@ // error. if (IsForDefinition && !Entry->isDeclaration()) { GlobalDecl OtherGD; - // Check that GD is not yet in ExplicitDefinitions is required to make - // sure that we issue an error only once. + // Check that GD is not yet in DiagnosedConflictingDefinitions is required + // to make sure that
Re: [PATCH] D16138: Correct setting of UserLabelPrefix for MCU target
andreybokhanko added a comment. @rafael, thank you! In http://reviews.llvm.org/D16138#325739, @rafael wrote: > LGTM, but could you change the default in a followup commit? TargetInfo > should really be setting it to "" since it is far more common than "_". Sure, will do. Yours, Andrey http://reviews.llvm.org/D16138 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16179: [clang-tidy] Handle decayed types and other improvements in VirtualNearMiss check.
xazax.hun created this revision. xazax.hun added reviewers: alexfh, congliu. xazax.hun added subscribers: cfe-commits, dkrupp. Herald added subscribers: rengolin, aemerson. First of all thank you for this great check! In this patch I propose one small cleanup and two changes in the behaviour. The first is to desugar decayed types in the arguments of functions. This way it is possible to detect override candidates when a method that has a pointer as an argument tries to override a method that has an array as an argument that is decayed to a pointer. The other change is not to take visibility into account since it is perfectly valid to override a public method with a private one and such code exists. I have one more proposal that is not implemented in this patch yet: it is a common mistake to not to override a method because the developer forget to add the qualifiers. What about a configuration option to also report near misses when only a qualifier is missing? What do you think? http://reviews.llvm.org/D16179 Files: clang-tidy/misc/VirtualNearMissCheck.cpp test/clang-tidy/misc-virtual-near-miss.cpp Index: test/clang-tidy/misc-virtual-near-miss.cpp === --- test/clang-tidy/misc-virtual-near-miss.cpp +++ test/clang-tidy/misc-virtual-near-miss.cpp @@ -36,6 +36,7 @@ static void method(); virtual int method(int argc, const char **argv); virtual int method(int argc) const; + virtual int decay(const char *str); }; class Child : private Father, private Mother { @@ -60,6 +61,10 @@ virtual Derived &&generat(); // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::generat' has {{.*}} 'Father::generate' + int decaz(const char str[]); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::decaz' has {{.*}} 'Mother::decay' + private: - void funk(); // Should not warn: access qualifers don't match. + void funk(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::funk' has {{.*}} 'Father::func' }; Index: clang-tidy/misc/VirtualNearMissCheck.cpp === --- clang-tidy/misc/VirtualNearMissCheck.cpp +++ clang-tidy/misc/VirtualNearMissCheck.cpp @@ -52,16 +52,10 @@ const CXXRecordDecl *BRD, *DRD; // Both types must be pointers or references to classes. - if (const auto *DerivedPT = DerivedReturnTy->getAs()) { -if (const auto *BasePT = BaseReturnTy->getAs()) { - DTy = DerivedPT->getPointeeType(); - BTy = BasePT->getPointeeType(); -} - } else if (const auto *DerivedRT = DerivedReturnTy->getAs()) { -if (const auto *BaseRT = BaseReturnTy->getAs()) { - DTy = DerivedRT->getPointeeType(); - BTy = BaseRT->getPointeeType(); -} + if ((BaseReturnTy->isPointerType() && DerivedReturnTy->isPointerType()) || + (BaseReturnTy->isReferenceType() && DerivedReturnTy->isReferenceType())) { +DTy = DerivedReturnTy->getPointeeType(); +BTy = BaseReturnTy->getPointeeType(); } // The return types aren't either both pointers or references to a class type. @@ -116,6 +110,13 @@ return true; } +/// \returns decayed type for arrays and functions. +static QualType getDecayedType(QualType Type) { + if (const auto *Decayed = Type->getAs()) +return Decayed->getDecayedType(); + return Type; +} + /// \returns true if the param types are the same. static bool checkParamTypes(const CXXMethodDecl *BaseMD, const CXXMethodDecl *DerivedMD) { @@ -125,8 +126,8 @@ return false; for (unsigned I = 0; I < NumParamA; I++) { -if (BaseMD->getParamDecl(I)->getType() != -DerivedMD->getParamDecl(I)->getType()) +if (getDecayedType(BaseMD->getParamDecl(I)->getType()) != +getDecayedType(DerivedMD->getParamDecl(I)->getType())) return false; } return true; @@ -143,9 +144,6 @@ if (BaseMD->isStatic() != DerivedMD->isStatic()) return false; - if (BaseMD->getAccess() != DerivedMD->getAccess()) -return false; - if (BaseMD->getType() == DerivedMD->getType()) return true; Index: test/clang-tidy/misc-virtual-near-miss.cpp === --- test/clang-tidy/misc-virtual-near-miss.cpp +++ test/clang-tidy/misc-virtual-near-miss.cpp @@ -36,6 +36,7 @@ static void method(); virtual int method(int argc, const char **argv); virtual int method(int argc) const; + virtual int decay(const char *str); }; class Child : private Father, private Mother { @@ -60,6 +61,10 @@ virtual Derived &&generat(); // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::generat' has {{.*}} 'Father::generate' + int decaz(const char str[]); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::decaz' has {{.*}} 'Mother::decay' + private: - void funk(); // Should not warn: access qualifers don't match. + void funk(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning:
Re: [PATCH] D16138: Correct setting of UserLabelPrefix for MCU target
This revision was automatically updated to reflect the committed changes. Closed by commit rL257756: Correct setting of UserLabelPrefix for MCU target. (authored by asbokhan). Changed prior to commit: http://reviews.llvm.org/D16138?vs=44726&id=44849#toc Repository: rL LLVM http://reviews.llvm.org/D16138 Files: cfe/trunk/lib/Basic/Targets.cpp cfe/trunk/test/Preprocessor/elfiamcu-predefines.c Index: cfe/trunk/test/Preprocessor/elfiamcu-predefines.c === --- cfe/trunk/test/Preprocessor/elfiamcu-predefines.c +++ cfe/trunk/test/Preprocessor/elfiamcu-predefines.c @@ -1,5 +1,6 @@ // RUN: %clang_cc1 -E -dM -triple i586-intel-elfiamcu | FileCheck %s +// CHECK: #define __USER_LABEL_PREFIX__ {{$}} // CHECK: #define __iamcu // CHECK: #define __iamcu__ Index: cfe/trunk/lib/Basic/Targets.cpp === --- cfe/trunk/lib/Basic/Targets.cpp +++ cfe/trunk/lib/Basic/Targets.cpp @@ -3899,6 +3899,7 @@ MCUX86_32TargetInfo(const llvm::Triple &Triple) : X86_32TargetInfo(Triple) { LongDoubleWidth = 64; LongDoubleFormat = &llvm::APFloat::IEEEdouble; +UserLabelPrefix = ""; } CallingConvCheckResult checkCallingConvention(CallingConv CC) const override { Index: cfe/trunk/test/Preprocessor/elfiamcu-predefines.c === --- cfe/trunk/test/Preprocessor/elfiamcu-predefines.c +++ cfe/trunk/test/Preprocessor/elfiamcu-predefines.c @@ -1,5 +1,6 @@ // RUN: %clang_cc1 -E -dM -triple i586-intel-elfiamcu | FileCheck %s +// CHECK: #define __USER_LABEL_PREFIX__ {{$}} // CHECK: #define __iamcu // CHECK: #define __iamcu__ Index: cfe/trunk/lib/Basic/Targets.cpp === --- cfe/trunk/lib/Basic/Targets.cpp +++ cfe/trunk/lib/Basic/Targets.cpp @@ -3899,6 +3899,7 @@ MCUX86_32TargetInfo(const llvm::Triple &Triple) : X86_32TargetInfo(Triple) { LongDoubleWidth = 64; LongDoubleFormat = &llvm::APFloat::IEEEdouble; +UserLabelPrefix = ""; } CallingConvCheckResult checkCallingConvention(CallingConv CC) const override { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r257756 - Correct setting of UserLabelPrefix for MCU target.
Author: asbokhan Date: Thu Jan 14 04:59:36 2016 New Revision: 257756 URL: http://llvm.org/viewvc/llvm-project?rev=257756&view=rev Log: Correct setting of UserLabelPrefix for MCU target. Differential Revision: http://reviews.llvm.org/D16138 Modified: cfe/trunk/lib/Basic/Targets.cpp cfe/trunk/test/Preprocessor/elfiamcu-predefines.c Modified: cfe/trunk/lib/Basic/Targets.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=257756&r1=257755&r2=257756&view=diff == --- cfe/trunk/lib/Basic/Targets.cpp (original) +++ cfe/trunk/lib/Basic/Targets.cpp Thu Jan 14 04:59:36 2016 @@ -3899,6 +3899,7 @@ public: MCUX86_32TargetInfo(const llvm::Triple &Triple) : X86_32TargetInfo(Triple) { LongDoubleWidth = 64; LongDoubleFormat = &llvm::APFloat::IEEEdouble; +UserLabelPrefix = ""; } CallingConvCheckResult checkCallingConvention(CallingConv CC) const override { Modified: cfe/trunk/test/Preprocessor/elfiamcu-predefines.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/elfiamcu-predefines.c?rev=257756&r1=257755&r2=257756&view=diff == --- cfe/trunk/test/Preprocessor/elfiamcu-predefines.c (original) +++ cfe/trunk/test/Preprocessor/elfiamcu-predefines.c Thu Jan 14 04:59:36 2016 @@ -1,5 +1,6 @@ // RUN: %clang_cc1 -E -dM -triple i586-intel-elfiamcu | FileCheck %s +// CHECK: #define __USER_LABEL_PREFIX__ {{$}} // CHECK: #define __iamcu // CHECK: #define __iamcu__ ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15914: [OpenCL] Pipe builtin functions
pekka.jaaskelainen added a comment. OK. Seems the LLVM 3.8 branching was done, it might be hard to get this in, but should we try and ask the release manager? Comment at: lib/CodeGen/CGBuiltin.cpp:1981 @@ +1980,3 @@ + const char *Name = + (BuiltinID == Builtin::BIread_pipe) ? "read_pipe_2" : "write_pipe_2"; + // Re-Creating the function type for this call, since the original type Shouldn't these have underscores, because they are not reserved OpenCL keywords/builtins? Cannot user functions be called read_pipe_2 or write_pipe_2? http://reviews.llvm.org/D15914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11182: [OPENMP 4.0] Initial support for 'omp declare reduction' construct.
ABataev updated this revision to Diff 44848. ABataev added a comment. Updated to latest version + some fixes and improvements http://reviews.llvm.org/D11182 Files: include/clang/AST/DeclBase.h include/clang/AST/DeclCXX.h include/clang/AST/DeclOpenMP.h include/clang/AST/RecursiveASTVisitor.h include/clang/Basic/DeclNodes.td include/clang/Basic/DiagnosticParseKinds.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/OpenMPKinds.def include/clang/Parse/Parser.h include/clang/Sema/ScopeInfo.h include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/CXXInheritance.cpp lib/AST/Decl.cpp lib/AST/DeclBase.cpp lib/AST/DeclOpenMP.cpp lib/AST/DeclPrinter.cpp lib/AST/ItaniumMangle.cpp lib/AST/MicrosoftMangle.cpp lib/Basic/OpenMPKinds.cpp lib/CodeGen/CGDecl.cpp lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h lib/Parse/ParseDecl.cpp lib/Parse/ParseDeclCXX.cpp lib/Parse/ParseOpenMP.cpp lib/Parse/Parser.cpp lib/Sema/ScopeInfo.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaLookup.cpp lib/Sema/SemaOpenMP.cpp lib/Sema/SemaTemplateInstantiateDecl.cpp lib/Serialization/ASTCommon.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriterDecl.cpp test/OpenMP/declare_reduction_ast_print.c test/OpenMP/declare_reduction_ast_print.cpp test/OpenMP/declare_reduction_messages.c test/OpenMP/declare_reduction_messages.cpp tools/libclang/CIndex.cpp Index: lib/Serialization/ASTWriterDecl.cpp === --- lib/Serialization/ASTWriterDecl.cpp +++ lib/Serialization/ASTWriterDecl.cpp @@ -131,6 +131,7 @@ void VisitObjCPropertyDecl(ObjCPropertyDecl *D); void VisitObjCPropertyImplDecl(ObjCPropertyImplDecl *D); void VisitOMPThreadPrivateDecl(OMPThreadPrivateDecl *D); +void VisitOMPDeclareReductionDecl(OMPDeclareReductionDecl *D); /// Add an Objective-C type parameter list to the given record. void AddObjCTypeParamList(ObjCTypeParamList *typeParams) { @@ -1628,6 +1629,16 @@ Code = serialization::DECL_OMP_THREADPRIVATE; } +void ASTDeclWriter::VisitOMPDeclareReductionDecl(OMPDeclareReductionDecl *D) { + VisitNamedDecl(D); + Writer.AddSourceLocation(D->getLocStart(), Record); + Writer.AddStmt(D->getCombiner()); + Writer.AddStmt(D->getInitializer()); + Writer.AddDeclRef(D->getPrevDeclInScope(), Record); + Writer.AddTypeRef(D->getType(), Record); + Code = serialization::DECL_OMP_DECLARE_REDUCTION; +} + //===--===// // ASTWriter Implementation //===--===// Index: lib/Serialization/ASTReaderDecl.cpp === --- lib/Serialization/ASTReaderDecl.cpp +++ lib/Serialization/ASTReaderDecl.cpp @@ -350,6 +350,7 @@ void VisitObjCPropertyDecl(ObjCPropertyDecl *D); void VisitObjCPropertyImplDecl(ObjCPropertyImplDecl *D); void VisitOMPThreadPrivateDecl(OMPThreadPrivateDecl *D); +void VisitOMPDeclareReductionDecl(OMPDeclareReductionDecl *D); /// We've merged the definition \p MergedDef into the existing definition /// \p Def. Ensure that \p Def is made visible whenever \p MergedDef is made @@ -2360,6 +2361,15 @@ D->setVars(Vars); } +void ASTDeclReader::VisitOMPDeclareReductionDecl(OMPDeclareReductionDecl *D) { + VisitNamedDecl(D); + D->setLocation(Reader.ReadSourceLocation(F, Record, Idx)); + D->setCombiner(Reader.ReadExpr(F)); + D->setInitializer(Reader.ReadExpr(F)); + D->PrevDeclInScope = Reader.ReadDeclID(F, Record, Idx); + D->setType(Reader.readType(F, Record, Idx)); +} + //===--===// // Attribute Reading //===--===// @@ -2409,7 +2419,8 @@ isa(D) || isa(D) || isa(D) || - isa(D)) + isa(D) || + isa(D)) return true; if (VarDecl *Var = dyn_cast(D)) return Var->isFileVarDecl() && @@ -3305,6 +3316,9 @@ case DECL_OMP_THREADPRIVATE: D = OMPThreadPrivateDecl::CreateDeserialized(Context, ID, Record[Idx++]); break; + case DECL_OMP_DECLARE_REDUCTION: +D = OMPDeclareReductionDecl::CreateDeserialized(Context, ID); +break; case DECL_EMPTY: D = EmptyDecl::CreateDeserialized(Context, ID); break; Index: lib/Serialization/ASTCommon.cpp === --- lib/Serialization/ASTCommon.cpp +++ lib/Serialization/ASTCommon.cpp @@ -329,6 +329,7 @@ case Decl::ClassScopeFunctionSpecialization: case Decl::Import: case Decl::OMPThreadPrivate: + case Decl::OMPDeclareReduction: case Decl::BuiltinTemplate: return false; Index: lib/Parse/Parser.cpp === --- l
Re: [PATCH] D16171: Warning on redeclaring with a conflicting asm label
Weiming Zhao wrote: weimingz created this revision. weimingz added a reviewer: nicholas. weimingz added a subscriber: cfe-commits. r255371 errors on redeclaring with a conflicting asm label. This patch changes errors to warnings to prevent breaking existing codes. Can you include a testcase where the warning is issued but clang and gcc emit the same .o file? http://reviews.llvm.org/D16171 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp test/Sema/asm-label.c Index: test/Sema/asm-label.c === --- test/Sema/asm-label.c +++ test/Sema/asm-label.c @@ -7,21 +7,21 @@ void f() { g(); } -void g() __asm__("gold"); // expected-error{{cannot apply asm label to function after its first use}} +void g() __asm__("gold"); // expected-warning{{cannot apply asm label to function after its first use}} void h() __asm__("hose"); // expected-note{{previous declaration is here}} -void h() __asm__("hair"); // expected-error{{conflicting asm label}} +void h() __asm__("hair"); // expected-warning{{conflicting asm label}} int x; int x __asm__("xenon"); int y; int test() { return y; } -int y __asm__("yacht"); // expected-error{{cannot apply asm label to variable after its first use}} +int y __asm__("yacht"); // expected-warning{{cannot apply asm label to variable after its first use}} int z __asm__("zebra"); // expected-note{{previous declaration is here}} -int z __asm__("zooms"); // expected-error{{conflicting asm label}} +int z __asm__("zooms"); // expected-warning{{conflicting asm label}} // No diagnostics on the following. Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -2385,13 +2385,13 @@ if (AsmLabelAttr *OldA = Old->getAttr()) { if (OldA->getLabel() != NewA->getLabel()) { // This redeclaration changes __asm__ label. -Diag(New->getLocation(), diag::err_different_asm_label); +Diag(New->getLocation(), diag::warn_different_asm_label); Diag(OldA->getLocation(), diag::note_previous_declaration); } } else if (Old->isUsed()) { // This redeclaration adds an __asm__ label to a declaration that has // already been ODR-used. - Diag(New->getLocation(), diag::err_late_asm_label_name) + Diag(New->getLocation(), diag::warn_late_asm_label_name) << isa(Old)<< New->getAttr()->getRange(); } } Index: include/clang/Basic/DiagnosticSemaKinds.td === --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -4266,9 +4266,11 @@ def err_conflicting_types : Error<"conflicting types for %0">; def err_different_pass_object_size_params : Error< "conflicting pass_object_size attributes on parameters">; -def err_late_asm_label_name : Error< - "cannot apply asm label to %select{variable|function}0 after its first use">; -def err_different_asm_label : Error<"conflicting asm label">; +def warn_late_asm_label_name : Warning< + "cannot apply asm label to %select{variable|function}0 after its first use">, + InGroup; +def warn_different_asm_label : Warning<"conflicting asm label">, + InGroup; def err_nested_redefinition : Error<"nested redefinition of %0">; def err_use_with_wrong_tag : Error< "use of %0 with tag type that does not match previous declaration">; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16171: Warning on redeclaring with a conflicting asm label
nicholas added a subscriber: nicholas. nicholas added a comment. Weiming Zhao wrote: > weimingz created this revision. > weimingz added a reviewer: nicholas. > weimingz added a subscriber: cfe-commits. > > r255371 errors on redeclaring with a conflicting asm label. > This patch changes errors to warnings to prevent breaking existing codes. Can you include a testcase where the warning is issued but clang and gcc emit the same .o file? > http://reviews.llvm.org/D16171 > > Files: > > include/clang/Basic/DiagnosticSemaKinds.td > lib/Sema/SemaDecl.cpp > test/Sema/asm-label.c > > Index: test/Sema/asm-label.c > === > > - test/Sema/asm-label.c +++ test/Sema/asm-label.c @@ -7,21 +7,21 @@ void f() > { g(); } -void g() __asm__("gold"); // expected-error{{cannot apply asm > label to function after its first use}} +void g() __asm__("gold"); // > expected-warning{{cannot apply asm label to function after its first use}} > > void h() __asm__("hose"); // expected-note{{previous declaration is here}} > -void h() __asm__("hair"); // expected-error{{conflicting asm label}} +void > h() __asm__("hair"); // expected-warning{{conflicting asm label}} > > int x; int x __asm__("xenon"); int y; > > int test() { return y; } > > -int y __asm__("yacht"); // expected-error{{cannot apply asm label to > variable after its first use}} +int y __asm__("yacht"); // > expected-warning{{cannot apply asm label to variable after its first use}} > > int z __asm__("zebra"); // expected-note{{previous declaration is here}} > -int z __asm__("zooms"); // expected-error{{conflicting asm label}} +int z > __asm__("zooms"); // expected-warning{{conflicting asm label}} > > > > // No diagnostics on the following. > > Index: lib/Sema/SemaDecl.cpp > === > > - lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -2385,13 +2385,13 @@ > if (AsmLabelAttr *OldA = Old->getAttr()) { if (OldA->getLabel() > != NewA->getLabel()) { // This redeclaration changes __asm__ label. > - Diag(New->getLocation(), diag::err_different_asm_label); + > Diag(New->getLocation(), diag::warn_different_asm_label); > Diag(OldA->getLocation(), diag::note_previous_declaration); } } else if > (Old->isUsed()) { // This redeclaration adds an __asm__ label to a > declaration that has // already been ODR-used. > - Diag(New->getLocation(), diag::err_late_asm_label_name) + > Diag(New->getLocation(), diag::warn_late_asm_label_name) << > isa(Old)<< New->getAttr()->getRange(); } } > Index: include/clang/Basic/DiagnosticSemaKinds.td > === > - include/clang/Basic/DiagnosticSemaKinds.td +++ > include/clang/Basic/DiagnosticSemaKinds.td @@ -4266,9 +4266,11 @@ def > err_conflicting_types : Error<"conflicting types for %0">; def > err_different_pass_object_size_params : Error< "conflicting pass_object_size > attributes on parameters">; -def err_late_asm_label_name : Error< > - "cannot apply asm label to %select{variable|function}0 after its first > use">; -def err_different_asm_label : Error<"conflicting asm label">; +def > warn_late_asm_label_name : Warning< + "cannot apply asm label to > %select{variable|function}0 after its first use">, + > InGroup; +def warn_different_asm_label : Warning<"conflicting > asm label">, + InGroup; def err_nested_redefinition : > Error<"nested redefinition of %0">; def err_use_with_wrong_tag : Error< "use > of %0 with tag type that does not match previous declaration">; http://reviews.llvm.org/D16171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15914: [OpenCL] Pipe builtin functions
Anastasia added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:1981 @@ +1980,3 @@ + const char *Name = + (BuiltinID == Builtin::BIread_pipe) ? "read_pipe_2" : "write_pipe_2"; + // Re-Creating the function type for this call, since the original type pekka.jaaskelainen wrote: > Shouldn't these have underscores, because they are not reserved OpenCL > keywords/builtins? Cannot user functions be called read_pipe_2 or > write_pipe_2? Yes, definitely! Add prefix "__" to each generated name! I think this should be done for all of them! Comment at: lib/Sema/SemaChecking.cpp:291 @@ +290,3 @@ + bool isValid = true; + // TODO: Now we check for all the pipe builtin with access qualifier, but in + // OpenCL spec does not tell about group functions, need to fix after get Please, remove this TODO. Have you created the bug to Khronos already? It seems sensible enough the access qualifiers must match. How can reserve/commit_read_pipe be called with a write_only pipe? Comment at: lib/Sema/SemaChecking.cpp:378 @@ +377,3 @@ +S.Diag(Call->getLocStart(), diag::err_opencl_builtin_pipe_arg_num) +<< getFunctionName(Call) << Call->getSourceRange(); +return true; I don't think we have any arguments there. It's all declared as variadic. But it seems to work without setting it. So I am not sure it's necessary to set it at all. http://reviews.llvm.org/D15914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r257757 - Fix for armv7-a15 and thumbv7-a15 buildbot fails.
Author: asbokhan Date: Thu Jan 14 05:53:50 2016 New Revision: 257757 URL: http://llvm.org/viewvc/llvm-project?rev=257757&view=rev Log: Fix for armv7-a15 and thumbv7-a15 buildbot fails. Modified: cfe/trunk/test/CodeGenCXX/duplicate-mangled-name.cpp Modified: cfe/trunk/test/CodeGenCXX/duplicate-mangled-name.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/duplicate-mangled-name.cpp?rev=257757&r1=257756&r2=257757&view=diff == --- cfe/trunk/test/CodeGenCXX/duplicate-mangled-name.cpp (original) +++ cfe/trunk/test/CodeGenCXX/duplicate-mangled-name.cpp Thu Jan 14 05:53:50 2016 @@ -38,9 +38,9 @@ namespace nm { float foo() { _ZN1TD1Ev(); -// CHECK: call void bitcast (void (%struct.T*)* @_ZN1TD1Ev to void ()*)() +// CHECK: call void bitcast ({{.*}} (%struct.T*)* @_ZN1TD1Ev to void ()*)() T t; -// CHECK: call void @_ZN1TD1Ev(%struct.T* %t) +// CHECK: call {{.*}} @_ZN1TD1Ev(%struct.T* %t) return _ZN2nm3abcE + nm::abc; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r257754 - PR25910: clang allows two var definitions with the same mangled name
On 14 January 2016 at 10:41, Andrey Bokhanko via cfe-commits wrote: > Author: asbokhan > Date: Thu Jan 14 04:41:16 2016 > New Revision: 257754 > > URL: http://llvm.org/viewvc/llvm-project?rev=257754&view=rev > Log: > PR25910: clang allows two var definitions with the same mangled name Hi Andrey, Just making sure you saw this: http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/9057 cheers, --renato ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r257754 - PR25910: clang allows two var definitions with the same mangled name
Renato, Thanks! -- I committed a fix already: http://llvm.org/viewvc/llvm-project?view=revision&revision=257757 Yours, Andrey On Thu, Jan 14, 2016 at 2:57 PM, Renato Golin wrote: > On 14 January 2016 at 10:41, Andrey Bokhanko via cfe-commits > wrote: >> Author: asbokhan >> Date: Thu Jan 14 04:41:16 2016 >> New Revision: 257754 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=257754&view=rev >> Log: >> PR25910: clang allows two var definitions with the same mangled name > > Hi Andrey, > > Just making sure you saw this: > http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/9057 > > cheers, > --renato ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16152: [clang-tidy] Add check performance-faster-string-find
alexfh added a comment. Awesome! See a few comments inline. Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:51 @@ +50,3 @@ + const auto StringFindFunctions = + anyOf(hasName("find"), hasName("rfind"), hasName("find_first_of"), +hasName("find_first_not_of"), hasName("find_last_of"), Probably out of scope of this patch, but I wonder how many times `hasName` is still more effective than one `matchesName`? Maybe we should add a `matchesName` variant for unqualified names or hasName taking a list of strings? Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:57 @@ +56,3 @@ + + for (auto &ClassName : StringLikeClasses) { +const auto HasName = hasName(ClassName); `const auto &` would be slightly clearer. Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:75 @@ +74,3 @@ + + diag(Literal->getLocStart(), "char overload is more efficient") + << FixItHint::CreateReplacement( The message might be confusing in some situations (e.g. macros). I think, it should at least mention what method (and maybe of what class) is in question, e.g. "std::string::find() called with a string literal consisting of a single character; consider using the more effective overload accepting a character". Comment at: clang-tidy/performance/FasterStringFindCheck.h:15 @@ +14,3 @@ + +#include +#include Sort includes, please. Comment at: test/clang-tidy/performance-faster-string-find.cpp:8 @@ +7,3 @@ +template +struct basic_string { + int find(const char *, int = 0) const; Should we move stubs to a common header(s)? Comment at: test/clang-tidy/performance-faster-string-find.cpp:9 @@ +8,3 @@ +struct basic_string { + int find(const char *, int = 0) const; + int find(const char *, int, int) const; Did you mean `const T *`? Comment at: test/clang-tidy/performance-faster-string-find.cpp:31 @@ +30,3 @@ + +void StringFind() { + std::string Str; As usual, please add tests with templates and macros ;) Comment at: test/clang-tidy/performance-faster-string-find.cpp:32 @@ +31,3 @@ +void StringFind() { + std::string Str; + Please add tests for std::wstring, std::u16string and std::u32string. At the very least, we should ensure we don't break the code using them. Comment at: test/clang-tidy/performance-faster-string-find.cpp:50 @@ +49,3 @@ + + // Some other overloads + Str.rfind("a"); "Some other methods." maybe? http://reviews.llvm.org/D16152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16112: PR26111: segmentation fault with __attribute__((mode(QI))) on function declaration
d.zobnin.bugzilla updated this revision to Diff 44859. d.zobnin.bugzilla added a comment. Thanks for the review! Updated the patch: added Subjects line for the attribute and removed the "err_attr_wrong_decl" diagnostics. http://reviews.llvm.org/D16112 Files: include/clang/AST/Decl.h include/clang/Basic/Attr.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDeclAttr.cpp test/Sema/attr-mode.c Index: lib/Sema/SemaDeclAttr.cpp === --- lib/Sema/SemaDeclAttr.cpp +++ lib/Sema/SemaDeclAttr.cpp @@ -3391,13 +3391,8 @@ QualType OldTy; if (TypedefNameDecl *TD = dyn_cast(D)) OldTy = TD->getUnderlyingType(); - else if (ValueDecl *VD = dyn_cast(D)) -OldTy = VD->getType(); - else { -S.Diag(D->getLocation(), diag::err_attr_wrong_decl) - << Attr.getName() << Attr.getRange(); -return; - } + else +OldTy = cast(D)->getType(); // Base type can also be a vector type (see PR17453). // Distinguish between base type and base element type. @@ -3470,7 +3465,7 @@ if (TypedefNameDecl *TD = dyn_cast(D)) TD->setModedTypeSourceInfo(TD->getTypeSourceInfo(), NewTy); else -cast(D)->setType(NewTy); +cast(D)->setType(NewTy); D->addAttr(::new (S.Context) ModeAttr(Attr.getRange(), S.Context, Name, Index: include/clang/AST/Decl.h === --- include/clang/AST/Decl.h +++ include/clang/AST/Decl.h @@ -1954,6 +1954,7 @@ unsigned getMinRequiredArguments() const; QualType getReturnType() const { +assert(getType()->getAs() && "Expected a FunctionType!"); return getType()->getAs()->getReturnType(); } @@ -1964,6 +1965,7 @@ /// \brief Determine the type of an expression that calls this function. QualType getCallResultType() const { +assert(getType()->getAs() && "Expected a FunctionType!"); return getType()->getAs()->getCallResultType(getASTContext()); } Index: include/clang/Basic/DiagnosticSemaKinds.td === --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -2844,8 +2844,6 @@ InGroup; def err_complex_mode_vector_type : Error< "type of machine mode does not support base vector types">; -def err_attr_wrong_decl : Error< - "%0 attribute invalid on this declaration, requires typedef or value">; def warn_attribute_nonnull_no_pointers : Warning< "'nonnull' attribute applied to function with no pointer arguments">, InGroup; Index: include/clang/Basic/Attr.td === --- include/clang/Basic/Attr.td +++ include/clang/Basic/Attr.td @@ -878,6 +878,8 @@ def Mode : Attr { let Spellings = [GCC<"mode">]; + let Subjects = SubjectList<[Var, TypedefName], ErrorDiag, + "ExpectedVariableOrTypedef">; let Args = [IdentifierArgument<"Mode">]; let Documentation = [Undocumented]; } Index: test/Sema/attr-mode.c === --- test/Sema/attr-mode.c +++ test/Sema/attr-mode.c @@ -24,6 +24,9 @@ int **__attribute((mode(QI)))* i32; // expected-error{{mode attribute}} +__attribute__((mode(QI))) int invalid_func() { return 1; } // expected-error{{'mode' attribute only applies to variables and typedefs}} +enum invalid_enum { A1 __attribute__((mode(QI))) }; // expected-error{{'mode' attribute only applies to variables and typedefs}} + typedef _Complex double c32 __attribute((mode(SC))); int c32_test[sizeof(c32) == 8 ? 1 : -1]; typedef _Complex float c64 __attribute((mode(DC))); Index: lib/Sema/SemaDeclAttr.cpp === --- lib/Sema/SemaDeclAttr.cpp +++ lib/Sema/SemaDeclAttr.cpp @@ -3391,13 +3391,8 @@ QualType OldTy; if (TypedefNameDecl *TD = dyn_cast(D)) OldTy = TD->getUnderlyingType(); - else if (ValueDecl *VD = dyn_cast(D)) -OldTy = VD->getType(); - else { -S.Diag(D->getLocation(), diag::err_attr_wrong_decl) - << Attr.getName() << Attr.getRange(); -return; - } + else +OldTy = cast(D)->getType(); // Base type can also be a vector type (see PR17453). // Distinguish between base type and base element type. @@ -3470,7 +3465,7 @@ if (TypedefNameDecl *TD = dyn_cast(D)) TD->setModedTypeSourceInfo(TD->getTypeSourceInfo(), NewTy); else -cast(D)->setType(NewTy); +cast(D)->setType(NewTy); D->addAttr(::new (S.Context) ModeAttr(Attr.getRange(), S.Context, Name, Index: include/clang/AST/Decl.h === --- include/clang/AST/Decl.h +++ include/clang/AST/Decl.h @@ -1954,6 +1954,7 @@ unsigned getMinRequiredArguments() const; QualType getReturnType() const { +assert(getType()->getAs() && "Expected a FunctionType!"); return getType()-
Re: [PATCH] D16178: [analyzer] A quick fix on top of D12901/r257464
pgousseau accepted this revision. pgousseau added a comment. This revision is now accepted and ready to land. LGTM! Thanks Artom for finding the bug and new test case, sorry for missing those in my patch! Removing "!IsNotTruncated" is definitely worth trying. Please commit if code owners are happy with it too. Pierre. http://reviews.llvm.org/D16178 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16179: [clang-tidy] Handle decayed types and other improvements in VirtualNearMiss check.
alexfh added a comment. > ... What about a configuration option to also report near misses when only a > qualifier is missing? Might be a useful thing. We should first check if it makes sense to always ignore a qualifier. Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:55 @@ -54,11 +54,3 @@ // Both types must be pointers or references to classes. - if (const auto *DerivedPT = DerivedReturnTy->getAs()) { -if (const auto *BasePT = BaseReturnTy->getAs()) { - DTy = DerivedPT->getPointeeType(); - BTy = BasePT->getPointeeType(); -} - } else if (const auto *DerivedRT = DerivedReturnTy->getAs()) { -if (const auto *BaseRT = BaseReturnTy->getAs()) { - DTy = DerivedRT->getPointeeType(); - BTy = BaseRT->getPointeeType(); -} + if ((BaseReturnTy->isPointerType() && DerivedReturnTy->isPointerType()) || + (BaseReturnTy->isReferenceType() && DerivedReturnTy->isReferenceType())) { Interesting, didn't know about `Type::getPointeeType()`. I'd better return, if the condition is not met. The next `if` would be not needed then and the variable definitions above could be moved after this `if`. Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:129 @@ -127,3 +128,3 @@ for (unsigned I = 0; I < NumParamA; I++) { -if (BaseMD->getParamDecl(I)->getType() != -DerivedMD->getParamDecl(I)->getType()) +if (getDecayedType(BaseMD->getParamDecl(I)->getType()) != +getDecayedType(DerivedMD->getParamDecl(I)->getType())) We should check whether this creates any false positives. http://reviews.llvm.org/D16179 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r257762 - [mips] Added support for -Wa,-mips32 and similar.
Author: s.egerton Date: Thu Jan 14 07:01:48 2016 New Revision: 257762 URL: http://llvm.org/viewvc/llvm-project?rev=257762&view=rev Log: [mips] Added support for -Wa,-mips32 and similar. Reviewers: vkalintiris, dsanders Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D15070 Modified: cfe/trunk/lib/Driver/Tools.cpp cfe/trunk/test/Driver/mips-ias-Wa.s Modified: cfe/trunk/lib/Driver/Tools.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=257762&r1=257761&r2=257762&view=diff == --- cfe/trunk/lib/Driver/Tools.cpp (original) +++ cfe/trunk/lib/Driver/Tools.cpp Thu Jan 14 07:01:48 2016 @@ -2580,6 +2580,7 @@ static void CollectArgsForIntegratedAsse // When using an integrated assembler, translate -Wa, and -Xassembler // options. bool CompressDebugSections = false; + const char *MipsTargetFeature = nullptr; for (const Arg *A : Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler)) { A->claim(); @@ -2618,7 +2619,26 @@ static void CollectArgsForIntegratedAsse CmdArgs.push_back("-soft-float"); continue; } -break; + +MipsTargetFeature = llvm::StringSwitch(Value) +.Case("-mips1", "+mips1") +.Case("-mips2", "+mips2") +.Case("-mips3", "+mips3") +.Case("-mips4", "+mips4") +.Case("-mips5", "+mips5") +.Case("-mips32", "+mips32") +.Case("-mips32r2", "+mips32r2") +.Case("-mips32r3", "+mips32r3") +.Case("-mips32r5", "+mips32r5") +.Case("-mips32r6", "+mips32r6") +.Case("-mips64", "+mips64") +.Case("-mips64r2", "+mips64r2") +.Case("-mips64r3", "+mips64r3") +.Case("-mips64r5", "+mips64r5") +.Case("-mips64r6", "+mips64r6") +.Default(nullptr); +if (MipsTargetFeature) + continue; } if (Value == "-force_cpusubtype_ALL") { @@ -2666,6 +2686,10 @@ static void CollectArgsForIntegratedAsse else D.Diag(diag::warn_debug_compression_unavailable); } + if (MipsTargetFeature != nullptr) { +CmdArgs.push_back("-target-feature"); +CmdArgs.push_back(MipsTargetFeature); + } } // This adds the static libclang_rt.builtins-arch.a directly to the command line Modified: cfe/trunk/test/Driver/mips-ias-Wa.s URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/mips-ias-Wa.s?rev=257762&r1=257761&r2=257762&view=diff == --- cfe/trunk/test/Driver/mips-ias-Wa.s (original) +++ cfe/trunk/test/Driver/mips-ias-Wa.s Thu Jan 14 07:01:48 2016 @@ -47,3 +47,94 @@ // RUN: FileCheck -check-prefix=MSOFT-FLOAT-BOTH-MHARD-FLOAT-FIRST %s // MSOFT-FLOAT-BOTH-MHARD-FLOAT-FIRST: -cc1as // MSOFT-FLOAT-BOTH-MHARD-FLOAT-FIRST: "-target-feature" "-soft-float" "-target-feature" "+soft-float" + +// RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s -Wa,-mips1 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS1 %s +// MIPS1: -cc1as +// MIPS1: "-target-feature" "+mips1" + +// RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s -Wa,-mips2 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS2 %s +// MIPS2: -cc1as +// MIPS2: "-target-feature" "+mips2" + +// RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s -Wa,-mips3 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS3 %s +// MIPS3: -cc1as +// MIPS3: "-target-feature" "+mips3" + +// RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s -Wa,-mips4 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS4 %s +// MIPS4: -cc1as +// MIPS4: "-target-feature" "+mips4" + +// RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s -Wa,-mips5 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS5 %s +// MIPS5: -cc1as +// MIPS5: "-target-feature" "+mips5" + +// RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s -Wa,-mips32 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS32 %s +// MIPS32: -cc1as +// MIPS32: "-target-feature" "+mips32" + +// RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s -Wa,-mips32r2 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS32R2 %s +// MIPS32R2: -cc1as +// MIPS32R2: "-target-feature" "+mips32r2" + +// RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s -Wa,-mips32r3 2>&1 | \ +// RUN: FileCheck -check-prefix=MIPS32R3 %s +// MIPS32R3: -cc1as +// MIPS32R3: "-target-feature" "+mips32r3" + +// RUN: %clang -target mips-linux-gnu -### -fintegrated-as -c %s -Wa,-mips32r5 2>&1 | \ +// RUN:
Re: [PATCH] D16183: Added CheckName field to YAML report
alexfh added a comment. I'm not sure `tooling::Replacement` is the best place to store check name. Maybe create a separate wrapper class and serialize it instead (clang-apply-replacements will have to be changed to support this format as well). This could be `ClangTidyDiagnostic` or just `Diagnostic`, and we could also store the message and other useful information in it. Comment at: tools/clang/include/clang/Tooling/Core/Replacement.h:93 @@ +92,3 @@ + const LangOptions &LangOpts = LangOptions(), + StringRef CheckName = "" + ); Please clang-format the code. Comment at: tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:80 @@ -79,3 +79,3 @@ - Error.Fix.insert(tooling::Replacement(SM, Range, FixIt.CodeToInsert)); + Error.Fix.insert(tooling::Replacement(SM, Range, FixIt.CodeToInsert, LangOptions(), StringRef(Error.CheckName))); } Doesn't it compile without `StringRef()` around `Error.CheckName`? http://reviews.llvm.org/D16183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.
alexfh added inline comments. Comment at: clang-tidy/tool/ClangTidyMain.cpp:78 @@ +77,3 @@ +static cl::opt +HeaderFileExtensions("header-file-extensions", + cl::desc("File extensions that regard as header file.\n" hokein wrote: > alexfh wrote: > > We don't need a command-line flag for this. The regular way to configure > > clang-tidy is .clang-tidy files. However, if needed, check options can be > > configured from the command line via the `-config=` option. > From my understanding, it is a global option for header-file-extensions, > right? If we remove this command-line flag, there is only local > `header-file-extensions` option remained for particular checks. Both local and global options reside in `CheckOptions` and can be configured in the same way using either a .clang-tidy file or the -config= command-line option. The only difference between local and global options is that the name of the latter is not prefixed with the "CheckName.". Makes sense? Repository: rL LLVM http://reviews.llvm.org/D16113 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15373: Fix for bug 25786 - Assertion "Chunk.Kind == DeclaratorChunk::Function" failed with regparm attribute.
a.makarov added a comment. Ping http://reviews.llvm.org/D15373 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Adding Python 3 compatibility to Clang Python bindings
For your consideration: Attached is a patch that adds Python 3 compatibility (without losing Python 2 compatibility) to Clang’s Python bindings. The patch also includes PEP8 formatting cleanups, and a setup.py file to make it easier to install the bindings into a working Python development environment. Yours, Russell Keith-Magee %-) diff --git a/bindings/python/clang/cindex.py b/bindings/python/clang/cindex.py index e4b3876..b1a9abe 100644 --- a/bindings/python/clang/cindex.py +++ b/bindings/python/clang/cindex.py @@ -6,6 +6,7 @@ # License. See LICENSE.TXT for details. # #======# +from __future__ import unicode_literals r""" Clang Indexing Library Bindings @@ -75,6 +76,7 @@ c_object_p = POINTER(c_void_p) callbacks = {} + ### Exception Classes ### class TranslationUnitLoadError(Exception): @@ -87,6 +89,7 @@ class TranslationUnitLoadError(Exception): """ pass + class TranslationUnitSaveError(Exception): """Represents an error that occurred when saving a TranslationUnit. @@ -117,6 +120,7 @@ class TranslationUnitSaveError(Exception): self.save_error = enumeration Exception.__init__(self, 'Error %d: %s' % (enumeration, message)) + ### Structures and Utility Classes ### class CachedProperty(object): @@ -157,6 +161,7 @@ class _CXString(Structure): assert isinstance(res, _CXString) return conf.lib.clang_getCString(res) + class SourceLocation(Structure): """ A SourceLocation represents a particular location within a source file. @@ -167,8 +172,9 @@ class SourceLocation(Structure): def _get_instantiation(self): if self._data is None: f, l, c, o = c_object_p(), c_uint(), c_uint(), c_uint() -conf.lib.clang_getInstantiationLocation(self, byref(f), byref(l), -byref(c), byref(o)) +conf.lib.clang_getInstantiationLocation( +self, byref(f), byref(l), byref(c), byref(o) +) if f: f = File(f) else: @@ -228,6 +234,7 @@ class SourceLocation(Structure): return "" % ( filename, self.line, self.column) + class SourceRange(Structure): """ A SourceRange describes a range of source locations within the source @@ -272,8 +279,8 @@ class SourceRange(Structure): return False if other.file is None and self.start.file is None: pass -elif ( self.start.file.name != other.file.name or - other.file.name != self.end.file.name): +elif (self.start.file.name != other.file.name or + other.file.name != self.end.file.name): # same file name return False # same file, in between lines @@ -292,6 +299,7 @@ class SourceRange(Structure): def __repr__(self): return "" % (self.start, self.end) + class Diagnostic(object): """ A Diagnostic is a single instance of a Clang diagnostic. It includes the @@ -300,10 +308,10 @@ class Diagnostic(object): """ Ignored = 0 -Note= 1 +Note = 1 Warning = 2 -Error = 3 -Fatal = 4 +Error = 3 +Fatal = 4 def __init__(self, ptr): self.ptr = ptr @@ -321,7 +329,7 @@ class Diagnostic(object): @property def spelling(self): -return conf.lib.clang_getDiagnosticSpelling(self) +return conf.lib.clang_getDiagnosticSpelling(self).decode('utf-8') @property def ranges(self): @@ -350,8 +358,9 @@ class Diagnostic(object): def __getitem__(self, key): range = SourceRange() -value = conf.lib.clang_getDiagnosticFixIt(self.diag, key, -byref(range)) +value = conf.lib.clang_getDiagnosticFixIt( +self.diag, key, byref(range) +) if len(value) == 0: raise IndexError @@ -367,12 +376,12 @@ class Diagnostic(object): @property def category_name(self): """The string name of the category for this diagnostic.""" -return conf.lib.clang_getDiagnosticCategoryText(self) +return conf.lib.clang_getDiagnosticCategoryText(self).decode('utf-8') @property def option(self): """The command-line option that enables this diagnostic.""" -return conf.lib.clang_getDiagnosticOption(self, None) +return conf.lib.clang_getDiagnosticOption(self, None).decode('utf-8') @property def disable_option(self): @@ -380,14 +389,15 @@ class Diagnostic(object): disable = _CXString() conf.lib.clang_getDiagnosticOption(self, byref(disable)) -return conf.lib.clang_getCString(disable) +return conf.lib.clang_getCString(disable).decode('utf-8') def __repr__(self): return "" % ( self.severity, self.location, self.spelli
[PATCH] D16183: Added CheckName field to YAML report
Elijah_Th created this revision. Elijah_Th added reviewers: klimek, alexfh, djasper, cfe-commits. Herald added a subscriber: klimek. See details in https://llvm.org/bugs/show_bug.cgi?id=26132 http://reviews.llvm.org/D16183 Files: tools/clang/include/clang/Tooling/Core/Replacement.h tools/clang/include/clang/Tooling/ReplacementsYaml.h tools/clang/lib/Tooling/Core/Replacement.cpp tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp Index: tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp === --- tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -77,7 +77,7 @@ assert(Range.getBegin().isFileID() && Range.getEnd().isFileID() && "Only file locations supported in fix-it hints."); - Error.Fix.insert(tooling::Replacement(SM, Range, FixIt.CodeToInsert)); + Error.Fix.insert(tooling::Replacement(SM, Range, FixIt.CodeToInsert, LangOptions(), StringRef(Error.CheckName))); } } Index: tools/clang/lib/Tooling/Core/Replacement.cpp === --- tools/clang/lib/Tooling/Core/Replacement.cpp +++ tools/clang/lib/Tooling/Core/Replacement.cpp @@ -32,9 +32,9 @@ : FilePath(InvalidLocation) {} Replacement::Replacement(StringRef FilePath, unsigned Offset, unsigned Length, - StringRef ReplacementText) + StringRef ReplacementText, StringRef CheckName) : FilePath(FilePath), ReplacementRange(Offset, Length), - ReplacementText(ReplacementText) {} + ReplacementText(ReplacementText), CheckName(CheckName) {} Replacement::Replacement(const SourceManager &Sources, SourceLocation Start, unsigned Length, StringRef ReplacementText) { @@ -44,8 +44,9 @@ Replacement::Replacement(const SourceManager &Sources, const CharSourceRange &Range, StringRef ReplacementText, - const LangOptions &LangOpts) { - setFromSourceRange(Sources, Range, ReplacementText, LangOpts); + const LangOptions &LangOpts, + StringRef CheckName) { + setFromSourceRange(Sources, Range, ReplacementText, LangOpts, CheckName); } bool Replacement::isApplicable() const { @@ -109,13 +110,14 @@ void Replacement::setFromSourceLocation(const SourceManager &Sources, SourceLocation Start, unsigned Length, -StringRef ReplacementText) { +StringRef ReplacementText, StringRef CheckName) { const std::pair DecomposedLocation = Sources.getDecomposedLoc(Start); const FileEntry *Entry = Sources.getFileEntryForID(DecomposedLocation.first); this->FilePath = Entry ? Entry->getName() : InvalidLocation; this->ReplacementRange = Range(DecomposedLocation.second, Length); this->ReplacementText = ReplacementText; + this->CheckName = CheckName; } // FIXME: This should go into the Lexer, but we need to figure out how @@ -137,10 +139,11 @@ void Replacement::setFromSourceRange(const SourceManager &Sources, const CharSourceRange &Range, StringRef ReplacementText, - const LangOptions &LangOpts) { + const LangOptions &LangOpts, + StringRef CheckName) { setFromSourceLocation(Sources, Sources.getSpellingLoc(Range.getBegin()), getRangeSize(Sources, Range, LangOpts), -ReplacementText); +ReplacementText, CheckName); } template Index: tools/clang/include/clang/Tooling/ReplacementsYaml.h === --- tools/clang/include/clang/Tooling/ReplacementsYaml.h +++ tools/clang/include/clang/Tooling/ReplacementsYaml.h @@ -33,21 +33,22 @@ /// access to its data members. struct NormalizedReplacement { NormalizedReplacement(const IO &) -: FilePath(""), Offset(0), Length(0), ReplacementText("") {} +: FilePath(""), Offset(0), Length(0), ReplacementText(""), CheckName("") {} NormalizedReplacement(const IO &, const clang::tooling::Replacement &R) -: FilePath(R.getFilePath()), Offset(R.getOffset()), - Length(R.getLength()), ReplacementText(R.getReplacementText()) {} +: FilePath(R.getFilePath()), Offset(R.getOffset()), Length(R.getLength()), + ReplacementText(R.getReplacementText()), CheckName(R.getCheckName()) {} clang::tooling::Replacement denormalize(const IO &) { return clang::tooling::Replacement(FilePath, Offset, Length, - Replacemen
Re: [PATCH] D15373: Fix for bug 25786 - Assertion "Chunk.Kind == DeclaratorChunk::Function" failed with regparm attribute.
aaron.ballman added a comment. In http://reviews.llvm.org/D15373#315457, @a.makarov wrote: > I've updated the patch. Please, re-review it again. > About creating attributed type - I think we shouldn't create it if we > ignored specified CC attribute. Ignoring specified CC attribute (and emitting > the warning, of course) leads to substituting it by default; calling > convention, substituted by default, is the same situation like there is no CC > attribute specified for chosen function. Thus, from this point of view, we > should not create AttributedType. I tend to agree with @rnk on this -- it is a shame to lose that syntactic information in the AST representation (for instance, tools may wish to detect the presence of that attribute in the source to perform different analyses on the type). Even if it is semantically ignored, it is still something the user wrote. For instance, this means we cannot round-trip the user's source code through pretty printing, despite the code not being ill-formed. Comment at: test/CodeGen/adding_defaulted_cc_attr_to_type.c:1 @@ +1,2 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -verify -emit-llvm -o - %s | FileCheck %s + Can you drop the svn props on the file? Our convention is to not rely on props, but instead save the file with UNIX line endings. http://reviews.llvm.org/D15373 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16171: Warning on redeclaring with a conflicting asm label
On Thu, Jan 14, 2016 at 02:05:21AM +, Weiming Zhao via cfe-commits wrote: > r255371 errors on redeclaring with a conflicting asm label. > This patch changes errors to warnings to prevent breaking existing codes. I'm not sure I agree with this. We have triggered this in NetBSD in two different situations and both are bugs. What example do you have where the result was "as intended"? Joerg ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r257763 - clang-format: Fix incorrectly enforced linebreak with ColumnLimit 0.
Author: djasper Date: Thu Jan 14 07:36:46 2016 New Revision: 257763 URL: http://llvm.org/viewvc/llvm-project?rev=257763&view=rev Log: clang-format: Fix incorrectly enforced linebreak with ColumnLimit 0. Before: [] .(); After: [].(); Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp cfe/trunk/unittests/Format/FormatTest.cpp Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=257763&r1=257762&r2=257763&view=diff == --- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original) +++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Thu Jan 14 07:36:46 2016 @@ -182,7 +182,7 @@ bool ContinuationIndenter::mustBreak(con return true; unsigned NewLineColumn = getNewLineColumn(State); - if (Current.isMemberAccess() && + if (Current.isMemberAccess() && Style.ColumnLimit != 0 && State.Column + getLengthToNextOperator(Current) > Style.ColumnLimit && (State.Column > NewLineColumn || Current.NestingLevel < State.StartOfLineLevel)) Modified: cfe/trunk/unittests/Format/FormatTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=257763&r1=257762&r2=257763&view=diff == --- cfe/trunk/unittests/Format/FormatTest.cpp (original) +++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Jan 14 07:36:46 2016 @@ -6135,6 +6135,9 @@ TEST_F(FormatTest, FormatsArrays) { " .aa();"); verifyNoCrash("a[,Y?)]", getLLVMStyleWithColumns(10)); + + FormatStyle NoColumnLimit = getLLVMStyleWithColumns(0); + verifyFormat("a[bb].cc()", NoColumnLimit); } TEST_F(FormatTest, LineStartsWithSpecialCharacter) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15709: [X86] Support 'interrupt' attribute for x86
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you! http://reviews.llvm.org/D15709 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16183: Added CheckName field to YAML report
Elijah_Th updated this revision to Diff 44876. Elijah_Th added a comment. - small format changes - removed unnecessary StringRef http://reviews.llvm.org/D16183 Files: tools/clang/include/clang/Tooling/Core/Replacement.h tools/clang/include/clang/Tooling/ReplacementsYaml.h tools/clang/lib/Tooling/Core/Replacement.cpp tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp Index: tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp === --- tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -77,7 +77,8 @@ assert(Range.getBegin().isFileID() && Range.getEnd().isFileID() && "Only file locations supported in fix-it hints."); - Error.Fix.insert(tooling::Replacement(SM, Range, FixIt.CodeToInsert)); + Error.Fix.insert(tooling::Replacement(SM, Range, FixIt.CodeToInsert, +LangOptions(), Error.CheckName)); } } Index: tools/clang/lib/Tooling/Core/Replacement.cpp === --- tools/clang/lib/Tooling/Core/Replacement.cpp +++ tools/clang/lib/Tooling/Core/Replacement.cpp @@ -32,20 +32,20 @@ : FilePath(InvalidLocation) {} Replacement::Replacement(StringRef FilePath, unsigned Offset, unsigned Length, - StringRef ReplacementText) + StringRef ReplacementText, StringRef CheckName) : FilePath(FilePath), ReplacementRange(Offset, Length), - ReplacementText(ReplacementText) {} + ReplacementText(ReplacementText), CheckName(CheckName) {} Replacement::Replacement(const SourceManager &Sources, SourceLocation Start, unsigned Length, StringRef ReplacementText) { setFromSourceLocation(Sources, Start, Length, ReplacementText); } Replacement::Replacement(const SourceManager &Sources, const CharSourceRange &Range, - StringRef ReplacementText, - const LangOptions &LangOpts) { - setFromSourceRange(Sources, Range, ReplacementText, LangOpts); + StringRef ReplacementText, const LangOptions &LangOpts, + StringRef CheckName) { + setFromSourceRange(Sources, Range, ReplacementText, LangOpts, CheckName); } bool Replacement::isApplicable() const { @@ -109,13 +109,15 @@ void Replacement::setFromSourceLocation(const SourceManager &Sources, SourceLocation Start, unsigned Length, -StringRef ReplacementText) { +StringRef ReplacementText, +StringRef CheckName) { const std::pair DecomposedLocation = Sources.getDecomposedLoc(Start); const FileEntry *Entry = Sources.getFileEntryForID(DecomposedLocation.first); this->FilePath = Entry ? Entry->getName() : InvalidLocation; this->ReplacementRange = Range(DecomposedLocation.second, Length); this->ReplacementText = ReplacementText; + this->CheckName = CheckName; } // FIXME: This should go into the Lexer, but we need to figure out how @@ -137,10 +139,11 @@ void Replacement::setFromSourceRange(const SourceManager &Sources, const CharSourceRange &Range, StringRef ReplacementText, - const LangOptions &LangOpts) { + const LangOptions &LangOpts, + StringRef CheckName) { setFromSourceLocation(Sources, Sources.getSpellingLoc(Range.getBegin()), -getRangeSize(Sources, Range, LangOpts), -ReplacementText); +getRangeSize(Sources, Range, LangOpts), ReplacementText, +CheckName); } template @@ -314,7 +317,7 @@ // Merges the next element 'R' into this merged element. As we always merge // from 'First' into 'Second' or vice versa, the MergedReplacement knows what - // set the next element is coming from. + // set the next element is coming from. void merge(const Replacement &R) { if (MergeSecond) { unsigned REnd = R.getOffset() + Delta + R.getLength(); @@ -416,4 +419,3 @@ } // end namespace tooling } // end namespace clang - Index: tools/clang/include/clang/Tooling/ReplacementsYaml.h === --- tools/clang/include/clang/Tooling/ReplacementsYaml.h +++ tools/clang/include/clang/Tooling/ReplacementsYaml.h @@ -33,30 +33,34 @@ /// access to its data members. struct NormalizedReplacement { NormalizedReplacement(const IO &) -: FilePath(""), Offset(0), Length(0), ReplacementText("") {} +: FilePath
Re: [PATCH] D15907: Allow various function attributes to be specified on Objective-C blocks too.
aaron.ballman added a comment. At a high level, is there a reason why these should be supported on block, but not on ObjC method declarations? It smells like these may be useful there as well, which could also simplify the patch slightly. Comment at: include/clang/Basic/Attr.td:992 @@ -993,1 +991,3 @@ + let Subjects = SubjectList<[ObjCMethod, HasFunctionProto, ParmVar], + WarnDiag, "ExpectedFunctionMethodParameterOrBlock">; let Args = [VariadicUnsignedArgument<"Args">]; Since you updated ClangAttrEmitter.cpp, you shouldn't need this last argument since it can now be inferred. Comment at: lib/Parse/ParseExpr.cpp:2679 @@ -2678,3 +2678,1 @@ -/// ParseBlockId - Parse a block-id, which roughly looks like int (int x). -/// This (and its related changes) look good to me, but should be in a separate patch since they are orthogonal to the rest of the changes in this patch. Comment at: lib/Sema/SemaDeclAttr.cpp:1282 @@ -1279,1 +1281,3 @@ + const AttributeList &Attr, + QualType ResultType) { SourceRange SR = getFunctionOrMethodResultSourceRange(D); Please do not add any parameters to these function signatures. We try to keep them uniform in the hopes that maybe someday we can refactor this code to do dispatch more easily. (It's fine to add an extra level of indirection through a helper function that then has the additional parameter.) Comment at: lib/Sema/SemaDeclAttr.cpp:5654 @@ -5644,1 +5653,3 @@ +void Sema::ProcessDeclAttributes(Scope *S, Decl *D, const Declarator &PD, + QualType *OverrideRetType) { // Apply decl attributes from the DeclSpec if present. This doesn't seem like the correct way to solve the problem -- everyone who calls ProcessDeclAttributes should not have to understand what an overridden return type is for objective-c blocks. I'm not certain of a better way to solve it right now, but I would rather see the changes for this one split out into its own patch so we can make progress on the other improvements. http://reviews.llvm.org/D15907 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r257765 - [Hexagon] Change all builtins returning "bool" to return "int" instead
Author: kparzysz Date: Thu Jan 14 08:26:36 2016 New Revision: 257765 URL: http://llvm.org/viewvc/llvm-project?rev=257765&view=rev Log: [Hexagon] Change all builtins returning "bool" to return "int" instead Modified: cfe/trunk/include/clang/Basic/BuiltinsHexagon.def Modified: cfe/trunk/include/clang/Basic/BuiltinsHexagon.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsHexagon.def?rev=257765&r1=257764&r2=257765&view=diff == --- cfe/trunk/include/clang/Basic/BuiltinsHexagon.def (original) +++ cfe/trunk/include/clang/Basic/BuiltinsHexagon.def Thu Jan 14 08:26:36 2016 @@ -23,52 +23,52 @@ BUILTIN(__builtin_circ_ldd, "LLi*LLi*LLi // The builtins above are not autogenerated from iset.py. // Make sure you do not overwrite these. -BUILTIN(__builtin_HEXAGON_C2_cmpeq,"bii","") -BUILTIN(__builtin_HEXAGON_C2_cmpgt,"bii","") -BUILTIN(__builtin_HEXAGON_C2_cmpgtu,"bii","") -BUILTIN(__builtin_HEXAGON_C2_cmpeqp,"bLLiLLi","") -BUILTIN(__builtin_HEXAGON_C2_cmpgtp,"bLLiLLi","") -BUILTIN(__builtin_HEXAGON_C2_cmpgtup,"bLLiLLi","") +BUILTIN(__builtin_HEXAGON_C2_cmpeq,"iii","") +BUILTIN(__builtin_HEXAGON_C2_cmpgt,"iii","") +BUILTIN(__builtin_HEXAGON_C2_cmpgtu,"iii","") +BUILTIN(__builtin_HEXAGON_C2_cmpeqp,"iLLiLLi","") +BUILTIN(__builtin_HEXAGON_C2_cmpgtp,"iLLiLLi","") +BUILTIN(__builtin_HEXAGON_C2_cmpgtup,"iLLiLLi","") BUILTIN(__builtin_HEXAGON_A4_rcmpeqi,"iii","") BUILTIN(__builtin_HEXAGON_A4_rcmpneqi,"iii","") BUILTIN(__builtin_HEXAGON_A4_rcmpeq,"iii","") BUILTIN(__builtin_HEXAGON_A4_rcmpneq,"iii","") -BUILTIN(__builtin_HEXAGON_C2_bitsset,"bii","") -BUILTIN(__builtin_HEXAGON_C2_bitsclr,"bii","") -BUILTIN(__builtin_HEXAGON_C4_nbitsset,"bii","") -BUILTIN(__builtin_HEXAGON_C4_nbitsclr,"bii","") -BUILTIN(__builtin_HEXAGON_C2_cmpeqi,"bii","") -BUILTIN(__builtin_HEXAGON_C2_cmpgti,"bii","") -BUILTIN(__builtin_HEXAGON_C2_cmpgtui,"bii","") -BUILTIN(__builtin_HEXAGON_C2_cmpgei,"bii","") -BUILTIN(__builtin_HEXAGON_C2_cmpgeui,"bii","") -BUILTIN(__builtin_HEXAGON_C2_cmplt,"bii","") -BUILTIN(__builtin_HEXAGON_C2_cmpltu,"bii","") -BUILTIN(__builtin_HEXAGON_C2_bitsclri,"bii","") -BUILTIN(__builtin_HEXAGON_C4_nbitsclri,"bii","") -BUILTIN(__builtin_HEXAGON_C4_cmpneqi,"bii","") -BUILTIN(__builtin_HEXAGON_C4_cmpltei,"bii","") -BUILTIN(__builtin_HEXAGON_C4_cmplteui,"bii","") -BUILTIN(__builtin_HEXAGON_C4_cmpneq,"bii","") -BUILTIN(__builtin_HEXAGON_C4_cmplte,"bii","") -BUILTIN(__builtin_HEXAGON_C4_cmplteu,"bii","") -BUILTIN(__builtin_HEXAGON_C2_and,"bii","") -BUILTIN(__builtin_HEXAGON_C2_or,"bii","") -BUILTIN(__builtin_HEXAGON_C2_xor,"bii","") -BUILTIN(__builtin_HEXAGON_C2_andn,"bii","") -BUILTIN(__builtin_HEXAGON_C2_not,"bi","") -BUILTIN(__builtin_HEXAGON_C2_orn,"bii","") -BUILTIN(__builtin_HEXAGON_C4_and_and,"biii","") -BUILTIN(__builtin_HEXAGON_C4_and_or,"biii","") -BUILTIN(__builtin_HEXAGON_C4_or_and,"biii","") -BUILTIN(__builtin_HEXAGON_C4_or_or,"biii","") -BUILTIN(__builtin_HEXAGON_C4_and_andn,"biii","") -BUILTIN(__builtin_HEXAGON_C4_and_orn,"biii","") -BUILTIN(__builtin_HEXAGON_C4_or_andn,"biii","") -BUILTIN(__builtin_HEXAGON_C4_or_orn,"biii","") -BUILTIN(__builtin_HEXAGON_C2_pxfer_map,"bi","") -BUILTIN(__builtin_HEXAGON_C2_any8,"bi","") -BUILTIN(__builtin_HEXAGON_C2_all8,"bi","") +BUILTIN(__builtin_HEXAGON_C2_bitsset,"iii","") +BUILTIN(__builtin_HEXAGON_C2_bitsclr,"iii","") +BUILTIN(__builtin_HEXAGON_C4_nbitsset,"iii","") +BUILTIN(__builtin_HEXAGON_C4_nbitsclr,"iii","") +BUILTIN(__builtin_HEXAGON_C2_cmpeqi,"iii","") +BUILTIN(__builtin_HEXAGON_C2_cmpgti,"iii","") +BUILTIN(__builtin_HEXAGON_C2_cmpgtui,"iii","") +BUILTIN(__builtin_HEXAGON_C2_cmpgei,"iii","") +BUILTIN(__builtin_HEXAGON_C2_cmpgeui,"iii","") +BUILTIN(__builtin_HEXAGON_C2_cmplt,"iii","") +BUILTIN(__builtin_HEXAGON_C2_cmpltu,"iii","") +BUILTIN(__builtin_HEXAGON_C2_bitsclri,"iii","") +BUILTIN(__builtin_HEXAGON_C4_nbitsclri,"iii","") +BUILTIN(__builtin_HEXAGON_C4_cmpneqi,"iii","") +BUILTIN(__builtin_HEXAGON_C4_cmpltei,"iii","") +BUILTIN(__builtin_HEXAGON_C4_cmplteui,"iii","") +BUILTIN(__builtin_HEXAGON_C4_cmpneq,"iii","") +BUILTIN(__builtin_HEXAGON_C4_cmplte,"iii","") +BUILTIN(__builtin_HEXAGON_C4_cmplteu,"iii","") +BUILTIN(__builtin_HEXAGON_C2_and,"iii","") +BUILTIN(__builtin_HEXAGON_C2_or,"iii","") +BUILTIN(__builtin_HEXAGON_C2_xor,"iii","") +BUILTIN(__builtin_HEXAGON_C2_andn,"iii","") +BUILTIN(__builtin_HEXAGON_C2_not,"ii","") +BUILTIN(__builtin_HEXAGON_C2_orn,"iii","") +BUILTIN(__builtin_HEXAGON_C4_and_and,"","") +BUILTIN(__builtin_HEXAGON_C4_and_or,"","") +BUILTIN(__builtin_HEXAGON_C4_or_and,"","") +BUILTIN(__builtin_HEXAGON_C4_or_or,"","") +BUILTIN(__builtin_HEXAGON_C4_and_andn,"","") +BUILTIN(__builtin_HEXAGON_C4_and_orn,"","") +BUILTIN(__builtin_HEXAGON_C4_or_andn,"","") +BUILTIN(__builtin_HEXAGON_C4_or_orn,"","") +BUILTIN(__builtin_HEXAGON_C2_pxfer_map,"ii","") +BUILTIN(__builtin_HEXAGON_C2_any8,"ii","") +BUILTIN(__built
Re: [PATCH] D16112: PR26111: segmentation fault with __attribute__((mode(QI))) on function declaration
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you! http://reviews.llvm.org/D16112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.
aaron.ballman added inline comments. Comment at: clang-tidy/ClangTidyOptions.h:216 @@ +215,3 @@ +/// HeaderFileExtensions. +bool endWithHeaderFileExtensions(llvm::StringRef FileName, + llvm::StringRef HeaderFileExtensions); alexfh wrote: > hokein wrote: > > aaron.ballman wrote: > > > alexfh wrote: > > > > This function doesn't belong here. I'm also not sure we need this > > > > function at all. First, it's ineffective to split the string containing > > > > the list of extensions each time. Second, if we store a set of > > > > extensions, then we can just search for the actual file extension in > > > > this set. > > > endsWithHeaderFileExtension instead? However, given that uses of this all > > > start with a SourceLocation, I wonder if that makes for a cleaner API: > > > isLocInHeaderFile(SourceLocation, ); > > > > > > Also, how does this work if I want to include an extension-less file as > > > the header file "extension?" It would be plausible if the extensions were > > > passed in as a list, but as it stands it doesn't seem possible without > > > weird conventions like leaving a blank in the list (e.g., `.h,,.hpp`), > > > which seems error-prone. > > > > > > Also, I'm not certain what I can pass in. The documentation should be > > > updated to state whether these extensions are intended to include the ".". > > > endsWithHeaderFileExtension instead? However, given that uses of this all > > > start with a SourceLocation, I wonder if that makes for a cleaner API: > > > isLocInHeaderFile(SourceLocation, ); > > > > Using `SourceLocation` only is not enough to retrieve the belonging file > > name (we need `SourceManager` too). > > > > >Also, how does this work if I want to include an extension-less file as > > >the header file "extension?" It would be plausible if the extensions were > > >passed in as a list, but as it stands it doesn't seem possible without > > >weird conventions like leaving a blank in the list (e.g., .h,,.hpp), which > > >seems error-prone. > > > > Yeah, for extensions-less header file, you can pass the string like > > `.h,,.hpp`, which is a bit of weird. Do you have a better idea here? > > Passing a string into `header-file-extensions` seems the most reasonable > > choice. > > > `isLocInHeaderFile(SourceLocation, ...)` is a nice idea, but we'd need to be > more specific: either `isExpansionLocInHeaderFile(SourceLoc, ...)` or > `isSpellingLocInHeaderFile(SourceLoc, ...)` (or both). > Yeah, for extensions-less header file, you can pass the string like .h,,.hpp, > which is a bit of weird. Do you have a better idea here? Passing a string > into header-file-extensions seems the most reasonable choice. I thought those user configurations from the command line were in YAML or JSON format, those both have the notion of lists, don't they? I would imagine this would take a SmallVectorImpl for the list of extensions. Comment at: clang-tidy/ClangTidyOptions.h:216 @@ +215,3 @@ +/// HeaderFileExtensions. +bool endWithHeaderFileExtensions(llvm::StringRef FileName, + llvm::StringRef HeaderFileExtensions); aaron.ballman wrote: > alexfh wrote: > > hokein wrote: > > > aaron.ballman wrote: > > > > alexfh wrote: > > > > > This function doesn't belong here. I'm also not sure we need this > > > > > function at all. First, it's ineffective to split the string > > > > > containing the list of extensions each time. Second, if we store a > > > > > set of extensions, then we can just search for the actual file > > > > > extension in this set. > > > > endsWithHeaderFileExtension instead? However, given that uses of this > > > > all start with a SourceLocation, I wonder if that makes for a cleaner > > > > API: isLocInHeaderFile(SourceLocation, ); > > > > > > > > Also, how does this work if I want to include an extension-less file as > > > > the header file "extension?" It would be plausible if the extensions > > > > were passed in as a list, but as it stands it doesn't seem possible > > > > without weird conventions like leaving a blank in the list (e.g., > > > > `.h,,.hpp`), which seems error-prone. > > > > > > > > Also, I'm not certain what I can pass in. The documentation should be > > > > updated to state whether these extensions are intended to include the > > > > ".". > > > > endsWithHeaderFileExtension instead? However, given that uses of this > > > > all start with a SourceLocation, I wonder if that makes for a cleaner > > > > API: isLocInHeaderFile(SourceLocation, ); > > > > > > Using `SourceLocation` only is not enough to retrieve the belonging file > > > name (we need `SourceManager` too). > > > > > > >Also, how does this work if I want to include an extension-less file as > > > >the header file "extension?" It would be plausible if the extensions > > > >were passed in as a list, but as it stands it doesn't seem possible
Re: [PATCH] D16152: [clang-tidy] Add check performance-faster-string-find
aaron.ballman added a subscriber: aaron.ballman. aaron.ballman added a reviewer: aaron.ballman. Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:25 @@ +24,3 @@ + SmallVector Classes; + Option.split(Classes, ","); + return std::vector(Classes.begin(), Classes.end()); It might be nice for this to be more tolerant of whitespace around the commas. Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:47 @@ +46,3 @@ +void FasterStringFindCheck::registerMatchers(MatchFinder *Finder) { + const auto SingleChar = + expr(ignoringParenCasts(stringLiteral(lengthIsOne()).bind("literal"))); Can you also disable registration of these matches outside of C++ mode? http://reviews.llvm.org/D16152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15914: [OpenCL] Pipe builtin functions
pxli168 added a comment. I think we'd better to ask about to add this in. The pipe type support actually have no use without builtins to read or write it. Comment at: lib/CodeGen/CGBuiltin.cpp:1981 @@ +1980,3 @@ + const char *Name = + (BuiltinID == Builtin::BIread_pipe) ? "read_pipe_2" : "write_pipe_2"; + // Re-Creating the function type for this call, since the original type Anastasia wrote: > pekka.jaaskelainen wrote: > > Shouldn't these have underscores, because they are not reserved OpenCL > > keywords/builtins? Cannot user functions be called read_pipe_2 or > > write_pipe_2? > Yes, definitely! Add prefix "__" to each generated name! I think this should > be done for all of them! I think that's a good idea! Comment at: lib/Sema/SemaChecking.cpp:291 @@ +290,3 @@ + bool isValid = true; + // TODO: Now we check for all the pipe builtin with access qualifier, but in + // OpenCL spec does not tell about group functions, need to fix after get Anastasia wrote: > Please, remove this TODO. Have you created the bug to Khronos already? > > It seems sensible enough the access qualifiers must match. How can > reserve/commit_read_pipe be called with a write_only pipe? OK, I will. I tried to create an account to do, but it seems I could not receive the email. I will attach the bug report web here once I created one. Comment at: lib/Sema/SemaChecking.cpp:378 @@ +377,3 @@ +S.Diag(Call->getLocStart(), diag::err_opencl_builtin_pipe_arg_num) +<< getFunctionName(Call) << Call->getSourceRange(); +return true; Anastasia wrote: > I don't think we have any arguments there. It's all declared as variadic. But > it seems to work without setting it. So I am not sure it's necessary to set > it at all. The return type of int and uint is set in the builtin.def. http://reviews.llvm.org/D15914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.
alexfh added inline comments. Comment at: clang-tidy/ClangTidyOptions.h:216 @@ +215,3 @@ +/// HeaderFileExtensions. +bool endWithHeaderFileExtensions(llvm::StringRef FileName, + llvm::StringRef HeaderFileExtensions); aaron.ballman wrote: > aaron.ballman wrote: > > alexfh wrote: > > > hokein wrote: > > > > aaron.ballman wrote: > > > > > alexfh wrote: > > > > > > This function doesn't belong here. I'm also not sure we need this > > > > > > function at all. First, it's ineffective to split the string > > > > > > containing the list of extensions each time. Second, if we store a > > > > > > set of extensions, then we can just search for the actual file > > > > > > extension in this set. > > > > > endsWithHeaderFileExtension instead? However, given that uses of this > > > > > all start with a SourceLocation, I wonder if that makes for a cleaner > > > > > API: isLocInHeaderFile(SourceLocation, ); > > > > > > > > > > Also, how does this work if I want to include an extension-less file > > > > > as the header file "extension?" It would be plausible if the > > > > > extensions were passed in as a list, but as it stands it doesn't seem > > > > > possible without weird conventions like leaving a blank in the list > > > > > (e.g., `.h,,.hpp`), which seems error-prone. > > > > > > > > > > Also, I'm not certain what I can pass in. The documentation should be > > > > > updated to state whether these extensions are intended to include the > > > > > ".". > > > > > endsWithHeaderFileExtension instead? However, given that uses of this > > > > > all start with a SourceLocation, I wonder if that makes for a cleaner > > > > > API: isLocInHeaderFile(SourceLocation, ); > > > > > > > > Using `SourceLocation` only is not enough to retrieve the belonging > > > > file name (we need `SourceManager` too). > > > > > > > > >Also, how does this work if I want to include an extension-less file > > > > >as the header file "extension?" It would be plausible if the > > > > >extensions were passed in as a list, but as it stands it doesn't seem > > > > >possible without weird conventions like leaving a blank in the list > > > > >(e.g., .h,,.hpp), which seems error-prone. > > > > > > > > Yeah, for extensions-less header file, you can pass the string like > > > > `.h,,.hpp`, which is a bit of weird. Do you have a better idea here? > > > > Passing a string into `header-file-extensions` seems the most > > > > reasonable choice. > > > > > > > `isLocInHeaderFile(SourceLocation, ...)` is a nice idea, but we'd need to > > > be more specific: either `isExpansionLocInHeaderFile(SourceLoc, ...)` or > > > `isSpellingLocInHeaderFile(SourceLoc, ...)` (or both). > > > Yeah, for extensions-less header file, you can pass the string like > > > .h,,.hpp, which is a bit of weird. Do you have a better idea here? > > > Passing a string into header-file-extensions seems the most reasonable > > > choice. > > > > I thought those user configurations from the command line were in YAML or > > JSON format, those both have the notion of lists, don't they? I would > > imagine this would take a SmallVectorImpl for the > > list of extensions. > > isLocInHeaderFile(SourceLocation, ...) is a nice idea, but we'd need to be > > more specific: either isExpansionLocInHeaderFile(SourceLoc, ...) or > > isSpellingLocInHeaderFile(SourceLoc, ...) (or both). > > That's true, and I would think both are reasonable to add. I rather prefer > that as an API instead of passing around file names as strings, personally. User configurations are stored in YAML, however, CheckOptions is a map of strings to strings, so we can't use YAML lists to represent lists of extensions. I personally see nothing wrong with `",.h,.hh,.hpp"` for example, to represent the list of extensions ( being the first one makes it somewhat stand out and provided there's a good documentation, this shouldn't be too confusing). Repository: rL LLVM http://reviews.llvm.org/D16113 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16152: [clang-tidy] Add check performance-faster-string-find
alexfh added inline comments. Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:25 @@ +24,3 @@ + SmallVector Classes; + Option.split(Classes, ","); + return std::vector(Classes.begin(), Classes.end()); aaron.ballman wrote: > It might be nice for this to be more tolerant of whitespace around the commas. I don't see much value in this, since it's not someone will change frequently (I guess, once per codebase on average). http://reviews.llvm.org/D16152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16183: Added CheckName field to YAML report
alexfh added a comment. Thanks for addressing the comments. However, the main concern is this: In http://reviews.llvm.org/D16183#326759, @alexfh wrote: > I'm not sure `tooling::Replacement` is the best place to store check name. > Maybe create a separate wrapper class and serialize it instead > (clang-apply-replacements will have to be changed to support this format as > well). This could be `ClangTidyDiagnostic` or just `Diagnostic`, and we could > also store the message and other useful information in it. http://reviews.llvm.org/D16183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16152: [clang-tidy] Add check performance-faster-string-find
aaron.ballman added inline comments. Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:25 @@ +24,3 @@ + SmallVector Classes; + Option.split(Classes, ","); + return std::vector(Classes.begin(), Classes.end()); alexfh wrote: > aaron.ballman wrote: > > It might be nice for this to be more tolerant of whitespace around the > > commas. > I don't see much value in this, since it's not someone will change frequently > (I guess, once per codebase on average). > I don't see much value in this, since it's not someone will change frequently > (I guess, once per codebase on average). Anything that's user-facing should be fault tolerant and sanitized for a better user experience. http://reviews.llvm.org/D16152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15120: Add support for __float128 type to be used by targets that support it
hubert.reinterpretcast added inline comments. Comment at: lib/AST/ASTContext.cpp:4635 @@ +4634,3 @@ + // If the two sides have Float128Rank and LongDoubleRank and the two types + // have the same representation on this platform, they have equal rank. + if ((LHSR == LongDoubleRank && RHSR == Float128Rank) || rjmccall wrote: > This is a really bad language rule with nasty implications for a lot of code. > Is there a really urgent need to emulate some other compiler bug-for-bug > here? > > It's okay for there to a formal rank difference even when types have the same > representation — that's basically always true for the integer types — so > unless there's a very specific and very good reason not to, let's just use > the rule that float128 has higher rank than long double. I am not very keen on the [[ http://reviews.llvm.org/D15120#inline-133280 | implications ]] of having the equal rank either. I believe the case where neither type represents the full set of values of the other should be considered "unordered" though (currently that is handled by `typesNotCompatible()`, which is in need of a better name since the function does not deal with "compatible types" as defined by C11 subclause 6.2.7). Repository: rL LLVM http://reviews.llvm.org/D15120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.
aaron.ballman added inline comments. Comment at: clang-tidy/ClangTidyOptions.h:216 @@ +215,3 @@ +/// HeaderFileExtensions. +bool endWithHeaderFileExtensions(llvm::StringRef FileName, + llvm::StringRef HeaderFileExtensions); alexfh wrote: > aaron.ballman wrote: > > aaron.ballman wrote: > > > alexfh wrote: > > > > hokein wrote: > > > > > aaron.ballman wrote: > > > > > > alexfh wrote: > > > > > > > This function doesn't belong here. I'm also not sure we need this > > > > > > > function at all. First, it's ineffective to split the string > > > > > > > containing the list of extensions each time. Second, if we store > > > > > > > a set of extensions, then we can just search for the actual file > > > > > > > extension in this set. > > > > > > endsWithHeaderFileExtension instead? However, given that uses of > > > > > > this all start with a SourceLocation, I wonder if that makes for a > > > > > > cleaner API: isLocInHeaderFile(SourceLocation, ); > > > > > > > > > > > > Also, how does this work if I want to include an extension-less > > > > > > file as the header file "extension?" It would be plausible if the > > > > > > extensions were passed in as a list, but as it stands it doesn't > > > > > > seem possible without weird conventions like leaving a blank in the > > > > > > list (e.g., `.h,,.hpp`), which seems error-prone. > > > > > > > > > > > > Also, I'm not certain what I can pass in. The documentation should > > > > > > be updated to state whether these extensions are intended to > > > > > > include the ".". > > > > > > endsWithHeaderFileExtension instead? However, given that uses of > > > > > > this all start with a SourceLocation, I wonder if that makes for a > > > > > > cleaner API: isLocInHeaderFile(SourceLocation, ); > > > > > > > > > > Using `SourceLocation` only is not enough to retrieve the belonging > > > > > file name (we need `SourceManager` too). > > > > > > > > > > >Also, how does this work if I want to include an extension-less file > > > > > >as the header file "extension?" It would be plausible if the > > > > > >extensions were passed in as a list, but as it stands it doesn't > > > > > >seem possible without weird conventions like leaving a blank in the > > > > > >list (e.g., .h,,.hpp), which seems error-prone. > > > > > > > > > > Yeah, for extensions-less header file, you can pass the string like > > > > > `.h,,.hpp`, which is a bit of weird. Do you have a better idea here? > > > > > Passing a string into `header-file-extensions` seems the most > > > > > reasonable choice. > > > > > > > > > `isLocInHeaderFile(SourceLocation, ...)` is a nice idea, but we'd need > > > > to be more specific: either `isExpansionLocInHeaderFile(SourceLoc, > > > > ...)` or `isSpellingLocInHeaderFile(SourceLoc, ...)` (or both). > > > > Yeah, for extensions-less header file, you can pass the string like > > > > .h,,.hpp, which is a bit of weird. Do you have a better idea here? > > > > Passing a string into header-file-extensions seems the most reasonable > > > > choice. > > > > > > I thought those user configurations from the command line were in YAML or > > > JSON format, those both have the notion of lists, don't they? I would > > > imagine this would take a SmallVectorImpl for the > > > list of extensions. > > > isLocInHeaderFile(SourceLocation, ...) is a nice idea, but we'd need to > > > be more specific: either isExpansionLocInHeaderFile(SourceLoc, ...) or > > > isSpellingLocInHeaderFile(SourceLoc, ...) (or both). > > > > That's true, and I would think both are reasonable to add. I rather prefer > > that as an API instead of passing around file names as strings, personally. > User configurations are stored in YAML, however, CheckOptions is a map of > strings to strings, so we can't use YAML lists to represent lists of > extensions. > I personally see nothing wrong with `",.h,.hh,.hpp"` for example, to > represent the list of extensions ( being the first one makes it > somewhat stand out and provided there's a good documentation, this shouldn't > be too confusing). > I personally see nothing wrong with ",.h,.hh,.hpp" for example, to represent > the list of extensions ( being the first one makes it somewhat stand > out and provided there's a good documentation, this shouldn't be too > confusing). I find it to be more clever than intuitive. If it were not user-facing, I would be less concerned. I don't think users should have to read documentation to figure out the *syntax* of how to pass options if we can at all avoid it. ;-) Regardless, I would like to separate the two concepts -- there's the way we expose the option to the users, and there's our internal APIs that we call. I don't think the internal API has to take such an awkward thing directly just because the user-facing option has to be that way currently. Repository: rL LLVM http://reviews.llvm.org/D16113 __
Re: [PATCH] D16152: [clang-tidy] Add check performance-faster-string-find
alexfh added inline comments. Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:25 @@ +24,3 @@ + SmallVector Classes; + Option.split(Classes, ","); + return std::vector(Classes.begin(), Classes.end()); aaron.ballman wrote: > alexfh wrote: > > aaron.ballman wrote: > > > It might be nice for this to be more tolerant of whitespace around the > > > commas. > > I don't see much value in this, since it's not someone will change > > frequently (I guess, once per codebase on average). > > I don't see much value in this, since it's not someone will change > > frequently (I guess, once per codebase on average). > > Anything that's user-facing should be fault tolerant and sanitized for a > better user experience. I also don't feel strongly about this. Trimming all strings after splitting should be a trivial 2-line addition to this patch. http://reviews.llvm.org/D16152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15914: [OpenCL] Pipe builtin functions
pxli168 updated this revision to Diff 44881. pxli168 added a comment. Add prefix "__" to CodeGen builtin function name. http://reviews.llvm.org/D15914 Files: include/clang/Basic/Builtins.def include/clang/Basic/Builtins.h include/clang/Basic/DiagnosticSemaKinds.td lib/Basic/Builtins.cpp lib/CodeGen/CGBuiltin.cpp lib/Sema/SemaChecking.cpp test/CodeGenOpenCL/pipe_builtin.cl test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl Index: test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl === --- /dev/null +++ test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl @@ -0,0 +1,55 @@ +// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0 + +void test1(read_only pipe int p, global int* ptr){ + int tmp; + reserve_id_t rid; + + // read/write_pipe + read_pipe(tmp, p);// expected-error {{first argument to read_pipe must be a pipe type}} + read_pipe(p); // expected-error {{invalid number of arguments to function: read_pipe}} + read_pipe(p, tmp, tmp, ptr); // expected-error {{invalid argument type to function read_pipe (expecting 'reserve_id_t')}} + read_pipe(p, rid, rid, ptr); // expected-error {{invalid argument type to function read_pipe (expecting 'unsigned int')}} + read_pipe(p, tmp); // expected-error {{invalid argument type to function read_pipe (expecting 'int *')}} + write_pipe(p, ptr);// expected-error {{invalid pipe access modifier (expecting write_only)}} + write_pipe(p, rid, tmp, ptr);// expected-error {{invalid pipe access modifier (expecting write_only)}} + + // reserve_read/write_pipe + reserve_read_pipe(p, ptr);// expected-error{{invalid argument type to function reserve_read_pipe (expecting 'unsigned int')}} + work_group_reserve_read_pipe(tmp, tmp);// expected-error{{first argument to work_group_reserve_read_pipe must be a pipe type}} + sub_group_reserve_write_pipe(p, tmp);// expected-error{{invalid pipe access modifier (expecting write_only)}} + + // commit_read/write_pipe + commit_read_pipe(tmp, rid);// expected-error{{first argument to commit_read_pipe must be a pipe type}} + work_group_commit_read_pipe(p, tmp);// expected-error{{invalid argument type to function work_group_commit_read_pipe (expecting 'reserve_id_t')}} + sub_group_commit_write_pipe(p, tmp);// expected-error{{nvalid pipe access modifier (expecting write_only)}} +} + +void test2(write_only pipe int p, global int* ptr){ + int tmp; + reserve_id_t rid; + + // read/write_pipe + write_pipe(tmp, p);// expected-error {{first argument to write_pipe must be a pipe type}} + write_pipe(p); // expected-error {{invalid number of arguments to function: write_pipe}} + write_pipe(p, tmp, tmp, ptr); // expected-error {{invalid argument type to function write_pipe (expecting 'reserve_id_t')}} + write_pipe(p, rid, rid, ptr); // expected-error {{invalid argument type to function write_pipe (expecting 'unsigned int')}} + write_pipe(p, tmp); // expected-error {{invalid argument type to function write_pipe (expecting 'int *')}} + read_pipe(p, ptr);// expected-error {{invalid pipe access modifier (expecting read_only)}} + read_pipe(p, rid, tmp, ptr);// expected-error {{invalid pipe access modifier (expecting read_only)}} + + // reserve_read/write_pipe + reserve_write_pipe(p, ptr);// expected-error{{invalid argument type to function reserve_write_pipe (expecting 'unsigned int')}} + work_group_reserve_write_pipe(tmp, tmp);// expected-error{{first argument to work_group_reserve_write_pipe must be a pipe type}} + sub_group_reserve_read_pipe(p, tmp);// expected-error{{invalid pipe access modifier (expecting read_only)}} + + // commit_read/write_pipe + commit_write_pipe(tmp, rid);// expected-error{{first argument to commit_write_pipe must be a pipe type}} + work_group_commit_write_pipe(p, tmp);// expected-error{{invalid argument type to function work_group_commit_write_pipe (expecting 'reserve_id_t')}} + sub_group_commit_read_pipe(p, tmp);// expected-error{{nvalid pipe access modifier (expecting read_only)}} +} + +void test3(){ + int tmp; + get_pipe_num_packets(tmp);// expected-error {{first argument to get_pipe_num_packets must be a pipe type}} + get_pipe_max_packets(tmp);// expected-error {{first argument to get_pipe_max_packets must be a pipe type}} +} Index: test/CodeGenOpenCL/pipe_builtin.cl === --- /dev/null +++ test/CodeGenOpenCL/pipe_builtin.cl @@ -0,0 +1,61 @@ +// RUN: %clang_cc1 -emit-llvm -O0 -cl-std=CL2.0 -o - %s | FileCheck %s + +// CHECK: %opencl.pipe_t = type opaque +// CHECK: %opencl.reserve_id_t = type opaque + +void test1(read_only pipe int p, global int *ptr) { + // CHECK: call i32 @__read_pipe_2(%opencl.pipe_t* %{{.*}}, i8 addrspace(4)* %{{.*}}) + read_pipe(p, ptr); + // CHECK: call %opencl.reserve_id_t* @__reserve_read_pipe(%opencl.pipe_t* %{{.*}}, i32 {{.*}}) + reserve_i
r257774 - [WebAssembly] Configure some simple include paths and runtime library settings.
Author: djg Date: Thu Jan 14 10:00:13 2016 New Revision: 257774 URL: http://llvm.org/viewvc/llvm-project?rev=257774&view=rev Log: [WebAssembly] Configure some simple include paths and runtime library settings. Modified: cfe/trunk/lib/Driver/ToolChains.cpp cfe/trunk/lib/Driver/ToolChains.h Modified: cfe/trunk/lib/Driver/ToolChains.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=257774&r1=257773&r2=257774&view=diff == --- cfe/trunk/lib/Driver/ToolChains.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains.cpp Thu Jan 14 10:00:13 2016 @@ -4449,6 +4449,29 @@ void WebAssembly::addClangTargetOptions( CC1Args.push_back("-fuse-init-array"); } +ToolChain::RuntimeLibType WebAssembly::GetDefaultRuntimeLibType() const { + return ToolChain::RLT_CompilerRT; +} + +ToolChain::CXXStdlibType WebAssembly::GetCXXStdlibType(const ArgList &Args) const { + return ToolChain::CST_Libcxx; +} + +void WebAssembly::AddClangSystemIncludeArgs(const ArgList &DriverArgs, +ArgStringList &CC1Args) const { + if (!DriverArgs.hasArg(options::OPT_nostdinc)) +addSystemInclude(DriverArgs, CC1Args, getDriver().SysRoot + "/include"); +} + +void WebAssembly::AddClangCXXStdlibIncludeArgs( + const llvm::opt::ArgList &DriverArgs, + llvm::opt::ArgStringList &CC1Args) const { + if (!DriverArgs.hasArg(options::OPT_nostdlibinc) && + !DriverArgs.hasArg(options::OPT_nostdincxx)) +addSystemInclude(DriverArgs, CC1Args, + getDriver().SysRoot + "/include/c++/v1"); +} + Tool *WebAssembly::buildLinker() const { return new tools::wasm::Linker(*this); } Modified: cfe/trunk/lib/Driver/ToolChains.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.h?rev=257774&r1=257773&r2=257774&view=diff == --- cfe/trunk/lib/Driver/ToolChains.h (original) +++ cfe/trunk/lib/Driver/ToolChains.h Thu Jan 14 10:00:13 2016 @@ -1107,6 +1107,14 @@ private: bool HasNativeLLVMSupport() const override; void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args) const override; + RuntimeLibType GetDefaultRuntimeLibType() const override; + CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList &Args) const override; + void AddClangSystemIncludeArgs( + const llvm::opt::ArgList &DriverArgs, + llvm::opt::ArgStringList &CC1Args) const override; + void AddClangCXXStdlibIncludeArgs( + const llvm::opt::ArgList &DriverArgs, + llvm::opt::ArgStringList &CC1Args) const override; Tool *buildLinker() const override; }; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16152: [clang-tidy] Add check performance-faster-string-find
sbenza updated this revision to Diff 44886. sbenza marked 4 inline comments as done. sbenza added a comment. Added template and macro tests. Fixed warning message. http://reviews.llvm.org/D16152 Files: clang-tidy/performance/CMakeLists.txt clang-tidy/performance/FasterStringFindCheck.cpp clang-tidy/performance/FasterStringFindCheck.h clang-tidy/performance/PerformanceTidyModule.cpp docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/performance-faster-string-find.rst test/clang-tidy/performance-faster-string-find.cpp Index: test/clang-tidy/performance-faster-string-find.cpp === --- /dev/null +++ test/clang-tidy/performance-faster-string-find.cpp @@ -0,0 +1,99 @@ +// RUN: %check_clang_tidy %s performance-faster-string-find %t -- \ +// RUN: -config="{CheckOptions: \ +// RUN: [{key: performance-faster-string-find.StringLikeClasses, \ +// RUN: value: 'std::basic_string,::llvm::StringRef'}]}" -- + +namespace std { +template +struct basic_string { + int find(const Char *, int = 0) const; + int find(const Char *, int, int) const; + int rfind(const Char *) const; + int find_first_of(const Char *) const; + int find_first_not_of(const Char *) const; + int find_last_of(const Char *) const; + int find_last_not_of(const Char *) const; +}; + +typedef basic_string string; +} // namespace std + +namespace llvm { +struct StringRef { + int find(const char *) const; +}; +} // namespace llvm + +struct NotStringRef { + int find(const char *); +}; + +void StringFind() { + std::string Str; + + Str.find("a"); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: find called with a string literal consisting of a single character; consider using the more effective overload accepting a character [performance-faster-string-find] + // CHECK-FIXES: Str.find('a'); + + // Works with the pos argument. + Str.find("a", 1); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: find called with a string literal + // CHECK-FIXES: Str.find('a', 1); + + // Doens't work with strings smaller or larger than 1 char. + Str.find(""); + Str.find("ab"); + + // Doesn't do anything with the 3 argument overload. + Str.find("a", 1, 1); + + // Other methods that can also be replaced + Str.rfind("a"); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: rfind called with a string literal + // CHECK-FIXES: Str.rfind('a'); + Str.find_first_of("a"); + // CHECK-MESSAGES: [[@LINE-1]]:21: warning: find_first_of called with a string + // CHECK-FIXES: Str.find_first_of('a'); + Str.find_first_not_of("a"); + // CHECK-MESSAGES: [[@LINE-1]]:25: warning: find_first_not_of called with a + // CHECK-FIXES: Str.find_first_not_of('a'); + Str.find_last_of("a"); + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: find_last_of called with a string + // CHECK-FIXES: Str.find_last_of('a'); + Str.find_last_not_of("a"); + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: find_last_not_of called with a + // CHECK-FIXES: Str.find_last_not_of('a'); + + // Also with other types, but only if it was specified in the options. + llvm::StringRef sr; + sr.find("x"); + // CHECK-MESSAGES: [[@LINE-1]]:11: warning: find called with a string literal + // CHECK-FIXES: sr.find('x'); + NotStringRef nsr; + nsr.find("x"); +} + + +template +int FindTemplateDependant(T value) { + return value.find("A"); +} +template +int FindTemplateNotDependant(T pos) { + return std::string().find("A", pos); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: find called with a string literal + // CHECK-FIXES: return std::string().find('A', pos); +} + +int FindStr() { + return FindTemplateDependant(std::string()) + FindTemplateNotDependant(1); +} + +#define STR_MACRO(str) str.find("A") +#define POS_MACRO(pos) std::string().find("A",pos) + +int Macros() { + return STR_MACRO(std::string()) + POS_MACRO(1); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: find called with a string literal + // CHECK-MESSAGES: [[@LINE-2]]:37: warning: find called with a string literal +} Index: docs/clang-tidy/checks/performance-faster-string-find.rst === --- /dev/null +++ docs/clang-tidy/checks/performance-faster-string-find.rst @@ -0,0 +1,18 @@ +.. title:: clang-tidy - performance-faster-string-find + +performance-faster-string-find +== + +Optimize calls to std::string::find() and friends when the needle passed is +a single character string literal. +The character literal overload is more efficient. + +Examples: + +.. code-block:: c++ + + str.find("A"); + + // becomes + + str.find('A'); Index: docs/clang-tidy/checks/list.rst === --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -75,6 +75,7 @@ modernize-use-default modernize-use-nullptr modernize-use-override + performance-faster-string-find readability-braces-around-
Re: [PATCH] D16152: [clang-tidy] Add check performance-faster-string-find
sbenza marked an inline comment as done. Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:51 @@ +50,3 @@ + const auto StringFindFunctions = + anyOf(hasName("find"), hasName("rfind"), hasName("find_first_of"), +hasName("find_first_not_of"), hasName("find_last_of"), alexfh wrote: > Probably out of scope of this patch, but I wonder how many times `hasName` is > still more effective than one `matchesName`? Maybe we should add a > `matchesName` variant for unqualified names or hasName taking a list of > strings? I have no idea how much more expensive is the regex engine. It also makes the code harder to read. I would totally be in favor of a variadic hasAnyName(). It is shorter and can be much faster to run. Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:75 @@ +74,3 @@ + + diag(Literal->getLocStart(), "char overload is more efficient") + << FixItHint::CreateReplacement( alexfh wrote: > The message might be confusing in some situations (e.g. macros). I think, it > should at least mention what method (and maybe of what class) is in question, > e.g. "std::string::find() called with a string literal consisting of a single > character; consider using the more effective overload accepting a character". Used unqualified function name instead. The qualified name for find is std::basic_string::find, which might be distracting. http://reviews.llvm.org/D16152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.
alexfh added inline comments. Comment at: clang-tidy/ClangTidyOptions.h:216 @@ +215,3 @@ +/// HeaderFileExtensions. +bool endWithHeaderFileExtensions(llvm::StringRef FileName, + llvm::StringRef HeaderFileExtensions); aaron.ballman wrote: > alexfh wrote: > > aaron.ballman wrote: > > > aaron.ballman wrote: > > > > alexfh wrote: > > > > > hokein wrote: > > > > > > aaron.ballman wrote: > > > > > > > alexfh wrote: > > > > > > > > This function doesn't belong here. I'm also not sure we need > > > > > > > > this function at all. First, it's ineffective to split the > > > > > > > > string containing the list of extensions each time. Second, if > > > > > > > > we store a set of extensions, then we can just search for the > > > > > > > > actual file extension in this set. > > > > > > > endsWithHeaderFileExtension instead? However, given that uses of > > > > > > > this all start with a SourceLocation, I wonder if that makes for > > > > > > > a cleaner API: isLocInHeaderFile(SourceLocation, ); > > > > > > > > > > > > > > Also, how does this work if I want to include an extension-less > > > > > > > file as the header file "extension?" It would be plausible if the > > > > > > > extensions were passed in as a list, but as it stands it doesn't > > > > > > > seem possible without weird conventions like leaving a blank in > > > > > > > the list (e.g., `.h,,.hpp`), which seems error-prone. > > > > > > > > > > > > > > Also, I'm not certain what I can pass in. The documentation > > > > > > > should be updated to state whether these extensions are intended > > > > > > > to include the ".". > > > > > > > endsWithHeaderFileExtension instead? However, given that uses of > > > > > > > this all start with a SourceLocation, I wonder if that makes for > > > > > > > a cleaner API: isLocInHeaderFile(SourceLocation, ); > > > > > > > > > > > > Using `SourceLocation` only is not enough to retrieve the belonging > > > > > > file name (we need `SourceManager` too). > > > > > > > > > > > > >Also, how does this work if I want to include an extension-less > > > > > > >file as the header file "extension?" It would be plausible if the > > > > > > >extensions were passed in as a list, but as it stands it doesn't > > > > > > >seem possible without weird conventions like leaving a blank in > > > > > > >the list (e.g., .h,,.hpp), which seems error-prone. > > > > > > > > > > > > Yeah, for extensions-less header file, you can pass the string like > > > > > > `.h,,.hpp`, which is a bit of weird. Do you have a better idea > > > > > > here? Passing a string into `header-file-extensions` seems the most > > > > > > reasonable choice. > > > > > > > > > > > `isLocInHeaderFile(SourceLocation, ...)` is a nice idea, but we'd > > > > > need to be more specific: either > > > > > `isExpansionLocInHeaderFile(SourceLoc, ...)` or > > > > > `isSpellingLocInHeaderFile(SourceLoc, ...)` (or both). > > > > > Yeah, for extensions-less header file, you can pass the string like > > > > > .h,,.hpp, which is a bit of weird. Do you have a better idea here? > > > > > Passing a string into header-file-extensions seems the most > > > > > reasonable choice. > > > > > > > > I thought those user configurations from the command line were in YAML > > > > or JSON format, those both have the notion of lists, don't they? I > > > > would imagine this would take a SmallVectorImpl > > > > for the list of extensions. > > > > isLocInHeaderFile(SourceLocation, ...) is a nice idea, but we'd need to > > > > be more specific: either isExpansionLocInHeaderFile(SourceLoc, ...) or > > > > isSpellingLocInHeaderFile(SourceLoc, ...) (or both). > > > > > > That's true, and I would think both are reasonable to add. I rather > > > prefer that as an API instead of passing around file names as strings, > > > personally. > > User configurations are stored in YAML, however, CheckOptions is a map of > > strings to strings, so we can't use YAML lists to represent lists of > > extensions. > > I personally see nothing wrong with `",.h,.hh,.hpp"` for example, to > > represent the list of extensions ( being the first one makes it > > somewhat stand out and provided there's a good documentation, this > > shouldn't be too confusing). > > I personally see nothing wrong with ",.h,.hh,.hpp" for example, to > > represent the list of extensions ( being the first one makes it > > somewhat stand out and provided there's a good documentation, this > > shouldn't be too confusing). > > I find it to be more clever than intuitive. If it were not user-facing, I > would be less concerned. I don't think users should have to read > documentation to figure out the *syntax* of how to pass options if we can at > all avoid it. ;-) > > Regardless, I would like to separate the two concepts -- there's the way we > expose the option to the users, and there's our internal APIs that we call. I > don't think the internal
Re: [PATCH] D16047: [OpenCL] Add Sema checks for OpenCL 2.0
pxli168 added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:593 @@ -592,2 +592,3 @@ +def err_no_declarators : Error<"declaration does not declare anything">; def ext_typedef_without_a_name : ExtWarn<"typedef requires a name">, InGroup; Anastasia wrote: > I just can't understand the intention here. Could you give any code example > or reference to spec? I will try, Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7670 @@ +7669,3 @@ + "%0 can only be used as a function parameter">; +def err_opencl_atomic_init_addressspace : Error< + "initialization of atomic variables is restricted to variables in global address space in opencl">; Anastasia wrote: > Could you do something like: > > def err_atomic_init_constant : Error< > "atomic variable can only be %select{assigned|initialised}0 to a > compile time constant" > " in the declaration statement in the program scope">; > Seems good. But that error message was from spec, this will change it. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7674 @@ +7673,3 @@ + "invalid block prototype, variadic arguments are not allowed in opencl">; +def err_opencl_invalid_block_array : Error< + "array of block is invalid in OpenCL">; Anastasia wrote: > Could we combine err_opencl_invalid_block_array and > err_opencl_pointer_to_image saying something like: > > "Declaring a %select{pointer|array}0 of type %1 is not allowed in OpenCL" OK, it will save few lines. Comment at: lib/Sema/SemaDecl.cpp:5724 @@ +5723,3 @@ + R->isPipeType()) { +Diag(D.getIdentifierLoc(), + diag::err_opencl_type_can_only_be_used_as_function_parameter) Anastasia wrote: > Some of them have to be. For example for C types that we use differently in > CL. But for CL only types do we need to check additionally that it's CL? Do > we not know it already? Yes, but it seems all old code write in that way. I just follow the style. Comment at: lib/Sema/SemaExpr.cpp:6299 @@ -6286,3 +6298,3 @@ // Now check the two expressions. if (LHS.get()->getType()->isVectorType() || RHS.get()->getType()->isVectorType()) Anastasia wrote: > I am not sure what the question is? > > I think using block in a condition should be disallowed. Could you add this > to testing as well? I don't see any thing talk about the block and condition. Spec only tells about block and selection. Comment at: lib/Sema/SemaExpr.cpp:6316 @@ +6315,3 @@ + if (getLangOpts().OpenCL && getLangOpts().OpenCLVersion >= 200) { +// should output error for both LHS and RHS, use | instead || +if (checkBlockType(*this, LHS.get()) | checkBlockType(*this, RHS.get())) Anastasia wrote: > Could you remove this comment? OK. but the "|" may seems strange. I just want to explain it. Comment at: lib/Sema/SemaExpr.cpp:7550 @@ -7527,3 +7549,3 @@ LHSType, RHSVecType->getElementType(), - RHSType)) + RHSType, Loc)) return RHSType; Anastasia wrote: > I am not clear about the purpose of this change. That seems some dead experiment. ): Comment at: lib/Sema/SemaExpr.cpp:10115 @@ +10114,3 @@ +// Block. +if (S.getLangOpts().OpenCL && S.getLangOpts().OpenCLVersion >= 200 && +Result->isBlockPointerType()) { Anastasia wrote: > The code above seems to do similar. Could we combine into one > function/diagnostic? Yes, they are. But they live in different function test for operand "&" and "*" and maybe it is hard to merge them together. I have uploaded the full diff as you asked and uou can expand the code to see they are in two function check for the two unary operators. Comment at: lib/Sema/SemaInit.cpp:6139 @@ +6138,3 @@ + // OpenCL v2.0 s6.13.11.1 - The ATOMIC_VAR_INIT macro expands to a token + // sequence suitable for initializing an atomic object of a type that is + // initialization-compatible with value. An atomic object with automatic Anastasia wrote: > I don't think we need to copy the spec text here. I would like to see some > explanation of the code actually. > > Could you write something like: "Non-program scope atomic variables can't be > initialized." But the example said there can be program scope atomic that only in global address space. > This macro can only be used to initialize atomic objects that are declared in > program scope in the global address space The quote from spec said so. Comment at: lib/Sema/SemaInit.cpp:6155 @@ +6154,3 @@ + TyQualifiers.getAddressSpace() == LangAS::opencl_global; +if (!HasGlobalAS && Entity.getKind() == InitializedEntity::EK_Variable && +Arg
Re: [PATCH] D16152: [clang-tidy] Add check performance-faster-string-find
sbenza updated this revision to Diff 44889. sbenza marked 2 inline comments as done. sbenza added a comment. Added support for non 'char' chars. http://reviews.llvm.org/D16152 Files: clang-tidy/performance/CMakeLists.txt clang-tidy/performance/FasterStringFindCheck.cpp clang-tidy/performance/FasterStringFindCheck.h clang-tidy/performance/PerformanceTidyModule.cpp docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/performance-faster-string-find.rst test/clang-tidy/performance-faster-string-find.cpp Index: test/clang-tidy/performance-faster-string-find.cpp === --- /dev/null +++ test/clang-tidy/performance-faster-string-find.cpp @@ -0,0 +1,110 @@ +// RUN: %check_clang_tidy %s performance-faster-string-find %t -- \ +// RUN: -config="{CheckOptions: \ +// RUN: [{key: performance-faster-string-find.StringLikeClasses, \ +// RUN: value: 'std::basic_string,::llvm::StringRef'}]}" -- + +namespace std { +template +struct basic_string { + int find(const Char *, int = 0) const; + int find(const Char *, int, int) const; + int rfind(const Char *) const; + int find_first_of(const Char *) const; + int find_first_not_of(const Char *) const; + int find_last_of(const Char *) const; + int find_last_not_of(const Char *) const; +}; + +typedef basic_string string; +typedef basic_string wstring; +} // namespace std + +namespace llvm { +struct StringRef { + int find(const char *) const; +}; +} // namespace llvm + +struct NotStringRef { + int find(const char *); +}; + +void StringFind() { + std::string Str; + + Str.find("a"); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: find called with a string literal consisting of a single character; consider using the more effective overload accepting a character [performance-faster-string-find] + // CHECK-FIXES: Str.find('a'); + + // Works with the pos argument. + Str.find("a", 1); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: find called with a string literal + // CHECK-FIXES: Str.find('a', 1); + + // Doens't work with strings smaller or larger than 1 char. + Str.find(""); + Str.find("ab"); + + // Doesn't do anything with the 3 argument overload. + Str.find("a", 1, 1); + + // Other methods that can also be replaced + Str.rfind("a"); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: rfind called with a string literal + // CHECK-FIXES: Str.rfind('a'); + Str.find_first_of("a"); + // CHECK-MESSAGES: [[@LINE-1]]:21: warning: find_first_of called with a string + // CHECK-FIXES: Str.find_first_of('a'); + Str.find_first_not_of("a"); + // CHECK-MESSAGES: [[@LINE-1]]:25: warning: find_first_not_of called with a + // CHECK-FIXES: Str.find_first_not_of('a'); + Str.find_last_of("a"); + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: find_last_of called with a string + // CHECK-FIXES: Str.find_last_of('a'); + Str.find_last_not_of("a"); + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: find_last_not_of called with a + // CHECK-FIXES: Str.find_last_not_of('a'); + + // std::wstring should work. + std::wstring WStr; + WStr.find(L"n"); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: find called with a string literal + // CHECK-FIXES: Str.find(L'n'); + // Even with unicode that fits in one wide char. + WStr.find(L"\x3A9"); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: find called with a string literal + // CHECK-FIXES: Str.find(L'\x3A9'); + + // Also with other types, but only if it was specified in the options. + llvm::StringRef sr; + sr.find("x"); + // CHECK-MESSAGES: [[@LINE-1]]:11: warning: find called with a string literal + // CHECK-FIXES: sr.find('x'); + NotStringRef nsr; + nsr.find("x"); +} + + +template +int FindTemplateDependant(T value) { + return value.find("A"); +} +template +int FindTemplateNotDependant(T pos) { + return std::string().find("A", pos); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: find called with a string literal + // CHECK-FIXES: return std::string().find('A', pos); +} + +int FindStr() { + return FindTemplateDependant(std::string()) + FindTemplateNotDependant(1); +} + +#define STR_MACRO(str) str.find("A") +#define POS_MACRO(pos) std::string().find("A",pos) + +int Macros() { + return STR_MACRO(std::string()) + POS_MACRO(1); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: find called with a string literal + // CHECK-MESSAGES: [[@LINE-2]]:37: warning: find called with a string literal +} Index: docs/clang-tidy/checks/performance-faster-string-find.rst === --- /dev/null +++ docs/clang-tidy/checks/performance-faster-string-find.rst @@ -0,0 +1,18 @@ +.. title:: clang-tidy - performance-faster-string-find + +performance-faster-string-find +== + +Optimize calls to std::string::find() and friends when the needle passed is +a single character string literal. +The character literal overload is more efficient. + +Examples: + +.. code-block::
Re: [PATCH] D16152: [clang-tidy] Add check performance-faster-string-find
sbenza marked 2 inline comments as done. sbenza added a comment. http://reviews.llvm.org/D16152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
RE: r256468 - On {mips, mipsel, mips64, mips64el}-freebsd, we need to pass any -G option to the assembler.
Did you get an answer to this question? I think it's just that the FreeBSD class hasn't refactored to that style yet. That switch statement is getting quite large so it would be a nice cleanup to switch to that style. > -Original Message- > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf > Of Joerg Sonnenberger via cfe-commits > Sent: 27 December 2015 11:59 > To: cfe-commits@lists.llvm.org > Subject: Re: r256468 - On {mips, mipsel, mips64, mips64el}-freebsd, we need > to pass any -G option to the assembler. > > On Sun, Dec 27, 2015 at 10:36:44AM -, Dimitry Andric via cfe-commits > wrote: > > Author: dim > > Date: Sun Dec 27 04:36:44 2015 > > New Revision: 256468 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=256468&view=rev > > Log: > > On {mips,mipsel,mips64,mips64el}-freebsd, we need to pass any -G option > to the assembler. > > Why is this reinventing the wheel and not using AddMIPSTargetArgs? > > Joerg > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16152: [clang-tidy] Add check performance-faster-string-find
aaron.ballman added inline comments. Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:29 @@ +28,3 @@ +Class = Class.trim(); + return std::vector(Classes.begin(), Classes.end()); +} I think hasName() will assert if given an empty string, so this should probably also guard against a class list like ",basic_string". Comment at: test/clang-tidy/performance-faster-string-find.cpp:9 @@ +8,3 @@ +struct basic_string { + int find(const Char *, int = 0) const; + int find(const Char *, int, int) const; > Should we move stubs to a common header(s)? Yes, please. However, that can be a separate patch that does something more comprehensive. http://reviews.llvm.org/D16152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16152: [clang-tidy] Add check performance-faster-string-find
sbenza updated this revision to Diff 44901. sbenza marked an inline comment as done. sbenza added a comment. More checks in argument parsing. http://reviews.llvm.org/D16152 Files: clang-tidy/performance/CMakeLists.txt clang-tidy/performance/FasterStringFindCheck.cpp clang-tidy/performance/FasterStringFindCheck.h clang-tidy/performance/PerformanceTidyModule.cpp docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/performance-faster-string-find.rst test/clang-tidy/performance-faster-string-find.cpp Index: test/clang-tidy/performance-faster-string-find.cpp === --- /dev/null +++ test/clang-tidy/performance-faster-string-find.cpp @@ -0,0 +1,110 @@ +// RUN: %check_clang_tidy %s performance-faster-string-find %t -- \ +// RUN: -config="{CheckOptions: \ +// RUN: [{key: performance-faster-string-find.StringLikeClasses, \ +// RUN: value: 'std::basic_string; ::llvm::StringRef;'}]}" -- + +namespace std { +template +struct basic_string { + int find(const Char *, int = 0) const; + int find(const Char *, int, int) const; + int rfind(const Char *) const; + int find_first_of(const Char *) const; + int find_first_not_of(const Char *) const; + int find_last_of(const Char *) const; + int find_last_not_of(const Char *) const; +}; + +typedef basic_string string; +typedef basic_string wstring; +} // namespace std + +namespace llvm { +struct StringRef { + int find(const char *) const; +}; +} // namespace llvm + +struct NotStringRef { + int find(const char *); +}; + +void StringFind() { + std::string Str; + + Str.find("a"); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: find called with a string literal consisting of a single character; consider using the more effective overload accepting a character [performance-faster-string-find] + // CHECK-FIXES: Str.find('a'); + + // Works with the pos argument. + Str.find("a", 1); + // CHECK-MESSAGES: [[@LINE-1]]:12: warning: find called with a string literal + // CHECK-FIXES: Str.find('a', 1); + + // Doens't work with strings smaller or larger than 1 char. + Str.find(""); + Str.find("ab"); + + // Doesn't do anything with the 3 argument overload. + Str.find("a", 1, 1); + + // Other methods that can also be replaced + Str.rfind("a"); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: rfind called with a string literal + // CHECK-FIXES: Str.rfind('a'); + Str.find_first_of("a"); + // CHECK-MESSAGES: [[@LINE-1]]:21: warning: find_first_of called with a string + // CHECK-FIXES: Str.find_first_of('a'); + Str.find_first_not_of("a"); + // CHECK-MESSAGES: [[@LINE-1]]:25: warning: find_first_not_of called with a + // CHECK-FIXES: Str.find_first_not_of('a'); + Str.find_last_of("a"); + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: find_last_of called with a string + // CHECK-FIXES: Str.find_last_of('a'); + Str.find_last_not_of("a"); + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: find_last_not_of called with a + // CHECK-FIXES: Str.find_last_not_of('a'); + + // std::wstring should work. + std::wstring WStr; + WStr.find(L"n"); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: find called with a string literal + // CHECK-FIXES: Str.find(L'n'); + // Even with unicode that fits in one wide char. + WStr.find(L"\x3A9"); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: find called with a string literal + // CHECK-FIXES: Str.find(L'\x3A9'); + + // Also with other types, but only if it was specified in the options. + llvm::StringRef sr; + sr.find("x"); + // CHECK-MESSAGES: [[@LINE-1]]:11: warning: find called with a string literal + // CHECK-FIXES: sr.find('x'); + NotStringRef nsr; + nsr.find("x"); +} + + +template +int FindTemplateDependant(T value) { + return value.find("A"); +} +template +int FindTemplateNotDependant(T pos) { + return std::string().find("A", pos); + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: find called with a string literal + // CHECK-FIXES: return std::string().find('A', pos); +} + +int FindStr() { + return FindTemplateDependant(std::string()) + FindTemplateNotDependant(1); +} + +#define STR_MACRO(str) str.find("A") +#define POS_MACRO(pos) std::string().find("A",pos) + +int Macros() { + return STR_MACRO(std::string()) + POS_MACRO(1); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: find called with a string literal + // CHECK-MESSAGES: [[@LINE-2]]:37: warning: find called with a string literal +} Index: docs/clang-tidy/checks/performance-faster-string-find.rst === --- /dev/null +++ docs/clang-tidy/checks/performance-faster-string-find.rst @@ -0,0 +1,18 @@ +.. title:: clang-tidy - performance-faster-string-find + +performance-faster-string-find +== + +Optimize calls to std::string::find() and friends when the needle passed is +a single character string literal. +The character literal overload is more efficient. + +Examples: + +.. code-block:: c
Re: [PATCH] D16152: [clang-tidy] Add check performance-faster-string-find
sbenza added inline comments. Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:29 @@ +28,3 @@ +Class = Class.trim(); + return std::vector(Classes.begin(), Classes.end()); +} aaron.ballman wrote: > I think hasName() will assert if given an empty string, so this should > probably also guard against a class list like ",basic_string". Also changed the separator to be ';' instead of ','. The latter could be part of a type name. Eg. Foo::Bar http://reviews.llvm.org/D16152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16171: Warning on redeclaring with a conflicting asm label
weimingz added a comment. Hi Nick, Below is a reduced code: t.c: static long double acoshl (long double __x) __asm__ ("" "acosh") ; // this is from /arm-linux-gnueabi/libc/usr/include/bits/mathcalls.h extern long double acoshl (long double) __asm__ ("" "__acoshl_finite") ; // this is from existing code GCC gives warning like: /tmp/t.c:2:1: warning: asm declaration ignored due to conflict with previous rename [-Wpragmas] extern long double acoshl (long double) __asm__ ("" "__acoshl_finite") ; http://reviews.llvm.org/D16171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15914: [OpenCL] Pipe builtin functions
Anastasia added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:2082 @@ +2081,3 @@ +if (BuiltinID == Builtin::BIget_pipe_num_packets) + Name = "get_pipe_num_packets"; +else prepend "__" here too! Comment at: lib/Sema/SemaChecking.cpp:291 @@ +290,3 @@ + bool isValid = true; + bool ReadOnly = getFunctionName(Call).find("read") != StringRef::npos; + if (ReadOnly) Ok, I think we can proceed with this review. You can let the spec be clarified on the side. Comment at: lib/Sema/SemaChecking.cpp:378 @@ +377,3 @@ + } + + return false; I am asking about setArg and not setType! http://reviews.llvm.org/D15914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16047: [OpenCL] Add Sema checks for OpenCL 2.0
Anastasia added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7670 @@ +7669,3 @@ + "%0 can only be used as a function parameter">; +def err_opencl_atomic_init_addressspace : Error< + "initialization of atomic variables is restricted to variables in global address space in opencl">; pxli168 wrote: > Anastasia wrote: > > Could you do something like: > > > > def err_atomic_init_constant : Error< > > "atomic variable can only be %select{assigned|initialised}0 to a > > compile time constant" > > " in the declaration statement in the program scope">; > > > Seems good. But that error message was from spec, this will change it. You can pick the wording you like. I personally find this one better! Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7674 @@ +7673,3 @@ + "invalid block prototype, variadic arguments are not allowed in opencl">; +def err_opencl_invalid_block_array : Error< + "array of block is invalid in OpenCL">; pxli168 wrote: > Anastasia wrote: > > Could we combine err_opencl_invalid_block_array and > > err_opencl_pointer_to_image saying something like: > > > > "Declaring a %select{pointer|array}0 of type %1 is not allowed in OpenCL" > OK, it will save few lines. And more importantly won't populate diagnostics. Comment at: lib/Sema/SemaDecl.cpp:5724 @@ +5723,3 @@ + R->isPipeType()) { +Diag(D.getIdentifierLoc(), + diag::err_opencl_type_can_only_be_used_as_function_parameter) pxli168 wrote: > Anastasia wrote: > > Some of them have to be. For example for C types that we use differently in > > CL. But for CL only types do we need to check additionally that it's CL? Do > > we not know it already? > Yes, but it seems all old code write in that way. I just follow the style. Ok, I think an improvement to the old code is always welcome! Comment at: lib/Sema/SemaExpr.cpp:6299 @@ -6286,3 +6298,3 @@ // Now check the two expressions. if (LHS.get()->getType()->isVectorType() || RHS.get()->getType()->isVectorType()) pxli168 wrote: > Anastasia wrote: > > I am not sure what the question is? > > > > I think using block in a condition should be disallowed. Could you add this > > to testing as well? > I don't see any thing talk about the block and condition. Spec only tells > about block and selection. Ok, in that case there is nothing left to do here I guess. Comment at: lib/Sema/SemaExpr.cpp:6316 @@ +6315,3 @@ + if (getLangOpts().OpenCL && getLangOpts().OpenCLVersion >= 200) { +// should output error for both LHS and RHS, use | instead || +if (checkBlockType(*this, LHS.get()) | checkBlockType(*this, RHS.get())) pxli168 wrote: > Anastasia wrote: > > Could you remove this comment? > OK. but the "|" may seems strange. I just want to explain it. Doesn't seem strange to me. Comment at: lib/Sema/SemaExpr.cpp:10115 @@ +10114,3 @@ +// Block. +if (S.getLangOpts().OpenCL && S.getLangOpts().OpenCLVersion >= 200 && +Result->isBlockPointerType()) { pxli168 wrote: > Anastasia wrote: > > The code above seems to do similar. Could we combine into one > > function/diagnostic? > Yes, they are. But they live in different function test for operand "&" and > "*" and maybe it is hard to merge them together. I have uploaded the full > diff as you asked and uou can expand the code to see they are in two function > check for the two unary operators. Yes, it's not a problem that they are in different functions. Could this code be encapsulated in a function, let's say checkBlockType(...) and be called from multiple places? Comment at: lib/Sema/SemaInit.cpp:6139 @@ +6138,3 @@ + // OpenCL v2.0 s6.13.11.1 - The ATOMIC_VAR_INIT macro expands to a token + // sequence suitable for initializing an atomic object of a type that is + // initialization-compatible with value. An atomic object with automatic pxli168 wrote: > Anastasia wrote: > > I don't think we need to copy the spec text here. I would like to see some > > explanation of the code actually. > > > > Could you write something like: "Non-program scope atomic variables can't > > be initialized." > But the example said there can be program scope atomic that only in global > address space. > > > This macro can only be used to initialize atomic objects that are declared > > in program scope in the global address space > > The quote from spec said so. Yes, so copying spec into code is not the goal. Let's try to make it shorter and still have the same meaning! Comment at: test/Parser/opencl-atomics-cl20.cl:71 @@ +70,3 @@ +void atomic_init_test() { +atomic_int guide = ATOMIC_VAR_INIT(42); // expected-error {{initialization of atomic variables is restricted to variables in global address space in opencl
Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs
ldionne updated this revision to Diff 44907. ldionne marked 4 inline comments as done. ldionne added a comment. Address Richard Smith's review comments: - Remove IndexType parameter, and make Index a std::size_t - Remove assertion about APSInt::GetExtValue() - Other style changes Still left to do: - Settle on a name for __nth_element - Should we produce a diagnostic when the index is out of bounds, or is this reserved for standard-mandated diagnostics (and we should use another mean to report the error)? http://reviews.llvm.org/D15421 Files: include/clang/AST/ASTContext.h include/clang/AST/DeclTemplate.h include/clang/Basic/Builtins.h include/clang/Basic/DiagnosticSemaKinds.td include/clang/Serialization/ASTBitCodes.h lib/AST/ASTContext.cpp lib/AST/DeclTemplate.cpp lib/Lex/PPMacroExpansion.cpp lib/Sema/SemaLookup.cpp lib/Sema/SemaTemplate.cpp lib/Serialization/ASTReader.cpp lib/Serialization/ASTWriter.cpp test/PCH/nth-element.cpp test/SemaCXX/nth_element.cpp Index: test/SemaCXX/nth_element.cpp === --- /dev/null +++ test/SemaCXX/nth_element.cpp @@ -0,0 +1,42 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s + +static_assert(__has_builtin(__nth_element), ""); + +using SizeT = decltype(sizeof(int)); + +template +using NthElement = __nth_element; + +template +struct X; + +static_assert(__is_same(NthElement<0, X<0>>, X<0>), ""); + +static_assert(__is_same(NthElement<0, X<0>, X<1>>, X<0>), ""); +static_assert(__is_same(NthElement<1, X<0>, X<1>>, X<1>), ""); + +static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>>, X<0>), ""); +static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>>, X<1>), ""); +static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>>, X<2>), ""); + +static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>, X<3>>, X<0>), ""); +static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>, X<3>>, X<1>), ""); +static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>, X<3>>, X<2>), ""); +static_assert(__is_same(NthElement<3, X<0>, X<1>, X<2>, X<3>>, X<3>), ""); + +static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>, X<3>, X<4>>, X<0>), ""); +static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>, X<3>, X<4>>, X<1>), ""); +static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>, X<3>, X<4>>, X<2>), ""); +static_assert(__is_same(NthElement<3, X<0>, X<1>, X<2>, X<3>, X<4>>, X<3>), ""); +static_assert(__is_same(NthElement<4, X<0>, X<1>, X<2>, X<3>, X<4>>, X<4>), ""); + +static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<0>), ""); +static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<1>), ""); +static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<2>), ""); +static_assert(__is_same(NthElement<3, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<3>), ""); +static_assert(__is_same(NthElement<4, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<4>), ""); +static_assert(__is_same(NthElement<5, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<5>), ""); + +template +using ErrorNthElement1 = __nth_element; // expected-error{{may not be accessed at an out of bounds index}} +using illformed1 = ErrorNthElement1<3, X<0>, X<1>>; // expected-note{{in instantiation}} Index: test/PCH/nth-element.cpp === --- /dev/null +++ test/PCH/nth-element.cpp @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -o %t.pch +// RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch + +template +struct X { }; + +using SizeT = decltype(sizeof(int)); + +template +using NthElement = __nth_element; + +void fn1() { + X<0> x0 = NthElement<0, X<0>, X<1>, X<2>>{}; + X<1> x1 = NthElement<1, X<0>, X<1>, X<2>>{}; + X<2> x2 = NthElement<2, X<0>, X<1>, X<2>>{}; +} Index: lib/Serialization/ASTWriter.cpp === --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -4152,6 +4152,7 @@ RegisterPredefDecl(Context.ExternCContext, PREDEF_DECL_EXTERN_C_CONTEXT_ID); RegisterPredefDecl(Context.MakeIntegerSeqDecl, PREDEF_DECL_MAKE_INTEGER_SEQ_ID); + RegisterPredefDecl(Context.NthElementDecl, PREDEF_DECL_NTH_ELEMENT_ID); // Build a record containing all of the tentative definitions in this file, in // TentativeDefinitions order. Generally, this record will be empty for Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -6444,6 +6444,9 @@ case PREDEF_DECL_MAKE_INTEGER_SEQ_ID: return Context.getMakeIntegerSeqDecl(); + + case PREDEF_DECL_NTH_ELEMENT_ID: +return Context.getNthElementDecl(); } llvm_unreachable("PredefinedDeclIDs unknown enum value"); } Index: lib/Sema/SemaTemplate.cpp === ---
Re: r256468 - On {mips, mipsel, mips64, mips64el}-freebsd, we need to pass any -G option to the assembler.
Sorry, but I fail to see where in AddMIPSTargetArgs the -G options are handled? What is the general idea about the "new style" cleanup? -Dimitry > On 14 Jan 2016, at 18:13, Daniel Sanders wrote: > > Did you get an answer to this question? I think it's just that the FreeBSD > class hasn't refactored to that style yet. > That switch statement is getting quite large so it would be a nice cleanup to > switch to that style. > >> -Original Message- >> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf >> Of Joerg Sonnenberger via cfe-commits >> Sent: 27 December 2015 11:59 >> To: cfe-commits@lists.llvm.org >> Subject: Re: r256468 - On {mips, mipsel, mips64, mips64el}-freebsd, we need >> to pass any -G option to the assembler. >> >> On Sun, Dec 27, 2015 at 10:36:44AM -, Dimitry Andric via cfe-commits >> wrote: >>> Author: dim >>> Date: Sun Dec 27 04:36:44 2015 >>> New Revision: 256468 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=256468&view=rev >>> Log: >>> On {mips,mipsel,mips64,mips64el}-freebsd, we need to pass any -G option >> to the assembler. >> >> Why is this reinventing the wheel and not using AddMIPSTargetArgs? >> >> Joerg >> ___ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits signature.asc Description: Message signed with OpenPGP using GPGMail ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15220: [OPENMP] dist_schedule clause for distribute directive
carlo.bertolli updated this revision to Diff 44912. carlo.bertolli added a comment. Remove duplicated error-messages and replace with error-messages2 and 3 Repository: rL LLVM http://reviews.llvm.org/D15220 Files: include/clang/AST/OpenMPClause.h include/clang/AST/RecursiveASTVisitor.h include/clang/Basic/OpenMPKinds.def include/clang/Basic/OpenMPKinds.h include/clang/Sema/Sema.h lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/Basic/OpenMPKinds.cpp lib/CodeGen/CGStmtOpenMP.cpp lib/Parse/ParseOpenMP.cpp lib/Sema/SemaOpenMP.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp test/OpenMP/distribute_dist_schedule_ast_print.cpp test/OpenMP/distribute_dist_schedule_messages.cpp tools/libclang/CIndex.cpp Index: tools/libclang/CIndex.cpp === --- tools/libclang/CIndex.cpp +++ tools/libclang/CIndex.cpp @@ -,6 +,11 @@ void OMPClauseEnqueue::VisitOMPMapClause(const OMPMapClause *C) { VisitOMPClauseList(C); } +void OMPClauseEnqueue::VisitOMPDistScheduleClause( +const OMPDistScheduleClause *C) { + Visitor->AddStmt(C->getChunkSize()); + Visitor->AddStmt(C->getHelperChunkSize()); +} } void EnqueueVisitor::EnqueueChildren(const OMPClause *S) { Index: test/OpenMP/distribute_dist_schedule_messages.cpp === --- /dev/null +++ test/OpenMP/distribute_dist_schedule_messages.cpp @@ -0,0 +1,63 @@ +// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify -fopenmp -ferror-limit 100 -o - %s + +void foo() { +} + +bool foobool(int argc) { + return argc; +} + +struct S1; // expected-note {{declared here}} expected-note {{declared here}} + +template +T tmain(T argc) { + T b = argc, c, d, e, f, g; + char ** argv; + static T a; +// CHECK: static T a; + #pragma omp distribute dist_schedule // expected-error {{expected '(' after 'dist_schedule'}} + for (int i = 0; i < 10; ++i) foo(); + #pragma omp distribute dist_schedule ( // expected-error {{expected 'static' in OpenMP clause 'dist_schedule'}} expected-error {{expected ')'}} expected-note {{to match this '('}} + for (int i = 0; i < 10; ++i) foo(); + #pragma omp distribute dist_schedule () // expected-error {{expected 'static' in OpenMP clause 'dist_schedule'}} + for (int i = 0; i < 10; ++i) foo(); + #pragma omp distribute dist_schedule (static // expected-error {{expected ')'}} expected-note {{to match this '('}} + for (int i = 0; i < 10; ++i) foo(); + #pragma omp distribute dist_schedule (static, // expected-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}} + for (int i = 0; i < 10; ++i) foo(); + #pragma omp distribute dist_schedule (argc)) // expected-error {{expected 'static' in OpenMP clause 'dist_schedule'}} expected-warning {{extra tokens at the end of '#pragma omp distribute' are ignored}} + for (int i = 0; i < 10; ++i) foo(); + #pragma omp distribute dist_schedule (static, argc > 0 ? argv[1] : argv[2]) // expected-error2 {{expression must have integral or unscoped enumeration type, not 'char *'}} + for (int i = 0; i < 10; ++i) foo(); + #pragma omp distribute dist_schedule (static), dist_schedule (static, 1) // expected-error {{directive '#pragma omp distribute' cannot contain more than one 'dist_schedule' clause}} + for (int i = 0; i < 10; ++i) foo(); + #pragma omp distribute dist_schedule (static, S1) // expected-error {{'S1' does not refer to a value}} + for (int i = 0; i < 10; ++i) foo(); + #pragma omp distribute dist_schedule (static, argv[1]=2) // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-error3 {{expression must have integral or unscoped enumeration type, not 'char *'}} + for (int i = 0; i < 10; ++i) foo(); + return T(); +} + +int main(int argc, char **argv) { + #pragma omp distribute dist_schedule // expected-error {{expected '(' after 'dist_schedule'}} + for (int i = 0; i < 10; ++i) foo(); + #pragma omp distribute dist_schedule ( // expected-error {{expected 'static' in OpenMP clause 'dist_schedule'}} expected-error {{expected ')'}} expected-note {{to match this '('}} + for (int i = 0; i < 10; ++i) foo(); + #pragma omp distribute dist_schedule () // expected-error {{expected 'static' in OpenMP clause 'dist_schedule'}} + for (int i = 0; i < 10; ++i) foo(); + #pragma omp distribute dist_schedule (static // expected-error {{expected ')'}} expected-note {{to match this '('}} + for (int i = 0; i < 10; ++i) foo(); + #pragma omp distribute dist_schedule (static, // expected-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}} + for (int i = 0; i < 10; ++i) foo(); + #pragma omp distribute dist_schedule (argc)) // expected-error {{expected 'static' in OpenMP clause 'dist_schedule'}} expected-warning {{extra tokens at the end of '#pragma omp distribute' are ignored}} + for (int
Re: [PATCH] D16175: Introduce -fsanitize-stats flag.
krasin1 added a subscriber: krasin1. krasin1 accepted this revision. krasin1 added a reviewer: krasin1. krasin1 added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Driver/SanitizerArgs.cpp:560 @@ +559,3 @@ + LinkerOptionFlag = "--linker-option=/include:"; + if (TC.getTriple().getArch() == llvm::Triple::x86) +LinkerOptionFlag += '_'; A comment about why does this underscore is needed for x86 is appreciated. http://reviews.llvm.org/D16175 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15866: Warn when using `defined` in a macro definition.
rsmith accepted this revision. This revision is now accepted and ready to land. Comment at: lib/Lex/PPExpressions.cpp:104-105 @@ +103,4 @@ + // #else branch. Emit a warning about this undefined behavior. + if (beginLoc.isMacroID()) +PP.Diag(beginLoc, diag::warn_defined_in_macro); + Move this down to the end of the function, after we've checked that we have a syntactically valid `defined` operator, to avoid duplicate diagnostics on a case like: #define FOO defined( #if FOO http://reviews.llvm.org/D15866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15305: [CUDA] Do not allow dynamic initialization of global device side variables.
rsmith added a comment. I think you missed this from my previous review: > This should be checked and diagnosed in Sema, not in CodeGen. Comment at: lib/CodeGen/CGDeclCXX.cpp:333-337 @@ +332,7 @@ + [](const CXXMethodDecl *Method) { return Method->isVirtual(); })) +return false; + + // .. and no virtual base classes. + if (RD->getNumVBases() != 0) +return false; + You can check these conditions with `RD->isDynamicClass()`. http://reviews.llvm.org/D15305 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
RE: r256468 - On {mips, mipsel, mips64, mips64el}-freebsd, we need to pass any -G option to the assembler.
-G isn't handled there at the moment but ClangAs::AddMIPSTargetArgs() and Clang::AddMIPSTargetArgs() would be the right place to add it in the Linux toolchain when we add it. The cleanup I'm referring to is that someone hoisted the bulk of the code out of the big switch-statement in Clang::ConstructJob() and ClangAs::ConstructJob() into AddTargetArgs() functions. The switch-statement in freebsd::Assembler::ConstructJob() isn't as big as either of those yet but it's already reached a couple screens long. I think freebsd::Assembler::ConstructJob() should take the same approach at some point in the near future. From: Dimitry Andric [dimi...@andric.com] Sent: 14 January 2016 19:20 To: Daniel Sanders Cc: Joerg Sonnenberger; cfe-commits@lists.llvm.org Subject: Re: r256468 - On {mips, mipsel, mips64, mips64el}-freebsd, we need to pass any -G option to the assembler. Sorry, but I fail to see where in AddMIPSTargetArgs the -G options are handled? What is the general idea about the "new style" cleanup? -Dimitry > On 14 Jan 2016, at 18:13, Daniel Sanders wrote: > > Did you get an answer to this question? I think it's just that the FreeBSD > class hasn't refactored to that style yet. > That switch statement is getting quite large so it would be a nice cleanup to > switch to that style. > >> -Original Message- >> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf >> Of Joerg Sonnenberger via cfe-commits >> Sent: 27 December 2015 11:59 >> To: cfe-commits@lists.llvm.org >> Subject: Re: r256468 - On {mips, mipsel, mips64, mips64el}-freebsd, we need >> to pass any -G option to the assembler. >> >> On Sun, Dec 27, 2015 at 10:36:44AM -, Dimitry Andric via cfe-commits >> wrote: >>> Author: dim >>> Date: Sun Dec 27 04:36:44 2015 >>> New Revision: 256468 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=256468&view=rev >>> Log: >>> On {mips,mipsel,mips64,mips64el}-freebsd, we need to pass any -G option >> to the assembler. >> >> Why is this reinventing the wheel and not using AddMIPSTargetArgs? >> >> Joerg >> ___ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jhen added a comment. rsmith, I think the patch is ready to be committed. Please take a look if you have a moment. Thanks for your help. http://reviews.llvm.org/D15858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r257802 - Update for LLVM function name change.
Author: ruiu Date: Thu Jan 14 15:00:27 2016 New Revision: 257802 URL: http://llvm.org/viewvc/llvm-project?rev=257802&view=rev Log: Update for LLVM function name change. Modified: cfe/trunk/include/clang/AST/CharUnits.h cfe/trunk/include/clang/AST/StmtOpenMP.h cfe/trunk/include/clang/AST/TypeLoc.h cfe/trunk/lib/AST/ASTContext.cpp cfe/trunk/lib/AST/ExprConstant.cpp cfe/trunk/lib/AST/Mangle.cpp cfe/trunk/lib/AST/MicrosoftCXXABI.cpp cfe/trunk/lib/AST/RecordLayoutBuilder.cpp cfe/trunk/lib/AST/Stmt.cpp cfe/trunk/lib/AST/StmtOpenMP.cpp cfe/trunk/lib/AST/TypeLoc.cpp cfe/trunk/lib/CodeGen/CGAtomic.cpp cfe/trunk/lib/CodeGen/CGBlocks.cpp cfe/trunk/lib/CodeGen/CGCleanup.cpp cfe/trunk/lib/CodeGen/CGCleanup.h cfe/trunk/lib/CodeGen/CGDebugInfo.cpp cfe/trunk/lib/CodeGen/CGExprConstant.cpp cfe/trunk/lib/CodeGen/CGObjCMac.cpp cfe/trunk/lib/CodeGen/CGObjCRuntime.cpp cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp cfe/trunk/lib/CodeGen/TargetInfo.cpp cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp Modified: cfe/trunk/include/clang/AST/CharUnits.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/CharUnits.h?rev=257802&r1=257801&r2=257802&view=diff == --- cfe/trunk/include/clang/AST/CharUnits.h (original) +++ cfe/trunk/include/clang/AST/CharUnits.h Thu Jan 14 15:00:27 2016 @@ -133,7 +133,7 @@ namespace clang { /// Test whether this is a multiple of the other value. /// /// Among other things, this promises that - /// self.RoundUpToAlignment(N) will just return self. + /// self.alignTo(N) will just return self. bool isMultipleOf(CharUnits N) const { return (*this % N) == 0; } @@ -170,12 +170,11 @@ namespace clang { /// getQuantity - Get the raw integer representation of this quantity. QuantityType getQuantity() const { return Quantity; } - /// RoundUpToAlignment - Returns the next integer (mod 2**64) that is + /// alignTo - Returns the next integer (mod 2**64) that is /// greater than or equal to this quantity and is a multiple of \p Align. /// Align must be non-zero. - CharUnits RoundUpToAlignment(const CharUnits &Align) const { -return CharUnits(llvm::RoundUpToAlignment(Quantity, - Align.Quantity)); + CharUnits alignTo(const CharUnits &Align) const { +return CharUnits(llvm::alignTo(Quantity, Align.Quantity)); } /// Given that this is a non-zero alignment value, what is the Modified: cfe/trunk/include/clang/AST/StmtOpenMP.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/StmtOpenMP.h?rev=257802&r1=257801&r2=257802&view=diff == --- cfe/trunk/include/clang/AST/StmtOpenMP.h (original) +++ cfe/trunk/include/clang/AST/StmtOpenMP.h Thu Jan 14 15:00:27 2016 @@ -70,8 +70,7 @@ protected: : Stmt(SC), Kind(K), StartLoc(std::move(StartLoc)), EndLoc(std::move(EndLoc)), NumClauses(NumClauses), NumChildren(NumChildren), -ClausesOffset(llvm::RoundUpToAlignment(sizeof(T), - llvm::alignOf())) {} +ClausesOffset(llvm::alignTo(sizeof(T), llvm::alignOf())) {} /// \brief Sets the list of variables for this clause. /// Modified: cfe/trunk/include/clang/AST/TypeLoc.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/TypeLoc.h?rev=257802&r1=257801&r2=257802&view=diff == --- cfe/trunk/include/clang/AST/TypeLoc.h (original) +++ cfe/trunk/include/clang/AST/TypeLoc.h Thu Jan 14 15:00:27 2016 @@ -254,7 +254,7 @@ public: unsigned align = TypeLoc::getLocalAlignmentForType(QualType(getTypePtr(), 0)); uintptr_t dataInt = reinterpret_cast(Data); -dataInt = llvm::RoundUpToAlignment(dataInt, align); +dataInt = llvm::alignTo(dataInt, align); return UnqualTypeLoc(getTypePtr(), reinterpret_cast(dataInt)); } @@ -353,7 +353,7 @@ public: unsigned getLocalDataSize() const { unsigned size = sizeof(LocalData); unsigned extraAlign = asDerived()->getExtraLocalDataAlignment(); -size = llvm::RoundUpToAlignment(size, extraAlign); +size = llvm::alignTo(size, extraAlign); size += asDerived()->getExtraLocalDataSize(); return size; } @@ -399,14 +399,14 @@ protected: void *getExtraLocalData() const { unsigned size = sizeof(LocalData); unsigned extraAlign = asDerived()->getExtraLocalDataAlignment(); -size = llvm::RoundUpToAlignment(size, extraAlign); +size = llvm::alignTo(size, extraAlign); return reinterpret_cast(Base::Data) + size; } void *getNonLocalData() const { uintptr_t data = reinterpret_cast(Base::Data);
Re: [PATCH] D16082: [CUDA] Invoke ptxas and fatbinary during compilation.
echristo accepted this revision. echristo added a comment. This is terrible, but the only other option is fixing bind arch and inverting the graph which is a major rewrite to the driver. So, LGTM. -eric http://reviews.llvm.org/D16082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r257765 - [Hexagon] Change all builtins returning "bool" to return "int" instead
This could be tested - though I'm not sure if there's precedent for testing the types of builtins for other targets, for example. Might be worth checking? On Thu, Jan 14, 2016 at 6:26 AM, Krzysztof Parzyszek via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: kparzysz > Date: Thu Jan 14 08:26:36 2016 > New Revision: 257765 > > URL: http://llvm.org/viewvc/llvm-project?rev=257765&view=rev > Log: > [Hexagon] Change all builtins returning "bool" to return "int" instead > > Modified: > cfe/trunk/include/clang/Basic/BuiltinsHexagon.def > > Modified: cfe/trunk/include/clang/Basic/BuiltinsHexagon.def > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsHexagon.def?rev=257765&r1=257764&r2=257765&view=diff > > == > --- cfe/trunk/include/clang/Basic/BuiltinsHexagon.def (original) > +++ cfe/trunk/include/clang/Basic/BuiltinsHexagon.def Thu Jan 14 08:26:36 > 2016 > @@ -23,52 +23,52 @@ BUILTIN(__builtin_circ_ldd, "LLi*LLi*LLi > // The builtins above are not autogenerated from iset.py. > // Make sure you do not overwrite these. > > -BUILTIN(__builtin_HEXAGON_C2_cmpeq,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_cmpgt,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_cmpgtu,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_cmpeqp,"bLLiLLi","") > -BUILTIN(__builtin_HEXAGON_C2_cmpgtp,"bLLiLLi","") > -BUILTIN(__builtin_HEXAGON_C2_cmpgtup,"bLLiLLi","") > +BUILTIN(__builtin_HEXAGON_C2_cmpeq,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_cmpgt,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_cmpgtu,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_cmpeqp,"iLLiLLi","") > +BUILTIN(__builtin_HEXAGON_C2_cmpgtp,"iLLiLLi","") > +BUILTIN(__builtin_HEXAGON_C2_cmpgtup,"iLLiLLi","") > BUILTIN(__builtin_HEXAGON_A4_rcmpeqi,"iii","") > BUILTIN(__builtin_HEXAGON_A4_rcmpneqi,"iii","") > BUILTIN(__builtin_HEXAGON_A4_rcmpeq,"iii","") > BUILTIN(__builtin_HEXAGON_A4_rcmpneq,"iii","") > -BUILTIN(__builtin_HEXAGON_C2_bitsset,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_bitsclr,"bii","") > -BUILTIN(__builtin_HEXAGON_C4_nbitsset,"bii","") > -BUILTIN(__builtin_HEXAGON_C4_nbitsclr,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_cmpeqi,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_cmpgti,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_cmpgtui,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_cmpgei,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_cmpgeui,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_cmplt,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_cmpltu,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_bitsclri,"bii","") > -BUILTIN(__builtin_HEXAGON_C4_nbitsclri,"bii","") > -BUILTIN(__builtin_HEXAGON_C4_cmpneqi,"bii","") > -BUILTIN(__builtin_HEXAGON_C4_cmpltei,"bii","") > -BUILTIN(__builtin_HEXAGON_C4_cmplteui,"bii","") > -BUILTIN(__builtin_HEXAGON_C4_cmpneq,"bii","") > -BUILTIN(__builtin_HEXAGON_C4_cmplte,"bii","") > -BUILTIN(__builtin_HEXAGON_C4_cmplteu,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_and,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_or,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_xor,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_andn,"bii","") > -BUILTIN(__builtin_HEXAGON_C2_not,"bi","") > -BUILTIN(__builtin_HEXAGON_C2_orn,"bii","") > -BUILTIN(__builtin_HEXAGON_C4_and_and,"biii","") > -BUILTIN(__builtin_HEXAGON_C4_and_or,"biii","") > -BUILTIN(__builtin_HEXAGON_C4_or_and,"biii","") > -BUILTIN(__builtin_HEXAGON_C4_or_or,"biii","") > -BUILTIN(__builtin_HEXAGON_C4_and_andn,"biii","") > -BUILTIN(__builtin_HEXAGON_C4_and_orn,"biii","") > -BUILTIN(__builtin_HEXAGON_C4_or_andn,"biii","") > -BUILTIN(__builtin_HEXAGON_C4_or_orn,"biii","") > -BUILTIN(__builtin_HEXAGON_C2_pxfer_map,"bi","") > -BUILTIN(__builtin_HEXAGON_C2_any8,"bi","") > -BUILTIN(__builtin_HEXAGON_C2_all8,"bi","") > +BUILTIN(__builtin_HEXAGON_C2_bitsset,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_bitsclr,"iii","") > +BUILTIN(__builtin_HEXAGON_C4_nbitsset,"iii","") > +BUILTIN(__builtin_HEXAGON_C4_nbitsclr,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_cmpeqi,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_cmpgti,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_cmpgtui,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_cmpgei,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_cmpgeui,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_cmplt,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_cmpltu,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_bitsclri,"iii","") > +BUILTIN(__builtin_HEXAGON_C4_nbitsclri,"iii","") > +BUILTIN(__builtin_HEXAGON_C4_cmpneqi,"iii","") > +BUILTIN(__builtin_HEXAGON_C4_cmpltei,"iii","") > +BUILTIN(__builtin_HEXAGON_C4_cmplteui,"iii","") > +BUILTIN(__builtin_HEXAGON_C4_cmpneq,"iii","") > +BUILTIN(__builtin_HEXAGON_C4_cmplte,"iii","") > +BUILTIN(__builtin_HEXAGON_C4_cmplteu,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_and,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_or,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_xor,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_andn,"iii","") > +BUILTIN(__builtin_HEXAGON_C2_not,"ii","") > +BUILTIN(__builtin_HEXAGON_C2_orn,"iii","") > +BUILTIN(__builtin_HEXAGON_C4_and_and,
r257807 - [CUDA] Add tests for compiling CUDA files with -E.
Author: jlebar Date: Thu Jan 14 15:41:18 2016 New Revision: 257807 URL: http://llvm.org/viewvc/llvm-project?rev=257807&view=rev Log: [CUDA] Add tests for compiling CUDA files with -E. Reviewers: tra Subscribers: cfe-commits, jhen Differential Revision: http://reviews.llvm.org/D16080 Added: cfe/trunk/test/Preprocessor/cuda-preprocess.cu Added: cfe/trunk/test/Preprocessor/cuda-preprocess.cu URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/cuda-preprocess.cu?rev=257807&view=auto == --- cfe/trunk/test/Preprocessor/cuda-preprocess.cu (added) +++ cfe/trunk/test/Preprocessor/cuda-preprocess.cu Thu Jan 14 15:41:18 2016 @@ -0,0 +1,32 @@ +// Tests CUDA compilation with -E. + +// REQUIRES: clang-driver +// REQUIRES: x86-registered-target +// REQUIRES: nvptx-registered-target + +#ifndef __CUDA_ARCH__ +#define PREPROCESSED_AWAY +clang_unittest_no_arch PREPROCESSED_AWAY +#else +clang_unittest_cuda_arch __CUDA_ARCH__ +#endif + +// CHECK-NOT: PREPROCESSED_AWAY + +// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 %s 2>&1 \ +// RUN: | FileCheck -check-prefix NOARCH %s +// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 --cuda-host-only %s 2>&1 \ +// RUN: | FileCheck -check-prefix NOARCH %s +// NOARCH: clang_unittest_no_arch + +// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 --cuda-device-only %s 2>&1 \ +// RUN: | FileCheck -check-prefix SM20 %s +// SM20: clang_unittest_cuda_arch 200 + +// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_30 --cuda-device-only %s 2>&1 \ +// RUN: | FileCheck -check-prefix SM30 %s +// SM30: clang_unittest_cuda_arch 300 + +// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 --cuda-gpu-arch=sm_30 \ +// RUN: --cuda-device-only %s 2>&1 \ +// RUN: | FileCheck -check-prefix SM20 -check-prefix SM30 %s ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r257809 - [CUDA] Invoke ptxas and fatbinary during compilation.
Author: jlebar Date: Thu Jan 14 15:41:27 2016 New Revision: 257809 URL: http://llvm.org/viewvc/llvm-project?rev=257809&view=rev Log: [CUDA] Invoke ptxas and fatbinary during compilation. Summary: Previously we compiled CUDA device code to PTX assembly and embedded that asm as text in our host binary. Now we compile to PTX assembly and then invoke ptxas to assemble the PTX into a cubin file. We gather the ptx and cubin files for each of our --cuda-gpu-archs and combine them using fatbinary, and then embed that into the host binary. Adds two new command-line flags, -Xcuda_ptxas and -Xcuda_fatbinary, which pass args down to the external tools. Reviewers: tra, echristo Subscribers: cfe-commits, jhen Differential Revision: http://reviews.llvm.org/D16082 Added: cfe/trunk/test/Driver/Inputs/CUDA/usr/local/cuda/bin/ cfe/trunk/test/Driver/Inputs/CUDA/usr/local/cuda/bin/.keep cfe/trunk/test/Driver/cuda-arch-translation.cu cfe/trunk/test/Driver/cuda-external-tools.cu Modified: cfe/trunk/include/clang/Driver/Action.h cfe/trunk/include/clang/Driver/Options.td cfe/trunk/include/clang/Driver/ToolChain.h cfe/trunk/include/clang/Driver/Types.def cfe/trunk/lib/CodeGen/CGCUDANV.cpp cfe/trunk/lib/Driver/Action.cpp cfe/trunk/lib/Driver/Driver.cpp cfe/trunk/lib/Driver/ToolChains.cpp cfe/trunk/lib/Driver/ToolChains.h cfe/trunk/lib/Driver/Tools.cpp cfe/trunk/lib/Driver/Tools.h cfe/trunk/lib/Driver/Types.cpp cfe/trunk/test/Driver/cuda-options.cu Modified: cfe/trunk/include/clang/Driver/Action.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Action.h?rev=257809&r1=257808&r2=257809&view=diff == --- cfe/trunk/include/clang/Driver/Action.h (original) +++ cfe/trunk/include/clang/Driver/Action.h Thu Jan 14 15:41:27 2016 @@ -136,7 +136,8 @@ public: class CudaDeviceAction : public Action { virtual void anchor(); - /// GPU architecture to bind. Always of the form /sm_\d+/. + /// GPU architecture to bind. Always of the form /sm_\d+/ or null (when the + /// action applies to multiple architectures). const char *GpuArchName; /// True when action results are not consumed by the host action (e.g when /// -fsyntax-only or --cuda-device-only options are used). @@ -147,7 +148,8 @@ public: const char *getGpuArchName() const { return GpuArchName; } - /// Gets the compute_XX that corresponds to getGpuArchName(). + /// Gets the compute_XX that corresponds to getGpuArchName(). Returns null + /// when getGpuArchName() is null. const char *getComputeArchName() const; bool isAtTopLevel() const { return AtTopLevel; } Modified: cfe/trunk/include/clang/Driver/Options.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=257809&r1=257808&r2=257809&view=diff == --- cfe/trunk/include/clang/Driver/Options.td (original) +++ cfe/trunk/include/clang/Driver/Options.td Thu Jan 14 15:41:27 2016 @@ -336,6 +336,10 @@ def Xassembler : Separate<["-"], "Xassem def Xclang : Separate<["-"], "Xclang">, HelpText<"Pass to the clang compiler">, MetaVarName<"">, Flags<[DriverOption, CoreOption]>; +def Xcuda_fatbinary : Separate<["-"], "Xcuda-fatbinary">, + HelpText<"Pass to fatbinary invocation">, MetaVarName<"">; +def Xcuda_ptxas : Separate<["-"], "Xcuda-ptxas">, + HelpText<"Pass to the ptxas assembler">, MetaVarName<"">; def z : Separate<["-"], "z">, Flags<[LinkerInput, RenderAsInput]>, HelpText<"Pass -z to the linker">, MetaVarName<"">; def Xlinker : Separate<["-"], "Xlinker">, Flags<[LinkerInput, RenderAsInput]>, Modified: cfe/trunk/include/clang/Driver/ToolChain.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/ToolChain.h?rev=257809&r1=257808&r2=257809&view=diff == --- cfe/trunk/include/clang/Driver/ToolChain.h (original) +++ cfe/trunk/include/clang/Driver/ToolChain.h Thu Jan 14 15:41:27 2016 @@ -228,7 +228,7 @@ public: virtual bool IsIntegratedAssemblerDefault() const { return false; } /// \brief Check if the toolchain should use the integrated assembler. - bool useIntegratedAs() const; + virtual bool useIntegratedAs() const; /// IsMathErrnoDefault - Does this tool chain use -fmath-errno by default. virtual bool IsMathErrnoDefault() const { return true; } Modified: cfe/trunk/include/clang/Driver/Types.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Types.def?rev=257809&r1=257808&r2=257809&view=diff == --- cfe/trunk/include/clang/Driver/Types.def (original) +++ cfe/trunk/include/clang/Driver/Types.def Thu Jan 14 15:41:27 2016 @@ -93,4 +93,5 @@ TYPE("treelang", Treelan TYPE("image",
Re: [PATCH] D16080: [CUDA] Add tests for compiling CUDA files with -E.
This revision was automatically updated to reflect the committed changes. Closed by commit rL257807: [CUDA] Add tests for compiling CUDA files with -E. (authored by jlebar). Changed prior to commit: http://reviews.llvm.org/D16080?vs=44823&id=44920#toc Repository: rL LLVM http://reviews.llvm.org/D16080 Files: cfe/trunk/test/Preprocessor/cuda-preprocess.cu Index: cfe/trunk/test/Preprocessor/cuda-preprocess.cu === --- cfe/trunk/test/Preprocessor/cuda-preprocess.cu +++ cfe/trunk/test/Preprocessor/cuda-preprocess.cu @@ -0,0 +1,32 @@ +// Tests CUDA compilation with -E. + +// REQUIRES: clang-driver +// REQUIRES: x86-registered-target +// REQUIRES: nvptx-registered-target + +#ifndef __CUDA_ARCH__ +#define PREPROCESSED_AWAY +clang_unittest_no_arch PREPROCESSED_AWAY +#else +clang_unittest_cuda_arch __CUDA_ARCH__ +#endif + +// CHECK-NOT: PREPROCESSED_AWAY + +// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 %s 2>&1 \ +// RUN: | FileCheck -check-prefix NOARCH %s +// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 --cuda-host-only %s 2>&1 \ +// RUN: | FileCheck -check-prefix NOARCH %s +// NOARCH: clang_unittest_no_arch + +// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 --cuda-device-only %s 2>&1 \ +// RUN: | FileCheck -check-prefix SM20 %s +// SM20: clang_unittest_cuda_arch 200 + +// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_30 --cuda-device-only %s 2>&1 \ +// RUN: | FileCheck -check-prefix SM30 %s +// SM30: clang_unittest_cuda_arch 300 + +// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 --cuda-gpu-arch=sm_30 \ +// RUN: --cuda-device-only %s 2>&1 \ +// RUN: | FileCheck -check-prefix SM20 -check-prefix SM30 %s Index: cfe/trunk/test/Preprocessor/cuda-preprocess.cu === --- cfe/trunk/test/Preprocessor/cuda-preprocess.cu +++ cfe/trunk/test/Preprocessor/cuda-preprocess.cu @@ -0,0 +1,32 @@ +// Tests CUDA compilation with -E. + +// REQUIRES: clang-driver +// REQUIRES: x86-registered-target +// REQUIRES: nvptx-registered-target + +#ifndef __CUDA_ARCH__ +#define PREPROCESSED_AWAY +clang_unittest_no_arch PREPROCESSED_AWAY +#else +clang_unittest_cuda_arch __CUDA_ARCH__ +#endif + +// CHECK-NOT: PREPROCESSED_AWAY + +// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 %s 2>&1 \ +// RUN: | FileCheck -check-prefix NOARCH %s +// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 --cuda-host-only %s 2>&1 \ +// RUN: | FileCheck -check-prefix NOARCH %s +// NOARCH: clang_unittest_no_arch + +// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 --cuda-device-only %s 2>&1 \ +// RUN: | FileCheck -check-prefix SM20 %s +// SM20: clang_unittest_cuda_arch 200 + +// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_30 --cuda-device-only %s 2>&1 \ +// RUN: | FileCheck -check-prefix SM30 %s +// SM30: clang_unittest_cuda_arch 300 + +// RUN: %clang -E -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 --cuda-gpu-arch=sm_30 \ +// RUN: --cuda-device-only %s 2>&1 \ +// RUN: | FileCheck -check-prefix SM20 -check-prefix SM30 %s ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r257810 - [CUDA] Add test for compiling CUDA code with -S.
Author: jlebar Date: Thu Jan 14 15:41:31 2016 New Revision: 257810 URL: http://llvm.org/viewvc/llvm-project?rev=257810&view=rev Log: [CUDA] Add test for compiling CUDA code with -S. Reviewers: tra Subscribers: cfe-commits, jhen Differential Revision: http://reviews.llvm.org/D16081 Added: cfe/trunk/test/Driver/cuda-output-asm.cu Added: cfe/trunk/test/Driver/cuda-output-asm.cu URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cuda-output-asm.cu?rev=257810&view=auto == --- cfe/trunk/test/Driver/cuda-output-asm.cu (added) +++ cfe/trunk/test/Driver/cuda-output-asm.cu Thu Jan 14 15:41:31 2016 @@ -0,0 +1,29 @@ +// Tests CUDA compilation with -S. + +// REQUIRES: clang-driver +// REQUIRES: x86-registered-target +// REQUIRES: nvptx-registered-target + +// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 %s 2>&1 \ +// RUN: | FileCheck -check-prefix HOST -check-prefix SM20 %s +// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-host-only -o foo.s %s 2>&1 \ +// RUN: | FileCheck -check-prefix HOST %s +// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 \ +// RUN: --cuda-device-only -o foo.s %s 2>&1 \ +// RUN: | FileCheck -check-prefix SM20 %s +// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 \ +// RUN: --cuda-gpu-arch=sm_30 --cuda-device-only %s 2>&1 \ +// RUN: | FileCheck -check-prefix SM20 -check-prefix SM30 %s + +// HOST-DAG: "-cc1" "-triple" "x86_64--linux-gnu" +// SM20-DAG: "-cc1" "-triple" "nvptx64-nvidia-cuda" +// SM20-same: "-target-cpu" "sm_20" +// SM30-DAG: "-cc1" "-triple" "nvptx64-nvidia-cuda" +// SM30-same: "-target-cpu" "sm_30" + +// RUN: %clang -### -S -target x86_64-linux-gnu -o foo.s %s 2>&1 \ +// RUN: | FileCheck -check-prefix MULTIPLE-OUTPUT-FILES %s +// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-device-only \ +// RUN: --cuda-gpu-arch=sm_20 --cuda-gpu-arch=sm_30 -o foo.s %s 2>&1 \ +// RUN: | FileCheck -check-prefix MULTIPLE-OUTPUT-FILES %s +// MULTIPLE-OUTPUT-FILES: error: cannot specify -o when generating multiple output files ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r257808 - Don't build jobs for the same Action + ToolChain twice.
Author: jlebar Date: Thu Jan 14 15:41:21 2016 New Revision: 257808 URL: http://llvm.org/viewvc/llvm-project?rev=257808&view=rev Log: Don't build jobs for the same Action + ToolChain twice. Summary: Right now if the Action graph is a DAG and we encounter an action twice, we will run it twice. This patch is difficult to test as-is, but I have testcases for this as used within CUDA compilation. Reviewers: Subscribers: Modified: cfe/trunk/include/clang/Driver/Driver.h cfe/trunk/lib/Driver/Driver.cpp Modified: cfe/trunk/include/clang/Driver/Driver.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=257808&r1=257807&r2=257808&view=diff == --- cfe/trunk/include/clang/Driver/Driver.h (original) +++ cfe/trunk/include/clang/Driver/Driver.h Thu Jan 14 15:41:21 2016 @@ -21,6 +21,7 @@ #include "llvm/Support/Path.h" // FIXME: Kill when CompilationInfo lands. #include +#include #include #include #include @@ -381,10 +382,13 @@ public: /// BuildJobsForAction - Construct the jobs to perform for the /// action \p A and return an InputInfo for the result of running \p A. + /// Will only construct jobs for a given (Action, ToolChain) pair once. InputInfo BuildJobsForAction(Compilation &C, const Action *A, const ToolChain *TC, const char *BoundArch, bool AtTopLevel, bool MultipleArchs, - const char *LinkingOutput) const; + const char *LinkingOutput, + std::map, +InputInfo> &CachedResults) const; /// Returns the default name for linked images (e.g., "a.out"). const char *getDefaultImageName() const; @@ -441,6 +445,16 @@ private: /// the driver mode. std::pair getIncludeExcludeOptionFlagMasks() const; + /// Helper used in BuildJobsForAction. Doesn't use the cache when building + /// jobs specifically for the given action, but will use the cache when + /// building jobs for the Action's inputs. + InputInfo BuildJobsForActionNoCache( + Compilation &C, const Action *A, const ToolChain *TC, + const char *BoundArch, bool AtTopLevel, bool MultipleArchs, + const char *LinkingOutput, + std::map, InputInfo> + &CachedResults) const; + public: /// GetReleaseVersion - Parse (([0-9]+)(.([0-9]+)(.([0-9]+)?))?)? and /// return the grouped values as integers. Numbers which are not Modified: cfe/trunk/lib/Driver/Driver.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=257808&r1=257807&r2=257808&view=diff == --- cfe/trunk/lib/Driver/Driver.cpp (original) +++ cfe/trunk/lib/Driver/Driver.cpp Thu Jan 14 15:41:21 2016 @@ -1632,6 +1632,8 @@ void Driver::BuildJobs(Compilation &C) c if (A->getOption().matches(options::OPT_arch)) ArchNames.insert(A->getValue()); + // Set of (Action, canonical ToolChain triple) pairs we've built jobs for. + std::map, InputInfo> CachedResults; for (Action *A : C.getActions()) { // If we are linking an image for multiple archs then the linker wants // -arch_multiple and -final_output . Unfortunately, this @@ -1651,7 +1653,7 @@ void Driver::BuildJobs(Compilation &C) c /*BoundArch*/ nullptr, /*AtTopLevel*/ true, /*MultipleArchs*/ ArchNames.size() > 1, - /*LinkingOutput*/ LinkingOutput); + /*LinkingOutput*/ LinkingOutput, CachedResults); } // If the user passed -Qunused-arguments or there were errors, don't warn @@ -1779,19 +1781,38 @@ static const Tool *selectToolForJob(Comp return ToolForJob; } -InputInfo Driver::BuildJobsForAction(Compilation &C, const Action *A, - const ToolChain *TC, const char *BoundArch, - bool AtTopLevel, bool MultipleArchs, - const char *LinkingOutput) const { +InputInfo Driver::BuildJobsForAction( +Compilation &C, const Action *A, const ToolChain *TC, const char *BoundArch, +bool AtTopLevel, bool MultipleArchs, const char *LinkingOutput, +std::map, InputInfo> &CachedResults) +const { + std::pair ActionTC = { + A, TC->getTriple().normalize()}; + auto CachedResult = CachedResults.find(ActionTC); + if (CachedResult != CachedResults.end()) { +return CachedResult->second; + } + InputInfo Result = + BuildJobsForActionNoCache(C, A, TC, BoundArch, AtTopLevel, MultipleArchs, +LinkingOutput, CachedResults); + CachedResults[ActionTC] = Result; + return Result; +} + +InputInfo Driver::BuildJobsForActionNoCache( +Compilation &C, const Action *A, const ToolChain *TC, const c
Re: [PATCH] D16081: [CUDA] Add test for compiling CUDA code with -S.
This revision was automatically updated to reflect the committed changes. Closed by commit rL257810: [CUDA] Add test for compiling CUDA code with -S. (authored by jlebar). Changed prior to commit: http://reviews.llvm.org/D16081?vs=44542&id=44922#toc Repository: rL LLVM http://reviews.llvm.org/D16081 Files: cfe/trunk/test/Driver/cuda-output-asm.cu Index: cfe/trunk/test/Driver/cuda-output-asm.cu === --- cfe/trunk/test/Driver/cuda-output-asm.cu +++ cfe/trunk/test/Driver/cuda-output-asm.cu @@ -0,0 +1,29 @@ +// Tests CUDA compilation with -S. + +// REQUIRES: clang-driver +// REQUIRES: x86-registered-target +// REQUIRES: nvptx-registered-target + +// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 %s 2>&1 \ +// RUN: | FileCheck -check-prefix HOST -check-prefix SM20 %s +// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-host-only -o foo.s %s 2>&1 \ +// RUN: | FileCheck -check-prefix HOST %s +// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 \ +// RUN: --cuda-device-only -o foo.s %s 2>&1 \ +// RUN: | FileCheck -check-prefix SM20 %s +// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 \ +// RUN: --cuda-gpu-arch=sm_30 --cuda-device-only %s 2>&1 \ +// RUN: | FileCheck -check-prefix SM20 -check-prefix SM30 %s + +// HOST-DAG: "-cc1" "-triple" "x86_64--linux-gnu" +// SM20-DAG: "-cc1" "-triple" "nvptx64-nvidia-cuda" +// SM20-same: "-target-cpu" "sm_20" +// SM30-DAG: "-cc1" "-triple" "nvptx64-nvidia-cuda" +// SM30-same: "-target-cpu" "sm_30" + +// RUN: %clang -### -S -target x86_64-linux-gnu -o foo.s %s 2>&1 \ +// RUN: | FileCheck -check-prefix MULTIPLE-OUTPUT-FILES %s +// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-device-only \ +// RUN: --cuda-gpu-arch=sm_20 --cuda-gpu-arch=sm_30 -o foo.s %s 2>&1 \ +// RUN: | FileCheck -check-prefix MULTIPLE-OUTPUT-FILES %s +// MULTIPLE-OUTPUT-FILES: error: cannot specify -o when generating multiple output files Index: cfe/trunk/test/Driver/cuda-output-asm.cu === --- cfe/trunk/test/Driver/cuda-output-asm.cu +++ cfe/trunk/test/Driver/cuda-output-asm.cu @@ -0,0 +1,29 @@ +// Tests CUDA compilation with -S. + +// REQUIRES: clang-driver +// REQUIRES: x86-registered-target +// REQUIRES: nvptx-registered-target + +// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 %s 2>&1 \ +// RUN: | FileCheck -check-prefix HOST -check-prefix SM20 %s +// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-host-only -o foo.s %s 2>&1 \ +// RUN: | FileCheck -check-prefix HOST %s +// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 \ +// RUN: --cuda-device-only -o foo.s %s 2>&1 \ +// RUN: | FileCheck -check-prefix SM20 %s +// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-gpu-arch=sm_20 \ +// RUN: --cuda-gpu-arch=sm_30 --cuda-device-only %s 2>&1 \ +// RUN: | FileCheck -check-prefix SM20 -check-prefix SM30 %s + +// HOST-DAG: "-cc1" "-triple" "x86_64--linux-gnu" +// SM20-DAG: "-cc1" "-triple" "nvptx64-nvidia-cuda" +// SM20-same: "-target-cpu" "sm_20" +// SM30-DAG: "-cc1" "-triple" "nvptx64-nvidia-cuda" +// SM30-same: "-target-cpu" "sm_30" + +// RUN: %clang -### -S -target x86_64-linux-gnu -o foo.s %s 2>&1 \ +// RUN: | FileCheck -check-prefix MULTIPLE-OUTPUT-FILES %s +// RUN: %clang -### -S -target x86_64-linux-gnu --cuda-device-only \ +// RUN: --cuda-gpu-arch=sm_20 --cuda-gpu-arch=sm_30 -o foo.s %s 2>&1 \ +// RUN: | FileCheck -check-prefix MULTIPLE-OUTPUT-FILES %s +// MULTIPLE-OUTPUT-FILES: error: cannot specify -o when generating multiple output files ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16082: [CUDA] Invoke ptxas and fatbinary during compilation.
This revision was automatically updated to reflect the committed changes. Closed by commit rL257809: [CUDA] Invoke ptxas and fatbinary during compilation. (authored by jlebar). Changed prior to commit: http://reviews.llvm.org/D16082?vs=44586&id=44921#toc Repository: rL LLVM http://reviews.llvm.org/D16082 Files: cfe/trunk/include/clang/Driver/Action.h cfe/trunk/include/clang/Driver/Options.td cfe/trunk/include/clang/Driver/ToolChain.h cfe/trunk/include/clang/Driver/Types.def cfe/trunk/lib/CodeGen/CGCUDANV.cpp cfe/trunk/lib/Driver/Action.cpp cfe/trunk/lib/Driver/Driver.cpp cfe/trunk/lib/Driver/ToolChains.cpp cfe/trunk/lib/Driver/ToolChains.h cfe/trunk/lib/Driver/Tools.cpp cfe/trunk/lib/Driver/Tools.h cfe/trunk/lib/Driver/Types.cpp cfe/trunk/test/Driver/Inputs/CUDA/usr/local/cuda/bin/.keep cfe/trunk/test/Driver/cuda-arch-translation.cu cfe/trunk/test/Driver/cuda-external-tools.cu cfe/trunk/test/Driver/cuda-options.cu Index: cfe/trunk/lib/CodeGen/CGCUDANV.cpp === --- cfe/trunk/lib/CodeGen/CGCUDANV.cpp +++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp @@ -259,6 +259,8 @@ TheModule, FatbinWrapperTy, true, llvm::GlobalValue::InternalLinkage, llvm::ConstantStruct::get(FatbinWrapperTy, Values), "__cuda_fatbin_wrapper"); +// NVIDIA's cuobjdump looks for fatbins in this section. +FatbinWrapper->setSection(".nvFatBinSegment"); // GpuBinaryHandle = __cudaRegisterFatBinary(&FatbinWrapper); llvm::CallInst *RegisterFatbinCall = CtorBuilder.CreateCall( Index: cfe/trunk/lib/Driver/Action.cpp === --- cfe/trunk/lib/Driver/Action.cpp +++ cfe/trunk/lib/Driver/Action.cpp @@ -75,7 +75,7 @@ bool AtTopLevel) : Action(CudaDeviceClass, Input), GpuArchName(ArchName), AtTopLevel(AtTopLevel) { - assert(IsValidGpuArchName(GpuArchName)); + assert(!GpuArchName || IsValidGpuArchName(GpuArchName)); } const char *CudaDeviceAction::getComputeArchName() const { Index: cfe/trunk/lib/Driver/ToolChains.h === --- cfe/trunk/lib/Driver/ToolChains.h +++ cfe/trunk/lib/Driver/ToolChains.h @@ -163,6 +163,7 @@ bool IsValid; const Driver &D; std::string CudaInstallPath; +std::string CudaBinPath; std::string CudaLibPath; std::string CudaLibDevicePath; std::string CudaIncludePath; @@ -179,6 +180,8 @@ /// \brief Get the detected Cuda installation path. StringRef getInstallPath() const { return CudaInstallPath; } +/// \brief Get the detected path to Cuda's bin directory. +StringRef getBinPath() const { return CudaBinPath; } /// \brief Get the detected Cuda Include path. StringRef getIncludePath() const { return CudaIncludePath; } /// \brief Get the detected Cuda library path. @@ -816,6 +819,14 @@ const char *BoundArch) const override; void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args) const override; + + // Never try to use the integrated assembler with CUDA; always fork out to + // ptxas. + bool useIntegratedAs() const override { return false; } + +protected: + Tool *buildAssembler() const override; // ptxas + Tool *buildLinker() const override; // fatbinary (ok, not really a linker) }; class LLVM_LIBRARY_VISIBILITY MipsLLVMToolChain : public Linux { Index: cfe/trunk/lib/Driver/Tools.h === --- cfe/trunk/lib/Driver/Tools.h +++ cfe/trunk/lib/Driver/Tools.h @@ -903,6 +903,41 @@ }; } // end namespace PS4cpu +namespace NVPTX { + +// Run ptxas, the NVPTX assembler. +class LLVM_LIBRARY_VISIBILITY Assembler : public Tool { + public: + Assembler(const ToolChain &TC) + : Tool("NVPTX::Assembler", "ptxas", TC, RF_Full, llvm::sys::WEM_UTF8, + "--options-file") {} + + bool hasIntegratedCPP() const override { return false; } + + void ConstructJob(Compilation &C, const JobAction &JA, + const InputInfo &Output, const InputInfoList &Inputs, + const llvm::opt::ArgList &TCArgs, + const char *LinkingOutput) const override; +}; + +// Runs fatbinary, which combines GPU object files ("cubin" files) and/or PTX +// assembly into a single output file. +class LLVM_LIBRARY_VISIBILITY Linker : public Tool { + public: + Linker(const ToolChain &TC) + : Tool("NVPTX::Linker", "fatbinary", TC, RF_Full, llvm::sys::WEM_UTF8, + "--options-file") {} + + bool hasIntegratedCPP() const override { return false; } + + void ConstructJob(Compilation &C, const JobAction &JA, + const InputInfo &Output, const InputInfoList &Inputs, + const llvm::opt::ArgList &TCArgs, + const ch
Re: [PATCH] D16171: Warning on redeclaring with a conflicting asm label
On 14 January 2016 at 10:38, Weiming Zhao via cfe-commits < cfe-commits@lists.llvm.org> wrote: > weimingz added a comment. > > Hi Nick, > > Below is a reduced code: > t.c: > > static long double acoshl (long double __x) __asm__ ("" "acosh") ; // > this is from /arm-linux-gnueabi/libc/usr/include/bits/mathcalls.h > extern long double acoshl (long double) __asm__ ("" "__acoshl_finite") ; > // this is from existing code > > GCC gives warning like: > /tmp/t.c:2:1: warning: asm declaration ignored due to conflict with > previous rename [-Wpragmas] > extern long double acoshl (long double) __asm__ ("" "__acoshl_finite") ; > That's the same case as in this testcase: void foo() __asm__("one"); void foo() __asm__("two"); void test() { foo(); } GCC emits a call to 'one' while Clang emits a call to 'two'. This is a real bug. Please don't downgrade this to a warning. As an alternative, I would accept a patch which changes how clang generates code so that it also produces a call to 'one', with a warning. It looks like what we need to do is drop subsequent asm label declarations on functions that already have one. Nick ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r257827 - [CMake] Set SVN_REVISION if CLANG_APPEND_VC_REV=On
Author: cbieneman Date: Thu Jan 14 16:45:12 2016 New Revision: 257827 URL: http://llvm.org/viewvc/llvm-project?rev=257827&view=rev Log: [CMake] Set SVN_REVISION if CLANG_APPEND_VC_REV=On This matches autoconf's ability to put clang revisions in the clang --version spew. Modified: cfe/trunk/CMakeLists.txt Modified: cfe/trunk/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=257827&r1=257826&r2=257827&view=diff == --- cfe/trunk/CMakeLists.txt (original) +++ cfe/trunk/CMakeLists.txt Thu Jan 14 16:45:12 2016 @@ -101,6 +101,7 @@ if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR include(AddLLVM) include(TableGen) include(HandleLLVMOptions) + include(VersionFromVCS) set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}") @@ -213,6 +214,18 @@ if(CLANG_REPOSITORY_STRING) add_definitions(-DCLANG_REPOSITORY_STRING="${CLANG_REPOSITORY_STRING}") endif() +option(CLANG_APPEND_VC_REV + "Append the version control system revision id to clang version spew" OFF) + +if(NOT SVN_REVISION) + # This macro will set SVN_REVISION in the parent scope + add_version_info_from_vcs(VERSION_VAR) +endif() + +if(SVN_REVISION) + add_definitions(-DSVN_REVISION="${SVN_REVISION}") +endif() + set(CLANG_VENDOR_UTI "org.llvm.clang" CACHE STRING "Vendor-specific uti.") ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
rsmith accepted this revision. rsmith added a reviewer: rsmith. rsmith added a comment. LGTM, thanks! Comment at: test/SemaCUDA/cxx11-kernel-call.cu:8 @@ +7,3 @@ +template void k1Wrapper() { + void (*f)() = [] { k1<<>>(); }; // expected-error {{initializer contains unexpanded parameter pack 'Dimensions'}} +} Perhaps also add another test: void (*g[])() = { [] { k1<<>>(); } ... }; // ok http://reviews.llvm.org/D15858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r257828 - [CMake] Move the install logic for libclang's headers into the libclang CMakelists
Author: cbieneman Date: Thu Jan 14 16:48:45 2016 New Revision: 257828 URL: http://llvm.org/viewvc/llvm-project?rev=257828&view=rev Log: [CMake] Move the install logic for libclang's headers into the libclang CMakelists This makes it so if you disable building libclang you won't install the headers as part of the 'install' target. Modified: cfe/trunk/CMakeLists.txt cfe/trunk/tools/libclang/CMakeLists.txt Modified: cfe/trunk/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=257828&r1=257827&r2=257828&view=diff == --- cfe/trunk/CMakeLists.txt (original) +++ cfe/trunk/CMakeLists.txt Thu Jan 14 16:48:45 2016 @@ -475,28 +475,6 @@ if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY) ) endif() -if(INTERNAL_INSTALL_PREFIX) - set(LIBCLANG_HEADERS_INSTALL_DESTINATION "${INTERNAL_INSTALL_PREFIX}/include") -else() - set(LIBCLANG_HEADERS_INSTALL_DESTINATION include) -endif() - -install(DIRECTORY include/clang-c - COMPONENT libclang-headers - DESTINATION "${LIBCLANG_HEADERS_INSTALL_DESTINATION}" - FILES_MATCHING - PATTERN "*.h" - PATTERN ".svn" EXCLUDE - ) - -if (NOT CMAKE_CONFIGURATION_TYPES) # don't add this for IDE's. - add_custom_target(install-libclang-headers -DEPENDS -COMMAND "${CMAKE_COMMAND}" --DCMAKE_INSTALL_COMPONENT=libclang-headers --P "${CMAKE_BINARY_DIR}/cmake_install.cmake") -endif() - add_definitions( -D_GNU_SOURCE ) option(CLANG_ENABLE_ARCMT "Build ARCMT." ON) Modified: cfe/trunk/tools/libclang/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CMakeLists.txt?rev=257828&r1=257827&r2=257828&view=diff == --- cfe/trunk/tools/libclang/CMakeLists.txt (original) +++ cfe/trunk/tools/libclang/CMakeLists.txt Thu Jan 14 16:48:45 2016 @@ -115,3 +115,25 @@ if(ENABLE_SHARED) DEFINE_SYMBOL _CINDEX_LIB_) endif() endif() + +if(INTERNAL_INSTALL_PREFIX) + set(LIBCLANG_HEADERS_INSTALL_DESTINATION "${INTERNAL_INSTALL_PREFIX}/include") +else() + set(LIBCLANG_HEADERS_INSTALL_DESTINATION include) +endif() + +install(DIRECTORY ../../include/clang-c + COMPONENT libclang-headers + DESTINATION "${LIBCLANG_HEADERS_INSTALL_DESTINATION}" + FILES_MATCHING + PATTERN "*.h" + PATTERN ".svn" EXCLUDE + ) + +if (NOT CMAKE_CONFIGURATION_TYPES) # don't add this for IDE's. + add_custom_target(install-libclang-headers +DEPENDS +COMMAND "${CMAKE_COMMAND}" +-DCMAKE_INSTALL_COMPONENT=libclang-headers +-P "${CMAKE_BINARY_DIR}/cmake_install.cmake") +endif() ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r257831 - Refactor template type diffing
Author: rtrieu Date: Thu Jan 14 16:56:39 2016 New Revision: 257831 URL: http://llvm.org/viewvc/llvm-project?rev=257831&view=rev Log: Refactor template type diffing 1) Instead of using pairs of From/To* fields, combine fields into a struct TemplateArgInfo and have two in each DiffNode. 2) Use default initialization in DiffNode so that the constructor shows the only field that is initialized differently on construction. 3) Use Set and Get functions per each DiffKind to make sure all fields for the diff is set. In one case, the Expr fields were not set. 4) Don't print boolean literals for boolean template arguments. This prevents printing 'false aka 0' Only #3 has a functional change, which is reflected in the test change. Modified: cfe/trunk/lib/AST/ASTDiagnostic.cpp cfe/trunk/test/Misc/diag-template-diffing.cpp Modified: cfe/trunk/lib/AST/ASTDiagnostic.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDiagnostic.cpp?rev=257831&r1=257830&r2=257831&view=diff == --- cfe/trunk/lib/AST/ASTDiagnostic.cpp (original) +++ cfe/trunk/lib/AST/ASTDiagnostic.cpp Thu Jan 14 16:56:39 2016 @@ -491,82 +491,67 @@ class TemplateDiff { /// DiffTree - A tree representation the differences between two types. class DiffTree { public: -/// DiffKind - The difference in a DiffNode and which fields are used. +/// DiffKind - The difference in a DiffNode. Fields of +/// TemplateArgumentInfo needed by each difference can be found in the +/// Set* and Get* functions. enum DiffKind { /// Incomplete or invalid node. Invalid, - /// Another level of templates, uses TemplateDecl and Qualifiers + /// Another level of templates, requires that Template, - /// Type difference, uses QualType + /// Type difference, all type differences except those falling under + /// the Template difference. Type, - /// Expression difference, uses Expr + /// Expression difference, this is only when both arguments are + /// expressions. If one argument is an expression and the other is + /// Integer or Declaration, then use that diff type instead. Expression, - /// Template argument difference, uses TemplateDecl + /// Template argument difference TemplateTemplate, - /// Integer difference, uses APSInt and Expr + /// Integer difference Integer, - /// Declaration difference, uses ValueDecl + /// Declaration difference, nullptr arguments are included here Declaration }; + private: +/// TemplateArgumentInfo - All the information needed to pretty print +/// a template argument. See the Set* and Get* functions to see which +/// fields are used for each DiffKind. +struct TemplateArgumentInfo { + QualType ArgType; + Qualifiers Qual; + llvm::APSInt Val; + bool IsValidInt = false; + Expr *ArgExpr = nullptr; + TemplateDecl *TD = nullptr; + ValueDecl *VD = nullptr; + bool NeedAddressOf = false; + bool IsNullPtr = false; + bool IsDefault = false; +}; + /// DiffNode - The root node stores the original type. Each child node /// stores template arguments of their parents. For templated types, the /// template decl is also stored. struct DiffNode { - DiffKind Kind; + DiffKind Kind = Invalid; /// NextNode - The index of the next sibling node or 0. - unsigned NextNode; + unsigned NextNode = 0; /// ChildNode - The index of the first child node or 0. - unsigned ChildNode; + unsigned ChildNode = 0; /// ParentNode - The index of the parent node. - unsigned ParentNode; - - /// FromType, ToType - The type arguments. - QualType FromType, ToType; - - /// FromExpr, ToExpr - The expression arguments. - Expr *FromExpr, *ToExpr; - - /// FromNullPtr, ToNullPtr - If the template argument is a nullptr - bool FromNullPtr, ToNullPtr; - - /// FromTD, ToTD - The template decl for template template - /// arguments or the type arguments that are templates. - TemplateDecl *FromTD, *ToTD; - - /// FromQual, ToQual - Qualifiers for template types. - Qualifiers FromQual, ToQual; - - /// FromInt, ToInt - APSInt's for integral arguments. - llvm::APSInt FromInt, ToInt; - - /// IsValidFromInt, IsValidToInt - Whether the APSInt's are valid. - bool IsValidFromInt, IsValidToInt; + unsigned ParentNode = 0; - /// FromValueDecl, ToValueDecl - Whether the argument is a decl. - ValueDecl *FromValueDecl, *ToValueDecl; - - /// FromAddressOf, ToAddressOf - Whether the ValueDecl needs an address of - /// operator before it. - bool FromAddressOf, ToAddressOf; - - /// FromDefault, ToDefault - Whether the argument is a default argument. - bool FromDefault, ToDefault; + TemplateArgum
Re: [PATCH] D16171: Warning on redeclaring with a conflicting asm label
weimingz updated this revision to Diff 44931. weimingz added a comment. if the new decl is not used, we can just give warnings http://reviews.llvm.org/D16171 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp test/Sema/asm-label.c Index: test/Sema/asm-label.c === --- test/Sema/asm-label.c +++ test/Sema/asm-label.c @@ -10,7 +10,7 @@ void g() __asm__("gold"); // expected-error{{cannot apply asm label to function after its first use}} void h() __asm__("hose"); // expected-note{{previous declaration is here}} -void h() __asm__("hair"); // expected-error{{conflicting asm label}} +void h() __asm__("hair"); // expected-warning{{conflicting asm label}} int x; int x __asm__("xenon"); @@ -21,7 +21,7 @@ int y __asm__("yacht"); // expected-error{{cannot apply asm label to variable after its first use}} int z __asm__("zebra"); // expected-note{{previous declaration is here}} -int z __asm__("zooms"); // expected-error{{conflicting asm label}} +int z __asm__("zooms"); // expected-warning{{conflicting asm label}} // No diagnostics on the following. Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -2385,7 +2385,10 @@ if (AsmLabelAttr *OldA = Old->getAttr()) { if (OldA->getLabel() != NewA->getLabel()) { // This redeclaration changes __asm__ label. -Diag(New->getLocation(), diag::err_different_asm_label); +if (New->isUsed()) + Diag(New->getLocation(), diag::err_different_asm_label); +else + Diag(New->getLocation(), diag::warn_different_asm_label); Diag(OldA->getLocation(), diag::note_previous_declaration); } } else if (Old->isUsed()) { Index: include/clang/Basic/DiagnosticSemaKinds.td === --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -4272,6 +4272,8 @@ def err_nested_redefinition : Error<"nested redefinition of %0">; def err_use_with_wrong_tag : Error< "use of %0 with tag type that does not match previous declaration">; +def warn_different_asm_label : Warning<"conflicting asm label">, +InGroup; def warn_struct_class_tag_mismatch : Warning< "%select{struct|interface|class}0%select{| template}1 %2 was previously " "declared as a %select{struct|interface|class}3%select{| template}1">, Index: test/Sema/asm-label.c === --- test/Sema/asm-label.c +++ test/Sema/asm-label.c @@ -10,7 +10,7 @@ void g() __asm__("gold"); // expected-error{{cannot apply asm label to function after its first use}} void h() __asm__("hose"); // expected-note{{previous declaration is here}} -void h() __asm__("hair"); // expected-error{{conflicting asm label}} +void h() __asm__("hair"); // expected-warning{{conflicting asm label}} int x; int x __asm__("xenon"); @@ -21,7 +21,7 @@ int y __asm__("yacht"); // expected-error{{cannot apply asm label to variable after its first use}} int z __asm__("zebra"); // expected-note{{previous declaration is here}} -int z __asm__("zooms"); // expected-error{{conflicting asm label}} +int z __asm__("zooms"); // expected-warning{{conflicting asm label}} // No diagnostics on the following. Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -2385,7 +2385,10 @@ if (AsmLabelAttr *OldA = Old->getAttr()) { if (OldA->getLabel() != NewA->getLabel()) { // This redeclaration changes __asm__ label. -Diag(New->getLocation(), diag::err_different_asm_label); +if (New->isUsed()) + Diag(New->getLocation(), diag::err_different_asm_label); +else + Diag(New->getLocation(), diag::warn_different_asm_label); Diag(OldA->getLocation(), diag::note_previous_declaration); } } else if (Old->isUsed()) { Index: include/clang/Basic/DiagnosticSemaKinds.td === --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -4272,6 +4272,8 @@ def err_nested_redefinition : Error<"nested redefinition of %0">; def err_use_with_wrong_tag : Error< "use of %0 with tag type that does not match previous declaration">; +def warn_different_asm_label : Warning<"conflicting asm label">, +InGroup; def warn_struct_class_tag_mismatch : Warning< "%select{struct|interface|class}0%select{| template}1 %2 was previously " "declared as a %select{struct|interface|class}3%select{| template}1">, ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16171: Warning on redeclaring with a conflicting asm label
I agree what you said about different code generated with clang and GCC generates. In this case, we should throw an error (err_late_asm_label). But in this example, there is no use of the function. They are just redundant declarations and there is no actual code generated. So I suggest we just give warnings for this case. Otherwise, it will break existing code like some SPEC benchmarks. Please review my 2nd patch. Weiming On 1/14/2016 2:28 PM, Nick Lewycky wrote: On 14 January 2016 at 10:38, Weiming Zhao via cfe-commits mailto:cfe-commits@lists.llvm.org>> wrote: weimingz added a comment. Hi Nick, Below is a reduced code: t.c: static long double acoshl (long double __x) __asm__ ("" "acosh") ; // this is from /arm-linux-gnueabi/libc/usr/include/bits/mathcalls.h extern long double acoshl (long double) __asm__ ("" "__acoshl_finite") ; // this is from existing code GCC gives warning like: /tmp/t.c:2:1: warning: asm declaration ignored due to conflict with previous rename [-Wpragmas] extern long double acoshl (long double) __asm__ ("" "__acoshl_finite") ; That's the same case as in this testcase: void foo() __asm__("one"); void foo() __asm__("two"); void test() { foo(); } GCC emits a call to 'one' while Clang emits a call to 'two'. This is a real bug. Please don't downgrade this to a warning. As an alternative, I would accept a patch which changes how clang generates code so that it also produces a call to 'one', with a warning. It looks like what we need to do is drop subsequent asm label declarations on functions that already have one. Nick ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jhen updated this revision to Diff 44933. jhen added a comment. - Add extra test for OK parameter pack http://reviews.llvm.org/D15858 Files: include/clang/AST/Expr.h include/clang/AST/ExprCXX.h lib/AST/Expr.cpp test/SemaCUDA/cxx11-kernel-call.cu test/SemaCUDA/kernel-call.cu Index: test/SemaCUDA/kernel-call.cu === --- test/SemaCUDA/kernel-call.cu +++ test/SemaCUDA/kernel-call.cu @@ -23,4 +23,6 @@ int (*fp)(int) = h2; fp<<<1, 1>>>(42); // expected-error {{must have void return type}} + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } Index: test/SemaCUDA/cxx11-kernel-call.cu === --- /dev/null +++ test/SemaCUDA/cxx11-kernel-call.cu @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s + +#include "Inputs/cuda.h" + +__global__ void k1() {} + +template void k1Wrapper() { + void (*f)() = [] { k1<<>>(); }; // expected-error {{initializer contains unexpanded parameter pack 'Dimensions'}} + void (*g[])() = { [] { k1<<>>(); } ... }; // ok +} Index: lib/AST/Expr.cpp === --- lib/AST/Expr.cpp +++ lib/AST/Expr.cpp @@ -1138,38 +1138,38 @@ // Postfix Operators. //===--===// -CallExpr::CallExpr(const ASTContext& C, StmtClass SC, Expr *fn, - unsigned NumPreArgs, ArrayRef args, QualType t, +CallExpr::CallExpr(const ASTContext &C, StmtClass SC, Expr *fn, + ArrayRef preargs, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation rparenloc) - : Expr(SC, t, VK, OK_Ordinary, - fn->isTypeDependent(), - fn->isValueDependent(), - fn->isInstantiationDependent(), - fn->containsUnexpandedParameterPack()), -NumArgs(args.size()) { - - SubExprs = new (C) Stmt*[args.size()+PREARGS_START+NumPreArgs]; +: Expr(SC, t, VK, OK_Ordinary, fn->isTypeDependent(), + fn->isValueDependent(), fn->isInstantiationDependent(), + fn->containsUnexpandedParameterPack()), + NumArgs(args.size()) { + + unsigned NumPreArgs = preargs.size(); + SubExprs = new (C) Stmt *[args.size()+PREARGS_START+NumPreArgs]; SubExprs[FN] = fn; + for (unsigned i = 0; i != NumPreArgs; ++i) { +updateDependenciesFromArg(preargs[i]); +SubExprs[i+PREARGS_START] = preargs[i]; + } for (unsigned i = 0; i != args.size(); ++i) { -if (args[i]->isTypeDependent()) - ExprBits.TypeDependent = true; -if (args[i]->isValueDependent()) - ExprBits.ValueDependent = true; -if (args[i]->isInstantiationDependent()) - ExprBits.InstantiationDependent = true; -if (args[i]->containsUnexpandedParameterPack()) - ExprBits.ContainsUnexpandedParameterPack = true; - +updateDependenciesFromArg(args[i]); SubExprs[i+PREARGS_START+NumPreArgs] = args[i]; } CallExprBits.NumPreArgs = NumPreArgs; RParenLoc = rparenloc; } +CallExpr::CallExpr(const ASTContext &C, StmtClass SC, Expr *fn, + ArrayRef args, QualType t, ExprValueKind VK, + SourceLocation rparenloc) +: CallExpr(C, SC, fn, ArrayRef(), args, t, VK, rparenloc) {} + CallExpr::CallExpr(const ASTContext &C, Expr *fn, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation rparenloc) -: CallExpr(C, CallExprClass, fn, /*NumPreArgs=*/0, args, t, VK, rparenloc) { +: CallExpr(C, CallExprClass, fn, ArrayRef(), args, t, VK, rparenloc) { } CallExpr::CallExpr(const ASTContext &C, StmtClass SC, EmptyShell Empty) @@ -1179,10 +1179,21 @@ EmptyShell Empty) : Expr(SC, Empty), SubExprs(nullptr), NumArgs(0) { // FIXME: Why do we allocate this? - SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs]; + SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs](); CallExprBits.NumPreArgs = NumPreArgs; } +void CallExpr::updateDependenciesFromArg(Expr *Arg) { + if (Arg->isTypeDependent()) +ExprBits.TypeDependent = true; + if (Arg->isValueDependent()) +ExprBits.ValueDependent = true; + if (Arg->isInstantiationDependent()) +ExprBits.InstantiationDependent = true; + if (Arg->containsUnexpandedParameterPack()) +ExprBits.ContainsUnexpandedParameterPack = true; +} + Decl *CallExpr::getCalleeDecl() { Expr *CEE = getCallee()->IgnoreParenImpCasts(); Index: include/clang/AST/ExprCXX.h === --- include/clang/AST/ExprCXX.h +++ include/clang/AST/ExprCXX.h @@ -66,8 +66,7 @@ CXXOperatorCallExpr(ASTContext& C, OverloadedOperatorKind Op, Expr *fn, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation operatorloc, bool fpContractable) -: CallExpr(C, CXXOperatorCallExprClass, fn, 0, args, t, VK, - operatorloc), +
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
jhen marked an inline comment as done. jhen added a comment. Thanks for the review rsmith! http://reviews.llvm.org/D15858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16171: Warning on redeclaring with a conflicting asm label
On Thu, Jan 14, 2016 at 03:05:26PM -0800, Zhao, Weiming via cfe-commits wrote: > I agree what you said about different code generated with clang and GCC > generates. In this case, we should throw an error (err_late_asm_label). > > But in this example, there is no use of the function. They are just > redundant declarations and there is no actual code generated. > So I suggest we just give warnings for this case. Otherwise, it will break > existing code like some SPEC benchmarks. By the same argument, redefinitions with different types should be valid in C. I don't like this slippery slope at all, code should really be just fixed here. Joerg ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16175: Introduce -fsanitize-stats flag.
pcc updated this revision to Diff 44936. pcc added a comment. - Switch to an alternative in-memory representation that avoids the need for linker magic http://reviews.llvm.org/D16175 Files: docs/SanitizerStats.rst docs/UsersManual.rst docs/index.rst include/clang/AST/ASTConsumer.h include/clang/Driver/CC1Options.td include/clang/Driver/Options.td include/clang/Driver/SanitizerArgs.h include/clang/Frontend/CodeGenOptions.def include/clang/Frontend/CodeGenOptions.h include/clang/Frontend/MultiplexConsumer.h lib/CodeGen/CGClass.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CodeGenAction.cpp lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h lib/CodeGen/ModuleBuilder.cpp lib/Driver/SanitizerArgs.cpp lib/Driver/ToolChains.cpp lib/Driver/Tools.cpp lib/Frontend/CompilerInvocation.cpp lib/Frontend/MultiplexConsumer.cpp lib/Sema/SemaAttr.cpp test/CodeGen/linker-option.c test/CodeGenCXX/cfi-stats.cpp test/Driver/fsanitize.c test/Driver/sanitizer-ld.c Index: test/Driver/sanitizer-ld.c === --- test/Driver/sanitizer-ld.c +++ test/Driver/sanitizer-ld.c @@ -345,6 +345,38 @@ // CHECK-SAFESTACK-LINUX: "-lpthread" // CHECK-SAFESTACK-LINUX: "-ldl" +// RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \ +// RUN: -target x86_64-unknown-linux \ +// RUN: --sysroot=%S/Inputs/basic_linux_tree \ +// RUN: | FileCheck --check-prefix=CHECK-CFI-STATS-LINUX %s +// CHECK-CFI-STATS-LINUX: "{{.*}}ld{{(.exe)?}}" +// CHECK-CFI-STATS-LINUX: "-whole-archive" "{{[^"]*}}libclang_rt.stats_client-x86_64.a" "-no-whole-archive" +// CHECK-CFI-STATS-LINUX: "-whole-archive" "{{[^"]*}}libclang_rt.stats-x86_64.a" "-no-whole-archive" + +// RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \ +// RUN: -target x86_64-apple-darwin \ +// RUN: --sysroot=%S/Inputs/basic_linux_tree \ +// RUN: | FileCheck --check-prefix=CHECK-CFI-STATS-DARWIN %s +// CHECK-CFI-STATS-DARWIN: "{{.*}}ld{{(.exe)?}}" +// CHECK-CFI-STATS-DARWIN: "{{[^"]*}}libclang_rt.stats_client_osx.a" +// CHECK-CFI-STATS-DARWIN: "{{[^"]*}}libclang_rt.stats_osx_dynamic.dylib" + +// RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \ +// RUN: -target x86_64-pc-windows \ +// RUN: --sysroot=%S/Inputs/basic_linux_tree \ +// RUN: | FileCheck --check-prefix=CHECK-CFI-STATS-WIN64 %s +// CHECK-CFI-STATS-WIN64: "--dependent-lib={{[^"]*}}clang_rt.stats_client-x86_64.lib" +// CHECK-CFI-STATS-WIN64: "--dependent-lib={{[^"]*}}clang_rt.stats-x86_64.lib" +// CHECK-CFI-STATS-WIN64: "--linker-option=/include:__sanitizer_stats_register" + +// RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \ +// RUN: -target i686-pc-windows \ +// RUN: --sysroot=%S/Inputs/basic_linux_tree \ +// RUN: | FileCheck --check-prefix=CHECK-CFI-STATS-WIN32 %s +// CHECK-CFI-STATS-WIN32: "--dependent-lib={{[^"]*}}clang_rt.stats_client-i386.lib" +// CHECK-CFI-STATS-WIN32: "--dependent-lib={{[^"]*}}clang_rt.stats-i386.lib" +// CHECK-CFI-STATS-WIN32: "--linker-option=/include:___sanitizer_stats_register" + // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ // RUN: -target arm-linux-androideabi -fsanitize=safe-stack \ // RUN: --sysroot=%S/Inputs/basic_android_tree \ Index: test/Driver/fsanitize.c === --- test/Driver/fsanitize.c +++ test/Driver/fsanitize.c @@ -272,6 +272,9 @@ // CHECK-CFI-NO-CROSS-DSO: -emit-llvm-bc // CHECK-CFI-NO-CROSS-DSO-NOT: -fsanitize-cfi-cross-dso +// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -fsanitize-stats -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-STATS +// CHECK-CFI-STATS: -fsanitize-stats + // RUN: %clang_cl -fsanitize=address -c -MDd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL // RUN: %clang_cl -fsanitize=address -c -MTd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL // RUN: %clang_cl -fsanitize=address -c -LDd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL Index: test/CodeGenCXX/cfi-stats.cpp === --- /dev/null +++ test/CodeGenCXX/cfi-stats.cpp @@ -0,0 +1,50 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-vcall,cfi-nvcall,cfi-derived-cast,cfi-unrelated-cast,cfi-icall -fsanitize-stats -emit-llvm -o - %s | FileCheck --check-prefix=CHECK %s + +// CHECK: [[STATS:@[^ ]*]] = private global { i8*, i32, [5 x [2 x i8*]] } { i8* null, i32 5, [5 x [2 x i8*]] +// CHECK: {{\[\[}}2 x i8*] zeroinitializer, +// CHECK: [2 x i8*] [i8* null, i8* inttoptr (i64 2305843009213693952 to i8*)], +// CHECK: [2 x i8*] [i8* null, i8* inttoptr (i64 4611686018427387904 to i8*)], +// CHECK: [2 x i8*] [i8* null, i8* inttoptr (i64 6917529027641081856 to i8*)], +// CHECK: [2 x i8*] [i8* null, i8* inttoptr (i64 -92
r257838 - Change the TSTiterator in Template Type Diffing.
Author: rtrieu Date: Thu Jan 14 17:30:12 2016 New Revision: 257838 URL: http://llvm.org/viewvc/llvm-project?rev=257838&view=rev Log: Change the TSTiterator in Template Type Diffing. Modify the TSTiterator to have two internal iterators, which will walk the provided sugared type and the desugared type. This will provide better access to the template argument information. No functional changes. Modified: cfe/trunk/lib/AST/ASTDiagnostic.cpp Modified: cfe/trunk/lib/AST/ASTDiagnostic.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDiagnostic.cpp?rev=257838&r1=257837&r2=257838&view=diff == --- cfe/trunk/lib/AST/ASTDiagnostic.cpp (original) +++ cfe/trunk/lib/AST/ASTDiagnostic.cpp Thu Jan 14 17:30:12 2016 @@ -808,102 +808,128 @@ class TemplateDiff { DiffTree Tree; - /// TSTiterator - an iterator that is used to enter a - /// TemplateSpecializationType and read TemplateArguments inside template - /// parameter packs in order with the rest of the TemplateArguments. - struct TSTiterator { + /// TSTiterator - a pair of iterators that walks the + /// TemplateSpecializationType and the desugared TemplateSpecializationType. + /// The deseguared TemplateArgument should provide the canonical argument + /// for comparisons. + class TSTiterator { typedef const TemplateArgument& reference; typedef const TemplateArgument* pointer; -/// TST - the template specialization whose arguments this iterator -/// traverse over. -const TemplateSpecializationType *TST; - -/// DesugarTST - desugared template specialization used to extract -/// default argument information -const TemplateSpecializationType *DesugarTST; - -/// Index - the index of the template argument in TST. -unsigned Index; - -/// CurrentTA - if CurrentTA is not the same as EndTA, then CurrentTA -/// points to a TemplateArgument within a parameter pack. -TemplateArgument::pack_iterator CurrentTA; +/// InternalIterator - an iterator that is used to enter a +/// TemplateSpecializationType and read TemplateArguments inside template +/// parameter packs in order with the rest of the TemplateArguments. +struct InternalIterator { + /// TST - the template specialization whose arguments this iterator + /// traverse over. + const TemplateSpecializationType *TST; + + /// Index - the index of the template argument in TST. + unsigned Index; + + /// CurrentTA - if CurrentTA is not the same as EndTA, then CurrentTA + /// points to a TemplateArgument within a parameter pack. + TemplateArgument::pack_iterator CurrentTA; + + /// EndTA - the end iterator of a parameter pack + TemplateArgument::pack_iterator EndTA; + + /// InternalIterator - Constructs an iterator and sets it to the first + /// template argument. + InternalIterator(const TemplateSpecializationType *TST) + : TST(TST), Index(0), CurrentTA(nullptr), EndTA(nullptr) { +if (isEnd()) return; + +// Set to first template argument. If not a parameter pack, done. +TemplateArgument TA = TST->getArg(0); +if (TA.getKind() != TemplateArgument::Pack) return; -/// EndTA - the end iterator of a parameter pack -TemplateArgument::pack_iterator EndTA; - -/// TSTiterator - Constructs an iterator and sets it to the first template -/// argument. -TSTiterator(ASTContext &Context, const TemplateSpecializationType *TST) -: TST(TST), - DesugarTST(GetTemplateSpecializationType(Context, TST->desugar())), - Index(0), CurrentTA(nullptr), EndTA(nullptr) { - if (isEnd()) return; +// Start looking into the parameter pack. +CurrentTA = TA.pack_begin(); +EndTA = TA.pack_end(); - // Set to first template argument. If not a parameter pack, done. - TemplateArgument TA = TST->getArg(0); - if (TA.getKind() != TemplateArgument::Pack) return; +// Found a valid template argument. +if (CurrentTA != EndTA) return; - // Start looking into the parameter pack. - CurrentTA = TA.pack_begin(); - EndTA = TA.pack_end(); +// Parameter pack is empty, use the increment to get to a valid +// template argument. +++(*this); + } - // Found a valid template argument. - if (CurrentTA != EndTA) return; + /// isEnd - Returns true if the iterator is one past the end. + bool isEnd() const { +return Index >= TST->getNumArgs(); + } - // Parameter pack is empty, use the increment to get to a valid - // template argument. - ++(*this); -} + /// &operator++ - Increment the iterator to the next template argument. + InternalIterator &operator++() { +if (isEnd()) { + return *this; +} -/// isEnd - Returns true if the iterator is one past the end. -bool
r257839 - [CUDA] Warn undeclared identifiers in CUDA kernel calls
Author: jlebar Date: Thu Jan 14 17:31:30 2016 New Revision: 257839 URL: http://llvm.org/viewvc/llvm-project?rev=257839&view=rev Log: [CUDA] Warn undeclared identifiers in CUDA kernel calls Value, type, and instantiation dependence were not being handled correctly for CUDAKernelCallExpr AST nodes. As a result, if an undeclared identifier was used in the triple-angle-bracket kernel call configuration, there would be no error during parsing, and there would be a crash during code gen. This patch makes sure that an error will be issued during parsing in this case, just as there would be for any other use of an undeclared identifier in C++. Patch by Jason Henline. Reviewers: jlebar, rsmith Differential Revision: http://reviews.llvm.org/D15858 Added: cfe/trunk/test/SemaCUDA/cxx11-kernel-call.cu Modified: cfe/trunk/include/clang/AST/Expr.h cfe/trunk/include/clang/AST/ExprCXX.h cfe/trunk/lib/AST/Expr.cpp cfe/trunk/test/SemaCUDA/kernel-call.cu Modified: cfe/trunk/include/clang/AST/Expr.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=257839&r1=257838&r2=257839&view=diff == --- cfe/trunk/include/clang/AST/Expr.h (original) +++ cfe/trunk/include/clang/AST/Expr.h Thu Jan 14 17:31:30 2016 @@ -2137,11 +2137,15 @@ class CallExpr : public Expr { unsigned NumArgs; SourceLocation RParenLoc; + void updateDependenciesFromArg(Expr *Arg); + protected: // These versions of the constructor are for derived classes. - CallExpr(const ASTContext& C, StmtClass SC, Expr *fn, unsigned NumPreArgs, - ArrayRef args, QualType t, ExprValueKind VK, - SourceLocation rparenloc); + CallExpr(const ASTContext &C, StmtClass SC, Expr *fn, + ArrayRef preargs, ArrayRef args, QualType t, + ExprValueKind VK, SourceLocation rparenloc); + CallExpr(const ASTContext &C, StmtClass SC, Expr *fn, ArrayRef args, + QualType t, ExprValueKind VK, SourceLocation rparenloc); CallExpr(const ASTContext &C, StmtClass SC, unsigned NumPreArgs, EmptyShell Empty); Modified: cfe/trunk/include/clang/AST/ExprCXX.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=257839&r1=257838&r2=257839&view=diff == --- cfe/trunk/include/clang/AST/ExprCXX.h (original) +++ cfe/trunk/include/clang/AST/ExprCXX.h Thu Jan 14 17:31:30 2016 @@ -66,8 +66,7 @@ public: CXXOperatorCallExpr(ASTContext& C, OverloadedOperatorKind Op, Expr *fn, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation operatorloc, bool fpContractable) -: CallExpr(C, CXXOperatorCallExprClass, fn, 0, args, t, VK, - operatorloc), +: CallExpr(C, CXXOperatorCallExprClass, fn, args, t, VK, operatorloc), Operator(Op), FPContractable(fpContractable) { Range = getSourceRangeImpl(); } @@ -125,7 +124,7 @@ class CXXMemberCallExpr : public CallExp public: CXXMemberCallExpr(ASTContext &C, Expr *fn, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation RP) -: CallExpr(C, CXXMemberCallExprClass, fn, 0, args, t, VK, RP) {} +: CallExpr(C, CXXMemberCallExprClass, fn, args, t, VK, RP) {} CXXMemberCallExpr(ASTContext &C, EmptyShell Empty) : CallExpr(C, CXXMemberCallExprClass, Empty) { } @@ -160,9 +159,7 @@ public: CUDAKernelCallExpr(ASTContext &C, Expr *fn, CallExpr *Config, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation RP) -: CallExpr(C, CUDAKernelCallExprClass, fn, END_PREARG, args, t, VK, RP) { -setConfig(Config); - } + : CallExpr(C, CUDAKernelCallExprClass, fn, Config, args, t, VK, RP) {} CUDAKernelCallExpr(ASTContext &C, EmptyShell Empty) : CallExpr(C, CUDAKernelCallExprClass, END_PREARG, Empty) { } @@ -171,7 +168,20 @@ public: return cast_or_null(getPreArg(CONFIG)); } CallExpr *getConfig() { return cast_or_null(getPreArg(CONFIG)); } - void setConfig(CallExpr *E) { setPreArg(CONFIG, E); } + + /// \brief Sets the kernel configuration expression. + /// + /// Note that this method cannot be called if config has already been set to a + /// non-null value. + void setConfig(CallExpr *E) { +assert(!getConfig() && + "Cannot call setConfig if config is not null"); +setPreArg(CONFIG, E); +setInstantiationDependent(isInstantiationDependent() || + E->isInstantiationDependent()); +setContainsUnexpandedParameterPack(containsUnexpandedParameterPack() || + E->containsUnexpandedParameterPack()); + } static bool classof(const Stmt *T) { return T->getStmtClass() == CUDAKernelCallExprClass; @@ -398,7 +408,7 @@ public: UserDefinedLiteral(const ASTContext &C, Expr *Fn, ArrayRef Args,
Re: [PATCH] D15858: Warn undeclared identifiers in CUDA kernel calls
This revision was automatically updated to reflect the committed changes. Closed by commit rL257839: [CUDA] Warn undeclared identifiers in CUDA kernel calls (authored by jlebar). Changed prior to commit: http://reviews.llvm.org/D15858?vs=44933&id=44938#toc Repository: rL LLVM http://reviews.llvm.org/D15858 Files: cfe/trunk/include/clang/AST/Expr.h cfe/trunk/include/clang/AST/ExprCXX.h cfe/trunk/lib/AST/Expr.cpp cfe/trunk/test/SemaCUDA/cxx11-kernel-call.cu cfe/trunk/test/SemaCUDA/kernel-call.cu Index: cfe/trunk/test/SemaCUDA/cxx11-kernel-call.cu === --- cfe/trunk/test/SemaCUDA/cxx11-kernel-call.cu +++ cfe/trunk/test/SemaCUDA/cxx11-kernel-call.cu @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s + +#include "Inputs/cuda.h" + +__global__ void k1() {} + +template void k1Wrapper() { + void (*f)() = [] { k1<<>>(); }; // expected-error {{initializer contains unexpanded parameter pack 'Dimensions'}} + void (*g[])() = { [] { k1<<>>(); } ... }; // ok +} Index: cfe/trunk/test/SemaCUDA/kernel-call.cu === --- cfe/trunk/test/SemaCUDA/kernel-call.cu +++ cfe/trunk/test/SemaCUDA/kernel-call.cu @@ -23,4 +23,6 @@ int (*fp)(int) = h2; fp<<<1, 1>>>(42); // expected-error {{must have void return type}} + + g1<<>>(42); // expected-error {{use of undeclared identifier 'undeclared'}} } Index: cfe/trunk/lib/AST/Expr.cpp === --- cfe/trunk/lib/AST/Expr.cpp +++ cfe/trunk/lib/AST/Expr.cpp @@ -1138,38 +1138,38 @@ // Postfix Operators. //===--===// -CallExpr::CallExpr(const ASTContext& C, StmtClass SC, Expr *fn, - unsigned NumPreArgs, ArrayRef args, QualType t, +CallExpr::CallExpr(const ASTContext &C, StmtClass SC, Expr *fn, + ArrayRef preargs, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation rparenloc) - : Expr(SC, t, VK, OK_Ordinary, - fn->isTypeDependent(), - fn->isValueDependent(), - fn->isInstantiationDependent(), - fn->containsUnexpandedParameterPack()), -NumArgs(args.size()) { +: Expr(SC, t, VK, OK_Ordinary, fn->isTypeDependent(), + fn->isValueDependent(), fn->isInstantiationDependent(), + fn->containsUnexpandedParameterPack()), + NumArgs(args.size()) { - SubExprs = new (C) Stmt*[args.size()+PREARGS_START+NumPreArgs]; + unsigned NumPreArgs = preargs.size(); + SubExprs = new (C) Stmt *[args.size()+PREARGS_START+NumPreArgs]; SubExprs[FN] = fn; + for (unsigned i = 0; i != NumPreArgs; ++i) { +updateDependenciesFromArg(preargs[i]); +SubExprs[i+PREARGS_START] = preargs[i]; + } for (unsigned i = 0; i != args.size(); ++i) { -if (args[i]->isTypeDependent()) - ExprBits.TypeDependent = true; -if (args[i]->isValueDependent()) - ExprBits.ValueDependent = true; -if (args[i]->isInstantiationDependent()) - ExprBits.InstantiationDependent = true; -if (args[i]->containsUnexpandedParameterPack()) - ExprBits.ContainsUnexpandedParameterPack = true; - +updateDependenciesFromArg(args[i]); SubExprs[i+PREARGS_START+NumPreArgs] = args[i]; } CallExprBits.NumPreArgs = NumPreArgs; RParenLoc = rparenloc; } +CallExpr::CallExpr(const ASTContext &C, StmtClass SC, Expr *fn, + ArrayRef args, QualType t, ExprValueKind VK, + SourceLocation rparenloc) +: CallExpr(C, SC, fn, ArrayRef(), args, t, VK, rparenloc) {} + CallExpr::CallExpr(const ASTContext &C, Expr *fn, ArrayRef args, QualType t, ExprValueKind VK, SourceLocation rparenloc) -: CallExpr(C, CallExprClass, fn, /*NumPreArgs=*/0, args, t, VK, rparenloc) { +: CallExpr(C, CallExprClass, fn, ArrayRef(), args, t, VK, rparenloc) { } CallExpr::CallExpr(const ASTContext &C, StmtClass SC, EmptyShell Empty) @@ -1179,10 +1179,21 @@ EmptyShell Empty) : Expr(SC, Empty), SubExprs(nullptr), NumArgs(0) { // FIXME: Why do we allocate this? - SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs]; + SubExprs = new (C) Stmt*[PREARGS_START+NumPreArgs](); CallExprBits.NumPreArgs = NumPreArgs; } +void CallExpr::updateDependenciesFromArg(Expr *Arg) { + if (Arg->isTypeDependent()) +ExprBits.TypeDependent = true; + if (Arg->isValueDependent()) +ExprBits.ValueDependent = true; + if (Arg->isInstantiationDependent()) +ExprBits.InstantiationDependent = true; + if (Arg->containsUnexpandedParameterPack()) +ExprBits.ContainsUnexpandedParameterPack = true; +} + Decl *CallExpr::getCalleeDecl() { Expr *CEE = getCallee()->IgnoreParenImpCasts(); Index: cfe/trunk/include/clang/AST/Expr.h === --- cfe/trunk/include/clang/AST/Expr.h +++ c
Re: [PATCH] D16171: Warning on redeclaring with a conflicting asm label
On 14 January 2016 at 15:05, Zhao, Weiming wrote: > I agree what you said about different code generated with clang and GCC > generates. In this case, we should throw an error (err_late_asm_label). > > But in this example, there is no use of the function. They are just > redundant declarations and there is no actual code generated. > So I suggest we just give warnings for this case. Otherwise, it will break > existing code like some SPEC benchmarks. > > Please review my 2nd patch. > I think your second patch checks whether it's used at the time of the redeclaration, which is too early. It may be used between there and the end of the program. I expect it not to warn but not error on the testcase I posted in my previous email? To fix, you'd need to store a list of different-asm-label declarations in Sema, check it each time something is ODR-used (see the Sema::Mark<...>Referenced family of calls) to emit the error and remove it from the list. Also, when emitting a PCH or a Module, you need to serialize and deserialize this list. Nick Weiming > > > On 1/14/2016 2:28 PM, Nick Lewycky wrote: > > On 14 January 2016 at 10:38, Weiming Zhao via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> weimingz added a comment. >> >> Hi Nick, >> >> Below is a reduced code: >> t.c: >> >> static long double acoshl (long double __x) __asm__ ("" "acosh") ; // >> this is from /arm-linux-gnueabi/libc/usr/include/bits/mathcalls.h >> extern long double acoshl (long double) __asm__ ("" "__acoshl_finite") >> ; // this is from existing code >> >> GCC gives warning like: >> /tmp/t.c:2:1: warning: asm declaration ignored due to conflict with >> previous rename [-Wpragmas] >> extern long double acoshl (long double) __asm__ ("" "__acoshl_finite") ; >> > > That's the same case as in this testcase: > > void foo() __asm__("one"); > void foo() __asm__("two"); > void test() { foo(); } > > GCC emits a call to 'one' while Clang emits a call to 'two'. This is a > real bug. Please don't downgrade this to a warning. > > As an alternative, I would accept a patch which changes how clang > generates code so that it also produces a call to 'one', with a warning. It > looks like what we need to do is drop subsequent asm label declarations on > functions that already have one. > > Nick > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16175: Introduce -fsanitize-stats flag.
pcc updated this revision to Diff 44945. pcc marked an inline comment as done. pcc added a comment. - Add note about win32 function name mangling - Stop using -whole-archive to link the stats runtime http://reviews.llvm.org/D16175 Files: docs/SanitizerStats.rst docs/UsersManual.rst docs/index.rst include/clang/AST/ASTConsumer.h include/clang/Driver/CC1Options.td include/clang/Driver/Options.td include/clang/Driver/SanitizerArgs.h include/clang/Frontend/CodeGenOptions.def include/clang/Frontend/CodeGenOptions.h include/clang/Frontend/MultiplexConsumer.h lib/CodeGen/CGClass.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CodeGenAction.cpp lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h lib/CodeGen/ModuleBuilder.cpp lib/Driver/SanitizerArgs.cpp lib/Driver/ToolChains.cpp lib/Driver/Tools.cpp lib/Frontend/CompilerInvocation.cpp lib/Frontend/MultiplexConsumer.cpp lib/Sema/SemaAttr.cpp test/CodeGen/linker-option.c test/CodeGenCXX/cfi-stats.cpp test/Driver/fsanitize.c test/Driver/sanitizer-ld.c Index: test/Driver/sanitizer-ld.c === --- test/Driver/sanitizer-ld.c +++ test/Driver/sanitizer-ld.c @@ -345,6 +345,39 @@ // CHECK-SAFESTACK-LINUX: "-lpthread" // CHECK-SAFESTACK-LINUX: "-ldl" +// RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \ +// RUN: -target x86_64-unknown-linux \ +// RUN: --sysroot=%S/Inputs/basic_linux_tree \ +// RUN: | FileCheck --check-prefix=CHECK-CFI-STATS-LINUX %s +// CHECK-CFI-STATS-LINUX: "{{.*}}ld{{(.exe)?}}" +// CHECK-CFI-STATS-LINUX: "-whole-archive" "{{[^"]*}}libclang_rt.stats_client-x86_64.a" "-no-whole-archive" +// CHECK-CFI-STATS-LINUX-NOT: "-whole-archive" +// CHECK-CFI-STATS-LINUX: "{{[^"]*}}libclang_rt.stats-x86_64.a" + +// RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \ +// RUN: -target x86_64-apple-darwin \ +// RUN: --sysroot=%S/Inputs/basic_linux_tree \ +// RUN: | FileCheck --check-prefix=CHECK-CFI-STATS-DARWIN %s +// CHECK-CFI-STATS-DARWIN: "{{.*}}ld{{(.exe)?}}" +// CHECK-CFI-STATS-DARWIN: "{{[^"]*}}libclang_rt.stats_client_osx.a" +// CHECK-CFI-STATS-DARWIN: "{{[^"]*}}libclang_rt.stats_osx_dynamic.dylib" + +// RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \ +// RUN: -target x86_64-pc-windows \ +// RUN: --sysroot=%S/Inputs/basic_linux_tree \ +// RUN: | FileCheck --check-prefix=CHECK-CFI-STATS-WIN64 %s +// CHECK-CFI-STATS-WIN64: "--dependent-lib={{[^"]*}}clang_rt.stats_client-x86_64.lib" +// CHECK-CFI-STATS-WIN64: "--dependent-lib={{[^"]*}}clang_rt.stats-x86_64.lib" +// CHECK-CFI-STATS-WIN64: "--linker-option=/include:__sanitizer_stats_register" + +// RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \ +// RUN: -target i686-pc-windows \ +// RUN: --sysroot=%S/Inputs/basic_linux_tree \ +// RUN: | FileCheck --check-prefix=CHECK-CFI-STATS-WIN32 %s +// CHECK-CFI-STATS-WIN32: "--dependent-lib={{[^"]*}}clang_rt.stats_client-i386.lib" +// CHECK-CFI-STATS-WIN32: "--dependent-lib={{[^"]*}}clang_rt.stats-i386.lib" +// CHECK-CFI-STATS-WIN32: "--linker-option=/include:___sanitizer_stats_register" + // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ // RUN: -target arm-linux-androideabi -fsanitize=safe-stack \ // RUN: --sysroot=%S/Inputs/basic_android_tree \ Index: test/Driver/fsanitize.c === --- test/Driver/fsanitize.c +++ test/Driver/fsanitize.c @@ -272,6 +272,9 @@ // CHECK-CFI-NO-CROSS-DSO: -emit-llvm-bc // CHECK-CFI-NO-CROSS-DSO-NOT: -fsanitize-cfi-cross-dso +// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -fsanitize-stats -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-STATS +// CHECK-CFI-STATS: -fsanitize-stats + // RUN: %clang_cl -fsanitize=address -c -MDd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL // RUN: %clang_cl -fsanitize=address -c -MTd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL // RUN: %clang_cl -fsanitize=address -c -LDd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL Index: test/CodeGenCXX/cfi-stats.cpp === --- /dev/null +++ test/CodeGenCXX/cfi-stats.cpp @@ -0,0 +1,50 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-vcall,cfi-nvcall,cfi-derived-cast,cfi-unrelated-cast,cfi-icall -fsanitize-stats -emit-llvm -o - %s | FileCheck --check-prefix=CHECK %s + +// CHECK: [[STATS:@[^ ]*]] = private global { i8*, i32, [5 x [2 x i8*]] } { i8* null, i32 5, [5 x [2 x i8*]] +// CHECK: {{\[\[}}2 x i8*] zeroinitializer, +// CHECK: [2 x i8*] [i8* null, i8* inttoptr (i64 2305843009213693952 to i8*)], +// CHECK: [2 x i8*] [i8* null, i8* inttoptr (i64 4611686018427387904 to i8*)], +// CHECK: [2 x i8*] [i8* null, i8* inttoptr (i64 6917529027641081856 to i8
[PATCH] D16206: Make -Wdelete-non-virtual-dtor warn on explicit `a->~A()` dtor calls too.
thakis created this revision. thakis added a reviewer: rtrieu. thakis added a subscriber: cfe-commits. -Wdelete-non-virtual-dtor warns if A is a type with virtual functions but without virtual dtor has its constructor called via `delete a`. This makes the warning also fire if the dtor is called via `a->~A()`. This would've found a security bug in Chromium at compile time. Fixes PR26137. Includes fixit for silencing the warning: ``` test.cc:12:3: warning: destructor called on 'B' that is abstract but has non-virtual destructor [-Wdelete-non-virtual-dtor] b->~B(); // No warning. ^ test.cc:12:6: note: qualify call to silence this warning b->~B(); // No warning. ^ B:: ``` http://reviews.llvm.org/D16206 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaExprCXX.cpp lib/Sema/SemaOverload.cpp test/SemaCXX/destructor.cpp Index: test/SemaCXX/destructor.cpp === --- test/SemaCXX/destructor.cpp +++ test/SemaCXX/destructor.cpp @@ -257,6 +257,7 @@ } } +// FIXME: Why are these supposed to not warn? void nowarnarray() { { B* b = new B[4]; @@ -311,6 +312,14 @@ } } +void nowarn0_explicit_dtor(F* f, VB* vb, VD* vd, VF* vf) { + f->~F(); + f->~F(); + vb->~VB(); + vd->~VD(); + vf->~VF(); +} + void warn0() { { B* b = new B(); @@ -326,6 +335,17 @@ } } +void warn0_explicit_dtor(B* b, B& br, D* d) { + b->~B(); // expected-warning {{destructor called on non-final 'dnvd::B' that has virtual functions but non-virtual destructor}} expected-note{{qualify call to silence this warning}} + b->B::~B(); // No warning when the call isn't virtual. + + br.~B(); // expected-warning {{destructor called on non-final 'dnvd::B' that has virtual functions but non-virtual destructor}} expected-note{{qualify call to silence this warning}} + br.B::~B(); + + d->~D(); // expected-warning {{destructor called on non-final 'dnvd::D' that has virtual functions but non-virtual destructor}} expected-note{{qualify call to silence this warning}} + d->D::~D(); +} + void nowarn1() { { simple_ptr f(new F()); Index: include/clang/Sema/Sema.h === --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -4698,6 +4698,10 @@ ExprResult ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal, bool ArrayForm, Expr *Operand); + void CheckVirtualDtorCall(CXXDestructorDecl *dtor, SourceLocation Loc, +bool IsDelete, bool CallCanBeVirtual, +bool WarnOnNonAbstractTypes, +SourceLocation DtorLoc); DeclResult ActOnCXXConditionDeclaration(Scope *S, Declarator &D); ExprResult CheckConditionVariable(VarDecl *ConditionVar, Index: include/clang/Basic/DiagnosticSemaKinds.td === --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -5802,12 +5802,14 @@ "%0 has virtual functions but non-virtual destructor">, InGroup, DefaultIgnore; def warn_delete_non_virtual_dtor : Warning< - "delete called on non-final %0 that has virtual functions " - "but non-virtual destructor">, + "%select{delete|destructor}0 called on non-final %1 that has " + "virtual functions but non-virtual destructor">, InGroup, DefaultIgnore; +def note_delete_non_virtual : Note< + "qualify call to silence this warning">; def warn_delete_abstract_non_virtual_dtor : Warning< - "delete called on %0 that is abstract but has non-virtual destructor">, - InGroup; + "%select{delete|destructor}0 called on %1 that is abstract but has " + "non-virtual destructor">, InGroup; def warn_overloaded_virtual : Warning< "%q0 hides overloaded virtual %select{function|functions}1">, InGroup, DefaultIgnore; Index: lib/Sema/SemaOverload.cpp === --- lib/Sema/SemaOverload.cpp +++ lib/Sema/SemaOverload.cpp @@ -12236,6 +12236,17 @@ << MD->getDeclName(); } } + + if (CXXDestructorDecl *DD = + dyn_cast(TheCall->getMethodDecl())) { +// a->A::f() doesn't go through the vtable, except in AppleKext mode. +bool CallCanBeVirtual = !cast(NakedMemExpr)->hasQualifier() || +getLangOpts().AppleKext; +CheckVirtualDtorCall(DD, MemExpr->getLocStart(), /*IsDelete=*/false, + CallCanBeVirtual, /*WarnOnNonAbstractTypes=*/true, + MemExpr->getMemberLoc()); + } + return MaybeBindToTemporary(TheCall); } Index: lib/Sema/SemaExprCXX.cpp === --- lib/Sema/SemaExprCXX.cpp +++ lib/Sema/SemaExprCXX.cpp @@ -2765,30 +2765,10 @@ return ExprError(); } - // C++ [expr.d
r257853 - Make template type diffing use the new desguared iterator.
Author: rtrieu Date: Thu Jan 14 19:08:56 2016 New Revision: 257853 URL: http://llvm.org/viewvc/llvm-project?rev=257853&view=rev Log: Make template type diffing use the new desguared iterator. If available, use the canonical template argument to fill in information for template type diffing instead of attempting to special case and evaluate Expr's for the value. Since those are the values used in template instantiation, we don't have to worry about difference between our evaluator and theirs. Also move the nullptr template arguments from DiffKind::Expression to DiffKind::Declaration and allow DiffKind::Declaration to set an Expr. The only effect that should result is that a named nullptr will show up as 'ptr aka nullptr' in diagnostics. Modified: cfe/trunk/lib/AST/ASTDiagnostic.cpp cfe/trunk/test/Misc/diag-template-diffing.cpp Modified: cfe/trunk/lib/AST/ASTDiagnostic.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDiagnostic.cpp?rev=257853&r1=257852&r2=257853&view=diff == --- cfe/trunk/lib/AST/ASTDiagnostic.cpp (original) +++ cfe/trunk/lib/AST/ASTDiagnostic.cpp Thu Jan 14 19:08:56 2016 @@ -595,14 +595,12 @@ class TemplateDiff { SetDefault(FromDefault, ToDefault); } -void SetExpressionDiff(Expr *FromExpr, Expr *ToExpr, bool IsFromNullPtr, - bool IsToNullPtr, bool FromDefault, bool ToDefault) { +void SetExpressionDiff(Expr *FromExpr, Expr *ToExpr, bool FromDefault, + bool ToDefault) { assert(FlatTree[CurrentNode].Kind == Invalid && "Node is not empty."); FlatTree[CurrentNode].Kind = Expression; FlatTree[CurrentNode].FromArgInfo.ArgExpr = FromExpr; FlatTree[CurrentNode].ToArgInfo.ArgExpr = ToExpr; - FlatTree[CurrentNode].FromArgInfo.IsNullPtr = IsFromNullPtr; - FlatTree[CurrentNode].ToArgInfo.IsNullPtr = IsToNullPtr; SetDefault(FromDefault, ToDefault); } @@ -632,8 +630,8 @@ class TemplateDiff { void SetDeclarationDiff(ValueDecl *FromValueDecl, ValueDecl *ToValueDecl, bool FromAddressOf, bool ToAddressOf, -bool FromNullPtr, bool ToNullPtr, bool FromDefault, -bool ToDefault) { +bool FromNullPtr, bool ToNullPtr, Expr *FromExpr, +Expr *ToExpr, bool FromDefault, bool ToDefault) { assert(FlatTree[CurrentNode].Kind == Invalid && "Node is not empty."); FlatTree[CurrentNode].Kind = Declaration; FlatTree[CurrentNode].FromArgInfo.VD = FromValueDecl; @@ -642,6 +640,8 @@ class TemplateDiff { FlatTree[CurrentNode].ToArgInfo.NeedAddressOf = ToAddressOf; FlatTree[CurrentNode].FromArgInfo.IsNullPtr = FromNullPtr; FlatTree[CurrentNode].ToArgInfo.IsNullPtr = ToNullPtr; + FlatTree[CurrentNode].FromArgInfo.ArgExpr = FromExpr; + FlatTree[CurrentNode].ToArgInfo.ArgExpr = ToExpr; SetDefault(FromDefault, ToDefault); } @@ -716,13 +716,10 @@ class TemplateDiff { ToType = FlatTree[ReadNode].ToArgInfo.ArgType; } -void GetExpressionDiff(Expr *&FromExpr, Expr *&ToExpr, bool &FromNullPtr, - bool &ToNullPtr) { +void GetExpressionDiff(Expr *&FromExpr, Expr *&ToExpr) { assert(FlatTree[ReadNode].Kind == Expression && "Unexpected kind"); FromExpr = FlatTree[ReadNode].FromArgInfo.ArgExpr; ToExpr = FlatTree[ReadNode].ToArgInfo.ArgExpr; - FromNullPtr = FlatTree[ReadNode].FromArgInfo.IsNullPtr; - ToNullPtr = FlatTree[ReadNode].ToArgInfo.IsNullPtr; } void GetTemplateTemplateDiff(TemplateDecl *&FromTD, TemplateDecl *&ToTD) { @@ -745,7 +742,8 @@ class TemplateDiff { void GetDeclarationDiff(ValueDecl *&FromValueDecl, ValueDecl *&ToValueDecl, bool &FromAddressOf, bool &ToAddressOf, -bool &FromNullPtr, bool &ToNullPtr) { +bool &FromNullPtr, bool &ToNullPtr, Expr *&FromExpr, +Expr *&ToExpr) { assert(FlatTree[ReadNode].Kind == Declaration && "Unexpected kind."); FromValueDecl = FlatTree[ReadNode].FromArgInfo.VD; ToValueDecl = FlatTree[ReadNode].ToArgInfo.VD; @@ -753,6 +751,8 @@ class TemplateDiff { ToAddressOf = FlatTree[ReadNode].ToArgInfo.NeedAddressOf; FromNullPtr = FlatTree[ReadNode].FromArgInfo.IsNullPtr; ToNullPtr = FlatTree[ReadNode].ToArgInfo.IsNullPtr; + FromExpr = FlatTree[ReadNode].FromArgInfo.ArgExpr; + ToExpr = FlatTree[ReadNode].ToArgInfo.ArgExpr; } /// FromDefault - Return true if the from argument is the default. @@ -1007,11 +1007,9 @@ class TemplateDiff { } /// DiffTypes - Fills a DiffNode with information about a type difference. - void DiffTypes(const TSTiterator &FromIter, const TSTiterator &ToIter, -
Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.
hokein updated this revision to Diff 44951. hokein added a comment. Address review comments. http://reviews.llvm.org/D16113 Files: clang-tidy/ClangTidy.cpp clang-tidy/ClangTidy.h clang-tidy/google/GlobalNamesInHeadersCheck.cpp clang-tidy/google/GlobalNamesInHeadersCheck.h clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp clang-tidy/google/UnnamedNamespaceInHeaderCheck.h clang-tidy/misc/DefinitionsInHeadersCheck.cpp clang-tidy/misc/DefinitionsInHeadersCheck.h clang-tidy/utils/CMakeLists.txt clang-tidy/utils/HeaderFileExtensionsUtils.cpp clang-tidy/utils/HeaderFileExtensionsUtils.h Index: clang-tidy/utils/HeaderFileExtensionsUtils.h === --- /dev/null +++ clang-tidy/utils/HeaderFileExtensionsUtils.h @@ -0,0 +1,54 @@ +//===--- HeaderFileExtensionsUtils.h - clang-tidy*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_FILE_EXTENSIONS_UTILS_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_FILE_EXTENSIONS_UTILS_H + +#include + +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/ADT/SmallSet.h" +#include "llvm/Support/Path.h" + +namespace clang { +namespace tidy { +namespace header_file_extensions_utils { + +typedef llvm::SmallSet HeaderFileExtensionsSet; + +/// \brief Checks whether expansion location of Loc is in header file. +bool isExpansionLocInHeaderFile( +SourceLocation Loc, +const SourceManager &SM, +const HeaderFileExtensionsSet &HeaderFileExtensions); + +/// \brief Checks whether presumed location of Loc is in header file. +bool isPresumedLocInHeaderFile( +SourceLocation Loc, +SourceManager &SM, +const HeaderFileExtensionsSet &HeaderFileExtensions); + +/// \brief Checks whether spelling location of Loc is in header file. +bool isSpellingLocInHeaderFile( +SourceLocation Loc, +SourceManager &SM, +const HeaderFileExtensionsSet &HeaderFileExtensions); + +/// \brief Parses header file extensions from a comma-separated list. +bool parseHeaderFileExtensions(llvm::StringRef AllHeaderFileExtensions, + HeaderFileExtensionsSet &HeaderFileExtensions); + + +} // namespace header_file_extensions_utils +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_FILE_EXTENSIONS_UTILS_H Index: clang-tidy/utils/HeaderFileExtensionsUtils.cpp === --- /dev/null +++ clang-tidy/utils/HeaderFileExtensionsUtils.cpp @@ -0,0 +1,67 @@ +//===--- HeaderFileExtensionsUtils.h - clang-tidy*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "HeaderFileExtensionsUtils.h" +#include "clang/Basic/CharInfo.h" + +namespace clang { +namespace tidy { +namespace header_file_extensions_utils { + +bool isExpansionLocInHeaderFile( +SourceLocation Loc, +const SourceManager &SM, +const HeaderFileExtensionsSet &HeaderFileExtensions) { + SourceLocation ExpansionLoc = SM.getExpansionLoc(Loc); + StringRef FileExtension = llvm::sys::path::extension( + SM.getFilename(ExpansionLoc)); + return HeaderFileExtensions.count(FileExtension.substr(1)) > 0; +} + +bool isPresumedLocInHeaderFile( +SourceLocation Loc, +SourceManager &SM, +const HeaderFileExtensionsSet &HeaderFileExtensions) { + PresumedLoc PresumedLocation = SM.getPresumedLoc(Loc); + StringRef FileExtension = llvm::sys::path::extension( + PresumedLocation.getFilename()); + return HeaderFileExtensions.count(FileExtension.substr(1)) > 0; +} + +bool isSpellingLocInHeaderFile( +SourceLocation Loc, +SourceManager &SM, +const HeaderFileExtensionsSet &HeaderFileExtensions) { + SourceLocation SpellingLoc = SM.getSpellingLoc(Loc); + StringRef FileExtension = llvm::sys::path::extension( + SM.getFilename(SpellingLoc)); + + return HeaderFileExtensions.count(FileExtension.substr(1)) > 0; +} + +bool parseHeaderFileExtensions(llvm::StringRef AllHeaderFileExtensions, + HeaderFileExtensionsSet &HeaderFileExtensions) { + SmallVector Suffixes; + AllHeaderFileExtensions.split(Suffixes, ','); + HeaderFileExtensions.clear(); + for (llvm::StringRef Suffix : Suffixes) { +llvm::StringRef Extension = Suffix.trim(); +for (llvm::StringRef::const_iterator it = Extension.begin(); + it != Extension.end(); ++it) { + if (!
Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.
hokein marked 11 inline comments as done. Comment at: clang-tidy/ClangTidy.h:65 @@ -56,3 +64,3 @@ /// \c CheckOptions. If the corresponding key is not present, returns /// \p Default. template Makes sense. I just refer from `std::string get(StringRef LocalName, std::string Default) const`. Also change `get` to use `StringRef`. Comment at: clang-tidy/tool/ClangTidyMain.cpp:78 @@ +77,3 @@ +static cl::opt +HeaderFileExtensions("header-file-extensions", + cl::desc("File extensions that regard as header file.\n" aaron.ballman wrote: > alexfh wrote: > > hokein wrote: > > > alexfh wrote: > > > > We don't need a command-line flag for this. The regular way to > > > > configure clang-tidy is .clang-tidy files. However, if needed, check > > > > options can be configured from the command line via the `-config=` > > > > option. > > > From my understanding, it is a global option for header-file-extensions, > > > right? If we remove this command-line flag, there is only local > > > `header-file-extensions` option remained for particular checks. > > Both local and global options reside in `CheckOptions` and can be > > configured in the same way using either a .clang-tidy file or the -config= > > command-line option. The only difference between local and global options > > is that the name of the latter is not prefixed with the "CheckName.". Makes > > sense? > > Both local and global options reside in CheckOptions and can be configured > > in the same way using either a .clang-tidy file or the -config= > > command-line option. The only difference between local and global options > > is that the name of the latter is not prefixed with the "CheckName.". Makes > > sense? > > Ah, thank you for that explanation. I learned something new today. ;-) I see it. http://reviews.llvm.org/D16113 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16206: Make -Wdelete-non-virtual-dtor warn on explicit `a->~A()` dtor calls too.
dblaikie added a subscriber: dblaikie. dblaikie accepted this revision. dblaikie added a reviewer: dblaikie. dblaikie added a comment. This revision is now accepted and ready to land. Looks good - thanks! http://reviews.llvm.org/D16206 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r257861 - Save the integer type for integral template arguments.
Author: rtrieu Date: Thu Jan 14 20:55:17 2016 New Revision: 257861 URL: http://llvm.org/viewvc/llvm-project?rev=257861&view=rev Log: Save the integer type for integral template arguments. Save the integer type when diffing integers in template type diffing. When integers are different sizes, print out the type along with the integer value. Also with the type information, print true and false instead of 1 and 0 for boolean values. Modified: cfe/trunk/lib/AST/ASTDiagnostic.cpp cfe/trunk/test/Misc/diag-template-diffing.cpp Modified: cfe/trunk/lib/AST/ASTDiagnostic.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDiagnostic.cpp?rev=257861&r1=257860&r2=257861&view=diff == --- cfe/trunk/lib/AST/ASTDiagnostic.cpp (original) +++ cfe/trunk/lib/AST/ASTDiagnostic.cpp Thu Jan 14 20:55:17 2016 @@ -615,6 +615,7 @@ class TemplateDiff { void SetIntegerDiff(llvm::APSInt FromInt, llvm::APSInt ToInt, bool IsValidFromInt, bool IsValidToInt, +QualType FromIntType, QualType ToIntType, Expr *FromExpr, Expr *ToExpr, bool FromDefault, bool ToDefault) { assert(FlatTree[CurrentNode].Kind == Invalid && "Node is not empty."); @@ -623,6 +624,8 @@ class TemplateDiff { FlatTree[CurrentNode].ToArgInfo.Val = ToInt; FlatTree[CurrentNode].FromArgInfo.IsValidInt = IsValidFromInt; FlatTree[CurrentNode].ToArgInfo.IsValidInt = IsValidToInt; + FlatTree[CurrentNode].FromArgInfo.ArgType = FromIntType; + FlatTree[CurrentNode].ToArgInfo.ArgType = ToIntType; FlatTree[CurrentNode].FromArgInfo.ArgExpr = FromExpr; FlatTree[CurrentNode].ToArgInfo.ArgExpr = ToExpr; SetDefault(FromDefault, ToDefault); @@ -730,12 +733,15 @@ class TemplateDiff { void GetIntegerDiff(llvm::APSInt &FromInt, llvm::APSInt &ToInt, bool &IsValidFromInt, bool &IsValidToInt, +QualType &FromIntType, QualType &ToIntType, Expr *&FromExpr, Expr *&ToExpr) { assert(FlatTree[ReadNode].Kind == Integer && "Unexpected kind."); FromInt = FlatTree[ReadNode].FromArgInfo.Val; ToInt = FlatTree[ReadNode].ToArgInfo.Val; IsValidFromInt = FlatTree[ReadNode].FromArgInfo.IsValidInt; IsValidToInt = FlatTree[ReadNode].ToArgInfo.IsValidInt; + FromIntType = FlatTree[ReadNode].FromArgInfo.ArgType; + ToIntType = FlatTree[ReadNode].ToArgInfo.ArgType; FromExpr = FlatTree[ReadNode].FromArgInfo.ArgExpr; ToExpr = FlatTree[ReadNode].ToArgInfo.ArgExpr; } @@ -1047,10 +1053,13 @@ class TemplateDiff { } /// InitializeNonTypeDiffVariables - Helper function for DiffNonTypes - static void InitializeNonTypeDiffVariables( - ASTContext &Context, const TSTiterator &Iter, - NonTypeTemplateParmDecl *Default, llvm::APSInt &Value, bool &HasInt, - bool &IsNullPtr, Expr *&E, ValueDecl *&VD, bool &NeedAddressOf) { + static void InitializeNonTypeDiffVariables(ASTContext &Context, + const TSTiterator &Iter, + NonTypeTemplateParmDecl *Default, + llvm::APSInt &Value, bool &HasInt, + QualType &IntType, bool &IsNullPtr, + Expr *&E, ValueDecl *&VD, + bool &NeedAddressOf) { if (!Iter.isEnd()) { switch (Iter->getKind()) { default: @@ -1058,6 +1067,7 @@ class TemplateDiff { case TemplateArgument::Integral: Value = Iter->getAsIntegral(); HasInt = true; + IntType = Iter->getIntegralType(); return; case TemplateArgument::Declaration: { VD = Iter->getAsDecl(); @@ -1087,6 +1097,7 @@ class TemplateDiff { case TemplateArgument::Integral: Value = TA.getAsIntegral(); HasInt = true; +IntType = TA.getIntegralType(); return; case TemplateArgument::Declaration: { VD = TA.getAsDecl(); @@ -1117,15 +1128,16 @@ class TemplateDiff { NonTypeTemplateParmDecl *ToDefaultNonTypeDecl) { Expr *FromExpr = nullptr, *ToExpr = nullptr; llvm::APSInt FromInt, ToInt; +QualType FromIntType, ToIntType; ValueDecl *FromValueDecl = nullptr, *ToValueDecl = nullptr; bool HasFromInt = false, HasToInt = false, FromNullPtr = false, ToNullPtr = false, NeedFromAddressOf = false, NeedToAddressOf = false; -InitializeNonTypeDiffVariables(Context, FromIter, FromDefaultNonTypeDecl, - FromInt, HasFromInt, FromNullPtr, FromExpr, - FromValueDecl, NeedFromAddressOf); +InitializeNonTypeDiffVariables( +Context, FromIter,
Re: [PATCH] D15866: Warn when using `defined` in a macro definition.
thakis updated this revision to Diff 44958. thakis added a comment. For function-type macros, make the warning only show up with -pedantic http://reviews.llvm.org/D15866 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticLexKinds.td lib/Lex/PPExpressions.cpp test/Lexer/cxx-features.cpp test/Preprocessor/expr_define_expansion.c Index: test/Lexer/cxx-features.cpp === --- test/Lexer/cxx-features.cpp +++ test/Lexer/cxx-features.cpp @@ -6,6 +6,7 @@ // expected-no-diagnostics +// FIXME using `defined` in a macro has undefined behavior. #if __cplusplus < 201103L #define check(macro, cxx98, cxx11, cxx1y) cxx98 == 0 ? defined(__cpp_##macro) : __cpp_##macro != cxx98 #elif __cplusplus < 201304L Index: lib/Lex/PPExpressions.cpp === --- lib/Lex/PPExpressions.cpp +++ lib/Lex/PPExpressions.cpp @@ -82,6 +82,52 @@ static bool EvaluateDefined(PPValue &Result, Token &PeekTok, DefinedTracker &DT, bool ValueLive, Preprocessor &PP) { SourceLocation beginLoc(PeekTok.getLocation()); + + // [cpp.cond]p4: + // Prior to evaluation, macro invocations in the list of preprocessing + // tokens that will become the controlling constant expression are replaced + // (except for those macro names modified by the 'defined' unary operator), + // just as in normal text. If the token 'defined' is generated as a result + // of this replacement process or use of the 'defined' unary operator does + // not match one of the two specified forms prior to macro replacement, the + // behavior is undefined. + // This isn't an idle threat, consider this program: + // #define FOO + // #define BAR defined(FOO) + // #if BAR + // ... + // #else + // ... + // #endif + // clang and gcc will pick the #if branch while Visual Studio will take the + // #else branch. Emit a warning about this undefined behavior. + if (beginLoc.isMacroID()) { +bool IsFunctionTypeMacro = +PP.getSourceManager() +.getSLocEntry(PP.getSourceManager().getFileID(beginLoc)) +.getExpansion() +.isFunctionMacroExpansion(); +// For object-type macros, it's easy to replace +// #define FOO defined(BAR) +// with +// #if defined(BAR) +// #define FOO 1 +// #else +// #define FOO 0 +// #endif +// and doing so makes sense since compilers handle this differently in +// practice (see example further up). But for function-type macros, +// there is no good way to write +// # define FOO(x) (defined(M_ ## x) && M_ ## x) +// in a different way, and compilers seem to agree on how to behave here. +// So warn by default on object-type macros, but only warn in -pedantic +// mode on function-type macros. +if (IsFunctionTypeMacro) + PP.Diag(beginLoc, diag::warn_defined_in_function_type_macro); +else + PP.Diag(beginLoc, diag::warn_defined_in_object_type_macro); + } + Result.setBegin(beginLoc); // Get the next token, don't expand it. @@ -177,8 +223,8 @@ if (IdentifierInfo *II = PeekTok.getIdentifierInfo()) { // Handle "defined X" and "defined(X)". if (II->isStr("defined")) - return(EvaluateDefined(Result, PeekTok, DT, ValueLive, PP)); - + return EvaluateDefined(Result, PeekTok, DT, ValueLive, PP); + // If this identifier isn't 'defined' or one of the special // preprocessor keywords and it wasn't macro expanded, it turns // into a simple 0, unless it is the C++ keyword "true", in which case it Index: include/clang/Basic/DiagnosticLexKinds.td === --- include/clang/Basic/DiagnosticLexKinds.td +++ include/clang/Basic/DiagnosticLexKinds.td @@ -658,6 +658,13 @@ def note_header_guard : Note< "%0 is defined here; did you mean %1?">; +def warn_defined_in_object_type_macro : Warning< + "macro expansion producing 'defined' has undefined behavior">, + InGroup; +def warn_defined_in_function_type_macro : Extension< + "macro expansion producing 'defined' has undefined behavior">, + InGroup; + let CategoryName = "Nullability Issue" in { def err_pp_assume_nonnull_syntax : Error<"expected 'begin' or 'end'">; Index: include/clang/Basic/DiagnosticGroups.td === --- include/clang/Basic/DiagnosticGroups.td +++ include/clang/Basic/DiagnosticGroups.td @@ -204,6 +204,7 @@ def DanglingElse: DiagGroup<"dangling-else">; def DanglingField : DiagGroup<"dangling-field">; def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">; +def ExpansionToUndefined : DiagGroup<"expansion-to-undefined">; def FlagEnum : DiagGroup<"flag-enum">; def IncrementBool : DiagGroup<"increment-bool", [DeprecatedIncrementBool]>; def InfiniteRecursion : DiagGroup<"infin