[PATCH] D50349: Port getStartLoc -> getBeginLoc

hans added a comment. In https://reviews.llvm.org/D50349#1190029, @rsmith wrote: > +Hans (release manager for LLVM 7) > > Hans, this patch series will affect the API of common Clang classes, > resulting in patches to Clang SVN needing some (mechanical) modifications to > be applied to the Clang

[PATCH] D50170: [libcxxabi] Fix test_exception_address_alignment test for ARM

hans added a comment. What do you other reviewers say? I'm not familiar with this code, but this seems reasonable to me. Repository: rCXXA libc++abi https://reviews.llvm.org/D50170 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://l

[PATCH] D50380: [Headers] Expand _Unwind_Exception for SEH on MinGW/x86_64

hans added a comment. In https://reviews.llvm.org/D50380#1191423, @mstorsjo wrote: > @hans Can you merge this for 7.0? This is necessary for > https://reviews.llvm.org/D49638 (merged well before the branch) to work > properly without causing heap corruption. Merged in r339220. Repository:

[PATCH] D50412: [libunwind] Fix pointer-to-integer cast warnings on LLP64.

hans added a comment. In https://reviews.llvm.org/D50412#1191843, @mstorsjo wrote: > @hans This looks 7.0-worthy to me. Okay, r339222. Repository: rL LLVM https://reviews.llvm.org/D50412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50513: clang-cl: Support /guard:cf,nochecks

hans created this revision. hans added reviewers: rnk, thakis. This extension emits the guard cf table without inserting the instrumentation. Currently that's what clang-cl does with /guard:cf anyway, but this allows a user to request that explicitly. https://reviews.llvm.org/D50513 Files:

[PATCH] D50513: clang-cl: Support /guard:cf,nochecks

hans added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:463 // We want function ID tables for Control Flow Guard. -getModule().addModuleFlag(llvm::Module::Warning, "cfguard", 1); +getModule().addModuleFlag(llvm::Module::Warning, "cfguardtable", 1); }

[PATCH] D50513: clang-cl: Support /guard:cf,nochecks

This revision was automatically updated to reflect the committed changes. Closed by commit rL339420: clang-cl: Support /guard:cf,nochecks (authored by hans, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50513?vs=159927&id=160076#toc

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

hans added a comment. The reason we noticed this was that it caused a *50 GB* size increase of the build output on our buildbots, which was enough to cause infrastructure problems. This change was also committed shortly before the 7.0 branch, so it's part of the 7.0.0 release candidates. Shou

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

hans added a comment. In https://reviews.llvm.org/D49240#1197052, @ldionne wrote: > In https://reviews.llvm.org/D49240#1196878, @hans wrote: > > > The reason we noticed this was that it caused a *50 GB* size increase of > > the build output on our buildbots, which was enough to cause infrastruct

[PATCH] D50691: [CMake] Fix the LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY option

hans added a comment. In https://reviews.llvm.org/D50691#1198514, @mstorsjo wrote: > @hans I think this should be merged to 7.0, if reviewers agree with the > change. Okay, I'll keep an eye on it. Repository: rCXX libc++ https://reviews.llvm.org/D50691 _

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

hans added a comment. In https://reviews.llvm.org/D50652#1197775, @rnk wrote: > I'd prefer not to do this, since internal_linkage generates smaller, more > debuggable code by default. IIUC, this is what we had before though, so it's a conservative "revert to safety" approach until a better so

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

hans added a comment. If we think the symbol/string table size increase is acceptable for most user's, I suppose Chromium could just do -D_LIBCPP_HIDE_FROM_ABI=_LIBCPP_ALWAYS_INLINE to get back the old behaviour and unblock us? And this could also be suggested in the release notes as a work-

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

hans accepted this revision. hans added a comment. >>> Oh, or could we do >>> >>> -D_LIBCPP_HIDE_FROM_ABI= >>> >>> >>> and just get regular odr linkage for these functions? >> >> No, you do need to use `_LIBCPP_HIDDEN _LIBCPP_ALWAYS_INLINE` because of the >> issue described in >> http://li

[PATCH] D50691: [CMake] Fix the LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY option

hans added a comment. Merged in r339850. Repository: rL LLVM https://reviews.llvm.org/D50691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50170: [libcxxabi] Fix test_exception_address_alignment test for ARM

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. It doesn't seem to get more reviewed than this. yroux, I'd say go ahead and commit it. Repository: rCXXA libc++abi https://reviews.llvm.org/D50170 __

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

hans added a comment. Thanks! Merged to 7.0 in r339882. Repository: rL LLVM https://reviews.llvm.org/D50652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50979: Eliminate instances of `EmitScalarExpr(E->getArg(n))` in EmitX86BuiltinExpr().

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

[PATCH] D51069: Notify pending API removal in the release notes

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. Looks great, thanks! Repository: rC Clang https://reviews.llvm.org/D51069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D51069: Notify pending API removal in the release notes

hans added a comment. In https://reviews.llvm.org/D51069#1208591, @steveire wrote: > @hans Could you commit this on my behalf? I was not able to do it myself for > some reason. > > $ svn commit > svn: E175002: Commit failed (details follow): > svn: E175002: Unexpected HTTP status 400 'Bad Req

[PATCH] D51069: Notify pending API removal in the release notes

This revision was automatically updated to reflect the committed changes. Closed by commit rL340375: Notify pending API removal in the release notes (authored by hans, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D51069 Files: cfe/bra

[PATCH] D51079: Update the docs for using LLVM toolset in Visual Studio

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

[PATCH] D51134: win: Omit ".exe" from clang and clang-cl driver-level diagnostics.

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. Nice, thanks! https://reviews.llvm.org/D51134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D46520: [Driver] Use -fuse-line-directives by default in MSVC mode

hans added a comment. In https://reviews.llvm.org/D46520#1092233, @rnk wrote: > Please don't do this, this is actually really problematic, since `#line` > directives lose information about what's a system header. That means the > result of -E usually won't compile, since Windows headers are typ

[PATCH] D45045: [DebugInfo] Generate debug information for labels.

hans added a comment. This broke the Chromium build. I've uploaded a reproducer at https://bugs.chromium.org/p/chromium/issues/detail?id=841170#c1 I'm guessing maybe a Clang bootstrap with debug info might also reproduce the problem, but I haven't tried that. Reverted in r331861. Repository:

[PATCH] D51212: [OpenCL][Docs] Release notes for OpenCL in Clang

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

[PATCH] D51172: [libcxx] Comment out #define __cpp_lib_node_extract, we only support half of that functionality

hans added a comment. In https://reviews.llvm.org/D51172#1211156, @erik.pilkington wrote: > Landed as r340544. @hans: Can you cherry-pick? Merged in r340662. Thanks! Repository: rCXX libc++ https://reviews.llvm.org/D51172 ___ cfe-commits maili

[PATCH] D51212: [OpenCL][Docs] Release notes for OpenCL in Clang

hans added a comment. Anastasia: will you commit this to the branch, or would like me to do it? https://reviews.llvm.org/D51212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51356: [docs][mips] Clang 7.0 Release notes

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm, thanks! Go ahead and commit directly to the branch, or let me know if you'd prefer me to do it. Repository: rC Clang https://reviews.llvm.org/D51356 __

[PATCH] D51391: [clang-cl,PCH] Add support for #pragma hdrstop

hans added a comment. Very cool! I only have some minor comments. Oh, and when it lands, you should probably add an update to docs/ReleaseNotes.rst about it too. Comment at: include/clang/Driver/CC1Options.td:613 + HelpText<"Stop PCH generation after #pragma hdrstop. When u

[PATCH] D51635: clang-cl: Pass /Brepro to linker if it was passed to the compiler

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. I suppose the alternative would be to make /Brepro a non-alias flag, expand it to -mno-incremental-linker-compatible for the cc1 invocation and look for it for the linker invocation. But this is

[PATCH] D50534: [libc++] Fix handling of negated character classes in regex

hans added a comment. In https://reviews.llvm.org/D50534#1225591, @xbolva00 wrote: > is this fixed in 7.0 release branch too? > > @hans I've merged it in r341529 now. Thanks for letting me know. Repository: rCXX libc++ https://reviews.llvm.org/D50534

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

hans added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244 + false)) +CmdArgs.push_back("-fvisibility-inlines-hidden"); + Huh, does this actually affect whether functions get dllexported or not? https://reviews.llvm

[PATCH] D51806: [clang-cl] Enable -march

hans added a comment. All flags in the m_x86_Features_Group, i.e. -msse, -mavx and so on are all supported. I'm just curious to hear what it is that you need from -march? Repository: rC Clang https://reviews.llvm.org/D51806 ___ cfe-commits maili

[PATCH] D51391: [clang-cl,PCH] Add support for #pragma hdrstop

hans added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:2862 + Opts.PCHWithHdrStopCreate = + Args.getLastArgValue(OPT_pch_through_hdrstop_EQ) == "create"; Opts.PCHThroughHeader = Args.getLastArgValue(OPT_pch_through_header_EQ); mikeri

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

hans added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244 + false)) +CmdArgs.push_back("-fvisibility-inlines-hidden"); + takuto.ikuta wrote: > hans wrote: > > Huh, does this actually affect whether functions get dlle

[PATCH] D51806: [clang-cl] Enable -march

hans added a comment. Okay, that sounds good to me. Please also add a test for this in test/Driver/cl-options.c in the "Accept "core" clang options" section. Repository: rC Clang https://reviews.llvm.org/D51806 ___ cfe-commits mailing list cfe-

[PATCH] D51806: [clang-cl] Enable -march

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

[PATCH] D51806: [clang-cl] Enable -march

hans added a comment. In https://reviews.llvm.org/D51806#1228893, @aganea wrote: > @hans Just an after thought: maybe we should prevent usage of `-march=` and > `/arch:` at the same time. What do you think? I can add another patch for > that purpose. Hmm, yes, at least we should warn or do so

[PATCH] D51391: [clang-cl,PCH] Add support for #pragma hdrstop

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm! Please add a note to docs/ReleaseNotes.rst when landing. https://reviews.llvm.org/D51391 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D42762: Rewrite the VS Integration Scripts

hans added a comment. Thanks for pushing this through! Will you upload the vsix to the marketplace? (Use the llvm-vs-code-extensi...@google.com account, which despite the name is also used for the clang-format vs plugin etc.) Then I can update the docs to point at it. Repository: rC Clang

[PATCH] D43108: Support for the mno-stack-arg-probe flag

hans added a comment. I see in the PR that matches a MinGW flag, but I'm curious to the motivation here. In what scenario would the user want to use this, i.e. how do they know it's safe to drop the probes? Comment at: lib/CodeGen/TargetInfo.cpp:2358 -static void addStackPr

[PATCH] D43108: Support for the mno-stack-arg-probe flag

hans added a comment. In https://reviews.llvm.org/D43108#1005904, @nruslan wrote: > @hans: One real-world example is when it is used to compile UEFI code using > PE/COFF targets natively. Obviously, UEFI uses ABI which is basically almost > the same as MS ABI, except that chkstk is not used. It

[PATCH] D43108: Support for the mno-stack-arg-probe flag

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. This looks good to me. https://reviews.llvm.org/D43108 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D43108: Support for the mno-stack-arg-probe flag

This revision was automatically updated to reflect the committed changes. Closed by commit rC325901: Support for the mno-stack-arg-probe flag (authored by hans, committed by ). Changed prior to commit: https://reviews.llvm.org/D43108?vs=134377&id=135631#toc Repository: rC Clang https://revi

[PATCH] D33616: [MS] Fix _bittest* intrinsics for values bigger than 31

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

[PATCH] D33616: [MS] Fix _bittest* intrinsics for values bigger than 31

hans added a comment. From looking in the Intel manual (Table 3-2, in 3.1.1.9 about Bit(BitBase,BitOffset)) it does sound like the bit offset can be negative *shudder*, so I suppose this is necessary and explains why the type is signed in the first place? Hopefully most of these will be inlined

[PATCH] D34279: Fix release_40 build with MSVC (VS 2015)

hans added a subscriber: tstellar. hans added a comment. That's strange. I build the llvm 4.0 branch (and trunk too, which has the same code) with Visual Studio 2015 without problems. (I don't build with warnings as errors though, so if this is a warning maybe I missed it.) Do you have any loca

[PATCH] D34290: [Driver] Do a PATH lookup when using only program name with -no-canonical-prefixes

hans added a comment. Thanks! Please add to the description that this is for PR9576. Comment at: tools/driver/driver.cpp:58 +SmallString<128> ExecutablePath(Argv0); +// Do a PATH lookup, if there are no directory components. +if (llvm::sys::path::filename(Executabl

[PATCH] D34290: [Driver] Do a PATH lookup when using only program name with -no-canonical-prefixes

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm Repository: rL LLVM https://reviews.llvm.org/D34290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[PATCH] D34279: Fix release_40 build with MSVC (VS 2015)

hans added a comment. I have 19.00.24210, so slightly earlier I suppose, but I believe we use Update 3 on our Chromium buildbots, and they seem happy. Can you paste the full error message? The part I see in your screenshot doesn't really make sense. Why should converting 1 to unsigned int be a

[PATCH] D34279: Fix release_40 build with MSVC (VS 2015)

hans added a comment. I still don't understand why this is breaking your build. Assuming this is the line the first error refers to: for (unsigned BinOp : {G_ADD, G_SUB, G_MUL, G_AND, G_OR, G_XOR, G_SHL}) { I don't see how converting G_ADD, even if it is an int, to unsigned would be a narrow

[PATCH] D43888: [clang-cl] Implement /X

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm Nicely tested too :-) https://reviews.llvm.org/D43888 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[PATCH] D44630: [ms] Parse #pragma optimize and ignore it behind its own flag

hans created this revision. hans added reviewers: thakis, rnk. This allows users to turn off warnings about this pragma specifically, while still receiving warnings about other ignored pragmas. https://reviews.llvm.org/D44630 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Bas

[PATCH] D44630: [ms] Parse #pragma optimize and ignore it behind its own flag

This revision was automatically updated to reflect the committed changes. hans marked an inline comment as done. Closed by commit rL327959: [ms] Parse #pragma optimize and ignore it behind its own flag (authored by hans, committed by ). Herald added a subscriber: llvm-commits. Changed prior to co

[PATCH] D44630: [ms] Parse #pragma optimize and ignore it behind its own flag

hans marked an inline comment as done. hans added inline comments. Comment at: include/clang/Basic/DiagnosticParseKinds.td:973 +def warn_pragma_optimize : Warning< + "'#pragma optimize' is not supported; use '#pragma clang optimize on|off' instead">, + InGroup; ---

[PATCH] D47875: [MS ABI] Mangle unnamed empty enums (PR37723)

hans created this revision. hans added a reviewer: majnemer. Clang would previously assert here. https://reviews.llvm.org/D47875 Files: lib/AST/MicrosoftMangle.cpp test/CodeGenCXX/mangle-ms-cxx11.cpp Index: test/CodeGenCXX/mangle-ms-cxx11.cpp ==

[PATCH] D47875: [MS ABI] Mangle unnamed empty enums (PR37723)

hans added a comment. Please take a look. I couldn't figure out a way to get a mangled name for this without using debug info. Do you have any ideas? https://reviews.llvm.org/D47875 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

hans added a comment. What's special about size_t though? If I understand your patch correctly, it would suppress warning about printing NSInteger with %zd, but still warn about %ld even though ssize_t=long on the target? As a user I'd find this confusing. If we really want to special-case NSIn

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

hans added a comment. In https://reviews.llvm.org/D47290#1124933, @aaron.ballman wrote: > In https://reviews.llvm.org/D47290#1124866, @hans wrote: > > > If we really want to special-case NSInteger, and given that you're > > targeting a specific wide-spread pattern maybe that's the right thing to

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

hans added a comment. In https://reviews.llvm.org/D47290#1124964, @aaron.ballman wrote: > In https://reviews.llvm.org/D47290#1124956, @hans wrote: > > > In https://reviews.llvm.org/D47290#1124933, @aaron.ballman wrote: > > > > > In https://reviews.llvm.org/D47290#1124866, @hans wrote: > > > > > >

[PATCH] D47875: [MS ABI] Mangle unnamed empty enums (PR37723)

hans added a comment. Thanks! Comment at: lib/AST/MicrosoftMangle.cpp:888-891 auto EnumeratorI = ED->enumerator_begin(); -assert(EnumeratorI != ED->enumerator_end()); -Name += "getName(); +if (EnumeratorI == ED->enumerator_end()) { + Na

[PATCH] D47875: [MS ABI] Mangle unnamed empty enums (PR37723)

hans updated this revision to Diff 150462. hans added a comment. Falling back to the "Otherwise, number using $S" code for non-empty enums. https://reviews.llvm.org/D47875 Files: lib/AST/MicrosoftMangle.cpp test/CodeGenCXX/mangle-ms-cxx11.cpp Index: test/CodeGenCXX/mangle-ms-cxx11.cpp ===

[PATCH] D47875: [MS ABI] Mangle unnamed empty enums (PR37723)

This revision was automatically updated to reflect the committed changes. Closed by commit rL334388: [MS ABI] Mangle unnamed empty enums (PR37723) (authored by hans, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47875?vs=150462&id=15

[PATCH] D47672: [Headers] Add _Interlocked*_HLEAcquire/_HLERelease

hans added a comment. It sounds like adding proper support for HLE prefixes is a largeish project. ctopper, rnk: Do you think it would be worth adding inline asm versions (with the xacquire/release prefixes) of these intrinsics in the meantime? It would inhibit optimizations but be better than

[PATCH] D46024: [clang-format] Add SpaceBeforeCpp11BracedList option.

hans added a comment. In https://reviews.llvm.org/D46024#1121242, @rkirsling wrote: > FWIW, please note that this space-before-brace style is not specific to > WebKit; CppCoreGuidelines exhibits it as well: > > http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es23-prefer-the--initia

[PATCH] D46024: [clang-format] Add SpaceBeforeCpp11BracedList option.

hans added a comment. Ross, do you have commit access or do you need someone to commit this for you? Repository: rC Clang https://reviews.llvm.org/D46024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

[PATCH] D47672: [Headers] Add _Interlocked*_HLEAcquire/_HLERelease

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. Nice! Looks good to me. https://reviews.llvm.org/D47672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D47578: Do not enforce absolute path argv0 in windows

hans added a comment. In https://reviews.llvm.org/D47578#1127749, @takuto.ikuta wrote: > Rui-san, can I ask you to merge this? Committed here: http://llvm.org/viewvc/llvm-project?view=revision&revision=334602 https://reviews.llvm.org/D47578 ___

[PATCH] D46024: [clang-format] Add SpaceBeforeCpp11BracedList option.

This revision was automatically updated to reflect the committed changes. Closed by commit rL334692: [clang-format] Add SpaceBeforeCpp11BracedList option. (authored by hans, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D46024?vs=1509

[PATCH] D46652: [clang-cl, PCH] Implement support for MS-style PCH through headers

hans added a comment. In https://reviews.llvm.org/D46652#1138375, @rnk wrote: > @hans, think you'll have time to look at this with your recent dllexport PCH > experimentation? Yes, I mean to look at it, marked it unread in my inbox and everything. https://reviews.llvm.org/D46652 _

[PATCH] D48426: [clang-cl] Don't emit dllexport inline functions etc. from pch files (PR37801)

hans created this revision. hans added reviewers: thakis, rnk, rsmith. With MSVC, PCH files are created along with an object file that needs to be linked into the final library or executable. That object file contains the code generated when building the headers. In particular, it will include d

[PATCH] D48426: [clang-cl] Don't emit dllexport inline functions etc. from pch files (PR37801)

hans added a comment. I think you revved the PCH version last. Did you have to do anything special, e.g. will it break vendors who ship PCH files as part of SDKs and such? Or are things generally set up to handle this? https://reviews.llvm.org/D48426

[PATCH] D48426: [clang-cl] Don't emit dllexport inline functions etc. from pch files (PR37801)

hans added a comment. In https://reviews.llvm.org/D48426#1139318, @thakis wrote: > PCHs aren't compatible with themselves if only the compiler revision changes, > so I'm not sure changing that field should be worse than a regular compiler > revision update (which happens at every commit). But I

[PATCH] D48426: [clang-cl] Don't emit dllexport inline functions etc. from pch files (PR37801)

hans marked 4 inline comments as done. hans added a comment. In https://reviews.llvm.org/D48426#1140057, @dblaikie wrote: > In https://reviews.llvm.org/D48426#1139823, @rnk wrote: > > > `LangOpts.ModulesCodegen` is very related in spirit to this, but I think we > > need a distinct option because

[PATCH] D48426: [clang-cl] Don't emit dllexport inline functions etc. from pch files (PR37801)

hans updated this revision to Diff 152444. hans marked 2 inline comments as done. hans added a comment. Addressing review comments. https://reviews.llvm.org/D48426 Files: include/clang/AST/ExternalASTSource.h include/clang/Basic/LangOptions.def include/clang/Driver/CC1Options.td include

[PATCH] D48426: [clang-cl] Don't emit dllexport inline functions etc. from pch files (PR37801)

hans added a comment. I hit a snag while building some more Chromium targets. For class templates with explicit instantiation decls in the PCH file and explicit instantiation definitions in a .cc file, the function definition will be marked as coming from the PCH, even though it wasn't defined

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

hans added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5599 +bool Sema::isInlineFunctionDLLExportable(const CXXMethodDecl *MD) { + assert(MD->isInlined()); Okay, breaking out this logic is a little better, but I still don't like that we now ha

[PATCH] D52773: clang-cl: Add /showFilenames option (PR31957)

hans updated this revision to Diff 168999. hans added a comment. Here's a version now using cc1 flags, but printing directly from the driver. Please take a look. https://reviews.llvm.org/D52773 Files: include/clang/Driver/CLCompatOptions.td include/clang/Driver/Job.h lib/Driver/Job.cpp

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

hans added a comment. Thanks! I think this is getting close now. Comment at: clang/lib/Sema/SemaDecl.cpp:11976 + +while (FD && !getDLLAttr(FD) && + !FD->hasAttr() && Why does this need to be a loop? I don't think FunctionDecl's can be nested? ==

[PATCH] D53052: [AST] Use -fvisibility value when ignoring -fv-i-h* inline static locals

hans added a comment. In https://reviews.llvm.org/D53052#1261158, @rnk wrote: > This is rewritten enough that you might want to re-review it, but I'm going > to push it today so we don't have to wait another day/night cycle for the fix. Looks good to me. It's hard to follow the logic, but the

[PATCH] D52773: clang-cl: Add /showFilenames option (PR31957)

hans added inline comments. Comment at: test/Driver/cl-showfilenames.c:7-8 +// multiple: cl-showfilenames.c +// multiple-NEXT: wildcard1.c +// multiple-NEXT: wildcard2.c rnk wrote: > I think it'd be nice to have the test show that diagnostics come out > interlea

[PATCH] D52773: clang-cl: Add /showFilenames option (PR31957)

This revision was automatically updated to reflect the committed changes. Closed by commit rL344234: clang-cl: Add /showFilenames option (PR31957) (authored by hans, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52773?vs=168999&id=16

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

hans added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:11976 + +while (FD && !getDLLAttr(FD) && + !FD->hasAttr() && takuto.ikuta wrote: > hans wrote: > > Why does this need to be a loop? I don't think FunctionDecl's can be nested? > Thi

[PATCH] D53102: Support for the mno-tls-direct-seg-refs flag

hans added a comment. Looking good, just a few minor comments. Comment at: docs/ClangCommandLineReference.rst:2241 + +Enable or disable direct TLS access through segment registers + This file is automatically generated based on the options .td files, so no need

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

hans added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:11976 + +while (FD && !getDLLAttr(FD) && + !FD->hasAttr() && takuto.ikuta wrote: > hans wrote: > > takuto.ikuta wrote: > > > hans wrote: > > > > Why does this need to be a loop? I d

[PATCH] D53434: Java annotation declaration being handled correctly

This revision was automatically updated to reflect the committed changes. Closed by commit rL344789: Java annotation declaration being handled correctly (authored by hans, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D53434?vs=170197

[PATCH] D53457: clang-cl: Add "/Xdriver:" pass-through arg support.

hans added a comment. I haven't started looking at the code yet. I'm not completely convinced that we want this. So far we've used the strategy of promoting clang options that are also useful in clang-cl to core options, and if someone wants to use more clang than that, maybe clang-cl isn't the

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

hans accepted this revision. hans added a comment. Looks good to me. Repository: rC Clang https://reviews.llvm.org/D53476 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53457: clang-cl: Add "/Xdriver:" pass-through arg support.

hans added a comment. In https://reviews.llvm.org/D53457#1271046, @neerajksingh wrote: > In https://reviews.llvm.org/D53457#1269769, @hans wrote: > > > I'm not completely convinced that we want this. So far we've used the > > strategy of promoting clang options that are also useful in clang-cl t

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

hans added a comment. In https://reviews.llvm.org/D51340#1266312, @takuto.ikuta wrote: > Hans, I addressed all your comments. > How do you think about current implementation? Just a few questions, but I think it's pretty good. Comment at: clang/include/clang/Basic/Attr.td:2

[PATCH] D53617: [AArch64] Support Windows stack probe command-line arguments.

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

[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.

hans added a comment. One note about flag ordering: the /clang: flags are concatenated to the end of the argument list, so in cases where the last flag wins, the /clang: flags will be chosen regardless of their order relative to other flags on the driver command line. This seems a little

[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.

hans added a comment. In https://reviews.llvm.org/D53457#1277950, @neerajksingh wrote: > In https://reviews.llvm.org/D53457#1277315, @hans wrote: > > > One note about flag ordering: the /clang: flags are concatenated to the > > end of > > the argument list, so in cases where the last flag wi

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

hans added a comment. I've been thinking more about your example with static locals in lambda and how this works with regular dllexport. It got me thinking more about the problem of exposing inline functions from a library. For example: `lib.h`: #ifndef LIB_H #define LIB_H int foo();

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

hans added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719 + TSK != TSK_ExplicitInstantiationDeclaration && + TSK != TSK_ExplicitInstantiationDefinition) { +if (ClassExported) { takuto.ikuta wrote: > takuto.ikuta wrote:

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

hans added a comment. >> $ g++ -fvisibility=hidden -fvisibility-inlines-hidden -fpic -shared -o >> lib.so lib.cc >> $ g++ main.cc lib.so >> /tmp/cc557J3i.o: In function `S::bar()': >> main.cc:(.text._ZN1S3barEv[_ZN1S3barEv]+0xd): undefined reference to >> `foo()' >> >> >> So this is a

[PATCH] D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496)

hans created this revision. hans added reviewers: rnk, thakis, takuto.ikuta. In the course of https://reviews.llvm.org/D51340, @takuto.ikuta discovered that Clang fails to put dllexport/import attributes on static locals during template instantiation. For regular functions, this happens in Sema

[PATCH] D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496)

This revision was automatically updated to reflect the committed changes. Closed by commit rC345699: [clang-cl] Inherit dllexport to static locals also in template instantiations… (authored by hans, committed by ). Repository: rC Clang https://reviews.llvm.org/D53870 Files: include/clang/Se

[PATCH] D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496)

hans added inline comments. Comment at: test/CodeGenCXX/dllimport.cpp:1010 +int bar() { T t; return t.foo(); } +// MO1-DAG: @"?x@?{{1|2}}??foo@?$T@H@pr39496@@Q{{[A-Z]*}}HXZ@4HA" = available_externally dllimport global i32 0, align 4 +} rnk wrote: > I notice that

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

hans added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719 + TSK != TSK_ExplicitInstantiationDeclaration && + TSK != TSK_ExplicitInstantiationDefinition) { +if (ClassExported) { hans wrote: > takuto.ikuta wrote: > > hans

  1   2   3   4   5   6   7   8   9   10   >