[PATCH] D148785: -fsanitize=function: use type hashes instead of RTTI objects

hans added a comment. This broke the tests on Mac, see e.g. https://green.lab.llvm.org/green/job/clang-stage1-RA/34396/consoleFull (or http://crbug.com/1447928) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148785/new/ https://reviews.llvm.org/D1

[PATCH] D151344: Reland "[CMake] Bumps minimum version to 3.20.0.

hans added a comment. This looks right to me. (I'm out of office at the moment, but this looks like what I tested in https://github.com/llvm/llvm-project/issues/62719 so it should work.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151344/new/ h

[PATCH] D147875: [clang][Diagnostics] Show line numbers when printing code snippets

hans added inline comments. Comment at: clang/docs/UsersManual.rst:578 + +.. option:: -fcaret-diagnostics-max-lines: + I think this is still a cc1-only option. Should it be made available as a driver option now? Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D144684: [clang][Driver] Pass /INFERASANLIBS:NO to link.exe under -fsanitize=address

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144684/new/ https://reviews.llvm.org/D144684 ___ cfe

[PATCH] D150001: [clang] Fix initializer_list matching failures with modules

hans added a comment. Nice! Comment at: clang/lib/Sema/SemaDeclCXX.cpp:11570 getStdNamespace()->setImplicit(true); +Context.getTranslationUnitDecl()->addDecl(getStdNamespace()); +getStdNamespace()->clearIdentifierNamespace(); This could use an expl

[PATCH] D149997: [clang] [test] Narrow down MSVC specific behaviours from "any windows" to only MSVC/clang-cl

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149997/new/ https://reviews.llvm.org/D149997 ___ cfe

[PATCH] D149999: [clang-tidy] [test] Narrow down a special case to MSVC mode

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14/new/ https://reviews.llvm.org/D14 ___ cfe

[PATCH] D144509: [CMake] Bumps minimum version to 3.20.0.

hans added a comment. In D144509#4347562 , @glandium wrote: > FYI, 65429b9af6a2c99d340ab2dcddd41dab201f399c > is > causing problems on Windows compiler-rt for some reason I haven't id

[PATCH] D144509: [CMake] Bumps minimum version to 3.20.0.

hans added a comment. In D144509#4349399 , @mstorsjo wrote: > In D144509#4349051 , @hans wrote: > >> In D144509#4347562 , @glandium >> wrote: >> >>> FYI, 65429b9af6a2c99d

[PATCH] D150817: Use windows baskslash on anonymous tag locations if using MSVCFormatting and it's not absolute path.

hans accepted this revision. hans added a comment. Nice! lgtm Comment at: clang/lib/AST/TypePrinter.cpp:1391 + WrittenFile = Callbacks->remapPath(File); +// The following tries to fix inconsistent path separator created by +// clang::DirectoryLookup::Lo

[PATCH] D144509: [CMake] Bumps minimum version to 3.20.0.

hans added a comment. In D144509#4350052 , @Mordante wrote: > In D144509#4349921 , @thakis wrote: > >> Reverted this and follow-ups in d763c6e5e2d0a6b34097aa7dabca31e9aff9b0b6 >>

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

hans added a comment. Herald added subscribers: hoy, Enna1, StephenFan. We gave this a try in Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=1434989 While the diagnostics are interesting, two things make this less useful for our developers: 1. It also fires in situations where

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

hans added a comment. In D115907#4295923 , @paulkirth wrote: >> 2. Due to inlining etc., it often gets the source locations wrong, which >> means it points at code where again there were no expectations -- but >> perhaps that code got inlined into an ex

[PATCH] D149187: [clang] Canonicalize system headers in dependency file when -canonical-prefixes

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. Thanks! I like it. Comment at: clang/include/clang/Frontend/Utils.h:44 class Preprocessor; +class FileManager; class PreprocessorOptions; nit: sort? Reposito

[PATCH] D149206: [clang][driver] Enable MisExpect diagnostics flag outside of CC1

hans added inline comments. Comment at: clang/test/Profile/misexpect-branch.c:10 // RUN: %clang_cc1 %s -O2 -o - -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify=foo -fdiagnostics-misexpect-tolerance=10 -Wmisexpect -debug-info-kind=line-tables-only +// RUN: %clang

[PATCH] D149206: [clang][driver] Enable MisExpect diagnostics flag outside of CC1

hans added inline comments. Comment at: clang/test/Driver/clang_f_opts.c:144 +// RUN: %clang -### -S -fprofile-instr-use=%t.profdata -fdiagnostics-misexpect-tolerance=10 -Wmisexpect %s 2>&1 | FileCheck %s --check-prefix=CHECK-MISEXPECT-TOLLERANCE +// CHECK-MISEXPECT-TOLLERANCE-

[PATCH] D149206: [clang][driver] Enable MisExpect diagnostics flag outside of CC1

hans accepted this revision. hans added a comment. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149206/new/ https://reviews.llvm.org/D149206 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D149695: MS inline asm: remove obsolete code adding AOK_SizeDirective (e.g. dword ptr)

hans added a comment. > The AOK_SizeDirective part from 5b37c181291210bedfbb7a6af5d51229f3652ef0 > (2014-08) seems unneeded nowadays (the root cause has likely been fixed > elsewhere). Would it be possible to confirm when/if the size directive here became unnecessary? Repository: rG LLVM Git

[PATCH] D149695: MS inline asm: remove obsolete code adding AOK_SizeDirective (e.g. dword ptr)

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149695/new/ https://reviews.llvm.org/D149695 ___ cfe

[PATCH] D152090: [clang][Driver] Add -fcaret-diagnostics-max-lines as a driver option

hans added a comment. Should there be a test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152090/new/ https://reviews.llvm.org/D152090 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D152696: Prevent deadlocks in death tests.

hans added a comment. I'm no expert here, but I went to check what Chromium does, and it seems to set the death_test_style to "threadsafe" for the whole test suite: https://source.chromium.org/chromium/chromium/src/+/main:base/test/test_suite.cc;l=367 > As the page linked above notes, "flags ar

[PATCH] D152696: Prevent deadlocks in death tests.

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152696/new/ https://reviews.llvm.org/D152696 ___ cfe

[PATCH] D139428: Handle char{8,16,32} and wchar_t in ASTContext::getIntegerRank()

hans created this revision. hans added a reviewer: aaron.ballman. Herald added a project: All. hans requested review of this revision. Herald added a project: clang. They were previously not handled, causing the llvm_unreachable with "getIntegerRank(): not a built-in integer" message to be hit.

[PATCH] D139428: Handle char{8,16,32} and wchar_t in ASTContext::getIntegerRank()

hans updated this revision to Diff 480538. hans marked 2 inline comments as done. hans added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139428/new/ https://reviews.llvm.org/D139428 Files: clang/lib/AST/ASTContext.cpp clang/test/SemaCXX/vector.cpp I

[PATCH] D139428: Handle char{8,16,32} and wchar_t in ASTContext::getIntegerRank()

hans added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:7109 + case BuiltinType::Char8: +return getIntegerRank(CharTy.getTypePtr()); + case BuiltinType::Char16: tahonermann wrote: > This should technically be `UnsignedCharTy` though it probably

[PATCH] D139428: Handle char{8,16,32} and wchar_t in ASTContext::getIntegerRank()

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcb3ea52a5ae5: Handle char{8,16,32} and wchar_t in ASTContext::getIntegerRank() (authored by hans). Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D139167: [clang][Windows]Ignore Options '/d1nodatetime' and '/d1import_no_registry'

hans added a comment. I see this landed in https://github.com/llvm/llvm-project/commit/4178671b2ed3d Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139167/new/ https://reviews.llvm.org/D139167 __

[PATCH] D136100: [clang-format] Do not parse certain characters in pragma directives

hans added a comment. Chromium is seeing a formatting regression after this: https://github.com/llvm/llvm-project/issues/59473 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136100/new/ https://reviews.llvm.org/D136100

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

hans added a comment. Could we add a release note and/or update to the clang manual explaining the new behavior? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136315/new/ https://reviews.llvm.org/D136315 __

[PATCH] D21113: Add support for case-insensitive header lookup

hans added a comment. In https://reviews.llvm.org/D21113#627413, @francisco.lopes wrote: > I'm waiting for this to land as well, it's crucial for coding windows code > with libclang assistance on a linux machine. I'm not actively working on this, so it might take a while. In the meantime, I r

[PATCH] D31248: [X86] Implement __readgsqword (and the rest) as builtins (PR32373)

hans created this revision. It seems MS headers have started using __readgsqword, and since it's used in a header that doesn't include intrin.h, we can't implement it as an inline function anymore. That was already the case for __readfsdword, which Saleem added support for in r220859. Thi

[PATCH] D31248: [X86] Implement __readgsqword (and the rest) as builtins (PR32373)

This revision was automatically updated to reflect the committed changes. Closed by commit rL298538: [X86] Implement __readgsqword (and the rest) as builtins (PR32373) (authored by hans). Changed prior to commit: https://reviews.llvm.org/D31248?vs=92664&id=92679#toc Repository: rL LLVM http

[PATCH] D29221: clang-format-vsix: "format on save" feature

hans added a comment. In https://reviews.llvm.org/D29221#698679, @amaiorano wrote: > Hey guys, any feedback on this? About 1.5 weeks ago, @hans asked @klimek and > @djasper for their opinions on "clang-format" vs "ClangFormat". Thanks! Let's get this committed now and not worry about renaming

[PATCH] D31736: Implement _interlockedbittestandset as a builtin

hans created this revision. It's used by MS headers in VS 2017 without including intrin.h, so we can't implement it in the header anymore. https://reviews.llvm.org/D31736 Files: include/clang/Basic/Builtins.def lib/CodeGen/CGBuiltin.cpp lib/Headers/intrin.h test/CodeGen/ms-intrinsics.c

[PATCH] D31736: Implement _interlockedbittestandset as a builtin

hans updated this revision to Diff 94433. hans added a comment. Add comment. https://reviews.llvm.org/D31736 Files: include/clang/Basic/Builtins.def lib/CodeGen/CGBuiltin.cpp lib/Headers/intrin.h test/CodeGen/ms-intrinsics.c Index: test/CodeGen/ms-intrinsics.c =

[PATCH] D31736: Implement _interlockedbittestandset as a builtin

hans added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:570 +llvm::AtomicOrdering::SequentiallyConsistent); +llvm::Value *Shifted = Builder.CreateLShr(RMWI, Bit); +llvm::Value *Truncated = rnk wrote: > Can you comment that this shifts th

[PATCH] D31736: Implement _interlockedbittestandset as a builtin

This revision was automatically updated to reflect the committed changes. Closed by commit rL299782: Implement _interlockedbittestandset as a builtin (authored by hans). Changed prior to commit: https://reviews.llvm.org/D31736?vs=94433&id=94539#toc Repository: rL LLVM https://reviews.llvm.o

[PATCH] D32138: clang-cl: Support the /Zc:twoPhase[-] command-line option (PR32680)

hans created this revision. It sounds like MSVC is adding support for two-phase name lookup in a future version, enabled by this flag (see bug). https://reviews.llvm.org/D32138 Files: include/clang/Driver/CLCompatOptions.td test/Driver/cl-options.c Index: test/Driver/cl-options.c ===

[PATCH] D32138: clang-cl: Support the /Zc:twoPhase[-] command-line option (PR32680)

hans added a comment. We could wait until a version of MSVC ships with this, in case they decide to change the name, or we could just land it now. https://reviews.llvm.org/D32138 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.

[PATCH] D32138: clang-cl: Support the /Zc:twoPhase[-] command-line option (PR32680)

This revision was automatically updated to reflect the committed changes. Closed by commit rL300501: clang-cl: Support the /Zc:twoPhase[-] command-line option (PR32680) (authored by hans). Changed prior to commit: https://reviews.llvm.org/D32138?vs=95489&id=95494#toc Repository: rL LLVM htt

[PATCH] D28606: Mention devirtualization in release notes

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm Can you write something about this option for UsersManual.rst too? Then we could make this note link to it. https://reviews.llvm.org/D28606 __

[PATCH] D28727: Add -fstrict-vtable-pointers to UserManual

hans added a comment. Would it be possible to expand on, or give some kind of pointer to, what "strict rules for overwriting polymorphic C++ objects" means? https://reviews.llvm.org/D28727 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D28727: Add -fstrict-vtable-pointers to UserManual

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm with comment Comment at: docs/UsersManual.rst:1103 + Enable optimizations based on the strict rules for overwriting polymorphic + C++ objects, e.g. vptr is invariant dur

[PATCH] D28933: Revert the return type for `emplace_(back|front)` to `void` in C++14 and before

hans added a comment. This sounds like something we'll want to merge to the release branch when it lands? https://reviews.llvm.org/D28933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D29152: Drop 'dllimport' when redeclaring inline function template without the attribute (PR31695)

hans created this revision. For non-template `dllimport` functions, MSVC allows providing an inline definition without spelling out the attribute again. In the example below, `f` remains a `dllimport` function. __declspec(dllimport) int f(); inline int f() { return 42; } int useit() {

[PATCH] D29152: Drop 'dllimport' when redeclaring inline function template without the attribute (PR31695)

hans added a reviewer: rsmith. hans added a comment. +Richard in case you have any theories on why they do it like this. https://reviews.llvm.org/D29152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D29198: clang-cl: Warn about /U flags that look like filenames (PR31662)

hans created this revision. Both on Mac and Windows, it's common to have a 'Users' directory in the root of the filesystem, so one might specify a filename as '/Users/me/myfile.c'. clang-cl (as well as MSVC's cl.exe) will interpret that as invoking '/U' option, which is probably not what the us

[PATCH] D29198: clang-cl: Warn about /U flags that look like filenames (PR31662)

hans added inline comments. Comment at: include/clang/Basic/DiagnosticDriverKinds.td:252 + InGroup>; +def note_use_dashdash : Note<"Use '--' to treat subsequent arguments as filenames">; + amccarth wrote: > For Windows, wouldn't it make more sense to suggest us

[PATCH] D29198: clang-cl: Warn about /U flags that look like filenames (PR31662)

This revision was automatically updated to reflect the committed changes. Closed by commit rL293305: clang-cl: Warn about /U flags that look like filenames (PR31662) (authored by hans). Changed prior to commit: https://reviews.llvm.org/D29198?vs=85963&id=86062#toc Repository: rL LLVM https:

[PATCH] D29152: Drop 'dllimport' when redeclaring inline function template without the attribute (PR31695)

hans added a comment. Ping? https://reviews.llvm.org/D29152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29350: clang-cl: Evaluate arguments left-to-right in constructor call with initializer list (PR31831)

hans created this revision. clang-cl would evaluate the arguments right-to-left (see PR), and for non-Windows targets I suppose we only got it because we were already emitting left-to-right in CodeGenFunction::EmitCallArgs. https://reviews.llvm.org/D29350 Files: lib/CodeGen/CGClass.cpp te

[PATCH] D29350: clang-cl: Evaluate arguments left-to-right in constructor call with initializer list (PR31831)

hans added a comment. In https://reviews.llvm.org/D29350#662525, @rnk wrote: > Can you check if we got this wrong in clang 3.8 and 3.9? I'm wondering if > this was a regression, or if we always got this wrong. I tried back to 3.5, and it seems we just always got it wrong. https://reviews.llv

[PATCH] D29350: clang-cl: Evaluate arguments left-to-right in constructor call with initializer list (PR31831)

This revision was automatically updated to reflect the committed changes. Closed by commit rL293732: clang-cl: Evaluate arguments left-to-right in constructor call with initializer… (authored by hans). Changed prior to commit: https://reviews.llvm.org/D29350?vs=86496&id=86549#toc Repository:

[PATCH] D29152: Drop 'dllimport' when redeclaring inline function template without the attribute (PR31695)

This revision was automatically updated to reflect the committed changes. Closed by commit rL293800: Drop 'dllimport' when redeclaring inline function template without the… (authored by hans). Changed prior to commit: https://reviews.llvm.org/D29152?vs=85818&id=86676#toc Repository: rL LLVM

[PATCH] D34873: Fix miscompiled 32bit binaries by mingw

hans added a reviewer: rnk. hans added a comment. From the bug, this is related to Richard's "r289413 - Add two new AST nodes to represent initialization of an array in terms of initialization of each array element", which broke MSVC builds due to under-alignment, which Reid addressed with "r28

[PATCH] D35578: Add -fswitch-tables and -fno-switch-tables flags

hans added a comment. I'm also curious about the motivation for this. LLVM backends can opt out of these kinds of tables if they're not suitable for the target, but why would a Clang user want to do it? https://reviews.llvm.org/D35578 ___ cfe-comm

[PATCH] D35578: Add -fswitch-tables and -fno-switch-tables flags

hans added a comment. In https://reviews.llvm.org/D35578#814590, @sgundapa wrote: > I will try to address the concerns here: > > > What exactly is the motivation? I'm trying to narrow down the justification > > for adding yet more flags. > > (I just typed this message in https://reviews.llvm.org

[PATCH] D35427: [clang] Fix handling of "%zd" format specifier

hans added a comment. Thanks for fixing this long-standing TODO :-) Repository: rL LLVM https://reviews.llvm.org/D35427 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33900: Print registered targets in clang's version information

hans added a comment. In https://reviews.llvm.org/D33900#819237, @dim wrote: > Also note that it is only added to the `--version` output, not the `-v` > output (the former is really a "verbose" version of the latter): Which seems to be the opposite of what gcc does (`gcc -v` is more verbose th

[PATCH] D35780: Introduce -nostdlib++ flag to disable linking the C++ standard library.

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. This seems reasonable to me. Comment at: lib/Driver/ToolChain.cpp:660 + assert(!Args.hasArg(options::OPT_nostdlibxx) && + "should not have called this"); CXXStdlibTyp

[PATCH] D35906: Add bitrig removal to release notes

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. Looks good. Thanks! Comment at: docs/ReleaseNotes.rst:61 +- Bitrig OS was merged back into OpenBSD, so Bitrig support has been +removed from Clang/LLVM. + I thi

[PATCH] D33900: Print registered targets in clang's version information

hans added a comment. In https://reviews.llvm.org/D33900#822313, @mehdi_amini wrote: > In https://reviews.llvm.org/D33900#820281, @hans wrote: > > > In https://reviews.llvm.org/D33900#818968, @mehdi_amini wrote: > > > > > I think @thakis is right: this too verbose to be the default --version. > >

[PATCH] D35577: Add -flookup-tables and -fno-lookup-tables flags

hans added a comment. In https://reviews.llvm.org/D35577#818956, @mcrosier wrote: > In https://reviews.llvm.org/D35577#818936, @kparzysz wrote: > > > In https://reviews.llvm.org/D35577#817944, @echristo wrote: > > > > > "Should this just be part of the tuning for the hexagon backend and not > >

[PATCH] D41538: [analyzer] Fix some checker's output plist not containing the checker name #2

hans added a comment. In https://reviews.llvm.org/D41538#969207, @xazax.hun wrote: > In https://reviews.llvm.org/D41538#969205, @sylvestre.ledru wrote: > > > It missed the 6.0 branching. Will you try to get it on this branch? > > Thanks > > > Sure! I will try to do so: https://reviews.llvm.org/rC

[PATCH] D42085: Add va_start()/va_copy()/va_end to Builtins.def

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm Seems that some of these are tested in Sema/implicit-builtin-decl.c, but it doesn't look exhaustive and I'm not sure it's worth adding these. https://reviews.llvm.org/D42085

[PATCH] D41708: [clang-tidy] Update fuchsia-overloaded-operator to check for valid loc

hans added a comment. Alex, do you think the fix is still OK for the 6.0 branch? Let me know, and I'll merge it. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41708 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D42157: [clang-cl] Let /FA output use intel assembly.

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm, thanks for fixing! https://reviews.llvm.org/D42157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

[PATCH] D41708: [clang-tidy] Update fuchsia-overloaded-operator to check for valid loc

hans added a comment. In https://reviews.llvm.org/D41708#978186, @alexfh wrote: > In https://reviews.llvm.org/D41708#977000, @hans wrote: > > > Alex, do you think the fix is still OK for the 6.0 branch? Let me know, and > > I'll merge it. > > > Yes, please. Thank you! Oh, it was already merged

[PATCH] D42349: [DOCS] Mention OpenMP Tools Interface in runtime library

hans added a comment. In https://reviews.llvm.org/D42349#983180, @Hahnfeld wrote: > @hans I'd also like to merge this to `release_60` because the support landed > before the branching. Is this ok? Sounds good to me. Go ahead and merge, or let me know and I'll do it. Repository: rC Clang h

[PATCH] D42352: [ReleaseNotes] Mention OpenMP Tools Interface in runtime library

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. Sounds good to me. Comment at: docs/ReleaseNotes.rst:230 + applications, please rebuild the runtime library with `-DLIBOMP_OMPT_SUPPORT=OFF` + and file a bug at `LLVM's Bugzil

[PATCH] D42360: [6.0.0 Release] Release notes for configuration files in clang

hans added a comment. Would it be possible to include a link to the documentation for configuration files? Repository: rC Clang https://reviews.llvm.org/D42360 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin

[PATCH] D42307: [OpenCL][6.0.0 Release] Release notes for OpenCL in Clang

hans accepted this revision. hans added a comment. Looks good to me (though hyperlinks would be nice). Go ahead and commit directly to the branch, or let me know if you'd like me to do it. Comment at: docs/ReleaseNotes.rst:224 +- All function calls are marked by the convergent

[PATCH] D42360: [6.0.0 Release] Release notes for configuration files in clang

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm, please commit directly to the branch of let me know if you'd like me to do it. Repository: rC Clang https://reviews.llvm.org/D42360 ___ cfe

[PATCH] D42497: clang-cl: Simplify handling of /arch: flag.

hans added a comment. Thanks for looking into this! Comment at: lib/Driver/ToolChains/Arch/X86.cpp:43 if (const Arg *A = Args.getLastArg(options::OPT__SLASH_arch)) { +// Mapping built by looking at lib/Basic's X86TargetInfo::initFeatureMap(). I wonder

[PATCH] D42497: clang-cl: Simplify handling of /arch: flag.

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D42497 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D42538: [clang-cl] Add support for /arch:AVX512F and /arch:AVX512

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D42538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D42641: [MinGW] Emit typeinfo locally for dllimported classes without key functions

hans added a comment. In https://reviews.llvm.org/D42641#995845, @mstorsjo wrote: > @hans I'd like to have this in 6.0 as well, to allow building Qt for windows > as DLLs. Okay, let's just have it bake in trunk a little bit longer and then I'll merge. Repository: rL LLVM https://reviews.l

[PATCH] D42860: [ReleaseNotes] Add note for the new -fexperimental-isel flag.

This revision was automatically updated to reflect the committed changes. Closed by commit rL324212: [ReleaseNotes] Add note for the new -fexperimental-isel flag. (authored by hans, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D42860

[PATCH] D42641: [MinGW] Emit typeinfo locally for dllimported classes without key functions

hans added a comment. In https://reviews.llvm.org/D42641#996117, @hans wrote: > In https://reviews.llvm.org/D42641#995845, @mstorsjo wrote: > > > @hans I'd like to have this in 6.0 as well, to allow building Qt for > > windows as DLLs. > > > Okay, let's just have it bake in trunk a little bit lo

[PATCH] D29830: [OpenCL][Doc] Relase 4.0 notes for OpenCL

hans accepted this revision. hans added a comment. Thanks for writing these! Go ahead and commit to the branch, or let me know if you'd like me to do it. https://reviews.llvm.org/D29830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D29221: clang-format-vsix: "format on save" feature

hans added a comment. I'm not really qualified to review the implementation details, but this seems good as far as I can tell. A few comments, mostly style related. Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:39 private string style = "file"; +

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

hans added a comment. This seems to be stuck. Bruno, Richard, do you think there's a chance this can be fixed for 4.0? https://reviews.llvm.org/D29753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D30239: enable -flto=thin, -flto-jobs=, and -fthinlto-index= in clang-cl

hans added a comment. In https://reviews.llvm.org/D30239#683697, @inglorion wrote: > @mehdi_amini: > > > Is clang-cl using lld as default? How is the switch done? Ideally we should > > have a nice error message from the driver if -flto is used without lld. > > I believe we use link.exe by defaul

[PATCH] D30239: enable -flto=thin in clang-cl

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm with a comment Comment at: test/Driver/cl-options.c:525 +// RUN: %clang_cl -### /c -flto %s 2>&1 | FileCheck -check-prefix=LTO %s +// LTO: -flto This need

[PATCH] D30239: enable -flto=thin in clang-cl

hans added inline comments. Comment at: test/Driver/cl-options.c:532 +// RUN: %clang_cl -### -Fe%t.exe -entry:main -flto -- %s 2>&1 | FileCheck -check-prefix=LTO-WITHOUT-LLD %s +// LTO-WITHOUT-LLD: invalid argument '-flto' only allowed with '-fuse-ld=lld' + This

[PATCH] D29542: [TargetInfo] Adjust x86-32 atomic support to the CPU used

hans added a reviewer: jyknight. hans added a comment. +jyknight, who had a similar patch in http://reviews.llvm.org/D17933 (see also r291477 and PR31864) Comment at: test/CodeGen/atomic-ops.c:1 -// RUN: %clang_cc1 %s -emit-llvm -o - -ffreestanding -ffake-address-space-map -t

[PATCH] D30239: enable -flto=thin in clang-cl

hans added a comment. lgtm2 https://reviews.llvm.org/D30239 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29221: clang-format-vsix: "format on save" feature

hans added a comment. In https://reviews.llvm.org/D29221#686865, @amaiorano wrote: > Made changes based on @hans 's feedback. > > I looked again at the categories, and I think it makes sense with my changes. > Here's an updated screenshot that shows what the options menu in Visual > Studio look

[PATCH] D29221: clang-format-vsix: "format on save" feature

hans added a subscriber: djasper. hans added a comment. >>> My only nit is that I'd prefer "clang-format" instead of "ClangFormat". >>> >>> Manuel: the menu options under Tools currently say "Clang Format >>> {Selection,Document}". What do you think about using "clang-format" here >>> instead?

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

hans added a subscriber: mehdi_amini. hans added a comment. To unblock 4.0, I'm leaning towards merging Bruno's patch as a stop-gap fix. I realize it probably only fixes the problem for PCH, and not modules. But PCH is used more widely than modules, so maybe it's good enough for now? We've run

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D29753#688845, @bruno wrote: > > What do folks think? > > IMO we should do it. Go ahead and commit this, but keep the bug open so we can work on fixing it properly ev

[PATCH] D30538: Add documentation for -fno-strict-aliasing

hans added a comment. +1 for documenting this, but I have to leave it to the language lawyers for how to fomulate it. > Enables/disables the strict aliasing assumption, which assumes that objects > of different types do not share the same location in memory. I think it needs to say incompatibl

[PATCH] D26335: [ms] Reinstate https://reviews.llvm.org/D14748 after https://reviews.llvm.org/D20291

hans added inline comments. Comment at: lib/Headers/x86intrin.h:49 +static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__)) +__tzcnt_u32(unsigned int __X) { return __X ? __builtin_ctz(__X) : 32; } +#ifdef __x86_64__ andreadb wrote: > hans w

[PATCH] D26657: [Sema] Respect DLL attributes more faithfully

hans added a comment. Apologies for the delay. I was out last week. In https://reviews.llvm.org/D26657#602083, @smeenai wrote: > General coding style question. Over here, I'm creating a local helper > function. However, that helper needs to access member functions of Sema, > which is why I mad

[PATCH] D26657: [Sema] Respect DLL attributes more faithfully

hans added inline comments. Comment at: include/clang/Sema/Sema.h:7495 + /// \brief Make an existing DLL attribute on a class take effect. + void ActOnDLLAttr(ClassTemplateSpecializationDecl *Def, +InheritableAttr *Attr); hans wrote: > I'd s

[PATCH] D26657: [Sema] Respect DLL attributes more faithfully

hans added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:7439 +void Sema::ActOnDLLAttr(ClassTemplateSpecializationDecl *Def, +InheritableAttr *Attr) { + // We reject explicit instantiations in class scope, so there should smeenai

[PATCH] D26657: [Sema] Respect DLL attributes more faithfully

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm Comment at: lib/Sema/SemaTemplate.cpp:7710 +(Context.getTargetInfo().getCXXABI().isMicrosoft() || + Context.getTargetInfo().getTriple().isWindowsItaniumEnvir

[PATCH] D27438: clang-format-vsix: add a date stamp to the VSIX version number to ensure upgradability

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. When building the snapshots and releases, I usually put the SVN revision number in there to avoid this same problem: http://llvm-cs.pcc.me.uk/utils/release/build_llvm_package.bat#25 Your patch se

[PATCH] D27438: clang-format-vsix: add a date stamp to the VSIX version number to ensure upgradability

hans added a comment. In https://reviews.llvm.org/D27438#614223, @amaiorano wrote: > In https://reviews.llvm.org/D27438#614219, @hans wrote: > > > When building the snapshots and releases, I usually put the SVN revision > > number in there to avoid this same problem: > > http://llvm-cs.pcc.me.u

[PATCH] D27438: clang-format-vsix: add a date stamp to the VSIX version number to ensure upgradability

hans added a comment. In https://reviews.llvm.org/D27438#615396, @amaiorano wrote: > > Yes, I think this is still a worthwhile change. Let's add the hour and > > minute and get it landed. > > Alright, added hour and minute. How do we land this change? Is this something > I can now do myself? Or

<    5   6   7   8   9   10   11   >