[PATCH] D54319: clang-cl: Add documentation for /Zc:dllexportInlines-

2018-11-13 Thread Hans Wennborg via Phabricator via cfe-commits
hans marked 3 inline comments as done.
hans added a comment.

Thanks for the review! I made adjustments in r346748.




Comment at: cfe/trunk/docs/UsersManual.rst:3104
+
+This causes the class-level `dllexport` and `dllimport` attributes not to be
+applied to inline member functions, as they otherwise would. For example, in

thakis wrote:
> "to not be applied" sounds more correct to me, but I'm not a native speaker 
> either. And I think "to not apply to inline member functions" works to and is 
> shorter and not passive, and it makes the part after the comma sound better 
> (with the passive I think it might have to be "as they otherwise would be"?)
Yeah, "to not apply to inline member functions" is better.


Repository:
  rL LLVM

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] D54319: clang-cl: Add documentation for /Zc:dllexportInlines-

2018-11-12 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Nice!




Comment at: cfe/trunk/docs/UsersManual.rst:2950
   /Z7 Enable CodeView debug information in object files
+  /Zc:dllexportInlines-   Don't dllexport/import inline member functions 
of dllexport/import classes
+  /Zc:dllexportInlinesdllexport/import inline member functions of 
dllexport/import classes (default)

very nit: I'd s/import/dllimport/. It's a bit longer, but not much, and more 
greppable. (for both + and - versions)



Comment at: cfe/trunk/docs/UsersManual.rst:3104
+
+This causes the class-level `dllexport` and `dllimport` attributes not to be
+applied to inline member functions, as they otherwise would. For example, in

"to not be applied" sounds more correct to me, but I'm not a native speaker 
either. And I think "to not apply to inline member functions" works to and is 
shorter and not passive, and it makes the part after the comma sound better 
(with the passive I think it might have to be "as they otherwise would be"?)



Comment at: cfe/trunk/docs/UsersManual.rst:3173
+This could lead to very subtle bugs. Using ``-fvisibility-inlines-hidden`` can
+lead to the same issue.
+

Maybe suggest a workaround in addition to pointing out the problem? ("In these 
cases, a workaround is to make foo() not inline" or similar)


Repository:
  rL LLVM

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] D54319: clang-cl: Add documentation for /Zc:dllexportInlines-

2018-11-12 Thread Hans Wennborg via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346639: clang-cl: Add documentation for 
/Zc:dllexportInlines- (authored by hans, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54319?vs=173338=173621#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D54319

Files:
  cfe/trunk/docs/UsersManual.rst
  cfe/trunk/include/clang/Driver/CLCompatOptions.td

Index: cfe/trunk/include/clang/Driver/CLCompatOptions.td
===
--- cfe/trunk/include/clang/Driver/CLCompatOptions.td
+++ cfe/trunk/include/clang/Driver/CLCompatOptions.td
@@ -335,8 +335,10 @@
   MetaVarName<"">;
 def _SLASH_Y_ : CLFlag<"Y-">,
   HelpText<"Disable precompiled headers, overrides /Yc and /Yu">;
-def _SLASH_Zc_dllexportInlines : CLFlag<"Zc:dllexportInlines">;
-def _SLASH_Zc_dllexportInlines_ : CLFlag<"Zc:dllexportInlines-">;
+def _SLASH_Zc_dllexportInlines : CLFlag<"Zc:dllexportInlines">,
+  HelpText<"dllexport/import inline member functions of dllexport/import classes (default)">;
+def _SLASH_Zc_dllexportInlines_ : CLFlag<"Zc:dllexportInlines-">,
+  HelpText<"Don't dllexport/import inline member functions of dllexport/import classes">;
 def _SLASH_Fp : CLJoined<"Fp">,
   HelpText<"Set pch filename (with /Yc and /Yu)">, MetaVarName<"">;
 
Index: cfe/trunk/docs/UsersManual.rst
===
--- cfe/trunk/docs/UsersManual.rst
+++ cfe/trunk/docs/UsersManual.rst
@@ -2947,6 +2947,8 @@
   /Yc   Generate a pch file for all code up to and including 
   /Yu   Load a pch file and use it instead of all code up to and including 
   /Z7 Enable CodeView debug information in object files
+  /Zc:dllexportInlines-   Don't dllexport/import inline member functions of dllexport/import classes
+  /Zc:dllexportInlinesdllexport/import inline member functions of dllexport/import classes (default)
   /Zc:sizedDealloc-   Disable C++14 sized global deallocation functions
   /Zc:sizedDeallocEnable C++14 sized global deallocation functions
   /Zc:strictStrings   Treat string literals as const
@@ -3096,6 +3098,80 @@
 arguments are treated as if they were passed at the end of the clang-cl command
 line.
 
+The /Zc:dllexportInlines- Option
+
+
+This causes the class-level `dllexport` and `dllimport` attributes not to be
+applied to inline member functions, as they otherwise would. For example, in
+the code below `S::foo()` would normally be defined and exported by the DLL,
+but when using the ``/Zc:dllexportInlines-`` flag it is not:
+
+.. code-block:: c
+
+  struct __declspec(dllexport) S {
+void foo() {}
+  }
+
+This has the benefit that the compiler doesn't need to emit a definition of
+`S::foo()` in every translation unit where the declaration is included, as it
+would otherwise do to ensure there's a definition in the DLL even if it's not
+used there. If the declaration occurs in a header file that's widely used, this
+can save significant compilation time and output size. It also reduces the
+number of functions exported by the DLL similarly to what
+``-fvisibility-inlines-hidden`` does for shared objects on ELF and Mach-O.
+Since the function declaration comes with an inline definition, users of the
+library can use that definition directly instead of importing it from the DLL.
+
+Note that the Microsoft Visual C++ compiler does not support this option, and
+if code in a DLL is compiled with ``/Zc:dllexportInlines-``, the code using the
+DLL must be compiled in the same way so that it doesn't attempt to dllimport
+the inline member functions. The reverse scenario should generally work though:
+a DLL compiled without this flag (such as a system library compiled with Visual
+C++) can be referenced from code compiled using the flag, meaning that the
+referencing code will use the inline definitions instead of importing them from
+the DLL.
+
+Also note that like when using ``-fvisibility-inlines-hidden``, the address of
+`S::foo()` will be different inside and outside the DLL, breaking the C/C++
+standard requirement that functions have a unique address.
+
+The flag does not apply to explicit class template instantiation definitions or
+declarations, as those are typically used to explicitly provide a single
+definition in a DLL, (dllexported instantiation definition) or to signal that
+the definition is available elsewhere (dllimport instantiation declaration). It
+also doesn't apply to inline members with static local variables, to ensure
+that the same instance of the variable is used inside and outside the DLL.
+
+Using this flag can cause problems when inline functions that would otherwise
+be dllexported refer to internal symbols of a DLL. For example:
+
+.. code-block:: c
+
+  void internal();
+
+  struct 

[PATCH] D54319: clang-cl: Add documentation for /Zc:dllexportInlines-

2018-11-10 Thread Takuto Ikuta via Phabricator via cfe-commits
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] D54319: clang-cl: Add documentation for /Zc:dllexportInlines-

2018-11-09 Thread Hans Wennborg via Phabricator via cfe-commits
hans created this revision.
hans added reviewers: takuto.ikuta, thakis, rnk.

A great feature needs great documentation. Please take a look!


https://reviews.llvm.org/D54319

Files:
  docs/UsersManual.rst
  include/clang/Driver/CLCompatOptions.td

Index: include/clang/Driver/CLCompatOptions.td
===
--- include/clang/Driver/CLCompatOptions.td
+++ include/clang/Driver/CLCompatOptions.td
@@ -335,8 +335,10 @@
   MetaVarName<"">;
 def _SLASH_Y_ : CLFlag<"Y-">,
   HelpText<"Disable precompiled headers, overrides /Yc and /Yu">;
-def _SLASH_Zc_dllexportInlines : CLFlag<"Zc:dllexportInlines">;
-def _SLASH_Zc_dllexportInlines_ : CLFlag<"Zc:dllexportInlines-">;
+def _SLASH_Zc_dllexportInlines : CLFlag<"Zc:dllexportInlines">,
+  HelpText<"dllexport/import inline member functions of dllexport/import classes (default)">;
+def _SLASH_Zc_dllexportInlines_ : CLFlag<"Zc:dllexportInlines-">,
+  HelpText<"Don't dllexport/import inline member functions of dllexport/import classes">;
 def _SLASH_Fp : CLJoined<"Fp">,
   HelpText<"Set pch filename (with /Yc and /Yu)">, MetaVarName<"">;
 
Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@ -2947,6 +2947,8 @@
   /Yc   Generate a pch file for all code up to and including 
   /Yu   Load a pch file and use it instead of all code up to and including 
   /Z7 Enable CodeView debug information in object files
+  /Zc:dllexportInlines-   Don't dllexport/import inline member functions of dllexport/import classes
+  /Zc:dllexportInlinesdllexport/import inline member functions of dllexport/import classes (default)
   /Zc:sizedDealloc-   Disable C++14 sized global deallocation functions
   /Zc:sizedDeallocEnable C++14 sized global deallocation functions
   /Zc:strictStrings   Treat string literals as const
@@ -3096,6 +3098,80 @@
 arguments are treated as if they were passed at the end of the clang-cl command
 line.
 
+The /Zc:dllexportInlines- Option
+
+
+This causes the class-level `dllexport` and `dllimport` attributes not to be
+applied to inline member functions, as they otherwise would. For example, in
+the code below `S::foo()` would normally be defined and exported by the DLL,
+but when using the ``/Zc:dllexportInlines-`` flag it is not:
+
+.. code-block:: c
+
+  struct __declspec(dllexport) S {
+void foo() {}
+  }
+
+This has the benefit that the compiler doesn't need to emit a definition of
+`S::foo()` in every translation unit where the declaration is included, as it
+would otherwise do to ensure there's a definition in the DLL even if it's not
+used there. If the declaration occurs in a header file that's widely used, this
+can save significant compilation time and output size. It also reduces the
+number of functions exported by the DLL similarly to what
+``-fvisibility-inlines-hidden`` does for shared objects on ELF and Mach-O.
+Since the function declaration comes with an inline definition, users of the
+library can use that definition directly instead of importing it from the DLL.
+
+Note that the Microsoft Visual C++ compiler does not support this option, and
+if code in a DLL is compiled with ``/Zc:dllexportInlines-``, the code using the
+DLL must be compiled in the same way so that it doesn't attempt to dllimport
+the inline member functions. The reverse scenario should generally work though:
+a DLL compiled without this flag (such as a system library compiled with Visual
+C++) can be referenced from code compiled using the flag, meaning that the
+referencing code will use the inline definitions instead of importing them from
+the DLL.
+
+Also note that like when using ``-fvisibility-inlines-hidden``, the address of
+`S::foo()` will be different inside and outside the DLL, breaking the C/C++
+standard requirement that functions have a unique address.
+
+The flag does not apply to explicit class template instantiation definitions or
+declarations, as those are typically used to explicitly provide a single
+definition in a DLL, (dllexported instantiation definition) or to signal that
+the definition is available elsewhere (dllimport instantiation declaration). It
+also doesn't apply to inline members with static local variables, to ensure
+that the same instance of the variable is used inside and outside the DLL.
+
+Using this flag can cause problems when inline functions that would otherwise
+be dllexported refer to internal symbols of a DLL. For example:
+
+.. code-block:: c
+
+  void internal();
+
+  struct __declspec(dllimport) S {
+void foo() { internal(); }
+  }
+
+Normally, references to `S::foo()` would use the definition in the DLL from
+which it was exported, and which presumably also has the definition of
+`internal()`. However, when using ``/Zc:dllexportInlines-``, the inline