[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

rjmccall added inline comments. Comment at: lib/CodeGen/CGObjCGNU.cpp:3542 + allSelectors.push_back(entry.first); +std::sort(allSelectors.begin(), allSelectors.end()); mgrang wrote: > Please use llvm::sort instead of std::sort. See >

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

beanz added a subscriber: jfb. beanz added a comment. Adding @jfb since this is his domain now too. Repository: rC Clang https://reviews.llvm.org/D45639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D44671: [libcxx] Enable static libcxxabi linking on Darwin

jfb added a comment. Were the concerns expressed in https://reviews.llvm.org/D8017 addressed? Repository: rCXX libc++ https://reviews.llvm.org/D44671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D36357: Added a better diagnostic when using the delete operator with lambdas

Rakete updated this revision to Diff 159832. Rakete added a comment. Rebase + friendly ping :) Repository: rC Clang https://reviews.llvm.org/D36357 Files: include/clang/Basic/DiagnosticParseKinds.td lib/Parse/ParseExprCXX.cpp test/FixIt/fixit-cxx0x.cpp

r339305 - Added another optimization pass to make vectorizing possible

Author: emmettneyman Date: Wed Aug 8 17:58:23 2018 New Revision: 339305 URL: http://llvm.org/viewvc/llvm-project?rev=339305=rev Log: Added another optimization pass to make vectorizing possible Summary: I noticed that my code wasn't going deep into the loop vectorizer code so added another

[PATCH] D50482: Added another optimization pass to make vectorizing possible

This revision was automatically updated to reflect the committed changes. Closed by commit rL339305: Added another optimization pass to make vectorizing possible (authored by emmettneyman, committed by ). Changed prior to commit: https://reviews.llvm.org/D50482?vs=159838=159839#toc

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

JonasToth updated this revision to Diff 159775. JonasToth added a comment. - fix bug with AnalyzeValues==false skipping whole check, adjust test code to 'const' instead of const Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files:

[PATCH] D50471: [Builtins] Add __bulitin_clrsb support to IntExprEvaluator::VisitBuiltinCallExpr

craig.topper created this revision. craig.topper added reviewers: bkramer, erichkeane. Now that __builtin_clrsb is supported by clang, this patch adds constant evaluation of it to address the FIXME. https://reviews.llvm.org/D50471 Files: lib/AST/ExprConstant.cpp

[PATCH] D50471: [Builtins] Add __bulitin_clrsb support to IntExprEvaluator::VisitBuiltinCallExpr

This revision was automatically updated to reflect the committed changes. Closed by commit rC339287: [Builtins] Add __builtin_clrsb support to IntExprEvaluator::VisitBuiltinCallExpr (authored by ctopper, committed by ). Herald added a subscriber: kristina. Changed prior to commit:

r339287 - [Builtins] Add __builtin_clrsb support to IntExprEvaluator::VisitBuiltinCallExpr

Author: ctopper Date: Wed Aug 8 13:59:40 2018 New Revision: 339287 URL: http://llvm.org/viewvc/llvm-project?rev=339287=rev Log: [Builtins] Add __builtin_clrsb support to IntExprEvaluator::VisitBuiltinCallExpr This addresses a FIXME that has existed since before clang supported the builtin.

r339289 - Revert r339287 "[Builtins] Add __builtin_clrsb support to IntExprEvaluator::VisitBuiltinCallExpr"

Author: ctopper Date: Wed Aug 8 14:21:21 2018 New Revision: 339289 URL: http://llvm.org/viewvc/llvm-project?rev=339289=rev Log: Revert r339287 "[Builtins] Add __builtin_clrsb support to IntExprEvaluator::VisitBuiltinCallExpr" This add an additional unintended change in it. Modified:

[PATCH] D50444: [ASTImporter] Fix structural inequivalency of forward EnumDecl

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. Yes, this seems to be correct. Thanks! Comment at: lib/AST/ASTStructuralEquivalence.cpp:1182 + + // Compare the definitions of these two eunums. If either or both are

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

nickdesaulniers updated this revision to Diff 159820. nickdesaulniers added a comment. - clean up conditional and add comment Repository: rC Clang https://reviews.llvm.org/D50467 Files: lib/Sema/SemaChecking.cpp test/SemaCXX/warn-float-conversion.cpp Index:

[PATCH] D50462: Try building complete AST after a fatal error was emitted if further diagnostics are expected

arphaman added a comment. This seems sensible to me. Comment at: include/clang/Basic/Diagnostic.h:764 + /// off extra processing that might be wasteful in this case. +bool areDiagnosticsSuppressedAfterFatalError() const { +return FatalErrorOccurred &&

[PATCH] D50482: Added another optimization pass to make vectorizing possible

morehouse added inline comments. Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:90 +getCodeModel(), OLvl); +} + If you have to pass that many parameters to a 3 line function, just inline instead.

[PATCH] D50482: Added another optimization pass to make vectorizing possible

emmettneyman added inline comments. Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:126 + + auto = static_cast(*TM); + Passes.add(LTM.createPassConfig(Passes)); morehouse wrote: > Is TM guaranteed to be an LLVMTargetMachine? Yes, since the

[PATCH] D50482: Added another optimization pass to make vectorizing possible

emmettneyman added inline comments. Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:103 + Triple ModuleTriple(M->getTargetTriple()); + const TargetOptions Options = InitTargetOptionsFromCodeGenFlags(); morehouse wrote: > I think you can

[PATCH] D50436: Correctly initialise global blocks on Windows.

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Alright, LGTM, at least until we have that backend support you mentioned. Repository: rC Clang https://reviews.llvm.org/D50436 ___

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

jfb added a comment. In https://reviews.llvm.org/D45639#1192849, @beanz wrote: > Adding @jfb since this is his domain now too. @ldionne is the libc++ expert :-) Repository: rC Clang https://reviews.llvm.org/D45639 ___ cfe-commits mailing list

[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions

rjmccall added inline comments. Comment at: include/clang/AST/ASTContext.h:248 + /// Mapping from __block VarDecls to their copy initialization expr. The + /// boolean flag indicates whether the expression can throw. + typedef llvm::DenseMap Maybe you should just make a type

[PATCH] D50482: Added another optimization pass to make vectorizing possible

morehouse accepted this revision. morehouse added inline comments. This revision is now accepted and ready to land. Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:103 + Triple ModuleTriple(M->getTargetTriple()); + const TargetOptions Options =

r339282 - [Builtins] Implement __builtin_clrsb to be compatible with gcc

Author: ctopper Date: Wed Aug 8 12:55:52 2018 New Revision: 339282 URL: http://llvm.org/viewvc/llvm-project?rev=339282=rev Log: [Builtins] Implement __builtin_clrsb to be compatible with gcc gcc defines an intrinsic called __builtin_clrsb which counts the number of extra sign bits on a number.

[PATCH] D50477: WIP: Ensure that the type of size_t is represended as one of the fixed width types

sbc100 updated this revision to Diff 159794. sbc100 added a comment. remove debugging Repository: rC Clang https://reviews.llvm.org/D50477 Files: lib/Frontend/InitPreprocessor.cpp Index: lib/Frontend/InitPreprocessor.cpp

[PATCH] D50477: WIP: Ensure that the type of size_t is represended as one of the fixed width types

sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin. We recently changes `size_t` on wasm from `int` to `long` which had the effect of making the type of size_t (long) not match either int32_t or int64_t. This solution is not very elegant but fixes the build

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

nickdesaulniers added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10411 +->getAs(); + if (!ResultBT || !(RBT && RBT->isFloatingPoint())) return; + nickdesaulniers wrote: > pirama wrote: > > Add a comment explaining this

[PATCH] D50482: Added another optimization pass to make vectorizing possible

emmettneyman updated this revision to Diff 159838. emmettneyman added a comment. Rebase Repository: rC Clang https://reviews.llvm.org/D50482 Files: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp Index: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp

[PATCH] D50471: [Builtins] Add __bulitin_clrsb support to IntExprEvaluator::VisitBuiltinCallExpr

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. Looks easy enough, just note the test issue. Comment at: test/Sema/constant-builtins-2.c:191 +char clrsb11[__builtin_clrsb(0xf) == BITSIZE(int) - 5 ? 1 : -1]; +char

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

pirama added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10411 +->getAs(); + if (!ResultBT || !(RBT && RBT->isFloatingPoint())) return; + Add a comment explaining this conditional as well? > Return if source and target

[PATCH] D50168: [Builtins] Implement __builtin_clrsb to be compatible with gcc

This revision was automatically updated to reflect the committed changes. Closed by commit rC339282: [Builtins] Implement __builtin_clrsb to be compatible with gcc (authored by ctopper, committed by ). Herald added a subscriber: kristina. Repository: rC Clang https://reviews.llvm.org/D50168

[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions

rjmccall added inline comments. Comment at: include/clang/AST/ASTContext.h:248 + /// Mapping from __block VarDecls to their copy initialization expr. The + /// boolean flag indicates whether the expression can throw. + typedef llvm::DenseMaphttps://reviews.llvm.org/D50152

[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions

ahatanak updated this revision to Diff 159800. ahatanak marked 2 inline comments as done. ahatanak added a comment. Modify getBlockVarCopyInits and setBlockVarCopyInits to get and set both the copy expression and the boolean flag that indicates whether the expression can throw or not.

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

ldionne added a comment. @phosek I don't understand how you can expect code compiled with new headers to link against an old dylib, unless you're setting the target platform, in which case anything that would fail to link will instead be a compiler error. I mean, we're adding stuff to the

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

ldionne added a comment. I must say I therefore don't understand the purpose of this patch. Repository: rC Clang https://reviews.llvm.org/D45639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50462: Try building complete AST after a fatal error was emitted if further diagnostics are expected

arphaman added a comment. On a second look I think that there is value separating the concepts of 'producing diagnostics' after hitting the fatal error (which is SuppressAfterFatalError currently does), and trying to build a more complete AST. For example, - An editor client might not want to

[PATCH] D50482: Added another optimization pass to make vectorizing possible

emmettneyman updated this revision to Diff 159831. emmettneyman added a comment. - minor style fix Repository: rC Clang https://reviews.llvm.org/D50482 Files: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp Index: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp

[PATCH] D37302: [Headers] Define *_HAS_SUBNORM for FLT, DBL, LDBL

This revision was automatically updated to reflect the committed changes. Closed by commit rC339284: [Headers] Define *_HAS_SUBNORM for FLT, DBL, LDBL (authored by pirama, committed by ). Changed prior to commit: https://reviews.llvm.org/D37302?vs=159330=159778#toc Repository: rC Clang

[PATCH] D50428: [ASTImporter] Add support for importing imaginary literals

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. LGTM! Just a stylish nit. Comment at: lib/AST/ASTImporter.cpp:5617 + + return new (Importer.getToContext()) + ImaginaryLiteral(SubE, T); The

r339296 - [VFS] Remove superfluous semicolon from unittest.

Author: ctopper Date: Wed Aug 8 15:31:14 2018 New Revision: 339296 URL: http://llvm.org/viewvc/llvm-project?rev=339296=rev Log: [VFS] Remove superfluous semicolon from unittest. Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Modified:

r339295 - [Builtins] Add __builtin_clrsb support to IntExprEvaluator::VisitBuiltinCallExpr

Author: ctopper Date: Wed Aug 8 15:31:12 2018 New Revision: 339295 URL: http://llvm.org/viewvc/llvm-project?rev=339295=rev Log: [Builtins] Add __builtin_clrsb support to IntExprEvaluator::VisitBuiltinCallExpr This addresses a FIXME that has existed since before clang supported the builtin.

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

nickdesaulniers added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10411 +->getAs(); + if (!ResultBT || !(RBT && RBT->isFloatingPoint())) return; + pirama wrote: > Add a comment explaining this conditional as well? > > >

[PATCH] D50122: Complex Variable defined in InitCapture Crash fix

erik.pilkington added inline comments. Comment at: lib/Sema/SemaTemplateInstantiate.cpp:2916-2918 + if (const VarDecl *VD = dyn_cast(D)) +if (VD->isInitCapture()) + return nullptr; Why are we failing to find the instantiation of the VarDecl in this

r339284 - [Headers] Define *_HAS_SUBNORM for FLT, DBL, LDBL

Author: pirama Date: Wed Aug 8 13:38:38 2018 New Revision: 339284 URL: http://llvm.org/viewvc/llvm-project?rev=339284=rev Log: [Headers] Define *_HAS_SUBNORM for FLT, DBL, LDBL Summary: These macros are defined in the C11 standard and can be defined based on the __*_HAS_DENORM__ default macros.

[PATCH] D50471: [Builtins] Add __bulitin_clrsb support to IntExprEvaluator::VisitBuiltinCallExpr

craig.topper updated this revision to Diff 159777. craig.topper added a comment. Fix duplicate variable name in the test. Not sure why that didn't complain https://reviews.llvm.org/D50471 Files: lib/AST/ExprConstant.cpp test/Sema/constant-builtins-2.c Index:

[PATCH] D50482: Added another optimization pass to make vectorizing possible

emmettneyman created this revision. emmettneyman added reviewers: morehouse, kcc. Herald added a subscriber: cfe-commits. I noticed that my code wasn't going deep into the loop vectorizer code so added another pass that makes it go further. Repository: rC Clang

[PATCH] D50482: Added another optimization pass to make vectorizing possible

emmettneyman updated this revision to Diff 159834. emmettneyman added a comment. - Inlined function Repository: rC Clang https://reviews.llvm.org/D50482 Files: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp Index: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp

r339304 - Delete some unreachable AST printing code.

Author: rsmith Date: Wed Aug 8 17:44:49 2018 New Revision: 339304 URL: http://llvm.org/viewvc/llvm-project?rev=339304=rev Log: Delete some unreachable AST printing code. Modified: cfe/trunk/lib/AST/TypePrinter.cpp Modified: cfe/trunk/lib/AST/TypePrinter.cpp URL:

[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions

ahatanak added inline comments. Comment at: include/clang/AST/ASTContext.h:158 +Expr *CopyExpr; +bool CanThrow; + }; rjmccall wrote: > Using a PointerIntPair in the implementation of this is still a reasonable > choice. Just make it a proper

[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions

ahatanak updated this revision to Diff 159850. ahatanak marked 5 inline comments as done. ahatanak added a comment. Address review comments. Repository: rC Clang https://reviews.llvm.org/D50152 Files: include/clang/AST/ASTContext.h lib/AST/ASTContext.cpp lib/CodeGen/CGBlocks.cpp

r339306 - Refactor attribute printing to be a bit more obviously-correct.

Author: rsmith Date: Wed Aug 8 18:21:06 2018 New Revision: 339306 URL: http://llvm.org/viewvc/llvm-project?rev=339306=rev Log: Refactor attribute printing to be a bit more obviously-correct. No functionality change intended. Added: cfe/trunk/test/Misc/ast-print-attr.c Modified:

r339307 - [CMake] Use normalized Windows target triples

Author: phosek Date: Wed Aug 8 19:16:18 2018 New Revision: 339307 URL: http://llvm.org/viewvc/llvm-project?rev=339307=rev Log: [CMake] Use normalized Windows target triples Changes the default Windows target triple returned by GetHostTriple.cmake from the old environment names (which we wanted

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like keys

mgrang created this revision. mgrang added reviewers: NoQ, george.karpenkov, whisperity. Herald added subscribers: mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, mgorny. Repository: rC Clang https://reviews.llvm.org/D50488 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td

[PATCH] D50447: [clang-tidy] Add a whitelistClasses option in performance-for-range-copy check.

shuaiwang added a comment. In https://reviews.llvm.org/D50447#1192393, @JonasToth wrote: > ... just check if the variable is dereferenced in the scope of the loop (any > declRefExpr exists). +1 And I would imagine it's very rare (as in categories not raw number of occurrences) for a loop

[PATCH] D50487: [CFG] [analyzer] Find argument constructors in CXXTemporaryObjectExprs.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, rnkovacs. Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware. `CXXTemporaryObjectExpr` is a sub-class of `CXXConstructExpr` that represents a construct-expression

[clang-tools-extra] r339307 - [CMake] Use normalized Windows target triples

Author: phosek Date: Wed Aug 8 19:16:18 2018 New Revision: 339307 URL: http://llvm.org/viewvc/llvm-project?rev=339307=rev Log: [CMake] Use normalized Windows target triples Changes the default Windows target triple returned by GetHostTriple.cmake from the old environment names (which we wanted

[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions

rjmccall added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:1682 + if (IsCopyHelper && Ctx.getBlockVarCopyInits(Var).CanThrow) +Name += "c"; +} ahatanak wrote: > rjmccall wrote: > > I don't think you need to add `d` to the

[PATCH] D50447: [clang-tidy] Add a whitelistClasses option in performance-for-range-copy check.

hokein created this revision. hokein added reviewers: ilya-biryukov, alexfh. Herald added a subscriber: xazax.hun. The upstream change r336737 causes a regression issue in our internal base codebase. Adding an option to the check allowing us whitelist these classes. Repository: rCTE Clang

[PATCH] D50175: [AArch64][NFC] better matching of AArch64 target in aarch64-cpus.c tests

olista01 added inline comments. Comment at: test/Driver/aarch64-cpus.c:10 +// GENERIC: "-cc1"{{.*}} "-triple" "aarch64"{{.*}} "-target-cpu" "generic" +// GENERIC-LE: "-cc1"{{.*}} "-triple" "aarch64--"{{.*}} "-target-cpu" "generic" Why do these need new check

[PATCH] D50447: [clang-tidy] Add a whitelistClasses option in performance-for-range-copy check.

lebedev.ri added subscribers: shuaiwang, JonasToth, lebedev.ri. lebedev.ri added a comment. Regression is very broad term. Is it a false-positive? Perhaps it is better to address the issue in the ExprMutationAnalyzer itself? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50447

[PATCH] D50414: [libunwind][include] Add SEH declarations to .

cdavis5x added a comment. In https://reviews.llvm.org/D50414#1191834, @mstorsjo wrote: > Should we maybe add the same declaration of `_GCC_specific_handler` to > clang's unwind.h? That would allow removing the forward declaration in > libcxxabi from https://reviews.llvm.org/D49638. Probably.

[PATCH] D50448: [CGObjCGNU] Rename GetSelector helper method to fix -Woverloaded-virtual warning (PR38210)

RKSimon created this revision. RKSimon added a reviewer: theraven. As suggested by @theraven on PR38210, this patch fixes the gcc -Woverloaded-virtual warnings by renaming the extra CGObjCGNU::GetSelector method to CGObjCGNU::GetTypedSelector Repository: rC Clang

[PATCH] D50447: [clang-tidy] Add a whitelistClasses option in performance-for-range-copy check.

JonasToth added a comment. If whitelisting works, thats fine. But I agree with @lebedev.ri that a contribution/improvement to the ExprMutAnalyzer is the better option. This is especially the case, because multiple checks (and maybe even some other parts then clang-tidy) will utilize this

[PATCH] D50446: [clangd] Record the currently active file in context for codeCompletion and findDefinitions.

ilya-biryukov added a comment. Short summary of the offline discussion: I suggest adding this parameter into the corresponding requests of the index (FuzzyFindRequest, FindDefinitionsRequest) instead of stashing it in the context. Context has all the same problems as the global variables, so

[PATCH] D50447: [clang-tidy] Add a whitelistClasses option in performance-for-range-copy check.

JonasToth added inline comments. Comment at: test/clang-tidy/performance-for-range-copy.cpp:1 -// RUN: %check_clang_tidy %s performance-for-range-copy %t -- -- -std=c++11 -fno-delayed-template-parsing +// RUN: %check_clang_tidy %s performance-for-range-copy %t

[PATCH] D50448: [CGObjCGNU] Rename GetSelector helper method to fix -Woverloaded-virtual warning (PR38210)

xbolva00 added a comment. Thanks, this warning is really annoying. Repository: rC Clang https://reviews.llvm.org/D50448 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50449: [clangd] Support textEdit in addition to insertText.

kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ioeric. Completion replies contains textEdits as well. Note that this change relies on https://reviews.llvm.org/D50443. Repository: rCTE Clang Tools

[PATCH] D50337: [clangd] DexIndex implementation prototype

ioeric added inline comments. Comment at: clang-tools-extra/clangd/index/MemIndex.h:45 // Index is a set of symbols that are deduplicated by symbol IDs. - // FIXME: build smarter index structure. llvm::DenseMap Index; I think this FIXME still applies

[PATCH] D50447: [clang-tidy] Add a whitelistClasses option in performance-for-range-copy check.

hokein added a comment. In https://reviews.llvm.org/D50447#1192280, @JonasToth wrote: > If whitelisting works, thats fine. But I agree with @lebedev.ri that a > contribution/improvement to the ExprMutAnalyzer is the better option. This is > especially the case, because multiple checks (and

[PATCH] D50447: [clang-tidy] Add a whitelistClasses option in performance-for-range-copy check.

lebedev.ri added a subscriber: EricWF. lebedev.ri added a comment. In https://reviews.llvm.org/D50447#1192316, @hokein wrote: > In https://reviews.llvm.org/D50447#1192280, @JonasToth wrote: > > > If whitelisting works, thats fine. But I agree with @lebedev.ri that a > > contribution/improvement

[PATCH] D50175: [AArch64][NFC] better matching of AArch64 target in aarch64-cpus.c tests

SjoerdMeijer added inline comments. Comment at: test/Driver/aarch64-cpus.c:10 +// GENERIC: "-cc1"{{.*}} "-triple" "aarch64"{{.*}} "-target-cpu" "generic" +// GENERIC-LE: "-cc1"{{.*}} "-triple" "aarch64--"{{.*}} "-target-cpu" "generic" olista01 wrote: > Why do

[PATCH] D47849: [OpenMP][Clang][NVPTX] Enable math functions called in an OpenMP NVPTX target device region to be resolved as device-native function calls

gtbercea added a comment. In https://reviews.llvm.org/D47849#1192245, @Hahnfeld wrote: > In https://reviews.llvm.org/D47849#1192134, @gtbercea wrote: > > > This patch is concerned with calling device functions when you're on the > > device. The correctness issues you mention are orthogonal to

[PATCH] D47849: [OpenMP][Clang][NVPTX] Enable math functions called in an OpenMP NVPTX target device region to be resolved as device-native function calls

gtbercea added a comment. > IIRC you started to work on this to fix the problem with inline assembly (see > https://reviews.llvm.org/D47849#1125019). AFAICS this patch fixes > declarations of math functions but you still cannot include `math.h` which > most "correct" codes do. I'm not sure

[PATCH] D50447: [clang-tidy] Add a whitelistClasses option in performance-for-range-copy check.

JonasToth added a comment. Is the type important, or specifics about the variable (e.g. the name?) The `_` variable names are sometimes used for RAII variables/lambdas that shall just do some cleanup, e.g. gsl::finally(). It might make sense to give `ExprMutAnalyzer` an ignore-mechanism. I

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

rjmccall added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:1276 +InitVar->setSection(".CRT$XCLa"); +CGM.addUsedGlobal(InitVar); + } DHowett-MSFT wrote: > rjmccall wrote: > > Is the priority system not good enough? > My reading of the LLVM

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

theraven updated this revision to Diff 159651. theraven marked 2 inline comments as done. theraven added a comment. Herald added a subscriber: mgrang. - Address Dustin's review comments. - Fix an issue in protocol generation. - Fix failing test. - [gnu-objc] Make selector order deterministic. -

[PATCH] D50385: [clangd] Collect symbol occurrences from AST.

ioeric added a comment. 2 high-level questions: 1. What's the reason for having a separate `SymbolOccurrenceSlab`? Could store occurrences as extra payload of `Symbol`? 2. Could we merge `SymbolOccurrenceCollector` into the existing `SymbolCollector`? They look a lot alike. Having another

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

theraven updated this revision to Diff 159653. theraven added a comment. - Revert blocks part of the patch to put in a separate review. Repository: rC Clang https://reviews.llvm.org/D50144 Files: include/clang/Driver/Options.td lib/AST/MicrosoftMangle.cpp lib/CodeGen/CGException.cpp

[PATCH] D50193: Added functionality to suggest FixIts for conversion of '->' to '.' and vice versa.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE339224: Added functionality to suggest FixIts for conversion of - to . and vice… (authored by kadircet, committed by ). Changed prior to commit: https://reviews.llvm.org/D50193?vs=159648=159655#toc

[PATCH] D47849: [OpenMP][Clang][NVPTX] Enable math functions called in an OpenMP NVPTX target device region to be resolved as device-native function calls

Hahnfeld added a comment. In https://reviews.llvm.org/D47849#1190997, @gtbercea wrote: > Don't we want to use device specific math functions? > It's not just about avoiding some the host specific assembly, it's also > about getting an implementation tailored to the device. Ok, so you are

[PATCH] D50337: [clangd] DexIndex implementation prototype

kbobyrev updated this revision to Diff 159658. kbobyrev added a comment. Minor code cleanup. This is now a fully functional symbol index. I have reflected my concerns and uncertainties in `FIXME`s, please indicate if you think there's something to improve in this patch. In general, I believe it

[PATCH] D49796: [ASTImporter] Load external Decls when getting field index.

balazske updated this revision to Diff 159659. balazske added a comment. - Added common getFieldIndex. - Corrected test ImportUnnamedFieldsInCorrectOrder. Repository: rC Clang https://reviews.llvm.org/D49796 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp

[PATCH] D49796: [ASTImporter] Load external Decls when getting field index.

This revision was automatically updated to reflect the committed changes. Closed by commit rL339226: [ASTImporter] Load external Decls when getting field index. (authored by balazske, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D49796

r339226 - [ASTImporter] Load external Decls when getting field index.

Author: balazske Date: Wed Aug 8 02:40:57 2018 New Revision: 339226 URL: http://llvm.org/viewvc/llvm-project?rev=339226=rev Log: [ASTImporter] Load external Decls when getting field index. Summary: At equality check of fields without name the index of fields is compared. At determining the

[PATCH] D50428: Add support for importing imaginary literals

martong updated this revision to Diff 159662. martong added a comment. Remove superflous newline Repository: rC Clang https://reviews.llvm.org/D50428 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp lib/ASTMatchers/ASTMatchersInternal.cpp

[PATCH] D50428: Add support for importing imaginary literals

martong created this revision. martong added reviewers: a_sidorin, r.stahl, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Repository: rC Clang https://reviews.llvm.org/D50428 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp

[PATCH] D50412: [libunwind] Fix pointer-to-integer cast warnings on LLP64.

mstorsjo added a subscriber: hans. mstorsjo added a comment. @hans This looks 7.0-worthy to me. Repository: rL LLVM https://reviews.llvm.org/D50412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50193: Added functionality to suggest FixIts for conversion of '->' to '.' and vice versa.

kadircet updated this revision to Diff 159648. kadircet marked 3 inline comments as done. kadircet added a comment. - Resolve some more discussions. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50193 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

mstorsjo added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:1276 +InitVar->setSection(".CRT$XCLa"); +CGM.addUsedGlobal(InitVar); + } rjmccall wrote: > DHowett-MSFT wrote: > > rjmccall wrote: > > > Is the priority system not good enough? > > My

[PATCH] D50413: [libunwind][include] Add some missing definitions to .

mstorsjo added inline comments. Comment at: include/unwind.h:46 +typedef uintptr_t _Unwind_Ptr; +#endif + What other reference is this list of typedefs for `_Unwind_Ptr` based on? I don't see any of these cases in clang's unwind.h at least.

[PATCH] D50380: [Headers] Expand _Unwind_Exception for SEH on MinGW/x86_64

hans added a comment. In https://reviews.llvm.org/D50380#1191423, @mstorsjo wrote: > @hans Can you merge this for 7.0? This is necessary for > https://reviews.llvm.org/D49638 (merged well before the branch) to work > properly without causing heap corruption. Merged in r339220. Repository:

Re: r339170 - [Headers] Expand _Unwind_Exception for SEH on MinGW/x86_64

Merged to 7.0 in r339220. On Tue, Aug 7, 2018 at 10:02 PM, Martin Storsjo via cfe-commits wrote: > Author: mstorsjo > Date: Tue Aug 7 13:02:40 2018 > New Revision: 339170 > > URL: http://llvm.org/viewvc/llvm-project?rev=339170=rev > Log: > [Headers] Expand _Unwind_Exception for SEH on

[PATCH] D50412: [libunwind] Fix pointer-to-integer cast warnings on LLP64.

hans added a comment. In https://reviews.llvm.org/D50412#1191843, @mstorsjo wrote: > @hans This looks 7.0-worthy to me. Okay, r339222. Repository: rL LLVM https://reviews.llvm.org/D50412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[libunwind] r339222 - Merging r339217:

Author: hans Date: Wed Aug 8 00:50:31 2018 New Revision: 339222 URL: http://llvm.org/viewvc/llvm-project?rev=339222=rev Log: Merging r339217: r339217 | cdavis | 2018-08-08 06:21:24 +0200 (Wed, 08 Aug 2018) | 11 lines

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

JonasToth added a comment. > Could you give a concrete example of this? vi llvm/lib/Demangle/ItaniumDemangle.cpp +1762 /home/jonas/opt/llvm/lib/Demangle/ItaniumDemangle.cpp:1762:7: warning: variable 'num' of type 'char [FloatData::max_demangled_size]' can be declared 'const'

[PATCH] D50414: [libunwind][include] Add SEH declarations to .

mstorsjo added a comment. Should we maybe add the same declaration of `_GCC_specific_handler` to clang's unwind.h? That would allow removing the forward declaration in libcxxabi from https://reviews.llvm.org/D49638. Do you plan on implementing these SEH specific bits in libunwind, or are you

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

DHowett-MSFT added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:1276 +InitVar->setSection(".CRT$XCLa"); +CGM.addUsedGlobal(InitVar); + } rjmccall wrote: > Is the priority system not good enough? My reading of the LLVM language reference leads

Re: [libunwind] r339217 - [libunwind] Fix pointer-to-integer cast warnings on LLP64.

Merged to 7.0 in r339222. On Wed, Aug 8, 2018 at 6:21 AM, Charles Davis via cfe-commits wrote: > Author: cdavis > Date: Tue Aug 7 21:21:24 2018 > New Revision: 339217 > > URL: http://llvm.org/viewvc/llvm-project?rev=339217=rev > Log: > [libunwind] Fix pointer-to-integer cast warnings on LLP64.

[PATCH] D50175: [AArch64][NFC] better matching of AArch64 target in aarch64-cpus.c tests

SjoerdMeijer updated this revision to Diff 159715. SjoerdMeijer added a comment. Addressed comments. https://reviews.llvm.org/D50175 Files: test/Driver/aarch64-cpus.c Index: test/Driver/aarch64-cpus.c === ---

[PATCH] D47849: [OpenMP][Clang][NVPTX] Enable math functions called in an OpenMP NVPTX target device region to be resolved as device-native function calls

gtbercea added a comment. In https://reviews.llvm.org/D47849#1192368, @Hahnfeld wrote: > In https://reviews.llvm.org/D47849#1192321, @gtbercea wrote: > > > > IIRC you started to work on this to fix the problem with inline assembly > > > (see https://reviews.llvm.org/D47849#1125019). AFAICS this

[PATCH] D50414: [libunwind][include] Add SEH declarations to .

cdavis5x updated this revision to Diff 159722. cdavis5x added a comment. - Remove unneeded preprocessor condition. Repository: rUNW libunwind https://reviews.llvm.org/D50414 Files: include/unwind.h Index: include/unwind.h

[PATCH] D50447: [clang-tidy] Add a whitelistClasses option in performance-for-range-copy check.

hokein added a comment. > That looks remarkably like google benchmark main loop. (i don't know at hand > if making this const will break it, but probably not?) > I wonder if instead there should be an option to not complain about the > variables that aren't actually used? Yeah, it is google

  1   2   >