aaron.ballman 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
----------------
> I don't think there is anything that can be really done here.
That's unfortunate, but I think you may be right. Blech.
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2452
@@ +2451,3 @@
+def err_unique_instantiation_wrong_decl : Error<
+ "unique_instantiation attribute on something that is not a explicit template
declaration or instantiation.">;
+def err_unique_instantiation_no_declaration : Error<
----------------
Please quote 'unique_instantiation' in the diagnostic (here and in the
subsequent uses).
Instead of "something", let's use "a declaration"?
"a explicit template" should be "an explicit template".
Also, diagnostics are not complete sentences, so should not be capitalized or
end with a full stop.
Actually, how about this for a new wording:
"'unique_instantiation' attribute only applies to an explicit template
declaration or instantiation"
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2456
@@ -2450,1 +2455,3 @@
+def err_unique_instantiation_not_previous : Error<
+ "The unique_instantiation must be specified on all declarations and
definitions of a particular explicit template instantiation.">;
----------------
Should say: the 'unique_instantiation' attribute.
Also, I'm not keen on "a particular" as it's very non-specific.
I wonder if we need err_unique_instantiation_no_declaration and
err_unique_instantiation_not_previous to be separate diagnostics? Aren't they
both effectively saying,
"a 'unique_instantiation' attribute must be specified for all declarations and
definitions of the explicit template instantiation"
================
Comment at: lib/AST/ASTContext.cpp:8246
@@ +8245,3 @@
+ if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) {
+ if (CTSD->hasAttr<UniqueInstantiationAttr>()) {
+ return true;
----------------
Elide braces
================
Comment at: lib/Sema/SemaDecl.cpp:2184
@@ +2183,3 @@
+
+ Attr *NewAttr = New->getAttr<UniqueInstantiationAttr>();
+ SourceLocation NewLoc = NewAttr ? NewAttr->getLocation() :
New->getLocStart();
----------------
hasAttr<> followed by getAttr<> that can be simplified.
================
Comment at: lib/Sema/SemaDecl.cpp:2287
@@ +2286,3 @@
+ // declaration, so we'd give incorrect warnings here.
+ if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(New)) {
+ TemplateSpecializationKind kind = CTSD->getSpecializationKind();
----------------
> 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.
Okay, thanks!
================
Comment at: lib/Sema/SemaTemplate.cpp:7874
@@ +7873,3 @@
+ diag::err_unique_instantiation_no_declaration);
+ else if (HadDeclaration && !OldUniqueInstantiation) {
+ Diag(NewUniqueInstantiation->getLocation(),
----------------
Elide braces
http://reviews.llvm.org/D13330
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
