[PATCH] D34860: [Objective-C] Fix non-determinism in clang

[David Blaikie via Phabricator via cfe-commits]

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

Sounds plausible to me


https://reviews.llvm.org/D34860



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

[PATCH] D34860: [Objective-C] Fix non-determinism in clang

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

Ping for reviews please.


https://reviews.llvm.org/D34860



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

[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lib/Driver/ToolChains/Linux.cpp:755
+!Version.isOlderThan(4, 8, 0)) {
+  // For gcc >= 4.8.x, clang will preinclude 
+  // -ffreestanding suppresses this behavior.

I don't see why it makes any sense to condition this based on a GCC 
installation existing and being >= 4.8, since this header comes from libc, and 
is necessary for standards compliance even if there's absolutely no GCC 
installed or involved.

Additionally, something like this -- a way for the libc to be involved in 
setting up predefined macros -- seems probably necessary for *any* libc, if 
they want to be strictly standards-compliant. Some of the 
required-to-be-predefined macros like `__STDC_ISO_10646__` can only really be 
provided by the libc, since the appropriate value is dependent on the locale 
implementation in the libc, and not on the compiler at all.

It's possible that glibc on Linux is the only one who's cared to implement it 
thus far (and, only when you're using GCC, at that, hence this change...). That 
seems likely, really, since mostly the impacted macros are nearly useless, so 
who cares... :) However, if others want to be conforming, I expect they _ought_ 
to be using this mechanism, as well...

Thus, perhaps it ought to be an unconditional behavior of clang to include the 
file if it exists, unless `-ffreestanding` is passed.


https://reviews.llvm.org/D34158



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

[PATCH] D35041: [analyzer] Fix modeling bool-based types

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap created this revision.
Herald added a subscriber: xazax.hun.

This is a follow up for one of the previous diffs 
https://reviews.llvm.org/D32328.
getTypeSize and with getIntWidth are not equivalent for bool 
(see https://clang.llvm.org/doxygen/ASTContext_8cpp_source.html#l08444),
this causes a number of issues
(for instance, if APint X representing a bool is created with the wrong bit 
width then X is not comparable against Min/Max
(because of the different bit width), that results in crashes (triggered 
asserts) inside assume* methods) (for example see the test case).
P.S. I have found some other suspicious places where getIntWidth probably 
should be used instead of getTypeSize, 
however fixing all of them in one go is not trivial 
(a couple of tests have failed on the larger diff (which might indicate some 
issues inside those checkers), but I have not investigated it yet,
so decided to break up my changes into smaller chunks and keep all the tests 
green.

Test plan:
make check-all


Repository:
  rL LLVM

https://reviews.llvm.org/D35041

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/enum.cpp


Index: test/Analysis/enum.cpp
===
--- test/Analysis/enum.cpp
+++ test/Analysis/enum.cpp
@@ -37,3 +37,12 @@
   }
   return true;
 }
+
+bool testNoCrashOnSwitchEnumBoolConstant() {
+  EnumBool E = EnumBool::F;
+  switch (E) {
+case EnumBool::F:
+  return false; 
+  }
+  return true; 
+}
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -71,7 +71,6 @@
 }
 
 SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) {
-
   bool isLocType = Loc::isLocType(castTy);
 
   if (val.getAs())
@@ -82,7 +81,7 @@
   return LI->getLoc();
 
 // FIXME: Correctly support promotions/truncations.
-unsigned castSize = Context.getTypeSize(castTy);
+unsigned castSize = Context.getIntWidth(castTy);
 if (castSize == LI->getNumBits())
   return val;
 return makeLocAsInteger(LI->getLoc(), castSize);
@@ -173,7 +172,7 @@
   }
 
   if (castTy->isIntegralOrEnumerationType()) {
-unsigned BitWidth = Context.getTypeSize(castTy);
+unsigned BitWidth = Context.getIntWidth(castTy);
 
 if (!val.getAs())
   return makeLocAsInteger(val, BitWidth);
Index: include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -124,7 +124,7 @@
   /// Returns the type of the APSInt used to store values of the given 
QualType.
   APSIntType getAPSIntType(QualType T) const {
 assert(T->isIntegralOrEnumerationType() || Loc::isLocType(T));
-return APSIntType(Ctx.getTypeSize(T),
+return APSIntType(Ctx.getIntWidth(T),
   !T->isSignedIntegerOrEnumerationType());
   }
 


Index: test/Analysis/enum.cpp
===
--- test/Analysis/enum.cpp
+++ test/Analysis/enum.cpp
@@ -37,3 +37,12 @@
   }
   return true;
 }
+
+bool testNoCrashOnSwitchEnumBoolConstant() {
+  EnumBool E = EnumBool::F;
+  switch (E) {
+case EnumBool::F:
+  return false; 
+  }
+  return true; 
+}
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -71,7 +71,6 @@
 }
 
 SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) {
-
   bool isLocType = Loc::isLocType(castTy);
 
   if (val.getAs())
@@ -82,7 +81,7 @@
   return LI->getLoc();
 
 // FIXME: Correctly support promotions/truncations.
-unsigned castSize = Context.getTypeSize(castTy);
+unsigned castSize = Context.getIntWidth(castTy);
 if (castSize == LI->getNumBits())
   return val;
 return makeLocAsInteger(LI->getLoc(), castSize);
@@ -173,7 +172,7 @@
   }
 
   if (castTy->isIntegralOrEnumerationType()) {
-unsigned BitWidth = Context.getTypeSize(castTy);
+unsigned BitWidth = Context.getIntWidth(castTy);
 
 if (!val.getAs())
   return makeLocAsInteger(val, BitWidth);
Index: include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -124,7 +124,7 @@
   /// Returns the type of the APSInt used to store values of the given QualType.
   APSIntType getAPSIntType(QualType T) const {
 

[PATCH] D34853: Fix (benignly) incorrect GoogleTest specs in various lit configs.

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.

In https://reviews.llvm.org/D34853#798699, @andrewng wrote:

> Hi,
>
> I believe that this "build mode" is intended for the Visual Studio MSVC 
> build. This build is special in that it can produce builds for multiple 
> configurations, e.g. Debug, Release & RelWithDebInfo, within the same top 
> level build output directory. It is this configuration type that defines the 
> "build mode". This means that the unit tests will only pick up the 
> configuration that matches that of the lit that was run. Without the "build 
> mode" I believe lit might end up running all configurations of unit tests 
> that have been built, which is probably not the intended behaviour.
>
> Cheers,
> Andrew


Ah, excellent. I suspected something like this.

(I don't usually have access to Windows, and even then I typically use Ninja 
instead of VS... so cleanups like this could be hit-or-miss, I guess.)


Repository:
  rL LLVM

https://reviews.llvm.org/D34853



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

[PATCH] D34489: [scan-build-py] Patch to fix "-analyzer-config" option

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D34489



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

[PATCH] D34574: [Sema] Disable c++17 aligned new and delete operators if not implemented in the deployment target's c++ standard library

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a subscriber: EricWF.
ahatanak added a comment.

Does r307218 unbreak the tests?


Repository:
  rL LLVM

https://reviews.llvm.org/D34574



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

[PATCH] D34649: Remove addtional parameters in function std::next() and std::prev()

[Eric Fiselier via Phabricator via cfe-commits]

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

> Creating a function pointer with proper parameters pointing to std::next() or 
> std::prev() should work.

Actually, according to the standard taking the address of a STL function is UB, 
and users should avoid doing it as much as possible since it's fundamentally 
non-portable.

However as a QoI change I think this is OK.


https://reviews.llvm.org/D34649



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

[PATCH] D35038: [libunwind] Add a test harness

[Eric Fiselier via Phabricator via cfe-commits]

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

This LGTM. I'm assuming it works but we can iron out the kinks once it lands. 
We should probably add builders that run the tests as well.




Comment at: test/lit.cfg:46
+libcxx.test.config.loadSiteConfig(
+lit_config, config, 'libunwind_site_config', 'LIBCXXABI_SITE_CONFIG')
+obj_root = getattr(config, 'libunwind_obj_root', None)

s/`LIBCXXABI_SITE_CONFIG`/`LIBUNWIND_SITE_CONFIG`.


https://reviews.llvm.org/D35038



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

r307232 - [modules ts] Do not emit strong function definitions from the module interface unit in every user.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Wed Jul  5 17:30:00 2017
New Revision: 307232

URL: http://llvm.org/viewvc/llvm-project?rev=307232=rev
Log:
[modules ts] Do not emit strong function definitions from the module interface 
unit in every user.

Added:
cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/
cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/
cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp
cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm
cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp
Modified:
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp

Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=307232=307231=307232=diff
==
--- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Wed Jul  5 17:30:00 2017
@@ -2233,8 +2233,18 @@ void ASTRecordWriter::AddFunctionDefinit
   Writer->ClearSwitchCaseIDs();
 
   assert(FD->doesThisDeclarationHaveABody());
-  bool ModulesCodegen = Writer->Context->getLangOpts().ModulesCodegen &&
-Writer->WritingModule && !FD->isDependentContext();
+  bool ModulesCodegen = false;
+  if (Writer->WritingModule && !FD->isDependentContext()) {
+// Under -fmodules-codegen, codegen is performed for all defined functions.
+// When building a C++ Modules TS module interface unit, a strong 
definition
+// in the module interface is provided by the compilation of that module
+// interface unit, not by its users. (Inline functions are still emitted
+// in module users.)
+ModulesCodegen =
+Writer->Context->getLangOpts().ModulesCodegen ||
+(Writer->WritingModule->Kind == Module::ModuleInterfaceUnit &&
+ Writer->Context->GetGVALinkageForFunction(FD) == GVA_StrongExternal);
+  }
   Record->push_back(ModulesCodegen);
   if (ModulesCodegen)
 Writer->ModularCodegenDecls.push_back(Writer->GetDeclRef(FD));

Added: cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp?rev=307232=auto
==
--- cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp (added)
+++ cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp Wed Jul  5 
17:30:00 2017
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fmodules-ts %S/module.cppm -triple %itanium_abi_triple 
-emit-module-interface -o %t
+// RUN: %clang_cc1 -fmodules-ts %s -triple %itanium_abi_triple 
-fmodule-file=%t -emit-llvm -o - | FileCheck %s --implicit-check-not=unused 
--implicit-check-not=global_module
+
+module Module;
+
+void use() {
+  // CHECK: define linkonce_odr {{.*}}@_Z20used_inline_exportedv
+  used_inline_exported();
+  // CHECK: declare {{.*}}@_Z18noninline_exportedv
+  noninline_exported();
+
+  // FIXME: This symbol should not be visible here.
+  // CHECK: define internal {{.*}}@_ZL26used_static_module_linkagev
+  used_static_module_linkage();
+
+  // FIXME: The module name should be mangled into the name of this function.
+  // CHECK: define linkonce_odr {{.*}}@_Z26used_inline_module_linkagev
+  used_inline_module_linkage();
+
+  // FIXME: The module name should be mangled into the name of this function.
+  // CHECK: declare {{.*}}@_Z24noninline_module_linkagev
+  noninline_module_linkage();
+}

Added: cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm?rev=307232=auto
==
--- cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm (added)
+++ cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm Wed Jul  5 
17:30:00 2017
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -fmodules-ts %s -triple %itanium_abi_triple -emit-llvm -o - 
| FileCheck %s --implicit-check-not=unused
+
+static void unused_static_global_module() {}
+static void used_static_global_module() {}
+inline void unused_inline_global_module() {}
+inline void used_inline_global_module() {}
+// CHECK: define void {{.*}}@_Z23noninline_global_modulev
+void noninline_global_module() {
+  // FIXME: This should be promoted to module linkage and given a
+  // module-mangled name, if it's called from an inline function within
+  // the module interface.
+  // (We should try to avoid this when it's not reachable from outside
+  // the module interface unit.)
+  // CHECK: define internal {{.*}}@_ZL25used_static_global_modulev
+  used_static_global_module();
+  // CHECK: define linkonce_odr {{.*}}@_Z25used_inline_global_modulev
+  used_inline_global_module();
+}
+
+export module Module;
+
+export {
+  // FIXME: These should be ill-formed: you can't 

r307231 - [cxx_status] Update link to Modules TS to latest working draft. Fix Coroutines TS flag to work if copy-pasted.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Wed Jul  5 17:29:13 2017
New Revision: 307231

URL: http://llvm.org/viewvc/llvm-project?rev=307231=rev
Log:
[cxx_status] Update link to Modules TS to latest working draft. Fix Coroutines 
TS flag to work if copy-pasted.

Modified:
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/www/cxx_status.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_status.html?rev=307231=307230=307231=diff
==
--- cfe/trunk/www/cxx_status.html (original)
+++ cfe/trunk/www/cxx_status.html Wed Jul  5 17:29:13 2017
@@ -14,6 +14,7 @@
 span:target { background-color: #BB; outline: #55 solid thin; }
 th { background-color: #FFDDAA }
 td { vertical-align: middle }
+tt { white-space: nowrap }
   
 
 
@@ -832,7 +833,7 @@ and library features that are not part o
 
   [DRAFT TS] Coroutines
   https://isocpp.org/files/papers/N4663.pdf;>N4663
-  fcoroutinests-stdlib=libc++
+  -fcoroutines-ts-stdlib=libc++
   SVN
 
 
@@ -849,7 +850,7 @@ and library features that are not part o
 
 
   [DRAFT TS] Modules
-  http://wg21.link/n4592;>N4592
+  http://wg21.link/n4667;>N4667
   -fmodules-ts
   WIP
 


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

[libcxxabi] r307230 - Fix incomplete type test on OS X; workaround weird DYLD_LIBRARY_PATH behavior

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Wed Jul  5 17:29:09 2017
New Revision: 307230

URL: http://llvm.org/viewvc/llvm-project?rev=307230=rev
Log:
Fix incomplete type test on OS X; workaround weird DYLD_LIBRARY_PATH behavior

Modified:
libcxxabi/trunk/test/incomplete_type.sh.cpp

Modified: libcxxabi/trunk/test/incomplete_type.sh.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/test/incomplete_type.sh.cpp?rev=307230=307229=307230=diff
==
--- libcxxabi/trunk/test/incomplete_type.sh.cpp (original)
+++ libcxxabi/trunk/test/incomplete_type.sh.cpp Wed Jul  5 17:29:09 2017
@@ -16,9 +16,13 @@
 
 // UNSUPPORTED: libcxxabi-no-exceptions
 
+// NOTE: Pass -lc++abi explicitly and before -lc++ so that -lc++ doesn't drag
+// in the system libc++abi installation on OS X. (DYLD_LIBRARY_PATH is ignored
+// for shell tests because of Apple security features).
+
 // RUN: %cxx %flags %compile_flags -c %s -o %t.one.o
 // RUN: %cxx %flags %compile_flags -c %s -o %t.two.o -DTU_ONE
-// RUN: %cxx %flags %t.one.o %t.two.o %link_flags -o %t.exe
+// RUN: %cxx %flags %t.one.o %t.two.o -lc++abi %link_flags -o %t.exe
 // RUN: %t.exe
 
 #include 


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

[PATCH] D33719: Add _Float16 as a C/C++ source language type

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

rsmith added a comment.

We need to be completely clear on the differences and interactions between 
`_Float16` and `__fp16`; you should include a documentation update that 
describes this as part of this change or as a companion change. In particular, 
an explanation of why we need two different types here is essential.




Comment at: lib/AST/ItaniumMangle.cpp:2457-2460
+  case BuiltinType::Float16:
   case BuiltinType::Half:
 Out << "Dh";
 break;

Distinct types should have distinct manglings. Please open an issue at 
https://github.com/itanium-cxx-abi/cxx-abi/ to discuss how to mangle these 
cases.



Comment at: lib/Lex/LiteralSupport.cpp:594
+  if (s + 2 < ThisTokEnd && s[1] == '1' && s[2] == '6') {
+  s += 2; // success, eat up 2 tokens.
+  isFloat16 = true;

s/tokens/characters/


https://reviews.llvm.org/D33719



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

[PATCH] D35038: [libunwind] Add a test harness

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs created this revision.
Herald added a subscriber: mgorny.

Mostly cargo-culted from libcxxabi, since the unwinder was forked from there in 
the first place.

Might still be some cruft that's only applicable to libcxxabi in here, so let 
me know if you spot anything like that. I killed some of it, but there could be 
more.


https://reviews.llvm.org/D35038

Files:
  CMakeLists.txt
  test/CMakeLists.txt
  test/libunwind/__init__.py
  test/libunwind/test/__init__.py
  test/libunwind/test/config.py
  test/libunwind_02.pass.cpp
  test/lit.cfg
  test/lit.site.cfg.in

Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -0,0 +1,25 @@
+@AUTO_GEN_COMMENT@
+config.cxx_under_test   = "@LIBUNWIND_COMPILER@"
+config.project_obj_root = "@CMAKE_BINARY_DIR@"
+config.libunwind_src_root   = "@LIBUNWIND_SOURCE_DIR@"
+config.libunwind_obj_root   = "@LIBUNWIND_BINARY_DIR@"
+config.abi_library_path = "@LIBUNWIND_LIBRARY_DIR@"
+config.libcxx_src_root  = "@LIBUNWIND_LIBCXX_PATH@"
+config.libunwind_headers= "@LIBUNWIND_SOURCE_DIR@/include"
+config.cxx_library_root = "@LIBUNWIND_LIBCXX_LIBRARY_PATH@"
+config.llvm_unwinder= "@LIBUNWIND_USE_LLVM_UNWINDER@"
+config.enable_threads   = "@LIBUNWIND_ENABLE_THREADS@"
+config.use_sanitizer= "@LLVM_USE_SANITIZER@"
+config.enable_32bit = "@LIBUNWIND_BUILD_32_BITS@"
+config.target_info  = "@LIBUNWIND_TARGET_INFO@"
+config.executor = "@LIBUNWIND_EXECUTOR@"
+config.libunwind_shared = "@LIBUNWIND_ENABLE_SHARED@"
+config.enable_shared= "@LIBCXX_ENABLE_SHARED@"
+config.enable_exceptions= "@LIBUNWIND_ENABLE_EXCEPTIONS@"
+config.host_triple  = "@LLVM_HOST_TRIPLE@"
+config.target_triple= "@TARGET_TRIPLE@"
+config.use_target   = len("@LIBUNWIND_TARGET_TRIPLE@") > 0
+config.cxx_ext_threads  = "@LIBUNWIND_BUILD_EXTERNAL_THREAD_LIBRARY@"
+
+# Let the main config do the real work.
+lit_config.load_config(config, "@LIBUNWIND_SOURCE_DIR@/test/lit.cfg")
Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -0,0 +1,67 @@
+# -*- Python -*- vim: set ft=python ts=4 sw=4 expandtab tw=79:
+
+# Configuration file for the 'lit' test runner.
+
+
+import os
+import site
+
+site.addsitedir(os.path.dirname(__file__))
+
+
+# Tell pylint that we know config and lit_config exist somewhere.
+if 'PYLINT_IMPORT' in os.environ:
+config = object()
+lit_config = object()
+
+# name: The name of this test suite.
+config.name = 'libunwind'
+
+# suffixes: A list of file extensions to treat as test files.
+config.suffixes = ['.cpp', '.s']
+
+# test_source_root: The root path where tests are located.
+config.test_source_root = os.path.dirname(__file__)
+
+# Infer the libcxx_test_source_root for configuration import.
+# If libcxx_source_root isn't specified in the config, assume that the libcxx
+# and libunwind source directories are sibling directories.
+libcxx_src_root = getattr(config, 'libcxx_src_root', None)
+if not libcxx_src_root:
+libcxx_src_root = os.path.join(config.test_source_root, '../../libcxx')
+libcxx_test_src_root = os.path.join(libcxx_src_root, 'utils')
+if os.path.isfile(os.path.join(libcxx_test_src_root, 'libcxx', '__init__.py')):
+site.addsitedir(libcxx_test_src_root)
+else:
+lit_config.fatal('Could not find libcxx test directory for test imports'
+ ' in: %s' % libcxx_test_src_root)
+
+# Infer the test_exec_root from the libcxx_object root.
+obj_root = getattr(config, 'libunwind_obj_root', None)
+
+# Check that the test exec root is known.
+if obj_root is None:
+import libcxx.test.config
+libcxx.test.config.loadSiteConfig(
+lit_config, config, 'libunwind_site_config', 'LIBCXXABI_SITE_CONFIG')
+obj_root = getattr(config, 'libunwind_obj_root', None)
+if obj_root is None:
+import tempfile
+obj_root = tempfile.mkdtemp(prefix='libunwind-testsuite-')
+lit_config.warning('Creating temporary directory for object root: %s' %
+   obj_root)
+
+config.test_exec_root = os.path.join(obj_root, 'test')
+
+cfg_variant = getattr(config, 'configuration_variant', 'libunwind')
+if cfg_variant:
+lit_config.note('Using configuration variant: %s' % cfg_variant)
+
+# Load the Configuration class from the module name .test.config.
+config_module_name = '.'.join([cfg_variant, 'test', 'config'])
+config_module = __import__(config_module_name, fromlist=['Configuration'])
+
+configuration = config_module.Configuration(lit_config, config)
+configuration.configure()
+configuration.print_config_info()
+config.test_format = configuration.get_test_format()
Index: test/libunwind_02.pass.cpp
===
--- 

[PATCH] D34992: Emit static constexpr member as available_externally definition

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

rsmith added inline comments.



Comment at: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp:15
+// CHECK: @_ZL4BAR3 = available_externally constant i32 44,
+static constexpr int BAR3 = 44;
+

mehdi_amini wrote:
> rsmith wrote:
> > mehdi_amini wrote:
> > > Looks like I have a bug here, this should be an internal.
> > I would imagine that we skip promotion of declaration to definition in this 
> > case if we already have a definition.
> > 
> > To that end, please add a testcase like this:
> > 
> > ```
> > struct Bar {
> >   static constexpr int BAZ = 42;
> > };
> > auto *use = ::BAZ;
> > const int Bar::BAZ;
> > ```
> > 
> > ... to make sure that we convert the definition of `Bar::BAZ` from 
> > `available_externally` to a strong definition (I think we should end up 
> > with `weak_odr` here).
> `weak_odr` in C++17 because it is an inline variable, but I expect a strong 
> definition in c++11.
> I'll add this, this is a good test-case!
Well, `weak_odr` is a kind of strong definition :)

Ah, I'd intended to change from emitting this as a strong definition to 
emitting it as `weak_odr` even in C++11 (for better forward-compatibility with 
C++17), but it looks like I never did so. So yes, we'd expect an 
ExternalLinkage global here (for now) in C++11.


https://reviews.llvm.org/D34992



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

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

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

gtbercea added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:478
+auto MArchList = DAL->getAllArgValues(options::OPT_march_EQ);
+assert(MArchList.size() < 2 && "At most one GPU arch allowed.");
+if (MArchList.empty())

hfinkel wrote:
> Can a user hit this? If so, it must be an actual diagnostic.
A user cannot hit this now, -Xopenmp-target does not lead to duplicate -march 
flags in DAL anymore.


https://reviews.llvm.org/D34784



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

[PATCH] D34114: [clang] A better format for unnecessary packed warning.

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

rsmith added a comment.

Some concrete suggestions throughout the patch, but I think we should take a 
step back and reconsider this warning approach: it seems bizarre for us to warn 
on any packed struct that happens to contain a `char`. It would make sense to 
warn if an `__attribute__((packed))` written in the source has *no* effect 
(because it's applied to a struct where all fields already have alignment 1, or 
because it's applied to a field that has alignment 1) -- even then I'm not 
entirely convinced this is a valuable warning, but I assume there's some reason 
you want to warn on it :)




Comment at: lib/AST/RecordLayoutBuilder.cpp:1894-1896
+  llvm::SmallString<128> WarningMsg = (UnnecessaryPackedFields.size() == 1)
+  ? StringRef("field:")
+  : StringRef("fields:");

We intend for our diagnostics to be translatable, so sentence fragments like 
this that assume a certain phrasing of an English-language diagnostic should 
not be hard-coded.

Instead, change the diagnostic to pass the number of fields and use %plural to 
select between the different word suffixes. See 
http://clang.llvm.org/docs/InternalsManual.html#formatting-a-diagnostic-argument
 for more information.



Comment at: lib/AST/RecordLayoutBuilder.cpp:1897-1900
+  for (const auto identifier : UnnecessaryPackedFields) {
+WarningMsg += " '";
+WarningMsg += identifier->getName();
+WarningMsg += "',";

Rather than building a potentially very long list here, consider passing each 
field name to the diagnostic as a separate argument, and only list the first 
few of them, for example:

```
packed attribute is unnecessary for field 'i'
packed attribute is unnecessary for fields 'i' and 'j'
packed attribute is unnecessary for fields 'i', 'j', and 'k'
packed attribute is unnecessary for fields 'i', 'j', 'k', ...
```



Comment at: test/CodeGenCXX/warn-padded-packed.cpp:19
 
-struct S4 {
-  int i; // expected-warning {{packed attribute is unnecessary for 'i'}}
+struct S4 { // expected-warning {{packed attribute is unnecessary for field: 
'i'}}
+  int i;

This looks backwards: the packed attribute *is* necessary for field `i` here 
(because it reduces `i`'s alignment from 4 to 1), but not for field `c`.



Comment at: test/CodeGenCXX/warn-padded-packed.cpp:49
 
 struct S9 { // expected-warning {{packed attribute is unnecessary for 'S9'}}
+  int x;

Again this seems wrong.



Comment at: test/CodeGenCXX/warn-padded-packed.cpp:75
 
+struct S14 { // expected-warning {{packed attribute is unnecessary for fields: 
'i', 'j'}}
+  int i;

Again, this looks exactly backwards.


Repository:
  rL LLVM

https://reviews.llvm.org/D34114



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

[PATCH] D34992: Emit static constexpr member as available_externally definition

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2374
+  llvm::Constant *Init = nullptr;
+  if (D && D->isConstexpr() && !D->isInline() && !D->hasAttr()) 
{
+const VarDecl *InitDecl;

rsmith wrote:
> Applying this for only `constexpr` variables seems unnecessarily 
> conservative; it seems we could equally do this for any variable that is 
> `const` and has no `mutable` fields (though we'd need to check for 
> `EmitConstantInit` failing in the general case).
OK I will improve, and include a test with a struct with a mutable field.



Comment at: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp:1
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple x86_64-linux-gnu | 
FileCheck %s
+

rsmith wrote:
> Please also test what happens in C++1z mode, particularly as static constexpr 
> data members are implicitly `inline` there.
This is already covered by ```clang/test/CodeGenCXX/cxx1z-inline-variables.cpp 
``` (I actually broke this test during development because I didn't know about 
inline variable). But I can also add coverage for it here if you'd like.



Comment at: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp:15
+// CHECK: @_ZL4BAR3 = available_externally constant i32 44,
+static constexpr int BAR3 = 44;
+

rsmith wrote:
> mehdi_amini wrote:
> > Looks like I have a bug here, this should be an internal.
> I would imagine that we skip promotion of declaration to definition in this 
> case if we already have a definition.
> 
> To that end, please add a testcase like this:
> 
> ```
> struct Bar {
>   static constexpr int BAZ = 42;
> };
> auto *use = ::BAZ;
> const int Bar::BAZ;
> ```
> 
> ... to make sure that we convert the definition of `Bar::BAZ` from 
> `available_externally` to a strong definition (I think we should end up with 
> `weak_odr` here).
`weak_odr` in C++17 because it is an inline variable, but I expect a strong 
definition in c++11.
I'll add this, this is a good test-case!


https://reviews.llvm.org/D34992



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

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

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

gtbercea updated this revision to Diff 105355.
gtbercea added a comment.

Address Comments.


https://reviews.llvm.org/D34784

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Cuda.cpp
  test/Driver/openmp-offload.c

Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -597,3 +597,19 @@
 // RUN:   | FileCheck -check-prefix=CHK-FOPENMP-IS-DEVICE %s
 
 // CHK-FOPENMP-IS-DEVICE: clang{{.*}} "-aux-triple" "powerpc64le--linux" {{.*}}.c" "-fopenmp-is-device" "-fopenmp-host-ir-file-path"
+
+/// ###
+
+/// Check -Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8 is passed when compiling for the device.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu -Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-EQ-TARGET %s
+
+// CHK-FOPENMP-EQ-TARGET: clang{{.*}} argument unused during compilation: '-Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8'
+
+/// ###
+
+/// Check -Xopenmp-target -march=pwr8 is passed when compiling for the device.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu -Xopenmp-target -march=pwr8 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-TARGET %s
+
+// CHK-FOPENMP-TARGET: clang{{.*}} argument unused during compilation: '-Xopenmp-target -march=pwr8'
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -212,8 +212,20 @@
   static_cast(getToolChain());
   assert(TC.getTriple().isNVPTX() && "Wrong platform");
 
+  StringRef GPUArchName;
+  std::vector GPUArchNames;
+  // If this is an OpenMP action we need to extract the device architecture from
+  // the -fopenmp-target-arch option.
+  if (JA.isDeviceOffloading(Action::OFK_OpenMP)) {
+GPUArchNames = Args.getAllArgValues(options::OPT_march_EQ);
+assert(GPUArchNames.size() == 1 &&
+   "Exactly one GPU Arch required for ptxas.");
+GPUArchName = GPUArchNames[0];
+  } else
+GPUArchName = JA.getOffloadingArch();
+
   // Obtain architecture from the action.
-  CudaArch gpu_arch = StringToCudaArch(JA.getOffloadingArch());
+  CudaArch gpu_arch = StringToCudaArch(GPUArchName);
   assert(gpu_arch != CudaArch::UNKNOWN &&
  "Device action expected to have an architecture.");
 
@@ -392,6 +404,22 @@
   CudaInstallation.AddCudaIncludeArgs(DriverArgs, CC1Args);
 }
 
+void AddMArchOption(DerivedArgList *DAL,
+const OptTable ,
+StringRef Opt) {
+  if (Opt.startswith("-march=")) {
+StringRef Arch = Opt.split("=").second;
+// Check if the arch provided is valid for this toolchain.
+// If not valid, ignore it.
+if (StringToCudaArch(Arch) != CudaArch::UNKNOWN) {
+  DAL->eraseArg(options::OPT_march_EQ);
+  DAL->AddJoinedArg(nullptr,
+  Opts.getOption(options::OPT_march_EQ),
+  Arch.str());
+}
+  }
+}
+
 llvm::opt::DerivedArgList *
 CudaToolChain::TranslateArgs(const llvm::opt::DerivedArgList ,
  StringRef BoundArch,
@@ -405,7 +433,7 @@
 
   // For OpenMP device offloading, append derived arguments. Make sure
   // flags are not duplicated.
-  // TODO: Append the compute capability.
+  // Also append the compute capability.
   if (DeviceOffloadKind == Action::OFK_OpenMP) {
 for (Arg *A : Args){
   bool IsDuplicate = false;
@@ -418,6 +446,49 @@
   if (!IsDuplicate)
 DAL->append(A);
 }
+
+// Get the compute capability from the -Xopenmp-target flag.
+auto OptList = Args.getAllArgValues(options::OPT_Xopenmp_target_EQ);
+
+// For each OPT_Xopenmp_target_EQ option, the function returns
+// two strings, the triple and the option.
+// The following format is assumed:
+//
+// -Xopenmp-target=nvptx64-nvidia-cuda -opt=val
+for (unsigned i = 0; i < OptList.size(); i+=2) {
+  StringRef Opt = OptList[i+1];
+  if (OptList[i] == getTripleString())
+AddMArchOption(DAL, Opts, Opt);
+}
+
+OptList = Args.getAllArgValues(options::OPT_Xopenmp_target);
+// When there is only one option in the list, the following format
+// is assumed:
+//
+// -Xopenmp-target -opt=val
+
+// By default, if no triple is explicitely specified, we
+// associate -opt=val with the toolchain specified under the
+// -fopenmp-targets flag (provided that there is only one such
+// toolchain specified).
+if (!OptList.empty() &&
+Args.getAllArgValues(
+options::OPT_fopenmp_targets_EQ).size() != 1)
+  

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

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

gtbercea updated this revision to Diff 105354.
gtbercea added a comment.

Address comments.


https://reviews.llvm.org/D34784

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Driver/ToolChains/CommonArgs.h
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/openmp-offload.c

Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -597,3 +597,76 @@
 // RUN:   | FileCheck -check-prefix=CHK-FOPENMP-IS-DEVICE %s
 
 // CHK-FOPENMP-IS-DEVICE: clang{{.*}} "-aux-triple" "powerpc64le--linux" {{.*}}.c" "-fopenmp-is-device" "-fopenmp-host-ir-file-path"
+
+/// ###
+
+/// Check -Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8 is passed when compiling for the device.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu -Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-EQ-TARGET %s
+
+// CHK-FOPENMP-EQ-TARGET: clang{{.*}} argument unused during compilation: '-Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8'
+
+/// ###
+
+/// Check -Xopenmp-target -march=pwr8 is passed when compiling for the device.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu -Xopenmp-target -march=pwr8 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-TARGET %s
+
+// CHK-FOPENMP-TARGET: clang{{.*}} argument unused during compilation: '-Xopenmp-target -march=pwr8'
+
+/// ###
+
+/// Check -Xopenmp-target triggers error when multiple triples are used.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu,nvptx64-nvidia-cuda -Xopenmp-target -march=pwr8 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-TARGET-ERROR %s
+
+// CHK-FOPENMP-TARGET-ERROR: clang{{.*}} error: cannot deduce implicit triple value for -Xopenmp-target, specify triple using -Xopenmp-target=
+
+/// ###
+
+/// Check -Xopenmp-target uses one of the archs provided when several archs are used.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -Xopenmp-target -march=sm_35 -Xopenmp-target -march=sm_60 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-TARGET-ARCHS %s
+
+// CHK-FOPENMP-TARGET-ARCHS: ptxas{{.*}}" "--gpu-name" "sm_60"
+// CHK-FOPENMP-TARGET-ARCHS: nvlink{{.*}}" "-arch" "sm_60"
+
+/// ###
+
+/// Check -Xopenmp-target -march=pwr8 is ignored in favor of sm_20 (default).
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu,nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvidia-cuda -march=pwr8 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-TARGET-WARNING %s
+
+// CHK-FOPENMP-TARGET-WARNING: ptxas{{.*}}" "--gpu-name" "sm_20"
+// CHK-FOPENMP-TARGET-WARNING: nvlink{{.*}}" "-arch" "sm_20"
+
+/// ###
+
+/// Check -Xopenmp-target -march=sm_35 works as expected when two triples are present.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu,nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvidia-cuda -march=sm_35 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-TARGET-COMPILATION %s
+
+// CHK-FOPENMP-TARGET-COMPILATION: ptxas{{.*}}" "--gpu-name" "sm_35"
+// CHK-FOPENMP-TARGET-COMPILATION: nvlink{{.*}}" "-arch" "sm_35"
+
+/// ###
+
+/// Check cubin file generation and usage by nvlink
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -save-temps -no-canonical-prefixes %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-CUBIN %s
+
+// CHK-CUBIN: clang{{.*}}" "-o" "{{.*}}-openmp-nvptx64-nvidia-cuda.s"
+// CHK-CUBIN-NEXT: ptxas{{.*}}" "--output-file" "{{.*}}-openmp-nvptx64-nvidia-cuda.cubin" "{{.*}}-openmp-nvptx64-nvidia-cuda.s"
+// CHK-CUBIN-NEXT: nvlink" "-o" "{{.*}}-openmp-nvptx64-nvidia-cuda" {{.*}} "openmp-offload-openmp-nvptx64-nvidia-cuda.cubin"
+
+/// ###
+
+/// Check cubin file generation and usage by nvlink
+// RUN:   touch %t1.o
+// RUN:   touch %t2.o
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -save-temps -no-canonical-prefixes %t1.o %t2.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-TWOCUBIN %s
+
+// CHK-TWOCUBIN: clang-offload-bundler" "-type=o" 

[PATCH] D34992: Emit static constexpr member as available_externally definition

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

rsmith added a comment.

We've had problems in the past with speculative emission of values like this 
resulting in us producing invalid symbol references. (Two cases I remember: an 
`available_externally` symbol references a symbol that should be emitted as 
`linkonce_odr` but which is not emitted in this TU, and an 
`available_externally` symbol references a symbol with `hidden` visibility that 
is actually defined in a different DSO. In both cases, if the value of the 
`available_externally` symbol is actually used, you end up with a program with 
an invalid symbol reference.)

I don't immediately see any ways that this change would be susceptible to such 
a problem, so perhaps our best bet is to try it and see if it actually breaks 
anything.




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2374
+  llvm::Constant *Init = nullptr;
+  if (D && D->isConstexpr() && !D->isInline() && !D->hasAttr()) 
{
+const VarDecl *InitDecl;

Applying this for only `constexpr` variables seems unnecessarily conservative; 
it seems we could equally do this for any variable that is `const` and has no 
`mutable` fields (though we'd need to check for `EmitConstantInit` failing in 
the general case).



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2376
+const VarDecl *InitDecl;
+const Expr *InitExpr = D->getAnyInitializer(InitDecl);
+if (InitExpr) {

ahatanak wrote:
> Does getAnyInitializer ever return a null pointer here when D is a c++11 
> constexpr?
Only in error cases, which shouldn't get this far.



Comment at: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp:1
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple x86_64-linux-gnu | 
FileCheck %s
+

Please also test what happens in C++1z mode, particularly as static constexpr 
data members are implicitly `inline` there.



Comment at: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp:15
+// CHECK: @_ZL4BAR3 = available_externally constant i32 44,
+static constexpr int BAR3 = 44;
+

mehdi_amini wrote:
> Looks like I have a bug here, this should be an internal.
I would imagine that we skip promotion of declaration to definition in this 
case if we already have a definition.

To that end, please add a testcase like this:

```
struct Bar {
  static constexpr int BAZ = 42;
};
auto *use = ::BAZ;
const int Bar::BAZ;
```

... to make sure that we convert the definition of `Bar::BAZ` from 
`available_externally` to a strong definition (I think we should end up with 
`weak_odr` here).


https://reviews.llvm.org/D34992



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

[PATCH] D34992: Emit static constexpr member as available_externally definition

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2376
+const VarDecl *InitDecl;
+const Expr *InitExpr = D->getAnyInitializer(InitDecl);
+if (InitExpr) {

ahatanak wrote:
> Does getAnyInitializer ever return a null pointer here when D is a c++11 
> constexpr?
That's a good question, I wouldn't expect so. I can try adding an assertion 
instead.
I guess @rsmith could confirm as well.


https://reviews.llvm.org/D34992



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

[PATCH] D34992: Emit static constexpr member as available_externally definition

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2376
+const VarDecl *InitDecl;
+const Expr *InitExpr = D->getAnyInitializer(InitDecl);
+if (InitExpr) {

Does getAnyInitializer ever return a null pointer here when D is a c++11 
constexpr?


https://reviews.llvm.org/D34992



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

[PATCH] D34114: [clang] A better format for unnecessary packed warning.

[Yan Wang via Phabricator via cfe-commits]

yawanng added a comment.

PING


Repository:
  rL LLVM

https://reviews.llvm.org/D34114



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

[libcxx] r307218 - Suppress -Waligned-allocation-unavailable warning in libc++ test suite

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Wed Jul  5 15:40:58 2017
New Revision: 307218

URL: http://llvm.org/viewvc/llvm-project?rev=307218=rev
Log:
Suppress -Waligned-allocation-unavailable warning in libc++ test suite

Modified:
libcxx/trunk/utils/libcxx/test/config.py

Modified: libcxx/trunk/utils/libcxx/test/config.py
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/utils/libcxx/test/config.py?rev=307218=307217=307218=diff
==
--- libcxx/trunk/utils/libcxx/test/config.py (original)
+++ libcxx/trunk/utils/libcxx/test/config.py Wed Jul  5 15:40:58 2017
@@ -888,6 +888,7 @@ class Configuration(object):
 self.cxx.addWarningFlagIfSupported('-Wno-c++11-extensions')
 self.cxx.addWarningFlagIfSupported('-Wno-user-defined-literals')
 self.cxx.addWarningFlagIfSupported('-Wno-noexcept-type')
+
self.cxx.addWarningFlagIfSupported('-Wno-aligned-allocation-unavailable')
 # These warnings should be enabled in order to support the MSVC
 # team using the test suite; They enable the warnings below and
 # expect the test suite to be clean.


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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

ping


https://reviews.llvm.org/D28953



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

Re: [PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

[Richard Smith via cfe-commits]

On 5 July 2017 at 14:09, Melanie Blower via Phabricator <
revi...@reviews.llvm.org> wrote:

> mibintc added a comment.
>
> Jonas asked about adding a new test to ensure that "-include
> stdc-predef.h" does not get added if the file doesn't exist.
>
> I added a reply to that but I can't see where it went. So I'm writing the
> reply again.
>
> The current version of the patch doesn't check for the existence of
> stdc-predef.h.  As far as I understand, this is consistent with gcc
> behavior. It is expected that if you are on a system without stdc-predef.h
> then you can add -ffreestanding.
>

That does not seem reasonable. Plenty of libc implementations exist that
don't provide this header. GCC might want to strongly express a preference
for glibc, but we don't.


> In a prior revision of this patch (see Diff 4), I attempted to iterate
> through the system directories to check for the existence of the file
> before adding -include with a full path name. However that patch didn't
> work. I was using this call to iterate through the system includes:
> getAllArgValues(options::OPT_isystem).  However, the directory where
> stdc-predef.h is located, /usr/include, is added into the search path by
> using the option -internal-externc-isystem.  
> getAllArgValues(options::OPT_isystem)
> does not iterate through the include directories which were added via
> -internal-externc-isystem. [Note: is this a bug?].  There is no enumeration
> value within options::OPT_? which corresponds to -internal-externc-isystem.
>
> I could check for the existence of this file: /usr/include/stdc-predef.h;
> I don't know whether this would be acceptable or if it's correct.


Trying to fake up headers search from the driver doesn't sound like a good
way forward. Perhaps instead you could add a -include-if-exists flag to
-cc1 for this purpose, where -include-if-exists X would expand to something
like

#if __has_include(X)
#include X
#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D31022: Implement P0298R3: `std::byte`

[Richard Smith via cfe-commits]

On 5 July 2017 at 14:29, James Y Knight  wrote:

> I actually didn't know that attribute existed. It does look like the right
> thing to use here. However, unfortunately, it appears it doesn't work for
> enum class in clang [[1]]. It *does* seem to work in GCC.
>
> Unfortunately, it seems that GCC has actually hardcoded the type name
> "std::byte" (in gcc/cp/decl.c). To be compatible with libstdc++, we'll
> either need to fix libstdc++ to use also may_alias, or also hardcode the
> name. The former seems like it ought to be an acceptable patch.
>

Hardcoding the name would be consistent with what we do for other library
types that the core language explicitly blesses (such as
std::initializer_list, std::type_info, std::bad_alloc, std::align_val_t). I
expect we will want the compiler to be able to identify "this is std::byte"
internally anyway, for diagnostic purposes (and one day probably for
constant expression evaluation, due to the "provides storage" rules).


> [[1]] E.g. Given:
> namespace std { enum class __attribute__((may_alias)) byte  : unsigned
> char {}; }
> int foo(std::byte* x) { return (int)*x; }
>
> Running:
> bin/clang -O2 -S -std=c++1z -emit-llvm -o - test.cc
>
> still has:
>   %0 = load i8, i8* %x, align 1, !tbaa !2
> ...
> !2 = !{!3, !3, i64 0}
> !3 = !{!"_ZTSSt4byte", !4, i64 0}
>
>
>
> On Wed, Jul 5, 2017 at 3:51 PM, David Majnemer via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> FWIW, doesn't this just need a may_alias attribute on the definition?
>> On Wed, Jul 5, 2017 at 2:39 PM James Y Knight via Phabricator via
>> cfe-commits  wrote:
>>
>>> jyknight added a comment.
>>>
>>> Ping. I don't think this got resolved, and I really wouldn't like to
>>> see  released in this state...can you either revert this from the library,
>>> or implement the compiler support, before the 5.0 release branch?
>>>
>>> In https://reviews.llvm.org/D31022#716702, @jyknight wrote:
>>>
>>> > I believe this needs compiler support, too, in order to treat
>>> >
>>> >   namespace std { enum class byte : unsigned char {}; }
>>> >
>>> > as directly having tbaa type "omnipotent char" instead of a subtype.
>>> >
>>> > That is, given:
>>> >
>>> >   void foo(char* x, int *y) {
>>> > x[1] = char(y[0] & 0xff);
>>> > x[0] = char((y[0] & 0xff00) >> 8);
>>> >   }
>>> >
>>> > the compiler assumes that x and y might alias each-other, and thus
>>> must have two loads of y[0]. If you replace "char" with "std::byte", the
>>> same should happen, but as of now does not.
>>>
>>>
>>>
>>>
>>>
>>> https://reviews.llvm.org/D31022
>>>
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: Option: Use GS instead of FS for TLS

[svk via cfe-commits]

I missed the second part.


Am 05.07.2017 um 23:38 schrieb svk:

Hello,

the attached patch introduces a -mtls-gs command line option for clang 
for the x86_64 target.
This option will force the usage of the GS instead of the FS register 
for TLS references and

stack canary references.
This is particularly useful for low-level kernel-side development.

Best regards,
Sven Konietzny


diff --git a/include/clang/Driver/Options.td b/include/clang/Driver/Options.td
index cdd4c94..dbf27b5 100644
--- a/include/clang/Driver/Options.td
+++ b/include/clang/Driver/Options.td
@@ -1801,6 +1801,9 @@ def mno_mpx : Flag<["-"], "mno-mpx">, Group;
 def mno_sgx : Flag<["-"], "mno-sgx">, Group;
 def mno_prefetchwt1 : Flag<["-"], "mno-prefetchwt1">, Group;
 
+def mtls_gs : Flag<["-"], "mtls-gs">, Group,
+  HelpText<"Use GS register instead of FS register for TLS (x86_64 only)">;
+
 def munaligned_access : Flag<["-"], "munaligned-access">, Group,
   HelpText<"Allow memory accesses to be unaligned (AArch32/AArch64 only)">;
 def mno_unaligned_access : Flag<["-"], "mno-unaligned-access">, Group,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35008: [AArch64] Produce the right kind of va_arg for windows

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Needs a testcase (to check the emitted IR).




Comment at: lib/CodeGen/TargetInfo.cpp:4769
 DarwinPCS
   };
 

Can we extend ABIKind rather than adding a separate boolean?


https://reviews.llvm.org/D35008



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

Option: Use GS instead of FS for TLS

[svk via cfe-commits]

Hello,

the attached patch introduces a -mtls-gs command line option for clang 
for the x86_64 target.
This option will force the usage of the GS instead of the FS register 
for TLS references and

stack canary references.
This is particularly useful for low-level kernel-side development.

Best regards,
Sven Konietzny
diff --git a/lib/Target/X86/X86.td b/lib/Target/X86/X86.td
index 4ca57fe..d5c5026 100644
--- a/lib/Target/X86/X86.td
+++ b/lib/Target/X86/X86.td
@@ -245,6 +245,10 @@ def FeatureSlowIncDec : SubtargetFeature<"slow-incdec", "SlowIncDec", "true",
 def FeatureSoftFloat
 : SubtargetFeature<"soft-float", "UseSoftFloat", "true",
"Use software floating point features.">;
+def FeatureTlsGs
+: SubtargetFeature<"tls-gs", "UseTlsGS", "true",
+   "Use GS instead of FS in 64-bit mode">;
+
 // On some X86 processors, there is no performance hazard to writing only the
 // lower parts of a YMM or ZMM register without clearing the upper part.
 def FeatureFastPartialYMMorZMMWrite
diff --git a/lib/Target/X86/X86ISelLowering.cpp b/lib/Target/X86/X86ISelLowering.cpp
index 12adb70..0d4a1d2 100644
--- a/lib/Target/X86/X86ISelLowering.cpp
+++ b/lib/Target/X86/X86ISelLowering.cpp
@@ -1982,7 +1982,8 @@ X86TargetLowering::findRepresentativeClass(const TargetRegisterInfo *TRI,
 
 unsigned X86TargetLowering::getAddressSpace() const {
   if (Subtarget.is64Bit())
-return (getTargetMachine().getCodeModel() == CodeModel::Kernel) ? 256 : 257;
+return (getTargetMachine().getCodeModel() == CodeModel::Kernel ||
+Subtarget.useTlsGS()) ? 256 : 257;
   return 256;
 }
 
@@ -2007,8 +2008,8 @@ Value *X86TargetLowering::getIRStackGuard(IRBuilder<> ) const {
   //  defines MX_TLS_STACK_GUARD_OFFSET with this value.
   return SegmentOffset(IRB, 0x10, getAddressSpace());
 } else {
-  // %fs:0x28, unless we're using a Kernel code model, in which case
-  // it's %gs:0x28.  gs:0x14 on i386.
+  // %fs:0x28, unless we're using a Kernel code model or tls-gs is
+  // specified, in which case it's %gs:0x28.  gs:0x14 on i386.
   unsigned Offset = (Subtarget.is64Bit()) ? 0x28 : 0x14;
   return SegmentOffset(IRB, Offset, getAddressSpace());
 }
@@ -14825,12 +14826,14 @@ static SDValue LowerToTLSLocalDynamicModel(GlobalAddressSDNode *GA,
 // Lower ISD::GlobalTLSAddress using the "initial exec" or "local exec" model.
 static SDValue LowerToTLSExecModel(GlobalAddressSDNode *GA, SelectionDAG ,
const EVT PtrVT, TLSModel::Model model,
-   bool is64Bit, bool isPIC) {
+   bool is64Bit, bool isPIC, bool useGS) {
   SDLoc dl(GA);
 
   // Get the Thread Pointer, which is %gs:0 (32-bit) or %fs:0 (64-bit).
-  Value *Ptr = Constant::getNullValue(Type::getInt8PtrTy(*DAG.getContext(),
- is64Bit ? 257 : 256));
+  // If useGS is true %gs:0 will be used in 64-bit mode.
+  Value *Ptr =
+Constant::getNullValue(Type::getInt8PtrTy(*DAG.getContext(),
+  (is64Bit && !useGS) ? 257 : 256));
 
   SDValue ThreadPointer =
   DAG.getLoad(PtrVT, dl, DAG.getEntryNode(), DAG.getIntPtrConstant(0, dl),
@@ -14902,7 +14905,7 @@ X86TargetLowering::LowerGlobalTLSAddress(SDValue Op, SelectionDAG ) const {
   case TLSModel::InitialExec:
   case TLSModel::LocalExec:
 return LowerToTLSExecModel(GA, DAG, PtrVT, model, Subtarget.is64Bit(),
-   PositionIndependent);
+   PositionIndependent, Subtarget.useTlsGS());
 }
 llvm_unreachable("Unknown TLS model.");
   }
@@ -25868,8 +25871,9 @@ X86TargetLowering::EmitLoweredSegAlloca(MachineInstr ,
 
   const bool Is64Bit = Subtarget.is64Bit();
   const bool IsLP64 = Subtarget.isTarget64BitLP64();
+  const bool UseTlsGS = Subtarget.useTlsGS();
 
-  const unsigned TlsReg = Is64Bit ? X86::FS : X86::GS;
+  const unsigned TlsReg = (Is64Bit && !UseTlsGS) ? X86::FS : X86::GS;
   const unsigned TlsOffset = IsLP64 ? 0x70 : Is64Bit ? 0x40 : 0x30;
 
   // BB:
diff --git a/lib/Target/X86/X86Subtarget.cpp b/lib/Target/X86/X86Subtarget.cpp
index 24845be..7ad4d99 100644
--- a/lib/Target/X86/X86Subtarget.cpp
+++ b/lib/Target/X86/X86Subtarget.cpp
@@ -340,6 +340,7 @@ void X86Subtarget::initializeEnvironment() {
   // FIXME: this is a known good value for Yonah. How about others?
   MaxInlineSizeThreshold = 128;
   UseSoftFloat = false;
+  UseTlsGS = false;
 }
 
 X86Subtarget ::initializeSubtargetDependencies(StringRef CPU,
diff --git a/lib/Target/X86/X86Subtarget.h b/lib/Target/X86/X86Subtarget.h
index fa0afe2..f16bd20 100644
--- a/lib/Target/X86/X86Subtarget.h
+++ b/lib/Target/X86/X86Subtarget.h
@@ -300,6 +300,9 @@ protected:
   /// Use software floating point for code generation.
   bool UseSoftFloat;
 
+  /// In 64-bit mode, use GS instead of FS for TLS references.
+  

Re: [PATCH] D31022: Implement P0298R3: `std::byte`

[James Y Knight via cfe-commits]

I actually didn't know that attribute existed. It does look like the right
thing to use here. However, unfortunately, it appears it doesn't work for
enum class in clang [[1]]. It *does* seem to work in GCC.

Unfortunately, it seems that GCC has actually hardcoded the type name
"std::byte" (in gcc/cp/decl.c). To be compatible with libstdc++, we'll
either need to fix libstdc++ to use also may_alias, or also hardcode the
name. The former seems like it ought to be an acceptable patch.


[[1]] E.g. Given:
namespace std { enum class __attribute__((may_alias)) byte  : unsigned char
{}; }
int foo(std::byte* x) { return (int)*x; }

Running:
bin/clang -O2 -S -std=c++1z -emit-llvm -o - test.cc

still has:
  %0 = load i8, i8* %x, align 1, !tbaa !2
...
!2 = !{!3, !3, i64 0}
!3 = !{!"_ZTSSt4byte", !4, i64 0}



On Wed, Jul 5, 2017 at 3:51 PM, David Majnemer via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> FWIW, doesn't this just need a may_alias attribute on the definition?
> On Wed, Jul 5, 2017 at 2:39 PM James Y Knight via Phabricator via
> cfe-commits  wrote:
>
>> jyknight added a comment.
>>
>> Ping. I don't think this got resolved, and I really wouldn't like to see
>> released in this state...can you either revert this from the library, or
>> implement the compiler support, before the 5.0 release branch?
>>
>> In https://reviews.llvm.org/D31022#716702, @jyknight wrote:
>>
>> > I believe this needs compiler support, too, in order to treat
>> >
>> >   namespace std { enum class byte : unsigned char {}; }
>> >
>> > as directly having tbaa type "omnipotent char" instead of a subtype.
>> >
>> > That is, given:
>> >
>> >   void foo(char* x, int *y) {
>> > x[1] = char(y[0] & 0xff);
>> > x[0] = char((y[0] & 0xff00) >> 8);
>> >   }
>> >
>> > the compiler assumes that x and y might alias each-other, and thus must
>> have two loads of y[0]. If you replace "char" with "std::byte", the same
>> should happen, but as of now does not.
>>
>>
>>
>>
>>
>> https://reviews.llvm.org/D31022
>>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r307197 - Cope with Range-v3's CONCEPT_REQUIRES idiom

[Aaron Ballman via cfe-commits]

On Wed, Jul 5, 2017 at 5:05 PM, Douglas Gregor  wrote:
>
> On Jul 5, 2017, at 1:33 PM, Aaron Ballman  wrote:
>
> On Wed, Jul 5, 2017 at 4:20 PM, Douglas Gregor via cfe-commits
>  wrote:
>
> Author: dgregor
> Date: Wed Jul  5 13:20:15 2017
> New Revision: 307197
>
> URL: http://llvm.org/viewvc/llvm-project?rev=307197=rev
> Log:
> Cope with Range-v3's CONCEPT_REQUIRES idiom
>
> Modified:
>cfe/trunk/lib/Sema/SemaTemplate.cpp
>cfe/trunk/test/SemaTemplate/overload-candidates.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=307197=307196=307197=diff
> ==
> --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplate.cpp Wed Jul  5 13:20:15 2017
> @@ -2831,10 +2831,44 @@ static void collectConjunctionTerms(Expr
>   Terms.push_back(Clause);
> }
>
> +// The ranges-v3 library uses an odd pattern of a top-level "||" with
> +// a left-hand side that is value-dependent but never true. Identify
> +// the idiom and ignore that term.
> +static Expr *lookThroughRangesV3Condition(Preprocessor , Expr *Cond) {
> +  // Top-level '||'.
> +  auto *BinOp = dyn_cast(Cond->IgnoreParenImpCasts());
> +  if (!BinOp) return Cond;
> +
> +  if (BinOp->getOpcode() != BO_LOr) return Cond;
> +
> +  // With an inner '==' that has a literal on the right-hand side.
> +  Expr *LHS = BinOp->getLHS();
> +  auto InnerBinOp = dyn_cast(LHS->IgnoreParenImpCasts());
>
>
> auto * please (or better yet, const auto * here and above).
>
>
> Sure, commit incoming.

Thanks!

>
>
> +  if (!InnerBinOp) return Cond;
> +
> +  if (InnerBinOp->getOpcode() != BO_EQ ||
> +  !isa(InnerBinOp->getRHS()))
> +return Cond;
> +
> +  // If the inner binary operation came from a macro expansion named
> +  // CONCEPT_REQUIRES or CONCEPT_REQUIRES_, return the right-hand side
> +  // of the '||', which is the real, user-provided condition.
> +  auto Loc = InnerBinOp->getExprLoc();
>
>
> Please do not use auto here.
>
>
> I’m fine with spelling it out, although I don’t know what criteria you’re
> applying here. Presumably it’s okay to use “auto” when the initializer is
> some form of cast to that type, but you prefer not to use “auto” elsewhere,
> despite the “Loc” in the variable name and in the initializer?

I believe we usually only use auto when the type is explicitly spelled
out in the initializer (dyn_cast, getAs, etc), when it's overly
complicated to spell (iterators), or when it's in a range-based for
loop (because it's generally quite obvious what the type will be based
on the container).

Loc here is somewhat obvious, but not entirely; we have a lot of
things named "location" (SourceLocation, PresumedLoc, FullSourceLoc,
and more).

>
> It's unfortunate that we have this special case instead of a more
> general mechanism. While I love Ranges v3, I'm not keen on a compiler
> hack just for it. These "tricks" have a habit of leaking out into
> other user code, which will behave differently than what happens with
>
> Ranges.
>
>
> It’s possible that we can do something more general here by converting the
> expression to disjunctive normal form and identifying when some of the
> top-level terms will never succeed.

That might work. Or Richard's suggestion below. I'm mostly worried
about people "trying this at home" and getting different results
because that seems inexplicable. Also, if the Ranges TS decides to
rename their macro, our compiler stops behaving nicely, which isn't
ideal.

~Aaron

>
> - Doug
>
> ~Aaron
>
> +  if (!Loc.isMacroID()) return Cond;
> +
> +  StringRef MacroName = PP.getImmediateMacroName(Loc);
> +  if (MacroName == "CONCEPT_REQUIRES" || MacroName == "CONCEPT_REQUIRES_")
> +return BinOp->getRHS();
> +
> +  return Cond;
> +}
> +
> /// Find the failed subexpression within enable_if, and describe it
> /// with a string.
> static std::pair
> findFailedEnableIfCondition(Sema , Expr *Cond) {
> +  Cond = lookThroughRangesV3Condition(S.PP, Cond);
> +
>   // Separate out all of the terms in a conjunction.
>   SmallVector Terms;
>   collectConjunctionTerms(Cond, Terms);
>
> Modified: cfe/trunk/test/SemaTemplate/overload-candidates.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/overload-candidates.cpp?rev=307197=307196=307197=diff
> ==
> --- cfe/trunk/test/SemaTemplate/overload-candidates.cpp (original)
> +++ cfe/trunk/test/SemaTemplate/overload-candidates.cpp Wed Jul  5 13:20:15
> 2017
> @@ -137,4 +137,30 @@ namespace PR15673 {
> #endif
>   void wibble() {}
>   void wobble() { wibble(); } // expected-error {{no matching function
> for call to 'wibble'}}
> +
> +  template
> +  struct some_passing_trait : std::true_type {};
> +
> +#if __cplusplus <= 199711L
> +  // 

Re: r307197 - Cope with Range-v3's CONCEPT_REQUIRES idiom

[Richard Smith via cfe-commits]

On 5 July 2017 at 14:05, Douglas Gregor via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> On Jul 5, 2017, at 1:33 PM, Aaron Ballman  wrote:
>
> On Wed, Jul 5, 2017 at 4:20 PM, Douglas Gregor via cfe-commits
>  wrote:
>
> Author: dgregor
> Date: Wed Jul  5 13:20:15 2017
> New Revision: 307197
>
> URL: http://llvm.org/viewvc/llvm-project?rev=307197=rev
> Log:
> Cope with Range-v3's CONCEPT_REQUIRES idiom
>
> Modified:
>cfe/trunk/lib/Sema/SemaTemplate.cpp
>cfe/trunk/test/SemaTemplate/overload-candidates.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaTemplate.cpp?rev=307197=307196=307197=diff
> 
> ==
> --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplate.cpp Wed Jul  5 13:20:15 2017
> @@ -2831,10 +2831,44 @@ static void collectConjunctionTerms(Expr
>   Terms.push_back(Clause);
> }
>
> +// The ranges-v3 library uses an odd pattern of a top-level "||" with
> +// a left-hand side that is value-dependent but never true. Identify
> +// the idiom and ignore that term.
> +static Expr *lookThroughRangesV3Condition(Preprocessor , Expr *Cond) {
> +  // Top-level '||'.
> +  auto *BinOp = dyn_cast(Cond->IgnoreParenImpCasts());
> +  if (!BinOp) return Cond;
> +
> +  if (BinOp->getOpcode() != BO_LOr) return Cond;
> +
> +  // With an inner '==' that has a literal on the right-hand side.
> +  Expr *LHS = BinOp->getLHS();
> +  auto InnerBinOp = dyn_cast(LHS->IgnoreParenImpCasts());
>
>
> auto * please (or better yet, const auto * here and above).
>
>
> Sure, commit incoming.
>
>
> +  if (!InnerBinOp) return Cond;
> +
> +  if (InnerBinOp->getOpcode() != BO_EQ ||
> +  !isa(InnerBinOp->getRHS()))
> +return Cond;
> +
> +  // If the inner binary operation came from a macro expansion named
> +  // CONCEPT_REQUIRES or CONCEPT_REQUIRES_, return the right-hand side
> +  // of the '||', which is the real, user-provided condition.
> +  auto Loc = InnerBinOp->getExprLoc();
>
>
> Please do not use auto here.
>
>
> I’m fine with spelling it out, although I don’t know what criteria you’re
> applying here. Presumably it’s okay to use “auto” when the initializer is
> some form of cast to that type, but you prefer not to use “auto” elsewhere,
> despite the “Loc” in the variable name and in the initializer?
>
> It's unfortunate that we have this special case instead of a more
> general mechanism. While I love Ranges v3, I'm not keen on a compiler
> hack just for it. These "tricks" have a habit of leaking out into
> other user code, which will behave differently than what happens with
>
> Ranges.
>
>
> It’s possible that we can do something more general here by converting the
> expression to disjunctive normal form and identifying when some of the
> top-level terms will never succeed.
>

Hmm. Ranges v3 produces things like:

template>

... for which the top-level "N == 43" check can succeed if N is explicitly
specified. Perhaps the trick we need here is to notice that N was not, in
fact, explicitly specified, and so a requirement on N is not likely to be
interesting.

- Doug
>
> ~Aaron
>
> +  if (!Loc.isMacroID()) return Cond;
> +
> +  StringRef MacroName = PP.getImmediateMacroName(Loc);
> +  if (MacroName == "CONCEPT_REQUIRES" || MacroName == "CONCEPT_REQUIRES_")
> +return BinOp->getRHS();
> +
> +  return Cond;
> +}
> +
> /// Find the failed subexpression within enable_if, and describe it
> /// with a string.
> static std::pair
> findFailedEnableIfCondition(Sema , Expr *Cond) {
> +  Cond = lookThroughRangesV3Condition(S.PP, Cond);
> +
>   // Separate out all of the terms in a conjunction.
>   SmallVector Terms;
>   collectConjunctionTerms(Cond, Terms);
>
> Modified: cfe/trunk/test/SemaTemplate/overload-candidates.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> SemaTemplate/overload-candidates.cpp?rev=307197=
> 307196=307197=diff
> 
> ==
> --- cfe/trunk/test/SemaTemplate/overload-candidates.cpp (original)
> +++ cfe/trunk/test/SemaTemplate/overload-candidates.cpp Wed Jul  5
> 13:20:15 2017
> @@ -137,4 +137,30 @@ namespace PR15673 {
> #endif
>   void wibble() {}
>   void wobble() { wibble(); } // expected-error {{no matching
> function for call to 'wibble'}}
> +
> +  template
> +  struct some_passing_trait : std::true_type {};
> +
> +#if __cplusplus <= 199711L
> +  // expected-warning@+4 {{default template arguments for a function
> template are a C++11 extension}}
> +  // expected-warning@+4 {{default template arguments for a function
> template are a C++11 extension}}
> +#endif
> +  template +   int n = 42,
> +   typename std::enable_if (some_passing_trait::value && some_trait::value), int>::type = 0>
> +  void almost_rangesv3(); // expected-note{{candidate template ignored:

r307202 - Fix test case in pre-C++11 mode; address Aaron Ballman's code review.

[Douglas Gregor via cfe-commits]

Author: dgregor
Date: Wed Jul  5 14:12:37 2017
New Revision: 307202

URL: http://llvm.org/viewvc/llvm-project?rev=307202=rev
Log:
Fix test case in pre-C++11 mode; address Aaron Ballman's code review.

Modified:
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/SemaTemplate/overload-candidates.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=307202=307201=307202=diff
==
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Wed Jul  5 14:12:37 2017
@@ -2843,7 +2843,7 @@ static Expr *lookThroughRangesV3Conditio
 
   // With an inner '==' that has a literal on the right-hand side.
   Expr *LHS = BinOp->getLHS();
-  auto InnerBinOp = dyn_cast(LHS->IgnoreParenImpCasts());
+  auto *InnerBinOp = dyn_cast(LHS->IgnoreParenImpCasts());
   if (!InnerBinOp) return Cond;
 
   if (InnerBinOp->getOpcode() != BO_EQ ||
@@ -2853,7 +2853,7 @@ static Expr *lookThroughRangesV3Conditio
   // If the inner binary operation came from a macro expansion named
   // CONCEPT_REQUIRES or CONCEPT_REQUIRES_, return the right-hand side
   // of the '||', which is the real, user-provided condition.
-  auto Loc = InnerBinOp->getExprLoc();
+  SourceLocation Loc = InnerBinOp->getExprLoc();
   if (!Loc.isMacroID()) return Cond;
 
   StringRef MacroName = PP.getImmediateMacroName(Loc);

Modified: cfe/trunk/test/SemaTemplate/overload-candidates.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/overload-candidates.cpp?rev=307202=307201=307202=diff
==
--- cfe/trunk/test/SemaTemplate/overload-candidates.cpp (original)
+++ cfe/trunk/test/SemaTemplate/overload-candidates.cpp Wed Jul  5 14:12:37 2017
@@ -157,7 +157,7 @@ namespace PR15673 {
 
 #if __cplusplus <= 199711L
   // expected-warning@+4 {{default template arguments for a function template 
are a C++11 extension}}
-  // expected-warning@+4 {{default template arguments for a function template 
are a C++11 extension}}
+  // expected-warning@+3 {{default template arguments for a function template 
are a C++11 extension}}
 #endif
   template::value && 
some_trait::value)>


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

[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

Jonas asked about adding a new test to ensure that "-include stdc-predef.h" 
does not get added if the file doesn't exist.

I added a reply to that but I can't see where it went. So I'm writing the reply 
again.

The current version of the patch doesn't check for the existence of 
stdc-predef.h.  As far as I understand, this is consistent with gcc behavior. 
It is expected that if you are on a system without stdc-predef.h then you can 
add -ffreestanding.

In a prior revision of this patch (see Diff 4), I attempted to iterate through 
the system directories to check for the existence of the file before adding 
-include with a full path name. However that patch didn't work. I was using 
this call to iterate through the system includes: 
getAllArgValues(options::OPT_isystem).  However, the directory where 
stdc-predef.h is located, /usr/include, is added into the search path by using 
the option -internal-externc-isystem.  getAllArgValues(options::OPT_isystem) 
does not iterate through the include directories which were added via 
-internal-externc-isystem. [Note: is this a bug?].  There is no enumeration 
value within options::OPT_? which corresponds to -internal-externc-isystem.

I could check for the existence of this file: /usr/include/stdc-predef.h; I 
don't know whether this would be acceptable or if it's correct.


https://reviews.llvm.org/D34158



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

r307201 - Fix one more reference to lit.util.capture()

[Michael Zolotukhin via cfe-commits]

Author: mzolotukhin
Date: Wed Jul  5 14:06:11 2017
New Revision: 307201

URL: http://llvm.org/viewvc/llvm-project?rev=307201=rev
Log:
Fix one more reference to lit.util.capture()

The capture method was removed in r306643.

Modified:
cfe/trunk/utils/perf-training/lit.cfg
cfe/trunk/utils/perf-training/order-files.lit.cfg

Modified: cfe/trunk/utils/perf-training/lit.cfg
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/perf-training/lit.cfg?rev=307201=307200=307201=diff
==
--- cfe/trunk/utils/perf-training/lit.cfg (original)
+++ cfe/trunk/utils/perf-training/lit.cfg Wed Jul  5 14:06:11 2017
@@ -3,13 +3,14 @@
 from lit import Test
 import lit.formats
 import lit.util
+import subprocess
 
 def getSysrootFlagsOnDarwin(config, lit_config):
 # On Darwin, support relocatable SDKs by providing Clang with a
 # default system root path.
 if 'darwin' in config.target_triple:
 try:
-out = lit.util.capture(['xcrun', '--show-sdk-path']).strip()
+out = subprocess.check_output(['xcrun', '--show-sdk-path']).strip()
 res = 0
 except OSError:
 res = -1

Modified: cfe/trunk/utils/perf-training/order-files.lit.cfg
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/perf-training/order-files.lit.cfg?rev=307201=307200=307201=diff
==
--- cfe/trunk/utils/perf-training/order-files.lit.cfg (original)
+++ cfe/trunk/utils/perf-training/order-files.lit.cfg Wed Jul  5 14:06:11 2017
@@ -4,13 +4,14 @@ from lit import Test
 import lit.formats
 import lit.util
 import os
+import subprocess
 
 def getSysrootFlagsOnDarwin(config, lit_config):
 # On Darwin, support relocatable SDKs by providing Clang with a
 # default system root path.
 if 'darwin' in config.target_triple:
 try:
-out = lit.util.capture(['xcrun', '--show-sdk-path']).strip()
+out = subprocess.check_output(['xcrun', '--show-sdk-path']).strip()
 res = 0
 except OSError:
 res = -1


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

Re: r307197 - Cope with Range-v3's CONCEPT_REQUIRES idiom

[Douglas Gregor via cfe-commits]

> On Jul 5, 2017, at 1:33 PM, Aaron Ballman  wrote:
> 
> On Wed, Jul 5, 2017 at 4:20 PM, Douglas Gregor via cfe-commits
> > wrote:
>> Author: dgregor
>> Date: Wed Jul  5 13:20:15 2017
>> New Revision: 307197
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=307197=rev
>> Log:
>> Cope with Range-v3's CONCEPT_REQUIRES idiom
>> 
>> Modified:
>>cfe/trunk/lib/Sema/SemaTemplate.cpp
>>cfe/trunk/test/SemaTemplate/overload-candidates.cpp
>> 
>> Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=307197=307196=307197=diff
>> ==
>> --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaTemplate.cpp Wed Jul  5 13:20:15 2017
>> @@ -2831,10 +2831,44 @@ static void collectConjunctionTerms(Expr
>>   Terms.push_back(Clause);
>> }
>> 
>> +// The ranges-v3 library uses an odd pattern of a top-level "||" with
>> +// a left-hand side that is value-dependent but never true. Identify
>> +// the idiom and ignore that term.
>> +static Expr *lookThroughRangesV3Condition(Preprocessor , Expr *Cond) {
>> +  // Top-level '||'.
>> +  auto *BinOp = dyn_cast(Cond->IgnoreParenImpCasts());
>> +  if (!BinOp) return Cond;
>> +
>> +  if (BinOp->getOpcode() != BO_LOr) return Cond;
>> +
>> +  // With an inner '==' that has a literal on the right-hand side.
>> +  Expr *LHS = BinOp->getLHS();
>> +  auto InnerBinOp = dyn_cast(LHS->IgnoreParenImpCasts());
> 
> auto * please (or better yet, const auto * here and above).

Sure, commit incoming.

> 
>> +  if (!InnerBinOp) return Cond;
>> +
>> +  if (InnerBinOp->getOpcode() != BO_EQ ||
>> +  !isa(InnerBinOp->getRHS()))
>> +return Cond;
>> +
>> +  // If the inner binary operation came from a macro expansion named
>> +  // CONCEPT_REQUIRES or CONCEPT_REQUIRES_, return the right-hand side
>> +  // of the '||', which is the real, user-provided condition.
>> +  auto Loc = InnerBinOp->getExprLoc();
> 
> Please do not use auto here.

I’m fine with spelling it out, although I don’t know what criteria you’re 
applying here. Presumably it’s okay to use “auto” when the initializer is some 
form of cast to that type, but you prefer not to use “auto” elsewhere, despite 
the “Loc” in the variable name and in the initializer?

> It's unfortunate that we have this special case instead of a more
> general mechanism. While I love Ranges v3, I'm not keen on a compiler
> hack just for it. These "tricks" have a habit of leaking out into
> other user code, which will behave differently than what happens with
> Ranges.

It’s possible that we can do something more general here by converting the 
expression to disjunctive normal form and identifying when some of the 
top-level terms will never succeed. 

- Doug

> ~Aaron
> 
>> +  if (!Loc.isMacroID()) return Cond;
>> +
>> +  StringRef MacroName = PP.getImmediateMacroName(Loc);
>> +  if (MacroName == "CONCEPT_REQUIRES" || MacroName == "CONCEPT_REQUIRES_")
>> +return BinOp->getRHS();
>> +
>> +  return Cond;
>> +}
>> +
>> /// Find the failed subexpression within enable_if, and describe it
>> /// with a string.
>> static std::pair
>> findFailedEnableIfCondition(Sema , Expr *Cond) {
>> +  Cond = lookThroughRangesV3Condition(S.PP, Cond);
>> +
>>   // Separate out all of the terms in a conjunction.
>>   SmallVector Terms;
>>   collectConjunctionTerms(Cond, Terms);
>> 
>> Modified: cfe/trunk/test/SemaTemplate/overload-candidates.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/overload-candidates.cpp?rev=307197=307196=307197=diff
>> ==
>> --- cfe/trunk/test/SemaTemplate/overload-candidates.cpp (original)
>> +++ cfe/trunk/test/SemaTemplate/overload-candidates.cpp Wed Jul  5 13:20:15 
>> 2017
>> @@ -137,4 +137,30 @@ namespace PR15673 {
>> #endif
>>   void wibble() {}
>>   void wobble() { wibble(); } // expected-error {{no matching function 
>> for call to 'wibble'}}
>> +
>> +  template
>> +  struct some_passing_trait : std::true_type {};
>> +
>> +#if __cplusplus <= 199711L
>> +  // expected-warning@+4 {{default template arguments for a function 
>> template are a C++11 extension}}
>> +  // expected-warning@+4 {{default template arguments for a function 
>> template are a C++11 extension}}
>> +#endif
>> +  template> +   int n = 42,
>> +   typename std::enable_if::value 
>> && some_trait::value), int>::type = 0>
>> +  void almost_rangesv3(); // expected-note{{candidate template ignored: 
>> requirement '42 == 43 || (some_passing_trait::value && 
>> some_trait::value)' was not satisfied}}
>> +  void test_almost_rangesv3() { almost_rangesv3(); } // 
>> expected-error{{no matching function for call to 'almost_rangesv3'}}
>> +
>> +  #define CONCEPT_REQUIRES_(...)  

[PATCH] D34927: [Bash-autocompletion] Fix a bug that -foo=bar doesn't handled properly

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.
This revision is now accepted and ready to land.

LGTM.


https://reviews.llvm.org/D34927



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

[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 105329.
mibintc added a comment.

I responded to the review from Jonas Hahnfeld: I moved the entire change into 
ToolChains/Linux and removed the modifications to Gnu.h and Gnu.cpp; I modified 
the new tests to use -### with FileCheck, and added a tree in Inputs/ for the 
new tests.  Jonas please see inline reply concerning whether to--or how 
to--check for existence of stdc-predef.h

Also, note that there are some tests from clang-tidy that need to be updated. I 
have a separate patch for that since it's in a different repo. I assume it 
would be good to get those modifications landed before this patch.


https://reviews.llvm.org/D34158

Files:
  lib/Driver/ToolChains/Linux.cpp
  lib/Driver/ToolChains/Linux.h
  test/Driver/Inputs/stdc-predef/bin/.keep
  test/Driver/Inputs/stdc-predef/usr/i386-unknown-linux/lib/.keep
  test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h
  test/Driver/Inputs/stdc-predef/usr/lib/.keep
  test/Driver/Inputs/stdc-predef/usr/x86_64-unknown-linux/lib/.keep
  test/Driver/clang_cpp.c
  test/Driver/crash-report.c
  test/Driver/gcc-predef-not.c
  test/Driver/gcc-predef.c
  test/Index/IBOutletCollection.m
  test/Index/annotate-macro-args.m
  test/Index/annotate-tokens-pp.c
  test/Index/annotate-tokens.c
  test/Index/c-index-getCursor-test.m
  test/Index/get-cursor-macro-args.m
  test/Index/get-cursor.cpp
  unittests/Tooling/TestVisitor.h

Index: lib/Driver/ToolChains/Linux.h
===
--- lib/Driver/ToolChains/Linux.h
+++ lib/Driver/ToolChains/Linux.h
@@ -31,6 +31,8 @@
   void addLibStdCxxIncludePaths(
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const override;
+  void AddGnuIncludeArgs(const llvm::opt::ArgList ,
+  llvm::opt::ArgStringList ) const;
   void AddCudaIncludeArgs(const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const override;
   void AddIAMCUIncludeArgs(const llvm::opt::ArgList ,
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -705,6 +705,8 @@
   addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/include");
 
   addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
+
+  AddGnuIncludeArgs(DriverArgs, CC1Args);
 }
 
 static std::string DetectLibcxxIncludePath(StringRef base) {
@@ -743,6 +745,22 @@
   return "";
 }
 
+void Linux::AddGnuIncludeArgs(const llvm::opt::ArgList ,
+  llvm::opt::ArgStringList ) const {
+  if (GCCInstallation.isValid()) {
+const Generic_GCC::GCCVersion  = GCCInstallation.getVersion();
+if (!DriverArgs.hasArg(options::OPT_ffreestanding) &&
+!DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
+!Version.isOlderThan(4, 8, 0)) {
+  // For gcc >= 4.8.x, clang will preinclude 
+  // -ffreestanding suppresses this behavior.
+  CC1Args.push_back("-include");
+  CC1Args.push_back("stdc-predef.h");
+}
+  }
+}
+
+
 void Linux::addLibStdCxxIncludePaths(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ) const {
   // We need a detected GCC installation on Linux to provide libstdc++'s
Index: unittests/Tooling/TestVisitor.h
===
--- unittests/Tooling/TestVisitor.h
+++ unittests/Tooling/TestVisitor.h
@@ -52,6 +52,7 @@
   /// \brief Runs the current AST visitor over the given code.
   bool runOver(StringRef Code, Language L = Lang_CXX) {
 std::vector Args;
+Args.push_back("-nostdinc");
 switch (L) {
   case Lang_C: Args.push_back("-std=c99"); break;
   case Lang_CXX98: Args.push_back("-std=c++98"); break;
Index: test/Driver/gcc-predef.c
===
--- test/Driver/gcc-predef.c
+++ test/Driver/gcc-predef.c
@@ -0,0 +1,8 @@
+// Test that clang preincludes stdc-predef.h on Linux with gcc installations
+//
+// RUN: %clang %s -### -c 2>&1 \
+// RUN: --gcc-toolchain="" --sysroot=%S/Inputs/stdc-predef \
+// RUN: | FileCheck -check-prefix CHECK-PREDEF %s
+
+// CHECK-PREDEF: "-include" "stdc-predef.h"
+int i;
Index: test/Driver/gcc-predef-not.c
===
--- test/Driver/gcc-predef-not.c
+++ test/Driver/gcc-predef-not.c
@@ -0,0 +1,7 @@
+// Test that -ffreestanding suppresses the preinclude of stdc-predef.h.
+//
+// RUN: %clang %s -### -c -ffreestanding 2>&1 \
+// RUN: --gcc-toolchain="" --sysroot=%S/Inputs/stdc-predef \
+// RUN: | FileCheck --implicit-check-not '"-include" "stdc-predef.h"' %s
+
+int i;
Index: test/Driver/crash-report.c
===
--- test/Driver/crash-report.c
+++ test/Driver/crash-report.c
@@ -2,7 +2,7 @@
 // RUN: mkdir %t
 // RUN: not env TMPDIR=%t TEMP=%t 

[PATCH] D34955: [Basic] Detect Git submodule version in CMake

[Jordan Rose via Phabricator via cfe-commits]

jordan_rose added a comment.

I'm not sure why we would care if a commit is made on a branch. The important 
information here is what commit is checked out, and that's what HEAD refers to.


https://reviews.llvm.org/D34955



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

Re: r307197 - Cope with Range-v3's CONCEPT_REQUIRES idiom

[Aaron Ballman via cfe-commits]

On Wed, Jul 5, 2017 at 4:20 PM, Douglas Gregor via cfe-commits
 wrote:
> Author: dgregor
> Date: Wed Jul  5 13:20:15 2017
> New Revision: 307197
>
> URL: http://llvm.org/viewvc/llvm-project?rev=307197=rev
> Log:
> Cope with Range-v3's CONCEPT_REQUIRES idiom
>
> Modified:
> cfe/trunk/lib/Sema/SemaTemplate.cpp
> cfe/trunk/test/SemaTemplate/overload-candidates.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=307197=307196=307197=diff
> ==
> --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplate.cpp Wed Jul  5 13:20:15 2017
> @@ -2831,10 +2831,44 @@ static void collectConjunctionTerms(Expr
>Terms.push_back(Clause);
>  }
>
> +// The ranges-v3 library uses an odd pattern of a top-level "||" with
> +// a left-hand side that is value-dependent but never true. Identify
> +// the idiom and ignore that term.
> +static Expr *lookThroughRangesV3Condition(Preprocessor , Expr *Cond) {
> +  // Top-level '||'.
> +  auto *BinOp = dyn_cast(Cond->IgnoreParenImpCasts());
> +  if (!BinOp) return Cond;
> +
> +  if (BinOp->getOpcode() != BO_LOr) return Cond;
> +
> +  // With an inner '==' that has a literal on the right-hand side.
> +  Expr *LHS = BinOp->getLHS();
> +  auto InnerBinOp = dyn_cast(LHS->IgnoreParenImpCasts());

auto * please (or better yet, const auto * here and above).

> +  if (!InnerBinOp) return Cond;
> +
> +  if (InnerBinOp->getOpcode() != BO_EQ ||
> +  !isa(InnerBinOp->getRHS()))
> +return Cond;
> +
> +  // If the inner binary operation came from a macro expansion named
> +  // CONCEPT_REQUIRES or CONCEPT_REQUIRES_, return the right-hand side
> +  // of the '||', which is the real, user-provided condition.
> +  auto Loc = InnerBinOp->getExprLoc();

Please do not use auto here.

It's unfortunate that we have this special case instead of a more
general mechanism. While I love Ranges v3, I'm not keen on a compiler
hack just for it. These "tricks" have a habit of leaking out into
other user code, which will behave differently than what happens with
Ranges.

~Aaron

> +  if (!Loc.isMacroID()) return Cond;
> +
> +  StringRef MacroName = PP.getImmediateMacroName(Loc);
> +  if (MacroName == "CONCEPT_REQUIRES" || MacroName == "CONCEPT_REQUIRES_")
> +return BinOp->getRHS();
> +
> +  return Cond;
> +}
> +
>  /// Find the failed subexpression within enable_if, and describe it
>  /// with a string.
>  static std::pair
>  findFailedEnableIfCondition(Sema , Expr *Cond) {
> +  Cond = lookThroughRangesV3Condition(S.PP, Cond);
> +
>// Separate out all of the terms in a conjunction.
>SmallVector Terms;
>collectConjunctionTerms(Cond, Terms);
>
> Modified: cfe/trunk/test/SemaTemplate/overload-candidates.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/overload-candidates.cpp?rev=307197=307196=307197=diff
> ==
> --- cfe/trunk/test/SemaTemplate/overload-candidates.cpp (original)
> +++ cfe/trunk/test/SemaTemplate/overload-candidates.cpp Wed Jul  5 13:20:15 
> 2017
> @@ -137,4 +137,30 @@ namespace PR15673 {
>  #endif
>void wibble() {}
>void wobble() { wibble(); } // expected-error {{no matching function 
> for call to 'wibble'}}
> +
> +  template
> +  struct some_passing_trait : std::true_type {};
> +
> +#if __cplusplus <= 199711L
> +  // expected-warning@+4 {{default template arguments for a function 
> template are a C++11 extension}}
> +  // expected-warning@+4 {{default template arguments for a function 
> template are a C++11 extension}}
> +#endif
> +  template +   int n = 42,
> +   typename std::enable_if::value 
> && some_trait::value), int>::type = 0>
> +  void almost_rangesv3(); // expected-note{{candidate template ignored: 
> requirement '42 == 43 || (some_passing_trait::value && 
> some_trait::value)' was not satisfied}}
> +  void test_almost_rangesv3() { almost_rangesv3(); } // 
> expected-error{{no matching function for call to 'almost_rangesv3'}}
> +
> +  #define CONCEPT_REQUIRES_(...)\
> +int x = 42, \
> +typename std::enable_if<(x == 43) || (__VA_ARGS__)>::type = 0
> +
> +#if __cplusplus <= 199711L
> +  // expected-warning@+4 {{default template arguments for a function 
> template are a C++11 extension}}
> +  // expected-warning@+4 {{default template arguments for a function 
> template are a C++11 extension}}
> +#endif
> +  template +   CONCEPT_REQUIRES_(some_passing_trait::value && 
> some_trait::value)>
> +  void rangesv3(); // expected-note{{candidate template ignored: requirement 
> 'some_trait::value' was not satisfied [with T = int, x = 42]}}
> +  void test_rangesv3() { rangesv3(); } // expected-error{{no matching 

[PATCH] D34955: [Basic] Detect Git submodule version in CMake

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

FYI, I don't think `HEAD` on its own will work because it is not necessarily 
updated on every commit, see discussion on https://reviews.llvm.org/D31985.


https://reviews.llvm.org/D34955



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

[PATCH] D34955: [Basic] Detect Git submodule version in CMake

[Brian Gesiak via Phabricator via cfe-commits]

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

Oh, nice catch @jordan_rose, you're absolutely right. I just tried updating my 
Git submodule reference for Clang and rebuilding my project, but the commit 
hash shown for Clang didn't change accordingly.

I'll need to find a file that changes on every commit. Following the Git 
submodule path, `gitdir: ../../.git/modules/external/llvm` and checking for 
`../../.git/modules/external/llvm/HEAD` appears to work. I might rewrite the 
patch to do that, or try migrating this to use LLVM's source control checking 
(I think it exists in `llvm/cmake/modules/VersionFromVCS.cmake`).


https://reviews.llvm.org/D34955



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

r307197 - Cope with Range-v3's CONCEPT_REQUIRES idiom

[Douglas Gregor via cfe-commits]

Author: dgregor
Date: Wed Jul  5 13:20:15 2017
New Revision: 307197

URL: http://llvm.org/viewvc/llvm-project?rev=307197=rev
Log:
Cope with Range-v3's CONCEPT_REQUIRES idiom

Modified:
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/SemaTemplate/overload-candidates.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=307197=307196=307197=diff
==
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Wed Jul  5 13:20:15 2017
@@ -2831,10 +2831,44 @@ static void collectConjunctionTerms(Expr
   Terms.push_back(Clause);
 }
 
+// The ranges-v3 library uses an odd pattern of a top-level "||" with
+// a left-hand side that is value-dependent but never true. Identify
+// the idiom and ignore that term.
+static Expr *lookThroughRangesV3Condition(Preprocessor , Expr *Cond) {
+  // Top-level '||'.
+  auto *BinOp = dyn_cast(Cond->IgnoreParenImpCasts());
+  if (!BinOp) return Cond;
+
+  if (BinOp->getOpcode() != BO_LOr) return Cond;
+
+  // With an inner '==' that has a literal on the right-hand side.
+  Expr *LHS = BinOp->getLHS();
+  auto InnerBinOp = dyn_cast(LHS->IgnoreParenImpCasts());
+  if (!InnerBinOp) return Cond;
+
+  if (InnerBinOp->getOpcode() != BO_EQ ||
+  !isa(InnerBinOp->getRHS()))
+return Cond;
+
+  // If the inner binary operation came from a macro expansion named
+  // CONCEPT_REQUIRES or CONCEPT_REQUIRES_, return the right-hand side
+  // of the '||', which is the real, user-provided condition.
+  auto Loc = InnerBinOp->getExprLoc();
+  if (!Loc.isMacroID()) return Cond;
+
+  StringRef MacroName = PP.getImmediateMacroName(Loc);
+  if (MacroName == "CONCEPT_REQUIRES" || MacroName == "CONCEPT_REQUIRES_")
+return BinOp->getRHS();
+
+  return Cond;
+}
+
 /// Find the failed subexpression within enable_if, and describe it
 /// with a string.
 static std::pair
 findFailedEnableIfCondition(Sema , Expr *Cond) {
+  Cond = lookThroughRangesV3Condition(S.PP, Cond);
+
   // Separate out all of the terms in a conjunction.
   SmallVector Terms;
   collectConjunctionTerms(Cond, Terms);

Modified: cfe/trunk/test/SemaTemplate/overload-candidates.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/overload-candidates.cpp?rev=307197=307196=307197=diff
==
--- cfe/trunk/test/SemaTemplate/overload-candidates.cpp (original)
+++ cfe/trunk/test/SemaTemplate/overload-candidates.cpp Wed Jul  5 13:20:15 2017
@@ -137,4 +137,30 @@ namespace PR15673 {
 #endif
   void wibble() {}
   void wobble() { wibble(); } // expected-error {{no matching function 
for call to 'wibble'}}
+
+  template
+  struct some_passing_trait : std::true_type {};
+
+#if __cplusplus <= 199711L
+  // expected-warning@+4 {{default template arguments for a function template 
are a C++11 extension}}
+  // expected-warning@+4 {{default template arguments for a function template 
are a C++11 extension}}
+#endif
+  template::value && 
some_trait::value), int>::type = 0>
+  void almost_rangesv3(); // expected-note{{candidate template ignored: 
requirement '42 == 43 || (some_passing_trait::value && 
some_trait::value)' was not satisfied}}
+  void test_almost_rangesv3() { almost_rangesv3(); } // 
expected-error{{no matching function for call to 'almost_rangesv3'}}
+
+  #define CONCEPT_REQUIRES_(...)\
+int x = 42, \
+typename std::enable_if<(x == 43) || (__VA_ARGS__)>::type = 0
+
+#if __cplusplus <= 199711L
+  // expected-warning@+4 {{default template arguments for a function template 
are a C++11 extension}}
+  // expected-warning@+4 {{default template arguments for a function template 
are a C++11 extension}}
+#endif
+  template::value && 
some_trait::value)>
+  void rangesv3(); // expected-note{{candidate template ignored: requirement 
'some_trait::value' was not satisfied [with T = int, x = 42]}}
+  void test_rangesv3() { rangesv3(); } // expected-error{{no matching 
function for call to 'rangesv3'}}
 }


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

r307196 - Customize the SFINAE diagnostics for enable_if to provide the failed condition.

[Douglas Gregor via cfe-commits]

Author: dgregor
Date: Wed Jul  5 13:20:14 2017
New Revision: 307196

URL: http://llvm.org/viewvc/llvm-project?rev=307196=rev
Log:
Customize the SFINAE diagnostics for enable_if to provide the failed condition.

When enable_if disables a particular overload resolution candidate,
rummage through the enable_if condition to find the specific condition
that caused the failure. For example, if we have something like:

template<
  typename Iter,
  typename = std::enable_if_t>>
void mysort(Iter first, Iter last) {}

and we call "mysort" with "std::list" iterators, we'll get a
diagnostic saying that the "Random_access_iterator" requirement
failed. If we call "mysort" with
"std::vector", we'll get a diagnostic saying
that the "Comparable<...>" requirement failed.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/PartialDiagnostic.h
cfe/trunk/include/clang/Sema/TemplateDeduction.h
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/SemaTemplate/constexpr-instantiate.cpp
cfe/trunk/test/SemaTemplate/overload-candidates.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=307196=307195=307196=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jul  5 13:20:14 
2017
@@ -3518,6 +3518,8 @@ def note_ovl_candidate_substitution_fail
 "candidate template ignored: substitution failure%0%1">;
 def note_ovl_candidate_disabled_by_enable_if : Note<
 "candidate template ignored: disabled by %0%1">;
+def note_ovl_candidate_disabled_by_requirement : Note<
+"candidate template ignored: requirement '%0' was not satisfied%1">;
 def note_ovl_candidate_has_pass_object_size_params: Note<
 "candidate address cannot be taken because parameter %0 has "
 "pass_object_size attribute">;
@@ -4431,6 +4433,9 @@ def err_typename_nested_not_found : Erro
 def err_typename_nested_not_found_enable_if : Error<
   "no type named 'type' in %0; 'enable_if' cannot be used to disable "
   "this declaration">;
+def err_typename_nested_not_found_requirement : Error<
+  "failed requirement '%0'; 'enable_if' cannot be used to disable this "
+  "declaration">;
 def err_typename_nested_not_type : Error<
 "typename specifier refers to non-type member %0 in %1">;
 def note_typename_refers_here : Note<

Modified: cfe/trunk/include/clang/Basic/PartialDiagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/PartialDiagnostic.h?rev=307196=307195=307196=diff
==
--- cfe/trunk/include/clang/Basic/PartialDiagnostic.h (original)
+++ cfe/trunk/include/clang/Basic/PartialDiagnostic.h Wed Jul  5 13:20:14 2017
@@ -329,6 +329,15 @@ public:
 
   bool hasStorage() const { return DiagStorage != nullptr; }
 
+  /// Retrieve the string argument at the given index.
+  StringRef getStringArg(unsigned I) {
+assert(DiagStorage && "No diagnostic storage?");
+assert(I < DiagStorage->NumDiagArgs && "Not enough diagnostic args");
+assert(DiagStorage->DiagArgumentsKind[I]
+ == DiagnosticsEngine::ak_std_string && "Not a string arg");
+return DiagStorage->DiagArgumentsStr[I];
+  }
+
   friend const PartialDiagnostic <<(const PartialDiagnostic ,
  unsigned I) {
 PD.AddTaggedVal(I, DiagnosticsEngine::ak_uint);

Modified: cfe/trunk/include/clang/Sema/TemplateDeduction.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/TemplateDeduction.h?rev=307196=307195=307196=diff
==
--- cfe/trunk/include/clang/Sema/TemplateDeduction.h (original)
+++ cfe/trunk/include/clang/Sema/TemplateDeduction.h Wed Jul  5 13:20:14 2017
@@ -88,6 +88,12 @@ public:
 HasSFINAEDiagnostic = false;
   }
 
+  /// Peek at the SFINAE diagnostic.
+  const PartialDiagnosticAt () const {
+assert(HasSFINAEDiagnostic);
+return SuppressedDiagnostics.front();
+  }
+
   /// \brief Provide a new template argument list that contains the
   /// results of template argument deduction.
   void reset(TemplateArgumentList *NewDeduced) {

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=307196=307195=307196=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Wed Jul  5 13:20:14 2017
@@ -9830,6 +9830,15 @@ static void DiagnoseBadDeduction(Sema 
   return;
 }

[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In https://reviews.llvm.org/D34158#797968, @Hahnfeld wrote:

> In https://reviews.llvm.org/D34158#797170, @mibintc wrote:
>
> > The other test that fails is my own new test! It fails because I don't know 
> > how to set it up so the test thinks it has a gcc toolchain with version > 
> > 4.8. I tried using gcc-toolchain set to various other Linux toolchains that 
> > i see in the test/Driver/Inputs - none of them cause the gcc version to be 
> > in the range. I also tried using -ccc-installation=Inputs/ which I see 
> > being used for gcc version parsing. How can I set up the test so that the 
> > GCCInstallation has a Version >= 4.8?  I test the new functionality from 
> > the console on Linux and can confirm it's working.
>
>
> You could try adding a completely new directory in `Inputs/`. Add the 
> directory `usr/lib/gcc/x86_64-unknown-linux/4.9.0` underneath which 
> //should// be recognized as the version. Maybe you have to add some more 
> files, I'm not really familiar with the detection mechanism...


Thanks. I did that by copying the directory from fake_install_tree into a new 
install directory named stdc-predef.  However, this modification didn't result 
in altering the macro values for gnu-major and gnu-minor.  On the other hand, 
it is necessary to have a gcc installation in the --sysroot directory.  The 
test didn't pass if the gcc installation wasn't present in the Inputs 
directory. I used the method you suggested and modified the test to use -### 
and check for the presence or absence of -include stdc-predef.h. Now these new 
tests are passing. Thank you very much for the suggestion.


Repository:
  rL LLVM

https://reviews.llvm.org/D34158



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

[PATCH] D29658: [OpenMP] Customize CUDA-based tool chain selection

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

gtbercea added a comment.

In https://reviews.llvm.org/D29658#732520, @ABataev wrote:

> Tests?


Can't write any meaningful tests. This will be tested by all future patches 
that perform offloading using OpenMP.


https://reviews.llvm.org/D29658



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

[PATCH] D34928: Add docs for -foptimization-record-file=

[Brian Gesiak via Phabricator via cfe-commits]

modocache added a comment.

Thanks for the review, Hal! :)


https://reviews.llvm.org/D34928



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

[PATCH] D29658: [OpenMP] Customize CUDA-based tool chain selection

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

gtbercea added inline comments.



Comment at: lib/Driver/Driver.cpp:564
+  auto  =
+  ToolChains[TT.str() + "/" + HostTC->getTriple().normalize()];
+  if (!CudaTC)

Hahnfeld wrote:
> The code above uses `HostTriple.str()`, maybe better align to this?
HostTriple is equivalent with HostTC->getTriple(). HostTC->getTriple() is only 
used once so no need for a local variable unless you want me to refactor the 
CUDA code above and take out HostTriple from within the if statement and then 
re0use HostTriple that way. I'd rather not make changes to the CUDA toolchain 
above.


https://reviews.llvm.org/D29658



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

r307193 - Add docs for -foptimization-record-file=

[Brian Gesiak via cfe-commits]

Author: modocache
Date: Wed Jul  5 12:55:51 2017
New Revision: 307193

URL: http://llvm.org/viewvc/llvm-project?rev=307193=rev
Log:
Add docs for -foptimization-record-file=

Summary: The Clang option was previously not included in the User's Manual.

Reviewers: anemet, davidxl, hfinkel

Reviewed By: hfinkel

Subscribers: cfe-commits

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

Modified:
cfe/trunk/docs/UsersManual.rst

Modified: cfe/trunk/docs/UsersManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=307193=307192=307193=diff
==
--- cfe/trunk/docs/UsersManual.rst (original)
+++ cfe/trunk/docs/UsersManual.rst Wed Jul  5 12:55:51 2017
@@ -332,6 +332,19 @@ output format of the diagnostics that it
using a structured YAML format, users can parse or sort the remarks in a
convenient way.
 
+.. _opt_foptimization-record-file:
+
+**-foptimization-record-file**
+   Control the file to which optimization reports are written.
+
+   When optimization reports are being output (see
+   :ref:`-fsave-optimization-record `), this
+   option controls the file to which those reports are written.
+
+   If this option is not used, optimization records are output to a file named
+   after the primary file being compiled. If that's "foo.c", for example,
+   optimization records are output to "foo.opt.yaml".
+
 .. _opt_fdiagnostics-show-hotness:
 
 **-f[no-]diagnostics-show-hotness**


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

[PATCH] D29658: [OpenMP] Customize CUDA-based tool chain selection

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

gtbercea updated this revision to Diff 105320.
gtbercea edited the summary of this revision.
gtbercea added a comment.

Rebase on latest master.


https://reviews.llvm.org/D29658

Files:
  lib/Driver/Driver.cpp


Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -572,8 +572,22 @@
   if (TT.getArch() == llvm::Triple::UnknownArch)
 Diag(clang::diag::err_drv_invalid_omp_target) << Val;
   else {
-const ToolChain  = getToolChain(C.getInputArgs(), TT);
-C.addOffloadDeviceToolChain(, Action::OFK_OpenMP);
+const ToolChain *TC;
+// CUDA toolchains have to be selected differently. They pair host
+// and device in their implementation.
+if (TT.isNVPTX()) {
+  const ToolChain *HostTC =
+  C.getSingleOffloadToolChain();
+  assert(HostTC && "Host toolchain should be always defined.");
+  auto  =
+  ToolChains[TT.str() + "/" + HostTC->getTriple().normalize()];
+  if (!CudaTC)
+CudaTC = llvm::make_unique(
+*this, TT, *HostTC, C.getInputArgs());
+  TC = CudaTC.get();
+} else
+  TC = (C.getInputArgs(), TT);
+C.addOffloadDeviceToolChain(TC, Action::OFK_OpenMP);
   }
 }
   } else


Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -572,8 +572,22 @@
   if (TT.getArch() == llvm::Triple::UnknownArch)
 Diag(clang::diag::err_drv_invalid_omp_target) << Val;
   else {
-const ToolChain  = getToolChain(C.getInputArgs(), TT);
-C.addOffloadDeviceToolChain(, Action::OFK_OpenMP);
+const ToolChain *TC;
+// CUDA toolchains have to be selected differently. They pair host
+// and device in their implementation.
+if (TT.isNVPTX()) {
+  const ToolChain *HostTC =
+  C.getSingleOffloadToolChain();
+  assert(HostTC && "Host toolchain should be always defined.");
+  auto  =
+  ToolChains[TT.str() + "/" + HostTC->getTriple().normalize()];
+  if (!CudaTC)
+CudaTC = llvm::make_unique(
+*this, TT, *HostTC, C.getInputArgs());
+  TC = CudaTC.get();
+} else
+  TC = (C.getInputArgs(), TT);
+C.addOffloadDeviceToolChain(TC, Action::OFK_OpenMP);
   }
 }
   } else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D31022: Implement P0298R3: `std::byte`

[David Majnemer via cfe-commits]

FWIW, doesn't this just need a may_alias attribute on the definition?
On Wed, Jul 5, 2017 at 2:39 PM James Y Knight via Phabricator via
cfe-commits  wrote:

> jyknight added a comment.
>
> Ping. I don't think this got resolved, and I really wouldn't like to see
> released in this state...can you either revert this from the library, or
> implement the compiler support, before the 5.0 release branch?
>
> In https://reviews.llvm.org/D31022#716702, @jyknight wrote:
>
> > I believe this needs compiler support, too, in order to treat
> >
> >   namespace std { enum class byte : unsigned char {}; }
> >
> > as directly having tbaa type "omnipotent char" instead of a subtype.
> >
> > That is, given:
> >
> >   void foo(char* x, int *y) {
> > x[1] = char(y[0] & 0xff);
> > x[0] = char((y[0] & 0xff00) >> 8);
> >   }
> >
> > the compiler assumes that x and y might alias each-other, and thus must
> have two loads of y[0]. If you replace "char" with "std::byte", the same
> should happen, but as of now does not.
>
>
>
>
>
> https://reviews.llvm.org/D31022
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33109: Enhance synchscope representation (clang)

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini accepted this revision.
mehdi_amini added a comment.

Make sure your commit message correctly reference the LLVM change.




Comment at: test/CodeGen/ms-barriers-intrinsics.c:39
 #endif
-

Don't remove: file should end with a new line I believe.


https://reviews.llvm.org/D33109



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

[PATCH] D33109: Enhance synchscope representation (clang)

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl accepted this revision.
yaxunl added a comment.

LGTM. Thanks!


https://reviews.llvm.org/D33109



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

[PATCH] D34031: [OpenCL] Diagnose some reserved types

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Sorry for the delay.

The spec says:

The data type names described in the table below are reserved and cannot be 
used by applications as user-defined type names.

However, it does not forbid using the reserved types as builtin types if the 
compiler supports it.

I would suggest to emit warning instead of error.




Comment at: lib/Sema/SemaType.cpp:1366
 }
+else if (S.getLangOpts().OpenCL) {
+  // OpenCL v2.0 s6.1.4: 'long long' is a reserved data type.

Please put `else` on the same line as `}` to be consistent with the rest of the 
code. same as below.


https://reviews.llvm.org/D34031



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

[PATCH] D34535: [libcxx] [test] Fix MSVC warning C4067 "unexpected tokens following preprocessor directive - expected a newline".

[Stephan T. Lavavej via Phabricator via cfe-commits]

STL_MSFT added a comment.

Ping for this one - there's no functional change here, but since I don't have 
any glibc coverage, I wanted to get a review.


https://reviews.llvm.org/D34535



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

[PATCH] D34536: [libcxx] [test] Fix Clang -Wunused-local-typedef warnings.

[Stephan T. Lavavej via Phabricator via cfe-commits]

STL_MSFT added a comment.

Yeah, I just wanted to check that you didn't mean to actually use these 
typedefs for something. Thanks!


https://reviews.llvm.org/D34536



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

[PATCH] D16403: Add scope information to CFG

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Oh, I see now. I was wrongly thinking that these patches do the same thing and 
we're duplicating the work. Thank you very much for your explanation, Devin.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D34536: [libcxx] [test] Fix Clang -Wunused-local-typedef warnings.

[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. @STL_MSFT This seems like you could commit it without review. You're a 
professional STL developer after all, if you can't get this right may God have 
mercy on your users.


https://reviews.llvm.org/D34536



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

[PATCH] D34992: Emit static constexpr member as available_externally definition

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added inline comments.



Comment at: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp:12
+// CHECK: @_ZN3Foo4BAR2E = external constant i32,
+  static const int BAR2 = 43; 
+};

Note: I'm not sure if the standard allows us to assume that we can fold the 
value for BAR2 without seeing the definition here? 
(so we could be even more aggressive)



Comment at: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp:15
+// CHECK: @_ZL4BAR3 = available_externally constant i32 44,
+static constexpr int BAR3 = 44;
+

Looks like I have a bug here, this should be an internal.


https://reviews.llvm.org/D34992



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

[PATCH] D34536: [libcxx] [test] Fix Clang -Wunused-local-typedef warnings.

[Stephan T. Lavavej via Phabricator via cfe-commits]

STL_MSFT added a comment.

Ping?


https://reviews.llvm.org/D34536



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

[PATCH] D16403: Add scope information to CFG

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added a comment.

> @dcoughlin As a reviewer of both patches - could you tell us what's the 
> difference between them? And how are we going to resolve this issue?

These two patches are emitting markers in the CFG for different things.

Here is how I would characterize the difference between the two patches.

- Despite its name, https://reviews.llvm.org/D15031, is really emitting markers 
for when the lifetime of a C++ object in an automatic variable ends.  For C++ 
objects with non-trivial destructors, this point is when the destructor is 
called. At this point the storage for the variable still exists, but what you 
can do with that storage is very restricted by the language because its 
contents have been destroyed. Note that even with the contents of the object 
have been destroyed, it is still meaningful to, say, take the address of the 
variable and construct a new object into it with a placement new. In other 
words, the same variable can have different objects, with different lifetimes, 
in it at different program points.

- In contrast, the purpose of this patch (https://reviews.llvm.org/D16403) is 
to add markers for when the storage duration for the variable begins and ends 
(this is, when the storage exists). Once the storage duration has ended, you 
can't placement new into the variables address, because another variable may 
already be at that address.

I don't think there is an "issue" to resolve here.  We should make sure the two 
patches play nicely with each other, though. In particular, we should make sure 
that the markers for when lifetime ends and when storage duration ends are 
ordered correctly.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D31022: Implement P0298R3: `std::byte`

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Ping. I don't think this got resolved, and I really wouldn't like to see  
released in this state...can you either revert this from the library, or 
implement the compiler support, before the 5.0 release branch?

In https://reviews.llvm.org/D31022#716702, @jyknight wrote:

> I believe this needs compiler support, too, in order to treat
>
>   namespace std { enum class byte : unsigned char {}; }
>
> as directly having tbaa type "omnipotent char" instead of a subtype.
>
> That is, given:
>
>   void foo(char* x, int *y) {
> x[1] = char(y[0] & 0xff);
> x[0] = char((y[0] & 0xff00) >> 8);
>   }
>
> the compiler assumes that x and y might alias each-other, and thus must have 
> two loads of y[0]. If you replace "char" with "std::byte", the same should 
> happen, but as of now does not.





https://reviews.llvm.org/D31022



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

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

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

gtbercea added inline comments.



Comment at: test/Driver/openmp-offload.c:607
+
+// CHK-FOPENMP-EQ-TARGET: clang{{.*}} argument unused during compilation: 
'-Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8'
+

hfinkel wrote:
> I don't see why you'd check that the arguments are unused. They should be 
> used. One exception might be that you might want to force -Xopenmp-target=foo 
> to be unused if foo is not a currently-targeted offloading triple. There 
> could be a separate test case for that.
> 
> Otherwise, I think you should be able to check the relevant backend commands, 
> no? (something like where CHK-COMMANDS is used above in this file).
Only the CUDA toolchain currently contains code which considers the value of 
the -Xopenmp-target flag. The CUDA toolchain is not capable of offloading until 
the next patch lands so any test for how the flag propagates to the CUDA 
toolchain will have to wait.

Passing a flag to some other toolchain again doesn't work because the other 
toolchains have not been instructed to look at this flag so they won't contain 
the passed flag in their respective command lines.

For a lack of a better test, what I wanted to show is that the usage of this 
flag doesn't throw an error such as unknown flag and is correctly recognized: 
"-Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8". 





https://reviews.llvm.org/D34784



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

Re: [PATCH] D34972: [CodeGen] Propagate dllexport to thunks

[Shoaib Meenai via cfe-commits]

I had initially also thought that thunks wouldn't need to be exported, since
you should always have one of two scenarios:

1. The class doesn't have a key function, in which case both its vtable and
   its thunks will just be emitted wherever they're needed.
2. The class has a key function, in which case both its vtable and its thunks
   will be present in the library with the key function, and the thunks are
   only referenced from the vtable.

The situation I ran into which required thunks to be exported was something
like the following (this is from memory, so some details might be a bit off):

* B virtually inherits from A and has a key function, so its vtable and thunks
  are localized to the library with the key function.
* C inherits from B and doesn't have a key function.
* The constructor vtable for B-in-C is emitted in some other library (due to
  C's lack of a key function), and it tries to reference B's thunks, causing
  link failures.

I can add this scenario to the commit message if it makes the motivation
clearer.

It should be noted that the thunks were exported before r298330, which only
changed dllexport behavior, so dllimport behavior for thunks should still be
the same as it was before. I can look into what that behavior is and make it
explicit in this diff as well.

From: David Majnemer 
Date: Wednesday, July 5, 2017 at 11:15 AM
To: Shoaib Meenai , Shoaib Meenai via Phabricator 
, "compn...@compnerd.org" , 
"reviews+d34972+public+8a22767368a5b...@reviews.llvm.org" 
, "r...@google.com" 

Cc: "cfe-commits@lists.llvm.org" , 
"ztur...@google.com" 
Subject: Re: [PATCH] D34972: [CodeGen] Propagate dllexport to thunks

If the thinks are not imported, why would it make sense to export them?

Maybe devirtualization could trigger this?

On Wed, Jul 5, 2017 at 1:17 PM Shoaib Meenai  wrote:
The thunks are (as far as I know, at least) only referenced from vtables, so
there's no opportunity to perform indirect calls via the IAT for them. The
vtable would just end up referencing the linker-generated import thunk
instead.

I can play around with marking the thunks as dllimport if the function being
thunked is dllimport, but I was curious if you had any specific scenarios in
mind where you thought it would make a difference.

From: cfe-commits  on behalf of David 
Majnemer via cfe-commits 
Reply-To: David Majnemer 
Date: Tuesday, July 4, 2017 at 8:42 AM
To: Shoaib Meenai via Phabricator , 
"compn...@compnerd.org" , 
"reviews+d34972+public+8a22767368a5b...@reviews.llvm.org" 
, "r...@google.com" 

Cc: "ztur...@google.com" , "cfe-commits@lists.llvm.org" 

Subject: Re: [PATCH] D34972: [CodeGen] Propagate dllexport to thunks

What about the import side?

On Mon, Jul 3, 2017 at 10:37 PM Shoaib Meenai via Phabricator via cfe-commits 
 wrote:
smeenai created this revision.

Under Windows Itanium, we need to export virtual and non-virtual thunks
if the functions being thunked are exported. These thunks would
previously inherit their dllexport attribute from the declaration, but
r298330 changed declarations to not have dllexport attributes. We
therefore need to add the dllexport attribute to the definition
ourselves now. This is consistent with MinGW GCC's behavior.

This redoes r306770 but limits the logic to Itanium. MicrosoftCXXABI's
setThunkLinkage ensures that thunks aren't exported under that ABI, so
I'm handling this in ItaniumCXXABI's setThunkLinkage for symmetry.


https://reviews.llvm.org/D34972

Files:
  lib/CodeGen/ItaniumCXXABI.cpp
  test/CodeGenCXX/dllexport-vtable-thunks.cpp


Index: test/CodeGenCXX/dllexport-vtable-thunks.cpp
===
--- /dev/null
+++ test/CodeGenCXX/dllexport-vtable-thunks.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -fdeclspec -emit-llvm -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-windows-itanium -fdeclspec -emit-llvm -o - 
%s | FileCheck %s
+
+struct __declspec(dllexport) A {
+  virtual void m();
+};
+struct __declspec(dllexport) B {
+  virtual void m();
+};
+struct __declspec(dllexport) C : A, B {
+  virtual void m();
+};
+void C::m() {}
+// CHECK: define dllexport void @_ZThn8_N1C1mEv
+
+struct Base {
+  virtual void m();
+};
+struct __declspec(dllexport) Derived : virtual Base {
+  virtual void m();
+};
+void Derived::m() {}
+// CHECK: define dllexport void @_ZTv0_n24_N7Derived1mEv
Index: lib/CodeGen/ItaniumCXXABI.cpp

[PATCH] D32411: [libcxx] Provide #include_next alternative for MSVC

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

@smeenai wrote:

> This is kinda ugly, but I can't think of a better way to do it. I'm fine with 
> this, but given that it's a pretty invasive change, I'm not comfortable 
> accepting. You may wanna ping @EricWF and @mclow.lists directly.

I'll be meeting with the MS compiler engineers next week; I'll see when/if they 
plan on implemementing `include_next`.


https://reviews.llvm.org/D32411



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

[PATCH] D33842: CodeGen: Fix address space of global variable

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 105310.
yaxunl marked 6 inline comments as done.
yaxunl retitled this revision from "[AMDGPU] Fix address space of global 
variable" to "CodeGen: Fix address space of global variable".
yaxunl edited the summary of this revision.
yaxunl added a comment.

Revised by John's comments.


https://reviews.llvm.org/D33842

Files:
  include/clang/Basic/TargetInfo.h
  lib/Basic/Targets.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/TargetInfo.cpp
  lib/CodeGen/TargetInfo.h
  test/CodeGen/address-space.c
  test/CodeGen/default-address-space.c
  test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp

Index: test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
===
--- test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
+++ test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=c++11 -triple x86_64-none-linux-gnu -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-none-linux-gnu -emit-llvm -o - %s | FileCheck -check-prefixes=X86,CHECK %s
+// RUN: %clang_cc1 -std=c++11 -triple amdgcn-amd-amdhsa-amdgiz -DNO_TLS -emit-llvm -o - %s | FileCheck -check-prefixes=AMD,CHECK %s
 
 namespace std {
   typedef decltype(sizeof(int)) size_t;
@@ -46,62 +47,82 @@
   wantslist1(std::initializer_list);
   ~wantslist1();
 };
-
-// CHECK: @_ZGR15globalInitList1_ = internal constant [3 x i32] [i32 1, i32 2, i32 3]
-// CHECK: @globalInitList1 = global %{{[^ ]+}} { i32* getelementptr inbounds ([3 x i32], [3 x i32]* @_ZGR15globalInitList1_, i32 0, i32 0), i{{32|64}} 3 }
+// X86: @_ZGR15globalInitList1_ = internal constant [3 x i32] [i32 1, i32 2, i32 3]
+// X86: @globalInitList1 = global %{{[^ ]+}} { i32* getelementptr inbounds ([3 x i32], [3 x i32]* @_ZGR15globalInitList1_, i32 0, i32 0), i{{32|64}} 3 }
+// AMD: @_ZGR15globalInitList1_ = internal addrspace(1) constant [3 x i32] [i32 1, i32 2, i32 3]
+// AMD: @globalInitList1 = addrspace(1) global %{{[^ ]+}} { i32* addrspacecast (i32 addrspace(1)* getelementptr inbounds ([3 x i32], [3 x i32] addrspace(1)* @_ZGR15globalInitList1_, i32 0, i32 0) to i32*), i{{32|64}} 3 }
 std::initializer_list globalInitList1 = {1, 2, 3};
 
+#ifndef NO_TLS
 namespace thread_local_global_array {
-  // FIXME: We should be able to constant-evaluate this even though the
-  // initializer is not a constant expression (pointers to thread_local
-  // objects aren't really a problem).
-  //
-  // CHECK: @_ZN25thread_local_global_array1xE = thread_local global
-  // CHECK: @_ZGRN25thread_local_global_array1xE_ = internal thread_local constant [4 x i32] [i32 1, i32 2, i32 3, i32 4]
-  std::initializer_list thread_local x = { 1, 2, 3, 4 };
+// FIXME: We should be able to constant-evaluate this even though the
+// initializer is not a constant expression (pointers to thread_local
+// objects aren't really a problem).
+//
+// X86: @_ZN25thread_local_global_array1xE = thread_local global
+// X86: @_ZGRN25thread_local_global_array1xE_ = internal thread_local constant [4 x i32] [i32 1, i32 2, i32 3, i32 4]
+std::initializer_list thread_local x = {1, 2, 3, 4};
 }
-
-// CHECK: @globalInitList2 = global %{{[^ ]+}} zeroinitializer
-// CHECK: @_ZGR15globalInitList2_ = internal global [2 x %[[WITHARG:[^ ]*]]] zeroinitializer
-
-// CHECK: @_ZN15partly_constant1kE = global i32 0, align 4
-// CHECK: @_ZN15partly_constant2ilE = global {{.*}} null, align 8
-// CHECK: @[[PARTLY_CONSTANT_OUTER:_ZGRN15partly_constant2ilE.*]] = internal global {{.*}} zeroinitializer, align 8
-// CHECK: @[[PARTLY_CONSTANT_INNER:_ZGRN15partly_constant2ilE.*]] = internal global [3 x {{.*}}] zeroinitializer, align 8
-// CHECK: @[[PARTLY_CONSTANT_FIRST:_ZGRN15partly_constant2ilE.*]] = internal constant [3 x i32] [i32 1, i32 2, i32 3], align 4
-// CHECK: @[[PARTLY_CONSTANT_SECOND:_ZGRN15partly_constant2ilE.*]] = internal global [2 x i32] zeroinitializer, align 4
-// CHECK: @[[PARTLY_CONSTANT_THIRD:_ZGRN15partly_constant2ilE.*]] = internal constant [4 x i32] [i32 5, i32 6, i32 7, i32 8], align 4
-
-// CHECK: @[[REFTMP1:.*]] = private constant [2 x i32] [i32 42, i32 43], align 4
-// CHECK: @[[REFTMP2:.*]] = private constant [3 x %{{.*}}] [%{{.*}} { i32 1 }, %{{.*}} { i32 2 }, %{{.*}} { i32 3 }], align 4
+#endif
+
+// X86: @globalInitList2 = global %{{[^ ]+}} zeroinitializer
+// X86: @_ZGR15globalInitList2_ = internal global [2 x %[[WITHARG:[^ ]*]]] zeroinitializer
+// AMD: @globalInitList2 = addrspace(1) global %{{[^ ]+}} zeroinitializer
+// AMD: @_ZGR15globalInitList2_ = internal addrspace(1) global [2 x %[[WITHARG:[^ ]*]]] zeroinitializer
+
+// X86: @_ZN15partly_constant1kE = global i32 0, align 4
+// X86: @_ZN15partly_constant2ilE = global {{.*}} null, align 8
+// X86: @[[PARTLY_CONSTANT_OUTER:_ZGRN15partly_constant2ilE.*]] = internal global 

Re: [PATCH] D34972: [CodeGen] Propagate dllexport to thunks

[David Majnemer via cfe-commits]

If the thinks are not imported, why would it make sense to export them?

Maybe devirtualization could trigger this?

On Wed, Jul 5, 2017 at 1:17 PM Shoaib Meenai  wrote:

> The thunks are (as far as I know, at least) only referenced from vtables,
> so
> there's no opportunity to perform indirect calls via the IAT for them. The
> vtable would just end up referencing the linker-generated import thunk
> instead.
>
> I can play around with marking the thunks as dllimport if the function
> being
> thunked is dllimport, but I was curious if you had any specific scenarios
> in
> mind where you thought it would make a difference.
>
> From: cfe-commits  on behalf of David
> Majnemer via cfe-commits 
> Reply-To: David Majnemer 
> Date: Tuesday, July 4, 2017 at 8:42 AM
> To: Shoaib Meenai via Phabricator , "
> compn...@compnerd.org" , "
> reviews+d34972+public+8a22767368a5b...@reviews.llvm.org" <
> reviews+d34972+public+8a22767368a5b...@reviews.llvm.org>, "r...@google.com"
> 
> Cc: "ztur...@google.com" , "cfe-commits@lists.llvm.org"
> 
> Subject: Re: [PATCH] D34972: [CodeGen] Propagate dllexport to thunks
>
> What about the import side?
>
> On Mon, Jul 3, 2017 at 10:37 PM Shoaib Meenai via Phabricator via
> cfe-commits  wrote:
> smeenai created this revision.
>
> Under Windows Itanium, we need to export virtual and non-virtual thunks
> if the functions being thunked are exported. These thunks would
> previously inherit their dllexport attribute from the declaration, but
> r298330 changed declarations to not have dllexport attributes. We
> therefore need to add the dllexport attribute to the definition
> ourselves now. This is consistent with MinGW GCC's behavior.
>
> This redoes r306770 but limits the logic to Itanium. MicrosoftCXXABI's
> setThunkLinkage ensures that thunks aren't exported under that ABI, so
> I'm handling this in ItaniumCXXABI's setThunkLinkage for symmetry.
>
>
> https://reviews.llvm.org/D34972
>
> Files:
>   lib/CodeGen/ItaniumCXXABI.cpp
>   test/CodeGenCXX/dllexport-vtable-thunks.cpp
>
>
> Index: test/CodeGenCXX/dllexport-vtable-thunks.cpp
> ===
> --- /dev/null
> +++ test/CodeGenCXX/dllexport-vtable-thunks.cpp
> @@ -0,0 +1,23 @@
> +// RUN: %clang_cc1 -triple x86_64-windows-gnu -fdeclspec -emit-llvm -o -
> %s | FileCheck %s
> +// RUN: %clang_cc1 -triple x86_64-windows-itanium -fdeclspec -emit-llvm
> -o - %s | FileCheck %s
> +
> +struct __declspec(dllexport) A {
> +  virtual void m();
> +};
> +struct __declspec(dllexport) B {
> +  virtual void m();
> +};
> +struct __declspec(dllexport) C : A, B {
> +  virtual void m();
> +};
> +void C::m() {}
> +// CHECK: define dllexport void @_ZThn8_N1C1mEv
> +
> +struct Base {
> +  virtual void m();
> +};
> +struct __declspec(dllexport) Derived : virtual Base {
> +  virtual void m();
> +};
> +void Derived::m() {}
> +// CHECK: define dllexport void @_ZTv0_n24_N7Derived1mEv
> Index: lib/CodeGen/ItaniumCXXABI.cpp
> ===
> --- lib/CodeGen/ItaniumCXXABI.cpp
> +++ lib/CodeGen/ItaniumCXXABI.cpp
> @@ -284,6 +284,11 @@
>  // linkage together with vtables when needed.
>  if (ForVTable && !Thunk->hasLocalLinkage())
>Thunk->setLinkage(llvm::GlobalValue::AvailableExternallyLinkage);
> +
> +// Propagate dllexport storage.
> +const CXXMethodDecl *MD = cast(GD.getDecl());
> +if (MD->hasAttr())
> +  Thunk->setDLLStorageClass(llvm::GlobalValue::DLLExportStorageClass);
>}
>
>llvm::Value *performThisAdjustment(CodeGenFunction , Address This,
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33842: [AMDGPU] Fix address space of global variable

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 12 inline comments as done.
yaxunl added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:969
+  /// LangAS::Default.
+  virtual unsigned getGlobalAddressSpace() const { return LangAS::Default; }
+

rjmccall wrote:
> I'm not sure this really needs to exist.
Removed.



Comment at: lib/CodeGen/CGBlocks.cpp:1144
+ ? CGM.getNSConcreteGlobalBlock()
+ : CGM.getNullPointer(CGM.VoidPtrPtrTy,
+  QualType(CGM.getContext().VoidPtrTy)));

rjmccall wrote:
> You used VoidPtrTy above.
They are different fields with different types.



Comment at: lib/CodeGen/CodeGenModule.cpp:2549
+   AddrSpace == LangAS::opencl_local ||
+   AddrSpace >= LangAS::FirstTargetAddressSpace);
+return AddrSpace;

rjmccall wrote:
> This does not seem like a very useful assertion.
This checks global variable has valid address space. If a global variable 
having private or generic address space is passed to backend, it may cause 
issues difficult to debug. This assertion helps to detect the issue early.



Comment at: lib/CodeGen/CodeGenModule.cpp:2554
+  if (K == LanguageExpected)
+return D ? D->getType().getAddressSpace() : LangAS::Default;
+

rjmccall wrote:
> I would expect this to be the general rule for all language modes.  Why is 
> OpenCL an exception?
> 
> It looks to me like GetGlobalVarAddressSpace is only ever called with a 
> possibly-null D by GetOrCreateLLVMGlobal, which is only called with a null D 
> by CreateRuntimeVariable.  I expect that every call to CreateRuntimeVariable 
> will probably need to be audited if it's going to suddenly start returning 
> pointers that aren't in the default address space, and we'll probably end up 
> needing some very different behavior at that point.  So for now, let's just 
> move the LanguageExpected case out to the one call site that uses it.
> 
> I do like the simplification of just taking the declaration instead of taking 
> that and an address space.
For OpenCL, TargetFavored and LanguageExpected are the same, i.e. global. For 
C++, TargetFavored is globa, but LanguageExpected is Default.

Done.



Comment at: lib/CodeGen/CodeGenModule.cpp:2578
+  }
+  return getTarget().getGlobalAddressSpace();
 }

rjmccall wrote:
> Okay, I think I see what you're trying to do here: when you compile normal 
> (i.e. non-OpenCL) code for your target, you want to implicitly change where 
> global variables are allocated by default, then translate the pointer into 
> LangAS::Default.  It's essentially the global-variable equivalent of what 
> we're already doing with stack pointers.
> 
> The right way to do this is to make a hook on the TargetCodeGenInfo like 
> getASTAllocaAddressSpace() which allows the target to override the address 
> space for a global on a case-by-case basis.  You can then do whatever 
> target-specific logic you need there, and you won't need to introduce 
> getGlobalAddressSpace() on TargetInfo.  The default implementation, of 
> course, will just return D->getType().getAddressSpace().
Done.

I am wondering if I should just move the whole function to 
TargetCodeGenInfo::getGlobalVarAddressSpace, which seems cleaner.



Comment at: lib/CodeGen/TargetInfo.cpp:7418
+  if (M.getLangOpts().OpenCL)
+appendOpenCLVersionMD(M);
 }

rjmccall wrote:
> You have a couple things in this patch that just touch your target and are 
> basically unrelated to the rest of your logic.  Please split those into a 
> separate patch.
Done.


https://reviews.llvm.org/D33842



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

Re: [PATCH] D34574: [Sema] Disable c++17 aligned new and delete operators if not implemented in the deployment target's c++ standard library

[Akira Hatanaka via cfe-commits]

No, it was not deliberate. I'll take a look at the failing tests today.

> On Jul 5, 2017, at 10:02 AM, Marshall Clow via Phabricator via cfe-commits 
>  wrote:
> 
> mclow.lists added a comment.
> 
> This commit breaks all the libc++ aligned new/delete tests on Mac OS. Was 
> that deliberate?
> 
> Failing Tests (8):
> 
>  libc++ :: 
> std/language.support/support.dynamic/new.delete/new.delete.array/delete_align_val_t_replace.pass.cpp
>  libc++ :: 
> std/language.support/support.dynamic/new.delete/new.delete.array/new_align_val_t.pass.cpp
>  libc++ :: 
> std/language.support/support.dynamic/new.delete/new.delete.array/new_align_val_t_nothrow.pass.cpp
>  libc++ :: 
> std/language.support/support.dynamic/new.delete/new.delete.array/new_align_val_t_nothrow_replace.pass.cpp
>  libc++ :: 
> std/language.support/support.dynamic/new.delete/new.delete.single/delete_align_val_t_replace.pass.cpp
>  libc++ :: 
> std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t.pass.cpp
>  libc++ :: 
> std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t_nothrow.pass.cpp
>  libc++ :: 
> std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t_nothrow_replace.pass.cpp
> 
> 
> Repository:
>  rL LLVM
> 
> https://reviews.llvm.org/D34574
> 
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[PATCH] D15031: CFG: Add CFGElement for automatic variables that leave the scope

[Devin Coughlin via Phabricator via cfe-commits]

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

Sorry for the long delay! This looks good to me. Do you have commit access, or 
do you need someone to commit it for you?

> Regarding " I think it would also be good to (eventually) add CFGElements 
> marking when the storage duration for underlying storage ends.":
> From what I understand, this only differs from the end of lifetime in case of 
> objects with non-trivial destructors, where the lifetime ends before
> the destructor is called and the storage duration ends afterwards.
> In which case is this difference important to the static analyzer? Accessing 
> an object after its lifetime ended is already UB, so the static analyzer 
> could warn on this,
> even before the storage duration for underlying storage ends.

There a couple of cases where the difference between storage duration and 
lifetime could be important to the analyzer.  For example, you can end the 
lifetime of an object prematurely by calling its destructor explicitly. Then, 
you can later create a new object in its place with new -- but only if the 
storage is still around. So

F f;
f.~F();
  // lifetime of object in 'f' ends
new () F;
  // lifetime of new object in 'f' begins

is fine, but

  F *p;
  {
F f;
f.~F();
p = 
  }
  new (p) F;

is not.


https://reviews.llvm.org/D15031



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

[PATCH] D35020: [Modules] Add ability to specify module name to module file mapping

[Boris Kolpackov via Phabricator via cfe-commits]

boris created this revision.

Extend the -fmodule-file option to support the [=] value format.
If the name is omitted, then the old semantics is preserved (the module file
is loaded whether needed or not). If the name is specified, then the mapping
is treated as just another prebuilt module search mechanism, similar to
-fprebuilt-module-path, and the module file is only loaded if actually
used (e.g., via import). With one exception: this mapping also overrides
module file references embedded in other modules (which can be useful if
module files are moved/renamed as often happens during remote compilation).

This override semantics requires some extra work: we now store the module
name in addition to the file name in the serialized AST representation as
well as keep track of already loaded prebuilt modules by-name in addition
to by-file.

Patch by Boris Kolpackov

---

Additional notes:

1. Based on this mailing list discussion:

  http://lists.llvm.org/pipermail/cfe-dev/2017-June/054431.html

2. The need to change the serialized AST representation was a bit more than 
what I hoped for my first patch but based on this FIXME comment I recon this is 
moving in right direction:

  ASTReader::ReadModuleOffsetMap():

  // FIXME: Looking up dependency modules by filename is horrible.

  So now, at least for prebuilt modules, we look up by the module name.

3. Overloading -fmodule-file for this admittedly fairly different semantics 
might look like a bad idea and source of some unnecessary complexity. The 
problem with a separate option approach is the difficulty of finding a decent 
name that is not already used (e.g., -fmodule-map is out because of 
-fmodule-map-file; more details in the mailing list thread). But I am still 
open to changing this to a separate option if there are strong feelings (and 
good name suggestions ;-)).

4. I plan to implement a companion option that will read this mapping from a 
file. I will submit it as a separate patch once the general validity of the 
approach is confirmed.


https://reviews.llvm.org/D35020

Files:
  docs/ClangCommandLineReference.rst
  docs/Modules.rst
  include/clang/Driver/Options.td
  include/clang/Lex/HeaderSearch.h
  include/clang/Lex/HeaderSearchOptions.h
  include/clang/Serialization/ASTReader.h
  include/clang/Serialization/ModuleManager.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInstance.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  lib/Lex/HeaderSearch.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/GlobalModuleIndex.cpp
  lib/Serialization/ModuleManager.cpp
  test/CXX/modules-ts/basic/basic.search/module-import.cpp

Index: test/CXX/modules-ts/basic/basic.search/module-import.cpp
===
--- /dev/null
+++ test/CXX/modules-ts/basic/basic.search/module-import.cpp
@@ -0,0 +1,39 @@
+// Tests for imported module search.
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: echo 'export module x; int a, b;' > %t/x.cppm
+// RUN: echo 'export module y; import x; int c;' > %t/y.cppm
+// RUN: echo 'export module z; import y; int d;' > %t/z.cppm
+//
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -emit-module-interface %t/x.cppm -o %t/x.pcm
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -emit-module-interface -fmodule-file=%t/x.pcm %t/y.cppm -o %t/y.pcm
+//
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=%t/x.pcm -verify %s \
+// RUN:-DMODULE_NAME=x
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=%t/y.pcm -verify %s \
+// RUN:-DMODULE_NAME=y
+//
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=x=%t/x.pcm -verify %s \
+// RUN:-DMODULE_NAME=x
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=y=%t/y.pcm -verify %s \
+// RUN:-DMODULE_NAME=y
+//
+// RUN: mv %t/x.pcm %t/a.pcm
+//
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=x=%t/a.pcm -verify %s \
+// RUN:-DMODULE_NAME=x
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=%t/y.pcm -fmodule-file=x=%t/a.pcm -verify %s \
+// RUN:-DMODULE_NAME=y
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=y=%t/y.pcm -fmodule-file=x=%t/a.pcm -verify %s \
+// RUN:-DMODULE_NAME=y
+//
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -emit-module-interface -fmodule-file=y=%t/y.pcm -fmodule-file=x=%t/a.pcm %t/z.cppm -o %t/z.pcm
+//
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=z=%t/z.pcm -fmodule-file=y=%t/y.pcm -fmodule-file=x=%t/a.pcm -verify %s \
+// RUN:-DMODULE_NAME=z
+//
+
+import MODULE_NAME;
+
+// expected-no-diagnostics
Index: lib/Serialization/ModuleManager.cpp
===
--- lib/Serialization/ModuleManager.cpp
+++ lib/Serialization/ModuleManager.cpp
@@ -45,6 +45,14 @@
   return Known->second;
 }
 
+ModuleFile 

Re: [PATCH] D34972: [CodeGen] Propagate dllexport to thunks

[Shoaib Meenai via cfe-commits]

The thunks are (as far as I know, at least) only referenced from vtables, so
there's no opportunity to perform indirect calls via the IAT for them. The
vtable would just end up referencing the linker-generated import thunk
instead.

I can play around with marking the thunks as dllimport if the function being
thunked is dllimport, but I was curious if you had any specific scenarios in
mind where you thought it would make a difference.

From: cfe-commits  on behalf of David 
Majnemer via cfe-commits 
Reply-To: David Majnemer 
Date: Tuesday, July 4, 2017 at 8:42 AM
To: Shoaib Meenai via Phabricator , 
"compn...@compnerd.org" , 
"reviews+d34972+public+8a22767368a5b...@reviews.llvm.org" 
, "r...@google.com" 

Cc: "ztur...@google.com" , "cfe-commits@lists.llvm.org" 

Subject: Re: [PATCH] D34972: [CodeGen] Propagate dllexport to thunks

What about the import side?

On Mon, Jul 3, 2017 at 10:37 PM Shoaib Meenai via Phabricator via cfe-commits 
 wrote:
smeenai created this revision.

Under Windows Itanium, we need to export virtual and non-virtual thunks
if the functions being thunked are exported. These thunks would
previously inherit their dllexport attribute from the declaration, but
r298330 changed declarations to not have dllexport attributes. We
therefore need to add the dllexport attribute to the definition
ourselves now. This is consistent with MinGW GCC's behavior.

This redoes r306770 but limits the logic to Itanium. MicrosoftCXXABI's
setThunkLinkage ensures that thunks aren't exported under that ABI, so
I'm handling this in ItaniumCXXABI's setThunkLinkage for symmetry.


https://reviews.llvm.org/D34972

Files:
  lib/CodeGen/ItaniumCXXABI.cpp
  test/CodeGenCXX/dllexport-vtable-thunks.cpp


Index: test/CodeGenCXX/dllexport-vtable-thunks.cpp
===
--- /dev/null
+++ test/CodeGenCXX/dllexport-vtable-thunks.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -fdeclspec -emit-llvm -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-windows-itanium -fdeclspec -emit-llvm -o - 
%s | FileCheck %s
+
+struct __declspec(dllexport) A {
+  virtual void m();
+};
+struct __declspec(dllexport) B {
+  virtual void m();
+};
+struct __declspec(dllexport) C : A, B {
+  virtual void m();
+};
+void C::m() {}
+// CHECK: define dllexport void @_ZThn8_N1C1mEv
+
+struct Base {
+  virtual void m();
+};
+struct __declspec(dllexport) Derived : virtual Base {
+  virtual void m();
+};
+void Derived::m() {}
+// CHECK: define dllexport void @_ZTv0_n24_N7Derived1mEv
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -284,6 +284,11 @@
     // linkage together with vtables when needed.
     if (ForVTable && !Thunk->hasLocalLinkage())
       Thunk->setLinkage(llvm::GlobalValue::AvailableExternallyLinkage);
+
+    // Propagate dllexport storage.
+    const CXXMethodDecl *MD = cast(GD.getDecl());
+    if (MD->hasAttr())
+      Thunk->setDLLStorageClass(llvm::GlobalValue::DLLExportStorageClass);
   }

   llvm::Value *performThisAdjustment(CodeGenFunction , Address This,


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


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

[PATCH] D34936: [clangd] Add -ffreestanding on VFS tests.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D34936#799589, @mibintc wrote:

> Thanks so much!


You're welcome.


Repository:
  rL LLVM

https://reviews.llvm.org/D34936



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

r307175 - [Sema] Don't allow -Wunguarded-availability to be silenced with redecls

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Wed Jul  5 10:08:56 2017
New Revision: 307175

URL: http://llvm.org/viewvc/llvm-project?rev=307175=rev
Log:
[Sema] Don't allow -Wunguarded-availability to be silenced with redecls

Differential revision: https://reviews.llvm.org/D33816

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/DelayedDiagnostic.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/attr-availability.c
cfe/trunk/test/Sema/attr-deprecated.c
cfe/trunk/test/Sema/attr-unavailable-message.c
cfe/trunk/test/SemaCXX/attr-deprecated.cpp
cfe/trunk/test/SemaObjC/attr-availability.m
cfe/trunk/test/SemaObjC/unguarded-availability-new.m
cfe/trunk/test/SemaObjC/unguarded-availability.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=307175=307174=307175=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jul  5 10:08:56 
2017
@@ -2880,7 +2880,7 @@ def warn_partial_availability : Warning<
 def warn_partial_availability_new : Warning,
   InGroup;
 def note_partial_availability_silence : Note<
-  "explicitly redeclare %0 to silence this warning">;
+  "annotate %select{%1|anonymous %1}0 with an availability attribute to 
silence">;
 def note_unguarded_available_silence : Note<
   "enclose %0 in %select{an @available|a __builtin_available}1 check to 
silence"
   " this warning">;

Modified: cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/DelayedDiagnostic.h?rev=307175=307174=307175=diff
==
--- cfe/trunk/include/clang/Sema/DelayedDiagnostic.h (original)
+++ cfe/trunk/include/clang/Sema/DelayedDiagnostic.h Wed Jul  5 10:08:56 2017
@@ -124,7 +124,8 @@ public:
 
   static DelayedDiagnostic makeAvailability(AvailabilityResult AR,
 SourceLocation Loc,
-const NamedDecl *D,
+const NamedDecl *ReferringDecl,
+const NamedDecl *OffendingDecl,
 const ObjCInterfaceDecl 
*UnknownObjCClass,
 const ObjCPropertyDecl  
*ObjCProperty,
 StringRef Msg,
@@ -164,9 +165,13 @@ public:
 return *reinterpret_cast(AccessData);
   }
 
-  const NamedDecl *getAvailabilityDecl() const {
+  const NamedDecl *getAvailabilityReferringDecl() const {
 assert(Kind == Availability && "Not an availability diagnostic.");
-return AvailabilityData.Decl;
+return AvailabilityData.ReferringDecl;
+  }
+
+  const NamedDecl *getAvailabilityOffendingDecl() const {
+return AvailabilityData.OffendingDecl;
   }
 
   StringRef getAvailabilityMessage() const {
@@ -213,7 +218,8 @@ public:
 private:
 
   struct AD {
-const NamedDecl *Decl;
+const NamedDecl *ReferringDecl;
+const NamedDecl *OffendingDecl;
 const ObjCInterfaceDecl *UnknownObjCClass;
 const ObjCPropertyDecl  *ObjCProperty;
 const char *Message;

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=307175=307174=307175=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Jul  5 10:08:56 2017
@@ -3881,7 +3881,9 @@ public:
 
   void redelayDiagnostics(sema::DelayedDiagnosticPool );
 
-  void EmitAvailabilityWarning(AvailabilityResult AR, NamedDecl *D,
+  void EmitAvailabilityWarning(AvailabilityResult AR,
+   const NamedDecl *ReferringDecl,
+   const NamedDecl *OffendingDecl,
StringRef Message, SourceLocation Loc,
const ObjCInterfaceDecl *UnknownObjCClass,
const ObjCPropertyDecl *ObjCProperty,
@@ -10413,16 +10415,14 @@ public:
 return OriginalLexicalContext ? OriginalLexicalContext : CurContext;
   }
 
-  /// \brief The diagnostic we should emit for \c D, or \c AR_Available.
-  ///
-  /// \param D The declaration to check. Note that this may be altered to point
-  /// to another declaration that \c D gets it's availability from. i.e., we
-  /// walk the list of typedefs to find an availability attribute.
+  /// The diagnostic we should emit for \c D, and the declaration that
+  /// originated it, or \c AR_Available.

[PATCH] D33816: [Sema][ObjC] Don't allow -Wunguarded-availability to be silenced with redeclarations

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL307175: [Sema] Don't allow -Wunguarded-availability to be 
silenced with redecls (authored by epilk).

Changed prior to commit:
  https://reviews.llvm.org/D33816?vs=104437=105296#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33816

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Sema/DelayedDiagnostic.cpp
  cfe/trunk/lib/Sema/SemaDeclAttr.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/Sema/attr-availability.c
  cfe/trunk/test/Sema/attr-deprecated.c
  cfe/trunk/test/Sema/attr-unavailable-message.c
  cfe/trunk/test/SemaCXX/attr-deprecated.cpp
  cfe/trunk/test/SemaObjC/attr-availability.m
  cfe/trunk/test/SemaObjC/unguarded-availability-new.m
  cfe/trunk/test/SemaObjC/unguarded-availability.m

Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -3881,7 +3881,9 @@
 
   void redelayDiagnostics(sema::DelayedDiagnosticPool );
 
-  void EmitAvailabilityWarning(AvailabilityResult AR, NamedDecl *D,
+  void EmitAvailabilityWarning(AvailabilityResult AR,
+   const NamedDecl *ReferringDecl,
+   const NamedDecl *OffendingDecl,
StringRef Message, SourceLocation Loc,
const ObjCInterfaceDecl *UnknownObjCClass,
const ObjCPropertyDecl *ObjCProperty,
@@ -10413,16 +10415,14 @@
 return OriginalLexicalContext ? OriginalLexicalContext : CurContext;
   }
 
-  /// \brief The diagnostic we should emit for \c D, or \c AR_Available.
-  ///
-  /// \param D The declaration to check. Note that this may be altered to point
-  /// to another declaration that \c D gets it's availability from. i.e., we
-  /// walk the list of typedefs to find an availability attribute.
+  /// The diagnostic we should emit for \c D, and the declaration that
+  /// originated it, or \c AR_Available.
   ///
+  /// \param D The declaration to check.
   /// \param Message If non-null, this will be populated with the message from
   /// the availability attribute that is selected.
-  AvailabilityResult ShouldDiagnoseAvailabilityOfDecl(NamedDecl *,
-  std::string *Message);
+  std::pair
+  ShouldDiagnoseAvailabilityOfDecl(const NamedDecl *D, std::string *Message);
 
   const DeclContext *getCurObjCLexicalContext() const {
 const DeclContext *DC = getCurLexicalContext();
Index: cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
===
--- cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
+++ cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
@@ -124,7 +124,8 @@
 
   static DelayedDiagnostic makeAvailability(AvailabilityResult AR,
 SourceLocation Loc,
-const NamedDecl *D,
+const NamedDecl *ReferringDecl,
+const NamedDecl *OffendingDecl,
 const ObjCInterfaceDecl *UnknownObjCClass,
 const ObjCPropertyDecl  *ObjCProperty,
 StringRef Msg,
@@ -164,9 +165,13 @@
 return *reinterpret_cast(AccessData);
   }
 
-  const NamedDecl *getAvailabilityDecl() const {
+  const NamedDecl *getAvailabilityReferringDecl() const {
 assert(Kind == Availability && "Not an availability diagnostic.");
-return AvailabilityData.Decl;
+return AvailabilityData.ReferringDecl;
+  }
+
+  const NamedDecl *getAvailabilityOffendingDecl() const {
+return AvailabilityData.OffendingDecl;
   }
 
   StringRef getAvailabilityMessage() const {
@@ -213,7 +218,8 @@
 private:
 
   struct AD {
-const NamedDecl *Decl;
+const NamedDecl *ReferringDecl;
+const NamedDecl *OffendingDecl;
 const ObjCInterfaceDecl *UnknownObjCClass;
 const ObjCPropertyDecl  *ObjCProperty;
 const char *Message;
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2880,7 +2880,7 @@
 def warn_partial_availability_new : Warning,
   InGroup;
 def note_partial_availability_silence : Note<
-  "explicitly redeclare %0 to silence this warning">;
+  "annotate %select{%1|anonymous %1}0 with an availability attribute to silence">;
 def note_unguarded_available_silence : Note<
   "enclose %0 in %select{an @available|a __builtin_available}1 check to 

[PATCH] D34913: [clang-tidy] Add a new Android check "android-cloexec-socket"

[Yan Wang via Phabricator via cfe-commits]

yawanng updated this revision to Diff 105293.
yawanng marked 8 inline comments as done.

https://reviews.llvm.org/D34913

Files:
  clang-tidy/android/AndroidTidyModule.cpp
  clang-tidy/android/CMakeLists.txt
  clang-tidy/android/CloexecOpenCheck.cpp
  clang-tidy/android/CloexecSocketCheck.cpp
  clang-tidy/android/CloexecSocketCheck.h
  clang-tidy/utils/ASTUtils.cpp
  clang-tidy/utils/ASTUtils.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/android-cloexec-socket.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/android-cloexec-socket.cpp

Index: test/clang-tidy/android-cloexec-socket.cpp
===
--- /dev/null
+++ test/clang-tidy/android-cloexec-socket.cpp
@@ -0,0 +1,91 @@
+// RUN: %check_clang_tidy %s android-cloexec-socket %t
+
+#define SOCK_STREAM 1
+#define SOCK_DGRAM 2
+#define __O_CLOEXEC 3
+#define SOCK_CLOEXEC __O_CLOEXEC
+#define TEMP_FAILURE_RETRY(exp) \
+  ({\
+int _rc;\
+do {\
+  _rc = (exp);  \
+} while (_rc == -1);\
+  })
+
+extern "C" int socket(int domain, int type, int protocol);
+
+void a() {
+  socket(0, SOCK_STREAM, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'socket' should use SOCK_CLOEXEC where possible [android-cloexec-socket]
+  // CHECK-FIXES: socket(0, SOCK_STREAM | SOCK_CLOEXEC, 0)
+  TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM, 0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:43: warning: 'socket'
+  // CHECK-FIXES: socket(0, SOCK_STREAM | SOCK_CLOEXEC, 0)
+  socket(0, SOCK_STREAM | SOCK_DGRAM, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: 'socket'
+  // CHECK-FIXES: socket(0, SOCK_STREAM | SOCK_DGRAM | SOCK_CLOEXEC, 0)
+  TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM | SOCK_DGRAM, 0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:56: warning: 'socket'
+  // CHECK-FIXES: socket(0, SOCK_STREAM | SOCK_DGRAM | SOCK_CLOEXEC, 0)
+}
+
+void f() {
+  socket(0, 3, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 'socket'
+  // CHECK-FIXES: socket(0, 3 | SOCK_CLOEXEC, 0)
+  TEMP_FAILURE_RETRY(socket(0, 3, 0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: 'socket'
+  // CHECK-FIXES: socket(0, 3 | SOCK_CLOEXEC, 0)
+
+  int flag = 3;
+  socket(0, flag, 0);
+  // CHECK-MESSAGES-NOT: warning:
+  TEMP_FAILURE_RETRY(socket(0, flag, 0));
+  // CHECK-MESSAGES-NOT: warning:
+}
+
+namespace i {
+int socket(int domain, int type, int protocol);
+
+void d() {
+  socket(0, SOCK_STREAM, 0);
+  // CHECK-MESSAGES-NOT: warning:
+  TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM, 0));
+  // CHECK-MESSAGES-NOT: warning:
+  socket(0, SOCK_STREAM | SOCK_DGRAM, 0);
+  // CHECK-MESSAGES-NOT: warning:
+  TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM | SOCK_DGRAM, 0));
+  // CHECK-MESSAGES-NOT: warning:
+}
+
+} // namespace i
+
+void e() {
+  socket(0, SOCK_CLOEXEC, 0);
+  // CHECK-MESSAGES-NOT: warning:
+  TEMP_FAILURE_RETRY(socket(0, SOCK_CLOEXEC, 0));
+  // CHECK-MESSAGES-NOT: warning:
+  socket(0, SOCK_STREAM | SOCK_CLOEXEC, 0);
+  // CHECK-MESSAGES-NOT: warning:
+  TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM | SOCK_CLOEXEC, 0));
+  // CHECK-MESSAGES-NOT: warning:
+  socket(0, SOCK_STREAM | SOCK_CLOEXEC | SOCK_DGRAM, 0);
+  // CHECK-MESSAGES-NOT: warning:
+  TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM | SOCK_CLOEXEC | SOCK_DGRAM, 0));
+  // CHECK-MESSAGES-NOT: warning:
+}
+
+class G {
+public:
+  int socket(int domain, int type, int protocol);
+  void d() {
+socket(0, SOCK_STREAM, 0);
+// CHECK-MESSAGES-NOT: warning:
+TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM, 0));
+// CHECK-MESSAGES-NOT: warning:
+socket(0, SOCK_STREAM | SOCK_DGRAM, 0);
+// CHECK-MESSAGES-NOT: warning:
+TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM | SOCK_DGRAM, 0));
+// CHECK-MESSAGES-NOT: warning:
+  }
+};
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -7,6 +7,7 @@
android-cloexec-creat
android-cloexec-fopen
android-cloexec-open
+   android-cloexec-socket
boost-use-to-string
cert-dcl03-c (redirects to misc-static-assert) 
cert-dcl21-cpp
Index: docs/clang-tidy/checks/android-cloexec-socket.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/android-cloexec-socket.rst
@@ -0,0 +1,17 @@
+.. title:: clang-tidy - android-cloexec-socket
+
+android-cloexec-socket
+==
+
+``socket()`` should include ``SOCK_CLOEXEC`` in its type argument to avoid the
+file descriptor leakage.
+
+Examples:
+
+.. code-block:: c++
+
+  socket(domain, type, SOCK_STREAM);
+
+  // becomes
+
+  socket(domain, type, SOCK_STREAM | SOCK_CLOEXEC);
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -73,6 +73,12 @@
 
   Checks if the required mode ``e`` exists in the 

[PATCH] D34574: [Sema] Disable c++17 aligned new and delete operators if not implemented in the deployment target's c++ standard library

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

This commit breaks all the libc++ aligned new/delete tests on Mac OS. Was that 
deliberate?

Failing Tests (8):

  libc++ :: 
std/language.support/support.dynamic/new.delete/new.delete.array/delete_align_val_t_replace.pass.cpp
  libc++ :: 
std/language.support/support.dynamic/new.delete/new.delete.array/new_align_val_t.pass.cpp
  libc++ :: 
std/language.support/support.dynamic/new.delete/new.delete.array/new_align_val_t_nothrow.pass.cpp
  libc++ :: 
std/language.support/support.dynamic/new.delete/new.delete.array/new_align_val_t_nothrow_replace.pass.cpp
  libc++ :: 
std/language.support/support.dynamic/new.delete/new.delete.single/delete_align_val_t_replace.pass.cpp
  libc++ :: 
std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t.pass.cpp
  libc++ :: 
std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t_nothrow.pass.cpp
  libc++ :: 
std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t_nothrow_replace.pass.cpp


Repository:
  rL LLVM

https://reviews.llvm.org/D34574



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

[PATCH] D34937: Suppressing Reference Counting Diagnostics for Functions Containing 'rc_ownership_trusted_implementation' Annotate Attribute

[Malhar Thakkar via Phabricator via cfe-commits]

malhar1995 updated this revision to Diff 105292.
malhar1995 added a comment.

Corrected one of the two test-cases added in the last-diff.


Repository:
  rL LLVM

https://reviews.llvm.org/D34937

Files:
  lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
  test/Analysis/retain-release-inline.m

Index: test/Analysis/retain-release-inline.m
===
--- test/Analysis/retain-release-inline.m
+++ test/Analysis/retain-release-inline.m
@@ -12,7 +12,7 @@
 //
 // It includes the basic definitions for the test cases below.
 //===--===//
-
+#define NULL 0
 typedef unsigned int __darwin_natural_t;
 typedef unsigned long uintptr_t;
 typedef unsigned int uint32_t;
@@ -267,6 +267,10 @@
 
 extern CFStringRef CFStringCreateWithCStringNoCopy(CFAllocatorRef alloc, const char *cStr, CFStringEncoding encoding, CFAllocatorRef contentsDeallocator);
 
+typedef struct {
+  int ref;
+} isl_basic_map;
+
 //===--===//
 // Test cases.
 //===--===//
@@ -285,6 +289,7 @@
   foo(s);
   bar(s);
 }
+
 void test_neg() {
   NSString *s = [[NSString alloc] init]; // no-warning  
   foo(s);
@@ -294,6 +299,48 @@
   bar(s);
 }
 
+__attribute__((cf_returns_retained)) isl_basic_map *isl_basic_map_copy(isl_basic_map *bmap);
+__attribute__((cf_returns_retained)) isl_basic_map *isl_basic_map_cow(__attribute__((cf_consumed)) isl_basic_map *bmap);
+isl_basic_map *isl_basic_map_decrement_reference_count(__attribute__((cf_consumed)) isl_basic_map *bmap);
+void free(void *);
+
+__attribute__((annotate("rc_ownership_trusted_implementation"))) __attribute__((cf_returns_retained)) isl_basic_map *test_use_after_release_with_trusted_implementation_annotate_attribute(__attribute__((cf_consumed)) isl_basic_map *bmap) {
+  // After this call, 'temp' has a +1 reference count.
+  isl_basic_map *temp = isl_basic_map_copy(bmap);
+  // After this call, 'bmap' has a +1 reference count.
+  bmap = isl_basic_map_cow(bmap);
+  // After this call, 'bmap' has a +0 reference count.
+  isl_basic_map_decrement_reference_count(bmap);
+  isl_basic_map_decrement_reference_count(bmap); // no-warning {{A 'use-after-release' warning is not raised here as this function has 'rc_ownership_trusted_implementation' annotate attribute}}
+  return temp;
+}
+
+isl_basic_map *isl_basic_map_free(__attribute__((cf_consumed)) isl_basic_map *bmap) {
+  if (!bmap)
+return NULL;
+
+  if (--bmap->ref > 0)
+return NULL;
+
+  free(bmap);
+  return NULL;
+}
+
+__attribute__((cf_returns_retained)) isl_basic_map *test_leak_with_trusted_implementation_annotate_attribute(__attribute__((cf_consumed)) isl_basic_map *bmap) {
+  // After this call, 'temp' has a +1 reference count.
+  isl_basic_map *temp = isl_basic_map_copy(bmap);
+  // After this call, 'bmap' has a +1 reference count.
+  bmap = isl_basic_map_cow(bmap); // no-warning {{A 'leak' warning is not raised here as 'foo_bar' (annotated with 'rc_ownership_trusted_implementation') is on the call-stack}}
+  // After this call, assuming the predicate of the second if branch to be true, 'bmap' has a +1 reference count.
+  isl_basic_map_free(bmap);
+  return temp;
+}
+
+__attribute__((annotate("rc_ownership_trusted_implementation"))) __attribute__((cf_returns_retained)) isl_basic_map *foo_bar(__attribute__((cf_consumed)) isl_basic_map *bmap) {
+  bmap = test_leak_with_trusted_implementation_annotate_attribute(bmap);
+  return bmap;
+}
+
 //===--===//
 // Test returning retained and not-retained values.
 //===--===//
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -1894,6 +1894,23 @@
   return SFC->getAnalysisDeclContext()->isBodyAutosynthesized();
 }
 
+/// Returns true if any declaration on the call-stack contains
+/// 'rc_ownership_trusted_implementation' annotate attribute.
+bool isTrustedReferenceCountImplementation(const LocationContext *LCtx) {
+  while (LCtx) {
+if (const StackFrameContext *SFC = dyn_cast(LCtx)) {
+  const Decl *D = SFC->getDecl();
+  for (const auto *Ann : D->specific_attrs()) {
+if (Ann->getAnnotation() == "rc_ownership_trusted_implementation") {
+  return true;
+}
+  }
+}
+LCtx = LCtx->getParent();
+  }
+  return false;
+}
+
 std::shared_ptr
 CFRefReportVisitor::VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN,
   BugReporterContext , BugReport ) {
@@ -3345,11 +3362,13 @@
   }
 
   assert(BT);
-  auto report = std::unique_ptr(
-  new CFRefReport(*BT, 

[PATCH] D34556: [libcxx] Annotate c++17 aligned new/delete operators with availability attribute

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

> I'm not exactly sure why the test failed, but the patch was reverted in 
> r306859.

My bad, since the tests are failing due to clang whining about the system dylib 
not supporting aligned new/delete on 10.12,  I meant to post this on 
https://reviews.llvm.org/D34574


Repository:
  rL LLVM

https://reviews.llvm.org/D34556



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

[PATCH] D34980: [OpenCL] Test on image access modifiers and image type can only be a type of a function argument.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

LGTM! Thanks!


https://reviews.llvm.org/D34980



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

[PATCH] D35000: [OpenCL] Added extended tests on metadata generation for half data type and arrays.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Btw, is there any reason to add testing specifically for half? Is there 
anything specific to half in the implementation of this?




Comment at: test/CodeGenOpenCL/kernel-arg-info.cl:79
+kernel void foo6_2(global const volatile half * restrict 
globalconstvolatilehalfrestrictp,
+   local half*localhalfp,
+   local half *restrict localhalfrestrictp,

could you add space b/w half and *. Also could we format consistently below?


https://reviews.llvm.org/D35000



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

[PATCH] D34556: [libcxx] Annotate c++17 aligned new/delete operators with availability attribute

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

I'm not exactly sure why the test failed, but the patch was reverted in r306859.


Repository:
  rL LLVM

https://reviews.llvm.org/D34556



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

[PATCH] D34671: This is to address more command from Richard Smith for my change of https://reviews.llvm.org/D33333

[Erich Keane via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL307172: Address comments that escaped D3 (authored by 
erichkeane).

Changed prior to commit:
  https://reviews.llvm.org/D34671?vs=104966=105284#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34671

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
  cfe/trunk/test/CXX/except/except.spec/p11.cpp
  cfe/trunk/test/SemaCXX/warn-throw-out-noexcept-func.cpp

Index: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
===
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
@@ -394,15 +394,21 @@
 
 static void EmitDiagForCXXThrowInNonThrowingFunc(Sema , SourceLocation OpLoc,
  const FunctionDecl *FD) {
-  if (!S.getSourceManager().isInSystemHeader(OpLoc)) {
+  if (!S.getSourceManager().isInSystemHeader(OpLoc) &&
+  FD->getTypeSourceInfo()) {
 S.Diag(OpLoc, diag::warn_throw_in_noexcept_func) << FD;
 if (S.getLangOpts().CPlusPlus11 &&
 (isa(FD) ||
  FD->getDeclName().getCXXOverloadedOperator() == OO_Delete ||
- FD->getDeclName().getCXXOverloadedOperator() == OO_Array_Delete))
-  S.Diag(FD->getLocation(), diag::note_throw_in_dtor);
-else
-  S.Diag(FD->getLocation(), diag::note_throw_in_function);
+ FD->getDeclName().getCXXOverloadedOperator() == OO_Array_Delete)) {
+  if (const auto *Ty = FD->getTypeSourceInfo()->getType()->
+ getAs())
+S.Diag(FD->getLocation(), diag::note_throw_in_dtor)
+<< !isa(FD) << !Ty->hasExceptionSpec()
+<< FD->getExceptionSpecSourceRange();
+} else 
+  S.Diag(FD->getLocation(), diag::note_throw_in_function)
+  << FD->getExceptionSpecSourceRange();
   }
 }
 
@@ -420,8 +426,7 @@
 
 static bool isNoexcept(const FunctionDecl *FD) {
   const auto *FPT = FD->getType()->castAs();
-  if (FPT->getExceptionSpecType() != EST_None &&
-  FPT->isNothrow(FD->getASTContext()))
+  if (FPT->isNothrow(FD->getASTContext()))
 return true;
   return false;
 }
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6355,15 +6355,13 @@
   "cannot use '%0' with exceptions disabled">;
 def err_objc_exceptions_disabled : Error<
   "cannot use '%0' with Objective-C exceptions disabled">;
-def warn_throw_in_noexcept_func 
-: Warning<"%0 has a non-throwing exception specification but can still "
-  "throw, resulting in unexpected program termination">,
-  InGroup;
-def note_throw_in_dtor 
-: Note<"destructor or deallocator has a (possibly implicit) non-throwing "
-  "excepton specification">;
-def note_throw_in_function 
-: Note<"non-throwing function declare here">;
+def warn_throw_in_noexcept_func : Warning<
+  "%0 has a non-throwing exception specification but can still throw">,
+  InGroup;
+def note_throw_in_dtor : Note<
+  "%select{destructor|deallocator}0 has a %select{non-throwing|implicit "
+  "non-throwing}1 exception specification">;
+def note_throw_in_function : Note<"function declared non-throwing here">;
 def err_seh_try_outside_functions : Error<
   "cannot use SEH '__try' in blocks, captured regions, or Obj-C method decls">;
 def err_mixing_cxx_try_seh_try : Error<
Index: cfe/trunk/test/SemaCXX/warn-throw-out-noexcept-func.cpp
===
--- cfe/trunk/test/SemaCXX/warn-throw-out-noexcept-func.cpp
+++ cfe/trunk/test/SemaCXX/warn-throw-out-noexcept-func.cpp
@@ -2,16 +2,16 @@
 struct A_ShouldDiag {
   ~A_ShouldDiag(); // implicitly noexcept(true)
 };
-A_ShouldDiag::~A_ShouldDiag() { // expected-note {{destructor or deallocator has a (possibly implicit) non-throwing excepton specification}}
-  throw 1; // expected-warning {{has a non-throwing exception specification but can still throw, resulting in unexpected program termination}}
+A_ShouldDiag::~A_ShouldDiag() { // expected-note {{destructor has a implicit non-throwing exception specification}}
+  throw 1; // expected-warning {{has a non-throwing exception specification but can still throw}}
 }
 struct B_ShouldDiag {
   int i;
   ~B_ShouldDiag() noexcept(true) {} //no disg, no throw stmt
 };
 struct R_ShouldDiag : A_ShouldDiag {
   B_ShouldDiag b;
-  ~R_ShouldDiag() { // expected-note  {{destructor or deallocator has a}}
+  ~R_ShouldDiag() { // expected-note  {{destructor has a implicit non-throwing exception specification}}
 throw 1; // expected-warning {{has a non-throwing exception specification but}}
   }
 };
@@ -30,18 +30,18 @@
   ~N_ShouldDiag(); //implicitly noexcept(true)
 };
 
-N_ShouldDiag::~N_ShouldDiag() {  // 

r307172 - Address comments that escaped D33333

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Wed Jul  5 09:43:45 2017
New Revision: 307172

URL: http://llvm.org/viewvc/llvm-project?rev=307172=rev
Log:
Address comments that escaped D3

Patch By: Jen Yu

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/test/CXX/except/except.spec/p11.cpp
cfe/trunk/test/SemaCXX/warn-throw-out-noexcept-func.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=307172=307171=307172=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jul  5 09:43:45 
2017
@@ -6355,15 +6355,13 @@ def err_exceptions_disabled : Error<
   "cannot use '%0' with exceptions disabled">;
 def err_objc_exceptions_disabled : Error<
   "cannot use '%0' with Objective-C exceptions disabled">;
-def warn_throw_in_noexcept_func 
-: Warning<"%0 has a non-throwing exception specification but can still "
-  "throw, resulting in unexpected program termination">,
-  InGroup;
-def note_throw_in_dtor 
-: Note<"destructor or deallocator has a (possibly implicit) non-throwing "
-  "excepton specification">;
-def note_throw_in_function 
-: Note<"non-throwing function declare here">;
+def warn_throw_in_noexcept_func : Warning<
+  "%0 has a non-throwing exception specification but can still throw">,
+  InGroup;
+def note_throw_in_dtor : Note<
+  "%select{destructor|deallocator}0 has a %select{non-throwing|implicit "
+  "non-throwing}1 exception specification">;
+def note_throw_in_function : Note<"function declared non-throwing here">;
 def err_seh_try_outside_functions : Error<
   "cannot use SEH '__try' in blocks, captured regions, or Obj-C method decls">;
 def err_mixing_cxx_try_seh_try : Error<

Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=307172=307171=307172=diff
==
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Wed Jul  5 09:43:45 2017
@@ -394,15 +394,21 @@ static bool hasThrowOutNonThrowingFunc(S
 
 static void EmitDiagForCXXThrowInNonThrowingFunc(Sema , SourceLocation OpLoc,
  const FunctionDecl *FD) {
-  if (!S.getSourceManager().isInSystemHeader(OpLoc)) {
+  if (!S.getSourceManager().isInSystemHeader(OpLoc) &&
+  FD->getTypeSourceInfo()) {
 S.Diag(OpLoc, diag::warn_throw_in_noexcept_func) << FD;
 if (S.getLangOpts().CPlusPlus11 &&
 (isa(FD) ||
  FD->getDeclName().getCXXOverloadedOperator() == OO_Delete ||
- FD->getDeclName().getCXXOverloadedOperator() == OO_Array_Delete))
-  S.Diag(FD->getLocation(), diag::note_throw_in_dtor);
-else
-  S.Diag(FD->getLocation(), diag::note_throw_in_function);
+ FD->getDeclName().getCXXOverloadedOperator() == OO_Array_Delete)) {
+  if (const auto *Ty = FD->getTypeSourceInfo()->getType()->
+ getAs())
+S.Diag(FD->getLocation(), diag::note_throw_in_dtor)
+<< !isa(FD) << !Ty->hasExceptionSpec()
+<< FD->getExceptionSpecSourceRange();
+} else 
+  S.Diag(FD->getLocation(), diag::note_throw_in_function)
+  << FD->getExceptionSpecSourceRange();
   }
 }
 
@@ -420,8 +426,7 @@ static void checkThrowInNonThrowingFunc(
 
 static bool isNoexcept(const FunctionDecl *FD) {
   const auto *FPT = FD->getType()->castAs();
-  if (FPT->getExceptionSpecType() != EST_None &&
-  FPT->isNothrow(FD->getASTContext()))
+  if (FPT->isNothrow(FD->getASTContext()))
 return true;
   return false;
 }

Modified: cfe/trunk/test/CXX/except/except.spec/p11.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p11.cpp?rev=307172=307171=307172=diff
==
--- cfe/trunk/test/CXX/except/except.spec/p11.cpp (original)
+++ cfe/trunk/test/CXX/except/except.spec/p11.cpp Wed Jul  5 09:43:45 2017
@@ -1,10 +1,10 @@
 // RUN: %clang_cc1 -std=c++11 -fexceptions -fcxx-exceptions -fsyntax-only 
-verify %s
 
 // This is the "let the user shoot themselves in the foot" clause.
-void f() noexcept { // expected-note {{non-throwing function declare here}}
+void f() noexcept { // expected-note {{function declared non-throwing here}}
   throw 0; // expected-warning {{has a non-throwing exception specification 
but}} 
 }
-void g() throw() { // expected-note {{non-throwing function declare here}}
+void g() throw() { // expected-note {{function declared non-throwing here}}
   throw 0; // 

Re: [PATCH] D34992: Emit static constexpr member as available_externally definition

[David Majnemer via cfe-commits]

On Wed, Jul 5, 2017 at 12:22 PM Mehdi AMINI  wrote:

> The LLVM verifier is complaining that dllimport have to be external
> linkage and isn't happy with available_externally, is the verifier wrong?
>

IMO, yes. I imagine that it is fine with dllimport available_externally
functions already.


> 2017-07-05 9:12 GMT-07:00 David Majnemer :
>
>> I don't think you need the dllimport restriction.
>>
>> On Wed, Jul 5, 2017 at 12:05 PM Alex Lorenz via Phabricator via
>> cfe-commits  wrote:
>>
>>> arphaman added a comment.
>>>
>>> Does this apply to all constexpr global variables? It could potentially
>>> fix https://bugs.llvm.org/show_bug.cgi?id=31860 .
>>>
>>>
>>> https://reviews.llvm.org/D34992
>>>
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34947: [clangd] Add support for per-file extra flags

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D34947



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

[libcxx] r307171 - Fix a bug in regex_Iterator where it would report zero-length matches forever. Reported as http://llvm.org/PR33681. Thanks to Karen Arutyunov for the report.

[Marshall Clow via cfe-commits]

Author: marshall
Date: Wed Jul  5 09:37:19 2017
New Revision: 307171

URL: http://llvm.org/viewvc/llvm-project?rev=307171=rev
Log:
Fix a bug in regex_Iterator where it would report zero-length matches forever. 
Reported as http://llvm.org/PR33681. Thanks to Karen Arutyunov for the report.

Modified:
libcxx/trunk/include/regex
libcxx/trunk/test/std/re/re.iter/re.regiter/re.regiter.incr/post.pass.cpp

Modified: libcxx/trunk/include/regex
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/regex?rev=307171=307170=307171=diff
==
--- libcxx/trunk/include/regex (original)
+++ libcxx/trunk/include/regex Wed Jul  5 09:37:19 2017
@@ -6142,7 +6142,7 @@ regex_iterator<_BidirectionalIterator, _
 {
 __flags_ |= regex_constants::__no_update_pos;
 _BidirectionalIterator __start = __match_[0].second;
-if (__match_.empty())
+if (__match_[0].first == __match_[0].second)
 {
 if (__start == __end_)
 {

Modified: 
libcxx/trunk/test/std/re/re.iter/re.regiter/re.regiter.incr/post.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/re/re.iter/re.regiter/re.regiter.incr/post.pass.cpp?rev=307171=307170=307171=diff
==
--- libcxx/trunk/test/std/re/re.iter/re.regiter/re.regiter.incr/post.pass.cpp 
(original)
+++ libcxx/trunk/test/std/re/re.iter/re.regiter/re.regiter.incr/post.pass.cpp 
Wed Jul  5 09:37:19 2017
@@ -95,4 +95,22 @@ int main()
 assert((*i2).position() == 0);
 assert((*i2).str() == "555-1234");
 }
+{ // http://llvm.org/PR33681
+std::regex rex(".*");
+const char foo[] = "foo";
+//  The -1 is because we don't want the implicit null from the array.
+std::cregex_iterator i(std::begin(foo), std::end(foo) - 1, rex);
+std::cregex_iterator e;
+assert(i != e);
+assert((*i).size() == 1);
+assert((*i).str() == "foo");
+
+++i;
+assert(i != e);
+assert((*i).size() == 1);
+assert((*i).str() == "");
+
+++i;
+assert(i == e);
+}
 }


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

[PATCH] D34404: [Clang-Tidy] Preserve Message, FileOffset, FilePath in Clang-Tidy YAML output

[Vladimir Plyashkun via Phabricator via cfe-commits]

vladimir.plyashkun updated this revision to Diff 105283.
vladimir.plyashkun edited the summary of this revision.
vladimir.plyashkun added a comment.

- made code less verbose
- used assignment initialization form
- removed extra `.c_str()` call


Repository:
  rL LLVM

https://reviews.llvm.org/D34404

Files:
  include/clang/Tooling/DiagnosticsYaml.h
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/DiagnosticsYamlTest.cpp
  unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
  unittests/clang-apply-replacements/CMakeLists.txt

Index: unittests/clang-apply-replacements/CMakeLists.txt
===
--- unittests/clang-apply-replacements/CMakeLists.txt
+++ unittests/clang-apply-replacements/CMakeLists.txt
@@ -8,6 +8,7 @@
   )
 
 add_extra_unittest(ClangApplyReplacementsTests
+  ApplyReplacementsTest.cpp
   ReformattingTest.cpp
   )
 
Index: unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
===
--- /dev/null
+++ unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
@@ -0,0 +1,49 @@
+//===- clang-apply-replacements/ApplyReplacementsTest.cpp --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang-apply-replacements/Tooling/ApplyReplacements.h"
+#include "gtest/gtest.h"
+
+using namespace clang::replace;
+using namespace llvm;
+
+namespace clang { 
+namespace tooling {
+
+static TUDiagnostics makeTUDiagnostics(const std::string ,
+   StringRef DiagnosticName,
+   const DiagnosticMessage ,
+   const StringMap ,
+   StringRef BuildDirectory) {
+  TUDiagnostics TUs;
+  TUs.push_back({ MainSourceFile, 
+  { { DiagnosticName, Message, Replacements, {}, 
+  Diagnostic::Warning, BuildDirectory } } 
+  });
+  return TUs;
+}
+
+// Test to ensure diagnostics with no fixes, will be merged correctly 
+// before applying.
+TEST(ApplyReplacementsTest, mergeDiagnosticsWithNoFixes) {
+  IntrusiveRefCntPtr DiagOpts(new DiagnosticOptions());
+  DiagnosticsEngine Diagnostics(
+IntrusiveRefCntPtr(new DiagnosticIDs()), DiagOpts.get());
+  FileManager Files((FileSystemOptions()));
+  SourceManager SM(Diagnostics, Files);
+  TUDiagnostics TUs = makeTUDiagnostics("path/to/source.cpp", "diagnostic",
+{}, {}, "path/to");
+  FileToReplacementsMap ReplacementsMap;
+
+  EXPECT_TRUE(mergeAndDeduplicate(TUs, ReplacementsMap, SM));
+  EXPECT_TRUE(ReplacementsMap.empty());
+}
+
+} // end namespace tooling
+} // end namespace clang
Index: unittests/Tooling/DiagnosticsYamlTest.cpp
===
--- /dev/null
+++ unittests/Tooling/DiagnosticsYamlTest.cpp
@@ -0,0 +1,173 @@
+//===- unittests/Tooling/DiagnosticsYamlTest.cpp - Serialization tests ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Tests for serialization of Diagnostics.
+//
+//===--===//
+
+#include "clang/Tooling/Core/Diagnostic.h"
+#include "clang/Tooling/DiagnosticsYaml.h"
+#include "clang/Tooling/ReplacementsYaml.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang::tooling;
+
+static Diagnostic makeDiagnostic(StringRef DiagnosticName, 
+ const std::string ,
+ int FileOffset,
+ const std::string ,
+ const StringMap ) {
+  DiagnosticMessage DiagMessage;
+  DiagMessage.Message = Message;
+  DiagMessage.FileOffset = FileOffset;
+  DiagMessage.FilePath = FilePath;
+  return Diagnostic(DiagnosticName, DiagMessage, Fix, {},
+Diagnostic::Warning, "path/to/build/directory");
+}
+
+TEST(DiagnosticsYamlTest, serializesDiagnostics) {
+  TranslationUnitDiagnostics TUD;
+  TUD.MainSourceFile = "path/to/source.cpp";
+
+  StringMap Fix1 = { 
+{ 
+  "path/to/source.cpp",  
+  Replacements({ "path/to/source.cpp", 100, 12, "replacement #1" }) 
+} 
+  };
+  TUD.Diagnostics.push_back(makeDiagnostic("diagnostic#1", "message #1", 55, 
+   "path/to/source.cpp", Fix1));
+  
+  StringMap Fix2 = {
+{ 
+  "path/to/header.h",
+  Replacements({ "path/to/header.h", 62, 2, "replacement #2" })
+} 
+  };
+  

Re: [PATCH] D34992: Emit static constexpr member as available_externally definition

[Mehdi AMINI via cfe-commits]

The LLVM verifier is complaining that dllimport have to be external linkage
and isn't happy with available_externally, is the verifier wrong?

2017-07-05 9:12 GMT-07:00 David Majnemer :

> I don't think you need the dllimport restriction.
>
> On Wed, Jul 5, 2017 at 12:05 PM Alex Lorenz via Phabricator via
> cfe-commits  wrote:
>
>> arphaman added a comment.
>>
>> Does this apply to all constexpr global variables? It could potentially
>> fix https://bugs.llvm.org/show_bug.cgi?id=31860 .
>>
>>
>> https://reviews.llvm.org/D34992
>>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D34992: Emit static constexpr member as available_externally definition

[David Majnemer via cfe-commits]

I don't think you need the dllimport restriction.

On Wed, Jul 5, 2017 at 12:05 PM Alex Lorenz via Phabricator via cfe-commits
 wrote:

> arphaman added a comment.
>
> Does this apply to all constexpr global variables? It could potentially
> fix https://bugs.llvm.org/show_bug.cgi?id=31860 .
>
>
> https://reviews.llvm.org/D34992
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34556: [libcxx] Annotate c++17 aligned new/delete operators with availability attribute

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

This doesn't work when running the libc++ tests against a non-system libc++. 
(which is what all the libc++ developers and AND all the test bots do)


Repository:
  rL LLVM

https://reviews.llvm.org/D34556



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

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

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

gtbercea updated this revision to Diff 105280.
gtbercea marked 3 inline comments as done.
gtbercea added a comment.

Address comments.


https://reviews.llvm.org/D34784

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Cuda.cpp
  test/Driver/openmp-offload.c

Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -597,3 +597,19 @@
 // RUN:   | FileCheck -check-prefix=CHK-FOPENMP-IS-DEVICE %s
 
 // CHK-FOPENMP-IS-DEVICE: clang{{.*}} "-aux-triple" "powerpc64le-unknown-linux-gnu" {{.*}}.c" "-fopenmp-is-device" "-fopenmp-host-ir-file-path"
+
+/// ###
+
+/// Check -Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8 is passed when compiling for the device.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu -Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-EQ-TARGET %s
+
+// CHK-FOPENMP-EQ-TARGET: clang{{.*}} argument unused during compilation: '-Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8'
+
+/// ###
+
+/// Check -Xopenmp-target -march=pwr8 is passed when compiling for the device.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu -Xopenmp-target -march=pwr8 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-TARGET %s
+
+// CHK-FOPENMP-TARGET: clang{{.*}} argument unused during compilation: '-Xopenmp-target -march=pwr8'
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -212,8 +212,20 @@
   static_cast(getToolChain());
   assert(TC.getTriple().isNVPTX() && "Wrong platform");
 
+  StringRef GPUArchName;
+  std::vector GPUArchNames;
+  // If this is an OpenMP action we need to extract the device architecture from
+  // the -fopenmp-target-arch option.
+  if (JA.isDeviceOffloading(Action::OFK_OpenMP)) {
+GPUArchNames = Args.getAllArgValues(options::OPT_march_EQ);
+assert(GPUArchNames.size() == 1 &&
+   "Exactly one GPU Arch required for ptxas.");
+GPUArchName = GPUArchNames[0];
+  } else
+GPUArchName = JA.getOffloadingArch();
+
   // Obtain architecture from the action.
-  CudaArch gpu_arch = StringToCudaArch(JA.getOffloadingArch());
+  CudaArch gpu_arch = StringToCudaArch(GPUArchName);
   assert(gpu_arch != CudaArch::UNKNOWN &&
  "Device action expected to have an architecture.");
 
@@ -392,6 +404,15 @@
   CudaInstallation.AddCudaIncludeArgs(DriverArgs, CC1Args);
 }
 
+void AddMArchOption(DerivedArgList *DAL,
+const OptTable ,
+StringRef Opt) {
+  if (Opt.startswith("-march="))
+DAL->AddJoinedArg(nullptr,
+Opts.getOption(options::OPT_march_EQ),
+Opt.split("=").second);
+}
+
 llvm::opt::DerivedArgList *
 CudaToolChain::TranslateArgs(const llvm::opt::DerivedArgList ,
  StringRef BoundArch,
@@ -405,7 +426,7 @@
 
   // For OpenMP device offloading, append derived arguments. Make sure
   // flags are not duplicated.
-  // TODO: Append the compute capability.
+  // Also append the compute capability.
   if (DeviceOffloadKind == Action::OFK_OpenMP) {
 for (Arg *A : Args){
   bool IsDuplicate = false;
@@ -418,6 +439,48 @@
   if (!IsDuplicate)
 DAL->append(A);
 }
+
+// Get the compute capability from the -Xopenmp-target flag.
+auto OptList = Args.getAllArgValues(options::OPT_Xopenmp_target_EQ);
+
+// For each OPT_Xopenmp_target_EQ option, the function returns
+// two strings, the triple and the option.
+// The following format is assumed:
+//
+// -Xopenmp-target=nvptx64-nvidia-cuda -opt=val
+for (unsigned i = 0; i < OptList.size(); i+=2) {
+  StringRef Opt = OptList[i+1];
+  if (OptList[i] == getTripleString())
+AddMArchOption(DAL, Opts, Opt);
+}
+
+OptList = Args.getAllArgValues(options::OPT_Xopenmp_target);
+// When there is only one option in the list, the following format
+// is assumed:
+//
+// -Xopenmp-target -opt=val
+
+// By default, if no triple is explicitely specified, we
+// associate -opt=val with the toolchain specified under the
+// -fopenmp-targets flag (provided that there is only one such
+// toolchain specified).
+if (!OptList.empty() &&
+Args.getAllArgValues(options::OPT_fopenmp_targets_EQ).size() != 1)
+  getDriver().Diag(diag::err_drv_Xopenmp_target_missing_triple);
+
+// Add arch
+for (StringRef Opt : OptList) {
+  AddMArchOption(DAL, Opts, Opt);
+}
+
+auto MArchList = DAL->getAllArgValues(options::OPT_march_EQ);
+if (MArchList.size() > 1)
+   

[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

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

gtbercea added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:443
+
+// Get the compute capability from the -fopenmp-targets flag.
+// The default compute capability is sm_20 since this is a CUDA

hfinkel wrote:
> Is this first sentence accurate?
Fixed. It should be -Xopenmp-target



Comment at: lib/Driver/ToolChains/Cuda.cpp:446
+// tool chain.
+auto OptList = Args.getAllArgValues(options::OPT_Xopenmp_target_EQ);
+

hfinkel wrote:
> Why is this logic in this function? Don't you need the same logic in 
> Generic_GCC::TranslateArgs to handle non-CUDA offloading toolchains?
> 
I would imagine that each toolchain needs to parse the list of flags since, 
given a toolchain, the flag may need to be passed to more than one tool and 
different tools may require different flags for passing the same information.



Comment at: lib/Driver/ToolChains/Cuda.cpp:474
+for (StringRef Opt : OptList) {
+  AddMArchOption(DAL, Opts, Opt);
+}

hfinkel wrote:
> Shouldn't you be adding all of the options, not just the -march= ones?
I thought that that would be the case but there are a few issues:

1. PTXAS and NVLINK each use a different flag for specifying the arch, and, in 
turn, each flag is different from -march.

2. -Xopenmp-target passes a flag to the entire toolchain not to individual 
components of the toolchain so a translation of flags is required in some cases 
to adapt the flag to the actual tool. -march is one example, I'm not sure if 
there are others.

3. At this point in the code, in order to add a flag and its value to the DAL 
list, I need to be able to specify the option type (i.e. 
options::OPT_march_EQ). I therefore need to manually recognize the flag in the 
string representing the value of -Xopenmp-target or -Xopenmp-target=triple.

4. This patch handles the passing of the arch and can be extended to pass other 
flags (as is stands, no other flags are passed through to the CUDA toolchain). 
This can be extended on a flag by flag basis for flags that need translating to 
a particular tool's flag. If the flag doesn't need translating then the flag 
and it's value can be appended to the command line as they are.


https://reviews.llvm.org/D34784



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

[PATCH] D35015: [clang-format] Add space between a message field key and the opening bracket in proto messages

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.
Herald added a subscriber: klimek.

This patch updates the formatting of message fields of type `a{...}` to `a 
{...}`
for proto messages.


https://reviews.llvm.org/D35015

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestProto.cpp
  unittests/Format/FormatTestTextProto.cpp

Index: unittests/Format/FormatTestTextProto.cpp
===
--- unittests/Format/FormatTestTextProto.cpp
+++ unittests/Format/FormatTestTextProto.cpp
@@ -61,13 +61,13 @@
"  field_e: 23\n"
"}");
 
-  verifyFormat("msg_field{}");
+  verifyFormat("msg_field {}");
 
-  verifyFormat("msg_field{field_a: A}");
+  verifyFormat("msg_field {field_a: A}");
 
-  verifyFormat("msg_field{field_a: \"OK\" field_b: 123}");
+  verifyFormat("msg_field {field_a: \"OK\" field_b: 123}");
 
-  verifyFormat("msg_field{\n"
+  verifyFormat("msg_field {\n"
"  field_a: 1\n"
"  field_b: OK\n"
"  field_c: \"OK\"\n"
@@ -78,12 +78,12 @@
"  field_h: 1234.567e-89\n"
"}");
 
-  verifyFormat("msg_field: {msg_field{field_a: 1}}");
+  verifyFormat("msg_field: {msg_field {field_a: 1}}");
 
   verifyFormat("id: \"ala.bala\"\n"
-   "item{type: ITEM_A rank: 1 score: 90.0}\n"
-   "item{type: ITEM_B rank: 2 score: 70.5}\n"
-   "item{\n"
+   "item {type: ITEM_A rank: 1 score: 90.0}\n"
+   "item {type: ITEM_B rank: 2 score: 70.5}\n"
+   "item {\n"
"  type: ITEM_A\n"
"  rank: 3\n"
"  score: 20.0\n"
@@ -113,13 +113,13 @@
 
   verifyFormat("a: {\n"
"  field_a: OK\n"
-   "  field_b{field_c: OK}\n"
+   "  field_b {field_c: OK}\n"
"  field_d: OKOKOK\n"
"  field_e: OK\n"
"}");
 
   verifyFormat("field_a: OK,\n"
-   "field_b{field_c: OK},\n"
+   "field_b {field_c: OK},\n"
"field_d: OKOKOK,\n"
"field_e: OK");
 }
@@ -134,33 +134,33 @@
"}");
 
   verifyFormat("field_a: OK\n"
-   "msg_field{\n"
+   "msg_field {\n"
"  field_b: OK  // Comment\n"
"}");
 }
 
 TEST_F(FormatTestTextProto, SupportsAngleBracketMessageFields) {
   // Single-line tests
-  verifyFormat("msg_field<>");
+  verifyFormat("msg_field <>");
   verifyFormat("msg_field: <>");
-  verifyFormat("msg_field");
+  verifyFormat("msg_field ");
   verifyFormat("msg_field: ");
-  verifyFormat("msg_field>");
-  verifyFormat("msg_field>>");
-  verifyFormat("msg_field: >");
-  verifyFormat("msg_field");
-  verifyFormat("msg_field, field_c: OK>");
-  verifyFormat("msg_field");
+  verifyFormat("msg_field >");
+  verifyFormat("msg_field >>");
+  verifyFormat("msg_field: >>");
+  verifyFormat("msg_field ");
+  verifyFormat("msg_field , field_c: OK>");
+  verifyFormat("msg_field >");
   verifyFormat("msg_field: ");
   verifyFormat("msg_field: , field_c: OK>");
-  verifyFormat("msg_field: ");
+  verifyFormat("msg_field: >");
   verifyFormat("field_a: \"OK\", msg_field: , field_c: {}");
-  verifyFormat("field_a, msg_field: , field_c<>");
-  verifyFormat("field_a msg_field:  field_c<>");
-  verifyFormat("field, field<>> field: ");
+  verifyFormat("field_a , msg_field: , field_c <>");
+  verifyFormat("field_a  msg_field:  field_c <>");
+  verifyFormat("field >, field <>> field: ");
 
   // Multiple lines tests
-  verifyFormat("msg_field<\n"
+  verifyFormat("msg_field <\n"
"  field_a: OK\n"
"  field_b: \"OK\"\n"
"  field_c: 1\n"
@@ -175,25 +175,25 @@
"msg_field: ");
 
   verifyFormat("field_a: OK,\n"
-   "field_b,\n"
+   "field_b ,\n"
"field_d: <12.5>,\n"
"field_e: OK");
 
   verifyFormat("field_a: OK\n"
-   "field_b\n"
+   "field_b \n"
"field_d: <12.5>\n"
"field_e: OKOKOK");
 
-  verifyFormat("msg_field<\n"
+  verifyFormat("msg_field <\n"
"  field_a: OK,\n"
-   "  field_b,\n"
+   "  field_b ,\n"
"  field_d: <12.5>,\n"
"  field_e: OK\n"
">");
 
-  verifyFormat("msg_field<\n"
+  verifyFormat("msg_field <\n"
"  field_a: ,\n"
-   "  field_b,\n"
+   "  field_b ,\n"
"  field_d: <12.5>,\n"
"  field_e: OK,\n"
">");
@@ -206,7 +206,7 @@
"  field_g: OK\n"
">");
 
-  verifyFormat("field_a{\n"
+  verifyFormat("field_a {\n"
"  field_d: ok\n"
"  field_b: \n"
"  field_d: ok\n"
@@ -221,16 

[PATCH] D34947: [clangd] Add support for per-file extra flags

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir updated this revision to Diff 105274.
krasimir added a comment.

- Address review comment


https://reviews.llvm.org/D34947

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdUnitStore.cpp
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  test/clangd/extra-flags.test

Index: test/clangd/extra-flags.test
===
--- /dev/null
+++ test/clangd/extra-flags.test
@@ -0,0 +1,22 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+#
+Content-Length: 205
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.c","languageId":"c","version":1,"text":"int main() { int i; return i; }"},"metadata":{"extraFlags":["-Wall"]}}}
+# CHECK: {"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":"file:///foo.c","diagnostics":[{"range":{"start": {"line": 0, "character": 28}, "end": {"line": 0, "character": 28}},"severity":2,"message":"variable 'i' is uninitialized when used here"},{"range":{"start": {"line": 0, "character": 19}, "end": {"line": 0, "character": 19}},"severity":3,"message":"initialize the variable 'i' to silence this warning"}]}}
+#
+Content-Length: 175
+
+{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"file:///foo.c","version":2},"contentChanges":[{"text":"int main() { int i; return i; }"}]}}
+# CHECK: {"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":"file:///foo.c","diagnostics":[{"range":{"start": {"line": 0, "character": 28}, "end": {"line": 0, "character": 28}},"severity":2,"message":"variable 'i' is uninitialized when used here"},{"range":{"start": {"line": 0, "character": 19}, "end": {"line": 0, "character": 19}},"severity":3,"message":"initialize the variable 'i' to silence this warning"}]}}
+#
+Content-Length: 44
+
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
+
+
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -118,6 +118,12 @@
   static std::string unparse(const Location );
 };
 
+struct Metadata {
+  std::vector extraFlags;
+
+  static llvm::Optional parse(llvm::yaml::MappingNode *Params);
+};
+
 struct TextEdit {
   /// The range of the text document to be manipulated. To insert
   /// text into a document create a range where start === end.
@@ -152,6 +158,9 @@
   /// The document that was opened.
   TextDocumentItem textDocument;
 
+  /// Extension storing per-file metadata, such as compilation flags.
+  llvm::Optional metadata;
+
   static llvm::Optional
   parse(llvm::yaml::MappingNode *Params);
 };
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -204,6 +204,33 @@
   return Result;
 }
 
+llvm::Optional Metadata::parse(llvm::yaml::MappingNode *Params) {
+  Metadata Result;
+  for (auto  : *Params) {
+auto *KeyString = dyn_cast(NextKeyValue.getKey());
+if (!KeyString)
+  return llvm::None;
+
+llvm::SmallString<10> KeyStorage;
+StringRef KeyValue = KeyString->getValue(KeyStorage);
+auto *Value = NextKeyValue.getValue();
+
+llvm::SmallString<10> Storage;
+if (KeyValue == "extraFlags") {
+  auto *Seq = dyn_cast(Value);
+  if (!Seq)
+return llvm::None;
+  for (auto  : *Seq) {
+auto *Node = dyn_cast();
+if (!Node)
+  return llvm::None;
+Result.extraFlags.push_back(Node->getValue(Storage));
+  }
+}
+  }
+  return Result;
+}
+
 llvm::Optional TextEdit::parse(llvm::yaml::MappingNode *Params) {
   TextEdit Result;
   for (auto  : *Params) {
@@ -265,6 +292,11 @@
   if (!Parsed)
 return llvm::None;
   Result.textDocument = std::move(*Parsed);
+} else if (KeyValue == "metadata") {
+  auto Parsed = Metadata::parse(Value);
+  if (!Parsed)
+return llvm::None;
+  Result.metadata = std::move(*Parsed);
 } else {
   return llvm::None;
 }
Index: clangd/GlobalCompilationDatabase.h
===
--- clangd/GlobalCompilationDatabase.h
+++ clangd/GlobalCompilationDatabase.h
@@ -25,6 +25,9 @@
 
 namespace clangd {
 
+/// Returns a default compile command to use for \p File.
+tooling::CompileCommand getDefaultCompileCommand(PathRef File);
+
 /// Provides compilation arguments used for building ClangdUnit.
 class GlobalCompilationDatabase {
 public:
@@ -45,14 +48,19 @@
   std::vector
   getCompileCommands(PathRef File) override;
 
+  void setExtraFlagsForFile(PathRef File, std::vector ExtraFlags);
+
 private:
   tooling::CompilationDatabase 

[PATCH] D34992: Emit static constexpr member as available_externally definition

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Does this apply to all constexpr global variables? It could potentially fix 
https://bugs.llvm.org/show_bug.cgi?id=31860 .


https://reviews.llvm.org/D34992



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

[PATCH] D34170: [libcxx] Moving compiler specific test infrastructure to compiler.py

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

I'll try and get to this tonight.


https://reviews.llvm.org/D34170



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

[PATCH] D34936: [clangd] Add -ffreestanding on VFS tests.

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

Thanks so much!


Repository:
  rL LLVM

https://reviews.llvm.org/D34936



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