[PATCH] clang-cl: Only mark dllexported constexpr functions once

Hans Wennborg hans at chromium.org
Thu Dec 4 09:18:00 PST 2014


Nice!

I think the patch description could be a little more explanatory, maybe something like "don't mark constexpr members of dllexport classes referenced, since they considered are referenced already".

================
Comment at: lib/Sema/SemaDeclCXX.cpp:4745
@@ -4744,1 +4744,3 @@
       if (MD->isUserProvided()) {
+        // Don't mark constexpr functions again
+        if (MD->isConstexpr())
----------------
The comment is a little vague (and should end with a period). Maybe something like "constexpr functions are already marked referenced."

Also, David pointed out that this does not only apply to user-defined functions, so it should be moved back to where you first suggested in the PR. Sorry for my misleading suggestion here before.

================
Comment at: test/CodeGenCXX/dllexport.cpp:621
@@ -618,1 +620,3 @@
+template class __declspec(dllexport) Q<void>;
+// M32-DAG: define weak_odr dllexport x86_thiscallcc %struct.Q* @"\01??0?$Q at X@@QAE at XZ"(%struct.Q* returned %this)
 
----------------
Nit: the template is declared as a struct, so it should be referenced as a struct when instantiated too.

Please also add a test where the constructor is explicitly defaulted, i.e.

  template <typename> struct P { constexpr P() = default; };

http://reviews.llvm.org/D6528






More information about the cfe-commits mailing list