date:20230521

[PATCH] D146809: [clang-repl] Implement Value pretty printing

2023-05-21 Thread Jun Zhang via Phabricator via cfe-commits

junaire updated this revision to Diff 524088.
junaire added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146809

Files:
  clang/include/clang/Interpreter/Interpreter.h
  clang/include/clang/Interpreter/Value.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/__clang_interpreter_runtime_printvalue.h
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/lib/Interpreter/IncrementalExecutor.h
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Interpreter/InterpreterUtils.cpp
  clang/lib/Interpreter/InterpreterUtils.h
  clang/lib/Interpreter/Value.cpp
  clang/lib/Interpreter/ValuePrinter.cpp
  clang/test/Interpreter/pretty-print.cpp
  clang/tools/clang-repl/CMakeLists.txt

Index: clang/tools/clang-repl/CMakeLists.txt
===
--- clang/tools/clang-repl/CMakeLists.txt
+++ clang/tools/clang-repl/CMakeLists.txt
@@ -11,6 +11,65 @@
   ClangRepl.cpp
   )
 
+if(MSVC)
+  set_target_properties(clang-repl PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS 1)
+
+  # RTTI/C++ symbols
+  set(clang_repl_exports ${clang_repl_exports} ??_7type_info@@6B@
+?__type_info_root_node@@3U__type_info_node@@A
+?nothrow@std@@3Unothrow_t@1@B
+  )
+
+  # Compiler added symbols for static variables. NOT for VStudio < 2015
+  set(clang_repl_exports ${clang_repl_exports} _Init_thread_abort _Init_thread_epoch
+_Init_thread_footer _Init_thread_header _tls_index
+  )
+
+  if(CMAKE_SIZEOF_VOID_P EQUAL 8)
+# new/delete variants needed when linking to static msvc runtime (esp. Debug)
+set(clang_repl_exports ${clang_repl_exports}
+  ??2@YAPEAX_K@Z
+  ??3@YAXPEAX@Z
+  ??_U@YAPEAX_K@Z
+  ??_V@YAXPEAX@Z
+  ??3@YAXPEAX_K@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QEAAAEAV01@H@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QEAAAEAV01@M@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QEAAAEAV01@N@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QEAAAEAV01@PEBX@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QEAAAEAV01@P6AAEAV01@AEAV01@@Z@Z
+  ??$?6U?$char_traits@D@std@@@std@@YAAEAV?$basic_ostream@DU?$char_traits@D@std@@@0@AEAV10@D@Z
+  ??$?6U?$char_traits@D@std@@@std@@YAAEAV?$basic_ostream@DU?$char_traits@D@std@@@0@AEAV10@PEBD@Z
+  ?_Facet_Register@std@@YAXPEAV_Facet_base@1@@Z
+)
+  else()
+set(clang_repl_exports ${clang_repl_exports}
+  ??2@YAPAXI@Z
+  ??3@YAXPAX@Z
+  ??3@YAXPAXI@Z
+  ??_U@YAPAXI@Z
+  ??_V@YAXPAX@Z
+  ??_V@YAXPAXI@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QAEAAV01@H@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QAEAAV01@M@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QAEAAV01@N@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QAEAAV01@PBX@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QAEAAV01@P6AAAV01@AAV01@@Z@Z
+  ??$?6U?$char_traits@D@std@@@std@@YAAAV?$basic_ostream@DU?$char_traits@D@std@@@0@AAV10@D@Z
+  ??$?6U?$char_traits@D@std@@@std@@YAAAV?$basic_ostream@DU?$char_traits@D@std@@@0@AAV10@PBD@Z
+  ?_Facet_Register@std@@YAXPAV_Facet_base@1@@Z
+)
+  endif()
+
+  # List to '/EXPORT:sym0 /EXPORT:sym1 /EXPORT:sym2 ...'
+  foreach(sym ${clang_repl_exports})
+set(clang_repl_link_str "${clang_repl_link_str} /EXPORT:${sym}")
+  endforeach(sym ${clang_repl_exports})
+
+  set_property(TARGET clang-repl APPEND_STRING PROPERTY LINK_FLAGS ${clang_repl_link_str})
+
+endif(MSVC)
+
 clang_target_link_libraries(clang-repl PRIVATE
   clangAST
   clangBasic
Index: clang/test/Interpreter/pretty-print.cpp
===
--- /dev/null
+++ clang/test/Interpreter/pretty-print.cpp
@@ -0,0 +1,180 @@
+// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
+// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck --check-prefix=CHECK-DRIVER %s
+// UNSUPPORTED: system-aix
+// CHECK-DRIVER: i = 10
+// RUN: cat %s | clang-repl -Xcc -fno-delayed-template-parsing | FileCheck %s
+extern "C" int printf(const char*,...);
+
+char c = 'a';
+c
+// CHECK: (char) 'a'
+
+const char* c_str = "Goodbye, world!";
+c_str
+// CHECK-NEXT: (const char *) "Goodbye, world!"
+
+const char* c_null_str = 0;
+c_null_str
+// CHECK-NEXT: (const char *) nullptr
+
+"Hello, world"
+// CHECK-NEXT: (const char[13]) "Hello, world"
+
+int x = 42;
+x
+// CHECK-NEXT: (int) 42
+
+&x
+// CHECK-NEXT: (int *) [[Addr:@0x.*]]
+
+x - 2
+// CHECK-NEXT: (int) 40
+
+float f = 4.2f;
+f
+// CHECK-NEXT: (float) 4.2f
+
+double d = 4.21;
+d
+// CHECK-NEXT: (double) 4.210
+
+struct S1{};
+S1 s1;
+s1
+// CHECK-NEXT: (S1 &) [[Addr:@0x.*]]
+
+S1{}
+// CHECK-NEXT: (S1) [[Addr:@0x.*]]
+
+struct S2 {int d;} E = {22};
+E
+// CHECK-NEXT: (struct S2 &) [[Addr:@0x.*]]
+E.d
+// CHECK-NEXT: (i

[PATCH] D146809: [clang-repl] Implement Value pretty printing

2023-05-21 Thread Jun Zhang via Phabricator via cfe-commits

junaire added a comment.

Invite more people to the party :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146809

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

[PATCH] D151033: [clang][AST] TextNodeDumper should not evaluate the initializer of constexpr variable template definition

2023-05-21 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added a comment.

Can you briefly explain why the `FieldDecl` we see is null in that case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151033

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

[clang] 8b1727f - [NFC] remove duplicated dash in release note for #62447

2023-05-21 Thread Congcong Cai via cfe-commits


Author: Congcong Cai
Date: 2023-05-21T11:26:16+02:00
New Revision: 8b1727f8d9104df5ced4bdfdc065dea85ff84baf

URL: 
https://github.com/llvm/llvm-project/commit/8b1727f8d9104df5ced4bdfdc065dea85ff84baf
DIFF: 
https://github.com/llvm/llvm-project/commit/8b1727f8d9104df5ced4bdfdc065dea85ff84baf.diff

LOG: [NFC] remove duplicated dash in release note for #62447

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ead3f0b5d8143..911bd9eb6435d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -392,7 +392,7 @@ Bug Fixes in This Version
 - Fix crash when attempting to perform parenthesized initialization of an
   aggregate with a base class with only non-public constructors.
   (`#62296 `_)
-- - Fix crash when redefining a variable with an invalid type again with an
+- Fix crash when redefining a variable with an invalid type again with an
   invalid type. (`#62447 `_)
 - Fix a stack overflow issue when evaluating ``consteval`` default arguments.
   (`#60082` `_)



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

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

2023-05-21 Thread Sedenion via Phabricator via cfe-commits

Sedeniono created this revision.
Sedeniono added a reviewer: curdeius.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.
Sedeniono requested review of this revision.

Fixes github issues #59178 and #58464.

The problem was that the LevelIndentTracker remembered
the indentation level of previous deeper levels when
leaving a scope. Afterwards, when it entered again a
deeper level, it blindly reused the the previous
indentation level. In case of the --lines option
configured such that the previous deeper level was not
formatted, that previous level was whatever happened
to be there in the source code. The formatter simply
believed it.

This is fixed by letting the LevelIndentTracker forget
the previous deeper levels when stepping out of them
(=> change in LevelIndentTracker::nextLine()).
Note that this used to be the case until LLVM 14.0.6,
but was changed in
https://github.com/llvm/llvm-project/issues/56352 to
fix a crash. Our commit here essentially reverts that
crash fix. It seemed to have been incorrect. The proper
fix is to set the AnnotedLine::Level of joined lines
correctly (=> change in LineJoiner::join()).

See
https://github.com/llvm/llvm-project/issues/59178#issuecomment-1542637781
for some more details.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151047

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTestSelective.cpp


Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -528,6 +528,26 @@
 format("  int a;\n"
"void ff() {}",
11, 0));
+
+  // https://github.com/llvm/llvm-project/issues/59178
+  Style = getMozillaStyle();
+  EXPECT_EQ("int a()\n"
+"{\n"
+"return 0;\n"
+"}\n"
+"int b()\n"
+"{\n"
+"  return 42;\n"
+"}",
+format("int a()\n"
+   "{\n"
+   "return 0;\n"
+   "}\n"
+   "int b()\n"
+   "{\n"
+   "return 42;\n" // Format this line only
+   "}",
+   32, 0));
 }
 
 TEST_F(FormatTestSelective, UnderstandsTabs) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -74,6 +74,12 @@
: Line.Level * PPIndentWidth;
   Indent += AdditionalIndent;
 } else {
+  // When going to lower levels, forget previous higher levels so that we
+  // recompute future higher levels. But don't forget them if we enter a PP
+  // directive, since these do not terminate a code block.
+  if (!Line.InPPDirective)
+IndentForLevel.resize(Line.Level + 1);
+
   Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
@@ -910,6 +916,7 @@
   Tok->TotalLength += LengthA;
   A.Last = Tok;
 }
+A.Level = B.Level;
   }
 
   const FormatStyle &Style;


Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -528,6 +528,26 @@
 format("  int a;\n"
"void ff() {}",
11, 0));
+
+  // https://github.com/llvm/llvm-project/issues/59178
+  Style = getMozillaStyle();
+  EXPECT_EQ("int a()\n"
+"{\n"
+"return 0;\n"
+"}\n"
+"int b()\n"
+"{\n"
+"  return 42;\n"
+"}",
+format("int a()\n"
+   "{\n"
+   "return 0;\n"
+   "}\n"
+   "int b()\n"
+   "{\n"
+   "return 42;\n" // Format this line only
+   "}",
+   32, 0));
 }
 
 TEST_F(FormatTestSelective, UnderstandsTabs) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -74,6 +74,12 @@
: Line.Level * PPIndentWidth;
   Indent += AdditionalIndent;
 } else {
+  // When going to lower levels, forget previous higher levels so that we
+  // recompute future higher levels. But don't forget them if we enter a PP
+  // directive, since these do not terminate a code block.
+  if (!Line.InPPDirective)
+IndentForLevel.resize(Line.Level + 1);
+
   Indent = getIndent(Line.Level);

[PATCH] D146809: [clang-repl] Implement Value pretty printing

2023-05-21 Thread Jun Zhang via Phabricator via cfe-commits

junaire updated this revision to Diff 524091.
junaire added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146809

Files:
  clang/include/clang/Interpreter/Interpreter.h
  clang/include/clang/Interpreter/Value.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/__clang_interpreter_runtime_printvalue.h
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/lib/Interpreter/IncrementalExecutor.h
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Interpreter/InterpreterUtils.cpp
  clang/lib/Interpreter/InterpreterUtils.h
  clang/lib/Interpreter/Value.cpp
  clang/lib/Interpreter/ValuePrinter.cpp
  clang/test/Interpreter/pretty-print.cpp
  clang/tools/clang-repl/CMakeLists.txt

Index: clang/tools/clang-repl/CMakeLists.txt
===
--- clang/tools/clang-repl/CMakeLists.txt
+++ clang/tools/clang-repl/CMakeLists.txt
@@ -11,6 +11,65 @@
   ClangRepl.cpp
   )
 
+if(MSVC)
+  set_target_properties(clang-repl PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS 1)
+
+  # RTTI/C++ symbols
+  set(clang_repl_exports ${clang_repl_exports} ??_7type_info@@6B@
+?__type_info_root_node@@3U__type_info_node@@A
+?nothrow@std@@3Unothrow_t@1@B
+  )
+
+  # Compiler added symbols for static variables. NOT for VStudio < 2015
+  set(clang_repl_exports ${clang_repl_exports} _Init_thread_abort _Init_thread_epoch
+_Init_thread_footer _Init_thread_header _tls_index
+  )
+
+  if(CMAKE_SIZEOF_VOID_P EQUAL 8)
+# new/delete variants needed when linking to static msvc runtime (esp. Debug)
+set(clang_repl_exports ${clang_repl_exports}
+  ??2@YAPEAX_K@Z
+  ??3@YAXPEAX@Z
+  ??_U@YAPEAX_K@Z
+  ??_V@YAXPEAX@Z
+  ??3@YAXPEAX_K@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QEAAAEAV01@H@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QEAAAEAV01@M@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QEAAAEAV01@N@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QEAAAEAV01@PEBX@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QEAAAEAV01@P6AAEAV01@AEAV01@@Z@Z
+  ??$?6U?$char_traits@D@std@@@std@@YAAEAV?$basic_ostream@DU?$char_traits@D@std@@@0@AEAV10@D@Z
+  ??$?6U?$char_traits@D@std@@@std@@YAAEAV?$basic_ostream@DU?$char_traits@D@std@@@0@AEAV10@PEBD@Z
+  ?_Facet_Register@std@@YAXPEAV_Facet_base@1@@Z
+)
+  else()
+set(clang_repl_exports ${clang_repl_exports}
+  ??2@YAPAXI@Z
+  ??3@YAXPAX@Z
+  ??3@YAXPAXI@Z
+  ??_U@YAPAXI@Z
+  ??_V@YAXPAX@Z
+  ??_V@YAXPAXI@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QAEAAV01@H@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QAEAAV01@M@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QAEAAV01@N@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QAEAAV01@PBX@Z
+  ??6?$basic_ostream@DU?$char_traits@D@std@@@std@@QAEAAV01@P6AAAV01@AAV01@@Z@Z
+  ??$?6U?$char_traits@D@std@@@std@@YAAAV?$basic_ostream@DU?$char_traits@D@std@@@0@AAV10@D@Z
+  ??$?6U?$char_traits@D@std@@@std@@YAAAV?$basic_ostream@DU?$char_traits@D@std@@@0@AAV10@PBD@Z
+  ?_Facet_Register@std@@YAXPAV_Facet_base@1@@Z
+)
+  endif()
+
+  # List to '/EXPORT:sym0 /EXPORT:sym1 /EXPORT:sym2 ...'
+  foreach(sym ${clang_repl_exports})
+set(clang_repl_link_str "${clang_repl_link_str} /EXPORT:${sym}")
+  endforeach(sym ${clang_repl_exports})
+
+  set_property(TARGET clang-repl APPEND_STRING PROPERTY LINK_FLAGS ${clang_repl_link_str})
+
+endif(MSVC)
+
 clang_target_link_libraries(clang-repl PRIVATE
   clangAST
   clangBasic
Index: clang/test/Interpreter/pretty-print.cpp
===
--- /dev/null
+++ clang/test/Interpreter/pretty-print.cpp
@@ -0,0 +1,180 @@
+// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
+// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck --check-prefix=CHECK-DRIVER %s
+// UNSUPPORTED: system-aix
+// CHECK-DRIVER: i = 10
+// RUN: cat %s | clang-repl -Xcc -fno-delayed-template-parsing | FileCheck %s
+extern "C" int printf(const char*,...);
+
+char c = 'a';
+c
+// CHECK: (char) 'a'
+
+const char* c_str = "Goodbye, world!";
+c_str
+// CHECK-NEXT: (const char *) "Goodbye, world!"
+
+const char* c_null_str = 0;
+c_null_str
+// CHECK-NEXT: (const char *) nullptr
+
+"Hello, world"
+// CHECK-NEXT: (const char[13]) "Hello, world"
+
+int x = 42;
+x
+// CHECK-NEXT: (int) 42
+
+&x
+// CHECK-NEXT: (int *) [[Addr:@0x.*]]
+
+x - 2
+// CHECK-NEXT: (int) 40
+
+float f = 4.2f;
+f
+// CHECK-NEXT: (float) 4.2f
+
+double d = 4.21;
+d
+// CHECK-NEXT: (double) 4.210
+
+struct S1{};
+S1 s1;
+s1
+// CHECK-NEXT: (S1 &) [[Addr:@0x.*]]
+
+S1{}
+// CHECK-NEXT: (S1) [[Addr:@0x.*]]
+
+struct S2 {int d;} E = {22};
+E
+// CHECK-NEXT: (struct S2 &) [[Addr:@0x.*]]
+E.d
+// CHECK-NEXT: (i

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

2023-05-21 Thread Sedenion via Phabricator via cfe-commits

Sedeniono added a comment.

The build failures seem to be unrelated to my changes. The builds of other 
reviews also show them, and the error messages point to things I haven't 
changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151047

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

[PATCH] D146809: [clang-repl] Implement Value pretty printing

2023-05-21 Thread Jun Zhang via Phabricator via cfe-commits

junaire updated this revision to Diff 524093.
junaire added a comment.

add `caas` namespace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146809

Files:
  clang/include/clang/Interpreter/Interpreter.h
  clang/include/clang/Interpreter/PartialTranslationUnit.h
  clang/include/clang/Interpreter/Value.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/__clang_interpreter_runtime_printvalue.h
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/lib/Interpreter/IncrementalExecutor.h
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Interpreter/InterpreterUtils.cpp
  clang/lib/Interpreter/InterpreterUtils.h
  clang/lib/Interpreter/Value.cpp
  clang/lib/Interpreter/ValuePrinter.cpp
  clang/test/Interpreter/pretty-print.cpp
  clang/tools/clang-repl/CMakeLists.txt
  clang/tools/clang-repl/ClangRepl.cpp
  clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
  clang/unittests/Interpreter/IncrementalProcessingTest.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -29,6 +29,7 @@
 #include "gtest/gtest.h"
 
 using namespace clang;
+using namespace caas;
 
 #if defined(_AIX)
 #define CLANG_INTERPRETER_NO_SUPPORT_EXEC
@@ -46,10 +47,11 @@
   DiagnosticConsumer *Client = nullptr) {
   Args ClangArgs = {"-Xclang", "-emit-llvm-only"};
   ClangArgs.insert(ClangArgs.end(), ExtraArgs.begin(), ExtraArgs.end());
-  auto CI = cantFail(clang::IncrementalCompilerBuilder::create(ClangArgs));
+  auto CI =
+  cantFail(clang::caas::IncrementalCompilerBuilder::create(ClangArgs));
   if (Client)
 CI->getDiagnostics().setClient(Client, /*ShouldOwnClient=*/false);
-  return cantFail(clang::Interpreter::create(std::move(CI)));
+  return cantFail(clang::caas::Interpreter::create(std::move(CI)));
 }
 
 static size_t DeclsSize(TranslationUnitDecl *PTUDecl) {
Index: clang/unittests/Interpreter/IncrementalProcessingTest.cpp
===
--- clang/unittests/Interpreter/IncrementalProcessingTest.cpp
+++ clang/unittests/Interpreter/IncrementalProcessingTest.cpp
@@ -27,6 +27,7 @@
 
 using namespace llvm;
 using namespace clang;
+using namespace clang::caas;
 
 namespace {
 
@@ -55,7 +56,7 @@
   auto CI = llvm::cantFail(IncrementalCompilerBuilder::create(ClangArgv));
   auto Interp = llvm::cantFail(Interpreter::create(std::move(CI)));
 
-  std::array PTUs;
+  std::array PTUs;
 
   PTUs[0] = &llvm::cantFail(Interp->Parse(TestProgram1));
   ASSERT_TRUE(PTUs[0]->TheModule);
Index: clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
===
--- clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
+++ clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
@@ -30,6 +30,7 @@
 #include "gtest/gtest.h"
 
 using namespace clang;
+using namespace clang::caas;
 
 namespace {
 using Args = std::vector;
@@ -38,10 +39,11 @@
   DiagnosticConsumer *Client = nullptr) {
   Args ClangArgs = {"-Xclang", "-emit-llvm-only"};
   ClangArgs.insert(ClangArgs.end(), ExtraArgs.begin(), ExtraArgs.end());
-  auto CI = cantFail(clang::IncrementalCompilerBuilder::create(ClangArgs));
+  auto CI =
+  cantFail(clang::caas::IncrementalCompilerBuilder::create(ClangArgs));
   if (Client)
 CI->getDiagnostics().setClient(Client, /*ShouldOwnClient=*/false);
-  return cantFail(clang::Interpreter::create(std::move(CI)));
+  return cantFail(clang::caas::Interpreter::create(std::move(CI)));
 }
 
 TEST(InterpreterTest, CatchException) {
Index: clang/tools/clang-repl/ClangRepl.cpp
===
--- clang/tools/clang-repl/ClangRepl.cpp
+++ clang/tools/clang-repl/ClangRepl.cpp
@@ -92,7 +92,8 @@
 
   // FIXME: Investigate if we could use runToolOnCodeWithArgs from tooling. It
   // can replace the boilerplate code for creation of the compiler instance.
-  auto CI = ExitOnErr(clang::IncrementalCompilerBuilder::create(ClangArgv));
+  auto CI =
+  ExitOnErr(clang::caas::IncrementalCompilerBuilder::create(ClangArgv));
 
   // Set an error handler, so that any LLVM backend diagnostics go through our
   // error handler.
@@ -102,7 +103,7 @@
   // Load any requested plugins.
   CI->LoadRequestedPlugins();
 
-  auto Interp = ExitOnErr(clang::Interpreter::create(std::move(CI)));
+  auto Interp = ExitOnErr(clang::caas::Interpreter::create(std::move(CI)));
   for (const std::string &input : OptInputs) {
 if (auto Err = Interp->ParseAndExecute(input))
   llvm::

[PATCH] D151033: [clang][AST] TextNodeDumper should not evaluate the initializer of constexpr variable template definition

2023-05-21 Thread Takuya Shimizu via Phabricator via cfe-commits

hazohelet added a comment.

In D151033#4359121 , @tbaeder wrote:

> Can you briefly explain why the `FieldDecl` we see is null in that case?

Consider `template constexpr T foo{};`

`evaluateValue` here leads to a call of `EvaluateAsInitializer`. It calls
`CheckConstantExpression`, which leads to the call of `CheckEvaluationResult`
with null `FieldDecl`.
The `APValue`-typed `Value` argument passed to `CheckConstantExpression` should
`hasValue` bacause otherwise it would cause assertion failure we saw before.
This `Value` object is calculated at `Evaluate` function called from
`EvaluateAsInitializer`.
The evaluated `Expr` object is `InitListExpr` that has type `void`, so the
branch for `void`-typed
expression(https://github.com/llvm/llvm-project/blob/8b1727f8d9104df5ced4bdfdc065dea85ff84baf/clang/lib/AST/ExprConstant.cpp#L15041-L15046)
is fired.
In this branch, the `Result` argument is left untouched, which leads to
`CheckEvaluationResult` receiving `Value` object whose `hasValue` evaluates to
`false`.

BTW, I noticed that when the type of the declaration is NOT a dependent type
(e.g. `template constexpr int foo{42}` ), we need to evaluate the
initializer and dump the value. I'll update the patch and retitle it.

Repository:
rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151033

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

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

2023-05-21 Thread Elliot Goodrich via Phabricator via cfe-commits

IncludeGuardian updated this revision to Diff 524094.
Herald added subscribers: bviyer, Moerafaat, zero9178, bzcheeseman, sdasgup3, 
wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, 
Kayjukh, grosul1, Joonsoo, stephenneuendorffer, liufengdb, aartbik, mgester, 
arpith-jacob, nicolasvasilache, antiagainst, shauheen, rriddle, mehdi_amini.
Herald added a reviewer: rriddle.
Herald added a project: MLIR.

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

https://reviews.llvm.org/D150997

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
  llvm/include/llvm/ADT/DenseMapInfo.h
  llvm/include/llvm/ADT/DenseMapInfoVariant.h
  llvm/include/llvm/CodeGen/CallingConvLower.h
  llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
  llvm/include/llvm/Object/DXContainer.h
  llvm/include/llvm/Transforms/Scalar/SROA.h
  mlir/include/mlir/IR/AsmState.h

Index: mlir/include/mlir/IR/AsmState.h
===
--- mlir/include/mlir/IR/AsmState.h
+++ mlir/include/mlir/IR/AsmState.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/StringMap.h"
 
 #include 
+#include 
 
 namespace mlir {
 class AsmResourcePrinter;
Index: llvm/include/llvm/Transforms/Scalar/SROA.h
===
--- llvm/include/llvm/Transforms/Scalar/SROA.h
+++ llvm/include/llvm/Transforms/Scalar/SROA.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/ValueHandle.h"
+#include 
 #include 
 
 namespace llvm {
Index: llvm/include/llvm/Object/DXContainer.h
===
--- llvm/include/llvm/Object/DXContainer.h
+++ llvm/include/llvm/Object/DXContainer.h
@@ -21,6 +21,7 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/TargetParser/Triple.h"
+#include 
 
 namespace llvm {
 namespace object {
Index: llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
===
--- llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
+++ llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
@@ -22,6 +22,7 @@
 #include "llvm/ExecutionEngine/Orc/ThreadSafeModule.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ThreadPool.h"
+#include 
 
 namespace llvm {
 namespace orc {
Index: llvm/include/llvm/CodeGen/CallingConvLower.h
===
--- llvm/include/llvm/CodeGen/CallingConvLower.h
+++ llvm/include/llvm/CodeGen/CallingConvLower.h
@@ -19,6 +19,7 @@
 #include "llvm/CodeGen/TargetCallingConv.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/Support/Alignment.h"
+#include 
 
 namespace llvm {
 
Index: llvm/include/llvm/ADT/DenseMapInfoVariant.h
===
--- /dev/null
+++ llvm/include/llvm/ADT/DenseMapInfoVariant.h
@@ -0,0 +1,56 @@
+//===- DenseMapInfoVariant.h - Type traits for DenseMap *- 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
+//
+//===--===//
+///
+/// \file
+/// This file defines DenseMapInfo traits for DenseMap>.
+///
+//===--===//
+
+#ifndef LLVM_ADT_DENSEMAPINFOVARIANT_H
+#define LLVM_ADT_DENSEMAPINFOVARIANT_H
+
+#include "llvm/ADT/DenseMapInfo.h"
+#include 
+#include 
+
+namespace llvm {
+
+// Provide DenseMapInfo for variants whose all alternatives have DenseMapInfo.
+template  struct DenseMapInfo> {
+  using Variant = std::variant;
+  using FirstT = std::variant_alternative_t<0, Variant>;
+
+  static inline Variant getEmptyKey() {
+return Variant(std::in_place_index<0>, DenseMapInfo::getEmptyKey());
+  }
+
+  static inline Variant getTombstoneKey() {
+return Variant(std::in_place_index<0>,
+   DenseMapInfo::getTombstoneKey());
+  }
+
+  static unsigned getHashValue(const Variant &Val) {
+return std::visit(
+[&Val](auto &&Alternative) {
+  using T = std::decay_t;
+  // Include index in hash to make sure same value as different
+  // alternatives don't collide.
+  return DenseMapInfo>::getHashValuePiecewise(
+  Val.index(), Alternative);
+},
+Val);
+  }
+
+  static bool isEqual(const Variant &LHS, const Variant &RHS) {
+return LHS == RHS;
+  }
+};
+
+} // end namespace llvm
+
+#endif // LLVM_ADT_DENSEMAPINFOVARIANT_H
Index: llvm/include/llvm/ADT/DenseMapInfo.h
===
--- llvm/include/llvm/ADT/DenseMapInfo.h
+++ llvm/include/llvm/ADT/DenseMapInfo.h
@@ -20,7 +20,6 @@
 #include 
 #include 
 #include 
-#include 
 
 namespace llvm {
 
@@ -234,6

[PATCH] D151033: [clang][AST] TextNodeDumper should not evaluate the initializer of constexpr variable declaration when it has a dependent type

2023-05-21 Thread Takuya Shimizu via Phabricator via cfe-commits

hazohelet updated this revision to Diff 524095.
hazohelet retitled this revision from "[clang][AST] TextNodeDumper should not 
evaluate the initializer of constexpr variable template definition" to 
"[clang][AST] TextNodeDumper should not evaluate the initializer of constexpr 
variable declaration when it has a dependent type".
hazohelet edited the summary of this revision.
hazohelet added a comment.

- Evaluate the initializer if the type of the declaration is not a dependent 
type
- Add a relevant test
- Update release note for this change


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

https://reviews.llvm.org/D151033

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/TextNodeDumper.cpp
  clang/test/AST/ast-dump-decl.cpp


Index: clang/test/AST/ast-dump-decl.cpp
===
--- clang/test/AST/ast-dump-decl.cpp
+++ clang/test/AST/ast-dump-decl.cpp
@@ -818,3 +818,38 @@
 // CHECK:   `-TextComment
 // CHECK: VarDecl {{.*}} Test 'int' extern
 // CHECK-NOT: FullComment
+
+namespace TestConstexprVariableTemplateWithInitializer {
+  template constexpr T foo{};
+  // CHECK:  VarTemplateDecl 0x{{.+}} <{{.+}}:[[@LINE-1]]:3, col:40> 
col:36 foo
+  // CHECK-NEXT: |-TemplateTypeParmDecl 0x{{.+}}  col:21 
referenced typename depth 0 index 0 T
+  // CHECK-NEXT: `-VarDecl 0x{{.+}}  col:36 foo 'const T' 
constexpr listinit
+  // CHECK-NEXT:  `-InitListExpr 0x{{.+}}  'void'
+
+  template constexpr int val{42};
+  // CHECK:  VarTemplateDecl 0x{{.+}} <{{.+}}:[[@LINE-1]]:3, col:44> 
col:38 val
+  // CHECK-NEXT: |-TemplateTypeParmDecl 0x{{.+}}  col:21 
typename depth 0 index 0 T
+  // CHECK-NEXT: `-VarDecl 0x{{.+}}  col:38 val 'const int' 
constexpr listinit
+  // CHECK-NEXT:  |-value: Int 42
+  // CHECK-NEXT:  `-InitListExpr 0x{{.+}}  'int'
+
+  template 
+  struct in_place_type_t {
+explicit in_place_type_t() = default;
+  };
+
+  template 
+  inline constexpr in_place_type_t<_Tp> in_place_type{};
+  // CHECK: -VarTemplateDecl 0x{{.+}}  col:41 in_place_type
+  // CHECK-NEXT: |-TemplateTypeParmDecl 0x{{.+}}  
col:22 referenced typename depth 0 index 0 _Tp
+  // CHECK-NEXT: `-VarDecl 0x{{.+}}  col:41 
in_place_type 'const in_place_type_t<_Tp>':'const in_place_type_t<_Tp>' inline 
constexpr listinit
+  // CHECK-NEXT:  `-InitListExpr 0x{{.+}}  'void'
+
+  template  constexpr T call_init(0);
+  // CHECK: -VarTemplateDecl 0x{{.+}}  col:37 
call_init
+  // CHECK-NEXT: |-TemplateTypeParmDecl 0x{{.+}}  col:22 
referenced typename depth 0 index 0 T
+  // CHECK-NEXT: `-VarDecl 0x{{.+}}  col:37 call_init 'const 
T' constexpr callinit
+  // CHECK-NEXT:  `-ParenListExpr 0x{{.+}}  'NULL TYPE'
+  // CHECK-NEXT:   `-IntegerLiteral 0x{{.+}}  'int' 0
+
+}
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -1821,7 +1821,8 @@
   if (D->hasInit()) {
 const Expr *E = D->getInit();
 // Only dump the value of constexpr VarDecls for now.
-if (E && !E->isValueDependent() && D->isConstexpr()) {
+if (E && !E->isValueDependent() && D->isConstexpr() &&
+!D->getType()->isDependentType()) {
   const APValue *Value = D->evaluateValue();
   if (Value)
 AddChild("value", [=] { Visit(*Value, E->getType()); });
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -418,6 +418,8 @@
 - Propagate the value-dependent bit for VAArgExpr. Fixes a crash where a
   __builtin_va_arg call has invalid arguments.
   (`#62711 `_).
+- Clang `TextNodeDumper` enabled through `-ast-dump` flag no longer evaluates 
the
+  initializer of constexpr `VarDecl` if the declaration has a dependent type.
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/AST/ast-dump-decl.cpp
===
--- clang/test/AST/ast-dump-decl.cpp
+++ clang/test/AST/ast-dump-decl.cpp
@@ -818,3 +818,38 @@
 // CHECK:   `-TextComment
 // CHECK: VarDecl {{.*}} Test 'int' extern
 // CHECK-NOT: FullComment
+
+namespace TestConstexprVariableTemplateWithInitializer {
+  template constexpr T foo{};
+  // CHECK:  VarTemplateDecl 0x{{.+}} <{{.+}}:[[@LINE-1]]:3, col:40> col:36 foo
+  // CHECK-NEXT: |-TemplateTypeParmDecl 0x{{.+}}  col:21 referenced typename depth 0 index 0 T
+  // CHECK-NEXT: `-VarDecl 0x{{.+}}  col:36 foo 'const T' constexpr listinit
+  // CHECK-NEXT:  `-InitListExpr 0x{{.+}}  'void'
+
+  template constexpr int val{42};
+  // CHECK:  VarTemplateDecl 0x{{.+}} <{{.+}}:[[@LINE-1]]:3, col:44> col:38 val
+  // CHECK-NEXT: |-TemplateTypeParmDecl 0x{{.+}}  col:21 typename depth 0 index 0 T
+  // CHECK-NEXT: `-VarDecl 0x{{.+}}  col:38 val 'const int' constexpr listinit
+  // C

[PATCH] D146809: [clang-repl] Implement Value pretty printing

2023-05-21 Thread Jun Zhang via Phabricator via cfe-commits

junaire updated this revision to Diff 524096.
junaire added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146809

Files:
  clang/include/clang/Interpreter/Interpreter.h
  clang/include/clang/Interpreter/PartialTranslationUnit.h
  clang/include/clang/Interpreter/Value.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/__clang_interpreter_runtime_printvalue.h
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/lib/Interpreter/IncrementalExecutor.h
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Interpreter/InterpreterUtils.cpp
  clang/lib/Interpreter/InterpreterUtils.h
  clang/lib/Interpreter/Value.cpp
  clang/lib/Interpreter/ValuePrinter.cpp
  clang/test/Interpreter/pretty-print.cpp
  clang/tools/clang-repl/CMakeLists.txt
  clang/tools/clang-repl/ClangRepl.cpp
  clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
  clang/unittests/Interpreter/IncrementalProcessingTest.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -29,6 +29,7 @@
 #include "gtest/gtest.h"
 
 using namespace clang;
+using namespace caas;
 
 #if defined(_AIX)
 #define CLANG_INTERPRETER_NO_SUPPORT_EXEC
@@ -46,10 +47,11 @@
   DiagnosticConsumer *Client = nullptr) {
   Args ClangArgs = {"-Xclang", "-emit-llvm-only"};
   ClangArgs.insert(ClangArgs.end(), ExtraArgs.begin(), ExtraArgs.end());
-  auto CI = cantFail(clang::IncrementalCompilerBuilder::create(ClangArgs));
+  auto CI =
+  cantFail(clang::caas::IncrementalCompilerBuilder::create(ClangArgs));
   if (Client)
 CI->getDiagnostics().setClient(Client, /*ShouldOwnClient=*/false);
-  return cantFail(clang::Interpreter::create(std::move(CI)));
+  return cantFail(clang::caas::Interpreter::create(std::move(CI)));
 }
 
 static size_t DeclsSize(TranslationUnitDecl *PTUDecl) {
Index: clang/unittests/Interpreter/IncrementalProcessingTest.cpp
===
--- clang/unittests/Interpreter/IncrementalProcessingTest.cpp
+++ clang/unittests/Interpreter/IncrementalProcessingTest.cpp
@@ -27,6 +27,7 @@
 
 using namespace llvm;
 using namespace clang;
+using namespace clang::caas;
 
 namespace {
 
@@ -55,7 +56,7 @@
   auto CI = llvm::cantFail(IncrementalCompilerBuilder::create(ClangArgv));
   auto Interp = llvm::cantFail(Interpreter::create(std::move(CI)));
 
-  std::array PTUs;
+  std::array PTUs;
 
   PTUs[0] = &llvm::cantFail(Interp->Parse(TestProgram1));
   ASSERT_TRUE(PTUs[0]->TheModule);
Index: clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
===
--- clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
+++ clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
@@ -30,6 +30,7 @@
 #include "gtest/gtest.h"
 
 using namespace clang;
+using namespace clang::caas;
 
 namespace {
 using Args = std::vector;
@@ -38,10 +39,11 @@
   DiagnosticConsumer *Client = nullptr) {
   Args ClangArgs = {"-Xclang", "-emit-llvm-only"};
   ClangArgs.insert(ClangArgs.end(), ExtraArgs.begin(), ExtraArgs.end());
-  auto CI = cantFail(clang::IncrementalCompilerBuilder::create(ClangArgs));
+  auto CI =
+  cantFail(clang::caas::IncrementalCompilerBuilder::create(ClangArgs));
   if (Client)
 CI->getDiagnostics().setClient(Client, /*ShouldOwnClient=*/false);
-  return cantFail(clang::Interpreter::create(std::move(CI)));
+  return cantFail(clang::caas::Interpreter::create(std::move(CI)));
 }
 
 TEST(InterpreterTest, CatchException) {
Index: clang/tools/clang-repl/ClangRepl.cpp
===
--- clang/tools/clang-repl/ClangRepl.cpp
+++ clang/tools/clang-repl/ClangRepl.cpp
@@ -92,7 +92,8 @@
 
   // FIXME: Investigate if we could use runToolOnCodeWithArgs from tooling. It
   // can replace the boilerplate code for creation of the compiler instance.
-  auto CI = ExitOnErr(clang::IncrementalCompilerBuilder::create(ClangArgv));
+  auto CI =
+  ExitOnErr(clang::caas::IncrementalCompilerBuilder::create(ClangArgv));
 
   // Set an error handler, so that any LLVM backend diagnostics go through our
   // error handler.
@@ -102,7 +103,7 @@
   // Load any requested plugins.
   CI->LoadRequestedPlugins();
 
-  auto Interp = ExitOnErr(clang::Interpreter::create(std::move(CI)));
+  auto Interp = ExitOnErr(clang::caas::Interpreter::create(std::move(CI)));
   for (const std::string &input : OptInputs) {
 if (auto Err = Interp->ParseAndExecute(input))
   llvm::logAllUnhandledErro

[PATCH] D151010: [NFC][CLANG] Fix issue with dereference null return value found by Coverity static analyzer tool

2023-05-21 Thread Soumi Manna via Phabricator via cfe-commits

Manna abandoned this revision.
Manna added a comment.

This is False Positive. We are correctly checking type mismatch.


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

https://reviews.llvm.org/D151010

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

[clang] bc9fda0 - Revert "Reland "[Driver] Support multi /guard: options""

2023-05-21 Thread Phoebe Wang via cfe-commits


Author: Phoebe Wang
Date: 2023-05-21T22:39:37+08:00
New Revision: bc9fda01ad1341519694bbf1e1b738298e3395f3

URL: 
https://github.com/llvm/llvm-project/commit/bc9fda01ad1341519694bbf1e1b738298e3395f3
DIFF: 
https://github.com/llvm/llvm-project/commit/bc9fda01ad1341519694bbf1e1b738298e3395f3.diff

LOG: Revert "Reland "[Driver] Support multi /guard: options""

This reverts commit a4f366dcd85c440a611bbc82f1d24c2d9a735251.

Found a problem during backport it to 16.x branch.
https://github.com/llvm/llvm-project-release-prs/actions/runs/5036930270/jobs/9033427592?pr=451

The reason is `-implicit-check-not='warning'` may conflict with option
name, e.g., `-treat-scalable-fixed-error-as-warning`

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/MSVC.cpp
clang/test/Driver/cl-options.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 809440b196fd8..a55fe1cedb2ef 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7876,7 +7876,7 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID 
InputType,
   if (Args.hasArg(options::OPT__SLASH_kernel))
 CmdArgs.push_back("-fms-kernel");
 
-  for (const Arg *A : Args.filtered(options::OPT__SLASH_guard)) {
+  if (Arg *A = Args.getLastArg(options::OPT__SLASH_guard)) {
 StringRef GuardArgs = A->getValue();
 // The only valid options are "cf", "cf,nochecks", "cf-", "ehcont" and
 // "ehcont-".
@@ -7895,7 +7895,6 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID 
InputType,
 } else {
   D.Diag(diag::err_drv_invalid_value) << A->getSpelling() << GuardArgs;
 }
-A->claim();
   }
 }
 

diff  --git a/clang/lib/Driver/ToolChains/MSVC.cpp 
b/clang/lib/Driver/ToolChains/MSVC.cpp
index d803ddad504c6..b23d0aa2f1b83 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -227,7 +227,7 @@ void visualstudio::Linker::ConstructJob(Compilation &C, 
const JobAction &JA,
   Args.AddAllArgValues(CmdArgs, options::OPT__SLASH_link);
 
   // Control Flow Guard checks
-  for (const Arg *A : Args.filtered(options::OPT__SLASH_guard)) {
+  if (Arg *A = Args.getLastArg(options::OPT__SLASH_guard)) {
 StringRef GuardArgs = A->getValue();
 if (GuardArgs.equals_insensitive("cf") ||
 GuardArgs.equals_insensitive("cf,nochecks")) {

diff  --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 781298bea7b94..12d7023bd61b1 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -647,13 +647,6 @@
 // RUN: %clang_cl /guard:ehcont -### -- %s 2>&1 | FileCheck 
-check-prefix=EHCONTGUARD %s
 // EHCONTGUARD: -ehcontguard
 
-// RUN: %clang_cl /guard:cf /guard:ehcont -Wall -Wno-msvc-not-found -### -- %s 
2>&1 | \
-// RUN:   FileCheck -check-prefix=BOTHGUARD %s --implicit-check-not=warning
-// BOTHGUARD: -cfguard
-// BOTHGUARD-SAME: -ehcontguard
-// BOTHGUARD: -guard:cf
-// BOTHGUARD-SAME: -guard:ehcont
-
 // RUN: %clang_cl /guard:foo -### -- %s 2>&1 | FileCheck 
-check-prefix=CFGUARDINVALID %s
 // CFGUARDINVALID: invalid value 'foo' in '/guard:'
 



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

[clang] caf01f9 - Reland "[Driver] Support multi /guard: options"

2023-05-21 Thread Phoebe Wang via cfe-commits


Author: Phoebe Wang
Date: 2023-05-21T22:43:30+08:00
New Revision: caf01f9b1df732d4eda1577123be7ec5da964f8f

URL: 
https://github.com/llvm/llvm-project/commit/caf01f9b1df732d4eda1577123be7ec5da964f8f
DIFF: 
https://github.com/llvm/llvm-project/commit/caf01f9b1df732d4eda1577123be7ec5da964f8f.diff

LOG: Reland "[Driver] Support multi /guard: options"

Fixes unexpected warning.

Differential Revision: https://reviews.llvm.org/D150645

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/MSVC.cpp
clang/test/Driver/cl-options.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index a55fe1cedb2ef..809440b196fd8 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7876,7 +7876,7 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID 
InputType,
   if (Args.hasArg(options::OPT__SLASH_kernel))
 CmdArgs.push_back("-fms-kernel");
 
-  if (Arg *A = Args.getLastArg(options::OPT__SLASH_guard)) {
+  for (const Arg *A : Args.filtered(options::OPT__SLASH_guard)) {
 StringRef GuardArgs = A->getValue();
 // The only valid options are "cf", "cf,nochecks", "cf-", "ehcont" and
 // "ehcont-".
@@ -7895,6 +7895,7 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID 
InputType,
 } else {
   D.Diag(diag::err_drv_invalid_value) << A->getSpelling() << GuardArgs;
 }
+A->claim();
   }
 }
 

diff  --git a/clang/lib/Driver/ToolChains/MSVC.cpp 
b/clang/lib/Driver/ToolChains/MSVC.cpp
index b23d0aa2f1b83..d803ddad504c6 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -227,7 +227,7 @@ void visualstudio::Linker::ConstructJob(Compilation &C, 
const JobAction &JA,
   Args.AddAllArgValues(CmdArgs, options::OPT__SLASH_link);
 
   // Control Flow Guard checks
-  if (Arg *A = Args.getLastArg(options::OPT__SLASH_guard)) {
+  for (const Arg *A : Args.filtered(options::OPT__SLASH_guard)) {
 StringRef GuardArgs = A->getValue();
 if (GuardArgs.equals_insensitive("cf") ||
 GuardArgs.equals_insensitive("cf,nochecks")) {

diff  --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 12d7023bd61b1..781298bea7b94 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -647,6 +647,13 @@
 // RUN: %clang_cl /guard:ehcont -### -- %s 2>&1 | FileCheck 
-check-prefix=EHCONTGUARD %s
 // EHCONTGUARD: -ehcontguard
 
+// RUN: %clang_cl /guard:cf /guard:ehcont -Wall -Wno-msvc-not-found -### -- %s 
2>&1 | \
+// RUN:   FileCheck -check-prefix=BOTHGUARD %s --implicit-check-not=warning
+// BOTHGUARD: -cfguard
+// BOTHGUARD-SAME: -ehcontguard
+// BOTHGUARD: -guard:cf
+// BOTHGUARD-SAME: -guard:ehcont
+
 // RUN: %clang_cl /guard:foo -### -- %s 2>&1 | FileCheck 
-check-prefix=CFGUARDINVALID %s
 // CFGUARDINVALID: invalid value 'foo' in '/guard:'
 



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

[clang] 2f0a169 - Reland "[Driver] Support multi /guard: options"

2023-05-21 Thread Phoebe Wang via cfe-commits


Author: Phoebe Wang
Date: 2023-05-21T22:49:34+08:00
New Revision: 2f0a1699eab7c00a64312e7f87e0d85a2e9b9e6e

URL: 
https://github.com/llvm/llvm-project/commit/2f0a1699eab7c00a64312e7f87e0d85a2e9b9e6e
DIFF: 
https://github.com/llvm/llvm-project/commit/2f0a1699eab7c00a64312e7f87e0d85a2e9b9e6e.diff

LOG: Reland "[Driver] Support multi /guard: options"

Fixes unexpected warning.

Differential Revision: https://reviews.llvm.org/D150645

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/MSVC.cpp
clang/test/Driver/cl-options.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index a55fe1cedb2ef..809440b196fd8 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7876,7 +7876,7 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID 
InputType,
   if (Args.hasArg(options::OPT__SLASH_kernel))
 CmdArgs.push_back("-fms-kernel");
 
-  if (Arg *A = Args.getLastArg(options::OPT__SLASH_guard)) {
+  for (const Arg *A : Args.filtered(options::OPT__SLASH_guard)) {
 StringRef GuardArgs = A->getValue();
 // The only valid options are "cf", "cf,nochecks", "cf-", "ehcont" and
 // "ehcont-".
@@ -7895,6 +7895,7 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID 
InputType,
 } else {
   D.Diag(diag::err_drv_invalid_value) << A->getSpelling() << GuardArgs;
 }
+A->claim();
   }
 }
 

diff  --git a/clang/lib/Driver/ToolChains/MSVC.cpp 
b/clang/lib/Driver/ToolChains/MSVC.cpp
index b23d0aa2f1b83..d803ddad504c6 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -227,7 +227,7 @@ void visualstudio::Linker::ConstructJob(Compilation &C, 
const JobAction &JA,
   Args.AddAllArgValues(CmdArgs, options::OPT__SLASH_link);
 
   // Control Flow Guard checks
-  if (Arg *A = Args.getLastArg(options::OPT__SLASH_guard)) {
+  for (const Arg *A : Args.filtered(options::OPT__SLASH_guard)) {
 StringRef GuardArgs = A->getValue();
 if (GuardArgs.equals_insensitive("cf") ||
 GuardArgs.equals_insensitive("cf,nochecks")) {

diff  --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 12d7023bd61b1..f07a9c2d5c572 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -647,6 +647,13 @@
 // RUN: %clang_cl /guard:ehcont -### -- %s 2>&1 | FileCheck 
-check-prefix=EHCONTGUARD %s
 // EHCONTGUARD: -ehcontguard
 
+// RUN: %clang_cl /guard:cf /guard:ehcont -Wall -Wno-msvc-not-found -### -- %s 
2>&1 | \
+// RUN:   FileCheck -check-prefix=BOTHGUARD %s --implicit-check-not=warning:
+// BOTHGUARD: -cfguard
+// BOTHGUARD-SAME: -ehcontguard
+// BOTHGUARD: -guard:cf
+// BOTHGUARD-SAME: -guard:ehcont
+
 // RUN: %clang_cl /guard:foo -### -- %s 2>&1 | FileCheck 
-check-prefix=CFGUARDINVALID %s
 // CFGUARDINVALID: invalid value 'foo' in '/guard:'
 



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

[clang] c2cec8a - Revert "Reland "[Driver] Support multi /guard: options""

2023-05-21 Thread Phoebe Wang via cfe-commits


Author: Phoebe Wang
Date: 2023-05-21T22:48:35+08:00
New Revision: c2cec8a91cf948aac6ec7c7b4e74aab2236769bd

URL: 
https://github.com/llvm/llvm-project/commit/c2cec8a91cf948aac6ec7c7b4e74aab2236769bd
DIFF: 
https://github.com/llvm/llvm-project/commit/c2cec8a91cf948aac6ec7c7b4e74aab2236769bd.diff

LOG: Revert "Reland "[Driver] Support multi /guard: options""

This reverts commit a4f366dcd85c440a611bbc82f1d24c2d9a735251.

Found a problem during backport it to 16.x branch.
https://github.com/llvm/llvm-project-release-prs/actions/runs/5036930270/jobs/9033427592?pr=451

The reason is `-implicit-check-not='warning'` may conflict with option
name, e.g., `-treat-scalable-fixed-error-as-warning`

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/MSVC.cpp
clang/test/Driver/cl-options.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 809440b196fd8..a55fe1cedb2ef 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7876,7 +7876,7 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID 
InputType,
   if (Args.hasArg(options::OPT__SLASH_kernel))
 CmdArgs.push_back("-fms-kernel");
 
-  for (const Arg *A : Args.filtered(options::OPT__SLASH_guard)) {
+  if (Arg *A = Args.getLastArg(options::OPT__SLASH_guard)) {
 StringRef GuardArgs = A->getValue();
 // The only valid options are "cf", "cf,nochecks", "cf-", "ehcont" and
 // "ehcont-".
@@ -7895,7 +7895,6 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID 
InputType,
 } else {
   D.Diag(diag::err_drv_invalid_value) << A->getSpelling() << GuardArgs;
 }
-A->claim();
   }
 }
 

diff  --git a/clang/lib/Driver/ToolChains/MSVC.cpp 
b/clang/lib/Driver/ToolChains/MSVC.cpp
index d803ddad504c6..b23d0aa2f1b83 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -227,7 +227,7 @@ void visualstudio::Linker::ConstructJob(Compilation &C, 
const JobAction &JA,
   Args.AddAllArgValues(CmdArgs, options::OPT__SLASH_link);
 
   // Control Flow Guard checks
-  for (const Arg *A : Args.filtered(options::OPT__SLASH_guard)) {
+  if (Arg *A = Args.getLastArg(options::OPT__SLASH_guard)) {
 StringRef GuardArgs = A->getValue();
 if (GuardArgs.equals_insensitive("cf") ||
 GuardArgs.equals_insensitive("cf,nochecks")) {

diff  --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 781298bea7b94..12d7023bd61b1 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -647,13 +647,6 @@
 // RUN: %clang_cl /guard:ehcont -### -- %s 2>&1 | FileCheck 
-check-prefix=EHCONTGUARD %s
 // EHCONTGUARD: -ehcontguard
 
-// RUN: %clang_cl /guard:cf /guard:ehcont -Wall -Wno-msvc-not-found -### -- %s 
2>&1 | \
-// RUN:   FileCheck -check-prefix=BOTHGUARD %s --implicit-check-not=warning
-// BOTHGUARD: -cfguard
-// BOTHGUARD-SAME: -ehcontguard
-// BOTHGUARD: -guard:cf
-// BOTHGUARD-SAME: -guard:ehcont
-
 // RUN: %clang_cl /guard:foo -### -- %s 2>&1 | FileCheck 
-check-prefix=CFGUARDINVALID %s
 // CFGUARDINVALID: invalid value 'foo' in '/guard:'
 



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

[PATCH] D150645: [Driver] Support multi /guard: options

2023-05-21 Thread Phoebe Wang via Phabricator via cfe-commits

pengfei added a comment.

Sorry for the back and forth! I just want to add a `:` after `warning`, but I 
forgot to amend it in the first reland..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150645

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

[PATCH] D150528: [Clang] Fix the diagnoses when the argument to alignas is an incomplete type

2023-05-21 Thread Yurong via Phabricator via cfe-commits

yronglin updated this revision to Diff 524106.
yronglin added a comment.

Improve the AST fidelity of ``alignas`` and ``_Alignas`` attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150528

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/AttrImpl.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/Sema/aix-attr-aligned-vector-warn.c
  clang/test/Sema/aix-attr-aligned-vector-warn.cpp
  clang/test/Sema/sizeless-1.c
  clang/test/SemaCXX/attr-cxx0x.cpp
  clang/test/SemaCXX/builtin-align-cxx.cpp
  clang/test/SemaCXX/cxx11-attr-print.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -508,6 +508,16 @@
   OS << "assert(!is" << getLowerName() << "Expr);\n";
   OS << "return " << getLowerName() << "Type;\n";
   OS << "  }";
+
+  OS << "  std::optional getCached" << getUpperName()
+ << "Value() const {\n";
+  OS << "return " << getLowerName() << "Cache;\n";
+  OS << "  }";
+
+  OS << "  void setCached" << getUpperName()
+ << "Value(unsigned AlignVal) {\n";
+  OS << "" << getLowerName() << "Cache = AlignVal;\n";
+  OS << "  }";
 }
 
 void writeAccessorDefinitions(raw_ostream &OS) const override {
@@ -530,21 +540,6 @@
   OS << "  return " << getLowerName()
  << "Type->getType()->containsErrors();\n";
   OS << "}\n";
-
-  // FIXME: Do not do the calculation here
-  // FIXME: Handle types correctly
-  // A null pointer means maximum alignment
-  OS << "unsigned " << getAttrName() << "Attr::get" << getUpperName()
- << "(ASTContext &Ctx) const {\n";
-  OS << "  assert(!is" << getUpperName() << "Dependent());\n";
-  OS << "  if (is" << getLowerName() << "Expr)\n";
-  OS << "return " << getLowerName() << "Expr ? " << getLowerName()
- << "Expr->EvaluateKnownConstInt(Ctx).getZExtValue()"
- << " * Ctx.getCharWidth() : "
- << "Ctx.getTargetDefaultAlignForAttributeAligned();\n";
-  OS << "  else\n";
-  OS << "return 0; // FIXME\n";
-  OS << "}\n";
 }
 
 void writeASTVisitorTraversal(raw_ostream &OS) const override {
@@ -601,7 +596,8 @@
   OS << "union {\n";
   OS << "Expr *" << getLowerName() << "Expr;\n";
   OS << "TypeSourceInfo *" << getLowerName() << "Type;\n";
-  OS << "};";
+  OS << "};\n";
+  OS << "std::optional " << getLowerName() << "Cache;\n";
 }
 
 void writePCHReadArgs(raw_ostream &OS) const override {
@@ -628,14 +624,17 @@
 }
 
 std::string getIsOmitted() const override {
-  return "!is" + getLowerName().str() + "Expr || !" + getLowerName().str()
- + "Expr";
+  return "!((is" + getLowerName().str() + "Expr && " +
+ getLowerName().str() + "Expr) || (!is" + getLowerName().str() +
+ "Expr && " + getLowerName().str() + "Type))";
 }
 
 void writeValue(raw_ostream &OS) const override {
   OS << "\";\n";
-  OS << "" << getLowerName()
- << "Expr->printPretty(OS, nullptr, Policy);\n";
+  OS << "if (is" << getLowerName() << "Expr && " << getLowerName() << "Expr)";
+  OS << "  " << getLowerName() << "Expr->printPretty(OS, nullptr, Policy);\n";
+  OS << "if (!is" << getLowerName() << "Expr && " << getLowerName() << "Type)";
+  OS << "  " << getLowerName() << "Type->getType().print(OS, Policy);\n";
   OS << "OS << \"";
 }
 
Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -73,7 +73,7 @@
   svint8_t __attribute__((aligned(4))) aligned_int8_2; // expected-error {{'aligned' attribute cannot be applied to sizeless type 'svint8_t'}}
   svint8_t _Alignas(int) aligned_int8_3;   // expected-error {{'_Alignas' attribute cannot be applied to sizeless type 'svint8_t'}}
 
-  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of 'alignof' to sizeless type 'svint8_t'}}
+  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of '_Alignas' to sizeless type 'svint8_t'}}
 
   // Using pointers to sizeless data isn't wrong here, but because the
   // type is incomplete, it doesn't provide any alignment guarantees.
Index: clang/test/SemaCXX/cxx11-attr-print.cpp
===
--- clang/test/SemaCXX/cxx11-attr-print.cpp
+++ clang/te

[PATCH] D150528: [Clang] Fix the diagnoses when the argument to alignas is an incomplete type

2023-05-21 Thread Yurong via Phabricator via cfe-commits

yronglin updated this revision to Diff 524107.
yronglin added a comment.

Remove extra include.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150528

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/AttrImpl.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/Sema/aix-attr-aligned-vector-warn.c
  clang/test/Sema/aix-attr-aligned-vector-warn.cpp
  clang/test/Sema/sizeless-1.c
  clang/test/SemaCXX/attr-cxx0x.cpp
  clang/test/SemaCXX/builtin-align-cxx.cpp
  clang/test/SemaCXX/cxx11-attr-print.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -508,6 +508,16 @@
   OS << "assert(!is" << getLowerName() << "Expr);\n";
   OS << "return " << getLowerName() << "Type;\n";
   OS << "  }";
+
+  OS << "  std::optional getCached" << getUpperName()
+ << "Value() const {\n";
+  OS << "return " << getLowerName() << "Cache;\n";
+  OS << "  }";
+
+  OS << "  void setCached" << getUpperName()
+ << "Value(unsigned AlignVal) {\n";
+  OS << "" << getLowerName() << "Cache = AlignVal;\n";
+  OS << "  }";
 }
 
 void writeAccessorDefinitions(raw_ostream &OS) const override {
@@ -530,21 +540,6 @@
   OS << "  return " << getLowerName()
  << "Type->getType()->containsErrors();\n";
   OS << "}\n";
-
-  // FIXME: Do not do the calculation here
-  // FIXME: Handle types correctly
-  // A null pointer means maximum alignment
-  OS << "unsigned " << getAttrName() << "Attr::get" << getUpperName()
- << "(ASTContext &Ctx) const {\n";
-  OS << "  assert(!is" << getUpperName() << "Dependent());\n";
-  OS << "  if (is" << getLowerName() << "Expr)\n";
-  OS << "return " << getLowerName() << "Expr ? " << getLowerName()
- << "Expr->EvaluateKnownConstInt(Ctx).getZExtValue()"
- << " * Ctx.getCharWidth() : "
- << "Ctx.getTargetDefaultAlignForAttributeAligned();\n";
-  OS << "  else\n";
-  OS << "return 0; // FIXME\n";
-  OS << "}\n";
 }
 
 void writeASTVisitorTraversal(raw_ostream &OS) const override {
@@ -601,7 +596,8 @@
   OS << "union {\n";
   OS << "Expr *" << getLowerName() << "Expr;\n";
   OS << "TypeSourceInfo *" << getLowerName() << "Type;\n";
-  OS << "};";
+  OS << "};\n";
+  OS << "std::optional " << getLowerName() << "Cache;\n";
 }
 
 void writePCHReadArgs(raw_ostream &OS) const override {
@@ -628,14 +624,17 @@
 }
 
 std::string getIsOmitted() const override {
-  return "!is" + getLowerName().str() + "Expr || !" + getLowerName().str()
- + "Expr";
+  return "!((is" + getLowerName().str() + "Expr && " +
+ getLowerName().str() + "Expr) || (!is" + getLowerName().str() +
+ "Expr && " + getLowerName().str() + "Type))";
 }
 
 void writeValue(raw_ostream &OS) const override {
   OS << "\";\n";
-  OS << "" << getLowerName()
- << "Expr->printPretty(OS, nullptr, Policy);\n";
+  OS << "if (is" << getLowerName() << "Expr && " << getLowerName() << "Expr)";
+  OS << "  " << getLowerName() << "Expr->printPretty(OS, nullptr, Policy);\n";
+  OS << "if (!is" << getLowerName() << "Expr && " << getLowerName() << "Type)";
+  OS << "  " << getLowerName() << "Type->getType().print(OS, Policy);\n";
   OS << "OS << \"";
 }
 
Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -73,7 +73,7 @@
   svint8_t __attribute__((aligned(4))) aligned_int8_2; // expected-error {{'aligned' attribute cannot be applied to sizeless type 'svint8_t'}}
   svint8_t _Alignas(int) aligned_int8_3;   // expected-error {{'_Alignas' attribute cannot be applied to sizeless type 'svint8_t'}}
 
-  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of 'alignof' to sizeless type 'svint8_t'}}
+  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of '_Alignas' to sizeless type 'svint8_t'}}
 
   // Using pointers to sizeless data isn't wrong here, but because the
   // type is incomplete, it doesn't provide any alignment guarantees.
Index: clang/test/SemaCXX/cxx11-attr-print.cpp
===
--- clang/test/SemaCXX/cxx11-attr-print.cpp
+++ clang/test/SemaCXX/cxx11-attr-print.cpp
@@ -28,7 +28,7

[PATCH] D150528: [Clang] Fix the diagnoses when the argument to alignas is an incomplete type

2023-05-21 Thread Yurong via Phabricator via cfe-commits

yronglin updated this revision to Diff 524108.
yronglin added a comment.

Test asm dump.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150528

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/AttrImpl.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Sema/aix-attr-aligned-vector-warn.c
  clang/test/Sema/aix-attr-aligned-vector-warn.cpp
  clang/test/Sema/sizeless-1.c
  clang/test/SemaCXX/attr-cxx0x.cpp
  clang/test/SemaCXX/builtin-align-cxx.cpp
  clang/test/SemaCXX/cxx11-attr-print.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -508,6 +508,16 @@
   OS << "assert(!is" << getLowerName() << "Expr);\n";
   OS << "return " << getLowerName() << "Type;\n";
   OS << "  }";
+
+  OS << "  std::optional getCached" << getUpperName()
+ << "Value() const {\n";
+  OS << "return " << getLowerName() << "Cache;\n";
+  OS << "  }";
+
+  OS << "  void setCached" << getUpperName()
+ << "Value(unsigned AlignVal) {\n";
+  OS << "" << getLowerName() << "Cache = AlignVal;\n";
+  OS << "  }";
 }
 
 void writeAccessorDefinitions(raw_ostream &OS) const override {
@@ -530,21 +540,6 @@
   OS << "  return " << getLowerName()
  << "Type->getType()->containsErrors();\n";
   OS << "}\n";
-
-  // FIXME: Do not do the calculation here
-  // FIXME: Handle types correctly
-  // A null pointer means maximum alignment
-  OS << "unsigned " << getAttrName() << "Attr::get" << getUpperName()
- << "(ASTContext &Ctx) const {\n";
-  OS << "  assert(!is" << getUpperName() << "Dependent());\n";
-  OS << "  if (is" << getLowerName() << "Expr)\n";
-  OS << "return " << getLowerName() << "Expr ? " << getLowerName()
- << "Expr->EvaluateKnownConstInt(Ctx).getZExtValue()"
- << " * Ctx.getCharWidth() : "
- << "Ctx.getTargetDefaultAlignForAttributeAligned();\n";
-  OS << "  else\n";
-  OS << "return 0; // FIXME\n";
-  OS << "}\n";
 }
 
 void writeASTVisitorTraversal(raw_ostream &OS) const override {
@@ -601,7 +596,8 @@
   OS << "union {\n";
   OS << "Expr *" << getLowerName() << "Expr;\n";
   OS << "TypeSourceInfo *" << getLowerName() << "Type;\n";
-  OS << "};";
+  OS << "};\n";
+  OS << "std::optional " << getLowerName() << "Cache;\n";
 }
 
 void writePCHReadArgs(raw_ostream &OS) const override {
@@ -628,14 +624,17 @@
 }
 
 std::string getIsOmitted() const override {
-  return "!is" + getLowerName().str() + "Expr || !" + getLowerName().str()
- + "Expr";
+  return "!((is" + getLowerName().str() + "Expr && " +
+ getLowerName().str() + "Expr) || (!is" + getLowerName().str() +
+ "Expr && " + getLowerName().str() + "Type))";
 }
 
 void writeValue(raw_ostream &OS) const override {
   OS << "\";\n";
-  OS << "" << getLowerName()
- << "Expr->printPretty(OS, nullptr, Policy);\n";
+  OS << "if (is" << getLowerName() << "Expr && " << getLowerName() << "Expr)";
+  OS << "  " << getLowerName() << "Expr->printPretty(OS, nullptr, Policy);\n";
+  OS << "if (!is" << getLowerName() << "Expr && " << getLowerName() << "Type)";
+  OS << "  " << getLowerName() << "Type->getType().print(OS, Policy);\n";
   OS << "OS << \"";
 }
 
Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -73,7 +73,7 @@
   svint8_t __attribute__((aligned(4))) aligned_int8_2; // expected-error {{'aligned' attribute cannot be applied to sizeless type 'svint8_t'}}
   svint8_t _Alignas(int) aligned_int8_3;   // expected-error {{'_Alignas' attribute cannot be applied to sizeless type 'svint8_t'}}
 
-  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of 'alignof' to sizeless type 'svint8_t'}}
+  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of '_Alignas' to sizeless type 'svint8_t'}}
 
   // Using pointers to sizeless data isn't wrong here, but because the
   // type is incomplete, it doesn't provide any alignment guarantees.
Index: clang/test/SemaCXX/cxx11-attr-print.cpp
===
--- clang/test/SemaCXX/cxx11-attr-print.cpp
+++ clang/test/SemaCXX/cxx11-a

[PATCH] D141892: Implement modernize-use-constraints

2023-05-21 Thread Chris Cotter via Phabricator via cfe-commits

ccotter added a comment.

bump - @njames93 let me know if you have any further feedback. thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141892

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

[clang] 4aa1cad - [Driver] Disable -fsanitize=function for ppc64be after D148573

2023-05-21 Thread Fangrui Song via cfe-commits


Author: Fangrui Song
Date: 2023-05-21T09:14:40-07:00
New Revision: 4aa1cadf3c67c61b394051630896f06f35b562ca

URL: 
https://github.com/llvm/llvm-project/commit/4aa1cadf3c67c61b394051630896f06f35b562ca
DIFF: 
https://github.com/llvm/llvm-project/commit/4aa1cadf3c67c61b394051630896f06f35b562ca.diff

LOG: [Driver] Disable -fsanitize=function for ppc64be after D148573

ELFObjectWriter.cpp may report
```
error: Cannot represent a difference across sections
```
on some ppc64be configurations, likely related to some interaction
between the obsoleted ELFv1 and MC.
Unfortunately I cannot reproduce this locally with --target=powerpc64-linux-gnu.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Linux.cpp
compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Linux.cpp 
b/clang/lib/Driver/ToolChains/Linux.cpp
index d416c287f8a2d..853ff99d9fe59 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -806,6 +806,9 @@ SanitizerMask Linux::getSupportedSanitizers() const {
   if (IsX86_64 || IsAArch64) {
 Res |= SanitizerKind::KernelHWAddress;
   }
+  // Work around "Cannot represent a 
diff erence across sections".
+  if (getTriple().getArch() == llvm::Triple::ppc64)
+Res &= ~SanitizerKind::Function;
   return Res;
 }
 

diff  --git a/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp 
b/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
index 2f7db994364ca..645ec3c8c6d76 100644
--- a/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
+++ b/compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
@@ -1,3 +1,6 @@
+// Work around "Cannot represent a 
diff erence across sections"
+// UNSUPPORTED: target=powerpc64-{{.*}}
+
 // RUN: %clangxx -DDETERMINE_UNIQUE %s -o %t-unique
 // RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -DSHARED_LIB -fPIC 
-shared -o %t-so.so
 // RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -o %t %t-so.so



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

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

2023-05-21 Thread Elliot Goodrich via Phabricator via cfe-commits

IncludeGuardian updated this revision to Diff 524109.

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

https://reviews.llvm.org/D150997

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
  llvm/include/llvm/ADT/DenseMapInfo.h
  llvm/include/llvm/ADT/DenseMapInfoVariant.h
  llvm/include/llvm/CodeGen/CallingConvLower.h
  llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
  llvm/include/llvm/Object/DXContainer.h
  llvm/include/llvm/Transforms/Scalar/SROA.h
  mlir/include/mlir/IR/AsmState.h

Index: mlir/include/mlir/IR/AsmState.h
===
--- mlir/include/mlir/IR/AsmState.h
+++ mlir/include/mlir/IR/AsmState.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/StringMap.h"
 
 #include 
+#include 
 
 namespace mlir {
 class AsmResourcePrinter;
Index: llvm/include/llvm/Transforms/Scalar/SROA.h
===
--- llvm/include/llvm/Transforms/Scalar/SROA.h
+++ llvm/include/llvm/Transforms/Scalar/SROA.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/ValueHandle.h"
+#include 
 #include 
 
 namespace llvm {
Index: llvm/include/llvm/Object/DXContainer.h
===
--- llvm/include/llvm/Object/DXContainer.h
+++ llvm/include/llvm/Object/DXContainer.h
@@ -21,6 +21,7 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/TargetParser/Triple.h"
+#include 
 
 namespace llvm {
 namespace object {
Index: llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
===
--- llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
+++ llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
@@ -22,6 +22,7 @@
 #include "llvm/ExecutionEngine/Orc/ThreadSafeModule.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ThreadPool.h"
+#include 
 
 namespace llvm {
 namespace orc {
Index: llvm/include/llvm/CodeGen/CallingConvLower.h
===
--- llvm/include/llvm/CodeGen/CallingConvLower.h
+++ llvm/include/llvm/CodeGen/CallingConvLower.h
@@ -19,6 +19,7 @@
 #include "llvm/CodeGen/TargetCallingConv.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/Support/Alignment.h"
+#include 
 
 namespace llvm {
 
Index: llvm/include/llvm/ADT/DenseMapInfoVariant.h
===
--- /dev/null
+++ llvm/include/llvm/ADT/DenseMapInfoVariant.h
@@ -0,0 +1,56 @@
+//===- DenseMapInfoVariant.h - Type traits for DenseMap *- 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
+//
+//===--===//
+///
+/// \file
+/// This file defines DenseMapInfo traits for DenseMap>.
+///
+//===--===//
+
+#ifndef LLVM_ADT_DENSEMAPINFOVARIANT_H
+#define LLVM_ADT_DENSEMAPINFOVARIANT_H
+
+#include "llvm/ADT/DenseMapInfo.h"
+#include 
+#include 
+
+namespace llvm {
+
+// Provide DenseMapInfo for variants whose all alternatives have DenseMapInfo.
+template  struct DenseMapInfo> {
+  using Variant = std::variant;
+  using FirstT = std::variant_alternative_t<0, Variant>;
+
+  static inline Variant getEmptyKey() {
+return Variant(std::in_place_index<0>, DenseMapInfo::getEmptyKey());
+  }
+
+  static inline Variant getTombstoneKey() {
+return Variant(std::in_place_index<0>,
+   DenseMapInfo::getTombstoneKey());
+  }
+
+  static unsigned getHashValue(const Variant &Val) {
+return std::visit(
+[&Val](auto &&Alternative) {
+  using T = std::decay_t;
+  // Include index in hash to make sure same value as different
+  // alternatives don't collide.
+  return DenseMapInfo>::getHashValuePiecewise(
+  Val.index(), Alternative);
+},
+Val);
+  }
+
+  static bool isEqual(const Variant &LHS, const Variant &RHS) {
+return LHS == RHS;
+  }
+};
+
+} // end namespace llvm
+
+#endif // LLVM_ADT_DENSEMAPINFOVARIANT_H
Index: llvm/include/llvm/ADT/DenseMapInfo.h
===
--- llvm/include/llvm/ADT/DenseMapInfo.h
+++ llvm/include/llvm/ADT/DenseMapInfo.h
@@ -20,7 +20,6 @@
 #include 
 #include 
 #include 
-#include 
 
 namespace llvm {
 
@@ -234,6 +233,14 @@
 SecondInfo::getHashValue(PairVal.second));
   }
 
+  // Expose an additional function intended to be used by other
+  // specializations of DenseMapInfo without needing to know how
+  // to combine hash values manually
+  static unsigned getHashValuePiecewise(const T& First, const U& Second) {
+return detail::combineHashValu

[PATCH] D148827: -fsanitize=function: support C

2023-05-21 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay updated this revision to Diff 524110.
MaskRay added a comment.

rebase.

unsupport powerpc64-* as ppc64be ELFv1 may have an existing
`error: Cannot represent a difference across sections` issue 
unrelated to the -fsanitize=function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148827

Files:
  clang/docs/ControlFlowIntegrity.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/ubsan-function.c
  compiler-rt/test/ubsan/TestCases/TypeCheck/Function/c.c

Index: compiler-rt/test/ubsan/TestCases/TypeCheck/Function/c.c
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/Function/c.c
@@ -0,0 +1,17 @@
+// Work around "Cannot represent a difference across sections"
+// UNSUPPORTED: target=powerpc64-{{.*}}
+
+// RUN: %clang -g -fsanitize=function %s -o %t
+// RUN: %run %t 2>&1 | FileCheck %s --check-prefix=CHECK --implicit-check-not='runtime error:'
+
+void f(void (*fp)(int (*)[])) { fp(0); }
+
+void callee0(int (*a)[]) {}
+void callee1(int (*a)[1]) {}
+
+int main() {
+  int a[1];
+  f(callee0);
+  // CHECK: runtime error: call to function callee1 through pointer to incorrect function type 'void (*)(int (*)[])'
+  f(callee1); // compatible type in C, but flagged
+}
Index: clang/test/CodeGen/ubsan-function.c
===
--- /dev/null
+++ clang/test/CodeGen/ubsan-function.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -emit-llvm -triple x86_64 -std=c17 -fsanitize=function %s -o - | FileCheck %s
+
+// CHECK-LABEL: define{{.*}} @call_no_prototype(
+// CHECK-NOT: __ubsan_handle_function_type_mismatch
+void call_no_prototype(void (*f)()) { f(); }
+
+// CHECK-LABEL: define{{.*}} @call_prototype(
+// CHECK: __ubsan_handle_function_type_mismatch
+void call_prototype(void (*f)(void)) { f(); }
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -572,10 +572,11 @@
 CodeGenFunction::getUBSanFunctionTypeHash(QualType Ty) const {
   // Remove any (C++17) exception specifications, to allow calling e.g. a
   // noexcept function through a non-noexcept pointer.
-  auto ProtoTy = getContext().getFunctionTypeWithExceptionSpec(Ty, EST_None);
+  if (!isa(Ty))
+Ty = getContext().getFunctionTypeWithExceptionSpec(Ty, EST_None);
   std::string Mangled;
   llvm::raw_string_ostream Out(Mangled);
-  CGM.getCXXABI().getMangleContext().mangleTypeName(ProtoTy, Out, false);
+  CGM.getCXXABI().getMangleContext().mangleTypeName(Ty, Out, false);
   return llvm::ConstantInt::get(CGM.Int32Ty,
 static_cast(llvm::xxHash64(Mangled)));
 }
@@ -945,7 +946,7 @@
 
   // If we are checking function types, emit a function type signature as
   // prologue data.
-  if (FD && getLangOpts().CPlusPlus && SanOpts.has(SanitizerKind::Function)) {
+  if (FD && SanOpts.has(SanitizerKind::Function)) {
 if (llvm::Constant *PrologueSig = getPrologueSignature(CGM, FD)) {
   llvm::LLVMContext &Ctx = Fn->getContext();
   llvm::MDBuilder MDB(Ctx);
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -5349,8 +5349,9 @@
 
   CGCallee Callee = OrigCallee;
 
-  if (getLangOpts().CPlusPlus && SanOpts.has(SanitizerKind::Function) &&
-  (!TargetDecl || !isa(TargetDecl))) {
+  if (SanOpts.has(SanitizerKind::Function) &&
+  (!TargetDecl || !isa(TargetDecl)) &&
+  !isa(PointeeType)) {
 if (llvm::Constant *PrefixSig =
 CGM.getTargetCodeGenInfo().getUBSanFunctionSignature(CGM)) {
   SanitizerScope SanScope(this);
Index: clang/docs/UndefinedBehaviorSanitizer.rst
===
--- clang/docs/UndefinedBehaviorSanitizer.rst
+++ clang/docs/UndefinedBehaviorSanitizer.rst
@@ -100,7 +100,7 @@
  by Clang (and by ISO/IEC/IEEE 60559 / IEEE 754) as producing either an
  infinity or NaN value, so is not included in ``-fsanitize=undefined``.
   -  ``-fsanitize=function``: Indirect call of a function through a
- function pointer of the wrong type (C++ only).
+ function pointer of the wrong type.
   -  ``-fsanitize=implicit-unsigned-integer-truncation``,
  ``-fsanitize=implicit-signed-integer-truncation``: Implicit conversion from
  integer of larger bit width to smaller bit width, if that results in data
Index: clang/docs/ControlFlowIntegrity.rst
===
--- clang/docs/ControlFlowIntegrity.rst
+++ clang/docs/ControlFlowIntegrity.rst
@@ -314,10 +314,8 @@
 is a security hardening mechanism designed to be deployed in r

[PATCH] D151048: [clang][ExtractAPI] Modify declaration fragment methods to add a new fragment at an arbitrary offset.

2023-05-21 Thread R4444 via Phabricator via cfe-commits

Ruturaj4 created this revision.
Ruturaj4 added a reviewer: dang.
Herald added a reviewer: ributzka.
Herald added a project: All.
Ruturaj4 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The current implementation doesn't support merging declaration fragments at 
arbitrary offsets. This patch adds that support
by modifying declaration fragment methods.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151048

Files:
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/include/clang/ExtractAPI/ExtractAPIVisitor.h


Index: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
===
--- clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
+++ clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
@@ -110,15 +110,16 @@
 static void modifyRecords(const T &Records, const StringRef &Name) {
   for (const auto &Record : Records) {
 if (Name == Record.second.get()->Name) {
-  Record.second.get()->Declaration.removeLast();
   Record.second.get()
   ->Declaration
-  .appendFront(" ", DeclarationFragments::FragmentKind::Text)
-  .appendFront("typedef", DeclarationFragments::FragmentKind::Keyword,
-   "", nullptr)
-  .append(" { ... } ", DeclarationFragments::FragmentKind::Text)
-  .append(Name, DeclarationFragments::FragmentKind::Identifier)
-  .append(";", DeclarationFragments::FragmentKind::Text);
+  .insertAtIndex(0, "typedef",
+ DeclarationFragments::FragmentKind::Keyword, "",
+ nullptr)
+  .insertAtIndex(1, " ", DeclarationFragments::FragmentKind::Text)
+  .insertAtIndex(-1, " { ... } ",
+ DeclarationFragments::FragmentKind::Text)
+  .insertAtIndex(-1, Name,
+ DeclarationFragments::FragmentKind::Identifier);
   break;
 }
   }
Index: clang/include/clang/ExtractAPI/DeclarationFragments.h
===
--- clang/include/clang/ExtractAPI/DeclarationFragments.h
+++ clang/include/clang/ExtractAPI/DeclarationFragments.h
@@ -99,25 +99,37 @@
 
   const std::vector &getFragments() const { return Fragments; }
 
-  // Add a new Fragment to the beginning of the Fragments.
-  DeclarationFragments &appendFront(StringRef Spelling, FragmentKind Kind,
-StringRef PreciseIdentifier = "",
-const Decl *Declaration = nullptr) {
-Fragments.emplace(Fragments.begin(), Spelling, Kind, PreciseIdentifier,
-  Declaration);
+  size_t calculateOffset(intmax_t Index) const {
+if (Index >= 0) {
+  size_t offset = static_cast(Index);
+  if (offset > Fragments.size()) {
+offset = Fragments.size();
+  }
+  return offset;
+}
+return Fragments.size() + static_cast(Index);
+  }
+
+  // Add a new Fragment at an arbitrary offset.
+  DeclarationFragments &insertAtIndex(intmax_t Index, StringRef Spelling,
+  FragmentKind Kind,
+  StringRef PreciseIdentifier = "",
+  const Decl *Declaration = nullptr) {
+Fragments.insert(
+Fragments.begin() + calculateOffset(Index),
+std::move(Fragment(Spelling, Kind, PreciseIdentifier, Declaration)));
 return *this;
   }
 
-  DeclarationFragments &appendFront(DeclarationFragments &&Other) {
-Fragments.insert(Fragments.begin(),
+  DeclarationFragments &insertAtIndex(intmax_t Index,
+  DeclarationFragments &&Other) {
+Fragments.insert(Fragments.begin() + calculateOffset(Index),
  std::make_move_iterator(Other.Fragments.begin()),
  std::make_move_iterator(Other.Fragments.end()));
 Other.Fragments.clear();
 return *this;
   }
 
-  void removeLast() { Fragments.pop_back(); }
-
   /// Append a new Fragment to the end of the Fragments.
   ///
   /// \returns a reference to the DeclarationFragments object itself after


Index: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
===
--- clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
+++ clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
@@ -110,15 +110,16 @@
 static void modifyRecords(const T &Records, const StringRef &Name) {
   for (const auto &Record : Records) {
 if (Name == Record.second.get()->Name) {
-  Record.second.get()->Declaration.removeLast();
   Record.second.get()
   ->Declaration
-  .appendFront(" ", DeclarationFragments::FragmentKind::Text)
-  .appendFront("typedef", DeclarationFragments::FragmentKind::Keyword,
-   "", nullptr)
-  .append(" { ... } ", DeclarationFragments::FragmentKind:

[PATCH] D150913: [Clang][Bfloat16] Upgrade __bf16 to arithmetic type, change mangling, and extend excess precision support.

2023-05-21 Thread Zahira Ammarguellat via Phabricator via cfe-commits

zahiraam added inline comments.



Comment at: clang/lib/Basic/Targets/X86.cpp:381
 
 HasBFloat16 = SSELevel >= SSE2;
 

codemzs wrote:
> rjmccall wrote:
> > pengfei wrote:
> > > I'm not sure if I understand the meaning of `HasFullBFloat16`. If it is 
> > > used for target that supports arithmetic `__bf16`, we should not use 
> > > `+fullbf16` but always enable it for SSE2, i.e., `HasFullBFloat16 = 
> > > SSELevel >= SSE2`. Because X86 GCC already supports arithmetic for 
> > > `__bf16`.
> > > 
> > > If this is used in the way like `HasLegalHalfType`, we should enable it 
> > > once we have a full BF16 ISA on X86. `fullbf16` doesn't make much sense 
> > > to me.
> > At the moment, we haven't done the work to emulate BFloat16 arithmetic in 
> > any of the three ways we can do that: Clang doesn't promote it in IRGen, 
> > LLVM doesn't promote it in legalization, and we don't have compiler-rt 
> > functions for it.  If we emit these instructions, they'll just sail through 
> > LLVM and fail in the backend.  So in the short term, we have to restrict 
> > this to targets that directly support BFloat16 arithmetic in hardware, 
> > which doesn't include x86.
> > 
> > Once we have that emulation support, I agree that the x86 targets should 
> > enable this whenever they would enable `__bf16`.
> @rjmccall, I concur and just wanted to confirm this change indeed intends to 
> provide `BFloat16` emulation support, utilizing excess precision for 
> promotion to `float`. The `HasFullBFloat16` switch is designed to determine 
> excess precision support automatically when the hardware does not natively 
> support `bfloat16` arithmetic.
> At the moment, we haven't done the work to emulate BFloat16 arithmetic in any 
> of the three ways we can do that: Clang doesn't promote it in IRGen, LLVM 
> doesn't promote it in legalization, and we don't have compiler-rt functions 
> for it.  If we emit these instructions, they'll just sail through LLVM and 
> fail in the backend.  So in the short term, we have to restrict this to 
> targets that directly support BFloat16 arithmetic in hardware, which doesn't 
> include x86.
> 
> Once we have that emulation support, I agree that the x86 targets should 
> enable this whenever they would enable `__bf16`.

Would be nice to add a comment to clarify it. 



Comment at: clang/test/CodeGen/X86/bfloat16.cpp:2-3
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --version 2
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -target-feature +fullbf16 
-S -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | 
FileCheck -check-prefix=CHECK-NBF16 %s
+

pengfei wrote:
> The backend has already support lowering of `bfloat`, I don't think it's 
> necessary to do extra work in FE unless for excess-precision.
> The backend has already support lowering of `bfloat`, I don't think it's 
> necessary to do extra work in FE unless for excess-precision.

+1.



Comment at: clang/test/CodeGen/X86/fexcess-precision-bfloat16.c:361
+}
\ No newline at end of file


Fix this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150913

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

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

2023-05-21 Thread Elliot Goodrich via Phabricator via cfe-commits

IncludeGuardian updated this revision to Diff 524125.

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

https://reviews.llvm.org/D150997

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
  flang/include/flang/Optimizer/HLFIR/HLFIROps.h
  llvm/include/llvm/ADT/DenseMapInfo.h
  llvm/include/llvm/ADT/DenseMapInfoVariant.h
  llvm/include/llvm/CodeGen/CallingConvLower.h
  llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
  llvm/include/llvm/Object/DXContainer.h
  llvm/include/llvm/Transforms/Scalar/SROA.h
  mlir/include/mlir/IR/AsmState.h

Index: mlir/include/mlir/IR/AsmState.h
===
--- mlir/include/mlir/IR/AsmState.h
+++ mlir/include/mlir/IR/AsmState.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/StringMap.h"
 
 #include 
+#include 
 
 namespace mlir {
 class AsmResourcePrinter;
Index: llvm/include/llvm/Transforms/Scalar/SROA.h
===
--- llvm/include/llvm/Transforms/Scalar/SROA.h
+++ llvm/include/llvm/Transforms/Scalar/SROA.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/ValueHandle.h"
+#include 
 #include 
 
 namespace llvm {
Index: llvm/include/llvm/Object/DXContainer.h
===
--- llvm/include/llvm/Object/DXContainer.h
+++ llvm/include/llvm/Object/DXContainer.h
@@ -21,6 +21,7 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/TargetParser/Triple.h"
+#include 
 
 namespace llvm {
 namespace object {
Index: llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
===
--- llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
+++ llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
@@ -22,6 +22,7 @@
 #include "llvm/ExecutionEngine/Orc/ThreadSafeModule.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ThreadPool.h"
+#include 
 
 namespace llvm {
 namespace orc {
Index: llvm/include/llvm/CodeGen/CallingConvLower.h
===
--- llvm/include/llvm/CodeGen/CallingConvLower.h
+++ llvm/include/llvm/CodeGen/CallingConvLower.h
@@ -19,6 +19,7 @@
 #include "llvm/CodeGen/TargetCallingConv.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/Support/Alignment.h"
+#include 
 
 namespace llvm {
 
Index: llvm/include/llvm/ADT/DenseMapInfoVariant.h
===
--- /dev/null
+++ llvm/include/llvm/ADT/DenseMapInfoVariant.h
@@ -0,0 +1,56 @@
+//===- DenseMapInfoVariant.h - Type traits for DenseMap *- 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
+//
+//===--===//
+///
+/// \file
+/// This file defines DenseMapInfo traits for DenseMap>.
+///
+//===--===//
+
+#ifndef LLVM_ADT_DENSEMAPINFOVARIANT_H
+#define LLVM_ADT_DENSEMAPINFOVARIANT_H
+
+#include "llvm/ADT/DenseMapInfo.h"
+#include 
+#include 
+
+namespace llvm {
+
+// Provide DenseMapInfo for variants whose all alternatives have DenseMapInfo.
+template  struct DenseMapInfo> {
+  using Variant = std::variant;
+  using FirstT = std::variant_alternative_t<0, Variant>;
+
+  static inline Variant getEmptyKey() {
+return Variant(std::in_place_index<0>, DenseMapInfo::getEmptyKey());
+  }
+
+  static inline Variant getTombstoneKey() {
+return Variant(std::in_place_index<0>,
+   DenseMapInfo::getTombstoneKey());
+  }
+
+  static unsigned getHashValue(const Variant &Val) {
+return std::visit(
+[&Val](auto &&Alternative) {
+  using T = std::decay_t;
+  // Include index in hash to make sure same value as different
+  // alternatives don't collide.
+  return DenseMapInfo>::getHashValuePiecewise(
+  Val.index(), Alternative);
+},
+Val);
+  }
+
+  static bool isEqual(const Variant &LHS, const Variant &RHS) {
+return LHS == RHS;
+  }
+};
+
+} // end namespace llvm
+
+#endif // LLVM_ADT_DENSEMAPINFOVARIANT_H
Index: llvm/include/llvm/ADT/DenseMapInfo.h
===
--- llvm/include/llvm/ADT/DenseMapInfo.h
+++ llvm/include/llvm/ADT/DenseMapInfo.h
@@ -20,7 +20,6 @@
 #include 
 #include 
 #include 
-#include 
 
 namespace llvm {
 
@@ -234,6 +233,14 @@
 SecondInfo::getHashValue(PairVal.second));
   }
 
+  // Expose an additional function intended to be used by other
+  // specializations of DenseMapInfo without needing to know how
+  // to combine hash values manually
+  static unsigned getHashValuePiecewise(const T& First, const

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

2023-05-21 Thread Joseph Huber via Phabricator via cfe-commits

jhuber6 added a comment.
Herald added a subscriber: arichardson.
Herald added a project: All.

Ran into this change trying to decay a qualifier pointer to a generic address 
space, e.g. https://godbolt.org/z/3dEd4TxjW. I understand that `addrspace_cast` 
was added to replace this functionality, but this isn't a C++ feature so as it 
stands there's no way to perform this operation in C++ and we need to rely on 
C-style casts to perform this basic functionality. I see it was brought up 
earlier, but should C++ really be limited by OpenCL here? I'm not aware of any 
specific mention of these, `reinterpret_cast` only explicitly disallows 
`volatile` and `const` qualifiers as far as I'm aware. The remaining case can 
be thought of as a compiler extension as `[[clang::address_space(n)]]` has no 
meaning otherwise.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58346

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

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

2023-05-21 Thread Elliot Goodrich via Phabricator via cfe-commits

IncludeGuardian requested review of this revision.
IncludeGuardian added inline comments.



Comment at: flang/include/flang/Optimizer/HLFIR/HLFIROps.h:22
 #include "mlir/Interfaces/SideEffectInterfaces.h"
+#include 
 

I wasn't sure if it is possible to include `` within `HLFIROps.td` so 
it has been included here instead.


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

https://reviews.llvm.org/D150997

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

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

2023-05-21 Thread Sergei Barannikov via Phabricator via cfe-commits

barannikov88 added inline comments.



Comment at: flang/include/flang/Optimizer/HLFIR/HLFIROps.h:22
 #include "mlir/Interfaces/SideEffectInterfaces.h"
+#include 
 

IncludeGuardian wrote:
> I wasn't sure if it is possible to include `` within `HLFIROps.td` so 
> it has been included here instead.
That's fine, we do that everywhere :)



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

https://reviews.llvm.org/D150997

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

[PATCH] D150126: [clang-tidy][WIP] Set traversal scope to prevent analysis in headers

2023-05-21 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL added a comment.

I love an idea, but i'm afraid that it may impact checks that build some 
internal database, like  misc-confusable-identifiers or misc-unused-using-decls.
Probably some of those checks would need to be rewrite to use some own visitor. 
And probably we should have access to that Cache (function that check if 
location should be ignored) from a check code, in such case in some cases it 
could be used instead of re-inventing a wheal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150126

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

[PATCH] D151051: [clang-tidy] Optimize misc-confusable-identifiers

2023-05-21 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL created this revision.
PiotrZSL added reviewers: njames93, carlosgalvezp.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Main performance issue in this check were caused by many
calls to getPrimaryContext and constant walk up to declaration
contexts using getParent. Also there were issue with forallBases
that is slow.

Profiled with perf and tested on open-source project Cataclysm-DDA.
Before changes check took 27320 seconds, after changes 3682 seconds.
That's 86.5% reduction. More optimizations are still possible in this
check.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151051

Files:
  clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.h

Index: clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.h
===
--- clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.h
+++ clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.h
@@ -11,6 +11,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_CONFUSABLE_IDENTIFIER_CHECK_H
 
 #include "../ClangTidyCheck.h"
+#include 
 
 namespace clang::tidy::misc {
 
@@ -26,9 +27,23 @@
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
+  struct ContextInfo {
+const DeclContext *PrimaryContext;
+const DeclContext *NonTransparentContext;
+llvm::SmallVector PrimaryContexts;
+llvm::SmallVector Bases;
+  };
+
 private:
-  std::string skeleton(StringRef);
-  llvm::StringMap> Mapper;
+  struct Entry {
+const NamedDecl *Declaration;
+const ContextInfo *Info;
+  };
+
+  const ContextInfo *getContextInfo(const DeclContext *DC);
+
+  llvm::StringMap> Mapper;
+  std::unordered_map ContextInfos;
 };
 
 } // namespace clang::tidy::misc
Index: clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp
@@ -45,7 +45,7 @@
 // We're skipping 1. and 3. for the sake of simplicity, but this can lead to
 // false positive.
 
-std::string ConfusableIdentifierCheck::skeleton(StringRef Name) {
+static std::string skeleton(StringRef Name) {
   using namespace llvm;
   std::string SName = Name.str();
   std::string Skeleton;
@@ -89,75 +89,107 @@
   return Skeleton;
 }
 
-static bool mayShadowImpl(const NamedDecl *ND0, const NamedDecl *ND1) {
-  const DeclContext *DC0 = ND0->getDeclContext()->getPrimaryContext();
-  const DeclContext *DC1 = ND1->getDeclContext()->getPrimaryContext();
+static bool mayShadowImpl(const DeclContext *DC0, const DeclContext *DC1) {
+  return DC0 && DC0 == DC1;
+}
 
-  if (isa(ND0) || isa(ND0))
-return true;
+static bool mayShadowImpl(const NamedDecl *ND0, const NamedDecl *ND1) {
+  return isa(ND0) || isa(ND1);
+}
 
-  while (DC0->isTransparentContext())
-DC0 = DC0->getParent();
-  while (DC1->isTransparentContext())
-DC1 = DC1->getParent();
+static bool isMemberOf(const ConfusableIdentifierCheck::ContextInfo *DC0,
+   const ConfusableIdentifierCheck::ContextInfo *DC1) {
+  if (DC0->Bases.empty())
+return false;
+  return llvm::is_contained(DC1->Bases, DC0->PrimaryContext);
+}
 
-  if (DC0->Equals(DC1))
+static bool enclosesContext(const ConfusableIdentifierCheck::ContextInfo *DC0,
+const ConfusableIdentifierCheck::ContextInfo *DC1) {
+  if (DC0->PrimaryContext == DC1->PrimaryContext)
 return true;
 
-  return false;
+  return llvm::is_contained(DC0->PrimaryContexts, DC1->PrimaryContext) ||
+ llvm::is_contained(DC1->PrimaryContexts, DC0->PrimaryContext);
 }
 
-static bool isMemberOf(const NamedDecl *ND, const CXXRecordDecl *RD) {
-  const DeclContext *NDParent = ND->getDeclContext();
-  if (!NDParent || !isa(NDParent))
-return false;
-  if (NDParent == RD)
+static bool mayShadow(const NamedDecl *ND0,
+  const ConfusableIdentifierCheck::ContextInfo *DC0,
+  const NamedDecl *ND1,
+  const ConfusableIdentifierCheck::ContextInfo *DC1) {
+  if (!DC0->Bases.empty() && ND1->getAccess() != AS_private &&
+  isMemberOf(DC1, DC0))
 return true;
-  return !RD->forallBases(
-  [NDParent](const CXXRecordDecl *Base) { return NDParent != Base; });
+  if (!DC1->Bases.empty() && ND0->getAccess() != AS_private &&
+  isMemberOf(DC0, DC1))
+return true;
+
+  return enclosesContext(DC0, DC1) &&
+ (mayShadowImpl(ND0, ND1) || mayShadowImpl(DC0->NonTransparentContext,
+   DC1->NonTransparentContext));
 }
 
-static bool mayShadow(cons

[PATCH] D136919: [X86][RFC] Change mangle name of bf16 from u6bf16 to DF16b

2023-05-21 Thread Phoebe Wang via Phabricator via cfe-commits

pengfei abandoned this revision.
pengfei added a comment.

Abandon this in favor of D150913 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136919

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

[clang] 33d3d51 - [RISCV] Add missing test for ctz_32 on RV64

2023-05-21 Thread Jim Lin via cfe-commits


Author: Jim Lin
Date: 2023-05-22T10:28:27+08:00
New Revision: 33d3d51d77a7fb7ca6b919f9ba47999cd8531844

URL: 
https://github.com/llvm/llvm-project/commit/33d3d51d77a7fb7ca6b919f9ba47999cd8531844
DIFF: 
https://github.com/llvm/llvm-project/commit/33d3d51d77a7fb7ca6b919f9ba47999cd8531844.diff

LOG: [RISCV] Add missing test for ctz_32 on RV64

Apparently, both of clz and ctz should have tests for _32 version on RV64.

Reviewed By: kito-cheng

Differential Revision: https://reviews.llvm.org/D150945

Added: 


Modified: 
clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c

Removed: 




diff  --git a/clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c 
b/clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c
index e136151e511b3..0133cb1ec202d 100644
--- a/clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c
+++ b/clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c
@@ -50,6 +50,18 @@ long clz_64(long a) {
   return __builtin_riscv_clz_64(a);
 }
 
+// RV64ZBB-LABEL: @ctz_32(
+// RV64ZBB-NEXT:  entry:
+// RV64ZBB-NEXT:[[A_ADDR:%.*]] = alloca i32, align 4
+// RV64ZBB-NEXT:store i32 [[A:%.*]], ptr [[A_ADDR]], align 4
+// RV64ZBB-NEXT:[[TMP0:%.*]] = load i32, ptr [[A_ADDR]], align 4
+// RV64ZBB-NEXT:[[TMP1:%.*]] = call i32 @llvm.cttz.i32(i32 [[TMP0]], i1 
false)
+// RV64ZBB-NEXT:ret i32 [[TMP1]]
+//
+int ctz_32(int a) {
+  return __builtin_riscv_ctz_32(a);
+}
+
 // RV64ZBB-LABEL: @ctz_64(
 // RV64ZBB-NEXT:  entry:
 // RV64ZBB-NEXT:[[A_ADDR:%.*]] = alloca i64, align 8



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

[PATCH] D150945: [RISCV] Add missing test for ctz_32 on RV64

2023-05-21 Thread Jim Lin via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG33d3d51d77a7: [RISCV] Add missing test for ctz_32 on RV64 
(authored by Jim).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150945

Files:
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c


Index: clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c
===
--- clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c
+++ clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c
@@ -50,6 +50,18 @@
   return __builtin_riscv_clz_64(a);
 }
 
+// RV64ZBB-LABEL: @ctz_32(
+// RV64ZBB-NEXT:  entry:
+// RV64ZBB-NEXT:[[A_ADDR:%.*]] = alloca i32, align 4
+// RV64ZBB-NEXT:store i32 [[A:%.*]], ptr [[A_ADDR]], align 4
+// RV64ZBB-NEXT:[[TMP0:%.*]] = load i32, ptr [[A_ADDR]], align 4
+// RV64ZBB-NEXT:[[TMP1:%.*]] = call i32 @llvm.cttz.i32(i32 [[TMP0]], i1 
false)
+// RV64ZBB-NEXT:ret i32 [[TMP1]]
+//
+int ctz_32(int a) {
+  return __builtin_riscv_ctz_32(a);
+}
+
 // RV64ZBB-LABEL: @ctz_64(
 // RV64ZBB-NEXT:  entry:
 // RV64ZBB-NEXT:[[A_ADDR:%.*]] = alloca i64, align 8


Index: clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c
===
--- clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c
+++ clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c
@@ -50,6 +50,18 @@
   return __builtin_riscv_clz_64(a);
 }
 
+// RV64ZBB-LABEL: @ctz_32(
+// RV64ZBB-NEXT:  entry:
+// RV64ZBB-NEXT:[[A_ADDR:%.*]] = alloca i32, align 4
+// RV64ZBB-NEXT:store i32 [[A:%.*]], ptr [[A_ADDR]], align 4
+// RV64ZBB-NEXT:[[TMP0:%.*]] = load i32, ptr [[A_ADDR]], align 4
+// RV64ZBB-NEXT:[[TMP1:%.*]] = call i32 @llvm.cttz.i32(i32 [[TMP0]], i1 false)
+// RV64ZBB-NEXT:ret i32 [[TMP1]]
+//
+int ctz_32(int a) {
+  return __builtin_riscv_ctz_32(a);
+}
+
 // RV64ZBB-LABEL: @ctz_64(
 // RV64ZBB-NEXT:  entry:
 // RV64ZBB-NEXT:[[A_ADDR:%.*]] = alloca i64, align 8
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

2023-05-21 Thread Yurong via Phabricator via cfe-commits

yronglin updated this revision to Diff 524158.
yronglin added a comment.
Herald added a reviewer: jdoerfert.
Herald added subscribers: jplehr, sstefan1.

Fix crash.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150528

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/AttrImpl.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Sema/aix-attr-aligned-vector-warn.c
  clang/test/Sema/aix-attr-aligned-vector-warn.cpp
  clang/test/Sema/sizeless-1.c
  clang/test/SemaCXX/attr-cxx0x.cpp
  clang/test/SemaCXX/builtin-align-cxx.cpp
  clang/test/SemaCXX/cxx11-attr-print.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -508,6 +508,16 @@
   OS << "assert(!is" << getLowerName() << "Expr);\n";
   OS << "return " << getLowerName() << "Type;\n";
   OS << "  }";
+
+  OS << "  std::optional getCached" << getUpperName()
+ << "Value() const {\n";
+  OS << "return " << getLowerName() << "Cache;\n";
+  OS << "  }";
+
+  OS << "  void setCached" << getUpperName()
+ << "Value(unsigned AlignVal) {\n";
+  OS << "" << getLowerName() << "Cache = AlignVal;\n";
+  OS << "  }";
 }
 
 void writeAccessorDefinitions(raw_ostream &OS) const override {
@@ -530,21 +540,6 @@
   OS << "  return " << getLowerName()
  << "Type->getType()->containsErrors();\n";
   OS << "}\n";
-
-  // FIXME: Do not do the calculation here
-  // FIXME: Handle types correctly
-  // A null pointer means maximum alignment
-  OS << "unsigned " << getAttrName() << "Attr::get" << getUpperName()
- << "(ASTContext &Ctx) const {\n";
-  OS << "  assert(!is" << getUpperName() << "Dependent());\n";
-  OS << "  if (is" << getLowerName() << "Expr)\n";
-  OS << "return " << getLowerName() << "Expr ? " << getLowerName()
- << "Expr->EvaluateKnownConstInt(Ctx).getZExtValue()"
- << " * Ctx.getCharWidth() : "
- << "Ctx.getTargetDefaultAlignForAttributeAligned();\n";
-  OS << "  else\n";
-  OS << "return 0; // FIXME\n";
-  OS << "}\n";
 }
 
 void writeASTVisitorTraversal(raw_ostream &OS) const override {
@@ -601,7 +596,8 @@
   OS << "union {\n";
   OS << "Expr *" << getLowerName() << "Expr;\n";
   OS << "TypeSourceInfo *" << getLowerName() << "Type;\n";
-  OS << "};";
+  OS << "};\n";
+  OS << "std::optional " << getLowerName() << "Cache;\n";
 }
 
 void writePCHReadArgs(raw_ostream &OS) const override {
@@ -628,14 +624,17 @@
 }
 
 std::string getIsOmitted() const override {
-  return "!is" + getLowerName().str() + "Expr || !" + getLowerName().str()
- + "Expr";
+  return "!((is" + getLowerName().str() + "Expr && " +
+ getLowerName().str() + "Expr) || (!is" + getLowerName().str() +
+ "Expr && " + getLowerName().str() + "Type))";
 }
 
 void writeValue(raw_ostream &OS) const override {
   OS << "\";\n";
-  OS << "" << getLowerName()
- << "Expr->printPretty(OS, nullptr, Policy);\n";
+  OS << "if (is" << getLowerName() << "Expr && " << getLowerName() << "Expr)";
+  OS << "  " << getLowerName() << "Expr->printPretty(OS, nullptr, Policy);\n";
+  OS << "if (!is" << getLowerName() << "Expr && " << getLowerName() << "Type)";
+  OS << "  " << getLowerName() << "Type->getType().print(OS, Policy);\n";
   OS << "OS << \"";
 }
 
Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -73,7 +73,7 @@
   svint8_t __attribute__((aligned(4))) aligned_int8_2; // expected-error {{'aligned' attribute cannot be applied to sizeless type 'svint8_t'}}
   svint8_t _Alignas(int) aligned_int8_3;   // expected-error {{'_Alignas' attribute cannot be applied to sizeless type 'svint8_t'}}
 
-  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of 'alignof' to sizeless type 'svint8_t'}}
+  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of '_Alignas' to sizeless type 'svint8_t'}}
 
   // Using pointers to sizeless data isn't wrong here, but because the
   // type is incomplete, it doesn't provide any alignment guarantees.
Index: clang/test/SemaCXX/cxx11-attr-print.cpp
==

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

2023-05-21 Thread Yurong via Phabricator via cfe-commits

yronglin updated this revision to Diff 524159.
yronglin added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150528

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/AttrImpl.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Sema/aix-attr-aligned-vector-warn.c
  clang/test/Sema/aix-attr-aligned-vector-warn.cpp
  clang/test/Sema/sizeless-1.c
  clang/test/SemaCXX/attr-cxx0x.cpp
  clang/test/SemaCXX/builtin-align-cxx.cpp
  clang/test/SemaCXX/cxx11-attr-print.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -508,6 +508,16 @@
   OS << "assert(!is" << getLowerName() << "Expr);\n";
   OS << "return " << getLowerName() << "Type;\n";
   OS << "  }";
+
+  OS << "  std::optional getCached" << getUpperName()
+ << "Value() const {\n";
+  OS << "return " << getLowerName() << "Cache;\n";
+  OS << "  }";
+
+  OS << "  void setCached" << getUpperName()
+ << "Value(unsigned AlignVal) {\n";
+  OS << "" << getLowerName() << "Cache = AlignVal;\n";
+  OS << "  }";
 }
 
 void writeAccessorDefinitions(raw_ostream &OS) const override {
@@ -530,21 +540,6 @@
   OS << "  return " << getLowerName()
  << "Type->getType()->containsErrors();\n";
   OS << "}\n";
-
-  // FIXME: Do not do the calculation here
-  // FIXME: Handle types correctly
-  // A null pointer means maximum alignment
-  OS << "unsigned " << getAttrName() << "Attr::get" << getUpperName()
- << "(ASTContext &Ctx) const {\n";
-  OS << "  assert(!is" << getUpperName() << "Dependent());\n";
-  OS << "  if (is" << getLowerName() << "Expr)\n";
-  OS << "return " << getLowerName() << "Expr ? " << getLowerName()
- << "Expr->EvaluateKnownConstInt(Ctx).getZExtValue()"
- << " * Ctx.getCharWidth() : "
- << "Ctx.getTargetDefaultAlignForAttributeAligned();\n";
-  OS << "  else\n";
-  OS << "return 0; // FIXME\n";
-  OS << "}\n";
 }
 
 void writeASTVisitorTraversal(raw_ostream &OS) const override {
@@ -601,7 +596,8 @@
   OS << "union {\n";
   OS << "Expr *" << getLowerName() << "Expr;\n";
   OS << "TypeSourceInfo *" << getLowerName() << "Type;\n";
-  OS << "};";
+  OS << "};\n";
+  OS << "std::optional " << getLowerName() << "Cache;\n";
 }
 
 void writePCHReadArgs(raw_ostream &OS) const override {
@@ -628,14 +624,17 @@
 }
 
 std::string getIsOmitted() const override {
-  return "!is" + getLowerName().str() + "Expr || !" + getLowerName().str()
- + "Expr";
+  return "!((is" + getLowerName().str() + "Expr && " +
+ getLowerName().str() + "Expr) || (!is" + getLowerName().str() +
+ "Expr && " + getLowerName().str() + "Type))";
 }
 
 void writeValue(raw_ostream &OS) const override {
   OS << "\";\n";
-  OS << "" << getLowerName()
- << "Expr->printPretty(OS, nullptr, Policy);\n";
+  OS << "if (is" << getLowerName() << "Expr && " << getLowerName() << "Expr)";
+  OS << "  " << getLowerName() << "Expr->printPretty(OS, nullptr, Policy);\n";
+  OS << "if (!is" << getLowerName() << "Expr && " << getLowerName() << "Type)";
+  OS << "  " << getLowerName() << "Type->getType().print(OS, Policy);\n";
   OS << "OS << \"";
 }
 
Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -73,7 +73,7 @@
   svint8_t __attribute__((aligned(4))) aligned_int8_2; // expected-error {{'aligned' attribute cannot be applied to sizeless type 'svint8_t'}}
   svint8_t _Alignas(int) aligned_int8_3;   // expected-error {{'_Alignas' attribute cannot be applied to sizeless type 'svint8_t'}}
 
-  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of 'alignof' to sizeless type 'svint8_t'}}
+  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of '_Alignas' to sizeless type 'svint8_t'}}
 
   // Using pointers to sizeless data isn't wrong here, but because the
   // type is incomplete, it doesn't provide any alignment guarantees.
Index: clang/test/SemaCXX/cxx11-attr-print.cpp
===
--- clang/test/SemaCXX/cxx11-attr-print.cpp
+++ clang/test/SemaCXX

[PATCH] D150840: [clang][NFC] Refactor emitSnippet()

2023-05-21 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added a comment.

Side-note: The `ToPrint` stuff here is just useful to "cache" the output 
string, isn't it? Is that really useful? I'd expect the output stream to be 
cached anyway?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150840

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

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

2023-05-21 Thread Yurong via Phabricator via cfe-commits

yronglin updated this revision to Diff 524165.
yronglin added a comment.

Format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150528

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/AttrImpl.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Sema/aix-attr-aligned-vector-warn.c
  clang/test/Sema/aix-attr-aligned-vector-warn.cpp
  clang/test/Sema/sizeless-1.c
  clang/test/SemaCXX/attr-cxx0x.cpp
  clang/test/SemaCXX/builtin-align-cxx.cpp
  clang/test/SemaCXX/cxx11-attr-print.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -508,6 +508,16 @@
   OS << "assert(!is" << getLowerName() << "Expr);\n";
   OS << "return " << getLowerName() << "Type;\n";
   OS << "  }";
+
+  OS << "  std::optional getCached" << getUpperName()
+ << "Value() const {\n";
+  OS << "return " << getLowerName() << "Cache;\n";
+  OS << "  }";
+
+  OS << "  void setCached" << getUpperName()
+ << "Value(unsigned AlignVal) {\n";
+  OS << "" << getLowerName() << "Cache = AlignVal;\n";
+  OS << "  }";
 }
 
 void writeAccessorDefinitions(raw_ostream &OS) const override {
@@ -530,21 +540,6 @@
   OS << "  return " << getLowerName()
  << "Type->getType()->containsErrors();\n";
   OS << "}\n";
-
-  // FIXME: Do not do the calculation here
-  // FIXME: Handle types correctly
-  // A null pointer means maximum alignment
-  OS << "unsigned " << getAttrName() << "Attr::get" << getUpperName()
- << "(ASTContext &Ctx) const {\n";
-  OS << "  assert(!is" << getUpperName() << "Dependent());\n";
-  OS << "  if (is" << getLowerName() << "Expr)\n";
-  OS << "return " << getLowerName() << "Expr ? " << getLowerName()
- << "Expr->EvaluateKnownConstInt(Ctx).getZExtValue()"
- << " * Ctx.getCharWidth() : "
- << "Ctx.getTargetDefaultAlignForAttributeAligned();\n";
-  OS << "  else\n";
-  OS << "return 0; // FIXME\n";
-  OS << "}\n";
 }
 
 void writeASTVisitorTraversal(raw_ostream &OS) const override {
@@ -601,7 +596,8 @@
   OS << "union {\n";
   OS << "Expr *" << getLowerName() << "Expr;\n";
   OS << "TypeSourceInfo *" << getLowerName() << "Type;\n";
-  OS << "};";
+  OS << "};\n";
+  OS << "std::optional " << getLowerName() << "Cache;\n";
 }
 
 void writePCHReadArgs(raw_ostream &OS) const override {
@@ -628,14 +624,21 @@
 }
 
 std::string getIsOmitted() const override {
-  return "!is" + getLowerName().str() + "Expr || !" + getLowerName().str()
- + "Expr";
+  return "!((is" + getLowerName().str() + "Expr && " +
+ getLowerName().str() + "Expr) || (!is" + getLowerName().str() +
+ "Expr && " + getLowerName().str() + "Type))";
 }
 
 void writeValue(raw_ostream &OS) const override {
   OS << "\";\n";
-  OS << "" << getLowerName()
+  OS << "if (is" << getLowerName() << "Expr && " << getLowerName()
+ << "Expr)";
+  OS << "  " << getLowerName()
  << "Expr->printPretty(OS, nullptr, Policy);\n";
+  OS << "if (!is" << getLowerName() << "Expr && " << getLowerName()
+ << "Type)";
+  OS << "  " << getLowerName()
+ << "Type->getType().print(OS, Policy);\n";
   OS << "OS << \"";
 }
 
Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -73,7 +73,7 @@
   svint8_t __attribute__((aligned(4))) aligned_int8_2; // expected-error {{'aligned' attribute cannot be applied to sizeless type 'svint8_t'}}
   svint8_t _Alignas(int) aligned_int8_3;   // expected-error {{'_Alignas' attribute cannot be applied to sizeless type 'svint8_t'}}
 
-  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of 'alignof' to sizeless type 'svint8_t'}}
+  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of '_Alignas' to sizeless type 'svint8_t'}}
 
   // Using pointers to sizeless data isn't wrong here, but because the
   // type is incomplete, it doesn't provide any alignment guarantees.
Index: clang/test/SemaCXX/cxx11-attr-print.cpp
===
--- clang/test/SemaCXX/cxx11-attr-print.cpp
+++ clang/test/SemaCXX/cxx11-attr-print

[PATCH] D150775: [clang][dataflow] Fix a bug in handling of `operator->` for optional checker.

2023-05-21 Thread Martin Böhme via Phabricator via cfe-commits

mboehme marked 2 inline comments as done.
mboehme added inline comments.



Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:789
-  // of these accessors.
-  .CaseOfCFGStmt(valueOperatorCall(std::nullopt),
[](const CallExpr *E,

ymandel wrote:
> mboehme wrote:
> > ymandel wrote:
> > > this function will be left with only one caller. Maybe inline it? For 
> > > that matter, should the other caller be changed in some parallel way?
> > > this function will be left with only one caller. Maybe inline it?
> > 
> > The general style in this file seems to be to pull matcher expressions out 
> > into helper functions, even if they're only used in one place. FWIW, I'm 
> > personally not convinced this is helpful because you _want_ to be able to 
> > see exactly what a `CaseOfCFGStmt` is matching on, so inlining makes sense 
> > IMO. But I'm not sure I want to be inconsistent with the general style of 
> > this file -- it would make more sense to do a general cleanup. WDYT?
> > 
> > > For that matter, should the other caller be changed in some parallel way?
> > 
> > No, that caller (`buildDiagnoseMatchSwitch()`) does not have the same 
> > problem because it looks only at the `optional`, not at the value that is 
> > contained in it (see also [D150756](https://reviews.llvm.org/D150756), 
> > which makes this explicit), and the distinction between `operator*` and 
> > `operator->` (for our purposes) is only in the way they return the 
> > contained value.
> regarding the matcher -- it's a judgment call; I agree that having to jump 
> back and forth to read this case "statement" is problematic, but some of the 
> matchers are kind of complicated, so a name can be a service to the reader. 
> This one looked sufficiently simple to consider just inlining it, but I'm 
> also fine leaving as is.
In that case, I think I'll leave this as-is for the time being and will revisit 
the question of inlining more generally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150775

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

[PATCH] D150937: [clang-repl] Disable all tests on unsupported platforms

2023-05-21 Thread Vassil Vassilev via Phabricator via cfe-commits

v.g.vassilev added a comment.

I would support this change but instead of asking the JIT we should just add 
`-fsyntax-only` to these invocations as they do not rely on the JIT at all.




Comment at: clang/test/Interpreter/incremental-mode.cpp:3
 // RUN: clang-repl -Xcc -emit-llvm 
+// UNSUPPORTED: system-aix
 // expected-no-diagnostics

This test does not run anything, so this should be supported. Perhaps we could 
add `-fsyntax-only` mode to the RUN lines above.



Comment at: clang/unittests/Interpreter/IncrementalProcessingTest.cpp:74
+return;
   std::vector ClangArgv = {"-Xclang", "-emit-llvm-only"};
   auto CI = llvm::cantFail(IncrementalCompilerBuilder::create(ClangArgv));

Likewise, here `-fsyntax-only` mode should be enough since we do not execute 
anything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150937

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

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

2023-05-21 Thread Bevin Hansson via Phabricator via cfe-commits

ebevhan added a comment.

In D58346#4359291 , @jhuber6 wrote:

> should C++ really be limited by OpenCL here?

It probably shouldn't. There's many places in the codebase where OpenCL flags 
restrict generic address space behavior. I have a patch at D62574 
 that attempts to formalize the address space 
rules a bit more (which I think also gets rid of the OpenCL restrictions you 
mention) but there's never been any huge interest and I don't have the time to 
push for it.

Whether that patch actually solves your problem or not, I don't know.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58346

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

[PATCH] D151059: [test] Add C++ ext_vector_type tests

2023-05-21 Thread Cassie Jones via Phabricator via cfe-commits

porglezomp created this revision.
porglezomp added reviewers: aaron.ballman, fhahn.
Herald added a subscriber: StephenFan.
Herald added a project: All.
porglezomp requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add initial tests for the behavior of ext_vector_type vectors for
vector vs scalar ops in C++. Their behavior doesn't agree with the behavior in
C and what the behavior seems like it should be, these are baseline tests before
implementing those changes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151059

Files:
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -545,3 +545,225 @@
   auto b4 = (v4 >= 0x12);
 }
 #endif
+
+namespace all_operators {
+typedef unsigned int v2u __attribute__((ext_vector_type(2)));
+typedef float v2f __attribute__((ext_vector_type(2)));
+
+void test_int_vector_scalar(unsigned int ua, v2u v2ua) {
+  // Integer vector vs integer operators. These will splat
+  (void)(v2ua + ua);
+  (void)(ua + v2ua);
+  (void)(v2ua - ua);
+  (void)(ua - v2ua);
+  (void)(v2ua * ua);
+  (void)(ua * v2ua);
+  (void)(v2ua / ua);
+  (void)(ua / v2ua);
+  (void)(v2ua % ua);
+  (void)(ua % v2ua);
+
+  (void)(v2ua == ua);
+  (void)(ua == v2ua);
+  (void)(v2ua != ua);
+  (void)(ua != v2ua);
+  (void)(v2ua <= ua);
+  (void)(ua <= v2ua);
+  (void)(v2ua >= ua);
+  (void)(ua >= v2ua);
+  (void)(v2ua < ua);
+  (void)(ua < v2ua);
+  (void)(v2ua > ua);
+  (void)(ua > v2ua);
+  (void)(v2ua && ua);
+  (void)(ua && v2ua);
+  (void)(v2ua || ua);
+  (void)(ua || v2ua);
+
+  (void)(v2ua & ua);
+  (void)(ua & v2ua);
+  (void)(v2ua | ua);
+  (void)(ua | v2ua);
+  (void)(v2ua ^ ua);
+  (void)(ua ^ v2ua);
+  (void)(v2ua << ua);
+  (void)(ua << v2ua);
+  (void)(v2ua >> ua);
+  (void)(ua >> v2ua);
+
+  v2ua += ua;
+  v2ua -= ua;
+  v2ua *= ua;
+  v2ua /= ua;
+  v2ua %= ua;
+  v2ua &= ua;
+  v2ua |= ua;
+  v2ua ^= ua;
+  v2ua >>= ua;
+  v2ua <<= ua;
+
+  ua += v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua -= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua *= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua /= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua %= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua &= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua |= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua ^= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua >>= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua <<= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+}
+
+void test_float_vector_scalar(float fa, unsigned int ua, v2f v2fa) {
+  // Float vector vs float operators. These will splat
+  (void)(v2fa + fa);
+  (void)(fa + v2fa);
+  (void)(v2fa - fa);
+  (void)(fa - v2fa);
+  (void)(v2fa * fa);
+  (void)(fa * v2fa);
+  (void)(v2fa / fa);
+  (void)(fa / v2fa);
+  (void)(v2fa % fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa % v2fa); // expected-error{{invalid operands to binary expression}}
+
+  (void)(v2fa == fa);
+  (void)(fa == v2fa);
+  (void)(v2fa != fa);
+  (void)(fa != v2fa);
+  (void)(v2fa <= fa);
+  (void)(fa <= v2fa);
+  (void)(v2fa >= fa);
+  (void)(fa >= v2fa);
+  (void)(v2fa < fa);
+  (void)(fa < v2fa);
+  (void)(v2fa > fa);
+  (void)(fa > v2fa);
+  (void)(v2fa && fa);
+  (void)(fa && v2fa);
+  (void)(v2fa || fa);
+  (void)(fa || v2fa);
+
+  (void)(v2fa & fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa & v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa | fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa | v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa ^ fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa ^ v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa << fa); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(v2fa << ua); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(fa << v2fa); // expected-error{{used type 'float' where integer is required}}
+  (void)(ua << v2fa); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(v2fa >> fa); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(v2fa >> ua); // expected-error{{used type 'v2f' (vector of 2 'float' values) w

[PATCH] D151060: [Clang][Sema] Generate vector vs scalar builtin overloads

2023-05-21 Thread Cassie Jones via Phabricator via cfe-commits

porglezomp created this revision.
porglezomp added reviewers: aaron.ballman, fhahn.
Herald added a subscriber: StephenFan.
Herald added a project: All.
porglezomp requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The bit-shifting operator builtin support vector vs scalars, but the
operator overloads did not. That mismatch caused an assertion failure
when trying to shift a vector by an enum.

This makes C++ match the behavior of C for these conversions on
bit-shifts.

rdar://108819842
Depends on D151059 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151060

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -726,11 +726,10 @@
   (void)(ea | v2ua); // expected-error{{cannot convert between vector values of different size}}
   (void)(v2ua ^ ea); // expected-error{{cannot convert between vector values of different size}}
   (void)(ea ^ v2ua); // expected-error{{cannot convert between vector values of different size}}
-  // FIXME: Vector/scalar shifts cause an assertion failure
-  // (void)(v2ua << ea);
-  // (void)(ea << v2ua);
-  // (void)(v2ua >> ea);
-  // (void)(ea >> v2ua);
+  (void)(v2ua << ea);
+  (void)(ea << v2ua);
+  (void)(v2ua >> ea);
+  (void)(ea >> v2ua);
 
   v2ua += ea; // expected-error{{cannot convert between vector values of different size}}
   v2ua -= ea; // expected-error{{cannot convert between vector values of different size}}
@@ -740,9 +739,8 @@
   v2ua &= ea; // expected-error{{cannot convert between vector values of different size}}
   v2ua |= ea; // expected-error{{cannot convert between vector values of different size}}
   v2ua ^= ea; // expected-error{{cannot convert between vector values of different size}}
-  // FIXME: Vector/scalar shifts cause an assertion failure
-  // v2ua >>= ea;
-  // v2ua <<= ea;
+  v2ua >>= ea;
+  v2ua <<= ea;
 
   ea += v2ua; // expected-error{{cannot convert between vector values of different size}}
   ea -= v2ua; // expected-error{{cannot convert between vector values of different size}}
@@ -752,9 +750,8 @@
   ea &= v2ua; // expected-error{{cannot convert between vector values of different size}}
   ea |= v2ua; // expected-error{{cannot convert between vector values of different size}}
   ea ^= v2ua; // expected-error{{cannot convert between vector values of different size}}
-  // FIXME: Vector/scalar shifts cause an assertion failure
-  // ea >>= v2ua; // not-expected-error{{assigning to 'enum Enum' from incompatible type 'v2u'}}
-  // ea <<= v2ua; // not-expected-error{{assigning to 'enum Enum' from incompatible type 'v2u'}}
+  ea >>= v2ua; // expected-error{{assigning to 'Enum' from incompatible type 'v2u'}}
+  ea <<= v2ua; // expected-error{{assigning to 'Enum' from incompatible type 'v2u'}}
 }
 
 #if __cplusplus >= 201103L // C++11 or later
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -9024,6 +9024,30 @@
 S.AddBuiltinCandidate(LandR, Args, CandidateSet);
   }
 }
+
+// Extension: Add the binary bitwise operators for vector types
+if (!CandidateTypes[0].vector_types().empty() &&
+!CandidateTypes[1].vector_types().empty()) {
+  // If both candidates have vector types, then add them pairwise...
+  for (QualType Vec1Ty : CandidateTypes[0].vector_types()) {
+for (QualType Vec2Ty : CandidateTypes[1].vector_types()) {
+  QualType LandR[2] = {Vec1Ty, Vec2Ty};
+  S.AddBuiltinCandidate(LandR, Args, CandidateSet);
+}
+  }
+} else {
+  // ...but if only one side has vector candidates, use that set of
+  // vector candidates pairwise for both sides of the operator
+  // (allowing splatting the scalar to a vector).
+  for (unsigned Candidate = 0; Candidate < 2; ++Candidate) {
+for (QualType Vec1Ty : CandidateTypes[Candidate].vector_types()) {
+  for (QualType Vec2Ty : CandidateTypes[Candidate].vector_types()) {
+QualType LandR[2] = {Vec1Ty, Vec2Ty};
+S.AddBuiltinCandidate(LandR, Args, CandidateSet);
+  }
+}
+  }
+}
   }
 
   // C++ [over.built]p20:
@@ -9250,6 +9274,25 @@
 });
   }
 }
+
+// Extension: Add the binary operators %=, <<=, >>=, &=, ^=, |= for vector types.
+for (QualType Vec1Ty : CandidateTypes[0].vector_types())
+  for (QualType Vec2Ty : CandidateTypes[0].vector_types()) {
+QualType ParamTypes[2];
+ParamTypes[1] = Vec2Ty;
+// Add this built-in operator as a candidate (VQ is empty).
+ParamTypes[0] = S.Context.getLValueReferenceType(Vec1Ty);
+S.AddBuiltinCandidate(ParamTypes, Args, Can

[clang] 5a16665 - [clang][dataflow] Use `Strict` accessors in more places in Transfer.cpp.

2023-05-21 Thread Martin Braenne via cfe-commits


Author: Martin Braenne
Date: 2023-05-22T06:51:10Z
New Revision: 5a16665ed53500b7ee6703be3d5b9e0bca6f1c14

URL: 
https://github.com/llvm/llvm-project/commit/5a16665ed53500b7ee6703be3d5b9e0bca6f1c14
DIFF: 
https://github.com/llvm/llvm-project/commit/5a16665ed53500b7ee6703be3d5b9e0bca6f1c14.diff

LOG: [clang][dataflow] Use `Strict` accessors in more places in Transfer.cpp.

This patch handles the straightforward cases. Upcoming separate patches will 
handle the cases that are more subtle.

This patch is part of the ongoing migration to strict handling of value 
categories (see https://discourse.llvm.org/t/70086 for details).

Depends On D150653

Reviewed By: sammccall, ymandel, xazax.hun

Differential Revision: https://reviews.llvm.org/D150655

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/Transfer.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp 
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 45c601ed49146..838ad934b5fbd 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -48,10 +48,8 @@ const Environment *StmtToEnvMap::getEnvironment(const Stmt 
&S) const {
 
 static BoolValue &evaluateBooleanEquality(const Expr &LHS, const Expr &RHS,
   Environment &Env) {
-  if (auto *LHSValue =
-  dyn_cast_or_null(Env.getValue(LHS, SkipPast::Reference)))
-if (auto *RHSValue =
-dyn_cast_or_null(Env.getValue(RHS, 
SkipPast::Reference)))
+  if (auto *LHSValue = dyn_cast_or_null(Env.getValueStrict(LHS)))
+if (auto *RHSValue = dyn_cast_or_null(Env.getValueStrict(RHS)))
   return Env.makeIff(*LHSValue, *RHSValue);
 
   return Env.makeAtomicBoolValue();
@@ -121,9 +119,7 @@ static BoolValue &unpackValue(BoolValue &V, Environment 
&Env) {
 // value, if any unpacking occured. Also, does the lvalue-to-rvalue conversion,
 // by skipping past the reference.
 static Value *maybeUnpackLValueExpr(const Expr &E, Environment &Env) {
-  // FIXME: this is too flexible: it _allows_ a reference, while it should
-  // _require_ one, since lvalues should always be wrapped in `ReferenceValue`.
-  auto *Loc = Env.getStorageLocation(E, SkipPast::Reference);
+  auto *Loc = Env.getStorageLocationStrict(E);
   if (Loc == nullptr)
 return nullptr;
   auto *Val = Env.getValue(*Loc);
@@ -139,6 +135,29 @@ static Value *maybeUnpackLValueExpr(const Expr &E, 
Environment &Env) {
   return &UnpackedVal;
 }
 
+static void propagateValue(const Expr &From, const Expr &To, Environment &Env) 
{
+  if (auto *Val = Env.getValueStrict(From))
+Env.setValueStrict(To, *Val);
+}
+
+static void propagateStorageLocation(const Expr &From, const Expr &To,
+ Environment &Env) {
+  if (auto *Loc = Env.getStorageLocationStrict(From))
+Env.setStorageLocationStrict(To, *Loc);
+}
+
+// Forwards the value or storage location of `From` to `To` in cases where
+// `From` may be either a glvalue or a prvalue. `To` must be a glvalue iff
+// `From` is a glvalue.
+static void propagateValueOrStorageLocation(const Expr &From, const Expr &To,
+Environment &Env) {
+  assert(From.isGLValue() == To.isGLValue());
+  if (From.isGLValue())
+propagateStorageLocation(From, To, Env);
+  else
+propagateValue(From, To, Env);
+}
+
 namespace {
 
 class TransferVisitor : public ConstStmtVisitor {
@@ -155,13 +174,11 @@ class TransferVisitor : public 
ConstStmtVisitor {
 
 switch (S->getOpcode()) {
 case BO_Assign: {
-  auto *LHSLoc = Env.getStorageLocation(*LHS, SkipPast::Reference);
+  auto *LHSLoc = Env.getStorageLocationStrict(*LHS);
   if (LHSLoc == nullptr)
 break;
 
-  // No skipping should be necessary, because any lvalues should have
-  // already been stripped off in evaluating the LValueToRValue cast.
-  auto *RHSVal = Env.getValue(*RHS, SkipPast::None);
+  auto *RHSVal = Env.getValueStrict(*RHS);
   if (RHSVal == nullptr)
 break;
 
@@ -276,7 +293,7 @@ class TransferVisitor : public 
ConstStmtVisitor {
 return;
   }
 
-  if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None))
+  if (auto *InitExprVal = Env.getValueStrict(*InitExpr))
 Env.setValue(Loc, *InitExprVal);
 
   if (Env.getValue(Loc) == nullptr) {
@@ -443,7 +460,7 @@ class TransferVisitor : public 
ConstStmtVisitor {
 }
 case UO_LNot: {
   auto *SubExprVal =
-  dyn_cast_or_null(Env.getValue(*SubExpr, SkipPast::None));
+  dyn_cast_or_null(Env.getValueStrict(*SubExpr));
   if (SubExprVal == nullptr)
 break;
 
@@ -653,19 +670,13 @@ class TransferVisitor : public 
ConstStmtVisitor {
   const Expr *SubExpr = S->getSubExpr();
   assert(SubExpr != nullptr);
 
-  auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None);

[PATCH] D150655: [clang][dataflow] Use `Strict` accessors in more places in Transfer.cpp.

2023-05-21 Thread Martin Böhme via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
mboehme marked an inline comment as done.
Closed by commit rG5a16665ed535: [clang][dataflow] Use `Strict` accessors in 
more places in Transfer.cpp. (authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150655

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp

Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -48,10 +48,8 @@
 
 static BoolValue &evaluateBooleanEquality(const Expr &LHS, const Expr &RHS,
   Environment &Env) {
-  if (auto *LHSValue =
-  dyn_cast_or_null(Env.getValue(LHS, SkipPast::Reference)))
-if (auto *RHSValue =
-dyn_cast_or_null(Env.getValue(RHS, SkipPast::Reference)))
+  if (auto *LHSValue = dyn_cast_or_null(Env.getValueStrict(LHS)))
+if (auto *RHSValue = dyn_cast_or_null(Env.getValueStrict(RHS)))
   return Env.makeIff(*LHSValue, *RHSValue);
 
   return Env.makeAtomicBoolValue();
@@ -121,9 +119,7 @@
 // value, if any unpacking occured. Also, does the lvalue-to-rvalue conversion,
 // by skipping past the reference.
 static Value *maybeUnpackLValueExpr(const Expr &E, Environment &Env) {
-  // FIXME: this is too flexible: it _allows_ a reference, while it should
-  // _require_ one, since lvalues should always be wrapped in `ReferenceValue`.
-  auto *Loc = Env.getStorageLocation(E, SkipPast::Reference);
+  auto *Loc = Env.getStorageLocationStrict(E);
   if (Loc == nullptr)
 return nullptr;
   auto *Val = Env.getValue(*Loc);
@@ -139,6 +135,29 @@
   return &UnpackedVal;
 }
 
+static void propagateValue(const Expr &From, const Expr &To, Environment &Env) {
+  if (auto *Val = Env.getValueStrict(From))
+Env.setValueStrict(To, *Val);
+}
+
+static void propagateStorageLocation(const Expr &From, const Expr &To,
+ Environment &Env) {
+  if (auto *Loc = Env.getStorageLocationStrict(From))
+Env.setStorageLocationStrict(To, *Loc);
+}
+
+// Forwards the value or storage location of `From` to `To` in cases where
+// `From` may be either a glvalue or a prvalue. `To` must be a glvalue iff
+// `From` is a glvalue.
+static void propagateValueOrStorageLocation(const Expr &From, const Expr &To,
+Environment &Env) {
+  assert(From.isGLValue() == To.isGLValue());
+  if (From.isGLValue())
+propagateStorageLocation(From, To, Env);
+  else
+propagateValue(From, To, Env);
+}
+
 namespace {
 
 class TransferVisitor : public ConstStmtVisitor {
@@ -155,13 +174,11 @@
 
 switch (S->getOpcode()) {
 case BO_Assign: {
-  auto *LHSLoc = Env.getStorageLocation(*LHS, SkipPast::Reference);
+  auto *LHSLoc = Env.getStorageLocationStrict(*LHS);
   if (LHSLoc == nullptr)
 break;
 
-  // No skipping should be necessary, because any lvalues should have
-  // already been stripped off in evaluating the LValueToRValue cast.
-  auto *RHSVal = Env.getValue(*RHS, SkipPast::None);
+  auto *RHSVal = Env.getValueStrict(*RHS);
   if (RHSVal == nullptr)
 break;
 
@@ -276,7 +293,7 @@
 return;
   }
 
-  if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None))
+  if (auto *InitExprVal = Env.getValueStrict(*InitExpr))
 Env.setValue(Loc, *InitExprVal);
 
   if (Env.getValue(Loc) == nullptr) {
@@ -443,7 +460,7 @@
 }
 case UO_LNot: {
   auto *SubExprVal =
-  dyn_cast_or_null(Env.getValue(*SubExpr, SkipPast::None));
+  dyn_cast_or_null(Env.getValueStrict(*SubExpr));
   if (SubExprVal == nullptr)
 break;
 
@@ -653,19 +670,13 @@
   const Expr *SubExpr = S->getSubExpr();
   assert(SubExpr != nullptr);
 
-  auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None);
-  if (SubExprLoc == nullptr)
-return;
-
-  Env.setStorageLocation(*S, *SubExprLoc);
+  propagateValue(*SubExpr, *S, Env);
 }
   }
 
   void VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *S) {
-auto &Loc = Env.createStorageLocation(*S);
-Env.setStorageLocation(*S, Loc);
 if (Value *Val = Env.createValue(S->getType()))
-  Env.setValue(Loc, *Val);
+  Env.setValueStrict(*S, *Val);
   }
 
   void VisitCallExpr(const CallExpr *S) {
@@ -703,22 +714,20 @@
 const Expr *SubExpr = S->getSubExpr();
 assert(SubExpr != nullptr);
 
-auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None);
-if (SubExprLoc == nullptr)
+Value *SubExprVal = Env.getValueStrict(*SubExpr);
+if (SubExprVal == nullptr)
   return;
 
-Env.setStorageLocation(*S, *SubExprLoc);
+auto &Loc = Env.createStorage

[clang] 96b22e1 - [clang][dataflow] Use `Strict` accessors in SignAnalysisTest.cpp.

2023-05-21 Thread Martin Braenne via cfe-commits


Author: Martin Braenne
Date: 2023-05-22T06:51:13Z
New Revision: 96b22e1c378a93a26cd8fc8051bfc7b90f65b665

URL: 
https://github.com/llvm/llvm-project/commit/96b22e1c378a93a26cd8fc8051bfc7b90f65b665
DIFF: 
https://github.com/llvm/llvm-project/commit/96b22e1c378a93a26cd8fc8051bfc7b90f65b665.diff

LOG: [clang][dataflow] Use `Strict` accessors in SignAnalysisTest.cpp.

This patch is part of the ongoing migration to strict handling of value 
categories (see https://discourse.llvm.org/t/70086 for details).

Depends On D150656

Reviewed By: sammccall, ymandel, xazax.hun

Differential Revision: https://reviews.llvm.org/D150657

Added: 


Modified: 
clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp

Removed: 




diff  --git a/clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
index 325ffe1af9914..fe4fdda888bf5 100644
--- a/clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
@@ -114,12 +114,10 @@ getValueAndSignProperties(const UnaryOperator *UO,
 return {nullptr, {}, {}};
 
   // Value of the unary op.
-  auto *UnaryOpValue = State.Env.getValue(*UO, SkipPast::None);
+  auto *UnaryOpValue = State.Env.getValueStrict(*UO);
   if (!UnaryOpValue) {
-auto &Loc = State.Env.createStorageLocation(*UO);
-State.Env.setStorageLocation(*UO, Loc);
 UnaryOpValue = &State.Env.makeAtomicBoolValue();
-State.Env.setValue(Loc, *UnaryOpValue);
+State.Env.setValueStrict(*UO, *UnaryOpValue);
   }
 
   // Properties for the operand (sub expression).
@@ -133,22 +131,17 @@ getValueAndSignProperties(const UnaryOperator *UO,
 
 void transferBinary(const BinaryOperator *BO, const MatchFinder::MatchResult 
&M,
 LatticeTransferState &State) {
-  StorageLocation *Loc = State.Env.getStorageLocation(*BO, SkipPast::None);
-  if (!Loc) {
-Loc = &State.Env.createStorageLocation(*BO);
-State.Env.setStorageLocation(*BO, *Loc);
-  }
-  BoolValue *Comp = cast_or_null(State.Env.getValue(*Loc));
+  BoolValue *Comp = cast_or_null(State.Env.getValueStrict(*BO));
   if (!Comp) {
 Comp = &State.Env.makeAtomicBoolValue();
-State.Env.setValue(*Loc, *Comp);
+State.Env.setValueStrict(*BO, *Comp);
   }
 
   // FIXME Use this as well:
   // auto *NegatedComp = &State.Env.makeNot(*Comp);
 
-  auto *LHS = State.Env.getValue(*BO->getLHS(), SkipPast::None);
-  auto *RHS = State.Env.getValue(*BO->getRHS(), SkipPast::None);
+  auto *LHS = State.Env.getValueStrict(*BO->getLHS());
+  auto *RHS = State.Env.getValueStrict(*BO->getRHS());
 
   if (!LHS || !RHS)
 return;
@@ -244,19 +237,43 @@ void transferUnaryNot(const UnaryOperator *UO,
   }
 }
 
+// Returns the `Value` associated with `E` (which may be either a prvalue or
+// glvalue). Creates a `Value` or `StorageLocation` as needed if `E` does not
+// have either of these associated with it yet.
+//
+// If this functionality turns out to be needed in more cases, this function
+// should be moved to a more central location.
+Value *getOrCreateValue(const Expr *E, Environment &Env) {
+  Value *Val = nullptr;
+  if (E->isGLValue()) {
+StorageLocation *Loc = Env.getStorageLocationStrict(*E);
+if (!Loc) {
+  Loc = &Env.createStorageLocation(*E);
+  Env.setStorageLocationStrict(*E, *Loc);
+}
+Val = Env.getValue(*Loc);
+if (!Val) {
+  Val = Env.createValue(E->getType());
+  Env.setValue(*Loc, *Val);
+}
+  } else {
+Val = Env.getValueStrict(*E);
+if (!Val) {
+  Val = Env.createValue(E->getType());
+  Env.setValueStrict(*E, *Val);
+}
+  }
+  assert(Val != nullptr);
+
+  return Val;
+}
+
 void transferExpr(const Expr *E, const MatchFinder::MatchResult &M,
   LatticeTransferState &State) {
   const ASTContext &Context = *M.Context;
-  StorageLocation *Loc = State.Env.getStorageLocation(*E, SkipPast::None);
-  if (!Loc) {
-Loc = &State.Env.createStorageLocation(*E);
-State.Env.setStorageLocation(*E, *Loc);
-  }
-  Value *Val = State.Env.getValue(*Loc);
-  if (!Val) {
-Val = State.Env.createValue(Context.IntTy);
-State.Env.setValue(*Loc, *Val);
-  }
+
+  Value *Val = getOrCreateValue(E, State.Env);
+
   // The sign symbolic values have been initialized already.
   if (Val->getProperty("neg"))
 return;



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

[clang] 8bc8fc6 - [clang][dataflow] Use `Strict` accessors in TypeErasedDataflowAnalysis.cpp.

2023-05-21 Thread Martin Braenne via cfe-commits


Author: Martin Braenne
Date: 2023-05-22T06:51:12Z
New Revision: 8bc8fc65c9b20de6ffad63d7ce6734c145265cc2

URL: 
https://github.com/llvm/llvm-project/commit/8bc8fc65c9b20de6ffad63d7ce6734c145265cc2
DIFF: 
https://github.com/llvm/llvm-project/commit/8bc8fc65c9b20de6ffad63d7ce6734c145265cc2.diff

LOG: [clang][dataflow] Use `Strict` accessors in TypeErasedDataflowAnalysis.cpp.

This patch is part of the ongoing migration to strict handling of value 
categories (see https://discourse.llvm.org/t/70086 for details).

Depends On D150655

Reviewed By: sammccall, ymandel, xazax.hun

Differential Revision: https://reviews.llvm.org/D150656

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp 
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 2e9893e78fa7..4fc8f27ffc9b 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -123,13 +123,10 @@ class TerminatorVisitor
 private:
   TerminatorVisitorRetTy extendFlowCondition(const Expr &Cond) {
 // The terminator sub-expression might not be evaluated.
-if (Env.getStorageLocation(Cond, SkipPast::None) == nullptr)
+if (Env.getValueStrict(Cond) == nullptr)
   transfer(StmtToEnv, Cond, Env);
 
-// FIXME: The flow condition must be an r-value, so `SkipPast::None` should
-// suffice.
-auto *Val =
-cast_or_null(Env.getValue(Cond, SkipPast::Reference));
+auto *Val = cast_or_null(Env.getValueStrict(Cond));
 // Value merging depends on flow conditions from 
diff erent environments
 // being mutually exclusive -- that is, they cannot both be true in their
 // entirety (even if they may share some clauses). So, we need *some* value
@@ -303,18 +300,14 @@ builtinTransferInitializer(const CFGInitializer &Elt,
   auto *InitStmt = Init->getInit();
   assert(InitStmt != nullptr);
 
-  auto *InitStmtLoc = Env.getStorageLocation(*InitStmt, SkipPast::Reference);
-  if (InitStmtLoc == nullptr)
-return;
-
-  auto *InitStmtVal = Env.getValue(*InitStmtLoc);
-  if (InitStmtVal == nullptr)
-return;
-
   if (Member->getType()->isReferenceType()) {
+auto *InitStmtLoc = Env.getStorageLocationStrict(*InitStmt);
+if (InitStmtLoc == nullptr)
+  return;
+
 auto &MemberLoc = ThisLoc.getChild(*Member);
 Env.setValue(MemberLoc, Env.create(*InitStmtLoc));
-  } else {
+  } else if (auto *InitStmtVal = Env.getValueStrict(*InitStmt)) {
 auto &MemberLoc = ThisLoc.getChild(*Member);
 Env.setValue(MemberLoc, *InitStmtVal);
   }



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

[PATCH] D150656: [clang][dataflow] Use `Strict` accessors in TypeErasedDataflowAnalysis.cpp.

2023-05-21 Thread Martin Böhme via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
mboehme marked an inline comment as done.
Closed by commit rG8bc8fc65c9b2: [clang][dataflow] Use `Strict` accessors in 
TypeErasedDataflowAnalysis.cpp. (authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150656

Files:
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -123,13 +123,10 @@
 private:
   TerminatorVisitorRetTy extendFlowCondition(const Expr &Cond) {
 // The terminator sub-expression might not be evaluated.
-if (Env.getStorageLocation(Cond, SkipPast::None) == nullptr)
+if (Env.getValueStrict(Cond) == nullptr)
   transfer(StmtToEnv, Cond, Env);
 
-// FIXME: The flow condition must be an r-value, so `SkipPast::None` should
-// suffice.
-auto *Val =
-cast_or_null(Env.getValue(Cond, SkipPast::Reference));
+auto *Val = cast_or_null(Env.getValueStrict(Cond));
 // Value merging depends on flow conditions from different environments
 // being mutually exclusive -- that is, they cannot both be true in their
 // entirety (even if they may share some clauses). So, we need *some* value
@@ -303,18 +300,14 @@
   auto *InitStmt = Init->getInit();
   assert(InitStmt != nullptr);
 
-  auto *InitStmtLoc = Env.getStorageLocation(*InitStmt, SkipPast::Reference);
-  if (InitStmtLoc == nullptr)
-return;
-
-  auto *InitStmtVal = Env.getValue(*InitStmtLoc);
-  if (InitStmtVal == nullptr)
-return;
-
   if (Member->getType()->isReferenceType()) {
+auto *InitStmtLoc = Env.getStorageLocationStrict(*InitStmt);
+if (InitStmtLoc == nullptr)
+  return;
+
 auto &MemberLoc = ThisLoc.getChild(*Member);
 Env.setValue(MemberLoc, Env.create(*InitStmtLoc));
-  } else {
+  } else if (auto *InitStmtVal = Env.getValueStrict(*InitStmt)) {
 auto &MemberLoc = ThisLoc.getChild(*Member);
 Env.setValue(MemberLoc, *InitStmtVal);
   }


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -123,13 +123,10 @@
 private:
   TerminatorVisitorRetTy extendFlowCondition(const Expr &Cond) {
 // The terminator sub-expression might not be evaluated.
-if (Env.getStorageLocation(Cond, SkipPast::None) == nullptr)
+if (Env.getValueStrict(Cond) == nullptr)
   transfer(StmtToEnv, Cond, Env);
 
-// FIXME: The flow condition must be an r-value, so `SkipPast::None` should
-// suffice.
-auto *Val =
-cast_or_null(Env.getValue(Cond, SkipPast::Reference));
+auto *Val = cast_or_null(Env.getValueStrict(Cond));
 // Value merging depends on flow conditions from different environments
 // being mutually exclusive -- that is, they cannot both be true in their
 // entirety (even if they may share some clauses). So, we need *some* value
@@ -303,18 +300,14 @@
   auto *InitStmt = Init->getInit();
   assert(InitStmt != nullptr);
 
-  auto *InitStmtLoc = Env.getStorageLocation(*InitStmt, SkipPast::Reference);
-  if (InitStmtLoc == nullptr)
-return;
-
-  auto *InitStmtVal = Env.getValue(*InitStmtLoc);
-  if (InitStmtVal == nullptr)
-return;
-
   if (Member->getType()->isReferenceType()) {
+auto *InitStmtLoc = Env.getStorageLocationStrict(*InitStmt);
+if (InitStmtLoc == nullptr)
+  return;
+
 auto &MemberLoc = ThisLoc.getChild(*Member);
 Env.setValue(MemberLoc, Env.create(*InitStmtLoc));
-  } else {
+  } else if (auto *InitStmtVal = Env.getValueStrict(*InitStmt)) {
 auto &MemberLoc = ThisLoc.getChild(*Member);
 Env.setValue(MemberLoc, *InitStmtVal);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 3bc1ea5 - [clang][dataflow] Fix a bug in handling of `operator->` for optional checker.

2023-05-21 Thread Martin Braenne via cfe-commits


Author: Martin Braenne
Date: 2023-05-22T06:51:15Z
New Revision: 3bc1ea5b0ac90e04e7b935a5d964613f8fbad4bf

URL: 
https://github.com/llvm/llvm-project/commit/3bc1ea5b0ac90e04e7b935a5d964613f8fbad4bf
DIFF: 
https://github.com/llvm/llvm-project/commit/3bc1ea5b0ac90e04e7b935a5d964613f8fbad4bf.diff

LOG: [clang][dataflow] Fix a bug in handling of `operator->` for optional 
checker.

Prior to this patch, `operator->` was being handled like `operator*`: It was
associating a `Value` of type `T` with the expression result (where `T` is the
template argument of the `optional`). This is correct for `operator*`, which
returns a reference (of some flavor) to `T`, so that the result of the
`CXXOperatorCallExpr` is a glvalue of type `T`. However, `operator*` returns a
`T*`, so the result of the `CXXOperatorCallExpr` is a prvalue `T*`, which should
therefore be associated with `PointerValue` that in turn refers to a `T`.

I noticed this issue while working on the migration to strict handling of
value categories (see https://discourse.llvm.org/t/70086). The current behavior
also seems problematic more generally because it's plausible that the framework
may at some point introduce behavior that assumes an `Expr` of pointer type is
always associated with a `PointerValue`.

As it turns out, this patch fixes an existing FIXME in the test
`OptionalValueInitialization`.

Depends On D150657

Reviewed By: ymandel

Differential Revision: https://reviews.llvm.org/D150775

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Removed: 




diff  --git 
a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp 
b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 24a3682f438cb..7d30473ea5226 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -399,6 +399,18 @@ void transferUnwrapCall(const Expr *UnwrapExpr, const Expr 
*ObjectExpr,
   }
 }
 
+void transferArrowOpCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
+ LatticeTransferState &State) {
+  if (auto *OptionalVal =
+  getValueBehindPossiblePointer(*ObjectExpr, State.Env)) {
+if (auto *Loc = maybeInitializeOptionalValueMember(
+UnwrapExpr->getType()->getPointeeType(), *OptionalVal, State.Env)) 
{
+  State.Env.setValueStrict(*UnwrapExpr,
+   State.Env.create(*Loc));
+}
+  }
+}
+
 void transferMakeOptionalCall(const CallExpr *E,
   const MatchFinder::MatchResult &,
   LatticeTransferState &State) {
@@ -774,25 +786,22 @@ auto buildTransferMatchSwitch() {
 transferUnwrapCall(E, E->getImplicitObjectArgument(), State);
   })
 
-  // optional::operator*, optional::operator->
-  // FIXME: This does something slightly strange for `operator->`.
-  // `transferUnwrapCall()` may create a new value of type `T` for the
-  // `optional`, and it associates that value with `E`. In the case of
-  // `operator->`, `E` is a pointer. As a result, we associate an
-  // expression of pointer type with a storage location of non-pointer type
-  // `T`. This can confound other code that expects expressions of
-  // pointer type to be associated with `PointerValue`s, such as the
-  // centrally provided accessors `getImplicitObjectLocation()` and
-  // `getBaseObjectLocation()`, and this is the reason we need to use our
-  // own 'maybeSkipPointer()` and `getValueBehindPossiblePointer()` instead
-  // of these accessors.
-  .CaseOfCFGStmt(valueOperatorCall(std::nullopt),
+  // optional::operator*
+  .CaseOfCFGStmt(isOptionalOperatorCallWithName("*"),
[](const CallExpr *E,
   const MatchFinder::MatchResult &,
   LatticeTransferState &State) {
  transferUnwrapCall(E, E->getArg(0), State);
})
 
+  // optional::operator->
+  .CaseOfCFGStmt(isOptionalOperatorCallWithName("->"),
+   [](const CallExpr *E,
+  const MatchFinder::MatchResult &,
+  LatticeTransferState &State) {
+ transferArrowOpCall(E, E->getArg(0), State);
+   })
+
   // optional::has_value
   .CaseOfCFGStmt(
   isOptionalMemberCallWithName("has_value"),

diff  --git 
a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index a9d235

[PATCH] D150657: [clang][dataflow] Use `Strict` accessors in SignAnalysisTest.cpp.

2023-05-21 Thread Martin Böhme via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
mboehme marked an inline comment as done.
Closed by commit rG96b22e1c378a: [clang][dataflow] Use `Strict` accessors in 
SignAnalysisTest.cpp. (authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150657

Files:
  clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
@@ -114,12 +114,10 @@
 return {nullptr, {}, {}};
 
   // Value of the unary op.
-  auto *UnaryOpValue = State.Env.getValue(*UO, SkipPast::None);
+  auto *UnaryOpValue = State.Env.getValueStrict(*UO);
   if (!UnaryOpValue) {
-auto &Loc = State.Env.createStorageLocation(*UO);
-State.Env.setStorageLocation(*UO, Loc);
 UnaryOpValue = &State.Env.makeAtomicBoolValue();
-State.Env.setValue(Loc, *UnaryOpValue);
+State.Env.setValueStrict(*UO, *UnaryOpValue);
   }
 
   // Properties for the operand (sub expression).
@@ -133,22 +131,17 @@
 
 void transferBinary(const BinaryOperator *BO, const MatchFinder::MatchResult 
&M,
 LatticeTransferState &State) {
-  StorageLocation *Loc = State.Env.getStorageLocation(*BO, SkipPast::None);
-  if (!Loc) {
-Loc = &State.Env.createStorageLocation(*BO);
-State.Env.setStorageLocation(*BO, *Loc);
-  }
-  BoolValue *Comp = cast_or_null(State.Env.getValue(*Loc));
+  BoolValue *Comp = cast_or_null(State.Env.getValueStrict(*BO));
   if (!Comp) {
 Comp = &State.Env.makeAtomicBoolValue();
-State.Env.setValue(*Loc, *Comp);
+State.Env.setValueStrict(*BO, *Comp);
   }
 
   // FIXME Use this as well:
   // auto *NegatedComp = &State.Env.makeNot(*Comp);
 
-  auto *LHS = State.Env.getValue(*BO->getLHS(), SkipPast::None);
-  auto *RHS = State.Env.getValue(*BO->getRHS(), SkipPast::None);
+  auto *LHS = State.Env.getValueStrict(*BO->getLHS());
+  auto *RHS = State.Env.getValueStrict(*BO->getRHS());
 
   if (!LHS || !RHS)
 return;
@@ -244,19 +237,43 @@
   }
 }
 
+// Returns the `Value` associated with `E` (which may be either a prvalue or
+// glvalue). Creates a `Value` or `StorageLocation` as needed if `E` does not
+// have either of these associated with it yet.
+//
+// If this functionality turns out to be needed in more cases, this function
+// should be moved to a more central location.
+Value *getOrCreateValue(const Expr *E, Environment &Env) {
+  Value *Val = nullptr;
+  if (E->isGLValue()) {
+StorageLocation *Loc = Env.getStorageLocationStrict(*E);
+if (!Loc) {
+  Loc = &Env.createStorageLocation(*E);
+  Env.setStorageLocationStrict(*E, *Loc);
+}
+Val = Env.getValue(*Loc);
+if (!Val) {
+  Val = Env.createValue(E->getType());
+  Env.setValue(*Loc, *Val);
+}
+  } else {
+Val = Env.getValueStrict(*E);
+if (!Val) {
+  Val = Env.createValue(E->getType());
+  Env.setValueStrict(*E, *Val);
+}
+  }
+  assert(Val != nullptr);
+
+  return Val;
+}
+
 void transferExpr(const Expr *E, const MatchFinder::MatchResult &M,
   LatticeTransferState &State) {
   const ASTContext &Context = *M.Context;
-  StorageLocation *Loc = State.Env.getStorageLocation(*E, SkipPast::None);
-  if (!Loc) {
-Loc = &State.Env.createStorageLocation(*E);
-State.Env.setStorageLocation(*E, *Loc);
-  }
-  Value *Val = State.Env.getValue(*Loc);
-  if (!Val) {
-Val = State.Env.createValue(Context.IntTy);
-State.Env.setValue(*Loc, *Val);
-  }
+
+  Value *Val = getOrCreateValue(E, State.Env);
+
   // The sign symbolic values have been initialized already.
   if (Val->getProperty("neg"))
 return;


Index: clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
@@ -114,12 +114,10 @@
 return {nullptr, {}, {}};
 
   // Value of the unary op.
-  auto *UnaryOpValue = State.Env.getValue(*UO, SkipPast::None);
+  auto *UnaryOpValue = State.Env.getValueStrict(*UO);
   if (!UnaryOpValue) {
-auto &Loc = State.Env.createStorageLocation(*UO);
-State.Env.setStorageLocation(*UO, Loc);
 UnaryOpValue = &State.Env.makeAtomicBoolValue();
-State.Env.setValue(Loc, *UnaryOpValue);
+State.Env.setValueStrict(*UO, *UnaryOpValue);
   }
 
   // Properties for the operand (sub expression).
@@ -133,22 +131,17 @@
 
 void transferBinary(const BinaryOperator *BO, const MatchFinder::MatchResult &M,
 LatticeTransferState &State) {
-  StorageLocation *Loc = State.Env.getStorageLocation(*BO, SkipPast::None);
-  if (!Lo

[PATCH] D150775: [clang][dataflow] Fix a bug in handling of `operator->` for optional checker.

2023-05-21 Thread Martin Böhme via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
mboehme marked an inline comment as done.
Closed by commit rG3bc1ea5b0ac9: [clang][dataflow] Fix a bug in handling of 
`operator->` for optional checker. (authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150775

Files:
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.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
@@ -2764,9 +2764,6 @@
 }
 
 TEST_P(UncheckedOptionalAccessTest, OptionalValueInitialization) {
-  // FIXME: Fix when to initialize `value`. All unwrapping should be safe in
-  // this example, but `value` initialization is done multiple times during the
-  // fixpoint iterations and joining the environment won't correctly merge 
them.
   ExpectDiagnosticsFor(
   R"(
 #include "unchecked_optional_access_test.h"
@@ -2786,7 +2783,7 @@
   }
   // Now we merge the two values. UncheckedOptionalAccessModel::merge() 
will
   // throw away the "value" property.
-  foo->value(); // [[unsafe]]
+  foo->value();
 }
   )");
 }
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -399,6 +399,18 @@
   }
 }
 
+void transferArrowOpCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
+ LatticeTransferState &State) {
+  if (auto *OptionalVal =
+  getValueBehindPossiblePointer(*ObjectExpr, State.Env)) {
+if (auto *Loc = maybeInitializeOptionalValueMember(
+UnwrapExpr->getType()->getPointeeType(), *OptionalVal, State.Env)) 
{
+  State.Env.setValueStrict(*UnwrapExpr,
+   State.Env.create(*Loc));
+}
+  }
+}
+
 void transferMakeOptionalCall(const CallExpr *E,
   const MatchFinder::MatchResult &,
   LatticeTransferState &State) {
@@ -774,25 +786,22 @@
 transferUnwrapCall(E, E->getImplicitObjectArgument(), State);
   })
 
-  // optional::operator*, optional::operator->
-  // FIXME: This does something slightly strange for `operator->`.
-  // `transferUnwrapCall()` may create a new value of type `T` for the
-  // `optional`, and it associates that value with `E`. In the case of
-  // `operator->`, `E` is a pointer. As a result, we associate an
-  // expression of pointer type with a storage location of non-pointer type
-  // `T`. This can confound other code that expects expressions of
-  // pointer type to be associated with `PointerValue`s, such as the
-  // centrally provided accessors `getImplicitObjectLocation()` and
-  // `getBaseObjectLocation()`, and this is the reason we need to use our
-  // own 'maybeSkipPointer()` and `getValueBehindPossiblePointer()` instead
-  // of these accessors.
-  .CaseOfCFGStmt(valueOperatorCall(std::nullopt),
+  // optional::operator*
+  .CaseOfCFGStmt(isOptionalOperatorCallWithName("*"),
[](const CallExpr *E,
   const MatchFinder::MatchResult &,
   LatticeTransferState &State) {
  transferUnwrapCall(E, E->getArg(0), State);
})
 
+  // optional::operator->
+  .CaseOfCFGStmt(isOptionalOperatorCallWithName("->"),
+   [](const CallExpr *E,
+  const MatchFinder::MatchResult &,
+  LatticeTransferState &State) {
+ transferArrowOpCall(E, E->getArg(0), State);
+   })
+
   // optional::has_value
   .CaseOfCFGStmt(
   isOptionalMemberCallWithName("has_value"),


Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2764,9 +2764,6 @@
 }
 
 TEST_P(UncheckedOptionalAccessTest, OptionalValueInitialization) {
-  // FIXME: Fix when to initialize `value`. All unwrapping should be safe in
-  // this example, but `value` initializa

53 matches

Mail list logo