[PATCH] D75323: Support relative dest paths in headermap files
takuto.ikuta added a comment. Nico, can we merge this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75323/new/ https://reviews.llvm.org/D75323 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75323: Support relative dest paths in headermap files
takuto.ikuta added inline comments.
Comment at: clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json:1
+{"mappings":{"empty/empty.h":"include/empty.h"}}
better to format like other hmap.json?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75323/new/
https://reviews.llvm.org/D75323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75323: Support relative dest paths in headermap files
takuto.ikuta added inline comments. Comment at: clang/test/Headers/headermap_relpath.cpp:8 + +// RUN: cd %S; %clang_cc1 -I %S/Inputs/empty.hmap %s + generate header map using hmaptool like https://github.com/llvm/llvm-project/blob/e32ff096858578f526b6d05ab97c8f083f2e1834/clang/test/Preprocessor/include-header-missing-in-framework-with-headermap.c#L2 ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75323/new/ https://reviews.llvm.org/D75323 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75323: Support relative dest paths in headermap files
takuto.ikuta requested changes to this revision. takuto.ikuta added a comment. This revision now requires changes to proceed. add test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75323/new/ https://reviews.llvm.org/D75323 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps
takuto.ikuta added inline comments.
Comment at: clang/lib/Parse/ParseAST.cpp:154
if (HaveLexer) {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
P.Initialize();
anton-afanasyev wrote:
> takuto.ikuta wrote:
> > Remove StringRef?
> I use `StringRef("")` to explicitly cast to one of overloaded
> `TimeScope(StringRef, StringRef)` and `TimeScope(StringRef,
> fuction_ref(std::string()))`.
I think function_ref(std::string()) does not have constructor receiving
StringRef, so I feel explicit cast is redundant.
Comment at: llvm/lib/Support/TimeProfiler.cpp:103
+
+*OS << "{ \"traceEvents\": [\n";
+
anton-afanasyev wrote:
> takuto.ikuta wrote:
> > Don't we want to use json utility for this?
> > https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/Support/JSON.h
> >
> This implementation looks compact and fast. I've read proposal for this
> library
> https://docs.google.com/document/d/1OEF9IauWwNuSigZzvvbjc1cVS1uGHRyGTXaoy3DjqM4
> , it's recent, so I'm not sure it's stable and speed optimized. Do you
> actually can advise it? I can do it in the next refactor commit as well.
Hmm, I think using json library is preferred because we don't need to take care
the correctness of json format.
Confirming correctness of json format with many escaped literals is bit hard
and I'm afraid to miss json format error.
Comment at: llvm/lib/Support/TimeProfiler.cpp:44
+default:
+ if (isPrint(C)) {
+OS += C;
anton-afanasyev wrote:
> takuto.ikuta wrote:
> > include StringExtras.h for this?
> Should one do it? It's already implicitly included.
I think it is better to do, because if someone removes the StringExtras.h
include used for this file, they need to include header in this file at the
same time. That may require to touch unrelated files in their change.
Comment at: llvm/lib/Support/TimeProfiler.cpp:75
+ void end() {
+assert(!Stack.empty() && "Must call Begin first");
+auto &E = Stack.back();
s/Begin/begin/
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58675/new/
https://reviews.llvm.org/D58675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps
takuto.ikuta added inline comments.
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1352
+
+ llvm::TimeTraceScope TimeScope("Backend", StringRef(""));
+
We don't need explicit StringRef constructor call here. Just passing "" is
enough.
Comment at: clang/lib/Parse/ParseAST.cpp:154
if (HaveLexer) {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
P.Initialize();
Remove StringRef?
Comment at: clang/lib/Serialization/GlobalModuleIndex.cpp:131
NumIdentifierLookupHits() {
+ llvm::TimeTraceScope TimeScope("Module LoadIndex", StringRef(""));
// Read the global index.
Remove StringRef?
Comment at: clang/lib/Serialization/GlobalModuleIndex.cpp:745
using namespace llvm;
+ llvm::TimeTraceScope TimeScope("Module WriteIndex", StringRef(""));
Remove StringRef?
Comment at: clang/tools/driver/cc1_main.cpp:224
+ {
+llvm::TimeTraceScope TimeScope("ExecuteCompiler", StringRef(""));
+Success = ExecuteCompilerInvocation(Clang.get());
Remove StringRef?
Comment at: llvm/lib/Support/TimeProfiler.cpp:44
+default:
+ if (isPrint(C)) {
+OS += C;
include StringExtras.h for this?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58675/new/
https://reviews.llvm.org/D58675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps
takuto.ikuta added inline comments.
Comment at: llvm/lib/Support/TimeProfiler.cpp:28
+
+static std::string escapeString(const char *Src) {
+ std::string OS;
you can pass StringRef here and remove .data() or .c_str() from caller.
Comment at: llvm/lib/Support/TimeProfiler.cpp:33
+switch (C) {
+case '"':
+case '\\':
Include escape for '/' too.
https://tools.ietf.org/html/rfc8259#section-7
Comment at: llvm/lib/Support/TimeProfiler.cpp:44
+default:
+ if (C >= 32 && C < 126) {
+OS += C;
I prefer to use isPrint here.
https://github.com/llvm/llvm-project/blob/2946cd701067404b99c39fb29dc9c74bd7193eb3/llvm/include/llvm/ADT/StringExtras.h#L105
Comment at: llvm/lib/Support/TimeProfiler.cpp:72
+Entry E = {steady_clock::now(), {}, Name, Detail};
+Stack.emplace_back(E);
+ }
I prefer to write
```
Stack.push_back(std::move(E));
```
or
```
Stack.emplace_back(steady_clock::now(), {}, Name, Detail);
```
Comment at: llvm/lib/Support/TimeProfiler.cpp:103
+
+*OS << "{ \"traceEvents\": [\n";
+
Don't we want to use json utility for this?
https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/Support/JSON.h
Comment at: llvm/lib/Support/TimeProfiler.cpp:147
+ std::unordered_map TotalPerName;
+ std::unordered_map CountPerName;
+ time_point StartTime;
better to have one hash map so that we don't need to call 2 lookup in L92 and
L93.
Also StringMap may be faster than unordered_map.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58675/new/
https://reviews.llvm.org/D58675
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag
takuto.ikuta updated this revision to Diff 173810.
takuto.ikuta marked an inline comment as done.
takuto.ikuta added a comment.
rebase
Repository:
rL LLVM
https://reviews.llvm.org/D54426
Files:
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/MSVC.cpp
clang/test/Driver/cl-options.c
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -495,7 +495,7 @@
// RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck
-check-prefix=DllExportInlines %s
// DllExportInlines-NOT: "-fno-dllexport-inlines"
// RUN: %clang_cl /fallback /Zc:dllexportInlines- /c -### -- %s 2>&1 |
FileCheck -check-prefix=DllExportInlinesFallback %s
-// DllExportInlinesFallback: warning: option '/Zc:dllexportInlines-' is
ignored when /fallback happens [-Woption-ignored]
+// DllExportInlinesFallback: error: option '/Zc:dllexportInlines-' is
ABI-changing and not compatible with '/fallback'
// RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
// Zi: "-gcodeview"
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -669,12 +669,6 @@
// them too.
Args.AddAllArgs(CmdArgs, options::OPT_UNKNOWN);
- // Warning for ignored flag.
- if (const Arg *dllexportInlines =
- Args.getLastArg(options::OPT__SLASH_Zc_dllexportInlines_))
-C.getDriver().Diag(clang::diag::warn_drv_non_fallback_argument_clang_cl)
- << dllexportInlines->getAsString(Args);
-
// Input filename.
assert(Inputs.size() == 1);
const InputInfo &II = Inputs[0];
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5502,8 +5502,13 @@
if (Args.hasFlag(options::OPT__SLASH_Zc_dllexportInlines_,
options::OPT__SLASH_Zc_dllexportInlines,
- false))
+ false)) {
+ if (Args.hasArg(options::OPT__SLASH_fallback)) {
+ D.Diag(clang::diag::err_drv_dllexport_inlines_and_fallback);
+ } else {
CmdArgs.push_back("-fno-dllexport-inlines");
+ }
+ }
Arg *MostGeneralArg = Args.getLastArg(options::OPT__SLASH_vmg);
Arg *BestCaseArg = Args.getLastArg(options::OPT__SLASH_vmb);
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -161,9 +161,8 @@
"support for '/Yc' with more than one source file not implemented yet; flag
ignored">,
InGroup;
-def warn_drv_non_fallback_argument_clang_cl : Warning<
- "option '%0' is ignored when /fallback happens">,
- InGroup;
+def err_drv_dllexport_inlines_and_fallback : Error<
+ "option '/Zc:dllexportInlines-' is ABI-changing and not compatible with
'/fallback'">;
def err_drv_invalid_value : Error<"invalid value '%1' in '%0'">;
def err_drv_invalid_int_value : Error<"invalid integral value '%1' in '%0'">;
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -495,7 +495,7 @@
// RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s
// DllExportInlines-NOT: "-fno-dllexport-inlines"
// RUN: %clang_cl /fallback /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlinesFallback %s
-// DllExportInlinesFallback: warning: option '/Zc:dllexportInlines-' is ignored when /fallback happens [-Woption-ignored]
+// DllExportInlinesFallback: error: option '/Zc:dllexportInlines-' is ABI-changing and not compatible with '/fallback'
// RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
// Zi: "-gcodeview"
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -669,12 +669,6 @@
// them too.
Args.AddAllArgs(CmdArgs, options::OPT_UNKNOWN);
- // Warning for ignored flag.
- if (const Arg *dllexportInlines =
- Args.getLastArg(options::OPT__SLASH_Zc_dllexportInlines_))
-C.getDriver().Diag(clang::diag::warn_drv_non_fallback_argument_clang_cl)
- << dllexportInlines->getAsString(Args);
-
// Input filename.
assert(Inputs.size() == 1);
const InputInfo &II = Inputs[0];
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5502,8 +5502,13 @@
i
[PATCH] D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag
takuto.ikuta marked 2 inline comments as done.
takuto.ikuta added a comment.
Thank you for review!
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5529
+ false)) {
+ Arg *dllexportInlines =
Args.getLastArg(options::OPT__SLASH_Zc_dllexportInlines_);
+ if (Args.hasArg(options::OPT__SLASH_fallback) && dllexportInlines) {
hans wrote:
> We know there must be a OPT__SLASH_Zc_dllexportInlines_ flag at this point
> (because it's checked in the if above), so I think think you need "&&
> dllexportInlines" below.
>
> In the previous version, you output the getAsString() of the arg to the
> diagnostic. Don't you want to do this still?
Ah, I don't need to call getAsString() anymore.
Repository:
rL LLVM
https://reviews.llvm.org/D54426
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag
takuto.ikuta updated this revision to Diff 173670.
takuto.ikuta added a comment.
short diag
Repository:
rL LLVM
https://reviews.llvm.org/D54426
Files:
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/MSVC.cpp
clang/test/Driver/cl-options.c
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -495,7 +495,7 @@
// RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck
-check-prefix=DllExportInlines %s
// DllExportInlines-NOT: "-fno-dllexport-inlines"
// RUN: %clang_cl /fallback /Zc:dllexportInlines- /c -### -- %s 2>&1 |
FileCheck -check-prefix=DllExportInlinesFallback %s
-// DllExportInlinesFallback: warning: option '/Zc:dllexportInlines-' is
ignored when /fallback happens [-Woption-ignored]
+// DllExportInlinesFallback: error: option '/Zc:dllexportInlines-' is
ABI-changing and not compatible with '/fallback'
// RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
// Zi: "-gcodeview"
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -669,12 +669,6 @@
// them too.
Args.AddAllArgs(CmdArgs, options::OPT_UNKNOWN);
- // Warning for ignored flag.
- if (const Arg *dllexportInlines =
- Args.getLastArg(options::OPT__SLASH_Zc_dllexportInlines_))
-C.getDriver().Diag(clang::diag::warn_drv_non_fallback_argument_clang_cl)
- << dllexportInlines->getAsString(Args);
-
// Input filename.
assert(Inputs.size() == 1);
const InputInfo &II = Inputs[0];
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5525,8 +5525,13 @@
if (Args.hasFlag(options::OPT__SLASH_Zc_dllexportInlines_,
options::OPT__SLASH_Zc_dllexportInlines,
- false))
+ false)) {
+ if (Args.hasArg(options::OPT__SLASH_fallback)) {
+ D.Diag(clang::diag::err_drv_dllexport_inlines_and_fallback);
+ } else {
CmdArgs.push_back("-fno-dllexport-inlines");
+ }
+ }
Arg *MostGeneralArg = Args.getLastArg(options::OPT__SLASH_vmg);
Arg *BestCaseArg = Args.getLastArg(options::OPT__SLASH_vmb);
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -161,9 +161,8 @@
"support for '/Yc' with more than one source file not implemented yet; flag
ignored">,
InGroup;
-def warn_drv_non_fallback_argument_clang_cl : Warning<
- "option '%0' is ignored when /fallback happens">,
- InGroup;
+def err_drv_dllexport_inlines_and_fallback : Error<
+ "option '/Zc:dllexportInlines-' is ABI-changing and not compatible with
'/fallback'">;
def err_drv_invalid_value : Error<"invalid value '%1' in '%0'">;
def err_drv_invalid_int_value : Error<"invalid integral value '%1' in '%0'">;
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -495,7 +495,7 @@
// RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s
// DllExportInlines-NOT: "-fno-dllexport-inlines"
// RUN: %clang_cl /fallback /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlinesFallback %s
-// DllExportInlinesFallback: warning: option '/Zc:dllexportInlines-' is ignored when /fallback happens [-Woption-ignored]
+// DllExportInlinesFallback: error: option '/Zc:dllexportInlines-' is ABI-changing and not compatible with '/fallback'
// RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
// Zi: "-gcodeview"
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -669,12 +669,6 @@
// them too.
Args.AddAllArgs(CmdArgs, options::OPT_UNKNOWN);
- // Warning for ignored flag.
- if (const Arg *dllexportInlines =
- Args.getLastArg(options::OPT__SLASH_Zc_dllexportInlines_))
-C.getDriver().Diag(clang::diag::warn_drv_non_fallback_argument_clang_cl)
- << dllexportInlines->getAsString(Args);
-
// Input filename.
assert(Inputs.size() == 1);
const InputInfo &II = Inputs[0];
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5525,8 +5525,13 @@
if (Args.hasFlag(options::OPT__SLASH_Zc_dlle
[PATCH] D54319: clang-cl: Add documentation for /Zc:dllexportInlines-
takuto.ikuta accepted this revision. takuto.ikuta added a comment. This revision is now accepted and ready to land. Seems good document! https://reviews.llvm.org/D54319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54298: [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used with /fallback
takuto.ikuta marked an inline comment as done. takuto.ikuta added a comment. Thank you for review! Repository: rL LLVM https://reviews.llvm.org/D54298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54298: [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used with /fallback
takuto.ikuta updated this revision to Diff 173302. takuto.ikuta added a comment. warn in GetCommand Repository: rL LLVM https://reviews.llvm.org/D54298 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/lib/Driver/ToolChains/MSVC.cpp clang/test/Driver/cl-options.c Index: clang/test/Driver/cl-options.c === --- clang/test/Driver/cl-options.c +++ clang/test/Driver/cl-options.c @@ -494,6 +494,8 @@ // NoDllExportInlines: "-fno-dllexport-inlines" // RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s // DllExportInlines-NOT: "-fno-dllexport-inlines" +// RUN: %clang_cl /fallback /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlinesFallback %s +// DllExportInlinesFallback: warning: option '/Zc:dllexportInlines-' is ignored when /fallback happens [-Woption-ignored] // RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s // Zi: "-gcodeview" Index: clang/lib/Driver/ToolChains/MSVC.cpp === --- clang/lib/Driver/ToolChains/MSVC.cpp +++ clang/lib/Driver/ToolChains/MSVC.cpp @@ -669,6 +669,12 @@ // them too. Args.AddAllArgs(CmdArgs, options::OPT_UNKNOWN); + // Warning for ignored flag. + if (const Arg *dllexportInlines = + Args.getLastArg(options::OPT__SLASH_Zc_dllexportInlines_)) +C.getDriver().Diag(clang::diag::warn_drv_non_fallback_argument_clang_cl) + << dllexportInlines->getAsString(Args); + // Input filename. assert(Inputs.size() == 1); const InputInfo &II = Inputs[0]; Index: clang/include/clang/Basic/DiagnosticDriverKinds.td === --- clang/include/clang/Basic/DiagnosticDriverKinds.td +++ clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -161,6 +161,10 @@ "support for '/Yc' with more than one source file not implemented yet; flag ignored">, InGroup; +def warn_drv_non_fallback_argument_clang_cl : Warning< + "option '%0' is ignored when /fallback happens">, + InGroup; + def err_drv_invalid_value : Error<"invalid value '%1' in '%0'">; def err_drv_invalid_int_value : Error<"invalid integral value '%1' in '%0'">; def err_drv_invalid_remap_file : Error< Index: clang/test/Driver/cl-options.c === --- clang/test/Driver/cl-options.c +++ clang/test/Driver/cl-options.c @@ -494,6 +494,8 @@ // NoDllExportInlines: "-fno-dllexport-inlines" // RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s // DllExportInlines-NOT: "-fno-dllexport-inlines" +// RUN: %clang_cl /fallback /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlinesFallback %s +// DllExportInlinesFallback: warning: option '/Zc:dllexportInlines-' is ignored when /fallback happens [-Woption-ignored] // RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s // Zi: "-gcodeview" Index: clang/lib/Driver/ToolChains/MSVC.cpp === --- clang/lib/Driver/ToolChains/MSVC.cpp +++ clang/lib/Driver/ToolChains/MSVC.cpp @@ -669,6 +669,12 @@ // them too. Args.AddAllArgs(CmdArgs, options::OPT_UNKNOWN); + // Warning for ignored flag. + if (const Arg *dllexportInlines = + Args.getLastArg(options::OPT__SLASH_Zc_dllexportInlines_)) +C.getDriver().Diag(clang::diag::warn_drv_non_fallback_argument_clang_cl) + << dllexportInlines->getAsString(Args); + // Input filename. assert(Inputs.size() == 1); const InputInfo &II = Inputs[0]; Index: clang/include/clang/Basic/DiagnosticDriverKinds.td === --- clang/include/clang/Basic/DiagnosticDriverKinds.td +++ clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -161,6 +161,10 @@ "support for '/Yc' with more than one source file not implemented yet; flag ignored">, InGroup; +def warn_drv_non_fallback_argument_clang_cl : Warning< + "option '%0' is ignored when /fallback happens">, + InGroup; + def err_drv_invalid_value : Error<"invalid value '%1' in '%0'">; def err_drv_invalid_int_value : Error<"invalid integral value '%1' in '%0'">; def err_drv_invalid_remap_file : Error< ___ 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
takuto.ikuta added a comment. Thank you! I'll submit this if other people does not have other comments. Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:9 +// RUN: -emit-llvm -O1 -o - | \ +// RUN: FileCheck --check-prefix=CHECK %s + hans wrote: > Did the "EXPORTINLINE" check-prefix get lost here? Good catch! 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
takuto.ikuta updated this revision to Diff 172348.
takuto.ikuta marked an inline comment as done.
takuto.ikuta added a comment.
Fix check-prefix
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
clang/test/Driver/cl-options.c
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -490,6 +490,11 @@
// RUN: %clang_cl /Zc:threadSafeInit /c -### -- %s 2>&1 | FileCheck -check-prefix=ThreadSafeStatics %s
// ThreadSafeStatics-NOT: "-fno-threadsafe-statics"
+// RUN: %clang_cl /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=NoDllExportInlines %s
+// NoDllExportInlines: "-fno-dllexport-inlines"
+// RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s
+// DllExportInlines-NOT: "-fno-dllexport-inlines"
+
// RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
// Zi: "-gcodeview"
// Zi: "-debug-info-kind=limited"
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,133 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=CHECK --check-prefix=NOEXPORTINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=CHECK --check-prefix=EXPORTINLINE %s
+
+
+struct __declspec(dllexport) ExportedClass {
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InclassDefFunc@ExportedClass@@
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@ExportedClass@@
+ void InclassDefFunc() {}
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+// CHECK-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+// CHECK-DAG: @"?static_x@?2???R@?0??InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ@QEBA?A?@@XZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ inline void InlineOutclassDefFunc();
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InlineOutclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ inline int InlineOutclassDefFuncWithStaticVariable();
+
+ // CHECK-DAG: define dso_local dllexport void @"?OutoflineDefFunc@ExportedClass@@QEAAXXZ"
+ void OutoflineDefFunc();
+};
+
+void ExportedClass::OutoflineDefFunc() {}
+
+inline void ExportedClass::InlineOutclassDefFunc() {}
+
+inline int ExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void ExportedClassUser() {
+ ExportedClass a;
+ a.InclassDefFunc();
+ a.InlineOutclassDefFunc();
+}
+
+template
+struct __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+
+ int InclassDefFuncWithStaticVariable() {
+static int static_x = 0;
+return ++static_x;
+ }
+};
+
+class A11{};
+class B22{};
+
+// CHECK-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11QEAAXXZ"
+// CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VA11QEAAHXZ"
+// CHECK-DAG: @"?static_x@?2??InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+template class TemplateExportedClass;
+
+// NOEXPORTINLINE-DAG: def
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. Thank you for quick review! Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5715 + (MD->isThisDeclarationADefinition() || + MD->isImplicitlyInstantiable()) && + TSK != TSK_ExplicitInstantiationDeclaration && hans wrote: > The isThisDeclarationADefinition() and isImplicitlyInstantiable() checks > don't look right. Just checking isInlined() should be enough. I see. 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
takuto.ikuta updated this revision to Diff 172332.
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
clang/test/Driver/cl-options.c
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -490,6 +490,11 @@
// RUN: %clang_cl /Zc:threadSafeInit /c -### -- %s 2>&1 | FileCheck -check-prefix=ThreadSafeStatics %s
// ThreadSafeStatics-NOT: "-fno-threadsafe-statics"
+// RUN: %clang_cl /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=NoDllExportInlines %s
+// NoDllExportInlines: "-fno-dllexport-inlines"
+// RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s
+// DllExportInlines-NOT: "-fno-dllexport-inlines"
+
// RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
// Zi: "-gcodeview"
// Zi: "-debug-info-kind=limited"
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,133 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=CHECK --check-prefix=NOEXPORTINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=CHECK %s
+
+
+struct __declspec(dllexport) ExportedClass {
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InclassDefFunc@ExportedClass@@
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@ExportedClass@@
+ void InclassDefFunc() {}
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+// CHECK-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+// CHECK-DAG: @"?static_x@?2???R@?0??InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ@QEBA?A?@@XZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ inline void InlineOutclassDefFunc();
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InlineOutclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ inline int InlineOutclassDefFuncWithStaticVariable();
+
+ // CHECK-DAG: define dso_local dllexport void @"?OutoflineDefFunc@ExportedClass@@QEAAXXZ"
+ void OutoflineDefFunc();
+};
+
+void ExportedClass::OutoflineDefFunc() {}
+
+inline void ExportedClass::InlineOutclassDefFunc() {}
+
+inline int ExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void ExportedClassUser() {
+ ExportedClass a;
+ a.InclassDefFunc();
+ a.InlineOutclassDefFunc();
+}
+
+template
+struct __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+
+ int InclassDefFuncWithStaticVariable() {
+static int static_x = 0;
+return ++static_x;
+ }
+};
+
+class A11{};
+class B22{};
+
+// CHECK-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11QEAAXXZ"
+// CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VA11QEAAHXZ"
+// CHECK-DAG: @"?static_x@?2??InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+template class TemplateExportedClass;
+
+// NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InclassDefFunc@?$TemplateExportedClass@VB22QEAAXXZ"
+// EXPORTINLINE-DAG: define weak_
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 172331.
takuto.ikuta added a comment.
added a few more check
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
clang/test/Driver/cl-options.c
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -490,6 +490,11 @@
// RUN: %clang_cl /Zc:threadSafeInit /c -### -- %s 2>&1 | FileCheck -check-prefix=ThreadSafeStatics %s
// ThreadSafeStatics-NOT: "-fno-threadsafe-statics"
+// RUN: %clang_cl /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=NoDllExportInlines %s
+// NoDllExportInlines: "-fno-dllexport-inlines"
+// RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s
+// DllExportInlines-NOT: "-fno-dllexport-inlines"
+
// RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
// Zi: "-gcodeview"
// Zi: "-debug-info-kind=limited"
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,133 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=CHECK --check-prefix=NOEXPORTINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=CHECK %s
+
+
+struct __declspec(dllexport) ExportedClass {
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InclassDefFunc@ExportedClass@@
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@ExportedClass@@
+ void InclassDefFunc() {}
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+// CHECK-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+// CHECK-DAG: @"?static_x@?2???R@?0??InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ@QEBA?A?@@XZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ inline void InlineOutclassDefFunc();
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InlineOutclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ inline int InlineOutclassDefFuncWithStaticVariable();
+
+ // CHECK-DAG: define dso_local dllexport void @"?OutoflineDefFunc@ExportedClass@@QEAAXXZ"
+ void OutoflineDefFunc();
+};
+
+void ExportedClass::OutoflineDefFunc() {}
+
+inline void ExportedClass::InlineOutclassDefFunc() {}
+
+inline int ExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void ExportedClassUser() {
+ ExportedClass a;
+ a.InclassDefFunc();
+ a.InlineOutclassDefFunc();
+}
+
+template
+struct __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+
+ int InclassDefFuncWithStaticVariable() {
+// CHECK-DAG: @"?static_x@?2??InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+// CHECK-DAG: @"?static_x@?2??InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VB22QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_x = 0;
+return ++static_x;
+ }
+};
+
+class A11{};
+class B22{};
+
+// CHECK-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11QEAAXXZ"
+// CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@?
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 172330.
takuto.ikuta marked 10 inline comments as done.
takuto.ikuta added a comment.
Address comment
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
clang/test/Driver/cl-options.c
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -490,6 +490,11 @@
// RUN: %clang_cl /Zc:threadSafeInit /c -### -- %s 2>&1 | FileCheck -check-prefix=ThreadSafeStatics %s
// ThreadSafeStatics-NOT: "-fno-threadsafe-statics"
+// RUN: %clang_cl /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=NoDllExportInlines %s
+// NoDllExportInlines: "-fno-dllexport-inlines"
+// RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s
+// DllExportInlines-NOT: "-fno-dllexport-inlines"
+
// RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
// Zi: "-gcodeview"
// Zi: "-debug-info-kind=limited"
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,130 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=CHECK --check-prefix=NOEXPORTINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=CHECK %s
+
+
+struct __declspec(dllexport) ExportedClass {
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InclassDefFunc@ExportedClass@@
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@ExportedClass@@
+ void InclassDefFunc() {}
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+// CHECK-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+// CHECK-DAG: @"?static_x@?2???R@?0??InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ@QEBA?A?@@XZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ inline void InlineOutclassDefFunc();
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InlineOutclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ inline int InlineOutclassDefFuncWithStaticVariable();
+
+ // CHECK-DAG: define dso_local dllexport void @"?OutoflineDefFunc@ExportedClass@@QEAAXXZ"
+ void OutoflineDefFunc();
+};
+
+void ExportedClass::OutoflineDefFunc() {}
+
+inline void ExportedClass::InlineOutclassDefFunc() {}
+
+inline int ExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void ExportedClassUser() {
+ ExportedClass a;
+ a.InclassDefFunc();
+ a.InlineOutclassDefFunc();
+}
+
+template
+struct __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+
+ int InclassDefFuncWithStaticVariable() {
+// CHECK-DAG: @"?static_x@?2??InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_x = 0;
+return ++static_x;
+ }
+};
+
+class A11{};
+class B22{};
+
+// CHECK-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11QEAAXXZ"
+// CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VA11QEAAHXZ"
+template class TemplateExportedClass;
+
+// NOEXPORTINLINE-DAG: define linkonce_odr dso_loca
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta marked an inline comment as done. takuto.ikuta added a comment. I missed the comment about driver flag test. 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
takuto.ikuta updated this revision to Diff 172317.
takuto.ikuta added a comment.
Added cl driver flag test
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
clang/test/Driver/cl-options.c
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -490,6 +490,11 @@
// RUN: %clang_cl /Zc:threadSafeInit /c -### -- %s 2>&1 | FileCheck -check-prefix=ThreadSafeStatics %s
// ThreadSafeStatics-NOT: "-fno-threadsafe-statics"
+// RUN: %clang_cl /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=NoDllExportInlines %s
+// NoDllExportInlines: "-fno-dllexport-inlines"
+// RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s
+// DllExportInlines-NOT: "-fno-dllexport-inlines"
+
// RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
// Zi: "-gcodeview"
// Zi: "-debug-info-kind=limited"
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,142 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=CHECK --check-prefix=NOEXPORTINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=STATIC %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=CHECK %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=STATIC %s
+
+
+
+struct __declspec(dllexport) ExportedClass {
+
+ // NOEXPORTINLINE-NOT: ?InclassDefFunc@ExportedClass@@
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@ExportedClass@@
+ void InclassDefFunc() {}
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+// STATIC-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+// STATIC-DAG: @"?static_x@?2???R@?0??InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ@QEBA?A?@@XZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // CHECK-NOT: InlineOutclassDefFuncWihtoutDefinition
+ inline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // CHECK-DAG:define weak_odr dso_local dllexport void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ inline void InlineOutclassDefFunc();
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InlineOutclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ inline int InlineOutclassDefFuncWithStaticVariable();
+
+ // CHECK-DAG: define dso_local dllexport void @"?OutclassDefFunc@ExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void ExportedClass::OutclassDefFunc() {}
+
+inline void ExportedClass::InlineOutclassDefFunc() {}
+
+inline int ExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) ExportedClassUser() {
+ ExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+struct __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+};
+
+class A11{};
+class B22{};
+class C33{};
+class D44{};
+
+// EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11AEAAX
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment.
Hans, thank you for review! I addressed all your comment and fixed small
behavior.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:76
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
hans wrote:
> Hmm, I would have thought we should export this function.
I see.
Added `MD->isThisDeclarationADefinition() ` check in
`Sema::checkClassLevelDLLAttribute`
This will make change like
https://chromium-review.googlesource.com/c/v8/v8/+/1186017 unnecessary.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:122
+template class TemplateExportedClass;
+
+
hans wrote:
> We should also have a test with implicit instantiation, and then the inline
> function should not be exported when using /Zc:dllexportInlines-.
Added test and `MD->isImplicitlyInstantiable()` check for the test.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:172
+ // INLINE-DAG: declare dllimport i32
@"?InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ"(%class.ImportedClass*)
#2
+ // NOINLINE-NOT:
declare{{.*}}"?InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ"
+ int InClassDefFuncWithStaticVariable() {
hans wrote:
> This check looks wrong. I think there should be a dllimport declaration for
> this function? (Or an available_externally definition if we use -O1?)
I think it is OK not dllimporting this function as far as static variable is
dllimported and definition of this function is emitted.
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
takuto.ikuta updated this revision to Diff 172304.
takuto.ikuta marked 17 inline comments as done.
takuto.ikuta added a comment.
Simplify test and fix some behavior.
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,142 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=CHECK --check-prefix=NOEXPORTINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=STATIC %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=CHECK %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=STATIC %s
+
+
+
+struct __declspec(dllexport) ExportedClass {
+
+ // NOEXPORTINLINE-NOT: ?InclassDefFunc@ExportedClass@@
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@ExportedClass@@
+ void InclassDefFunc() {}
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+// STATIC-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+// STATIC-DAG: @"?static_x@?2???R@?0??InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ@QEBA?A?@@XZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // CHECK-NOT: InlineOutclassDefFuncWihtoutDefinition
+ inline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // CHECK-DAG:define weak_odr dso_local dllexport void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ inline void InlineOutclassDefFunc();
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InlineOutclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ inline int InlineOutclassDefFuncWithStaticVariable();
+
+ // CHECK-DAG: define dso_local dllexport void @"?OutclassDefFunc@ExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void ExportedClass::OutclassDefFunc() {}
+
+inline void ExportedClass::InlineOutclassDefFunc() {}
+
+inline int ExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) ExportedClassUser() {
+ ExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+struct __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+};
+
+class A11{};
+class B22{};
+class C33{};
+class D44{};
+
+// EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11AEAAXXZ"
+template class __declspec(dllexport) TemplateExportedClass;
+
+// EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VB22AEAAXXZ"
+template class TemplateExportedClass;
+
+// NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InclassDefFunc@?$TemplateExportedClass@VC33QEAAXXZ"
+// EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VC33QEAAXXZ
+TemplateExportedClass c33;
+
+void c33user() {
+ c33.InclassDefFunc();
+}
+
+
+template
+struct TemplateNoExportedClass {
+ void InclassDefFunc() {}
+ int InclassDefFuncWithStaticLocal() {
+static int static_x;
+return ++static_x;
+ }
+};
+
+// CHECK-DAG: define weak_odr dso_local dllexport v
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. Thank you for review! Comment at: clang/include/clang/Basic/LangOptions.h:246 + /// If set, dllexported classes dllexport their inline methods. + bool DllExportInlines = true; + rnk wrote: > We should define this in the LangOptions.def file. Moved. 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
takuto.ikuta updated this revision to Diff 172090.
takuto.ikuta marked 2 inline comments as done.
takuto.ikuta added a comment.
Added option to LangOptions.def
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,192 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+
+// DEFAULT-DAG: @"?static_x@?2??InclassDefFuncWithStaticLocal@?$TemplateNoExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefF
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta marked an inline comment as done.
takuto.ikuta added inline comments.
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
hans wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > takuto.ikuta wrote:
> > > > takuto.ikuta wrote:
> > > > > hans wrote:
> > > > > > Can you give an example for why this is needed?
> > > > > Sorry, this change does not need. Removed.
> > > > Sorry, this change is necessary still.
> > > >
> > > > Without this, definition of inline function in explicit template
> > > > instantiation declaration is not be emitted, due to
> > > > GVA_AvailableExternally linkage.
> > > > But we stop exporting definition of inline function in explicit
> > > > template instantiation definition too.
> > > >
> > > > So without this, definition of dllimported inline function of explicit
> > > > template instantiation declaration won't be available.
> > > >
> > > Can you provide a code example of why this is needed?
> > If we don't change function linkage, following code will be compiled like
> > below with -fno-dllexport-inlines.
> >
> > ```
> > template
> > class M{
> > public:
> > void foo() {}
> > };
> >
> > template class __declspec(dllexport) M;
> >
> > extern template class __declspec(dllimport) M;
> >
> > void f (){
> > M mi;
> > mi.foo();
> >
> > M ms;
> > ms.foo();
> > }
> > ```
> >
> > ```
> > $"?foo@?$M@H@@QEAAXXZ" = comdat any
> >
> > ; Function Attrs: noinline nounwind optnone
> > define weak_odr dso_local void @"?foo@?$M@H@@QEAAXXZ"(%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
> > ret void
> > }
> >
> > ; Function Attrs: noinline nounwind optnone
> > define dso_local void @"?f@@YAXXZ"() #0 {
> > entry:
> > %mi = alloca %class.M, align 1
> > %ms = alloca %class.M.0, align 1
> > call void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %mi)
> > call void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0* %ms)
> > ret void
> > }
> >
> > declare dso_local void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0*) #1
> > ```
> >
> > M::foo() is declared, but inline function is not dllexported (See
> > M::foo() is not dllexported). So this code cannot be linked because
> > definition of M::foo does not exist. If the function is properly
> > inlined, we don't need to have this code. But I'm not sure why the function
> > is not inlined.
> Interesting. I wonder how -fvisibility-inlines-hidden handles this...
>
>
> ```
> template struct S {
> void foo() {}
> };
>
> template struct S;
>
> void use() {
> S s;
> s.foo();
> }
>
> $ g++ -fvisibility-inlines-hidden -c a.cc && objdump -t a.o | grep
> _ZN1SIiE3fooEv
> ld .text._ZN1SIiE3fooEv
> .text._ZN1SIiE3fooEv
> wF .text._ZN1SIiE3fooEv 000b _ZN1SIiE3fooEv
> <--- Not hidden.
> ```
>
> (If I comment out the explicit instantiation definition above, foo() is
> hidden as expected.)
>
> Okay, it seems that for explicit instantiation definitions,
> -fvisibility-inlines-hidden does not apply.
>
> And thinking more about it, that makes sense.
>
> I don't think we should change the linkage like this though, I think we
> should just not apply the /Zc:dllexportInlines- for explicit instantiation
> decls and definitions in checkClassLevelDLLAttribute(). I realize you had
> code to check for this before, but now we have a good and well understood
> reason.
Thank you for confirmation. I revived the check.
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
takuto.ikuta updated this revision to Diff 171916.
takuto.ikuta added a comment.
export/import explicit template instantiation function
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,192 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+
+// DEFAULT-DAG: @"?static_x@?2??InclassDefFuncWithStaticLocal@?$TemplateNoExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValu
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added inline comments.
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
hans wrote:
> takuto.ikuta wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > Can you give an example for why this is needed?
> > > Sorry, this change does not need. Removed.
> > Sorry, this change is necessary still.
> >
> > Without this, definition of inline function in explicit template
> > instantiation declaration is not be emitted, due to GVA_AvailableExternally
> > linkage.
> > But we stop exporting definition of inline function in explicit template
> > instantiation definition too.
> >
> > So without this, definition of dllimported inline function of explicit
> > template instantiation declaration won't be available.
> >
> Can you provide a code example of why this is needed?
If we don't change function linkage, following code will be compiled like below
with -fno-dllexport-inlines.
```
template
class M{
public:
void foo() {}
};
template class __declspec(dllexport) M;
extern template class __declspec(dllimport) M;
void f (){
M mi;
mi.foo();
M ms;
ms.foo();
}
```
```
$"?foo@?$M@H@@QEAAXXZ" = comdat any
; Function Attrs: noinline nounwind optnone
define weak_odr dso_local void @"?foo@?$M@H@@QEAAXXZ"(%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
ret void
}
; Function Attrs: noinline nounwind optnone
define dso_local void @"?f@@YAXXZ"() #0 {
entry:
%mi = alloca %class.M, align 1
%ms = alloca %class.M.0, align 1
call void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %mi)
call void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0* %ms)
ret void
}
declare dso_local void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0*) #1
```
M::foo() is declared, but inline function is not dllexported (See
M::foo() is not dllexported). So this code cannot be linked because
definition of M::foo does not exist. If the function is properly
inlined, we don't need to have this code. But I'm not sure why the function is
not inlined.
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
takuto.ikuta added a comment.
Thank you for quick fix!
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
takuto.ikuta wrote:
> hans wrote:
> > Can you give an example for why this is needed?
> Sorry, this change does not need. Removed.
Sorry, this change is necessary still.
Without this, definition of inline function in explicit template instantiation
declaration is not be emitted, due to GVA_AvailableExternally linkage.
But we stop exporting definition of inline function in explicit template
instantiation definition too.
So without this, definition of dllimported inline function of explicit template
instantiation declaration won't be available.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
hans wrote:
> 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
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 171877.
takuto.ikuta added a comment.
Rebased to take r345699
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/AST/ASTContext.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,199 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+
+// DEFAULT-DAG: @"?static_x@?2??InclassDefFuncWithStaticLocal@?$TemplateNoExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_x@?2??InclassDefFuncWithStaticLocal@?$TemplateNoExportedClass@VD44QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a
[PATCH] D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496)
takuto.ikuta accepted this revision. takuto.ikuta added a comment. LGTM, thank you for fix! 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
takuto.ikuta added a comment.
In https://reviews.llvm.org/D51340#1278799, @hans wrote:
> 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.
Yeah, that is the reason of few chromium code changes.
https://chromium-review.googlesource.com/c/chromium/src/+/1212379
> 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.
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.
$ 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
This is the same behavior with `/Zc:dllexportInlines-`.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
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 diff
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 171466.
takuto.ikuta marked an inline comment as done.
takuto.ikuta added a comment.
Added explanation comment for added attributes and rebased
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,181 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void Template
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added inline comments.
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
hans wrote:
> Can you give an example for why this is needed?
Sorry, this change does not need. Removed.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
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.
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
takuto.ikuta added a comment. ping? Can I go forward in this way? 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
takuto.ikuta added a comment.
Herald added a subscriber: nhaehnle.
Hans, I addressed all your comments.
How do you think about current implementation?
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
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.
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
takuto.ikuta updated this revision to Diff 169792.
takuto.ikuta added a comment.
remove comment out code
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/AST/ASTContext.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,174 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 169791.
takuto.ikuta added a comment.
Fix linkage for inline function of explicit template instantiation declaration
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/AST/ASTContext.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,174 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTempl
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 169783.
takuto.ikuta added a comment.
Remove unnecessary attr creation
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/AST/ASTContext.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,167 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+cla
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 169652.
takuto.ikuta retitled this revision from "[WIP] Add /Zc:DllexportInlines option
to clang-cl" to "Add /Zc:DllexportInlines option to clang-cl".
takuto.ikuta added a comment.
Export function inside explicit template instantiation definition
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/AST/ASTContext.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,167 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T tem
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added inline comments.
Comment at: clang/lib/Sema/SemaDecl.cpp:11976
+
+while (FD && !getDLLAttr(FD) &&
+ !FD->hasAttr() &&
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.
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:
> > 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?
> I see. I'll investigate which code path emits static variables
static local variable seems to be exported in
https://github.com/llvm-project/llvm-project-20170507/blob/f54514c50350743d3d1a3c5aa80cbe4bb449eb71/clang/lib/CodeGen/CGDecl.cpp#L378
But emitting static local var only by bypassing function emission seems
difficult.
Let me leave as-is here.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
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.
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
takuto.ikuta updated this revision to Diff 169365.
takuto.ikuta added a comment.
Address comment
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/AST/ASTContext.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,164 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+class A11{};
+class
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added inline comments.
Comment at: clang/lib/Sema/SemaDecl.cpp:11976
+
+while (FD && !getDLLAttr(FD) &&
+ !FD->hasAttr() &&
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?
Comment at: clang/lib/Sema/SemaDecl.cpp:11995
+
+// Function having static local variables should be exported.
+auto *ExportAttr =
cast(NewAttr->clone(getASTContext()));
hans wrote:
> 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?
I see. I'll investigate which code path emits static variables
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
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.
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
takuto.ikuta added a comment.
Thank you for review! I updated the code.
Comment at: clang/lib/Sema/SemaDecl.cpp:11976
+
+while (FD && !getDLLAttr(FD) &&
+ !FD->hasAttr() &&
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; })();
}
};
```
Comment at: clang/lib/Sema/SemaDecl.cpp:11995
+
+// Function having static local variables should be exported.
+auto *ExportAttr =
cast(NewAttr->clone(getASTContext()));
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.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5705
+TSK != TSK_ExplicitInstantiationDeclaration &&
+TSK != TSK_ExplicitInstantiationDefinition) {
+ if (ClassExported) {
hans wrote:
> 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?
> But I don't understand why the template stuff is checked here...
Templated inline function is not always inlined, it seems depending on
optimization level.
I updated the code as you wrote in later part of comment.
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
takuto.ikuta updated this revision to Diff 169179.
takuto.ikuta marked an inline comment as done.
takuto.ikuta added a comment.
address comment
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,163 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+clas
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment.
Thank you for review!
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5599
+bool Sema::isInlineFunctionDLLExportable(const CXXMethodDecl *MD) {
+ assert(MD->isInlined());
hans wrote:
> 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?
I implemented your idea the way I understood.
Use new attribute to check static local var later.
Due to difference of treating between linkage and dll attribute, I inherit dll
attribute in Sema/SemaDecl.cpp instead of AST/Decl.cpp
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
takuto.ikuta updated this revision to Diff 168979.
takuto.ikuta added a comment.
address comment
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,160 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+class A11{};
+class B22{};
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11@@
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?OutclassDefFu
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1256299, @hans wrote: > In https://reviews.llvm.org/D51340#1254898, @takuto.ikuta wrote: > > > Ping? > > > Sorry, I know I'm being slow to look at this :-( I have not forgotten, it's > just a lot of other things happening at the same time. I see. Sorry for rushing you. I wait until you have time. 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
takuto.ikuta added a comment. Ping? 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
takuto.ikuta added inline comments.
Comment at: clang/lib/Sema/SemaDecl.cpp:12624
+ isa(D)) {
+CXXMethodDecl *MD = dyn_cast(D);
+CXXRecordDecl *Class = MD->getParent();
hans wrote:
> Hmm, now we're adding an AST walk over all inline methods which probably
> slows us down a bit. Not sure I have any better ideas though.
>
> In any case, ActOnFinishInlineFunctionDef needs a comment explaining why it's
> doing this.
Added comment. I think typical code does not have static variables in inline
function and this check is worth to be done for the performance improvement.
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
takuto.ikuta added a comment.
Thank you for review!
In https://reviews.llvm.org/D51340#1246508, @hans wrote:
> The static local stuff still makes me nervous. How much does Chromium break
> if we just ignore the problem? And how does -fvisibility-inlines-hidden
> handle the issue?
I'm not sure how much chromium will break, if we ignore static local variables.
But ignoring static local var may cause some unhappy behavior and experience to
developer.
I'd like to have check for local static variables as
-fvisibility-inlines-hidden does.
-fvisibility-inlines-hidden checks static local around here.
https://github.com/llvm-project/llvm-project-20170507/blob/77b7ca42b0ea496373c5e5a9710b1fe11e1fba68/clang/lib/AST/Decl.cpp#L1265
> Also, Sema already has a check for static locals in C inline functions:
>
> $ echo "inline void f() { static int x; }" | bin/clang -c -x c -
> :1:19: warning: non-constant static local variable in inline
> function may be different in different files [-Wstatic-local-in-inline]
> inline void f() { static int x; }
> ^
>
>
>
> could we reuse that check somehow for this case with static locals in
> dllexport inline methods?
I think we can do the same thing when we are given -fno-dllexport-inlines
around here.
https://github.com/llvm-project/llvm-project-20170507/blob/77b7ca42b0ea496373c5e5a9710b1fe11e1fba68/clang/lib/Sema/SemaDecl.cpp#L6661
But I bit prefer to do export functions with static local variables. Because
that is consistent with -fvisibility-inlines-hidden and we won't need more
changes to chromium than the CLs in description.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5702
+!getLangOpts().DllExportInlines &&
+Class->getTemplateSpecializationKind() !=
TSK_ExplicitInstantiationDeclaration &&
+Class->getTemplateSpecializationKind() !=
TSK_ExplicitInstantiationDefinition) {
hans wrote:
> What worries me is that now we have logic in two different places about
> inheriting the dll attribute.
>
> The new place in ActOnFinishInlineFunctionDef doesn't have the same checks
> (checking for MS ABI and the template specialization kind) which means the
> logic for when to inherit is already a little out of sync...
Agree. Do you allow me to extract check to function and re-use that?
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
takuto.ikuta updated this revision to Diff 167683.
takuto.ikuta added a comment.
Update comment for Sema::ActOnFinishInlineFunctionDef
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/include/clang/Sema/Sema.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,159 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+class A11{};
+class B22{};
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11@@
+// DEFAULT-DAG: define weak_odr dso_local dll
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 167682.
takuto.ikuta added a comment.
Extract inline function export check to function
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/include/clang/Sema/Sema.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,159 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+class A11{};
+class B22{};
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11@@
+// DEFAULT-DAG: define weak_odr dso_local dllexpor
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1243453, @hans wrote: > In https://reviews.llvm.org/D51340#1243331, @takuto.ikuta wrote: > > > Ping? > > > > This patch reduced obj size largely, and I expect this makes distributed > > build (like goma) faster by reducing data transferred on network. > > > I'll try to look at it this week. > > Have you confirmed on some Chromium file, e.g. stroke_opacity_custom.cc > (go/stroke-opactity-custom) that the number of functions that we codegen > decreases as expected? I'd expect this to save a lot of compile time. Yes, stroke_opacity_custom.obj export 18 symbols, reduced from 781 in PCH build, compile time is reduced from 8.2 seconds to 6.7 seconds. 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
takuto.ikuta added a comment.
Ping?
This patch reduced obj size largely, and I expect this makes distributed build
(like goma) faster by reducing data transferred on network.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244
+ false))
+CmdArgs.push_back("-fvisibility-inlines-hidden");
+
takuto.ikuta wrote:
> rnk wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > 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.
> > > >
> > > I confirmed that -fvisibility-inlines-hidden treats local static var
> > > correctly in linux.
> > > So I'm trying to export inline functions if it has local static variables.
> > This sounds like it would be really hard in general, since you can hide
> > static locals almost anywhere:
> > ```
> > struct Foo {
> > static int foo() {
> > return ([]() { static int x; return ++x; })();
> > }
> > };
> > ```
> > Can we reuse the RecursiveASTVisitor @hans added to check if we can emit
> > dllimport inline functions as available_externally definitions? I think it
> > will find exactly the circumstances we are looking for, i.e. export all the
> > things that cannot be emitted inline in other DLLs.
> Actually, StmtVisitor added dll export attribute to local static variable in
> lambda function. And static variables seems exported.
>
> But I found other issue in current implementation, some cxx method decls
> passed to emitting function before local static variable checker adds dll
> export attributes. In such case local static variables are not exported and
> link failure happens.
>
> So let me try to use DLLImportFunctionVisitor in
> CodeGenModule::shouldEmitFunction for exported function instead of
> processing/checking dll attribute around SemaDeclCXX.
I think avoiding dll export is better to be done before we reached around
CodeGenModule. Also removing dll attribute later made it difficult to pass
tests.
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
takuto.ikuta updated this revision to Diff 166450.
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,159 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+class A11{};
+class B22{};
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11@@
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?OutclassDefFunc@?$TemplateExportedClass@VA11@@
+template class __declspec(dllexport) TemplateExportedClas
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 166448.
takuto.ikuta edited the summary of this revision.
takuto.ikuta added a comment.
Remove unnecessary willHaveBody check condition
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// DEFAULT-DAG: @"?static_variable@?1??InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFu
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. PTAL again. I confirmed that current patch can link chrome and functions with local static variable are exported. But current ToT clang was not improved well by this patch. I guess there is some change recently making effect of this patch smaller. Or chromium has many inline functions with static local variable? 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
takuto.ikuta updated this revision to Diff 166087.
takuto.ikuta retitled this revision from "[WIP] Add /Zc:DllexportInlines option
to clang-cl" to "Add /Zc:DllexportInlines option to clang-cl".
takuto.ikuta edited the summary of this revision.
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// DEFAULT-DAG: @"?static_variable@?1??InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExp
[PATCH] D51340: [WIP] Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 165966.
takuto.ikuta edited the summary of this revision.
takuto.ikuta added a comment.
Current implementation cannot build chrome when pch is enabled.
undefined symbol error happens during linking blink_modules.dll
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-import-fvisibility-inlines-hidden.cpp
clang/test/CodeGenCXX/hidden-dllimport.cpp
Index: clang/test/CodeGenCXX/hidden-dllimport.cpp
===
--- clang/test/CodeGenCXX/hidden-dllimport.cpp
+++ clang/test/CodeGenCXX/hidden-dllimport.cpp
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -fvisibility-inlines-hidden -o - %s | FileCheck %s
-// We used to declare this hidden dllimport, which is contradictory.
+// We don't declare this hidden dllimport.
// CHECK: declare dllimport void @"?bar@foo@@QEAAXXZ"(%struct.foo*)
Index: clang/test/CodeGenCXX/dllexport-import-fvisibility-inlines-hidden.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-import-fvisibility-inlines-hidden.cpp
@@ -0,0 +1,161 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// DEFAULT-DAG: @"?static_variable@?1??InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+
+
+ // Copy assignment operator.
+ // DEFAULT-DAG: define weak_odr dso_local dllexport dereferenceable(1) %class.NoTemplateExportedClass* @"??4NoTemplateExportedClass@@QEAAAEAV0@AEBV0@@Z"
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void N
[PATCH] D51340: [WIP] Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added inline comments.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244
+ false))
+CmdArgs.push_back("-fvisibility-inlines-hidden");
+
rnk wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > 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.
> > >
> > I confirmed that -fvisibility-inlines-hidden treats local static var
> > correctly in linux.
> > So I'm trying to export inline functions if it has local static variables.
> This sounds like it would be really hard in general, since you can hide
> static locals almost anywhere:
> ```
> struct Foo {
> static int foo() {
> return ([]() { static int x; return ++x; })();
> }
> };
> ```
> Can we reuse the RecursiveASTVisitor @hans added to check if we can emit
> dllimport inline functions as available_externally definitions? I think it
> will find exactly the circumstances we are looking for, i.e. export all the
> things that cannot be emitted inline in other DLLs.
Actually, StmtVisitor added dll export attribute to local static variable in
lambda function. And static variables seems exported.
But I found other issue in current implementation, some cxx method decls passed
to emitting function before local static variable checker adds dll export
attributes. In such case local static variables are not exported and link
failure happens.
So let me try to use DLLImportFunctionVisitor in
CodeGenModule::shouldEmitFunction for exported function instead of
processing/checking dll attribute around SemaDeclCXX.
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
takuto.ikuta added inline comments.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244
+ false))
+CmdArgs.push_back("-fvisibility-inlines-hidden");
+
hans wrote:
> 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.
>
I confirmed that -fvisibility-inlines-hidden treats local static var correctly
in linux.
So I'm trying to export inline functions if it has local static variables.
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
takuto.ikuta updated this revision to Diff 164822.
takuto.ikuta added a comment.
I'm trying to handle local static var correctly.
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Driver/CLCompatOptions.td
clang/include/clang/Sema/Sema.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/CodeGenCXX/dllexport-import-fvisibility-inlines-hidden.cpp
clang/test/CodeGenCXX/hidden-dllimport.cpp
Index: clang/test/CodeGenCXX/hidden-dllimport.cpp
===
--- clang/test/CodeGenCXX/hidden-dllimport.cpp
+++ clang/test/CodeGenCXX/hidden-dllimport.cpp
@@ -1,8 +1,8 @@
// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -fvisibility-inlines-hidden -o - %s | FileCheck %s
-// We used to declare this hidden dllimport, which is contradictory.
+// We don't declare this hidden dllimport.
-// CHECK: declare dllimport void @"?bar@foo@@QEAAXXZ"(%struct.foo*)
+// CHECK-NOT: declare dllimport void @"?bar@foo@@QEAAXXZ"(%struct.foo*)
struct __attribute__((dllimport)) foo {
void bar() {}
Index: clang/test/CodeGenCXX/dllexport-import-fvisibility-inlines-hidden.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-import-fvisibility-inlines-hidden.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fvisibility-inlines-hidden -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// NOINLINE-DAG: define weak_odr hidden dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+// INLINE-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// DEFAULT-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+// DEFAULT-DAG: @"?static_variable@?1??InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// local static var in ImportedClass
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+// INLINE-NOT: static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // INLINE-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ // NOINLINE-DAG: define weak_odr hidden dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+vo
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added inline comments.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244
+ false))
+CmdArgs.push_back("-fvisibility-inlines-hidden");
+
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.
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
takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1226989, @takuto.ikuta wrote: > In https://reviews.llvm.org/D51340#1222013, @hans wrote: > > > Did both your builds use PCH? It'd be interesting to see the difference > > without PCH too; the effect should be even larger. > > > Added stats of without PCH build. > > > The summary should probably reference > > https://bugs.llvm.org/show_bug.cgi?id=33628 and it needs to mention how it > > affects dllimport too. > > Added to description, thanks! > > > Okay, after reading through the patch, it seems we're still marking class > > members dllexport, and then you selectively remove the attribute later. > > That does feel a little bit backward... Does -fvisibility-inlines-hidden > > also have the static local problem, or how does that flag handle it? > > Ah, maybe I can get performance improvement just support > fvisibility-inlines-hidden in clang-cl. Let me try. I just support fvisibility-inlines-hidden in clang-cl and that looks to work intended. 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
takuto.ikuta updated this revision to Diff 164379.
takuto.ikuta edited the summary of this revision.
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-inline.cpp
clang/test/CodeGenCXX/hidden-dllimport.cpp
Index: clang/test/CodeGenCXX/hidden-dllimport.cpp
===
--- clang/test/CodeGenCXX/hidden-dllimport.cpp
+++ clang/test/CodeGenCXX/hidden-dllimport.cpp
@@ -1,8 +1,8 @@
// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -fvisibility-inlines-hidden -o - %s | FileCheck %s
-// We used to declare this hidden dllimport, which is contradictory.
+// We don't declare this hidden dllimport.
-// CHECK: declare dllimport void @"?bar@foo@@QEAAXXZ"(%struct.foo*)
+// CHECK-NOT: declare dllimport void @"?bar@foo@@QEAAXXZ"(%struct.foo*)
struct __attribute__((dllimport)) foo {
void bar() {}
Index: clang/test/CodeGenCXX/dllexport-no-inline.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-inline.cpp
@@ -0,0 +1,148 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fvisibility-inlines-hidden -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// NOINLINE-DAG: define weak_odr hidden dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+// INLINE-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+// NOINLINE-DAG: @"?static_const_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HB" = linkonce_odr dso_local constant i32 1, comdat, align 4
+// INLINE-DAG: @"?static_const_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HB" = weak_odr dso_local dllexport constant i32 1, comdat, align 4
+// NOINLINE-DAG: @"?static_const_array@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4QBHB" = linkonce_odr dso_local constant [3 x i32] [i32 1, i32 2, i32 3], comdat, align 4
+// INLINE-DAG: @"?static_const_array@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4QBHB" = weak_odr dso_local dllexport constant [3 x i32] [i32 1, i32 2, i32 3], comdat, align 4
+// NOINLINE-DAG: @"?static_variable_non_const_cse@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 4, comdat, align 4
+// INLINE-DAG: @"?static_variable_non_const_cse@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 4, comdat, align 4
+// NOINLINE-DAG: @"?static_variable@?1??InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // NOINLINE-NOT: InclassDefFuncWithStaticVariable@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = f();
+static const int static_const_variable = 1;
+// DEFAULT-NOT: static_constexpr_variable
+static constexpr int static_constexpr_variable = 2;
+static const int static_const_array[] = {1, 2, 3};
+static int static_variable_non_const_cse = 4;
+
+++static_variable_non_const_cse;
+++static_variable;
+return static_const_variable + static_constexpr_variable +
+
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta marked 2 inline comments as done. takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1222013, @hans wrote: > Did both your builds use PCH? It'd be interesting to see the difference > without PCH too; the effect should be even larger. Added stats of without PCH build. > The summary should probably reference > https://bugs.llvm.org/show_bug.cgi?id=33628 and it needs to mention how it > affects dllimport too. Added to description, thanks! > Okay, after reading through the patch, it seems we're still marking class > members dllexport, and then you selectively remove the attribute later. That > does feel a little bit backward... Does -fvisibility-inlines-hidden also have > the static local problem, or how does that flag handle it? Ah, maybe I can get performance improvement just support fvisibility-inlines-hidden in clang-cl. Let me try. 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
takuto.ikuta marked 4 inline comments as done.
takuto.ikuta added inline comments.
Comment at: clang/include/clang/Basic/LangOptions.def:117
LANGOPT(Digraphs , 1, 0, "digraphs")
+LANGOPT(DllexportInlines , 1, 1, "If dllexport a class should dllexport
implicit inline methods in the Microsoft ABI")
BENIGN_LANGOPT(HexFloats , 1, C99, "C99 hexadecimal float constants")
hans wrote:
> Same comment about "implicit" here as above.
>
> And is the "In the MS ABI" part of the comment important? Does the flag
> change depending on ABI?
>
>
> And is the "In the MS ABI" part of the comment important? Does the flag
> change depending on ABI?
Currently it works only for Microsoft ABI.
In this patch, I want to focus only on Microsoft ABI.
Comment at: clang/include/clang/Basic/LangOptions.h:217
+ /// If set, dllexported classes dllexport their implicit inline methods.
+ bool DllexportInlines = true;
hans wrote:
> not sure what you mean by implicit here.. this applies to all inline defined
> member functions.
Dropped.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5542
+// or checkClassLevelDLLAttribute?
+if (MD->isInlined() && getDLLAttr(Member)) {
+ const Stmt* Body = nullptr;
hans wrote:
> Hmm, yes I think the intention was to not put the attribute on the members so
> I'm not sure why this is needed?
> Hmm, yes I think the intention was to not put the attribute on the members so
> I'm not sure why this is needed?
I changed the code around here from dropping DLL attribute to warning static
local variable.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5556
+ Body && !Checker.Visit(Body)) {
+MD->dropAttr();
+ }
takuto.ikuta wrote:
> I noticed that this does not work correctly yet.
> Occasionally, some functions are not inlined but not exported, and those
> cannot be linked.
I gave up to support local static variable.
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
takuto.ikuta updated this revision to Diff 164353.
takuto.ikuta edited the summary of this revision.
takuto.ikuta added a comment.
Herald added a subscriber: dschuff.
Make patch closer to Nico's original implementation, but warns local static
variable instead of detecting it.
In checkClassLevelDLLAttribute, inline function definition is not fully parsed
and I cannot make test passed in other way adding export only to inline
function having local static variables.
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-inline.cpp
Index: clang/test/CodeGenCXX/dllexport-no-inline.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-inline.cpp
@@ -0,0 +1,131 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - -verify -DNOEXPORT_INLINE | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+
+// Class member function
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@AEAAXXZ"
+ void InclassDefFunc() {}
+
+ int f();
+
+ // FIX: make this DEFAULT-DAG.
+ // INLINE-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@AEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+#ifdef NOEXPORT_INLINE
+// expected-warning@+2 {{static 'static_variable' used in an inline function of exported class isn't exported due to -fno-dllexport-inlines}}
+#endif
+static int static_variable = f();
+
+static const int static_const_variable = 1; // expected-no-warning
+constexpr int static_constexpr_variable = 2; // expected-no-warning
+static const int static_const_array[] = {1, 2, 3}; // expected-no-warning
+
+return ++static_variable + static_const_variable + static_constexpr_variable +
+static_const_array[0];
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // FIX: Do not export this when -ms-no-dllexport-inline.
+ // DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InlineOutclassDefFunc@NoTemplateExportedClass@@AEAAXXZ"
+ __forceinline void InlineOutclassDefFunc();
+
+ // FIX: Do not export this when -ms-no-dllexport-inline is given and warn for local static var.
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@AEAAHXZ"
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@AEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+return ++static_variable;
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+class A11{};
+class B22{};
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11@@
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?OutclassDefFunc@?$TemplateExportedClass@VA11@@
+template class __declspec(dllexport) TemplateExportedClass;
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VB22@@
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?OutclassDefFunc@?$TemplateExportedClass@VB22@@
+template class TemplateExportedClass;
+
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1225805, @takuto.ikuta wrote: > I found that current ToT with original Nico's patch does not allow to link > ui_base.dll > > https://github.com/atetubou/llvm-project-20170507/tree/totwin_dbg_1236_nico > gives the following link error > https://pastebin.com/9LVRbRVn > > I will do bisection to find when some inline functions are not inlined. Sorry, this is due to my local build config of chromium. 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
takuto.ikuta added a comment. I found that current ToT with original Nico's patch does not allow to link ui_base.dll https://github.com/atetubou/llvm-project-20170507/tree/totwin_dbg_1236_nico gives the following link error https://pastebin.com/9LVRbRVn I will do bisection to find when some inline functions are not inlined. 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] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta added a comment. Thank you! 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] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta marked 5 inline comments as done. takuto.ikuta added a comment. Rui-san, can I ask you to merge this? 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] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta updated this revision to Diff 150685.
takuto.ikuta added a comment.
Follow llvm style
https://reviews.llvm.org/D47578
Files:
llvm/lib/Support/Windows/Process.inc
llvm/unittests/Support/CommandLineTest.cpp
Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -13,6 +13,7 @@
#include "llvm/ADT/Triple.h"
#include "llvm/Config/config.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/InitLLVM.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Program.h"
#include "llvm/Support/StringSaver.h"
@@ -821,4 +822,22 @@
EXPECT_TRUE(Errs.empty());
}
+#ifdef _WIN32
+TEST(CommandLineTest, GetCommandLineArguments) {
+ int argc = __argc;
+ char **argv = __argv;
+
+ // GetCommandLineArguments is called in InitLLVM.
+ llvm::InitLLVM X(argc, argv);
+
+ EXPECT_EQ(llvm::sys::path::is_absolute(argv[0]),
+llvm::sys::path::is_absolute(__argv[0]));
+
+ EXPECT_TRUE(llvm::sys::path::filename(argv[0])
+ .equals_lower("supporttests.exe"))
+ << "Filename of test executable is "
+ << llvm::sys::path::filename(argv[0]);
+}
+#endif
+
} // anonymous namespace
Index: llvm/lib/Support/Windows/Process.inc
===
--- llvm/lib/Support/Windows/Process.inc
+++ llvm/lib/Support/Windows/Process.inc
@@ -209,55 +209,65 @@
return ec;
}
-static std::error_code ExpandShortFileName(const wchar_t *Arg,
- SmallVectorImpl &Args,
- BumpPtrAllocator &Alloc) {
- SmallVector LongPath;
- DWORD Length = GetLongPathNameW(Arg, LongPath.data(), LongPath.capacity());
+static std::error_code GetExecutableName(SmallVectorImpl &Filename) {
+ // The first argument may contain just the name of the executable (e.g.,
+ // "clang") rather than the full path, so swap it with the full path.
+ wchar_t ModuleName[MAX_PATH];
+ size_t Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH);
+ if (Length == 0 || Length == MAX_PATH) {
+return mapWindowsError(GetLastError());
+ }
+
+ // If the first argument is a shortened (8.3) name (which is possible even
+ // if we got the module name), the driver will have trouble distinguishing it
+ // (e.g., clang.exe v. clang++.exe), so expand it now.
+ Length = GetLongPathNameW(ModuleName, ModuleName, MAX_PATH);
if (Length == 0)
return mapWindowsError(GetLastError());
- if (Length > LongPath.capacity()) {
+ if (Length > MAX_PATH) {
// We're not going to try to deal with paths longer than MAX_PATH, so we'll
// treat this as an error. GetLastError() returns ERROR_SUCCESS, which
// isn't useful, so we'll hardcode an appropriate error value.
return mapWindowsError(ERROR_INSUFFICIENT_BUFFER);
}
- LongPath.set_size(Length);
- return ConvertAndPushArg(LongPath.data(), Args, Alloc);
+
+ std::error_code EC = windows::UTF16ToUTF8(ModuleName, Length, Filename);
+ if (EC)
+return EC;
+
+ StringRef Base = sys::path::filename(Filename.data());
+ Filename.assign(Base.begin(), Base.end());
+ return std::error_code();
}
std::error_code
windows::GetCommandLineArguments(SmallVectorImpl &Args,
BumpPtrAllocator &Alloc) {
int ArgCount;
- wchar_t **UnicodeCommandLine =
- CommandLineToArgvW(GetCommandLineW(), &ArgCount);
+ std::unique_ptr UnicodeCommandLine{
+CommandLineToArgvW(GetCommandLineW(), &ArgCount), &LocalFree};
if (!UnicodeCommandLine)
return mapWindowsError(::GetLastError());
- Args.reserve(ArgCount);
- std::error_code ec;
-
- // The first argument may contain just the name of the executable (e.g.,
- // "clang") rather than the full path, so swap it with the full path.
- wchar_t ModuleName[MAX_PATH];
- int Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH);
- if (0 < Length && Length < MAX_PATH)
-UnicodeCommandLine[0] = ModuleName;
+ std::error_code EC;
- // If the first argument is a shortened (8.3) name (which is possible even
- // if we got the module name), the driver will have trouble distinguishing it
- // (e.g., clang.exe v. clang++.exe), so expand it now.
- ec = ExpandShortFileName(UnicodeCommandLine[0], Args, Alloc);
+ Args.reserve(ArgCount);
- for (int i = 1; i < ArgCount && !ec; ++i) {
-ec = WildcardExpand(UnicodeCommandLine[i], Args, Alloc);
-if (ec)
- break;
+ for (int I = 0; I < ArgCount; ++I) {
+EC = WildcardExpand(UnicodeCommandLine[I], Args, Alloc);
+if (EC)
+ return EC;
}
- LocalFree(UnicodeCommandLine);
- return ec;
+ SmallVector Arg0(Args[0], Args[0] + strlen(Args[0]));
+ SmallVector Filename;
+ sys::path::remove_filename(Arg0);
+ EC = GetExecutableName(Filename);
+ if (EC)
+return EC;
+ sys::path::append(Arg0, Filename);
+ Args[0] = Allocat
[PATCH] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta marked 2 inline comments as done. takuto.ikuta added a comment. Can the patch be merged this time? 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] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta updated this revision to Diff 150515.
https://reviews.llvm.org/D47578
Files:
llvm/lib/Support/Windows/Process.inc
llvm/unittests/Support/CommandLineTest.cpp
Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -13,6 +13,7 @@
#include "llvm/ADT/Triple.h"
#include "llvm/Config/config.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/InitLLVM.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Program.h"
#include "llvm/Support/StringSaver.h"
@@ -821,4 +822,22 @@
EXPECT_TRUE(Errs.empty());
}
+#ifdef _WIN32
+TEST(CommandLineTest, GetCommandLineArguments) {
+ int argc = __argc;
+ char **argv = __argv;
+
+ // GetCommandLineArguments is called in InitLLVM.
+ llvm::InitLLVM X(argc, argv);
+
+ EXPECT_EQ(llvm::sys::path::is_absolute(argv[0]),
+llvm::sys::path::is_absolute(__argv[0]));
+
+ EXPECT_TRUE(llvm::sys::path::filename(argv[0])
+ .equals_lower("supporttests.exe"))
+ << "Filename of test executable is "
+ << llvm::sys::path::filename(argv[0]);
+}
+#endif
+
} // anonymous namespace
Index: llvm/lib/Support/Windows/Process.inc
===
--- llvm/lib/Support/Windows/Process.inc
+++ llvm/lib/Support/Windows/Process.inc
@@ -209,55 +209,65 @@
return ec;
}
-static std::error_code ExpandShortFileName(const wchar_t *Arg,
- SmallVectorImpl &Args,
- BumpPtrAllocator &Alloc) {
- SmallVector LongPath;
- DWORD Length = GetLongPathNameW(Arg, LongPath.data(), LongPath.capacity());
+static std::error_code GetExecutableName(SmallVectorImpl &Filename) {
+ // The first argument may contain just the name of the executable (e.g.,
+ // "clang") rather than the full path, so swap it with the full path.
+ wchar_t ModuleName[MAX_PATH];
+ int Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH);
+ if (Length == 0 || Length == MAX_PATH) {
+return mapWindowsError(GetLastError());
+ }
+
+ // If the first argument is a shortened (8.3) name (which is possible even
+ // if we got the module name), the driver will have trouble distinguishing it
+ // (e.g., clang.exe v. clang++.exe), so expand it now.
+ Length = GetLongPathNameW(ModuleName, ModuleName, MAX_PATH);
if (Length == 0)
return mapWindowsError(GetLastError());
- if (Length > LongPath.capacity()) {
+ if (static_cast(Length) > MAX_PATH) {
// We're not going to try to deal with paths longer than MAX_PATH, so we'll
// treat this as an error. GetLastError() returns ERROR_SUCCESS, which
// isn't useful, so we'll hardcode an appropriate error value.
return mapWindowsError(ERROR_INSUFFICIENT_BUFFER);
}
- LongPath.set_size(Length);
- return ConvertAndPushArg(LongPath.data(), Args, Alloc);
+
+ std::error_code ec = windows::UTF16ToUTF8(ModuleName, Length, Filename);
+ if (ec)
+return ec;
+
+ StringRef Base = sys::path::filename(Filename.data());
+ Filename.assign(Base.begin(), Base.end());
+ return std::error_code();
}
std::error_code
windows::GetCommandLineArguments(SmallVectorImpl &Args,
BumpPtrAllocator &Alloc) {
int ArgCount;
- wchar_t **UnicodeCommandLine =
- CommandLineToArgvW(GetCommandLineW(), &ArgCount);
+ std::unique_ptr UnicodeCommandLine{
+CommandLineToArgvW(GetCommandLineW(), &ArgCount), &LocalFree};
if (!UnicodeCommandLine)
return mapWindowsError(::GetLastError());
- Args.reserve(ArgCount);
std::error_code ec;
- // The first argument may contain just the name of the executable (e.g.,
- // "clang") rather than the full path, so swap it with the full path.
- wchar_t ModuleName[MAX_PATH];
- int Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH);
- if (0 < Length && Length < MAX_PATH)
-UnicodeCommandLine[0] = ModuleName;
-
- // If the first argument is a shortened (8.3) name (which is possible even
- // if we got the module name), the driver will have trouble distinguishing it
- // (e.g., clang.exe v. clang++.exe), so expand it now.
- ec = ExpandShortFileName(UnicodeCommandLine[0], Args, Alloc);
+ Args.reserve(ArgCount);
- for (int i = 1; i < ArgCount && !ec; ++i) {
+ for (int i = 0; i < ArgCount; ++i) {
ec = WildcardExpand(UnicodeCommandLine[i], Args, Alloc);
if (ec)
- break;
+ return ec;
}
- LocalFree(UnicodeCommandLine);
- return ec;
+ SmallVector Arg0(Args[0], Args[0] + strlen(Args[0]));
+ SmallVector Filename;
+ sys::path::remove_filename(Arg0);
+ ec = GetExecutableName(Filename);
+ if (ec)
+return ec;
+ sys::path::append(Arg0, Filename);
+ Args[0] = AllocateString(Arg0, Alloc);
+ return std::error_code();
}
std::error_code Process::FixupStandardFileDescriptors() {
___
[PATCH] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta added a comment. I see. Changed not to convert many times. I confirmed that this passed check-lld, check-llvm and check-clang. If this looks good for you, can I ask you to merge this? Thanks. 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] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta updated this revision to Diff 150300.
https://reviews.llvm.org/D47578
Files:
llvm/lib/Support/Windows/Process.inc
llvm/unittests/Support/CommandLineTest.cpp
Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -13,6 +13,7 @@
#include "llvm/ADT/Triple.h"
#include "llvm/Config/config.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/InitLLVM.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Program.h"
#include "llvm/Support/StringSaver.h"
@@ -821,4 +822,22 @@
EXPECT_TRUE(Errs.empty());
}
+#ifdef _WIN32
+TEST(CommandLineTest, GetCommandLineArguments) {
+ int argc = __argc;
+ char **argv = __argv;
+
+ // GetCommandLineArguments is called in InitLLVM.
+ llvm::InitLLVM X(argc, argv);
+
+ EXPECT_EQ(llvm::sys::path::is_absolute(argv[0]),
+llvm::sys::path::is_absolute(__argv[0]));
+
+ EXPECT_TRUE(llvm::sys::path::filename(argv[0])
+ .equals_lower("supporttests.exe"))
+ << "Filename of test executable is "
+ << llvm::sys::path::filename(argv[0]);
+}
+#endif
+
} // anonymous namespace
Index: llvm/lib/Support/Windows/Process.inc
===
--- llvm/lib/Support/Windows/Process.inc
+++ llvm/lib/Support/Windows/Process.inc
@@ -209,54 +209,63 @@
return ec;
}
-static std::error_code ExpandShortFileName(const wchar_t *Arg,
- SmallVectorImpl &Args,
- BumpPtrAllocator &Alloc) {
- SmallVector LongPath;
- DWORD Length = GetLongPathNameW(Arg, LongPath.data(), LongPath.capacity());
+static std::error_code GetExecutableName(SmallVectorImpl &Filename) {
+ // The first argument may contain just the name of the executable (e.g.,
+ // "clang") rather than the full path, so swap it with the full path.
+ wchar_t ModuleName[MAX_PATH];
+ int Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH);
+ if (Length == 0 || Length == MAX_PATH) {
+return mapWindowsError(GetLastError());
+ }
+
+ // If the first argument is a shortened (8.3) name (which is possible even
+ // if we got the module name), the driver will have trouble distinguishing it
+ // (e.g., clang.exe v. clang++.exe), so expand it now.
+ Length = GetLongPathNameW(ModuleName, ModuleName, MAX_PATH);
if (Length == 0)
return mapWindowsError(GetLastError());
- if (Length > LongPath.capacity()) {
+ if (static_cast(Length) > MAX_PATH) {
// We're not going to try to deal with paths longer than MAX_PATH, so we'll
// treat this as an error. GetLastError() returns ERROR_SUCCESS, which
// isn't useful, so we'll hardcode an appropriate error value.
return mapWindowsError(ERROR_INSUFFICIENT_BUFFER);
}
- LongPath.set_size(Length);
- return ConvertAndPushArg(LongPath.data(), Args, Alloc);
+
+ std::error_code ec = windows::UTF16ToUTF8(ModuleName, Length, Filename);
+ if (ec)
+return ec;
+
+ StringRef Base = sys::path::filename(Filename.data());
+ Filename.assign(Base.begin(), Base.end());
+ return ec;
}
std::error_code
windows::GetCommandLineArguments(SmallVectorImpl &Args,
BumpPtrAllocator &Alloc) {
int ArgCount;
- wchar_t **UnicodeCommandLine =
- CommandLineToArgvW(GetCommandLineW(), &ArgCount);
+ std::unique_ptr UnicodeCommandLine{
+CommandLineToArgvW(GetCommandLineW(), &ArgCount), &LocalFree};
if (!UnicodeCommandLine)
return mapWindowsError(::GetLastError());
- Args.reserve(ArgCount);
std::error_code ec;
- // The first argument may contain just the name of the executable (e.g.,
- // "clang") rather than the full path, so swap it with the full path.
- wchar_t ModuleName[MAX_PATH];
- int Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH);
- if (0 < Length && Length < MAX_PATH)
-UnicodeCommandLine[0] = ModuleName;
-
- // If the first argument is a shortened (8.3) name (which is possible even
- // if we got the module name), the driver will have trouble distinguishing it
- // (e.g., clang.exe v. clang++.exe), so expand it now.
- ec = ExpandShortFileName(UnicodeCommandLine[0], Args, Alloc);
+ Args.reserve(ArgCount);
- for (int i = 1; i < ArgCount && !ec; ++i) {
+ for (int i = 0; i < ArgCount; ++i) {
ec = WildcardExpand(UnicodeCommandLine[i], Args, Alloc);
if (ec)
- break;
+ return ec;
}
- LocalFree(UnicodeCommandLine);
+ SmallVector Arg0(Args[0], Args[0] + strlen(Args[0])), Filename;
+ sys::path::remove_filename(Arg0);
+ ec = GetExecutableName(Filename);
+ if (ec)
+return ec;
+ sys::path::append(Arg0, Filename);
+ Args[0] = AllocateString(Arg0, Alloc);
return ec;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/lis
[PATCH] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta marked 3 inline comments as done. takuto.ikuta added a comment. Thank you for review Comment at: llvm/lib/Support/Windows/Process.inc:251 + // This may change argv0 like below, + // * clang -> clang.exe (just add extension) + // * CLANG_~1.EXE -> clang++.exe (extend shorname) ruiu wrote: > I believe GetModuleFileName always returns a path with an extension, though > it might be 8.3 path. I meant to say about filename in original argv0 here. 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] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta updated this revision to Diff 149694.
takuto.ikuta added a comment.
Address comments
https://reviews.llvm.org/D47578
Files:
llvm/lib/Support/Windows/Process.inc
llvm/unittests/Support/CommandLineTest.cpp
Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -13,6 +13,7 @@
#include "llvm/ADT/Triple.h"
#include "llvm/Config/config.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/InitLLVM.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Program.h"
#include "llvm/Support/StringSaver.h"
@@ -821,4 +822,22 @@
EXPECT_TRUE(Errs.empty());
}
+#ifdef _WIN32
+TEST(CommandLineTest, GetCommandLineArguments) {
+ int argc = __argc;
+ char **argv = __argv;
+
+ // GetCommandLineArguments is called in InitLLVM.
+ llvm::InitLLVM X(argc, argv);
+
+ EXPECT_EQ(llvm::sys::path::is_absolute(argv[0]),
+llvm::sys::path::is_absolute(__argv[0]));
+
+ EXPECT_TRUE(llvm::sys::path::filename(argv[0])
+ .equals_lower("supporttests.exe"))
+ << "Filename of test executable is "
+ << llvm::sys::path::filename(argv[0]);
+}
+#endif
+
} // anonymous namespace
Index: llvm/lib/Support/Windows/Process.inc
===
--- llvm/lib/Support/Windows/Process.inc
+++ llvm/lib/Support/Windows/Process.inc
@@ -209,21 +209,63 @@
return ec;
}
-static std::error_code ExpandShortFileName(const wchar_t *Arg,
- SmallVectorImpl &Args,
- BumpPtrAllocator &Alloc) {
- SmallVector LongPath;
- DWORD Length = GetLongPathNameW(Arg, LongPath.data(), LongPath.capacity());
+static std::error_code GetLongArgv0FullPath(SmallVectorImpl &LongArgv0) {
+ // The first argument may contain just the name of the executable (e.g.,
+ // "clang") rather than the full path, so swap it with the full path.
+ wchar_t ModuleName[MAX_PATH];
+ int Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH);
+ if (Length == 0 || Length == MAX_PATH) {
+return mapWindowsError(GetLastError());
+ }
+
+ // If the first argument is a shortened (8.3) name (which is possible even
+ // if we got the module name), the driver will have trouble distinguishing it
+ // (e.g., clang.exe v. clang++.exe), so expand it now.
+ Length = GetLongPathNameW(ModuleName, LongArgv0.data(), LongArgv0.capacity());
if (Length == 0)
return mapWindowsError(GetLastError());
- if (Length > LongPath.capacity()) {
+ if (static_cast(Length) > LongArgv0.capacity()) {
// We're not going to try to deal with paths longer than MAX_PATH, so we'll
// treat this as an error. GetLastError() returns ERROR_SUCCESS, which
// isn't useful, so we'll hardcode an appropriate error value.
return mapWindowsError(ERROR_INSUFFICIENT_BUFFER);
}
- LongPath.set_size(Length);
- return ConvertAndPushArg(LongPath.data(), Args, Alloc);
+ LongArgv0.set_size(Length);
+ return std::error_code();
+}
+
+static std::error_code GetLongArgv0Path(const wchar_t *Argv0,
+SmallVectorImpl &LongArgv0) {
+ // Replace filename in original argv0 with expanded filename.
+ // This may change original filename argv0 like below,
+ // * clang -> clang.exe (just add extension)
+ // * CLANG_~1.EXE -> clang++.exe (extend shorname)
+ // This is for keeping relativeness of original argv0 path.
+
+ SmallVector UTF16LongArgv0Full;
+
+ std::error_code ec = GetLongArgv0FullPath(UTF16LongArgv0Full);
+ if (ec)
+return ec;
+
+ SmallVector UTF8LongArgv0Full;
+ ec = windows::UTF16ToUTF8(UTF16LongArgv0Full.data(), UTF16LongArgv0Full.size(),
+UTF8LongArgv0Full);
+ if (ec)
+return ec;
+
+ ec = windows::UTF16ToUTF8(Argv0, wcslen(Argv0), LongArgv0);
+ if (ec)
+return ec;
+
+ sys::path::remove_filename(LongArgv0);
+ sys::path::append(LongArgv0, sys::path::filename(UTF8LongArgv0Full.data()));
+
+ // null terminates.
+ LongArgv0.push_back(0);
+ LongArgv0.pop_back();
+
+ return std::error_code();
}
std::error_code
@@ -236,19 +278,11 @@
return mapWindowsError(::GetLastError());
Args.reserve(ArgCount);
- std::error_code ec;
+ SmallVector LongArgv0;
+ std::error_code ec = GetLongArgv0Path(UnicodeCommandLine[0], LongArgv0);
- // The first argument may contain just the name of the executable (e.g.,
- // "clang") rather than the full path, so swap it with the full path.
- wchar_t ModuleName[MAX_PATH];
- int Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH);
- if (0 < Length && Length < MAX_PATH)
-UnicodeCommandLine[0] = ModuleName;
-
- // If the first argument is a shortened (8.3) name (which is possible even
- // if we got the module name), the driver will have trouble distinguishing it
- // (e.g., clang.exe v. clang++.ex
[PATCH] D47480: clang-cl: Expose -no-canonical-prefixes
takuto.ikuta accepted this revision. takuto.ikuta added a comment. I confirmed this CL and https://reviews.llvm.org/D47578 remove absolute path from /showIncludes when include paths are given in relative. https://reviews.llvm.org/D47480 ___ 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
takuto.ikuta marked an inline comment as done. takuto.ikuta added a comment. In https://reviews.llvm.org/D47578#1117874, @rnk wrote: > I think this would be easy to unit test in > llvm/unittests/Support/CommandLine.cpp. We'd just check that the filename is > "SupportTests.exe" on Windows and the path is relative after calling this, I > guess. Right? Look at how ProgramTest.cpp does this to get a reasonable argv0: > > sys::fs::getMainExecutable(TestMainArgv0, &ProgramTestStringArg1); > Added test. > > > In https://reviews.llvm.org/D47578#1117790, @takuto.ikuta wrote: > >> In https://reviews.llvm.org/D47578#1117760, @amccarth wrote: >> >> > I was under the impression that some tools rely on the fact that arg[0] is >> > always expanded to an absolute path. Does this work with lldb and its >> > test suite? >> >> >> I tried, but there is no check-lldb target. Can I ask you what is the target >> name to run lldb test suite? > > > The LLDB test suite isn't in very good shape on Windows. It is complicated to > set up and build, I don't want to block this fix on @takuto.ikuta setting up > that build environment. This is a Windows-only change, and I believe it makes > it more consistent with Linux, so as long as check-llvm, check-clang, and > check-lld pass, this should be relatively safe. I confirmed that this patch passed check-llvm, check-clang and check-lld. I tried to test lldb, but failed to build. lldb looks cannot be built with MSVC 2017 15.7.2 FAILED: tools/lldb/tools/lldb-mi/CMakeFiles/lldb-mi.dir/MIDriverMain.cpp.obj c:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin\clang-cl.exe /nologo -DGTEST_HAS_RTTI=0 -DIMPORT_LIBLLDB -DLLDB_CONFIGURATION_RELEASE -DLLDB_DISABLE_CURSES -DLLDB_DISABLE_LIBEDIT -DLLDB_DISABLE_PYTHON -DLLDB_PYTHON_HOME=\"\" -DLLDB_USE_BUILTIN_DEMANGLER -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_DEBUG_POINTER_IMPL="" -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\lldb\tools\lldb-mi -IC:\src\git\llvm-project-20170507\lldb\tools\lldb-mi -Itools\lldb\include -IC:\src\git\llvm-project-20170507\lldb\include -Iinclude -IC:\src\git\llvm-project-20170507\llvm\include -IC:\src\git\llvm-project-20170507\llvm\..\clang\include -Itools\lldb\..\clang\include /DWIN32 /D_WINDOWS /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /Brepro /W4 -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wcovered-switch-default -Wdelete-non-virtual-dtor -Wstring-conversion -Wno-deprecated-declarations -Wno-unknown-pragmas -Wno-strict-aliasing -Wno-deprecated-register -Wno-vla-extension /MD /O2 /Ob2 /DNDEBUG -wd4018 -wd4068 -wd4150 -wd4251 -wd4521 -wd4530 /EHs-c- /GR- /showIncludes /Fotools\lldb\tools\lldb-mi\CMakeFiles\lldb-mi.dir\MIDriverMain.cpp.obj /Fdtools\lldb\tools\lldb-mi\CMakeFiles\lldb-mi.dir\ -c C:\src\git\llvm-project-20170507\lldb\tools\lldb-mi\MIDriverMain.cpp In file included from C:\src\git\llvm-project-20170507\lldb\tools\lldb-mi\MIDriverMain.cpp:37: C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Tools\MSVC\14.14.26428\include\csignal(19,13): error: no member named 'sig_atomic_t' in the global namespace using _CSTD sig_atomic_t; using _CSTD raise; using _CSTD signal; ~ ^ C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Tools\MSVC\14.14.26428\include\csignal(19,39): error: no member named 'raise' in the global namespace using _CSTD sig_atomic_t; using _CSTD raise; using _CSTD signal; ~ ^ C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\VC\Tools\MSVC\14.14.26428\include\csignal(19,58): error: no member named 'signal' in the global namespace using _CSTD sig_atomic_t; using _CSTD raise; using _CSTD signal; ~ ^ C:\src\git\llvm-project-20170507\lldb\tools\lldb-mi\MIDriverMain.cpp(74,10): error: use of undeclared identifier 'SIGINT' signal(SIGINT, sigint_handler); ^ C:\src\git\llvm-project-20170507\lldb\tools\lldb-mi\MIDriverMain.cpp(185,10): error: use of undeclared identifier 'SIGINT' signal(SIGINT, sigint_handler); ^ 5 errors generated. 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] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta updated this revision to Diff 149394.
takuto.ikuta added a comment.
Add test and split function
https://reviews.llvm.org/D47578
Files:
llvm/lib/Support/Windows/Process.inc
llvm/unittests/Support/CommandLineTest.cpp
Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -13,6 +13,7 @@
#include "llvm/ADT/Triple.h"
#include "llvm/Config/config.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/InitLLVM.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Program.h"
#include "llvm/Support/StringSaver.h"
@@ -821,4 +822,22 @@
EXPECT_TRUE(Errs.empty());
}
+#ifdef _WIN32
+TEST(CommandLineTest, GetCommandLineArguments) {
+ int argc = __argc;
+ char **argv = __argv;
+
+ // GetCommandLineArguments is called in InitLLVM.
+ llvm::InitLLVM X(argc, argv);
+
+ EXPECT_EQ(llvm::sys::path::is_absolute(argv[0]),
+llvm::sys::path::is_absolute(__argv[0]));
+
+ EXPECT_TRUE(llvm::sys::path::filename(argv[0])
+ .equals_lower("supporttests.exe"))
+ << "Filename of test executable is "
+ << llvm::sys::path::filename(argv[0]);
+}
+#endif
+
} // anonymous namespace
Index: llvm/lib/Support/Windows/Process.inc
===
--- llvm/lib/Support/Windows/Process.inc
+++ llvm/lib/Support/Windows/Process.inc
@@ -209,9 +209,7 @@
}
static std::error_code ExpandShortFileName(const wchar_t *Arg,
- SmallVectorImpl &Args,
- BumpPtrAllocator &Alloc) {
- SmallVector LongPath;
+ SmallVectorImpl &LongPath) {
DWORD Length = GetLongPathNameW(Arg, LongPath.data(), LongPath.capacity());
if (Length == 0)
return mapWindowsError(GetLastError());
@@ -222,7 +220,53 @@
return mapWindowsError(ERROR_INSUFFICIENT_BUFFER);
}
LongPath.set_size(Length);
- return ConvertAndPushArg(LongPath.data(), Args, Alloc);
+ return std::error_code();
+}
+
+static std::error_code GetLongArgv0FullPath(const wchar_t *Argv0,
+SmallVectorImpl &LongArgv0) {
+ // The first argument may contain just the name of the executable (e.g.,
+ // "clang") rather than the full path, so swap it with the full path.
+ wchar_t ModuleName[MAX_PATH];
+ int Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH);
+ if (0 < Length && Length < MAX_PATH)
+Argv0 = ModuleName;
+
+ // If the first argument is a shortened (8.3) name (which is possible even
+ // if we got the module name), the driver will have trouble distinguishing it
+ // (e.g., clang.exe v. clang++.exe), so expand it now.
+ return ExpandShortFileName(Argv0, LongArgv0);
+}
+
+static std::error_code GetLongArgv0Path(const wchar_t *Argv0,
+SmallVectorImpl &LongArgv0) {
+ SmallVector UTF16LongArgv0Full;
+
+ std::error_code ec = GetLongArgv0FullPath(Argv0, UTF16LongArgv0Full);
+ if (ec)
+return ec;
+
+ // Replace filename in original argv0 with expanded filename.
+ // This may change argv0 like below,
+ // * clang -> clang.exe (just add extension)
+ // * CLANG_~1.EXE -> clang++.exe (extend shorname)
+ // This is for keeping relativeness of original argv0 path.
+ SmallVector UTF8LongArgv0Full;
+ ec = windows::UTF16ToUTF8(UTF16LongArgv0Full.data(), UTF16LongArgv0Full.size(),
+UTF8LongArgv0Full);
+ if (ec)
+return ec;
+
+ SmallVector UTF8Argv0;
+ ec = windows::UTF16ToUTF8(Argv0, wcslen(Argv0), UTF8Argv0);
+ if (ec)
+return ec;
+
+ sys::path::remove_filename(UTF8Argv0);
+ sys::path::append(UTF8Argv0, sys::path::filename(UTF8LongArgv0Full.data()));
+
+ return windows::UTF8ToUTF16(StringRef(UTF8Argv0.data(), UTF8Argv0.size()),
+ LongArgv0);
}
std::error_code
@@ -235,19 +279,11 @@
return mapWindowsError(::GetLastError());
Args.reserve(ArgCount);
- std::error_code ec;
-
- // The first argument may contain just the name of the executable (e.g.,
- // "clang") rather than the full path, so swap it with the full path.
- wchar_t ModuleName[MAX_PATH];
- int Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH);
- if (0 < Length && Length < MAX_PATH)
-UnicodeCommandLine[0] = ModuleName;
+ SmallVector LongArgv0;
+ std::error_code ec = GetLongArgv0Path(UnicodeCommandLine[0], LongArgv0);
- // If the first argument is a shortened (8.3) name (which is possible even
- // if we got the module name), the driver will have trouble distinguishing it
- // (e.g., clang.exe v. clang++.exe), so expand it now.
- ec = ExpandShortFileName(UnicodeCommandLine[0], Args, Alloc);
+ if (!ec)
+ec = ConvertAndPushArg(LongArgv0.data(), Args, Alloc);
for (int i = 1; i < ArgCount && !ec; ++i) {
ec = Wild
[PATCH] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta added a comment. In https://reviews.llvm.org/D47578#1117540, @ruiu wrote: > Looks like this patch contains two unrelated changes. Please separate your > change from the lit config changes. First patch made on some wrong assumption, fixed and reverted test config change. In https://reviews.llvm.org/D47578#1117760, @amccarth wrote: > I was under the impression that some tools rely on the fact that arg[0] is > always expanded to an absolute path. Does this work with lldb and its test > suite? I tried, but there is no check-lldb target. Can I ask you what is the target name to run lldb test suite? 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] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta updated this revision to Diff 149304.
takuto.ikuta edited the summary of this revision.
https://reviews.llvm.org/D47578
Files:
llvm/lib/Support/Windows/Process.inc
Index: llvm/lib/Support/Windows/Process.inc
===
--- llvm/lib/Support/Windows/Process.inc
+++ llvm/lib/Support/Windows/Process.inc
@@ -209,9 +209,7 @@
}
static std::error_code ExpandShortFileName(const wchar_t *Arg,
- SmallVectorImpl &Args,
- BumpPtrAllocator &Alloc) {
- SmallVector LongPath;
+ SmallVectorImpl &LongPath)
{
DWORD Length = GetLongPathNameW(Arg, LongPath.data(), LongPath.capacity());
if (Length == 0)
return mapWindowsError(GetLastError());
@@ -222,7 +220,51 @@
return mapWindowsError(ERROR_INSUFFICIENT_BUFFER);
}
LongPath.set_size(Length);
- return ConvertAndPushArg(LongPath.data(), Args, Alloc);
+ return std::error_code();
+}
+
+static std::error_code GetLongArgv0Path(const wchar_t *Argv0,
+SmallVectorImpl &Args,
+BumpPtrAllocator &Alloc) {
+ // The first argument may contain just the name of the executable (e.g.,
+ // "clang") rather than the full path, so swap it with the full path.
+ wchar_t ModuleName[MAX_PATH];
+ int Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH);
+ SmallVector UTF16LongpathArgv0, UTF16Argv0;
+
+ if (0 < Length && Length < MAX_PATH)
+Argv0 = ModuleName;
+
+ // If the first argument is a shortened (8.3) name (which is possible even
+ // if we got the module name), the driver will have trouble distinguishing it
+ // (e.g., clang.exe v. clang++.exe), so expand it now.
+ std::error_code ec = ExpandShortFileName(Argv0, UTF16LongpathArgv0);
+ if (ec)
+return ec;
+
+ // Replace filename in original argv0 with expanded filename.
+ // This may change argv0 like below,
+ // * clang -> clang.exe (just add extension)
+ // * CLANG_~1.EXE -> clang++.exe (extend shorname)
+ // This is for keeping relativeness of original argv0 path.
+ SmallVector UTF8LongPathArgv0;
+ ec = windows::UTF16ToUTF8(UTF16LongpathArgv0.data(),
UTF16LongpathArgv0.size(),
+UTF8LongPathArgv0);
+ if (ec)
+return ec;
+
+ SmallVector UTF8Argv0;
+ ec = windows::UTF16ToUTF8(Argv0, wcslen(Argv0), UTF8Argv0);
+ if (ec)
+return ec;
+
+ sys::path::remove_filename(UTF8Argv0);
+ sys::path::append(UTF8Argv0, sys::path::filename(UTF8LongPathArgv0.data()));
+ ec = windows::UTF8ToUTF16(UTF8Argv0.data(), UTF16Argv0);
+ if (ec)
+return ec;
+
+ return ConvertAndPushArg(UTF16Argv0.data(), Args, Alloc);
}
std::error_code
@@ -235,19 +277,7 @@
return mapWindowsError(::GetLastError());
Args.reserve(ArgCount);
- std::error_code ec;
-
- // The first argument may contain just the name of the executable (e.g.,
- // "clang") rather than the full path, so swap it with the full path.
- wchar_t ModuleName[MAX_PATH];
- int Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH);
- if (0 < Length && Length < MAX_PATH)
-UnicodeCommandLine[0] = ModuleName;
-
- // If the first argument is a shortened (8.3) name (which is possible even
- // if we got the module name), the driver will have trouble distinguishing it
- // (e.g., clang.exe v. clang++.exe), so expand it now.
- ec = ExpandShortFileName(UnicodeCommandLine[0], Args, Alloc);
+ std::error_code ec = GetLongArgv0Path(UnicodeCommandLine[0], Args, Alloc);
for (int i = 1; i < ArgCount && !ec; ++i) {
ec = WildcardExpand(UnicodeCommandLine[i], Args, Alloc);
Index: llvm/lib/Support/Windows/Process.inc
===
--- llvm/lib/Support/Windows/Process.inc
+++ llvm/lib/Support/Windows/Process.inc
@@ -209,9 +209,7 @@
}
static std::error_code ExpandShortFileName(const wchar_t *Arg,
- SmallVectorImpl &Args,
- BumpPtrAllocator &Alloc) {
- SmallVector LongPath;
+ SmallVectorImpl &LongPath) {
DWORD Length = GetLongPathNameW(Arg, LongPath.data(), LongPath.capacity());
if (Length == 0)
return mapWindowsError(GetLastError());
@@ -222,7 +220,51 @@
return mapWindowsError(ERROR_INSUFFICIENT_BUFFER);
}
LongPath.set_size(Length);
- return ConvertAndPushArg(LongPath.data(), Args, Alloc);
+ return std::error_code();
+}
+
+static std::error_code GetLongArgv0Path(const wchar_t *Argv0,
+SmallVectorImpl &Args,
+BumpPtrAllocator &Alloc) {
+ // The first argument may contain just the name of the executable (e.g.,
+ // "clang") rather than the full path, so swap it with the full path.
+ wchar_t ModuleName[MAX_PATH];
+ int Length = ::Get
[PATCH] D47578: Do not enforce absolute path argv0 in windows
takuto.ikuta created this revision.
takuto.ikuta added reviewers: thakis, rnk.
Herald added a subscriber: hiraditya.
Herald added a reviewer: alexshap.
Even if we support no-canonical-prefix on
clang-cl(https://reviews.llvm.org/D47480), argv0 becomes absolute path in
clang-cl and that embeds absolute path in /showIncludes.
This patch removes such full path normalization from InitLLVM on windows.
Also this patch revealed that some tests may use binary out of build dir.
https://reviews.llvm.org/D47578
Files:
clang/test/lit.cfg.py
lld/test/lit.cfg.py
llvm/lib/Support/Windows/Process.inc
llvm/test/lit.cfg.py
Index: llvm/test/lit.cfg.py
===
--- llvm/test/lit.cfg.py
+++ llvm/test/lit.cfg.py
@@ -142,10 +142,10 @@
tools.extend([
'dsymutil', 'lli', 'lli-child-target', 'llvm-ar', 'llvm-as',
'llvm-bcanalyzer',
'llvm-config', 'llvm-cov', 'llvm-cxxdump', 'llvm-cvtres', 'llvm-diff',
'llvm-dis',
-'llvm-dwarfdump', 'llvm-extract', 'llvm-isel-fuzzer', 'llvm-opt-fuzzer',
'llvm-lib',
-'llvm-link', 'llvm-lto', 'llvm-lto2', 'llvm-mc', 'llvm-mca',
-'llvm-modextract', 'llvm-nm', 'llvm-objcopy', 'llvm-objdump',
-'llvm-pdbutil', 'llvm-profdata', 'llvm-ranlib', 'llvm-readobj',
+'llvm-dlltool', 'llvm-dwarfdump', 'llvm-dwp', 'llvm-extract',
'llvm-isel-fuzzer',
+'llvm-opt-fuzzer', 'llvm-lib', 'llvm-link', 'llvm-lto', 'llvm-lto2',
'llvm-mc', 'llvm-mca',
+'llvm-modextract', 'llvm-nm', 'llvm-objcopy', 'llvm-objdump',
'llvm-opt-report',
+'llvm-pdbutil', 'llvm-profdata', 'llvm-ranlib', 'llvm-rc', 'llvm-readelf',
'llvm-readobj',
'llvm-rtdyld', 'llvm-size', 'llvm-split', 'llvm-strings', 'llvm-strip',
'llvm-tblgen',
'llvm-c-test', 'llvm-cxxfilt', 'llvm-xray', 'yaml2obj', 'obj2yaml',
'yaml-bench', 'verify-uselistorder',
Index: llvm/lib/Support/Windows/Process.inc
===
--- llvm/lib/Support/Windows/Process.inc
+++ llvm/lib/Support/Windows/Process.inc
@@ -235,19 +235,11 @@
return mapWindowsError(::GetLastError());
Args.reserve(ArgCount);
- std::error_code ec;
-
- // The first argument may contain just the name of the executable (e.g.,
- // "clang") rather than the full path, so swap it with the full path.
- wchar_t ModuleName[MAX_PATH];
- int Length = ::GetModuleFileNameW(NULL, ModuleName, MAX_PATH);
- if (0 < Length && Length < MAX_PATH)
-UnicodeCommandLine[0] = ModuleName;
// If the first argument is a shortened (8.3) name (which is possible even
// if we got the module name), the driver will have trouble distinguishing it
// (e.g., clang.exe v. clang++.exe), so expand it now.
- ec = ExpandShortFileName(UnicodeCommandLine[0], Args, Alloc);
+ std::error_code ec = ExpandShortFileName(UnicodeCommandLine[0], Args, Alloc);
for (int i = 1; i < ArgCount && !ec; ++i) {
ec = WildcardExpand(UnicodeCommandLine[i], Args, Alloc);
Index: lld/test/lit.cfg.py
===
--- lld/test/lit.cfg.py
+++ lld/test/lit.cfg.py
@@ -39,9 +39,12 @@
llvm_config.use_lld()
tool_patterns = [
-'llc', 'llvm-as', 'llvm-mc', 'llvm-nm', 'llvm-objdump', 'llvm-pdbutil',
-'llvm-dwarfdump', 'llvm-readelf', 'llvm-readobj', 'obj2yaml', 'yaml2obj',
-'opt', 'llvm-dis']
+'llc', 'llvm-ar', 'llvm-as', 'llvm-bcanalyzer', 'llvm-lib', 'llvm-mc',
'llvm-nm',
+'llvm-objdump', 'llvm-pdbutil', 'llvm-dwarfdump', 'llvm-readelf',
'llvm-readobj',
+'obj2yaml', 'yaml2obj', 'opt', 'llvm-dis']
+
+if platform.system() == 'Windows':
+tool_patterns += ['LLD-LINK']
llvm_config.add_tool_substitutions(tool_patterns)
Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -58,7 +58,8 @@
tools = [
'c-index-test', 'clang-check', 'clang-diff', 'clang-format',
'clang-tblgen',
-'opt',
+'llc', 'llvm-bcanalyzer', 'llvm-dis', 'llvm-lto', 'llvm-nm',
'llvm-objdump',
+'llvm-profdata', 'llvm-readobj', 'opt',
ToolSubst('%clang_func_map', command=FindTool(
'clang-func-mapping'), unresolved='ignore'),
]
Index: llvm/test/lit.cfg.py
===
--- llvm/test/lit.cfg.py
+++ llvm/test/lit.cfg.py
@@ -142,10 +142,10 @@
tools.extend([
'dsymutil', 'lli', 'lli-child-target', 'llvm-ar', 'llvm-as', 'llvm-bcanalyzer',
'llvm-config', 'llvm-cov', 'llvm-cxxdump', 'llvm-cvtres', 'llvm-diff', 'llvm-dis',
-'llvm-dwarfdump', 'llvm-extract', 'llvm-isel-fuzzer', 'llvm-opt-fuzzer', 'llvm-lib',
-'llvm-link', 'llvm-lto', 'llvm-lto2', 'llvm-mc', 'llvm-mca',
-'llvm-modextract', 'llvm-nm', 'llvm-objcopy', 'llvm-objdump',
-'llvm-pdbutil', 'llvm-profdata', 'llvm-ranlib', 'llvm-readobj',
+'llvm-dlltool', 'llvm-dwarfdump', 'llvm-dwp', 'llvm-extract', 'llvm-isel-fuzzer',
+'llvm-opt-fuzzer', 'llvm
[PATCH] D47480: clang-cl: Expose -no-canonical-prefixes
takuto.ikuta added a comment. GetExecutablePath is called here. http://llvm-cs.pcc.me.uk/tools/clang/tools/driver/driver.cpp#427 You'll understand what I said if you see the behavior of clang-cl on windows. Comment at: test/Driver/cl-options.c:595 +// RUN: -no-canonical-prefixes \ +// RUN: -fno-coverage-mapping \ // RUN: --version \ Is this related to this change? https://reviews.llvm.org/D47480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47480: clang-cl: Expose -no-canonical-prefixes
takuto.ikuta added a comment. On windows, argv0 is changed to absolute path before we call GetExecutablePath. That makes resource-dir absolute. I need clang-cl on windows has relative --resource-dir. See around heres. https://github.com/llvm-project/llvm-project-20170507/blob/83b39c10b1d7a9189b7ee646212eccf6e61dcfbf/clang/tools/driver/driver.cpp#L323 https://github.com/llvm-project/llvm-project-20170507/blob/ca50c5d758541decdb59330fb15fe6b73ba6467d/llvm/lib/Support/InitLLVM.cpp#L28 https://reviews.llvm.org/D47480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47480: clang-cl: Expose -no-canonical-prefixes
takuto.ikuta added a comment. Thank you for quick supporting, but this is not sufficient for clang-cl on windows. llvm::InitLLVM calls windows::GetCommandLineArguments, and argv0 is changed to absolute path. https://github.com/llvm-project/llvm-project-20170507/blob/7c81daae49eaf7f9681457b46c569c1bc6b80283/llvm/lib/Support/Windows/Process.inc#L229 Then clang's builtin include files are shown in absolute path even we specify -no-canonical-prefixes. Can we keep using original argv0 when argv0 is not 8.3 filename? Or is there better solution? https://reviews.llvm.org/D47480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32747: [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments
takuto.ikuta added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:271 + InvalidatedBugType.reset( + new BugType(this, "Iterator invalidated", "Misuse of STL APIs")); + InvalidatedBugType->setSuppressOnSink(true); OK to use make_unique here? https://reviews.llvm.org/D32747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32642: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks
takuto.ikuta added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:357
+return;
+ } else if (isEndCall(Func)) {
handleEnd(C, OrigExpr, Call.getReturnValue(),
We cannot use else after return?
http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:576
+auto &SymMgr = C.getSymbolManager();
+const auto oldOffset = Pos->getOffset();
+auto newOffset =
oldOffset -> OldOffset?
same with L577, L580, L595, L596, L599 and so on.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:941
+ return State;
+} else {
+ const auto CData = CDataPtr->newBegin(Sym);
else after return?
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:945
+}
+ } else {
+const auto CData = ContainerData::fromBegin(Sym);
else after return too.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1195
+Expr->getType());
+ } else {
+return LExpr->getLHS();
else after return?
https://reviews.llvm.org/D32642
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32906: [Analyzer] Iterator Checker - Part 10: Support for iterators passed as parameter
takuto.ikuta added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:711
+void IteratorChecker::checkBeginFunction(CheckerContext &C) const {
+ // Copy state of iterator arguments to iterator parameters
takuto.ikuta wrote:
> Can we use `const CheckerContext &C` here?
I didn't see C.addTransition, sorry.
https://reviews.llvm.org/D32906
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32906: [Analyzer] Iterator Checker - Part 10: Support for iterators passed as parameter
takuto.ikuta added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:711
+void IteratorChecker::checkBeginFunction(CheckerContext &C) const {
+ // Copy state of iterator arguments to iterator parameters
Can we use `const CheckerContext &C` here?
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:730
+ int idx = 0;
+ for (const auto P : FD->parameters()) {
+auto Param = State->getLValue(P, LCtx);
`const auto *P`?
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:731
+ for (const auto P : FD->parameters()) {
+auto Param = State->getLValue(P, LCtx);
+auto Arg = State->getSVal(CE->getArg(idx++), LCtx->getParent());
Can we declare this after L735?
https://reviews.llvm.org/D32906
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32903: Remove unused variable and argument from Lex/HeaderSearch.cpp
takuto.ikuta updated this revision to Diff 97949.
takuto.ikuta added a comment.
Remove IncludeLoc
https://reviews.llvm.org/D32903
Files:
clang/include/clang/Lex/DirectoryLookup.h
clang/include/clang/Lex/HeaderSearch.h
clang/lib/Lex/HeaderSearch.cpp
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -234,9 +234,9 @@
break;
}
}
-
+
// FIXME: Figure out how header maps and module maps will work together.
-
+
// Only deal with normal search directories.
if (!SearchDirs[Idx].isNormalDir())
continue;
@@ -251,7 +251,7 @@
if (Module)
break;
}
-
+
// Search for a module map in a subdirectory with the same name as the
// module.
SmallString<128> NestedModuleMapDirName;
@@ -299,7 +299,7 @@
}
const FileEntry *HeaderSearch::getFileAndSuggestModule(
-StringRef FileName, SourceLocation IncludeLoc, const DirectoryEntry *Dir,
+StringRef FileName, const DirectoryEntry *Dir,
bool IsSystemHeaderDir, Module *RequestingModule,
ModuleMap::KnownHeader *SuggestedModule) {
// If we have a module map that might map this header, load it and
@@ -322,7 +322,6 @@
const FileEntry *DirectoryLookup::LookupFile(
StringRef &Filename,
HeaderSearch &HS,
-SourceLocation IncludeLoc,
SmallVectorImpl *SearchPath,
SmallVectorImpl *RelativePath,
Module *RequestingModule,
@@ -348,7 +347,7 @@
RelativePath->append(Filename.begin(), Filename.end());
}
-return HS.getFileAndSuggestModule(TmpDir, IncludeLoc, getDir(),
+return HS.getFileAndSuggestModule(TmpDir, getDir(),
isSystemHeaderDirectory(),
RequestingModule, SuggestedModule);
}
@@ -399,11 +398,8 @@
///
/// \param FileMgr The file manager to use for directory lookups.
/// \param DirName The name of the framework directory.
-/// \param SubmodulePath Will be populated with the submodule path from the
-/// returned top-level module to the originally named framework.
static const DirectoryEntry *
-getTopFrameworkDir(FileManager &FileMgr, StringRef DirName,
- SmallVectorImpl &SubmodulePath) {
+getTopFrameworkDir(FileManager &FileMgr, StringRef DirName) {
assert(llvm::sys::path::extension(DirName) == ".framework" &&
"Not a framework directory");
@@ -437,7 +433,6 @@
// If this is a framework directory, then we're a subframework of this
// framework.
if (llvm::sys::path::extension(DirName) == ".framework") {
- SubmodulePath.push_back(llvm::sys::path::stem(DirName));
TopFrameworkDir = Dir;
}
} while (true);
@@ -518,7 +513,7 @@
RelativePath->clear();
RelativePath->append(Filename.begin()+SlashPos+1, Filename.end());
}
-
+
// Check "/System/Library/Frameworks/Cocoa.framework/Headers/file.h"
unsigned OrigSize = FrameworkName.size();
@@ -630,7 +625,7 @@
if (SuggestedModule)
*SuggestedModule = ModuleMap::KnownHeader();
-
+
// If 'Filename' is absolute, check to see if it exists and no searching.
if (llvm::sys::path::is_absolute(Filename)) {
CurDir = nullptr;
@@ -645,7 +640,7 @@
RelativePath->append(Filename.begin(), Filename.end());
}
// Otherwise, just return the file.
-return getFileAndSuggestModule(Filename, IncludeLoc, nullptr,
+return getFileAndSuggestModule(Filename, nullptr,
/*IsSystemHeaderDir*/false,
RequestingModule, SuggestedModule);
}
@@ -682,7 +677,7 @@
Includer ? getFileInfo(Includer).DirInfo != SrcMgr::C_User :
BuildSystemModule;
if (const FileEntry *FE = getFileAndSuggestModule(
- TmpDir, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader,
+ TmpDir, IncluderAndDir.second, IncluderIsSystemHeader,
RequestingModule, SuggestedModule)) {
if (!Includer) {
assert(First && "only first includer can have no file");
@@ -776,7 +771,7 @@
bool InUserSpecifiedSystemFramework = false;
bool HasBeenMapped = false;
const FileEntry *FE = SearchDirs[i].LookupFile(
-Filename, *this, IncludeLoc, SearchPath, RelativePath, RequestingModule,
+Filename, *this, SearchPath, RelativePath, RequestingModule,
SuggestedModule, InUserSpecifiedSystemFramework, HasBeenMapped,
MappedName);
if (HasBeenMapped) {
@@ -815,7 +810,7 @@
size_t SlashPos = Filename.find('/');
if (SlashPos != StringRef::npos) {
HFI.IndexHeaderMapHeader = 1;
-HFI.Framework = getUniqueFrameworkName(StringRef(Filename.begin(),
+HFI.Framework = getUniqueFrameworkName(StringRef(Filename.begin(),
SlashPos));
}
[PATCH] D32903: Remove unused variable and argument from Lex/HeaderSearch.cpp
takuto.ikuta created this revision.
https://reviews.llvm.org/D32903
Files:
clang/lib/Lex/HeaderSearch.cpp
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -234,9 +234,9 @@
break;
}
}
-
+
// FIXME: Figure out how header maps and module maps will work together.
-
+
// Only deal with normal search directories.
if (!SearchDirs[Idx].isNormalDir())
continue;
@@ -251,7 +251,7 @@
if (Module)
break;
}
-
+
// Search for a module map in a subdirectory with the same name as the
// module.
SmallString<128> NestedModuleMapDirName;
@@ -399,11 +399,8 @@
///
/// \param FileMgr The file manager to use for directory lookups.
/// \param DirName The name of the framework directory.
-/// \param SubmodulePath Will be populated with the submodule path from the
-/// returned top-level module to the originally named framework.
static const DirectoryEntry *
-getTopFrameworkDir(FileManager &FileMgr, StringRef DirName,
- SmallVectorImpl &SubmodulePath) {
+getTopFrameworkDir(FileManager &FileMgr, StringRef DirName) {
assert(llvm::sys::path::extension(DirName) == ".framework" &&
"Not a framework directory");
@@ -437,7 +434,6 @@
// If this is a framework directory, then we're a subframework of this
// framework.
if (llvm::sys::path::extension(DirName) == ".framework") {
- SubmodulePath.push_back(llvm::sys::path::stem(DirName));
TopFrameworkDir = Dir;
}
} while (true);
@@ -518,7 +514,7 @@
RelativePath->clear();
RelativePath->append(Filename.begin()+SlashPos+1, Filename.end());
}
-
+
// Check "/System/Library/Frameworks/Cocoa.framework/Headers/file.h"
unsigned OrigSize = FrameworkName.size();
@@ -630,7 +626,7 @@
if (SuggestedModule)
*SuggestedModule = ModuleMap::KnownHeader();
-
+
// If 'Filename' is absolute, check to see if it exists and no searching.
if (llvm::sys::path::is_absolute(Filename)) {
CurDir = nullptr;
@@ -815,7 +811,7 @@
size_t SlashPos = Filename.find('/');
if (SlashPos != StringRef::npos) {
HFI.IndexHeaderMapHeader = 1;
-HFI.Framework = getUniqueFrameworkName(StringRef(Filename.begin(),
+HFI.Framework = getUniqueFrameworkName(StringRef(Filename.begin(),
SlashPos));
}
}
@@ -991,7 +987,7 @@
/// \brief Merge the header file info provided by \p OtherHFI into the current
/// header file info (\p HFI)
-static void mergeHeaderFileInfo(HeaderFileInfo &HFI,
+static void mergeHeaderFileInfo(HeaderFileInfo &HFI,
const HeaderFileInfo &OtherHFI) {
assert(OtherHFI.External && "expected to merge external HFI");
@@ -1013,7 +1009,7 @@
if (HFI.Framework.empty())
HFI.Framework = OtherHFI.Framework;
}
-
+
/// getFileInfo - Return the HeaderFileInfo structure for the specified
/// FileEntry.
HeaderFileInfo &HeaderSearch::getFileInfo(const FileEntry *FE) {
@@ -1195,14 +1191,14 @@
return FrameworkNames.insert(Framework).first->first();
}
-bool HeaderSearch::hasModuleMap(StringRef FileName,
+bool HeaderSearch::hasModuleMap(StringRef FileName,
const DirectoryEntry *Root,
bool IsSystem) {
if (!HSOpts->ImplicitModuleMaps)
return false;
SmallVector FixUpDirectories;
-
+
StringRef DirName = FileName;
do {
// Get the parent directory name.
@@ -1235,7 +1231,7 @@
// If we hit the top of our search, we're done.
if (Dir == Root)
return false;
-
+
// Keep track of all of the directories we checked, so we can mark them as
// having module maps if we eventually do find a module map.
FixUpDirectories.push_back(Dir);
@@ -1292,10 +1288,9 @@
// If we're supposed to suggest a module, look for one now.
if (needModuleLookup(RequestingModule, SuggestedModule)) {
// Find the top-level framework based on this framework.
-SmallVector SubmodulePath;
-const DirectoryEntry *TopFrameworkDir
- = ::getTopFrameworkDir(FileMgr, FrameworkName, SubmodulePath);
-
+const DirectoryEntry *TopFrameworkDir =
+::getTopFrameworkDir(FileMgr, FrameworkName);
+
// Determine the name of the top-level framework.
StringRef ModuleName = llvm::sys::path::stem(TopFrameworkDir->getName());
@@ -1428,16 +1423,16 @@
}
-HeaderSearch::LoadModuleMapResult
+HeaderSearch::LoadModuleMapResult
HeaderSearch::loadModuleMapFile(StringRef DirName, bool IsSystem,
bool IsFramework) {
if (const DirectoryEntry *Dir = FileMgr.getDirectory(DirName))
return loadModuleMapFile(Dir, IsSystem, IsFramework);
-
+
return LMM_NoDirectory;
}
