[PATCH] D119656: [Driver][DragonFly] -r: imply -nostdlib like GCC

[Brad Smith via Phabricator via cfe-commits]

brad created this revision.
brad added a reviewer: MaskRay.
brad requested review of this revision.
Herald added a project: clang.

Similar to D116843  for Gnu.cpp


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119656

Files:
  clang/lib/Driver/ToolChains/DragonFly.cpp
  clang/test/Driver/dragonfly.c


Index: clang/test/Driver/dragonfly.c
===
--- clang/test/Driver/dragonfly.c
+++ clang/test/Driver/dragonfly.c
@@ -4,4 +4,9 @@
 // CHECK: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-dragonfly"
 // CHECK: ld{{.*}}" "--eh-frame-hdr" "-dynamic-linker" 
"/usr/libexec/ld-elf.so.{{.*}}" "--hash-style=gnu" "--enable-new-dtags" "-o" 
"a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "{{.*}}.o" 
"-L{{.*}}gcc{{.*}}" "-rpath" "{{.*}}gcc{{.*}}" "-lc" "-lgcc" "{{.*}}crtend.o" 
"{{.*}}crtn.o"
 
-
+// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -### %s --target=x86_64-pc-dragonfly -r \
+// RUN:   2>&1 | FileCheck %s --check-prefix=RELOCATABLE
+// RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-l
+// RELOCATABLE-NOT: {{.*}}crt{{[^.]+}}.o
Index: clang/lib/Driver/ToolChains/DragonFly.cpp
===
--- clang/lib/Driver/ToolChains/DragonFly.cpp
+++ clang/lib/Driver/ToolChains/DragonFly.cpp
@@ -91,7 +91,8 @@
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!Args.hasArg(options::OPT_shared)) {
   if (Args.hasArg(options::OPT_pg))
 CmdArgs.push_back(
@@ -119,7 +120,8 @@
 
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 CmdArgs.push_back("-L/usr/lib/gcc80");
 
 if (!Args.hasArg(options::OPT_static)) {
@@ -158,7 +160,8 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
   CmdArgs.push_back(
   Args.MakeArgString(getToolChain().GetFilePath("crtendS.o")));


Index: clang/test/Driver/dragonfly.c
===
--- clang/test/Driver/dragonfly.c
+++ clang/test/Driver/dragonfly.c
@@ -4,4 +4,9 @@
 // CHECK: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-dragonfly"
 // CHECK: ld{{.*}}" "--eh-frame-hdr" "-dynamic-linker" "/usr/libexec/ld-elf.so.{{.*}}" "--hash-style=gnu" "--enable-new-dtags" "-o" "a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" "{{.*}}.o" "-L{{.*}}gcc{{.*}}" "-rpath" "{{.*}}gcc{{.*}}" "-lc" "-lgcc" "{{.*}}crtend.o" "{{.*}}crtn.o"
 
-
+// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -### %s --target=x86_64-pc-dragonfly -r \
+// RUN:   2>&1 | FileCheck %s --check-prefix=RELOCATABLE
+// RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-l
+// RELOCATABLE-NOT: {{.*}}crt{{[^.]+}}.o
Index: clang/lib/Driver/ToolChains/DragonFly.cpp
===
--- clang/lib/Driver/ToolChains/DragonFly.cpp
+++ clang/lib/Driver/ToolChains/DragonFly.cpp
@@ -91,7 +91,8 @@
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!Args.hasArg(options::OPT_shared)) {
   if (Args.hasArg(options::OPT_pg))
 CmdArgs.push_back(
@@ -119,7 +120,8 @@
 
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 CmdArgs.push_back("-L/usr/lib/gcc80");
 
 if (!Args.hasArg(options::OPT_static)) {
@@ -158,7 +160,8 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
   CmdArgs.push_back(
   Args.MakeArgString(getToolChain().GetFilePath("crtendS.o")));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119655: [Driver][NetBSD] -r: imply -nostdlib like GCC

[Brad Smith via Phabricator via cfe-commits]

brad created this revision.
brad added a reviewer: MaskRay.
brad requested review of this revision.
Herald added a project: clang.

Similar to D116843  for Gnu.cpp


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119655

Files:
  clang/lib/Driver/ToolChains/NetBSD.cpp
  clang/test/Driver/netbsd.c


Index: clang/test/Driver/netbsd.c
===
--- clang/test/Driver/netbsd.c
+++ clang/test/Driver/netbsd.c
@@ -467,3 +467,11 @@
 // RUN: %clang -target powerpc-unknown-netbsd -### -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=POWERPC-SECUREPLT %s
 // POWERPC-SECUREPLT: "-target-feature" "+secure-plt"
+
+// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -no-canonical-prefixes -target x86_64-unknown-netbsd -r \
+// RUN: --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=RELOCATABLE %s
+// RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-l
+// RELOCATABLE-NOT: crt{{[^.]+}}.o
Index: clang/lib/Driver/ToolChains/NetBSD.cpp
===
--- clang/lib/Driver/ToolChains/NetBSD.cpp
+++ clang/lib/Driver/ToolChains/NetBSD.cpp
@@ -236,7 +236,8 @@
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!Args.hasArg(options::OPT_shared)) {
   CmdArgs.push_back(
   Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
@@ -294,7 +295,8 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 // Use the static OpenMP runtime with -static-openmp
 bool StaticOpenMP = Args.hasArg(options::OPT_static_openmp) &&
 !Args.hasArg(options::OPT_static);
@@ -330,7 +332,8 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
   CmdArgs.push_back(
   Args.MakeArgString(ToolChain.GetFilePath("crtendS.o")));


Index: clang/test/Driver/netbsd.c
===
--- clang/test/Driver/netbsd.c
+++ clang/test/Driver/netbsd.c
@@ -467,3 +467,11 @@
 // RUN: %clang -target powerpc-unknown-netbsd -### -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=POWERPC-SECUREPLT %s
 // POWERPC-SECUREPLT: "-target-feature" "+secure-plt"
+
+// -r suppresses default -l and crt*.o like -nostdlib.
+// RUN: %clang -no-canonical-prefixes -target x86_64-unknown-netbsd -r \
+// RUN: --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=RELOCATABLE %s
+// RELOCATABLE: "-r"
+// RELOCATABLE-NOT: "-l
+// RELOCATABLE-NOT: crt{{[^.]+}}.o
Index: clang/lib/Driver/ToolChains/NetBSD.cpp
===
--- clang/lib/Driver/ToolChains/NetBSD.cpp
+++ clang/lib/Driver/ToolChains/NetBSD.cpp
@@ -236,7 +236,8 @@
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!Args.hasArg(options::OPT_shared)) {
   CmdArgs.push_back(
   Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
@@ -294,7 +295,8 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 // Use the static OpenMP runtime with -static-openmp
 bool StaticOpenMP = Args.hasArg(options::OPT_static_openmp) &&
 !Args.hasArg(options::OPT_static);
@@ -330,7 +332,8 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
   CmdArgs.push_back(
   Args.MakeArgString(ToolChain.GetFilePath("crtendS.o")));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood updated this revision to Diff 408235.
LegalizeAdulthood added a comment.

- Move small methods to class decl


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

https://reviews.llvm.org/D117522

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-to-enum.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum2.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum3.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
@@ -0,0 +1,190 @@
+// RUN: %check_clang_tidy %s modernize-macro-to-enum %t -- -- -I%S/Inputs/modernize-macro-to-enum
+
+#if 1
+#include "modernize-macro-to-enum.h"
+
+// These macros are skipped due to being inside a conditional compilation block.
+#define GOO_RED 1
+#define GOO_GREEN 2
+#define GOO_BLUE 3
+
+#endif
+
+#define RED 0xFF
+#define GREEN 0x00FF00
+#define BLUE 0xFF
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum [modernize-macro-to-enum]
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'RED' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GREEN' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BLUE' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: RED = 0xFF,
+// CHECK-FIXES-NEXT: GREEN = 0x00FF00,
+// CHECK-FIXES-NEXT: BLUE = 0xFF
+// CHECK-FIXES-NEXT: };
+
+// Verify that comments are preserved.
+#define CoordModeOrigin 0   /* relative to the origin */
+#define CoordModePrevious   1   /* relative to previous point */
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModeOrigin' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModePrevious' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: CoordModeOrigin = 0,   /* relative to the origin */
+// CHECK-FIXES-NEXT: CoordModePrevious =   1   /* relative to previous point */
+// CHECK-FIXES-NEXT: };
+
+// Verify that multiline comments are preserved.
+#define BadDrawable 9   /* parameter not a Pixmap or Window */
+#define BadAccess   10  /* depending on context:
+- key/button already grabbed
+- attempt to free an illegal 
+  cmap entry 
+- attempt to store into a read-only 
+  color map entry. */
+// - attempt to modify the access control
+//   list from other than the local host.
+//
+#define BadAlloc11  /* insufficient resources */
+// CHECK-MESSAGES: :[[@LINE-11]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadDrawable' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadAccess' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BadAlloc' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: BadDrawable = 9,   /* parameter not a Pixmap or Window */
+// CHECK-FIXES-NEXT: BadAccess =   10,  /* depending on context:
+// CHECK-FIXES-NEXT: - key/button already grabbed
+// CHECK-FIXES-NEXT: - attempt to free an illegal 
+// CHECK-FIXES-NEXT:   cmap entry 
+// CHECK-FIXES-NEXT: - attempt to store into a read-only 
+// CHECK-FIXES-NEXT:   color map entry. */
+// CHECK-FIXES-NEXT: // - attempt to modify the access control
+// CHECK-FIXES-NEXT: //   list from other than the local host.
+// CHECK-FIXES-NEXT:   

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood updated this revision to Diff 408233.
LegalizeAdulthood added a comment.

- clang-format


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

https://reviews.llvm.org/D117522

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-to-enum.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum2.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum3.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
@@ -0,0 +1,190 @@
+// RUN: %check_clang_tidy %s modernize-macro-to-enum %t -- -- -I%S/Inputs/modernize-macro-to-enum
+
+#if 1
+#include "modernize-macro-to-enum.h"
+
+// These macros are skipped due to being inside a conditional compilation block.
+#define GOO_RED 1
+#define GOO_GREEN 2
+#define GOO_BLUE 3
+
+#endif
+
+#define RED 0xFF
+#define GREEN 0x00FF00
+#define BLUE 0xFF
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum [modernize-macro-to-enum]
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'RED' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GREEN' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BLUE' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: RED = 0xFF,
+// CHECK-FIXES-NEXT: GREEN = 0x00FF00,
+// CHECK-FIXES-NEXT: BLUE = 0xFF
+// CHECK-FIXES-NEXT: };
+
+// Verify that comments are preserved.
+#define CoordModeOrigin 0   /* relative to the origin */
+#define CoordModePrevious   1   /* relative to previous point */
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModeOrigin' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModePrevious' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: CoordModeOrigin = 0,   /* relative to the origin */
+// CHECK-FIXES-NEXT: CoordModePrevious =   1   /* relative to previous point */
+// CHECK-FIXES-NEXT: };
+
+// Verify that multiline comments are preserved.
+#define BadDrawable 9   /* parameter not a Pixmap or Window */
+#define BadAccess   10  /* depending on context:
+- key/button already grabbed
+- attempt to free an illegal 
+  cmap entry 
+- attempt to store into a read-only 
+  color map entry. */
+// - attempt to modify the access control
+//   list from other than the local host.
+//
+#define BadAlloc11  /* insufficient resources */
+// CHECK-MESSAGES: :[[@LINE-11]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadDrawable' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadAccess' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BadAlloc' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: BadDrawable = 9,   /* parameter not a Pixmap or Window */
+// CHECK-FIXES-NEXT: BadAccess =   10,  /* depending on context:
+// CHECK-FIXES-NEXT: - key/button already grabbed
+// CHECK-FIXES-NEXT: - attempt to free an illegal 
+// CHECK-FIXES-NEXT:   cmap entry 
+// CHECK-FIXES-NEXT: - attempt to store into a read-only 
+// CHECK-FIXES-NEXT:   color map entry. */
+// CHECK-FIXES-NEXT: // - attempt to modify the access control
+// CHECK-FIXES-NEXT: //   list from other than the local host.
+// CHECK-FIXES-NEXT: 

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood updated this revision to Diff 408231.
LegalizeAdulthood added a comment.
Herald added subscribers: kbarton, nemanjai.

- Rejects macros that are used in preprocessor conditionals
- cppcoreguidelines-macro-to-enum is an alias for modernize-macro-to-enum
- Document alias


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

https://reviews.llvm.org/D117522

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-to-enum.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum2.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum3.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
@@ -0,0 +1,190 @@
+// RUN: %check_clang_tidy %s modernize-macro-to-enum %t -- -- -I%S/Inputs/modernize-macro-to-enum
+
+#if 1
+#include "modernize-macro-to-enum.h"
+
+// These macros are skipped due to being inside a conditional compilation block.
+#define GOO_RED 1
+#define GOO_GREEN 2
+#define GOO_BLUE 3
+
+#endif
+
+#define RED 0xFF
+#define GREEN 0x00FF00
+#define BLUE 0xFF
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum [modernize-macro-to-enum]
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'RED' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GREEN' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BLUE' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: RED = 0xFF,
+// CHECK-FIXES-NEXT: GREEN = 0x00FF00,
+// CHECK-FIXES-NEXT: BLUE = 0xFF
+// CHECK-FIXES-NEXT: };
+
+// Verify that comments are preserved.
+#define CoordModeOrigin 0   /* relative to the origin */
+#define CoordModePrevious   1   /* relative to previous point */
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModeOrigin' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModePrevious' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: CoordModeOrigin = 0,   /* relative to the origin */
+// CHECK-FIXES-NEXT: CoordModePrevious =   1   /* relative to previous point */
+// CHECK-FIXES-NEXT: };
+
+// Verify that multiline comments are preserved.
+#define BadDrawable 9   /* parameter not a Pixmap or Window */
+#define BadAccess   10  /* depending on context:
+- key/button already grabbed
+- attempt to free an illegal 
+  cmap entry 
+- attempt to store into a read-only 
+  color map entry. */
+// - attempt to modify the access control
+//   list from other than the local host.
+//
+#define BadAlloc11  /* insufficient resources */
+// CHECK-MESSAGES: :[[@LINE-11]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadDrawable' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadAccess' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BadAlloc' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: BadDrawable = 9,   /* parameter not a Pixmap or Window */
+// CHECK-FIXES-NEXT: BadAccess =   10,  /* depending on context:
+// CHECK-FIXES-NEXT: - key/button already grabbed
+// CHECK-FIXES-NEXT: - attempt to free an illegal 
+// CHECK-FIXES-NEXT:   cmap entry 
+// CHECK-FIXES-NEXT: - attempt to store into a read-only 
+// CHECK-FIXES-NEXT:   color map entry. */
+// CHECK-FIXES-NEXT: // - attempt 

[PATCH] D119651: [clang] Fix evaluation context type for consteval function calls in instantiations

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a subscriber: cor3ntin.
erichkeane added a comment.

I'm pretty sure @cor3ntin just messed with this quite a bit, so I'd like to 
hear his thoughts on this. But this generally looks right on first blush.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119651

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

[PATCH] D119646: [clang] Allow consteval functions in default arguments

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/AST/Decl.cpp:2816
   if (auto *E = dyn_cast_or_null(Arg))
-return E->getSubExpr();
+if (!isa(E))
+  return E->getSubExpr();

So what is happening here?  I would still expect us to give the proper 
default-arg in this case?  What subtly am I missing?



Comment at: clang/lib/Sema/SemaExpr.cpp:18967
 
+  void VisitConstantExpr(ConstantExpr *E) {
+// Don't mark declarations within a ConstantExpression, as this expression

This part makes sense to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119646

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

[PATCH] D119601: [analyzer] Refactor makeNull to makeNullWithWidth

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> steakhal performed systems testing across a large set of open source projects.

I'm curious if any findings there caused you to make changes in the patch. If 
so it'd be great to reduce them into test cases so that other people didn't 
make the same mistake when they edit your new code.




Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:885
 
-auto NullVal = C.getSValBuilder().makeNull();
+auto NullVal = C.getSValBuilder().makeNullWithType(CallExpr->getType());
 // Explicitly tracking the region as null.

NoQ wrote:
> I suspect that this type is going to be `bool` which is probably not what you 
> want.
I think it makes sense to add an assertion in `makeNullWithType()` to protect 
against such cases. This will probably also allow you to cover this with a test 
case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119601

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

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 408226.
tianshilei1992 marked an inline comment as done.
tianshilei1992 added a comment.

add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116261

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_ast_print.cpp

Index: clang/test/OpenMP/atomic_ast_print.cpp
===
--- clang/test/OpenMP/atomic_ast_print.cpp
+++ clang/test/OpenMP/atomic_ast_print.cpp
@@ -20,6 +20,7 @@
 
 template 
 T foo(T argc) {
+  T v = T();
   T c = T();
   T b = T();
   T a = T();
@@ -45,6 +46,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic seq_cst
   a++;
@@ -68,6 +75,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare seq_cst
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture seq_cst
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare seq_cst capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture seq_cst
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic
   a++;
@@ -91,6 +104,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare acq_rel
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture acq_rel
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare acq_rel capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture acq_rel
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic
   a++;
@@ -114,6 +133,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare acquire
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture acquire
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare acquire capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture acquire
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic release
   a++;
@@ -137,6 +162,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare release
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture release
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare release capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture release
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic relaxed
   a++;
@@ -160,6 +191,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare relaxed
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture relaxed
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare relaxed capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture relaxed
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic hint(6)
   a++;
@@ -183,6 +220,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare hint(6)
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture hint(6)
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare hint(6) capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture hint(6)
+  { v = a == b; if (v) a = c; }
 #endif
   return T();
 }
@@ -215,6 +258,22 @@
 // CHECK-51-NEXT: {
 // CHECK-51-NEXT: a = a == b ? c : a;
 // CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare capture
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a;
+// CHECK-51-NEXT: a = a > b ? b : a;
+// CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare capture
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a;
+// CHECK-51-NEXT: a = a < b ? b : a;
+// CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare capture
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a == b;
+// CHECK-51-NEXT: if (v)
+// CHECK-51-NEXT: a = c;
+// CHECK-51-NEXT: }
 // CHECK-NEXT: #pragma omp atomic seq_cst
 // CHECK-NEXT: a++;
 // CHECK-NEXT: #pragma omp atomic read seq_cst
@@ -242,6 +301,22 @@
 // CHECK-51-NEXT: {
 // CHECK-51-NEXT: a = a == b ? c : a;
 // CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare capture seq_cst
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a;
+// CHECK-51-NEXT: a = a > b ? b : a;
+// CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare seq_cst capture
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a;
+// CHECK-51-NEXT: a = a < b ? b : a;
+// CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare capture seq_cst
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a == b;
+// CHECK-51-NEXT: if (v)
+// CHECK-51-NEXT: a = c;
+// CHECK-51-NEXT: }
 // CHECK-NEXT: #pragma omp atomic
 // CHECK-NEXT: a++;
 // CHECK-NEXT: #pragma omp atomic read
@@ -269,6 +344,22 @@
 // CHECK-51-NEXT: {
 // CHECK-51-NEXT: a = a == b ? c : a;
 // CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare capture acq_rel
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a;
+// CHECK-51-NEXT: a 

[PATCH] D119597: [clang-format][NFC] Give State.Stack.back() a meaningful name

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks updated this revision to Diff 408222.
HazardyKnusperkeks marked an inline comment as done.
HazardyKnusperkeks added a comment.
This revision is now accepted and ready to land.

Added const


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

https://reviews.llvm.org/D119597

Files:
  clang/lib/Format/ContinuationIndenter.cpp

Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -263,9 +263,10 @@
   if (Style.Language == FormatStyle::LK_TextProto) {
 // We need this in order to deal with the bin packing of text fields at
 // global scope.
-State.Stack.back().AvoidBinPacking = true;
-State.Stack.back().BreakBeforeParameter = true;
-State.Stack.back().AlignColons = false;
+auto  = State.Stack.back();
+CurrentState.AvoidBinPacking = true;
+CurrentState.BreakBeforeParameter = true;
+CurrentState.AlignColons = false;
   }
 
   // The first token has already been indented and thus consumed.
@@ -276,8 +277,9 @@
 bool ContinuationIndenter::canBreak(const LineState ) {
   const FormatToken  = *State.NextToken;
   const FormatToken  = *Current.Previous;
+  const auto  = State.Stack.back();
   assert( == Current.Previous);
-  if (!Current.CanBreakBefore && !(State.Stack.back().BreakBeforeClosingBrace &&
+  if (!Current.CanBreakBefore && !(CurrentState.BreakBeforeClosingBrace &&
Current.closesBlockOrBlockTypeList(Style)))
 return false;
   // The opening "{" of a braced list has to be on the same line as the first
@@ -296,7 +298,7 @@
   State.LowestLevelOnLine < State.StartOfLineLevel &&
   State.LowestLevelOnLine < Current.NestingLevel)
 return false;
-  if (Current.isMemberAccess() && State.Stack.back().ContainsUnwrappedBuilder)
+  if (Current.isMemberAccess() && CurrentState.ContainsUnwrappedBuilder)
 return false;
 
   // Don't create a 'hanging' indent if there are multiple blocks in a single
@@ -316,18 +318,19 @@
   // If binary operators are moved to the next line (including commas for some
   // styles of constructor initializers), that's always ok.
   if (!Current.isOneOf(TT_BinaryOperator, tok::comma) &&
-  State.Stack.back().NoLineBreakInOperand)
+  CurrentState.NoLineBreakInOperand)
 return false;
 
   if (Previous.is(tok::l_square) && Previous.is(TT_ObjCMethodExpr))
 return false;
 
-  return !State.Stack.back().NoLineBreak;
+  return !CurrentState.NoLineBreak;
 }
 
 bool ContinuationIndenter::mustBreak(const LineState ) {
   const FormatToken  = *State.NextToken;
   const FormatToken  = *Current.Previous;
+  const auto  = State.Stack.back();
   if (Style.BraceWrapping.BeforeLambdaBody && Current.CanBreakBefore &&
   Current.is(TT_LambdaLBrace) && Previous.isNot(TT_LineComment)) {
 auto LambdaBodyLength = getLengthToMatchingParen(Current, State.Stack);
@@ -335,10 +338,10 @@
   }
   if (Current.MustBreakBefore || Current.is(TT_InlineASMColon))
 return true;
-  if (State.Stack.back().BreakBeforeClosingBrace &&
+  if (CurrentState.BreakBeforeClosingBrace &&
   Current.closesBlockOrBlockTypeList(Style))
 return true;
-  if (State.Stack.back().BreakBeforeClosingParen && Current.is(tok::r_paren))
+  if (CurrentState.BreakBeforeClosingParen && Current.is(tok::r_paren))
 return true;
   if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)
 return true;
@@ -349,7 +352,7 @@
 return true;
   // Avoid producing inconsistent states by requiring breaks where they are not
   // permitted for C# generic type constraints.
-  if (State.Stack.back().IsCSharpGenericTypeConstraint &&
+  if (CurrentState.IsCSharpGenericTypeConstraint &&
   Previous.isNot(TT_CSharpGenericTypeConstraintComma))
 return false;
   if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) ||
@@ -364,10 +367,10 @@
 Previous.isNot(tok::question)) ||
(!Style.BreakBeforeTernaryOperators &&
 Previous.is(TT_ConditionalExpr))) &&
-  State.Stack.back().BreakBeforeParameter && !Current.isTrailingComment() &&
+  CurrentState.BreakBeforeParameter && !Current.isTrailingComment() &&
   !Current.isOneOf(tok::r_paren, tok::r_brace))
 return true;
-  if (State.Stack.back().IsChainedConditional &&
+  if (CurrentState.IsChainedConditional &&
   ((Style.BreakBeforeTernaryOperators && Current.is(TT_ConditionalExpr) &&
 Current.is(tok::colon)) ||
(!Style.BreakBeforeTernaryOperators && Previous.is(TT_ConditionalExpr) &&
@@ -389,7 +392,7 @@
   if (BreakConstructorInitializersToken.is(TT_CtorInitializerColon) &&
   (State.Column + State.Line->Last->TotalLength - Previous.TotalLength >
getColumnLimit(State) ||
-   State.Stack.back().BreakBeforeParameter) &&
+   CurrentState.BreakBeforeParameter) &&
   

[PATCH] D119138: [clang-format] Further improve support for requires expressions

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks marked 3 inline comments as done.
HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2835-2841
+  // FIXME: We need an annotation on the paren to really know if it is a
+  // function call:
+  // ... foo() && requires ...
+  // or a declaration:
+  // void foo() && requires ...
+  // there is no distinction possible right now. We go for the latter,
+  // because it's more likely to appear in code.

HazardyKnusperkeks wrote:
> Quuxplusone wrote:
> > I think it's weird that your heuristic parses backward rather than forward. 
> > I would think that the next token //after// the `requires` keyword tells 
> > you what it is with pretty high probability:
> > `requires requires` — it's a clause
> > `requires identifier` — it's a clause
> > `requires {` — it's an expression
> > `requires (` — unclear, apply further heuristics
> > 
> > Or are those heuristics already present in trunk, and this PR is just 
> > dealing with the "unclear" case?
> That would be so much better, but I can't easily look forward. `Next` is 
> still `nullptr`, until I call `nextToken()`, but then I'm already moved along.
> 
> But this got me thinking, at least for the easy stuff I can just go forward 
> and don't start on the keyword in `parseRequiresClause()` and 
> `parseRequiresExpression()`. The paren case is more tricky, but I will try 
> something.
Present in main everything is a clause, except for requires expressions in a 
constraint expression. So the stuff where you use the requires expression in a 
"normal" boolean expression are misparsed and thus most likely misformatted.

There is actually a `peekToken()`, let's see if this is better.


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

https://reviews.llvm.org/D119138

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

[PATCH] D119138: [clang-format] Further improve support for requires expressions

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks updated this revision to Diff 408221.
HazardyKnusperkeks added a comment.

New approach for identifying the expressions.


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

https://reviews.llvm.org/D119138

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -141,6 +141,9 @@
 "  { t.foo() };\n"
 "} && Bar && Baz;");
   ASSERT_EQ(Tokens.size(), 35u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[9], tok::l_paren, TT_RequiresExpressionLParen);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_RequiresExpressionLBrace);
   EXPECT_TOKEN(Tokens[23], tok::ampamp, TT_BinaryOperator);
   EXPECT_TOKEN(Tokens[28], tok::ampamp, TT_BinaryOperator);
 
@@ -148,6 +151,7 @@
 "requires C1 && (C21 || C22 && C2e) && C3\n"
 "struct Foo;");
   ASSERT_EQ(Tokens.size(), 36u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresClause);
   EXPECT_TOKEN(Tokens[6], tok::identifier, TT_Unknown);
   EXPECT_EQ(Tokens[6]->FakeLParens.size(), 1u);
   EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_BinaryOperator);
@@ -163,6 +167,7 @@
"requires (C1 && (C21 || C22 && C2e) && C3)\n"
"struct Foo;");
   ASSERT_EQ(Tokens.size(), 38u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresClause);
   EXPECT_TOKEN(Tokens[7], tok::identifier, TT_Unknown);
   EXPECT_EQ(Tokens[7]->FakeLParens.size(), 1u);
   EXPECT_TOKEN(Tokens[11], tok::ampamp, TT_BinaryOperator);
@@ -173,6 +178,127 @@
   EXPECT_EQ(Tokens[32]->FakeRParens, 1u);
   EXPECT_TOKEN(Tokens[33], tok::r_paren, TT_Unknown);
   EXPECT_TRUE(Tokens[33]->ClosesRequiresClause);
+
+  Tokens = annotate("template \n"
+"void foo(T) noexcept requires Bar;");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[11], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("template \n"
+"struct S {\n"
+"  void foo() const requires Bar;\n"
+"  void bar() const & requires Baz;\n"
+"  void bar() && requires Baz2;\n"
+"  void baz() const & noexcept requires Baz;\n"
+"  void baz() && noexcept requires Baz2;\n"
+"};\n"
+"\n"
+"void S::bar() const & requires Baz { }");
+  ASSERT_EQ(Tokens.size(), 85u) << Tokens;
+  EXPECT_TOKEN(Tokens[13], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[25], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[36], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[49], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[61], tok::kw_requires, TT_RequiresClause);
+  EXPECT_TOKEN(Tokens[77], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("void Class::member() && requires(Constant) {}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("void Class::member() && requires(Constant) {}");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::kw_requires, TT_RequiresClause);
+
+  Tokens =
+  annotate("void Class::member() && requires(Namespace::Constant) {}");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("void Class::member() && requires(typename "
+"Namespace::Outer::Inner::Constant) {}");
+  ASSERT_EQ(Tokens.size(), 24u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::kw_requires, TT_RequiresClause);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {
+  auto Tokens = annotate("bool b = requires(int i) { i + 5; };");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_RequiresExpressionLParen);
+  EXPECT_TOKEN(Tokens[8], tok::l_brace, TT_RequiresExpressionLBrace);
+
+  Tokens = annotate("if (requires(int i) { i + 5; }) return;");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_RequiresExpressionLParen);
+  EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_RequiresExpressionLBrace);
+
+  Tokens = annotate("if (func() && requires(int i) { i + 5; }) return;");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::kw_requires, TT_RequiresExpression);
+  EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_RequiresExpressionLParen);
+  EXPECT_TOKEN(Tokens[11], 

[PATCH] D119651: [clang] Fix evaluation context type for consteval function calls in instantiations

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron updated this revision to Diff 408220.
Izaron added a comment.

(Fast test comment 80 character line fix)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119651

Files:
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -613,6 +613,24 @@
 
 } // namespace unevaluated
 
+namespace templated {
+
+consteval int f(int v) {
+  return v;
+}
+
+template 
+consteval int g(T a) {
+  // Previously this call was rejected due to incorrect evaluation context
+  // type in instantiations. Now we show that this call is OK.
+  int n = f(a);
+  return n;
+}
+
+static_assert(g(100) == 100);
+
+} // namespace templated
+
 namespace PR50779 {
 struct derp {
   int b = 0;
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5326,8 +5326,13 @@
 Var->setImplicitlyInline();
 
   if (OldVar->getInit()) {
-EnterExpressionEvaluationContext Evaluated(
-*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, Var);
+ExpressionEvaluationContext Context =
+ExpressionEvaluationContext::PotentiallyEvaluated;
+// If current context is constant evaluated, the variable initializer
+// context is also constant evaluated
+if (isConstantEvaluated())
+  Context = ExpressionEvaluationContext::ConstantEvaluated;
+EnterExpressionEvaluationContext Evaluated(*this, Context, Var);
 
 // Instantiate the initializer.
 ExprResult Init;


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -613,6 +613,24 @@
 
 } // namespace unevaluated
 
+namespace templated {
+
+consteval int f(int v) {
+  return v;
+}
+
+template 
+consteval int g(T a) {
+  // Previously this call was rejected due to incorrect evaluation context
+  // type in instantiations. Now we show that this call is OK.
+  int n = f(a);
+  return n;
+}
+
+static_assert(g(100) == 100);
+
+} // namespace templated
+
 namespace PR50779 {
 struct derp {
   int b = 0;
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5326,8 +5326,13 @@
 Var->setImplicitlyInline();
 
   if (OldVar->getInit()) {
-EnterExpressionEvaluationContext Evaluated(
-*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, Var);
+ExpressionEvaluationContext Context =
+ExpressionEvaluationContext::PotentiallyEvaluated;
+// If current context is constant evaluated, the variable initializer
+// context is also constant evaluated
+if (isConstantEvaluated())
+  Context = ExpressionEvaluationContext::ConstantEvaluated;
+EnterExpressionEvaluationContext Evaluated(*this, Context, Var);
 
 // Instantiate the initializer.
 ExprResult Init;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119651: [clang] Fix evaluation context type for consteval function calls in instantiations

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron created this revision.
Izaron added reviewers: aaron.ballman, erichkeane, rsmith.
Izaron requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When instantiating a VarDecl initializing sub-AST, the evaluation
context was only PotentiallyEvaluated, even if we are inside a consteval
function. It dictated clang to construct a redundant ConstantExpr node, which
could be not evaluable.

Fixes https://github.com/llvm/llvm-project/issues/51182
and https://github.com/llvm/llvm-project/issues/52648


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119651

Files:
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -613,6 +613,23 @@
 
 } // namespace unevaluated
 
+namespace templated {
+
+consteval int f(int v) {
+  return v;
+}
+
+template 
+consteval int g(T a) {
+  // Previously this call was rejected due to incorrect evaluation context 
type in instantiations. Now we show that this call is OK.
+  int n = f(a);
+  return n;
+}
+
+static_assert(g(100) == 100);
+
+} // namespace templated
+
 namespace PR50779 {
 struct derp {
   int b = 0;
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5326,8 +5326,13 @@
 Var->setImplicitlyInline();
 
   if (OldVar->getInit()) {
-EnterExpressionEvaluationContext Evaluated(
-*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, Var);
+ExpressionEvaluationContext Context =
+ExpressionEvaluationContext::PotentiallyEvaluated;
+// If current context is constant evaluated, the variable initializer
+// context is also constant evaluated
+if (isConstantEvaluated())
+  Context = ExpressionEvaluationContext::ConstantEvaluated;
+EnterExpressionEvaluationContext Evaluated(*this, Context, Var);
 
 // Instantiate the initializer.
 ExprResult Init;


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -613,6 +613,23 @@
 
 } // namespace unevaluated
 
+namespace templated {
+
+consteval int f(int v) {
+  return v;
+}
+
+template 
+consteval int g(T a) {
+  // Previously this call was rejected due to incorrect evaluation context type in instantiations. Now we show that this call is OK.
+  int n = f(a);
+  return n;
+}
+
+static_assert(g(100) == 100);
+
+} // namespace templated
+
 namespace PR50779 {
 struct derp {
   int b = 0;
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5326,8 +5326,13 @@
 Var->setImplicitlyInline();
 
   if (OldVar->getInit()) {
-EnterExpressionEvaluationContext Evaluated(
-*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, Var);
+ExpressionEvaluationContext Context =
+ExpressionEvaluationContext::PotentiallyEvaluated;
+// If current context is constant evaluated, the variable initializer
+// context is also constant evaluated
+if (isConstantEvaluated())
+  Context = ExpressionEvaluationContext::ConstantEvaluated;
+EnterExpressionEvaluationContext Evaluated(*this, Context, Var);
 
 // Instantiate the initializer.
 ExprResult Init;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119648: [clang-format] Fix PointerAlignment within lambdas in a multi-variable declaration statement.

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:8415-8417
+  verifyFormat("if (int *p, *q; p != q) {\n  p = p->next;\n}", Style);
+  verifyFormat("/*comment*/ if (int *p, *q; p != q) {\n  p = p->next;\n}",
+   Style);

Same as lines 8412-8414.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119648

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

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked 3 inline comments as done.
tianshilei1992 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11320
+  bool MutexClauseEncountered = false;
+  llvm::SmallSet EncounteredAtomicKinds;
   for (const OMPClause *C : Clauses) {

ABataev wrote:
> ABataev wrote:
> > tianshilei1992 wrote:
> > > ABataev wrote:
> > > > tianshilei1992 wrote:
> > > > > ABataev wrote:
> > > > > > tianshilei1992 wrote:
> > > > > > > ABataev wrote:
> > > > > > > > SmallVector with 2 elements should be enough, no need to use 
> > > > > > > > SmallSet here.
> > > > > > > `SmallVector` doesn't provide `contains`, which we use to check 
> > > > > > > if `OMPC_compare` and `OMPC_capture` exist at the same time. 
> > > > > > > Using `SmallVector` requires us to have a complicated comparison, 
> > > > > > > for example:
> > > > > > > ```
> > > > > > > (EncounteredAtomicKinds[0] == OMPC_compare && 
> > > > > > > EncounteredAtomicKinds[1] == OMPC_capture) || 
> > > > > > > (EncounteredAtomicKinds[1] == OMPC_compare && 
> > > > > > > EncounteredAtomicKinds[0] == OMPC_capture)
> > > > > > > ```
> > > > > > > which is not as neat as `SmallSet`.
> > > > > > Use llvm::find
> > > > > That is linear time. Why it is better than `SmallSet`?
> > > > Because there are only 2 elements, no?
> > > It could have more. If user writes it like `atomic compare compare 
> > > capture capture capture`, then it will have 5 elements. 
> > > `SmallSet::insert` returns a `pair` and we know if it is already in 
> > > the set, which I have not added it yet but I'll. `SmallVector` then is 
> > > not that easy to use. Of course you could say do `llvm::find` beforehand 
> > > but it's not neat.
> > Is this correct at all?
> You still can scan over the small vector and check if it has specified clause 
> already. 
It's not correct. We can easily check via the following code:
```
if (!EncounteredAtomicKinds.insert(C->getClauseKind()).second) {
  // emit an error
}
```
Using `SmallVector` we need to:
```
if (llvm::find(EncounteredAtomicKinds.begin(), EncounteredAtomicKinds.end(), 
C->getClauseKind() != EncounteredAtomicKinds.end()) {
  // emit an error
} else {
  EncounteredAtomicKinds.push_back(C->getClauseKind());
}
```
It's obvious which one looks better and have better readability. I can't see 
any point of using `SmallVector` here, unless there is huge performance 
difference, which I really doubt that matters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116261

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

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11320
+  bool MutexClauseEncountered = false;
+  llvm::SmallSet EncounteredAtomicKinds;
   for (const OMPClause *C : Clauses) {

ABataev wrote:
> tianshilei1992 wrote:
> > ABataev wrote:
> > > tianshilei1992 wrote:
> > > > ABataev wrote:
> > > > > tianshilei1992 wrote:
> > > > > > ABataev wrote:
> > > > > > > SmallVector with 2 elements should be enough, no need to use 
> > > > > > > SmallSet here.
> > > > > > `SmallVector` doesn't provide `contains`, which we use to check if 
> > > > > > `OMPC_compare` and `OMPC_capture` exist at the same time. Using 
> > > > > > `SmallVector` requires us to have a complicated comparison, for 
> > > > > > example:
> > > > > > ```
> > > > > > (EncounteredAtomicKinds[0] == OMPC_compare && 
> > > > > > EncounteredAtomicKinds[1] == OMPC_capture) || 
> > > > > > (EncounteredAtomicKinds[1] == OMPC_compare && 
> > > > > > EncounteredAtomicKinds[0] == OMPC_capture)
> > > > > > ```
> > > > > > which is not as neat as `SmallSet`.
> > > > > Use llvm::find
> > > > That is linear time. Why it is better than `SmallSet`?
> > > Because there are only 2 elements, no?
> > It could have more. If user writes it like `atomic compare compare capture 
> > capture capture`, then it will have 5 elements. `SmallSet::insert` 
> > returns a `pair` and we know if it is already in the set, which I have not 
> > added it yet but I'll. `SmallVector` then is not that easy to use. Of 
> > course you could say do `llvm::find` beforehand but it's not neat.
> Is this correct at all?
You still can scan over the small vector and check if it has specified clause 
already. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116261

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

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11320
+  bool MutexClauseEncountered = false;
+  llvm::SmallSet EncounteredAtomicKinds;
   for (const OMPClause *C : Clauses) {

tianshilei1992 wrote:
> ABataev wrote:
> > tianshilei1992 wrote:
> > > ABataev wrote:
> > > > tianshilei1992 wrote:
> > > > > ABataev wrote:
> > > > > > SmallVector with 2 elements should be enough, no need to use 
> > > > > > SmallSet here.
> > > > > `SmallVector` doesn't provide `contains`, which we use to check if 
> > > > > `OMPC_compare` and `OMPC_capture` exist at the same time. Using 
> > > > > `SmallVector` requires us to have a complicated comparison, for 
> > > > > example:
> > > > > ```
> > > > > (EncounteredAtomicKinds[0] == OMPC_compare && 
> > > > > EncounteredAtomicKinds[1] == OMPC_capture) || 
> > > > > (EncounteredAtomicKinds[1] == OMPC_compare && 
> > > > > EncounteredAtomicKinds[0] == OMPC_capture)
> > > > > ```
> > > > > which is not as neat as `SmallSet`.
> > > > Use llvm::find
> > > That is linear time. Why it is better than `SmallSet`?
> > Because there are only 2 elements, no?
> It could have more. If user writes it like `atomic compare compare capture 
> capture capture`, then it will have 5 elements. `SmallSet::insert` returns 
> a `pair` and we know if it is already in the set, which I have not added it 
> yet but I'll. `SmallVector` then is not that easy to use. Of course you could 
> say do `llvm::find` beforehand but it's not neat.
Is this correct at all?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116261

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

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11320
+  bool MutexClauseEncountered = false;
+  llvm::SmallSet EncounteredAtomicKinds;
   for (const OMPClause *C : Clauses) {

ABataev wrote:
> tianshilei1992 wrote:
> > ABataev wrote:
> > > tianshilei1992 wrote:
> > > > ABataev wrote:
> > > > > SmallVector with 2 elements should be enough, no need to use SmallSet 
> > > > > here.
> > > > `SmallVector` doesn't provide `contains`, which we use to check if 
> > > > `OMPC_compare` and `OMPC_capture` exist at the same time. Using 
> > > > `SmallVector` requires us to have a complicated comparison, for example:
> > > > ```
> > > > (EncounteredAtomicKinds[0] == OMPC_compare && EncounteredAtomicKinds[1] 
> > > > == OMPC_capture) || (EncounteredAtomicKinds[1] == OMPC_compare && 
> > > > EncounteredAtomicKinds[0] == OMPC_capture)
> > > > ```
> > > > which is not as neat as `SmallSet`.
> > > Use llvm::find
> > That is linear time. Why it is better than `SmallSet`?
> Because there are only 2 elements, no?
It could have more. If user writes it like `atomic compare compare capture 
capture capture`, then it will have 5 elements. `SmallSet::insert` returns a 
`pair` and we know if it is already in the set, which I have not added it yet 
but I'll. `SmallVector` then is not that easy to use. Of course you could say 
do `llvm::find` beforehand but it's not neat.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116261

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

[PATCH] D119613: [OpenMP] Add support for CPU offloading in new driver

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 408218.
jhuber6 added a comment.

Fixing LTO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119613

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -646,6 +646,83 @@
 }
 } // namespace amdgcn
 
+namespace generic {
+
+const char *getLDMOption(const llvm::Triple ) {
+  switch (T.getArch()) {
+  case llvm::Triple::x86:
+if (T.isOSIAMCU())
+  return "elf_iamcu";
+return "elf_i386";
+  case llvm::Triple::aarch64:
+return "aarch64linux";
+  case llvm::Triple::aarch64_be:
+return "aarch64linuxb";
+  case llvm::Triple::ppc64:
+return "elf64ppc";
+  case llvm::Triple::ppc64le:
+return "elf64lppc";
+  case llvm::Triple::x86_64:
+if (T.isX32())
+  return "elf32_x86_64";
+return "elf_x86_64";
+  case llvm::Triple::ve:
+return "elf64ve";
+  default:
+return nullptr;
+  }
+}
+
+Expected link(ArrayRef InputFiles, Triple TheTriple,
+   StringRef Arch) {
+  // Create a new file to write the linked device image to.
+  SmallString<128> TempFile;
+  if (Error Err = createOutputFile(sys::path::filename(ExecutableName) + "-" +
+   TheTriple.getArchName() + "-" + Arch,
+   "out", TempFile))
+return std::move(Err);
+
+  // Use the host linker to perform generic offloading. Use the same libraries
+  // and paths as the host application does.
+  SmallVector CmdArgs;
+  CmdArgs.push_back(LinkerUserPath);
+  CmdArgs.push_back("-m");
+  CmdArgs.push_back(getLDMOption(TheTriple));
+  CmdArgs.push_back("-shared");
+  for (auto AI = HostLinkerArgs.begin(), AE = HostLinkerArgs.end(); AI != AE;
+   ++AI) {
+StringRef Arg = *AI;
+if (Arg.startswith("-L"))
+  CmdArgs.push_back(Arg);
+else if (Arg.startswith("-l"))
+  CmdArgs.push_back(Arg);
+else if (Arg.startswith("--as-needed"))
+  CmdArgs.push_back(Arg);
+else if (Arg.startswith("--no-as-needed"))
+  CmdArgs.push_back(Arg);
+else if (Arg.startswith("-rpath")) {
+  CmdArgs.push_back(Arg);
+  CmdArgs.push_back(*(AI + 1));
+} else if (Arg.startswith("-dynamic-linker")) {
+  CmdArgs.push_back(Arg);
+  CmdArgs.push_back(*(AI + 1));
+}
+  }
+  CmdArgs.push_back("-Bsymbolic");
+  CmdArgs.push_back("-o");
+  CmdArgs.push_back(TempFile);
+
+  // Add extracted input files.
+  for (StringRef Input : InputFiles)
+CmdArgs.push_back(Input);
+
+  if (sys::ExecuteAndWait(LinkerUserPath, CmdArgs))
+return createStringError(inconvertibleErrorCode(), "'linker' failed");
+
+  return static_cast(TempFile);
+}
+} // namespace generic
+
 Expected linkDevice(ArrayRef InputFiles,
  Triple TheTriple, StringRef Arch) {
   switch (TheTriple.getArch()) {
@@ -656,7 +733,11 @@
 return amdgcn::link(InputFiles, TheTriple, Arch);
   case Triple::x86:
   case Triple::x86_64:
-// TODO: x86 linking support.
+  case Triple::aarch64:
+  case Triple::aarch64_be:
+  case Triple::ppc64:
+  case Triple::ppc64le:
+return generic::link(InputFiles, TheTriple, Arch);
   default:
 return createStringError(inconvertibleErrorCode(),
  TheTriple.getArchName() +
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -8200,14 +8200,19 @@
   ArgStringList FeatureArgs;
   TC->addClangTargetOptions(TCArgs, FeatureArgs, Action::OFK_OpenMP);
   auto FeatureIt = llvm::find(FeatureArgs, "-target-feature");
-  CmdArgs.push_back(Args.MakeArgString(
-  "-target-feature=" + TC->getTripleString() + "=" + *(FeatureIt + 1)));
+  if (FeatureIt != FeatureArgs.end())
+CmdArgs.push_back(
+Args.MakeArgString("-target-feature=" + TC->getTripleString() +
+   "=" + *(FeatureIt + 1)));
 }
 
 // Pass in the bitcode library to be linked during LTO.
 for (auto  :
  llvm::make_range(OpenMPTCRange.first, OpenMPTCRange.second)) {
   const ToolChain *TC = I.second;
+  if (!(TC->getTriple().isNVPTX() || TC->getTriple().isAMDGPU()))
+continue;
+
   const Driver  = TC->getDriver();
   const ArgList  = C.getArgsForToolChain(TC, "", Action::OFK_OpenMP);
   StringRef Arch = TCArgs.getLastArgValue(options::OPT_march_EQ);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119650: [clang-format] Handle PointerAlignment in `if` statements with initializers (C++17) the same way as in `for` loops.

[Owen Pan via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/TokenAnnotator.cpp:31
+/// with an initializer.
+static bool startsWithStatementWithInitializer(const AnnotatedLine ) {
+  return Line.startsWith(tok::kw_for) || Line.startsWith(tok::kw_if);

Nit: rename the function to `...WithInit`, `...WithInitStmt`, or 
`...WithInitStatement`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119650

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

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11320
+  bool MutexClauseEncountered = false;
+  llvm::SmallSet EncounteredAtomicKinds;
   for (const OMPClause *C : Clauses) {

tianshilei1992 wrote:
> ABataev wrote:
> > tianshilei1992 wrote:
> > > ABataev wrote:
> > > > SmallVector with 2 elements should be enough, no need to use SmallSet 
> > > > here.
> > > `SmallVector` doesn't provide `contains`, which we use to check if 
> > > `OMPC_compare` and `OMPC_capture` exist at the same time. Using 
> > > `SmallVector` requires us to have a complicated comparison, for example:
> > > ```
> > > (EncounteredAtomicKinds[0] == OMPC_compare && EncounteredAtomicKinds[1] 
> > > == OMPC_capture) || (EncounteredAtomicKinds[1] == OMPC_compare && 
> > > EncounteredAtomicKinds[0] == OMPC_capture)
> > > ```
> > > which is not as neat as `SmallSet`.
> > Use llvm::find
> That is linear time. Why it is better than `SmallSet`?
Because there are only 2 elements, no?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116261

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

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11320
+  bool MutexClauseEncountered = false;
+  llvm::SmallSet EncounteredAtomicKinds;
   for (const OMPClause *C : Clauses) {

ABataev wrote:
> tianshilei1992 wrote:
> > ABataev wrote:
> > > SmallVector with 2 elements should be enough, no need to use SmallSet 
> > > here.
> > `SmallVector` doesn't provide `contains`, which we use to check if 
> > `OMPC_compare` and `OMPC_capture` exist at the same time. Using 
> > `SmallVector` requires us to have a complicated comparison, for example:
> > ```
> > (EncounteredAtomicKinds[0] == OMPC_compare && EncounteredAtomicKinds[1] == 
> > OMPC_capture) || (EncounteredAtomicKinds[1] == OMPC_compare && 
> > EncounteredAtomicKinds[0] == OMPC_capture)
> > ```
> > which is not as neat as `SmallSet`.
> Use llvm::find
That is linear time. Why it is better than `SmallSet`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116261

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

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11320
+  bool MutexClauseEncountered = false;
+  llvm::SmallSet EncounteredAtomicKinds;
   for (const OMPClause *C : Clauses) {

tianshilei1992 wrote:
> ABataev wrote:
> > SmallVector with 2 elements should be enough, no need to use SmallSet here.
> `SmallVector` doesn't provide `contains`, which we use to check if 
> `OMPC_compare` and `OMPC_capture` exist at the same time. Using `SmallVector` 
> requires us to have a complicated comparison, for example:
> ```
> (EncounteredAtomicKinds[0] == OMPC_compare && EncounteredAtomicKinds[1] == 
> OMPC_capture) || (EncounteredAtomicKinds[1] == OMPC_compare && 
> EncounteredAtomicKinds[0] == OMPC_capture)
> ```
> which is not as neat as `SmallSet`.
Use llvm::find


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116261

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

[PATCH] D119599: Add option to align compound assignments like `+=`

[sstwcw via Phabricator via cfe-commits]

sstwcw added inline comments.



Comment at: clang/include/clang/Format/Format.h:157
+  ///   a   &= 2;
+  ///   bbb  = 2;
+  ///

HazardyKnusperkeks wrote:
> sstwcw wrote:
> > HazardyKnusperkeks wrote:
> > > curdeius wrote:
> > > > I guess it would be complicated to avoid adding an additional space 
> > > > here. I mean, it could be:
> > > > ```
> > > > a  &= 2;
> > > > bbb = 2;
> > > > ```
> > > > And with 3-char operators, there's one more space.
> > > That would be awesome, but it should be an option to turn off or on.
> > > But I think this would really be complicated.
> > I can do it either way. But I thought without the extra space the formatted 
> > code looked ugly, especially when mixing `>>=` and `=`.  Which way do you 
> > prefer?
> > 
> > 
> > ```
> > a >>= 2;
> > bbb = 2;
> > ```
> I would prefer an option, to be able to do both.
> I think I would use the variant which is more compact, but can't say for sure 
> until I really have the option and tried it.
You mean like a new entry under `AlignConsecutiveStyle` with the options to 
align the left ends of the operators and to align the equal sign with and 
without padding them to the same length? I don't want the new option to be 
merged with `AlignCompound`, because we are working on adding support for a 
language that uses both `=` and `<=` for regular assignments, and maybe someone 
will want to support a language that has both `=` and `:=` in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D119649: [clang-format] Correctly format loops and `if` statements even if preceded with comments.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:379
 if (NextLine.First->is(tok::l_brace)) {
   if ((TheLine->First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,
tok::kw_for, tok::kw_switch, tok::kw_try,

sstwcw wrote:
> Why not use `FirstNonComment` instead of `TheLine->First` down here?
Because I haven't added a test case for it and prefer doing it in a different 
revision (and because as you see, I already got confused enough :) ).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119649

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

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 marked an inline comment as done.
tianshilei1992 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11320
+  bool MutexClauseEncountered = false;
+  llvm::SmallSet EncounteredAtomicKinds;
   for (const OMPClause *C : Clauses) {

ABataev wrote:
> SmallVector with 2 elements should be enough, no need to use SmallSet here.
`SmallVector` doesn't provide `contains`, which we use to check if 
`OMPC_compare` and `OMPC_capture` exist at the same time. Using `SmallVector` 
requires us to have a complicated comparison, for example:
```
(EncounteredAtomicKinds[0] == OMPC_compare && EncounteredAtomicKinds[1] == 
OMPC_capture) || (EncounteredAtomicKinds[1] == OMPC_compare && 
EncounteredAtomicKinds[0] == OMPC_capture)
```
which is not as neat as `SmallSet`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116261

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

[PATCH] D119648: [clang-format] Fix PointerAlignment within lambdas in a multi-variable declaration statement.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius updated this revision to Diff 408217.
curdeius added a comment.
This revision is now accepted and ready to land.

Split from and rebase on top of D119650 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119648

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2031,6 +2031,10 @@
   verifyFormat("for (int x = 0; int  : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto  : {1, 2, 3})", Style);
   verifyFormat("for (f(); int  : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int ) { return 0; },\n"
+  " res2 = [](int ) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int *c;\n"
@@ -2068,6 +2072,10 @@
   verifyFormat("for (int x = 0; int& c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto& c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int& c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int& a) { return 0; },\n"
+  "res2 = [](int& a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int* c;\n"
@@ -2121,6 +2129,10 @@
   verifyFormat("for (int x = 0; int & c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto & c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int & c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int & a) { return 0; },\n"
+  " res2 = [](int & a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int*  c;\n"
@@ -8400,6 +8412,9 @@
   verifyFormat("if (int *p, *q; p != q) {\n  p = p->next;\n}", Style);
   verifyFormat("/*comment*/ if (int *p, *q; p != q) {\n  p = p->next;\n}",
Style);
+  verifyFormat("if (int *p, *q; p != q) {\n  p = p->next;\n}", Style);
+  verifyFormat("/*comment*/ if (int *p, *q; p != q) {\n  p = p->next;\n}",
+   Style);
 }
 
 TEST_F(FormatTest, ConditionalExpressionsInBrackets) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3135,7 +3135,15 @@
   return false;
 if (getTokenPointerOrReferenceAlignment(Left) == FormatStyle::PAS_Right)
   return false;
-if (Line.IsMultiVariableDeclStmt)
+// FIXME: Setting IsMultiVariableDeclStmt for the whole line is 
error-prone,
+// because it does not take into account nested scopes like lambdas.
+// In multi-variable declaration statements, attach */& to the variable
+// independently of the style. However, avoid doing it if we are in a 
nested
+// scope, e.g. lambda. We still need to special-case statements with
+// initializers.
+if (Line.IsMultiVariableDeclStmt &&
+(Left.NestingLevel == Line.First->NestingLevel ||
+ startsWithStatementWithInitializer(Line)))
   return false;
 return Left.Previous && !Left.Previous->isOneOf(
 tok::l_paren, tok::coloncolon, tok::l_square);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2031,6 +2031,10 @@
   verifyFormat("for (int x = 0; int  : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto  : {1, 2, 3})", Style);
   verifyFormat("for (f(); int  : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int ) { return 0; },\n"
+  " res2 = [](int ) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int *c;\n"
@@ -2068,6 +2072,10 @@
   verifyFormat("for (int x = 0; int& c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto& c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int& c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int& a) { return 0; },\n"
+  "res2 = [](int& a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int* c;\n"
@@ -2121,6 +2129,10 @@
   verifyFormat("for (int x = 0; int & c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto & c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int & c : {1, 2, 3})", Style);
+  verifyFormat(
+   

[PATCH] D118632: [Clang]OpenMP] Add the codegen support for `atomic compare`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118632

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

[PATCH] D119650: [clang-format] Handle PointerAlignment in `if` statements with initializers (C++17) the same way as in `for` loops.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius created this revision.
curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan.
curdeius requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Depends on D119649 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119650

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8396,6 +8396,10 @@
Style);
   verifyFormat("vector a, b;", Style);
   verifyFormat("for (int *p, *q; p != q; p = p->next) {\n}", Style);
+  verifyFormat("/*comment*/ for (int *p, *q; p != q; p = p->next) {\n}", 
Style);
+  verifyFormat("if (int *p, *q; p != q) {\n  p = p->next;\n}", Style);
+  verifyFormat("/*comment*/ if (int *p, *q; p != q) {\n  p = p->next;\n}",
+   Style);
 }
 
 TEST_F(FormatTest, ConditionalExpressionsInBrackets) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -26,6 +26,12 @@
 
 namespace {
 
+/// Returns \c true if the line starts with a token that can start a statement
+/// with an initializer.
+static bool startsWithStatementWithInitializer(const AnnotatedLine ) {
+  return Line.startsWith(tok::kw_for) || Line.startsWith(tok::kw_if);
+}
+
 /// Returns \c true if the token can be used as an identifier in
 /// an Objective-C \c \@selector, \c false otherwise.
 ///
@@ -1135,7 +1141,8 @@
   else if (Contexts.back().InInheritanceList)
 Tok->setType(TT_InheritanceComma);
   else if (Contexts.back().FirstStartOfName &&
-   (Contexts.size() == 1 || Line.startsWith(tok::kw_for))) {
+   (Contexts.size() == 1 ||
+startsWithStatementWithInitializer(Line))) {
 Contexts.back().FirstStartOfName->PartOfMultiVariableDeclStmt = true;
 Line.IsMultiVariableDeclStmt = true;
   }
@@ -3090,7 +3097,8 @@
 FormatStyle::PAS_Left ||
 (Line.IsMultiVariableDeclStmt &&
  (Left.NestingLevel == 0 ||
-  (Left.NestingLevel == 1 && Line.First->is(tok::kw_for);
+  (Left.NestingLevel == 1 &&
+   startsWithStatementWithInitializer(Line);
   }
   if (Right.is(TT_FunctionTypeLParen) && Left.isNot(tok::l_paren) &&
   (!Left.is(TT_PointerOrReference) ||


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8396,6 +8396,10 @@
Style);
   verifyFormat("vector a, b;", Style);
   verifyFormat("for (int *p, *q; p != q; p = p->next) {\n}", Style);
+  verifyFormat("/*comment*/ for (int *p, *q; p != q; p = p->next) {\n}", Style);
+  verifyFormat("if (int *p, *q; p != q) {\n  p = p->next;\n}", Style);
+  verifyFormat("/*comment*/ if (int *p, *q; p != q) {\n  p = p->next;\n}",
+   Style);
 }
 
 TEST_F(FormatTest, ConditionalExpressionsInBrackets) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -26,6 +26,12 @@
 
 namespace {
 
+/// Returns \c true if the line starts with a token that can start a statement
+/// with an initializer.
+static bool startsWithStatementWithInitializer(const AnnotatedLine ) {
+  return Line.startsWith(tok::kw_for) || Line.startsWith(tok::kw_if);
+}
+
 /// Returns \c true if the token can be used as an identifier in
 /// an Objective-C \c \@selector, \c false otherwise.
 ///
@@ -1135,7 +1141,8 @@
   else if (Contexts.back().InInheritanceList)
 Tok->setType(TT_InheritanceComma);
   else if (Contexts.back().FirstStartOfName &&
-   (Contexts.size() == 1 || Line.startsWith(tok::kw_for))) {
+   (Contexts.size() == 1 ||
+startsWithStatementWithInitializer(Line))) {
 Contexts.back().FirstStartOfName->PartOfMultiVariableDeclStmt = true;
 Line.IsMultiVariableDeclStmt = true;
   }
@@ -3090,7 +3097,8 @@
 FormatStyle::PAS_Left ||
 (Line.IsMultiVariableDeclStmt &&
  (Left.NestingLevel == 0 ||
-  (Left.NestingLevel == 1 && Line.First->is(tok::kw_for);
+  (Left.NestingLevel == 1 &&
+   startsWithStatementWithInitializer(Line);
   }
   if (Right.is(TT_FunctionTypeLParen) && Left.isNot(tok::l_paren) &&
   (!Left.is(TT_PointerOrReference) ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D119392: [Clang][OpenMP][Sema] Remove support for floating point values in atomic compare

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119392

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

[PATCH] D119649: [clang-format] Correctly format loops and `if` statements even if preceded with comments.

[sstwcw via Phabricator via cfe-commits]

sstwcw added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:379
 if (NextLine.First->is(tok::l_brace)) {
   if ((TheLine->First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,
tok::kw_for, tok::kw_switch, tok::kw_try,

Why not use `FirstNonComment` instead of `TheLine->First` down here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119649

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

[PATCH] D119648: [clang-format] Fix PointerAlignment within lambdas in a multi-variable declaration statement.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius planned changes to this revision.
curdeius added a comment.

In D119648#3317212 , @owenpan wrote:

> Does any C++ standard support init statements in `while` loops?

Argh, I initially thought about `if` statements and then somehow my brain 
turned it into `while`.
You're right, there are only `if` and `for` statements with initializers (C++20 
added initializers for ranged-base for loops).

Let me split this revision (fixing `if` and not `while`) and clear things up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119648

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

[PATCH] D119648: [clang-format] Fix PointerAlignment within lambdas in a multi-variable declaration statement.

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Does any C++ standard support init statements in `while` loops?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119648

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

[PATCH] D119649: [clang-format] Correctly format loops even if preceded with comments.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius updated this revision to Diff 408215.
curdeius added a comment.

Test ifs and FOREACH loops.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119649

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1520,6 +1520,36 @@
 
 TEST_F(FormatTest, FormatShortBracedStatements) {
   FormatStyle AllowSimpleBracedStatements = getLLVMStyle();
+  EXPECT_EQ(AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine, false);
+  EXPECT_EQ(AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine,
+FormatStyle::SIS_Never);
+  EXPECT_EQ(AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine, false);
+  EXPECT_EQ(AllowSimpleBracedStatements.BraceWrapping.AfterFunction, false);
+  verifyFormat("for (;;) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("/*comment*/ for (;;) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("BOOST_FOREACH (int v, vec) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("/*comment*/ BOOST_FOREACH (int v, vec) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("while (true) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("/*comment*/ while (true) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("if (true) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("/*comment*/ if (true) {\n"
+   "  f();\n"
+   "}");
+
   AllowSimpleBracedStatements.IfMacros.push_back("MYIF");
   // Where line-lengths matter, a 2-letter synonym that maintains line length.
   // Not IF to avoid any confusion that IF is somehow special.
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -319,6 +319,15 @@
 
 bool MergeShortFunctions = ShouldMergeShortFunctions();
 
+const FormatToken *FirstNonComment = TheLine->First;
+if (FirstNonComment->is(tok::comment)) {
+  FirstNonComment = FirstNonComment->getNextNonComment();
+  if (!FirstNonComment)
+return 0;
+}
+// FIXME: There are probably cases where we should use FirstNonComment
+// instead of TheLine->First.
+
 if (Style.CompactNamespaces) {
   if (auto nsToken = TheLine->First->getNamespaceToken()) {
 int i = 0;
@@ -358,9 +367,9 @@
 if (TheLine->Last->is(TT_FunctionLBrace) && TheLine->First != 
TheLine->Last)
   return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
 // Try to merge a control statement block with left brace unwrapped.
-if (TheLine->Last->is(tok::l_brace) && TheLine->First != TheLine->Last &&
-TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
-TT_ForEachMacro)) {
+if (TheLine->Last->is(tok::l_brace) && FirstNonComment != TheLine->Last &&
+FirstNonComment->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
+ TT_ForEachMacro)) {
   return Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never
  ? tryMergeSimpleBlock(I, E, Limit)
  : 0;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1520,6 +1520,36 @@
 
 TEST_F(FormatTest, FormatShortBracedStatements) {
   FormatStyle AllowSimpleBracedStatements = getLLVMStyle();
+  EXPECT_EQ(AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine, false);
+  EXPECT_EQ(AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine,
+FormatStyle::SIS_Never);
+  EXPECT_EQ(AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine, false);
+  EXPECT_EQ(AllowSimpleBracedStatements.BraceWrapping.AfterFunction, false);
+  verifyFormat("for (;;) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("/*comment*/ for (;;) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("BOOST_FOREACH (int v, vec) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("/*comment*/ BOOST_FOREACH (int v, vec) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("while (true) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("/*comment*/ while (true) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("if (true) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("/*comment*/ if (true) {\n"
+   "  f();\n"
+   "}");
+
   

[PATCH] D119621: [SanitizerCoverage] Add instrumentation callbacks for FP cmp instructions

[Chaofan Shou via Phabricator via cfe-commits]

shouc updated this revision to Diff 408214.
shouc added a comment.

Add support for dfsan


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

https://reviews.llvm.org/D119621

Files:
  clang/docs/SanitizerCoverage.rst
  compiler-rt/lib/dfsan/dfsan_custom.cpp
  compiler-rt/lib/dfsan/done_abilist.txt
  compiler-rt/lib/fuzzer/dataflow/DataFlowCallbacks.cpp
  compiler-rt/lib/sanitizer_common/sanitizer_coverage_fuchsia.cpp
  compiler-rt/lib/sanitizer_common/sanitizer_coverage_interface.inc
  compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cpp
  compiler-rt/lib/sanitizer_common/sanitizer_interface_internal.h
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/test/Instrumentation/SanitizerCoverage/cmp-tracing-api-x86_32.ll

Index: llvm/test/Instrumentation/SanitizerCoverage/cmp-tracing-api-x86_32.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/cmp-tracing-api-x86_32.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/cmp-tracing-api-x86_32.ll
@@ -12,10 +12,16 @@
 ; CHECK-DAG: declare void @__sanitizer_cov_trace_cmp2(i16 zeroext, i16 zeroext)
 ; CHECK-DAG: declare void @__sanitizer_cov_trace_cmp4(i32 zeroext, i32 zeroext)
 ; CHECK-DAG: declare void @__sanitizer_cov_trace_cmp8(i64, i64)
+; CHECK-DAG: declare void @__sanitizer_cov_trace_cmp_fp2(half, half)
+; CHECK-DAG: declare void @__sanitizer_cov_trace_cmp_fp4(float, float)
+; CHECK-DAG: declare void @__sanitizer_cov_trace_cmp_fp8(double, double)
 ; CHECK-DAG: declare void @__sanitizer_cov_trace_const_cmp1(i8 zeroext, i8 zeroext)
 ; CHECK-DAG: declare void @__sanitizer_cov_trace_const_cmp2(i16 zeroext, i16 zeroext)
 ; CHECK-DAG: declare void @__sanitizer_cov_trace_const_cmp4(i32 zeroext, i32 zeroext)
 ; CHECK-DAG: declare void @__sanitizer_cov_trace_const_cmp8(i64, i64)
+; CHECK-DAG: declare void @__sanitizer_cov_trace_const_cmp_fp2(half, half)
+; CHECK-DAG: declare void @__sanitizer_cov_trace_const_cmp_fp4(float, float)
+; CHECK-DAG: declare void @__sanitizer_cov_trace_const_cmp_fp8(double, double)
 ; CHECK-DAG: declare void @__sanitizer_cov_trace_div4(i32 zeroext)
 ; CHECK-DAG: declare void @__sanitizer_cov_trace_div8(i64)
 ; CHECK-DAG: declare void @__sanitizer_cov_trace_gep(i64)
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -52,10 +52,16 @@
 const char SanCovTraceCmp2[] = "__sanitizer_cov_trace_cmp2";
 const char SanCovTraceCmp4[] = "__sanitizer_cov_trace_cmp4";
 const char SanCovTraceCmp8[] = "__sanitizer_cov_trace_cmp8";
+const char SanCovTraceCmpF2[] = "__sanitizer_cov_trace_cmp_fp2";
+const char SanCovTraceCmpF4[] = "__sanitizer_cov_trace_cmp_fp4";
+const char SanCovTraceCmpF8[] = "__sanitizer_cov_trace_cmp_fp8";
 const char SanCovTraceConstCmp1[] = "__sanitizer_cov_trace_const_cmp1";
 const char SanCovTraceConstCmp2[] = "__sanitizer_cov_trace_const_cmp2";
 const char SanCovTraceConstCmp4[] = "__sanitizer_cov_trace_const_cmp4";
 const char SanCovTraceConstCmp8[] = "__sanitizer_cov_trace_const_cmp8";
+const char SanCovTraceConstCmpF2[] = "__sanitizer_cov_trace_const_cmp_fp2";
+const char SanCovTraceConstCmpF4[] = "__sanitizer_cov_trace_const_cmp_fp4";
+const char SanCovTraceConstCmpF8[] = "__sanitizer_cov_trace_const_cmp_fp8";
 const char SanCovLoad1[] = "__sanitizer_cov_load1";
 const char SanCovLoad2[] = "__sanitizer_cov_load2";
 const char SanCovLoad4[] = "__sanitizer_cov_load4";
@@ -257,8 +263,8 @@
   std::string getSectionEnd(const std::string ) const;
   FunctionCallee SanCovTracePCIndir;
   FunctionCallee SanCovTracePC, SanCovTracePCGuard;
-  std::array SanCovTraceCmpFunction;
-  std::array SanCovTraceConstCmpFunction;
+  std::array SanCovTraceCmpFunction;
+  std::array SanCovTraceConstCmpFunction;
   std::array SanCovLoadFunction;
   std::array SanCovStoreFunction;
   std::array SanCovTraceDivFunction;
@@ -267,7 +273,8 @@
   GlobalVariable *SanCovLowestStack;
   Type *Int128PtrTy, *IntptrTy, *IntptrPtrTy, *Int64Ty, *Int64PtrTy, *Int32Ty,
   *Int32PtrTy, *Int16PtrTy, *Int16Ty, *Int8Ty, *Int8PtrTy, *Int1Ty,
-  *Int1PtrTy;
+  *Int1PtrTy,
+  *Fp16Ty, *Fp16PtrTy, *Fp32Ty, *Fp32PtrTy, *Fp64Ty, *Fp64PtrTy;
   Module *CurModule;
   std::string CurModuleUniqueId;
   Triple TargetTriple;
@@ -443,11 +450,17 @@
   Int32PtrTy = PointerType::getUnqual(IRB.getInt32Ty());
   Int8PtrTy = PointerType::getUnqual(IRB.getInt8Ty());
   Int1PtrTy = PointerType::getUnqual(IRB.getInt1Ty());
+  Fp16PtrTy = PointerType::getUnqual(IRB.getHalfTy());
+  Fp32PtrTy = PointerType::getUnqual(IRB.getFloatTy());
+  Fp64PtrTy = PointerType::getUnqual(IRB.getDoubleTy());
   Int64Ty = IRB.getInt64Ty();
   Int32Ty = IRB.getInt32Ty();
   Int16Ty = IRB.getInt16Ty();
   Int8Ty = IRB.getInt8Ty();
   Int1Ty = IRB.getInt1Ty();
+  Fp16Ty = 

[PATCH] D119648: [clang-format] Fix PointerAlignment within lambdas in a multi-variable declaration statement.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

In D119648#3317188 , 
@HazardyKnusperkeks wrote:

> What about if?

True, I need to fix them in D119649  too.
And I think that I'll split the fix for while loops from this revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119648

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

[PATCH] D119648: [clang-format] Fix PointerAlignment within lambdas in a multi-variable declaration statement.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius updated this revision to Diff 408213.
curdeius added a comment.

Rebase on top of D119649 , add a similar case 
with a leading comment.

Without it, there would be a different spacing of multi-variable initializers 
in loops, e.g. (-expected, +incorrect):

  -/*comment*/ for (int *p, *q; p != q; p = p->next) {
  +/*comment*/ for (int* p, *q; p != q; p = p->next) {


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119648

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2017,6 +2017,10 @@
   verifyFormat("for (int x = 0; int  : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto  : {1, 2, 3})", Style);
   verifyFormat("for (f(); int  : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int ) { return 0; },\n"
+  " res2 = [](int ) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int *c;\n"
@@ -2054,6 +2058,10 @@
   verifyFormat("for (int x = 0; int& c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto& c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int& c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int& a) { return 0; },\n"
+  "res2 = [](int& a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int* c;\n"
@@ -2107,6 +2115,10 @@
   verifyFormat("for (int x = 0; int & c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto & c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int & c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int & a) { return 0; },\n"
+  " res2 = [](int & a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int*  c;\n"
@@ -8382,6 +8394,10 @@
Style);
   verifyFormat("vector a, b;", Style);
   verifyFormat("for (int *p, *q; p != q; p = p->next) {\n}", Style);
+  verifyFormat("/*comment*/ for (int *p, *q; p != q; p = p->next) {\n}", Style);
+  verifyFormat("while (int *p, *q; p != q) {\n  p = p->next;\n}", Style);
+  verifyFormat("/*comment*/ while (int *p, *q; p != q) {\n  p = p->next;\n}",
+   Style);
 }
 
 TEST_F(FormatTest, ConditionalExpressionsInBrackets) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -26,6 +26,12 @@
 
 namespace {
 
+/// Returns \c true if the line starts with a token that can start a loop with
+/// an initializer.
+static bool startsWithLoopWithInitializer(const AnnotatedLine ) {
+  return Line.startsWith(tok::kw_for) || Line.startsWith(tok::kw_while);
+}
+
 /// Returns \c true if the token can be used as an identifier in
 /// an Objective-C \c \@selector, \c false otherwise.
 ///
@@ -1135,7 +1141,7 @@
   else if (Contexts.back().InInheritanceList)
 Tok->setType(TT_InheritanceComma);
   else if (Contexts.back().FirstStartOfName &&
-   (Contexts.size() == 1 || Line.startsWith(tok::kw_for))) {
+   (Contexts.size() == 1 || startsWithLoopWithInitializer(Line))) {
 Contexts.back().FirstStartOfName->PartOfMultiVariableDeclStmt = true;
 Line.IsMultiVariableDeclStmt = true;
   }
@@ -3090,7 +3096,8 @@
 FormatStyle::PAS_Left ||
 (Line.IsMultiVariableDeclStmt &&
  (Left.NestingLevel == 0 ||
-  (Left.NestingLevel == 1 && Line.First->is(tok::kw_for);
+  (Left.NestingLevel == 1 &&
+   startsWithLoopWithInitializer(Line);
   }
   if (Right.is(TT_FunctionTypeLParen) && Left.isNot(tok::l_paren) &&
   (!Left.is(TT_PointerOrReference) ||
@@ -3127,7 +3134,15 @@
   return false;
 if (getTokenPointerOrReferenceAlignment(Left) == FormatStyle::PAS_Right)
   return false;
-if (Line.IsMultiVariableDeclStmt)
+// FIXME: Setting IsMultiVariableDeclStmt for the whole line is error-prone,
+// because it does not take into account nested scopes like lambdas.
+// In multi-variable declaration statements, attach */& to the variable
+// independently of the style. However, avoid doing it if we are in a nested
+// scope, e.g. lambda. We still need to special-case loops with
+// initializers.
+if (Line.IsMultiVariableDeclStmt &&
+(Left.NestingLevel == Line.First->NestingLevel ||
+ 

[PATCH] D119648: [clang-format] Fix PointerAlignment within lambdas in a multi-variable declaration statement.

[Björn Schäpers via Phabricator via cfe-commits]

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

What about if?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119648

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

[PATCH] D119649: [clang-format] Correctly format loops even if preceded with comments.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius created this revision.
curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan.
curdeius requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://github.com/llvm/llvm-project/issues/53758.

Braces in loops with leading (block) comments were formatted according to 
BraceWrapping.AfterFunction and not 
AllowShortBlocksOnASingleLine/AllowShortLoopsOnASingleLine.

Previously, the code:

  while (true) {
f();
  }
  /*comment*/ while (true) {
f();
  }

was incorrectly formatted to:

  while (true) {
f();
  }
  /*comment*/ while (true) { f(); }

when using config:

  BasedOnStyle: LLVM
  BreakBeforeBraces: Custom
  BraceWrapping:
AfterFunction: false
  AllowShortBlocksOnASingleLine: false
  AllowShortLoopsOnASingleLine: false

and it was (correctly but by chance) formatted to:

  while (true) {
f();
  }
  /*comment*/ while (true) {
f();
  }

when using enabling brace wrapping after functions:

  BasedOnStyle: LLVM
  BreakBeforeBraces: Custom
  BraceWrapping:
AfterFunction: true
  AllowShortBlocksOnASingleLine: false
  AllowShortLoopsOnASingleLine: false


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119649

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1520,6 +1520,22 @@
 
 TEST_F(FormatTest, FormatShortBracedStatements) {
   FormatStyle AllowSimpleBracedStatements = getLLVMStyle();
+  EXPECT_EQ(AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine, false);
+  EXPECT_EQ(AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine, false);
+  EXPECT_EQ(AllowSimpleBracedStatements.BraceWrapping.AfterFunction, false);
+  verifyFormat("for (;;) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("/*comment*/ for (;;) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("while (true) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("/*comment*/ while (true) {\n"
+   "  f();\n"
+   "}");
+
   AllowSimpleBracedStatements.IfMacros.push_back("MYIF");
   // Where line-lengths matter, a 2-letter synonym that maintains line length.
   // Not IF to avoid any confusion that IF is somehow special.
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -319,6 +319,15 @@
 
 bool MergeShortFunctions = ShouldMergeShortFunctions();
 
+const FormatToken *FirstNonComment = TheLine->First;
+if (FirstNonComment->is(tok::comment)) {
+  FirstNonComment = FirstNonComment->getNextNonComment();
+  if (!FirstNonComment)
+return 0;
+}
+// FIXME: There are probably cases where we should use FirstNonComment
+// instead of TheLine->First.
+
 if (Style.CompactNamespaces) {
   if (auto nsToken = TheLine->First->getNamespaceToken()) {
 int i = 0;
@@ -358,9 +367,9 @@
 if (TheLine->Last->is(TT_FunctionLBrace) && TheLine->First != 
TheLine->Last)
   return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
 // Try to merge a control statement block with left brace unwrapped.
-if (TheLine->Last->is(tok::l_brace) && TheLine->First != TheLine->Last &&
-TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
-TT_ForEachMacro)) {
+if (TheLine->Last->is(tok::l_brace) && FirstNonComment != TheLine->Last &&
+FirstNonComment->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
+ TT_ForEachMacro)) {
   return Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never
  ? tryMergeSimpleBlock(I, E, Limit)
  : 0;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1520,6 +1520,22 @@
 
 TEST_F(FormatTest, FormatShortBracedStatements) {
   FormatStyle AllowSimpleBracedStatements = getLLVMStyle();
+  EXPECT_EQ(AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine, false);
+  EXPECT_EQ(AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine, false);
+  EXPECT_EQ(AllowSimpleBracedStatements.BraceWrapping.AfterFunction, false);
+  verifyFormat("for (;;) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("/*comment*/ for (;;) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("while (true) {\n"
+   "  f();\n"
+   "}");
+  verifyFormat("/*comment*/ while (true) {\n"
+   "  f();\n"
+   "}");
+
   

[clang] 7d5062c - [clang-format] Remove unnecessary parentheses in return statements. NFC.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-02-12T21:25:52+01:00
New Revision: 7d5062c6ac868537605dd0860b2714aba9ece90d

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

LOG: [clang-format] Remove unnecessary parentheses in return statements. NFC.

Added: 


Modified: 
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/FormatTokenLexer.cpp
clang/lib/Format/NamespaceEndCommentsFixer.cpp
clang/lib/Format/TokenAnnotator.cpp

Removed: 




diff  --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index 42c3d2e4326b9..23892be8e9e8e 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -331,7 +331,7 @@ bool ContinuationIndenter::mustBreak(const LineState 
) {
   if (Style.BraceWrapping.BeforeLambdaBody && Current.CanBreakBefore &&
   Current.is(TT_LambdaLBrace) && Previous.isNot(TT_LineComment)) {
 auto LambdaBodyLength = getLengthToMatchingParen(Current, State.Stack);
-return (LambdaBodyLength > getColumnLimit(State));
+return LambdaBodyLength > getColumnLimit(State);
   }
   if (Current.MustBreakBefore || Current.is(TT_InlineASMColon))
 return true;
@@ -1234,7 +1234,7 @@ static bool hasNestedBlockInlined(const FormatToken 
*Previous,
 return true;
 
   // Also a nested block if contains a lambda inside function with 1 parameter
-  return (Style.BraceWrapping.BeforeLambdaBody && 
Current.is(TT_LambdaLSquare));
+  return Style.BraceWrapping.BeforeLambdaBody && Current.is(TT_LambdaLSquare);
 }
 
 unsigned ContinuationIndenter::moveStateToNextToken(LineState ,

diff  --git a/clang/lib/Format/FormatTokenLexer.cpp 
b/clang/lib/Format/FormatTokenLexer.cpp
index 0ecae6af4c46b..1540c14686faa 100644
--- a/clang/lib/Format/FormatTokenLexer.cpp
+++ b/clang/lib/Format/FormatTokenLexer.cpp
@@ -500,7 +500,7 @@ bool FormatTokenLexer::canPrecedeRegexLiteral(FormatToken 
*Prev) {
   // `!` is an unary prefix operator, but also a post-fix operator that casts
   // away nullability, so the same check applies.
   if (Prev->isOneOf(tok::plusplus, tok::minusminus, tok::exclaim))
-return (Tokens.size() < 3 || precedesOperand(Tokens[Tokens.size() - 3]));
+return Tokens.size() < 3 || precedesOperand(Tokens[Tokens.size() - 3]);
 
   // The previous token must introduce an operand location where regex
   // literals can occur.

diff  --git a/clang/lib/Format/NamespaceEndCommentsFixer.cpp 
b/clang/lib/Format/NamespaceEndCommentsFixer.cpp
index 00d161890906f..65f965548da37 100644
--- a/clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ b/clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -136,7 +136,7 @@ bool validEndComment(const FormatToken *RBraceTok, 
StringRef NamespaceName,
 return false;
   NamespaceNameInComment = Groups.size() > 2 ? Groups[2] : "";
 
-  return (NamespaceNameInComment == NamespaceName);
+  return NamespaceNameInComment == NamespaceName;
 }
 
 void addEndComment(const FormatToken *RBraceTok, StringRef EndCommentText,

diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index dabecbf9c74ab..f1db5224fc55b 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1615,10 +1615,10 @@ class AnnotatingParser {
   PriorLeadingIdentifier->is(tok::kw_explicit))
 PriorLeadingIdentifier = PriorLeadingIdentifier->Previous;
 
-  return (PriorLeadingIdentifier &&
-  (PriorLeadingIdentifier->is(TT_TemplateCloser) ||
-   PriorLeadingIdentifier->ClosesRequiresClause) &&
-  LeadingIdentifier->TokenText == Current.Next->TokenText);
+  return PriorLeadingIdentifier &&
+ (PriorLeadingIdentifier->is(TT_TemplateCloser) ||
+  PriorLeadingIdentifier->ClosesRequiresClause) &&
+ LeadingIdentifier->TokenText == Current.Next->TokenText;
 }
   }
 }
@@ -1868,7 +1868,7 @@ class AnnotatingParser {
   return true;
 
 // const a = in JavaScript.
-return (Style.isJavaScript() && PreviousNotConst->is(tok::kw_const));
+return Style.isJavaScript() && PreviousNotConst->is(tok::kw_const);
   }
 
   /// Determine whether ')' is ending a cast.
@@ -3085,12 +3085,12 @@ bool TokenAnnotator::spaceRequiredBetween(const 
AnnotatedLine ,
 Right.Next->Next->is(TT_RangeBasedForLoopColon))
   return getTokenPointerOrReferenceAlignment(Right) !=
  FormatStyle::PAS_Left;
-return (
-(!Left.isOneOf(TT_PointerOrReference, tok::l_paren) &&
- (getTokenPointerOrReferenceAlignment(Right) != FormatStyle::PAS_Left 
||
-  (Line.IsMultiVariableDeclStmt &&
-   (Left.NestingLevel == 0 ||
-(Left.NestingLevel == 

[PATCH] D119648: [clang-format] Fix PointerAlignment within lambdas in a multi-variable declaration statement.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius created this revision.
curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan.
curdeius requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://github.com/llvm/llvm-project/issues/43115.

Also, handle while loops with initializers (C++20) the same way as for loops.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119648

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2001,6 +2001,10 @@
   verifyFormat("for (int x = 0; int  : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto  : {1, 2, 3})", Style);
   verifyFormat("for (f(); int  : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int ) { return 0; },\n"
+  " res2 = [](int ) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int *c;\n"
@@ -2038,6 +2042,10 @@
   verifyFormat("for (int x = 0; int& c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto& c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int& c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int& a) { return 0; },\n"
+  "res2 = [](int& a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int* c;\n"
@@ -2091,6 +2099,10 @@
   verifyFormat("for (int x = 0; int & c : {1, 2, 3})", Style);
   verifyFormat("for (f(); auto & c : {1, 2, 3})", Style);
   verifyFormat("for (f(); int & c : {1, 2, 3})", Style);
+  verifyFormat(
+  "function res1 = [](int & a) { return 0; },\n"
+  " res2 = [](int & a) { return 0; };",
+  Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int*  c;\n"
@@ -8366,6 +8378,7 @@
Style);
   verifyFormat("vector a, b;", Style);
   verifyFormat("for (int *p, *q; p != q; p = p->next) {\n}", Style);
+  verifyFormat("while (int *p, *q; p != q) {\n  p = p->next;\n}", Style);
 }
 
 TEST_F(FormatTest, ConditionalExpressionsInBrackets) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -26,6 +26,10 @@
 
 namespace {
 
+static bool startsWithLoopWithInitializer(const AnnotatedLine ) {
+  return Line.startsWith(tok::kw_for) || Line.startsWith(tok::kw_while);
+}
+
 /// Returns \c true if the token can be used as an identifier in
 /// an Objective-C \c \@selector, \c false otherwise.
 ///
@@ -1135,7 +1139,7 @@
   else if (Contexts.back().InInheritanceList)
 Tok->setType(TT_InheritanceComma);
   else if (Contexts.back().FirstStartOfName &&
-   (Contexts.size() == 1 || Line.startsWith(tok::kw_for))) {
+   (Contexts.size() == 1 || startsWithLoopWithInitializer(Line))) {
 Contexts.back().FirstStartOfName->PartOfMultiVariableDeclStmt = true;
 Line.IsMultiVariableDeclStmt = true;
   }
@@ -3085,12 +3089,13 @@
 Right.Next->Next->is(TT_RangeBasedForLoopColon))
   return getTokenPointerOrReferenceAlignment(Right) !=
  FormatStyle::PAS_Left;
-return (
-(!Left.isOneOf(TT_PointerOrReference, tok::l_paren) &&
- (getTokenPointerOrReferenceAlignment(Right) != FormatStyle::PAS_Left ||
-  (Line.IsMultiVariableDeclStmt &&
-   (Left.NestingLevel == 0 ||
-(Left.NestingLevel == 1 && Line.First->is(tok::kw_for)));
+return !Left.isOneOf(TT_PointerOrReference, tok::l_paren) &&
+   (getTokenPointerOrReferenceAlignment(Right) !=
+FormatStyle::PAS_Left ||
+(Line.IsMultiVariableDeclStmt &&
+ (Left.NestingLevel == 0 ||
+  (Left.NestingLevel == 1 &&
+   startsWithLoopWithInitializer(Line);
   }
   if (Right.is(TT_FunctionTypeLParen) && Left.isNot(tok::l_paren) &&
   (!Left.is(TT_PointerOrReference) ||
@@ -3127,7 +3132,15 @@
   return false;
 if (getTokenPointerOrReferenceAlignment(Left) == FormatStyle::PAS_Right)
   return false;
-if (Line.IsMultiVariableDeclStmt)
+// FIXME: Setting IsMultiVariableDeclStmt for the whole line is error-prone,
+// because it does not take into account nested scopes like lambdas.
+// In multi-variable declaration statements, attach */& to the variable
+// independently of the style. However, avoid doing it if we are in a nested
+// scope, e.g. lambda. We still need to special-case loops with
+// initializers.
+if 

[clang] abe1bb7 - [clang] MicrosoftCXXNameMangler::mangleObjCKindOfType - use castAs<> instead of getAs<> to avoid dereference of nullptr

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-12T20:11:21Z
New Revision: abe1bb763e3a7ce197f310afbff6112eb605136f

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

LOG: [clang] MicrosoftCXXNameMangler::mangleObjCKindOfType - use castAs<> 
instead of getAs<> to avoid dereference of nullptr

The pointer is referenced in the mangleType call, so assert the cast is correct 
instead of returning nullptr

Added: 


Modified: 
clang/lib/AST/MicrosoftMangle.cpp

Removed: 




diff  --git a/clang/lib/AST/MicrosoftMangle.cpp 
b/clang/lib/AST/MicrosoftMangle.cpp
index 8802b6e500a67..7329f163d12c9 100644
--- a/clang/lib/AST/MicrosoftMangle.cpp
+++ b/clang/lib/AST/MicrosoftMangle.cpp
@@ -1943,7 +1943,7 @@ void MicrosoftCXXNameMangler::mangleObjCKindOfType(const 
ObjCObjectType *T,
   Extra.mangleSourceName("KindOf");
   Extra.mangleType(QualType(T, 0)
.stripObjCKindOfType(getASTContext())
-   ->getAs(),
+   ->castAs(),
Quals, Range);
 
   mangleArtificialTagType(TTK_Struct, TemplateMangling, {"__ObjC"});



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

[PATCH] D119364: Refactor nested if else with ternary operator in CGExprScalar.cpp

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

It's nothing to do with your patch, and you don't need to worry about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119364

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

[PATCH] D119364: Refactor nested if else with ternary operator in CGExprScalar.cpp

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets added a comment.

Hey @rjmccall , would you tell me about this build fail 
https://green.lab.llvm.org/green/job/lldb-cmake/41339/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119364

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

[clang-tools-extra] 5d1e3ed - [clang-tidy] SimplifyBooleanExprCheck - use cast<> instead of dyn_cast<> to avoid dereference of nullptr

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-12T19:59:30Z
New Revision: 5d1e3ed3e257535fbd51706023685d2c2d9eba51

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

LOG: [clang-tidy] SimplifyBooleanExprCheck - use cast<> instead of dyn_cast<> 
to avoid dereference of nullptr

The IfStmt pointer is always referenced inside the 
replaceCompoundReturnWithCondition call, so assert the cast is correct instead 
of returning nullptr

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index 9c2ddf139a128..61ba4b857636c 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -750,7 +750,7 @@ void 
SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition(
 void SimplifyBooleanExprCheck::replaceCaseCompoundReturnWithCondition(
 const MatchFinder::MatchResult , bool Negated) {
   const auto *CaseDefault = Result.Nodes.getNodeAs(CaseId);
-  const auto *If = dyn_cast(CaseDefault->getSubStmt());
+  const auto *If = cast(CaseDefault->getSubStmt());
   replaceCompoundReturnWithCondition(Result, Negated, If);
 }
 
@@ -758,14 +758,14 @@ void 
SimplifyBooleanExprCheck::replaceDefaultCompoundReturnWithCondition(
 const MatchFinder::MatchResult , bool Negated) {
   const SwitchCase *CaseDefault =
   Result.Nodes.getNodeAs(DefaultId);
-  const auto *If = dyn_cast(CaseDefault->getSubStmt());
+  const auto *If = cast(CaseDefault->getSubStmt());
   replaceCompoundReturnWithCondition(Result, Negated, If);
 }
 
 void SimplifyBooleanExprCheck::replaceLabelCompoundReturnWithCondition(
 const MatchFinder::MatchResult , bool Negated) {
   const auto *Label = Result.Nodes.getNodeAs(LabelId);
-  const auto *If = dyn_cast(Label->getSubStmt());
+  const auto *If = cast(Label->getSubStmt());
   replaceCompoundReturnWithCondition(Result, Negated, If);
 }
 



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

[PATCH] D119638: [OpenMP][NFC] Simplify identifying the device bitcode library

[Joseph Huber via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa0e8077d285a: [OpenMP][NFC] Simplify identifying the device 
bitcode library (authored by jhuber6).

Changed prior to commit:
  https://reviews.llvm.org/D119638?vs=408193=408208#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119638

Files:
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -748,9 +748,7 @@
 if (getDriver().isUsingLTO(/* IsOffload */ true))
   return;
 
-std::string BitcodeSuffix = "nvptx-" + GpuArch.str();
-
-addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, BitcodeSuffix,
+addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, GpuArch.str(),
getTriple());
 AddStaticDeviceLibsPostLinking(getDriver(), DriverArgs, CC1Args, "nvptx",
GpuArch, /*isBitCodeSDL=*/true,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -2011,8 +2011,9 @@
   Triple.isAMDGCN() ? options::OPT_libomptarget_amdgpu_bc_path_EQ
 : options::OPT_libomptarget_nvptx_bc_path_EQ;
 
-  StringRef ArchPrefix = Triple.isAMDGCN() ? "amdgcn" : "nvptx";
-  std::string LibOmpTargetName = "libomptarget-" + BitcodeSuffix.str() + ".bc";
+  StringRef ArchPrefix = Triple.isAMDGCN() ? "amdgpu" : "nvptx";
+  std::string LibOmpTargetName =
+  ("libomptarget-" + ArchPrefix + "-" + BitcodeSuffix + ".bc").str();
 
   // First check whether user specifies bc library
   if (const Arg *A = DriverArgs.getLastArg(LibomptargetBCPathOpt)) {
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -8212,15 +8212,8 @@
   const ArgList  = C.getArgsForToolChain(TC, "", 
Action::OFK_OpenMP);
   StringRef Arch = TCArgs.getLastArgValue(options::OPT_march_EQ);
 
-  std::string BitcodeSuffix;
-  if (TC->getTriple().isNVPTX())
-BitcodeSuffix += "nvptx-";
-  else if (TC->getTriple().isAMDGPU())
-BitcodeSuffix += "amdgpu-";
-  BitcodeSuffix += Arch;
-
   ArgStringList BitcodeLibrary;
-  addOpenMPDeviceRTL(TCDriver, TCArgs, BitcodeLibrary, BitcodeSuffix,
+  addOpenMPDeviceRTL(TCDriver, TCArgs, BitcodeLibrary, Arch,
  TC->getTriple());
 
   if (!BitcodeLibrary.empty())
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -289,10 +289,7 @@
   if (getDriver().isUsingLTO(/* IsOffload */ true))
 return;
 
-  std::string BitcodeSuffix = "amdgpu-" + GPUArch;
-
-  addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, BitcodeSuffix,
- getTriple());
+  addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, GPUArch, getTriple());
 }
 
 llvm::opt::DerivedArgList *AMDGPUOpenMPToolChain::TranslateArgs(


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -748,9 +748,7 @@
 if (getDriver().isUsingLTO(/* IsOffload */ true))
   return;
 
-std::string BitcodeSuffix = "nvptx-" + GpuArch.str();
-
-addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, BitcodeSuffix,
+addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, GpuArch.str(),
getTriple());
 AddStaticDeviceLibsPostLinking(getDriver(), DriverArgs, CC1Args, "nvptx",
GpuArch, /*isBitCodeSDL=*/true,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -2011,8 +2011,9 @@
   Triple.isAMDGCN() ? options::OPT_libomptarget_amdgpu_bc_path_EQ
 : options::OPT_libomptarget_nvptx_bc_path_EQ;
 
-  StringRef ArchPrefix = Triple.isAMDGCN() ? "amdgcn" : "nvptx";
-  std::string LibOmpTargetName = "libomptarget-" + BitcodeSuffix.str() + ".bc";
+  StringRef ArchPrefix = Triple.isAMDGCN() ? "amdgpu" : "nvptx";
+  std::string LibOmpTargetName =
+  ("libomptarget-" + ArchPrefix + "-" + BitcodeSuffix + ".bc").str();
 
   // First check whether user specifies bc 

[clang] a0e8077 - [OpenMP][NFC] Simplify identifying the device bitcode library

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2022-02-12T14:55:47-05:00
New Revision: a0e8077d285ad35578677e3e03375cb9f1208e36

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

LOG: [OpenMP][NFC] Simplify identifying the device bitcode library

Now that the old device runtime has been deleted there is only a single
target that differs by the triple and the architecture. Simplify the
scheme for identifying the library but directly using the triple.

Reviewed By: JonChesterfield

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/lib/Driver/ToolChains/Cuda.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp 
b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
index 3088ba593c246..998bb0b9f7c94 100644
--- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -289,10 +289,7 @@ void AMDGPUOpenMPToolChain::addClangTargetOptions(
   if (getDriver().isUsingLTO(/* IsOffload */ true))
 return;
 
-  std::string BitcodeSuffix = "amdgpu-" + GPUArch;
-
-  addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, BitcodeSuffix,
- getTriple());
+  addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, GPUArch, getTriple());
 }
 
 llvm::opt::DerivedArgList *AMDGPUOpenMPToolChain::TranslateArgs(

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index fb71f98ac7dbd..f87ce37011366 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -8212,15 +8212,8 @@ void LinkerWrapper::ConstructJob(Compilation , const 
JobAction ,
   const ArgList  = C.getArgsForToolChain(TC, "", 
Action::OFK_OpenMP);
   StringRef Arch = TCArgs.getLastArgValue(options::OPT_march_EQ);
 
-  std::string BitcodeSuffix;
-  if (TC->getTriple().isNVPTX())
-BitcodeSuffix += "nvptx-";
-  else if (TC->getTriple().isAMDGPU())
-BitcodeSuffix += "amdgpu-";
-  BitcodeSuffix += Arch;
-
   ArgStringList BitcodeLibrary;
-  addOpenMPDeviceRTL(TCDriver, TCArgs, BitcodeLibrary, BitcodeSuffix,
+  addOpenMPDeviceRTL(TCDriver, TCArgs, BitcodeLibrary, Arch,
  TC->getTriple());
 
   if (!BitcodeLibrary.empty())

diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 71f5bd7cb2820..f0c32cd02f55a 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -2011,8 +2011,9 @@ void tools::addOpenMPDeviceRTL(const Driver ,
   Triple.isAMDGCN() ? options::OPT_libomptarget_amdgpu_bc_path_EQ
 : options::OPT_libomptarget_nvptx_bc_path_EQ;
 
-  StringRef ArchPrefix = Triple.isAMDGCN() ? "amdgcn" : "nvptx";
-  std::string LibOmpTargetName = "libomptarget-" + BitcodeSuffix.str() + ".bc";
+  StringRef ArchPrefix = Triple.isAMDGCN() ? "amdgpu" : "nvptx";
+  std::string LibOmpTargetName =
+  ("libomptarget-" + ArchPrefix + "-" + BitcodeSuffix + ".bc").str();
 
   // First check whether user specifies bc library
   if (const Arg *A = DriverArgs.getLastArg(LibomptargetBCPathOpt)) {

diff  --git a/clang/lib/Driver/ToolChains/Cuda.cpp 
b/clang/lib/Driver/ToolChains/Cuda.cpp
index 7dd54f87b5f44..d85327864c3d4 100644
--- a/clang/lib/Driver/ToolChains/Cuda.cpp
+++ b/clang/lib/Driver/ToolChains/Cuda.cpp
@@ -748,9 +748,7 @@ void CudaToolChain::addClangTargetOptions(
 if (getDriver().isUsingLTO(/* IsOffload */ true))
   return;
 
-std::string BitcodeSuffix = "nvptx-" + GpuArch.str();
-
-addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, BitcodeSuffix,
+addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, GpuArch.str(),
getTriple());
 AddStaticDeviceLibsPostLinking(getDriver(), DriverArgs, CC1Args, "nvptx",
GpuArch, /*isBitCodeSDL=*/true,



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

[PATCH] D119646: [clang] Allow consteval functions in default arguments

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron created this revision.
Izaron added reviewers: aaron.ballman, erichkeane, andreasfertig.
Herald added a subscriber: kristof.beyls.
Izaron requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We should not mark a function as "referenced" if we call it within a
ConstantExpr, because the expression will be folded to a value in LLVM IR.
To prevent emitting consteval function declarations, we should not "jump over"
a ConstantExpr when it is a top-level ParmVarDecl's subexpression.

Fixes https://github.com/llvm/llvm-project/issues/48230


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119646

Files:
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -613,6 +613,27 @@
 
 } // namespace unevaluated
 
+namespace default_argument {
+
+// Previously calls of consteval functions in default arguments were rejected.
+// Now we show that we don't reject such calls.
+consteval int foo() { return 1; }
+consteval int bar(int i = foo()) { return i * i; }
+
+struct Test1 {
+  Test1(int i = bar(13)) {}
+  void v(int i = bar(13) * 2 + bar(15)) {}
+};
+Test1 t1;
+
+struct Test2 {
+  constexpr Test2(int i = bar()) {}
+  constexpr void v(int i = bar(bar(bar(foo() {}
+};
+Test2 t2;
+
+} // namespace default_argument
+
 namespace PR50779 {
 struct derp {
   int b = 0;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -18964,6 +18964,12 @@
 Inherited::Visit(E);
   }
 
+  void VisitConstantExpr(ConstantExpr *E) {
+// Don't mark declarations within a ConstantExpression, as this expression
+// will be evaluated and folded to a value.
+return;
+  }
+
   void VisitDeclRefExpr(DeclRefExpr *E) {
 // If we were asked not to visit local variables, don't.
 if (SkipLocalVariables) {
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2813,7 +2813,8 @@
 
   Expr *Arg = getInit();
   if (auto *E = dyn_cast_or_null(Arg))
-return E->getSubExpr();
+if (!isa(E))
+  return E->getSubExpr();
 
   return Arg;
 }


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -613,6 +613,27 @@
 
 } // namespace unevaluated
 
+namespace default_argument {
+
+// Previously calls of consteval functions in default arguments were rejected.
+// Now we show that we don't reject such calls.
+consteval int foo() { return 1; }
+consteval int bar(int i = foo()) { return i * i; }
+
+struct Test1 {
+  Test1(int i = bar(13)) {}
+  void v(int i = bar(13) * 2 + bar(15)) {}
+};
+Test1 t1;
+
+struct Test2 {
+  constexpr Test2(int i = bar()) {}
+  constexpr void v(int i = bar(bar(bar(foo() {}
+};
+Test2 t2;
+
+} // namespace default_argument
+
 namespace PR50779 {
 struct derp {
   int b = 0;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -18964,6 +18964,12 @@
 Inherited::Visit(E);
   }
 
+  void VisitConstantExpr(ConstantExpr *E) {
+// Don't mark declarations within a ConstantExpression, as this expression
+// will be evaluated and folded to a value.
+return;
+  }
+
   void VisitDeclRefExpr(DeclRefExpr *E) {
 // If we were asked not to visit local variables, don't.
 if (SkipLocalVariables) {
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2813,7 +2813,8 @@
 
   Expr *Arg = getInit();
   if (auto *E = dyn_cast_or_null(Arg))
-return E->getSubExpr();
+if (!isa(E))
+  return E->getSubExpr();
 
   return Arg;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118070: Make lld-link work in a non-MSVC shell

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks for the revert!

FWIW, also failed on 
https://ci.chromium.org/ui/p/chromium/builders/try/win_upload_clang/2069/overview
 / 
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8822456541383185105/+/u/package_clang/stdout?format=raw
 . So maybe it's not a super exotic failure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118070

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

[PATCH] D119364: Refactor nested if else with ternary operator in CGExprScalar.cpp

[Shivam Rajput via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGde4e855204aa: Refactor nested if else with ternary operator 
in CGExprScalar.cpp (authored by phyBrackets).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119364

Files:
  clang/lib/CodeGen/CGExprScalar.cpp


Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -65,20 +65,14 @@
   const auto  = LHS->getValue();
   const auto  = RHS->getValue();
   if (Opcode == BO_Add) {
-if (Signed)
-  Result = LHSAP.sadd_ov(RHSAP, Overflow);
-else
-  Result = LHSAP.uadd_ov(RHSAP, Overflow);
+Result = Signed ? LHSAP.sadd_ov(RHSAP, Overflow)
+: LHSAP.uadd_ov(RHSAP, Overflow);
   } else if (Opcode == BO_Sub) {
-if (Signed)
-  Result = LHSAP.ssub_ov(RHSAP, Overflow);
-else
-  Result = LHSAP.usub_ov(RHSAP, Overflow);
+Result = Signed ? LHSAP.ssub_ov(RHSAP, Overflow)
+: LHSAP.usub_ov(RHSAP, Overflow);
   } else if (Opcode == BO_Mul) {
-if (Signed)
-  Result = LHSAP.smul_ov(RHSAP, Overflow);
-else
-  Result = LHSAP.umul_ov(RHSAP, Overflow);
+Result = Signed ? LHSAP.smul_ov(RHSAP, Overflow)
+: LHSAP.umul_ov(RHSAP, Overflow);
   } else if (Opcode == BO_Div || Opcode == BO_Rem) {
 if (Signed && !RHS->isZero())
   Result = LHSAP.sdiv_ov(RHSAP, Overflow);


Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -65,20 +65,14 @@
   const auto  = LHS->getValue();
   const auto  = RHS->getValue();
   if (Opcode == BO_Add) {
-if (Signed)
-  Result = LHSAP.sadd_ov(RHSAP, Overflow);
-else
-  Result = LHSAP.uadd_ov(RHSAP, Overflow);
+Result = Signed ? LHSAP.sadd_ov(RHSAP, Overflow)
+: LHSAP.uadd_ov(RHSAP, Overflow);
   } else if (Opcode == BO_Sub) {
-if (Signed)
-  Result = LHSAP.ssub_ov(RHSAP, Overflow);
-else
-  Result = LHSAP.usub_ov(RHSAP, Overflow);
+Result = Signed ? LHSAP.ssub_ov(RHSAP, Overflow)
+: LHSAP.usub_ov(RHSAP, Overflow);
   } else if (Opcode == BO_Mul) {
-if (Signed)
-  Result = LHSAP.smul_ov(RHSAP, Overflow);
-else
-  Result = LHSAP.umul_ov(RHSAP, Overflow);
+Result = Signed ? LHSAP.smul_ov(RHSAP, Overflow)
+: LHSAP.umul_ov(RHSAP, Overflow);
   } else if (Opcode == BO_Div || Opcode == BO_Rem) {
 if (Signed && !RHS->isZero())
   Result = LHSAP.sdiv_ov(RHSAP, Overflow);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] de4e855 - Refactor nested if else with ternary operator in CGExprScalar.cpp

[via cfe-commits]

Author: phyBrackets
Date: 2022-02-13T00:15:35+05:30
New Revision: de4e855204aab9ecf0507f57341cad9c349b3c49

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

LOG: Refactor nested if else with ternary operator in CGExprScalar.cpp

Reviewed By: rjmccall

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

Added: 


Modified: 
clang/lib/CodeGen/CGExprScalar.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index 59d0bd08d33d2..a235639054935 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -65,20 +65,14 @@ bool mayHaveIntegerOverflow(llvm::ConstantInt *LHS, 
llvm::ConstantInt *RHS,
   const auto  = LHS->getValue();
   const auto  = RHS->getValue();
   if (Opcode == BO_Add) {
-if (Signed)
-  Result = LHSAP.sadd_ov(RHSAP, Overflow);
-else
-  Result = LHSAP.uadd_ov(RHSAP, Overflow);
+Result = Signed ? LHSAP.sadd_ov(RHSAP, Overflow)
+: LHSAP.uadd_ov(RHSAP, Overflow);
   } else if (Opcode == BO_Sub) {
-if (Signed)
-  Result = LHSAP.ssub_ov(RHSAP, Overflow);
-else
-  Result = LHSAP.usub_ov(RHSAP, Overflow);
+Result = Signed ? LHSAP.ssub_ov(RHSAP, Overflow)
+: LHSAP.usub_ov(RHSAP, Overflow);
   } else if (Opcode == BO_Mul) {
-if (Signed)
-  Result = LHSAP.smul_ov(RHSAP, Overflow);
-else
-  Result = LHSAP.umul_ov(RHSAP, Overflow);
+Result = Signed ? LHSAP.smul_ov(RHSAP, Overflow)
+: LHSAP.umul_ov(RHSAP, Overflow);
   } else if (Opcode == BO_Div || Opcode == BO_Rem) {
 if (Signed && !RHS->isZero())
   Result = LHSAP.sdiv_ov(RHSAP, Overflow);



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

[PATCH] D119638: [OpenMP][NFC] Simplify identifying the device bitcode library

[Jon Chesterfield via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:2014
 
-  StringRef ArchPrefix = Triple.isAMDGCN() ? "amdgcn" : "nvptx";
-  std::string LibOmpTargetName = "libomptarget-" + BitcodeSuffix.str() + ".bc";
+  std::string ArchPrefix = Triple.isAMDGCN() ? "amdgpu" : "nvptx";
+  std::string LibOmpTargetName =

ArchPrefix could be a const char* or stringref as before, one fewer malloc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119638

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

[PATCH] D119638: [OpenMP][NFC] Simplify identifying the device bitcode library

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added reviewers: JonChesterfield, jdoerfert.
Herald added subscribers: kerbowa, guansong, yaxunl, jvesely.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Now that the old device runtime has been deleted there is only a single
target that differs by the triple and the architecture. Simplify the
scheme for identifying the library but directly using the triple.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119638

Files:
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -748,9 +748,7 @@
 if (getDriver().isUsingLTO(/* IsOffload */ true))
   return;
 
-std::string BitcodeSuffix = "nvptx-" + GpuArch.str();
-
-addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, BitcodeSuffix,
+addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, GpuArch.str(),
getTriple());
 AddStaticDeviceLibsPostLinking(getDriver(), DriverArgs, CC1Args, "nvptx",
GpuArch, /*isBitCodeSDL=*/true,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -2011,8 +2011,9 @@
   Triple.isAMDGCN() ? options::OPT_libomptarget_amdgpu_bc_path_EQ
 : options::OPT_libomptarget_nvptx_bc_path_EQ;
 
-  StringRef ArchPrefix = Triple.isAMDGCN() ? "amdgcn" : "nvptx";
-  std::string LibOmpTargetName = "libomptarget-" + BitcodeSuffix.str() + ".bc";
+  std::string ArchPrefix = Triple.isAMDGCN() ? "amdgpu" : "nvptx";
+  std::string LibOmpTargetName =
+  "libomptarget-" + ArchPrefix + "-" + BitcodeSuffix.str() + ".bc";
 
   // First check whether user specifies bc library
   if (const Arg *A = DriverArgs.getLastArg(LibomptargetBCPathOpt)) {
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -8212,15 +8212,8 @@
   const ArgList  = C.getArgsForToolChain(TC, "", 
Action::OFK_OpenMP);
   StringRef Arch = TCArgs.getLastArgValue(options::OPT_march_EQ);
 
-  std::string BitcodeSuffix;
-  if (TC->getTriple().isNVPTX())
-BitcodeSuffix += "nvptx-";
-  else if (TC->getTriple().isAMDGPU())
-BitcodeSuffix += "amdgpu-";
-  BitcodeSuffix += Arch;
-
   ArgStringList BitcodeLibrary;
-  addOpenMPDeviceRTL(TCDriver, TCArgs, BitcodeLibrary, BitcodeSuffix,
+  addOpenMPDeviceRTL(TCDriver, TCArgs, BitcodeLibrary, Arch,
  TC->getTriple());
 
   if (!BitcodeLibrary.empty())
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -289,10 +289,7 @@
   if (getDriver().isUsingLTO(/* IsOffload */ true))
 return;
 
-  std::string BitcodeSuffix = "amdgpu-" + GPUArch;
-
-  addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, BitcodeSuffix,
- getTriple());
+  addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, GPUArch, getTriple());
 }
 
 llvm::opt::DerivedArgList *AMDGPUOpenMPToolChain::TranslateArgs(


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -748,9 +748,7 @@
 if (getDriver().isUsingLTO(/* IsOffload */ true))
   return;
 
-std::string BitcodeSuffix = "nvptx-" + GpuArch.str();
-
-addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, BitcodeSuffix,
+addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args, GpuArch.str(),
getTriple());
 AddStaticDeviceLibsPostLinking(getDriver(), DriverArgs, CC1Args, "nvptx",
GpuArch, /*isBitCodeSDL=*/true,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -2011,8 +2011,9 @@
   Triple.isAMDGCN() ? options::OPT_libomptarget_amdgpu_bc_path_EQ
 : options::OPT_libomptarget_nvptx_bc_path_EQ;
 
-  StringRef ArchPrefix = Triple.isAMDGCN() ? "amdgcn" : "nvptx";
-  std::string LibOmpTargetName = "libomptarget-" + BitcodeSuffix.str() + ".bc";
+  std::string ArchPrefix = Triple.isAMDGCN() ? "amdgpu" : "nvptx";
+  std::string 

[PATCH] D119599: Add option to align compound assignments like `+=`

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:151
 
+  /// When aligning assignments, whether compound assignments like
+  /// ``+=``'s are aligned along with ``=``'s.

sstwcw wrote:
> curdeius wrote:
> > You need to update the RST files when updating the doc comments here. 
> > Please use 
> > https://github.com/llvm/llvm-project/blob/main/clang/docs/tools/dump_format_style.py
> >  for that.
> I guess it is saying I need to say what version the option first appeared in. 
>  How do I know what version it will be?
The next major, currently 15.
If it would take to long to review we change the version in between. But since 
the branching for 14 has quite recently happened this is either going in 15, or 
not at all. :)



Comment at: clang/include/clang/Format/Format.h:157
+  ///   a   &= 2;
+  ///   bbb  = 2;
+  ///

sstwcw wrote:
> HazardyKnusperkeks wrote:
> > curdeius wrote:
> > > I guess it would be complicated to avoid adding an additional space here. 
> > > I mean, it could be:
> > > ```
> > > a  &= 2;
> > > bbb = 2;
> > > ```
> > > And with 3-char operators, there's one more space.
> > That would be awesome, but it should be an option to turn off or on.
> > But I think this would really be complicated.
> I can do it either way. But I thought without the extra space the formatted 
> code looked ugly, especially when mixing `>>=` and `=`.  Which way do you 
> prefer?
> 
> 
> ```
> a >>= 2;
> bbb = 2;
> ```
I would prefer an option, to be able to do both.
I think I would use the variant which is more compact, but can't say for sure 
until I really have the option and tried it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D119625: Fix integer underflow bug when aligning code in clang-format

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a reviewer: HazardyKnusperkeks.
HazardyKnusperkeks requested changes to this revision.
HazardyKnusperkeks added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Format/WhitespaceManager.cpp:578
 }
-unsigned ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter;
+int ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter;
 

Could you elaborate the problem? Add a test/assert?



Comment at: clang/unittests/Format/FormatTest.cpp:16854
+  // that things still get aligned.
+  Alignment.ColumnLimit = 20;
   EXPECT_EQ("inta(int   x,\n"

I disagree. Limit 0 and Limit != 0 behaves differently and you should not 
remove the old test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119625

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

[PATCH] D7982: [clang-tidy] Add readability-duplicate-include check

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

Thanks for this, it found 74 occurrences of this in the firefox code base
https://hg.mozilla.org/mozilla-central/rev/b07f125d09fd


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D7982

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

[PATCH] D119599: Add option to align compound assignments like `+=`

[sstwcw via Phabricator via cfe-commits]

sstwcw added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:273
 AlignTokenSequence(const FormatStyle , unsigned Start, unsigned End,
-   unsigned Column, F &,
+   unsigned Column, unsigned AnchorWidth, F &,
SmallVector ) {

MyDeveloperDay wrote:
> I guess I just don't understand what we mean by AnchorWidth
Now that I look at it again it seems like only one column parameter is needed.



Comment at: clang/unittests/Format/FormatTest.cpp:16452
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACS_AcrossEmptyLines;
+  EXPECT_EQ("int a   += 5;\n"
+"int one  = 1;\n"

MyDeveloperDay wrote:
> Is there a reason you couldn't use verifyFormat here? I would expect it to 
> `messUp` the formatting and put it back correctly no?
`messUp` turns every newline into a space.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D119599: Add option to align compound assignments like `+=`

[sstwcw via Phabricator via cfe-commits]

sstwcw updated this revision to Diff 408177.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16392,6 +16392,122 @@
Alignment));
 }
 
+TEST_F(FormatTest, AlignCompoundAssignments) {
+  FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Alignment.AlignCompoundAssignments = true;
+  verifyFormat("aa <= 5;\n"
+   "a   = 5;\n"
+   "bcd = 5;\n"
+   "ghtyf   = 5;\n"
+   "dvfvdb  = 5;\n"
+   "a   = 5;\n"
+   "vdsvsv  = 5;\n"
+   "sfdbddfbdfbb= 5;\n"
+   "dvsdsv  = 5;\n"
+   "int dsvvdvsdvvv = 123;",
+   Alignment);
+  verifyFormat("aa <= 5;\n"
+   "a   &= 5;\n"
+   "bcd *= 5;\n"
+   "ghtyf   += 5;\n"
+   "dvfvdb  -= 5;\n"
+   "a   /= 5;\n"
+   "vdsvsv  %= 5;\n"
+   "sfdbddfbdfbb^= 5;\n"
+   "dvsdsv  |= 5;\n"
+   "int dsvvdvsdvvv  = 123;",
+   Alignment);
+  verifyFormat("a   &= 5;\n"
+   "bcd *= 5;\n"
+   "ghtyf  >>= 5;\n"
+   "dvfvdb  -= 5;\n"
+   "a   /= 5;\n"
+   "aa <= 5;\n"
+   "vdsvsv   %= 5;\n"
+   "sfdbddfbdfbb ^= 5;\n"
+   "dvsdsv  <<= 5;\n"
+   "int dsvvdvsdvvv   = 123;",
+   Alignment);
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  EXPECT_EQ("int a   += 5;\n"
+"int one  = 1;\n"
+"\n"
+"int oneTwoThree = 123;\n",
+format("int a += 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;\n",
+   Alignment));
+  EXPECT_EQ("int a   += 5;\n"
+"int one  = 1;\n"
+"//\n"
+"int oneTwoThree = 123;\n",
+format("int a += 5;\n"
+   "int one = 1;\n"
+   "//\n"
+   "int oneTwoThree = 123;\n",
+   Alignment));
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACS_AcrossEmptyLines;
+  EXPECT_EQ("int a   += 5;\n"
+"int one  = 1;\n"
+"\n"
+"int oneTwoThree  = 123;\n",
+format("int a += 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;\n",
+   Alignment));
+  EXPECT_EQ("int a   += 5;\n"
+"int one  = 1;\n"
+"//\n"
+"int oneTwoThree = 123;\n",
+format("int a += 5;\n"
+   "int one = 1;\n"
+   "//\n"
+   "int oneTwoThree = 123;\n",
+   Alignment));
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACS_AcrossComments;
+  EXPECT_EQ("int a   += 5;\n"
+"int one  = 1;\n"
+"\n"
+"int oneTwoThree = 123;\n",
+format("int a += 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;\n",
+   Alignment));
+  EXPECT_EQ("int a   += 5;\n"
+"int one  = 1;\n"
+"//\n"
+"int oneTwoThree  = 123;\n",
+format("int a += 5;\n"
+   "int one = 1;\n"
+   "//\n"
+   "int oneTwoThree = 123;\n",
+   Alignment));
+  Alignment.AlignConsecutiveAssignments =
+  FormatStyle::ACS_AcrossEmptyLinesAndComments;
+  EXPECT_EQ("int a+= 5;\n"
+"int one >>= 1;\n"
+"\n"
+"int oneTwoThree   = 123;\n",
+format("int a += 5;\n"
+   "int one >>= 1;\n"
+   "\n"
+   "int oneTwoThree = 123;\n",
+   Alignment));
+  EXPECT_EQ("int a+= 5;\n"
+"int one   = 1;\n"
+"//\n"
+"int oneTwoThree <<= 123;\n",
+format("int a += 5;\n"
+   "int one = 1;\n"
+   "//\n"
+   "int oneTwoThree <<= 123;\n",
+   Alignment));
+}
+
 TEST_F(FormatTest, AlignConsecutiveAssignments) {
   

[PATCH] D119597: [clang-format][NFC] Give State.Stack.back() a meaningful name

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

@HazardyKnusperkeks agreed, I'm fine with this approach (especially on what is 
one of the more confusing parts of clang-format)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119597

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

[PATCH] D119599: Add option to align compound assignments like `+=`

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay requested changes to this revision.
MyDeveloperDay added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Format/WhitespaceManager.cpp:273
 AlignTokenSequence(const FormatStyle , unsigned Start, unsigned End,
-   unsigned Column, F &,
+   unsigned Column, unsigned AnchorWidth, F &,
SmallVector ) {

I guess I just don't understand what we mean by AnchorWidth



Comment at: clang/unittests/Format/FormatTest.cpp:16452
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACS_AcrossEmptyLines;
+  EXPECT_EQ("int a   += 5;\n"
+"int one  = 1;\n"

Is there a reason you couldn't use verifyFormat here? I would expect it to 
`messUp` the formatting and put it back correctly no?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D113372: [Driver] Add CLANG_DEFAULT_PIE_ON_LINUX to emulate GCC --enable-default-pie

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

When enabled, this seems to break a fair number of tests:

  Clang :: CodeGen/mips-vector-return.c
  Clang :: Driver/hexagon-toolchain-elf.c
  Clang :: Driver/hip-fpie-option.hip
  Clang :: Driver/mips-cs.cpp
  Clang :: Driver/mips-fsf.cpp
  Clang :: Driver/mips-img-v2.cpp
  Clang :: Driver/mips-img.cpp
  Clang :: Driver/mips-mti-linux.c

I've been able to get the hexagon test to work by passing `-fno-pie` explicitly 
but others don't seem this easy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113372

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

[PATCH] D119599: Add option to align compound assignments like `+=`

[sstwcw via Phabricator via cfe-commits]

sstwcw updated this revision to Diff 408172.
sstwcw marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16392,6 +16392,122 @@
Alignment));
 }
 
+TEST_F(FormatTest, AlignCompoundAssignments) {
+  FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Alignment.AlignCompoundAssignments = true;
+  verifyFormat("aa <= 5;\n"
+   "a   = 5;\n"
+   "bcd = 5;\n"
+   "ghtyf   = 5;\n"
+   "dvfvdb  = 5;\n"
+   "a   = 5;\n"
+   "vdsvsv  = 5;\n"
+   "sfdbddfbdfbb= 5;\n"
+   "dvsdsv  = 5;\n"
+   "int dsvvdvsdvvv = 123;",
+   Alignment);
+  verifyFormat("aa <= 5;\n"
+   "a   &= 5;\n"
+   "bcd *= 5;\n"
+   "ghtyf   += 5;\n"
+   "dvfvdb  -= 5;\n"
+   "a   /= 5;\n"
+   "vdsvsv  %= 5;\n"
+   "sfdbddfbdfbb^= 5;\n"
+   "dvsdsv  |= 5;\n"
+   "int dsvvdvsdvvv  = 123;",
+   Alignment);
+  verifyFormat("a   &= 5;\n"
+   "bcd *= 5;\n"
+   "ghtyf  >>= 5;\n"
+   "dvfvdb  -= 5;\n"
+   "a   /= 5;\n"
+   "aa <= 5;\n"
+   "vdsvsv   %= 5;\n"
+   "sfdbddfbdfbb ^= 5;\n"
+   "dvsdsv  <<= 5;\n"
+   "int dsvvdvsdvvv   = 123;",
+   Alignment);
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  EXPECT_EQ("int a   += 5;\n"
+"int one  = 1;\n"
+"\n"
+"int oneTwoThree = 123;\n",
+format("int a += 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;\n",
+   Alignment));
+  EXPECT_EQ("int a   += 5;\n"
+"int one  = 1;\n"
+"//\n"
+"int oneTwoThree = 123;\n",
+format("int a += 5;\n"
+   "int one = 1;\n"
+   "//\n"
+   "int oneTwoThree = 123;\n",
+   Alignment));
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACS_AcrossEmptyLines;
+  EXPECT_EQ("int a   += 5;\n"
+"int one  = 1;\n"
+"\n"
+"int oneTwoThree  = 123;\n",
+format("int a += 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;\n",
+   Alignment));
+  EXPECT_EQ("int a   += 5;\n"
+"int one  = 1;\n"
+"//\n"
+"int oneTwoThree = 123;\n",
+format("int a += 5;\n"
+   "int one = 1;\n"
+   "//\n"
+   "int oneTwoThree = 123;\n",
+   Alignment));
+  Alignment.AlignConsecutiveAssignments = FormatStyle::ACS_AcrossComments;
+  EXPECT_EQ("int a   += 5;\n"
+"int one  = 1;\n"
+"\n"
+"int oneTwoThree = 123;\n",
+format("int a += 5;\n"
+   "int one = 1;\n"
+   "\n"
+   "int oneTwoThree = 123;\n",
+   Alignment));
+  EXPECT_EQ("int a   += 5;\n"
+"int one  = 1;\n"
+"//\n"
+"int oneTwoThree  = 123;\n",
+format("int a += 5;\n"
+   "int one = 1;\n"
+   "//\n"
+   "int oneTwoThree = 123;\n",
+   Alignment));
+  Alignment.AlignConsecutiveAssignments =
+  FormatStyle::ACS_AcrossEmptyLinesAndComments;
+  EXPECT_EQ("int a+= 5;\n"
+"int one >>= 1;\n"
+"\n"
+"int oneTwoThree   = 123;\n",
+format("int a += 5;\n"
+   "int one >>= 1;\n"
+   "\n"
+   "int oneTwoThree = 123;\n",
+   Alignment));
+  EXPECT_EQ("int a+= 5;\n"
+"int one   = 1;\n"
+"//\n"
+"int oneTwoThree <<= 123;\n",
+format("int a += 5;\n"
+   "int one = 1;\n"
+   "//\n"
+   "int oneTwoThree <<= 123;\n",
+   Alignment));
+}
+
 

[PATCH] D119599: Add option to align compound assignments like `+=`

[sstwcw via Phabricator via cfe-commits]

sstwcw marked 6 inline comments as done.
sstwcw added inline comments.



Comment at: clang/include/clang/Format/Format.h:151
 
+  /// When aligning assignments, whether compound assignments like
+  /// ``+=``'s are aligned along with ``=``'s.

curdeius wrote:
> You need to update the RST files when updating the doc comments here. Please 
> use 
> https://github.com/llvm/llvm-project/blob/main/clang/docs/tools/dump_format_style.py
>  for that.
I guess it is saying I need to say what version the option first appeared in.  
How do I know what version it will be?



Comment at: clang/include/clang/Format/Format.h:157
+  ///   a   &= 2;
+  ///   bbb  = 2;
+  ///

HazardyKnusperkeks wrote:
> curdeius wrote:
> > I guess it would be complicated to avoid adding an additional space here. I 
> > mean, it could be:
> > ```
> > a  &= 2;
> > bbb = 2;
> > ```
> > And with 3-char operators, there's one more space.
> That would be awesome, but it should be an option to turn off or on.
> But I think this would really be complicated.
I can do it either way. But I thought without the extra space the formatted 
code looked ugly, especially when mixing `>>=` and `=`.  Which way do you 
prefer?


```
a >>= 2;
bbb = 2;
```



Comment at: clang/lib/Format/WhitespaceManager.cpp:580
 
-unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn;
-int LineLengthAfter = Changes[i].TokenLength;
+unsigned ChangeWidthLeft = Changes[i].StartOfTokenColumn;
+unsigned ChangeWidthAnchor = 0;

MyDeveloperDay wrote:
> I like the final outcome, I'm uneasy about renaming all the variables, just 
> because you now understand them. I'm struggling to read the algorithm in the 
> same context as the prior version
> 
> 
Originally we tracked the column to put the operators to, so `ChangeMinColumn` 
and `ChangeMaxColumn` made sense.  Now the column we are tracking is no longer 
the column to which we change the start of the operators to, the original names 
don't make sense any more.



Comment at: clang/unittests/Format/FormatTest.cpp:16852
-  // otherwise the function parameters will be re-flowed onto a single line.
-  Alignment.ColumnLimit = 0;
   EXPECT_EQ("inta(int   x,\n"

MyDeveloperDay wrote:
> Huge `no` from me, don't change the tests because they break based on your 
> change.
Here is the problem in isolation: https://reviews.llvm.org/D119625.  What do 
you suggest about this test?



Comment at: clang/unittests/Format/FormatTest.cpp:16872
+  // that things still get aligned.
+  Alignment.ColumnLimit = 20;
   EXPECT_EQ("inta(int   x,\n"

curdeius wrote:
> Is it something that can be done/fixed separately?
I added a revision here https://reviews.llvm.org/D119625.  If you commit it 
separately there will be a merge conflict though.



Comment at: clang/unittests/Format/FormatTest.cpp:19277
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
+  CHECK_PARSE_BOOL(AlignTrailingComments);
   CHECK_PARSE_BOOL(BinPackArguments);

MyDeveloperDay wrote:
> `IJKL` last L looked i came before l right? so why did you move this down
Sorry.  I forgot my ABC's.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D119625: Fix integer underflow bug when aligning code in clang-format

[sstwcw via Phabricator via cfe-commits]

sstwcw added a comment.

This isolates the problem that changes the unit test mentioned in 
https://reviews.llvm.org/D119599.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119625

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

[PATCH] D119625: Fix integer underflow bug when aligning code in clang-format

[sstwcw via Phabricator via cfe-commits]

sstwcw created this revision.
sstwcw added a reviewer: clang-format.
sstwcw added a project: clang-format.
sstwcw 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/D119625

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16847,9 +16847,11 @@
"double b();",
Alignment);
   unsigned OldColumnLimit = Alignment.ColumnLimit;
-  // We need to set ColumnLimit to zero, in order to stress nested alignments,
-  // otherwise the function parameters will be re-flowed onto a single line.
-  Alignment.ColumnLimit = 0;
+  // We need to set ColumnLimit to a small number, in order to stress
+  // nested alignments, otherwise the function parameters will be
+  // re-flowed onto a single line. It also needs to be wide enough so
+  // that things still get aligned.
+  Alignment.ColumnLimit = 20;
   EXPECT_EQ("inta(int   x,\n"
 " float y);\n"
 "double b(intx,\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -575,7 +575,7 @@
   if (!Changes[j].IsInsideToken)
 LineLengthAfter += Changes[j].TokenLength;
 }
-unsigned ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter;
+int ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter;
 
 // If we are restricted by the maximum column width, end the sequence.
 if (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn ||


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16847,9 +16847,11 @@
"double b();",
Alignment);
   unsigned OldColumnLimit = Alignment.ColumnLimit;
-  // We need to set ColumnLimit to zero, in order to stress nested alignments,
-  // otherwise the function parameters will be re-flowed onto a single line.
-  Alignment.ColumnLimit = 0;
+  // We need to set ColumnLimit to a small number, in order to stress
+  // nested alignments, otherwise the function parameters will be
+  // re-flowed onto a single line. It also needs to be wide enough so
+  // that things still get aligned.
+  Alignment.ColumnLimit = 20;
   EXPECT_EQ("inta(int   x,\n"
 " float y);\n"
 "double b(intx,\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -575,7 +575,7 @@
   if (!Changes[j].IsInsideToken)
 LineLengthAfter += Changes[j].TokenLength;
 }
-unsigned ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter;
+int ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter;
 
 // If we are restricted by the maximum column width, end the sequence.
 if (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 2188e61 - [clang-doc] populateParentNamespaces - use cast<> instead of dyn_cast<> to avoid dereference of nullptr

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-12T11:28:50Z
New Revision: 2188e61691ad89e27583fd3a4197322be830df59

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

LOG: [clang-doc] populateParentNamespaces - use cast<> instead of dyn_cast<> to 
avoid dereference of nullptr

The pointer is referenced immediately, so assert the cast is correct instead of 
returning nullptr

It's only later iterations of the loop where the getParent() call might return 
nullptr

Added: 


Modified: 
clang-tools-extra/clang-doc/Serialize.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-doc/Serialize.cpp 
b/clang-tools-extra/clang-doc/Serialize.cpp
index 29762b6b54b1e..c567cda9b6e2c 100644
--- a/clang-tools-extra/clang-doc/Serialize.cpp
+++ b/clang-tools-extra/clang-doc/Serialize.cpp
@@ -357,7 +357,7 @@ template 
 static void
 populateParentNamespaces(llvm::SmallVector ,
  const T *D, bool ) {
-  const auto *DC = dyn_cast(D);
+  const auto *DC = cast(D);
   while ((DC = DC->getParent())) {
 if (const auto *N = dyn_cast(DC)) {
   std::string Namespace;



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

[clang-tools-extra] 7ad94bd - [clang-tidy] ContainerSizeEmptyCheck::check - simplify isa<> and dyn_cast<> repeated calls

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-12T11:31:27Z
New Revision: 7ad94bd74bb5e2c984f541bc1921a9eda73ed973

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

LOG: [clang-tidy] ContainerSizeEmptyCheck::check - simplify isa<> and 
dyn_cast<> repeated calls

Just use dyn_cast<> to determine literal + container values from the binop

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
index 548fed9a47c3..d399c957c7c7 100644
--- a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
@@ -218,23 +218,22 @@ void ContainerSizeEmptyCheck::check(const 
MatchFinder::MatchResult ) {
 Hint = FixItHint::CreateReplacement(BinCmpRewritten->getSourceRange(),
 ReplacementText);
   } else if (BinaryOp) { // Determine the correct transformation.
+const auto *LiteralLHS =
+llvm::dyn_cast(BinaryOp->getLHS()->IgnoreImpCasts());
+const auto *LiteralRHS =
+llvm::dyn_cast(BinaryOp->getRHS()->IgnoreImpCasts());
+const bool ContainerIsLHS = !LiteralLHS;
+
+uint64_t Value = 0;
+if (LiteralLHS)
+  Value = LiteralLHS->getValue().getLimitedValue();
+else if (LiteralRHS)
+  Value = LiteralRHS->getValue().getLimitedValue();
+else
+  return;
+
 bool Negation = false;
-const bool ContainerIsLHS =
-!llvm::isa(BinaryOp->getLHS()->IgnoreImpCasts());
 const auto OpCode = BinaryOp->getOpcode();
-uint64_t Value = 0;
-if (ContainerIsLHS) {
-  if (const auto *Literal = llvm::dyn_cast(
-  BinaryOp->getRHS()->IgnoreImpCasts()))
-Value = Literal->getValue().getLimitedValue();
-  else
-return;
-} else {
-  Value =
-  llvm::dyn_cast(BinaryOp->getLHS()->IgnoreImpCasts())
-  ->getValue()
-  .getLimitedValue();
-}
 
 // Constant that is not handled.
 if (Value > 1)



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

[clang] fbe0ca5 - [clang][sema] checkNoThrow - use cast<> instead of dyn_cast<> to avoid dereference of nullptr

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-12T11:18:49Z
New Revision: fbe0ca576d2b96c523182200529b5afbf709506f

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

LOG: [clang][sema] checkNoThrow - use cast<> instead of dyn_cast<> to avoid 
dereference of nullptr

The pointer is referenced immediately, so assert the cast is correct instead of 
returning nullptr

Added: 


Modified: 
clang/lib/Sema/SemaCoroutine.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index aae90c403f0f..71fcf4aebda8 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -680,7 +680,7 @@ static void checkNoThrow(Sema , const Stmt *E,
 QualType::DestructionKind::DK_cxx_destructor) {
   const auto *T =
   cast(ReturnType.getCanonicalType().getTypePtr());
-  checkDeclNoexcept(dyn_cast(T->getDecl())->getDestructor(),
+  checkDeclNoexcept(cast(T->getDecl())->getDestructor(),
 /*IsDtor=*/true);
 }
   } else



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

[clang] f0f4452 - [clang][sema] Sema::CheckFreeArguments - use cast<> instead of dyn_cast<> to avoid dereference of nullptr

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-12T11:17:02Z
New Revision: f0f4452ed488bc7d9c3b9693dc110f1107921a08

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

LOG: [clang][sema] Sema::CheckFreeArguments - use cast<> instead of dyn_cast<> 
to avoid dereference of nullptr

The pointer is referenced immediately, so assert the cast is correct instead of 
returning nullptr

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 575181a6b61c..c422981a1a2e 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -11310,7 +11310,7 @@ void CheckFreeArgumentsCast(Sema , const std::string 
,
 /// Alerts the user that they are attempting to free a non-malloc'd object.
 void Sema::CheckFreeArguments(const CallExpr *E) {
   const std::string CalleeName =
-  dyn_cast(E->getCalleeDecl())->getQualifiedNameAsString();
+  cast(E->getCalleeDecl())->getQualifiedNameAsString();
 
   { // Prefer something that doesn't involve a cast to make things simpler.
 const Expr *Arg = E->getArg(0)->IgnoreParenCasts();



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

[clang] 5e27487 - [clang][sema] TryStaticCast - use castAs<> instead of getAs<> to avoid dereference of nullptr

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-12T11:14:19Z
New Revision: 5e27487fc992cece0d703f0bd83480381764b77b

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

LOG: [clang][sema] TryStaticCast - use castAs<> instead of getAs<> to avoid 
dereference of nullptr

The pointer is referenced immediately, so assert the cast is correct instead of 
returning nullptr

Added: 


Modified: 
clang/lib/Sema/SemaCast.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index e9c1116257d66..7d580dc5b6b3a 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -1356,7 +1356,7 @@ static TryCastResult TryStaticCast(Sema , ExprResult 
,
 if (SrcType->isIntegralOrEnumerationType()) {
   // [expr.static.cast]p10 If the enumeration type has a fixed underlying
   // type, the value is first converted to that type by integral conversion
-  const EnumType *Enum = DestType->getAs();
+  const EnumType *Enum = DestType->castAs();
   Kind = Enum->getDecl()->isFixed() &&
  Enum->getDecl()->getIntegerType()->isBooleanType()
  ? CK_IntegralToBoolean



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

[clang-tools-extra] 869c066 - [clang-tidy] RedundantControlFlowCheck::issueDiagnostic - use cast<> instead of dyn_cast<> to avoid dereference of nullptr

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-12T11:05:59Z
New Revision: 869c066ca8a4231b950b5367f90758de0c6768a3

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

LOG: [clang-tidy] RedundantControlFlowCheck::issueDiagnostic - use cast<> 
instead of dyn_cast<> to avoid dereference of nullptr

The pointer is referenced immediately, so assert the cast is correct instead of 
returning nullptr

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
index 6af77635aa2b..dd63de3f048e 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
@@ -79,7 +79,7 @@ void RedundantControlFlowCheck::issueDiagnostic(
   SourceLocation Start;
   if (Previous != Block->body_rend())
 Start = Lexer::findLocationAfterToken(
-dyn_cast(*Previous)->getEndLoc(), tok::semi, SM, getLangOpts(),
+cast(*Previous)->getEndLoc(), tok::semi, SM, getLangOpts(),
 /*SkipTrailingWhitespaceAndNewLine=*/true);
   if (!Start.isValid())
 Start = StmtRange.getBegin();



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

[clang-tools-extra] 9acba52 - [clang-tidy] FunctionASTVisitor::TraverseStmt - use cast<> instead of dyn_cast<> to avoid dereference of nullptr

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-12T11:04:20Z
New Revision: 9acba5206a91d735bc56ba41f18eabfc81db0513

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

LOG: [clang-tidy] FunctionASTVisitor::TraverseStmt - use cast<> instead of 
dyn_cast<> to avoid dereference of nullptr

The pointer is referenced immediately, so assert the cast is correct instead of 
returning nullptr

Added: 


Modified: 

clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
index c27733c040833..40542cca54a44 100644
--- 
a/clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
+++ 
b/clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
@@ -444,8 +444,7 @@ class FunctionASTVisitor final
   // A little beautification.
   // For conditional operator "cond ? true : false" point at the "?"
   // symbol.
-  ConditionalOperator *COp = dyn_cast(Node);
-  Location = COp->getQuestionLoc();
+  Location = cast(Node)->getQuestionLoc();
 }
 
 // If we have found any reasons, let's account it.



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

[clang-tools-extra] ed5d659 - [clang-tidy] NoexceptMoveConstructorCheck::check - use castAs<> instead of getAs<> to avoid dereference of nullptr

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-12T10:57:09Z
New Revision: ed5d659038362d6ccfebad3d9b7e10e73e681196

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

LOG: [clang-tidy] NoexceptMoveConstructorCheck::check - use castAs<> instead of 
getAs<> to avoid dereference of nullptr

The pointer is referenced immediately, so assert the cast is correct instead of 
returning nullptr

Added: 


Modified: 
clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp 
b/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
index f87f214945fa2..782498bd69e9a 100644
--- a/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
@@ -38,7 +38,7 @@ void NoexceptMoveConstructorCheck::check(
   return;
 }
 
-const auto *ProtoType = Decl->getType()->getAs();
+const auto *ProtoType = Decl->getType()->castAs();
 
 if (isUnresolvedExceptionSpec(ProtoType->getExceptionSpecType()))
   return;



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

[clang-tools-extra] 99acc0d - [clang-tidy] UseNoexceptCheck::check - use cast<> instead of dyn_cast<> to avoid dereference of nullptr

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-12T10:51:48Z
New Revision: 99acc0dc6434373dc391ac4c89f6ae6b72ec7208

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

LOG: [clang-tidy] UseNoexceptCheck::check - use cast<> instead of dyn_cast<> to 
avoid dereference of nullptr

The pointer is referenced immediately, so assert the cast is correct instead of 
returning nullptr

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.cpp
index c4e7f12e74acb..0043eb0dab9a4 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.cpp
@@ -65,7 +65,7 @@ void UseNoexceptCheck::check(const MatchFinder::MatchResult 
) {
   } else if (const auto *ParmDecl =
  Result.Nodes.getNodeAs("parmVarDecl")) {
 FnTy = ParmDecl->getType()
-   ->getAs()
+   ->castAs()
->getPointeeType()
->getAs();
 



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

[clang-tools-extra] 003e038 - [clang-tidy] getOutermostNamespace - remove redundant nullptr check

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-12T10:49:54Z
New Revision: 003e0382647fa311e1bd48bc5298a3182d6f2552

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

LOG: [clang-tidy] getOutermostNamespace - remove redundant nullptr check

The pointer is always dereferenced

Added: 


Modified: 
clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp 
b/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
index 0636883c48193..91e9e026d0ad1 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
@@ -22,7 +22,7 @@ namespace llvm_libc {
 // Unit.
 const DeclContext *getOutermostNamespace(const DeclContext *Decl) {
   const DeclContext *Parent = Decl->getParent();
-  if (Parent && Parent->isTranslationUnit())
+  if (Parent->isTranslationUnit())
 return Decl;
   return getOutermostNamespace(Parent);
 }



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

[clang-tools-extra] 5bc13f9 - [clang-tidy] ProBoundsConstantArrayIndexCheck::check - use cast<> instead of dyn_cast<> to avoid dereference of nullptr

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-12T10:43:21Z
New Revision: 5bc13f9d9471adc81c8bf2b62775b6e2fd72f8e6

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

LOG: [clang-tidy] ProBoundsConstantArrayIndexCheck::check - use cast<> instead 
of dyn_cast<> to avoid dereference of nullptr

The pointer is referenced immediately, so assert the cast is correct instead of 
returning nullptr

Added: 


Modified: 

clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
index 59886ee4a3ebc..d7bc56511dfb5 100644
--- 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -71,7 +71,7 @@ void ProBoundsConstantArrayIndexCheck::check(
   BaseRange = ArraySubscriptE->getBase()->getSourceRange();
 else
   BaseRange =
-  dyn_cast(Matched)->getArg(0)->getSourceRange();
+  cast(Matched)->getArg(0)->getSourceRange();
 SourceRange IndexRange = IndexExpr->getSourceRange();
 
 auto Diag = diag(Matched->getExprLoc(),



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

[clang-tools-extra] 389bee7 - [clang-tidy] checkOverridingFunctionReturnType - use castAs<> instead of getAs<> to avoid dereference of nullptr

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-12T10:41:43Z
New Revision: 389bee71ea71bf5a36e3786d3f60d3aa62e30be3

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

LOG: [clang-tidy] checkOverridingFunctionReturnType - use castAs<> instead of 
getAs<> to avoid dereference of nullptr

The pointers are referenced immediately, so assert the cast is correct instead 
of returning nullptr

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/VirtualNearMissCheck.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/VirtualNearMissCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/VirtualNearMissCheck.cpp
index 150b517811b66..f2aeac2430958 100644
--- a/clang-tools-extra/clang-tidy/bugprone/VirtualNearMissCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/VirtualNearMissCheck.cpp
@@ -40,11 +40,11 @@ static bool checkOverridingFunctionReturnType(const 
ASTContext *Context,
   const CXXMethodDecl *BaseMD,
   const CXXMethodDecl *DerivedMD) {
   QualType BaseReturnTy = BaseMD->getType()
-  ->getAs()
+  ->castAs()
   ->getReturnType()
   .getCanonicalType();
   QualType DerivedReturnTy = DerivedMD->getType()
- ->getAs()
+ ->castAs()
  ->getReturnType()
  .getCanonicalType();
 



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

[PATCH] D119597: [clang-format][NFC] Give State.Stack.back() a meaningful name

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D119597#3316643 , @MyDeveloperDay 
wrote:

> I like this change for clarity reasons, my only concern and it's not based on 
> evidence is what if any of these functions get passed in State, and then they 
> themselves alter the State.Stack?
>
> In most cases I'd expect CurrentState to always be correct, but doesn't it 
> only represent the state of the stack as it was at the beginning of the 
> function
>
> Isn't the point of State.Stack.back() to get the current state? if you do 
> that with a variable like `CurrentState` or just have 
> `State.getCurrentState()` I don't know but the later seems a good step 
> because you could use that in even more places
>
> i.e.
>
> `unsigned NestedBlockIndent = State.Stack.back().NestedBlockIndent;`
>
> `unsigned NestedBlockIndent = State.getCurrentState().NestedBlockIndent;`

Currently everywhere where I have created the reference there are no changes to 
the stack afterwards. If someone will change that in the future they will get 
massive test failures, that I can promise from experience in this field.
My reason for this change was debugging, I had a really hard time to understand 
what all these values and their changes (and why they change) mean. With the 
local variable I could easily monitor the values in my debugger view. If we 
replace one function call `State.Stack.back()` with `State.getCurrentState()` 
we are back at square one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119597

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

[PATCH] D119599: Add option to align compound assignments like `+=`

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:157
+  ///   a   &= 2;
+  ///   bbb  = 2;
+  ///

curdeius wrote:
> I guess it would be complicated to avoid adding an additional space here. I 
> mean, it could be:
> ```
> a  &= 2;
> bbb = 2;
> ```
> And with 3-char operators, there's one more space.
That would be awesome, but it should be an option to turn off or on.
But I think this would really be complicated.



Comment at: clang/include/clang/Format/Format.h:163
+  /// \endcode
+  bool AlignCompoundAssignments;
+

MyDeveloperDay wrote:
> This option is not independent of `AlignConsecutiveAssignments` is it? this 
> will cause confusion when people turn it on without turning on 
> `AlignConsecutiveAssignments` 
> 
> Options have a lifecycle we have noticed
> 
> 1) They start as bool
> 2) They become enums
> 3) They then end up as a struct of bool
> 4) Sometimes the struct becomes of enums
> 
> Whilst I like what you are doing here I fear we will get bugs where people 
> say I set AlignCompoundAssignments: true but its doesn't work.
> 
> `AlignConsecutiveAssignments` is already gone too far on the enum, it should 
> be a struct
> 
> so rather than
> 
> ```
> enum AlignConsecutiveStyle {
> ACS_None,
> ACS_Consecutive,
> ACS_AcrossEmptyLines,
> ACS_AcrossComments,
> ACS_AcrossEmptyLinesAndComments
>   };
> AlignConsecutiveStyle  AlignConsecutiveAssignments ;
> 
> ```
> it should be perhaps
> 
> ```
> struct {
> bool AcrossEmptyLines,
> bool AcrossComments,
> bool AlignCompound
> } AlignConsecutiveStyle;
> 
> AlignConsecutiveStyle  AlignConsecutiveAssignments;
> ```
> 
> in the .clang-format file it would become
> 
> ```
> AlignConsecutiveAssignments: Custom
> AlignConsecutiveAssignmentsOptions:
>AcrossEmptyLines: true
>AcrossComments: true
>AlignCompound: false
> ```
> 
> I realise this would be a much bigger piece of work (in the config) but the 
> existing options would map to the new options, and then we have a structure 
> for which have space for future expansion.
> 
> The introduction of a dependent option in my view triggers the need for that 
> config change? @HazardyKnusperkeks 
>  you thoughts, I know we've done this before, what  do you think in this 
> case? 
> 
> 
> 
I would even go further (and that I already told the last time). Drop the 
``Custom`` and map the old enums to the struct when parsing, so no new option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D119599: Add option to align compound assignments like `+=`

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:468
+  // Widths of the aligned text.
+  // The part to the left of the anchor. For right-justified assignment
+  // operators, this includes the initial space before the sign but not

whats an anchor?



Comment at: clang/lib/Format/WhitespaceManager.cpp:580
 
-unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn;
-int LineLengthAfter = Changes[i].TokenLength;
+unsigned ChangeWidthLeft = Changes[i].StartOfTokenColumn;
+unsigned ChangeWidthAnchor = 0;

I like the final outcome, I'm uneasy about renaming all the variables, just 
because you now understand them. I'm struggling to read the algorithm in the 
same context as the prior version





Comment at: clang/unittests/Format/FormatTest.cpp:16399
+  Alignment.AlignCompoundAssignments = true;
+  verifyFormat("aa <= 5;\n"
+   "a   &= 5;\n"

curdeius wrote:
> Please test spacing in separate `verifyFormat` cases as in the Format.h 
> option description.
I sort of feel this isn't enough tests



Comment at: clang/unittests/Format/FormatTest.cpp:16852
-  // otherwise the function parameters will be re-flowed onto a single line.
-  Alignment.ColumnLimit = 0;
   EXPECT_EQ("inta(int   x,\n"

Huge `no` from me, don't change the tests because they break based on your 
change.



Comment at: clang/unittests/Format/FormatTest.cpp:19273
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
+  CHECK_PARSE_BOOL(AlignCompoundAssignments);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);

alphabetic, but see my note as you shouldn't need this?



Comment at: clang/unittests/Format/FormatTest.cpp:19277
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
+  CHECK_PARSE_BOOL(AlignTrailingComments);
   CHECK_PARSE_BOOL(BinPackArguments);

`IJKL` last L looked i came before l right? so why did you move this down


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D119599: Add option to align compound assignments like `+=`

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/include/clang/Format/Format.h:163
+  /// \endcode
+  bool AlignCompoundAssignments;
+

This option is not independent of `AlignConsecutiveAssignments` is it? this 
will cause confusion when people turn it on without turning on 
`AlignConsecutiveAssignments` 

Options have a lifecycle we have noticed

1) They start as bool
2) They become enums
3) They then end up as a struct of bool
4) Sometimes the struct becomes of enums

Whilst I like what you are doing here I fear we will get bugs where people say 
I set AlignCompoundAssignments: true but its doesn't work.

`AlignConsecutiveAssignments` is already gone too far on the enum, it should be 
a struct

so rather than

```
enum AlignConsecutiveStyle {
ACS_None,
ACS_Consecutive,
ACS_AcrossEmptyLines,
ACS_AcrossComments,
ACS_AcrossEmptyLinesAndComments
  };
AlignConsecutiveStyle  AlignConsecutiveAssignments ;

```
it should be perhaps

```
struct {
bool AcrossEmptyLines,
bool AcrossComments,
bool AlignCompound
} AlignConsecutiveStyle;

AlignConsecutiveStyle  AlignConsecutiveAssignments;
```

in the .clang-format file it would become

```
AlignConsecutiveAssignments: Custom
AlignConsecutiveAssignmentsOptions:
   AcrossEmptyLines: true
   AcrossComments: true
   AlignCompound: false
```

I realise this would be a much bigger piece of work (in the config) but the 
existing options would map to the new options, and then we have a structure for 
which have space for future expansion.

The introduction of a dependent option in my view triggers the need for that 
config change? @HazardyKnusperkeks 
 you thoughts, I know we've done this before, what  do you think in this case?  
   





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D112730: [clang-tidy] Add AUTOSAR module

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Thanks a lot @tonic for your update and for the work done in the last months ! 
It's indeed very sad to hear the news. It's a pity that the work will probably 
be duplicated in many local forks with sub-optimal solutions instead of a 
centralized, high-quality, peer-reviewed open-source solution.

I'm not very familiar with the terms so I'm not sure I fully understand the 
reasons why we are advised not to proceed. Could you clarify/elaborate on what 
it means "the exact patent burden is not disclosed"? I was hoping that the 
written consent we got from AUTOSAR would be a strong basis to move forward.


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

https://reviews.llvm.org/D112730

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

[PATCH] D119597: [clang-format][NFC] Give State.Stack.back() a meaningful name

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I like this change for clarity reasons, my only concern and it's not based on 
evidence is what if any of these functions get passed in State, and then they 
themselves alter the State.Stack?

In most cases I'd expect CurrentState to always be correct, but doesn't it only 
represent the state of the stack as it was at the beginning of the function

Isn't the point of State.Stack.back() to get the current state? if you do that 
with a variable like `CurrentState` or just have `State.getCurrentState()` I 
don't know but the later seems a good step because you could use that in even 
more places

i.e.

`unsigned NestedBlockIndent = State.Stack.back().NestedBlockIndent;`

`unsigned NestedBlockIndent = State.getCurrentState().NestedBlockIndent;`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119597

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

[PATCH] D119597: [clang-format][NFC] Give State.Stack.back() a meaningful name

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks planned changes to this revision.
HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1356
   moveStatePastScopeCloser(State);
+  // Do not use CurrentState here, since the two functions before may change 
the
+  // Stack.

curdeius wrote:
> Maybe it would be better to use a pointer instead of a reference and then you 
> could rebind it to the need `back()`?
I'm not a fan of using pointers for such things. I would've done that, if the 
remaining function was longer or we had more calls to the state.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1514
 
+  auto  = State.Stack.back();
+

owenpan wrote:
> Can you make it `const` here?
I think yeah I could. This is maybe an remainder of some of my tries to get the 
requires stuff working, that I would've changed the current state here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119597

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

[PATCH] D119138: [clang-format] Further improve support for requires expressions

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks planned changes to this revision.
HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2835-2841
+  // FIXME: We need an annotation on the paren to really know if it is a
+  // function call:
+  // ... foo() && requires ...
+  // or a declaration:
+  // void foo() && requires ...
+  // there is no distinction possible right now. We go for the latter,
+  // because it's more likely to appear in code.

Quuxplusone wrote:
> I think it's weird that your heuristic parses backward rather than forward. I 
> would think that the next token //after// the `requires` keyword tells you 
> what it is with pretty high probability:
> `requires requires` — it's a clause
> `requires identifier` — it's a clause
> `requires {` — it's an expression
> `requires (` — unclear, apply further heuristics
> 
> Or are those heuristics already present in trunk, and this PR is just dealing 
> with the "unclear" case?
That would be so much better, but I can't easily look forward. `Next` is still 
`nullptr`, until I call `nextToken()`, but then I'm already moved along.

But this got me thinking, at least for the easy stuff I can just go forward and 
don't start on the keyword in `parseRequiresClause()` and 
`parseRequiresExpression()`. The paren case is more tricky, but I will try 
something.


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

https://reviews.llvm.org/D119138

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

[PATCH] D119599: Add option to align compound assignments like `+=`

[Marek Kurdej via Phabricator via cfe-commits]

curdeius requested changes to this revision.
curdeius added a comment.
This revision now requires changes to proceed.

Thanks for working on this!
Looks pretty good already.




Comment at: clang/include/clang/Format/Format.h:151
 
+  /// When aligning assignments, whether compound assignments like
+  /// ``+=``'s are aligned along with ``=``'s.

You need to update the RST files when updating the doc comments here. Please 
use 
https://github.com/llvm/llvm-project/blob/main/clang/docs/tools/dump_format_style.py
 for that.



Comment at: clang/include/clang/Format/Format.h:157
+  ///   a   &= 2;
+  ///   bbb  = 2;
+  ///

I guess it would be complicated to avoid adding an additional space here. I 
mean, it could be:
```
a  &= 2;
bbb = 2;
```
And with 3-char operators, there's one more space.



Comment at: clang/unittests/Format/FormatTest.cpp:16399
+  Alignment.AlignCompoundAssignments = true;
+  verifyFormat("aa <= 5;\n"
+   "a   &= 5;\n"

Please test spacing in separate `verifyFormat` cases as in the Format.h option 
description.



Comment at: clang/unittests/Format/FormatTest.cpp:16408
+   "dvsdsv  |= 5;\n"
+   "int dsvvdvsdvvv  = 123;",
+   Alignment);

Please test shift operators `<<=` and `>>=`.



Comment at: clang/unittests/Format/FormatTest.cpp:16872
+  // that things still get aligned.
+  Alignment.ColumnLimit = 20;
   EXPECT_EQ("inta(int   x,\n"

Is it something that can be done/fixed separately?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119599

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

[PATCH] D119597: [clang-format][NFC] Give State.Stack.back() a meaningful name

[Marek Kurdej via Phabricator via cfe-commits]

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

LGTM whether you keep it as is or use a pointer.
It looks like it would be cool if we had some sort of expression aliases in the 
language... `using CurrentState = State.Stack.back()`... One can always 
dream... :)




Comment at: clang/lib/Format/ContinuationIndenter.cpp:1356
   moveStatePastScopeCloser(State);
+  // Do not use CurrentState here, since the two functions before may change 
the
+  // Stack.

Maybe it would be better to use a pointer instead of a reference and then you 
could rebind it to the need `back()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119597

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

[PATCH] D119597: [clang-format][NFC] Give State.Stack.back() a meaningful name

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1514
 
+  auto  = State.Stack.back();
+

Can you make it `const` here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119597

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

[PATCH] D112663: [clang-repl] Allow Interpreter::getSymbolAddress to take a mangled name.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev closed this revision.
v.g.vassilev added a comment.

That change landed in 4fb0805c6525b13e50067b9ddfe8677a0b7b2d7c 



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

https://reviews.llvm.org/D112663

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

[PATCH] D119615: [CUDA][HIP] Do not promote constexpr var with non-constant initializer

[Artem Belevich via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaCUDA.cpp:148-150
+  if ((Var->isConstexpr() || Var->getType().isConstQualified()) &&
+  Var->hasAttr() &&
   !hasExplicitAttr(Var))

So the idea here is that the constexpr vars that have an initializer that may 
not be static enough for CUDA would not get the implicit __const__ attribute. 
I.e. it's tied to the change below that checks for allowed initializers.
We could use a comment about that here.


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

https://reviews.llvm.org/D119615

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