rsmith added a comment.
I reverted this in r284081, and relanded with fixes described here as r284284.
Comment at: lib/Sema/SemaDecl.cpp:9712
+
+ // Demote the newly parsed definition to a fake declaration.
+ if (!VDecl->isThisDeclarationADemotedDefinition())
v.g.vassilev closed this revision.
v.g.vassilev marked an inline comment as done.
v.g.vassilev added a comment.
Relanded in r284008.
https://reviews.llvm.org/D24508
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
v.g.vassilev updated this revision to Diff 74353.
v.g.vassilev marked 2 inline comments as done.
v.g.vassilev added a comment.
We should not access the IsThisDeclarationADemotedDefinition bits if the decl
is ParmVarDecl.
https://reviews.llvm.org/D24508
Files:
include/clang/AST/Decl.h
rsmith added inline comments.
Comment at: include/clang/AST/Decl.h:1213
+ bool isThisDeclarationADemotedDefinition() const {
+return NonParmVarDeclBits.IsThisDeclarationADemotedDefinition;
+ }
This is the bug. You can't read this bit here without first
v.g.vassilev marked 2 inline comments as done.
v.g.vassilev added a comment.
Landed in r283882.
Comment at: include/clang/AST/Decl.h:1222
+ void demoteThisDefinitionToDeclaration() {
+assert (!isThisDeclarationADemotedDefinition() && "Aleady demoted!");
+assert
rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.
Comment at: include/clang/AST/Decl.h:1222
+ void demoteThisDefinitionToDeclaration() {
+assert (!isThisDeclarationADemotedDefinition() && "Aleady demoted!");
+
v.g.vassilev updated this revision to Diff 74129.
v.g.vassilev marked 3 inline comments as done.
v.g.vassilev added a comment.
Address comments.
https://reviews.llvm.org/D24508
Files:
include/clang/AST/Decl.h
lib/AST/Decl.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaTemplate.cpp
rsmith added inline comments.
> SemaDecl.cpp:3692-3693
> + // Demote the newly parsed definition to a fake declaration.
> + // FIXME: Sema::AddInitializationToDecl still allows two definitions,
> + // which make the AST variants inconsistent.
> + assert (Def != New && "There
v.g.vassilev added inline comments.
> rsmith wrote in SemaTemplate.cpp:509
> This function still appears to be able to return true (indicating to the
> caller that a diagnostic was produced) without actually producing a
> diagnostic.
Is it better now?
> rsmith wrote in SemaTemplate.cpp:505
>
v.g.vassilev updated this revision to Diff 73803.
v.g.vassilev marked 4 inline comments as done.
v.g.vassilev added a comment.
Rebase, comments some more fixes.
https://reviews.llvm.org/D24508
Files:
include/clang/AST/Decl.h
lib/AST/Decl.cpp
lib/Sema/SemaDecl.cpp
rsmith added a comment.
This looks like it's going in the right direction.
> Decl.cpp:2269-2272
> + // If we have hit a point where the user provided a specialization of
> + // this template, we're done looking.
> + if (VarTemplate->isMemberSpecialization())
> +break;
I
v.g.vassilev updated this revision to Diff 73688.
v.g.vassilev added a comment.
Address some comments and publish current progress.
https://reviews.llvm.org/D24508
Files:
include/clang/AST/Decl.h
lib/AST/Decl.cpp
lib/Sema/SemaTemplate.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
rsmith added a comment.
I think you'll also need to update the `ASTDeclReader` to merge `VarDecl`
definitions together if it reads a definition and there already is one in the
AST. I note that right now `Sema::AddInitializerToDecl` permits multiple
definitions of a `VarDecl`, which doesn't
rsmith added a comment.
I expect this patch to cause problems if the two definitions of the variable
template come from different modules, because at deserialization time we don't
merge the definitions together sensibly (it looks like we end up with a
redeclaration chain with multiple
v.g.vassilev created this revision.
v.g.vassilev added a reviewer: rsmith.
v.g.vassilev added a subscriber: cfe-commits.
v.g.vassilev set the repository for this revision to rL LLVM.
Repository:
rL LLVM
https://reviews.llvm.org/D24508
Files:
lib/Sema/SemaTemplate.cpp
15 matches
Mail list logo
