This revision was automatically updated to reflect the committed changes.
Closed by commit rL298410: Correct class-template deprecation behavior
(authored by erichkeane).
Changed prior to commit:
https://reviews.llvm.org/D27486?vs=84330&id=92514#toc
Repository:
rL LLVM
https://reviews.llvm.
erichkeane added a comment.
@rsmith did you ever get a chance to re-review this? Is this what you were
wanting for this?
https://reviews.llvm.org/D27486
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman
rsmith added inline comments.
Comment at: include/clang/Basic/Attr.td:301
bit DuplicatesAllowedWhileMerging = 0;
+ // Set to true if this attribute should apply to template declarations,
+ // remains false if this should only be applied to the definition.
er
erichkeane updated this revision to Diff 84330.
erichkeane added a comment.
Fixed the deprecated location 'note'. Observe the changes to a variety of
tests.
https://reviews.llvm.org/D27486
Files:
include/clang/Basic/Attr.td
include/clang/Sema/Sema.h
lib/Sema/SemaDeclAttr.cpp
lib/Sema/
erichkeane added a comment.
I've actually figured out how to fix the diagnosis location. I switched the
diagnosis to use the location of the actual [[deprecated]] attribute instead of
the Declaration location. IMO, this is a BETTER note anyway (since the arrow
points to the actual [[deprecate
erichkeane updated this revision to Diff 84192.
erichkeane added a comment.
I've updated this patch based on some of Richard's suggestions, simplified my
implementation, and added a few more tests. Currently the A and B
notes are in the wrong place, but I was hoping someone could help me figure
erichkeane added a comment.
Thanks for the feedback Richard! I'll look into whether instantiating the full
attribute set upon creation is a possibility.
Comment at: include/clang/Basic/Attr.td:301
bit DuplicatesAllowedWhileMerging = 0;
+ // Set to true if this attribute s
rsmith added inline comments.
Comment at: include/clang/Basic/Attr.td:301
bit DuplicatesAllowedWhileMerging = 0;
+ // Set to true if this attribute should apply to template declarations,
+ // remains false if this should only be applied to the definition.
I
erichkeane added a comment.
Thanks for your help here Aaron.
I agree about waiting for @rsmith, thanks!
https://reviews.llvm.org/D27486
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
This LGTM, but you should wait for @rsmith to sign off as well.
https://reviews.llvm.org/D27486
___
cfe-commits mailing list
cfe-co
erichkeane added a comment.
Marking Aaron's comments done.
https://reviews.llvm.org/D27486
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
erichkeane updated this revision to Diff 83081.
erichkeane marked 3 inline comments as done.
erichkeane added a comment.
Aaron's new comments
https://reviews.llvm.org/D27486
Files:
include/clang/Basic/Attr.td
include/clang/Sema/Sema.h
lib/Sema/SemaTemplate.cpp
lib/Sema/SemaTemplateInsta
aaron.ballman added inline comments.
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:322
+ CXXThisScopeRAII ThisScope(*this, ThisContext, /*TypeQuals*/ 0,
+ ND && ND->isCXXInstanceMember());
+
No need to check for `ND` he
erichkeane updated this revision to Diff 83072.
erichkeane marked 2 inline comments as done.
erichkeane added a comment.
Updated based on Aarons comments.
https://reviews.llvm.org/D27486
Files:
include/clang/Basic/Attr.td
include/clang/Sema/Sema.h
lib/Sema/SemaTemplate.cpp
lib/Sema/Sema
erichkeane marked 6 inline comments as done.
erichkeane added a comment.
All Aaron's comments addressed in a patch that is on its way!
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:320
+CXXRecordDecl *ThisContext =
+dyn_cast_or_null(ND->getDeclContext());
+
aaron.ballman added inline comments.
Comment at: include/clang/Basic/Attr.td:301
bit DuplicatesAllowedWhileMerging = 0;
+ // Set to true if this attribute should apply to template declaration,
+ // remains false if this should only be applied on definition.
erichkeane updated this revision to Diff 82909.
erichkeane added a comment.
Updated based on Richard Smith's suggestion, all tests pass with no alteration,
and the initial incorrect behavior was corrected in an existing test.
https://reviews.llvm.org/D27486
Files:
include/clang/Basic/Attr.td
erichkeane updated this revision to Diff 80907.
erichkeane added a comment.
I've been trying to do what @rsmith suggested, so this is a WIP checkpoint, I
was hoping you could take a look and tell me if I'm on the right track. I
beleive I'm very nearly done, and all but 1 of the tests pass, whic
erichkeane added a comment.
Thanks David!
To all - I'm actually doing my best to rewrite this based on Richard's
suggestions, so look for a 'in progress' update to this review as soon as I get
something that is reasonably presentable.
https://reviews.llvm.org/D27486
majnemer added inline comments.
Comment at: lib/Sema/SemaTemplate.cpp:2355
Converted, nullptr);
+ if (auto *attr = ClassTemplate->getTemplatedDecl()
+ ->getAttr())
Please capital
erichkeane added a comment.
In https://reviews.llvm.org/D27486#615174, @rsmith wrote:
> Thanks!
>
> If you look at `Sema::InstantiateClass`, we instantiate all of the attributes
> there. The problem seems to be that that happens only when instantiating the
> class definition, which happens afte
rsmith added a comment.
Thanks!
If you look at `Sema::InstantiateClass`, we instantiate all of the attributes
there. The problem seems to be that that happens only when instantiating the
class definition, which happens after the point at which we would diagnose use
of a deprecated declaration.
erichkeane removed rL LLVM as the repository for this revision.
erichkeane updated this revision to Diff 80489.
erichkeane added a comment.
Corrected single line statement formatting re-squiggly braces.
https://reviews.llvm.org/D27486
Files:
lib/Sema/SemaTemplate.cpp
test/CXX/dcl.dcl/dcl.at
erichkeane created this revision.
erichkeane added reviewers: cfe-commits, chandlerc, majnemer.
erichkeane set the repository for this revision to rL LLVM.
Based on the comment in the test, and my reading of the standard, a deprecated
warning should be issued in the following case:
template [[dep
24 matches
Mail list logo
