Re: [clang-tools-extra] r347520 - A bit of AST matcher cleanup, NFC.

On Sat, Nov 24, 2018 at 9:43 PM Alexander Kornienko via cfe-commits wrote: > > Author: alexfh > Date: Sat Nov 24 18:41:01 2018 > New Revision: 347520 > > URL: http://llvm.org/viewvc/llvm-project?rev=347520&view=rev > Log: > A bit of AST matcher cleanup, NFC. > > Removed the uses of the allOf() mat

[PATCH] D54878: [clangd] NFC: Eliminate the unused variable warning.

MTC created this revision. MTC added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ioeric. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54878 Files: clangd/AST.cpp Index: clangd/AST.cpp =

Re: [clang-tools-extra] r347520 - A bit of AST matcher cleanup, NFC.

On Sun, Nov 25, 2018 at 1:52 PM Aaron Ballman wrote: > On Sat, Nov 24, 2018 at 9:43 PM Alexander Kornienko via cfe-commits > wrote: > > > > Author: alexfh > > Date: Sat Nov 24 18:41:01 2018 > > New Revision: 347520 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=347520&view=rev > > Log: > >

[PATCH] D53076: [analyzer] Enhance ConditionBRVisitor to write out more information

Charusso added a comment. Sorry for the huge noise! It took me a while to see what is the problem. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53076/new/ https://reviews.llvm.org/D53076 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D53708: [ASTImporter] Add importer specific lookup

a_sidorin added a subscriber: jingham. a_sidorin added a comment. Hi Gabor, I think it is a good patch with a nice test set. There are some mine comments inline. I'd also like to have LLDB guys opinion on it. Comment at: lib/AST/ASTImporter.cpp:7605 + +ASTImporter::ASTImporte

[PATCH] D53818: [ASTImporter] Changed use of Import to Import_New in ASTImporter.

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. LGTM. Thank you for addressing my questions! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53818/new/ https://reviews.llvm.org/D53818

[PATCH] D53818: [ASTImporter] Changed use of Import to Import_New in ASTImporter.

martong added a comment. In D53818#1307321 , @a_sidorin wrote: > LGTM. Thank you for addressing my questions! Hi Alexei, Could you please also take a look on that patch which this one depends on? https://reviews.llvm.org/D53751 I think the changes of t

[PATCH] D53076: [analyzer] Enhance ConditionBRVisitor to write out more information

Szelethus added a comment. I think the reason why the printed message was either along the lines of "this is 0" and "this is non-0", is that we don't necessarily know what constraint solver we're using, and adding more non-general code `BugReporter` is most probably a bad approach. How about we

r347527 - [CodeGen] translate MS rotate builtins to LLVM funnel-shift intrinsics

Author: spatel Date: Sun Nov 25 09:53:16 2018 New Revision: 347527 URL: http://llvm.org/viewvc/llvm-project?rev=347527&view=rev Log: [CodeGen] translate MS rotate builtins to LLVM funnel-shift intrinsics This was originally part of: D50924 and should resolve PR37387: https://bugs.llvm.org/show_b

[PATCH] D53655: [ASTImporter] Fix redecl chain of classes and class templates

a_sidorin added a comment. Hi Gabor, The change looks mostly Ok but there are some comments inline. Comment at: include/clang/AST/DeclContextInternals.h:128 +DeclsTy &Vec = *getAsVector(); +DeclsTy::iterator I = std::find(Vec.begin(), Vec.end(), D); +return I != Ve

[PATCH] D53751: [ASTImporter] Added Import functions for transition to new API.

a_sidorin added a subscriber: spyffe. a_sidorin added a comment. Hi Balazs, The changes look mostly fine to me. I think they can be accepted after some minor stylish fixes. Hi Shafik, > I think these changes make sense at a high level but I am not sure about the > refactoring strategy. I am e

[PATCH] D53655: [ASTImporter] Fix redecl chain of classes and class templates

a_sidorin added subscribers: aaron.ballman, rsmith. a_sidorin added a comment. Please note that changes in AST lib will require AST code owners' approval. @rsmith, @aaron.ballman could you please take a look? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53655/ne

[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull

a_sidorin added a comment. More constants are always welcome. Comment at: lib/AST/ExternalASTMerger.cpp:154 ToContainer->setHasExternalLexicalStorage(); - ToContainer->setMustBuildLookupTable(); + ToContainer->getPrimaryContext()->setMustBuildLookupTable();

[PATCH] D53693: [ASTImporter] Typedef import brings in the complete type

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53693/new/ https://reviews.llvm.org/D53693 ___ cfe-c

[PATCH] D53655: [ASTImporter] Fix redecl chain of classes and class templates

aaron.ballman added inline comments. Comment at: include/clang/AST/DeclContextInternals.h:128 +DeclsTy &Vec = *getAsVector(); +DeclsTy::iterator I = std::find(Vec.begin(), Vec.end(), D); +return I != Vec.end(); a_sidorin wrote: > `auto I = llvm::find(

r347529 - [CodeComplete] Simplify CodeCompleteConsumer.cpp, NFC

Author: maskray Date: Sun Nov 25 12:57:05 2018 New Revision: 347529 URL: http://llvm.org/viewvc/llvm-project?rev=347529&view=rev Log: [CodeComplete] Simplify CodeCompleteConsumer.cpp, NFC Use range-based for loops Use XStr.compare(YStr) < 0 Format misaligned code Modified: cfe/trunk/lib/Sema

[PATCH] D54880: Ignore gcc's stack-clash-protection flag

v.g.vassilev created this revision. v.g.vassilev added reviewers: rsmith, bruno. Patch by Mattias Ellert! Repository: rC Clang https://reviews.llvm.org/D54880 Files: include/clang/Driver/Options.td Index: include/clang/Driver/Options.td ===

[PATCH] D53076: [analyzer] Enhance ConditionBRVisitor to write out more information

Charusso added a comment. @Szelethus thanks you for the comments! In D53076#1307336 , @Szelethus wrote: > I think the reason why the printed message was either along the lines of > "this is 0" and "this is non-0", is that we don't necessarily know what

[PATCH] D54881: Prevent Clang-Format from editing leading whitespace on lines outside of the format range

russellmcc created this revision. russellmcc added a reviewer: djasper. Herald added a subscriber: cfe-commits. When clang-format is set to always use tabs (with tab width 4), when asked to format line 3 (using the `-lines=3:3` flag): int f() { int a; foobar(); int b; } We

[PATCH] D54881: Prevent Clang-Format from editing leading whitespace on lines outside of the format range

russellmcc updated this revision to Diff 175185. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54881/new/ https://reviews.llvm.org/D54881 Files: lib/Format/UnwrappedLineFormatter.cpp lib/Format/UnwrappedLineFormatter.h lib/Format/WhitespaceManager.cpp lib/Format/WhitespaceManager

[PATCH] D54379: Add Hurd toolchain support to Clang

sthibaul marked 13 inline comments as done. sthibaul added a comment. I'll update the diff according to the comments. Comment at: lib/Driver/ToolChains/Hurd.cpp:74 + + // Similar to the logic for GCC above, if we currently running Clang inside + // of the requested system roo

[PATCH] D54379: Add Hurd toolchain support to Clang

sthibaul updated this revision to Diff 175186. sthibaul marked 4 inline comments as done. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54379/new/ https://reviews.llvm.org/D54379 Files: lib/Basic/Targets.cpp lib/Basic/Targets/OSTargets.h lib/Driver/CMakeList

[PATCH] D54379: Add Hurd toolchain support to Clang

sthibaul added a comment. (there still needs to be work on the test part) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54379/new/ https://reviews.llvm.org/D54379 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

MaskRay added a comment. https://reviews.llvm.org/rC347417 makes `constexpr string_view service = "HELLO WORD SERVICE"` (P0426) broken with libstdc++ % cat a.cc constexpr bool __constant_string_p(const char *__s) { while (__builtin_constant_p(*__s) && *__s) __s++; return __buil

[PATCH] D54379: Add Hurd toolchain support to Clang

sthibaul updated this revision to Diff 175188. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54379/new/ https://reviews.llvm.org/D54379 Files: lib/Basic/Targets.cpp lib/Basic/Targets/OSTargets.h lib/Driver/CMakeLists.txt lib/Driver/Driver.cpp lib/Driver/

[PATCH] D54379: Add Hurd toolchain support to Clang

sthibaul added a comment. I have added static and shared library tests > a short unit test with regards to creating the actual target instance) I'm not sure what you mean? The tests create an actual binary for the target. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D53696: [Haiku] Support __float128 for Haiku x86 and x86_64

kallisti5 added a comment. sorry, busy weekend. If you find the time feel free :-) - _GLIBCXX_USE_FLOAT128 can (and should) go - everything else is valid per the discussions here. Otherwise i'll pick up in the next few days and add the requested context as well. Repository: rC Clang CHANGE

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

void added a comment. Doh! I'll take a look at it. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54355/new/ https://reviews.llvm.org/D54355 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.or

[PATCH] D54878: [clangd] NFC: Eliminate the unused variable warning.

MaskRay accepted this revision. MaskRay added inline comments. This revision is now accepted and ready to land. Comment at: clangd/AST.cpp:98 // The name was empty, so present an anonymous entity. - if (auto *NS = llvm::dyn_cast(&ND)) + if (isa(&ND)) return "(anonymous

r347531 - A "constexpr" is evaluated in a constant context. Make sure this is reflected

Author: void Date: Sun Nov 25 18:10:53 2018 New Revision: 347531 URL: http://llvm.org/viewvc/llvm-project?rev=347531&view=rev Log: A "constexpr" is evaluated in a constant context. Make sure this is reflected if a __builtin_constant_p() is a part of a constexpr. Modified: cfe/trunk/lib/AST/Ex

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

void added a comment. Fixed in r347531. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54355/new/ https://reviews.llvm.org/D54355 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[PATCH] D54878: [clangd] NFC: Eliminate the unused variable warning.

MTC marked an inline comment as done. MTC added inline comments. Comment at: clangd/AST.cpp:98 // The name was empty, so present an anonymous entity. - if (auto *NS = llvm::dyn_cast(&ND)) + if (isa(&ND)) return "(anonymous namespace)"; MaskRay wrote: >

[PATCH] D54792: add -march=cascadelake support in clang

craig.topper accepted this revision. craig.topper added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54792/new/ https://reviews.llvm.org/D54792 ___ cf

Re: [clang-tools-extra] r347498 - [clangd] Add support for hierarchical documentSymbol

Hi Ilya, This patch doesn't compile for me with clang 3.6.0. I get: ../tools/clang/tools/extra/clangd/Protocol.cpp:474:10: error: no viable conversion from 'json::Object' to 'llvm::json::Value' return Result; ^~ ../include/llvm/Support/JSON.h:291:3: note: candidate constructor n

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

rjmccall added inline comments. Comment at: lib/CodeGen/CGCall.cpp:80 +// used with the same version of generated operators. +RecTy = Context.getAddrSpaceQualType(RecTy, LangAS::opencl_generic); + I would suggest taking this opportunity to set up the AST