[PATCH] D131046: [clang-tools-extra]Replace find/insert with try_emplace

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang added inline comments.



Comment at: 
clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:140
+// This replacement is a duplicate of one suggested by another TU.
+if (!InsertPos.second) {
   return;

Previously, we had an additional check `It->second != SourceTU` here. I don't 
understand what this check was doing, but this does not seem to be a pure 
refactoring.

If this is still an NFC change, please update the commit message to explain why 
this condition was unneeded


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131046

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

[PATCH] D130807: [InstrProf] Add the skipprofile attribute

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130807

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

[PATCH] D130808: [InstrProf] Add new format for -fprofile-list=

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/include/clang/Basic/ProfileList.h:31-38
+  enum ExclusionType {
+/// Profiling is allowed.
+ALLOW,
+/// Profiling is skipped using the \p skipprofile attribute.
+SKIP,
+/// Profiling is forbidden using the \p noprofile attribute.
+FORBID,

It's more common in LLVM to use capitalized names for enum values, see 
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130808

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

[PATCH] D130130: [clang-tidy] Add an IgnoreMacros option to readability-avoid-const-params-in-decls

[Thomas Etter via Phabricator via cfe-commits]

thomasetter updated this revision to Diff 449565.
thomasetter added a comment.

Added a description to the release notes


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

https://reviews.llvm.org/D130130

Files:
  clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
  clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls-macros.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp
@@ -170,12 +170,17 @@
 void NF(const int* const*);
 void NF(alias_const_type);
 
-// Regression test for when the 'const' token is not in the code.
+// Regression tests involving macros, which are ignored by default.
 #define CONCAT(a, b) a##b
 void ConstNotVisible(CONCAT(cons, t) int i);
-// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: parameter 'i'
-// We warn, but we can't give a fix
-// CHECK-FIXES: void ConstNotVisible(CONCAT(cons, t) int i);
+
+#define CONST_INT_PARAM const int i
+void ConstInMacro(CONST_INT_PARAM);
+
+#define DECLARE_FUNCTION_WITH_ARG(x) struct InsideMacro{ x }
+DECLARE_FUNCTION_WITH_ARG(
+void member_function(const int i);
+);
 
 // Regression test. We should not warn (or crash) on lambda expressions
 auto lambda_with_name = [](const int n) {};
Index: clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls-macros.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls-macros.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s readability-avoid-const-params-in-decls %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-avoid-const-params-in-decls.IgnoreMacros, value: false}]}"
+
+// Regression tests involving macros
+#define CONCAT(a, b) a##b
+void ConstNotVisible(CONCAT(cons, t) int i);
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: parameter 'i'
+// We warn, but we can't give a fix
+// CHECK-FIXES: void ConstNotVisible(CONCAT(cons, t) int i);
+
+#define CONST_INT_PARAM const int i
+void ConstInMacro(CONST_INT_PARAM);
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: parameter 'i'
+// We warn, but we can't give a fix
+// CHECK-FIXES: void ConstInMacro(CONST_INT_PARAM);
+
+#define DECLARE_FUNCTION_WITH_ARG(x) struct InsideMacro{ x }
+DECLARE_FUNCTION_WITH_ARG(
+void member_function(const int i);
+);
+// CHECK-MESSAGES: :[[@LINE-2]]:26: warning: parameter 'i'
+// CHECK-FIXES: void member_function(int i);
Index: clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
@@ -15,3 +15,11 @@
 
   void f(const string);   // Bad: const is top level.
   void f(const string&);  // Good: const is not top level.
+
+Options
+---
+
+.. option:: IgnoreMacros
+
+   If set to `true`, the check will not give warnings inside macros. Default
+   is `true`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -96,6 +96,11 @@
 Improvements to clang-tidy
 --
 
+- The :doc:`readability-avoid-const-params-in-decls
+  ` check does not
+  warn about const value parameters in declarations inside macros anymore by
+  default.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
===
--- clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
+++ clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
@@ -20,13 +20,18 @@
 class AvoidConstParamsInDecls : public ClangTidyCheck {
 public:
   AvoidConstParamsInDecls(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
 
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   llvm::Optional getCheckTraversalKind() const override 

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: sammccall, tstellar, smeenai, beanz.
Herald added subscribers: carlosgalvezp, mgorny.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added projects: LLVM, clang-tools-extra.
Herald added a subscriber: cfe-commits.

This avoids having to specify the location of all individual tools.
In current builds, one may want to specify LLVM_TABLEGEN, CLANG_TABLEGEN,
LLDB_TABLEGEN, LLVM_CONFIG_PATH, CLANG_PSEUDO_GEN and
CLANG_TIDY_CONFUSABLE_CHARS_GEN; specifying just the base directory
containing all of them is much more convenient.

Note that HOST_EXECUTABLE_SUFFIX is different from CMAKE_EXECUTABLE_SUFFIX
(the suffix used for the cross compiled binaries). There's an upstream
request for CMake to provide such a variable, e.g.
CMAKE_HOST_EXECUTABLE_SUFFIX, in
https://gitlab.kitware.com/cmake/cmake/-/issues/17553, but it hasn't been
implemented yet.

This is a bit of an RFC, we could of course factorize setting of
HOST_EXECUTABLE_SUFFIX to a helper function/macro, and possibly
factorize the whole setup (including checking LLVM_USE_HOST_TOOLS and
calling build_native_tools) of clang-pseudo-gen and
clang-tidy-confusable-chars-gen, as it does end up as a fair bit of
boilerplate.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131052

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/pseudo/include/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/TableGen.cmake
  llvm/tools/llvm-config/CMakeLists.txt

Index: llvm/tools/llvm-config/CMakeLists.txt
===
--- llvm/tools/llvm-config/CMakeLists.txt
+++ llvm/tools/llvm-config/CMakeLists.txt
@@ -82,10 +82,23 @@
 # Add the dependency on the generation step.
 add_file_dependencies(${CMAKE_CURRENT_SOURCE_DIR}/llvm-config.cpp ${BUILDVARIABLES_OBJPATH})
 
-if(CMAKE_CROSSCOMPILING AND NOT LLVM_CONFIG_PATH)
-  build_native_tool(llvm-config LLVM_CONFIG_PATH)
-  set(LLVM_CONFIG_PATH "${LLVM_CONFIG_PATH}" CACHE STRING "")
+if(CMAKE_CROSSCOMPILING)
+  if (LLVM_NATIVE_TOOL_DIR AND NOT LLVM_CONFIG_PATH)
+if (CMAKE_HOST_WIN32)
+  set(HOST_EXECUTABLE_SUFFIX ".exe")
+else()
+  set(HOST_EXECUTABLE_SUFFIX "")
+endif()
+if (EXISTS "${LLVM_NATIVE_TOOL_DIR}/llvm-config${HOST_EXECUTABLE_SUFFIX}")
+  set(LLVM_CONFIG_PATH "${LLVM_NATIVE_TOOL_DIR}/llvm-config${HOST_EXECUTABLE_SUFFIX}")
+endif()
+  endif()
+
+  if (NOT LLVM_CONFIG_PATH)
+build_native_tool(llvm-config LLVM_CONFIG_PATH)
+set(LLVM_CONFIG_PATH "${LLVM_CONFIG_PATH}" CACHE STRING "")
 
-  add_custom_target(NativeLLVMConfig DEPENDS ${LLVM_CONFIG_PATH})
-  add_dependencies(llvm-config NativeLLVMConfig)
+add_custom_target(NativeLLVMConfig DEPENDS ${LLVM_CONFIG_PATH})
+add_dependencies(llvm-config NativeLLVMConfig)
+  endif()
 endif()
Index: llvm/cmake/modules/TableGen.cmake
===
--- llvm/cmake/modules/TableGen.cmake
+++ llvm/cmake/modules/TableGen.cmake
@@ -152,7 +152,18 @@
   add_llvm_executable(${target} DISABLE_LLVM_LINK_LLVM_DYLIB ${ARGN})
   set(LLVM_LINK_COMPONENTS ${${target}_OLD_LLVM_LINK_COMPONENTS})
 
-  set(${project}_TABLEGEN "${target}" CACHE
+  set(${project}_TABLEGEN_DEFAULT "${target}")
+  if (LLVM_NATIVE_TOOL_DIR)
+if (CMAKE_HOST_WIN32)
+  set(HOST_EXECUTABLE_SUFFIX ".exe")
+else()
+  set(HOST_EXECUTABLE_SUFFIX "")
+endif()
+if (EXISTS "${LLVM_NATIVE_TOOL_DIR}/${target}${HOST_EXECUTABLE_SUFFIX}")
+  set(${project}_TABLEGEN_DEFAULT "${LLVM_NATIVE_TOOL_DIR}/${target}${HOST_EXECUTABLE_SUFFIX}")
+endif()
+  endif()
+  set(${project}_TABLEGEN "${${project}_TABLEGEN_DEFAULT}" CACHE
   STRING "Native TableGen executable. Saves building one when cross-compiling.")
 
   # Effective tblgen executable to be used:
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -598,6 +598,7 @@
 if( WIN32 AND NOT CYGWIN )
   set(LLVM_LIT_TOOLS_DIR "" CACHE PATH "Path to GnuWin32 tools")
 endif()
+set(LLVM_NATIVE_TOOL_DIR "" CACHE PATH "Path to a directory containing prebuilt matching native tools (such as llvm-tblgen)")
 
 set(LLVM_INTEGRATED_CRT_ALLOC "" CACHE PATH "Replace the Windows CRT allocator with any of {rpmalloc|mimalloc|snmalloc}. Only works with /MT enabled.")
 if(LLVM_INTEGRATED_CRT_ALLOC)
Index: clang-tools-extra/pseudo/include/CMakeLists.txt
===
--- clang-tools-extra/pseudo/include/CMakeLists.txt
+++ clang-tools-extra/pseudo/include/CMakeLists.txt
@@ -1,7 +1,20 @@
 # The cxx.bnf grammar file
 set(cxx_bnf ${CMAKE_CURRENT_SOURCE_DIR}/../lib/cxx/cxx.bnf)
 
-set(CLANG_PSEUDO_GEN "clang-pseudo-gen" CACHE
+set(CLANG_PSEUDO_GEN_DEFAULT "clang-pseudo-gen")
+
+if (LLVM_NATIVE_TOOL_DIR)
+  if (CMAKE_HOST_WIN32)
+set(HOST_EXECUTABLE_SUFFIX ".ex

[PATCH] D131046: [clang-tools-extra]Replace find/insert with try_emplace

[ppenguin via Phabricator via cfe-commits]

prehistoric-penguin updated this revision to Diff 449569.
prehistoric-penguin added a comment.

Fix per comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131046

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp


Index: 
clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===
--- clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -135,12 +135,11 @@
 if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) {
   if (SourceTU) {
 auto &Replaces = DiagReplacements[*Entry];
-auto It = Replaces.find(R);
-if (It == Replaces.end())
-  Replaces.emplace(R, SourceTU);
-else if (It->second != SourceTU)
-  // This replacement is a duplicate of one suggested by another TU.
+auto InsertPos = Replaces.try_emplace(R, SourceTU);
+// This replacement is a duplicate of one suggested by another TU.
+if (!InsertPos.second && InsertPos.first->second != SourceTU) {
   return;
+}
   }
   GroupedReplacements[*Entry].push_back(R);
 } else if (Warned.insert(R.getFilePath()).second) {


Index: clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===
--- clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -135,12 +135,11 @@
 if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) {
   if (SourceTU) {
 auto &Replaces = DiagReplacements[*Entry];
-auto It = Replaces.find(R);
-if (It == Replaces.end())
-  Replaces.emplace(R, SourceTU);
-else if (It->second != SourceTU)
-  // This replacement is a duplicate of one suggested by another TU.
+auto InsertPos = Replaces.try_emplace(R, SourceTU);
+// This replacement is a duplicate of one suggested by another TU.
+if (!InsertPos.second && InsertPos.first->second != SourceTU) {
   return;
+}
   }
   GroupedReplacements[*Entry].push_back(R);
 } else if (Warned.insert(R.getFilePath()).second) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131046: [clang-tools-extra]Replace find/insert with try_emplace

[ppenguin via Phabricator via cfe-commits]

prehistoric-penguin updated this revision to Diff 449570.
prehistoric-penguin added a comment.

Update commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131046

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp


Index: 
clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===
--- clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -135,12 +135,11 @@
 if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) {
   if (SourceTU) {
 auto &Replaces = DiagReplacements[*Entry];
-auto It = Replaces.find(R);
-if (It == Replaces.end())
-  Replaces.emplace(R, SourceTU);
-else if (It->second != SourceTU)
-  // This replacement is a duplicate of one suggested by another TU.
+auto InsertPos = Replaces.try_emplace(R, SourceTU);
+// This replacement is a duplicate of one suggested by another TU.
+if (!InsertPos.second && InsertPos.first->second != SourceTU) {
   return;
+}
   }
   GroupedReplacements[*Entry].push_back(R);
 } else if (Warned.insert(R.getFilePath()).second) {


Index: clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===
--- clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -135,12 +135,11 @@
 if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) {
   if (SourceTU) {
 auto &Replaces = DiagReplacements[*Entry];
-auto It = Replaces.find(R);
-if (It == Replaces.end())
-  Replaces.emplace(R, SourceTU);
-else if (It->second != SourceTU)
-  // This replacement is a duplicate of one suggested by another TU.
+auto InsertPos = Replaces.try_emplace(R, SourceTU);
+// This replacement is a duplicate of one suggested by another TU.
+if (!InsertPos.second && InsertPos.first->second != SourceTU) {
   return;
+}
   }
   GroupedReplacements[*Entry].push_back(R);
 } else if (Warned.insert(R.getFilePath()).second) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131046: [clang-tools-extra]Replace find/insert with try_emplace

[ppenguin via Phabricator via cfe-commits]

prehistoric-penguin added inline comments.



Comment at: 
clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:140
+// This replacement is a duplicate of one suggested by another TU.
+if (!InsertPos.second) {
   return;

avogelsgesang wrote:
> Previously, we had an additional check `It->second != SourceTU` here. I don't 
> understand what this check was doing, but this does not seem to be a pure 
> refactoring.
> 
> If this is still an NFC change, please update the commit message to explain 
> why this condition was unneeded
Thanks! I have reviewed the change and fixed it, now the logic is aligned


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131046

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

[PATCH] D131046: [clang-tools-extra]Replace find/insert with try_emplace

[ppenguin via Phabricator via cfe-commits]

prehistoric-penguin updated this revision to Diff 449572.
prehistoric-penguin added a comment.

Format change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131046

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp


Index: 
clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===
--- clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -135,11 +135,9 @@
 if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) {
   if (SourceTU) {
 auto &Replaces = DiagReplacements[*Entry];
-auto It = Replaces.find(R);
-if (It == Replaces.end())
-  Replaces.emplace(R, SourceTU);
-else if (It->second != SourceTU)
-  // This replacement is a duplicate of one suggested by another TU.
+auto InsertPos = Replaces.try_emplace(R, SourceTU);
+// This replacement is a duplicate of one suggested by another TU.
+if (!InsertPos.second && InsertPos.first->second != SourceTU)
   return;
   }
   GroupedReplacements[*Entry].push_back(R);


Index: clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===
--- clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -135,11 +135,9 @@
 if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) {
   if (SourceTU) {
 auto &Replaces = DiagReplacements[*Entry];
-auto It = Replaces.find(R);
-if (It == Replaces.end())
-  Replaces.emplace(R, SourceTU);
-else if (It->second != SourceTU)
-  // This replacement is a duplicate of one suggested by another TU.
+auto InsertPos = Replaces.try_emplace(R, SourceTU);
+// This replacement is a duplicate of one suggested by another TU.
+if (!InsertPos.second && InsertPos.first->second != SourceTU)
   return;
   }
   GroupedReplacements[*Entry].push_back(R);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks a lot, lgtm!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128379

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

[PATCH] D131046: [NFC][clang-tools-extra]Replace find/insert with try_emplace

[Adrian Vogelsgesang via Phabricator via cfe-commits]

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

The change itself looks good.

Please look into the CI failures before landing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131046

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

[PATCH] D131057: [Sema] -Wformat: support C23 printf %b %B

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: aaron.ballman, dim, enh, erik.pilkington.
Herald added a subscriber: StephenFan.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Close #56885. %b %B are from WG14 N2630 and supported by glibc from 2.35 
onwards and latest Android bionic.

Like GCC, we don't test library support.

GCC 12 -Wformat -pedantic emits a warning:

> warning: ISO C17 does not support the ‘%b’ gnu_printf format [-Wformat=]

The behavior is not ported (and it's unclear to me how to implement it).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131057

Files:
  clang/include/clang/AST/FormatString.h
  clang/lib/AST/FormatString.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/test/Sema/format-strings-fixit.c
  clang/test/Sema/format-strings.c
  clang/test/SemaObjC/format-strings-objc.m

Index: clang/test/SemaObjC/format-strings-objc.m
===
--- clang/test/SemaObjC/format-strings-objc.m
+++ clang/test/SemaObjC/format-strings-objc.m
@@ -56,7 +56,7 @@
 
 void check_nslog(unsigned k) {
   NSLog(@"%d%%", k); // no-warning
-  NSLog(@"%s%lb%d", "unix", 10, 20); // expected-warning {{invalid conversion specifier 'b'}} expected-warning {{data argument not used by format string}}
+  NSLog(@"%s%lv%d", "unix", 10, 20); // expected-warning {{invalid conversion specifier 'v'}} expected-warning {{data argument not used by format string}}
 }
 
 // Check type validation
Index: clang/test/Sema/format-strings.c
===
--- clang/test/Sema/format-strings.c
+++ clang/test/Sema/format-strings.c
@@ -199,7 +199,7 @@
 
 void check_invalid_specifier(FILE* fp, char *buf)
 {
-  printf("%s%lb%d","unix",10,20); // expected-warning {{invalid conversion specifier 'b'}} expected-warning {{data argument not used by format string}}
+  printf("%s%lv%d","unix",10,20); // expected-warning {{invalid conversion specifier 'v'}} expected-warning {{data argument not used by format string}}
   fprintf(fp,"%%%l"); // expected-warning {{incomplete format specifier}}
   sprintf(buf,"%ld%d%d", 1, 2, 3); // expected-warning{{format specifies type 'long' but the argument has type 'int'}}
   snprintf(buf, 2, "%ld%;%d", 1, 2, 3); // expected-warning{{format specifies type 'long' but the argument has type 'int'}} expected-warning {{invalid conversion specifier ';'}} expected-warning {{data argument not used by format string}}
@@ -304,6 +304,7 @@
   printf("%qp", (void *)0); // expected-warning{{length modifier 'q' results in undefined behavior or no effect with 'p' conversion specifier}}
   printf("hhX %hhX", (unsigned char)10); // no-warning
   printf("llX %llX", (long long) 10); // no-warning
+  printf("%llb %llB", (long long) 10, (long long) 10); // no-warning
   // This is fine, because there is an implicit conversion to an int.
   printf("%d", (unsigned char) 10); // no-warning
   printf("%d", (long long) 10); // expected-warning{{format specifies type 'int' but the argument has type 'long long'}}
@@ -429,6 +430,7 @@
   // Bad flag usage
   printf("%#p", (void *) 0); // expected-warning{{flag '#' results in undefined behavior with 'p' conversion specifier}}
   printf("%0d", -1); // no-warning
+  printf("%0b%0B", -1u, -1u); // no-warning
   printf("%-p", (void *) 0); // no-warning
 #if !defined(__ANDROID__) && !defined(__Fuchsia__)
   printf("%#n", (int *) 0); // expected-warning{{flag '#' results in undefined behavior with 'n' conversion specifier}}
@@ -499,6 +501,7 @@
 void pr8641(void) {
   printf("%#x\n", 10);
   printf("%#X\n", 10);
+  printf("%#b %#B\n", 10, 10u);
 }
 
 void posix_extensions(void) {
@@ -508,6 +511,8 @@
   printf("%'i\n", 123456789); // no-warning
   printf("%'f\n", (float) 1.0); // no-warning
   printf("%'p\n", (void*) 0); // expected-warning{{results in undefined behavior with 'p' conversion specifier}}
+  printf("%'b\n", 123456789); // expected-warning{{results in undefined behavior with 'b' conversion specifier}}
+  printf("%'B\n", 123456789); // expected-warning{{results in undefined behavior with 'B' conversion specifier}}
 }
 
 // PR8486
@@ -540,6 +545,7 @@
   printf("%hhi", x); // no-warning
   printf("%c", x); // no-warning
   printf("%hhu", y); // no-warning
+  printf("%hhb %hhB", x, x); // no-warning
 }
 
 // Test suppression of individual warnings.
Index: clang/test/Sema/format-strings-fixit.c
===
--- clang/test/Sema/format-strings-fixit.c
+++ clang/test/Sema/format-strings-fixit.c
@@ -21,6 +21,7 @@
   printf("%s", (int) 123);
   printf("abc%0f", "testing testing 123");
   printf("%u", (long) -12);
+  printf("%b", (long) -13);
   printf("%p", 123);
   printf("%c\n", "x");
   printf("%c\n", 1.23);
@@ -179,6 +180,7 @@
 // CHECK: printf("%d", (int) 123);
 // CHECK: printf("abc%s", "testing te

[PATCH] D131046: [NFC][clang-tools-extra]Replace find/insert with try_emplace

[ppenguin via Phabricator via cfe-commits]

prehistoric-penguin updated this revision to Diff 449579.
prehistoric-penguin added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131046

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  llvm/include/llvm/ADT/Twine.h

Index: llvm/include/llvm/ADT/Twine.h
===
--- llvm/include/llvm/ADT/Twine.h
+++ llvm/include/llvm/ADT/Twine.h
@@ -291,9 +291,9 @@
 /// Construct from an std::string_view by converting it to a pointer and
 /// length.  This handles string_views on a pure API basis, and avoids
 /// storing one (or a pointer to one) inside a Twine, which avoids problems
-/// when mixing code compiled under various C++ standards.
-/*implicit*/ Twine(const std::string_view &Str)
-: LHSKind(PtrAndLengthKind) {
+/// when mixing code compiled under various C++ standards. Pass string_view
+/// by value is preferred.
+/*implicit*/ Twine(std::string_view Str) : LHSKind(PtrAndLengthKind) {
   LHS.ptrAndLength.ptr = Str.data();
   LHS.ptrAndLength.length = Str.length();
   assert(isValid() && "Invalid twine!");
Index: clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===
--- clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -37,9 +37,11 @@
 namespace clang {
 namespace replace {
 
-std::error_code collectReplacementsFromDirectory(
-const llvm::StringRef Directory, TUReplacements &TUs,
-TUReplacementFiles &TUFiles, clang::DiagnosticsEngine &Diagnostics) {
+template 
+std::error_code
+collectReplacementsFromDirectoryImpl(const llvm::StringRef Directory, T &TUs,
+ TUReplacementFiles &TUFiles,
+ clang::DiagnosticsEngine &Diagnostics) {
   using namespace llvm::sys::fs;
   using namespace llvm::sys::path;
 
@@ -67,7 +69,7 @@
 }
 
 yaml::Input YIn(Out.get()->getBuffer(), nullptr, &eatDiagnostics);
-tooling::TranslationUnitReplacements TU;
+typename T::value_type TU;
 YIn >> TU;
 if (YIn.error()) {
   // File doesn't appear to be a header change description. Ignore it.
@@ -82,47 +84,17 @@
 }
 
 std::error_code collectReplacementsFromDirectory(
-const llvm::StringRef Directory, TUDiagnostics &TUs,
+const llvm::StringRef Directory, TUReplacements &TUs,
 TUReplacementFiles &TUFiles, clang::DiagnosticsEngine &Diagnostics) {
-  using namespace llvm::sys::fs;
-  using namespace llvm::sys::path;
-
-  std::error_code ErrorCode;
-
-  for (recursive_directory_iterator I(Directory, ErrorCode), E;
-   I != E && !ErrorCode; I.increment(ErrorCode)) {
-if (filename(I->path())[0] == '.') {
-  // Indicate not to descend into directories beginning with '.'
-  I.no_push();
-  continue;
-}
-
-if (extension(I->path()) != ".yaml")
-  continue;
-
-TUFiles.push_back(I->path());
-
-ErrorOr> Out =
-MemoryBuffer::getFile(I->path());
-if (std::error_code BufferError = Out.getError()) {
-  errs() << "Error reading " << I->path() << ": " << BufferError.message()
- << "\n";
-  continue;
-}
-
-yaml::Input YIn(Out.get()->getBuffer(), nullptr, &eatDiagnostics);
-tooling::TranslationUnitDiagnostics TU;
-YIn >> TU;
-if (YIn.error()) {
-  // File doesn't appear to be a header change description. Ignore it.
-  continue;
-}
-
-// Only keep files that properly parse.
-TUs.push_back(TU);
-  }
+  return collectReplacementsFromDirectoryImpl(Directory, TUs, TUFiles,
+  Diagnostics);
+}
 
-  return ErrorCode;
+std::error_code collectReplacementsFromDirectory(
+const llvm::StringRef Directory, TUDiagnostics &TUs,
+TUReplacementFiles &TUFiles, clang::DiagnosticsEngine &Diagnostics) {
+  return collectReplacementsFromDirectoryImpl(Directory, TUs, TUFiles,
+  Diagnostics);
 }
 
 /// Extract replacements from collected TranslationUnitReplacements and
@@ -163,11 +135,9 @@
 if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) {
   if (SourceTU) {
 auto &Replaces = DiagReplacements[*Entry];
-auto It = Replaces.find(R);
-if (It == Replaces.end())
-  Replaces.emplace(R, SourceTU);
-else if (It->second != SourceTU)
-  // This replacement is a duplicate of one suggested by another TU.
+auto InsertPos = Replaces.try_emplace(R, SourceTU);
+// This replacement is a duplicate of one suggested by another TU.
+if (!InsertPos.second && InsertPos.first->secon

[PATCH] D131046: [NFC][clang-tools-extra]Replace find/insert with try_emplace

[ppenguin via Phabricator via cfe-commits]

prehistoric-penguin updated this revision to Diff 449581.
prehistoric-penguin added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131046

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp


Index: 
clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===
--- clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -168,11 +168,9 @@
 if (auto Entry = SM.getFileManager().getFile(Path)) {
   if (SourceTU) {
 auto &Replaces = DiagReplacements[*Entry];
-auto It = Replaces.find(R);
-if (It == Replaces.end())
-  Replaces.emplace(R, SourceTU);
-else if (It->second != SourceTU)
-  // This replacement is a duplicate of one suggested by another TU.
+auto InsertPos = Replaces.try_emplace(R, SourceTU);
+// This replacement is a duplicate of one suggested by another TU.
+if (!InsertPos.second && InsertPos.first->second != SourceTU)
   return;
   }
   GroupedReplacements[*Entry].push_back(R);


Index: clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===
--- clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -168,11 +168,9 @@
 if (auto Entry = SM.getFileManager().getFile(Path)) {
   if (SourceTU) {
 auto &Replaces = DiagReplacements[*Entry];
-auto It = Replaces.find(R);
-if (It == Replaces.end())
-  Replaces.emplace(R, SourceTU);
-else if (It->second != SourceTU)
-  // This replacement is a duplicate of one suggested by another TU.
+auto InsertPos = Replaces.try_emplace(R, SourceTU);
+// This replacement is a duplicate of one suggested by another TU.
+if (!InsertPos.second && InsertPos.first->second != SourceTU)
   return;
   }
   GroupedReplacements[*Entry].push_back(R);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 11e52ec - [clang] Short-circuit trivial constructors when evaluating arrays

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2022-08-03T10:38:15+02:00
New Revision: 11e52ecf74e942b738fa8496960bbb2f0a7373de

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

LOG: [clang] Short-circuit trivial constructors when evaluating arrays

VisitCXXConstructExpr() will later do something similar, but for large
arrays, we should try to do it once an not for every element.

Fixes #56774

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/AST/ExprConstant.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9dc08639dafb4..2c439a01d7216 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -54,6 +54,9 @@ Bug Fixes
   `Issue 56800 `_.
 - Fix `#56772 `_ - invalid
   destructor names were incorrectly accepted on template classes.
+- Improve compile-times with large dynamic array allocations with trivial
+  constructors. This fixes
+  `Issue 56774`_.
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 32a04ca9e3de3..efaccf52132b3 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -10833,6 +10833,9 @@ bool ArrayExprEvaluator::VisitCXXConstructExpr(const 
CXXConstructExpr *E,
 if (FinalSize == 0)
   return true;
 
+bool HasTrivialConstructor = CheckTrivialDefaultConstructor(
+Info, E->getExprLoc(), E->getConstructor(),
+E->requiresZeroInitialization());
 LValue ArrayElt = Subobject;
 ArrayElt.addArray(Info, E, CAT);
 // We do the whole initialization in two passes, first for just one 
element,
@@ -10856,19 +10859,26 @@ bool ArrayExprEvaluator::VisitCXXConstructExpr(const 
CXXConstructExpr *E,
 for (unsigned I = OldElts; I < N; ++I)
   Value->getArrayInitializedElt(I) = Filler;
 
-  // Initialize the elements.
-  for (unsigned I = OldElts; I < N; ++I) {
-if (!VisitCXXConstructExpr(E, ArrayElt,
-   &Value->getArrayInitializedElt(I),
-   CAT->getElementType()) ||
-!HandleLValueArrayAdjustment(Info, E, ArrayElt,
- CAT->getElementType(), 1))
-  return false;
-// When checking for const initilization any diagnostic is considered
-// an error.
-if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
-!Info.keepEvaluatingAfterFailure())
-  return false;
+  if (HasTrivialConstructor && N == FinalSize) {
+// If we have a trivial constructor, only evaluate it once and copy
+// the result into all the array elements.
+APValue &FirstResult = Value->getArrayInitializedElt(0);
+for (unsigned I = OldElts; I < FinalSize; ++I)
+  Value->getArrayInitializedElt(I) = FirstResult;
+  } else {
+for (unsigned I = OldElts; I < N; ++I) {
+  if (!VisitCXXConstructExpr(E, ArrayElt,
+ &Value->getArrayInitializedElt(I),
+ CAT->getElementType()) ||
+  !HandleLValueArrayAdjustment(Info, E, ArrayElt,
+   CAT->getElementType(), 1))
+return false;
+  // When checking for const initilization any diagnostic is considered
+  // an error.
+  if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+  !Info.keepEvaluatingAfterFailure())
+return false;
+}
   }
 }
 



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

[PATCH] D130791: [clang] Short-circuit trivial constexpr array constructors

[Timm Bäder via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG11e52ecf74e9: [clang] Short-circuit trivial constructors 
when evaluating arrays (authored by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D130791?vs=449182&id=449585#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130791

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ExprConstant.cpp


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10833,6 +10833,9 @@
 if (FinalSize == 0)
   return true;
 
+bool HasTrivialConstructor = CheckTrivialDefaultConstructor(
+Info, E->getExprLoc(), E->getConstructor(),
+E->requiresZeroInitialization());
 LValue ArrayElt = Subobject;
 ArrayElt.addArray(Info, E, CAT);
 // We do the whole initialization in two passes, first for just one 
element,
@@ -10856,19 +10859,26 @@
 for (unsigned I = OldElts; I < N; ++I)
   Value->getArrayInitializedElt(I) = Filler;
 
-  // Initialize the elements.
-  for (unsigned I = OldElts; I < N; ++I) {
-if (!VisitCXXConstructExpr(E, ArrayElt,
-   &Value->getArrayInitializedElt(I),
-   CAT->getElementType()) ||
-!HandleLValueArrayAdjustment(Info, E, ArrayElt,
- CAT->getElementType(), 1))
-  return false;
-// When checking for const initilization any diagnostic is considered
-// an error.
-if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
-!Info.keepEvaluatingAfterFailure())
-  return false;
+  if (HasTrivialConstructor && N == FinalSize) {
+// If we have a trivial constructor, only evaluate it once and copy
+// the result into all the array elements.
+APValue &FirstResult = Value->getArrayInitializedElt(0);
+for (unsigned I = OldElts; I < FinalSize; ++I)
+  Value->getArrayInitializedElt(I) = FirstResult;
+  } else {
+for (unsigned I = OldElts; I < N; ++I) {
+  if (!VisitCXXConstructExpr(E, ArrayElt,
+ &Value->getArrayInitializedElt(I),
+ CAT->getElementType()) ||
+  !HandleLValueArrayAdjustment(Info, E, ArrayElt,
+   CAT->getElementType(), 1))
+return false;
+  // When checking for const initilization any diagnostic is considered
+  // an error.
+  if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+  !Info.keepEvaluatingAfterFailure())
+return false;
+}
   }
 }
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -54,6 +54,9 @@
   `Issue 56800 `_.
 - Fix `#56772 `_ - invalid
   destructor names were incorrectly accepted on template classes.
+- Improve compile-times with large dynamic array allocations with trivial
+  constructors. This fixes
+  `Issue 56774`_.
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10833,6 +10833,9 @@
 if (FinalSize == 0)
   return true;
 
+bool HasTrivialConstructor = CheckTrivialDefaultConstructor(
+Info, E->getExprLoc(), E->getConstructor(),
+E->requiresZeroInitialization());
 LValue ArrayElt = Subobject;
 ArrayElt.addArray(Info, E, CAT);
 // We do the whole initialization in two passes, first for just one element,
@@ -10856,19 +10859,26 @@
 for (unsigned I = OldElts; I < N; ++I)
   Value->getArrayInitializedElt(I) = Filler;
 
-  // Initialize the elements.
-  for (unsigned I = OldElts; I < N; ++I) {
-if (!VisitCXXConstructExpr(E, ArrayElt,
-   &Value->getArrayInitializedElt(I),
-   CAT->getElementType()) ||
-!HandleLValueArrayAdjustment(Info, E, ArrayElt,
- CAT->getElementType(), 1))
-  return false;
-// When checking for const initilization any diagnostic is considered
-// an error.
-if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
-!Info.keepEvaluatingAfterFailure())
-  return false;
+  if (H

[PATCH] D130894: [clang] Print more information about failed static assertions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 449589.
tbaeder added a comment.

I just took the plunge and made `DiagnoseStaticAssertDetails()` take into 
account all static assertions, regardless of integer literals. It now also 
prints integers, booleans, chars and floats correctly.

For example:

  test.cpp:30:1: error: static assertion failed due to requirement 'inv(true) 
== inv(false)'
  static_assert(inv(true) == inv(false));
  ^ ~~~
  test.cpp:30:15: note: left-hand side of operator '==' evaluates to 'false'
  static_assert(inv(true) == inv(false));
^
  test.cpp:30:28: note: right-hand side of operator '==' evaluates to 'true'
  static_assert(inv(true) == inv(false));
 ^~

the output when both sides are printed is rather clumsy to me, and some things 
are not handled at all (e.g. failed static assertions when the toplevel 
expression is not a `BinaryOpertator`, e.g. `static_assert(!k)` - the user 
doesn't know what value `k` is, which is important in case `k` is of an integer 
type).

But again, this kind of begs people to suggest more and more improvements, but 
I think this is a good enough start :)


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

https://reviews.llvm.org/D130894

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/init-capture.cpp
  clang/test/Lexer/cxx1z-trigraphs.cpp
  clang/test/PCH/cxx-templates.cpp
  clang/test/Parser/objc-static-assert.mm
  clang/test/Sema/static-assert.c
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/static-assert-cxx17.cpp
  clang/test/SemaCXX/static-assert.cpp
  clang/test/SemaTemplate/instantiate-var-template.cpp
  clang/test/SemaTemplate/instantiation-dependence.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
@@ -184,7 +184,9 @@
 
 namespace Diags {
   struct A { int n, m; };
-  template struct X { static_assert(a.n == a.m); }; // expected-error {{static assertion failed due to requirement 'Diags::A{1, 2}.n == Diags::A{1, 2}.m'}}
+  template struct X { static_assert(a.n == a.m); }; // expected-error {{static assertion failed due to requirement 'Diags::A{1, 2}.n == Diags::A{1, 2}.m'}} \
+ // expected-note {{evaluates to '1'}} \
+ // expected-note {{evaluates to '2'}}
   template struct X; // expected-note {{in instantiation of template class 'Diags::X<{1, 2}>' requested here}}
 }
 
Index: clang/test/SemaTemplate/instantiation-dependence.cpp
===
--- clang/test/SemaTemplate/instantiation-dependence.cpp
+++ clang/test/SemaTemplate/instantiation-dependence.cpp
@@ -68,8 +68,10 @@
   struct D : B, C {};
 
   static_assert(trait::specialization == 0);
-  static_assert(trait::specialization == 1); // FIXME expected-error {{failed}}
-  static_assert(trait::specialization == 2); // FIXME expected-error {{failed}}
+  static_assert(trait::specialization == 1); // FIXME expected-error {{failed}} \
+// expected-note {{evaluates to '0'}}
+  static_assert(trait::specialization == 2); // FIXME expected-error {{failed}} \
+// expected-note {{evaluates to '0'}}
   static_assert(trait::specialization == 0); // FIXME-error {{ambiguous partial specialization}}
 }
 
Index: clang/test/SemaTemplate/instantiate-var-template.cpp
===
--- clang/test/SemaTemplate/instantiate-var-template.cpp
+++ clang/test/SemaTemplate/instantiate-var-template.cpp
@@ -31,7 +31,8 @@
   static_assert(b == 1, ""); // expected-note {{in instantiation of}} expected-error {{not an integral constant}}
 
   template void f() {
-static_assert(a == 0, ""); // expected-error {{static assertion failed due to requirement 'a == 0'}}
+static_assert(a == 0, ""); // expected-error {{static assertion failed due to requirement 'a == 0'}} \
+   // expected-note {{evaluates to '1'}}
   }
 }
 
Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -22,7 +22,8 @@
 T<2> t2;
 
 template struct S {
-static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static ass

[clang] f4b9c07 - [clang][NFC] Try to fix the docs build

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2022-08-03T10:52:02+02:00
New Revision: f4b9c0735e339b177d98fe69cf210eea00c0af62

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

LOG: [clang][NFC] Try to fix the docs build

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2c439a01d721..8e35bd567b65 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -56,7 +56,7 @@ Bug Fixes
   destructor names were incorrectly accepted on template classes.
 - Improve compile-times with large dynamic array allocations with trivial
   constructors. This fixes
-  `Issue 56774`_.
+  `Issue 56774 `_.
 
 Improvements to Clang's diagnostics
 ^^^



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

[PATCH] D130894: [clang] Print more information about failed static assertions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16574
+  if (BoolValue) {
+Str.push_back('t');
+Str.push_back('r');

I really hope there's a better way to do this that I just don't know about :)


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

https://reviews.llvm.org/D130894

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

[PATCH] D130754: [X86] Support ``-mindirect-branch-cs-prefix`` for call and jmp to indirect thunk

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6350
 
+  if (Args.hasArg(options::OPT_mindirect_branch_cs_prefix))
+CmdArgs.push_back("-mindirect-branch-cs-prefix");

MaskRay wrote:
> This is not needed with the TableGen CC1Option change.
Do you have an example how to change the CC1Option? Removing the code directly 
won't generate the expected module flag anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130754

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

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

In D125142#3505767 , @jfb wrote:

> I think the most relevant post from @rsmith is: 
> https://discourse.llvm.org/t/making-ftrivial-auto-var-init-zero-a-first-class-option/55143/40
>
> He has a prototype: https://reviews.llvm.org/D79249
> I assume he would like someone to pursue it further, it was a good faith 
> attempt at not just demanding. I'd played with it and it needed a few fixes, 
> but overall it was pretty complete. Does someone want to give it a go?

Is there a community interest to have such feature in Clang? I consider it 
quite useful.

If so, maybe I (we) could continue work on it, if @rsmith is busy.

You mentioned the need for few fixes or few bugs, do you remember which ones?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125142

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

[PATCH] D129131: Remove uses of llvm_shutdown

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added inline comments.



Comment at: polly/lib/External/isl/interface/extract_interface.cc:590
delete Clang;
-   llvm::llvm_shutdown();
 

Meinersbur wrote:
> This file is imported from the upstream project 
> (https://repo.or.cz/isl.git/blob/295cf91923295ca694e9e4433a0b818150421bee:/interface/extract_interface.cc#l590)
>  and this change will be lost when synchronizing with it. However, this file 
> is also not used within LLVM. I recommend to just keep as-is.
Thanks, will do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129131

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

[PATCH] D129131: Remove uses of llvm_shutdown

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle updated this revision to Diff 449609.
nhaehnle added a comment.

Address review comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129131

Files:
  bolt/tools/driver/llvm-bolt.cpp
  bolt/tools/merge-fdata/merge-fdata.cpp
  clang/include/clang/Frontend/CompilerInstance.h
  clang/tools/clang-repl/ClangRepl.cpp
  clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp
  clang/utils/TableGen/TableGen.cpp
  libclc/utils/prepare-builtins.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-test/lldb-test.cpp
  lldb/unittests/Utility/LogTest.cpp
  lldb/utils/TableGen/LLDBTableGen.cpp
  llvm/examples/BrainF/BrainFDriver.cpp
  llvm/examples/HowToUseJIT/HowToUseJIT.cpp
  llvm/include/llvm/PassRegistry.h
  llvm/include/llvm/Support/DynamicLibrary.h
  llvm/include/llvm/Support/InitLLVM.h
  llvm/lib/IR/Pass.cpp
  llvm/lib/Support/InitLLVM.cpp
  llvm/lib/Support/Unix/DynamicLibrary.inc
  llvm/lib/Support/Unix/Signals.inc
  llvm/lib/Support/Windows/DynamicLibrary.inc
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
  llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
  llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
  llvm/utils/KillTheDoctor/KillTheDoctor.cpp
  mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
  polly/lib/External/isl/interface/extract_interface.cc

Index: polly/lib/External/isl/interface/extract_interface.cc
===
--- polly/lib/External/isl/interface/extract_interface.cc
+++ polly/lib/External/isl/interface/extract_interface.cc
@@ -48,7 +48,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
Index: mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
===
--- mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
+++ mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
@@ -62,7 +62,6 @@
 }
 
 int main(int argc, char **argv) {
-  llvm::llvm_shutdown_obj x;
   registerPassManagerCLOptions();
 
   llvm::InitLLVM y(argc, argv);
Index: llvm/utils/KillTheDoctor/KillTheDoctor.cpp
===
--- llvm/utils/KillTheDoctor/KillTheDoctor.cpp
+++ llvm/utils/KillTheDoctor/KillTheDoctor.cpp
@@ -36,7 +36,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/CommandLine.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
@@ -297,7 +296,6 @@
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0]);
   PrettyStackTraceProgram X(argc, argv);
-  llvm_shutdown_obj Y;  // Call llvm_shutdown() on exit.
 
   ToolName = argv[0];
 
Index: llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
===
--- llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
+++ llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
@@ -9,7 +9,6 @@
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Config/config.h"
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "gtest/gtest.h"
 
@@ -59,7 +58,6 @@
 TEST(DynamicLibrary, Overload) {
   {
 std::string Err;
-llvm_shutdown_obj Shutdown;
 DynamicLibrary DL =
 DynamicLibrary::getPermanentLibrary(LibPath().c_str(), &Err);
 EXPECT_TRUE(DL.isValid());
@@ -109,9 +107,6 @@
   }
   EXPECT_TRUE(FuncPtr(DynamicLibrary::SearchForAddressOfSymbol(
   "TestA")) == nullptr);
-
-  // Check serach ordering is reset to default after call to llvm_shutdown
-  EXPECT_EQ(DynamicLibrary::SearchOrder, DynamicLibrary::SO_Linker);
 }
 
 TEST(DynamicLibrary, Shutdown) {
@@ -119,7 +114,6 @@
   std::vector Order;
   {
 std::string Err;
-llvm_shutdown_obj Shutdown;
 DynamicLibrary DL =
 DynamicLibrary::getPermanentLibrary(LibPath(A).c_str(), &Err);
 EXPECT_TRUE(DL.isValid());
Index: llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
===
--- llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
+++ llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
@@ -14,7 +14,6 @@
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/DynamicLibrary.h"
-#include "llvm/Support/ManagedStatic.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
@@ -22,9 +21,6 @@
 namespace {
 
 class ExecutionEngineTest : public testing::Test {
-private:
-  llvm_shutdown_obj Y; // Call llvm_shutdown() on exit.
-
 protected:
   ExecutionEngineTest() {
 auto Owner = std::make_unique("", Context);
Index: llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp

[PATCH] D131046: [NFC][clang-tools-extra]Replace find/insert with try_emplace

[ppenguin via Phabricator via cfe-commits]

prehistoric-penguin added a comment.

CI failed since LLVM are still using c++14 to build

  CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/clang++ 
-DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-I/var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/tools/extra/clang-apply-replacements
 
-I/var/lib/buildkite-agent/builds/llvm-project/clang-tools-extra/clang-apply-replacements
 -I/var/lib/buildkite-agent/builds/llvm-project/clang/include 
-I/var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/include 
-I/var/lib/buildkite-agent/builds/llvm-project/build/include 
-I/var/lib/buildkite-agent/builds/llvm-project/llvm/include 
-I/var/lib/buildkite-agent/builds/llvm-project/clang-tools-extra/clang-apply-replacements/include
 -gmlt -fPIC -fvisibility-inlines-hidden -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
-Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
-fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 
-DNDEBUG  -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT 
tools/clang/tools/extra/clang-apply-replacements/CMakeFiles/obj.clangApplyReplacements.dir/lib/Tooling/ApplyReplacements.cpp.o
 -MF 
tools/clang/tools/extra/clang-apply-replacements/CMakeFiles/obj.clangApplyReplacements.dir/lib/Tooling/ApplyReplacements.cpp.o.d
 -o 
tools/clang/tools/extra/clang-apply-replacements/CMakeFiles/obj.clangApplyReplacements.dir/lib/Tooling/ApplyReplacements.cpp.o
 -c 
/var/lib/buildkite-agent/builds/llvm-project/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  
  
/var/lib/buildkite-agent/builds/llvm-project/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:171:35:
 error: no member named 'try_emplace' in 'std::map'
  
  auto InsertPos = Replaces.try_emplace(R, SourceTU);

We may close this temporarily and reopen it after the c++17 flag is ready.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131046

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

[PATCH] D131062: [docs] Add "C++20 Modules"

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu created this revision.
ChuanqiXu added reviewers: iains, urnathan, erichkeane, aaron.ballman, 
tahonermann, ilya-biryukov, Mordante, tschuett, philnik.
ChuanqiXu added projects: clang-modules, clang-language-wg.
Herald added a subscriber: arphaman.
Herald added a project: All.
ChuanqiXu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We get some C++20 module things done in clang15.x. But we lack a user 
documentation for it. The implementation of c++20 modules share a big part of 
codes with clang modules. But they have very different semantics and user 
interfaces, so I think it is necessary to add a document for C++20 modules. 
Previously, there were also some people ask the document for C++20 Modules and 
I couldn't offer that time.

I wish we could land this before September and I can backport this to 15.x. 
This implies that even we solve any bug during the reviewing process. We 
shouldn't edit the document in time. We should land it and edit it.

Tested with grammarly  and https://overbits.herokuapp.com/rsteditor/.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131062

Files:
  clang/docs/CPlusPlus20Modules.rst
  clang/docs/index.rst

Index: clang/docs/index.rst
===
--- clang/docs/index.rst
+++ clang/docs/index.rst
@@ -40,6 +40,7 @@
SafeStack
ShadowCallStack
SourceBasedCodeCoverage
+   CPlusPlus20Modules
Modules
MSVCCompatibility
MisExpect
Index: clang/docs/CPlusPlus20Modules.rst
===
--- /dev/null
+++ clang/docs/CPlusPlus20Modules.rst
@@ -0,0 +1,605 @@
+=
+C++20 Modules
+=
+
+.. contents::
+   :local:
+
+Introduction
+
+
+Modules have a lot of meanings. For the users of clang compiler, modules may
+refer to `Objective-C Modules`, `Clang C++ Modules` (or `Clang Header Modules`,
+etc) and C++20 modules. The implementation of all kinds of the modules in clang 
+share a big part of codes. But from the perspective of users, their semantics and
+command line interfaces are very different. So it should be helpful for the users
+to introduce how to use C++20 modules.
+
+There is already a detailed document about clang modules Modules_, it
+should be helpful to read Modules_ if you want to know more about the general
+idea of modules. But due to the C++20 modules having very different semantics, it
+might be more friendly for users who care about C++20 modules only to create a
+new page.
+
+Although the term `modules` has a unique meaning in C++20 Language Specification,
+when people talk about C++20 modules, they may refer to another C++20 feature:
+header units. So this document would try to cover header units too.
+
+C++20 Modules
+=
+
+This document was intended to be pure manual. But it should be helpful to
+introduce some language background here for readers who are not familiar with
+the new language feature. This document is not intended to be a language
+tutorial. The document would only introduce concepts about the the
+structure and building of the project.
+
+Background and terminology
+--
+
+Modules
+~~~
+
+In this document, the term `Modules`/`modules` refers to C++20 modules
+feature if it is not decorated by `clang`.
+
+Clang Modules
+~
+
+In this document, the term `Clang Modules`/`clang modules` refer to clang
+c++ modules extension. It is also known as `clang header modules` and
+`clang module map modules` or `clang c++ modules`.
+
+Module and module unit
+~~
+
+A module consists of one or multiple module units. A module unit is a special
+translation unit. Every module unit should have a module declaration. The syntax
+of the module declaration is:
+
+.. code-block:: c++
+
+  [export] module module_name[:partition_name];
+
+Things in ``[]`` means optional. The syntax of ``module_name`` and ``partition_name``
+in regex form should be ``[a-zA-Z_][a-zA-Z_0-9.]*``. The dot ``.`` in the name has
+no special meaning.
+
+In this document, module units are classified into:
+
+* Primary module interface unit.
+
+* Module implementation unit.
+
+* Module partition interface unit.
+
+* Module partition implementation unit.
+
+A primary module interface unit is a module unit whose module declaration is
+`export module module_name;`. The `module_name` here denotes the name of the
+module. A module should have one and only primary module interface unit.
+
+A module implementation unit is a module unit whose module declaration is
+`module module_name;`. A module could have multiple module implementation
+units with the same declaration.
+
+A module partition interface unit is a module unit whose module declaration is
+`export module module_name:partition_name;`. The `partition_name` should be
+unique to the module.
+
+A module partition implementation unit is a 

[PATCH] D131062: [docs] Add "C++20 Modules"

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/docs/CPlusPlus20Modules.rst:377-393
+  │ │  
  ││
+  ├─  frontend ─┼─  middle end 
──┼─── backend ┤
+  │ │  
  ││
+  └─── parsing  sema  codegen ──┴─── optimizations ─── IPO  ─── 
optimizations ─── codegen ───┴─── optimizations ─── codegen ──┘
+   
   
+  
┌───┐
+  │
   │

I'm not sure if this could be rendered properly, if there is any suggestion?



Comment at: clang/docs/CPlusPlus20Modules.rst:585-587
+But according to [cpp.import](http://eel.is/c++draft/cpp.import#5),
+the import of header unit should export undefinition too.
+So the expected result of using header units here should be ``"Not defined."``.

@iains maybe we need to double check on this. I also filed an issue: 
https://github.com/llvm/llvm-project/issues/56899. My reading for 
`[cpp.import]p5` is `the point of undefinition` is also a macro definition, so 
we should emit it. But I maybe wrong. 

If I am wrong, I wish we could get an example to show differences (If any)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131062

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

@nikic @thakis I fixed this issue in https://reviews.llvm.org/D131063 and it 
can be built with CXX_STANDARD=17 and OSX_DEPLOYMENT_TARGET=10.11.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D129973: [clang] Pass FoundDecl to DeclRefExpr creator for operator overloads

[joanahalili via Phabricator via cfe-commits]

joanahalili added a comment.

Heads up: This commit causes clang crashes on our end. We are currently working 
on a reproducer and will post it as soon as its ready.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129973

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

[PATCH] D130630: [clang-tidy] Add readability-nested-ifs check

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 449621.
njames93 added a comment.

Handle cases where the fix is incorrect due to operator precedence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130630

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/NestedIfsCheck.cpp
  clang-tools-extra/clang-tidy/readability/NestedIfsCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/nested-ifs.rst
  clang-tools-extra/test/clang-tidy/checkers/readability/nested-ifs-cxx17.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability/nested-ifs.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/nested-ifs.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/nested-ifs.cpp
@@ -0,0 +1,116 @@
+// RUN: %check_clang_tidy %s readability-nested-ifs %t
+
+void foo();
+void bar();
+
+bool cond(int X = 0);
+
+void good() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Nested ifs can be combined
+  if (cond()) {
+if (cond(1)) {
+  foo();
+  bar();
+}
+  } // End
+  //  CHECK-FIXES: if (cond() && cond(1))
+  // CHECK-FIXES-NEXT: {
+  // CHECK-FIXES-NEXT:   foo();
+  // CHECK-FIXES-NEXT:   bar();
+  // CHECK-FIXES-NEXT: }
+  // CHECK-FIXES-NEXT: // End
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: Nested ifs can be combined
+  // CHECK-MESSAGES: :[[@LINE+4]]:7: warning: Nested ifs can be combined
+  if (cond()) {
+if (cond(1)) {
+  foo();
+  if (cond(2)) {
+if (cond(3)) {
+  bar();
+}
+  }
+}
+  } // End
+
+  //  CHECK-FIXES:  if (cond() && cond(1))
+  // CHECK-FIXES-NEXT:{
+  // CHECK-FIXES-NEXT:  foo();
+  // CHECK-FIXES-NEXT:  if (cond(2) && cond(3))
+  // CHECK-FIXES-NEXT:{
+  // CHECK-FIXES-NEXT:  bar();
+  // CHECK-FIXES-NEXT:}
+  // CHECK-FIXES-NEXT: {{ }}
+  // CHECK-FIXES-NEXT:}
+  // CHECK-FIXES-NEXT:  // End
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:5: warning: Nested ifs can be combined
+  if (bool B = cond()) {
+if (cond(1)) {
+  if (cond(2)) {
+foo();
+  }
+}
+  } // End
+
+  //  CHECK-FIXES:  if (bool B = cond()) {
+  // CHECK-FIXES-NEXT:if (cond(1) && cond(2))
+  // CHECK-FIXES-NEXT:{
+  // CHECK-FIXES-NEXT:  foo();
+  // CHECK-FIXES-NEXT:}
+  // CHECK-FIXES-NEXT: {{}}
+  // CHECK-FIXES-NEXT:  } // End
+
+
+  // Ensure parens are inserted when needed.
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Nested ifs can be combined
+  if (cond() || cond(1)) if (cond(2)) {}
+  // CHECK-FIXES: if ((cond() || cond(1)) && cond(2)) {}
+  
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Nested ifs can be combined
+  if (cond()) if (cond(1), cond(2)) {}
+  // CHECK-FIXES: if (cond() && (cond(1), cond(2))) {}
+
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Nested ifs can be combined
+  if (cond() || cond(1)) if (cond(2) ? cond(3) : cond(4)) {}
+  // CHECK-FIXES: if ((cond() || cond(1)) && (cond(2) ? cond(3) : cond(4))) {}
+}
+
+void bad() {
+  // Condition variable on either nesting can't be converted.
+  if (bool B = cond())
+if (cond())
+  foo();
+  if (cond()) {
+if (bool B = cond())
+  bar();
+  }
+
+  // Can only have a single if statement as the body of the outer if.
+  if (cond()) {
+foo();
+if (cond())
+  bar();
+  }
+  if (cond()) {
+if (cond())
+  foo();
+bar();
+  }
+
+  // Can't have an else statement on either if.
+  if (cond()) {
+if (cond())
+  foo();
+else
+  bar();
+  }
+
+  if (cond()) {
+if (cond())
+  foo();
+  } else {
+bar();
+  }
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/nested-ifs-cxx17.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/nested-ifs-cxx17.cpp
@@ -0,0 +1,74 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s readability-nested-ifs %t -- -- -fno-delayed-template-parsing
+
+// Ensure the check handles if initializers and constexpr if statements
+// correctly.
+
+void foo();
+void bar();
+
+bool cond(int X = 0);
+
+void good() {
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Nested ifs can be combined
+  if (bool X = cond(); X) {
+if (cond()) {
+  if (cond(1))
+bar();
+}
+  } // End
+  // CHECK-FIXES: if (bool X = cond(); X && cond() && cond(1))
+  // CHECK-FIXES-NEXT: {{}}
+  // CHECK-FIXES-NEXT: {{  }}
+  // CHECK-FIXES-NEXT: bar();
+  // CHECK-FIXES-NEXT: {{}}
+  // CHECK-FIXES-NEXT: // End
+
+  // The initializer doesn't have to be a variable declaration.
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warni

[PATCH] D130894: [clang] Print more information about failed static assertions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 449625.
tbaeder added a comment.

Whatever, I had time and you're still asleep, so I updated the output:

  test.cpp:30:1: error: static assertion failed due to requirement 'inv(true) 
== inv(false)'
  static_assert(inv(true) == inv(false));
  ^ ~~~
  test.cpp:30:25: note: expression evaluates to false == true
  static_assert(inv(true) == inv(false));
~~^
  1 error generated.


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

https://reviews.llvm.org/D130894

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/init-capture.cpp
  clang/test/Lexer/cxx1z-trigraphs.cpp
  clang/test/PCH/cxx-templates.cpp
  clang/test/Parser/objc-static-assert.mm
  clang/test/Sema/static-assert.c
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/static-assert-cxx17.cpp
  clang/test/SemaCXX/static-assert.cpp
  clang/test/SemaTemplate/instantiate-var-template.cpp
  clang/test/SemaTemplate/instantiation-dependence.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
@@ -184,7 +184,8 @@
 
 namespace Diags {
   struct A { int n, m; };
-  template struct X { static_assert(a.n == a.m); }; // expected-error {{static assertion failed due to requirement 'Diags::A{1, 2}.n == Diags::A{1, 2}.m'}}
+  template struct X { static_assert(a.n == a.m); }; // expected-error {{static assertion failed due to requirement 'Diags::A{1, 2}.n == Diags::A{1, 2}.m'}} \
+ // expected-note {{evaluates to 1 == 2}}
   template struct X; // expected-note {{in instantiation of template class 'Diags::X<{1, 2}>' requested here}}
 }
 
Index: clang/test/SemaTemplate/instantiation-dependence.cpp
===
--- clang/test/SemaTemplate/instantiation-dependence.cpp
+++ clang/test/SemaTemplate/instantiation-dependence.cpp
@@ -68,8 +68,10 @@
   struct D : B, C {};
 
   static_assert(trait::specialization == 0);
-  static_assert(trait::specialization == 1); // FIXME expected-error {{failed}}
-  static_assert(trait::specialization == 2); // FIXME expected-error {{failed}}
+  static_assert(trait::specialization == 1); // FIXME expected-error {{failed}} \
+// expected-note {{evaluates to 0}}
+  static_assert(trait::specialization == 2); // FIXME expected-error {{failed}} \
+// expected-note {{evaluates to 0}}
   static_assert(trait::specialization == 0); // FIXME-error {{ambiguous partial specialization}}
 }
 
Index: clang/test/SemaTemplate/instantiate-var-template.cpp
===
--- clang/test/SemaTemplate/instantiate-var-template.cpp
+++ clang/test/SemaTemplate/instantiate-var-template.cpp
@@ -31,7 +31,8 @@
   static_assert(b == 1, ""); // expected-note {{in instantiation of}} expected-error {{not an integral constant}}
 
   template void f() {
-static_assert(a == 0, ""); // expected-error {{static assertion failed due to requirement 'a == 0'}}
+static_assert(a == 0, ""); // expected-error {{static assertion failed due to requirement 'a == 0'}} \
+   // expected-note {{evaluates to 1}}
   }
 }
 
Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -22,7 +22,8 @@
 T<2> t2;
 
 template struct S {
-static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static assertion failed due to requirement 'sizeof(char) > sizeof(char)': Type not big enough!}}
+static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static assertion failed due to requirement 'sizeof(char) > sizeof(char)': Type not big enough!}} \
+ // expected-note {{1 > 1}}
 };
 
 S s1; // expected-note {{in instantiation of template class 'S' requested here}}
@@ -215,3 +216,32 @@
 static_assert(constexprNotBool, "message"); // expected-error {{value of type 'const NotBool' is not contextually convertible to 'bool'}}
 
 static_assert(1 , "") // expected-error {{expected ';' after 'static_assert'}}
+
+
+namespace Diagnostics {
+  /// No notes for literals.
+  static_assert(false, ""); // expected-error {{failed}}
+  stat

[PATCH] D131065: [clang][dataflow] Store DeclContext of block being analysed in Environment if available.

[weiyi via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131065

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -154,7 +154,7 @@
 : DACtx(&DACtx), FlowConditionToken(&DACtx.makeFlowConditionToken()) {}
 
 Environment::Environment(const Environment &Other)
-: DACtx(Other.DACtx), DeclToLoc(Other.DeclToLoc),
+: DACtx(Other.DACtx), DeclCtx(Other.DeclCtx), DeclToLoc(Other.DeclToLoc),
   ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
   MemberLocToStruct(Other.MemberLocToStruct),
   FlowConditionToken(&DACtx->forkFlowCondition(*Other.FlowConditionToken)) 
{
@@ -167,9 +167,11 @@
 }
 
 Environment::Environment(DataflowAnalysisContext &DACtx,
- const DeclContext &DeclCtx)
+ const DeclContext &DeclCtxArg)
 : Environment(DACtx) {
-  if (const auto *FuncDecl = dyn_cast(&DeclCtx)) {
+  DeclCtx = &DeclCtxArg;
+
+  if (const auto *FuncDecl = dyn_cast(DeclCtx)) {
 assert(FuncDecl->getBody() != nullptr);
 initGlobalVars(*FuncDecl->getBody(), *this);
 for (const auto *ParamDecl : FuncDecl->parameters()) {
@@ -181,7 +183,7 @@
 }
   }
 
-  if (const auto *MethodDecl = dyn_cast(&DeclCtx)) {
+  if (const auto *MethodDecl = dyn_cast(DeclCtx)) {
 auto *Parent = MethodDecl->getParent();
 assert(Parent != nullptr);
 if (Parent->isLambda())
@@ -268,12 +270,14 @@
 
 LatticeJoinEffect Environment::join(const Environment &Other,
 Environment::ValueModel &Model) {
-  assert(DACtx == Other.DACtx);
+  assert(DACtx == Other.DACtx && DeclCtx == Other.DeclCtx);
 
   auto Effect = LatticeJoinEffect::Unchanged;
 
   Environment JoinedEnv(*DACtx);
 
+  JoinedEnv.DeclCtx = DeclCtx;
+
   JoinedEnv.DeclToLoc = intersectDenseMaps(DeclToLoc, Other.DeclToLoc);
   if (DeclToLoc.size() != JoinedEnv.DeclToLoc.size())
 Effect = LatticeJoinEffect::Changed;
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -167,6 +167,10 @@
   LatticeJoinEffect join(const Environment &Other,
  Environment::ValueModel &Model);
 
+  /// Returns the `DeclCtx` of the block being analysed if provided, otherwise
+  /// returns nullptr.
+  const DeclContext *getDeclCtx() { return DeclCtx; }
+
   // FIXME: Rename `createOrGetStorageLocation` to 
`getOrCreateStorageLocation`,
   // `getStableStorageLocation`, or something more appropriate.
 
@@ -363,6 +367,9 @@
   // `DACtx` is not null and not owned by this object.
   DataflowAnalysisContext *DACtx;
 
+  // `DeclCtx` of the block being analysed if provided.
+  const DeclContext *DeclCtx;
+
   // Maps from program declarations and statements to storage locations that 
are
   // assigned to them. Unlike the maps in `DataflowAnalysisContext`, these
   // include only storage locations that are in scope for a particular basic


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -154,7 +154,7 @@
 : DACtx(&DACtx), FlowConditionToken(&DACtx.makeFlowConditionToken()) {}
 
 Environment::Environment(const Environment &Other)
-: DACtx(Other.DACtx), DeclToLoc(Other.DeclToLoc),
+: DACtx(Other.DACtx), DeclCtx(Other.DeclCtx), DeclToLoc(Other.DeclToLoc),
   ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
   MemberLocToStruct(Other.MemberLocToStruct),
   FlowConditionToken(&DACtx->forkFlowCondition(*Other.FlowConditionToken)) {
@@ -167,9 +167,11 @@
 }
 
 Environment::Environment(DataflowAnalysisContext &DACtx,
- const DeclContext &DeclCtx)
+ const DeclContext &DeclCtxArg)
 : Environment(DACtx) {
-  if (const auto *FuncDecl = dyn_cast(&DeclCtx)) {
+  DeclCtx = &DeclCtxArg;
+
+  if (const auto *FuncDecl = dyn_cast(DeclCtx)) {
 assert(FuncDecl->getBody() != nullptr);
 initGlobalVars(*FuncDecl->getBody(), *this);
 for (const auto *ParamDecl : FuncDecl->parameters()) {
@@ -181,7 +183,7 @@
 }
   }
 
-  if (const auto *MethodDecl = dyn_cast(&DeclCtx)) {
+  if (con

[PATCH] D131066: [clangd] Add option to disable inlay hints for init lists when building array.

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: sammccall, nridge.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: All.
njames93 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Inlay hints can be pretty annoying when using an initializer list to build an 
array.

  string HelloW[] = {<0=>"Hello", <1=>"World"};

To silence these I've extended the `Designators` config option to work as an 
enum that will let users choose to enable or disable arrays while still letting 
them use the normal designators.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131066

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -76,7 +76,7 @@
   Config C;
   C.InlayHints.Parameters = false;
   C.InlayHints.DeducedTypes = false;
-  C.InlayHints.Designators = false;
+  C.InlayHints.Designators = Config::InlayHints::None;
   return C;
 }
 
@@ -114,10 +114,11 @@
 }
 
 template 
-void assertDesignatorHints(llvm::StringRef AnnotatedSource,
+void assertDesignatorHints(llvm::StringRef AnnotatedSource, bool IncludeArray,
ExpectedHints... Expected) {
   Config Cfg;
-  Cfg.InlayHints.Designators = true;
+  Cfg.InlayHints.Designators =
+  IncludeArray ? Config::InlayHints::All : Config::InlayHints::IgnoreArrays;
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
   assertHints(InlayHintKind::Designator, AnnotatedSource, Expected...);
 }
@@ -1353,14 +1354,17 @@
 }
 
 TEST(DesignatorHints, Basic) {
-  assertDesignatorHints(R"cpp(
+  StringRef Source = R"cpp(
 struct S { int x, y, z; };
 S s {$x[[1]], $y[[2+2]]};
 
 int x[] = {$0[[0]], $1[[1]]};
-  )cpp",
-ExpectedHint{".x=", "x"}, ExpectedHint{".y=", "y"},
-ExpectedHint{"[0]=", "0"}, ExpectedHint{"[1]=", "1"});
+  )cpp";
+  assertDesignatorHints(Source, true, ExpectedHint{".x=", "x"},
+ExpectedHint{".y=", "y"}, ExpectedHint{"[0]=", "0"},
+ExpectedHint{"[1]=", "1"});
+  assertDesignatorHints(Source, false, ExpectedHint{".x=", "x"},
+ExpectedHint{".y=", "y"});
 }
 
 TEST(DesignatorHints, Nested) {
@@ -1369,8 +1373,9 @@
 struct Outer { Inner a, b; };
 Outer o{ $a[[{ $x[[1]], $y[[2]] }]], $bx[[3]] };
   )cpp",
-ExpectedHint{".a=", "a"}, ExpectedHint{".x=", "x"},
-ExpectedHint{".y=", "y"}, ExpectedHint{".b.x=", "bx"});
+true, ExpectedHint{".a=", "a"},
+ExpectedHint{".x=", "x"}, ExpectedHint{".y=", "y"},
+ExpectedHint{".b.x=", "bx"});
 }
 
 TEST(DesignatorHints, AnonymousRecord) {
@@ -1386,7 +1391,7 @@
 };
 S s{$xy[[42]]};
   )cpp",
-ExpectedHint{".x.y=", "xy"});
+true, ExpectedHint{".x.y=", "xy"});
 }
 
 TEST(DesignatorHints, Suppression) {
@@ -1394,7 +1399,7 @@
 struct Point { int a, b, c, d, e, f, g, h; };
 Point p{/*a=*/1, .c=2, /* .d = */3, $e[[4]]};
   )cpp",
-ExpectedHint{".e=", "e"});
+true, ExpectedHint{".e=", "e"});
 }
 
 TEST(DesignatorHints, StdArray) {
@@ -1404,17 +1409,20 @@
 template  struct Array { T __elements[N]; };
 Array x = {$0[[0]], $1[[1]]};
   )cpp",
-ExpectedHint{"[0]=", "0"}, ExpectedHint{"[1]=", "1"});
+true, ExpectedHint{"[0]=", "0"},
+ExpectedHint{"[1]=", "1"});
 }
 
 TEST(DesignatorHints, OnlyAggregateInit) {
-  assertDesignatorHints(R"cpp(
+  assertDesignatorHints(
+  R"cpp(
 struct Copyable { int x; } c;
 Copyable d{c};
 
 struct Constructible { Constructible(int x); };
 Constructible x{42};
-  )cpp" /*no designator hints expected (but param hints!)*/);
+  )cpp",
+  true /*no designator hints expected (but param hints!)*/);
 }
 
 TEST(InlayHints, RestrictRange) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -66,12 +66,15 @@
   }
   // Print the designator to Out.
   // Returns false if we could not produce a designator for this element.
-  bool append(std::string &Out, bool ForSubobject) {
+  bool append(std::string &Out, bool ForSubobject, bool CollectArrays) {
 if (IsArray) {
-  Ou

[PATCH] D131066: [clangd] Add option to disable inlay hints for init lists when building array.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

I'm not 100% sold on this way of setting this up.
There is definitely a use case for splitting the current Designators option 
into 2, one of them just controlling arrays.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131066

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

[PATCH] D131062: [docs] Add "C++20 Modules"

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/docs/CPlusPlus20Modules.rst:468-469
+
+(This may be an implementation defect. Although we could argue that header 
units have no name according to the spec,
+it is natural that we couldn't search it. But this is not user friendly 
enough.)
+

I am not sure if we should remain this. I feel both (remain or not remain) is 
OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131062

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

[PATCH] D131067: [analyzer] Treat values passed as parameter as escaped

[Thomas Weißschuh via Phabricator via cfe-commits]

t-8ch created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, 
xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
t-8ch requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131067

Files:
  clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  clang/test/Analysis/dead-stores.cpp


Index: clang/test/Analysis/dead-stores.cpp
===
--- clang/test/Analysis/dead-stores.cpp
+++ clang/test/Analysis/dead-stores.cpp
@@ -217,3 +217,26 @@
   return i + j;
 }
 
+//===--===//
+// Dead store checking involving references.
+//===--===//
+
+void functionReferenceParameter(int &i) {
+  i = 5; // no warning
+}
+
+struct constructorReferenceParameter {
+  constructorReferenceParameter(int &i) {
+i = 5; // no warning
+  }
+};
+
+void referenceParameters() {
+  int i = 5;
+  functionReferenceParameter(i);
+  i = 6;
+
+  int j = 7;
+  constructorReferenceParameter k(j);
+  j = 8;
+}
Index: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -483,12 +483,21 @@
   void operator()(const Stmt *S) {
 // Check for '&'. Any VarDecl whose address has been taken we treat as
 // escaped.
-// FIXME: What about references?
 if (auto *LE = dyn_cast(S)) {
   findLambdaReferenceCaptures(LE);
   return;
 }
 
+if (auto *CE = dyn_cast(S)) {
+  findCallReferenceParameters(CE);
+  return;
+}
+
+if (auto *CE = dyn_cast(S)) {
+  findConstructorReferenceParameters(CE);
+  return;
+}
+
 const UnaryOperator *U = dyn_cast(S);
 if (!U)
   return;
@@ -522,6 +531,33 @@
 Escaped.insert(VD);
 }
   }
+
+  void findReferenceParameter(const ParmVarDecl *Param, const Expr *Arg) {
+if (Param->getType()->isReferenceType()) {
+  if (auto *DRE = dyn_cast(Arg)) {
+if (auto *VD = dyn_cast(DRE->getDecl())) {
+  Escaped.insert(VD);
+}
+  }
+}
+  }
+
+  void findCallReferenceParameters(const CallExpr *CE) {
+if (auto *FD = dyn_cast(CE->getCalleeDecl())) {
+  unsigned numParams = FD->getNumParams();
+  for (unsigned i = 0; i < numParams; ++i) {
+findReferenceParameter(FD->getParamDecl(i), CE->getArg(i));
+  }
+}
+  }
+
+  void findConstructorReferenceParameters(const CXXConstructExpr *CE) {
+const FunctionDecl *FD = CE->getConstructor();
+unsigned numParams = FD->getNumParams();
+for (unsigned i = 0; i < numParams; ++i) {
+  findReferenceParameter(FD->getParamDecl(i), CE->getArg(i));
+}
+  }
 };
 } // end anonymous namespace
 


Index: clang/test/Analysis/dead-stores.cpp
===
--- clang/test/Analysis/dead-stores.cpp
+++ clang/test/Analysis/dead-stores.cpp
@@ -217,3 +217,26 @@
   return i + j;
 }
 
+//===--===//
+// Dead store checking involving references.
+//===--===//
+
+void functionReferenceParameter(int &i) {
+  i = 5; // no warning
+}
+
+struct constructorReferenceParameter {
+  constructorReferenceParameter(int &i) {
+i = 5; // no warning
+  }
+};
+
+void referenceParameters() {
+  int i = 5;
+  functionReferenceParameter(i);
+  i = 6;
+
+  int j = 7;
+  constructorReferenceParameter k(j);
+  j = 8;
+}
Index: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -483,12 +483,21 @@
   void operator()(const Stmt *S) {
 // Check for '&'. Any VarDecl whose address has been taken we treat as
 // escaped.
-// FIXME: What about references?
 if (auto *LE = dyn_cast(S)) {
   findLambdaReferenceCaptures(LE);
   return;
 }
 
+if (auto *CE = dyn_cast(S)) {
+  findCallReferenceParameters(CE);
+  return;
+}
+
+if (auto *CE = dyn_cast(S)) {
+  findConstructorReferenceParameters(CE);
+  return;
+}
+
 const UnaryOperator *U = dyn_cast(S);
 if (!U)
   return;
@@ -522,6 +531,33 @@
 Escaped.insert(VD);
 }
   }
+
+  void findReferenceParameter(const ParmVarDecl *Param, const Expr *Arg) {
+if (Param->getType()->isReferenceType()) {
+  if (auto *DRE = dyn_cast(Arg)) {
+if (auto *VD = d

[PATCH] D131065: [clang][dataflow] Store DeclContext of block being analysed in Environment if available.

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:170
 
+  /// Returns the `DeclCtx` of the block being analysed if provided, otherwise
+  /// returns nullptr.





Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:370
 
+  // `DeclCtx` of the block being analysed if provided.
+  const DeclContext *DeclCtx;





Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:170-172
+ const DeclContext &DeclCtxArg)
 : Environment(DACtx) {
+  DeclCtx = &DeclCtxArg;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131065

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Fraser Cormack via Phabricator via cfe-commits]

frasercrmck added inline comments.



Comment at: llvm/docs/ReleaseNotes.rst:55
+This means that the the following versions are now required to build LLVM
+and there is no way to supress this error.
+

`suppress`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130894: [clang] Print more information about failed static assertions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 449637.
tbaeder added a comment.

Also print `nullptr` expressions.


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

https://reviews.llvm.org/D130894

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/init-capture.cpp
  clang/test/Lexer/cxx1z-trigraphs.cpp
  clang/test/PCH/cxx-templates.cpp
  clang/test/Parser/objc-static-assert.mm
  clang/test/Sema/static-assert.c
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/static-assert-cxx17.cpp
  clang/test/SemaCXX/static-assert.cpp
  clang/test/SemaTemplate/instantiate-var-template.cpp
  clang/test/SemaTemplate/instantiation-dependence.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
@@ -184,7 +184,8 @@
 
 namespace Diags {
   struct A { int n, m; };
-  template struct X { static_assert(a.n == a.m); }; // expected-error {{static assertion failed due to requirement 'Diags::A{1, 2}.n == Diags::A{1, 2}.m'}}
+  template struct X { static_assert(a.n == a.m); }; // expected-error {{static assertion failed due to requirement 'Diags::A{1, 2}.n == Diags::A{1, 2}.m'}} \
+ // expected-note {{evaluates to 1 == 2}}
   template struct X; // expected-note {{in instantiation of template class 'Diags::X<{1, 2}>' requested here}}
 }
 
Index: clang/test/SemaTemplate/instantiation-dependence.cpp
===
--- clang/test/SemaTemplate/instantiation-dependence.cpp
+++ clang/test/SemaTemplate/instantiation-dependence.cpp
@@ -68,8 +68,10 @@
   struct D : B, C {};
 
   static_assert(trait::specialization == 0);
-  static_assert(trait::specialization == 1); // FIXME expected-error {{failed}}
-  static_assert(trait::specialization == 2); // FIXME expected-error {{failed}}
+  static_assert(trait::specialization == 1); // FIXME expected-error {{failed}} \
+// expected-note {{evaluates to 0}}
+  static_assert(trait::specialization == 2); // FIXME expected-error {{failed}} \
+// expected-note {{evaluates to 0}}
   static_assert(trait::specialization == 0); // FIXME-error {{ambiguous partial specialization}}
 }
 
Index: clang/test/SemaTemplate/instantiate-var-template.cpp
===
--- clang/test/SemaTemplate/instantiate-var-template.cpp
+++ clang/test/SemaTemplate/instantiate-var-template.cpp
@@ -31,7 +31,8 @@
   static_assert(b == 1, ""); // expected-note {{in instantiation of}} expected-error {{not an integral constant}}
 
   template void f() {
-static_assert(a == 0, ""); // expected-error {{static assertion failed due to requirement 'a == 0'}}
+static_assert(a == 0, ""); // expected-error {{static assertion failed due to requirement 'a == 0'}} \
+   // expected-note {{evaluates to 1}}
   }
 }
 
Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -22,7 +22,8 @@
 T<2> t2;
 
 template struct S {
-static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static assertion failed due to requirement 'sizeof(char) > sizeof(char)': Type not big enough!}}
+static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static assertion failed due to requirement 'sizeof(char) > sizeof(char)': Type not big enough!}} \
+ // expected-note {{1 > 1}}
 };
 
 S s1; // expected-note {{in instantiation of template class 'S' requested here}}
@@ -215,3 +216,32 @@
 static_assert(constexprNotBool, "message"); // expected-error {{value of type 'const NotBool' is not contextually convertible to 'bool'}}
 
 static_assert(1 , "") // expected-error {{expected ';' after 'static_assert'}}
+
+
+namespace Diagnostics {
+  /// No notes for literals.
+  static_assert(false, ""); // expected-error {{failed}}
+  static_assert(1.0 > 2.0, ""); // expected-error {{failed}}
+  static_assert('c' == 'd', ""); // expected-error {{failed}}
+  static_assert(1 == 2, ""); // expected-error {{failed}}
+
+  /// Simple things are ignored.
+  static_assert(1 == (-(1)), ""); //expected-error {{failed}}
+
+  /// Chars are printed as chars.
+  constexpr char getChar() {
+return 'c';
+  }
+  static_asse

[PATCH] D131065: [clang][dataflow] Store DeclContext of block being analysed in Environment if available.

[Sam Estep via Phabricator via cfe-commits]

samestep added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:216
 
   const auto *FuncDecl = Call->getDirectCallee();
   assert(FuncDecl != nullptr);

I think you need to set the `DeclCtx` field here (or at least, somewhere in 
this method).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131065

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

[PATCH] D130951: [HLSL] CodeGen hlsl resource binding.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:46
 public:
+  struct ResBinding {
+llvm::Optional Reg;

Does this apply to buffers only? In which case it might be better to either 
nest this into Buffer definition or rename into something like  
`BufferResBinding`. Also adding some documenting comments would help here, even 
if they could just refer to the language documentation.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:64
   CodeGenModule &CGM;
   uint32_t ResourceCounters[static_cast(
   hlsl::ResourceClass::NumClasses)] = {0};

This is not part of this change but any reason why it is a protected member and 
not private?



Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:77
 private:
+  void addResourceAnnotation(llvm::GlobalVariable *GV, llvm::StringRef TyName,
+ hlsl::ResourceClass RC, ResBinding &Binding);

Similarly - is this buffer specific? Then you could either rename to 
`addBufferResourceAnnotation` or make this a member of a Buffer instead...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130951

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

[PATCH] D130894: [clang] Print more information about failed static assertions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 449639.

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

https://reviews.llvm.org/D130894

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/init-capture.cpp
  clang/test/Lexer/cxx1z-trigraphs.cpp
  clang/test/PCH/cxx-templates.cpp
  clang/test/Parser/objc-static-assert.mm
  clang/test/Sema/static-assert.c
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/static-assert-cxx17.cpp
  clang/test/SemaCXX/static-assert.cpp
  clang/test/SemaTemplate/instantiate-var-template.cpp
  clang/test/SemaTemplate/instantiation-dependence.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
@@ -184,7 +184,8 @@
 
 namespace Diags {
   struct A { int n, m; };
-  template struct X { static_assert(a.n == a.m); }; // expected-error {{static assertion failed due to requirement 'Diags::A{1, 2}.n == Diags::A{1, 2}.m'}}
+  template struct X { static_assert(a.n == a.m); }; // expected-error {{static assertion failed due to requirement 'Diags::A{1, 2}.n == Diags::A{1, 2}.m'}} \
+ // expected-note {{evaluates to 1 == 2}}
   template struct X; // expected-note {{in instantiation of template class 'Diags::X<{1, 2}>' requested here}}
 }
 
Index: clang/test/SemaTemplate/instantiation-dependence.cpp
===
--- clang/test/SemaTemplate/instantiation-dependence.cpp
+++ clang/test/SemaTemplate/instantiation-dependence.cpp
@@ -68,8 +68,10 @@
   struct D : B, C {};
 
   static_assert(trait::specialization == 0);
-  static_assert(trait::specialization == 1); // FIXME expected-error {{failed}}
-  static_assert(trait::specialization == 2); // FIXME expected-error {{failed}}
+  static_assert(trait::specialization == 1); // FIXME expected-error {{failed}} \
+// expected-note {{evaluates to 0}}
+  static_assert(trait::specialization == 2); // FIXME expected-error {{failed}} \
+// expected-note {{evaluates to 0}}
   static_assert(trait::specialization == 0); // FIXME-error {{ambiguous partial specialization}}
 }
 
Index: clang/test/SemaTemplate/instantiate-var-template.cpp
===
--- clang/test/SemaTemplate/instantiate-var-template.cpp
+++ clang/test/SemaTemplate/instantiate-var-template.cpp
@@ -31,7 +31,8 @@
   static_assert(b == 1, ""); // expected-note {{in instantiation of}} expected-error {{not an integral constant}}
 
   template void f() {
-static_assert(a == 0, ""); // expected-error {{static assertion failed due to requirement 'a == 0'}}
+static_assert(a == 0, ""); // expected-error {{static assertion failed due to requirement 'a == 0'}} \
+   // expected-note {{evaluates to 1}}
   }
 }
 
Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -22,7 +22,8 @@
 T<2> t2;
 
 template struct S {
-static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static assertion failed due to requirement 'sizeof(char) > sizeof(char)': Type not big enough!}}
+static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static assertion failed due to requirement 'sizeof(char) > sizeof(char)': Type not big enough!}} \
+ // expected-note {{1 > 1}}
 };
 
 S s1; // expected-note {{in instantiation of template class 'S' requested here}}
@@ -215,3 +216,32 @@
 static_assert(constexprNotBool, "message"); // expected-error {{value of type 'const NotBool' is not contextually convertible to 'bool'}}
 
 static_assert(1 , "") // expected-error {{expected ';' after 'static_assert'}}
+
+
+namespace Diagnostics {
+  /// No notes for literals.
+  static_assert(false, ""); // expected-error {{failed}}
+  static_assert(1.0 > 2.0, ""); // expected-error {{failed}}
+  static_assert('c' == 'd', ""); // expected-error {{failed}}
+  static_assert(1 == 2, ""); // expected-error {{failed}}
+
+  /// Simple things are ignored.
+  static_assert(1 == (-(1)), ""); //expected-error {{failed}}
+
+  /// Chars are printed as chars.
+  constexpr char getChar() {
+return 'c';
+  }
+  static_assert(getChar() == 'a', ""); // expected-error {{failed}} \
+   

[PATCH] D131012: No longer place const volatile global variables in a read only section

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman planned changes to this revision.
aaron.ballman added a comment.

In D131012#3695110 , @anakryiko wrote:

> This will severly break BPF users, as we have a heavy reliance on `const 
> volatile` variables being allocated to `.rodata`, which are passed to BPF 
> verifier in Linux kernel as read-only values. This is critical for BPF 
> Complie Once - Run Everywhere approach of writing portable BPF applications. 
> We've been relying on this behavior for years now and changes this will break 
> many existing BPF applications.

Thank you for this feedback! I guess I'm a bit surprised given the contents 
from the issue seem to imply that BPF needed Clang's behavior to change: `Note 
that this is causing practical difficulties: the kernel's bpftool is generating 
different skeleton headers for BPF code compiled from LLVM and GCC, because the 
names of the containing sections get reflected.`

That said, I'm asking on the WG14 reflectors whether there's a normative 
requirement here or not, so I'm marking this as having planned changes until I 
hear back from WG14 and until we've resolved whether the changes will fix code 
vs break code (or both!) so we don't accidentally land this.


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

https://reviews.llvm.org/D131012

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

[PATCH] D131039: [clang][dataflow] Add cache of `ControlFlowContext`s for function decls.

[Sam Estep via Phabricator via cfe-commits]

samestep added a comment.

Looks awesome, thanks @ymandel!




Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:23
 #include "clang/AST/Stmt.h"
-#include "clang/Analysis/CFG.h"
 #include "clang/Analysis/FlowSensitive/ControlFlowContext.h"

I'm guessing this change is unrelated to the patch as a whole?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131039

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

[PATCH] D131039: [clang][dataflow] Add cache of `ControlFlowContext`s for function decls.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 4 inline comments as done.
ymandel added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:34
+  /// Builds a ControlFlowContext from an AST node. `D` is the function in 
which
+  /// `S` resides. All arguments must be non-null.
   static llvm::Expected build(const Decl *D, Stmt *S,

sgatev wrote:
> Can we make them references?
Turns out I was wrong - `D` can be null. Would that CFG has comments about what 
its parameters do... We have a test that excercises this (though I can't say 
why):

https://github.com/llvm/llvm-project/blob/9a976f36615dbe15e76c12b22f711b2e597a8e51/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp#L70

The other two -- yes -- but then we'd have to update the various callers as 
well. I'd rather not do that in this patch, but I'll add the new overload and 
we can follow up with cleanups in another patch.





Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:50
 private:
-  ControlFlowContext(std::unique_ptr Cfg,
+  // `D` must not be null.
+  ControlFlowContext(const Decl *D, std::unique_ptr Cfg,

sgatev wrote:
> Can we make it a reference?
See above.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:23
 #include "clang/AST/Stmt.h"
-#include "clang/Analysis/CFG.h"
 #include "clang/Analysis/FlowSensitive/ControlFlowContext.h"

samestep wrote:
> I'm guessing this change is unrelated to the patch as a whole?
correct. it was a driveby fix (for better or worse).



Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:369
+
+  // Keyed on the function's fully qualified name. No leading "::".
+  llvm::StringMap FunctionModels;

sgatev wrote:
> Can we make the keys `FunctionDecl *` and use `FunctionDecl::getDefinition` 
> in `getControlFlowContext` to obtain a canonical pointer?
We can for now -- for the purposes of the cache that makes the most sense. I 
thought of strings because the goal is a map from *specific* (named) functions, 
for which we won't a priori have the function decls. still, I'll change for now 
and we can change it back (or to a better implementation) in the follow up 
patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131039

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

[PATCH] D131039: [clang][dataflow] Add cache of `ControlFlowContext`s for function decls.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 449648.
ymandel marked 4 inline comments as done.
ymandel added a comment.

addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131039

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  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
@@ -507,28 +507,27 @@
 return;
   Env.setStorageLocation(*S, *ArgLoc);
 } else if (const FunctionDecl *F = S->getDirectCallee()) {
-  // This case is for context-sensitive analysis, which we only do if we
-  // have the callee body available in the translation unit.
-  if (!Options.ContextSensitive || F->getBody() == nullptr)
+  // This case is for context-sensitive analysis.
+  if (!Options.ContextSensitive)
+return;
+
+  const ControlFlowContext *CFCtx = Env.getControlFlowContext(F);
+  if (!CFCtx)
 return;
 
   // FIXME: We don't support context-sensitive analysis of recursion, so
   // we should return early here if `F` is the same as the `FunctionDecl`
   // holding `S` itself.
 
-  auto &ASTCtx = F->getASTContext();
-
-  // FIXME: Cache these CFGs.
-  auto CFCtx = ControlFlowContext::build(F, F->getBody(), &ASTCtx);
-  // FIXME: Handle errors here and below.
-  assert(CFCtx);
   auto ExitBlock = CFCtx->getCFG().getExit().getBlockID();
 
   auto CalleeEnv = Env.pushCall(S);
 
-  // FIXME: Use the same analysis as the caller for the callee.
-  DataflowAnalysisOptions Options;
-  auto Analysis = NoopAnalysis(ASTCtx, Options);
+  // FIXME: Use the same analysis as the caller for the callee. Note,
+  // though, that doing so would require support for changing the analysis's
+  // ASTContext.
+  auto Analysis = NoopAnalysis(CFCtx->getDecl().getASTContext(),
+   DataflowAnalysisOptions());
 
   auto BlockToOutputState =
   dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv);
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -16,6 +16,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/Analysis/FlowSensitive/DebugSupport.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
+#include "clang/Basic/IdentifierTable.h"
 #include "llvm/Support/Debug.h"
 #include 
 #include 
@@ -335,6 +336,27 @@
   llvm::dbgs() << debugString(Constraints, AtomNames);
 }
 
+const ControlFlowContext *
+DataflowAnalysisContext::getControlFlowContext(const FunctionDecl *F) {
+  // Canonicalize the key:
+  F = F->getDefinition();
+  if (F == nullptr)
+return nullptr;
+  auto It = FunctionModels.find(F);
+  if (It != FunctionModels.end())
+return &It->second;
+
+  if (Stmt *Body = F->getBody()) {
+auto CFCtx = ControlFlowContext::build(F, *Body, F->getASTContext());
+// FIXME: Handle errors.
+assert(CFCtx);
+auto Result = FunctionModels.insert({F, std::move(*CFCtx)});
+return &Result.first->second;
+  }
+
+  return nullptr;
+}
+
 } // namespace dataflow
 } // namespace clang
 
Index: clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -45,7 +45,7 @@
 }
 
 llvm::Expected
-ControlFlowContext::build(const Decl *D, Stmt *S, ASTContext *C) {
+ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) {
   CFG::BuildOptions Options;
   Options.PruneTriviallyFalseEdges = false;
   Options.AddImplicitDtors = true;
@@ -56,7 +56,7 @@
   // Ensure that all sub-expressions in basic blocks are evaluated.
   Options.setAllAlwaysAdd();
 
-  auto Cfg = CFG::buildCFG(D, S, C, Options);
+  auto Cfg = CFG::buildCFG(D, &S, &C, Options);
   if (Cfg == nullptr)
 return llvm::createStringError(
 std::make_error_code(std::errc::invalid_argument),
@@ -64,7 +64,14 @@
 
   llvm::DenseMap StmtToBlock =
   buildStmtToBasicBlockMap(*Cfg);
-  return ControlFlowContext(std::move(Cfg), std::move(StmtToBlock));
+  return ControlFlowContext(D, std::move(Cfg), std::move(StmtToBlock));
+}
+
+llvm::Expected
+ControlFlowC

[PATCH] D131039: [clang][dataflow] Add cache of `ControlFlowContext`s for function decls.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 449649.
ymandel added a comment.

adusted interface to return (nullable) pointer rather than reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131039

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  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
@@ -507,28 +507,30 @@
 return;
   Env.setStorageLocation(*S, *ArgLoc);
 } else if (const FunctionDecl *F = S->getDirectCallee()) {
-  // This case is for context-sensitive analysis, which we only do if we
-  // have the callee body available in the translation unit.
-  if (!Options.ContextSensitive || F->getBody() == nullptr)
+  // This case is for context-sensitive analysis.
+  if (!Options.ContextSensitive)
+return;
+
+  const ControlFlowContext *CFCtx = Env.getControlFlowContext(F);
+  if (!CFCtx)
 return;
 
   // FIXME: We don't support context-sensitive analysis of recursion, so
   // we should return early here if `F` is the same as the `FunctionDecl`
   // holding `S` itself.
 
-  auto &ASTCtx = F->getASTContext();
-
-  // FIXME: Cache these CFGs.
-  auto CFCtx = ControlFlowContext::build(F, F->getBody(), &ASTCtx);
-  // FIXME: Handle errors here and below.
-  assert(CFCtx);
   auto ExitBlock = CFCtx->getCFG().getExit().getBlockID();
 
   auto CalleeEnv = Env.pushCall(S);
 
-  // FIXME: Use the same analysis as the caller for the callee.
-  DataflowAnalysisOptions Options;
-  auto Analysis = NoopAnalysis(ASTCtx, Options);
+  // FIXME: Use the same analysis as the caller for the callee. Note,
+  // though, that doing so would require support for changing the analysis's
+  // ASTContext.
+  assert(
+  CFCtx->getDecl() != nullptr &&
+  "ControlFlowContexts in the environment should always carry a decl");
+  auto Analysis = NoopAnalysis(CFCtx->getDecl()->getASTContext(),
+   DataflowAnalysisOptions());
 
   auto BlockToOutputState =
   dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv);
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -16,6 +16,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/Analysis/FlowSensitive/DebugSupport.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
+#include "clang/Basic/IdentifierTable.h"
 #include "llvm/Support/Debug.h"
 #include 
 #include 
@@ -335,6 +336,27 @@
   llvm::dbgs() << debugString(Constraints, AtomNames);
 }
 
+const ControlFlowContext *
+DataflowAnalysisContext::getControlFlowContext(const FunctionDecl *F) {
+  // Canonicalize the key:
+  F = F->getDefinition();
+  if (F == nullptr)
+return nullptr;
+  auto It = FunctionModels.find(F);
+  if (It != FunctionModels.end())
+return &It->second;
+
+  if (Stmt *Body = F->getBody()) {
+auto CFCtx = ControlFlowContext::build(F, *Body, F->getASTContext());
+// FIXME: Handle errors.
+assert(CFCtx);
+auto Result = FunctionModels.insert({F, std::move(*CFCtx)});
+return &Result.first->second;
+  }
+
+  return nullptr;
+}
+
 } // namespace dataflow
 } // namespace clang
 
Index: clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -45,7 +45,7 @@
 }
 
 llvm::Expected
-ControlFlowContext::build(const Decl *D, Stmt *S, ASTContext *C) {
+ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) {
   CFG::BuildOptions Options;
   Options.PruneTriviallyFalseEdges = false;
   Options.AddImplicitDtors = true;
@@ -56,7 +56,7 @@
   // Ensure that all sub-expressions in basic blocks are evaluated.
   Options.setAllAlwaysAdd();
 
-  auto Cfg = CFG::buildCFG(D, S, C, Options);
+  auto Cfg = CFG::buildCFG(D, &S, &C, Options);
   if (Cfg == nullptr)
 return llvm::createStringError(
 std::make_error_code(std::errc::invalid_argument),
@@ -64,7 +64,14 @@
 
   llvm::DenseMap StmtToBlock =
   buildStmtToBasicBlockMap(*Cfg);
-  return ControlFlowContext(

[PATCH] D131039: [clang][dataflow] Add cache of `ControlFlowContext`s for function decls.

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:34
+  /// Builds a ControlFlowContext from an AST node. `D` is the function in 
which
+  /// `S` resides. All arguments must be non-null.
   static llvm::Expected build(const Decl *D, Stmt *S,

ymandel wrote:
> sgatev wrote:
> > Can we make them references?
> Turns out I was wrong - `D` can be null. Would that CFG has comments about 
> what its parameters do... We have a test that excercises this (though I can't 
> say why):
> 
> https://github.com/llvm/llvm-project/blob/9a976f36615dbe15e76c12b22f711b2e597a8e51/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp#L70
> 
> The other two -- yes -- but then we'd have to update the various callers as 
> well. I'd rather not do that in this patch, but I'll add the new overload and 
> we can follow up with cleanups in another patch.
> 
> 
Weird. I think we should change the test to pass the decl.



Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:369
+
+  llvm::DenseMap FunctionModels;
 };

Perhaps `FunctionContexts`?



Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:19
 #include "clang/Analysis/FlowSensitive/Value.h"
+#include "clang/Basic/IdentifierTable.h"
 #include "llvm/Support/Debug.h"

Is this still needed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131039

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

[PATCH] D131070: [clang][sema] Fix collectConjunctionTerms()

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added a reviewer: aaron.ballman.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Consider:

A == 5 && A != 5

IfA is 5, the old collectConjunctionTerms() would call itself again for
the LHS (which it ignores), then  the RHS (which it also ignores) and
then just return without ever adding anything to the Terms array.

For example, there's a test case in `clang/test/SemaCXX/recovery-expr-type.cpp`:

  namespace test13 {
  enum Circular { // expected-note {{not complete until the closing 
'}'}}
Circular_A = Circular(1), // expected-error {{'Circular' is an incomplete 
type}}
  };
  // Enumerators can be evaluated (they evaluate as zero, but we don't care).
  static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error 
{{static assertion failed}}
  }

which currently prints:

  test2.cpp:6:1: error: static assertion failed due to requirement 'Circular_A 
== 0 && Circular_A != 0':
  static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error 
{{static assertion failed}}
  ^ ~~
  2 errors generated.

(other diagnostics omitted)

but after this change prints:

  test2.cpp:6:1: error: static assertion failed due to requirement 'Circular_A 
!= 0':
  static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error 
{{static assertion failed}}
  ^~~~
  2 errors generated.

The patch depends on https://reviews.llvm.org/D130894 because of the note it 
adds, but that's not necessary. It's just easier because they are both in my 
local tree.

I wanted to add Douglas Gregor as reviewer as well but seems like he isn't 
around anymore.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131070

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaCXX/recovery-expr-type.cpp


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -149,7 +149,8 @@
   Circular_A = Circular(1), // expected-error {{'Circular' is an incomplete 
type}}
 };
 // Enumerators can be evaluated (they evaluate as zero, but we don't care).
-static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error 
{{static assertion failed}}
+static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error 
{{static assertion failed}} \
+   // expected-note 
{{evaluates to 0}}
 }
 
 namespace test14 {
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -3585,9 +3585,8 @@
 if (BinOp->getOpcode() == BO_LAnd) {
   collectConjunctionTerms(BinOp->getLHS(), Terms);
   collectConjunctionTerms(BinOp->getRHS(), Terms);
+  return;
 }
-
-return;
   }
 
   Terms.push_back(Clause);


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -149,7 +149,8 @@
   Circular_A = Circular(1), // expected-error {{'Circular' is an incomplete type}}
 };
 // Enumerators can be evaluated (they evaluate as zero, but we don't care).
-static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error {{static assertion failed}}
+static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error {{static assertion failed}} \
+   // expected-note {{evaluates to 0}}
 }
 
 namespace test14 {
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -3585,9 +3585,8 @@
 if (BinOp->getOpcode() == BO_LAnd) {
   collectConjunctionTerms(BinOp->getLHS(), Terms);
   collectConjunctionTerms(BinOp->getRHS(), Terms);
+  return;
 }
-
-return;
   }
 
   Terms.push_back(Clause);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130863: [clangd] Make git ignore index directories

[sums via Phabricator via cfe-commits]

sums marked an inline comment as done.
sums added a comment.

Thank you for the reviews and discussion, everyone!

TIL about `.git/info/exclude`. It'll let me exclude stuff without updating the 
checked in `.gitignore`. I was achieving the same effect for clangd by manually 
creating a `.gitignore` similar to this change.

The ideal scenario would be one where all tools ignore their temporary 
directories automatically. Will send patches whenever I encounter a case like 
this :)

@kadircet kindly take another look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130863

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

[libunwind] 44b4f4d - [libunwind] Remove unused substitution in AIX libunwind config

[Louis Dionne via cfe-commits]

Author: Louis Dionne
Date: 2022-08-03T09:28:39-04:00
New Revision: 44b4f4df31ab71457ecd7127e64526cd8237bfa4

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

LOG: [libunwind] Remove unused substitution in AIX libunwind config

It must have been a copy-paste error, since cxx-include is never defined
by the libunwind config.

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

Added: 


Modified: 
libunwind/test/configs/ibm-libunwind-shared.cfg.in

Removed: 




diff  --git a/libunwind/test/configs/ibm-libunwind-shared.cfg.in 
b/libunwind/test/configs/ibm-libunwind-shared.cfg.in
index c3c0ddd5c726e..599de72b1399d 100644
--- a/libunwind/test/configs/ibm-libunwind-shared.cfg.in
+++ b/libunwind/test/configs/ibm-libunwind-shared.cfg.in
@@ -5,7 +5,7 @@ lit_config.load_config(config, 
'@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}', ''))
 config.substitutions.append(('%{compile_flags}',
-'-nostdinc++ -I %{include} -I %{cxx-include}'
+'-nostdinc++ -I %{include}'
 ))
 config.substitutions.append(('%{link_flags}',
 '-nostdlib++ -L %{lib} -lunwind -ldl -Wl,-bbigtoc'



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

[clang] 84831bd - [SystemZ] Make 128 bit integers be aligned to 8 bytes.

[Jonas Paulsson via cfe-commits]

Author: Jonas Paulsson
Date: 2022-08-03T15:39:54+02:00
New Revision: 84831bdfedbad8221a529a640d0f4bd9d0e3ba7b

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

LOG: [SystemZ] Make 128 bit integers be aligned to 8 bytes.

The SystemZ ABI says that 128 bit integers should be aligned to only 8 bytes.

Reviewed By: Ulrich Weigand, Nikita Popov

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

Added: 
llvm/test/CodeGen/SystemZ/unaligned-02.ll

Modified: 
clang/include/clang/Basic/TargetInfo.h
clang/lib/AST/ASTContext.cpp
clang/lib/Basic/TargetInfo.cpp
clang/lib/Basic/Targets/SystemZ.h
clang/test/CodeGen/SystemZ/align-systemz.c
clang/test/CodeGen/SystemZ/systemz-abi.c
clang/test/CodeGen/SystemZ/zos-alignment.c

Removed: 




diff  --git a/clang/include/clang/Basic/TargetInfo.h 
b/clang/include/clang/Basic/TargetInfo.h
index b4f3a69259fad..6f9ee65544450 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -78,6 +78,7 @@ struct TransferrableTargetInfo {
   unsigned char LargeArrayMinWidth, LargeArrayAlign;
   unsigned char LongWidth, LongAlign;
   unsigned char LongLongWidth, LongLongAlign;
+  unsigned char Int128Align;
 
   // Fixed point bit widths
   unsigned char ShortAccumWidth, ShortAccumAlign;
@@ -470,6 +471,9 @@ class TargetInfo : public virtual TransferrableTargetInfo,
   unsigned getLongLongWidth() const { return LongLongWidth; }
   unsigned getLongLongAlign() const { return LongLongAlign; }
 
+  /// getInt128Align() - Returns the alignment of Int128.
+  unsigned getInt128Align() const { return Int128Align; }
+
   /// getShortAccumWidth/Align - Return the size of 'signed short _Accum' and
   /// 'unsigned short _Accum' for this target, in bits.
   unsigned getShortAccumWidth() const { return ShortAccumWidth; }

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index eefb07465b1cb..21fd3953eb159 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -2082,7 +2082,7 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const 
{
 case BuiltinType::Int128:
 case BuiltinType::UInt128:
   Width = 128;
-  Align = 128; // int128_t is 128-bit aligned on all targets.
+  Align = Target->getInt128Align();
   break;
 case BuiltinType::ShortAccum:
 case BuiltinType::UShortAccum:

diff  --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index 6685145ea6d2e..4e2446315a379 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -45,6 +45,7 @@ TargetInfo::TargetInfo(const llvm::Triple &T) : Triple(T) {
   IntWidth = IntAlign = 32;
   LongWidth = LongAlign = 32;
   LongLongWidth = LongLongAlign = 64;
+  Int128Align = 128;
 
   // Fixed point default bit widths
   ShortAccumWidth = ShortAccumAlign = 16;

diff  --git a/clang/lib/Basic/Targets/SystemZ.h 
b/clang/lib/Basic/Targets/SystemZ.h
index e4f242e624cb1..7def024d07a77 100644
--- a/clang/lib/Basic/Targets/SystemZ.h
+++ b/clang/lib/Basic/Targets/SystemZ.h
@@ -40,6 +40,7 @@ class LLVM_LIBRARY_VISIBILITY SystemZTargetInfo : public 
TargetInfo {
 TLSSupported = true;
 IntWidth = IntAlign = 32;
 LongWidth = LongLongWidth = LongAlign = LongLongAlign = 64;
+Int128Align = 64;
 PointerWidth = PointerAlign = 64;
 LongDoubleWidth = 128;
 LongDoubleAlign = 64;

diff  --git a/clang/test/CodeGen/SystemZ/align-systemz.c 
b/clang/test/CodeGen/SystemZ/align-systemz.c
index 8cc33a83f8686..72d988e413291 100644
--- a/clang/test/CodeGen/SystemZ/align-systemz.c
+++ b/clang/test/CodeGen/SystemZ/align-systemz.c
@@ -26,6 +26,14 @@ void func (void)
 }
 
 
+// The SystemZ ABI aligns __int128_t to only eight bytes.
+
+struct S_int128 {  __int128_t B; } Obj_I128;
+__int128_t GlobI128;
+// CHECK: @Obj_I128 = global %struct.S_int128 zeroinitializer, align 8
+// CHECK: @GlobI128 = global i128 0, align 8
+
+
 // Alignment should be respected for coerced argument loads
 
 struct arg { long y __attribute__((packed, aligned(4))); };
@@ -40,4 +48,3 @@ void test (void)
 
 // CHECK-LABEL: @test
 // CHECK: load i64, i64* getelementptr inbounds (%struct.arg, %struct.arg* @x, 
i32 0, i32 0), align 4
-

diff  --git a/clang/test/CodeGen/SystemZ/systemz-abi.c 
b/clang/test/CodeGen/SystemZ/systemz-abi.c
index e79a287852e2d..1e7e83684005a 100644
--- a/clang/test/CodeGen/SystemZ/systemz-abi.c
+++ b/clang/test/CodeGen/SystemZ/systemz-abi.c
@@ -43,7 +43,7 @@ long long pass_longlong(long long arg) { return arg; }
 // CHECK-LABEL: define{{.*}} i64 @pass_longlong(i64 %{{.*}})
 
 __int128 pass_int128(__int128 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_int128(i128* noalias sret(i128) align 
16 %{{.*}}, i128* %0)
+// CHECK-LABEL: define{{.*}} vo

[PATCH] D130900: [SystemZ] Make 128 bit integers be aligned to 8 bytes.

[Jonas Paulsson via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG84831bdfedba: [SystemZ] Make 128 bit integers be aligned to 
8 bytes. (authored by jonpa).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130900

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/SystemZ.h
  clang/test/CodeGen/SystemZ/align-systemz.c
  clang/test/CodeGen/SystemZ/systemz-abi.c
  clang/test/CodeGen/SystemZ/zos-alignment.c
  llvm/test/CodeGen/SystemZ/unaligned-02.ll

Index: llvm/test/CodeGen/SystemZ/unaligned-02.ll
===
--- /dev/null
+++ llvm/test/CodeGen/SystemZ/unaligned-02.ll
@@ -0,0 +1,13 @@
+; Check that an unaligned i128 access get the correct alignment added.
+;
+; RUN: llc < %s -mtriple=s390x-linux-gnu -stop-after=pre-isel-intrinsic-lowering \
+; RUN:   | FileCheck %s
+
+define void @f1(ptr %ptr) {
+; CHECK:   define void @f1(ptr %ptr) {
+; CHECK-NEXT:store i128 0, ptr %ptr, align 8
+; CHECK-NEXT:ret void
+; CHECK-NEXT:  }
+  store i128 0, ptr %ptr
+  ret void
+}
Index: clang/test/CodeGen/SystemZ/zos-alignment.c
===
--- clang/test/CodeGen/SystemZ/zos-alignment.c
+++ clang/test/CodeGen/SystemZ/zos-alignment.c
@@ -160,6 +160,13 @@
 // CHECK-NEXT: 8 |   char b
 // CHECK-NEXT:   | [sizeof=16, align=8]
 
+struct s12 {
+  __int128_t a;
+} S12;
+// CHECK:  0 | struct s12
+// CHECK-NEXT: 0 |   __int128_t a
+// CHECK-NEXT:   | [sizeof=16, align=8]
+
 union u0 {
   unsigned short d1 __attribute__((packed));
   int d2 : 10;
Index: clang/test/CodeGen/SystemZ/systemz-abi.c
===
--- clang/test/CodeGen/SystemZ/systemz-abi.c
+++ clang/test/CodeGen/SystemZ/systemz-abi.c
@@ -43,7 +43,7 @@
 // CHECK-LABEL: define{{.*}} i64 @pass_longlong(i64 %{{.*}})
 
 __int128 pass_int128(__int128 arg) { return arg; }
-// CHECK-LABEL: define{{.*}} void @pass_int128(i128* noalias sret(i128) align 16 %{{.*}}, i128* %0)
+// CHECK-LABEL: define{{.*}} void @pass_int128(i128* noalias sret(i128) align 8 %{{.*}}, i128* %0)
 
 float pass_float(float arg) { return arg; }
 // CHECK-LABEL: define{{.*}} float @pass_float(float %{{.*}})
Index: clang/test/CodeGen/SystemZ/align-systemz.c
===
--- clang/test/CodeGen/SystemZ/align-systemz.c
+++ clang/test/CodeGen/SystemZ/align-systemz.c
@@ -26,6 +26,14 @@
 }
 
 
+// The SystemZ ABI aligns __int128_t to only eight bytes.
+
+struct S_int128 {  __int128_t B; } Obj_I128;
+__int128_t GlobI128;
+// CHECK: @Obj_I128 = global %struct.S_int128 zeroinitializer, align 8
+// CHECK: @GlobI128 = global i128 0, align 8
+
+
 // Alignment should be respected for coerced argument loads
 
 struct arg { long y __attribute__((packed, aligned(4))); };
@@ -40,4 +48,3 @@
 
 // CHECK-LABEL: @test
 // CHECK: load i64, i64* getelementptr inbounds (%struct.arg, %struct.arg* @x, i32 0, i32 0), align 4
-
Index: clang/lib/Basic/Targets/SystemZ.h
===
--- clang/lib/Basic/Targets/SystemZ.h
+++ clang/lib/Basic/Targets/SystemZ.h
@@ -40,6 +40,7 @@
 TLSSupported = true;
 IntWidth = IntAlign = 32;
 LongWidth = LongLongWidth = LongAlign = LongLongAlign = 64;
+Int128Align = 64;
 PointerWidth = PointerAlign = 64;
 LongDoubleWidth = 128;
 LongDoubleAlign = 64;
Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -45,6 +45,7 @@
   IntWidth = IntAlign = 32;
   LongWidth = LongAlign = 32;
   LongLongWidth = LongLongAlign = 64;
+  Int128Align = 128;
 
   // Fixed point default bit widths
   ShortAccumWidth = ShortAccumAlign = 16;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2082,7 +2082,7 @@
 case BuiltinType::Int128:
 case BuiltinType::UInt128:
   Width = 128;
-  Align = 128; // int128_t is 128-bit aligned on all targets.
+  Align = Target->getInt128Align();
   break;
 case BuiltinType::ShortAccum:
 case BuiltinType::UShortAccum:
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -78,6 +78,7 @@
   unsigned char LargeArrayMinWidth, LargeArrayAlign;
   unsigned char LongWidth, LongAlign;
   unsigned char LongLongWidth, Lon

[clang] fb65b17 - [NFCI] Refactor how KeywordStatus is calculated

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2022-08-03T06:41:43-07:00
New Revision: fb65b17932e1163adeda943a3a36f9b482b97f47

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

LOG: [NFCI] Refactor how KeywordStatus is calculated

The getKeywordStatus function is a horrible mess of inter-dependent 'if'
statements that depend significantly on the ORDER of the checks.  This
patch removes the dependency on order by checking each set-flag only
once.

It does this by looping through each of the set bits, and checks each
individual flag for its effect, then combines them at the end.

This might slow down startup performance slightly, as there are only a
few hundred keywords, and a vast majority will only get checked 1x
still.

This patch ALSO removes the KEYWORD_CONCEPTS flag, because it has since
become synonymous with C++20.

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

Added: 


Modified: 
clang/include/clang/Basic/TokenKinds.def
clang/lib/Basic/IdentifierTable.cpp
clang/test/Lexer/keywords_test.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/TokenKinds.def 
b/clang/include/clang/Basic/TokenKinds.def
index 84fc0893c8b59..565e5a37be3d8 100644
--- a/clang/include/clang/Basic/TokenKinds.def
+++ b/clang/include/clang/Basic/TokenKinds.def
@@ -9,8 +9,7 @@
 // This file defines the TokenKind database.  This includes normal tokens like
 // tok::ampamp (corresponding to the && token) as well as keywords for various
 // languages.  Users of this file must optionally #define the TOK, KEYWORD,
-// CXX11_KEYWORD, CONCEPTS_KEYWORD, ALIAS, or PPKEYWORD macros to make use of
-// this file.
+// CXX11_KEYWORD, ALIAS, or PPKEYWORD macros to make use of this file.
 //
 
//===--===//
 
@@ -29,9 +28,6 @@
 #ifndef CXX20_KEYWORD
 #define CXX20_KEYWORD(X,Y) KEYWORD(X,KEYCXX20|(Y))
 #endif
-#ifndef CONCEPTS_KEYWORD
-#define CONCEPTS_KEYWORD(X) CXX20_KEYWORD(X,KEYCONCEPTS)
-#endif
 #ifndef COROUTINES_KEYWORD
 #define COROUTINES_KEYWORD(X) CXX20_KEYWORD(X,KEYCOROUTINES)
 #endif
@@ -259,8 +255,6 @@ PUNCTUATOR(caretcaret,"^^")
 //   KEYNOCXX - This is a keyword in every non-C++ dialect.
 //   KEYCXX11 - This is a C++ keyword introduced to C++ in C++11
 //   KEYCXX20 - This is a C++ keyword introduced to C++ in C++20
-//   KEYCONCEPTS - This is a keyword if the C++ extensions for concepts
-// are enabled.
 //   KEYMODULES - This is a keyword if the C++ extensions for modules
 //are enabled.
 //   KEYGNU   - This is a keyword if GNU extensions are enabled
@@ -390,10 +384,6 @@ CXX11_KEYWORD(nullptr   , 0)
 CXX11_KEYWORD(static_assert , KEYMSCOMPAT)
 CXX11_KEYWORD(thread_local  , 0)
 
-// C++20 keywords
-CONCEPTS_KEYWORD(concept)
-CONCEPTS_KEYWORD(requires)
-
 // C++20 / coroutines TS keywords
 COROUTINES_KEYWORD(co_await)
 COROUTINES_KEYWORD(co_return)
@@ -406,6 +396,9 @@ MODULES_KEYWORD(import)
 // C++20 keywords.
 CXX20_KEYWORD(consteval , 0)
 CXX20_KEYWORD(constinit , 0)
+CXX20_KEYWORD(concept   , 0)
+CXX20_KEYWORD(requires  , 0)
+
 // Not a CXX20_KEYWORD because it is disabled by -fno-char8_t.
 KEYWORD(char8_t , CHAR8SUPPORT)
 
@@ -935,7 +928,6 @@ ANNOTATION(header_unit)
 #undef TYPE_TRAIT_2
 #undef TYPE_TRAIT_1
 #undef TYPE_TRAIT
-#undef CONCEPTS_KEYWORD
 #undef CXX20_KEYWORD
 #undef CXX11_KEYWORD
 #undef KEYWORD

diff  --git a/clang/lib/Basic/IdentifierTable.cpp 
b/clang/lib/Basic/IdentifierTable.cpp
index 82cee4aa052dd..4bf9e12af83ca 100644
--- a/clang/lib/Basic/IdentifierTable.cpp
+++ b/clang/lib/Basic/IdentifierTable.cpp
@@ -82,7 +82,7 @@ IdentifierTable::IdentifierTable(const LangOptions &LangOpts,
 // Constants for TokenKinds.def
 namespace {
 
-  enum {
+  enum TokenKey : unsigned {
 KEYC99= 0x1,
 KEYCXX= 0x2,
 KEYCXX11  = 0x4,
@@ -99,70 +99,146 @@ namespace {
 WCHARSUPPORT  = 0x2000,
 HALFSUPPORT   = 0x4000,
 CHAR8SUPPORT  = 0x8000,
-KEYCONCEPTS   = 0x1,
-KEYOBJC   = 0x2,
-KEYZVECTOR= 0x4,
-KEYCOROUTINES = 0x8,
-KEYMODULES= 0x10,
-KEYCXX20  = 0x20,
-KEYOPENCLCXX  = 0x40,
-KEYMSCOMPAT   = 0x80,
-KEYSYCL   = 0x100,
-KEYCUDA   = 0x200,
+KEYOBJC   = 0x1,
+KEYZVECTOR= 0x2,
+KEYCOROUTINES = 0x4,
+KEYMODULES= 0x8,
+KEYCXX20  = 0x10,
+KEYOPENCLCXX  = 0x20,
+KEYMSCOMPAT   = 0x40,
+KEYSYCL   = 0x80,
+KEYCUDA   = 0x100,
 KEYMAX= KEYCUDA, // The maximum key
 KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20,
 KEYALL = (KEYMAX | (KEYMAX-1)) & ~KEYN

[PATCH] D131007: [NFCI] Refactor how KeywordStatus is calculated

[Erich Keane via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfb65b17932e1: [NFCI] Refactor how KeywordStatus is 
calculated (authored by erichkeane).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D131007?vs=449404&id=449655#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131007

Files:
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Basic/IdentifierTable.cpp
  clang/test/Lexer/keywords_test.cpp

Index: clang/test/Lexer/keywords_test.cpp
===
--- clang/test/Lexer/keywords_test.cpp
+++ clang/test/Lexer/keywords_test.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -std=c++03 -fsyntax-only %s
 // RUN: %clang_cc1 -std=c++11 -DCXX11 -fsyntax-only %s
-// RUN: %clang_cc1 -std=c++2a -DCXX11 -DCXX2A -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++20 -DCXX11 -DCXX20 -fsyntax-only %s
 // RUN: %clang_cc1 -std=c++03 -fdeclspec -DDECLSPEC -fsyntax-only %s
 // RUN: %clang_cc1 -std=c++03 -fms-extensions -DDECLSPEC -fsyntax-only %s
 // RUN: %clang_cc1 -std=c++03 -fborland-extensions -DDECLSPEC -fsyntax-only %s
@@ -19,10 +19,10 @@
 #define NOT_KEYWORD(NAME) _Static_assert(__is_identifier(NAME), #NAME)
 #define IS_TYPE(NAME) void is_##NAME##_type() { int f(NAME); }
 
-#if defined(CXX2A)
-#define CONCEPTS_KEYWORD(NAME)  IS_KEYWORD(NAME)
+#if defined(CXX20)
+#define CXX20_KEYWORD(NAME)  IS_KEYWORD(NAME)
 #else
-#define CONCEPTS_KEYWORD(NAME)  NOT_KEYWORD(NAME)
+#define CXX20_KEYWORD(NAME)  NOT_KEYWORD(NAME)
 #endif
 
 #ifdef DECLSPEC
@@ -59,8 +59,8 @@
 CXX11_KEYWORD(thread_local);
 
 // Concepts keywords
-CONCEPTS_KEYWORD(concept);
-CONCEPTS_KEYWORD(requires);
+CXX20_KEYWORD(concept);
+CXX20_KEYWORD(requires);
 
 // __declspec extension
 DECLSPEC_KEYWORD(__declspec);
Index: clang/lib/Basic/IdentifierTable.cpp
===
--- clang/lib/Basic/IdentifierTable.cpp
+++ clang/lib/Basic/IdentifierTable.cpp
@@ -82,7 +82,7 @@
 // Constants for TokenKinds.def
 namespace {
 
-  enum {
+  enum TokenKey : unsigned {
 KEYC99= 0x1,
 KEYCXX= 0x2,
 KEYCXX11  = 0x4,
@@ -99,70 +99,146 @@
 WCHARSUPPORT  = 0x2000,
 HALFSUPPORT   = 0x4000,
 CHAR8SUPPORT  = 0x8000,
-KEYCONCEPTS   = 0x1,
-KEYOBJC   = 0x2,
-KEYZVECTOR= 0x4,
-KEYCOROUTINES = 0x8,
-KEYMODULES= 0x10,
-KEYCXX20  = 0x20,
-KEYOPENCLCXX  = 0x40,
-KEYMSCOMPAT   = 0x80,
-KEYSYCL   = 0x100,
-KEYCUDA   = 0x200,
+KEYOBJC   = 0x1,
+KEYZVECTOR= 0x2,
+KEYCOROUTINES = 0x4,
+KEYMODULES= 0x8,
+KEYCXX20  = 0x10,
+KEYOPENCLCXX  = 0x20,
+KEYMSCOMPAT   = 0x40,
+KEYSYCL   = 0x80,
+KEYCUDA   = 0x100,
 KEYMAX= KEYCUDA, // The maximum key
 KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20,
 KEYALL = (KEYMAX | (KEYMAX-1)) & ~KEYNOMS18 &
  ~KEYNOOPENCL // KEYNOMS18 and KEYNOOPENCL are used to exclude.
   };
 
-  /// How a keyword is treated in the selected standard.
+  /// How a keyword is treated in the selected standard. This enum is ordered
+  /// intentionally so that the value that 'wins' is the most 'permissive'.
   enum KeywordStatus {
+KS_Unknown, // Not yet calculated. Used when figuring out the status.
 KS_Disabled,// Disabled
+KS_Future,  // Is a keyword in future standard
 KS_Extension,   // Is an extension
 KS_Enabled, // Enabled
-KS_Future   // Is a keyword in future standard
   };
 
 } // namespace
 
+// This works on a single TokenKey flag and checks the LangOpts to get the
+// KeywordStatus based exclusively on this flag, so that it can be merged in
+// getKeywordStatus. Most should be enabled/disabled, but some might imply
+// 'future' versions, or extensions. Returns 'unknown' unless this is KNOWN to
+// be disabled, and the calling function makes it 'disabled' if no other flag
+// changes it. This is necessary for the KEYNOCXX and KEYNOOPENCL flags.
+static KeywordStatus getKeywordStatusHelper(const LangOptions &LangOpts,
+TokenKey Flag) {
+  // Flag is a single bit version of TokenKey (that is, not
+  // KEYALL/KEYALLCXX/etc), so we can check with == throughout this function.
+  assert((Flag & ~(Flag - 1)) == Flag && "Multiple bits set?");
+
+  switch (Flag) {
+  case KEYC99:
+// FIXME: This should have KS_Future logic here, but that can only happen if
+// getFutureCompatDiagKind ALSO gets updated. This is safe, since C mode is
+// ALWAYS implied.
+return LangOpts.C99 ? KS_Enabled : KS_Unknown;
+  case KEYC11:
+// FIXME: This should have KS_Future logic here, but that can only happen if
+// getFut

[PATCH] D130130: [clang-tidy] Add an IgnoreMacros option to readability-avoid-const-params-in-decls

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:99
 
+- The :doc:`readability-avoid-const-params-in-decls
+  ` check does not

Please move it to `Changes in existing checks` section.


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

https://reviews.llvm.org/D130130

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

[PATCH] D131062: [docs] Add "C++20 Modules"

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 449657.
ChuanqiXu added a comment.

Remove the discussion about the difference about macro in clang module and 
header units since we need more discussion about it. And the discussion is not 
necessary about the docs.


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

https://reviews.llvm.org/D131062

Files:
  clang/docs/CPlusPlus20Modules.rst
  clang/docs/index.rst

Index: clang/docs/index.rst
===
--- clang/docs/index.rst
+++ clang/docs/index.rst
@@ -40,6 +40,7 @@
SafeStack
ShadowCallStack
SourceBasedCodeCoverage
+   CPlusPlus20Modules
Modules
MSVCCompatibility
MisExpect
Index: clang/docs/CPlusPlus20Modules.rst
===
--- /dev/null
+++ clang/docs/CPlusPlus20Modules.rst
@@ -0,0 +1,605 @@
+=
+C++20 Modules
+=
+
+.. contents::
+   :local:
+
+Introduction
+
+
+Modules have a lot of meanings. For the users of clang compiler, modules may
+refer to `Objective-C Modules`, `Clang C++ Modules` (or `Clang Header Modules`,
+etc) and C++20 modules. The implementation of all kinds of the modules in clang 
+share a big part of codes. But from the perspective of users, their semantics and
+command line interfaces are very different. So it should be helpful for the users
+to introduce how to use C++20 modules.
+
+There is already a detailed document about clang modules Modules_, it
+should be helpful to read Modules_ if you want to know more about the general
+idea of modules. But due to the C++20 modules having very different semantics, it
+might be more friendly for users who care about C++20 modules only to create a
+new page.
+
+Although the term `modules` has a unique meaning in C++20 Language Specification,
+when people talk about C++20 modules, they may refer to another C++20 feature:
+header units. So this document would try to cover header units too.
+
+C++20 Modules
+=
+
+This document was intended to be pure manual. But it should be helpful to
+introduce some language background here for readers who are not familiar with
+the new language feature. This document is not intended to be a language
+tutorial. The document would only introduce concepts about the the
+structure and building of the project.
+
+Background and terminology
+--
+
+Modules
+~~~
+
+In this document, the term `Modules`/`modules` refers to C++20 modules
+feature if it is not decorated by `clang`.
+
+Clang Modules
+~
+
+In this document, the term `Clang Modules`/`clang modules` refer to clang
+c++ modules extension. It is also known as `clang header modules` and
+`clang module map modules` or `clang c++ modules`.
+
+Module and module unit
+~~
+
+A module consists of one or multiple module units. A module unit is a special
+translation unit. Every module unit should have a module declaration. The syntax
+of the module declaration is:
+
+.. code-block:: c++
+
+  [export] module module_name[:partition_name];
+
+Things in ``[]`` means optional. The syntax of ``module_name`` and ``partition_name``
+in regex form should be ``[a-zA-Z_][a-zA-Z_0-9.]*``. The dot ``.`` in the name has
+no special meaning.
+
+In this document, module units are classified into:
+
+* Primary module interface unit.
+
+* Module implementation unit.
+
+* Module partition interface unit.
+
+* Module partition implementation unit.
+
+A primary module interface unit is a module unit whose module declaration is
+`export module module_name;`. The `module_name` here denotes the name of the
+module. A module should have one and only primary module interface unit.
+
+A module implementation unit is a module unit whose module declaration is
+`module module_name;`. A module could have multiple module implementation
+units with the same declaration.
+
+A module partition interface unit is a module unit whose module declaration is
+`export module module_name:partition_name;`. The `partition_name` should be
+unique to the module.
+
+A module partition implementation unit is a module unit whose module declaration
+is `module module_name:partition_name;`. The `partition_name` should be
+unique to the module.
+
+In this document, we call `primary module interface unit` and
+`module partition interface unit` as `module interface unit`. We call `module
+interface unit` and `module partition implementation unit` as
+`importable module unit`. We call `module partition interface unit` and
+`module partition implementation unit` as `module partition unit`.
+
+Module file
+~~~
+
+A module file stands for the precompiled result of an importable module unit.
+
+Global module fragment
+~~
+
+In a module unit, the section from `module;` to the module declaration is called the global module fragment.
+
+
+How to build projects using modules
+--

[PATCH] D129353: [clang-tidy] Improve check cert-dcl58-cpp.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

This appears to be causing a crash with friend declarations. The crash happens 
during matching, see https://github.com/llvm/llvm-project/issues/56902


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129353

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

[PATCH] D131039: [clang][dataflow] Add cache of `ControlFlowContext`s for function decls.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 449658.
ymandel marked 2 inline comments as done.
ymandel added a comment.

update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131039

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  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
@@ -507,28 +507,30 @@
 return;
   Env.setStorageLocation(*S, *ArgLoc);
 } else if (const FunctionDecl *F = S->getDirectCallee()) {
-  // This case is for context-sensitive analysis, which we only do if we
-  // have the callee body available in the translation unit.
-  if (!Options.ContextSensitive || F->getBody() == nullptr)
+  // This case is for context-sensitive analysis.
+  if (!Options.ContextSensitive)
+return;
+
+  const ControlFlowContext *CFCtx = Env.getControlFlowContext(F);
+  if (!CFCtx)
 return;
 
   // FIXME: We don't support context-sensitive analysis of recursion, so
   // we should return early here if `F` is the same as the `FunctionDecl`
   // holding `S` itself.
 
-  auto &ASTCtx = F->getASTContext();
-
-  // FIXME: Cache these CFGs.
-  auto CFCtx = ControlFlowContext::build(F, F->getBody(), &ASTCtx);
-  // FIXME: Handle errors here and below.
-  assert(CFCtx);
   auto ExitBlock = CFCtx->getCFG().getExit().getBlockID();
 
   auto CalleeEnv = Env.pushCall(S);
 
-  // FIXME: Use the same analysis as the caller for the callee.
-  DataflowAnalysisOptions Options;
-  auto Analysis = NoopAnalysis(ASTCtx, Options);
+  // FIXME: Use the same analysis as the caller for the callee. Note,
+  // though, that doing so would require support for changing the analysis's
+  // ASTContext.
+  assert(
+  CFCtx->getDecl() != nullptr &&
+  "ControlFlowContexts in the environment should always carry a decl");
+  auto Analysis = NoopAnalysis(CFCtx->getDecl()->getASTContext(),
+   DataflowAnalysisOptions());
 
   auto BlockToOutputState =
   dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv);
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -335,6 +335,27 @@
   llvm::dbgs() << debugString(Constraints, AtomNames);
 }
 
+const ControlFlowContext *
+DataflowAnalysisContext::getControlFlowContext(const FunctionDecl *F) {
+  // Canonicalize the key:
+  F = F->getDefinition();
+  if (F == nullptr)
+return nullptr;
+  auto It = FunctionContexts.find(F);
+  if (It != FunctionContexts.end())
+return &It->second;
+
+  if (Stmt *Body = F->getBody()) {
+auto CFCtx = ControlFlowContext::build(F, *Body, F->getASTContext());
+// FIXME: Handle errors.
+assert(CFCtx);
+auto Result = FunctionContexts.insert({F, std::move(*CFCtx)});
+return &Result.first->second;
+  }
+
+  return nullptr;
+}
+
 } // namespace dataflow
 } // namespace clang
 
Index: clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -45,7 +45,7 @@
 }
 
 llvm::Expected
-ControlFlowContext::build(const Decl *D, Stmt *S, ASTContext *C) {
+ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) {
   CFG::BuildOptions Options;
   Options.PruneTriviallyFalseEdges = false;
   Options.AddImplicitDtors = true;
@@ -56,7 +56,7 @@
   // Ensure that all sub-expressions in basic blocks are evaluated.
   Options.setAllAlwaysAdd();
 
-  auto Cfg = CFG::buildCFG(D, S, C, Options);
+  auto Cfg = CFG::buildCFG(D, &S, &C, Options);
   if (Cfg == nullptr)
 return llvm::createStringError(
 std::make_error_code(std::errc::invalid_argument),
@@ -64,7 +64,14 @@
 
   llvm::DenseMap StmtToBlock =
   buildStmtToBasicBlockMap(*Cfg);
-  return ControlFlowContext(std::move(Cfg), std::move(StmtToBlock));
+  return ControlFlowContext(D, std::move(Cfg), std::move(StmtToBlock));
+}
+
+llvm::Expected
+ControlFlowContext::build(const Decl *D, Stmt *S, ASTContext *C) {
+  assert(S != nullptr);
+  assert(C != nullptr);
+  return bui

[PATCH] D131039: [clang][dataflow] Add cache of `ControlFlowContext`s for function decls.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 449660.
ymandel added a comment.

tweaks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131039

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  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
@@ -507,28 +507,30 @@
 return;
   Env.setStorageLocation(*S, *ArgLoc);
 } else if (const FunctionDecl *F = S->getDirectCallee()) {
-  // This case is for context-sensitive analysis, which we only do if we
-  // have the callee body available in the translation unit.
-  if (!Options.ContextSensitive || F->getBody() == nullptr)
+  // This case is for context-sensitive analysis.
+  if (!Options.ContextSensitive)
+return;
+
+  const ControlFlowContext *CFCtx = Env.getControlFlowContext(F);
+  if (!CFCtx)
 return;
 
   // FIXME: We don't support context-sensitive analysis of recursion, so
   // we should return early here if `F` is the same as the `FunctionDecl`
   // holding `S` itself.
 
-  auto &ASTCtx = F->getASTContext();
-
-  // FIXME: Cache these CFGs.
-  auto CFCtx = ControlFlowContext::build(F, F->getBody(), &ASTCtx);
-  // FIXME: Handle errors here and below.
-  assert(CFCtx);
   auto ExitBlock = CFCtx->getCFG().getExit().getBlockID();
 
   auto CalleeEnv = Env.pushCall(S);
 
-  // FIXME: Use the same analysis as the caller for the callee.
-  DataflowAnalysisOptions Options;
-  auto Analysis = NoopAnalysis(ASTCtx, Options);
+  // FIXME: Use the same analysis as the caller for the callee. Note,
+  // though, that doing so would require support for changing the analysis's
+  // ASTContext.
+  assert(
+  CFCtx->getDecl() != nullptr &&
+  "ControlFlowContexts in the environment should always carry a decl");
+  auto Analysis = NoopAnalysis(CFCtx->getDecl()->getASTContext(),
+   DataflowAnalysisOptions());
 
   auto BlockToOutputState =
   dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv);
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -335,6 +335,27 @@
   llvm::dbgs() << debugString(Constraints, AtomNames);
 }
 
+const ControlFlowContext *
+DataflowAnalysisContext::getControlFlowContext(const FunctionDecl *F) {
+  // Canonicalize the key:
+  F = F->getDefinition();
+  if (F == nullptr)
+return nullptr;
+  auto It = FunctionContexts.find(F);
+  if (It != FunctionContexts.end())
+return &It->second;
+
+  if (Stmt *Body = F->getBody()) {
+auto CFCtx = ControlFlowContext::build(F, *Body, F->getASTContext());
+// FIXME: Handle errors.
+assert(CFCtx);
+auto Result = FunctionContexts.insert({F, std::move(*CFCtx)});
+return &Result.first->second;
+  }
+
+  return nullptr;
+}
+
 } // namespace dataflow
 } // namespace clang
 
Index: clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -45,7 +45,7 @@
 }
 
 llvm::Expected
-ControlFlowContext::build(const Decl *D, Stmt *S, ASTContext *C) {
+ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) {
   CFG::BuildOptions Options;
   Options.PruneTriviallyFalseEdges = false;
   Options.AddImplicitDtors = true;
@@ -56,7 +56,7 @@
   // Ensure that all sub-expressions in basic blocks are evaluated.
   Options.setAllAlwaysAdd();
 
-  auto Cfg = CFG::buildCFG(D, S, C, Options);
+  auto Cfg = CFG::buildCFG(D, &S, &C, Options);
   if (Cfg == nullptr)
 return llvm::createStringError(
 std::make_error_code(std::errc::invalid_argument),
@@ -64,7 +64,14 @@
 
   llvm::DenseMap StmtToBlock =
   buildStmtToBasicBlockMap(*Cfg);
-  return ControlFlowContext(std::move(Cfg), std::move(StmtToBlock));
+  return ControlFlowContext(D, std::move(Cfg), std::move(StmtToBlock));
+}
+
+llvm::Expected
+ControlFlowContext::build(const Decl *D, Stmt *S, ASTContext *C) {
+  assert(S != nullptr);
+  assert(C != nullptr);
+  return build(D, *S, *C);
 }
 
 } // namespace datafl

[PATCH] D131039: [clang][dataflow] Add cache of `ControlFlowContext`s for function decls.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:34
+  /// Builds a ControlFlowContext from an AST node. `D` is the function in 
which
+  /// `S` resides. All arguments must be non-null.
   static llvm::Expected build(const Decl *D, Stmt *S,

sgatev wrote:
> ymandel wrote:
> > sgatev wrote:
> > > Can we make them references?
> > Turns out I was wrong - `D` can be null. Would that CFG has comments about 
> > what its parameters do... We have a test that excercises this (though I 
> > can't say why):
> > 
> > https://github.com/llvm/llvm-project/blob/9a976f36615dbe15e76c12b22f711b2e597a8e51/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp#L70
> > 
> > The other two -- yes -- but then we'd have to update the various callers as 
> > well. I'd rather not do that in this patch, but I'll add the new overload 
> > and we can follow up with cleanups in another patch.
> > 
> > 
> Weird. I think we should change the test to pass the decl.
I'd like that, but when looking through other uses found a bunch that pass 
nullptr (and for no apparent reason - they all have the function decl handy).

So, I propose that we mark the new API as such and then in the cleanup we'll 
make sure to pass a non-null argument.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:19
 #include "clang/Analysis/FlowSensitive/Value.h"
+#include "clang/Basic/IdentifierTable.h"
 #include "llvm/Support/Debug.h"

sgatev wrote:
> Is this still needed?
Nope.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131039

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

[PATCH] D131039: [clang][dataflow] Add cache of `ControlFlowContext`s for function decls.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 449661.
ymandel added a comment.

comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131039

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  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
@@ -507,28 +507,30 @@
 return;
   Env.setStorageLocation(*S, *ArgLoc);
 } else if (const FunctionDecl *F = S->getDirectCallee()) {
-  // This case is for context-sensitive analysis, which we only do if we
-  // have the callee body available in the translation unit.
-  if (!Options.ContextSensitive || F->getBody() == nullptr)
+  // This case is for context-sensitive analysis.
+  if (!Options.ContextSensitive)
+return;
+
+  const ControlFlowContext *CFCtx = Env.getControlFlowContext(F);
+  if (!CFCtx)
 return;
 
   // FIXME: We don't support context-sensitive analysis of recursion, so
   // we should return early here if `F` is the same as the `FunctionDecl`
   // holding `S` itself.
 
-  auto &ASTCtx = F->getASTContext();
-
-  // FIXME: Cache these CFGs.
-  auto CFCtx = ControlFlowContext::build(F, F->getBody(), &ASTCtx);
-  // FIXME: Handle errors here and below.
-  assert(CFCtx);
   auto ExitBlock = CFCtx->getCFG().getExit().getBlockID();
 
   auto CalleeEnv = Env.pushCall(S);
 
-  // FIXME: Use the same analysis as the caller for the callee.
-  DataflowAnalysisOptions Options;
-  auto Analysis = NoopAnalysis(ASTCtx, Options);
+  // FIXME: Use the same analysis as the caller for the callee. Note,
+  // though, that doing so would require support for changing the analysis's
+  // ASTContext.
+  assert(
+  CFCtx->getDecl() != nullptr &&
+  "ControlFlowContexts in the environment should always carry a decl");
+  auto Analysis = NoopAnalysis(CFCtx->getDecl()->getASTContext(),
+   DataflowAnalysisOptions());
 
   auto BlockToOutputState =
   dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv);
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -335,6 +335,27 @@
   llvm::dbgs() << debugString(Constraints, AtomNames);
 }
 
+const ControlFlowContext *
+DataflowAnalysisContext::getControlFlowContext(const FunctionDecl *F) {
+  // Canonicalize the key:
+  F = F->getDefinition();
+  if (F == nullptr)
+return nullptr;
+  auto It = FunctionContexts.find(F);
+  if (It != FunctionContexts.end())
+return &It->second;
+
+  if (Stmt *Body = F->getBody()) {
+auto CFCtx = ControlFlowContext::build(F, *Body, F->getASTContext());
+// FIXME: Handle errors.
+assert(CFCtx);
+auto Result = FunctionContexts.insert({F, std::move(*CFCtx)});
+return &Result.first->second;
+  }
+
+  return nullptr;
+}
+
 } // namespace dataflow
 } // namespace clang
 
Index: clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -45,7 +45,7 @@
 }
 
 llvm::Expected
-ControlFlowContext::build(const Decl *D, Stmt *S, ASTContext *C) {
+ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) {
   CFG::BuildOptions Options;
   Options.PruneTriviallyFalseEdges = false;
   Options.AddImplicitDtors = true;
@@ -56,7 +56,7 @@
   // Ensure that all sub-expressions in basic blocks are evaluated.
   Options.setAllAlwaysAdd();
 
-  auto Cfg = CFG::buildCFG(D, S, C, Options);
+  auto Cfg = CFG::buildCFG(D, &S, &C, Options);
   if (Cfg == nullptr)
 return llvm::createStringError(
 std::make_error_code(std::errc::invalid_argument),
@@ -64,7 +64,14 @@
 
   llvm::DenseMap StmtToBlock =
   buildStmtToBasicBlockMap(*Cfg);
-  return ControlFlowContext(std::move(Cfg), std::move(StmtToBlock));
+  return ControlFlowContext(D, std::move(Cfg), std::move(StmtToBlock));
+}
+
+llvm::Expected
+ControlFlowContext::build(const Decl *D, Stmt *S, ASTContext *C) {
+  assert(S != nullptr);
+  assert(C != nullptr);
+  return build(D, *S, *C);
 }
 
 } // namespace dataf

[PATCH] D131007: [NFCI] Refactor how KeywordStatus is calculated

[Pavel Labath via Phabricator via cfe-commits]

labath added a comment.

This seems to break lldb tests, specifically the compilation of this 

 source file.

The compile fails with an assertion (`Keyword not known to come from a newer 
Standard or proposed Standard`), so I am assuming that change was not 
intentional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131007

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

[PATCH] D131007: [NFCI] Refactor how KeywordStatus is calculated

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D131007#3696486 , @labath wrote:

> This seems to break lldb tests, specifically the compilation of this 
> 
>  source file.
>
> The compile fails with an assertion (`Keyword not known to come from a newer 
> Standard or proposed Standard`), so I am assuming that change was not 
> intentional.

It was not, thanks for the heads up.  Looking into it now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131007

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

[clang] bf6db18 - Fix char8_t in C mode regression from fb65b179

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2022-08-03T07:15:30-07:00
New Revision: bf6db18e52815475baebff2c330763fedac6b5e4

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

LOG: Fix char8_t in C mode regression from fb65b179

When doing that NFC refactor, I'd messed up how char8_t was reported,
which resulted in it being considered a 'future' keyword, without the
corresponding diagnostic, which lead to an assert.  This patch corrects
the char8_t to ONLY be future in C++ mode.

Added: 


Modified: 
clang/lib/Basic/IdentifierTable.cpp

Removed: 




diff  --git a/clang/lib/Basic/IdentifierTable.cpp 
b/clang/lib/Basic/IdentifierTable.cpp
index 4bf9e12af83c..06f0850430f2 100644
--- a/clang/lib/Basic/IdentifierTable.cpp
+++ b/clang/lib/Basic/IdentifierTable.cpp
@@ -179,7 +179,8 @@ static KeywordStatus getKeywordStatusHelper(const 
LangOptions &LangOpts,
   case CHAR8SUPPORT:
 if (LangOpts.Char8) return KS_Enabled;
 if (LangOpts.CPlusPlus20) return KS_Unknown;
-return KS_Future;
+if (LangOpts.CPlusPlus) return KS_Future;
+return KS_Unknown;
   case KEYOBJC:
 // We treat bridge casts as objective-C keywords so we can warn on them
 // in non-arc mode.



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

[PATCH] D131007: [NFCI] Refactor how KeywordStatus is calculated

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D131007#3696489 , @erichkeane 
wrote:

> In D131007#3696486 , @labath wrote:
>
>> This seems to break lldb tests, specifically the compilation of this 
>> 
>>  source file.
>>
>> The compile fails with an assertion (`Keyword not known to come from a newer 
>> Standard or proposed Standard`), so I am assuming that change was not 
>> intentional.
>
> It was not, thanks for the heads up.  Looking into it now.

I see the problem now, patch incoming as soon as I validate it locally.  
char8_t was mis-reported as 'future' in C mode, so I was missing a condition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131007

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

[PATCH] D131007: [NFCI] Refactor how KeywordStatus is calculated

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D131007#3696495 , @erichkeane 
wrote:

> In D131007#3696489 , @erichkeane 
> wrote:
>
>> In D131007#3696486 , @labath wrote:
>>
>>> This seems to break lldb tests, specifically the compilation of this 
>>> 
>>>  source file.
>>>
>>> The compile fails with an assertion (`Keyword not known to come from a 
>>> newer Standard or proposed Standard`), so I am assuming that change was not 
>>> intentional.
>>
>> It was not, thanks for the heads up.  Looking into it now.
>
> I see the problem now, patch incoming as soon as I validate it locally.  
> char8_t was mis-reported as 'future' in C mode, so I was missing a condition.

Fix committed in: bf6db18e52815475baebff2c330763fedac6b5e4 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131007

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

[PATCH] D130894: [clang] Print more information about failed static assertions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

FWIW, complex numbers are already covered it seems:

  test.cpp:35:1: error: static assertion failed due to requirement '__real c == 
__imag c'
  static_assert(__real c == __imag c);
  ^ 
  test.cpp:35:24: note: expression evaluates to 5 == 6
  static_assert(__real c == __imag c);
~^~~
  1 error generated.


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

https://reviews.llvm.org/D130894

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

[PATCH] D131065: [clang][dataflow] Store DeclContext of block being analysed in Environment if available.

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 449664.
wyt added a comment.

Update DeclCtx when pushing/popping calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131065

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  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
@@ -534,7 +534,9 @@
   auto ExitState = (*BlockToOutputState)[ExitBlock];
   assert(ExitState);
 
-  Env = ExitState->Env;
+  auto *Caller = Env.getDeclCtx();
+  Env = Environment(ExitState->Env);
+  Env.setDeclCtx(Caller);
 }
   }
 
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -154,7 +154,7 @@
 : DACtx(&DACtx), FlowConditionToken(&DACtx.makeFlowConditionToken()) {}
 
 Environment::Environment(const Environment &Other)
-: DACtx(Other.DACtx), DeclToLoc(Other.DeclToLoc),
+: DACtx(Other.DACtx), DeclCtx(Other.DeclCtx), DeclToLoc(Other.DeclToLoc),
   ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
   MemberLocToStruct(Other.MemberLocToStruct),
   FlowConditionToken(&DACtx->forkFlowCondition(*Other.FlowConditionToken)) {
@@ -167,9 +167,11 @@
 }
 
 Environment::Environment(DataflowAnalysisContext &DACtx,
- const DeclContext &DeclCtx)
+ const DeclContext &DeclCtxArg)
 : Environment(DACtx) {
-  if (const auto *FuncDecl = dyn_cast(&DeclCtx)) {
+  setDeclCtx(&DeclCtxArg);
+
+  if (const auto *FuncDecl = dyn_cast(DeclCtx)) {
 assert(FuncDecl->getBody() != nullptr);
 initGlobalVars(*FuncDecl->getBody(), *this);
 for (const auto *ParamDecl : FuncDecl->parameters()) {
@@ -181,7 +183,7 @@
 }
   }
 
-  if (const auto *MethodDecl = dyn_cast(&DeclCtx)) {
+  if (const auto *MethodDecl = dyn_cast(DeclCtx)) {
 auto *Parent = MethodDecl->getParent();
 assert(Parent != nullptr);
 if (Parent->isLambda())
@@ -214,6 +216,8 @@
   const auto *FuncDecl = Call->getDirectCallee();
   assert(FuncDecl != nullptr);
   assert(FuncDecl->getBody() != nullptr);
+
+  Env.setDeclCtx(FuncDecl);
   // FIXME: In order to allow the callee to reference globals, we probably need
   // to call `initGlobalVars` here in some way.
 
@@ -268,12 +272,14 @@
 
 LatticeJoinEffect Environment::join(const Environment &Other,
 Environment::ValueModel &Model) {
-  assert(DACtx == Other.DACtx);
+  assert(DACtx == Other.DACtx && DeclCtx == Other.DeclCtx);
 
   auto Effect = LatticeJoinEffect::Unchanged;
 
   Environment JoinedEnv(*DACtx);
 
+  JoinedEnv.setDeclCtx(DeclCtx);
+
   JoinedEnv.DeclToLoc = intersectDenseMaps(DeclToLoc, Other.DeclToLoc);
   if (DeclToLoc.size() != JoinedEnv.DeclToLoc.size())
 Effect = LatticeJoinEffect::Changed;
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -167,6 +167,13 @@
   LatticeJoinEffect join(const Environment &Other,
  Environment::ValueModel &Model);
 
+  /// Returns the `DeclCtx` of the block being analysed if provided, otherwise
+  /// returns nullptr.
+  const DeclContext *getDeclCtx() { return DeclCtx; }
+
+  /// Sets the `DeclCtx` of the block being analysed.
+  void setDeclCtx(const DeclContext *Ctx) { DeclCtx = Ctx; }
+
   // FIXME: Rename `createOrGetStorageLocation` to `getOrCreateStorageLocation`,
   // `getStableStorageLocation`, or something more appropriate.
 
@@ -363,6 +370,9 @@
   // `DACtx` is not null and not owned by this object.
   DataflowAnalysisContext *DACtx;
 
+  // `DeclCtx` of the block being analysed if provided.
+  const DeclContext *DeclCtx;
+
   // Maps from program declarations and statements to storage locations that are
   // assigned to them. Unlike the maps in `DataflowAnalysisContext`, these
   // include only storage locations that are in scope for a particular basic
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D102107#3685694 , @dhruvachak 
wrote:

> @jdoerfert With this patch, additional remarks are being generated. Please 
> check whether the new OMP121 remarks in the following tests are OK.
>
> Clang :: OpenMP/remarks_parallel_in_multiple_target_state_machines.c
> Clang :: OpenMP/remarks_parallel_in_target_state_machine.c

Can you send me the device IR generated for these (-save-temps). I need to 
check what's happening and building the patch myself will take a while.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102107

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

[PATCH] D131065: [clang][dataflow] Store DeclContext of block being analysed in Environment if available.

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 449666.
wyt marked 3 inline comments as done.
wyt added a comment.

Formatting fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131065

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  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
@@ -534,7 +534,9 @@
   auto ExitState = (*BlockToOutputState)[ExitBlock];
   assert(ExitState);
 
-  Env = ExitState->Env;
+  auto *Caller = Env.getDeclCtx();
+  Env = Environment(ExitState->Env);
+  Env.setDeclCtx(Caller);
 }
   }
 
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -154,7 +154,7 @@
 : DACtx(&DACtx), FlowConditionToken(&DACtx.makeFlowConditionToken()) {}
 
 Environment::Environment(const Environment &Other)
-: DACtx(Other.DACtx), DeclToLoc(Other.DeclToLoc),
+: DACtx(Other.DACtx), DeclCtx(Other.DeclCtx), DeclToLoc(Other.DeclToLoc),
   ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
   MemberLocToStruct(Other.MemberLocToStruct),
   FlowConditionToken(&DACtx->forkFlowCondition(*Other.FlowConditionToken)) {
@@ -167,9 +167,11 @@
 }
 
 Environment::Environment(DataflowAnalysisContext &DACtx,
- const DeclContext &DeclCtx)
+ const DeclContext &DeclCtxArg)
 : Environment(DACtx) {
-  if (const auto *FuncDecl = dyn_cast(&DeclCtx)) {
+  setDeclCtx(&DeclCtxArg);
+
+  if (const auto *FuncDecl = dyn_cast(DeclCtx)) {
 assert(FuncDecl->getBody() != nullptr);
 initGlobalVars(*FuncDecl->getBody(), *this);
 for (const auto *ParamDecl : FuncDecl->parameters()) {
@@ -181,7 +183,7 @@
 }
   }
 
-  if (const auto *MethodDecl = dyn_cast(&DeclCtx)) {
+  if (const auto *MethodDecl = dyn_cast(DeclCtx)) {
 auto *Parent = MethodDecl->getParent();
 assert(Parent != nullptr);
 if (Parent->isLambda())
@@ -214,6 +216,8 @@
   const auto *FuncDecl = Call->getDirectCallee();
   assert(FuncDecl != nullptr);
   assert(FuncDecl->getBody() != nullptr);
+
+  Env.setDeclCtx(FuncDecl);
   // FIXME: In order to allow the callee to reference globals, we probably need
   // to call `initGlobalVars` here in some way.
 
@@ -268,12 +272,14 @@
 
 LatticeJoinEffect Environment::join(const Environment &Other,
 Environment::ValueModel &Model) {
-  assert(DACtx == Other.DACtx);
+  assert(DACtx == Other.DACtx && DeclCtx == Other.DeclCtx);
 
   auto Effect = LatticeJoinEffect::Unchanged;
 
   Environment JoinedEnv(*DACtx);
 
+  JoinedEnv.setDeclCtx(DeclCtx);
+
   JoinedEnv.DeclToLoc = intersectDenseMaps(DeclToLoc, Other.DeclToLoc);
   if (DeclToLoc.size() != JoinedEnv.DeclToLoc.size())
 Effect = LatticeJoinEffect::Changed;
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -167,6 +167,13 @@
   LatticeJoinEffect join(const Environment &Other,
  Environment::ValueModel &Model);
 
+  /// Returns the `DeclContext` of the block being analysed if provided,
+  /// otherwise returns nullptr.
+  const DeclContext *getDeclCtx() { return DeclCtx; }
+
+  /// Sets the `DeclContext` of the block being analysed.
+  void setDeclCtx(const DeclContext *Ctx) { DeclCtx = Ctx; }
+
   // FIXME: Rename `createOrGetStorageLocation` to `getOrCreateStorageLocation`,
   // `getStableStorageLocation`, or something more appropriate.
 
@@ -363,6 +370,9 @@
   // `DACtx` is not null and not owned by this object.
   DataflowAnalysisContext *DACtx;
 
+  // `DeclContext` of the block being analysed if provided.
+  const DeclContext *DeclCtx;
+
   // Maps from program declarations and statements to storage locations that are
   // assigned to them. Unlike the maps in `DataflowAnalysisContext`, these
   // include only storage locations that are in scope for a particular basic
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131065: [clang][dataflow] Store DeclContext of block being analysed in Environment if available.

[weiyi via Phabricator via cfe-commits]

wyt added a reviewer: samestep.
wyt removed a subscriber: samestep.
wyt added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:170-172
+ const DeclContext &DeclCtxArg)
 : Environment(DACtx) {
+  DeclCtx = &DeclCtxArg;

sgatev wrote:
> 
Unfortunately doesn't work, I get a warning about "An initializer for a 
delegating constructor must appear alone".



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131065

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

[PATCH] D130600: [clang][dataflow] Handle return statements

[Sam Estep via Phabricator via cfe-commits]

samestep updated this revision to Diff 449667.
samestep added a comment.

Rebase and address all comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130600

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

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -4121,4 +4121,112 @@
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
 }
 
+TEST(TransferTest, ContextSensitiveReturnVoid) {
+  std::string Code = R"(
+void Noop() { return; }
+
+void target() {
+  Noop();
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+// This just tests that the analysis doesn't crash.
+  },
+  {/*.ApplyBuiltinTransfer=*/true,
+   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+}
+
+TEST(TransferTest, ContextSensitiveReturnTrue) {
+  std::string Code = R"(
+bool GiveBool() { return true; }
+
+void target() {
+  bool Foo = GiveBool();
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+auto &FooVal =
+*cast(Env.getValue(*FooDecl, SkipPast::None));
+EXPECT_TRUE(Env.flowConditionImplies(FooVal));
+  },
+  {/*.ApplyBuiltinTransfer=*/true,
+   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+}
+
+TEST(TransferTest, ContextSensitiveReturnFalse) {
+  std::string Code = R"(
+bool GiveBool() { return false; }
+
+void target() {
+  bool Foo = GiveBool();
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+auto &FooVal =
+*cast(Env.getValue(*FooDecl, SkipPast::None));
+EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(FooVal)));
+  },
+  {/*.ApplyBuiltinTransfer=*/true,
+   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+}
+
+TEST(TransferTest, ContextSensitiveReturnArg) {
+  std::string Code = R"(
+bool GiveBool();
+bool GiveBack(bool Arg) { return Arg; }
+
+void target() {
+  bool Foo = GiveBool();
+  bool Bar = GiveBack(Foo);
+  bool Baz = Foo == Bar;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+auto &BazVal =
+*cast(Env.getValue(*BazDecl, SkipPast::None));
+EXPECT_TRUE(Env.flowConditionImplies(BazVal));
+  },
+  {/*.ApplyBuiltinTransfer=*/true,
+   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -332,6 +332,24 @@
   std::make_unique(*ThisPointeeLoc)));
   }
 
+  void VisitReturnStmt(const ReturnStmt *S) {
+auto *Ret = S->getRetValue();
+if (Ret == nullptr)
+  return;
+
+auto *Val = Env.getValue(*Ret, SkipPast::None);
+if (Val == nullptr)
+  return;
+
+// FIXME: Support reference-t

[PATCH] D130705: [clang][ASTImporter] Improve import of functions with auto return type.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 449668.
balazske added a comment.

- Added a new test
- Call the visitor only if the function has auto return type


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130705

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -6318,64 +6318,195 @@
   EXPECT_EQ(ToEPI.ExceptionSpec.SourceDecl, ToCtor);
 }
 
-struct ImportAutoFunctions : ASTImporterOptionSpecificTestBase {};
+struct ImportAutoFunctions : ASTImporterOptionSpecificTestBase {
+  void testImport(llvm::StringRef Code, clang::TestLanguage Lang = Lang_CXX14) {
+Decl *FromTU = getTuDecl(Code, Lang, "input0.cc");
+FunctionDecl *From = FirstDeclMatcher().match(
+FromTU, functionDecl(hasName("foo")));
+
+FunctionDecl *To = Import(From, Lang);
+EXPECT_TRUE(To);
+// We check here only that the type is auto type.
+// These tests are to verify that no crash happens.
+// The crash possibility is the presence of a reference to a declaration
+// in the function's body from the return type, if the function has auto
+// return type.
+EXPECT_TRUE(isa(To->getReturnType()));
+  }
+};
+
+TEST_P(ImportAutoFunctions, ReturnWithAutoUnresolvedArg) {
+  testImport(
+  R"(
+  template
+  auto foo() {
+return 22;
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithTemplateTemplateArg) {
+  // FIXME: Is it possible to have the template arg inside the function?
+  testImport(
+  R"(
+  template struct Tmpl {};
+  template class> struct TmplTmpl {};
+  auto foo() {
+return TmplTmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithDeclarationTemplateArg) {
+  // FIXME: Is it possible to have the template arg inside the function?
+  testImport(
+  R"(
+  template struct Tmpl {};
+  int A[10];
+  auto foo() {
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithNullPtrTemplateArg) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+constexpr int* A = nullptr;
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithIntegralTemplateArg) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+using Int = int;
+constexpr Int A = 7;
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithDecltypeTypeDeclaredInside) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+struct X {};
+X x;
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithUsingTypeDeclaredInside) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  namespace A { struct X {}; }
+  auto foo() {
+using A::X;
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithArrayTypeDeclaredInside) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+struct X {};
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithArraySizeExprDeclaredInside) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+constexpr int S = 10;
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithPackArgDeclaredInside) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+using X = bool;
+return Tmpl();
+  }
+  )");
+}
 
 TEST_P(ImportAutoFunctions, ReturnWithTemplateWithIntegerArgDeclaredInside) {
-  Decl *FromTU = getTuDecl(
+  testImport(
   R"(
   template struct Tmpl {};
   auto foo() {
 constexpr int X = 1;
 return Tmpl();
   }
-  )",
-  Lang_CXX14, "input0.cc");
-  FunctionDecl *From = FirstDeclMatcher().match(
-  FromTU, functionDecl(hasName("foo")));
+  )");
+}
 
-  FunctionDecl *To = Import(From, Lang_CXX14);
-  EXPECT_TRUE(To);
-  EXPECT_TRUE(isa(To->getReturnType()));
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithPtrToStructDeclaredInside) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+struct X {};
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithRefToStructDeclaredInside) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  struct X {};
+  auto foo() {
+using Y = X;
+return Tmpl();
+  }
+  )");
 }
 
 TEST_P(ImportAutoFunctions, ReturnWithTemplateWithStructDeclare

[PATCH] D130600: [clang][dataflow] Handle return statements

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel accepted this revision.
ymandel added inline comments.
This revision is now accepted and ready to land.



Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:134
   StorageLocation *ThisPointeeLoc = nullptr;
+  StorageLocation *ReturnLoc = nullptr;
 };

This looks optional (since it is a pointer). If so, please comment to explain.



Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:224-225
 
+  /// Returns the storage location of the return value or null if it was not 
set
+  /// yet.
+  StorageLocation *getReturnStorageLocation() const;

This implies a timing/initialization aspect to its state. Is this necessary? If 
not, can we restructure to avoid it? Alternatively, if it is valid to have it 
unset, please update the comment to reflect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130600

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

[PATCH] D130894: [clang] Print more information about failed static assertions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 449669.
tbaeder added a comment.

Ignore what I said, they are supported now though.


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

https://reviews.llvm.org/D130894

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/init-capture.cpp
  clang/test/Lexer/cxx1z-trigraphs.cpp
  clang/test/PCH/cxx-templates.cpp
  clang/test/Parser/objc-static-assert.mm
  clang/test/Sema/static-assert.c
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/static-assert-cxx17.cpp
  clang/test/SemaCXX/static-assert.cpp
  clang/test/SemaTemplate/instantiate-var-template.cpp
  clang/test/SemaTemplate/instantiation-dependence.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
@@ -184,7 +184,8 @@
 
 namespace Diags {
   struct A { int n, m; };
-  template struct X { static_assert(a.n == a.m); }; // expected-error {{static assertion failed due to requirement 'Diags::A{1, 2}.n == Diags::A{1, 2}.m'}}
+  template struct X { static_assert(a.n == a.m); }; // expected-error {{static assertion failed due to requirement 'Diags::A{1, 2}.n == Diags::A{1, 2}.m'}} \
+ // expected-note {{evaluates to 1 == 2}}
   template struct X; // expected-note {{in instantiation of template class 'Diags::X<{1, 2}>' requested here}}
 }
 
Index: clang/test/SemaTemplate/instantiation-dependence.cpp
===
--- clang/test/SemaTemplate/instantiation-dependence.cpp
+++ clang/test/SemaTemplate/instantiation-dependence.cpp
@@ -68,8 +68,10 @@
   struct D : B, C {};
 
   static_assert(trait::specialization == 0);
-  static_assert(trait::specialization == 1); // FIXME expected-error {{failed}}
-  static_assert(trait::specialization == 2); // FIXME expected-error {{failed}}
+  static_assert(trait::specialization == 1); // FIXME expected-error {{failed}} \
+// expected-note {{evaluates to 0}}
+  static_assert(trait::specialization == 2); // FIXME expected-error {{failed}} \
+// expected-note {{evaluates to 0}}
   static_assert(trait::specialization == 0); // FIXME-error {{ambiguous partial specialization}}
 }
 
Index: clang/test/SemaTemplate/instantiate-var-template.cpp
===
--- clang/test/SemaTemplate/instantiate-var-template.cpp
+++ clang/test/SemaTemplate/instantiate-var-template.cpp
@@ -31,7 +31,8 @@
   static_assert(b == 1, ""); // expected-note {{in instantiation of}} expected-error {{not an integral constant}}
 
   template void f() {
-static_assert(a == 0, ""); // expected-error {{static assertion failed due to requirement 'a == 0'}}
+static_assert(a == 0, ""); // expected-error {{static assertion failed due to requirement 'a == 0'}} \
+   // expected-note {{evaluates to 1}}
   }
 }
 
Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -22,7 +22,8 @@
 T<2> t2;
 
 template struct S {
-static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static assertion failed due to requirement 'sizeof(char) > sizeof(char)': Type not big enough!}}
+static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static assertion failed due to requirement 'sizeof(char) > sizeof(char)': Type not big enough!}} \
+ // expected-note {{1 > 1}}
 };
 
 S s1; // expected-note {{in instantiation of template class 'S' requested here}}
@@ -215,3 +216,40 @@
 static_assert(constexprNotBool, "message"); // expected-error {{value of type 'const NotBool' is not contextually convertible to 'bool'}}
 
 static_assert(1 , "") // expected-error {{expected ';' after 'static_assert'}}
+
+
+namespace Diagnostics {
+  /// No notes for literals.
+  static_assert(false, ""); // expected-error {{failed}}
+  static_assert(1.0 > 2.0, ""); // expected-error {{failed}}
+  static_assert('c' == 'd', ""); // expected-error {{failed}}
+  static_assert(1 == 2, ""); // expected-error {{failed}}
+
+  /// Simple things are ignored.
+  static_assert(1 == (-(1)), ""); //expected-error {{failed}}
+
+  /// Chars are printed as chars.
+  constexpr char getChar() {
+return 'c';
+ 

[PATCH] D130600: [clang][dataflow] Handle return statements

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Please ignore my previous comments from an earlier revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130600

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

[PATCH] D130894: [clang] Print more information about failed static assertions

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

In D130894#3696534 , @tbaeder wrote:

> FWIW, complex numbers are already covered it seems:
>
>   test.cpp:35:1: error: static assertion failed due to requirement '__real c 
> == __imag c'
>   static_assert(__real c == __imag c);
>   ^ 
>   test.cpp:35:24: note: expression evaluates to 5 == 6
>   static_assert(__real c == __imag c);
> ~^~~
>   1 error generated.

Use ‘5 ==6’ ? So add quotes ..


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

https://reviews.llvm.org/D130894

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

[clang] 692e030 - [clang][dataflow] Add cache of `ControlFlowContext`s for function decls.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2022-08-03T15:17:49Z
New Revision: 692e03039d1ee57c06c0cfae68b91bc5bfd99f6e

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

LOG: [clang][dataflow] Add cache of `ControlFlowContext`s for function decls.

This patch modifies context-sensitive analysis of functions to use a cache,
rather than recreate the `ControlFlowContext` from a function decl on each
encounter. However, this is just step 1 (of N) in adding support for a
configurable map of "modeled" function decls (see issue #56879). The map will go
from the actual function decl to the `ControlFlowContext` used to model it. Only
functions pre-configured in the map will be modeled in a context-sensitive way.

We start with a cache because it introduces the desired map, while retaining the
current behavior. Here, functions are mapped to their actual implementations
(when available).

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h 
b/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
index e6ceb3a89131..58e901fac004 100644
--- a/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
@@ -30,10 +30,19 @@ namespace dataflow {
 /// analysis.
 class ControlFlowContext {
 public:
-  /// Builds a ControlFlowContext from an AST node.
+  /// Builds a ControlFlowContext from an AST node. `D` is the function in 
which
+  /// `S` resides and must not be null.
+  static llvm::Expected build(const Decl *D, Stmt &S,
+  ASTContext &C);
+
+  // DEPRECATED. Use overload above.
   static llvm::Expected build(const Decl *D, Stmt *S,
   ASTContext *C);
 
+  /// Returns the `Decl` containing the statement used to construct the CFG, if
+  /// available.
+  const Decl *getDecl() const { return ContainingDecl; }
+
   /// Returns the CFG that is stored in this context.
   const CFG &getCFG() const { return *Cfg; }
 
@@ -43,10 +52,15 @@ class ControlFlowContext {
   }
 
 private:
-  ControlFlowContext(std::unique_ptr Cfg,
+  // FIXME: Once the deprecated `build` method is removed, mark `D` as "must 
not
+  // be null" and add an assertion.
+  ControlFlowContext(const Decl *D, std::unique_ptr Cfg,
  llvm::DenseMap 
StmtToBlock)
-  : Cfg(std::move(Cfg)), StmtToBlock(std::move(StmtToBlock)) {}
+  : ContainingDecl(D), Cfg(std::move(Cfg)),
+StmtToBlock(std::move(StmtToBlock)) {}
 
+  /// The `Decl` containing the statement used to construct the CFG.
+  const Decl *ContainingDecl;
   std::unique_ptr Cfg;
   llvm::DenseMap StmtToBlock;
 };

diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
index c7903fd7ee89..99e16f825544 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
@@ -20,7 +20,6 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Stmt.h"
-#include "clang/Analysis/CFG.h"
 #include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"

diff  --git 
a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index 6a9a35ac0baf..a31e08fd16c4 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -18,6 +18,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/TypeOrdering.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/Solver.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
@@ -254,6 +255,10 @@ class DataflowAnalysisContext {
 
   LLVM_DUMP_METHOD void dumpFlowCondition(AtomicBoolValue &Token);
 
+  /// Returns the `ControlFlowContext` registered for `F`, if any. Otherwise,
+  /// returns null.
+  c

[PATCH] D131039: [clang][dataflow] Add cache of `ControlFlowContext`s for function decls.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG692e03039d1e: [clang][dataflow] Add cache of 
`ControlFlowContext`s for function decls. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131039

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  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
@@ -507,28 +507,30 @@
 return;
   Env.setStorageLocation(*S, *ArgLoc);
 } else if (const FunctionDecl *F = S->getDirectCallee()) {
-  // This case is for context-sensitive analysis, which we only do if we
-  // have the callee body available in the translation unit.
-  if (!Options.ContextSensitive || F->getBody() == nullptr)
+  // This case is for context-sensitive analysis.
+  if (!Options.ContextSensitive)
+return;
+
+  const ControlFlowContext *CFCtx = Env.getControlFlowContext(F);
+  if (!CFCtx)
 return;
 
   // FIXME: We don't support context-sensitive analysis of recursion, so
   // we should return early here if `F` is the same as the `FunctionDecl`
   // holding `S` itself.
 
-  auto &ASTCtx = F->getASTContext();
-
-  // FIXME: Cache these CFGs.
-  auto CFCtx = ControlFlowContext::build(F, F->getBody(), &ASTCtx);
-  // FIXME: Handle errors here and below.
-  assert(CFCtx);
   auto ExitBlock = CFCtx->getCFG().getExit().getBlockID();
 
   auto CalleeEnv = Env.pushCall(S);
 
-  // FIXME: Use the same analysis as the caller for the callee.
-  DataflowAnalysisOptions Options;
-  auto Analysis = NoopAnalysis(ASTCtx, Options);
+  // FIXME: Use the same analysis as the caller for the callee. Note,
+  // though, that doing so would require support for changing the analysis's
+  // ASTContext.
+  assert(
+  CFCtx->getDecl() != nullptr &&
+  "ControlFlowContexts in the environment should always carry a decl");
+  auto Analysis = NoopAnalysis(CFCtx->getDecl()->getASTContext(),
+   DataflowAnalysisOptions());
 
   auto BlockToOutputState =
   dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv);
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -334,6 +334,27 @@
   llvm::dbgs() << debugString(Constraints, AtomNames);
 }
 
+const ControlFlowContext *
+DataflowAnalysisContext::getControlFlowContext(const FunctionDecl *F) {
+  // Canonicalize the key:
+  F = F->getDefinition();
+  if (F == nullptr)
+return nullptr;
+  auto It = FunctionContexts.find(F);
+  if (It != FunctionContexts.end())
+return &It->second;
+
+  if (Stmt *Body = F->getBody()) {
+auto CFCtx = ControlFlowContext::build(F, *Body, F->getASTContext());
+// FIXME: Handle errors.
+assert(CFCtx);
+auto Result = FunctionContexts.insert({F, std::move(*CFCtx)});
+return &Result.first->second;
+  }
+
+  return nullptr;
+}
+
 } // namespace dataflow
 } // namespace clang
 
Index: clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -45,7 +45,7 @@
 }
 
 llvm::Expected
-ControlFlowContext::build(const Decl *D, Stmt *S, ASTContext *C) {
+ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) {
   CFG::BuildOptions Options;
   Options.PruneTriviallyFalseEdges = false;
   Options.AddImplicitDtors = true;
@@ -56,7 +56,7 @@
   // Ensure that all sub-expressions in basic blocks are evaluated.
   Options.setAllAlwaysAdd();
 
-  auto Cfg = CFG::buildCFG(D, S, C, Options);
+  auto Cfg = CFG::buildCFG(D, &S, &C, Options);
   if (Cfg == nullptr)
 return llvm::createStringError(
 std::make_error_code(std::errc::invalid_argument),
@@ -64,7 +64,14 @@
 
   llvm::DenseMap StmtToBlock =
   buildStmtToBasicBlockMap(*Cfg);
-  return ControlFlowContext(std::move(Cfg), std::move(StmtToBlock));
+  return ControlFlowContext(D, std::move(Cfg), std::move(StmtToBlock));
+}
+
+llvm::Exp

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

[Marco Elver via Phabricator via cfe-commits]

melver updated this revision to Diff 449671.
melver marked 4 inline comments as done.
melver added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130888

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/fsanitize-metadata.c

Index: clang/test/Driver/fsanitize-metadata.c
===
--- /dev/null
+++ clang/test/Driver/fsanitize-metadata.c
@@ -0,0 +1,34 @@
+// RUN: %clang --target=x86_64-linux-gnu \
+// RUN:   -fexperimental-sanitize-metadata=all \
+// RUN:   -fno-experimental-sanitize-metadata=all %s -### 2>&1 | FileCheck %s
+// CHECK-NOT: -fexperimental-sanitize-metadata
+
+// RUN: %clang --target=x86_64-linux-gnu -fexperimental-sanitize-metadata=bad_arg %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-INVALID %s
+// CHECK-INVALID: error: unsupported argument 'bad_arg' to option '-fexperimental-sanitize-metadata='
+
+// RUN: %clang --target=x86_64-linux-gnu -fexperimental-sanitize-metadata=covered %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-COVERED %s
+// RUN: %clang --target=x86_64-linux-gnu \
+// RUN:   -fexperimental-sanitize-metadata=atomics \
+// RUN:   -fno-experimental-sanitize-metadata=atomics \
+// RUN:   -fexperimental-sanitize-metadata=covered %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-COVERED %s
+// CHECK-COVERED: "-fexperimental-sanitize-metadata=covered"
+// CHECK-COVERED-NOT: "-fexperimental-sanitize-metadata=atomics"
+
+// RUN: %clang --target=x86_64-linux-gnu -fexperimental-sanitize-metadata=atomics %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-ATOMICS %s
+// CHECK-ATOMICS: "-fexperimental-sanitize-metadata=atomics"
+
+// RUN: %clang --target=x86_64-linux-gnu \
+// RUN:   -fexperimental-sanitize-metadata=covered,atomics %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-ALL %s
+// RUN: %clang --target=x86_64-linux-gnu \
+// RUN:   -fexperimental-sanitize-metadata=covered \
+// RUN:   -fexperimental-sanitize-metadata=atomics %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-ALL %s
+// RUN: %clang --target=x86_64-linux-gnu -fexperimental-sanitize-metadata=all %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-ALL %s
+// CHECK-ALL: "-fexperimental-sanitize-metadata=covered"
+// CHECK-ALL: "-fexperimental-sanitize-metadata=atomics"
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -98,6 +98,11 @@
   CoverageTraceStores = 1 << 17,
 };
 
+enum BinaryMetadataFeature {
+  BinaryMetadataCovered = 1 << 0,
+  BinaryMetadataAtomics = 1 << 1,
+};
+
 /// Parse a -fsanitize= or -fno-sanitize= argument's values, diagnosing any
 /// invalid components. Returns a SanitizerMask.
 static SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A,
@@ -108,6 +113,11 @@
 static int parseCoverageFeatures(const Driver &D, const llvm::opt::Arg *A,
  bool DiagnoseErrors);
 
+/// Parse -f(no-)?sanitize-metadata= flag values, diagnosing any invalid
+/// components. Returns OR of members of \c BinaryMetadataFeature enumeration.
+static int parseBinaryMetadataFeatures(const Driver &D, const llvm::opt::Arg *A,
+   bool DiagnoseErrors);
+
 /// Produce an argument string from ArgList \p Args, which shows how it
 /// provides some sanitizer kind from \p Mask. For example, the argument list
 /// "-fsanitize=thread,vptr -fsanitize=address" with mask \c NeedsUbsanRt
@@ -825,6 +835,22 @@
 DiagnoseErrors);
   }
 
+  // Parse -f(no-)?sanitize-metadata.
+  for (const auto *Arg :
+   Args.filtered(options::OPT_fexperimental_sanitize_metadata_EQ,
+ options::OPT_fno_experimental_sanitize_metadata_EQ)) {
+if (Arg->getOption().matches(
+options::OPT_fexperimental_sanitize_metadata_EQ)) {
+  Arg->claim();
+  BinaryMetadataFeatures |=
+  parseBinaryMetadataFeatures(D, Arg, DiagnoseErrors);
+} else {
+  Arg->claim();
+  BinaryMetadataFeatures &=
+  ~parseBinaryMetadataFeatures(D, Arg, DiagnoseErrors);
+}
+  }
+
   SharedRuntime =
   Args.hasFlag(options::OPT_shared_libsan, options::OPT_static_libsan,
TC.getTriple().isAndroid() || TC.getTriple().isOSFuchsia() ||
@@ -1086,6 +1112,17 @@
   addSpecialCaseListOpt(Args, CmdArgs, "-fsanitize-coverage-ignorelist=",
 CoverageIgnorelistFiles);
 
+  // Translate available BinaryMetadataFeatures to corresponding clang-cc1
+  // flags. Does not depend on any other sanitizers.
+  const std

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

[Marco Elver via Phabricator via cfe-commits]

melver added inline comments.



Comment at: clang/lib/Driver/SanitizerArgs.cpp:839
+  // Parse -f(no-)?sanitize-metadata.
+  for (const auto *Arg : Args) {
+if (Arg->getOption().matches(

MaskRay wrote:
> Use `Args.getLastArg(...)`
This won't work if someone does:

  -fsanitize-metadata=feature1 -fsanitize-metadata=feature2

(instead of '-fsanitize-metadata=feature1,feature2')

Added a test case.



Comment at: clang/test/Driver/fsanitize-metadata.c:1
+// RUN: %clang -target x86_64-linux-gnu %s -### 2>&1 | FileCheck %s
+// CHECK-NOT: -fexperimental-sanitize-metadata

MaskRay wrote:
> This RUN line is redundant. For other opt-in features, we don't check that 
> the cc1 command line doesn't have an option. 
I've made the negative-presence test more useful by checking -fno- option works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130888

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

[PATCH] D130933: Add docs for function attributes hot/cold

[David Li via Phabricator via cfe-commits]

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

LGTM from the content point of view. Please also address aaron's comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130933

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

[PATCH] D130889: [llvm] Introduce a pass plugin registry and loader

[wael yehia via Phabricator via cfe-commits]

w2yehia added a comment.

Looks good. Thanks.




Comment at: llvm/lib/Passes/PassPluginLoader.cpp:23
+void PassPluginLoader::operator=(const std::string &Filename) {
+  sys::SmartScopedLock Lock(*PluginsLock);
+  auto PassPlugin = PassPlugin::Load(Filename);

why do we need a lock? 
Is there a use case where multiple threads enter this function.
Right now, the only place we create a `PassPluginLoader` is in 
`cl::opt PassPlugins`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130889

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

[PATCH] D130894: [clang] Print more information about failed static assertions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D130894#3696590 , @xbolva00 wrote:

> Use ‘5 ==6’ ? So add quotes ..

+1 to the suggestion to use quotes for a bit of visual distinction between the 
diagnostic message and the code embedded within it.




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16573-16584
+  if (BoolValue) {
+Str.push_back('t');
+Str.push_back('r');
+Str.push_back('u');
+Str.push_back('e');
+  } else {
+Str.push_back('f');

Thankfully, there is a better way: 
```
llvm::raw_svector_ostream OS(Str);
OS << (BoolValue ? "true" : "false");
```
which applies to the other cases in this function as well.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16647-16649
+  if (isa(E) || isa(E) ||
+  isa(E) || isa(E) ||
+  isa(E))

`FixedPointLiteral`? `ImaginaryLiteral`?



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16658
+  // -5 is also simple to understand.
+  if (const UnaryOperator *UnaryOp = dyn_cast_or_null(E))
+return UsefulToPrintExpr(UnaryOp->getSubExpr());





Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16672
+void Sema::DiagnoseStaticAssertDetails(const Expr *E) {
+  if (const BinaryOperator *Op = dyn_cast_or_null(E)) {
+const Expr *LHS = Op->getLHS()->IgnoreParenImpCasts();




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

https://reviews.llvm.org/D130894

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

[PATCH] D123967: Disable update_cc_test_checks.py tests in stand-alone builds

[Tom Stellard via Phabricator via cfe-commits]

tstellar updated this revision to Diff 449681.
tstellar marked an inline comment as done.
tstellar added a comment.

Fix option spelling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123967

Files:
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  clang/test/utils/update_cc_test_checks/lit.local.cfg


Index: clang/test/utils/update_cc_test_checks/lit.local.cfg
===
--- clang/test/utils/update_cc_test_checks/lit.local.cfg
+++ clang/test/utils/update_cc_test_checks/lit.local.cfg
@@ -10,27 +10,36 @@
 from pipes import quote as shell_quote
 
 
-config.test_format = lit.formats.ShTest(execute_external=False)
-config.suffixes = ['.test']
-
-clang_path = os.path.join(config.clang_tools_dir, 'clang')
-extra_args = '--clang ' + shell_quote(clang_path)
-opt_path = os.path.join(config.llvm_tools_dir, 'opt')
-extra_args += ' --opt ' + shell_quote(opt_path)
-script_path = os.path.join(config.llvm_src_root, 'utils',
-   'update_cc_test_checks.py')
-assert os.path.isfile(script_path)
-# Windows: llvm-lit.py, Linux: llvm-lit
-if config.llvm_external_lit:
-lit = config.llvm_external_lit
+if config.standalone_build:
+# These tests require the update_cc_test_checks.py script from the llvm
+# source tree, so skip these tests if we are doing standalone builds.
+# These tests are only relevant to developers working with the
+# update_cc_test_checks.py tool; they don't provide any coverage
+# for any of the clang source code.
+config.unsupported = True
 else:
-lit = shell_quote(glob.glob(os.path.join(config.llvm_tools_dir, 
'llvm-lit*'))[0])
-python = shell_quote(config.python_executable)
-config.substitutions.append(
-('%update_cc_test_checks', "%s %s %s" % (
-python, shell_quote(script_path), extra_args)))
-config.substitutions.append(
-('%clang_tools_dir', shell_quote(config.clang_tools_dir)))
-config.substitutions.append(
-('%lit', "%s %s -Dclang_lit_site_cfg=%s -j1 -vv" % (
-python, lit, shell_quote(config.clang_lit_site_cfg
+
+config.test_format = lit.formats.ShTest(execute_external=False)
+config.suffixes = ['.test']
+
+clang_path = os.path.join(config.clang_tools_dir, 'clang')
+extra_args = '--clang ' + shell_quote(clang_path)
+opt_path = os.path.join(config.llvm_tools_dir, 'opt')
+extra_args += ' --opt ' + shell_quote(opt_path)
+script_path = os.path.join(config.llvm_src_root, 'utils',
+   'update_cc_test_checks.py')
+assert os.path.isfile(script_path)
+# Windows: llvm-lit.py, Linux: llvm-lit
+if config.llvm_external_lit:
+lit = config.llvm_external_lit
+else:
+lit = shell_quote(glob.glob(os.path.join(config.llvm_tools_dir, 
'llvm-lit*'))[0])
+python = shell_quote(config.python_executable)
+config.substitutions.append(
+('%update_cc_test_checks', "%s %s %s" % (
+python, shell_quote(script_path), extra_args)))
+config.substitutions.append(
+('%clang_tools_dir', shell_quote(config.clang_tools_dir)))
+config.substitutions.append(
+('%lit', "%s %s -Dclang_lit_site_cfg=%s -j1 -vv" % (
+python, lit, shell_quote(config.clang_lit_site_cfg
Index: clang/test/lit.site.cfg.py.in
===
--- clang/test/lit.site.cfg.py.in
+++ clang/test/lit.site.cfg.py.in
@@ -37,6 +37,7 @@
 config.has_plugins = @CLANG_PLUGIN_SUPPORT@
 config.clang_vendor_uti = "@CLANG_VENDOR_UTI@"
 config.llvm_external_lit = path(r"@LLVM_EXTERNAL_LIT@")
+config.standalone_build = @CLANG_BUILT_STANDALONE@
 
 import lit.llvm
 lit.llvm.initialize(lit_config, config)
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -3,6 +3,7 @@
 
 llvm_canonicalize_cmake_booleans(
   CLANG_BUILD_EXAMPLES
+  CLANG_BUILT_STANDALONE
   CLANG_DEFAULT_PIE_ON_LINUX
   CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL
   CLANG_ENABLE_ARCMT


Index: clang/test/utils/update_cc_test_checks/lit.local.cfg
===
--- clang/test/utils/update_cc_test_checks/lit.local.cfg
+++ clang/test/utils/update_cc_test_checks/lit.local.cfg
@@ -10,27 +10,36 @@
 from pipes import quote as shell_quote
 
 
-config.test_format = lit.formats.ShTest(execute_external=False)
-config.suffixes = ['.test']
-
-clang_path = os.path.join(config.clang_tools_dir, 'clang')
-extra_args = '--clang ' + shell_quote(clang_path)
-opt_path = os.path.join(config.llvm_tools_dir, 'opt')
-extra_args += ' --opt ' + shell_quote(opt_path)
-script_path = os.path.join(config.llvm_src_root, 'utils',
-   'update_cc_test_checks.py')
-assert os.path.isfile(script_path)
-# Windows: llvm-lit.py, Linux: 

[PATCH] D123967: Disable update_cc_test_checks.py tests in stand-alone builds

[Tom Stellard via Phabricator via cfe-commits]

tstellar added inline comments.



Comment at: clang/test/utils/update_cc_test_checks/lit.local.cfg:19
+# for any of the clang source code.
+config.unsupported = True
 else:

h-vetinari wrote:
> I couldn't tell from the diff where this is used (though admittedly I hardly 
> know the LLVM infra). I also don't see it in [[ 
> https://github.com/llvm/llvm-project/blob/main/clang/test/lit.cfg.py | 
> `lit.cfg.py` ]], the respective [[ 
> https://github.com/llvm/llvm-project/blob/main/clang/test/CMakeLists.txt | 
> `CMakeLists.txt` ]] or [[ 
> https://github.com/llvm/llvm-project/blob/llvmorg-14.0.6/llvm/cmake/modules/AddLLVM.cmake#L1616
>  | `configure_lit_site_cfg` ]]
> 
> I trust that this does what's intended.
The lit tool uses this to decide whether or not to run the tests in the 
directory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123967

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

[PATCH] D131076: [clang][modules] Don't depend on sharing FileManager during module build

[Ben Langmuir via Phabricator via cfe-commits]

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

Sharing the FileManager between the importer and the module build should only 
be an optimization. Add a cc1 option -fno-modules-share-filemanager to allow us 
to test this. Fix the path to modulemap files, which previously depended on the 
shared FileManager when using path mapped to an external file in a VFS.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131076

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Frontend/CompilerInstance.cpp
  clang/test/ClangScanDeps/modulemap-via-vfs.m
  clang/test/Modules/submodule-in-private-mmap-vfs.m
  clang/test/VFS/module-import.m

Index: clang/test/VFS/module-import.m
===
--- clang/test/VFS/module-import.m
+++ clang/test/VFS/module-import.m
@@ -1,6 +1,7 @@
-// RUN: rm -rf %t
+// RUN: rm -rf %t %t-unshared
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsoverlay.yaml > %t.yaml
 // RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -I %t -fsyntax-only %s
+// RUN: %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t.yaml -I %t -fsyntax-only %s
 
 @import not_real;
 
@@ -18,9 +19,12 @@
 // Override the module map (vfsoverlay2 on top)
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsoverlay2.yaml > %t2.yaml
 // RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -I %t -fsyntax-only %s
+// RUN: %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -I %t -fsyntax-only %s
 
 // vfsoverlay2 not present
 // RUN: not %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
+// RUN: not %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
 
 // vfsoverlay2 on the bottom
 // RUN: not %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t2.yaml -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
+// RUN: not %clang_cc1 -Werror -fmodules -fno-modules-share-filemanager -fimplicit-module-maps -fmodules-cache-path=%t-unshared -ivfsoverlay %t2.yaml -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s
Index: clang/test/Modules/submodule-in-private-mmap-vfs.m
===
--- /dev/null
+++ clang/test/Modules/submodule-in-private-mmap-vfs.m
@@ -0,0 +1,38 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/vfs.json.in > %t/vfs.json
+// RUN: %clang_cc1 -fmodules -fno-modules-share-filemanager -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t -I/tests -ivfsoverlay %t/vfs.json -fsyntax-only %t/tu.m -verify
+
+//--- Dir1/module.modulemap
+
+//--- Dir2/module.private.modulemap
+module Foo_Private {}
+
+//--- vfs.json.in
+{
+  'version': 0,
+  'use-external-names': true,
+  'roots': [
+{
+  'name': '/tests',
+  'type': 'directory',
+  'contents': [
+{
+  'name': 'module.modulemap',
+  'type': 'file',
+  'external-contents': 'DIR/Dir1/module.modulemap'
+},
+{
+  'name': 'module.private.modulemap',
+  'type': 'file',
+  'external-contents': 'DIR/Dir2/module.private.modulemap'
+}
+  ]
+}
+  ]
+}
+
+//--- tu.m
+@import Foo_Private;
+// expected-no-diagnostics
\ No newline at end of file
Index: clang/test/ClangScanDeps/modulemap-via-vfs.m
===
--- clang/test/ClangScanDeps/modulemap-via-vfs.m
+++ clang/test/ClangScanDeps/modulemap-via-vfs.m
@@ -9,6 +9,7 @@
 
 // CHECK-NOT: build/module.modulemap
 // CHECK: A/module.modulemap
+// CHECK-NOT: build/module.modulemap
 
 //--- build/compile-commands.json.in
 
@@ -17,6 +18,11 @@
   "directory": "DIR",
   "command": "clang DIR/main.m -Imodules/A -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps -ivfsoverlay build/vfs.yaml",
   "file": "DIR/main.m"
+},
+{
+  "directory": "DIR",
+  "command": "clang DIR/main.m -Imodules/A -fmodules -Xclang -fno-modules-share-filemana

[PATCH] D129016: [PowerPC] implemented @llvm.ppc.kill.canary to corrupt stack guard

[Paul Scoropan via Phabricator via cfe-commits]

pscoro updated this revision to Diff 449683.
pscoro added a comment.

removed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129016

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/CodeGen/PowerPC/builtins-ppc-stackprotect.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll

Index: llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll
@@ -0,0 +1,142 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX64
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-LINUX-LE
+
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX64
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-LINUX-LE
+
+attributes #1 = { nounwind }
+declare void @llvm.ppc.kill.canary()
+define dso_local void @test_kill_canary() #1 {
+; CHECK-AIX-LABEL: test_kill_canary:
+; CHECK-AIX:   # %bb.0: # %entry
+; CHECK-AIX-NEXT:blr
+;
+; CHECK-LABEL: test_kill_canary:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:blr
+;
+; CHECK-AIX64-LABEL: test_kill_canary:
+; CHECK-AIX64:   # %bb.0: # %entry
+; CHECK-AIX64-NEXT:blr
+;
+; CHECK-LINUX-LE-LABEL: test_kill_canary:
+; CHECK-LINUX-LE:   # %bb.0: # %entry
+; CHECK-LINUX-LE-NEXT:blr
+entry:
+  call void @llvm.ppc.kill.canary()
+  ret void
+}
+
+attributes #0 = { sspreq }
+; Function Attrs: sspreq
+define dso_local void @test_kill_canary_ssp() #0 #1 {
+; CHECK-AIX-LABEL: test_kill_canary_ssp:
+; CHECK-AIX:   # %bb.0: # %entry
+; CHECK-AIX-NEXT:mflr r0
+; CHECK-AIX-NEXT:stw r0, 8(r1)
+; CHECK-AIX-NEXT:stwu r1, -64(r1)
+; CHECK-AIX-NEXT:lwz r3, L..C0(r2) # @__ssp_canary_word
+; CHECK-AIX-NEXT:lwz r4, 0(r3)
+; CHECK-AIX-NEXT:stw r4, 60(r1)
+; CHECK-AIX-NEXT:lwz r4, 0(r3)
+; CHECK-AIX-NEXT:not r4, r4
+; CHECK-AIX-NEXT:stw r4, 60(r1)
+; CHECK-AIX-NEXT:lwz r3, 0(r3)
+; CHECK-AIX-NEXT:lwz r4, 60(r1)
+; CHECK-AIX-NEXT:cmplw r3, r4
+; CHECK-AIX-NEXT:bne cr0, L..BB1_2
+; CHECK-AIX-NEXT:  # %bb.1: # %entry
+; CHECK-AIX-NEXT:addi r1, r1, 64
+; CHECK-AIX-NEXT:lwz r0, 8(r1)
+; CHECK-AIX-NEXT:mtlr r0
+; CHECK-AIX-NEXT:blr
+; CHECK-AIX-NEXT:  L..BB1_2: # %entry
+; CHECK-AIX-NEXT:bl .__stack_chk_fail[PR]
+; CHECK-AIX-NEXT:nop
+;
+; CHECK-LABEL: test_kill_canary_ssp:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mflr r0
+; CHECK-NEXT:std r0, 16(r1)
+; CHECK-NEXT:stdu r1, -128(r1)
+; CHECK-NEXT:ld r3, -28688(r13)
+; CHECK-NEXT:std r3, 120(r1)
+; CHECK-NEXT:ld r3, -28688(r13)
+; CHECK-NEXT:not r3, r3
+; CHECK-NEXT:std r3, 120(r1)
+; CHECK-NEXT:ld r3, 120(r1)
+; CHECK-NEXT:ld r4, -28688(r13)
+; CHECK-NEXT:cmpld r4, r3
+; CHECK-NEXT:bne cr0, .LBB1_2
+; CHECK-NEXT:  # %bb.1: # %entry
+; CHECK-NEXT:addi r1, r1, 128
+; CHECK-NEXT:ld r0, 16(r1)
+; CHECK-NEXT:mtlr r0
+; CHECK-NEXT:blr
+; CHECK-NEXT:  .LBB1_2: # %entry
+; CHECK-NEXT:bl __stack_chk_fail
+; CHECK-NEXT:nop
+;
+; CHECK-AIX64-LABEL: test_kill_canary_ssp:
+; CHECK-AIX64:   # %bb.0: # %entry
+; CHECK-AIX64-NEXT:mflr r0
+; CHECK-AIX64-NEXT:std r0, 16(r1)
+; CHECK-AIX64-NEXT:stdu r1, -128(r1)
+; CHECK-AIX64-NEXT:ld r3, L..C0(r2) # @__ssp_canary_word
+; CHECK-AIX64-NEXT:ld r4, 0(r3)
+; CHECK-AIX64-NEXT:std r4, 120(r1)
+; CHECK-AIX64-NEXT:ld r4, 0(r3)
+; CHECK-AIX64-NEXT:not r4, r4
+; CHECK-AIX64-NEXT:std r4, 120(r1)
+; CHECK-AIX64-NEXT:ld r3, 0(r3)
+; CHECK

[clang] a7bca18 - Fix assert during the call to getCanonicalDecl.

[Jennifer Yu via cfe-commits]

Author: Jennifer Yu
Date: 2022-08-03T09:14:28-07:00
New Revision: a7bca18bc50cd2483fded0c77706980b2721ce6a

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

LOG: Fix assert during the call to getCanonicalDecl.
https://github.com/llvm/llvm-project/issues/56884

The root problem is in isOpenMPRebuildMemberExpr, it is only need to rebuild
for field expression.  No need for member function call.

The fix is to check field for member expression and skip rebuild for member
function call.

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

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/parallel_default_messages.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index dc1470bf7a9d..a92fec6a0232 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -2270,6 +2270,9 @@ bool Sema::isInOpenMPTargetExecutionDirective() const {
 }
 
 bool Sema::isOpenMPRebuildMemberExpr(ValueDecl *D) {
+  // Only rebuild for Field.
+  if (!dyn_cast(D))
+return false;
   DSAStackTy::DSAVarData DVarPrivate = DSAStack->hasDSA(
   D,
   [](OpenMPClauseKind C, bool AppliedToPointee,

diff  --git a/clang/test/OpenMP/parallel_default_messages.cpp 
b/clang/test/OpenMP/parallel_default_messages.cpp
index 7db15d726731..65e0d92c32fc 100644
--- a/clang/test/OpenMP/parallel_default_messages.cpp
+++ b/clang/test/OpenMP/parallel_default_messages.cpp
@@ -49,3 +49,10 @@ int main(int argc, char **argv) {
 
   return 0;
 }
+
+class A{
+  void a() {
+#pragma omp parallel
+a(b); // expected-error {{use of undeclared identifier 'b'}}
+ }
+};



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

[PATCH] D131024: Fix assert in the call to isOpenMPRebuildMemberExpr

[Jennifer Yu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa7bca18bc50c: Fix assert during the call to 
getCanonicalDecl. (authored by jyu2).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131024

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/parallel_default_messages.cpp


Index: clang/test/OpenMP/parallel_default_messages.cpp
===
--- clang/test/OpenMP/parallel_default_messages.cpp
+++ clang/test/OpenMP/parallel_default_messages.cpp
@@ -49,3 +49,10 @@
 
   return 0;
 }
+
+class A{
+  void a() {
+#pragma omp parallel
+a(b); // expected-error {{use of undeclared identifier 'b'}}
+ }
+};
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -2270,6 +2270,9 @@
 }
 
 bool Sema::isOpenMPRebuildMemberExpr(ValueDecl *D) {
+  // Only rebuild for Field.
+  if (!dyn_cast(D))
+return false;
   DSAStackTy::DSAVarData DVarPrivate = DSAStack->hasDSA(
   D,
   [](OpenMPClauseKind C, bool AppliedToPointee,


Index: clang/test/OpenMP/parallel_default_messages.cpp
===
--- clang/test/OpenMP/parallel_default_messages.cpp
+++ clang/test/OpenMP/parallel_default_messages.cpp
@@ -49,3 +49,10 @@
 
   return 0;
 }
+
+class A{
+  void a() {
+#pragma omp parallel
+a(b); // expected-error {{use of undeclared identifier 'b'}}
+ }
+};
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -2270,6 +2270,9 @@
 }
 
 bool Sema::isOpenMPRebuildMemberExpr(ValueDecl *D) {
+  // Only rebuild for Field.
+  if (!dyn_cast(D))
+return false;
   DSAStackTy::DSAVarData DVarPrivate = DSAStack->hasDSA(
   D,
   [](OpenMPClauseKind C, bool AppliedToPointee,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130123: Extend ptr32 support to be applied on typedef

[Ariel Burton via Phabricator via cfe-commits]

Ariel-Burton added a comment.

In D130123#3691011 , @rnk wrote:

> That sounds reasonable to me, I confirmed that MSVC really only lets you 
> apply these attributes directly to pointer types and to typedefs.
>
> Can you add a test for the other most common type sugar node, the template 
> parameter? It looks like this:
>
>   template 
>   void f(T __ptr32 a) {
>   (*a) += 1;
>   }
>   void g(int *p) {
>   f(p);
>   }
>
> If there isn't already a C++ test for __ptr32 & co, go ahead and make one.

What is your expectation for your template code fragment?  MSVC does not accept 
it.

On the other hand, MSVC does accept this:

  template 
  void f(T  a) {
(*a) += sizeof(a);
  }
  void g(int *p) {
f(p);
  }
  void h(int *__ptr32 p) {
  f(p);
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130123

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

[PATCH] D130123: Extend ptr32 support to be applied on typedef

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D130123#3696752 , @Ariel-Burton 
wrote:

> What is your expectation for your template code fragment?  MSVC does not 
> accept it.

Yes, I checked, MSVC rejects it, so clang should have test expectations to 
confirm that. It seems interesting or surprising, to me at least, that MSVC 
really only accepts __ptr32 on pointers and typedefs of them.

> On the other hand, MSVC does accept this:
>
>   template 
>   void f(T  a) {
> (*a) += sizeof(a);
>   }
>   void g(int *p) {
> f(p);
>   }
>   void h(int *__ptr32 p) {
>   f(p);
>   }

Right, this makes sense to me. MSVC's diagnostics say something about the 
__ptr32 qualifier needing to appear after a `*`, so this extension must be 
implemented at a pretty low-level, with some exception for typedefs, just like 
what you have.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130123

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

[PATCH] D131012: No longer place const volatile global variables in a read only section

[Andrii Nakryiko via Phabricator via cfe-commits]

anakryiko added a comment.

In D131012#3696327 , @aaron.ballman 
wrote:

> In D131012#3695110 , @anakryiko 
> wrote:
>
>> This will severly break BPF users, as we have a heavy reliance on `const 
>> volatile` variables being allocated to `.rodata`, which are passed to BPF 
>> verifier in Linux kernel as read-only values. This is critical for BPF 
>> Complie Once - Run Everywhere approach of writing portable BPF applications. 
>> We've been relying on this behavior for years now and changes this will 
>> break many existing BPF applications.
>
> Thank you for this feedback! I guess I'm a bit surprised given the contents 
> from the issue seem to imply that BPF needed Clang's behavior to change: 
> `Note that this is causing practical difficulties: the kernel's bpftool is 
> generating different skeleton headers for BPF code compiled from LLVM and 
> GCC, because the names of the containing sections get reflected.`

GCC folks are trying to make their BPF backend usable. But instead of working 
with community to make sure they do things the same way that Clang does (which 
for years now if de facto standard for compiling BPF code and we rely on this 
behavior heavily in libbpf and other BPF loader libraries), they instead work 
against BPF community and try to modify/adjust/break the world around them, 
instead of working with us to make GCC-BPF compatible with Clang.

> That said, I'm asking on the WG14 reflectors whether there's a normative 
> requirement here or not, so I'm marking this as having planned changes until 
> I hear back from WG14 and until we've resolved whether the changes will fix 
> code vs break code (or both!) so we don't accidentally land this.

Thanks! But note that `const volatile` being in `.rodata` is a very explicit 
expectation in BPF world, so changing that to `.data` will immediately break a 
bunch of different BPF applications that rely on this for BPF CO-RE (Compile 
Once - Run Everywhere) for guarding BPF code that shouldn't work on old 
kernels. .rodata is reported to BPF verifier as fixed, read-only, known values 
and BPF verifier is using those values for control flow analysis and dead code 
elimination. This is crucial feature.


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

https://reviews.llvm.org/D131012

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

[PATCH] D130964: [X86][BF16] Enable __bf16 for x86 targets.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D130964#3695408 , @FreddyYe wrote:

> In D130964#3694540 , @bkramer wrote:
>
>> In D130964#3694473 , @rjmccall 
>> wrote:
>>
>>> How are you actually implementing `__bf16` on these targets?  There isn't 
>>> even hardware support for conversions.
>>
>> `bf16` -> `float` is really just a bit shift. The other direction gets 
>> lowered to a libcall, compiler-rt has a conversion function with proper 
>> rounding. I added some support to make the backend promote all other 
>> arithmetic to float, but I think that's only enabled on x86 so far.
>
> About hardware support, x86 actually has supported bf16 since AVX512BF16 
> (https://reviews.llvm.org/D60552), which has vector conversion support 
> between float and bf16.

Right, but this patch is adding x86 support whenever SSE2 is available.  
AVX512BF16 is available on a *very* small slice of processors.  In contrast, 
e.g. F16C is relatively broadly available, although I understand that we 
formally support `_Float16` all the way back to SSE2 and thus on some 
processors that lack F16C.

But okay, pure intrinsic support is fine if that's what we're doing.

I think the patch looks fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130964

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

[clang] 6a79e2f - [clang] Add FileEntryRef::getNameAsRequested()

[Ben Langmuir via cfe-commits]

Author: Ben Langmuir
Date: 2022-08-03T09:41:08-07:00
New Revision: 6a79e2ff1989b48f4a8ebf3ac51092eb8ad29e37

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

LOG: [clang] Add FileEntryRef::getNameAsRequested()

As progress towards having FileManager::getFileRef() return the path
as-requested by default, return a FileEntryRef that can use
getNameAsRequested() to retrieve this path, with the ultimate goal that
this should be the behaviour of getName() and clients should explicitly
request the "external" name if they need to (see comment in
FileManager::getFileRef). For now, getName() continues to return the
external path by looking through the redirects.

For now, the new function is only used in unit tests.

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

Added: 


Modified: 
clang/include/clang/Basic/FileEntry.h
clang/lib/Basic/FileManager.cpp
clang/unittests/Basic/FileEntryTest.cpp
clang/unittests/Basic/FileManagerTest.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/FileEntry.h 
b/clang/include/clang/Basic/FileEntry.h
index 8676604b48364..3ca07cb422b64 100644
--- a/clang/include/clang/Basic/FileEntry.h
+++ b/clang/include/clang/Basic/FileEntry.h
@@ -59,11 +59,21 @@ class FileEntry;
 /// accessed by the FileManager's client.
 class FileEntryRef {
 public:
-  StringRef getName() const { return ME->first(); }
+  /// The name of this FileEntry. If a VFS uses 'use-external-name', this is
+  /// the redirected name. See getRequestedName().
+  StringRef getName() const { return getBaseMapEntry().first(); }
+
+  /// The name of this FileEntry, as originally requested without applying any
+  /// remappings for VFS 'use-external-name'.
+  ///
+  /// FIXME: this should be the semantics of getName(). See comment in
+  /// FileManager::getFileRef().
+  StringRef getNameAsRequested() const { return ME->first(); }
+
   const FileEntry &getFileEntry() const {
-return *ME->second->V.get();
+return *getBaseMapEntry().second->V.get();
   }
-  DirectoryEntryRef getDir() const { return *ME->second->Dir; }
+  DirectoryEntryRef getDir() const { return *getBaseMapEntry().second->Dir; }
 
   inline off_t getSize() const;
   inline unsigned getUID() const;
@@ -150,13 +160,20 @@ class FileEntryRef {
   explicit FileEntryRef(const MapEntry &ME) : ME(&ME) {
 assert(ME.second && "Expected payload");
 assert(ME.second->V && "Expected non-null");
-assert(ME.second->V.is() && "Expected FileEntry");
   }
 
   /// Expose the underlying MapEntry to simplify packing in a PointerIntPair or
   /// PointerUnion and allow construction in Optional.
   const clang::FileEntryRef::MapEntry &getMapEntry() const { return *ME; }
 
+  /// Retrieve the base MapEntry after redirects.
+  const MapEntry &getBaseMapEntry() const {
+const MapEntry *ME = this->ME;
+while (const void *Next = ME->second->V.dyn_cast())
+  ME = static_cast(Next);
+return *ME;
+  }
+
 private:
   friend class FileMgr::MapEntryOptionalStorage;
   struct optional_none_tag {};

diff  --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index 451c7c3b8a0dc..cb719762ec7c8 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -293,12 +293,8 @@ FileManager::getFileRef(StringRef Filename, bool openFile, 
bool CacheFailure) {
 // name-as-accessed on the \a FileEntryRef.
 //
 // A potential plan to remove this is as follows -
-//   - Expose the requested filename. One possibility would be to allow
-// redirection-FileEntryRefs to be returned, rather than returning
-// the pointed-at-FileEntryRef, and customizing `getName()` to look
-// through the indirection.
 //   - Update callers such as `HeaderSearch::findUsableModuleForHeader()`
-// to explicitly use the requested filename rather than just using
+// to explicitly use the `getNameAsRequested()` rather than just using
 // `getName()`.
 //   - Add a `FileManager::getExternalPath` API for explicitly getting the
 // remapped external filename when there is one available. Adopt it in
@@ -329,9 +325,6 @@ FileManager::getFileRef(StringRef Filename, bool openFile, 
bool CacheFailure) {
 // Cache the redirection in the previously-inserted entry, still available
 // in the tentative return value.
 NamedFileEnt->second = FileEntryRef::MapValue(Redirection);
-
-// Fix the tentative return value.
-NamedFileEnt = &Redirection;
   }
 
   FileEntryRef ReturnedRef(*NamedFileEnt);

diff  --git a/clang/unittests/Basic/FileEntryTest.cpp 
b/clang/unittests/Basic/FileEntryTest.cpp
index 6c070fb2469df..4249a3d550148 100644
--- a/clang/unittests/Basic/FileEntryTest.cpp
+++ b/clang/unittests/Basic/FileEntryTest.cpp
@

[PATCH] D131004: [clang] Add FileEntryRef::getNameAsRequested()

[Ben Langmuir via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6a79e2ff1989: [clang] Add FileEntryRef::getNameAsRequested() 
(authored by benlangmuir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131004

Files:
  clang/include/clang/Basic/FileEntry.h
  clang/lib/Basic/FileManager.cpp
  clang/unittests/Basic/FileEntryTest.cpp
  clang/unittests/Basic/FileManagerTest.cpp

Index: clang/unittests/Basic/FileManagerTest.cpp
===
--- clang/unittests/Basic/FileManagerTest.cpp
+++ clang/unittests/Basic/FileManagerTest.cpp
@@ -356,9 +356,13 @@
   EXPECT_EQ("dir/f1.cpp", F1Redirect->getName());
   EXPECT_EQ("dir/f2.cpp", F2->getName());
 
+  EXPECT_EQ("dir/f1.cpp", F1->getNameAsRequested());
+  EXPECT_EQ("dir/f1-redirect.cpp", F1Redirect->getNameAsRequested());
+
   // Compare against FileEntry*.
   EXPECT_EQ(&F1->getFileEntry(), *F1);
   EXPECT_EQ(*F1, &F1->getFileEntry());
+  EXPECT_EQ(&F1->getFileEntry(), &F1Redirect->getFileEntry());
   EXPECT_NE(&F2->getFileEntry(), *F1);
   EXPECT_NE(*F1, &F2->getFileEntry());
 
@@ -374,7 +378,7 @@
 
   // Compare using isSameRef.
   EXPECT_TRUE(F1->isSameRef(*F1Again));
-  EXPECT_TRUE(F1->isSameRef(*F1Redirect));
+  EXPECT_FALSE(F1->isSameRef(*F1Redirect));
   EXPECT_FALSE(F1->isSameRef(*F1Also));
   EXPECT_FALSE(F1->isSameRef(*F2));
 }
Index: clang/unittests/Basic/FileEntryTest.cpp
===
--- clang/unittests/Basic/FileEntryTest.cpp
+++ clang/unittests/Basic/FileEntryTest.cpp
@@ -50,6 +50,14 @@
 const_cast(Base.getFileEntry()), DR)})
  .first);
   }
+  FileEntryRef addFileRedirect(StringRef Name, FileEntryRef Base) {
+return FileEntryRef(
+*Files
+ .insert({Name, FileEntryRef::MapValue(
+const_cast(
+Base.getMapEntry()))})
+ .first);
+  }
 };
 
 namespace {
@@ -58,13 +66,23 @@
   FileEntryRef R1 = Refs.addFile("1");
   FileEntryRef R2 = Refs.addFile("2");
   FileEntryRef R1Also = Refs.addFileAlias("1-also", R1);
+  FileEntryRef R1Redirect = Refs.addFileRedirect("1-redirect", R1);
+  FileEntryRef R1Redirect2 = Refs.addFileRedirect("1-redirect2", R1Redirect);
 
   EXPECT_EQ("1", R1.getName());
   EXPECT_EQ("2", R2.getName());
   EXPECT_EQ("1-also", R1Also.getName());
+  EXPECT_EQ("1", R1Redirect.getName());
+  EXPECT_EQ("1", R1Redirect2.getName());
+
+  EXPECT_EQ("1", R1.getNameAsRequested());
+  EXPECT_EQ("1-redirect", R1Redirect.getNameAsRequested());
+  EXPECT_EQ("1-redirect2", R1Redirect2.getNameAsRequested());
 
   EXPECT_NE(&R1.getFileEntry(), &R2.getFileEntry());
   EXPECT_EQ(&R1.getFileEntry(), &R1Also.getFileEntry());
+  EXPECT_EQ(&R1.getFileEntry(), &R1Redirect.getFileEntry());
+  EXPECT_EQ(&R1Redirect.getFileEntry(), &R1Redirect2.getFileEntry());
 
   const FileEntry *CE1 = R1;
   EXPECT_EQ(CE1, &R1.getFileEntry());
@@ -93,6 +111,8 @@
   FileEntryRef R1 = Refs.addFile("1");
   FileEntryRef R2 = Refs.addFile("2");
   FileEntryRef R1Also = Refs.addFileAlias("1-also", R1);
+  FileEntryRef R1Redirect = Refs.addFileRedirect("1-redirect", R1);
+  FileEntryRef R1Redirect2 = Refs.addFileRedirect("1-redirect2", R1Redirect);
 
   EXPECT_EQ(R1, &R1.getFileEntry());
   EXPECT_EQ(&R1.getFileEntry(), R1);
@@ -100,6 +120,8 @@
   EXPECT_NE(R1, &R2.getFileEntry());
   EXPECT_NE(&R2.getFileEntry(), R1);
   EXPECT_NE(R1, R2);
+  EXPECT_EQ(R1, R1Redirect);
+  EXPECT_EQ(R1, R1Redirect2);
 
   OptionalFileEntryRefDegradesToFileEntryPtr M1 = R1;
 
@@ -114,11 +136,16 @@
   FileEntryRef R1 = Refs.addFile("1");
   FileEntryRef R2 = Refs.addFile("2");
   FileEntryRef R1Also = Refs.addFileAlias("1-also", R1);
+  FileEntryRef R1Redirect = Refs.addFileRedirect("1-redirect", R1);
+  FileEntryRef R1Redirect2 = Refs.addFileRedirect("1-redirect2", R1Redirect);
 
   EXPECT_TRUE(R1.isSameRef(FileEntryRef(R1)));
   EXPECT_TRUE(R1.isSameRef(FileEntryRef(R1.getMapEntry(;
   EXPECT_FALSE(R1.isSameRef(R2));
   EXPECT_FALSE(R1.isSameRef(R1Also));
+  EXPECT_FALSE(R1.isSameRef(R1Redirect));
+  EXPECT_FALSE(R1.isSameRef(R1Redirect2));
+  EXPECT_FALSE(R1Redirect.isSameRef(R1Redirect2));
 }
 
 TEST(FileEntryTest, DenseMapInfo) {
Index: clang/lib/Basic/FileManager.cpp
===
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -293,12 +293,8 @@
 // name-as-accessed on the \a FileEntryRef.
 //
 // A potential plan to remove this is as follows -
-//   - Expose the requested filename. One possibility would be to allow
-// redirection-FileEntryRefs to be returned, rather than returning
-// the pointed-at-FileEntryRef, and customizing `getName()` to look
-// through the indirection.
 //   - Update callers such as

[PATCH] D122768: [Clang][C++20] Support capturing structured bindings in lambdas

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 449690.
cor3ntin added a comment.

Missed a comment (s/ValueDecl/auto/ on the LHS of a `cast`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122768

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/LambdaCapture.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/Analysis/AnalysisDeclContext.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedLambdaCapturesChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  clang/test/CodeGenCXX/cxx20-decomposition.cpp
  clang/test/SemaCXX/cxx1z-decomposition.cpp
  clang/test/SemaCXX/cxx20-decomposition.cpp
  clang/test/SemaCXX/decomposition-blocks.cpp
  clang/test/SemaCXX/decomposition-openmp.cpp
  clang/tools/libclang/CIndex.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1140,7 +1140,7 @@
 
   Structured binding extensions
   https://wg21.link/p1091r3";>P1091R3
-  Partial
+  Clang 16
 
   
 https://wg21.link/p1381r1";>P1381R1
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3482,9 +3482,11 @@
C != CEnd; ++C) {
 if (!C->capturesVariable())
   continue;
-
-if (Visit(MakeCursorVariableRef(C->getCapturedVar(), C->getLocation(),
-TU)))
+// TODO: handle structured bindings here ?
+if (!isa(C->getCapturedVar()))
+  continue;
+if (Visit(MakeCursorVariableRef(cast(C->getCapturedVar()),
+C->getLocation(), TU)))
   return true;
   }
   // Visit init captures
Index: clang/test/SemaCXX/decomposition-openmp.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/decomposition-openmp.cpp
@@ -0,0 +1,13 @@
+
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -fopenmp %s
+
+// FIXME: OpenMP should support capturing structured bindings
+auto f() {
+  int i[2] = {};
+  auto [a, b] = i; // expected-note 2{{declared here}}
+  return [=, &a] {
+// expected-error@-1 {{capturing a structured binding is not yet supported in OpenMP}}
+return a + b;
+// expected-error@-1 {{capturing a structured binding is not yet supported in OpenMP}}
+  };
+}
Index: clang/test/SemaCXX/decomposition-blocks.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/decomposition-blocks.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s -fblocks
+
+struct S {
+  int i : 1;
+  int j;
+};
+
+void run(void (^)());
+void test() {
+  auto [i, j] = S{1, 42}; // expected-note {{'i' declared here}}
+  run(^{
+(void)i; // expected-error {{reference to local binding 'i' declared in enclosing function 'test'}}
+  });
+}
Index: clang/test/SemaCXX/cxx20-decomposition.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-decomposition.cpp
@@ -0,0 +1,141 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
+// expected-no-diagnostics
+
+template 
+constexpr bool is_same = false;
+template 
+constexpr bool is_same = true;
+
+struct S {
+  int i;
+  int &j;
+};
+
+void check_category() {
+  int a = 42;
+  {
+auto [v, r] = S{1, a};
+(void)[ v, r ] {
+  static_assert(is_same);
+  static_assert(is_same);
+};
+  }
+  {
+auto [v, r] = S{1, a};
+(void)[&v, &r ] {
+  static_assert(is_same);
+  static_assert(is_same);
+};
+  }
+  {
+S s{1, a};
+const auto &[v, r] = s;
+(void)[ v, r ] {
+  static_assert(is_same);
+  static_assert(is_same

[PATCH] D122768: [Clang][C++20] Support capturing structured bindings in lambdas

[Aaron Ballman via Phabricator via cfe-commits]

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

I mostly only have minor nits, but otherwise this LGTM!




Comment at: clang/docs/ReleaseNotes.rst:104-105
 
+- Support capturing structured bindings in lambdas
+  (`P1091R3 `_ and `P1381R1 
`)
+

You should probably mention some of the issues that it fixes as well, since it 
handles a bunch of them.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9060
 continue;
-  const VarDecl *VD = LC.getCapturedVar();
+  const VarDecl *VD = cast(LC.getCapturedVar());
   if (LC.getCaptureKind() != LCK_ByRef && !VD->getType()->isPointerType())

ABataev wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > cor3ntin wrote:
> > > > cor3ntin wrote:
> > > > > I'm not sure we currently prohibit capturing a structured binding in 
> > > > > openmp, i should fix that
> > > > I disabled the feature in OpenMP mode, as this needs more investigation
> > > Ping @jdoerfert -- do you happen to know what's up here?
> > Ping @mikerice or @ABataev in case @jdoerfert is on vacation.
> > 
> > (Note, I don't think an answer to this question is needed to accept this 
> > review.)
> Need to support capturing of structured bindings in OpenMP regions, some 
> people asked for it long time ago.
Thank you for confirming @ABataev -- I had missed that 
https://github.com/llvm/llvm-project/issues/33025 was already filed. Nothing to 
do here for this review, what's happening is an improvement.



Comment at: clang/lib/Sema/SemaExpr.cpp:18324
+
+  if (VarDecl *VD = dyn_cast(Underlying)) {
+if (VD->hasLocalStorage() && Diagnose)





Comment at: clang/lib/Sema/SemaLambda.cpp:1207-1211
+if (isa(Var)) {
+  Underlying =
+  dyn_cast(cast(Var)->getDecomposedDecl());
+} else
+  Underlying = cast(Var);




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122768

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

[PATCH] D124435: [X86] Always extend the integer parameters in callee

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

That is an interesting idea.  I like that it integrates this into 
`-fclang-abi-compat`.  The way that `-mno-conservative-small-integer-abi` ends 
up meaning opposite things based on the abi-compat setting worries me a lot, 
though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124435

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

[PATCH] D129097: [clang][dataflow] Handle null pointers of type std::nullptr_t

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua marked an inline comment as done.
li.zhe.hua added inline comments.



Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:96
   ///
   ///  `Type` must not be null.
   StorageLocation &getStableStorageLocation(QualType Type);

sgatev wrote:
> This is inconsistent with the change introduced by this patch.
Ah, you're correct. I can mail a fix out later today.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:27
 DataflowAnalysisContext::getStableStorageLocation(QualType Type) {
-  assert(!Type.isNull());
-  if (Type->isStructureOrClassType() || Type->isUnionType()) {
+  if (!Type.isNull() &&
+  (Type->isStructureOrClassType() || Type->isUnionType())) {

sgatev wrote:
> What does that mean? We are analyzing an incomplete translation unit? Why 
> would the type ever be null here?
See the motivating test case:

```
// Alternatively, use `std::nullptr_t` instead of `my_nullptr_t`.
using my_nullptr_t = decltype(nullptr);
my_nullptr_t Null = 0;
```

This triggers `getOrCreateNullPointerValue` to be called with a null pointee 
type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129097

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