[PATCH] D66040: [Sema] Implement DR2386 for C++17 structured binding

2019-08-15 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 215442. rnk added a comment. - add DR test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66040/new/ https://reviews.llvm.org/D66040 Files: clang/lib/Sema/SemaDeclCXX.cpp clang/test/CXX/dcl.decl/dcl.decomp

[PATCH] D66040: [Sema] Implement DR2386 for C++17 structured binding

2019-08-15 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL369043: [Sema] Implement DR2386 for C++17 structured binding (authored by rnk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://rev

[PATCH] D66361: Improve behavior in the case of stack exhaustion.

2019-08-16 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: include/clang/Basic/Stack.h:42 + llvm::function_ref Fn) { +if (LLVM_UNLIKELY(isStackNearlyExhausted())) { + runWithSufficientStackSpaceSlow(Diag, Fn); For LLVM_THREADS_DISABL

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

2019-08-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I'd be happy to take this patch, address the comments, and land it, if you don't want to deal with all the nits. Comment at: test/CodeGen/debug-info-no-location.cpp:1 +// RUN: %clang -cc1 -emit-llvm %s -gcodeview -debug-info-kind=limited -o - | FileCheck

[PATCH] D66394: clang-cl: Enable /Zc:twoPhase by default if targeting MSVC 2017 update 3 or newer

2019-08-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm! I can't believe we didn't notice this for a year. Not long ago a Firefox developer filed a bug for a crash with `-fdelayed-template-parsing`, and if this works for them, we won't have to debug

[PATCH] D63625: [CodeGen][test] Use -fno-discard-value-names for better test support

2019-06-20 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Thanks. This is surprisingly the first time I've heard of fdiscard-value-names. Seems useful, since now you can test your IR both ways without having to completely recompile clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D63600: [test][Driver] Fix Clang :: Driver/cl-response-file.c

2019-06-21 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. This causes the test to fail on Windows: http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/7712 There seems to be something wrong with the blamelist, so it didn't send email. I see this in the log from the previous build on that bot: Updating llvm to 363241

[PATCH] D63678: Fix test Clang :: Driver/cl-response-file.c for Solaris

2019-06-21 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I would almost prefer to XFAIL this on Solaris, I feel like the intent is clearer with echo. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63678/new/ https://reviews.llvm.org/D63678 ___ cfe-commits mailing list cfe-com

[PATCH] D63678: Fix test Clang :: Driver/cl-response-file.c for Solaris

2019-06-21 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D63678#1554513 , @ro wrote: > Please have a look at what the autoconf manual has to say on echo vs. printf: Yikes, fair enough. I actually really wish lit had some support for the here-document pattern: cat

[PATCH] D63678: Fix test Clang :: Driver/cl-response-file.c for Solaris

2019-06-24 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63678/new/ https://reviews.llvm.org/D63678 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D63048: Update __VERSION__ to remove the hardcoded 4.2.1 version

2019-06-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I'd be in favor of removing them. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63048/new/ https://reviews.llvm.org/D63048 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D64067: [X86][PPC] Support -mlong-double-64

2019-07-02 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Sounds good to me. I had a minor suggestion, though. Comment at: lib/Basic/Targets/PPC.cpp:468 Opts.AltiVec = 1; - TargetInfo::adjust(Opts); + if (Opts.LongDoubleSize == 64) { +LongDoubleWidth = LongDoubleAlign = 64; I think it w

[PATCH] D64062: Remove both -dumpversion and __VERSION__

2019-07-02 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. But, based on the github search, I don't think we could reasonably change dumpversion to print 8.0.0, so I'm not sure it really matters. Firefox builds with clang these days, so I'm not sure what they could possibly be using -dumpversion for that would matter. Repository:

[PATCH] D64067: [X86][PPC] Support -mlong-double-64

2019-07-03 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D64067#1568533 , @andrew.w.kaylor wrote: > In this review (https://reviews.llvm.org/D6260) @rsmith mentions that this > should also have an effect on name mangling. I'm not sure that's consistent with GCC, at least not anymore:

[PATCH] D64164: [clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems.

2019-07-03 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. Please check the commit message: > [clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems. I think you mean "use int on LP64 systems", since long is 32-bits on LLP64, right? Otherwise, sounds good. Repository: rC Clang CHANGES S

[PATCH] D64253: Let unaliased Args track which Alias they were created from, and use that in Arg::getAsString() for diagnostics

2019-07-08 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: llvm/include/llvm/Option/Arg.h:59 /// The argument values, as C strings. SmallVector Values; I'm not sure this is really true: > For conve

[PATCH] D64301: Use `ln -n` to prevent forming a symlink cycle, instead of rm'ing the source

2019-07-08 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a subscriber: zturner. rnk added a comment. This revision is now accepted and ready to land. lgtm I think @zturner wanted to teach lit to completely remove the Output/ directory for every test so we don't have to deal with stale files from previous tests ha

[PATCH] D64349: clang-cl: Port cl.exe's C4659 to clang-cl

2019-07-08 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:2283 if (Attr *ClassAttr = getDLLAttr(Class)) { - if (auto *BaseTemplate = dyn_cast_or_null( - BaseType->getAsCXXRecordDecl())) { This file seems untouched except for wh

[PATCH] D64349: clang-cl: Port cl.exe's C4659 to clang-cl

2019-07-08 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm May I ask what inspired this? :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64349/new/ https://reviews.llvm.org/D64349 ___ cfe-commit

[PATCH] D64067: [X86][PPC] Support -mlong-double-64

2019-07-08 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64067/new/ https://reviews.llvm.org/D64067 ___ cfe-commits mailing

[PATCH] D64062: Remove both -dumpversion and __VERSION__

2019-07-08 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D64062#1567175 , @sylvestre.ledru wrote: > This isn't firefox per say but thirdparty apps. > If you feel confident, sure, we can land that and see what happens :) One concern that I have is that we don't have a replacement for `

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

2019-07-09 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:420 +std::error_code setTimeRes = +llvm::sys::fs::setLastAccessAndModificationTime(FD, NewTimePt, +NewTimePt);

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

2019-07-09 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Reverted in rC365581 . Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D64458: add -fthinlto-index= option to clang-cl

2019-07-09 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D64458#1577435 , @pcc wrote: > Do we really need to support clang-cl syntax here? Can't the user of > -fthinlto-index= use the regular clang driver? I think it depends on how many compiler flags we have to pass to backend compil

[PATCH] D64527: driver: Don't warn about assembler flags being unused when not assembling

2019-07-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64527/new/ https://reviews.llvm.org/D64527 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D64506: clang-cl: Remove -O0 option

2019-07-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Does `clang-cl -O0` without a `/` still work, though? It's used: $ git grep O0 ../compiler-rt/test/asan/TestCases/Windows/ ../compiler-rt/test/asan/TestCases/Windows/aligned_mallocs.cc:// RUN: %clang_cl_asan -O0 %s -Fe%t ../compiler-rt/test/asan/TestCases/Windows/allocators_

[PATCH] D64504: Various minor tweaks to CLCompatOptions.td

2019-07-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: clang/include/clang/Driver/CLCompatOptions.td:102 def _SLASH_Gy_ : CLFlag<"Gy-">, HelpText<"Don't put each function in its own section (default)">, Alias; ditto Comment at: clang/include/clang/Drive

[PATCH] D64538: [Driver] Don't escape backslashes on Windows

2019-07-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. An extremely convenient feature of the current escaping pattern is that it magically works for both the cmd shell and various bash implementations. You can simply copy paste the commands and run them. This matters, for example, for the .sh crash reproducer script. On Window

[PATCH] D64538: [Driver] Don't escape backslashes on Windows

2019-07-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D64538#1579573 , @smeenai wrote: > In D64538#1579561 , @rnk wrote: > > > An extremely convenient feature of the current escaping pattern is that it > > magically works for both the cmd shell

[PATCH] D64504: Various minor tweaks to CLCompatOptions.td

2019-07-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang/include/clang/Driver/CLCompatOptions.td:294 def _SLASH_GX : CLFlag<"GX">, - HelpText<"Enable exception handling">; + HelpText<"Deprecated (like /EHsc)">;

[PATCH] D64506: clang-cl: Remove -O0 option

2019-07-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64506/new/ https://reviews.llvm.org/D64506 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D64538: [Driver] Don't escape backslashes on Windows

2019-07-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D64538#1579632 , @smeenai wrote: > I'm not fully understanding this. If the .sh file was generated on Windows, > it'll contain backslashes in any arguments that are paths, right? Before this > change, those backslashes would have

[PATCH] D61646: Include corecrt.h/vcruntime.h to improve MS compatibility

2019-05-09 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a subscriber: echristo. rnk added a comment. @echristo mentioned that this patch was also causing issues in another freestanding environment where _MSC_VER was defined, but no corecrt.h header existed on the system. So, I think it's likely that we don't want to do this in the long run

[PATCH] D61742: [Driver][Windows] Add dependent lib argument for profile instr generate

2019-05-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61742/new/ https://reviews.llvm.org/D61742 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D62202: Work around a Visual C++ bug

2019-05-21 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I'd go ahead and commit the workaround, even if it goes against the style guide. This problem will go away in two years or so. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62202/new/ https://reviews.llvm.org/D62202 __

[PATCH] D62202: Work around a Visual C++ bug

2019-05-21 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D62202#1510522 , @dblaikie wrote: > Yeah, if we're going this way I'd certainly advocate having a comment of some > kind explaining why it's this way so it doesn't regress. In the past, I've found these sorts of comments to be pr

[PATCH] D62200: [Driver][Windows] Add dependent lib argument for -fprofile-generate and -fcs-profile-generate

2019-05-21 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk 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/D62200/new/ https://reviews.llvm.org/D62200 ___ cfe-c

[PATCH] D62167: CodeView - add static data members to global variable debug info.

2019-05-21 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. This needs a clang-side test to show that clang generates the expected IR for static data members. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62167/new/ https://reviews.llvm.org/D62167 ___

[PATCH] D62214: Remove extra if case.

2019-05-21 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Testing offline showed that it was impossible to reach the return with a real global variable with an enumeration type. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D62270: [Driver] Move the "-o OUT -x TYPE SRC.c" flags to the end of -cc1

2019-05-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added a reviewer: hans. Herald added a subscriber: jdoerfert. Herald added a project: clang. New -cc1 arguments, such as -faddrsig, have started appearing after the input name. I personally find it convenient for the input to be the last argument to the compile comma

[PATCH] D62276: lld-link, clang: Treat non-existent input files as possible spellos for option flags

2019-05-23 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. Thanks, this has been a longstanding issue! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62276/new/ https://reviews.llvm.org/D62276 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D62270: [Driver] Move the "-o OUT -x TYPE SRC.c" flags to the end of -cc1

2019-05-23 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC361530: [Driver] Move the "-o OUT -x TYPE SRC.c" flags to the end of -cc1 (authored by rnk, committed by ). Changed prior to commit: https://reviews.llvm.org/D62270?vs=200827&id=201026#toc Repository:

[PATCH] D61617: [NewPassManager] Add tuning option: SLPVectorization [clang-change]

2019-05-23 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: test/CodeGen/loop-vectorize.c:1 +// RUN: %clang -target x86_64 -S -c -O1 -fvectorize -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK-ENABLE-VECT +// RUN: %clang -target x86_64 -S -c -O1 -fno-vectorize -emit-llvm -o - %s | FileCheck %

[PATCH] D61742: [Driver][Windows] Add dependent lib argument for profile instr generate

2019-05-28 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I was going to suggest that maybe what we should do is just embed the basename, i.e. `/nodefaultlib:clang_rt.profile-x86_64.lib`, and then we just ask users to add one `/libpath:` flag to their linker invocation. That saves users from having to come up with a mapping from t

[PATCH] D62167: CodeView - add static data members to global variable debug info.

2019-05-28 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added subscribers: manmanren, probinson. rnk added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4385 +// Use the global scope for static members. +DContext = getContextDescriptor( + cast(CGM.getContext().getTranslationUnitDecl()), TheCU); -

[PATCH] D61742: [Driver][Windows] Add dependent lib argument for profile instr generate

2019-05-28 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D61742#1519539 , @russell.gallop wrote: > > I was going to suggest that maybe what we should do is just embed the > > basename, i.e. /nodefaultlib:clang_rt.profile-x86_64.lib ... > > Do you mean /defaultlib:clang_rt.profile-x86_64

[PATCH] D62493: [Driver] Always use Unix-style paths in the Darwin driver

2019-05-28 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. So, there's nothing wrong, functionally speaking, with what we do today, right? It's just inconvenient to test. The difficulty of testing the driver has been a long standing problem. I think we might want to instead invent some new kind of alternative to `-###` for writing

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

2019-05-28 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Thanks! I don't think this suite is going to get too far out of control. I think for most debug info features, checking the info itself gives enough confidence that things work, but there are these cases where I also want an integration test. I'm picking this up again beca

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

2019-05-28 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 201779. rnk added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. - rebase, fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54187/new/ https://reviews.llvm.org/D54187 Files:

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

2019-05-28 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL361889: Add debuginfo-tests that use cdb on Windows (authored by rnk, committed by ). Changed prior to commit: https://reviews.llvm.org/D54187?vs=201779&id=201781#toc Repository: rL LLVM CHANGES SIN

[PATCH] D62167: CodeView - add static data members to global variable debug info.

2019-05-29 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk 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/D62167/new/ https://reviews.llvm.org/D62167 ___ cfe-

[PATCH] D53238: [Driver] Add -static-{rtlib, stdlib} and make -static-{libgcc, libstdc++} their aliases

2018-12-11 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. (sub) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53238/new/ https://reviews.llvm.org/D53238 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D55229: [COFF] Statically link certain runtime library functions

2018-12-11 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Thanks. I think what we really want to do here is reconsider our default for applying dllimport. Leaving things unannotated is a good safe default for every environment. In the absence of any flags, clang should assume runtime functions are statically linked. The linker wil

[PATCH] D55229: [COFF] Statically link certain runtime library functions

2018-12-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: CodeGenObjC/gnu-init.m:103 +// Make sure we do not have dllimport on the load function +// CHECK-WIN: declare dso_local void @__objc_load theraven wrote: > compnerd wrote: > > rnk wrote: > > > compnerd wrote: > > > > rnk w

[PATCH] D55672: Mangle calling conventions into function pointer types where GCC does

2018-12-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: rjmccall, efriedma. GCC 5.1 began mangling these Windows calling conventions into function types, since they can be used for overloading. They've always been mangled in the MS ABI, but they are new to the Itanium mangler. Note that the calling conven

[PATCH] D55672: Mangle calling conventions into function pointer types where GCC does

2018-12-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk planned changes to this revision. rnk added a comment. I guess that was premature, I suppose I should test these cases John enumerated: > I think some basic validation of that is probably called for before > committing. The obvious places to check: > > - a function pointer type > - a funct

[PATCH] D55672: Mangle calling conventions into function pointer types where GCC does

2018-12-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 178137. rnk added a comment. - add more tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55672/new/ https://reviews.llvm.org/D55672 Files: clang/lib/AST/ItaniumMangle.cpp clang/test/CodeGenCXX/mangle-win-ccs.cpp Index: clang/test/CodeGenCXX/ma

[PATCH] D55672: Mangle calling conventions into function pointer types where GCC does

2018-12-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D55672#1330402 , @rnk wrote: > - a specialization of a function template that happens to be declared with > this calling convention (this also probably shouldn't include the CC in the > mangling, but it's okay to match GCC if it d

[PATCH] D55672: Mangle calling conventions into function pointer types where GCC does

2018-12-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D55672#1330460 , @rjmccall wrote: > Thanks. I think you missed these, though: > > - a template argument that's just a function type, not a function pointer or > reference type I think useTemplateFnType is supposed to handle that

[PATCH] D55672: Mangle calling conventions into function pointer types where GCC does

2018-12-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 178138. rnk added a comment. - more tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55672/new/ https://reviews.llvm.org/D55672 Files: clang/lib/AST/ItaniumMangle.cpp clang/test/CodeGenCXX/mangle-win-ccs.cpp Index: clang/test/CodeGenCXX/mangle

[PATCH] D55672: Mangle calling conventions into function pointer types where GCC does

2018-12-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 178144. rnk added a comment. - Add ms_abi and sysv_abi tests to match GCC CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55672/new/ https://reviews.llvm.org/D55672 Files: clang/lib/AST/ItaniumMangle.cpp clang/test/CodeGenCXX/mangle-win-ccs.cpp cla

[PATCH] D55677: [Builltins][X86] Provide implementations of __lzcnt16, __lzcnt, __lzcnt64 for MS compatibility. Remove declarations from intrin.h and implementations from lzcntintrin.h

2018-12-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. This has the effect of removing __lzcnt* from non-MSVC targets using immintrin.h. Is that intended? It doesn't seem like these implementations were guarded by ifdef _MSC_VER. Otherwise, yep, makes sense. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55677/new/ ht

[PATCH] D55677: [Builltins][X86] Provide implementations of __lzcnt16, __lzcnt, __lzcnt64 for MS compatibility. Remove declarations from intrin.h and implementations from lzcntintrin.h

2018-12-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D55677#1330559 , @craig.topper wrote: > yeah I'm now realizing that gcc does implement __lzcnt16 and __lzcnt64. How > do I make this work? I tried guarding them with _MSC_VER, but I got > redefinition errors on a couple tests fro

[PATCH] D55677: [Builltins][X86] Provide implementations of __lzcnt16, __lzcnt, __lzcnt64 for MS compatibility. Remove declarations from intrin.h and implementations from lzcntintrin.h

2018-12-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm The macro seems like a reasonable solution. It also helps keep the global namespace cleaner for non-windows users. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55677/new/ https://re

[PATCH] D55715: Add AddressSpace mangling to MS mode

2018-12-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: test/CodeGenCXX/mangle-address-space.cpp:7 +// CHECKNOOCL-LABEL: define {{.*}}void @_Z2f0Pc +// WINNOOCL-LABEL: define {{.*}}void @"?f0@@YAXPEAD@Z" +// CHECKOCL-LABEL: define {{.*}}void @_Z2f0PU9CLgenericc You know, now that

[PATCH] D55715: Add AddressSpace mangling to MS mode

2018-12-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/AST/MicrosoftMangle.cpp:1806-1836 +Extra.mangleSourceName("AS_"); +Extra.mangleIntegerLiteral(llvm::APSInt::getUnsigned(TargetAS), + /*IsBoolean*/ false); + } else { +switch (AS) { +default:

[PATCH] D55698: [MinGW] Produce a vtable and RTTI for dllexported classes without a key function

2018-12-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:5754-5756 + if (ClassExported && + Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) +MarkVTableUsed(Class->getLocation(), Class, true); This may be too early, you can get into

[PATCH] D55685: Update Microsoft name mangling scheme for exception specifiers in the type system

2018-12-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/AST/MicrosoftMangle.cpp:2311-2314 + if (FT->canThrow()) +Out << 'Z'; + else +Out << "_E"; zahen wrote: > zturner wrote: > > I knew that the mangling changed whenever a pointer to a `noexcept` > > function is p

[PATCH] D55715: Add AddressSpace mangling to MS mode

2018-12-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: test/CodeGenCXX/mangle-address-space.cpp:7 +// CHECKNOOCL-LABEL: define {{.*}}void @_Z2f0Pc +// WINNOOCL-LABEL: define {{.*}}void @"?f0@@YAXPEAD@Z" +// CHECKOCL-LA

[PATCH] D55698: [MinGW] Produce a vtable and RTTI for dllexported classes without a key function

2018-12-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55698/new/ https://reviews.llvm.org/D55698 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D55672: Mangle calling conventions into function pointer types where GCC does

2018-12-14 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL349212: Mangle calling conventions into function pointer types where GCC does (authored by rnk, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org

[PATCH] D55749: [Driver] Automatically enable -munwind-tables if -fseh-exceptions is enabled

2018-12-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:3929 + options::OPT_fno_asynchronous_unwind_tables, + (TC.IsUnwindTablesDefault(Args) || + (ExceptionArg && Can you put this logic in the Windows or mingw toolchain instead by

[PATCH] D55781: Make CC mangling conditional on the ABI version

2018-12-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: rjmccall, rsmith. Herald added a subscriber: krytarowski. https://reviews.llvm.org/D55781 Files: clang/lib/AST/ItaniumMangle.cpp clang/test/CodeGenCXX/mangle-win-ccs-old.cpp Index: clang/test/CodeGenCXX/mangle-win-ccs-old.cpp ==

[PATCH] D55781: Make CC mangling conditional on the ABI version

2018-12-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. @rsmith asked if we should make this mangling conditional on the ABI version. I implemented it here, let me know if you think it's worth the extra code complexity. Our mingw ABI hasn't been particularly stable, but I think going forward we'll want to start mangling new call

[PATCH] D55749: [Driver] Automatically enable -munwind-tables if -fseh-exceptions is enabled

2018-12-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55749/new/ https://reviews.llvm.org/D55749 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D55685: Update Microsoft name mangling scheme for exception specifiers in the type system

2018-12-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55685/new/ https://reviews.llvm.org/D55685 ___ cfe-commits mailing list cfe-commit

[PATCH] D55685: Update Microsoft name mangling scheme for exception specifiers in the type system

2018-12-17 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL349414: Update Microsoft name mangling scheme for exception specifiers in the type… (authored by rnk, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.ll

[PATCH] D55853: Ignore ConstantExpr in IgnoreParenNoopCasts

2018-12-18 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: void, rsmith. It is ignored by IgnoreParenImpCasts, so it seems reasonable to ignore it here as well. Besides, this preserves behavior from before this AST node was added. Fixes PR39881 https://reviews.llvm.org/D55853 Files: clang/lib/AST/Expr.

[PATCH] D55853: Ignore ConstantExpr in IgnoreParenNoopCasts

2018-12-18 Thread Reid Kleckner via Phabricator via cfe-commits
rnk marked an inline comment as done. rnk added inline comments. Comment at: clang/lib/AST/Expr.cpp:2722-2725 +if (ConstantExpr *CE = dyn_cast(E)) { + E = CE->getSubExpr(); + continue; +} majnemer wrote: > Just curious, why not go even further a

[PATCH] D55853: Ignore ConstantExpr in IgnoreParenNoopCasts

2018-12-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 178960. rnk added a comment. - move to IgnoreParens CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55853/new/ https://reviews.llvm.org/D55853 Files: clang/lib/AST/Expr.cpp clang/test/CodeGenCXX/mangle-ms-templates.cpp Index: clang/test/CodeGenCXX/

[PATCH] D55853: Ignore ConstantExpr in IgnoreParenNoopCasts

2018-12-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk marked an inline comment as done. rnk added inline comments. Comment at: clang/lib/AST/Expr.cpp:2694 while (true) { E = E->IgnoreParens(); rsmith wrote: > Maybe `IgnoreParens` should be doing this itself? I can do that, it passes tests. CHANGES SI

[PATCH] D55853: Ignore ConstantExpr in IgnoreParens

2018-12-21 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Want to stamp this? It's 4pm on the Friday before Christmas, what could go wrong? :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55853/new/ https://reviews.llvm.org/D55853 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D55853: Ignore ConstantExpr in IgnoreParens

2018-12-26 Thread Reid Kleckner via Phabricator via cfe-commits
rnk marked 4 inline comments as done. rnk added inline comments. Comment at: clang/lib/AST/Expr.cpp:2550 } +if (ConstantExpr *CE = dyn_cast(E)) { + E = CE->getSubExpr(); rsmith wrote: > Does this pass the tests if you use `FullExpr` here instead? I

[PATCH] D55853: Ignore ConstantExpr in IgnoreParens

2018-12-26 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL350068: Ignore ConstantExpr in IgnoreParens (authored by rnk, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D55853?vs=178960&id=179515#toc R

[PATCH] D55915: [Driver] Make -fno-omit-frame-pointer imply -mno-omit-leaf-frame-pointer

2018-12-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: test/Driver/cl-options.c:177 // RUN: %clang_cl --target=i686-pc-win32 -Werror /Oy- /O2 -### -- %s 2>&1 | FileCheck -check-prefix=Oy_2 %s -// Oy_2: -momit-leaf-frame-pointer +// Oy_2: -mdisable-fp-elim // Oy_2: -O2 tabloid

[PATCH] D56273: Validate -add-plugin arguments.

2019-01-03 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56273/new/ https://reviews.llvm.org/D56273 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D67304: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI

2019-09-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D67304#1664696 , @thakis wrote: > We have the old TODO of changing this warning to be emitted at enum use time > (e.g. when Foo_Val is compared to 0) instead of declaration time. Maybe > that's a better fix. Or is implementing tha

[PATCH] D47956: [MS] Consder constexpr globals to be inline, as in C++17

2019-09-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 219638. rnk added a comment. Herald added a subscriber: mstorsjo. Herald added a project: clang. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47956/new/ https://reviews.llvm.org/D47956 Files: clang/lib

[PATCH] D47956: [MS] Consder constexpr globals to be inline, as in C++17

2019-09-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D47956#1138555 , @rnk wrote: > In D47956#1138521 , @rsmith wrote: > > > Can we now remove the corresponding MSVC-specific hacks elsewhere (eg, > > `ASTContext::isMSStaticDataMemberInlineDefi

[PATCH] D67426: Don't warn about selectany on implicitly inline variables

2019-09-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: epastor, thakis, hans. Herald added a project: clang. This avoids a -Wignored-attribute warning on the code pattern Microsoft recommends for integral const static data members defined in headers here: https://docs.microsoft.com/en-us/cpp/build/refere

[PATCH] D67304: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI

2019-09-10 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL371581: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI (authored by rnk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: h

[PATCH] D47956: [MS] Consder constexpr globals to be inline, as in C++17

2019-09-11 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL371642: [MS] Consder constexpr globals to be inline, as in C++17 (authored by rnk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https:/

[PATCH] D67454: Start porting ivfsoverlay tests to Windows

2019-09-11 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added a reviewer: amccarth. Herald added a subscriber: arphaman. Herald added a project: clang. Part of PR43272, the changes are: 1. Use @ as the sed pattern delimiter instead of : so that the drive letter in lit substitutions isn't an issue. 2. Use the %/t and %/

[PATCH] D67463: [MS] Warn when shadowing template parameters under -fms-compatibility

2019-09-11 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: thakis, hans. Herald added a project: clang. C++ does not allow shadowing template parameters, but previously we allowed it under -fms-extensions. Now this behavior is controlled by -fms-compatibility, and we emit a -Wmicrosoft-template warning when

[PATCH] D67454: Start porting ivfsoverlay tests to Windows

2019-09-11 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL371663: Start porting ivfsoverlay tests to Windows (authored by rnk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.

[PATCH] D67426: Don't warn about selectany on implicitly inline variables

2019-09-12 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb00a49d1b3a1: Don't warn about selectany on implicitly inline variables (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67426/new/ https://

[PATCH] D67463: [MS] Warn when shadowing template parameters under -fms-compatibility

2019-09-12 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL371753: [MS] Warn when shadowing template parameters under -fms-compatibility (authored by rnk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to comm

[PATCH] D67590: Properly ignore mismatched exception specifiers in MSVC Compat mode.

2019-09-16 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/Sema/SemaDecl.cpp:3565-3572 +if (OldQTypeForComparison == NewQType || +// In Microsoft compatibility mode, the intent is to only warn on +// mismatched exception specifiers. By this point, that warning has +/

[PATCH] D67590: Properly ignore mismatched exception specifiers in MSVC Compat mode.

2019-09-16 Thread Reid Kleckner via Phabricator via cfe-commits
rnk commandeered this revision. rnk edited reviewers, added: alexfusco; removed: rnk. rnk added a comment. Taking this to move the test around and try the other version... Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67590/new/ https://reviews.llvm.org/D67590

[PATCH] D67590: Properly ignore mismatched exception specifiers in MSVC Compat mode.

2019-09-16 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 220376. rnk added a comment. - move test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67590/new/ https://reviews.llvm.org/D67590 Files: clang/lib/Sema/SemaDecl.cpp clang/test/SemaCXX/ms-exception-spec.cpp I

<    2   3   4   5   6   7   8   9   10   11   >