[PATCH] D46190: For a referenced declaration, mark any associated usings as referenced.

2018-05-16 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso updated this revision to Diff 147050.
CarlosAlbertoEnciso edited the summary of this revision.
CarlosAlbertoEnciso added a comment.

Address the issues raised by the reviewer (rsmith).


https://reviews.llvm.org/D46190

Files:
  include/clang/Sema/Sema.h
  lib/Sema/Sema.cpp
  lib/Sema/SemaCXXScopeSpec.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.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'
+//CHECK-NEXT: `-FunctionDe

[PATCH] D46190: For a referenced declaration, mark any associated usings as referenced.

2018-05-23 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Ping.

Thanks


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] D44826: Add -Wunused-using, a warning that finds unused using declarations.

2018-05-25 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso updated this revision to Diff 148578.
CarlosAlbertoEnciso edited the summary of this revision.
CarlosAlbertoEnciso added a comment.

This patch addresses the reviewers comments:

1. Merge the -Wunused-local-typedefs and -Wunused-usings implementations
2. Update the Release Notes
3. Review the test cases


https://reviews.llvm.org/D44826

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/ExternalSemaSource.h
  include/clang/Sema/MultiplexExternalSemaSource.h
  include/clang/Sema/Sema.h
  include/clang/Sema/SemaInternal.h
  include/clang/Serialization/ASTBitCodes.h
  include/clang/Serialization/ASTReader.h
  lib/Sema/MultiplexExternalSemaSource.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/FixIt/fixit.cpp
  test/Modules/Inputs/module.map
  test/Modules/Inputs/warn-unused-using.h
  test/Modules/warn-unused-using.cpp
  test/PCH/cxx-templates.cpp
  test/SemaCXX/coreturn.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,26 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+  typedef int Integer;
+  int var;
+}
+
+void Fa() {
+  using namespace N;  // Referenced
+  var = 1;
+}
+
+void Fb() {
+  using namespace N;  // expected-warning {{unused using ''}}
+  N::var = 1;
+}
+
+void Fc() {
+  using namespace N;  // Referenced
+  Integer var = 1;
+}
+
+void Fd() {
+  using namespace N;  // expected-warning {{unused using ''}}
+  N::Integer var = 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,20 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+  typedef int Integer;
+  typedef char Char;
+}
+
+using N::Integer;   // expected-warning {{unused using '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;// expected-warning {{unused using 'Char'}}
+  return 0;
+}
Index: test/SemaCXX/referenced_using_declaration_1.cpp
===
--- test/SemaCXX/referenced_using_declaration_1.cpp
+++ test/SemaCXX/referenced_using_declaration_1.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+  // Types.
+  typedef int Integer;
+  struct Record {
+int a;
+  };
+
+  // Variables.
+  int var1;
+  int var2;
+
+  // Functions.
+  void func1();
+  void func2();
+}
+
+using N::Integer;   // expected-warning {{unused using 'Integer'}}
+using N::Record;// expected-warning {{unused using 'Record'}}
+using N::var1;  // expected-warning {{unused using 'var1'}}
+using N::var2;  // expected-warning {{unused using 'var2'}}
+using N::func1; // expected-warning {{unused using 'func1'}}
+using N::func2; // expected-warning {{unused using 'func2'}}
+
+void Foo() {
+  using N::Integer; // expected-warning {{unused using 'Integer'}}
+  N::Integer int_var;
+  int_var = 1;
+
+  using N::Record;  // Referenced
+  Record rec_var;
+  rec_var.a = 2;
+
+  using N::var1;// expected-warning {{unused using 'var1'}}
+  N::var1 = 3;
+
+  using N::var2;// Referenced
+  var2 = 4;
+
+  using N::func1;   // expected-warning {{unused using 'func1'}}
+  N::func1();
+
+  using N::func2;   // Referenced
+  func2();
+}
Index: test/SemaCXX/referenced_using_all.cpp
===
--- test/SemaCXX/referenced_using_all.cpp
+++ test/SemaCXX/referenced_using_all.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace A {
+  typedef char Char;
+  typedef int Integer;
+  typedef float Float;
+  int var;
+}
+
+using A::Char;  // expected-warning {{unused using 'Char'}}
+using A::Integer;   // Referenced
+using A::Float; // expected-warning {{unused using 'Float'}}
+
+namespace B {
+  using A::Char;// Referenced
+  template 
+  T FuncTempl(T p1,Char p2) {
+using A::Float; // Referenced
+typedef Float Type;
+Integer I;
+return p1;
+  }
+}
+
+using A::Char;

[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

2018-05-25 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso marked 2 inline comments as done.
CarlosAlbertoEnciso added inline comments.



Comment at: lib/Serialization/ASTReader.cpp:8101-8103
+UsingDecl *D = dyn_cast_or_null(
+GetDecl(UnusedUsingCandidates[I]));
+if (D)

dblaikie wrote:
> roll the declaration into the condition, perhaps:
> 
>   if (auto *D = dyn_cast_or_null...)
> Decls.insert(D);
Done.



Comment at: test/Modules/warn-unused-using.cpp:5
+
+// For modules, the warning should only fire the first time, when the module is
+// built.

dblaikie wrote:
> This would only warn for the unused local using, but wouldn't fire for an 
> namespace-scoped unused using? Perhaps there should be a test for that?
I have added a namespace-scoped unused using.


https://reviews.llvm.org/D44826



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

2018-05-26 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso marked 2 inline comments as done.
CarlosAlbertoEnciso added a comment.

@lebedev.ri,

Thanks very much for your review. I will address those issues and update the 
patch.


https://reviews.llvm.org/D44826



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

2018-05-29 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added inline comments.



Comment at: include/clang/Basic/DiagnosticGroups.td:828-829
 // -Wunused-local-typedefs = -Wunused-local-typedef
+def : DiagGroup<"unused-usings", [UnusedUsing]>;
+// -Wunused-usings = -Wunused-using
 

lebedev.ri wrote:
> Why? gcc compatibility?
No particular reason. I just follow the 'unused-local-typedefs' model.
If there is not objection from others reviewers, I will drop the gcc 
compatibility.


https://reviews.llvm.org/D44826



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

2017-10-24 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso created this revision.

An unscoped enumeration used as template argument, should not
have any qualified information about its enclosing scope, as
its visibility is global.

In the case of scoped enumerations, they must include information
about their enclosing scope.

For the below test:

  enum unscoped { a };
  enum class scoped { b };
  
  template struct tmpl_u { };
  template struct tmpl_s { };
  
  tmpl_u tmpl_unscoped;
  tmpl_s tmpl_scoped;

gives the following encoded names:

  DW_TAG_structure_type "tmpl_u"
  DW_TAG_structure_type "tmpl_s"

The incorrectly qualified name causes a debugger to show qualified
names for both scoped and unscoped enumerations.


https://reviews.llvm.org/D39239

Files:
  lib/AST/Decl.cpp
  test/Modules/odr.cpp
  test/SemaCXX/return-noreturn.cpp
  test/SemaTemplate/temp_arg_enum_printing.cpp
  test/SemaTemplate/temp_arg_enum_printing_more.cpp
  unittests/AST/NamedDeclPrinterTest.cpp

Index: unittests/AST/NamedDeclPrinterTest.cpp
===
--- unittests/AST/NamedDeclPrinterTest.cpp
+++ unittests/AST/NamedDeclPrinterTest.cpp
@@ -143,7 +143,7 @@
   ASSERT_TRUE(PrintedWrittenNamedDeclCXX11Matches(
 "enum X { A };",
 "A",
-"X::A"));
+"A"));
 }
 
 TEST(NamedDeclPrinter, TestScopedNamedEnum) {
@@ -164,7 +164,7 @@
   ASSERT_TRUE(PrintedWrittenNamedDeclCXX11Matches(
 "class X { enum Y { A }; };",
 "A",
-"X::Y::A"));
+"X::A"));
 }
 
 TEST(NamedDeclPrinter, TestClassWithScopedNamedEnum) {
Index: test/SemaTemplate/temp_arg_enum_printing_more.cpp
===
--- test/SemaTemplate/temp_arg_enum_printing_more.cpp
+++ test/SemaTemplate/temp_arg_enum_printing_more.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -ast-print %s -std=c++11 | FileCheck %s
+
+// Make sure that for template value arguments that are unscoped enumerators,
+// no qualified enum information is included in their name, as their visibility
+// is global. In the case of scoped enumerators, they must include information
+// about their enum enclosing scope.
+
+enum E1 { e1 };
+template struct tmpl_1 {};
+// CHECK: template<> struct tmpl_1
+tmpl_1 TMPL_1;  // Name must be 'e1'.
+
+namespace nsp_1 { enum E2 { e2 }; }
+template struct tmpl_2 {};
+// CHECK: template<> struct tmpl_2
+tmpl_2 TMPL_2;   // Name must be 'nsp_1::e2'.
+
+enum class E3 { e3 };
+template struct tmpl_3 {};
+// CHECK: template<> struct tmpl_3
+tmpl_3 TMPL_3;  // Name must be 'E3::e3'.
+
+namespace nsp_2 { enum class E4 { e4 }; }
+template struct tmpl_4 {};
+// CHECK: template<> struct tmpl_4
+tmpl_4 TMPL_4;   // Name must be 'nsp_2::E4::e4'.
Index: test/SemaTemplate/temp_arg_enum_printing.cpp
===
--- test/SemaTemplate/temp_arg_enum_printing.cpp
+++ test/SemaTemplate/temp_arg_enum_printing.cpp
@@ -13,9 +13,9 @@
 void foo();
   
 void test() {
-  // CHECK: template<> void foo()
+  // CHECK: template<> void foo()
   NamedEnumNS::foo();
-  // CHECK: template<> void foo()
+  // CHECK: template<> void foo()
   NamedEnumNS::foo<(NamedEnum)1>();
   // CHECK: template<> void foo<2>()
   NamedEnumNS::foo<(NamedEnum)2>();
Index: test/SemaCXX/return-noreturn.cpp
===
--- test/SemaCXX/return-noreturn.cpp
+++ test/SemaCXX/return-noreturn.cpp
@@ -143,7 +143,7 @@
 } // expected-warning {{control reaches end of non-void function}}
 
 void PR9412_f() {
-PR9412_t(); // expected-note {{in instantiation of function template specialization 'PR9412_t' requested here}}
+PR9412_t(); // expected-note {{in instantiation of function template specialization 'PR9412_t' requested here}}
 }
 
 struct NoReturn {
Index: test/Modules/odr.cpp
===
--- test/Modules/odr.cpp
+++ test/Modules/odr.cpp
@@ -18,6 +18,6 @@
 // expected-note@a.h:3 {{declaration of 'f' does not match}}
 // expected-note@a.h:1 {{definition has no member 'm'}}
 
-// expected-error@b.h:5 {{'E::e2' from module 'b' is not present in definition of 'E' in module 'a'}}
+// expected-error@b.h:5 {{'e2' from module 'b' is not present in definition of 'E' in module 'a'}}
 // expected-error@b.h:3 {{'Y::f' from module 'b' is not present in definition of 'Y' in module 'a'}}
 // expected-error@b.h:2 {{'Y::m' from module 'b' is not present in definition of 'Y' in module 'a'}}
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -1514,7 +1514,10 @@
   // enumerator is declared in the scope that immediately contains
   // the enum-specifier. Each scoped enumerator is declared in the
   // scope of the enumeration.
-  if (ED->isScoped() || ED->getIdentifier())
+  // For the case of unscoped enumerator, do

[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

2017-10-24 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso updated this revision to Diff 120066.

https://reviews.llvm.org/D39239

Files:
  lib/AST/Decl.cpp
  test/Modules/odr.cpp
  test/SemaCXX/return-noreturn.cpp
  test/SemaTemplate/temp_arg_enum_printing.cpp
  test/SemaTemplate/temp_arg_enum_printing_more.cpp
  unittests/AST/NamedDeclPrinterTest.cpp

Index: unittests/AST/NamedDeclPrinterTest.cpp
===
--- unittests/AST/NamedDeclPrinterTest.cpp
+++ unittests/AST/NamedDeclPrinterTest.cpp
@@ -143,7 +143,7 @@
   ASSERT_TRUE(PrintedWrittenNamedDeclCXX11Matches(
 "enum X { A };",
 "A",
-"X::A"));
+"A"));
 }
 
 TEST(NamedDeclPrinter, TestScopedNamedEnum) {
@@ -164,7 +164,7 @@
   ASSERT_TRUE(PrintedWrittenNamedDeclCXX11Matches(
 "class X { enum Y { A }; };",
 "A",
-"X::Y::A"));
+"X::A"));
 }
 
 TEST(NamedDeclPrinter, TestClassWithScopedNamedEnum) {
Index: test/SemaTemplate/temp_arg_enum_printing_more.cpp
===
--- test/SemaTemplate/temp_arg_enum_printing_more.cpp
+++ test/SemaTemplate/temp_arg_enum_printing_more.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -ast-print %s -std=c++11 | FileCheck %s
+
+// Make sure that for template value arguments that are unscoped enumerators,
+// no qualified enum information is included in their name, as their visibility
+// is global. In the case of scoped enumerators, they must include information
+// about their enum enclosing scope.
+
+enum E1 { e1 };
+template struct tmpl_1 {};
+// CHECK: template<> struct tmpl_1
+tmpl_1 TMPL_1;  // Name must be 'e1'.
+
+namespace nsp_1 { enum E2 { e2 }; }
+template struct tmpl_2 {};
+// CHECK: template<> struct tmpl_2
+tmpl_2 TMPL_2;   // Name must be 'nsp_1::e2'.
+
+enum class E3 { e3 };
+template struct tmpl_3 {};
+// CHECK: template<> struct tmpl_3
+tmpl_3 TMPL_3;  // Name must be 'E3::e3'.
+
+namespace nsp_2 { enum class E4 { e4 }; }
+template struct tmpl_4 {};
+// CHECK: template<> struct tmpl_4
+tmpl_4 TMPL_4;   // Name must be 'nsp_2::E4::e4'.
Index: test/SemaTemplate/temp_arg_enum_printing.cpp
===
--- test/SemaTemplate/temp_arg_enum_printing.cpp
+++ test/SemaTemplate/temp_arg_enum_printing.cpp
@@ -13,9 +13,9 @@
 void foo();
   
 void test() {
-  // CHECK: template<> void foo()
+  // CHECK: template<> void foo()
   NamedEnumNS::foo();
-  // CHECK: template<> void foo()
+  // CHECK: template<> void foo()
   NamedEnumNS::foo<(NamedEnum)1>();
   // CHECK: template<> void foo<2>()
   NamedEnumNS::foo<(NamedEnum)2>();
Index: test/SemaCXX/return-noreturn.cpp
===
--- test/SemaCXX/return-noreturn.cpp
+++ test/SemaCXX/return-noreturn.cpp
@@ -143,7 +143,7 @@
 } // expected-warning {{control reaches end of non-void function}}
 
 void PR9412_f() {
-PR9412_t(); // expected-note {{in instantiation of function template specialization 'PR9412_t' requested here}}
+PR9412_t(); // expected-note {{in instantiation of function template specialization 'PR9412_t' requested here}}
 }
 
 struct NoReturn {
Index: test/Modules/odr.cpp
===
--- test/Modules/odr.cpp
+++ test/Modules/odr.cpp
@@ -18,6 +18,6 @@
 // expected-note@a.h:3 {{declaration of 'f' does not match}}
 // expected-note@a.h:1 {{definition has no member 'm'}}
 
-// expected-error@b.h:5 {{'E::e2' from module 'b' is not present in definition of 'E' in module 'a'}}
+// expected-error@b.h:5 {{'e2' from module 'b' is not present in definition of 'E' in module 'a'}}
 // expected-error@b.h:3 {{'Y::f' from module 'b' is not present in definition of 'Y' in module 'a'}}
 // expected-error@b.h:2 {{'Y::m' from module 'b' is not present in definition of 'Y' in module 'a'}}
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -1514,7 +1514,10 @@
   // enumerator is declared in the scope that immediately contains
   // the enum-specifier. Each scoped enumerator is declared in the
   // scope of the enumeration.
-  if (ED->isScoped() || ED->getIdentifier())
+  // For the case of unscoped enumerator, do not include in the qualified
+  // name any information about its enum enclosing scope, as is visibility
+  // is global.
+  if (ED->isScoped())
 OS << *ED;
   else
 continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

2017-10-25 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D39239#905641, @probinson wrote:

> Have you tried this change against the GDB and LLDB test suites?  If they 
> have failures then we should think about whether those tests are 
> over-specific or whether we should put this under a tuning option.


LLDB test suite:
There are some specific tests that use enums and templates:

tools/lldb/packages/Python/lldbsuite/test/lang/cpp/template

but they cover only the case of scoped enums and are not affected by this 
patch. May be the LLDB test suite should be updated to cover scoped and 
unscoped enums.

I will check the GDB test suite and include the results.


https://reviews.llvm.org/D39239



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

2017-10-25 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D39239#905641, @probinson wrote:

> Have you tried this change against the GDB and LLDB test suites?  If they 
> have failures then we should think about whether those tests are 
> over-specific or whether we should put this under a tuning option.


I double check the LLDB test suite and there are 3 cases that fail with this 
patch. But the DWARF generated is correct, as the template name is encoded 
correctly:

  DW_TAG_compile_unit "main.cpp"
DW_AT_producer "clang version 6.0.0 (trunk 316571)"
DW_AT_comp_dir 
"/home/carlos/llvm-root/llvm/tools/lldb/packages/Python/lldbsuite/test/lang/cpp/template"
...
DW_TAG_enumeration_type "EnumType"
  DW_AT_enum_class DW_FORM_flag_present
  DW_TAG_enumerator "Member"
  DW_TAG_enumerator "Subclass"
  ...
DW_TAG_class_type "EnumTemplate"
  ...
DW_TAG_class_type "EnumTemplate"
  ...

I will investigate the issue, as the test case should pass as it does not use 
unscope enums, which is what the patch should affect.


https://reviews.llvm.org/D39239



___
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.

2018-07-19 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
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.

2018-07-20 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
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.

2018-07-20 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
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.

2018-07-26 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
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] D41698: [DebugInfo] Enable debug information for C99 VLA types

2018-02-09 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Hi @sdesmalen!

First of all my apologies for commenting after the issue has been closed, but I 
do not have an account to add a comment to the associated bugzilla.

I have found what it seems to be an issue with the current implementation.

For the given test case

  int main() {
int size = 2;
  
int vla_expr[size];
vla_expr[1] = 1;
  
return 0;
  }

and while debugging with LLDB, the following error is generated:

  (lldb) n
  Process 21014 stopped
  * thread #1, name = 'bad.out', stop reason = step over
  frame #0: 0x00400502 bad.out`main at vla_2.cpp:7
 4int vla_expr[size];
 5vla_expr[1] = 1;
 6  
  -> 7return 0;
 8  }
  
  (lldb) p vla_expr
  (unsigned long) $0 = 2
  
  (lldb) p vla_expr[1]
  error: subscripted value is not an array, pointer, or vector
  
  (lldb) 

Looking at the DWARF generated, there are 2 variables with the same name at the 
same scope

  DW_TAG_subprogram "main"
...
DW_TAG_variable "size"
DW_TAG_variable "vla_expr"
DW_TAG_variable "vla_expr"

I think there are 2 issues:

The compiler generated variable 'vla_expr'

- should be flagged as artificial (DW_AT_artificial)
- its name should start with double underscore to avoid conflicting with 
user-defined names.

Thanks,
Carlos


Repository:
  rL LLVM

https://reviews.llvm.org/D41698



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D41698: [DebugInfo] Enable debug information for C99 VLA types

2018-02-09 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Hi @sdesmalen!

For the following test case

  int main() {
int size = 2;
  
int var[size];
var[1] = 1;
  
return 0;
  }

I compared the DWARF generated by GCC and it looks like

  DW_TAG_variable "var"
DW_AT_location ...
DW_AT_type DW_FORM_ref4
  DW_TAG_array_type 
DW_AT_type -> "int"
DW_TAG_subrange_type 
  DW_AT_type -> "sizetype"
  DW_AT_upper_bound DW_FORM_exprloc [4] = { DW_OP_fbreg 0xffb8 
DW_OP_deref }

GCC use DW_AT_upper_bound with an associated location expression to describe 
the VLA boundaries.

In order to reduce the side effects created by the artifical-variable as 
described in my previous comment and to keep the generated DWARF within a 
reasonable size, I would suggest the GCC aproach as a size optimization.

The DWARF description of the artificial-variable could be removed and its 
location expression used by the array's subrange_type, instead of the 
subrange_type making a reference to the artificial-variable.


Repository:
  rL LLVM

https://reviews.llvm.org/D41698



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43189: [DebugInfo] Avoid name conflict of generated VLA expression variable.

2018-02-12 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

LGTM.

Thanks.


https://reviews.llvm.org/D43189



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43272: [clangd] Fix tracing now that spans lifetimes can overlap on a thread.

2018-02-15 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Hi,

It seems that your submission have broken the Windows and Linux Builds for 
'clangd'.

This is the error message:

/home/xxx/llvm-root/llvm/tools/clang/tools/extra/clangd/Trace.cpp:54:76: error: 
call of overloaded ‘make_unique(clang::clangd::trace::{anonymous}::JSONTracer*, 
llvm::StringRef&, clang::clangd::json::obj*&)’ is ambiguous

  make_unique(this, Name, Args));
^

Thanks,


Repository:
  rL LLVM

https://reviews.llvm.org/D43272



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43272: [clangd] Fix tracing now that spans lifetimes can overlap on a thread.

2018-02-15 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D43272#1008703, @sammccall wrote:

> r325239 should fix, sorry!


Build now is OK. Thanks @sammccall.


Repository:
  rL LLVM

https://reviews.llvm.org/D43272



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-15 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Hi,

It seems that your submission broke the Windows/Linux builds for 'clangd'.

The generated error message is:

  C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\future(263): 
error C2512: 
'llvm::Expected>>':
 no appropriate default constructor available 
[C:\j\w\opensource\opensource_to_sce_merge\build\tools\clang\tools\extra\unittests\clangd\ClangdTests.vcxproj]
  
  C:\xxx\llvm\tools\clang\tools\extra\clangd\ClangdServer.h(231): note: see 
declaration of 
'llvm::Expected>>'
  C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\future(255): 
note: while compiling class template member function 
'std::_Associated_state<_Ty>::_Associated_state(std::_Deleter_base<_Ty> *)'
  C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\future(1491): 
note: see reference to function template instantiation 
'std::_Associated_state<_Ty>::_Associated_state(std::_Deleter_base<_Ty> *)' 
being compiled
  C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\future(989): 
note: see reference to class template instantiation 
'std::_Associated_state<_Ty>' being compiled
  C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\future(987): 
note: while compiling class template member function 'bool 
std::_State_manager<_Ty>::valid(void) throw() const'

Thanks


Repository:
  rL LLVM

https://reviews.llvm.org/D43227



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based

2018-02-16 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Build now is OK. Thanks @ilya-biryukov


Repository:
  rL LLVM

https://reviews.llvm.org/D43227



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

2018-02-16 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Hi @ioeric:

Just to let you know that your submission seems to break a Windows test.

  FAIL: Extra Tools Unit Tests :: 
clangd/Checking/./ClangdTests.exe/SymbolCollectorTest.IWYUPragma (15474 of 
36599)

Thanks


Repository:
  rL LLVM

https://reviews.llvm.org/D42640



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

2018-02-19 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a subscriber: aheejin.
CarlosAlbertoEnciso added a comment.

Thanks @aheejin


Repository:
  rL LLVM

https://reviews.llvm.org/D42640



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

2017-12-20 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D39239#912837, @probinson wrote:

> Have you tried the GDB suite yet?  If it has no problems, and we can make 
> LLDB happy, I'm okay with it.


Hi Paul,

Following the instructions from David Blaikie on how to build/run the GDB suite:

"Directions for how to run it would be divined from the buildbot configuration.

https://github.com/llvm-mirror/zorg/blob/master/zorg/buildbot/builders/ClangBuilder.py#L883";

1. I have managed to build the GDB suite using clang as the default compiler.
2. Run the test before and after my changes and the results are the same.



=== gdb Summary ===
  
  # of expected passes  20501
  # of unexpected failures  1481
  # of expected failures262
  # of unknown successes2
  # of known failures   78
  # of unresolved testcases 9
  # of untested testcases   48
  # of unsupported tests36

The unexpected failures falls into the categories:

- Invalid option '-w' to the ADA compiler
- Timeout issues
- Threading and attaching issues

Possible incorrect settings for the GDB suite.


https://reviews.llvm.org/D39239



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

2017-12-20 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D39239#912837, @probinson wrote:

> Have you tried the GDB suite yet?  If it has no problems, and we can make 
> LLDB happy, I'm okay with it.


These are the results when using GCC as the default compiler:

=== gdb Summary ===
  
  # of expected passes  21368
  # of unexpected failures  1788
  # of expected failures213
  # of unknown successes2
  # of known failures   75
  # of unresolved testcases 10
  # of untested testcases   29
  # of unsupported tests34

The unexpected failures falls into the categories (same as with clang):

- Invalid option '-w' to the ADA compiler
- Timeout issues
- Threading and attaching issues


https://reviews.llvm.org/D39239



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

2017-12-21 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D39239#961367, @probinson wrote:

> With the GDB test results and LLDB able to handle it, this LGTM.
>  Carlos, do you have commit access?


Hi Paul,

Thanks for the LGTM.

I do not have commit access.


https://reviews.llvm.org/D39239



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43821: [SemaCXX] _Pragma("clang optimize off") not affecting lambda.

2018-02-27 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso created this revision.
CarlosAlbertoEnciso added reviewers: rsmith, erichkeane, andreadb.
CarlosAlbertoEnciso added a project: clang.

Declaring "_Pragma("clang optimize off")" before the body of a
function with a lambda leads to the lambda functions in the body
not being affected.

For the below test:

  _Pragma("clang optimize off")
  
  void foo(int p) {
auto lambda = [&p]() { ++p; };
lambda();
  }
  
  _Pragma("clang optimize on")

At -O1 the attributes for the 'foo' function are:

  attributes #0 = { noinline nounwind optnone uwtable ... }

At -O1 the attributes for the lambda function are:

  attributes #1 = { inlinehint norecurse nounwind uwtable ... }

The incorrect attribute causes the lambda function to be considered
for inlining ignoring the given _Pragma.


Repository:
  rC Clang

https://reviews.llvm.org/D43821

Files:
  lib/Sema/SemaDecl.cpp
  test/CodeGenCXX/optnone-pragma-optimize-off.cpp


Index: test/CodeGenCXX/optnone-pragma-optimize-off.cpp
===
--- test/CodeGenCXX/optnone-pragma-optimize-off.cpp
+++ test/CodeGenCXX/optnone-pragma-optimize-off.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 %s -triple %itanium_abi_triple -O1 -disable-llvm-passes 
-emit-llvm -o - | FileCheck %s
+
+// Test the attributes for the lambda function contains 'optnone' as result of
+// the _Pragma("clang optimize off").
+
+_Pragma("clang optimize off")
+
+void foo(int p) {
+  auto lambda = [&p]() { ++p; };
+  lambda();
+  // CHECK: define {{.*}} @"_ZZ3fooiENK3$_0clEv"({{.*}}) #[[LAMBDA_ATR:[0-9]+]]
+}
+
+_Pragma("clang optimize on")
+
+// CHECK: attributes #[[LAMBDA_ATR]] = { {{.*}} optnone {{.*}} }
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -12593,6 +12593,10 @@
 FD->setBody(Body);
 FD->setWillHaveBody(false);
 
+// This represents the function body for the lambda function, check if we
+// have to apply optnone due to a pragma.
+AddRangeBasedOptnone(FD);
+
 if (getLangOpts().CPlusPlus14) {
   if (!FD->isInvalidDecl() && Body && !FD->isDependentContext() &&
   FD->getReturnType()->isUndeducedType()) {


Index: test/CodeGenCXX/optnone-pragma-optimize-off.cpp
===
--- test/CodeGenCXX/optnone-pragma-optimize-off.cpp
+++ test/CodeGenCXX/optnone-pragma-optimize-off.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 %s -triple %itanium_abi_triple -O1 -disable-llvm-passes -emit-llvm -o - | FileCheck %s
+
+// Test the attributes for the lambda function contains 'optnone' as result of
+// the _Pragma("clang optimize off").
+
+_Pragma("clang optimize off")
+
+void foo(int p) {
+  auto lambda = [&p]() { ++p; };
+  lambda();
+  // CHECK: define {{.*}} @"_ZZ3fooiENK3$_0clEv"({{.*}}) #[[LAMBDA_ATR:[0-9]+]]
+}
+
+_Pragma("clang optimize on")
+
+// CHECK: attributes #[[LAMBDA_ATR]] = { {{.*}} optnone {{.*}} }
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -12593,6 +12593,10 @@
 FD->setBody(Body);
 FD->setWillHaveBody(false);
 
+// This represents the function body for the lambda function, check if we
+// have to apply optnone due to a pragma.
+AddRangeBasedOptnone(FD);
+
 if (getLangOpts().CPlusPlus14) {
   if (!FD->isInvalidDecl() && Body && !FD->isDependentContext() &&
   FD->getReturnType()->isUndeducedType()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43821: [SemaCXX] _Pragma("clang optimize off") not affecting lambda.

2018-03-06 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Ping.

Thanks


Repository:
  rC Clang

https://reviews.llvm.org/D43821



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43821: [SemaCXX] _Pragma("clang optimize off") not affecting lambda.

2018-03-08 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:12598
+// have to apply optnone due to a pragma.
+AddRangeBasedOptnone(FD);
+

erichkeane wrote:
> Should thi sonly happen if this is a definition?  Additionally, does this 
> negate the need for the other call to this in ActOnFunctionDeclarator?
First of all, thanks for your review.

Based on your comments, I found couple of issues with my patch:

For a normal function, the setting of the 'optnone' attribute was done twice:
ActOnFunctionDeclarator and ActOnFinishFunctionBody.

The setting of the 'optnone' attribute is not consistent for both lambda and
normal functions. For normal functions being done at the start of the definition
and for lambdas at the end of the function body.

Based on the previous findings, now I am setting the 'optnone' attribute in
ActOnStartOfLambdaDefinition.

I will update the patch to reflect the changes.


Repository:
  rC Clang

https://reviews.llvm.org/D43821



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43821: [SemaCXX] _Pragma("clang optimize off") not affecting lambda.

2018-03-08 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso updated this revision to Diff 137550.
CarlosAlbertoEnciso added a comment.

Correct the issues raised by the reviews.


Repository:
  rC Clang

https://reviews.llvm.org/D43821

Files:
  lib/Sema/SemaLambda.cpp
  test/CodeGenCXX/optnone-pragma-optimize-off.cpp


Index: test/CodeGenCXX/optnone-pragma-optimize-off.cpp
===
--- test/CodeGenCXX/optnone-pragma-optimize-off.cpp
+++ test/CodeGenCXX/optnone-pragma-optimize-off.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 %s -triple %itanium_abi_triple -O1 -disable-llvm-passes 
-emit-llvm -o - | FileCheck %s
+
+// Test the attributes for the lambda function contains 'optnone' as result of
+// the _Pragma("clang optimize off").
+
+_Pragma("clang optimize off")
+
+void foo(int p) {
+  auto lambda = [&p]() { ++p; };
+  lambda();
+  // CHECK: define {{.*}} @"_ZZ3fooiENK3$_0clEv"({{.*}}) #[[LAMBDA_ATR:[0-9]+]]
+}
+
+_Pragma("clang optimize on")
+
+// CHECK: attributes #[[LAMBDA_ATR]] = { {{.*}} optnone {{.*}} }
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -904,6 +904,10 @@
 ParamInfo.getDeclSpec().isConstexprSpecified());
   if (ExplicitParams)
 CheckCXXDefaultArguments(Method);
+
+  // This represents the function body for the lambda function, check if we
+  // have to apply optnone due to a pragma.
+  AddRangeBasedOptnone(Method);
   
   // Attributes on the lambda apply to the method.  
   ProcessDeclAttributes(CurScope, Method, ParamInfo);


Index: test/CodeGenCXX/optnone-pragma-optimize-off.cpp
===
--- test/CodeGenCXX/optnone-pragma-optimize-off.cpp
+++ test/CodeGenCXX/optnone-pragma-optimize-off.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 %s -triple %itanium_abi_triple -O1 -disable-llvm-passes -emit-llvm -o - | FileCheck %s
+
+// Test the attributes for the lambda function contains 'optnone' as result of
+// the _Pragma("clang optimize off").
+
+_Pragma("clang optimize off")
+
+void foo(int p) {
+  auto lambda = [&p]() { ++p; };
+  lambda();
+  // CHECK: define {{.*}} @"_ZZ3fooiENK3$_0clEv"({{.*}}) #[[LAMBDA_ATR:[0-9]+]]
+}
+
+_Pragma("clang optimize on")
+
+// CHECK: attributes #[[LAMBDA_ATR]] = { {{.*}} optnone {{.*}} }
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -904,6 +904,10 @@
 ParamInfo.getDeclSpec().isConstexprSpecified());
   if (ExplicitParams)
 CheckCXXDefaultArguments(Method);
+
+  // This represents the function body for the lambda function, check if we
+  // have to apply optnone due to a pragma.
+  AddRangeBasedOptnone(Method);
   
   // Attributes on the lambda apply to the method.  
   ProcessDeclAttributes(CurScope, Method, ParamInfo);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43821: [SemaCXX] _Pragma("clang optimize off") not affecting lambda.

2018-03-15 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Ping.

Thanks


Repository:
  rC Clang

https://reviews.llvm.org/D43821



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D46190: For a referenced declaration, mark any associated usings as referenced.

2018-05-31 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Ping.

Thanks


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] D44826: Add -Wunused-using, a warning that finds unused using declarations.

2018-05-31 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso updated this revision to Diff 149261.
CarlosAlbertoEnciso added a comment.

This patch addresses the reviewers comments:

- Additional test cases to cover: -Wunused, -Wall and -Wno-unused-using
- Formatting in ReleaseNotes
- Removed the '-Wunused-usings' alias (GCC compatibility).
- Added individual warning messages for using-declaration, using-directive and 
namespace-alias.
- Fixed SVN attribute changes.


https://reviews.llvm.org/D44826

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/ExternalSemaSource.h
  include/clang/Sema/MultiplexExternalSemaSource.h
  include/clang/Sema/Sema.h
  include/clang/Sema/SemaInternal.h
  include/clang/Serialization/ASTBitCodes.h
  include/clang/Serialization/ASTReader.h
  lib/Sema/MultiplexExternalSemaSource.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/FixIt/fixit.cpp
  test/Modules/Inputs/module.map
  test/Modules/Inputs/warn-unused-using.h
  test/Modules/warn-unused-using.cpp
  test/PCH/cxx-templates.cpp
  test/SemaCXX/coreturn.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
  test/SemaCXX/referenced_using_options.cpp

Index: test/SemaCXX/referenced_using_options.cpp
===
--- test/SemaCXX/referenced_using_options.cpp
+++ test/SemaCXX/referenced_using_options.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wall -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+  typedef int Integer;
+}
+
+using N::Integer;   // expected-warning {{unused using declaration 'Integer'}}
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunused-using"
+using N::Integer;   // no warning
+#pragma clang diagnostic pop
+
+using N::Integer;   // expected-warning {{unused using declaration 'Integer'}}
+
Index: test/SemaCXX/referenced_using_directive.cpp
===
--- test/SemaCXX/referenced_using_directive.cpp
+++ test/SemaCXX/referenced_using_directive.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+  typedef int Integer;
+  int var;
+}
+
+void Fa() {
+  using namespace N;  // Referenced
+  var = 1;
+}
+
+void Fb() {
+  using namespace N;  // expected-warning {{unused using directive 'N'}}
+  N::var = 1;
+}
+
+void Fc() {
+  using namespace N;  // Referenced
+  Integer var = 1;
+}
+
+void Fd() {
+  using namespace N;  // expected-warning {{unused using directive 'N'}}
+  N::Integer var = 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,21 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+  typedef int Integer;
+  typedef char Char;
+}
+
+using N::Integer;   // expected-warning {{unused using declaration '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;// expected-warning {{unused using declaration 'Char'}}
+  return 0;
+}
+
Index: test/SemaCXX/referenced_using_declaration_1.cpp
===
--- test/SemaCXX/referenced_using_declaration_1.cpp
+++ test/SemaCXX/referenced_using_declaration_1.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+  // Types.
+  typedef int Integer;
+  struct Record {
+int a;
+  };
+
+  // Variables.
+  int var1;
+  int var2;
+
+  // Functions.
+  void func1();
+  void func2();
+}
+
+using N::Integer;   // expected-warning {{unused using declaration 'Integer'}}
+using N::Record;// expected-warning {{unused using declaration 'Record'}}
+using N::var1;  // expected-warning {{unused using declaration 'var1'}}
+using N::var2;  // expected-warning {{unused using declaration 'var2'}}
+using N::func1; // expected-warning {{unused using declaration 'func1'}}
+using N::func2; // expected-warning {{unused using declaration 'func2'}}
+
+void Foo() {
+  using N::Integer; // expected-warning {{unused using declaration 'Integer'}}
+  N::Integer int_var;
+  int_var = 1;
+
+  using N::Record;  // Referenced
+  Record rec_var;
+  rec_var.a = 2;
+
+  using N::var1;// expected-warning {{u

[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

2018-06-01 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added inline comments.



Comment at: docs/ReleaseNotes.rst:139
+  used in conjunction with ``-Werror`` and as a result, the new warnings
+  are turned into new errors.
+

thakis wrote:
> nit: I'd omit this paragraph -- this is true for all warnings and not special 
> for this warning.
Removed that specific paragraph.



Comment at: include/clang/Basic/DiagnosticGroups.td:828-829
 // -Wunused-local-typedefs = -Wunused-local-typedef
+def : DiagGroup<"unused-usings", [UnusedUsing]>;
+// -Wunused-usings = -Wunused-using
 

thakis wrote:
> CarlosAlbertoEnciso wrote:
> > lebedev.ri wrote:
> > > Why? gcc compatibility?
> > No particular reason. I just follow the 'unused-local-typedefs' model.
> > If there is not objection from others reviewers, I will drop the gcc 
> > compatibility.
> Does gcc have a `-Wunused-usings`? As far as I can tell it doesn't, so I 
> agree not having the alias makes sense. -Wunused-local-typedefs is here 
> because gcc has that flag.
As far as I can see, GCC does not have the ``-Wunused-usings`` alias.


https://reviews.llvm.org/D44826



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

2018-06-01 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso updated this revision to Diff 149425.
CarlosAlbertoEnciso marked 3 inline comments as done.
CarlosAlbertoEnciso added a comment.

Address feedback from @thakis in relation to the Release Notes.


https://reviews.llvm.org/D44826

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/ExternalSemaSource.h
  include/clang/Sema/MultiplexExternalSemaSource.h
  include/clang/Sema/Sema.h
  include/clang/Sema/SemaInternal.h
  include/clang/Serialization/ASTBitCodes.h
  include/clang/Serialization/ASTReader.h
  lib/Sema/MultiplexExternalSemaSource.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/FixIt/fixit.cpp
  test/Modules/Inputs/module.map
  test/Modules/Inputs/warn-unused-using.h
  test/Modules/warn-unused-using.cpp
  test/PCH/cxx-templates.cpp
  test/SemaCXX/coreturn.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
  test/SemaCXX/referenced_using_options.cpp

Index: test/SemaCXX/referenced_using_options.cpp
===
--- test/SemaCXX/referenced_using_options.cpp
+++ test/SemaCXX/referenced_using_options.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wall -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+  typedef int Integer;
+}
+
+using N::Integer;   // expected-warning {{unused using declaration 'Integer'}}
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunused-using"
+using N::Integer;   // no warning
+#pragma clang diagnostic pop
+
+using N::Integer;   // expected-warning {{unused using declaration 'Integer'}}
+
Index: test/SemaCXX/referenced_using_directive.cpp
===
--- test/SemaCXX/referenced_using_directive.cpp
+++ test/SemaCXX/referenced_using_directive.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+  typedef int Integer;
+  int var;
+}
+
+void Fa() {
+  using namespace N;  // Referenced
+  var = 1;
+}
+
+void Fb() {
+  using namespace N;  // expected-warning {{unused using directive 'N'}}
+  N::var = 1;
+}
+
+void Fc() {
+  using namespace N;  // Referenced
+  Integer var = 1;
+}
+
+void Fd() {
+  using namespace N;  // expected-warning {{unused using directive 'N'}}
+  N::Integer var = 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,21 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+  typedef int Integer;
+  typedef char Char;
+}
+
+using N::Integer;   // expected-warning {{unused using declaration '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;// expected-warning {{unused using declaration 'Char'}}
+  return 0;
+}
+
Index: test/SemaCXX/referenced_using_declaration_1.cpp
===
--- test/SemaCXX/referenced_using_declaration_1.cpp
+++ test/SemaCXX/referenced_using_declaration_1.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+  // Types.
+  typedef int Integer;
+  struct Record {
+int a;
+  };
+
+  // Variables.
+  int var1;
+  int var2;
+
+  // Functions.
+  void func1();
+  void func2();
+}
+
+using N::Integer;   // expected-warning {{unused using declaration 'Integer'}}
+using N::Record;// expected-warning {{unused using declaration 'Record'}}
+using N::var1;  // expected-warning {{unused using declaration 'var1'}}
+using N::var2;  // expected-warning {{unused using declaration 'var2'}}
+using N::func1; // expected-warning {{unused using declaration 'func1'}}
+using N::func2; // expected-warning {{unused using declaration 'func2'}}
+
+void Foo() {
+  using N::Integer; // expected-warning {{unused using declaration 'Integer'}}
+  N::Integer int_var;
+  int_var = 1;
+
+  using N::Record;  // Referenced
+  Record rec_var;
+  rec_var.a = 2;
+
+  using N::var1;// expected-warning {{unused using declaration 'var1'}}
+  N::var1 = 3;
+
+  using N::var2;// Referenced
+  var2 = 4;
+
+  using N::func1;   // expected-warning {{unused using declaration 'func1'}}
+  N::func1();
+
+  using N::func2;  

[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

2018-06-01 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Thanks to all reviewers for your comments and suggestions.


https://reviews.llvm.org/D44826



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

2018-06-01 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:282-290
+def warn_unused_using_declaration : Warning<
+  "unused using declaration %0">,
+  InGroup, DefaultIgnore;
+def warn_unused_using_directive : Warning<
+  "unused using directive %0">,
+  InGroup, DefaultIgnore;
+def warn_unused_using_alias : Warning<

lebedev.ri wrote:
> JFYI you can condense it down to just
> ```
> def warn_unused_using_declaration : Warning<
>   "unused %select{using declaration|using directive|namespace alias}0 %1">,
>   InGroup, DefaultIgnore;
> ```
> if that simplifies the code that actually emits that warning.
Thanks for the suggestion.

I will have a look at it and see if that simplifies the code that emits the 
warning.


https://reviews.llvm.org/D44826



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D46190: For a referenced declaration, mark any associated usings as referenced.

2018-06-07 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Ping.

The review

https://reviews.llvm.org/D44826

is already approved and it is dependent on this patch being reviewed.

Is there anything I can add to this patch?

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] D43015: clang-format: Introduce BreakInheritanceList option

2018-06-12 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

@Typz,

It seems that the changes done to `ClangFormatStyleOptions.rst` @334408 are 
causing the generation of the documentation to fail, with the following error:

  Warning, treated as error:
  /llvm/tools/clang/docs/ClangFormatStyleOptions.rst:1060: WARNING: Definition 
list ends without a blank line; unexpected unindent.

by reverting to change @332436 the build is fine and no errors are generated.

Thanks


Repository:
  rC Clang

https://reviews.llvm.org/D43015



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D46190: For a referenced declaration, mark any associated usings as referenced.

2018-06-14 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Ping.

The review

https://reviews.llvm.org/D44826

is already approved and it is dependent on this patch being reviewed.

@rsmith Is there anything I can add to this patch?

Thanks


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 referenced declaration, mark any associated usings as referenced.

2018-06-19 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

@probinson Thanks very much for your review. I will address them.


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 referenced declaration, mark any associated usings as referenced.

2018-06-19 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

rsmith Thanks very much for your review. I will address them.


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 an ODR declaration, set the 'Used' bit on its associated declarations.

2018-04-27 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso created this revision.
CarlosAlbertoEnciso added reviewers: rsmith, erichkeane, probinson, dblaikie.
CarlosAlbertoEnciso added a project: clang.
Herald added a subscriber: aprantl.

The following submission under review

https://reviews.llvm.org/D44826

introduced an option to warn on unused 'using declarations'.

But it can't detect cases like

  namespace nsp {
void foo();
int var;
  }using var
  
  using foo; <-- warning
  using var; <-- MISSING warning  
  
  void bar() {
using foo; <-- warning
using var; <-- warning
  }
  
  void bar() {
nsp::var = 1;
  }

One of the reviewers (dblaikie) mentioned the possible cause for that 
limitation.

  You mention a missed case in the description - where the target of a using 
decl is
  used and that causes the unused-using not to warn about the using decl? That
  seems like a big limitation. Does that mean the 'used' bit is being tracked 
in the
  wrong place, on the target of the using decl instead of on the using decl 
itself?

When a declaration is found to be ODR, only its associated 'Used' bit is set.

This patch, traverse its associated declarations and sets the 'Used' bit. The 
goal
is to create additional information to help in the reduction of the debug 
information
size, which is affected by extra generation of 'non-used' declarations.

Note:

To solve the debug information issue (size), there are 3 pieces of
work:

- Set the 'Used' bit recursively. This patch.
- Review the - Wunused-usings and -Wunused-local-typedefs implementation to 
take advantage of new 'Used' settings.
- Do not generate debug information for the unused using

Thanks for your view on this issue and on the general approach.


Repository:
  rC Clang

https://reviews.llvm.org/D46190

Files:
  include/clang/Sema/Sema.h
  include/clang/Sema/SemaInternal.h
  lib/AST/ASTDumper.cpp
  lib/AST/DeclBase.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/Frontend/float16.cpp
  test/Misc/ast-dump-attr.cpp
  test/Misc/ast-dump-color.cpp
  test/PCH/cxx-templates.cpp
  test/SemaCXX/used_class_struct_union.cpp
  test/SemaCXX/used_enum.cpp
  test/SemaCXX/used_goto.cpp
  test/SemaCXX/used_pointer.cpp
  test/SemaCXX/used_template.cpp
  test/SemaCXX/used_typedef.cpp
  test/SemaCXX/used_using.cpp

Index: test/SemaCXX/used_using.cpp
===
--- test/SemaCXX/used_using.cpp
+++ test/SemaCXX/used_using.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
+
+namespace A {
+  typedef char CHAR;
+}
+
+//CHECK:  |-NamespaceDecl {{.*}} used A
+//CHECK-NEXT: | `-TypedefDecl {{.*}} used CHAR 'char'
+//CHECK-NEXT: |   `-BuiltinType {{.*}}
+
+namespace B {
+  typedef char CHAR;
+}
+
+//CHECK:  |-NamespaceDecl {{.*}} used B
+//CHECK-NEXT: | `-TypedefDecl {{.*}} used CHAR 'char'
+//CHECK-NEXT: |   `-BuiltinType {{.*}}
+
+namespace C {
+  typedef char CHAR;
+  typedef int INTEGER;
+  typedef unsigned UNSIGNED;
+
+  void FA();
+  void FB();
+
+  int Var1;
+  int Var2;
+}
+
+//CHECK:  |-NamespaceDecl {{.*}} used C
+//CHECK-NEXT: | |-TypedefDecl {{.*}} CHAR 'char'
+//CHECK-NEXT: | | `-BuiltinType {{.*}}
+//CHECK-NEXT: | |-TypedefDecl {{.*}} INTEGER 'int'
+//CHECK-NEXT: | | `-BuiltinType {{.*}}
+//CHECK-NEXT: | |-TypedefDecl {{.*}} UNSIGNED 'unsigned int'
+//CHECK-NEXT: | | `-BuiltinType {{.*}}
+//CHECK-NEXT: | |-FunctionDecl {{.*}} used FA 'void ()'
+//CHECK-NEXT: | |-FunctionDecl {{.*}} used FB 'void ()'
+//CHECK-NEXT: | |-VarDecl {{.*}} used Var1 'int'
+//CHECK-NEXT: | `-VarDecl {{.*}} used Var2 'int'
+
+using A::CHAR;
+using B::CHAR;
+
+//CHECK:  |-UsingDecl {{.*}} A::CHAR
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} 'CHAR'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'A::CHAR' sugar
+//CHECK-NEXT: |   |-Typedef {{.*}}
+//CHECK-NEXT: |   `-BuiltinType {{.*}}
+//CHECK-NEXT: |-UsingDecl {{.*}} B::CHAR
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} 'CHAR'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'B::CHAR' sugar
+//CHECK-NEXT: |   |-Typedef {{.*}} 'CHAR'
+//CHECK-NEXT: |   `-BuiltinType {{.*}}
+
+void F1() {
+  using A::CHAR;
+  CHAR ca;
+  ca = '1';
+
+  using B::CHAR;
+  B::CHAR cb;
+  cb = '1';
+}
+
+//CHECK:  |-FunctionDecl {{.*}} used F1 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: |   |-DeclStmt {{.*}}
+//CHECK-NEXT: |   | `-UsingDecl {{.*}} used A::CHAR
+//CHECK-NEXT: |   |-DeclStmt {{.*}}
+//CHECK-NEXT: |   | `-VarDecl {{.*}} used ca 'A::CHAR':'char'
+//CHECK-NEXT: |   |-BinaryOperator {{.*}}
+//CHECK-NEXT: |   | |-DeclRefExpr {{.*}} 'ca' 'A::CHAR':'char'
+//CHECK-NEXT: |   | `-CharacterLiteral {{.*}}
+//CHECK-NEXT: |   |-DeclStmt {{.*}}
+//CHECK-NEXT: |   | `-UsingDecl {{.*}} B::CHAR
+//CHECK-NEXT: |   |-DeclStmt {{.*}}
+//CHECK-NEXT: |   | `-VarDecl {{.*}} used cb 'B::CHAR':'char'
+//CHECK-NEXT: |   `-B

[PATCH] D46190: For an ODR declaration, set the 'Used' bit on its associated declarations.

2018-04-27 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Note:

The patch includes the entry points to set Objective-C and OpenMP declarations, 
but I do not
know anything about Objective-C or OpenMP. The functions just return without 
updating any
extra 'Used' bit.

Any suggestion on how to deal with those functions, is very much appreciated.

Thanks.


Repository:
  rC Clang

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 an ODR declaration, set the 'Used' bit on its associated declarations.

2018-04-30 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D46190#1081836, @rsmith wrote:

> In https://reviews.llvm.org/D46190#1081773, @rsmith wrote:
>
> > No, absolutely not. This would be insanely expensive, and marks entirely 
> > unrelated things "used".
>
>
> My apologies, reading this again it comes off a lot harsher than I'd intended.


I appreciate your apology and I understand your concerns. I do not have much 
knowledge on this area.

My initial intention was to traverse just the associated type for an odr-used 
declaration and mark them as used, as an indication that those types are 
"really needed" for that odr-declaration.  The type chain is traversed just 
once.

The approach was intended as a general solution, not only for the original 
'using declaration' issue.

I will have a look at your suggested approach.

Thanks very much for your feedback.


Repository:
  rC Clang

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 an ODR declaration, set the 'Used' bit on its associated declarations.

2018-04-30 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

My initial approach was based on the following test case:

  void Bar() {
typedef int I_Ref;
I_Ref var_bar;
  }
  
  void Foo() {
typedef int I_Used;
I_Used var_foo;
var_foo = 2;
  }
  
  void Test() {
Foo();
  }

and the generated AST looks like:

  |-FunctionDecl Bar 'void ()'
  |   |-DeclStmt
  |   | `-TypedefDecl referenced I_Ref 'int'
  |   |-DeclStmt
  | `-VarDecl var_bar 'I_Ref':'int'
  
  |-FunctionDecl used Foo 'void ()'
  |   |-DeclStmt
  |   | `-TypedefDecl referenced I_Used 'int'
  |   |-DeclStmt
  |   | `-VarDecl used var_foo 'I_Used':'int'

The typedef 'I_Ref' is marked as 'referenced' due to its association with 
'var_bar'.
The typedef 'I_Used' is marked as 'referenced' despite that its association 
with 'var_foo' which is 'used'
Both 'typedefs' are marked as 'referenced'.

With the intended patch, the AST looks like:

  |-FunctionDecl Bar 'void ()'
  |   |-DeclStmt
  |   | `-TypedefDecl referenced I_Ref 'int'
  |   `-DeclStmt
  | `-VarDecl var_bar 'I_Ref':'int'
  
  |-FunctionDecl used Foo 'void ()'
  |   |-DeclStmt
  |   | `-TypedefDecl used I_Used 'int'
  |   |-DeclStmt
  |   | `-VarDecl used var_foo 'I_Used':'int'

The typedef 'I_Ref' is marked as 'referenced'.
The typedef 'I_Used' is marked as 'used'.


Repository:
  rC Clang

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 an ODR declaration, set the 'Used' bit on its associated declarations.

2018-05-02 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D46190#1083502, @rsmith wrote:

> This is already the correct outcome. Both typedefs are referenced; this is 
> correct because they are named by the source code. Neither typedef is marked 
> as "used"; this is correct because a typedef cannot be odr-used.


I appreciate very much your explanations on the typedef case.

My misunderstanding comes from my experience with a commercial C/C++ front-end, 
which has the concept of 'needed' and 'really needed' and those concepts seems 
different to the 'referenced' and 'used' in Clang. I was trying to match that 
concept:

needed -> referenced.
really needed -> used.


Repository:
  rC Clang

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 an ODR declaration, set the 'Used' bit on its associated declarations.

2018-05-02 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D46190#1083502, @rsmith wrote:

> Let's look at your original example again (I've cleaned it up and extended it 
> a bit, but hopefully this matches your intent):
>
>   namespace nsp {
> int var1, var2;
>   }
>  
>   using nsp::var1;
>   using nsp::var2;
>  
>   void bar() {
> nsp::var1 = 1;
> var2 = 1;
>   }
>
>
> Now, what should happen here is:
>
> - The use of `nsp::var1` in `bar` marks `nsp::var1` as referenced (because it 
> was named), and marks `nsp::var1` as used (because it was odr-used by the 
> assignment). The `UsingShadowDecl` for `::var1` is not marked as referenced 
> (nor used), so we emit an unused-using-declaration warning.
> - The use of `var2` in `bar` marks `::var2` as referenced (because it was 
> named), and marks `nsp::var2` as used (because it was odr-used by the 
> assignment). The `UsingShadowDecl` for `::var2` is marked as referenced, so 
> we do not warn on it.


Your extended example matches my original intent. That is a good starting 
point. Now I am clear on the expected behavior.

Thanks very much for your comments.


Repository:
  rC Clang

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] D48441: [clangd] Incorporate transitive #includes into code complete proximity scoring.

2018-07-04 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Hi,

It seems that your patch is causing an error in the tests for

http://lab.llvm.org:8011/builders/clang-x64-ninja-win7

http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/11510/steps/ninja%20check%201/logs/FAIL%3A%20Extra%20Tools%20Unit%20Tests%3A%3AFileDistanceTests.URI

Thanks very much


Repository:
  rL LLVM

https://reviews.llvm.org/D48441



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D46190: For a referenced declaration, mark any associated usings as referenced.

2018-07-04 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D46190#1135688, @rsmith wrote:

> The right way to handle this is to pass both the ultimately-selected 
> declaration and the declaration found by name lookup into the calls to 
> `MarkAnyDeclReferenced` and friends. We should mark the selected declaration 
> as `Used` (when appropriate), and mark the found declaration as `Referenced`.
>
> We should not be trying to reconstruct which using declarations are in scope 
> after the fact. Only declarations actually found by name lookup and then 
> selected by the context performing the lookup should be marked referenced. 
> (There may be cases where name lookup discards equivalent lookup results that 
> are not redeclarations of one another, though, and to handle such cases 
> properly, you may need to teach name lookup to track a list of found 
> declarations rather than only one.)


Following your comments, I have replaced the scope traversal with `LookupName` 
in order to find the declarations.

But there are 2 specific cases:

- using-directives

The `LookupName` function does not record the using-directives. With this 
patch, there is an extra option to store those directives in the lookup 
results, to be processed during the setting of the 'Referenced' bit.

- namespace-alias

I am aware of your comment on the function that recursively traverse the 
namespace alias, but I can't see another way to do it. If you have a more 
specific idea, I am happy to explore it.

For both cases, may be `LookupName` function can have that functionality and 
store in the results any namespace-directives and namespace-alias associated 
with the given declaration.




Comment at: lib/Sema/Sema.cpp:1879
+void Sema::MarkNamespaceAliasReferenced(NamedDecl *ND) {
+  if (ND && !ND->isReferenced()) {
+NamespaceAliasDecl *NA = nullptr;

probinson wrote:
> You could do this as an early return and reduce indentation in the rest of 
> the method.
> ```
> if (!ND || ND->isReferenced())
>   return;
> ```
> 
Corrected to

```
if (!ND || ND->isReferenced())
  return;
```




Comment at: lib/Sema/Sema.cpp:1880
+  if (ND && !ND->isReferenced()) {
+NamespaceAliasDecl *NA = nullptr;
+while ((NA = dyn_cast(ND)) && !NA->isReferenced()) {

probinson wrote:
> Initializing this to nullptr is redundant, as you set NA in the while-loop 
> expression.
Removed the `nullptr` initialization.



Comment at: lib/Sema/Sema.cpp: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) {

probinson wrote:
> `\brief` is unnecessary, as we have auto-brief turned on.
Removed the `\brief` format.




Comment at: lib/Sema/Sema.cpp:1903
+  if (auto *NNS = SS ? SS->getScopeRep()
+ : E ? dyn_cast(E)->getQualifier()
+ : nullptr) {

probinson wrote:
> This dyn_cast<> can be simply a cast<>.
Changed the dyn_cast<> to cast<>



Comment at: lib/Sema/Sema.cpp:1916
+  if ((USD = dyn_cast(DR)) && !USD->isReferenced()) {
+if (USD->getTargetDecl() == D) {
+  USD->setReferenced();

probinson wrote:
> You could sink the declaration of USD like so:
> ```
> for (auto *DR : S->decls())
>   if (auto *USD = dyn_cast(DR))
> if (!USD->isReferenced() && USD->getTargetDecl() == D) {
> ```
> Also I would put braces around the 'for' loop body, even though it is 
> technically one statement.
Not available in the new patch.



Comment at: lib/Sema/Sema.cpp:1927
+// Check if the declaration was introduced by a 'using-directive'.
+auto *Target = dyn_cast(DC);
+for (auto *UD : S->using_directives())

probinson wrote:
> You verified that his is a namespace earlier, so use cast<> not dyn_cast<>.
Not available in the new patch.


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 referenced declaration, mark any associated usings as referenced.

2018-07-04 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D46190#1135295, @probinson wrote:

> Style comments.
>  The two new Sema methods (for namespaces and using references) are C++ 
> specific, so SemaDeclCXX.cpp would seem like a more appropriate home for them.


Both functions have been moved to SemaDeclCXX.cpp.


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] D48441: [clangd] Incorporate transitive #includes into code complete proximity scoring.

2018-07-04 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D48441#1151889, @ioeric wrote:

> Hi Carlos, thanks for letting us know! I sent r336249 to fix the windows
>  test.


Hi @ioeric,

Thanks very much. Happy to help.


Repository:
  rL LLVM

https://reviews.llvm.org/D48441



___
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.

2018-07-04 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
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'
+//

[PATCH] D46190: For a used declaration, mark any associated usings as referenced.

2018-07-04 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
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.

2018-07-11 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
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.

2018-07-18 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
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.

2018-07-19 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
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.

2018-07-19 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
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.

2018-07-19 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
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] D77184: Make it possible for lit.site.cfg to contain relative paths, and use it for llvm and clang

2020-04-04 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In D77184#1960473 , @thakis wrote:

> I pushed Andrew's fix (thanks!) (with minor formatting tweaks) in 
> dbb0d8ecb3a024bd6817ebd8ad8c5c199a51d933 
>  . Let 
> me know if you still see issues.


It fixes the Windows issues when building from different drive. Thanks


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77184/new/

https://reviews.llvm.org/D77184



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

2021-03-09 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In D97411#2611142 , @probinson wrote:

> In D97411#2598625 , @akhuang wrote:
>
>> I started looking into some diffs of debug info in libc++ tests, but it's 
>> pretty hard to tell what's different - as far as I can see, there are just a 
>> bunch of `__hash_value_type`s and `__value_type`s.
>
> This is a job for llvm-dva!  See the preliminary patch at D88661 
> , although it's getting a bit old and might 
> not apply/build cleanly.
>
> (llvm-dva is undergoing an internal review at the moment, we hope to have a 
> proper reviewable patch series up soon-ish.)

Uploaded an updated `llvm-dva` single patch the builds with the TOT.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97411/new/

https://reviews.llvm.org/D97411

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D131665: [CMake] Support passing arguments to build tool (bootstrap).

2022-08-11 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso created this revision.
CarlosAlbertoEnciso added reviewers: andrewng, phosek, beanz, russell.gallop.
CarlosAlbertoEnciso added a project: LLVM.
Herald added a subscriber: mgorny.
Herald added a project: All.
CarlosAlbertoEnciso requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For bootstrap builds (CLANG_ENABLE_BOOTSTRAP=ON) allow 
arguments to be passed to the native tool used in CMake 
for the stage2 step.

Can be used to pass extra arguments for enhanced versions 
of build tools, e.g. distributed build options.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131665

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -787,6 +787,14 @@
 endif()
   endforeach()
 
+  # Build arguments for native tool used in CMake.
+  set(build_configuration "$")
+  set(build_tool_args "${LLVM_EXTERNAL_PROJECT_BUILD_TOOL_ARGS}")
+  if(NOT build_tool_args STREQUAL "")
+string(PREPEND build_tool_args "-- ")
+separate_arguments(build_tool_args UNIX_COMMAND "${build_tool_args}")
+  endif()
+
   ExternalProject_Add(${NEXT_CLANG_STAGE}
 DEPENDS clang-bootstrap-deps
 PREFIX ${NEXT_CLANG_STAGE}
@@ -809,6 +817,9 @@
 ${${CLANG_STAGE}_RANLIB}
 ${${CLANG_STAGE}_OBJCOPY}
 ${${CLANG_STAGE}_STRIP}
+BUILD_COMMAND ${CMAKE_COMMAND} --build ${BINARY_DIR}
+   --config ${build_configuration}
+   ${build_tool_args}
 INSTALL_COMMAND ""
 STEP_TARGETS configure build
 USES_TERMINAL_CONFIGURE 1


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -787,6 +787,14 @@
 endif()
   endforeach()
 
+  # Build arguments for native tool used in CMake.
+  set(build_configuration "$")
+  set(build_tool_args "${LLVM_EXTERNAL_PROJECT_BUILD_TOOL_ARGS}")
+  if(NOT build_tool_args STREQUAL "")
+string(PREPEND build_tool_args "-- ")
+separate_arguments(build_tool_args UNIX_COMMAND "${build_tool_args}")
+  endif()
+
   ExternalProject_Add(${NEXT_CLANG_STAGE}
 DEPENDS clang-bootstrap-deps
 PREFIX ${NEXT_CLANG_STAGE}
@@ -809,6 +817,9 @@
 ${${CLANG_STAGE}_RANLIB}
 ${${CLANG_STAGE}_OBJCOPY}
 ${${CLANG_STAGE}_STRIP}
+BUILD_COMMAND ${CMAKE_COMMAND} --build ${BINARY_DIR}
+   --config ${build_configuration}
+   ${build_tool_args}
 INSTALL_COMMAND ""
 STEP_TARGETS configure build
 USES_TERMINAL_CONFIGURE 1
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D131755: [CMake] Explicit bootstrap options override any passthrough ones.

2022-08-12 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso created this revision.
CarlosAlbertoEnciso added reviewers: sylvestre.ledru, andrewng, russell.gallop, 
phosek, beanz.
CarlosAlbertoEnciso added projects: All, LLVM, clang.
Herald added a subscriber: mgorny.
CarlosAlbertoEnciso requested review of this revision.
Herald added a subscriber: cfe-commits.

The https://reviews.llvm.org/D53014 added CMAKE_BUILD_TYPE to 
the list of BOOTSTRAP_DEFAULT_PASSTHROUGH variables.

The downside is that both stage-1 and stage-2 configurations
are the same. So it is not possible to build different stage
configurations.

This patch allow explicit bootstrap options to override any 
passthrough ones.

For instance, the following settings would build:
stage-1 (Release) and stage-2 (Debug)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131755

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -758,6 +758,19 @@
 set(LTO_RANLIB)
   endif()
 
+  # Populate the passthrough variables
+  foreach(variableName ${CLANG_BOOTSTRAP_PASSTHROUGH} 
${_BOOTSTRAP_DEFAULT_PASSTHROUGH})
+if(DEFINED ${variableName})
+  if("${${variableName}}" STREQUAL "")
+set(value "")
+  else()
+string(REPLACE ";" "|" value "${${variableName}}")
+  endif()
+  list(APPEND PASSTHROUGH_VARIABLES
+-D${variableName}=${value})
+endif()
+  endforeach()
+
   # Find all variables that start with BOOTSTRAP_ and populate a variable with
   # them.
   get_cmake_property(variableNames VARIABLES)
@@ -774,19 +787,6 @@
 endif()
   endforeach()
 
-  # Populate the passthrough variables
-  foreach(variableName ${CLANG_BOOTSTRAP_PASSTHROUGH} 
${_BOOTSTRAP_DEFAULT_PASSTHROUGH})
-if(DEFINED ${variableName})
-  if("${${variableName}}" STREQUAL "")
-set(value "")
-  else()
-string(REPLACE ";" "|" value "${${variableName}}")
-  endif()
-  list(APPEND PASSTHROUGH_VARIABLES
--D${variableName}=${value})
-endif()
-  endforeach()
-
   ExternalProject_Add(${NEXT_CLANG_STAGE}
 DEPENDS clang-bootstrap-deps
 PREFIX ${NEXT_CLANG_STAGE}


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -758,6 +758,19 @@
 set(LTO_RANLIB)
   endif()
 
+  # Populate the passthrough variables
+  foreach(variableName ${CLANG_BOOTSTRAP_PASSTHROUGH} ${_BOOTSTRAP_DEFAULT_PASSTHROUGH})
+if(DEFINED ${variableName})
+  if("${${variableName}}" STREQUAL "")
+set(value "")
+  else()
+string(REPLACE ";" "|" value "${${variableName}}")
+  endif()
+  list(APPEND PASSTHROUGH_VARIABLES
+-D${variableName}=${value})
+endif()
+  endforeach()
+
   # Find all variables that start with BOOTSTRAP_ and populate a variable with
   # them.
   get_cmake_property(variableNames VARIABLES)
@@ -774,19 +787,6 @@
 endif()
   endforeach()
 
-  # Populate the passthrough variables
-  foreach(variableName ${CLANG_BOOTSTRAP_PASSTHROUGH} ${_BOOTSTRAP_DEFAULT_PASSTHROUGH})
-if(DEFINED ${variableName})
-  if("${${variableName}}" STREQUAL "")
-set(value "")
-  else()
-string(REPLACE ";" "|" value "${${variableName}}")
-  endif()
-  list(APPEND PASSTHROUGH_VARIABLES
--D${variableName}=${value})
-endif()
-  endforeach()
-
   ExternalProject_Add(${NEXT_CLANG_STAGE}
 DEPENDS clang-bootstrap-deps
 PREFIX ${NEXT_CLANG_STAGE}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D131755: [CMake] Explicit bootstrap options override any passthrough ones.

2022-08-17 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In D131755#3723366 , @beanz wrote:

> Looks good to me.

Thanks for the review.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131755/new/

https://reviews.llvm.org/D131755

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D131755: [CMake] Explicit bootstrap options override any passthrough ones.

2022-08-17 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG177cbb1c9b66: [CMake] Explicit bootstrap options override 
any passthrough ones. (authored by CarlosAlbertoEnciso).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131755/new/

https://reviews.llvm.org/D131755

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -758,6 +758,19 @@
 set(LTO_RANLIB)
   endif()
 
+  # Populate the passthrough variables
+  foreach(variableName ${CLANG_BOOTSTRAP_PASSTHROUGH} 
${_BOOTSTRAP_DEFAULT_PASSTHROUGH})
+if(DEFINED ${variableName})
+  if("${${variableName}}" STREQUAL "")
+set(value "")
+  else()
+string(REPLACE ";" "|" value "${${variableName}}")
+  endif()
+  list(APPEND PASSTHROUGH_VARIABLES
+-D${variableName}=${value})
+endif()
+  endforeach()
+
   # Find all variables that start with BOOTSTRAP_ and populate a variable with
   # them.
   get_cmake_property(variableNames VARIABLES)
@@ -774,19 +787,6 @@
 endif()
   endforeach()
 
-  # Populate the passthrough variables
-  foreach(variableName ${CLANG_BOOTSTRAP_PASSTHROUGH} 
${_BOOTSTRAP_DEFAULT_PASSTHROUGH})
-if(DEFINED ${variableName})
-  if("${${variableName}}" STREQUAL "")
-set(value "")
-  else()
-string(REPLACE ";" "|" value "${${variableName}}")
-  endif()
-  list(APPEND PASSTHROUGH_VARIABLES
--D${variableName}=${value})
-endif()
-  endforeach()
-
   ExternalProject_Add(${NEXT_CLANG_STAGE}
 DEPENDS clang-bootstrap-deps
 PREFIX ${NEXT_CLANG_STAGE}


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -758,6 +758,19 @@
 set(LTO_RANLIB)
   endif()
 
+  # Populate the passthrough variables
+  foreach(variableName ${CLANG_BOOTSTRAP_PASSTHROUGH} ${_BOOTSTRAP_DEFAULT_PASSTHROUGH})
+if(DEFINED ${variableName})
+  if("${${variableName}}" STREQUAL "")
+set(value "")
+  else()
+string(REPLACE ";" "|" value "${${variableName}}")
+  endif()
+  list(APPEND PASSTHROUGH_VARIABLES
+-D${variableName}=${value})
+endif()
+  endforeach()
+
   # Find all variables that start with BOOTSTRAP_ and populate a variable with
   # them.
   get_cmake_property(variableNames VARIABLES)
@@ -774,19 +787,6 @@
 endif()
   endforeach()
 
-  # Populate the passthrough variables
-  foreach(variableName ${CLANG_BOOTSTRAP_PASSTHROUGH} ${_BOOTSTRAP_DEFAULT_PASSTHROUGH})
-if(DEFINED ${variableName})
-  if("${${variableName}}" STREQUAL "")
-set(value "")
-  else()
-string(REPLACE ";" "|" value "${${variableName}}")
-  endif()
-  list(APPEND PASSTHROUGH_VARIABLES
--D${variableName}=${value})
-endif()
-  endforeach()
-
   ExternalProject_Add(${NEXT_CLANG_STAGE}
 DEPENDS clang-bootstrap-deps
 PREFIX ${NEXT_CLANG_STAGE}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D131665: [CMake] Support passing arguments to build tool (bootstrap).

2022-08-19 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

To give more scope to the reviewers, the following CMake options illustrate the 
benefit of this patch.

For example, if I want to do a dry run on stage-2, I will pass 
`-DLLVM_EXTERNAL_PROJECT_BUILD_TOOL_ARGS="-n --verbose"`

  -DCLANG_ENABLE_BOOTSTRAP=ON -DLLVM_ENABLE_PROJECTS=clang 
-DCMAKE_BUILD_TYPE=Release -DLLVM_TARGETS_TO_BUILD=X86 -G"Ninja" 
-DLLVM_EXTERNAL_PROJECT_BUILD_TOOL_ARGS="-n --verbose"

When `ninja` builds `stage-2` it receives the additional `-n` and `--verbose` 
options.

The `build_tool_args` passed to `BUILD_COMMAND` looks like:

  '--;-n;--verbose'


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131665/new/

https://reviews.llvm.org/D131665

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D131665: [CMake] Support passing arguments to build tool (bootstrap).

2022-08-22 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6c6c4f6a9b3e: [CMake] Support passing arguments to build 
tool (bootstrap). (authored by CarlosAlbertoEnciso).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131665/new/

https://reviews.llvm.org/D131665

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -787,6 +787,14 @@
 endif()
   endforeach()
 
+  # Build arguments for native tool used in CMake.
+  set(build_configuration "$")
+  set(build_tool_args "${LLVM_EXTERNAL_PROJECT_BUILD_TOOL_ARGS}")
+  if(NOT build_tool_args STREQUAL "")
+string(PREPEND build_tool_args "-- ")
+separate_arguments(build_tool_args UNIX_COMMAND "${build_tool_args}")
+  endif()
+
   ExternalProject_Add(${NEXT_CLANG_STAGE}
 DEPENDS clang-bootstrap-deps
 PREFIX ${NEXT_CLANG_STAGE}
@@ -809,6 +817,9 @@
 ${${CLANG_STAGE}_RANLIB}
 ${${CLANG_STAGE}_OBJCOPY}
 ${${CLANG_STAGE}_STRIP}
+BUILD_COMMAND ${CMAKE_COMMAND} --build ${BINARY_DIR}
+   --config ${build_configuration}
+   ${build_tool_args}
 INSTALL_COMMAND ""
 STEP_TARGETS configure build
 USES_TERMINAL_CONFIGURE 1


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -787,6 +787,14 @@
 endif()
   endforeach()
 
+  # Build arguments for native tool used in CMake.
+  set(build_configuration "$")
+  set(build_tool_args "${LLVM_EXTERNAL_PROJECT_BUILD_TOOL_ARGS}")
+  if(NOT build_tool_args STREQUAL "")
+string(PREPEND build_tool_args "-- ")
+separate_arguments(build_tool_args UNIX_COMMAND "${build_tool_args}")
+  endif()
+
   ExternalProject_Add(${NEXT_CLANG_STAGE}
 DEPENDS clang-bootstrap-deps
 PREFIX ${NEXT_CLANG_STAGE}
@@ -809,6 +817,9 @@
 ${${CLANG_STAGE}_RANLIB}
 ${${CLANG_STAGE}_OBJCOPY}
 ${${CLANG_STAGE}_STRIP}
+BUILD_COMMAND ${CMAKE_COMMAND} --build ${BINARY_DIR}
+   --config ${build_configuration}
+   ${build_tool_args}
 INSTALL_COMMAND ""
 STEP_TARGETS configure build
 USES_TERMINAL_CONFIGURE 1
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D131665: [CMake] Support passing arguments to build tool (bootstrap).

2022-08-22 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In D131665#3740037 , @phosek wrote:

> LGTM

@phosek Thanks for your review.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131665/new/

https://reviews.llvm.org/D131665

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

2022-08-25 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

@Ericson2314 It seems that these changes break building on Windows using Visual 
Studio 2019.
There are 2 issues:

- CMake now creates a directory `$(Configuration)` in the root build directory.

  $(Configuration)\lib\cmake\llvm
  Location before the changes: lib\cmake\llvm

- The build fails with the errors:

  Error C1083 Cannot open include file: 'PPCGenRegisterInfo.inc': No such file 
or directory LLVMExegesisPowerPC 
llvm-project\llvm\lib\Target\PowerPC\MCTargetDesc\PPCMCTargetDesc.h
  Error C1083 Cannot open include file: 'MipsGenRegisterInfo.inc': No such file 
or directoryLLVMExegesisMips
llvm-project\llvm\lib\Target\Mips\MCTargetDesc\MipsMCTargetDesc.h
  Error C1083 Cannot open include file: 'X86GenRegisterInfo.inc': No such file 
or directory LLVMExegesisX86 
llvm-project\llvm\lib\Target\X86\MCTargetDesc\X86MCTargetDesc.h
  Error C1083 Cannot open include file: 'AArch64GenRegisterInfo.inc': No such 
file or directory LLVMExegesisAArch64 
llvm-project\llvm\lib\Target\AArch64\MCTargetDesc\AArch64MCTargetDesc.h


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132316/new/

https://reviews.llvm.org/D132316

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

2022-08-25 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In D132316#3748845 , @Ericson2314 
wrote:

> Let's just revert this. I'll do it later today if no one beats me to it.

Thanks


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132316/new/

https://reviews.llvm.org/D132316

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D127863: [clang] Dont print implicit forrange initializer

2022-06-17 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

@kadircet It seems that your change broke couple of clang tests:

https://lab.llvm.org/buildbot/#/builders/109/builds/40797


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127863/new/

https://reviews.llvm.org/D127863

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D77184: Make it possible for lit.site.cfg to contain relative paths, and use it for llvm and clang

2020-04-03 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In D77184#1957632 , @thakis wrote:

> Thanks!


Hi,

After the series of changes you have done, I am experiencing a build error on 
Windows. These are some of the errors:




- Attribute ignored -- Loadable modules not supported on this platform.

Traceback (most recent call last):

  File "", line 1, in 
  File "", line 1, in 
  File "C:\ProgramData\Python27\lib\ntpath.py", line 529, in relpath
% (path_prefix, start_prefix))

ValueError: path is on drive X:, start on drive E:
CMake Error at X:/llvm-root/llvm-project/llvm/cmake/modules/AddLLVM.cmake:1508 
(message):

  PATHS lengths got confused

Call Stack (most recent call first):

  X:/llvm-root/llvm-project/clang/test/CMakeLists.txt:33 
(configure_lit_site_cfg)

CMake Error at X:/llvm-root/llvm-project/llvm/cmake/modules/AddLLVM.cmake:1517 
(list):

  list GET given empty list

Call Stack (most recent call first):

  X:/llvm-root/llvm-project/clang/test/CMakeLists.txt:33 
(configure_lit_site_cfg)

CMake Error at X:/llvm-root/llvm-project/llvm/cmake/modules/AddLLVM.cmake:1517 
(list):

  list GET given empty list

Call Stack (most recent call first):

  X:/llvm-root/llvm-project/clang/test/CMakeLists.txt:33 
(configure_lit_site_cfg)

CMake Error at X:/llvm-root/llvm-project/llvm/cmake/modules/AddLLVM.cmake:1517 
(list):

  list GET given empty list

Call Stack (most recent call first):

  X:/llvm-root/llvm-project/clang/test/CMakeLists.txt:33 
(configure_lit_site_cfg)

CMake Error at X:/llvm-root/llvm-project/llvm/cmake/modules/AddLLVM.cmake:1517 
(list):

  list GET given empty list

Call Stack (most recent call first):

  X:/llvm-root/llvm-project/clang/test/CMakeLists.txt:33 
(configure_lit_site_cfg)




My CMake command line is:

E:\llvm-root\builds\win\debug\x64>cmake -G"Visual Studio 15 2017 Win64" 
-Thost=x64 -DCMAKE_BUILD_TYPE=Debug 
-DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-pc-linux-gnu
-DLLVM_ENABLE_PROJECTS="clang" x:\llvm-root\llvm-project\llvm 
-DLLVM_ENABLE_ZLIB=OFF

Drive E: contains the build files
Drive X: contains the llvm sources




The only way to have a successful build is to revert : 
ab11b9eefa16661017c2c7b3b34c46b069f43fb7 





Thanks


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77184/new/

https://reviews.llvm.org/D77184



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43821: [SemaCXX] _Pragma("clang optimize off") not affecting lambda.

2018-03-22 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Ping.

Thanks


Repository:
  rC Clang

https://reviews.llvm.org/D43821



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43821: [SemaCXX] _Pragma("clang optimize off") not affecting lambda.

2018-03-22 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Thanks @erichkeane for your review.


Repository:
  rC Clang

https://reviews.llvm.org/D43821



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

2018-03-23 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso created this revision.
CarlosAlbertoEnciso added reviewers: rsmith, erichkeane, probinson, dblaikie.
CarlosAlbertoEnciso added a project: clang.
Herald added subscribers: cfe-commits, JDevlieghere, aprantl.

Add -Wunused-using, a warning that finds unused using declarations.
Also added an alias -Wunused-usings.

This patch uses a similar approach as the work done for the typedefs
as discussed in:

https://reviews.llvm.org/rC217298

It uses the 'used' and 'referenced' available bits for the declarations.

As consequence of a previous work on Debug Information done by:

https://reviews.llvm.org/D6173

The size of the Debug Information increased by a considerable factor,
as discussed in:

http://clang-developers.42468.n3.nabble.com/r20-causes-real-debug-info-bloat-td4045257.html

For the below test:

  namespace nsp {
void foo();
  }
  
  using foo;

A debug information entries are generated for the namespace
and for the using declaration.

In order to reduce the debug information for those specific cases,
the work is divided in 2 parts:

- Emit a warning for the unused using
- Do not generate debug information for the unused using

The current patch deals with the first part and it covers global
and local detection of unused using declarations.

For the below test, I have marked the generated warnings

  namespace nsp {
void foo();
int var;
  }
  
  using foo; <-- warning
  using var; <-- warning  
  
  void bar() {
using foo; <-- warning
using var; <-- warning
  }

However, there is a specific case, where an unused using
escapes the warning.

Adding the following function 'bar'

  void bar() {
nsp::var = 1;
  }

it causes the warning to the global unused 'using var'
not being issued, as the assignment sets the 'used' bit,
despite of using the qualified 'var'.

  namespace nsp {
void foo();
int var;
  }using var
  
  using foo; <-- warning
  using var; <-- MISSING warning  
  
  void bar() {
using foo; <-- warning
using var; <-- warning
  }
  
  void bar() {
nsp::var = 1;
  }

Thanks for your view on this issue and on the general approach.


Repository:
  rC Clang

https://reviews.llvm.org/D44826

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/ExternalSemaSource.h
  include/clang/Sema/MultiplexExternalSemaSource.h
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  include/clang/Serialization/ASTReader.h
  lib/Sema/MultiplexExternalSemaSource.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/FixIt/fixit.cpp
  test/Modules/Inputs/module.map
  test/Modules/Inputs/warn-unused-using.h
  test/Modules/warn-unused-using.cpp
  test/SemaCXX/coreturn.cpp
  test/SemaCXX/warn-unused-using-serialize.cpp
  test/SemaCXX/warn-unused-using.cpp

Index: test/SemaCXX/warn-unused-using.cpp
===
--- test/SemaCXX/warn-unused-using.cpp
+++ test/SemaCXX/warn-unused-using.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-using -verify -std=c++1y %s
+
+namespace nsp {
+  void Print();
+  int var;
+  typedef int INT;
+  typedef char CHAR;
+  typedef float FLOAT;
+}
+
+using nsp::Print; // expected-warning {{unused using 'Print'}}
+using nsp::var; // expected-warning {{unused using 'var'}}
+
+void Foo() {
+  using nsp::Print; // expected-warning {{unused using 'Print'}}
+  {
+using nsp::var; // expected-warning {{unused using 'var'}}
+{
+  using nsp::INT; // expected-warning {{unused using 'INT'}}
+  using nsp::FLOAT; // expected-warning {{unused using 'FLOAT'}}
+}
+  }
+}
+
+void Bar() {
+  using nsp::FLOAT; // no diag
+  FLOAT f;
+  f = 2.0;
+}
+
+struct A {
+  virtual void Foo(double x) const;
+};
+struct B : A {
+  using A::Foo;  // expected-warning {{unused using 'Foo'}}
+  virtual void Foo(double x) const;
+};
+
+void one() {
+  B b;
+  b.Foo(1.0);
+}
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunused-using"
+using nsp::CHAR; // no diag
+#pragma clang diagnostic pop
+
+using nsp::CHAR;  // expected-warning {{unused using 'CHAR'}}
Index: test/SemaCXX/warn-unused-using-serialize.cpp
===
--- test/SemaCXX/warn-unused-using-serialize.cpp
+++ test/SemaCXX/warn-unused-using-serialize.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang -x c++-header -c -Wunused-using %s -o %t.gch -Werror
+// RUN: %clang -DBE_THE_SOURCE -c -Wunused-using -include %t %s -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang -DBE_THE_SOURCE -c -Wunused-using -include %t %s -o /dev/null 2>&1 | FileCheck %s
+
+#ifndef BE_THE_SOURCE
+namespace nsp {
+  typedef int INT;
+}
+//inline void foo() {
+// The warning should fire every time the pch file is used, not when it's built.
+// CHECK: warning: unused using 'INT'

[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

2018-03-23 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D44826#1046671, @erichkeane wrote:

> My opinion matters less than @rsmith or @dblaikie on the review, but it seems 
> to me that Typedef and Using are SO similar that the implementations should 
> just be combined.  You'd likely have to change a couple of types along the 
> way to be more generic, but the implementations are essentially a copy/paste 
> of eachother.


That is a very valid point and it simplifies quite a lot the patch.

If the other reviewers do not have any objection, I will combine both 
implementations and update the uploaded patch.

Thanks,
Carlos


Repository:
  rC Clang

https://reviews.llvm.org/D44826



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

2018-03-26 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D44826#1046910, @Eugene.Zelenko wrote:

> This duplicates Clang-tidy misc-unused-using-decls 
> . 
> If Clang will provide same or better functionality, it should be removed.


Thanks for bringing this up.


Repository:
  rC Clang

https://reviews.llvm.org/D44826



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

2018-03-26 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D44826#1046913, @Eugene.Zelenko wrote:

> Please also mention new warning in Release Notes and documentation.


I will do it.

> Will this warning be part of -Wall or -Wextra?

The warning is part of -Wall

Thanks for your feedback.


Repository:
  rC Clang

https://reviews.llvm.org/D44826



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43821: [SemaCXX] _Pragma("clang optimize off") not affecting lambda.

2018-03-26 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL328494: [SemaCXX] _Pragma("clang optimize off") 
not affecting lambda. (authored by CarlosAlbertoEnciso, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43821?vs=137550&id=139783#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43821

Files:
  cfe/trunk/lib/Sema/SemaLambda.cpp
  cfe/trunk/test/CodeGenCXX/optnone-pragma-optimize-off.cpp


Index: cfe/trunk/test/CodeGenCXX/optnone-pragma-optimize-off.cpp
===
--- cfe/trunk/test/CodeGenCXX/optnone-pragma-optimize-off.cpp
+++ cfe/trunk/test/CodeGenCXX/optnone-pragma-optimize-off.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 %s -triple %itanium_abi_triple -O1 -disable-llvm-passes 
-emit-llvm -o - | FileCheck %s
+
+// Test the attributes for the lambda function contains 'optnone' as result of
+// the _Pragma("clang optimize off").
+
+_Pragma("clang optimize off")
+
+void foo(int p) {
+  auto lambda = [&p]() { ++p; };
+  lambda();
+  // CHECK: define {{.*}} @"_ZZ3fooiENK3$_0clEv"({{.*}}) #[[LAMBDA_ATR:[0-9]+]]
+}
+
+_Pragma("clang optimize on")
+
+// CHECK: attributes #[[LAMBDA_ATR]] = { {{.*}} optnone {{.*}} }
\ No newline at end of file
Index: cfe/trunk/lib/Sema/SemaLambda.cpp
===
--- cfe/trunk/lib/Sema/SemaLambda.cpp
+++ cfe/trunk/lib/Sema/SemaLambda.cpp
@@ -904,6 +904,10 @@
 ParamInfo.getDeclSpec().isConstexprSpecified());
   if (ExplicitParams)
 CheckCXXDefaultArguments(Method);
+
+  // This represents the function body for the lambda function, check if we
+  // have to apply optnone due to a pragma.
+  AddRangeBasedOptnone(Method);
   
   // Attributes on the lambda apply to the method.  
   ProcessDeclAttributes(CurScope, Method, ParamInfo);


Index: cfe/trunk/test/CodeGenCXX/optnone-pragma-optimize-off.cpp
===
--- cfe/trunk/test/CodeGenCXX/optnone-pragma-optimize-off.cpp
+++ cfe/trunk/test/CodeGenCXX/optnone-pragma-optimize-off.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 %s -triple %itanium_abi_triple -O1 -disable-llvm-passes -emit-llvm -o - | FileCheck %s
+
+// Test the attributes for the lambda function contains 'optnone' as result of
+// the _Pragma("clang optimize off").
+
+_Pragma("clang optimize off")
+
+void foo(int p) {
+  auto lambda = [&p]() { ++p; };
+  lambda();
+  // CHECK: define {{.*}} @"_ZZ3fooiENK3$_0clEv"({{.*}}) #[[LAMBDA_ATR:[0-9]+]]
+}
+
+_Pragma("clang optimize on")
+
+// CHECK: attributes #[[LAMBDA_ATR]] = { {{.*}} optnone {{.*}} }
\ No newline at end of file
Index: cfe/trunk/lib/Sema/SemaLambda.cpp
===
--- cfe/trunk/lib/Sema/SemaLambda.cpp
+++ cfe/trunk/lib/Sema/SemaLambda.cpp
@@ -904,6 +904,10 @@
 ParamInfo.getDeclSpec().isConstexprSpecified());
   if (ExplicitParams)
 CheckCXXDefaultArguments(Method);
+
+  // This represents the function body for the lambda function, check if we
+  // have to apply optnone due to a pragma.
+  AddRangeBasedOptnone(Method);
   
   // Attributes on the lambda apply to the method.  
   ProcessDeclAttributes(CurScope, Method, ParamInfo);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

2018-03-27 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

In https://reviews.llvm.org/D44826#1046906, @dblaikie wrote:

>


First of all, thanks very much for your feedback.

> While implementing the warning is great (wonder if there's any codebase
>  that isn't -Wunused-using clean, that we could use to compare Clang and
>  GCC's behavior broadly - make sure it's catching the same cases (or
>  justify/investigate differences)) - and using it to motivate the debug info
>  is an improvement to the debug info - it won't quite address all the wasted
>  debug info, unfortunately :/

You are making a very good point. The -Wunused-using is just addressing
one of the factors affecting the debug info size.

> Consider this:
> 
> namespace a {
> 
>   struct b;
> 
> };
>  namespace x {
> 
>   using a::b;
>   inline void f(b*) {
>   }
> 
> }
> 
> Now the using declaration is used, but if 'f' is never called in this
>  translation unit, it's a bit weird to produce debug info for the using decl
>  and could still substantially bloat debug info. (indeed most of the bloat
>  that the using decl/directive debug info is producing is probably from
>  directives that are used, but not in a way that's relevant to a certain
>  translation unit)

The modified patch catches that 'referenced' but 'unused' using declaration.

> I've not looked at the change yet, but if it's particularly
>  expensive/complicated to wire up the debug info side, it might not be worth
>  it given it's probably not a significant savings & somewhat of a dead-end
>  compared to what would be needed for a more complete fix. But I guess it's
>  probably not expensive/complicated, so probably some fine low hanging fruit
>  to pick until a more complete fix/improvement is implemented.

The warning generation check is done at the point where a lexical scope
is closed and it does not 'see' further changes on the same using after that
point.

The second part (do not generate debug info) uses a similar approach, with
the only difference that the checks and debug info generation is done at
the end of the compilation unit, when more information is available.


Repository:
  rC Clang

https://reviews.llvm.org/D44826



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

2018-03-29 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

@dblaikie,

Thanks for your feedback and questions. I will address them.

Currently I am looking with more detail the tracking of the 'used' and 
'referenced' bits.


Repository:
  rC Clang

https://reviews.llvm.org/D44826



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

2017-10-30 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a reviewer: tberghammer.
CarlosAlbertoEnciso added a comment.

Hi Tamas,

I have added you to this review, as I think I have found something odd with the 
LLDB.

Basically after this intended patch to the compiler (to emit scoped information 
only for scoped enums) the following test fail:

tools/lldb/packages/Python/lldbsuite/test/lang/cpp/template/TestTemplateArgs.py

I have saved the objects generated before and after the change for both 
32/64bits mode.  The test case is checking for scoped information which is 
present in the debug information.

  def test_enum_args(self):
  frame = self.prepareProcess()
  
  # Make sure "member" can be displayed and also used in an expression
  # correctly
  member = frame.FindVariable('member')
  self.assertTrue(
  member.IsValid(),
  'make sure we find a local variabble named "member"')
  
  self.assertTrue(member.GetType().GetName() ==
  'EnumTemplate')

After some debugging, 'TestTemplateArgs.py' fails when retrieving the member 
type name, which is expected to have the 'EnumTemplate'. As 
per my previous comment, that string is present; but the expression 
'member.GetType().GetName()' returns just 'EnumTemplate' causing the 
test to fail.

I would appreciate it very much your feedback.

Thanks


https://reviews.llvm.org/D39239



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

2017-10-31 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Hi Tamas,

Thanks very much for your message.

In https://reviews.llvm.org/D39239#910797, @tberghammer wrote:

> - Can you do a diff of the debug_info dump before and after your change? 
> Understanding what have changed should give us a pretty good clue about the 
> issue.


For this specific case, the debug_info is the same before and after my change, 
as the patch affects only unscoped enums.

  DW_TAG_compile_unit "main.cpp"
DW_AT_producer "clang version 6.0.0 (trunk 316983)"
DW_AT_comp_dir 
"/home/carlos/llvm-root/llvm/tools/lldb/packages/Python/lldbsuite/test/lang/cpp/template"
...
DW_TAG_enumeration_type "EnumType"
  DW_AT_enum_class DW_FORM_flag_present
  DW_TAG_enumerator "Member"
  DW_TAG_enumerator "Subclass"
  ...
DW_TAG_class_type "EnumTemplate"
  ...
DW_TAG_class_type "EnumTemplate"
  ...

The DW_AT_name string for the templates are correctly generated, including the 
scoped information.

> - My first guess is that after your change we emit DW_TAG_enumeration_type 
> for scoped enums (what seems to be the correct thing to do) instead of 
> something else (not sure what) and you have to update 
> https://github.com/llvm-mirror/lldb/blob/master/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp#L1512
>  to handle it correctly.

Thanks for the link.

Looking at:
https://github.com/llvm-mirror/lldb/blob/master/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp#L1512

  const char *DWARFDebugInfoEntry::GetQualifiedName(
  SymbolFileDWARF *dwarf2Data, DWARFCompileUnit *cu,
  const DWARFAttributes &attributes, std::string &storage) const {
  
const char *name = GetName(dwarf2Data, cu);
...
return storage.c_str();
  }

The values for 'name' and 'storage' are correct and include the full qualified 
name: 'EnumTemplate'.

https://github.com/llvm-mirror/lldb/blob/master/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L926

During the processing of DW_TAG_enumeration_type, it seems that 
'DW_AT_enum_class' is not processed.

  DW_TAG_enumeration_type "EnumType"
...
DW_AT_enum_class DW_FORM_flag_present  
...
DW_TAG_enumerator "Member"
DW_TAG_enumerator "Subclass"

which at the end can have an impact on the name generated for the enumerators 
to be:

"EnumTemplate"

or

"EnumTemplate"

Thanks.


https://reviews.llvm.org/D39239



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits