[PATCH] D17444: [MSVC] Recognize "static_assert" keyword in C mode

rnk added a comment. In D17444#1397179 , @STL_MSFT wrote: > @rnk I've forwarded this to the compiler front-end and Universal CRT teams. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D17444/new/ htt

[PATCH] D58157: Stop enabling clang-tools-extra automatically when clang is in LLVM_ENABLE_PROJECTS

rnk added a comment. In D58157#1397274 , @mehdi_amini wrote: > It'd still be nice to identify the end goal with most cfe people (i.e. even > if you land this right now, discussing the desired layout in the monorepo) I sent off http://lists.llvm.org/pipe

[PATCH] D17444: [MSVC] Recognize "static_assert" keyword in C mode

This revision was automatically updated to reflect the committed changes. Closed by commit rC354162: [MSVC] Recognize `static_assert` keyword in C and C++98 (authored by rnk, committed by ). Changed prior to commit: https://reviews.llvm.org/D17444?vs=186741&id=187061#toc Repository: rC Clang

[PATCH] D37122: Change Diagnostic Category size error from runtime to compiletime

rnk added inline comments. Comment at: include/clang/Basic/AllDiagnostics.h:37 +#define STRINGIFY_NAME(NAME) #NAME +#define VALIDATE_DIAG_SIZE(NAME) \ + static_assert( \

[PATCH] D37122: Change Diagnostic Category size error from runtime to compiletime

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

[PATCH] D26764: [compiler-rt] [cmake] Remove i686 target that is duplicate to i386

rnk added a comment. In https://reviews.llvm.org/D26764#855791, @eugenis wrote: > IMHO it was a very bad idea to include the "architecture name" (not even a > target triple!) in the library path in the first place. No one else does > that. GCC made the right choice and called theirs "libasan.so

[PATCH] D37324: Suppress -Wdelete-non-virtual-dtor warnings _about_ classes defined in system headers.

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

[PATCH] D37448: Fix cast assertion on MS inline assembly with vector spills (PR34021)

rnk added a comment. I think this is a reasonable stop-gap fix since the code isn't trying to return EAX:EDX or XMM0 from the inline asm blob. This affects any function that contains inline asm and returns a vector, which is potentially a lot of stuff. Comment at: test/CodeGe

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

rnk added a subscriber: echristo. rnk added inline comments. Comment at: clang/lib/CodeGen/CGStmt.cpp:38 +InhibitDebugLocation CodeGenFunction::EmitStmtStopPoint(const Stmt *S) { + InhibitDebugLocation IDL; "Stop point" is a hold-over from the llvm.dbg.stoppoi

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

rnk added inline comments. Comment at: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp:12 + +// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]] +// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]] inglorion wrote: > rnk wrote: > > This is pretty painful

[PATCH] D37530: [MinGW] Allow overriding which version of msvcrt to link to

rnk added a comment. What do you think of letting people spell this as `-lmsvcrt120`? We could forward those options and suppress our implicit addition of `-lmsvcrt` if we see `-lmsvcr*` anywhere. Comment at: lib/Driver/ToolChains/MinGW.cpp:161 if (TC.getArch() == llvm

[PATCH] D37448: Fix cast assertion on MS inline assembly with vector spills (PR34021)

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm I was hoping for a test case that didn't require assertions, but this is enough to land the fix. Repository: rL LLVM https://reviews.llvm.org/D37448

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang/lib/CodeGen/CodeGenModule.h:517 + /// Return true if we should emit location information for nested expressions. + bool getNestedExpressionLocationsEnable

[PATCH] D37530: [MinGW] Don't link -lmsvcrt if a different msvcrt version is to be linked

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

[PATCH] D37530: [MinGW] Don't link -lmsvcrt if a different msvcrt version is to be linked

rnk added a comment. In https://reviews.llvm.org/D37530#865333, @mstorsjo wrote: > Attempted implementing Reid's suggestion. > > On the mingw-w64 mailing list > (https://sourceforge.net/p/mingw-w64/mailman/message/36030072/), there were > points made that it would be better with some mechanism

[PATCH] D37529: [codeview] omit debug locations for nested exprs unless column info enabled

rnk added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:32 #include "clang/Basic/XRayLists.h" +#include "clang/Frontend/CodeGenOptions.h" #include "llvm/ADT/DenseMap.h" majnemer wrote: > Any reason to do this? I'd just keep getNestedExpressionL

[PATCH] D37727: Driver: Remove custom MinGW linker detection

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Nice! IIUC, now that the gnu ld driver works on Windows we can remove this logic. Thanks! Repository: rL LLVM https://reviews.llvm.org/D37727 ___ c

[PATCH] D37413: [X86][MS-InlineAsm] Extended support for variables / identifiers on memory / immediate expressions

rnk added inline comments. Comment at: lib/Sema/SemaStmtAsm.cpp:617 +return; + } else if (Res->isRValue()) { +bool Enum = isa(T) && Res->EvaluateAsRValue(Eval, Context); RKSimon wrote: > (style) Split these instead of an if-elseif chain LLVM suggests ea

[PATCH] D37787: Driver: Make -fwhole-program-vtables a core option so it can be used from clang-cl.

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

[PATCH] D37818: [lit] Update clang and lld to use the new shared LLVMConfig stuff

rnk added inline comments. Comment at: llvm/utils/lit/lit/llvm/config.py:92 def with_environment(self, variable, value, append_path = False): +if append_path: Rather than having an optional parameter that makes this append, maybe have a new method

[PATCH] D37089: [Sema] Error out early for tags defined inside an enumeration.

rnk added a comment. I think this looks good, even without fixing the access control crash, this seems like a diagnostic improvement. Comment at: clang/test/Sema/enum.c:128 +// PR28903 +struct PR28903 { // expected-warning {{empty struct is a GNU extension}} + enum {

[PATCH] D37308: Fix the __interface inheritence rules to work better with IUnknown and IDispatch

rnk added inline comments. Comment at: lib/AST/DeclCXX.cpp:1474 +bool CXXRecordDecl::isInterfaceLike() const { + // All __interfaces are inheritently interface like. + if (isInterface()) nit: use "interface-like" for consistency with the comments below ==

[PATCH] D37308: Fix the __interface inheritence rules to work better with IUnknown and IDispatch

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

[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver

rnk added a comment. I do remember recommending this approach over IRC, but I thought we concluded that we should leave all the defaults in lib/Basic/Targets/ and make the -cc1 -fwchar-type= and -f[no-]signed-wchar overrides that affected all targets equally. That would avoid the need for these

[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver

rnk added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:477 Context.getTypeSizeInChars(Context.getWideCharType()).getQuantity(); - assert((LangOpts.ShortWChar || - llvm::TargetLibraryInfoImpl::getTargetWCharSize(Target.getTriple()) == --

[PATCH] D32049: [Preprocessor] Implement empty vs omitted __VA_ARGS__ string-expansion MSVC Extension

rnk added a comment. Would this help with https://bugs.llvm.org/show_bug.cgi?id=32021 ? https://reviews.llvm.org/D32049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37865: [Sema] Fix a pair of crashes when generating exception specifiers with an error'ed field for a template class' default ctor.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks like the correct fix, thanks! https://reviews.llvm.org/D37865 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-b

[PATCH] D37413: [X86][MS-InlineAsm] Extended support for variables / identifiers on memory / immediate expressions

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! Repository: rL LLVM https://reviews.llvm.org/D37413 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D37466: D37461: fixups for existing InlineAsm tests + adding new ones

rnk added inline comments. Comment at: test/CodeGen/ms-inline-asm.cpp:37-38 - int lvar = 10; - __asm mov eax, offset Foo::ptr - __asm mov eax, offset Foo::Bar::ptr -// CHECK-LABEL: define void @_Z2t2v() These don't seem tested anywhere now Repository: rL L

[PATCH] D38092: [MS Compat]Allow __interfaces to have properties.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: lib/Sema/SemaDeclCXX.cpp:2871 + InvalidDecl = + (DS.getStorageClassSpec() == DeclSpec::SCS_typedef || MSPropertyAttr) + ? 0

[PATCH] D38109: [fixup][Sema] Allow in C to define tags inside enumerations.

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

[PATCH] D62635: Add enums as global variables in the IR metadata.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm That was... easier and more painless than I would've imagined. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62635/new/ https://reviews.llvm.org

[PATCH] D62746: msabi: Fix exponential mangling time for certain pathological inputs

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Thanks for this, I remember a user complained about this a long time ago but we never did anything. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62746/new/ https://reviews.llvm.org/

[PATCH] D62780: msabi: Fix exponential mangling time for even more contrived inputs

rnk added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:819-823 ArgBackRefMap::iterator Found = TemplateArgBackReferences.find(ND); if (Found == TemplateArgBackReferences.end()) { - // Mangle full template name into temporary buffer. - llvm::S

[PATCH] D62780: msabi: Fix exponential mangling time for even more contrived inputs

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang/lib/AST/MicrosoftMangle.cpp:819-823 ArgBackRefMap::iterator Found = TemplateArgBackReferences.find(ND); if (Found == TemplateArgBackReferences.end(

[PATCH] D62975: Require stdcall etc parameters to be complete on ODR use

rnk created this revision. rnk added reviewers: thakis, rsmith, hans. Herald added a project: clang. Functions using stdcall, fastcall, or vectorcall with C linkage mangle in the size of the parameter pack. Calculating the size of the pack requires the parameter types to complete, which may requir

[PATCH] D62975: Require stdcall etc parameters to be complete on ODR use

rnk marked 2 inline comments as done. rnk added a comment. In D62975#1533019 , @inglorion wrote: > Can you clarify "which will usually result in a linker error"? E.g. an > example of link.exe rejecting the object file or the wrong function being > called

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

rnk added a subscriber: dexonsmith. rnk added a comment. Personally, I think it's safe to do this. I believe this "4.2.1" compat thing was something Apple added as part of their switch from GCC, and then llvm-gcc, over to Clang, and I think most of the code they care about probably doesn't rely

[PATCH] D62975: Require stdcall etc parameters to be complete on ODR use

This revision was automatically updated to reflect the committed changes. Closed by commit rL363000: Require stdcall etc parameters to be complete on ODR use (authored by rnk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https:/

[PATCH] D63012: Use fully qualified name when printing S_CONSTANT records

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

[PATCH] D63175: [MS] Pretend constexpr variable template specializations are inline

rnk created this revision. rnk added a reviewer: rsmith. Herald added a project: clang. Fixes link errors with clang and the latest Visual C++ 14.21.27702 headers, which was reported as PR42027. I chose to intentionally make these things linkonce_odr, i.e. discardable, so that we don't emit defin

[PATCH] D63175: [MS] Pretend constexpr variable template specializations are inline

This revision was automatically updated to reflect the committed changes. Closed by commit rL363191: [MS] Pretend constexpr variable template specializations are inline (authored by rnk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit

[PATCH] D63175: [MS] Pretend constexpr variable template specializations are inline

rnk marked an inline comment as done. rnk added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:9809 + // variable template specializations inline. + if (isa(VD) && VD->isConstexpr()) +return GVA_DiscardableODR; rsmith wrote: > It'd be

[PATCH] D61790: [C++20] add Basic consteval specifier

rnk added inline comments. Comment at: cfe/trunk/include/clang/Sema/DeclSpec.h:366 // constexpr-specifier - unsigned Constexpr_specified : 1; + ConstexprSpecKind ConstexprSpecifier : 2; Please always use `unsigned` for bitfields to avoid sign extensions un

[PATCH] D63361: Pretend NRVO variables are references so they can be found by debug info

rnk added a comment. +@aprantl @dblaikie The basic idea of this change is that clang should store implicit sret pointer arguments into a static alloca, and the dbg.declare should reference the static alloca instead of referring to the LLVM argument directly. The idea is that that will be more

[PATCH] D62635: Add enums as global variables in the IR metadata.

rnk added a comment. We did things this way to track which **enumerators** were used, not which enums were used. Size data showed it was worth doing (6%). The existing format doesn't support tracking usage of individual enumerators, so we pretended they were const integers to avoid changing the

[PATCH] D62635: Add enums as global variables in the IR metadata.

rnk added a comment. In D62635#1548648 , @dblaikie wrote: > In D62635#1548157 , @rnk wrote: > > > We did things this way to track which **enumerators** were used, not which > > enums were used. Size data showed it

[PATCH] D63361: Pretend NRVO variables are references so they can be found by debug info

rnk added a comment. This needs two unit tests: - A clang test at `clang/test/CodeGenCXX/debug-info-nrvo.cpp` similar to other debug-info-* tests there. This test should have a second RUN line and extra checks for -fno-elide-constructors. - An LLVM test at `llvm/test/DebugInfo/COFF/nrvo.ll` to

[PATCH] D63361: Pretend NRVO variables are references so they can be found by debug info

rnk added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3946 + // the address of the variable. + if (VD->isNRVOVariable()) +Expr.push_back(llvm::dwarf::DW_OP_deref); akhuang wrote: > rnk wrote: > > I think we should check for `getLangOpts()

[PATCH] D63361: Pretend NRVO variables are references so they can be found by debug info

rnk added a comment. I see we don't have any tests for inalloca to model this on, so I think we should skip that for this change. I'll add one later that handles arguments as well, since those are interesting. Comment at: clang/lib/CodeGen/CGDecl.cpp:1564 +Address DebugAd

[PATCH] D63361: Pretend NRVO variables are references so they can be found by debug info

rnk accepted this revision. rnk added a comment. lgtm, thanks! Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:908 --EI; llvm::Value *Addr = Builder.CreateStructGEP(nullptr, &*EI, Idx); Addr = Builder.CreateAlignedLoad(Addr, getPointerAlign(), "agg.result"); --

[PATCH] D63617: [COFF, ARM64] Fix encoding of __debugbreak

rnk added a comment. I think it would be preferable to make `llvm.debugtrap` emit `brk #0xF000` on aarch64-windows-*, so other frontends (Rust etc) get the right behavior by default. Right now, AArch64 doesn't do anything special for DEBUGTRAP, so we get the default behavior of `llvm.trap`. Is

[PATCH] D63617: [COFF, ARM64] Fix encoding of __debugbreak

rnk added a comment. Even if `BRK #0xF000` is a Windows convention, it's still possible for ISel to select different instructions for different OSs, and I'd prefer to implement it that way. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63617/new/ https://review

[PATCH] D64062: Remove both -dumpversion and __VERSION__

rnk added a comment. I still have this open concern: In D64062#1574868 , @rnk wrote: > One concern that I have is that we don't have a replacement for > `-dumpversion`. I thought I saw someone propose adding such a flag, but I > can't find it. Basically

[PATCH] D64062: Remove both -dumpversion and __VERSION__

rnk added a comment. In D64062#1581662 , @sylvestre.ledru wrote: > In D64062#1581290 , @rnk wrote: > > > Perhaps we should just remove `__VERSION__` and keep `-dumpversion`. > > > As you wish. clang --version seems

[PATCH] D64277: [X86][PowerPC] Support -mlong-double-128

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm with suggestion Comment at: lib/Driver/ToolChains/Clang.cpp:4009 TC.getArch() == llvm::Triple::ppc || TC.getTriple().isPPC64()) { - CmdArgs.push_back("-mlong-dou

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

rnk added a comment. I think the intention of this change was to ignore assembler flags in pre-processing actions without warning about them. It implements that behavior by running the code that gathers and validates assembler flags. The validation is whats emitting the problematic errors. I do

[PATCH] D64062: Remove __VERSION__

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, minor changes Comment at: docs/ReleaseNotes.rst:82 +Removed Compiler Options + I guess move the bullet to the generic "list of chan

[PATCH] D64655: [Clang][Driver] don't error for unsupported as options for -no-integrated-as

rnk added a comment. I don't think this is the right fix, because now I think we get this behavior that we don't want: $ clang -E t.c -o t.i -Wa,-mbig-obj # no warning $ clang -E t.c -o t.i -Wa,-mbig-obj -fno-integrated-as clang: warning: argument unused during compilation: '-Wa,-mbig-ob

[PATCH] D64655: [Clang][Driver] don't error for unsupported as options for -no-integrated-as

rnk added a comment. I reverted @thakis's change in rC365956 , but I think in retrospect I should've just stamped your change. I was glazing over the "Integrated" part of "CollectArgsForIntegratedAssembler". Anyway, I think we should leave it alone and let him

[PATCH] D64610: [clang] allow -fthinlto-index= without -x ir

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

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

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

[PATCH] D64062: Remove __VERSION__

rnk added a comment. In D64062#1584219 , @saugustine wrote: > This revision breaks python 2.7.16 builds, which are still supported by > upstream python for a few more months. I'm preparing a revert. > > The file is getcompiler.c: I just want to point ou

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

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm too, since now this is just whitelisting a clang flag for clang-cl. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64458/new/ https://reviews.llvm

[PATCH] D64482: [Driver] Define _FILE_OFFSET_BITS=64 on Solaris

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. > To avoid a similar inconsistence with host compilers that don't predefine > _FILE_OFFSET_BITS=64 > (e.g. clang < 9, gcc < 9), this needs a compantion patch to be submitted > shortly. I'm curious

[PATCH] D64780: Disallow most calling convention attributes on PS4.

rnk added a comment. Some suggestions, you don't have to take all of them. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2961 +def error_cconv_unsupported : Error< + "%0 calling convention not supported %select{" + // Use CallingConventionIgnoredReason Enum to specif

[PATCH] D64491: [Driver] Enable __cxa_atexit on Solaris

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

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

rnk added a comment. I'm in favor of this as is. We should loop in and get confirmation from @rsmith though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64799/new/ https://reviews.llvm.org/D64799 __

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

rnk added a comment. In D64067#1592201 , @troyj wrote: > Hi, we just inherited this commit at Cray when we did our latest upstream > merge and there are a few problems with it that I'd like to point out. Sorry > that I was not part of the initial discus

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

rnk added a comment. I suggested to @akhuang offline that we should discuss this on llvm-dev. I'll add some other X86 reviewers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64931/new/ https://reviews.llvm.org/D64931 __

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

rnk added a comment. In D64799#1591732 , @ilya-biryukov wrote: > @rsmith, I'll look into emitting the typos when we pop expression evaluation > context, but do we expect this to cover **all** cases where `TypoExpr`s are > produced? > (conservatively as

[PATCH] D64780: Disallow most calling convention attributes on PS4.

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

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

rnk added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3564 +ArgStringList DummyArgs; +CollectArgsForIntegratedAssembler(C, Args, DummyArgs, D, + TC.useIntegratedAs()); I think it would be better

[PATCH] D65249: [NFC] use C++11 in AlignOf.h

rnk added a comment. I still think this concern applies: https://reviews.llvm.org/D64417#1576675 I'm not sure how to track down an 19.11 release to test if we can pass std::aligned_storage values through function calls on x86. We might be better off raising the minimum to 19.14, which is a more

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

rnk added a comment. In D64793#1597765 , @ro wrote: > In D64793#1597550 , @jyknight wrote: > > > How about instead just adding "values-xpg6.o" unconditionally, alongside > > the current unconditional "values-Xa.o",

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

rnk added a comment. In D63048#1605050 , @xbolva00 wrote: > Should we bump __GNUC__, __GNUC_MINOR__ too? We discussed this two weeks ago: http://lists.llvm.org/pipermail/cfe-dev/2019-July/062890.html It's unlikely that we will make any policy change befo

[PATCH] D65249: [NFC] use C++11 in AlignOf.h, remove AlignedCharArray

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. I have concerns that some of the patches that you landed prior to this will cause issues with old versions of MSVC, but in isolation, this is fine, and if anyone complains, we can address it on a ca

[PATCH] D65426: [Coverage] Hide coverage for regions with incorrect end locations (PR39942)

rnk added a comment. I did some digging and I figured out where things go wrong. The issue is the repeated string-izing of the macro argument, the repetition of `#func` here. This hacky patch seems to fix the issue: diff --git a/clang/lib/Lex/MacroArgs.cpp b/clang/lib/Lex/MacroArgs.cpp inde

[PATCH] D65428: Remove cache for macro arg stringization

rnk created this revision. rnk added reviewers: vsk, rsmith. Herald added a project: clang. The cache recorded the wrong expansion location for all but the first stringization. It seems uncommon to stringize the same macro argument multiple times, so this cache doesn't seem that important. Fixes

[PATCH] D65426: [Coverage] Hide coverage for regions with incorrect end locations (PR39942)

rnk added a comment. PTAL: https://reviews.llvm.org/D65428 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65426/new/ https://reviews.llvm.org/D65426 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailm

[PATCH] D65458: [NFC] Remove LLVM_ALIGNAS

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

[PATCH] D65428: Remove cache for macro arg stringization

rnk added a comment. Thanks for looking into it as well, I would not have seen the issue without reviewing the test case again. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65428/new/ https://reviews.llvm.org/D65428

[PATCH] D65428: Remove cache for macro arg stringization

This revision was automatically updated to reflect the committed changes. Closed by commit rL367337: Remove cache for macro arg stringization (authored by rnk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.or

[PATCH] D65511: Delay emitting dllexport explicitly defaulted members until the class is fully parsed (PR40006)

rnk added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:11545 +for (CXXMethodDecl *M : WorkList) { + DefineImplicitSpecialMember(*this, M, M->getLocation()); + ActOnFinishInlineFunctionDef(M); Do we need to do this until fixpoint? Supp

[PATCH] D65511: Delay emitting dllexport explicitly defaulted members until the class is fully parsed (PR40006)

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang/lib/Sema/SemaDeclCXX.cpp:11545 +for (CXXMethodDecl *M : WorkList) { + DefineImplicitSpecialMember(*this, M, M->getLocation()); + ActOnFinishInl

[PATCH] D65543: Use library basenames when autolinking on Windows

rnk created this revision. rnk added reviewers: russell.gallop, thakis, pcc. Herald added a subscriber: srhines. Herald added a project: clang. Otherwise we embed a potentially absolute and potentially relative path to clang's resource directory into the object file. Object files with paths embedd

[PATCH] D65579: Don't try emitting dllexported explicitly defaulted non-trivial ctors twice during explicit template instantiation definition (PR42857)

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm yeesh. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65579/new/ https://reviews.llvm.org/D65579 ___ cfe-commits mailing list cfe-commits

[PATCH] D65543: Use library basenames when autolinking on Windows

rnk added a comment. I'll look into that. I also noticed that check-ubsan fails. I think we should also change clang's driver to add this libpath when it invokes the linker, so that this works transparently when using the GCC-style driver. Repository: rG LLVM Github Monorepo CHANGES SINCE L

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

rnk added a comment. This is awesome! Thanks for doing the test updates. As I read it, LLVM will still read old-style IR just fine, which seems nice. I think it would be worth noting this in the 10.0 release notes, since it will affect someone upgrading a frontend as you mention. Otherwise, I

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

rnk added a comment. Thanks! Comment at: include/clang/Basic/LangStandard.h:19 +/// standard and possible actions. +enum Language { + Unknown, Is it feasible to make this an `enum class`? I'm worried about namespace clashes on these otherwise very short names

[PATCH] D61027: Fix crash on switch conditions of non-integer types in templates

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks good to me. I was hoping Richard would take a look, but I reproduced the crash, and I'd rather see the fix land sooner than later. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org

[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

[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] 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] 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] 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] 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] 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] 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] 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.

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