[PATCH] D83992: [ASTImporter] Add Visitor for TypedefNameDecl's

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf761acfb1a73: [ASTImporter] Add Visitor for 
TypedefNameDecls (authored by vabridgers, committed by einvbri 
vince.a.bridg...@ericsson.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83992

Files:
  clang/lib/AST/ASTImporterLookupTable.cpp
  clang/test/Analysis/Inputs/ctu-import.c
  clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/ctu-implicit.c


Index: clang/test/Analysis/ctu-implicit.c
===
--- /dev/null
+++ clang/test/Analysis/ctu-implicit.c
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir2
+// RUN: %clang_cc1  \
+// RUN:   -emit-pch -o %t/ctudir2/ctu-import.c.ast %S/Inputs/ctu-import.c
+// RUN: cp %S/Inputs/ctu-import.c.externalDefMap.ast-dump.txt 
%t/ctudir2/externalDefMap.txt
+// RUN: %clang_cc1 -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config  display-ctu-progress=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir2 \
+// RUN:   -verify %s
+
+void clang_analyzer_eval(int);
+
+int testStaticImplicit(void);
+int func(void) {
+  int ret = testStaticImplicit();
+  clang_analyzer_eval(ret == 4); // expected-warning{{TRUE}}
+  return testStaticImplicit();
+}
Index: clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
@@ -0,0 +1 @@
+c:@F@testStaticImplicit ctu-import.c.ast
Index: clang/test/Analysis/Inputs/ctu-import.c
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-import.c
@@ -0,0 +1,15 @@
+
+// Use an internal, implicitly defined type, called by
+// a function imported for CTU. This should not crash.
+int foo(void);
+int foobar(int skip) {
+  __NSConstantString str = {.flags = 1};
+
+  if (str.flags >= 0)
+str.flags = 0;
+  return 4;
+}
+
+int testStaticImplicit(void) {
+  return foobar(3);
+}
Index: clang/lib/AST/ASTImporterLookupTable.cpp
===
--- clang/lib/AST/ASTImporterLookupTable.cpp
+++ clang/lib/AST/ASTImporterLookupTable.cpp
@@ -22,6 +22,20 @@
 struct Builder : RecursiveASTVisitor {
   ASTImporterLookupTable 
   Builder(ASTImporterLookupTable ) : LT(LT) {}
+
+  bool VisitTypedefNameDecl(TypedefNameDecl *D) {
+QualType Ty = D->getUnderlyingType();
+Ty = Ty.getCanonicalType();
+if (const auto *RTy = dyn_cast(Ty)) {
+  LT.add(RTy->getAsRecordDecl());
+  // iterate over the field decls, adding them
+  for (auto *it : RTy->getAsRecordDecl()->fields()) {
+LT.add(it);
+  }
+}
+return true;
+  }
+
   bool VisitNamedDecl(NamedDecl *D) {
 LT.add(D);
 return true;


Index: clang/test/Analysis/ctu-implicit.c
===
--- /dev/null
+++ clang/test/Analysis/ctu-implicit.c
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir2
+// RUN: %clang_cc1  \
+// RUN:   -emit-pch -o %t/ctudir2/ctu-import.c.ast %S/Inputs/ctu-import.c
+// RUN: cp %S/Inputs/ctu-import.c.externalDefMap.ast-dump.txt %t/ctudir2/externalDefMap.txt
+// RUN: %clang_cc1 -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config  display-ctu-progress=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir2 \
+// RUN:   -verify %s
+
+void clang_analyzer_eval(int);
+
+int testStaticImplicit(void);
+int func(void) {
+  int ret = testStaticImplicit();
+  clang_analyzer_eval(ret == 4); // expected-warning{{TRUE}}
+  return testStaticImplicit();
+}
Index: clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
@@ -0,0 +1 @@
+c:@F@testStaticImplicit ctu-import.c.ast
Index: clang/test/Analysis/Inputs/ctu-import.c
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-import.c
@@ -0,0 +1,15 @@
+
+// Use an internal, implicitly defined type, called by
+// a function imported for CTU. This should not crash.
+int foo(void);
+int foobar(int skip) {
+  __NSConstantString str = {.flags = 1};
+
+  if (str.flags >= 0)
+str.flags = 0;
+  return 4;
+}
+
+int testStaticImplicit(void) {
+  return foobar(3);
+}
Index: clang/lib/AST/ASTImporterLookupTable.cpp
===
--- clang/lib/AST/ASTImporterLookupTable.cpp
+++ 

[PATCH] D83992: [ASTImporter] Add Visitor for TypedefNameDecl's

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

LGTM! Thanks for the test!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83992

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

[PATCH] D83992: [ASTImporter] Add Visitor for TypedefNameDecl's

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers updated this revision to Diff 280700.
vabridgers added a comment.

Adding negative test case that exposes the original problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83992

Files:
  clang/lib/AST/ASTImporterLookupTable.cpp
  clang/test/Analysis/Inputs/ctu-import.c
  clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/ctu-implicit.c


Index: clang/test/Analysis/ctu-implicit.c
===
--- /dev/null
+++ clang/test/Analysis/ctu-implicit.c
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir2
+// RUN: %clang_cc1  \
+// RUN:   -emit-pch -o %t/ctudir2/ctu-import.c.ast %S/Inputs/ctu-import.c
+// RUN: cp %S/Inputs/ctu-import.c.externalDefMap.ast-dump.txt 
%t/ctudir2/externalDefMap.txt
+// RUN: %clang_cc1 -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config  display-ctu-progress=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir2 \
+// RUN:   -verify %s
+
+void clang_analyzer_eval(int);
+
+int testStaticImplicit(void);
+int func(void) {
+  int ret = testStaticImplicit();
+  clang_analyzer_eval(ret == 4); // expected-warning{{TRUE}}
+  return testStaticImplicit();
+}
Index: clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
@@ -0,0 +1 @@
+c:@F@testStaticImplicit ctu-import.c.ast
Index: clang/test/Analysis/Inputs/ctu-import.c
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-import.c
@@ -0,0 +1,15 @@
+
+// Use an internal, implicitly defined type, called by
+// a function imported for CTU. This should not crash.
+int foo(void);
+int foobar(int skip) {
+  __NSConstantString str = {.flags = 1};
+
+  if (str.flags >= 0)
+str.flags = 0;
+  return 4;
+}
+
+int testStaticImplicit(void) {
+  return foobar(3);
+}
Index: clang/lib/AST/ASTImporterLookupTable.cpp
===
--- clang/lib/AST/ASTImporterLookupTable.cpp
+++ clang/lib/AST/ASTImporterLookupTable.cpp
@@ -22,6 +22,20 @@
 struct Builder : RecursiveASTVisitor {
   ASTImporterLookupTable 
   Builder(ASTImporterLookupTable ) : LT(LT) {}
+
+  bool VisitTypedefNameDecl(TypedefNameDecl *D) {
+QualType Ty = D->getUnderlyingType();
+Ty = Ty.getCanonicalType();
+if (const auto *RTy = dyn_cast(Ty)) {
+  LT.add(RTy->getAsRecordDecl());
+  // iterate over the field decls, adding them
+  for (auto *it : RTy->getAsRecordDecl()->fields()) {
+LT.add(it);
+  }
+}
+return true;
+  }
+
   bool VisitNamedDecl(NamedDecl *D) {
 LT.add(D);
 return true;


Index: clang/test/Analysis/ctu-implicit.c
===
--- /dev/null
+++ clang/test/Analysis/ctu-implicit.c
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir2
+// RUN: %clang_cc1  \
+// RUN:   -emit-pch -o %t/ctudir2/ctu-import.c.ast %S/Inputs/ctu-import.c
+// RUN: cp %S/Inputs/ctu-import.c.externalDefMap.ast-dump.txt %t/ctudir2/externalDefMap.txt
+// RUN: %clang_cc1 -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config  display-ctu-progress=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir2 \
+// RUN:   -verify %s
+
+void clang_analyzer_eval(int);
+
+int testStaticImplicit(void);
+int func(void) {
+  int ret = testStaticImplicit();
+  clang_analyzer_eval(ret == 4); // expected-warning{{TRUE}}
+  return testStaticImplicit();
+}
Index: clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
@@ -0,0 +1 @@
+c:@F@testStaticImplicit ctu-import.c.ast
Index: clang/test/Analysis/Inputs/ctu-import.c
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-import.c
@@ -0,0 +1,15 @@
+
+// Use an internal, implicitly defined type, called by
+// a function imported for CTU. This should not crash.
+int foo(void);
+int foobar(int skip) {
+  __NSConstantString str = {.flags = 1};
+
+  if (str.flags >= 0)
+str.flags = 0;
+  return 4;
+}
+
+int testStaticImplicit(void) {
+  return foobar(3);
+}
Index: clang/lib/AST/ASTImporterLookupTable.cpp
===
--- clang/lib/AST/ASTImporterLookupTable.cpp
+++ clang/lib/AST/ASTImporterLookupTable.cpp
@@ -22,6 +22,20 @@
 struct Builder : RecursiveASTVisitor 

[PATCH] D83992: [ASTImporter] Add Visitor for TypedefNameDecl's

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers added a comment.

I discussed with Gabor. We don't feel comfortable landing this without a 
covering negative test case, so I'll work on that a little more, see what I can 
come up with. Thanks Gabor!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83992



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

[PATCH] D83992: [ASTImporter] Add Visitor for TypedefNameDecl's

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers updated this revision to Diff 278630.
vabridgers added a comment.

Fix lint pre-merge check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83992

Files:
  clang/lib/AST/ASTImporterLookupTable.cpp


Index: clang/lib/AST/ASTImporterLookupTable.cpp
===
--- clang/lib/AST/ASTImporterLookupTable.cpp
+++ clang/lib/AST/ASTImporterLookupTable.cpp
@@ -22,6 +22,20 @@
 struct Builder : RecursiveASTVisitor {
   ASTImporterLookupTable 
   Builder(ASTImporterLookupTable ) : LT(LT) {}
+
+  bool VisitTypedefNameDecl(TypedefNameDecl *D) {
+QualType Ty = D->getUnderlyingType();
+Ty = Ty.getCanonicalType();
+if (const auto *RTy = dyn_cast(Ty)) {
+  LT.add(RTy->getAsRecordDecl());
+  // iterate over the field decls, adding them
+  for (auto *it : RTy->getAsRecordDecl()->fields()) {
+LT.add(it);
+  }
+}
+return true;
+  }
+
   bool VisitNamedDecl(NamedDecl *D) {
 LT.add(D);
 return true;


Index: clang/lib/AST/ASTImporterLookupTable.cpp
===
--- clang/lib/AST/ASTImporterLookupTable.cpp
+++ clang/lib/AST/ASTImporterLookupTable.cpp
@@ -22,6 +22,20 @@
 struct Builder : RecursiveASTVisitor {
   ASTImporterLookupTable 
   Builder(ASTImporterLookupTable ) : LT(LT) {}
+
+  bool VisitTypedefNameDecl(TypedefNameDecl *D) {
+QualType Ty = D->getUnderlyingType();
+Ty = Ty.getCanonicalType();
+if (const auto *RTy = dyn_cast(Ty)) {
+  LT.add(RTy->getAsRecordDecl());
+  // iterate over the field decls, adding them
+  for (auto *it : RTy->getAsRecordDecl()->fields()) {
+LT.add(it);
+  }
+}
+return true;
+  }
+
   bool VisitNamedDecl(NamedDecl *D) {
 LT.add(D);
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83992: [ASTImporter] Add Visitor for TypedefNameDecl's

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers created this revision.
vabridgers added a reviewer: martong.
Herald added subscribers: cfe-commits, teemperor, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.

We found a case where Typedef Name Declarations were not being added
correctly when importing builtin types. This exposed the need for a
TypedefNameDecl visitor so these types can be added by RecordDecl and
fields.

This code is covered by the ASTImporterTest cases that use the implicit
struct __NSConstantString_tag definitions.

Thanks to @martong for the debugging assist!

Depends on D83970 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83992

Files:
  clang/lib/AST/ASTImporterLookupTable.cpp


Index: clang/lib/AST/ASTImporterLookupTable.cpp
===
--- clang/lib/AST/ASTImporterLookupTable.cpp
+++ clang/lib/AST/ASTImporterLookupTable.cpp
@@ -22,6 +22,20 @@
 struct Builder : RecursiveASTVisitor {
   ASTImporterLookupTable 
   Builder(ASTImporterLookupTable ) : LT(LT) {}
+
+  bool VisitTypedefNameDecl(TypedefNameDecl *D) {
+QualType Ty = D->getUnderlyingType();
+Ty = Ty.getCanonicalType();
+if (const auto *RTy = dyn_cast(Ty)) {
+  LT.add(RTy->getAsRecordDecl());
+  // iterate over the field decls, adding them
+  for (auto it : RTy->getAsRecordDecl()->fields()) {
+LT.add(it);
+  }
+}
+return true;
+  }
+
   bool VisitNamedDecl(NamedDecl *D) {
 LT.add(D);
 return true;


Index: clang/lib/AST/ASTImporterLookupTable.cpp
===
--- clang/lib/AST/ASTImporterLookupTable.cpp
+++ clang/lib/AST/ASTImporterLookupTable.cpp
@@ -22,6 +22,20 @@
 struct Builder : RecursiveASTVisitor {
   ASTImporterLookupTable 
   Builder(ASTImporterLookupTable ) : LT(LT) {}
+
+  bool VisitTypedefNameDecl(TypedefNameDecl *D) {
+QualType Ty = D->getUnderlyingType();
+Ty = Ty.getCanonicalType();
+if (const auto *RTy = dyn_cast(Ty)) {
+  LT.add(RTy->getAsRecordDecl());
+  // iterate over the field decls, adding them
+  for (auto it : RTy->getAsRecordDecl()->fields()) {
+LT.add(it);
+  }
+}
+return true;
+  }
+
   bool VisitNamedDecl(NamedDecl *D) {
 LT.add(D);
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits