Re: r339428 - Add Windows support for the GNUstep Objective-C ABI V2.

2018-08-11 Thread David Chisnall via cfe-commits
Thanks, That fix looks exactly what is needed - and is something I didn’t realise FileCheck could do! The order of those matters in the linked binary, but not in the IR - the linker will arrange them sensibly based on the subsection name. Thanks again, David > On 11 Aug 2018, at 04:12, >

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2018-08-11 Thread Shuai Wang via Phabricator via cfe-commits
shuaiwang added a comment. In https://reviews.llvm.org/D45444#1196271, @JonasToth wrote: > Always the same with the templates ;) So uninstantiated templates should > just be ignored. > > I think it would be better to have it in the ExprMutAnalyzer, because > that part can not decide on

r339505 - [ASTImporter] Added test case for CXXConversionDecl importing

2018-08-11 Thread Raphael Isemann via cfe-commits
Author: teemperor Date: Sat Aug 11 16:43:02 2018 New Revision: 339505 URL: http://llvm.org/viewvc/llvm-project?rev=339505=rev Log: [ASTImporter] Added test case for CXXConversionDecl importing Reviewers: a.sidorin, a_sidorin Reviewed By: a_sidorin Subscribers: a_sidorin, martong, cfe-commits

[PATCH] D50550: [ASTImporter] Added test case for opaque enums

2018-08-11 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL339506: [ASTImporter] Added test case for opaque enums (authored by teemperor, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

r339506 - [ASTImporter] Added test case for opaque enums

2018-08-11 Thread Raphael Isemann via cfe-commits
Author: teemperor Date: Sat Aug 11 16:43:46 2018 New Revision: 339506 URL: http://llvm.org/viewvc/llvm-project?rev=339506=rev Log: [ASTImporter] Added test case for opaque enums Reviewers: a.sidorin, a_sidorin Reviewed By: a_sidorin Subscribers: a_sidorin, martong, cfe-commits Differential

[PATCH] D50552: [ASTImporter] Added test case for CXXConversionDecl importing

2018-08-11 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL339505: [ASTImporter] Added test case for CXXConversionDecl importing (authored by teemperor, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

2018-08-11 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. No concerns except the small nits from my side. Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:24 + + // TODO(hugoeg): refactor matcher to be configurable or just match on any internal access from outside the enclosing namespace. +

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2018-08-11 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Always the same with the templates ;) So uninstantiated templates should just be ignored. I think it would be better to have it in the ExprMutAnalyzer, because that part can not decide on const-ness. Fixing it here would just circumvent the problem but not fix it,

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2018-08-11 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 160239. JonasToth added a comment. - explicitly ignore lambdas in VarDecls Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp

[PATCH] D41217: [Concepts] Concept Specialization Expressions

2018-08-11 Thread Saar Raz via Phabricator via cfe-commits
saar.raz updated this revision to Diff 160237. saar.raz added a comment. Removed unused "note_in_concept_specialization" diagnostic ID. Repository: rC Clang https://reviews.llvm.org/D41217 Files: include/clang/AST/DeclTemplate.h include/clang/AST/ExprCXX.h

[PATCH] D50447: [clang-tidy] Omit cases where loop variable is not used in loop body in performance-for-range-copy.

2018-08-11 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Could there even be a false positive? In a sense, the variable is never used, so it does not matter, not? Am 10.08.2018 um 22:17 schrieb Shuai Wang via Phabricator: > shuaiwang added a comment. > > In https://reviews.llvm.org/D50447#1194967, @JonasToth wrote: > >>

[PATCH] D50580: [clang-tidy] Abseil: no namespace check

2018-08-11 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. Could it happen that some template specializations or so need to land in `absl`? Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:28 +void NoNamespaceCheck::check(const MatchFinder::MatchResult ) { + const auto *decl =

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

2018-08-11 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 160242. JonasToth added a comment. - update to current master of clang introduce CHECK-NOTES - use new BeginLoc api Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48714 Files: clang-tidy/hicpp/ExceptionBaseclassCheck.cpp

[PATCH] D50559: [gnu-objc] Make selector order deterministic.

2018-08-11 Thread David Chisnall via Phabricator via cfe-commits
theraven added inline comments. Comment at: lib/CodeGen/CGObjCGNU.cpp:3547 + allSelectors.push_back(entry.first); +llvm::sort(allSelectors.begin(), allSelectors.end()); bmwiedemann wrote: > compilation failed here: >

[PATCH] D41217: [Concepts] Concept Specialization Expressions

2018-08-11 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments. Comment at: include/clang/Sema/Sema.h:7134 + // We are substituting template arguments into a constraint expression. + ConstraintSubstitution } Kind; Missing a trailing comma here. Comment at:

[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

2018-08-11 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin added a comment. Hi Gabor, While importing methods looks harmless, importing fields can be a breaking change. Do you have any test results on this? Comment at: lib/AST/ASTImporter.cpp:2872 Importer.MapImported(D, FoundField); +// In case of a

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

2018-08-11 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin added a comment. Hello Richard, Thank you for clarification. However, I think that moving ParentMap into libTooling is out of scope for this patch. Are you OK if this change will be committed with adding a TODO or FIXME for this move? https://reviews.llvm.org/D46940

[PATCH] D49798: [ASTImporter] Adding some friend function related unittests.

2018-08-11 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin added a comment. Hi Balazs, The patch looks mostly fine. Comment at: unittests/AST/ASTImporterTest.cpp:2256 +FirstDeclMatcher().match(FromTU, cxxRecordDecl()); +auto lookup_res = Class->noload_lookup(FromName); +ASSERT_EQ(lookup_res.size(), 0u);

[libcxxabi] r339503 - Add missing _LIBCXXABI_FUNC_VIS to __gxx_personality_seh0

2018-08-11 Thread Martin Storsjo via cfe-commits
Author: mstorsjo Date: Sat Aug 11 12:36:06 2018 New Revision: 339503 URL: http://llvm.org/viewvc/llvm-project?rev=339503=rev Log: Add missing _LIBCXXABI_FUNC_VIS to __gxx_personality_seh0 This was missed in SVN r337754. Modified: libcxxabi/trunk/src/cxa_personality.cpp Modified:

[PATCH] D50516: [ASTImporter] Improved import of friend templates.

2018-08-11 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin accepted this revision. a_sidorin added inline comments. This revision is now accepted and ready to land. Comment at: unittests/AST/ASTImporterTest.cpp:2721 + EXPECT_EQ(ToFriendClass->getDefinition(), ToClass); + ASSERT_EQ(ToFriendClass->getPreviousDecl(), ToClass);

[PATCH] D50550: [ASTImporter] Added test case for opaque enums

2018-08-11 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D50550 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

2018-08-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- updated this revision to Diff 160243. 0x8000- added a comment. Rebased on curent master. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/readability/CMakeLists.txt

[PATCH] D41217: [Concepts] Concept Specialization Expressions

2018-08-11 Thread Saar Raz via Phabricator via cfe-commits
saar.raz added inline comments. Comment at: test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp:121 +template +concept C7 = sizeof(T) == 1 || sizeof(typename T3::type) == 1; // expected-note{{while substituting template arguments into constraint expression here}}

[PATCH] D50559: [gnu-objc] Make selector order deterministic.

2018-08-11 Thread Bernhard M. Wiedemann via Phabricator via cfe-commits
bmwiedemann added a comment. I got it compiled with std::sort on top of 6.0.1 and found that it indeed removes the indeterminism: When I compiled libobjc2-1.8.1/arc.m 25 times, I got the same md5sum every time - and the same result with and without ASLR. Repository: rC Clang

[PATCH] D50552: [ASTImporter] Added test case for CXXConversionDecl importing

2018-08-11 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. Thanks! Repository: rC Clang https://reviews.llvm.org/D50552 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50605: [ASTMatchers] Let hasAnyArgument also support CXXUnresolvedConstructExpr

2018-08-11 Thread Shuai Wang via Phabricator via cfe-commits
shuaiwang created this revision. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D50605 Files: include/clang/AST/ExprCXX.h include/clang/ASTMatchers/ASTMatchers.h unittests/ASTMatchers/ASTMatchersTraversalTest.cpp Index: