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

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] 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] 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] 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] 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] 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] 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] 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] 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 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] D36610: [Tooling] Add option to getFullyQualifiedName using a custom PritingPolicy

rnk accepted this revision. rnk added a comment. Mostly looks good. Comment at: unittests/Tooling/QualTypeNamesTest.cpp:225 + + TypeNameVisitor PrintingPolicy; + PrintingPolicy.ExpectedQualTypeNames["a"] = "short"; Please choose a different variable name that

[PATCH] D45504: [MinGW] Look for a cross sysroot relative to the clang binary

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

[PATCH] D45504: [MinGW] Look for a cross sysroot relative to the clang binary

rnk added a comment. Sorry, I skipped over the message and looked at the code, which seems pretty straightforward. > Tests still are TBD, but posting this early to see if there's comments. (How > do I easily do a test that checks something relative to the clang binary, > since I don't control

[PATCH] D45829: Remove impossible _MSC_VER check

rnk created this revision. rnk added a reviewer: EricWF. Herald added a subscriber: christof. It is immediately preceded by this check: #if _MSC_VER < 1900 #error "MSVC versions prior to Visual Studio 2015 are not supported" #endif https://reviews.llvm.org/D45829 Files: libcxx/include/

[PATCH] D45829: Remove impossible _MSC_VER check

This revision was automatically updated to reflect the committed changes. Closed by commit rL330360: Remove impossible _MSC_VER check (authored by rnk, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D45829?vs=143137&id=143151#toc Repo

[PATCH] D45836: Don't do aligned allocations on MSVCRT before 19.12 (update 15.3)

rnk created this revision. rnk added reviewers: EricWF, pcc. Herald added a subscriber: christof. https://reviews.llvm.org/D45836 Files: libcxx/include/__config Index: libcxx/include/__config === --- libcxx/include/__config +++ l

[PATCH] D45836: Don't do aligned allocations on MSVCRT before 19.12 (update 15.3)

This revision was automatically updated to reflect the committed changes. Closed by commit rCXX330372: Don't do aligned allocations on MSVCRT before 19.12 (update 15.3) (authored by rnk, committed by ). Changed prior to commit: https://reviews.llvm.org/D45836?vs=143161&id=143173#toc Repository

[PATCH] D45985: [test] Add a testcase for MinGW sysroot detections from SVN r330244. NFC.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: test/Driver/mingw-sysroot.cpp:1-2 +// REQUIRES: shell +// UNSUPPORTED: system-windows + Hah. :) Repository: rC Clang https://reviews.llvm.org

[PATCH] D46131: Add Microsoft Mangling for OpenCL Half Type

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

[PATCH] D46132: [X86] Make __builtin_ia32_readeflags_u32 and __builtin_ia32_writeeflags_u32 only available on 32-bit targets.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks good. Repository: rC Clang https://reviews.llvm.org/D46132 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-b

[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] 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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 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] 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] 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] 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] 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: [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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] D76184: [OpenMP][NFC] Remove the need to include `OpenMPClause.h`

rnk updated this revision to Diff 254889. rnk added a comment. Herald added subscribers: usaxena95, kadircet, arphaman, jkorous. - seems to build all.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76184/new/ https://reviews.llvm.org/D76184 Files:

[PATCH] D76184: [OpenMP][NFC] Remove the need to include `OpenMPClause.h`

rnk commandeered this revision. rnk edited reviewers, added: jdoerfert; removed: rnk. rnk added a comment. This revision now requires review to proceed. I got it building... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76184/new/ https://reviews.l

[PATCH] D76184: [OpenMP][NFC] Remove the need to include `OpenMPClause.h`

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGba1ffd25c1f7: [OpenMP][NFC] Remove the need to include `OpenMPClause.h` (authored by rnk). Changed prior to commit: htt

[PATCH] D77432: [DebugInfo] Change to constructor homing debug info mode: skip literal types

rnk added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2263 if (DebugKind == codegenoptions::DebugInfoConstructor && - !CXXDecl->isLambda() && !isClassOrMethodDLLImport(CXXDecl)) { -for (const auto *Ctor : CXXDecl->ctors()) { + !CXXDecl->isLambd

[PATCH] D77436: [DebugInfo] Fix for adding "returns cxx udt" option to functions in CodeView.

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

[PATCH] D77432: [DebugInfo] Change to constructor homing debug info mode: skip constexpr constructed types

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

[PATCH] D77520: Treat default values in LangOptions.def in the scope of enums

rnk requested changes to this revision. rnk added a comment. This revision now requires changes to proceed. I don't think we can do this because GCC 5.1 won't support it. To confirm, that's still the minimum supported version: https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compil

[PATCH] D73307: Unique Names for Functions with Internal Linkage

rnk added a comment. Sorry for the delay, thanks for the update. Code looks good, but I want the documentation to be specific. Comment at: clang/docs/UsersManual.rst:1684 + linkage symbols. The unique name is obtained by appending the hash of the + full module name to the

[PATCH] D77081: [MS] Mark vbase dtors ref'd when ref'ing dtor

rnk added a comment. ptal Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77081/new/ https://reviews.llvm.org/D77081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin

[PATCH] D77754: [MS] Fix packed struct layout for arrays of aligned non-record types

rnk created this revision. rnk added a reviewer: rjmccall. Herald added a subscriber: jfb. Herald added a project: clang. In particular, this affects Clang's vectors. Users encounter this issue when a struct contains an __m128 type. Fixes PR45420 Repository: rG LLVM Github Monorepo https://r

[PATCH] D77249: [MSan] Pass command line options to MSan with new pass manager

rnk added inline comments. Comment at: compiler-rt/test/msan/chained_origin_empty_stack_npm.cpp:4 +// this test. +// RUN: %clangxx_msan -fsanitize-memory-track-origins=2 \ +// RUN: -fexperimental-new-pass-manager -O3 %s -o %t && \ leonardchan wrote: > nemanja

[PATCH] D73307: Unique Names for Functions with Internal Linkage

rnk added a comment. Regarding the alias attribute, it occurs to me that reimplementing this as an early LLVM pass would not have that problem. Do you think that would be worth doing? Comment at: clang/docs/UsersManual.rst:1684 + linkage symbols. The unique name is obtained

[PATCH] D77081: [MS] Mark vbase dtors ref'd when ref'ing dtor

rnk added a comment. I'm going to go ahead and push this today. Richard hasn't stamped it, but I did incorporate the feedback, and I'm fairly happy with the results and confident that I addressed the feedback adquately. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D77081: [MS] Mark vbase dtors ref'd when ref'ing dtor

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG55efb68c19b4: [MS] Mark vbase dtors used when marking dtor used (authored by rnk). Changed prior to commit: https://rev

[PATCH] D75903: [AArch64][CodeGen] Fixing stack alignment of HFA arguments on AArch64 PCS

rnk added a comment. In D75903#1963382 , @pratlucas wrote: > In regards to the compatibility with other compilers, I'm not sure that > following what seems to be an uncompliant behavior would be the best way to > proceed. @rnk and @ostannard, what would

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

rnk added a comment. I like the implementation strategy, but I have one corner case, which is probably worth a test. Comment at: clang/lib/CodeGen/CGCleanup.cpp:848 // Build a switch-out if we need it: // - if there are branch-afters threaded through the scop

[PATCH] D77962: PR38490: Support reading values out of __uuidof(X) expressions during constant evaluation.

rnk added a comment. Thanks for looking into this. This code is very string-y. Should clang be parsing uuids into u32-u16-u16-u8[8] earlier, so that we can get the semantic form directly from the UuidAttr? Based on the test cases you had to pass, is there any reason that would be difficult?

[PATCH] D78171: Rework how UuidAttr, CXXUuidofExpr, and GUID template arguments and constants are represented.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm There's a lot of stuff going on here, and I didn't review that deeply. Let me know if you want a closer reading of the code. Comment at: clang/include/clang/AST/TemplateBase

[PATCH] D77754: [MS] Fix packed struct layout for arrays of aligned non-record types

This revision was automatically updated to reflect the committed changes. Closed by commit rG8da5b9083691: [MS] Fix packed struct layout for arrays of aligned non-record types (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77754/new

[PATCH] D78273: [clang-tools-extra] reimplement PreprocessorTracker in terms of StringSet.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang-tools-extra/modularize/PreprocessorTracker.cpp:466 -bool operator<(const StringHandle &H1, const StringHandle &H2) { - const char *S1 = (H1 ? *H1 : ""); -

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

rnk added subscribers: tstellar, hans, rnk. rnk added a comment. We ran into three distinct issues with this change. We updated the compiler and break some VS std::map visualizers: https://crbug.com/1068394 I haven't 100% confirmed that this caused the issue, but looking at the code suggests tha

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

rnk added a subscriber: amccarth. rnk added a comment. In D76801#1989641 , @sammccall wrote: > I've got a sinking feeling that I misunderstood the implications of updating > CodeGenCXX/debug-info-template-explicit-specialization.cpp and the > Modules/*De

[PATCH] D73307: Unique Names for Functions with Internal Linkage

rnk added a subscriber: erichkeane. rnk added a comment. In D73307#1978140 , @tmsriram wrote: > In D73307#1972388 , @rnk wrote: > > > Regarding the alias attribute, it occurs to me that reimplementing this as > > a

[PATCH] D78573: [Clang][Sema] Capturing section type conflicts between #pragma clang section and section attributes

rnk added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:3008 +/// Insertion operator for diagnostics. +inline const DiagnosticBuilder & +operator<<(const DiagnosticBuilder &DB, It seems like there is no need for this to be defined inline, sinc

[PATCH] D78572: [Clang][Sema] Capturing section type conflicts on #pragma clang section

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Seems reasonable. Comment at: clang/test/Sema/pragma-clang-section.c:2 // RUN: %clang_cc1 -fsyntax-only -verify %s -triple arm-none-eabi -#pragma clang section bss="mybss.1" data=

[PATCH] D75068: libclang: Add static build support for Windows

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, assuming my understanding is correct. Comment at: clang/tools/libclang/CMakeLists.txt:88 +if (WIN32 AND ENABLE_SHARED AND ENABLE_STATIC) + unset(ENABLE_STATIC) ---

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

rnk added a comment. Thanks David, I'm comfortable with that stance for DWARF. I know @amccarth is looking into the recent VS visualizer issue, and I would like him to confirm if it was this change or not when he gets a chance. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

rnk updated this revision to Diff 259751. rnk added a comment. - Lots of documentation words Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65543/new/ https://reviews.llvm.org/D65543 Files: clang/docs/ReleaseNotes.rst clang/docs/UsersManual.rst

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

rnk added a comment. In D78659#2003152 , @zequanwu wrote: > Add check in LICM to prevent sink or hoist on `nomerge` call Does sinking and hoisting remove the debug source location? I assumed that it wouldn't, but now after all the smooth stepping work,

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGb8000c0ce845: [Windows] Autolink with basenames and add libdir to libpath (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65543/new/ https:

[PATCH] D79076: [clang] [MinGW] Add the compiler rt libdirs to the search path

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

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

rnk added a comment. This looks pretty close, let's do one more iteration. Comment at: llvm/include/llvm/IR/InstrTypes.h:1720 + /// Determine if the invoke cannot be tail merged. + bool cannotMerge() const { return hasFnAttr(Attribute::NoMerge); } Let's say

[PATCH] D79121: Add nomerge function attribute to clang

rnk added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1799 + let Spellings = [Clang<"nomerge">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [NoMergeDocs]; Clang supports many function-like things that are not functions,

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

rnk added a comment. I think this looks good, but I would like to get a second opinion from Alina (@asbirlea). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78659/new/ https://reviews.llvm.org/D78659 ___ cfe-commits mailing list cfe-commit

[PATCH] D79121: Add nomerge function attribute to clang

rnk added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1799 + let Spellings = [Clang<"nomerge">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [NoMergeDocs]; rnk wrote: > aaron.ballman wrote: > > zequanwu wrote: > > > rnk w

[PATCH] D79121: Add nomerge function attribute to clang

rnk added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1799 + let Spellings = [Clang<"nomerge">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [NoMergeDocs]; aaron.ballman wrote: > zequanwu wrote: > > rnk wrote: > > > Clang

[PATCH] D79274: Fix template class debug info for Visual Studio visualizers

rnk added a comment. I think testing it is a matter of adding later C++ standard versions to the existing test here: `../clang/test/CodeGenCXX/debug-info-codeview-display-name.cpp` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79274/new/ https://reviews.llvm.org/D79274

[PATCH] D79121: Add nomerge function attribute to clang

rnk added inline comments. Comment at: clang/lib/CodeGen/CGStmt.cpp:611 void CodeGenFunction::EmitAttributedStmt(const AttributedStmt &S) { + for (const auto *A: S.getAttrs()) +if (A->getKind() == attr::NoMerge) { Can we use S.hasAttr, or does that not exis

[PATCH] D78573: [Clang][Sema] Capturing section type conflicts between #pragma clang section and section attributes

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang/include/clang/AST/ASTContext.h:3008 +/// Insertion operator for diagnostics. +inline const DiagnosticBuilder & +operator<<(const DiagnosticBuilder &DB, -

[PATCH] D73307: Unique Names for Functions with Internal Linkage

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Sorry for the delay. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 ___ cfe-commits mailing li

[PATCH] D79274: Fix template class debug info for Visual Studio visualizers

rnk added inline comments. Comment at: clang/test/CodeGenCXX/debug-info-codeview-display-name.cpp:10 +// RUN: %clang_cc1 -fblocks -debug-info-kind=limited -gcodeview -emit-llvm %s \ +// RUN: -o - -triple=x86_64-pc-win32 -Wno-new-returns-null -fms-compatibility | \ +// RUN:

[PATCH] D79531: Make -Wnonportable-include-path ignore drive case on Windows.

rnk added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:2118 +// given drive letter case as correct for the purpose of this warning. +SmallString<128> FixedDriveRealPath; +if (llvm::sys::path::is_absolute(Name) && It seems like trySimpli

[PATCH] D79673: Allow 32-bit pointer extensions to be used without -fms-extensions

rnk added a comment. Needs a test. I believe these are only implemented for x86 in LLVM. What happens if you try to use this on non-x86? I wouldn't be surprised if we crash, but we should probably produce a proper error and test it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D79636: [LangRef] Clarify the semantics of the `byval` attribute

rnk added a comment. Some copy editing comments, but I agree with the semantics: From the IR perspective, it is better to think of argument stack memory as belonging to the callee. A byval argument has more in common with a local static alloca than a passed in pointer. Comme

[PATCH] D79274: Fix template class debug info for Visual Studio visualizers

rnk added inline comments. Comment at: clang/test/CodeGenCXX/debug-info-codeview-display-name.cpp:10 +// RUN: %clang_cc1 -fblocks -debug-info-kind=limited -gcodeview -emit-llvm %s \ +// RUN: -o - -triple=x86_64-pc-win32 -Wno-new-returns-null -fms-compatibility | \ +// RUN:

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

This revision was automatically updated to reflect the committed changes. Closed by commit rGcb22ab740355: Add nomerge function attribute to supress tail merge optimization in simplifyCFG (authored by zequanwu, committed by rnk). Changed prior to commit: https://reviews.llvm.org/D78659?vs=26297

[PATCH] D79274: Fix template class debug info for Visual Studio visualizers

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

[PATCH] D79895: Fix warning about using uninitialized variable as function const reference parameter

rnk added a comment. Arthur recently went through the process of refining a diagnostic, so he should be able to help guide you in this if you have more questions. Comment at: clang/lib/Analysis/UninitializedValues.cpp:425 if ((*I)->getType().isConstQualified()) -

[PATCH] D78659: Add nomerge function attribute to supress tail merge optimization in simplifyCFG

rnk added inline comments. Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:1305 +if (const auto *CB1 = dyn_cast(I1)) + if (CB1->cannotMerge()) +return Changed; zequanwu wrote: > rnk wrote: > > It seems inconsistent that we don't apply the s

[PATCH] D79121: Add nomerge function attribute to clang

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. I think this looks good and @zequanwu has addressed the concerns of other reviewers. Please wait until Wednesday before pushing in case they raise other concerns. Comment at: cla

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