[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 7 release branch if we land it now. What do you think > about that? Would you prefer that we wait until after the 7.0.0 release to > make cherry-picks easier? Thanks for the heads up. It looks mechanical enough that if it does cause merge conflicts, they will be easy enough to resolve. No need to wait. Repository: rC Clang https://reviews.llvm.org/D50349 ___ 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 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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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: rL LLVM https://reviews.llvm.org/D50380 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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:
include/clang/Driver/CLCompatOptions.td
lib/CodeGen/CodeGenModule.cpp
lib/Driver/ToolChains/Clang.cpp
test/CodeGen/cfguardtable.c
test/Driver/cl-options.c
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -420,8 +420,6 @@
// RUN: /Gr \
// RUN: /GS \
// RUN: /GT \
-// RUN: /guard:cf \
-// RUN: /guard:cf- \
// RUN: /GX \
// RUN: /Gv \
// RUN: /Gz \
@@ -562,6 +560,18 @@
// RUN: %clang_cl -### -Fe%t.exe -entry:main -flto -- %s 2>&1 | FileCheck -check-prefix=LTO-WITHOUT-LLD %s
// LTO-WITHOUT-LLD: LTO requires -fuse-ld=lld
+// RUN: %clang_cl -### -- %s 2>&1 | FileCheck -check-prefix=NOCFGUARD %s
+// RUN: %clang_cl /guard:cf- -### -- %s 2>&1 | FileCheck -check-prefix=NOCFGUARD %s
+// NOCFGUARD-NOT: -guardcf
+
+// RUN: %clang_cl /guard:cf -### -- %s 2>&1 | FileCheck -check-prefix=CFGUARD %s
+// RUN: %clang_cl /guard:cf,nochecks -### -- %s 2>&1 | FileCheck -check-prefix=CFGUARD %s
+// RUN: %clang_cl /guard:nochecks -### -- %s 2>&1 | FileCheck -check-prefix=CFGUARD %s
+// CFGUARD: -cfguard
+
+// RUN: %clang_cl /guard:foo -### -- %s 2>&1 | FileCheck -check-prefix=CFGUARDINVALID %s
+// CFGUARDINVALID: invalid value 'foo' in '/guard:'
+
// Accept "core" clang options.
// (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
// RUN: %clang_cl \
Index: test/CodeGen/cfguardtable.c
===
--- /dev/null
+++ test/CodeGen/cfguardtable.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -cfguard -emit-llvm %s -o - | FileCheck %s
+
+void f() {}
+
+// Check that the cfguardtable metadata flag gets set on the module.
+// CHECK: !"cfguardtable", i32 1}
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5273,9 +5273,28 @@
CmdArgs.push_back("msvc");
}
- if (Args.hasArg(options::OPT__SLASH_Guard) &&
- Args.getLastArgValue(options::OPT__SLASH_Guard).equals_lower("cf"))
-CmdArgs.push_back("-cfguard");
+ if (Arg *A = Args.getLastArg(options::OPT__SLASH_guard)) {
+SmallVector SplitArgs;
+StringRef(A->getValue()).split(SplitArgs, ",");
+bool Instrument = false;
+bool NoChecks = false;
+for (StringRef Arg : SplitArgs) {
+ if (Arg.equals_lower("cf"))
+Instrument = true;
+ else if (Arg.equals_lower("cf-"))
+Instrument = false;
+ else if (Arg.equals_lower("nochecks"))
+NoChecks = true;
+ else if (Arg.equals_lower("nochecks-"))
+NoChecks = false;
+ else
+D.Diag(diag::err_drv_invalid_value) << A->getSpelling() << Arg;
+}
+// Currently there's no support emitting CFG instrumentation; the flag only
+// emits the table of address-taken functions.
+if (Instrument || NoChecks)
+ CmdArgs.push_back("-cfguard");
+ }
}
visualstudio::Compiler *Clang::getCLFallback() const {
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -460,7 +460,7 @@
}
if (CodeGenOpts.ControlFlowGuard) {
// We want function ID tables for Control Flow Guard.
-getModule().addModuleFlag(llvm::Module::Warning, "cfguard", 1);
+getModule().addModuleFlag(llvm::Module::Warning, "cfguardtable", 1);
}
if (CodeGenOpts.OptimizationLevel > 0 && CodeGenOpts.StrictVTablePointers) {
// We don't support LTO with 2 with different StrictVTablePointers
Index: include/clang/Driver/CLCompatOptions.td
===
--- include/clang/Driver/CLCompatOptions.td
+++ include/clang/Driver/CLCompatOptions.td
@@ -240,8 +240,8 @@
def _SLASH_Fo : CLCompileJoined<"Fo">,
HelpText<"Set output object file, or directory (ends in / or \\) (with /c)">,
MetaVarName<"">;
-def _SLASH_Guard : CLJoined<"guard:">,
- HelpText<"Enable Control Flow Guard with /guard:cf">;
+def _SLASH_guard : CLJoined<"guard:">,
+ HelpText<"Enable Control Flow Guard with /guard:cf, or only the table with /guard:cf,nochecks">;
def _SLASH_GX : CLFlag<"GX">,
HelpText<"Enable exception handling">;
def _SLASH_GX_ : CLFlag<"GX-">,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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); } I'll update the LLVM side before landing this, but I think this name better explains what's being emitted. https://reviews.llvm.org/D50513 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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
Repository:
rL LLVM
https://reviews.llvm.org/D50513
Files:
cfe/trunk/include/clang/Driver/CLCompatOptions.td
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/CodeGen/cfguardtable.c
cfe/trunk/test/Driver/cl-options.c
Index: cfe/trunk/test/CodeGen/cfguardtable.c
===
--- cfe/trunk/test/CodeGen/cfguardtable.c
+++ cfe/trunk/test/CodeGen/cfguardtable.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -cfguard -emit-llvm %s -o - | FileCheck %s
+
+void f() {}
+
+// Check that the cfguardtable metadata flag gets set on the module.
+// CHECK: !"cfguardtable", i32 1}
Index: cfe/trunk/test/Driver/cl-options.c
===
--- cfe/trunk/test/Driver/cl-options.c
+++ cfe/trunk/test/Driver/cl-options.c
@@ -420,8 +420,6 @@
// RUN: /Gr \
// RUN: /GS \
// RUN: /GT \
-// RUN: /guard:cf \
-// RUN: /guard:cf- \
// RUN: /GX \
// RUN: /Gv \
// RUN: /Gz \
@@ -562,6 +560,18 @@
// RUN: %clang_cl -### -Fe%t.exe -entry:main -flto -- %s 2>&1 | FileCheck -check-prefix=LTO-WITHOUT-LLD %s
// LTO-WITHOUT-LLD: LTO requires -fuse-ld=lld
+// RUN: %clang_cl -### -- %s 2>&1 | FileCheck -check-prefix=NOCFGUARD %s
+// RUN: %clang_cl /guard:cf- -### -- %s 2>&1 | FileCheck -check-prefix=NOCFGUARD %s
+// NOCFGUARD-NOT: -guardcf
+
+// RUN: %clang_cl /guard:cf -### -- %s 2>&1 | FileCheck -check-prefix=CFGUARD %s
+// RUN: %clang_cl /guard:cf,nochecks -### -- %s 2>&1 | FileCheck -check-prefix=CFGUARD %s
+// RUN: %clang_cl /guard:nochecks -### -- %s 2>&1 | FileCheck -check-prefix=CFGUARD %s
+// CFGUARD: -cfguard
+
+// RUN: %clang_cl /guard:foo -### -- %s 2>&1 | FileCheck -check-prefix=CFGUARDINVALID %s
+// CFGUARDINVALID: invalid value 'foo' in '/guard:'
+
// Accept "core" clang options.
// (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
// RUN: %clang_cl \
Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp
@@ -460,7 +460,7 @@
}
if (CodeGenOpts.ControlFlowGuard) {
// We want function ID tables for Control Flow Guard.
-getModule().addModuleFlag(llvm::Module::Warning, "cfguard", 1);
+getModule().addModuleFlag(llvm::Module::Warning, "cfguardtable", 1);
}
if (CodeGenOpts.OptimizationLevel > 0 && CodeGenOpts.StrictVTablePointers) {
// We don't support LTO with 2 with different StrictVTablePointers
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -5273,9 +5273,28 @@
CmdArgs.push_back("msvc");
}
- if (Args.hasArg(options::OPT__SLASH_Guard) &&
- Args.getLastArgValue(options::OPT__SLASH_Guard).equals_lower("cf"))
-CmdArgs.push_back("-cfguard");
+ if (Arg *A = Args.getLastArg(options::OPT__SLASH_guard)) {
+SmallVector SplitArgs;
+StringRef(A->getValue()).split(SplitArgs, ",");
+bool Instrument = false;
+bool NoChecks = false;
+for (StringRef Arg : SplitArgs) {
+ if (Arg.equals_lower("cf"))
+Instrument = true;
+ else if (Arg.equals_lower("cf-"))
+Instrument = false;
+ else if (Arg.equals_lower("nochecks"))
+NoChecks = true;
+ else if (Arg.equals_lower("nochecks-"))
+NoChecks = false;
+ else
+D.Diag(diag::err_drv_invalid_value) << A->getSpelling() << Arg;
+}
+// Currently there's no support emitting CFG instrumentation; the flag only
+// emits the table of address-taken functions.
+if (Instrument || NoChecks)
+ CmdArgs.push_back("-cfguard");
+ }
}
visualstudio::Compiler *Clang::getCLFallback() const {
Index: cfe/trunk/include/clang/Driver/CLCompatOptions.td
===
--- cfe/trunk/include/clang/Driver/CLCompatOptions.td
+++ cfe/trunk/include/clang/Driver/CLCompatOptions.td
@@ -240,8 +240,8 @@
def _SLASH_Fo : CLCompileJoined<"Fo">,
HelpText<"Set output object file, or directory (ends in / or \\) (with /c)">,
MetaVarName<"">;
-def _SLASH_Guard : CLJoined<"guard:">,
- HelpText<"Enable Control Flow Guard with /guard:cf">;
+def _SLASH_guard : CLJoined<"guard:">,
+ HelpText<"Enable Control Flow Guard with /guard:cf, or only the table with /guard:cf,nochecks">;
def _SLASH_GX : CLFlag<"GX">,
HelpText<"Enable exception handling">;
def _SLASH_GX_ : CLFlag<"GX-">,
[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. Should we back it out until a solution is found? Repository: rCXX libc++ https://reviews.llvm.org/D49240 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 infrastructure > > problems. > > > > This change was also committed shortly before the 7.0 branch, so it's part > > of the 7.0.0 release candidates. > > > > Should we back it out until a solution is found? > > > The thing is -- there's no solution without changing the guarantees that > libc++ make. Today, libc++ guarantees that you can link TUs that were built > with different versions of libc++ together. If we remove that guarantee, then > we can use linkonce_odr and solve the problem that Chromium is having. > > Is that guarantee something that Chromium is relying upon? I'm not sure (thakis can probably answer better), but isn't it enough to link against some system libraries that might use libc++ for this to be true? The previous solution, using inlining, while not very elegant didn't have this giant binary size problem, so maybe it was better? What should we put in the release notes, that folks should expect significantly larger binaries using the 7.0.0 version? Repository: rCXX libc++ https://reviews.llvm.org/D49240 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 solution can be found. > I think the symbol table size increase may be specific to MachO, and it may > be possible to work around this by changing ld64 to pool strings for symbols > by default. I don't know enough about MachO to say if this is possible. I looked some more at my local build. The sum of object file sizes didn't increase very much, the big growth is for the executable, and there it's mostly the string table as your analysis showed. As far as I understand, string pooling should work fine for Mach-O; LLVM does it for object files already. And yes, ld64 doesn't do it. I looked at this symbol: __ZNSt3__1eqERKNS_21__tree_const_iteratorIN7testing11ExpectationEPNS_11__tree_nodeIS2_PvEElEES9_ If I understand correctly that function would previously have been inlined everywhere, but now it occurs 20+ times in the symbol table of base_unittests, and yes it's repeated 20+ times in the string table too :-/ > I also think the symbol table size problem may be limited to "large" users of > C++: people with 500+ object files using libc++ in a DSO. I'm more > comfortable imposing a cost on these users to ask them to opt in to some > setting that disables internal_linkage and always_inline than I am leaving > always_inline on by default, which adversely affects all users. Sure, if there's a macro we can use to opt in to get the old inlining behaviour, or maybe even better getting linkonce_odr linkage, I'd be a happy camper. But the cost of just growing the executables this much seems like a real problem for our build infrastructure. Repository: rCXX libc++ 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] 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-around for large projects that don't want the size increase? Repository: rCXX libc++ 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] 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://lists.llvm.org/pipermail/llvm-dev/2018-July/124549.html. But yeah, >> Chromium could use this workaround. > > Actually, scratch that, it does work. One can either use > `-D_LIBCPP_HIDE_FROM_ABI=_LIBCPP_HIDDEN _LIBCPP_ALWAYS_INLINE` to restore the > old behavior, or `-D_LIBCPP_HIDE_FROM_ABI=` to get odr linkage. I tried `-D_LIBCPP_HIDE_FROM_ABI=` but got lots of link errors (see https://bugs.chromium.org/p/chromium/issues/detail?id=872926#c21). `-D_LIBCPP_HIDE_FROM_ABI=_LIBCPP_HIDDEN _LIBCPP_ALWAYS_INLINE` works nicely though. I also tried applying your patch and verified that works for Chromium. If I understand correctly, without setting LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT, it restores the old behaviour of force inlining. I think this makes sense for Chromium since it allows us to build without any special flags until we can get ODR linkage, and I think it also makes sense for the 7.0 branch to prevent regressing binary sizes for users of the release. lgtm Repository: rCXX libc++ 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] 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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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-commits
[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.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 Request' on > '/svn/llvm-project/!svn/act/2ca88059-6dd7-44fe-aa3e-9fc9f021f0bb' Committed r340375. Repository: rL LLVM https://reviews.llvm.org/D51069 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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/branches/release_70/docs/ReleaseNotes.rst Index: cfe/branches/release_70/docs/ReleaseNotes.rst === --- cfe/branches/release_70/docs/ReleaseNotes.rst +++ cfe/branches/release_70/docs/ReleaseNotes.rst @@ -265,7 +265,10 @@ Clang. If upgrading an external codebase that uses Clang as a library, this section should help get you past the largest hurdles of upgrading. -- ... +- The methods ``getLocStart``, ``getStartLoc`` and ``getLocEnd`` in the AST + classes are deprecated. New APIs ``getBeginLoc`` and ``getEndLoc`` should + be used instead. While the old methods remain in this release, they will + not be present in the next release of Clang. AST Matchers Index: cfe/branches/release_70/docs/ReleaseNotes.rst === --- cfe/branches/release_70/docs/ReleaseNotes.rst +++ cfe/branches/release_70/docs/ReleaseNotes.rst @@ -265,7 +265,10 @@ Clang. If upgrading an external codebase that uses Clang as a library, this section should help get you past the largest hurdles of upgrading. -- ... +- The methods ``getLocStart``, ``getStartLoc`` and ``getLocEnd`` in the AST + classes are deprecated. New APIs ``getBeginLoc`` and ``getEndLoc`` should + be used instead. While the old methods remain in this release, they will + not be present in the next release of Clang. AST Matchers ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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/mailman/listinfo/cfe-commits
[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/cfe-commits
[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 typically full > of warnings and default-error warnings. Sorry, I didn't realize this was the case :-/ Repository: rL LLVM https://reviews.llvm.org/D46520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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: rL LLVM https://reviews.llvm.org/D45045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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/cfe-commits
[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 mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 using a PCH, "
+ "skip tokens until after a #pragma hdrstop">;
def fno_pch_timestamp : Flag<["-"], "fno-pch-timestamp">,
should this mention the argument is supposed to be "create" or "use"?
Comment at: lib/Driver/Driver.cpp:2987
// Diagnose unsupported forms of /Yc /Yu. Ignore /Yc/Yu for now if:
// * no filename after it
// * both /Yc and /Yu passed but with different filenames
I think this part of the comment no longer applies.
Comment at: lib/Driver/Driver.cpp:4274
} else {
-Output = BaseName;
+Arg *YcArg = C.getArgs().getLastArg(options::OPT__SLASH_Yc);
+if (YcArg)
The declaration of YcArg could still be part of the if-statement below, I
think. It doesn't look like you're using it outside the if.
Comment at: lib/Driver/ToolChains/Clang.cpp:1112
+Twine("-pch-through-hdrstop=") + (YcArg ? "create" : "use")));
+ } else
+CmdArgs.push_back(
nit: i'd probably use a curly brace here since the statement below doesn't fit
on one line. maybe that's just me though.
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);
hmm, what if the value is not "create", but also not "use" but something else?
maybe that should be diagnosed, or maybe the flag should be split into two?
Comment at: lib/Lex/Pragma.cpp:904
+CurLexer->FormTokenWithChars(Result, CurLexer->BufferEnd, tok::eof);
+CurLexer->cutOffLexing();
+ }
Nice!
Comment at: test/Driver/cl-pch.cpp:280
+// CHECK-YC-NOARG: cl-pch.cpp
+// 2. Use .pch file: Includes ycnoarg.pch
+// CHECK-YC-NOARG: cc1
i suppose if there's a "2." comment, might as well have a comment for step 1
too. same for "/Yc with no argument and no /FP" below
https://reviews.llvm.org/D51391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 fine too. https://reviews.llvm.org/D51635 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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); mikerice wrote: > hans wrote: > > hmm, what if the value is not "create", but also not "use" but something > > else? maybe that should be diagnosed, or maybe the flag should be split > > into two? > I am not totally happy with this but I thought one option might be a little > better than two. It would be equivalent to create two options > OPT_pch_through_hdrstop_create and OPT_pch_through_hdrstop_use if that seems > better (or more usual) to everyone. I added a diagnostic if the value is not > "create" or "use". A user should never see that though if the front end and > driver as in sync. Thinking about it again, having two separate flags seems more common to me. https://reviews.llvm.org/D51391 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 dllexported or not?
> Sorry, what you want to ask?
>
> This will used to not add dllexport attr in L5690 of SemaDeclCXX.cpp.
>
Oops, I didn't see that. I'm glad to see this is looking so simple :-)
Actually, I don't think we should the same flag name for this, since "hidden"
is an ELF concept, not a COFF one, just that we should match the behaviour of
the flag.
Hmm, or do people use -fvisibility-inlines-hidden on MinGW or something? Where
does the hidden-dllimport.cpp file come from?
Also, is it the case that -fvisibility-inlines-hidden just ignores the problem
of static local variables? If that's the case we can probably do it too, we
just have to be sure, and document it eventually.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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/mailman/listinfo/cfe-commits
[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 something smart. Currently it doesn't look like they'd interact well together in x86::getX86TargetCPU Repository: rC Clang https://reviews.llvm.org/D51806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 https://reviews.llvm.org/D42762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 addStackProbeSizeTargetAttribute(const Decl *D,
- llvm::GlobalValue *GV,
- CodeGen::CodeGenModule &CGM) {
+static void addStackProbeParamsTargetAttribute(const Decl *D,
+ llvm::GlobalValue *GV,
I'd suggest perhaps "addStackProbeTargetAttributes" as a name instead, since
I'm not sure what Params is for.
Comment at: lib/CodeGen/TargetInfo.cpp:2361
+ CodeGen::CodeGenModule &CGM) {
if (D && isa(D)) {
+llvm::Function *Fn = cast(GV);
This could be written as
```
if (llvm::Function *Fn = dyn_cast_or_null(GV)) {
```
Comment at: lib/Driver/ToolChains/Clang.cpp:4038
+ if (Args.hasArg(options::OPT_mno_stack_arg_probe))
+CmdArgs.push_back("-mno-stack-arg-probe");
+
I think you can just do
```
Args.AddLastArg(CmdArgs, options::OPT_mno_stack_arg_probe)
```
which avoids the if-statement and having to spell out the flag.
Repository:
rC Clang
https://reviews.llvm.org/D43108
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 mostly works (I > actually was able to get it running) except the cases when the code contains > variable-sized arrays allocated on stacks. Unfortunately, stack-probe-size > will only help with fixed sized array but will not help to solve the problem > described in the bug description since stack usage is unknown at compile > time. MinGW does not have this problem because it provides this flag. I see, interesting. Might be worth mentioning in the commit message for others wondering what the flag is useful for. > @MatzeB : there is a test on LLVM side (related review). Do you think the > test is needed for clang side? If so, please let me know, what kind of test > it is supposed to be. Yes please, I think think there should be on in test/Driver/ to check that forwarding the flag to cc1 (and if we have a -mno-foo, there should maybe be a -mfoo variant too?), and a test in test/CodeGen/ to check that the attribute gets put on the functions correctly. Perhaps r321992 is a good example to look at. Repository: rC Clang https://reviews.llvm.org/D43108 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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/listinfo/cfe-commits
[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://reviews.llvm.org/D43108
Files:
docs/ClangCommandLineReference.rst
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/TargetInfo.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGen/stack-arg-probe.c
test/Driver/stack-arg-probe.c
Index: docs/ClangCommandLineReference.rst
===
--- docs/ClangCommandLineReference.rst
+++ docs/ClangCommandLineReference.rst
@@ -2192,6 +2192,10 @@
Set the stack probe size
+.. option:: -mstack-arg-probe, -mno-stack-arg-probe
+
+Disable stack probes
+
.. option:: -mstackrealign, -mno-stackrealign
Force realign the stack at entry to every function
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1842,6 +1842,10 @@
HelpText<"Set the stack alignment">;
def mstack_probe_size : Joined<["-"], "mstack-probe-size=">, Group, Flags<[CC1Option]>,
HelpText<"Set the stack probe size">;
+def mstack_arg_probe : Flag<["-"], "mstack-arg-probe">, Group,
+ HelpText<"Enable stack probes">;
+def mno_stack_arg_probe : Flag<["-"], "mno-stack-arg-probe">, Group, Flags<[CC1Option]>,
+ HelpText<"Disable stack probes which are enabled by default">;
def mthread_model : Separate<["-"], "mthread-model">, Group, Flags<[CC1Option]>,
HelpText<"The thread model to use, e.g. posix, single (posix by default)">, Values<"posix,single">;
def meabi : Separate<["-"], "meabi">, Group, Flags<[CC1Option]>,
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -227,6 +227,7 @@
///< alignment, if not 0.
VALUE_CODEGENOPT(StackProbeSize, 32, 4096) ///< Overrides default stack
///< probe size, even if 0.
+CODEGENOPT(NoStackArgProbe, 1, 0) ///< Set when -mno-stack-arg-probe is used
CODEGENOPT(DebugColumnInfo, 1, 0) ///< Whether or not to use column information
///< in debug info.
Index: test/CodeGen/stack-arg-probe.c
===
--- test/CodeGen/stack-arg-probe.c
+++ test/CodeGen/stack-arg-probe.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -triple=i686-windows-msvc -emit-llvm -o - -mno-stack-arg-probe | FileCheck %s -check-prefix=NO-STACKPROBE
+// RUN: %clang_cc1 %s -triple=i686-windows-msvc -emit-llvm -o - | FileCheck %s -check-prefix=STACKPROBE
+
+// NO-STACKPROBE: attributes #{{[0-9]+}} = {{{.*}} "no-stack-arg-probe"
+// STACKPROBE-NOT: attributes #{{[0-9]+}} = {{{.*}} "no-stack-arg-probe"
+
+void test1() {
+}
Index: test/Driver/stack-arg-probe.c
===
--- test/Driver/stack-arg-probe.c
+++ test/Driver/stack-arg-probe.c
@@ -0,0 +1,7 @@
+// RUN: %clang -### %s 2>&1 | FileCheck %s -check-prefix=STACKPROBE
+// RUN: %clang -### -mno-stack-arg-probe -mstack-arg-probe %s 2>&1 | FileCheck %s -check-prefix=STACKPROBE
+// RUN: %clang -### -mstack-arg-probe -mno-stack-arg-probe %s 2>&1 | FileCheck %s -check-prefix=NO-STACKPROBE
+// REQUIRES: clang-driver
+
+// NO-STACKPROBE: -mno-stack-arg-probe
+// STACKPROBE-NOT: -mno-stack-arg-probe
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4047,6 +4047,10 @@
CmdArgs.push_back("-mstack-probe-size=0");
}
+ if (!Args.hasFlag(options::OPT_mstack_arg_probe,
+options::OPT_mno_stack_arg_probe, true))
+CmdArgs.push_back(Args.MakeArgString("-mno-stack-arg-probe"));
+
if (Arg *A = Args.getLastArg(options::OPT_mrestrict_it,
options::OPT_mno_restrict_it)) {
if (A->getOption().matches(options::OPT_mrestrict_it)) {
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -923,6 +923,8 @@
Opts.StackProbeSize = StackProbeSize;
}
+ Opts.NoStackArgProbe = Args.hasArg(OPT_mno_stack_arg_probe);
+
if (Arg *A = Args.getLastArg(OPT_fobjc_dispatch_method_EQ)) {
StringRef Name = A->getValue();
unsigned Method = llvm::StringSwitch(Name)
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
[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-commits
[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 into a context where BitPos is constant or unsigned. https://reviews.llvm.org/D33616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 local patches that might cause this problem? In any case, I don't think Tom is taking more patches for the 4.0.1 release. https://reviews.llvm.org/D34279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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(ExecutablePath) == ExecutablePath) What if "clang" is in the current directory? Presumably that should then be preferred over PATH.. will we get that right with this patch? Another way would be to check access() on Argv0, which is what we do when trying to execute anyway. Is there any downside to that? Repository: rL LLVM https://reviews.llvm.org/D34290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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/mailman/listinfo/cfe-commits
[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 narrowing conversion? I'm just trying to understand why we need this. https://reviews.llvm.org/D34279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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
narrowing conversion.
I did a fresh checkout of LLVM and built with VS 19.00.2415.1 for x64:
> svn export http://llvm.org/svn/llvm-project/llvm/trunk llvmtest
> mkdir llvmtest\build
> cd llvmtest\build
> cmake -GNinja -DCMAKE_BUILD_TYPE=Release ..
-- The C compiler identification is MSVC 19.0.24215.1
-- The CXX compiler identification is MSVC 19.0.24215.1
-- The ASM compiler identification is MSVC
...
> ninja
...
And everything built fine.
There must be something different with your configuration.
https://reviews.llvm.org/D34279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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/mailman/listinfo/cfe-commits
[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/Basic/DiagnosticParseKinds.td
include/clang/Parse/Parser.h
lib/Parse/ParsePragma.cpp
test/Preprocessor/pragma_microsoft.c
Index: test/Preprocessor/pragma_microsoft.c
===
--- test/Preprocessor/pragma_microsoft.c
+++ test/Preprocessor/pragma_microsoft.c
@@ -190,3 +190,11 @@
#pragma intrinsic(asdf) // no-warning
#pragma clang diagnostic pop
#pragma intrinsic(asdf) // expected-warning {{'asdf' is not a recognized builtin; consider including }}
+
+#pragma optimize // expected-warning{{missing '(' after '#pragma optimize'}}
+#pragma optimize( // expected-warning{{expected string literal in '#pragma optimize'}}
+#pragma optimize(a// expected-warning{{expected string literal in '#pragma optimize'}}
+#pragma optimize("g" // expected-warning{{expected ',' in '#pragma optimize'}}
+#pragma optimize("g", // expected-warning{{missing argument to '#pragma optimize'; expected 'on' or 'off'}}
+#pragma optimize("g",xyz // expected-warning{{unexpected argument 'xyz' to '#pragma optimize'; expected 'on' or 'off'}}
+#pragma optimize("g",on) // expected-warning{{#pragma optimize' is not supported; use '#pragma clang optimize on|off' instead}}
Index: lib/Parse/ParsePragma.cpp
===
--- lib/Parse/ParsePragma.cpp
+++ lib/Parse/ParsePragma.cpp
@@ -220,6 +220,12 @@
Token &FirstToken) override;
};
+struct PragmaMSOptimizeHandler : public PragmaHandler {
+ PragmaMSOptimizeHandler() : PragmaHandler("optimize") {}
+ void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+Token &FirstToken) override;
+};
+
struct PragmaForceCUDAHostDeviceHandler : public PragmaHandler {
PragmaForceCUDAHostDeviceHandler(Sema &Actions)
: PragmaHandler("force_cuda_host_device"), Actions(Actions) {}
@@ -324,6 +330,8 @@
PP.AddPragmaHandler(MSRuntimeChecks.get());
MSIntrinsic.reset(new PragmaMSIntrinsicHandler());
PP.AddPragmaHandler(MSIntrinsic.get());
+MSOptimize.reset(new PragmaMSOptimizeHandler());
+PP.AddPragmaHandler(MSOptimize.get());
}
if (getLangOpts().CUDA) {
@@ -410,6 +418,8 @@
MSRuntimeChecks.reset();
PP.RemovePragmaHandler(MSIntrinsic.get());
MSIntrinsic.reset();
+PP.RemovePragmaHandler(MSOptimize.get());
+MSOptimize.reset();
}
if (getLangOpts().CUDA) {
@@ -2949,6 +2959,61 @@
PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
<< "intrinsic";
}
+
+// #pragma optimize("gsty", on|off)
+void PragmaMSOptimizeHandler::HandlePragma(Preprocessor &PP,
+ PragmaIntroducerKind Introducer,
+ Token &Tok) {
+ SourceLocation StartLoc = Tok.getLocation();
+ PP.Lex(Tok);
+
+ if (Tok.isNot(tok::l_paren)) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_lparen) << "optimize";
+return;
+ }
+ PP.Lex(Tok);
+
+ if (Tok.isNot(tok::string_literal)) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_string) << "optimize";
+return;
+ }
+ // We could syntax check the string but it's probably not worth the effort.
+ PP.Lex(Tok);
+
+ if (Tok.isNot(tok::comma)) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_comma) << "optimize";
+return;
+ }
+ PP.Lex(Tok);
+
+ if (Tok.is(tok::eod) || Tok.is(tok::r_paren)) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_missing_argument)
+<< "optimize" << /*Expected=*/true << "'on' or 'off'";
+return;
+ }
+ IdentifierInfo *II = Tok.getIdentifierInfo();
+ if (!II || (!II->isStr("on") && !II->isStr("off"))) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_invalid_argument)
+<< PP.getSpelling(Tok) << "optimize" << /*Expected=*/true
+<< "'on' or 'off'";
+return;
+ }
+ PP.Lex(Tok);
+
+ if (Tok.isNot(tok::r_paren)) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_rparen) << "optimize";
+return;
+ }
+ PP.Lex(Tok);
+
+ if (Tok.isNot(tok::eod)) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
+<< "optimize";
+return;
+ }
+ PP.Diag(StartLoc, diag::warn_pragma_optimize);
+}
+
void PragmaForceCUDAHostDeviceHandler::HandlePragma(
Preprocessor &PP, PragmaIntroducerKind Introducer, Token &Tok) {
Token FirstTok = Tok;
Index: include/clang/Parse/Parser.h
===
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -179,6 +179,7 @@
std::unique_ptr MSSection;
std::unique_ptr MSRuntimeChecks;
std::unique_ptr MSIn
[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 commit:
https://reviews.llvm.org/D44630?vs=138940&id=139087#toc
Repository:
rL LLVM
https://reviews.llvm.org/D44630
Files:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/lib/Parse/ParsePragma.cpp
cfe/trunk/test/Preprocessor/pragma_microsoft.c
Index: cfe/trunk/test/Preprocessor/pragma_microsoft.c
===
--- cfe/trunk/test/Preprocessor/pragma_microsoft.c
+++ cfe/trunk/test/Preprocessor/pragma_microsoft.c
@@ -190,3 +190,11 @@
#pragma intrinsic(asdf) // no-warning
#pragma clang diagnostic pop
#pragma intrinsic(asdf) // expected-warning {{'asdf' is not a recognized builtin; consider including }}
+
+#pragma optimize // expected-warning{{missing '(' after '#pragma optimize'}}
+#pragma optimize( // expected-warning{{expected string literal in '#pragma optimize'}}
+#pragma optimize(a// expected-warning{{expected string literal in '#pragma optimize'}}
+#pragma optimize("g" // expected-warning{{expected ',' in '#pragma optimize'}}
+#pragma optimize("g", // expected-warning{{missing argument to '#pragma optimize'; expected 'on' or 'off'}}
+#pragma optimize("g",xyz // expected-warning{{unexpected argument 'xyz' to '#pragma optimize'; expected 'on' or 'off'}}
+#pragma optimize("g",on) // expected-warning{{#pragma optimize' is not supported}}
Index: cfe/trunk/lib/Parse/ParsePragma.cpp
===
--- cfe/trunk/lib/Parse/ParsePragma.cpp
+++ cfe/trunk/lib/Parse/ParsePragma.cpp
@@ -220,6 +220,12 @@
Token &FirstToken) override;
};
+struct PragmaMSOptimizeHandler : public PragmaHandler {
+ PragmaMSOptimizeHandler() : PragmaHandler("optimize") {}
+ void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+Token &FirstToken) override;
+};
+
struct PragmaForceCUDAHostDeviceHandler : public PragmaHandler {
PragmaForceCUDAHostDeviceHandler(Sema &Actions)
: PragmaHandler("force_cuda_host_device"), Actions(Actions) {}
@@ -324,6 +330,8 @@
PP.AddPragmaHandler(MSRuntimeChecks.get());
MSIntrinsic.reset(new PragmaMSIntrinsicHandler());
PP.AddPragmaHandler(MSIntrinsic.get());
+MSOptimize.reset(new PragmaMSOptimizeHandler());
+PP.AddPragmaHandler(MSOptimize.get());
}
if (getLangOpts().CUDA) {
@@ -410,6 +418,8 @@
MSRuntimeChecks.reset();
PP.RemovePragmaHandler(MSIntrinsic.get());
MSIntrinsic.reset();
+PP.RemovePragmaHandler(MSOptimize.get());
+MSOptimize.reset();
}
if (getLangOpts().CUDA) {
@@ -2949,6 +2959,61 @@
PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
<< "intrinsic";
}
+
+// #pragma optimize("gsty", on|off)
+void PragmaMSOptimizeHandler::HandlePragma(Preprocessor &PP,
+ PragmaIntroducerKind Introducer,
+ Token &Tok) {
+ SourceLocation StartLoc = Tok.getLocation();
+ PP.Lex(Tok);
+
+ if (Tok.isNot(tok::l_paren)) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_lparen) << "optimize";
+return;
+ }
+ PP.Lex(Tok);
+
+ if (Tok.isNot(tok::string_literal)) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_string) << "optimize";
+return;
+ }
+ // We could syntax check the string but it's probably not worth the effort.
+ PP.Lex(Tok);
+
+ if (Tok.isNot(tok::comma)) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_comma) << "optimize";
+return;
+ }
+ PP.Lex(Tok);
+
+ if (Tok.is(tok::eod) || Tok.is(tok::r_paren)) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_missing_argument)
+<< "optimize" << /*Expected=*/true << "'on' or 'off'";
+return;
+ }
+ IdentifierInfo *II = Tok.getIdentifierInfo();
+ if (!II || (!II->isStr("on") && !II->isStr("off"))) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_invalid_argument)
+<< PP.getSpelling(Tok) << "optimize" << /*Expected=*/true
+<< "'on' or 'off'";
+return;
+ }
+ PP.Lex(Tok);
+
+ if (Tok.isNot(tok::r_paren)) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_rparen) << "optimize";
+return;
+ }
+ PP.Lex(Tok);
+
+ if (Tok.isNot(tok::eod)) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
+<< "optimize";
+return;
+ }
+ PP.Diag(StartLoc, diag::warn_pragma_optimize);
+}
+
void PragmaForceCUDAHostDeviceHandler::HandlePragma(
Preprocessor &PP, PragmaIntroducerKind Introducer, Token &Tok) {
Token FirstTok = Tok;
Index: cfe/trunk/include/
[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; thakis wrote: > Is `pragma clang optimize` really what we want to recommend here? `pragma > optimize` is used in practice mostly to work around cl.exe compiler bugs, or > to disable inlining. In neither case, `pragma clang optimize` is what you'd > really want to use. Maybe just omit everything after ; and instead add a > blurb about this in DiagnosticDocs.td ? I'll drop the suggestion for now. Turns out it was not so easy to get a blurb into DiagnosticDocs or DiagnosticsReference. They're auto-generated but don't seem to have a good way to enter extra documentation. I'll just leave it for now. https://reviews.llvm.org/D44630 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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
===
--- test/CodeGenCXX/mangle-ms-cxx11.cpp
+++ test/CodeGenCXX/mangle-ms-cxx11.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o -
-triple=i386-pc-win32 -fms-compatibility-version=19.00 | FileCheck %s
--check-prefix=CHECK --check-prefix=MSVC2015
// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o -
-triple=i386-pc-win32 -fms-compatibility-version=18.00 | FileCheck %s
--check-prefix=CHECK --check-prefix=MSVC2013
+// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o -
-triple=i386-pc-win32 -gcodeview -debug-info-kind=limited | FileCheck %s
--check-prefix=DBG
namespace FTypeWithQuals {
template
@@ -350,3 +351,9 @@
void f(decltype(enumerator)) {}
// CHECK-DAG: define internal void @"?f@@YAXW4@@@Z"(
void use_f() { f(enumerator); }
+
+namespace pr37723 {
+struct s { enum {}; };
+// DBG-DAG: DW_TAG_enumeration_type{{.*}}identifier:
".?AW4@s@pr37723@@"
+s x;
+}
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -886,9 +886,12 @@
Name += TND->getName();
} else if (auto *ED = dyn_cast(TD)) {
auto EnumeratorI = ED->enumerator_begin();
-assert(EnumeratorI != ED->enumerator_end());
-Name += "getName();
+if (EnumeratorI == ED->enumerator_end()) {
+ Name += "getName();
+}
} else {
// Otherwise, number the types using a $S prefix.
Name += "Index: test/CodeGenCXX/mangle-ms-cxx11.cpp
===
--- test/CodeGenCXX/mangle-ms-cxx11.cpp
+++ test/CodeGenCXX/mangle-ms-cxx11.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o - -triple=i386-pc-win32 -fms-compatibility-version=19.00 | FileCheck %s --check-prefix=CHECK --check-prefix=MSVC2015
// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o - -triple=i386-pc-win32 -fms-compatibility-version=18.00 | FileCheck %s --check-prefix=CHECK --check-prefix=MSVC2013
+// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o - -triple=i386-pc-win32 -gcodeview -debug-info-kind=limited | FileCheck %s --check-prefix=DBG
namespace FTypeWithQuals {
template
@@ -350,3 +351,9 @@
void f(decltype(enumerator)) {}
// CHECK-DAG: define internal void @"?f@@YAXW4@@@Z"(
void use_f() { f(enumerator); }
+
+namespace pr37723 {
+struct s { enum {}; };
+// DBG-DAG: DW_TAG_enumeration_type{{.*}}identifier: ".?AW4@s@pr37723@@"
+s x;
+}
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -886,9 +886,12 @@
Name += TND->getName();
} else if (auto *ED = dyn_cast(TD)) {
auto EnumeratorI = ED->enumerator_begin();
-assert(EnumeratorI != ED->enumerator_end());
-Name += "getName();
+if (EnumeratorI == ED->enumerator_end()) {
+ Name += "getName();
+}
} else {
// Otherwise, number the types using a $S prefix.
Name += "___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 NSInteger, and given that you're targeting a specific wide-spread pattern maybe that's the right thing to do, I think we should make -Wformat accept (move the warning behind -Wformat-pedantic I suppose) printing NSInteger with *any* integral type of the right size, not just size_t. Also, I haven't looked at what happens on the scanf side. Does that need an equivalent change? Repository: rC Clang https://reviews.llvm.org/D47290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 > > do, I think we should make -Wformat accept (move the warning behind > > -Wformat-pedantic I suppose) printing NSInteger with *any* integral type of > > the right size, not just size_t. > > > Would you be similarly okay with %ld and %d on Windows platforms when mixing > up int and long? No, I'm against a general relaxation of -Wformat, but to solve JF's problem I think special-casing NSInteger might be reasonable. Repository: rC Clang https://reviews.llvm.org/D47290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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: > > > > > > > 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 do, I think we should make -Wformat accept (move the warning behind > > > > -Wformat-pedantic I suppose) printing NSInteger with *any* integral > > > > type of the right size, not just size_t. > > > > > > > > > Would you be similarly okay with %ld and %d on Windows platforms when > > > mixing up int and long? > > > > > > No, I'm against a general relaxation of -Wformat, but to solve JF's problem > > I think special-casing NSInteger might be reasonable. > > > How is JF's problem different? It concerns a vendor-specific type. Of course I personally think it would be better if the code could be fixed, but it doesn't sound like that's an option so then I think special-casing for NSInteger is an acceptable solution. Repository: rC Clang https://reviews.llvm.org/D47290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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()) {
+ Name += " Thinking about it some more, it'd be better if we handled this like the else
> case:
> Name += " Name += llvm::utostr(Context.getAnonymousStructId(TD) + 1);
>
> Reason being that something like:
> struct S {
> enum {};
> enum {};
> };
>
> Would otherwise end up with two identical mangles.
>
> This would also make us more consistent with other mangles, for example:
> enum {} x;
> struct {} y;
>
> These are mangled as: and .
Okay, yeah that makes sense.
I just checked
struct S {
enum {};
enum {};
};
with MSVC, and they seem to give them identical mangling.
I have to head out, but will update the patch tomorrow morning unless you're
keen to do it :-)
https://reviews.llvm.org/D47875
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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
===
--- test/CodeGenCXX/mangle-ms-cxx11.cpp
+++ test/CodeGenCXX/mangle-ms-cxx11.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o -
-triple=i386-pc-win32 -fms-compatibility-version=19.00 | FileCheck %s
--check-prefix=CHECK --check-prefix=MSVC2015
// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o -
-triple=i386-pc-win32 -fms-compatibility-version=18.00 | FileCheck %s
--check-prefix=CHECK --check-prefix=MSVC2013
+// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o -
-triple=i386-pc-win32 -gcodeview -debug-info-kind=limited | FileCheck %s
--check-prefix=DBG
namespace FTypeWithQuals {
template
@@ -350,3 +351,10 @@
void f(decltype(enumerator)) {}
// CHECK-DAG: define internal void @"?f@@YAXW4@@@Z"(
void use_f() { f(enumerator); }
+
+namespace pr37723 {
+struct s { enum {}; enum {}; };
+// DBG-DAG: DW_TAG_enumeration_type{{.*}}identifier:
".?AW4@s@pr37723@@"
+// DBG-DAG: DW_TAG_enumeration_type{{.*}}identifier:
".?AW4@s@pr37723@@"
+s x;
+}
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -884,11 +884,13 @@
// associate typedef mangled in if they have one.
Name += "getName();
- } else if (auto *ED = dyn_cast(TD)) {
-auto EnumeratorI = ED->enumerator_begin();
-assert(EnumeratorI != ED->enumerator_end());
+ } else if (isa(TD) &&
+ cast(TD)->enumerator_begin() !=
+ cast(TD)->enumerator_end()) {
+// Anonymous non-empty enums mangle in the first enumerator.
+auto *ED = cast(TD);
Name += "getName();
+Name += ED->enumerator_begin()->getName();
} else {
// Otherwise, number the types using a $S prefix.
Name += "Index: test/CodeGenCXX/mangle-ms-cxx11.cpp
===
--- test/CodeGenCXX/mangle-ms-cxx11.cpp
+++ test/CodeGenCXX/mangle-ms-cxx11.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o - -triple=i386-pc-win32 -fms-compatibility-version=19.00 | FileCheck %s --check-prefix=CHECK --check-prefix=MSVC2015
// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o - -triple=i386-pc-win32 -fms-compatibility-version=18.00 | FileCheck %s --check-prefix=CHECK --check-prefix=MSVC2013
+// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o - -triple=i386-pc-win32 -gcodeview -debug-info-kind=limited | FileCheck %s --check-prefix=DBG
namespace FTypeWithQuals {
template
@@ -350,3 +351,10 @@
void f(decltype(enumerator)) {}
// CHECK-DAG: define internal void @"?f@@YAXW4@@@Z"(
void use_f() { f(enumerator); }
+
+namespace pr37723 {
+struct s { enum {}; enum {}; };
+// DBG-DAG: DW_TAG_enumeration_type{{.*}}identifier: ".?AW4@s@pr37723@@"
+// DBG-DAG: DW_TAG_enumeration_type{{.*}}identifier: ".?AW4@s@pr37723@@"
+s x;
+}
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -884,11 +884,13 @@
// associate typedef mangled in if they have one.
Name += "getName();
- } else if (auto *ED = dyn_cast(TD)) {
-auto EnumeratorI = ED->enumerator_begin();
-assert(EnumeratorI != ED->enumerator_end());
+ } else if (isa(TD) &&
+ cast(TD)->enumerator_begin() !=
+ cast(TD)->enumerator_end()) {
+// Anonymous non-empty enums mangle in the first enumerator.
+auto *ED = cast(TD);
Name += "getName();
+Name += ED->enumerator_begin()->getName();
} else {
// Otherwise, number the types using a $S prefix.
Name += "___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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=150677#toc
Repository:
rL LLVM
https://reviews.llvm.org/D47875
Files:
cfe/trunk/lib/AST/MicrosoftMangle.cpp
cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp
Index: cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp
===
--- cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp
+++ cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o -
-triple=i386-pc-win32 -fms-compatibility-version=19.00 | FileCheck %s
--check-prefix=CHECK --check-prefix=MSVC2015
// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o -
-triple=i386-pc-win32 -fms-compatibility-version=18.00 | FileCheck %s
--check-prefix=CHECK --check-prefix=MSVC2013
+// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o -
-triple=i386-pc-win32 -gcodeview -debug-info-kind=limited | FileCheck %s
--check-prefix=DBG
namespace FTypeWithQuals {
template
@@ -350,3 +351,10 @@
void f(decltype(enumerator)) {}
// CHECK-DAG: define internal void @"?f@@YAXW4@@@Z"(
void use_f() { f(enumerator); }
+
+namespace pr37723 {
+struct s { enum {}; enum {}; };
+// DBG-DAG: DW_TAG_enumeration_type{{.*}}identifier:
".?AW4@s@pr37723@@"
+// DBG-DAG: DW_TAG_enumeration_type{{.*}}identifier:
".?AW4@s@pr37723@@"
+s x;
+}
Index: cfe/trunk/lib/AST/MicrosoftMangle.cpp
===
--- cfe/trunk/lib/AST/MicrosoftMangle.cpp
+++ cfe/trunk/lib/AST/MicrosoftMangle.cpp
@@ -884,11 +884,13 @@
// associate typedef mangled in if they have one.
Name += "getName();
- } else if (auto *ED = dyn_cast(TD)) {
-auto EnumeratorI = ED->enumerator_begin();
-assert(EnumeratorI != ED->enumerator_end());
+ } else if (isa(TD) &&
+ cast(TD)->enumerator_begin() !=
+ cast(TD)->enumerator_end()) {
+// Anonymous non-empty enums mangle in the first enumerator.
+auto *ED = cast(TD);
Name += "getName();
+Name += ED->enumerator_begin()->getName();
} else {
// Otherwise, number the types using a $S prefix.
Name += "Index: cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp
===
--- cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp
+++ cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o - -triple=i386-pc-win32 -fms-compatibility-version=19.00 | FileCheck %s --check-prefix=CHECK --check-prefix=MSVC2015
// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o - -triple=i386-pc-win32 -fms-compatibility-version=18.00 | FileCheck %s --check-prefix=CHECK --check-prefix=MSVC2013
+// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o - -triple=i386-pc-win32 -gcodeview -debug-info-kind=limited | FileCheck %s --check-prefix=DBG
namespace FTypeWithQuals {
template
@@ -350,3 +351,10 @@
void f(decltype(enumerator)) {}
// CHECK-DAG: define internal void @"?f@@YAXW4@@@Z"(
void use_f() { f(enumerator); }
+
+namespace pr37723 {
+struct s { enum {}; enum {}; };
+// DBG-DAG: DW_TAG_enumeration_type{{.*}}identifier: ".?AW4@s@pr37723@@"
+// DBG-DAG: DW_TAG_enumeration_type{{.*}}identifier: ".?AW4@s@pr37723@@"
+s x;
+}
Index: cfe/trunk/lib/AST/MicrosoftMangle.cpp
===
--- cfe/trunk/lib/AST/MicrosoftMangle.cpp
+++ cfe/trunk/lib/AST/MicrosoftMangle.cpp
@@ -884,11 +884,13 @@
// associate typedef mangled in if they have one.
Name += "getName();
- } else if (auto *ED = dyn_cast(TD)) {
-auto EnumeratorI = ED->enumerator_begin();
-assert(EnumeratorI != ED->enumerator_end());
+ } else if (isa(TD) &&
+ cast(TD)->enumerator_begin() !=
+ cast(TD)->enumerator_end()) {
+// Anonymous non-empty enums mangle in the first enumerator.
+auto *ED = cast(TD);
Name += "getName();
+Name += ED->enumerator_begin()->getName();
} else {
// Otherwise, number the types using a $S prefix.
Name += "___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 the current state of not having the intrinsics at all. Repository: rC Clang https://reviews.llvm.org/D47672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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--initializer-syntax This and WebKit's style seem like compelling arguments to support this option. klimek, djasper: Do you have any objections against landing this? Repository: rC Clang https://reviews.llvm.org/D46024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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/mailman/listinfo/cfe-commits
[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/listinfo/cfe-commits
[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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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=150984&id=151308#toc
Repository:
rL LLVM
https://reviews.llvm.org/D46024
Files:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -1495,6 +1495,17 @@
/// \endcode
bool SpaceBeforeAssignmentOperators;
+ /// If ``true``, a space will be inserted before a C++11 braced list
+ /// used to initialize an object (after the preceding identifier or type).
+ /// \code
+ ///true: false:
+ ///Foo foo { bar }; vs. Foo foo{ bar };
+ ///Foo {};Foo{};
+ ///vector { 1, 2, 3 }; vector{ 1, 2, 3 };
+ ///new int[3] { 1, 2, 3 };new int[3]{ 1, 2, 3 };
+ /// \endcode
+ bool SpaceBeforeCpp11BracedList;
+
/// If ``false``, spaces will be removed before constructor initializer
/// colon.
/// \code
@@ -1738,6 +1749,7 @@
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
SpaceAfterTemplateKeyword == R.SpaceAfterTemplateKeyword &&
SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators &&
+ SpaceBeforeCpp11BracedList == R.SpaceBeforeCpp11BracedList &&
SpaceBeforeCtorInitializerColon ==
R.SpaceBeforeCtorInitializerColon &&
SpaceBeforeInheritanceColon == R.SpaceBeforeInheritanceColon &&
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2548,6 +2548,9 @@
if (Style.isCpp()) {
if (Left.is(tok::kw_operator))
return Right.is(tok::coloncolon);
+if (Right.is(tok::l_brace) && Right.BlockKind == BK_BracedInit &&
+!Left.opensScope() && Style.SpaceBeforeCpp11BracedList)
+ return true;
} else if (Style.Language == FormatStyle::LK_Proto ||
Style.Language == FormatStyle::LK_TextProto) {
if (Right.is(tok::period) &&
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -449,6 +449,8 @@
Style.SpaceAfterTemplateKeyword);
IO.mapOptional("SpaceBeforeAssignmentOperators",
Style.SpaceBeforeAssignmentOperators);
+IO.mapOptional("SpaceBeforeCpp11BracedList",
+ Style.SpaceBeforeCpp11BracedList);
IO.mapOptional("SpaceBeforeCtorInitializerColon",
Style.SpaceBeforeCtorInitializerColon);
IO.mapOptional("SpaceBeforeInheritanceColon",
@@ -697,6 +699,7 @@
LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
LLVMStyle.SpaceBeforeRangeBasedForLoopColon = true;
LLVMStyle.SpaceBeforeAssignmentOperators = true;
+ LLVMStyle.SpaceBeforeCpp11BracedList = false;
LLVMStyle.SpacesInAngles = false;
LLVMStyle.PenaltyBreakAssignment = prec::Assignment;
@@ -892,6 +895,7 @@
Style.ObjCBlockIndentWidth = 4;
Style.ObjCSpaceAfterProperty = true;
Style.PointerAlignment = FormatStyle::PAS_Left;
+ Style.SpaceBeforeCpp11BracedList = true;
return Style;
}
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -7019,6 +7019,11 @@
" { \"c\", 2 }\n"
"};",
ExtraSpaces);
+
+ FormatStyle SpaceBeforeBrace = getLLVMStyle();
+ SpaceBeforeBrace.SpaceBeforeCpp11BracedList = true;
+ verifyFormat("vector x {1, 2, 3, 4};", SpaceBeforeBrace);
+ verifyFormat("f({}, {{}, {}}, MyMap[{k, v}]);", SpaceBeforeBrace);
}
TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
@@ -10622,6 +10627,7 @@
CHECK_PARSE_BOOL(SpaceAfterCStyleCast);
CHECK_PARSE_BOOL(SpaceAfterTemplateKeyword);
CHECK_PARSE_BOOL(SpaceBeforeAssignmentOperators);
+ CHECK_PARSE_BOOL(SpaceBeforeCpp11BracedList);
CHECK_PARSE_BOOL(SpaceBeforeCtorInitializerColon);
CHECK_PARSE_BOOL(SpaceBeforeInheritanceColon);
CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon);
Index: cfe/trunk/docs/ClangFormatStyleOptions.rst
===
--- cfe/trunk/docs/ClangFormatStyleOptions.rst
+++
[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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 definitions
of inline dllexport functions, and because they are emitted in this object
file, other files using the PCH do not need to emit them. See the bug for an
example.
This patch makes clang-cl match MSVC's behaviour in this regard, causing
significant compile-time savings when building dlls using precompiled headers.
For example, in a 64-bit optimized shared library build of Chromium with PCH,
it reduces the binary size and compile time of stroke_opacity_custom.obj from
9315564 bytes to 3659629 bytes and 14.6 to 6.63 s. The wall-clock time of
building blink_core.dll goes from 38m41s to 22m33s. ("user" time goes from
1979m to 1142m).
https://reviews.llvm.org/D48426
Files:
include/clang/AST/ASTContext.h
include/clang/AST/ExternalASTSource.h
include/clang/Driver/CC1Options.td
include/clang/Frontend/FrontendOptions.h
include/clang/Sema/MultiplexExternalSemaSource.h
include/clang/Serialization/ASTBitCodes.h
include/clang/Serialization/ASTReader.h
include/clang/Serialization/ASTWriter.h
include/clang/Serialization/Module.h
lib/AST/ASTContext.cpp
lib/Driver/Driver.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInstance.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/FrontendActions.cpp
lib/Sema/MultiplexExternalSemaSource.cpp
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTWriter.cpp
lib/Serialization/GeneratePCH.cpp
test/CodeGen/pch-dllexport.cpp
test/Driver/cl-pch.cpp
Index: test/Driver/cl-pch.cpp
===
--- test/Driver/cl-pch.cpp
+++ test/Driver/cl-pch.cpp
@@ -7,13 +7,15 @@
// 1. Build .pch file.
// CHECK-YC: cc1
// CHECK-YC: -emit-pch
+// CHECK-YC: -building-pch-with-obj
// CHECK-YC: -o
// CHECK-YC: pchfile.pch
// CHECK-YC: -x
// CHECK-YC: "c++-header"
// 2. Use .pch file.
// CHECK-YC: cc1
// CHECK-YC: -emit-obj
+// CHECK-YC: -building-pch-with-obj
// CHECK-YC: -include-pch
// CHECK-YC: pchfile.pch
@@ -24,11 +26,13 @@
// 1. Build .pch file.
// CHECK-YCO: cc1
// CHECK-YCO: -emit-pch
+// CHECK-YCO: -building-pch-with-obj
// CHECK-YCO: -o
// CHECK-YCO: pchfile.pch
// 2. Use .pch file.
// CHECK-YCO: cc1
// CHECK-YCO: -emit-obj
+// CHECK-YCO: -building-pch-with-obj
// CHECK-YCO: -include-pch
// CHECK-YCO: pchfile.pch
// CHECK-YCO: -o
@@ -46,6 +50,7 @@
// RUN: | FileCheck -check-prefix=CHECK-YU %s
// Use .pch file, but don't build it.
// CHECK-YU-NOT: -emit-pch
+// CHECK-YU-NOT: -building-pch-with-obj
// CHECK-YU: cc1
// CHECK-YU: -emit-obj
// CHECK-YU: -include-pch
@@ -63,6 +68,7 @@
// 1. Build .pch file.
// CHECK-YC-YU: cc1
// CHECK-YC-YU: -emit-pch
+// CHECK-YC-YU: -building-pch-with-obj
// CHECK-YC-YU: -o
// CHECK-YC-YU: pchfile.pch
// 2. Use .pch file.
Index: test/CodeGen/pch-dllexport.cpp
===
--- /dev/null
+++ test/CodeGen/pch-dllexport.cpp
@@ -0,0 +1,25 @@
+// Build PCH without object file, then use it.
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-pch -o %t %s
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-obj -emit-llvm -include-pch %t -o - %s | FileCheck -check-prefix=PCH %s
+
+// Build PCH with object file, then use it.
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-pch -building-pch-with-obj -o %t %s
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-obj -emit-llvm -include-pch %t -building-pch-with-obj -o - %s | FileCheck -check-prefix=OBJ %s
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-obj -emit-llvm -include-pch %t -o - %s | FileCheck -check-prefix=PCHWITHOBJ %s
+
+#ifndef IN_HEADER
+#define IN_HEADER
+
+inline void __declspec(dllexport) foo() {}
+// OBJ: define weak_odr dso_local dllexport void @"?foo@@YAXXZ"
+// PCH: define weak_odr dso_local dllexport void @"?foo@@YAXXZ"
+// PCHWITHOBJ-NOT: define weak_odr dso_local dllexport void @"?foo@@YAXXZ"
+
+struct __declspec(dllexport) S {
+ void bar() {}
+// OBJ: define weak_odr dso_local dllexport x86_thiscallcc void @"?bar@S@@QAEXXZ"
+// PCH: define weak_odr dso_local dllexport x86_thiscallcc void @"?bar@S@@QAEXXZ"
+// PCHWITHOBJ-NOT: define weak_odr dso_local dllexport x86_thiscallcc void @"?bar@S@@QAEXXZ"
+};
+
+#endif
Index: lib/Serialization/GeneratePCH.cpp
===
--- lib/Serialization/GeneratePCH.cpp
+++ lib/Serialization/GeneratePCH.cpp
@@ -25,11 +25,11 @@
const Preprocessor &PP, StringRef OutputFile, StringRef isysroot,
std::shared_ptr Buffer,
ArrayRef> Extensions,
-bool AllowASTWithErrors, bool IncludeTimestamps)
+bool AllowASTWithErrors, bool In
[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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 don't know what this > field is for. I don't remember any trouble from me changing it though, and if > it was bad to change it I'd hope there'd be a comment right above the field > telling us why. (If I had run into problems, I would've expected me to add a > comment like this.) Sounds good, thanks. https://reviews.llvm.org/D48426 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 that was designed to handle all inline
> > functions (too much), not just dllexport inline functions. + @dblaikie
>
>
> Does it have to be only the dllexported inline functions - or could this be
> used to home all inline functions? (I guess not - presumably it's not
> acceptable for the compiler to implicitly promote something to dllexport (&
> if it doesn't do that promotion, then the function wouldn't be visible from
> the use))
Right, I don't think we could do that. But regular inline functions isn't as
large a problem as the dllexport ones, which we have to emit even if they're
not referenced, causing a huge amount of redundant codegen.
> Is it valid to provide a definition for optimization purposes (LLVM's
> available_externally linkage) for these inline functions? Or is it required
> that, after a change to the header (modifying the implementation of one of
> these dllexported inline functions), the DLL they're exported could be
> rebuilt without rebuilding the clients & the change in behavior would be
> correctly applied?
Yes, we will provide such definitions (not just available_externally, but
weak_odr) if the inline function is referenced, as usual. This is only homing
unreferenced but "force emitted", i.e. DeclMustBeEmitted, functions.
Comment at: include/clang/AST/ASTContext.h:2886-2887
+
+ // XXX: I don't like adding this to ASTContext, but I ran out of ideas for
how ASTContext::DeclMustBeEmitted() would know about it otherwise.
+ bool BuildingPCHWithObjectFile = false;
};
rnk wrote:
> Does `LangOpts.CompilingPCH` do the right thing, or is that also true when we
> make a PCH with no object file? In any case, I'd probably make this a
> langopt. Even if it's functionally different from ModulesCodegen and
> ModulesDebugInfo, it's the same basic idea.
Yes, CompilingPCH is always set when building PCHs, also when there's no object
file so we can't use that.
Moving BuildingPCHWithObjectFile to LangOpts sounds good to me.
Actually that's interesting, because it means the flag will get written into
the PCH will all the other LangOpts, and we could in theory look for that in
ASTReader. But it seems the code isn't really set up to store the LangOpts
coming out of an AST file, we only verify that they're compatible.
Comment at: test/CodeGen/pch-dllexport.cpp:22
+// PCH: define weak_odr dso_local dllexport x86_thiscallcc void
@"?bar@S@@QAEXXZ"
+// PCHWITHOBJ-NOT: define weak_odr dso_local dllexport x86_thiscallcc void
@"?bar@S@@QAEXXZ"
+};
dblaikie wrote:
> This is a pretty specific "NOT" - maybe:
>
> define {{.*}}@"?bar@...
>
> would be better to ensure no definition is provided? (similarly above)
>
> Also, should this file have some non-exported inline functions to check those
> do the right thing?
Updated the -NOT checks and added test.
https://reviews.llvm.org/D48426
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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/clang/Sema/MultiplexExternalSemaSource.h
include/clang/Serialization/ASTBitCodes.h
include/clang/Serialization/ASTReader.h
include/clang/Serialization/Module.h
lib/AST/ASTContext.cpp
lib/Driver/Driver.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/FrontendActions.cpp
lib/Sema/MultiplexExternalSemaSource.cpp
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTWriter.cpp
test/CodeGen/pch-dllexport.cpp
test/Driver/cl-pch.cpp
Index: test/Driver/cl-pch.cpp
===
--- test/Driver/cl-pch.cpp
+++ test/Driver/cl-pch.cpp
@@ -7,13 +7,15 @@
// 1. Build .pch file.
// CHECK-YC: cc1
// CHECK-YC: -emit-pch
+// CHECK-YC: -building-pch-with-obj
// CHECK-YC: -o
// CHECK-YC: pchfile.pch
// CHECK-YC: -x
// CHECK-YC: "c++-header"
// 2. Use .pch file.
// CHECK-YC: cc1
// CHECK-YC: -emit-obj
+// CHECK-YC: -building-pch-with-obj
// CHECK-YC: -include-pch
// CHECK-YC: pchfile.pch
@@ -24,11 +26,13 @@
// 1. Build .pch file.
// CHECK-YCO: cc1
// CHECK-YCO: -emit-pch
+// CHECK-YCO: -building-pch-with-obj
// CHECK-YCO: -o
// CHECK-YCO: pchfile.pch
// 2. Use .pch file.
// CHECK-YCO: cc1
// CHECK-YCO: -emit-obj
+// CHECK-YCO: -building-pch-with-obj
// CHECK-YCO: -include-pch
// CHECK-YCO: pchfile.pch
// CHECK-YCO: -o
@@ -46,6 +50,7 @@
// RUN: | FileCheck -check-prefix=CHECK-YU %s
// Use .pch file, but don't build it.
// CHECK-YU-NOT: -emit-pch
+// CHECK-YU-NOT: -building-pch-with-obj
// CHECK-YU: cc1
// CHECK-YU: -emit-obj
// CHECK-YU: -include-pch
@@ -63,6 +68,7 @@
// 1. Build .pch file.
// CHECK-YC-YU: cc1
// CHECK-YC-YU: -emit-pch
+// CHECK-YC-YU: -building-pch-with-obj
// CHECK-YC-YU: -o
// CHECK-YC-YU: pchfile.pch
// 2. Use .pch file.
Index: test/CodeGen/pch-dllexport.cpp
===
--- /dev/null
+++ test/CodeGen/pch-dllexport.cpp
@@ -0,0 +1,43 @@
+// Build PCH without object file, then use it.
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-pch -o %t %s
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-obj -emit-llvm -include-pch %t -o - %s | FileCheck -check-prefix=PCH %s
+
+// Build PCH with object file, then use it.
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-pch -building-pch-with-obj -o %t %s
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-obj -emit-llvm -include-pch %t -building-pch-with-obj -o - %s | FileCheck -check-prefix=OBJ %s
+// RUN: %clang_cc1 -triple i686-pc-win32 -fms-extensions -emit-obj -emit-llvm -include-pch %t -o - %s | FileCheck -check-prefix=PCHWITHOBJ %s
+
+#ifndef IN_HEADER
+#define IN_HEADER
+
+inline void __declspec(dllexport) foo() {}
+// OBJ: define weak_odr dso_local dllexport void @"?foo@@YAXXZ"
+// PCH: define weak_odr dso_local dllexport void @"?foo@@YAXXZ"
+// PCHWITHOBJ-NOT: define {{.*}}foo
+
+
+// This function is referenced, so gets emitted as usual.
+inline void __declspec(dllexport) baz() {}
+// OBJ: define weak_odr dso_local dllexport void @"?baz@@YAXXZ"
+// PCH: define weak_odr dso_local dllexport void @"?baz@@YAXXZ"
+// PCHWITHOBJ: define weak_odr dso_local dllexport void @"?baz@@YAXXZ"
+
+
+struct __declspec(dllexport) S {
+ void bar() {}
+// OBJ: define weak_odr dso_local dllexport x86_thiscallcc void @"?bar@S@@QAEXXZ"
+// PCH: define weak_odr dso_local dllexport x86_thiscallcc void @"?bar@S@@QAEXXZ"
+// PCHWITHOBJ-NOT: define {{.*}}bar
+};
+
+// This isn't dllexported, attribute((used)) or referenced, so not emitted.
+inline void quux() {}
+// OBJ-NOT: define {{.*}}quux
+// PCH-NOT: define {{.*}}quux
+// PCHWITHOBJ-NOT: define {{.*}}quux
+
+#else
+
+void use() { baz(); }
+
+#endif
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -1458,16 +1458,23 @@
MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 16)); // Clang min.
MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Relocatable
MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Timestamps
+ MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // PCHHasObjectFile
MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Errors
MetadataAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // SVN branch/tag
unsigned MetadataAbbrevCode = Stream.EmitAbbrev(std::move(MetadataAbbrev));
assert((!WritingModule || isysroot.empty()) &&
"writing module as a relocatable PCH?");
{
-RecordData::value_type Record[] = {METADATA, VERSION_MAJO
[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 there. For example:
#ifndef IN_HEADER
#define IN_HEADER
template struct Template { Template() {} };
extern template class Template;
#else
template class __declspec(dllexport) Template;
#endif
This isn't a problem for regular functions where we build a decl chain: the
first declaration may come from a PCH, but the definition would be a separate
Decl object which does not.
I'm trying to figure out how to handle this.
https://reviews.llvm.org/D48426
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 have split the "inherit dllexport attribute" in two places: one for
non-inline functions and one for inline (even if they both call this function).
It feels like it will be hard to maintain.
Here is another idea:
When we inherit the dllexport attribute to class members, if
getLangOpts().DllExportInlines is false, we don't put dllexport on inline
functions, but instead we put a new attribute "dllexportstaticlocals".
That attribute only has the effect that it makes static locals exported. We
would check for it when computing the linkage of the static local, similar to
how it works in hidden functions.
This has two benefits: it doesn't complicate the "inherit dllexport" code very
much, and it avoids the need for a separate AST walk of the function.
What do you think?
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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
lib/Driver/ToolChains/Clang.cpp
test/Driver/cl-showfilenames.c
Index: test/Driver/cl-showfilenames.c
===
--- /dev/null
+++ test/Driver/cl-showfilenames.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cl /c /showFilenames -- %s 2>&1 | FileCheck -check-prefix=show
%s
+// RUN: %clang_cl /c /showFilenames -- %s %S/Inputs/wildcard*.c 2>&1 |
FileCheck -check-prefix=multiple %s
+
+// show: cl-showfilenames.c
+
+// multiple: cl-showfilenames.c
+// multiple-NEXT: wildcard1.c
+// multiple-NEXT: wildcard2.c
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5066,6 +5066,13 @@
C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
}
+ // Make the compile command echo its inputs for /showFilenames.
+ if (Output.getType() == types::TY_Object &&
+ Args.hasFlag(options::OPT__SLASH_showFilenames,
+ options::OPT__SLASH_showFilenames_, false)) {
+C.getJobs().getJobs().back()->setPrintInputFilenames(true);
+ }
+
if (Arg *A = Args.getLastArg(options::OPT_pg))
if (!shouldUseFramePointer(Args, Triple))
D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
Index: lib/Driver/Job.cpp
===
--- lib/Driver/Job.cpp
+++ lib/Driver/Job.cpp
@@ -315,6 +315,11 @@
int Command::Execute(ArrayRef> Redirects,
std::string *ErrMsg, bool *ExecutionFailed) const {
+ if (PrintInputFilenames) {
+for (const char *Arg : InputFilenames)
+ llvm::outs() << llvm::sys::path::filename(Arg) << "\n";
+ }
+
SmallVector Argv;
Optional> Env;
Index: include/clang/Driver/Job.h
===
--- include/clang/Driver/Job.h
+++ include/clang/Driver/Job.h
@@ -59,6 +59,9 @@
/// The list of program arguments which are inputs.
llvm::opt::ArgStringList InputFilenames;
+ /// Whether to print the input filenames when executing.
+ bool PrintInputFilenames = false;
+
/// Response file name, if this command is set to use one, or nullptr
/// otherwise
const char *ResponseFile = nullptr;
@@ -128,6 +131,9 @@
/// Print a command argument, and optionally quote it.
static void printArg(llvm::raw_ostream &OS, StringRef Arg, bool Quote);
+
+ /// Set whether to print the input filenames when executing.
+ void setPrintInputFilenames(bool P) { PrintInputFilenames = P; }
};
/// Like Command, but with a fallback which is executed in case
Index: include/clang/Driver/CLCompatOptions.td
===
--- include/clang/Driver/CLCompatOptions.td
+++ include/clang/Driver/CLCompatOptions.td
@@ -158,6 +158,10 @@
def _SLASH_showIncludes : CLFlag<"showIncludes">,
HelpText<"Print info about included files to stderr">,
Alias;
+def _SLASH_showFilenames : CLFlag<"showFilenames">,
+ HelpText<"Print the name of each compiled file">;
+def _SLASH_showFilenames_ : CLFlag<"showFilenames-">,
+ HelpText<"Don't print the name of each compiled file (default)">;
def _SLASH_source_charset : CLCompileJoined<"source-charset:">,
HelpText<"Source encoding, supports only UTF-8">, Alias;
def _SLASH_execution_charset : CLCompileJoined<"execution-charset:">,
Index: test/Driver/cl-showfilenames.c
===
--- /dev/null
+++ test/Driver/cl-showfilenames.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cl /c /showFilenames -- %s 2>&1 | FileCheck -check-prefix=show %s
+// RUN: %clang_cl /c /showFilenames -- %s %S/Inputs/wildcard*.c 2>&1 | FileCheck -check-prefix=multiple %s
+
+// show: cl-showfilenames.c
+
+// multiple: cl-showfilenames.c
+// multiple-NEXT: wildcard1.c
+// multiple-NEXT: wildcard2.c
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5066,6 +5066,13 @@
C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
}
+ // Make the compile command echo its inputs for /showFilenames.
+ if (Output.getType() == types::TY_Object &&
+ Args.hasFlag(options::OPT__SLASH_showFilenames,
+ options::OPT__SLASH_showFilenames_, false)) {
+C.getJobs().getJobs().back()->setPrintInputFilenames(true);
+ }
+
if (Arg *A = Args.getLastArg(options::OPT_pg))
if (!shouldUseFramePointer(Args, Triple))
D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
Index: lib/Dr
[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?
Comment at: clang/lib/Sema/SemaDecl.cpp:11995
+
+// Function having static local variables should be exported.
+auto *ExportAttr =
cast(NewAttr->clone(getASTContext()));
Isn't it enough that the static local is exported, does the function itself
need to be exported too?
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5703
+if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+!getLangOpts().DllExportInlines &&
+TSK != TSK_ExplicitInstantiationDeclaration &&
I don't think checking for Microsoft ABI is necessary, just checking the
DllExportInlines flag should be enough.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5705
+TSK != TSK_ExplicitInstantiationDeclaration &&
+TSK != TSK_ExplicitInstantiationDefinition) {
+ if (ClassExported) {
But I don't understand why the template stuff is checked here...
The way I imagined this, we'd only need to change the code when creating
NewAttr below..
Something like
```
NewAttr = ClassAtr->clone()...
if (!getLandOpts().DllExportInlines() && Member is an inline method)
NewAttr = our new dllimport/export static locals attribute
```
What do you think?
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 testing seems pretty comprehensive now. It's a lot like the dll attributes in that way :-) Repository: rC Clang https://reviews.llvm.org/D53052 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 > interleaved between the filenames. You can use `#pragma message` in the input > files or something else to create warnings. Good idea, thanks. https://reviews.llvm.org/D52773 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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=169178#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52773
Files:
cfe/trunk/include/clang/Driver/CLCompatOptions.td
cfe/trunk/include/clang/Driver/Job.h
cfe/trunk/lib/Driver/Job.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/cl-showfilenames.c
Index: cfe/trunk/include/clang/Driver/Job.h
===
--- cfe/trunk/include/clang/Driver/Job.h
+++ cfe/trunk/include/clang/Driver/Job.h
@@ -59,6 +59,9 @@
/// The list of program arguments which are inputs.
llvm::opt::ArgStringList InputFilenames;
+ /// Whether to print the input filenames when executing.
+ bool PrintInputFilenames = false;
+
/// Response file name, if this command is set to use one, or nullptr
/// otherwise
const char *ResponseFile = nullptr;
@@ -128,6 +131,9 @@
/// Print a command argument, and optionally quote it.
static void printArg(llvm::raw_ostream &OS, StringRef Arg, bool Quote);
+
+ /// Set whether to print the input filenames when executing.
+ void setPrintInputFilenames(bool P) { PrintInputFilenames = P; }
};
/// Like Command, but with a fallback which is executed in case
Index: cfe/trunk/include/clang/Driver/CLCompatOptions.td
===
--- cfe/trunk/include/clang/Driver/CLCompatOptions.td
+++ cfe/trunk/include/clang/Driver/CLCompatOptions.td
@@ -158,6 +158,10 @@
def _SLASH_showIncludes : CLFlag<"showIncludes">,
HelpText<"Print info about included files to stderr">,
Alias;
+def _SLASH_showFilenames : CLFlag<"showFilenames">,
+ HelpText<"Print the name of each compiled file">;
+def _SLASH_showFilenames_ : CLFlag<"showFilenames-">,
+ HelpText<"Don't print the name of each compiled file (default)">;
def _SLASH_source_charset : CLCompileJoined<"source-charset:">,
HelpText<"Source encoding, supports only UTF-8">, Alias;
def _SLASH_execution_charset : CLCompileJoined<"execution-charset:">,
Index: cfe/trunk/test/Driver/cl-showfilenames.c
===
--- cfe/trunk/test/Driver/cl-showfilenames.c
+++ cfe/trunk/test/Driver/cl-showfilenames.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cl /c /showFilenames -- %s 2>&1 | FileCheck -check-prefix=show
%s
+// RUN: %clang_cl /c /showFilenames -- %s %S/Inputs/wildcard*.c 2>&1 |
FileCheck -check-prefix=multiple %s
+
+// RUN: %clang_cl /c -- %s 2>&1 | FileCheck -check-prefix=noshow %s
+// RUN: %clang_cl /c /showFilenames /showFilenames- -- %s 2>&1 | FileCheck
-check-prefix=noshow %s
+
+
+#pragma message "Hello"
+
+// show: cl-showfilenames.c
+// show-NEXT: warning: Hello
+
+// multiple: cl-showfilenames.c
+// multiple-NEXT: warning: Hello
+// multiple: wildcard1.c
+// multiple-NEXT: wildcard2.c
+
+// noshow: warning: Hello
+// noshow-NOT: cl-showfilenames.c
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -5067,6 +5067,13 @@
C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
}
+ // Make the compile command echo its inputs for /showFilenames.
+ if (Output.getType() == types::TY_Object &&
+ Args.hasFlag(options::OPT__SLASH_showFilenames,
+ options::OPT__SLASH_showFilenames_, false)) {
+C.getJobs().getJobs().back()->setPrintInputFilenames(true);
+ }
+
if (Arg *A = Args.getLastArg(options::OPT_pg))
if (!shouldUseFramePointer(Args, Triple))
D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
Index: cfe/trunk/lib/Driver/Job.cpp
===
--- cfe/trunk/lib/Driver/Job.cpp
+++ cfe/trunk/lib/Driver/Job.cpp
@@ -315,6 +315,12 @@
int Command::Execute(ArrayRef> Redirects,
std::string *ErrMsg, bool *ExecutionFailed) const {
+ if (PrintInputFilenames) {
+for (const char *Arg : InputFilenames)
+ llvm::outs() << llvm::sys::path::filename(Arg) << "\n";
+llvm::outs().flush();
+ }
+
SmallVector Argv;
Optional> Env;
Index: cfe/trunk/include/clang/Driver/Job.h
===
--- cfe/trunk/include/clang/Driver/Job.h
+++ cfe/trunk/include/clang/Driver/Job.h
@@ -59,6 +59,9 @@
/// The list of program arguments which are inputs.
llvm::opt::ArgStringList InputFilenames;
+ /// Whether to print the input filenames when executing.
+ bool PrintInputFilenames = false;
+
/// Response file name, if this command is set to use one, or nullptr
/// otherwise
const char *ResponseFile
[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?
> This is for static local var in lambda function.
> static_x's ParentFunction does not become f().
> ```
> class __declspec(dllexport) C {
> int f() {
> return ([]() { static int static_x; return ++static_x; })();
> }
> };
> ```
I don't think the lambda (or its static local) should be exported in this case.
Comment at: clang/lib/Sema/SemaDecl.cpp:11995
+
+// Function having static local variables should be exported.
+auto *ExportAttr =
cast(NewAttr->clone(getASTContext()));
takuto.ikuta wrote:
> hans wrote:
> > Isn't it enough that the static local is exported, does the function itself
> > need to be exported too?
> Adding dllexport only to variable does not export variable when the function
> is not used.
> But dllimport is not necessary for function, removed.
Hmm, I wonder if we could fix that instead? That is, export the variable in
that case even if the function is not used?
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
I still don't understand why we need these checks for template instantiations.
Why does it matter whether the functions get inlined or not?
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 to update it here. Comment at: include/clang/Driver/CC1Options.td:195 HelpText<"Do not emit code that uses the red zone.">; +def indirect_tls_seg_refs : Flag<["-"], "indirect-tls-seg-refs">, + HelpText<"Do not emit code that uses direct TLS segment access.">; Could mno_tls_direct_seg_refs be used as a cc1 flag instead? Comment at: include/clang/Driver/Options.td:2167 +def mtls_direct_seg_refs : Flag<["-"], "mtls-direct-seg-refs">, Group, + HelpText<"Enable direct TLS access through segment registers">; def mregparm_EQ : Joined<["-"], "mregparm=">, Group; Maybe add (default) to the help text to indicate this is the default? Repository: rC Clang https://reviews.llvm.org/D53102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 don't think FunctionDecl's can be
> > > > nested?
> > > This is for static local var in lambda function.
> > > static_x's ParentFunction does not become f().
> > > ```
> > > class __declspec(dllexport) C {
> > > int f() {
> > > return ([]() { static int static_x; return ++static_x; })();
> > > }
> > > };
> > > ```
> > I don't think the lambda (or its static local) should be exported in this
> > case.
> If we don't export static_x, dllimport library cannot find static_x when
> linking?
>
>
I believe even if C is marked dllimport, the lambda will not be dllimport. The
inline definition will be used.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
takuto.ikuta wrote:
> hans wrote:
> > I still don't understand why we need these checks for template
> > instantiations. Why does it matter whether the functions get inlined or not?
> When function of dllimported class is not inlined, such funtion needs to be
> dllexported from implementation library.
>
> c.h
> ```
> template
> class EXPORT C {
> public:
> void f() {}
> };
> ```
>
> cuser.cc
> ```
> #include "c.h"
>
> void cuser() {
> C c;
> c.f(); // This function may not be inlined when EXPORT is
> __declspec(dllimport), so link may fail.
> }
> ```
>
> When cuser.cc and c.h are built to different library, cuser.cc needs to be
> able to see dllexported C::f() when C::f() is not inlined.
> This is my understanding.
Your example doesn't use explicit instantiation definition or declaration, so
doesn't apply here I think.
As long as the dllexport and dllimport attributes matches it's fine. It doesn't
matter whether the function gets inlined or not, the only thing that matters is
that if it's marked dllimport on the "consumer side", it must be dllexport when
building the dll.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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&id=170210#toc
Repository:
rL LLVM
https://reviews.llvm.org/D53434
Files:
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTestJava.cpp
Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1130,6 +1130,10 @@
nextToken();
parseBracedList();
break;
+ } else if (Style.Language == FormatStyle::LK_Java &&
+ FormatTok->is(Keywords.kw_interface)) {
+nextToken();
+break;
}
switch (FormatTok->Tok.getObjCKeywordID()) {
case tok::objc_public:
Index: cfe/trunk/unittests/Format/FormatTestJava.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJava.cpp
+++ cfe/trunk/unittests/Format/FormatTestJava.cpp
@@ -155,6 +155,15 @@
" void doStuff(int theStuff);\n"
" void doMoreStuff(int moreStuff);\n"
"}");
+ verifyFormat("class A {\n"
+ " public @interface SomeInterface {\n"
+ "int stuff;\n"
+ "void doMoreStuff(int moreStuff);\n"
+ " }\n"
+ "}");
+ verifyFormat("class A {\n"
+ " public @interface SomeInterface {}\n"
+ "}");
}
TEST_F(FormatTestJava, AnonymousClasses) {
Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1130,6 +1130,10 @@
nextToken();
parseBracedList();
break;
+ } else if (Style.Language == FormatStyle::LK_Java &&
+ FormatTok->is(Keywords.kw_interface)) {
+nextToken();
+break;
}
switch (FormatTok->Tok.getObjCKeywordID()) {
case tok::objc_public:
Index: cfe/trunk/unittests/Format/FormatTestJava.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJava.cpp
+++ cfe/trunk/unittests/Format/FormatTestJava.cpp
@@ -155,6 +155,15 @@
" void doStuff(int theStuff);\n"
" void doMoreStuff(int moreStuff);\n"
"}");
+ verifyFormat("class A {\n"
+ " public @interface SomeInterface {\n"
+ "int stuff;\n"
+ "void doMoreStuff(int moreStuff);\n"
+ " }\n"
+ "}");
+ verifyFormat("class A {\n"
+ " public @interface SomeInterface {}\n"
+ "}");
}
TEST_F(FormatTestJava, AnonymousClasses) {
___
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. 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 right driver for that use-case. But I suppose an argument could be made for having an escape hatch from clang-cl if it doesn't add too much complexity to the code. I'm not sure I'm a fan of calling it /Xdriver: though, because what does it mean - clang-cl is the driver, but the option refers to the clang driver. The natural name would of course be -Xclang but that already means something else. Maybe we could just call it /clang: Repository: rC Clang https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 to > > core options, and if someone wants to use more clang than that, maybe > > clang-cl isn't the right driver for that use-case. > > > > But I suppose an argument could be made for having an escape hatch from > > clang-cl if it doesn't add too much complexity to the code. > > > This is a good point. However, having this escape hatch gets you and Reid and > others out of the business of having to promote options. Also, as new flags > are added to the compiler people might need one revision of the official > builds to realize they need a flag and one revision to get the flag added to > the binary release. This obviously isn't a problem for people building Clang > from source, but it does add unnecessary friction as I found myself. Yeah, let's add the escape hatch. > > >> I'm not sure I'm a fan of calling it /Xdriver: though, because what does it >> mean - clang-cl is the driver, but the option refers to the clang driver. >> The natural name would of course be -Xclang but that already means something >> else. Maybe we could just call it /clang: > > At the conference last week I discussed this with Reid Kleckner. One of the > options we discussed was trying to make things work such that -Xclang serves > both purposes. We quickly decided that this wouldn't work. /clang: would be > fine, but it might be more confusing since people will wonder what's the > difference between /Xclang and /clang:. We could use something more verbose > like /Xclang-driver:. I'd be happy to change the flag to whatever spelling > will build consensus. Let's go with "/clang:" for the flag. I don't think having both that and -Xclang would be too confusing. I think -Xclang is undocumented anyway and really something that should just be used by experts. We should add /clang: to the documentation and I think it will be straight-forward to understand. Repository: rC Clang https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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:2688
+ // This attribute is used internally only when -fno-dllexport-inlines is
+ // passed.
+ let Spellings = [];
The comment should explain what the attribute does, also the dllimport one
below.
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
Can you give an example for why this is needed?
Comment at: clang/lib/Sema/SemaDecl.cpp:11976
+
+while (FD && !getDLLAttr(FD) &&
+ !FD->hasAttr() &&
takuto.ikuta wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > takuto.ikuta wrote:
> > > > > hans wrote:
> > > > > > Why does this need to be a loop? I don't think FunctionDecl's can
> > > > > > be nested?
> > > > > This is for static local var in lambda function.
> > > > > static_x's ParentFunction does not become f().
> > > > > ```
> > > > > class __declspec(dllexport) C {
> > > > > int f() {
> > > > > return ([]() { static int static_x; return ++static_x; })();
> > > > > }
> > > > > };
> > > > > ```
> > > > I don't think the lambda (or its static local) should be exported in
> > > > this case.
> > > If we don't export static_x, dllimport library cannot find static_x when
> > > linking?
> > >
> > >
> > I believe even if C is marked dllimport, the lambda will not be dllimport.
> > The inline definition will be used.
> Do you say importing/implementation library should have different instance of
> static_x here?
> Current clang does not generate such obj.
>
> I think static_x should be dllimported. But without this loop, static_x
> cannot find FD of C::f() having DLLImport/ExportStaticLocalAttr and static_x
> won't be treated as imported/exported variable.
>
> And if static_x is not exported, importing library and implementation library
> will not have the same instance of static_x when C::f() is inlined.
Look at what MSVC does:
```
>type a.cc
struct __declspec(dllexport) S {
int f() {
static int y;
return ([]() { static int static_x; return ++static_x; })() + y;
}
};
void use(S *s) { s->f(); }
>cl -c a.cc && dumpbin /directives a.obj
Microsoft (R) C/C++ Optimizing Compiler Version 19.12.25834 for x86
Copyright (C) Microsoft Corporation. All rights reserved.
a.cc
Microsoft (R) COFF/PE Dumper Version 14.12.25834.0
Copyright (C) Microsoft Corporation. All rights reserved.
Dump of file a.obj
File Type: COFF OBJECT
Linker Directives
-
/DEFAULTLIB:LIBCMT
/DEFAULTLIB:OLDNAMES
/EXPORT:?f@S@@QAEHXZ
/EXPORT:??4S@@QAEAAU0@ABU0@@Z
/EXPORT:??4S@@QAEAAU0@$$QAU0@@Z
/EXPORT:?y@?1??f@S@@QAEHXZ@4HA,DATA
Summary
8 .bss
50 .chks64
64 .debug$S
A6 .drectve
6A .text$mn
```
As you can see S::f and S::f::y is exported, but the lambda and its static
local are not.
Oh, but I guess you're asking what if we don't dllexport S::f anymore because
we're not dllexporting inline methods anymore... hmm, yes, then I do suppose it
needs to be exported so maybe this is right after all.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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/mailman/listinfo/cfe-commits
[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 unfortunate. I wonder if it would be possible to not have this restriction, i.e. to process these in-line with the rest of the flags? One way to achieve this would be to change Driver::ParseArgStrings() to handle the "/clang:" arguments. After doing the regular option parsing, it would look for "/clang:" options and substitute them for the underlying option. This has the downside of adding some option-specific logic to ParseArgStrings, but on the other hand it's less intrusive than passing around the IsCLMode bool. Do you think something like this could work? Comment at: docs/UsersManual.rst:3100 + +The /clang: Option + Thanks for thinking about the docs! I think we should put this above the /fallback section, since this new flag is more important and /fallback shouldn't be used much these days. Comment at: include/clang/Driver/CLCompatOptions.td:324 HelpText<"Volatile loads and stores have acquire and release semantics">; +def _SLASH_clang : CLJoinedOrSeparate<"clang:">, + HelpText<"Pass to the clang driver">, MetaVarName<"">; Do we really want the "OrSeparate" part of this? Is there any downside of limiting it to "/clang:-foo" rather than also allowing "/clang: -foo"? https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 wins, the /clang: flags > > will be chosen regardless of their order relative to other flags on the > > driver > > command line. > > > > > > This seems a little unfortunate. I wonder if it would be possible to not > > have this restriction, i.e. to process these in-line with the rest of the > > flags? > > > > One way to achieve this would be to change Driver::ParseArgStrings() to > > handle the "/clang:" arguments. After doing the regular option parsing, it > > would look for "/clang:" options and substitute them for the underlying > > option. This has the downside of adding some option-specific logic to > > ParseArgStrings, but on the other hand it's less intrusive than passing > > around the IsCLMode bool. Do you think something like this could work? > > > One difficulty occurs with options that have separated values, like the > `/clang:-MF /clang:` case. In this case, we need to take two > /clang: arguments and process them next to each other in order to form a > single coherent argument/value pair. Another option is to allow the user to > specify the option like `/clang:"-MF "` and then require that the > user specify any flags that need a value in a single /clang: argument. This > would require some code to split the value string on spaces before passing it > on to clang argument parsing. > > Do you have any preference or better suggestion for the option-with-value > case? The `-Xclang` option has the same issue, and I think `/clang:` should work similarly, i.e. `/clang:-MF /clang:`. It's not pretty, but at least it's consistent. Yes, that means processing consecutive `/Xclang:` options together, but hopefully that's doable. https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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();
struct __declspec(dllimport) S {
int bar() { return foo(); }
};
#endif
`lib.cc`:
#include "lib.h"
int foo() { return 123; }
`main.cc`:
#include "lib.h"
int main() {
S s;
return s.bar();
}
Here, Clang will not emit the body of `S::bar()`, because it references the
non-dllimport function `foo()`, and trying to referencing `foo()` outside the
library would result in a link error. This is what the
`DLLImportFunctionVisitor` is used for. For the same reason, MSVC will also not
inline dllimport functions.
Now, with `/Zc:dllexportInlines-`, we no longer mark `S::bar()` dllimport, and
so we do emit it, causing that link error. The same problem happens with
`-fvisibility-inlines-hidden`: substitute the `declspec` above for
`__attribute__((visibility("default")))` above and try it:
$ 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 known issue with `-fvisibility-inlines-hidden`. This doesn't come
up often in practice, but when it does the developer needs to deal with it.
However, the static local problem is much scarier, because that leads to
run-time bugs:
`lib.h`:
#ifndef LIB_H
#define LIB_H
inline int foo() { static int x = 0; return x++; }
struct __attribute__((visibility("default"))) S {
int bar() { return foo(); }
int baz();
};
#endif
`lib.cc`:
#include "lib.h"
int S::baz() { return foo(); }
`main.cc`:
#include
#include "lib.h"
int main() {
S s;
printf("s.bar(): %d\n", s.bar());
printf("s.baz(): %d\n", s.baz());
return 0;
}
If we build the program normally, we get the expected output:
$ g++ lib.cc main.cc && ./a.out
s.bar(): 0
s.baz(): 1
but building as a shared library shows the problem:
$ g++ -fvisibility=hidden -fvisibility-inlines-hidden -fpic -shared -o lib.so
lib.cc
$ g++ main.cc lib.so
$ LD_LIBRARY_PATH=$PWD:$LD_LIBRARY_PATH ./a.out
s.bar(): 0
s.baz(): 0
Oops.
This does show that it's a pre-existing problem with the model of not exporting
inline functions though. I don't think we need to solve this specifically for
Windows, I think we should match what `-fvisibility-inlines-hidden` is doing.
And what about the lambdas?
`lib.h`:
#ifndef LIB_H
#define LIB_H
struct __attribute__((visibility("default"))) S {
int bar() { return ([]() { static int x; return x++; })(); }
int baz();
};
#endif
`lib.cc`:
#include "lib.h"
int S::baz() { return bar(); }
$ g++ -fvisibility=hidden -fvisibility-inlines-hidden -fpic -shared -o lib.so
lib.cc && g++ main.cc lib.so && LD_LIBRARY_PATH=$PWD:$LD_LIBRARY_PATH ./a.out
s.bar(): 0
s.baz(): 1
Interesting, the lambda function and its static local are not hidden.
Either that's because they're applying the "static locals of hidden functions
in visible decl contexts are visible" thing. Or alternatively, they never
applied the hidden visibility to the lambda in the first place.
I'm not entirely sure what this means for the dllexport case though. Maybe the
loop in the current patch is fine.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > takuto.ikuta wrote:
> > > > > hans wrote:
> > > > > > I still don't understand why we need these checks for template
> > > > > > instantiations. Why does it matter whether the functions get
> > > > > > inlined or not?
> > > > > When function of dllimported class is not inlined, such funtion needs
> > > > > to be dllexported from implementation library.
> > > > >
> > > > > c.h
> > > > > ```
> > > > > template
> > > > > class EXPORT C {
> > > > > public:
> > > > > void f() {}
> > > > > };
> > > > > ```
> > > > >
> > > > > cuser.cc
> > > > > ```
> > > > > #include "c.h"
> > > > >
> > > > > void cuser() {
> > > > > C c;
> > > > > c.f(); // This function may not be inlined when EXPORT is
> > > > > __declspec(dllimport), so link may fail.
> > > > > }
> > > > > ```
> > > > >
> > > > > When cuser.cc and c.h are built to different library, cuser.cc needs
> > > > > to be able to see dllexported C::f() when C::f() is not inlined.
> > > > > This is my understanding.
> > > > Your example doesn't use explicit instantiation definition or
> > > > declaration, so doesn't apply here I think.
> > > >
> > > > As long as the dllexport and dllimport attributes matches it's fine. It
> > > > doesn't matter whether the function gets inlined or not, the only thing
> > > > that matters is that if it's marked dllimport on the "consumer side",
> > > > it must be dllexport when building the dll.
> > > Hmm, I changed the linkage for functions having
> > > DLLExport/ImportStaticLocal Attr.
> > >
> > I changed linkage in ASTContext so that inline function is emitted when it
> > is necessary when we use fno-dllexport-inlines.
> I revived explicit template instantiation check.
> Found that this is necessary.
>
> For explicit template instantiation, inheriting dll attribute from function
> for local static var is run before inheriting dll attribute from class for
> member functions. So I cannot detect local static var of inline function
> after class level dll attribute processing for explicit template
> instantiation.
>
Oh I see, it's a static local problem..
Can you provide a concrete example that does not work without your check?
Maybe this is the right thing to do, but I would like to understand exactly
what the problem is.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 known issue with `-fvisibility-inlines-hidden`. This doesn't
>> come up often in practice, but when it does the developer needs to deal with
>> it.
>
> Yeah, that is the reason of few chromium code changes.
> https://chromium-review.googlesource.com/c/chromium/src/+/1212379
Ah, thanks! I hadn't seen what the fixes would look like.
>> However, the static local problem is much scarier, because that leads to
>> run-time bugs:
>
> Currently this CL doesn't take care of inline function that is not member of
> a class.
>
> `lib.h`:
>
> #ifndef LIB_H
> #define LIB_H
>
> struct __attribute__((visibility("default"))) S {
> int bar() {
> static int x = 0; return x++;
> }
> int baz();
> };
>
> #endif
>
>
> `lib.cc`:
>
> #include "lib.h"
>
> int S::baz() {
> return bar();
> }
>
>
> Then, static local in inline function is treated correctly.
I think it's possible to get the same problem with member functions, but that
doesn't matter, it's an existing problem so we don't need to solve it, just be
aware.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
takuto.ikuta wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > takuto.ikuta wrote:
> > > > takuto.ikuta wrote:
> > > > > hans wrote:
> > > > > > takuto.ikuta wrote:
> > > > > > > hans wrote:
> > > > > > > > I still don't understand why we need these checks for template
> > > > > > > > instantiations. Why does it matter whether the functions get
> > > > > > > > inlined or not?
> > > > > > > When function of dllimported class is not inlined, such funtion
> > > > > > > needs to be dllexported from implementation library.
> > > > > > >
> > > > > > > c.h
> > > > > > > ```
> > > > > > > template
> > > > > > > class EXPORT C {
> > > > > > > public:
> > > > > > > void f() {}
> > > > > > > };
> > > > > > > ```
> > > > > > >
> > > > > > > cuser.cc
> > > > > > > ```
> > > > > > > #include "c.h"
> > > > > > >
> > > > > > > void cuser() {
> > > > > > > C c;
> > > > > > > c.f(); // This function may not be inlined when EXPORT is
> > > > > > > __declspec(dllimport), so link may fail.
> > > > > > > }
> > > > > > > ```
> > > > > > >
> > > > > > > When cuser.cc and c.h are built to different library, cuser.cc
> > > > > > > needs to be able to see dllexported C::f() when C::f() is not
> > > > > > > inlined.
> > > > > > > This is my understanding.
> > > > > > Your example doesn't use explicit instantiation definition or
> > > > > > declaration, so doesn't apply here I think.
> > > > > >
> > > > > > As long as the dllexport and dllimport attributes matches it's
> > > > > > fine. It doesn't matter whether the function gets inlined or not,
> > > > > > the only thing that matters is that if it's marked dllimport on the
> > > > > > "consumer side", it must be dllexport when building the dll.
> > > > > Hmm, I changed the linkage for functions having
> > > > > DLLExport/ImportStaticLocal Attr.
> > > > >
> > > > I changed linkage in ASTContext so that inline function is emitted when
> > > > it is necessary when we use fno-dllexport-inlines.
> > > I revived explicit template instantiation check.
> > > Found that this is necessary.
> > >
> > > For explicit template instantiation, inheriting dll attribute from
> > > function for local static var is run before inheriting dll attribute from
> > > class for member functions. So I cannot detect local static var of inline
> > > function after class level dll attribute processing for explicit template
> > > instantiation.
> > >
> > Oh I see, it's a static local problem..
> > Can you provide a concrete example that does not work without your check?
> > Maybe this is the right thing to do, but I would like to understand exactly
> > what the problem is.
> For the following code
> ```
> template
> class M{
> public:
>
> T Inclass() {
> static T static_x;
> ++static_x;
> return static_x;
> }
> };
>
> template class __declspec(dllexport) M;
>
> extern template class __declspec(dllimport) M;
>
> int f (){
> M mi;
> M ms;
> return mi.Inclass() + ms.Inclass();
> }
>
> ```
>
> llvm code without instantiation check become like below. Both inline
> functions of M and M is not dllimported/exported.
> ```
> $"?Inclass@?$M@H@@QEAAHXZ" = comdat any
>
> $"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA" = comdat any
>
> @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA" = linkonce_odr dso_local global
> i32 0, comdat, align 4
>
> ; Function Attrs: noinline nounwind optnone
> define weak_
[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::FinalizeDeclaration(), however for
template instantiations we need to do something in or around
TemplateDeclInstantiator::VisitVarDecl(). This patch does that, and extracts
the code to a utility function.
Please take a look!
https://reviews.llvm.org/D53870
Files:
include/clang/Sema/Sema.h
lib/Sema/SemaDecl.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
test/CodeGenCXX/dllexport.cpp
test/CodeGenCXX/dllimport.cpp
Index: test/CodeGenCXX/dllimport.cpp
===
--- test/CodeGenCXX/dllimport.cpp
+++ test/CodeGenCXX/dllimport.cpp
@@ -996,3 +996,16 @@
USEMEMFUNC(ExplicitInstantiationDeclTemplateBase2, func)
// M32-DAG: declare dllimport x86_thiscallcc void @"?func@?$ExplicitInstantiationDeclTemplateBase2@H@@QAEXXZ"
// G32-DAG: define weak_odr dso_local x86_thiscallcc void @_ZN38ExplicitInstantiationDeclTemplateBase2IiE4funcEv
+
+namespace pr39496 {
+// Make sure dll attribute are inherited by static locals also in template
+// specializations.
+template struct __declspec(dllimport) S { int foo() { static int x; return x++; } };
+int foo() { S s; return s.foo(); }
+// MO1-DAG: @"?x@?{{1|2}}??foo@?$S@H@pr39496@@Q{{[A-Z]*}}HXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+template struct T { int foo() { static int x; return x++; } };
+extern template struct __declspec(dllimport) T;
+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
+}
Index: test/CodeGenCXX/dllexport.cpp
===
--- test/CodeGenCXX/dllexport.cpp
+++ test/CodeGenCXX/dllexport.cpp
@@ -1012,6 +1012,18 @@
// M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc %"struct.LayerTreeImpl::ElementLayers"* @"??0ElementLayers@LayerTreeImpl@@QAE@XZ"
// M64-DAG: define weak_odr dso_local dllexport %"struct.LayerTreeImpl::ElementLayers"* @"??0ElementLayers@LayerTreeImpl@@QEAA@XZ"
+namespace pr39496 {
+// Make sure dll attribute are inherited by static locals also in template
+// specializations.
+template struct __declspec(dllexport) S { int foo() { static int x; return x++; } };
+int foo() { S s; return s.foo(); }
+// MSC-DAG: @"?x@?{{1|2}}??foo@?$S@H@pr39496@@Q{{[A-Z]*}}HXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+template struct T { int foo() { static int x; return x++; } };
+template struct __declspec(dllexport) T;
+// MSC-DAG: @"?x@?{{1|2}}??foo@?$T@H@pr39496@@Q{{[A-Z]*}}HXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+}
+
class __declspec(dllexport) ACE_Shared_Object {
public:
virtual ~ACE_Shared_Object();
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -728,6 +728,9 @@
D->getLocation(), D->getIdentifier(), DI->getType(),
DI, D->getStorageClass());
+ if (Var->isStaticLocal())
+SemaRef.CheckStaticLocalForDllExport(Var);
+
// In ARC, infer 'retaining' for variables of retainable type.
if (SemaRef.getLangOpts().ObjCAutoRefCount &&
SemaRef.inferObjCARCLifetime(Var))
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11924,6 +11924,23 @@
return false;
}
+/// Check if VD needs to be dllexport/dllimport due to being in a
+/// dllexport/import function.
+void Sema::CheckStaticLocalForDllExport(VarDecl *VD) {
+ assert(VD->isStaticLocal());
+
+ auto *FD = dyn_cast_or_null(VD->getParentFunctionOrMethod());
+ if (!FD)
+return;
+
+ // Static locals inherit dll attributes from their function.
+ if (Attr *A = getDLLAttr(FD)) {
+auto *NewAttr = cast(A->clone(getASTContext()));
+NewAttr->setInherited(true);
+VD->addAttr(NewAttr);
+ }
+}
+
/// FinalizeDeclaration - called by ParseDeclarationAfterDeclarator to perform
/// any semantic actions necessary after any initializer has been attached.
void Sema::FinalizeDeclaration(Decl *ThisDecl) {
@@ -11977,14 +11994,9 @@
}
if (VD->isStaticLocal()) {
-if (FunctionDecl *FD =
-dyn_cast_or_null(VD->getParentFunctionOrMethod())) {
- // Static locals inherit dll attributes from their function.
- if (Attr *A = getDLLAttr(FD)) {
-auto *NewAttr = cast(A->clone(getASTContext()));
-NewAttr->setInherited(true);
-VD->addAttr(NewAttr);
- }
+CheckStaticLocalForDllExport(VD);
+
+if (dyn_cast_or_nul
[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/Sema/Sema.h
lib/Sema/SemaDecl.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
test/CodeGenCXX/dllexport.cpp
test/CodeGenCXX/dllimport.cpp
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -728,6 +728,9 @@
D->getLocation(), D->getIdentifier(), DI->getType(),
DI, D->getStorageClass());
+ if (Var->isStaticLocal())
+SemaRef.CheckStaticLocalForDllExport(Var);
+
// In ARC, infer 'retaining' for variables of retainable type.
if (SemaRef.getLangOpts().ObjCAutoRefCount &&
SemaRef.inferObjCARCLifetime(Var))
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11924,6 +11924,23 @@
return false;
}
+/// Check if VD needs to be dllexport/dllimport due to being in a
+/// dllexport/import function.
+void Sema::CheckStaticLocalForDllExport(VarDecl *VD) {
+ assert(VD->isStaticLocal());
+
+ auto *FD = dyn_cast_or_null(VD->getParentFunctionOrMethod());
+ if (!FD)
+return;
+
+ // Static locals inherit dll attributes from their function.
+ if (Attr *A = getDLLAttr(FD)) {
+auto *NewAttr = cast(A->clone(getASTContext()));
+NewAttr->setInherited(true);
+VD->addAttr(NewAttr);
+ }
+}
+
/// FinalizeDeclaration - called by ParseDeclarationAfterDeclarator to perform
/// any semantic actions necessary after any initializer has been attached.
void Sema::FinalizeDeclaration(Decl *ThisDecl) {
@@ -11977,14 +11994,9 @@
}
if (VD->isStaticLocal()) {
-if (FunctionDecl *FD =
-dyn_cast_or_null(VD->getParentFunctionOrMethod())) {
- // Static locals inherit dll attributes from their function.
- if (Attr *A = getDLLAttr(FD)) {
-auto *NewAttr = cast(A->clone(getASTContext()));
-NewAttr->setInherited(true);
-VD->addAttr(NewAttr);
- }
+CheckStaticLocalForDllExport(VD);
+
+if (dyn_cast_or_null(VD->getParentFunctionOrMethod())) {
// CUDA 8.0 E.3.9.4: Within the body of a __device__ or __global__
// function, only __shared__ variables or variables without any device
// memory qualifiers may be declared with static storage class.
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -2001,6 +2001,7 @@
SourceLocation AttrEnd);
void SetDeclDeleted(Decl *dcl, SourceLocation DelLoc);
void SetDeclDefaulted(Decl *dcl, SourceLocation DefaultLoc);
+ void CheckStaticLocalForDllExport(VarDecl *VD);
void FinalizeDeclaration(Decl *D);
DeclGroupPtrTy FinalizeDeclaratorGroup(Scope *S, const DeclSpec &DS,
ArrayRef Group);
Index: test/CodeGenCXX/dllexport.cpp
===
--- test/CodeGenCXX/dllexport.cpp
+++ test/CodeGenCXX/dllexport.cpp
@@ -1012,6 +1012,18 @@
// M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc %"struct.LayerTreeImpl::ElementLayers"* @"??0ElementLayers@LayerTreeImpl@@QAE@XZ"
// M64-DAG: define weak_odr dso_local dllexport %"struct.LayerTreeImpl::ElementLayers"* @"??0ElementLayers@LayerTreeImpl@@QEAA@XZ"
+namespace pr39496 {
+// Make sure dll attribute are inherited by static locals also in template
+// specializations.
+template struct __declspec(dllexport) S { int foo() { static int x; return x++; } };
+int foo() { S s; return s.foo(); }
+// MSC-DAG: @"?x@?{{1|2}}??foo@?$S@H@pr39496@@Q{{[A-Z]*}}HXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+template struct T { int foo() { static int x; return x++; } };
+template struct __declspec(dllexport) T;
+// MSC-DAG: @"?x@?{{1|2}}??foo@?$T@H@pr39496@@Q{{[A-Z]*}}HXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+}
+
class __declspec(dllexport) ACE_Shared_Object {
public:
virtual ~ACE_Shared_Object();
Index: test/CodeGenCXX/dllimport.cpp
===
--- test/CodeGenCXX/dllimport.cpp
+++ test/CodeGenCXX/dllimport.cpp
@@ -996,3 +996,16 @@
USEMEMFUNC(ExplicitInstantiationDeclTemplateBase2, func)
// M32-DAG: declare dllimport x86_thiscallcc void @"?func@?$ExplicitInstantiationDeclTemplateBase2@H@@QAEXXZ"
// G32-DAG: define weak_odr dso_local x86_thiscallcc void @_ZN38ExplicitInstantiationDeclTemplateBase2IiE4funcEv
+
+namespace pr39496 {
+// Make sure dll attribute are inherited by static locals
[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 we don't emit `foo` as an available_externally definition right
> now. With your change, will we do so? Should we?
Ah, good point. It's actually the static local that was previously preventing
us from emitting it available_externally. We normally would, but
DLLImportFunctionVisitor would discover that the function referenced a
non-dllimport "global" variable, and determine that it was not safe to emit the
definition.
But now that the static local inherits the dll attribute, this works out
automatically.
```
$ bin/clang -cc1 -triple i686-windows-msvc -fms-extensions -emit-llvm
-std=c++1y -O1 -disable-llvm-passes -o -
../tools/clang/test/CodeGenCXX/dllimport.cpp -DMSABI -w | grep
'define.*?foo@?$T@H@pr39496'
define available_externally dllimport x86_thiscallcc i32
@"?foo@?$T@H@pr39496@@QAEHXZ"(%"struct.pr39496::T"* %this) #0 align 2 {
```
(Same for S::foo() one.)
Repository:
rC Clang
https://reviews.llvm.org/D53870
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 wrote:
> > > takuto.ikuta wrote:
> > > > takuto.ikuta wrote:
> > > > > takuto.ikuta wrote:
> > > > > > hans wrote:
> > > > > > > takuto.ikuta wrote:
> > > > > > > > hans wrote:
> > > > > > > > > I still don't understand why we need these checks for
> > > > > > > > > template instantiations. Why does it matter whether the
> > > > > > > > > functions get inlined or not?
> > > > > > > > When function of dllimported class is not inlined, such funtion
> > > > > > > > needs to be dllexported from implementation library.
> > > > > > > >
> > > > > > > > c.h
> > > > > > > > ```
> > > > > > > > template
> > > > > > > > class EXPORT C {
> > > > > > > > public:
> > > > > > > > void f() {}
> > > > > > > > };
> > > > > > > > ```
> > > > > > > >
> > > > > > > > cuser.cc
> > > > > > > > ```
> > > > > > > > #include "c.h"
> > > > > > > >
> > > > > > > > void cuser() {
> > > > > > > > C c;
> > > > > > > > c.f(); // This function may not be inlined when EXPORT is
> > > > > > > > __declspec(dllimport), so link may fail.
> > > > > > > > }
> > > > > > > > ```
> > > > > > > >
> > > > > > > > When cuser.cc and c.h are built to different library, cuser.cc
> > > > > > > > needs to be able to see dllexported C::f() when C::f() is not
> > > > > > > > inlined.
> > > > > > > > This is my understanding.
> > > > > > > Your example doesn't use explicit instantiation definition or
> > > > > > > declaration, so doesn't apply here I think.
> > > > > > >
> > > > > > > As long as the dllexport and dllimport attributes matches it's
> > > > > > > fine. It doesn't matter whether the function gets inlined or not,
> > > > > > > the only thing that matters is that if it's marked dllimport on
> > > > > > > the "consumer side", it must be dllexport when building the dll.
> > > > > > Hmm, I changed the linkage for functions having
> > > > > > DLLExport/ImportStaticLocal Attr.
> > > > > >
> > > > > I changed linkage in ASTContext so that inline function is emitted
> > > > > when it is necessary when we use fno-dllexport-inlines.
> > > > I revived explicit template instantiation check.
> > > > Found that this is necessary.
> > > >
> > > > For explicit template instantiation, inheriting dll attribute from
> > > > function for local static var is run before inheriting dll attribute
> > > > from class for member functions. So I cannot detect local static var of
> > > > inline function after class level dll attribute processing for explicit
> > > > template instantiation.
> > > >
> > > Oh I see, it's a static local problem..
> > > Can you provide a concrete example that does not work without your check?
> > > Maybe this is the right thing to do, but I would like to understand
> > > exactly what the problem is.
> > For the following code
> > ```
> > template
> > class M{
> > public:
> >
> > T Inclass() {
> > static T static_x;
> > ++static_x;
> > return static_x;
> > }
> > };
> >
> > template class __declspec(dllexport) M;
> >
> > extern template class __declspec(dllimport) M;
> >
> > int f (){
> > M mi;
> > M ms;
> > return mi.Inclass() + ms.Inclass();
> > }
> >
> > ```
> >
> > llvm code without instantiation check become like below. Both inline
> > functions of M and M is not dllimported/exported.
> > ```
> > $"?Inclass@?$M@H@@QEAAHXZ" = comdat any
> >
> > $"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA" = comdat any
> >
> > @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA" = linkonce_odr dso_local global
> > i32 0, comdat, align 4
> >
> > ; Function Attrs: noinline nounwind optnone
> > define weak_odr dso_local i32 @"?Inclass@?$M@H@@QEAAHXZ"(%class.M* %this)
> > #0 comdat align 2 {
> > entry:
> > %this.addr = alloca %class.M*, align 8
> > store %class.M* %this, %class.M** %this.addr, align 8
> > %this1 = load %class.M*, %class.M** %this.addr, align 8
> > %0 = load i32, i32* @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA", align 4
> > %inc = add nsw i32 %0, 1
> > store i32 %inc, i32* @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA", align 4
> > %1 = load i32, i32* @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA", align 4
> > ret i32 %1
> > }
> >
> > ; Function Attrs: noinline nounwind optnone
> > define dso_local i32 @"?f@@YAHXZ"() #0 {
> > entry:
> > %mi = alloca %class.M, align 1
> > %ms = alloca %class.M.0, align 1
> > %call = call i32 @"?Inclass@?$M@H@@QEAAHXZ"(%class.M* %mi)
> > %call1 = call i16 @"?Inclass@?$M@F@@QEAAFXZ"(%class.M.0* %ms)
> > %conv = sext i16 %call1 to i32
> > %add = add nsw i32 %call, %conv
> > ret i32 %add
> > }
> >
> > declare dso_local i16 @"?Inclass@?$M@F@@QEAAFXZ"(%class.M.0*) #1
> > ```
> >
> >
> > With the check, both functions are dllimported/exported and
