Re: [PATCH] D18347: [PATCH] Fix thread_annotation negtest for thread safety.
jamesr added a comment. In http://reviews.llvm.org/D18347#380792, @EricWF wrote: > And looking at the current state of the single-threaded test suite this isn't > the only test that needs this fix applied. > > http://lab.llvm.org:8011/builders/libcxx-libcxxabi-singlethreaded-x86_64-linux-debian/builds/869 Indeed! I assumed by reading other nearby tests that there was some mechanism that disabled the thread/ tests when threads were disabled, but didn't verify this. Sorry about the breakage. http://reviews.llvm.org/D18347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision
danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:84 @@ +83,3 @@ +// Can E value be greater or equal than Val? +static bool canBeGreaterEqual(CheckerContext &C, const Expr *E, + unsigned long long Val) { zaks.anna wrote: > danielmarjamaki wrote: > > zaks.anna wrote: > > > This function returns true if the value "is" greater or equal, not "can > > > be" greater or equal. The latter would be "return StGE". > > > > > > Also, it's slightly better to return the StGE state and use it to report > > > the bug. This way, our assumption is explicitly recorded in the error > > > state. > > NoQ made the same comment. I disagree. > > > > int A = 0; > > if (X) { > > A = 1000; > > } > > U8 = A; // <- Imho; A _can_ be 1000 > > > > Imho it's better to say that A _can_ be 1000 unless A is 1000 for all > > possible execution paths through the code. > > > > Do you still think "is" is better than "can be"? > The Clang Static Analyzer performs path sensitive analysis of the program. > (It does not merge the paths at the "U8 = A" statement!!!) You will only be > changing the state along a single execution path of this program. Along that > path, A will always be 1000. > > When analyzing your example, the analyzer is going to separately analyze 2 > paths: > 1st path: A=0; X != 0; A =1000; U8 = A; // Here U8 is definitely 1000. > 2d path: A=0; X == 0; U8 = A; // Here U8 is definitely 0. > > This video contains an intuitive explanation of symbolic execution technique > we use: http://llvm.org/devmtg/2012-11/videos/Zaks-Rose-Checker24Hours.mp4 I understand that and I still think that value of A "can be" 1000. Yes in that path the value "is" 1000. But as far as I see, you and others disagree with me. And therefore I will change to "is". http://reviews.llvm.org/D13126 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r264146 - Add check for unneeded copies of locals
Author: hokein Date: Wed Mar 23 04:33:07 2016 New Revision: 264146 URL: http://llvm.org/viewvc/llvm-project?rev=264146&view=rev Log: Add check for unneeded copies of locals Summary: Extends the UnnecessaryCopyInitialization to detect copies of local variables and parameters that are unneeded. Patch by Matt Kulukundis! Reviewers: alexfh, hokein Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D18149 Modified: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization.cpp Modified: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp?rev=264146&r1=264145&r2=264146&view=diff == --- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp Wed Mar 23 04:33:07 2016 @@ -16,16 +16,32 @@ namespace clang { namespace tidy { namespace performance { +namespace { + +void recordFixes(const VarDecl &Var, ASTContext &Context, + DiagnosticBuilder &Diagnostic) { + // Do not propose fixes in macros since we cannot place them correctly. + if (Var.getLocation().isMacroID()) +return; + + Diagnostic << utils::create_fix_it::changeVarDeclToReference(Var, Context); + if (!Var.getType().isLocalConstQualified()) +Diagnostic << utils::create_fix_it::changeVarDeclToConst(Var); +} + +} // namespace + using namespace ::clang::ast_matchers; +using decl_ref_expr_utils::isOnlyUsedAsConst; -void UnnecessaryCopyInitialization::registerMatchers( -ast_matchers::MatchFinder *Finder) { +void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) { auto ConstReference = referenceType(pointee(qualType(isConstQualified(; auto ConstOrConstReference = allOf(anyOf(ConstReference, isConstQualified()), unless(allOf(pointerType(), unless(pointerType(pointee( qualType(isConstQualified(; + // Match method call expressions where the this argument is a const // type or const reference. This returned const reference is highly likely to // outlive the local const reference of the variable being declared. @@ -37,45 +53,82 @@ void UnnecessaryCopyInitialization::regi auto ConstRefReturningFunctionCall = callExpr(callee(functionDecl(returns(ConstReference))), unless(callee(cxxMethodDecl(; + + auto localVarCopiedFrom = [](const internal::Matcher &CopyCtorArg) { +return compoundStmt( + forEachDescendant( + varDecl(hasLocalStorage(), + hasType(matchers::isExpensiveToCopy()), + hasInitializer(cxxConstructExpr( + hasDeclaration(cxxConstructorDecl( + isCopyConstructor())), + hasArgument(0, CopyCtorArg)) + .bind("ctorCall"))) + .bind("newVarDecl"))) +.bind("blockStmt"); + }; + Finder->addMatcher( - compoundStmt( - forEachDescendant( - varDecl( - hasLocalStorage(), hasType(matchers::isExpensiveToCopy()), - hasInitializer(cxxConstructExpr( - hasDeclaration(cxxConstructorDecl(isCopyConstructor())), - hasArgument( - 0, anyOf(ConstRefReturningFunctionCall, - ConstRefReturningMethodCallOfConstParam) - .bind("varDecl"))).bind("blockStmt"), + localVarCopiedFrom(anyOf(ConstRefReturningFunctionCall, + ConstRefReturningMethodCallOfConstParam)), this); + + Finder->addMatcher(localVarCopiedFrom(declRefExpr( + to(varDecl(hasLocalStorage()).bind("oldVarDecl", + this); } void UnnecessaryCopyInitialization::check( -const ast_matchers::MatchFinder::MatchResult &Result) { - const auto *Var = Result.Nodes.getNodeAs("varDecl"); +const MatchFinder::MatchResult &Result) { + const auto *NewVar = Result.Nodes.getNodeAs("newVarDecl"); + const auto *OldVar = Result.Nodes.getNodeAs("oldVarDecl"); const auto *BlockStmt = Result.Nodes.getNodeAs("blockStmt"); - bool IsConstQualified = Var->getType().isConstQualified(); - if (!IsConstQualified && - !decl_ref_expr_u
Re: [PATCH] D18149: Add check for unneeded copies of locals
This revision was automatically updated to reflect the committed changes. Closed by commit rL264146: Add check for unneeded copies of locals (authored by hokein). Changed prior to commit: http://reviews.llvm.org/D18149?vs=51289&id=51391#toc Repository: rL LLVM http://reviews.llvm.org/D18149 Files: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization.cpp Index: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp === --- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -16,16 +16,32 @@ namespace clang { namespace tidy { namespace performance { +namespace { + +void recordFixes(const VarDecl &Var, ASTContext &Context, + DiagnosticBuilder &Diagnostic) { + // Do not propose fixes in macros since we cannot place them correctly. + if (Var.getLocation().isMacroID()) +return; + + Diagnostic << utils::create_fix_it::changeVarDeclToReference(Var, Context); + if (!Var.getType().isLocalConstQualified()) +Diagnostic << utils::create_fix_it::changeVarDeclToConst(Var); +} + +} // namespace + using namespace ::clang::ast_matchers; +using decl_ref_expr_utils::isOnlyUsedAsConst; -void UnnecessaryCopyInitialization::registerMatchers( -ast_matchers::MatchFinder *Finder) { +void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) { auto ConstReference = referenceType(pointee(qualType(isConstQualified(; auto ConstOrConstReference = allOf(anyOf(ConstReference, isConstQualified()), unless(allOf(pointerType(), unless(pointerType(pointee( qualType(isConstQualified(; + // Match method call expressions where the this argument is a const // type or const reference. This returned const reference is highly likely to // outlive the local const reference of the variable being declared. @@ -37,45 +53,82 @@ auto ConstRefReturningFunctionCall = callExpr(callee(functionDecl(returns(ConstReference))), unless(callee(cxxMethodDecl(; + + auto localVarCopiedFrom = [](const internal::Matcher &CopyCtorArg) { +return compoundStmt( + forEachDescendant( + varDecl(hasLocalStorage(), + hasType(matchers::isExpensiveToCopy()), + hasInitializer(cxxConstructExpr( + hasDeclaration(cxxConstructorDecl( + isCopyConstructor())), + hasArgument(0, CopyCtorArg)) + .bind("ctorCall"))) + .bind("newVarDecl"))) +.bind("blockStmt"); + }; + Finder->addMatcher( - compoundStmt( - forEachDescendant( - varDecl( - hasLocalStorage(), hasType(matchers::isExpensiveToCopy()), - hasInitializer(cxxConstructExpr( - hasDeclaration(cxxConstructorDecl(isCopyConstructor())), - hasArgument( - 0, anyOf(ConstRefReturningFunctionCall, - ConstRefReturningMethodCallOfConstParam) - .bind("varDecl"))).bind("blockStmt"), + localVarCopiedFrom(anyOf(ConstRefReturningFunctionCall, + ConstRefReturningMethodCallOfConstParam)), this); + + Finder->addMatcher(localVarCopiedFrom(declRefExpr( + to(varDecl(hasLocalStorage()).bind("oldVarDecl", + this); } void UnnecessaryCopyInitialization::check( -const ast_matchers::MatchFinder::MatchResult &Result) { - const auto *Var = Result.Nodes.getNodeAs("varDecl"); +const MatchFinder::MatchResult &Result) { + const auto *NewVar = Result.Nodes.getNodeAs("newVarDecl"); + const auto *OldVar = Result.Nodes.getNodeAs("oldVarDecl"); const auto *BlockStmt = Result.Nodes.getNodeAs("blockStmt"); - bool IsConstQualified = Var->getType().isConstQualified(); - if (!IsConstQualified && - !decl_ref_expr_utils::isOnlyUsedAsConst(*Var, *BlockStmt, - *Result.Context)) + const auto *CtorCall = Result.Nodes.getNodeAs("ctorCall"); + + // A constructor that looks like T(const T& t, bool arg = false) counts as a + // copy only when it is called with default arguments for the arguments after + // the first. + for (unsigned int i = 1; i < CtorCall->getNumArgs();
Re: [PATCH] D18149: Add check for unneeded copies of locals
hokein added a comment. @fowles, I have commit the patch for you. Repository: rL LLVM http://reviews.llvm.org/D18149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision
danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:84 @@ +83,3 @@ +// Can E value be greater or equal than Val? +static bool canBeGreaterEqual(CheckerContext &C, const Expr *E, + unsigned long long Val) { danielmarjamaki wrote: > zaks.anna wrote: > > danielmarjamaki wrote: > > > zaks.anna wrote: > > > > This function returns true if the value "is" greater or equal, not "can > > > > be" greater or equal. The latter would be "return StGE". > > > > > > > > Also, it's slightly better to return the StGE state and use it to > > > > report the bug. This way, our assumption is explicitly recorded in the > > > > error state. > > > NoQ made the same comment. I disagree. > > > > > > int A = 0; > > > if (X) { > > > A = 1000; > > > } > > > U8 = A; // <- Imho; A _can_ be 1000 > > > > > > Imho it's better to say that A _can_ be 1000 unless A is 1000 for all > > > possible execution paths through the code. > > > > > > Do you still think "is" is better than "can be"? > > The Clang Static Analyzer performs path sensitive analysis of the program. > > (It does not merge the paths at the "U8 = A" statement!!!) You will only be > > changing the state along a single execution path of this program. Along > > that path, A will always be 1000. > > > > When analyzing your example, the analyzer is going to separately analyze 2 > > paths: > > 1st path: A=0; X != 0; A =1000; U8 = A; // Here U8 is definitely 1000. > > 2d path: A=0; X == 0; U8 = A; // Here U8 is definitely 0. > > > > This video contains an intuitive explanation of symbolic execution > > technique we use: > > http://llvm.org/devmtg/2012-11/videos/Zaks-Rose-Checker24Hours.mp4 > I understand that and I still think that value of A "can be" 1000. Yes in > that path the value "is" 1000. > > But as far as I see, you and others disagree with me. And therefore I will > change to "is". For your information in Cppcheck I say that a value is "possible" if some path(s) generates that value. And "always" when all paths generate that value. Code example: int f(int x) { int a = 1000; int b = 0; if (x == 500) a = 3; return a + b - x; } Debug output (cppcheck --debug-normal file.c): ##Value flow Line 3 1000 always 1000 Line 4 0 always 0 Line 5 x possible 500 == possible 1 500 always 500 Line 6 3 always 3 Line 7 a possible {1000,3} + possible {1000,3} b always 0 x possible 500 For me personally it is confusing to say that A "is" 1000. That is different to how I normally think of it in Cppcheck. http://reviews.llvm.org/D13126 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r264149 - [X86] Add "x87" in x86 target feature map.
Author: aturetsk Date: Wed Mar 23 06:15:10 2016 New Revision: 264149 URL: http://llvm.org/viewvc/llvm-project?rev=264149&view=rev Log: [X86] Add "x87" in x86 target feature map. Differential Revision: http://reviews.llvm.org/D13980 Added: cfe/trunk/test/CodeGen/attr-target-x87-softfp.c (with props) Modified: cfe/trunk/lib/Basic/Targets.cpp cfe/trunk/test/CodeGen/attr-target-x86-mmx.c cfe/trunk/test/CodeGen/attr-target-x86.c Modified: cfe/trunk/lib/Basic/Targets.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=264149&r1=264148&r2=264149&view=diff == --- cfe/trunk/lib/Basic/Targets.cpp (original) +++ cfe/trunk/lib/Basic/Targets.cpp Wed Mar 23 06:15:10 2016 @@ -2585,6 +2585,9 @@ bool X86TargetInfo::initFeatureMap( if (getTriple().getArch() == llvm::Triple::x86_64) setFeatureEnabledImpl(Features, "sse2", true); + // Enable X87 for all X86 processors. + setFeatureEnabledImpl(Features, "x87", true); + switch (getCPUKind(CPU)) { case CK_Generic: case CK_i386: Modified: cfe/trunk/test/CodeGen/attr-target-x86-mmx.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/attr-target-x86-mmx.c?rev=264149&r1=264148&r2=264149&view=diff == --- cfe/trunk/test/CodeGen/attr-target-x86-mmx.c (original) +++ cfe/trunk/test/CodeGen/attr-target-x86-mmx.c Wed Mar 23 06:15:10 2016 @@ -19,4 +19,4 @@ void __attribute__((target("sse"))) shif _mm_srai_pi32(a, c); } -// CHECK: "target-features"="+mmx,+sse" +// CHECK: "target-features"="+mmx,+sse,+x87" Modified: cfe/trunk/test/CodeGen/attr-target-x86.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/attr-target-x86.c?rev=264149&r1=264148&r2=264149&view=diff == --- cfe/trunk/test/CodeGen/attr-target-x86.c (original) +++ cfe/trunk/test/CodeGen/attr-target-x86.c Wed Mar 23 06:15:10 2016 @@ -31,9 +31,9 @@ int __attribute__((target("no-mmx"))) qq // CHECK: qux{{.*}} #1 // CHECK: qax{{.*}} #4 // CHECK: qq{{.*}} #5 -// CHECK: #0 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2" -// CHECK: #1 = {{.*}}"target-cpu"="ivybridge" "target-features"="+aes,+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+xsave,+xsaveopt" -// CHECK: #2 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,-aes,-avx,-avx2,-avx512bw,-avx512cd,-avx512dq,-avx512er,-avx512f,-avx512ifma,-avx512pf,-avx512vbmi,-avx512vl,-f16c,-fma,-fma4,-pclmul,-sha,-sse2,-sse3,-sse4.1,-sse4.2,-sse4a,-ssse3,-xop,-xsave,-xsaveopt" -// CHECK: #3 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3" -// CHECK: #4 = {{.*}}"target-cpu"="ivybridge" "target-features"="+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+xsave,+xsaveopt,-aes" -// CHECK: #5 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+sse,+sse2,-3dnow,-3dnowa,-mmx" +// CHECK: #0 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" +// CHECK: #1 = {{.*}}"target-cpu"="ivybridge" "target-features"="+aes,+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave,+xsaveopt" +// CHECK: #2 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+x87,-aes,-avx,-avx2,-avx512bw,-avx512cd,-avx512dq,-avx512er,-avx512f,-avx512ifma,-avx512pf,-avx512vbmi,-avx512vl,-f16c,-fma,-fma4,-pclmul,-sha,-sse2,-sse3,-sse4.1,-sse4.2,-sse4a,-ssse3,-xop,-xsave,-xsaveopt" +// CHECK: #3 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87" +// CHECK: #4 = {{.*}}"target-cpu"="ivybridge" "target-features"="+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave,+xsaveopt,-aes" +// CHECK: #5 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+sse,+sse2,+x87,-3dnow,-3dnowa,-mmx" Added: cfe/trunk/test/CodeGen/attr-target-x87-softfp.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/attr-target-x87-softfp.c?rev=264149&view=auto == --- cfe/trunk/test/CodeGen/attr-target-x87-softfp.c (added) +++ cfe/trunk/test/CodeGen/attr-target-x87-softfp.c Wed Mar 23 06:15:10 2016 @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -triple x86_64-linux-gnu -target-cpu x86-64 -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=HARD +// RUN: %clang_cc1 -msoft-float -triple x86_64-linux-gnu -target-cpu x86-64 -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=SOFT + +int __attribute__((target("x87"))) foo(int a) { return 4; } +int __attribute__((target("no-x87"))) bar(int a) { retu
Re: [PATCH] D13980: Add "x87" in x86 target feature map
This revision was automatically updated to reflect the committed changes. Closed by commit rL264149: [X86] Add "x87" in x86 target feature map. (authored by aturetsk). Changed prior to commit: http://reviews.llvm.org/D13980?vs=48904&id=51398#toc Repository: rL LLVM http://reviews.llvm.org/D13980 Files: cfe/trunk/lib/Basic/Targets.cpp cfe/trunk/test/CodeGen/attr-target-x86-mmx.c cfe/trunk/test/CodeGen/attr-target-x86.c cfe/trunk/test/CodeGen/attr-target-x87-softfp.c Index: cfe/trunk/test/CodeGen/attr-target-x86.c === --- cfe/trunk/test/CodeGen/attr-target-x86.c +++ cfe/trunk/test/CodeGen/attr-target-x86.c @@ -31,9 +31,9 @@ // CHECK: qux{{.*}} #1 // CHECK: qax{{.*}} #4 // CHECK: qq{{.*}} #5 -// CHECK: #0 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2" -// CHECK: #1 = {{.*}}"target-cpu"="ivybridge" "target-features"="+aes,+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+xsave,+xsaveopt" -// CHECK: #2 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,-aes,-avx,-avx2,-avx512bw,-avx512cd,-avx512dq,-avx512er,-avx512f,-avx512ifma,-avx512pf,-avx512vbmi,-avx512vl,-f16c,-fma,-fma4,-pclmul,-sha,-sse2,-sse3,-sse4.1,-sse4.2,-sse4a,-ssse3,-xop,-xsave,-xsaveopt" -// CHECK: #3 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3" -// CHECK: #4 = {{.*}}"target-cpu"="ivybridge" "target-features"="+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+xsave,+xsaveopt,-aes" -// CHECK: #5 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+sse,+sse2,-3dnow,-3dnowa,-mmx" +// CHECK: #0 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" +// CHECK: #1 = {{.*}}"target-cpu"="ivybridge" "target-features"="+aes,+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave,+xsaveopt" +// CHECK: #2 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+x87,-aes,-avx,-avx2,-avx512bw,-avx512cd,-avx512dq,-avx512er,-avx512f,-avx512ifma,-avx512pf,-avx512vbmi,-avx512vl,-f16c,-fma,-fma4,-pclmul,-sha,-sse2,-sse3,-sse4.1,-sse4.2,-sse4a,-ssse3,-xop,-xsave,-xsaveopt" +// CHECK: #3 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87" +// CHECK: #4 = {{.*}}"target-cpu"="ivybridge" "target-features"="+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave,+xsaveopt,-aes" +// CHECK: #5 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+sse,+sse2,+x87,-3dnow,-3dnowa,-mmx" Index: cfe/trunk/test/CodeGen/attr-target-x87-softfp.c === --- cfe/trunk/test/CodeGen/attr-target-x87-softfp.c +++ cfe/trunk/test/CodeGen/attr-target-x87-softfp.c @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -triple x86_64-linux-gnu -target-cpu x86-64 -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=HARD +// RUN: %clang_cc1 -msoft-float -triple x86_64-linux-gnu -target-cpu x86-64 -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=SOFT + +int __attribute__((target("x87"))) foo(int a) { return 4; } +int __attribute__((target("no-x87"))) bar(int a) { return 4; } + +// CHECK: foo{{.*}} #0 +// CHECK: bar{{.*}} #1 + +// CHECK: #0 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" +// HARD: "use-soft-float"="false" +// SOFT: "use-soft-float"="true" + +// CHECK: #1 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,-x87" +// HARD: "use-soft-float"="false" +// SOFT: "use-soft-float"="true" Index: cfe/trunk/test/CodeGen/attr-target-x86-mmx.c === --- cfe/trunk/test/CodeGen/attr-target-x86-mmx.c +++ cfe/trunk/test/CodeGen/attr-target-x86-mmx.c @@ -19,4 +19,4 @@ _mm_srai_pi32(a, c); } -// CHECK: "target-features"="+mmx,+sse" +// CHECK: "target-features"="+mmx,+sse,+x87" Index: cfe/trunk/lib/Basic/Targets.cpp === --- cfe/trunk/lib/Basic/Targets.cpp +++ cfe/trunk/lib/Basic/Targets.cpp @@ -2585,6 +2585,9 @@ if (getTriple().getArch() == llvm::Triple::x86_64) setFeatureEnabledImpl(Features, "sse2", true); + // Enable X87 for all X86 processors. + setFeatureEnabledImpl(Features, "x87", true); + switch (getCPUKind(CPU)) { case CK_Generic: case CK_i386: Index: cfe/trunk/test/CodeGen/attr-target-x86.c === --- cfe/trunk/test/CodeGen/attr-target-x86.c +++ cfe/trunk/test/CodeGen/attr-target-x86.c @@ -31,9 +31,9 @@ // CHECK: qux{{.*}} #1 // CHECK: qax{{.*}} #4 // CHECK: qq{{.*}} #5 -// CHECK: #0 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+
Re: [PATCH] D13980: Add "x87" in x86 target feature map
aturetsk added a comment. The test Eric asked for was added. Repository: rL LLVM http://reviews.llvm.org/D13980 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision
NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:84 @@ +83,3 @@ +// Can E value be greater or equal than Val? +static bool canBeGreaterEqual(CheckerContext &C, const Expr *E, + unsigned long long Val) { danielmarjamaki wrote: > danielmarjamaki wrote: > > zaks.anna wrote: > > > danielmarjamaki wrote: > > > > zaks.anna wrote: > > > > > This function returns true if the value "is" greater or equal, not > > > > > "can be" greater or equal. The latter would be "return StGE". > > > > > > > > > > Also, it's slightly better to return the StGE state and use it to > > > > > report the bug. This way, our assumption is explicitly recorded in > > > > > the error state. > > > > NoQ made the same comment. I disagree. > > > > > > > > int A = 0; > > > > if (X) { > > > > A = 1000; > > > > } > > > > U8 = A; // <- Imho; A _can_ be 1000 > > > > > > > > Imho it's better to say that A _can_ be 1000 unless A is 1000 for all > > > > possible execution paths through the code. > > > > > > > > Do you still think "is" is better than "can be"? > > > The Clang Static Analyzer performs path sensitive analysis of the > > > program. (It does not merge the paths at the "U8 = A" statement!!!) You > > > will only be changing the state along a single execution path of this > > > program. Along that path, A will always be 1000. > > > > > > When analyzing your example, the analyzer is going to separately analyze > > > 2 paths: > > > 1st path: A=0; X != 0; A =1000; U8 = A; // Here U8 is definitely 1000. > > > 2d path: A=0; X == 0; U8 = A; // Here U8 is definitely 0. > > > > > > This video contains an intuitive explanation of symbolic execution > > > technique we use: > > > http://llvm.org/devmtg/2012-11/videos/Zaks-Rose-Checker24Hours.mp4 > > I understand that and I still think that value of A "can be" 1000. Yes in > > that path the value "is" 1000. > > > > But as far as I see, you and others disagree with me. And therefore I will > > change to "is". > For your information in Cppcheck I say that a value is "possible" if some > path(s) generates that value. And "always" when all paths generate that value. > > Code example: > > int f(int x) { > int a = 1000; > int b = 0; > if (x == 500) > a = 3; > return a + b - x; > } > > Debug output (cppcheck --debug-normal file.c): > > ##Value flow > Line 3 > 1000 always 1000 > Line 4 > 0 always 0 > Line 5 > x possible 500 > == possible 1 > 500 always 500 > Line 6 > 3 always 3 > Line 7 > a possible {1000,3} > + possible {1000,3} > b always 0 > x possible 500 > > For me personally it is confusing to say that A "is" 1000. That is different > to how I normally think of it in Cppcheck. Consider the following examples: ``` // Example 1. int a = rand(); use(a); ``` ``` // Example 2. int b; scanf("%d", &b); use(b); ``` Here `a` and `b` "can be" 1000 when passed into `use()`, however your function would return `false`, because there is no particular execution path found on which they are "certainly" 1000. On the other hand, in the following example: ``` // Example 3. int a = rand(); if (a == 1000) { } use(a); ``` at the `use()` of `a` your function would return `false` on the path that passes through one branch and `true` on another path, however the `false` return value would not indicate that `a` "cannot be" 1000 on this line; it simply indicates that `a` is not certainly 1000 on one (but not all) of the paths. These examples show that the function returns "false" much more often than its name suggests, hence we propose a different terminology. The good name for the function would be "the value 'certainly is' [greater or equal] on this particular path, though probably not on every path". Additionally, example 2 is of particular interest: it gives an example of a "tainted" symbol which is explicitly known to take all values that fit its integral type, depending on user (cf. "attacker") input. You might (some day, no rush, i guess) like to extend your checker to consider tainted values as truly "can-be-anything" and throw warnings even without finding a particular path. For such cases, your function would actually be something like "there exists a path on which it 'certainly can be', though maybe on other paths it's not as certain". There's already a basic support for such taint analysis in the analyzer. In fact, example 1 is also a curious discussion point - even when there's no attacker to substitute your random number generator, symbols produced by `rand()` and such are also explicitly known to take all values between `0` and some kind of `RAND_MAX`, which is a sort of information that may be useful to the analyzer and can be described as a weaker kind of taint without much security implications, bu
[PATCH] RHEL: Look in more places to find g++ headers and runtime
Some distros with ten years of support ship an old gcc but later offer more recent versions for installation in parallel. These versions are typically not only needed for the compilation of llvm/clang, but also to properly use the clang binary that comes out. Clang already searches /usr at runtime for the most recent installation of gcc. This patch appends paths for add-on installations of gcc in RHEL. --- a/tools/clang/lib/Driver/ToolChains.cpp 2014-08-27 22:07:31.0 +0200 +++ b/tools/clang/lib/Driver/ToolChains.cpp 2014-09-08 02:51:38.197987523 +0200 @@ -1249,8 +1249,14 @@ Prefixes.push_back(D.InstalledDir + "/.."); // And finally in /usr. -if (D.SysRoot.empty()) +if (D.SysRoot.empty()) { + Prefixes.push_back("/opt/rh/devtoolset-4/root/usr"); + Prefixes.push_back("/opt/rh/devtoolset-3/root/usr"); + Prefixes.push_back("/opt/rh/devtoolset-2/root/usr"); + Prefixes.push_back("/opt/rh/devtoolset-1.1/root/usr"); + Prefixes.push_back("/opt/rh/devtoolset-1.0/root/usr"); Prefixes.push_back("/usr"); +} } // Loop over the various components which exist and select the best GCC ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] Proper detection and handling of RHEL and variants
- Don't consider "/etc/lsb-release" to be Ubuntu only. - Detect SL, too. - Only add "--no-add-needed" for RHEL7 (or Fedora), not for RHEL6 (that's what the compilers shipped with RHEL do). --- a/tools/clang/lib/Driver/ToolChains.cpp 2015-09-02 04:26:13.266233474 +0200 +++ b/tools/clang/lib/Driver/ToolChains.cpp 2015-09-02 04:35:04.475976086 +0200 @@ -2934,7 +2934,8 @@ .Case("vivid", UbuntuVivid) .Case("wily", UbuntuWily) .Default(UnknownDistro); -return Version; +if (Version != UnknownDistro) + return Version; } File = llvm::MemoryBuffer::getFile("/etc/redhat-release"); @@ -2943,7 +2944,8 @@ if (Data.startswith("Fedora release")) return Fedora; if (Data.startswith("Red Hat Enterprise Linux") || -Data.startswith("CentOS")) { +Data.startswith("CentOS") || +Data.startswith("Scientific Linux")) { if (Data.find("release 7") != StringRef::npos) return RHEL7; else if (Data.find("release 6") != StringRef::npos) @@ -3176,7 +3178,7 @@ ExtraOpts.push_back("--hash-style=both"); } - if (IsRedhat(Distro)) + if (Distro == Fedora || Distro == RHEL7) ExtraOpts.push_back("--no-add-needed"); if ((IsDebian(Distro) && Distro >= DebianSqueeze) || IsOpenSUSE(Distro) || ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18283: clang-format: [JS] do not break location pragma comments.
mprobst abandoned this revision. mprobst added a comment. Abandoning this in favour of the "don't break imports at all" change. http://reviews.llvm.org/D18283 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D18391: Combine identical check-prefixes in Clang test/Preprocessor/arm-target-features.c
tyomitch created this revision. tyomitch added reviewers: rengolin, t.p.northover. tyomitch added a subscriber: cfe-commits. Herald added subscribers: rengolin, aemerson. http://reviews.llvm.org/D18391 Files: test/Preprocessor/arm-target-features.c Index: test/Preprocessor/arm-target-features.c === --- test/Preprocessor/arm-target-features.c +++ test/Preprocessor/arm-target-features.c @@ -55,45 +55,24 @@ // Check that -mhwdiv works properly for armv8/thumbv8 (enabled by default). -// RUN: %clang -target armv8 -x c -E -dM %s -o - | FileCheck --check-prefix=ARMV8 %s -// ARMV8:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv8 -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBV8 %s -// THUMBV8:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv8-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=ARMV8-EABI %s -// ARMV8-EABI:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv8-eabi -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBV8-EABI %s -// THUMBV8-EABI:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv8 -mhwdiv=none -x c -E -dM %s -o - | FileCheck --check-prefix=NONEHWDIV-ARMV8 %s -// NONEHWDIV-ARMV8-NOT:#define __ARM_ARCH_EXT_IDIV__ - -// RUN: %clang -target armv8 -mthumb -mhwdiv=none -x c -E -dM %s -o - | FileCheck --check-prefix=NONEHWDIV-THUMBV8 %s -// NONEHWDIV-THUMBV8-NOT:#define __ARM_ARCH_EXT_IDIV__ - -// RUN: %clang -target armv8 -mhwdiv=thumb -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBHWDIV-ARMV8 %s -// THUMBHWDIV-ARMV8-NOT:#define __ARM_ARCH_EXT_IDIV__ - -// RUN: %clang -target armv8 -mthumb -mhwdiv=arm -x c -E -dM %s -o - | FileCheck --check-prefix=ARMHWDIV-THUMBV8 %s -// ARMHWDIV-THUMBV8-NOT:#define __ARM_ARCH_EXT_IDIV__ - -// RUN: %clang -target armv8a -x c -E -dM %s -o - | FileCheck --check-prefix=ARMV8A %s -// ARMV8A:#define __ARM_ARCH_EXT_IDIV__ 1 -// ARMV8A: #define __ARM_FP 0xE - -// RUN: %clang -target armv8a -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBV8A %s -// THUMBV8A:#define __ARM_ARCH_EXT_IDIV__ 1 -// THUMBV8A: #define __ARM_FP 0xE - -// RUN: %clang -target armv8a-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=ARMV8A-EABI %s -// ARMV8A-EABI:#define __ARM_ARCH_EXT_IDIV__ 1 -// ARMV8A-EABI: #define __ARM_FP 0xE - -// RUN: %clang -target armv8a-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBV8A-EABI %s -// THUMBV8A-EABI:#define __ARM_ARCH_EXT_IDIV__ 1 -// THUMBV8A-EABI: #define __ARM_FP 0xE +// RUN: %clang -target armv8 -x c -E -dM %s -o - | FileCheck --check-prefix=V8 %s +// RUN: %clang -target armv8 -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=V8 %s +// RUN: %clang -target armv8-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=V8 %s +// RUN: %clang -target armv8-eabi -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=V8 %s +// V8:#define __ARM_ARCH_EXT_IDIV__ 1 + +// RUN: %clang -target armv8 -mhwdiv=none -x c -E -dM %s -o - | FileCheck --check-prefix=NOHWDIV-V8 %s +// RUN: %clang -target armv8 -mthumb -mhwdiv=none -x c -E -dM %s -o - | FileCheck --check-prefix=NOHWDIV-V8 %s +// RUN: %clang -target armv8 -mhwdiv=thumb -x c -E -dM %s -o - | FileCheck --check-prefix=NOHWDIV-V8 %s +// RUN: %clang -target armv8 -mthumb -mhwdiv=arm -x c -E -dM %s -o - | FileCheck --check-prefix=NOHWDIV-V8 %s +// NOHWDIV-V8-NOT:#define __ARM_ARCH_EXT_IDIV__ + +// RUN: %clang -target armv8a -x c -E -dM %s -o - | FileCheck --check-prefix=V8A %s +// RUN: %clang -target armv8a -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=V8A %s +// RUN: %clang -target armv8a-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=V8A %s +// RUN: %clang -target armv8a-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=V8A %s +// V8A:#define __ARM_ARCH_EXT_IDIV__ 1 +// V8A:#define __ARM_FP 0xE // RUN: %clang -target armv8m.base-none-linux-gnu -x c -E -dM %s -o - | FileCheck --check-prefix=V8M_BASELINE %s // V8M_BASELINE: __ARM_ARCH 8 @@ -150,29 +129,17 @@ // CHECK-SHORTENUMS:#define __ARM_SIZEOF_MINIMAL_ENUM 1 // Test that -mhwdiv has the right effect for a target CPU which has hwdiv enabled by default. -// RUN: %clang -target armv7 -mcpu=cortex-a15 -x c -E -dM %s -o - | FileCheck --check-prefix=DEFAULTHWDIV-ARM %s -// DEFAULTHWDIV-ARM:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv7 -mthumb -mcpu=cortex-a15 -x c -E -dM %s -o - | FileCheck --check-prefix=DEFAULTHWDIV-THUMB %s -// DEFAULTHWDIV-THUMB:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv7 -mcpu=cortex-a15 -mhwdiv=arm -x c -E -dM %s -o - | FileCheck --check-prefix=ARMHWDIV-ARM %s -// ARMHWDIV-ARM:#define __ARM_ARCH_EXT_IDIV__ 1 +// RUN: %clang -target armv7 -mcpu=cortex-a15 -x c -E -dM %s -o - | FileCheck --check-prefix=HWDIV %s +// RUN: %clang -target armv7 -mthumb -mcpu=cortex-a15 -x c -E -dM %s -o - | FileCheck --check-prefix=HWDIV %s +// RUN: %clang -target armv7 -mcpu=cortex-a15 -mhwdiv=arm -x c -E -dM %s -o - | FileCheck -
Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision
danielmarjamaki added a comment. Here are some false positives I have marked... let me know if you want more... I have marked this as a FP: URL: ftp://ftp.se.debian.org/debian/pool/main/m/m17n-lib/m17n-lib_1.7.0.orig.tar.gz File: m17n-lib-1.7.0/src/mtext.c Line 1946 Code: int mtext_set_char (MText *mt, int pos, int c) { ... switch (mt->format) { case MTEXT_FORMAT_US_ASCII: mt->data[pos_unit] = c; // <- WARNING break; . } } The precision of "c" can be too large for "mt->data[pos_unit]" if the format is not MTEXT_FORMAT_US_ASCII. I believe it's a FP but it can probably be fixed by an assertion inside the "case". I have marked this as a FP: URL: ftp://ftp.se.debian.org/debian/pool/main/n/netkit-ftp/netkit-ftp_0.17.orig.tar.gz File: netkit-ftp-0.17/ftp/ftp.c Line 416 Code: while ((c = getc(cin)) != '\n') { ... if (c == EOF) { return 4; } ... *cp++ = c; // <- WARNING } I have marked this as a FP: URL: ftp://ftp.se.debian.org/debian/pool/main/o/owl/owl_2.2.2.orig.tar.gz File: owl-2.2.2.orig/keypress.c Line: 185 Code: if (isascii(j)) { kb2[0] = j; // <- WARNING kb2[1] = 0; strcat(kb, kb2); } If isascii() returns 1 then j is a 7-bit value. Then there is not loss of precision. http://reviews.llvm.org/D13126 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision
danielmarjamaki updated this revision to Diff 51402. danielmarjamaki added a comment. Minor fixes of review comments. - Improved comments about the checker in the source code. - Improved capitalization and punctuation in error messages. - Renamed "canBe..." functions and changed their return type to ProgramStateRef to get better diagnostics. http://reviews.llvm.org/D13126 Files: lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/ConversionChecker.cpp test/Analysis/conversion.c Index: test/Analysis/conversion.c === --- test/Analysis/conversion.c +++ test/Analysis/conversion.c @@ -0,0 +1,80 @@ +// RUN: %clang_cc1 -Wno-conversion -analyze -analyzer-checker=core,alpha.core.Conversion -verify %s + +unsigned char U8; +signed char S8; + +void assign(unsigned U, signed S) { + if (S < -10) +U8 = S; // expected-warning {{Loss of sign in implicit conversion}} + if (U > 300) +S8 = U; // expected-warning {{Loss of precision in implicit conversion}} + if (S > 10) +U8 = S; + if (U < 200) +S8 = U; +} + +void init1() { + long long A = 1LL << 60; + short X = A; // expected-warning {{Loss of precision in implicit conversion}} +} + +void relational(unsigned U, signed S) { + if (S > 10) { +if (U < S) { +} + } + if (S < -10) { +if (U < S) { +} // expected-warning {{Loss of sign in implicit conversion}} + } +} + +void multiplication(unsigned U, signed S) { + if (S > 5) +S = U * S; + if (S < -10) +S = U * S; // expected-warning {{Loss of sign}} +} + +void division(unsigned U, signed S) { + if (S > 5) +S = U / S; + if (S < -10) +S = U / S; // expected-warning {{Loss of sign}} +} + +void dontwarn1(unsigned U, signed S) { + U8 = S; // It might be known that S is always 0x00-0xff. + S8 = U; // It might be known that U is always 0x00-0xff. + + U8 = -1; // Explicit conversion. + S8 = ~0U; // Explicit conversion. + if (U > 300) +U8 &= U; // No loss of precision since there is &=. +} + +void dontwarn2(unsigned int U) { + if (U <= 4294967295) { + } + if (U <= (2147483647 * 2U + 1U)) { + } +} + +void dontwarn3(int X) { + S8 = X ? 'a' : 'b'; +} + +// don't warn for macros +#define DOSTUFF ({ unsigned X = 1000; U8 = X; }) +void dontwarn4() { + DOSTUFF; +} + +// don't warn for calculations +// seen some fp. For instance: c2 = (c2 >= 'A' && c2 <= 'Z') ? c2 - 'A' + 'a' : c2; +// there is a todo in the checker to handle calculations +void dontwarn5() { + signed S = -32; + U8 = S + 10; +} Index: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp === --- lib/StaticAnalyzer/Checkers/ConversionChecker.cpp +++ lib/StaticAnalyzer/Checkers/ConversionChecker.cpp @@ -0,0 +1,181 @@ +//=== ConversionChecker.cpp -*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// +// +// Check that there is not loss of sign/precision. +// +// ConversionChecker generates a subset of the warnings that are reported by +// Wconversion. It is designed to be an alternative to Wconversion. +// +// ConversionChecker uses path sensitive analysis to avoid false positives. As +// warnings are only reported when there is possible loss of sign/precision, +// ConversionChecker is significantly less noisy than Wconversion. +// +//===--===// +#include "ClangSACheckers.h" +#include "clang/AST/ParentMap.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" + +using namespace clang; +using namespace ento; + +namespace { +class ConversionChecker : public Checker> { +public: + void checkPreStmt(const ImplicitCastExpr *Cast, CheckerContext &C) const; + +private: + mutable std::unique_ptr BT; + + void diagnoseLossOfPrecision(const ImplicitCastExpr *Cast, + CheckerContext &C) const; + void diagnoseLossOfSign(const ImplicitCastExpr *Cast, + CheckerContext &C) const; + + void reportBug(CheckerContext &C, ProgramStateRef St, const char Msg[]) const; +}; +} + +void ConversionChecker::checkPreStmt(const ImplicitCastExpr *Cast, + CheckerContext &C) const { + // TODO: For now we only warn about DeclRefExpr, to avoid noise. Warn for + // calculations also. + if (!isa(Cast->IgnoreParenImpCasts())) +return; + + // Don't warn for loss of sign/precision in macros + if (Cast->getExprLoc().isMacroID()) +return; + + //
Re: [PATCH] D18035: [GCC] PR23529 Mangler part of attrbute abi_tag support
DmitryPolukhin added inline comments. Comment at: lib/AST/ItaniumMangle.cpp:321-323 @@ +320,5 @@ + // track. + // + // FIXME: how to handle substituted names? They should add the tags used in + // the substitution to the list of available tags. + class AbiTagState final { rsmith wrote: > Do we need to? IIUC, the only time we need this list is when determining the > set of "available" tags for a function declaration with a tagged return type, > and in that case, a tag can only be available from a substitution if it's > also available from the target of that substitution (right?). It is not only for functions but also for variables but yes, as far as I understand, substitution itself cannot add tags and target should have all tags. I removed FIXME. Comment at: lib/AST/ItaniumMangle.cpp:348-361 @@ +347,16 @@ + +void pop() { + if (!LinkActive) +return; + + assert(LinkHead == this && + "abi tag link head must point to us on destruction"); + LinkActive = false; + if (Parent) { +Parent->UsedAbiTags.insert(UsedAbiTags.begin(), UsedAbiTags.end()); +Parent->EmittedAbiTags.insert(EmittedAbiTags.begin(), + EmittedAbiTags.end()); + } + LinkHead = Parent; +} + rsmith wrote: > Why do we need a stack of these? It seems like we only need one set of > available tags for the complete mangling process (it should only be used once > at the top level). Stack is required for mangling local names. In that case we have more than one list of available tags and it can be enclosed. http://reviews.llvm.org/D18035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18035: [GCC] PR23529 Mangler part of attrbute abi_tag support
DmitryPolukhin updated this revision to Diff 51403. DmitryPolukhin marked 4 inline comments as done. DmitryPolukhin added a comment. Fixed comments. PTAL http://reviews.llvm.org/D18035 Files: lib/AST/ItaniumMangle.cpp lib/Sema/SemaDeclAttr.cpp test/CodeGenCXX/mangle-abi-tag.cpp test/SemaCXX/attr-abi-tag-syntax.cpp Index: test/SemaCXX/attr-abi-tag-syntax.cpp === --- test/SemaCXX/attr-abi-tag-syntax.cpp +++ test/SemaCXX/attr-abi-tag-syntax.cpp @@ -16,28 +16,18 @@ // expected-warning@-1 {{'abi_tag' attribute on anonymous namespace ignored}} inline namespace N __attribute__((__abi_tag__)) {} -// FIXME: remove this warning as soon as attribute fully supported. -// expected-warning@-2 {{'__abi_tag__' attribute ignored}} } // namespcace N2 __attribute__((abi_tag("B", "A"))) extern int a1; -// FIXME: remove this warning as soon as attribute fully supported. -// expected-warning@-2 {{'abi_tag' attribute ignored}} __attribute__((abi_tag("A", "B"))) extern int a1; // expected-note@-1 {{previous declaration is here}} -// FIXME: remove this warning as soon as attribute fully supported. -// expected-warning@-3 {{'abi_tag' attribute ignored}} __attribute__((abi_tag("A", "C"))) extern int a1; // expected-error@-1 {{'abi_tag' C missing in original declaration}} -// FIXME: remove this warning as soon as attribute fully supported. -// expected-warning@-3 {{'abi_tag' attribute ignored}} extern int a2; // expected-note@-1 {{previous declaration is here}} __attribute__((abi_tag("A")))extern int a2; // expected-error@-1 {{cannot add 'abi_tag' attribute in a redeclaration}} -// FIXME: remove this warning as soon as attribute fully supported. -// expected-warning@-3 {{'abi_tag' attribute ignored}} Index: test/CodeGenCXX/mangle-abi-tag.cpp === --- /dev/null +++ test/CodeGenCXX/mangle-abi-tag.cpp @@ -0,0 +1,137 @@ +// RUN: %clang_cc1 %s -emit-llvm -triple %itanium_abi_triple -std=c++11 -o - | FileCheck %s + +struct __attribute__((abi_tag("A", "B"))) A { }; + +struct B: A { }; + +template + +struct C { +}; + +struct D { A* p; }; + +template +struct __attribute__((abi_tag("C", "D"))) E { +}; + +struct __attribute__((abi_tag("A", "B"))) F { }; + +A a1; +// CHECK: @_Z2a1B1AB1B = + +__attribute__((abi_tag("C", "D"))) +A a2; +// CHECK: @_Z2a2B1AB1BB1CB1D = + +B a3; +// CHECK: @a3 = + +C a4; +// CHECK: @_Z2a4B1AB1B = + +D a5; +// CHECK: @a5 = + +E a6; +// CHECK: @_Z2a6B1CB1D = + +E a7; +// CHECK: @_Z2a7B1AB1BB1CB1D = + +template<> +struct E { + static float a8; +}; +float E::a8; +// CHECK: @_ZN1EB1CB1DIfE2a8E = + +template<> +struct E { + static bool a9; +}; +bool E::a9; +// CHECK: @_ZN1EB1CB1DI1FB1AB1BE2a9E = + +struct __attribute__((abi_tag("A", "B"))) A10 { + virtual ~A10() {} +} a10; +// vtable +// CHECK: @_ZTV3A10B1AB1B = +// typeinfo +// CHECK: @_ZTI3A10B1AB1B = + +// Local variables from f9. +// f11()::L::foo[abi:C][abi:D]()::a[abi:A][abi:B] +// CHECK-DAG: @_ZZZ3f11vEN1L3fooB1CB1DEvE1aB1AB1B = +// guard variable for f11()::L::foo[abi:C][abi:D]()::a[abi:A][abi:B] +// CHECK-DAG: @_ZGVZZ3f11vEN1L3fooB1CB1DEvE1aB1AB1B = + +__attribute__ ((abi_tag("C", "D"))) +void* f1() { + return 0; +} +// CHECK: define {{.*}} @_Z2f1B1CB1Dv( + +__attribute__ ((abi_tag("C", "D"))) +A* f2() { + return 0; +} +// CHECK: define {{.*}} @_Z2f2B1AB1BB1CB1Dv( + +B* f3() { + return 0; +} +// CHECK: define {{.*}} @_Z2f3v( + +C* f4() { + return 0; +} +// CHECK: define {{.*}} @_Z2f4B1AB1Bv( + +D* f5() { + return 0; +} +// CHECK: define {{.*}} @_Z2f5v( + +E* f6() { + return 0; +} +// CHECK: define {{.*}} @_Z2f6B1CB1Dv( + +E* f7() { + return 0; +} +// CHECK: define {{.*}} @_Z2f7B1AB1BB1CB1Dv( + +void f8(E*) { +} +// CHECK: define {{.*}} @_Z2f8P1EB1CB1DI1AB1AB1BE( + +inline namespace Names1 __attribute__((__abi_tag__)) { +class C1 {}; +} +C1 f9() { return C1(); } +// CHECK: @_Z2f9B6Names1v() + +inline namespace Names2 __attribute__((__abi_tag__("Tag1", "Tag2"))) { +class C2 {}; +} +C2 f10() { return C2(); } +// CHECK: @_Z3f10B4Tag1B4Tag2v() + +inline void f11() { + struct L { +static E* foo() { + static A10 a; + return 0; +} + }; + L::foo(); +} +void f11_test() { + f11(); +} +// f11()::L::foo[abi:C][abi:D]() +// CHECK: define linkonce_odr %struct.E* @_ZZ3f11vEN1L3fooB1CB1DEv( + Index: lib/Sema/SemaDeclAttr.cpp === --- lib/Sema/SemaDeclAttr.cpp +++ lib/Sema/SemaDeclAttr.cpp @@ -4684,10 +4684,6 @@ D->addAttr(::new (S.Context) AbiTagAttr(Attr.getRange(), S.Context, Tags.data(), Tags.size(), Attr.getAttributeSpellingListIndex())); - - // FIXME: remove this warning as soon as mangled part is ready. - S.Diag(Attr.getRange().getBegin(), diag::warn_attribute_ignored) -<< Attr.getName(); } static void handleARMInterruptAttr(Sema &S, Decl *D, Index: lib/AST/
Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision
danielmarjamaki marked 3 inline comments as done. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:12 @@ +11,3 @@ +// +// ConversionChecker generates a subset of the warnings that are reported by +// Wconversion. It is designed to be an alternative to Wconversion. Thanks! I have tried to do that. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:30 @@ +29,3 @@ + +namespace { +class ConversionChecker : public Checker> { ok Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:85 @@ +84,3 @@ + if (!N) +return; + I renamed and changed these functions. Hope we all like it better now. The name is now "greaterEqualState" and it returns the state when the value is greater or equal. If there is no such state it returns nullptr. As far as I see the diagnostics are showing the proper path now.. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:148 @@ +147,3 @@ + QualType SubType = Cast->IgnoreParenImpCasts()->getType(); + + if (!CastType->isIntegerType() || !SubType->isIntegerType()) sorry about that, I have fixed it http://reviews.llvm.org/D13126 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D17820: Clang Code Completion Filtering
CrisCristescu updated this revision to Diff 51404. CrisCristescu added a comment. The code_completion token now stores the information, if there is one, about the identifier that it is lexing. During the parsing there is a common entry point which is ConsumerToken where we can set the information necessary for filtering in the Sema in the case of a code_completion token. The information is retrieved when doing the actual filtering. This introduces a change in functionality which means that some tests had to be modified(6 tests for ObjC) due to the fact that if you press tab at the end of a word which could be a language specific keyword, you will get the possible completions of this identifier, not what could be written after the space. This is in accordance with the behaviour of IDEs, for instance XCode. As an example: const will only give completions for variables like constVar, and if you have const the you get the possible completions: const double, const int etc. Repository: rL LLVM http://reviews.llvm.org/D17820 Files: include/clang/Parse/Parser.h include/clang/Sema/CodeCompleteConsumer.h include/clang/Sema/Sema.h lib/Lex/Lexer.cpp lib/Sema/CodeCompleteConsumer.cpp lib/Sema/Sema.cpp test/CodeCompletion/objc-message.mm test/Index/complete-method-decls.m test/Index/complete-objc-message-id.m test/Index/complete-objc-message.m test/Index/complete-recovery.m test/Index/complete-super.m Index: test/Index/complete-super.m === --- test/Index/complete-super.m +++ test/Index/complete-super.m @@ -60,7 +60,7 @@ // RUN: c-index-test -code-completion-at=%s:20:16 %s | FileCheck -check-prefix=CHECK-ADD-TO %s // CHECK-ADD-TO: ObjCInstanceMethodDecl:{ResultType void}{Informative add:}{TypedText to:}{Placeholder b} (20) -// RUN: c-index-test -code-completion-at=%s:24:28 %s | FileCheck -check-prefix=CHECK-SELECTOR-FIRST %s +// RUN: c-index-test -code-completion-at=%s:24:29 %s | FileCheck -check-prefix=CHECK-SELECTOR-FIRST %s // CHECK-SELECTOR-FIRST: ObjCClassMethodDecl:{ResultType void}{Informative select:}{TypedText first:}{Placeholder a}{HorizontalSpace }{Text second:}{Placeholder b} (20) // Check "super" completion at the third identifier @@ -69,7 +69,7 @@ // Check "super" completion with missing '['. // RUN: c-index-test -code-completion-at=%s:25:10 %s | FileCheck -check-prefix=CHECK-SELECTOR-SELECTOR %s -// RUN: c-index-test -code-completion-at=%s:25:28 %s | FileCheck -check-prefix=CHECK-SELECTOR-FIRST %s +// RUN: c-index-test -code-completion-at=%s:25:29 %s | FileCheck -check-prefix=CHECK-SELECTOR-FIRST %s // RUN: c-index-test -code-completion-at=%s:25:37 %s | FileCheck -check-prefix=CHECK-SELECTOR-SECOND %s // Check "super" completion for a method declared in a category. Index: test/Index/complete-recovery.m === --- test/Index/complete-recovery.m +++ test/Index/complete-recovery.m @@ -26,7 +26,7 @@ // Test case for fix committed in r145441. // RUN: env CINDEXTEST_CODE_COMPLETE_PATTERNS=1 c-index-test -code-completion-at=%s:9:20 %s -fms-compatibility | FileCheck -check-prefix=CHECK-CC1 %s -// RUN: env CINDEXTEST_CODE_COMPLETE_PATTERNS=1 c-index-test -code-completion-at=%s:10:24 %s | FileCheck -check-prefix=CHECK-CC2 %s +// RUN: env CINDEXTEST_CODE_COMPLETE_PATTERNS=1 c-index-test -code-completion-at=%s:10:25 %s | FileCheck -check-prefix=CHECK-CC2 %s // CHECK-CC2: NotImplemented:{ResultType char[]}{TypedText @encode}{LeftParen (}{Placeholder type-name}{RightParen )} // CHECK-CC2: NotImplemented:{TypedText _Bool} // CHECK-CC2: VarDecl:{ResultType A *}{TypedText a} Index: test/Index/complete-objc-message.m === --- test/Index/complete-objc-message.m +++ test/Index/complete-objc-message.m @@ -218,13 +218,13 @@ // CHECK-CC2-NEXT: Container Kind: ObjCInterfaceDecl // CHECK-CC2-NEXT: Container is complete // CHECK-CC2-NEXT: Container USR: c:objc(cs)Foo -// RUN: c-index-test -code-completion-at=%s:61:16 %s | FileCheck -check-prefix=CHECK-CC3 %s +// RUN: c-index-test -code-completion-at=%s:61:17 %s | FileCheck -check-prefix=CHECK-CC3 %s // CHECK-CC3: ObjCClassMethodDecl:{ResultType int}{TypedText MyClassMethod:}{Placeholder (id)} // CHECK-CC3: ObjCClassMethodDecl:{ResultType int}{TypedText MyPrivateMethod} -// RUN: c-index-test -code-completion-at=%s:65:16 %s | FileCheck -check-prefix=CHECK-CC4 %s +// RUN: c-index-test -code-completion-at=%s:65:17 %s | FileCheck -check-prefix=CHECK-CC4 %s // CHECK-CC4: ObjCInstanceMethodDecl:{ResultType int}{TypedText MyInstMethod:}{Placeholder (id)}{HorizontalSpace }{TypedText second:}{Placeholder (id)} // CHECK-CC4: ObjCInstanceMethodDecl:{ResultType int}{TypedText MyPrivateInstMethod} -// RUN: c-index-test -code-completion-at=%s:74:9 %s | FileCheck -check-prefix=CHECK-CC5 %s +// RUN: c-index-test -code-completion-at=%s:74:10 %s | File
[PATCH] D18392: clang-cl: Don't warn about /Oy- being unused in 64-bit builds.
thakis created this revision. thakis added reviewers: hans, rnk. thakis added a subscriber: cfe-commits. http://reviews.llvm.org/D18392 Files: lib/Driver/MSVCToolChain.cpp test/Driver/cl-options.c Index: test/Driver/cl-options.c === --- test/Driver/cl-options.c +++ test/Driver/cl-options.c @@ -123,13 +123,13 @@ // PR24003: -momit-leaf-frame-pointer // PR24003: -Os -// RUN: %clang_cl --target=i686-pc-win32 /Oy- /O2 -### -- %s 2>&1 | FileCheck -check-prefix=Oy_2 %s +// RUN: %clang_cl --target=i686-pc-win32 -Werror /Oy- /O2 -### -- %s 2>&1 | FileCheck -check-prefix=Oy_2 %s // Oy_2: -momit-leaf-frame-pointer // Oy_2: -O2 -// RUN: %clang_cl /Zs /Oy -- %s 2>&1 +// RUN: %clang_cl /Zs -Werror /Oy -- %s 2>&1 -// RUN: %clang_cl --target=i686-pc-win32 /Oy- -### -- %s 2>&1 | FileCheck -check-prefix=Oy_ %s +// RUN: %clang_cl --target=i686-pc-win32 -Werror /Oy- -### -- %s 2>&1 | FileCheck -check-prefix=Oy_ %s // Oy_: -mdisable-fp-elim // RUN: %clang_cl /Qvec -### -- %s 2>&1 | FileCheck -check-prefix=Qvec %s Index: lib/Driver/MSVCToolChain.cpp === --- lib/Driver/MSVCToolChain.cpp +++ lib/Driver/MSVCToolChain.cpp @@ -702,6 +702,12 @@ else DAL.AddFlagArg( A, Opts.getOption(options::OPT_fno_omit_frame_pointer)); + } else { +// Don't warn about /Oy- in 64-bit builds (where +// SupportsForcingFramePointer is false). The flag having no effect +// there is a compiler-internal optimization, and people shouldn't have +// to special-case their build files for 64-bit clang-cl. +A->claim(); } break; } Index: test/Driver/cl-options.c === --- test/Driver/cl-options.c +++ test/Driver/cl-options.c @@ -123,13 +123,13 @@ // PR24003: -momit-leaf-frame-pointer // PR24003: -Os -// RUN: %clang_cl --target=i686-pc-win32 /Oy- /O2 -### -- %s 2>&1 | FileCheck -check-prefix=Oy_2 %s +// RUN: %clang_cl --target=i686-pc-win32 -Werror /Oy- /O2 -### -- %s 2>&1 | FileCheck -check-prefix=Oy_2 %s // Oy_2: -momit-leaf-frame-pointer // Oy_2: -O2 -// RUN: %clang_cl /Zs /Oy -- %s 2>&1 +// RUN: %clang_cl /Zs -Werror /Oy -- %s 2>&1 -// RUN: %clang_cl --target=i686-pc-win32 /Oy- -### -- %s 2>&1 | FileCheck -check-prefix=Oy_ %s +// RUN: %clang_cl --target=i686-pc-win32 -Werror /Oy- -### -- %s 2>&1 | FileCheck -check-prefix=Oy_ %s // Oy_: -mdisable-fp-elim // RUN: %clang_cl /Qvec -### -- %s 2>&1 | FileCheck -check-prefix=Qvec %s Index: lib/Driver/MSVCToolChain.cpp === --- lib/Driver/MSVCToolChain.cpp +++ lib/Driver/MSVCToolChain.cpp @@ -702,6 +702,12 @@ else DAL.AddFlagArg( A, Opts.getOption(options::OPT_fno_omit_frame_pointer)); + } else { +// Don't warn about /Oy- in 64-bit builds (where +// SupportsForcingFramePointer is false). The flag having no effect +// there is a compiler-internal optimization, and people shouldn't have +// to special-case their build files for 64-bit clang-cl. +A->claim(); } break; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D17820: Clang Code Completion Filtering
CrisCristescu added a comment. For the filtering itself the motivation for not doing it on the client side is the following in our use case: - we do not re-filter on every key stroke, we only filter when the key is pressed i.e there is one completion point; - we have a PCH which will help with the generation time; - performance is acceptable with the patch included: less than 1s even when the file contains several "heavier" includes such as or ; - without the patch if the key is pressed in the middle of the identifier name (i.e setDouble) an error is thrown: fiter-member-access.cpp:19:17: error: no member named 'set' in 'MyClass' objectMyClass->set ~ ^ - and most importantly, we plan to propose another set of patches which would do lookup's only on subsets of the DeclContext of the identifier i.e incomplete lookup-s in order to reduce the amount of PCH deserializations. Repository: rL LLVM http://reviews.llvm.org/D17820 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r264155 - Combine identical check-prefixes in Clang test/Preprocessor/arm-target-features.c
Author: askrobov Date: Wed Mar 23 08:32:33 2016 New Revision: 264155 URL: http://llvm.org/viewvc/llvm-project?rev=264155&view=rev Log: Combine identical check-prefixes in Clang test/Preprocessor/arm-target-features.c Reviewers: rengolin, t.p.northover Subscribers: aemerson, cfe-commits, rengolin Differential Revision: http://reviews.llvm.org/D18391 Modified: cfe/trunk/test/Preprocessor/arm-target-features.c Modified: cfe/trunk/test/Preprocessor/arm-target-features.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/arm-target-features.c?rev=264155&r1=264154&r2=264155&view=diff == --- cfe/trunk/test/Preprocessor/arm-target-features.c (original) +++ cfe/trunk/test/Preprocessor/arm-target-features.c Wed Mar 23 08:32:33 2016 @@ -55,45 +55,24 @@ // Check that -mhwdiv works properly for armv8/thumbv8 (enabled by default). -// RUN: %clang -target armv8 -x c -E -dM %s -o - | FileCheck --check-prefix=ARMV8 %s -// ARMV8:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv8 -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBV8 %s -// THUMBV8:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv8-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=ARMV8-EABI %s -// ARMV8-EABI:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv8-eabi -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBV8-EABI %s -// THUMBV8-EABI:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv8 -mhwdiv=none -x c -E -dM %s -o - | FileCheck --check-prefix=NONEHWDIV-ARMV8 %s -// NONEHWDIV-ARMV8-NOT:#define __ARM_ARCH_EXT_IDIV__ - -// RUN: %clang -target armv8 -mthumb -mhwdiv=none -x c -E -dM %s -o - | FileCheck --check-prefix=NONEHWDIV-THUMBV8 %s -// NONEHWDIV-THUMBV8-NOT:#define __ARM_ARCH_EXT_IDIV__ - -// RUN: %clang -target armv8 -mhwdiv=thumb -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBHWDIV-ARMV8 %s -// THUMBHWDIV-ARMV8-NOT:#define __ARM_ARCH_EXT_IDIV__ - -// RUN: %clang -target armv8 -mthumb -mhwdiv=arm -x c -E -dM %s -o - | FileCheck --check-prefix=ARMHWDIV-THUMBV8 %s -// ARMHWDIV-THUMBV8-NOT:#define __ARM_ARCH_EXT_IDIV__ - -// RUN: %clang -target armv8a -x c -E -dM %s -o - | FileCheck --check-prefix=ARMV8A %s -// ARMV8A:#define __ARM_ARCH_EXT_IDIV__ 1 -// ARMV8A: #define __ARM_FP 0xE - -// RUN: %clang -target armv8a -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBV8A %s -// THUMBV8A:#define __ARM_ARCH_EXT_IDIV__ 1 -// THUMBV8A: #define __ARM_FP 0xE - -// RUN: %clang -target armv8a-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=ARMV8A-EABI %s -// ARMV8A-EABI:#define __ARM_ARCH_EXT_IDIV__ 1 -// ARMV8A-EABI: #define __ARM_FP 0xE - -// RUN: %clang -target armv8a-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBV8A-EABI %s -// THUMBV8A-EABI:#define __ARM_ARCH_EXT_IDIV__ 1 -// THUMBV8A-EABI: #define __ARM_FP 0xE +// RUN: %clang -target armv8 -x c -E -dM %s -o - | FileCheck --check-prefix=V8 %s +// RUN: %clang -target armv8 -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=V8 %s +// RUN: %clang -target armv8-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=V8 %s +// RUN: %clang -target armv8-eabi -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=V8 %s +// V8:#define __ARM_ARCH_EXT_IDIV__ 1 + +// RUN: %clang -target armv8 -mhwdiv=none -x c -E -dM %s -o - | FileCheck --check-prefix=NOHWDIV-V8 %s +// RUN: %clang -target armv8 -mthumb -mhwdiv=none -x c -E -dM %s -o - | FileCheck --check-prefix=NOHWDIV-V8 %s +// RUN: %clang -target armv8 -mhwdiv=thumb -x c -E -dM %s -o - | FileCheck --check-prefix=NOHWDIV-V8 %s +// RUN: %clang -target armv8 -mthumb -mhwdiv=arm -x c -E -dM %s -o - | FileCheck --check-prefix=NOHWDIV-V8 %s +// NOHWDIV-V8-NOT:#define __ARM_ARCH_EXT_IDIV__ + +// RUN: %clang -target armv8a -x c -E -dM %s -o - | FileCheck --check-prefix=V8A %s +// RUN: %clang -target armv8a -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=V8A %s +// RUN: %clang -target armv8a-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=V8A %s +// RUN: %clang -target armv8a-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=V8A %s +// V8A:#define __ARM_ARCH_EXT_IDIV__ 1 +// V8A:#define __ARM_FP 0xE // RUN: %clang -target armv8m.base-none-linux-gnu -x c -E -dM %s -o - | FileCheck --check-prefix=V8M_BASELINE %s // V8M_BASELINE: __ARM_ARCH 8 @@ -150,29 +129,17 @@ // CHECK-SHORTENUMS:#define __ARM_SIZEOF_MINIMAL_ENUM 1 // Test that -mhwdiv has the right effect for a target CPU which has hwdiv enabled by default. -// RUN: %clang -target armv7 -mcpu=cortex-a15 -x c -E -dM %s -o - | FileCheck --check-prefix=DEFAULTHWDIV-ARM %s -// DEFAULTHWDIV-ARM:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv7 -mthumb -mcpu=cortex-a15 -x c -E -dM %s -o - | FileCheck --check-prefix=DEFAULTHWDIV-THUMB %s -// DEFAULTHWDIV-THUMB:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv7 -mcpu=cortex-a15 -mhwdiv=arm -x c -E -
Re: [PATCH] D18391: Combine identical check-prefixes in Clang test/Preprocessor/arm-target-features.c
rengolin accepted this revision. rengolin added a comment. This revision is now accepted and ready to land. Nice cleaning! LGTM. Thanks! http://reviews.llvm.org/D18391 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18391: Combine identical check-prefixes in Clang test/Preprocessor/arm-target-features.c
This revision was automatically updated to reflect the committed changes. Closed by commit rL264155: Combine identical check-prefixes in Clang test/Preprocessor/arm-target… (authored by askrobov). Changed prior to commit: http://reviews.llvm.org/D18391?vs=51400&id=51410#toc Repository: rL LLVM http://reviews.llvm.org/D18391 Files: cfe/trunk/test/Preprocessor/arm-target-features.c Index: cfe/trunk/test/Preprocessor/arm-target-features.c === --- cfe/trunk/test/Preprocessor/arm-target-features.c +++ cfe/trunk/test/Preprocessor/arm-target-features.c @@ -55,45 +55,24 @@ // Check that -mhwdiv works properly for armv8/thumbv8 (enabled by default). -// RUN: %clang -target armv8 -x c -E -dM %s -o - | FileCheck --check-prefix=ARMV8 %s -// ARMV8:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv8 -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBV8 %s -// THUMBV8:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv8-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=ARMV8-EABI %s -// ARMV8-EABI:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv8-eabi -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBV8-EABI %s -// THUMBV8-EABI:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv8 -mhwdiv=none -x c -E -dM %s -o - | FileCheck --check-prefix=NONEHWDIV-ARMV8 %s -// NONEHWDIV-ARMV8-NOT:#define __ARM_ARCH_EXT_IDIV__ - -// RUN: %clang -target armv8 -mthumb -mhwdiv=none -x c -E -dM %s -o - | FileCheck --check-prefix=NONEHWDIV-THUMBV8 %s -// NONEHWDIV-THUMBV8-NOT:#define __ARM_ARCH_EXT_IDIV__ - -// RUN: %clang -target armv8 -mhwdiv=thumb -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBHWDIV-ARMV8 %s -// THUMBHWDIV-ARMV8-NOT:#define __ARM_ARCH_EXT_IDIV__ - -// RUN: %clang -target armv8 -mthumb -mhwdiv=arm -x c -E -dM %s -o - | FileCheck --check-prefix=ARMHWDIV-THUMBV8 %s -// ARMHWDIV-THUMBV8-NOT:#define __ARM_ARCH_EXT_IDIV__ - -// RUN: %clang -target armv8a -x c -E -dM %s -o - | FileCheck --check-prefix=ARMV8A %s -// ARMV8A:#define __ARM_ARCH_EXT_IDIV__ 1 -// ARMV8A: #define __ARM_FP 0xE - -// RUN: %clang -target armv8a -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBV8A %s -// THUMBV8A:#define __ARM_ARCH_EXT_IDIV__ 1 -// THUMBV8A: #define __ARM_FP 0xE - -// RUN: %clang -target armv8a-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=ARMV8A-EABI %s -// ARMV8A-EABI:#define __ARM_ARCH_EXT_IDIV__ 1 -// ARMV8A-EABI: #define __ARM_FP 0xE - -// RUN: %clang -target armv8a-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBV8A-EABI %s -// THUMBV8A-EABI:#define __ARM_ARCH_EXT_IDIV__ 1 -// THUMBV8A-EABI: #define __ARM_FP 0xE +// RUN: %clang -target armv8 -x c -E -dM %s -o - | FileCheck --check-prefix=V8 %s +// RUN: %clang -target armv8 -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=V8 %s +// RUN: %clang -target armv8-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=V8 %s +// RUN: %clang -target armv8-eabi -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=V8 %s +// V8:#define __ARM_ARCH_EXT_IDIV__ 1 + +// RUN: %clang -target armv8 -mhwdiv=none -x c -E -dM %s -o - | FileCheck --check-prefix=NOHWDIV-V8 %s +// RUN: %clang -target armv8 -mthumb -mhwdiv=none -x c -E -dM %s -o - | FileCheck --check-prefix=NOHWDIV-V8 %s +// RUN: %clang -target armv8 -mhwdiv=thumb -x c -E -dM %s -o - | FileCheck --check-prefix=NOHWDIV-V8 %s +// RUN: %clang -target armv8 -mthumb -mhwdiv=arm -x c -E -dM %s -o - | FileCheck --check-prefix=NOHWDIV-V8 %s +// NOHWDIV-V8-NOT:#define __ARM_ARCH_EXT_IDIV__ + +// RUN: %clang -target armv8a -x c -E -dM %s -o - | FileCheck --check-prefix=V8A %s +// RUN: %clang -target armv8a -mthumb -x c -E -dM %s -o - | FileCheck --check-prefix=V8A %s +// RUN: %clang -target armv8a-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=V8A %s +// RUN: %clang -target armv8a-eabi -x c -E -dM %s -o - | FileCheck --check-prefix=V8A %s +// V8A:#define __ARM_ARCH_EXT_IDIV__ 1 +// V8A:#define __ARM_FP 0xE // RUN: %clang -target armv8m.base-none-linux-gnu -x c -E -dM %s -o - | FileCheck --check-prefix=V8M_BASELINE %s // V8M_BASELINE: __ARM_ARCH 8 @@ -150,29 +129,17 @@ // CHECK-SHORTENUMS:#define __ARM_SIZEOF_MINIMAL_ENUM 1 // Test that -mhwdiv has the right effect for a target CPU which has hwdiv enabled by default. -// RUN: %clang -target armv7 -mcpu=cortex-a15 -x c -E -dM %s -o - | FileCheck --check-prefix=DEFAULTHWDIV-ARM %s -// DEFAULTHWDIV-ARM:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv7 -mthumb -mcpu=cortex-a15 -x c -E -dM %s -o - | FileCheck --check-prefix=DEFAULTHWDIV-THUMB %s -// DEFAULTHWDIV-THUMB:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv7 -mcpu=cortex-a15 -mhwdiv=arm -x c -E -dM %s -o - | FileCheck --check-prefix=ARMHWDIV-ARM %s -// ARMHWDIV-ARM:#define __ARM_ARCH_EXT_IDIV__ 1 - -// RUN: %clang -target armv7 -mthumb -mcpu=cortex-a15 -mhwdiv=thumb -x c -E -dM %s -o - | FileCheck --check-prefix=THUMBHWDIV-THUMB
Re: [PATCH] D18347: [PATCH] Fix thread_annotation negtest for thread safety.
richard.barton.arm updated this revision to Diff 51409. richard.barton.arm added a comment. My local run with no threads works with this updated patch. http://reviews.llvm.org/D18347 Files: test/libcxx/thread/thread.mutex/thread_safety_annotations_not_enabled.pass.cpp test/libcxx/thread/thread.mutex/thread_safety_lock_guard.pass.cpp test/libcxx/thread/thread.mutex/thread_safety_lock_unlock.pass.cpp test/libcxx/thread/thread.mutex/thread_safety_missing_unlock.fail.cpp test/libcxx/thread/thread.mutex/thread_safety_requires_capability.pass.cpp test/std/thread/futures/futures.promise/copy_assign.fail.cpp test/std/thread/futures/futures.promise/copy_ctor.fail.cpp test/std/thread/futures/futures.tas/futures.task.members/assign_copy.fail.cpp test/std/thread/futures/futures.tas/futures.task.members/ctor1.fail.cpp test/std/thread/futures/futures.tas/futures.task.members/ctor2.fail.cpp test/std/thread/futures/futures.tas/futures.task.members/ctor_copy.fail.cpp test/std/thread/futures/futures.unique_future/copy_assign.fail.cpp test/std/thread/futures/futures.unique_future/copy_ctor.fail.cpp test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/assign.fail.cpp test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/copy.fail.cpp Index: test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/copy.fail.cpp === --- test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/copy.fail.cpp +++ test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/copy.fail.cpp @@ -7,6 +7,7 @@ // //===--===// // +// UNSUPPORTED: libcpp-has-no-threads // UNSUPPORTED: c++98, c++03, c++11, c++14 // Index: test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/assign.fail.cpp === --- test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/assign.fail.cpp +++ test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/assign.fail.cpp @@ -7,6 +7,7 @@ // //===--===// // +// UNSUPPORTED: libcpp-has-no-threads // UNSUPPORTED: c++98, c++03, c++11, c++14 // Index: test/std/thread/futures/futures.unique_future/copy_ctor.fail.cpp === --- test/std/thread/futures/futures.unique_future/copy_ctor.fail.cpp +++ test/std/thread/futures/futures.unique_future/copy_ctor.fail.cpp @@ -6,6 +6,8 @@ // Source Licenses. See LICENSE.TXT for details. // //===--===// +// +// UNSUPPORTED: libcpp-has-no-threads // Index: test/std/thread/futures/futures.unique_future/copy_assign.fail.cpp === --- test/std/thread/futures/futures.unique_future/copy_assign.fail.cpp +++ test/std/thread/futures/futures.unique_future/copy_assign.fail.cpp @@ -6,6 +6,8 @@ // Source Licenses. See LICENSE.TXT for details. // //===--===// +// +// UNSUPPORTED: libcpp-has-no-threads // Index: test/std/thread/futures/futures.tas/futures.task.members/ctor_copy.fail.cpp === --- test/std/thread/futures/futures.tas/futures.task.members/ctor_copy.fail.cpp +++ test/std/thread/futures/futures.tas/futures.task.members/ctor_copy.fail.cpp @@ -6,7 +6,8 @@ // Source Licenses. See LICENSE.TXT for details. // //===--===// - +// +// UNSUPPORTED: libcpp-has-no-threads // UNSUPPORTED: c++98, c++03 // Index: test/std/thread/futures/futures.tas/futures.task.members/ctor2.fail.cpp === --- test/std/thread/futures/futures.tas/futures.task.members/ctor2.fail.cpp +++ test/std/thread/futures/futures.tas/futures.task.members/ctor2.fail.cpp @@ -6,7 +6,8 @@ // Source Licenses. See LICENSE.TXT for details. // //===--===// - +// +// UNSUPPORTED: libcpp-has-no-threads // UNSUPPORTED: c++98, c++03 // Index: test/std/thread/futures/futures.tas/futures.task.members/ctor1.fail.cpp === --- test/std/thread/futures/futures.tas/futures.task.members/ctor1.fail.cpp +++
Re: [PATCH] D18149: Add check for unneeded copies of locals
fowles added a comment. thank you! Repository: rL LLVM http://reviews.llvm.org/D18149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r264158 - Use an enum instead of hardcoded indices. NFC.
Author: alexfh Date: Wed Mar 23 09:28:52 2016 New Revision: 264158 URL: http://llvm.org/viewvc/llvm-project?rev=264158&view=rev Log: Use an enum instead of hardcoded indices. NFC. Modified: cfe/trunk/lib/Sema/SemaDecl.cpp Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=264158&r1=264157&r2=264158&view=diff == --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Mar 23 09:28:52 2016 @@ -6423,16 +6423,20 @@ void Sema::CheckShadow(Scope *S, VarDecl } // Determine what kind of declaration we're shadowing. - unsigned Kind; + + // The order must be consistent with the %select in the warning message. + enum ShadowedDeclKind { Local, Global, StaticMember, Field }; + ShadowedDeclKind Kind; if (isa(OldDC)) { if (isa(ShadowedDecl)) - Kind = 3; // field + Kind = Field; else - Kind = 2; // static data member - } else if (OldDC->isFileContext()) -Kind = 1; // global - else -Kind = 0; // local + Kind = StaticMember; + } else if (OldDC->isFileContext()) { +Kind = Global; + } else { +Kind = Local; + } DeclarationName Name = R.getLookupName(); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D18395: -Wshadow: don't warn on ctor parameters with the same name as a field name
alexfh created this revision. alexfh added reviewers: rsmith, rnk. alexfh added a subscriber: cfe-commits. -Wshadow: don't warn on ctor parameters with the same name as a field name. This fixes a broad class of false positives resulting from a widely used pattern: struct A { int q; A(int q) : q(q) {} }; Fixes http://llvm.org/PR16088. http://reviews.llvm.org/D18395 Files: lib/Sema/SemaDecl.cpp test/SemaCXX/warn-shadow.cpp Index: test/SemaCXX/warn-shadow.cpp === --- test/SemaCXX/warn-shadow.cpp +++ test/SemaCXX/warn-shadow.cpp @@ -71,6 +71,14 @@ }; } +// http://llvm.org/PR16088 +namespace PR16088 { +struct S { + int i; + S(int i) : i(i) {} +}; +} + extern int bob; // expected-note {{previous declaration is here}} // rdar://8883302 Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -6406,6 +6406,11 @@ } } + // Don't warn on constructor parameters with the same name as a field name. + if (isa(ShadowedDecl) && isa(NewDC) && + isa(D)) +return; + DeclContext *OldDC = ShadowedDecl->getDeclContext(); // Only warn about certain kinds of shadowing for class members. Index: test/SemaCXX/warn-shadow.cpp === --- test/SemaCXX/warn-shadow.cpp +++ test/SemaCXX/warn-shadow.cpp @@ -71,6 +71,14 @@ }; } +// http://llvm.org/PR16088 +namespace PR16088 { +struct S { + int i; + S(int i) : i(i) {} +}; +} + extern int bob; // expected-note {{previous declaration is here}} // rdar://8883302 Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -6406,6 +6406,11 @@ } } + // Don't warn on constructor parameters with the same name as a field name. + if (isa(ShadowedDecl) && isa(NewDC) && + isa(D)) +return; + DeclContext *OldDC = ShadowedDecl->getDeclContext(); // Only warn about certain kinds of shadowing for class members. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18395: -Wshadow: don't warn on ctor parameters with the same name as a field name
alexfh added a comment. Hm, didn't notice Reid's http://reviews.llvm.org/D18271, which seems to cover this in a better way. http://reviews.llvm.org/D18395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse
Rob created this revision. Rob added reviewers: alexfh, aaron.ballman. Rob added a subscriber: cfe-commits. This patch is to address 2 problems I found with Clang-tidy:modernize-use-override. 1: missing spaces on pure function decls. Orig: void pure() const=0 Problem: void pure() constoverride =0 Fixed: void pure() const override =0 2: This is ms-extension specific, but possibly applies to other attribute types. The override is placed before the attribute which doesn’t work well with declspec as this attribute can be inherited or placed before the method identifier. Orig: class __declspec(dllexport) X : public Y { void p(); }; Problem: class override __declspec(dllexport) class X : public Y { void p(); }; Fixed: class __declspec(dllexport) class X : public Y { void p() override; }; http://reviews.llvm.org/D18396 Files: clang-tidy/modernize/UseOverrideCheck.cpp test/clang-tidy/modernize-use-override-ms.cpp test/clang-tidy/modernize-use-override.cpp Index: test/clang-tidy/modernize-use-override.cpp === --- test/clang-tidy/modernize-use-override.cpp +++ test/clang-tidy/modernize-use-override.cpp @@ -21,6 +21,7 @@ virtual void d2(); virtual void e() = 0; virtual void f() = 0; + virtual void f2() const = 0; virtual void g() = 0; virtual void j() const; @@ -74,7 +75,11 @@ virtual void f()=0; // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using - // CHECK-FIXES: {{^}} void f()override =0; + // CHECK-FIXES: {{^}} void f() override =0; + + virtual void f2() const=0; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using + // CHECK-FIXES: {{^}} void f2() const override =0; virtual void g() ABSTRACT; // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using Index: test/clang-tidy/modernize-use-override-ms.cpp === --- /dev/null +++ test/clang-tidy/modernize-use-override-ms.cpp @@ -0,0 +1,45 @@ +// RUN: %check_clang_tidy %s modernize-use-override %t + +// This attribute type is inherited and precedes the declaration +#define EXPORT __declspec(dllexport) + +class EXPORT ExpBaseMacro { + virtual void a(); +}; + +class __declspec(dllexport) ExpBase { + virtual void a(); +}; + +class BaseMacro { + virtual EXPORT void a(); +}; + +class Base { + virtual __declspec(dllexport) void a(); +}; + + +class EXPORT ExpDerivedMacro : public ExpBaseMacro { + virtual void a(); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override] + // CHECK-FIXES: {{^}} void a() override; +}; + +class __declspec(dllexport) ExpDerived : public ExpBase { + virtual void a(); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using + // CHECK-FIXES: {{^}} void a() override; +}; + +class DerivedMacro : public BaseMacro { + virtual EXPORT void a(); + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: prefer using + // CHECK-FIXES: {{^}} EXPORT void a() override; +}; + +class Derived : public Base { + virtual __declspec(dllexport) void a(); + // CHECK-MESSAGES: :[[@LINE-1]]:38: warning: prefer using + // CHECK-FIXES: {{^}} __declspec(dllexport) void a() override; +}; Index: clang-tidy/modernize/UseOverrideCheck.cpp === --- clang-tidy/modernize/UseOverrideCheck.cpp +++ clang-tidy/modernize/UseOverrideCheck.cpp @@ -118,25 +118,28 @@ if (!HasFinal && !HasOverride) { SourceLocation InsertLoc; StringRef ReplacementText = "override "; +SourceLocation MethodLoc = Method->getLocation(); for (Token T : Tokens) { - if (T.is(tok::kw___attribute)) { + if (T.is(tok::kw___attribute) && + !(Sources.isBeforeInTranslationUnit(T.getLocation(), MethodLoc))) { InsertLoc = T.getLocation(); break; } } if (Method->hasAttrs()) { for (const clang::Attr *A : Method->getAttrs()) { -if (!A->isImplicit()) { +if (!A->isImplicit() && !A->isInherited()) { SourceLocation Loc = Sources.getExpansionLoc(A->getRange().getBegin()); - if (!InsertLoc.isValid() || - Sources.isBeforeInTranslationUnit(Loc, InsertLoc)) + if ((!InsertLoc.isValid() || + Sources.isBeforeInTranslationUnit(Loc, InsertLoc)) && + !Sources.isBeforeInTranslationUnit(Loc, MethodLoc)) InsertLoc = Loc; + } } } -} if (InsertLoc.isInvalid() && Method->doesThisDeclarationHaveABody() && Method->getBody() && !Method->isDefaulted()) { @@ -163,6 +166,9 @@ Tokens.back().is(tok::kw_delete)) && GetText(Tokens[Tokens.size() - 2], Sources) == "=") { InsertLoc = Tokens[Tokens.size() - 2].getLocation(); +// Check if we need to insert a space. +if ((Tokens[Tokens.
Re: [PATCH] D18392: clang-cl: Don't warn about /Oy- being unused in 64-bit builds.
hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm, thanks! http://reviews.llvm.org/D18392 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D17987: [clang-tidy] Extension of checker misc-misplaced-widening-cast
baloghadamsoftware updated this revision to Diff 51418. baloghadamsoftware added a comment. Required fixes done. http://reviews.llvm.org/D17987 Files: clang-tidy/misc/BoolPointerImplicitConversionCheck.cpp clang-tidy/misc/MisplacedWideningCastCheck.cpp clang-tidy/misc/MisplacedWideningCastCheck.h docs/clang-tidy/checks/misc-misplaced-widening-cast.rst test/clang-tidy/misc-misplaced-widening-cast.cpp Index: test/clang-tidy/misc-misplaced-widening-cast.cpp === --- test/clang-tidy/misc-misplaced-widening-cast.cpp +++ test/clang-tidy/misc-misplaced-widening-cast.cpp @@ -1,29 +1,67 @@ -// RUN: %check_clang_tidy %s misc-misplaced-widening-cast %t +// RUN: %check_clang_tidy %s misc-misplaced-widening-cast %t -- -config="{CheckOptions: [{key: misc-misplaced-widening-cast.CheckImplicitCasts, value: 1}]}" -- + +void func(long arg) {} void assign(int a, int b) { long l; l = a * b; - l = (long)(a * b); // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' is ineffective, or there is loss of precision before the conversion [misc-misplaced-widening-cast] + l = (long)(a * b); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' l = (long)a * b; + l = a << 8; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' l = (long)(a << 8); // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' l = (long)b << 8; l = static_cast(a * b); - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' is ineffective, or there is loss of precision before the conversion [misc-misplaced-widening-cast] + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' +} + +void compare(int a, int b, long c) { + bool l; + + l = a * b == c; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' + l = c == a * b; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' + l = (long)(a * b) == c; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' + l = c == (long)(a * b); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' + l = (long)a * b == c; + l = c == (long)a * b; } void init(unsigned int n) { - long l = (long)(n << 8); - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'unsigned int' + long l1 = n << 8; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long' + long l2 = (long)(n << 8); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long' + long l3 = (long)n << 8; +} + +void call(unsigned int n) { + func(n << 8); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long' + func((long)(n << 8)); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long' + func((long)n << 8); } long ret(int a) { - return (long)(a * 1000); - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: either cast from 'int' to 'long' + if (a < 0) { +return a * 1000; +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' + } else if (a > 0) { +return (long)(a * 1000); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long' + } else { +return (long)a * 1000; + } } void dontwarn1(unsigned char a, int i, unsigned char *p) { @@ -33,9 +71,9 @@ // The result is a 12 bit value, there is no truncation in the implicit cast. l = (long)(a << 4); // The result is a 3 bit value, there is no truncation in the implicit cast. - l = (long)((i%5)+1); + l = (long)((i % 5) + 1); // The result is a 16 bit value, there is no truncation in the implicit cast. - l = (long)(((*p)<<8) + *(p+1)); + l = (long)(((*p) << 8) + *(p + 1)); } template struct DontWarn2 { Index: docs/clang-tidy/checks/misc-misplaced-widening-cast.rst === --- docs/clang-tidy/checks/misc-misplaced-widening-cast.rst +++ docs/clang-tidy/checks/misc-misplaced-widening-cast.rst @@ -3,10 +3,10 @@ misc-misplaced-widening-cast -This check will warn when there is a explicit redundant cast of a calculation -result to a bigger type. If the intention of the cast is to avoid loss of -precision then the cast is misplaced, and there can be loss of precision. -Otherwise the cast is ineffective. +This check will warn when there is a cast of a calculation result to a bigger +type. If the intention of the cast is to avoid loss of precision then the cast +is misplaced, and there can be loss of precision. Otherwise the cast is +ineffective. Example code:: @@ -28,6 +28,17 @@ return (long)x * 1000; } +Implicit casts +-- + +Forgetting to place the cast at all is at least as dangerous and at least as +common as misplacin
Re: [PATCH] D16139: [MIPS] initFeatureMap() to handle empty string argument
dsanders added a comment. > > > b) CPUs are not subtarget features (or they shouldn't be), they're CPUs > > > that contain features. They may be generic names for ISAs as well, but > > > probably best to keep them separate. > > > > I agree, we have two separate concepts that happen to use the same > strings. We should probably map them explicitly. > > > Yes, and that's what you should do instead of this patch. Just making the mapping explicit isn't enough by itself. We still have to resolve the empty string to the default CPU because initFeatureMap()'s CPU argument dodges the defaults that are normally handled in the constructor. I'll post patches soon (I've been away for a few days too) I just need to test a couple changes we're agreed on first and sort out some remaining IAS work. > > > c) You should set the features based on the CPUs given to the function. > > > The typical way the cpu comes in, is via -target-cpu which comes via: > > > > > > case llvm::Triple::mips: > > > case llvm::Triple::mipsel: > > > case llvm::Triple::mips64: > > > case llvm::Triple::mips64el: { > > > StringRef CPUName; > > > StringRef ABIName; > > > mips::getMipsCPUAndABI(Args, T, CPUName, ABIName); > > > return CPUName; > > > } > > > > > > for mips. > > > > > > Now if your triple is returning an empty string here you might have > gotten to where you are (I tried mips64r2-linux-gnu as the -target option). > Which is what typically happens down > > > this path. > > > > This usage is from the clang driver. On this path, getMipsCPUAndABI > ensures that the CPU is never empty. > > I gave you a testcase that can prove otherwise in my earlier email. I don't think it proves otherwise. The triple you quoted isn't a supported MIPS triple and LLVM will reject it. LLVM for MIPS doesn't accept CPU names in the first component of the triple and this particular one isn't known to gcc either. Can you give me the exact command you tried? I get this: $ bin/clang -target mips64r2-linux-gnu -o hello.s -S hello.c error: unknown target triple 'mips64r2--linux-gnu', please use -triple or -arch $ bin/llc -mtriple mips64r2-linux-gnu -o hello.s hello.bc bin/llc: : error: unable to get target for 'mips64r2--linux-gnu', see --version and --triple. and if a triple that wasn't mips/mipsel/mips64/mips64el somehow got in to getMipsCPUAndABI() it would trigger an llvm_unreachable(). It's impossible to provide a known MIPS triple and end up with the empty string as the CPU name within the clang driver. The empty string is only known to occur from LLDB's usage. Repository: rL LLVM http://reviews.llvm.org/D16139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D18398: Compilation for Intel MCU (Part 1/3)
aturetsk created this revision. aturetsk added reviewers: rsmith, tra, thakis, ddunbar, bob.wilson, dougk. aturetsk added subscribers: cfe-commits, zinovy.nis, DavidKreitzer. Add -miamcu option which: * Sets IAMCU triple * Sets IAMCU ABI * Enforces static compilation http://reviews.llvm.org/D18398 Files: include/clang/Driver/Options.td lib/Driver/Driver.cpp lib/Driver/Tools.cpp test/Driver/miamcu-opt.c Index: test/Driver/miamcu-opt.c === --- /dev/null +++ test/Driver/miamcu-opt.c @@ -0,0 +1,18 @@ +// RUN: %clang -miamcu %s -### -o %t.o 2>&1 | FileCheck %s +// RUN: %clang -miamcu -m32 %s -### -o %t.o 2>&1 | FileCheck %s +// RUN: %clang -miamcu -target x86_64-unknown-linux-gnu %s -### -o %t.o 2>&1 | FileCheck %s +// RUN: %clang -miamcu -m64 %s -### -o %t.o 2>&1 | FileCheck %s -check-prefix=ERROR +// RUN: %clang -miamcu -dynamic %s -### -o %t.o 2>&1 | FileCheck -check-prefix=CHECK -check-prefix=DYNAMIC %s + +// ERROR: error: invalid argument '-miamcu' not allowed with '-m64' + +// DYNAMIC: warning: argument unused during compilation: '-dynamic' + +// CHECK: "-cc1" +// CHECK: "-triple" "i586-intel-elfiamcu" +// CHECK: "-static-define" +// CHECK: "-mfloat-abi" "soft" +// CHECK: "-mstack-alignment=4" + +// CHECK: bin/gcc +// CHECK: "-static" Index: lib/Driver/Tools.cpp === --- lib/Driver/Tools.cpp +++ lib/Driver/Tools.cpp @@ -2119,6 +2119,13 @@ << A->getOption().getName() << Value; } } + + // Set flags to support MCU ABI. + if (Args.hasArg(options::OPT_miamcu)) { +CmdArgs.push_back("-mfloat-abi"); +CmdArgs.push_back("soft"); +CmdArgs.push_back("-mstack-alignment=4"); + } } void Clang::AddHexagonTargetArgs(const ArgList &Args, Index: lib/Driver/Driver.cpp === --- lib/Driver/Driver.cpp +++ lib/Driver/Driver.cpp @@ -210,6 +210,7 @@ DerivedArgList *Driver::TranslateInputArgs(const InputArgList &Args) const { DerivedArgList *DAL = new DerivedArgList(Args); + bool HasMiamcu = Args.hasArg(options::OPT_miamcu); bool HasNostdlib = Args.hasArg(options::OPT_nostdlib); bool HasNodefaultlib = Args.hasArg(options::OPT_nodefaultlibs); for (Arg *A : Args) { @@ -275,9 +276,19 @@ continue; } +// Ignore -dynamic if -miamcu is present. +if (A->getOption().matches(options::OPT_dynamic) && HasMiamcu) { + Diag(diag::warn_drv_unused_argument) << "-dynamic"; + continue; +} + DAL->append(A); } + // Enforce -static if -miamcu is present. + if (HasMiamcu) +DAL->AddFlagArg(0, Opts->getOption(options::OPT_static)); + // Add a default value of -mlinker-version=, if one was given and the user // didn't specify one. #if defined(HOST_LINK_VERSION) @@ -296,7 +307,8 @@ /// /// This routine provides the logic to compute a target triple from various /// args passed to the driver and the default triple string. -static llvm::Triple computeTargetTriple(StringRef DefaultTargetTriple, +static llvm::Triple computeTargetTriple(const Driver &D, +StringRef DefaultTargetTriple, const ArgList &Args, StringRef DarwinArchName = "") { // FIXME: Already done in Compilation *Driver::BuildCompilation @@ -341,8 +353,9 @@ return Target; // Handle pseudo-target flags '-m64', '-mx32', '-m32' and '-m16'. - if (Arg *A = Args.getLastArg(options::OPT_m64, options::OPT_mx32, - options::OPT_m32, options::OPT_m16)) { + Arg *A = Args.getLastArg(options::OPT_m64, options::OPT_mx32, + options::OPT_m32, options::OPT_m16); + if (A) { llvm::Triple::ArchType AT = llvm::Triple::UnknownArch; if (A->getOption().matches(options::OPT_m64)) { @@ -367,6 +380,21 @@ Target.setArch(AT); } + // Handle -miamcu flag. + if (Args.hasArg(options::OPT_miamcu)) { +if (A && !A->getOption().matches(options::OPT_m32)) + D.Diag(diag::err_drv_argument_not_allowed_with) + << "-miamcu" << A->getBaseArg().getAsString(Args); + +Target.setArch(llvm::Triple::x86); +Target.setArchName("i586"); +Target.setEnvironment(llvm::Triple::UnknownEnvironment); +Target.setEnvironmentName(""); +Target.setOS(llvm::Triple::ELFIAMCU); +Target.setVendor(llvm::Triple::UnknownVendor); +Target.setVendorName("intel"); + } + return Target; } @@ -501,8 +529,8 @@ DerivedArgList *TranslatedArgs = TranslateInputArgs(*UArgs); // Owned by the host. - const ToolChain &TC = - getToolChain(*UArgs, computeTargetTriple(DefaultTargetTriple, *UArgs)); + const ToolChain &TC = getToolChain( + *UArgs, computeTargetTriple(*this, DefaultTargetTriple, *UArgs)); // The compilation takes ownership of Args. Compilation *C = new Compilatio
Re: [PATCH] D18392: clang-cl: Don't warn about /Oy- being unused in 64-bit builds.
thakis closed this revision. thakis added a comment. r264163, thanks! http://reviews.llvm.org/D18392 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r264163 - clang-cl: Don't warn about /Oy- being unused in 64-bit builds.
Author: nico Date: Wed Mar 23 10:37:41 2016 New Revision: 264163 URL: http://llvm.org/viewvc/llvm-project?rev=264163&view=rev Log: clang-cl: Don't warn about /Oy- being unused in 64-bit builds. http://reviews.llvm.org/D18392 Modified: cfe/trunk/lib/Driver/MSVCToolChain.cpp cfe/trunk/test/Driver/cl-options.c Modified: cfe/trunk/lib/Driver/MSVCToolChain.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/MSVCToolChain.cpp?rev=264163&r1=264162&r2=264163&view=diff == --- cfe/trunk/lib/Driver/MSVCToolChain.cpp (original) +++ cfe/trunk/lib/Driver/MSVCToolChain.cpp Wed Mar 23 10:37:41 2016 @@ -702,6 +702,12 @@ static void TranslateOptArg(Arg *A, llvm else DAL.AddFlagArg( A, Opts.getOption(options::OPT_fno_omit_frame_pointer)); + } else { +// Don't warn about /Oy- in 64-bit builds (where +// SupportsForcingFramePointer is false). The flag having no effect +// there is a compiler-internal optimization, and people shouldn't have +// to special-case their build files for 64-bit clang-cl. +A->claim(); } break; } Modified: cfe/trunk/test/Driver/cl-options.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=264163&r1=264162&r2=264163&view=diff == --- cfe/trunk/test/Driver/cl-options.c (original) +++ cfe/trunk/test/Driver/cl-options.c Wed Mar 23 10:37:41 2016 @@ -123,13 +123,13 @@ // PR24003: -momit-leaf-frame-pointer // PR24003: -Os -// RUN: %clang_cl --target=i686-pc-win32 /Oy- /O2 -### -- %s 2>&1 | FileCheck -check-prefix=Oy_2 %s +// RUN: %clang_cl --target=i686-pc-win32 -Werror /Oy- /O2 -### -- %s 2>&1 | FileCheck -check-prefix=Oy_2 %s // Oy_2: -momit-leaf-frame-pointer // Oy_2: -O2 -// RUN: %clang_cl /Zs /Oy -- %s 2>&1 +// RUN: %clang_cl /Zs -Werror /Oy -- %s 2>&1 -// RUN: %clang_cl --target=i686-pc-win32 /Oy- -### -- %s 2>&1 | FileCheck -check-prefix=Oy_ %s +// RUN: %clang_cl --target=i686-pc-win32 -Werror /Oy- -### -- %s 2>&1 | FileCheck -check-prefix=Oy_ %s // Oy_: -mdisable-fp-elim // RUN: %clang_cl /Qvec -### -- %s 2>&1 | FileCheck -check-prefix=Qvec %s ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D18399: Added support for different VFSs in format::getStyle.
ioeric created this revision. ioeric added reviewers: klimek, djasper. ioeric added a subscriber: cfe-commits. Herald added a subscriber: klimek. Previously, format::getStyle assumes that the given file resides in the real file system, which prevents the use of virtual file system in testing etc. This patch adds a parameter in format::getStyle interface so that users can specify the right file system. By default, the file system is the real file system. http://reviews.llvm.org/D18399 Files: include/clang/Format/Format.h lib/Format/Format.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -14,6 +14,7 @@ #include "clang/Frontend/TextDiagnosticPrinter.h" #include "llvm/Support/Debug.h" +#include "llvm/Support/MemoryBuffer.h" #include "gtest/gtest.h" #define DEBUG_TYPE "format-test" @@ -11199,6 +11200,33 @@ verifyFormat("include \"a.td\"\ninclude \"b.td\"", Style); } +TEST(FormatStyle, GetStyleOfFile) { + vfs::InMemoryFileSystem FS; + // Test 1: format file in the same directory. + ASSERT_TRUE( + FS.addFile("/a/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: LLVM"))); + ASSERT_TRUE( + FS.addFile("/a/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); + auto Style1 = getStyle("file", "/a/.clang-format", "Google", &FS); + ASSERT_EQ(Style1, getLLVMStyle()); + + // Test 2: fallback to default. + ASSERT_TRUE( + FS.addFile("/b/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); + auto Style2 = getStyle("file", "/b/test.cpp", "Mozilla", &FS); + ASSERT_EQ(Style2, getMozillaStyle()); + + // Test 3: format file in parent directory. + ASSERT_TRUE( + FS.addFile("/c/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google"))); + ASSERT_TRUE(FS.addFile("/c/sub/sub/sub/test.cpp", 0, + llvm::MemoryBuffer::getMemBuffer("int i;"))); + auto Style3 = getStyle("file", "/c/sub/sub/sub/test.cpp", "LLVM", &FS); + ASSERT_EQ(Style3, getGoogleStyle()); +} + class ReplacementTest : public ::testing::Test { protected: tooling::Replacement createReplacement(SourceLocation Start, unsigned Length, Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -2098,7 +2098,11 @@ } FormatStyle getStyle(StringRef StyleName, StringRef FileName, - StringRef FallbackStyle) { + StringRef FallbackStyle, vfs::FileSystem *FS) { + if (!FS) { +// If FS is not specified, the default file system is the real file system. +FS = vfs::getRealFileSystem().get(); + } FormatStyle Style = getLLVMStyle(); Style.Language = getLanguageByFileName(FileName); if (!getPredefinedStyle(FallbackStyle, Style.Language, &Style)) { @@ -2129,28 +2133,35 @@ llvm::sys::fs::make_absolute(Path); for (StringRef Directory = Path; !Directory.empty(); Directory = llvm::sys::path::parent_path(Directory)) { -if (!llvm::sys::fs::is_directory(Directory)) + +auto Status = FS->status(Directory); +if (!Status || +Status->getType() != llvm::sys::fs::file_type::directory_file) { continue; +} + SmallString<128> ConfigFile(Directory); llvm::sys::path::append(ConfigFile, ".clang-format"); DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n"); -bool IsFile = false; // Ignore errors from is_regular_file: we only need to know if we can read // the file or not. -llvm::sys::fs::is_regular_file(Twine(ConfigFile), IsFile); - +auto FileStatus = FS->status(ConfigFile.c_str()); +bool IsFile = FileStatus && (FileStatus->getType() == + llvm::sys::fs::file_type::regular_file); if (!IsFile) { // Try _clang-format too, since dotfiles are not commonly used on Windows. ConfigFile = Directory; llvm::sys::path::append(ConfigFile, "_clang-format"); DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n"); - llvm::sys::fs::is_regular_file(Twine(ConfigFile), IsFile); + auto Status = FS->status(Twine(ConfigFile)); + IsFile = Status && + (Status->getType() == llvm::sys::fs::file_type::regular_file); } if (IsFile) { llvm::ErrorOr> Text = - llvm::MemoryBuffer::getFile(ConfigFile.c_str()); + FS->getBufferForFile(ConfigFile.c_str()); if (std::error_code EC = Text.getError()) { llvm::errs() << EC.message() << "\n"; break; Index: include/clang/Format/Format.h === --- include/clang/Format/Format.h +++ include/clang/Format/Format.h @@ -16,6 +16,7 @@ #define LLVM_CLANG_FORMAT_FORMAT_H #include "clang/Basic/LangOptions.h" +#includ
Re: [PATCH] D18399: Added support for different VFSs in format::getStyle.
klimek added inline comments. Comment at: lib/Format/Format.cpp:2103 @@ +2102,3 @@ + if (!FS) { +// If FS is not specified, the default file system is the real file system. +FS = vfs::getRealFileSystem().get(); I think this comment is redundant. I'd remove it. Comment at: lib/Format/Format.cpp:2149 @@ -2140,3 +2148,3 @@ // the file or not. -llvm::sys::fs::is_regular_file(Twine(ConfigFile), IsFile); - +auto FileStatus = FS->status(ConfigFile.c_str()); +bool IsFile = FileStatus && (FileStatus->getType() == Any reason to not just reassign Status (here and below)? Comment at: lib/Format/Format.cpp:2157 @@ -2147,2 +2156,3 @@ DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n"); - llvm::sys::fs::is_regular_file(Twine(ConfigFile), IsFile); + auto Status = FS->status(Twine(ConfigFile)); + IsFile = Status && I'm aware you didn't change this, but is Twine() really necessary? http://reviews.llvm.org/D18399 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16993: Add missing __builtin_bitreverse8
rsmith added a subscriber: rsmith. rsmith accepted this revision. rsmith added a reviewer: rsmith. This revision is now accepted and ready to land. Comment at: docs/LanguageExtensions.rst:1533 @@ +1532,3 @@ +the bitpattern of an integer value; for example ``0b1234567`` becomes +``0b7654321``. + This example doesn't make much sense: it looks like it's bit reversing a 7 bit number containing non-binary digits? Can you provide a real example instead? http://reviews.llvm.org/D16993 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18399: Added support for different VFSs in format::getStyle.
ioeric updated this revision to Diff 51426. ioeric added a comment. - removed redundant code. http://reviews.llvm.org/D18399 Files: include/clang/Format/Format.h lib/Format/Format.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -14,6 +14,7 @@ #include "clang/Frontend/TextDiagnosticPrinter.h" #include "llvm/Support/Debug.h" +#include "llvm/Support/MemoryBuffer.h" #include "gtest/gtest.h" #define DEBUG_TYPE "format-test" @@ -11199,6 +11200,33 @@ verifyFormat("include \"a.td\"\ninclude \"b.td\"", Style); } +TEST(FormatStyle, GetStyleOfFile) { + vfs::InMemoryFileSystem FS; + // Test 1: format file in the same directory. + ASSERT_TRUE( + FS.addFile("/a/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: LLVM"))); + ASSERT_TRUE( + FS.addFile("/a/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); + auto Style1 = getStyle("file", "/a/.clang-format", "Google", &FS); + ASSERT_EQ(Style1, getLLVMStyle()); + + // Test 2: fallback to default. + ASSERT_TRUE( + FS.addFile("/b/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); + auto Style2 = getStyle("file", "/b/test.cpp", "Mozilla", &FS); + ASSERT_EQ(Style2, getMozillaStyle()); + + // Test 3: format file in parent directory. + ASSERT_TRUE( + FS.addFile("/c/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google"))); + ASSERT_TRUE(FS.addFile("/c/sub/sub/sub/test.cpp", 0, + llvm::MemoryBuffer::getMemBuffer("int i;"))); + auto Style3 = getStyle("file", "/c/sub/sub/sub/test.cpp", "LLVM", &FS); + ASSERT_EQ(Style3, getGoogleStyle()); +} + class ReplacementTest : public ::testing::Test { protected: tooling::Replacement createReplacement(SourceLocation Start, unsigned Length, Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -2098,7 +2098,10 @@ } FormatStyle getStyle(StringRef StyleName, StringRef FileName, - StringRef FallbackStyle) { + StringRef FallbackStyle, vfs::FileSystem *FS) { + if (!FS) { +FS = vfs::getRealFileSystem().get(); + } FormatStyle Style = getLLVMStyle(); Style.Language = getLanguageByFileName(FileName); if (!getPredefinedStyle(FallbackStyle, Style.Language, &Style)) { @@ -2129,28 +2132,35 @@ llvm::sys::fs::make_absolute(Path); for (StringRef Directory = Path; !Directory.empty(); Directory = llvm::sys::path::parent_path(Directory)) { -if (!llvm::sys::fs::is_directory(Directory)) + +auto Status = FS->status(Directory); +if (!Status || +Status->getType() != llvm::sys::fs::file_type::directory_file) { continue; +} + SmallString<128> ConfigFile(Directory); llvm::sys::path::append(ConfigFile, ".clang-format"); DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n"); -bool IsFile = false; // Ignore errors from is_regular_file: we only need to know if we can read // the file or not. -llvm::sys::fs::is_regular_file(Twine(ConfigFile), IsFile); - +auto FileStatus = FS->status(ConfigFile.c_str()); +bool IsFile = FileStatus && (FileStatus->getType() == + llvm::sys::fs::file_type::regular_file); if (!IsFile) { // Try _clang-format too, since dotfiles are not commonly used on Windows. ConfigFile = Directory; llvm::sys::path::append(ConfigFile, "_clang-format"); DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n"); - llvm::sys::fs::is_regular_file(Twine(ConfigFile), IsFile); + Status = FS->status(ConfigFile.c_str()); + IsFile = Status && + (Status->getType() == llvm::sys::fs::file_type::regular_file); } if (IsFile) { llvm::ErrorOr> Text = - llvm::MemoryBuffer::getFile(ConfigFile.c_str()); + FS->getBufferForFile(ConfigFile.c_str()); if (std::error_code EC = Text.getError()) { llvm::errs() << EC.message() << "\n"; break; Index: include/clang/Format/Format.h === --- include/clang/Format/Format.h +++ include/clang/Format/Format.h @@ -16,6 +16,7 @@ #define LLVM_CLANG_FORMAT_FORMAT_H #include "clang/Basic/LangOptions.h" +#include "clang/Basic/VirtualFileSystem.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/ArrayRef.h" #include @@ -832,11 +833,13 @@ /// == "file". /// \param[in] FallbackStyle The name of a predefined style used to fallback to /// in case the style can't be determined from \p StyleName. +/// \param[in] FS The underlying file system, in which the file resides. By +/// default, the file system is the real file system. /// /// \ret
r264164 - [analyzer] Fix typo s/initalize/initialize/
Author: chh Date: Wed Mar 23 11:14:12 2016 New Revision: 264164 URL: http://llvm.org/viewvc/llvm-project?rev=264164&view=rev Log: [analyzer] Fix typo s/initalize/initialize/ Differential Revision: http://reviews.llvm.org/D18363 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp?rev=264164&r1=264163&r2=264164&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp Wed Mar 23 11:14:12 2016 @@ -313,7 +313,7 @@ void CallAndMessageChecker::checkPreStmt if (L.isUndef()) { if (!BT_call_undef) BT_call_undef.reset(new BuiltinBug( - this, "Called function pointer is an uninitalized pointer value")); + this, "Called function pointer is an uninitialized pointer value")); emitBadCall(BT_call_undef.get(), C, Callee); return; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18363: Fix typo s/initalize/initialize/
This revision was automatically updated to reflect the committed changes. Closed by commit rL264164: [analyzer] Fix typo s/initalize/initialize/ (authored by chh). Changed prior to commit: http://reviews.llvm.org/D18363?vs=51307&id=51429#toc Repository: rL LLVM http://reviews.llvm.org/D18363 Files: cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp === --- cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -313,7 +313,7 @@ if (L.isUndef()) { if (!BT_call_undef) BT_call_undef.reset(new BuiltinBug( - this, "Called function pointer is an uninitalized pointer value")); + this, "Called function pointer is an uninitialized pointer value")); emitBadCall(BT_call_undef.get(), C, Callee); return; } Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp === --- cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -313,7 +313,7 @@ if (L.isUndef()) { if (!BT_call_undef) BT_call_undef.reset(new BuiltinBug( - this, "Called function pointer is an uninitalized pointer value")); + this, "Called function pointer is an uninitialized pointer value")); emitBadCall(BT_call_undef.get(), C, Callee); return; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18399: Added support for different VFSs in format::getStyle.
ioeric updated this revision to Diff 51431. ioeric marked 3 inline comments as done. ioeric added a comment. - some more redundancy removed http://reviews.llvm.org/D18399 Files: include/clang/Format/Format.h lib/Format/Format.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -14,6 +14,7 @@ #include "clang/Frontend/TextDiagnosticPrinter.h" #include "llvm/Support/Debug.h" +#include "llvm/Support/MemoryBuffer.h" #include "gtest/gtest.h" #define DEBUG_TYPE "format-test" @@ -11199,6 +11200,33 @@ verifyFormat("include \"a.td\"\ninclude \"b.td\"", Style); } +TEST(FormatStyle, GetStyleOfFile) { + vfs::InMemoryFileSystem FS; + // Test 1: format file in the same directory. + ASSERT_TRUE( + FS.addFile("/a/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: LLVM"))); + ASSERT_TRUE( + FS.addFile("/a/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); + auto Style1 = getStyle("file", "/a/.clang-format", "Google", &FS); + ASSERT_EQ(Style1, getLLVMStyle()); + + // Test 2: fallback to default. + ASSERT_TRUE( + FS.addFile("/b/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); + auto Style2 = getStyle("file", "/b/test.cpp", "Mozilla", &FS); + ASSERT_EQ(Style2, getMozillaStyle()); + + // Test 3: format file in parent directory. + ASSERT_TRUE( + FS.addFile("/c/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google"))); + ASSERT_TRUE(FS.addFile("/c/sub/sub/sub/test.cpp", 0, + llvm::MemoryBuffer::getMemBuffer("int i;"))); + auto Style3 = getStyle("file", "/c/sub/sub/sub/test.cpp", "LLVM", &FS); + ASSERT_EQ(Style3, getGoogleStyle()); +} + class ReplacementTest : public ::testing::Test { protected: tooling::Replacement createReplacement(SourceLocation Start, unsigned Length, Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -2098,7 +2098,10 @@ } FormatStyle getStyle(StringRef StyleName, StringRef FileName, - StringRef FallbackStyle) { + StringRef FallbackStyle, vfs::FileSystem *FS) { + if (!FS) { +FS = vfs::getRealFileSystem().get(); + } FormatStyle Style = getLLVMStyle(); Style.Language = getLanguageByFileName(FileName); if (!getPredefinedStyle(FallbackStyle, Style.Language, &Style)) { @@ -2129,28 +2132,35 @@ llvm::sys::fs::make_absolute(Path); for (StringRef Directory = Path; !Directory.empty(); Directory = llvm::sys::path::parent_path(Directory)) { -if (!llvm::sys::fs::is_directory(Directory)) + +auto Status = FS->status(Directory); +if (!Status || +Status->getType() != llvm::sys::fs::file_type::directory_file) { continue; +} + SmallString<128> ConfigFile(Directory); llvm::sys::path::append(ConfigFile, ".clang-format"); DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n"); -bool IsFile = false; // Ignore errors from is_regular_file: we only need to know if we can read // the file or not. -llvm::sys::fs::is_regular_file(Twine(ConfigFile), IsFile); - +Status = FS->status(ConfigFile.c_str()); +bool IsFile = +Status && (Status->getType() == llvm::sys::fs::file_type::regular_file); if (!IsFile) { // Try _clang-format too, since dotfiles are not commonly used on Windows. ConfigFile = Directory; llvm::sys::path::append(ConfigFile, "_clang-format"); DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n"); - llvm::sys::fs::is_regular_file(Twine(ConfigFile), IsFile); + Status = FS->status(ConfigFile.c_str()); + IsFile = Status && + (Status->getType() == llvm::sys::fs::file_type::regular_file); } if (IsFile) { llvm::ErrorOr> Text = - llvm::MemoryBuffer::getFile(ConfigFile.c_str()); + FS->getBufferForFile(ConfigFile.c_str()); if (std::error_code EC = Text.getError()) { llvm::errs() << EC.message() << "\n"; break; Index: include/clang/Format/Format.h === --- include/clang/Format/Format.h +++ include/clang/Format/Format.h @@ -16,6 +16,7 @@ #define LLVM_CLANG_FORMAT_FORMAT_H #include "clang/Basic/LangOptions.h" +#include "clang/Basic/VirtualFileSystem.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/ArrayRef.h" #include @@ -832,11 +833,13 @@ /// == "file". /// \param[in] FallbackStyle The name of a predefined style used to fallback to /// in case the style can't be determined from \p StyleName. +/// \param[in] FS The underlying file system, in which the file resides. By +/// default, the file system is the real file system. /// ///
Re: [PATCH] D17451: PR26448: [Sema] Fix determined type of ternary operator acting on two xvalue class types
erik.pilkington added a comment. ping! http://reviews.llvm.org/D17451 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D18401: clang-cl: Include /FI headers in /showIncludes output.
thakis created this revision. thakis added a reviewer: hans. thakis added a subscriber: cfe-commits. `-H` in gcc mode doesn't print `-include` headers, but they are included in depfiles written by MMD and friends. Since `/showIncludes` is what's used instead of depfiles, printing `/FI` there seems important (and matches cl.exe). There's more work to do for the interaction of `/FI`, `/showIncludes`, and PCH flags, but this is a strict improvement over what we have today. Instead of giving HeaderIncludeGen more options, another approach would be to just switch on ShowAllHeaders in clang-cl mode and let clang::InitializePreprocessor() not put `-include` flags in the `` block. This would change the behavior of `-E` slightly, and it would remove the `` flag from the output triggered by setting the obscure `CC_PRINT_HEADERS=1` env var to true while running clang. So don't do that as it would change behavior. (But if someone reading this thinks that's ok to change, I'd prefer doing that, as it makes things simpler.) http://reviews.llvm.org/D18401 Files: include/clang/Frontend/Utils.h lib/Frontend/CompilerInstance.cpp lib/Frontend/HeaderIncludeGen.cpp test/Driver/cl-pch-showincludes.cpp test/Frontend/print-header-includes.c Index: test/Frontend/print-header-includes.c === --- test/Frontend/print-header-includes.c +++ test/Frontend/print-header-includes.c @@ -1,24 +1,24 @@ -// RUN: cd %S -// RUN: %clang_cc1 -include Inputs/test3.h -E -H -o %t.out %s 2> %t.stderr +// RUN: %clang_cc1 -I%S -include Inputs/test3.h -E -H -o /dev/null %s 2> %t.stderr // RUN: FileCheck < %t.stderr %s // CHECK-NOT: test3.h // CHECK: . {{.*test.h}} // CHECK: .. {{.*test2.h}} -// RUN: %clang_cc1 -include Inputs/test3.h -E --show-includes -o %t.out %s > %t.stdout -// RUN: FileCheck --check-prefix=MS < %t.stdout %s -// MS-NOT: test3.h -// MS: Note: including file: {{.*test.h}} -// MS: Note: including file: {{.*test2.h}} +// RUN: %clang_cc1 -I%S -include Inputs/test3.h -E --show-includes -o /dev/null %s | \ +// RUN: FileCheck --strict-whitespace --check-prefix=MS %s +// MS-NOT: +// MS: Note: including file: {{[^ ]*test3.h}} +// MS: Note: including file: {{[^ ]*test.h}} +// MS: Note: including file: {{[^ ]*test2.h}} // MS-NOT: Note // RUN: echo "fun:foo" > %t.blacklist -// RUN: %clang_cc1 -fsanitize=address -fdepfile-entry=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout -// RUN: FileCheck --check-prefix=MS-BLACKLIST < %t.stdout %s -// MS-BLACKLIST: Note: including file: {{.*\.blacklist}} -// MS-BLACKLIST: Note: including file: {{.*test.h}} -// MS-BLACKLIST: Note: including file: {{.*test2.h}} +// RUN: %clang_cc1 -I%S -fsanitize=address -fdepfile-entry=%t.blacklist -E --show-includes -o /dev/null %s | \ +// RUN: FileCheck --strict-whitespace --check-prefix=MS-BLACKLIST %s +// MS-BLACKLIST: Note: including file: {{[^ ]*\.blacklist}} +// MS-BLACKLIST: Note: including file: {{[^ ]*test.h}} +// MS-BLACKLIST: Note: including file: {{[^ ]*test2.h}} // MS-BLACKLIST-NOT: Note #include "Inputs/test.h" Index: test/Driver/cl-pch-showincludes.cpp === --- test/Driver/cl-pch-showincludes.cpp +++ test/Driver/cl-pch-showincludes.cpp @@ -8,16 +8,17 @@ // When building the pch, header1.h (included by header2.h), header2.h (the pch // input itself) and header3.h (included directly, above) should be printed. -// RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Ycheader2.h /FIheader2.h /Fp%t.pch /c /Fo%t -- %s 2>&1 \ -// RUN: | FileCheck -check-prefix=CHECK-YC %s -// CHECK-YC: Note: including file: {{.+header2.h}} -// CHECK-YC: Note: including file: {{.+header1.h}} -// CHECK-YC: Note: including file: {{.+header3.h}} +// RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Ycheader2.h /FIheader2.h /Fp%t.pch /c /Fo%t -- %s \ +// RUN: | FileCheck --strict-whitespace -check-prefix=CHECK-YC %s +// CHECK-YC: Note: including file: {{[^ ]*header2.h}} +// FIXME: header1.h should be indented one more: +// CHECK-YC: Note: including file: {{[^ ]*header1.h}} +// CHECK-YC: Note: including file: {{[^ ]*header3.h}} // When using the pch, only the direct include is printed. -// RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Yuheader2.h /FIheader2.h /Fp%t.pch /c /Fo%t -- %s 2>&1 \ -// RUN: | FileCheck -check-prefix=CHECK-YU %s +// RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Yuheader2.h /FIheader2.h /Fp%t.pch /c /Fo%t -- %s \ +// RUN: | FileCheck --strict-whitespace -check-prefix=CHECK-YU %s // CHECK-YU-NOT: Note: including file: {{.*pch}} // CHECK-YU-NOT: Note: including file: {{.*header1.h}} // CHECK-YU-NOT: Note: including file: {{.*header2.h}} -// CHECK-YU: Note: including file: {{.+header3.h}} +// CHECK-YU: Note: including file: {{[^ ]*header3.h}} Index: lib/Frontend/HeaderIncludeGen.cpp === --- lib/Fronte
Re: [PATCH] D17852: Added formatAndApplyAllReplacements that works on multiple files in libTooling.
ioeric updated this revision to Diff 51433. ioeric marked 3 inline comments as done. ioeric added a comment. - removed format::getStyle test case from ToolingTests; http://reviews.llvm.org/D17852 Files: include/clang/Basic/SourceManager.h include/clang/Format/Format.h include/clang/Tooling/Core/Replacement.h include/clang/Tooling/Refactoring.h lib/Format/Format.cpp lib/Tooling/CMakeLists.txt lib/Tooling/Core/Replacement.cpp lib/Tooling/Refactoring.cpp unittests/Format/FormatTest.cpp unittests/Tooling/CMakeLists.txt unittests/Tooling/RefactoringTest.cpp Index: unittests/Tooling/RefactoringTest.cpp === --- unittests/Tooling/RefactoringTest.cpp +++ unittests/Tooling/RefactoringTest.cpp @@ -18,6 +18,7 @@ #include "clang/Basic/FileManager.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceManager.h" +#include "clang/Format/Format.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendAction.h" #include "clang/Frontend/TextDiagnosticPrinter.h" @@ -166,6 +167,41 @@ EXPECT_EQ("z", Context.getRewrittenText(IDz)); } +TEST_F(ReplacementTest, MultipleFilesReplaceAndFormat) { + // Column limit is 20. + std::string Code1 = "Long *a =\n" + "new Long();\n" + "long x = 1;"; + std::string Expected1 = "auto a = new Long();\n" + "long x =\n" + "12345678901;"; + std::string Code2 = "int x = 123;\n" + "int y = 0;"; + std::string Expected2 = "int x =\n" + "1234567890123;\n" + "int y = 10;"; + FileID ID1 = Context.createInMemoryFile("format_1.cpp", Code1); + FileID ID2 = Context.createInMemoryFile("format_2.cpp", Code2); + + tooling::Replacements Replaces; + // Scrambled the order of replacements. + Replaces.insert(tooling::Replacement( + Context.Sources, Context.getLocation(ID2, 1, 12), 0, "4567890123")); + Replaces.insert(tooling::Replacement( + Context.Sources, Context.getLocation(ID1, 1, 1), 6, "auto ")); + Replaces.insert(tooling::Replacement( + Context.Sources, Context.getLocation(ID2, 2, 9), 1, "10")); + Replaces.insert(tooling::Replacement( + Context.Sources, Context.getLocation(ID1, 3, 10), 1, "12345678901")); + + format::FormatStyle Style = format::getLLVMStyle(); + Style.ColumnLimit = 20; // Set column limit to 20 to increase readibility. + + EXPECT_TRUE(formatAndApplyAllReplacements(Replaces, Context.Rewrite, Style)); + EXPECT_EQ(Expected1, Context.getRewrittenText(ID1)); + EXPECT_EQ(Expected2, Context.getRewrittenText(ID2)); +} + TEST(ShiftedCodePositionTest, FindsNewCodePosition) { Replacements Replaces; Replaces.insert(Replacement("", 0, 1, "")); @@ -426,7 +462,7 @@ Replaces.insert(Replacement("foo", 10, 1, "zz")); Replaces.insert(Replacement("foo", 11, 0, "")); - std::vector Ranges = calculateChangedRangesInFile(Replaces); + std::vector Ranges = calculateChangedRanges(Replaces); EXPECT_EQ(3ul, Ranges.size()); EXPECT_TRUE(Ranges[0].getOffset() == 0); Index: unittests/Tooling/CMakeLists.txt === --- unittests/Tooling/CMakeLists.txt +++ unittests/Tooling/CMakeLists.txt @@ -30,6 +30,7 @@ clangAST clangASTMatchers clangBasic + clangFormat clangFrontend clangLex clangRewrite Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -11234,7 +11234,8 @@ format::FormatStyle Style = format::getLLVMStyle(); Style.ColumnLimit = 20; // Set column limit to 20 to increase readibility. - EXPECT_EQ(Expected, applyAllReplacementsAndFormat(Code, Replaces, Style)); + EXPECT_EQ(Expected, applyAllReplacements( + Code, formatReplacements(Code, Replaces, Style))); } } // end namespace Index: lib/Tooling/Refactoring.cpp === --- lib/Tooling/Refactoring.cpp +++ lib/Tooling/Refactoring.cpp @@ -14,6 +14,7 @@ #include "clang/Basic/DiagnosticOptions.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/SourceManager.h" +#include "clang/Format/Format.h" #include "clang/Frontend/TextDiagnosticPrinter.h" #include "clang/Lex/Lexer.h" #include "clang/Rewrite/Core/Rewriter.h" @@ -61,5 +62,41 @@ return Rewrite.overwriteChangedFiles() ? 1 : 0; } +static bool formatAndApply(const Replacements &Replaces, Rewriter &Rewrite, + const format::FormatStyle *Style) { + SourceManager &SM = Rewrite.getSourceMgr(); + FileManager &Files = SM.getFileManager(); + + auto FileToReplaces = groupReplacementsByFile(Replaces); + + bool Result = true; + for (auto &FileAndReplaces : FileToReplaces) { +const std::string F
Re: [PATCH] D18395: -Wshadow: don't warn on ctor parameters with the same name as a field name
Reid sent out a different patch for this warning improvement. Have you checked that one out? Is it abandoned? I'm still a bit concerned about whitelisting all these cases & not catching cases where the parameter is then used inside the function in some problematic way (the classic being a unique_ptr parameter, moved into a member, then the parameter (instead of the member) is referenced in the body of the function). On Wed, Mar 23, 2016 at 7:43 AM, Alexander Kornienko via cfe-commits < cfe-commits@lists.llvm.org> wrote: > alexfh created this revision. > alexfh added reviewers: rsmith, rnk. > alexfh added a subscriber: cfe-commits. > > -Wshadow: don't warn on ctor parameters with the same name as a field > name. This fixes a broad class of false positives resulting from a widely > used > pattern: > > struct A { > int q; > A(int q) : q(q) {} > }; > > Fixes http://llvm.org/PR16088. > > http://reviews.llvm.org/D18395 > > Files: > lib/Sema/SemaDecl.cpp > test/SemaCXX/warn-shadow.cpp > > Index: test/SemaCXX/warn-shadow.cpp > === > --- test/SemaCXX/warn-shadow.cpp > +++ test/SemaCXX/warn-shadow.cpp > @@ -71,6 +71,14 @@ > }; > } > > +// http://llvm.org/PR16088 > +namespace PR16088 { > +struct S { > + int i; > + S(int i) : i(i) {} > +}; > +} > + > extern int bob; // expected-note {{previous declaration is here}} > > // rdar://8883302 > Index: lib/Sema/SemaDecl.cpp > === > --- lib/Sema/SemaDecl.cpp > +++ lib/Sema/SemaDecl.cpp > @@ -6406,6 +6406,11 @@ > } > } > > + // Don't warn on constructor parameters with the same name as a field > name. > + if (isa(ShadowedDecl) && isa(NewDC) && > + isa(D)) > +return; > + >DeclContext *OldDC = ShadowedDecl->getDeclContext(); > >// Only warn about certain kinds of shadowing for class members. > > > > ___ > 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] D18395: -Wshadow: don't warn on ctor parameters with the same name as a field name
Ah, now I see your follow-up email. Soryr I missed it... On Wed, Mar 23, 2016 at 9:30 AM, David Blaikie wrote: > Reid sent out a different patch for this warning improvement. Have you > checked that one out? Is it abandoned? > > I'm still a bit concerned about whitelisting all these cases & not > catching cases where the parameter is then used inside the function in some > problematic way (the classic being a unique_ptr parameter, moved into a > member, then the parameter (instead of the member) is referenced in the > body of the function). > > On Wed, Mar 23, 2016 at 7:43 AM, Alexander Kornienko via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> alexfh created this revision. >> alexfh added reviewers: rsmith, rnk. >> alexfh added a subscriber: cfe-commits. >> >> -Wshadow: don't warn on ctor parameters with the same name as a field >> name. This fixes a broad class of false positives resulting from a widely >> used >> pattern: >> >> struct A { >> int q; >> A(int q) : q(q) {} >> }; >> >> Fixes http://llvm.org/PR16088. >> >> http://reviews.llvm.org/D18395 >> >> Files: >> lib/Sema/SemaDecl.cpp >> test/SemaCXX/warn-shadow.cpp >> >> Index: test/SemaCXX/warn-shadow.cpp >> === >> --- test/SemaCXX/warn-shadow.cpp >> +++ test/SemaCXX/warn-shadow.cpp >> @@ -71,6 +71,14 @@ >> }; >> } >> >> +// http://llvm.org/PR16088 >> +namespace PR16088 { >> +struct S { >> + int i; >> + S(int i) : i(i) {} >> +}; >> +} >> + >> extern int bob; // expected-note {{previous declaration is here}} >> >> // rdar://8883302 >> Index: lib/Sema/SemaDecl.cpp >> === >> --- lib/Sema/SemaDecl.cpp >> +++ lib/Sema/SemaDecl.cpp >> @@ -6406,6 +6406,11 @@ >> } >> } >> >> + // Don't warn on constructor parameters with the same name as a field >> name. >> + if (isa(ShadowedDecl) && isa(NewDC) && >> + isa(D)) >> +return; >> + >>DeclContext *OldDC = ShadowedDecl->getDeclContext(); >> >>// Only warn about certain kinds of shadowing for class members. >> >> >> >> ___ >> 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
r264167 - ObjC: Handle boolean fixed type for enum.
Author: mren Date: Wed Mar 23 11:28:28 2016 New Revision: 264167 URL: http://llvm.org/viewvc/llvm-project?rev=264167&view=rev Log: ObjC: Handle boolean fixed type for enum. Before this commit, we assert failure in ImplicitCastExpr "unheralded conversion to bool". This commit fixes the assertion by using the correct cast type when the fixed type is boolean. This commit also fixes the behavior for Microsoft mode as well, since Obj-C and Microsoft mode share the same code path. rdar://24999533 Modified: cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/test/SemaObjC/enum-fixed-type.m Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=264167&r1=264166&r2=264167&view=diff == --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Mar 23 11:28:28 2016 @@ -14146,7 +14146,10 @@ EnumConstantDecl *Sema::CheckEnumConstan } else Diag(IdLoc, diag::err_enumerator_too_large) << EltTy; } else -Val = ImpCastExprToType(Val, EltTy, CK_IntegralCast).get(); +Val = ImpCastExprToType(Val, EltTy, +EltTy->isBooleanType() ? +CK_IntegralToBoolean : CK_IntegralCast) +.get(); } else if (getLangOpts().CPlusPlus) { // C++11 [dcl.enum]p5: // If the underlying type is not fixed, the type of each enumerator Modified: cfe/trunk/test/SemaObjC/enum-fixed-type.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/enum-fixed-type.m?rev=264167&r1=264166&r2=264167&view=diff == --- cfe/trunk/test/SemaObjC/enum-fixed-type.m (original) +++ cfe/trunk/test/SemaObjC/enum-fixed-type.m Wed Mar 23 11:28:28 2016 @@ -38,3 +38,8 @@ int arr3[(long long)Bar == (long long)-1 typedef enum : Integer { BaseElem } BaseEnum; typedef enum : BaseEnum { DerivedElem } DerivedEnum; // expected-error {{non-integral type 'BaseEnum' is an invalid underlying type}} + +// +enum MyEnum : _Bool { + MyThing = 0 +}; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18396: Clang-tidy:modernize-use-override. Fix for __declspec attributes and const=0 without spacse
alexfh added a comment. Thank you for the patch! Looks good in general. A few nits. Comment at: clang-tidy/modernize/UseOverrideCheck.cpp:125 @@ -124,1 +124,3 @@ + if (T.is(tok::kw___attribute) && + !(Sources.isBeforeInTranslationUnit(T.getLocation(), MethodLoc))) { InsertLoc = T.getLocation(); nit: no need for the parentheses around the function call. Comment at: clang-tidy/modernize/UseOverrideCheck.cpp:136 @@ -133,3 +135,3 @@ Sources.getExpansionLoc(A->getRange().getBegin()); - if (!InsertLoc.isValid() || - Sources.isBeforeInTranslationUnit(Loc, InsertLoc)) + if ((!InsertLoc.isValid() || + Sources.isBeforeInTranslationUnit(Loc, InsertLoc)) && Please clang-format this change. Comment at: test/clang-tidy/modernize-use-override-ms.cpp:1 @@ +1,2 @@ +// RUN: %check_clang_tidy %s modernize-use-override %t + Does this test pass on linux? If no additional compiler arguments needed and it just works, maybe just merge this test code to the main test file? http://reviews.llvm.org/D18396 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18243: [ASTMatchers] Existing matcher hasAnyArgument fixed
alexfh added inline comments. Comment at: docs/ReleaseNotes.rst:123 @@ +122,3 @@ +- hasAnyArgument: Matcher no longer ignores parentheses and implicit casts on + the argument before applying the inner matcher. The fix was done allow for + greater control by the user. In all existing checkers that use this matcher nit: The sentence doesn't parse. Did you mean "The fix allows" or "The fix was done to allow"? Comment at: docs/ReleaseNotes.rst:125 @@ +124,3 @@ + greater control by the user. In all existing checkers that use this matcher + all instances of code hasAnyArgument() must be changed to + hasAnyArgument(ignoringParenImpCasts()). nit: Please enclose inline code snippets in double backquotes. http://reviews.llvm.org/D18243 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18264: [clang-tidy] misc-assign-operator-signature checker checks return value of all assign operators
baloghadamsoftware updated this revision to Diff 51435. baloghadamsoftware added a comment. Check for non copy and move assign operators made optional. http://reviews.llvm.org/D18264 Files: clang-tidy/misc/AssignOperatorSignatureCheck.cpp clang-tidy/misc/AssignOperatorSignatureCheck.h test/clang-tidy/misc-assign-operator-signature.cpp Index: test/clang-tidy/misc-assign-operator-signature.cpp === --- test/clang-tidy/misc-assign-operator-signature.cpp +++ test/clang-tidy/misc-assign-operator-signature.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s misc-assign-operator-signature %t +// RUN: %check_clang_tidy %s misc-assign-operator-signature %t -- -config="{CheckOptions: [{key: misc-assign-operator-signature.CheckAllAssignOperators, value: 1}]}" -- struct Good { Good& operator=(const Good&); @@ -18,6 +18,8 @@ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'BadReturn&' [misc-assign-operator-signature] const BadReturn& operator=(BadReturn&&); // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'Bad + void operator=(int); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 'Bad }; struct BadReturn2 { BadReturn2&& operator=(const BadReturn2&); Index: clang-tidy/misc/AssignOperatorSignatureCheck.h === --- clang-tidy/misc/AssignOperatorSignatureCheck.h +++ clang-tidy/misc/AssignOperatorSignatureCheck.h @@ -24,10 +24,13 @@ /// * Private and deleted operators are ignored. class AssignOperatorSignatureCheck : public ClangTidyCheck { public: - AssignOperatorSignatureCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + AssignOperatorSignatureCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const bool CheckAllAssignOperators; }; } // namespace misc Index: clang-tidy/misc/AssignOperatorSignatureCheck.cpp === --- clang-tidy/misc/AssignOperatorSignatureCheck.cpp +++ clang-tidy/misc/AssignOperatorSignatureCheck.cpp @@ -17,6 +17,16 @@ namespace tidy { namespace misc { +AssignOperatorSignatureCheck::AssignOperatorSignatureCheck( +StringRef Name, ClangTidyContext *Context) +: ClangTidyCheck(Name, Context), + CheckAllAssignOperators(Options.get("CheckAllAssignOperators", false)) {} + +void AssignOperatorSignatureCheck::storeOptions( +ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "CheckAllAssignOperators", CheckAllAssignOperators); +} + void AssignOperatorSignatureCheck::registerMatchers( ast_matchers::MatchFinder *Finder) { // Only register the matchers for C++; the functionality currently does not @@ -31,16 +41,19 @@ const auto IsSelf = qualType( anyOf(hasDeclaration(equalsBoundNode("class")), referenceType(pointee(hasDeclaration(equalsBoundNode("class")); - const auto IsSelfAssign = + const auto IsAssign = cxxMethodDecl(unless(anyOf(isDeleted(), isPrivate(), isImplicit())), -hasName("operator="), ofClass(recordDecl().bind("class")), -hasParameter(0, parmVarDecl(hasType(IsSelf +hasName("operator="), ofClass(recordDecl().bind("class"))) + .bind("method"); + const auto IsSelfAssign = + cxxMethodDecl(IsAssign, hasParameter(0, parmVarDecl(hasType(IsSelf .bind("method"); Finder->addMatcher( - cxxMethodDecl(IsSelfAssign, unless(HasGoodReturnType)).bind("ReturnType"), + cxxMethodDecl(CheckAllAssignOperators ? IsAssign : IsSelfAssign, +unless(HasGoodReturnType)) + .bind("ReturnType"), this); - const auto BadSelf = referenceType( anyOf(lValueReferenceType(pointee(unless(isConstQualified(, rValueReferenceType(pointee(isConstQualified(); @@ -58,14 +71,13 @@ void AssignOperatorSignatureCheck::check( const MatchFinder::MatchResult &Result) { - const auto* Method = Result.Nodes.getNodeAs("method"); + const auto *Method = Result.Nodes.getNodeAs("method"); std::string Name = Method->getParent()->getName(); static const char *const Messages[][2] = { {"ReturnType", "operator=() should return '%0&'"}, {"ArgumentType", "operator=() should take '%0 const&', '%0&&' or '%0'"}, - {"cv", "operator=() should not be marked '%1'"} - }; + {"cv", "operator=() should not be marked '%1'"}}; for (const auto &Message : Messages) { if (Result.Nodes.getNodeAs(Message[0])) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m
AUTO: Jacob Hansen is out of the office (returning 05/04/2016)
I am out of the office until 05/04/2016. Note: This is an automated response to your message "cfe-commits Digest, Vol 105, Issue 505" sent on 3/23/2016 4:59:28 PM. This is the only notification you will receive while this person is away. This message and any attachments are intended for the use of the addressee or addressees only. The unauthorised disclosure, use, dissemination or copying (either in whole or in part) of its content is not permitted. If you received this message in error, please notify the sender and delete it from your system. Emails can be altered and their integrity cannot be guaranteed by the sender. Please consider the environment before printing this email. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18243: [ASTMatchers] Existing matcher hasAnyArgument fixed
baloghadamsoftware updated this revision to Diff 51437. baloghadamsoftware added a comment. Release notes fixed. http://reviews.llvm.org/D18243 Files: docs/LibASTMatchersReference.html docs/ReleaseNotes.rst include/clang/ASTMatchers/ASTMatchers.h unittests/ASTMatchers/ASTMatchersTest.cpp Index: unittests/ASTMatchers/ASTMatchersTest.cpp === --- unittests/ASTMatchers/ASTMatchersTest.cpp +++ unittests/ASTMatchers/ASTMatchersTest.cpp @@ -1633,10 +1633,15 @@ TEST(Matcher, AnyArgument) { StatementMatcher CallArgumentY = callExpr( - hasAnyArgument(declRefExpr(to(varDecl(hasName("y")); + hasAnyArgument( + ignoringParenImpCasts(declRefExpr(to(varDecl(hasName("y"))); EXPECT_TRUE(matches("void x(int, int) { int y; x(1, y); }", CallArgumentY)); EXPECT_TRUE(matches("void x(int, int) { int y; x(y, 42); }", CallArgumentY)); EXPECT_TRUE(notMatches("void x(int, int) { x(1, 2); }", CallArgumentY)); + + StatementMatcher ImplicitCastedArgument = callExpr( + hasAnyArgument(implicitCastExpr())); + EXPECT_TRUE(matches("void x(long) { int y; x(y); }", ImplicitCastedArgument)); } TEST(ForEachArgumentWithParam, ReportsNoFalsePositives) { Index: include/clang/ASTMatchers/ASTMatchers.h === --- include/clang/ASTMatchers/ASTMatchers.h +++ include/clang/ASTMatchers/ASTMatchers.h @@ -2989,18 +2989,13 @@ /// matches x(1, y, 42) /// with hasAnyArgument(...) /// matching y -/// -/// FIXME: Currently this will ignore parentheses and implicit casts on -/// the argument before applying the inner matcher. We'll want to remove -/// this to allow for greater control by the user once \c ignoreImplicit() -/// has been implemented. AST_POLYMORPHIC_MATCHER_P(hasAnyArgument, AST_POLYMORPHIC_SUPPORTED_TYPES(CallExpr, CXXConstructExpr), internal::Matcher, InnerMatcher) { for (const Expr *Arg : Node.arguments()) { BoundNodesTreeBuilder Result(*Builder); -if (InnerMatcher.matches(*Arg->IgnoreParenImpCasts(), Finder, &Result)) { +if (InnerMatcher.matches(*Arg, Finder, &Result)) { *Builder = std::move(Result); return true; } Index: docs/ReleaseNotes.rst === --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -119,6 +119,12 @@ AST Matchers +- hasAnyArgument: Matcher no longer ignores parentheses and implicit casts on + the argument before applying the inner matcher. The fix was done to allow for + greater control by the user. In all existing checkers that use this matcher + all instances of code ``hasAnyArgument()`` must be changed to + ``hasAnyArgument(ignoringParenImpCasts())``. + ... libclang Index: docs/LibASTMatchersReference.html === --- docs/LibASTMatchersReference.html +++ docs/LibASTMatchersReference.html @@ -3636,11 +3636,6 @@ matches x(1, y, 42) with hasAnyArgument(...) matching y - -FIXME: Currently this will ignore parentheses and implicit casts on -the argument before applying the inner matcher. We'll want to remove -this to allow for greater control by the user once ignoreImplicit() -has been implemented. @@ -3907,11 +3902,6 @@ matches x(1, y, 42) with hasAnyArgument(...) matching y - -FIXME: Currently this will ignore parentheses and implicit casts on -the argument before applying the inner matcher. We'll want to remove -this to allow for greater control by the user once ignoreImplicit() -has been implemented. Index: unittests/ASTMatchers/ASTMatchersTest.cpp === --- unittests/ASTMatchers/ASTMatchersTest.cpp +++ unittests/ASTMatchers/ASTMatchersTest.cpp @@ -1633,10 +1633,15 @@ TEST(Matcher, AnyArgument) { StatementMatcher CallArgumentY = callExpr( - hasAnyArgument(declRefExpr(to(varDecl(hasName("y")); + hasAnyArgument( + ignoringParenImpCasts(declRefExpr(to(varDecl(hasName("y"))); EXPECT_TRUE(matches("void x(int, int) { int y; x(1, y); }", CallArgumentY)); EXPECT_TRUE(matches("void x(int, int) { int y; x(y, 42); }", CallArgumentY)); EXPECT_TRUE(notMatches("void x(int, int) { x(1, 2); }", CallArgumentY)); + + StatementMatcher ImplicitCastedArgument = callExpr( + hasAnyArgument(implicitCastExpr())); + EXPECT_TRUE(matches("void x(long) { int y; x(y); }", ImplicitCastedArgument)); } TEST(ForEachArgumentWithParam, ReportsNoFalsePositives) { Index: include/clang/ASTMatchers/ASTMatchers.h === --- include/clang/ASTMatchers/ASTMatchers.h +++ include/clang/ASTMatchers/ASTMatchers.h @@ -2989,18 +2989,13 @@ /// matches x(1, y, 42) /// with hasAn
Re: [PATCH] D18399: Added support for different VFSs in format::getStyle.
klimek added inline comments. Comment at: lib/Format/Format.cpp:2148 @@ -2140,3 +2147,3 @@ // the file or not. -llvm::sys::fs::is_regular_file(Twine(ConfigFile), IsFile); - +Status = FS->status(ConfigFile.c_str()); +bool IsFile = Prefer .str to .c_str, the former is nicely typed. (here and below) http://reviews.llvm.org/D18399 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18401: clang-cl: Include /FI headers in /showIncludes output.
thakis added a comment. On second thought, and after looking at -E output with -include and gch files, and at CC_PRINT_HEADERS behavior (it prints but doesn't indent), I think I like the alternative implementation I'm mentioning better. Let me make that change. http://reviews.llvm.org/D18401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18385: [CUDA] Simplify SemaCUDA/function-overload.cu test.
tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. Small nit, LGTM otherwise. Comment at: test/SemaCUDA/function-overload.cu:66 @@ +65,3 @@ +__device__ int d() { return 8; } +// expected-note@-1 0+ {{'d' declared here}} +// expected-note@-2 0+ {{candidate function not viable: call to __device__ function from __host__ function}} Minor nit: Notes that are common to all test runs should probably be 1+. http://reviews.llvm.org/D18385 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18399: Added support for different VFSs in format::getStyle.
ioeric updated this revision to Diff 51440. ioeric added a comment. - changed c_str() to str() http://reviews.llvm.org/D18399 Files: include/clang/Format/Format.h lib/Format/Format.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -14,6 +14,7 @@ #include "clang/Frontend/TextDiagnosticPrinter.h" #include "llvm/Support/Debug.h" +#include "llvm/Support/MemoryBuffer.h" #include "gtest/gtest.h" #define DEBUG_TYPE "format-test" @@ -11199,6 +11200,33 @@ verifyFormat("include \"a.td\"\ninclude \"b.td\"", Style); } +TEST(FormatStyle, GetStyleOfFile) { + vfs::InMemoryFileSystem FS; + // Test 1: format file in the same directory. + ASSERT_TRUE( + FS.addFile("/a/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: LLVM"))); + ASSERT_TRUE( + FS.addFile("/a/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); + auto Style1 = getStyle("file", "/a/.clang-format", "Google", &FS); + ASSERT_EQ(Style1, getLLVMStyle()); + + // Test 2: fallback to default. + ASSERT_TRUE( + FS.addFile("/b/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); + auto Style2 = getStyle("file", "/b/test.cpp", "Mozilla", &FS); + ASSERT_EQ(Style2, getMozillaStyle()); + + // Test 3: format file in parent directory. + ASSERT_TRUE( + FS.addFile("/c/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google"))); + ASSERT_TRUE(FS.addFile("/c/sub/sub/sub/test.cpp", 0, + llvm::MemoryBuffer::getMemBuffer("int i;"))); + auto Style3 = getStyle("file", "/c/sub/sub/sub/test.cpp", "LLVM", &FS); + ASSERT_EQ(Style3, getGoogleStyle()); +} + class ReplacementTest : public ::testing::Test { protected: tooling::Replacement createReplacement(SourceLocation Start, unsigned Length, Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -2098,7 +2098,10 @@ } FormatStyle getStyle(StringRef StyleName, StringRef FileName, - StringRef FallbackStyle) { + StringRef FallbackStyle, vfs::FileSystem *FS) { + if (!FS) { +FS = vfs::getRealFileSystem().get(); + } FormatStyle Style = getLLVMStyle(); Style.Language = getLanguageByFileName(FileName); if (!getPredefinedStyle(FallbackStyle, Style.Language, &Style)) { @@ -2129,28 +2132,35 @@ llvm::sys::fs::make_absolute(Path); for (StringRef Directory = Path; !Directory.empty(); Directory = llvm::sys::path::parent_path(Directory)) { -if (!llvm::sys::fs::is_directory(Directory)) + +auto Status = FS->status(Directory); +if (!Status || +Status->getType() != llvm::sys::fs::file_type::directory_file) { continue; +} + SmallString<128> ConfigFile(Directory); llvm::sys::path::append(ConfigFile, ".clang-format"); DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n"); -bool IsFile = false; // Ignore errors from is_regular_file: we only need to know if we can read // the file or not. -llvm::sys::fs::is_regular_file(Twine(ConfigFile), IsFile); - +Status = FS->status(ConfigFile.str()); +bool IsFile = +Status && (Status->getType() == llvm::sys::fs::file_type::regular_file); if (!IsFile) { // Try _clang-format too, since dotfiles are not commonly used on Windows. ConfigFile = Directory; llvm::sys::path::append(ConfigFile, "_clang-format"); DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n"); - llvm::sys::fs::is_regular_file(Twine(ConfigFile), IsFile); + Status = FS->status(ConfigFile.str()); + IsFile = Status && + (Status->getType() == llvm::sys::fs::file_type::regular_file); } if (IsFile) { llvm::ErrorOr> Text = - llvm::MemoryBuffer::getFile(ConfigFile.c_str()); + FS->getBufferForFile(ConfigFile.str()); if (std::error_code EC = Text.getError()) { llvm::errs() << EC.message() << "\n"; break; Index: include/clang/Format/Format.h === --- include/clang/Format/Format.h +++ include/clang/Format/Format.h @@ -16,6 +16,7 @@ #define LLVM_CLANG_FORMAT_FORMAT_H #include "clang/Basic/LangOptions.h" +#include "clang/Basic/VirtualFileSystem.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/ArrayRef.h" #include @@ -832,11 +833,13 @@ /// == "file". /// \param[in] FallbackStyle The name of a predefined style used to fallback to /// in case the style can't be determined from \p StyleName. +/// \param[in] FS The underlying file system, in which the file resides. By +/// default, the file system is the real file system. /// /// \returns FormatStyle as specified by ``StyleName``.
Re: [PATCH] D18399: Added support for different VFSs in format::getStyle.
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg http://reviews.llvm.org/D18399 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18264: [clang-tidy] misc-assign-operator-signature checker checks return value of all assign operators
xazax.hun added a comment. According to cpp core guidelines any assignment operator should return *this. https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rf-assignment-op So in case this check is activated by using an alias it should not be a special case. http://reviews.llvm.org/D18264 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D17762: Fix an assertion failure in setPointOfInstantiation.
rsmith added a comment. It seems to me that the bug is that we're producing an invalid source location, not how we handle that downstream. Typo correction should be able to preserve the source location of whatever expression it was correcting. Comment at: lib/Sema/SemaExpr.cpp:14047-14048 @@ -14046,1 +14046,4 @@ + if (Loc.isInvalid()) +return true; + General rule: for a function that returns `true` to say "an error occurred", it is not acceptable to return `true` without either (a) producing an error or (b) observing some state that proves someone else emitted an error (such as a decl that has been set as invalid). http://reviews.llvm.org/D17762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D17412: PR19957: [OpenCL] incorrectly accepts implicit address space conversion with ternary operator
yaxunl marked 15 inline comments as done. Comment at: lib/AST/ASTContext.cpp:7613 @@ +7612,3 @@ +if (getLangOpts().OpenCL) { + if (LHS.getUnqualifiedType() != RHS.getUnqualifiedType() || + LQuals.getCVRQualifiers() != RQuals.getCVRQualifiers()) pxli168 wrote: > Anastasia wrote: > > Do you think we should check the types here? I was thinking we should do > > the check exactly as below apart from AS specific part. > I think the mergeType function it very complex, too. It seems to check type > can be merged recursively later. Here if unqualified types are different or CVS qualifiers are different, the two types cannot be merged, so an empty QualType is returned. If unqualified types and CVS qualifiers are the same, we check if address spaces are compatible and returns the merged type. Comment at: lib/Sema/SemaExpr.cpp:6168 @@ -6168,3 +6167,3 @@ QualType CompositeTy = S.Context.mergeTypes(lhptee, rhptee); if (CompositeTy.isNull()) { pxli168 wrote: > Anastasia wrote: > > Could we instead add a comment explaining different cases with AS we can > > have here i.e. 1(a-c)&2(a-c)! > > > > And may be we could refer to each case by adding comments in code below. > Good idea. Will add comments. Comment at: lib/Sema/SemaExpr.cpp:6220-6232 @@ -6186,7 +6219,15 @@ ResultTy = S.Context.getBlockPointerType(ResultTy); - else + else { +auto ResultAddrSpace = ResultTy.getQualifiers().getAddressSpace(); +LHSCastKind = lhQual.getAddressSpace() == ResultAddrSpace + ? CK_BitCast + : CK_AddressSpaceConversion; +RHSCastKind = rhQual.getAddressSpace() == ResultAddrSpace + ? CK_BitCast + : CK_AddressSpaceConversion; ResultTy = S.Context.getPointerType(ResultTy); + } - LHS = S.ImpCastExprToType(LHS.get(), ResultTy, CK_BitCast); - RHS = S.ImpCastExprToType(RHS.get(), ResultTy, CK_BitCast); + LHS = S.ImpCastExprToType(LHS.get(), ResultTy, LHSCastKind); + RHS = S.ImpCastExprToType(RHS.get(), ResultTy, RHSCastKind); return ResultTy; Anastasia wrote: > There are still blocks in ObjC, we shouldn't change! line 6218 handles the blocks Comment at: lib/Sema/SemaExpr.cpp:6222-6227 @@ -6188,1 +6221,8 @@ +auto ResultAddrSpace = ResultTy.getQualifiers().getAddressSpace(); +LHSCastKind = lhQual.getAddressSpace() == ResultAddrSpace + ? CK_BitCast + : CK_AddressSpaceConversion; +RHSCastKind = rhQual.getAddressSpace() == ResultAddrSpace + ? CK_BitCast + : CK_AddressSpaceConversion; ResultTy = S.Context.getPointerType(ResultTy); pxli168 wrote: > Anastasia wrote: > > pxli168 wrote: > > > What will mergetypes return? > > > It seems the LHS and RHS are compatibel here, and may be they did not > > > need bitcast? > > I think we always need a cast here, because types are not exactly the same > > at this point! > I tried to figure out what mergetypes will return and find it seems to have > logic far more complex than this. ImpCastExprToType handles the case when the target type matches the value, it simply does not generate the cast. Comment at: lib/Sema/SemaExpr.cpp:6231 @@ -6189,3 +6230,3 @@ - LHS = S.ImpCastExprToType(LHS.get(), ResultTy, CK_BitCast); - RHS = S.ImpCastExprToType(RHS.get(), ResultTy, CK_BitCast); + LHS = S.ImpCastExprToType(LHS.get(), ResultTy, LHSCastKind); + RHS = S.ImpCastExprToType(RHS.get(), ResultTy, RHSCastKind); Anastasia wrote: > Btw, could we do the same thing with {LHS/RHS}CastKind variables in abode > code. It looks cleaner I think. Sorry what is abode code? Comment at: test/CodeGenOpenCL/address-spaces-conversions.cl:1 @@ -1,2 +1,2 @@ // RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 -ffake-address-space-map -cl-std=CL2.0 -emit-llvm -o - | FileCheck %s it seems to be separate issue. I will take a look in another patch Repository: rL LLVM http://reviews.llvm.org/D17412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r264169 - Visualize fields of records as they were declared in Visual Studio debugger
Author: mps Date: Wed Mar 23 12:29:42 2016 New Revision: 264169 URL: http://llvm.org/viewvc/llvm-project?rev=264169&view=rev Log: Visualize fields of records as they were declared in Visual Studio debugger Modified: cfe/trunk/utils/clang.natvis Modified: cfe/trunk/utils/clang.natvis URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/clang.natvis?rev=264169&r1=264168&r2=264169&view=diff == --- cfe/trunk/utils/clang.natvis (original) +++ cfe/trunk/utils/clang.natvis Wed Mar 23 12:29:42 2016 @@ -95,7 +95,7 @@ or create a symbolic link so it updates -Field {{{*(clang::NamedDecl *)this,view(cpp)nd}}} +Field {{{*(clang::DeclaratorDecl *)this,view(cpp)nd}}} {*(clang::FunctionDecl *)this,nd} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18401: clang-cl: Include /FI headers in /showIncludes output.
thakis updated the summary for this revision. thakis updated this revision to Diff 51442. http://reviews.llvm.org/D18401 Files: lib/Frontend/CompilerInstance.cpp lib/Frontend/HeaderIncludeGen.cpp lib/Frontend/InitPreprocessor.cpp test/Driver/cl-pch-showincludes.cpp test/Frontend/print-header-includes.c Index: test/Frontend/print-header-includes.c === --- test/Frontend/print-header-includes.c +++ test/Frontend/print-header-includes.c @@ -1,24 +1,24 @@ -// RUN: cd %S -// RUN: %clang_cc1 -include Inputs/test3.h -E -H -o %t.out %s 2> %t.stderr +// RUN: %clang_cc1 -I%S -include Inputs/test3.h -E -H -o /dev/null %s 2> %t.stderr // RUN: FileCheck < %t.stderr %s // CHECK-NOT: test3.h // CHECK: . {{.*test.h}} // CHECK: .. {{.*test2.h}} -// RUN: %clang_cc1 -include Inputs/test3.h -E --show-includes -o %t.out %s > %t.stdout -// RUN: FileCheck --check-prefix=MS < %t.stdout %s -// MS-NOT: test3.h -// MS: Note: including file: {{.*test.h}} -// MS: Note: including file: {{.*test2.h}} +// RUN: %clang_cc1 -I%S -include Inputs/test3.h -E --show-includes -o /dev/null %s | \ +// RUN: FileCheck --strict-whitespace --check-prefix=MS %s +// MS-NOT: +// MS: Note: including file: {{[^ ]*test3.h}} +// MS: Note: including file: {{[^ ]*test.h}} +// MS: Note: including file: {{[^ ]*test2.h}} // MS-NOT: Note // RUN: echo "fun:foo" > %t.blacklist -// RUN: %clang_cc1 -fsanitize=address -fdepfile-entry=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout -// RUN: FileCheck --check-prefix=MS-BLACKLIST < %t.stdout %s -// MS-BLACKLIST: Note: including file: {{.*\.blacklist}} -// MS-BLACKLIST: Note: including file: {{.*test.h}} -// MS-BLACKLIST: Note: including file: {{.*test2.h}} +// RUN: %clang_cc1 -I%S -fsanitize=address -fdepfile-entry=%t.blacklist -E --show-includes -o /dev/null %s | \ +// RUN: FileCheck --strict-whitespace --check-prefix=MS-BLACKLIST %s +// MS-BLACKLIST: Note: including file: {{[^ ]*\.blacklist}} +// MS-BLACKLIST: Note: including file: {{[^ ]*test.h}} +// MS-BLACKLIST: Note: including file: {{[^ ]*test2.h}} // MS-BLACKLIST-NOT: Note #include "Inputs/test.h" Index: test/Driver/cl-pch-showincludes.cpp === --- test/Driver/cl-pch-showincludes.cpp +++ test/Driver/cl-pch-showincludes.cpp @@ -8,16 +8,17 @@ // When building the pch, header1.h (included by header2.h), header2.h (the pch // input itself) and header3.h (included directly, above) should be printed. -// RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Ycheader2.h /FIheader2.h /Fp%t.pch /c /Fo%t -- %s 2>&1 \ -// RUN: | FileCheck -check-prefix=CHECK-YC %s -// CHECK-YC: Note: including file: {{.+header2.h}} -// CHECK-YC: Note: including file: {{.+header1.h}} -// CHECK-YC: Note: including file: {{.+header3.h}} +// RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Ycheader2.h /FIheader2.h /Fp%t.pch /c /Fo%t -- %s \ +// RUN: | FileCheck --strict-whitespace -check-prefix=CHECK-YC %s +// CHECK-YC: Note: including file: {{[^ ]*header2.h}} +// FIXME: header1.h should be indented one more: +// CHECK-YC: Note: including file: {{[^ ]*header1.h}} +// CHECK-YC: Note: including file: {{[^ ]*header3.h}} // When using the pch, only the direct include is printed. -// RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Yuheader2.h /FIheader2.h /Fp%t.pch /c /Fo%t -- %s 2>&1 \ -// RUN: | FileCheck -check-prefix=CHECK-YU %s +// RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Yuheader2.h /FIheader2.h /Fp%t.pch /c /Fo%t -- %s \ +// RUN: | FileCheck --strict-whitespace -check-prefix=CHECK-YU %s // CHECK-YU-NOT: Note: including file: {{.*pch}} // CHECK-YU-NOT: Note: including file: {{.*header1.h}} // CHECK-YU-NOT: Note: including file: {{.*header2.h}} -// CHECK-YU: Note: including file: {{.+header3.h}} +// CHECK-YU: Note: including file: {{[^ ]*header3.h}} Index: lib/Frontend/InitPreprocessor.cpp === --- lib/Frontend/InitPreprocessor.cpp +++ lib/Frontend/InitPreprocessor.cpp @@ -972,6 +972,10 @@ PP.getDiagnostics()); } + // Exit the command line and go back to (2 is LC_LEAVE). + if (!PP.getLangOpts().AsmPreprocessor) +Builder.append("# 1 \"\" 2"); + // If -imacros are specified, include them now. These are processed before // any -include directives. for (unsigned i = 0, e = InitOpts.MacroIncludes.size(); i != e; ++i) @@ -990,10 +994,6 @@ AddImplicitInclude(Builder, Path); } - // Exit the command line and go back to (2 is LC_LEAVE). - if (!PP.getLangOpts().AsmPreprocessor) -Builder.append("# 1 \"\" 2"); - // Instruct the preprocessor to skip the preamble. PP.setSkipMainFilePreamble(InitOpts.PrecompiledPreambleBytes.first, InitOpts.PrecompiledPreambleBytes.second); Index: lib/Frontend/HeaderIncludeGen.cpp =
r264170 - [NFC] Delete an unused function parameter from a static function
Author: faisalv Date: Wed Mar 23 12:39:51 2016 New Revision: 264170 URL: http://llvm.org/viewvc/llvm-project?rev=264170&view=rev Log: [NFC] Delete an unused function parameter from a static function Modified: cfe/trunk/lib/Sema/SemaExpr.cpp Modified: cfe/trunk/lib/Sema/SemaExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=264170&r1=264169&r2=264170&view=diff == --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Mar 23 12:39:51 2016 @@ -13249,8 +13249,7 @@ static bool captureInCapturedRegion(Capt /// \brief Create a field within the lambda class for the variable /// being captured. -// FIXME: Delete VarDecl *Var below, it is not used in the function. -static void addAsFieldToClosureType(Sema &S, LambdaScopeInfo *LSI, VarDecl *Var, +static void addAsFieldToClosureType(Sema &S, LambdaScopeInfo *LSI, QualType FieldType, QualType DeclRefType, SourceLocation Loc, bool RefersToCapturedVariable) { @@ -13344,7 +13343,7 @@ static bool captureInLambda(LambdaScopeI // Capture this variable in the lambda. if (BuildAndDiagnose) -addAsFieldToClosureType(S, LSI, Var, CaptureType, DeclRefType, Loc, +addAsFieldToClosureType(S, LSI, CaptureType, DeclRefType, Loc, RefersToCapturedVariable); // Compute the type of a reference to this captured variable. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18401: clang-cl: Include /FI headers in /showIncludes output.
rsmith added a subscriber: rsmith. rsmith accepted this revision. rsmith added a reviewer: rsmith. This revision is now accepted and ready to land. Comment at: lib/Frontend/HeaderIncludeGen.cpp:155-156 @@ -151,3 +154,4 @@ // Dump the header include information we are past the predefines buffer or // are showing all headers. + if (ShowHeader && Reason == PPCallbacks::EnterFile && Missing "if" or similar in this comment? Comment at: lib/Frontend/HeaderIncludeGen.cpp:158 @@ +157,3 @@ + if (ShowHeader && Reason == PPCallbacks::EnterFile && + strcmp(UserLoc.getFilename(), "")) { +PrintHeaderInfo(OutputFile, UserLoc.getFilename(), ShowDepth, IncludeDepth, I'm not a fan of using `strcmp` like this (implicitly converting the result to `bool` to mean "check strings are different"). I'd prefer you used UserLoc.getFilename() != StringRef("") (Of course, ideally we'd use something more principled than a check against the filename string... please add a FIXME for that.) http://reviews.llvm.org/D18401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D18408: readability check for const params in declarations
fowles created this revision. fowles added a reviewer: alexfh. fowles added a subscriber: cfe-commits. Adds a clang-tidy warning for top-level consts in function declarations. http://reviews.llvm.org/D18408 Files: clang-tidy/readability/AvoidConstParamsInDecls.cpp clang-tidy/readability/AvoidConstParamsInDecls.h clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst test/clang-tidy/readability-avoid-const-params-in-decls.cpp Index: test/clang-tidy/readability-avoid-const-params-in-decls.cpp === --- /dev/null +++ test/clang-tidy/readability-avoid-const-params-in-decls.cpp @@ -0,0 +1,78 @@ +// RUN: %check_clang_tidy %s readability-avoid-const-params-in-decls %t + +using alias_type = bool; +using alias_const_type = const bool; + + +void F1(const int i); +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls] +// CHECK-FIXES: void F1(int i); + +void F2(const int *const i); +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified +// CHECK-FIXES: void F2(const int * i); + +void F3(int const i); +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified +// CHECK-FIXES: void F3(int i); + +void F4(alias_type const i); +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified +// CHECK-FIXES: void F4(alias_type i); + +void F5(const int); +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 1 is const-qualified +// CHECK-FIXES: void F5(int); + +void F6(const int *const); +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 1 is const-qualified +// BUG(b/27584482): void F6(const int *); should be produced + +void F7(int, const int); +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: parameter 2 is const-qualified +// CHECK-FIXES: void F7(int, int); + +void F8(const int, const int); +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 1 is const-qualified +// CHECK-MESSAGES: :[[@LINE-2]]:20: warning: parameter 2 is const-qualified +// CHECK-FIXES: void F8(int, int); + +void F9(const int long_name); +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'long_name' +// CHECK-FIXES: void F9(int long_name); + +void F10(const int *const *const long_name); +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 'long_name' +// CHECK-FIXES: void F10(const int *const * long_name); + + +struct Foo { + Foo(const int i); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: parameter 'i' + // CHECK-FIXES: Foo(int i); + + void operator()(const int i); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: parameter 'i' + // CHECK-FIXES: void operator()(int i); +}; + +// Do not match on definitions +void NF1(const int i) {} +void NF2(const int *const i) {} +void NF3(int const i) {} +void NF4(alias_type const i) {} +void NF5(const int) {} +void NF6(const int *const) {} +void NF7(int, const int) {} +void NF8(const int, const int) {} + +// Do not match on other stuff +void NF(const alias_type& i); +void NF(const int &i); +void NF(const int *i); +void NF(alias_const_type i); +void NF(const alias_type&); +void NF(const int&); +void NF(const int*); +void NF(const int* const*); +void NF(alias_const_type); Index: docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst === --- /dev/null +++ docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst @@ -0,0 +1,17 @@ +.. title:: clang-tidy - readability-avoid-const-params-in-decls + +readability-avoid-const-params-in-decls +== + +Checks whether a function declaration has parameters that are top level const. + +`const` values in declarations do not have any affect on the signature of a +function, so they should not be put there. For example: + +Examples: + +.. code:: c++ + + void f(const string); // bad. const is top level + void f(const string&); // ok. const is not top level + Index: docs/clang-tidy/checks/list.rst === --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -88,6 +88,7 @@ performance-faster-string-find performance-for-range-copy performance-implicit-cast-in-loop + readability-avoid-const-params-in-decls readability-braces-around-statements readability-container-size-empty readability-else-after-return Index: clang-tidy/readability/ReadabilityTidyModule.cpp === --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #in
Re: [PATCH] D18380: [CUDA] Implement -fcuda-relaxed-constexpr, and enable it by default.
rsmith added inline comments. Comment at: include/clang/Driver/CC1Options.td:702-703 @@ -701,2 +701,4 @@ HelpText<"Allow variadic functions in CUDA device code.">; +def fcuda_relaxed_constexpr : Flag<["-"], "fcuda-relaxed-constexpr">, + HelpText<"Treat constexpr functions as __host__ __device__.">; Is there a better name we can use for this? I don't think this is "relaxed" in any obvious sense. `-fcuda-host-device-constexpr` or `-fcuda-constexpr-on-device` might be clearer? Comment at: lib/Driver/Tools.cpp:3597 @@ -3596,2 +3596,3 @@ CmdArgs.push_back("-fcuda-disable-target-call-checks"); +CmdArgs.push_back("-fcuda-relaxed-constexpr"); } For flags that are enabled by default, we usually have the -cc1 flag be a `-fno-*` flag. This allows people to use (for instance) `clang blah.cu -Xclang -fno-cuda-relaxed-constexpr` if necessary. Comment at: lib/Sema/SemaOverload.cpp:1132 @@ -1136,1 +1131,3 @@ +// one viable function with this signature on any side of CUDA compilation. +if ((NewTarget == CFT_Global) || (OldTarget == CFT_Global)) return false; No parens around `==` comparisons. http://reviews.llvm.org/D18380 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18275: [ASTMatchers] Add own version of VariadicFunction.
sbenza updated this revision to Diff 51447. sbenza added a comment. Minor fix http://reviews.llvm.org/D18275 Files: include/clang/ASTMatchers/ASTMatchers.h include/clang/ASTMatchers/ASTMatchersInternal.h lib/ASTMatchers/Dynamic/Marshallers.h unittests/ASTMatchers/ASTMatchersTest.cpp Index: unittests/ASTMatchers/ASTMatchersTest.cpp === --- unittests/ASTMatchers/ASTMatchersTest.cpp +++ unittests/ASTMatchers/ASTMatchersTest.cpp @@ -3000,6 +3000,9 @@ EXPECT_TRUE(notMatches(Code, recordDecl(hasAnyName("::C", "::b::C"; EXPECT_TRUE( matches(Code, recordDecl(hasAnyName("::C", "::b::C", "::a::b::C"; + + std::vector Names = {"::C", "::b::C", "::a::b::C"}; + EXPECT_TRUE(matches(Code, recordDecl(hasAnyName(Names; } TEST(Matcher, IsDefinition) { Index: lib/ASTMatchers/Dynamic/Marshallers.h === --- lib/ASTMatchers/Dynamic/Marshallers.h +++ lib/ASTMatchers/Dynamic/Marshallers.h @@ -325,8 +325,9 @@ template )> - VariadicFuncMatcherDescriptor(llvm::VariadicFunction Func, - StringRef MatcherName) + VariadicFuncMatcherDescriptor( + ast_matchers::internal::VariadicFunction Func, + StringRef MatcherName) : Func(&variadicMatcherDescriptor), MatcherName(MatcherName.str()), ArgsKind(ArgTypeTraits::getKind()) { @@ -655,9 +656,9 @@ /// \brief Variadic overload. template )> -MatcherDescriptor * -makeMatcherAutoMarshall(llvm::VariadicFunction VarFunc, -StringRef MatcherName) { +MatcherDescriptor *makeMatcherAutoMarshall( +ast_matchers::internal::VariadicFunction VarFunc, +StringRef MatcherName) { return new VariadicFuncMatcherDescriptor(VarFunc, MatcherName); } Index: include/clang/ASTMatchers/ASTMatchersInternal.h === --- include/clang/ASTMatchers/ASTMatchersInternal.h +++ include/clang/ASTMatchers/ASTMatchersInternal.h @@ -46,8 +46,9 @@ #include "clang/AST/StmtCXX.h" #include "clang/AST/StmtObjC.h" #include "clang/AST/Type.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/Optional.h" -#include "llvm/ADT/VariadicFunction.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/Support/ManagedStatic.h" #include #include @@ -60,6 +61,39 @@ namespace internal { +/// \brief Variadic function object. +/// +/// Most of the functions below that use VariadicFunction could be implemented +/// using plain C++11 variadic functions, but the function object allows us to +/// capture it on the dynamic matcher registry. +template )> +struct VariadicFunction { + ResultT operator()() const { return Func({}); } + + template + ResultT operator()(const ArgT &Arg1, const ArgsT &... Args) const { +return Execute(Arg1, static_cast(Args)...); + } + + // We also allow calls with an already created array, in case the caller + // already had it. + ResultT operator()(ArrayRef Args) const { +SmallVector InnerArgs; +for (const ArgT &Arg : Args) + InnerArgs.push_back(&Arg); +return Func(InnerArgs); + } + +private: + // Trampoline function to allow for implicit conversions to take place + // before we make the array. + template ResultT Execute(const ArgsT &... Args) const { +const ArgT *const ArgsArray[] = {&Args...}; +return Func(ArgsArray); + } +}; + /// \brief Unifies obtaining the underlying type of a regular node through /// `getType` and a TypedefNameDecl node through `getUnderlyingType`. template @@ -1405,9 +1439,8 @@ /// casted to CXXRecordDecl and all given matchers match. template class VariadicDynCastAllOfMatcher -: public llvm::VariadicFunction< -BindableMatcher, Matcher, -makeDynCastAllOfComposite > { +: public VariadicFunction, Matcher, + makeDynCastAllOfComposite> { public: VariadicDynCastAllOfMatcher() {} }; @@ -1423,9 +1456,9 @@ /// \c Matcher. /// The returned matcher matches if all given matchers match. template -class VariadicAllOfMatcher : public llvm::VariadicFunction< - BindableMatcher, Matcher, - makeAllOfComposite > { +class VariadicAllOfMatcher +: public VariadicFunction, Matcher, + makeAllOfComposite> { public: VariadicAllOfMatcher() {} }; @@ -1546,8 +1579,8 @@ new MatcherImpl(InnerMatcher, Getter::value())); } - struct Func : public llvm::VariadicFunction, - &Self::create> { + struct Func + : public VariadicFunction, &Self::create> { Func() {} }; Index: include/clang/ASTMatchers/ASTMatchers.h === --- include/clang/ASTMatchers/ASTMatchers.h +++ include/clang/ASTMatchers/ASTMatchers.h @@ -1990,8 +1990,8 @@ /// \code /// anyOf(hasName(a),
Re: [PATCH] D18275: [ASTMatchers] Add own version of VariadicFunction.
sbenza added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:80 @@ +79,3 @@ + ResultT operator()(ArrayRef Args) const { +std::vector InnerArgs; +for (const ArgT &Arg : Args) alexfh wrote: > It's unfortunate that we need to create an array of the argument pointers > here, but it seems there's no larger common denominator of the two ways this > functions can be called. > > One nit though: SmallVector will be a better container here. > It's unfortunate that we need to create an array of the argument pointers > here, but it seems there's no larger common denominator of the two ways this > functions can be called. Now that we control it, we can change it to be something different. For example, instead of passing a function pointer as template parameter we could pass a class that contains overloaded operator() for both ArrayRef and ArrayRef. On the other hand, constructing the matchers has never been the performance bottleneck of the framework. They are usually created once and used thousands/millions of times. Might not be worth it to optimize this too much. > One nit though: SmallVector will be a better container here. Done. http://reviews.llvm.org/D18275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18271: Avoid -Wshadow warnings about constructor parameters named after fields
alexfh added a subscriber: alexfh. alexfh added a comment. As a data point: I ran -Wshadow on our code base with a similar, but simpler patch (http://reviews.llvm.org/D18395, just disables the warning on ctor parameters named after fields). It removes more than half of the hits (from ~100k initially) and a random sample of ~100 removed removed hits didn't contain any issues that we'd like to still be warning about. Ultimately, I think, this diagnostic should be made much less noisy to be actually useful. So maybe even starting with a removal of a larger subset of warnings (and then gradually adding back the cases that seem to be important to flag) would be better. Comment at: lib/Sema/SemaExpr.cpp:9887 @@ -9854,1 +9886,3 @@ if (ConvTy == Compatible) { + const Expr *InnerLHS = LHSExpr->IgnoreParenCasts(); + const DeclRefExpr *DRE = dyn_cast(InnerLHS); Why this change? http://reviews.llvm.org/D18271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18271: Avoid -Wshadow warnings about constructor parameters named after fields
alexfh added a comment. In http://reviews.llvm.org/D18271#381728, @alexfh wrote: > ... and then gradually adding back the cases that seem to be important to > flag ... Maybe as separate warnings. http://reviews.llvm.org/D18271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18401: clang-cl: Include /FI headers in /showIncludes output.
thakis updated this revision to Diff 51448. thakis marked 2 inline comments as done. thakis added a comment. Thanks! All done. http://reviews.llvm.org/D18401 Files: lib/Frontend/CompilerInstance.cpp lib/Frontend/HeaderIncludeGen.cpp lib/Frontend/InitPreprocessor.cpp test/Driver/cl-pch-showincludes.cpp test/Frontend/print-header-includes.c Index: test/Frontend/print-header-includes.c === --- test/Frontend/print-header-includes.c +++ test/Frontend/print-header-includes.c @@ -1,24 +1,24 @@ -// RUN: cd %S -// RUN: %clang_cc1 -include Inputs/test3.h -E -H -o %t.out %s 2> %t.stderr +// RUN: %clang_cc1 -I%S -include Inputs/test3.h -E -H -o /dev/null %s 2> %t.stderr // RUN: FileCheck < %t.stderr %s // CHECK-NOT: test3.h // CHECK: . {{.*test.h}} // CHECK: .. {{.*test2.h}} -// RUN: %clang_cc1 -include Inputs/test3.h -E --show-includes -o %t.out %s > %t.stdout -// RUN: FileCheck --check-prefix=MS < %t.stdout %s -// MS-NOT: test3.h -// MS: Note: including file: {{.*test.h}} -// MS: Note: including file: {{.*test2.h}} +// RUN: %clang_cc1 -I%S -include Inputs/test3.h -E --show-includes -o /dev/null %s | \ +// RUN: FileCheck --strict-whitespace --check-prefix=MS %s +// MS-NOT: +// MS: Note: including file: {{[^ ]*test3.h}} +// MS: Note: including file: {{[^ ]*test.h}} +// MS: Note: including file: {{[^ ]*test2.h}} // MS-NOT: Note // RUN: echo "fun:foo" > %t.blacklist -// RUN: %clang_cc1 -fsanitize=address -fdepfile-entry=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout -// RUN: FileCheck --check-prefix=MS-BLACKLIST < %t.stdout %s -// MS-BLACKLIST: Note: including file: {{.*\.blacklist}} -// MS-BLACKLIST: Note: including file: {{.*test.h}} -// MS-BLACKLIST: Note: including file: {{.*test2.h}} +// RUN: %clang_cc1 -I%S -fsanitize=address -fdepfile-entry=%t.blacklist -E --show-includes -o /dev/null %s | \ +// RUN: FileCheck --strict-whitespace --check-prefix=MS-BLACKLIST %s +// MS-BLACKLIST: Note: including file: {{[^ ]*\.blacklist}} +// MS-BLACKLIST: Note: including file: {{[^ ]*test.h}} +// MS-BLACKLIST: Note: including file: {{[^ ]*test2.h}} // MS-BLACKLIST-NOT: Note #include "Inputs/test.h" Index: test/Driver/cl-pch-showincludes.cpp === --- test/Driver/cl-pch-showincludes.cpp +++ test/Driver/cl-pch-showincludes.cpp @@ -8,16 +8,17 @@ // When building the pch, header1.h (included by header2.h), header2.h (the pch // input itself) and header3.h (included directly, above) should be printed. -// RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Ycheader2.h /FIheader2.h /Fp%t.pch /c /Fo%t -- %s 2>&1 \ -// RUN: | FileCheck -check-prefix=CHECK-YC %s -// CHECK-YC: Note: including file: {{.+header2.h}} -// CHECK-YC: Note: including file: {{.+header1.h}} -// CHECK-YC: Note: including file: {{.+header3.h}} +// RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Ycheader2.h /FIheader2.h /Fp%t.pch /c /Fo%t -- %s \ +// RUN: | FileCheck --strict-whitespace -check-prefix=CHECK-YC %s +// CHECK-YC: Note: including file: {{[^ ]*header2.h}} +// FIXME: header1.h should be indented one more: +// CHECK-YC: Note: including file: {{[^ ]*header1.h}} +// CHECK-YC: Note: including file: {{[^ ]*header3.h}} // When using the pch, only the direct include is printed. -// RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Yuheader2.h /FIheader2.h /Fp%t.pch /c /Fo%t -- %s 2>&1 \ -// RUN: | FileCheck -check-prefix=CHECK-YU %s +// RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Yuheader2.h /FIheader2.h /Fp%t.pch /c /Fo%t -- %s \ +// RUN: | FileCheck --strict-whitespace -check-prefix=CHECK-YU %s // CHECK-YU-NOT: Note: including file: {{.*pch}} // CHECK-YU-NOT: Note: including file: {{.*header1.h}} // CHECK-YU-NOT: Note: including file: {{.*header2.h}} -// CHECK-YU: Note: including file: {{.+header3.h}} +// CHECK-YU: Note: including file: {{[^ ]*header3.h}} Index: lib/Frontend/InitPreprocessor.cpp === --- lib/Frontend/InitPreprocessor.cpp +++ lib/Frontend/InitPreprocessor.cpp @@ -972,6 +972,10 @@ PP.getDiagnostics()); } + // Exit the command line and go back to (2 is LC_LEAVE). + if (!PP.getLangOpts().AsmPreprocessor) +Builder.append("# 1 \"\" 2"); + // If -imacros are specified, include them now. These are processed before // any -include directives. for (unsigned i = 0, e = InitOpts.MacroIncludes.size(); i != e; ++i) @@ -990,10 +994,6 @@ AddImplicitInclude(Builder, Path); } - // Exit the command line and go back to (2 is LC_LEAVE). - if (!PP.getLangOpts().AsmPreprocessor) -Builder.append("# 1 \"\" 2"); - // Instruct the preprocessor to skip the preamble. PP.setSkipMainFilePreamble(InitOpts.PrecompiledPreambleBytes.first, InitOpts.PrecompiledPreambleBytes.second); Index: lib/Frontend/HeaderIncludeGen.cp
r264174 - clang-cl: Include /FI headers in /showIncludes output.
Author: nico Date: Wed Mar 23 13:00:22 2016 New Revision: 264174 URL: http://llvm.org/viewvc/llvm-project?rev=264174&view=rev Log: clang-cl: Include /FI headers in /showIncludes output. -H in gcc mode doesn't print -include headers, but they are included in depfiles written by MMD and friends. Since /showIncludes is what's used instead of depfiles, printing /FI there seems important (and matches cl.exe). Instead of giving HeaderIncludeGen more options, just switch on ShowAllHeaders in clang-cl mode and let clang::InitializePreprocessor() not put -include flags in the block. This changes the behavior of -E slightly, and it removes the flag from the output triggered by setting the obscure CC_PRINT_HEADERS=1 env var to true while running clang. Both of these seem ok to change. http://reviews.llvm.org/D18401 Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp cfe/trunk/lib/Frontend/HeaderIncludeGen.cpp cfe/trunk/lib/Frontend/InitPreprocessor.cpp cfe/trunk/test/Driver/cl-pch-showincludes.cpp cfe/trunk/test/Frontend/print-header-includes.c Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=264174&r1=264173&r2=264174&view=diff == --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original) +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Wed Mar 23 13:00:22 2016 @@ -372,7 +372,7 @@ void CompilerInstance::createPreprocesso if (DepOpts.PrintShowIncludes) { AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps, - /*ShowAllHeaders=*/false, /*OutputPath=*/"", + /*ShowAllHeaders=*/true, /*OutputPath=*/"", /*ShowDepth=*/true, /*MSStyle=*/true); } } Modified: cfe/trunk/lib/Frontend/HeaderIncludeGen.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/HeaderIncludeGen.cpp?rev=264174&r1=264173&r2=264174&view=diff == --- cfe/trunk/lib/Frontend/HeaderIncludeGen.cpp (original) +++ cfe/trunk/lib/Frontend/HeaderIncludeGen.cpp Wed Mar 23 13:00:22 2016 @@ -148,11 +148,18 @@ void HeaderIncludesCallback::FileChanged // line buffers. bool ShowHeader = (HasProcessedPredefines || (ShowAllHeaders && CurrentIncludeDepth > 2)); + unsigned IncludeDepth = CurrentIncludeDepth; + if (!HasProcessedPredefines) +--IncludeDepth; // Ignore indent from . // Dump the header include information we are past the predefines buffer or - // are showing all headers. - if (ShowHeader && Reason == PPCallbacks::EnterFile) { -PrintHeaderInfo(OutputFile, UserLoc.getFilename(), -ShowDepth, CurrentIncludeDepth, MSStyle); + // are showing all headers and this isn't the magic implicit + // header. + // FIXME: Identify headers in a more robust way than comparing their name to + // "" and "" in a bunch of places. + if (ShowHeader && Reason == PPCallbacks::EnterFile && + UserLoc.getFilename() != StringRef("")) { +PrintHeaderInfo(OutputFile, UserLoc.getFilename(), ShowDepth, IncludeDepth, +MSStyle); } } Modified: cfe/trunk/lib/Frontend/InitPreprocessor.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitPreprocessor.cpp?rev=264174&r1=264173&r2=264174&view=diff == --- cfe/trunk/lib/Frontend/InitPreprocessor.cpp (original) +++ cfe/trunk/lib/Frontend/InitPreprocessor.cpp Wed Mar 23 13:00:22 2016 @@ -972,6 +972,10 @@ void clang::InitializePreprocessor( PP.getDiagnostics()); } + // Exit the command line and go back to (2 is LC_LEAVE). + if (!PP.getLangOpts().AsmPreprocessor) +Builder.append("# 1 \"\" 2"); + // If -imacros are specified, include them now. These are processed before // any -include directives. for (unsigned i = 0, e = InitOpts.MacroIncludes.size(); i != e; ++i) @@ -990,10 +994,6 @@ void clang::InitializePreprocessor( AddImplicitInclude(Builder, Path); } - // Exit the command line and go back to (2 is LC_LEAVE). - if (!PP.getLangOpts().AsmPreprocessor) -Builder.append("# 1 \"\" 2"); - // Instruct the preprocessor to skip the preamble. PP.setSkipMainFilePreamble(InitOpts.PrecompiledPreambleBytes.first, InitOpts.PrecompiledPreambleBytes.second); Modified: cfe/trunk/test/Driver/cl-pch-showincludes.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-pch-showincludes.cpp?rev=264174&r1=264173&r2=264174&view=diff == --- cfe/trunk/test/Driver/cl-pch-showincludes.cpp (original) +++ cfe/trunk/test/Driver/cl-pch-showincludes.cpp Wed Mar 23 13:00:22 2016 @@ -8,16 +8,17 @@ // When building the pch, header1.h
Re: [PATCH] D18401: clang-cl: Include /FI headers in /showIncludes output.
thakis closed this revision. thakis added a comment. …and landed in r264174. http://reviews.llvm.org/D18401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18271: Avoid -Wshadow warnings about constructor parameters named after fields
On Wed, Mar 23, 2016 at 11:03 AM, Alexander Kornienko via cfe-commits < cfe-commits@lists.llvm.org> wrote: > alexfh added a subscriber: alexfh. > alexfh added a comment. > > As a data point: I ran -Wshadow on our code base with a similar, but > simpler patch (http://reviews.llvm.org/D18395, just disables the warning > on ctor parameters named after fields). It removes more than half of the > hits (from ~100k initially) and a random sample of ~100 removed removed > hits didn't contain any issues that we'd like to still be warning about. > > Ultimately, I think, this diagnostic should be made much less noisy to be > actually useful. So maybe even starting with a removal of a larger subset > of warnings (and then gradually adding back the cases that seem to be > important to flag) would be better. > Right - my concern is that for people who are already using the warning, that might be a loss of functionality for them (if they've gone to the hassle/code convention of not naming members the same as ctor parameters, etc) & so they'll start missing mistakes they thought they were guarded against. > > > > Comment at: lib/Sema/SemaExpr.cpp:9887 > @@ -9854,1 +9886,3 @@ > if (ConvTy == Compatible) { > + const Expr *InnerLHS = LHSExpr->IgnoreParenCasts(); > + const DeclRefExpr *DRE = dyn_cast(InnerLHS); > > Why this change? > > > http://reviews.llvm.org/D18271 > > > > ___ > 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] D18271: Avoid -Wshadow warnings about constructor parameters named after fields
alexfh added a comment. In http://reviews.llvm.org/D18271#378196, @dblaikie wrote: > It's not just modifications of shadowed variables that are a problem - one of > the one's I'm concerned we should catch is: > > struct foo { > std::unique_ptr p; > foo(std::unique_ptr p) : p(std::move(p)) { > f(*p); > } > }; This seems totally out of scope for -Wshadow, and seeing a -Wshadow warning on this wouldn't help (imagine a few tens of lines before the `*p` is used). http://reviews.llvm.org/D18271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18271: Avoid -Wshadow warnings about constructor parameters named after fields
thakis added a subscriber: thakis. thakis added a comment. FWIW we don't currently use this warning on Chromium because it's way to noisy. So something like this looks like a great change to me. dblaikie, are you aware of any codebases that use this warning in its current form? http://reviews.llvm.org/D18271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18271: Avoid -Wshadow warnings about constructor parameters named after fields
bcraig added a subscriber: bcraig. bcraig added a comment. In http://reviews.llvm.org/D18271#381744, @thakis wrote: > FWIW we don't currently use this warning on Chromium because it's way to > noisy. So something like this looks like a great change to me. > > dblaikie, are you aware of any codebases that use this warning in its current > form? Not dblaikie, but I've used the GCC version of -Wshadow on reasonably large code bases before. The ctor pattern that this is trying to squelch was certainly a significant portion of the warnings, particularly when (old) boost headers got involved. I believe that newer versions of boost (~1.50 and newer?) attempt to be -Wshadow clean in the headers. http://reviews.llvm.org/D18271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r264178 - clang-cl: Add more tests for the interaction of /FI and /Yc /Yu.
Author: nico Date: Wed Mar 23 13:17:02 2016 New Revision: 264178 URL: http://llvm.org/viewvc/llvm-project?rev=264178&view=rev Log: clang-cl: Add more tests for the interaction of /FI and /Yc /Yu. Most things even work; see the included FIXMEs for things that need polishing. Also don't warn about unused flags for the `/Yuh2.h /FIh1.h /FIh2.h`. The common case is that the pch was built with `/Ych2.h /FIh1.h /FIh2.h`, so h1.h is in the PCH, and we shouldn't warn about /FIh1.h not having an effect. (If we wanted to get fancy, we could store the list of -include flags in the pch and then check that it matches later on.) Added: cfe/trunk/test/Driver/Inputs/header0.h cfe/trunk/test/Driver/Inputs/header4.h Modified: cfe/trunk/lib/Driver/Tools.cpp cfe/trunk/test/Driver/cl-pch-showincludes.cpp Modified: cfe/trunk/lib/Driver/Tools.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=264178&r1=264177&r2=264178&view=diff == --- cfe/trunk/lib/Driver/Tools.cpp (original) +++ cfe/trunk/lib/Driver/Tools.cpp Wed Mar 23 13:17:02 2016 @@ -428,7 +428,7 @@ void Clang::AddPreprocessingOptions(Comp // FIXME: The code here assumes that /Yc and /Yu refer to the same file. // cl.exe seems to support both flags with different values, but that // seems strange (which flag does /Fp now refer to?), so don't implement - // that until someone needs that. + // that until someone needs it. int PchIndex = YcIndex != -1 ? YcIndex : YuIndex; if (PchIndex != -1) { if (isa(JA)) { @@ -438,8 +438,10 @@ void Clang::AddPreprocessingOptions(Comp continue; } else { // When using the pch, skip all includes prior to the pch. - if (AI < PchIndex) + if (AI < PchIndex) { +A->claim(); continue; + } if (AI == PchIndex) { A->claim(); CmdArgs.push_back("-include-pch"); Added: cfe/trunk/test/Driver/Inputs/header0.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/header0.h?rev=264178&view=auto == (empty) Added: cfe/trunk/test/Driver/Inputs/header4.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/header4.h?rev=264178&view=auto == (empty) Modified: cfe/trunk/test/Driver/cl-pch-showincludes.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-pch-showincludes.cpp?rev=264178&r1=264177&r2=264178&view=diff == --- cfe/trunk/test/Driver/cl-pch-showincludes.cpp (original) +++ cfe/trunk/test/Driver/cl-pch-showincludes.cpp Wed Mar 23 13:17:02 2016 @@ -22,3 +22,32 @@ // CHECK-YU-NOT: Note: including file: {{.*header1.h}} // CHECK-YU-NOT: Note: including file: {{.*header2.h}} // CHECK-YU: Note: including file: {{[^ ]*header3.h}} + +// When not using pch at all, all the /FI files are printed. +// RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /FIheader2.h /c /Fo%t -- %s \ +// RUN: | FileCheck --strict-whitespace -check-prefix=CHECK-FI %s +// CHECK-FI: Note: including file: {{[^ ]*header2.h}} +// CHECK-FI: Note: including file: {{[^ ]*header1.h}} +// CHECK-FI: Note: including file: {{[^ ]*header3.h}} + +// Also check that /FI arguments before the /Yc / /Yu flags are printed right. + +// /FI flags before the /Yc arg should be printed, /FI flags after it shouldn't. +// RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Ycheader2.h /FIheader0.h /FIheader2.h /FIheader4.h /Fp%t.pch /c /Fo%t -- %s \ +// RUN: | FileCheck --strict-whitespace -check-prefix=CHECK-YCFI %s +// FIXME: The order of the first two lines here must be reversed: +// CHECK-YCFI: Note: including file: {{[^ ]*header2.h}} +// CHECK-YCFI: Note: including file: {{[^ ]*header0.h}} +// FIXME: header1.h should be indented one more: +// CHECK-YCFI: Note: including file: {{[^ ]*header1.h}} +// CHECK-YCFI: Note: including file: {{[^ ]*header4.h}} +// CHECK-YCFI: Note: including file: {{[^ ]*header3.h}} + +// RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Yuheader2.h /FIheader0.h /FIheader2.h /FIheader4.h /Fp%t.pch /c /Fo%t -- %s \ +// RUN: | FileCheck --strict-whitespace -check-prefix=CHECK-YUFI %s +// CHECK-YUFI-NOT: Note: including file: {{.*pch}} +// CHECK-YUFI-NOT: Note: including file: {{.*header0.h}} +// CHECK-YUFI-NOT: Note: including file: {{.*header2.h}} +// CHECK-YUFI-NOT: Note: including file: {{.*header1.h}} +// CHECK-YUFI: Note: including file: {{[^ ]*header4.h}} +// CHECK-YUFI: Note: including file: {{[^ ]*header3.h}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18271: Avoid -Wshadow warnings about constructor parameters named after fields
thakis added a comment. In http://reviews.llvm.org/D18271#381758, @bcraig wrote: > In http://reviews.llvm.org/D18271#381744, @thakis wrote: > > > FWIW we don't currently use this warning on Chromium because it's way to > > noisy. So something like this looks like a great change to me. > > > > dblaikie, are you aware of any codebases that use this warning in its > > current form? > > > Not dblaikie, but I've used the GCC version of -Wshadow on reasonably large > code bases before. The ctor pattern that this is trying to squelch was > certainly a significant portion of the warnings, particularly when (old) > boost headers got involved. > > I believe that newer versions of boost (~1.50 and newer?) attempt to be > -Wshadow clean in the headers. Did squelching this ctor pattern find any bugs? Do you know if boost would miss the warning firing in this case? http://reviews.llvm.org/D18271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18380: [CUDA] Implement -fcuda-relaxed-constexpr, and enable it by default.
jlebar added inline comments. Comment at: include/clang/Driver/CC1Options.td:702-703 @@ -701,2 +701,4 @@ HelpText<"Allow variadic functions in CUDA device code.">; +def fcuda_relaxed_constexpr : Flag<["-"], "fcuda-relaxed-constexpr">, + HelpText<"Treat constexpr functions as __host__ __device__.">; rsmith wrote: > Is there a better name we can use for this? I don't think this is "relaxed" > in any obvious sense. `-fcuda-host-device-constexpr` or > `-fcuda-constexpr-on-device` might be clearer? "relaxed constexpr" is nvidia's term -- do you think it might be helpful to use the same terminology? I understand there's some prior art here, with respect to clang accepting gcc's flags, although the situation here is of course different. Comment at: lib/Driver/Tools.cpp:3597 @@ -3596,2 +3596,3 @@ CmdArgs.push_back("-fcuda-disable-target-call-checks"); +CmdArgs.push_back("-fcuda-relaxed-constexpr"); } rsmith wrote: > For flags that are enabled by default, we usually have the -cc1 flag be a > `-fno-*` flag. This allows people to use (for instance) `clang blah.cu > -Xclang -fno-cuda-relaxed-constexpr` if necessary. Yeah, Artem and I had a discussion about this yesterday. As you can see, there are two other flags above which are turned on by default -- these also lack -fno variants. I think it would be good to be consistent here. I'm tempted to add another patch below this one which makes the other two -fno, then we can make this one -fno as well. It seems that convention is to just get rid of the existing non-fno flags, rather than leave both positive and negative versions. Does that sound OK to you? http://reviews.llvm.org/D18380 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18271: Avoid -Wshadow warnings about constructor parameters named after fields
rsmith added a comment. I think a reasonable approach would be: "do not warn on shadowing if the idiom of naming a constructor parameter after a class member is used, and the class member is initialized from that constructor parameter, and all uses of the parameter in the constructor body would still be valid if it were declared `const`". In particular, I think we should probably warn if a non-const reference is bound to the parameter, or if the constructor's member initializer list does not contain `field(field)`. (I agree that catching `A(X x) : x(std::move(x)) { use(x); }` is outside the scope of this change.) Comment at: lib/Sema/SemaDecl.cpp:6400 @@ +6399,3 @@ +if (isa(NewDC) && isa(D)) + return; + } Instead of redoing class member lookup every time we check to see if a variable is modifiable, perhaps you could populate a set of shadowed-but-not-diagnosed `VarDecl*`s here? That way, you can also suppress the duplicate diagnostics if the variable is modified multiple times by removing it from the set. Comment at: lib/Sema/SemaExpr.cpp:9663 @@ +9662,3 @@ + if (!S.getLangOpts().CPlusPlus || + S.Diags.isIgnored(diag::warn_decl_shadow, Loc)) +return; This query is actually quite expensive (more so than some or all of the other checks below). http://reviews.llvm.org/D18271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18275: [ASTMatchers] Add own version of VariadicFunction.
alexfh added a comment. Still LG. Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:82 @@ +81,3 @@ + ResultT operator()(ArrayRef Args) const { +SmallVector InnerArgs; +for (const ArgT &Arg : Args) > On the other hand, constructing the matchers has never been the performance > bottleneck of the framework. I was confused then. I thought, the operator() in question gets called when traversing the AST, not when constructing the matcher. http://reviews.llvm.org/D18275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision
zaks.anna added a comment. Could you add the reduced false positives to the tests file? > As far as I see the diagnostics are showing the proper path now.. What do you mean? Does this refer to supplying more information the the path about why the error occurs? Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:12 @@ +11,3 @@ +// +// ConversionChecker generates a subset of the warnings that are reported by +// Wconversion. It is designed to be an alternative to Wconversion. danielmarjamaki wrote: > Thanks! I have tried to do that. Can you describe what it does without referencing Wconversion? A reader might not know what that warning does. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:85 @@ +84,3 @@ + if (!N) +return; + danielmarjamaki wrote: > I renamed and changed these functions. Hope we all like it better now. The > name is now "greaterEqualState" and it returns the state when the value is > greater or equal. If there is no such state it returns nullptr. > > As far as I see the diagnostics are showing the proper path now.. Cppcheck is probably performing control-flow sensitive analysis, which is completely different than the algorithm that is used by the Clang Static Analyzer, which performs path-sensitive analysis. The meaning of may and must are different. It is very important to understand the fundamentals behind how the analyzer works. Please, watch the video and let me know if you are interested in more information on symbolic execution. When we talk about possible values for a variable(or a symbol) along one path in the static analyzer we can have both may and must: - may be greater than: StGE != nullptr - is greater than: StGE && !StLT The whole point behind "assumeDual" is to allow us to differentiate between them. Your function specifically only cares about the "must" case. It would be a bug if it checked that the state is a "may" state. http://reviews.llvm.org/D13126 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18380: [CUDA] Implement -fcuda-relaxed-constexpr, and enable it by default.
rsmith added inline comments. Comment at: include/clang/Driver/CC1Options.td:702-703 @@ -701,2 +701,4 @@ HelpText<"Allow variadic functions in CUDA device code.">; +def fcuda_relaxed_constexpr : Flag<["-"], "fcuda-relaxed-constexpr">, + HelpText<"Treat constexpr functions as __host__ __device__.">; jlebar wrote: > rsmith wrote: > > Is there a better name we can use for this? I don't think this is "relaxed" > > in any obvious sense. `-fcuda-host-device-constexpr` or > > `-fcuda-constexpr-on-device` might be clearer? > "relaxed constexpr" is nvidia's term -- do you think it might be helpful to > use the same terminology? I understand there's some prior art here, with > respect to clang accepting gcc's flags, although the situation here is of > course different. I think it's problematic to use that terminology, as "relaxed constexpr" is also used to describe the C++14 `constexpr` rules (see [n3652](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3652.html)). Comment at: lib/Driver/Tools.cpp:3597 @@ -3596,2 +3596,3 @@ CmdArgs.push_back("-fcuda-disable-target-call-checks"); +CmdArgs.push_back("-fcuda-relaxed-constexpr"); } jlebar wrote: > rsmith wrote: > > For flags that are enabled by default, we usually have the -cc1 flag be a > > `-fno-*` flag. This allows people to use (for instance) `clang blah.cu > > -Xclang -fno-cuda-relaxed-constexpr` if necessary. > Yeah, Artem and I had a discussion about this yesterday. As you can see, > there are two other flags above which are turned on by default -- these also > lack -fno variants. > > I think it would be good to be consistent here. I'm tempted to add another > patch below this one which makes the other two -fno, then we can make this > one -fno as well. It seems that convention is to just get rid of the > existing non-fno flags, rather than leave both positive and negative versions. > > Does that sound OK to you? Yes, that sounds fine. http://reviews.llvm.org/D18380 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r264182 - clang-cl: Fix remaining bugs in interaction of /Yc and /FI /showIncludes.
Author: nico Date: Wed Mar 23 13:46:57 2016 New Revision: 264182 URL: http://llvm.org/viewvc/llvm-project?rev=264182&view=rev Log: clang-cl: Fix remaining bugs in interaction of /Yc and /FI /showIncludes. Instead of putting the /Yc header into ExtraDeps, give DependencyOutputOptions a dedicated field for /Yc mode, and let HeaderIncludesCallback hang on to the full DependencyOutputOptions object, not just ExtraDeps. Reverts parts of r263352 that are now no longer needed. Modified: cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h cfe/trunk/include/clang/Frontend/Utils.h cfe/trunk/lib/Frontend/CompilerInstance.cpp cfe/trunk/lib/Frontend/HeaderIncludeGen.cpp cfe/trunk/test/Driver/cl-pch-showincludes.cpp Modified: cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h?rev=264182&r1=264181&r2=264182&view=diff == --- cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h (original) +++ cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h Wed Mar 23 13:46:57 2016 @@ -50,6 +50,9 @@ public: /// A list of filenames to be used as extra dependencies for every target. std::vector ExtraDeps; + /// In /showIncludes mode, pretend the main TU is a header with this name. + std::string ShowIncludesPretendHeader; + /// \brief The file to write GraphViz-formatted header dependencies to. std::string DOTOutputFile; Modified: cfe/trunk/include/clang/Frontend/Utils.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/Utils.h?rev=264182&r1=264181&r2=264182&view=diff == --- cfe/trunk/include/clang/Frontend/Utils.h (original) +++ cfe/trunk/include/clang/Frontend/Utils.h Wed Mar 23 13:46:57 2016 @@ -142,15 +142,13 @@ public: /// AttachDependencyGraphGen - Create a dependency graph generator, and attach /// it to the given preprocessor. - void AttachDependencyGraphGen(Preprocessor &PP, StringRef OutputFile, -StringRef SysRoot); +void AttachDependencyGraphGen(Preprocessor &PP, StringRef OutputFile, + StringRef SysRoot); /// AttachHeaderIncludeGen - Create a header include list generator, and attach /// it to the given preprocessor. /// -/// \param ExtraHeaders - If not empty, will write the header filenames, just -/// like they were included during a regular preprocessing. Useful for -/// implicit include dependencies, like sanitizer blacklists. +/// \param DepOpts - Options controlling the output. /// \param ShowAllHeaders - If true, show all header information instead of just /// headers following the predefines buffer. This is useful for making sure /// includes mentioned on the command line are also reported, but differs from @@ -160,7 +158,7 @@ public: /// \param ShowDepth - Whether to indent to show the nesting of the includes. /// \param MSStyle - Whether to print in cl.exe /showIncludes style. void AttachHeaderIncludeGen(Preprocessor &PP, -const std::vector &ExtraHeaders, +const DependencyOutputOptions &DepOpts, bool ShowAllHeaders = false, StringRef OutputPath = "", bool ShowDepth = true, bool MSStyle = false); Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=264182&r1=264181&r2=264182&view=diff == --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original) +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Wed Mar 23 13:46:57 2016 @@ -360,18 +360,18 @@ void CompilerInstance::createPreprocesso // Handle generating header include information, if requested. if (DepOpts.ShowHeaderIncludes) -AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps); +AttachHeaderIncludeGen(*PP, DepOpts); if (!DepOpts.HeaderIncludeOutputFile.empty()) { StringRef OutputPath = DepOpts.HeaderIncludeOutputFile; if (OutputPath == "-") OutputPath = ""; -AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps, +AttachHeaderIncludeGen(*PP, DepOpts, /*ShowAllHeaders=*/true, OutputPath, /*ShowDepth=*/false); } if (DepOpts.PrintShowIncludes) { -AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps, +AttachHeaderIncludeGen(*PP, DepOpts, /*ShowAllHeaders=*/true, /*OutputPath=*/"", /*ShowDepth=*/true, /*MSStyle=*/true); } @@ -765,7 +765,7 @@ bool CompilerInstance::InitializeSourceM /*SuggestedModule=*/nullptr, /*SkipCache=*/true); // Also add the header to /showInc
Re: [PATCH] D17821: [OpenCL] Complete image types support
mgrang added a subscriber: mgrang. Comment at: include/clang/AST/ASTContext.h:903 @@ +902,3 @@ +#define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \ + CanQualType SingletonId; +#include "clang/AST/OpenCLImageTypes.def" remove extra spacing in front of the \ http://reviews.llvm.org/D17821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D17412: PR19957: [OpenCL] incorrectly accepts implicit address space conversion with ternary operator
yaxunl removed rL LLVM as the repository for this revision. yaxunl updated this revision to Diff 51460. yaxunl marked 13 inline comments as done. yaxunl added a comment. Added comments. Revised test. http://reviews.llvm.org/D17412 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/AST/ASTContext.cpp lib/Sema/SemaExpr.cpp test/CodeGenOpenCL/address-spaces-conversions.cl test/SemaOpenCL/address-spaces-conversions-cl2.0.cl Index: test/SemaOpenCL/address-spaces-conversions-cl2.0.cl === --- test/SemaOpenCL/address-spaces-conversions-cl2.0.cl +++ test/SemaOpenCL/address-spaces-conversions-cl2.0.cl @@ -225,3 +225,69 @@ // expected-error@-2{{passing '__constant int *' to parameter of type '__generic int *' changes address space of pointer}} #endif } + +void test_ternary() { + AS int *var_cond; + generic int *arg_gen; + global int *arg_glob; + arg_gen = 0 ? var_cond : arg_glob; +#ifdef CONSTANT +// expected-error@-2{{conditional operator with the second and third operands of type ('__constant int *' and '__global int *') which are pointers to non-overlapping address spaces}} +#endif + + local int *arg_loc; + arg_gen = 0 ? var_cond : arg_loc; +#ifndef GENERIC +// expected-error-re@-2{{conditional operator with the second and third operands of type ('__{{global|constant}} int *' and '__local int *') which are pointers to non-overlapping address spaces}} +#endif + + constant int *arg_const; + var_cond = 0 ? var_cond : arg_const; +#ifndef CONSTANT +// expected-error-re@-2{{conditional operator with the second and third operands of type ('__{{global|generic}} int *' and '__constant int *') which are pointers to non-overlapping address spaces}} +#endif + + private int *arg_priv; + arg_gen = 0 ? var_cond : arg_priv; +#ifndef GENERIC +// expected-error-re@-2{{conditional operator with the second and third operands of type ('__{{global|constant}} int *' and 'int *') which are pointers to non-overlapping address spaces}} +#endif + + arg_gen = 0 ? var_cond : arg_gen; +#ifdef CONSTANT +// expected-error@-2{{conditional operator with the second and third operands of type ('__constant int *' and '__generic int *') which are pointers to non-overlapping address spaces}} +#endif + + void *var_void_gen; + global char *arg_glob_ch; + var_void_gen = 0 ? var_cond : arg_glob_ch; +#ifdef CONSTANT +// expected-error@-2{{conditional operator with the second and third operands of type ('__constant int *' and '__global char *') which are pointers to non-overlapping address spaces}} +#endif + + local char *arg_loc_ch; + var_void_gen = 0 ? var_cond : arg_loc_ch; +#ifndef GENERIC +// expected-error-re@-2{{conditional operator with the second and third operands of type ('__{{global|constant}} int *' and '__local char *') which are pointers to non-overlapping address spaces}} +#endif + + constant void *var_void_const; + constant char *arg_const_ch; + var_void_const = 0 ? var_cond : arg_const_ch; +#ifndef CONSTANT +// expected-error-re@-2{{conditional operator with the second and third operands of type ('__{{global|generic}} int *' and '__constant char *') which are pointers to non-overlapping address spaces}} +#endif + + private char *arg_priv_ch; + var_void_gen = 0 ? var_cond : arg_priv_ch; +#ifndef GENERIC +// expected-error-re@-2{{conditional operator with the second and third operands of type ('__{{global|constant}} int *' and 'char *') which are pointers to non-overlapping address spaces}} +#endif + + generic char *arg_gen_ch; + var_void_gen = 0 ? var_cond : arg_gen_ch; +#ifdef CONSTANT +// expected-error@-2{{conditional operator with the second and third operands of type ('__constant int *' and '__generic char *') which are pointers to non-overlapping address spaces}} +#endif +} + Index: test/CodeGenOpenCL/address-spaces-conversions.cl === --- test/CodeGenOpenCL/address-spaces-conversions.cl +++ test/CodeGenOpenCL/address-spaces-conversions.cl @@ -3,20 +3,35 @@ // test that we generate address space casts everywhere we need conversions of // pointers to different address spaces -void test(global int *arg_glob, generic int *arg_gen) { +void test(global int *arg_glob, generic int *arg_gen, global float *arg_glob_f, + generic void *arg_gen_v) { int var_priv; + arg_gen = arg_glob; // implicit cast global -> generic // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to i32 addrspace(4)* + arg_gen = &var_priv; // implicit cast with obtaining adr, private -> generic // CHECK: %{{[0-9]+}} = addrspacecast i32* %var_priv to i32 addrspace(4)* + arg_glob = (global int *)arg_gen; // explicit cast // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(4)* %{{[0-9]+}} to i32 addrspace(1)* + global int *var_glob = (global int *)arg_glob; // explicit cast in the same address space // CHECK-NOT: %{{[0-9]+}} = ad
Re: [PATCH] D18271: Avoid -Wshadow warnings about constructor parameters named after fields
bcraig added a subscriber: mclow.lists. bcraig added a comment. > > Not dblaikie, but I've used the GCC version of -Wshadow on reasonably large > > code bases before. The ctor pattern that this is trying to squelch was > > certainly a significant portion of the warnings, particularly when (old) > > boost headers got involved. > > > > > > I believe that newer versions of boost (~1.50 and newer?) attempt to be > > -Wshadow clean in the headers. > > > Did squelching this ctor pattern find any bugs? Do you know if boost would > miss the warning firing in this case? I don't recall fixing any bugs with the ctor pattern. I can only recall two different classes of things that I have fixed that were flagged with -Wshadow. 1. Long, horrible, deeply nested method, with a local that was trivially shadowing a different local in a larger scope. 2. Mutex lock guards and similar "sentry" objects. "std::unique_lock(name_of_a_member);" The user wanted that to lock 'name_of_member' and release it at end of scope, but instead it declared a default constructed unique_lock called 'name_of_a_member' that shadows the member variable. I've had to fix this kind of bug a number of times, and it is my main justification for turning on -Wshadow in the first place. On that note, I'm really looking forward to the day I can say "auto guard = std::unique_lock(name_of_member);" to effectively get rid of this problem. I doubt boost would miss the warning, and probably wouldn't even notice (barring overlapping list membership). If you're really wondering if boost would care / notice, @mclow.lists would have a better idea. http://reviews.llvm.org/D18271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D18412: [clang-tidy] Add support for different char-types for the readability-redundant-string-cstr checker.
etienneb created this revision. etienneb added a reviewer: alexfh. etienneb added a subscriber: cfe-commits. The current checker is able to recognize std::string but do not recognize other string variants. This path is adding the support for any string define with basic_string without consiring the the char type. The most common variant is: std::wstring based on wchar_t. There are also other string variants added to the standard: u16string, u32string, etc... http://reviews.llvm.org/D18412 Files: clang-tidy/readability/RedundantStringCStrCheck.cpp test/clang-tidy/readability-redundant-string-cstr.cpp Index: test/clang-tidy/readability-redundant-string-cstr.cpp === --- test/clang-tidy/readability-redundant-string-cstr.cpp +++ test/clang-tidy/readability-redundant-string-cstr.cpp @@ -12,14 +12,20 @@ const C *c_str() const; }; typedef basic_string, std::allocator> string; +typedef basic_string, std::allocator> wstring; +typedef basic_string, std::allocator> u16string; +typedef basic_string, std::allocator> u32string; } + namespace llvm { struct StringRef { StringRef(const char *p); StringRef(const std::string &); }; } +// Tests for std::string. + void f1(const std::string &s) { f1(s.c_str()); // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr] @@ -39,3 +45,27 @@ // CHECK-FIXES: {{^ }}std::string s;{{$}} // CHECK-FIXES-NEXT: {{^ }}f3(s);{{$}} } + +// Tests for std::wstring. + +void g1(const std::wstring &s) { + g1(s.c_str()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr] + // CHECK-FIXES: {{^ }}f1(s);{{$}} +} + +// Tests for std::u16string. + +void h1(const std::u16string &s) { + h1(s.c_str()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr] + // CHECK-FIXES: {{^ }}f1(s);{{$}} +} + +// Tests for std::u32string. + +void k1(const std::u32string &s) { + k1(s.c_str()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr] + // CHECK-FIXES: {{^ }}f1(s);{{$}} +} Index: clang-tidy/readability/RedundantStringCStrCheck.cpp === --- clang-tidy/readability/RedundantStringCStrCheck.cpp +++ clang-tidy/readability/RedundantStringCStrCheck.cpp @@ -66,11 +66,11 @@ } const char StringConstructor[] = -"::std::basic_string, std::allocator >" +"::std::basic_string<.*, std::char_traits<.*>, std::allocator<.*> >" "::basic_string"; const char StringCStrMethod[] = -"::std::basic_string, std::allocator >" +"::std::basic_string<.*, std::char_traits<.*>, std::allocator<.*> >" "::c_str"; } // end namespace @@ -89,18 +89,18 @@ const auto StringConstructorExpr = expr(anyOf( cxxConstructExpr( argumentCountIs(1), - hasDeclaration(cxxMethodDecl(hasName(StringConstructor, + hasDeclaration(cxxMethodDecl(matchesName(StringConstructor, cxxConstructExpr( argumentCountIs(2), - hasDeclaration(cxxMethodDecl(hasName(StringConstructor))), + hasDeclaration(cxxMethodDecl(matchesName(StringConstructor))), // If present, the second argument is the alloc object which must not // be present explicitly. hasArgument(1, cxxDefaultArgExpr(); // Match a call to the string 'c_str()' method. - const auto StringCStrCallExpr = cxxMemberCallExpr( -callee(memberExpr().bind("member")), -callee(cxxMethodDecl(hasName(StringCStrMethod))), + const auto StringCStrCallExpr = + cxxMemberCallExpr(callee(memberExpr().bind("member")), +callee(cxxMethodDecl(matchesName(StringCStrMethod))), on(expr().bind("arg"))).bind("call"); Finder->addMatcher( Index: test/clang-tidy/readability-redundant-string-cstr.cpp === --- test/clang-tidy/readability-redundant-string-cstr.cpp +++ test/clang-tidy/readability-redundant-string-cstr.cpp @@ -12,14 +12,20 @@ const C *c_str() const; }; typedef basic_string, std::allocator> string; +typedef basic_string, std::allocator> wstring; +typedef basic_string, std::allocator> u16string; +typedef basic_string, std::allocator> u32string; } + namespace llvm { struct StringRef { StringRef(const char *p); StringRef(const std::string &); }; } +// Tests for std::string. + void f1(const std::string &s) { f1(s.c_str()); // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr] @@ -39,3 +45,27 @@ // CHECK-FIXES: {{^ }}std::string s;{{$}} // CHECK-FIXES-NEXT: {{^ }}f3(s);{{$}} } + +// Tests for std::wstring. + +void g1(c
Re: [PATCH] D18412: [clang-tidy] Add support for different char-types for the readability-redundant-string-cstr checker.
can you just match on the name of the template instead? On Wed, Mar 23, 2016 at 12:46 PM, Etienne Bergeron via cfe-commits < cfe-commits@lists.llvm.org> wrote: > etienneb created this revision. > etienneb added a reviewer: alexfh. > etienneb added a subscriber: cfe-commits. > > The current checker is able to recognize std::string but do not recognize > other string variants. > This path is adding the support for any string define with basic_string > without consiring the > the char type. > > The most common variant is: std::wstring based on wchar_t. > > There are also other string variants added to the standard: u16string, > u32string, etc... > > http://reviews.llvm.org/D18412 > > Files: > clang-tidy/readability/RedundantStringCStrCheck.cpp > test/clang-tidy/readability-redundant-string-cstr.cpp > > Index: test/clang-tidy/readability-redundant-string-cstr.cpp > === > --- test/clang-tidy/readability-redundant-string-cstr.cpp > +++ test/clang-tidy/readability-redundant-string-cstr.cpp > @@ -12,14 +12,20 @@ >const C *c_str() const; > }; > typedef basic_string, std::allocator> > string; > +typedef basic_string, > std::allocator> wstring; > +typedef basic_string, > std::allocator> u16string; > +typedef basic_string, > std::allocator> u32string; > } > + > namespace llvm { > struct StringRef { >StringRef(const char *p); >StringRef(const std::string &); > }; > } > > +// Tests for std::string. > + > void f1(const std::string &s) { >f1(s.c_str()); >// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` > [readability-redundant-string-cstr] > @@ -39,3 +45,27 @@ >// CHECK-FIXES: {{^ }}std::string s;{{$}} >// CHECK-FIXES-NEXT: {{^ }}f3(s);{{$}} > } > + > +// Tests for std::wstring. > + > +void g1(const std::wstring &s) { > + g1(s.c_str()); > + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` > [readability-redundant-string-cstr] > + // CHECK-FIXES: {{^ }}f1(s);{{$}} > +} > + > +// Tests for std::u16string. > + > +void h1(const std::u16string &s) { > + h1(s.c_str()); > + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` > [readability-redundant-string-cstr] > + // CHECK-FIXES: {{^ }}f1(s);{{$}} > +} > + > +// Tests for std::u32string. > + > +void k1(const std::u32string &s) { > + k1(s.c_str()); > + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` > [readability-redundant-string-cstr] > + // CHECK-FIXES: {{^ }}f1(s);{{$}} > +} > Index: clang-tidy/readability/RedundantStringCStrCheck.cpp > === > --- clang-tidy/readability/RedundantStringCStrCheck.cpp > +++ clang-tidy/readability/RedundantStringCStrCheck.cpp > @@ -66,11 +66,11 @@ > } > > const char StringConstructor[] = > -"::std::basic_string, > std::allocator >" > +"::std::basic_string<.*, std::char_traits<.*>, std::allocator<.*> >" > "::basic_string"; > > const char StringCStrMethod[] = > -"::std::basic_string, > std::allocator >" > +"::std::basic_string<.*, std::char_traits<.*>, std::allocator<.*> >" > "::c_str"; > > } // end namespace > @@ -89,18 +89,18 @@ >const auto StringConstructorExpr = expr(anyOf( >cxxConstructExpr( >argumentCountIs(1), > - hasDeclaration(cxxMethodDecl(hasName(StringConstructor, > + hasDeclaration(cxxMethodDecl(matchesName(StringConstructor, >cxxConstructExpr( >argumentCountIs(2), > - hasDeclaration(cxxMethodDecl(hasName(StringConstructor))), > + hasDeclaration(cxxMethodDecl(matchesName(StringConstructor))), >// If present, the second argument is the alloc object which > must not >// be present explicitly. >hasArgument(1, cxxDefaultArgExpr(); > >// Match a call to the string 'c_str()' method. > - const auto StringCStrCallExpr = cxxMemberCallExpr( > -callee(memberExpr().bind("member")), > - > callee(cxxMethodDecl(hasName(StringCStrMethod))), > + const auto StringCStrCallExpr = > + cxxMemberCallExpr(callee(memberExpr().bind("member")), > + > callee(cxxMethodDecl(matchesName(StringCStrMethod))), > on(expr().bind("arg"))).bind("call"); > >Finder->addMatcher( > > > > ___ > 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] D18264: [clang-tidy] misc-assign-operator-signature checker checks return value of all assign operators
baloghadamsoftware added a comment. Oh, I was searching in the C++ Core Guidlines, but at the wrong place because I did not find it. So I will change this option to be enabled by default. DSL users who do not follow this rule for the non copy and non move assign operators can disable it. http://reviews.llvm.org/D18264 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r264184 - Make sure to perform dependent access checks when instantiating a
Author: rsmith Date: Wed Mar 23 15:07:07 2016 New Revision: 264184 URL: http://llvm.org/viewvc/llvm-project?rev=264184&view=rev Log: Make sure to perform dependent access checks when instantiating a lambda-expression. We don't actually instantiate the closure type / operator() in the template in order to produce the closure type / operator() in the instantiation, so this isn't caught by the normal path. Modified: cfe/trunk/lib/Sema/SemaLambda.cpp cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp cfe/trunk/test/SemaCXX/access.cpp Modified: cfe/trunk/lib/Sema/SemaLambda.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLambda.cpp?rev=264184&r1=264183&r2=264184&view=diff == --- cfe/trunk/lib/Sema/SemaLambda.cpp (original) +++ cfe/trunk/lib/Sema/SemaLambda.cpp Wed Mar 23 15:07:07 2016 @@ -809,19 +809,14 @@ void Sema::ActOnStartOfLambdaDefinition( bool KnownDependent = false; LambdaScopeInfo *const LSI = getCurLambda(); assert(LSI && "LambdaScopeInfo should be on stack!"); - TemplateParameterList *TemplateParams = -getGenericLambdaTemplateParameterList(LSI, *this); - if (Scope *TmplScope = CurScope->getTemplateParamParent()) { -// Since we have our own TemplateParams, so check if an outer scope -// has template params, only then are we in a dependent scope. -if (TemplateParams) { - TmplScope = TmplScope->getParent(); - TmplScope = TmplScope ? TmplScope->getTemplateParamParent() : nullptr; -} -if (TmplScope && !TmplScope->decl_empty()) + // The lambda-expression's closure type might be dependent even if its + // semantic context isn't, if it appears within a default argument of a + // function template. + if (Scope *TmplScope = CurScope->getTemplateParamParent()) +if (!TmplScope->decl_empty()) KnownDependent = true; - } + // Determine the signature of the call operator. TypeSourceInfo *MethodTyInfo; bool ExplicitParams = true; Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp?rev=264184&r1=264183&r2=264184&view=diff == --- cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp (original) +++ cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp Wed Mar 23 15:07:07 2016 @@ -729,6 +729,11 @@ namespace { } SemaRef.CurrentInstantiationScope->InstantiatedLocal(Old, New); + + // We recreated a local declaration, but not by instantiating it. There + // may be pending dependent diagnostics to produce. + if (auto *DC = dyn_cast(Old)) +SemaRef.PerformDependentDiagnostics(DC, TemplateArgs); } /// \brief Transform the definition of the given declaration by Modified: cfe/trunk/test/SemaCXX/access.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/access.cpp?rev=264184&r1=264183&r2=264184&view=diff == --- cfe/trunk/test/SemaCXX/access.cpp (original) +++ cfe/trunk/test/SemaCXX/access.cpp Wed Mar 23 15:07:07 2016 @@ -158,3 +158,14 @@ namespace LocalExternVar { int array[sizeof(test::private_struct)]; // expected-error {{private}} } + +namespace ThisLambdaIsNotMyFriend { + class A { +friend class D; +static void foo(); // expected-note {{here}} + }; + template void foo() { +[]() { A::foo(); }(); // expected-error {{private}} + } + void bar() { foo(); } // expected-note {{instantiation}} +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18347: [PATCH] Fix thread_annotation negtest for thread safety.
EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM! Thanks for the complete patch! The "no-threads" bot should now be green so future breakage will be detected immediately! http://reviews.llvm.org/D18347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18412: [clang-tidy] Add support for different char-types for the readability-redundant-string-cstr checker.
etienneb updated this revision to Diff 51464. etienneb marked an inline comment as done. etienneb added a comment. Fix unittests. http://reviews.llvm.org/D18412 Files: clang-tidy/readability/RedundantStringCStrCheck.cpp test/clang-tidy/readability-redundant-string-cstr.cpp Index: test/clang-tidy/readability-redundant-string-cstr.cpp === --- test/clang-tidy/readability-redundant-string-cstr.cpp +++ test/clang-tidy/readability-redundant-string-cstr.cpp @@ -12,14 +12,20 @@ const C *c_str() const; }; typedef basic_string, std::allocator> string; +typedef basic_string, std::allocator> wstring; +typedef basic_string, std::allocator> u16string; +typedef basic_string, std::allocator> u32string; } + namespace llvm { struct StringRef { StringRef(const char *p); StringRef(const std::string &); }; } +// Tests for std::string. + void f1(const std::string &s) { f1(s.c_str()); // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr] @@ -39,3 +45,27 @@ // CHECK-FIXES: {{^ }}std::string s;{{$}} // CHECK-FIXES-NEXT: {{^ }}f3(s);{{$}} } + +// Tests for std::wstring. + +void g1(const std::wstring &s) { + g1(s.c_str()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr] + // CHECK-FIXES: {{^ }}g1(s);{{$}} +} + +// Tests for std::u16string. + +void h1(const std::u16string &s) { + h1(s.c_str()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr] + // CHECK-FIXES: {{^ }}h1(s);{{$}} +} + +// Tests for std::u32string. + +void k1(const std::u32string &s) { + k1(s.c_str()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr] + // CHECK-FIXES: {{^ }}k1(s);{{$}} +} Index: clang-tidy/readability/RedundantStringCStrCheck.cpp === --- clang-tidy/readability/RedundantStringCStrCheck.cpp +++ clang-tidy/readability/RedundantStringCStrCheck.cpp @@ -66,11 +66,11 @@ } const char StringConstructor[] = -"::std::basic_string, std::allocator >" +"::std::basic_string<.*, std::char_traits<.*>, std::allocator<.*> >" "::basic_string"; const char StringCStrMethod[] = -"::std::basic_string, std::allocator >" +"::std::basic_string<.*, std::char_traits<.*>, std::allocator<.*> >" "::c_str"; } // end namespace @@ -89,18 +89,18 @@ const auto StringConstructorExpr = expr(anyOf( cxxConstructExpr( argumentCountIs(1), - hasDeclaration(cxxMethodDecl(hasName(StringConstructor, + hasDeclaration(cxxMethodDecl(matchesName(StringConstructor, cxxConstructExpr( argumentCountIs(2), - hasDeclaration(cxxMethodDecl(hasName(StringConstructor))), + hasDeclaration(cxxMethodDecl(matchesName(StringConstructor))), // If present, the second argument is the alloc object which must not // be present explicitly. hasArgument(1, cxxDefaultArgExpr(); // Match a call to the string 'c_str()' method. - const auto StringCStrCallExpr = cxxMemberCallExpr( -callee(memberExpr().bind("member")), -callee(cxxMethodDecl(hasName(StringCStrMethod))), + const auto StringCStrCallExpr = + cxxMemberCallExpr(callee(memberExpr().bind("member")), +callee(cxxMethodDecl(matchesName(StringCStrMethod))), on(expr().bind("arg"))).bind("call"); Finder->addMatcher( Index: test/clang-tidy/readability-redundant-string-cstr.cpp === --- test/clang-tidy/readability-redundant-string-cstr.cpp +++ test/clang-tidy/readability-redundant-string-cstr.cpp @@ -12,14 +12,20 @@ const C *c_str() const; }; typedef basic_string, std::allocator> string; +typedef basic_string, std::allocator> wstring; +typedef basic_string, std::allocator> u16string; +typedef basic_string, std::allocator> u32string; } + namespace llvm { struct StringRef { StringRef(const char *p); StringRef(const std::string &); }; } +// Tests for std::string. + void f1(const std::string &s) { f1(s.c_str()); // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr] @@ -39,3 +45,27 @@ // CHECK-FIXES: {{^ }}std::string s;{{$}} // CHECK-FIXES-NEXT: {{^ }}f3(s);{{$}} } + +// Tests for std::wstring. + +void g1(const std::wstring &s) { + g1(s.c_str()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr] + // CHECK-FIXES: {{^ }}g1(s);{{$}} +} + +// Tests for std::u16string. + +void h1(const std::u16string &s) { + h1(s.c_str()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundan
Re: [PATCH] D18412: [clang-tidy] Add support for different char-types for the readability-redundant-string-cstr checker.
mamai added a subscriber: mamai. mamai added a comment. nit: in summary, consiring -> considering ? Comment at: test/clang-tidy/readability-redundant-string-cstr.cpp:54 @@ +53,3 @@ + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr] + // CHECK-FIXES: {{^ }}f1(s);{{$}} +} Isn't this a copy-paste error to reference f1 ? Same question for line 62 and 70... http://reviews.llvm.org/D18412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18412: [clang-tidy] Add support for different char-types for the readability-redundant-string-cstr checker.
etienneb added a comment. In http://reviews.llvm.org/D18412#381843, @dblaikie wrote: > can you just match on the name of the template instead? I'm not sure to get your point? Are you proposing to match "basic_string" instead of the whole regexp? I'm in favor of that change as I don't see any counter example or any false positive. In fact, many checkers are using hasName("basic_string"). I just kept the change as minimal as possible to keep the same semantic and allow matching more cases. Example from RedundantStringInitCheck.cpp: // Match string constructor. const auto StringConstructorExpr = expr(anyOf( cxxConstructExpr(argumentCountIs(1), hasDeclaration(cxxMethodDecl(hasName("basic_string", Comment at: test/clang-tidy/readability-redundant-string-cstr.cpp:54 @@ +53,3 @@ + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant call to `c_str()` [readability-redundant-string-cstr] + // CHECK-FIXES: {{^ }}f1(s);{{$}} +} mamai wrote: > Isn't this a copy-paste error to reference f1 ? Same question for line 62 and > 70... Good catch. http://reviews.llvm.org/D18412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18380: [CUDA] Implement -fcuda-relaxed-constexpr, and enable it by default.
jlebar added inline comments. Comment at: include/clang/Driver/CC1Options.td:702-703 @@ -701,2 +701,4 @@ HelpText<"Allow variadic functions in CUDA device code.">; +def fcuda_relaxed_constexpr : Flag<["-"], "fcuda-relaxed-constexpr">, + HelpText<"Treat constexpr functions as __host__ __device__.">; rsmith wrote: > jlebar wrote: > > rsmith wrote: > > > Is there a better name we can use for this? I don't think this is > > > "relaxed" in any obvious sense. `-fcuda-host-device-constexpr` or > > > `-fcuda-constexpr-on-device` might be clearer? > > "relaxed constexpr" is nvidia's term -- do you think it might be helpful to > > use the same terminology? I understand there's some prior art here, with > > respect to clang accepting gcc's flags, although the situation here is of > > course different. > I think it's problematic to use that terminology, as "relaxed constexpr" is > also used to describe the C++14 `constexpr` rules (see > [n3652](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3652.html)). Heh, I can't argue with that. Comment at: lib/Driver/Tools.cpp:3597 @@ -3596,2 +3596,3 @@ CmdArgs.push_back("-fcuda-disable-target-call-checks"); +CmdArgs.push_back("-fcuda-relaxed-constexpr"); } rsmith wrote: > jlebar wrote: > > rsmith wrote: > > > For flags that are enabled by default, we usually have the -cc1 flag be a > > > `-fno-*` flag. This allows people to use (for instance) `clang blah.cu > > > -Xclang -fno-cuda-relaxed-constexpr` if necessary. > > Yeah, Artem and I had a discussion about this yesterday. As you can see, > > there are two other flags above which are turned on by default -- these > > also lack -fno variants. > > > > I think it would be good to be consistent here. I'm tempted to add another > > patch below this one which makes the other two -fno, then we can make this > > one -fno as well. It seems that convention is to just get rid of the > > existing non-fno flags, rather than leave both positive and negative > > versions. > > > > Does that sound OK to you? > Yes, that sounds fine. Okay, thank you. After talking to Artem, we're just going to remove those two flags entirely. So after we convert relaxed-constexpr to an fno flag, there should be no changes to this file in this patch. http://reviews.llvm.org/D18380 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits