[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)
lebedev.ri added a comment. In https://reviews.llvm.org/D44883#1068400, @brooksmoses wrote: > I have noticed two things when attempting to release LLVM with this revision > internally at Google: > > 1. It's catching real bugs, all in constructors where someone wrote "member_ > = member_" when they meant "member_ = member". Nice, just like the one that caused me to write this :) > 2. It's catching at least as many cases of tests where people are > intentionally testing that self-assignment doesn't corrupt the data values. > > Thus, this seems valuable but problematic, and the problems mean that > initially we're facing turning off -Wself-assign completely until this is > resolved. That's definitely an issue, and at least means that this needs to > be placed under its own -W option. And the real bugs it's finding seem to be > very specific, and could be found by a more-focused warning. See all the previous disscussion about that earlier in the differential / mail thread.. I guess some consensus needs to be found, and then i'll implement it. > EDIT: Looks like "var = *&var" is supposed to avoid the warning, so my > (now-deleted) complaints about this not having a source workaround are > erroneous. See release notes. > I still find the fact that "var = (var)" doesn't suppress this to be > surprising, but it's not critical. Repository: rC Clang https://reviews.llvm.org/D44883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44948: Add diagnostic -Waggregate-ctors, "aggregate type has user-declared constructors"
Quuxplusone abandoned this revision. Quuxplusone added a comment. This patch's new home is https://github.com/Quuxplusone/clang/commits/aggregate-ctors — which is where I should have put it to begin with. Repository: rC Clang https://reviews.llvm.org/D44948 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45613: [X86] Introduce archs: goldmont-plus & tremont
This revision was automatically updated to reflect the committed changes. Closed by commit rL330110: [X86] Introduce archs: goldmont-plus & tremont (authored by GBuella, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D45613 Files: cfe/trunk/include/clang/Basic/X86Target.def cfe/trunk/lib/Basic/Targets/X86.cpp cfe/trunk/test/Driver/x86-march.c cfe/trunk/test/Misc/target-invalid-cpu-note.c cfe/trunk/test/Preprocessor/predefined-arch-macros.c Index: cfe/trunk/lib/Basic/Targets/X86.cpp === --- cfe/trunk/lib/Basic/Targets/X86.cpp +++ cfe/trunk/lib/Basic/Targets/X86.cpp @@ -244,6 +244,14 @@ setFeatureEnabledImpl(Features, "fxsr", true); break; + case CK_Tremont: +setFeatureEnabledImpl(Features, "cldemote", true); +setFeatureEnabledImpl(Features, "gfni", true); +LLVM_FALLTHROUGH; + case CK_GoldmontPlus: +setFeatureEnabledImpl(Features, "rdpid", true); +setFeatureEnabledImpl(Features, "sgx", true); +LLVM_FALLTHROUGH; case CK_Goldmont: setFeatureEnabledImpl(Features, "sha", true); setFeatureEnabledImpl(Features, "rdseed", true); @@ -930,6 +938,12 @@ case CK_Goldmont: defineCPUMacros(Builder, "goldmont"); break; + case CK_GoldmontPlus: +defineCPUMacros(Builder, "goldmont_plus"); +break; + case CK_Tremont: +defineCPUMacros(Builder, "tremont"); +break; case CK_Nehalem: case CK_Westmere: case CK_SandyBridge: Index: cfe/trunk/include/clang/Basic/X86Target.def === --- cfe/trunk/include/clang/Basic/X86Target.def +++ cfe/trunk/include/clang/Basic/X86Target.def @@ -104,6 +104,9 @@ PROC_ALIAS(Silvermont, "slm") PROC(Goldmont, "goldmont", PROC_64_BIT) +PROC(GoldmontPlus, "goldmont-plus", PROC_64_BIT) + +PROC(Tremont, "tremont", PROC_64_BIT) //@} /// \name Nehalem Index: cfe/trunk/test/Misc/target-invalid-cpu-note.c === --- cfe/trunk/test/Misc/target-invalid-cpu-note.c +++ cfe/trunk/test/Misc/target-invalid-cpu-note.c @@ -13,7 +13,7 @@ // X86: note: valid target CPU values are: i386, i486, winchip-c6, winchip2, c3, // X86-SAME: i586, pentium, pentium-mmx, pentiumpro, i686, pentium2, pentium3, // X86-SAME: pentium3m, pentium-m, c3-2, yonah, pentium4, pentium4m, prescott, -// X86-SAME: nocona, core2, penryn, bonnell, atom, silvermont, slm, goldmont, +// X86-SAME: nocona, core2, penryn, bonnell, atom, silvermont, slm, goldmont, goldmont-plus, tremont, // X86-SAME: nehalem, corei7, westmere, sandybridge, corei7-avx, ivybridge, // X86-SAME: core-avx-i, haswell, core-avx2, broadwell, skylake, skylake-avx512, // X86-SAME: skx, cannonlake, icelake-client, icelake-server, knl, knm, lakemont, k6, k6-2, k6-3, @@ -25,7 +25,7 @@ // RUN: not %clang_cc1 -triple x86_64--- -target-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix X86_64 // X86_64: error: unknown target CPU 'not-a-cpu' // X86_64: note: valid target CPU values are: nocona, core2, penryn, bonnell, -// X86_64-SAME: atom, silvermont, slm, goldmont, nehalem, corei7, westmere, +// X86_64-SAME: atom, silvermont, slm, goldmont, goldmont-plus, tremont, nehalem, corei7, westmere, // X86_64-SAME: sandybridge, corei7-avx, ivybridge, core-avx-i, haswell, // X86_64-SAME: core-avx2, broadwell, skylake, skylake-avx512, skx, cannonlake, // X86_64-SAME: icelake-client, icelake-server, knl, knm, k8, athlon64, athlon-fx, opteron, k8-sse3, Index: cfe/trunk/test/Preprocessor/predefined-arch-macros.c === --- cfe/trunk/test/Preprocessor/predefined-arch-macros.c +++ cfe/trunk/test/Preprocessor/predefined-arch-macros.c @@ -1371,6 +1371,7 @@ // CHECK_GLM_M64: #define __PRFCHW__ 1 // CHECK_GLM_M64: #define __RDRND__ 1 // CHECK_GLM_M64: #define __RDSEED__ 1 +// CHECK_GLM_M64: #define __SHA__ 1 // CHECK_GLM_M64: #define __SSE2__ 1 // CHECK_GLM_M64: #define __SSE3__ 1 // CHECK_GLM_M64: #define __SSE4_1__ 1 @@ -1387,6 +1388,146 @@ // CHECK_GLM_M64: #define __x86_64 1 // CHECK_GLM_M64: #define __x86_64__ 1 +// RUN: %clang -march=goldmont-plus -m32 -E -dM %s -o - 2>&1 \ +// RUN: -target i386-unknown-linux \ +// RUN: | FileCheck %s -check-prefix=CHECK_GLMP_M32 +// CHECK_GLMP_M32: #define __AES__ 1 +// CHECK_GLMP_M32: #define __CLFLUSHOPT__ 1 +// CHECK_GLMP_M32: #define __FSGSBASE__ 1 +// CHECK_GLMP_M32: #define __FXSR__ 1 +// CHECK_GLMP_M32: #define __MMX__ 1 +// CHECK_GLMP_M32: #define __MPX__ 1 +// CHECK_GLMP_M32: #define __PCLMUL__ 1 +// CHECK_GLMP_M32: #define __POPCNT__ 1 +// CHECK_GLMP_M32: #define __PRFCHW__ 1 +// CHECK_GLMP_M32: #define __RDPID__ 1 +// CHECK_GLMP_M32: #define __RDRND__ 1 +// CHECK_GLMP_M32: #define __RDSEED__ 1 +// CHECK_GLMP_M32: #define __SGX__ 1 +// CHECK_GLMP_M32: #define __SHA__ 1 +// CHECK_GLMP_M32: #define __SSE2__ 1 +
[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt
EricWF created this revision. EricWF added a reviewer: rsmith. Herald added a subscriber: mgrang. This is a work-in-progress attempt to add `operator<=>` rewriting. It's nowhere close to complete, but I would like some initial feedback on the direction. As currently implemented, rewritten and synthesized candidates are only partially checked for viability when they're added to the overload candidates (the conversion sequence from the argument type -> parameter type is checked, but nothing else). This can lead non-viable candidates being selected or causing ambiguity when final overload resolution is attempted. The solution implemented in this patch is to fully build "potentially viable" candidates if they are selected or cause ambiguity. If building the candidate fails, it is marked as non-viable, and a new best viable function is computed. The problem with this approach is that it's expensive and potentially wasteful. For example, if overload resolution results in ambiguity with `N` rewritten candidates with the same partial ordering, then all `N` candidates are fully built, and those results potentially thrown away. For builtin candidates this can be avoided by separating out the bits of `CheckBinaryOperands` which compute it's result type from the bits which actually build the expression and convert the arguments. Once we know the return type of the builtin, we can deduce if the comparison category type can be used in the expression `0 @ ` without much effect. However, for non-builtin overloads of `operator<=>` (which don't return a comparison category type), it seems non-trivial to check that `0 @ ` is valid without actually attempting to build the overloaded binary operator. @rsmith Could you provide initial feedback on this direction? Or any ideas you have about how to best implement it? Repository: rC Clang https://reviews.llvm.org/D45680 Files: include/clang/Sema/Overload.h include/clang/Sema/Sema.h lib/Sema/SemaOverload.cpp test/SemaCXX/compare-cxx2a.cpp Index: test/SemaCXX/compare-cxx2a.cpp === --- test/SemaCXX/compare-cxx2a.cpp +++ test/SemaCXX/compare-cxx2a.cpp @@ -293,20 +293,21 @@ template struct Tag {}; -// expected-note@+1 {{candidate}} -Tag<0> operator<=>(EnumA, EnumA) { - return {}; +std::strong_ordering operator<=>(EnumA, EnumA) { + return std::strong_ordering::equal; } -Tag<1> operator<=>(EnumA, EnumB) { - return {}; +// expected-note@+1 {{candidate function}}, +std::strong_ordering operator<=>(EnumA a, EnumB b) { + return ((int)a <=> (int)b); } void test_enum_ovl_provided() { auto r1 = (EnumA::A <=> EnumA::A); - ASSERT_EXPR_TYPE(r1, Tag<0>); + ASSERT_EXPR_TYPE(r1, std::strong_ordering); auto r2 = (EnumA::A <=> EnumB::B); - ASSERT_EXPR_TYPE(r2, Tag<1>); - (void)(EnumB::B <=> EnumA::A); // expected-error {{invalid operands to binary expression ('EnumCompareTests::EnumB' and 'EnumCompareTests::EnumA')}} + ASSERT_EXPR_TYPE(r2, std::strong_ordering); + (void)(EnumB::B <=> EnumA::A); // OK, chooses reverse order synthesized candidate. + (void)(EnumB::B <=> EnumC::C); // expected-error {{invalid operands to binary expression ('EnumCompareTests::EnumB' and 'EnumCompareTests::EnumC')}} } void enum_float_test() { @@ -375,3 +376,60 @@ ASSERT_EXPR_TYPE(r4, std::partial_ordering); } + +namespace TestRewritting { + +struct T { + int x; + // expected-note@+1 {{candidate}} + constexpr std::strong_ordering operator<=>(T y) const { +return (x <=> y.x); + } +}; + +struct U { + int x; + // FIXME: This diagnostic is terrible. + // expected-note@+1 {{candidate function not viable: requires single argument 'y', but 2 arguments were provided}} + constexpr std::strong_equality operator<=>(T y) const { +if (x == y.x) + return std::strong_equality::equal; +return std::strong_equality::nonequal; + } +}; + +struct X { int x; }; +struct Y { int x; }; +struct Tag {}; +// expected-note@+1 2 {{candidate}} +Tag operator<=>(X, Y) { + return {}; +} +// expected-note@+1 2 {{candidate}} +constexpr auto operator<=>(Y y, X x) { + return y.x <=> x.x; +} + +void foo() { + T t{42}; + T t2{0}; + U u{101}; + auto r1 = (t <=> u); + ASSERT_EXPR_TYPE(r1, std::strong_equality); + auto r2 = (t <=> t2); + ASSERT_EXPR_TYPE(r2, std::strong_ordering); + + auto r3 = t == u; + ASSERT_EXPR_TYPE(r3, bool); + + (void)(t < u); // expected-error {{invalid operands to binary expression ('TestRewritting::T' and 'TestRewritting::U')}} + + constexpr X x{1}; + constexpr Y y{2}; + constexpr auto r4 = (y < x); + static_assert(r4 == false); + constexpr auto r5 = (x < y); + static_assert(r5 == true); +} + +} // namespace TestRewritting Index: lib/Sema/SemaOverload.cpp === --- lib/Sema/SemaOverload.cpp +++ lib/Sema/SemaOverload.cpp @@ -831,6 +831,29 @@ } } +const ImplicitConversionSequence & +OverloadCandi
[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)
brooksmoses added a comment. Some further statistics, now that I've done a full cleanup on our code: 8 actual bugs newly found by -Wself-assign-field. (Thank you!) 2 actual bugs newly found by -Wself-assign 6 instances of redundant code newly found by -Wself-assign 90 (approximately) instances of intentional self assignments in tests that now need "*&" annotations. That seems like an awfully large amount of noise for the -Wself-assign part of this, and I continue feeling rather dubious about whether it should be part of -Wall. By contrast, the -Wself-assign-field part has been entirely true positives. Repository: rC Clang https://reviews.llvm.org/D44883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43578: -ftime-report switch support in Clang
mzolotukhin added a comment. > What kinds of would be useful to you? (How do you want the > runtime of Clang broken down?) Are vertical slices (through all Clang's > various layers and potentially parts of LLVM) -- as this patch will produce > -- useful, or would you really want horizontal slices (as in, what part of > Clang is actually spending the time)? Or just anything that's basically > expected to be consistent from run to run, so you can identify that something > has slowed down, even if you don't have enough information to really know > what? For me "something has slowed down" would be enough. I.e. even if "parse templates" would be erroneously attributed to 90% time spent in front-end, I would be able to see a jump from 90% to 95%. While these numbers are not reflecting the actual ratio, they still will indicate changes. I would find horizontal slicing more interesting, as we can get vertical slices from profilers. I.e. if in LLVM profile I see that time spent in APInt::add has grown a lot, I'd like to know which pass started to use it more extensively - it's rarely the case that APInt::add slowed down itself. > I think we need to fix the overlap issue as a prerequisite to adding timers > with large amounts of overlap, especially self-overlap. Otherwise the numbers > will likely do more harm than good, as they will significantly misattribute > runtime. Fortunately, I think that should only require some relatively small > changes to the LLVM timer infrastructure. I definitely would support that :) > Well, that depends on the profiler. With some profilers, you can just attach > a profiler to your entire compilation and then ask it what the hottest > functions were. But sure, if you have existing infrastructure built around > -ftime-report, I can see that it makes sense to reuse that infrastructure. Yeah, that was exactly my point. > While LLVM may have some overlap between regions, the vertical timing slices > are still pretty well aligned with the horizontal functionality slices. I > expect the problems in Clang to be a lot worse. Suppose you enter N levels of > parsing templates, and then trigger a whole bunch of template instantiation, > AST deserialization, and code generation. Let's say that takes 1s in total. > With this patch, I think we'd end up attributing Ns of compile time to > "parsing templates", which is clearly very far from the truth, but will > likely be listed as (by far) the slowest thing in the compilation. This does > not even rise to the level of "not-perfect", it's going to be actively > misleading in a lot of cases, and won't even necessarily point anywhere near > the problematic spot. > I think with this patch and no fix to the overlap problem, we will find > ourselves frequently fielding bugs where people say "X is slow" when it > actually isn't. Plus I think we have the opportunity to deliver something > that's hugely useful and not much harder to achieve (providing timing > information that relates back to the source code), and I'd prefer we spend > our complexity budget for this feature there. I see your point, and I agree it would be really misleading if e.g the sum of timers won't match the total time due to self-overlaps. I think adding these timers still might be worthwhile as my guess is that no one usually uses them anyway unless they know what to expect from the reported timers, but I might be mistaken here. And anyway, if you think it's possible to fix the issue first, it's totally fine with me. Thanks, Michael https://reviews.llvm.org/D43578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r330112 - Clean carriage returns from lib/ and include/. NFC.
Author: malcolm.parsons Date: Mon Apr 16 01:31:08 2018 New Revision: 330112 URL: http://llvm.org/viewvc/llvm-project?rev=330112&view=rev Log: Clean carriage returns from lib/ and include/. NFC. Summary: Clean carriage returns from lib/ and include/. NFC. (I have to make this change locally in order for `git diff` to show sane output after I edit a file, so I might as well ask for it to be committed. I don't have commit privs myself.) (Without this patch, `git rebase`ing any change involving SemaDeclCXX.cpp is a real nightmare. :( So while I have no right to ask for this to be committed, geez would it make my workflow easier if it were.) Here's the command I used to reformat things. (Requires bash and OSX/FreeBSD sed.) git grep -l $'\r' lib include | xargs sed -i -e $'s/\r//' find lib include -name '*-e' -delete Reviewers: malcolm.parsons Reviewed By: malcolm.parsons Subscribers: emaste, krytarowski, cfe-commits Differential Revision: https://reviews.llvm.org/D45591 Patch by Arthur O'Dwyer. Modified: cfe/trunk/lib/AST/ASTDumper.cpp cfe/trunk/lib/AST/ExprConstant.cpp cfe/trunk/lib/CodeGen/CGExprScalar.cpp cfe/trunk/lib/Sema/SemaDeclCXX.cpp cfe/trunk/lib/Sema/SemaPseudoObject.cpp cfe/trunk/lib/Sema/SemaTemplate.cpp cfe/trunk/lib/Serialization/ASTWriterStmt.cpp cfe/trunk/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp Modified: cfe/trunk/lib/AST/ASTDumper.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=330112&r1=330111&r2=330112&view=diff == --- cfe/trunk/lib/AST/ASTDumper.cpp (original) +++ cfe/trunk/lib/AST/ASTDumper.cpp Mon Apr 16 01:31:08 2018 @@ -2213,14 +2213,14 @@ void ASTDumper::VisitArrayInitIndexExpr( } void ASTDumper::VisitUnaryOperator(const UnaryOperator *Node) { - VisitExpr(Node); - OS << " " << (Node->isPostfix() ? "postfix" : "prefix") - << " '" << UnaryOperator::getOpcodeStr(Node->getOpcode()) << "'"; - if (!Node->canOverflow()) -OS << " cannot overflow"; -} - -void ASTDumper::VisitUnaryExprOrTypeTraitExpr( + VisitExpr(Node); + OS << " " << (Node->isPostfix() ? "postfix" : "prefix") + << " '" << UnaryOperator::getOpcodeStr(Node->getOpcode()) << "'"; + if (!Node->canOverflow()) +OS << " cannot overflow"; +} + +void ASTDumper::VisitUnaryExprOrTypeTraitExpr( const UnaryExprOrTypeTraitExpr *Node) { VisitExpr(Node); switch(Node->getKind()) { Modified: cfe/trunk/lib/AST/ExprConstant.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=330112&r1=330111&r2=330112&view=diff == --- cfe/trunk/lib/AST/ExprConstant.cpp (original) +++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Apr 16 01:31:08 2018 @@ -3328,12 +3328,12 @@ static bool handleAssignment(EvalInfo &I } CompleteObject Obj = findCompleteObject(Info, E, AK_Assign, LVal, LValType); - return Obj && modifySubobject(Info, E, Obj, LVal.Designator, Val); -} - -namespace { -struct CompoundAssignSubobjectHandler { - EvalInfo &Info; + return Obj && modifySubobject(Info, E, Obj, LVal.Designator, Val); +} + +namespace { +struct CompoundAssignSubobjectHandler { + EvalInfo &Info; const Expr *E; QualType PromotedLHSType; BinaryOperatorKind Opcode; @@ -3449,13 +3449,13 @@ static bool handleCompoundAssignment( return Obj && findSubobject(Info, E, Obj, LVal.Designator, Handler); } -namespace { -struct IncDecSubobjectHandler { - EvalInfo &Info; - const UnaryOperator *E; - AccessKinds AccessKind; - APValue *Old; - +namespace { +struct IncDecSubobjectHandler { + EvalInfo &Info; + const UnaryOperator *E; + AccessKinds AccessKind; + APValue *Old; + typedef bool result_type; bool checkConst(QualType QT) { @@ -3521,20 +3521,20 @@ struct IncDecSubobjectHandler { } bool WasNegative = Value.isNegative(); -if (AccessKind == AK_Increment) { - ++Value; - - if (!WasNegative && Value.isNegative() && E->canOverflow()) { -APSInt ActualValue(Value, /*IsUnsigned*/true); -return HandleOverflow(Info, E, ActualValue, SubobjType); - } -} else { - --Value; - - if (WasNegative && !Value.isNegative() && E->canOverflow()) { -unsigned BitWidth = Value.getBitWidth(); -APSInt ActualValue(Value.sext(BitWidth + 1), /*IsUnsigned*/false); -ActualValue.setBit(BitWidth); +if (AccessKind == AK_Increment) { + ++Value; + + if (!WasNegative && Value.isNegative() && E->canOverflow()) { +APSInt ActualValue(Value, /*IsUnsigned*/true); +return HandleOverflow(Info, E, ActualValue, SubobjType); + } +} else { + --Value; + + if (WasNegative && !Value.isNegative() && E->canOverflow()) { +unsigned BitWidth = Value.getBitWidth(); +APSInt ActualValue(Value.sext(BitWidth + 1), /*IsUnsigned*/false); +ActualValue.setBit(B
[PATCH] D45591: Clean carriage returns from lib/ and include/. NFC.
This revision was automatically updated to reflect the committed changes. Closed by commit rC330112: Clean carriage returns from lib/ and include/. NFC. (authored by malcolm.parsons, committed by ). Repository: rC Clang https://reviews.llvm.org/D45591 Files: lib/AST/ASTDumper.cpp lib/AST/ExprConstant.cpp lib/CodeGen/CGExprScalar.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaPseudoObject.cpp lib/Sema/SemaTemplate.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Checkers/PaddingChecker.cpp Index: lib/CodeGen/CGExprScalar.cpp === --- lib/CodeGen/CGExprScalar.cpp +++ lib/CodeGen/CGExprScalar.cpp @@ -162,13 +162,13 @@ // we can elide the overflow check. if (!Op.mayHaveIntegerOverflow()) return true; - - // If a unary op has a widened operand, the op cannot overflow. - if (const auto *UO = dyn_cast(Op.E)) -return !UO->canOverflow(); - - // We usually don't need overflow checks for binops with widened operands. - // Multiplication with promoted unsigned operands is a special case. + + // If a unary op has a widened operand, the op cannot overflow. + if (const auto *UO = dyn_cast(Op.E)) +return !UO->canOverflow(); + + // We usually don't need overflow checks for binops with widened operands. + // Multiplication with promoted unsigned operands is a special case. const auto *BO = cast(Op.E); auto OptionalLHSTy = getUnwidenedIntegerType(Ctx, BO->getLHS()); if (!OptionalLHSTy) @@ -1871,13 +1871,13 @@ return Builder.CreateAdd(InVal, Amount, Name); case LangOptions::SOB_Undefined: if (!CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) - return Builder.CreateNSWAdd(InVal, Amount, Name); -// Fall through. - case LangOptions::SOB_Trapping: -if (!E->canOverflow()) - return Builder.CreateNSWAdd(InVal, Amount, Name); -return EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(E, InVal, IsInc)); - } + return Builder.CreateNSWAdd(InVal, Amount, Name); +// Fall through. + case LangOptions::SOB_Trapping: +if (!E->canOverflow()) + return Builder.CreateNSWAdd(InVal, Amount, Name); +return EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(E, InVal, IsInc)); + } llvm_unreachable("Unknown SignedOverflowBehaviorTy"); } @@ -1953,15 +1953,15 @@ value = Builder.getTrue(); // Most common case by far: integer increment. - } else if (type->isIntegerType()) { -// Note that signed integer inc/dec with width less than int can't -// overflow because of promotion rules; we're just eliding a few steps here. -if (E->canOverflow() && type->isSignedIntegerOrEnumerationType()) { - value = EmitIncDecConsiderOverflowBehavior(E, value, isInc); -} else if (E->canOverflow() && type->isUnsignedIntegerType() && - CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) { - value = - EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(E, value, isInc)); + } else if (type->isIntegerType()) { +// Note that signed integer inc/dec with width less than int can't +// overflow because of promotion rules; we're just eliding a few steps here. +if (E->canOverflow() && type->isSignedIntegerOrEnumerationType()) { + value = EmitIncDecConsiderOverflowBehavior(E, value, isInc); +} else if (E->canOverflow() && type->isUnsignedIntegerType() && + CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) { + value = + EmitOverflowCheckedBinOp(createBinOpInfoFromIncDec(E, value, isInc)); } else { llvm::Value *amt = llvm::ConstantInt::get(value->getType(), amount, true); value = Builder.CreateAdd(value, amt, isInc ? "inc" : "dec"); Index: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp === --- lib/StaticAnalyzer/Checkers/PaddingChecker.cpp +++ lib/StaticAnalyzer/Checkers/PaddingChecker.cpp @@ -1,330 +1,330 @@ -//===- PaddingChecker.cpp *- C++ -*-==// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===--===// -// -// This file defines a checker that checks for padding that could be -// removed by re-ordering members. -// -//===--===// - -#include "ClangSACheckers.h" -#include "clang/AST/CharUnits.h" -#include "clang/AST/DeclTemplate.h" -#include "clang/AST/RecordLayout.h" -#include "clang/AST/RecursiveASTVisitor.h" -#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" -#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" -#include "clang/StaticAnalyzer/Core/Checker.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" -#include "llvm/ADT/SmallString.h
[PATCH] D43578: -ftime-report switch support in Clang
avt77 added a comment. In https://reviews.llvm.org/D43578#1067995, @rsmith wrote: > I think https://reviews.llvm.org/D45619 is a good change, and I'd like to see > that get committed. Could you give me LGTM in this case? I'm going to publish "recursive"(ovelaped) timers but I'd like to base it on https://reviews.llvm.org/D45619. https://reviews.llvm.org/D43578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43578: -ftime-report switch support in Clang
avt77 added a comment. In https://reviews.llvm.org/D43578#1068417, @rsmith wrote: > I think we need to fix the overlap issue as a prerequisite to adding timers > with large amounts of overlap, especially self-overlap. Otherwise the numbers > will likely do more harm than good, as they will significantly misattribute > runtime. Fortunately, I think that should only require some relatively small > changes to the LLVM timer infrastructure. JFYI: the given patch introduces non-recursive (self-overlaps) timers only. I'm going to introduce self-overlaps timers when https://reviews.llvm.org/D45619 is committed. https://reviews.llvm.org/D43578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)
brooksmoses added a comment. A further concern about this in the general case from the reviewer of one of my test-cleanup changes: The "var = *&var" idiom is not necessarily equivalent to "var = var" in cases of user-defined types, because operator& may be overloaded. Repository: rC Clang https://reviews.llvm.org/D44883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)
lebedev.ri added a comment. Re false-positives - at least two [post-]reviewers need to agree on the way forward (see previous comments, mail thread), and then i will implement it. In https://reviews.llvm.org/D44883#1068576, @brooksmoses wrote: > A further concern about this in the general case from the reviewer of one of > my test-cleanup changes: The "var = *&var" idiom is not necessarily > equivalent to "var = var" in cases of user-defined types, because operator& > may be overloaded. Release notes state: If you are doing such an assignment intentionally, e.g. in a unit test for a data structure, the warning can be suppressed by adding ``*&`` to the right-hand side or casting it to the appropriate reference type. So it could also be `var = static_cast(var);` Repository: rC Clang https://reviews.llvm.org/D44883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45578: Add a command line option `fregister_global_dtors_with_atexit` to register destructor functions annotated with __attribute__((destructor)) using __cxa_atexit or atexit.
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thank you. Minor request and then LGTM. Comment at: lib/CodeGen/ItaniumCXXABI.cpp:2202 +// __attribute__((destructor)) in a constructor function. +addr = llvm::Constant::getNullValue(CGF.Int8PtrTy); + You should probably mention that using null here is okay because this value is just passed back as an argument to the destructor function. Repository: rC Clang https://reviews.llvm.org/D45578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)
chandlerc added a comment. These kinds of improvements to warnings are awesome, but the way we are deploying them presents serious challenges to adoption which I think we need to address. I think significant new warning functionality should as a matter of course go behind some warning group at least initially. I'd be fine with this being a generic "-beta" group or some such, and I would be very happy with a clear timeline for merging some of these flags away. But I think we need to give users of Clang the ability to stage the deployment of a new compiler and the cleanup of their code more effectively. Secondly, there seems to be some interest in splitting off `OP=` combined assignement-and-functionality warnings, but delays around getting consensus. While we wait, users (including us) are forced to work around a problematic warning. This is pretty disruptive. I think we need to be much more rapid it moving this into a separate flag while it is under discussion because we don't have anything like the above policy in place and well followed, we are currently facing a really bad choice: disable the *entire* warning and potentially regress our codebase, or commit really ugly hacks to work around a bad warning. We can probably make it through this for this warning, but we need to prioritize cleaning this up because it didn't go behind any kind of staging, and going forward i strongly think we need a better way of staging these kinds of improvements. Note that this isn't novel -- the thread-safety warnings created exactly such a staging arrangement to help address these kinds of rollout issues. Repository: rC Clang https://reviews.llvm.org/D44883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output
Szelethus added a comment. I just had a look with `-analyzer-config notes-as-events=true`, and it works (with CodeChecker, at least). I'd still implement a better support for notes, but this is a great workaround for now. Thanks! Repository: rC Clang https://reviews.llvm.org/D45407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)
lebedev.ri added a comment. There are several options: 1. @rjmccall's idea: `-wtest` (lowercase), which in this case will disable that new code in `BuildOverloadedBinOp()`. i quite like it actually. 2. split it up like i had in the first revision - ``-Wself-assign-builtin``, ``-Wself-assign-field-builtin``; ``-Wself-assign-overloaded``, ``-Wself-assign-field-overloaded`` - we could just assume that `BuildOverloadedBinOp()` implies overloaded, - or check that the particular operator is non-trivial 3. ??? @rjmccall, @thakis, @dblaikie, @aaron.ballman, @brooksmoses, @chandlerc I'm going to go ahead and look into `1.`, since it does not seem there will be any consensus in a timely manner. Repository: rC Clang https://reviews.llvm.org/D44883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44381: [mips] Prevent PIC to be set to level 2
This revision was automatically updated to reflect the committed changes. Closed by commit rC330118: [mips] Prevent PIC to be set to level 2 (authored by abeserminji, committed by ). Repository: rL LLVM https://reviews.llvm.org/D44381 Files: lib/Driver/ToolChains/CommonArgs.cpp test/Driver/pic.c Index: test/Driver/pic.c === --- test/Driver/pic.c +++ test/Driver/pic.c @@ -292,9 +292,9 @@ // RUN: %clang -c %s -target mipsel-linux-android14 -### 2>&1 \ // RUN: | FileCheck %s --check-prefix=CHECK-PIC1 // RUN: %clang -c %s -target mipsel-linux-android16 -### 2>&1 \ -// RUN: | FileCheck %s --check-prefix=CHECK-PIE2 +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 // RUN: %clang -c %s -target mipsel-linux-android24 -### 2>&1 \ -// RUN: | FileCheck %s --check-prefix=CHECK-PIE2 +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 // // 64-bit Android targets are always PIE. // RUN: %clang -c %s -target aarch64-linux-android -### 2>&1 \ Index: lib/Driver/ToolChains/CommonArgs.cpp === --- lib/Driver/ToolChains/CommonArgs.cpp +++ lib/Driver/ToolChains/CommonArgs.cpp @@ -1007,12 +1007,17 @@ if ((ROPI || RWPI) && (PIC || PIE)) ToolChain.getDriver().Diag(diag::err_drv_ropi_rwpi_incompatible_with_pic); - // When targettng MIPS64 with N64, the default is PIC, unless -mno-abicalls is - // used. - if ((Triple.getArch() == llvm::Triple::mips64 || - Triple.getArch() == llvm::Triple::mips64el) && - Args.hasArg(options::OPT_mno_abicalls)) -return std::make_tuple(llvm::Reloc::Static, 0U, false); + if (Triple.getArch() == llvm::Triple::mips || + Triple.getArch() == llvm::Triple::mipsel || + Triple.getArch() == llvm::Triple::mips64 || + Triple.getArch() == llvm::Triple::mips64el) { +// When targettng MIPS with -mno-abicalls, it's always static. +if(Args.hasArg(options::OPT_mno_abicalls)) + return std::make_tuple(llvm::Reloc::Static, 0U, false); +// Unlike other architectures, MIPS, even with -fPIC/-mxgot/multigot, +// does not use PIC level 2 for historical reasons. +IsPICLevelTwo = false; + } if (PIC) return std::make_tuple(llvm::Reloc::PIC_, IsPICLevelTwo ? 2U : 1U, PIE); Index: test/Driver/pic.c === --- test/Driver/pic.c +++ test/Driver/pic.c @@ -292,9 +292,9 @@ // RUN: %clang -c %s -target mipsel-linux-android14 -### 2>&1 \ // RUN: | FileCheck %s --check-prefix=CHECK-PIC1 // RUN: %clang -c %s -target mipsel-linux-android16 -### 2>&1 \ -// RUN: | FileCheck %s --check-prefix=CHECK-PIE2 +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 // RUN: %clang -c %s -target mipsel-linux-android24 -### 2>&1 \ -// RUN: | FileCheck %s --check-prefix=CHECK-PIE2 +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 // // 64-bit Android targets are always PIE. // RUN: %clang -c %s -target aarch64-linux-android -### 2>&1 \ Index: lib/Driver/ToolChains/CommonArgs.cpp === --- lib/Driver/ToolChains/CommonArgs.cpp +++ lib/Driver/ToolChains/CommonArgs.cpp @@ -1007,12 +1007,17 @@ if ((ROPI || RWPI) && (PIC || PIE)) ToolChain.getDriver().Diag(diag::err_drv_ropi_rwpi_incompatible_with_pic); - // When targettng MIPS64 with N64, the default is PIC, unless -mno-abicalls is - // used. - if ((Triple.getArch() == llvm::Triple::mips64 || - Triple.getArch() == llvm::Triple::mips64el) && - Args.hasArg(options::OPT_mno_abicalls)) -return std::make_tuple(llvm::Reloc::Static, 0U, false); + if (Triple.getArch() == llvm::Triple::mips || + Triple.getArch() == llvm::Triple::mipsel || + Triple.getArch() == llvm::Triple::mips64 || + Triple.getArch() == llvm::Triple::mips64el) { +// When targettng MIPS with -mno-abicalls, it's always static. +if(Args.hasArg(options::OPT_mno_abicalls)) + return std::make_tuple(llvm::Reloc::Static, 0U, false); +// Unlike other architectures, MIPS, even with -fPIC/-mxgot/multigot, +// does not use PIC level 2 for historical reasons. +IsPICLevelTwo = false; + } if (PIC) return std::make_tuple(llvm::Reloc::PIC_, IsPICLevelTwo ? 2U : 1U, PIE); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44381: [mips] Prevent PIC to be set to level 2
This revision was automatically updated to reflect the committed changes. Closed by commit rL330118: [mips] Prevent PIC to be set to level 2 (authored by abeserminji, committed by ). Changed prior to commit: https://reviews.llvm.org/D44381?vs=142406&id=142609#toc Repository: rL LLVM https://reviews.llvm.org/D44381 Files: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp cfe/trunk/test/Driver/pic.c Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp === --- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp +++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp @@ -1007,12 +1007,17 @@ if ((ROPI || RWPI) && (PIC || PIE)) ToolChain.getDriver().Diag(diag::err_drv_ropi_rwpi_incompatible_with_pic); - // When targettng MIPS64 with N64, the default is PIC, unless -mno-abicalls is - // used. - if ((Triple.getArch() == llvm::Triple::mips64 || - Triple.getArch() == llvm::Triple::mips64el) && - Args.hasArg(options::OPT_mno_abicalls)) -return std::make_tuple(llvm::Reloc::Static, 0U, false); + if (Triple.getArch() == llvm::Triple::mips || + Triple.getArch() == llvm::Triple::mipsel || + Triple.getArch() == llvm::Triple::mips64 || + Triple.getArch() == llvm::Triple::mips64el) { +// When targettng MIPS with -mno-abicalls, it's always static. +if(Args.hasArg(options::OPT_mno_abicalls)) + return std::make_tuple(llvm::Reloc::Static, 0U, false); +// Unlike other architectures, MIPS, even with -fPIC/-mxgot/multigot, +// does not use PIC level 2 for historical reasons. +IsPICLevelTwo = false; + } if (PIC) return std::make_tuple(llvm::Reloc::PIC_, IsPICLevelTwo ? 2U : 1U, PIE); Index: cfe/trunk/test/Driver/pic.c === --- cfe/trunk/test/Driver/pic.c +++ cfe/trunk/test/Driver/pic.c @@ -292,9 +292,9 @@ // RUN: %clang -c %s -target mipsel-linux-android14 -### 2>&1 \ // RUN: | FileCheck %s --check-prefix=CHECK-PIC1 // RUN: %clang -c %s -target mipsel-linux-android16 -### 2>&1 \ -// RUN: | FileCheck %s --check-prefix=CHECK-PIE2 +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 // RUN: %clang -c %s -target mipsel-linux-android24 -### 2>&1 \ -// RUN: | FileCheck %s --check-prefix=CHECK-PIE2 +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 // // 64-bit Android targets are always PIE. // RUN: %clang -c %s -target aarch64-linux-android -### 2>&1 \ Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp === --- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp +++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp @@ -1007,12 +1007,17 @@ if ((ROPI || RWPI) && (PIC || PIE)) ToolChain.getDriver().Diag(diag::err_drv_ropi_rwpi_incompatible_with_pic); - // When targettng MIPS64 with N64, the default is PIC, unless -mno-abicalls is - // used. - if ((Triple.getArch() == llvm::Triple::mips64 || - Triple.getArch() == llvm::Triple::mips64el) && - Args.hasArg(options::OPT_mno_abicalls)) -return std::make_tuple(llvm::Reloc::Static, 0U, false); + if (Triple.getArch() == llvm::Triple::mips || + Triple.getArch() == llvm::Triple::mipsel || + Triple.getArch() == llvm::Triple::mips64 || + Triple.getArch() == llvm::Triple::mips64el) { +// When targettng MIPS with -mno-abicalls, it's always static. +if(Args.hasArg(options::OPT_mno_abicalls)) + return std::make_tuple(llvm::Reloc::Static, 0U, false); +// Unlike other architectures, MIPS, even with -fPIC/-mxgot/multigot, +// does not use PIC level 2 for historical reasons. +IsPICLevelTwo = false; + } if (PIC) return std::make_tuple(llvm::Reloc::PIC_, IsPICLevelTwo ? 2U : 1U, PIE); Index: cfe/trunk/test/Driver/pic.c === --- cfe/trunk/test/Driver/pic.c +++ cfe/trunk/test/Driver/pic.c @@ -292,9 +292,9 @@ // RUN: %clang -c %s -target mipsel-linux-android14 -### 2>&1 \ // RUN: | FileCheck %s --check-prefix=CHECK-PIC1 // RUN: %clang -c %s -target mipsel-linux-android16 -### 2>&1 \ -// RUN: | FileCheck %s --check-prefix=CHECK-PIE2 +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 // RUN: %clang -c %s -target mipsel-linux-android24 -### 2>&1 \ -// RUN: | FileCheck %s --check-prefix=CHECK-PIE2 +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 // // 64-bit Android targets are always PIE. // RUN: %clang -c %s -target aarch64-linux-android -### 2>&1 \ ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt
EricWF added a comment. I'm guessing this implementation is super non-conforming, since expressions are evaluated during overload resolution. My plan is to keep the general shape of this patch intact, but fix the eager building of expressions with something less wasteful. Repository: rC Clang https://reviews.llvm.org/D45680 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
bartpeeters added a comment. We too would like this feature to be in the official release Repository: rL LLVM https://reviews.llvm.org/D28462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45284: [RISCV] More validations on the input value of -march=
asb added inline comments. Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:50 + +static void getExtensionVersion(StringRef In, std::string &Version) { + auto I = In.begin(); You should probably document the limitation that this doesn't currently parse minor versions e.g. i2p0. Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:144-146 + // Currently LLVM does not support 'e'. + D.Diag(diag::err_drv_invalid_riscv_arch_name) +<< MArch << "unsupported standard user-level extension 'e'"; This could be tightened up by also rejected rv64e as invalid. Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:201-202 + if (StdExtsItr == StdExtsEnd) { +size_t Pos; +if (hasExtension(StdExts, std::string(1, c), Pos)) { + D.Diag(diag::err_drv_invalid_riscv_ext_arch_name) I'd suggest either just using StringRef::contains and getting rid of hasExtension, or adding a doc comment to hasExtension to explain its semantics. It might also be worth adding a comment to explain why you want to check the extension is present in the StdExts string (e.g. We have reached the end of the StdExts string. Either the current extension was given outside of the canonical order (in which case issue an error), or else no canonical ordering is defined meaning no error should be generated'. Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:211 // Move to next char to prevent repeated letter. ++StdExtsItr; Won't this now iterate StdExtsItr past StdExtsEnd if StdExtsItr == StdExtsEnd but the hasExtension call is false? Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:265-267 if (HasD && !HasF) - D.Diag(diag::err_drv_invalid_arch_name) << MArch; + D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch +<< "d requires f extension to also be specified"; Add a TODO about other dependencies perhaps? e.g. ef and efd are invalid, and q requires rv64. https://reviews.llvm.org/D45284 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45679: [clang-tidy] Add a new check, readability-unmodified-non-const-variable, that finds declarations of non-const variables that never get modified.
JonasToth added a comment. Hi @shuaiwang, i am currently working on the same check and we should definilty coordinate our work. :) What do you think about joining the forces and to work together on such a check? Things i would like to mention: - i differentiate between values and handles. I want to be able to transform to `const int * const` if possible, similar for references - i believe you do not test on all occasion, a variable can not be const, e.g. if it is casted, if it is catched by a lambda, array access, access through member functions and object members Did you run the check over a codebase like llvm yet and what did you find? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45679 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45679: [clang-tidy] Add a new check, readability-unmodified-non-const-variable, that finds declarations of non-const variables that never get modified.
JonasToth added inline comments. Comment at: clang-tidy/utils/ASTUtils.cpp:85 + // LHS of any assignment operators. + const auto AsAssignmentLhs = binaryOperator( + anyOf(hasOperatorName("="), hasOperatorName("+="), hasOperatorName("-="), there is a `util::isAssignmentOperator` in clang-tidy already. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45679 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call
dkrupp added a comment. Would be interesting to extend this checker (maybe in an upcoming patch) to report on uninitialized members not only in constructors, but also copy constructors and move constructors. See related https://bugs.llvm.org/show_bug.cgi?id=37086 This bug report also mentions assignment operator. But for that a warning may be not so useful. In that case the members of the assigned to object should have some initialized value already which the programmer may not want to overwrite in the assignment operator. https://reviews.llvm.org/D45532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness
JonasToth added a comment. > Coming from https://reviews.llvm.org/D45679, which I should have sent out > much earlier to get in front of you :p > > Kidding aside this check is more mature than mine and I'm happy to help > incorporate mine into this one. I would love to, yours is more elegant :) > I do have a strong opinion that requires non-trivial refactoring of your diff > though: having a standalone reusable helper function like isModified(Expr, > Stmt), that checks whether Expr is (potentially) modified within Stmt, is > something I believe to be quite useful: Yes please! > performance/{ForRangeCopyCheck, UnnecessaryCopyInitialization} are using a > much less sophisticated isOnlyUsedAsConst(), and can benefit from a more > powerful one. I did not think about such possibilities, but maybe other analysis task could benefit too? Warnings? > I would imagine things could get messier if this check expands to also > check for turning member functions const: it's basically checking > CxxThisExpr, being a handle, is not modified within a member function, but > note there's no VarDecl for "this". Using my approach, you can check if any member variable is used as non-const. Then mark this method as const, if it is not virtual. Similar for member variables: private non-consts can be converted into consts. > It's cleaner to follow (non-const) reference chains and only mark a decl as > "can't be const" if everything on the chain is not modified, avoiding false > negatives when a variable is bound to a never-modified > non-const reference. For warning only yes. But i want automatic code transformation which will break such code if there are non-const references taken. Having it in one pass is certainly nice. With my approach multiple runs are required if the handles are marked as const. > It's also cleaner to follow member access chains, e.g. "v.a.b.c = 10" > modifies "v". This one is particularly tricky because the modifying behavior > happens at "c = 10" while the variable we'd like to mark as "can't be const" > is arbitrary layers away, and MemberExpr alone doesn't warrant marking "can't > be const" because that'll introduce way too many false negatives, ... and I > haven't figure out a way to write a matcher to match > memberExpr(hasObjectExpression(memberExpr(hasObjectExpression(... layers deep> ... (VarDeclRef) ...), not to mention any layer could either > member access or array subscript access. Does your approach work with the nesting? Maybe something recursive or `hasDescendent(modifying)` could do it? Is read your comment on this check later, i first saw your new check. Maybe we should discuss only under one of both :) (which one is not important for me :)) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps
dstenb added a comment. Ping. Repository: rC Clang https://reviews.llvm.org/D44568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45668: [NEON] Define vget_high_f16() and vget_low_f16() intrinsics in AArch64 mode only
SjoerdMeijer added a comment. Not really familiar with these 2 intrinsics, I had a quick look at the ACLE: > T vget_high_ST(T 2 a); > T vget_low_ST(T 2 a); > > Gets the high, or low, half of a 128-bit vector. There are 24 intrinsics. > ARMv8 > adds 4 more intrinsics for 128-bit vectors with float64_t and poly64_t lane > type. I don't read here that they are unavailable in AArch32. Have I missed something? https://reviews.llvm.org/D45668 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45682: [analyzer] Move `TaintBugVisitor` from `GenericTaintChecker.cpp` to `BugReporterVisitors.h`.
MTC created this revision. MTC added reviewers: NoQ, george.karpenkov, xazax.hun. Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet. `TaintBugVisitor` is a universal visitor, and many checkers rely on it, such as `ArrayBoundCheckerV2.cpp`, `DivZeroChecker.cpp` and `VLASizeChecker.cpp`. Moving `TaintBugVisitor` to `BugReporterVisitors.h` enables other checker can also track where `tainted` value came from. Repository: rC Clang https://reviews.llvm.org/D45682 Files: include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp === --- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -2333,3 +2333,24 @@ return std::move(Piece); } + +std::shared_ptr +TaintBugVisitor::VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN, + BugReporterContext &BRC, BugReport &BR) { + + // Find the ExplodedNode where the taint was first introduced + if (!N->getState()->isTainted(V) || PrevN->getState()->isTainted(V)) +return nullptr; + + const Stmt *S = PathDiagnosticLocation::getStmt(N); + if (!S) +return nullptr; + + const LocationContext *NCtx = N->getLocationContext(); + PathDiagnosticLocation L = + PathDiagnosticLocation::createBegin(S, BRC.getSourceManager(), NCtx); + if (!L.isValid() || !L.asLocation().isValid()) +return nullptr; + + return std::make_shared(L, "Taint originated here"); +} Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp === --- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -100,23 +100,6 @@ bool generateReportIfTainted(const Expr *E, const char Msg[], CheckerContext &C) const; - /// The bug visitor prints a diagnostic message at the location where a given - /// variable was tainted. - class TaintBugVisitor - : public BugReporterVisitorImpl { - private: -const SVal V; - - public: -TaintBugVisitor(const SVal V) : V(V) {} -void Profile(llvm::FoldingSetNodeID &ID) const override { ID.Add(V); } - -std::shared_ptr VisitNode(const ExplodedNode *N, - const ExplodedNode *PrevN, - BugReporterContext &BRC, - BugReport &BR) override; - }; - typedef SmallVector ArgVector; /// \brief A struct used to specify taint propagation rules for a function. @@ -214,28 +197,6 @@ /// points to data, which should be tainted on return. REGISTER_SET_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, unsigned) -std::shared_ptr -GenericTaintChecker::TaintBugVisitor::VisitNode(const ExplodedNode *N, -const ExplodedNode *PrevN, BugReporterContext &BRC, BugReport &BR) { - - // Find the ExplodedNode where the taint was first introduced - if (!N->getState()->isTainted(V) || PrevN->getState()->isTainted(V)) -return nullptr; - - const Stmt *S = PathDiagnosticLocation::getStmt(N); - if (!S) -return nullptr; - - const LocationContext *NCtx = N->getLocationContext(); - PathDiagnosticLocation L = - PathDiagnosticLocation::createBegin(S, BRC.getSourceManager(), NCtx); - if (!L.isValid() || !L.asLocation().isValid()) -return nullptr; - - return std::make_shared( - L, "Taint originated here"); -} - GenericTaintChecker::TaintPropagationRule GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( const FunctionDecl *FDecl, Index: include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h === --- include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h +++ include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h @@ -343,6 +343,22 @@ BugReport &BR) override; }; +/// The bug visitor prints a diagnostic message at the location where a given +/// variable was tainted. +class TaintBugVisitor final : public BugReporterVisitorImpl { +private: + const SVal V; + +public: + TaintBugVisitor(const SVal V) : V(V) {} + void Profile(llvm::FoldingSetNodeID &ID) const override { ID.Add(V); } + + std::shared_ptr VisitNode(const ExplodedNode *N, + const ExplodedNode *PrevN, + BugReporterContext &BRC, + BugReport &BR) override; +}; + namespace bugreporter { /// Attempts to add visitors to trace a null or undefined value back to its ___
r330118 - [mips] Prevent PIC to be set to level 2
Author: abeserminji Date: Mon Apr 16 03:21:24 2018 New Revision: 330118 URL: http://llvm.org/viewvc/llvm-project?rev=330118&view=rev Log: [mips] Prevent PIC to be set to level 2 MIPS does not use PIC level 2 for historical reasons, even with -fPIC/-mxgot/multigot options. This patch prevents PIC to be set to level 2 for MIPS. Differential Revision: https://reviews.llvm.org/D44381 Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp cfe/trunk/test/Driver/pic.c Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=330118&r1=330117&r2=330118&view=diff == --- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Mon Apr 16 03:21:24 2018 @@ -1007,12 +1007,17 @@ tools::ParsePICArgs(const ToolChain &Too if ((ROPI || RWPI) && (PIC || PIE)) ToolChain.getDriver().Diag(diag::err_drv_ropi_rwpi_incompatible_with_pic); - // When targettng MIPS64 with N64, the default is PIC, unless -mno-abicalls is - // used. - if ((Triple.getArch() == llvm::Triple::mips64 || - Triple.getArch() == llvm::Triple::mips64el) && - Args.hasArg(options::OPT_mno_abicalls)) -return std::make_tuple(llvm::Reloc::Static, 0U, false); + if (Triple.getArch() == llvm::Triple::mips || + Triple.getArch() == llvm::Triple::mipsel || + Triple.getArch() == llvm::Triple::mips64 || + Triple.getArch() == llvm::Triple::mips64el) { +// When targettng MIPS with -mno-abicalls, it's always static. +if(Args.hasArg(options::OPT_mno_abicalls)) + return std::make_tuple(llvm::Reloc::Static, 0U, false); +// Unlike other architectures, MIPS, even with -fPIC/-mxgot/multigot, +// does not use PIC level 2 for historical reasons. +IsPICLevelTwo = false; + } if (PIC) return std::make_tuple(llvm::Reloc::PIC_, IsPICLevelTwo ? 2U : 1U, PIE); Modified: cfe/trunk/test/Driver/pic.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/pic.c?rev=330118&r1=330117&r2=330118&view=diff == --- cfe/trunk/test/Driver/pic.c (original) +++ cfe/trunk/test/Driver/pic.c Mon Apr 16 03:21:24 2018 @@ -292,9 +292,9 @@ // RUN: %clang -c %s -target mipsel-linux-android14 -### 2>&1 \ // RUN: | FileCheck %s --check-prefix=CHECK-PIC1 // RUN: %clang -c %s -target mipsel-linux-android16 -### 2>&1 \ -// RUN: | FileCheck %s --check-prefix=CHECK-PIE2 +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 // RUN: %clang -c %s -target mipsel-linux-android24 -### 2>&1 \ -// RUN: | FileCheck %s --check-prefix=CHECK-PIE2 +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 // // 64-bit Android targets are always PIE. // RUN: %clang -c %s -target aarch64-linux-android -### 2>&1 \ ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r330110 - [X86] Introduce archs: goldmont-plus & tremont
Author: gbuella Date: Mon Apr 16 01:10:10 2018 New Revision: 330110 URL: http://llvm.org/viewvc/llvm-project?rev=330110&view=rev Log: [X86] Introduce archs: goldmont-plus & tremont Reviewers: craig.topper Reviewed By: craig.topper Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D45613 Modified: cfe/trunk/include/clang/Basic/X86Target.def cfe/trunk/lib/Basic/Targets/X86.cpp cfe/trunk/test/Driver/x86-march.c cfe/trunk/test/Misc/target-invalid-cpu-note.c cfe/trunk/test/Preprocessor/predefined-arch-macros.c Modified: cfe/trunk/include/clang/Basic/X86Target.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/X86Target.def?rev=330110&r1=330109&r2=330110&view=diff == --- cfe/trunk/include/clang/Basic/X86Target.def (original) +++ cfe/trunk/include/clang/Basic/X86Target.def Mon Apr 16 01:10:10 2018 @@ -104,6 +104,9 @@ PROC_WITH_FEAT(Silvermont, "silvermont", PROC_ALIAS(Silvermont, "slm") PROC(Goldmont, "goldmont", PROC_64_BIT) +PROC(GoldmontPlus, "goldmont-plus", PROC_64_BIT) + +PROC(Tremont, "tremont", PROC_64_BIT) //@} /// \name Nehalem Modified: cfe/trunk/lib/Basic/Targets/X86.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=330110&r1=330109&r2=330110&view=diff == --- cfe/trunk/lib/Basic/Targets/X86.cpp (original) +++ cfe/trunk/lib/Basic/Targets/X86.cpp Mon Apr 16 01:10:10 2018 @@ -244,6 +244,14 @@ bool X86TargetInfo::initFeatureMap( setFeatureEnabledImpl(Features, "fxsr", true); break; + case CK_Tremont: +setFeatureEnabledImpl(Features, "cldemote", true); +setFeatureEnabledImpl(Features, "gfni", true); +LLVM_FALLTHROUGH; + case CK_GoldmontPlus: +setFeatureEnabledImpl(Features, "rdpid", true); +setFeatureEnabledImpl(Features, "sgx", true); +LLVM_FALLTHROUGH; case CK_Goldmont: setFeatureEnabledImpl(Features, "sha", true); setFeatureEnabledImpl(Features, "rdseed", true); @@ -930,6 +938,12 @@ void X86TargetInfo::getTargetDefines(con case CK_Goldmont: defineCPUMacros(Builder, "goldmont"); break; + case CK_GoldmontPlus: +defineCPUMacros(Builder, "goldmont_plus"); +break; + case CK_Tremont: +defineCPUMacros(Builder, "tremont"); +break; case CK_Nehalem: case CK_Westmere: case CK_SandyBridge: Modified: cfe/trunk/test/Driver/x86-march.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/x86-march.c?rev=330110&r1=330109&r2=330110&view=diff == --- cfe/trunk/test/Driver/x86-march.c (original) +++ cfe/trunk/test/Driver/x86-march.c Mon Apr 16 01:10:10 2018 @@ -84,6 +84,14 @@ // RUN: | FileCheck %s -check-prefix=goldmont // goldmont: "-target-cpu" "goldmont" // +// RUN: %clang -target x86_64-unknown-unknown -c -### %s -march=goldmont-plus 2>&1 \ +// RUN: | FileCheck %s -check-prefix=goldmont-plus +// goldmont-plus: "-target-cpu" "goldmont-plus" +// +// RUN: %clang -target x86_64-unknown-unknown -c -### %s -march=tremont 2>&1 \ +// RUN: | FileCheck %s -check-prefix=tremont +// tremont: "-target-cpu" "tremont" +// // RUN: %clang -target x86_64-unknown-unknown -c -### %s -march=k8 2>&1 \ // RUN: | FileCheck %s -check-prefix=k8 // k8: "-target-cpu" "k8" Modified: cfe/trunk/test/Misc/target-invalid-cpu-note.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/target-invalid-cpu-note.c?rev=330110&r1=330109&r2=330110&view=diff == --- cfe/trunk/test/Misc/target-invalid-cpu-note.c (original) +++ cfe/trunk/test/Misc/target-invalid-cpu-note.c Mon Apr 16 01:10:10 2018 @@ -13,7 +13,7 @@ // X86: note: valid target CPU values are: i386, i486, winchip-c6, winchip2, c3, // X86-SAME: i586, pentium, pentium-mmx, pentiumpro, i686, pentium2, pentium3, // X86-SAME: pentium3m, pentium-m, c3-2, yonah, pentium4, pentium4m, prescott, -// X86-SAME: nocona, core2, penryn, bonnell, atom, silvermont, slm, goldmont, +// X86-SAME: nocona, core2, penryn, bonnell, atom, silvermont, slm, goldmont, goldmont-plus, tremont, // X86-SAME: nehalem, corei7, westmere, sandybridge, corei7-avx, ivybridge, // X86-SAME: core-avx-i, haswell, core-avx2, broadwell, skylake, skylake-avx512, // X86-SAME: skx, cannonlake, icelake-client, icelake-server, knl, knm, lakemont, k6, k6-2, k6-3, @@ -25,7 +25,7 @@ // RUN: not %clang_cc1 -triple x86_64--- -target-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix X86_64 // X86_64: error: unknown target CPU 'not-a-cpu' // X86_64: note: valid target CPU values are: nocona, core2, penryn, bonnell, -// X86_64-SAME: atom, silvermont, slm, goldmont, nehalem, corei7, westmere, +// X86_64-SAME: atom, silvermont, slm, goldmont, goldmont-plus, tremont, nehalem, corei7, w
[PATCH] D45669: [NEON] Fix the architecture condition for the crypto intrinsics
SjoerdMeijer accepted this revision. SjoerdMeijer added a comment. This revision is now accepted and ready to land. This looks correct to me, the ACLE indeed says that: > This is only available when __ARM_ARCH >= 8 Thanks for fixing this. https://reviews.llvm.org/D45669 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45569: [Sema] Disable built-in increment operator for bool in overload resolution in C++17
rsmith added inline comments. Comment at: SemaCXX/overloaded-builtin-operators-cxx17.cpp:8 +void foo(BoolRef br) { + // C++ [over.built]p3: Increment for bool got deprecated in C++17. + bool b = br++; // expected-error{{cannot increment value of type 'BoolRef'}} Removed, not deprecated. Repository: rC Clang https://reviews.llvm.org/D45569 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45670: [NEON} Define vfma_n_f32() and vfmaq_n_f32() intrinsics in AArch32 mode
SjoerdMeijer added inline comments. Comment at: test/CodeGen/arm-neon-fma.c:27 +// CHECK: [[VECINIT1_I:%.*]] = insertelement <2 x float> [[VECINIT_I]], float %n, i32 1 +// CHECK: [[TMP0:%.*]] = bitcast <2 x float> %a to <8 x i8> +// CHECK: [[TMP1:%.*]] = bitcast <2 x float> %b to <8 x i8> If these bitcasts are not used, we also don't need to check for them; that looks to be inline with the other tests here. https://reviews.llvm.org/D45670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt
EricWF planned changes to this revision. EricWF added a comment. I know how to do better. Comment at: include/clang/Sema/Overload.h:938 +/// exited. +struct RewrittenCandidateContextGuard { + RewrittenCandidateContextGuard(OverloadCandidateSet &CS) This is unneeded. As long as the rewritten candidates are added last, restoring the `Functions` member is unneeded. Repository: rC Clang https://reviews.llvm.org/D45680 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r329780 - [Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option)
Hi Adam and Artem, If I run clang-tidy -header-filter=.\* -checks=clang-analyzer-\*,-clang-analyzer-unix\*,-clang-analyzer-osx\*,-clang-analyzer-security\*,-clang-analyzer-valist\*,-clang-analyzer-optin\*,-clang-analyzer-nullability\*,-clang-analyzer-llvm.Conventions,-clang-analyzer-deadcode.DeadStores,-clang-analyzer-cplusplus\*,-clang-analyzer-apiModeling\* test.c -- on the attached test.c it takes seemingly forever, but if I peel off a number of calls to "f" it starts terminating. It seems like every additional level of calls to "f" doubles the compilation time with your commit. Without your commit even the full repriducer terminates quickly. Regards, Mikael On 04/11/2018 08:21 AM, Adam Balogh via cfe-commits wrote: Author: baloghadamsoftware Date: Tue Apr 10 23:21:12 2018 New Revision: 329780 URL: http://llvm.org/viewvc/llvm-project?rev=329780&view=rev Log: [Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option) Since the range-based constraint manager (default) is weak in handling comparisons where symbols are on both sides it is wise to rearrange them to have symbols only on the left side. Thus e.g. A + n >= B + m becomes A - B >= m - n which enables the constraint manager to store a range m - n .. MAX_VALUE for the symbolic expression A - B. This can be used later to check whether e.g. A + k == B + l can be true, which is also rearranged to A - B == l - k so the constraint manager can check whether l - k is in the range (thus greater than or equal to m - n). The restriction in this version is the the rearrangement happens only if both the symbols and the concrete integers are within the range [min/4 .. max/4] where min and max are the minimal and maximal values of their type. The rearrangement is not enabled by default. It has to be enabled by using -analyzer-config aggressive-relational-comparison-simplification=true. Co-author of this patch is Artem Dergachev (NoQ). Differential Revision: https://reviews.llvm.org/D41938 Added: cfe/trunk/test/Analysis/svalbuilder-rearrange-comparisons.c Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp cfe/trunk/test/Analysis/conditional-path-notes.c cfe/trunk/test/Analysis/explain-svals.cpp Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=329780&r1=329779&r2=329780&view=diff == --- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Tue Apr 10 23:21:12 2018 @@ -312,6 +312,9 @@ private: /// \sa shouldDisplayNotesAsEvents Optional DisplayNotesAsEvents; + /// \sa shouldAggressivelySimplifyRelationalComparison + Optional AggressiveRelationalComparisonSimplification; + /// \sa getCTUDir Optional CTUDir; @@ -666,6 +669,20 @@ public: /// to false when unset. bool shouldDisplayNotesAsEvents(); + /// Returns true if SValBuilder should rearrange comparisons of symbolic + /// expressions which consist of a sum of a symbol and a concrete integer + /// into the format where symbols are on the left-hand side and the integer + /// is on the right. This is only done if both symbols and both concrete + /// integers are signed, greater than or equal to the quarter of the minimum + /// value of the type and less than or equal to the quarter of the maximum + /// value of that type. + /// + /// A + n B + m becomes A - B m - n, where A and B symbolic, + /// n and m are integers. is any of '==', '!=', '<', '<=', '>' or '>='. + /// The rearrangement also happens with '-' instead of '+' on either or both + /// side and also if any or both integers are missing. + bool shouldAggressivelySimplifyRelationalComparison(); + /// Returns the directory containing the CTU related files. StringRef getCTUDir(); Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=329780&r1=329779&r2=329780&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp Tue Apr 10 23:21:12 2018 @@ -445,6 +445,14 @@ bool AnalyzerOptions::shouldDisplayNotes return DisplayNotesAsEvents.getValue(); } +bool AnalyzerOptions::shouldAggressivelySimplifyRelationalComparison() { + if (!AggressiveRelationalComparisonSimplification.hasValue()) +AggressiveRelationalComparisonSimplification = + getBooleanOption("aggressive-relational-comparison-simplification", +
[PATCH] D45383: Limit types of builtins that can be redeclared.
erichkeane added a comment. In https://reviews.llvm.org/D45383#1068085, @compnerd wrote: > Snipping bits from `va_defs.h`: > > > #elif defined _M_ARM64 > > void __cdecl __va_start(va_list*, ...); > > #define __crt_va_start_a(ap,v) ((void)(__va_start(&ap, _ADDRESSOF(v), > _SLOTSIZEOF(v), __alignof(v), _ADDRESSOF(v > ... > > #elif defined _M_X64 > > void __cdecl __va_start(va_list* , ...); > > #define __crt_va_start_a(ap, x) ((void)(__va_start(&ap, x))) > > ... > > > This looks like a declaration to me. Although, this is in system headers, so > maybe we can ignore it by means of the system header suppression? The minor > problem with that is that clang-cl (and the clang driver with the windows > triple) do not support `-isystem` or `-isysroot` or `--sysroot` arguments. I > suppose that as long as we expose the cc1 option (I imagine that clang-cl > will pass the system paths appropriately), that is one option. Ah, thank you! I couldn't find that when I looked after your last message, perhaps my grep skills are lacking. Unfortunately, allowing this in headers won't fix the actual bug, it'll just let it continue to crash on these headers. I'm kind of out of ideas here... @efriedma : Ideas? https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call
Szelethus updated this revision to Diff 142624. Szelethus added a comment. > Would be interesting to extend this checker (maybe in an upcoming patch) to > report on uninitialized members not only in constructors, but also copy > constructors and move constructors. Added 3 new test cases. to cover them. Interestingly, move constructors don't emit any warnings - the core can only assert that the fields after a move construction are valid (returns `true` for Val::isValid()`). Came to think of it, I'm not 100% confident in the checkers name. It could be misleading, as this checker doesn't check constructors, but rather objects after construction. The end of a constructor call is only the point at which we know that analysis can be done. > This bug report also mentions assignment operator. But for that a warning may > be not so useful. In that case the members of the assigned to object should > have some initialized value already which the programmer may not want to > overwrite in the assignment operator. I believe there's a checker for that already, but I'm really not sure whether `UndefinedAssignmentChecker` covers all such cases. Also, I managed to cause a crash with the class `linked_ptr_internal` from google's boringssl when I analyzed the grpc project. I'll look deeper into this, but I have a strong suspicion that the error lies within the CSA core. https://reviews.llvm.org/D45532 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp test/Analysis/ctor-uninitialized-member-inheritance.cpp test/Analysis/ctor-uninitialized-member.cpp Index: test/Analysis/ctor-uninitialized-member.cpp === --- /dev/null +++ test/Analysis/ctor-uninitialized-member.cpp @@ -0,0 +1,960 @@ +//RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.CtorUninitializedMember -std=c++11 -verify %s + +//===--===// +// Default constructor test. +//===--===// + +class CompilerGeneratedConstructorTest { + int a, b, c, d, e, f, g, h, i, j; + +public: + CompilerGeneratedConstructorTest() = default; +}; + +void f000() { + CompilerGeneratedConstructorTest(); +} + +class DefaultConstructorTest { + int a; // expected-note{{uninitialized field 'this->a'}} + +public: + DefaultConstructorTest(); +}; + +DefaultConstructorTest::DefaultConstructorTest() = default; + +void f00() { + DefaultConstructorTest(); // expected-warning{{1 uninitialized field}} +} + +//===--===// +// Initializer list test. +//===--===// + +class InitListTest1 { + int a; + int b; + +public: + InitListTest1() + : a(1), +b(2) { +// All good! + } +}; + +void f01() { + InitListTest1(); +} + +class InitListTest2 { + int a; + int b; // expected-note{{uninitialized field 'this->b'}} + +public: + InitListTest2() + : a(3) {} // expected-warning{{1 uninitialized field}} +}; + +void f02() { + InitListTest2(); +} + +class InitListTest3 { + int a; // expected-note{{uninitialized field 'this->a'}} + int b; + +public: + InitListTest3() + : b(4) {} // expected-warning{{1 uninitialized field}} +}; + +void f03() { + InitListTest3(); +} + +//===--===// +// Constructor body test. +//===--===// + +class CtorBodyTest1 { + int a, b; + +public: + CtorBodyTest1() { +a = 5; +b = 6; +// All good! + } +}; + +void f04() { + CtorBodyTest1(); +} + +class CtorBodyTest2 { + int a; + int b; // expected-note{{uninitialized field 'this->b'}} + +public: + CtorBodyTest2() { +a = 7; // expected-warning{{1 uninitialized field}} + } +}; + +void f05() { + CtorBodyTest2(); +} + +class CtorBodyTest3 { + int a; // expected-note{{uninitialized field 'this->a'}} + int b; + +public: + CtorBodyTest3() { +b = 8; // expected-warning{{1 uninitialized field}} + } +}; + +void f06() { + CtorBodyTest3(); +} + +class CtorBodyTest4 { + int a; // expected-note{{uninitialized field 'this->a'}} + int b; // expected-note{{uninitialized field 'this->b'}} + +public: + CtorBodyTest4() {} +}; + +void f07() { + CtorBodyTest4(); // expected-warning{{2 uninitialized fields}} +} + +//===--===// +// Constructor delegation test. +//===--===// + +class CtorDelegationTest1 { + int a; + int b; + +public: + CtorDelegationTest1(int) + : a(9) { +// leaves 'b' unintialized, but we'll never check this function + } + + C
[PATCH] D45685: [Sema] Add -Wtest global flag that silences -Wself-assign for overloaded operators.
lebedev.ri created this revision. lebedev.ri added reviewers: rjmccall, aaron.ballman, dblaikie, thakis, rsmith, chandlerc, brooksmoses. Herald added a subscriber: klimek. Credit for the idea goes to @rjmccall. I'm not quite sure how to do it with `-wtest` (lowercase). so i just modelled it after `-Wsystem-headers`. As tests show, only the overloaded non-field assign is silenced by it. Testing: `$ ninja check-clang-sema check-clang-semacxx check-clang` Repository: rC Clang https://reviews.llvm.org/D45685 Files: docs/ReleaseNotes.rst docs/UsersManual.rst include/clang/AST/ASTDiagnostic.h include/clang/AST/CommentDiagnostic.h include/clang/Basic/Diagnostic.h include/clang/Basic/Diagnostic.td include/clang/Basic/DiagnosticIDs.h include/clang/Basic/DiagnosticSemaKinds.td include/clang/CrossTU/CrossTUDiagnostic.h include/clang/Driver/DriverDiagnostic.h include/clang/Frontend/FrontendDiagnostic.h include/clang/Lex/LexDiagnostic.h include/clang/Parse/ParseDiagnostic.h include/clang/Sema/SemaDiagnostic.h include/clang/Serialization/SerializationDiagnostic.h include/clang/Tooling/Refactoring/RefactoringDiagnostic.h lib/Basic/DiagnosticIDs.cpp lib/Basic/Warnings.cpp lib/Sema/SemaExpr.cpp test/Sema/warn-self-assign-builtin-warn-tests.c test/SemaCXX/warn-self-assign-builtin-warn-tests.cpp test/SemaCXX/warn-self-assign-field-builtin-warn-tests.cpp test/SemaCXX/warn-self-assign-field-overloaded-warn-tests.cpp test/SemaCXX/warn-self-assign-overloaded-warn-tests.cpp tools/diagtool/DiagnosticNames.cpp utils/TableGen/ClangDiagnosticsEmitter.cpp Index: utils/TableGen/ClangDiagnosticsEmitter.cpp === --- utils/TableGen/ClangDiagnosticsEmitter.cpp +++ utils/TableGen/ClangDiagnosticsEmitter.cpp @@ -552,6 +552,11 @@ else OS << ", false"; +if (R.getValueAsBit("HideInTests")) + OS << ", true"; +else + OS << ", false"; + // Category number. OS << ", " << CategoryIDs.getID(getDiagnosticCategory(&R, DGParentMap)); OS << ")\n"; Index: tools/diagtool/DiagnosticNames.cpp === --- tools/diagtool/DiagnosticNames.cpp +++ tools/diagtool/DiagnosticNames.cpp @@ -28,9 +28,9 @@ // FIXME: Is it worth having two tables, especially when this one can get // out of sync easily? static const DiagnosticRecord BuiltinDiagnosticsByID[] = { -#define DIAG(ENUM,CLASS,DEFAULT_MAPPING,DESC,GROUP, \ - SFINAE,NOWERROR,SHOWINSYSHEADER,CATEGORY)\ - { #ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t) }, +#define DIAG(ENUM, CLASS, DEFAULT_MAPPING, DESC, GROUP, SFINAE, NOWERROR, \ + SHOWINSYSHEADER, HIDEINTESTS, CATEGORY) \ + {#ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t)}, #include "clang/Basic/DiagnosticCommonKinds.inc" #include "clang/Basic/DiagnosticCrossTUKinds.inc" #include "clang/Basic/DiagnosticDriverKinds.inc" Index: test/SemaCXX/warn-self-assign-overloaded-warn-tests.cpp === --- /dev/null +++ test/SemaCXX/warn-self-assign-overloaded-warn-tests.cpp @@ -0,0 +1,15 @@ +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-tests -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wtests -verify -DSILENCE %s + +struct S {}; + +void f() { + S a; +#ifndef SILENCE + a = a; // expected-warning{{explicitly assigning}} +#else + // expected-no-diagnostics + a = a; +#endif +} Index: test/SemaCXX/warn-self-assign-field-overloaded-warn-tests.cpp === --- /dev/null +++ test/SemaCXX/warn-self-assign-field-overloaded-warn-tests.cpp @@ -0,0 +1,18 @@ +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wno-tests -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wtests -verify %s + +struct S {}; + +struct C { + S a; + + void f() { +#ifndef SILENCE +a = a; // expected-warning{{assigning field to itself}} +#else +// expected-no-diagnostics +a = a; +#endif + } +}; Index: test/SemaCXX/warn-self-assign-field-builtin-warn-tests.cpp === --- /dev/null +++ test/SemaCXX/warn-self-assign-field-builtin-warn-tests.cpp @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wno-tests -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wtests -verify %s + +struct C { + int a; + + void f() { +#ifndef SILENCE +a = a; // expected-warning{{assigning field to itself}} +#else +// expected-no-diagnostics +a = a; +#endif + } +}; Index: test/SemaCXX/warn-self-assign-builtin-warn-tests.cpp =
[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)
lebedev.ri added a comment. In https://reviews.llvm.org/D44883#1068602, @lebedev.ri wrote: > There are several options: > > 1. @rjmccall's idea: `-wtest` (lowercase), which in this case will disable > that new code in `BuildOverloadedBinOp()`. i quite like it actually. > 2. split it up like i had in the first revision - ``-Wself-assign-builtin``, > ``-Wself-assign-field-builtin``; ``-Wself-assign-overloaded``, > ``-Wself-assign-field-overloaded`` > - we could just assume that `BuildOverloadedBinOp()` implies overloaded, > - or check that the particular operator is non-trivial > 3. ??? > > @rjmccall, @thakis, @dblaikie, @aaron.ballman, @brooksmoses, @chandlerc > > I'm going to go ahead and look into `1.`, since it does not seem there will > be any consensus in a timely manner. ... and here it is https://reviews.llvm.org/D45685, please take a look Repository: rC Clang https://reviews.llvm.org/D44883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45685: [Sema] Add -Wtest global flag that silences -Wself-assign for overloaded operators.
lebedev.ri updated this revision to Diff 142627. lebedev.ri added a comment. - Don't mis-spell the name of the flag. FIXME: So do we want it to be `-Wtests`, `-Wtest`, or we really want it to be `-wtest` ? Repository: rC Clang https://reviews.llvm.org/D45685 Files: docs/ReleaseNotes.rst docs/UsersManual.rst include/clang/AST/ASTDiagnostic.h include/clang/AST/CommentDiagnostic.h include/clang/Basic/Diagnostic.h include/clang/Basic/Diagnostic.td include/clang/Basic/DiagnosticIDs.h include/clang/Basic/DiagnosticSemaKinds.td include/clang/CrossTU/CrossTUDiagnostic.h include/clang/Driver/DriverDiagnostic.h include/clang/Frontend/FrontendDiagnostic.h include/clang/Lex/LexDiagnostic.h include/clang/Parse/ParseDiagnostic.h include/clang/Sema/SemaDiagnostic.h include/clang/Serialization/SerializationDiagnostic.h include/clang/Tooling/Refactoring/RefactoringDiagnostic.h lib/Basic/DiagnosticIDs.cpp lib/Basic/Warnings.cpp lib/Sema/SemaExpr.cpp test/Sema/warn-self-assign-builtin-warn-tests.c test/SemaCXX/warn-self-assign-builtin-warn-tests.cpp test/SemaCXX/warn-self-assign-field-builtin-warn-tests.cpp test/SemaCXX/warn-self-assign-field-overloaded-warn-tests.cpp test/SemaCXX/warn-self-assign-overloaded-warn-tests.cpp tools/diagtool/DiagnosticNames.cpp utils/TableGen/ClangDiagnosticsEmitter.cpp Index: utils/TableGen/ClangDiagnosticsEmitter.cpp === --- utils/TableGen/ClangDiagnosticsEmitter.cpp +++ utils/TableGen/ClangDiagnosticsEmitter.cpp @@ -552,6 +552,11 @@ else OS << ", false"; +if (R.getValueAsBit("HideInTests")) + OS << ", true"; +else + OS << ", false"; + // Category number. OS << ", " << CategoryIDs.getID(getDiagnosticCategory(&R, DGParentMap)); OS << ")\n"; Index: tools/diagtool/DiagnosticNames.cpp === --- tools/diagtool/DiagnosticNames.cpp +++ tools/diagtool/DiagnosticNames.cpp @@ -28,9 +28,9 @@ // FIXME: Is it worth having two tables, especially when this one can get // out of sync easily? static const DiagnosticRecord BuiltinDiagnosticsByID[] = { -#define DIAG(ENUM,CLASS,DEFAULT_MAPPING,DESC,GROUP, \ - SFINAE,NOWERROR,SHOWINSYSHEADER,CATEGORY)\ - { #ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t) }, +#define DIAG(ENUM, CLASS, DEFAULT_MAPPING, DESC, GROUP, SFINAE, NOWERROR, \ + SHOWINSYSHEADER, HIDEINTESTS, CATEGORY) \ + {#ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t)}, #include "clang/Basic/DiagnosticCommonKinds.inc" #include "clang/Basic/DiagnosticCrossTUKinds.inc" #include "clang/Basic/DiagnosticDriverKinds.inc" Index: test/SemaCXX/warn-self-assign-overloaded-warn-tests.cpp === --- /dev/null +++ test/SemaCXX/warn-self-assign-overloaded-warn-tests.cpp @@ -0,0 +1,15 @@ +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-tests -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wtests -verify -DSILENCE %s + +struct S {}; + +void f() { + S a; +#ifndef SILENCE + a = a; // expected-warning{{explicitly assigning}} +#else + // expected-no-diagnostics + a = a; +#endif +} Index: test/SemaCXX/warn-self-assign-field-overloaded-warn-tests.cpp === --- /dev/null +++ test/SemaCXX/warn-self-assign-field-overloaded-warn-tests.cpp @@ -0,0 +1,18 @@ +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wno-tests -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wtests -verify %s + +struct S {}; + +struct C { + S a; + + void f() { +#ifndef SILENCE +a = a; // expected-warning{{assigning field to itself}} +#else +// expected-no-diagnostics +a = a; +#endif + } +}; Index: test/SemaCXX/warn-self-assign-field-builtin-warn-tests.cpp === --- /dev/null +++ test/SemaCXX/warn-self-assign-field-builtin-warn-tests.cpp @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wno-tests -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -Wtests -verify %s + +struct C { + int a; + + void f() { +#ifndef SILENCE +a = a; // expected-warning{{assigning field to itself}} +#else +// expected-no-diagnostics +a = a; +#endif + } +}; Index: test/SemaCXX/warn-self-assign-builtin-warn-tests.cpp === --- /dev/null +++ test/SemaCXX/warn-self-assign-builtin-warn-tests.cpp @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-tests -verify %s +// RUN:
[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output
whisperity added a comment. @NoQ The problem with emitting notes as events is that we lose the information that the node was a `node`. How does Xcode behave with these notes? Does it ignore them, or can read them from the command-line output of the analyser? Repository: rC Clang https://reviews.llvm.org/D45407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45686: [Tooling] Clean up tmp files when creating a fixed compilation database
dstenb created this revision. dstenb added reviewers: klimek, sepavloff, arphaman. Herald added a subscriber: cfe-commits. In https://reviews.llvm.org/rL327851 the createUniqueFile() and createTemporaryFile() variants that do not return the file descriptors were changed to create empty files, rather than only check if the paths are free. This change was done in order to make the functions race-free. That change led to clang-tidy (and possibly other tools) leaving behind temporary assembly files, of the form placeholder-*, when using a target that does not support the internal assembler. The temporary files are created when building the Compilation object in stripPositionalArgs(), as a part of creating the compilation database for the arguments after the double-dash. The files are created by Driver::GetNamedOutputPath(). Fix this issue by cleaning out the Compilation object's temporary file list before the object is deleted at end-of-scope in stripPositionalArgs(). I am not very well-versed in Driver/Tooling, so I don't know if this should be seen as a proper fix, or as a temporary workaround. I at least assume that we want to use a race-free variant rather than getPotentiallyUniqueTempFileName() in Driver::GetNamedOutputPath(). This fixes https://bugs.llvm.org/show_bug.cgi?id=37091. Repository: rC Clang https://reviews.llvm.org/D45686 Files: lib/Tooling/CompilationDatabase.cpp Index: lib/Tooling/CompilationDatabase.cpp === --- lib/Tooling/CompilationDatabase.cpp +++ lib/Tooling/CompilationDatabase.cpp @@ -299,6 +299,9 @@ } } + // Remove temp files. + Compilation->CleanupFileList(Compilation->getTempFiles()); + if (CompileAnalyzer.Inputs.empty()) { ErrorMsg = "warning: no compile jobs found\n"; return false; Index: lib/Tooling/CompilationDatabase.cpp === --- lib/Tooling/CompilationDatabase.cpp +++ lib/Tooling/CompilationDatabase.cpp @@ -299,6 +299,9 @@ } } + // Remove temp files. + Compilation->CleanupFileList(Compilation->getTempFiles()); + if (CompileAnalyzer.Inputs.empty()) { ErrorMsg = "warning: no compile jobs found\n"; return false; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45045: [DebugInfo] Generate debug information for labels.
HsiangKai marked 2 inline comments as done. HsiangKai added a comment. Always add labels to DISubprogram even unreachable. Repository: rC Clang https://reviews.llvm.org/D45045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.
lebedev.ri updated this revision to Diff 142636. lebedev.ri retitled this revision from "[Sema] Add -Wtest global flag that silences -Wself-assign for overloaded operators." to "[Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.". lebedev.ri edited the summary of this revision. lebedev.ri added a comment. Actually make it `-wtest` (all lowercase). Repository: rC Clang https://reviews.llvm.org/D45685 Files: docs/ReleaseNotes.rst docs/UsersManual.rst include/clang/AST/ASTDiagnostic.h include/clang/AST/CommentDiagnostic.h include/clang/Basic/Diagnostic.h include/clang/Basic/Diagnostic.td include/clang/Basic/DiagnosticIDs.h include/clang/Basic/DiagnosticOptions.def include/clang/Basic/DiagnosticSemaKinds.td include/clang/CrossTU/CrossTUDiagnostic.h include/clang/Driver/DriverDiagnostic.h include/clang/Driver/Options.td include/clang/Frontend/FrontendDiagnostic.h include/clang/Lex/LexDiagnostic.h include/clang/Parse/ParseDiagnostic.h include/clang/Sema/SemaDiagnostic.h include/clang/Serialization/SerializationDiagnostic.h include/clang/Tooling/Refactoring/RefactoringDiagnostic.h lib/Basic/DiagnosticIDs.cpp lib/Basic/Warnings.cpp lib/Driver/Driver.cpp lib/Frontend/CompilerInvocation.cpp lib/Sema/SemaExpr.cpp test/Sema/warn-self-assign-builtin-warn-test.c test/SemaCXX/warn-self-assign-builtin-warn-test.cpp test/SemaCXX/warn-self-assign-field-builtin-warn-test.cpp test/SemaCXX/warn-self-assign-field-overloaded-warn-test.cpp test/SemaCXX/warn-self-assign-overloaded-warn-test.cpp tools/diagtool/DiagnosticNames.cpp utils/TableGen/ClangDiagnosticsEmitter.cpp Index: utils/TableGen/ClangDiagnosticsEmitter.cpp === --- utils/TableGen/ClangDiagnosticsEmitter.cpp +++ utils/TableGen/ClangDiagnosticsEmitter.cpp @@ -552,6 +552,11 @@ else OS << ", false"; +if (R.getValueAsBit("HideInTests")) + OS << ", true"; +else + OS << ", false"; + // Category number. OS << ", " << CategoryIDs.getID(getDiagnosticCategory(&R, DGParentMap)); OS << ")\n"; Index: tools/diagtool/DiagnosticNames.cpp === --- tools/diagtool/DiagnosticNames.cpp +++ tools/diagtool/DiagnosticNames.cpp @@ -28,9 +28,9 @@ // FIXME: Is it worth having two tables, especially when this one can get // out of sync easily? static const DiagnosticRecord BuiltinDiagnosticsByID[] = { -#define DIAG(ENUM,CLASS,DEFAULT_MAPPING,DESC,GROUP, \ - SFINAE,NOWERROR,SHOWINSYSHEADER,CATEGORY)\ - { #ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t) }, +#define DIAG(ENUM, CLASS, DEFAULT_MAPPING, DESC, GROUP, SFINAE, NOWERROR, \ + SHOWINSYSHEADER, HIDEINTESTS, CATEGORY) \ + {#ENUM, diag::ENUM, STR_SIZE(#ENUM, uint8_t)}, #include "clang/Basic/DiagnosticCommonKinds.inc" #include "clang/Basic/DiagnosticCrossTUKinds.inc" #include "clang/Basic/DiagnosticDriverKinds.inc" Index: test/SemaCXX/warn-self-assign-overloaded-warn-test.cpp === --- /dev/null +++ test/SemaCXX/warn-self-assign-overloaded-warn-test.cpp @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -wtest -verify -DSILENCE %s + +struct S {}; + +void f() { + S a; +#ifndef SILENCE + a = a; // expected-warning{{explicitly assigning}} +#else + // expected-no-diagnostics + a = a; +#endif +} Index: test/SemaCXX/warn-self-assign-field-overloaded-warn-test.cpp === --- /dev/null +++ test/SemaCXX/warn-self-assign-field-overloaded-warn-test.cpp @@ -0,0 +1,17 @@ +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -wtest -verify %s + +struct S {}; + +struct C { + S a; + + void f() { +#ifndef SILENCE +a = a; // expected-warning{{assigning field to itself}} +#else +// expected-no-diagnostics +a = a; +#endif + } +}; Index: test/SemaCXX/warn-self-assign-field-builtin-warn-test.cpp === --- /dev/null +++ test/SemaCXX/warn-self-assign-field-builtin-warn-test.cpp @@ -0,0 +1,15 @@ +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -wtest -verify %s + +struct C { + int a; + + void f() { +#ifndef SILENCE +a = a; // expected-warning{{assigning field to itself}} +#else +// expected-no-diagnostics +a = a; +#endif + } +}; Index: test/SemaCXX/warn-self-assign-builtin-warn-test.cpp === --- /dev/null +++ test/SemaCXX/warn-self-assign-builtin-warn-test.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -verify %s
[PATCH] D44888: [RISCV] Default enable linker relaxation and add -mrelax, -mno-relax flags
asb requested changes to this revision. asb added a comment. This revision now requires changes to proceed. Thanks Kito. -mrelax and -mno-relax currently only affect the backend. For completeness, I think this patch needs to pass the appropriate flag to the linker depending on relax/no-relax. Repository: rL LLVM https://reviews.llvm.org/D44888 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45692: [clangd] Fix "fail to create file URI" warnings in FileIndexTest.
hokein created this revision. hokein added a reviewer: ioeric. Herald added subscribers: MaskRay, jkorous-apple, ilya-biryukov, klimek. When running the FileIndexTest, it shows "Failed to create an URI for file XXX: not a valid absolute file path" warnings, and the generated Symbols is missing Location information. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45692 Files: unittests/clangd/FileIndexTests.cpp Index: unittests/clangd/FileIndexTests.cpp === --- unittests/clangd/FileIndexTests.cpp +++ unittests/clangd/FileIndexTests.cpp @@ -94,8 +94,8 @@ "BasePath must be a base file path without extension."); llvm::IntrusiveRefCntPtr VFS( new vfs::InMemoryFileSystem); - std::string Path = (BasePath + ".cpp").str(); - std::string Header = (BasePath + ".h").str(); + std::string Path = testPath((BasePath + ".cpp").str()); + std::string Header = testPath((BasePath + ".h").str()); VFS->addFile(Path, 0, llvm::MemoryBuffer::getMemBuffer("")); VFS->addFile(Header, 0, llvm::MemoryBuffer::getMemBuffer(Code)); const char *Args[] = {"clang", "-xc++", "-include", Header.c_str(), Index: unittests/clangd/FileIndexTests.cpp === --- unittests/clangd/FileIndexTests.cpp +++ unittests/clangd/FileIndexTests.cpp @@ -94,8 +94,8 @@ "BasePath must be a base file path without extension."); llvm::IntrusiveRefCntPtr VFS( new vfs::InMemoryFileSystem); - std::string Path = (BasePath + ".cpp").str(); - std::string Header = (BasePath + ".h").str(); + std::string Path = testPath((BasePath + ".cpp").str()); + std::string Header = testPath((BasePath + ".h").str()); VFS->addFile(Path, 0, llvm::MemoryBuffer::getMemBuffer("")); VFS->addFile(Header, 0, llvm::MemoryBuffer::getMemBuffer(Code)); const char *Args[] = {"clang", "-xc++", "-include", Header.c_str(), ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45643: [Failing one test] Reword [-Wreturn-type] messages to "non-void x does not return a value"
easyaspi314 added inline comments. Comment at: docs/DiagnosticsReference.rst:9097 | |++| | +---+--+-+ rsmith wrote: > This is a generated file; please don't manually update it. (Though we should > regenerate it, it's probably quite stale...) Serious question though, why in the world do we have generated files in an open source repo? Isn't is as stupid as putting the precompiled binaries in the source tree? I'd rather wait a few minutes to generate some files than spend 4 hours trying to figure out how to deal with serialized-diags-stable.dia. Repository: rC Clang https://reviews.llvm.org/D45643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt
rsmith added inline comments. Comment at: lib/Sema/SemaOverload.cpp:839 + +unsigned OverloadCandidate::getTrueArgIndex(unsigned Idx) const { + if (getRewrittenKind() != ROC_Synthesized) I think this might be clearer if named `getParamIndexForArgIndex` or similar. Comment at: lib/Sema/SemaOverload.cpp:8880-8882 +/// \brief Add the rewritten and synthesized candidates for binary comparison +//operators. No additional semantic checking is done to see if the candidate +//is well formed. No `\brief` here (we use autobrief), and please use `///` for continuation lines. Comment at: lib/Sema/SemaOverload.cpp: + bool PerformADL) { + assert(getLangOpts().CPlusPlus2a); + auto Opc = BinaryOperator::getOverloadedOpcode(Op); Does the implementation below actually rely on this? Comment at: lib/Sema/SemaOverload.cpp:8901 + // Lookup possible candidates for the rewritten operator. + // FIXME: should this really be done in the current scope? + LookupResult Operators(*this, OpName, SourceLocation(), No, you shouldn't be doing any lookups here. Instead, what you should do is to change the `LookupOverloadedOperatorName` call in `BuildOverloadedBinOp` to look up both `operator@` and `operator<=>` in the case where `@` is an equality or relational op, and then filter `Fns` by operator name both here and when adding the non-member function candidates. The reason is that we need to do these lookups in the context of the template definition when an equality or relational expression appears within a template, so we need to look up both the original operator and `<=>` eagerly. Comment at: lib/Sema/SemaOverload.cpp:8936-8938 + // TODO: We should be able to avoid adding synthesized candidates when LHS and + // RHS have the same type, since the synthesized candidates for <=> should be + // the same as the rewritten ones. Note: It's still possible for the result Type isn't sufficient to conclude this; you'll need to check the other relevant properties of LHS and RHS too (at least value kind, but maybe also bit-field-ness and others?). Comment at: lib/Sema/SemaOverload.cpp:9218-9219 +// --- F2 is a rewritten candidate ([over.match.oper]) and F1 is not. +if (Cand2.getRewrittenKind() && !Cand1.getRewrittenKind()) + return true; +if (Cand1.getRewrittenKind() && Cand2.getRewrittenKind() && You also need to check the reverse condition and return false (the "or if not that" is ... somehow ... supposed to imply that). Comment at: lib/Sema/SemaOverload.cpp:12502-12535 +ExprResult RewrittenCandidateOverloadResolver::BuildRewrittenCandidate( +const OverloadCandidate &Ovl) { + Expr *RewrittenArgs[2] = {Args[0], Args[1]}; + bool IsSynthesized = Ovl.getRewrittenKind() == ROC_Synthesized; + if (IsSynthesized) +std::swap(RewrittenArgs[0], RewrittenArgs[1]); + This results in an inadequate representation for the rewritten `<=>` form. We need to retain the "as-written" form of the operator, for multiple reasons (source fidelity for tooling, expression mangling, pretty-printing, ...). Generally, Clang's philosophy is to desugar the minimum amount that's necessary to capture both the syntactic and semantic forms. A couple of possible representations come to mind here: 1) A specific `Expr` subclass for a rewritten `<=>` operator, which is just a wrapper around a `(a <=> b) op 0` expression, and knows how to extract the subexpressions itself, or... 2) A general "rewritten operator wrapper" `Expr` subclass, which holds its operands and a rewritten expression, and for which the rewritten expression refers to its operands via `OpaqueValueExpr`s. We already have such a class (`PseudoObjectExpr`). The former is a smaller and simpler representation, but will likely be more work to implement. Comment at: lib/Sema/SemaOverload.cpp:12537-12565 +/// Rewritten candidates have been added but not checked for validity. They +/// could still be non-viable if: +/// (A) The rewritten call (x <=> y) is a builtin, but it will be ill-formed +/// when built (for example it has narrowing conversions). +/// (B) The expression (x <=> y) @ 0 is ill-formed for the result of (x <=> y). +/// +/// If either is the case, this function should be considered non-viable and Hmm, this is an interesting approach. It's quite a divergence from our usual approach, which is to perform the viability checks first, before partial ordering, even if we could eliminate some candidates based on partial ordering. And it's observable -- it will result in fewer template instantiations being performed, which will mean we do not diagnose some ill-formed (no diagnostic required) situations which would
[PATCH] D45643: [Failing one test] Reword [-Wreturn-type] messages to "non-void x does not return a value"
easyaspi314 added inline comments. Comment at: bindings/python/tests/cindex/test_diagnostics.py:18 self.assertEqual(tu.diagnostics[0].spelling, -'control reaches end of non-void function') +'non-void function does not return a value') Quuxplusone wrote: > rsmith wrote: > > It seems like part of the problem here is that "non-void function" is > > sort-of nonsense due to a few missing words. How about: > > > > > "control can reach end of function with non-void return type" > > > > or similar? > > > > I think we still need the mention of control flow, because we're *not* > > saying the function contains no return statements, we're saying there's a > > control flow path that reaches the end of the function. > I think OP's issue here is that the current message about "control" is > intrinsically confusing. //Of course// "control can reach end of function"; > if control never reaches the end of the function, you must have an infinite > loop somewhere! The important missing piece of the current message is that > control reaches the end of the function //without encountering any return > statement//. > > > we're *not* saying the function contains no return statements > > Not sure, but I think in this case we //are// saying that. There's a > different message, "non-void function //might// not return a value," for > those other cases. > > However, if I'm wrong about that, then another wording option close to OP's > suggestion would be "non-void function does not //always// return a value" / > "non-void function might not //always// return a value." > I think we still need the mention of control flow. I see where you are going. However, I have another idea, but I can't implement it myself. What if we give the user a note like this: ``` int someCondition; int a() { if (someCondition) { return 3; } } ``` ``` foo.c:3:1: warning: non-void function might not return a value [-Wreturn-type] int a() { ^ foo.c:4:5: note: assuming 'someCondition' is false if (someCondition) { ^ foo.c:7:1: note: control flow reaches end of function without a return value } ^ 1 warning generated. ``` Repository: rC Clang https://reviews.llvm.org/D45643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39053: [Bitfield] Add more cases to making the bitfield a separate location
spetrovic added a comment. ping https://reviews.llvm.org/D39053 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness
Eugene.Zelenko added a comment. In https://reviews.llvm.org/D45444#1068496, @shuaiwang wrote: > - I would imagine things could get messier if this check expands to also > check for turning member functions const: it's basically checking > CxxThisExpr, being a handle, is not modified within a member function, but > note there's no VarDecl for "this". Probably this should be separate check. See also PR21981. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45093: [AST] Fix -ast-print for _Bool when have diagnostics
jdenny updated this revision to Diff 142662. jdenny added a comment. Rebased onto a more recent master. https://reviews.llvm.org/D45093 Files: include/clang/AST/ASTContext.h include/clang/Sema/Sema.h lib/Parse/ParseDecl.cpp lib/Sema/Sema.cpp lib/Sema/SemaChecking.cpp lib/Sema/SemaCodeComplete.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/SemaLookup.cpp lib/Sema/SemaOverload.cpp lib/Sema/SemaTemplate.cpp lib/Sema/SemaTemplateInstantiate.cpp test/Misc/ast-print-bool.c Index: test/Misc/ast-print-bool.c === --- /dev/null +++ test/Misc/ast-print-bool.c @@ -0,0 +1,44 @@ +// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_CBOOL \ +// RUN: | FileCheck %s --check-prefixes=BOOL-AS-CBOOL,CBOOL +// +// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_CBOOL -DDIAG \ +// RUN: | FileCheck %s --check-prefixes=BOOL-AS-CBOOL,CBOOL +// +// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_INT \ +// RUN: | FileCheck %s --check-prefixes=BOOL-AS-INT,CBOOL +// +// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_INT -DDIAG \ +// RUN: | FileCheck %s --check-prefixes=BOOL-AS-INT,CBOOL +// +// RUN: %clang_cc1 -verify -ast-print %s -xc++ \ +// RUN: | FileCheck %s --check-prefixes=BOOL-AS-BOOL +// +// RUN: %clang_cc1 -verify -ast-print %s -xc++ -DDIAG \ +// RUN: | FileCheck %s --check-prefixes=BOOL-AS-BOOL + +#if DEF_BOOL_CBOOL +# define bool _Bool +#elif DEF_BOOL_INT +# define bool int +#endif + +// BOOL-AS-CBOOL: _Bool i; +// BOOL-AS-INT: int i; +// BOOL-AS-BOOL: bool i; +bool i; + +#ifndef __cplusplus +// CBOOL: _Bool j; +_Bool j; +#endif + +// Induce a diagnostic (and verify we actually managed to do so), which used to +// permanently alter the -ast-print printing policy for _Bool. How bool is +// defined by the preprocessor is examined only once per compilation, when the +// diagnostic is emitted, and it used to affect the entirety of -ast-print, so +// test only one definition of bool per compilation. +#if DIAG +void fn() { 1; } // expected-warning {{expression result unused}} +#else +// expected-no-diagnostics +#endif Index: lib/Sema/SemaTemplateInstantiate.cpp === --- lib/Sema/SemaTemplateInstantiate.cpp +++ lib/Sema/SemaTemplateInstantiate.cpp @@ -505,7 +505,7 @@ llvm::raw_svector_ostream OS(TemplateArgsStr); Template->printName(OS); printTemplateArgumentList(OS, Active->template_arguments(), -getPrintingPolicy()); +getDiagPrintingPolicy()); Diags.Report(Active->PointOfInstantiation, diag::note_default_arg_instantiation_here) << OS.str() @@ -571,7 +571,7 @@ llvm::raw_svector_ostream OS(TemplateArgsStr); FD->printName(OS); printTemplateArgumentList(OS, Active->template_arguments(), -getPrintingPolicy()); +getDiagPrintingPolicy()); Diags.Report(Active->PointOfInstantiation, diag::note_default_function_arg_instantiation_here) << OS.str() Index: lib/Sema/SemaTemplate.cpp === --- lib/Sema/SemaTemplate.cpp +++ lib/Sema/SemaTemplate.cpp @@ -3012,7 +3012,7 @@ std::string Description; { llvm::raw_string_ostream Out(Description); -FailedCond->printPretty(Out, nullptr, getPrintingPolicy()); +FailedCond->printPretty(Out, nullptr, getDiagPrintingPolicy()); } return { FailedCond, Description }; } @@ -9821,7 +9821,7 @@ } Out << " = "; -Args[I].print(getPrintingPolicy(), Out); +Args[I].print(getDiagPrintingPolicy(), Out); } Out << ']'; Index: lib/Sema/SemaOverload.cpp === --- lib/Sema/SemaOverload.cpp +++ lib/Sema/SemaOverload.cpp @@ -5546,7 +5546,7 @@ // conversion; use it. QualType ConvTy = Conversion->getConversionType().getNonReferenceType(); std::string TypeStr; -ConvTy.getAsStringInternal(TypeStr, SemaRef.getPrintingPolicy()); +ConvTy.getAsStringInternal(TypeStr, SemaRef.getDiagPrintingPolicy()); Converter.diagnoseExplicitConv(SemaRef, Loc, T, ConvTy) << FixItHint::CreateInsertion(From->getLocStart(), Index: lib/Sema/SemaLookup.cpp === --- lib/Sema/SemaLookup.cpp +++ lib/Sema/SemaLookup.cpp @@ -4202,7 +4202,7 @@ std::string NewQualified = TC.getAsString(SemaRef.getLangOpts()); std::string OldQualified; llvm::raw_string_ostream OldOStream(OldQualified); - SS->getScopeRep()->print(OldOStream, SemaRef.getPrintingPolicy()); + SS->getScopeRep()->print(OldOStream, SemaRef.getDiagPrintingPolicy()); OldOStream << Typ
[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.
Quuxplusone added a comment. I don't understand the spelling of this option. You spell it `-wtest` (because rjmccall suggested that spelling); but for my money it should be spelled `-Wno-self-assign-nonfield`. Also, if you go this route, you miss the true positive reported by someone on the other patch, where the self-assignment involved a local variable named `color`. Sorry I can't find the example right now. You'd have to trawl through all the patches opened on this subject in the past couple of weeks. Repository: rC Clang https://reviews.llvm.org/D45685 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)
On Mon, Apr 16, 2018 at 5:11 AM, Roman Lebedev via Phabricator via cfe-commits wrote: > lebedev.ri added a comment. > > Re false-positives - at least two [post-]reviewers need to agree on the > way forward (see previous comments, mail thread), and then i will implement > it. > > In https://reviews.llvm.org/D44883#1068576, @brooksmoses wrote: > > > A further concern about this in the general case from the reviewer of > one of my test-cleanup changes: The "var = *&var" idiom is not necessarily > equivalent to "var = var" in cases of user-defined types, because operator& > may be overloaded. > > > Release notes state: > > If you are doing such an assignment intentionally, e.g. in a unit test > for > a data structure, the warning can be suppressed by adding ``*&`` to the > right-hand side or casting it to the appropriate reference type. > > So it could also be `var = static_cast(var);` > Most people using clang won't read release notes. Maybe this could print a "to silence this warning, insert *&" fixit note, like many other warnings do. I think of the 3 points you mention, "separate flag for this" is probably the way to go. > > > Repository: > rC Clang > > https://reviews.llvm.org/D44883 > > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.
lebedev.ri added a comment. In https://reviews.llvm.org/D45685#1068981, @Quuxplusone wrote: > I don't understand the spelling of this option. You spell it `-wtest` > (because rjmccall suggested that spelling); but for my money it should be > spelled `-Wno-self-assign-nonfield`. You did read the documentation change, right? > Also, if you go this route, you miss the true positive reported by someone on > the other patch, where the self-assignment involved a local variable named > `color`. Uhm, why? `-wtest` is not the default, so the warning would still have been issued there. > Sorry I can't find the example right now. You'd have to trawl through all the > patches opened on this subject in the past couple of weeks. Repository: rC Clang https://reviews.llvm.org/D45685 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds
tejohnson marked 2 inline comments as done. tejohnson added inline comments. Comment at: include/clang/Driver/Options.td:2255 HelpText<"Save intermediate compilation results.">; -def save_temps : Flag<["-", "--"], "save-temps">, Flags<[DriverOption]>, +def save_temps : Flag<["-", "--"], "save-temps">, Flags<[CC1Option, DriverOption]>, Alias, AliasArgs<["cwd"]>, pcc wrote: > This is still just a DriverOption, right? No, we need to pass to cc1 to get it into the CodeGenOptions. See the changes to Clang.cpp (passes to cc1 invocation) and CompilerInvocation.cpp (consumes during cc1 invocation). Comment at: lib/Frontend/CompilerInvocation.cpp:747 +.Case("obj", FrontendOpts.OutputFile) +.Default("./" + + llvm::sys::path::filename(FrontendOpts.OutputFile).str()); pcc wrote: > Why do you need the `"./"` part? Not needed, removed. Comment at: test/CodeGen/thinlto_backend.ll:30 ; Ensure f2 was imported -; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc +; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s pcc wrote: > Should there be another test for `-save-temps=cwd`? Added -save-temps=cwd and -save-temps Repository: rC Clang https://reviews.llvm.org/D45217 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds
tejohnson updated this revision to Diff 142668. tejohnson marked 2 inline comments as done. tejohnson added a comment. Addressed comments Repository: rC Clang https://reviews.llvm.org/D45217 Files: include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.h lib/CodeGen/BackendUtil.cpp lib/Driver/Driver.cpp lib/Driver/ToolChains/Clang.cpp lib/Frontend/CompilerInvocation.cpp test/CodeGen/thinlto_backend.ll test/Driver/thinlto_backend.c Index: test/Driver/thinlto_backend.c === --- test/Driver/thinlto_backend.c +++ test/Driver/thinlto_backend.c @@ -5,6 +5,15 @@ // RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### 2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION // CHECK-THINLTOBE-ACTION: -fthinlto-index= +// -save-temps should be passed to cc1 +// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-CWD +// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-CWD +// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-OBJ +// CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc +// CHECK-SAVE-TEMPS-CWD: -save-temps=cwd +// CHECK-SAVE-TEMPS-OBJ: -save-temps=obj +// CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc + // Ensure clang driver gives the expected error for incorrect input type // RUN: not %clang -O2 -o %t1.o %s -c -fthinlto-index=%t.thinlto.bc 2>&1 | FileCheck %s -check-prefix=CHECK-WARNING // CHECK-WARNING: error: invalid argument '-fthinlto-index={{.*}}' only allowed with '-x ir' Index: test/CodeGen/thinlto_backend.ll === --- test/CodeGen/thinlto_backend.ll +++ test/CodeGen/thinlto_backend.ll @@ -26,8 +26,17 @@ ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t4.o -x ir %t5.o -c -fthinlto-index=%t.thinlto.bc ; RUN: llvm-nm %t4.o | count 0 -; Ensure f2 was imported -; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc +; Ensure f2 was imported. Check for all 3 flavors of -save-temps[=cwd|obj]. +; RUN: rm -f %t1.s.0.3.import.bc +; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj +; RUN: llvm-dis %t1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s +; RUN: rm -f %t1.s.0.3.import.bc +; RUN: (cd %T && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd) +; RUN: llvm-dis %t1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s +; RUN: rm -f %t1.s.0.3.import.bc +; RUN: (cd %T && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps) +; RUN: llvm-dis %t1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s +; CHECK-IMPORT: define available_externally void @f2() ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s ; CHECK-OBJ: T f1 ; CHECK-OBJ-NOT: U f2 Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -487,7 +487,8 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK, DiagnosticsEngine &Diags, - const TargetOptions &TargetOpts) { + const TargetOptions &TargetOpts, + const FrontendOptions &FrontendOpts) { bool Success = true; llvm::Triple Triple = llvm::Triple(TargetOpts.Triple); @@ -739,6 +740,12 @@ << A->getAsString(Args) << "-x ir"; Opts.ThinLTOIndexFile = Args.getLastArgValue(OPT_fthinlto_index_EQ); } + if (Arg *A = Args.getLastArg(OPT_save_temps_EQ)) +Opts.SaveTempsFilePrefix = +llvm::StringSwitch(A->getValue()) +.Case("obj", FrontendOpts.OutputFile) +.Default(llvm::sys::path::filename(FrontendOpts.OutputFile).str()); + Opts.ThinLinkBitcodeFile = Args.getLastArgValue(OPT_fthin_link_bitcode_EQ); Opts.MSVolatile = Args.hasArg(OPT_fms_volatile); @@ -2890,7 +2897,7 @@ LangOpts.IsHeaderFile); ParseTargetArgs(Res.getTargetOpts(), Args, Diags); Success &= ParseCodeGenArgs(Res.getCodeGenOpts(), Args, DashX, Diags, - Res.getTargetOpts()); + Res.getTargetOpts(), Res.getFrontendOpts()); ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args, Res.getFileSystemOpts().WorkingDir); if (DashX.getFormat() == InputKind::Precom
[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness
shuaiwang added a comment. >> I would imagine things could get messier if this check expands to also >> check for turning member functions const: it's basically checking >> CxxThisExpr, being a handle, is not modified within a member function, but >> note there's no VarDecl for "this". > > Using my approach, you can check if any member variable is used as non-const. > Then mark this method as const, if it is not virtual. > Similar for member variables: private non-consts can be converted into > consts. You'll also need to check: - a member function calling another non-const member function -> *this can't be const - *this is passed as non-const reference param to a function -> *this can't be const Also when marking decls as "can't be const" we'll have to do record separately for modifying behaviors coming from different functions. Which of course are all possible but code will get too complex than necessary IMO. I think member variables are a separate topic: I think we should just treat them as globals and not check on them, the same argument that they can be accessed from multiple translation unit applies to global/namespace scoped variables and class scoped variables. We can only reliably check function/block scope variables. (reliable meaning being able to achieve zero false positives with useful level of recall, false negatives are inevitable because whenever a modifiable reference/handle escape the current block/translation unit we'll have to assume it's modified.) >> It's also cleaner to follow member access chains, e.g. "v.a.b.c = 10" >> modifies "v". This one is particularly tricky because the modifying behavior >> happens at "c = 10" while the variable we'd like to mark as "can't be const" >> is arbitrary layers away, and MemberExpr alone doesn't warrant marking >> "can't be const" because that'll introduce way too many false negatives, ... >> and I haven't figure out a way to write a matcher to match >> memberExpr(hasObjectExpression(memberExpr(hasObjectExpression(... > layers deep> ... (VarDeclRef) ...), not to mention any layer could either >> member access or array subscript access. > > Does your approach work with the nesting? Maybe something recursive or > `hasDescendent(modifying)` could do it? Yes my approach is doing multi-pass matching by calling isModified() recursively. I consider the multi-pass matching "necessary evil" because otherwise we'll have too many false negatives. I thought about hasDescendent (hasAncestor actually) but I don't think that makes things easier: varDeclRef(hasAncestor(modifying)) would match both "v.a.b.c. = 10" and "map[v] = 10" and we'll still need to double check the modifying behavior does link back to the particular varDeclRef. Reviewers, what are your thoughts about the approaches? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness
shuaiwang added a comment. In https://reviews.llvm.org/D45444#1068967, @Eugene.Zelenko wrote: > In https://reviews.llvm.org/D45444#1068496, @shuaiwang wrote: > > > - I would imagine things could get messier if this check expands to also > > check for turning member functions const: it's basically checking > > CxxThisExpr, being a handle, is not modified within a member function, but > > note there's no VarDecl for "this". > > > Probably this should be separate check. See also PR21981. Sure, a separate check sounds better. Which makes even strong argument of having a reusable utility checking whether something is modified that can be shared between different checks :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds
pcc added inline comments. Comment at: include/clang/Driver/Options.td:2255 HelpText<"Save intermediate compilation results.">; -def save_temps : Flag<["-", "--"], "save-temps">, Flags<[DriverOption]>, +def save_temps : Flag<["-", "--"], "save-temps">, Flags<[CC1Option, DriverOption]>, Alias, AliasArgs<["cwd"]>, tejohnson wrote: > pcc wrote: > > This is still just a DriverOption, right? > No, we need to pass to cc1 to get it into the CodeGenOptions. See the changes > to Clang.cpp (passes to cc1 invocation) and CompilerInvocation.cpp (consumes > during cc1 invocation). You are producing a `-save-temps=` flag, not `-save-temps`. i.e. I think you should be able to remove the CC1Option from this line and keep it on line 2253. Comment at: test/CodeGen/thinlto_backend.ll:30 ; Ensure f2 was imported -; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc +; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s tejohnson wrote: > pcc wrote: > > Should there be another test for `-save-temps=cwd`? > Added -save-temps=cwd and -save-temps This isn't actually testing that the behaviour is different between `-save-temps=obj` and `-save-temps=cwd`, right? Maybe create a new directory, cd there and verify that the files are created there? Repository: rC Clang https://reviews.llvm.org/D45217 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds
tejohnson marked 2 inline comments as done. tejohnson added inline comments. Comment at: include/clang/Driver/Options.td:2255 HelpText<"Save intermediate compilation results.">; -def save_temps : Flag<["-", "--"], "save-temps">, Flags<[DriverOption]>, +def save_temps : Flag<["-", "--"], "save-temps">, Flags<[CC1Option, DriverOption]>, Alias, AliasArgs<["cwd"]>, pcc wrote: > tejohnson wrote: > > pcc wrote: > > > This is still just a DriverOption, right? > > No, we need to pass to cc1 to get it into the CodeGenOptions. See the > > changes to Clang.cpp (passes to cc1 invocation) and CompilerInvocation.cpp > > (consumes during cc1 invocation). > You are producing a `-save-temps=` flag, not `-save-temps`. > > i.e. I think you should be able to remove the CC1Option from this line and > keep it on line 2253. Oh, I see what you are saying. Removed CC1Option from the plain -save-temps. Comment at: test/CodeGen/thinlto_backend.ll:30 ; Ensure f2 was imported -; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc +; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s pcc wrote: > tejohnson wrote: > > pcc wrote: > > > Should there be another test for `-save-temps=cwd`? > > Added -save-temps=cwd and -save-temps > This isn't actually testing that the behaviour is different between > `-save-temps=obj` and `-save-temps=cwd`, right? > > Maybe create a new directory, cd there and verify that the files are created > there? Using different directories for all 3 cases now. Repository: rC Clang https://reviews.llvm.org/D45217 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds
tejohnson updated this revision to Diff 142669. tejohnson marked 2 inline comments as done. tejohnson added a comment. Address comments Repository: rC Clang https://reviews.llvm.org/D45217 Files: include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.h lib/CodeGen/BackendUtil.cpp lib/Driver/Driver.cpp lib/Driver/ToolChains/Clang.cpp lib/Frontend/CompilerInvocation.cpp test/CodeGen/thinlto_backend.ll test/Driver/thinlto_backend.c Index: test/Driver/thinlto_backend.c === --- test/Driver/thinlto_backend.c +++ test/Driver/thinlto_backend.c @@ -5,6 +5,15 @@ // RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### 2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION // CHECK-THINLTOBE-ACTION: -fthinlto-index= +// -save-temps should be passed to cc1 +// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-CWD +// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-CWD +// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-OBJ +// CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc +// CHECK-SAVE-TEMPS-CWD: -save-temps=cwd +// CHECK-SAVE-TEMPS-OBJ: -save-temps=obj +// CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc + // Ensure clang driver gives the expected error for incorrect input type // RUN: not %clang -O2 -o %t1.o %s -c -fthinlto-index=%t.thinlto.bc 2>&1 | FileCheck %s -check-prefix=CHECK-WARNING // CHECK-WARNING: error: invalid argument '-fthinlto-index={{.*}}' only allowed with '-x ir' Index: test/CodeGen/thinlto_backend.ll === --- test/CodeGen/thinlto_backend.ll +++ test/CodeGen/thinlto_backend.ll @@ -26,8 +26,16 @@ ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t4.o -x ir %t5.o -c -fthinlto-index=%t.thinlto.bc ; RUN: llvm-nm %t4.o | count 0 -; Ensure f2 was imported -; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc +; Ensure f2 was imported. Check for all 3 flavors of -save-temps[=cwd|obj]. +; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj +; RUN: llvm-dis %t1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s +; RUN: mkdir -p %T/dir1 +; RUN: (cd %T/dir1 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd) +; RUN: llvm-dis %T/dir1/*1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s +; RUN: mkdir -p %T/dir2 +; RUN: (cd %T/dir2 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps) +; RUN: llvm-dis %T/dir2/*1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s +; CHECK-IMPORT: define available_externally void @f2() ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s ; CHECK-OBJ: T f1 ; CHECK-OBJ-NOT: U f2 Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -487,7 +487,8 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK, DiagnosticsEngine &Diags, - const TargetOptions &TargetOpts) { + const TargetOptions &TargetOpts, + const FrontendOptions &FrontendOpts) { bool Success = true; llvm::Triple Triple = llvm::Triple(TargetOpts.Triple); @@ -739,6 +740,12 @@ << A->getAsString(Args) << "-x ir"; Opts.ThinLTOIndexFile = Args.getLastArgValue(OPT_fthinlto_index_EQ); } + if (Arg *A = Args.getLastArg(OPT_save_temps_EQ)) +Opts.SaveTempsFilePrefix = +llvm::StringSwitch(A->getValue()) +.Case("obj", FrontendOpts.OutputFile) +.Default(llvm::sys::path::filename(FrontendOpts.OutputFile).str()); + Opts.ThinLinkBitcodeFile = Args.getLastArgValue(OPT_fthin_link_bitcode_EQ); Opts.MSVolatile = Args.hasArg(OPT_fms_volatile); @@ -2890,7 +2897,7 @@ LangOpts.IsHeaderFile); ParseTargetArgs(Res.getTargetOpts(), Args, Diags); Success &= ParseCodeGenArgs(Res.getCodeGenOpts(), Args, DashX, Diags, - Res.getTargetOpts()); + Res.getTargetOpts(), Res.getFrontendOpts()); ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args, Res.getFileSystemOpts().WorkingDir); if (DashX.getFormat() == InputKind::Precompiled || Index: lib/Driver/Too
[PATCH] D45697: [clang-tidy] Fix clang-tidy doesn't read .clangtidy configuration file.
hokein created this revision. hokein added a reviewer: alexfh. Herald added subscribers: xazax.hun, klimek. Fix https://bugs.llvm.org/show_bug.cgi?id=34900. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45697 Files: clang-tidy/ClangTidyOptions.cpp clang-tidy/ClangTidyOptions.h clang-tidy/tool/ClangTidyMain.cpp test/clang-tidy/read_file_config.cpp Index: test/clang-tidy/read_file_config.cpp === --- /dev/null +++ test/clang-tidy/read_file_config.cpp @@ -0,0 +1,11 @@ +// RUN: mkdir -p %T/read-file-config/ +// RUN: cp %s %T/read-file-config/test.cpp +// RUN: echo 'Checks: "-*,modernize-use-nullptr"' > %T/read-file-config/.clang-tidy +// RUN: echo '[{"command": "cc -c -o test.o test.cpp", "directory": "%T/read-file-config", "file": "%T/read-file-config/test.cpp"}]' > %T/read-file-config/compile_commands.json +// RUN: clang-tidy %T/read-file-config/test.cpp | not grep "clang-analyzer-*" + +void f() { + int x; + x = 1; + x = 2; +} Index: clang-tidy/tool/ClangTidyMain.cpp === --- clang-tidy/tool/ClangTidyMain.cpp +++ clang-tidy/tool/ClangTidyMain.cpp @@ -286,7 +286,8 @@ OS.flush(); } -static std::unique_ptr createOptionsProvider() { +static std::unique_ptr createOptionsProvider( + llvm::IntrusiveRefCntPtr FS) { ClangTidyGlobalOptions GlobalOptions; if (std::error_code Err = parseLineFilter(LineFilter, GlobalOptions)) { llvm::errs() << "Invalid LineFilter: " << Err.message() << "\n\nUsage:\n"; @@ -334,7 +335,7 @@ } } return llvm::make_unique(GlobalOptions, DefaultOptions, -OverrideOptions); +OverrideOptions, std::move(FS)); } llvm::IntrusiveRefCntPtr @@ -364,8 +365,13 @@ static int clangTidyMain(int argc, const char **argv) { CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory, cl::ZeroOrMore); + llvm::IntrusiveRefCntPtr BaseFS( + VfsOverlay.empty() ? vfs::getRealFileSystem() + : getVfsOverlayFromFile(VfsOverlay)); + if (!BaseFS) +return 1; - auto OwningOptionsProvider = createOptionsProvider(); + auto OwningOptionsProvider = createOptionsProvider(BaseFS); auto *OptionsProvider = OwningOptionsProvider.get(); if (!OptionsProvider) return 1; @@ -432,12 +438,6 @@ llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true); return 1; } - llvm::IntrusiveRefCntPtr BaseFS( - VfsOverlay.empty() ? vfs::getRealFileSystem() - : getVfsOverlayFromFile(VfsOverlay)); - if (!BaseFS) -return 1; - ProfileData Profile; llvm::InitializeAllTargetInfos(); Index: clang-tidy/ClangTidyOptions.h === --- clang-tidy/ClangTidyOptions.h +++ clang-tidy/ClangTidyOptions.h @@ -13,7 +13,9 @@ #include "llvm/ADT/Optional.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/Support/ErrorOr.h" +#include "clang/Basic/VirtualFileSystem.h" #include #include #include @@ -221,7 +223,8 @@ /// whatever options are read from the configuration file. FileOptionsProvider(const ClangTidyGlobalOptions &GlobalOptions, const ClangTidyOptions &DefaultOptions, - const ClangTidyOptions &OverrideOptions); + const ClangTidyOptions &OverrideOptions, + llvm::IntrusiveRefCntPtr FS = nullptr); /// \brief Initializes the \c FileOptionsProvider instance with a custom set /// of configuration file handlers. @@ -255,6 +258,7 @@ llvm::StringMap CachedOptions; ClangTidyOptions OverrideOptions; ConfigFileHandlers ConfigHandlers; + llvm::IntrusiveRefCntPtr FS; }; /// \brief Parses LineFilter from JSON and stores it to the \p Options. Index: clang-tidy/ClangTidyOptions.cpp === --- clang-tidy/ClangTidyOptions.cpp +++ clang-tidy/ClangTidyOptions.cpp @@ -204,9 +204,12 @@ FileOptionsProvider::FileOptionsProvider( const ClangTidyGlobalOptions &GlobalOptions, const ClangTidyOptions &DefaultOptions, -const ClangTidyOptions &OverrideOptions) +const ClangTidyOptions &OverrideOptions, +llvm::IntrusiveRefCntPtr VFS) : DefaultOptionsProvider(GlobalOptions, DefaultOptions), - OverrideOptions(OverrideOptions) { + OverrideOptions(OverrideOptions), FS(std::move(VFS)) { + if (!FS) +FS = vfs::getRealFileSystem(); ConfigHandlers.emplace_back(".clang-tidy", parseConfiguration); } @@ -224,14 +227,19 @@ std::vector FileOptionsProvider::getRawOptions(StringRef FileName) { DEBUG(llvm::dbgs() << "Getting options for file " << FileName << "...\n"); + assert(FS && "FS must be set."); + + ll
[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.
dblaikie added a subscriber: lebedev.ri. dblaikie added a comment. I'm not sure this is a practical direction to pursue - though perhaps others disagree. It's likely non-trivial to plumb a flag through most build systems to be applied only to test code (& likely would suppress the warning in headers only included in test code - so for example, in a header-only library if the user ran their tests they wouldn't see the warning, but when the users code was used in some other production code it might warn (& possibly break the build if -Werror is in use)) Repository: rC Clang https://reviews.llvm.org/D45685 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.
I'm not sure this is a practical direction to pursue - though perhaps others disagree. It's likely non-trivial to plumb a flag through most build systems to be applied only to test code (& likely would suppress the warning in headers only included in test code - so for example, in a header-only library if the user ran their tests they wouldn't see the warning, but when the users code was used in some other production code it might warn (& possibly break the build if -Werror is in use)) On Mon, Apr 16, 2018 at 8:10 AM Roman Lebedev via Phabricator < revi...@reviews.llvm.org> wrote: > lebedev.ri updated this revision to Diff 142636. > lebedev.ri retitled this revision from "[Sema] Add -Wtest global flag that > silences -Wself-assign for overloaded operators." to "[Sema] Add -wtest > global flag that silences -Wself-assign for overloaded operators.". > lebedev.ri edited the summary of this revision. > lebedev.ri added a comment. > > Actually make it `-wtest` (all lowercase). > > > Repository: > rC Clang > > https://reviews.llvm.org/D45685 > > Files: > docs/ReleaseNotes.rst > docs/UsersManual.rst > include/clang/AST/ASTDiagnostic.h > include/clang/AST/CommentDiagnostic.h > include/clang/Basic/Diagnostic.h > include/clang/Basic/Diagnostic.td > include/clang/Basic/DiagnosticIDs.h > include/clang/Basic/DiagnosticOptions.def > include/clang/Basic/DiagnosticSemaKinds.td > include/clang/CrossTU/CrossTUDiagnostic.h > include/clang/Driver/DriverDiagnostic.h > include/clang/Driver/Options.td > include/clang/Frontend/FrontendDiagnostic.h > include/clang/Lex/LexDiagnostic.h > include/clang/Parse/ParseDiagnostic.h > include/clang/Sema/SemaDiagnostic.h > include/clang/Serialization/SerializationDiagnostic.h > include/clang/Tooling/Refactoring/RefactoringDiagnostic.h > lib/Basic/DiagnosticIDs.cpp > lib/Basic/Warnings.cpp > lib/Driver/Driver.cpp > lib/Frontend/CompilerInvocation.cpp > lib/Sema/SemaExpr.cpp > test/Sema/warn-self-assign-builtin-warn-test.c > test/SemaCXX/warn-self-assign-builtin-warn-test.cpp > test/SemaCXX/warn-self-assign-field-builtin-warn-test.cpp > test/SemaCXX/warn-self-assign-field-overloaded-warn-test.cpp > test/SemaCXX/warn-self-assign-overloaded-warn-test.cpp > tools/diagtool/DiagnosticNames.cpp > utils/TableGen/ClangDiagnosticsEmitter.cpp > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r330132 - [CodeView] Initial support for emitting S_THUNK32 symbols for compiler...
Author: bwyma Date: Mon Apr 16 09:53:57 2018 New Revision: 330132 URL: http://llvm.org/viewvc/llvm-project?rev=330132&view=rev Log: [CodeView] Initial support for emitting S_THUNK32 symbols for compiler... When emitting CodeView debug information, compiler-generated thunk routines should be emitted using S_THUNK32 symbols instead of S_GPROC32_ID symbols so Visual Studio can properly step into the user code. This initial support only handles standard thunk ordinals. Differential Revision: https://reviews.llvm.org/D43838 Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp cfe/trunk/lib/CodeGen/CGDebugInfo.h cfe/trunk/lib/CodeGen/CodeGenFunction.cpp cfe/trunk/test/CodeGenCXX/debug-info-thunk.cpp Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=330132&r1=330131&r2=330132&view=diff == --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original) +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Apr 16 09:53:57 2018 @@ -3217,7 +3217,8 @@ llvm::DISubroutineType *CGDebugInfo::get void CGDebugInfo::EmitFunctionStart(GlobalDecl GD, SourceLocation Loc, SourceLocation ScopeLoc, QualType FnType, -llvm::Function *Fn, CGBuilderTy &Builder) { +llvm::Function *Fn, bool CurFuncIsThunk, +CGBuilderTy &Builder) { StringRef Name; StringRef LinkageName; @@ -3263,6 +3264,10 @@ void CGDebugInfo::EmitFunctionStart(Glob // Artificial functions should not silently reuse CurLoc. CurLoc = SourceLocation(); } + + if (CurFuncIsThunk) +Flags |= llvm::DINode::FlagThunk; + unsigned LineNo = getLineNumber(Loc); unsigned ScopeLine = getLineNumber(ScopeLoc); Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=330132&r1=330131&r2=330132&view=diff == --- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original) +++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Mon Apr 16 09:53:57 2018 @@ -366,7 +366,8 @@ public: /// \param ScopeLoc The location of the function body. void EmitFunctionStart(GlobalDecl GD, SourceLocation Loc, SourceLocation ScopeLoc, QualType FnType, - llvm::Function *Fn, CGBuilderTy &Builder); + llvm::Function *Fn, bool CurFnIsThunk, + CGBuilderTy &Builder); /// Start a new scope for an inlined function. void EmitInlineFunctionStart(CGBuilderTy &Builder, GlobalDecl GD); Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=330132&r1=330131&r2=330132&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Mon Apr 16 09:53:57 2018 @@ -1018,7 +1018,8 @@ void CodeGenFunction::StartFunction(Glob ArgTypes.push_back(VD->getType()); QualType FnType = getContext().getFunctionType( RetTy, ArgTypes, FunctionProtoType::ExtProtoInfo(CC)); -DI->EmitFunctionStart(GD, Loc, StartLoc, FnType, CurFn, Builder); +DI->EmitFunctionStart(GD, Loc, StartLoc, FnType, CurFn, CurFuncIsThunk, + Builder); } if (ShouldInstrumentFunction()) { Modified: cfe/trunk/test/CodeGenCXX/debug-info-thunk.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-thunk.cpp?rev=330132&r1=330131&r2=330132&view=diff == --- cfe/trunk/test/CodeGenCXX/debug-info-thunk.cpp (original) +++ cfe/trunk/test/CodeGenCXX/debug-info-thunk.cpp Mon Apr 16 09:53:57 2018 @@ -1,29 +1,277 @@ -// RUN: %clang_cc1 %s -triple %itanium_abi_triple -debug-info-kind=limited -S -emit-llvm -o - | FileCheck %s - -struct A { - virtual void f(); -}; - -struct B { - virtual void f(); -}; - -struct C : A, B { - virtual void f(); -}; - -void C::f() { } -// CHECK: define {{.*}}void @_ZThn{{[48]}}_N1C1fEv -// CHECK-SAME: !dbg ![[SP:[0-9]+]] -// CHECK-NOT: {{ret }} -// CHECK: = load{{.*}} !dbg ![[DBG:[0-9]+]] -// CHECK-NOT: {{ret }} -// CHECK: ret void, !dbg ![[DBG]] +// RUN: %clang_cc1 %s -triple=x86_64-pc-windows-msvc -debug-info-kind=limited -S -emit-llvm -o - | FileCheck %s +// RUN: %clang_cc1 %s -triple %itanium_abi_triple -debug-info-kind=limited -S -emit-llvm -o - | FileCheck %s -check-prefix=ITANIUM +// +// Validate we emit a "DIFlagThunk" flag on DISubprogram entries for thunks. +// This flag is used for emitting S_THUNK32 symbols for CodeView debugging. +// +// NOTE: +// Because thunks are compiler generated and don't exist in the source,
[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.
lebedev.ri added a comment. In https://reviews.llvm.org/D45685#1069040, @dblaikie wrote: > I'm not sure this is a practical direction to pursue - though perhaps > others disagree. > It's likely non-trivial to plumb a flag through most build systems to be > applied only to test code I'm sorry, I don't understand. If you don't separate between source code and `*_test.cpp` sources, sure, you will loose the warning coverage either way. What difference is there between passing `-wtest` and passing `-Wno-self-assign-overloaded`? Just pass it alongside with the googletest include paths so to speak. > (& likely would suppress the warning in headers > only included in test code - so for example, in a header-only library if > the user ran their tests they wouldn't see the warning, but when the users > code was used in some other production code it might warn (& possibly break > the build if -Werror is in use)) Could you please explain how it is not an issue if this would be a `-Wno-self-assign-overloaded`? Repository: rC Clang https://reviews.llvm.org/D45685 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
efriedma added a comment. We can could add an exception to the "don't allow redeclarations with custom type-checking" rule to specifically allow redeclaring `__va_start`. The general rule is that we don't want user code redeclaring them because they don't have a specific signature, so our internal representation could change between releases. But `__va_start` has a specific fixed signature from the Microsoft SDK headers, so it should be fine to allow redeclaring it without a warning. https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.
On Mon, Apr 16, 2018 at 12:08 PM Roman Lebedev via Phabricator < revi...@reviews.llvm.org> wrote: > lebedev.ri added a comment. > > In https://reviews.llvm.org/D45685#1069040, @dblaikie wrote: > > > I'm not sure this is a practical direction to pursue - though perhaps > > others disagree. > > > > > > It's likely non-trivial to plumb a flag through most build systems to be > > applied only to test code > > I'm sorry, I don't understand. > > If you don't separate between source code and `*_test.cpp` sources, sure, > you will loose the warning coverage either way. > > What difference is there between passing `-wtest` and passing > `-Wno-self-assign-overloaded`? > Not much, if any. > Just pass it alongside with the googletest include paths so to speak. > But build systems don't necessarily expose that kind of ability. (For example, googletest (not the only kind of test suite/code) is checked into the LLVM codebase like another library - so depending on it is just another library dependency, not a special "test" library dependency). > > > (& likely would suppress the warning in headers > > only included in test code - so for example, in a header-only library if > > the user ran their tests they wouldn't see the warning, but when the > users > > code was used in some other production code it might warn (& possibly > break > > the build if -Werror is in use)) > > Could you please explain how it is not an issue if this would be a > `-Wno-self-assign-overloaded`? > It isn't any different - though it sounds like "-wtest" is being proposed as a way that test code could specifically be corrected without impacting the rest of the code. I'm not sure that's true/viable/accessible to most developers, so the justification seems a bit infeasible - likely naming it -Wno-self-assign-overloaded would at least name it how it'll be used, to turn the warning off across a whole codebase because the false positive rate across the whole codebase is unacceptable for a given user. > > > Repository: > rC Clang > > https://reviews.llvm.org/D45685 > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45456: [Attr] Print enum attributes at correct position
jdenny updated this revision to Diff 142672. jdenny edited the summary of this revision. jdenny added a comment. Rebased onto a more recent master. Added example to summary. https://reviews.llvm.org/D45456 Files: lib/AST/DeclPrinter.cpp test/Sema/ast-print.c Index: test/Sema/ast-print.c === --- test/Sema/ast-print.c +++ test/Sema/ast-print.c @@ -1,5 +1,12 @@ -// RUN: %clang_cc1 %s -ast-print | FileCheck %s -// RUN: %clang_cc1 %s -ast-print | %clang_cc1 -fsyntax-only - +// RUN: %clang_cc1 %s -ast-print -verify > %t.c +// RUN: FileCheck %s --input-file %t.c +// +// RUN: echo >> %t.c "// expected""-warning@* {{use of GNU old-style field designator extension}}" +// RUN: echo >> %t.c "// expected""-warning@* {{'EnumWithAttributes' is deprecated}}" +// RUN: echo >> %t.c "// expected""-note@* {{'EnumWithAttributes' has been explicitly marked deprecated here}}" +// RUN: echo >> %t.c "// expected""-warning@* {{'EnumWithAttributes3' is deprecated}}" +// RUN: echo >> %t.c "// expected""-note@* {{'EnumWithAttributes3' has been explicitly marked deprecated here}}" +// RUN: %clang_cc1 -fsyntax-only %t.c -verify typedef void func_typedef(); func_typedef xxx; @@ -58,7 +65,7 @@ }; // CHECK: struct pair_t p = {a: 3, .b = 4}; -struct pair_t p = {a: 3, .b = 4}; +struct pair_t p = {a: 3, .b = 4}; // expected-warning {{use of GNU old-style field designator extension}} void initializers() { // CHECK: int *x = ((void *)0), *y = ((void *)0); @@ -70,11 +77,30 @@ } z = {(struct Z){}}; } -// CHECK-LABEL: enum EnumWithAttributes { -enum EnumWithAttributes { +// CHECK-LABEL: enum __attribute__((deprecated(""))) EnumWithAttributes { +enum EnumWithAttributes { // expected-warning {{'EnumWithAttributes' is deprecated}} // CHECK-NEXT: EnumWithAttributesFoo __attribute__((deprecated(""))), EnumWithAttributesFoo __attribute__((deprecated)), // CHECK-NEXT: EnumWithAttributesBar __attribute__((unavailable(""))) = 50 EnumWithAttributesBar __attribute__((unavailable)) = 50 - // CHECK-NEXT: } __attribute__((deprecated(""))) -} __attribute__((deprecated)); + // CHECK-NEXT: }; + // CHECK-NEXT: enum EnumWithAttributes *EnumWithAttributesPtr; +} __attribute__((deprecated)) *EnumWithAttributesPtr; // expected-note {{'EnumWithAttributes' has been explicitly marked deprecated here}} + +// FIXME: If enum is forward-declared at file scope, attributes are lost. +// CHECK-LABEL: enum EnumWithAttributes2 *EnumWithAttributes2Ptr; +// expected-warning@+2 {{'EnumWithAttributes2' is deprecated}} +// expected-note@+1 {{'EnumWithAttributes2' has been explicitly marked deprecated here}} +enum __attribute__((deprecated)) EnumWithAttributes2 *EnumWithAttributes2Ptr; + +// CHECK-LABEL: EnumWithAttributes3Fn +void EnumWithAttributes3Fn() { + // CHECK-NEXT: enum __attribute__((deprecated(""))) EnumWithAttributes3 *EnumWithAttributes3Ptr; + // expected-warning@+2 {{'EnumWithAttributes3' is deprecated}} + // expected-note@+1 {{'EnumWithAttributes3' has been explicitly marked deprecated here}} + enum __attribute__((deprecated)) EnumWithAttributes3 *EnumWithAttributes3Ptr; + // Printing must not put the attribute after the tag where it would apply to + // the variable instead of the type, and then our deprecation warning would + // move to this use of the variable. + void *p = EnumWithAttributes3Ptr; +} Index: lib/AST/DeclPrinter.cpp === --- lib/AST/DeclPrinter.cpp +++ lib/AST/DeclPrinter.cpp @@ -496,14 +496,17 @@ void DeclPrinter::VisitEnumDecl(EnumDecl *D) { if (!Policy.SuppressSpecifiers && D->isModulePrivate()) Out << "__module_private__ "; - Out << "enum "; + Out << "enum"; if (D->isScoped()) { if (D->isScopedUsingClassTag()) - Out << "class "; + Out << " class"; else - Out << "struct "; + Out << " struct"; } - Out << *D; + + prettyPrintAttributes(D); + + Out << ' ' << *D; if (D->isFixed() && D->getASTContext().getLangOpts().CPlusPlus11) Out << " : " << D->getIntegerType().stream(Policy); @@ -513,7 +516,6 @@ VisitDeclContext(D); Indent() << "}"; } - prettyPrintAttributes(D); } void DeclPrinter::VisitRecordDecl(RecordDecl *D) { Index: test/Sema/ast-print.c === --- test/Sema/ast-print.c +++ test/Sema/ast-print.c @@ -1,5 +1,12 @@ -// RUN: %clang_cc1 %s -ast-print | FileCheck %s -// RUN: %clang_cc1 %s -ast-print | %clang_cc1 -fsyntax-only - +// RUN: %clang_cc1 %s -ast-print -verify > %t.c +// RUN: FileCheck %s --input-file %t.c +// +// RUN: echo >> %t.c "// expected""-warning@* {{use of GNU old-style field designator extension}}" +// RUN: echo >> %t.c "// expected""-warning@* {{'EnumWithAttributes' is deprecated}}" +// RUN: echo >> %t.c "// expected""-note@* {{'EnumWithAttributes' has been explicitly marked deprecated here}}" +// RUN: echo >>
r330150 - [Hexagon] Emit a warning when -fvectorize is given without -mhvx
Author: kparzysz Date: Mon Apr 16 12:11:17 2018 New Revision: 330150 URL: http://llvm.org/viewvc/llvm-project?rev=330150&view=rev Log: [Hexagon] Emit a warning when -fvectorize is given without -mhvx Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp cfe/trunk/lib/Driver/ToolChains/Hexagon.h cfe/trunk/test/Driver/hexagon-vectorize.c Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=330150&r1=330149&r2=330150&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Mon Apr 16 12:11:17 2018 @@ -288,6 +288,9 @@ def err_analyzer_config_multiple_values def err_drv_invalid_hvx_length : Error< "-mhvx-length is not supported without a -mhvx/-mhvx= flag">; +def warn_drv_vectorize_needs_hvx : Warning< + "auto-vectorization requires HVX, use -mhvx to enable it">, + InGroup; def err_drv_modules_validate_once_requires_timestamp : Error< "option '-fmodules-validate-once-per-build-session' requires " Modified: cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp?rev=330150&r1=330149&r2=330150&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp Mon Apr 16 12:11:17 2018 @@ -108,8 +108,11 @@ void hexagon::getHexagonTargetFeatures(c Features.push_back(UseLongCalls ? "+long-calls" : "-long-calls"); - bool HasHVX(false); + bool HasHVX = false; handleHVXTargetFeatures(D, Args, Features, HasHVX); + + if (HexagonToolChain::isAutoHVXEnabled(Args) && !HasHVX) +D.Diag(diag::warn_drv_vectorize_needs_hvx); } // Hexagon tools start. @@ -520,12 +523,9 @@ void HexagonToolChain::addClangTargetOpt CC1Args.push_back("-target-feature"); CC1Args.push_back("+reserved-r19"); } - if (Arg *A = DriverArgs.getLastArg(options::OPT_fvectorize, - options::OPT_fno_vectorize)) { -if (A->getOption().matches(options::OPT_fvectorize)) { - CC1Args.push_back("-mllvm"); - CC1Args.push_back("-hexagon-autohvx"); -} + if (isAutoHVXEnabled(DriverArgs)) { +CC1Args.push_back("-mllvm"); +CC1Args.push_back("-hexagon-autohvx"); } } @@ -564,6 +564,13 @@ HexagonToolChain::GetCXXStdlibType(const return ToolChain::CST_Libstdcxx; } +bool HexagonToolChain::isAutoHVXEnabled(const llvm::opt::ArgList &Args) { + if (Arg *A = Args.getLastArg(options::OPT_fvectorize, + options::OPT_fno_vectorize)) +return A->getOption().matches(options::OPT_fvectorize); + return false; +} + // // Returns the default CPU for Hexagon. This is the default compilation target // if no Hexagon processor is selected at the command-line. Modified: cfe/trunk/lib/Driver/ToolChains/Hexagon.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Hexagon.h?rev=330150&r1=330149&r2=330150&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Hexagon.h (original) +++ cfe/trunk/lib/Driver/ToolChains/Hexagon.h Mon Apr 16 12:11:17 2018 @@ -94,6 +94,7 @@ public: void getHexagonLibraryPaths(const llvm::opt::ArgList &Args, ToolChain::path_list &LibPaths) const; + static bool isAutoHVXEnabled(const llvm::opt::ArgList &Args); static const StringRef GetDefaultCPU(); static const StringRef GetTargetCPUVersion(const llvm::opt::ArgList &Args); Modified: cfe/trunk/test/Driver/hexagon-vectorize.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hexagon-vectorize.c?rev=330150&r1=330149&r2=330150&view=diff == --- cfe/trunk/test/Driver/hexagon-vectorize.c (original) +++ cfe/trunk/test/Driver/hexagon-vectorize.c Mon Apr 16 12:11:17 2018 @@ -1,7 +1,9 @@ // RUN: %clang -target hexagon -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT // RUN: %clang -target hexagon -fvectorize -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-VECTOR // RUN: %clang -target hexagon -fvectorize -fno-vectorize -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-NOVECTOR +// RUN: %clang -target hexagon -fvectorize -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-NEEDHVX // CHECK-DEFAULT-NOT: hexagon-autohvx // CHECK-VECTOR: "-mllvm" "-hexagon-autohvx" // CHECK-NOVECTOR-NOT: hexagon-autohvx +// CHECK-NEEDHVX: warning: auto-vectorization requires HVX, use -mhvx to enable it ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li
[PATCH] D45383: Limit types of builtins that can be redeclared.
erichkeane added a comment. In https://reviews.llvm.org/D45383#1069049, @efriedma wrote: > We can could add an exception to the "don't allow redeclarations with custom > type-checking" rule to specifically allow redeclaring `__va_start`. The > general rule is that we don't want user code redeclaring them because they > don't have a specific signature, so our internal representation could change > between releases. But `__va_start` has a specific fixed signature from the > Microsoft SDK headers, so it should be fine to allow redeclaring it without a > warning. That still doesn't fix the bug, since redeclaring __va_start in 'C' mode when va_list is a reference type causes the assert. https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
efriedma added a comment. I don't see an `A` in `LANGBUILTIN(__va_start, "vc**.", "nt", ALL_MS_LANGUAGES)` https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45444: [clang-tidy] WIP: implement new check for const-correctness
JonasToth added a comment. > You'll also need to check: > > - a member function calling another non-const member function -> *this can't > be const > - *this is passed as non-const reference param to a function -> *this can't > be const Yes. That is similar behaviour to checking if a function can be `noexcept`. But this is all future stuff :) > Also when marking decls as "can't be const" we'll have to do record > separately for modifying behaviors coming from different functions. > Which of course are all possible but code will get too complex than > necessary IMO. I cant follow you on that one. I consider every path that allows future modification (like taking a non-const reference/pointer) as `cant be const`. That would be enough, wouldnt it? A separate function can only modify a variable if it has some form of non-const handle to it, which must have been created at some point. > I think member variables are a separate topic: I think we should just treat > them as globals and not check on them, the same argument that they can be > accessed from multiple translation unit applies to global/namespace scoped > variables and class scoped variables. We can only reliably check > function/block scope variables. > (reliable meaning being able to achieve zero false positives with useful > level of recall, false negatives are inevitable because whenever a modifiable > reference/handle escape the current block/translation unit we'll have to > assume it's modified.) You are probably right. The only zero-false positive case is: "only const methods called". One could split implementation of a class into several translation units and render the analysis approach useless. > Yes my approach is doing multi-pass matching by calling isModified() > recursively. I consider the multi-pass matching "necessary evil" because > otherwise we'll have too many false negatives. > I thought about hasDescendent (hasAncestor actually) but I don't think that > makes things easier: varDeclRef(hasAncestor(modifying)) would match both > "v.a.b.c. = 10" and "map[v] = 10" and we'll still need to double check the > modifying behavior does link back to the particular varDeclRef. Example as reference for later: https://godbolt.org/g/cvhoUN I will add such cases to my tests. @shuaiwang Are you in IRC from time to time? I think it will be better to discuss changes in chat, when i start to introduce your approach here. > isModified(Expr, Stmt), that checks whether Expr is (potentially) modified > within Stmt, is something I believe to be quite useful: What i understand from your comments and code: - implement a utility taking `DeclRefExpr` and check if there is modification in some scope (`Stmt`, e.g. `CompoundStmt`) -> `isModified` or `cantBeModifiedWithin` - match all relevant non-const variable declarations as potential candidates - track all uses of the candidates and check for modification in their scope One Note: We could include variables in unnamed namespaces and `static` declared variables. They have TU scope. My deviations: - a variable should be considered modified if a non-const handle is create from it, even if that handle does not modify its referenced value. As first step, those handles should be marked const, then the original value can be marked as const. That is required to produce compiling code after potential code-transformation. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
erichkeane added a comment. In https://reviews.llvm.org/D45383#1069057, @efriedma wrote: > I don't see an `A` in `LANGBUILTIN(__va_start, "vc**.", "nt", > ALL_MS_LANGUAGES)` Ah, you're right! I was confusing it with LIBBUILTIN(va_start, "vA.", "fn", "stdarg.h", ALL_LANGUAGES) and BUILTIN(__builtin_va_start, "vA.", "nt"). It seems that the MS builtins dont' actually use 'A'. I'll implement your suggestion, thanks for your patience :) https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45463: [AST] Print correct tag decl for tag specifier
jdenny updated this revision to Diff 142673. jdenny added a comment. Rebased onto a more recent master. https://reviews.llvm.org/D45463 Files: include/clang-c/Index.h include/clang/AST/PrettyPrinter.h lib/AST/DeclPrinter.cpp lib/AST/TypePrinter.cpp test/Misc/ast-print-enum-decl.c test/Misc/ast-print-record-decl.c tools/c-index-test/c-index-test.c tools/libclang/CIndex.cpp Index: tools/libclang/CIndex.cpp === --- tools/libclang/CIndex.cpp +++ tools/libclang/CIndex.cpp @@ -4739,8 +4739,6 @@ return P->SuppressSpecifiers; case CXPrintingPolicy_SuppressTagKeyword: return P->SuppressTagKeyword; - case CXPrintingPolicy_IncludeTagDefinition: -return P->IncludeTagDefinition; case CXPrintingPolicy_SuppressScope: return P->SuppressScope; case CXPrintingPolicy_SuppressUnwrittenScope: @@ -4808,9 +4806,6 @@ case CXPrintingPolicy_SuppressTagKeyword: P->SuppressTagKeyword = Value; return; - case CXPrintingPolicy_IncludeTagDefinition: -P->IncludeTagDefinition = Value; -return; case CXPrintingPolicy_SuppressScope: P->SuppressScope = Value; return; Index: tools/c-index-test/c-index-test.c === --- tools/c-index-test/c-index-test.c +++ tools/c-index-test/c-index-test.c @@ -97,8 +97,6 @@ CXPrintingPolicy_SuppressSpecifiers}, {"CINDEXTEST_PRINTINGPOLICY_SUPPRESSTAGKEYWORD", CXPrintingPolicy_SuppressTagKeyword}, - {"CINDEXTEST_PRINTINGPOLICY_INCLUDETAGDEFINITION", - CXPrintingPolicy_IncludeTagDefinition}, {"CINDEXTEST_PRINTINGPOLICY_SUPPRESSSCOPE", CXPrintingPolicy_SuppressScope}, {"CINDEXTEST_PRINTINGPOLICY_SUPPRESSUNWRITTENSCOPE", Index: test/Misc/ast-print-record-decl.c === --- /dev/null +++ test/Misc/ast-print-record-decl.c @@ -0,0 +1,251 @@ +// Check struct: +// +// First check compiling and printing of this file. +// +// RUN: %clang -Xclang -verify -S -emit-llvm -DKW=struct -DBASES= -o - %s \ +// RUN: | FileCheck --check-prefixes=CHECK,LLVM %s +// +// RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES= %s > %t.c +// RUN: FileCheck --check-prefixes=CHECK,PRINT -DKW=struct -DBASES= %s \ +// RUN: --input-file %t.c +// +// Now check compiling and printing of the printed file. +// +// RUN: echo "// expected""-warning@* 10 {{'T' is deprecated}}" >> %t.c +// RUN: echo "// expected""-note@* 10 {{'T' has been explicitly marked deprecated here}}" >> %t.c +// +// RUN: %clang -Xclang -verify -S -emit-llvm -o - %t.c \ +// RUN: | FileCheck --check-prefixes=CHECK,LLVM %s +// +// RUN: %clang_cc1 -verify -ast-print %t.c \ +// RUN: | FileCheck --check-prefixes=CHECK,PRINT -DKW=struct -DBASES= %s + +// Repeat for union: +// +// First check compiling and printing of this file. +// +// RUN: %clang -Xclang -verify -S -emit-llvm -DKW=union -DBASES= -o - %s \ +// RUN: | FileCheck --check-prefixes=CHECK,LLVM %s +// +// RUN: %clang_cc1 -verify -ast-print -DKW=union -DBASES= %s > %t.c +// RUN: FileCheck --check-prefixes=CHECK,PRINT -DKW=union -DBASES= %s \ +// RUN: --input-file %t.c +// +// Now check compiling and printing of the printed file. +// +// RUN: echo "// expected""-warning@* 10 {{'T' is deprecated}}" >> %t.c +// RUN: echo "// expected""-note@* 10 {{'T' has been explicitly marked deprecated here}}" >> %t.c +// +// RUN: %clang -Xclang -verify -S -emit-llvm -o - %t.c \ +// RUN: | FileCheck --check-prefixes=CHECK,LLVM %s +// +// RUN: %clang_cc1 -verify -ast-print %t.c \ +// RUN: | FileCheck --check-prefixes=CHECK,PRINT -DKW=union -DBASES= %s + +// Repeat for C++ (BASES helps ensure we're printing as C++ not as C): +// +// First check compiling and printing of this file. +// +// RUN: %clang -Xclang -verify -S -emit-llvm -DKW=struct -DBASES=' : B' -o - \ +// RUN:-xc++ %s \ +// RUN: | FileCheck --check-prefixes=CHECK,LLVM %s +// +// RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES=' : B' -xc++ %s \ +// RUN: > %t.cpp +// RUN: FileCheck --check-prefixes=CHECK,PRINT,CXX -DKW=struct \ +// RUN: -DBASES=' : B' %s --input-file %t.cpp +// +// Now check compiling and printing of the printed file. +// +// RUN: echo "// expected""-warning@* 10 {{'T' is deprecated}}" >> %t.cpp +// RUN: echo "// expected""-note@* 10 {{'T' has been explicitly marked deprecated here}}" >> %t.cpp +// +// RUN: %clang -Xclang -verify -S -emit-llvm -o - %t.cpp \ +// RUN: | FileCheck --check-prefixes=CHECK,LLVM %s +// +// RUN: %clang_cc1 -verify -ast-print %t.cpp \ +// RUN: | FileCheck --check-prefixes=CHECK,PRINT,CXX -DKW=struct \ +// RUN: -DBASES=' : B' %s + +// END. + +#ifndef KW +# error KW undefined +# define KW struct // help syntax checkers +#endif + +#ifndef BASES +# error BASES undefined +# define BASES //
r330151 - [CodeGen] Fix a crash that occurs when a non-trivial C struct with a
Author: ahatanak Date: Mon Apr 16 12:38:00 2018 New Revision: 330151 URL: http://llvm.org/viewvc/llvm-project?rev=330151&view=rev Log: [CodeGen] Fix a crash that occurs when a non-trivial C struct with a volatile array field is copied. The crash occurs because method 'visitArray' passes a null FieldDecl to method 'visit' and some of the methods called downstream expect a non-null FieldDecl to be passed. rdar://problem/33599681 Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp?rev=330151&r1=330150&r2=330151&view=diff == --- cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp (original) +++ cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp Mon Apr 16 12:38:00 2018 @@ -21,10 +21,11 @@ using namespace clang; using namespace CodeGen; // Return the size of a field in number of bits. -static uint64_t getFieldSize(const FieldDecl *FD, ASTContext &Ctx) { - if (FD->isBitField()) +static uint64_t getFieldSize(const FieldDecl *FD, QualType FT, + ASTContext &Ctx) { + if (FD && FD->isBitField()) return FD->getBitWidthValue(Ctx); - return Ctx.getTypeSize(FD->getType()); + return Ctx.getTypeSize(FT); } namespace { @@ -187,7 +188,7 @@ struct CopyStructVisitor : StructVisitor Ts... Args) { assert(!FT.isVolatileQualified() && "volatile field not expected"); ASTContext &Ctx = asDerived().getContext(); -uint64_t FieldSize = getFieldSize(FD, Ctx); +uint64_t FieldSize = getFieldSize(FD, FT, Ctx); // Ignore zero-sized fields. if (FieldSize == 0) @@ -336,7 +337,7 @@ struct GenBinaryFuncName : CopyStructVis uint64_t OffsetInBits = this->Ctx.toBits(CurStackOffset) + this->getFieldOffsetInBits(FD); this->appendStr("_tv" + llvm::to_string(OffsetInBits) + "w" + -llvm::to_string(getFieldSize(FD, this->Ctx))); +llvm::to_string(getFieldSize(FD, FT, this->Ctx))); } }; @@ -595,16 +596,25 @@ struct GenBinaryFunc : CopyStructVisitor template void visitVolatileTrivial(QualType FT, const FieldDecl *FD, CharUnits Offset, std::array Addrs) { -QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0); -llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo(); -Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset); -LValue DstBase = this->CGF->MakeAddrLValue( -this->CGF->Builder.CreateBitCast(DstAddr, PtrTy), FT); -LValue DstLV = this->CGF->EmitLValueForField(DstBase, FD); -Address SrcAddr = this->getAddrWithOffset(Addrs[SrcIdx], Offset); -LValue SrcBase = this->CGF->MakeAddrLValue( -this->CGF->Builder.CreateBitCast(SrcAddr, PtrTy), FT); -LValue SrcLV = this->CGF->EmitLValueForField(SrcBase, FD); +LValue DstLV, SrcLV; +if (FD) { + QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0); + llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo(); + Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset); + LValue DstBase = this->CGF->MakeAddrLValue( + this->CGF->Builder.CreateBitCast(DstAddr, PtrTy), FT); + DstLV = this->CGF->EmitLValueForField(DstBase, FD); + Address SrcAddr = this->getAddrWithOffset(Addrs[SrcIdx], Offset); + LValue SrcBase = this->CGF->MakeAddrLValue( + this->CGF->Builder.CreateBitCast(SrcAddr, PtrTy), FT); + SrcLV = this->CGF->EmitLValueForField(SrcBase, FD); +} else { + llvm::PointerType *Ty = this->CGF->ConvertType(FT)->getPointerTo(); + Address DstAddr = this->CGF->Builder.CreateBitCast(Addrs[DstIdx], Ty); + Address SrcAddr = this->CGF->Builder.CreateBitCast(Addrs[SrcIdx], Ty); + DstLV = this->CGF->MakeAddrLValue(DstAddr, FT); + SrcLV = this->CGF->MakeAddrLValue(SrcAddr, FT); +} RValue SrcVal = this->CGF->EmitLoadOfLValue(SrcLV, SourceLocation()); this->CGF->EmitStoreThroughLValue(SrcVal, DstLV); } Modified: cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m?rev=330151&r1=330150&r2=330151&view=diff == --- cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m (original) +++ cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m Mon Apr 16 12:38:00 2018 @@ -70,6 +70,11 @@ typedef struct { volatile char i6; } Bitfield1; +typedef struct { + id x; + volatile int a[16]; +} VolatileArray ; + #endif #ifdef USESTRUCT @@ -540,4 +545,17 @@ void test_strong_in_union() { U t; } +// CHECK: define void @test_copy_constructor_VolatileArray( +// CHECK: call void @__copy_constructor_8_8_s0_AB8s4n16_tv64w32_AE
[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.
lebedev.ri added a comment. Uuuh, the fact that phab posts the top-postings, but silently ignores inline replies is annoying. >> lebedev.ri added a comment. >> >> In https://reviews.llvm.org/D45685#1069040, @dblaikie wrote: >> >>> I'm not sure this is a practical direction to pursue - though perhaps >> > others disagree. >> >>> It's likely non-trivial to plumb a flag through most build systems to be >> > applied only to test code >> >> I'm sorry, I don't understand. >> >> If you don't separate between source code and `*_test.cpp` sources, sure, >> you will loose the warning coverage either way. >> >> What difference is there between passing `-wtest` and passing >> `-Wno-self-assign-overloaded`? > > Not much, if any. Ok, so this was a non-argument then? >> Just pass it alongside with the googletest include paths so to speak. > > But build systems don't necessarily expose that kind of ability. (For > example, googletest (not the only kind of test suite/code) is checked into > the LLVM codebase like another library - so depending on it is just another > library dependency, not a special "test" library dependency). It's a bit hard to talk about all and every spherical build system / project in the vacuum, because there is an infinite number of possibilities. Of course some build systems are horribly designed, and it will be hard to do that there. But I sure hope that isn't the case in most of the cases. It might be more productive to talk about a few good known realities. In llvm's case you would simply add `-wtest` (or `-Wno-self-assign-overloaded`) in here https://github.com/llvm-mirror/llvm/blob/2a6cf85828509e89e18739e5f4b9a958820d66d4/cmake/modules/AddLLVM.cmake#L1079-L1085 and around here https://github.com/llvm-mirror/libcxx/blob/73e00f8321b13559b3c41f6656686d980fe92fbe/utils/libcxx/test/config.py#L914 I'd say that is rather trivial. Repository: rC Clang https://reviews.llvm.org/D45685 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r330152 - Use export_as for autolinking frameworks
Author: bruno Date: Mon Apr 16 12:42:32 2018 New Revision: 330152 URL: http://llvm.org/viewvc/llvm-project?rev=330152&view=rev Log: Use export_as for autolinking frameworks framework module SomeKitCore { ... export_as SomeKit } Given the module above, while generting autolink information during codegen, clang should to emit '-framework SomeKitCore' only if SomeKit was not imported in the relevant TU, otherwise it should use '-framework SomeKit' instead. rdar://problem/38269782 Added: cfe/trunk/test/Modules/Inputs/exportas-link/ cfe/trunk/test/Modules/Inputs/exportas-link/OtherKit.framework/ cfe/trunk/test/Modules/Inputs/exportas-link/OtherKit.framework/Headers/ cfe/trunk/test/Modules/Inputs/exportas-link/OtherKit.framework/Headers/OtherKit.h cfe/trunk/test/Modules/Inputs/exportas-link/OtherKit.framework/Modules/ cfe/trunk/test/Modules/Inputs/exportas-link/OtherKit.framework/Modules/module.modulemap cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/ cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/ cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/SKWidget.h cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Headers/SomeKit.h cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Modules/ cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/Modules/module.modulemap cfe/trunk/test/Modules/Inputs/exportas-link/SomeKit.framework/SomeKit.tbd cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/ cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/Headers/ cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/Headers/SKWidget.h cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/Headers/SomeKitCore.h cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/Modules/ cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/Modules/module.modulemap cfe/trunk/test/Modules/Inputs/exportas-link/SomeKitCore.framework/SomeKitCore.tbd cfe/trunk/test/Modules/use-exportas-for-link.m Modified: cfe/trunk/include/clang/Basic/Module.h cfe/trunk/include/clang/Lex/ModuleMap.h cfe/trunk/lib/CodeGen/CodeGenModule.cpp cfe/trunk/lib/Frontend/CompilerInstance.cpp cfe/trunk/lib/Lex/ModuleMap.cpp cfe/trunk/lib/Serialization/ASTReader.cpp Modified: cfe/trunk/include/clang/Basic/Module.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=330152&r1=330151&r2=330152&view=diff == --- cfe/trunk/include/clang/Basic/Module.h (original) +++ cfe/trunk/include/clang/Basic/Module.h Mon Apr 16 12:42:32 2018 @@ -331,6 +331,10 @@ public: /// an entity from this module is used. llvm::SmallVector LinkLibraries; + /// Autolinking uses the framework name for linking purposes + /// when this is false and the export_as name otherwise. + bool UseExportAsModuleLinkName = false; + /// \brief The set of "configuration macros", which are macros that /// (intentionally) change how this module is built. std::vector ConfigMacros; Modified: cfe/trunk/include/clang/Lex/ModuleMap.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=330152&r1=330151&r2=330152&view=diff == --- cfe/trunk/include/clang/Lex/ModuleMap.h (original) +++ cfe/trunk/include/clang/Lex/ModuleMap.h Mon Apr 16 12:42:32 2018 @@ -21,6 +21,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/PointerIntPair.h" +#include "llvm/ADT/StringSet.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringMap.h" @@ -104,7 +105,19 @@ class ModuleMap { /// \brief The number of modules we have created in total. unsigned NumCreatedModules = 0; + /// In case a module has a export_as entry, it might have a pending link + /// name to be determined if that module is imported. + llvm::StringMap> PendingLinkAsModule; + public: + /// Use PendingLinkAsModule information to mark top level link names that + /// are going to be replaced by export_as aliases. + void resolveLinkAsDependencies(Module *Mod); + + /// Make module to use export_as as the link dependency name if enough + /// information is available or add it to a pending list otherwise. + void addLinkAsDependency(Module *Mod); + /// \brief Flags describing the role of a module header. enum ModuleHeaderRole { /// \brief This header is normally included in the module. Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=330152&r1=330151&r2=330152&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenModule
[PATCH] D45665: [builtin-dump-struct] printf formats generation testing
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang https://reviews.llvm.org/D45665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45619: [Time-report] (1) Use special new Clang flag 'FrontendTimesIsEnabled' instead of 'llvm::TimePassesIsEnabled' inside -ftime-report feature
efriedma added inline comments. Comment at: lib/Basic/FrontendTiming.cpp:18 + +bool FrontendTimesIsEnabled = false; + avt77 wrote: > efriedma wrote: > > Why is this in lib/Basic, when the declaration is in > > include/clang/Frontend/? > Because this library is being linked to all others and as result this global > variable could be seen in any Clang library w/o any changes in config files. > Or you mean it should be moved from Frontend? But where? It's a frontend > feature that's why I put its declaration there. The include structure should match the library structure; if the definition needs to be in lib/Basic/, the declaration should be in include/clang/Basic. https://reviews.llvm.org/D45619 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r330153 - Revert "[CodeGen] Fix a crash that occurs when a non-trivial C struct with a"
Author: ahatanak Date: Mon Apr 16 12:53:59 2018 New Revision: 330153 URL: http://llvm.org/viewvc/llvm-project?rev=330153&view=rev Log: Revert "[CodeGen] Fix a crash that occurs when a non-trivial C struct with a" This reverts commit r330151, which caused bots to fail. Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp?rev=330153&r1=330152&r2=330153&view=diff == --- cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp (original) +++ cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp Mon Apr 16 12:53:59 2018 @@ -21,11 +21,10 @@ using namespace clang; using namespace CodeGen; // Return the size of a field in number of bits. -static uint64_t getFieldSize(const FieldDecl *FD, QualType FT, - ASTContext &Ctx) { - if (FD && FD->isBitField()) +static uint64_t getFieldSize(const FieldDecl *FD, ASTContext &Ctx) { + if (FD->isBitField()) return FD->getBitWidthValue(Ctx); - return Ctx.getTypeSize(FT); + return Ctx.getTypeSize(FD->getType()); } namespace { @@ -188,7 +187,7 @@ struct CopyStructVisitor : StructVisitor Ts... Args) { assert(!FT.isVolatileQualified() && "volatile field not expected"); ASTContext &Ctx = asDerived().getContext(); -uint64_t FieldSize = getFieldSize(FD, FT, Ctx); +uint64_t FieldSize = getFieldSize(FD, Ctx); // Ignore zero-sized fields. if (FieldSize == 0) @@ -337,7 +336,7 @@ struct GenBinaryFuncName : CopyStructVis uint64_t OffsetInBits = this->Ctx.toBits(CurStackOffset) + this->getFieldOffsetInBits(FD); this->appendStr("_tv" + llvm::to_string(OffsetInBits) + "w" + -llvm::to_string(getFieldSize(FD, FT, this->Ctx))); +llvm::to_string(getFieldSize(FD, this->Ctx))); } }; @@ -596,25 +595,16 @@ struct GenBinaryFunc : CopyStructVisitor template void visitVolatileTrivial(QualType FT, const FieldDecl *FD, CharUnits Offset, std::array Addrs) { -LValue DstLV, SrcLV; -if (FD) { - QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0); - llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo(); - Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset); - LValue DstBase = this->CGF->MakeAddrLValue( - this->CGF->Builder.CreateBitCast(DstAddr, PtrTy), FT); - DstLV = this->CGF->EmitLValueForField(DstBase, FD); - Address SrcAddr = this->getAddrWithOffset(Addrs[SrcIdx], Offset); - LValue SrcBase = this->CGF->MakeAddrLValue( - this->CGF->Builder.CreateBitCast(SrcAddr, PtrTy), FT); - SrcLV = this->CGF->EmitLValueForField(SrcBase, FD); -} else { - llvm::PointerType *Ty = this->CGF->ConvertType(FT)->getPointerTo(); - Address DstAddr = this->CGF->Builder.CreateBitCast(Addrs[DstIdx], Ty); - Address SrcAddr = this->CGF->Builder.CreateBitCast(Addrs[SrcIdx], Ty); - DstLV = this->CGF->MakeAddrLValue(DstAddr, FT); - SrcLV = this->CGF->MakeAddrLValue(SrcAddr, FT); -} +QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0); +llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo(); +Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset); +LValue DstBase = this->CGF->MakeAddrLValue( +this->CGF->Builder.CreateBitCast(DstAddr, PtrTy), FT); +LValue DstLV = this->CGF->EmitLValueForField(DstBase, FD); +Address SrcAddr = this->getAddrWithOffset(Addrs[SrcIdx], Offset); +LValue SrcBase = this->CGF->MakeAddrLValue( +this->CGF->Builder.CreateBitCast(SrcAddr, PtrTy), FT); +LValue SrcLV = this->CGF->EmitLValueForField(SrcBase, FD); RValue SrcVal = this->CGF->EmitLoadOfLValue(SrcLV, SourceLocation()); this->CGF->EmitStoreThroughLValue(SrcVal, DstLV); } Modified: cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m?rev=330153&r1=330152&r2=330153&view=diff == --- cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m (original) +++ cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m Mon Apr 16 12:53:59 2018 @@ -70,11 +70,6 @@ typedef struct { volatile char i6; } Bitfield1; -typedef struct { - id x; - volatile int a[16]; -} VolatileArray ; - #endif #ifdef USESTRUCT @@ -545,17 +540,4 @@ void test_strong_in_union() { U t; } -// CHECK: define void @test_copy_constructor_VolatileArray( -// CHECK: call void @__copy_constructor_8_8_s0_AB8s4n16_tv64w32_AE( - -// CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_s0_AB8s4n16_tv64w32_AE( -// CHECK: %[[V12:.*]] = bitcast i8** %{{.*}} to i32* -// CHECK: %[
[PATCH] D45383: Limit types of builtins that can be redeclared.
erichkeane updated this revision to Diff 142685. erichkeane added a comment. Added the exception for MSVC's __va_start. __va_start is a normal function on Linux, so it isn't an error on non-windows either. https://reviews.llvm.org/D45383 Files: include/clang/AST/ASTContext.h include/clang/Basic/Builtins.h include/clang/Basic/DiagnosticSemaKinds.td lib/AST/ASTContext.cpp lib/Basic/Builtins.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaOverload.cpp test/Sema/MicrosoftExtensions.c test/Sema/builtin-redecl.cpp test/SemaCXX/microsoft-varargs.cpp Index: lib/AST/ASTContext.cpp === --- lib/AST/ASTContext.cpp +++ lib/AST/ASTContext.cpp @@ -7244,6 +7244,10 @@ return BuiltinMSVaListDecl; } +bool ASTContext::canBuiltinBeRedeclared(const FunctionDecl *FD) const { + return BuiltinInfo.canBeRedeclared(FD->getBuiltinID()); +} + void ASTContext::setObjCConstantStringInterface(ObjCInterfaceDecl *Decl) { assert(ObjCConstantStringType.isNull() && "'NSConstantString' type already set!"); Index: lib/Sema/SemaOverload.cpp === --- lib/Sema/SemaOverload.cpp +++ lib/Sema/SemaOverload.cpp @@ -998,6 +998,13 @@ Match = *I; return Ovl_Match; } + + // Builtins that have custom typechecking or have a reference should + // not be overloadable or redeclarable. + if (!getASTContext().canBuiltinBeRedeclared(OldF)) { +Match = *I; +return Ovl_NonFunction; + } } else if (isa(OldD) || isa(OldD)) { // We can overload with these, which can show up when doing // redeclaration checks for UsingDecls. Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -3011,6 +3011,14 @@ if (Old->isInvalidDecl()) return true; + // Disallow redeclaration of some builtins. + if (!getASTContext().canBuiltinBeRedeclared(Old)) { +Diag(New->getLocation(), diag::err_builtin_redeclare) << Old->getDeclName(); +Diag(Old->getLocation(), diag::note_previous_builtin_declaration) +<< Old << Old->getType(); +return true; + } + diag::kind PrevDiag; SourceLocation OldLocation; std::tie(PrevDiag, OldLocation) = Index: lib/Basic/Builtins.cpp === --- lib/Basic/Builtins.cpp +++ lib/Basic/Builtins.cpp @@ -139,3 +139,10 @@ bool &HasVAListArg) { return isLike(ID, FormatIdx, HasVAListArg, "sS"); } + +bool Builtin::Context::canBeRedeclared(unsigned ID) const { + return ID == Builtin::NotBuiltin || + ID == Builtin::BI__va_start || + (!hasReferenceArgsOrResult(ID) && + !hasCustomTypechecking(ID)); +} Index: include/clang/AST/ASTContext.h === --- include/clang/AST/ASTContext.h +++ include/clang/AST/ASTContext.h @@ -1881,6 +1881,10 @@ return getTypeDeclType(getBuiltinMSVaListDecl()); } + /// Return whether a declaration to a builtin is allowed to be + /// overloaded/redeclared. + bool canBuiltinBeRedeclared(const FunctionDecl *) const; + /// \brief Return a type with additional \c const, \c volatile, or /// \c restrict qualifiers. QualType getCVRQualifiedType(QualType T, unsigned CVR) const { Index: include/clang/Basic/DiagnosticSemaKinds.td === --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -602,6 +602,7 @@ "incompatible redeclaration of library function %0">, InGroup>; def err_builtin_definition : Error<"definition of builtin function %0">; +def err_builtin_redeclare : Error<"cannot redeclare builtin function %0">; def err_arm_invalid_specialreg : Error<"invalid special register for builtin">; def err_invalid_cpu_supports : Error<"invalid cpu feature string for builtin">; def err_invalid_cpu_is : Error<"invalid cpu name for builtin">; Index: include/clang/Basic/Builtins.h === --- include/clang/Basic/Builtins.h +++ include/clang/Basic/Builtins.h @@ -167,6 +167,13 @@ return strchr(getRecord(ID).Type, '*') != nullptr; } + /// \brief Return true if this builtin has a result or any arguments which are + /// reference types. + bool hasReferenceArgsOrResult(unsigned ID) const { +return strchr(getRecord(ID).Type, '&') != nullptr || + strchr(getRecord(ID).Type, 'A') != nullptr; + } + /// \brief Completely forget that the given ID was ever considered a builtin, /// e.g., because the user provided a conflicting signature. void forgetBuiltin(unsigned ID, IdentifierTable &Table); @@ -212,6 +219,10 @@ /// prefix. static bool isBuiltinFunc(const char *Name); + /// Returns true if this
r330154 - [OPENMP] General code improvements.
Author: abataev Date: Mon Apr 16 13:16:21 2018 New Revision: 330154 URL: http://llvm.org/viewvc/llvm-project?rev=330154&view=rev Log: [OPENMP] General code improvements. Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h cfe/trunk/test/OpenMP/nvptx_teams_reduction_codegen.cpp Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=330154&r1=330153&r2=330154&view=diff == --- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original) +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Mon Apr 16 13:16:21 2018 @@ -91,11 +91,11 @@ enum OpenMPRTLFunctionNVPTX { /// Pre(post)-action for different OpenMP constructs specialized for NVPTX. class NVPTXActionTy final : public PrePostActionTy { - llvm::Value *EnterCallee; + llvm::Value *EnterCallee = nullptr; ArrayRef EnterArgs; - llvm::Value *ExitCallee; + llvm::Value *ExitCallee = nullptr; ArrayRef ExitArgs; - bool Conditional; + bool Conditional = false; llvm::BasicBlock *ContBlock = nullptr; public: @@ -179,7 +179,7 @@ class CheckVarsEscapingDeclContext final static llvm::Optional isDeclareTargetDeclaration(const ValueDecl *VD) { -for (const auto *D : VD->redecls()) { +for (const Decl *D : VD->redecls()) { if (!D->hasAttrs()) continue; if (const auto *Attr = D->getAttr()) @@ -233,7 +233,7 @@ class CheckVarsEscapingDeclContext final void VisitOpenMPCapturedStmt(const CapturedStmt *S) { if (!S) return; -for (const auto &C : S->captures()) { +for (const CapturedStmt::Capture &C : S->captures()) { if (C.capturesVariable() && !C.capturesVariableByCopy()) { const ValueDecl *VD = C.getCapturedVar(); markAsEscaped(VD); @@ -255,7 +255,7 @@ class CheckVarsEscapingDeclContext final return; ASTContext &C = CGF.getContext(); SmallVector GlobalizedVars; -for (const auto *D : EscapedDecls) +for (const ValueDecl *D : EscapedDecls) GlobalizedVars.emplace_back(C.getDeclAlign(D), D); std::stable_sort(GlobalizedVars.begin(), GlobalizedVars.end(), stable_sort_comparator); @@ -296,7 +296,7 @@ public: void VisitDeclStmt(const DeclStmt *S) { if (!S) return; -for (const auto *D : S->decls()) +for (const Decl *D : S->decls()) if (const auto *VD = dyn_cast_or_null(D)) VisitValueDecl(VD); } @@ -312,7 +312,7 @@ public: void VisitCapturedStmt(const CapturedStmt *S) { if (!S) return; -for (const auto &C : S->captures()) { +for (const CapturedStmt::Capture &C : S->captures()) { if (C.capturesVariable() && !C.capturesVariableByCopy()) { const ValueDecl *VD = C.getCapturedVar(); markAsEscaped(VD); @@ -324,7 +324,7 @@ public: void VisitLambdaExpr(const LambdaExpr *E) { if (!E) return; -for (const auto &C : E->captures()) { +for (const LambdaCapture &C : E->captures()) { if (C.capturesVariable()) { if (C.getCaptureKind() == LCK_ByRef) { const ValueDecl *VD = C.getCapturedVar(); @@ -338,7 +338,7 @@ public: void VisitBlockExpr(const BlockExpr *E) { if (!E) return; -for (const auto &C : E->getBlockDecl()->captures()) { +for (const BlockDecl::Capture &C : E->getBlockDecl()->captures()) { if (C.isByRef()) { const VarDecl *VD = C.getVariable(); markAsEscaped(VD); @@ -358,8 +358,9 @@ public: AllEscaped = true; Visit(Arg); AllEscaped = SavedAllEscaped; - } else + } else { Visit(Arg); + } } Visit(E->getCallee()); } @@ -383,8 +384,9 @@ public: AllEscaped = true; Visit(E->getSubExpr()); AllEscaped = SavedAllEscaped; -} else +} else { Visit(E->getSubExpr()); +} } void VisitImplicitCastExpr(const ImplicitCastExpr *E) { if (!E) @@ -394,8 +396,9 @@ public: AllEscaped = true; Visit(E->getSubExpr()); AllEscaped = SavedAllEscaped; -} else +} else { Visit(E->getSubExpr()); +} } void VisitExpr(const Expr *E) { if (!E) @@ -403,7 +406,7 @@ public: bool SavedAllEscaped = AllEscaped; if (!E->isLValue()) AllEscaped = false; -for (const auto *Child : E->children()) +for (const Stmt *Child : E->children()) if (Child) Visit(Child); AllEscaped = SavedAllEscaped; @@ -411,7 +414,7 @@ public: void VisitStmt(const Stmt *S) { if (!S) return; -for (const auto *Child : S->children()) +for (const Stmt *Child : S->children()) if (Child) Visit(Child); } @@ -553,19 +556,19 @@ static llvm::Value *getMasterThreadID(Co CGOpenMPRuntimeNVPTX::WorkerFunctionState::WorkerFunctionState( CodeGenModule &CGM, Sour
[PATCH] D45662: Fuzzer, add libcxx for OpenBSD
devnexen updated this revision to Diff 142686. devnexen added a comment. - Putting the change on the driver itself. https://reviews.llvm.org/D45662 Files: lib/Driver/ToolChains/OpenBSD.cpp Index: lib/Driver/ToolChains/OpenBSD.cpp === --- lib/Driver/ToolChains/OpenBSD.cpp +++ lib/Driver/ToolChains/OpenBSD.cpp @@ -194,6 +194,9 @@ } if (NeedsSanitizerDeps) { CmdArgs.push_back(ToolChain.getCompilerRTArgString(Args, "builtins", false)); + if (getToolChain().getSanitizerArgs().needsFuzzer() && +!Args.hasArg(options::OPT_shared)) +CmdArgs.push_back(Args.hasArg(options::OPT_pg) ? "-lc++_p" : "-lc++"); linkSanitizerRuntimeDeps(ToolChain, CmdArgs); } if (NeedsXRayDeps) { Index: lib/Driver/ToolChains/OpenBSD.cpp === --- lib/Driver/ToolChains/OpenBSD.cpp +++ lib/Driver/ToolChains/OpenBSD.cpp @@ -194,6 +194,9 @@ } if (NeedsSanitizerDeps) { CmdArgs.push_back(ToolChain.getCompilerRTArgString(Args, "builtins", false)); + if (getToolChain().getSanitizerArgs().needsFuzzer() && +!Args.hasArg(options::OPT_shared)) +CmdArgs.push_back(Args.hasArg(options::OPT_pg) ? "-lc++_p" : "-lc++"); linkSanitizerRuntimeDeps(ToolChain, CmdArgs); } if (NeedsXRayDeps) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r330155 - [CodeGen] Fix a crash that occurs when a non-trivial C struct with a
Author: ahatanak Date: Mon Apr 16 13:23:52 2018 New Revision: 330155 URL: http://llvm.org/viewvc/llvm-project?rev=330155&view=rev Log: [CodeGen] Fix a crash that occurs when a non-trivial C struct with a volatile array field is copied. The crash occurs because method 'visitArray' passes a null FieldDecl to method 'visit' and some of the methods called downstream expect a non-null FieldDecl to be passed. This reapplies r330151 with a fix to the test case. rdar://problem/33599681 Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp?rev=330155&r1=330154&r2=330155&view=diff == --- cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp (original) +++ cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp Mon Apr 16 13:23:52 2018 @@ -21,10 +21,11 @@ using namespace clang; using namespace CodeGen; // Return the size of a field in number of bits. -static uint64_t getFieldSize(const FieldDecl *FD, ASTContext &Ctx) { - if (FD->isBitField()) +static uint64_t getFieldSize(const FieldDecl *FD, QualType FT, + ASTContext &Ctx) { + if (FD && FD->isBitField()) return FD->getBitWidthValue(Ctx); - return Ctx.getTypeSize(FD->getType()); + return Ctx.getTypeSize(FT); } namespace { @@ -187,7 +188,7 @@ struct CopyStructVisitor : StructVisitor Ts... Args) { assert(!FT.isVolatileQualified() && "volatile field not expected"); ASTContext &Ctx = asDerived().getContext(); -uint64_t FieldSize = getFieldSize(FD, Ctx); +uint64_t FieldSize = getFieldSize(FD, FT, Ctx); // Ignore zero-sized fields. if (FieldSize == 0) @@ -336,7 +337,7 @@ struct GenBinaryFuncName : CopyStructVis uint64_t OffsetInBits = this->Ctx.toBits(CurStackOffset) + this->getFieldOffsetInBits(FD); this->appendStr("_tv" + llvm::to_string(OffsetInBits) + "w" + -llvm::to_string(getFieldSize(FD, this->Ctx))); +llvm::to_string(getFieldSize(FD, FT, this->Ctx))); } }; @@ -595,16 +596,25 @@ struct GenBinaryFunc : CopyStructVisitor template void visitVolatileTrivial(QualType FT, const FieldDecl *FD, CharUnits Offset, std::array Addrs) { -QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0); -llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo(); -Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset); -LValue DstBase = this->CGF->MakeAddrLValue( -this->CGF->Builder.CreateBitCast(DstAddr, PtrTy), FT); -LValue DstLV = this->CGF->EmitLValueForField(DstBase, FD); -Address SrcAddr = this->getAddrWithOffset(Addrs[SrcIdx], Offset); -LValue SrcBase = this->CGF->MakeAddrLValue( -this->CGF->Builder.CreateBitCast(SrcAddr, PtrTy), FT); -LValue SrcLV = this->CGF->EmitLValueForField(SrcBase, FD); +LValue DstLV, SrcLV; +if (FD) { + QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0); + llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo(); + Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset); + LValue DstBase = this->CGF->MakeAddrLValue( + this->CGF->Builder.CreateBitCast(DstAddr, PtrTy), FT); + DstLV = this->CGF->EmitLValueForField(DstBase, FD); + Address SrcAddr = this->getAddrWithOffset(Addrs[SrcIdx], Offset); + LValue SrcBase = this->CGF->MakeAddrLValue( + this->CGF->Builder.CreateBitCast(SrcAddr, PtrTy), FT); + SrcLV = this->CGF->EmitLValueForField(SrcBase, FD); +} else { + llvm::PointerType *Ty = this->CGF->ConvertType(FT)->getPointerTo(); + Address DstAddr = this->CGF->Builder.CreateBitCast(Addrs[DstIdx], Ty); + Address SrcAddr = this->CGF->Builder.CreateBitCast(Addrs[SrcIdx], Ty); + DstLV = this->CGF->MakeAddrLValue(DstAddr, FT); + SrcLV = this->CGF->MakeAddrLValue(SrcAddr, FT); +} RValue SrcVal = this->CGF->EmitLoadOfLValue(SrcLV, SourceLocation()); this->CGF->EmitStoreThroughLValue(SrcVal, DstLV); } Modified: cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m?rev=330155&r1=330154&r2=330155&view=diff == --- cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m (original) +++ cfe/trunk/test/CodeGenObjC/strong-in-c-struct.m Mon Apr 16 13:23:52 2018 @@ -70,6 +70,11 @@ typedef struct { volatile char i6; } Bitfield1; +typedef struct { + id x; + volatile int a[16]; +} VolatileArray ; + #endif #ifdef USESTRUCT @@ -540,4 +545,19 @@ void test_strong_in_union() { U t; } +// CHECK: define void @test_copy_constructor_VolatileArray( +// CHECK: cal
[PATCH] D45465: [AST] Fix printing tag decl groups in decl contexts
jdenny updated this revision to Diff 142687. jdenny added a comment. Rebased onto a more recent master. https://reviews.llvm.org/D45465 Files: lib/AST/DeclPrinter.cpp test/Misc/ast-print-enum-decl.c test/Misc/ast-print-record-decl.c test/Sema/ast-print.c test/SemaCXX/ast-print.cpp Index: test/SemaCXX/ast-print.cpp === --- test/SemaCXX/ast-print.cpp +++ test/SemaCXX/ast-print.cpp @@ -214,10 +214,13 @@ struct [[gnu::visibility("hidden")]] S; } -// CHECK: struct CXXFunctionalCastExprPrint fce = CXXFunctionalCastExprPrint{}; +// CHECK: struct CXXFunctionalCastExprPrint { +// CHECK-NEXT: } fce = CXXFunctionalCastExprPrint{}; struct CXXFunctionalCastExprPrint {} fce = CXXFunctionalCastExprPrint{}; -// CHECK: struct CXXTemporaryObjectExprPrint toe = CXXTemporaryObjectExprPrint{}; +// CHECK: struct CXXTemporaryObjectExprPrint { +// CHECK-NEXT: CXXTemporaryObjectExprPrint(); +// CHECK-NEXT: } toe = CXXTemporaryObjectExprPrint{}; struct CXXTemporaryObjectExprPrint { CXXTemporaryObjectExprPrint(); } toe = CXXTemporaryObjectExprPrint{}; namespace PR24872 { Index: test/Sema/ast-print.c === --- test/Sema/ast-print.c +++ test/Sema/ast-print.c @@ -83,8 +83,7 @@ EnumWithAttributesFoo __attribute__((deprecated)), // CHECK-NEXT: EnumWithAttributesBar __attribute__((unavailable(""))) = 50 EnumWithAttributesBar __attribute__((unavailable)) = 50 - // CHECK-NEXT: }; - // CHECK-NEXT: enum EnumWithAttributes *EnumWithAttributesPtr; + // CHECK-NEXT: } *EnumWithAttributesPtr; } __attribute__((deprecated)) *EnumWithAttributesPtr; // expected-note {{'EnumWithAttributes' has been explicitly marked deprecated here}} // FIXME: If enum is forward-declared at file scope, attributes are lost. Index: test/Misc/ast-print-record-decl.c === --- test/Misc/ast-print-record-decl.c +++ test/Misc/ast-print-record-decl.c @@ -6,8 +6,8 @@ // RUN: | FileCheck --check-prefixes=CHECK,LLVM %s // // RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES= %s > %t.c -// RUN: FileCheck --check-prefixes=CHECK,PRINT -DKW=struct -DBASES= %s \ -// RUN: --input-file %t.c +// RUN: FileCheck --check-prefixes=CHECK,PRINT,PRINT-C -DKW=struct -DBASES= \ +// RUN: %s --input-file %t.c // // Now check compiling and printing of the printed file. // @@ -18,7 +18,8 @@ // RUN: | FileCheck --check-prefixes=CHECK,LLVM %s // // RUN: %clang_cc1 -verify -ast-print %t.c \ -// RUN: | FileCheck --check-prefixes=CHECK,PRINT -DKW=struct -DBASES= %s +// RUN: | FileCheck --check-prefixes=CHECK,PRINT,PRINT-C -DKW=struct \ +// RUN: -DBASES= %s // Repeat for union: // @@ -28,8 +29,8 @@ // RUN: | FileCheck --check-prefixes=CHECK,LLVM %s // // RUN: %clang_cc1 -verify -ast-print -DKW=union -DBASES= %s > %t.c -// RUN: FileCheck --check-prefixes=CHECK,PRINT -DKW=union -DBASES= %s \ -// RUN: --input-file %t.c +// RUN: FileCheck --check-prefixes=CHECK,PRINT,PRINT-C -DKW=union -DBASES= \ +// RUN: %s --input-file %t.c // // Now check compiling and printing of the printed file. // @@ -40,7 +41,8 @@ // RUN: | FileCheck --check-prefixes=CHECK,LLVM %s // // RUN: %clang_cc1 -verify -ast-print %t.c \ -// RUN: | FileCheck --check-prefixes=CHECK,PRINT -DKW=union -DBASES= %s +// RUN: | FileCheck --check-prefixes=CHECK,PRINT,PRINT-C -DKW=union \ +// RUN: -DBASES= %s // Repeat for C++ (BASES helps ensure we're printing as C++ not as C): // @@ -52,7 +54,7 @@ // // RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES=' : B' -xc++ %s \ // RUN: > %t.cpp -// RUN: FileCheck --check-prefixes=CHECK,PRINT,CXX -DKW=struct \ +// RUN: FileCheck --check-prefixes=CHECK,PRINT,PRINT-CXX -DKW=struct \ // RUN: -DBASES=' : B' %s --input-file %t.cpp // // Now check compiling and printing of the printed file. @@ -64,7 +66,7 @@ // RUN: | FileCheck --check-prefixes=CHECK,LLVM %s // // RUN: %clang_cc1 -verify -ast-print %t.cpp \ -// RUN: | FileCheck --check-prefixes=CHECK,PRINT,CXX -DKW=struct \ +// RUN: | FileCheck --check-prefixes=CHECK,PRINT,PRINT-CXX -DKW=struct \ // RUN: -DBASES=' : B' %s // END. @@ -161,29 +163,41 @@ // expected-note@+1 2 {{'T' has been explicitly marked deprecated here}} KW __attribute__((deprecated(""))) T *p0; - // PRINT-NEXT: [[KW]] - // PRINT-DAG: __attribute__((deprecated(""))) - // PRINT-DAG: __attribute__((aligned(16))) - // PRINT-NOT: __attribute__ - // PRINT-SAME: T[[BASES]] { - // PRINT-NEXT: int i; - // PRINT-NEXT: [[KW]] T *p2; - // PRINT-NEXT: } *p1; - KW __attribute__((aligned(16))) T BASES { // expected-warning {{'T' is deprecated}} + // PRINT-NEXT: [[KW]] + // PRINT-DAG: __attribute__((deprecated(""))) + // PRINT-DAG: __attribute
r330156 - [OPENMP] Allow to use declare target variables in map clauses
Author: abataev Date: Mon Apr 16 13:34:41 2018 New Revision: 330156 URL: http://llvm.org/viewvc/llvm-project?rev=330156&view=rev Log: [OPENMP] Allow to use declare target variables in map clauses Global variables marked as declare target are allowed to be used in map clauses. Patch fixes the crash of the compiler on the declare target variables in map clauses. Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp cfe/trunk/test/OpenMP/declare_target_codegen.cpp Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=330156&r1=330155&r2=330156&view=diff == --- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original) +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Mon Apr 16 13:34:41 2018 @@ -6799,16 +6799,13 @@ public: if (const auto *VD = dyn_cast_or_null(I->getAssociatedDeclaration())) { if (llvm::Optional Res = -isDeclareTargetDeclaration(VD)) { - assert(*Res == OMPDeclareTargetDeclAttr::MT_Link && - "Declare target link is expected."); - // Avoid warning in release build. - (void)*Res; - IsLink = true; - BP = CGF.CGM.getOpenMPRuntime() - .getAddrOfDeclareTargetLink(VD) - .getPointer(); -} +isDeclareTargetDeclaration(VD)) + if (*Res == OMPDeclareTargetDeclAttr::MT_Link) { +IsLink = true; +BP = CGF.CGM.getOpenMPRuntime() + .getAddrOfDeclareTargetLink(VD) + .getPointer(); + } } // If the variable is a pointer and is being dereferenced (i.e. is not Modified: cfe/trunk/test/OpenMP/declare_target_codegen.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_target_codegen.cpp?rev=330156&r1=330155&r2=330156&view=diff == --- cfe/trunk/test/OpenMP/declare_target_codegen.cpp (original) +++ cfe/trunk/test/OpenMP/declare_target_codegen.cpp Mon Apr 16 13:34:41 2018 @@ -45,7 +45,7 @@ int maini1() { static long aa = 32; // CHECK-DAG: define void @__omp_offloading_{{.*}}maini1{{.*}}_l[[@LINE+1]](i32* dereferenceable{{.*}}, i64 {{.*}}, i64 {{.*}}) #pragma omp target map(tofrom \ - : a) + : a, b) { static long aaa = 23; a = foo() + bar() + b + c + d + aa + aaa; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45698: [analyzer] When we fail to evaluate a pointer cast, escape the pointer.
NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. If a pointer cast fails (evaluates to an `UnknownVal`) and such cast is the last use of the pointer, the pointer is no longer referenced by the program state and a leak is (mis-)diagnosed. Produce pointer escape (but not invalidation) when the cast fails to avoid such false positives. Repository: rC Clang https://reviews.llvm.org/D45698 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/ExprEngineC.cpp test/Analysis/malloc.mm test/Analysis/pr22954.c Index: test/Analysis/pr22954.c === --- test/Analysis/pr22954.c +++ test/Analysis/pr22954.c @@ -624,9 +624,10 @@ clang_analyzer_eval(m29[i].s3[1] == 1); // expected-warning{{UNKNOWN}} clang_analyzer_eval(m29[i].s3[2] == 1); // expected-warning{{UNKNOWN}} clang_analyzer_eval(m29[i].s3[3] == 1); // expected-warning{{UNKNOWN}} - clang_analyzer_eval(m29[j].s3[k] == 1); // expected-warning{{TRUE}}\ - expected-warning{{Potential leak of memory pointed to by field 's4'}} + clang_analyzer_eval(m29[j].s3[k] == 1); // expected-warning{{TRUE}} clang_analyzer_eval(l29->s1[m] == 2); // expected-warning{{UNKNOWN}} + // FIXME: Should warn that m29[i].s4 leaks. But not on the previous line, + // because l29 and m29 alias. return 0; } Index: test/Analysis/malloc.mm === --- test/Analysis/malloc.mm +++ test/Analysis/malloc.mm @@ -320,3 +320,13 @@ NSString *string = [[NSString alloc] initWithCharactersNoCopy:characters length:12 freeWhenDone:1]; if (string) free(characters); // expected-warning{{Attempt to free non-owned memory}} } + +void *test_reinterpret_cast_to_block() { + // Used to leak because the pointer was disappearing + // during the reinterpret_cast. + using BlockPtrTy = void (^)(); + struct Block {}; + Block* block = static_cast(malloc(sizeof(Block))); + BlockPtrTy blockPtr = reinterpret_cast(block); // no-warning + return blockPtr; +} Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp === --- lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -258,12 +258,15 @@ QualType T, QualType ExTy, const CastExpr* CastE, StmtNodeBuilder& Bldr, ExplodedNode* Pred) { // Delegate to SValBuilder to process. - SVal V = state->getSVal(Ex, LCtx); - V = svalBuilder.evalCast(V, T, ExTy); + SVal OrigV = state->getSVal(Ex, LCtx); + SVal V = svalBuilder.evalCast(OrigV, T, ExTy); // Negate the result if we're treating the boolean as a signed i1 if (CastE->getCastKind() == CK_BooleanToSignedIntegral) V = evalMinus(V); state = state->BindExpr(CastE, LCtx, V); + if (V.isUnknown() && !OrigV.isUnknown()) { +state = escapeValue(state, OrigV, PSK_EscapeOther); + } Bldr.generateNode(CastE, Pred, state); return state; Index: lib/StaticAnalyzer/Core/ExprEngine.cpp === --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1231,23 +1231,27 @@ } } -namespace { +ProgramStateRef ExprEngine::escapeValue(ProgramStateRef State, SVal V, +PointerEscapeKind K) const { + class CollectReachableSymbolsCallback final : public SymbolVisitor { +InvalidatedSymbols Symbols; -class CollectReachableSymbolsCallback final : public SymbolVisitor { - InvalidatedSymbols Symbols; + public: +explicit CollectReachableSymbolsCallback(ProgramStateRef State) {} -public: - explicit CollectReachableSymbolsCallback(ProgramStateRef State) {} +const InvalidatedSymbols &getSymbols() const { return Symbols; } - const InvalidatedSymbols &getSymbols() const { return Symbols; } - - bool VisitSymbol(SymbolRef Sym) override { -Symbols.insert(Sym); -return true; - } -}; +bool VisitSymbol(SymbolRef Sym) override { + Symbols.insert(Sym); + return true; +} + }; -} // namespace + const CollectReachableSymbolsCallback &Scanner = + State->scanReachableSymbols(V); + return getCheckerManager().runCheckersForPointerEscape( + State, Scanner.getSymbols(), /*CallEvent*/ nullptr, K, nullptr); +} void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, ExplodedNodeSet &DstTop) { @@ -1529,17 +1533,8 @@ ->getType()->isRecordType())) for (auto Child : Ex->children()) { assert(Child); - SVal Val = State->getSVal(Child, LCtx); - -CollectReachableSymbolsCallback Scanner = -State->scanReachableSymbols( -Val); -const InvalidatedSymbols &EscapedSy
[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds
pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D45217 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45310: Warn about memcpy'ing non-trivial C structs
ahatanak updated this revision to Diff 142691. ahatanak marked an inline comment as done. ahatanak added a comment. Move method visitArray to the derived classes and pass the array type to the method instead of the QualType. Repository: rC Clang https://reviews.llvm.org/D45310 Files: include/clang/AST/NonTrivialTypeVisitor.h include/clang/Basic/DiagnosticSemaKinds.td lib/CodeGen/CGNonTrivialStruct.cpp lib/Sema/SemaChecking.cpp test/SemaObjC/warn-nontrivial-struct-memaccess.m Index: test/SemaObjC/warn-nontrivial-struct-memaccess.m === --- /dev/null +++ test/SemaObjC/warn-nontrivial-struct-memaccess.m @@ -0,0 +1,39 @@ +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-runtime-has-weak -x objective-c -fobjc-arc -verify %s + +void *memset(void *, int, __SIZE_TYPE__); +void bzero(void *, __SIZE_TYPE__); +void *memcpy(void *, const void *, __SIZE_TYPE__); +void *memmove(void *, const void *, __SIZE_TYPE__); + +struct Trivial { + int f0; + volatile int f1; +}; + +struct NonTrivial0 { + int f0; + __weak id f1; // expected-note 2 {{non-trivial to default-initialize}} expected-note 2 {{non-trivial to copy}} + volatile int f2; + id f3[10]; // expected-note 2 {{non-trivial to default-initialize}} expected-note 2 {{non-trivial to copy}} +}; + +struct NonTrivial1 { + id f0; // expected-note 2 {{non-trivial to default-initialize}} expected-note 2 {{non-trivial to copy}} + int f1; + struct NonTrivial0 f2; +}; + +void testTrivial(struct Trivial *d, struct Trivial *s) { + memset(d, 0, sizeof(struct Trivial)); + bzero(d, sizeof(struct Trivial)); + memcpy(d, s, sizeof(struct Trivial)); + memmove(d, s, sizeof(struct Trivial)); +} + +void testNonTrivial1(struct NonTrivial1 *d, struct NonTrivial1 *s) { + memset(d, 0, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-default-initialize}} expected-note {{explicitly cast the pointer to silence}} + memset((void *)d, 0, sizeof(struct NonTrivial1)); + bzero(d, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-default-initialize}} expected-note {{explicitly cast the pointer to silence}} + memcpy(d, s, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-copy}} expected-note {{explicitly cast the pointer to silence}} + memmove(d, s, sizeof(struct NonTrivial1)); // expected-warning {{that is not trivial to primitive-copy}} expected-note {{explicitly cast the pointer to silence}} +} Index: lib/Sema/SemaChecking.cpp === --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -28,6 +28,7 @@ #include "clang/AST/ExprObjC.h" #include "clang/AST/ExprOpenMP.h" #include "clang/AST/NSAPI.h" +#include "clang/AST/NonTrivialTypeVisitor.h" #include "clang/AST/OperationKinds.h" #include "clang/AST/Stmt.h" #include "clang/AST/TemplateBase.h" @@ -7378,6 +7379,98 @@ return QualType(); } +namespace { + +struct SearchNonTrivialToInitializeField +: DefaultInitializedTypeVisitor { + using Super = + DefaultInitializedTypeVisitor; + + SearchNonTrivialToInitializeField(const Expr *E, Sema &S) : E(E), S(S) {} + + void visitWithKind(QualType::PrimitiveDefaultInitializeKind PDIK, QualType FT, + SourceLocation SL) { +if (const auto *AT = asDerived().getContext().getAsArrayType(FT)) { + asDerived().visitArray(PDIK, AT, SL); + return; +} + +Super::visitWithKind(PDIK, FT, SL); + } + + void visitARCStrong(QualType FT, SourceLocation SL) { +S.DiagRuntimeBehavior(SL, E, S.PDiag(diag::note_nontrivial_field) << 1); + } + void visitARCWeak(QualType FT, SourceLocation SL) { +S.DiagRuntimeBehavior(SL, E, S.PDiag(diag::note_nontrivial_field) << 1); + } + void visitStruct(QualType FT, SourceLocation SL) { +for (const FieldDecl *FD : FT->castAs()->getDecl()->fields()) + visit(FD->getType(), FD->getLocation()); + } + void visitArray(QualType::PrimitiveDefaultInitializeKind PDIK, + const ArrayType *AT, SourceLocation SL) { +visit(getContext().getBaseElementType(AT), SL); + } + void visitTrivial(QualType FT, SourceLocation SL) {} + + static void diag(QualType RT, const Expr *E, Sema &S) { +SearchNonTrivialToInitializeField(E, S).visitStruct(RT, SourceLocation()); + } + + ASTContext &getContext() { return S.getASTContext(); } + + const Expr *E; + Sema &S; +}; + +struct SearchNonTrivialToCopyField +: CopiedTypeVisitor { + using Super = CopiedTypeVisitor; + + SearchNonTrivialToCopyField(const Expr *E, Sema &S) : E(E), S(S) {} + + void visitWithKind(QualType::PrimitiveCopyKind PCK, QualType FT, + SourceLocation SL) { +if (const auto *AT = asDerived().getContext().getAsArrayType(FT)) { + asDerived().visitArray(PCK, AT, SL); + return; +} + +Super::visitWithKind(PCK, FT, SL); + } + + void visitARC
[PATCH] D45310: Warn about memcpy'ing non-trivial C structs
ahatanak added inline comments. Comment at: include/clang/AST/NonTrivialCStructTypeVisitor.h:30 +if (asDerived().getContext().getAsArrayType(FT)) + return asDerived().visitArray(DK, FT, std::forward(Args)...); + rjmccall wrote: > ahatanak wrote: > > rjmccall wrote: > > > Should you have this pass the array type down? And is it really > > > important to do this in the generic visitor? It seems like something you > > > could do in an IRGen subclass. > > The subclasses in CGNonTrivialStruct.cpp need the size and the element type > > of the array to be passed to visitArray, so I think we have to pass the > > array type to visitArray. I guess it's possible to move this to the > > subclasses, but then the visit methods in the subclasses have to check > > whether the type is an array or not. I think we had a discussion on how > > arrays should be handled in this review: https://reviews.llvm.org/D41228. > > > > But perhaps you have a better idea in mind? > Well, you could "override" the visit method in the subclass, e.g.: > > template > class CGDestructedTypeVisitor : public DestructedTypeVisitor RetTy> { > using super = DestructedTypeVisitor; > > public: > using super::asDerived; > using super::visit; > > template > RetTy visit(QualType::DestructionKind DK, QualType FT, Ts &&... Args) { > if (asDerived().getContext().getAsArrayType(FT)) > return asDerived().visitArray(DK, FT, std::forward(Args)...); > > return super::visit(DK, FT, std::forward(Args)...); > } > }; > > It's a bit more boilerplate, but I really feel like the array logic doesn't > belong in the generic visitor. > > About the array type: I wasn't trying to suggest that you should pass the > element type to visitArray, I was suggesting you could just pass the array > type as an `ArrayType*`, since that's what `visitArray` actually wants. I incorporated both of your suggestions. I renamed the overloaded method 'visit' to 'visitWithKind' so that the 'visit' method that doesn't take the DestructionKind doesn't get hidden by the one that takes DestructionKind and is defined in the derived classes. There are a couple of places in CGNonTrivialStruct.cpp that call the former method. Repository: rC Clang https://reviews.llvm.org/D45310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45698: [analyzer] When we fail to evaluate a pointer cast, escape the pointer.
xazax.hun added a comment. I am in favour of this is what. This is what I suggested back than in https://reviews.llvm.org/D23014 but it was somehowoverlooked. Repository: rC Clang https://reviews.llvm.org/D45698 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45699: [Availability] Improve availability to consider functions run at load time
steven_wu created this revision. steven_wu added a reviewer: arphaman. There are some functions/methods that run when the application launches or the library loads. Those functions will run reguardless the OS version as long as it satifies the minimum deployment target. Annotate them with availability attributes doesn't really make sense because they are essentially available on all targets since minimum deployment target. rdar://problem/36093384 Repository: rC Clang https://reviews.llvm.org/D45699 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclAttr.cpp lib/Sema/SemaDeclObjC.cpp test/SemaObjC/unguarded-availability.m Index: test/SemaObjC/unguarded-availability.m === --- test/SemaObjC/unguarded-availability.m +++ test/SemaObjC/unguarded-availability.m @@ -7,7 +7,7 @@ typedef int AVAILABLE_10_12 new_int; // expected-note + {{marked partial here}} -int func_10_11() AVAILABLE_10_11; // expected-note 4 {{'func_10_11' has been explicitly marked partial here}} +int func_10_11() AVAILABLE_10_11; // expected-note 8 {{'func_10_11' has been explicitly marked partial here}} #ifdef OBJCPP // expected-note@+2 6 {{marked partial here}} @@ -311,3 +311,41 @@ (void)AK_Cat; // no warning (void)AK_CyborgCat; // expected-warning{{'AK_CyborgCat' is only available on macOS 10.12 or newer}} expected-note {{@available}} } + + +// test static initializers has the same availability as the deployment target and it cannot be overwritten. +@interface HasStaticInitializer : BaseClass ++ (void)load AVAILABLE_10_11; // expected-warning{{ignoring availability attribute on '+load' method}} +@end + +@implementation HasStaticInitializer ++ (void)load { + func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}} +} +@end + +// test availability from interface is ignored when checking the unguarded availability in +load method. +AVAILABLE_10_11 +@interface HasStaticInitializer1 : BaseClass ++ (void)load; +@end + +@implementation HasStaticInitializer1 ++ (void)load { + func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}} +} +@end + +__attribute__((constructor)) +void is_constructor(); + +AVAILABLE_10_11 // expected-warning{{ignoring availability attribute with constructor attribute}} +void is_constructor() { + func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}} +} + +AVAILABLE_10_11 // expected-warning{{ignoring availability attribute with desctructor attribute}} +__attribute__((destructor)) +void is_destructor() { + func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}} +} Index: lib/Sema/SemaDeclObjC.cpp === --- lib/Sema/SemaDeclObjC.cpp +++ lib/Sema/SemaDeclObjC.cpp @@ -4734,6 +4734,20 @@ Context.getTargetInfo().getTriple().getArch() == llvm::Triple::x86) checkObjCMethodX86VectorTypes(*this, ObjCMethod); + // + load method cannot have availability attributes. It has to be the same + // as deployment target. + for (const auto& attr: ObjCMethod->attrs()) { +if (!isa(attr)) + continue; + +if (ObjCMethod->isClassMethod() && +ObjCMethod->getNameAsString() == "load") { + Diag(attr->getLocation(), diag::warn_availability_on_static_initializer) + << 0; + ObjCMethod->dropAttr(); +} + } + ActOnDocumentableDecl(ObjCMethod); return ObjCMethod; Index: lib/Sema/SemaDeclAttr.cpp === --- lib/Sema/SemaDeclAttr.cpp +++ lib/Sema/SemaDeclAttr.cpp @@ -6823,6 +6823,11 @@ return false; // An implementation implicitly has the availability of the interface. +// Unless it is "+load" method. +if (const auto *MethodD = dyn_cast(Ctx)) + if (MethodD->isClassMethod() && MethodD->getNameAsString() == "load") +return true; + if (const auto *CatOrImpl = dyn_cast(Ctx)) { if (const ObjCInterfaceDecl *Interface = CatOrImpl->getClassInterface()) if (CheckContext(Interface)) Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -9131,6 +9131,25 @@ AddToScope = false; } + // Diagnose availability attributes. Availability cannot be used on functions + // that are run during load/unload. + for (const auto& attr: NewFD->attrs()) { +if (!isa(attr)) + continue; + +if (NewFD->hasAttr()) { +
r330159 - Defer adding keywords to the identifier table until after the language options have been loaded from the AST file.
Author: aaronballman Date: Mon Apr 16 14:07:08 2018 New Revision: 330159 URL: http://llvm.org/viewvc/llvm-project?rev=330159&view=rev Log: Defer adding keywords to the identifier table until after the language options have been loaded from the AST file. This fixes issues with "class" being reported as an identifier in "enum class" because the construct is not present when using default language options. Patch by Johann Klähn. Modified: cfe/trunk/include/clang/Basic/IdentifierTable.h cfe/trunk/lib/Basic/IdentifierTable.cpp cfe/trunk/lib/Lex/Preprocessor.cpp cfe/trunk/unittests/libclang/LibclangTest.cpp Modified: cfe/trunk/include/clang/Basic/IdentifierTable.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/IdentifierTable.h?rev=330159&r1=330158&r2=330159&view=diff == --- cfe/trunk/include/clang/Basic/IdentifierTable.h (original) +++ cfe/trunk/include/clang/Basic/IdentifierTable.h Mon Apr 16 14:07:08 2018 @@ -467,10 +467,13 @@ class IdentifierTable { IdentifierInfoLookup* ExternalLookup; public: + /// \brief Create the identifier table. + explicit IdentifierTable(IdentifierInfoLookup *ExternalLookup = nullptr); + /// \brief Create the identifier table, populating it with info about the /// language keywords for the language specified by \p LangOpts. - IdentifierTable(const LangOptions &LangOpts, - IdentifierInfoLookup* externalLookup = nullptr); + explicit IdentifierTable(const LangOptions &LangOpts, + IdentifierInfoLookup *ExternalLookup = nullptr); /// \brief Set the external identifier lookup mechanism. void setExternalIdentifierLookup(IdentifierInfoLookup *IILookup) { @@ -558,6 +561,8 @@ public: /// hashing is doing. void PrintStats() const; + /// \brief Populate the identifier table with info about the language keywords + /// for the language specified by \p LangOpts. void AddKeywords(const LangOptions &LangOpts); }; Modified: cfe/trunk/lib/Basic/IdentifierTable.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/IdentifierTable.cpp?rev=330159&r1=330158&r2=330159&view=diff == --- cfe/trunk/lib/Basic/IdentifierTable.cpp (original) +++ cfe/trunk/lib/Basic/IdentifierTable.cpp Mon Apr 16 14:07:08 2018 @@ -79,16 +79,16 @@ IdentifierIterator *IdentifierInfoLookup return new EmptyLookupIterator(); } +IdentifierTable::IdentifierTable(IdentifierInfoLookup *ExternalLookup) +: HashTable(8192), // Start with space for 8K identifiers. + ExternalLookup(ExternalLookup) {} + IdentifierTable::IdentifierTable(const LangOptions &LangOpts, - IdentifierInfoLookup* externalLookup) - : HashTable(8192), // Start with space for 8K identifiers. -ExternalLookup(externalLookup) { + IdentifierInfoLookup *ExternalLookup) +: IdentifierTable(ExternalLookup) { // Populate the identifier table with info about keywords for the current // language. AddKeywords(LangOpts); - - // Add the '_experimental_modules_import' contextual keyword. - get("import").setModulesImport(true); } //===--===// @@ -237,6 +237,9 @@ void IdentifierTable::AddKeywords(const if (LangOpts.DeclSpecKeyword) AddKeyword("__declspec", tok::kw___declspec, KEYALL, LangOpts, *this); + + // Add the '_experimental_modules_import' contextual keyword. + get("import").setModulesImport(true); } /// \brief Checks if the specified token kind represents a keyword in the Modified: cfe/trunk/lib/Lex/Preprocessor.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Preprocessor.cpp?rev=330159&r1=330158&r2=330159&view=diff == --- cfe/trunk/lib/Lex/Preprocessor.cpp (original) +++ cfe/trunk/lib/Lex/Preprocessor.cpp Mon Apr 16 14:07:08 2018 @@ -85,12 +85,14 @@ Preprocessor::Preprocessor(std::shared_p IdentifierInfoLookup *IILookup, bool OwnsHeaders, TranslationUnitKind TUKind) : PPOpts(std::move(PPOpts)), Diags(&diags), LangOpts(opts), - FileMgr(Headers.getFileMgr()), SourceMgr(SM), - PCMCache(PCMCache), ScratchBuf(new ScratchBuffer(SourceMgr)), - HeaderInfo(Headers), TheModuleLoader(TheModuleLoader), - ExternalSource(nullptr), Identifiers(opts, IILookup), - PragmaHandlers(new PragmaNamespace(StringRef())), TUKind(TUKind), - SkipMainFilePreamble(0, true), + FileMgr(Headers.getFileMgr()), SourceMgr(SM), PCMCache(PCMCache), + ScratchBuf(new ScratchBuffer(SourceMgr)), HeaderInfo(Headers), + TheModuleLoader(TheModuleLoader), ExternalSource(nullptr), + // As the language options may have not been loaded yet (when + /
[PATCH] D35181: Defer addition of keywords to identifier table when loading AST
aaron.ballman closed this revision. aaron.ballman added a comment. Committed in r330159. Thank you for the patch! https://reviews.llvm.org/D35181 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
efriedma added a comment. It looks like you didn't fix the tests? https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34331: func.wrap.func.con: Unset function before destroying anything
vsapsai added a comment. Ping. https://reviews.llvm.org/D34331 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45698: [analyzer] When we fail to evaluate a pointer cast, escape the pointer.
NoQ added a comment. Hmm, yeah, indeed, i must have overlooked it, nice one :) I'll see if i can fix the other place as well. Repository: rC Clang https://reviews.llvm.org/D45698 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
erichkeane updated this revision to Diff 142695. erichkeane added a comment. Revert unnecessary test changes... :/ https://reviews.llvm.org/D45383 Files: include/clang/AST/ASTContext.h include/clang/Basic/Builtins.h include/clang/Basic/DiagnosticSemaKinds.td lib/AST/ASTContext.cpp lib/Basic/Builtins.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaOverload.cpp test/Sema/builtin-redecl.cpp Index: lib/AST/ASTContext.cpp === --- lib/AST/ASTContext.cpp +++ lib/AST/ASTContext.cpp @@ -7244,6 +7244,10 @@ return BuiltinMSVaListDecl; } +bool ASTContext::canBuiltinBeRedeclared(const FunctionDecl *FD) const { + return BuiltinInfo.canBeRedeclared(FD->getBuiltinID()); +} + void ASTContext::setObjCConstantStringInterface(ObjCInterfaceDecl *Decl) { assert(ObjCConstantStringType.isNull() && "'NSConstantString' type already set!"); Index: lib/Sema/SemaOverload.cpp === --- lib/Sema/SemaOverload.cpp +++ lib/Sema/SemaOverload.cpp @@ -998,6 +998,13 @@ Match = *I; return Ovl_Match; } + + // Builtins that have custom typechecking or have a reference should + // not be overloadable or redeclarable. + if (!getASTContext().canBuiltinBeRedeclared(OldF)) { +Match = *I; +return Ovl_NonFunction; + } } else if (isa(OldD) || isa(OldD)) { // We can overload with these, which can show up when doing // redeclaration checks for UsingDecls. Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -3011,6 +3011,14 @@ if (Old->isInvalidDecl()) return true; + // Disallow redeclaration of some builtins. + if (!getASTContext().canBuiltinBeRedeclared(Old)) { +Diag(New->getLocation(), diag::err_builtin_redeclare) << Old->getDeclName(); +Diag(Old->getLocation(), diag::note_previous_builtin_declaration) +<< Old << Old->getType(); +return true; + } + diag::kind PrevDiag; SourceLocation OldLocation; std::tie(PrevDiag, OldLocation) = Index: lib/Basic/Builtins.cpp === --- lib/Basic/Builtins.cpp +++ lib/Basic/Builtins.cpp @@ -139,3 +139,10 @@ bool &HasVAListArg) { return isLike(ID, FormatIdx, HasVAListArg, "sS"); } + +bool Builtin::Context::canBeRedeclared(unsigned ID) const { + return ID == Builtin::NotBuiltin || + ID == Builtin::BI__va_start || + (!hasReferenceArgsOrResult(ID) && + !hasCustomTypechecking(ID)); +} Index: include/clang/AST/ASTContext.h === --- include/clang/AST/ASTContext.h +++ include/clang/AST/ASTContext.h @@ -1881,6 +1881,10 @@ return getTypeDeclType(getBuiltinMSVaListDecl()); } + /// Return whether a declaration to a builtin is allowed to be + /// overloaded/redeclared. + bool canBuiltinBeRedeclared(const FunctionDecl *) const; + /// \brief Return a type with additional \c const, \c volatile, or /// \c restrict qualifiers. QualType getCVRQualifiedType(QualType T, unsigned CVR) const { Index: include/clang/Basic/DiagnosticSemaKinds.td === --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -602,6 +602,7 @@ "incompatible redeclaration of library function %0">, InGroup>; def err_builtin_definition : Error<"definition of builtin function %0">; +def err_builtin_redeclare : Error<"cannot redeclare builtin function %0">; def err_arm_invalid_specialreg : Error<"invalid special register for builtin">; def err_invalid_cpu_supports : Error<"invalid cpu feature string for builtin">; def err_invalid_cpu_is : Error<"invalid cpu name for builtin">; Index: include/clang/Basic/Builtins.h === --- include/clang/Basic/Builtins.h +++ include/clang/Basic/Builtins.h @@ -167,6 +167,13 @@ return strchr(getRecord(ID).Type, '*') != nullptr; } + /// \brief Return true if this builtin has a result or any arguments which are + /// reference types. + bool hasReferenceArgsOrResult(unsigned ID) const { +return strchr(getRecord(ID).Type, '&') != nullptr || + strchr(getRecord(ID).Type, 'A') != nullptr; + } + /// \brief Completely forget that the given ID was ever considered a builtin, /// e.g., because the user provided a conflicting signature. void forgetBuiltin(unsigned ID, IdentifierTable &Table); @@ -212,6 +219,10 @@ /// prefix. static bool isBuiltinFunc(const char *Name); + /// Returns true if this is a builtin that can be redeclared. Returns true + /// for non-builtins. + bool canBeRedeclared(unsigned ID) const; + private: const Info &getRecord(unsig