[PATCH] D137077: [Diagnostic] Clarify -Winfinite-recursion message

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat created this revision.
Herald added a project: All.
merrymeerkat requested review of this revision.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137077

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/SemaCXX/warn-infinite-recursion.cpp

Index: clang/test/SemaCXX/warn-infinite-recursion.cpp
===
--- clang/test/SemaCXX/warn-infinite-recursion.cpp
+++ clang/test/SemaCXX/warn-infinite-recursion.cpp
@@ -1,10 +1,10 @@
 // RUN: %clang_cc1 %s -fsyntax-only -verify -Winfinite-recursion
 
-void a() {  // expected-warning{{call itself}}
+void a() {  // expected-warning{{to understand recursion}}
   a();
 }
 
-void b(int x) {  // expected-warning{{call itself}}
+void b(int x) {  // expected-warning{{to understand recursion}}
   if (x)
 b(x);
   else
@@ -16,7 +16,7 @@
 c(5);
 }
 
-void d(int x) {  // expected-warning{{call itself}}
+void d(int x) {  // expected-warning{{to understand recursion}}
   if (x)
 ++x;
   return d(x);
@@ -29,7 +29,7 @@
 void e() { f(); }
 void f() { e(); }
 
-void g() {  // expected-warning{{call itself}}
+void g() {  // expected-warning{{to understand recursion}}
   while (true)
 g();
 
@@ -42,14 +42,14 @@
   }
 }
 
-void i(int x) {  // expected-warning{{call itself}}
+void i(int x) {  // expected-warning{{to understand recursion}}
   while (x < 5) {
 --x;
   }
   i(0);
 }
 
-int j() {  // expected-warning{{call itself}}
+int j() {  // expected-warning{{to understand recursion}}
   return 5 + j();
 }
 
@@ -80,11 +80,11 @@
   void b();
 };
 
-void S::a() {  // expected-warning{{call itself}}
+void S::a() {  // expected-warning{{to understand recursion}}
   return a();
 }
 
-void S::b() {  // expected-warning{{call itself}}
+void S::b() {  // expected-warning{{to understand recursion}}
   int i = 0;
   do {
 ++i;
@@ -95,8 +95,8 @@
 template
 struct T {
   member m;
-  void a() { return a(); }  // expected-warning{{call itself}}
-  static void b() { return b(); }  // expected-warning{{call itself}}
+  void a() { return a(); }  // expected-warning{{to understand recursion}}
+  static void b() { return b(); }  // expected-warning{{to understand recursion}}
 };
 
 void test_T() {
@@ -107,13 +107,13 @@
 
 class U {
   U* u;
-  void Fun() {  // expected-warning{{call itself}}
+  void Fun() {  // expected-warning{{to understand recursion}}
 u->Fun();
   }
 };
 
 // No warnings on templated functions
-// sum<0>() is instantiated, does recursively call itself, but never runs.
+// sum<0>() is instantiated, does recursively to understand recursion, but never runs.
 template 
 int sum() {
   return value + sum();
@@ -157,7 +157,7 @@
   return 0;
 return Wrapper::run();
   }
-  static int run2() {  // expected-warning{{call itself}}
+  static int run2() {  // expected-warning{{to understand recursion}}
 return run2();
   }
 };
@@ -194,7 +194,7 @@
 };
 
 Q q;
-Q &evaluated_recursive_function(int x) { // expected-warning{{call itself}}
+Q &evaluated_recursive_function(int x) { // expected-warning{{to understand recursion}}
   (void)typeid(evaluated_recursive_function(x)); // expected-warning {{expression with side effects will be evaluated despite being used as an operand to 'typeid'}}
   return q;
 }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -60,7 +60,7 @@
   "remove call to max function and unsigned zero argument">;
 
 def warn_infinite_recursive_function : Warning<
-  "all paths through this function will call itself">,
+  "in order to understand recursion, you must first understand recursion">,
   InGroup, DefaultIgnore;
 
 def warn_comma_operator : Warning<"possible misuse of comma operator here">,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137077: [Diagnostic] Clarify -Winfinite-recursion message

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat added a comment.

In D137077#3896369 , @aaron.ballman 
wrote:

> Thank you for the patch! However, I think this isn't an improvement to the 
> diagnostic -- it's a cute phrase, but it doesn't help the programmer to 
> understand what about their code needs to change.

Hi Aaron! Agreed--thank you for the review! I was just following a tutorial on 
how to make changes to LLVM, so this was not a real code review :)
Cheers!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137077

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

[PATCH] D140475: BEGIN_PUBLIC test phabricator

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat created this revision.
Herald added a reviewer: NoQ.
Herald added a project: All.
merrymeerkat requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

END_PUBLIC


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140475

Files:
  clang/include/clang/Analysis/AnalysisDiagnostic.h


Index: clang/include/clang/Analysis/AnalysisDiagnostic.h
===
--- clang/include/clang/Analysis/AnalysisDiagnostic.h
+++ clang/include/clang/Analysis/AnalysisDiagnostic.h
@@ -9,6 +9,8 @@
 #ifndef LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
 #define LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
 
+// why am i here
+//
 #include "clang/Basic/DiagnosticAnalysis.h"
 
 #endif


Index: clang/include/clang/Analysis/AnalysisDiagnostic.h
===
--- clang/include/clang/Analysis/AnalysisDiagnostic.h
+++ clang/include/clang/Analysis/AnalysisDiagnostic.h
@@ -9,6 +9,8 @@
 #ifndef LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
 #define LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
 
+// why am i here
+//
 #include "clang/Basic/DiagnosticAnalysis.h"
 
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140475: BEGIN_PUBLIC test phabricator

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat updated this revision to Diff 484554.
merrymeerkat added a comment.

?:


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140475

Files:
  clang/include/clang/Analysis/AnalysisDiagnostic.h


Index: clang/include/clang/Analysis/AnalysisDiagnostic.h
===
--- clang/include/clang/Analysis/AnalysisDiagnostic.h
+++ clang/include/clang/Analysis/AnalysisDiagnostic.h
@@ -9,6 +9,8 @@
 #ifndef LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
 #define LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
 
+// why am i here ??
+
 #include "clang/Basic/DiagnosticAnalysis.h"
 
 #endif


Index: clang/include/clang/Analysis/AnalysisDiagnostic.h
===
--- clang/include/clang/Analysis/AnalysisDiagnostic.h
+++ clang/include/clang/Analysis/AnalysisDiagnostic.h
@@ -9,6 +9,8 @@
 #ifndef LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
 #define LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
 
+// why am i here ??
+
 #include "clang/Basic/DiagnosticAnalysis.h"
 
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140483: Remove empty header file.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat created this revision.
Herald added a reviewer: NoQ.
Herald added a project: All.
merrymeerkat requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140483

Files:
  clang/docs/tools/clang-formatted-files.txt
  clang/include/clang/Analysis/AnalysisDiagnostic.h
  clang/include/clang/module.modulemap


Index: clang/include/clang/module.modulemap
===
--- clang/include/clang/module.modulemap
+++ clang/include/clang/module.modulemap
@@ -94,7 +94,6 @@
   requires cplusplus
 
   module All { header "Basic/AllDiagnostics.h" export * }
-  module Analysis { header "Analysis/AnalysisDiagnostic.h" export * }
   module AST { header "AST/ASTDiagnostic.h" export * }
   module Comment { header "AST/CommentDiagnostic.h" export * }
   module Driver { header "Driver/DriverDiagnostic.h" export * }
Index: clang/include/clang/Analysis/AnalysisDiagnostic.h
===
--- clang/include/clang/Analysis/AnalysisDiagnostic.h
+++ /dev/null
@@ -1,14 +0,0 @@
-//===--- DiagnosticAnalysis.h - Diagnostics for libanalysis -*- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
-#define LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
-
-#include "clang/Basic/DiagnosticAnalysis.h"
-
-#endif
Index: clang/docs/tools/clang-formatted-files.txt
===
--- clang/docs/tools/clang-formatted-files.txt
+++ clang/docs/tools/clang-formatted-files.txt
@@ -116,7 +116,6 @@
 clang/examples/Attribute/Attribute.cpp
 clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
 clang/examples/PluginsOrder/PluginsOrder.cpp
-clang/include/clang/Analysis/AnalysisDiagnostic.h
 clang/include/clang/Analysis/BodyFarm.h
 clang/include/clang/Analysis/IssueHash.h
 clang/include/clang/Analysis/MacroExpansionContext.h


Index: clang/include/clang/module.modulemap
===
--- clang/include/clang/module.modulemap
+++ clang/include/clang/module.modulemap
@@ -94,7 +94,6 @@
   requires cplusplus
 
   module All { header "Basic/AllDiagnostics.h" export * }
-  module Analysis { header "Analysis/AnalysisDiagnostic.h" export * }
   module AST { header "AST/ASTDiagnostic.h" export * }
   module Comment { header "AST/CommentDiagnostic.h" export * }
   module Driver { header "Driver/DriverDiagnostic.h" export * }
Index: clang/include/clang/Analysis/AnalysisDiagnostic.h
===
--- clang/include/clang/Analysis/AnalysisDiagnostic.h
+++ /dev/null
@@ -1,14 +0,0 @@
-//===--- DiagnosticAnalysis.h - Diagnostics for libanalysis -*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
-#define LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
-
-#include "clang/Basic/DiagnosticAnalysis.h"
-
-#endif
Index: clang/docs/tools/clang-formatted-files.txt
===
--- clang/docs/tools/clang-formatted-files.txt
+++ clang/docs/tools/clang-formatted-files.txt
@@ -116,7 +116,6 @@
 clang/examples/Attribute/Attribute.cpp
 clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
 clang/examples/PluginsOrder/PluginsOrder.cpp
-clang/include/clang/Analysis/AnalysisDiagnostic.h
 clang/include/clang/Analysis/BodyFarm.h
 clang/include/clang/Analysis/IssueHash.h
 clang/include/clang/Analysis/MacroExpansionContext.h
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140483: Remove empty header file.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe0fa7c730d01: Remove empty header file. (authored by 
merrymeerkat).

Changed prior to commit:
  https://reviews.llvm.org/D140483?vs=484578&id=485075#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140483

Files:
  clang/docs/tools/clang-formatted-files.txt
  clang/include/clang/Analysis/AnalysisDiagnostic.h
  clang/include/clang/module.modulemap


Index: clang/include/clang/module.modulemap
===
--- clang/include/clang/module.modulemap
+++ clang/include/clang/module.modulemap
@@ -94,10 +94,7 @@
   requires cplusplus
 
   module All { header "Basic/AllDiagnostics.h" export * }
-  module Analysis {
-header "Analysis/AnalysisDiagnostic.h" export *
-textual header "Analysis/Analyses/UnsafeBufferUsageGadgets.def"
-  }
+  module Analysis { textual header 
"Analysis/Analyses/UnsafeBufferUsageGadgets.def" }
   module AST { header "AST/ASTDiagnostic.h" export * }
   module Comment { header "AST/CommentDiagnostic.h" export * }
   module Driver { header "Driver/DriverDiagnostic.h" export * }
Index: clang/include/clang/Analysis/AnalysisDiagnostic.h
===
--- clang/include/clang/Analysis/AnalysisDiagnostic.h
+++ /dev/null
@@ -1,14 +0,0 @@
-//===--- DiagnosticAnalysis.h - Diagnostics for libanalysis -*- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
-#define LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
-
-#include "clang/Basic/DiagnosticAnalysis.h"
-
-#endif
Index: clang/docs/tools/clang-formatted-files.txt
===
--- clang/docs/tools/clang-formatted-files.txt
+++ clang/docs/tools/clang-formatted-files.txt
@@ -116,7 +116,6 @@
 clang/examples/Attribute/Attribute.cpp
 clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
 clang/examples/PluginsOrder/PluginsOrder.cpp
-clang/include/clang/Analysis/AnalysisDiagnostic.h
 clang/include/clang/Analysis/BodyFarm.h
 clang/include/clang/Analysis/IssueHash.h
 clang/include/clang/Analysis/MacroExpansionContext.h


Index: clang/include/clang/module.modulemap
===
--- clang/include/clang/module.modulemap
+++ clang/include/clang/module.modulemap
@@ -94,10 +94,7 @@
   requires cplusplus
 
   module All { header "Basic/AllDiagnostics.h" export * }
-  module Analysis {
-header "Analysis/AnalysisDiagnostic.h" export *
-textual header "Analysis/Analyses/UnsafeBufferUsageGadgets.def"
-  }
+  module Analysis { textual header "Analysis/Analyses/UnsafeBufferUsageGadgets.def" }
   module AST { header "AST/ASTDiagnostic.h" export * }
   module Comment { header "AST/CommentDiagnostic.h" export * }
   module Driver { header "Driver/DriverDiagnostic.h" export * }
Index: clang/include/clang/Analysis/AnalysisDiagnostic.h
===
--- clang/include/clang/Analysis/AnalysisDiagnostic.h
+++ /dev/null
@@ -1,14 +0,0 @@
-//===--- DiagnosticAnalysis.h - Diagnostics for libanalysis -*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
-#define LLVM_CLANG_ANALYSIS_ANALYSISDIAGNOSTIC_H
-
-#include "clang/Basic/DiagnosticAnalysis.h"
-
-#endif
Index: clang/docs/tools/clang-formatted-files.txt
===
--- clang/docs/tools/clang-formatted-files.txt
+++ clang/docs/tools/clang-formatted-files.txt
@@ -116,7 +116,6 @@
 clang/examples/Attribute/Attribute.cpp
 clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
 clang/examples/PluginsOrder/PluginsOrder.cpp
-clang/include/clang/Analysis/AnalysisDiagnostic.h
 clang/include/clang/Analysis/BodyFarm.h
 clang/include/clang/Analysis/IssueHash.h
 clang/include/clang/Analysis/MacroExpansionContext.h
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140626: [clang][nullability] Remove old overload for getNullability()

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat created this revision.
Herald added a project: All.
merrymeerkat requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140626

Files:
  clang/include/clang/AST/Type.h
  clang/lib/AST/Type.cpp


Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -4157,10 +4157,6 @@
   }
   return std::nullopt;
 }
-// TODO: Remove overload.
-Optional Type::getNullability(const ASTContext &) const {
-  return getNullability();
-}
 
 bool Type::canHaveNullability(bool ResultIfUnknown) const {
   QualType type = getCanonicalTypeInternal();
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -2548,8 +2548,6 @@
   /// system, not as part of the canonical type, so nullability will
   /// be lost by canonicalization and desugaring.
   Optional getNullability() const;
-  // TODO: Remove overload.
-  Optional getNullability(const ASTContext &) const;
 
   /// Determine whether the given type can have a nullability
   /// specifier applied to it, i.e., if it is any kind of pointer type.


Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -4157,10 +4157,6 @@
   }
   return std::nullopt;
 }
-// TODO: Remove overload.
-Optional Type::getNullability(const ASTContext &) const {
-  return getNullability();
-}
 
 bool Type::canHaveNullability(bool ResultIfUnknown) const {
   QualType type = getCanonicalTypeInternal();
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -2548,8 +2548,6 @@
   /// system, not as part of the canonical type, so nullability will
   /// be lost by canonicalization and desugaring.
   Optional getNullability() const;
-  // TODO: Remove overload.
-  Optional getNullability(const ASTContext &) const;
 
   /// Determine whether the given type can have a nullability
   /// specifier applied to it, i.e., if it is any kind of pointer type.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140626: [clang][nullability] Remove old overload for getNullability()

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfbee2dd9a4ba: [clang][nullability] Remove old overload for 
getNullability() (authored by merrymeerkat).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140626

Files:
  clang/include/clang/AST/Type.h
  clang/lib/AST/Type.cpp


Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -4157,10 +4157,6 @@
   }
   return std::nullopt;
 }
-// TODO: Remove overload.
-Optional Type::getNullability(const ASTContext &) const {
-  return getNullability();
-}
 
 bool Type::canHaveNullability(bool ResultIfUnknown) const {
   QualType type = getCanonicalTypeInternal();
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -2548,8 +2548,6 @@
   /// system, not as part of the canonical type, so nullability will
   /// be lost by canonicalization and desugaring.
   Optional getNullability() const;
-  // TODO: Remove overload.
-  Optional getNullability(const ASTContext &) const;
 
   /// Determine whether the given type can have a nullability
   /// specifier applied to it, i.e., if it is any kind of pointer type.


Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -4157,10 +4157,6 @@
   }
   return std::nullopt;
 }
-// TODO: Remove overload.
-Optional Type::getNullability(const ASTContext &) const {
-  return getNullability();
-}
 
 bool Type::canHaveNullability(bool ResultIfUnknown) const {
   QualType type = getCanonicalTypeInternal();
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -2548,8 +2548,6 @@
   /// system, not as part of the canonical type, so nullability will
   /// be lost by canonicalization and desugaring.
   Optional getNullability() const;
-  // TODO: Remove overload.
-  Optional getNullability(const ASTContext &) const;
 
   /// Determine whether the given type can have a nullability
   /// specifier applied to it, i.e., if it is any kind of pointer type.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
merrymeerkat requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a straightfoward way to handle unions in dataflow analysis. Without 
this change, nullability verification crashes on files that contain unions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140696

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1518,6 +1518,54 @@
   });
 }
 
+TEST(TransferTest, UnionThisMember) {
+  std::string Code = R"(
+union A {
+  int Foo;
+  double Bar;
+
+  void target() {
+(void)0; // [[p]]
+  }
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const auto *FooLoc =
+cast(&ThisLoc->getChild(*FooDecl));
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
+
+const Value *FooVal = Env.getValue(*FooLoc);
+// TODO: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(FooVal));
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+// TODO: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(BarVal));
+  });
+}
+
 TEST(TransferTest, StructThisInLambda) {
   std::string ThisCaptureCode = R"(
 struct A {
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -225,12 +225,9 @@
 
 if (MethodDecl && !MethodDecl->isStatic()) {
   QualType ThisPointeeType = MethodDecl->getThisObjectType();
-  // FIXME: Add support for union types.
-  if (!ThisPointeeType->isUnionType()) {
-ThisPointeeLoc = &createStorageLocation(ThisPointeeType);
-if (Value *ThisPointeeVal = createValue(ThisPointeeType))
-  setValue(*ThisPointeeLoc, *ThisPointeeVal);
-  }
+  ThisPointeeLoc = &createStorageLocation(ThisPointeeType);
+  if (Value *ThisPointeeVal = createValue(ThisPointeeType))
+setValue(*ThisPointeeLoc, *ThisPointeeVal);
 }
   }
 }


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1518,6 +1518,54 @@
   });
 }
 
+TEST(TransferTest, UnionThisMember) {
+  std::string Code = R"(
+union A {
+  int Foo;
+  double Bar;
+
+  void target() {
+(void)0; // [[p]]
+  }
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const auto *FooLoc =
+cast(&ThisLoc->getChild(*FooDecl));
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
+
+const Value *FooVal = Env.getValue(*FooLoc);
+// TODO: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(FooVal));
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+// TODO: Initialise values inside unions, the

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1551-1552
+const Value *FooVal = Env.getValue(*FooLoc);
+// TODO: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(FooVal));

ymandel wrote:
> Why push this off to another patch?
good point! I was pushing it because this is just a quick fix to avoid a crash, 
and the current changes are sufficient for that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696

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

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat updated this revision to Diff 485407.
merrymeerkat added a comment.

Change TODO to FIXME


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1518,6 +1518,54 @@
   });
 }
 
+TEST(TransferTest, UnionThisMember) {
+  std::string Code = R"(
+union A {
+  int Foo;
+  double Bar;
+
+  void target() {
+(void)0; // [[p]]
+  }
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const auto *FooLoc =
+cast(&ThisLoc->getChild(*FooDecl));
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
+
+const Value *FooVal = Env.getValue(*FooLoc);
+// FIXME: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(FooVal));
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+// FIXME: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(BarVal));
+  });
+}
+
 TEST(TransferTest, StructThisInLambda) {
   std::string ThisCaptureCode = R"(
 struct A {
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -225,12 +225,9 @@
 
 if (MethodDecl && !MethodDecl->isStatic()) {
   QualType ThisPointeeType = MethodDecl->getThisObjectType();
-  // FIXME: Add support for union types.
-  if (!ThisPointeeType->isUnionType()) {
-ThisPointeeLoc = &createStorageLocation(ThisPointeeType);
-if (Value *ThisPointeeVal = createValue(ThisPointeeType))
-  setValue(*ThisPointeeLoc, *ThisPointeeVal);
-  }
+  ThisPointeeLoc = &createStorageLocation(ThisPointeeType);
+  if (Value *ThisPointeeVal = createValue(ThisPointeeType))
+setValue(*ThisPointeeLoc, *ThisPointeeVal);
 }
   }
 }


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1518,6 +1518,54 @@
   });
 }
 
+TEST(TransferTest, UnionThisMember) {
+  std::string Code = R"(
+union A {
+  int Foo;
+  double Bar;
+
+  void target() {
+(void)0; // [[p]]
+  }
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const auto *FooLoc =
+cast(&ThisLoc->getChild(*FooDecl));
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
+
+const Value *FooVal = Env.getValue(*FooLoc);
+// FIXME: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(FooVal));
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+// FIXME: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(BarVal));
+  });
+}
+
 TEST(TransferTest, StructThisInLambda) {
   std::string ThisCaptureCode = R"(
 struct A {
Index: clang/lib/Analysis/FlowSensitiv

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1551-1552
+const Value *FooVal = Env.getValue(*FooLoc);
+// TODO: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(FooVal));

ymandel wrote:
> merrymeerkat wrote:
> > ymandel wrote:
> > > Why push this off to another patch?
> > good point! I was pushing it because this is just a quick fix to avoid a 
> > crash, and the current changes are sufficient for that
> SGTM. Please use FIXME instead of TODO, though. But, glad to see this is 
> enough to avoid the crashing!
> 
> That said, can you expand on where the actual crash was happening? I'm 
> concerned that its possible that the crash site should be more robust and 
> that this patch is hiding that weakness.
Done!

The crash was happening because of a null pointer cast in the builtin transfer 
function for CFGInitializers: 
https://github.com/llvm/llvm-project/blob/781eabeb40b8e47e3a46b0b927784e63f0aad9ab/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp#L316

Hmm so do you think it'd be helpful to add a null check in the file above? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696

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

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1551-1552
+const Value *FooVal = Env.getValue(*FooLoc);
+// TODO: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(FooVal));

ymandel wrote:
> merrymeerkat wrote:
> > ymandel wrote:
> > > merrymeerkat wrote:
> > > > ymandel wrote:
> > > > > Why push this off to another patch?
> > > > good point! I was pushing it because this is just a quick fix to avoid 
> > > > a crash, and the current changes are sufficient for that
> > > SGTM. Please use FIXME instead of TODO, though. But, glad to see this is 
> > > enough to avoid the crashing!
> > > 
> > > That said, can you expand on where the actual crash was happening? I'm 
> > > concerned that its possible that the crash site should be more robust and 
> > > that this patch is hiding that weakness.
> > Done!
> > 
> > The crash was happening because of a null pointer cast in the builtin 
> > transfer function for CFGInitializers: 
> > https://github.com/llvm/llvm-project/blob/781eabeb40b8e47e3a46b0b927784e63f0aad9ab/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp#L316
> > 
> > Hmm so do you think it'd be helpful to add a null check in the file above? 
> Thanks, that's quite helpful! Yes, I think that would be a better fix. It's a 
> matter of perspective (and opinion) so feel free to push back but I'd say 
> that the code you pointed to is buggy -- it assumes the `this` loc is 
> populated, but *by design* it's not populated for unions (I didn't know that 
> unions could ever have `this` so #TIL?).  I think we're admitting that we 
> have a class of initializers for which we knowingly don't create the this 
> pointer and therefore should express that in the code.
> 
> Alternatively, you could argue that the complete class of `this` pointer 
> generating code is structs and unions, so if we just make sure (like your 
> fix) to generate a this pointer in both cases, we can assert it's 
> non-nullness (via the cast) and be done.
> 
> Given that the framework in general is riddled with places where we don't 
> know if a value or storage location or... is initialized and we have to 
> check, I think that's the better (and consistent) approach. Moreover, given 
> that we're not actually adding support for unions, just trying to avoid a 
> crash, I think changing that code site better expresses that intent.
> 
> Still, it's a matter of opinion, so I'll leave it to you to decide.  WDYT?
Thanks for the comment!

What you say makes sense. I guess we introduced the union initialization 
because it could also be useful in the future, but I don't know if it makes 
sense to add a feature that doesn't have any semantic use yet.

If the framework does these kinds of null checks in lots of other places, then 
I agree that it'd be good to have it here too for consistency. I'm leaning 
towards making the change you suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696

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

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat updated this revision to Diff 485485.
merrymeerkat added a comment.

Add FIXME


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1518,6 +1518,54 @@
   });
 }
 
+TEST(TransferTest, UnionThisMember) {
+  std::string Code = R"(
+union A {
+  int Foo;
+  double Bar;
+
+  void target() {
+(void)0; // [[p]]
+  }
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const auto *FooLoc =
+cast(&ThisLoc->getChild(*FooDecl));
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
+
+const Value *FooVal = Env.getValue(*FooLoc);
+// FIXME: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(FooVal));
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+// FIXME: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(BarVal));
+  });
+}
+
 TEST(TransferTest, StructThisInLambda) {
   std::string ThisCaptureCode = R"(
 struct A {
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -223,14 +223,12 @@
 if (Parent->isLambda())
   MethodDecl = dyn_cast(Parent->getDeclContext());
 
+// FIXME: Initialize the ThisPointeeLoc of lambdas too.
 if (MethodDecl && !MethodDecl->isStatic()) {
   QualType ThisPointeeType = MethodDecl->getThisObjectType();
-  // FIXME: Add support for union types.
-  if (!ThisPointeeType->isUnionType()) {
-ThisPointeeLoc = &createStorageLocation(ThisPointeeType);
-if (Value *ThisPointeeVal = createValue(ThisPointeeType))
-  setValue(*ThisPointeeLoc, *ThisPointeeVal);
-  }
+  ThisPointeeLoc = &createStorageLocation(ThisPointeeType);
+  if (Value *ThisPointeeVal = createValue(ThisPointeeType))
+setValue(*ThisPointeeLoc, *ThisPointeeVal);
 }
   }
 }


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1518,6 +1518,54 @@
   });
 }
 
+TEST(TransferTest, UnionThisMember) {
+  std::string Code = R"(
+union A {
+  int Foo;
+  double Bar;
+
+  void target() {
+(void)0; // [[p]]
+  }
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const auto *FooLoc =
+cast(&ThisLoc->getChild(*FooDecl));
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
+
+const Value *FooVal = Env.getValue(*FooLoc);
+// FIXME: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(FooVal));
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+// FIXME: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(BarVal));
+  });
+}

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1551-1552
+const Value *FooVal = Env.getValue(*FooLoc);
+// TODO: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(FooVal));

merrymeerkat wrote:
> ymandel wrote:
> > merrymeerkat wrote:
> > > ymandel wrote:
> > > > merrymeerkat wrote:
> > > > > ymandel wrote:
> > > > > > Why push this off to another patch?
> > > > > good point! I was pushing it because this is just a quick fix to 
> > > > > avoid a crash, and the current changes are sufficient for that
> > > > SGTM. Please use FIXME instead of TODO, though. But, glad to see this 
> > > > is enough to avoid the crashing!
> > > > 
> > > > That said, can you expand on where the actual crash was happening? I'm 
> > > > concerned that its possible that the crash site should be more robust 
> > > > and that this patch is hiding that weakness.
> > > Done!
> > > 
> > > The crash was happening because of a null pointer cast in the builtin 
> > > transfer function for CFGInitializers: 
> > > https://github.com/llvm/llvm-project/blob/781eabeb40b8e47e3a46b0b927784e63f0aad9ab/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp#L316
> > > 
> > > Hmm so do you think it'd be helpful to add a null check in the file 
> > > above? 
> > Thanks, that's quite helpful! Yes, I think that would be a better fix. It's 
> > a matter of perspective (and opinion) so feel free to push back but I'd say 
> > that the code you pointed to is buggy -- it assumes the `this` loc is 
> > populated, but *by design* it's not populated for unions (I didn't know 
> > that unions could ever have `this` so #TIL?).  I think we're admitting that 
> > we have a class of initializers for which we knowingly don't create the 
> > this pointer and therefore should express that in the code.
> > 
> > Alternatively, you could argue that the complete class of `this` pointer 
> > generating code is structs and unions, so if we just make sure (like your 
> > fix) to generate a this pointer in both cases, we can assert it's 
> > non-nullness (via the cast) and be done.
> > 
> > Given that the framework in general is riddled with places where we don't 
> > know if a value or storage location or... is initialized and we have to 
> > check, I think that's the better (and consistent) approach. Moreover, given 
> > that we're not actually adding support for unions, just trying to avoid a 
> > crash, I think changing that code site better expresses that intent.
> > 
> > Still, it's a matter of opinion, so I'll leave it to you to decide.  WDYT?
> Thanks for the comment!
> 
> What you say makes sense. I guess we introduced the union initialization 
> because it could also be useful in the future, but I don't know if it makes 
> sense to add a feature that doesn't have any semantic use yet.
> 
> If the framework does these kinds of null checks in lots of other places, 
> then I agree that it'd be good to have it here too for consistency. I'm 
> leaning towards making the change you suggested.
Hi @ymandel! Apologies for the late reply.

To clarify, were you suggesting that I remove the support for union and add a 
null check at the site of crash, or keep the support and add the null check? I 
had assumed the former, but now I re-read your comment and I am not sure. 

I thought more about this and I think I'd actually prefer to add support for 
unions and skip the null check. The support could be useful for users. Now, as 
for why I think we should skip the null check: in l. 226 of 
`Analysis/FlowSensitive/DataflowEnvironment.cpp`, we check for methods that are 
not static. These are the prerequisites for something having a "this" pointer*. 
So, since we initialise the `ThisPointeeLoc` there, any intializer we're 
processing in `TypeErasedDataflowAnalysis::builtinTransferInitializer` should 
already have a not-null "this" pointer, making the check unnecessary. What do 
you think?

*lambdas could also have a this pointer, but I will leave them to be the 
subject of another revision (I've added a FIXME for that). And, in any case, 
lambdas wouldn't be passed to `builtinTransferInitializer`, so the old crash is 
avoided anyway.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696

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

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1551-1552
+const Value *FooVal = Env.getValue(*FooLoc);
+// TODO: Initialise values inside unions, then change below to
+// ASSERT_TRUE.
+ASSERT_FALSE(isa_and_nonnull(FooVal));

ymandel wrote:
> merrymeerkat wrote:
> > merrymeerkat wrote:
> > > ymandel wrote:
> > > > merrymeerkat wrote:
> > > > > ymandel wrote:
> > > > > > merrymeerkat wrote:
> > > > > > > ymandel wrote:
> > > > > > > > Why push this off to another patch?
> > > > > > > good point! I was pushing it because this is just a quick fix to 
> > > > > > > avoid a crash, and the current changes are sufficient for that
> > > > > > SGTM. Please use FIXME instead of TODO, though. But, glad to see 
> > > > > > this is enough to avoid the crashing!
> > > > > > 
> > > > > > That said, can you expand on where the actual crash was happening? 
> > > > > > I'm concerned that its possible that the crash site should be more 
> > > > > > robust and that this patch is hiding that weakness.
> > > > > Done!
> > > > > 
> > > > > The crash was happening because of a null pointer cast in the builtin 
> > > > > transfer function for CFGInitializers: 
> > > > > https://github.com/llvm/llvm-project/blob/781eabeb40b8e47e3a46b0b927784e63f0aad9ab/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp#L316
> > > > > 
> > > > > Hmm so do you think it'd be helpful to add a null check in the file 
> > > > > above? 
> > > > Thanks, that's quite helpful! Yes, I think that would be a better fix. 
> > > > It's a matter of perspective (and opinion) so feel free to push back 
> > > > but I'd say that the code you pointed to is buggy -- it assumes the 
> > > > `this` loc is populated, but *by design* it's not populated for unions 
> > > > (I didn't know that unions could ever have `this` so #TIL?).  I think 
> > > > we're admitting that we have a class of initializers for which we 
> > > > knowingly don't create the this pointer and therefore should express 
> > > > that in the code.
> > > > 
> > > > Alternatively, you could argue that the complete class of `this` 
> > > > pointer generating code is structs and unions, so if we just make sure 
> > > > (like your fix) to generate a this pointer in both cases, we can assert 
> > > > it's non-nullness (via the cast) and be done.
> > > > 
> > > > Given that the framework in general is riddled with places where we 
> > > > don't know if a value or storage location or... is initialized and we 
> > > > have to check, I think that's the better (and consistent) approach. 
> > > > Moreover, given that we're not actually adding support for unions, just 
> > > > trying to avoid a crash, I think changing that code site better 
> > > > expresses that intent.
> > > > 
> > > > Still, it's a matter of opinion, so I'll leave it to you to decide.  
> > > > WDYT?
> > > Thanks for the comment!
> > > 
> > > What you say makes sense. I guess we introduced the union initialization 
> > > because it could also be useful in the future, but I don't know if it 
> > > makes sense to add a feature that doesn't have any semantic use yet.
> > > 
> > > If the framework does these kinds of null checks in lots of other places, 
> > > then I agree that it'd be good to have it here too for consistency. I'm 
> > > leaning towards making the change you suggested.
> > Hi @ymandel! Apologies for the late reply.
> > 
> > To clarify, were you suggesting that I remove the support for union and add 
> > a null check at the site of crash, or keep the support and add the null 
> > check? I had assumed the former, but now I re-read your comment and I am 
> > not sure. 
> > 
> > I thought more about this and I think I'd actually prefer to add support 
> > for unions and skip the null check. The support could be useful for users. 
> > Now, as for why I think we should skip the null check: in l. 226 of 
> > `Analysis/FlowSensitive/DataflowEnvironment.cpp`, we check for methods that 
> > are not static. These are the prerequisites for something having a "this" 
> > pointer*. So, since we initialise the `ThisPointeeLoc` there, any 
> > intializer we're processing in 
> > `TypeErasedDataflowAnalysis::builtinTransferInitializer` should already 
> > have a not-null "this" pointer, making the check unnecessary. What do you 
> > think?
> > 
> > *lambdas could also have a this pointer, but I will leave them to be the 
> > subject of another revision (I've added a FIXME for that). And, in any 
> > case, lambdas wouldn't be passed to `builtinTransferInitializer`, so the 
> > old crash is avoided anyway.
> > 
> Sounds good!  I had meant the former. But, I think you make a compelling case 
> for adding the support and skipping the null check.
> 
> > And, in any case, lambdas wouldn't be passed to builtinTransferInitializer, 
> > so the old crash is avoided anyway.
> Why is this? I'm not sure of the structure of lambda'

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat updated this revision to Diff 485626.
merrymeerkat added a comment.

Resolving review comment. Add createValue functionality for unions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1518,6 +1518,50 @@
   });
 }
 
+TEST(TransferTest, UnionThisMember) {
+  std::string Code = R"(
+union A {
+  int Foo;
+  int Bar;
+
+  void target() {
+(void)0; // [[p]]
+  }
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const auto *FooLoc =
+cast(&ThisLoc->getChild(*FooDecl));
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
+
+const Value *FooVal = Env.getValue(*FooLoc);
+ASSERT_TRUE(isa_and_nonnull(FooVal));
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+ASSERT_TRUE(isa_and_nonnull(BarVal));
+  });
+}
+
 TEST(TransferTest, StructThisInLambda) {
   std::string ThisCaptureCode = R"(
 struct A {
@@ -2537,12 +2581,34 @@
 ASSERT_THAT(BazDecl, NotNull());
 ASSERT_TRUE(BazDecl->getType()->isUnionType());
 
+auto BazFields = BazDecl->getType()->getAsRecordDecl()->fields();
+FieldDecl *FooDecl = nullptr;
+for (FieldDecl *Field : BazFields) {
+  if (Field->getNameAsString() == "Foo") {
+FooDecl = Field;
+  } else {
+FAIL() << "Unexpected field: " << Field->getNameAsString();
+  }
+}
+ASSERT_THAT(FooDecl, NotNull());
+
 const auto *BazLoc = dyn_cast_or_null(
 Env.getStorageLocation(*BazDecl, SkipPast::None));
 ASSERT_THAT(BazLoc, NotNull());
+ASSERT_THAT(Env.getValue(*BazLoc), NotNull());
+
+const auto *BazVal = cast(Env.getValue(*BazLoc));
+const auto *FooValFromBazVal = cast(BazVal->getChild(*FooDecl));
+const auto *FooValFromBazLoc = cast(Env.getValue(BazLoc->getChild(*FooDecl)));
+EXPECT_EQ(FooValFromBazLoc, FooValFromBazVal);
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+const auto *BarLoc = Env.getStorageLocation(*BarDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
 
-// FIXME: Add support for union types.
-EXPECT_THAT(Env.getValue(*BazLoc), IsNull());
+EXPECT_EQ(Env.getValue(*BarLoc), FooValFromBazVal);
+EXPECT_EQ(Env.getValue(*BarLoc), FooValFromBazLoc);
   });
 }
 
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -480,10 +480,6 @@
 if (BaseLoc == nullptr)
   return;
 
-// FIXME: Add support for union types.
-if (BaseLoc->getType()->isUnionType())
-  return;
-
 auto &MemberLoc = BaseLoc->getChild(*Member);
 if (MemberLoc.getType()->isReferenceType()) {
   Env.setStorageLocation(*S, MemberLoc);
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -223,14 +223,12 @@
 if (Parent->isLambda())
   MethodDecl = dyn_cast(Parent->getDeclContext());
 
+// FIXME: Initialize the ThisPointeeLoc of lambdas too.
 if (MethodDecl && !MethodDecl->isStatic()) {
   QualType ThisPointeeType = MethodDecl->getThisObjectType();
-  // FIXME: Add support for union types.
-  if (!ThisPointeeType->isUnionType()) {
-ThisPointeeLoc = &createStorageLocation(ThisPointeeType);
-if (Value *ThisPointeeVal = createValue(ThisPointeeType))
-  setValue(*ThisPointeeLoc, *ThisPointeeVal);
-  }
+  ThisPoi

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat added a comment.

> another thought: please verify that `createStorageLoction` and `createValue` 
> can correctly handle union types. Otherwise, you'll likely end up with other 
> (surprising) crashes in the system. E.g. 
> https://github.com/llvm/llvm-project/blob/781eabeb40b8e47e3a46b0b927784e63f0aad9ab/clang/lib/Analysis/FlowSensitive/Transfer.cpp#L457
> assumes that the base of a member expression is an 
> `AggregateStorageLocation`, which means that in the case that `this` is such 
> a base and it's a union type, you need to be sure that 
> `createStorageLocation` has allocated an `AggregateStorageLocation`.
>
> Unfortunately, we don't have written down anywhere (aside from the code 
> itself) the invariants we require.

Good point!

Based on the assertions in TransferTest.UnionThisMember, we are creating 
storage locations for unions. So, the code you linked to should not crash (at 
least on unions as simple as that in the test).

We weren't creating values yet though. Turns out that was because `createValue` 
calls `createValueUnlessSelfReferential`, which creates values only for certain 
types, returning a nullptr otherwise: 
https://github.com/llvm/llvm-project/blob/8eb3698b940c4064b772f3ff5848d45f28523753/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp#L618-L699
 . I've just changed this function's if-statement on structs and classes to 
accept union types too, and now we are creating values for unions. So, values 
should hopefully not be a problem either.

There was already another unit test relating to unions 
(TransferTest.AssignToUnionMember), but it was only partly implemented. Now 
that we initialize locations and values for union types, I implemented the rest 
of the test. (thank you @gribozavr2 for the help debugging!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696

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

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat updated this revision to Diff 485693.
merrymeerkat added a comment.

Extend documentation comment on storage locations with a FIXME about unions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696

Files:
  clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1518,6 +1518,50 @@
   });
 }
 
+TEST(TransferTest, UnionThisMember) {
+  std::string Code = R"(
+union A {
+  int Foo;
+  int Bar;
+
+  void target() {
+(void)0; // [[p]]
+  }
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const auto *FooLoc =
+cast(&ThisLoc->getChild(*FooDecl));
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
+
+const Value *FooVal = Env.getValue(*FooLoc);
+ASSERT_TRUE(isa_and_nonnull(FooVal));
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+ASSERT_TRUE(isa_and_nonnull(BarVal));
+  });
+}
+
 TEST(TransferTest, StructThisInLambda) {
   std::string ThisCaptureCode = R"(
 struct A {
@@ -2537,12 +2581,34 @@
 ASSERT_THAT(BazDecl, NotNull());
 ASSERT_TRUE(BazDecl->getType()->isUnionType());
 
+auto BazFields = BazDecl->getType()->getAsRecordDecl()->fields();
+FieldDecl *FooDecl = nullptr;
+for (FieldDecl *Field : BazFields) {
+  if (Field->getNameAsString() == "Foo") {
+FooDecl = Field;
+  } else {
+FAIL() << "Unexpected field: " << Field->getNameAsString();
+  }
+}
+ASSERT_THAT(FooDecl, NotNull());
+
 const auto *BazLoc = dyn_cast_or_null(
 Env.getStorageLocation(*BazDecl, SkipPast::None));
 ASSERT_THAT(BazLoc, NotNull());
+ASSERT_THAT(Env.getValue(*BazLoc), NotNull());
+
+const auto *BazVal = cast(Env.getValue(*BazLoc));
+const auto *FooValFromBazVal = cast(BazVal->getChild(*FooDecl));
+const auto *FooValFromBazLoc = cast(Env.getValue(BazLoc->getChild(*FooDecl)));
+EXPECT_EQ(FooValFromBazLoc, FooValFromBazVal);
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+const auto *BarLoc = Env.getStorageLocation(*BarDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
 
-// FIXME: Add support for union types.
-EXPECT_THAT(Env.getValue(*BazLoc), IsNull());
+EXPECT_EQ(Env.getValue(*BarLoc), FooValFromBazVal);
+EXPECT_EQ(Env.getValue(*BarLoc), FooValFromBazLoc);
   });
 }
 
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -480,10 +480,6 @@
 if (BaseLoc == nullptr)
   return;
 
-// FIXME: Add support for union types.
-if (BaseLoc->getType()->isUnionType())
-  return;
-
 auto &MemberLoc = BaseLoc->getChild(*Member);
 if (MemberLoc.getType()->isReferenceType()) {
   Env.setStorageLocation(*S, MemberLoc);
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -223,14 +223,12 @@
 if (Parent->isLambda())
   MethodDecl = dyn_cast(Parent->getDeclContext());
 
+// FIXME: Initialize the ThisPointeeLoc of lambdas too.
 if (MethodDecl && !MethodDecl->isStatic()) {
   QualType ThisPointeeType = MethodDecl->getThisObjectType();
-  // FIXME: Add support for union types.
-  if (!ThisPointeeType->isUnionType()) {
-ThisPointeeLoc = &createStorageLocation(ThisPointeeType);
-if (Value *ThisPointeeVal = createValue(ThisPointeeType))
- 

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat added a comment.

@ymandel - thank you for pointing all of this out! And no need to apologise at 
all :)

I agree that the changes made here are not ideal. But the alternative is also 
not ideal.

The question becomes: is it better to model unions in a way that is lacking, or 
to not model them at all?

To model unions properly, we would need to be able to set the storage location 
of all the members of a union to be the same. But storage location is tied to 
type, so we can't create the same storage location for members of different 
types. From what I can tell, correctly modelling unions would require us to do 
an overhaul of how storage locations are implemented.

Like Dmitri said, I think most of the issues with this simple approach to 
modelling unions would only be revealed with UB code. So, unless we want to 
compare the storage locations of pointers to different union members (which I 
don't think will be a very common use), this modelling should work all right 
for defined behaviour. I added a documentation comment regarding the lacking 
implementation of storage location.

Coming back to the question above. Yitzhak, you seem to prefer not modelling 
union at all rather than having this model that falls short. Dmitri seems to 
prefer a model that works on most cases than no model. I am a bit torn, but I 
think I'm leaning a bit on the side of having a model that falls short. I don't 
think we plan on doing the storage location overhaul anytime soon, and until 
then it'd be good to at least have some functionality.

Happy new year by the way!!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696

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

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd862f66221de: [clang][dataflow] Treat unions as structs. 
(authored by merrymeerkat).

Changed prior to commit:
  https://reviews.llvm.org/D140696?vs=485693&id=486025#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696

Files:
  clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1518,6 +1518,50 @@
   });
 }
 
+TEST(TransferTest, UnionThisMember) {
+  std::string Code = R"(
+union A {
+  int Foo;
+  int Bar;
+
+  void target() {
+(void)0; // [[p]]
+  }
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+const auto *ThisLoc = dyn_cast(
+Env.getThisPointeeStorageLocation());
+ASSERT_THAT(ThisLoc, NotNull());
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const auto *FooLoc =
+cast(&ThisLoc->getChild(*FooDecl));
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
+
+const Value *FooVal = Env.getValue(*FooLoc);
+ASSERT_TRUE(isa_and_nonnull(FooVal));
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *BarLoc =
+cast(&ThisLoc->getChild(*BarDecl));
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+ASSERT_TRUE(isa_and_nonnull(BarVal));
+  });
+}
+
 TEST(TransferTest, StructThisInLambda) {
   std::string ThisCaptureCode = R"(
 struct A {
@@ -2537,12 +2581,34 @@
 ASSERT_THAT(BazDecl, NotNull());
 ASSERT_TRUE(BazDecl->getType()->isUnionType());
 
+auto BazFields = BazDecl->getType()->getAsRecordDecl()->fields();
+FieldDecl *FooDecl = nullptr;
+for (FieldDecl *Field : BazFields) {
+  if (Field->getNameAsString() == "Foo") {
+FooDecl = Field;
+  } else {
+FAIL() << "Unexpected field: " << Field->getNameAsString();
+  }
+}
+ASSERT_THAT(FooDecl, NotNull());
+
 const auto *BazLoc = dyn_cast_or_null(
 Env.getStorageLocation(*BazDecl, SkipPast::None));
 ASSERT_THAT(BazLoc, NotNull());
+ASSERT_THAT(Env.getValue(*BazLoc), NotNull());
+
+const auto *BazVal = cast(Env.getValue(*BazLoc));
+const auto *FooValFromBazVal = cast(BazVal->getChild(*FooDecl));
+const auto *FooValFromBazLoc = cast(Env.getValue(BazLoc->getChild(*FooDecl)));
+EXPECT_EQ(FooValFromBazLoc, FooValFromBazVal);
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+const auto *BarLoc = Env.getStorageLocation(*BarDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
 
-// FIXME: Add support for union types.
-EXPECT_THAT(Env.getValue(*BazLoc), IsNull());
+EXPECT_EQ(Env.getValue(*BarLoc), FooValFromBazVal);
+EXPECT_EQ(Env.getValue(*BarLoc), FooValFromBazLoc);
   });
 }
 
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -487,10 +487,6 @@
 if (BaseLoc == nullptr)
   return;
 
-// FIXME: Add support for union types.
-if (BaseLoc->getType()->isUnionType())
-  return;
-
 auto &MemberLoc = BaseLoc->getChild(*Member);
 if (MemberLoc.getType()->isReferenceType()) {
   Env.setStorageLocation(*S, MemberLoc);
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -235,14 +235,12 @@
 if (Parent->isLambda())
   MethodDecl = dyn_cast(Parent->getDeclContext());
 
+// FIXME: Initialize the ThisPointeeLoc of lambdas too.
 if (MethodDecl && !MethodDecl->isStatic()) {
   QualType ThisPointeeType = MethodDecl->getThisObjectType();
-  // FIXME: Add support for union types.
-  if (!ThisPointeeTyp

[PATCH] D140696: [clang][dataflow] Treat unions as structs.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat added a comment.

Thanks for clarifying! I've gone ahead and landed the change. At least this 
should reduce the number of false negatives we get (hopefully without 
introducing too much complexity).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696

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

[PATCH] D137432: [clang][dataflow] Change transfer and diagnoser functions to receive a CFGElement.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
merrymeerkat requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For example, previously, if the dataflow analysis wanted to warn the user about 
a CXXCtorInitializer, it could not show a precise warning because diagnosers 
only accepted Stmts.
Now, diagnosers receive a CFGElement and can thus warn about any type of C++ 
construct.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137432

Files:
  clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h
  clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
  clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1235,6 +1235,7 @@
 UncheckedOptionalAccessModelOptions Options{
 /*IgnoreSmartPointerDereference=*/true};
 std::vector Diagnostics;
+UncheckedOptionalAccessDiagnoser Diagnoser(Options);
 llvm::Error Error = checkDataflow(
 AnalysisInputs(
 SourceCode, std::move(FuncMatcher),
@@ -1242,8 +1243,7 @@
   return UncheckedOptionalAccessModel(Ctx, Options);
 })
 .withPostVisitCFG(
-[&Diagnostics,
- Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
+[&Diagnostics, &Diagnoser](
 ASTContext &Ctx, const CFGElement &Elt,
 const TypeErasedDataflowAnalysisState &State) mutable {
   auto EltDiagnostics =
Index: clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
@@ -84,7 +84,7 @@
   dyn_cast_or_null(Val.getProperty("pos"))};
 }
 
-void transferUninitializedInt(const DeclStmt *D,
+void transferUninitializedInt(const CFGElement &Element, const DeclStmt *D,
   const MatchFinder::MatchResult &M,
   LatticeTransferState &State) {
   const auto *Var = M.Nodes.getNodeAs(kVar);
@@ -133,7 +133,8 @@
   return {UnaryOpValue, UnaryOpProps, OperandProps};
 }
 
-void transferBinary(const BinaryOperator *BO, const MatchFinder::MatchResult &M,
+void transferBinary(const CFGElement &Element, const BinaryOperator *BO,
+const MatchFinder::MatchResult &M,
 LatticeTransferState &State) {
   StorageLocation *Loc = State.Env.getStorageLocation(*BO, SkipPast::None);
   if (!Loc) {
@@ -202,7 +203,7 @@
   }
 }
 
-void transferUnaryMinus(const UnaryOperator *UO,
+void transferUnaryMinus(const CFGElement &Element, const UnaryOperator *UO,
 const MatchFinder::MatchResult &M,
 LatticeTransferState &State) {
   auto [UnaryOpValue, UnaryOpProps, OperandProps] =
@@ -221,7 +222,7 @@
   State.Env.makeImplication(*OperandProps.Zero, *UnaryOpProps.Zero));
 }
 
-void transferUnaryNot(const UnaryOperator *UO,
+void transferUnaryNot(const CFGElement &Element, const UnaryOperator *UO,
   const MatchFinder::MatchResult &M,
   LatticeTransferState &State) {
   auto [UnaryOpValue, UnaryOpProps, OperandProps] =
@@ -246,7 +247,8 @@
   }
 }
 
-void transferExpr(const Expr *E, const MatchFinder::MatchResult &M,
+void transferExpr(const CFGElement &Element, const Expr *E,
+  const MatchFinder::MatchResult &M,
   LatticeTransferState &State) {
   const ASTContext &Context = *M.Context;
   StorageLocation *Loc = State.Env.getStorageLocation(*E, SkipPast::None);
Index: clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
@@ -34,11 +34,13 @@
   .CaseOfCFGStmt(
   declStmt(hasSingleDecl(
   varDecl(hasInitializer(integerLiteral(equals(42)),
-  [](const DeclStmt *, const MatchFinder::MatchResult &,
+  [](const CFGElement &Element, const DeclStmt *,
+ const MatchFinder::MatchResult &,
  CFGElementMatches &Counter) { Counter.StmtMatches++; })
   .CaseOfCFGInit(
   cxxCtorInitializer

[PATCH] D137584: Add const information about AST nodes used to construct CFG elements.

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat created this revision.
Herald added a reviewer: NoQ.
Herald added a project: All.
merrymeerkat requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Most constructors and destructors in CFG.h already specify const arguments, but 
some are missing this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137584

Files:
  clang/include/clang/Analysis/CFG.h


Index: clang/include/clang/Analysis/CFG.h
===
--- clang/include/clang/Analysis/CFG.h
+++ clang/include/clang/Analysis/CFG.h
@@ -131,7 +131,7 @@
 
 class CFGStmt : public CFGElement {
 public:
-  explicit CFGStmt(Stmt *S, Kind K = Statement) : CFGElement(K, S) {
+  explicit CFGStmt(const Stmt *S, Kind K = Statement) : CFGElement(K, S) {
 assert(isKind(*this));
   }
 
@@ -155,7 +155,8 @@
 /// this is only used by the analyzer's CFG.
 class CFGConstructor : public CFGStmt {
 public:
-  explicit CFGConstructor(CXXConstructExpr *CE, const ConstructionContext *C)
+  explicit CFGConstructor(const CXXConstructExpr *CE,
+  const ConstructionContext *C)
   : CFGStmt(CE, Constructor) {
 assert(C);
 Data2.setPointer(const_cast(C));
@@ -185,7 +186,7 @@
 public:
   /// Returns true when call expression \p CE needs to be represented
   /// by CFGCXXRecordTypedCall, as opposed to a regular CFGStmt.
-  static bool isCXXRecordTypedCall(Expr *E) {
+  static bool isCXXRecordTypedCall(const Expr *E) {
 assert(isa(E) || isa(E));
 // There is no such thing as reference-type expression. If the function
 // returns a reference, it'll return the respective lvalue or xvalue
@@ -194,7 +195,7 @@
E->getType().getCanonicalType()->getAsCXXRecordDecl();
   }
 
-  explicit CFGCXXRecordTypedCall(Expr *E, const ConstructionContext *C)
+  explicit CFGCXXRecordTypedCall(const Expr *E, const ConstructionContext *C)
   : CFGStmt(E, CXXRecordTypedCall) {
 assert(isCXXRecordTypedCall(E));
 assert(C && (isa(C) ||
@@ -225,7 +226,7 @@
 /// list.
 class CFGInitializer : public CFGElement {
 public:
-  explicit CFGInitializer(CXXCtorInitializer *initializer)
+  explicit CFGInitializer(const CXXCtorInitializer *initializer)
   : CFGElement(Initializer, initializer) {}
 
   CXXCtorInitializer* getInitializer() const {
@@ -482,7 +483,7 @@
 /// expression for temporary object.
 class CFGTemporaryDtor : public CFGImplicitDtor {
 public:
-  CFGTemporaryDtor(CXXBindTemporaryExpr *expr)
+  CFGTemporaryDtor(const CXXBindTemporaryExpr *expr)
   : CFGImplicitDtor(TemporaryDtor, expr, nullptr) {}
 
   const CXXBindTemporaryExpr *getBindTemporaryExpr() const {


Index: clang/include/clang/Analysis/CFG.h
===
--- clang/include/clang/Analysis/CFG.h
+++ clang/include/clang/Analysis/CFG.h
@@ -131,7 +131,7 @@
 
 class CFGStmt : public CFGElement {
 public:
-  explicit CFGStmt(Stmt *S, Kind K = Statement) : CFGElement(K, S) {
+  explicit CFGStmt(const Stmt *S, Kind K = Statement) : CFGElement(K, S) {
 assert(isKind(*this));
   }
 
@@ -155,7 +155,8 @@
 /// this is only used by the analyzer's CFG.
 class CFGConstructor : public CFGStmt {
 public:
-  explicit CFGConstructor(CXXConstructExpr *CE, const ConstructionContext *C)
+  explicit CFGConstructor(const CXXConstructExpr *CE,
+  const ConstructionContext *C)
   : CFGStmt(CE, Constructor) {
 assert(C);
 Data2.setPointer(const_cast(C));
@@ -185,7 +186,7 @@
 public:
   /// Returns true when call expression \p CE needs to be represented
   /// by CFGCXXRecordTypedCall, as opposed to a regular CFGStmt.
-  static bool isCXXRecordTypedCall(Expr *E) {
+  static bool isCXXRecordTypedCall(const Expr *E) {
 assert(isa(E) || isa(E));
 // There is no such thing as reference-type expression. If the function
 // returns a reference, it'll return the respective lvalue or xvalue
@@ -194,7 +195,7 @@
E->getType().getCanonicalType()->getAsCXXRecordDecl();
   }
 
-  explicit CFGCXXRecordTypedCall(Expr *E, const ConstructionContext *C)
+  explicit CFGCXXRecordTypedCall(const Expr *E, const ConstructionContext *C)
   : CFGStmt(E, CXXRecordTypedCall) {
 assert(isCXXRecordTypedCall(E));
 assert(C && (isa(C) ||
@@ -225,7 +226,7 @@
 /// list.
 class CFGInitializer : public CFGElement {
 public:
-  explicit CFGInitializer(CXXCtorInitializer *initializer)
+  explicit CFGInitializer(const CXXCtorInitializer *initializer)
   : CFGElement(Initializer, initializer) {}
 
   CXXCtorInitializer* getInitializer() const {
@@ -482,7 +483,7 @@
 /// expression for temporary object.
 class CFGTemporaryDtor : public CFGImplicitDtor {
 public:
-  CFGTemporaryDtor(CXXBindTemporaryExpr *expr)
+  CFGTemporaryDtor(const CXXBindTemporaryExpr *expr)
   : CFGImplicitDtor(TemporaryDtor, expr, nullptr) {}
 
   const CXXBindTemporaryExpr *getBindTempora

[PATCH] D142637: A more concise AST dump If the modifiedType and the minimally desugared type of an AttributedType are the same, then we do not need to visit both (or show both in an AST dump). Here

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat created this revision.
Herald added a project: All.
merrymeerkat requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

...when it is different from the equivalentType (the minimally desugared type), 
because the latter is already visited by default.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142637

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/test/AST/ast-dump-attr-type.cpp
  clang/test/AST/ast-dump-types-json.cpp


Index: clang/test/AST/ast-dump-types-json.cpp
===
--- clang/test/AST/ast-dump-types-json.cpp
+++ clang/test/AST/ast-dump-types-json.cpp
@@ -203,32 +203,6 @@
 // CHECK-NEXT:]
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
-// CHECK-NEXT: },
-// CHECK-NEXT: {
-// CHECK-NEXT:  "id": "0x{{.*}}",
-// CHECK-NEXT:  "kind": "ParenType",
-// CHECK-NEXT:  "type": {
-// CHECK-NEXT:   "qualType": "void ()"
-// CHECK-NEXT:  },
-// CHECK-NEXT:  "inner": [
-// CHECK-NEXT:   {
-// CHECK-NEXT:"id": "0x{{.*}}",
-// CHECK-NEXT:"kind": "FunctionProtoType",
-// CHECK-NEXT:"type": {
-// CHECK-NEXT: "qualType": "void ()"
-// CHECK-NEXT:},
-// CHECK-NEXT:"cc": "cdecl",
-// CHECK-NEXT:"inner": [
-// CHECK-NEXT: {
-// CHECK-NEXT:  "id": "0x{{.*}}",
-// CHECK-NEXT:  "kind": "BuiltinType",
-// CHECK-NEXT:  "type": {
-// CHECK-NEXT:   "qualType": "void"
-// CHECK-NEXT:  }
-// CHECK-NEXT: }
-// CHECK-NEXT:]
-// CHECK-NEXT:   }
-// CHECK-NEXT:  ]
 // CHECK-NEXT: }
 // CHECK-NEXT:]
 // CHECK-NEXT:   }
Index: clang/test/AST/ast-dump-attr-type.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-attr-type.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump %s | 
FileCheck %s
+
+int * _Nonnull x;
+using Ty = decltype(x);
+
+// CHECK:`-TypeAliasDecl 0x{{[^ ]*}}   col:7 Ty 
'decltype(x)':'int *'
+// CHECK-NEXT:  `-DecltypeType 0x{{[^ ]*}} 'decltype(x)' sugar
+// CHECK-NEXT: |-DeclRefExpr 0x{{[^ ]*}}  'int * _Nonnull':'int *' 
lvalue Var 0x{{[^ ]*}} 'x' 'int * _Nonnull':'int *' non_odr_use_unevaluated
+// CHECK-NEXT:`-AttributedType 0x{{[^ ]*}} 'int * _Nonnull' sugar
+// CHECK-NEXT:  `-PointerType 0x{{[^ ]*}} 'int *'
+// CHECK-NEXT:`-BuiltinType 0x{{[^ ]*}} 'int'
+// CHECK-NOT:   `-PointerType 0x564c31a07520 'int *'
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -384,7 +384,8 @@
   }
   void VisitAttributedType(const AttributedType *T) {
 // FIXME: AttrKind
-Visit(T->getModifiedType());
+if (T->getModifiedType() != T->getEquivalentType())
+  Visit(T->getModifiedType());
   }
   void VisitBTFTagAttributedType(const BTFTagAttributedType *T) {
 Visit(T->getWrappedType());


Index: clang/test/AST/ast-dump-types-json.cpp
===
--- clang/test/AST/ast-dump-types-json.cpp
+++ clang/test/AST/ast-dump-types-json.cpp
@@ -203,32 +203,6 @@
 // CHECK-NEXT:]
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
-// CHECK-NEXT: },
-// CHECK-NEXT: {
-// CHECK-NEXT:  "id": "0x{{.*}}",
-// CHECK-NEXT:  "kind": "ParenType",
-// CHECK-NEXT:  "type": {
-// CHECK-NEXT:   "qualType": "void ()"
-// CHECK-NEXT:  },
-// CHECK-NEXT:  "inner": [
-// CHECK-NEXT:   {
-// CHECK-NEXT:"id": "0x{{.*}}",
-// CHECK-NEXT:"kind": "FunctionProtoType",
-// CHECK-NEXT:"type": {
-// CHECK-NEXT: "qualType": "void ()"
-// CHECK-NEXT:},
-// CHECK-NEXT:"cc": "cdecl",
-// CHECK-NEXT:"inner": [
-// CHECK-NEXT: {
-// CHECK-NEXT:  "id": "0x{{.*}}",
-// CHECK-NEXT:  "kind": "BuiltinType",
-// CHECK-NEXT:  "type": {
-// CHECK-NEXT:   "qualType": "void"
-// CHECK-NEXT:  }
-// CHECK-NEXT: }
-// CHECK-NEXT:]
-// CHECK-NEXT:   }
-// CHECK-NEXT:  ]
 // CHECK-NEXT: }
 // CHECK-NEXT:]
 // CHECK-NEXT:   }
Index: clang/test/AST/ast-dump-attr-type.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-attr-type.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump %s | FileCheck %s
+
+int * _Nonnull x;
+using Ty = decltype(x);
+
+// CHECK:`-TypeAliasDecl 0x{{

[PATCH] D142637: A slightly more concise AST dump :)

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat updated this revision to Diff 493557.
merrymeerkat added a comment.

Addressing review comments (improve tests)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142637

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/test/AST/ast-dump-attr-type.cpp
  clang/test/AST/ast-dump-types-json.cpp


Index: clang/test/AST/ast-dump-types-json.cpp
===
--- clang/test/AST/ast-dump-types-json.cpp
+++ clang/test/AST/ast-dump-types-json.cpp
@@ -203,32 +203,6 @@
 // CHECK-NEXT:]
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
-// CHECK-NEXT: },
-// CHECK-NEXT: {
-// CHECK-NEXT:  "id": "0x{{.*}}",
-// CHECK-NEXT:  "kind": "ParenType",
-// CHECK-NEXT:  "type": {
-// CHECK-NEXT:   "qualType": "void ()"
-// CHECK-NEXT:  },
-// CHECK-NEXT:  "inner": [
-// CHECK-NEXT:   {
-// CHECK-NEXT:"id": "0x{{.*}}",
-// CHECK-NEXT:"kind": "FunctionProtoType",
-// CHECK-NEXT:"type": {
-// CHECK-NEXT: "qualType": "void ()"
-// CHECK-NEXT:},
-// CHECK-NEXT:"cc": "cdecl",
-// CHECK-NEXT:"inner": [
-// CHECK-NEXT: {
-// CHECK-NEXT:  "id": "0x{{.*}}",
-// CHECK-NEXT:  "kind": "BuiltinType",
-// CHECK-NEXT:  "type": {
-// CHECK-NEXT:   "qualType": "void"
-// CHECK-NEXT:  }
-// CHECK-NEXT: }
-// CHECK-NEXT:]
-// CHECK-NEXT:   }
-// CHECK-NEXT:  ]
 // CHECK-NEXT: }
 // CHECK-NEXT:]
 // CHECK-NEXT:   }
Index: clang/test/AST/ast-dump-attr-type.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-attr-type.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump %s | 
FileCheck %s
+
+int * _Nonnull x;
+using Ty = decltype(x);
+
+// CHECK: TypeAliasDecl 0x{{[^ ]*}}   col:7 Ty 
'decltype(x)':'int *'
+// CHECK-NEXT:  `-DecltypeType 0x{{[^ ]*}} 'decltype(x)' sugar
+// CHECK-NEXT: |-DeclRefExpr 0x{{[^ ]*}}  'int * _Nonnull':'int *' 
lvalue Var 0x{{[^ ]*}} 'x' 'int * _Nonnull':'int *' non_odr_use_unevaluated
+// CHECK-NEXT:`-AttributedType 0x{{[^ ]*}} 'int * _Nonnull' sugar
+// CHECK-NEXT:  `-PointerType 0x{{[^ ]*}} 'int *'
+// CHECK-NEXT:`-BuiltinType 0x{{[^ ]*}} 'int'
+// CHECK-NOT:   `-PointerType
+
+[[clang::address_space(3)]] int *y;
+using Ty1 = decltype(y);
+
+// CHECK: TypeAliasDecl 0x{{[^ ]*}}  col:7 Ty1 
'decltype(y)':'__attribute__((address_space(3))) int *'
+// CHECK-NEXT: `-DecltypeType 0x{{[^ ]*}} 'decltype(y)' sugar
+// CHECK-NEXT:   |-DeclRefExpr 0x{{[^ ]*}}  
'__attribute__((address_space(3))) int *' lvalue Var 0x{{[^ ]*}} 'y' 
'__attribute__((address_space(3))) int *' non_odr_use_unevaluated
+// CHECK-NEXT: `-PointerType 0x{{[^ ]*}} 
'__attribute__((address_space(3))) int *'
+// CHECK-NEXT:   `-AttributedType 0x{{[^ ]*}} 
'__attribute__((address_space(3))) int' sugar
+// CHECK-NEXT  |-BuiltinType 0x{{[^ ]*}} 'int'
+// CHECK-NEXT  `-QualType 0x{{[^ ]*}} 
'__attribute__((address_space(3))) int' __attribute__((address_space(3)))
+// CHECK-NEXT`-BuiltinType 0x{{[^ ]*}} 'int'
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -384,7 +384,8 @@
   }
   void VisitAttributedType(const AttributedType *T) {
 // FIXME: AttrKind
-Visit(T->getModifiedType());
+if (T->getModifiedType() != T->getEquivalentType())
+  Visit(T->getModifiedType());
   }
   void VisitBTFTagAttributedType(const BTFTagAttributedType *T) {
 Visit(T->getWrappedType());


Index: clang/test/AST/ast-dump-types-json.cpp
===
--- clang/test/AST/ast-dump-types-json.cpp
+++ clang/test/AST/ast-dump-types-json.cpp
@@ -203,32 +203,6 @@
 // CHECK-NEXT:]
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
-// CHECK-NEXT: },
-// CHECK-NEXT: {
-// CHECK-NEXT:  "id": "0x{{.*}}",
-// CHECK-NEXT:  "kind": "ParenType",
-// CHECK-NEXT:  "type": {
-// CHECK-NEXT:   "qualType": "void ()"
-// CHECK-NEXT:  },
-// CHECK-NEXT:  "inner": [
-// CHECK-NEXT:   {
-// CHECK-NEXT:"id": "0x{{.*}}",
-// CHECK-NEXT:"kind": "FunctionProtoType",
-// CHECK-NEXT:"type": {
-// CHECK-NEXT: "qualType": "void ()"
-// CHECK-NEXT:},
-// CHECK-NEXT:"cc": "cdecl",
-// CHECK-NEXT:"inner": [
-// CHECK-NEXT: {
-// CHECK-NEXT:  "id": "0x{{.*}}",
-// CHECK-NEXT:  

[PATCH] D142637: A slightly more concise AST dump :)

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat marked 3 inline comments as done.
merrymeerkat added a comment.

thank you @sammccall @gribozavr2 for spotting these issues!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142637

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

[PATCH] D142637: A slightly more concise AST dump :)

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG70acb3aab3a1: A slightly more concise AST dump :) (authored 
by merrymeerkat).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142637

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/test/AST/ast-dump-attr-type.cpp
  clang/test/AST/ast-dump-types-json.cpp


Index: clang/test/AST/ast-dump-types-json.cpp
===
--- clang/test/AST/ast-dump-types-json.cpp
+++ clang/test/AST/ast-dump-types-json.cpp
@@ -203,32 +203,6 @@
 // CHECK-NEXT:]
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
-// CHECK-NEXT: },
-// CHECK-NEXT: {
-// CHECK-NEXT:  "id": "0x{{.*}}",
-// CHECK-NEXT:  "kind": "ParenType",
-// CHECK-NEXT:  "type": {
-// CHECK-NEXT:   "qualType": "void ()"
-// CHECK-NEXT:  },
-// CHECK-NEXT:  "inner": [
-// CHECK-NEXT:   {
-// CHECK-NEXT:"id": "0x{{.*}}",
-// CHECK-NEXT:"kind": "FunctionProtoType",
-// CHECK-NEXT:"type": {
-// CHECK-NEXT: "qualType": "void ()"
-// CHECK-NEXT:},
-// CHECK-NEXT:"cc": "cdecl",
-// CHECK-NEXT:"inner": [
-// CHECK-NEXT: {
-// CHECK-NEXT:  "id": "0x{{.*}}",
-// CHECK-NEXT:  "kind": "BuiltinType",
-// CHECK-NEXT:  "type": {
-// CHECK-NEXT:   "qualType": "void"
-// CHECK-NEXT:  }
-// CHECK-NEXT: }
-// CHECK-NEXT:]
-// CHECK-NEXT:   }
-// CHECK-NEXT:  ]
 // CHECK-NEXT: }
 // CHECK-NEXT:]
 // CHECK-NEXT:   }
Index: clang/test/AST/ast-dump-attr-type.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-attr-type.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump %s | 
FileCheck %s
+
+int * _Nonnull x;
+using Ty = decltype(x);
+
+// CHECK: TypeAliasDecl 0x{{[^ ]*}}   col:7 Ty 
'decltype(x)':'int *'
+// CHECK-NEXT:  `-DecltypeType 0x{{[^ ]*}} 'decltype(x)' sugar
+// CHECK-NEXT: |-DeclRefExpr 0x{{[^ ]*}}  'int * _Nonnull':'int *' 
lvalue Var 0x{{[^ ]*}} 'x' 'int * _Nonnull':'int *' non_odr_use_unevaluated
+// CHECK-NEXT:`-AttributedType 0x{{[^ ]*}} 'int * _Nonnull' sugar
+// CHECK-NEXT:  `-PointerType 0x{{[^ ]*}} 'int *'
+// CHECK-NEXT:`-BuiltinType 0x{{[^ ]*}} 'int'
+// CHECK-NOT:   `-PointerType
+
+[[clang::address_space(3)]] int *y;
+using Ty1 = decltype(y);
+
+// CHECK: TypeAliasDecl 0x{{[^ ]*}}  col:7 Ty1 
'decltype(y)':'__attribute__((address_space(3))) int *'
+// CHECK-NEXT: `-DecltypeType 0x{{[^ ]*}} 'decltype(y)' sugar
+// CHECK-NEXT:   |-DeclRefExpr 0x{{[^ ]*}}  
'__attribute__((address_space(3))) int *' lvalue Var 0x{{[^ ]*}} 'y' 
'__attribute__((address_space(3))) int *' non_odr_use_unevaluated
+// CHECK-NEXT: `-PointerType 0x{{[^ ]*}} 
'__attribute__((address_space(3))) int *'
+// CHECK-NEXT:   `-AttributedType 0x{{[^ ]*}} 
'__attribute__((address_space(3))) int' sugar
+// CHECK-NEXT  |-BuiltinType 0x{{[^ ]*}} 'int'
+// CHECK-NEXT  `-QualType 0x{{[^ ]*}} 
'__attribute__((address_space(3))) int' __attribute__((address_space(3)))
+// CHECK-NEXT`-BuiltinType 0x{{[^ ]*}} 'int'
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -384,7 +384,8 @@
   }
   void VisitAttributedType(const AttributedType *T) {
 // FIXME: AttrKind
-Visit(T->getModifiedType());
+if (T->getModifiedType() != T->getEquivalentType())
+  Visit(T->getModifiedType());
   }
   void VisitBTFTagAttributedType(const BTFTagAttributedType *T) {
 Visit(T->getWrappedType());


Index: clang/test/AST/ast-dump-types-json.cpp
===
--- clang/test/AST/ast-dump-types-json.cpp
+++ clang/test/AST/ast-dump-types-json.cpp
@@ -203,32 +203,6 @@
 // CHECK-NEXT:]
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
-// CHECK-NEXT: },
-// CHECK-NEXT: {
-// CHECK-NEXT:  "id": "0x{{.*}}",
-// CHECK-NEXT:  "kind": "ParenType",
-// CHECK-NEXT:  "type": {
-// CHECK-NEXT:   "qualType": "void ()"
-// CHECK-NEXT:  },
-// CHECK-NEXT:  "inner": [
-// CHECK-NEXT:   {
-// CHECK-NEXT:"id": "0x{{.*}}",
-// CHECK-NEXT:"kind": "FunctionProtoType",
-// CHECK-NEXT:"type": {
-// CHECK-NEXT: "qualType": "void ()"
-// CHECK-NEXT:},
-// CHECK-NEXT:"cc": "cdecl",
-// CHECK-NEXT:"inner": [

[PATCH] D139868: [clang][dataflow] Change the diagnoser API to receive a correctly typed lattice element Previously, the diagnoser could only receive the Environment at a given program point. Now, it

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
merrymeerkat requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

...lattice element.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139868

Files:
  clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1314,7 +1314,8 @@
 [&Diagnostics,
  Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
 ASTContext &Ctx, const CFGElement &Elt,
-const TypeErasedDataflowAnalysisState &State) mutable {
+const TransferStateForDiagnostics
+&State) mutable {
   auto EltDiagnostics =
   Diagnoser.diagnose(Ctx, &Elt, State.Env);
   llvm::move(EltDiagnostics, std::back_inserter(Diagnostics));
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -30,6 +30,7 @@
 #include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Serialization/PCHContainerOperations.h"
@@ -116,11 +117,26 @@
 SetupTest = std::move(Arg);
 return std::move(*this);
   }
+  AnalysisInputs &&withPostVisitCFG(
+  std::function &)>
+  Arg) && {
+PostVisitCFG = std::move(Arg);
+return std::move(*this);
+  }
+
   AnalysisInputs &&
   withPostVisitCFG(std::function
Arg) && {
-PostVisitCFG = std::move(Arg);
+PostVisitCFG =
+[=](ASTContext &Context, const CFGElement &Element,
+const TransferStateForDiagnostics
+&State) {
+  Arg(Context, Element,
+  TypeErasedDataflowAnalysisState({State.Lattice}, State.Env));
+};
 return std::move(*this);
   }
   AnalysisInputs &&withASTBuildArgs(ArrayRef Arg) && {
@@ -148,8 +164,9 @@
   std::function SetupTest = nullptr;
   /// Optional. If provided, this function is applied on each CFG element after
   /// the analysis has been run.
-  std::function
+  std::function &)>
   PostVisitCFG = nullptr;
 
   /// Optional. Options for building the AST context.
@@ -226,11 +243,15 @@
  const TypeErasedDataflowAnalysisState &)>
   PostVisitCFGClosure = nullptr;
   if (AI.PostVisitCFG) {
-PostVisitCFGClosure =
-[&AI, &Context](const CFGElement &Element,
-const TypeErasedDataflowAnalysisState &State) {
-  AI.PostVisitCFG(Context, Element, State);
-};
+PostVisitCFGClosure = [&AI, &Context](
+  const CFGElement &Element,
+  const TypeErasedDataflowAnalysisState &State) {
+  AI.PostVisitCFG(Context, Element,
+  TransferStateForDiagnostics(
+  llvm::any_cast(
+  State.Lattice.Value),
+  State.Env));
+};
   }
 
   // Additional test setup.
@@ -326,28 +347,28 @@
   // Save the states computed for program points immediately following annotated
   // statements. The saved states are keyed by the content of the annotation.
   llvm::StringMap AnnotationStates;
-  auto PostVisitCFG = [&StmtToAnnotations, &AnnotationStates,
-   PrevPostVisitCFG = std::move(AI.PostVisitCFG)](
-  ASTContext &Ctx, const CFGElement &Elt,
-  const TypeErasedDataflowAnalysisState &State) {
-if (PrevPostVisitCFG) {
-  PrevPostVisitCFG(Ctx, Elt, State);
-}
-// FIXME: Extend retrieval of state for non statement constructs.
-auto Stmt = Elt.getAs();
-if (!Stmt)
-  return;
-auto It = StmtToAnnotations.find(Stmt->getStmt());
-if (It == StmtToAnnotations.end())
-  return;
-auto *Lattice =
-llvm::any_cast(&State.Lattice.Value);
-auto [_, InsertSuccess] =
-AnnotationStates.insert({It->second, StateT{*Lattice, State.Env}});
-(void)_;
-(void)InsertSuccess;
-assert(InsertSucce

[PATCH] D139868: [clang][dataflow] Change the diagnoser API to receive a correctly typed lattice element

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat updated this revision to Diff 482393.
merrymeerkat added a comment.

Address review comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139868

Files:
  clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1314,7 +1314,8 @@
 [&Diagnostics,
  Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
 ASTContext &Ctx, const CFGElement &Elt,
-const TypeErasedDataflowAnalysisState &State) mutable {
+const TransferStateForDiagnostics
+&State) mutable {
   auto EltDiagnostics =
   Diagnoser.diagnose(Ctx, &Elt, State.Env);
   llvm::move(EltDiagnostics, std::back_inserter(Diagnostics));
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -30,6 +30,7 @@
 #include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Serialization/PCHContainerOperations.h"
@@ -116,11 +117,26 @@
 SetupTest = std::move(Arg);
 return std::move(*this);
   }
+  AnalysisInputs &&withPostVisitCFG(
+  std::function &)>
+  Arg) && {
+PostVisitCFG = std::move(Arg);
+return std::move(*this);
+  }
+
   AnalysisInputs &&
   withPostVisitCFG(std::function
Arg) && {
-PostVisitCFG = std::move(Arg);
+PostVisitCFG =
+[Arg = std::move(Arg)](ASTContext &Context, const CFGElement &Element,
+const TransferStateForDiagnostics
+&State) {
+  Arg(Context, Element,
+  TypeErasedDataflowAnalysisState({State.Lattice}, State.Env));
+};
 return std::move(*this);
   }
   AnalysisInputs &&withASTBuildArgs(ArrayRef Arg) && {
@@ -148,8 +164,9 @@
   std::function SetupTest = nullptr;
   /// Optional. If provided, this function is applied on each CFG element after
   /// the analysis has been run.
-  std::function
+  std::function &)>
   PostVisitCFG = nullptr;
 
   /// Optional. Options for building the AST context.
@@ -226,11 +243,15 @@
  const TypeErasedDataflowAnalysisState &)>
   PostVisitCFGClosure = nullptr;
   if (AI.PostVisitCFG) {
-PostVisitCFGClosure =
-[&AI, &Context](const CFGElement &Element,
-const TypeErasedDataflowAnalysisState &State) {
-  AI.PostVisitCFG(Context, Element, State);
-};
+PostVisitCFGClosure = [&AI, &Context](
+  const CFGElement &Element,
+  const TypeErasedDataflowAnalysisState &State) {
+  AI.PostVisitCFG(Context, Element,
+  TransferStateForDiagnostics(
+  llvm::any_cast(
+  State.Lattice.Value),
+  State.Env));
+};
   }
 
   // Additional test setup.
@@ -326,28 +347,28 @@
   // Save the states computed for program points immediately following annotated
   // statements. The saved states are keyed by the content of the annotation.
   llvm::StringMap AnnotationStates;
-  auto PostVisitCFG = [&StmtToAnnotations, &AnnotationStates,
-   PrevPostVisitCFG = std::move(AI.PostVisitCFG)](
-  ASTContext &Ctx, const CFGElement &Elt,
-  const TypeErasedDataflowAnalysisState &State) {
-if (PrevPostVisitCFG) {
-  PrevPostVisitCFG(Ctx, Elt, State);
-}
-// FIXME: Extend retrieval of state for non statement constructs.
-auto Stmt = Elt.getAs();
-if (!Stmt)
-  return;
-auto It = StmtToAnnotations.find(Stmt->getStmt());
-if (It == StmtToAnnotations.end())
-  return;
-auto *Lattice =
-llvm::any_cast(&State.Lattice.Value);
-auto [_, InsertSuccess] =
-AnnotationStates.insert({It->second, StateT{*Lattice, State.Env}});
-(void)_;
-(void)InsertSuccess;
-assert(InsertSuccess);
-  };
+  auto PostVisitCFG =
+  [&StmtToAnnotations, &AnnotationStates,
+   P

[PATCH] D140037: [clang][dataflow] Remove old diagnoser API

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a project: All.
merrymeerkat requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a clean up following the revision D139868 
 (https://reviews.llvm.org/D139868).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140037

Files:
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h


Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -125,20 +125,6 @@
 PostVisitCFG = std::move(Arg);
 return std::move(*this);
   }
-
-  AnalysisInputs &&
-  withPostVisitCFG(std::function
-   Arg) && {
-PostVisitCFG =
-[Arg = std::move(Arg)](ASTContext &Context, const CFGElement &Element,
-const TransferStateForDiagnostics
-&State) {
-  Arg(Context, Element,
-  TypeErasedDataflowAnalysisState({State.Lattice}, State.Env));
-};
-return std::move(*this);
-  }
   AnalysisInputs &&withASTBuildArgs(ArrayRef Arg) && {
 ASTBuildArgs = std::move(Arg);
 return std::move(*this);


Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -125,20 +125,6 @@
 PostVisitCFG = std::move(Arg);
 return std::move(*this);
   }
-
-  AnalysisInputs &&
-  withPostVisitCFG(std::function
-   Arg) && {
-PostVisitCFG =
-[Arg = std::move(Arg)](ASTContext &Context, const CFGElement &Element,
-const TransferStateForDiagnostics
-&State) {
-  Arg(Context, Element,
-  TypeErasedDataflowAnalysisState({State.Lattice}, State.Env));
-};
-return std::move(*this);
-  }
   AnalysisInputs &&withASTBuildArgs(ArrayRef Arg) && {
 ASTBuildArgs = std::move(Arg);
 return std::move(*this);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140104: [clang][dataflow] Remove unused argument in getNullability

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat created this revision.
Herald added subscribers: martong, kadircet, arphaman, xazax.hun.
Herald added a project: All.
merrymeerkat requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

This change will allow users to call getNullability() without providing an 
ASTContext.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140104

Files:
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Type.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/Sema/SemaType.cpp
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -1330,8 +1330,7 @@
   if (T.isNull())
 return CXTypeNullability_Invalid;
 
-  ASTContext &Ctx = cxtu::getASTUnit(GetTU(CT))->getASTContext();
-  if (auto nullability = T->getNullability(Ctx)) {
+  if (auto nullability = T->getNullability()) {
 switch (*nullability) {
   case NullabilityKind::NonNull:
 return CXTypeNullability_NonNull;
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4701,8 +4701,8 @@
 // inner pointers.
 complainAboutMissingNullability = CAMN_InnerPointers;
 
-if (T->canHaveNullability(/*ResultIfUnknown*/false) &&
-!T->getNullability(S.Context)) {
+if (T->canHaveNullability(/*ResultIfUnknown*/ false) &&
+!T->getNullability()) {
   // Note that we allow but don't require nullability on dependent types.
   ++NumPointersRemaining;
 }
@@ -4923,8 +4923,8 @@
   // If the type itself could have nullability but does not, infer pointer
   // nullability and perform consistency checking.
   if (S.CodeSynthesisContexts.empty()) {
-if (T->canHaveNullability(/*ResultIfUnknown*/false) &&
-!T->getNullability(S.Context)) {
+if (T->canHaveNullability(/*ResultIfUnknown*/ false) &&
+!T->getNullability()) {
   if (isVaList(T)) {
 // Record that we've seen a pointer, but do nothing else.
 if (NumPointersRemaining > 0)
@@ -4947,9 +4947,8 @@
   }
 }
 
-if (complainAboutMissingNullability == CAMN_Yes &&
-T->isArrayType() && !T->getNullability(S.Context) && !isVaList(T) &&
-D.isPrototypeContext() &&
+if (complainAboutMissingNullability == CAMN_Yes && T->isArrayType() &&
+!T->getNullability() && !isVaList(T) && D.isPrototypeContext() &&
 !hasOuterPointerLikeChunk(D, D.getNumTypeObjects())) {
   checkNullabilityConsistency(S, SimplePointerKind::Array,
   D.getDeclSpec().getTypeSpecTypeLoc());
@@ -7389,7 +7388,7 @@
   // This (unlike the code above) looks through typedefs that might
   // have nullability specifiers on them, which means we cannot
   // provide a useful Fix-It.
-  if (auto existingNullability = desugared->getNullability(S.Context)) {
+  if (auto existingNullability = desugared->getNullability()) {
 if (nullability != *existingNullability) {
   S.Diag(nullabilityLoc, diag::err_nullability_conflicting)
 << DiagNullabilityKind(nullability, isContextSensitive)
@@ -7488,7 +7487,7 @@
   // If we started with an object pointer type, rebuild it.
   if (ptrType) {
 equivType = S.Context.getObjCObjectPointerType(equivType);
-if (auto nullability = type->getNullability(S.Context)) {
+if (auto nullability = type->getNullability()) {
   // We create a nullability attribute from the __kindof attribute.
   // Make sure that will make sense.
   assert(attr.getAttributeSpellingListIndex() == 0 &&
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -2754,7 +2754,7 @@
 
   if (Attributes & ObjCPropertyAttribute::kind_weak) {
 // 'weak' and 'nonnull' are mutually exclusive.
-if (auto nullability = PropertyTy->getNullability(Context)) {
+if (auto nullability = PropertyTy->getNullability()) {
   if (*nullability == NullabilityKind::NonNull)
 Diag(Loc, diag::err_objc_property_attr_mutually_exclusive)
   << "nonnull" << "weak";
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@

[PATCH] D140104: [clang][dataflow] Remove unused argument in getNullability

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat updated this revision to Diff 483345.
merrymeerkat added a comment.

Resolving review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140104

Files:
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Type.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/Sema/SemaType.cpp
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -1330,8 +1330,7 @@
   if (T.isNull())
 return CXTypeNullability_Invalid;
 
-  ASTContext &Ctx = cxtu::getASTUnit(GetTU(CT))->getASTContext();
-  if (auto nullability = T->getNullability(Ctx)) {
+  if (auto nullability = T->getNullability()) {
 switch (*nullability) {
   case NullabilityKind::NonNull:
 return CXTypeNullability_NonNull;
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4701,8 +4701,8 @@
 // inner pointers.
 complainAboutMissingNullability = CAMN_InnerPointers;
 
-if (T->canHaveNullability(/*ResultIfUnknown*/false) &&
-!T->getNullability(S.Context)) {
+if (T->canHaveNullability(/*ResultIfUnknown*/ false) &&
+!T->getNullability()) {
   // Note that we allow but don't require nullability on dependent types.
   ++NumPointersRemaining;
 }
@@ -4923,8 +4923,8 @@
   // If the type itself could have nullability but does not, infer pointer
   // nullability and perform consistency checking.
   if (S.CodeSynthesisContexts.empty()) {
-if (T->canHaveNullability(/*ResultIfUnknown*/false) &&
-!T->getNullability(S.Context)) {
+if (T->canHaveNullability(/*ResultIfUnknown*/ false) &&
+!T->getNullability()) {
   if (isVaList(T)) {
 // Record that we've seen a pointer, but do nothing else.
 if (NumPointersRemaining > 0)
@@ -4947,9 +4947,8 @@
   }
 }
 
-if (complainAboutMissingNullability == CAMN_Yes &&
-T->isArrayType() && !T->getNullability(S.Context) && !isVaList(T) &&
-D.isPrototypeContext() &&
+if (complainAboutMissingNullability == CAMN_Yes && T->isArrayType() &&
+!T->getNullability() && !isVaList(T) && D.isPrototypeContext() &&
 !hasOuterPointerLikeChunk(D, D.getNumTypeObjects())) {
   checkNullabilityConsistency(S, SimplePointerKind::Array,
   D.getDeclSpec().getTypeSpecTypeLoc());
@@ -7389,7 +7388,7 @@
   // This (unlike the code above) looks through typedefs that might
   // have nullability specifiers on them, which means we cannot
   // provide a useful Fix-It.
-  if (auto existingNullability = desugared->getNullability(S.Context)) {
+  if (auto existingNullability = desugared->getNullability()) {
 if (nullability != *existingNullability) {
   S.Diag(nullabilityLoc, diag::err_nullability_conflicting)
 << DiagNullabilityKind(nullability, isContextSensitive)
@@ -7488,7 +7487,7 @@
   // If we started with an object pointer type, rebuild it.
   if (ptrType) {
 equivType = S.Context.getObjCObjectPointerType(equivType);
-if (auto nullability = type->getNullability(S.Context)) {
+if (auto nullability = type->getNullability()) {
   // We create a nullability attribute from the __kindof attribute.
   // Make sure that will make sense.
   assert(attr.getAttributeSpellingListIndex() == 0 &&
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -2754,7 +2754,7 @@
 
   if (Attributes & ObjCPropertyAttribute::kind_weak) {
 // 'weak' and 'nonnull' are mutually exclusive.
-if (auto nullability = PropertyTy->getNullability(Context)) {
+if (auto nullability = PropertyTy->getNullability()) {
   if (*nullability == NullabilityKind::NonNull)
 Diag(Loc, diag::err_objc_property_attr_mutually_exclusive)
   << "nonnull" << "weak";
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -775,8 +775,8 @@
 if (Context.getCanonicalFunctionResultType(ReturnType) ==
   Context.getCanonicalFunctionResultType(CSI.ReturnType)) {
   // Use the return type 

[PATCH] D140104: [clang][dataflow] Remove unused argument in getNullability

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat updated this revision to Diff 483355.
merrymeerkat added a comment.

Address review comment (add temporary overload)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140104

Files:
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Type.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/Sema/SemaType.cpp
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -1330,8 +1330,7 @@
   if (T.isNull())
 return CXTypeNullability_Invalid;
 
-  ASTContext &Ctx = cxtu::getASTUnit(GetTU(CT))->getASTContext();
-  if (auto nullability = T->getNullability(Ctx)) {
+  if (auto nullability = T->getNullability()) {
 switch (*nullability) {
   case NullabilityKind::NonNull:
 return CXTypeNullability_NonNull;
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4701,8 +4701,8 @@
 // inner pointers.
 complainAboutMissingNullability = CAMN_InnerPointers;
 
-if (T->canHaveNullability(/*ResultIfUnknown*/false) &&
-!T->getNullability(S.Context)) {
+if (T->canHaveNullability(/*ResultIfUnknown*/ false) &&
+!T->getNullability()) {
   // Note that we allow but don't require nullability on dependent types.
   ++NumPointersRemaining;
 }
@@ -4923,8 +4923,8 @@
   // If the type itself could have nullability but does not, infer pointer
   // nullability and perform consistency checking.
   if (S.CodeSynthesisContexts.empty()) {
-if (T->canHaveNullability(/*ResultIfUnknown*/false) &&
-!T->getNullability(S.Context)) {
+if (T->canHaveNullability(/*ResultIfUnknown*/ false) &&
+!T->getNullability()) {
   if (isVaList(T)) {
 // Record that we've seen a pointer, but do nothing else.
 if (NumPointersRemaining > 0)
@@ -4947,9 +4947,8 @@
   }
 }
 
-if (complainAboutMissingNullability == CAMN_Yes &&
-T->isArrayType() && !T->getNullability(S.Context) && !isVaList(T) &&
-D.isPrototypeContext() &&
+if (complainAboutMissingNullability == CAMN_Yes && T->isArrayType() &&
+!T->getNullability() && !isVaList(T) && D.isPrototypeContext() &&
 !hasOuterPointerLikeChunk(D, D.getNumTypeObjects())) {
   checkNullabilityConsistency(S, SimplePointerKind::Array,
   D.getDeclSpec().getTypeSpecTypeLoc());
@@ -7389,7 +7388,7 @@
   // This (unlike the code above) looks through typedefs that might
   // have nullability specifiers on them, which means we cannot
   // provide a useful Fix-It.
-  if (auto existingNullability = desugared->getNullability(S.Context)) {
+  if (auto existingNullability = desugared->getNullability()) {
 if (nullability != *existingNullability) {
   S.Diag(nullabilityLoc, diag::err_nullability_conflicting)
 << DiagNullabilityKind(nullability, isContextSensitive)
@@ -7488,7 +7487,7 @@
   // If we started with an object pointer type, rebuild it.
   if (ptrType) {
 equivType = S.Context.getObjCObjectPointerType(equivType);
-if (auto nullability = type->getNullability(S.Context)) {
+if (auto nullability = type->getNullability()) {
   // We create a nullability attribute from the __kindof attribute.
   // Make sure that will make sense.
   assert(attr.getAttributeSpellingListIndex() == 0 &&
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -2754,7 +2754,7 @@
 
   if (Attributes & ObjCPropertyAttribute::kind_weak) {
 // 'weak' and 'nonnull' are mutually exclusive.
-if (auto nullability = PropertyTy->getNullability(Context)) {
+if (auto nullability = PropertyTy->getNullability()) {
   if (*nullability == NullabilityKind::NonNull)
 Diag(Loc, diag::err_objc_property_attr_mutually_exclusive)
   << "nonnull" << "weak";
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -775,8 +775,8 @@
 if (Context.getCanonicalFunctionResultType(ReturnType) ==
   Context.getCanonicalFunctionResultType(CSI.ReturnType)) {
   //

[PATCH] D140104: [clang][dataflow] Remove unused argument in getNullability

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat updated this revision to Diff 483460.
merrymeerkat added a comment.

Fix formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140104

Files:
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Type.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/Sema/SemaType.cpp
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -1330,8 +1330,7 @@
   if (T.isNull())
 return CXTypeNullability_Invalid;
 
-  ASTContext &Ctx = cxtu::getASTUnit(GetTU(CT))->getASTContext();
-  if (auto nullability = T->getNullability(Ctx)) {
+  if (auto nullability = T->getNullability()) {
 switch (*nullability) {
   case NullabilityKind::NonNull:
 return CXTypeNullability_NonNull;
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4701,8 +4701,8 @@
 // inner pointers.
 complainAboutMissingNullability = CAMN_InnerPointers;
 
-if (T->canHaveNullability(/*ResultIfUnknown*/false) &&
-!T->getNullability(S.Context)) {
+if (T->canHaveNullability(/*ResultIfUnknown*/ false) &&
+!T->getNullability()) {
   // Note that we allow but don't require nullability on dependent types.
   ++NumPointersRemaining;
 }
@@ -4923,8 +4923,8 @@
   // If the type itself could have nullability but does not, infer pointer
   // nullability and perform consistency checking.
   if (S.CodeSynthesisContexts.empty()) {
-if (T->canHaveNullability(/*ResultIfUnknown*/false) &&
-!T->getNullability(S.Context)) {
+if (T->canHaveNullability(/*ResultIfUnknown*/ false) &&
+!T->getNullability()) {
   if (isVaList(T)) {
 // Record that we've seen a pointer, but do nothing else.
 if (NumPointersRemaining > 0)
@@ -4947,9 +4947,8 @@
   }
 }
 
-if (complainAboutMissingNullability == CAMN_Yes &&
-T->isArrayType() && !T->getNullability(S.Context) && !isVaList(T) &&
-D.isPrototypeContext() &&
+if (complainAboutMissingNullability == CAMN_Yes && T->isArrayType() &&
+!T->getNullability() && !isVaList(T) && D.isPrototypeContext() &&
 !hasOuterPointerLikeChunk(D, D.getNumTypeObjects())) {
   checkNullabilityConsistency(S, SimplePointerKind::Array,
   D.getDeclSpec().getTypeSpecTypeLoc());
@@ -7389,7 +7388,7 @@
   // This (unlike the code above) looks through typedefs that might
   // have nullability specifiers on them, which means we cannot
   // provide a useful Fix-It.
-  if (auto existingNullability = desugared->getNullability(S.Context)) {
+  if (auto existingNullability = desugared->getNullability()) {
 if (nullability != *existingNullability) {
   S.Diag(nullabilityLoc, diag::err_nullability_conflicting)
 << DiagNullabilityKind(nullability, isContextSensitive)
@@ -7488,7 +7487,7 @@
   // If we started with an object pointer type, rebuild it.
   if (ptrType) {
 equivType = S.Context.getObjCObjectPointerType(equivType);
-if (auto nullability = type->getNullability(S.Context)) {
+if (auto nullability = type->getNullability()) {
   // We create a nullability attribute from the __kindof attribute.
   // Make sure that will make sense.
   assert(attr.getAttributeSpellingListIndex() == 0 &&
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -2754,7 +2754,7 @@
 
   if (Attributes & ObjCPropertyAttribute::kind_weak) {
 // 'weak' and 'nonnull' are mutually exclusive.
-if (auto nullability = PropertyTy->getNullability(Context)) {
+if (auto nullability = PropertyTy->getNullability()) {
   if (*nullability == NullabilityKind::NonNull)
 Diag(Loc, diag::err_objc_property_attr_mutually_exclusive)
   << "nonnull" << "weak";
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -775,8 +775,8 @@
 if (Context.getCanonicalFunctionResultType(ReturnType) ==
   Context.getCanonicalFunctionResultType(CSI.ReturnType)) {
   // Use the return type with the str

[PATCH] D140211: [clang][dataflow] Remove unused lambda capture

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a project: All.
merrymeerkat requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140211

Files:
  clang/lib/Sema/SemaExpr.cpp


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -9069,7 +9069,7 @@
   if (!ResTy->isAnyPointerType())
 return ResTy;
 
-  auto GetNullability = [&Ctx](QualType Ty) {
+  auto GetNullability = [](QualType Ty) {
 Optional Kind = Ty->getNullability();
 if (Kind) {
   // For our purposes, treat _Nullable_result as _Nullable.


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -9069,7 +9069,7 @@
   if (!ResTy->isAnyPointerType())
 return ResTy;
 
-  auto GetNullability = [&Ctx](QualType Ty) {
+  auto GetNullability = [](QualType Ty) {
 Optional Kind = Ty->getNullability();
 if (Kind) {
   // For our purposes, treat _Nullable_result as _Nullable.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140104: [clang][dataflow] Remove unused argument in getNullability

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:9072
 
   auto GetNullability = [&Ctx](QualType Ty) {
+Optional Kind = Ty->getNullability();

barannikov88 wrote:
> This now gives a warning
> `clang/lib/Sema/SemaExpr.cpp:9072:27: warning: lambda capture 'Ctx' is not 
> used [-Wunused-lambda-capture]`
Thank you for pointing this out! This patch should resolve this: 
https://reviews.llvm.org/D140211


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140104

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