[PATCH] D130161: Added command-line options to exclude functions from function instrumentation output.

hans added a comment. Can you share the motivation for this? There's already an attribute to exclude functions from this instrumentation. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:524 /// instrumented with __cyg_profile_func_* calls -bool CodeGenFunction::ShouldInstru

[PATCH] D128927: [libc++] Always build c++experimental.a

hans added a comment. In D128927#3662659 , @ldionne wrote: > The weird part here is that you're configuring libc++, but you are building > neither the static nor the shared library. I don't understand why you do > that, and that may hide some other more

[PATCH] D128927: [libc++] Always build c++experimental.a

hans added a comment. >> In https://github.com/llvm/llvm-zorg/pull/28 you mentioned that "for a >> couple of years now, Clang and libc++ are entirely separate projects w.r.t. >> how they are shipped on Apple platforms, and it doesn't make sense to build >> libc++ at the same time as Clang any

[PATCH] D122766: [clang] Add the flag -ffile-reproducible

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. I still wish this could just be the default behavior, but this flag seems a good compromise. LGTM (but please wait for Aaron too) Comment at: clang/include/clang/Driver/Options

[PATCH] D122766: [clang] Add the flag -ffile-reproducible

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6398f3f2e904: [clang] Add the flag -ffile-reproducible (authored by ayzhao, committed by hans). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D124701: [clang] Honor __attribute__((no_builtin("foo"))) on functions

hans added inline comments. Comment at: clang/test/CodeGen/no-builtin-2.c:51 +// CHECK: {{.*}}call {{.*}} @memmove +void foo4(char *s, char *d, size_t n) __attribute__((no_builtin("*"))) { + bar(s); In AttrDocs.td it says the wildcard case is spelled "__attri

[PATCH] D125513: [clang-cl] Add /Zc:wchar_t- option

hans added inline comments. Comment at: clang/test/Driver/cl-zc.cpp:50 // RUN: %clang_cl /c -### /Zc:wchar_t- -- %s 2>&1 | FileCheck -check-prefix=WCHAR_T-OFF %s -// WCHAR_T-OFF: argument unused during compilation +// WCHAR_T-OFF: "-fno-wchar" Should probably

[PATCH] D124701: [clang] Honor __attribute__((no_builtin("foo"))) on functions

hans added a comment. Just one more comment, then I'm happy :) Comment at: clang/lib/CodeGen/CGExpr.cpp:5067 +// recursion. Honor __attribute__((no_builtin("foo"))) or +// __attribute((no_builtin("*"))) on the current function unless foo is +// not a predefined libr

[PATCH] D125889: clang-cl: Move /Zc: tests from cl-options.c to cl-zc.cpp

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

[PATCH] D124701: [clang] Honor __attribute__((no_builtin("foo"))) on functions

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

[PATCH] D53476: [clang-cl] Allowed -fopenmp work and link openmp library from per-runtime library directory

hans added a comment. It looks like it was never committed. Peiyuan, what's the status here? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53476/new/ https://reviews.llvm.org/D53476 ___ cfe-commits mailing list cfe-

[PATCH] D66765: [analyzer] (Urgent!) Add 9.0.0. release notes.

hans added a comment. In D66765#1645726 , @Szelethus wrote: > Please note that LLVM 9.0.0-final is due on the 28th of August. There are still a lot of open bugs, so it will probably slip at least a little. Comment at: clang/docs/Relea

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

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

[PATCH] D66765: [analyzer] (Urgent!) Add 9.0.0. release notes.

This revision was automatically updated to reflect the committed changes. Closed by commit rL370166: [analyzer] Add 9.0.0. release notes. (authored by hans, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D

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

hans added a comment. In D66556#1648118 , @dexonsmith wrote: > In D66556#1648109 , @rnk wrote: > > > I'm not sure what happens, but I see you added .gitattributes. I'd commit > > it as is. Buildbots using svn will

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

hans added a comment. In D66556#1648807 , @aganea wrote: > In D66556#1648591 , @hans wrote: > > > In D66556#1648118 , @dexonsmith > > wrote: > > > > > In D66556#1648109

[PATCH] D65527: Avoid assemble step in verbose-output-quoting.c

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. Looks good to me. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65527/new/ https://reviews.llvm.org/D65527 __

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

hans added a comment. Herald added a subscriber: ychen. This is very exciting! I didn't look closely at the actual instrumentation code, as rnk knows that better and had some good comments. Comment at: clang/include/clang/Driver/Options.td:500 def b : JoinedOrSeparate<["-"],

[PATCH] D65699: [Driver] Prioritize SYSROOT/usr/include over RESOURCE_DIR/include on linux-musl

hans added a comment. In D65699#1662936 , @MaskRay wrote: > @hans Can this be put on your list of things for 9.0.1? I searched for > `[meta] 9.0.1` on bugs.llvm.org but can't find a release blocker bug so I > have to bother you directly... Right, the b

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

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm with comment > The main functional change is that -fno-ms-compatibility now no longer > sends us down the hard error diagnostic code path for ObjC fixed enums. > Instead, complete-but-not-fi

[PATCH] D36453: [libclang] Fix PR34055 (incompatible update of clang-c/Index.h)

hans added a comment. In https://reviews.llvm.org/D36453#834949, @n.bozhenov wrote: > If the patch is accepted, it should also be merged into 5.0, I believe. Sounds like a good idea. r310390 https://reviews.llvm.org/D36453 ___ cfe-commits mailing

[PATCH] D36532: Add -std=c++17

hans created this revision. As suggested on PR33912. Trying to keep this small to make it easy to merge to the 5.0 branch. We can do a follow-up with more thorough renaming (diagnostic text, options, ids, etc.) later. (For c++14 this was done in r215982, and I think a smaller patch for the 3.5

[PATCH] D36532: Add -std=c++17

hans added inline comments. Comment at: include/clang/Frontend/LangStandards.def:113 +LANGSTANDARD(cxx17, "c++17", + CXX, "ISO C++ 2017", LineComment | CPlusPlus | CPlusPlus11 | CPlusPlus14 | CPlusPlus1z | rsmith wrote: > Missing "with

[PATCH] D36532: Add -std=c++17

This revision was automatically updated to reflect the committed changes. Closed by commit rL310516: Make -std=c++17 an alias of -std=c++1z (authored by hans). Changed prior to commit: https://reviews.llvm.org/D36532?vs=110430&id=110462#toc Repository: rL LLVM https://reviews.llvm.org/D3653

[PATCH] D36954: [Sema] Update release notes with details of implicit scalar to vector conversions

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm, please commit to the 5.0 branch https://reviews.llvm.org/D36954 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D36872: Fixed a crash on replaying Preamble's PP conditional stack.

hans added a comment. In https://reviews.llvm.org/D36872#847416, @nik wrote: > I see this in trunk/master submitted, but not in the release_50 branch. Could > this be cherry-picked to 5.0? I've replied on the commit message thread. Thanks! Repository: rL LLVM https://reviews.llvm.org/D368

[PATCH] D36501: Add flag to request Clang is ABI-compatible with older versions of itself

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. I'm not familiar with the details of the ABI changes, but the mechanics of the code look good to me. Repository: rL LLVM https://reviews.llvm.org/D36501

[PATCH] D70568: [Support] Possibly use exception handler in the Crash Recovery Context in the same way as global exceptions

hans added a comment. In D70568#1789722 , @hans wrote: > > The way `Enable()/Disable()` is currently implemented will not work when > > the tool executes jobs in parallel (ie. llvm-buildozer > > I presented a

[PATCH] D70568: [Support] Possibly use exception handler in the Crash Recovery Context in the same way as global exceptions

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. Looks good to me. Thanks for pushing this forward! (Only one very minor comment.) Comment at: llvm/unittests/Support/CrashRecoveryTest.cpp:72 + sys::RemoveFileOnSignal(Filename

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

hans added inline comments. Comment at: clang/tools/driver/driver.cpp:267 + + StringRef SpawnCC1Str = ::getenv("CLANG_SPAWN_CC1"); + if (!SpawnCC1Str.empty()) { aganea wrote: > hans wrote: > > Maybe just do the "!!" thing like for the environment variables abov

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. Looks good to me (with a minor comment). Thanks! Comment at: clang/tools/driver/driver.cpp:271 +unsigned long long Enable; +if (getAsUnsignedInteger(SpawnCC1Str, 10, Enab

[PATCH] D72703: Add a warning, flags and pragmas to limit the number of pre-processor tokens in a translation unit

hans created this revision. hans added reviewers: thakis, rnk, rsmith. See https://docs.google.com/document/d/1xMkTZMKx9llnMPgso0jrx3ankI4cv60xeZ0y4ksf4wc/preview for background discussion. This adds a warning, flags and pragmas to limit the number of pre-processor tokens either at a certain p

[PATCH] D72769: Replace CLANG_SPAWN_CC1 env var with a driver mode flag

hans added a comment. In D72769#1823473 , @sammccall wrote: > As I understand, the original patch made the new behaviour on-by-default and > will make the release, so we want this flag on the release branch too, right? Yes, this is on my list. Thanks fo

[PATCH] D72630: [clang-tidy] Ignore implicit casts in modernize-use-default-member-init

hans added a comment. It seems that while this commit got pushed to GitHub, it's not actually part of master or any other branches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72630/new/ https://reviews.llvm.org/D72630 ___

[PATCH] D73425: [PPC] Fix platform definitions when compiling FreeBSD powerpc64 as LE

hans added a comment. This is new functionality, so I don't think we should merge it so late in the release process. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73425/new/ https://reviews.llvm.org/D73425

[PATCH] D86532: (Urgent!) [docs][analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm please commit so this makes it into the release. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86532/new/ https://reviews.llvm.org/D86532

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

hans accepted this revision. hans added a comment. This has probably had enough time for review. Please commit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86533/new/ https://reviews.llvm.org/D86533 ___ cfe-commits mailing list cfe-commits@

[PATCH] D86562: [OPENMP][NFC]Release notes for OpenMP in clang (11.x).

hans added a comment. Please go ahead and commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86562/new/ https://reviews.llvm.org/D86562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D86958: [Docs] Add/update release notes for D71913 (LTO WPD changes)

hans added a comment. Please go ahead and commit to the branch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86958/new/ https://reviews.llvm.org/D86958 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

hans added a comment. No worries :) Thanks for the notes! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86533/new/ https://reviews.llvm.org/D86533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/

[PATCH] D77882: [clang-tidy] Add option to use alpha checkers from clang-analyzer when using `run-clang-tidy.py`

hans added a comment. This seems to have caused https://bugs.llvm.org/show_bug.cgi?id=47512 Can you take a look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77882/new/ https://reviews.llvm.org/D77882

[PATCH] D86562: [OPENMP][NFC]Release notes for OpenMP in clang (11.x).

hans added a comment. In D86562#2274039 , @ABataev wrote: > In D86562#2274036 , @hans wrote: > >> Please go ahead and commit. > > Hi Hans, can you commit it yourself? I have some technical issues and not > able to

[PATCH] D87425: [CodeGen][typeid] Emit typeinfo directly if type is known at compile-time

hans added a comment. >> I think it would be safer to do the change purely as an optimization in >> codegen (maybe we could add a new helper method that could also be used by >> the warning). > > For "optimization in codegen", do you mean optimization after the IR is > generated or like I did

[PATCH] D86958: [Docs] Add/update release notes for D71913 (LTO WPD changes)

hans added a comment. I see it now. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86958/new/ https://reviews.llvm.org/D86958 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

[PATCH] D77882: [clang-tidy] Add option to use alpha checkers from clang-analyzer when using `run-clang-tidy.py`

hans added a comment. In D77882#2274570 , @Abpostelnicu wrote: > Has been pushed to the 11.x branch > https://github.com/llvm/llvm-project/commit/1596c2dfd548b21cf33ad3353882ac465d78c1bb Great, that was the missing piece :) Thanks! Repository: rG LL

[PATCH] D87425: [CodeGen][typeid] Emit typeinfo directly if type is known at compile-time

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang/test/SemaCXX/constant-expression-cxx2a.cpp:312 // expected-note@+1 {{typeid applied to object 'extern_b2' whose dynamic type is not constant}} - stati

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

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

[PATCH] D87710: [clangd] Actually parse Index section of the YAML file.

hans added a comment. In D87710#2275933 , @sammccall wrote: > Argh, sorry. I think this might be too late to get into 11 - we're in the > "soon=final" stage and maybe any changes at all cause logistical problems? :-( > > @hans to confirm - this is a featu

[PATCH] D87710: [clangd] Actually parse Index section of the YAML file.

hans added a comment. Pushed to 11.x as 4a26e3b33798424dc5a4843f7b29a617bef81656 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87710/new/ https://reviews.llvm.org/D87710 _

[PATCH] D31413: [libc++] Use __attribute__((init_priority(101))) to ensure streams get initialized early

hans added a comment. In D31413#2277085 , @sbc100 wrote: > Might even be worth backporting such as simple but useful fix to the 11 > release? I think it's too late in the process for that. Might be a good candidate for 11.0.1 though. Repository: rG

[PATCH] D86091: [cmake] Fix build of attribute plugin example on Windows

hans added a comment. In D86091#2296576 , @mati865 wrote: > @hans sorry for not spotting it earlier but is it possible to still backport > it to LLVM 11? > It's totally OK if it's too late. I can easily keep it as a patch in MSYS2 > repository. I'd pref

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

hans added a comment. In D85998#2231001 , @zahen wrote: > The build system strives to be deterministic so all file probes need to be > accounted for; environment variables are also problematic. Our builds > deliberately don't run from a Visual Studio co

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

hans added inline comments. Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:73 + MSVCToolChain::ToolsetLayout &VSLayout) { + // Don't validate the input; trust the value supplied by the user. + // The primary motivation is to prevent unnecessary fi

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

hans added inline comments. Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:73 + MSVCToolChain::ToolsetLayout &VSLayout) { + // Don't validate the input; trust the value supplied by the user. + // The primary motivation is to prevent unnecessary fi

[PATCH] D84049: Disable use of _ExtInt with '__atomic' builtins

hans added a comment. In D84049#2224061 , @jtmott-intel wrote: > Committed to master, and created bug to cherry pick into 11.0. > > https://bugs.llvm.org/show_bug.cgi?id=47222 Pushed to 11.x as 82e48a579024d0ffbc352702ec0c52b47a6fe691

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

hans added inline comments. Comment at: clang/test/Driver/cl-options.c:686 +// vctoolsdir is handled by the driver; just check that we don't error. Pass -c because fakedir isn't a real toolchain path +// RUN: %clang_cl -c -vctoolsdir fakedir -- %s 2>&1 + zahen w

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

hans added a comment. Lucas, this seems to have casued https://bugs.llvm.org/show_bug.cgi?id=47001. Can you take a look? (I would cc you on the bug, but I couldn't find your email in bugzilla.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75169/n

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

hans added inline comments. Comment at: clang/test/SemaCXX/ms_dynamic_cast.cpp:16 +B *b = new D1(); +auto d = dynamic_cast(b); // expected-warning{{should not use dynamic_cast with /GR-}} +} zequanwu wrote: > lebedev.ri wrote: > > I'm not sure it makes s

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

hans added a comment. Seems reasonable to me. I'm still curious why it seems it's not looking for link.exe in the /fake dir though. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85998/new/ https://reviews.llvm.org/D85998 ___ cfe-commits mai

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

hans added a comment. I'll land tomorrow unless Alexandre beats me to it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85998/new/ https://reviews.llvm.org/D85998 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

This revision was automatically updated to reflect the committed changes. Closed by commit rG087047144210: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain (authored by zahen, committed by hans). Changed prior to commit: https://reviews.llvm.org/D85998?vs=287671&i

[PATCH] D86622: Fix failing tests after VCTOOLSDIR change

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7a34dca0f391: Fix failing tests after VCTOOLSDIR change (authored by zahen, committed by hans). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D86622: Fix failing tests after VCTOOLSDIR change

hans added a comment. In D86622#2238992 , @zahen wrote: > Sorry again about the break! As with the initial patch, I'll need you to > land this change. No worries, sorry for not catching it in the review. Pushed now as 7a34dca0f3918ab1c0397e56dd64a3c041

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

hans added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7436 "use of dynamic_cast requires -frtti">; +def warn_no_dynamic_cast_with_RTTI_disabled: Warning< + "dynamic_cast will not work since RTTI data is disabled by " The ot

[PATCH] D86562: [OPENMP][NFC]Release notes for OpenMP in clang (11.x).

hans added a comment. >> but we can merge this right away (IMHO) > > It is up to Hans if he has some time. But I think I covered almost all new > features (+ complex for NVPTX), so after the update it can be merged with > 11.x release. It's fine by me if you want to wait a day or two for more c

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

hans added inline comments. Comment at: clang/lib/Sema/SemaCast.cpp:895 + if (!Self.getLangOpts().RTTIData) { +bool isMSVC = Self.getDiagnostics().getDiagnosticOptions().getFormat() == + DiagnosticOptions::MSVC; I'm not sure isMSVC is the be

[PATCH] D86626: [OpenCL][Docs] 10.x release notes

hans added a comment. In D86626#2243876 , @Anastasia wrote: > @hans, would you be able to commit this to the release branch? Committed dae9fe408793def8a49f5e1d10d2a859627785e3 . Thanks!

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

hans added inline comments. Comment at: clang/lib/Sema/SemaCast.cpp:895 + if (!Self.getLangOpts().RTTIData) { +bool isMSVC = Self.getDiagnostics().getDiagnosticOptions().getFormat() == + DiagnosticOptions::MSVC; zequanwu wrote: > hans wrote:

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

hans added inline comments. Comment at: clang/lib/Sema/SemaCast.cpp:895 + if (!Self.getLangOpts().RTTIData) { +bool isMSVC = Self.getDiagnostics().getDiagnosticOptions().getFormat() == + DiagnosticOptions::MSVC; zequanwu wrote: > hans wrote:

[PATCH] D86820: [X86] Add a /tune: option for clang-cl

hans added a comment. Would it be enough for users to specify /clang:-mtune instead? How does icc spell its option? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86820/new/ https://reviews.llvm.org/D86820 ___ cfe-commits mailing list cfe-com

[PATCH] D86820: [X86] Add a /tune: option for clang-cl

hans added a comment. Okay, adding it to clang-cl seems fine to me. But I think it could be a simple alias? Comment at: clang/include/clang/Driver/Options.td:4693 HelpText<"Set architecture for code generation">; +def _SLASH_tune : CLCompileJoined<"tune:">, + HelpText<"Set

[PATCH] D86958: [Docs] Add/update release notes for D71913 (LTO WPD changes)

hans accepted this revision. hans added a comment. Looks great, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86958/new/ https://reviews.llvm.org/D86958 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D86820: [X86] Add a /tune: option for clang-cl

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

[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

hans added a comment. Seems reasonable to me. Just some nits. Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:29 + auto EqualSignIndex = ArgRef.find('='); + return StringRef(ArgRef.data() + EqualSignIndex + 1); +} Instead of manually indexing

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

hans added inline comments. Comment at: clang/lib/Sema/SemaCast.cpp:895 + if (!Self.getLangOpts().RTTIData) { +bool isMSVC = Self.getDiagnostics().getDiagnosticOptions().getFormat() == + DiagnosticOptions::MSVC; zequanwu wrote: > hans wrote:

[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

hans added inline comments. Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:117 + // do not remove those when using the cl driver. + bool IsDependencyFileArg; + if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes")) shivanshu3

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

hans added a comment. Okay, almost there.. Comment at: clang/lib/Sema/SemaCast.cpp:897 + DiagnosticOptions::MSVC; +if (isMSVC || !DestPointee->isVoidType()) + Self.Diag(OpRange.getBegin(), I don't think the if-statement is necessary. I

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

hans added inline comments. Comment at: clang/lib/Sema/SemaCast.cpp:897 + DiagnosticOptions::MSVC; +if (isMSVC || !DestPointee->isVoidType()) + Self.Diag(OpRange.getBegin(), zequanwu wrote: > hans wrote: > > I don't think the if-statemen

[PATCH] D87118: Add an explicit toggle for the static analyzer in clang-tidy

hans added a comment. Cool! Comment at: clang-tools-extra/CMakeLists.txt:4 +option(CLANG_TIDY_ENABLE_STATIC_ANALYZER + "Include static analyzer checks in clang-tidy" ON) + Should this default to CLANG_ENABLE_STATIC_ANALYZER instead of ON? Co

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. LGTM But please add a -triple parameter to the test files, and check the dynamic_cast behavior in each case before committing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

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 rG2168dbf4cc76: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl… (authored by shivanshu3, co

[PATCH] D80197: [DebugInfo] Upgrade DISubrange to support Fortran dynamic arrays

hans added a comment. Alok, Adrian: please work on addressing the concerns raised in https://bugs.llvm.org/show_bug.cgi?id=47287, or please revert this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80197/new/ https://reviews.llvm.org/D8019

[PATCH] D87118: Add an explicit toggle for the static analyzer in clang-tidy

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

[PATCH] D87425: [CodeGen][typeid] Emit typeinfo directly if type is known at compile-time

hans added a comment. I'm not sure that changing isPotentiallyEvaluated() is the right thing to do. The meaning of that corresponds to text in the standard: https://eel.is/c++draft/expr.typeid#3 so changing it to something that doesn't match the standard exactly seems wrong. I think it would b

[PATCH] D87360: clang-cl: Alias /wd4101 to -Wno-unused-variable

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

[PATCH] D87364: clang-cl: Alias /wd4238 to -Wno-address-of-temporary

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

[PATCH] D87368: clang-cl: Alias /wd5054 to -Wno-deprecated-anon-enum-enum-conversion

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

[PATCH] D87372: clang-cl: Ignore /Zc:externConstexpr and /Zc:throwingNew

hans added a comment. I'm not sure ignoring these is the right thing to do. Maybe they should be "Unsupported but parsed options" instead? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87372/new/ https://reviews.llvm.org/D87372 _

[PATCH] D90849: [dllexport] Avoid multiple codegen assert for explicitly defaulted methods in explicit instantiation definitions (PR47683)

hans created this revision. hans added a reviewer: rnk. Herald added a project: clang. hans requested review of this revision. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D90849 Files: clang/lib/Sema/SemaDeclCXX.cpp clang/test/CodeGenCXX/dllexport.cpp Index: clang/test/C

[PATCH] D87372: clang-cl: Ignore /Zc:externConstexpr and /Zc:throwingNew

hans added a comment. In D87372#2376305 , @ki.stfu wrote: > @hans they are ignored because Clang behaves like MSVC with these options > enabled (C++ standard conformance mode) Ah, I got it backwards. Yeah, ignoring these makes sense. Can you add the vari

[PATCH] D90849: [dllexport] Avoid multiple codegen assert for explicitly defaulted methods in explicit instantiation definitions (PR47683)

hans marked 3 inline comments as done. hans added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5898 // encountered. } else if (!MD->isTrivial() || MD->isExplicitlyDefaulted() || MD->isCopyAssignmentOperator() || rn

[PATCH] D90849: [dllexport] Avoid multiple codegen assert for explicitly defaulted methods in explicit instantiation definitions (PR47683)

hans updated this revision to Diff 303778. hans marked an inline comment as done. hans added a comment. Addressing comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90849/new/ https://reviews.llvm.org/D90849 Files: clang/lib/Sema/SemaDeclCXX.cpp clang/test/CodeGenCXX/dllexpor

[PATCH] D90849: [dllexport] Avoid multiple codegen assert for explicitly defaulted methods in explicit instantiation definitions (PR47683)

hans added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5917 // There is no later point when we will see the definition of this // function, so pass it to the consumer now. S.Consumer.HandleTopLevelDecl(DeclGroupRef(MD));

[PATCH] D91089: [dllexport] Instantiate default ctor default args for explicit specializations (PR45811)

hans created this revision. hans added reviewers: rnk, zahiraam. Herald added a project: clang. hans requested review of this revision. For dllexported default constructors with default arguments, we export default constructor closures which pass in the default args. (See D8331

[PATCH] D91089: [dllexport] Instantiate default ctor default args for explicit specializations (PR45811)

hans added a comment. Please take a look. Comment at: clang/lib/Sema/SemaDecl.cpp:13996 } + if (auto *Ctor = dyn_cast(FD)) { I was searching for the right place to do this, looking mostly in the sema template code, but ended up here as an explicit specia

[PATCH] D91089: [dllexport] Instantiate default ctor default args for explicit specializations (PR45811)

hans added a comment. > clang.exe -c test.cpp > Assertion failed: !hasUninstantiatedDefaultArg() && "Default argument is not > yet instantiated!", file > D:\IUSERS\zahiraam\llorg_ws\ws1\llvm\clang\lib\AST\Decl.cpp, line 2719 > PLEASE submit a bug report to https://bugs.llvm.org/ and include the

[PATCH] D91089: [dllexport] Instantiate default ctor default args for explicit specializations (PR45811)

hans updated this revision to Diff 304190. hans added a comment. Handling the decl only case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91089/new/ https://reviews.llvm.org/D91089 Files: clang/include/clang/Sema/Sema.h clang/lib/CodeGen/MicrosoftCXXABI.cpp clang/lib/Sema/SemaD

[PATCH] D90849: [dllexport] Avoid multiple codegen assert for explicitly defaulted methods in explicit instantiation definitions (PR47683)

This revision was automatically updated to reflect the committed changes. Closed by commit rGb9d36540a85b: [dllexport] Avoid assert for explicitly defaulted methods in explicit… (authored by hans). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90849/

[PATCH] D91089: [dllexport] Instantiate default ctor default args for explicit specializations (PR45811)

hans added a comment. >> I don't think it makes sense to try to emit the closure until we have the >> ctor definition. I'll update the patch to handle this. > > Ok. That's a bit different than what MSVC is doing. It generates a closure > constructor even if there is only a declaration. But I gue

[PATCH] D91089: [dllexport] Instantiate default ctor default args for explicit specializations (PR45811)

This revision was automatically updated to reflect the committed changes. Closed by commit rGa088766508aa: [dllexport] Instantiate default ctor default args for explicit specializations… (authored by hans). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

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