r335814 - [Analyzer] Constraint Manager Negates Difference
Author: baloghadamsoftware Date: Thu Jun 28 00:35:23 2018 New Revision: 335814 URL: http://llvm.org/viewvc/llvm-project?rev=335814&view=rev Log: [Analyzer] Constraint Manager Negates Difference If range [m .. n] is stored for symbolic expression A - B, then we can deduce the range for B - A which is [-n .. -m]. This is only true for signed types, unless the range is [0 .. 0]. Differential Revision: https://reviews.llvm.org/D35110 Added: cfe/trunk/test/Analysis/constraint_manager_negate_difference.c Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp cfe/trunk/test/Analysis/ptr-arith.c Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h?rev=335814&r1=335813&r2=335814&view=diff == --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h Thu Jun 28 00:35:23 2018 @@ -115,6 +115,8 @@ public: RangeSet Intersect(BasicValueFactory &BV, Factory &F, llvm::APSInt Lower, llvm::APSInt Upper) const; + RangeSet Negate(BasicValueFactory &BV, Factory &F) const; + void print(raw_ostream &os) const; bool operator==(const RangeSet &other) const { Modified: cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp?rev=335814&r1=335813&r2=335814&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp Thu Jun 28 00:35:23 2018 @@ -174,6 +174,38 @@ RangeSet RangeSet::Intersect(BasicValueF return newRanges; } +// Turn all [A, B] ranges to [-B, -A]. Ranges [MIN, B] are turned to range set +// [MIN, MIN] U [-B, MAX], when MIN and MAX are the minimal and the maximal +// signed values of the type. +RangeSet RangeSet::Negate(BasicValueFactory &BV, Factory &F) const { + PrimRangeSet newRanges = F.getEmptySet(); + + for (iterator i = begin(), e = end(); i != e; ++i) { +const llvm::APSInt &from = i->From(), &to = i->To(); +const llvm::APSInt &newTo = (from.isMinSignedValue() ? + BV.getMaxValue(from) : + BV.getValue(- from)); +if (to.isMaxSignedValue() && !newRanges.isEmpty() && +newRanges.begin()->From().isMinSignedValue()) { + assert(newRanges.begin()->To().isMinSignedValue() && + "Ranges should not overlap"); + assert(!from.isMinSignedValue() && "Ranges should not overlap"); + const llvm::APSInt &newFrom = newRanges.begin()->From(); + newRanges = +F.add(F.remove(newRanges, *newRanges.begin()), Range(newFrom, newTo)); +} else if (!to.isMinSignedValue()) { + const llvm::APSInt &newFrom = BV.getValue(- to); + newRanges = F.add(newRanges, Range(newFrom, newTo)); +} +if (from.isMinSignedValue()) { + newRanges = F.add(newRanges, Range(BV.getMinValue(from), + BV.getMinValue(from))); +} + } + + return newRanges; +} + void RangeSet::print(raw_ostream &os) const { bool isFirst = true; os << "{ "; @@ -252,6 +284,8 @@ private: RangeSet::Factory F; RangeSet getRange(ProgramStateRef State, SymbolRef Sym); + const RangeSet* getRangeForMinusSymbol(ProgramStateRef State, + SymbolRef Sym); RangeSet getSymLTRange(ProgramStateRef St, SymbolRef Sym, const llvm::APSInt &Int, @@ -268,6 +302,7 @@ private: RangeSet getSymGERange(ProgramStateRef St, SymbolRef Sym, const llvm::APSInt &Int, const llvm::APSInt &Adjustment); + }; } // end anonymous namespace @@ -423,9 +458,15 @@ RangeSet RangeConstraintManager::getRang if (ConstraintRangeTy::data_type *V = State->get(Sym)) return *V; + BasicValueFactory &BV = getBasicVals(); + + // If Sym is a difference of symbols A - B, then maybe we have range set + // stored for B - A. + if (const RangeSet *R = getRangeForMinusSymbol(State, Sym)) +return R->Negate(BV, F); + // Lazily generate a new RangeSet representing all possible values for the // given symbol type. - BasicValueFactory &BV = getBasicVals(); QualType T = Sym->getType(); RangeSet Result(F, BV.getMinValue(T), BV.getMaxValue(T)); @@ -441,6 +482,32 @@ RangeSet RangeConstraintManager::getRang return Result; } +// FIXME: Once SValBuilder supports unary minus, we should use SV
[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference
This revision was automatically updated to reflect the committed changes. Closed by commit rC335814: [Analyzer] Constraint Manager Negates Difference (authored by baloghadamsoftware, committed by ). Repository: rC Clang https://reviews.llvm.org/D35110 Files: include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h lib/StaticAnalyzer/Core/RangeConstraintManager.cpp test/Analysis/constraint_manager_negate_difference.c test/Analysis/ptr-arith.c Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp === --- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp +++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp @@ -174,6 +174,38 @@ return newRanges; } +// Turn all [A, B] ranges to [-B, -A]. Ranges [MIN, B] are turned to range set +// [MIN, MIN] U [-B, MAX], when MIN and MAX are the minimal and the maximal +// signed values of the type. +RangeSet RangeSet::Negate(BasicValueFactory &BV, Factory &F) const { + PrimRangeSet newRanges = F.getEmptySet(); + + for (iterator i = begin(), e = end(); i != e; ++i) { +const llvm::APSInt &from = i->From(), &to = i->To(); +const llvm::APSInt &newTo = (from.isMinSignedValue() ? + BV.getMaxValue(from) : + BV.getValue(- from)); +if (to.isMaxSignedValue() && !newRanges.isEmpty() && +newRanges.begin()->From().isMinSignedValue()) { + assert(newRanges.begin()->To().isMinSignedValue() && + "Ranges should not overlap"); + assert(!from.isMinSignedValue() && "Ranges should not overlap"); + const llvm::APSInt &newFrom = newRanges.begin()->From(); + newRanges = +F.add(F.remove(newRanges, *newRanges.begin()), Range(newFrom, newTo)); +} else if (!to.isMinSignedValue()) { + const llvm::APSInt &newFrom = BV.getValue(- to); + newRanges = F.add(newRanges, Range(newFrom, newTo)); +} +if (from.isMinSignedValue()) { + newRanges = F.add(newRanges, Range(BV.getMinValue(from), + BV.getMinValue(from))); +} + } + + return newRanges; +} + void RangeSet::print(raw_ostream &os) const { bool isFirst = true; os << "{ "; @@ -252,6 +284,8 @@ RangeSet::Factory F; RangeSet getRange(ProgramStateRef State, SymbolRef Sym); + const RangeSet* getRangeForMinusSymbol(ProgramStateRef State, + SymbolRef Sym); RangeSet getSymLTRange(ProgramStateRef St, SymbolRef Sym, const llvm::APSInt &Int, @@ -268,6 +302,7 @@ RangeSet getSymGERange(ProgramStateRef St, SymbolRef Sym, const llvm::APSInt &Int, const llvm::APSInt &Adjustment); + }; } // end anonymous namespace @@ -423,9 +458,15 @@ if (ConstraintRangeTy::data_type *V = State->get(Sym)) return *V; + BasicValueFactory &BV = getBasicVals(); + + // If Sym is a difference of symbols A - B, then maybe we have range set + // stored for B - A. + if (const RangeSet *R = getRangeForMinusSymbol(State, Sym)) +return R->Negate(BV, F); + // Lazily generate a new RangeSet representing all possible values for the // given symbol type. - BasicValueFactory &BV = getBasicVals(); QualType T = Sym->getType(); RangeSet Result(F, BV.getMinValue(T), BV.getMaxValue(T)); @@ -441,6 +482,32 @@ return Result; } +// FIXME: Once SValBuilder supports unary minus, we should use SValBuilder to +//obtain the negated symbolic expression instead of constructing the +//symbol manually. This will allow us to support finding ranges of not +//only negated SymSymExpr-type expressions, but also of other, simpler +//expressions which we currently do not know how to negate. +const RangeSet* +RangeConstraintManager::getRangeForMinusSymbol(ProgramStateRef State, + SymbolRef Sym) { + if (const SymSymExpr *SSE = dyn_cast(Sym)) { +if (SSE->getOpcode() == BO_Sub) { + QualType T = Sym->getType(); + SymbolManager &SymMgr = State->getSymbolManager(); + SymbolRef negSym = SymMgr.getSymSymExpr(SSE->getRHS(), BO_Sub, + SSE->getLHS(), T); + if (const RangeSet *negV = State->get(negSym)) { +// Unsigned range set cannot be negated, unless it is [0, 0]. +if ((negV->getConcreteValue() && + (*negV->getConcreteValue() == 0)) || +T->isSignedIntegerOrEnumerationType()) + return negV; + } +} + } + return nullptr; +} + //====== // assumeSymX methods: protected interface for RangeConstraintManager. //======/ Index: include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h ==
[PATCH] D45719: [clang-Format] Fix indentation of member call after block
ank added a comment. awesome, I do not have merge rights so help with merging this would be greatly appreciated Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48159: [clangd] Implement hover for "auto" and "decltype"
sammccall accepted this revision. sammccall added a subscriber: klimek. sammccall added a comment. All sounds good to me. Comment at: clangd/XRefs.cpp:559 + //- auto& i = 1; + bool VisitDeclaratorDecl(DeclaratorDecl *D) { +if (!D->getTypeSourceInfo() || malaperle wrote: > sammccall wrote: > > out of curiosity, why not implement `VisitTypeLoc` and handle all the cases > > where it turns out to be `auto` etc? > > Even for `auto&` I'd expect the inner `auto` to have a `TypeLoc` you could > > visit, saving the trouble of unwrapping. > > > > (I'm probably wrong about all this, I don't know the AST well. But I'd like > > to learn!) > From what I saw, there are actually two different AutoType* for each textual > "auto". The AutoType* containing the deduced type does not get visited via a > typeloc. It's not entirely clear to me why since I don't know the AST well > either. I was thinking maybe the first is created when the type is not > deduced yet and later on, then the rest of the function or expression is > parsed, a second one with the actual type deduced is created. If I look at > the code paths where they are created, it seems like this is roughly what's > happening. The first one is created when the declarator is parsed (no deduced > type yet) and the second is created when the expression of the initializer > (or return statement) is evaluated and the type is then deduced. The visitor > only visits the first one's typeloc. I don't think I'm knowledgeable enough > to say whether or not that's a bug but it seems on purpose that it is > modelled this way. Although it would be much nicer to only have to visit > typelocs... > The AutoType* containing the deduced type does not get visited via a typeloc Ah, OK. Could you add a high level comment (maybe on the class) saying this is the reason for the implementation? Otherwise as a reader I'll think "this seems unneccesarily complicated" but not understand why. @klimek Can you shed any light on this? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48626: New option -fwindows-filesystem, affecting #include paths.
simon_tatham added a comment. @thakis : no, the backslash support is needed for include directives //in source files//, not just on command lines, because in my experience it's not unusual for Windows-only code bases to be full of things like `#include "Subdir\Header.h"`. @mstorsjo : in this draft, I traverse the pathname from left to right, and at each level, if the normal case-sensitive lookup succeeds then I don't scan the whole directory looking for other options. As I mentioned above, though, I've since realised I should have done it the other way round, trying the case sensitive lookup on the //whole// pathname in one go first, and not even trying it one level at a time unless that fails. Although, come to think of it, that's not good enough, because if you have multiple directories on your include //path// then you expect a lot of lookups to fail for reasons that have nothing to do with case. Say, if you compile with `-Ifoo -Ibar`, then every include of a file intended to be found in `bar` will force lots of pointless directory enumeration in `foo` when the initial lookup there doesn't find the file. Hmmm. That suggests to me that perhaps this ought not to be a //global// `-ffudge-my-paths` type option applying to all include directories, and instead perhaps it should be a new kind of `-I` option, so that only lookups relative to that particular include-path directory would get the unusual semantics. What do people think? Repository: rC Clang https://reviews.llvm.org/D48626 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48708: NFC Build fix in RegisterCustomCheckersTest.cpp
GBuella created this revision. GBuella added reviewers: alexfh, george.karpenkov. Herald added a subscriber: cfe-commits. `ninja-build check-clang` failed using GCC 4.8.5 with: unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp:64:12: error: could not convert ‘AnalysisConsumer’ from ‘std::unique_ptr’ to ‘std::unique_ptr’ return AnalysisConsumer; ^ Repository: rC Clang https://reviews.llvm.org/D48708 Files: unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp Index: unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp === --- unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp +++ unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp @@ -61,7 +61,7 @@ AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry &Registry) { Registry.addChecker("custom.CustomChecker", "Description"); }); -return AnalysisConsumer; +return std::unique_ptr(AnalysisConsumer.release()); } }; Index: unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp === --- unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp +++ unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp @@ -61,7 +61,7 @@ AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry &Registry) { Registry.addChecker("custom.CustomChecker", "Description"); }); -return AnalysisConsumer; +return std::unique_ptr(AnalysisConsumer.release()); } }; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol
sammccall added inline comments. Comment at: clangd/SourceCode.cpp:194 + if (!llvm::sys::path::is_absolute(FilePath)) { +if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath)) { + log("Could not turn relative path to absolute: " + FilePath); sammccall wrote: > the common case when tryGetRealPathName() is empty seems to be when it's a > file that was part of the preamble we're reusing. > Does this fallback tend to give the same answer in that case? (If so, great! > I know some other places we should reuse this function!) Sorry, I just noticed this is only being moved. Please disregard (and thangs for moving it somewhere common). Note that D48687 modifies this function, let's make sure not to lose anything in the merge. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47846 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48159: [clangd] Implement hover for "auto" and "decltype"
klimek added inline comments. Comment at: clangd/XRefs.cpp:559 + //- auto& i = 1; + bool VisitDeclaratorDecl(DeclaratorDecl *D) { +if (!D->getTypeSourceInfo() || sammccall wrote: > malaperle wrote: > > sammccall wrote: > > > out of curiosity, why not implement `VisitTypeLoc` and handle all the > > > cases where it turns out to be `auto` etc? > > > Even for `auto&` I'd expect the inner `auto` to have a `TypeLoc` you > > > could visit, saving the trouble of unwrapping. > > > > > > (I'm probably wrong about all this, I don't know the AST well. But I'd > > > like to learn!) > > From what I saw, there are actually two different AutoType* for each > > textual "auto". The AutoType* containing the deduced type does not get > > visited via a typeloc. It's not entirely clear to me why since I don't know > > the AST well either. I was thinking maybe the first is created when the > > type is not deduced yet and later on, then the rest of the function or > > expression is parsed, a second one with the actual type deduced is created. > > If I look at the code paths where they are created, it seems like this is > > roughly what's happening. The first one is created when the declarator is > > parsed (no deduced type yet) and the second is created when the expression > > of the initializer (or return statement) is evaluated and the type is then > > deduced. The visitor only visits the first one's typeloc. I don't think I'm > > knowledgeable enough to say whether or not that's a bug but it seems on > > purpose that it is modelled this way. Although it would be much nicer to > > only have to visit typelocs... > > The AutoType* containing the deduced type does not get visited via a typeloc > Ah, OK. > Could you add a high level comment (maybe on the class) saying this is the > reason for the implementation? Otherwise as a reader I'll think "this seems > unneccesarily complicated" but not understand why. > > @klimek Can you shed any light on this? Can't you go from AutoTypeLoc -> AutoType -> getDeducedType()? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48159 ___ 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 updated this revision to Diff 153267. JonasToth added a comment. - rebase on commited ExpMutationAnalyzer - clean up, documentation Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/ConstCheck.cpp clang-tidy/cppcoreguidelines/ConstCheck.h clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp docs/ReleaseNotes.rst docs/clang-tidy/checks/cppcoreguidelines-const.rst docs/clang-tidy/checks/list.rst test/clang-tidy/cppcoreguidelines-const-pointers-as-values.cpp test/clang-tidy/cppcoreguidelines-const-values.cpp Index: test/clang-tidy/cppcoreguidelines-const-values.cpp === --- /dev/null +++ test/clang-tidy/cppcoreguidelines-const-values.cpp @@ -0,0 +1,557 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-const %t + +// --- Provide test samples for primitive builtins - +// - every 'p_*' variable is a 'potential_const_*' variable +// - every 'np_*' variable is a 'non_potential_const_*' variable + +bool global; +char np_global = 0; // globals can't be known to be const + +namespace foo { +int scoped; +float np_scoped = 1; // namespace variables are like globals +} // namespace foo + +void some_function(double, wchar_t); + +void some_function(double np_arg0, wchar_t np_arg1) { + int p_local0 = 2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + + int np_local0; + const int np_local1 = 42; + + unsigned int np_local2 = 3; + np_local2 <<= 4; + + int np_local3 = 4; + ++np_local3; + int np_local4 = 4; + np_local4++; + + int np_local5 = 4; + --np_local5; + int np_local6 = 4; + np_local6--; +} + +void nested_scopes() { + int p_local0 = 2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + int np_local0 = 42; + + { +int p_local1 = 42; +// CHECK-MESSAGES: [[@LINE-1]]:5: warning: variable 'p_local1' of type 'int' can be declared const +np_local0 *= 2; + } +} + +void some_lambda_environment_capture_all_by_reference(double np_arg0) { + int np_local0 = 0; + int p_local0 = 1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + + int np_local2; + const int np_local3 = 2; + + // Capturing all variables by reference prohibits making them const. + [&]() { ++np_local0; }; + + int p_local1 = 0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const +} + +void some_lambda_environment_capture_all_by_value(double np_arg0) { + int np_local0 = 0; + int p_local0 = 1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + + int np_local1; + const int np_local2 = 2; + + // Capturing by value has no influence on them. + [=]() { (void)p_local0; }; + + np_local0 += 10; +} + +void function_inout_pointer(int *inout); +void function_in_pointer(const int *in); + +void some_pointer_taking(int *out) { + int np_local0 = 42; + const int *const p0_np_local0 = &np_local0; + int *const p1_np_local0 = &np_local0; + + int np_local1 = 42; + const int *const p0_np_local1 = &np_local1; + int *const p1_np_local1 = &np_local1; + *p1_np_local0 = 43; + + int np_local2 = 42; + function_inout_pointer(&np_local2); + + // Prevents const. + int np_local3 = 42; + out = &np_local3; // This returns and invalid address, its just about the AST + + int p_local1 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const + const int *const p0_p_local1 = &p_local1; + + int p_local2 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int' can be declared const + function_in_pointer(&p_local2); +} + +void function_inout_ref(int &inout); +void function_in_ref(const int &in); + +void some_reference_taking() { + int np_local0 = 42; + const int &r0_np_local0 = np_local0; + int &r1_np_local0 = np_local0; + r1_np_local0 = 43; + const int &r2_np_local0 = r1_np_local0; + + int np_local1 = 42; + function_inout_ref(np_local1); + + int p_local0 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + const int &r0_p_local0 = p_local0; + + int p_local1 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const + function_in_ref(p_local1); +} + +double *non_const_pointer_return() { + double p_local0 = 0.0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const + double np_local0 = 24.4; + + return &np_local0; +} + +const double *const_pointer_return() { + double p_local0 = 0.0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const + double p_local1 = 24.4; + // CHECK-MESSAGES: [[@
[PATCH] D45444: [clang-tidy] implement new check for const-correctness
JonasToth updated this revision to Diff 153268. JonasToth marked 4 inline comments as done. JonasToth added a comment. - [Misc] order in release notes Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/ConstCheck.cpp clang-tidy/cppcoreguidelines/ConstCheck.h clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp docs/ReleaseNotes.rst docs/clang-tidy/checks/cppcoreguidelines-const.rst docs/clang-tidy/checks/list.rst test/clang-tidy/cppcoreguidelines-const-pointers-as-values.cpp test/clang-tidy/cppcoreguidelines-const-values.cpp Index: test/clang-tidy/cppcoreguidelines-const-values.cpp === --- /dev/null +++ test/clang-tidy/cppcoreguidelines-const-values.cpp @@ -0,0 +1,557 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-const %t + +// --- Provide test samples for primitive builtins - +// - every 'p_*' variable is a 'potential_const_*' variable +// - every 'np_*' variable is a 'non_potential_const_*' variable + +bool global; +char np_global = 0; // globals can't be known to be const + +namespace foo { +int scoped; +float np_scoped = 1; // namespace variables are like globals +} // namespace foo + +void some_function(double, wchar_t); + +void some_function(double np_arg0, wchar_t np_arg1) { + int p_local0 = 2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + + int np_local0; + const int np_local1 = 42; + + unsigned int np_local2 = 3; + np_local2 <<= 4; + + int np_local3 = 4; + ++np_local3; + int np_local4 = 4; + np_local4++; + + int np_local5 = 4; + --np_local5; + int np_local6 = 4; + np_local6--; +} + +void nested_scopes() { + int p_local0 = 2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + int np_local0 = 42; + + { +int p_local1 = 42; +// CHECK-MESSAGES: [[@LINE-1]]:5: warning: variable 'p_local1' of type 'int' can be declared const +np_local0 *= 2; + } +} + +void some_lambda_environment_capture_all_by_reference(double np_arg0) { + int np_local0 = 0; + int p_local0 = 1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + + int np_local2; + const int np_local3 = 2; + + // Capturing all variables by reference prohibits making them const. + [&]() { ++np_local0; }; + + int p_local1 = 0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const +} + +void some_lambda_environment_capture_all_by_value(double np_arg0) { + int np_local0 = 0; + int p_local0 = 1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + + int np_local1; + const int np_local2 = 2; + + // Capturing by value has no influence on them. + [=]() { (void)p_local0; }; + + np_local0 += 10; +} + +void function_inout_pointer(int *inout); +void function_in_pointer(const int *in); + +void some_pointer_taking(int *out) { + int np_local0 = 42; + const int *const p0_np_local0 = &np_local0; + int *const p1_np_local0 = &np_local0; + + int np_local1 = 42; + const int *const p0_np_local1 = &np_local1; + int *const p1_np_local1 = &np_local1; + *p1_np_local0 = 43; + + int np_local2 = 42; + function_inout_pointer(&np_local2); + + // Prevents const. + int np_local3 = 42; + out = &np_local3; // This returns and invalid address, its just about the AST + + int p_local1 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const + const int *const p0_p_local1 = &p_local1; + + int p_local2 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int' can be declared const + function_in_pointer(&p_local2); +} + +void function_inout_ref(int &inout); +void function_in_ref(const int &in); + +void some_reference_taking() { + int np_local0 = 42; + const int &r0_np_local0 = np_local0; + int &r1_np_local0 = np_local0; + r1_np_local0 = 43; + const int &r2_np_local0 = r1_np_local0; + + int np_local1 = 42; + function_inout_ref(np_local1); + + int p_local0 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + const int &r0_p_local0 = p_local0; + + int p_local1 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const + function_in_ref(p_local1); +} + +double *non_const_pointer_return() { + double p_local0 = 0.0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const + double np_local0 = 24.4; + + return &np_local0; +} + +const double *const_pointer_return() { + double p_local0 = 0.0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const + double p_local1 = 24.4; + // CHECK-MESS
[PATCH] D45444: [clang-tidy] implement new check for const-correctness
JonasToth added a comment. - fixed some comments 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] D48687: [clangd] Avoid duplicates in findDefinitions response
ilya-biryukov added a comment. Thanks for the patch! Could we try to figure out why the duplicates were there in the first place and why the paths were different? It should be easy to mock exactly the same setup you have in #37963, i.e. create a vfs with three files and compilation database that has exactly those compile commands. You mention you tried to repro the issue, did you try doing that with the unit-test or a lit-test? After looking at the makefile, my guess would be that the problem comes from the paths starting with `../` inside the compilation database. Comment at: clangd/XRefs.cpp:179 +const SourceManager &SourceMgr, +const vfs::FileSystem &VFS) { SmallString<64> FilePath = F->tryGetRealPathName(); Do we really need an extra vfs param? Could we use `SourceMgr.getFileManager().getVirtualFileSystem()` instead? Comment at: unittests/clangd/TestTU.h:47 - ParsedAST build() const; + ParsedAST build(IntrusiveRefCntPtr *OutFS = nullptr) const; SymbolSlab headerSymbols() const; We don't need an extra output param here. There's a way to get the vfs from the ASTContext: `ParsedAST().getASTContext().getSourceManager().getFileManager().getVirtualFileSystem()`. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response
ilya-biryukov added a comment. Also some heads-up: this would probably conflict https://reviews.llvm.org/D47846 that moves the same function into a different file. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48707: [CMake] Disable per-target runtimes for the first stage Fuchsia build
phosek updated this revision to Diff 153270. phosek edited the summary of this revision. Repository: rC Clang https://reviews.llvm.org/D48707 Files: clang/cmake/caches/Fuchsia.cmake Index: clang/cmake/caches/Fuchsia.cmake === --- clang/cmake/caches/Fuchsia.cmake +++ clang/cmake/caches/Fuchsia.cmake @@ -29,6 +29,18 @@ set(COMPILER_RT_ENABLE_IOS OFF CACHE BOOL "") set(COMPILER_RT_ENABLE_TVOS OFF CACHE BOOL "") set(COMPILER_RT_ENABLE_WATCHOS OFF CACHE BOOL "") +else() + set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "") + set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "") + set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "") + set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "") + set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "") + set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "") + set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "") + set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "") + set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "") + set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "") + set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "") endif() set(CLANG_BOOTSTRAP_TARGETS Index: clang/cmake/caches/Fuchsia.cmake === --- clang/cmake/caches/Fuchsia.cmake +++ clang/cmake/caches/Fuchsia.cmake @@ -29,6 +29,18 @@ set(COMPILER_RT_ENABLE_IOS OFF CACHE BOOL "") set(COMPILER_RT_ENABLE_TVOS OFF CACHE BOOL "") set(COMPILER_RT_ENABLE_WATCHOS OFF CACHE BOOL "") +else() + set(LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "") + set(LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "") + set(LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "") + set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "") + set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "") + set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "") + set(LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "") + set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "") + set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "") + set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "") + set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "") endif() set(CLANG_BOOTSTRAP_TARGETS ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48701: [CMake] Rename cxx_headers back to cxx-headers.
phosek accepted this revision. phosek added a comment. This revision is now accepted and ready to land. LGTM Repository: rCXX libc++ https://reviews.llvm.org/D48701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36357: Added a better diagnostic when using the delete operator with lambdas
Rakete updated this revision to Diff 153271. Rakete added a comment. Addressed review comments :) https://reviews.llvm.org/D36357 Files: include/clang/Basic/DiagnosticParseKinds.td lib/Parse/ParseExprCXX.cpp test/Parser/cxx0x-lambda-expressions.cpp test/SemaCXX/new-delete-0x.cpp Index: test/SemaCXX/new-delete-0x.cpp === --- test/SemaCXX/new-delete-0x.cpp +++ test/SemaCXX/new-delete-0x.cpp @@ -34,6 +34,7 @@ void bad_deletes() { // 'delete []' is always array delete, per [expr.delete]p1. - // FIXME: Give a better diagnostic. - delete []{ return (int*)0; }(); // expected-error {{expected expression}} + delete []{ return (int*)0; }(); // expected-error {{'[]' after delete interpreted as 'delete[]'}} + // expected-note@-1 {{add parentheses around the lambda}} } + Index: test/Parser/cxx0x-lambda-expressions.cpp === --- test/Parser/cxx0x-lambda-expressions.cpp +++ test/Parser/cxx0x-lambda-expressions.cpp @@ -53,7 +53,8 @@ void delete_lambda(int *p) { delete [] p; delete [] (int*) { new int }; // ok, compound-literal, not lambda -delete [] { return new int; } (); // expected-error{{expected expression}} +delete [] { return new int; } (); // expected-error {{'[]' after delete interpreted as 'delete[]'}} +// expected-note@-1 {{add parentheses around the lambda}} delete [&] { return new int; } (); // ok, lambda } Index: lib/Parse/ParseExprCXX.cpp === --- lib/Parse/ParseExprCXX.cpp +++ lib/Parse/ParseExprCXX.cpp @@ -2951,15 +2951,47 @@ // [Footnote: A lambda expression with a lambda-introducer that consists // of empty square brackets can follow the delete keyword if // the lambda expression is enclosed in parentheses.] -// FIXME: Produce a better diagnostic if the '[]' is unambiguously a -//lambda-introducer. -ArrayDelete = true; -BalancedDelimiterTracker T(*this, tok::l_square); -T.consumeOpen(); -T.consumeClose(); -if (T.getCloseLocation().isInvalid()) - return ExprError(); +const Token Next = GetLookAheadToken(2); +const auto GetAfter = [this] { return GetLookAheadToken(3); }; + +// Basic lookahead to check if we have a lambda expression. +if (Next.isOneOf(tok::l_brace, tok::less) || +(Next.is(tok::l_paren) && + (GetAfter().is(tok::r_paren) || + (GetAfter().is(tok::identifier) && + GetLookAheadToken(4).is(tok::identifier) { + // Warn if the non-capturing lambda isn't surrounded by parenthesis + // to disambiguate it from 'delete[]'. + ExprResult Lambda = ParseLambdaExpression(); + if (Lambda.isInvalid()) +return ExprError(); + + SourceLocation StartLoc = Lambda.get()->getLocStart(); + Diag(Start, diag::err_lambda_after_delete) + << SourceRange(Start, StartLoc.getLocWithOffset(1)); + + Diag(StartLoc, diag::note_lambda_after_delete) + << FixItHint::CreateInsertion(StartLoc, "(") + << FixItHint::CreateInsertion( + Lexer::getLocForEndOfToken(Lambda.get()->getLocEnd(), 1, +Actions.getSourceManager(), +getLangOpts()), + ")"); + + // Evaluate any postfix expressions used on the lambda. + Lambda = ParsePostfixExpressionSuffix(Lambda); + return Actions.ActOnCXXDelete(Start, UseGlobal, /*ArrayForm=*/false, +Lambda.get()); +} else { + ArrayDelete = true; + BalancedDelimiterTracker T(*this, tok::l_square); + + T.consumeOpen(); + T.consumeClose(); + if (T.getCloseLocation().isInvalid()) +return ExprError(); +} } ExprResult Operand(ParseCastExpression(false)); Index: include/clang/Basic/DiagnosticParseKinds.td === --- include/clang/Basic/DiagnosticParseKinds.td +++ include/clang/Basic/DiagnosticParseKinds.td @@ -99,6 +99,10 @@ InGroup, DefaultIgnore; def ext_alignof_expr : ExtWarn< "%0 applied to an expression is a GNU extension">, InGroup; +def err_lambda_after_delete : Error< + "'[]' after delete interpreted as 'delete[]'">; +def note_lambda_after_delete : Note< + "add parentheses around the lambda">; def warn_microsoft_dependent_exists : Warning< "dependent %select{__if_not_exists|__if_exists}0 declarations are ignored">, ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48700: [CMake] Make bootstrap and compiler-rt depend on cxx-headers.
phosek accepted this revision. phosek added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D48700 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48701: [CMake] Rename cxx_headers back to cxx-headers.
phosek added a comment. It's not obvious to me how anything could depend on `cxx-header`s target (not the component) since that target did not exist until r334468 which has landed only recently (just before r334477). The motivation behind this change was just consistency since all other targets in libc++ use underscores rather than dashes, so reverting that change is fine with me if it's breaking you. Repository: rCXX libc++ https://reviews.llvm.org/D48701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48710: [CMake] Don't build Linux targets on Darwin in Fuchsia toolchain
phosek created this revision. phosek added a reviewer: mcgrathr. Herald added subscribers: cfe-commits, mgorny. This is currently breaking because Linux target sysroots rely on case sensitive filesystem which is not by default enabled on macOS. Repository: rC Clang https://reviews.llvm.org/D48710 Files: clang/cmake/caches/Fuchsia-stage2.cmake Index: clang/cmake/caches/Fuchsia-stage2.cmake === --- clang/cmake/caches/Fuchsia-stage2.cmake +++ clang/cmake/caches/Fuchsia-stage2.cmake @@ -31,35 +31,35 @@ if(APPLE) list(APPEND BUILTIN_TARGETS "default") list(APPEND RUNTIME_TARGETS "default") -endif() - -foreach(target i386;x86_64;armhf;aarch64) - if(LINUX_${target}_SYSROOT) -# Set the per-target builtins options. -list(APPEND BUILTIN_TARGETS "${target}-linux-gnu") -set(BUILTINS_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "") -set(BUILTINS_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "") -set(BUILTINS_${target}-linux-gnu_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} CACHE STRING "") +else() + foreach(target i386;x86_64;armhf;aarch64) +if(LINUX_${target}_SYSROOT) + # Set the per-target builtins options. + list(APPEND BUILTIN_TARGETS "${target}-linux-gnu") + set(BUILTINS_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "") + set(BUILTINS_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "") + set(BUILTINS_${target}-linux-gnu_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} CACHE STRING "") -# Set the per-target runtimes options. -list(APPEND RUNTIME_TARGETS "${target}-linux-gnu") -set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "") -set(RUNTIMES_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "") -set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} CACHE STRING "") -set(RUNTIMES_${target}-linux-gnu_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "") -set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "") -set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "") -set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "") -set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "") -set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "") -set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "") -set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "") -set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "") -set(RUNTIMES_${target}-linux-gnu_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "") -set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "") -set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "") - endif() -endforeach() + # Set the per-target runtimes options. + list(APPEND RUNTIME_TARGETS "${target}-linux-gnu") + set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "") + set(RUNTIMES_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "") + set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} CACHE STRING "") + set(RUNTIMES_${target}-linux-gnu_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "") + set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "") + set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "") + set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "") + set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "") + set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "") + set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "") + set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "") + set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "") + set(RUNTIMES_${target}-linux-gnu_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "") + set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "") + set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "") +endif() + endforeach() +endif() if(FUCHSIA_SDK) set(FUCHSIA_aarch64_NAME arm64) Index: clang/cmake/caches/Fuchsia-stage2.cmake === --- clang/cmake/caches/Fuchsia-stage2.cmake +++ clang/cmake/caches/Fuchsia-stage2.cmake @@ -31,35 +31,35 @@ if(APPLE) list(APPEND BUILTIN_TARGETS "default") list(APPEND RUNTIME_TARGETS "default") -endif() - -foreach(target i386;x86_64;armhf;aarch64) - if(LINUX_${target}_SYSROOT) -# Set the per-target builtins options. -list(APPEND BUILTIN_TARGETS "${target}-linux-gnu") -set(BUILTINS_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CA
[PATCH] D48634: [clangd] Improve output of --help and --version. NFC.
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48634 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48712: [X86] Lowering integer truncation intrinsics to native IR
mike.dvoretsky created this revision. mike.dvoretsky added a reviewer: craig.topper. Herald added a subscriber: cfe-commits. This patch lowers the _mm[256|512]_cvtepi{64|32|16}_epi{32|16|8} intrinsics to native IR in cases where the result's length is less than 128 bits. The resulting IR is folded into VPMOV instructions in https://reviews.llvm.org/D46957, with the exception of _mm_cvtepi64_epi8, where a PSHUFB instruction is currently produced instead. Repository: rC Clang https://reviews.llvm.org/D48712 Files: clang/lib/Headers/avx512vlbwintrin.h clang/lib/Headers/avx512vlintrin.h clang/test/CodeGen/avx512vl-builtins.c clang/test/CodeGen/avx512vlbw-builtins.c Index: clang/test/CodeGen/avx512vlbw-builtins.c === --- clang/test/CodeGen/avx512vlbw-builtins.c +++ clang/test/CodeGen/avx512vlbw-builtins.c @@ -1792,7 +1792,8 @@ __m128i test_mm_cvtepi16_epi8(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi16_epi8 - // CHECK: @llvm.x86.avx512.mask.pmov.wb.128 + // CHECK: trunc <8 x i16> %{{.*}} to <8 x i8> + // CHECK: shufflevector <8 x i8> %{{.*}}, <8 x i8> %{{.*}}, <16 x i32> return _mm_cvtepi16_epi8(__A); } Index: clang/test/CodeGen/avx512vl-builtins.c === --- clang/test/CodeGen/avx512vl-builtins.c +++ clang/test/CodeGen/avx512vl-builtins.c @@ -6974,7 +6974,8 @@ __m128i test_mm_cvtepi32_epi8(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi32_epi8 - // CHECK: @llvm.x86.avx512.mask.pmov.db.128 + // CHECK: trunc <4 x i32> %{{.*}} to <4 x i8> + // CHECK: shufflevector <4 x i8> %{{.*}}, <4 x i8> %{{.*}}, <16 x i32> return _mm_cvtepi32_epi8(__A); } @@ -6998,7 +6999,8 @@ __m128i test_mm256_cvtepi32_epi8(__m256i __A) { // CHECK-LABEL: @test_mm256_cvtepi32_epi8 - // CHECK: @llvm.x86.avx512.mask.pmov.db.256 + // CHECK: trunc <8 x i32> %{{.*}} to <8 x i8> + // CHECK: shufflevector <8 x i8> %{{.*}}, <8 x i8> %{{.*}}, <16 x i32> return _mm256_cvtepi32_epi8(__A); } @@ -7022,7 +7024,8 @@ __m128i test_mm_cvtepi32_epi16(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi32_epi16 - // CHECK: @llvm.x86.avx512.mask.pmov.dw.128 + // CHECK: trunc <4 x i32> %{{.*}} to <4 x i16> + // CHECK: shufflevector <4 x i16> %{{.*}}, <4 x i16> %{{.*}}, <8 x i32> return _mm_cvtepi32_epi16(__A); } @@ -7070,7 +7073,8 @@ __m128i test_mm_cvtepi64_epi8(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi64_epi8 - // CHECK: @llvm.x86.avx512.mask.pmov.qb.128 + // CHECK: trunc <2 x i64> %{{.*}} to <2 x i8> + // CHECK: shufflevector <2 x i8> %{{.*}}, <2 x i8> %{{.*}}, <16 x i32> return _mm_cvtepi64_epi8(__A); } @@ -7094,7 +7098,8 @@ __m128i test_mm256_cvtepi64_epi8(__m256i __A) { // CHECK-LABEL: @test_mm256_cvtepi64_epi8 - // CHECK: @llvm.x86.avx512.mask.pmov.qb.256 + // CHECK: trunc <4 x i64> %{{.*}} to <4 x i8> + // CHECK: shufflevector <4 x i8> %{{.*}}, <4 x i8> %{{.*}}, <16 x i32> return _mm256_cvtepi64_epi8(__A); } @@ -7118,7 +7123,8 @@ __m128i test_mm_cvtepi64_epi32(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi64_epi32 - // CHECK: @llvm.x86.avx512.mask.pmov.qd.128 + // CHECK: trunc <2 x i64> %{{.*}} to <2 x i32> + // CHECK: shufflevector <2 x i32> %{{.*}}, <2 x i32> %{{.*}}, <4 x i32> return _mm_cvtepi64_epi32(__A); } @@ -7168,7 +7174,8 @@ __m128i test_mm_cvtepi64_epi16(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi64_epi16 - // CHECK: @llvm.x86.avx512.mask.pmov.qw.128 + // CHECK: trunc <2 x i64> %{{.*}} to <2 x i16> + // CHECK: shufflevector <2 x i16> %{{.*}}, <2 x i16> %{{.*}}, <8 x i32> return _mm_cvtepi64_epi16(__A); } @@ -7192,7 +7199,8 @@ __m128i test_mm256_cvtepi64_epi16(__m256i __A) { // CHECK-LABEL: @test_mm256_cvtepi64_epi16 - // CHECK: @llvm.x86.avx512.mask.pmov.qw.256 + // CHECK: trunc <4 x i64> %{{.*}} to <4 x i16> + // CHECK: shufflevector <4 x i16> %{{.*}}, <4 x i16> %{{.*}}, <8 x i32> return _mm256_cvtepi64_epi16(__A); } Index: clang/lib/Headers/avx512vlintrin.h === --- clang/lib/Headers/avx512vlintrin.h +++ clang/lib/Headers/avx512vlintrin.h @@ -30,6 +30,7 @@ #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("avx512vl"))) +typedef short __v2hi __attribute__((__vector_size__(4))); typedef char __v4qi __attribute__((__vector_size__(4))); typedef char __v2qi __attribute__((__vector_size__(2))); @@ -7415,10 +7416,9 @@ static __inline__ __m128i __DEFAULT_FN_ATTRS _mm_cvtepi32_epi8 (__m128i __A) { - return __builtin_shufflevector(__builtin_convertvector((__v4si)__A, __v4qi), - (__v4qi) {0, 0, 0, 0}, - 0, 1, 2, 3, 4, 5, 6, 7, - 7, 7, 7, 7, 7, 7, 7, 7); + return (__m128i)__builtin_shufflevector( + __builtin_convertvector((__v4si)__A, __v4qi), (__v4q
[PATCH] D48712: [X86] Lowering integer truncation intrinsics to native IR
mike.dvoretsky updated this revision to Diff 153277. mike.dvoretsky added a comment. Uploaded the correct diff. https://reviews.llvm.org/D48712 Files: clang/lib/Headers/avx512vlbwintrin.h clang/lib/Headers/avx512vlintrin.h clang/test/CodeGen/avx512vl-builtins.c clang/test/CodeGen/avx512vlbw-builtins.c Index: clang/test/CodeGen/avx512vlbw-builtins.c === --- clang/test/CodeGen/avx512vlbw-builtins.c +++ clang/test/CodeGen/avx512vlbw-builtins.c @@ -1792,7 +1792,8 @@ __m128i test_mm_cvtepi16_epi8(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi16_epi8 - // CHECK: @llvm.x86.avx512.mask.pmov.wb.128 + // CHECK: trunc <8 x i16> %{{.*}} to <8 x i8> + // CHECK: shufflevector <8 x i8> %{{.*}}, <8 x i8> %{{.*}}, <16 x i32> return _mm_cvtepi16_epi8(__A); } Index: clang/test/CodeGen/avx512vl-builtins.c === --- clang/test/CodeGen/avx512vl-builtins.c +++ clang/test/CodeGen/avx512vl-builtins.c @@ -6974,7 +6974,8 @@ __m128i test_mm_cvtepi32_epi8(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi32_epi8 - // CHECK: @llvm.x86.avx512.mask.pmov.db.128 + // CHECK: trunc <4 x i32> %{{.*}} to <4 x i8> + // CHECK: shufflevector <4 x i8> %{{.*}}, <4 x i8> %{{.*}}, <16 x i32> return _mm_cvtepi32_epi8(__A); } @@ -6998,7 +6999,8 @@ __m128i test_mm256_cvtepi32_epi8(__m256i __A) { // CHECK-LABEL: @test_mm256_cvtepi32_epi8 - // CHECK: @llvm.x86.avx512.mask.pmov.db.256 + // CHECK: trunc <8 x i32> %{{.*}} to <8 x i8> + // CHECK: shufflevector <8 x i8> %{{.*}}, <8 x i8> %{{.*}}, <16 x i32> return _mm256_cvtepi32_epi8(__A); } @@ -7022,7 +7024,8 @@ __m128i test_mm_cvtepi32_epi16(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi32_epi16 - // CHECK: @llvm.x86.avx512.mask.pmov.dw.128 + // CHECK: trunc <4 x i32> %{{.*}} to <4 x i16> + // CHECK: shufflevector <4 x i16> %{{.*}}, <4 x i16> %{{.*}}, <8 x i32> return _mm_cvtepi32_epi16(__A); } @@ -7070,7 +7073,8 @@ __m128i test_mm_cvtepi64_epi8(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi64_epi8 - // CHECK: @llvm.x86.avx512.mask.pmov.qb.128 + // CHECK: trunc <2 x i64> %{{.*}} to <2 x i8> + // CHECK: shufflevector <2 x i8> %{{.*}}, <2 x i8> %{{.*}}, <16 x i32> return _mm_cvtepi64_epi8(__A); } @@ -7094,7 +7098,8 @@ __m128i test_mm256_cvtepi64_epi8(__m256i __A) { // CHECK-LABEL: @test_mm256_cvtepi64_epi8 - // CHECK: @llvm.x86.avx512.mask.pmov.qb.256 + // CHECK: trunc <4 x i64> %{{.*}} to <4 x i8> + // CHECK: shufflevector <4 x i8> %{{.*}}, <4 x i8> %{{.*}}, <16 x i32> return _mm256_cvtepi64_epi8(__A); } @@ -7118,7 +7123,8 @@ __m128i test_mm_cvtepi64_epi32(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi64_epi32 - // CHECK: @llvm.x86.avx512.mask.pmov.qd.128 + // CHECK: trunc <2 x i64> %{{.*}} to <2 x i32> + // CHECK: shufflevector <2 x i32> %{{.*}}, <2 x i32> %{{.*}}, <4 x i32> return _mm_cvtepi64_epi32(__A); } @@ -7168,7 +7174,8 @@ __m128i test_mm_cvtepi64_epi16(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi64_epi16 - // CHECK: @llvm.x86.avx512.mask.pmov.qw.128 + // CHECK: trunc <2 x i64> %{{.*}} to <2 x i16> + // CHECK: shufflevector <2 x i16> %{{.*}}, <2 x i16> %{{.*}}, <8 x i32> return _mm_cvtepi64_epi16(__A); } @@ -7192,7 +7199,8 @@ __m128i test_mm256_cvtepi64_epi16(__m256i __A) { // CHECK-LABEL: @test_mm256_cvtepi64_epi16 - // CHECK: @llvm.x86.avx512.mask.pmov.qw.256 + // CHECK: trunc <4 x i64> %{{.*}} to <4 x i16> + // CHECK: shufflevector <4 x i16> %{{.*}}, <4 x i16> %{{.*}}, <8 x i32> return _mm256_cvtepi64_epi16(__A); } Index: clang/lib/Headers/avx512vlintrin.h === --- clang/lib/Headers/avx512vlintrin.h +++ clang/lib/Headers/avx512vlintrin.h @@ -30,6 +30,10 @@ #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("avx512vl"))) +typedef short __v2hi __attribute__((__vector_size__(4))); +typedef char __v4qi __attribute__((__vector_size__(4))); +typedef char __v2qi __attribute__((__vector_size__(2))); + /* Integer compare */ #define _mm_cmpeq_epi32_mask(A, B) \ @@ -7412,9 +7416,9 @@ static __inline__ __m128i __DEFAULT_FN_ATTRS _mm_cvtepi32_epi8 (__m128i __A) { - return (__m128i) __builtin_ia32_pmovdb128_mask ((__v4si) __A, - (__v16qi)_mm_undefined_si128(), - (__mmask8) -1); + return (__m128i)__builtin_shufflevector( + __builtin_convertvector((__v4si)__A, __v4qi), (__v4qi){0, 0, 0, 0}, 0, 1, + 2, 3, 4, 5, 6, 7, 7, 7, 7, 7, 7, 7, 7, 7); } static __inline__ __m128i __DEFAULT_FN_ATTRS @@ -7442,9 +7446,10 @@ static __inline__ __m128i __DEFAULT_FN_ATTRS _mm256_cvtepi32_epi8 (__m256i __A) { - return (__m128i) __builtin_ia32_pmovdb256_mask ((__v8si) __A, - (__v16qi)_mm_undefined_si128(), - (__mmask8) -1); + return (__m128i)__builtin_shufflevector( +
[PATCH] D48574: OpenBSD driver needs ld.lld in sanitiser context
devnexen added a comment. ping :) Repository: rC Clang https://reviews.llvm.org/D48574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly
JonasToth updated this revision to Diff 153280. JonasToth added a comment. - remove bad code snippet which was dead Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48714 Files: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp test/clang-tidy/hicpp-exception-baseclass.cpp Index: test/clang-tidy/hicpp-exception-baseclass.cpp === --- test/clang-tidy/hicpp-exception-baseclass.cpp +++ test/clang-tidy/hicpp-exception-baseclass.cpp @@ -2,6 +2,7 @@ namespace std { class exception {}; +class invalid_argument : public exception {}; } // namespace std class derived_exception : public std::exception {}; @@ -36,38 +37,38 @@ try { throw non_derived_exception(); // CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception' -// CHECK-MESSAGES: 9:1: note: type defined here +// CHECK-MESSAGES: 10:1: note: type defined here } catch (non_derived_exception &e) { } throw non_derived_exception(); // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception' - // CHECK-MESSAGES: 9:1: note: type defined here + // CHECK-MESSAGES: 10:1: note: type defined here // FIXME: More complicated kinds of inheritance should be checked later, but there is // currently no way use ASTMatchers for this kind of task. #if 0 // Handle private inheritance cases correctly. try { throw bad_inheritance(); // CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception' -// CHECK MESSAGES: 10:1: note: type defined here +// CHECK MESSAGES: 11:1: note: type defined here throw no_good_inheritance(); // CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception' -// CHECK MESSAGES: 11:1: note: type defined here +// CHECK MESSAGES: 12:1: note: type defined here throw really_creative(); // CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception' -// CHECK MESSAGES: 12:1: note: type defined here +// CHECK MESSAGES: 13:1: note: type defined here } catch (...) { } throw bad_inheritance(); // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception' - // CHECK MESSAGES: 10:1: note: type defined here + // CHECK MESSAGES: 11:1: note: type defined here throw no_good_inheritance(); // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception' - // CHECK MESSAGES: 11:1: note: type defined here + // CHECK MESSAGES: 12:1: note: type defined here throw really_creative(); // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception' - // CHECK MESSAGES: 12:1: note: type defined here + // CHECK MESSAGES: 13:1: note: type defined here #endif } @@ -91,7 +92,7 @@ throw deep_hierarchy(); // Ok try { -throw terrible_idea(); // Ok, but multiple inheritance isn't clean +throw terrible_idea(); // Ok, but multiple inheritance isn't clean } catch (std::exception &e) { // Can be caught as std::exception, even with multiple inheritance } throw terrible_idea(); // Ok, but multiple inheritance @@ -101,14 +102,14 @@ template void ThrowException() { throw T(); } // CHECK-MESSAGES: [[@LINE-1]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception' -// CHECK-MESSAGES: 120:1: note: type defined here +// CHECK-MESSAGES: 121:1: note: type defined here // CHECK-MESSAGES: [[@LINE-3]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception' -// CHECK-MESSAGES: 120:1: note: type defined here +// CHECK-MESSAGES: 121:1: note: type defined here // CHECK-MESSAGES: [[@LINE-5]]:31: warning: throwing an exception whose type 'exotic_exception' is not derived from 'std::exception' -// CHECK-MESSAGES: 123:1: note: type defined here +// CHECK-MESSAGES: 124:1: note: type defined here // CHECK-MESSAGES: [[@LINE-7]]:31: warning: throwing an exception whose type 'int' is not derived from 'std::exception' // CHECK-MESSAGES: [[@LINE-8]]:31: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception' -// CHECK-MESSAGES: 9:1: note: type defined here +// CHECK-MESSAGES: 10:1: note: type defined here #define THROW_EXCEPTION(CLASS) ThrowException() #define THROW_BAD_EXCEPTION throw int(42); #define THROW_GOOD_EXCEPTION throw std::exception(); @@ -128,7 +129,7 @@ // CHECK MESSAGES: [[@LINE-1]]:3: warning: throwing an exception whose type 'int' i
[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly
JonasToth created this revision. JonasToth added reviewers: aaron.ballman, alexfh, hokein, ilya-biryukov. Herald added subscribers: cfe-commits, xazax.hun. JonasToth updated this revision to Diff 153280. JonasToth added a comment. - remove bad code snippet which was dead PR37913 documents wrong behaviour for a templated exception factory function. The check does misidentify dependent types as not derived from std::exception. The fix to this problem is to ignore dependent types, the analysis works correctly on the instantiated function. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48714 Files: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp test/clang-tidy/hicpp-exception-baseclass.cpp Index: test/clang-tidy/hicpp-exception-baseclass.cpp === --- test/clang-tidy/hicpp-exception-baseclass.cpp +++ test/clang-tidy/hicpp-exception-baseclass.cpp @@ -2,6 +2,7 @@ namespace std { class exception {}; +class invalid_argument : public exception {}; } // namespace std class derived_exception : public std::exception {}; @@ -36,38 +37,38 @@ try { throw non_derived_exception(); // CHECK-MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception' -// CHECK-MESSAGES: 9:1: note: type defined here +// CHECK-MESSAGES: 10:1: note: type defined here } catch (non_derived_exception &e) { } throw non_derived_exception(); // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception' - // CHECK-MESSAGES: 9:1: note: type defined here + // CHECK-MESSAGES: 10:1: note: type defined here // FIXME: More complicated kinds of inheritance should be checked later, but there is // currently no way use ASTMatchers for this kind of task. #if 0 // Handle private inheritance cases correctly. try { throw bad_inheritance(); // CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception' -// CHECK MESSAGES: 10:1: note: type defined here +// CHECK MESSAGES: 11:1: note: type defined here throw no_good_inheritance(); // CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception' -// CHECK MESSAGES: 11:1: note: type defined here +// CHECK MESSAGES: 12:1: note: type defined here throw really_creative(); // CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception' -// CHECK MESSAGES: 12:1: note: type defined here +// CHECK MESSAGES: 13:1: note: type defined here } catch (...) { } throw bad_inheritance(); // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception' - // CHECK MESSAGES: 10:1: note: type defined here + // CHECK MESSAGES: 11:1: note: type defined here throw no_good_inheritance(); // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception' - // CHECK MESSAGES: 11:1: note: type defined here + // CHECK MESSAGES: 12:1: note: type defined here throw really_creative(); // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception' - // CHECK MESSAGES: 12:1: note: type defined here + // CHECK MESSAGES: 13:1: note: type defined here #endif } @@ -91,7 +92,7 @@ throw deep_hierarchy(); // Ok try { -throw terrible_idea(); // Ok, but multiple inheritance isn't clean +throw terrible_idea(); // Ok, but multiple inheritance isn't clean } catch (std::exception &e) { // Can be caught as std::exception, even with multiple inheritance } throw terrible_idea(); // Ok, but multiple inheritance @@ -101,14 +102,14 @@ template void ThrowException() { throw T(); } // CHECK-MESSAGES: [[@LINE-1]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception' -// CHECK-MESSAGES: 120:1: note: type defined here +// CHECK-MESSAGES: 121:1: note: type defined here // CHECK-MESSAGES: [[@LINE-3]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception' -// CHECK-MESSAGES: 120:1: note: type defined here +// CHECK-MESSAGES: 121:1: note: type defined here // CHECK-MESSAGES: [[@LINE-5]]:31: warning: throwing an exception whose type 'exotic_exception' is not derived from 'std::exception' -// CHECK-MESSAGES: 123:1: note: type defined here +// CHECK-MESSAGES: 124:1: note: type defined here // CHECK-MESSAGES: [[@LINE-7]]:31: warning: throwing an exception whose type 'int' is not derived from 'std::exception' // CHECK-MESSAGES: [[@LINE-8]]:31: warning: throwing an exce
[PATCH] D48715: [X86] Fix some vector cmp builtins - TRUE/FALSE predicates
GBuella created this revision. GBuella added reviewers: craig.topper, uriel.k, RKSimon, andrew.w.kaylor, spatel, scanon, efriedma. Herald added a subscriber: cfe-commits. This patch removes on optimization used with the TRUE/FALSE predicates, as was suggested in https://reviews.llvm.org/D45616 for r335339. The optimization was buggy, since r335339 used it also for *_mask builtins, without actually applying the mask -- the mask argument was just ignored. Repository: rC Clang https://reviews.llvm.org/D48715 Files: lib/CodeGen/CGBuiltin.cpp test/CodeGen/avx-builtins.c test/CodeGen/avx512f-builtins.c test/CodeGen/avx512vl-builtins.c Index: test/CodeGen/avx512vl-builtins.c === --- test/CodeGen/avx512vl-builtins.c +++ test/CodeGen/avx512vl-builtins.c @@ -1077,34 +1077,6 @@ return (__mmask8)_mm256_cmp_ps_mask(__A, __B, 0); } -__mmask8 test_mm256_cmp_ps_mask_true_uq(__m256 __A, __m256 __B) { - // CHECK-LABEL: @test_mm256_cmp_ps_mask_true_uq - // CHECK-NOT: call - // CHECK: ret i8 -1 - return (__mmask8)_mm256_cmp_ps_mask(__A, __B, _CMP_TRUE_UQ); -} - -__mmask8 test_mm256_cmp_ps_mask_true_us(__m256 __A, __m256 __B) { - // CHECK-LABEL: @test_mm256_cmp_ps_mask_true_us - // CHECK-NOT: call - // CHECK: ret i8 -1 - return (__mmask8)_mm256_cmp_ps_mask(__A, __B, _CMP_TRUE_US); -} - -__mmask8 test_mm256_cmp_ps_mask_false_oq(__m256 __A, __m256 __B) { - // CHECK-LABEL: @test_mm256_cmp_ps_mask_false_oq - // CHECK-NOT: call - // CHECK: ret i8 0 - return (__mmask8)_mm256_cmp_ps_mask(__A, __B, _CMP_FALSE_OQ); -} - -__mmask8 test_mm256_cmp_ps_mask_false_os(__m256 __A, __m256 __B) { - // CHECK-LABEL: @test_mm256_cmp_ps_mask_false_os - // CHECK-NOT: call - // CHECK: ret i8 0 - return (__mmask8)_mm256_cmp_ps_mask(__A, __B, _CMP_FALSE_OS); -} - __mmask8 test_mm256_mask_cmp_ps_mask(__mmask8 m, __m256 __A, __m256 __B) { // CHECK-LABEL: @test_mm256_mask_cmp_ps_mask // CHECK: fcmp oeq <8 x float> %{{.*}}, %{{.*}} @@ -1118,34 +1090,6 @@ return (__mmask8)_mm_cmp_ps_mask(__A, __B, 0); } -__mmask8 test_mm_cmp_ps_mask_true_uq(__m128 __A, __m128 __B) { - // CHECK-LABEL: @test_mm_cmp_ps_mask_true_uq - // CHECK-NOT: call - // CHECK: ret i8 -1 - return (__mmask8)_mm_cmp_ps_mask(__A, __B, _CMP_TRUE_UQ); -} - -__mmask8 test_mm_cmp_ps_mask_true_us(__m128 __A, __m128 __B) { - // CHECK-LABEL: @test_mm_cmp_ps_mask_true_us - // CHECK-NOT: call - // CHECK: ret i8 -1 - return (__mmask8)_mm_cmp_ps_mask(__A, __B, _CMP_TRUE_US); -} - -__mmask8 test_mm_cmp_ps_mask_false_oq(__m128 __A, __m128 __B) { - // CHECK-LABEL: @test_mm_cmp_ps_mask_false_oq - // CHECK-NOT: call - // CHECK: ret i8 0 - return (__mmask8)_mm_cmp_ps_mask(__A, __B, _CMP_FALSE_OQ); -} - -__mmask8 test_mm_cmp_ps_mask_false_os(__m128 __A, __m128 __B) { - // CHECK-LABEL: @test_mm_cmp_ps_mask_false_os - // CHECK-NOT: call - // CHECK: ret i8 0 - return (__mmask8)_mm_cmp_ps_mask(__A, __B, _CMP_FALSE_OS); -} - __mmask8 test_mm_mask_cmp_ps_mask(__mmask8 m, __m128 __A, __m128 __B) { // CHECK-LABEL: @test_mm_mask_cmp_ps_mask // CHECK: fcmp oeq <4 x float> %{{.*}}, %{{.*}} @@ -1160,34 +1104,6 @@ return (__mmask8)_mm256_cmp_pd_mask(__A, __B, 0); } -__mmask8 test_mm256_cmp_pd_mask_true_uq(__m256d __A, __m256d __B) { - // CHECK-LABEL: @test_mm256_cmp_pd_mask_true_uq - // CHECK-NOT: call - // CHECK: ret i8 -1 - return (__mmask8)_mm256_cmp_pd_mask(__A, __B, _CMP_TRUE_UQ); -} - -__mmask8 test_mm256_cmp_pd_mask_true_us(__m256d __A, __m256d __B) { - // CHECK-LABEL: @test_mm256_cmp_pd_mask_true_us - // CHECK-NOT: call - // CHECK: ret i8 -1 - return (__mmask8)_mm256_cmp_pd_mask(__A, __B, _CMP_TRUE_US); -} - -__mmask8 test_mm256_cmp_pd_mask_false_oq(__m256d __A, __m256d __B) { - // CHECK-LABEL: @test_mm256_cmp_pd_mask_false_oq - // CHECK-NOT: call - // CHECK: ret i8 0 - return (__mmask8)_mm256_cmp_pd_mask(__A, __B, _CMP_FALSE_OQ); -} - -__mmask8 test_mm256_cmp_pd_mask_false_os(__m256d __A, __m256d __B) { - // CHECK-LABEL: @test_mm256_cmp_pd_mask_false_os - // CHECK-NOT: call - // CHECK: ret i8 0 - return (__mmask8)_mm256_cmp_pd_mask(__A, __B, _CMP_FALSE_OS); -} - __mmask8 test_mm256_mask_cmp_pd_mask(__mmask8 m, __m256d __A, __m256d __B) { // CHECK-LABEL: @test_mm256_mask_cmp_pd_mask // CHECK: fcmp oeq <4 x double> %{{.*}}, %{{.*}} @@ -1202,34 +1118,6 @@ return (__mmask8)_mm_cmp_pd_mask(__A, __B, 0); } -__mmask8 test_mm_cmp_pd_mask_true_uq(__m128d __A, __m128d __B) { - // CHECK-LABEL: @test_mm_cmp_pd_mask_true_uq - // CHECK-NOT: call - // CHECK: ret i8 -1 - return (__mmask8)_mm_cmp_pd_mask(__A, __B, _CMP_TRUE_UQ); -} - -__mmask8 test_mm_cmp_pd_mask_true_us(__m128d __A, __m128d __B) { - // CHECK-LABEL: @test_mm_cmp_pd_mask_true_us - // CHECK-NOT: call - // CHECK: ret i8 -1 - return (__mmask8)_mm_cmp_pd_mask(__A, __B, _CMP_TRUE_US); -} - -__mmask8 test_mm_cmp_pd_mask_false_oq(__m128d __A, __m128d __B) { - // CHECK-LABEL: @test_mm_cmp_pd_mas
[PATCH] D45444: [clang-tidy] implement new check for const-correctness
JonasToth updated this revision to Diff 153282. JonasToth added a comment. - Merge branch 'master' into check_const Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/ConstCheck.cpp clang-tidy/cppcoreguidelines/ConstCheck.h clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp docs/ReleaseNotes.rst docs/clang-tidy/checks/cppcoreguidelines-const.rst docs/clang-tidy/checks/list.rst test/clang-tidy/cppcoreguidelines-const-pointers-as-values.cpp test/clang-tidy/cppcoreguidelines-const-values.cpp Index: test/clang-tidy/cppcoreguidelines-const-values.cpp === --- /dev/null +++ test/clang-tidy/cppcoreguidelines-const-values.cpp @@ -0,0 +1,557 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-const %t + +// --- Provide test samples for primitive builtins - +// - every 'p_*' variable is a 'potential_const_*' variable +// - every 'np_*' variable is a 'non_potential_const_*' variable + +bool global; +char np_global = 0; // globals can't be known to be const + +namespace foo { +int scoped; +float np_scoped = 1; // namespace variables are like globals +} // namespace foo + +void some_function(double, wchar_t); + +void some_function(double np_arg0, wchar_t np_arg1) { + int p_local0 = 2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + + int np_local0; + const int np_local1 = 42; + + unsigned int np_local2 = 3; + np_local2 <<= 4; + + int np_local3 = 4; + ++np_local3; + int np_local4 = 4; + np_local4++; + + int np_local5 = 4; + --np_local5; + int np_local6 = 4; + np_local6--; +} + +void nested_scopes() { + int p_local0 = 2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + int np_local0 = 42; + + { +int p_local1 = 42; +// CHECK-MESSAGES: [[@LINE-1]]:5: warning: variable 'p_local1' of type 'int' can be declared const +np_local0 *= 2; + } +} + +void some_lambda_environment_capture_all_by_reference(double np_arg0) { + int np_local0 = 0; + int p_local0 = 1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + + int np_local2; + const int np_local3 = 2; + + // Capturing all variables by reference prohibits making them const. + [&]() { ++np_local0; }; + + int p_local1 = 0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const +} + +void some_lambda_environment_capture_all_by_value(double np_arg0) { + int np_local0 = 0; + int p_local0 = 1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + + int np_local1; + const int np_local2 = 2; + + // Capturing by value has no influence on them. + [=]() { (void)p_local0; }; + + np_local0 += 10; +} + +void function_inout_pointer(int *inout); +void function_in_pointer(const int *in); + +void some_pointer_taking(int *out) { + int np_local0 = 42; + const int *const p0_np_local0 = &np_local0; + int *const p1_np_local0 = &np_local0; + + int np_local1 = 42; + const int *const p0_np_local1 = &np_local1; + int *const p1_np_local1 = &np_local1; + *p1_np_local0 = 43; + + int np_local2 = 42; + function_inout_pointer(&np_local2); + + // Prevents const. + int np_local3 = 42; + out = &np_local3; // This returns and invalid address, its just about the AST + + int p_local1 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const + const int *const p0_p_local1 = &p_local1; + + int p_local2 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int' can be declared const + function_in_pointer(&p_local2); +} + +void function_inout_ref(int &inout); +void function_in_ref(const int &in); + +void some_reference_taking() { + int np_local0 = 42; + const int &r0_np_local0 = np_local0; + int &r1_np_local0 = np_local0; + r1_np_local0 = 43; + const int &r2_np_local0 = r1_np_local0; + + int np_local1 = 42; + function_inout_ref(np_local1); + + int p_local0 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + const int &r0_p_local0 = p_local0; + + int p_local1 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const + function_in_ref(p_local1); +} + +double *non_const_pointer_return() { + double p_local0 = 0.0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const + double np_local0 = 24.4; + + return &np_local0; +} + +const double *const_pointer_return() { + double p_local0 = 0.0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const + double p_local1 = 24.4; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: varia
[PATCH] D48661: [Fixed Point Arithmetic] Fixed Point Constant
ebevhan added inline comments. Comment at: include/clang/Basic/FixedPoint.h:23 + +class FixedPointNumber { + public: rjmccall wrote: > The established naming convention here — as seen in `APInt`, `APFloat`, > `APValue`, etc. — would call this `APFixedPoint`. Maybe that's not a great > convention, but we should at least discuss deviating from it. > > You might also want a type which encapsulates the details of a fixed-point > type, i.e. the semantic width, scale, and saturating-ness. (Like the "float > semantics" of `APFloat`.) > > I think a key question here is whether you want a FixedPointNumber to exactly > represent the bit-pattern or just the semantic value. I think it would > eliminate a non-trivial source of bugs in this type if it just represents the > semantic value, i.e. if a 16-bit unsigned fract value on a target where that > uses a padded representation did not explicitly represent the padding bit, > and then just added it back in in some accessor that asks for the > bit-pattern. Regardless, that should be clearly documented. > a 16-bit unsigned fract value on a target where that uses a padded > representation did not explicitly represent the padding bit So does that mean that the underlying APInt in this type would be 15 bits instead of 16, to avoid representing the padding? It feels a bit scary to throw around values with different internal representation than what other parts of the code (say, the target specification) have specified them to be. Comment at: include/clang/Basic/FixedPoint.h:45 + + FixedPointNumber extend(unsigned Width) const { +llvm::APSInt ValCpy = Val_; I'm not so sure that extension and truncation on their own are particularly meaningful operations on fixed-point values. I think you need to provide both a width and a scale for these kinds of operations so you can both resize and rescale the value simultaneously. Perhaps you can even provide a 'saturation width' that the value should be saturating on when converting. You have the `convert` method, but it only takes a QualType, so if you need to convert to something that doesn't exist as a type, it's not enough. Maybe I'm overdesigning. Comment at: include/clang/Basic/FixedPoint.h:57 + + llvm::APSInt getIntPart() const { return Val_ >> Scale_; } + As with the integer conversion in the earlier patch, this does not round toward zero. This might lead to surprising results. Comment at: lib/AST/ASTContext.cpp:10303 + if (Ty->isSaturatedFixedPointType()) { +FixedPointNumber MaxVal = Context.getFixedPointMax(Ty); +FixedPointNumber MinVal = Context.getFixedPointMin(Ty); It is possible to perform saturation without extending and comparing, by looking at the bits above the 'imagined' sign bit in the resulting type. If they are all the same, then the value is in range, otherwise you must saturate. Explicit comparison probably gets the point across better. Comment at: lib/AST/ASTContext.cpp:10320 + + if (DstWidth > SrcWidth) Val_ = Val_.extend(DstWidth); + If you do rescaling before and after resizing, you can use `extOrTrunc` instead. Comment at: lib/AST/ASTContext.cpp:10342 + + unsigned CommonWidth = std::max(Val_.getBitWidth(), OtherWidth); + ThisVal = ThisVal.extend(CommonWidth); This width and scale commoning looks odd. If you have two types for which the width is the same but the scale is different, you will discard bits from the value with lower scale and the comparison might be wrong. You need to find a width and scale that can fit all of the values of both. Repository: rC Clang https://reviews.llvm.org/D48661 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47814: Teach libc++ to use native NetBSD's max_align_t
krytarowski added a comment. I saw this and I think that it shall be handled in libc++. NetBSD doesn't care if c++ runtime library is libstdc++, libc++, none or a different one. Repository: rL LLVM https://reviews.llvm.org/D47814 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48626: New option -fwindows-filesystem, affecting #include paths.
mstorsjo added a comment. In https://reviews.llvm.org/D48626#1146282, @simon_tatham wrote: > Although, come to think of it, that's not good enough, because if you have > multiple directories on your include //path// then you expect a lot of > lookups to fail for reasons that have nothing to do with case. Say, if you > compile with `-Ifoo -Ibar`, then every include of a file intended to be found > in `bar` will force lots of pointless directory enumeration in `foo` when the > initial lookup there doesn't find the file. Hmmm. That suggests to me that > perhaps this ought not to be a //global// `-ffudge-my-paths` type option > applying to all include directories, and instead perhaps it should be a new > kind of `-I` option, so that only lookups relative to that particular > include-path directory would get the unusual semantics. What do people think? The cases where I would have needed this (the windows sdk), I'm not specifying that path explicitly via `-I` directives, but implicitly via the `INCLUDE` env variable, but perhaps limiting this to the directories known to have problematic case could be helpful performance wise. Otherwise I guess this would need to go on a higher level in the algorithm; first check all directories in a fully case sensitive manner, just like today. And if that fails (and we'd have a real failure that we'd otherwise return to the caller), redo it all with case insensitivity. I think that would be the best performance wise, but that makes for much messier integration... Repository: rC Clang https://reviews.llvm.org/D48626 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48626: New option -fwindows-filesystem, affecting #include paths.
simon_tatham added a comment. > first check all directories in a fully case sensitive manner, just like > today. And if that fails (and we'd have a real failure that we'd otherwise > return to the caller), redo it all with case insensitivity. I agree that the integration would be a bigger headache doing it that way. It would also depart from the expected semantics – a correctly cased header file in an include directory late on the path would override an incorrectly cased one earlier, which isn't how the real Windows compiler would handle the same situation. > The cases where I would have needed this (the windows sdk), I'm not > specifying that path explicitly via -I directives, but implicitly via the > INCLUDE env variable *blush* I //ought// to have thought of that, because I've noticed the same problem! > but perhaps limiting this to the directories known to have problematic case > could be helpful performance wise. Yes. OK, in that case I think my proposed replacement design is to have an option along the lines of `-fwindows-paths=`//prefix// (spelling still open to change), with the effect that only pathname lookups starting with that prefix will be subject to this transformation. Repository: rC Clang https://reviews.llvm.org/D48626 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r335834 - [DebugInfo] Follow-up commit to improve consistency. NFC
Author: jdevlieghere Date: Thu Jun 28 03:56:40 2018 New Revision: 335834 URL: http://llvm.org/viewvc/llvm-project?rev=335834&view=rev Log: [DebugInfo] Follow-up commit to improve consistency. NFC Follow-up commit for r335757 to address some inconsistencies. Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp cfe/trunk/lib/CodeGen/CGDebugInfo.h Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=335834&r1=335833&r2=335834&view=diff == --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original) +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Jun 28 03:56:40 2018 @@ -289,8 +289,7 @@ StringRef CGDebugInfo::getObjCMethodName << OC->getIdentifier()->getNameStart() << ')'; } } else if (const auto *OCD = dyn_cast(DC)) { -OS << OCD->getClassInterface()->getName() << '(' - << OCD->getName() << ')'; +OS << OCD->getClassInterface()->getName() << '(' << OCD->getName() << ')'; } else if (isa(DC)) { // We can extract the type of the class from the self pointer. if (ImplicitParamDecl *SelfDecl = OMD->getSelfDecl()) { @@ -385,7 +384,8 @@ CGDebugInfo::computeChecksum(FileID FID, return llvm::DIFile::CSK_MD5; } -Optional CGDebugInfo::getSource(const SourceManager &SM, FileID FID) { +Optional CGDebugInfo::getSource(const SourceManager &SM, + FileID FID) { if (!CGM.getCodeGenOpts().EmbedSource) return None; @@ -412,11 +412,11 @@ llvm::DIFile *CGDebugInfo::getOrCreateFi // Cache the results. const char *fname = PLoc.getFilename(); - auto it = DIFileCache.find(fname); + auto It = DIFileCache.find(fname); - if (it != DIFileCache.end()) { + if (It != DIFileCache.end()) { // Verify that the information still exists. -if (llvm::Metadata *V = it->second) +if (llvm::Metadata *V = It->second) return cast(V); } @@ -427,10 +427,9 @@ llvm::DIFile *CGDebugInfo::getOrCreateFi if (CSKind) CSInfo.emplace(*CSKind, Checksum); - llvm::DIFile *F = DBuilder.createFile(remapDIPath(PLoc.getFilename()), -remapDIPath(getCurrentDirname()), -CSInfo, -getSource(SM, SM.getFileID(Loc))); + llvm::DIFile *F = DBuilder.createFile( + remapDIPath(PLoc.getFilename()), remapDIPath(getCurrentDirname()), CSInfo, + getSource(SM, SM.getFileID(Loc))); DIFileCache[fname].reset(F); return F; @@ -438,8 +437,7 @@ llvm::DIFile *CGDebugInfo::getOrCreateFi llvm::DIFile *CGDebugInfo::getOrCreateMainFile() { return DBuilder.createFile( - remapDIPath(TheCU->getFilename()), - remapDIPath(TheCU->getDirectory()), + remapDIPath(TheCU->getFilename()), remapDIPath(TheCU->getDirectory()), TheCU->getFile()->getChecksum(), CGM.getCodeGenOpts().EmbedSource ? TheCU->getSource() : None); } @@ -574,8 +572,7 @@ void CGDebugInfo::CreateCompileUnit() { TheCU = DBuilder.createCompileUnit( LangTag, DBuilder.createFile(remapDIPath(MainFileName), - remapDIPath(getCurrentDirname()), - CSInfo, + remapDIPath(getCurrentDirname()), CSInfo, getSource(SM, SM.getMainFileID())), CGOpts.EmitVersionIdentMetadata ? Producer : "", LO.Optimize || CGOpts.PrepareForLTO || CGOpts.PrepareForThinLTO, @@ -640,14 +637,13 @@ llvm::DIType *CGDebugInfo::CreateType(co return SelTy; } -#define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \ - case BuiltinType::Id: \ -return getOrCreateStructPtrType("opencl_" #ImgType "_" #Suffix "_t", \ +#define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \ + case BuiltinType::Id: \ +return getOrCreateStructPtrType("opencl_" #ImgType "_" #Suffix "_t", \ SingletonId); #include "clang/Basic/OpenCLImageTypes.def" case BuiltinType::OCLSampler: -return getOrCreateStructPtrType("opencl_sampler_t", -OCLSamplerDITy); +return getOrCreateStructPtrType("opencl_sampler_t", OCLSamplerDITy); case BuiltinType::OCLEvent: return getOrCreateStructPtrType("opencl_event_t", OCLEventDITy); case BuiltinType::OCLClkEvent: @@ -830,8 +826,7 @@ static bool hasCXXMangling(const TagDecl } // Determines if the tag declaration will require a type identifier. -static bool needsTypeIdentifier(const TagDecl *TD, -CodeGenModule& CGM, +static bool needsTypeIdentifier(const TagDecl *TD, CodeGenModule &CGM, llvm::DICompileUnit *TheCU) { // We only add a type identifier for types with C++ name mangling. if (!hasCXXMangling(TD, TheCU)) @@
r335835 - [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks
Author: baloghadamsoftware Date: Thu Jun 28 03:58:53 2018 New Revision: 335835 URL: http://llvm.org/viewvc/llvm-project?rev=335835&view=rev Log: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks Add handling of the begin() funcion of containers to the iterator checkers, together with the pre- and postfix ++ and -- operators of the iterators. This makes possible the checking of iterators dereferenced ahead of the begin of the container. Differential Revision: https://reviews.llvm.org/D32642 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp cfe/trunk/test/Analysis/iterator-range.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp?rev=335835&r1=335834&r2=335835&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp Thu Jun 28 03:58:53 2018 @@ -46,19 +46,25 @@ // use setter and getters functions which separate the three cases. To store // them we use a pointer union of symbol and memory region. // -// The checker works the following way: We record the past-end iterator for -// all containers whenever their `.end()` is called. Since the Constraint -// Manager cannot handle SVals we need to take over its role. We post-check -// equality and non-equality comparisons and propagate the position of the -// iterator to the other side of the comparison if it is past-end and we are in -// the 'equal' branch (true-branch for `==` and false-branch for `!=`). +// The checker works the following way: We record the begin and the +// past-end iterator for all containers whenever their `.begin()` and `.end()` +// are called. Since the Constraint Manager cannot handle such SVals we need +// to take over its role. We post-check equality and non-equality comparisons +// and record that the two sides are equal if we are in the 'equal' branch +// (true-branch for `==` and false-branch for `!=`). // // In case of type-I or type-II iterators we get a concrete integer as a result // of the comparison (1 or 0) but in case of type-III we only get a Symbol. In // this latter case we record the symbol and reload it in evalAssume() and do // the propagation there. We also handle (maybe double) negated comparisons -// which are represented in the form of (x == 0 or x !=0 ) where x is the +// which are represented in the form of (x == 0 or x != 0) where x is the // comparison itself. +// +// Since `SimpleConstraintManager` cannot handle complex symbolic expressions +// we only use expressions of the format S, S+n or S-n for iterator positions +// where S is a conjured symbol and n is an unsigned concrete integer. When +// making an assumption e.g. `S1 + n == S2 + m` we store `S1 - S2 == m - n` as +// a constraint which we later retrieve when doing an actual comparison. #include "ClangSACheckers.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" @@ -80,7 +86,7 @@ private: const MemRegion *Cont; // Abstract offset - SymbolRef Offset; + const SymbolRef Offset; IteratorPosition(const MemRegion *C, SymbolRef Of) : Cont(C), Offset(Of) {} @@ -113,31 +119,39 @@ public: typedef llvm::PointerUnion RegionOrSymbol; -// Structure to record the symbolic end position of a container +// Structure to record the symbolic begin and end position of a container struct ContainerData { private: - SymbolRef End; + const SymbolRef Begin, End; - ContainerData(SymbolRef E) : End(E) {} + ContainerData(SymbolRef B, SymbolRef E) : Begin(B), End(E) {} public: + static ContainerData fromBegin(SymbolRef B) { +return ContainerData(B, nullptr); + } + static ContainerData fromEnd(SymbolRef E) { -return ContainerData(E); +return ContainerData(nullptr, E); } + SymbolRef getBegin() const { return Begin; } SymbolRef getEnd() const { return End; } - ContainerData newEnd(SymbolRef E) const { return ContainerData(E); } + ContainerData newBegin(SymbolRef B) const { return ContainerData(B, End); } + + ContainerData newEnd(SymbolRef E) const { return ContainerData(Begin, E); } bool operator==(const ContainerData &X) const { -return End == X.End; +return Begin == X.Begin && End == X.End; } bool operator!=(const ContainerData &X) const { -return End != X.End; +return Begin != X.Begin || End != X.End; } void Profile(llvm::FoldingSetNodeID &ID) const { +ID.Add(Begin); ID.Add(End); } }; @@ -167,8 +181,9 @@ public: class IteratorChecker : public Checker, check::PostStmt, - check::DeadSymbols, +
[PATCH] D32642: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks
This revision was automatically updated to reflect the committed changes. Closed by commit rL335835: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead… (authored by baloghadamsoftware, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D32642?vs=153023&id=153287#toc Repository: rL LLVM https://reviews.llvm.org/D32642 Files: cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp cfe/trunk/test/Analysis/iterator-range.cpp Index: cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp === --- cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp +++ cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp @@ -19,6 +19,6 @@ void testCopyNull(C *I, C *E) { std::copy(I, E, (C *)0); #ifndef SUPPRESSED - // expected-warning@../Inputs/system-header-simulator-cxx.h:490 {{Called C++ object pointer is null}} + // expected-warning@../Inputs/system-header-simulator-cxx.h:514 {{Called C++ object pointer is null}} #endif } Index: cfe/trunk/test/Analysis/iterator-range.cpp === --- cfe/trunk/test/Analysis/iterator-range.cpp +++ cfe/trunk/test/Analysis/iterator-range.cpp @@ -1,5 +1,5 @@ -// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config c++-container-inlining=false %s -verify -// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify +// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=false %s -verify +// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify #include "Inputs/system-header-simulator-cxx.h" @@ -13,7 +13,110 @@ } } +void simple_good_end_negated(const std::vector &v) { + auto i = v.end(); + if (!(i == v.end())) { +clang_analyzer_warnIfReached(); +*i; // no-warning + } +} + void simple_bad_end(const std::vector &v) { auto i = v.end(); *i; // expected-warning{{Iterator accessed outside of its range}} } + +void simple_good_begin(const std::vector &v) { + auto i = v.begin(); + if (i != v.begin()) { +clang_analyzer_warnIfReached(); +*--i; // no-warning + } +} + +void simple_good_begin_negated(const std::vector &v) { + auto i = v.begin(); + if (!(i == v.begin())) { +clang_analyzer_warnIfReached(); +*--i; // no-warning + } +} + +void simple_bad_begin(const std::vector &v) { + auto i = v.begin(); + *--i; // expected-warning{{Iterator accessed outside of its range}} +} + +void copy(const std::vector &v) { + auto i1 = v.end(); + auto i2 = i1; + *i2; // expected-warning{{Iterator accessed outside of its range}} +} + +void decrease(const std::vector &v) { + auto i = v.end(); + --i; + *i; // no-warning +} + +void copy_and_decrease1(const std::vector &v) { + auto i1 = v.end(); + auto i2 = i1; + --i1; + *i1; // no-warning +} + +void copy_and_decrease2(const std::vector &v) { + auto i1 = v.end(); + auto i2 = i1; + --i1; + *i2; // expected-warning{{Iterator accessed outside of its range}} +} + +void copy_and_increase1(const std::vector &v) { + auto i1 = v.begin(); + auto i2 = i1; + ++i1; + if (i1 == v.end()) +*i2; // no-warning +} + +void copy_and_increase2(const std::vector &v) { + auto i1 = v.begin(); + auto i2 = i1; + ++i1; + if (i2 == v.end()) +*i2; // expected-warning{{Iterator accessed outside of its range}} +} + +void copy_and_increase3(const std::vector &v) { + auto i1 = v.begin(); + auto i2 = i1; + ++i1; + if (v.end() == i2) +*i2; // expected-warning{{Iterator accessed outside of its range}} +} + +void tricky(std::vector &V, int e) { + const auto first = V.begin(); + const auto comp1 = (first != V.end()), comp2 = (first == V.end()); + if (comp1) +*first; +} + +void loop(std::vector &V, int e) { + auto start = V.begin(); + while (true) { +auto item = std::find(start, V.end(), e); +if (item == V.end()) + break; +*item; // no-warning +start = ++item; // no-warning + } +} + +void bad_move(std::list &L1, std::list &L2) { + auto i0 = --L2.cend(); + L1 = std::move(L2); + *++i0; // expected-warning{{Iterator accessed outside of its range}} +} Index: cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h =
[clang-tools-extra] r335836 - [clangd] Fix a data race in TUScheduler
Author: ibiryukov Date: Thu Jun 28 04:04:45 2018 New Revision: 335836 URL: http://llvm.org/viewvc/llvm-project?rev=335836&view=rev Log: [clangd] Fix a data race in TUScheduler By recomputing CompilerInvocation instead of copying it. The problem was caused by the fact that copies of CompilerInvocation store references to the shared state (DiagnosticOptions) when copied, causing data races when two different copies are destroyed from different threads. Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=335836&r1=335835&r2=335836&view=diff == --- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original) +++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Thu Jun 28 04:04:45 2018 @@ -221,8 +221,6 @@ private: Semaphore &Barrier; /// Inputs, corresponding to the current state of AST. ParseInputs FileInputs; - /// CompilerInvocation used for FileInputs. - std::unique_ptr Invocation; /// Size of the last AST /// Guards members used by both TUScheduler and the worker thread. mutable std::mutex Mutex; @@ -325,7 +323,8 @@ void ASTWorker::update(ParseInputs Input Inputs.CompileCommand.Directory + "] " + llvm::join(Inputs.CompileCommand.CommandLine, " ")); // Rebuild the preamble and the AST. -Invocation = buildCompilerInvocation(Inputs); +std::unique_ptr Invocation = +buildCompilerInvocation(Inputs); if (!Invocation) { log("Could not build CompilerInvocation for file " + FileName); return; @@ -341,8 +340,7 @@ void ASTWorker::update(ParseInputs Input } // Build the AST for diagnostics. llvm::Optional AST = -buildAST(FileName, llvm::make_unique(*Invocation), - Inputs, NewPreamble, PCHs); +buildAST(FileName, std::move(Invocation), Inputs, NewPreamble, PCHs); // We want to report the diagnostics even if this update was cancelled. // It seems more useful than making the clients wait indefinitely if they // spam us with updates. @@ -362,6 +360,8 @@ void ASTWorker::runWithAST( auto Task = [=](decltype(Action) Action) { llvm::Optional> AST = IdleASTs.take(this); if (!AST) { + std::unique_ptr Invocation = + buildCompilerInvocation(FileInputs); // Try rebuilding the AST. llvm::Optional NewAST = Invocation ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47814: Teach libc++ to use native NetBSD's max_align_t
krytarowski added a comment. It's similar to MUSL, __DEFINED_max_align_t is musl specific and they don't care about libc++ or not on top of it. Repository: rL LLVM https://reviews.llvm.org/D47814 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly
hokein added inline comments. Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:191 +void templated_thrower() { throw T{}(); } +// CHECK-MESSAGES: [[@LINE-1]]:34: warning: throwing an exception whose type 'int' is not derived from 'std::exception' + I think giving message on the template function here is confusing to users even it gets instantiated somewhere in this TU -- because users have to find the location that triggers the template instantiation. Maybe 1) Add a note which gives the instantiation location to the message, or 2) ignore template case (some clang-tidy checks do this) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48441: [clangd] Incorporate transitive #includes into code complete proximity scoring.
ioeric added inline comments. Comment at: clangd/FileDistance.cpp:9 +//===--===// +// +// Is this intentionally reserved? Comment at: clangd/FileDistance.cpp:51 +for (StringRef Rest = Canonical; !Rest.empty();) { + Rest = parent_path(Rest, sys::path::Style::posix); + auto NextHash = hash_value(Rest); nit: this probably deserve a comment or a better formatting. It looks like part of the loop incremental. Comment at: clangd/FileDistance.cpp:86 + auto Canonical = canonicalize(Path); + unsigned Cost = kUnreachable; + SmallVector Ancestors; Should we also check the cache here? Comment at: unittests/clangd/FileDistanceTests.cpp:36 + // Ancestor (up+up+up+up) + EXPECT_EQ(D.distance("/"), 22u); + // Sibling (up+down) sammccall wrote: > ioeric wrote: > > I think this is an interesting case. IIUC, 22u is contributed by 4 ups > > (4*5) and 1 include (1*2, via `llvm/ADT/StringRef.h`). > > > > Suppose `a/b/c/d/e/f.cc` includes `base/x.h`, then the shortest path into > > directory `other/` is likely to go via `base/x.h`. This might become a > > problem if `base/x.h` is very commonly used. One solution is probably to > > penalize edits that hit the root. > Yeah, this could be a weakness. I don't think this is specific to #includes, > or to root directories. > I think the generalization is that there exist directories (like > /usr/include) whose siblings are unrelated. Down-traversals in these > directories should be more expensive. I've added a Caveats section to the > FileDistance documentation, but I would like to avoid adding special cases > until we can see how much difference they make. As chatted offline, this could be still concerning as it's not uncommon for files to include headers from top-level directories. I think the idea you proposed - restricting up traversals from included files, could address the problem. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r335841 - Fix the indentation in this documentation to remove a Sphinx warning; NFC.
Author: aaronballman Date: Thu Jun 28 05:00:16 2018 New Revision: 335841 URL: http://llvm.org/viewvc/llvm-project?rev=335841&view=rev Log: Fix the indentation in this documentation to remove a Sphinx warning; NFC. Modified: cfe/trunk/include/clang/Basic/AttrDocs.td Modified: cfe/trunk/include/clang/Basic/AttrDocs.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=335841&r1=335840&r2=335841&view=diff == --- cfe/trunk/include/clang/Basic/AttrDocs.td (original) +++ cfe/trunk/include/clang/Basic/AttrDocs.td Thu Jun 28 05:00:16 2018 @@ -2946,11 +2946,10 @@ X86 Supports Indirect Branch Tracking (I Enforcement Technology (CET). IBT instruments ENDBR instructions used to specify valid targets of indirect call / jmp. The ``nocf_check`` attribute has two roles: -1. Appertains to a function - do not add ENDBR instruction at the -beginning of the function. -2. Appertains to a function pointer - do not track the target -function of this pointer (by adding nocf_check prefix to the -indirect-call instruction). +1. Appertains to a function - do not add ENDBR instruction at the beginning of +the function. +2. Appertains to a function pointer - do not track the target function of this +pointer (by adding nocf_check prefix to the indirect-call instruction). }]; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r335842 - Correct the code highlighting marker to be Objective-C rather than C++ which fixes a Sphinx build warning; NFC.
Author: aaronballman Date: Thu Jun 28 05:02:38 2018 New Revision: 335842 URL: http://llvm.org/viewvc/llvm-project?rev=335842&view=rev Log: Correct the code highlighting marker to be Objective-C rather than C++ which fixes a Sphinx build warning; NFC. Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=335842&r1=335841&r2=335842&view=diff == --- cfe/trunk/docs/ClangFormatStyleOptions.rst (original) +++ cfe/trunk/docs/ClangFormatStyleOptions.rst Thu Jun 28 05:02:38 2018 @@ -1588,7 +1588,7 @@ the configuration (without a prefix: ``A onto individual lines whenever they go over ``ColumnLimit``. - .. code-block:: c++ + .. code-block:: objc Always (or Auto, if BinPackParameters=true): @interface c () < ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response
simark added a comment. In https://reviews.llvm.org/D48687#1146308, @ilya-biryukov wrote: > Thanks for the patch! > Could we try to figure out why the duplicates were there in the first place > and why the paths were different? I tried to do that, but it goes deep in the clang internals with which I'm not familiar. All I could see was that when creating the `FileEntry` representing the `/home/emaisin/src/ls-interact/cpp-test/build/../src/first.h` file, `FileManager::getFile` is called with `OpenFile=false`. This makes it so that the `RealPathName` field is not set (at `FileManager.cpp:320`). Because `RealPathName` is not set (well, empty), we use the non-normalized name in `getAbsoluteFilePath`. That's all I can tell. > It should be easy to mock exactly the same setup you have in #37963, i.e. > create a vfs with three files and compilation database that has exactly those > compile commands. You mention you tried to repro the issue, did you try doing > that with the unit-test or a lit-test? I tried doing a unit test that mimics the setup in #37963. For some reason I can't explain, the header file would always come out "correct". If you want to investigate further, I can update the patch with what I have so far. > After looking at the makefile, my guess would be that the problem comes from > the paths starting with `../` inside the compilation database. Yes. But AFAIK it's valid (it's relative to `directory`, which is the build directory. The compile_commands.json is generated with Bear. Comment at: clangd/XRefs.cpp:179 +const SourceManager &SourceMgr, +const vfs::FileSystem &VFS) { SmallString<64> FilePath = F->tryGetRealPathName(); ilya-biryukov wrote: > Do we really need an extra vfs param? > Could we use `SourceMgr.getFileManager().getVirtualFileSystem()` instead? > > Ah probably not. I couldn't find a way to get a handle on the VFS, but that looks good. Comment at: unittests/clangd/TestTU.h:47 - ParsedAST build() const; + ParsedAST build(IntrusiveRefCntPtr *OutFS = nullptr) const; SymbolSlab headerSymbols() const; ilya-biryukov wrote: > We don't need an extra output param here. > There's a way to get the vfs from the ASTContext: > `ParsedAST().getASTContext().getSourceManager().getFileManager().getVirtualFileSystem()`. Thanks. It's just not obvious :). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r335799 - Fixup test to compile with -frtti when trying to use typeid() as the PS4 does not have it on by default and it was failing on the PS4 linux bot because of this.
Thank you for fixing the test! On Thu, Jun 28, 2018 at 2:23 AM Douglas Yung via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: dyung > Date: Wed Jun 27 17:19:12 2018 > New Revision: 335799 > > URL: http://llvm.org/viewvc/llvm-project?rev=335799&view=rev > Log: > Fixup test to compile with -frtti when trying to use typeid() as the PS4 > does not have it on by default and it was failing on the PS4 linux bot > because of this. > > Modified: > > clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp > > Modified: > clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp?rev=335799&r1=335798&r2=335799&view=diff > > == > --- > clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp > (original) > +++ > clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp > Wed Jun 27 17:19:12 2018 > @@ -579,8 +579,10 @@ TEST(ExprMutationAnalyzerTest, Unevaluat >Results = match(withEnclosingCompound(declRefTo("x")), > AST->getASTContext()); >EXPECT_FALSE(isMutated(Results, AST.get())); > > - AST = tooling::buildASTFromCode("namespace std { class type_info; }" > - "void f() { int x; typeid(x = 10); }"); > + AST = > +tooling::buildASTFromCodeWithArgs("namespace std { class type_info; }" > + "void f() { int x; typeid(x = 10); > }", > + std::vector > ({"-frtti"})); >Results = match(withEnclosingCompound(declRefTo("x")), > AST->getASTContext()); >EXPECT_FALSE(isMutated(Results, AST.get())); > > @@ -596,10 +598,11 @@ TEST(ExprMutationAnalyzerTest, NotUneval >match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); >EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x++")); > > - AST = tooling::buildASTFromCode( > + AST = tooling::buildASTFromCodeWithArgs( >"namespace std { class type_info; }" >"struct A { virtual ~A(); }; struct B : A {};" > - "struct X { A& f(); }; void f() { X x; typeid(x.f()); }"); > + "struct X { A& f(); }; void f() { X x; typeid(x.f()); }", > + std::vector ({"-frtti"})); >Results = match(withEnclosingCompound(declRefTo("x")), > AST->getASTContext()); >EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.f()")); > } > > > ___ > 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
r335843 - The :option: syntax was generating Sphinx build warnings; switched to double backticks to silence the warning; NFC.
Author: aaronballman Date: Thu Jun 28 05:05:40 2018 New Revision: 335843 URL: http://llvm.org/viewvc/llvm-project?rev=335843&view=rev Log: The :option: syntax was generating Sphinx build warnings; switched to double backticks to silence the warning; NFC. Modified: cfe/trunk/docs/ReleaseNotes.rst Modified: cfe/trunk/docs/ReleaseNotes.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=335843&r1=335842&r2=335843&view=diff == --- cfe/trunk/docs/ReleaseNotes.rst (original) +++ cfe/trunk/docs/ReleaseNotes.rst Thu Jun 28 05:05:40 2018 @@ -103,8 +103,7 @@ Non-comprehensive list of changes in thi New Compiler Flags -- -- :option:`-fstrict-float-cast-overflow` and - :option:`-fno-strict-float-cast-overflow`. +- ``-fstrict-float-cast-overflow`` and ``-fno-strict-float-cast-overflow``. When a floating-point value is not representable in a destination integer type, the code has undefined behavior according to the language standard. By @@ -112,7 +111,7 @@ New Compiler Flags 'no-strict' option, Clang attempts to match the overflowing behavior of the target's native float-to-int conversion instructions. -- :option: `-fforce-emit-vtables` and `-fno-force-emit-vtables`. +- ``-fforce-emit-vtables`` and ``-fno-force-emit-vtables``. In order to improve devirtualization, forces emitting of vtables even in modules where it isn't necessary. It causes more inline virtual functions ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.
alexfh added inline comments. Comment at: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp:582 + + AST = tooling::buildASTFromCode("namespace std { class type_info; }" + "void f() { int x; typeid(x = 10); }"); FYI, this test had to be fixed for a ps4 buildbot. See r335799 for details. 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
[clang-tools-extra] r335845 - Fix formatting. NFC.
Author: alexfh Date: Thu Jun 28 05:15:17 2018 New Revision: 335845 URL: http://llvm.org/viewvc/llvm-project?rev=335845&view=rev Log: Fix formatting. NFC. Modified: clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp Modified: clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp?rev=335845&r1=335844&r2=335845&view=diff == --- clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp (original) +++ clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp Thu Jun 28 05:15:17 2018 @@ -579,10 +579,9 @@ TEST(ExprMutationAnalyzerTest, Unevaluat Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_FALSE(isMutated(Results, AST.get())); - AST = -tooling::buildASTFromCodeWithArgs("namespace std { class type_info; }" - "void f() { int x; typeid(x = 10); }", - std::vector ({"-frtti"})); + AST = tooling::buildASTFromCodeWithArgs("namespace std { class type_info; }" + "void f() { int x; typeid(x = 10); }", + std::vector({"-frtti"})); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_FALSE(isMutated(Results, AST.get())); @@ -602,7 +601,7 @@ TEST(ExprMutationAnalyzerTest, NotUneval "namespace std { class type_info; }" "struct A { virtual ~A(); }; struct B : A {};" "struct X { A& f(); }; void f() { X x; typeid(x.f()); }", - std::vector ({"-frtti"})); + std::vector({"-frtti"})); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.f()")); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r335846 - Remove explicit type from an initializer list. NFC.
Author: alexfh Date: Thu Jun 28 05:18:42 2018 New Revision: 335846 URL: http://llvm.org/viewvc/llvm-project?rev=335846&view=rev Log: Remove explicit type from an initializer list. NFC. Modified: clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp Modified: clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp?rev=335846&r1=335845&r2=335846&view=diff == --- clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp (original) +++ clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp Thu Jun 28 05:18:42 2018 @@ -581,7 +581,7 @@ TEST(ExprMutationAnalyzerTest, Unevaluat AST = tooling::buildASTFromCodeWithArgs("namespace std { class type_info; }" "void f() { int x; typeid(x = 10); }", - std::vector({"-frtti"})); + {"-frtti"}); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_FALSE(isMutated(Results, AST.get())); @@ -601,7 +601,7 @@ TEST(ExprMutationAnalyzerTest, NotUneval "namespace std { class type_info; }" "struct A { virtual ~A(); }; struct B : A {};" "struct X { A& f(); }; void f() { X x; typeid(x.f()); }", - std::vector({"-frtti"})); + {"-frtti"}); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.f()")); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48716: [clang-format] Fix counting parameters/arguments for ObjC
jolesiak created this revision. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D48716 Files: lib/Format/FormatToken.h lib/Format/TokenAnnotator.cpp Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -515,11 +515,23 @@ } Left->MatchingParen = CurrentToken; CurrentToken->MatchingParen = Left; +// FirstObjCSelectorName is set when a colon is found. This does +// not work, however, when method has no parameters. +// Here, we set FirstObjCSelectorName when the end of the expression is +// reached, in case it was not set already. +if (!Contexts.back().FirstObjCSelectorName) { +FormatToken* Previous = CurrentToken->getPreviousNonComment(); +if (Previous && Previous->is(TT_SelectorName)) { + Previous->ObjCSelectorNameParts = 1; + Contexts.back().FirstObjCSelectorName = Previous; +} +} else { + Left->ParameterCount = + Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts; +} if (Contexts.back().FirstObjCSelectorName) { Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = Contexts.back().LongestObjCSelectorName; - Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts = - Left->ParameterCount; if (Left->BlockParameterCount > 1) Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0; } @@ -539,11 +551,6 @@ TT_DesignatedInitializerLSquare)) { Left->Type = TT_ObjCMethodExpr; StartsObjCMethodExpr = true; - // ParameterCount might have been set to 1 before expression was - // recognized as ObjCMethodExpr (as '1 + number of commas' formula is - // used for other expression types). Parameter counter has to be, - // therefore, reset to 0. - Left->ParameterCount = 0; Contexts.back().ColonIsObjCMethodExpr = true; if (Parent && Parent->is(tok::r_paren)) // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen. @@ -617,12 +624,12 @@ } void updateParameterCount(FormatToken *Left, FormatToken *Current) { +// For ObjC methods number of parameters is calculated differently as +// method declarations have a different structure (parameters are not inside +// parenthesis scope). if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block) ++Left->BlockParameterCount; -if (Left->Type == TT_ObjCMethodExpr) { - if (Current->is(tok::colon)) -++Left->ParameterCount; -} else if (Current->is(tok::comma)) { +if (Current->is(tok::comma)) { ++Left->ParameterCount; if (!Left->Role) Left->Role.reset(new CommaSeparatedList(Style)); @@ -712,6 +719,7 @@ Contexts.back().LongestObjCSelectorName) Contexts.back().LongestObjCSelectorName = Tok->Previous->ColumnWidth; + ++Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts; } } else if (Contexts.back().ColonIsForRangeExpr) { Tok->Type = TT_RangeBasedForLoopColon; Index: lib/Format/FormatToken.h === --- lib/Format/FormatToken.h +++ lib/Format/FormatToken.h @@ -243,8 +243,9 @@ /// e.g. because several of them are block-type. unsigned LongestObjCSelectorName = 0; - /// How many parts ObjC selector have (i.e. how many parameters method - /// has). + /// If this is the first ObjC selector name in an ObjC method + /// definition or call, this contains the number of parts that whole selector + /// consist of. unsigned ObjCSelectorNameParts = 0; /// Stores the number of required fake parentheses and the Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -515,11 +515,23 @@ } Left->MatchingParen = CurrentToken; CurrentToken->MatchingParen = Left; +// FirstObjCSelectorName is set when a colon is found. This does +// not work, however, when method has no parameters. +// Here, we set FirstObjCSelectorName when the end of the expression is +// reached, in case it was not set already. +if (!Contexts.back().FirstObjCSelectorName) { +FormatToken* Previous = CurrentToken->getPreviousNonComment(); +if (Previous && Previous->is(TT_SelectorName)) { + Previous->ObjCSelectorNameParts = 1; + Contexts.back().FirstObjCSelectorName = Previous; +} +} else { + Left->ParameterCount = + Contex
[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well
JonasToth created this revision. JonasToth added reviewers: alexfh, aaron.ballman, hokein, ilya-biryukov. Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai. The cppcoreguidelines-pro-bounds-pointer-arithmetic warns on all occassion where pointer arithmetic is used, but does not check values where the pointer types is deduced via ``auto``. This patch adjusts this behaviour and solved PR36489. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48717 Files: clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic-pr36489.cpp test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp Index: test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp === --- test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp +++ test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp @@ -85,5 +85,5 @@ auto diff = p - q; // OK, result is arithmetic - for(int ii : a) ; // OK, pointer arithmetic generated by compiler + for (int ii : a) ; // OK, pointer arithmetic generated by compiler } Index: test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic-pr36489.cpp === --- /dev/null +++ test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic-pr36489.cpp @@ -0,0 +1,39 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-pointer-arithmetic %t -- -- -std=c++14 + + +// Fix PR36489 and detect auto-deduced value correctly. +char *getPtr(); +auto getPtrAuto() { return getPtr(); } +auto getPtrWithTrailingReturnType() -> char *; + +void auto_deduction_binary() { + auto p1 = getPtr() + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not use pointer arithmetic + auto p2 = getPtrAuto() + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: do not use pointer arithmetic + auto p3 = getPtrWithTrailingReturnType() + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: do not use pointer arithmetic + auto p4 = getPtr(); + auto *p5 = getPtr(); + p4 = p4 + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use pointer arithmetic + p5 = p5 + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use pointer arithmetic +} + +void auto_deduction_subscript() { + char p1 = getPtr()[2]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use pointer arithmetic + auto p2 = getPtr()[3]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use pointer arithmetic + + char p3 = getPtrAuto()[4]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use pointer arithmetic + auto p4 = getPtrAuto()[5]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use pointer arithmetic + + char p5 = getPtrWithTrailingReturnType()[6]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use pointer arithmetic + auto p6 = getPtrWithTrailingReturnType()[7]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use pointer arithmetic +} Index: clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp === --- clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp +++ clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp @@ -21,12 +21,15 @@ if (!getLangOpts().CPlusPlus) return; + const auto AllPointerTypes = anyOf( + hasType(pointerType()), hasType(autoType(hasDeducedType(pointerType(); + // Flag all operators +, -, +=, -=, ++, -- that result in a pointer Finder->addMatcher( binaryOperator( anyOf(hasOperatorName("+"), hasOperatorName("-"), hasOperatorName("+="), hasOperatorName("-=")), - hasType(pointerType()), + AllPointerTypes, unless(hasLHS(ignoringImpCasts(declRefExpr(to(isImplicit())) .bind("expr"), this); @@ -41,7 +44,7 @@ Finder->addMatcher( arraySubscriptExpr( hasBase(ignoringImpCasts( - anyOf(hasType(pointerType()), + anyOf(AllPointerTypes, hasType(decayedType(hasDecayedType(pointerType( .bind("expr"), this); Index: test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp === --- test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp +++ test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp @@ -85,5 +85,5 @@ auto diff = p - q; // OK, result is arithmetic - for(int ii : a) ; // OK, pointer arithmetic generated by compiler + for (int ii : a) ; // OK, pointer arithmetic generated by compiler } Index: test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic-pr36489.cpp === --- /dev/null +++ test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arith
[PATCH] D48716: [clang-format] Fix counting parameters/arguments for ObjC
jolesiak updated this revision to Diff 153298. jolesiak added a comment. Fix comment Repository: rC Clang https://reviews.llvm.org/D48716 Files: lib/Format/FormatToken.h lib/Format/TokenAnnotator.cpp Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -515,11 +515,23 @@ } Left->MatchingParen = CurrentToken; CurrentToken->MatchingParen = Left; +// FirstObjCSelectorName is set when a colon is found. This does +// not work, however, when method has no parameters. +// Here, we set FirstObjCSelectorName when the end of the expression is +// reached, in case it was not set already. +if (!Contexts.back().FirstObjCSelectorName) { +FormatToken* Previous = CurrentToken->getPreviousNonComment(); +if (Previous && Previous->is(TT_SelectorName)) { + Previous->ObjCSelectorNameParts = 1; + Contexts.back().FirstObjCSelectorName = Previous; +} +} else { + Left->ParameterCount = + Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts; +} if (Contexts.back().FirstObjCSelectorName) { Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = Contexts.back().LongestObjCSelectorName; - Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts = - Left->ParameterCount; if (Left->BlockParameterCount > 1) Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0; } @@ -539,11 +551,6 @@ TT_DesignatedInitializerLSquare)) { Left->Type = TT_ObjCMethodExpr; StartsObjCMethodExpr = true; - // ParameterCount might have been set to 1 before expression was - // recognized as ObjCMethodExpr (as '1 + number of commas' formula is - // used for other expression types). Parameter counter has to be, - // therefore, reset to 0. - Left->ParameterCount = 0; Contexts.back().ColonIsObjCMethodExpr = true; if (Parent && Parent->is(tok::r_paren)) // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen. @@ -617,12 +624,12 @@ } void updateParameterCount(FormatToken *Left, FormatToken *Current) { +// For ObjC methods number of parameters is calculated differently as +// method declarations have different structure (parameters are not inside +// parenthesis scope). if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block) ++Left->BlockParameterCount; -if (Left->Type == TT_ObjCMethodExpr) { - if (Current->is(tok::colon)) -++Left->ParameterCount; -} else if (Current->is(tok::comma)) { +if (Current->is(tok::comma)) { ++Left->ParameterCount; if (!Left->Role) Left->Role.reset(new CommaSeparatedList(Style)); @@ -712,6 +719,7 @@ Contexts.back().LongestObjCSelectorName) Contexts.back().LongestObjCSelectorName = Tok->Previous->ColumnWidth; + ++Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts; } } else if (Contexts.back().ColonIsForRangeExpr) { Tok->Type = TT_RangeBasedForLoopColon; Index: lib/Format/FormatToken.h === --- lib/Format/FormatToken.h +++ lib/Format/FormatToken.h @@ -243,8 +243,9 @@ /// e.g. because several of them are block-type. unsigned LongestObjCSelectorName = 0; - /// How many parts ObjC selector have (i.e. how many parameters method - /// has). + /// If this is the first ObjC selector name in an ObjC method + /// definition or call, this contains the number of parts that whole selector + /// consist of. unsigned ObjCSelectorNameParts = 0; /// Stores the number of required fake parentheses and the Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -515,11 +515,23 @@ } Left->MatchingParen = CurrentToken; CurrentToken->MatchingParen = Left; +// FirstObjCSelectorName is set when a colon is found. This does +// not work, however, when method has no parameters. +// Here, we set FirstObjCSelectorName when the end of the expression is +// reached, in case it was not set already. +if (!Contexts.back().FirstObjCSelectorName) { +FormatToken* Previous = CurrentToken->getPreviousNonComment(); +if (Previous && Previous->is(TT_SelectorName)) { + Previous->ObjCSelectorNameParts = 1; + Contexts.back().FirstObjCSelectorName = Previous; +} +} else { + Left->ParameterCount = +
[PATCH] D48718: [clang-format] Prohibit breaking after a bracket opening ObjC method expression
jolesiak created this revision. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D48718 Files: lib/Format/ContinuationIndenter.cpp Index: lib/Format/ContinuationIndenter.cpp === --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -321,6 +321,9 @@ State.Stack.back().NoLineBreakInOperand) return false; + if (Previous.is(tok::l_square) && Previous.is(TT_ObjCMethodExpr)) +return false; + return !State.Stack.back().NoLineBreak; } Index: lib/Format/ContinuationIndenter.cpp === --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -321,6 +321,9 @@ State.Stack.back().NoLineBreakInOperand) return false; + if (Previous.is(tok::l_square) && Previous.is(TT_ObjCMethodExpr)) +return false; + return !State.Stack.back().NoLineBreak; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response
malaperle added a comment. In https://reviews.llvm.org/D48687#1146515, @simark wrote: > In https://reviews.llvm.org/D48687#1146308, @ilya-biryukov wrote: > > > Thanks for the patch! > > Could we try to figure out why the duplicates were there in the first > > place and why the paths were different? > > > I tried to do that, but it goes deep in the clang internals with which I'm > not familiar. All I could see was that when creating the `FileEntry` > representing the > `/home/emaisin/src/ls-interact/cpp-test/build/../src/first.h` file, > `FileManager::getFile` is called with `OpenFile=false`. This makes it so > that the `RealPathName` field is not set (at `FileManager.cpp:320`). Because > `RealPathName` is not set (well, empty), we use the non-normalized name in > `getAbsoluteFilePath`. That's all I can tell. I think from what we've seen before, it had to do with location coming from the preamble vs not. Simon, maybe we can spend some time together to investigate this. I know I said I'd rather not touch the Clang part as I thought it might be on purpose and/or performance sensitive, but the fact that the locations are inconsistent between the preamble and outside...it's not ideal and error-prone. Either way I think we need this fix in Clangd as a defensive fix because it's not in the AST's contract/guarantees (preamble or not) to provide locations with realpaths. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48719: [clang-format] Fix split priorities for ObjC methods
jolesiak created this revision. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D48719 Files: lib/Format/FormatToken.h lib/Format/TokenAnnotator.cpp unittests/Format/FormatTestObjC.cpp Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -678,6 +678,18 @@ verifyFormat("[(id)foo bar:(id) ? baz : quux];"); verifyFormat("4 > 4 ? (id)a : (id)baz;"); + unsigned PreviousColumnLimit = Style.ColumnLimit; + Style.ColumnLimit = 50; + // Instead of: + // bool a = + // ([object a:42] == 0 || [object a:42 + //b:42] == 0); + verifyFormat("bool a = ([object a:42] == 0 ||\n" + " [object a:42 b:42] == 0);"); + Style.ColumnLimit = PreviousColumnLimit; + verifyFormat("bool a = ([ a] == a ||\n" + " [ a] == );"); + // This tests that the formatter doesn't break after "backing" but before ":", // which would be at 80 columns. verifyFormat( @@ -754,11 +766,10 @@ "[self a:aaa, aaa, aaa,\n" "aaa, aaa, aaa,\n" "aaa, aaa];"); + verifyFormat("[self // break\n" " a:a\n" "aaa:aaa];"); - verifyFormat("bool a = ([ a] == a ||\n" - " [ a] == );"); // Formats pair-parameters. verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];"); @@ -803,6 +814,12 @@ verifyFormat("[((Foo *)foo) bar];"); verifyFormat("[((Foo *)foo) bar:1 blech:2];"); + Style.ColumnLimit = 20; + verifyFormat("a = [a aa:aa\n" + " aa:aa];"); + verifyFormat("aa = [aa aa:aa\n" + " aa:aa];"); + Style.ColumnLimit = 70; verifyFormat( "void f() {\n" Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -719,6 +719,8 @@ Contexts.back().LongestObjCSelectorName) Contexts.back().LongestObjCSelectorName = Tok->Previous->ColumnWidth; + Tok->Previous->ParameterIndex = + Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts; ++Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts; } } else if (Contexts.back().ColonIsForRangeExpr) { @@ -2136,8 +2138,20 @@ // FIXME: Only calculate this if CanBreakBefore is true once static // initializers etc. are sorted out. // FIXME: Move magic numbers to a better place. -Current->SplitPenalty = 20 * Current->BindingStrength + -splitPenalty(Line, *Current, InFunctionDecl); + +// Reduce penalty for aligning ObjC method arguments using the colon +// alignment as this is the canonical way (still prefer fitting everything +// into one line if possible). Trying to fit a whole epression into one +// line should not force other line breaks (e.g. when ObjC method +// expression is a part of other expression). +Current->SplitPenalty = splitPenalty(Line, *Current, InFunctionDecl); +if (Style.Language == FormatStyle::LK_ObjC && +Current->is(TT_SelectorName) && Current->ParameterIndex > 0) { + if (Current->ParameterIndex == 1) +Current->SplitPenalty += 5 * Current->BindingStrength; +} else { + Current->SplitPenalty += 20 * Current->BindingStrength; +} Current = Current->Next; } Index: lib/Format/FormatToken.h === --- lib/Format/FormatToken.h +++ lib/Format/FormatToken.h @@ -248,6 +248,11 @@ /// consist of. unsigned ObjCSelectorNameParts = 0; + /// The 0-based index of the parameter/argument. For ObjC it is set + /// for the selector name token. + /// For now calculated only for ObjC. + unsigned ParameterIndex = 0; + /// Stores the number of required fake parentheses and the /// corresponding operator precedence. /// ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48720: [clang-format] Put ObjC method arguments into one line when they fit
jolesiak created this revision. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D48720 Files: lib/Format/ContinuationIndenter.cpp unittests/Format/FormatTestObjC.cpp Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -820,6 +820,48 @@ verifyFormat("aa = [aa aa:aa\n" " aa:aa];"); + // Message receiver taking multiple lines. + // Non-corner case. + verifyFormat("[[object block:^{\n" + " return 42;\n" + "}] a:42 b:42];"); + // Arguments just fit into one line. + verifyFormat("[[object block:^{\n" + " return 42;\n" + "}] aaa:42 b:42];"); + // Arguments just over a column limit. + verifyFormat("[[object block:^{\n" + " return 42;\n" + "}] aaa:42\n" + "bb:42];"); + // Arguments just fit into one line. + Style.ColumnLimit = 23; + verifyFormat("[[obj a:42\n" + " b:42\n" + " c:42\n" + " d:42] e:42 f:42];"); + + // Arguments do not fit into one line with a receiver. + Style.ColumnLimit = 20; + verifyFormat("[[obj a:42] a:42\n" + "b:42];"); + verifyFormat("[[obj a:42] a:42\n" + "b:42\n" + "c:42];"); + verifyFormat("[[obj aa:42\n" + " b:42]\n" + "cc:42\n" + " d:42];"); + + // Avoid breaking receiver expression. + Style.ColumnLimit = 30; + verifyFormat("fooo =\n" + "[[obj fooo] aaa:42\n" + "aaa:42];"); + verifyFormat("[[[obj foo] bar] aa:42\n" + " bb:42\n" + " cc:42];"); + Style.ColumnLimit = 70; verifyFormat( "void f() {\n" Index: lib/Format/ContinuationIndenter.cpp === --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -1398,6 +1398,29 @@ (Current.is(tok::greater) && Current.is(TT_DictLiteral State.Stack.pop_back(); + // Reevaluate whether ObjC message arguments fit into one line. + // If a receiver spans multiple lines, e.g.: + // [[object block:^{ + // return 42; + // }] a:42 b:42]; + // BreakBeforeParameter is calculated based on an incorrect assumption + // (it is checked whether the whole expression fits into one line without + // considering a line break inside a message receiver). + // We check whether arguements fit after receiver scope closer (into the same + // line). + if (Current.MatchingParen && Current.MatchingParen->Previous) { +const FormatToken &CurrentScopeOpener = *Current.MatchingParen->Previous; +if (CurrentScopeOpener.is(TT_ObjCMethodExpr) && +CurrentScopeOpener.MatchingParen) { + int NecessarySpaceInLine = + getLengthToMatchingParen(CurrentScopeOpener, State.Stack) + + CurrentScopeOpener.TotalLength - Current.TotalLength - 1; + if (State.Column + Current.ColumnWidth + NecessarySpaceInLine <= + Style.ColumnLimit) +State.Stack.back().BreakBeforeParameter = false; +} + } + if (Current.is(tok::r_square)) { // If this ends the array subscript expr, reset the corresponding value. const FormatToken *NextNonComment = Current.getNextNonComment(); Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -820,6 +820,48 @@ verifyFormat("aa = [aa aa:aa\n" " aa:aa];"); + // Message receiver taking multiple lines. + // Non-corner case. + verifyFormat("[[object block:^{\n" + " return 42;\n" + "}] a:42 b:42];"); + // Arguments just fit into one line. + verifyFormat("[[object block:^{\n" + " return 42;\n" + "}] aaa:42 b:42];"); + // Arguments just over a column limit. + verifyFormat("[[object block:^{\n" + " return 42;\n" + "}] aaa:42\n" + "bb:42];"); + // Arguments just fit into one line. + Style.ColumnLimit = 23; + verifyFormat("[[obj a:42\n" + " b:42\n" + " c:42\n" + " d:42] e:42 f:42];"); + + // Arguments do not fit into one line with a receiver. + Style.ColumnLimit = 20; + verifyFormat("[[obj a:42] a:42\n" + "b:42];"); + verifyFormat("[[obj a:42] a:42\n" + "b:42\n" + "c:42];"); + verifyFormat("[[obj aa:42\n" + " b:42]\n" + "cc:42\n" +
[PATCH] D32747: [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments
baloghadamsoftware updated this revision to Diff 153308. baloghadamsoftware added a comment. Herald added a reviewer: george.karpenkov. Herald added a subscriber: mikhail.ramalho. Rebased to https://reviews.llvm.org/rL335835. https://reviews.llvm.org/D32747 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/IteratorChecker.cpp test/Analysis/Inputs/system-header-simulator-cxx.h test/Analysis/diagnostics/explicit-suppression.cpp test/Analysis/invalidated-iterator.cpp Index: test/Analysis/invalidated-iterator.cpp === --- /dev/null +++ test/Analysis/invalidated-iterator.cpp @@ -0,0 +1,32 @@ +// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=false %s -verify +// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify + +#include "Inputs/system-header-simulator-cxx.h" + +void bad_copy_assign_operator_list1(std::list &L1, +const std::list &L2) { + auto i0 = L1.cbegin(); + L1 = L2; + *i0; // expected-warning{{Invalidated iterator accessed}} +} + +void bad_copy_assign_operator_vector1(std::vector &V1, + const std::vector &V2) { + auto i0 = V1.cbegin(); + V1 = V2; + *i0; // expected-warning{{Invalidated iterator accessed}} +} + +void bad_copy_assign_operator_deque1(std::deque &D1, + const std::deque &D2) { + auto i0 = D1.cbegin(); + D1 = D2; + *i0; // expected-warning{{Invalidated iterator accessed}} +} + +void bad_copy_assign_operator_forward_list1(std::forward_list &FL1, +const std::forward_list &FL2) { + auto i0 = FL1.cbegin(); + FL1 = FL2; + *i0; // expected-warning{{Invalidated iterator accessed}} +} Index: test/Analysis/diagnostics/explicit-suppression.cpp === --- test/Analysis/diagnostics/explicit-suppression.cpp +++ test/Analysis/diagnostics/explicit-suppression.cpp @@ -19,6 +19,6 @@ void testCopyNull(C *I, C *E) { std::copy(I, E, (C *)0); #ifndef SUPPRESSED - // expected-warning@../Inputs/system-header-simulator-cxx.h:514 {{Called C++ object pointer is null}} + // expected-warning@../Inputs/system-header-simulator-cxx.h:554 {{Called C++ object pointer is null}} #endif } Index: test/Analysis/Inputs/system-header-simulator-cxx.h === --- test/Analysis/Inputs/system-header-simulator-cxx.h +++ test/Analysis/Inputs/system-header-simulator-cxx.h @@ -252,6 +252,15 @@ return size_t(_finish - _start); } +vector& operator=(const vector &other); +vector& operator=(vector &&other); +vector& operator=(std::initializer_list ilist); + +void assign(size_type count, const T &value); +template +void assign(InputIterator first, InputIterator last); +void assign(std::initializer_list ilist); + void clear(); void push_back(const T &value); @@ -301,8 +310,21 @@ list& operator=(list &&other); list& operator=(std::initializer_list ilist); +void assign(size_type count, const T &value); +template +void assign(InputIterator first, InputIterator last); +void assign(std::initializer_list ilist); + void clear(); +void push_back(const T &value); +void push_back(T &&value); +void pop_back(); + +void push_front(const T &value); +void push_front(T &&value); +void pop_front(); + iterator begin() { return iterator(_start); } const_iterator begin() const { return const_iterator(_start); } const_iterator cbegin() const { return const_iterator(_start); } @@ -338,6 +360,15 @@ return size_t(_finish - _start); } +deque& operator=(const deque &other); +deque& operator=(deque &&other); +deque& operator=(std::initializer_list ilist); + +void assign(size_type count, const T &value); +template +void assign(InputIterator first, InputIterator last); +void assign(std::initializer_list ilist); + void clear(); void push_back(const T &value); @@ -351,11 +382,11 @@ T &operator[](size_t n) { return _start[n]; } - + const T &operator[](size_t n) const { return _start[n]; } - + iterator begin() { return iterator(_start); } const_iterator begin() const { return const_iterator(_start); } const_iterator cbegin() const { return const_iterator(_start); } @@ -367,7 +398,7 @@ T& back() { return *(end() - 1); } const T&
r335853 - [ODRHash] Do not rely on Type* when computing the hash.
Author: vvassilev Date: Thu Jun 28 06:28:44 2018 New Revision: 335853 URL: http://llvm.org/viewvc/llvm-project?rev=335853&view=rev Log: [ODRHash] Do not rely on Type* when computing the hash. ODRHash aims to provide Cross-TU stable hashing. Making clang::Type pointer part of the hash connects (remotely) the ODRHash with the TU-specific ::Profile hasher. r332281 exposed the issue by changing the way the ASTContext different elaborated types if there is an owning tag. In that case, ODRHash stores two different types in its TypeMap which yields false ODR violation in modules. The current state of implementation shouldn't need the TypeMap concept anymore. Rip it out. Differential Revision: https://reviews.llvm.org/D48524 Added: cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/ cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/first.h cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/second.h cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h cfe/trunk/test/Modules/odr_hash-elaborated-types.cpp Modified: cfe/trunk/include/clang/AST/ODRHash.h cfe/trunk/lib/AST/ODRHash.cpp Modified: cfe/trunk/include/clang/AST/ODRHash.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ODRHash.h?rev=335853&r1=335852&r2=335853&view=diff == --- cfe/trunk/include/clang/AST/ODRHash.h (original) +++ cfe/trunk/include/clang/AST/ODRHash.h Thu Jun 28 06:28:44 2018 @@ -40,7 +40,6 @@ class ODRHash { // Use DenseMaps to convert from DeclarationName and Type pointers // to an index value. llvm::DenseMap DeclNameMap; - llvm::DenseMap TypeMap; // Save space by processing bools at the end. llvm::SmallVector Bools; Modified: cfe/trunk/lib/AST/ODRHash.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ODRHash.cpp?rev=335853&r1=335852&r2=335853&view=diff == --- cfe/trunk/lib/AST/ODRHash.cpp (original) +++ cfe/trunk/lib/AST/ODRHash.cpp Thu Jun 28 06:28:44 2018 @@ -180,7 +180,6 @@ void ODRHash::AddTemplateParameterList(c void ODRHash::clear() { DeclNameMap.clear(); - TypeMap.clear(); Bools.clear(); ID.clear(); } @@ -770,14 +769,6 @@ public: void ODRHash::AddType(const Type *T) { assert(T && "Expecting non-null pointer."); - auto Result = TypeMap.insert(std::make_pair(T, TypeMap.size())); - ID.AddInteger(Result.first->second); - // On first encounter of a Type pointer, process it. Every time afterwards, - // only the index value is needed. - if (!Result.second) { -return; - } - ODRTypeVisitor(ID, *this).Visit(T); } Added: cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/first.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/first.h?rev=335853&view=auto == --- cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/first.h (added) +++ cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/first.h Thu Jun 28 06:28:44 2018 @@ -0,0 +1,6 @@ +#ifndef FIRST +#define FIRST + +#include "textual_time.h" + +#endif Added: cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap?rev=335853&view=auto == --- cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap (added) +++ cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap Thu Jun 28 06:28:44 2018 @@ -0,0 +1,5 @@ +module M { + module first { header "first.h" export *} + module second { header "second.h" export *} + export * +} Added: cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/second.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/second.h?rev=335853&view=auto == --- cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/second.h (added) +++ cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/second.h Thu Jun 28 06:28:44 2018 @@ -0,0 +1,6 @@ +#ifndef SECOND +#define SECOND + +#include "textual_stat.h" + +#endif Added: cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h?rev=335853&view=auto == --- cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h (added) +++ cfe/trunk/test/Modules/Inputs/odr_hash-elabor
[PATCH] D48524: [ODRHash] Do not rely on Type* when computing the hash.
This revision was automatically updated to reflect the committed changes. Closed by commit rL335853: [ODRHash] Do not rely on Type* when computing the hash. (authored by vvassilev, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D48524?vs=153086&id=153309#toc Repository: rL LLVM https://reviews.llvm.org/D48524 Files: cfe/trunk/include/clang/AST/ODRHash.h cfe/trunk/lib/AST/ODRHash.cpp cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/first.h cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/second.h cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h cfe/trunk/test/Modules/odr_hash-elaborated-types.cpp Index: cfe/trunk/include/clang/AST/ODRHash.h === --- cfe/trunk/include/clang/AST/ODRHash.h +++ cfe/trunk/include/clang/AST/ODRHash.h @@ -40,7 +40,6 @@ // Use DenseMaps to convert from DeclarationName and Type pointers // to an index value. llvm::DenseMap DeclNameMap; - llvm::DenseMap TypeMap; // Save space by processing bools at the end. llvm::SmallVector Bools; Index: cfe/trunk/test/Modules/odr_hash-elaborated-types.cpp === --- cfe/trunk/test/Modules/odr_hash-elaborated-types.cpp +++ cfe/trunk/test/Modules/odr_hash-elaborated-types.cpp @@ -0,0 +1,12 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -std=c++1z -I%S/Inputs/odr_hash-elaborated-types -verify %s +// RUN: %clang_cc1 -std=c++1z -fmodules -fmodules-local-submodule-visibility -fmodule-map-file=%S/Inputs/odr_hash-elaborated-types/module.modulemap -fmodules-cache-path=%t -x c++ -I%S/Inputs/odr_hash-elaborated-types -verify %s + +#include "textual_stat.h" + +#include "first.h" +#include "second.h" + +void use() { struct stat value; } + +// expected-no-diagnostics Index: cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/second.h === --- cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/second.h +++ cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/second.h @@ -0,0 +1,6 @@ +#ifndef SECOND +#define SECOND + +#include "textual_stat.h" + +#endif Index: cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/first.h === --- cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/first.h +++ cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/first.h @@ -0,0 +1,6 @@ +#ifndef FIRST +#define FIRST + +#include "textual_time.h" + +#endif Index: cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h === --- cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h +++ cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h @@ -0,0 +1,6 @@ +#ifndef _TIME_H +#define _TIME_H + +struct timespec { }; + +#endif Index: cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h === --- cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h +++ cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h @@ -0,0 +1,11 @@ +#ifndef _SYS_STAT_H +#define _SYS_STAT_H + +#include "textual_time.h" + +struct stat { + struct timespec st_atim; + struct timespec st_mtim; +}; + +#endif Index: cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap === --- cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap +++ cfe/trunk/test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap @@ -0,0 +1,5 @@ +module M { + module first { header "first.h" export *} + module second { header "second.h" export *} + export * +} Index: cfe/trunk/lib/AST/ODRHash.cpp === --- cfe/trunk/lib/AST/ODRHash.cpp +++ cfe/trunk/lib/AST/ODRHash.cpp @@ -180,7 +180,6 @@ void ODRHash::clear() { DeclNameMap.clear(); - TypeMap.clear(); Bools.clear(); ID.clear(); } @@ -770,14 +769,6 @@ void ODRHash::AddType(const Type *T) { assert(T && "Expecting non-null pointer."); - auto Result = TypeMap.insert(std::make_pair(T, TypeMap.size())); - ID.AddInteger(Result.first->second); - // On first encounter of a Type pointer, process it. Every time afterwards, - // only the index value is needed. - if (!Result.second) { -return; - } - ODRTypeVisitor(ID, *this).Visit(T); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r335854 - Fix unittest build with GCC older than 5.
Author: d0k Date: Thu Jun 28 06:31:36 2018 New Revision: 335854 URL: http://llvm.org/viewvc/llvm-project?rev=335854&view=rev Log: Fix unittest build with GCC older than 5. Old GCCs have an annoying bug where RVO disables the automatic conversion to base for unique_ptr. Add a pessimizing std::move as a workaround. Modified: cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp Modified: cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp?rev=335854&r1=335853&r2=335854&view=diff == --- cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp (original) +++ cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp Thu Jun 28 06:31:36 2018 @@ -61,7 +61,7 @@ public: AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry &Registry) { Registry.addChecker("custom.CustomChecker", "Description"); }); -return AnalysisConsumer; +return std::move(AnalysisConsumer); } }; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48719: [clang-format] Fix split priorities for ObjC methods
jolesiak updated this revision to Diff 153312. jolesiak added a comment. Fix comment. Repository: rC Clang https://reviews.llvm.org/D48719 Files: lib/Format/ContinuationIndenter.cpp lib/Format/FormatToken.h lib/Format/TokenAnnotator.cpp unittests/Format/FormatTestObjC.cpp Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -678,6 +678,18 @@ verifyFormat("[(id)foo bar:(id) ? baz : quux];"); verifyFormat("4 > 4 ? (id)a : (id)baz;"); + unsigned PreviousColumnLimit = Style.ColumnLimit; + Style.ColumnLimit = 50; + // Instead of: + // bool a = + // ([object a:42] == 0 || [object a:42 + //b:42] == 0); + verifyFormat("bool a = ([object a:42] == 0 ||\n" + " [object a:42 b:42] == 0);"); + Style.ColumnLimit = PreviousColumnLimit; + verifyFormat("bool a = ([ a] == a ||\n" + " [ a] == );"); + // This tests that the formatter doesn't break after "backing" but before ":", // which would be at 80 columns. verifyFormat( @@ -754,11 +766,10 @@ "[self a:aaa, aaa, aaa,\n" "aaa, aaa, aaa,\n" "aaa, aaa];"); + verifyFormat("[self // break\n" " a:a\n" "aaa:aaa];"); - verifyFormat("bool a = ([ a] == a ||\n" - " [ a] == );"); // Formats pair-parameters. verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];"); @@ -803,6 +814,12 @@ verifyFormat("[((Foo *)foo) bar];"); verifyFormat("[((Foo *)foo) bar:1 blech:2];"); + Style.ColumnLimit = 20; + verifyFormat("a = [a aa:aa\n" + " aa:aa];"); + verifyFormat("aa = [aa aa:aa\n" + " aa:aa];"); + Style.ColumnLimit = 70; verifyFormat( "void f() {\n" Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -515,11 +515,23 @@ } Left->MatchingParen = CurrentToken; CurrentToken->MatchingParen = Left; +// FirstObjCSelectorName is set when a colon is found. This does +// not work, however, when method has no parameters. +// Here, we set FirstObjCSelectorName when the end of the expression is +// reached, in case it was not set already. +if (!Contexts.back().FirstObjCSelectorName) { +FormatToken* Previous = CurrentToken->getPreviousNonComment(); +if (Previous && Previous->is(TT_SelectorName)) { + Previous->ObjCSelectorNameParts = 1; + Contexts.back().FirstObjCSelectorName = Previous; +} +} else { + Left->ParameterCount = + Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts; +} if (Contexts.back().FirstObjCSelectorName) { Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = Contexts.back().LongestObjCSelectorName; - Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts = - Left->ParameterCount; if (Left->BlockParameterCount > 1) Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0; } @@ -539,11 +551,6 @@ TT_DesignatedInitializerLSquare)) { Left->Type = TT_ObjCMethodExpr; StartsObjCMethodExpr = true; - // ParameterCount might have been set to 1 before expression was - // recognized as ObjCMethodExpr (as '1 + number of commas' formula is - // used for other expression types). Parameter counter has to be, - // therefore, reset to 0. - Left->ParameterCount = 0; Contexts.back().ColonIsObjCMethodExpr = true; if (Parent && Parent->is(tok::r_paren)) // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen. @@ -617,12 +624,12 @@ } void updateParameterCount(FormatToken *Left, FormatToken *Current) { +// For ObjC methods number of parameters is calculated differently as +// method declarations have different structure (parameters are not inside +// parenthesis scope). if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block) ++Left->BlockParameterCount; -if (Left->Type == TT_ObjCMethodExpr) { - if (Current->is(tok::colon)) -++Left->ParameterCount; -} else if (Current->is(tok::comma)) { +if (Current->is(tok::comma)) { ++Left->ParameterCount; if (!Left->Role) Le
[PATCH] D48721: Patch to fix pragma metadata for do-while loops
deepak2427 created this revision. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D48721 Files: lib/CodeGen/CGStmt.cpp Index: lib/CodeGen/CGStmt.cpp === --- lib/CodeGen/CGStmt.cpp +++ lib/CodeGen/CGStmt.cpp @@ -777,11 +777,6 @@ // Emit the body of the loop. llvm::BasicBlock *LoopBody = createBasicBlock("do.body"); - const SourceRange &R = S.getSourceRange(); - LoopStack.push(LoopBody, CGM.getContext(), DoAttrs, - SourceLocToDebugLoc(R.getBegin()), - SourceLocToDebugLoc(R.getEnd())); - EmitBlockWithFallThrough(LoopBody, &S); { RunCleanupsScope BodyScope(*this); @@ -790,6 +785,11 @@ EmitBlock(LoopCond.getBlock()); + const SourceRange &R = S.getSourceRange(); + LoopStack.push(LoopBody, CGM.getContext(), DoAttrs, + SourceLocToDebugLoc(R.getBegin()), + SourceLocToDebugLoc(R.getEnd())); + // C99 6.8.5.2: "The evaluation of the controlling expression takes place // after each execution of the loop body." Index: lib/CodeGen/CGStmt.cpp === --- lib/CodeGen/CGStmt.cpp +++ lib/CodeGen/CGStmt.cpp @@ -777,11 +777,6 @@ // Emit the body of the loop. llvm::BasicBlock *LoopBody = createBasicBlock("do.body"); - const SourceRange &R = S.getSourceRange(); - LoopStack.push(LoopBody, CGM.getContext(), DoAttrs, - SourceLocToDebugLoc(R.getBegin()), - SourceLocToDebugLoc(R.getEnd())); - EmitBlockWithFallThrough(LoopBody, &S); { RunCleanupsScope BodyScope(*this); @@ -790,6 +785,11 @@ EmitBlock(LoopCond.getBlock()); + const SourceRange &R = S.getSourceRange(); + LoopStack.push(LoopBody, CGM.getContext(), DoAttrs, + SourceLocToDebugLoc(R.getBegin()), + SourceLocToDebugLoc(R.getEnd())); + // C99 6.8.5.2: "The evaluation of the controlling expression takes place // after each execution of the loop body." ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48574: OpenBSD driver needs ld.lld in sanitiser context
dberris accepted this revision. dberris added a comment. This revision is now accepted and ready to land. LGTM PS. I really wish at some point this will lead to an OpenBSD build bot! Repository: rC Clang https://reviews.llvm.org/D48574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48719: [clang-format] Fix split priorities for ObjC methods
jolesiak updated this revision to Diff 153313. jolesiak added a comment. Fix base change. Repository: rC Clang https://reviews.llvm.org/D48719 Files: lib/Format/FormatToken.h lib/Format/TokenAnnotator.cpp unittests/Format/FormatTestObjC.cpp Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -678,6 +678,18 @@ verifyFormat("[(id)foo bar:(id) ? baz : quux];"); verifyFormat("4 > 4 ? (id)a : (id)baz;"); + unsigned PreviousColumnLimit = Style.ColumnLimit; + Style.ColumnLimit = 50; + // Instead of: + // bool a = + // ([object a:42] == 0 || [object a:42 + //b:42] == 0); + verifyFormat("bool a = ([object a:42] == 0 ||\n" + " [object a:42 b:42] == 0);"); + Style.ColumnLimit = PreviousColumnLimit; + verifyFormat("bool a = ([ a] == a ||\n" + " [ a] == );"); + // This tests that the formatter doesn't break after "backing" but before ":", // which would be at 80 columns. verifyFormat( @@ -754,11 +766,10 @@ "[self a:aaa, aaa, aaa,\n" "aaa, aaa, aaa,\n" "aaa, aaa];"); + verifyFormat("[self // break\n" " a:a\n" "aaa:aaa];"); - verifyFormat("bool a = ([ a] == a ||\n" - " [ a] == );"); // Formats pair-parameters. verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];"); @@ -803,6 +814,12 @@ verifyFormat("[((Foo *)foo) bar];"); verifyFormat("[((Foo *)foo) bar:1 blech:2];"); + Style.ColumnLimit = 20; + verifyFormat("a = [a aa:aa\n" + " aa:aa];"); + verifyFormat("aa = [aa aa:aa\n" + " aa:aa];"); + Style.ColumnLimit = 70; verifyFormat( "void f() {\n" Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -719,6 +719,8 @@ Contexts.back().LongestObjCSelectorName) Contexts.back().LongestObjCSelectorName = Tok->Previous->ColumnWidth; + Tok->Previous->ParameterIndex = + Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts; ++Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts; } } else if (Contexts.back().ColonIsForRangeExpr) { @@ -2136,8 +2138,20 @@ // FIXME: Only calculate this if CanBreakBefore is true once static // initializers etc. are sorted out. // FIXME: Move magic numbers to a better place. -Current->SplitPenalty = 20 * Current->BindingStrength + -splitPenalty(Line, *Current, InFunctionDecl); + +// Reduce penalty for aligning ObjC method arguments using the colon +// alignment as this is the canonical way (still prefer fitting everything +// into one line if possible). Trying to fit a whole expression into one +// line should not force other line breaks (e.g. when ObjC method +// expression is a part of other expression). +Current->SplitPenalty = splitPenalty(Line, *Current, InFunctionDecl); +if (Style.Language == FormatStyle::LK_ObjC && +Current->is(TT_SelectorName) && Current->ParameterIndex > 0) { + if (Current->ParameterIndex == 1) +Current->SplitPenalty += 5 * Current->BindingStrength; +} else { + Current->SplitPenalty += 20 * Current->BindingStrength; +} Current = Current->Next; } Index: lib/Format/FormatToken.h === --- lib/Format/FormatToken.h +++ lib/Format/FormatToken.h @@ -248,6 +248,11 @@ /// consist of. unsigned ObjCSelectorNameParts = 0; + /// The 0-based index of the parameter/argument. For ObjC it is set + /// for the selector name token. + /// For now calculated only for ObjC. + unsigned ParameterIndex = 0; + /// Stores the number of required fake parentheses and the /// corresponding operator precedence. /// ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r335854 - Fix unittest build with GCC older than 5.
Thanks! I was going to push the same fix, but you also explained why it's needed ;) On Thu, Jun 28, 2018 at 3:36 PM Benjamin Kramer via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: d0k > Date: Thu Jun 28 06:31:36 2018 > New Revision: 335854 > > URL: http://llvm.org/viewvc/llvm-project?rev=335854&view=rev > Log: > Fix unittest build with GCC older than 5. > > Old GCCs have an annoying bug where RVO disables the automatic > conversion to base for unique_ptr. Add a pessimizing std::move as a > workaround. > > Modified: > cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp > > Modified: cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp?rev=335854&r1=335853&r2=335854&view=diff > > == > --- cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp > (original) > +++ cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp Thu > Jun 28 06:31:36 2018 > @@ -61,7 +61,7 @@ public: > AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry > &Registry) { >Registry.addChecker("custom.CustomChecker", > "Description"); > }); > -return AnalysisConsumer; > +return std::move(AnalysisConsumer); >} > }; > > > > ___ > 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] D32845: [Analyzer] Iterator Checker - Part 4: Mismatched iterator checker for function parameters
baloghadamsoftware updated this revision to Diff 153314. baloghadamsoftware added a comment. Herald added a reviewer: george.karpenkov. Herald added a subscriber: mikhail.ramalho. Rebased to https://reviews.llvm.org/rL335835. https://reviews.llvm.org/D32845 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/IteratorChecker.cpp test/Analysis/Inputs/system-header-simulator-cxx.h test/Analysis/diagnostics/explicit-suppression.cpp test/Analysis/invalidated-iterator.cpp test/Analysis/mismatched-iterator.cpp Index: test/Analysis/mismatched-iterator.cpp === --- /dev/null +++ test/Analysis/mismatched-iterator.cpp @@ -0,0 +1,25 @@ +// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.MismatchedIterator -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=false %s -verify +// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.MismatchedIterator -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify + +#include "Inputs/system-header-simulator-cxx.h" + +void good_find(std::vector &v, int n) { + std::find(v.cbegin(), v.cend(), n); // no-warning +} + +void good_find_first_of(std::vector &v1, std::vector &v2) { + std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v2.cend()); // no-warning +} + +void good_copy(std::vector &v1, std::vector &v2, int n) { + std::copy(v1.cbegin(), v1.cend(), v2.begin()); // no-warning +} + +void bad_find(std::vector &v1, std::vector &v2, int n) { + std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterator access mismatched}} +} + +void bad_find_first_of(std::vector &v1, std::vector &v2) { + std::find_first_of(v1.cbegin(), v2.cend(), v2.cbegin(), v2.cend()); // expected-warning{{Iterator access mismatched}} + std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v1.cend()); // expected-warning{{Iterator access mismatched}} +} Index: test/Analysis/invalidated-iterator.cpp === --- /dev/null +++ test/Analysis/invalidated-iterator.cpp @@ -0,0 +1,32 @@ +// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=false %s -verify +// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify + +#include "Inputs/system-header-simulator-cxx.h" + +void bad_copy_assign_operator_list1(std::list &L1, +const std::list &L2) { + auto i0 = L1.cbegin(); + L1 = L2; + *i0; // expected-warning{{Invalidated iterator accessed}} +} + +void bad_copy_assign_operator_vector1(std::vector &V1, + const std::vector &V2) { + auto i0 = V1.cbegin(); + V1 = V2; + *i0; // expected-warning{{Invalidated iterator accessed}} +} + +void bad_copy_assign_operator_deque1(std::deque &D1, + const std::deque &D2) { + auto i0 = D1.cbegin(); + D1 = D2; + *i0; // expected-warning{{Invalidated iterator accessed}} +} + +void bad_copy_assign_operator_forward_list1(std::forward_list &FL1, +const std::forward_list &FL2) { + auto i0 = FL1.cbegin(); + FL1 = FL2; + *i0; // expected-warning{{Invalidated iterator accessed}} +} Index: test/Analysis/diagnostics/explicit-suppression.cpp === --- test/Analysis/diagnostics/explicit-suppression.cpp +++ test/Analysis/diagnostics/explicit-suppression.cpp @@ -19,6 +19,6 @@ void testCopyNull(C *I, C *E) { std::copy(I, E, (C *)0); #ifndef SUPPRESSED - // expected-warning@../Inputs/system-header-simulator-cxx.h:514 {{Called C++ object pointer is null}} + // expected-warning@../Inputs/system-header-simulator-cxx.h:554 {{Called C++ object pointer is null}} #endif } Index: test/Analysis/Inputs/system-header-simulator-cxx.h === --- test/Analysis/Inputs/system-header-simulator-cxx.h +++ test/Analysis/Inputs/system-header-simulator-cxx.h @@ -252,6 +252,15 @@ return size_t(_finish - _start); } +vector& operator=(const vector &other); +vector& operator=(vector &&other); +vector& operator=(std::initializer_list ilist); + +void assign(size_type count, const T &value); +template +void assign(InputIterator first, InputIterator last); +void assign(std::initi
[PATCH] D48721: Patch to fix pragma metadata for do-while loops
lebedev.ri added a comment. Test? (Or was this meant to contain `[Private]` in title?) Repository: rC Clang https://reviews.llvm.org/D48721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48574: OpenBSD driver needs ld.lld in sanitiser context
devnexen added a comment. In https://reviews.llvm.org/D48574#1146619, @dberris wrote: > LGTM > > PS. I really wish at some point this will lead to an OpenBSD build bot! Would be interesting, it can even launch ubsan minimal tests and this change will simplify things a bit. Repository: rC Clang https://reviews.llvm.org/D48574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48352: [clang-format] Improve ObjC method expressions formatting
jolesiak abandoned this revision. jolesiak added a comment. Split: https://reviews.llvm.org/D48716 https://reviews.llvm.org/D48718 https://reviews.llvm.org/D48719 https://reviews.llvm.org/D48720 Repository: rC Clang https://reviews.llvm.org/D48352 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well
alexfh added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp:24 + const auto AllPointerTypes = anyOf( + hasType(pointerType()), hasType(autoType(hasDeducedType(pointerType(); Can anyOf be pushed inside hasType? (`hasType(anyOf(pointerType(), autoType(...)))`) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48717 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48717 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response
hokein added a comment. > Could we try to figure out why the duplicates were there in the first place > and why the paths were different? +1, I think there are two issues here: 1. the result contains two candidates, which should be one, IIUC. 2. the absolute file path problem, we encountered similar problem in SymbolCollector, and we have similar function `ToURI` there, I think we can share the implementation instead of having different duplications. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48708: NFC Build fix in RegisterCustomCheckersTest.cpp
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp:64 }); -return AnalysisConsumer; +return std::unique_ptr(AnalysisConsumer.release()); } d0k has committed a better fix in r335854 (a workaround for GCC < 5, actually, since the original code was fine w.r.t. the standard). Repository: rC Clang https://reviews.llvm.org/D48708 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48574: OpenBSD driver needs ld.lld in sanitiser context
This revision was automatically updated to reflect the committed changes. Closed by commit rC335856: OpenBSD driver needs ld.lld in sanitizer context (authored by devnexen, committed by ). Repository: rC Clang https://reviews.llvm.org/D48574 Files: lib/Driver/ToolChains/OpenBSD.cpp Index: lib/Driver/ToolChains/OpenBSD.cpp === --- lib/Driver/ToolChains/OpenBSD.cpp +++ lib/Driver/ToolChains/OpenBSD.cpp @@ -230,7 +230,9 @@ Args.MakeArgString(getToolChain().GetFilePath("crtendS.o"))); } - const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath()); + const char *Exec = Args.MakeArgString( + !NeedsSanitizerDeps ? getToolChain().GetLinkerPath() + : getToolChain().GetProgramPath("ld.lld")); C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs)); } Index: lib/Driver/ToolChains/OpenBSD.cpp === --- lib/Driver/ToolChains/OpenBSD.cpp +++ lib/Driver/ToolChains/OpenBSD.cpp @@ -230,7 +230,9 @@ Args.MakeArgString(getToolChain().GetFilePath("crtendS.o"))); } - const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath()); + const char *Exec = Args.MakeArgString( + !NeedsSanitizerDeps ? getToolChain().GetLinkerPath() + : getToolChain().GetProgramPath("ld.lld")); C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs)); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D17700: [clang-format] Proposal for changes to Objective-C block formatting
Myrronth added a comment. Do you mind sharing your modified version? The fork in the discussion you mentioned (https://github.com/square/spacecommander/issues/33) seems to be out-of-date or at least does not have the AllowNewlineBeforeBlockParameter setting. https://reviews.llvm.org/D17700 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48721: Patch to fix pragma metadata for do-while loops
deepak2427 added a comment. It's a patch for a bug in clang. I have requested for a Bugzilla account, however thought of putting up the patch in the meantime. Do I need to mark it '[Private]'? Repository: rC Clang https://reviews.llvm.org/D48721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48561: [Analyzer] Moved RangeConstraintManager to header. NFC.
mikhail.ramalho added a comment. In https://reviews.llvm.org/D48561#1146114, @george.karpenkov wrote: > After thinking about this change a bit longer, I think it does not make sense. > > Albeit poorly named, the previous design had a purpose: > `RangedConstraintManager` is a public interface, and `RangeConstraintManager` > is a private implementation. > Exposing both in the header does not make sense. > > For exposing the factory could you just move the factory and it's getter? > Another solution is just merging the two classes entirely, but that's more > heavyweight, and would force exposing private functions in a header (but > those could be just moved to static C functions). > @NoQ further comments? Since we decided to go with the other approach in https://reviews.llvm.org/D48565, we don't actually need this patch anymore. Repository: rC Clang https://reviews.llvm.org/D48561 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well
aaron.ballman requested changes to this revision. aaron.ballman added inline comments. This revision now requires changes to proceed. Comment at: test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic-pr36489.cpp:4 + +// Fix PR36489 and detect auto-deduced value correctly. +char *getPtr(); Can this also include tests for other deduced types, like `decltype(expr)` and `decltype(auto)`? Comment at: test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp:88 - for(int ii : a) ; // OK, pointer arithmetic generated by compiler + for (int ii : a) ; // OK, pointer arithmetic generated by compiler } Spurious whitespace change. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48717 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r335856 - OpenBSD driver needs ld.lld in sanitizer context
Author: devnexen Date: Thu Jun 28 06:49:41 2018 New Revision: 335856 URL: http://llvm.org/viewvc/llvm-project?rev=335856&view=rev Log: OpenBSD driver needs ld.lld in sanitizer context Base GNU ld is pretty ancient and does not support --dynamic-list flag. For conveniency, we can it automatically when compile with ubsan sanitizer flag. Reviewers: dberris Reviewed by: dberris Differential Revision: https://reviews.llvm.org/D48574 Modified: cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp Modified: cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp?rev=335856&r1=335855&r2=335856&view=diff == --- cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains/OpenBSD.cpp Thu Jun 28 06:49:41 2018 @@ -230,7 +230,9 @@ void openbsd::Linker::ConstructJob(Compi Args.MakeArgString(getToolChain().GetFilePath("crtendS.o"))); } - const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath()); + const char *Exec = Args.MakeArgString( + !NeedsSanitizerDeps ? getToolChain().GetLinkerPath() + : getToolChain().GetProgramPath("ld.lld")); C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs)); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D48721: Patch to fix pragma metadata for do-while loops
It's a patch for a bug in clang. I have requested for a Bugzilla account, however thought of putting up the patch in the meantime. Do I need to mark it '[Private]'? On Thu, Jun 28, 2018 at 2:41 PM Roman Lebedev via Phabricator < revi...@reviews.llvm.org> wrote: > lebedev.ri added a comment. > > Test? > (Or was this meant to contain `[Private]` in title?) > > > Repository: > rC Clang > > https://reviews.llvm.org/D48721 > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48721: Patch to fix pragma metadata for do-while loops
xbolva00 added a comment. Please upload patch with full context Repository: rC Clang https://reviews.llvm.org/D48721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48721: Patch to fix pragma metadata for do-while loops
lebedev.ri added a comment. In https://reviews.llvm.org/D48721#1146662, @deepak2427 wrote: > It's a patch for a bug in clang. > I have requested for a Bugzilla account, however thought of putting up the > patch in the meantime. > Do I need to mark it '[Private]'? Phab is the correct way to submit patches. But having a bugreport in bugzilla is good too. But the test will be needed regardless of the patch submission method. And yes, please do always upload all patches with full context (`-U9`). Repository: rC Clang https://reviews.llvm.org/D48721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48721: Patch to fix pragma metadata for do-while loops
deepak2427 updated this revision to Diff 153316. deepak2427 added a comment. Add full context https://reviews.llvm.org/D48721 Files: lib/CodeGen/CGStmt.cpp Index: lib/CodeGen/CGStmt.cpp === --- lib/CodeGen/CGStmt.cpp +++ lib/CodeGen/CGStmt.cpp @@ -777,19 +777,19 @@ // Emit the body of the loop. llvm::BasicBlock *LoopBody = createBasicBlock("do.body"); - const SourceRange &R = S.getSourceRange(); - LoopStack.push(LoopBody, CGM.getContext(), DoAttrs, - SourceLocToDebugLoc(R.getBegin()), - SourceLocToDebugLoc(R.getEnd())); - EmitBlockWithFallThrough(LoopBody, &S); { RunCleanupsScope BodyScope(*this); EmitStmt(S.getBody()); } EmitBlock(LoopCond.getBlock()); + const SourceRange &R = S.getSourceRange(); + LoopStack.push(LoopBody, CGM.getContext(), DoAttrs, + SourceLocToDebugLoc(R.getBegin()), + SourceLocToDebugLoc(R.getEnd())); + // C99 6.8.5.2: "The evaluation of the controlling expression takes place // after each execution of the loop body." Index: lib/CodeGen/CGStmt.cpp === --- lib/CodeGen/CGStmt.cpp +++ lib/CodeGen/CGStmt.cpp @@ -777,19 +777,19 @@ // Emit the body of the loop. llvm::BasicBlock *LoopBody = createBasicBlock("do.body"); - const SourceRange &R = S.getSourceRange(); - LoopStack.push(LoopBody, CGM.getContext(), DoAttrs, - SourceLocToDebugLoc(R.getBegin()), - SourceLocToDebugLoc(R.getEnd())); - EmitBlockWithFallThrough(LoopBody, &S); { RunCleanupsScope BodyScope(*this); EmitStmt(S.getBody()); } EmitBlock(LoopCond.getBlock()); + const SourceRange &R = S.getSourceRange(); + LoopStack.push(LoopBody, CGM.getContext(), DoAttrs, + SourceLocToDebugLoc(R.getBegin()), + SourceLocToDebugLoc(R.getEnd())); + // C99 6.8.5.2: "The evaluation of the controlling expression takes place // after each execution of the loop body." ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly
alexfh added inline comments. Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:191 +void templated_thrower() { throw T{}(); } +// CHECK-MESSAGES: [[@LINE-1]]:34: warning: throwing an exception whose type 'int' is not derived from 'std::exception' + hokein wrote: > I think giving message on the template function here is confusing to users > even it gets instantiated somewhere in this TU -- because users have to find > the location that triggers the template instantiation. > > Maybe > 1) Add a note which gives the instantiation location to the message, or > 2) ignore template case (some clang-tidy checks do this) In this particular case it seems to be useful to get warnings from template instantiations. But the message will indeed be confusing. Ideally, the message should have "in instantiation of xxx requested here" notes attached, as clang warnings do. But this is not working automatically, and it's implemented in Sema (`Sema::PrintInstantiationStack()` in lib/Sema/SemaTemplateInstantiate.cpp). I wonder whether it's feasible to produce similar notes after Sema is dead? Maybe not the whole instantiation stack, but at least it should be possible to figure out that the enclosing function is a template instantiation or is a member function of an type that is an instantiation of a template. That would be useful for other checks as well. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48721: Patch to fix pragma metadata for do-while loops
deepak2427 added a comment. > Phab is the correct way to submit patches. > But having a bugreport in bugzilla is good too. > But the test will be needed regardless of the patch submission method. > And yes, please do always upload all patches with full context (`-U9`). Sorry about the context. Can I add the test file to this patch itself? Or should that be another patch? https://reviews.llvm.org/D48721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D17700: [clang-format] Proposal for changes to Objective-C block formatting
ksuther added a comment. My fork of spacecommander has a version of clang-format with the option. It's a couple of years old at this point, but it has been running without any issues. https://reviews.llvm.org/D17700 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D48721: Patch to fix pragma metadata for do-while loops
> > Phab is the correct way to submit patches. > But having a bugreport in bugzilla is good too. > But the test will be needed regardless of the patch submission method. > And yes, please do always upload all patches with full context (`-U9`). Sorry about the context. Can I add the test file to this patch itself? Or should that be another patch? On Thu, Jun 28, 2018 at 3:08 PM Deepak Panickal via Phabricator < revi...@reviews.llvm.org> wrote: > deepak2427 updated this revision to Diff 153316. > deepak2427 added a comment. > > Add full context > > > https://reviews.llvm.org/D48721 > > Files: > lib/CodeGen/CGStmt.cpp > > > Index: lib/CodeGen/CGStmt.cpp > === > --- lib/CodeGen/CGStmt.cpp > +++ lib/CodeGen/CGStmt.cpp > @@ -777,19 +777,19 @@ >// Emit the body of the loop. >llvm::BasicBlock *LoopBody = createBasicBlock("do.body"); > > - const SourceRange &R = S.getSourceRange(); > - LoopStack.push(LoopBody, CGM.getContext(), DoAttrs, > - SourceLocToDebugLoc(R.getBegin()), > - SourceLocToDebugLoc(R.getEnd())); > - >EmitBlockWithFallThrough(LoopBody, &S); >{ > RunCleanupsScope BodyScope(*this); > EmitStmt(S.getBody()); >} > >EmitBlock(LoopCond.getBlock()); > > + const SourceRange &R = S.getSourceRange(); > + LoopStack.push(LoopBody, CGM.getContext(), DoAttrs, > + SourceLocToDebugLoc(R.getBegin()), > + SourceLocToDebugLoc(R.getEnd())); > + >// C99 6.8.5.2: "The evaluation of the controlling expression takes > place >// after each execution of the loop body." > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r335858 - Fix test that was failing on Windows due to too many backslashes
Author: filcab Date: Thu Jun 28 07:16:13 2018 New Revision: 335858 URL: http://llvm.org/viewvc/llvm-project?rev=335858&view=rev Log: Fix test that was failing on Windows due to too many backslashes Modified: cfe/trunk/test/Driver/linux-per-target-runtime-dir.c Modified: cfe/trunk/test/Driver/linux-per-target-runtime-dir.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/linux-per-target-runtime-dir.c?rev=335858&r1=335857&r2=335858&view=diff == --- cfe/trunk/test/Driver/linux-per-target-runtime-dir.c (original) +++ cfe/trunk/test/Driver/linux-per-target-runtime-dir.c Thu Jun 28 07:16:13 2018 @@ -18,4 +18,4 @@ // RUN: --target=x86_64-linux-gnu \ // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \ // RUN: | FileCheck --check-prefix=CHECK-CLANGRT-X8664 %s -// CHECK-CLANGRT-X8664: x86_64-linux-gnu{{/|}}lib{{/|}}libclang_rt.builtins.a +// CHECK-CLANGRT-X8664: x86_64-linux-gnu{{/|\\}}lib{{/|\\}}libclang_rt.builtins.a ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48721: Patch to fix pragma metadata for do-while loops
lebedev.ri added a comment. In https://reviews.llvm.org/D48721#1146681, @deepak2427 wrote: > > Phab is the correct way to submit patches. > > But having a bugreport in bugzilla is good too. > > But the test will be needed regardless of the patch submission method. > > And yes, please do always upload all patches with full context (`-U9`). > > Sorry about the context. > Can I add the test file to this patch itself? Or should that be another > patch? The test needs to be in `test/CodeGen/` (i'm guessing). See other files there for examples. (And do check that `$ ninja check-clang` passes) https://reviews.llvm.org/D48721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48722: [ASTImporter] Update isUsed flag at Decl import.
balazske created this revision. Herald added subscribers: cfe-commits, martong. Herald added a reviewer: a.sidorin. When a Decl is imported over an already existing similar one, the isUsed flag is updated if it is set in the imported Decl. This can happen if the Decl is used somewhere in the imported code but was not used in the existing code. Repository: rC Clang https://reviews.llvm.org/D48722 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -290,13 +290,24 @@ std::string FileName; std::unique_ptr Unit; TranslationUnitDecl *TUDecl = nullptr; +std::unique_ptr Importer; TU(StringRef Code, StringRef FileName, ArgVector Args) : Code(Code), FileName(FileName), Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args, this->FileName)), TUDecl(Unit->getASTContext().getTranslationUnitDecl()) { Unit->enableSourceFileDiagnostics(); } + +Decl *import(ASTUnit *ToAST, Decl *FromDecl) { + assert(ToAST); + if (!Importer) { +Importer.reset(new ASTImporter( +ToAST->getASTContext(), ToAST->getFileManager(), +Unit->getASTContext(), Unit->getFileManager(), false)); + } + return Importer->Import(FromDecl); +} }; // We may have several From contexts and related translation units. In each @@ -329,14 +340,10 @@ ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName); ToAST->enableSourceFileDiagnostics(); -ASTContext &FromCtx = FromTU.Unit->getASTContext(), - &ToCtx = ToAST->getASTContext(); +ASTContext &FromCtx = FromTU.Unit->getASTContext(); createVirtualFileIfNeeded(ToAST.get(), InputFileName, FromTU.Code); -ASTImporter Importer(ToCtx, ToAST->getFileManager(), FromCtx, - FromTU.Unit->getFileManager(), false); - IdentifierInfo *ImportedII = &FromCtx.Idents.get(Identifier); assert(ImportedII && "Declaration with the given identifier " "should be specified in test!"); @@ -347,7 +354,8 @@ assert(FoundDecls.size() == 1); -Decl *Imported = Importer.Import(FoundDecls.front()); +Decl *Imported = FromTU.import(ToAST.get(), FoundDecls.front()); + assert(Imported); return std::make_tuple(*FoundDecls.begin(), Imported); } @@ -401,11 +409,7 @@ assert(It != FromTUs.end()); createVirtualFileIfNeeded(ToAST.get(), It->FileName, It->Code); -ASTContext &FromCtx = From->getASTContext(), - &ToCtx = ToAST->getASTContext(); -ASTImporter Importer(ToCtx, ToAST->getFileManager(), FromCtx, - FromCtx.getSourceManager().getFileManager(), false); -return Importer.Import(From); +return It->import(ToAST.get(), From); } ~ASTImporterTestBase() { @@ -469,6 +473,48 @@ EXPECT_THAT(RedeclsD1, ::testing::ContainerEq(RedeclsD2)); } +TEST_P(ASTImporterTestBase, ImportDoesUpdateUsedFlag) { + auto Pattern = varDecl(hasName("x")); + VarDecl *Imported1; + { +Decl *FromTU = getTuDecl("extern int x;", Lang_CXX, "input0.cc"); +auto *FromD = FirstDeclMatcher().match(FromTU, Pattern); +Imported1 = cast(Import(FromD, Lang_CXX)); + } + VarDecl *Imported2; + { +Decl *FromTU = getTuDecl("int x;", Lang_CXX, "input1.cc"); +auto *FromD = FirstDeclMatcher().match(FromTU, Pattern); +Imported2 = cast(Import(FromD, Lang_CXX)); + } + EXPECT_EQ(Imported1->getCanonicalDecl(), Imported2->getCanonicalDecl()); + EXPECT_FALSE(Imported2->isUsed(false)); + { +Decl *FromTU = getTuDecl( +"extern int x; int f() { return x; }", Lang_CXX, "input2.cc"); +auto *FromD = +FirstDeclMatcher().match(FromTU, functionDecl()); +Import(FromD, Lang_CXX); + } + EXPECT_TRUE(Imported2->isUsed(false)); +} + +TEST_P(ASTImporterTestBase, ReimportWithUsedFlag) { + auto Pattern = varDecl(hasName("x")); + + Decl *FromTU = getTuDecl("int x;", Lang_CXX, "input0.cc"); + auto *FromD = FirstDeclMatcher().match(FromTU, Pattern); + + auto *Imported1 = cast(Import(FromD, Lang_CXX)); + + ASSERT_FALSE(Imported1->isUsed(false)); + + FromD->setIsUsed(); + auto *Imported2 = cast(Import(FromD, Lang_CXX)); + + EXPECT_EQ(Imported1, Imported2); + EXPECT_TRUE(Imported2->isUsed(false)); +} TEST_P(ImportExpr, ImportStringLiteral) { MatchVerifier Verifier; Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -90,6 +90,13 @@ return getCanonicalForwardRedeclChain(FD); } + void updateAttrAndFlags(const Decl *From, Decl *To) { +// Check if some flags or attrs are new in 'From' and copy into 'To'. +// FIXME: Other f
[PATCH] D48715: [X86] Fix some vector cmp builtins - TRUE/FALSE predicates
spatel added inline comments. Comment at: test/CodeGen/avx-builtins.c:1423 - -__m256 test_mm256_cmp_ps_true(__m256 a, __m256 b) { - // CHECK-LABEL: @test_mm256_cmp_ps_true Why are we deleting tests instead of replacing the CHECK lines with the new output? Repository: rC Clang https://reviews.llvm.org/D48715 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48615: [CUDA] Place all CUDA sections in __NV_CUDA segment on Mac.
Hahnfeld accepted this revision. Hahnfeld added a comment. This revision is now accepted and ready to land. LGTM. These were the section names I suspected, but I wasn't able to verify back then. https://reviews.llvm.org/D48615 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48715: [X86] Fix some vector cmp builtins - TRUE/FALSE predicates
GBuella added inline comments. Comment at: test/CodeGen/avx-builtins.c:1423 - -__m256 test_mm256_cmp_ps_true(__m256 a, __m256 b) { - // CHECK-LABEL: @test_mm256_cmp_ps_true spatel wrote: > Why are we deleting tests instead of replacing the CHECK lines with the new > output? These cases were only added, when the TRUE/FALSE related special cases were added in `CGBuiltin.cpp`. Now that the special cases are removed from `CGBuiltin.cpp`, it seemed consistent to also remove the related special cases from the tests, as these are not special anymore. Should we have tests with all predicates? Repository: rC Clang https://reviews.llvm.org/D48715 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D17700: [clang-format] Proposal for changes to Objective-C block formatting
Myrronth added a comment. Thanks, I’ll give it a try. https://reviews.llvm.org/D17700 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48724: [clangd] Log sema completion context kind and query scopes. NFC
ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48724 Files: clangd/CodeComplete.cpp Index: clangd/CodeComplete.cpp === --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -40,6 +40,7 @@ #include "clang/Sema/Sema.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/Support/Format.h" +#include "llvm/Support/FormatVariadic.h" #include // We log detailed candidate here if you run with -debug-only=codecomplete. @@ -939,6 +940,7 @@ int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging. bool Incomplete = false; // Would more be available with a higher limit? llvm::Optional Filter; // Initialized once Sema runs. + std::vector QueryScopes; // Initialized once Sema runs. std::unique_ptr Includes; // Initialized once compiler runs. FileProximityMatcher FileProximityMatch; @@ -968,6 +970,10 @@ Includes.reset(); // Make sure this doesn't out-live Clang. SPAN_ATTACH(Tracer, "sema_completion_kind", getCompletionKindString(Recorder->CCContext.getKind())); + log(llvm::formatv( + "Code complete: sema context {0}, query scopes [{1}]", + getCompletionKindString(Recorder->CCContext.getKind()), + llvm::join(QueryScopes.begin(), QueryScopes.end(), ","))); }); Recorder = RecorderOwner.get(); @@ -995,6 +1001,8 @@ CompletionList runWithSema() { Filter = FuzzyMatcher( Recorder->CCSema->getPreprocessor().getCodeCompletionFilter()); +QueryScopes = getQueryScopes(Recorder->CCContext, + Recorder->CCSema->getSourceManager()); // Sema provides the needed context to query the index. // FIXME: in addition to querying for extra/overlapping symbols, we should //explicitly request symbols corresponding to Sema results. @@ -1024,8 +1032,7 @@ Req.MaxCandidateCount = Opts.Limit; Req.Query = Filter->pattern(); Req.RestrictForCodeCompletion = true; -Req.Scopes = getQueryScopes(Recorder->CCContext, -Recorder->CCSema->getSourceManager()); +Req.Scopes = QueryScopes; Req.ProximityPaths.push_back(FileName); log(llvm::formatv("Code complete: fuzzyFind(\"{0}\", scopes=[{1}])", Req.Query, Index: clangd/CodeComplete.cpp === --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -40,6 +40,7 @@ #include "clang/Sema/Sema.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/Support/Format.h" +#include "llvm/Support/FormatVariadic.h" #include // We log detailed candidate here if you run with -debug-only=codecomplete. @@ -939,6 +940,7 @@ int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging. bool Incomplete = false; // Would more be available with a higher limit? llvm::Optional Filter; // Initialized once Sema runs. + std::vector QueryScopes; // Initialized once Sema runs. std::unique_ptr Includes; // Initialized once compiler runs. FileProximityMatcher FileProximityMatch; @@ -968,6 +970,10 @@ Includes.reset(); // Make sure this doesn't out-live Clang. SPAN_ATTACH(Tracer, "sema_completion_kind", getCompletionKindString(Recorder->CCContext.getKind())); + log(llvm::formatv( + "Code complete: sema context {0}, query scopes [{1}]", + getCompletionKindString(Recorder->CCContext.getKind()), + llvm::join(QueryScopes.begin(), QueryScopes.end(), ","))); }); Recorder = RecorderOwner.get(); @@ -995,6 +1001,8 @@ CompletionList runWithSema() { Filter = FuzzyMatcher( Recorder->CCSema->getPreprocessor().getCodeCompletionFilter()); +QueryScopes = getQueryScopes(Recorder->CCContext, + Recorder->CCSema->getSourceManager()); // Sema provides the needed context to query the index. // FIXME: in addition to querying for extra/overlapping symbols, we should //explicitly request symbols corresponding to Sema results. @@ -1024,8 +1032,7 @@ Req.MaxCandidateCount = Opts.Limit; Req.Query = Filter->pattern(); Req.RestrictForCodeCompletion = true; -Req.Scopes = getQueryScopes(Recorder->CCContext, -Recorder->CCSema->getSourceManager()); +Req.Scopes = QueryScopes; Req.ProximityPaths.push_back(FileName); log(llvm::formatv("Code complete: fuzzyFind(\"{0}\", scopes=[{1}])", Req.Query, ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response
ilya-biryukov added a comment. I'm somewhat familiar with the internals of clang around the FileManager/VFS, so I could try creating the repro of this issue. The bugreport has enough info to get started. (Probably tomorrow, I certainly won't get to it today). In https://reviews.llvm.org/D48687#1146645, @hokein wrote: > 2. the absolute file path problem, we encountered similar problem in > SymbolCollector, and we have similar function `ToURI` there, I think we can > share the implementation instead of having different duplications. +1 to sharing the code. I guess we're struggling with similar problems here. Any pointers to the functions we should use? Comment at: unittests/clangd/TestTU.h:47 - ParsedAST build() const; + ParsedAST build(IntrusiveRefCntPtr *OutFS = nullptr) const; SymbolSlab headerSymbols() const; simark wrote: > ilya-biryukov wrote: > > We don't need an extra output param here. > > There's a way to get the vfs from the ASTContext: > > `ParsedAST().getASTContext().getSourceManager().getFileManager().getVirtualFileSystem()`. > Thanks. It's just not obvious :). Yeah, it is quite deeply hidden :-) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r335781 - DR1687: When overload resolution selects a built-in operator, implicit
On Wed, Jun 27, 2018 at 10:30 PM, Richard Smith via cfe-commits wrote: > Author: rsmith > Date: Wed Jun 27 13:30:34 2018 > New Revision: 335781 > > URL: http://llvm.org/viewvc/llvm-project?rev=335781&view=rev > Log: > DR1687: When overload resolution selects a built-in operator, implicit > conversions are only applied to operands of class type, and the second > standard conversion sequence is not applied. > > When diagnosing an invalid builtin binary operator, talk about the > original types rather than the converted types. If these differ by a > user-defined conversion, tell the user what happened. This also had the nice side-effect of making -Wconstant-logical-operand more powerful, finding three bugs in Chromium so far: https://crbug.com/857393 :-) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48715: [X86] Fix some vector cmp builtins - TRUE/FALSE predicates
spatel added inline comments. Comment at: test/CodeGen/avx-builtins.c:1423 - -__m256 test_mm256_cmp_ps_true(__m256 a, __m256 b) { - // CHECK-LABEL: @test_mm256_cmp_ps_true GBuella wrote: > spatel wrote: > > Why are we deleting tests instead of replacing the CHECK lines with the new > > output? > These cases were only added, when the TRUE/FALSE related special cases were > added in `CGBuiltin.cpp`. Now that the special cases are removed from > `CGBuiltin.cpp`, it seemed consistent to also remove the related special > cases from the tests, as these are not special anymore. > Should we have tests with all predicates? Yes. IIUC, we would have caught the bug before it was committed if we had proper tests for each predicate and C intrinsic. Repository: rC Clang https://reviews.llvm.org/D48715 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46951: [clang-tidy] misc-unused-parameters - retain old behavior under StrictMode
alexfh updated this revision to Diff 153327. alexfh marked 3 inline comments as done. alexfh added a comment. - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46951 Files: clang-tidy/misc/UnusedParametersCheck.cpp clang-tidy/misc/UnusedParametersCheck.h docs/clang-tidy/checks/misc-unused-parameters.rst test/clang-tidy/misc-unused-parameters-strict.cpp test/clang-tidy/misc-unused-parameters.cpp Index: test/clang-tidy/misc-unused-parameters.cpp === --- test/clang-tidy/misc-unused-parameters.cpp +++ test/clang-tidy/misc-unused-parameters.cpp @@ -222,5 +222,41 @@ return Function(); } +namespace strict_mode_off { // Do not warn on empty function bodies. -void f(int foo) {} +void f1(int foo1) {} +void f2(int foo2) { + // "empty" in the AST sense, not in textual sense. +} +void f3(int foo3) {;} +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: parameter 'foo3' is unused +// CHECK-FIXES: {{^}}void f3(int /*foo3*/) {;}{{$}} + +class E { + int i; + +public: + E(int j) {} +}; +class F { + int i; + +public: + // Constructor initializer counts as a non-empty body. + F(int j) : i() {} +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'j' is unused +// CHECK-FIXES: {{^}} F(int /*j*/) : i() {}{{$}} +}; + +class A { +public: + A(); + A(int); +}; +class B : public A { +public: + B(int i) : A() {} +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is unused +// CHECK-FIXES: {{^}} B(int /*i*/) : A() {}{{$}} +}; +} // namespace strict_mode_off Index: test/clang-tidy/misc-unused-parameters-strict.cpp === --- /dev/null +++ test/clang-tidy/misc-unused-parameters-strict.cpp @@ -0,0 +1,25 @@ +// RUN: %check_clang_tidy %s misc-unused-parameters %t -- \ +// RUN: -config="{CheckOptions: [{key: StrictMode, value: 1}]}" -- + +// Warn on empty function bodies in StrictMode. +namespace strict_mode { +void f(int foo) {} +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'foo' is unused [misc-unused-parameters] +// CHECK-FIXES: {{^}}void f(int /*foo*/) {}{{$}} +class E { + int i; + +public: + E(int j) {} +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'j' is unused +// CHECK-FIXES: {{^}} E(int /*j*/) {}{{$}} +}; +class F { + int i; + +public: + F(int j) : i() {} +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'j' is unused +// CHECK-FIXES: {{^}} F(int /*j*/) : i() {}{{$}} +}; +} Index: docs/clang-tidy/checks/misc-unused-parameters.rst === --- docs/clang-tidy/checks/misc-unused-parameters.rst +++ docs/clang-tidy/checks/misc-unused-parameters.rst @@ -3,24 +3,40 @@ misc-unused-parameters == -Finds unused parameters and fixes them, so that `-Wunused-parameter` can be -turned on. +Finds unused function parameters. Unused parameters may signify a bug in the +code (e.g. when a different parameter is used instead). The suggested fixes +either comment parameter name out or remove the parameter completely, if all +callers of the function are in the same translation unit and can be updated. + +The check is similar to the `-Wunused-parameter` compiler diagnostic and can be +used to prepare a codebase to enabling of that diagnostic. By default the check +is more permissive (see :option:`StrictMode`). .. code-block:: c++ - void a(int i) {} + void a(int i) { /*some code that doesn't use `i`*/ } // becomes - void a(int /*i*/) {} - + void a(int /*i*/) { /*some code that doesn't use `i`*/ } .. code-block:: c++ static void staticFunctionA(int i); - static void staticFunctionA(int i) {} + static void staticFunctionA(int i) { /*some code that doesn't use `i`*/ } // becomes static void staticFunctionA() - static void staticFunctionA() {} + static void staticFunctionA() { /*some code that doesn't use `i`*/ } + +Options +--- + +.. option:: StrictMode + + When zero (default value), the check will ignore trivially unused parameters, + i.e. when the corresponding function has an empty body (and in case of + constructors - no constructor initializers). When the function body is empty, + an unused parameter is unlikely to be unnoticed by a human reader, and + there's basically no place for a bug to hide. Index: clang-tidy/misc/UnusedParametersCheck.h === --- clang-tidy/misc/UnusedParametersCheck.h +++ clang-tidy/misc/UnusedParametersCheck.h @@ -24,8 +24,10 @@ ~UnusedParametersCheck(); void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; private: + const bool StrictMode; class IndexerVisitor; std::unique_ptr Indexer; Index: clang-tidy/misc/UnusedParamet
[PATCH] D46951: [clang-tidy] misc-unused-parameters - retain old behavior under StrictMode
alexfh added inline comments. Comment at: clang-tidy/misc/UnusedParametersCheck.cpp:180 +// (constructor initializer counts for non-empty body). +if (StrictMode || llvm::distance(Function->getBody()->children()) > 0 || +(isa(Function) && lebedev.ri wrote: > So this checks that there are children. Yes, and llvm::distance (recently renamed to llvm::size) doesn't work here any more, so the code now checks for iterator equality, which should be easier to read. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46951 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response
simark added a comment. Btw, this seems to happen only when the included header is in the preamble. If I put a variable declaration before `#include "first.h"`, things work as expected, we have not duplicates and the path is normalized. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r335863 - [clang-tidy] misc-unused-parameters - retain old behavior under StrictMode
Author: alexfh Date: Thu Jun 28 08:21:25 2018 New Revision: 335863 URL: http://llvm.org/viewvc/llvm-project?rev=335863&view=rev Log: [clang-tidy] misc-unused-parameters - retain old behavior under StrictMode Summary: This addresses https://bugs.llvm.org/show_bug.cgi?id=37467. Reviewers: klimek, ilya-biryukov, lebedev.ri, aaron.ballman Reviewed By: lebedev.ri, aaron.ballman Subscribers: aaron.ballman, lebedev.ri, xazax.hun, cfe-commits Differential Revision: https://reviews.llvm.org/D46951 Added: clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters-strict.cpp Modified: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp Modified: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp?rev=335863&r1=335862&r2=335863&view=diff == --- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp Thu Jun 28 08:21:25 2018 @@ -12,6 +12,7 @@ #include "clang/AST/RecursiveASTVisitor.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/Lexer.h" +#include "llvm/ADT/STLExtras.h" #include using namespace clang::ast_matchers; @@ -29,11 +30,10 @@ bool isOverrideMethod(const FunctionDecl } // namespace void UnusedParametersCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher(functionDecl(isDefinition(), - hasBody(stmt(hasDescendant(stmt(, - hasAnyParameter(decl())) - .bind("function"), - this); + Finder->addMatcher( + functionDecl(isDefinition(), hasBody(stmt()), hasAnyParameter(decl())) + .bind("function"), + this); } template @@ -122,7 +122,12 @@ UnusedParametersCheck::~UnusedParameters UnusedParametersCheck::UnusedParametersCheck(StringRef Name, ClangTidyContext *Context) -: ClangTidyCheck(Name, Context) {} +: ClangTidyCheck(Name, Context), + StrictMode(Options.getLocalOrGlobal("StrictMode", 0) != 0) {} + +void UnusedParametersCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "StrictMode", StrictMode); +} void UnusedParametersCheck::warnOnUnusedParameter( const MatchFinder::MatchResult &Result, const FunctionDecl *Function, @@ -170,7 +175,15 @@ void UnusedParametersCheck::check(const if (Param->isUsed() || Param->isReferenced() || !Param->getDeclName() || Param->hasAttr()) continue; -warnOnUnusedParameter(Result, Function, i); + +// In non-strict mode ignore function definitions with empty bodies +// (constructor initializer counts for non-empty body). +if (StrictMode || +(Function->getBody()->child_begin() != + Function->getBody()->child_end()) || +(isa(Function) && + cast(Function)->getNumCtorInitializers() > 0)) + warnOnUnusedParameter(Result, Function, i); } } Modified: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h?rev=335863&r1=335862&r2=335863&view=diff == --- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h (original) +++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h Thu Jun 28 08:21:25 2018 @@ -24,8 +24,10 @@ public: ~UnusedParametersCheck(); void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; private: + const bool StrictMode; class IndexerVisitor; std::unique_ptr Indexer; Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst?rev=335863&r1=335862&r2=335863&view=diff == --- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst (original) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst Thu Jun 28 08:21:25 2018 @@ -3,24 +3,40 @@ misc-unused-parameters == -Finds unused parameters and fixes them, so that `-Wunused-parameter` can be -turned on. +Finds unused function parameters. Unused parameters may signify a bug in the +code (e.g. when
[PATCH] D46951: [clang-tidy] misc-unused-parameters - retain old behavior under StrictMode
This revision was automatically updated to reflect the committed changes. Closed by commit rL335863: [clang-tidy] misc-unused-parameters - retain old behavior under StrictMode (authored by alexfh, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D46951 Files: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters-strict.cpp clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp Index: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst === --- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-parameters.rst @@ -3,24 +3,40 @@ misc-unused-parameters == -Finds unused parameters and fixes them, so that `-Wunused-parameter` can be -turned on. +Finds unused function parameters. Unused parameters may signify a bug in the +code (e.g. when a different parameter is used instead). The suggested fixes +either comment parameter name out or remove the parameter completely, if all +callers of the function are in the same translation unit and can be updated. + +The check is similar to the `-Wunused-parameter` compiler diagnostic and can be +used to prepare a codebase to enabling of that diagnostic. By default the check +is more permissive (see :option:`StrictMode`). .. code-block:: c++ - void a(int i) {} + void a(int i) { /*some code that doesn't use `i`*/ } // becomes - void a(int /*i*/) {} - + void a(int /*i*/) { /*some code that doesn't use `i`*/ } .. code-block:: c++ static void staticFunctionA(int i); - static void staticFunctionA(int i) {} + static void staticFunctionA(int i) { /*some code that doesn't use `i`*/ } // becomes static void staticFunctionA() - static void staticFunctionA() {} + static void staticFunctionA() { /*some code that doesn't use `i`*/ } + +Options +--- + +.. option:: StrictMode + + When zero (default value), the check will ignore trivially unused parameters, + i.e. when the corresponding function has an empty body (and in case of + constructors - no constructor initializers). When the function body is empty, + an unused parameter is unlikely to be unnoticed by a human reader, and + there's basically no place for a bug to hide. Index: clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp === --- clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp +++ clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp @@ -222,5 +222,41 @@ return Function(); } +namespace strict_mode_off { // Do not warn on empty function bodies. -void f(int foo) {} +void f1(int foo1) {} +void f2(int foo2) { + // "empty" in the AST sense, not in textual sense. +} +void f3(int foo3) {;} +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: parameter 'foo3' is unused +// CHECK-FIXES: {{^}}void f3(int /*foo3*/) {;}{{$}} + +class E { + int i; + +public: + E(int j) {} +}; +class F { + int i; + +public: + // Constructor initializer counts as a non-empty body. + F(int j) : i() {} +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'j' is unused +// CHECK-FIXES: {{^}} F(int /*j*/) : i() {}{{$}} +}; + +class A { +public: + A(); + A(int); +}; +class B : public A { +public: + B(int i) : A() {} +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is unused +// CHECK-FIXES: {{^}} B(int /*i*/) : A() {}{{$}} +}; +} // namespace strict_mode_off Index: clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters-strict.cpp === --- clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters-strict.cpp +++ clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters-strict.cpp @@ -0,0 +1,25 @@ +// RUN: %check_clang_tidy %s misc-unused-parameters %t -- \ +// RUN: -config="{CheckOptions: [{key: StrictMode, value: 1}]}" -- + +// Warn on empty function bodies in StrictMode. +namespace strict_mode { +void f(int foo) {} +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'foo' is unused [misc-unused-parameters] +// CHECK-FIXES: {{^}}void f(int /*foo*/) {}{{$}} +class E { + int i; + +public: + E(int j) {} +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'j' is unused +// CHECK-FIXES: {{^}} E(int /*j*/) {}{{$}} +}; +class F { + int i; + +public: + F(int j) : i() {} +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'j' is unused +// CHECK-FIXES: {{^}} F(int /*j*/) : i() {}{{$}} +}; +} Index: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h