[PATCH] D69760: [Clang][Driver] Don't pun -fuse-ld=lld as -fuse-ld=lld-link with msvc

rnk added a comment. > I think a better distinguisher would be whether clang was invoked as > clang[++] or clang-cl (i.e. driver mode). I think Martin said most of what I wanted to say. Making this dependent on the driver mode seems reasonable and would be the easier way forward. If you still

[PATCH] D69626: Fix Microsoft compatibility handling of commas in nested macro expansions.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Based on the comments it looks like you confirmed this matches MSVC's behavior, at least in this regard. I haven't stared at this deeply to think of all the corner cases, though. To Nico's po

[PATCH] D69322: [hip][cuda] Enable extended lambda support on Windows.

rnk added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:395-400 /// The number used to indicate this lambda expression for name /// mangling in the Itanium C++ ABI. unsigned ManglingNumber : 31; +/// The device side mangling number. +unsi

[PATCH] D69582: Let clang driver support parallel jobs

rnk added subscribers: aganea, amccarth, rnk. rnk added a comment. +@aganea @amccarth Users have been asking for /MP support in clang-cl for a while, which is basically this. Is there anything in JobScheduler that could reasonably be moved down to llvm/lib/Support? I would also like to be able

[PATCH] D69322: [hip][cuda] Enable extended lambda support on Windows.

rnk added inline comments. Comment at: clang/include/clang/AST/MangleNumberingContext.h:57-58 + + /// Has device mangle numbering context. + virtual bool hasDeviceMangleNumberingContext() const { return false; } + It would be nicer if there were a single entry

[PATCH] D67723: [DebugInfo] Add option to disable inline line tables.

rnk added a comment. In D67723#1717468 , @aprantl wrote: > I agree that it would make sense to have a `-ginline-info-threshold=<#insns>` > or `-gno-small-inline-functions` with a hardcoded threshold to implement the > feature Paul described, and this pat

[PATCH] D67723: [DebugInfo] Add option to disable inline line tables.

rnk added a comment. In D67723#1710134 , @probinson wrote: > FTR, since @rnk has mentioned my years-ago writings, what Sony has internally > nowadays is a little different than what I said back then. We have an option > spelled `-gno-inlined-scopes` wh

[PATCH] D69012: [Headers] Fix compatibility between arm_acle.h and intrin.h

rnk accepted this revision. rnk added a comment. lgtm Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69012/new/ https://reviews.llvm.org/D69012 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D69262: Prune include of DataLayout.h from include/clang/Basic/TargetInfo.h. NFC

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm It's interesting that Basic depends on IR at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69262/new/ https://reviews.llvm.org/D69262 __

[PATCH] D69261: Prune Pass.h include from DataLayout.h. NFCI

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. > That include seemed irrelevant to DataLayout, as > wee as lots of users of DataLayout. Typo in commit message (well as...), worth fixing before landing. lgtm Comment at: llvm/

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

rnk added a comment. I think this looks pretty good, thanks! I really only noticed style nits. I think with some small fixes, we should go ahead and merge it. If you want, I can commit it on your behalf, but I know there are other folks at Microsoft with commit access to LLVM, if you'd prefer.

[PATCH] D67723: [DebugInfo] Add option to disable inline line tables.

rnk added a comment. In D67723#1708671 , @aprantl wrote: > Who is "we" in this context? The CodeView backend? Yes, the CodeView backend, sorry for the ambiguity. > As far as DWARF is concerned (and LLVM mostly inherits the DWARF semantics) > line 0 is

[PATCH] D67723: [DebugInfo] Add option to disable inline line tables.

rnk added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1428 +if (isa(BI)) { + BI = --(BI->eraseFromParent()); + continue; Will this work if the dbg.value is the first instruction of a basic block? I'd expect

[PATCH] D68720: Support -fstack-clash-protection for x86

rnk added a comment. For maintenance reasons, I'd really prefer it if we could find a way to reuse the existing code that calls an external stack probe function. What do you think about taking a look at X86RetpolineThunks.cpp and doing something similar to that? Basically, when the user sets `-

[PATCH] D68953: Enable most VFS tests on Windows

rnk added subscribers: arphaman, vsapsai, Bigcheese. rnk added a comment. +VFS people, mostly just to let them know @arphaman @Bigcheese @vsapsai Code changes all look good to me, but I had a bunch of questions. Comment at: clang/lib/Lex/HeaderSearch.cpp:782-783 // Conc

[PATCH] D67723: [DebugInfo] Add option to disable inline line tables.

rnk added a comment. Based on what we learned in https://llvm.org/PR43530, I think we still want to use the location of the call site and not line zero. :( Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1431 +} +BI->setDebugLoc(TheCallDL); +co

[PATCH] D67723: [CodeView] Add option to disable inline line tables.

rnk added a comment. I guess the commit message shouldn't say "[CodeView] Add option to disable inline line tables." It's really an option for all debug info. You could put "[DebugInfo]" on there, or just drop the tag. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D67723: [CodeView] Add option to disable inline line tables.

rnk added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1427 +// Remove debug info intrinsics. +if (auto *DbgInst = dyn_cast(BI)) { + BI = --(DbgInst->eraseFromParent()); Each of these inherit from DbgVariableI

[PATCH] D68055: Add -fgnuc-version= to control __GNUC__ and other GCC macros

This revision was automatically updated to reflect the committed changes. Closed by commit rG5e866e411caa: Add -fgnuc-version= to control __GNUC__ and other GCC macros (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D68055?vs=224155&id=224472#toc Repository: rG LLVM Gith

[PATCH] D68733: Use -fdebug-compilation-dir to form absolute paths in coverage mappings

This revision was automatically updated to reflect the committed changes. Closed by commit rGf6777964bde2: Use -fdebug-compilation-dir to form absolute paths in coverage mappings (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D68733?vs=224172&id=224221#toc Repository: r

[PATCH] D68733: Use -fdebug-compilation-dir to form absolute paths in coverage mappings

rnk marked an inline comment as done. rnk added inline comments. Comment at: clang/test/CoverageMapping/debug-dir.cpp:14 +// +// RELATIVE: @__llvm_coverage_mapping = {{.*"\\01[^/]*foobar.*debug-dir\.cpp}} + vsk wrote: > Does the "[^/]" check work on Windows, or i

[PATCH] D68736: [MSVC] Automatically add atlmfc include and lib directories as system paths.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm I think the old VS Express editions used to exclude atlmfc, so there's the possibility that we'll be searching non-existent directories, but I think that's OK. Whoever wrote this code was prob

[PATCH] D68733: Use -fdebug-compilation-dir to form absolute paths in coverage mappings

rnk created this revision. rnk added reviewers: vsk, arphaman. Herald added a subscriber: dexonsmith. Herald added a project: clang. This allows users to explicitly request relative paths with `-fdebug-compilation-dir .`. Fixes PR43614 Repository: rG LLVM Github Monorepo https://reviews.llvm

[PATCH] D68055: Add -fgnuc-version= to control __GNUC__ and other GCC macros

rnk updated this revision to Diff 224155. rnk added a comment. - Fix __GNUG__, tighten tests for it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68055/new/ https://reviews.llvm.org/D68055 Files: clang/docs/ReleaseNotes.rst clang/docs/UsersMan

[PATCH] D68055: Add -fgnuc-version= to control __GNUC__ and other GCC macros

rnk updated this revision to Diff 224141. rnk added a comment. - add docs, release note - keep __EXCEPTIONS if !ms - keep __private_extern__ if !ms - set __GNUG__ value with flag Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68055/new/ https://revi

[PATCH] D68716: [clang] prevent crash for nonnull attribut in constant context (Bug 43601)

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks. Comment at: clang/lib/AST/ExprConstant.cpp:5446 I != E; ++I) { if (!Evaluate(ArgValues[I - Args.begin()], Info, *I)) { // If we're checking for a

[PATCH] D68055: Add -fgnuc-version= to control __GNUC__ and other GCC macros

rnk added a comment. In D68055#1698653 , @rsmith wrote: > From a big-picture perspective, I would like us to move to treating MS and > GNU extensions in the same way (at the cc1 level) to the extent that we can. > I think this moves us in that direction.

[PATCH] D68610: [clang] enable_trivial_var_init_zero should not be Joined<>

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/D68610/new/ https://reviews.llvm.org/D68610 ___ cfe-c

[PATCH] D68608: [clang] Accept -ftrivial-auto-var-init in clang-cl

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang/include/clang/Driver/Options.td:1720 " | pattern">, Values<"uninitialized,pattern">; def enable_trivial_var_init_zero : Joined<["-"], "enable-trivial-au

[PATCH] D68584: Fix Calling Convention through aliases

rnk accepted this revision. rnk added a comment. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68584/new/ https://reviews.llvm.org/D68584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D68099: [MS ABI]: Fix mangling function arguments for template types to be compatible with MSVC

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm I confirmed the test case matches the name that MSVC produces. I guess this regressed in rC362293 / D62746 , so the bug is

[PATCH] D68114: Fix for expanding __pragmas in macro arguments

rnk added a subscriber: Bigcheese. rnk added a comment. + @Bigcheese, for amusement. We discussed some interesting behavior of _Pragma and C++ raw string literals last night. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68114/new/ https://reviews

[PATCH] D67847: [Support] make report_fatal_error `abort` instead of `exit`

rnk accepted this revision. rnk added a comment. In D67847#1691898 , @jyknight wrote: > The `abort()` function raises SIGABRT, for which the default behavior is to > trigger a coredump. Do we actually want that behavior? > > Either `_exit()` (long availab

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

rnk added a comment. One last idea is that we could teach LLD to automatically add this directory to library search path, but then users who link with Visual C++ (not many anymore) will run into this as a corner case issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

rnk added a comment. In D65543#1691819 , @mstorsjo wrote: > Another slightly related thread, regarding libs from the clang resource dir > and how they are specified to the linker (regarding the builtins library): > https://reviews.llvm.org/D51440 I sup

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

rnk updated this revision to Diff 223128. rnk added a comment. Herald added subscribers: llvm-commits, mgorny. Herald added a project: LLVM. - Fix PGO build Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65543/new/ https://reviews.llvm.org/D65543 F

[PATCH] D68114: Fix for expanding __pragmas in macro arguments

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Nice, the solution was right in front of us the whole time. :) lgtm, but please share the TokenCollector class. Comment at: clang/lib/Lex/Pragma.cpp:169 struct TokenCollector

[PATCH] D67723: [CodeView] Add option to disable inline line tables.

rnk added inline comments. Comment at: clang/include/clang/Driver/Options.td:1943 +def gno_inline_line_tables : Flag<["-"], "gno-inline-line-tables">, + Flags<[CC1Option, CoreOption]>, HelpText<"Don't emit inline line tables">; aprantl wrote: > As a DWARF pers

[PATCH] D68321: Fix clang Visual Studio build instructions

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/D68321/new/ https://reviews.llvm.org/D68321 ___ cfe-commits mailing

[PATCH] D68255: [X86] Remove AVX/AVX512 check from validateOperandSize, just always accept 512

rnk added subscribers: ahatanak, void. rnk added a comment. I notice that x86 is the only target for which validateInput/OutputSize are implemented. If you are going to disable these checks, perhaps we should get rid of these methods and leave all these errors to the backend? You could add `-S`

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

rnk added a comment. I happened to notice that I felt the same way about this in 2014 that I do today: https://reviews.llvm.org/D4428#56536 That makes me feel like I should keep pushing for this change, even though so far people don't seem enthusiastic about it. :) I changed the driver to add t

[PATCH] D68055: Add -fgnuc-version= to control __GNUC__ and other GCC macros

rnk created this revision. rnk added reviewers: rjmccall, rsmith, hans. Herald added subscribers: s.egerton, simoncook, fedor.sergeev, aheejin. Herald added a project: clang. I noticed that compiling on Windows with -fno-ms-compatibility had the side effect of defining __GNUC__, along with __EXCEP

[PATCH] D67723: [CodeView] Add option to disable inline line tables.

rnk added a comment. + other debug info people Comment at: llvm/docs/LangRef.rst:1436 function. This can have very system-specific consequences. +``no-inline-line-tables`` +When this attribute is set to true, inline line tables are not generated This i

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL372178: Ignore exception specifier mismatch when merging redeclarations (authored by rnk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

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

rnk updated this revision to Diff 220564. rnk added a comment. - simplify merging check 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

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

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

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

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.

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] D67463: [MS] Warn when shadowing template parameters under -fms-compatibility

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] D67426: Don't warn about selectany on implicitly inline variables

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] D67454: Start porting ivfsoverlay tests to Windows

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] D67463: [MS] Warn when shadowing template parameters under -fms-compatibility

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

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] D47956: [MS] Consder constexpr globals to be inline, as in C++17

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] D67304: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI

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] D67426: Don't warn about selectany on implicitly inline variables

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] D47956: [MS] Consder constexpr globals to be inline, as in C++17

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] D47956: [MS] Consder constexpr globals to be inline, as in C++17

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] D67304: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI

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] D67304: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI

rnk requested review of this revision. rnk added a comment. Ptal, new patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67304/new/ https://reviews.llvm.org/D67304 ___ cfe-commits mailing list cfe-com

[PATCH] D67304: Unify checking enumerator values in ObjC, C, and MSVC modes

rnk added a comment. In D67304#1663042 , @hans wrote: > Should there be a test exercising this part? The updated tests both have > -fms-compatibility, so were already just warning. Good point, we aren't testing that this unfixed enum behavior is a part

[PATCH] D67304: Unify checking enumerator values in ObjC, C, and MSVC modes

rnk updated this revision to Diff 219425. rnk added a comment. - rewrite, abandon unification Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67304/new/ https://reviews.llvm.org/D67304 Files: clang/lib/Sema/SemaDecl.cpp clang/test/Sema/Microsoft

[PATCH] D67028: Use musttail for variadic method thunks when possible

This revision was automatically updated to reflect the committed changes. Closed by commit rL371269: Use musttail for variadic method thunks when possible (authored by rnk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://re

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk marked an inline comment as done. rnk added inline comments. Comment at: clang/test/CodeGenCXX/ms-thunks-variadic-return.cpp:9 +struct B : virtual A { + // expected-error@+1 2 {{cannot compile this return-adjusting thunk with variadic arguments yet}} + B *clone(const char

[PATCH] D67304: Unify checking enumerator values in ObjC, C, and MSVC modes

rnk created this revision. rnk added reviewers: hans, rsmith, STL_MSFT. Herald added a project: clang. These three modes need to range check enumerator values differently than C++ does. Before this change, we had two codepaths doing the same thing in two cases: 1. enum complete (ObjC fixed & Micr

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

rnk added inline comments. Comment at: llvm/lib/Target/X86/X86FixupCFGuard.cpp:13 +/// for such cases and replaces the pair of instructions with a single +/// call/invoke. For example: +/// hans wrote: > Naive question: Why do we generate code as in the examples

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

rnk added a subscriber: hans. rnk added a comment. Here is some feedback, I apologize for dragging my feet. Also, I would really like to get feedback from @pcc. He's OOO currently, though. Comment at: clang/include/clang/Driver/Options.td:500 def b : JoinedOrSeparate<["-"], "

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk updated this revision to Diff 218995. rnk added a comment. - emit an error if we try to clone without a definition Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67028/new/ https://reviews.llvm.org/D67028 Files: clang/lib/CodeGen/CGVTables.cp

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk added a comment. In D67028#1659881 , @efriedma wrote: > > In your test case, we hit the early return that I linked to, so we don't > > try to clone, and we don't need to emit an error. > > Yes, that's what happens with the Itanium ABI; what happens wi

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk updated this revision to Diff 218992. rnk added a comment. - merge into thunks.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67028/new/ https://reviews.llvm.org/D67028 Files: clang/lib/CodeGen/CGVTables.cpp clang/test/CodeGenCXX/lineta

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk added a comment. I think what I said applies to your test case. Basically, in the Itanium C++ ABI, virtual method definitions provide all their thunks as `weak_odr`. We only emit thunks referenced by vtables as an optimization, and they are marked `available_externally`. In your test case,

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk marked an inline comment as done. rnk added a comment. In D67028#1653439 , @efriedma wrote: > Do we have test coverage for a variadic, covariant thunk for a function > without a definition? I don't think there's any way for us to actually emit > tha

[PATCH] D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm with the GlobalArrayDestructor name. Comment at: include/clang/AST/GlobalDecl.h:34 AtExit, + GlobalDestructor }; Maybe we should rename this GlobalArrayD

[PATCH] D67141: [DebugInfo] Emit DW_TAG_enumeration_type for referenced global enumerator.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67141/new/ https://reviews.llvm.org/D67141

[PATCH] D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

rnk added a comment. I'm trying to figure out what exactly the new GlobalDestructor thing is. :) It's not clear to me why we ever emit `__cxx_global_array_dtor` in the MS ABI. We always call it directly from the `?__F` atexit helper stub instead of registering it, so we really don't need it at

[PATCH] D67141: [DebugInfo] Emit DW_TAG_enumeration_type for referenced global enumerator.

rnk added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4441-4462 + // Use global variables for enums in CodeView, use DW_TAG_enumeration_type for + // enums for non-CodeView. if (const auto *ECD = dyn_cast(VD)) { const auto *ED = cast(ECD->getDeclCont

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk created this revision. rnk added reviewers: rsmith, hans, efriedma. Herald added a subscriber: fedor.sergeev. Herald added a project: clang. This avoids cloning variadic virtual methods when the target supports musttail and the return type is not covariant. I think we never implemented this pr

[PATCH] D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

rnk added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:3581 +llvm::DILocalScope *PrevScope = +!LexicalBlockStack.empty() +? dyn_cast(LexicalBlockStack.back()) Is it OK to look up the lexical block stack at this point? The block

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

rnk added a comment. I'm not sure what happens, but I see you added .gitattributes. I'd commit it as is. Buildbots using svn will keep working. You can check that the monorepo has the right line endings afterwards, and try again if not. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D665

[PATCH] D66827: Add support for MS qualifiers __ptr32, __ptr64, __sptr, __uptr.

rnk added a comment. This change needs a clang CodeGen test to show that we generate IR with `__ptr32` / `__ptr64` in the correct places. We should already have some semantic tests, but we may need more. Comment at: clang/lib/AST/MicrosoftMangle.cpp:1874 +case LangAS::ptr

[PATCH] D66770: Move EH spec mismatches under -fms-compatibility

This revision was automatically updated to reflect the committed changes. Closed by commit rG39aa8954a484: Move EH spec mismatches under -fms-compatibility (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66770/new/ https://reviews.l

[PATCH] D66813: Remove clang-tidy-vs plugin from clang-tools-extra

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/D66813/new/ https://reviews.llvm.org/D66813 ___ cfe-c

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

rnk added a comment. I plan to take a look at this tomorrow, sorry for putting this off for a while. :( Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65761/new/ https://reviews.llvm.org/D65761 ___ cfe

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. I think we're ready to proceed here, lgtm. Shout if I've misrepresented anything. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64931/new/ https:/

[PATCH] D66770: Move EH spec mismatches under -fms-compatibility

rnk created this revision. rnk added a reviewer: hans. Herald added a project: clang. -fms-extensions is intended to enable conforming language extensions and -fms-compatibility is intended to language rule relaxations, so a user could plausibly compile with -fno-ms-compatibility on Windows while

[PATCH] D66606: IR. Change strip* family of functions to not look through aliases.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm GlobalOpt already replaces replaceable aliases, and if it doesn't do it enough, we can adjust the pass pipeline to fix the phase ordering problem. Repository: rG LLVM Github Monorepo CHANG

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

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] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

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] D66361: Improve behavior in the case of stack exhaustion.

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] D66040: [Sema] Implement DR2386 for C++17 structured binding

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] D66040: [Sema] Implement DR2386 for C++17 structured binding

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

rnk marked an inline comment as done. rnk added a comment. In D66040#1631658 , @thakis wrote: > rnk, what's the status here? I think I can get this in today, things just got busy here and I forgot about this. I have to add that test.

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

rnk created this revision. rnk added a reviewer: rsmith. Herald added a project: clang. Allow implementations to provide complete definitions of std::tuple_size, but to omit the 'value' member to signal that T is not tuple-like. The Microsoft standard library implements std::tuple_size this way.

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

rnk added inline comments. Comment at: clang/lib/Basic/Targets/OSTargets.h:771-772 } else if (Triple.getArch() == llvm::Triple::x86) { - this->resetDataLayout("e-m:e-p:32:32-i64:64-n8:16:32-S128"); + this->resetDataLayout("e-m:e-p:32:32-p253:32:32-p254:32:32-p255:6

[PATCH] D65956: clang: Diag running out of file handles while looking for files

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/D65956/new/ https://reviews.llvm.org/D65956 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D65838: [Driver] Use enumeration for quoting mode. NFC

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

[PATCH] D64793: [Driver] Properly use values-X[ca].o, values-xpg[46].o on Solaris

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm If I interpret @jyknight correctly, having failed to dissuade you, he doesn't feel strongly enough about this to block it, he just wants to reduce legacy when feasible. Repository: rC Clan

[PATCH] D65582: IR: accept and print numbered %N names for function args

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: llvm/utils/add_argument_names.py:5 +def fix_string(s): +TYPE = re.compile('\s*(i[0-9]+|float|double|x86_fp80|fp128|ppc_fp128|\[\[.*?\]\]|\[2 x \[\[[A-Z_0-9]+

[PATCH] D65562: Move LangStandard*, InputKind::Language to Basic

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks good either way. Comment at: include/clang/Basic/LangStandard.h:19 +/// standard and possible actions. +enum Language { + Unknown, ro wrote: > rnk wrote: >

[PATCH] D65668: [Driver][test] Avoid undefined grep in darwin-ld.c

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Probably like most programmers, I am not familiar with section 9.3.2 of posix. I assume this is affecting a real platform that you care about, but I'm curious what it is. lgtm Repository: rC Cl

<    5   6   7   8   9   10   11   12   13   14   >