[PATCH] D59176: Modules: Add LangOptions::CacheGeneratedPCH

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

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



Comment at: clang/lib/Frontend/FrontendActions.cpp:182
+  CI.getFrontendOpts().BuildingImplicitModule &&
+  CI.getLangOpts().isCompilingModule()));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(

dexonsmith wrote:
> jordan_rose wrote:
> > Why is this the condition, as opposed to just "do this for all modules, 
> > don't do it for PCHs"? And doesn't `BuildingImplicitModule` imply 
> > `isCompilingModule()`? (Note that `BuildingImplicitModule` probably isn't 
> > the same as the original condition `ImplicitModules`.)
> > (Note that BuildingImplicitModule probably isn't the same as the original 
> > condition ImplicitModules.)
> 
> Nice catch; I ported the logic from `ASTWriter` incorrectly.  I'll fix this.
> 
> > Why is this the condition, as opposed to just "do this for all modules, 
> > don't do it for PCHs"? And doesn't BuildingImplicitModule imply 
> > isCompilingModule()?
> 
> I was cargo-culting the logic for how `PCHGenerator::HandleTranslationUnit` 
> sets the `Module` parameter to `ASTWriter::WriteAST`.  I think that if I fix 
> the above to match the original condition I'll need to check 
> `isCompilingModule()`... but maybe `BuildingImplicitModule` is already 
> `&&`-ing these together?  I'll check.
Updated the patch to just use `BuildingImplicitModule`.  The previous condition 
in `PCHGenerator::HandleTranslationUnit` seems to have been equivalent.

- Previously in `PCHGenerator::HandleTranslationUnit`, `WritingModule` would be 
non-null only if we're currently building a module, as opposed to writing out a 
PCH.  The logic to write to the in-memory cache also checked if 
`LangOptions::ImplicitModules` was set.
- Now in `GenerateModuleAction::CreateASTConsumer` we check 
`BuildingImplicitModule` which is set in `compileModuleImpl` before executing 
the action.

I'm choosing this because it better matches the code around.


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

https://reviews.llvm.org/D59176



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

[PATCH] D59176: Modules: Add LangOptions::CacheGeneratedPCH

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

dexonsmith updated this revision to Diff 190203.
dexonsmith added a comment.

Updated the constructor call to `PCHGenerator` in 
`GenerateModuleAction::CreateASTConsumer` to use `BuildingImplicitModule` on 
its own.  Checking where it's set (only in `compileModuleImpl`), it's exactly 
the condition we want here.


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

https://reviews.llvm.org/D59176

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/GeneratePCH.cpp
  clang/unittests/Frontend/FrontendActionTest.cpp

Index: clang/unittests/Frontend/FrontendActionTest.cpp
===
--- clang/unittests/Frontend/FrontendActionTest.cpp
+++ clang/unittests/Frontend/FrontendActionTest.cpp
@@ -6,18 +6,20 @@
 //
 //===--===//
 
+#include "clang/Frontend/FrontendAction.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
-#include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/Sema.h"
+#include "clang/Serialization/InMemoryModuleCache.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/ToolOutputFile.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
@@ -253,4 +255,40 @@
   EXPECT_EQ("This is a note", TDC->Note.str().str());
 }
 
+TEST(GeneratePCHFrontendAction, CacheGeneratedPCH) {
+  // Create a temporary file for writing out the PCH that will be cleaned up.
+  int PCHFD;
+  llvm::SmallString<128> PCHFilename;
+  ASSERT_FALSE(
+  llvm::sys::fs::createTemporaryFile("test.h", "pch", PCHFD, PCHFilename));
+  llvm::ToolOutputFile PCHFile(PCHFilename, PCHFD);
+
+  for (bool ShouldCache : {false, true}) {
+auto Invocation = std::make_shared();
+Invocation->getLangOpts()->CacheGeneratedPCH = ShouldCache;
+Invocation->getPreprocessorOpts().addRemappedFile(
+"test.h",
+MemoryBuffer::getMemBuffer("int foo(void) { return 1; }\n").release());
+Invocation->getFrontendOpts().Inputs.push_back(
+FrontendInputFile("test.h", InputKind::C));
+Invocation->getFrontendOpts().OutputFile = StringRef(PCHFilename);
+Invocation->getFrontendOpts().ProgramAction = frontend::GeneratePCH;
+Invocation->getTargetOpts().Triple = "x86_64-apple-darwin19.0.0";
+CompilerInstance Compiler;
+Compiler.setInvocation(std::move(Invocation));
+Compiler.createDiagnostics();
+
+GeneratePCHAction TestAction;
+ASSERT_TRUE(Compiler.ExecuteAction(TestAction));
+
+// Check whether the PCH was cached.
+if (ShouldCache)
+  EXPECT_EQ(InMemoryModuleCache::Final,
+Compiler.getModuleCache().getPCMState(PCHFilename));
+else
+  EXPECT_EQ(InMemoryModuleCache::Unknown,
+Compiler.getModuleCache().getPCMState(PCHFilename));
+  }
+}
+
 } // anonymous namespace
Index: clang/lib/Serialization/GeneratePCH.cpp
===
--- clang/lib/Serialization/GeneratePCH.cpp
+++ clang/lib/Serialization/GeneratePCH.cpp
@@ -24,12 +24,14 @@
 const Preprocessor , InMemoryModuleCache ,
 StringRef OutputFile, StringRef isysroot, std::shared_ptr Buffer,
 ArrayRef> Extensions,
-bool AllowASTWithErrors, bool IncludeTimestamps)
+bool AllowASTWithErrors, bool IncludeTimestamps,
+bool ShouldCacheASTInMemory)
 : PP(PP), OutputFile(OutputFile), isysroot(isysroot.str()),
   SemaPtr(nullptr), Buffer(std::move(Buffer)), Stream(this->Buffer->Data),
   Writer(Stream, this->Buffer->Data, ModuleCache, Extensions,
  IncludeTimestamps),
-  AllowASTWithErrors(AllowASTWithErrors) {
+  AllowASTWithErrors(AllowASTWithErrors),
+  ShouldCacheASTInMemory(ShouldCacheASTInMemory) {
   this->Buffer->IsComplete = false;
 }
 
@@ -61,7 +63,8 @@
   Writer.WriteAST(*SemaPtr, OutputFile, Module, isysroot,
   // For serialization we are lenient if the errors were
   // only warn-as-error kind.
-  PP.getDiagnostics().hasUncompilableErrorOccurred());
+  PP.getDiagnostics().hasUncompilableErrorOccurred(),
+  ShouldCacheASTInMemory);
 
   Buffer->IsComplete = true;
 }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -4596,7 +4596,8 @@
 ASTFileSignature ASTWriter::WriteAST(Sema ,
  const 

Buildbot numbers for the week of 03/03/2019 - 03/09/2019

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the last week of 03/03/2019 -
03/09/2019.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername   | was_red
-+-
 clang-with-lto-ubuntu   | 27:27:15
 clang-cmake-aarch64-lld | 21:01:04
 clang-cmake-armv8-lld   | 20:03:30
 clang-cmake-aarch64-full| 19:18:13
 clang-lld-x86_64-2stage | 17:38:50
 clang-cmake-aarch64-quick   | 17:36:32
 clang-cmake-aarch64-global-isel | 17:36:03
 lldb-x64-windows-ninja  | 16:43:34
 llvm-clang-x86_64-expensive-checks-win  | 16:17:04
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast| 16:16:51
 clang-cmake-armv7-selfhost-neon | 14:59:02
 clang-x64-windows-msvc  | 14:58:33
 clang-x86_64-linux-selfhost-modules | 13:31:36
 clang-cmake-thumbv7-full-sh | 12:55:57
 clang-cmake-thumbv8-full-sh | 12:12:58
 clang-cmake-armv8-global-isel   | 11:51:11
 clang-cmake-armv7-global-isel   | 10:45:03
 clang-cmake-armv8-full  | 10:35:39
 clang-cmake-armv8-quick | 10:33:28
 clang-cmake-armv8-selfhost-neon | 10:14:53
 sanitizer-x86_64-linux-fast | 09:53:12
 clang-cmake-armv7-full  | 09:30:33
 clang-cmake-armv7-quick | 09:29:34
 sanitizer-x86_64-linux-bootstrap| 07:49:50
 clang-cmake-armv7-selfhost  | 06:43:08
 clang-s390x-linux-multistage| 06:31:14
 netbsd-amd64| 06:24:57
 clang-ppc64le-linux | 05:52:31
 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc49-cxx11| 05:41:16
 clang-ppc64be-linux-multistage  | 05:30:22
 clang-ppc64be-linux-lnt | 05:26:20
 clang-ppc64be-linux | 05:14:47
 clang-s390x-linux   | 05:14:18
 clang-s390x-linux-lnt   | 05:10:56
 clang-ppc64le-linux-lnt | 05:00:22
 ppc64le-lld-multistage-test | 04:51:46
 clang-ppc64le-linux-multistage  | 04:05:50
 sanitizer-x86_64-linux  | 03:46:40
 clang-cuda-build| 03:41:20
 clang-with-thin-lto-ubuntu  | 03:26:23
 sanitizer-x86_64-linux-bootstrap-ubsan  | 03:18:30
 sanitizer-ppc64le-linux | 03:13:50
 clang-hexagon-elf   | 02:40:12
 sanitizer-ppc64be-linux | 02:30:33
 sanitizer-windows   | 02:25:09
 clang-cmake-x86_64-avx2-linux-perf  | 02:22:31
 lld-perf-testsuite  | 02:19:37
 llvm-hexagon-elf| 02:18:11
 sanitizer-x86_64-linux-bootstrap-msan   | 02:08:02
 lld-x86_64-darwin13 | 02:06:07
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast  | 02:00:21
 lld-x86_64-win7 | 01:55:03
 reverse-iteration   | 01:54:36
 lld-x86_64-freebsd  | 01:53:32
 sanitizer-x86_64-linux-android  | 01:44:46
 clang-cmake-x86_64-avx2-linux   | 01:39:02
 clang-cmake-armv8-lnt   | 01:27:01
 clang-x86_64-debian-fast| 01:24:00
 clang-cmake-x86_64-sde-avx512-linux | 01:23:37
 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc-tot-latest-std | 01:08:24
 clang-sphinx-docs   | 00:21:32
 clang-x86_64-linux-abi-test | 00:20:56
 

Buildbot numbers for the week of 02/24/2019 - 03/02/2019

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the week of 02/24/2019 - 03/02/2019.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername   |  was_red
-+--
 reverse-iteration   | 136:36:57
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx14  | 31:09:17
 sanitizer-x86_64-linux  | 31:05:36
 libcxx-libcxxabi-libunwind-x86_64-linux-ubuntu  | 29:20:45
 libcxx-libcxxabi-x86_64-linux-ubuntu-asan   | 28:49:37
 sanitizer-x86_64-linux-android  | 27:32:44
 openmp-clang-ppc64le-linux-rhel | 26:05:04
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx2a  | 24:53:44
 openmp-gcc-x86_64-linux-debian  | 24:21:24
 clang-tools-sphinx-docs | 21:46:25
 netbsd-amd64| 21:05:39
 sanitizer-x86_64-linux-fuzzer   | 20:45:55
 sanitizer-ppc64be-linux | 20:36:23
 sanitizer-x86_64-linux-autoconf | 19:39:04
 clang-ppc64be-linux-multistage  | 19:33:57
 sanitizer-windows   | 19:26:19
 clang-ppc64be-linux-lnt | 19:15:46
 clang-ppc64be-linux | 19:12:04
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx17  | 14:23:52
 clang-lld-x86_64-2stage | 12:50:37
 libcxx-libcxxabi-x86_64-linux-ubuntu-tsan   | 12:47:19
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx11  | 12:43:23
 clang-cmake-thumbv7-full-sh | 12:31:40
 libcxx-libcxxabi-x86_64-linux-ubuntu-msan   | 12:08:33
 sanitizer-x86_64-linux-fast | 11:45:24
 clang-cmake-armv8-lld   | 11:44:21
 clang-armv7-linux-build-cache   | 11:43:00
 llvm-clang-x86_64-expensive-checks-win  | 11:16:11
 libcxx-libcxxabi-x86_64-linux-ubuntu-32bit  | 10:31:15
 clang-cmake-aarch64-lld | 10:31:00
 clang-x86_64-linux-selfhost-modules | 10:22:48
 clang-cmake-armv7-selfhost  | 10:03:43
 libcxx-libcxxabi-x86_64-linux-ubuntu-ubsan  | 09:40:39
 sanitizer-x86_64-linux-bootstrap| 09:33:07
 clang-cmake-aarch64-full| 08:38:20
 sanitizer-ppc64le-linux | 08:34:45
 sanitizer-x86_64-linux-bootstrap-ubsan  | 06:20:45
 clang-ppc64le-linux-multistage  | 06:04:58
 clang-with-thin-lto-ubuntu  | 05:54:43
 lldb-amd64-ninja-netbsd8| 05:28:37
 clang-with-lto-ubuntu   | 05:17:00
 clang-cmake-armv7-full  | 04:21:02
 clang-s390x-linux-multistage| 04:13:20
 clang-cmake-thumbv8-full-sh | 04:00:29
 lld-x86_64-darwin13 | 03:44:33
 lld-perf-testsuite  | 03:39:48
 ppc64le-lld-multistage-test | 03:36:55
 clang-ppc64le-linux-lnt | 03:36:26
 clang-cmake-armv8-full  | 03:34:11
 clang-x64-windows-msvc  | 03:10:09
 clang-cmake-aarch64-quick   | 02:51:13
 clang-ppc64le-linux | 02:49:02
 clang-s390x-linux   | 02:45:44
 sanitizer-x86_64-linux-bootstrap-msan   | 02:38:05
 lldb-x64-windows-ninja  | 02:36:11
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast| 02:22:23
 libcxx-libcxxabi-libunwind-armv8-linux  | 02:07:09
 libcxx-libcxxabi-libunwind-armv8-linux-noexceptions | 02:05:13
 libcxx-libcxxabi-libunwind-aarch64-linux| 02:05:08
 libcxx-libcxxabi-libunwind-aarch64-linux-noexceptions   | 02:02:58
 clang-s390x-linux-lnt   | 01:52:24
 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc-tot-latest-std | 01:47:42
 

Buildbot numbers for the week of 02/17/2019 - 02/23/2019

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the week of 02/17/2019 - 02/23/2019.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername| was_red
--+-
 clang-with-lto-ubuntu| 68:07:41
 sanitizer-windows| 29:35:55
 clang-cmake-aarch64-lld  | 19:55:10
 clang-lld-x86_64-2stage  | 10:46:10
 llvm-clang-x86_64-expensive-checks-win   | 10:16:06
 clang-cmake-thumbv7-full-sh  | 07:59:33
 clang-x86_64-linux-selfhost-modules  | 07:57:04
 lldb-amd64-ninja-netbsd8 | 07:55:21
 sanitizer-x86_64-linux   | 07:23:25
 clang-ppc64le-linux-lnt  | 07:16:52
 clang-with-thin-lto-ubuntu   | 06:38:56
 sanitizer-ppc64le-linux  | 06:27:03
 sanitizer-x86_64-linux-fast  | 06:04:26
 sanitizer-x86_64-linux-autoconf  | 06:01:11
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast | 05:53:20
 sanitizer-x86_64-linux-bootstrap | 05:45:34
 clang-cmake-armv8-lld| 05:16:45
 clang-cmake-aarch64-full | 05:00:28
 sanitizer-ppc64be-linux  | 04:27:20
 clang-ppc64le-linux  | 04:11:25
 clang-cmake-armv8-selfhost-neon  | 04:11:21
 clang-ppc64le-linux-multistage   | 03:52:31
 clang-s390x-linux| 03:51:20
 clang-cmake-armv7-global-isel| 03:48:11
 clang-cmake-thumbv8-full-sh  | 03:44:03
 lldb-x64-windows-ninja   | 03:41:58
 clang-cmake-aarch64-global-isel  | 03:32:57
 clang-cmake-aarch64-quick| 03:21:19
 openmp-gcc-x86_64-linux-debian   | 03:10:00
 openmp-clang-x86_64-linux-debian | 03:09:59
 clang-s390x-linux-lnt| 03:05:28
 clang-x64-windows-msvc   | 03:00:26
 clang-cmake-armv8-global-isel| 02:58:27
 ppc64le-lld-multistage-test  | 02:46:44
 sanitizer-x86_64-linux-android   | 02:41:48
 clang-ppc64be-linux-lnt  | 02:40:25
 clang-ppc64be-linux-multistage   | 02:39:21
 clang-atom-d525-fedora-rel   | 02:38:48
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast   | 02:30:10
 clang-cmake-armv8-full   | 02:28:53
 clang-cmake-armv8-lnt| 02:25:22
 clang-ppc64be-linux  | 02:24:04
 clang-x86_64-debian-fast | 02:24:00
 clang-cuda-build | 02:21:57
 sanitizer-x86_64-linux-bootstrap-ubsan   | 02:20:29
 lld-x86_64-darwin13  | 02:11:45
 clang-cmake-x86_64-avx2-linux| 02:05:15
 clang-cmake-x86_64-avx2-linux-perf   | 02:01:50
 clang-s390x-linux-multistage | 02:00:48
 llvm-hexagon-elf | 01:59:46
 clang-cmake-armv8-quick  | 01:59:43
 clang-cmake-armv7-quick  | 01:58:26
 clang-tools-sphinx-docs  | 01:53:22
 polly-amd64-linux| 01:39:58
 polly-arm-linux  | 01:39:17
 clang-cmake-x86_64-sde-avx512-linux  | 01:36:26
 clang-cmake-armv7-full   | 01:34:17
 clang-cmake-armv7-lnt| 01:33:14
 clang-aarch64-linux-build-cache  | 01:30:23
 clang-armv7-linux-build-cache| 01:29:11
 lld-sphinx-docs  | 01:22:25
 clang-hexagon-elf| 01:13:45
 sanitizer-x86_64-linux-bootstrap-msan| 01:02:21
 lld-perf-testsuite   | 00:40:31
 clang-x86_64-linux-abi-test  | 00:29:24
 lld-x86_64-win7  | 00:22:51
 llvm-sphinx-docs | 00:11:54
(67 rows)


"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green):
   

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

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355891: [Driver] Support object files in addition to static 
and shared libraries in… (authored by phosek, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56044?vs=189552=190198#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56044

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

Index: cfe/trunk/include/clang/Driver/ToolChain.h
===
--- cfe/trunk/include/clang/Driver/ToolChain.h
+++ cfe/trunk/include/clang/Driver/ToolChain.h
@@ -104,6 +104,8 @@
 RM_Disabled,
   };
 
+  enum FileType { FT_Object, FT_Static, FT_Shared };
+
 private:
   friend class RegisterEffectiveTriple;
 
@@ -371,11 +373,11 @@
 
   virtual std::string getCompilerRT(const llvm::opt::ArgList ,
 StringRef Component,
-bool Shared = false) const;
+FileType Type = ToolChain::FT_Static) const;
 
-  const char *getCompilerRTArgString(const llvm::opt::ArgList ,
- StringRef Component,
- bool Shared = false) const;
+  const char *
+  getCompilerRTArgString(const llvm::opt::ArgList , StringRef Component,
+ FileType Type = ToolChain::FT_Static) const;
 
   // Returns /lib//.  This is used by runtimes (such
   // as OpenMP) to find arch-specific libraries.
Index: cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
+++ cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
@@ -248,8 +248,8 @@
 
   if (Sanitize.needsAsanRt()) {
 // MinGW always links against a shared MSVCRT.
-CmdArgs.push_back(
-TC.getCompilerRTArgString(Args, "asan_dynamic", true));
+CmdArgs.push_back(TC.getCompilerRTArgString(Args, "asan_dynamic",
+ToolChain::FT_Shared));
 CmdArgs.push_back(
 TC.getCompilerRTArgString(Args, "asan_dynamic_runtime_thunk"));
 CmdArgs.push_back(Args.MakeArgString("--require-defined"));
Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -535,7 +535,8 @@
   // Wrap any static runtimes that must be forced into executable in
   // whole-archive.
   if (IsWhole) CmdArgs.push_back("--whole-archive");
-  CmdArgs.push_back(TC.getCompilerRTArgString(Args, Sanitizer, IsShared));
+  CmdArgs.push_back(TC.getCompilerRTArgString(
+  Args, Sanitizer, IsShared ? ToolChain::FT_Shared : ToolChain::FT_Static));
   if (IsWhole) CmdArgs.push_back("--no-whole-archive");
 
   if (IsShared) {
Index: cfe/trunk/lib/Driver/ToolChains/MipsLinux.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/MipsLinux.cpp
+++ cfe/trunk/lib/Driver/ToolChains/MipsLinux.cpp
@@ -118,11 +118,23 @@
 
 std::string MipsLLVMToolChain::getCompilerRT(const ArgList ,
  StringRef Component,
- bool Shared) const {
+ FileType Type) const {
   SmallString<128> Path(getDriver().ResourceDir);
   llvm::sys::path::append(Path, SelectedMultilib.osSuffix(), "lib" + LibSuffix,
   getOS());
-  llvm::sys::path::append(Path, Twine("libclang_rt." + Component + "-" +
-  "mips" + (Shared ? ".so" : ".a")));
+  const char *Suffix;
+  switch (Type) {
+  case ToolChain::FT_Object:
+Suffix = ".o";
+break;
+  case ToolChain::FT_Static:
+Suffix = ".a";
+break;
+  case ToolChain::FT_Shared:
+Suffix = ".so";
+break;
+  }
+  llvm::sys::path::append(
+  Path, Twine("libclang_rt." + Component + "-" + "mips" + Suffix));
   return Path.str();
 }
Index: cfe/trunk/lib/Driver/ToolChains/MipsLinux.h
===
--- cfe/trunk/lib/Driver/ToolChains/MipsLinux.h
+++ cfe/trunk/lib/Driver/ToolChains/MipsLinux.h
@@ -37,8 +37,9 @@
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
 
-  std::string getCompilerRT(const llvm::opt::ArgList , StringRef Component,
-bool Shared = false) const override;

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

[Petr Hosek via cfe-commits]

Author: phosek
Date: Mon Mar 11 19:12:48 2019
New Revision: 355891

URL: http://llvm.org/viewvc/llvm-project?rev=355891=rev
Log:
[Driver] Support object files in addition to static and shared libraries in 
compiler-rt

This change introduces support for object files in addition to static
and shared libraries which were already supported which requires
changing the type of the argument from boolean to an enum.

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

Modified:
cfe/trunk/include/clang/Driver/ToolChain.h
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
cfe/trunk/lib/Driver/ToolChains/MipsLinux.cpp
cfe/trunk/lib/Driver/ToolChains/MipsLinux.h

Modified: cfe/trunk/include/clang/Driver/ToolChain.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/ToolChain.h?rev=355891=355890=355891=diff
==
--- cfe/trunk/include/clang/Driver/ToolChain.h (original)
+++ cfe/trunk/include/clang/Driver/ToolChain.h Mon Mar 11 19:12:48 2019
@@ -104,6 +104,8 @@ public:
 RM_Disabled,
   };
 
+  enum FileType { FT_Object, FT_Static, FT_Shared };
+
 private:
   friend class RegisterEffectiveTriple;
 
@@ -371,11 +373,11 @@ public:
 
   virtual std::string getCompilerRT(const llvm::opt::ArgList ,
 StringRef Component,
-bool Shared = false) const;
+FileType Type = ToolChain::FT_Static) 
const;
 
-  const char *getCompilerRTArgString(const llvm::opt::ArgList ,
- StringRef Component,
- bool Shared = false) const;
+  const char *
+  getCompilerRTArgString(const llvm::opt::ArgList , StringRef Component,
+ FileType Type = ToolChain::FT_Static) const;
 
   // Returns /lib//.  This is used by runtimes (such
   // as OpenMP) to find arch-specific libraries.

Modified: cfe/trunk/lib/Driver/ToolChain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=355891=355890=355891=diff
==
--- cfe/trunk/lib/Driver/ToolChain.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChain.cpp Mon Mar 11 19:12:48 2019
@@ -362,16 +362,27 @@ std::string ToolChain::getCompilerRTPath
 }
 
 std::string ToolChain::getCompilerRT(const ArgList , StringRef Component,
- bool Shared) const {
+ FileType Type) const {
   const llvm::Triple  = getTriple();
   bool IsITANMSVCWindows =
   TT.isWindowsMSVCEnvironment() || TT.isWindowsItaniumEnvironment();
 
-  const char *Prefix = IsITANMSVCWindows ? "" : "lib";
-  const char *Suffix = Shared ? (Triple.isOSWindows() ? ".lib" : ".so")
-  : (IsITANMSVCWindows ? ".lib" : ".a");
-  if (Shared && Triple.isWindowsGNUEnvironment())
-Suffix = ".dll.a";
+  const char *Prefix =
+  IsITANMSVCWindows || Type == ToolChain::FT_Object ? "" : "lib";
+  const char *Suffix;
+  switch (Type) {
+  case ToolChain::FT_Object:
+Suffix = IsITANMSVCWindows ? ".obj" : ".o";
+break;
+  case ToolChain::FT_Static:
+Suffix = IsITANMSVCWindows ? ".lib" : ".a";
+break;
+  case ToolChain::FT_Shared:
+Suffix = Triple.isOSWindows()
+ ? (Triple.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
+ : ".so";
+break;
+  }
 
   for (const auto  : getLibraryPaths()) {
 SmallString<128> P(LibPath);
@@ -390,8 +401,8 @@ std::string ToolChain::getCompilerRT(con
 
 const char *ToolChain::getCompilerRTArgString(const llvm::opt::ArgList ,
   StringRef Component,
-  bool Shared) const {
-  return Args.MakeArgString(getCompilerRT(Args, Component, Shared));
+  FileType Type) const {
+  return Args.MakeArgString(getCompilerRT(Args, Component, Type));
 }
 
 std::string ToolChain::getArchSpecificLibPath() const {

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=355891=355890=355891=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Mon Mar 11 19:12:48 2019
@@ -535,7 +535,8 @@ static void addSanitizerRuntime(const To
   // Wrap any static runtimes that must be forced into executable in
   // whole-archive.
   if (IsWhole) CmdArgs.push_back("--whole-archive");
-  CmdArgs.push_back(TC.getCompilerRTArgString(Args, Sanitizer, IsShared));
+  CmdArgs.push_back(TC.getCompilerRTArgString(
+  Args, Sanitizer, IsShared ? ToolChain::FT_Shared : 

[PATCH] D59010: [DebugInfo] Add test cases for FlagNonTrivial

[Aaron Smith via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC355890: [DebugInfo] Add test cases for FlagNonTrivial 
(authored by asmith, committed by ).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

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

https://reviews.llvm.org/D59010

Files:
  test/CodeGenCXX/debug-info-composite-triviality.cpp


Index: test/CodeGenCXX/debug-info-composite-triviality.cpp
===
--- test/CodeGenCXX/debug-info-composite-triviality.cpp
+++ test/CodeGenCXX/debug-info-composite-triviality.cpp
@@ -0,0 +1,90 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -triple 
%itanium_abi_triple %s -o - | FileCheck %s
+
+// Cases to show some non-trivial types with flags combined with 
DIFlagNonTrivial and DIFlagTypePassByValue.
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Explicit",{{.*}}flags: 
DIFlagTypePassByValue | DIFlagNonTrivial
+struct Explicit {
+  explicit Explicit();
+  int a;
+} Explicit;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Struct",{{.*}}flags: 
DIFlagTypePassByValue | DIFlagNonTrivial
+struct Struct {
+  Struct() {}
+} Struct;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Annotated",{{.*}}flags: 
DIFlagTypePassByValue | DIFlagNonTrivial
+struct __attribute__((trivial_abi)) Annotated {
+  Annotated() {};
+} Annotated;
+
+
+// Check a non-composite type
+// CHECK-DAG: !DIGlobalVariable(name: "GlobalVar", {{.*}}type: {{.*}}, 
isLocal: false, isDefinition: true)
+int GlobalVar = 0;
+
+// Cases to test composite type's triviality
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Union",{{.*}}flags: 
{{.*}}DIFlagTrivial
+union Union {
+  int a;
+} Union;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Trivial",{{.*}}flags: 
{{.*}}DIFlagTrivial
+struct Trivial {
+  int i;
+} Trivial;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialA",{{.*}}flags: 
{{.*}}DIFlagTrivial
+struct TrivialA {
+  TrivialA() = default;
+} TrivialA;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialB",{{.*}}flags: 
{{.*}}DIFlagTrivial
+struct TrivialB {
+  int m;
+  TrivialB(int x) { m = x; }
+  TrivialB() = default;
+} TrivialB;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialC",{{.*}}flags: 
{{.*}}DIFlagTrivial
+struct TrivialC {
+  struct Trivial x;
+} TrivialC;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialD",{{.*}}flags: 
{{.*}}DIFlagTrivial
+struct NT {
+  NT() {};
+};
+struct TrivialD {
+  static struct NT x; // Member is non-trivial but is static.
+} TrivialD;
+
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivial",{{.*}}flags: 
{{.*}}DIFlagNonTrivial
+struct NonTrivial {
+  NonTrivial() {}
+} NonTrivial;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivialA",{{.*}}flags: 
{{.*}}DIFlagNonTrivial
+struct NonTrivialA {
+  ~NonTrivialA();
+} NonTrivialA;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivialB",{{.*}}flags: 
{{.*}}DIFlagNonTrivial
+struct NonTrivialB {
+  struct NonTrivial x;
+} NonTrivialB;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivialC",{{.*}}flags: 
{{.*}}DIFlagNonTrivial
+struct NonTrivialC {
+  virtual void f() {}
+} NonTrivialC;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivialD",{{.*}}flags: 
{{.*}}DIFlagNonTrivial
+struct NonTrivialD : NonTrivial {
+} NonTrivialD;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivialE",{{.*}}flags: 
{{.*}}DIFlagNonTrivial
+struct NonTrivialE : Trivial, NonTrivial {
+} NonTrivialE;


Index: test/CodeGenCXX/debug-info-composite-triviality.cpp
===
--- test/CodeGenCXX/debug-info-composite-triviality.cpp
+++ test/CodeGenCXX/debug-info-composite-triviality.cpp
@@ -0,0 +1,90 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -triple %itanium_abi_triple %s -o - | FileCheck %s
+
+// Cases to show some non-trivial types with flags combined with DIFlagNonTrivial and DIFlagTypePassByValue.
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Explicit",{{.*}}flags: DIFlagTypePassByValue | DIFlagNonTrivial
+struct Explicit {
+  explicit Explicit();
+  int a;
+} Explicit;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Struct",{{.*}}flags: DIFlagTypePassByValue | DIFlagNonTrivial
+struct Struct {
+  Struct() {}
+} Struct;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Annotated",{{.*}}flags: DIFlagTypePassByValue | DIFlagNonTrivial
+struct __attribute__((trivial_abi)) Annotated {
+  Annotated() {};
+} Annotated;
+
+
+// Check a non-composite type
+// CHECK-DAG: !DIGlobalVariable(name: "GlobalVar", {{.*}}type: {{.*}}, isLocal: false, isDefinition: true)
+int GlobalVar = 0;
+
+// Cases to test composite type's triviality
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Union",{{.*}}flags: {{.*}}DIFlagTrivial
+union Union {
+  int a;
+} Union;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Trivial",{{.*}}flags: 

r355890 - [DebugInfo] Add test cases for FlagNonTrivial

[Aaron Smith via cfe-commits]

Author: asmith
Date: Mon Mar 11 19:00:39 2019
New Revision: 355890

URL: http://llvm.org/viewvc/llvm-project?rev=355890=rev
Log:
[DebugInfo] Add test cases for FlagNonTrivial

Summary:
This is a test case to go with D44406 which added FlagNonTrivial to mark that a 
C++ record is non-trivial to support CodeView debug emission. 

While it looks like FlagTypePassByValue can imply triviality and 
FlagTypePassByReference can imply non-triviality that is not true. Some 
non-trivial cases use a combination of FlagNonTrivial and FlagTypePassByValue 
instead of FlagTypePassByReference. See the test cases and D44406 for 
discussion.

Reviewers: dblaikie, rnk, zturner

Reviewed By: dblaikie

Subscribers: jdoerfert, llvm-commits

Tags: #llvm

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

Added:
cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp

Added: cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp?rev=355890=auto
==
--- cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/debug-info-composite-triviality.cpp Mon Mar 11 
19:00:39 2019
@@ -0,0 +1,90 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -triple 
%itanium_abi_triple %s -o - | FileCheck %s
+
+// Cases to show some non-trivial types with flags combined with 
DIFlagNonTrivial and DIFlagTypePassByValue.
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Explicit",{{.*}}flags: 
DIFlagTypePassByValue | DIFlagNonTrivial
+struct Explicit {
+  explicit Explicit();
+  int a;
+} Explicit;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Struct",{{.*}}flags: 
DIFlagTypePassByValue | DIFlagNonTrivial
+struct Struct {
+  Struct() {}
+} Struct;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Annotated",{{.*}}flags: 
DIFlagTypePassByValue | DIFlagNonTrivial
+struct __attribute__((trivial_abi)) Annotated {
+  Annotated() {};
+} Annotated;
+
+
+// Check a non-composite type
+// CHECK-DAG: !DIGlobalVariable(name: "GlobalVar", {{.*}}type: {{.*}}, 
isLocal: false, isDefinition: true)
+int GlobalVar = 0;
+
+// Cases to test composite type's triviality
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Union",{{.*}}flags: 
{{.*}}DIFlagTrivial
+union Union {
+  int a;
+} Union;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "Trivial",{{.*}}flags: 
{{.*}}DIFlagTrivial
+struct Trivial {
+  int i;
+} Trivial;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialA",{{.*}}flags: 
{{.*}}DIFlagTrivial
+struct TrivialA {
+  TrivialA() = default;
+} TrivialA;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialB",{{.*}}flags: 
{{.*}}DIFlagTrivial
+struct TrivialB {
+  int m;
+  TrivialB(int x) { m = x; }
+  TrivialB() = default;
+} TrivialB;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialC",{{.*}}flags: 
{{.*}}DIFlagTrivial
+struct TrivialC {
+  struct Trivial x;
+} TrivialC;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialD",{{.*}}flags: 
{{.*}}DIFlagTrivial
+struct NT {
+  NT() {};
+};
+struct TrivialD {
+  static struct NT x; // Member is non-trivial but is static.
+} TrivialD;
+
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivial",{{.*}}flags: 
{{.*}}DIFlagNonTrivial
+struct NonTrivial {
+  NonTrivial() {}
+} NonTrivial;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivialA",{{.*}}flags: 
{{.*}}DIFlagNonTrivial
+struct NonTrivialA {
+  ~NonTrivialA();
+} NonTrivialA;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivialB",{{.*}}flags: 
{{.*}}DIFlagNonTrivial
+struct NonTrivialB {
+  struct NonTrivial x;
+} NonTrivialB;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivialC",{{.*}}flags: 
{{.*}}DIFlagNonTrivial
+struct NonTrivialC {
+  virtual void f() {}
+} NonTrivialC;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivialD",{{.*}}flags: 
{{.*}}DIFlagNonTrivial
+struct NonTrivialD : NonTrivial {
+} NonTrivialD;
+
+// CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivialE",{{.*}}flags: 
{{.*}}DIFlagNonTrivial
+struct NonTrivialE : Trivial, NonTrivial {
+} NonTrivialE;


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

[PATCH] D59118: creduce script for clang crashes

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

(A few python style comments; feel free to ignore, and feel free to land no 
matter what you do with the comments.)




Comment at: clang/utils/creduce-clang-crash.py:1
+#!/usr/bin/env python
+# A tool that calls C-Reduce to create a minimal reproducer for clang crashes

rnk wrote:
> george.burgess.iv wrote:
> > nit: please specify a python version here, since the world is steadily 
> > making `python` == `python3` (if `pipes.quote` is working, I assume you'll 
> > want `#!/usr/bin/env python2`)
> Please don't do that, there is no python2.exe or python3.exe on Windows. 
> `#!/usr/bin/env python` is the simplest thing that could work.
There's no python2 on mac either. `#!/usr/bin/env python` is the correct first 
line for executable python scripts.



Comment at: clang/utils/creduce-clang-crash.py:29
+return exe_file
+  return None
+

There's `distutils.spawn.find_executable()` which does this for you.



Comment at: clang/utils/creduce-clang-crash.py:68
+def main():
+  parser = ArgumentParser(description='Runs C-Reduce on the input file')
+  parser.add_argument('build_script', type=str, nargs=1,

It's a somewhat common pattern to put a module docstring at the top of the file 
instead of a comment which explains usage, and then say `description=__doc__` 
here; see llvm/utils/gn/build/symlink_or_copy.py for an example.



Comment at: clang/utils/creduce-clang-crash.py:90
+print(("ERROR: input file '%s' does not exist") % build_script)
+sys.exit(1)
+

It's a bit more common to say `return 1` here and do `sys.exit(main())` at the 
bottom.


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

https://reviews.llvm.org/D59118



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

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Nico Weber via Phabricator via cfe-commits]

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



Comment at: clang/test/Parser/objc-static-assert.mm:1
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+

aaron.ballman wrote:
> erik.pilkington wrote:
> > thakis wrote:
> > > aaron.ballman wrote:
> > > > thakis wrote:
> > > > > aaron.ballman wrote:
> > > > > > Can you try explicitly specifying C++98 as the underlying language 
> > > > > > standard mode? I feel like `_Static_assert()` will continue to work 
> > > > > > there (because we made it a language extension in all modes) but 
> > > > > > `static_assert()` may fail (because that's gated on C++11 support). 
> > > > > > If that turns out to be the case, then I think `objc_static_assert` 
> > > > > > should be more complex than expanding to `true`, or we should talk 
> > > > > > about supporting `static_assert()` in all C++ language modes like 
> > > > > > we do for `_Static_assert()`.
> > > > > Correct, with -std=c++98 static_assert isn't available but 
> > > > > _Static_assert still is. If you want, I can add a test for this, but 
> > > > > this is covered by regular c++ tests already.
> > > > > 
> > > > > I think the has_feature() should stay as-is though: Else you have no 
> > > > > way to know if _Static_assert works in obj-c mode, and you can check 
> > > > > if static_assert works by checkout has_feature && __cplusplus >= 
> > > > > 201103L if you still care about c++98.
> > > > > 
> > > > > (And adding one feature each for static_assert and _Static_assert 
> > > > > seems like overkill.)
> > > > > Correct, with -std=c++98 static_assert isn't available but 
> > > > > _Static_assert still is. If you want, I can add a test for this, but 
> > > > > this is covered by regular c++ tests already.
> > > > 
> > > > Please do (if we don't change the availability of `static_assert()`), 
> > > > because the test currently relies on the unwritten -std option in order 
> > > > to pass.
> > > > 
> > > > > I think the has_feature() should stay as-is though: Else you have no 
> > > > > way to know if _Static_assert works in obj-c mode, and you can check 
> > > > > if static_assert works by checkout has_feature && __cplusplus >= 
> > > > > 201103L if you still care about c++98.
> > > > 
> > > > I don't think this is the correct approach. Testing for 
> > > > `static_assert()` support should not leave the user guessing at what 
> > > > the correct spelling is.
> > > > 
> > > > > (And adding one feature each for static_assert and _Static_assert 
> > > > > seems like overkill.)
> > > > 
> > > > Definitely agreed there.
> > > > 
> > > > I think the correct way forward is to support `static_assert()` in all 
> > > > language modes like we already do for `_Static_assert()`, then 
> > > > `objc_static_assert` becomes useful as-is. I cannot think of a reason 
> > > > why we would want to allow `_Static_assert()` in C++ but not allow 
> > > > `static_assert()`.
> > > I updated the test.
> > > 
> > > Accepting `static_assert()` independent of language mode seems pretty 
> > > unrelated to this patch here, so I don't want to do this.
> > > 
> > > If you don't like the current has_feature approach, I'm all ears for 
> > > other approaches. The current approach allows you to detect if clang can 
> > > handle static_assert in objc objects, and codebases that still care about 
> > > c++98 will have a static_assert wrapping macro keyed off __cplusplus 
> > > already, so that part will transparently just work as well. And codebases 
> > > that are c++11 and newer are in a good position too. I think the current 
> > > setup is pretty good. (But I'm happy to hear better suggestions.)
> > > Accepting static_assert() independent of language mode seems pretty 
> > > unrelated to this patch here, so I don't want to do this.
> > 
> > Yeah, we shouldn't be treating `static_assert` as a keyword in C++98 or C, 
> > I think. It would break code.
> > 
> > > If you don't like the current has_feature approach, I'm all ears for 
> > > other approaches. The current approach allows you to detect if clang can 
> > > handle static_assert in objc objects, and codebases that still care about 
> > > c++98 will have a static_assert wrapping macro keyed off __cplusplus 
> > > already, so that part will transparently just work as well. And codebases 
> > > that are c++11 and newer are in a good position too. I think the current 
> > > setup is pretty good. (But I'm happy to hear better suggestions.)
> > 
> > This is pretty weird. This feature flag doesn't actually correspond to any 
> > feature, just the possibility of the existence of a feature (there isn't 
> > any way you could use this `__has_feature` portably without also including 
> > another `__has_feature` check). I think the most internally consistent way 
> > of doing this is to have two flags, as you mentioned above, 
> > `objc_c_static_assert` and `objc_cxx_static_assert`.
> > 
> > Just to keep the bikeshed going, 

[PATCH] D59118: creduce script for clang crashes

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 190189.
akhuang added a comment.

add interestingness test sanity check;
revive ctrl-c hack


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

https://reviews.llvm.org/D59118

Files:
  clang/utils/creduce-clang-crash.py

Index: clang/utils/creduce-clang-crash.py
===
--- /dev/null
+++ clang/utils/creduce-clang-crash.py
@@ -0,0 +1,131 @@
+#!/usr/bin/env python
+# A tool that calls C-Reduce to create a minimal reproducer for clang crashes
+# Requires C-Reduce and not (part of LLVM utils) to be installed
+
+from argparse import ArgumentParser
+import os
+import re
+import stat
+import sys
+import subprocess
+import pipes
+
+def is_exe(fpath):
+  return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
+
+def which(program):
+  """
+  Return the full path to a program or return None.
+  """
+  fpath, fname = os.path.split(program)
+  if fpath:
+if is_exe(program):
+  return program
+  else:
+for path in os.environ["PATH"].split(os.pathsep):
+  exe_file = os.path.join(path, program)
+  if is_exe(exe_file):
+return exe_file
+  return None
+
+def create_test(build_script, llvm_not):
+  """
+  Create an interestingness test from the crash output.
+  Return as a string.
+  """
+  # Get clang call from build script
+  # Assumes the call is the last line of the script
+  with open(build_script) as f:
+cmd = f.readlines()[-1].rstrip('\n\r')
+
+  # Get crash output
+  p = subprocess.Popen(build_script,
+   stdout=subprocess.PIPE,
+   stderr=subprocess.STDOUT)
+  crash_output, _ = p.communicate()
+
+  output = ['#!/bin/bash']
+  output.append('%s --crash %s >& t.log || exit 1' % (pipes.quote(llvm_not),
+  cmd))
+
+  # Add messages from crash output to the test
+  # If there is an Assertion failure, use that; otherwise use the
+  # last five stack trace functions
+  assertion_re = r'Assertion `([^\']+)\' failed'
+  assertion_match = re.search(assertion_re, crash_output)
+  if assertion_match:
+msg = assertion_match.group(1)
+output.append('grep %s t.log || exit 1' % pipes.quote(msg))
+  else:
+stacktrace_re = r'#[0-9]+\s+0[xX][0-9a-fA-F]+\s*([^(]+)\('
+matches = re.findall(stacktrace_re, crash_output)
+del matches[:-5]
+output += ['grep %s t.log || exit 1' % pipes.quote(msg) for msg in matches]
+
+  return output
+
+def main():
+  parser = ArgumentParser(description='Runs C-Reduce on the input file')
+  parser.add_argument('build_script', type=str, nargs=1,
+  help='Name of the script that generates the crash.')
+  parser.add_argument('file_to_reduce', type=str, nargs=1,
+  help='Name of the file to be reduced.')
+  parser.add_argument('-o', '--output', dest='output', type=str,
+  help='Name of the output file for the reduction. Optional.')
+  parser.add_argument('--llvm-not', dest='llvm_not', type=str,
+  help="The path to the llvm-not executable. "
+  "Required if 'not' is not in PATH environment.");
+  parser.add_argument('--creduce', dest='creduce', type=str,
+  help="The path to the C-Reduce executable. "
+  "Required if 'creduce' is not in PATH environment.");
+  args = parser.parse_args()
+
+  build_script = os.path.abspath(args.build_script[0])
+  file_to_reduce = os.path.abspath(args.file_to_reduce[0])
+  llvm_not = which(args.llvm_not) if args.llvm_not else which('not')
+  creduce = which(args.creduce) if args.creduce else which('creduce')
+
+  if not os.path.isfile(build_script):
+print(("ERROR: input file '%s' does not exist") % build_script)
+sys.exit(1)
+
+  if not os.path.isfile(file_to_reduce):
+print(("ERROR: input file '%s' does not exist") % file_to_reduce)
+sys.exit(1)
+
+  if not llvm_not:
+parser.print_help()
+sys.exit(1)
+
+  if not creduce:
+parser.print_help()
+sys.exit(1)
+
+  # Write interestingness test to file
+  test_contents = create_test(build_script, llvm_not)
+  testname, _ = os.path.splitext(file_to_reduce)
+  testfile = testname + '.test.sh'
+  with open(testfile, 'w') as f:
+f.write('\n'.join(test_contents))
+  os.chmod(testfile, os.stat(testfile).st_mode | stat.S_IEXEC)
+
+  # Confirm that the interestingness test passes
+  try:
+with open(os.devnull, 'w') as devnull:
+  subprocess.check_call(testfile, stdout=devnull)
+  except subprocess.CalledProcessError:
+print("For some reason the interestingness test does not return zero")
+sys.exit(1)
+
+  # FIXME: try running clang preprocessor first
+
+  try:
+p = subprocess.Popen([creduce, testfile, file_to_reduce])
+p.communicate()
+  except KeyboardInterrupt:
+# Hack to kill C-Reduce because it jumps into its own pgid
+print('\n\nctrl-c detected, killed creduce')
+

[PATCH] D59118: creduce script for clang crashes

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/utils/creduce-clang-crash.py:1
+#!/usr/bin/env python
+# A tool that calls C-Reduce to create a minimal reproducer for clang crashes

george.burgess.iv wrote:
> nit: please specify a python version here, since the world is steadily making 
> `python` == `python3` (if `pipes.quote` is working, I assume you'll want 
> `#!/usr/bin/env python2`)
Please don't do that, there is no python2.exe or python3.exe on Windows. 
`#!/usr/bin/env python` is the simplest thing that could work.


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

https://reviews.llvm.org/D59118



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

[PATCH] D59118: creduce script for clang crashes

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

> I think we should do one more round of fixes, we can commit that for you, and 
> then move on to the next steps

+1. This looks good to me with outstanding nits fixed

> and filter out the # lines afterwards.

AIUI, the very-recently-merged 
https://github.com/csmith-project/creduce/pull/183 and 
https://github.com/csmith-project/creduce/pull/188 should hopefully handle the 
majority of those?




Comment at: clang/utils/creduce-clang-crash.py:1
+#!/usr/bin/env python
+# A tool that calls C-Reduce to create a minimal reproducer for clang crashes

nit: please specify a python version here, since the world is steadily making 
`python` == `python3` (if `pipes.quote` is working, I assume you'll want 
`#!/usr/bin/env python2`)


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

https://reviews.llvm.org/D59118



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

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: rsmith, aaron.ballman.
aaron.ballman added inline comments.



Comment at: clang/test/Parser/objc-static-assert.mm:1
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+

erik.pilkington wrote:
> thakis wrote:
> > aaron.ballman wrote:
> > > thakis wrote:
> > > > aaron.ballman wrote:
> > > > > Can you try explicitly specifying C++98 as the underlying language 
> > > > > standard mode? I feel like `_Static_assert()` will continue to work 
> > > > > there (because we made it a language extension in all modes) but 
> > > > > `static_assert()` may fail (because that's gated on C++11 support). 
> > > > > If that turns out to be the case, then I think `objc_static_assert` 
> > > > > should be more complex than expanding to `true`, or we should talk 
> > > > > about supporting `static_assert()` in all C++ language modes like we 
> > > > > do for `_Static_assert()`.
> > > > Correct, with -std=c++98 static_assert isn't available but 
> > > > _Static_assert still is. If you want, I can add a test for this, but 
> > > > this is covered by regular c++ tests already.
> > > > 
> > > > I think the has_feature() should stay as-is though: Else you have no 
> > > > way to know if _Static_assert works in obj-c mode, and you can check if 
> > > > static_assert works by checkout has_feature && __cplusplus >= 201103L 
> > > > if you still care about c++98.
> > > > 
> > > > (And adding one feature each for static_assert and _Static_assert seems 
> > > > like overkill.)
> > > > Correct, with -std=c++98 static_assert isn't available but 
> > > > _Static_assert still is. If you want, I can add a test for this, but 
> > > > this is covered by regular c++ tests already.
> > > 
> > > Please do (if we don't change the availability of `static_assert()`), 
> > > because the test currently relies on the unwritten -std option in order 
> > > to pass.
> > > 
> > > > I think the has_feature() should stay as-is though: Else you have no 
> > > > way to know if _Static_assert works in obj-c mode, and you can check if 
> > > > static_assert works by checkout has_feature && __cplusplus >= 201103L 
> > > > if you still care about c++98.
> > > 
> > > I don't think this is the correct approach. Testing for `static_assert()` 
> > > support should not leave the user guessing at what the correct spelling 
> > > is.
> > > 
> > > > (And adding one feature each for static_assert and _Static_assert seems 
> > > > like overkill.)
> > > 
> > > Definitely agreed there.
> > > 
> > > I think the correct way forward is to support `static_assert()` in all 
> > > language modes like we already do for `_Static_assert()`, then 
> > > `objc_static_assert` becomes useful as-is. I cannot think of a reason why 
> > > we would want to allow `_Static_assert()` in C++ but not allow 
> > > `static_assert()`.
> > I updated the test.
> > 
> > Accepting `static_assert()` independent of language mode seems pretty 
> > unrelated to this patch here, so I don't want to do this.
> > 
> > If you don't like the current has_feature approach, I'm all ears for other 
> > approaches. The current approach allows you to detect if clang can handle 
> > static_assert in objc objects, and codebases that still care about c++98 
> > will have a static_assert wrapping macro keyed off __cplusplus already, so 
> > that part will transparently just work as well. And codebases that are 
> > c++11 and newer are in a good position too. I think the current setup is 
> > pretty good. (But I'm happy to hear better suggestions.)
> > Accepting static_assert() independent of language mode seems pretty 
> > unrelated to this patch here, so I don't want to do this.
> 
> Yeah, we shouldn't be treating `static_assert` as a keyword in C++98 or C, I 
> think. It would break code.
> 
> > If you don't like the current has_feature approach, I'm all ears for other 
> > approaches. The current approach allows you to detect if clang can handle 
> > static_assert in objc objects, and codebases that still care about c++98 
> > will have a static_assert wrapping macro keyed off __cplusplus already, so 
> > that part will transparently just work as well. And codebases that are 
> > c++11 and newer are in a good position too. I think the current setup is 
> > pretty good. (But I'm happy to hear better suggestions.)
> 
> This is pretty weird. This feature flag doesn't actually correspond to any 
> feature, just the possibility of the existence of a feature (there isn't any 
> way you could use this `__has_feature` portably without also including 
> another `__has_feature` check). I think the most internally consistent way of 
> doing this is to have two flags, as you mentioned above, 
> `objc_c_static_assert` and `objc_cxx_static_assert`.
> 
> Just to keep the bikeshed going, maybe it should be spelt 
> `objc_interface_c_static_assert` or something, to show that it doesn't 
> control static_assert in ObjC, but static_assert in interfaces in ObjC.
> 

[PATCH] D59233: libclang/CIndexer.cpp: Use loadquery() on AIX for path to library

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

I believe that the conditions being checked for with `llvm_unreachable` in this 
patch are of the debug-only variety; however, some input would be appreciated 
regarding the choice of using `llvm_unreachable` instead of 
`report_fatal_error` or `assert`.




Comment at: tools/libclang/CIndexer.cpp:61
+if (errno != ENOMEM)
+  llvm_unreachable("Encountered an unexpected loadquery() failure");
+

Based on available documentation, this situation should not occur (and thus I 
believe `llvm_unreachable` is appropriate).



Comment at: tools/libclang/CIndexer.cpp:65
+if ((BufSize & ~((-1u) >> 1u)) != 0u)
+  llvm_unreachable("BufSize needed for loadquery() too large");
+

This situation is not impossible, but highly improbable. This is a 
non-programmatic error, and the Programmer's Manual appears to recommend the 
use of `report_fatal_error`. Some additional guidance would be appreciated.



Comment at: tools/libclang/CIndexer.cpp:84
+if (CurInfo->ldinfo_next == 0u)
+  llvm_unreachable("Cannot locate entry point in the loadquery() results");
+CurInfo = reinterpret_cast(reinterpret_cast(CurInfo) +

This is also supposed to not happen.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59233



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

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: clang/test/Parser/objc-static-assert.mm:1
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+

thakis wrote:
> aaron.ballman wrote:
> > thakis wrote:
> > > aaron.ballman wrote:
> > > > Can you try explicitly specifying C++98 as the underlying language 
> > > > standard mode? I feel like `_Static_assert()` will continue to work 
> > > > there (because we made it a language extension in all modes) but 
> > > > `static_assert()` may fail (because that's gated on C++11 support). If 
> > > > that turns out to be the case, then I think `objc_static_assert` should 
> > > > be more complex than expanding to `true`, or we should talk about 
> > > > supporting `static_assert()` in all C++ language modes like we do for 
> > > > `_Static_assert()`.
> > > Correct, with -std=c++98 static_assert isn't available but _Static_assert 
> > > still is. If you want, I can add a test for this, but this is covered by 
> > > regular c++ tests already.
> > > 
> > > I think the has_feature() should stay as-is though: Else you have no way 
> > > to know if _Static_assert works in obj-c mode, and you can check if 
> > > static_assert works by checkout has_feature && __cplusplus >= 201103L if 
> > > you still care about c++98.
> > > 
> > > (And adding one feature each for static_assert and _Static_assert seems 
> > > like overkill.)
> > > Correct, with -std=c++98 static_assert isn't available but _Static_assert 
> > > still is. If you want, I can add a test for this, but this is covered by 
> > > regular c++ tests already.
> > 
> > Please do (if we don't change the availability of `static_assert()`), 
> > because the test currently relies on the unwritten -std option in order to 
> > pass.
> > 
> > > I think the has_feature() should stay as-is though: Else you have no way 
> > > to know if _Static_assert works in obj-c mode, and you can check if 
> > > static_assert works by checkout has_feature && __cplusplus >= 201103L if 
> > > you still care about c++98.
> > 
> > I don't think this is the correct approach. Testing for `static_assert()` 
> > support should not leave the user guessing at what the correct spelling is.
> > 
> > > (And adding one feature each for static_assert and _Static_assert seems 
> > > like overkill.)
> > 
> > Definitely agreed there.
> > 
> > I think the correct way forward is to support `static_assert()` in all 
> > language modes like we already do for `_Static_assert()`, then 
> > `objc_static_assert` becomes useful as-is. I cannot think of a reason why 
> > we would want to allow `_Static_assert()` in C++ but not allow 
> > `static_assert()`.
> I updated the test.
> 
> Accepting `static_assert()` independent of language mode seems pretty 
> unrelated to this patch here, so I don't want to do this.
> 
> If you don't like the current has_feature approach, I'm all ears for other 
> approaches. The current approach allows you to detect if clang can handle 
> static_assert in objc objects, and codebases that still care about c++98 will 
> have a static_assert wrapping macro keyed off __cplusplus already, so that 
> part will transparently just work as well. And codebases that are c++11 and 
> newer are in a good position too. I think the current setup is pretty good. 
> (But I'm happy to hear better suggestions.)
> Accepting static_assert() independent of language mode seems pretty unrelated 
> to this patch here, so I don't want to do this.

Yeah, we shouldn't be treating `static_assert` as a keyword in C++98 or C, I 
think. It would break code.

> If you don't like the current has_feature approach, I'm all ears for other 
> approaches. The current approach allows you to detect if clang can handle 
> static_assert in objc objects, and codebases that still care about c++98 will 
> have a static_assert wrapping macro keyed off __cplusplus already, so that 
> part will transparently just work as well. And codebases that are c++11 and 
> newer are in a good position too. I think the current setup is pretty good. 
> (But I'm happy to hear better suggestions.)

This is pretty weird. This feature flag doesn't actually correspond to any 
feature, just the possibility of the existence of a feature (there isn't any 
way you could use this `__has_feature` portably without also including another 
`__has_feature` check). I think the most internally consistent way of doing 
this is to have two flags, as you mentioned above, `objc_c_static_assert` and 
`objc_cxx_static_assert`.

Just to keep the bikeshed going, maybe it should be spelt 
`objc_interface_c_static_assert` or something, to show that it doesn't control 
static_assert in ObjC, but static_assert in interfaces in ObjC.


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

https://reviews.llvm.org/D59223



___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D59118: creduce script for clang crashes

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: gbiv.
rnk added a comment.

@gbiv already got all my shell quoting comments.

I think we should do one more round of fixes, we can commit that for you, and 
then move on to the next steps.




Comment at: clang/utils/creduce-clang-crash.py:109
+  open(testfile, 'w').write('\n'.join(test_contents))
+  os.chmod(testfile, os.stat(testfile).st_mode | stat.S_IEXEC)
+

We could try validating that the interestingness test passes here. If it 
doesn't, that's a bug in this script, I suppose.

CReduce already does this for the user, but it's not clear with our usage model 
how to do this.



Comment at: clang/utils/creduce-clang-crash.py:110
+  os.chmod(testfile, os.stat(testfile).st_mode | stat.S_IEXEC)
+
+  # Call C-Reduce

Let's add a TODO (or FIXME, that's more LLVM-y) here to add a step that runs 
the full pre-processor with -E and -P here. As we've discussed, it often 
doesn't work, but when it does, it avoids all those issues with #defines, 
comments, etc, breaking up topformflat. This doesn't have to be in the first 
version, of course.

I see @joerg added a comment about this as well.


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

https://reviews.llvm.org/D59118



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

[PATCH] D59234: [CodeGen][ObjC] Remove the leading 'l' from symbols for protocol metadata and protocol list

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

There are other symbols that starts with 'l' and have private linkage. I 
haven't made any changes to those symbols in this patch, but I think it's 
possible to remove the leading 'l' from those symbols and change the linkage to 
internal if we want to keep the symbol names.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59234



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

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Nico Weber via Phabricator via cfe-commits]

thakis updated this revision to Diff 190172.
thakis marked an inline comment as done.
thakis added a comment.

test c++98


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

https://reviews.llvm.org/D59223

Files:
  clang/include/clang/Basic/Features.def
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Parser/objc-static-assert.m
  clang/test/Parser/objc-static-assert.mm

Index: clang/test/Parser/objc-static-assert.mm
===
--- /dev/null
+++ clang/test/Parser/objc-static-assert.mm
@@ -0,0 +1,67 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -std=c++98 -fsyntax-only -verify -Wno-objc-root-class %s
+
+// __has_feature(objc_static_assert) is true in C++98 too, where
+// static_assert() isn't accepted but _Static_assert is. Clients can check
+// can check for __cplusplus to see if the static_assert spelling is available.
+#if !__has_feature(objc_static_assert)
+#error failed
+#endif
+
+#if __cplusplus >= 201103L
+// C++11
+
+@interface A {
+  int a;
+  static_assert(1, "");
+  _Static_assert(1, "");
+
+  static_assert(0, ""); // expected-error {{static_assert failed}}
+  _Static_assert(0, ""); // expected-error {{static_assert failed}}
+
+  static_assert(a, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+  static_assert(sizeof(a) == 4, "");
+  static_assert(sizeof(a) == 3, ""); // expected-error {{static_assert failed}}
+}
+
+static_assert(1, "");
+_Static_assert(1, "");
+
+- (void)f;
+@end
+
+@implementation A {
+  int b;
+  static_assert(1, "");
+  _Static_assert(1, "");
+  static_assert(sizeof(b) == 4, "");
+  static_assert(sizeof(b) == 3, ""); // expected-error {{static_assert failed}}
+}
+
+static_assert(1, "");
+
+- (void)f {
+  static_assert(1, "");
+}
+@end
+
+@interface B
+@end
+
+@interface B () {
+  int b;
+  static_assert(sizeof(b) == 4, "");
+  static_assert(sizeof(b) == 3, ""); // expected-error {{static_assert failed}}
+}
+@end
+
+#else
+// C++98
+@interface A {
+  int a;
+  static_assert(1, ""); // expected-error {{type name requires a specifier or qualifier}} expected-error{{expected parameter declarator}} expected-error {{expected ')'}} expected-note {{to match this '('}}
+  _Static_assert(1, "");
+  _Static_assert(0, ""); // expected-error {{static_assert failed}}
+}
+@end
+#endif
Index: clang/test/Parser/objc-static-assert.m
===
--- /dev/null
+++ clang/test/Parser/objc-static-assert.m
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fobjc-runtime=macosx-fragile -fsyntax-only -verify -Wno-objc-root-class %s
+
+#if !__has_feature(objc_static_assert)
+#error failed
+#endif
+
+@interface A {
+  int a;
+  _Static_assert(1, "");
+  _Static_assert(0, ""); // expected-error {{static_assert failed}}
+
+  _Static_assert(a, ""); // expected-error {{use of undeclared identifier 'a'}}
+  _Static_assert(sizeof(a), ""); // expected-error {{use of undeclared identifier 'a'}}
+}
+
+_Static_assert(1, "");
+
+@end
+
+struct S {
+  @defs(A);
+};
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2998,7 +2998,6 @@
 
 // These shouldn't make it here.
 case Decl::ObjCAtDefsField:
-case Decl::ObjCIvar:
   llvm_unreachable("forming non-member reference to ivar?");
 
 // Enum constants are always r-values and never references.
@@ -3016,6 +3015,7 @@
 // exist in the high-level semantics.
 case Decl::Field:
 case Decl::IndirectField:
+case Decl::ObjCIvar:
   assert(getLangOpts().CPlusPlus &&
  "building reference to field in C?");
 
Index: clang/lib/Parse/ParseObjc.cpp
===
--- clang/lib/Parse/ParseObjc.cpp
+++ clang/lib/Parse/ParseObjc.cpp
@@ -623,6 +623,8 @@
 }
 // Ignore excess semicolons.
 if (Tok.is(tok::semi)) {
+  // FIXME: This should use ConsumeExtraSemi() for extraneous semicolons,
+  // to make -Wextra-semi diagnose them.
   ConsumeToken();
   continue;
 }
@@ -646,7 +648,19 @@
   // erroneous r_brace would cause an infinite loop if not handled here.
   if (Tok.is(tok::r_brace))
 break;
+
   ParsedAttributesWithRange attrs(AttrFactory);
+
+  // Since we call ParseDeclarationOrFunctionDefinition() instead of
+  // ParseExternalDeclaration() below (so that this doesn't parse nested
+  // @interfaces), this needs to duplicate some code from the latter.
+  if (Tok.isOneOf(tok::kw_static_assert, tok::kw__Static_assert)) {
+SourceLocation DeclEnd;
+allTUVariables.push_back(
+ParseDeclaration(DeclaratorContext::FileContext, DeclEnd, attrs));
+continue;
+  }
+
   

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/test/Parser/objc-static-assert.mm:1
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+

aaron.ballman wrote:
> thakis wrote:
> > aaron.ballman wrote:
> > > Can you try explicitly specifying C++98 as the underlying language 
> > > standard mode? I feel like `_Static_assert()` will continue to work there 
> > > (because we made it a language extension in all modes) but 
> > > `static_assert()` may fail (because that's gated on C++11 support). If 
> > > that turns out to be the case, then I think `objc_static_assert` should 
> > > be more complex than expanding to `true`, or we should talk about 
> > > supporting `static_assert()` in all C++ language modes like we do for 
> > > `_Static_assert()`.
> > Correct, with -std=c++98 static_assert isn't available but _Static_assert 
> > still is. If you want, I can add a test for this, but this is covered by 
> > regular c++ tests already.
> > 
> > I think the has_feature() should stay as-is though: Else you have no way to 
> > know if _Static_assert works in obj-c mode, and you can check if 
> > static_assert works by checkout has_feature && __cplusplus >= 201103L if 
> > you still care about c++98.
> > 
> > (And adding one feature each for static_assert and _Static_assert seems 
> > like overkill.)
> > Correct, with -std=c++98 static_assert isn't available but _Static_assert 
> > still is. If you want, I can add a test for this, but this is covered by 
> > regular c++ tests already.
> 
> Please do (if we don't change the availability of `static_assert()`), because 
> the test currently relies on the unwritten -std option in order to pass.
> 
> > I think the has_feature() should stay as-is though: Else you have no way to 
> > know if _Static_assert works in obj-c mode, and you can check if 
> > static_assert works by checkout has_feature && __cplusplus >= 201103L if 
> > you still care about c++98.
> 
> I don't think this is the correct approach. Testing for `static_assert()` 
> support should not leave the user guessing at what the correct spelling is.
> 
> > (And adding one feature each for static_assert and _Static_assert seems 
> > like overkill.)
> 
> Definitely agreed there.
> 
> I think the correct way forward is to support `static_assert()` in all 
> language modes like we already do for `_Static_assert()`, then 
> `objc_static_assert` becomes useful as-is. I cannot think of a reason why we 
> would want to allow `_Static_assert()` in C++ but not allow `static_assert()`.
I updated the test.

Accepting `static_assert()` independent of language mode seems pretty unrelated 
to this patch here, so I don't want to do this.

If you don't like the current has_feature approach, I'm all ears for other 
approaches. The current approach allows you to detect if clang can handle 
static_assert in objc objects, and codebases that still care about c++98 will 
have a static_assert wrapping macro keyed off __cplusplus already, so that part 
will transparently just work as well. And codebases that are c++11 and newer 
are in a good position too. I think the current setup is pretty good. (But I'm 
happy to hear better suggestions.)


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

https://reviews.llvm.org/D59223



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

[PATCH] D59234: [CodeGen][ObjC] Remove the leading 'l' from symbols for protocol metadata and protocol list

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
ahatanak added a project: clang.
Herald added subscribers: dexonsmith, jkorous.

The leading 'l' tells ld64 to remove the symbol name, which can make debugging 
difficult.

rdar://problem/47256637


Repository:
  rC Clang

https://reviews.llvm.org/D59234

Files:
  lib/CodeGen/CGObjCMac.cpp
  test/CodeGenObjC/forward-protocol-metadata-symbols.m
  test/CodeGenObjC/hidden-visibility.m
  test/CodeGenObjC/metadata-class-properties.m
  test/CodeGenObjC/metadata-symbols-64.m
  test/CodeGenObjC/protocol-comdat.m
  test/CodeGenObjC/protocol-in-extended-class.m
  test/CodeGenObjC/protocols.m
  test/CodeGenObjC/sections.m
  test/CodeGenObjC/undefined-protocol2.m

Index: test/CodeGenObjC/undefined-protocol2.m
===
--- test/CodeGenObjC/undefined-protocol2.m
+++ test/CodeGenObjC/undefined-protocol2.m
@@ -3,7 +3,7 @@
 // Test that we produce a declaration for the protocol. It must be matched
 // by a definition in another TU, so external is the correct linkage
 // (not extern_weak).
-// CHECK: @"\01l_OBJC_PROTOCOL_$_p1" = external global
+// CHECK: @"_OBJC_PROTOCOL_$_p1" = external global
 
 @interface NSObject
 @end
Index: test/CodeGenObjC/sections.m
===
--- test/CodeGenObjC/sections.m
+++ test/CodeGenObjC/sections.m
@@ -38,7 +38,7 @@
 // CHECK-COFF: @OBJC_SELECTOR_REFERENCES_ = {{.*}}, section ".objc_selrefs$B"
 // CHECK-COFF: @"OBJC_CLASSLIST_REFERENCES_$_" = {{.*}}, section ".objc_classrefs$B"
 // CHECK-COFF: @"\01l_objc_msgSend_fixup_class" = {{.*}}, section ".objc_msgrefs$B"
-// CHECK-COFF: @"\01l_OBJC_LABEL_PROTOCOL_$_P" = {{.*}}, section ".objc_protolist$B"
+// CHECK-COFF: @"_OBJC_LABEL_PROTOCOL_$_P" = {{.*}}, section ".objc_protolist$B"
 // CHECK-COFF: @"\01l_OBJC_PROTOCOL_REFERENCE_$_P" = {{.*}}, section ".objc_protorefs$B"
 // CHECK-COFF: @"OBJC_LABEL_CLASS_$" = {{.*}}, section ".objc_classlist$B"
 // CHECK-COFF: @"OBJC_LABEL_NONLAZY_CLASS_$" = {{.*}}, section ".objc_nlclslist$B"
@@ -50,7 +50,7 @@
 // CHECK-ELF: @OBJC_SELECTOR_REFERENCES_ = {{.*}}, section "objc_selrefs"
 // CHECK-ELF: @"OBJC_CLASSLIST_REFERENCES_$_" = {{.*}}, section "objc_classrefs"
 // CHECK-ELF: @"\01l_objc_msgSend_fixup_class" = {{.*}}, section "objc_msgrefs"
-// CHECK-ELF: @"\01l_OBJC_LABEL_PROTOCOL_$_P" = {{.*}}, section "objc_protolist"
+// CHECK-ELF: @"_OBJC_LABEL_PROTOCOL_$_P" = {{.*}}, section "objc_protolist"
 // CHECK-ELF: @"\01l_OBJC_PROTOCOL_REFERENCE_$_P" = {{.*}}, section "objc_protorefs"
 // CHECK-ELF: @"OBJC_LABEL_CLASS_$" = {{.*}}, section "objc_classlist"
 // CHECK-ELF: @"OBJC_LABEL_NONLAZY_CLASS_$" = {{.*}}, section "objc_nlclslist"
@@ -62,7 +62,7 @@
 // CHECK-MACHO: @OBJC_SELECTOR_REFERENCES_ = {{.*}}, section "__DATA,__objc_selrefs,literal_pointers,no_dead_strip"
 // CHECK-MACHO: @"OBJC_CLASSLIST_REFERENCES_$_" = {{.*}}, section "__DATA,__objc_classrefs,regular,no_dead_strip"
 // CHECK-MACHO: @"\01l_objc_msgSend_fixup_class" = {{.*}}, section "__DATA,__objc_msgrefs,coalesced"
-// CHECK-MACHO: @"\01l_OBJC_LABEL_PROTOCOL_$_P" = {{.*}}, section "__DATA,__objc_protolist,coalesced,no_dead_strip"
+// CHECK-MACHO: @"_OBJC_LABEL_PROTOCOL_$_P" = {{.*}}, section "__DATA,__objc_protolist,coalesced,no_dead_strip"
 // CHECK-MACHO: @"\01l_OBJC_PROTOCOL_REFERENCE_$_P" = {{.*}}, section "__DATA,__objc_protorefs,coalesced,no_dead_strip"
 // CHECK-MACHO: @"OBJC_LABEL_CLASS_$" = {{.*}}, section "__DATA,__objc_classlist,regular,no_dead_strip"
 // CHECK-MACHO: @"OBJC_LABEL_NONLAZY_CLASS_$" = {{.*}}, section "__DATA,__objc_nlclslist,regular,no_dead_strip"
Index: test/CodeGenObjC/protocols.m
===
--- test/CodeGenObjC/protocols.m
+++ test/CodeGenObjC/protocols.m
@@ -1,4 +1,19 @@
-// RUN: %clang_cc1 -emit-llvm-only %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -o - %s | FileCheck %s
+
+// HECK: @[[PROTO_P1:"_OBJC_PROTOCOL_$_P1"]] = weak hidden
+// CHECK: @[[PROTO_P1:"_OBJC_PROTOCOL_\$_P1"]] = weak hidden
+// CHECK: @[[LABEL_PROTO_P1:"_OBJC_LABEL_PROTOCOL_\$_P1"]] = weak hidden global %{{.*}}* @[[PROTO_P1]]
+// CHECK: @[[PROTO_P2:"_OBJC_PROTOCOL_\$_P2"]] = weak hidden
+// CHECK: @[[LABEL_PROTO_P2:"_OBJC_LABEL_PROTOCOL_\$_P2"]] = weak hidden global %{{.*}}* @[[PROTO_P2]]
+// CHECK: @"\01l_OBJC_$_PROTOCOL_REFS_P3" = private global { i64, [3 x %{{.*}}] } { i64 2, [3 x %{{.*}}*] [%{{.*}}* @[[PROTO_P1]], %{{.*}}* @[[PROTO_P2]], %{{.*}}* null] }
+// CHECK: @[[PROTO_P3:"_OBJC_PROTOCOL_\$_P3"]] = weak hidden
+// CHECK: @[[LABEL_PROTO_P3:"_OBJC_LABEL_PROTOCOL_\$_P3"]] = weak hidden global %{{.*}}* @[[PROTO_P3]]
+// CHECK: "\01l_OBJC_PROTOCOL_REFERENCE_$_P3" = weak hidden global %{{.*}}* bitcast (%{{.*}}* @[[PROTO_P3]] to %{{.*}}*)
+// CHECK: @[[PROTO_P0:"_OBJC_PROTOCOL_\$_P0"]] = weak hidden
+// CHECK: @[[LABEL_PROTO_P0:"_OBJC_LABEL_PROTOCOL_\$_P0"]] = weak hidden global %{{.*}}* 

[PATCH] D59233: libclang/CIndexer.cpp: Use loadquery() on AIX for path to library

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: sfertile, xingxue, jasonliu.
Herald added a subscriber: arphaman.
Herald added a project: clang.

`dladdr` is not available on AIX. Similar functionality is presented through 
`loadquery`. This patch replaces a use of `dladdr` with a version based on 
`loadquery`.


Repository:
  rC Clang

https://reviews.llvm.org/D59233

Files:
  tools/libclang/CIndexer.cpp


Index: tools/libclang/CIndexer.cpp
===
--- tools/libclang/CIndexer.cpp
+++ tools/libclang/CIndexer.cpp
@@ -32,12 +32,68 @@
 
 #ifdef _WIN32
 #include 
+#elif defined(_AIX)
+#include 
+#include 
 #else
 #include 
 #endif
 
 using namespace clang;
 
+#ifdef _AIX
+namespace clang {
+namespace {
+
+template 
+void getClangResourcesPathImplAIX(LibClangPathType ) {
+  int PrevErrno = errno;
+
+  size_t BufSize = 2048u;
+  std::unique_ptr Buf;
+  while (true) {
+Buf = llvm::make_unique(BufSize);
+errno = 0;
+int Ret = loadquery(L_GETXINFO, Buf.get(), (unsigned int)BufSize);
+if (Ret != -1)
+  break; // loadquery() was successful.
+if (errno != ENOMEM)
+  llvm_unreachable("Encountered an unexpected loadquery() failure");
+
+// errno == ENOMEM; try to allocate more memory.
+if ((BufSize & ~((-1u) >> 1u)) != 0u)
+  llvm_unreachable("BufSize needed for loadquery() too large");
+
+Buf.release();
+BufSize <<= 1u;
+  }
+
+  // Extract the function entry point from the function descriptor.
+  uint64_t EntryAddr =
+  reinterpret_cast(clang_createTranslationUnit);
+
+  // Loop to locate the function entry point in the loadquery() results.
+  ld_xinfo *CurInfo = reinterpret_cast(Buf.get());
+  while (true) {
+uint64_t CurTextStart = (uint64_t)CurInfo->ldinfo_textorg;
+uint64_t CurTextEnd = CurTextStart + CurInfo->ldinfo_textsize;
+if (CurTextStart <= EntryAddr && EntryAddr < CurTextEnd)
+  break; // Successfully located.
+
+if (CurInfo->ldinfo_next == 0u)
+  llvm_unreachable("Cannot locate entry point in the loadquery() results");
+CurInfo = reinterpret_cast(reinterpret_cast(CurInfo) +
+   CurInfo->ldinfo_next);
+  }
+
+  LibClangPath += reinterpret_cast(CurInfo) + CurInfo->ldinfo_filename;
+  errno = PrevErrno;
+}
+
+} // end anonymous namespace
+} // end namespace clang
+#endif
+
 const std::string ::getClangResourcesPath() {
   // Did we already compute the path?
   if (!ResourcesPath.empty())
@@ -64,6 +120,8 @@
 #endif
 
   LibClangPath += path;
+#elif defined(_AIX)
+  getClangResourcesPathImplAIX(LibClangPath);
 #else
   // This silly cast below avoids a C++ warning.
   Dl_info info;


Index: tools/libclang/CIndexer.cpp
===
--- tools/libclang/CIndexer.cpp
+++ tools/libclang/CIndexer.cpp
@@ -32,12 +32,68 @@
 
 #ifdef _WIN32
 #include 
+#elif defined(_AIX)
+#include 
+#include 
 #else
 #include 
 #endif
 
 using namespace clang;
 
+#ifdef _AIX
+namespace clang {
+namespace {
+
+template 
+void getClangResourcesPathImplAIX(LibClangPathType ) {
+  int PrevErrno = errno;
+
+  size_t BufSize = 2048u;
+  std::unique_ptr Buf;
+  while (true) {
+Buf = llvm::make_unique(BufSize);
+errno = 0;
+int Ret = loadquery(L_GETXINFO, Buf.get(), (unsigned int)BufSize);
+if (Ret != -1)
+  break; // loadquery() was successful.
+if (errno != ENOMEM)
+  llvm_unreachable("Encountered an unexpected loadquery() failure");
+
+// errno == ENOMEM; try to allocate more memory.
+if ((BufSize & ~((-1u) >> 1u)) != 0u)
+  llvm_unreachable("BufSize needed for loadquery() too large");
+
+Buf.release();
+BufSize <<= 1u;
+  }
+
+  // Extract the function entry point from the function descriptor.
+  uint64_t EntryAddr =
+  reinterpret_cast(clang_createTranslationUnit);
+
+  // Loop to locate the function entry point in the loadquery() results.
+  ld_xinfo *CurInfo = reinterpret_cast(Buf.get());
+  while (true) {
+uint64_t CurTextStart = (uint64_t)CurInfo->ldinfo_textorg;
+uint64_t CurTextEnd = CurTextStart + CurInfo->ldinfo_textsize;
+if (CurTextStart <= EntryAddr && EntryAddr < CurTextEnd)
+  break; // Successfully located.
+
+if (CurInfo->ldinfo_next == 0u)
+  llvm_unreachable("Cannot locate entry point in the loadquery() results");
+CurInfo = reinterpret_cast(reinterpret_cast(CurInfo) +
+   CurInfo->ldinfo_next);
+  }
+
+  LibClangPath += reinterpret_cast(CurInfo) + CurInfo->ldinfo_filename;
+  errno = PrevErrno;
+}
+
+} // end anonymous namespace
+} // end namespace clang
+#endif
+
 const std::string ::getClangResourcesPath() {
   // Did we already compute the path?
   if (!ResourcesPath.empty())
@@ -64,6 +120,8 @@
 #endif
 
   LibClangPath += path;
+#elif defined(_AIX)
+  getClangResourcesPathImplAIX(LibClangPath);
 #else
   // This silly cast below 

[PATCH] D59048: Add AIX Target Info

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a subscriber: WuZhao.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

I think all comments have been addressed or otherwise responded to.
@jasonliu: Since this patch is dependent on D58930 
, I would suggest that you commit it after 
D58930 . A friendly reminder to include the 
patch attribution for @andusy. @apaprocki, and @WuZhao. Thanks.


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

https://reviews.llvm.org/D59048



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

Re: r355858 - [OPENMP]Fix codegen for declare target link in target regions.

[Alexey Bataev via cfe-commits]

I see that the next builds passed. Seems to me it wqs some kind of flacky fails.

Best regards,
Alexey Bataev

11 марта 2019 г., в 17:56, Azhar Mohammed 
mailto:az...@apple.com>> написал(а):

Hi Alexey

Looks like this is failing on Darwin, can you please take a look? Refer to 
http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/58951/consoleFull.

 TEST 'Clang :: Modules/builtins.m' FAILED 
 Script: -- : 'RUN: at line 1'; rm -rf 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp
 : 'RUN: at line 2'; 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/bin/clang
 -cc1 -internal-isystem 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/9.0.0/include
 -nostdsysteminc 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp
 -fmodules -fimplicit-module-maps -I 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs
 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/builtins.m
 -verify : 'RUN: at line 3'; 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/bin/clang
 -cc1 -internal-isystem 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/9.0.0/include
 -nostdsysteminc 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp
 -fmodules -fimplicit-module-maps -I 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs
 -x c 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/builtins.m
 -verify : 'RUN: at line 4'; 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/bin/clang
 -cc1 -internal-isystem 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/9.0.0/include
 -nostdsysteminc 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp
 -fmodules -fimplicit-module-maps -I 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs
 -x objective-c++ 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/builtins.m
 -verify : 'RUN: at line 6'; rm -rf 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp.pch.cache
 : 'RUN: at line 7'; 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/bin/clang
 -cc1 -internal-isystem 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/9.0.0/include
 -nostdsysteminc 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp.pch.cache
 -fmodules -fimplicit-module-maps -I 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs
 -emit-pch -o 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp.pch
 -x objective-c-header 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs/use-builtin.h
 : 'RUN: at line 8'; 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/bin/clang
 -cc1 -internal-isystem 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/9.0.0/include
 -nostdsysteminc 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp.pch.cache
 -fmodules -fimplicit-module-maps -I 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs
 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/builtins.m
 -include-pch 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp.pch
 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/builtins.m
 -verify -- Exit Code: 134 Command Output (stderr): -- Assertion failed: 
((Imported == nullptr || Imported == SuggestedModule.getModule()) && "the 
imported module is different than the suggested one"), function 
HandleIncludeDirective, file 

Re: r355858 - [OPENMP]Fix codegen for declare target link in target regions.

[Alexey Bataev via cfe-commits]

I don't think my fix did this, this test has nothing to do with OpenMP.

Best regards,
Alexey Bataev

11 марта 2019 г., в 17:56, Azhar Mohammed 
mailto:az...@apple.com>> написал(а):

Hi Alexey

Looks like this is failing on Darwin, can you please take a look? Refer to 
http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/58951/consoleFull.

 TEST 'Clang :: Modules/builtins.m' FAILED 
 Script: -- : 'RUN: at line 1'; rm -rf 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp
 : 'RUN: at line 2'; 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/bin/clang
 -cc1 -internal-isystem 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/9.0.0/include
 -nostdsysteminc 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp
 -fmodules -fimplicit-module-maps -I 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs
 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/builtins.m
 -verify : 'RUN: at line 3'; 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/bin/clang
 -cc1 -internal-isystem 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/9.0.0/include
 -nostdsysteminc 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp
 -fmodules -fimplicit-module-maps -I 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs
 -x c 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/builtins.m
 -verify : 'RUN: at line 4'; 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/bin/clang
 -cc1 -internal-isystem 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/9.0.0/include
 -nostdsysteminc 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp
 -fmodules -fimplicit-module-maps -I 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs
 -x objective-c++ 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/builtins.m
 -verify : 'RUN: at line 6'; rm -rf 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp.pch.cache
 : 'RUN: at line 7'; 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/bin/clang
 -cc1 -internal-isystem 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/9.0.0/include
 -nostdsysteminc 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp.pch.cache
 -fmodules -fimplicit-module-maps -I 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs
 -emit-pch -o 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp.pch
 -x objective-c-header 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs/use-builtin.h
 : 'RUN: at line 8'; 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/bin/clang
 -cc1 -internal-isystem 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/9.0.0/include
 -nostdsysteminc 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp.pch.cache
 -fmodules -fimplicit-module-maps -I 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs
 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/builtins.m
 -include-pch 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp.pch
 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/builtins.m
 -verify -- Exit Code: 134 Command Output (stderr): -- Assertion failed: 
((Imported == nullptr || Imported == SuggestedModule.getModule()) && "the 
imported module is different than the suggested one"), function 
HandleIncludeDirective, file 

[PATCH] D59118: creduce script for clang crashes

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 190164.
akhuang added a comment.

fixed diff with style edits


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

https://reviews.llvm.org/D59118

Files:
  .arcconfig
  clang/utils/creduce-clang-crash.py

Index: clang/utils/creduce-clang-crash.py
===
--- /dev/null
+++ clang/utils/creduce-clang-crash.py
@@ -0,0 +1,115 @@
+#!/usr/bin/env python
+# A tool that calls C-Reduce to create a minimal reproducer for clang crashes
+# Requires C-Reduce and not (part of LLVM utils) to be installed
+
+from argparse import ArgumentParser
+import os
+import re
+import stat
+import sys
+import subprocess
+import pipes
+
+def is_exe(fpath):
+  return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
+
+def which(program):
+  """
+  Return the full path to a program or return None.
+  """
+  fpath, fname = os.path.split(program)
+  if fpath:
+if is_exe(program):
+  return program
+  else:
+for path in os.environ["PATH"].split(os.pathsep):
+  exe_file = os.path.join(path, program)
+  if is_exe(exe_file):
+return exe_file
+  return None
+
+def create_test(build_script, llvm_not):
+  """
+  Create an interestingness test from the crash output.
+  Return as a string.
+  """
+  # Get clang call from build script
+  # Assumes the call is the last line of the script
+  with open(build_script) as f:
+cmd = f.readlines()[-1].rstrip('\n\r')
+
+  # Get crash output
+  p = subprocess.Popen(build_script,
+   stdout=subprocess.PIPE,
+   stderr=subprocess.STDOUT)
+  crash_output, _ = p.communicate()
+
+  output = ['#!/bin/bash']
+  output.append('%s --crash %s >& t.log || exit 1' % (pipes.quote(llvm_not),
+  cmd))
+
+  # Add messages from crash output to the test
+  # If there is an Assertion failure, use that; otherwise use the
+  # last five stack trace functions
+  assertion_re = r'Assertion `([^\']+)\' failed'
+  assertion_match = re.search(assertion_re, crash_output)
+  if assertion_match:
+msg = assertion_match.group(1)
+output.append('grep %s t.log || exit 1' % pipes.quote(msg))
+  else:
+stacktrace_re = r'#[0-9]+\s+0[xX][0-9a-fA-F]+\s*([^(]+)\('
+matches = re.findall(stacktrace_re, crash_output)
+del matches[:-5]
+output += ['grep %s t.log || exit 1' % pipes.quote(msg) for msg in matches]
+
+  return output
+
+def main():
+  parser = ArgumentParser(description='Runs C-Reduce on the input file')
+  parser.add_argument('build_script', type=str, nargs=1,
+  help='Name of the script that generates the crash.')
+  parser.add_argument('file_to_reduce', type=str, nargs=1,
+  help='Name of the file to be reduced.')
+  parser.add_argument('-o', '--output', dest='output', type=str,
+  help='Name of the output file for the reduction. Optional.')
+  parser.add_argument('--llvm-not', dest='llvm_not', type=str,
+  help="The path to the llvm-not executable. "
+  "Required if 'not' is not in PATH environment.");
+  parser.add_argument('--creduce', dest='creduce', type=str,
+  help="The path to the C-Reduce executable. "
+  "Required if 'creduce' is not in PATH environment.");
+  args = parser.parse_args()
+
+  build_script = os.path.abspath(args.build_script[0])
+  file_to_reduce = os.path.abspath(args.file_to_reduce[0])
+  llvm_not = which(args.llvm_not) if args.llvm_not else which('not')
+  creduce = which(args.creduce) if args.creduce else which('creduce')
+
+  if not os.path.isfile(build_script):
+print(("ERROR: input file '%s' does not exist") % build_script)
+sys.exit(1)
+
+  if not os.path.isfile(file_to_reduce):
+print(("ERROR: input file '%s' does not exist") % file_to_reduce)
+sys.exit(1)
+
+  if not llvm_not:
+parser.print_help()
+sys.exit(1)
+
+  if not creduce:
+parser.print_help()
+sys.exit(1)
+
+  # Write interestingness test to file
+  test_contents = create_test(build_script, llvm_not)
+  testname, _ = os.path.splitext(file_to_reduce)
+  testfile = testname + '.test.sh'
+  with open(testfile, 'w') as f:
+f.write('\n'.join(test_contents))
+  os.chmod(testfile, os.stat(testfile).st_mode | stat.S_IEXEC)
+
+  sys.exit(subprocess.call([creduce, testfile, file_to_reduce]))
+
+if __name__ == '__main__':
+  main()
Index: .arcconfig
===
--- .arcconfig
+++ .arcconfig
@@ -1,4 +1,4 @@
 {
-  "repository.callsign" : "G",
-  "conduit_uri" : "https://reviews.llvm.org/;
+"repository.callsign" : "G",
+"conduit_uri" : "https://reviews.llvm.org/;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r355858 - [OPENMP]Fix codegen for declare target link in target regions.

[Azhar Mohammed via cfe-commits]

Hi Alexey

Looks like this is failing on Darwin, can you please take a look? Refer to 
http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/58951/consoleFull
 
.

 TEST 'Clang :: Modules/builtins.m' FAILED 

Script:
--
: 'RUN: at line 1';   rm -rf 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp
: 'RUN: at line 2';   
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/bin/clang
 -cc1 -internal-isystem 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/9.0.0/include
 -nostdsysteminc 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp
 -fmodules -fimplicit-module-maps -I 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs
 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/builtins.m
 -verify
: 'RUN: at line 3';   
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/bin/clang
 -cc1 -internal-isystem 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/9.0.0/include
 -nostdsysteminc 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp
 -fmodules -fimplicit-module-maps -I 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs
 -x c 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/builtins.m
 -verify
: 'RUN: at line 4';   
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/bin/clang
 -cc1 -internal-isystem 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/9.0.0/include
 -nostdsysteminc 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp
 -fmodules -fimplicit-module-maps -I 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs
 -x objective-c++ 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/builtins.m
 -verify
: 'RUN: at line 6';   rm -rf 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp.pch.cache
: 'RUN: at line 7';   
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/bin/clang
 -cc1 -internal-isystem 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/9.0.0/include
 -nostdsysteminc 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp.pch.cache
 -fmodules -fimplicit-module-maps -I 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs
 -emit-pch -o 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp.pch
 -x objective-c-header 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs/use-builtin.h
: 'RUN: at line 8';   
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/bin/clang
 -cc1 -internal-isystem 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/9.0.0/include
 -nostdsysteminc 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp.pch.cache
 -fmodules -fimplicit-module-maps -I 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/Inputs
 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/builtins.m
 -include-pch 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/tools/clang/test/Modules/Output/builtins.m.tmp.pch
 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/Modules/builtins.m
 -verify
--
Exit Code: 134

Command Output (stderr):
--
 <>Assertion failed: ((Imported == nullptr || Imported == 
SuggestedModule.getModule()) && "the imported module is different than the 
suggested one"), function HandleIncludeDirective, file 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/lib/Lex/PPDirectives.cpp,
 line 1952.

> On Mar 11, 2019, at 12:51 PM, Alexey Bataev via cfe-commits 

[PATCH] D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

ping

The Polly-powered additional transformations 
 now also generate this kind of 
metadata.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57978



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

[PATCH] D59118: creduce script for clang crashes

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

For it to be really useful for the majority of bugs, it would be nice to figure 
out automatically how to get the preprocessing step done and filter out the # 
lines afterwards. That part alone significantly cuts down the creduce time.


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

https://reviews.llvm.org/D59118



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

[PATCH] D59176: Modules: Add LangOptions::CacheGeneratedPCH

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

dexonsmith marked 2 inline comments as done.
dexonsmith added a comment.

In D59176#1424864 , @jordan_rose wrote:

> This commit by itself doesn't change any behavior, right?


Correct, it just exposes an option.




Comment at: clang/lib/Frontend/FrontendActions.cpp:115
   CI.getPreprocessorOpts().AllowPCHWithCompilerErrors,
-  FrontendOpts.IncludeTimestamps));
+  FrontendOpts.IncludeTimestamps, +CI.getLangOpts().CacheGeneratedPCH));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(

jordan_rose wrote:
> What's the `+` for?
Heh, I was trying to figure out those `+`s as well until I added this without 
one and it didn't compile.  The problem is that bitfields can't be forwarded 
through the `llvm::make_unique` variadic.  The `+` creates a local temporary 
with the same value.



Comment at: clang/lib/Frontend/FrontendActions.cpp:182
+  CI.getFrontendOpts().BuildingImplicitModule &&
+  CI.getLangOpts().isCompilingModule()));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(

jordan_rose wrote:
> Why is this the condition, as opposed to just "do this for all modules, don't 
> do it for PCHs"? And doesn't `BuildingImplicitModule` imply 
> `isCompilingModule()`? (Note that `BuildingImplicitModule` probably isn't the 
> same as the original condition `ImplicitModules`.)
> (Note that BuildingImplicitModule probably isn't the same as the original 
> condition ImplicitModules.)

Nice catch; I ported the logic from `ASTWriter` incorrectly.  I'll fix this.

> Why is this the condition, as opposed to just "do this for all modules, don't 
> do it for PCHs"? And doesn't BuildingImplicitModule imply isCompilingModule()?

I was cargo-culting the logic for how `PCHGenerator::HandleTranslationUnit` 
sets the `Module` parameter to `ASTWriter::WriteAST`.  I think that if I fix 
the above to match the original condition I'll need to check 
`isCompilingModule()`... but maybe `BuildingImplicitModule` is already `&&`-ing 
these together?  I'll check.


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

https://reviews.llvm.org/D59176



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

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

[Reuben Thomas via Phabricator via cfe-commits]

reuk added a comment.

The code looks good now. There's just a few places left where the formatting 
looks a bit suspect (sorry for missing those last time). I think once that's 
fixed this will be good to go.




Comment at: clang/unittests/Format/FormatTest.cpp:553
 
-  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = 
FormatStyle::SIS_WithoutElse;
   AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;

This line, along with some of the following changed lines, have more than 80 
characters. Make sure to clang-format changes! Consider using 
`git-clang-format` to format just this patch.


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

https://reviews.llvm.org/D59087



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

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/Parser/objc-static-assert.mm:1
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+

thakis wrote:
> aaron.ballman wrote:
> > Can you try explicitly specifying C++98 as the underlying language standard 
> > mode? I feel like `_Static_assert()` will continue to work there (because 
> > we made it a language extension in all modes) but `static_assert()` may 
> > fail (because that's gated on C++11 support). If that turns out to be the 
> > case, then I think `objc_static_assert` should be more complex than 
> > expanding to `true`, or we should talk about supporting `static_assert()` 
> > in all C++ language modes like we do for `_Static_assert()`.
> Correct, with -std=c++98 static_assert isn't available but _Static_assert 
> still is. If you want, I can add a test for this, but this is covered by 
> regular c++ tests already.
> 
> I think the has_feature() should stay as-is though: Else you have no way to 
> know if _Static_assert works in obj-c mode, and you can check if 
> static_assert works by checkout has_feature && __cplusplus >= 201103L if you 
> still care about c++98.
> 
> (And adding one feature each for static_assert and _Static_assert seems like 
> overkill.)
> Correct, with -std=c++98 static_assert isn't available but _Static_assert 
> still is. If you want, I can add a test for this, but this is covered by 
> regular c++ tests already.

Please do (if we don't change the availability of `static_assert()`), because 
the test currently relies on the unwritten -std option in order to pass.

> I think the has_feature() should stay as-is though: Else you have no way to 
> know if _Static_assert works in obj-c mode, and you can check if 
> static_assert works by checkout has_feature && __cplusplus >= 201103L if you 
> still care about c++98.

I don't think this is the correct approach. Testing for `static_assert()` 
support should not leave the user guessing at what the correct spelling is.

> (And adding one feature each for static_assert and _Static_assert seems like 
> overkill.)

Definitely agreed there.

I think the correct way forward is to support `static_assert()` in all language 
modes like we already do for `_Static_assert()`, then `objc_static_assert` 
becomes useful as-is. I cannot think of a reason why we would want to allow 
`_Static_assert()` in C++ but not allow `static_assert()`.


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

https://reviews.llvm.org/D59223



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

[PATCH] D59133: Remove esan.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC355862: Remove esan. (authored by nico, committed by ).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59133?vs=189858=190147#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59133

Files:
  include/clang/Basic/Features.def
  include/clang/Basic/Sanitizers.def
  include/clang/Driver/SanitizerArgs.h
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Driver/ToolChains/Darwin.cpp
  lib/Driver/ToolChains/Linux.cpp
  lib/Driver/ToolChains/NetBSD.cpp
  test/Driver/esan.c
  test/Driver/fsanitize.c
  test/Driver/sanitize_unwind_tables.c
  test/Driver/sanitizer-ld.c
  test/Lexer/has_feature_efficiency_sanitizer.cpp

Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -319,19 +319,6 @@
   PM.add(createDataFlowSanitizerPass(LangOpts.SanitizerBlacklistFiles));
 }
 
-static void addEfficiencySanitizerPass(const PassManagerBuilder ,
-   legacy::PassManagerBase ) {
-  const PassManagerBuilderWrapper  =
-  static_cast(Builder);
-  const LangOptions  = BuilderWrapper.getLangOpts();
-  EfficiencySanitizerOptions Opts;
-  if (LangOpts.Sanitize.has(SanitizerKind::EfficiencyCacheFrag))
-Opts.ToolType = EfficiencySanitizerOptions::ESAN_CacheFrag;
-  else if (LangOpts.Sanitize.has(SanitizerKind::EfficiencyWorkingSet))
-Opts.ToolType = EfficiencySanitizerOptions::ESAN_WorkingSet;
-  PM.add(createEfficiencySanitizerPass(Opts));
-}
-
 static TargetLibraryInfoImpl *createTLII(llvm::Triple ,
  const CodeGenOptions ) {
   TargetLibraryInfoImpl *TLII = new TargetLibraryInfoImpl(TargetTriple);
@@ -656,13 +643,6 @@
addDataFlowSanitizerPass);
   }
 
-  if (LangOpts.Sanitize.hasOneOf(SanitizerKind::Efficiency)) {
-PMBuilder.addExtension(PassManagerBuilder::EP_OptimizerLast,
-   addEfficiencySanitizerPass);
-PMBuilder.addExtension(PassManagerBuilder::EP_EnabledOnOptLevel0,
-   addEfficiencySanitizerPass);
-  }
-
   // Set up the per-function pass manager.
   FPM.add(new TargetLibraryInfoWrapperPass(*TLII));
   if (CodeGenOpts.VerifyModule)
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -401,31 +401,24 @@
   std::make_pair(SanitizerKind::HWAddress,
  SanitizerKind::Address | SanitizerKind::Thread |
  SanitizerKind::Memory | SanitizerKind::KernelAddress),
-  std::make_pair(SanitizerKind::Efficiency,
- SanitizerKind::Address | SanitizerKind::HWAddress |
- SanitizerKind::Leak | SanitizerKind::Thread |
- SanitizerKind::Memory | SanitizerKind::KernelAddress),
   std::make_pair(SanitizerKind::Scudo,
  SanitizerKind::Address | SanitizerKind::HWAddress |
  SanitizerKind::Leak | SanitizerKind::Thread |
- SanitizerKind::Memory | SanitizerKind::KernelAddress |
- SanitizerKind::Efficiency),
+ SanitizerKind::Memory | SanitizerKind::KernelAddress),
   std::make_pair(SanitizerKind::SafeStack,
  SanitizerKind::Address | SanitizerKind::HWAddress |
  SanitizerKind::Leak | SanitizerKind::Thread |
- SanitizerKind::Memory | SanitizerKind::KernelAddress |
- SanitizerKind::Efficiency),
+ SanitizerKind::Memory | SanitizerKind::KernelAddress),
   std::make_pair(SanitizerKind::KernelHWAddress,
  SanitizerKind::Address | SanitizerKind::HWAddress |
  SanitizerKind::Leak | SanitizerKind::Thread |
  SanitizerKind::Memory | SanitizerKind::KernelAddress |
- SanitizerKind::Efficiency | SanitizerKind::SafeStack),
+ SanitizerKind::SafeStack),
   std::make_pair(SanitizerKind::KernelMemory,
  SanitizerKind::Address | SanitizerKind::HWAddress |
  SanitizerKind::Leak | SanitizerKind::Thread |
  SanitizerKind::Memory | SanitizerKind::KernelAddress |
- SanitizerKind::Efficiency | SanitizerKind::Scudo |
- SanitizerKind::SafeStack)};
+ SanitizerKind::Scudo | SanitizerKind::SafeStack)};
   // Enable toolchain specific default sanitizers if not explicitly disabled.
   

r355862 - Remove esan.

[Nico Weber via cfe-commits]

Author: nico
Date: Mon Mar 11 13:23:40 2019
New Revision: 355862

URL: http://llvm.org/viewvc/llvm-project?rev=355862=rev
Log:
Remove esan.

It hasn't seen active development in years, and it hasn't reached a
state where it was useful.

Remove the code until someone is interested in working on it again.

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

Removed:
cfe/trunk/test/Driver/esan.c
cfe/trunk/test/Lexer/has_feature_efficiency_sanitizer.cpp
Modified:
cfe/trunk/include/clang/Basic/Features.def
cfe/trunk/include/clang/Basic/Sanitizers.def
cfe/trunk/include/clang/Driver/SanitizerArgs.h
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/lib/Driver/ToolChains/Linux.cpp
cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp
cfe/trunk/test/Driver/fsanitize.c
cfe/trunk/test/Driver/sanitize_unwind_tables.c
cfe/trunk/test/Driver/sanitizer-ld.c

Modified: cfe/trunk/include/clang/Basic/Features.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Features.def?rev=355862=355861=355862=diff
==
--- cfe/trunk/include/clang/Basic/Features.def (original)
+++ cfe/trunk/include/clang/Basic/Features.def Mon Mar 11 13:23:40 2019
@@ -86,8 +86,6 @@ FEATURE(memory_sanitizer,
SanitizerKind::KernelMemory))
 FEATURE(thread_sanitizer, LangOpts.Sanitize.has(SanitizerKind::Thread))
 FEATURE(dataflow_sanitizer, LangOpts.Sanitize.has(SanitizerKind::DataFlow))
-FEATURE(efficiency_sanitizer,
-LangOpts.Sanitize.hasOneOf(SanitizerKind::Efficiency))
 FEATURE(scudo, LangOpts.Sanitize.hasOneOf(SanitizerKind::Scudo))
 // Objective-C features
 FEATURE(objc_arr, LangOpts.ObjCAutoRefCount) // FIXME: REMOVE?

Modified: cfe/trunk/include/clang/Basic/Sanitizers.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Sanitizers.def?rev=355862=355861=355862=diff
==
--- cfe/trunk/include/clang/Basic/Sanitizers.def (original)
+++ cfe/trunk/include/clang/Basic/Sanitizers.def Mon Mar 11 13:23:40 2019
@@ -165,13 +165,6 @@ SANITIZER_GROUP("integer", Integer,
 SANITIZER("local-bounds", LocalBounds)
 SANITIZER_GROUP("bounds", Bounds, ArrayBounds | LocalBounds)
 
-// EfficiencySanitizer
-SANITIZER("efficiency-cache-frag", EfficiencyCacheFrag)
-SANITIZER("efficiency-working-set", EfficiencyWorkingSet)
-// Meta-group only used internally.
-SANITIZER_GROUP("efficiency-all", Efficiency,
-EfficiencyCacheFrag | EfficiencyWorkingSet)
-
 // Scudo hardened allocator
 SANITIZER("scudo", Scudo)
 

Modified: cfe/trunk/include/clang/Driver/SanitizerArgs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/SanitizerArgs.h?rev=355862=355861=355862=diff
==
--- cfe/trunk/include/clang/Driver/SanitizerArgs.h (original)
+++ cfe/trunk/include/clang/Driver/SanitizerArgs.h Mon Mar 11 13:23:40 2019
@@ -73,9 +73,6 @@ class SanitizerArgs {
   bool needsCfiRt() const;
   bool needsCfiDiagRt() const;
   bool needsStatsRt() const { return Stats; }
-  bool needsEsanRt() const {
-return Sanitizers.hasOneOf(SanitizerKind::Efficiency);
-  }
   bool needsScudoRt() const { return Sanitizers.has(SanitizerKind::Scudo); }
 
   bool requiresPIE() const;

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=355862=355861=355862=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Mon Mar 11 13:23:40 2019
@@ -319,19 +319,6 @@ static void addDataFlowSanitizerPass(con
   PM.add(createDataFlowSanitizerPass(LangOpts.SanitizerBlacklistFiles));
 }
 
-static void addEfficiencySanitizerPass(const PassManagerBuilder ,
-   legacy::PassManagerBase ) {
-  const PassManagerBuilderWrapper  =
-  static_cast(Builder);
-  const LangOptions  = BuilderWrapper.getLangOpts();
-  EfficiencySanitizerOptions Opts;
-  if (LangOpts.Sanitize.has(SanitizerKind::EfficiencyCacheFrag))
-Opts.ToolType = EfficiencySanitizerOptions::ESAN_CacheFrag;
-  else if (LangOpts.Sanitize.has(SanitizerKind::EfficiencyWorkingSet))
-Opts.ToolType = EfficiencySanitizerOptions::ESAN_WorkingSet;
-  PM.add(createEfficiencySanitizerPass(Opts));
-}
-
 static TargetLibraryInfoImpl *createTLII(llvm::Triple ,
  const CodeGenOptions ) {
   TargetLibraryInfoImpl *TLII = new TargetLibraryInfoImpl(TargetTriple);
@@ -656,13 +643,6 @@ void EmitAssemblyHelper::CreatePasses(le
addDataFlowSanitizerPass);
   }
 
-  if 

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Nico Weber via Phabricator via cfe-commits]

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



Comment at: clang/lib/Parse/ParseDecl.cpp:3892-3893
 ///
+/// Note that a struct declaration refers to a declaration in a struct,
+/// not to the declaration of a struct.
+///

aaron.ballman wrote:
> Seems to be unrelated to this patch? Feel free to commit separately.
It's somewhat related in that ParseStructDeclaration() is called a few lines 
below where I'm adding the comment talking about ParseStructUnionBody()



Comment at: clang/test/Parser/objc-static-assert.mm:1
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+

aaron.ballman wrote:
> Can you try explicitly specifying C++98 as the underlying language standard 
> mode? I feel like `_Static_assert()` will continue to work there (because we 
> made it a language extension in all modes) but `static_assert()` may fail 
> (because that's gated on C++11 support). If that turns out to be the case, 
> then I think `objc_static_assert` should be more complex than expanding to 
> `true`, or we should talk about supporting `static_assert()` in all C++ 
> language modes like we do for `_Static_assert()`.
Correct, with -std=c++98 static_assert isn't available but _Static_assert still 
is. If you want, I can add a test for this, but this is covered by regular c++ 
tests already.

I think the has_feature() should stay as-is though: Else you have no way to 
know if _Static_assert works in obj-c mode, and you can check if static_assert 
works by checkout has_feature && __cplusplus >= 201103L if you still care about 
c++98.

(And adding one feature each for static_assert and _Static_assert seems like 
overkill.)


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

https://reviews.llvm.org/D59223



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

Re: r355182 - [clang-format] [NFC] clang-format the Format library

[Michael Kruse via cfe-commits]

Am Sa., 9. März 2019 um 13:54 Uhr schrieb MyDeveloper Day
:
>
> Yes ideally I'd agree, It also says:
>
> > Avoid committing formatting- or whitespace-only changes outside of code you 
> > plan to make subsequent changes to.
>
> This was getting to be a problem, every time we edited the file (with 
> auto-clang-format) a large number of other changes would also get made, 
> presumably because others hadn't been clang-formatting their commits 
> correctly! we'd have to merge out a lot of changes everytime.

This is the same motivation behind r338291 which motivated adding this
developer policy (see
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20180730/236802.html).

I use "git clang-format origin/master" to only reformat lines that I touched.


> >Also, try to separate formatting or whitespace changes from functional 
> >changes, either by correcting the format first (ideally) or afterward
>
> which means you want those changes to be separate commits.. so ideally its 
> better we clang-format the file when there is no other related changes so 
> future revisions don't need to have a mixture of real changes and whitespace 
> changes.

The policy wants only those parts you are going to change to be
re-formatted, up to complete files (I've seen those sometimes), not
entire components.


> I also think given we also encourage people to clang-format out code,  its 
> ironic clang-format itself is not clang-formatted!

This should be pointed-out in reviews. Unfortunately, things will
always slip through. Also, clang-format changes over time, i.e. we
would have to reformat regularly anyway.


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

[PATCH] D59048: Add AIX Target Info

[Andus Yu via Phabricator via cfe-commits]

andusy updated this revision to Diff 190133.
andusy added a comment.

- Fixed comment.
- Checked that `_LP64`, `__LP64__`, and `__64BIT__` aren't defined in 32 bit 
mode.
- Checked that `_ILP32` and `__ILP32__` aren't defined in 64 bit mode.


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

https://reviews.llvm.org/D59048

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

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

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

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



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

aaron.ballman wrote:
> I think this work should be done within `SubstituteDeducedTypeTransform` 
> rather than on the side. Any caller to `Sema::ReplaceAutoType()` should get 
> this same behavior.
Doing this work in `SubstituteDeducedTypeTransform` involves teaching 
`SubstituteDeducedTypeTransform` about `TypeProcessingState` and probably 
adding `TypeProcessingState` to `Sema::ReplaceAutoType()`. As for me, it 
exposes `TypeProcessingState` in more places than necessary. And it feels 
somewhat awkward that `SubstituteDeducedTypeTransform` is used in multiple 
places but `TypeProcessingState` is required only here.

I've modelled my approach after `TypeProcessingState::getAttributedType` where 
it forwards the call to Sema and keeps `AttrsForTypes` up to date. Do you still 
think `SubstituteDeducedTypeTransform` would be a better place for this code?


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

https://reviews.llvm.org/D58659



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

r355858 - [OPENMP]Fix codegen for declare target link in target regions.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Mar 11 12:51:42 2019
New Revision: 355858

URL: http://llvm.org/viewvc/llvm-project?rev=355858=rev
Log:
[OPENMP]Fix codegen for declare target link in target regions.

If the declare target link global is used in the target region
indirectly (used in the inner parallel, teams, etc. regions), we may
miss this variable and it leads to incorrect codegen.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=355858=355857=355858=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Mon Mar 11 12:51:42 2019
@@ -147,6 +147,9 @@ private:
 /// Reference to the taskgroup task_reduction reference expression.
 Expr *TaskgroupReductionRef = nullptr;
 llvm::DenseSet MappedClassesQualTypes;
+/// List of globals marked as declare target link in this target region
+/// (isOpenMPTargetExecutionDirective(Directive) == true).
+llvm::SmallVector DeclareTargetLinkVarDecls;
 SharingMapTy(OpenMPDirectiveKind DKind, DeclarationNameInfo Name,
  Scope *CurScope, SourceLocation Loc)
 : Directive(DKind), DirectiveName(Name), CurScope(CurScope),
@@ -674,6 +677,31 @@ public:
 return StackElem.MappedClassesQualTypes.count(QT) != 0;
   }
 
+  /// Adds global declare target to the parent target region.
+  void addToParentTargetRegionLinkGlobals(DeclRefExpr *E) {
+assert(*OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(
+   E->getDecl()) == OMPDeclareTargetDeclAttr::MT_Link &&
+   "Expected declare target link global.");
+if (isStackEmpty())
+  return;
+auto It = Stack.back().first.rbegin();
+while (It != Stack.back().first.rend() &&
+   !isOpenMPTargetExecutionDirective(It->Directive))
+  ++It;
+if (It != Stack.back().first.rend()) {
+  assert(isOpenMPTargetExecutionDirective(It->Directive) &&
+ "Expected target executable directive.");
+  It->DeclareTargetLinkVarDecls.push_back(E);
+}
+  }
+
+  /// Returns the list of globals with declare target link if current directive
+  /// is target.
+  ArrayRef getLinkGlobals() const {
+assert(isOpenMPTargetExecutionDirective(getCurrentDirective()) &&
+   "Expected target executable directive.");
+return Stack.back().first.back().DeclareTargetLinkVarDecls;
+  }
 };
 
 bool isImplicitTaskingRegion(OpenMPDirectiveKind DKind) {
@@ -2414,8 +2442,18 @@ public:
   // Define implicit data-sharing attributes for task.
   DVar = Stack->getImplicitDSA(VD, /*FromParent=*/false);
   if (isOpenMPTaskingDirective(DKind) && DVar.CKind != OMPC_shared &&
-  !Stack->isLoopControlVariable(VD).first)
+  !Stack->isLoopControlVariable(VD).first) {
 ImplicitFirstprivate.push_back(E);
+return;
+  }
+
+  // Store implicitly used globals with declare target link for parent
+  // target.
+  if (!isOpenMPTargetExecutionDirective(DKind) && Res &&
+  *Res == OMPDeclareTargetDeclAttr::MT_Link) {
+Stack->addToParentTargetRegionLinkGlobals(E);
+return;
+  }
 }
   }
   void VisitMemberExpr(MemberExpr *E) {
@@ -2573,7 +2611,13 @@ public:
   }
 
   DSAAttrChecker(DSAStackTy *S, Sema , CapturedStmt *CS)
-  : Stack(S), SemaRef(SemaRef), ErrorFound(false), CS(CS) {}
+  : Stack(S), SemaRef(SemaRef), ErrorFound(false), CS(CS) {
+// Process declare target link variables for the target directives.
+if (isOpenMPTargetExecutionDirective(S->getCurrentDirective())) {
+  for (DeclRefExpr *E : Stack->getLinkGlobals())
+Visit(E);
+}
+  }
 };
 } // namespace
 

Modified: cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp?rev=355858=355857=355858=diff
==
--- cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp Mon Mar 11 12:51:42 
2019
@@ -37,10 +37,13 @@ int maini1() {
   {
 a = c;
   }
+#pragma omp target
+#pragma omp teams
+  c = a;
   return 0;
 }
 
-// DEVICE: define weak void 
@__omp_offloading_{{.*}}_{{.*}}maini1{{.*}}_l[[@LINE-7]](i32* 
dereferenceable{{[^,]*}}
+// DEVICE: define weak void 
@__omp_offloading_{{.*}}_{{.*}}maini1{{.*}}_l[[@LINE-10]](i32* 
dereferenceable{{[^,]*}}
 // DEVICE: [[C_REF:%.+]] = load i32*, i32** @c_decl_tgt_link_ptr,
 // DEVICE: [[C:%.+]] = load i32, i32* [[C_REF]],
 // DEVICE: store i32 [[C]], i32* %
@@ -59,9 +62,10 @@ int maini1() {
 // HOST: [[BP0:%.+]] = getelementptr inbounds [2 x i8*], [2 x i8*]* 
[[BASEPTRS]], i{{[0-9]+}} 0, i{{[0-9]+}} 0
 // HOST: [[P0:%.+]] = 

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:3892-3893
 ///
+/// Note that a struct declaration refers to a declaration in a struct,
+/// not to the declaration of a struct.
+///

Seems to be unrelated to this patch? Feel free to commit separately.



Comment at: clang/test/Parser/objc-static-assert.mm:1
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+

Can you try explicitly specifying C++98 as the underlying language standard 
mode? I feel like `_Static_assert()` will continue to work there (because we 
made it a language extension in all modes) but `static_assert()` may fail 
(because that's gated on C++11 support). If that turns out to be the case, then 
I think `objc_static_assert` should be more complex than expanding to `true`, 
or we should talk about supporting `static_assert()` in all C++ language modes 
like we do for `_Static_assert()`.


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

https://reviews.llvm.org/D59223



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

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

(Since jfb added a few people who might not have seen it: This was discussed in 
"Objective-C++11: concerns about adding static_assert support to @interface / 
@implementation?" on cfe-dev)


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

https://reviews.llvm.org/D59223



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

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

[Shoaib Meenai via Phabricator via cfe-commits]

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

LGTM, though you may wanna wait for @jdenny too.

In D59168#1423587 , @phosek wrote:

> The layout currently looks as follows:
>
>   compiler-rt:
> headers: $prefix/lib/clang/$version/include
> libraries: $prefix/lib/clang/$version/$triple/lib/$name.$ext
>  
>   libc++, libc++abi, libunwind:
> headers: $prefix/include/c++/v1
> libraries: $prefix/lib/clang/$triple/$name.$ext
>
>
> So if we take `x86_64-linux-gnu` as an example target, it'd be:
>
>   include/c++/v1
>   lib/clang/x86_64-linux-gnu/{libc++.so,libc++abi.so,libunwind.so}
>   lib/clang/9.0.0/x86_64-linux-gnu/lib/libclang_rt.builtins.a
>
>
> I'm not super enthusiastic about the duplicated triple, but the only way to 
> eliminate it would be to move the Clang resource directory inside of 
> `lib/clang/x86_64-linux-gnu`, i.e. we'd have 
> `lib/clang/x86_64-linux-gnu/9.0.0/{include,lib}`.


I don't think the duplicated triple is too huge of a deal. I think the layout 
where the resource directory is moved inside the triple directory is a bit 
nicer, but I also don't know how much work that change would be and if it's 
worth it.




Comment at: clang/test/Driver/linux-per-target-runtime-dir.c:15
 // CHECK-PER-TARGET-RUNTIME: "--sysroot=[[SYSROOT]]"
+// CHECK-PER-TARGET-RUNTIME: 
"-L{{.*}}{{/|}}..{{/|}}lib{{/|}}clang{{/|}}x86_64-linux-gnu"
 // CHECK-PER-TARGET-RUNTIME: 
"-L[[RESDIR]]{{/|}}x86_64-linux-gnu{{/|}}lib"

Idk if it's worth making this a bit more specific – `clang -###` should print 
out its InstallerDir, so you could capture that in a FileCheck regex and use it 
to check the exact path.


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

https://reviews.llvm.org/D59168



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

[PATCH] D58317: [clang] Add install targets for API headers

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355853: [clang] Add install targets for API headers 
(authored by smeenai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58317?vs=189199=190135#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58317

Files:
  cfe/trunk/CMakeLists.txt
  cfe/trunk/docs/ReleaseNotes.rst


Index: cfe/trunk/docs/ReleaseNotes.rst
===
--- cfe/trunk/docs/ReleaseNotes.rst
+++ cfe/trunk/docs/ReleaseNotes.rst
@@ -156,7 +156,9 @@
 - In 8.0.0 and below, the install-clang-headers target would install clang's
   resource directory headers. This installation is now performed by the
   install-clang-resource-headers target. Users of the old install-clang-headers
-  target should switch to the new install-clang-resource-headers target.
+  target should switch to the new install-clang-resource-headers target. The
+  install-clang-headers target now installs clang's API headers (corresponding
+  to its libraries), which is consistent with the install-llvm-headers target.
 
 -  ...
 
Index: cfe/trunk/CMakeLists.txt
===
--- cfe/trunk/CMakeLists.txt
+++ cfe/trunk/CMakeLists.txt
@@ -388,6 +388,7 @@
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
   install(DIRECTORY include/clang include/clang-c
 DESTINATION include
+COMPONENT clang-headers
 FILES_MATCHING
 PATTERN "*.def"
 PATTERN "*.h"
@@ -397,12 +398,23 @@
 
   install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/include/clang
 DESTINATION include
+COMPONENT clang-headers
 FILES_MATCHING
 PATTERN "CMakeFiles" EXCLUDE
 PATTERN "*.inc"
 PATTERN "*.h"
 )
 
+  # Installing the headers needs to depend on generating any public
+  # tablegen'd headers.
+  add_custom_target(clang-headers DEPENDS clang-tablegen-targets)
+  set_target_properties(clang-headers PROPERTIES FOLDER "Misc")
+  if(NOT LLVM_ENABLE_IDE)
+add_llvm_install_targets(install-clang-headers
+ DEPENDS clang-headers
+ COMPONENT clang-headers)
+  endif()
+
   install(PROGRAMS utils/bash-autocomplete.sh
 DESTINATION share/clang
 )


Index: cfe/trunk/docs/ReleaseNotes.rst
===
--- cfe/trunk/docs/ReleaseNotes.rst
+++ cfe/trunk/docs/ReleaseNotes.rst
@@ -156,7 +156,9 @@
 - In 8.0.0 and below, the install-clang-headers target would install clang's
   resource directory headers. This installation is now performed by the
   install-clang-resource-headers target. Users of the old install-clang-headers
-  target should switch to the new install-clang-resource-headers target.
+  target should switch to the new install-clang-resource-headers target. The
+  install-clang-headers target now installs clang's API headers (corresponding
+  to its libraries), which is consistent with the install-llvm-headers target.
 
 -  ...
 
Index: cfe/trunk/CMakeLists.txt
===
--- cfe/trunk/CMakeLists.txt
+++ cfe/trunk/CMakeLists.txt
@@ -388,6 +388,7 @@
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
   install(DIRECTORY include/clang include/clang-c
 DESTINATION include
+COMPONENT clang-headers
 FILES_MATCHING
 PATTERN "*.def"
 PATTERN "*.h"
@@ -397,12 +398,23 @@
 
   install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/include/clang
 DESTINATION include
+COMPONENT clang-headers
 FILES_MATCHING
 PATTERN "CMakeFiles" EXCLUDE
 PATTERN "*.inc"
 PATTERN "*.h"
 )
 
+  # Installing the headers needs to depend on generating any public
+  # tablegen'd headers.
+  add_custom_target(clang-headers DEPENDS clang-tablegen-targets)
+  set_target_properties(clang-headers PROPERTIES FOLDER "Misc")
+  if(NOT LLVM_ENABLE_IDE)
+add_llvm_install_targets(install-clang-headers
+ DEPENDS clang-headers
+ COMPONENT clang-headers)
+  endif()
+
   install(PROGRAMS utils/bash-autocomplete.sh
 DESTINATION share/clang
 )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r355853 - [clang] Add install targets for API headers

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Mon Mar 11 11:53:57 2019
New Revision: 355853

URL: http://llvm.org/viewvc/llvm-project?rev=355853=rev
Log:
[clang] Add install targets for API headers

Add an install target for clang's API headers, which allows them to be
included in distributions. The install rules already existed, but they
lacked a component and a target, making them only accessible via a full
install. These headers are useful for writing clang-based tooling, for
example. They're the clang equivalent to the llvm-headers target and
complement the clang-libraries target.

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

Modified:
cfe/trunk/CMakeLists.txt
cfe/trunk/docs/ReleaseNotes.rst

Modified: cfe/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=355853=355852=355853=diff
==
--- cfe/trunk/CMakeLists.txt (original)
+++ cfe/trunk/CMakeLists.txt Mon Mar 11 11:53:57 2019
@@ -388,6 +388,7 @@ include_directories(BEFORE
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
   install(DIRECTORY include/clang include/clang-c
 DESTINATION include
+COMPONENT clang-headers
 FILES_MATCHING
 PATTERN "*.def"
 PATTERN "*.h"
@@ -397,12 +398,23 @@ if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
 
   install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/include/clang
 DESTINATION include
+COMPONENT clang-headers
 FILES_MATCHING
 PATTERN "CMakeFiles" EXCLUDE
 PATTERN "*.inc"
 PATTERN "*.h"
 )
 
+  # Installing the headers needs to depend on generating any public
+  # tablegen'd headers.
+  add_custom_target(clang-headers DEPENDS clang-tablegen-targets)
+  set_target_properties(clang-headers PROPERTIES FOLDER "Misc")
+  if(NOT LLVM_ENABLE_IDE)
+add_llvm_install_targets(install-clang-headers
+ DEPENDS clang-headers
+ COMPONENT clang-headers)
+  endif()
+
   install(PROGRAMS utils/bash-autocomplete.sh
 DESTINATION share/clang
 )

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=355853=355852=355853=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Mon Mar 11 11:53:57 2019
@@ -156,7 +156,9 @@ release of Clang. Users of the build sys
 - In 8.0.0 and below, the install-clang-headers target would install clang's
   resource directory headers. This installation is now performed by the
   install-clang-resource-headers target. Users of the old install-clang-headers
-  target should switch to the new install-clang-resource-headers target.
+  target should switch to the new install-clang-resource-headers target. The
+  install-clang-headers target now installs clang's API headers (corresponding
+  to its libraries), which is consistent with the install-llvm-headers target.
 
 -  ...
 


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

[PATCH] D59118: creduce script for clang crashes

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

The new diffs should not be relative to the previously uploaded diff, but to 
the git master/svn trunk.


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

https://reviews.llvm.org/D59118



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

[PATCH] D59118: creduce script for clang crashes

[Amy Huang via Phabricator via cfe-commits]

akhuang added inline comments.



Comment at: clang/utils/creduce-clang-crash.py:43
+  # Get crash output
+  p = subprocess.Popen(build_script,
+   stdout=subprocess.PIPE,

george.burgess.iv wrote:
> nit: can replace with `subprocess.check_output` unless we explicitly want to 
> ignore the return value (in which case, we should probably still call 
> `wait()` anyway?)
`check_output()` raises an error when the return code is nonzero, which it is 
in this case. I think `communicate()` calls `wait()`, though.



Comment at: clang/utils/creduce-clang-crash.py:49
+  output = ['#!/bin/bash']
+  output.append('%s --crash %s >& t.log || exit 1' % (llvm_not, cmd))
+

george.burgess.iv wrote:
> please `pipes.quote(llvm_not)` and `pipes.quote(cmd)`
`cmd` is already quoted, since it's read out of another file


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

https://reviews.llvm.org/D59118



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

[PATCH] D59118: creduce script for clang crashes

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 190126.
akhuang marked 15 inline comments as done.
akhuang added a comment.

Addressed readability comments


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

https://reviews.llvm.org/D59118

Files:
  clang/utils/creduce-clang-crash.py


Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -8,6 +8,7 @@
 import stat
 import sys
 import subprocess
+import pipes
 
 def is_exe(fpath):
   return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
@@ -33,11 +34,9 @@
   Return as a string.
   """
   # Get clang call from build script
-  cmd = None
-  with open(build_script, 'r') as f:
-cmd = f.read()
-cmd = re.sub("#!.*\n", "", cmd)
-cmd = cmd.rstrip('\n\r')
+  # Assumes the call is the last line of the script
+  with open(build_script) as f:
+cmd = f.readlines()[-1].rstrip('\n\r')
 
   # Get crash output
   p = subprocess.Popen(build_script,
@@ -46,21 +45,22 @@
   crash_output, _ = p.communicate()
 
   output = ['#!/bin/bash']
-  output.append('%s --crash %s >& t.log || exit 1' % (llvm_not, cmd))
+  output.append('%s --crash %s >& t.log || exit 1' % (pipes.quote(llvm_not),
+  cmd))
 
   # Add messages from crash output to the test
   # If there is an Assertion failure, use that; otherwise use the
   # last five stack trace functions
-  assertion_re = "Assertion \`([^\']+)\' failed"
+  assertion_re = r'Assertion `([^\']+)\' failed'
   assertion_match = re.search(assertion_re, crash_output)
   if assertion_match:
-msg = assertion_match.group(1).replace('"', '\\"')
-output.append('grep "%s" t.log || exit 1' % msg)
+msg = assertion_match.group(1)
+output.append('grep %s t.log || exit 1' % pipes.quote(msg))
   else:
-stacktrace_re = "#[0-9]+\s+0[xX][0-9a-fA-F]+\s*([^(]+)\("
+stacktrace_re = r'#[0-9]+\s+0[xX][0-9a-fA-F]+\s*([^(]+)\('
 matches = re.findall(stacktrace_re, crash_output)
-del matches[:len(matches)-5]
-output += ['grep "%s" t.log || exit 1' % msg for msg in matches]
+del matches[:-5]
+output += ['grep %s t.log || exit 1' % pipes.quote(msg) for msg in matches]
 
   return output
 
@@ -73,11 +73,11 @@
   parser.add_argument('-o', '--output', dest='output', type=str,
   help='Name of the output file for the reduction. 
Optional.')
   parser.add_argument('--llvm-not', dest='llvm_not', type=str,
-  help="""The path to the llvm-not executable.
-  Required if 'not' is not in PATH environment.""");
+  help="The path to the llvm-not executable. "
+  "Required if 'not' is not in PATH environment.");
   parser.add_argument('--creduce', dest='creduce', type=str,
-  help="""The path to the C-Reduce executable.
-  Required if 'creduce' is not in PATH environment.""");
+  help="The path to the C-Reduce executable. "
+  "Required if 'creduce' is not in PATH environment.");
   args = parser.parse_args()
 
   build_script = os.path.abspath(args.build_script[0])
@@ -105,19 +105,11 @@
   test_contents = create_test(build_script, llvm_not)
   testname, _ = os.path.splitext(file_to_reduce)
   testfile = testname + '.test.sh'
-  open(testfile, 'w').write('\n'.join(test_contents))
+  with open(testfile, 'w') as f:
+f.write('\n'.join(test_contents))
   os.chmod(testfile, os.stat(testfile).st_mode | stat.S_IEXEC)
 
-  # Call C-Reduce
-  try:
-p = subprocess.Popen([creduce, testfile, file_to_reduce])
-p.communicate()
-
-  except KeyboardInterrupt:
-print('\n\nctrl-c detected, killed creduce')
-p.kill()
-
-  sys.exit(0)
+  sys.exit(subprocess.call([creduce, testfile, file_to_reduce]))
 
 if __name__ == '__main__':
   main()


Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -8,6 +8,7 @@
 import stat
 import sys
 import subprocess
+import pipes
 
 def is_exe(fpath):
   return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
@@ -33,11 +34,9 @@
   Return as a string.
   """
   # Get clang call from build script
-  cmd = None
-  with open(build_script, 'r') as f:
-cmd = f.read()
-cmd = re.sub("#!.*\n", "", cmd)
-cmd = cmd.rstrip('\n\r')
+  # Assumes the call is the last line of the script
+  with open(build_script) as f:
+cmd = f.readlines()[-1].rstrip('\n\r')
 
   # Get crash output
   p = subprocess.Popen(build_script,
@@ -46,21 +45,22 @@
   crash_output, _ = p.communicate()
 
   output = ['#!/bin/bash']
-  output.append('%s --crash %s >& t.log || exit 1' % (llvm_not, cmd))
+  output.append('%s --crash %s >& t.log || exit 1' % (pipes.quote(llvm_not),
+   

[PATCH] D45978: dllexport const variables must have external linkage.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 190124.
zahiraam marked an inline comment as done.

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

https://reviews.llvm.org/D45978

Files:
  lib/Sema/SemaDecl.cpp
  test/CodeGen/dllexport-1.c
  test/Sema/dllexport-1.cpp
  test/Sema/dllexport-2.cpp
  test/SemaCXX/dllexport.cpp
  test/SemaCXX/dllimport.cpp

Index: test/SemaCXX/dllimport.cpp
===
--- test/SemaCXX/dllimport.cpp
+++ test/SemaCXX/dllimport.cpp
@@ -121,7 +121,9 @@
 // External linkage is required.
 __declspec(dllimport) static int StaticGlobal; // expected-error{{'StaticGlobal' must have external linkage when declared 'dllimport'}}
 __declspec(dllimport) Internal InternalTypeGlobal; // expected-error{{'InternalTypeGlobal' must have external linkage when declared 'dllimport'}}
+#ifndef MS
 namespace{ __declspec(dllimport) int InternalGlobal; } // expected-error{{'(anonymous namespace)::InternalGlobal' must have external linkage when declared 'dllimport'}}
+#endif
 namespace ns { __declspec(dllimport) int ExternalGlobal; }
 
 __declspec(dllimport) auto InternalAutoTypeGlobal = Internal(); // expected-error{{'InternalAutoTypeGlobal' must have external linkage when declared 'dllimport'}}
@@ -213,7 +215,9 @@
 // External linkage is required.
 template __declspec(dllimport) static int StaticVarTmpl; // expected-error{{'StaticVarTmpl' must have external linkage when declared 'dllimport'}}
 template __declspec(dllimport) Internal InternalTypeVarTmpl; // expected-error{{'InternalTypeVarTmpl' must have external linkage when declared 'dllimport'}}
+#ifndef MS
 namespace{ template __declspec(dllimport) int InternalVarTmpl; } // expected-error{{'(anonymous namespace)::InternalVarTmpl' must have external linkage when declared 'dllimport'}}
+#endif
 namespace ns { template __declspec(dllimport) int ExternalVarTmpl; }
 
 template __declspec(dllimport) auto InternalAutoTypeVarTmpl = Internal(); // expected-error{{definition of dllimport data}} // expected-error{{'InternalAutoTypeVarTmpl' must have external linkage when declared 'dllimport'}}
Index: test/SemaCXX/dllexport.cpp
===
--- test/SemaCXX/dllexport.cpp
+++ test/SemaCXX/dllexport.cpp
@@ -69,7 +69,9 @@
 // External linkage is required.
 __declspec(dllexport) static int StaticGlobal; // expected-error{{'StaticGlobal' must have external linkage when declared 'dllexport'}}
 __declspec(dllexport) Internal InternalTypeGlobal; // expected-error{{'InternalTypeGlobal' must have external linkage when declared 'dllexport'}}
+#ifndef MS
 namespace{ __declspec(dllexport) int InternalGlobal; } // expected-error{{'(anonymous namespace)::InternalGlobal' must have external linkage when declared 'dllexport'}}
+#endif
 namespace ns { __declspec(dllexport) int ExternalGlobal; }
 
 __declspec(dllexport) auto InternalAutoTypeGlobal = Internal(); // expected-error{{'InternalAutoTypeGlobal' must have external linkage when declared 'dllexport'}}
@@ -124,7 +126,9 @@
 // External linkage is required.
 template __declspec(dllexport) static int StaticVarTmpl; // expected-error{{'StaticVarTmpl' must have external linkage when declared 'dllexport'}}
 template __declspec(dllexport) Internal InternalTypeVarTmpl; // expected-error{{'InternalTypeVarTmpl' must have external linkage when declared 'dllexport'}}
+#ifndef MS
 namespace{ template __declspec(dllexport) int InternalVarTmpl; } // expected-error{{'(anonymous namespace)::InternalVarTmpl' must have external linkage when declared 'dllexport'}}
+#endif
 namespace ns { template __declspec(dllexport) int ExternalVarTmpl = 1; }
 
 template __declspec(dllexport) auto InternalAutoTypeVarTmpl = Internal(); // expected-error{{'InternalAutoTypeVarTmpl' must have external linkage when declared 'dllexport'}}
Index: test/Sema/dllexport-2.cpp
===
--- /dev/null
+++ test/Sema/dllexport-2.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -fms-extensions -verify %s
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsyntax-only -fms-extensions -verify %s -DMSVC
+
+// Export const variable.
+
+#ifdef MSVC
+// expected-error@+4 {{'j' must have external linkage when declared 'dllexport'}}
+#else
+// expected-warning@+2 {{__declspec attribute 'dllexport' is not supported}}
+#endif
+__declspec(dllexport) int const j; // expected-error {{default initialization of an object of const type 'const int'}}
+
+// With typedef
+typedef const int CInt;
+
+#ifdef MSVC
+// expected-error@+4 {{'j2' must have external linkage when declared 'dllexport'}}
+#else
+// expected-warning@+2 {{__declspec attribute 'dllexport' is not supported}}
+#endif
+__declspec(dllexport) CInt j2; //expected-error {{default initialization of an object of const type 'CInt'}}
+
+#ifndef MSVC
+// expected-warning@+2 {{__declspec attribute 'dllexport' is 

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

And one more comment that hasn't yet been addressed: "Some patterns are covered 
by compiler diagnostics: https://godbolt.org/g/HvsjnP. Is there any benefit in 
re-implementing them?"

And one more: please update file headers. The license has changed since.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D48866



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

[PATCH] D45978: dllexport const variables must have external linkage.

[Zahira Ammarguellat via Phabricator via cfe-commits]

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



Comment at: test/SemaCXX/dllexport.cpp:72-74
+#ifndef MS
 namespace{ __declspec(dllexport) int InternalGlobal; } // 
expected-error{{'(anonymous namespace)::InternalGlobal' must have external 
linkage when declared 'dllexport'}}
+#endif

aaron.ballman wrote:
> I don't have a copy of mingw handy -- can you test that this behavior matches 
> the behavior when targeting mingw32 with GCC? Might also be a good idea to 
> test cygwin as a target as well. I would have thought that they would behave 
> the same as MSVC, but I can't easily test it myself.
Tested it with a set of combinations. The test is passing... 


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

https://reviews.llvm.org/D45978



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

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

[Alexander Kornienko via Phabricator via cfe-commits]

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

As per the very first comment, "bugprone" will be a more suitable category for 
this check. The rename_check.py script should be able to help here.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D48866



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

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: erik.pilkington.

This adds support for static_assert() (and _Static_assert()) in 
@interface/@implementation ivar lists and in @interface method declarations.

It was already supported in @implementation blocks outside of the ivar lists.

The assert AST nodes are added at file scoped, matching where other 
(non-Objective-C) declarations at @interface / @implementation level go (cf 
`allTUVariables`).

Also add a `__has_feature(objc_static_assert)` that's always true after this 
patch, so it's possible to check if this is supported.


https://reviews.llvm.org/D59223

Files:
  clang/include/clang/Basic/Features.def
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Parser/objc-static-assert.m
  clang/test/Parser/objc-static-assert.mm

Index: clang/test/Parser/objc-static-assert.mm
===
--- /dev/null
+++ clang/test/Parser/objc-static-assert.mm
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+
+#if !__has_feature(objc_static_assert)
+#error failed
+#endif
+
+@interface A {
+  int a;
+  static_assert(1, "");
+  _Static_assert(1, "");
+
+  static_assert(0, ""); // expected-error {{static_assert failed}}
+  _Static_assert(0, ""); // expected-error {{static_assert failed}}
+
+  static_assert(a, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+  static_assert(sizeof(a) == 4, "");
+  static_assert(sizeof(a) == 3, ""); // expected-error {{static_assert failed}}
+}
+
+static_assert(1, "");
+_Static_assert(1, "");
+
+- (void)f;
+@end
+
+@implementation A {
+  int b;
+  static_assert(1, "");
+  _Static_assert(1, "");
+  static_assert(sizeof(b) == 4, "");
+  static_assert(sizeof(b) == 3, ""); // expected-error {{static_assert failed}}
+}
+
+static_assert(1, "");
+
+- (void)f {
+  static_assert(1, "");
+}
+@end
+
+@interface B
+@end
+
+@interface B () {
+  int b;
+  static_assert(sizeof(b) == 4, "");
+  static_assert(sizeof(b) == 3, ""); // expected-error {{static_assert failed}}
+}
+@end
Index: clang/test/Parser/objc-static-assert.m
===
--- /dev/null
+++ clang/test/Parser/objc-static-assert.m
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fobjc-runtime=macosx-fragile -fsyntax-only -verify -Wno-objc-root-class %s
+
+#if !__has_feature(objc_static_assert)
+#error failed
+#endif
+
+@interface A {
+  int a;
+  _Static_assert(1, "");
+  _Static_assert(0, ""); // expected-error {{static_assert failed}}
+
+  _Static_assert(a, ""); // expected-error {{use of undeclared identifier 'a'}}
+  _Static_assert(sizeof(a), ""); // expected-error {{use of undeclared identifier 'a'}}
+}
+
+_Static_assert(1, "");
+
+@end
+
+struct S {
+  @defs(A);
+};
+
+
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2998,7 +2998,6 @@
 
 // These shouldn't make it here.
 case Decl::ObjCAtDefsField:
-case Decl::ObjCIvar:
   llvm_unreachable("forming non-member reference to ivar?");
 
 // Enum constants are always r-values and never references.
@@ -3016,6 +3015,7 @@
 // exist in the high-level semantics.
 case Decl::Field:
 case Decl::IndirectField:
+case Decl::ObjCIvar:
   assert(getLangOpts().CPlusPlus &&
  "building reference to field in C?");
 
Index: clang/lib/Parse/ParseObjc.cpp
===
--- clang/lib/Parse/ParseObjc.cpp
+++ clang/lib/Parse/ParseObjc.cpp
@@ -623,6 +623,8 @@
 }
 // Ignore excess semicolons.
 if (Tok.is(tok::semi)) {
+  // FIXME: This should use ConsumeExtraSemi() for extraneous semicolons,
+  // to make -Wextra-semi diagnose them.
   ConsumeToken();
   continue;
 }
@@ -646,7 +648,19 @@
   // erroneous r_brace would cause an infinite loop if not handled here.
   if (Tok.is(tok::r_brace))
 break;
+
   ParsedAttributesWithRange attrs(AttrFactory);
+
+  // Since we call ParseDeclarationOrFunctionDefinition() instead of
+  // ParseExternalDeclaration() below (so that this doesn't parse nested
+  // @interfaces), this needs to duplicate some code from the latter.
+  if (Tok.isOneOf(tok::kw_static_assert, tok::kw__Static_assert)) {
+SourceLocation DeclEnd;
+allTUVariables.push_back(
+ParseDeclaration(DeclaratorContext::FileContext, DeclEnd, attrs));
+continue;
+  }
+
   allTUVariables.push_back(ParseDeclarationOrFunctionDefinition(attrs));
   continue;
 }
@@ -1875,6 +1889,7 @@
 /// ';'
 /// objc-instance-variable-decl-list objc-visibility-spec
 /// objc-instance-variable-decl-list objc-instance-variable-decl ';'
+/// objc-instance-variable-decl-list static_assert-declaration
 

[PATCH] D59197: [NFC][clang][PCH][ObjC] Add some missing `VisitStmt(S); `

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D59197#1424832 , @jordan_rose wrote:

> I'm no Clang serialization expert but this makes sense to me. It's consistent 
> with all the other statement visitor methods.


Thank you.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59197



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

[PATCH] D59176: Modules: Add LangOptions::CacheGeneratedPCH

[Jordan Rose via Phabricator via cfe-commits]

jordan_rose added a comment.

This commit by itself doesn't change any behavior, right?




Comment at: clang/lib/Frontend/FrontendActions.cpp:115
   CI.getPreprocessorOpts().AllowPCHWithCompilerErrors,
-  FrontendOpts.IncludeTimestamps));
+  FrontendOpts.IncludeTimestamps, +CI.getLangOpts().CacheGeneratedPCH));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(

What's the `+` for?



Comment at: clang/lib/Frontend/FrontendActions.cpp:182
+  CI.getFrontendOpts().BuildingImplicitModule &&
+  CI.getLangOpts().isCompilingModule()));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(

Why is this the condition, as opposed to just "do this for all modules, don't 
do it for PCHs"? And doesn't `BuildingImplicitModule` imply 
`isCompilingModule()`? (Note that `BuildingImplicitModule` probably isn't the 
same as the original condition `ImplicitModules`.)


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

https://reviews.llvm.org/D59176



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

[PATCH] D59197: [NFC][clang][PCH][ObjC] Add some missing `VisitStmt(S); `

[Jordan Rose via Phabricator via cfe-commits]

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

I'm no Clang serialization expert but this makes sense to me. It's consistent 
with all the other statement visitor methods.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59197



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

[PATCH] D59219: [PR41007][OpenCL] Allow printf and toolchain reserved variadic functions in C++

[Neil Hickey via Phabricator via cfe-commits]

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

LGTM!


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

https://reviews.llvm.org/D59219



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

[PATCH] D59221: [asan] Add gcc 8's driver option -fsanitize=pointer-compare and -fsanitize=pointer-substract.

[pierre gousseau via Phabricator via cfe-commits]

pgousseau created this revision.
pgousseau added reviewers: rsmith, filcab, wristow, gbedwell, kcc, riccibruno, 
probinson.
Herald added subscribers: cfe-commits, jdoerfert, emaste.
Herald added a project: clang.

Disabled by default as this is still an experimental feature.
This is the clang side of https://reviews.llvm.org/D59220


Repository:
  rC Clang

https://reviews.llvm.org/D59221

Files:
  include/clang/Basic/Sanitizers.def
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChains/CrossWindows.cpp
  lib/Driver/ToolChains/Darwin.cpp
  lib/Driver/ToolChains/FreeBSD.cpp
  lib/Driver/ToolChains/Fuchsia.cpp
  lib/Driver/ToolChains/Linux.cpp
  lib/Driver/ToolChains/MSVC.cpp
  lib/Driver/ToolChains/MinGW.cpp
  lib/Driver/ToolChains/NetBSD.cpp
  lib/Driver/ToolChains/PS4CPU.cpp
  lib/Driver/ToolChains/Solaris.cpp
  test/Driver/fsanitize.c

Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -884,3 +884,14 @@
 // CHECK-HWASAN-INTERCEPTOR-ABI: "-default-function-attr" "hwasan-abi=interceptor"
 // CHECK-HWASAN-PLATFORM-ABI: "-default-function-attr" "hwasan-abi=platform"
 // CHECK-HWASAN-FOO-ABI: error: invalid value 'foo' in '-fsanitize-hwaddress-abi=foo'
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address,pointer-compare,pointer-subtract %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POINTER-ALL
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=pointer-compare %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POINTER-CMP-NEEDS-ADDRESS
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=pointer-subtract %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POINTER-SUB-NEEDS-ADDRESS
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=pointer-subtract -fno-sanitize=pointer-subtract %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-POINTER-SUB
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=pointer-compare -fno-sanitize=pointer-compare %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-POINTER-CMP
+// CHECK-POINTER-ALL: "-cc1{{.*}}-fsanitize={{.*}}pointer-compare,pointer-subtract{{.*}}"
+// CHECK-POINTER-CMP-NEEDS-ADDRESS: error: invalid argument '-fsanitize=pointer-compare' only allowed with '-fsanitize=address'
+// CHECK-POINTER-SUB-NEEDS-ADDRESS: error: invalid argument '-fsanitize=pointer-subtract' only allowed with '-fsanitize=address'
+// CHECK-NO-POINTER-SUB-NOT: "{{.*}}-fsanitize{{.*}}"
+// CHECK-NO-POINTER-CMP-NOT: "{{.*}}-fsanitize{{.*}}"
Index: lib/Driver/ToolChains/Solaris.cpp
===
--- lib/Driver/ToolChains/Solaris.cpp
+++ lib/Driver/ToolChains/Solaris.cpp
@@ -199,6 +199,8 @@
   // FIXME: Omit X86_64 until 64-bit support is figured out.
   if (IsX86) {
 Res |= SanitizerKind::Address;
+Res |= SanitizerKind::PointerCompare;
+Res |= SanitizerKind::PointerSubtract;
   }
   Res |= SanitizerKind::Vptr;
   return Res;
Index: lib/Driver/ToolChains/PS4CPU.cpp
===
--- lib/Driver/ToolChains/PS4CPU.cpp
+++ lib/Driver/ToolChains/PS4CPU.cpp
@@ -425,6 +425,8 @@
 SanitizerMask toolchains::PS4CPU::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res |= SanitizerKind::PointerCompare;
+  Res |= SanitizerKind::PointerSubtract;
   Res |= SanitizerKind::Vptr;
   return Res;
 }
Index: lib/Driver/ToolChains/NetBSD.cpp
===
--- lib/Driver/ToolChains/NetBSD.cpp
+++ lib/Driver/ToolChains/NetBSD.cpp
@@ -463,6 +463,8 @@
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   if (IsX86 || IsX86_64) {
 Res |= SanitizerKind::Address;
+Res |= SanitizerKind::PointerCompare;
+Res |= SanitizerKind::PointerSubtract;
 Res |= SanitizerKind::Function;
 Res |= SanitizerKind::Leak;
 Res |= SanitizerKind::SafeStack;
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -459,6 +459,8 @@
 SanitizerMask toolchains::MinGW::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res |= SanitizerKind::PointerCompare;
+  Res |= SanitizerKind::PointerSubtract;
   return Res;
 }
 
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -1317,6 +1317,8 @@
 SanitizerMask MSVCToolChain::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res |= SanitizerKind::PointerCompare;
+  Res |= SanitizerKind::PointerSubtract;
   Res |= SanitizerKind::Fuzzer;
   Res |= SanitizerKind::FuzzerNoLink;
   Res &= 

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

[Hyrum Wright via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE355835: [clang-tidy] Add the abseil-time-compare check 
(authored by hwright, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58977?vs=190075=190113#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58977

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationComparisonCheck.cpp
  clang-tidy/abseil/DurationRewriter.h
  clang-tidy/abseil/TimeComparisonCheck.cpp
  clang-tidy/abseil/TimeComparisonCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-time-comparison.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-time-comparison.cpp

Index: clang-tidy/abseil/CMakeLists.txt
===
--- clang-tidy/abseil/CMakeLists.txt
+++ clang-tidy/abseil/CMakeLists.txt
@@ -17,6 +17,7 @@
   RedundantStrcatCallsCheck.cpp
   StrCatAppendCheck.cpp
   StringFindStartswithCheck.cpp
+  TimeComparisonCheck.cpp
   TimeSubtractionCheck.cpp
   UpgradeDurationConversionsCheck.cpp
 
Index: clang-tidy/abseil/TimeComparisonCheck.h
===
--- clang-tidy/abseil/TimeComparisonCheck.h
+++ clang-tidy/abseil/TimeComparisonCheck.h
@@ -0,0 +1,35 @@
+//===--- TimeComparisonCheck.h - clang-tidy -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_TIMECOMPARECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_TIMECOMPARECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Prefer comparison in the `absl::Time` domain instead of the numeric
+/// domain.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-time-comparison.html
+class TimeComparisonCheck : public ClangTidyCheck {
+public:
+  TimeComparisonCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_TIMECOMPARECHECK_H
Index: clang-tidy/abseil/DurationComparisonCheck.cpp
===
--- clang-tidy/abseil/DurationComparisonCheck.cpp
+++ clang-tidy/abseil/DurationComparisonCheck.cpp
@@ -19,14 +19,10 @@
 namespace abseil {
 
 void DurationComparisonCheck::registerMatchers(MatchFinder *Finder) {
-  auto Matcher =
-  binaryOperator(anyOf(hasOperatorName(">"), hasOperatorName(">="),
-   hasOperatorName("=="), hasOperatorName("<="),
-   hasOperatorName("<")),
- hasEitherOperand(ignoringImpCasts(callExpr(
- callee(functionDecl(DurationConversionFunction())
-.bind("function_decl"))
-  .bind("binop");
+  auto Matcher = expr(comparisonOperatorWithCallee(functionDecl(
+  functionDecl(DurationConversionFunction())
+  .bind("function_decl"
+ .bind("binop");
 
   Finder->addMatcher(Matcher, this);
 }
Index: clang-tidy/abseil/AbseilTidyModule.cpp
===
--- clang-tidy/abseil/AbseilTidyModule.cpp
+++ clang-tidy/abseil/AbseilTidyModule.cpp
@@ -23,6 +23,7 @@
 #include "RedundantStrcatCallsCheck.h"
 #include "StringFindStartswithCheck.h"
 #include "StrCatAppendCheck.h"
+#include "TimeComparisonCheck.h"
 #include "TimeSubtractionCheck.h"
 #include "UpgradeDurationConversionsCheck.h"
 
@@ -60,6 +61,8 @@
 "abseil-str-cat-append");
 CheckFactories.registerCheck(
 "abseil-string-find-startswith");
+CheckFactories.registerCheck(
+"abseil-time-comparison");
 CheckFactories.registerCheck(
 "abseil-time-subtraction");
 CheckFactories.registerCheck(
Index: clang-tidy/abseil/TimeComparisonCheck.cpp
===
--- clang-tidy/abseil/TimeComparisonCheck.cpp
+++ clang-tidy/abseil/TimeComparisonCheck.cpp
@@ -0,0 +1,61 @@
+//===--- TimeComparisonCheck.cpp - clang-tidy
+//===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// 

[clang-tools-extra] r355835 - [clang-tidy] Add the abseil-time-compare check

[Hyrum Wright via cfe-commits]

Author: hwright
Date: Mon Mar 11 09:47:45 2019
New Revision: 355835

URL: http://llvm.org/viewvc/llvm-project?rev=355835=rev
Log:
[clang-tidy] Add the abseil-time-compare check

This is an analog of the abseil-duration-comparison check, but for the
absl::Time domain. It has a similar implementation and tests.

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

Added:
clang-tools-extra/trunk/clang-tidy/abseil/TimeComparisonCheck.cpp
  - copied, changed from r355820, 
clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp
clang-tools-extra/trunk/clang-tidy/abseil/TimeComparisonCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-time-comparison.rst
clang-tools-extra/trunk/test/clang-tidy/abseil-time-comparison.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp
clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=355835=355834=355835=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Mon Mar 11 
09:47:45 2019
@@ -23,6 +23,7 @@
 #include "RedundantStrcatCallsCheck.h"
 #include "StringFindStartswithCheck.h"
 #include "StrCatAppendCheck.h"
+#include "TimeComparisonCheck.h"
 #include "TimeSubtractionCheck.h"
 #include "UpgradeDurationConversionsCheck.h"
 
@@ -60,6 +61,8 @@ public:
 "abseil-str-cat-append");
 CheckFactories.registerCheck(
 "abseil-string-find-startswith");
+CheckFactories.registerCheck(
+"abseil-time-comparison");
 CheckFactories.registerCheck(
 "abseil-time-subtraction");
 CheckFactories.registerCheck(

Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=355835=355834=355835=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Mon Mar 11 
09:47:45 2019
@@ -17,6 +17,7 @@ add_clang_library(clangTidyAbseilModule
   RedundantStrcatCallsCheck.cpp
   StrCatAppendCheck.cpp
   StringFindStartswithCheck.cpp
+  TimeComparisonCheck.cpp
   TimeSubtractionCheck.cpp
   UpgradeDurationConversionsCheck.cpp
 

Modified: clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp?rev=355835=355834=355835=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp Mon 
Mar 11 09:47:45 2019
@@ -19,14 +19,10 @@ namespace tidy {
 namespace abseil {
 
 void DurationComparisonCheck::registerMatchers(MatchFinder *Finder) {
-  auto Matcher =
-  binaryOperator(anyOf(hasOperatorName(">"), hasOperatorName(">="),
-   hasOperatorName("=="), hasOperatorName("<="),
-   hasOperatorName("<")),
- hasEitherOperand(ignoringImpCasts(callExpr(
- callee(functionDecl(DurationConversionFunction())
-.bind("function_decl"))
-  .bind("binop");
+  auto Matcher = expr(comparisonOperatorWithCallee(functionDecl(
+  functionDecl(DurationConversionFunction())
+  .bind("function_decl"
+ .bind("binop");
 
   Finder->addMatcher(Matcher, this);
 }

Modified: clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h?rev=355835=355834=355835=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h Mon Mar 11 
09:47:45 2019
@@ -124,6 +124,16 @@ AST_MATCHER_FUNCTION(ast_matchers::inter
   "::absl::ToUnixMillis", "::absl::ToUnixMicros", "::absl::ToUnixNanos"));
 }
 
+AST_MATCHER_FUNCTION_P(ast_matchers::internal::Matcher,
+   comparisonOperatorWithCallee,
+   ast_matchers::internal::Matcher, funcDecl) {
+  using 

[PATCH] D59214: [clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: include/clang/AST/StmtOpenMP.h:335
 
+  llvm::Optional getStructuredBlockImpl() const {
+return const_cast(getInnermostCapturedStmt()->getCapturedStmt());

No need to insert it into each class, just add:
```
Stmt * OMPExecutableDirective::getStructuredBlock() const {
  if (!hasAssociatedStmt() || !getAssociatedStmt())
return nullptr;
  if (auto *LD = dyn_cast(this))
return LD->getBody();
  return getInnermostCapturedStmt()->getCapturedStmt();
}
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D59214



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

[PATCH] D31739: Add markup for libc++ dylib availability

[Louis Dionne via Phabricator via cfe-commits]

ldionne added inline comments.



Comment at: 
libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/lit.local.cfg:1
+if 'availability' in config.available_features:
+config.unsupported = True

mehdi_amini wrote:
> thakis wrote:
> > Did you mean `not in` here? I don't understand why we want to skip this 
> > tests is availability annotations are present.
> Thanks for looking into this, unfortunately I don't remember the context from 
> 2 years ago.  It may have been a typo indeed.
> 
> @ldionne and @Bigcheese  may be able to help here!
Hmm, I wasn't aware of this. I guess this is because `shared_***` used to be 
marked as unconditionally unavailable, but that changed. I'll figure out when 
we can enable these tests.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D31739



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

[PATCH] D59219: [PR41007][OpenCL] Allow printf and toolchain reserved variadic functions in C++

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added a reviewer: neil.hickey.
Herald added subscribers: ebevhan, yaxunl.

As for OpenCL C, we will allow `printf` and other variadic functions (prefixed 
by "__") in C++ mode.


https://reviews.llvm.org/D59219

Files:
  lib/Sema/SemaType.cpp
  test/SemaOpenCL/extensions.cl


Index: test/SemaOpenCL/extensions.cl
===
--- test/SemaOpenCL/extensions.cl
+++ test/SemaOpenCL/extensions.cl
@@ -28,7 +28,7 @@
 // enabled by default with -cl-std=CL2.0).
 //
 // RUN: %clang_cc1 %s -triple amdgcn-unknown-unknown -verify -pedantic 
-fsyntax-only -cl-std=CL2.0 -finclude-default-header
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic 
-fsyntax-only -cl-std=c++
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic 
-fsyntax-only -cl-std=c++ -finclude-default-header
 
 #ifdef _OPENCL_H_
 // expected-no-diagnostics
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -4585,7 +4585,7 @@
 if (FTI.isVariadic &&
 !(D.getIdentifier() &&
   ((D.getIdentifier()->getName() == "printf" &&
-LangOpts.OpenCLVersion >= 120) ||
+(LangOpts.OpenCLCPlusPlus || LangOpts.OpenCLVersion >= 120)) ||
D.getIdentifier()->getName().startswith("__" {
   S.Diag(D.getIdentifierLoc(), diag::err_opencl_variadic_function);
   D.setInvalidType(true);


Index: test/SemaOpenCL/extensions.cl
===
--- test/SemaOpenCL/extensions.cl
+++ test/SemaOpenCL/extensions.cl
@@ -28,7 +28,7 @@
 // enabled by default with -cl-std=CL2.0).
 //
 // RUN: %clang_cc1 %s -triple amdgcn-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=CL2.0 -finclude-default-header
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=c++
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=c++ -finclude-default-header
 
 #ifdef _OPENCL_H_
 // expected-no-diagnostics
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -4585,7 +4585,7 @@
 if (FTI.isVariadic &&
 !(D.getIdentifier() &&
   ((D.getIdentifier()->getName() == "printf" &&
-LangOpts.OpenCLVersion >= 120) ||
+(LangOpts.OpenCLCPlusPlus || LangOpts.OpenCLVersion >= 120)) ||
D.getIdentifier()->getName().startswith("__" {
   S.Diag(D.getIdentifierLoc(), diag::err_opencl_variadic_function);
   D.setInvalidType(true);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31739: Add markup for libc++ dylib availability

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini marked an inline comment as done.
mehdi_amini added subscribers: ldionne, Bigcheese.
mehdi_amini added inline comments.



Comment at: 
libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/lit.local.cfg:1
+if 'availability' in config.available_features:
+config.unsupported = True

thakis wrote:
> Did you mean `not in` here? I don't understand why we want to skip this tests 
> is availability annotations are present.
Thanks for looking into this, unfortunately I don't remember the context from 2 
years ago.  It may have been a typo indeed.

@ldionne and @Bigcheese  may be able to help here!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D31739



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

[PATCH] D59197: [NFC][clang][PCH][ObjC] Add some missing `VisitStmt(S); `

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D59197#1424644 , @aprantl wrote:

> How can this change be NFC?


`VisitStmt()` is empty currently, there is currently no data serialized by the 
`Stmt` itself.
D59214  changes that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59197



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

[PATCH] D59210: clang-format: distinguish ObjC call subexpressions after r355434

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355831: clang-format: distinguish ObjC call subexpressions 
after r355434 (authored by krasimir, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59210

Files:
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp
  cfe/trunk/unittests/Format/FormatTestObjC.cpp


Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1401,6 +1401,8 @@
   if (!tryToParseLambdaIntroducer())
 return false;
 
+  bool SeenArrow = false;
+
   while (FormatTok->isNot(tok::l_brace)) {
 if (FormatTok->isSimpleTypeSpecifier()) {
   nextToken();
@@ -1423,8 +1425,19 @@
 case tok::coloncolon:
 case tok::kw_mutable:
 case tok::kw_noexcept:
+  nextToken();
+  break;
 // Specialization of a template with an integer parameter can contain
 // arithmetic, logical, comparison and ternary operators.
+//
+// FIXME: This also accepts sequences of operators that are not in the 
scope
+// of a template argument list.
+//
+// In a C++ lambda a template type can only occur after an arrow. We use
+// this as an heuristic to distinguish between Objective-C expressions
+// followed by an `a->b` expression, such as:
+// ([obj func:arg] + a->b)
+// Otherwise the code below would parse as a lambda.
 case tok::plus:
 case tok::minus:
 case tok::exclaim:
@@ -1444,13 +1457,17 @@
 case tok::colon:
 case tok::kw_true:
 case tok::kw_false:
-  nextToken();
-  break;
+  if (SeenArrow) {
+nextToken();
+break;
+  }
+  return true;
 case tok::arrow:
   // This might or might not actually be a lambda arrow (this could be an
   // ObjC method invocation followed by a dereferencing arrow). We might
   // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
+  SeenArrow = true;
   nextToken();
   break;
 default:
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -1329,6 +1329,34 @@
"   @\"f\"];");
 }
 
+TEST_F(FormatTestObjC, DisambiguatesCallsFromCppLambdas) {
+  verifyFormat("x = ([a foo:bar] && b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] + b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] + !b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] + ~b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] - b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] / b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] % b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] | b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] || b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] && b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] == b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] != b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] <= b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] >= b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] << b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] ? b->c == 'd' : 'e');");
+  // FIXME: The following are wrongly classified as C++ lambda expressions.
+  // For example this code:
+  //   x = ([a foo:bar] & b->c == 'd');
+  // is formatted as:
+  //   x = ([a foo:bar] & b -> c == 'd');
+  // verifyFormat("x = ([a foo:bar] & b->c == 'd');");
+  // verifyFormat("x = ([a foo:bar] > b->c == 'd');");
+  // verifyFormat("x = ([a foo:bar] < b->c == 'd');");
+  // verifyFormat("x = ([a foo:bar] >> b->c == 'd');");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang


Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1401,6 +1401,8 @@
   if (!tryToParseLambdaIntroducer())
 return false;
 
+  bool SeenArrow = false;
+
   while (FormatTok->isNot(tok::l_brace)) {
 if (FormatTok->isSimpleTypeSpecifier()) {
   nextToken();
@@ -1423,8 +1425,19 @@
 case tok::coloncolon:
 case tok::kw_mutable:
 case tok::kw_noexcept:
+  nextToken();
+  break;
 // Specialization of a template with an integer parameter can contain
 // arithmetic, logical, comparison and ternary operators.
+//
+// FIXME: This also accepts sequences of operators that are not in the scope
+// of a template argument list.
+//
+// In a C++ lambda a template type can only occur after an arrow. We use
+// this as an heuristic to distinguish 

[PATCH] D59214: [private][clang][OpeMP] Model OpenMP structured-block in AST (PR40563)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: ABataev, rjmccall, hfinkel, rsmith, riccibruno, 
gribozavr.
lebedev.ri added projects: clang, OpenMP.
Herald added subscribers: jdoerfert, jfb, guansong.

https://www.openmp.org/wp-content/uploads/OpenMP-API-Specification-5.0.pdf, 
page 3:

  structured block
  
  For C/C++, an executable statement, possibly compound, with a single entry at 
the
  top and a single exit at the bottom, or an OpenMP construct.
  
  COMMENT: See Section 2.1 on page 38 for restrictions on structured
  blocks.

  2.1 Directive Format
  
  Some executable directives include a structured block. A structured block:
  • may contain infinite loops where the point of exit is never reached;
  • may halt due to an IEEE exception;
  • may contain calls to exit(), _Exit(), quick_exit(), abort() or functions 
with a
  _Noreturn specifier (in C) or a noreturn attribute (in C/C++);
  • may be an expression statement, iteration statement, selection statement, 
or try block, provided
  that the corresponding compound statement obtained by enclosing it in { and } 
would be a
  structured block; and
  
  Restrictions
  Restrictions to structured blocks are as follows:
  • Entry to a structured block must not be the result of a branch.
  • The point of exit cannot be a branch out of the structured block.
  C / C++
  • The point of entry to a structured block must not be a call to setjmp().
  • longjmp() and throw() must not violate the entry/exit criteria.

Of particular note here is the fact that OpenMP structured blocks are as-if 
`noexcept`,
in the same sense as with the normal `noexcept` functions in C++.
I.e. if throw happens, and it attempts to travel out of the `noexcept` function
(here: out of the current structured-block), then the program terminates.

Now, one of course can say that since it is explicitly prohibited by the 
Specification,
then any and all programs that violate this Specification contain undefined 
behavior,
and are unspecified, and thus no one should care about them. Just don't write 
broken code /s

But i'm not sure this is a reasonable approach. 
I have personally had oss-fuzz issues of this origin - exception thrown inside
of an OpenMP structured-block that is not caught, thus causing program 
termination.
This issue isn't all that hard to catch, it's not any particularly different 
from
diagnosing the same situation with the normal `noexcept` function.

Now, clang static analyzer does not presently model exceptions.
But clang-tidy has a simplisic bugprone-exception-escape 
 
check,
and it is even refactored as a `ExceptionAnalyzer` class for reuse.
So it would be trivial to use that analyzer to check for
exceptions escaping out of OpenMP structured blocks. (patch to follow.)

All that sounds too great to be true. Indeed, there is a caveat.
Presently, it's practically impossible to do. To check a OpenMP structured block
you need to somehow 'get' the OpenMP structured block, and you can't because 
it's simply not modelled in AST. `CapturedStmt`/`CapturedDecl` is not it's 
representation.

Now, it is of course possible to write e.g. some AST matcher that would e.g.
match every OpenMP executable directive, and then return the whatever `Stmt` is
the structured block of said executable directive, if any.
But i said //practically//. This isn't practical for the following reasons:

1. This **will** bitrot. That matcher will need to be kept up-to-date, and 
refreshed with every new OpenMP spec version.
2. Every single piece of code that would want that knowledge would need to have 
such matcher. Well, okay, if it is an AST matcher, it could be shared. But then 
you still have `RecursiveASTVisitor` and friends. `2 > 1`, so now you have code 
duplication.

So it would be reasonable (and is fully within clang AST spirit) to not
force every single consumer to do that work, but instead store that knowledge
in the correct, and appropriate place - AST, class structure.

Now, there is another hoop we need to get through.
It isn't fully obvious //how// to model this.
The best solution would of course be to simply add a `OMPStructuredBlock` 
transparent
node. It would be optimal, it would give us two properties:

- Given this `OMPExecutableDirective`, what's it OpenMP structured block?
- It is trivial to  check whether the `Stmt*` is a OpenMP structured block 
(`isa(ptr)`)

But OpenMP structured block isn't **necessarily** the first, direct child of 
`OMP*Directive`.
(even ignoring the clang's `CapturedStmt`/`CapturedDecl` that were inserted 
inbetween).
So i'm not sure whether or not we could re-create AST statements after they 
were already created?
There would be other costs to a new AST node: 
https://bugs.llvm.org/show_bug.cgi?id=40563#c12

  1. You will need to break the representation of loops. The body should be 
replaced by the "structured block" entity.
  2. You will need to support 

r355831 - clang-format: distinguish ObjC call subexpressions after r355434

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Mon Mar 11 09:02:52 2019
New Revision: 355831

URL: http://llvm.org/viewvc/llvm-project?rev=355831=rev
Log:
clang-format: distinguish ObjC call subexpressions after r355434

Summary:
The revision r355434 had the unfortunate side-effect that it started to
recognize certain ObjC expressions with a call subexpression followed by a
`a->b` subexpression as C++ lambda expressions.

This patch adds a bit of logic to handle these cases and documents them in
tests.

The commented-out test cases in the new test suite are ones that were
problematic before r355434.

Reviewers: MyDeveloperDay, gribozavr

Reviewed By: MyDeveloperDay, gribozavr

Subscribers: MyDeveloperDay, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=355831=355830=355831=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Mon Mar 11 09:02:52 2019
@@ -1401,6 +1401,8 @@ bool UnwrappedLineParser::tryToParseLamb
   if (!tryToParseLambdaIntroducer())
 return false;
 
+  bool SeenArrow = false;
+
   while (FormatTok->isNot(tok::l_brace)) {
 if (FormatTok->isSimpleTypeSpecifier()) {
   nextToken();
@@ -1423,8 +1425,19 @@ bool UnwrappedLineParser::tryToParseLamb
 case tok::coloncolon:
 case tok::kw_mutable:
 case tok::kw_noexcept:
+  nextToken();
+  break;
 // Specialization of a template with an integer parameter can contain
 // arithmetic, logical, comparison and ternary operators.
+//
+// FIXME: This also accepts sequences of operators that are not in the 
scope
+// of a template argument list.
+//
+// In a C++ lambda a template type can only occur after an arrow. We use
+// this as an heuristic to distinguish between Objective-C expressions
+// followed by an `a->b` expression, such as:
+// ([obj func:arg] + a->b)
+// Otherwise the code below would parse as a lambda.
 case tok::plus:
 case tok::minus:
 case tok::exclaim:
@@ -1444,13 +1457,17 @@ bool UnwrappedLineParser::tryToParseLamb
 case tok::colon:
 case tok::kw_true:
 case tok::kw_false:
-  nextToken();
-  break;
+  if (SeenArrow) {
+nextToken();
+break;
+  }
+  return true;
 case tok::arrow:
   // This might or might not actually be a lambda arrow (this could be an
   // ObjC method invocation followed by a dereferencing arrow). We might
   // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
+  SeenArrow = true;
   nextToken();
   break;
 default:

Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=355831=355830=355831=diff
==
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Mon Mar 11 09:02:52 2019
@@ -1329,6 +1329,34 @@ TEST_F(FormatTestObjC, AlwaysBreakBefore
"   @\"f\"];");
 }
 
+TEST_F(FormatTestObjC, DisambiguatesCallsFromCppLambdas) {
+  verifyFormat("x = ([a foo:bar] && b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] + b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] + !b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] + ~b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] - b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] / b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] % b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] | b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] || b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] && b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] == b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] != b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] <= b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] >= b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] << b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] ? b->c == 'd' : 'e');");
+  // FIXME: The following are wrongly classified as C++ lambda expressions.
+  // For example this code:
+  //   x = ([a foo:bar] & b->c == 'd');
+  // is formatted as:
+  //   x = ([a foo:bar] & b -> c == 'd');
+  // verifyFormat("x = ([a foo:bar] & b->c == 'd');");
+  // verifyFormat("x = ([a foo:bar] > b->c == 'd');");
+  // verifyFormat("x = ([a foo:bar] < b->c == 'd');");
+  // verifyFormat("x = ([a foo:bar] >> b->c == 'd');");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang


___
cfe-commits mailing list

[PATCH] D59197: [NFC][clang][PCH][ObjC] Add some missing `VisitStmt(S); `

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

How can this change be NFC?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59197



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

[PATCH] D31739: Add markup for libc++ dylib availability

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.
Herald added subscribers: llvm-commits, jdoerfert, jfb, christof.
Herald added a reviewer: serge-sans-paille.
Herald added a project: LLVM.



Comment at: 
libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/lit.local.cfg:1
+if 'availability' in config.available_features:
+config.unsupported = True

Did you mean `not in` here? I don't understand why we want to skip this tests 
is availability annotations are present.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D31739



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

[PATCH] D59105: [RFC] Create an Arbitrary Precision Integer Type.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: lib/AST/ASTContext.cpp:1687-1693
+  // toCharUnitsFromBits always rounds down, which isn't a problem when the 
size
+  // Width is a multiple of chars, however ArbPrecInt makes this not a valid
+  // assumption.
+  return std::make_pair(toCharUnitsFromBits(Info.Width) +
+(Info.Width % getCharWidth() == 0
+ ? CharUnits::Zero()
+ : CharUnits::One()),

rsmith wrote:
> This should not be necessary. `getTypeInfo` should not be returning sizes 
> that are not a multiple of the bit-width of `char`.
Is this really an invariant of `getTypeInfo`? We have struck upon this issue 
downstream as well, as we have types which are not a multiple of the char size. 
The assumption in many places seems to be that when faced with scalar types, 
`getTypeInfo` does return the true bit width, not the width in chars*char 
width. `getTypeInfoInChars` obviously cannot handle this case, since it uses 
`getTypeInfo` as a basis for its calculation.

This is an issue for essentially any non-record, non-array type (such as 
typedefs or elaborated types) which consists of non-char-width types. Direct 
array types only work as there is a fast path (`getConstantArrayInfoInChars`) 
which does the per-element calculation correctly, and record types work because 
that calculation is delegated to the record layout builder.


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

https://reviews.llvm.org/D59105



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

[PATCH] D57855: [analyzer] Reimplement checker options

[Balogh , Ádám via Phabricator via cfe-commits]

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



Comment at: include/clang/StaticAnalyzer/Checkers/CheckerBase.td:49
+  string  PackageName = name;
+  list PackageOptions;
+  Package ParentPackage;

Please add a comment that this field is optional.



Comment at: include/clang/StaticAnalyzer/Checkers/CheckerBase.td:87
+  string  HelpText;
+  list CheckerOptions;
+  list   Dependencies;

Same as above.



Comment at: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h:156
+  return FullName == Rhs.FullName;
+}
+

Are we able now to distinguish checkers of the same short names but in 
different packages? In earlier versions the short name had to be different to 
avoid conflicts.



Comment at: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h:282
+  /// \c Checkers field. For convenience purposes.
+  llvm::StringMap PackageSizes;
 

Cannot we store the size of the `Package` in the `Package` itself?



Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:50
+  }
+};
+

Could we maybe use `std::less`?



Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:68
+CheckerOrPackageFullNameLT{}) &&
+ "Can't do binary search on a non-sorted collection!");
+

I would better keep the original message here. It has a more formal style.



Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:181
+  llvm::sort(Checkers, CheckerNameLT{});
+  llvm::sort(Packages, PackageNameLT{});
 

We always have packages first. Please swap these lines.



Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:307
+  assert(CheckerIt != Checkers.end() &&
+ "Failed to find the checker while attempting to set up it's "
+ "dependencies!");

Typo: it's -> its.



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

Please explain the problem with `std::lower_bound()`. I do not see why we 
cannot use it (followed by a check whether we found an element equal to the 
searched element, not a greater one). I cannot see any excuse to use linear 
search instead of binary one in a sorted collection.


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

https://reviews.llvm.org/D57855



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

[PATCH] D59083: [clangd] Store explicit template specializations in index for code navigation purposes

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

One of the use cases I imagined for this (unrelated to D58880 
) is that if the user searches for `vector` 
(using `workspace/symbols`), they get a separate search result for the `vector` 
primary template, and a separate one for `vector`. For this to be useful, 
the server needs to print the `` as part of the search result.

(For D58880 , I would find it useful for be 
able to look up an explicit spec. by template-id in the tests, but that can 
probably be worked around.)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59083



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

[PATCH] D59210: clang-format: distinguish ObjC call subexpressions after r355434

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

most of the cases that we were adding here were for a templated return types

e.g.

  verifyFormat("[]() -> foo<5 + 2> { return {}; };");
  verifyFormat("[]() -> foo<5 - 2> { return {}; };");

So we should probably have handle that "-> (return type)", where the return 
type contains  separately, where those operation tokens are inside the XXX


Repository:
  rC Clang

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

https://reviews.llvm.org/D59210



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

[PATCH] D59083: [clangd] Store explicit template specializations in index for code navigation purposes

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D59083#1424509 , @nridge wrote:

> Is any representation of the template arguments stored in the index?


No we only have, class name if you want to do a "name based" search/lookup. But 
symbol IDs for template specializations are different, so you can query them by 
ID.
Do you have any use case that needs names?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59083



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

[PATCH] D59210: clang-format: distinguish ObjC call subexpressions after r355434

[MyDeveloperDay via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D59210



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

[PATCH] D59210: clang-format: distinguish ObjC call subexpressions after r355434

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The revision r355434 had the unfortunate side-effect that it started to
recognize certain ObjC expressions with a call subexpression followed by a
`a->b` subexpression as C++ lambda expressions.

This patch adds a bit of logic to handle these cases and documents them in
tests.


Repository:
  rC Clang

https://reviews.llvm.org/D59210

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1329,6 +1329,34 @@
"   @\"f\"];");
 }
 
+TEST_F(FormatTestObjC, DisambiguatesCallsFromCppLambdas) {
+  verifyFormat("x = ([a foo:bar] && b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] + b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] + !b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] + ~b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] - b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] / b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] % b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] | b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] || b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] && b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] == b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] != b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] <= b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] >= b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] << b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] ? b->c == 'd' : 'e');");
+  // FIXME: The following are wrongly classified as C++ lambda expressions.
+  // For example this code:
+  //   x = ([a foo:bar] & b->c == 'd');
+  // is formatted as:
+  //   x = ([a foo:bar] & b -> c == 'd');
+  // verifyFormat("x = ([a foo:bar] & b->c == 'd');");
+  // verifyFormat("x = ([a foo:bar] > b->c == 'd');");
+  // verifyFormat("x = ([a foo:bar] < b->c == 'd');");
+  // verifyFormat("x = ([a foo:bar] >> b->c == 'd');");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1401,6 +1401,8 @@
   if (!tryToParseLambdaIntroducer())
 return false;
 
+  bool SeenArrow = false;
+
   while (FormatTok->isNot(tok::l_brace)) {
 if (FormatTok->isSimpleTypeSpecifier()) {
   nextToken();
@@ -1423,8 +1425,19 @@
 case tok::coloncolon:
 case tok::kw_mutable:
 case tok::kw_noexcept:
+  nextToken();
+  break;
 // Specialization of a template with an integer parameter can contain
 // arithmetic, logical, comparison and ternary operators.
+//
+// FIXME: This also accepts sequences of operators that are not in the 
scope
+// of a template argument list.
+//
+// In a C++ lambda a template type can only occur after an arrow. We use
+// this as an heuristic to distinguish between Objective-C expressions
+// followed by an `a->b` expression, such as:
+// ([obj func:arg] + a->b)
+// Otherwise the code below would parse as a lambda.
 case tok::plus:
 case tok::minus:
 case tok::exclaim:
@@ -1444,13 +1457,17 @@
 case tok::colon:
 case tok::kw_true:
 case tok::kw_false:
-  nextToken();
-  break;
+  if (SeenArrow) {
+nextToken();
+break;
+  }
+  return true;
 case tok::arrow:
   // This might or might not actually be a lambda arrow (this could be an
   // ObjC method invocation followed by a dereferencing arrow). We might
   // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
+  SeenArrow = true;
   nextToken();
   break;
 default:


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1329,6 +1329,34 @@
"   @\"f\"];");
 }
 
+TEST_F(FormatTestObjC, DisambiguatesCallsFromCppLambdas) {
+  verifyFormat("x = ([a foo:bar] && b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] + b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] + !b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] + ~b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] - b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] / b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] % b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] | b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] || b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] && b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] == b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] != b->c == 'd');");
+  verifyFormat("x = ([a foo:bar] <= b->c == 

[PATCH] D59083: [clangd] Store explicit template specializations in index for code navigation purposes

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Is any representation of the template arguments stored in the index?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59083



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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



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

Perhaps I'm a bit dense on a Monday morning, but why should this be a 
diagnostic? I am worried this will diagnose situations like (untested guesses):
```
#define const const
const int f(void); // Why no fixit?

#define attr __attribute__((frobble))
attr void g(void); // Why diagnosed as needing a trailing return type?
```



Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:234
+  bool ExtendedLeft = false;
+  for (size_t I = 0; I < Tokens.size(); I++) {
+// If we found the beginning of the return type, include const and volatile

As a torture test for this, how well does it handle a declaration like:
```
const long static int volatile unsigned inline long foo();
```
Does it get the fixit to spit out:
```
static inline auto foo() -> const unsigned long long int;
```



Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:293
+  if (!TSI) {
+diag(F->getLocation(), Message);
+return;

This seems incorrect to me; if we cannot get the type source information, why 
do we assume it has a return type that needs to be turned into a trailing 
return type?



Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:298
+  TSI->getTypeLoc().IgnoreParens().getAs();
+  if (!FTL) {
+diag(F->getLocation(), Message);

I think this can turn into an assertion that `FTL` is valid.



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

aaron.ballman wrote:
> Should this also bail out if the function is `main()`?
This comment was marked as done, but I don't see any changes or mention of what 
should happen. I suppose the more general question is: should there be a list 
of functions that should not have this transformation applied, like program 
entrypoints? Or do you want to see this check diagnose those functions as well?



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

aaron.ballman wrote:
> This is a rather unexpected transformation, to me.
This comment is marked as done, but there are no changes or explanations.


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

https://reviews.llvm.org/D56160



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

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

[Hyrum Wright via Phabricator via cfe-commits]

hwright updated this revision to Diff 190075.
hwright marked an inline comment as done.

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

https://reviews.llvm.org/D58977

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationComparisonCheck.cpp
  clang-tidy/abseil/DurationRewriter.h
  clang-tidy/abseil/TimeComparisonCheck.cpp
  clang-tidy/abseil/TimeComparisonCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-time-comparison.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-time-comparison.cpp

Index: test/clang-tidy/abseil-time-comparison.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-time-comparison.cpp
@@ -0,0 +1,129 @@
+// RUN: %check_clang_tidy %s abseil-time-comparison %t -- -- -I%S/Inputs
+
+#include "absl/time/time.h"
+
+void f() {
+  double x;
+  absl::Duration d1, d2;
+  bool b;
+  absl::Time t1, t2;
+
+  // Check against the RHS
+  b = x > absl::ToUnixSeconds(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixSeconds(x) > t1;
+  b = x >= absl::ToUnixSeconds(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixSeconds(x) >= t1;
+  b = x == absl::ToUnixSeconds(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixSeconds(x) == t1;
+  b = x <= absl::ToUnixSeconds(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixSeconds(x) <= t1;
+  b = x < absl::ToUnixSeconds(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixSeconds(x) < t1;
+  b = x == absl::ToUnixSeconds(t1 - d2);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixSeconds(x) == t1 - d2;
+  b = absl::ToUnixSeconds(t1) > absl::ToUnixSeconds(t2);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 > t2;
+
+  // Check against the LHS
+  b = absl::ToUnixSeconds(t1) < x;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 < absl::FromUnixSeconds(x);
+  b = absl::ToUnixSeconds(t1) <= x;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 <= absl::FromUnixSeconds(x);
+  b = absl::ToUnixSeconds(t1) == x;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 == absl::FromUnixSeconds(x);
+  b = absl::ToUnixSeconds(t1) >= x;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 >= absl::FromUnixSeconds(x);
+  b = absl::ToUnixSeconds(t1) > x;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 > absl::FromUnixSeconds(x);
+
+  // Comparison against zero
+  b = absl::ToUnixSeconds(t1) < 0.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 < absl::UnixEpoch();
+  b = absl::ToUnixSeconds(t1) < 0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 < absl::UnixEpoch();
+
+  // Scales other than Seconds
+  b = x > absl::ToUnixMicros(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixMicros(x) > t1;
+  b = x >= absl::ToUnixMillis(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixMillis(x) >= t1;
+  b = x == absl::ToUnixNanos(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixNanos(x) == t1;
+  b = x <= absl::ToUnixMinutes(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixMinutes(x) <= t1;
+  b = x < absl::ToUnixHours(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixHours(x) < t1;
+
+  // A long expression
+  bool some_condition;
+  int very_very_very_very_long_variable_name;
+  absl::Time SomeTime;
+  if (some_condition && 

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

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

Szelethus accepted this revision.
Szelethus added a comment.

LGTM!


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

https://reviews.llvm.org/D59054



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

[PATCH] D58841: [Diagnostics] Support -Wtype-limits for GCC compatibility

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

xbolva00 added a comment.

Ping


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

https://reviews.llvm.org/D58841



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

[PATCH] D58065: [analyzer] Document the frontend library

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

Szelethus added a reviewer: aaron.ballman.
Szelethus added a comment.

Oops.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58065



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

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

[Eric Liu via Phabricator via cfe-commits]

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



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

hwright wrote:
> ioeric wrote:
> > `DurationComparisonCheck.cpp` has a very similar matcher pattern.
> > 
> > Maybe pull out a common matcher for this? E.g. 
> > `comparisonOperatorWithCallee(...)`?
> > 
> My one concern about doing so is that it would move the name bindings into a 
> separate location away from the callback consuming those bindings.
> 
> Since this is only the second instance of this pattern, would it be 
> reasonable to wait until there's a third to combine them?
> My one concern about doing so is that it would move the name bindings into a 
> separate location away from the callback consuming those bindings.
I think the name binding can stay in the same file. 
`comparisonOperatorWithCallee` is a matcher for a FunctionDecl, so, on the call 
site, you could do 
`comparisonOperatorWithCallee(functionDecl(TimeConversionFunction()).bind("function_decl")).bind("binop")`.


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

https://reviews.llvm.org/D58977



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

[PATCH] D59205: [clangd] Respect Origin option in createStaticIndexingAction

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE355820: [clangd] Respect Origin option in 
createStaticIndexingAction (authored by kadircet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59205?vs=190062=190066#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59205

Files:
  clangd/index/IndexAction.cpp
  clangd/index/IndexAction.h


Index: clangd/index/IndexAction.h
===
--- clangd/index/IndexAction.h
+++ clangd/index/IndexAction.h
@@ -22,7 +22,7 @@
 //   - include paths are always collected, and canonicalized appropriately
 //   - references are always counted
 //   - all references are collected (if RefsCallback is non-null)
-//   - the symbol origin is always Static
+//   - the symbol origin is set to Static if not specified by caller
 std::unique_ptr createStaticIndexingAction(
 SymbolCollector::Options Opts,
 std::function SymbolsCallback,
Index: clangd/index/IndexAction.cpp
===
--- clangd/index/IndexAction.cpp
+++ clangd/index/IndexAction.cpp
@@ -183,7 +183,8 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   Opts.CollectIncludePath = true;
   Opts.CountReferences = true;
-  Opts.Origin = SymbolOrigin::Static;
+  if (Opts.Origin == SymbolOrigin::Unknown)
+Opts.Origin = SymbolOrigin::Static;
   Opts.StoreAllDocumentation = false;
   if (RefsCallback != nullptr) {
 Opts.RefFilter = RefKind::All;


Index: clangd/index/IndexAction.h
===
--- clangd/index/IndexAction.h
+++ clangd/index/IndexAction.h
@@ -22,7 +22,7 @@
 //   - include paths are always collected, and canonicalized appropriately
 //   - references are always counted
 //   - all references are collected (if RefsCallback is non-null)
-//   - the symbol origin is always Static
+//   - the symbol origin is set to Static if not specified by caller
 std::unique_ptr createStaticIndexingAction(
 SymbolCollector::Options Opts,
 std::function SymbolsCallback,
Index: clangd/index/IndexAction.cpp
===
--- clangd/index/IndexAction.cpp
+++ clangd/index/IndexAction.cpp
@@ -183,7 +183,8 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   Opts.CollectIncludePath = true;
   Opts.CountReferences = true;
-  Opts.Origin = SymbolOrigin::Static;
+  if (Opts.Origin == SymbolOrigin::Unknown)
+Opts.Origin = SymbolOrigin::Static;
   Opts.StoreAllDocumentation = false;
   if (RefsCallback != nullptr) {
 Opts.RefFilter = RefKind::All;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r355820 - [clangd] Respect Origin option in createStaticIndexingAction

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Mon Mar 11 04:01:14 2019
New Revision: 355820

URL: http://llvm.org/viewvc/llvm-project?rev=355820=rev
Log:
[clangd] Respect Origin option in createStaticIndexingAction

Summary:
Currently createStaticIndexingAction always set Origin to Static, which
makes it hard to change it later on by different indexers(One needs to go over
each symbol making a new copy).

This patch changes that behavior to rather respect it if set by user.

Reviewers: ioeric

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/index/IndexAction.cpp
clang-tools-extra/trunk/clangd/index/IndexAction.h

Modified: clang-tools-extra/trunk/clangd/index/IndexAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/IndexAction.cpp?rev=355820=355819=355820=diff
==
--- clang-tools-extra/trunk/clangd/index/IndexAction.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/IndexAction.cpp Mon Mar 11 04:01:14 
2019
@@ -183,7 +183,8 @@ std::unique_ptr createSt
   index::IndexingOptions::SystemSymbolFilterKind::All;
   Opts.CollectIncludePath = true;
   Opts.CountReferences = true;
-  Opts.Origin = SymbolOrigin::Static;
+  if (Opts.Origin == SymbolOrigin::Unknown)
+Opts.Origin = SymbolOrigin::Static;
   Opts.StoreAllDocumentation = false;
   if (RefsCallback != nullptr) {
 Opts.RefFilter = RefKind::All;

Modified: clang-tools-extra/trunk/clangd/index/IndexAction.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/IndexAction.h?rev=355820=355819=355820=diff
==
--- clang-tools-extra/trunk/clangd/index/IndexAction.h (original)
+++ clang-tools-extra/trunk/clangd/index/IndexAction.h Mon Mar 11 04:01:14 2019
@@ -22,7 +22,7 @@ namespace clangd {
 //   - include paths are always collected, and canonicalized appropriately
 //   - references are always counted
 //   - all references are collected (if RefsCallback is non-null)
-//   - the symbol origin is always Static
+//   - the symbol origin is set to Static if not specified by caller
 std::unique_ptr createStaticIndexingAction(
 SymbolCollector::Options Opts,
 std::function SymbolsCallback,


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

[PATCH] D59205: [clangd] Respect Origin option in createStaticIndexingAction

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: ioeric.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Currently createStaticIndexingAction always set Origin to Static, which
makes it hard to change it later on by different indexers(One needs to go over
each symbol making a new copy).

This patch changes that behavior to rather respect it if set by user.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D59205

Files:
  clangd/index/IndexAction.cpp
  clangd/index/IndexAction.h


Index: clangd/index/IndexAction.h
===
--- clangd/index/IndexAction.h
+++ clangd/index/IndexAction.h
@@ -22,7 +22,7 @@
 //   - include paths are always collected, and canonicalized appropriately
 //   - references are always counted
 //   - all references are collected (if RefsCallback is non-null)
-//   - the symbol origin is always Static
+//   - the symbol origin is set to Static if not specified by caller
 std::unique_ptr createStaticIndexingAction(
 SymbolCollector::Options Opts,
 std::function SymbolsCallback,
Index: clangd/index/IndexAction.cpp
===
--- clangd/index/IndexAction.cpp
+++ clangd/index/IndexAction.cpp
@@ -183,7 +183,8 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   Opts.CollectIncludePath = true;
   Opts.CountReferences = true;
-  Opts.Origin = SymbolOrigin::Static;
+  if (Opts.Origin == SymbolOrigin::Unknown)
+Opts.Origin = SymbolOrigin::Static;
   Opts.StoreAllDocumentation = false;
   if (RefsCallback != nullptr) {
 Opts.RefFilter = RefKind::All;


Index: clangd/index/IndexAction.h
===
--- clangd/index/IndexAction.h
+++ clangd/index/IndexAction.h
@@ -22,7 +22,7 @@
 //   - include paths are always collected, and canonicalized appropriately
 //   - references are always counted
 //   - all references are collected (if RefsCallback is non-null)
-//   - the symbol origin is always Static
+//   - the symbol origin is set to Static if not specified by caller
 std::unique_ptr createStaticIndexingAction(
 SymbolCollector::Options Opts,
 std::function SymbolsCallback,
Index: clangd/index/IndexAction.cpp
===
--- clangd/index/IndexAction.cpp
+++ clangd/index/IndexAction.cpp
@@ -183,7 +183,8 @@
   index::IndexingOptions::SystemSymbolFilterKind::All;
   Opts.CollectIncludePath = true;
   Opts.CountReferences = true;
-  Opts.Origin = SymbolOrigin::Static;
+  if (Opts.Origin == SymbolOrigin::Unknown)
+Opts.Origin = SymbolOrigin::Static;
   Opts.StoreAllDocumentation = false;
   if (RefsCallback != nullptr) {
 Opts.RefFilter = RefKind::All;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r355819 - Remove an unnecessary -f when cp'ing to a file that was just deleted. NFC.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Mon Mar 11 03:44:10 2019
New Revision: 355819

URL: http://llvm.org/viewvc/llvm-project?rev=355819=rev
Log:
Remove an unnecessary -f when cp'ing to a file that was just deleted. NFC.

Modified:
cfe/trunk/test/Modules/relative-import-path.c

Modified: cfe/trunk/test/Modules/relative-import-path.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/relative-import-path.c?rev=355819=355818=355819=diff
==
--- cfe/trunk/test/Modules/relative-import-path.c (original)
+++ cfe/trunk/test/Modules/relative-import-path.c Mon Mar 11 03:44:10 2019
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: cp -rf %S/Inputs/relative-import-path %t
+// RUN: cp -r %S/Inputs/relative-import-path %t
 // RUN: cp %s %t/t.c
 
 // Use FileCheck, which is more flexible.


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

r355816 - [Serialization] Add missing include

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Mon Mar 11 03:30:51 2019
New Revision: 355816

URL: http://llvm.org/viewvc/llvm-project?rev=355816=rev
Log:
[Serialization] Add missing include

forward decl is not sufficient for destroying a unique_ptr.

Modified:
cfe/trunk/include/clang/Serialization/InMemoryModuleCache.h

Modified: cfe/trunk/include/clang/Serialization/InMemoryModuleCache.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/InMemoryModuleCache.h?rev=355816=355815=355816=diff
==
--- cfe/trunk/include/clang/Serialization/InMemoryModuleCache.h (original)
+++ cfe/trunk/include/clang/Serialization/InMemoryModuleCache.h Mon Mar 11 
03:30:51 2019
@@ -12,12 +12,9 @@
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/MemoryBuffer.h"
 #include 
 
-namespace llvm {
-class MemoryBuffer;
-} // end namespace llvm
-
 namespace clang {
 
 /// In-memory cache for modules.


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

[PATCH] D55358: [ASTImporter] Fix import of NestedNameSpecifierLoc.

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

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

Rebase.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55358

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


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1232,6 +1232,26 @@
   has(fieldDecl(hasType(dependentSizedArrayType(;
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportBeginLocOfDeclRefExpr) {
+  Decl *FromTU = getTuDecl(
+  "class A { public: static int X; }; void f() { (void)A::X; }", Lang_CXX);
+  auto From = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("f")));
+  ASSERT_TRUE(From);
+  ASSERT_TRUE(
+  cast(cast(From->getBody())->body_front())
+  ->getSubExpr()
+  ->getBeginLoc()
+  .isValid());
+  FunctionDecl *To = Import(From, Lang_CXX);
+  ASSERT_TRUE(To);
+  ASSERT_TRUE(
+  cast(cast(To->getBody())->body_front())
+  ->getSubExpr()
+  ->getBeginLoc()
+  .isValid());
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase,
ImportOfTemplatedDeclOfClassTemplateDecl) {
   Decl *FromTU = getTuDecl("template struct S{};", Lang_CXX);
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -8092,8 +8092,11 @@
 
 case NestedNameSpecifier::TypeSpec:
 case NestedNameSpecifier::TypeSpecWithTemplate: {
+  SourceLocation ToTLoc;
+  if (Error Err = importInto(ToTLoc, NNS.getTypeLoc().getBeginLoc()))
+return std::move(Err);
   TypeSourceInfo *TSI = getToContext().getTrivialTypeSourceInfo(
-QualType(Spec->getAsType(), 0));
+QualType(Spec->getAsType(), 0), ToTLoc);
   Builder.Extend(getToContext(), ToLocalBeginLoc, TSI->getTypeLoc(),
  ToLocalEndLoc);
   break;


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1232,6 +1232,26 @@
   has(fieldDecl(hasType(dependentSizedArrayType(;
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportBeginLocOfDeclRefExpr) {
+  Decl *FromTU = getTuDecl(
+  "class A { public: static int X; }; void f() { (void)A::X; }", Lang_CXX);
+  auto From = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("f")));
+  ASSERT_TRUE(From);
+  ASSERT_TRUE(
+  cast(cast(From->getBody())->body_front())
+  ->getSubExpr()
+  ->getBeginLoc()
+  .isValid());
+  FunctionDecl *To = Import(From, Lang_CXX);
+  ASSERT_TRUE(To);
+  ASSERT_TRUE(
+  cast(cast(To->getBody())->body_front())
+  ->getSubExpr()
+  ->getBeginLoc()
+  .isValid());
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase,
ImportOfTemplatedDeclOfClassTemplateDecl) {
   Decl *FromTU = getTuDecl("template struct S{};", Lang_CXX);
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -8092,8 +8092,11 @@
 
 case NestedNameSpecifier::TypeSpec:
 case NestedNameSpecifier::TypeSpecWithTemplate: {
+  SourceLocation ToTLoc;
+  if (Error Err = importInto(ToTLoc, NNS.getTypeLoc().getBeginLoc()))
+return std::move(Err);
   TypeSourceInfo *TSI = getToContext().getTrivialTypeSourceInfo(
-QualType(Spec->getAsType(), 0));
+QualType(Spec->getAsType(), 0), ToTLoc);
   Builder.Extend(getToContext(), ToLocalBeginLoc, TSI->getTypeLoc(),
  ToLocalEndLoc);
   break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 190053.
MyDeveloperDay marked 8 inline comments as done.
MyDeveloperDay added a comment.

Address review comment

- add document and comment spelling and punctuation
- add additional unit tests to show non compound else clause case


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

https://reviews.llvm.org/D59087

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

Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -98,7 +98,7 @@
 }
 
 TEST_F(FormatTestSelective, FormatsIfWithoutCompoundStatement) {
-  Style.AllowShortIfStatementsOnASingleLine = true;
+  Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
   EXPECT_EQ("if (a) return;", format("if(a)\nreturn;", 7, 1));
   EXPECT_EQ("if (a) return; // comment",
 format("if(a)\nreturn; // comment", 20, 1));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -439,7 +439,7 @@
 
   FormatStyle AllowsMergedIf = getLLVMStyle();
   AllowsMergedIf.AlignEscapedNewlines = FormatStyle::ENAS_Left;
-  AllowsMergedIf.AllowShortIfStatementsOnASingleLine = true;
+  AllowsMergedIf.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
   verifyFormat("if (a)\n"
"  // comment\n"
"  f();",
@@ -487,6 +487,41 @@
   verifyFormat("if (a)\n  return;", AllowsMergedIf);
 }
 
+TEST_F(FormatTest, FormatIfWithoutCompoundStatementButElseWith) {
+  FormatStyle AllowsMergedIf = getLLVMStyle();
+  AllowsMergedIf.AlignEscapedNewlines = FormatStyle::ENAS_Left;
+  AllowsMergedIf.AllowShortIfStatementsOnASingleLine =
+  FormatStyle::SIS_WithoutElse;
+  verifyFormat("if (a)\n"
+   "  f();\n"
+   "else {\n"
+   "  g();\n"
+   "}",
+   AllowsMergedIf);
+  verifyFormat("if (a)\n"
+   "  f();\n"
+   "else\n"
+   "  g();\n",
+   AllowsMergedIf);
+
+  AllowsMergedIf.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Always;
+
+  verifyFormat("if (a) f();\n"
+   "else {\n"
+   "  g();\n"
+   "}",
+   AllowsMergedIf);
+  verifyFormat("if (a) f();\n"
+   "else {\n"
+   "  if (a) f();\n"
+   "  else {\n"
+   "g();\n"
+   "  }\n"
+   "  g();\n"
+   "}",
+   AllowsMergedIf);
+}
+
 TEST_F(FormatTest, FormatLoopsWithoutCompoundStatement) {
   FormatStyle AllowsMergedLoops = getLLVMStyle();
   AllowsMergedLoops.AllowShortLoopsOnASingleLine = true;
@@ -515,7 +550,7 @@
   AllowSimpleBracedStatements.ColumnLimit = 40;
   AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true;
 
-  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
   AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
 
   AllowSimpleBracedStatements.BreakBeforeBraces = FormatStyle::BS_Custom;
@@ -563,7 +598,7 @@
"};",
AllowSimpleBracedStatements);
 
-  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
   verifyFormat("if (true) {}", AllowSimpleBracedStatements);
   verifyFormat("if (true) {\n"
"  f();\n"
@@ -588,7 +623,7 @@
"}",
AllowSimpleBracedStatements);
 
-  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
   AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
   AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement = true;
 
@@ -625,7 +660,7 @@
"}",
AllowSimpleBracedStatements);
 
-  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
   verifyFormat("if (true) {}", AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
"{\n"
@@ -659,7 +694,7 @@
 TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
   FormatStyle Style = getLLVMStyleWithColumns(60);
   Style.AllowShortBlocksOnASingleLine = true;
-  Style.AllowShortIfStatementsOnASingleLine = true;
+  

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

[Reuben Thomas via Phabricator via cfe-commits]

reuk added a comment.

This looks much better now, thanks for taking another look! I've flagged some 
formatting/spelling nits, and I think the tests/docs could be a little more 
explicit about the behaviour of `WithoutElse`. Other than that, looks great!




Comment at: clang/docs/ClangFormatStyleOptions.rst:375
+  * ``SIS_Never`` (in configuration: ``Never``)
+Do not allow short if functions
+

For consistency, these descriptions should end with periods `.`



Comment at: clang/docs/ClangFormatStyleOptions.rst:386
+Allow short if functions on the same line, as long as else
+is not a compound statement
 

Missing `.`



Comment at: clang/docs/ClangFormatStyleOptions.rst:390
+
+   if (a) return;
+   else 

I think an example of the generated formatting *with* a compound else statement 
would be useful here too, in addition to the existing example.



Comment at: clang/docs/ClangFormatStyleOptions.rst:395
+  * ``SIS_Always`` (in configuration: ``Always``)
+Allow short if statements even if the else is a compounf statement
+

compounf -> compound



Comment at: clang/docs/ClangFormatStyleOptions.rst:395
+  * ``SIS_Always`` (in configuration: ``Always``)
+Allow short if statements even if the else is a compounf statement
+

reuk wrote:
> compounf -> compound
Missing `.`



Comment at: clang/include/clang/Format/Format.h:264
+/// Always put short ifs on the same line if
+/// the else is not a compound statement or not
+/// \code

Missing `.`



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:417
+// Only inline simple if's (no nested if or else), unless specified
+if (Style.AllowShortIfStatementsOnASingleLine!=FormatStyle::SIS_Always) {
+  if (I + 2 != E && Line.startsWith(tok::kw_if) &&

Has this line been clang-formatted? I'd expect spaces around the binop



Comment at: clang/unittests/Format/FormatTest.cpp:494
+  AllowsMergedIf.AllowShortIfStatementsOnASingleLine = 
FormatStyle::SIS_WithoutElse;
+  verifyFormat("if (a)\n"
+   "  f();\n"

I think having a test here which shows the expected behaviour for non-compound 
else statements with `WithoutElse` would be helpful, as in the documentation.


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

https://reviews.llvm.org/D59087



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

[PATCH] D59132: [clangd] Add TOC section to clangd doc.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355811: [clangd] Add TOC section to clangd doc. (authored by 
hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59132

Files:
  clang-tools-extra/trunk/docs/clangd/Extensions.rst
  clang-tools-extra/trunk/docs/clangd/Features.rst
  clang-tools-extra/trunk/docs/clangd/Installation.rst


Index: clang-tools-extra/trunk/docs/clangd/Features.rst
===
--- clang-tools-extra/trunk/docs/clangd/Features.rst
+++ clang-tools-extra/trunk/docs/clangd/Features.rst
@@ -2,6 +2,8 @@
 Features
 
 
+.. contents::
+
 .. role:: raw-html(raw)
:format: html
 
Index: clang-tools-extra/trunk/docs/clangd/Installation.rst
===
--- clang-tools-extra/trunk/docs/clangd/Installation.rst
+++ clang-tools-extra/trunk/docs/clangd/Installation.rst
@@ -2,6 +2,8 @@
 Getting started with clangd
 ===
 
+.. contents::
+
 .. role:: raw-html(raw)
:format: html
 
Index: clang-tools-extra/trunk/docs/clangd/Extensions.rst
===
--- clang-tools-extra/trunk/docs/clangd/Extensions.rst
+++ clang-tools-extra/trunk/docs/clangd/Extensions.rst
@@ -2,6 +2,8 @@
 Protocol extensions
 ===
 
+.. contents::
+
 clangd supports some features that are not in the official
 `Language Server Protocol specification
 `__.


Index: clang-tools-extra/trunk/docs/clangd/Features.rst
===
--- clang-tools-extra/trunk/docs/clangd/Features.rst
+++ clang-tools-extra/trunk/docs/clangd/Features.rst
@@ -2,6 +2,8 @@
 Features
 
 
+.. contents::
+
 .. role:: raw-html(raw)
:format: html
 
Index: clang-tools-extra/trunk/docs/clangd/Installation.rst
===
--- clang-tools-extra/trunk/docs/clangd/Installation.rst
+++ clang-tools-extra/trunk/docs/clangd/Installation.rst
@@ -2,6 +2,8 @@
 Getting started with clangd
 ===
 
+.. contents::
+
 .. role:: raw-html(raw)
:format: html
 
Index: clang-tools-extra/trunk/docs/clangd/Extensions.rst
===
--- clang-tools-extra/trunk/docs/clangd/Extensions.rst
+++ clang-tools-extra/trunk/docs/clangd/Extensions.rst
@@ -2,6 +2,8 @@
 Protocol extensions
 ===
 
+.. contents::
+
 clangd supports some features that are not in the official
 `Language Server Protocol specification
 `__.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r355811 - [clangd] Add TOC section to clangd doc.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Mon Mar 11 01:45:56 2019
New Revision: 355811

URL: http://llvm.org/viewvc/llvm-project?rev=355811=rev
Log:
[clangd] Add TOC section to clangd doc.

Reviewers: gribozavr

Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, 
cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/docs/clangd/Extensions.rst
clang-tools-extra/trunk/docs/clangd/Features.rst
clang-tools-extra/trunk/docs/clangd/Installation.rst

Modified: clang-tools-extra/trunk/docs/clangd/Extensions.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clangd/Extensions.rst?rev=355811=355810=355811=diff
==
--- clang-tools-extra/trunk/docs/clangd/Extensions.rst (original)
+++ clang-tools-extra/trunk/docs/clangd/Extensions.rst Mon Mar 11 01:45:56 2019
@@ -2,6 +2,8 @@
 Protocol extensions
 ===
 
+.. contents::
+
 clangd supports some features that are not in the official
 `Language Server Protocol specification
 `__.

Modified: clang-tools-extra/trunk/docs/clangd/Features.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clangd/Features.rst?rev=355811=355810=355811=diff
==
--- clang-tools-extra/trunk/docs/clangd/Features.rst (original)
+++ clang-tools-extra/trunk/docs/clangd/Features.rst Mon Mar 11 01:45:56 2019
@@ -2,6 +2,8 @@
 Features
 
 
+.. contents::
+
 .. role:: raw-html(raw)
:format: html
 

Modified: clang-tools-extra/trunk/docs/clangd/Installation.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clangd/Installation.rst?rev=355811=355810=355811=diff
==
--- clang-tools-extra/trunk/docs/clangd/Installation.rst (original)
+++ clang-tools-extra/trunk/docs/clangd/Installation.rst Mon Mar 11 01:45:56 
2019
@@ -2,6 +2,8 @@
 Getting started with clangd
 ===
 
+.. contents::
+
 .. role:: raw-html(raw)
:format: html
 


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

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 190049.
MyDeveloperDay added a comment.

Address review comments

- collapse two options into one


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

https://reviews.llvm.org/D59087

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

Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -98,7 +98,7 @@
 }
 
 TEST_F(FormatTestSelective, FormatsIfWithoutCompoundStatement) {
-  Style.AllowShortIfStatementsOnASingleLine = true;
+  Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
   EXPECT_EQ("if (a) return;", format("if(a)\nreturn;", 7, 1));
   EXPECT_EQ("if (a) return; // comment",
 format("if(a)\nreturn; // comment", 20, 1));
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -439,7 +439,7 @@
 
   FormatStyle AllowsMergedIf = getLLVMStyle();
   AllowsMergedIf.AlignEscapedNewlines = FormatStyle::ENAS_Left;
-  AllowsMergedIf.AllowShortIfStatementsOnASingleLine = true;
+  AllowsMergedIf.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
   verifyFormat("if (a)\n"
"  // comment\n"
"  f();",
@@ -487,6 +487,34 @@
   verifyFormat("if (a)\n  return;", AllowsMergedIf);
 }
 
+TEST_F(FormatTest, FormatIfWithoutCompoundStatementButElseWith) {
+  FormatStyle AllowsMergedIf = getLLVMStyle();
+  AllowsMergedIf.AlignEscapedNewlines = FormatStyle::ENAS_Left;
+  AllowsMergedIf.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
+  verifyFormat("if (a)\n"
+   "  f();\n"
+   "else {\n"
+   "  g();\n"
+   "}",
+   AllowsMergedIf);
+
+  AllowsMergedIf.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Always;
+  verifyFormat("if (a) f();\n"
+   "else {\n"
+   "  g();\n"
+   "}",
+   AllowsMergedIf);
+  verifyFormat("if (a) f();\n"
+   "else {\n"
+   "  if (a) f();\n"
+   "  else {\n"
+   "g();\n"
+   "  }\n"
+   "  g();\n"
+   "}",
+   AllowsMergedIf);
+}
+
 TEST_F(FormatTest, FormatLoopsWithoutCompoundStatement) {
   FormatStyle AllowsMergedLoops = getLLVMStyle();
   AllowsMergedLoops.AllowShortLoopsOnASingleLine = true;
@@ -515,7 +543,7 @@
   AllowSimpleBracedStatements.ColumnLimit = 40;
   AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true;
 
-  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
   AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
 
   AllowSimpleBracedStatements.BreakBeforeBraces = FormatStyle::BS_Custom;
@@ -563,7 +591,7 @@
"};",
AllowSimpleBracedStatements);
 
-  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
   verifyFormat("if (true) {}", AllowSimpleBracedStatements);
   verifyFormat("if (true) {\n"
"  f();\n"
@@ -588,7 +616,7 @@
"}",
AllowSimpleBracedStatements);
 
-  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
   AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
   AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement = true;
 
@@ -625,7 +653,7 @@
"}",
AllowSimpleBracedStatements);
 
-  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
   verifyFormat("if (true) {}", AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
"{\n"
@@ -659,7 +687,7 @@
 TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
   FormatStyle Style = getLLVMStyleWithColumns(60);
   Style.AllowShortBlocksOnASingleLine = true;
-  Style.AllowShortIfStatementsOnASingleLine = true;
+  Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
   Style.BreakBeforeBraces = FormatStyle::BS_Allman;
   EXPECT_EQ("#define A  \\\n"
 "  if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
@@ -3157,7 +3185,7 

[PATCH] D58897: [ASTImporter] Make ODR error handling configurable

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hi Gabor,
This patch LGTM mostly, but there is a comment inline.




Comment at: include/clang/AST/ASTStructuralEquivalence.h:81
 EqKind(EqKind), StrictTypeSpelling(StrictTypeSpelling),
-ErrorOnTagTypeMismatch(ErrorOnTagTypeMismatch), Complain(Complain) {}
+Complain(Complain), ErrorOnTagTypeMismatch(ErrorOnTagTypeMismatch) {}
 

I see the argument order change but I don't see any callers changed. Do we 
really need this order change?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58897



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

[PATCH] D56990: Bugfix for Replacement of tied operand of inline asm

[Xiang Zhang via Phabricator via cfe-commits]

xiangzhangllvm added a comment.

Hi, efriedma
could you help he commit this patch?
Thank you very much!


Repository:
  rC Clang

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

https://reviews.llvm.org/D56990



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