loladiro added inline comments.
================
Comment at: include/clang/Basic/AttrDocs.td:1638
@@ +1637,3 @@
+
+Note that to ensure correct execution the user MUST make certain that no
+other translation unit has an implicit instantiation of the same entity. In
----------------
aaron.ballman wrote:
> In the unlikely event the user gets this wrong (lol), is there a way to
> diagnose this on the backend? The wording here makes it sound like it's
> possible for this to cause silent miscompiles, which would be unfortunate.
I don't think there is anything that can be really done here. I believe the
static linker will link this just fine (there'll be one weak and one strong
symbol). The difficulty of course arises with dynamic linkers that won't bother
looking for any weak symbols to merge with (at least the OS X one doesn't). I
suppose it's not really that different from missing a `weak` attribute on some
declarations but not others.
================
Comment at: lib/CodeGen/CGVTables.cpp:744
@@ +743,3 @@
+ // UniqueInstantiationAttr), the VTable should too.
+ keyFunction->dump();
+ if (Context.containedInUniqueInstantiation(keyFunction))
----------------
aaron.ballman wrote:
> Looks like some debugging code accidentally made it in.
Thanks!
================
Comment at: lib/Sema/SemaDecl.cpp:2285
@@ +2284,3 @@
+ // here.
+ if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(New)) {
+ TemplateSpecializationKind kind = CTSD->getSpecializationKind();
----------------
aaron.ballman wrote:
> Isn't this a bit broad? I do agree that we don't want to give incorrect
> warnings, but it seems like this may inadvertently silence correct warnings
> as well. If my gut feeling is wrong (which it might be), it would be good to
> have some test cases confirming that this doesn't silence correct
> diagnostics.
As far as I know, this is only called from mergeDeclAttributes, which didn't
use to be called on these before this patch. I also don't think any attributes
but dllexport or unique_instantiation apply to these declarations and those two
both need special processing.
http://reviews.llvm.org/D13330
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
