[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
rsmith added inline comments.
Comment at: include/clang/Sema/Sema.h:4040-4041
// explicit nested-name-specifier).
- void MarkAnyDeclReferenced(SourceLocation Loc, Decl *D, bool MightBeOdrUse);
+ void MarkAnyDeclReferenced(SourceLocation Loc, Decl *D, bool MightBeOdrUse,
+ CXXScopeSpec *SS = nullptr, Expr *E = nullptr);
void MarkFunctionReferenced(SourceLocation Loc, FunctionDecl *Func,
Pass in the FoundDecl instead of a scope specifier and an expression.
Comment at: lib/Sema/SemaDeclCXX.cpp:15520-15525
+ while ((NA = dyn_cast(ND)) && !NA->isReferenced()) {
+NA->setReferenced();
+ND = NA->getAliasedNamespace();
+if (auto *NNS = NA->getQualifier())
+ MarkNamespaceAliasReferenced(NNS->getAsNamespaceAlias());
+ }
This is not the right approach. You should not iteratively mark namespaces as
referenced here. Instead, when we parse the first namespace alias declaration
in the chain, *it* should mark the subsequent one referenced immediately. So:
```
namespace A { int n; }
namespace B = A; // this declaration marks A referenced
namespace C = B; // this declaration marks B referenced
int k = C::n; // parsing the nested-name-specifier of this expression marks C
referenced
```
Comment at: lib/Sema/SemaDeclCXX.cpp:15531-15586
+void Sema::MarkUsingReferenced(NamedDecl *ND, SourceLocation Loc,
+ CXXScopeSpec *SS, Expr *E) {
+ // The declaration was not defined in a namespace.
+ auto *DC = ND->getDeclContext();
+ if (!DC->isNamespace())
+return;
+
You should not do any of this. You should keep track of how each name was
actually originally found, and mark the FoundDecl as referenced. Do not attempt
to redo the lookup like you do here. You will get it wrong, and in any case,
redoing the lookup is wasteful.
https://reviews.llvm.org/D46190
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
probinson accepted this revision. probinson added a comment. Although I am far from expert in how Clang manages declarations, AFAICT this does what @rsmith requested, so LGTM. https://reviews.llvm.org/D46190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso added a comment.
@rsmith & @probinson,
Is there anything I can add to this patch in order to submit it?
This patch is blocking the review https://reviews.llvm.org/D44826 which is
already approved.
Thanks very much.
Comment at: lib/Sema/SemaDeclCXX.cpp:15515
+
+// Mark the alias declaration and any associated chain as referenced.
+void Sema::MarkNamespaceAliasReferenced(NamedDecl *ND) {
probinson wrote:
> This should use `///` to be picked up by doxygen.
Uploaded a new patch that contains that change.
https://reviews.llvm.org/D46190
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso updated this revision to Diff 156460.
CarlosAlbertoEnciso marked an inline comment as done.
CarlosAlbertoEnciso added a comment.
A minor modification to allow the comments for `MarkNamespaceAliasReferenced`
to be picked up by doxygen.
https://reviews.llvm.org/D46190
Files:
include/clang/Sema/Lookup.h
include/clang/Sema/Sema.h
include/clang/Sema/SemaInternal.h
lib/Sema/SemaCXXScopeSpec.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprCXX.cpp
lib/Sema/SemaLookup.cpp
lib/Sema/SemaTemplate.cpp
test/PCH/cxx-templates.cpp
test/SemaCXX/referenced_alias_declaration_1.cpp
test/SemaCXX/referenced_alias_declaration_2.cpp
test/SemaCXX/referenced_using_all.cpp
test/SemaCXX/referenced_using_declaration_1.cpp
test/SemaCXX/referenced_using_declaration_2.cpp
test/SemaCXX/referenced_using_directive.cpp
Index: test/SemaCXX/referenced_using_directive.cpp
===
--- test/SemaCXX/referenced_using_directive.cpp
+++ test/SemaCXX/referenced_using_directive.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
+
+namespace N {
+typedef int Integer;
+int var;
+} // namespace N
+
+void Fa() {
+ using namespace N; // Referenced
+ var = 1;
+}
+
+void Fb() {
+ using namespace N;
+ N::var = 1;
+}
+
+void Fc() {
+ using namespace N; // Referenced
+ Integer var = 1;
+}
+
+void Fd() {
+ using namespace N;
+ N::Integer var = 1;
+}
+
+//CHECK: |-FunctionDecl {{.*}} Fa 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDirectiveDecl {{.*}} referenced Namespace {{.*}} 'N'
+//CHECK-NEXT: | `-BinaryOperator {{.*}} 'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'var' 'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: |-FunctionDecl {{.*}} Fb 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDirectiveDecl {{.*}} Namespace {{.*}} 'N'
+//CHECK-NEXT: | `-BinaryOperator {{.*}} 'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'var' 'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: |-FunctionDecl {{.*}} Fc 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDirectiveDecl {{.*}} referenced Namespace {{.*}} 'N'
+//CHECK-NEXT: | `-DeclStmt {{.*}}
+//CHECK-NEXT: | `-VarDecl {{.*}} var 'N::Integer':'int' cinit
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: `-FunctionDecl {{.*}} Fd 'void ()'
+//CHECK-NEXT: `-CompoundStmt {{.*}}
+//CHECK-NEXT: |-DeclStmt {{.*}}
+//CHECK-NEXT: | `-UsingDirectiveDecl {{.*}} Namespace {{.*}} 'N'
+//CHECK-NEXT: `-DeclStmt {{.*}}
+//CHECK-NEXT: `-VarDecl {{.*}} var 'N::Integer':'int' cinit
+//CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
Index: test/SemaCXX/referenced_using_declaration_2.cpp
===
--- test/SemaCXX/referenced_using_declaration_2.cpp
+++ test/SemaCXX/referenced_using_declaration_2.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
+
+namespace N {
+typedef int Integer;
+typedef char Char;
+} // namespace N
+
+using N::Char; // Referenced
+using N::Integer;
+
+void Foo(int p1, N::Integer p2, Char p3) {
+ N::Integer var;
+ var = 0;
+}
+
+using N::Integer; // Referenced
+Integer Bar() {
+ using N::Char;
+ return 0;
+}
+
+//CHECK: |-UsingDecl {{.*}} referenced N::Char
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit referenced Typedef {{.*}} 'Char'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Char' sugar
+//CHECK-NEXT: | |-Typedef {{.*}} 'Char'
+//CHECK-NEXT: | `-BuiltinType {{.*}} 'char'
+//CHECK-NEXT: |-UsingDecl {{.*}} N::Integer
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Integer' sugar
+//CHECK-NEXT: | |-Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-BuiltinType {{.*}} 'int'
+//CHECK-NEXT: |-FunctionDecl {{.*}} Foo 'void (int, N::Integer, N::Char)'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p1 'int'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p2 'N::Integer':'int'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p3 'N::Char':'char'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-VarDecl {{.*}} used var 'N::Integer':'int'
+//CHECK-NEXT: | `-BinaryOperator {{.*}} 'N::Integer':'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'N::Integer':'int' lvalue Var {{.*}} 'var' 'N::Integer':'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 0
+//CHECK-NEXT: |-UsingDecl {{.*}} referenced N::Integer
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit referenced Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-TypedefType {{.*}} '
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso updated this revision to Diff 156456.
CarlosAlbertoEnciso added a comment.
Used `clang-format-diff` as indicated by @probinson:
- The lib and include files follow the clang format.
- The tests mostly follow the clang format. I removed some extra formatting
which was introduced and disturb their readability.
https://reviews.llvm.org/D46190
Files:
include/clang/Sema/Lookup.h
include/clang/Sema/Sema.h
include/clang/Sema/SemaInternal.h
lib/Sema/SemaCXXScopeSpec.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprCXX.cpp
lib/Sema/SemaLookup.cpp
lib/Sema/SemaTemplate.cpp
test/PCH/cxx-templates.cpp
test/SemaCXX/referenced_alias_declaration_1.cpp
test/SemaCXX/referenced_alias_declaration_2.cpp
test/SemaCXX/referenced_using_all.cpp
test/SemaCXX/referenced_using_declaration_1.cpp
test/SemaCXX/referenced_using_declaration_2.cpp
test/SemaCXX/referenced_using_directive.cpp
Index: test/SemaCXX/referenced_using_directive.cpp
===
--- test/SemaCXX/referenced_using_directive.cpp
+++ test/SemaCXX/referenced_using_directive.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
+
+namespace N {
+typedef int Integer;
+int var;
+} // namespace N
+
+void Fa() {
+ using namespace N; // Referenced
+ var = 1;
+}
+
+void Fb() {
+ using namespace N;
+ N::var = 1;
+}
+
+void Fc() {
+ using namespace N; // Referenced
+ Integer var = 1;
+}
+
+void Fd() {
+ using namespace N;
+ N::Integer var = 1;
+}
+
+//CHECK: |-FunctionDecl {{.*}} Fa 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDirectiveDecl {{.*}} referenced Namespace {{.*}} 'N'
+//CHECK-NEXT: | `-BinaryOperator {{.*}} 'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'var' 'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: |-FunctionDecl {{.*}} Fb 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDirectiveDecl {{.*}} Namespace {{.*}} 'N'
+//CHECK-NEXT: | `-BinaryOperator {{.*}} 'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'var' 'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: |-FunctionDecl {{.*}} Fc 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDirectiveDecl {{.*}} referenced Namespace {{.*}} 'N'
+//CHECK-NEXT: | `-DeclStmt {{.*}}
+//CHECK-NEXT: | `-VarDecl {{.*}} var 'N::Integer':'int' cinit
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: `-FunctionDecl {{.*}} Fd 'void ()'
+//CHECK-NEXT: `-CompoundStmt {{.*}}
+//CHECK-NEXT: |-DeclStmt {{.*}}
+//CHECK-NEXT: | `-UsingDirectiveDecl {{.*}} Namespace {{.*}} 'N'
+//CHECK-NEXT: `-DeclStmt {{.*}}
+//CHECK-NEXT: `-VarDecl {{.*}} var 'N::Integer':'int' cinit
+//CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
Index: test/SemaCXX/referenced_using_declaration_2.cpp
===
--- test/SemaCXX/referenced_using_declaration_2.cpp
+++ test/SemaCXX/referenced_using_declaration_2.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
+
+namespace N {
+typedef int Integer;
+typedef char Char;
+} // namespace N
+
+using N::Char; // Referenced
+using N::Integer;
+
+void Foo(int p1, N::Integer p2, Char p3) {
+ N::Integer var;
+ var = 0;
+}
+
+using N::Integer; // Referenced
+Integer Bar() {
+ using N::Char;
+ return 0;
+}
+
+//CHECK: |-UsingDecl {{.*}} referenced N::Char
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit referenced Typedef {{.*}} 'Char'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Char' sugar
+//CHECK-NEXT: | |-Typedef {{.*}} 'Char'
+//CHECK-NEXT: | `-BuiltinType {{.*}} 'char'
+//CHECK-NEXT: |-UsingDecl {{.*}} N::Integer
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Integer' sugar
+//CHECK-NEXT: | |-Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-BuiltinType {{.*}} 'int'
+//CHECK-NEXT: |-FunctionDecl {{.*}} Foo 'void (int, N::Integer, N::Char)'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p1 'int'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p2 'N::Integer':'int'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p3 'N::Char':'char'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-VarDecl {{.*}} used var 'N::Integer':'int'
+//CHECK-NEXT: | `-BinaryOperator {{.*}} 'N::Integer':'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'N::Integer':'int' lvalue Var {{.*}} 'var' 'N::Integer':'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 0
+//CHECK-NEXT: |-UsingDecl {{.*}} referenced N::Integer
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
probinson added a comment.
@rsmith any further thoughts? We would really like to get this in before the
LLVM 7.0 branch, currently scheduled for 1 August.
In https://reviews.llvm.org/D46190#1168027, @CarlosAlbertoEnciso wrote:
> @probinson: I tried `clang-format-diff` and the main format issues are with
> the test cases.
>
> Do you want the test cases to follow the clang format?
The lib and include files absolutely need to follow clang format. Tests should
also, if doing so doesn't disturb their readability. If clang-format-diff is
doing things like rearranging comments in the test files, it depends on whether
it improves or reduces clarity. Up to you.
Comment at: lib/Sema/SemaDeclCXX.cpp:15515
+
+// Mark the alias declaration and any associated chain as referenced.
+void Sema::MarkNamespaceAliasReferenced(NamedDecl *ND) {
This should use `///` to be picked up by doxygen.
https://reviews.llvm.org/D46190
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso added a comment. @probinson: I tried `clang-format-diff` and the main format issues are with the test cases. Do you want the test cases to follow the clang format? https://reviews.llvm.org/D46190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso added a comment. In https://reviews.llvm.org/D46190#1166775, @probinson wrote: > A bunch of style comments. Maybe try clang-format-diff. @probinson: Thanks very much for your review. https://reviews.llvm.org/D46190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso added inline comments.
Comment at: include/clang/Sema/SemaInternal.h:91
Var->markUsed(SemaRef.Context);
+ SemaRef.MarkUsingReferenced(Var, Loc, /*CXXScopeSpec*=*/nullptr, RefExpr);
}
probinson wrote:
> The comments on a nullptr parameter usually use the formal parameter name,
> not its type.
Replaced the type name with the formal parameter name.
Comment at: lib/Sema/SemaDeclCXX.cpp:15554
+
+ for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I) {
+NamedDecl *D = (*I)->getUnderlyingDecl();
probinson wrote:
> Could this be a range-based for loop?
Replaced by a range-based for loop.
Comment at: lib/Sema/SemaExpr.cpp:14460
Func->markUsed(Context);
+ MarkUsingReferenced(Func, Loc, /*CXXScopeSpec*=*/nullptr, E);
}
probinson wrote:
> Parameter comments usually use the formal parameter name, not its type.
Replaced the type name with the formal parameter name.
Comment at: lib/Sema/SemaExpr.cpp:15374
+ SemaRef.MarkAnyDeclReferenced(Loc, D, MightBeOdrUse,
+/*CXXScopeSpec*=*/nullptr, E);
probinson wrote:
> Parameter comments usually use the formal parameter name, not its type.
Replaced the type name with the formal parameter name.
Comment at: lib/Sema/SemaExpr.cpp:15396
+SemaRef.MarkAnyDeclReferenced(Loc, DM, MightBeOdrUse,
+ /*CXXScopeSpec*=*/nullptr, E);
}
probinson wrote:
> Parameter comments usually use the formal parameter name, not its type.
Replaced the type name with the formal parameter name.
Comment at: lib/Sema/SemaLookup.cpp:98
+using UDirs = llvm::SmallPtrSet;
+UDirs usings;
As `UDirs` is used to reference an instance of `UnqualUsingDirectiveSet`, I
have changed `UDirs` to `UsingDirs`.
Comment at: lib/Sema/SemaLookup.cpp:196
+void addUsingDirective(LookupResult &R) {
+ for (auto I = usings.begin(), E = usings.end(); I != E; ++I)
+R.addDecl((*I));
probinson wrote:
> Can this be a range-based for loop?
Replaced with a range-based for loop.
Comment at: lib/Sema/SemaLookup.cpp:1064
+static void
+AddUsingDirectives(LookupResult &R,UnqualUsingDirectiveSet &UDirs) {
+ if (R.isAddUsingDirectives())
probinson wrote:
> Space after the comma.
> `UDirs` has a different meaning elsewhere in this file, maybe `UsingDirs`
> instead?
Preserved 'UDirs' in this function, but changed the 'UDirs' typedef into
'UsingDirs'.
https://reviews.llvm.org/D46190
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso updated this revision to Diff 156234.
CarlosAlbertoEnciso marked 11 inline comments as done.
CarlosAlbertoEnciso added a comment.
Address review comments from @probinson
https://reviews.llvm.org/D46190
Files:
include/clang/Sema/Lookup.h
include/clang/Sema/Sema.h
include/clang/Sema/SemaInternal.h
lib/Sema/SemaCXXScopeSpec.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprCXX.cpp
lib/Sema/SemaLookup.cpp
lib/Sema/SemaTemplate.cpp
test/PCH/cxx-templates.cpp
test/SemaCXX/referenced_alias_declaration_1.cpp
test/SemaCXX/referenced_alias_declaration_2.cpp
test/SemaCXX/referenced_using_all.cpp
test/SemaCXX/referenced_using_declaration_1.cpp
test/SemaCXX/referenced_using_declaration_2.cpp
test/SemaCXX/referenced_using_directive.cpp
Index: test/SemaCXX/referenced_using_directive.cpp
===
--- test/SemaCXX/referenced_using_directive.cpp
+++ test/SemaCXX/referenced_using_directive.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
+
+namespace N {
+ typedef int Integer;
+ int var;
+}
+
+void Fa() {
+ using namespace N; // Referenced
+ var = 1;
+}
+
+void Fb() {
+ using namespace N;
+ N::var = 1;
+}
+
+void Fc() {
+ using namespace N; // Referenced
+ Integer var = 1;
+}
+
+void Fd() {
+ using namespace N;
+ N::Integer var = 1;
+}
+
+//CHECK: |-FunctionDecl {{.*}} Fa 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDirectiveDecl {{.*}} referenced Namespace {{.*}} 'N'
+//CHECK-NEXT: | `-BinaryOperator {{.*}} 'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'var' 'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: |-FunctionDecl {{.*}} Fb 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDirectiveDecl {{.*}} Namespace {{.*}} 'N'
+//CHECK-NEXT: | `-BinaryOperator {{.*}} 'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'var' 'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: |-FunctionDecl {{.*}} Fc 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDirectiveDecl {{.*}} referenced Namespace {{.*}} 'N'
+//CHECK-NEXT: | `-DeclStmt {{.*}}
+//CHECK-NEXT: | `-VarDecl {{.*}} var 'N::Integer':'int' cinit
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: `-FunctionDecl {{.*}} Fd 'void ()'
+//CHECK-NEXT: `-CompoundStmt {{.*}}
+//CHECK-NEXT: |-DeclStmt {{.*}}
+//CHECK-NEXT: | `-UsingDirectiveDecl {{.*}} Namespace {{.*}} 'N'
+//CHECK-NEXT: `-DeclStmt {{.*}}
+//CHECK-NEXT: `-VarDecl {{.*}} var 'N::Integer':'int' cinit
+//CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
Index: test/SemaCXX/referenced_using_declaration_2.cpp
===
--- test/SemaCXX/referenced_using_declaration_2.cpp
+++ test/SemaCXX/referenced_using_declaration_2.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
+
+namespace N {
+ typedef int Integer;
+ typedef char Char;
+}
+
+using N::Integer;
+using N::Char; // Referenced
+
+void Foo(int p1, N::Integer p2, Char p3) {
+ N::Integer var;
+ var = 0;
+}
+
+using N::Integer; // Referenced
+Integer Bar() {
+ using N::Char;
+ return 0;
+}
+
+//CHECK: |-UsingDecl {{.*}} N::Integer
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Integer' sugar
+//CHECK-NEXT: | |-Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-BuiltinType {{.*}} 'int'
+//CHECK-NEXT: |-UsingDecl {{.*}} referenced N::Char
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit referenced Typedef {{.*}} 'Char'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Char' sugar
+//CHECK-NEXT: | |-Typedef {{.*}} 'Char'
+//CHECK-NEXT: | `-BuiltinType {{.*}} 'char'
+//CHECK-NEXT: |-FunctionDecl {{.*}} Foo 'void (int, N::Integer, N::Char)'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p1 'int'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p2 'N::Integer':'int'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p3 'N::Char':'char'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-VarDecl {{.*}} used var 'N::Integer':'int'
+//CHECK-NEXT: | `-BinaryOperator {{.*}} 'N::Integer':'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'N::Integer':'int' lvalue Var {{.*}} 'var' 'N::Integer':'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 0
+//CHECK-NEXT: |-UsingDecl {{.*}} referenced N::Integer
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit referenced Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Integer' sugar
+//CHECK-NEXT: | |-Typedef {{.*}} 'Integer'
+//CHECK-NEXT:
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
probinson added a comment.
A bunch of style comments. Maybe try clang-format-diff.
Comment at: include/clang/Sema/SemaInternal.h:91
Var->markUsed(SemaRef.Context);
+ SemaRef.MarkUsingReferenced(Var, Loc, /*CXXScopeSpec*=*/nullptr, RefExpr);
}
The comments on a nullptr parameter usually use the formal parameter name, not
its type.
Comment at: lib/Sema/SemaDeclCXX.cpp:15549
+ R.setAddUsingDirectives();
+ LookupName(R,getCurScope());
+
Space after the comma.
Comment at: lib/Sema/SemaDeclCXX.cpp:15554
+
+ for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I) {
+NamedDecl *D = (*I)->getUnderlyingDecl();
Could this be a range-based for loop?
Comment at: lib/Sema/SemaDeclCXX.cpp:15566
+// Check if the declaration was introduced by a 'using-directive'.
+if (auto *UDir = dyn_cast(D))
+ if (UDir->getNominatedNamespace() == Target) {
else if
Comment at: lib/Sema/SemaExpr.cpp:14460
Func->markUsed(Context);
+ MarkUsingReferenced(Func, Loc, /*CXXScopeSpec*=*/nullptr, E);
}
Parameter comments usually use the formal parameter name, not its type.
Comment at: lib/Sema/SemaExpr.cpp:15374
+ SemaRef.MarkAnyDeclReferenced(Loc, D, MightBeOdrUse,
+/*CXXScopeSpec*=*/nullptr, E);
Parameter comments usually use the formal parameter name, not its type.
Comment at: lib/Sema/SemaExpr.cpp:15396
+SemaRef.MarkAnyDeclReferenced(Loc, DM, MightBeOdrUse,
+ /*CXXScopeSpec*=*/nullptr, E);
}
Parameter comments usually use the formal parameter name, not its type.
Comment at: lib/Sema/SemaLookup.cpp:196
+void addUsingDirective(LookupResult &R) {
+ for (auto I = usings.begin(), E = usings.end(); I != E; ++I)
+R.addDecl((*I));
Can this be a range-based for loop?
Comment at: lib/Sema/SemaLookup.cpp:1064
+static void
+AddUsingDirectives(LookupResult &R,UnqualUsingDirectiveSet &UDirs) {
+ if (R.isAddUsingDirectives())
Space after the comma.
`UDirs` has a different meaning elsewhere in this file, maybe `UsingDirs`
instead?
Comment at: lib/Sema/SemaLookup.cpp:1236
UDirs.done();
+AddUsingDirectives(R,UDirs);
Space after the comma.
Comment at: lib/Sema/SemaLookup.cpp:1277
UDirs.done();
+AddUsingDirectives(R,UDirs);
}
Space after the comma.
https://reviews.llvm.org/D46190
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso added a comment. @rsmith and @probinson: Is there anything I can add to this patch? The review https://reviews.llvm.org/D44826 is already approved and it is blocked by this patch being reviewed. Thanks very much https://reviews.llvm.org/D46190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso added a comment. @rsmith Is there anything I can add to this patch? The review https://reviews.llvm.org/D44826 is already approved and it is dependent on this patch being reviewed. Thanks very much https://reviews.llvm.org/D46190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso added inline comments.
Comment at: lib/Sema/Sema.cpp:1884-1885
+ ND = NA->getAliasedNamespace();
+ if (auto *NNS = NA->getQualifier())
+MarkNamespaceAliasReferenced(NNS->getAsNamespaceAlias());
+}
rsmith wrote:
> The loop and recursion here -- and indeed this whole function -- seem
> unnecessary.
>
> ```
> namespace A {} // #1
> namespace X {
> namespace B = A; // #2
> }
> namespace Y = X; // #3
> namespace C = Y::B; // #4
> ```
>
> Declaration `#4` should mark `#2` and `#3` referenced. So the only
> 'unreferenced namespace alias' warning we should get here is for `#4` -- and
> there is no need for marking `#4` as referenced to affect `#2` or `#3`.
The function and recursion is to be able to handle cases like:
```
namespace N {
int var;
}
void Foo() {
namespace NA = N;
namespace NB = NA;
NB::var = 4;
}
```
'var' is referenced and N, NA and NB should be marked as 'referenced' as well.
Comment at: lib/Sema/Sema.cpp:1890-1891
+
+/// \brief Mark as referenced any 'using declaration' that have introduced
+/// the given declaration in the current context.
+void Sema::MarkUsingReferenced(Decl *D, CXXScopeSpec *SS, Expr *E) {
rsmith wrote:
> Why is this ever appropriate? I would think we should just mark the named
> declaration from the relevant lookup as referenced in all cases.
My intention is to detect cases where the using statements do not affect other
declarations.
```
namespace N {
int var;
}
void Foo() {
using N::var;<- No Referenced
N::var = 1; <- Used
}
```
https://reviews.llvm.org/D46190
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso updated this revision to Diff 154091.
CarlosAlbertoEnciso added a comment.
Update the patch to address the comments from the reviewers.
https://reviews.llvm.org/D46190
Files:
include/clang/Sema/Lookup.h
include/clang/Sema/Sema.h
include/clang/Sema/SemaInternal.h
lib/Sema/SemaCXXScopeSpec.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprCXX.cpp
lib/Sema/SemaLookup.cpp
lib/Sema/SemaTemplate.cpp
test/PCH/cxx-templates.cpp
test/SemaCXX/referenced_alias_declaration_1.cpp
test/SemaCXX/referenced_alias_declaration_2.cpp
test/SemaCXX/referenced_using_all.cpp
test/SemaCXX/referenced_using_declaration_1.cpp
test/SemaCXX/referenced_using_declaration_2.cpp
test/SemaCXX/referenced_using_directive.cpp
Index: test/SemaCXX/referenced_using_directive.cpp
===
--- test/SemaCXX/referenced_using_directive.cpp
+++ test/SemaCXX/referenced_using_directive.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
+
+namespace N {
+ typedef int Integer;
+ int var;
+}
+
+void Fa() {
+ using namespace N; // Referenced
+ var = 1;
+}
+
+void Fb() {
+ using namespace N;
+ N::var = 1;
+}
+
+void Fc() {
+ using namespace N; // Referenced
+ Integer var = 1;
+}
+
+void Fd() {
+ using namespace N;
+ N::Integer var = 1;
+}
+
+//CHECK: |-FunctionDecl {{.*}} Fa 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDirectiveDecl {{.*}} referenced Namespace {{.*}} 'N'
+//CHECK-NEXT: | `-BinaryOperator {{.*}} 'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'var' 'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: |-FunctionDecl {{.*}} Fb 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDirectiveDecl {{.*}} Namespace {{.*}} 'N'
+//CHECK-NEXT: | `-BinaryOperator {{.*}} 'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'var' 'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: |-FunctionDecl {{.*}} Fc 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDirectiveDecl {{.*}} referenced Namespace {{.*}} 'N'
+//CHECK-NEXT: | `-DeclStmt {{.*}}
+//CHECK-NEXT: | `-VarDecl {{.*}} var 'N::Integer':'int' cinit
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: `-FunctionDecl {{.*}} Fd 'void ()'
+//CHECK-NEXT: `-CompoundStmt {{.*}}
+//CHECK-NEXT: |-DeclStmt {{.*}}
+//CHECK-NEXT: | `-UsingDirectiveDecl {{.*}} Namespace {{.*}} 'N'
+//CHECK-NEXT: `-DeclStmt {{.*}}
+//CHECK-NEXT: `-VarDecl {{.*}} var 'N::Integer':'int' cinit
+//CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
Index: test/SemaCXX/referenced_using_declaration_2.cpp
===
--- test/SemaCXX/referenced_using_declaration_2.cpp
+++ test/SemaCXX/referenced_using_declaration_2.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
+
+namespace N {
+ typedef int Integer;
+ typedef char Char;
+}
+
+using N::Integer;
+using N::Char; // Referenced
+
+void Foo(int p1, N::Integer p2, Char p3) {
+ N::Integer var;
+ var = 0;
+}
+
+using N::Integer; // Referenced
+Integer Bar() {
+ using N::Char;
+ return 0;
+}
+
+//CHECK: |-UsingDecl {{.*}} N::Integer
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Integer' sugar
+//CHECK-NEXT: | |-Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-BuiltinType {{.*}} 'int'
+//CHECK-NEXT: |-UsingDecl {{.*}} referenced N::Char
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit referenced Typedef {{.*}} 'Char'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Char' sugar
+//CHECK-NEXT: | |-Typedef {{.*}} 'Char'
+//CHECK-NEXT: | `-BuiltinType {{.*}} 'char'
+//CHECK-NEXT: |-FunctionDecl {{.*}} Foo 'void (int, N::Integer, N::Char)'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p1 'int'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p2 'N::Integer':'int'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p3 'N::Char':'char'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-VarDecl {{.*}} used var 'N::Integer':'int'
+//CHECK-NEXT: | `-BinaryOperator {{.*}} 'N::Integer':'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'N::Integer':'int' lvalue Var {{.*}} 'var' 'N::Integer':'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 0
+//CHECK-NEXT: |-UsingDecl {{.*}} referenced N::Integer
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit referenced Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Integer' sugar
+//CHECK-NEXT: | |-Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-BuiltinType {{.*}} 'int'
+//
