[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 189657.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.

Improve diagnostic message.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729

Files:
  include/clang/AST/ExprObjC.h
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGObjC.cpp
  lib/Sema/SemaExprObjC.cpp
  test/CodeGenObjC/boxing.m
  test/SemaObjC/boxing-illegal.m
  test/SemaObjC/objc-literal-sig.m
  test/SemaObjC/transfer-boxed-string-nullability.m
  test/SemaObjCXX/literals.mm

Index: test/SemaObjCXX/literals.mm
===
--- test/SemaObjCXX/literals.mm
+++ test/SemaObjCXX/literals.mm
@@ -50,6 +50,9 @@
 + (id)dictionaryWithObjects:(const id [])objects forKeys:(const id [])keys count:(unsigned long)cnt;
 @end
 
+@interface NSString
+@end
+
 template
 struct ConvertibleTo {
   operator T();
@@ -185,3 +188,8 @@
 void test_dictionary_colon() {
   id dict = @{ key : value };
 }
+
+void testConstExpr() {
+  constexpr NSString *t0 = @"abc";
+  constexpr NSString *t1 = @("abc");
+}
Index: test/SemaObjC/transfer-boxed-string-nullability.m
===
--- test/SemaObjC/transfer-boxed-string-nullability.m
+++ test/SemaObjC/transfer-boxed-string-nullability.m
@@ -16,13 +16,23 @@
 void takesNonNull(NSString * _Nonnull ptr);
 
 void testBoxedString() {
+  // No diagnostic emitted as this doesn't need a stringWithUTF8String message
+  // send.
+  takesNonNull(@("hey"));
+  takesNonNull(@(u8"hey"));
+
+  // If the string isn't a valid UTF-8 string, a diagnostic is emitted since the
+  // boxed expression turns into a message send.
+  takesNonNull(@(u8"\xFF")); // expected-warning {{string is ill-formed as UTF-8}}
+  takesNonNull(@(u8"\xC0\x80")); // expected-warning {{string is ill-formed as UTF-8}}
+
   const char *str = "hey";
   takesNonNull([NSString stringWithUTF8String:str]);
   takesNonNull(@(str));
 #ifndef NOWARN
-  // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
-  // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
-#else
-  // expected-no-diagnostics
+  // expected-warning@-7 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-7 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-5 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-5 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
 #endif
 }
Index: test/SemaObjC/objc-literal-sig.m
===
--- test/SemaObjC/objc-literal-sig.m
+++ test/SemaObjC/objc-literal-sig.m
@@ -39,6 +39,8 @@
 
 // All tests are doubled to make sure that a bad method is not saved
 // and then used un-checked.
+const char *getStr(void);
+
 void test_sig() {
   (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
   (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
@@ -46,6 +48,6 @@
   id array2 = @[ @17 ]; // expected-error{{literal construction method 'arrayWithObjects:count:' has incompatible signature}}
   id dict = @{ @"hello" : @17 }; // expected-error{{literal construction method 'dictionaryWithObjects:forKeys:count:' has incompatible signature}}
   id dict2 = @{ @"hello" : @17 }; // expected-error{{literal construction method 'dictionaryWithObjects:forKeys:count:' has incompatible signature}}
-  id str = @("hello"); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
-  id str2 = @("hello"); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
+  id str = @(getStr()); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
+  id str2 = @(getStr()); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
 }
Index: test/SemaObjC/boxing-illegal.m
===
--- test/SemaObjC/boxing-illegal.m
+++ test/SemaObjC/boxing-illegal.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wattributes %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wattributes -fpascal-strings %s
 
 typedef long NSInteger;
 typedef unsigned long NSUInteger;
@@ 

[PATCH] D59013: [CMake][runtimes] Move libunwind, libc++abi and libc++ to lib/ and include/

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 189656.
phosek marked an inline comment as done.

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

https://reviews.llvm.org/D59013

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/linux-per-target-runtime-dir.c
  libcxx/CMakeLists.txt
  libcxx/lib/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libcxxabi/src/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  llvm/runtimes/CMakeLists.txt

Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -124,8 +124,8 @@
 string(REPLACE "-" "_" canon_name ${projName})
 string(TOUPPER ${canon_name} canon_name)
 
-if(LLVM_RUNTIMES_LIBDIR_SUFFIX)
-  set(${canon_name}_LIBDIR_SUFFIX "${LLVM_RUNTIMES_LIBDIR_SUFFIX}" CACHE STRING "" FORCE)
+if(LLVM_RUNTIMES_LIBDIR_SUBDIR)
+  set(${canon_name}_LIBDIR_SUBDIR "${LLVM_RUNTIMES_LIBDIR_SUBDIR}" CACHE STRING "" FORCE)
 endif()
 
 # Setting a variable to let sub-projects detect which other projects
@@ -349,6 +349,7 @@
  # Builtins were built separately above
  CMAKE_ARGS -DCOMPILER_RT_BUILD_BUILTINS=Off
 -DLLVM_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
+-DLLVM_BINARY_DIR=${LLVM_BINARY_DIR}
 -DLLVM_LIBRARY_DIR=${LLVM_LIBRARY_DIR}
 -DLLVM_DEFAULT_TARGET_TRIPLE=${TARGET_TRIPLE}
 -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON
@@ -432,6 +433,7 @@
  # Builtins were built separately above
  CMAKE_ARGS -DCOMPILER_RT_BUILD_BUILTINS=Off
 -DLLVM_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
+-DLLVM_BINARY_DIR=${LLVM_BINARY_DIR}
 -DLLVM_LIBRARY_DIR=${LLVM_LIBRARY_DIR}
 -DLLVM_DEFAULT_TARGET_TRIPLE=${target}
 -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON
@@ -514,8 +516,7 @@
   runtime_register_target(${name}-${sanitizer_name} ${name}
 DEPENDS runtimes-${name}
 CMAKE_ARGS -DLLVM_USE_SANITIZER=${sanitizer}
-   -DLLVM_RUNTIMES_PREFIX=${name}/
-   -DLLVM_RUNTIMES_LIBDIR_SUFFIX=/${sanitizer_name})
+   -DLLVM_RUNTIMES_LIBDIR_SUBDIR=${sanitizer_name})
   add_dependencies(runtimes runtimes-${name}-${sanitizer_name})
   add_dependencies(runtimes-configure runtimes-${name}-${sanitizer_name}-configure)
   add_dependencies(install-runtimes install-runtimes-${name}-${sanitizer_name})
Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -178,8 +178,8 @@
 
 if (LIBUNWIND_INSTALL_LIBRARY)
   install(TARGETS ${LIBUNWIND_INSTALL_TARGETS}
-LIBRARY DESTINATION ${LIBUNWIND_INSTALL_PREFIX}lib${LIBUNWIND_LIBDIR_SUFFIX} COMPONENT unwind
-ARCHIVE DESTINATION ${LIBUNWIND_INSTALL_PREFIX}lib${LIBUNWIND_LIBDIR_SUFFIX} COMPONENT unwind)
+LIBRARY DESTINATION ${LIBUNWIND_INSTALL_PREFIX}${LIBUNWIND_INSTALL_LIBRARY_DIR} COMPONENT unwind
+ARCHIVE DESTINATION ${LIBUNWIND_INSTALL_PREFIX}${LIBUNWIND_INSTALL_LIBRARY_DIR} COMPONENT unwind)
 endif()
 
 if (NOT CMAKE_CONFIGURATION_TYPES AND LIBUNWIND_INSTALL_LIBRARY)
Index: libunwind/CMakeLists.txt
===
--- libunwind/CMakeLists.txt
+++ libunwind/CMakeLists.txt
@@ -187,12 +187,18 @@
${PACKAGE_VERSION})
 
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
-  set(DEFAULT_INSTALL_PREFIX lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
-  set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}${LIBUNWIND_LIBDIR_SUBDIR})
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE})
+  if(LIBCXX_LIBDIR_SUBDIR)
+set(LIBUNWIND_LIBRARY_DIR ${LIBUNWIND_LIBRARY_DIR}/${LIBUNWIND_LIBDIR_SUBDIR})
+set(LIBUNWIND_INSTALL_LIBRARY_DIR ${LIBUNWIND_INSTALL_LIBRARY_DIR}/${LIBUNWIND_LIBDIR_SUBDIR})
+  endif()
 elseif(LLVM_LIBRARY_OUTPUT_INTDIR)
   set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LIBUNWIND_LIBDIR_SUFFIX})
 else()
   set(LIBUNWIND_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_INSTALL_LIBRARY_DIR lib${LIBUNWIND_LIBDIR_SU

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Brian Gesiak via Phabricator via cfe-commits]

modocache added inline comments.



Comment at: lib/Sema/SemaCoroutine.cpp:675
+  // Second emphasis of [expr.await]p2: must be outside of an exception 
handler.
+  if (S.getCurScope()->getFlags() & Scope::CatchScope) {
+S.Diag(Loc, diag::err_coroutine_within_handler) << Keyword;

EricWF wrote:
> We can still build a valid AST after encountering this error, no?
> 
> 
I believe so. Just to be clear: you'd like me to continue building the AST even 
after emitting this error diagnostic? My understanding is that most of this 
file bails soon after any error is encountered (correct me if that's wrong). 
I'm happy to change that, but I wonder if it'd be better to do that in a 
separate diff...?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59076



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

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: lib/Sema/SemaCoroutine.cpp:675
+  // Second emphasis of [expr.await]p2: must be outside of an exception 
handler.
+  if (S.getCurScope()->getFlags() & Scope::CatchScope) {
+S.Diag(Loc, diag::err_coroutine_within_handler) << Keyword;

We can still build a valid AST after encountering this error, no?




Repository:
  rC Clang

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

https://reviews.llvm.org/D59076



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

[PATCH] D59013: [CMake][runtimes] Move libunwind, libc++abi and libc++ to lib/ and include/

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 189654.
phosek marked an inline comment as done.

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

https://reviews.llvm.org/D59013

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/linux-per-target-runtime-dir.c
  libcxx/CMakeLists.txt
  libcxx/lib/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libcxxabi/src/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  llvm/runtimes/CMakeLists.txt

Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -124,8 +124,8 @@
 string(REPLACE "-" "_" canon_name ${projName})
 string(TOUPPER ${canon_name} canon_name)
 
-if(LLVM_RUNTIMES_LIBDIR_SUFFIX)
-  set(${canon_name}_LIBDIR_SUFFIX "${LLVM_RUNTIMES_LIBDIR_SUFFIX}" CACHE STRING "" FORCE)
+if(LLVM_RUNTIMES_LIBDIR_SUBDIR)
+  set(${canon_name}_LIBDIR_SUBDIR "${LLVM_RUNTIMES_LIBDIR_SUBDIR}" CACHE STRING "" FORCE)
 endif()
 
 # Setting a variable to let sub-projects detect which other projects
@@ -349,6 +349,7 @@
  # Builtins were built separately above
  CMAKE_ARGS -DCOMPILER_RT_BUILD_BUILTINS=Off
 -DLLVM_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
+-DLLVM_BINARY_DIR=${LLVM_BINARY_DIR}
 -DLLVM_LIBRARY_DIR=${LLVM_LIBRARY_DIR}
 -DLLVM_DEFAULT_TARGET_TRIPLE=${TARGET_TRIPLE}
 -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON
@@ -432,6 +433,7 @@
  # Builtins were built separately above
  CMAKE_ARGS -DCOMPILER_RT_BUILD_BUILTINS=Off
 -DLLVM_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
+-DLLVM_BINARY_DIR=${LLVM_BINARY_DIR}
 -DLLVM_LIBRARY_DIR=${LLVM_LIBRARY_DIR}
 -DLLVM_DEFAULT_TARGET_TRIPLE=${target}
 -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON
@@ -515,7 +517,7 @@
 DEPENDS runtimes-${name}
 CMAKE_ARGS -DLLVM_USE_SANITIZER=${sanitizer}
-DLLVM_RUNTIMES_PREFIX=${name}/
-   -DLLVM_RUNTIMES_LIBDIR_SUFFIX=/${sanitizer_name})
+   -DLLVM_RUNTIMES_LIBDIR_SUBDIR=/${sanitizer_name})
   add_dependencies(runtimes runtimes-${name}-${sanitizer_name})
   add_dependencies(runtimes-configure runtimes-${name}-${sanitizer_name}-configure)
   add_dependencies(install-runtimes install-runtimes-${name}-${sanitizer_name})
Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -178,8 +178,8 @@
 
 if (LIBUNWIND_INSTALL_LIBRARY)
   install(TARGETS ${LIBUNWIND_INSTALL_TARGETS}
-LIBRARY DESTINATION ${LIBUNWIND_INSTALL_PREFIX}lib${LIBUNWIND_LIBDIR_SUFFIX} COMPONENT unwind
-ARCHIVE DESTINATION ${LIBUNWIND_INSTALL_PREFIX}lib${LIBUNWIND_LIBDIR_SUFFIX} COMPONENT unwind)
+LIBRARY DESTINATION ${LIBUNWIND_INSTALL_PREFIX}${LIBUNWIND_INSTALL_LIBDIR} COMPONENT unwind
+ARCHIVE DESTINATION ${LIBUNWIND_INSTALL_PREFIX}${LIBUNWIND_INSTALL_LIBDIR} COMPONENT unwind)
 endif()
 
 if (NOT CMAKE_CONFIGURATION_TYPES AND LIBUNWIND_INSTALL_LIBRARY)
Index: libunwind/CMakeLists.txt
===
--- libunwind/CMakeLists.txt
+++ libunwind/CMakeLists.txt
@@ -187,12 +187,15 @@
${PACKAGE_VERSION})
 
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
-  set(DEFAULT_INSTALL_PREFIX lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
-  set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(DEFAULT_INSTALL_PREFIX lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
+  set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}${LIBUNWIND_LIBDIR_SUBDIR})
+  set(LIBUNWIND_INSTALL_LIBDIR ${LIBUNWIND_LIBDIR_SUBDIR})
 elseif(LLVM_LIBRARY_OUTPUT_INTDIR)
   set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
+  set(LIBUNWIND_INSTALL_LIBDIR lib${LIBUNWIND_LIBDIR_SUFFIX})
 else()
   set(LIBUNWIND_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_INSTALL_LIBDIR lib${LIBUNWIND_LIBDIR_SUFFIX})
 endif()
 
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIBUNWIND_LIBRARY_DIR})
Index: libcxxabi/src/CMakeLists.txt
===
--- libcxxabi/src/CMakeLists.txt
+++ libcxxabi/src/CMakeLists.txt
@@ -252

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Brian Gesiak via Phabricator via cfe-commits]

modocache created this revision.
modocache added reviewers: GorNishanov, tks2103, rsmith.
Herald added subscribers: jdoerfert, EricWF.
Herald added a project: clang.

As reported in https://bugs.llvm.org/show_bug.cgi?id=40978, it's an
error to use the `co_yield` or `co_await` keywords outside of a valid
"suspension context" as defined by [expr.await]p2 of
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/n4775.pdf.

Whether or not the current scope was in a function-try-block's
(https://en.cppreference.com/w/cpp/language/function-try-block) handler
could be determined using scope flag `Scope::FnTryCatchScope`. No
such flag existed for a simple C++ catch statement, so this commit adds
one.


Repository:
  rC Clang

https://reviews.llvm.org/D59076

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Scope.h
  lib/Parse/ParseStmt.cpp
  lib/Sema/SemaCoroutine.cpp
  test/SemaCXX/coroutines.cpp

Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -328,6 +328,27 @@
 // not allowed. A user may not understand that this is "outside a function."
 void default_argument(int arg = co_await 0) {} // expected-error {{'co_await' cannot be used outside a function}}
 
+void await_in_catch_coroutine() {
+  try {
+  } catch (...) {
+co_await a; // expected-error {{'co_await' cannot be used in the handler of a try block}}
+  }
+}
+
+void yield_in_catch_coroutine() {
+  try {
+  } catch (...) {
+co_yield a; // expected-error {{'co_yield' cannot be used in the handler of a try block}}
+  }
+}
+
+void return_in_catch_coroutine() {
+  try {
+  } catch (...) {
+co_return 123; // OK
+  }
+}
+
 constexpr auto constexpr_deduced_return_coroutine() {
   co_yield 0; // expected-error {{'co_yield' cannot be used in a constexpr function}}
   // expected-error@-1 {{'co_yield' cannot be used in a function with a deduced return type}}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -185,21 +185,8 @@
 
 static bool isValidCoroutineContext(Sema &S, SourceLocation Loc,
 StringRef Keyword) {
-  // 'co_await' and 'co_yield' are not permitted in unevaluated operands,
-  // such as subexpressions of \c sizeof.
-  //
-  // [expr.await]p2, emphasis added: "An await-expression shall appear only in
-  // a *potentially evaluated* expression within the compound-statement of a
-  // function-body outside of a handler [...] A context within a function where
-  // an await-expression can appear is called a suspension context of the
-  // function." And per [expr.yield]p1: "A yield-expression shall appear only
-  // within a suspension context of a function."
-  if (S.isUnevaluatedContext()) {
-S.Diag(Loc, diag::err_coroutine_unevaluated_context) << Keyword;
-return false;
-  }
-
-  // Per [expr.await]p2, any other usage must be within a function.
+  // [expr.await]p2 dictates that 'co_await' and 'co_yield' must be used within
+  // a function body.
   // FIXME: This also covers [expr.await]p2: "An await-expression shall not
   // appear in a default argument." But the diagnostic QoI here could be
   // improved to inform the user that default arguments specifically are not
@@ -668,8 +655,34 @@
   return true;
 }
 
+// [expr.await]p2, emphasis added: "An await-expression shall appear only in
+// a *potentially evaluated* expression within the compound-statement of a
+// function-body *outside of a handler* [...] A context within a function
+// where an await-expression can appear is called a suspension context of the
+// function."
+static bool isValidSuspensionContext(Sema &S, SourceLocation Loc,
+ StringRef Keyword,
+ bool IsImplicit = false) {
+  // First emphasis of [expr.await]p2: must be a potentially evaluated context.
+  // That is, 'co_await' and 'co_yield' cannot appear in subexpressions of
+  // \c sizeof.
+  if (S.isUnevaluatedContext()) {
+S.Diag(Loc, diag::err_coroutine_unevaluated_context) << Keyword;
+return false;
+  }
+
+  // Second emphasis of [expr.await]p2: must be outside of an exception handler.
+  if (S.getCurScope()->getFlags() & Scope::CatchScope) {
+S.Diag(Loc, diag::err_coroutine_within_handler) << Keyword;
+return false;
+  }
+
+  return true;
+}
+
 ExprResult Sema::ActOnCoawaitExpr(Scope *S, SourceLocation Loc, Expr *E) {
-  if (!ActOnCoroutineBodyStart(S, Loc, "co_await")) {
+  if (!ActOnCoroutineBodyStart(S, Loc, "co_await") ||
+  !isValidSuspensionContext(*this, Loc, "co_await")) {
 CorrectDelayedTyposInExpr(E);
 return ExprError();
   }
@@ -689,7 +702,7 @@
 ExprResult Sema::BuildUnresolvedCoawaitExpr(SourceLocation Loc, Expr *E,
 UnresolvedLookupExpr *Lookup) {
   auto *

[PATCH] D59048: Add AIX Target Info

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:626
+
+// FIXME: Define AIX OS-Version Macros
+Builder.defineMacro("_AIX");

apaprocki wrote:
> hubert.reinterpretcast wrote:
> > Comments should be full sentences (with periods). Please update throughout 
> > this patch.
> I do not think it is realistic to support an AIX target prior to 7.1, so 
> should this unconditionally define up to and including `_AIX71` similar to 
> D18360 did (it was written earlier and defined up to `_AIX61` 
> unconditionally) and leave a `FIXME` to determine when to emit `_AIX72` or 
> any other later versions?
I agree with your assessment re: older versions of AIX in terms of having a 
full solution for modern C++. Nevertheless, we can leave room open for hobby 
builds targeting older versions where the immediate cost is not high. Cross 
compiling to an older AIX target will probably be entirely possible for C.



Comment at: clang/test/Preprocessor/init.c:6420
+// PPC64-AIX:#define _LONG_LONG 1
+// PPC64-AIX:#define _POWER 1
+// PPC64-AIX:#define __64BIT__ 1

apaprocki wrote:
> XL on AIX emits `#define _LP64 1` in 64-bit mode and `#define _ILP32 1` in 
> 32-bit mode in the pre-defined macros. Is that important to capture?
I think so. The v16.1 XL compiler's `xlclang` also produces these.

```
#define __LP64__ 1
#define _LP64 1
```

```
#define __ILP32__ 1
#define _ILP32 1
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59048



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

[PATCH] D59013: [CMake][runtimes] Move libunwind, libc++abi and libc++ to lib/ and include/

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added inline comments.



Comment at: clang/test/Driver/linux-per-target-runtime-dir.c:13
 // CHECK-PER-TARGET-RUNTIME: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-PER-TARGET-RUNTIME: "-internal-isystem" "[[RESDIR]]/include/c++/v1"
 // CHECK-PER-TARGET-RUNTIME: "-internal-isystem" 
"[[SYSROOT]]/usr/local/include"

Shouldn't this one be different now?



Comment at: libcxx/CMakeLists.txt:421
+  set(DEFAULT_INSTALL_PREFIX 
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
+  set(LIBCXX_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}${LIBCXX_LIBDIR_SUBDIR})
+  set(LIBCXX_HEADER_DIR ${LLVM_BINARY_DIR})

It's expected for LIBCXX_LIBDIR_SUBDIR to always begin with a `/`, correct? I 
feel like it would be less error-prone to have places using that variable add 
the `/` rather than relying on the variable containing it. In particular, since 
the variable name contains SUBDIR, I wouldn't expect to have to specify the 
leading `/` of the subdirectory myself.

Case in point: if I'm reading correctly, `DEFAULT_INSTALL_PREFIX` will end with 
a trailing `/`, and then `LIBCXX_INSTALL_LIBDIR` will begin with a leading `/`, 
so you'll end up with two consecutive `/` when you join the two in 
lib/CMakeLists.txt. Not a big deal, but also easily avoidable, I think.


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

https://reviews.llvm.org/D59013



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

[PATCH] D59013: [CMake][runtimes] Move libunwind, libc++abi and libc++ to lib/ and include/

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 189643.
phosek marked 2 inline comments as done.

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

https://reviews.llvm.org/D59013

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/linux-per-target-runtime-dir.c
  libcxx/CMakeLists.txt
  libcxx/lib/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libcxxabi/src/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/src/CMakeLists.txt
  llvm/runtimes/CMakeLists.txt

Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -124,8 +124,8 @@
 string(REPLACE "-" "_" canon_name ${projName})
 string(TOUPPER ${canon_name} canon_name)
 
-if(LLVM_RUNTIMES_LIBDIR_SUFFIX)
-  set(${canon_name}_LIBDIR_SUFFIX "${LLVM_RUNTIMES_LIBDIR_SUFFIX}" CACHE STRING "" FORCE)
+if(LLVM_RUNTIMES_LIBDIR_SUBDIR)
+  set(${canon_name}_LIBDIR_SUBDIR "${LLVM_RUNTIMES_LIBDIR_SUBDIR}" CACHE STRING "" FORCE)
 endif()
 
 # Setting a variable to let sub-projects detect which other projects
@@ -349,6 +349,7 @@
  # Builtins were built separately above
  CMAKE_ARGS -DCOMPILER_RT_BUILD_BUILTINS=Off
 -DLLVM_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
+-DLLVM_BINARY_DIR=${LLVM_BINARY_DIR}
 -DLLVM_LIBRARY_DIR=${LLVM_LIBRARY_DIR}
 -DLLVM_DEFAULT_TARGET_TRIPLE=${TARGET_TRIPLE}
 -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON
@@ -432,6 +433,7 @@
  # Builtins were built separately above
  CMAKE_ARGS -DCOMPILER_RT_BUILD_BUILTINS=Off
 -DLLVM_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
+-DLLVM_BINARY_DIR=${LLVM_BINARY_DIR}
 -DLLVM_LIBRARY_DIR=${LLVM_LIBRARY_DIR}
 -DLLVM_DEFAULT_TARGET_TRIPLE=${target}
 -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON
@@ -515,7 +517,7 @@
 DEPENDS runtimes-${name}
 CMAKE_ARGS -DLLVM_USE_SANITIZER=${sanitizer}
-DLLVM_RUNTIMES_PREFIX=${name}/
-   -DLLVM_RUNTIMES_LIBDIR_SUFFIX=/${sanitizer_name})
+   -DLLVM_RUNTIMES_LIBDIR_SUBDIR=/${sanitizer_name})
   add_dependencies(runtimes runtimes-${name}-${sanitizer_name})
   add_dependencies(runtimes-configure runtimes-${name}-${sanitizer_name}-configure)
   add_dependencies(install-runtimes install-runtimes-${name}-${sanitizer_name})
Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -178,8 +178,8 @@
 
 if (LIBUNWIND_INSTALL_LIBRARY)
   install(TARGETS ${LIBUNWIND_INSTALL_TARGETS}
-LIBRARY DESTINATION ${LIBUNWIND_INSTALL_PREFIX}lib${LIBUNWIND_LIBDIR_SUFFIX} COMPONENT unwind
-ARCHIVE DESTINATION ${LIBUNWIND_INSTALL_PREFIX}lib${LIBUNWIND_LIBDIR_SUFFIX} COMPONENT unwind)
+LIBRARY DESTINATION ${LIBUNWIND_INSTALL_PREFIX}${LIBUNWIND_INSTALL_LIBDIR} COMPONENT unwind
+ARCHIVE DESTINATION ${LIBUNWIND_INSTALL_PREFIX}${LIBUNWIND_INSTALL_LIBDIR} COMPONENT unwind)
 endif()
 
 if (NOT CMAKE_CONFIGURATION_TYPES AND LIBUNWIND_INSTALL_LIBRARY)
Index: libunwind/CMakeLists.txt
===
--- libunwind/CMakeLists.txt
+++ libunwind/CMakeLists.txt
@@ -187,12 +187,15 @@
${PACKAGE_VERSION})
 
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
-  set(DEFAULT_INSTALL_PREFIX lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
-  set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(DEFAULT_INSTALL_PREFIX lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
+  set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}${LIBUNWIND_LIBDIR_SUBDIR})
+  set(LIBUNWIND_INSTALL_LIBDIR ${LIBUNWIND_LIBDIR_SUBDIR})
 elseif(LLVM_LIBRARY_OUTPUT_INTDIR)
   set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
+  set(LIBUNWIND_INSTALL_LIBDIR lib${LIBUNWIND_LIBDIR_SUFFIX})
 else()
   set(LIBUNWIND_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBUNWIND_LIBDIR_SUFFIX})
+  set(LIBUNWIND_INSTALL_LIBDIR lib${LIBUNWIND_LIBDIR_SUFFIX})
 endif()
 
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIBUNWIND_LIBRARY_DIR})
Index: libcxxabi/src/CMakeLists.txt
===
--- libcxxabi/src/CMakeLists.txt
+++ libcxxabi/src/CMakeLists.txt
@@ -252,8 +252,8 @@
 
 if (LIBCXXABI_INSTALL_LI

[PATCH] D58719: [PR40778][Sema] Adjust address space of operands in builtin operators

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Looks great, thanks!


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

https://reviews.llvm.org/D58719



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

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/Basic/DiagnosticGroups.td:322
 def InvalidSourceEncoding : DiagGroup<"invalid-source-encoding">;
+def InvalidUTF8String : DiagGroup<"invalid-utf8-string">;
 def KNRPromotedParameter : DiagGroup<"knr-promoted-parameter">;

I think this warning group should mention that this is specific to ObjC boxing 
somehow.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5807
   "Clang encodes unprefixed narrow string literals as UTF-8">;
+def warn_invalid_utf8_string : Warning<"invalid UTF-8 string">, 
InGroup;
 def err_array_init_different_type : Error<

Maybe "string is ill-formed as UTF-8 and will become a null NSString* when 
boxed", where the type is whatever the type of the box-expression would be?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729



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

[PATCH] D58665: [analyzer] Handle comparison between non-default AS symbol and constant

[Artem Dergachev via Phabricator via cfe-commits]

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

Thanks again! Please commit.


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

https://reviews.llvm.org/D58665



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

[PATCH] D59048: Add AIX Target Info

[Andrew Paprocki via Phabricator via cfe-commits]

apaprocki added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:626
+
+// FIXME: Define AIX OS-Version Macros
+Builder.defineMacro("_AIX");

hubert.reinterpretcast wrote:
> Comments should be full sentences (with periods). Please update throughout 
> this patch.
I do not think it is realistic to support an AIX target prior to 7.1, so should 
this unconditionally define up to and including `_AIX71` similar to D18360 did 
(it was written earlier and defined up to `_AIX61` unconditionally) and leave a 
`FIXME` to determine when to emit `_AIX72` or any other later versions?



Comment at: clang/test/Preprocessor/init.c:6420
+// PPC64-AIX:#define _LONG_LONG 1
+// PPC64-AIX:#define _POWER 1
+// PPC64-AIX:#define __64BIT__ 1

XL on AIX emits `#define _LP64 1` in 64-bit mode and `#define _ILP32 1` in 
32-bit mode in the pre-defined macros. Is that important to capture?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59048



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

[PATCH] D53754: [Analyzer] Skip symbolic regions based on conjured symbols in comparison of the containers of iterators

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: Charusso.

In D53754#1401162 , 
@baloghadamsoftware wrote:

> I think that here the main difference is that if we analyze this function as 
> top level, then we find a true positive: the regions for `v1` and `v2` //may 
> be// the same but generally they are difference (hence the different 
> parameters).


Aha, right, that's an interesting heuristic. I guess that the developer may 
also add a specific check (eg., `if (&v1 == &v2) ...`, but that's a separate 
story of aliasing and renaming, and i do admit that i don't see this sort of 
code being written sensibly.

Well, you can't really rely on my imagination, because i can still say the same 
about the `SymbolConjured` examples. I'm really curious how did this originally 
look, i.e. even if the user knows that a certain function always returns the 
same container, why would they call it twice? Was this happening in some sort 
of loop? Is there a more realistic test case that we can add?

Anyway, let's add a huge comment that explains why `SymbolConjured`s are 
special and commit. I mean, this definitely deserves a comment :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D53754



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

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 189630.
ahatanak added a comment.

Diagnose invalid UTF-8 strings in boxed expressions.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729

Files:
  include/clang/AST/ExprObjC.h
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGObjC.cpp
  lib/Sema/SemaExprObjC.cpp
  test/CodeGenObjC/boxing.m
  test/SemaObjC/boxing-illegal.m
  test/SemaObjC/objc-literal-sig.m
  test/SemaObjC/transfer-boxed-string-nullability.m
  test/SemaObjCXX/literals.mm

Index: test/SemaObjCXX/literals.mm
===
--- test/SemaObjCXX/literals.mm
+++ test/SemaObjCXX/literals.mm
@@ -50,6 +50,9 @@
 + (id)dictionaryWithObjects:(const id [])objects forKeys:(const id [])keys count:(unsigned long)cnt;
 @end
 
+@interface NSString
+@end
+
 template
 struct ConvertibleTo {
   operator T();
@@ -185,3 +188,8 @@
 void test_dictionary_colon() {
   id dict = @{ key : value };
 }
+
+void testConstExpr() {
+  constexpr NSString *t0 = @"abc";
+  constexpr NSString *t1 = @("abc");
+}
Index: test/SemaObjC/transfer-boxed-string-nullability.m
===
--- test/SemaObjC/transfer-boxed-string-nullability.m
+++ test/SemaObjC/transfer-boxed-string-nullability.m
@@ -16,13 +16,23 @@
 void takesNonNull(NSString * _Nonnull ptr);
 
 void testBoxedString() {
+  // No diagnostic emitted as this doesn't need a stringWithUTF8String message
+  // send.
+  takesNonNull(@("hey"));
+  takesNonNull(@(u8"hey"));
+
+  // If the string isn't a valid UTF-8 string, a diagnostic is emitted since the
+  // boxed expression turns into a message send.
+  takesNonNull(@(u8"\xFF")); // expected-warning {{invalid UTF-8 string}}
+  takesNonNull(@(u8"\xC0\x80")); // expected-warning {{invalid UTF-8 string}}
+
   const char *str = "hey";
   takesNonNull([NSString stringWithUTF8String:str]);
   takesNonNull(@(str));
 #ifndef NOWARN
-  // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
-  // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
-#else
-  // expected-no-diagnostics
+  // expected-warning@-7 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-7 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-5 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-5 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
 #endif
 }
Index: test/SemaObjC/objc-literal-sig.m
===
--- test/SemaObjC/objc-literal-sig.m
+++ test/SemaObjC/objc-literal-sig.m
@@ -39,6 +39,8 @@
 
 // All tests are doubled to make sure that a bad method is not saved
 // and then used un-checked.
+const char *getStr(void);
+
 void test_sig() {
   (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
   (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
@@ -46,6 +48,6 @@
   id array2 = @[ @17 ]; // expected-error{{literal construction method 'arrayWithObjects:count:' has incompatible signature}}
   id dict = @{ @"hello" : @17 }; // expected-error{{literal construction method 'dictionaryWithObjects:forKeys:count:' has incompatible signature}}
   id dict2 = @{ @"hello" : @17 }; // expected-error{{literal construction method 'dictionaryWithObjects:forKeys:count:' has incompatible signature}}
-  id str = @("hello"); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
-  id str2 = @("hello"); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
+  id str = @(getStr()); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
+  id str2 = @(getStr()); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
 }
Index: test/SemaObjC/boxing-illegal.m
===
--- test/SemaObjC/boxing-illegal.m
+++ test/SemaObjC/boxing-illegal.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wattributes %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wattributes -fpascal-strings %s
 
 typedef long NSInteger;
 typedef unsigned long NSUInteger;
@@ -57,6 +57,19 @@
   box = @(*(enum Fo

[PATCH] D58121: [analyzer][WIP] Attempt to fix traversing bindings of non-base regions in ClusterAnalysis

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: Charusso.

Ugh, it takes forever to debug, because it gets reduced to a true positive now, 
and the original unprunned report is ~1300 pieces long (the normal report being 
completely worthless) and the exploded graph is too slow to view. While i find 
myself parsing dot files with regular expressions, i'll be totally happy if you 
simply add the test and commit, but i'll definitely keep trying to understand 
what's going on, because it's still a problem down there.


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

https://reviews.llvm.org/D58121



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

[PATCH] D58530: Add PragmaHandler for MSVC pragma execution_character_set

[Matt Gardner via Phabricator via cfe-commits]

sigatrev updated this revision to Diff 189629.
sigatrev added a comment.

Added tests, and slightly improved associated messaging.


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

https://reviews.llvm.org/D58530

Files:
  clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
  clang-tools-extra/pp-trace/PPCallbacksTracker.h
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/PPCallbacks.h
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/lib/Lex/Pragma.cpp
  clang/test/Preprocessor/pragma_microsoft.c

Index: clang/test/Preprocessor/pragma_microsoft.c
===
--- clang/test/Preprocessor/pragma_microsoft.c
+++ clang/test/Preprocessor/pragma_microsoft.c
@@ -198,3 +198,21 @@
 #pragma optimize("g", // expected-warning{{missing argument to '#pragma optimize'; expected 'on' or 'off'}}
 #pragma optimize("g",xyz  // expected-warning{{unexpected argument 'xyz' to '#pragma optimize'; expected 'on' or 'off'}}
 #pragma optimize("g",on)  // expected-warning{{#pragma optimize' is not supported}}
+
+#pragma execution_character_set // expected-warning {{expected '('}}
+#pragma execution_character_set(// expected-warning {{expected 'push' or 'pop'}}
+#pragma execution_character_set()   // expected-warning {{expected 'push' or 'pop'}}
+#pragma execution_character_set(asdf// expected-warning {{expected 'push' or 'pop'}}
+#pragma execution_character_set(asdf)   // expected-warning {{expected 'push' or 'pop'}}
+#pragma execution_character_set(push// expected-warning {{expected ')'}}
+#pragma execution_character_set(pop,)   // expected-warning {{expected ')'}}
+#pragma execution_character_set(pop,"asdf") // expected-warning {{expected ')'}}
+#pragma execution_character_set(push,   // expected-error {{expected string literal}}
+#pragma execution_character_set(push,)  // expected-error {{expected string literal}}
+#pragma execution_character_set(push,asdf)  // expected-error {{expected string literal}}
+#pragma execution_character_set(push, "asdf")   // expected-warning {{only 'UTF-8' is supported}}
+
+#pragma execution_character_set(push)
+#pragma execution_character_set(push, "utf-8")
+#pragma execution_character_set(push, "UTF-8")
+#pragma execution_character_set(pop)
\ No newline at end of file
Index: clang/lib/Lex/Pragma.cpp
===
--- clang/lib/Lex/Pragma.cpp
+++ clang/lib/Lex/Pragma.cpp
@@ -1368,6 +1368,70 @@
   }
 };
 
+/// "\#pragma execution_character_set(...)". MSVC supports this pragma only
+/// for "UTF-8". We parse it and ignore it if UTF-8 is provided and warn
+/// otherwise to avoid -Wunknown-pragma warnings.
+struct PragmaExecCharsetHandler : public PragmaHandler {
+  PragmaExecCharsetHandler() : PragmaHandler("execution_character_set") {}
+
+  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+Token &Tok) override {
+// Parse things like:
+// execution_character_set(push, "UTF-8")
+// execution_character_set(pop)
+SourceLocation DiagLoc = Tok.getLocation();
+PPCallbacks *Callbacks = PP.getPPCallbacks();
+
+PP.Lex(Tok);
+if (Tok.isNot(tok::l_paren)) {
+  PP.Diag(Tok, diag::warn_pragma_exec_charset_expected) << "(";
+  return;
+}
+
+PP.Lex(Tok);
+IdentifierInfo *II = Tok.getIdentifierInfo();
+
+if (II && II->isStr("push")) {
+  // #pragma execution_character_set( push[ , string ] )
+  PP.Lex(Tok);
+  if (Tok.is(tok::comma)) {
+PP.Lex(Tok);
+
+std::string ExecCharset;
+if (!PP.FinishLexStringLiteral(Tok, ExecCharset,
+   "pragma execution_character_set",
+   /*MacroExpansion=*/false))
+  return;
+
+// MSVC supports either of these, but nothing else.
+if (ExecCharset != "UTF-8" && ExecCharset != "utf-8") {
+  PP.Diag(Tok, diag::warn_pragma_exec_charset_push_invalid) << ExecCharset;
+  return;
+}
+  }
+  if (Callbacks)
+Callbacks->PragmaExecCharsetPush(DiagLoc, "UTF-8");
+} else if (II && II->isStr("pop")) {
+  // #pragma execution_character_set( pop )
+  PP.Lex(Tok);
+  if (Callbacks)
+Callbacks->PragmaExecCharsetPop(DiagLoc);
+} else {
+  PP.Diag(Tok, diag::warn_pragma_exec_charset_spec_invalid);
+  return;
+}
+
+if (Tok.isNot(tok::r_paren)) {
+  PP.Diag(Tok, diag::warn_pragma_exec_charset_expected) << ")";
+  return;
+}
+
+PP.Lex(Tok);
+if (Tok.isNot(tok::eod))
+  PP.Diag(Tok, diag::ext_pp_extra_tokens_at_eol) << "pragma execution_character_set";
+  }
+};
+
 /// PragmaIncludeAliasHandler - "\#pragma include_alias("...")".
 struct PragmaIncludeAliasHandler : public PragmaHandler {
   PragmaIncludeAliasHandler() : Prag

[PATCH] D58154: Add support for -fpermissive.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith abandoned this revision.
rsmith added a comment.

In D58154#1420565 , @aaron.ballman 
wrote:

> If we go with a different name for the flag, then the user has to update 
> their build scripts to get code to compile with Clang, which means it 
> shouldn't be too onerous for them to spell out the specific diagnostics they 
> need disabled (and it sort of forces them into somewhat better code hygiene 
> by not disabling all diagnostics). I'm kind of leaning towards not providing 
> a flag at all.


I would certainly prefer that people explicitly list the `-Wno-whatever` flags 
they're relying on when porting to Clang. There might still be some benefit in 
`-Wno-error=everything`, but it seems questionable (as James points out, users 
who don't enjoy build spam would likely reach for `-Wno-everything` instead).

In any case, I think we have consensus that this patch is the wrong direction. 
Abandoning.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58154



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

[PATCH] D58320: [Darwin] Introduce a new flag, -fapple-link-rtlib that forces linking of the builtins library.

[Ahmed Bougacha via Phabricator via cfe-commits]

ab added a comment.

Please check the embedded thing (the other comments are minor).  Otherwise, 
LGTM; if you have found this flag to be necessary, this looks like a reasonable 
way to implement it




Comment at: clang/include/clang/Driver/Options.td:1252
+def fapple_link_rtlib : Flag<["-"], "fapple-link-rtlib">, Group,
+  HelpText<"Apple specific option to force linking the clang builtins runtime 
library">;
 def flto_EQ : Joined<["-"], "flto=">, Flags<[CoreOption, CC1Option]>, 
Group,

Hmm, maybe move "Apple specific" to a parenthesis at the end?

Also, I'm guessing this belonged here when it was "-flink-rtlib", it doesn't 
anymore.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:573
+  bool NoStdOrDefaultLibs =
+  Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs);
+  bool ForceLinkBuiltins = Args.hasArg(options::OPT_fapple_link_rtlib);

Maybe sink this to the AddLinkRuntimeLibArgs implementations (as an extra 
parameter or directly checking the args)?  The flags don't bypass the whole 
thing anymore, might as well treat it like -mkernel and whatnot.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2084
 
   AddLinkRuntimeLib(Args, CmdArgs, CompilerRT, RLO_IsEmbedded);
 }

This is different from 'builtins'.  Are you OK with the difference?  Otherwise 
maybe this should be an error for now;  I wouldn't be surprised if we never hit 
this path (this part, I'm not familiar with)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58320



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

> which is making things hard for the folks testing this with the Linux kernel 
> and needing to apply this patch themselves.

If we can get inlining of callbr working first before landing this, then all of 
our CI won't go immediately red (as it would from landing this first, then 
getting inlining support).  Otherwise, we'll have to come up with hacks in the 
kernel to keep the CI green (I have one for x86.  The one for aarch64 quickly 
became infeasible).

I'm anxious for this to land, and the rebases are work indeed.  But if we can 
just wait a little longer and get inlining support working, then land this, it 
will be much less painful than the other way around.

I'm focused on inlining support in https://reviews.llvm.org/D58260 (which 
doesn't look like much now, but I have changes locally and have been actively 
working on it this week).


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

https://reviews.llvm.org/D56571



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

[PATCH] D18360: Add AIX Target/ToolChain to Clang Driver

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

D59048  has been posted with some initial 
`OSTargetInfo` changes. Noticeable differences include not defining 
`_ALL_SOURCE` and focus on 64-bit long double. This is consistent with the 
invocation provided with the recent v16.1 release of the IBM XL C/C++ compiler 
for AIX that incorporates Clang-based technology.


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

https://reviews.llvm.org/D18360



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

[PATCH] D59048: Add AIX Target Info

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a subscriber: rsmith.
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:626
+
+// FIXME: Define AIX OS-Version Macros
+Builder.defineMacro("_AIX");

Comments should be full sentences (with periods). Please update throughout this 
patch.



Comment at: clang/lib/Basic/Targets/OSTargets.h:629
+   
+// FIXME: Determine the proper way for users to select the no-long-long 
+// version of the standard library

@rsmith suggested that having a `-fno-long-long` option to disable support for 
long long would be preferable to having an `-maix-no-long-long` that just 
controls the AIX binary-compatibility aspects. Perhaps the FIXME should be 
updated to mention checking `-fno-long-long`.



Comment at: clang/lib/Basic/Targets/OSTargets.h:637
+
+// Define _WCHAR_T only for C++
+if (Opts.CPlusPlus && Opts.WChar) {

Suggestion for the comment text:
Define `_WCHAR_T` when it is a fundamental type (i.e., for C++ without 
`-fno-wchar`).



Comment at: clang/lib/Basic/Targets/OSTargets.h:641
+}
+  }
+

D18360 sets `_THREAD_SAFE` to `1` when `-pthread` is specified. I believe that 
to be correct. I believe whether or not `-pthread` is taken to be the default 
on the platform is a separate consideration.



Comment at: clang/lib/Basic/Targets/OSTargets.h:651
+}
+this->UseZeroLengthBitfieldAlignment = true;
+  }

Can we have a test for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59048



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Eli Friedman via Phabricator via cfe-commits]

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

I'm fine merging in this state... but please wait for rsmith to respond before 
you merge.


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

https://reviews.llvm.org/D56571



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

@rsmith, @efriedma, @chandlerc, is this in good enough shape that we can commit 
and move to incremental fixes/improvement? Jennifer has had to rebase this a 
couple times which is making things hard for the folks testing this with the 
Linux kernel and needing to apply this patch themselves.


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

https://reviews.llvm.org/D56571



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

[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 189602.
Quuxplusone added a comment.

Oops, really replace the destructor this time. (Hadn't `git commit`ed.)


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

https://reviews.llvm.org/D47344

Files:
  src/experimental/memory_resource.cpp
  www/cxx2a_status.html


Index: www/cxx2a_status.html
===
--- www/cxx2a_status.html
+++ www/cxx2a_status.html
@@ -219,7 +219,7 @@
https://wg21.link/LWG2164";>2164What are 
the semantics of vector.emplace(vector.begin(), 
vector.back())?Jacksonville
https://wg21.link/LWG2243";>2243istream::putback 
problemJacksonvilleComplete
https://wg21.link/LWG2816";>2816resize_file has 
impossible postconditionJacksonvilleNothing to 
do
-   https://wg21.link/LWG2843";>2843Unclear 
behavior of 
std::pmr::memory_resource::do_allocate()Jacksonville
+   https://wg21.link/LWG2843";>2843Unclear 
behavior of 
std::pmr::memory_resource::do_allocate()JacksonvilleComplete
https://wg21.link/LWG2849";>2849Why does 
!is_regular_file(from) cause copy_file to report a "file 
already exists" error?JacksonvilleNothing to 
do
https://wg21.link/LWG2851";>2851std::filesystem enum 
classes are now underspecifiedJacksonvilleNothing to 
do
https://wg21.link/LWG2946";>2946LWG 2758's 
resolution missed further 
correctionsJacksonvilleComplete
Index: src/experimental/memory_resource.cpp
===
--- src/experimental/memory_resource.cpp
+++ src/experimental/memory_resource.cpp
@@ -25,19 +25,23 @@
 class _LIBCPP_TYPE_VIS __new_delete_memory_resource_imp
 : public memory_resource
 {
-public:
-~__new_delete_memory_resource_imp() = default;
-
-protected:
-virtual void* do_allocate(size_t __size, size_t __align)
-{ return _VSTD::__libcpp_allocate(__size, __align); /* FIXME */}
+void *do_allocate(size_t size, size_t align) override {
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (__is_overaligned_for_new(align))
+__throw_bad_alloc();
+#endif
+return _VSTD::__libcpp_allocate(size, align);
+}
 
-virtual void do_deallocate(void* __p, size_t __n, size_t __align) {
-  _VSTD::__libcpp_deallocate(__p, __n, __align); /* FIXME */
+void do_deallocate(void *p, size_t n, size_t align) override {
+  _VSTD::__libcpp_deallocate(p, n, align);
 }
 
-virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
-{ return &__other == this; }
+bool do_is_equal(memory_resource const & other) const _NOEXCEPT override
+{ return &other == this; }
+
+public:
+~__new_delete_memory_resource_imp() override = default;
 };
 
 // null_memory_resource()


Index: www/cxx2a_status.html
===
--- www/cxx2a_status.html
+++ www/cxx2a_status.html
@@ -219,7 +219,7 @@
 	https://wg21.link/LWG2164";>2164What are the semantics of vector.emplace(vector.begin(), vector.back())?Jacksonville
 	https://wg21.link/LWG2243";>2243istream::putback problemJacksonvilleComplete
 	https://wg21.link/LWG2816";>2816resize_file has impossible postconditionJacksonvilleNothing to do
-	https://wg21.link/LWG2843";>2843Unclear behavior of std::pmr::memory_resource::do_allocate()Jacksonville
+	https://wg21.link/LWG2843";>2843Unclear behavior of std::pmr::memory_resource::do_allocate()JacksonvilleComplete
 	https://wg21.link/LWG2849";>2849Why does !is_regular_file(from) cause copy_file to report a "file already exists" error?JacksonvilleNothing to do
 	https://wg21.link/LWG2851";>2851std::filesystem enum classes are now underspecifiedJacksonvilleNothing to do
 	https://wg21.link/LWG2946";>2946LWG 2758's resolution missed further correctionsJacksonvilleComplete
Index: src/experimental/memory_resource.cpp
===
--- src/experimental/memory_resource.cpp
+++ src/experimental/memory_resource.cpp
@@ -25,19 +25,23 @@
 class _LIBCPP_TYPE_VIS __new_delete_memory_resource_imp
 : public memory_resource
 {
-public:
-~__new_delete_memory_resource_imp() = default;
-
-protected:
-virtual void* do_allocate(size_t __size, size_t __align)
-{ return _VSTD::__libcpp_allocate(__size, __align); /* FIXME */}
+void *do_allocate(size_t size, size_t align) override {
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (__is_overaligned_for_new(align))
+__throw_bad_alloc();
+#endif
+return _VSTD::__libcpp_allocate(size, align);
+}
 
-virtual void do_deallocate(void* __p, size_t __n, size_t __align) {
-  _VSTD::__libcpp_deallocate(__p, __n, __align); /* FIXME */
+void do_deallocate(void *p, size_t n, size_t align) override {
+  _VSTD::__libcpp_deallocate(p, n, align);
 }
 
-virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
-{ return &__ot

[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 189600.
Quuxplusone added a comment.
Herald added a subscriber: jdoerfert.

- Replace the unnecessary destructor in `class __new_delete_resource_imp`.
- Add LWG2843 to the list of completed issues (thanks @zoecarver!)


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D47344

Files:
  src/experimental/memory_resource.cpp
  www/cxx2a_status.html


Index: www/cxx2a_status.html
===
--- www/cxx2a_status.html
+++ www/cxx2a_status.html
@@ -219,7 +219,7 @@
https://wg21.link/LWG2164";>2164What are 
the semantics of vector.emplace(vector.begin(), 
vector.back())?Jacksonville
https://wg21.link/LWG2243";>2243istream::putback 
problemJacksonvilleComplete
https://wg21.link/LWG2816";>2816resize_file has 
impossible postconditionJacksonvilleNothing to 
do
-   https://wg21.link/LWG2843";>2843Unclear 
behavior of 
std::pmr::memory_resource::do_allocate()Jacksonville
+   https://wg21.link/LWG2843";>2843Unclear 
behavior of 
std::pmr::memory_resource::do_allocate()JacksonvilleComplete
https://wg21.link/LWG2849";>2849Why does 
!is_regular_file(from) cause copy_file to report a "file 
already exists" error?JacksonvilleNothing to 
do
https://wg21.link/LWG2851";>2851std::filesystem enum 
classes are now underspecifiedJacksonvilleNothing to 
do
https://wg21.link/LWG2946";>2946LWG 2758's 
resolution missed further 
correctionsJacksonvilleComplete
Index: src/experimental/memory_resource.cpp
===
--- src/experimental/memory_resource.cpp
+++ src/experimental/memory_resource.cpp
@@ -25,19 +25,20 @@
 class _LIBCPP_TYPE_VIS __new_delete_memory_resource_imp
 : public memory_resource
 {
-public:
-~__new_delete_memory_resource_imp() = default;
-
-protected:
-virtual void* do_allocate(size_t __size, size_t __align)
-{ return _VSTD::__libcpp_allocate(__size, __align); /* FIXME */}
+void *do_allocate(size_t size, size_t align) override {
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (__is_overaligned_for_new(align))
+__throw_bad_alloc();
+#endif
+return _VSTD::__libcpp_allocate(size, align);
+}
 
-virtual void do_deallocate(void* __p, size_t __n, size_t __align) {
-  _VSTD::__libcpp_deallocate(__p, __n, __align); /* FIXME */
+void do_deallocate(void *p, size_t n, size_t align) override {
+  _VSTD::__libcpp_deallocate(p, n, align);
 }
 
-virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
-{ return &__other == this; }
+bool do_is_equal(memory_resource const & other) const _NOEXCEPT override
+{ return &other == this; }
 };
 
 // null_memory_resource()


Index: www/cxx2a_status.html
===
--- www/cxx2a_status.html
+++ www/cxx2a_status.html
@@ -219,7 +219,7 @@
 	https://wg21.link/LWG2164";>2164What are the semantics of vector.emplace(vector.begin(), vector.back())?Jacksonville
 	https://wg21.link/LWG2243";>2243istream::putback problemJacksonvilleComplete
 	https://wg21.link/LWG2816";>2816resize_file has impossible postconditionJacksonvilleNothing to do
-	https://wg21.link/LWG2843";>2843Unclear behavior of std::pmr::memory_resource::do_allocate()Jacksonville
+	https://wg21.link/LWG2843";>2843Unclear behavior of std::pmr::memory_resource::do_allocate()JacksonvilleComplete
 	https://wg21.link/LWG2849";>2849Why does !is_regular_file(from) cause copy_file to report a "file already exists" error?JacksonvilleNothing to do
 	https://wg21.link/LWG2851";>2851std::filesystem enum classes are now underspecifiedJacksonvilleNothing to do
 	https://wg21.link/LWG2946";>2946LWG 2758's resolution missed further correctionsJacksonvilleComplete
Index: src/experimental/memory_resource.cpp
===
--- src/experimental/memory_resource.cpp
+++ src/experimental/memory_resource.cpp
@@ -25,19 +25,20 @@
 class _LIBCPP_TYPE_VIS __new_delete_memory_resource_imp
 : public memory_resource
 {
-public:
-~__new_delete_memory_resource_imp() = default;
-
-protected:
-virtual void* do_allocate(size_t __size, size_t __align)
-{ return _VSTD::__libcpp_allocate(__size, __align); /* FIXME */}
+void *do_allocate(size_t size, size_t align) override {
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (__is_overaligned_for_new(align))
+__throw_bad_alloc();
+#endif
+return _VSTD::__libcpp_allocate(size, align);
+}
 
-virtual void do_deallocate(void* __p, size_t __n, size_t __align) {
-  _VSTD::__libcpp_deallocate(__p, __n, __align); /* FIXME */
+void do_deallocate(void *p, size_t n, size_t align) override {
+  _VSTD::__libcpp_deallocate(p, n, align);
 }
 
-virtual bool do_is_equal(memory_re

[PATCH] D58216: Support attribute used in member funcs of class templates II

[Rafael Auler via Phabricator via cfe-commits]

rafauler added a comment.

I'm not an expert in swift either. I'll wait for @davezarzycki input on what's 
happening, but I suspect the code base is indeed incompatible with gcc due to 
the nature of the error they experienced.  This testcase is creduce-d from what 
I observed in swift.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58216



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

r355563 - [PGO] Re-submit: Clang part of change for context-sensitive PGO (part2)

[Rong Xu via cfe-commits]

Author: xur
Date: Wed Mar  6 15:00:38 2019
New Revision: 355563

URL: http://llvm.org/viewvc/llvm-project?rev=355563&view=rev
Log:
[PGO] Re-submit: Clang part of change for context-sensitive PGO (part2)

Part 2 of CSPGO change in Clang: Add test cases.

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


Added:
cfe/trunk/test/CodeGen/Inputs/pgotestir.proftext
cfe/trunk/test/CodeGen/Inputs/pgotestir_cs.proftext
cfe/trunk/test/CodeGen/cspgo-instrumentation.c
cfe/trunk/test/CodeGen/cspgo-instrumentation_lto.c
cfe/trunk/test/CodeGen/cspgo-instrumentation_thinlto.c

Added: cfe/trunk/test/CodeGen/Inputs/pgotestir.proftext
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/Inputs/pgotestir.proftext?rev=355563&view=auto
==
--- cfe/trunk/test/CodeGen/Inputs/pgotestir.proftext (added)
+++ cfe/trunk/test/CodeGen/Inputs/pgotestir.proftext Wed Mar  6 15:00:38 2019
@@ -0,0 +1,2 @@
+# IR level Instrumentation Flag
+:ir

Added: cfe/trunk/test/CodeGen/Inputs/pgotestir_cs.proftext
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/Inputs/pgotestir_cs.proftext?rev=355563&view=auto
==
--- cfe/trunk/test/CodeGen/Inputs/pgotestir_cs.proftext (added)
+++ cfe/trunk/test/CodeGen/Inputs/pgotestir_cs.proftext Wed Mar  6 15:00:38 2019
@@ -0,0 +1,2 @@
+# IR level Instrumentation Flag with CS
+:csir

Added: cfe/trunk/test/CodeGen/cspgo-instrumentation.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/cspgo-instrumentation.c?rev=355563&view=auto
==
--- cfe/trunk/test/CodeGen/cspgo-instrumentation.c (added)
+++ cfe/trunk/test/CodeGen/cspgo-instrumentation.c Wed Mar  6 15:00:38 2019
@@ -0,0 +1,41 @@
+// Test if CSPGO instrumentation and use pass are invoked.
+//
+// Ensure Pass PGOInstrumentationGenPass is invoked.
+// RUN: %clang_cc1 -O2 -fprofile-instrument=csllvm 
-fprofile-instrument-path=default.profraw  %s -mllvm -debug-pass=Structure 
-emit-llvm -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN
+// RUN: %clang_cc1 -O2 -fprofile-instrument=csllvm 
-fprofile-instrument-path=default.profraw  %s -fexperimental-new-pass-manager 
-fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-NEWPM
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN: PGOInstrumentationGenCreateVarPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN: PGOInstrumentationGenPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-NEWPM: Running pass: 
PGOInstrumentationGenCreateVar on
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-NEWPM: Running pass: 
PGOInstrumentationGen on
+//
+// RUN: rm -rf %t && mkdir %t
+// RUN: llvm-profdata merge -o %t/noncs.profdata %S/Inputs/pgotestir.proftext
+//
+// Ensure Pass PGOInstrumentationUsePass and PGOInstrumentationGenPass are 
invoked.
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata 
-fprofile-instrument=csllvm -fprofile-instrument-path=default.profraw  %s 
-mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata 
-fprofile-instrument=csllvm -fprofile-instrument-path=default.profraw  %s 
-fexperimental-new-pass-manager -fdebug-pass-manager  -emit-llvm -o - 2>&1 | 
FileCheck %s -check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2-NEWPM
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2: PGOInstrumentationUsePass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2: PGOInstrumentationGenCreateVarPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2: PGOInstrumentationGenPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2-NEWPM: Running pass: 
PGOInstrumentationUse
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2-NEWPM: Running pass: 
PGOInstrumentationGenCreateVar on
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2-NEWPM: Running pass: 
PGOInstrumentationGen on
+
+// Ensure Pass PGOInstrumentationUsePass is invoked only once.
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata %s 
-mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-PGOUSEPASS-INVOKED-USE
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata %s 
-fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm -o - 2>&1 | 
FileCheck %s -check-prefix=CHECK-PGOUSEPASS-INVOKED-USE-NEWPM
+// CHECK-PGOUSEPASS-INVOKED-USE: PGOInstrumentationUsePass
+// CHECK-PGOUSEPASS-INVOKED-USE-NOT: PGOInstrumentationGenCreateVarPass
+// CHECK-PGOUSEPASS-INVOKED-USE-NOT: PGOInstrumentationUsePass
+// CHECK-PGOUSEPASS-INVOKED-USE-NEWPM: Running pass: PGOInstrumentationUse
+// CHECK-PGOUSEPASS-INVOKED-USE-NEWPM-NOT: Running pass: 
PGOInstrumentationGenCreateVar
+// CHECK-PGOUSEPASS-INVOKED-USE-NEWPM-NOT: Running pass: PGOInstrumentationUse
+//
+// Ensure Pass PGOInstrum

[PATCH] D59055: [analyzer] Prepare generic taint checker for new sources

[Borsik Gábor via Phabricator via cfe-commits]

boga95 created this revision.
boga95 added reviewers: gerazo, xazax.hun, Szelethus, a_sidorin, dcoughlin, 
george.karpenkov, NoQ.
boga95 added a project: clang.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity.

Previously the taint propagation rules and the taint sources were checked in 
different steps.
Taint propagation goes in two steps: addSourcesPre marked the tainted arguments 
and the return value, then the propagateFromPre set the tainted flag. After 
that addSourcesPost set the tainted flag for the source function's(scanf, 
socket, e.g) arguments or return value.
There is no reason why it should be that way. A source function can be 
interpreted as a propagation rule when no srcArg is defined.
I modified the TaintPropagationRule to support source functions and merged them 
with the propagation rules.


Repository:
  rC Clang

https://reviews.llvm.org/D59055

Files:
  lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -62,9 +62,6 @@
   /// Propagate taint generated at pre-visit.
   bool propagateFromPre(const CallExpr *CE, CheckerContext &C) const;
 
-  /// Add taint sources on a post visit.
-  void addSourcesPost(const CallExpr *CE, CheckerContext &C) const;
-
   /// Check if the region the expression evaluates to is the standard input,
   /// and thus, is tainted.
   static bool isStdin(const Expr *E, CheckerContext &C);
@@ -72,16 +69,6 @@
   /// Given a pointer argument, return the value it points to.
   static Optional getPointedToSVal(CheckerContext &C, const Expr *Arg);
 
-  /// Functions defining the attack surface.
-  using FnCheck = ProgramStateRef (GenericTaintChecker::*)(
-  const CallExpr *, CheckerContext &C) const;
-  ProgramStateRef postScanf(const CallExpr *CE, CheckerContext &C) const;
-  ProgramStateRef postSocket(const CallExpr *CE, CheckerContext &C) const;
-  ProgramStateRef postRetTaint(const CallExpr *CE, CheckerContext &C) const;
-
-  /// Taint the scanned input if the file is tainted.
-  ProgramStateRef preFscanf(const CallExpr *CE, CheckerContext &C) const;
-
   /// Check for CWE-134: Uncontrolled Format String.
   static const char MsgUncontrolledFormatString[];
   bool checkUncontrolledFormatString(const CallExpr *CE,
@@ -118,6 +105,9 @@
   struct TaintPropagationRule {
 enum class VariadicType { None, Src, Dst };
 
+using PropagationFuncType = bool (*)(bool IsTainted, const CallExpr *,
+ CheckerContext &C);
+
 /// List of arguments which can be taint sources and should be checked.
 ArgVector SrcArgs;
 /// List of arguments which should be tainted on function return.
@@ -127,16 +117,21 @@
 /// Show when a function has variadic parameters. If it has, it marks all
 /// of them as source or destination.
 VariadicType VarType;
+/// Special function for tainted source determination. If defined, it can
+/// override the default behavior.
+PropagationFuncType PropagationFunc;
 
 TaintPropagationRule()
-: VariadicIndex(InvalidArgIndex), VarType(VariadicType::None) {}
+: VariadicIndex(InvalidArgIndex), VarType(VariadicType::None),
+  PropagationFunc(nullptr) {}
 
 TaintPropagationRule(std::initializer_list &&Src,
  std::initializer_list &&Dst,
  VariadicType Var = VariadicType::None,
- unsigned VarIndex = InvalidArgIndex)
+ unsigned VarIndex = InvalidArgIndex,
+ PropagationFuncType Func = nullptr)
 : SrcArgs(std::move(Src)), DstArgs(std::move(Dst)),
-  VariadicIndex(VarIndex), VarType(Var) {}
+  VariadicIndex(VarIndex), VarType(Var), PropagationFunc(Func) {}
 
 /// Get the propagation rule for a given function.
 static TaintPropagationRule
@@ -170,6 +165,10 @@
 /// Pre-process a function which propagates taint according to the
 /// taint rule.
 ProgramStateRef process(const CallExpr *CE, CheckerContext &C) const;
+
+// Functions for custom taintedness propagation.
+static bool postSocket(bool IsTainted, const CallExpr *CE,
+   CheckerContext &C);
   };
 };
 
@@ -206,25 +205,42 @@
   // Check for exact name match for functions without builtin substitutes.
   TaintPropagationRule Rule =
   llvm::StringSwitch(Name)
+  // Source functions
+  // TODO: Add support for vfscanf & family.
+  .Case("fdopen", TaintPropagationRule({}, {ReturnValueIndex}))
+  .Case("fopen", TaintPropagationRule({}, {ReturnValueIndex}))
+  .Case("freopen", TaintPropagationRule({}, {ReturnValueIndex}))
+  .Case("g

[PATCH] D59054: [analyzer] C++17: PR40022: Support aggregate initialization with compound values in presence of base classes.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, 
mikhail.ramalho, Szelethus, baloghadamsoftware.
Herald added subscribers: cfe-commits, Charusso, jdoerfert, dkrupp, donat.nagy, 
a.sidorin, szepet.
Herald added a project: clang.

I'll add more tests soon, to see if it is actually initialized *correctly* now, 
but for now here's the code that fixes the crash in 
https://bugs.llvm.org/show_bug.cgi?id=40022 .

It turns out that C++17 aggregate initialization with base classes wasn't 
really ever implemented for the case when it *doesn't* involve calling any 
sub-object constructors. That it, when the aggregate is initialized with a 
simple initializer list and the resulting value is a `nonloc::CompoundVal`, we 
simply didn't know that we need to perform the store bind for the base classes 
as well. Now it should work.

The original bug report causes us to crash when there's a flexible array at the 
end of the structure (which is a non-standard thing), which made me 
underestimate the problem a bit :)

The case where we *do* have constructors in base classes is, of course, still 
unimplemented; it was last touched in D40841 .


Repository:
  rC Clang

https://reviews.llvm.org/D59054

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/array-struct-region.cpp


Index: clang/test/Analysis/array-struct-region.cpp
===
--- clang/test/Analysis/array-struct-region.cpp
+++ clang/test/Analysis/array-struct-region.cpp
@@ -1,7 +1,9 @@
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c %s
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ 
-analyzer-config c++-inlining=constructors %s
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ 
-std=c++17 -analyzer-config c++-inlining=constructors %s
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c %s
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c++ 
-analyzer-config c++-inlining=constructors %s
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c++ 
-std=c++17 -analyzer-config c++-inlining=constructors %s
 
 void clang_analyzer_eval(int);
 
@@ -196,4 +198,21 @@
   }
 }
 
+namespace flex_array_inheritance_cxx17 {
+struct A {
+  int flexible_array[];
+};
+
+struct B {
+  long cookie;
+};
+
+struct C : B {
+  A a;
+};
+
+void k() {
+  C c{};
+}
+} // namespace flex_array_inheritance_cxx17
 #endif
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2337,9 +2337,37 @@
   const nonloc::CompoundVal& CV = V.castAs();
   nonloc::CompoundVal::iterator VI = CV.begin(), VE = CV.end();
 
-  RecordDecl::field_iterator FI, FE;
   RegionBindingsRef NewB(B);
 
+  // In C++17 aggregates may have base classes, handle those as well.
+  // They appear before fields in the initializer list / compound value.
+  if (const auto *CRD = dyn_cast(RD)) {
+assert(CRD->isAggregate());
+
+// Virtual bases still aren't allowed. Multiple bases are fine though.
+for (auto B : CRD->bases()) {
+  assert(B.isVirtual() == false);
+
+  if (VI == VE)
+break;
+
+  QualType BTy = B.getType();
+  assert(BTy->isStructureOrClassType());
+
+  const CXXRecordDecl *BRD = BTy->getAsCXXRecordDecl();
+  assert(BRD);
+
+  const CXXBaseObjectRegion *BR =
+  MRMgr.getCXXBaseObjectRegion(BRD, R, /*IsVirtual=*/false);
+
+  NewB = bindStruct(NewB, BR, *VI);
+
+  ++VI;
+}
+  }
+
+  RecordDecl::field_iterator FI, FE;
+
   for (FI = RD->field_begin(), FE = RD->field_end(); FI != FE; ++FI) {
 
 if (VI == VE)


Index: clang/test/Analysis/array-struct-region.cpp
===
--- clang/test/Analysis/array-struct-region.cpp
+++ clang/test/Analysis/array-struct-region.cpp
@@ -1,7 +1,9 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ -analyzer-config c++-inlining=constructors %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ -std=c++17 -analyzer-config c++-inlining=constructors %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c++ -analyzer-config c++-inlining=constructors %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,a

[PATCH] D58154: Add support for -fpermissive.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D58154#1420344 , @rsmith wrote:

> I don't have particularly strong feelings one way or the other about 
> accepting `-fpermissive` at all. For GCC compatibility, it seems like a 
> moderately useful thing to support, but I don't think we have any interest in 
> accepting everything that GCC accepts under `-fpermissive`.


Agreed that we shouldn't accept everything GCC accepts under `-fpermissive`.

> Perhaps the better choice is to not provide the flag at all, rather than to 
> provide something that has the same interface but doesn't accept the same 
> code. If not that, making `-fpermissive` an alias for `-Wno-error=everything` 
> is probably a better approach than that of this patch.

Past experience has shown that when we add a flag for compatibility purposes, 
users expect compatibility and when I saw the title "Add support for 
-fpermissive", I got very scared that we meant *all* of `-fpermissive` 
(especially when the rationale was for GCC compatibility). I think we should 
avoid spelling this `-fpermissive`.

If we go with a different name for the flag, then the user has to update their 
build scripts to get code to compile with Clang, which means it shouldn't be 
too onerous for them to spell out the specific diagnostics they need disabled 
(and it sort of forces them into somewhat better code hygiene by not disabling 
all diagnostics). I'm kind of leaning towards not providing a flag at all.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58154



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

[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

@Quuxplusone if this fixes 2843 can you update it in `www/cxx2a_status.html`?


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D47344



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

[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: src/experimental/memory_resource.cpp:29
-public:
-~__new_delete_memory_resource_imp() = default;
-

Quuxplusone wrote:
> EricWF wrote:
> > Why are you removing this?
> Just removing unnecessary cruft. Same reason I added `override` (and removed 
> `virtual`) on the other methods, and removed `protected:`, and removed the 
> `__` sigilling on function parameters in this .cpp file.
> 
> I had done a lot more random cleanup originally, and then you said 
> "unrelated, please revert" so I did, but I assume I just kept all the 
> cleanups in this one class since I was touching this range of lines no matter 
> what.
I strongly prefer seeing explicit declarations of non-trivial or virtual 
destructors.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D47344



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

[PATCH] D59048: Add AIX Target Info

[Andus Yu via Phabricator via cfe-commits]

andusy created this revision.
andusy added reviewers: apaprocki, chandlerc, hubert.reinterpretcast, jasonliu, 
xingxue, sfertile.
Herald added subscribers: jsji, jfb, kbarton, nemanjai.
Herald added a project: clang.

A first pass over platform-specific properties of the C API/ABI on AIX for both 
32-bit and 64-bit modes. This is a continuation of D18360 
 by Andrew Paprocki and further work by Wu 
Zhao.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59048

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/test/Driver/types.c
  clang/test/Headers/max_align.c
  clang/test/Preprocessor/init.c
  clang/test/Sema/varargs-aix.c

Index: clang/test/Sema/varargs-aix.c
===
--- /dev/null
+++ clang/test/Sema/varargs-aix.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple powerpc-ibm-aix
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple powerpc64-ibm-aix
+// expected-no-diagnostics
+
+extern __builtin_va_list ap;
+extern char *ap;
Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -6409,6 +6409,205 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-feature +float128 -target-cpu power9 -fno-signed-char < /dev/null | FileCheck -check-prefix PPC-FLOAT128 %s
 // PPC-FLOAT128:#define __FLOAT128__ 1
 //
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC64-AIX %s
+//
+// PPC64-AIX:#define _AIX 1
+// PPC64-AIX:#define _ARCH_PPC 1
+// PPC64-AIX:#define _ARCH_PPC64 1
+// PPC64-AIX:#define _BIG_ENDIAN 1
+// PPC64-AIX:#define _IBMR2 1
+// PPC64-AIX:#define _LONG_LONG 1
+// PPC64-AIX:#define _POWER 1
+// PPC64-AIX:#define __64BIT__ 1
+// PPC64-AIX:#define __BIGGEST_ALIGNMENT__ 8 
+// PPC64-AIX:#define __BIG_ENDIAN__ 1
+// PPC64-AIX:#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
+// PPC64-AIX:#define __CHAR16_TYPE__ unsigned short
+// PPC64-AIX:#define __CHAR32_TYPE__ unsigned int
+// PPC64-AIX:#define __CHAR_BIT__ 8
+// PPC64-AIX:#define __CHAR_UNSIGNED__ 1
+// PPC64-AIX:#define __DBL_DENORM_MIN__ 4.9406564584124654e-324
+// PPC64-AIX:#define __DBL_DIG__ 15
+// PPC64-AIX:#define __DBL_EPSILON__ 2.2204460492503131e-16
+// PPC64-AIX:#define __DBL_HAS_DENORM__ 1
+// PPC64-AIX:#define __DBL_HAS_INFINITY__ 1
+// PPC64-AIX:#define __DBL_HAS_QUIET_NAN__ 1
+// PPC64-AIX:#define __DBL_MANT_DIG__ 53
+// PPC64-AIX:#define __DBL_MAX_10_EXP__ 308
+// PPC64-AIX:#define __DBL_MAX_EXP__ 1024
+// PPC64-AIX:#define __DBL_MAX__ 1.7976931348623157e+308
+// PPC64-AIX:#define __DBL_MIN_10_EXP__ (-307)
+// PPC64-AIX:#define __DBL_MIN_EXP__ (-1021)
+// PPC64-AIX:#define __DBL_MIN__ 2.2250738585072014e-308
+// PPC64-AIX:#define __DECIMAL_DIG__ __LDBL_DECIMAL_DIG__
+// PPC64-AIX:#define __FLT_DENORM_MIN__ 1.40129846e-45F
+// PPC64-AIX:#define __FLT_DIG__ 6
+// PPC64-AIX:#define __FLT_EPSILON__ 1.19209290e-7F
+// PPC64-AIX:#define __FLT_EVAL_METHOD__ 1  
+// PPC64-AIX:#define __FLT_HAS_DENORM__ 1
+// PPC64-AIX:#define __FLT_HAS_INFINITY__ 1
+// PPC64-AIX:#define __FLT_HAS_QUIET_NAN__ 1
+// PPC64-AIX:#define __FLT_MANT_DIG__ 24
+// PPC64-AIX:#define __FLT_MAX_10_EXP__ 38
+// PPC64-AIX:#define __FLT_MAX_EXP__ 128
+// PPC64-AIX:#define __FLT_MAX__ 3.40282347e+38F
+// PPC64-AIX:#define __FLT_MIN_10_EXP__ (-37)
+// PPC64-AIX:#define __FLT_MIN_EXP__ (-125)
+// PPC64-AIX:#define __FLT_MIN__ 1.17549435e-38F
+// PPC64-AIX:#define __FLT_RADIX__ 2
+// PPC64-AIX:#define __INT16_C_SUFFIX__
+// PPC64-AIX:#define __INT16_FMTd__ "hd"
+// PPC64-AIX:#define __INT16_FMTi__ "hi"
+// PPC64-AIX:#define __INT16_MAX__ 32767
+// PPC64-AIX:#define __INT16_TYPE__ short
+// PPC64-AIX:#define __INT32_C_SUFFIX__
+// PPC64-AIX:#define __INT32_FMTd__ "d"
+// PPC64-AIX:#define __INT32_FMTi__ "i"
+// PPC64-AIX:#define __INT32_MAX__ 2147483647
+// PPC64-AIX:#define __INT32_TYPE__ int
+// PPC64-AIX:#define __INT64_C_SUFFIX__ L
+// PPC64-AIX:#define __INT64_FMTd__ "ld"
+// PPC64-AIX:#define __INT64_FMTi__ "li"
+// PPC64-AIX:#define __INT64_MAX__ 9223372036854775807L
+// PPC64-AIX:#define __INT64_TYPE__ long int
+// PPC64-AIX:#define __INT8_C_SUFFIX__
+// PPC64-AIX:#define __INT8_FMTd__ "hhd"
+// PPC64-AIX:#define __INT8_FMTi__ "hhi"
+// PPC64-AIX:#define __INT8_MAX__ 127
+// PPC64-AIX:#define __INT8_TYPE__ signed char
+// PPC64-AIX:#define __INTMAX_C_SUFFIX__ L
+// PPC64-AIX:#define __INTMAX_FMTd__ "ld"
+// PPC64-AIX:#define __INTMAX_FMTi__ "li"
+// PPC64-AIX:#define __INTMAX_MAX__ 9223372036854775807L
+// PPC64-AIX:#define __INTMAX_TYPE__ long int
+// PPC64-AIX:#define __INTMAX_WIDTH__ 64
+// PPC64-AIX:#define __INTPTR_FMTd__ "ld"
+// PPC64-AIX:#define __INTPTR_FMTi__ "li"
+// PPC64-AIX:#define __INTPTR_MAX__

[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"

[Arthur O'Dwyer via Phabricator via cfe-commits]

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



Comment at: src/experimental/memory_resource.cpp:29
-public:
-~__new_delete_memory_resource_imp() = default;
-

EricWF wrote:
> Why are you removing this?
Just removing unnecessary cruft. Same reason I added `override` (and removed 
`virtual`) on the other methods, and removed `protected:`, and removed the `__` 
sigilling on function parameters in this .cpp file.

I had done a lot more random cleanup originally, and then you said "unrelated, 
please revert" so I did, but I assume I just kept all the cleanups in this one 
class since I was touching this range of lines no matter what.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D47344



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

[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"

[Eric Fiselier via Phabricator via cfe-commits]

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

LGTM other than removing the destructor.




Comment at: src/experimental/memory_resource.cpp:29
-public:
-~__new_delete_memory_resource_imp() = default;
-

Why are you removing this?


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D47344



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

r355551 - [CUDA][HIP][DebugInfo] Skip reference device function

[Michael Liao via cfe-commits]

Author: hliao
Date: Wed Mar  6 13:16:27 2019
New Revision: 31

URL: http://llvm.org/viewvc/llvm-project?rev=31&view=rev
Log:
[CUDA][HIP][DebugInfo] Skip reference device function

Summary:
- A device functions could be used as a non-type template parameter in a
  global/host function template. However, we should not try to retrieve that
  device function and reference it in the host-side debug info as it's
  only valid at device side.

Subscribers: aprantl, jdoerfert, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/CodeGenCUDA/debug-info-template.cu
Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=31&r1=30&r2=31&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Mar  6 13:16:27 2019
@@ -1725,31 +1725,37 @@ CGDebugInfo::CollectTemplateParams(const
   QualType T = TA.getParamTypeForDecl().getDesugaredType(CGM.getContext());
   llvm::DIType *TTy = getOrCreateType(T, Unit);
   llvm::Constant *V = nullptr;
-  const CXXMethodDecl *MD;
-  // Variable pointer template parameters have a value that is the address
-  // of the variable.
-  if (const auto *VD = dyn_cast(D))
-V = CGM.GetAddrOfGlobalVar(VD);
-  // Member function pointers have special support for building them, 
though
-  // this is currently unsupported in LLVM CodeGen.
-  else if ((MD = dyn_cast(D)) && MD->isInstance())
-V = CGM.getCXXABI().EmitMemberFunctionPointer(MD);
-  else if (const auto *FD = dyn_cast(D))
-V = CGM.GetAddrOfFunction(FD);
-  // Member data pointers have special handling too to compute the fixed
-  // offset within the object.
-  else if (const auto *MPT = dyn_cast(T.getTypePtr())) {
-// These five lines (& possibly the above member function pointer
-// handling) might be able to be refactored to use similar code in
-// CodeGenModule::getMemberPointerConstant
-uint64_t fieldOffset = CGM.getContext().getFieldOffset(D);
-CharUnits chars =
-CGM.getContext().toCharUnitsFromBits((int64_t)fieldOffset);
-V = CGM.getCXXABI().EmitMemberDataPointer(MPT, chars);
+  // Skip retrieve the value if that template parameter has cuda device
+  // attribute, i.e. that value is not available at the host side.
+  if (!CGM.getLangOpts().CUDA || CGM.getLangOpts().CUDAIsDevice ||
+  !D->hasAttr()) {
+const CXXMethodDecl *MD;
+// Variable pointer template parameters have a value that is the 
address
+// of the variable.
+if (const auto *VD = dyn_cast(D))
+  V = CGM.GetAddrOfGlobalVar(VD);
+// Member function pointers have special support for building them,
+// though this is currently unsupported in LLVM CodeGen.
+else if ((MD = dyn_cast(D)) && MD->isInstance())
+  V = CGM.getCXXABI().EmitMemberFunctionPointer(MD);
+else if (const auto *FD = dyn_cast(D))
+  V = CGM.GetAddrOfFunction(FD);
+// Member data pointers have special handling too to compute the fixed
+// offset within the object.
+else if (const auto *MPT =
+ dyn_cast(T.getTypePtr())) {
+  // These five lines (& possibly the above member function pointer
+  // handling) might be able to be refactored to use similar code in
+  // CodeGenModule::getMemberPointerConstant
+  uint64_t fieldOffset = CGM.getContext().getFieldOffset(D);
+  CharUnits chars =
+  CGM.getContext().toCharUnitsFromBits((int64_t)fieldOffset);
+  V = CGM.getCXXABI().EmitMemberDataPointer(MPT, chars);
+}
+V = V->stripPointerCasts();
   }
   TemplateParams.push_back(DBuilder.createTemplateValueParameter(
-  TheCU, Name, TTy,
-  cast_or_null(V->stripPointerCasts(;
+  TheCU, Name, TTy, cast_or_null(V)));
 } break;
 case TemplateArgument::NullPtr: {
   QualType T = TA.getNullPtrType();

Added: cfe/trunk/test/CodeGenCUDA/debug-info-template.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCUDA/debug-info-template.cu?rev=31&view=auto
==
--- cfe/trunk/test/CodeGenCUDA/debug-info-template.cu (added)
+++ cfe/trunk/test/CodeGenCUDA/debug-info-template.cu Wed Mar  6 13:16:27 2019
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple x86_64-unknown-linux -emit-llvm -o - 
-debug-info-kind=limited -dwarf-version=2 -debugger-tuning=gdb | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+__device__ void f();
+template __global__ void t() { F(); }
+__host__ void g() { t<<<1,1>>>(); }
+
+// Ensure

[PATCH] D58992: [CUDA][HIP][DebugInfo] Skip reference device function

[Michael Liao via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL31: [CUDA][HIP][DebugInfo] Skip reference device 
function (authored by hliao, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58992?vs=189488&id=189570#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58992

Files:
  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
  cfe/trunk/test/CodeGenCUDA/debug-info-template.cu


Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -1725,31 +1725,37 @@
   QualType T = TA.getParamTypeForDecl().getDesugaredType(CGM.getContext());
   llvm::DIType *TTy = getOrCreateType(T, Unit);
   llvm::Constant *V = nullptr;
-  const CXXMethodDecl *MD;
-  // Variable pointer template parameters have a value that is the address
-  // of the variable.
-  if (const auto *VD = dyn_cast(D))
-V = CGM.GetAddrOfGlobalVar(VD);
-  // Member function pointers have special support for building them, 
though
-  // this is currently unsupported in LLVM CodeGen.
-  else if ((MD = dyn_cast(D)) && MD->isInstance())
-V = CGM.getCXXABI().EmitMemberFunctionPointer(MD);
-  else if (const auto *FD = dyn_cast(D))
-V = CGM.GetAddrOfFunction(FD);
-  // Member data pointers have special handling too to compute the fixed
-  // offset within the object.
-  else if (const auto *MPT = dyn_cast(T.getTypePtr())) {
-// These five lines (& possibly the above member function pointer
-// handling) might be able to be refactored to use similar code in
-// CodeGenModule::getMemberPointerConstant
-uint64_t fieldOffset = CGM.getContext().getFieldOffset(D);
-CharUnits chars =
-CGM.getContext().toCharUnitsFromBits((int64_t)fieldOffset);
-V = CGM.getCXXABI().EmitMemberDataPointer(MPT, chars);
+  // Skip retrieve the value if that template parameter has cuda device
+  // attribute, i.e. that value is not available at the host side.
+  if (!CGM.getLangOpts().CUDA || CGM.getLangOpts().CUDAIsDevice ||
+  !D->hasAttr()) {
+const CXXMethodDecl *MD;
+// Variable pointer template parameters have a value that is the 
address
+// of the variable.
+if (const auto *VD = dyn_cast(D))
+  V = CGM.GetAddrOfGlobalVar(VD);
+// Member function pointers have special support for building them,
+// though this is currently unsupported in LLVM CodeGen.
+else if ((MD = dyn_cast(D)) && MD->isInstance())
+  V = CGM.getCXXABI().EmitMemberFunctionPointer(MD);
+else if (const auto *FD = dyn_cast(D))
+  V = CGM.GetAddrOfFunction(FD);
+// Member data pointers have special handling too to compute the fixed
+// offset within the object.
+else if (const auto *MPT =
+ dyn_cast(T.getTypePtr())) {
+  // These five lines (& possibly the above member function pointer
+  // handling) might be able to be refactored to use similar code in
+  // CodeGenModule::getMemberPointerConstant
+  uint64_t fieldOffset = CGM.getContext().getFieldOffset(D);
+  CharUnits chars =
+  CGM.getContext().toCharUnitsFromBits((int64_t)fieldOffset);
+  V = CGM.getCXXABI().EmitMemberDataPointer(MPT, chars);
+}
+V = V->stripPointerCasts();
   }
   TemplateParams.push_back(DBuilder.createTemplateValueParameter(
-  TheCU, Name, TTy,
-  cast_or_null(V->stripPointerCasts(;
+  TheCU, Name, TTy, cast_or_null(V)));
 } break;
 case TemplateArgument::NullPtr: {
   QualType T = TA.getNullPtrType();
Index: cfe/trunk/test/CodeGenCUDA/debug-info-template.cu
===
--- cfe/trunk/test/CodeGenCUDA/debug-info-template.cu
+++ cfe/trunk/test/CodeGenCUDA/debug-info-template.cu
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple x86_64-unknown-linux -emit-llvm -o - 
-debug-info-kind=limited -dwarf-version=2 -debugger-tuning=gdb | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+__device__ void f();
+template __global__ void t() { F(); }
+__host__ void g() { t<<<1,1>>>(); }
+
+// Ensure the value of device-function (as value template parameter) is null.
+// CHECK: !DITemplateValueParameter(name: "F", type: !{{[0-9]+}}, value: null)


Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -1725,31 +1725,37 @@
   QualType T = TA.getParamTypeForDecl().getDesugaredType(CGM.getContext());
   llvm::DIType *TTy

[PATCH] D58992: [CUDA][HIP][DebugInfo] Skip reference device function

[Adrian Prantl via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1755
+}
+V = V->stripPointerCasts();
   }

hliao wrote:
> aprantl wrote:
> > This wasn't there before... why is this necessary?
> That's from the original line 1752.
Oh :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58992



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

[PATCH] D58216: Support attribute used in member funcs of class templates II

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D58216#1420149 , @rafauler wrote:

> Both approaches make sense to me. I'll re-land the previous patch in favor of 
> gcc compatibility, since the semantics of attribute used in member functions 
> of class templates were first implemented in gcc.


I think we should be compatible with GCC here.

> @davezarzycki  Heads up that this will land again. Can you change the code in 
> swift to use the attribute used in the declaration of the specialization, not 
> in the declaration of the template? (that is, if you really need the 
> attribute, of course).

I know very little about the Swift code base. Can it compile with GCC? If it 
can, then I think something is missing from the test case. If it can't compile 
with GCC, then I'm guessing the attribute was effectively a noop before and can 
be removed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58216



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

[PATCH] D58847: AMDGPU: Fix the mapping of sub group sync scope

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC355549: AMDGPU: Fix the mapping of sub group sync scope 
(authored by kzhuravl, committed by ).
Herald added a project: clang.

Repository:
  rC Clang

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

https://reviews.llvm.org/D58847

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGenOpenCL/atomic-ops.cl


Index: test/CodeGenOpenCL/atomic-ops.cl
===
--- test/CodeGenOpenCL/atomic-ops.cl
+++ test/CodeGenOpenCL/atomic-ops.cl
@@ -41,7 +41,7 @@
   // CHECK: load atomic i32, i32* %{{[.0-9A-Z_a-z]+}} seq_cst
   x = __opencl_atomic_load(i, memory_order_seq_cst, 
memory_scope_all_svm_devices);
 
-  // CHECK: load atomic i32, i32* %{{[.0-9A-Z_a-z]+}} syncscope("subgroup") 
seq_cst
+  // CHECK: load atomic i32, i32* %{{[.0-9A-Z_a-z]+}} syncscope("wavefront") 
seq_cst
   x = __opencl_atomic_load(i, memory_order_seq_cst, memory_scope_sub_group);
 }
 
@@ -109,7 +109,7 @@
   // CHECK: load atomic i32, i32* %{{.*}} seq_cst
   // CHECK: br label %[[continue]]
   // CHECK: [[opencl_subgroup]]:
-  // CHECK: load atomic i32, i32* %{{.*}} syncscope("subgroup") seq_cst
+  // CHECK: load atomic i32, i32* %{{.*}} syncscope("wavefront") seq_cst
   // CHECK: br label %[[continue]]
   // CHECK: [[continue]]:
   int x = __opencl_atomic_load(i, memory_order_seq_cst, scope);
@@ -147,7 +147,7 @@
   // CHECK: [[MON_ALL]]:
   // CHECK: load atomic i32, i32* %{{.*}} monotonic
   // CHECK: [[MON_SUB]]:
-  // CHECK: load atomic i32, i32* %{{.*}} syncscope("subgroup") monotonic
+  // CHECK: load atomic i32, i32* %{{.*}} syncscope("wavefront") monotonic
   // CHECK: [[ACQ_WG]]:
   // CHECK: load atomic i32, i32* %{{.*}} syncscope("workgroup") acquire
   // CHECK: [[ACQ_DEV]]:
@@ -155,7 +155,7 @@
   // CHECK: [[ACQ_ALL]]:
   // CHECK: load atomic i32, i32* %{{.*}} acquire
   // CHECK: [[ACQ_SUB]]:
-  // CHECK: load atomic i32, i32* %{{.*}} syncscope("subgroup") acquire
+  // CHECK: load atomic i32, i32* %{{.*}} syncscope("wavefront") acquire
   // CHECK: [[SEQ_WG]]:
   // CHECK: load atomic i32, i32* %{{.*}} syncscope("workgroup") seq_cst
   // CHECK: [[SEQ_DEV]]:
@@ -163,7 +163,7 @@
   // CHECK: [[SEQ_ALL]]:
   // CHECK: load atomic i32, i32* %{{.*}} seq_cst
   // CHECK: [[SEQ_SUB]]:
-  // CHECK: load atomic i32, i32* %{{.*}} syncscope("subgroup") seq_cst
+  // CHECK: load atomic i32, i32* %{{.*}} syncscope("wavefront") seq_cst
   int x = __opencl_atomic_load(i, order, scope);
 }
 
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -7959,7 +7959,7 @@
 Name = "";
 break;
   case SyncScope::OpenCLSubGroup:
-Name = "subgroup";
+Name = "wavefront";
   }
   return C.getOrInsertSyncScopeID(Name);
 }


Index: test/CodeGenOpenCL/atomic-ops.cl
===
--- test/CodeGenOpenCL/atomic-ops.cl
+++ test/CodeGenOpenCL/atomic-ops.cl
@@ -41,7 +41,7 @@
   // CHECK: load atomic i32, i32* %{{[.0-9A-Z_a-z]+}} seq_cst
   x = __opencl_atomic_load(i, memory_order_seq_cst, memory_scope_all_svm_devices);
 
-  // CHECK: load atomic i32, i32* %{{[.0-9A-Z_a-z]+}} syncscope("subgroup") seq_cst
+  // CHECK: load atomic i32, i32* %{{[.0-9A-Z_a-z]+}} syncscope("wavefront") seq_cst
   x = __opencl_atomic_load(i, memory_order_seq_cst, memory_scope_sub_group);
 }
 
@@ -109,7 +109,7 @@
   // CHECK: load atomic i32, i32* %{{.*}} seq_cst
   // CHECK: br label %[[continue]]
   // CHECK: [[opencl_subgroup]]:
-  // CHECK: load atomic i32, i32* %{{.*}} syncscope("subgroup") seq_cst
+  // CHECK: load atomic i32, i32* %{{.*}} syncscope("wavefront") seq_cst
   // CHECK: br label %[[continue]]
   // CHECK: [[continue]]:
   int x = __opencl_atomic_load(i, memory_order_seq_cst, scope);
@@ -147,7 +147,7 @@
   // CHECK: [[MON_ALL]]:
   // CHECK: load atomic i32, i32* %{{.*}} monotonic
   // CHECK: [[MON_SUB]]:
-  // CHECK: load atomic i32, i32* %{{.*}} syncscope("subgroup") monotonic
+  // CHECK: load atomic i32, i32* %{{.*}} syncscope("wavefront") monotonic
   // CHECK: [[ACQ_WG]]:
   // CHECK: load atomic i32, i32* %{{.*}} syncscope("workgroup") acquire
   // CHECK: [[ACQ_DEV]]:
@@ -155,7 +155,7 @@
   // CHECK: [[ACQ_ALL]]:
   // CHECK: load atomic i32, i32* %{{.*}} acquire
   // CHECK: [[ACQ_SUB]]:
-  // CHECK: load atomic i32, i32* %{{.*}} syncscope("subgroup") acquire
+  // CHECK: load atomic i32, i32* %{{.*}} syncscope("wavefront") acquire
   // CHECK: [[SEQ_WG]]:
   // CHECK: load atomic i32, i32* %{{.*}} syncscope("workgroup") seq_cst
   // CHECK: [[SEQ_DEV]]:
@@ -163,7 +163,7 @@
   // CHECK: [[SEQ_ALL]]:
   // CHECK: load atomic i32, i32* %{{.*}} seq_cst
   // CHECK: [[SEQ_SUB]]:
-  // CHECK: load atomic i32, i32* %{{.*}} syncscope("subgroup") seq_cst
+  // CHECK: load atomic i32, i32* %{{.*}} syncscope("wavefront") seq_cst
   i

r355549 - AMDGPU: Fix the mapping of sub group sync scope

[Konstantin Zhuravlyov via cfe-commits]

Author: kzhuravl
Date: Wed Mar  6 12:54:48 2019
New Revision: 355549

URL: http://llvm.org/viewvc/llvm-project?rev=355549&view=rev
Log:
AMDGPU: Fix the mapping of sub group sync scope

Map memory_scope_sub_group to "wavefront" sync scope

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

Modified:
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/test/CodeGenOpenCL/atomic-ops.cl

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=355549&r1=355548&r2=355549&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Wed Mar  6 12:54:48 2019
@@ -7959,7 +7959,7 @@ AMDGPUTargetCodeGenInfo::getLLVMSyncScop
 Name = "";
 break;
   case SyncScope::OpenCLSubGroup:
-Name = "subgroup";
+Name = "wavefront";
   }
   return C.getOrInsertSyncScopeID(Name);
 }

Modified: cfe/trunk/test/CodeGenOpenCL/atomic-ops.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/atomic-ops.cl?rev=355549&r1=355548&r2=355549&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/atomic-ops.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/atomic-ops.cl Wed Mar  6 12:54:48 2019
@@ -41,7 +41,7 @@ void fi1(atomic_int *i) {
   // CHECK: load atomic i32, i32* %{{[.0-9A-Z_a-z]+}} seq_cst
   x = __opencl_atomic_load(i, memory_order_seq_cst, 
memory_scope_all_svm_devices);
 
-  // CHECK: load atomic i32, i32* %{{[.0-9A-Z_a-z]+}} syncscope("subgroup") 
seq_cst
+  // CHECK: load atomic i32, i32* %{{[.0-9A-Z_a-z]+}} syncscope("wavefront") 
seq_cst
   x = __opencl_atomic_load(i, memory_order_seq_cst, memory_scope_sub_group);
 }
 
@@ -109,7 +109,7 @@ void fi5(atomic_int *i, int scope) {
   // CHECK: load atomic i32, i32* %{{.*}} seq_cst
   // CHECK: br label %[[continue]]
   // CHECK: [[opencl_subgroup]]:
-  // CHECK: load atomic i32, i32* %{{.*}} syncscope("subgroup") seq_cst
+  // CHECK: load atomic i32, i32* %{{.*}} syncscope("wavefront") seq_cst
   // CHECK: br label %[[continue]]
   // CHECK: [[continue]]:
   int x = __opencl_atomic_load(i, memory_order_seq_cst, scope);
@@ -147,7 +147,7 @@ void fi6(atomic_int *i, int order, int s
   // CHECK: [[MON_ALL]]:
   // CHECK: load atomic i32, i32* %{{.*}} monotonic
   // CHECK: [[MON_SUB]]:
-  // CHECK: load atomic i32, i32* %{{.*}} syncscope("subgroup") monotonic
+  // CHECK: load atomic i32, i32* %{{.*}} syncscope("wavefront") monotonic
   // CHECK: [[ACQ_WG]]:
   // CHECK: load atomic i32, i32* %{{.*}} syncscope("workgroup") acquire
   // CHECK: [[ACQ_DEV]]:
@@ -155,7 +155,7 @@ void fi6(atomic_int *i, int order, int s
   // CHECK: [[ACQ_ALL]]:
   // CHECK: load atomic i32, i32* %{{.*}} acquire
   // CHECK: [[ACQ_SUB]]:
-  // CHECK: load atomic i32, i32* %{{.*}} syncscope("subgroup") acquire
+  // CHECK: load atomic i32, i32* %{{.*}} syncscope("wavefront") acquire
   // CHECK: [[SEQ_WG]]:
   // CHECK: load atomic i32, i32* %{{.*}} syncscope("workgroup") seq_cst
   // CHECK: [[SEQ_DEV]]:
@@ -163,7 +163,7 @@ void fi6(atomic_int *i, int order, int s
   // CHECK: [[SEQ_ALL]]:
   // CHECK: load atomic i32, i32* %{{.*}} seq_cst
   // CHECK: [[SEQ_SUB]]:
-  // CHECK: load atomic i32, i32* %{{.*}} syncscope("subgroup") seq_cst
+  // CHECK: load atomic i32, i32* %{{.*}} syncscope("wavefront") seq_cst
   int x = __opencl_atomic_load(i, order, scope);
 }
 


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

[PATCH] D58154: Add support for -fpermissive.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D58154#1419956 , @jyknight wrote:

> I also can't really tell what the intent is behind classifying some 
> diagnostics in Clang as ExtWarn,DefaultError and others Warning,DefaultError 
> -- or if there even is any useful purpose there at all. I note with 
> special-puzzlement the existence of both `ext_return_missing_expr` and 
> `warn_return_missing_expr`.


`ExtWarn,DefaultError` is for extensions that we disable by default. It should 
only ever fire on non-conforming code. Such extensions are implicitly enabled 
in system headers and can be explicitly enabled with a `-Wno-` flag.

`Warning,DefaultError` is for conforming code that we can accept, but that's 
doing something so egregious that we choose to reject it by default. Such cases 
should be incredibly rare. As above, such cases are permitted in system headers 
and the error can be downgraded via a -W flag.

`ext_return_missing_expr` is used in languages where a `return` statement with 
no expression is ill-formed (everything later than C90), and 
`warn_return_missing_expr` is used in the case where it's valid but is very 
likely to result in undefined behavior (C90). (This patch certainly has 
surprising behavior for these diagnostics -- demoting the former case to a 
warning but leaving the latter as an error -- which I think strongly suggests 
that this is the wrong approach for `-fpermissive`.)

> I guess my feeling now is that perhaps we should just eliminate that 
> categorization as meaningless, and just make -Wno-error=everything work 
> properly (for anyone who wants to not abort the compile on broken code, but 
> for whatever reason also loves to see build-spam so doesn't want 
> -Wno-everything).

The existing categories seem meaningful to me. I agree that making 
`-Wno-error=everything` work would probably be a good idea.

I don't have particularly strong feelings one way or the other about accepting 
`-fpermissive` at all. For GCC compatibility, it seems like a moderately useful 
thing to support, but I don't think we have any interest in accepting 
everything that GCC accepts under `-fpermissive`. Perhaps the better choice is 
to not provide the flag at all, rather than to provide something that has the 
same interface but doesn't accept the same code. If not that, making 
`-fpermissive` an alias for `-Wno-error=everything` is probably a better 
approach than that of this patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58154



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

[PATCH] D57662: [clang-tidy] Parallelize clang-tidy-diff.py

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis updated this revision to Diff 189562.
zinovy.nis added a comment.

- honest '--export-fixes': multiple yamls are merged into the resulting one.
- minor cosmetic fixes.


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

https://reviews.llvm.org/D57662

Files:
  clang-tidy/tool/clang-tidy-diff.py
  clang-tidy/tool/run-clang-tidy.py

Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -279,8 +279,8 @@
   if file_name_re.search(name):
 task_queue.put(name)
 
-# Wait for all threads to be done.
-task_queue.join()
+  # Wait for all threads to be done.
+  task_queue.join()
 if len(failed_files):
   return_code = 1
 
Index: clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tidy/tool/clang-tidy-diff.py
+++ clang-tidy/tool/clang-tidy-diff.py
@@ -24,10 +24,88 @@
 """
 
 import argparse
+import glob
 import json
+import multiprocessing
+import os
 import re
+import shutil
 import subprocess
 import sys
+import tempfile
+import threading
+import traceback
+import yaml
+
+is_py2 = sys.version[0] == '2'
+
+if is_py2:
+import Queue as queue
+else:
+import queue as queue
+
+
+def run_tidy(task_queue, lock, timeout):
+  watchdog = None
+  while True:
+command = task_queue.get()
+try:
+  proc = subprocess.Popen(command,
+  stdout=subprocess.PIPE,
+  stderr=subprocess.PIPE)
+
+  if timeout is not None:
+watchdog = threading.Timer(timeout, proc.kill)
+watchdog.start()
+
+  stdout, stderr = proc.communicate()
+
+  with lock:
+sys.stdout.write(stdout.decode('utf-8') + '\n')
+if stderr:
+  sys.stderr.write(stderr.decode('utf-8') + '\n')
+except Exception as e:
+  with lock:
+sys.stderr.write('Failed: ' + str(e) + ': '.join(command) + '\n')
+finally:
+  with lock:
+if (not timeout is None) and (not watchdog is None):
+  if not watchdog.is_alive():
+  sys.stderr.write('Terminated by timeout: ' +
+   ' '.join(command) + '\n')
+  watchdog.cancel()
+  task_queue.task_done()
+
+
+def start_workers(max_tasks, tidy_caller, task_queue, lock, timeout):
+  for _ in range(max_tasks):
+t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
+t.daemon = True
+t.start()
+
+def merge_replacement_files(tmpdir, mergefile):
+  """Merge all replacement files in a directory into a single file"""
+  # The fixes suggested by clang-tidy >= 4.0.0 are given under
+  # the top level key 'Diagnostics' in the output yaml files
+  mergekey="Diagnostics"
+  merged=[]
+  for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')):
+content = yaml.safe_load(open(replacefile, 'r'))
+if not content:
+  continue # Skip empty files.
+merged.extend(content.get(mergekey, []))
+
+  if merged:
+# MainSourceFile: The key is required by the definition inside
+# include/clang/Tooling/ReplacementsYaml.h, but the value
+# is actually never used inside clang-apply-replacements,
+# so we set it to '' here.
+output = { 'MainSourceFile': '', mergekey: merged }
+with open(mergefile, 'w') as out:
+  yaml.safe_dump(output, out)
+  else:
+# Empty the file:
+open(mergefile, 'w').close()
 
 
 def main():
@@ -48,6 +126,10 @@
   help='custom pattern selecting file paths to check '
   '(case insensitive, overridden by -regex)')
 
+  parser.add_argument('-j', type=int, default=1,
+  help='number of tidy instances to be run in parallel.')
+  parser.add_argument('-timeout', type=int, default=None,
+  help='timeout per each file in seconds.')
   parser.add_argument('-fix', action='store_true', default=False,
   help='apply suggested fixes')
   parser.add_argument('-checks',
@@ -84,7 +166,7 @@
 match = re.search('^\+\+\+\ \"?(.*?/){%s}([^ \t\n\"]*)' % args.p, line)
 if match:
   filename = match.group(2)
-if filename == None:
+if filename is None:
   continue
 
 if args.regex is not None:
@@ -102,44 +184,83 @@
 line_count = int(match.group(3))
   if line_count == 0:
 continue
-  end_line = start_line + line_count - 1;
+  end_line = start_line + line_count - 1
   lines_by_file.setdefault(filename, []).append([start_line, end_line])
 
-  if len(lines_by_file) == 0:
+  if not any(lines_by_file):
 print("No relevant changes found.")
 sys.exit(0)
 
-  line_filter_json = json.dumps(
-[{"name" : name, "lines" : lines_by_file[name]} for name in lines_by_file],
-separators = (',', ':'))
+  max_task_count = args.j
+  if max_task_count == 0:
+  max_task_count = multipro

[PATCH] D59032: Passthrough compiler launcher

[JF Bastien via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC355547: Passthrough compiler launcher (authored by jfb, 
committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59032?vs=189537&id=189563#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59032

Files:
  CMakeLists.txt


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -666,6 +666,8 @@
 LLVM_VERSION_SUFFIX
 LLVM_BINUTILS_INCDIR
 CLANG_REPOSITORY_STRING
+CMAKE_C_COMPILER_LAUNCHER
+CMAKE_CXX_COMPILER_LAUNCHER
 CMAKE_MAKE_PROGRAM
 CMAKE_OSX_ARCHITECTURES
 LLVM_ENABLE_PROJECTS


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -666,6 +666,8 @@
 LLVM_VERSION_SUFFIX
 LLVM_BINUTILS_INCDIR
 CLANG_REPOSITORY_STRING
+CMAKE_C_COMPILER_LAUNCHER
+CMAKE_CXX_COMPILER_LAUNCHER
 CMAKE_MAKE_PROGRAM
 CMAKE_OSX_ARCHITECTURES
 LLVM_ENABLE_PROJECTS
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r355547 - Passthrough compiler launcher

[JF Bastien via cfe-commits]

Author: jfb
Date: Wed Mar  6 12:36:00 2019
New Revision: 355547

URL: http://llvm.org/viewvc/llvm-project?rev=355547&view=rev
Log:
Passthrough compiler launcher

Summary: Not having this seems like an oversight, and makes stage2 builds odd.

Reviewers: ddunbar, dexonsmith

Subscribers: mgorny, jkorous, jdoerfert, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/CMakeLists.txt

Modified: cfe/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=355547&r1=355546&r2=355547&view=diff
==
--- cfe/trunk/CMakeLists.txt (original)
+++ cfe/trunk/CMakeLists.txt Wed Mar  6 12:36:00 2019
@@ -666,6 +666,8 @@ if (CLANG_ENABLE_BOOTSTRAP)
 LLVM_VERSION_SUFFIX
 LLVM_BINUTILS_INCDIR
 CLANG_REPOSITORY_STRING
+CMAKE_C_COMPILER_LAUNCHER
+CMAKE_CXX_COMPILER_LAUNCHER
 CMAKE_MAKE_PROGRAM
 CMAKE_OSX_ARCHITECTURES
 LLVM_ENABLE_PROJECTS


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

[PATCH] D59034: Delete x86_64 ShadowCallStack support

[Vlad Tsyrklevich via Phabricator via cfe-commits]

vlad.tsyrklevich created this revision.
vlad.tsyrklevich added a reviewer: pcc.
Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, jdoerfert, 
hiraditya, javed.absar, mgorny.
Herald added projects: clang, Sanitizers, LLVM.

ShadowCallStack on x86_64 suffered from the same racy security issues as
Return Flow Guard and had performance overhead as high as 13% depending
on the benchmark. x86_64 ShadowCallStack was always an experimental
feature and never shipped a runtime required to support it, as such
there are no expected downstream users.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59034

Files:
  clang/docs/ShadowCallStack.rst
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/test/shadowcallstack/libc_support.h
  compiler-rt/test/shadowcallstack/lit.cfg
  compiler-rt/test/shadowcallstack/minimal_runtime.h
  compiler-rt/test/shadowcallstack/overflow-aarch64.c
  compiler-rt/test/shadowcallstack/overflow-x86_64.c
  compiler-rt/test/shadowcallstack/overflow.c
  llvm/lib/Target/X86/CMakeLists.txt
  llvm/lib/Target/X86/ShadowCallStack.cpp
  llvm/lib/Target/X86/X86.h
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/O3-pipeline.ll
  llvm/test/CodeGen/X86/shadow-call-stack.mir
  llvm/utils/gn/secondary/llvm/lib/Target/X86/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/lib/Target/X86/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/lib/Target/X86/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/lib/Target/X86/BUILD.gn
@@ -76,7 +76,6 @@
 deps += [ ":X86GenFoldTables" ]
   }
   sources = [
-"ShadowCallStack.cpp",
 "X86AsmPrinter.cpp",
 "X86AvoidStoreForwardingBlocks.cpp",
 "X86CallFrameOptimization.cpp",
Index: llvm/test/CodeGen/X86/shadow-call-stack.mir
===
--- llvm/test/CodeGen/X86/shadow-call-stack.mir
+++ /dev/null
@@ -1,212 +0,0 @@
-# RUN: llc -mtriple=x86_64-unknown-linux-gnu -run-pass shadow-call-stack -verify-machineinstrs -o - %s | FileCheck %s
 |
-
-  define void @no_return() #0 { ret void }
-  define void @normal_return() #0 { ret void }
-  define void @normal_return_leaf_func() #0 { ret void }
-  define void @short_leaf_func() #0 { ret void }
-  define void @normal_tail_call() #0 { ret void }
-  define void @r11_tail_call() #0 { ret void }
-  define void @conditional_tail_call() #0 { ret void }
-  define void @r10_live_in() #0 { ret void }
-
-  attributes #0 = { shadowcallstack }
-
-...

-# CHECK-LABEL: name: no_return
-name: no_return
-tracksRegLiveness: true
-frameInfo:
-  adjustsStack: true # not a leaf function
-body: |
-  ; CHECK: bb.0:
-  bb.0:
-; CHECK-NEXT: $eax = MOV32ri 13
-$eax = MOV32ri 13
-...

-# CHECK-LABEL: name: normal_return
-name: normal_return
-tracksRegLiveness: true
-frameInfo:
-  adjustsStack: true # not a leaf function
-body: |
-  ; CHECK: bb.0:
-  bb.0:
-; CHECK: $r10 = MOV64rm $rsp, 1, $noreg, 0, $noreg
-; CHECK-NEXT: $r11 = XOR64rr undef $r11, undef $r11, implicit-def $eflags
-; CHECK-NEXT: ADD64mi8 $r11, 1, $noreg, 0, $gs, 8, implicit-def $eflags
-; CHECK-NEXT: $r11 = MOV64rm $r11, 1, $noreg, 0, $gs
-; CHECK-NEXT: MOV64mr $r11, 1, $noreg, 0, $gs, $r10
-; CHECK-NEXT: $eax = MOV32ri 13
-$eax = MOV32ri 13
-
-; CHECK-NEXT: $r11 = XOR64rr undef $r11, undef $r11, implicit-def $eflags
-; CHECK-NEXT: $r10 = MOV64rm $r11, 1, $noreg, 0, $gs
-; CHECK-NEXT: $r10 = MOV64rm $r10, 1, $noreg, 0, $gs
-; CHECK-NEXT: SUB64mi8 $r11, 1, $noreg, 0, $gs, 8, implicit-def $eflags
-; CHECK-NEXT: CMP64mr $rsp, 1, $noreg, 0, $noreg, $r10, implicit-def $eflags
-; CHECK-NEXT: JNE_1 %bb.1, implicit $eflags
-; CHECK-NEXT: RETQ $eax
-RETQ $eax
-
-; CHECK: bb.1:
-; CHECK-NEXT; TRAP
-...

-# CHECK-LABEL: name: normal_return_leaf_func
-name: normal_return_leaf_func
-tracksRegLiveness: true
-frameInfo:
-  adjustsStack: false # leaf function
-body: |
-  ; CHECK: bb.0:
-  ; CHECK: liveins: $rcx
-  bb.0:
-liveins: $rcx
-
-; CHECK: $rdx = MOV64rm $rsp, 1, $noreg, 0, $noreg
-; CHECK-NEXT: $eax = MOV32ri 0
-$eax = MOV32ri 0
-; CHECK-NEXT: CMP64ri8 $rcx, 5, implicit-def $eflags
-CMP64ri8 $rcx, 5, implicit-def $eflags
-; CHECK-NEXT: JA_1 %bb.1, implicit $eflags
-JA_1 %bb.1, implicit $eflags
-; CHECK-NEXT: JMP_1 %bb.2
-JMP_1 %bb.2
-
-  ; CHECK: bb.1
-  ; CHECK: liveins: $eax, $rdx
-  bb.1:
-liveins: $eax
-
-; CHECKT: $eax = MOV32ri 1
-$eax = MOV32ri 1
-
-  ; CHECK: bb.2
-  ; CHECK: liveins: $eax, $rdx
-  bb.2:
-liveins: $eax
-
-; CHECK: CMP64mr $rsp, 1, $noreg, 0, $noreg, $rdx, implicit-def $eflags
-; CHECK-NEXT: JNE_1 %bb.3, implicit $eflags
-; CHECK-NEXT: RETQ $eax
-RETQ $eax
-
-; CHECK: bb.3:
-; CHECK-NEXT; TRAP
-...

-# CHECK-LABEL: name: short_leaf_func
-name: short_leaf_func
-tracksRegLiveness: true
-frameInfo:
-  adj

r355537 - Revert "[IR][ARM] Add function pointer alignment to datalayout"

[Mitch Phillips via cfe-commits]

Author: hctim
Date: Wed Mar  6 11:17:18 2019
New Revision: 355537

URL: http://llvm.org/viewvc/llvm-project?rev=355537&view=rev
Log:
Revert "[IR][ARM] Add function pointer alignment to datalayout"

This reverts commit 2391bfca97290181ae65796ea6da135d1b6d037b.

This reverts rL355522 (https://reviews.llvm.org/D57335).

Kills buildbots that use '-Werror' with the following error:

/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm/lib/IR/Value.cpp:657:7:
 error: default label in switch which covers all enumeration values 
[-Werror,-Wcovered-switch-default]

See buildbots 
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/30200/steps/check-llvm%20asan/logs/stdio
 for more information.

Modified:
cfe/trunk/lib/Basic/Targets/ARM.cpp
cfe/trunk/test/CodeGen/armv7k-abi.c
cfe/trunk/test/CodeGen/target-data.c

Modified: cfe/trunk/lib/Basic/Targets/ARM.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/ARM.cpp?rev=355537&r1=355536&r2=355537&view=diff
==
--- cfe/trunk/lib/Basic/Targets/ARM.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/ARM.cpp Wed Mar  6 11:17:18 2019
@@ -40,14 +40,13 @@ void ARMTargetInfo::setABIAAPCS() {
   // so set preferred for small types to 32.
   if (T.isOSBinFormatMachO()) {
 resetDataLayout(BigEndian
-? "E-m:o-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
-: 
"e-m:o-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64");
+? "E-m:o-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+: "e-m:o-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64");
   } else if (T.isOSWindows()) {
 assert(!BigEndian && "Windows on ARM does not support big endian");
 resetDataLayout("e"
 "-m:w"
 "-p:32:32"
-"-Fi8"
 "-i64:64"
 "-v128:64:128"
 "-a:0:32"
@@ -55,11 +54,11 @@ void ARMTargetInfo::setABIAAPCS() {
 "-S64");
   } else if (T.isOSNaCl()) {
 assert(!BigEndian && "NaCl on ARM does not support big endian");
-resetDataLayout("e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S128");
+resetDataLayout("e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S128");
   } else {
 resetDataLayout(BigEndian
-? "E-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
-: 
"e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64");
+? "E-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+: "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64");
   }
 
   // FIXME: Enumerated types are variable width in straight AAPCS.
@@ -88,17 +87,17 @@ void ARMTargetInfo::setABIAPCS(bool IsAA
 
   if (T.isOSBinFormatMachO() && IsAAPCS16) {
 assert(!BigEndian && "AAPCS16 does not support big-endian");
-resetDataLayout("e-m:o-p:32:32-Fi8-i64:64-a:0:32-n32-S128");
+resetDataLayout("e-m:o-p:32:32-i64:64-a:0:32-n32-S128");
   } else if (T.isOSBinFormatMachO())
 resetDataLayout(
 BigEndian
-? 
"E-m:o-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
-: 
"e-m:o-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32");
+? "E-m:o-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
+: "e-m:o-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32");
   else
 resetDataLayout(
 BigEndian
-? 
"E-m:e-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
-: 
"e-m:e-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32");
+? "E-m:e-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
+: "e-m:e-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32");
 
   // FIXME: Override "preferred align" for double and long long.
 }
@@ -1056,7 +1055,7 @@ CygwinARMTargetInfo::CygwinARMTargetInfo
   this->WCharType = TargetInfo::UnsignedShort;
   TLSSupported = false;
   DoubleAlign = LongLongAlign = 64;
-  resetDataLayout("e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64");
+  resetDataLayout("e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64");
 }
 
 void CygwinARMTargetInfo::getTargetDefines(const LangOptions &Opts,

Modified: cfe/trunk/test/CodeGen/armv7k-abi.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/armv7k-abi.c?rev=355537&r1=355536&r2=355537&view=diff
==
--- cfe/trunk/test/CodeGen/armv7k-abi.c (original)
+++ cfe/trunk/test/CodeGen/armv7k-abi.c Wed Mar  6 11:17:18 2019
@@ -4,7 +4,7 @@
 
 // Make sure 64 and 128 bit types are naturally aligned by the v7k ABI:
 
-// CHECK: target datalayout = "e-m:o-p:32:32-Fi8-i64:64-a:0:32-n32-S128"
+// CHECK: target datalayout = "e-m:o-p:32:32-i64:64-a:0:32-n32-S128"
 
 typedef struct {
   float arr[4

[PATCH] D59028: [OpenMP] Enable on device linking with NVLINK to ignore dynamic libraries

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added a comment.

For now this patch doesn't make sense until the main linking patch lands but 
it's a case we need to keep in mind.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59028



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

[PATCH] D59038: [8.0 Regression] Fix handling of `__builtin_constant_p` inside the enable_if attribute.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF created this revision.
EricWF added reviewers: rsmith, hans, sbenza.
Herald added a subscriber: kristina.
Herald added a project: clang.

The following code is accepted by Clang 7 and prior but rejected by the 
upcoming 8 release and in trunk [1]

  // error {{never produces a constant expression}}
  void foo(const char* s) __attribute__((enable_if(__builtin_constant_p(*s) == 
false, "trap"))) {}
  void test() { foo("abc"); }

Prior to Clang 8, the call to `__builtin_constant_p` was a constant expression 
returning false. Currently, it's not a valid constant expression.

The bug is caused because we failed to set `InConstantContext` when attempting 
to evaluate unevaluated constant expressions.

[1]  https://godbolt.org/z/ksAjmq


Repository:
  rC Clang

https://reviews.llvm.org/D59038

Files:
  lib/AST/ExprConstant.cpp
  test/SemaCXX/constant-expression-cxx1y.cpp
  test/SemaCXX/enable_if.cpp


Index: test/SemaCXX/enable_if.cpp
===
--- test/SemaCXX/enable_if.cpp
+++ test/SemaCXX/enable_if.cpp
@@ -514,3 +514,11 @@
 
   static_assert(is_same<__typeof__(foo)*, decltype(&foo)>::value, "");
 }
+
+namespace InConstantContext {
+void foo(const char *s) 
__attribute__((enable_if(((void)__builtin_constant_p(*s), true), "trap"))) {}
+
+void test() {
+  InConstantContext::foo("abc");
+}
+} // namespace InConstantContext
Index: test/SemaCXX/constant-expression-cxx1y.cpp
===
--- test/SemaCXX/constant-expression-cxx1y.cpp
+++ test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1135,3 +1135,7 @@
   return __builtin_constant_p(*__s);
 }
 constexpr bool n = indirect_builtin_constant_p("a");
+
+__attribute__((enable_if(indirect_builtin_constant_p("a") == n, "OK")))
+int test_in_enable_if() { return 0; }
+int n2 = test_in_enable_if();
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -11767,6 +11767,7 @@
 const Expr *This) const {
   Expr::EvalStatus Status;
   EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantExpressionUnevaluated);
+  Info.InConstantContext = true;
 
   LValue ThisVal;
   const LValue *ThisPtr = nullptr;
@@ -11850,6 +11851,7 @@
 
   EvalInfo Info(FD->getASTContext(), Status,
 EvalInfo::EM_PotentialConstantExpressionUnevaluated);
+  Info.InConstantContext = true;
 
   // Fabricate a call stack frame to give the arguments a plausible cover 
story.
   ArrayRef Args;


Index: test/SemaCXX/enable_if.cpp
===
--- test/SemaCXX/enable_if.cpp
+++ test/SemaCXX/enable_if.cpp
@@ -514,3 +514,11 @@
 
   static_assert(is_same<__typeof__(foo)*, decltype(&foo)>::value, "");
 }
+
+namespace InConstantContext {
+void foo(const char *s) __attribute__((enable_if(((void)__builtin_constant_p(*s), true), "trap"))) {}
+
+void test() {
+  InConstantContext::foo("abc");
+}
+} // namespace InConstantContext
Index: test/SemaCXX/constant-expression-cxx1y.cpp
===
--- test/SemaCXX/constant-expression-cxx1y.cpp
+++ test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1135,3 +1135,7 @@
   return __builtin_constant_p(*__s);
 }
 constexpr bool n = indirect_builtin_constant_p("a");
+
+__attribute__((enable_if(indirect_builtin_constant_p("a") == n, "OK")))
+int test_in_enable_if() { return 0; }
+int n2 = test_in_enable_if();
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -11767,6 +11767,7 @@
 const Expr *This) const {
   Expr::EvalStatus Status;
   EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantExpressionUnevaluated);
+  Info.InConstantContext = true;
 
   LValue ThisVal;
   const LValue *ThisPtr = nullptr;
@@ -11850,6 +11851,7 @@
 
   EvalInfo Info(FD->getASTContext(), Status,
 EvalInfo::EM_PotentialConstantExpressionUnevaluated);
+  Info.InConstantContext = true;
 
   // Fabricate a call stack frame to give the arguments a plausible cover story.
   ArrayRef Args;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56044: [Driver] Support object files in addition to static and shared libraries in compiler-rt

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 189552.
phosek marked 2 inline comments as done.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.

Repository:
  rC Clang

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

https://reviews.llvm.org/D56044

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MipsLinux.cpp
  clang/lib/Driver/ToolChains/MipsLinux.h

Index: clang/lib/Driver/ToolChains/MipsLinux.h
===
--- clang/lib/Driver/ToolChains/MipsLinux.h
+++ clang/lib/Driver/ToolChains/MipsLinux.h
@@ -37,8 +37,9 @@
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) const override;
 
-  std::string getCompilerRT(const llvm::opt::ArgList &Args, StringRef Component,
-bool Shared = false) const override;
+  std::string
+  getCompilerRT(const llvm::opt::ArgList &Args, StringRef Component,
+FileType Type = ToolChain::FT_Static) const override;
 
   std::string computeSysRoot() const override;
 
Index: clang/lib/Driver/ToolChains/MipsLinux.cpp
===
--- clang/lib/Driver/ToolChains/MipsLinux.cpp
+++ clang/lib/Driver/ToolChains/MipsLinux.cpp
@@ -118,11 +118,23 @@
 
 std::string MipsLLVMToolChain::getCompilerRT(const ArgList &Args,
  StringRef Component,
- bool Shared) const {
+ FileType Type) const {
   SmallString<128> Path(getDriver().ResourceDir);
   llvm::sys::path::append(Path, SelectedMultilib.osSuffix(), "lib" + LibSuffix,
   getOS());
-  llvm::sys::path::append(Path, Twine("libclang_rt." + Component + "-" +
-  "mips" + (Shared ? ".so" : ".a")));
+  const char *Suffix;
+  switch (Type) {
+  case ToolChain::FT_Object:
+Suffix = ".o";
+break;
+  case ToolChain::FT_Static:
+Suffix = ".a";
+break;
+  case ToolChain::FT_Shared:
+Suffix = ".so";
+break;
+  }
+  llvm::sys::path::append(
+  Path, Twine("libclang_rt." + Component + "-" + "mips" + Suffix));
   return Path.str();
 }
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -248,8 +248,8 @@
 
   if (Sanitize.needsAsanRt()) {
 // MinGW always links against a shared MSVCRT.
-CmdArgs.push_back(
-TC.getCompilerRTArgString(Args, "asan_dynamic", true));
+CmdArgs.push_back(TC.getCompilerRTArgString(Args, "asan_dynamic",
+ToolChain::FT_Shared));
 CmdArgs.push_back(
 TC.getCompilerRTArgString(Args, "asan_dynamic_runtime_thunk"));
 CmdArgs.push_back(Args.MakeArgString("--require-defined"));
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -535,7 +535,8 @@
   // Wrap any static runtimes that must be forced into executable in
   // whole-archive.
   if (IsWhole) CmdArgs.push_back("--whole-archive");
-  CmdArgs.push_back(TC.getCompilerRTArgString(Args, Sanitizer, IsShared));
+  CmdArgs.push_back(TC.getCompilerRTArgString(
+  Args, Sanitizer, IsShared ? ToolChain::FT_Shared : ToolChain::FT_Static));
   if (IsWhole) CmdArgs.push_back("--no-whole-archive");
 
   if (IsShared) {
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -362,16 +362,27 @@
 }
 
 std::string ToolChain::getCompilerRT(const ArgList &Args, StringRef Component,
- bool Shared) const {
+ FileType Type) const {
   const llvm::Triple &TT = getTriple();
   bool IsITANMSVCWindows =
   TT.isWindowsMSVCEnvironment() || TT.isWindowsItaniumEnvironment();
 
-  const char *Prefix = IsITANMSVCWindows ? "" : "lib";
-  const char *Suffix = Shared ? (Triple.isOSWindows() ? ".lib" : ".so")
-  : (IsITANMSVCWindows ? ".lib" : ".a");
-  if (Shared && Triple.isWindowsGNUEnvironment())
-Suffix = ".dll.a";
+  const char *Prefix =
+  IsITANMSVCWindows || Type == ToolChain::FT_Object ? "" : "lib";
+  const char *Suffix;
+  switch (Type) {
+  case ToolChain::FT_Object:
+Suffix = IsITANMSVCWindows ? ".obj" : ".o";
+break;
+  case ToolChain::FT_Static:
+Suffix = IsITANMSVCWindows ? ".lib" : ".a";
+break;
+  case ToolChain::FT_Shared:
+

[PATCH] D59013: [CMake][runtimes] Move libunwind, libc++abi and libc++ to lib/ and include/

[Petr Hosek via Phabricator via cfe-commits]

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



Comment at: libcxx/CMakeLists.txt:421
+  set(DEFAULT_INSTALL_PREFIX 
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
+  set(LIBCXX_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}${LIBCXX_LIBDIR_SUFFIX})
+  set(LIBCXX_HEADER_DIR ${LLVM_BINARY_DIR})

smeenai wrote:
> Having the libdir suffix after the target triple looks weird ... also, the 
> driver code doesn't appear to be taking the possibility of a suffix into 
> account? I might be missing something here.
This is a bit of a hack, we allow building runtimes with sanitizer 
instrumentation and we install those sanitized versions into 
`lib//`, e.g. `lib/x86_64-fuchsia/asan`. Those are never 
linked against (the ABI should be the same as for the non-sanitized version), 
they're only intended for packaging, hence the missing driver logic. We use the 
`_LIBDIR_SUFFIX` to set the `` portion of the install path, 
see 
https://github.com/llvm/llvm-project/blob/master/llvm/runtimes/CMakeLists.txt#L517.
 However, if anyone ever tries to use both the `LIBDIR_SUFFIX` and 
instrumentation, things are going to break. Maybe we should introduce another 
variable for this, e.g. `_LIBDIR_VARIANT`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59013



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

[PATCH] D39050: Add index-while-building support to Clang

[Argyrios Kyrtzidis via Phabricator via cfe-commits]

akyrtzi added inline comments.



Comment at: include/clang/Frontend/FrontendOptions.h:380
+  /// Whether to ignore system files when writing out index data
+  unsigned IndexIgnoreSystemSymbols : 1;
+  /// Whether to include the codegen name of symbols in the index data

gribozavr wrote:
> Would it make more sense to flip this boolean to positive?  
> "IndexIncludeSystemSymbols"?
@jkorous I noticed this name can be misleading, it may seem as if what this 
does is "avoid indexing system symbol occurrences" but what it actually does is 
"avoid indexing symbol occurrences from system files". We should rename it to 
"IndexIgnoreSystemHeaders" or "IndexIncludeSystemHeaders" per Dmitri's 
suggestion.


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

https://reviews.llvm.org/D39050



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

[PATCH] D59032: Passthrough compiler launcher

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM.  Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D59032



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

[PATCH] D59034: Delete x86_64 ShadowCallStack support

[Peter Collingbourne via Phabricator via cfe-commits]

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

Thanks for cleaning this up.




Comment at: clang/docs/ShadowCallStack.rst:23
+to have critical performance and security deficiencies--it was removed in
+LLVM 9.0.
 

Should we keep a link to the Clang 7.0.1 docs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59034



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

[PATCH] D58216: Support attribute used in member funcs of class templates II

[Rafael Auler via Phabricator via cfe-commits]

rafauler added a subscriber: davezarzycki.
rafauler added a comment.

Both approaches make sense to me. I'll re-land the previous patch in favor of 
gcc compatibility, since the semantics of attribute used in member functions of 
class templates were first implemented in gcc.

@davezarzycki  Heads up that this will land again. Can you change the code in 
swift to use the attribute used in the declaration of the specialization, not 
in the declaration of the template? (that is, if you really need the attribute, 
of course).


Repository:
  rC Clang

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

https://reviews.llvm.org/D58216



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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Jason Liu via Phabricator via cfe-commits]

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



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079
+  if (log)
+log->Printf("sorry: unimplemented for XCOFF");
+  return false;

hubert.reinterpretcast wrote:
> davide wrote:
> > hubert.reinterpretcast wrote:
> > > JDevlieghere wrote:
> > > > jasonliu wrote:
> > > > > JDevlieghere wrote:
> > > > > > jasonliu wrote:
> > > > > > > apaprocki wrote:
> > > > > > > > No need to be `sorry:` :) This should probably just say `error: 
> > > > > > > > XCOFF is unimplemented` to be more direct in case anything is 
> > > > > > > > expecting "error:" in the output.
> > > > > > > Sure. Will address in next revision.
> > > > > > Just bundle this with the WASM case, the error message is correct 
> > > > > > for both.
> > > > > I think they are different. 
> > > > > The error message for WASM seems to suggest that it will never ever 
> > > > > get supported on WASM. 
> > > > > But it is not the case for XCOFF, we want to indicate that it is not 
> > > > > implemented yet.  
> > > > I don't think the error message suggests that at all, and it's 
> > > > definitely not true. At this point neither XCOFF nor WASM is supported, 
> > > > and that's exactly what the log message says.
> > > > 
> > > I agree that the error message for WASM does not indicate that the lack 
> > > of support is inherent or intended to be permanent; however, it is not 
> > > indicative either of an intent to implement the support. I am not sure 
> > > what the intent is for WASM, but I do know that the intent for XCOFF is 
> > > to eventually implement the support. I do not see how using an ambiguous 
> > > message in this commit (when we know what the intent is) is superior to 
> > > the alternative of having an unambiguous message.
> > I think we should keep this consistent with the other target so my vote is 
> > for grouping XCOFF with WASM. After all, if it's going to be implemented 
> > soon, the message will go away :)
> Well, I don't know about "soon"...
> Using the WASM message for XCOFF is not actually wrong; so, I can be okay 
> with it.
Okay. Shared message it is. 



Comment at: llvm/lib/MC/MCContext.cpp:165
+case MCObjectFileInfo::IsXCOFF:
+  // TODO: Need to implement class MCSymbolXCOFF.
+  break;

sfertile wrote:
> jasonliu wrote:
> > JDevlieghere wrote:
> > > See previous comment.
> > It is certain that we will need MCSymbolXCOFF. But before we run into cases 
> > where we actually need a MCSymbolXCOFF, we could use the generic MCSymbol 
> > first for XCOFF platform. So I don't want to put a llvm_unreachable here. 
> Would it make sense to add an llvm_unreachable now, and the first patch that 
> actually uses an MCSymbol stubs out the class and removes the unreachable?
The first patch that uses MCSymbol do not necessarily need to stub out 
MCSymbolXCOFF, as MCSymbol seems to be generic and usable until we are doing 
some XCOFF specific things that needs to be represented by MCSymbolXCOFF. If 
the intention is MCSymbol should never get used, and different object file 
should have their own MCSymbolXXX class from the start, then I could add in the 
llvm_unreachable here, and I would also propose to replace the "return new 
(Name, *this) MCSymbol(MCSymbol::SymbolKindUnset, Name, IsTemporary);" with an 
llvm_unreachable as well. 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930



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

[PATCH] D39050: Add index-while-building support to Clang

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

I'll address comments for this patch in the new set of patches.

@gribozavr I haven't put up this part of code for the new round of review yet. 
I will keep this on mind.

@mgrang This already landed in edbbe470f66 
 as 
`clang/lib/Index/FileIndexRecord.cpp` but luckily the implementation isn't 
using `sort()` at all. Thanks for pointing this out anyway!


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

https://reviews.llvm.org/D39050



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

[PATCH] D59028: [OpenMP] Enable on device linking with NVLINK to ignore dynamic libraries

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:707
   CmdArgs.push_back(StaticLibName);
+} else if (OrigInputFileName.endswith(".so")) {
+  // Only static libraries are supported.

gtbercea wrote:
> ABataev wrote:
> > gtbercea wrote:
> > > ABataev wrote:
> > > > gtbercea wrote:
> > > > > ABataev wrote:
> > > > > > I would suggest to use `types::lookupTypeForExtension` function and 
> > > > > > compare the result with the `types::TY_Object`.
> > > > > Isn't that too broad? I only want .so files.
> > > > I don't think nvlink supports anything else except for objects. Also, 
> > > > in future, we could extend it if it is required.
> > > But won't this exclude .o files too? I don't want to ignore those.
> > This function returns `TY_Object` for `.o` and `.obj` extensions (on 
> > Windows)
> Oh now I think I understand what you mean. You mean ignore everything that's 
> not a TY_Object? That doesn't work here because I am looking at the original 
> input file name.
Yes, but you can use `types::lookupTypeForExtension` function to classify the 
original input file name.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59028



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

[PATCH] D59028: [OpenMP] Enable on device linking with NVLINK to ignore dynamic libraries

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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



Comment at: lib/Driver/ToolChains/Cuda.cpp:707
   CmdArgs.push_back(StaticLibName);
+} else if (OrigInputFileName.endswith(".so")) {
+  // Only static libraries are supported.

ABataev wrote:
> gtbercea wrote:
> > ABataev wrote:
> > > gtbercea wrote:
> > > > ABataev wrote:
> > > > > I would suggest to use `types::lookupTypeForExtension` function and 
> > > > > compare the result with the `types::TY_Object`.
> > > > Isn't that too broad? I only want .so files.
> > > I don't think nvlink supports anything else except for objects. Also, in 
> > > future, we could extend it if it is required.
> > But won't this exclude .o files too? I don't want to ignore those.
> This function returns `TY_Object` for `.o` and `.obj` extensions (on Windows)
Oh now I think I understand what you mean. You mean ignore everything that's 
not a TY_Object? That doesn't work here because I am looking at the original 
input file name.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59028



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

[PATCH] D39050: Add index-while-building support to Clang

[Jan Korous via Phabricator via cfe-commits]

jkorous abandoned this revision.
jkorous added a comment.

It's time to officially abandon these patches in favor of new push for 
upstreaming index-while-building.

Current reviews in progress
https://reviews.llvm.org/D58749
https://reviews.llvm.org/D58418

RFC
http://lists.llvm.org/pipermail/cfe-dev/2019-February/061432.html


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

https://reviews.llvm.org/D39050



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

[PATCH] D59032: Passthrough compiler launcher

[JF Bastien via Phabricator via cfe-commits]

jfb created this revision.
jfb added reviewers: ddunbar, dexonsmith.
Herald added subscribers: cfe-commits, jdoerfert, jkorous, mgorny.
Herald added a project: clang.

Not having this seems like an oversight, and makes stage2 builds odd.


Repository:
  rC Clang

https://reviews.llvm.org/D59032

Files:
  CMakeLists.txt


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -666,6 +666,8 @@
 LLVM_VERSION_SUFFIX
 LLVM_BINUTILS_INCDIR
 CLANG_REPOSITORY_STRING
+CMAKE_C_COMPILER_LAUNCHER
+CMAKE_CXX_COMPILER_LAUNCHER
 CMAKE_MAKE_PROGRAM
 CMAKE_OSX_ARCHITECTURES
 LLVM_ENABLE_PROJECTS


Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -666,6 +666,8 @@
 LLVM_VERSION_SUFFIX
 LLVM_BINUTILS_INCDIR
 CLANG_REPOSITORY_STRING
+CMAKE_C_COMPILER_LAUNCHER
+CMAKE_CXX_COMPILER_LAUNCHER
 CMAKE_MAKE_PROGRAM
 CMAKE_OSX_ARCHITECTURES
 LLVM_ENABLE_PROJECTS
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59028: [OpenMP] Enable on device linking with NVLINK to ignore dynamic libraries

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:707
   CmdArgs.push_back(StaticLibName);
+} else if (OrigInputFileName.endswith(".so")) {
+  // Only static libraries are supported.

gtbercea wrote:
> ABataev wrote:
> > gtbercea wrote:
> > > ABataev wrote:
> > > > I would suggest to use `types::lookupTypeForExtension` function and 
> > > > compare the result with the `types::TY_Object`.
> > > Isn't that too broad? I only want .so files.
> > I don't think nvlink supports anything else except for objects. Also, in 
> > future, we could extend it if it is required.
> But won't this exclude .o files too? I don't want to ignore those.
This function returns `TY_Object` for `.o` and `.obj` extensions (on Windows)


Repository:
  rC Clang

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

https://reviews.llvm.org/D59028



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

[PATCH] D59028: [OpenMP] Enable on device linking with NVLINK to ignore dynamic libraries

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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



Comment at: lib/Driver/ToolChains/Cuda.cpp:707
   CmdArgs.push_back(StaticLibName);
+} else if (OrigInputFileName.endswith(".so")) {
+  // Only static libraries are supported.

ABataev wrote:
> gtbercea wrote:
> > ABataev wrote:
> > > I would suggest to use `types::lookupTypeForExtension` function and 
> > > compare the result with the `types::TY_Object`.
> > Isn't that too broad? I only want .so files.
> I don't think nvlink supports anything else except for objects. Also, in 
> future, we could extend it if it is required.
But won't this exclude .o files too? I don't want to ignore those.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59028



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

[PATCH] D59028: [OpenMP] Enable on device linking with NVLINK to ignore dynamic libraries

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:707
   CmdArgs.push_back(StaticLibName);
+} else if (OrigInputFileName.endswith(".so")) {
+  // Only static libraries are supported.

gtbercea wrote:
> ABataev wrote:
> > I would suggest to use `types::lookupTypeForExtension` function and compare 
> > the result with the `types::TY_Object`.
> Isn't that too broad? I only want .so files.
I don't think nvlink supports anything else except for objects. Also, in 
future, we could extend it if it is required.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59028



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

[PATCH] D59028: [OpenMP] Enable on device linking with NVLINK to ignore dynamic libraries

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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



Comment at: lib/Driver/ToolChains/Cuda.cpp:707
   CmdArgs.push_back(StaticLibName);
+} else if (OrigInputFileName.endswith(".so")) {
+  // Only static libraries are supported.

ABataev wrote:
> I would suggest to use `types::lookupTypeForExtension` function and compare 
> the result with the `types::TY_Object`.
Isn't that too broad? I only want .so files.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59028



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

[PATCH] D39050: Add index-while-building support to Clang

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added inline comments.



Comment at: lib/Index/FileIndexData.cpp:31
+  std::vector Sorted(Decls);
+  std::sort(Sorted.begin(), Sorted.end());
+  return Sorted;

Please use range-based llvm::sort instead of std::sort:

```
llvm::sort(Sorted);
```
See 
https://llvm.org/docs/CodingStandards.html#beware-of-non-deterministic-sorting-order-of-equal-elements


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

https://reviews.llvm.org/D39050



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

[PATCH] D59013: [CMake][runtimes] Move libunwind, libc++abi and libc++ to lib/ and include/

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added inline comments.



Comment at: clang/lib/Driver/ToolChain.cpp:89
+  P.assign(D.Dir);
+  llvm::sys::path::append(P, "..", "lib", D.getTargetTriple(), "lib");
+  if (getVFS().exists(P))

Is this supposed to append lib twice?



Comment at: libcxx/CMakeLists.txt:421
+  set(DEFAULT_INSTALL_PREFIX 
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/)
+  set(LIBCXX_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}${LIBCXX_LIBDIR_SUFFIX})
+  set(LIBCXX_HEADER_DIR ${LLVM_BINARY_DIR})

Having the libdir suffix after the target triple looks weird ... also, the 
driver code doesn't appear to be taking the possibility of a suffix into 
account? I might be missing something here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59013



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

[PATCH] D58890: Modules: Rename MemoryBufferCache to InMemoryModuleCache

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

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



> `ninja check-clang` passes... is there anything else I should be testing?

I'm not sure, just double checking :)

The general approach LGTM though.


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

https://reviews.llvm.org/D58890



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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Sean Fertile via Phabricator via cfe-commits]

sfertile added inline comments.



Comment at: llvm/lib/MC/MCContext.cpp:165
+case MCObjectFileInfo::IsXCOFF:
+  // TODO: Need to implement class MCSymbolXCOFF.
+  break;

jasonliu wrote:
> JDevlieghere wrote:
> > See previous comment.
> It is certain that we will need MCSymbolXCOFF. But before we run into cases 
> where we actually need a MCSymbolXCOFF, we could use the generic MCSymbol 
> first for XCOFF platform. So I don't want to put a llvm_unreachable here. 
Would it make sense to add an llvm_unreachable now, and the first patch that 
actually uses an MCSymbol stubs out the class and removes the unreachable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930



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

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

[Roman Lebedev via Phabricator via cfe-commits]

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

Anyways, this looks good in this state.
Thank you for working on this!

If you don't intend to immediately work on fixing the rest of `,` cases here,
*please* do file a bug so it won't get lost. (and link it here)




Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:27
 
+AST_MATCHER_P(Expr, skipCommaOps,
+ ast_matchers::internal::Matcher, InnerMatcher) {

djtodoro wrote:
> lebedev.ri wrote:
> > djtodoro wrote:
> > > lebedev.ri wrote:
> > > > (Can anyone suggest a better name maybe?
> > > > I'm not sure this is the most correct name, but i can't suggest a 
> > > > better alternative.)
> > > Maybe `evalCommaExpr`?
> > Ah, yes, `eval` sounds better.
> > But, it won't always eval comma op or bailout.
> > It will eval if it is there.
> > So maybe `maybeEvalCommaExpr` (pun intended!).
> Sure, `maybeEvalCommaExpr ` sounds better.
> Or, one more suggestion is `tryEvalCommaExpr` ?
`try`, at least to me, has the same meaning as `evalCommaExpr`, i.e. one might 
expect that if it fails, it will fail.
So not sure.


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

https://reviews.llvm.org/D58894



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

[PATCH] D59028: [OpenMP] Enable on device linking with NVLINK to ignore dynamic libraries

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:707
   CmdArgs.push_back(StaticLibName);
+} else if (OrigInputFileName.endswith(".so")) {
+  // Only static libraries are supported.

I would suggest to use `types::lookupTypeForExtension` function and compare the 
result with the `types::TY_Object`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59028



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

[PATCH] D58659: [Sema] Fix assertion when `auto` parameter in lambda has an attribute.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:261
+/// necessary.
+QualType ReplaceAutoType(QualType TypeWithAuto, QualType Replacement) {
+  QualType T = sema.ReplaceAutoType(TypeWithAuto, Replacement);

I think this work should be done within `SubstituteDeducedTypeTransform` rather 
than on the side. Any caller to `Sema::ReplaceAutoType()` should get this same 
behavior.


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

https://reviews.llvm.org/D58659



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

[PATCH] D58719: [PR40778][Sema] Adjust address space of operands in builtin operators

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 189527.
Anastasia added a comment.
Herald added a subscriber: jdoerfert.

I now create overloads for all address spaces that are being called.


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

https://reviews.llvm.org/D58719

Files:
  lib/Sema/SemaOverload.cpp
  test/CodeGenOpenCLCXX/addrspace-operators.cl

Index: test/CodeGenOpenCLCXX/addrspace-operators.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/addrspace-operators.cl
@@ -0,0 +1,46 @@
+//RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+enum E {
+  a,
+  b,
+};
+
+class C {
+public:
+  void Assign(E e) { me = e; }
+  void OrAssign(E e) { mi |= e; }
+  E me;
+  int mi;
+};
+
+__global E globE;
+//CHECK-LABEL: define spir_func void @_Z3barv()
+void bar() {
+  C c;
+  //CHECK: addrspacecast %class.C* %c to %class.C addrspace(4)*
+  //CHECK: call void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* %{{[0-9]+}}, i32 0)
+  c.Assign(a);
+  //CHECK: addrspacecast %class.C* %c to %class.C addrspace(4)*
+  //CHECK: call void @_ZNU3AS41C8OrAssignE1E(%class.C addrspace(4)* %{{[0-9]+}}, i32 0)
+  c.OrAssign(a);
+
+  E e;
+  // CHECK: store i32 1, i32* %e
+  e = b;
+  // CHECK: store i32 0, i32 addrspace(1)* @globE
+  globE = a;
+  // FIXME: Sema fails here because it thinks the types are incompatible.
+  //e = b;
+  //globE = a;
+}
+
+//CHECK: define linkonce_odr void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* %this, i32 %e)
+//CHECK: [[E:%[0-9]+]] = load i32, i32* %e.addr
+//CHECK: %me = getelementptr inbounds %class.C, %class.C addrspace(4)* %this1, i32 0, i32 0
+//CHECK: store i32 [[E]], i32 addrspace(4)* %me
+
+//CHECK define linkonce_odr void @_ZNU3AS41C8OrAssignE1E(%class.C addrspace(4)* %this, i32 %e)
+//CHECK: [[E:%[0-9]+]] = load i32, i32* %e.addr
+//CHECK: %mi = getelementptr inbounds %class.C, %class.C addrspace(4)* %this1, i32 0, i32 1
+//CHECK: [[MI:%[0-9]+]] = load i32, i32 addrspace(4)* %mi
+//CHECK: %or = or i32 [[MI]], [[E]]
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -7659,6 +7659,12 @@
 }
   }
 }
+/// Helper function for adjusting address spaces for the pointer or reference
+/// operands of builtin operators depending on the argument.
+static QualType AdjustAddressSpaceForBuiltinOperandType(Sema &S, QualType T,
+Expr *Arg) {
+  return S.Context.getAddrSpaceQualType(T, Arg->getType().getAddressSpace());
+}
 
 /// Helper function for AddBuiltinOperatorCandidates() that adds
 /// the volatile- and non-volatile-qualified assignment operators for the
@@ -7670,15 +7676,17 @@
   QualType ParamTypes[2];
 
   // T& operator=(T&, T)
-  ParamTypes[0] = S.Context.getLValueReferenceType(T);
+  ParamTypes[0] = S.Context.getLValueReferenceType(
+  AdjustAddressSpaceForBuiltinOperandType(S, T, Args[0]));
   ParamTypes[1] = T;
   S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
 /*IsAssignmentOperator=*/true);
 
   if (!S.Context.getCanonicalType(T).isVolatileQualified()) {
 // volatile T& operator=(volatile T&, T)
-ParamTypes[0]
-  = S.Context.getLValueReferenceType(S.Context.getVolatileType(T));
+ParamTypes[0] = S.Context.getLValueReferenceType(
+AdjustAddressSpaceForBuiltinOperandType(S, S.Context.getVolatileType(T),
+Args[0]));
 ParamTypes[1] = T;
 S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
   /*IsAssignmentOperator=*/true);
@@ -8573,8 +8581,9 @@
 ParamTypes[1] = ArithmeticTypes[Right];
 
 // Add this built-in operator as a candidate (VQ is empty).
-ParamTypes[0] =
-  S.Context.getLValueReferenceType(ArithmeticTypes[Left]);
+ParamTypes[0] = S.Context.getLValueReferenceType(
+AdjustAddressSpaceForBuiltinOperandType(S, ArithmeticTypes[Left],
+Args[0]));
 S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
 if (VisibleTypeConversionsQuals.hasVolatile()) {
   // Add this built-in operator as a candidate (VQ is 'volatile').
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r355522 - [IR][ARM] Add function pointer alignment to datalayout

[Michael Platings via cfe-commits]

Author: michaelplatings
Date: Wed Mar  6 09:24:11 2019
New Revision: 355522

URL: http://llvm.org/viewvc/llvm-project?rev=355522&view=rev
Log:
[IR][ARM] Add function pointer alignment to datalayout

Use this feature to fix a bug on ARM where 4 byte alignment is
incorrectly assumed.

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

Modified:
cfe/trunk/lib/Basic/Targets/ARM.cpp
cfe/trunk/test/CodeGen/armv7k-abi.c
cfe/trunk/test/CodeGen/target-data.c

Modified: cfe/trunk/lib/Basic/Targets/ARM.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/ARM.cpp?rev=355522&r1=355521&r2=355522&view=diff
==
--- cfe/trunk/lib/Basic/Targets/ARM.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/ARM.cpp Wed Mar  6 09:24:11 2019
@@ -40,13 +40,14 @@ void ARMTargetInfo::setABIAAPCS() {
   // so set preferred for small types to 32.
   if (T.isOSBinFormatMachO()) {
 resetDataLayout(BigEndian
-? "E-m:o-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
-: "e-m:o-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64");
+? "E-m:o-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+: 
"e-m:o-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64");
   } else if (T.isOSWindows()) {
 assert(!BigEndian && "Windows on ARM does not support big endian");
 resetDataLayout("e"
 "-m:w"
 "-p:32:32"
+"-Fi8"
 "-i64:64"
 "-v128:64:128"
 "-a:0:32"
@@ -54,11 +55,11 @@ void ARMTargetInfo::setABIAAPCS() {
 "-S64");
   } else if (T.isOSNaCl()) {
 assert(!BigEndian && "NaCl on ARM does not support big endian");
-resetDataLayout("e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S128");
+resetDataLayout("e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S128");
   } else {
 resetDataLayout(BigEndian
-? "E-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
-: "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64");
+? "E-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+: 
"e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64");
   }
 
   // FIXME: Enumerated types are variable width in straight AAPCS.
@@ -87,17 +88,17 @@ void ARMTargetInfo::setABIAPCS(bool IsAA
 
   if (T.isOSBinFormatMachO() && IsAAPCS16) {
 assert(!BigEndian && "AAPCS16 does not support big-endian");
-resetDataLayout("e-m:o-p:32:32-i64:64-a:0:32-n32-S128");
+resetDataLayout("e-m:o-p:32:32-Fi8-i64:64-a:0:32-n32-S128");
   } else if (T.isOSBinFormatMachO())
 resetDataLayout(
 BigEndian
-? "E-m:o-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
-: "e-m:o-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32");
+? 
"E-m:o-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
+: 
"e-m:o-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32");
   else
 resetDataLayout(
 BigEndian
-? "E-m:e-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
-: "e-m:e-p:32:32-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32");
+? 
"E-m:e-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
+: 
"e-m:e-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32");
 
   // FIXME: Override "preferred align" for double and long long.
 }
@@ -1055,7 +1056,7 @@ CygwinARMTargetInfo::CygwinARMTargetInfo
   this->WCharType = TargetInfo::UnsignedShort;
   TLSSupported = false;
   DoubleAlign = LongLongAlign = 64;
-  resetDataLayout("e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64");
+  resetDataLayout("e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64");
 }
 
 void CygwinARMTargetInfo::getTargetDefines(const LangOptions &Opts,

Modified: cfe/trunk/test/CodeGen/armv7k-abi.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/armv7k-abi.c?rev=355522&r1=355521&r2=355522&view=diff
==
--- cfe/trunk/test/CodeGen/armv7k-abi.c (original)
+++ cfe/trunk/test/CodeGen/armv7k-abi.c Wed Mar  6 09:24:11 2019
@@ -4,7 +4,7 @@
 
 // Make sure 64 and 128 bit types are naturally aligned by the v7k ABI:
 
-// CHECK: target datalayout = "e-m:o-p:32:32-i64:64-a:0:32-n32-S128"
+// CHECK: target datalayout = "e-m:o-p:32:32-Fi8-i64:64-a:0:32-n32-S128"
 
 typedef struct {
   float arr[4];

Modified: cfe/trunk/test/CodeGen/target-data.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-data.c?rev=355522&r1=355521&r2=355522&view=diff
==
--- cfe/trunk/test/CodeGen/target-data.c (original)
+++ cfe/trunk/test/CodeGen/target-data.c Wed Mar  6 09:24:11 2019
@@ -96,7 +96,7 @@
 
 // RUN: %clang_cc1

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return-type check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:34
+  bool VisitUnqualName(StringRef UnqualName) {
+// Check for collisions with function arguments
+for (ParmVarDecl *Param : F.parameters())

Missing full stop at the end of the sentence.



Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:150
+
+if (T.isOneOf(tok::amp, tok::ampamp, tok::kw_const, tok::kw_volatile)) {
+  Result = T.getEndLoc();

This list misses an edge case for `__restrict`. e.g., 
https://godbolt.org/z/d4WDcA



Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:180
+  if (Info.hasMacroDefinition()) {
+// The CV qualifiers of the return type are inside macros
+diag(F.getLocation(), Message);

Missing full stop.



Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:229
+  auto IsCV = [](Token T) {
+return T.isOneOf(tok::kw_const, tok::kw_volatile);
+  };

This also misses the `__restrict` case.



Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:274
+
+  if (F->getLocation().isInvalid())
+return;

Should this also bail out if the function is `main()`?



Comment at: test/clang-tidy/modernize-use-trailing-return-type.cpp:95-97
+extern "C" int d2(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use a trailing return type for 
this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}extern "C" auto d2(int arg) -> int;{{$}}

This is a rather unexpected transformation, to me.


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

https://reviews.llvm.org/D56160



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

[PATCH] D57335: [IR] Don't assume all functions are 4 byte aligned

[Michael Platings via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355522: [IR][ARM] Add function pointer alignment to 
datalayout (authored by michaelplatings, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57335?vs=188325&id=189524#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57335

Files:
  cfe/trunk/lib/Basic/Targets/ARM.cpp
  cfe/trunk/test/CodeGen/armv7k-abi.c
  cfe/trunk/test/CodeGen/target-data.c
  llvm/trunk/docs/LangRef.rst
  llvm/trunk/include/llvm/IR/DataLayout.h
  llvm/trunk/lib/IR/ConstantFold.cpp
  llvm/trunk/lib/IR/DataLayout.cpp
  llvm/trunk/lib/IR/Value.cpp
  llvm/trunk/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/trunk/unittests/IR/CMakeLists.txt
  llvm/trunk/unittests/IR/ConstantsTest.cpp
  llvm/trunk/unittests/IR/DataLayoutTest.cpp
  llvm/trunk/unittests/IR/FunctionTest.cpp

Index: llvm/trunk/lib/Target/ARM/ARMTargetMachine.cpp
===
--- llvm/trunk/lib/Target/ARM/ARMTargetMachine.cpp
+++ llvm/trunk/lib/Target/ARM/ARMTargetMachine.cpp
@@ -141,6 +141,10 @@
   // Pointers are 32 bits and aligned to 32 bits.
   Ret += "-p:32:32";
 
+  // Function pointers are aligned to 8 bits (because the LSB stores the
+  // ARM/Thumb state).
+  Ret += "-Fi8";
+
   // ABIs other than APCS have 64 bit integers with natural alignment.
   if (ABI != ARMBaseTargetMachine::ARM_ABI_APCS)
 Ret += "-i64:64";
Index: llvm/trunk/lib/IR/DataLayout.cpp
===
--- llvm/trunk/lib/IR/DataLayout.cpp
+++ llvm/trunk/lib/IR/DataLayout.cpp
@@ -184,6 +184,8 @@
   AllocaAddrSpace = 0;
   StackNaturalAlign = 0;
   ProgramAddrSpace = 0;
+  FunctionPtrAlign = 0;
+  TheFunctionPtrAlignType = FunctionPtrAlignType::Independent;
   ManglingMode = MM_None;
   NonIntegralAddressSpaces.clear();
 
@@ -379,6 +381,22 @@
   StackNaturalAlign = inBytes(getInt(Tok));
   break;
 }
+case 'F': {
+  switch (Tok.front()) {
+  case 'i':
+TheFunctionPtrAlignType = FunctionPtrAlignType::Independent;
+break;
+  case 'n':
+TheFunctionPtrAlignType = FunctionPtrAlignType::MultipleOfFunctionAlign;
+break;
+  default:
+report_fatal_error("Unknown function pointer alignment type in "
+   "datalayout string");
+  }
+  Tok = Tok.substr(1);
+  FunctionPtrAlign = inBytes(getInt(Tok));
+  break;
+}
 case 'P': { // Function address space.
   ProgramAddrSpace = getAddrSpace(Tok);
   break;
@@ -432,6 +450,8 @@
  AllocaAddrSpace == Other.AllocaAddrSpace &&
  StackNaturalAlign == Other.StackNaturalAlign &&
  ProgramAddrSpace == Other.ProgramAddrSpace &&
+ FunctionPtrAlign == Other.FunctionPtrAlign &&
+ TheFunctionPtrAlignType == Other.TheFunctionPtrAlignType &&
  ManglingMode == Other.ManglingMode &&
  LegalIntWidths == Other.LegalIntWidths &&
  Alignments == Other.Alignments && Pointers == Other.Pointers;
Index: llvm/trunk/lib/IR/ConstantFold.cpp
===
--- llvm/trunk/lib/IR/ConstantFold.cpp
+++ llvm/trunk/lib/IR/ConstantFold.cpp
@@ -26,6 +26,7 @@
 #include "llvm/IR/GlobalAlias.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/Module.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/PatternMatch.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -1076,10 +1077,29 @@
 isa(CE1->getOperand(0))) {
   GlobalValue *GV = cast(CE1->getOperand(0));
 
-  // Functions are at least 4-byte aligned.
-  unsigned GVAlign = GV->getAlignment();
-  if (isa(GV))
-GVAlign = std::max(GVAlign, 4U);
+  unsigned GVAlign;
+
+  if (Module *TheModule = GV->getParent()) {
+GVAlign = GV->getPointerAlignment(TheModule->getDataLayout());
+
+// If the function alignment is not specified then assume that it
+// is 4.
+// This is dangerous; on x86, the alignment of the pointer
+// corresponds to the alignment of the function, but might be less
+// than 4 if it isn't explicitly specified.
+// However, a fix for this behaviour was reverted because it
+// increased code size (see https://reviews.llvm.org/D55115)
+// FIXME: This code should be deleted once existing targets have
+// appropriate defaults
+if (GVAlign == 0U && isa(GV))
+  GVAlign = 4U;
+  } else if (isa(GV)) {
+// Without a datalayout we have to assume the worst case: that the
+// function pointer isn't aligned at all.
+GVAlign = 0U;
+  } else {
+GVAlign = GV->getAlignment();
+  }
 
 

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Do you have some evidence that the current behavior is causing a lot of false 
positives in the wild? For ASCII character literals, I can sort of guess at why 
people might want to do this, but for things like wide character literals, or 
character literals relying on the current code page, etc, I'm less convinced.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58896



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

[PATCH] D58154: Add support for -fpermissive.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Ah -- I now understand your concern, and managing user expectations 
appropriately does seem like a potential concern.

I did not look too closely before into what exactly GCC categorized as 
"permerror" (errors that can be disabled with -fpermissive) vs what Clang 
categorized as "ExtWarn,DefaultError" diagnostics. Looking into it a little 
more now, I think there may not actually be substantial overlap between the two 
right now.

Many errors we have warning flags for are unconditional in GCC, and many that 
gcc allows disabling with -fpermissive are unconditional errors in clang.

You gave examples of the latter already, and e.g. here's a bunch of bogus code 
which you can currently build with -Wno-everything in clang, but most of which 
cannot be disabled in gcc (and I believe most if not all of these flags are 
ExtWarn, so would be disabled by this proposed -fpermissive):
https://godbolt.org/z/81NkJQ

Given that, yeah, it may not actually make sense to call this behavior 
-fpermissive.

I also can't really tell what the intent is behind classifying some diagnostics 
in Clang as ExtWarn,DefaultError and others Warning,DefaultError -- or if there 
even is any useful purpose there at all. I note with special-puzzlement the 
existence of both `ext_return_missing_expr` and `warn_return_missing_expr`.

I guess my feeling now is that perhaps we should just eliminate that 
categorization as meaningless, and just make -Wno-error=everything work 
properly (for anyone who wants to not abort the compile on broken code, but for 
whatever reason also loves to see build-spam so doesn't want -Wno-everything).


Repository:
  rC Clang

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

https://reviews.llvm.org/D58154



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

[PATCH] D57855: [analyzer] Reimplement checker options

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from some minor nits, but you should wait for someone more 
well-versed in the static analyzer to sign off before committing.




Comment at: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h:164
+// Used for lower_bound.
+CheckerInfo(StringRef FullName) : FullName(FullName) {}
   };

Make this `explicit`?



Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:371
+// We can't get away with binaryFind here, as it uses lower_bound.
+auto CheckerIt = llvm::find(Checkers, CheckerInfo(SuppliedChecker));
+if (CheckerIt != Checkers.end())

`llvm::any_of()` instead of `find()`? Same below.


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

https://reviews.llvm.org/D57855



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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079
+  if (log)
+log->Printf("sorry: unimplemented for XCOFF");
+  return false;

davide wrote:
> hubert.reinterpretcast wrote:
> > JDevlieghere wrote:
> > > jasonliu wrote:
> > > > JDevlieghere wrote:
> > > > > jasonliu wrote:
> > > > > > apaprocki wrote:
> > > > > > > No need to be `sorry:` :) This should probably just say `error: 
> > > > > > > XCOFF is unimplemented` to be more direct in case anything is 
> > > > > > > expecting "error:" in the output.
> > > > > > Sure. Will address in next revision.
> > > > > Just bundle this with the WASM case, the error message is correct for 
> > > > > both.
> > > > I think they are different. 
> > > > The error message for WASM seems to suggest that it will never ever get 
> > > > supported on WASM. 
> > > > But it is not the case for XCOFF, we want to indicate that it is not 
> > > > implemented yet.  
> > > I don't think the error message suggests that at all, and it's definitely 
> > > not true. At this point neither XCOFF nor WASM is supported, and that's 
> > > exactly what the log message says.
> > > 
> > I agree that the error message for WASM does not indicate that the lack of 
> > support is inherent or intended to be permanent; however, it is not 
> > indicative either of an intent to implement the support. I am not sure what 
> > the intent is for WASM, but I do know that the intent for XCOFF is to 
> > eventually implement the support. I do not see how using an ambiguous 
> > message in this commit (when we know what the intent is) is superior to the 
> > alternative of having an unambiguous message.
> I think we should keep this consistent with the other target so my vote is 
> for grouping XCOFF with WASM. After all, if it's going to be implemented 
> soon, the message will go away :)
Well, I don't know about "soon"...
Using the WASM message for XCOFF is not actually wrong; so, I can be okay with 
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930



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

[PATCH] D58977: [clang-tidy] Add the abseil-time-comparison check

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tidy/abseil/TimeComparisonCheck.cpp:23
+  auto Matcher =
+  binaryOperator(anyOf(hasOperatorName(">"), hasOperatorName(">="),
+   hasOperatorName("=="), hasOperatorName("<="),

`DurationComparisonCheck.cpp` has a very similar matcher pattern.

Maybe pull out a common matcher for this? E.g. 
`comparisonOperatorWithCallee(...)`?




Comment at: clang-tidy/abseil/TimeComparisonCheck.cpp:42
+
+  // In most cases, we'll only need to rewrite one of the sides, but we also
+  // want to handle the case of rewriting both sides. This is much simpler if

Move this comment closer the replacement logic below?



Comment at: clang-tidy/abseil/TimeComparisonCheck.cpp:46
+  // if nothing needs to be done.
+  if (!isNotInMacro(Result, Binop->getLHS()) ||
+  !isNotInMacro(Result, Binop->getRHS()))

nit: double negation is a bit hard to read. maybe `!(isNotInMacro(LSH) && 
isNotInMacro(RHS))`? Ideally, the helper should be `isInMacro` instead of 
`isNotInMacro`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58977



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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Davide Italiano via Phabricator via cfe-commits]

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



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079
+  if (log)
+log->Printf("sorry: unimplemented for XCOFF");
+  return false;

hubert.reinterpretcast wrote:
> JDevlieghere wrote:
> > jasonliu wrote:
> > > JDevlieghere wrote:
> > > > jasonliu wrote:
> > > > > apaprocki wrote:
> > > > > > No need to be `sorry:` :) This should probably just say `error: 
> > > > > > XCOFF is unimplemented` to be more direct in case anything is 
> > > > > > expecting "error:" in the output.
> > > > > Sure. Will address in next revision.
> > > > Just bundle this with the WASM case, the error message is correct for 
> > > > both.
> > > I think they are different. 
> > > The error message for WASM seems to suggest that it will never ever get 
> > > supported on WASM. 
> > > But it is not the case for XCOFF, we want to indicate that it is not 
> > > implemented yet.  
> > I don't think the error message suggests that at all, and it's definitely 
> > not true. At this point neither XCOFF nor WASM is supported, and that's 
> > exactly what the log message says.
> > 
> I agree that the error message for WASM does not indicate that the lack of 
> support is inherent or intended to be permanent; however, it is not 
> indicative either of an intent to implement the support. I am not sure what 
> the intent is for WASM, but I do know that the intent for XCOFF is to 
> eventually implement the support. I do not see how using an ambiguous message 
> in this commit (when we know what the intent is) is superior to the 
> alternative of having an unambiguous message.
I think we should keep this consistent with the other target so my vote is for 
grouping XCOFF with WASM. After all, if it's going to be implemented soon, the 
message will go away :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930



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

[PATCH] D59028: [OpenMP] Enable on device linking with NVLINK to ignore dynamic libraries

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea created this revision.
gtbercea added reviewers: ABataev, Hahnfeld, caomhin.
Herald added subscribers: cfe-commits, jdoerfert, guansong.
Herald added a project: clang.

NVLINK does not support dynamic linking so passing dynamic libraries to NVLINK 
should be avoided.

This patch fixes a bug whereby an explicit passing of the dynamic library via 
it's full path will result in it being treated like an input.


Repository:
  rC Clang

https://reviews.llvm.org/D59028

Files:
  lib/Driver/ToolChains/Cuda.cpp
  test/Driver/openmp-offload-gpu.c


Index: test/Driver/openmp-offload-gpu.c
===
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -278,3 +278,12 @@
 // RUN:   | FileCheck -check-prefix=CUDA_RED_RECS %s
 // CUDA_RED_RECS: clang{{.*}}"-cc1"{{.*}}"-triple" "nvptx64-nvidia-cuda"
 // CUDA_RED_RECS-SAME: "-fopenmp-cuda-teams-reduction-recs-num=2048"
+
+/// ###
+
+/// Check NVLINK ignores dynamic libraries.
+// RUN:   touch %t-dynlib.so
+// RUN:   %clang -### -no-canonical-prefixes -fopenmp=libomp 
-fopenmp-targets=nvptx64-nvidia-cuda %t-dynlib.so %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-NVLINK-IGNORE-DYNLIB %s
+
+// CHK-NVLINK-IGNORE-DYNLIB-NOT: nvlink{{.*}}dynlib
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -704,6 +704,10 @@
   const char *StaticLibName =
   C.getArgs().MakeArgString(II.getFilename());
   CmdArgs.push_back(StaticLibName);
+} else if (OrigInputFileName.endswith(".so")) {
+  // Only static libraries are supported.
+  // Dynamic libraries are ignored.
+  continue;
 } else {
   // If the original input is not an object file then it means the
   // assembly step has actually produced a cubin so we need to


Index: test/Driver/openmp-offload-gpu.c
===
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -278,3 +278,12 @@
 // RUN:   | FileCheck -check-prefix=CUDA_RED_RECS %s
 // CUDA_RED_RECS: clang{{.*}}"-cc1"{{.*}}"-triple" "nvptx64-nvidia-cuda"
 // CUDA_RED_RECS-SAME: "-fopenmp-cuda-teams-reduction-recs-num=2048"
+
+/// ###
+
+/// Check NVLINK ignores dynamic libraries.
+// RUN:   touch %t-dynlib.so
+// RUN:   %clang -### -no-canonical-prefixes -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda %t-dynlib.so %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-NVLINK-IGNORE-DYNLIB %s
+
+// CHK-NVLINK-IGNORE-DYNLIB-NOT: nvlink{{.*}}dynlib
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -704,6 +704,10 @@
   const char *StaticLibName =
   C.getArgs().MakeArgString(II.getFilename());
   CmdArgs.push_back(StaticLibName);
+} else if (OrigInputFileName.endswith(".so")) {
+  // Only static libraries are supported.
+  // Dynamic libraries are ignored.
+  continue;
 } else {
   // If the original input is not an object file then it means the
   // assembly step has actually produced a cubin so we need to
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58992: [CUDA][HIP][DebugInfo] Skip reference device function

[Michael Liao via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1755
+}
+V = V->stripPointerCasts();
   }

aprantl wrote:
> This wasn't there before... why is this necessary?
That's from the original line 1752.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58992



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

[PATCH] D58992: [CUDA][HIP][DebugInfo] Skip reference device function

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1755
+}
+V = V->stripPointerCasts();
   }

This wasn't there before... why is this necessary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58992



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

[PATCH] D58216: Support attribute used in member funcs of class templates II

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D58216#1414864 , @modocache wrote:

> Friendly ping! @aaron.ballman it looks like you accepted D56928 
> , could you take a look at this as well?


Sorry for the delay -- I was at WG21 meetings and then on vacation. I think the 
behavior in D56928  was correct; at least, it 
matches the behavior of GCC: https://godbolt.org/z/Un5ugT whereas the current 
patch does not.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58216



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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Jason Liu via Phabricator via cfe-commits]

jasonliu updated this revision to Diff 189514.
jasonliu added a comment.

Address comments in last revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/unittests/ADT/TripleTest.cpp

Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -1258,6 +1258,11 @@
   EXPECT_EQ(Triple::Wasm,
 Triple("wasm64-unknown-wasi-musl-wasm").getObjectFormat());
 
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc-ibm-aix").getObjectFormat());
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc64-ibm-aix").getObjectFormat());
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc---xcoff").getObjectFormat());
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc64---xcoff").getObjectFormat());
+
   Triple MSVCNormalized(Triple::normalize("i686-pc-windows-msvc-elf"));
   EXPECT_EQ(Triple::ELF, MSVCNormalized.getObjectFormat());
 
@@ -1276,6 +1281,9 @@
 
   T.setObjectFormat(Triple::MachO);
   EXPECT_EQ(Triple::MachO, T.getObjectFormat());
+  
+  T.setObjectFormat(Triple::XCOFF);
+  EXPECT_EQ(Triple::XCOFF, T.getObjectFormat());
 }
 
 TEST(TripleTest, NormalizeWindows) {
Index: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
===
--- llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -5594,6 +5594,9 @@
   case MCObjectFileInfo::IsWasm:
 CurrentFormat = WASM;
 break;
+  case MCObjectFileInfo::IsXCOFF:
+llvm_unreachable("unexpected object format");
+break;
   }
 
   if (~Prefix->SupportedFormats & CurrentFormat) {
Index: llvm/lib/Support/Triple.cpp
===
--- llvm/lib/Support/Triple.cpp
+++ llvm/lib/Support/Triple.cpp
@@ -534,6 +534,9 @@
 
 static Triple::ObjectFormatType parseFormat(StringRef EnvironmentName) {
   return StringSwitch(EnvironmentName)
+// "xcoff" must come before "coff" because of the order-dependendent
+// pattern matching.
+.EndsWith("xcoff", Triple::XCOFF)
 .EndsWith("coff", Triple::COFF)
 .EndsWith("elf", Triple::ELF)
 .EndsWith("macho", Triple::MachO)
@@ -622,6 +625,7 @@
   case Triple::ELF: return "elf";
   case Triple::MachO: return "macho";
   case Triple::Wasm: return "wasm";
+  case Triple::XCOFF: return "xcoff";
   }
   llvm_unreachable("unknown object format type");
 }
@@ -686,6 +690,8 @@
   case Triple::ppc64:
 if (T.isOSDarwin())
   return Triple::MachO;
+else if (T.isOSAIX())
+  return Triple::XCOFF;
 return Triple::ELF;
 
   case Triple::wasm32:
Index: llvm/lib/MC/MCParser/AsmParser.cpp
===
--- llvm/lib/MC/MCParser/AsmParser.cpp
+++ llvm/lib/MC/MCParser/AsmParser.cpp
@@ -710,6 +710,9 @@
   case MCObjectFileInfo::IsWasm:
 PlatformParser.reset(createWasmAsmParser());
 break;
+  case MCObjectFileInfo::IsXCOFF:
+// TODO: Need to implement createXCOFFAsmParser for XCOFF format.
+break;
   }
 
   PlatformParser->Initialize(*this);
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -801,6 +801,10 @@
 Env = IsWasm;
 initWasmMCObjectFileInfo(TT);
 break;
+  case Triple::XCOFF:
+Env = IsXCOFF;
+// TODO: Initialize MCObjectFileInfo for XCOFF format when MCSectionXCOFF is ready.
+break;
   case Triple::UnknownObjectFormat:
 report_fatal_error("Cannot initialize MC for unknown object file format.");
 break;
@@ -816,6 +820,7 @@
   case Triple::MachO:
   case Triple::COFF:
   case Triple::Wasm:
+  case Triple::XCOFF:
   case Triple::UnknownObjectFormat:
 report_fatal_error("Cannot get DWARF comdat section for this object file "
"format: not implemented.");
Index: llvm/lib/MC/MCContext.cpp
===
--- llvm/lib/MC/MCContext.cpp
+++ llvm/lib/MC/MCContext.cpp
@@ -161,6 +161,9 @@
   return new (Name, *this) MCSymbolMachO(Name, IsTemporary);
 case MCObjectFileInfo::IsWasm:
   return new (Name, *this) MCSymbolWasm(Name, IsTemporary);
+case MCObjectFileInfo::IsXCOFF:
+  // TODO: Need to implement class MCSymbolXCOFF.
+  break;
 }
   }
   return new (Name, *this) MCSymbol(MCSymbol::SymbolKindUnset, Name,
Index: llvm/include/llv

[PATCH] D58186: Sync some doc changes ClangFormatStyleOptions.rst with doc comments in `Format.h`

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

Ping? Can somebody commit this for me? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58186



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

[PATCH] D58977: [clang-tidy] Add the abseil-time-comparison check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/abseil-time-comparison.rst:6
+
+Checks for comparisons which should be in the ``absl::Time`` domain instead
+of the integer domains.

Please synchronize with Release Notes.



Comment at: docs/clang-tidy/checks/abseil-time-comparison.rst:10
+N.B.: In cases where an ``absl::Time`` is being converted to an integer,
+alignment may occur.  If the comparison depends on this alingment, doing the
+comparison in the ``absl::Time`` domain may yield a different result.  In

Please fix double spaces. Same below.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58977



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

[PATCH] D58757: Add a version of the pass_object_size attribute that works with builtin_dynamic_object_size

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1570
+def PassDynamicObjectSize : InheritableParamAttr {
+  let Spellings = [Clang<"pass_dynamic_object_size">];
+  let Args = [IntArgument<"Type">];

Why use a separate attribute as opposed to a separate spelling and an accessor 
on `PassObjectSizeAttr` to ask whether it's dynamic or not? The two attributes 
seem to have identical semantics aside from which builtin is called, so I think 
it makes sense to use the same semantic attribute type.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1108
+static void handlePassObjectSizeAttr(Sema &S, Decl *D, const ParsedAttr &AL,
+ bool IsDynamic) {
+  assert(isa(D) && "Should be checked already!");

Please do not add a parameter here -- we keep all of the `handleFooAttr()` 
function signatures the same because some day we'd like to refactor the switch 
to be more tablegenerated as well.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1140
 
-  D->addAttr(::new (S.Context) PassObjectSizeAttr(
-  AL.getRange(), S.Context, (int)Type, 
AL.getAttributeSpellingListIndex()));
+  if (IsDynamic) {
+D->addAttr(::new (S.Context) PassDynamicObjectSizeAttr(

Elide braces here and below.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58757



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

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D52835#1412416 , @xbolva00 wrote:

> @aaron.ballman does it make sense to warn for this case only in C/pre-C++11 
> mode? Since this case in C++11/14/17 is handled by -Wnarrowing, as we see 
> from tests.


I don't think so, because code like this still needs the diagnostic even in 
C++11 mode, but there's no -Wnarrowing warning for it: `float a1 = (1ULL << 31) 
+ 1;`




Comment at: test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp:136
   Agg f8 = {EnumVal};  // OK
+  // expected-warning@+1 {{implicit conversion from 'int' to 'float' changes 
value from 123456789 to 1.2345679E+8}}
   Agg f9 = {123456789};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}

xbolva00 wrote:
> aaron.ballman wrote:
> > I don't think we want the warning triggered in either of these cases -- 
> > they already have an error diagnostic on the same line for the same issue.
> Any recommended way how it should be handled?
Why is it triggering twice in the first place? Template instantiation, maybe?


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

https://reviews.llvm.org/D52835



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

[PATCH] D57850: [analyzer] Emit an error rather than assert on invalid checker option input

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus marked an inline comment as done.
Szelethus added inline comments.
Herald added a subscriber: Charusso.



Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:352-355
+  if (Checker->AllowedPad < 0)
+
Mgr.getDiagnostics().Report(diag::err_analyzer_checker_option_invalid_input)
+<< (llvm::Twine() + Checker->getTagDescription() + ":AllowedPad").str()
+<< "a non-negative";

NoQ wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > 
> > > I passively wish for a certain amount of de-duplication that wouldn't 
> > > require every checker to obtain a diagnostics engine every time it tries 
> > > to read an option. Eg.,
> > > ```lang=c++
> > >   auto *Checker = Mgr.registerChecker();
> > >   Checker->AllowedPad = Mgr.getAnalyzerOptions()
> > >   .getCheckerIntegerOption(Checker, "AllowedPad", 24);
> > >   if (Checker->AllowedPad < 0)
> > > Mgr.reportInvalidOptionValue(Checker, "AllowedPad", "a non-negative");
> > > ```
> > > 
> > > Or maybe even something like that:
> > > 
> > > ```lang=c++
> > >   auto *Checker = Mgr.registerChecker();
> > >   Checker->AllowedPad = Mgr.getAnalyzerOptions()
> > >   .getCheckerIntegerOption(Checker, "AllowedPad", 24,
> > >   [](int x) -> Option {
> > >   if (x < 0) {
> > > // Makes getCheckerIntegerOption() emit a 
> > > diagnostic
> > > // and return the default value.
> > > return "a non-negative";
> > >   }
> > >   // Makes getCheckerIntegerOption() successfully 
> > > return
> > >   // the user-specified value.
> > >   return None;
> > >   });
> > > ```
> > > I.e., a validator lambda.
> > First one, sure. I'm a little unsure about the second: No other 
> > "options"-like classes have access to a `DiagnosticsEngine` in clang, as 
> > far as I'm aware, and I guess keeping `AnalyzerOptions` as simple is 
> > possible is preferred. Not only that, but a validator lambda seems an to be 
> > an overkill (though really-really cool) solution. Your first bit of code is 
> > far more readable IMO.
> Hmm, in the first example we'll also have to manually reset the option to the 
> default value if it is invalid, which is also annoying - even if easy to 
> understand, it's also easy to forget.
> 
> And with that and a bit of polish, the lambda approach isn't really much more 
> verbose, and definitely involves less duplication:
> 
> ```lang=c++
> auto *Checker = Mgr.registerChecker();
> Checker->AllowedPad = Mgr.getAnalyzerOptions()
> .getCheckerIntegerOption(Checker, "AllowedPad", 24);
> if (Checker->AllowedPad < 0) {
>   Mgr.reportInvalidOptionValue(Checker, "AllowedPad", "a non-negative value");
>   Checker->AllowedPad = 24;
> }
> ```
> vs.
> ```lang=c++
> auto *Checker = Mgr.registerChecker();
> Checker->AllowedPad = Mgr.getAnalyzerOptions()
> .getCheckerIntegerOption(Checker, "AllowedPad", /*Default=*/ 24,
>  /*Validate=*/ [](int x) { return x >= 0; },
>  /*ValidMsg=*/ "a non-negative value");
> ```
Alright, so I've given this a lot of thought, here's where I'm standing on the 
issue:

* I would prefer not to add `DiagnosticsEngine` to `AnalyzerOptions`. In fact, 
I'd prefer not to add it even as a parameter to one of it's methods -- 
designwise, it should be a simple mapping of the command line parameters, not 
doing any complicated hackery.
* You got me convinced the validator lambda thing ;). However, a nice 
implementation of this (emphasis on //nice//) is most definitely a bigger 
undertaking.
* Once we're at the topic of "easy to forget", we could also verify 
compile-time whether checker options are actually used -- what I'm thinking 
here, is something like this:

```
auto *Checker = Mgr.registerChecker();
Mgr.initFieldWithOption(Checker, "AllowedPad",
// Note that we should be able
// to know the default value.
Checker->AllowedPad,
// We could make this optional by defining a
// default validator...
/*Validate=*/ [](int x) { return x >= 0; },
// ...nd a default error message.
/*ValidMsg=*/ "a non-negative value");
```
`CheckerManager` later (once all checker registry functions finished) could 
validate, with the help of `CheckerRegistry`, whether
* All options for a given checker were queried for,
* The supplied checker options is valid, if not, restore them in compatibility 
mode, emit an error otherwise,
* No list is complete without a third item.

For now, I admit, I have little interest in this. Would you mind me committing 
as is?


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

https://reviews.llvm.org

r355514 - Reland "[Remarks] Refactor remark diagnostic emission in a RemarkStreamer"

[Francis Visoiu Mistrih via cfe-commits]

Author: thegameg
Date: Wed Mar  6 07:20:13 2019
New Revision: 355514

URL: http://llvm.org/viewvc/llvm-project?rev=355514&view=rev
Log:
Reland "[Remarks] Refactor remark diagnostic emission in a RemarkStreamer"

This allows us to store more info about where we're emitting the remarks
without cluttering LLVMContext. This is needed for future support for
the remark section.

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

Original llvm-svn: 355507

Modified:
cfe/trunk/lib/CodeGen/CodeGenAction.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=355514&r1=355513&r2=355514&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Wed Mar  6 07:20:13 2019
@@ -30,6 +30,7 @@
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/RemarkStreamer.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/Linker/Linker.h"
 #include "llvm/Pass.h"
@@ -276,8 +277,8 @@ namespace clang {
   return;
 }
 
-Ctx.setDiagnosticsOutputFile(
-llvm::make_unique(OptRecordFile->os()));
+Ctx.setRemarkStreamer(llvm::make_unique(
+CodeGenOpts.OptRecordFile, OptRecordFile->os()));
 
 if (CodeGenOpts.getProfileUse() != CodeGenOptions::ProfileNone)
   Ctx.setDiagnosticsHotnessRequested(true);


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

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

[John McCall via Phabricator via cfe-commits]

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

Thanks, LGTM.


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

https://reviews.llvm.org/D58346



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

[PATCH] D58708: [PR40778] Preserve addr space in Derived to Base cast

[John McCall via Phabricator via cfe-commits]

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

Thanks, LGTM.


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

https://reviews.llvm.org/D58708



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

r355511 - Revert "[Remarks] Refactor remark diagnostic emission in a RemarkStreamer"

[Francis Visoiu Mistrih via cfe-commits]

Author: thegameg
Date: Wed Mar  6 06:52:37 2019
New Revision: 355511

URL: http://llvm.org/viewvc/llvm-project?rev=355511&view=rev
Log:
Revert "[Remarks] Refactor remark diagnostic emission in a RemarkStreamer"

This reverts commit 2e8c4997a2089f8228c843fd81b148d903472e02.

Breaks bots.

Modified:
cfe/trunk/lib/CodeGen/CodeGenAction.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=355511&r1=355510&r2=355511&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Wed Mar  6 06:52:37 2019
@@ -30,7 +30,6 @@
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
-#include "llvm/IR/RemarkStreamer.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/Linker/Linker.h"
 #include "llvm/Pass.h"
@@ -277,8 +276,8 @@ namespace clang {
   return;
 }
 
-Ctx.setRemarkStreamer(llvm::make_unique(
-CodeGenOpts.OptRecordFile, OptRecordFile->os()));
+Ctx.setDiagnosticsOutputFile(
+llvm::make_unique(OptRecordFile->os()));
 
 if (CodeGenOpts.getProfileUse() != CodeGenOptions::ProfileNone)
   Ctx.setDiagnosticsHotnessRequested(true);


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

[PATCH] D58154: Add support for -fpermissive.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D58154#1418594 , @jyknight wrote:

> The errors disabled by this feature are default-error warnings -- you can 
> already get the same effect by using -Wno-. Why is it bad to 
> additionally allow -fpermissive to disable them? (If we have any diagnostics 
> which are currently default-on-warnings which should not _ever_ be 
> disable-able, then maybe we should just fix those?)


I believe -fpermissive controls more than just default-error warnings in GCC, 
and that's the kind of stuff I really don't want to support. For instance, 
there's no warning flag for this nonsense, but GCC accepts it under 
-fpermissive: https://godbolt.org/z/Q-Fl0i (If you think "but who does that?", 
this blog post shows up on the first page of a popular search engine when I 
search for fpermissive: 
http://blog.binchen.org/posts/always-turn-on-fpermissive-for-gcc-4-6.html). 
Another example of -fpermissive that I've seen in the wild is: 
https://godbolt.org/z/-44pK5 (which is warned on in C, but not controllable via 
a warning flag in C++). If we're going to claim support for -fpermissive as a 
GCC compatibility feature, then we'll get requests to support dialects like the 
above examples at some point as well and that's where my concern comes in -- I 
don't think we should ever support that kind of broken code.

I'd be fine with what's proposed here under a different flag name though.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58154



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

[PATCH] D58996: [Remarks] Refactor remark diagnostic emission in a RemarkStreamer

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC355507: [Remarks] Refactor remark diagnostic emission in a 
RemarkStreamer (authored by thegameg, committed by ).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58996?vs=189425&id=189500#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58996

Files:
  lib/CodeGen/CodeGenAction.cpp


Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -30,6 +30,7 @@
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/RemarkStreamer.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/Linker/Linker.h"
 #include "llvm/Pass.h"
@@ -276,8 +277,8 @@
   return;
 }
 
-Ctx.setDiagnosticsOutputFile(
-llvm::make_unique(OptRecordFile->os()));
+Ctx.setRemarkStreamer(llvm::make_unique(
+CodeGenOpts.OptRecordFile, OptRecordFile->os()));
 
 if (CodeGenOpts.getProfileUse() != CodeGenOptions::ProfileNone)
   Ctx.setDiagnosticsHotnessRequested(true);


Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -30,6 +30,7 @@
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/RemarkStreamer.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/Linker/Linker.h"
 #include "llvm/Pass.h"
@@ -276,8 +277,8 @@
   return;
 }
 
-Ctx.setDiagnosticsOutputFile(
-llvm::make_unique(OptRecordFile->os()));
+Ctx.setRemarkStreamer(llvm::make_unique(
+CodeGenOpts.OptRecordFile, OptRecordFile->os()));
 
 if (CodeGenOpts.getProfileUse() != CodeGenOptions::ProfileNone)
   Ctx.setDiagnosticsHotnessRequested(true);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r355507 - [Remarks] Refactor remark diagnostic emission in a RemarkStreamer

[Francis Visoiu Mistrih via cfe-commits]

Author: thegameg
Date: Wed Mar  6 06:32:08 2019
New Revision: 355507

URL: http://llvm.org/viewvc/llvm-project?rev=355507&view=rev
Log:
[Remarks] Refactor remark diagnostic emission in a RemarkStreamer

This allows us to store more info about where we're emitting the remarks
without cluttering LLVMContext. This is needed for future support for
the remark section.

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

Modified:
cfe/trunk/lib/CodeGen/CodeGenAction.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=355507&r1=355506&r2=355507&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Wed Mar  6 06:32:08 2019
@@ -30,6 +30,7 @@
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/RemarkStreamer.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/Linker/Linker.h"
 #include "llvm/Pass.h"
@@ -276,8 +277,8 @@ namespace clang {
   return;
 }
 
-Ctx.setDiagnosticsOutputFile(
-llvm::make_unique(OptRecordFile->os()));
+Ctx.setRemarkStreamer(llvm::make_unique(
+CodeGenOpts.OptRecordFile, OptRecordFile->os()));
 
 if (CodeGenOpts.getProfileUse() != CodeGenOptions::ProfileNone)
   Ctx.setDiagnosticsHotnessRequested(true);


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