[PATCH] D35216: [analyzer] Escape symbols when creating std::initializer_list.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 105897.
NoQ edited the summary of this revision.
NoQ added a comment.
Herald added a subscriber: mgorny.

This other diff implements approach (1) through a draft of a checker (that 
doesn't model much yet). I had to additionally make sure we already have a 
region to construct metadata for, in spirit of https://reviews.llvm.org/D27202.

For easier comparison, i thought it'd be better to stick it to the same 
differential revision.


https://reviews.llvm.org/D35216

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/StdInitializerListChecker.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  test/Analysis/initializer.cpp
  test/Analysis/temporaries-callback-order.cpp

Index: test/Analysis/temporaries-callback-order.cpp
===
--- test/Analysis/temporaries-callback-order.cpp
+++ test/Analysis/temporaries-callback-order.cpp
@@ -18,14 +18,12 @@
 
 void seeIfCheckBindWorks() {
   // This should trigger checkBind. The rest of the code shouldn't.
-  // This also triggers checkRegionChanges after that.
   // Note that this function is analyzed first, so the messages would be on top.
   int x = 1;
 }
 
 // seeIfCheckBindWorks():
 // CHECK: Bind
-// CHECK-NEXT: RegionChanges
 
 // testTemporaries():
 // CHECK-NEXT: RegionChanges
Index: test/Analysis/initializer.cpp
===
--- test/Analysis/initializer.cpp
+++ test/Analysis/initializer.cpp
@@ -1,7 +1,9 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-inlining=constructors -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks,cplusplus.StdInitializerList,debug.ExprInspection -analyzer-config c++-inlining=constructors -std=c++11 -verify %s
 
 void clang_analyzer_eval(bool);
 
+#include "Inputs/system-header-simulator-cxx.h"
+
 class A {
   int x;
 public:
@@ -204,3 +206,13 @@
const char()[2];
 };
 }
+
+namespace CXX_initializer_lists {
+struct C {
+  C(std::initializer_list list);
+};
+void foo() {
+  int *x = new int;
+  C c{x}; // no-warning
+}
+}
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -116,13 +116,18 @@
   const LocationContext *LCtx,
   bool notifyChanges) const {
   ProgramStateManager  = getStateManager();
-  ProgramStateRef newState = makeWithStore(Mgr.StoreMgr->Bind(getStore(),
- LV, V));
+  ProgramStateRef NewState =
+  makeWithStore(Mgr.StoreMgr->Bind(getStore(), LV, V));
   const MemRegion *MR = LV.getAsRegion();
-  if (MR && Mgr.getOwningEngine() && notifyChanges)
-return Mgr.getOwningEngine()->processRegionChange(newState, MR, LCtx);
+  if (MR && Mgr.getOwningEngine() && notifyChanges) {
+NewState = Mgr.getOwningEngine()->processRegionChange(NewState, MR, LCtx);
+if (!NewState)
+  return nullptr;
+return Mgr.getOwningEngine()->processPointerEscapedOnBind(NewState, LV, V,
+  LCtx);
+  }
 
-  return newState;
+  return NewState;
 }
 
 ProgramStateRef ProgramState::bindDefault(SVal loc,
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1074,12 +1074,19 @@
   for (ExplodedNodeSet::iterator it = preVisit.begin(), et = preVisit.end();
it != et; ++it) {
 ExplodedNode *N = *it;
+ProgramStateRef State = N->getState();
 const LocationContext *LCtx = N->getLocationContext();
-SVal result = svalBuilder.conjureSymbolVal(nullptr, Ex, LCtx,
-   resultType,
-   currBldrCtx->blockCount());
-ProgramStateRef state = N->getState()->BindExpr(Ex, LCtx, result);
-Bldr2.generateNode(S, N, state);
+SVal Result;
+if (isa(Ex)) {
+  const MemRegion *InitListRegion =
+  svalBuilder.getRegionManager().getCXXStaticTempObjectRegion(Ex);
+  Result = State->getSVal(InitListRegion);
+} else {
+  Result = svalBuilder.conjureSymbolVal(nullptr, Ex, LCtx, resultType,
+currBldrCtx->blockCount());
+}
+State = State->BindExpr(Ex, LCtx, Result);
+Bldr2.generateNode(S, N, State);
   }
 
   getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);
@@ -2232,7 +2239,7 @@
   // 

[PATCH] D34210: Add __has_feature(leak_sanitizer)

[Francis Ricci via Phabricator via cfe-commits]

fjricci added a comment.

In https://reviews.llvm.org/D34210#804084, @smeenai wrote:

> COFF supports weak externals: https://stackoverflow.com/a/11529277/382079. 
> Would it suffice here?


Looks like it could work, thanks.


https://reviews.llvm.org/D34210



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

[PATCH] D35194: [clang-tidy] clang-apply-replacements: Don't insert null entry

[Kevin Funk via Phabricator via cfe-commits]

kfunk updated this revision to Diff 105929.
kfunk added a comment.

Remove unnecessary `sed` line in test driver


https://reviews.llvm.org/D35194

Files:
  clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml
  test/clang-apply-replacements/invalid-files.cpp


Index: test/clang-apply-replacements/invalid-files.cpp
===
--- /dev/null
+++ test/clang-apply-replacements/invalid-files.cpp
@@ -0,0 +1,7 @@
+// RUN: mkdir -p %T/Inputs/invalid-files
+// RUN: clang-apply-replacements %T/Inputs/invalid-files
+//
+// Check that the yaml files are *not* deleted after running 
clang-apply-replacements without remove-change-desc-files.
+// RUN: ls -1 %T/Inputs/invalid-files | FileCheck %s --check-prefix=YAML
+//
+ YAML: {{.+\.yaml$}}
Index: test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml
===
--- /dev/null
+++ test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml
@@ -0,0 +1,12 @@
+---
+MainSourceFile:  ''
+Replacements:
+  - FilePath:idontexist.h
+Offset:  2669
+Length:  0
+ReplacementText: ' override'
+  - FilePath:idontexist.h
+Offset:  2669
+Length:  0
+ReplacementText: ' override'
+...
Index: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===
--- clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -289,9 +289,11 @@
   // Use the file manager to deduplicate paths. FileEntries are
   // automatically canonicalized.
   const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath());
-  if (!Entry && Warned.insert(R.getFilePath()).second) {
-errs() << "Described file '" << R.getFilePath()
-   << "' doesn't exist. Ignoring...\n";
+  if (!Entry) {
+if (Warned.insert(R.getFilePath()).second) {
+  errs() << "Described file '" << R.getFilePath()
+ << "' doesn't exist. Ignoring...\n";
+}
 continue;
   }
   GroupedReplacements[Entry].push_back(R);
@@ -315,9 +317,11 @@
   // Use the file manager to deduplicate paths. FileEntries are
   // automatically canonicalized.
   const FileEntry *Entry = 
SM.getFileManager().getFile(R.getFilePath());
-  if (!Entry && Warned.insert(R.getFilePath()).second) {
-errs() << "Described file '" << R.getFilePath()
-   << "' doesn't exist. Ignoring...\n";
+  if (!Entry) {
+if (Warned.insert(R.getFilePath()).second) {
+  errs() << "Described file '" << R.getFilePath()
+ << "' doesn't exist. Ignoring...\n";
+}
 continue;
   }
   GroupedReplacements[Entry].push_back(R);


Index: test/clang-apply-replacements/invalid-files.cpp
===
--- /dev/null
+++ test/clang-apply-replacements/invalid-files.cpp
@@ -0,0 +1,7 @@
+// RUN: mkdir -p %T/Inputs/invalid-files
+// RUN: clang-apply-replacements %T/Inputs/invalid-files
+//
+// Check that the yaml files are *not* deleted after running clang-apply-replacements without remove-change-desc-files.
+// RUN: ls -1 %T/Inputs/invalid-files | FileCheck %s --check-prefix=YAML
+//
+ YAML: {{.+\.yaml$}}
Index: test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml
===
--- /dev/null
+++ test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml
@@ -0,0 +1,12 @@
+---
+MainSourceFile:  ''
+Replacements:
+  - FilePath:idontexist.h
+Offset:  2669
+Length:  0
+ReplacementText: ' override'
+  - FilePath:idontexist.h
+Offset:  2669
+Length:  0
+ReplacementText: ' override'
+...
Index: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===
--- clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -289,9 +289,11 @@
   // Use the file manager to deduplicate paths. FileEntries are
   // automatically canonicalized.
   const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath());
-  if (!Entry && Warned.insert(R.getFilePath()).second) {
-errs() << "Described file '" << R.getFilePath()
-   << "' doesn't exist. Ignoring...\n";
+  if (!Entry) {
+if (Warned.insert(R.getFilePath()).second) {
+  errs() << "Described file '" << R.getFilePath()
+ << "' doesn't exist. Ignoring...\n";
+}
 continue;
   }
   

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

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

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

Address comments.


https://reviews.llvm.org/D34784

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

Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -597,3 +597,19 @@
 // RUN:   | FileCheck -check-prefix=CHK-FOPENMP-IS-DEVICE %s
 
 // CHK-FOPENMP-IS-DEVICE: clang{{.*}} "-aux-triple" "powerpc64le--linux" {{.*}}.c" "-fopenmp-is-device" "-fopenmp-host-ir-file-path"
+
+/// ###
+
+/// Check -Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8 is passed when compiling for the device.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu -Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-EQ-TARGET %s
+
+// CHK-FOPENMP-EQ-TARGET: clang{{.*}} argument unused during compilation: '-Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8'
+
+/// ###
+
+/// Check -Xopenmp-target -march=pwr8 is passed when compiling for the device.
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu -Xopenmp-target -march=pwr8 %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-FOPENMP-TARGET %s
+
+// CHK-FOPENMP-TARGET: clang{{.*}} argument unused during compilation: '-Xopenmp-target -march=pwr8'
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -212,8 +212,20 @@
   static_cast(getToolChain());
   assert(TC.getTriple().isNVPTX() && "Wrong platform");
 
+  StringRef GPUArchName;
+  std::vector GPUArchNames;
+  // If this is an OpenMP action we need to extract the device architecture from
+  // the -fopenmp-target-arch option.
+  if (JA.isDeviceOffloading(Action::OFK_OpenMP)) {
+GPUArchNames = Args.getAllArgValues(options::OPT_march_EQ);
+assert(GPUArchNames.size() == 1 &&
+   "Exactly one GPU Arch required for ptxas.");
+GPUArchName = GPUArchNames[0];
+  } else
+GPUArchName = JA.getOffloadingArch();
+
   // Obtain architecture from the action.
-  CudaArch gpu_arch = StringToCudaArch(JA.getOffloadingArch());
+  CudaArch gpu_arch = StringToCudaArch(GPUArchName);
   assert(gpu_arch != CudaArch::UNKNOWN &&
  "Device action expected to have an architecture.");
 
@@ -405,7 +417,7 @@
 
   // For OpenMP device offloading, append derived arguments. Make sure
   // flags are not duplicated.
-  // TODO: Append the compute capability.
+  // Also append the compute capability.
   if (DeviceOffloadKind == Action::OFK_OpenMP) {
 for (Arg *A : Args){
   bool IsDuplicate = false;
@@ -418,6 +430,57 @@
   if (!IsDuplicate)
 DAL->append(A);
 }
+
+// Do not propagate host arch to device.
+// Passing arch to device is done via -Xopenmp-target flag.
+DAL->eraseArg(options::OPT_march_EQ);
+
+for (Arg *A : Args) {
+  unsigned Index = -1;
+  unsigned Prev;
+  bool XOpenMPTargetNoTriple = A->getOption().matches(
+  options::OPT_Xopenmp_target);
+
+  if (A->getOption().matches(options::OPT_Xopenmp_target_EQ)) {
+if (A->getValue(0) == getTripleString())
+  Index = Args.getBaseArgs().MakeIndex(A->getValue(1));
+  } else if (XOpenMPTargetNoTriple)
+// Passing device args: -fopenmp-target -opt=val.
+Index = Args.getBaseArgs().MakeIndex(A->getValue(0));
+  else
+continue;
+
+  // Parse the argument to -Xopenmp-target.
+  Prev = Index;
+  std::unique_ptr XOpenMPTargetArg(Opts.ParseOneArg(Args, Index));
+  if (!XOpenMPTargetArg || Index > Prev + 1) {
+getDriver().Diag(diag::err_drv_invalid_Xopenmp_target_with_args)
+<< A->getAsString(Args);
+continue;
+  }
+  if (XOpenMPTargetNoTriple && XOpenMPTargetArg &&
+  Args.getAllArgValues(
+  options::OPT_fopenmp_targets_EQ).size() != 1) {
+getDriver().Diag(diag::err_drv_Xopenmp_target_missing_triple);
+continue;
+  }
+  XOpenMPTargetArg->setBaseArg(A);
+  A = XOpenMPTargetArg.release();
+
+  // Ignore all but last -march=arch flag.
+  if (A->getOption().matches(options::OPT_march_EQ))
+DAL->eraseArg(options::OPT_march_EQ);
+  DAL->append(A);
+}
+
+auto MArchList = DAL->getAllArgValues(options::OPT_march_EQ);
+assert(MArchList.size() < 2 &&
+   "Too many archs under -Xopenmp-targets");
+if (MArchList.empty())
+  // Default compute capability for CUDA toolchain is sm_20.
+  DAL->AddJoinedArg(nullptr,
+  Opts.getOption(options::OPT_march_EQ), 

[PATCH] D35194: [clang-tidy] clang-apply-replacements: Don't insert null entry

[Yan Wang via Phabricator via cfe-commits]

yawanng added a comment.

LGTM. Please wait for Alexander approval.




Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:296
+ << "' doesn't exist. Ignoring...\n";
+}
 continue;

Maybe add some error message here if it's not an existence issue. like "invalid 
file path" or something? The same below.


https://reviews.llvm.org/D35194



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

[PATCH] D35230: [clang] buildFixItInsertionLine should use Hints of the same FID and LineNo

[Chih-Hung Hsieh via Phabricator via cfe-commits]

chh created this revision.

Fix bug https://bugs.llvm.org/show_bug.cgi?id=33734


https://reviews.llvm.org/D35230

Files:
  lib/Frontend/TextDiagnostic.cpp


Index: lib/Frontend/TextDiagnostic.cpp
===
--- lib/Frontend/TextDiagnostic.cpp
+++ lib/Frontend/TextDiagnostic.cpp
@@ -1052,7 +1052,8 @@
   std::fill(CaretLine.begin()+StartColNo,CaretLine.begin()+EndColNo,'~');
 }
 
-static std::string buildFixItInsertionLine(unsigned LineNo,
+static std::string buildFixItInsertionLine(FileID FID,
+   unsigned LineNo,
const SourceColumnMap ,
ArrayRef Hints,
const SourceManager ,
@@ -1069,7 +1070,8 @@
   // code contains no newlines and is on the same line as the caret.
   std::pair HintLocInfo
 = SM.getDecomposedExpansionLoc(I->RemoveRange.getBegin());
-  if (LineNo == SM.getLineNumber(HintLocInfo.first, HintLocInfo.second) &&
+  if (FID == HintLocInfo.first &&
+  LineNo == SM.getLineNumber(HintLocInfo.first, HintLocInfo.second) &&
   StringRef(I->CodeToInsert).find_first_of("\n\r") == StringRef::npos) 
{
 // Insert the new code into the line just below the code
 // that the user wrote.
@@ -1105,9 +1107,6 @@
 
 PrevHintEndCol =
   HintCol + llvm::sys::locale::columnWidth(I->CodeToInsert);
-  } else {
-FixItInsertionLine.clear();
-break;
   }
 }
   }
@@ -1222,7 +1221,7 @@
 }
 
 std::string FixItInsertionLine = buildFixItInsertionLine(
-LineNo, sourceColMap, Hints, SM, DiagOpts.get());
+FID, LineNo, sourceColMap, Hints, SM, DiagOpts.get());
 
 // If the source line is too long for our terminal, select only the
 // "interesting" source region within that line.


Index: lib/Frontend/TextDiagnostic.cpp
===
--- lib/Frontend/TextDiagnostic.cpp
+++ lib/Frontend/TextDiagnostic.cpp
@@ -1052,7 +1052,8 @@
   std::fill(CaretLine.begin()+StartColNo,CaretLine.begin()+EndColNo,'~');
 }
 
-static std::string buildFixItInsertionLine(unsigned LineNo,
+static std::string buildFixItInsertionLine(FileID FID,
+   unsigned LineNo,
const SourceColumnMap ,
ArrayRef Hints,
const SourceManager ,
@@ -1069,7 +1070,8 @@
   // code contains no newlines and is on the same line as the caret.
   std::pair HintLocInfo
 = SM.getDecomposedExpansionLoc(I->RemoveRange.getBegin());
-  if (LineNo == SM.getLineNumber(HintLocInfo.first, HintLocInfo.second) &&
+  if (FID == HintLocInfo.first &&
+  LineNo == SM.getLineNumber(HintLocInfo.first, HintLocInfo.second) &&
   StringRef(I->CodeToInsert).find_first_of("\n\r") == StringRef::npos) {
 // Insert the new code into the line just below the code
 // that the user wrote.
@@ -1105,9 +1107,6 @@
 
 PrevHintEndCol =
   HintCol + llvm::sys::locale::columnWidth(I->CodeToInsert);
-  } else {
-FixItInsertionLine.clear();
-break;
   }
 }
   }
@@ -1222,7 +1221,7 @@
 }
 
 std::string FixItInsertionLine = buildFixItInsertionLine(
-LineNo, sourceColMap, Hints, SM, DiagOpts.get());
+FID, LineNo, sourceColMap, Hints, SM, DiagOpts.get());
 
 // If the source line is too long for our terminal, select only the
 // "interesting" source region within that line.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r307601 - [CMake] Use tools template for clangd and modularize

[Petr Hosek via cfe-commits]

Author: phosek
Date: Mon Jul 10 17:18:07 2017
New Revision: 307601

URL: http://llvm.org/viewvc/llvm-project?rev=307601=rev
Log:
[CMake] Use tools template for clangd and modularize

This makes them usable as distribution components in the build.

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

Modified:
clang-tools-extra/trunk/clangd/tool/CMakeLists.txt
clang-tools-extra/trunk/modularize/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/tool/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/CMakeLists.txt?rev=307601=307600=307601=diff
==
--- clang-tools-extra/trunk/clangd/tool/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/tool/CMakeLists.txt Mon Jul 10 17:18:07 2017
@@ -1,6 +1,6 @@
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
 
-add_clang_executable(clangd
+add_clang_tool(clangd
   ClangdMain.cpp
   )
 

Modified: clang-tools-extra/trunk/modularize/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/CMakeLists.txt?rev=307601=307600=307601=diff
==
--- clang-tools-extra/trunk/modularize/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/modularize/CMakeLists.txt Mon Jul 10 17:18:07 2017
@@ -3,7 +3,7 @@ set(LLVM_LINK_COMPONENTS
   Support
   )
 
-add_clang_executable(modularize
+add_clang_tool(modularize
   Modularize.cpp
   ModuleAssistant.cpp
   ModularizeUtilities.cpp


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

[PATCH] D35216: [analyzer] Escape symbols when creating std::initializer_list.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:810
+public:
+  CollectReachableSymbolsCallback(ProgramStateRef State) {}
+  const InvalidatedSymbols () const { return Symbols; }

alexshap wrote:
> explicit ?
This code was moved from below, so i didn't bother changing it. Good point 
though.


https://reviews.llvm.org/D35216



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

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

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: test/Analysis/enum.cpp:50
+
+typedef __INTPTR_TYPE__ intptr_t;
+bool testNoCrashOnSwitchEnumBoolConstantCastedFromNullptr() {

Nice! Will use in my tests.


Repository:
  rL LLVM

https://reviews.llvm.org/D35041



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

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

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



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

gtbercea wrote:
> hfinkel wrote:
> > gtbercea wrote:
> > > hfinkel wrote:
> > > > Shouldn't you be adding all of the options, not just the -march= ones?
> > > I thought that that would be the case but there are a few issues:
> > > 
> > > 1. PTXAS and NVLINK each use a different flag for specifying the arch, 
> > > and, in turn, each flag is different from -march.
> > > 
> > > 2. -Xopenmp-target passes a flag to the entire toolchain not to 
> > > individual components of the toolchain so a translation of flags is 
> > > required in some cases to adapt the flag to the actual tool. -march is 
> > > one example, I'm not sure if there are others.
> > > 
> > > 3. At this point in the code, in order to add a flag and its value to the 
> > > DAL list, I need to be able to specify the option type (i.e. 
> > > options::OPT_march_EQ). I therefore need to manually recognize the flag 
> > > in the string representing the value of -Xopenmp-target or 
> > > -Xopenmp-target=triple.
> > > 
> > > 4. This patch handles the passing of the arch and can be extended to pass 
> > > other flags (as is stands, no other flags are passed through to the CUDA 
> > > toolchain). This can be extended on a flag by flag basis for flags that 
> > > need translating to a particular tool's flag. If the flag doesn't need 
> > > translating then the flag and it's value can be appended to the command 
> > > line as they are.
> > > 1. PTXAS and NVLINK each use a different flag for specifying the arch, 
> > > and, in turn, each flag is different from -march.
> > 
> > I don't understand why this is relevant. Don't 
> > NVPTX::Assembler::ConstructJob and NVPTX::Linker::ConstructJob handle that 
> > in either case?
> > 
> > This seems to be the same comment to point 2 as well.
> > 
> > > 3. At this point in the code, in order to add a flag and its value to the 
> > > DAL list, I need to be able to specify the option type (i.e. 
> > > options::OPT_march_EQ). I therefore need to manually recognize the flag 
> > > in the string representing the value of -Xopenmp-target or 
> > > -Xopenmp-target=triple.
> > 
> > I don't understand why this is true. Doesn't the code just below this, 
> > which handles -Xarch, do the general thing (it calls Opts.ParseOneArg and 
> > then adds it to the list of derived arguments)? Can't we handle this like 
> > -Xarch?
> > 
> > > This patch handles the passing of the arch and can be extended to pass 
> > > other flags (as is stands, no other flags are passed through to the CUDA 
> > > toolchain). This can be extended on a flag by flag basis for flags that 
> > > need translating to a particular tool's flag. If the flag doesn't need 
> > > translating then the flag and it's value can be appended to the command 
> > > line as they are.
> > 
> > I don't understand this either. If we need to extend this on a flag-by-flag 
> > basis, then it seems fundamentally broken. How could we append a flag to 
> > the command line without it also affecting the host compile?
> @hfinkel 
> 
> The problem is that when using -Xopenmp-target= -opt=val the value of 
> this flag is a list of two strings:
> 
> ['', '-opt=val']
> 
> What needs to happen next is to parse the string containing "-opt=val". The 
> reason I have to do this is because if I use -march, I can't pass -march as 
> is to PTXAS and NVLINK which have different flags for expressing the arch. I 
> need to translate the -march=sm_60 flag. I will have to do this for all flags 
> which require translation. There is no way I can just append this string to 
> the PTXAS and NVLINK commands because the flags for the 2 tools are 
> different. A flag which works for one of them, will not work for the other. 
> 
> So I need to actually parse that value to check whether it is a "-march" and 
> create an Arg object with the OPT_march_EQ identifier and the sm_60 value. 
> When invoking the commands for PTXAS and NVLINK, the dervied arg list will be 
> travered and every -march=sm_60 option will be transformed into "--gpu-name" 
> "sm_60" for PTXAS and into "-arch" "sm_60" for NVLINK. 
> 
> In the case of -Xarch, you will see that after we have traversed the entire 
> arg list we still have to special case -march and add it is manually added to 
> the DAL.
> 
> Let me know your thoughts on this.
> 
> Thanks,
> 
> --Doru
> What needs to happen next is to parse the string containing "-opt=val". 

Yes, that's what ParseOneArg will do.

> The reason I have to do this is because if I use -march, I can't pass -march 
> as is to PTXAS and NVLINK which have different flags for expressing the arch. 
> I need to translate the -march=sm_60 flag. I will have to do this for all 
> flags which require translation. There is no way I can just append this 
> string to the PTXAS and NVLINK commands because the flags 

[PATCH] D35225: [clang-tidy] add regression test to performance-unnecessary-value-param

[Chih-Hung Hsieh via Phabricator via cfe-commits]

chh added a comment.

These tests should be added after https://bugs.llvm.org/show_bug.cgi?id=33734 
is fixed.


https://reviews.llvm.org/D35225



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

[PATCH] D34981: RecursiveASTVisitor should visit the nested name qualifiers in a template specialisation

[Vedant Kumar via Phabricator via cfe-commits]

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

I haven't touched any AST code but this looks good to me: it's in line with 
what's done in TraverseRecordHelper and the test case is comprehensive.




Comment at: unittests/Tooling/RecursiveASTVisitorTest.cpp:269
+TEST(RecursiveASTVisitor,
+ NestedNameSpecifiersForTemplateSpecializatoinsAreVisited) {
+  StringRef Source = R"(

Nit, 'specializations'


Repository:
  rL LLVM

https://reviews.llvm.org/D34981



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

[PATCH] D35194: [clang-tidy] clang-apply-replacements: Don't insert null entry

[Kevin Funk via Phabricator via cfe-commits]

kfunk updated this revision to Diff 105928.
kfunk added a comment.

Add test


https://reviews.llvm.org/D35194

Files:
  clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml
  test/clang-apply-replacements/invalid-files.cpp


Index: test/clang-apply-replacements/invalid-files.cpp
===
--- /dev/null
+++ test/clang-apply-replacements/invalid-files.cpp
@@ -0,0 +1,8 @@
+// RUN: mkdir -p %T/Inputs/invalid-files
+// RUN: sed "s#\$(path)#%/T/Inputs/invalid-files#" 
%S/Inputs/invalid-files/invalid-files.yaml > 
%T/Inputs/invalid-files/invalid-files.yaml
+// RUN: clang-apply-replacements %T/Inputs/invalid-files
+//
+// Check that the yaml files are *not* deleted after running 
clang-apply-replacements without remove-change-desc-files.
+// RUN: ls -1 %T/Inputs/invalid-files | FileCheck %s --check-prefix=YAML
+//
+ YAML: {{.+\.yaml$}}
Index: test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml
===
--- /dev/null
+++ test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml
@@ -0,0 +1,12 @@
+---
+MainSourceFile:  ''
+Replacements:
+  - FilePath:idontexist.h
+Offset:  2669
+Length:  0
+ReplacementText: ' override'
+  - FilePath:idontexist.h
+Offset:  2669
+Length:  0
+ReplacementText: ' override'
+...
Index: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===
--- clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -289,9 +289,11 @@
   // Use the file manager to deduplicate paths. FileEntries are
   // automatically canonicalized.
   const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath());
-  if (!Entry && Warned.insert(R.getFilePath()).second) {
-errs() << "Described file '" << R.getFilePath()
-   << "' doesn't exist. Ignoring...\n";
+  if (!Entry) {
+if (Warned.insert(R.getFilePath()).second) {
+  errs() << "Described file '" << R.getFilePath()
+ << "' doesn't exist. Ignoring...\n";
+}
 continue;
   }
   GroupedReplacements[Entry].push_back(R);
@@ -315,9 +317,11 @@
   // Use the file manager to deduplicate paths. FileEntries are
   // automatically canonicalized.
   const FileEntry *Entry = 
SM.getFileManager().getFile(R.getFilePath());
-  if (!Entry && Warned.insert(R.getFilePath()).second) {
-errs() << "Described file '" << R.getFilePath()
-   << "' doesn't exist. Ignoring...\n";
+  if (!Entry) {
+if (Warned.insert(R.getFilePath()).second) {
+  errs() << "Described file '" << R.getFilePath()
+ << "' doesn't exist. Ignoring...\n";
+}
 continue;
   }
   GroupedReplacements[Entry].push_back(R);


Index: test/clang-apply-replacements/invalid-files.cpp
===
--- /dev/null
+++ test/clang-apply-replacements/invalid-files.cpp
@@ -0,0 +1,8 @@
+// RUN: mkdir -p %T/Inputs/invalid-files
+// RUN: sed "s#\$(path)#%/T/Inputs/invalid-files#" %S/Inputs/invalid-files/invalid-files.yaml > %T/Inputs/invalid-files/invalid-files.yaml
+// RUN: clang-apply-replacements %T/Inputs/invalid-files
+//
+// Check that the yaml files are *not* deleted after running clang-apply-replacements without remove-change-desc-files.
+// RUN: ls -1 %T/Inputs/invalid-files | FileCheck %s --check-prefix=YAML
+//
+ YAML: {{.+\.yaml$}}
Index: test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml
===
--- /dev/null
+++ test/clang-apply-replacements/Inputs/invalid-files/invalid-files.yaml
@@ -0,0 +1,12 @@
+---
+MainSourceFile:  ''
+Replacements:
+  - FilePath:idontexist.h
+Offset:  2669
+Length:  0
+ReplacementText: ' override'
+  - FilePath:idontexist.h
+Offset:  2669
+Length:  0
+ReplacementText: ' override'
+...
Index: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
===
--- clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -289,9 +289,11 @@
   // Use the file manager to deduplicate paths. FileEntries are
   // automatically canonicalized.
   const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath());
-  if (!Entry && Warned.insert(R.getFilePath()).second) {
-errs() << "Described file '" << R.getFilePath()
-   << "' doesn't exist. Ignoring...\n";
+   

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

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

gtbercea added a comment.

@hfinkel

I think I have something that works which is similar to what you were 
requesting. Please let me know your thoughts!

Thanks,
--Doru


https://reviews.llvm.org/D34784



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

Re: [PATCH] D34896: Enable the new PM + SamlePGO + ThinLTO testing.

[Dehao Chen via cfe-commits]

This test was originally added in https://reviews.llvm.org/D34721 with
clang change. It's kind of dup of the previous test
(-check-prefix=SAMPLEPGO) in terms of testing the clang bits. But we
want to make sure the new PM has the expected behavior. I guess it
would be acceptable to remove one of them, but I'd prefer to remove
the one with legacy PM. Let me know if you think I should do that.

Thanks,
Dehao

On Mon, Jul 10, 2017 at 9:45 AM, David Blaikie  wrote:
> Does this test any code in Clang? (given the test is being committed without
> any change in Clang, I'm guessing maybe not) - perhaps this test doesn't
> belong in Clang's test suite?
>
> Looks like changes/functionality in LTOPreLinkDefaultPipeline are tested in
> test/Other/new-pm-thinlto-defaults.ll at least?
>
> On Fri, Jun 30, 2017 at 10:05 AM Dehao Chen via Phabricator via cfe-commits
>  wrote:
>>
>> danielcdh created this revision.
>> Herald added subscribers: eraman, inglorion, mehdi_amini, sanjoy.
>>
>> This patch should be enabled after https://reviews.llvm.org/D34895
>>
>>
>> https://reviews.llvm.org/D34896
>>
>> Files:
>>   test/CodeGen/pgo-sample-thinlto-summary.c
>>
>>
>> Index: test/CodeGen/pgo-sample-thinlto-summary.c
>> ===
>> --- test/CodeGen/pgo-sample-thinlto-summary.c
>> +++ test/CodeGen/pgo-sample-thinlto-summary.c
>> @@ -1,9 +1,7 @@
>>  // RUN: %clang_cc1 -O2
>> -fprofile-sample-use=%S/Inputs/pgo-sample-thinlto-summary.prof %s -emit-llvm
>> -o - 2>&1 | FileCheck %s -check-prefix=SAMPLEPGO
>>  // RUN: %clang_cc1 -O2
>> -fprofile-sample-use=%S/Inputs/pgo-sample-thinlto-summary.prof %s -emit-llvm
>> -flto=thin -o - 2>&1 | FileCheck %s -check-prefix=THINLTO
>>  // RUN: %clang_cc1 -O2 -fexperimental-new-pass-manager
>> -fprofile-sample-use=%S/Inputs/pgo-sample-thinlto-summary.prof %s -emit-llvm
>> -o - 2>&1 | FileCheck %s -check-prefix=SAMPLEPGO
>> -// FIXME: Run the following command once LTOPreLinkDefaultPipeline is
>> -//customized.
>> -// %clang_cc1 -O2 -fexperimental-new-pass-manager
>> -fprofile-sample-use=%S/Inputs/pgo-sample-thinlto-summary.prof %s -emit-llvm
>> -flto=thin -o - 2>&1 | FileCheck %s -check-prefix=THINLTO
>> +// RUN: %clang_cc1 -O2 -fexperimental-new-pass-manager
>> -fprofile-sample-use=%S/Inputs/pgo-sample-thinlto-summary.prof %s -emit-llvm
>> -flto=thin -o - 2>&1 | FileCheck %s -check-prefix=THINLTO
>>  // Checks if hot call is inlined by normal compile, but not inlined by
>>  // thinlto compile.
>>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Patch: Preprocessor::HandleIncludeDirective()

[James Skerrett via cfe-commits]

The attached patch fixes what looks like a bug in the preprocessor when
dealing with missing include files.

Currently, if unable to find a file included via an angled include, the
preprocessor will generate an error diagnostic even when a recovery path is
provided via a callback client.

The patch modifies the if-statement such that the condition expression
matches the descriptive comment above it.


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

r307600 - clang-import-test had some dead code. I did the following to eliminate it:

[Sean Callanan via cfe-commits]

Author: spyffe
Date: Mon Jul 10 16:47:00 2017
New Revision: 307600

URL: http://llvm.org/viewvc/llvm-project?rev=307600=rev
Log:
clang-import-test had some dead code. I did the following to eliminate it:

- eliminated error handling for the indirect CompilerInstance, which should 
  never generate an error as it is created;
- added a new test for direct importation; and
- removed an unused implementation of the CompleteType() API.

This brings clang-import-test.cpp and ExternalASTMerge.cpp back to 100% 
coverage on all metrics measured by DLLVM_BUILD_INSTRUMENTED_COVERAGE.

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

Added:
cfe/trunk/test/Import/direct/
  - copied from r307452, cfe/trunk/test/Import/member-in-struct/
Modified:
cfe/trunk/include/clang/AST/ExternalASTMerger.h
cfe/trunk/lib/AST/ExternalASTMerger.cpp
cfe/trunk/test/Import/direct/test.c

Modified: cfe/trunk/include/clang/AST/ExternalASTMerger.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExternalASTMerger.h?rev=307600=307599=307600=diff
==
--- cfe/trunk/include/clang/AST/ExternalASTMerger.h (original)
+++ cfe/trunk/include/clang/AST/ExternalASTMerger.h Mon Jul 10 16:47:00 2017
@@ -44,10 +44,6 @@ public:
   FindExternalLexicalDecls(const DeclContext *DC,
llvm::function_ref IsKindWeWant,
SmallVectorImpl ) override;
-
-   using ExternalASTSource::CompleteType;
-
-   void CompleteType(TagDecl *Tag) override;
 };
 
 } // end namespace clang

Modified: cfe/trunk/lib/AST/ExternalASTMerger.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExternalASTMerger.cpp?rev=307600=307599=307600=diff
==
--- cfe/trunk/lib/AST/ExternalASTMerger.cpp (original)
+++ cfe/trunk/lib/AST/ExternalASTMerger.cpp Mon Jul 10 16:47:00 2017
@@ -180,8 +180,3 @@ void ExternalASTMerger::FindExternalLexi
   });
 }
 
-void ExternalASTMerger::CompleteType(TagDecl *Tag) {
-  SmallVector Result;
-  FindExternalLexicalDecls(Tag, [](Decl::Kind) { return true; }, Result);
-  Tag->setHasExternalLexicalStorage(false);
-}

Modified: cfe/trunk/test/Import/direct/test.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/direct/test.c?rev=307600=307452=307600=diff
==
--- cfe/trunk/test/Import/direct/test.c (original)
+++ cfe/trunk/test/Import/direct/test.c Mon Jul 10 16:47:00 2017
@@ -1,4 +1,4 @@
-// RUN: clang-import-test -import %S/Inputs/S.c -expression %s
+// RUN: clang-import-test -direct -import %S/Inputs/S.c -expression %s
 void expr() {
   struct S MyS;
   MyS.a = 3;


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

r307602 - [clang-import-test] Committed missed update to clang-import-test.cpp

[Sean Callanan via cfe-commits]

Author: spyffe
Date: Mon Jul 10 17:27:57 2017
New Revision: 307602

URL: http://llvm.org/viewvc/llvm-project?rev=307602=rev
Log:
[clang-import-test] Committed missed update to clang-import-test.cpp

I didn't commit clang-import-test.cpp in r307600, but it had some
changes that were part of https://reviews.llvm.org/D35220

Corrected that now.

Modified:
cfe/trunk/tools/clang-import-test/clang-import-test.cpp

Modified: cfe/trunk/tools/clang-import-test/clang-import-test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-import-test/clang-import-test.cpp?rev=307602=307601=307602=diff
==
--- cfe/trunk/tools/clang-import-test/clang-import-test.cpp (original)
+++ cfe/trunk/tools/clang-import-test/clang-import-test.cpp Mon Jul 10 17:27:57 
2017
@@ -182,14 +182,6 @@ BuildCompilerInstance(ArrayRef
-BuildCompilerInstance(ArrayRef ClangArgs) {
-  std::vector ClangArgv(ClangArgs.size());
-  std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),
- [](const std::string ) -> const char * { return s.data(); 
});
-  return init_convenience::BuildCompilerInstance(ClangArgv);
-}
-
 std::unique_ptr
 BuildASTContext(CompilerInstance , SelectorTable , Builtin::Context ) 
{
   auto AST = llvm::make_unique(
@@ -313,14 +305,8 @@ int main(int argc, const char **argv) {
   std::vector IndirectCIs;
   if (!Direct) {
 for (auto  : ImportCIs) {
-  llvm::Expected IndirectCI =
-  BuildIndirect(ImportCI);
-  if (auto E = IndirectCI.takeError()) {
-llvm::errs() << llvm::toString(std::move(E));
-exit(-1);
-  } else {
-IndirectCIs.push_back(std::move(*IndirectCI));
-  }
+  std::unique_ptr IndirectCI = BuildIndirect(ImportCI);
+  IndirectCIs.push_back(std::move(IndirectCI));
 }
   }
   llvm::Expected ExpressionCI =


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

[PATCH] D31326: Add option to export fixes to run-clang-tidy.py

[Michael F. Herbst via Phabricator via cfe-commits]

mfherbst updated this revision to Diff 105818.
mfherbst added a comment.

Correct nits and typos.


https://reviews.llvm.org/D31326

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


Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -35,6 +35,7 @@
 """
 
 import argparse
+import glob
 import json
 import multiprocessing
 import os
@@ -45,6 +46,7 @@
 import sys
 import tempfile
 import threading
+import yaml
 
 
 def find_compilation_database(path):
@@ -87,14 +89,36 @@
   return start
 
 
+def merge_replacement_files(tmpdir, mergefile):
+  """Merge all replacement files in a directory into a single file"""
+  # MainSourceFile: The key is required by the definition inside
+  # include/clang/Tooling/ReplacementsYaml.h, but the value
+  # is actually never used inside clang-apply-replacements,
+  # so we set it to '' here.
+  merged={ 'MainSourceFile': '', 'Replacements': [] }
+
+  for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')):
+with open(replacefile, 'r') as f:
+  content = yaml.safe_load(f)
+  if not content:
+continue # Skip empty files.
+
+try:
+  merged['Replacements'].extend(content['Replacements'])
+except KeyError:
+  pass # Ignore files with missing keys.
+
+  if merged['Replacements']:
+with open(mergefile,'w') as out:
+  yaml.safe_dump(merged, out)
+
 def apply_fixes(args, tmpdir):
-  """Calls clang-apply-fixes on a given directory. Deletes the dir when 
done."""
+  """Calls clang-apply-fixes on a given directory."""
   invocation = [args.clang_apply_replacements_binary]
   if args.format:
 invocation.append('-format')
   invocation.append(tmpdir)
   subprocess.call(invocation)
-  shutil.rmtree(tmpdir)
 
 
 def run_tidy(args, tmpdir, build_path, queue):
@@ -129,6 +153,9 @@
   'headers to output diagnostics from. Diagnostics from '
   'the main file of each translation unit are always '
   'displayed.')
+  parser.add_argument('-export-fixes', metavar='filename', dest='export_fixes',
+  help='Create a yaml file to store suggested fixes in, '
+  'which can be applied with clang-apply-replacements')
   parser.add_argument('-j', type=int, default=0,
   help='number of tidy instances to be run in parallel.')
   parser.add_argument('files', nargs='*', default=['.*'],
@@ -178,7 +205,7 @@
 max_task = multiprocessing.cpu_count()
 
   tmpdir = None
-  if args.fix:
+  if args.fix or args.export_fixes:
 tmpdir = tempfile.mkdtemp()
 
   # Build up a big regexy filter from all command line arguments.
@@ -205,13 +232,20 @@
 # This is a sad hack. Unfortunately subprocess goes
 # bonkers with ctrl-c and we start forking merrily.
 print '\nCtrl-C detected, goodbye.'
-if args.fix:
+if tmpdir:
   shutil.rmtree(tmpdir)
 os.kill(0, 9)
 
+  if args.export_fixes:
+print 'Writing fixes to ' + args.export_fixes
+merge_replacement_files(tmpdir, args.export_fixes)
+
   if args.fix:
 print 'Applying fixes ...'
 apply_fixes(args, tmpdir)
 
+  if tmpdir:
+shutil.rmtree(tmpdir)
+
 if __name__ == '__main__':
   main()


Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -35,6 +35,7 @@
 """
 
 import argparse
+import glob
 import json
 import multiprocessing
 import os
@@ -45,6 +46,7 @@
 import sys
 import tempfile
 import threading
+import yaml
 
 
 def find_compilation_database(path):
@@ -87,14 +89,36 @@
   return start
 
 
+def merge_replacement_files(tmpdir, mergefile):
+  """Merge all replacement files in a directory into a single file"""
+  # MainSourceFile: The key is required by the definition inside
+  # include/clang/Tooling/ReplacementsYaml.h, but the value
+  # is actually never used inside clang-apply-replacements,
+  # so we set it to '' here.
+  merged={ 'MainSourceFile': '', 'Replacements': [] }
+
+  for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')):
+with open(replacefile, 'r') as f:
+  content = yaml.safe_load(f)
+  if not content:
+continue # Skip empty files.
+
+try:
+  merged['Replacements'].extend(content['Replacements'])
+except KeyError:
+  pass # Ignore files with missing keys.
+
+  if merged['Replacements']:
+with open(mergefile,'w') as out:
+  yaml.safe_dump(merged, out)
+
 def apply_fixes(args, tmpdir):
-  """Calls clang-apply-fixes on a given directory. Deletes the dir when done."""
+  """Calls clang-apply-fixes on a given directory."""
   invocation = [args.clang_apply_replacements_binary]
   if args.format:
 invocation.append('-format')
   invocation.append(tmpdir)
   subprocess.call(invocation)
-  shutil.rmtree(tmpdir)
 
 
 def run_tidy(args, 

[PATCH] D31326: Add option to export fixes to run-clang-tidy.py

[Michael F. Herbst via Phabricator via cfe-commits]

mfherbst added inline comments.



Comment at: run-clang-tidy.py:93
+def merge_replacement_files(tmpdir, fixfile):
+  """Merge all replacement files in a directory into a single fixfile"""
+  # MainSourceFile: The key is required by the definition inside

alexfh wrote:
> I'm not sure "fixfile" is a word. Just "file" maybe?
I thought I would refer to the name of the input argument, but I guess you are 
right, file makes it less clunky to read.


https://reviews.llvm.org/D31326



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

[PATCH] D34331: func.wrap.func.con: Unset function before destroying anything

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D34331#803105, @dexonsmith wrote:

> In https://reviews.llvm.org/D34331#802747, @EricWF wrote:
>
> > @dexonsmith Mind if I hijack this and check in your changes to 
> > `` with my tests?
>
>
> Not at all.
>
> In https://reviews.llvm.org/D34331#802821, @EricWF wrote:
>
> > @dexonsmith I'm not sure it's sane to allow reentrant behavior. Could you 
> > explain why you think it is?
>
>
> It didn't seem sane to me at first either, despite this being supported by 
> `std::unique_ptr` and `std::shared_ptr`.  I found our user fairly convincing, 
> though:
>
> - They had an underlying reference counted object, so only the destruction of 
> the last copy of A nulled out the pointer (mimicked that with the `bool 
> cancel`).
> - They had a callback function intended to be called once, and dropping that 
> function on cancellation (mimicked that with a global variable). The cancel 
> operation nulled out a `std::function`, but destroying the objects captured 
> in the lambda in that std::function also separately decided to perform a 
> cancel, which should have been OK. The cancel function just contained 
> `callback = nullptr`.


According to [reentrancy] it is implementation defined what STL functions are 
allowed to be recursively reentered. I'm not opposed to providing reentrancy 
where it's useful, but we would have to provide it well.
This is where I was running into problems while I was writing tests. There are 
so many different ways you can reenter std::function's special members that it 
makes it impossible for an implementation
to truly support reentrancy as the standard would require.

If you consider that every possible copy construction or destruction of a user 
type is a potential reentrancy point, the complexity of having well-defined 
reentrant behavior starts to become clear.
Any time a copy constructor or destructor is invoked you have a potential 
reentrancy point which, in order to provide well defined behavior, must also 
act as a sort of _sequence point_ where the side effects of the current call 
are visible to the reentrant callers (For example your patches use of 
`operator=(nullptr_t)`).

The methods fixed in this patch are seemingly improvements; It's clear that 
`operator=(nullptr)` can safely be made reentrant. On the other hand consider 
`operator=(function const& rhs)`, which is specified as equivalent to 
`function(rhs).swap(*this)`.
I posit `swap` is not the kind of thing that can easily be made reentrant, but 
that making `std::function` assignment reentrant in general clearly requires it.

If fixing this bug is sufficiently important I'm willing to accept that, but I 
don't think it improves the status quo; We may have fixed a specific users bug, 
but we haven't made these functions safely reentrant (in fact most of the 
special members are likely still full of reentrancy bugs).


https://reviews.llvm.org/D34331



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

[PATCH] D35184: X86 Intrinsics: _bit_scan_forward should not be under #ifdef __RDRND__

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D35184



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

[PATCH] D35184: X86 Intrinsics: _bit_scan_forward should not be under #ifdef __RDRND__

[Zvi Rackover via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL307524: X86 Intrinsics: _bit_scan_forward should not be 
under #ifdef __RDRND__ (authored by zvi).

Repository:
  rL LLVM

https://reviews.llvm.org/D35184

Files:
  cfe/trunk/lib/Headers/immintrin.h
  cfe/trunk/test/CodeGen/bitscan-builtins.c


Index: cfe/trunk/test/CodeGen/bitscan-builtins.c
===
--- cfe/trunk/test/CodeGen/bitscan-builtins.c
+++ cfe/trunk/test/CodeGen/bitscan-builtins.c
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown -emit-llvm -o 
- %s | FileCheck %s
 
+// PR33722
+// RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown -D_MSC_VER 
-emit-llvm -o - %s | FileCheck %s
+
 #include 
 
 int test_bit_scan_forward(int a) {
Index: cfe/trunk/lib/Headers/immintrin.h
===
--- cfe/trunk/lib/Headers/immintrin.h
+++ cfe/trunk/lib/Headers/immintrin.h
@@ -212,6 +212,15 @@
   return __builtin_ia32_rdrand32_step(__p);
 }
 
+#ifdef __x86_64__
+static __inline__ int __attribute__((__always_inline__, __nodebug__, 
__target__("rdrnd")))
+_rdrand64_step(unsigned long long *__p)
+{
+  return __builtin_ia32_rdrand64_step(__p);
+}
+#endif
+#endif /* __RDRND__ */
+
 /* __bit_scan_forward */
 static __inline__ int __attribute__((__always_inline__, __nodebug__))
 _bit_scan_forward(int __A) {
@@ -224,15 +233,6 @@
   return 31 - __builtin_clz(__A);
 }
 
-#ifdef __x86_64__
-static __inline__ int __attribute__((__always_inline__, __nodebug__, 
__target__("rdrnd")))
-_rdrand64_step(unsigned long long *__p)
-{
-  return __builtin_ia32_rdrand64_step(__p);
-}
-#endif
-#endif /* __RDRND__ */
-
 #if !defined(_MSC_VER) || __has_feature(modules) || defined(__FSGSBASE__)
 #ifdef __x86_64__
 static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, 
__target__("fsgsbase")))


Index: cfe/trunk/test/CodeGen/bitscan-builtins.c
===
--- cfe/trunk/test/CodeGen/bitscan-builtins.c
+++ cfe/trunk/test/CodeGen/bitscan-builtins.c
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
 
+// PR33722
+// RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown -D_MSC_VER -emit-llvm -o - %s | FileCheck %s
+
 #include 
 
 int test_bit_scan_forward(int a) {
Index: cfe/trunk/lib/Headers/immintrin.h
===
--- cfe/trunk/lib/Headers/immintrin.h
+++ cfe/trunk/lib/Headers/immintrin.h
@@ -212,6 +212,15 @@
   return __builtin_ia32_rdrand32_step(__p);
 }
 
+#ifdef __x86_64__
+static __inline__ int __attribute__((__always_inline__, __nodebug__, __target__("rdrnd")))
+_rdrand64_step(unsigned long long *__p)
+{
+  return __builtin_ia32_rdrand64_step(__p);
+}
+#endif
+#endif /* __RDRND__ */
+
 /* __bit_scan_forward */
 static __inline__ int __attribute__((__always_inline__, __nodebug__))
 _bit_scan_forward(int __A) {
@@ -224,15 +233,6 @@
   return 31 - __builtin_clz(__A);
 }
 
-#ifdef __x86_64__
-static __inline__ int __attribute__((__always_inline__, __nodebug__, __target__("rdrnd")))
-_rdrand64_step(unsigned long long *__p)
-{
-  return __builtin_ia32_rdrand64_step(__p);
-}
-#endif
-#endif /* __RDRND__ */
-
 #if !defined(_MSC_VER) || __has_feature(modules) || defined(__FSGSBASE__)
 #ifdef __x86_64__
 static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, __target__("fsgsbase")))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35172: Keep the IdentifierInfo in the Token for alternative operator keyword

[Olivier Goffart via Phabricator via cfe-commits]

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



Comment at: lib/Lex/PPExpressions.cpp:242
   switch (PeekTok.getKind()) {
-  default:  // Non-value token.
+  default:
+// If this token's spelling is a pp-identifier, check to see if it is

rsmith wrote:
> I'm concerned that this will do the wrong thing for a keyword operator that 
> is not permitted in a pp expression, like and_eq. It seems safer to revert 
> this change and instead add a isCPlusPlusOperatorKeyword check to the "if" 
> condition above.
Well spotted.  I kept this code in the default:  so true and false are handled 
separatly, but added a condition for isCPlusPlusOperatorKeyword, and an 
additional test.


https://reviews.llvm.org/D35172



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

[PATCH] D35172: Keep the IdentifierInfo in the Token for alternative operator keyword

[Olivier Goffart via Phabricator via cfe-commits]

ogoffart updated this revision to Diff 105815.
ogoffart marked an inline comment as done.
ogoffart added a comment.

Added check for  "#if and_eq"


https://reviews.llvm.org/D35172

Files:
  include/clang/Basic/IdentifierTable.h
  lib/Lex/PPDirectives.cpp
  lib/Lex/PPExpressions.cpp
  lib/Lex/Preprocessor.cpp
  test/Parser/MicrosoftExtensions.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -333,6 +333,16 @@
   verifyFormat("x = (a) xor (b);");
 }
 
+TEST_F(FormatTest, RecognizesUnaryOperatorKeywords) {
+  verifyFormat("x = compl(a);");
+  verifyFormat("x = not(a);");
+  verifyFormat("x = bitand(a);");
+  // Unary operator must not be merged with the next identifier
+  verifyFormat("x = compl a;");
+  verifyFormat("x = not a;");
+  verifyFormat("x = bitand a;");
+}
+
 //===--===//
 // Tests for control statements.
 //===--===//
Index: test/Parser/MicrosoftExtensions.cpp
===
--- test/Parser/MicrosoftExtensions.cpp
+++ test/Parser/MicrosoftExtensions.cpp
@@ -261,9 +261,7 @@
 #define identifier_weird(x) __identifier(x
 int k = identifier_weird(if)); // expected-error {{use of undeclared identifier 'if'}}
 
-// This is a bit weird, but the alternative tokens aren't keywords, and this
-// behavior matches MSVC. FIXME: Consider supporting this anyway.
-extern int __identifier(and) r; // expected-error {{cannot convert '&&' token to an identifier}}
+extern int __identifier(and);
 
 void f() {
   __identifier(() // expected-error {{cannot convert '(' token to an identifier}}
Index: lib/Lex/Preprocessor.cpp
===
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -712,14 +712,6 @@
 II.setIsFutureCompatKeyword(false);
   }
 
-  // C++ 2.11p2: If this is an alternative representation of a C++ operator,
-  // then we act as if it is the actual operator and not the textual
-  // representation of it.
-  if (II.isCPlusPlusOperatorKeyword() &&
-  !(getLangOpts().MSVCCompat &&
-getSourceManager().isInSystemHeader(Identifier.getLocation(
-Identifier.setIdentifierInfo(nullptr);
-
   // If this is an extension token, diagnose its use.
   // We avoid diagnosing tokens that originate from macro definitions.
   // FIXME: This warning is disabled in cases where it shouldn't be,
Index: lib/Lex/PPExpressions.cpp
===
--- lib/Lex/PPExpressions.cpp
+++ lib/Lex/PPExpressions.cpp
@@ -237,35 +237,30 @@
 PP.setCodeCompletionReached();
 PP.LexNonComment(PeekTok);
   }
-  
-  // If this token's spelling is a pp-identifier, check to see if it is
-  // 'defined' or if it is a macro.  Note that we check here because many
-  // keywords are pp-identifiers, so we can't check the kind.
-  if (IdentifierInfo *II = PeekTok.getIdentifierInfo()) {
-// Handle "defined X" and "defined(X)".
-if (II->isStr("defined"))
-  return EvaluateDefined(Result, PeekTok, DT, ValueLive, PP);
-
-// If this identifier isn't 'defined' or one of the special
-// preprocessor keywords and it wasn't macro expanded, it turns
-// into a simple 0, unless it is the C++ keyword "true", in which case it
-// turns into "1".
-if (ValueLive &&
-II->getTokenID() != tok::kw_true &&
-II->getTokenID() != tok::kw_false)
-  PP.Diag(PeekTok, diag::warn_pp_undef_identifier) << II;
-Result.Val = II->getTokenID() == tok::kw_true;
-Result.Val.setIsUnsigned(false);  // "0" is signed intmax_t 0.
-Result.setIdentifier(II);
-Result.setRange(PeekTok.getLocation());
-DT.IncludedUndefinedIds = (II->getTokenID() != tok::kw_true &&
-   II->getTokenID() != tok::kw_false);
-PP.LexNonComment(PeekTok);
-return false;
-  }
 
   switch (PeekTok.getKind()) {
-  default:  // Non-value token.
+  default:
+// If this token's spelling is a pp-identifier, check to see if it is
+// 'defined' or if it is a macro.  Note that we check here because many
+// keywords are pp-identifiers, so we can't check the kind.
+if (IdentifierInfo *II = PeekTok.getIdentifierInfo()) {
+  // Handle "defined X" and "defined(X)".
+  if (II->isStr("defined"))
+return EvaluateDefined(Result, PeekTok, DT, ValueLive, PP);
+
+  // If this identifier isn't 'defined' or one of the special
+  // preprocessor keywords and it wasn't macro expanded, it turns
+  // into a simple 0
+  if (ValueLive)
+PP.Diag(PeekTok, diag::warn_pp_undef_identifier) << II;
+  Result.Val = 0;
+  Result.Val.setIsUnsigned(false); // "0" is signed intmax_t 0.
+  

[PATCH] D35172: Keep the IdentifierInfo in the Token for alternative operator keyword

[Olivier Goffart via Phabricator via cfe-commits]

ogoffart updated this revision to Diff 105816.

https://reviews.llvm.org/D35172

Files:
  include/clang/Basic/IdentifierTable.h
  lib/Lex/PPDirectives.cpp
  lib/Lex/PPExpressions.cpp
  lib/Lex/Preprocessor.cpp
  test/Parser/MicrosoftExtensions.cpp
  test/Preprocessor/cxx_oper_keyword.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -333,6 +333,16 @@
   verifyFormat("x = (a) xor (b);");
 }
 
+TEST_F(FormatTest, RecognizesUnaryOperatorKeywords) {
+  verifyFormat("x = compl(a);");
+  verifyFormat("x = not(a);");
+  verifyFormat("x = bitand(a);");
+  // Unary operator must not be merged with the next identifier
+  verifyFormat("x = compl a;");
+  verifyFormat("x = not a;");
+  verifyFormat("x = bitand a;");
+}
+
 //===--===//
 // Tests for control statements.
 //===--===//
Index: test/Preprocessor/cxx_oper_keyword.cpp
===
--- test/Preprocessor/cxx_oper_keyword.cpp
+++ test/Preprocessor/cxx_oper_keyword.cpp
@@ -29,3 +29,15 @@
 #ifdef and
 #warning and is defined
 #endif
+
+#ifdef OPERATOR_NAMES
+//expected-error@+2 {{invalid token at start of a preprocessor expression}}
+#endif
+#if or
+#endif
+
+#ifdef OPERATOR_NAMES
+//expected-error@+2 {{invalid token at start of a preprocessor expression}}
+#endif
+#if and_eq
+#endif
Index: test/Parser/MicrosoftExtensions.cpp
===
--- test/Parser/MicrosoftExtensions.cpp
+++ test/Parser/MicrosoftExtensions.cpp
@@ -261,9 +261,7 @@
 #define identifier_weird(x) __identifier(x
 int k = identifier_weird(if)); // expected-error {{use of undeclared identifier 'if'}}
 
-// This is a bit weird, but the alternative tokens aren't keywords, and this
-// behavior matches MSVC. FIXME: Consider supporting this anyway.
-extern int __identifier(and) r; // expected-error {{cannot convert '&&' token to an identifier}}
+extern int __identifier(and);
 
 void f() {
   __identifier(() // expected-error {{cannot convert '(' token to an identifier}}
Index: lib/Lex/Preprocessor.cpp
===
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -710,14 +710,6 @@
 II.setIsFutureCompatKeyword(false);
   }
 
-  // C++ 2.11p2: If this is an alternative representation of a C++ operator,
-  // then we act as if it is the actual operator and not the textual
-  // representation of it.
-  if (II.isCPlusPlusOperatorKeyword() &&
-  !(getLangOpts().MSVCCompat &&
-getSourceManager().isInSystemHeader(Identifier.getLocation(
-Identifier.setIdentifierInfo(nullptr);
-
   // If this is an extension token, diagnose its use.
   // We avoid diagnosing tokens that originate from macro definitions.
   // FIXME: This warning is disabled in cases where it shouldn't be,
Index: lib/Lex/PPExpressions.cpp
===
--- lib/Lex/PPExpressions.cpp
+++ lib/Lex/PPExpressions.cpp
@@ -237,35 +237,32 @@
 PP.setCodeCompletionReached();
 PP.LexNonComment(PeekTok);
   }
-  
-  // If this token's spelling is a pp-identifier, check to see if it is
-  // 'defined' or if it is a macro.  Note that we check here because many
-  // keywords are pp-identifiers, so we can't check the kind.
-  if (IdentifierInfo *II = PeekTok.getIdentifierInfo()) {
-// Handle "defined X" and "defined(X)".
-if (II->isStr("defined"))
-  return EvaluateDefined(Result, PeekTok, DT, ValueLive, PP);
-
-// If this identifier isn't 'defined' or one of the special
-// preprocessor keywords and it wasn't macro expanded, it turns
-// into a simple 0, unless it is the C++ keyword "true", in which case it
-// turns into "1".
-if (ValueLive &&
-II->getTokenID() != tok::kw_true &&
-II->getTokenID() != tok::kw_false)
-  PP.Diag(PeekTok, diag::warn_pp_undef_identifier) << II;
-Result.Val = II->getTokenID() == tok::kw_true;
-Result.Val.setIsUnsigned(false);  // "0" is signed intmax_t 0.
-Result.setIdentifier(II);
-Result.setRange(PeekTok.getLocation());
-DT.IncludedUndefinedIds = (II->getTokenID() != tok::kw_true &&
-   II->getTokenID() != tok::kw_false);
-PP.LexNonComment(PeekTok);
-return false;
-  }
 
   switch (PeekTok.getKind()) {
-  default:  // Non-value token.
+  default:
+// If this token's spelling is a pp-identifier, check to see if it is
+// 'defined' or if it is a macro.  Note that we check here because many
+// keywords are pp-identifiers, so we can't check the kind.
+if (IdentifierInfo *II = PeekTok.getIdentifierInfo()) {
+  // Handle "defined X" and 

[PATCH] D35190: __builtin_constant_p should consider the parameter of a constexpr function as constant

[Olivier Goffart via Phabricator via cfe-commits]

ogoffart created this revision.

This allows to do something like:

  
  constexpr int func(int x) {
return __builtin_constant_p(x) ? compute_constexpr(x) : compute_runtime(x);
  }


This kind of code is accepted by GCC since GCC 4.8.

  

The problem was that EvaluateBuiltinConstantP was discarding the EvalInfo and 
its frames. So just keep the EvalInfo


https://reviews.llvm.org/D35190

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


Index: test/SemaCXX/constant-expression-cxx1y.cpp
===
--- test/SemaCXX/constant-expression-cxx1y.cpp
+++ test/SemaCXX/constant-expression-cxx1y.cpp
@@ -982,3 +982,8 @@
   int *p = 
   p += (__int128)(unsigned long)-1; // expected-note {{cannot refer to element 
18446744073709551615 of non-array object in a constant expression}}
 }
+
+namespace BuiltinConstantP {
+  constexpr int f1(int i) { return __builtin_constant_p(i += 4) ? 0 : i += 3; }
+  static_assert(f1(10) == 13, "");
+}
Index: test/SemaCXX/constant-expression-cxx11.cpp
===
--- test/SemaCXX/constant-expression-cxx11.cpp
+++ test/SemaCXX/constant-expression-cxx11.cpp
@@ -2169,3 +2169,13 @@
   constexpr int *q = ( + 1) - (unsigned __int128)-1; // expected-error 
{{constant expression}} expected-note {{cannot refer to element -3402}}
   constexpr int *r = &( + 1)[(unsigned __int128)-1]; // expected-error 
{{constant expression}} expected-note {{cannot refer to element 3402}}
 }
+
+namespace BuiltinConstantP {
+  int computeRuntime(int);
+  constexpr int compute(int x) {
+return __builtin_constant_p(x) ? x + 2 : computeRuntime(x);
+  }
+  constexpr int x = compute(25);
+  int n; // expected-note{{declared here}}
+  constexpr int z = compute(n); // expected-error {{constant expression}} 
expected-note{{non-const variable}}
+}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -7203,7 +7203,7 @@
 
 /// EvaluateBuiltinConstantP - Evaluate __builtin_constant_p as similarly to
 /// GCC as we can manage.
-static bool EvaluateBuiltinConstantP(ASTContext , const Expr *Arg) {
+static bool EvaluateBuiltinConstantP(EvalInfo , const Expr *Arg) {
   QualType ArgType = Arg->getType();
 
   // __builtin_constant_p always has one operand. The rules which gcc follows
@@ -7219,25 +7219,24 @@
   //
   // FIXME: GCC also intends to return 1 for literals of aggregate types, but
   // its support for this does not currently work.
-  if (ArgType->isIntegralOrEnumerationType()) {
-Expr::EvalResult Result;
-if (!Arg->EvaluateAsRValue(Result, Ctx) || Result.HasSideEffects)
+  SpeculativeEvaluationRAII SpeculativeEval(Info);
+  FoldConstant Fold(Info, true);
+  if (ArgType->isIntegralOrEnumerationType() || ArgType->isFloatingType() ||
+  ArgType->isAnyComplexType()) {
+APValue V;
+if (!EvaluateAsRValue(Info, Arg, V) || Info.EvalStatus.HasSideEffects)
   return false;
-
-APValue  = Result.Val;
-if (V.getKind() == APValue::Int)
+if (V.getKind() == APValue::Int || V.getKind() == APValue::Float ||
+V.getKind() == APValue::ComplexInt ||
+V.getKind() == APValue::ComplexFloat)
   return true;
 if (V.getKind() == APValue::LValue)
   return EvaluateBuiltinConstantPForLValue(V);
-  } else if (ArgType->isFloatingType() || ArgType->isAnyComplexType()) {
-return Arg->isEvaluatable(Ctx);
   } else if (ArgType->isPointerType() || Arg->isGLValue()) {
 LValue LV;
-Expr::EvalStatus Status;
-EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantFold);
 if ((Arg->isGLValue() ? EvaluateLValue(Arg, LV, Info)
   : EvaluatePointer(Arg, LV, Info)) &&
-!Status.HasSideEffects)
+!Info.EvalStatus.HasSideEffects)
   return EvaluateBuiltinConstantPForLValue(LV);
   }
 
@@ -7628,7 +7627,7 @@
   }
 
   case Builtin::BI__builtin_constant_p:
-return Success(EvaluateBuiltinConstantP(Info.Ctx, E->getArg(0)), E);
+return Success(EvaluateBuiltinConstantP(Info, E->getArg(0)), E);
 
   case Builtin::BI__builtin_ctz:
   case Builtin::BI__builtin_ctzl:


Index: test/SemaCXX/constant-expression-cxx1y.cpp
===
--- test/SemaCXX/constant-expression-cxx1y.cpp
+++ test/SemaCXX/constant-expression-cxx1y.cpp
@@ -982,3 +982,8 @@
   int *p = 
   p += (__int128)(unsigned long)-1; // expected-note {{cannot refer to element 18446744073709551615 of non-array object in a constant expression}}
 }
+
+namespace BuiltinConstantP {
+  constexpr int f1(int i) { return __builtin_constant_p(i += 4) ? 0 : i += 3; }
+  static_assert(f1(10) == 13, "");
+}
Index: test/SemaCXX/constant-expression-cxx11.cpp
===
--- 

[PATCH] D34624: extra test modifications for D34158

[Melanie Blower via Phabricator via cfe-commits]

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

With the latest proposed fix for https://reviews.llvm.org/D34158, a few more 
test corrections are needed. The correction consists of suppressing the new 
preprocessor behavior. https://reviews.llvm.org/D34158 preincludes the file 
 if the file is available.


https://reviews.llvm.org/D34624

Files:
  test/clang-tidy/llvm-include-order.cpp
  test/clang-tidy/misc-move-constructor-init.cpp
  test/clang-tidy/misc-unconventional-assign-operator.cpp
  test/clang-tidy/misc-unused-using-decls.cpp
  test/clang-tidy/modernize-deprecated-headers-cxx03.cpp
  test/clang-tidy/modernize-deprecated-headers-cxx11.cpp
  test/clang-tidy/modernize-pass-by-value-macro-header.cpp
  test/clang-tidy/readability-identifier-naming.cpp
  test/pp-trace/pp-trace-conditional.cpp
  test/pp-trace/pp-trace-ident.cpp
  test/pp-trace/pp-trace-include.cpp
  test/pp-trace/pp-trace-macro.cpp
  test/pp-trace/pp-trace-modules.cpp
  test/pp-trace/pp-trace-pragma-general.cpp
  test/pp-trace/pp-trace-pragma-ms.cpp
  test/pp-trace/pp-trace-pragma-opencl.cpp

Index: test/pp-trace/pp-trace-pragma-ms.cpp
===
--- test/pp-trace/pp-trace-pragma-ms.cpp
+++ test/pp-trace/pp-trace-pragma-ms.cpp
@@ -1,4 +1,4 @@
-// RUN: pp-trace -ignore FileChanged,MacroDefined %s -target x86_64 -fms-extensions -w | FileCheck --strict-whitespace %s
+// RUN: pp-trace -ignore FileChanged,MacroDefined %s -target x86_64 -fms-extensions -w -ffreestanding | FileCheck --strict-whitespace %s
 
 #pragma comment(compiler, "compiler comment")
 #pragma comment(exestr, "exestr comment")
Index: test/pp-trace/pp-trace-include.cpp
===
--- test/pp-trace/pp-trace-include.cpp
+++ test/pp-trace/pp-trace-include.cpp
@@ -1,4 +1,4 @@
-// RUN: pp-trace %s -undef -target x86_64 -std=c++11 | FileCheck --strict-whitespace %s
+// RUN: pp-trace %s -undef -ffreestanding -target x86_64 -std=c++11 | FileCheck --strict-whitespace %s
 
 #include "Inputs/Level1A.h"
 #include "Inputs/Level1B.h"
Index: test/pp-trace/pp-trace-ident.cpp
===
--- test/pp-trace/pp-trace-ident.cpp
+++ test/pp-trace/pp-trace-ident.cpp
@@ -1,4 +1,4 @@
-// RUN: pp-trace -ignore FileChanged,MacroDefined %s -undef -target x86_64 -std=c++11 | FileCheck --strict-whitespace %s
+// RUN: pp-trace -ignore FileChanged,MacroDefined %s -undef -ffreestanding -target x86_64 -std=c++11 | FileCheck --strict-whitespace %s
 
 #ident "$Id$"
 
Index: test/pp-trace/pp-trace-conditional.cpp
===
--- test/pp-trace/pp-trace-conditional.cpp
+++ test/pp-trace/pp-trace-conditional.cpp
@@ -1,4 +1,4 @@
-// RUN: pp-trace -ignore FileChanged %s -undef -target x86_64 -std=c++11 | FileCheck --strict-whitespace %s
+// RUN: pp-trace -ignore FileChanged %s -ffreestanding -undef -target x86_64 -std=c++11 | FileCheck --strict-whitespace %s
 
 #if 1
 #endif
Index: test/pp-trace/pp-trace-pragma-general.cpp
===
--- test/pp-trace/pp-trace-pragma-general.cpp
+++ test/pp-trace/pp-trace-pragma-general.cpp
@@ -1,4 +1,4 @@
-// RUN: pp-trace -ignore FileChanged,MacroDefined %s | FileCheck --strict-whitespace %s
+// RUN: pp-trace -ignore FileChanged,MacroDefined %s -ffreestanding | FileCheck --strict-whitespace %s
 
 #pragma clang diagnostic push
 #pragma clang diagnostic pop
Index: test/pp-trace/pp-trace-macro.cpp
===
--- test/pp-trace/pp-trace-macro.cpp
+++ test/pp-trace/pp-trace-macro.cpp
@@ -1,4 +1,4 @@
-// RUN: pp-trace -ignore FileChanged %s -undef -target x86_64 -std=c++11 | FileCheck --strict-whitespace %s
+// RUN: pp-trace -ignore FileChanged %s -undef -ffreestanding -target x86_64 -std=c++11 | FileCheck --strict-whitespace %s
 
 #define MACRO 1
 int i = MACRO;
Index: test/pp-trace/pp-trace-modules.cpp
===
--- test/pp-trace/pp-trace-modules.cpp
+++ test/pp-trace/pp-trace-modules.cpp
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: pp-trace -ignore FileChanged,MacroDefined %s -x objective-c++ -undef -target x86_64 -std=c++11 -fmodules -fcxx-modules -fmodules-cache-path=%t -I%S -I%S/Input | FileCheck --strict-whitespace %s
+// RUN: pp-trace -ignore FileChanged,MacroDefined %s -x objective-c++ -undef -ffreestanding -target x86_64 -std=c++11 -fmodules -fcxx-modules -fmodules-cache-path=%t -I%S -I%S/Input | FileCheck --strict-whitespace %s
 
 // CHECK: ---
 
Index: test/pp-trace/pp-trace-pragma-opencl.cpp
===
--- test/pp-trace/pp-trace-pragma-opencl.cpp
+++ test/pp-trace/pp-trace-pragma-opencl.cpp
@@ -1,4 +1,4 @@
-// RUN: pp-trace -ignore FileChanged,MacroDefined %s -x cl | FileCheck --strict-whitespace %s
+// 

r307551 - Avoid white spaces in file names. NFC

[Eric Liu via cfe-commits]

Author: ioeric
Date: Mon Jul 10 09:05:48 2017
New Revision: 307551

URL: http://llvm.org/viewvc/llvm-project?rev=307551=rev
Log:
Avoid white spaces in file names. NFC

Summary: White spaces in file names are causing Phabricator/SVN to crash.

Reviewers: bkramer

Subscribers: bogner, cfe-commits

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

Added:
cfe/trunk/test/Driver/crash-report-spaces.c
  - copied, changed from r307524, cfe/trunk/test/Driver/crash report 
spaces.c
Removed:
cfe/trunk/test/Driver/crash report spaces.c

Removed: cfe/trunk/test/Driver/crash report spaces.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/crash%20report%20spaces.c?rev=307550=auto
==
--- cfe/trunk/test/Driver/crash report spaces.c (original)
+++ cfe/trunk/test/Driver/crash report spaces.c (removed)
@@ -1,18 +0,0 @@
-// RUN: rm -rf "%t"
-// RUN: mkdir "%t"
-// RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTIONS=1 %clang 
-fsyntax-only "%s" 2>&1 | FileCheck "%s"
-// RUN: cat "%t/crash report spaces"-*.c | FileCheck --check-prefix=CHECKSRC 
"%s"
-// RUN: cat "%t/crash report spaces"-*.sh | FileCheck --check-prefix=CHECKSH 
"%s"
-// REQUIRES: crash-recovery
-
-// because of the glob (*.c, *.sh)
-// REQUIRES: shell
-
-#pragma clang __debug parser_crash
-// CHECK: Preprocessed source(s) and associated run script(s) are located at:
-// CHECK-NEXT: note: diagnostic msg: {{.*}}.c
-FOO
-// CHECKSRC: FOO
-// CHECKSH: "-cc1"
-// CHECKSH: "-main-file-name" "crash report spaces.c"
-// CHECKSH: "crash report spaces-{{[^ ]*}}.c"

Copied: cfe/trunk/test/Driver/crash-report-spaces.c (from r307524, 
cfe/trunk/test/Driver/crash report spaces.c)
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/crash-report-spaces.c?p2=cfe/trunk/test/Driver/crash-report-spaces.c=cfe/trunk/test/Driver/crash%20report%20spaces.c=307524=307551=307551=diff
==
--- cfe/trunk/test/Driver/crash report spaces.c (original)
+++ cfe/trunk/test/Driver/crash-report-spaces.c Mon Jul 10 09:05:48 2017
@@ -1,6 +1,7 @@
 // RUN: rm -rf "%t"
 // RUN: mkdir "%t"
-// RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTIONS=1 %clang 
-fsyntax-only "%s" 2>&1 | FileCheck "%s"
+// RUN: cp "%s" "%t/crash report spaces.c"
+// RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTIONS=1 %clang 
-fsyntax-only "%t/crash report spaces.c" 2>&1 | FileCheck "%s"
 // RUN: cat "%t/crash report spaces"-*.c | FileCheck --check-prefix=CHECKSRC 
"%s"
 // RUN: cat "%t/crash report spaces"-*.sh | FileCheck --check-prefix=CHECKSH 
"%s"
 // REQUIRES: crash-recovery


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

[PATCH] D35216: [analyzer] Escape symbols when creating std::initializer_list.

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:810
+public:
+  CollectReachableSymbolsCallback(ProgramStateRef State) {}
+  const InvalidatedSymbols () const { return Symbols; }

explicit ?


https://reviews.llvm.org/D35216



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

[PATCH] D34158: For standards compatibility, preinclude if the file is available

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 105904.
mibintc edited the summary of this revision.
mibintc added a comment.

I removed the check on -nostdinc; and made some updates to the test cases


Repository:
  rL LLVM

https://reviews.llvm.org/D34158

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Lex/PreprocessorOptions.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/InitPreprocessor.cpp
  test/Driver/Inputs/stdc-predef/bin/.keep
  test/Driver/Inputs/stdc-predef/usr/i386-unknown-linux/lib/.keep
  test/Driver/Inputs/stdc-predef/usr/include/stdc-predef.h
  test/Driver/Inputs/stdc-predef/usr/lib/.keep
  test/Driver/Inputs/stdc-predef/usr/x86_64-unknown-linux/lib/.keep
  test/Driver/clang_cpp.c
  test/Driver/crash report spaces.c
  test/Driver/crash-report-header.h
  test/Driver/crash-report.c
  test/Driver/rewrite-legacy-objc.m
  test/Driver/rewrite-map-in-diagnostics.c
  test/Driver/rewrite-objc.m
  test/Driver/stdc-predef-not.c
  test/Driver/stdc-predef.c
  test/Index/IBOutletCollection.m
  test/Index/annotate-macro-args.m
  test/Index/annotate-tokens-pp.c
  test/Index/annotate-tokens.c
  test/Index/c-index-getCursor-test.m
  test/Index/get-cursor-macro-args.m
  test/Index/get-cursor.cpp
  test/Preprocessor/ignore-pragmas.c
  unittests/Tooling/TestVisitor.h

Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3175,6 +3175,11 @@
   KernelOrKext) {
 CmdArgs.push_back("-ffreestanding");
 IsHosted = false;
+  } else {
+// For standards compliance, clang will preinclude 
+// -ffreestanding suppresses this behavior.
+CmdArgs.push_back("-finclude-if-exists");
+CmdArgs.push_back("stdc-predef.h");
   }
 
   // Forward -f (flag) options which we can pass directly.
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2425,6 +2425,10 @@
   for (const Arg *A : Args.filtered(OPT_chain_include))
 Opts.ChainedIncludes.emplace_back(A->getValue());
 
+  // Add the ordered list of -finclude-if-exists.
+  for (const Arg *A : Args.filtered(OPT_finclude_if_exists))
+Opts.FIncludeIfExists.emplace_back(A->getValue());
+
   for (const Arg *A : Args.filtered(OPT_remap_file)) {
 std::pair Split = StringRef(A->getValue()).split(';');
 
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -69,6 +69,12 @@
   Builder.append(Twine("#include \"") + File + "\"");
 }
 
+static void AddImplicitIncludeIfExists(MacroBuilder , StringRef File) {
+  Builder.append(Twine("#if __has_include( \"") + File + "\")");
+  Builder.append(Twine("#include \"") + File + "\"");
+  Builder.append(Twine("#endif"));
+}
+
 static void AddImplicitIncludeMacros(MacroBuilder , StringRef File) {
   Builder.append(Twine("#__include_macros \"") + File + "\"");
   // Marker token to stop the __include_macros fetch loop.
@@ -1088,6 +1094,12 @@
   // Exit the command line and go back to  (2 is LC_LEAVE).
   if (!PP.getLangOpts().AsmPreprocessor)
 Builder.append("# 1 \"\" 2");
+  
+  // Process -finclude-if-exists directives.
+  for (unsigned i = 0, e = InitOpts.FIncludeIfExists.size(); i != e; ++i) {
+const std::string  = InitOpts.FIncludeIfExists[i];
+AddImplicitIncludeIfExists(Builder, Path);
+  }
 
   // If -imacros are specified, include them now.  These are processed before
   // any -include directives.
Index: unittests/Tooling/TestVisitor.h
===
--- unittests/Tooling/TestVisitor.h
+++ unittests/Tooling/TestVisitor.h
@@ -52,6 +52,7 @@
   /// \brief Runs the current AST visitor over the given code.
   bool runOver(StringRef Code, Language L = Lang_CXX) {
 std::vector Args;
+Args.push_back("-ffreestanding");
 switch (L) {
   case Lang_C: Args.push_back("-std=c99"); break;
   case Lang_CXX98: Args.push_back("-std=c++98"); break;
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -729,6 +729,8 @@
   HelpText<"Use specified token cache file">;
 def detailed_preprocessing_record : Flag<["-"], "detailed-preprocessing-record">,
   HelpText<"include a detailed record of preprocessing actions">;
+def finclude_if_exists : Separate<["-"], "finclude-if-exists">, MetaVarName<"">,
+  HelpText<"Include file before parsing if file exists">;
 
 //===--===//
 // OpenCL Options
Index: include/clang/Lex/PreprocessorOptions.h

[PATCH] D34624: extra test modifications for D34158

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 105906.
mibintc edited the summary of this revision.
mibintc added a comment.

This patch corresponds to these modifications to clang: 
https://reviews.llvm.org/D34158?id=105904


Repository:
  rL LLVM

https://reviews.llvm.org/D34624

Files:
  test/clang-tidy/llvm-include-order.cpp
  test/pp-trace/pp-trace-conditional.cpp
  test/pp-trace/pp-trace-ident.cpp
  test/pp-trace/pp-trace-include.cpp
  test/pp-trace/pp-trace-macro.cpp
  test/pp-trace/pp-trace-modules.cpp
  test/pp-trace/pp-trace-pragma-general.cpp
  test/pp-trace/pp-trace-pragma-ms.cpp
  test/pp-trace/pp-trace-pragma-opencl.cpp


Index: test/pp-trace/pp-trace-pragma-ms.cpp
===
--- test/pp-trace/pp-trace-pragma-ms.cpp
+++ test/pp-trace/pp-trace-pragma-ms.cpp
@@ -1,4 +1,4 @@
-// RUN: pp-trace -ignore FileChanged,MacroDefined %s -target x86_64 
-fms-extensions -w | FileCheck --strict-whitespace %s
+// RUN: pp-trace -ignore FileChanged,MacroDefined %s -target x86_64 
-fms-extensions -w -ffreestanding | FileCheck --strict-whitespace %s
 
 #pragma comment(compiler, "compiler comment")
 #pragma comment(exestr, "exestr comment")
Index: test/pp-trace/pp-trace-include.cpp
===
--- test/pp-trace/pp-trace-include.cpp
+++ test/pp-trace/pp-trace-include.cpp
@@ -1,4 +1,4 @@
-// RUN: pp-trace %s -undef -target x86_64 -std=c++11 | FileCheck 
--strict-whitespace %s
+// RUN: pp-trace %s -undef -ffreestanding -target x86_64 -std=c++11 | 
FileCheck --strict-whitespace %s
 
 #include "Inputs/Level1A.h"
 #include "Inputs/Level1B.h"
Index: test/pp-trace/pp-trace-ident.cpp
===
--- test/pp-trace/pp-trace-ident.cpp
+++ test/pp-trace/pp-trace-ident.cpp
@@ -1,4 +1,4 @@
-// RUN: pp-trace -ignore FileChanged,MacroDefined %s -undef -target x86_64 
-std=c++11 | FileCheck --strict-whitespace %s
+// RUN: pp-trace -ignore FileChanged,MacroDefined %s -undef -ffreestanding 
-target x86_64 -std=c++11 | FileCheck --strict-whitespace %s
 
 #ident "$Id$"
 
Index: test/pp-trace/pp-trace-conditional.cpp
===
--- test/pp-trace/pp-trace-conditional.cpp
+++ test/pp-trace/pp-trace-conditional.cpp
@@ -1,4 +1,4 @@
-// RUN: pp-trace -ignore FileChanged %s -undef -target x86_64 -std=c++11 | 
FileCheck --strict-whitespace %s
+// RUN: pp-trace -ignore FileChanged %s -ffreestanding -undef -target x86_64 
-std=c++11 | FileCheck --strict-whitespace %s
 
 #if 1
 #endif
Index: test/pp-trace/pp-trace-pragma-general.cpp
===
--- test/pp-trace/pp-trace-pragma-general.cpp
+++ test/pp-trace/pp-trace-pragma-general.cpp
@@ -1,4 +1,4 @@
-// RUN: pp-trace -ignore FileChanged,MacroDefined %s | FileCheck 
--strict-whitespace %s
+// RUN: pp-trace -ignore FileChanged,MacroDefined %s -ffreestanding | 
FileCheck --strict-whitespace %s
 
 #pragma clang diagnostic push
 #pragma clang diagnostic pop
Index: test/pp-trace/pp-trace-macro.cpp
===
--- test/pp-trace/pp-trace-macro.cpp
+++ test/pp-trace/pp-trace-macro.cpp
@@ -1,4 +1,4 @@
-// RUN: pp-trace -ignore FileChanged %s -undef -target x86_64 -std=c++11 | 
FileCheck --strict-whitespace %s
+// RUN: pp-trace -ignore FileChanged %s -undef -ffreestanding -target x86_64 
-std=c++11 | FileCheck --strict-whitespace %s
 
 #define MACRO 1
 int i = MACRO;
Index: test/pp-trace/pp-trace-modules.cpp
===
--- test/pp-trace/pp-trace-modules.cpp
+++ test/pp-trace/pp-trace-modules.cpp
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: pp-trace -ignore FileChanged,MacroDefined %s -x objective-c++ -undef 
-target x86_64 -std=c++11 -fmodules -fcxx-modules -fmodules-cache-path=%t -I%S 
-I%S/Input | FileCheck --strict-whitespace %s
+// RUN: pp-trace -ignore FileChanged,MacroDefined %s -x objective-c++ -undef 
-ffreestanding -target x86_64 -std=c++11 -fmodules -fcxx-modules 
-fmodules-cache-path=%t -I%S -I%S/Input | FileCheck --strict-whitespace %s
 
 // CHECK: ---
 
Index: test/pp-trace/pp-trace-pragma-opencl.cpp
===
--- test/pp-trace/pp-trace-pragma-opencl.cpp
+++ test/pp-trace/pp-trace-pragma-opencl.cpp
@@ -1,4 +1,4 @@
-// RUN: pp-trace -ignore FileChanged,MacroDefined %s -x cl | FileCheck 
--strict-whitespace %s
+// RUN: pp-trace -ignore FileChanged,MacroDefined %s -ffreestanding -x cl | 
FileCheck --strict-whitespace %s
 
 #pragma OPENCL EXTENSION all : disable
 #pragma OPENCL EXTENSION cl_khr_int64_base_atomics : disable
Index: test/clang-tidy/llvm-include-order.cpp
===
--- test/clang-tidy/llvm-include-order.cpp
+++ test/clang-tidy/llvm-include-order.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy 

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

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap added a comment.

ping


Repository:
  rL LLVM

https://reviews.llvm.org/D35041



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

[PATCH] D35216: [analyzer] Escape symbols when creating std::initializer_list.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Or maybe it wasn't a good idea to make two diffs in one place. Dunno.




Comment at: test/Analysis/temporaries-callback-order.cpp:28
 // CHECK: Bind
-// CHECK-NEXT: RegionChanges
 

Apparently, this was caused by the check if the states are equal within 
`processPointerEscapedOnBind`, so it was a dry run for checkers.


https://reviews.llvm.org/D35216



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

[PATCH] D35216: [analyzer] Escape symbols when creating std::initializer_list.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.

I've seen a few false positives that appear because we construct C++11 
`std::initializer_list` objects with brace initializers, and such construction 
is not properly modeled. For instance, if a new object is constructed on the 
heap only to be put into a brace-initialized STL container, the object is 
reported to be leaked.

Approach (0): This can be trivially fixed by this patch, which causes pointers 
passed into initializer list expressions to immediately escape.

This fix is overly conservative though. So i did a bit of investigation as to 
how model `std::initializer_list` better.

According to the standard, `std::initializer_list` is an object that has 
methods `begin()`, `end()`, and `size()`, where `begin()` returns a pointer to 
continous array of `size()` objects of type `T`, and `end()` is equal to 
`begin()` plus `size()`. The standard does hint that it should be possible to 
implement `std::initializer_list` as a pair of pointers, or as a pointer and 
a size integer, however specific fields that the object would contain are an 
implementation detail.

Ideally, we should be able to model the initializer list's methods precisely. 
Or, at least, it should be possible to explain to the analyzer that the list 
somehow "takes hold" of the values put into it. Initializer lists can also be 
copied, which is a separate story that i'm not trying to address here.

The obvious approach to modeling `std::initializer_list` in a //checker// would 
be to construct a `SymbolMetadata` for the memory region of the initializer 
list object, which would be of type `T*` and represent `begin()`, so we'd 
trivially model `begin()` as a function that returns this symbol. The array 
pointed to by that symbol would be `bindLoc()`ed to contain the list's contents 
(probably as a `CompoundVal` to produce less bindings in the store). Extent of 
this array would represent `size()` and would be equal to the length of the 
list as written.

So this sounds good, however apparently it does nothing to address our false 
positives: when the list escapes, our `RegionStoreManager` is not magically 
guessing that the metadata symbol attached to it, together with its contents, 
should also escape. In fact, it's impossible to trigger a pointer escape from 
within the checker.

Approach (1): If only we enabled `ProgramState::bindLoc(..., 
notifyChanges=true)` to cause pointer escapes (not only region changes) (which 
sounds like the right thing to do anyway) such checker would be able to solve 
the false positives by triggering escapes when binding list elements to the 
list. However, it'd be as conservative as the current patch's solution. 
Ideally, we do not want escapes to happen so early. Instead, we'd prefer them 
to be delayed until the list itself escapes.

So i believe that escaping metadata symbols whenever their base regions escape 
would be the right thing to do. Currently we didn't think about that because we 
had neither pointer-type metadatas nor non-pointer escapes.

Approach (2): We could teach the Store to scan itself for bindings to 
metadata-symbolic-based regions during `scanReachableSymbols()` whenever a 
region turns out to be reachable. This requires no work on checker side, but it 
sounds performance-heavy.

Approach (3): We could let checkers maintain the set of active metadata symbols 
in the program state (ideally somewhere in the Store, which sounds weird but 
causes the smallest amount of layering violations), so that the core knew what 
to escape. This puts a stress on the checkers, but with a smart data map it 
wouldn't be a problem.

Approach (4): We could allow checkers to trigger pointer escapes in arbitrary 
moments. If we allow doing this within `checkPointerEscape` callback itself, we 
would be able to express facts like "when this region escapes, that metadata 
symbol attached to it should also escape". This sounds like an ultimate 
freedom, with maximum stress on the checkers - still not too much stress when 
we have smart data maps.

Does anybody like/hate any of these solutions or have anything better in mind? 
I'm personally liking the solution with `scanReachableSymbols()` - it should be 
possible to avoid performance overhead, and clarity seems nice.


https://reviews.llvm.org/D35216

Files:
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/initializer.cpp

Index: test/Analysis/initializer.cpp
===
--- test/Analysis/initializer.cpp
+++ test/Analysis/initializer.cpp
@@ -1,7 +1,9 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-inlining=constructors -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,cplusplus.NewDeleteLeaks,debug.ExprInspection -analyzer-config c++-inlining=constructors -std=c++11 -verify %s
 
 void clang_analyzer_eval(bool);
 
+#include "Inputs/system-header-simulator-cxx.h"
+
 class A 

[PATCH] D34210: Add __has_feature(leak_sanitizer)

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

COFF supports weak externals: https://stackoverflow.com/a/11529277/382079. 
Would it suffice here?


https://reviews.llvm.org/D34210



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

[PATCH] D34624: extra test modifications for D34158

[Melanie Blower via Phabricator via cfe-commits]

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

I need to redo the test changes. Will resubmit the diff.


https://reviews.llvm.org/D34624



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

Re: r305850 - Preserve CXX method overrides in ASTImporter

[David Blaikie via cfe-commits]

ping on CR feedback

On Mon, Jun 26, 2017 at 7:02 PM David Blaikie  wrote:

> On Tue, Jun 20, 2017 at 2:06 PM Lang Hames via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: lhames
>> Date: Tue Jun 20 16:06:00 2017
>> New Revision: 305850
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=305850=rev
>> Log:
>> Preserve CXX method overrides in ASTImporter
>>
>> Summary:
>> The ASTImporter should import CXX method overrides from the source context
>> when it imports a method decl.
>>
>> Reviewers: spyffe, rsmith, doug.gregor
>>
>> Reviewed By: spyffe
>>
>> Differential Revision: https://reviews.llvm.org/D34371
>>
>> Modified:
>> cfe/trunk/lib/AST/ASTDumper.cpp
>> cfe/trunk/lib/AST/ASTImporter.cpp
>> cfe/trunk/tools/clang-import-test/clang-import-test.cpp
>>
>> Modified: cfe/trunk/lib/AST/ASTDumper.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=305850=305849=305850=diff
>>
>> ==
>> --- cfe/trunk/lib/AST/ASTDumper.cpp (original)
>> +++ cfe/trunk/lib/AST/ASTDumper.cpp Tue Jun 20 16:06:00 2017
>> @@ -1184,6 +1184,28 @@ void ASTDumper::VisitFunctionDecl(const
>>   I != E; ++I)
>>dumpCXXCtorInitializer(*I);
>>
>> +  if (const CXXMethodDecl *MD = dyn_cast(D))
>> +if (MD->size_overridden_methods() != 0) {
>>
>
> I'd probably make sure CXXMethodDecl has a range-based accessor for
> overridden_methods and call it once here, check begin != end, then use it
> in the range-based for later. That way it'd avoid multiple lookups of the
> CXXMethodDecl in the ASTContext.
>
> Probably not a big deal/efficiency concern, but just a thought :)
>
>
>> +  auto dumpOverride =
>> +[=](const CXXMethodDecl *D) {
>> +  SplitQualType T_split = D->getType().split();
>> +  OS << D << " " << D->getParent()->getName() << "::"
>> + << D->getName() << " '"
>> + << QualType::getAsString(T_split) << "'";
>> +};
>> +
>> +  dumpChild([=] {
>> +auto FirstOverrideItr = MD->begin_overridden_methods();
>> +OS << "Overrides: [ ";
>> +dumpOverride(*FirstOverrideItr);
>>
>
> Why is this one ^ pulled out separately from the rest of the loop?
>
>
>> +for (const auto *Override :
>> +   llvm::make_range(FirstOverrideItr + 1,
>> +MD->end_overridden_methods()))
>> +  dumpOverride(Override);
>> +OS << " ]";
>> +  });
>> +}
>> +
>>if (D->doesThisDeclarationHaveABody())
>>  dumpStmt(D->getBody());
>>  }
>>
>> Modified: cfe/trunk/lib/AST/ASTImporter.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=305850=305849=305850=diff
>>
>> ==
>> --- cfe/trunk/lib/AST/ASTImporter.cpp (original)
>> +++ cfe/trunk/lib/AST/ASTImporter.cpp Tue Jun 20 16:06:00 2017
>> @@ -319,6 +319,9 @@ namespace clang {
>>  bool ImportArrayChecked(const InContainerTy , OIter
>> Obegin) {
>>return ImportArrayChecked(InContainer.begin(), InContainer.end(),
>> Obegin);
>>  }
>> +
>> +// Importing overrides.
>> +void ImportOverrides(CXXMethodDecl *ToMethod, CXXMethodDecl
>> *FromMethod);
>>};
>>  }
>>
>> @@ -2025,6 +2028,9 @@ Decl *ASTNodeImporter::VisitFunctionDecl
>>// Add this function to the lexical context.
>>LexicalDC->addDeclInternal(ToFunction);
>>
>> +  if (auto *FromCXXMethod = dyn_cast(D))
>> +ImportOverrides(cast(ToFunction), FromCXXMethod);
>> +
>>return ToFunction;
>>  }
>>
>> @@ -5499,6 +5505,14 @@ Expr *ASTNodeImporter::VisitSubstNonType
>>  Replacement);
>>  }
>>
>> +void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod,
>> +  CXXMethodDecl *FromMethod) {
>> +  for (auto *FromOverriddenMethod : FromMethod->overridden_methods())
>> +ToMethod->addOverriddenMethod(
>> +  cast(Importer.Import(const_cast(
>> +FromOverriddenMethod;
>> +}
>> +
>>  ASTImporter::ASTImporter(ASTContext , FileManager
>> ,
>>   ASTContext , FileManager
>> ,
>>   bool MinimalImport)
>>
>> Modified: cfe/trunk/tools/clang-import-test/clang-import-test.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-import-test/clang-import-test.cpp?rev=305850=305849=305850=diff
>>
>> ==
>> --- cfe/trunk/tools/clang-import-test/clang-import-test.cpp (original)
>> +++ cfe/trunk/tools/clang-import-test/clang-import-test.cpp Tue Jun 20
>> 16:06:00 2017
>> @@ -17,7 +17,9 @@
>>  #include "clang/Basic/TargetInfo.h"
>>  #include "clang/Basic/TargetOptions.h"
>>  #include "clang/CodeGen/ModuleBuilder.h"
>> +#include "clang/Frontend/ASTConsumers.h"
>>  #include 

[PATCH] D34210: Add __has_feature(leak_sanitizer)

[Kostya Serebryany via Phabricator via cfe-commits]

kcc added a comment.

In https://reviews.llvm.org/D34210#785828, @rnk wrote:

> If LSan is a runtime thing, why not use weak hooks or something to detect 
> LSan at runtime instead of using a macro?


+1

For a run-time-only feature the checking should also be run-time-only (not 
compile-time)


https://reviews.llvm.org/D34210



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

Re: r307316 - Reject attempts to build a module without -fmodules, rather than silently doing weird things.

[David Blaikie via cfe-commits]

What's the reason that passing a module file can't imply -fmodules without
the user needing to specify it?

On Thu, Jul 6, 2017 at 2:06 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Thu Jul  6 14:05:56 2017
> New Revision: 307316
>
> URL: http://llvm.org/viewvc/llvm-project?rev=307316=rev
> Log:
> Reject attempts to build a module without -fmodules, rather than silently
> doing weird things.
>
> Added:
> cfe/trunk/test/Modules/missing-flag.cpp
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
> cfe/trunk/include/clang/Frontend/FrontendActions.h
> cfe/trunk/lib/Frontend/FrontendActions.cpp
> cfe/trunk/test/Modules/preprocess-build.cpp
> cfe/trunk/test/Modules/relative-dep-gen.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=307316=307315=307316=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Thu Jul  6
> 14:05:56 2017
> @@ -179,6 +179,8 @@ def warn_incompatible_analyzer_plugin_ap
>  def note_incompatible_analyzer_plugin_api : Note<
>  "current API version is '%0', but plugin was compiled with version
> '%1'">;
>
> +def err_module_build_requires_fmodules : Error<
> +  "module compilation requires '-fmodules'">;
>  def err_module_interface_requires_modules_ts : Error<
>"module interface compilation requires '-fmodules-ts'">;
>  def warn_module_config_mismatch : Warning<
>
> Modified: cfe/trunk/include/clang/Frontend/FrontendActions.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/FrontendActions.h?rev=307316=307315=307316=diff
>
> ==
> --- cfe/trunk/include/clang/Frontend/FrontendActions.h (original)
> +++ cfe/trunk/include/clang/Frontend/FrontendActions.h Thu Jul  6 14:05:56
> 2017
> @@ -111,6 +111,8 @@ protected:
>
>  class GenerateModuleFromModuleMapAction : public GenerateModuleAction {
>  private:
> +  bool BeginSourceFileAction(CompilerInstance ) override;
> +
>std::unique_ptr
>CreateOutputFile(CompilerInstance , StringRef InFile) override;
>  };
>
> Modified: cfe/trunk/lib/Frontend/FrontendActions.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendActions.cpp?rev=307316=307315=307316=diff
>
> ==
> --- cfe/trunk/lib/Frontend/FrontendActions.cpp (original)
> +++ cfe/trunk/lib/Frontend/FrontendActions.cpp Thu Jul  6 14:05:56 2017
> @@ -163,6 +163,16 @@ GenerateModuleAction::CreateASTConsumer(
>return llvm::make_unique(std::move(Consumers));
>  }
>
> +bool GenerateModuleFromModuleMapAction::BeginSourceFileAction(
> +CompilerInstance ) {
> +  if (!CI.getLangOpts().Modules) {
> +CI.getDiagnostics().Report(diag::err_module_build_requires_fmodules);
> +return false;
> +  }
> +
> +  return GenerateModuleAction::BeginSourceFileAction(CI);
> +}
> +
>  std::unique_ptr
>  GenerateModuleFromModuleMapAction::CreateOutputFile(CompilerInstance ,
>  StringRef InFile) {
>
> Added: cfe/trunk/test/Modules/missing-flag.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/missing-flag.cpp?rev=307316=auto
>
> ==
> --- cfe/trunk/test/Modules/missing-flag.cpp (added)
> +++ cfe/trunk/test/Modules/missing-flag.cpp Thu Jul  6 14:05:56 2017
> @@ -0,0 +1,4 @@
> +// RUN: not %clang_cc1 -x c++-module-map %s -emit-module
> -fmodule-name=Foo -o %t 2>&1 | FileCheck %s
> +// CHECK: module compilation requires '-fmodules'
> +module Foo {}
> +#pragma clang module contents
>
> Modified: cfe/trunk/test/Modules/preprocess-build.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/preprocess-build.cpp?rev=307316=307315=307316=diff
>
> ==
> --- cfe/trunk/test/Modules/preprocess-build.cpp (original)
> +++ cfe/trunk/test/Modules/preprocess-build.cpp Thu Jul  6 14:05:56 2017
> @@ -1,4 +1,4 @@
> -// RUN: %clang_cc1 -std=c++1z %s -verify
> +// RUN: %clang_cc1 -std=c++1z -fmodules %s -verify
>
>  #pragma clang module build baz
>module baz {}
>
> Modified: cfe/trunk/test/Modules/relative-dep-gen.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/relative-dep-gen.cpp?rev=307316=307315=307316=diff
>
> ==
> --- cfe/trunk/test/Modules/relative-dep-gen.cpp (original)
> +++ cfe/trunk/test/Modules/relative-dep-gen.cpp Thu Jul  6 14:05:56 2017
> @@ -2,17 +2,17 @@
>  // RUN: rm -rf %t
>  

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

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

gtbercea added inline comments.



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

hfinkel wrote:
> gtbercea wrote:
> > hfinkel wrote:
> > > Shouldn't you be adding all of the options, not just the -march= ones?
> > I thought that that would be the case but there are a few issues:
> > 
> > 1. PTXAS and NVLINK each use a different flag for specifying the arch, and, 
> > in turn, each flag is different from -march.
> > 
> > 2. -Xopenmp-target passes a flag to the entire toolchain not to individual 
> > components of the toolchain so a translation of flags is required in some 
> > cases to adapt the flag to the actual tool. -march is one example, I'm not 
> > sure if there are others.
> > 
> > 3. At this point in the code, in order to add a flag and its value to the 
> > DAL list, I need to be able to specify the option type (i.e. 
> > options::OPT_march_EQ). I therefore need to manually recognize the flag in 
> > the string representing the value of -Xopenmp-target or 
> > -Xopenmp-target=triple.
> > 
> > 4. This patch handles the passing of the arch and can be extended to pass 
> > other flags (as is stands, no other flags are passed through to the CUDA 
> > toolchain). This can be extended on a flag by flag basis for flags that 
> > need translating to a particular tool's flag. If the flag doesn't need 
> > translating then the flag and it's value can be appended to the command 
> > line as they are.
> > 1. PTXAS and NVLINK each use a different flag for specifying the arch, and, 
> > in turn, each flag is different from -march.
> 
> I don't understand why this is relevant. Don't NVPTX::Assembler::ConstructJob 
> and NVPTX::Linker::ConstructJob handle that in either case?
> 
> This seems to be the same comment to point 2 as well.
> 
> > 3. At this point in the code, in order to add a flag and its value to the 
> > DAL list, I need to be able to specify the option type (i.e. 
> > options::OPT_march_EQ). I therefore need to manually recognize the flag in 
> > the string representing the value of -Xopenmp-target or 
> > -Xopenmp-target=triple.
> 
> I don't understand why this is true. Doesn't the code just below this, which 
> handles -Xarch, do the general thing (it calls Opts.ParseOneArg and then adds 
> it to the list of derived arguments)? Can't we handle this like -Xarch?
> 
> > This patch handles the passing of the arch and can be extended to pass 
> > other flags (as is stands, no other flags are passed through to the CUDA 
> > toolchain). This can be extended on a flag by flag basis for flags that 
> > need translating to a particular tool's flag. If the flag doesn't need 
> > translating then the flag and it's value can be appended to the command 
> > line as they are.
> 
> I don't understand this either. If we need to extend this on a flag-by-flag 
> basis, then it seems fundamentally broken. How could we append a flag to the 
> command line without it also affecting the host compile?
@hfinkel 

The problem is that when using -Xopenmp-target= -opt=val the value of 
this flag is a list of two strings:

['', '-opt=val']

What needs to happen next is to parse the string containing "-opt=val". The 
reason I have to do this is because if I use -march, I can't pass -march as is 
to PTXAS and NVLINK which have different flags for expressing the arch. I need 
to translate the -march=sm_60 flag. I will have to do this for all flags which 
require translation. There is no way I can just append this string to the PTXAS 
and NVLINK commands because the flags for the 2 tools are different. A flag 
which works for one of them, will not work for the other. 

So I need to actually parse that value to check whether it is a "-march" and 
create an Arg object with the OPT_march_EQ identifier and the sm_60 value. When 
invoking the commands for PTXAS and NVLINK, the dervied arg list will be 
travered and every -march=sm_60 option will be transformed into "--gpu-name" 
"sm_60" for PTXAS and into "-arch" "sm_60" for NVLINK. 

In the case of -Xarch, you will see that after we have traversed the entire arg 
list we still have to special case -march and add it is manually added to the 
DAL.

Let me know your thoughts on this.

Thanks,

--Doru


https://reviews.llvm.org/D34784



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

[PATCH] D35212: [Index] Prevent canonical decl becoming nullptr

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.

This patch prevents getCanonicalDecl returning nullptr in case it finds
a canonical TemplateDeclaration with no attached TemplatedDecl.
Found by running the indexer over a version of the standard library deep inside
a template metaprogramming mess.


https://reviews.llvm.org/D35212

Files:
  lib/Index/IndexingContext.cpp


Index: lib/Index/IndexingContext.cpp
===
--- lib/Index/IndexingContext.cpp
+++ lib/Index/IndexingContext.cpp
@@ -254,8 +254,10 @@
 static const Decl *getCanonicalDecl(const Decl *D) {
   D = D->getCanonicalDecl();
   if (auto TD = dyn_cast(D)) {
-D = TD->getTemplatedDecl();
-assert(D->isCanonicalDecl());
+if (auto TTD = TD->getTemplatedDecl()) {
+  D = TTD;
+  assert(D->isCanonicalDecl());
+}
   }
 
   return D;


Index: lib/Index/IndexingContext.cpp
===
--- lib/Index/IndexingContext.cpp
+++ lib/Index/IndexingContext.cpp
@@ -254,8 +254,10 @@
 static const Decl *getCanonicalDecl(const Decl *D) {
   D = D->getCanonicalDecl();
   if (auto TD = dyn_cast(D)) {
-D = TD->getTemplatedDecl();
-assert(D->isCanonicalDecl());
+if (auto TTD = TD->getTemplatedDecl()) {
+  D = TTD;
+  assert(D->isCanonicalDecl());
+}
   }
 
   return D;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D16579: Warn if friend function depends on template parameters.

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 105853.
sepavloff added a comment.

Updated patch

- Remove changes that are not needed anymore,
- Function matching is implemented on recursive function instead of 
TypeVisitor. It support subset of checks, which should be enough for practical 
use.
- Added more comments.


https://reviews.llvm.org/D16579

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/Sema.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  test/CXX/drs/dr3xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/CXX/temp/temp.res/p3.cpp
  test/SemaCXX/friend.cpp
  test/SemaCXX/overload-call.cpp

Index: test/SemaCXX/overload-call.cpp
===
--- test/SemaCXX/overload-call.cpp
+++ test/SemaCXX/overload-call.cpp
@@ -574,13 +574,17 @@
   // Ensure that overload resolution attempts to complete argument types when
   // performing ADL.
   template struct S {
-friend int f(const S&);
+friend int f(const S&);  // expected-warning{{friend declaration 'IncompleteArg::f' depends on template parameter but is not a function template}}
+ // expected-note@-1{{declare function outside class template to suppress this warning}}
+ // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
   };
   extern S s;
   int k = f(s);
 
   template struct Op {
-friend bool operator==(const Op &, const Op &);
+friend bool operator==(const Op &, const Op &);  // expected-warning{{friend declaration 'IncompleteArg::operator==' depends on template parameter but is not a function template}}
+ // expected-note@-1{{declare function outside class template to suppress this warning}}
+ // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
   };
   extern Op op;
   bool b = op == op;
Index: test/SemaCXX/friend.cpp
===
--- test/SemaCXX/friend.cpp
+++ test/SemaCXX/friend.cpp
@@ -388,3 +388,116 @@
 friend void f(void *p = 0) {} // expected-error {{must be the only}}
   };
 }
+
+template void pr23342_func(T x);
+template
+struct pr23342_C1 {
+  friend void pr23342_func<>(T x);
+  friend bool func(T x);  // expected-warning{{friend declaration 'func' depends on template parameter but is not a function template}}
+  // expected-note@-1{{declare function outside class template to suppress this warning}}
+  // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+  friend bool func2(int x);
+  template friend bool func3(T2 x);
+  friend T func4();  // expected-warning{{friend declaration 'func4' depends on template parameter but is not a function template}}
+ // expected-note@-1{{declare function outside class template to suppress this warning}}
+ // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+};
+
+namespace pr23342 {
+
+template
+struct C1 {
+  friend void pr23342_func<>(T x);
+  friend bool func(T x);  // expected-warning{{friend declaration 'pr23342::func' depends on template parameter but is not a function template}}
+  // expected-note@-1{{declare function outside class template to suppress this warning}}
+  // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+  friend bool func2(int x);
+  template friend bool func3(T2 x);
+  friend T func4();// expected-warning{{friend declaration 'pr23342::func4' depends on template parameter but is not a function template}}
+   // expected-note@-1{{declare function outside class template to suppress this warning}}
+   // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+};
+
+template 
+struct Arg {
+  friend bool operator==(const Arg& lhs, T rhs) {
+   return false;
+  }
+  friend bool operator!=(const Arg& lhs, T rhs);  // expected-warning{{friend declaration 'pr23342::operator!=' depends on template parameter but is not a function template}}
+   // expected-note@-1{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+};
+template 
+bool operator!=(const Arg& lhs, T rhs) {
+  return true;
+}
+bool foo() {
+  Arg arg;
+  return (arg == 42) || (arg != 42);
+}
+
+
+template class C0 {
+  friend void func0(C0 &);  // expected-warning{{friend declaration 

[PATCH] D34158: For standards compatibility, preinclude if the file is available

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

This version is still disabling upon -nostdinc, which doesn't make sense to me.

You didn't remove that, nor respond explaining why you think it does make sense?


https://reviews.llvm.org/D34158



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

Re: r305860 - Special-case handling of destructors in override lists when dumping ASTs.

[David Blaikie via cfe-commits]

Ping for a response from Lang on Richard's CR feedback

On Tue, Jun 20, 2017 at 3:30 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On 20 June 2017 at 14:30, Lang Hames via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: lhames
>> Date: Tue Jun 20 16:30:43 2017
>> New Revision: 305860
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=305860=rev
>> Log:
>> Special-case handling of destructors in override lists when dumping ASTs.
>>
>> Fixes a bug in r305850: CXXDestructors don't have names, so we need to
>> handle
>> printing of them separately.
>>
>
> Clang models destructors as having names, and should print them properly.
> What was actually going wrong here? There are other kinds of name that can
> be virtual and aren't simple identifiers...
>
>
>> Modified:
>> cfe/trunk/lib/AST/ASTDumper.cpp
>>
>> Modified: cfe/trunk/lib/AST/ASTDumper.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=305860=305859=305860=diff
>>
>> ==
>> --- cfe/trunk/lib/AST/ASTDumper.cpp (original)
>> +++ cfe/trunk/lib/AST/ASTDumper.cpp Tue Jun 20 16:30:43 2017
>> @@ -1189,9 +1189,12 @@ void ASTDumper::VisitFunctionDecl(const
>>auto dumpOverride =
>>  [=](const CXXMethodDecl *D) {
>>SplitQualType T_split = D->getType().split();
>> -  OS << D << " " << D->getParent()->getName() << "::"
>> - << D->getName() << " '"
>> - << QualType::getAsString(T_split) << "'";
>> +  OS << D << " " << D->getParent()->getName() << "::";
>> +  if (isa(D))
>> +OS << "~" << D->getParent()->getName();
>> +  else
>> +OS << D->getName();
>> +  OS << " '" << QualType::getAsString(T_split) << "'";
>>  };
>>
>>dumpChild([=] {
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35203: Avoid white spaces in file names. NFC

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL307551: Avoid white spaces in file names. NFC (authored by 
ioeric).

Changed prior to commit:
  https://reviews.llvm.org/D35203?vs=105860=105871#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35203

Files:
  cfe/trunk/test/Driver/crash report spaces.c
  cfe/trunk/test/Driver/crash-report-spaces.c


Index: cfe/trunk/test/Driver/crash-report-spaces.c
===
--- cfe/trunk/test/Driver/crash-report-spaces.c
+++ cfe/trunk/test/Driver/crash-report-spaces.c
@@ -0,0 +1,19 @@
+// RUN: rm -rf "%t"
+// RUN: mkdir "%t"
+// RUN: cp "%s" "%t/crash report spaces.c"
+// RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTIONS=1 %clang 
-fsyntax-only "%t/crash report spaces.c" 2>&1 | FileCheck "%s"
+// RUN: cat "%t/crash report spaces"-*.c | FileCheck --check-prefix=CHECKSRC 
"%s"
+// RUN: cat "%t/crash report spaces"-*.sh | FileCheck --check-prefix=CHECKSH 
"%s"
+// REQUIRES: crash-recovery
+
+// because of the glob (*.c, *.sh)
+// REQUIRES: shell
+
+#pragma clang __debug parser_crash
+// CHECK: Preprocessed source(s) and associated run script(s) are located at:
+// CHECK-NEXT: note: diagnostic msg: {{.*}}.c
+FOO
+// CHECKSRC: FOO
+// CHECKSH: "-cc1"
+// CHECKSH: "-main-file-name" "crash report spaces.c"
+// CHECKSH: "crash report spaces-{{[^ ]*}}.c"
Index: cfe/trunk/test/Driver/crash report spaces.c
===
--- cfe/trunk/test/Driver/crash report spaces.c
+++ cfe/trunk/test/Driver/crash report spaces.c
@@ -1,18 +0,0 @@
-// RUN: rm -rf "%t"
-// RUN: mkdir "%t"
-// RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTIONS=1 %clang 
-fsyntax-only "%s" 2>&1 | FileCheck "%s"
-// RUN: cat "%t/crash report spaces"-*.c | FileCheck --check-prefix=CHECKSRC 
"%s"
-// RUN: cat "%t/crash report spaces"-*.sh | FileCheck --check-prefix=CHECKSH 
"%s"
-// REQUIRES: crash-recovery
-
-// because of the glob (*.c, *.sh)
-// REQUIRES: shell
-
-#pragma clang __debug parser_crash
-// CHECK: Preprocessed source(s) and associated run script(s) are located at:
-// CHECK-NEXT: note: diagnostic msg: {{.*}}.c
-FOO
-// CHECKSRC: FOO
-// CHECKSH: "-cc1"
-// CHECKSH: "-main-file-name" "crash report spaces.c"
-// CHECKSH: "crash report spaces-{{[^ ]*}}.c"


Index: cfe/trunk/test/Driver/crash-report-spaces.c
===
--- cfe/trunk/test/Driver/crash-report-spaces.c
+++ cfe/trunk/test/Driver/crash-report-spaces.c
@@ -0,0 +1,19 @@
+// RUN: rm -rf "%t"
+// RUN: mkdir "%t"
+// RUN: cp "%s" "%t/crash report spaces.c"
+// RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTIONS=1 %clang -fsyntax-only "%t/crash report spaces.c" 2>&1 | FileCheck "%s"
+// RUN: cat "%t/crash report spaces"-*.c | FileCheck --check-prefix=CHECKSRC "%s"
+// RUN: cat "%t/crash report spaces"-*.sh | FileCheck --check-prefix=CHECKSH "%s"
+// REQUIRES: crash-recovery
+
+// because of the glob (*.c, *.sh)
+// REQUIRES: shell
+
+#pragma clang __debug parser_crash
+// CHECK: Preprocessed source(s) and associated run script(s) are located at:
+// CHECK-NEXT: note: diagnostic msg: {{.*}}.c
+FOO
+// CHECKSRC: FOO
+// CHECKSH: "-cc1"
+// CHECKSH: "-main-file-name" "crash report spaces.c"
+// CHECKSH: "crash report spaces-{{[^ ]*}}.c"
Index: cfe/trunk/test/Driver/crash report spaces.c
===
--- cfe/trunk/test/Driver/crash report spaces.c
+++ cfe/trunk/test/Driver/crash report spaces.c
@@ -1,18 +0,0 @@
-// RUN: rm -rf "%t"
-// RUN: mkdir "%t"
-// RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTIONS=1 %clang -fsyntax-only "%s" 2>&1 | FileCheck "%s"
-// RUN: cat "%t/crash report spaces"-*.c | FileCheck --check-prefix=CHECKSRC "%s"
-// RUN: cat "%t/crash report spaces"-*.sh | FileCheck --check-prefix=CHECKSH "%s"
-// REQUIRES: crash-recovery
-
-// because of the glob (*.c, *.sh)
-// REQUIRES: shell
-
-#pragma clang __debug parser_crash
-// CHECK: Preprocessed source(s) and associated run script(s) are located at:
-// CHECK-NEXT: note: diagnostic msg: {{.*}}.c
-FOO
-// CHECKSRC: FOO
-// CHECKSH: "-cc1"
-// CHECKSH: "-main-file-name" "crash report spaces.c"
-// CHECKSH: "crash report spaces-{{[^ ]*}}.c"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33644: Add default values for function parameter chunks

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:2411-2412
+  if (DefValue.at(0) != '=') {
+// If we don't have = in front of value
+return " = " + DefValue;
+  }

Can you expand in the comment on when each of these happens? Intuitively I'd 
have expected that only one of these cases can happen (I assume this is because 
of the forward-declared type?)


https://reviews.llvm.org/D33644



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

[PATCH] D34158: For standards compatibility, preinclude if the file is available

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D34158#803752, @jyknight wrote:

> This version is still disabling upon -nostdinc, which doesn't make sense to 
> me.


I agree. If I pass -nostdinc, so that I then arrange include paths for my own 
libc headers, I'd then like to pick up the predef header from that library (if 
available). There's no clearly right answer for whether  `__STDC_NO_THREADS__` 
is defined, for example, regardless of whether we're using -nostdinc.

> You didn't remove that, nor respond explaining why you think it does make 
> sense?




https://reviews.llvm.org/D34158



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

[PATCH] D34891: Replace assert(0) with llvm_unreachable.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

If possible, please upload patches with full context - makes it easier to check 
around nearby code when reviewing. (the 'arc' tool for phab helps with this)




Comment at: include/clang/AST/Redeclarable.h:257-261
 if (PassedFirst) {
-  assert(0 && "Passed first decl twice, invalid redecl chain!");
+  llvm_unreachable("Passed first decl twice, invalid redecl chain!");
   Current = nullptr;
   return *this;
 }

Branch-to-unreachable is generally to be avoided.

Looks like this code should actually change to:

  if (Current->isFirstDecl()) {
assert(PassedFirst);

Similar comments on many of the other changes here


Repository:
  rL LLVM

https://reviews.llvm.org/D34891



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

[PATCH] D34158: For standards compatibility, preinclude if the file is available

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In https://reviews.llvm.org/D34158#803752, @jyknight wrote:

> This version is still disabling upon -nostdinc, which doesn't make sense to 
> me.
>
> You didn't remove that, nor respond explaining why you think it does make 
> sense?


You're right, I should remove the check on nostdinc.  Sorry I missed that 
remark from you initially, that's why I didn't reply.  I will upload another 
patch which removes the check on nostdinc.


https://reviews.llvm.org/D34158



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

[PATCH] D34512: Add preliminary Cross Translation Unit support library

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

Specifically, ping Richard for new top-level lib in clang.


https://reviews.llvm.org/D34512



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

[PATCH] D35203: Avoid white spaces in file names. NFC

[Benjamin Kramer via Phabricator via cfe-commits]

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

This is fine. We should fix phab though.


https://reviews.llvm.org/D35203



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

Re: r306809 - Ambiguity might be also uninitialized. Use llvm::Optional.

[David Blaikie via cfe-commits]

Which compiler/what warning was flagging this?

It doesn't look like Clang builds by default with
-Wconditional-uninitialized on, so I'm surprised if Clang was diagnosing
anything here.

In any case - it's probably better to omit the "WasAmbiguous" flag.
Optional already has a flag that says whether it has a value, so either you
can rely on that or assert against it explicitly:

  assert(SavedAK);
  Ambiguity = *SavedAK;

But I'd probably skip adding the assertion here - especially since it
currently doesn't have a message, so it's probably no better than the
assert that Optional's operator* will do anyway.

On Fri, Jun 30, 2017 at 2:25 AM Vassil Vassilev via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: vvassilev
> Date: Fri Jun 30 02:25:43 2017
> New Revision: 306809
>
> URL: http://llvm.org/viewvc/llvm-project?rev=306809=rev
> Log:
> Ambiguity might be also uninitialized. Use llvm::Optional.
>
>
> Modified:
> cfe/trunk/include/clang/Sema/Lookup.h
>
> Modified: cfe/trunk/include/clang/Sema/Lookup.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Lookup.h?rev=306809=306808=306809=diff
>
> ==
> --- cfe/trunk/include/clang/Sema/Lookup.h (original)
> +++ cfe/trunk/include/clang/Sema/Lookup.h Fri Jun 30 02:25:43 2017
> @@ -18,6 +18,8 @@
>  #include "clang/AST/DeclCXX.h"
>  #include "clang/Sema/Sema.h"
>
> +#include "llvm/ADT/Optional.h"
> +
>  namespace clang {
>
>  /// @brief Represents the results of name lookup.
> @@ -465,9 +467,10 @@ public:
>  Paths = nullptr;
>}
>  } else {
> -  AmbiguityKind SavedAK = Ambiguity;
> +  llvm::Optional SavedAK;
>bool WasAmbiguous = false;
>if (ResultKind == Ambiguous) {
> +SavedAK = Ambiguity;
>  WasAmbiguous = true;
>}
>ResultKind = Found;
> @@ -478,7 +481,7 @@ public:
>if (ResultKind == Ambiguous) {
>  (void)WasAmbiguous;
>  assert(WasAmbiguous);
> -Ambiguity = SavedAK;
> +Ambiguity = SavedAK.getValue();
>} else if (Paths) {
>  deletePaths(Paths);
>  Paths = nullptr;
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D34896: Enable the new PM + SamlePGO + ThinLTO testing.

[David Blaikie via cfe-commits]

Does this test any code in Clang? (given the test is being committed
without any change in Clang, I'm guessing maybe not) - perhaps this test
doesn't belong in Clang's test suite?

Looks like changes/functionality in LTOPreLinkDefaultPipeline are tested in
test/Other/new-pm-thinlto-defaults.ll at least?

On Fri, Jun 30, 2017 at 10:05 AM Dehao Chen via Phabricator via cfe-commits
 wrote:

> danielcdh created this revision.
> Herald added subscribers: eraman, inglorion, mehdi_amini, sanjoy.
>
> This patch should be enabled after https://reviews.llvm.org/D34895
>
>
> https://reviews.llvm.org/D34896
>
> Files:
>   test/CodeGen/pgo-sample-thinlto-summary.c
>
>
> Index: test/CodeGen/pgo-sample-thinlto-summary.c
> ===
> --- test/CodeGen/pgo-sample-thinlto-summary.c
> +++ test/CodeGen/pgo-sample-thinlto-summary.c
> @@ -1,9 +1,7 @@
>  // RUN: %clang_cc1 -O2
> -fprofile-sample-use=%S/Inputs/pgo-sample-thinlto-summary.prof %s
> -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=SAMPLEPGO
>  // RUN: %clang_cc1 -O2
> -fprofile-sample-use=%S/Inputs/pgo-sample-thinlto-summary.prof %s
> -emit-llvm -flto=thin -o - 2>&1 | FileCheck %s -check-prefix=THINLTO
>  // RUN: %clang_cc1 -O2 -fexperimental-new-pass-manager
> -fprofile-sample-use=%S/Inputs/pgo-sample-thinlto-summary.prof %s
> -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=SAMPLEPGO
> -// FIXME: Run the following command once LTOPreLinkDefaultPipeline is
> -//customized.
> -// %clang_cc1 -O2 -fexperimental-new-pass-manager
> -fprofile-sample-use=%S/Inputs/pgo-sample-thinlto-summary.prof %s
> -emit-llvm -flto=thin -o - 2>&1 | FileCheck %s -check-prefix=THINLTO
> +// RUN: %clang_cc1 -O2 -fexperimental-new-pass-manager
> -fprofile-sample-use=%S/Inputs/pgo-sample-thinlto-summary.prof %s
> -emit-llvm -flto=thin -o - 2>&1 | FileCheck %s -check-prefix=THINLTO
>  // Checks if hot call is inlined by normal compile, but not inlined by
>  // thinlto compile.
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Yan Wang via Phabricator via cfe-commits]

yawanng updated this revision to Diff 105878.
yawanng marked 5 inline comments as done.

https://reviews.llvm.org/D34913

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

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

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

[Yan Wang via Phabricator via cfe-commits]

yawanng added inline comments.



Comment at: clang-tidy/utils/ASTUtils.h:27
+///  or  |  | ...
+bool hasFlag(const Expr *Flags, const SourceManager ,
+ const LangOptions , const char *CloseOnExecFlag);

alexfh wrote:
> `hasFlag` is too generic and may give a totally wrong impression of what the 
> function does. Even in the context of this utility library I would prefer to 
> expand the name and make it less ambiguous. Something along the lines of 
> `exprHasBitFlagWithSpelling`. I wouldn't also insist on the flag being 
> implemented as a macro, since this restriction would prevent many valid use 
> cases with enum or constexpr int flags.
Yes, the name is a little misleading and only consider the macro is not 
thorough. But for the purpose of the current Android checks, where this 
function is used, this simple solution can cover about 95% cases. Few use 
`enum` or `constexpr` for flags like `O_CLOEXEC` or other similar ones.  
Moreover, if extending the function to cover the constant integer variables, 
it's not easy to get the value of the checked flag, which may depend on the 
target. Generally, in current stage, this function works well in the Android 
cases and it's still a TODO for other more sophisticated user cases. 


https://reviews.llvm.org/D34913



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

[PATCH] D35203: Avoid white spaces in file names. NFC

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.

White spaces in file names are causing Phabricator/SVN to crash.


https://reviews.llvm.org/D35203

Files:
  test/Driver/crash report spaces.c
  test/Driver/crash-report-spaces.c


Index: test/Driver/crash-report-spaces.c
===
--- test/Driver/crash-report-spaces.c
+++ test/Driver/crash-report-spaces.c
@@ -1,6 +1,7 @@
 // RUN: rm -rf "%t"
 // RUN: mkdir "%t"
-// RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTIONS=1 %clang 
-fsyntax-only "%s" 2>&1 | FileCheck "%s"
+// RUN: cp "%s" "%t/crash report spaces.c"
+// RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTIONS=1 %clang 
-fsyntax-only "%t/crash report spaces.c" 2>&1 | FileCheck "%s"
 // RUN: cat "%t/crash report spaces"-*.c | FileCheck --check-prefix=CHECKSRC 
"%s"
 // RUN: cat "%t/crash report spaces"-*.sh | FileCheck --check-prefix=CHECKSH 
"%s"
 // REQUIRES: crash-recovery


Index: test/Driver/crash-report-spaces.c
===
--- test/Driver/crash-report-spaces.c
+++ test/Driver/crash-report-spaces.c
@@ -1,6 +1,7 @@
 // RUN: rm -rf "%t"
 // RUN: mkdir "%t"
-// RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTIONS=1 %clang -fsyntax-only "%s" 2>&1 | FileCheck "%s"
+// RUN: cp "%s" "%t/crash report spaces.c"
+// RUN: not env TMPDIR="%t" TEMP="%t" TMP="%t" RC_DEBUG_OPTIONS=1 %clang -fsyntax-only "%t/crash report spaces.c" 2>&1 | FileCheck "%s"
 // RUN: cat "%t/crash report spaces"-*.c | FileCheck --check-prefix=CHECKSRC "%s"
 // RUN: cat "%t/crash report spaces"-*.sh | FileCheck --check-prefix=CHECKSH "%s"
 // REQUIRES: crash-recovery
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

[don hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 105961.
hintonda added a comment.

- Make option cc1 only.  Rename function, and add test.


https://reviews.llvm.org/D35175

Files:
  include/clang/Basic/DiagnosticIDs.h
  include/clang/Basic/DiagnosticOptions.def
  include/clang/Driver/CC1Options.td
  lib/Basic/DiagnosticIDs.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/TextDiagnosticPrinter.cpp
  test/Frontend/diagnostics-option-diag-ids.cpp

Index: test/Frontend/diagnostics-option-diag-ids.cpp
===
--- /dev/null
+++ test/Frontend/diagnostics-option-diag-ids.cpp
@@ -0,0 +1,15 @@
+// RUN: not %clang_cc1 -fdiagnostics-show-option -fdiagnostics-show-diag-ids %s 2> %t
+// RUN: FileCheck < %t %s
+
+class xx {
+  xx(){}
+};
+
+void foo(sss) {
+// CHECK: [[@LINE-1]]:10: error: unknown type name 'sss' [err_unknown_typename:{{[0-9]+}}]
+  int;
+// CHECK: [[@LINE-1]]:3: warning: declaration does not declare anything [-Wmissing-declarations,ext_no_declarators:{{[0-9]+}}]
+  xx x;
+// CHECK: [[@LINE-1]]:6: error: calling a private constructor of class 'xx' [err_access_ctor:{{[0-9]+}}]
+// CHECK: [[@LINE-9]]:3: note: implicitly declared private here [note_access_natural:{{[0-9]+}}]
+}
Index: lib/Frontend/TextDiagnosticPrinter.cpp
===
--- lib/Frontend/TextDiagnosticPrinter.cpp
+++ lib/Frontend/TextDiagnosticPrinter.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/Signals.h"
 #include 
 using namespace clang;
 
@@ -103,6 +104,12 @@
   }
 }
   }
+  // If the user wants to see diagnostic ids, include it too.
+  if (DiagOpts.ShowDiagIDs) {
+OS << (Started ? "," : " [");
+Started = true;
+OS << DiagnosticIDs::getNameFromID(Info.getID()) << ":" << Info.getID();
+  }
   if (Started)
 OS << ']';
 }
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1079,6 +1079,8 @@
   << ShowCategory;
   }
 
+  Opts.ShowDiagIDs = Args.hasArg(OPT_fdiagnostics_show_diag_ids);
+
   StringRef Format =
 Args.getLastArgValue(OPT_fdiagnostics_format, "clang");
   if (Format == "clang")
Index: lib/Basic/DiagnosticIDs.cpp
===
--- lib/Basic/DiagnosticIDs.cpp
+++ lib/Basic/DiagnosticIDs.cpp
@@ -37,6 +37,7 @@
 };
 
 struct StaticDiagInfoRec {
+  const char *IDName;
   uint16_t DiagID;
   unsigned DefaultSeverity : 3;
   unsigned Class : 3;
@@ -74,8 +75,9 @@
 #define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \
  SHOWINSYSHEADER, CATEGORY)\
   {\
-diag::ENUM, DEFAULT_SEVERITY, CLASS, DiagnosticIDs::SFINAE, NOWERROR,  \
-SHOWINSYSHEADER, CATEGORY, GROUP, STR_SIZE(DESC, uint16_t), DESC   \
+#ENUM, diag::ENUM, DEFAULT_SEVERITY, CLASS, DiagnosticIDs::SFINAE, \
+NOWERROR, SHOWINSYSHEADER, CATEGORY, GROUP, STR_SIZE(DESC, uint16_t),  \
+DESC   \
   }\
   ,
 #include "clang/Basic/DiagnosticCommonKinds.inc"
@@ -228,7 +230,11 @@
   return CategoryNameTable[CategoryID].getName();
 }
 
-
+StringRef DiagnosticIDs::getNameFromID(unsigned DiagID) {
+  if (const StaticDiagInfoRec *Info = GetDiagInfo(DiagID))
+return StringRef(Info->IDName);
+  return StringRef();
+}
 
 DiagnosticIDs::SFINAEResponse
 DiagnosticIDs::getDiagnosticSFINAEResponse(unsigned DiagID) {
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -380,6 +380,8 @@
   HelpText<"Ignore unexpected diagnostic messages">;
 def Wno_rewrite_macros : Flag<["-"], "Wno-rewrite-macros">,
   HelpText<"Silence ObjC rewriting warnings">;
+def fdiagnostics_show_diag_ids : Flag<["-"], "fdiagnostics-show-diag-ids">,
+  HelpText<"Show Diagnostic enum name and index">;
 
 //===--===//
 // Frontend Options
Index: include/clang/Basic/DiagnosticOptions.def
===
--- include/clang/Basic/DiagnosticOptions.def
+++ include/clang/Basic/DiagnosticOptions.def
@@ -61,6 +61,7 @@
 DIAGOPT(ShowNoteIncludeStack, 1, 0) /// Show include stacks for notes.
 VALUE_DIAGOPT(ShowCategories, 2, 0) /// Show categories: 0 -> none, 1 -> Number,
 /// 2 -> Full Name.
+DIAGOPT(ShowDiagIDs, 1, 0) /// Show Diagnostic ID
  

[PATCH] D35186: [analyzer] Add annotation for functions taking user-facing strings

[Devin Coughlin via Phabricator via cfe-commits]

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

Thanks for the patch! This looks very good to me.

The one thing I would suggest is renaming 'isAnnotatedAsLocalized()' and 
'isAnnotatedTakingLocalized()' to make it more clear what each does, now that 
there are two of them. How about: 'isAnnotatedAsReturningLocalized()' and 
'isAnnotatedAsTakingLocalized()'?


https://reviews.llvm.org/D35186



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

[libcxx] r307615 - [libcxx][CMake] Add install path variable to allow overriding the destination

[Petr Hosek via cfe-commits]

Author: phosek
Date: Mon Jul 10 19:39:50 2017
New Revision: 307615

URL: http://llvm.org/viewvc/llvm-project?rev=307615=rev
Log:
[libcxx][CMake] Add install path variable to allow overriding the destination

This is going to be used by the runtime build in the multi-target
setup to allow using different install prefix for each target.

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

Modified:
libcxx/trunk/CMakeLists.txt
libcxx/trunk/cmake/Modules/HandleLibCXXABI.cmake
libcxx/trunk/docs/BuildingLibcxx.rst
libcxx/trunk/include/CMakeLists.txt
libcxx/trunk/lib/CMakeLists.txt

Modified: libcxx/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/CMakeLists.txt?rev=307615=307614=307615=diff
==
--- libcxx/trunk/CMakeLists.txt (original)
+++ libcxx/trunk/CMakeLists.txt Mon Jul 10 19:39:50 2017
@@ -352,6 +352,13 @@ else()
 endif()
 file(MAKE_DIRECTORY "${LIBCXX_BINARY_INCLUDE_DIR}")
 
+set(LIBCXX_INSTALL_PREFIX "" CACHE STRING
+"Define libc++ destination prefix.")
+
+if (NOT LIBCXX_INSTALL_PREFIX MATCHES "^$|.*/")
+  message(FATAL_ERROR "LIBCXX_INSTALL_PREFIX has to end with \"/\".")
+endif()
+
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIBCXX_LIBRARY_DIR})
 set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LIBCXX_LIBRARY_DIR})
 set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LIBCXX_LIBRARY_DIR})

Modified: libcxx/trunk/cmake/Modules/HandleLibCXXABI.cmake
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/cmake/Modules/HandleLibCXXABI.cmake?rev=307615=307614=307615=diff
==
--- libcxx/trunk/cmake/Modules/HandleLibCXXABI.cmake (original)
+++ libcxx/trunk/cmake/Modules/HandleLibCXXABI.cmake Mon Jul 10 19:39:50 2017
@@ -55,7 +55,7 @@ macro(setup_abi_lib abidefines abilib ab
   )
 if (LIBCXX_INSTALL_HEADERS)
   install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
-DESTINATION include/c++/v1/${dstdir}
+DESTINATION ${LIBCXX_INSTALL_PREFIX}include/c++/v1/${dstdir}
 COMPONENT libcxx
 PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
 )

Modified: libcxx/trunk/docs/BuildingLibcxx.rst
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/BuildingLibcxx.rst?rev=307615=307614=307615=diff
==
--- libcxx/trunk/docs/BuildingLibcxx.rst (original)
+++ libcxx/trunk/docs/BuildingLibcxx.rst Mon Jul 10 19:39:50 2017
@@ -216,6 +216,11 @@ libc++ specific options
   Extra suffix to append to the directory where libraries are to be installed.
   This option overrides `LLVM_LIBDIR_SUFFIX`.
 
+.. option:: LIBCXX_INSTALL_PREFIX:STRING
+
+  **Default**: ``""``
+
+  Define libc++ destination prefix.
 
 .. _libc++experimental options:
 

Modified: libcxx/trunk/include/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/CMakeLists.txt?rev=307615=307614=307615=diff
==
--- libcxx/trunk/include/CMakeLists.txt (original)
+++ libcxx/trunk/include/CMakeLists.txt Mon Jul 10 19:39:50 2017
@@ -20,7 +20,7 @@ endif()
 
 if (LIBCXX_INSTALL_HEADERS)
   install(DIRECTORY .
-DESTINATION include/c++/v1
+DESTINATION ${LIBCXX_INSTALL_PREFIX}include/c++/v1
 COMPONENT cxx-headers
 FILES_MATCHING
 ${LIBCXX_HEADER_PATTERN}
@@ -44,7 +44,7 @@ if (LIBCXX_INSTALL_HEADERS)
 set(generated_config_deps generate_config_header)
 # Install the generated header as __config.
 install(FILES ${LIBCXX_BINARY_DIR}/__generated_config
-  DESTINATION include/c++/v1
+  DESTINATION ${LIBCXX_INSTALL_PREFIX}include/c++/v1
   PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
   RENAME __config
   COMPONENT cxx-headers)

Modified: libcxx/trunk/lib/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/lib/CMakeLists.txt?rev=307615=307614=307615=diff
==
--- libcxx/trunk/lib/CMakeLists.txt (original)
+++ libcxx/trunk/lib/CMakeLists.txt Mon Jul 10 19:39:50 2017
@@ -355,8 +355,8 @@ if (LIBCXX_INSTALL_LIBRARY)
 set(experimental_lib cxx_experimental)
   endif()
   install(TARGETS ${LIBCXX_TARGETS} ${experimental_lib}
-LIBRARY DESTINATION lib${LIBCXX_LIBDIR_SUFFIX} COMPONENT cxx
-ARCHIVE DESTINATION lib${LIBCXX_LIBDIR_SUFFIX} COMPONENT cxx
+LIBRARY DESTINATION ${LIBCXX_INSTALL_PREFIX}lib${LIBCXX_LIBDIR_SUFFIX} 
COMPONENT cxx
+ARCHIVE DESTINATION ${LIBCXX_INSTALL_PREFIX}lib${LIBCXX_LIBDIR_SUFFIX} 
COMPONENT cxx
 )
   # NOTE: This install command must go after the cxx install command otherwise
   # it will not be executed after the library symlinks are installed.
@@ -364,7 +364,7 @@ if (LIBCXX_INSTALL_LIBRARY)
 # Replace the libc++ filename with $
 # after we required CMake 

r307603 - [clang-import-test] Test that enumerators and their values are found.

[Sean Callanan via cfe-commits]

Author: spyffe
Date: Mon Jul 10 17:29:37 2017
New Revision: 307603

URL: http://llvm.org/viewvc/llvm-project?rev=307603=rev
Log:
[clang-import-test] Test that enumerators and their values are found.

Added:
cfe/trunk/test/Import/enum/
  - copied from r307452, cfe/trunk/test/Import/member-in-struct/
cfe/trunk/test/Import/enum/Inputs/S.cpp
  - copied, changed from r307452, 
cfe/trunk/test/Import/member-in-struct/Inputs/S.c
cfe/trunk/test/Import/enum/test.cpp
  - copied, changed from r307452, 
cfe/trunk/test/Import/member-in-struct/test.c
Removed:
cfe/trunk/test/Import/enum/Inputs/S.c
cfe/trunk/test/Import/enum/test.c

Removed: cfe/trunk/test/Import/enum/Inputs/S.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/member-in-struct/Inputs/S.c?rev=307452=auto
==
--- cfe/trunk/test/Import/enum/Inputs/S.c (original)
+++ cfe/trunk/test/Import/enum/Inputs/S.c (removed)
@@ -1,3 +0,0 @@
-struct S {
-  int a;
-};

Copied: cfe/trunk/test/Import/enum/Inputs/S.cpp (from r307452, 
cfe/trunk/test/Import/member-in-struct/Inputs/S.c)
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/enum/Inputs/S.cpp?p2=cfe/trunk/test/Import/enum/Inputs/S.cpp=cfe/trunk/test/Import/member-in-struct/Inputs/S.c=307452=307603=307603=diff
==
--- cfe/trunk/test/Import/member-in-struct/Inputs/S.c (original)
+++ cfe/trunk/test/Import/enum/Inputs/S.cpp Mon Jul 10 17:29:37 2017
@@ -1,3 +1,4 @@
-struct S {
-  int a;
+enum E {
+  a = 1,
+  b = 2
 };

Removed: cfe/trunk/test/Import/enum/test.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/member-in-struct/test.c?rev=307452=auto
==
--- cfe/trunk/test/Import/enum/test.c (original)
+++ cfe/trunk/test/Import/enum/test.c (removed)
@@ -1,5 +0,0 @@
-// RUN: clang-import-test -import %S/Inputs/S.c -expression %s
-void expr() {
-  struct S MyS;
-  MyS.a = 3;
-}

Copied: cfe/trunk/test/Import/enum/test.cpp (from r307452, 
cfe/trunk/test/Import/member-in-struct/test.c)
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/enum/test.cpp?p2=cfe/trunk/test/Import/enum/test.cpp=cfe/trunk/test/Import/member-in-struct/test.c=307452=307603=307603=diff
==
--- cfe/trunk/test/Import/member-in-struct/test.c (original)
+++ cfe/trunk/test/Import/enum/test.cpp Mon Jul 10 17:29:37 2017
@@ -1,5 +1,4 @@
-// RUN: clang-import-test -import %S/Inputs/S.c -expression %s
+// RUN: clang-import-test -import %S/Inputs/S.cpp -expression %s
 void expr() {
-  struct S MyS;
-  MyS.a = 3;
+  static_assert(E::a + E::b == 3);
 }


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

[PATCH] D35230: [clang] buildFixItInsertionLine should use Hints of the same FID and LineNo

[Chih-Hung Hsieh via Phabricator via cfe-commits]

chh added a comment.

See https://reviews.llvm.org/D35225 for a test case.


https://reviews.llvm.org/D35230



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

r307604 - [analyzer] Start fixing modeling of bool based types

[Alexander Shaposhnikov via cfe-commits]

Author: alexshap
Date: Mon Jul 10 17:30:14 2017
New Revision: 307604

URL: http://llvm.org/viewvc/llvm-project?rev=307604=rev
Log:
[analyzer] Start fixing modeling of bool based types

This is a follow up for one of 
the previous diffs https://reviews.llvm.org/D32328.
getTypeSize and with getIntWidth are not equivalent for bool 
(see https://clang.llvm.org/doxygen/ASTContext_8cpp_source.html#l08444),
this causes a number of issues
(for instance, if APint X representing a bool is created 
with the wrong bit width then X is not comparable against Min/Max
(because of the different bit width), that results in crashes 
(triggered asserts) inside assume* methods), 
for examples see the newly added test cases.

Test plan: make check-all

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

Modified:

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
cfe/trunk/test/Analysis/enum.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h?rev=307604=307603=307604=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h 
(original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h 
Mon Jul 10 17:30:14 2017
@@ -124,7 +124,7 @@ public:
   /// Returns the type of the APSInt used to store values of the given 
QualType.
   APSIntType getAPSIntType(QualType T) const {
 assert(T->isIntegralOrEnumerationType() || Loc::isLocType(T));
-return APSIntType(Ctx.getTypeSize(T),
+return APSIntType(Ctx.getIntWidth(T),
   !T->isSignedIntegerOrEnumerationType());
   }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp?rev=307604=307603=307604=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Mon Jul 10 17:30:14 
2017
@@ -71,18 +71,15 @@ SVal SimpleSValBuilder::dispatchCast(SVa
 }
 
 SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) {
-
   bool isLocType = Loc::isLocType(castTy);
-
   if (val.getAs())
 return val;
 
   if (Optional LI = val.getAs()) {
 if (isLocType)
   return LI->getLoc();
-
 // FIXME: Correctly support promotions/truncations.
-unsigned castSize = Context.getTypeSize(castTy);
+unsigned castSize = Context.getIntWidth(castTy);
 if (castSize == LI->getNumBits())
   return val;
 return makeLocAsInteger(LI->getLoc(), castSize);
@@ -173,7 +170,7 @@ SVal SimpleSValBuilder::evalCastFromLoc(
   }
 
   if (castTy->isIntegralOrEnumerationType()) {
-unsigned BitWidth = Context.getTypeSize(castTy);
+unsigned BitWidth = Context.getIntWidth(castTy);
 
 if (!val.getAs())
   return makeLocAsInteger(val, BitWidth);

Modified: cfe/trunk/test/Analysis/enum.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/enum.cpp?rev=307604=307603=307604=diff
==
--- cfe/trunk/test/Analysis/enum.cpp (original)
+++ cfe/trunk/test/Analysis/enum.cpp Mon Jul 10 17:30:14 2017
@@ -37,3 +37,33 @@ bool testNoCrashOnSwitchEnumBool(EnumBoo
   }
   return true;
 }
+
+bool testNoCrashOnSwitchEnumBoolConstant() {
+  EnumBool E = EnumBool::F;
+  switch (E) {
+case EnumBool::F:
+  return false; 
+  }
+  return true; 
+}
+
+typedef __INTPTR_TYPE__ intptr_t;
+bool testNoCrashOnSwitchEnumBoolConstantCastedFromNullptr() {
+  EnumBool E = static_cast((intptr_t)nullptr);
+  switch (E) {
+  case EnumBool::F:
+return false;
+  }
+  return true;
+}
+
+bool testNoCrashOnSwitchEnumBoolConstantCastedFromPtr() {
+  int X;
+  intptr_t P = (intptr_t)
+  EnumBool E = static_cast(P);
+  switch (E) {
+  case EnumBool::F:
+return false;
+  }
+  return true;
+}


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

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

[Alexander Shaposhnikov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL307604: [analyzer] Start fixing modeling of bool based types 
(authored by alexshap).

Changed prior to commit:
  https://reviews.llvm.org/D35041?vs=105597=105947#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35041

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


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


Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -124,7 +124,7 @@
   /// Returns the type of the APSInt used to store values of the given QualType.
   APSIntType getAPSIntType(QualType T) const {
 assert(T->isIntegralOrEnumerationType() || Loc::isLocType(T));
-return APSIntType(Ctx.getTypeSize(T),
+return APSIntType(Ctx.getIntWidth(T),
   !T->isSignedIntegerOrEnumerationType());
   }
 
Index: cfe/trunk/test/Analysis/enum.cpp
===
--- cfe/trunk/test/Analysis/enum.cpp
+++ cfe/trunk/test/Analysis/enum.cpp
@@ -37,3 +37,33 @@
   }
   return true;
 }
+
+bool testNoCrashOnSwitchEnumBoolConstant() {
+  EnumBool E = EnumBool::F;
+  switch (E) {
+case EnumBool::F:
+  return false; 
+  }
+  return true; 
+}
+
+typedef __INTPTR_TYPE__ intptr_t;
+bool testNoCrashOnSwitchEnumBoolConstantCastedFromNullptr() {
+  EnumBool E = static_cast((intptr_t)nullptr);
+  switch (E) {
+  case EnumBool::F:
+return false;
+  }
+  return true;
+}
+
+bool testNoCrashOnSwitchEnumBoolConstantCastedFromPtr() {
+  int X;
+  intptr_t P = (intptr_t)
+  EnumBool E = static_cast(P);
+  switch (E) {
+  case EnumBool::F:
+return false;
+  }
+  return true;
+}
Index: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -71,18 +71,15 @@
 }
 
 SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) {
-
   bool isLocType = Loc::isLocType(castTy);
-
   

[libunwind] r307606 - [libunwind][CMake] Add install path variable to allow overriding the destination

[Petr Hosek via cfe-commits]

Author: phosek
Date: Mon Jul 10 18:12:09 2017
New Revision: 307606

URL: http://llvm.org/viewvc/llvm-project?rev=307606=rev
Log:
[libunwind][CMake] Add install path variable to allow overriding the destination

This is going to be used by the runtime build in the multi-target
setup to allow using different install prefix for each target.

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

Modified:
libunwind/trunk/CMakeLists.txt
libunwind/trunk/src/CMakeLists.txt

Modified: libunwind/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/CMakeLists.txt?rev=307606=307605=307606=diff
==
--- libunwind/trunk/CMakeLists.txt (original)
+++ libunwind/trunk/CMakeLists.txt Mon Jul 10 18:12:09 2017
@@ -164,6 +164,13 @@ set(LIBUNWIND_SOURCE_DIR  ${CMAKE_CURREN
 set(LIBUNWIND_BINARY_DIR  ${CMAKE_CURRENT_BINARY_DIR})
 set(LIBUNWIND_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
 
+set(LIBUNWIND_INSTALL_PREFIX "" CACHE STRING
+"Define libunwind destination prefix.")
+
+if (NOT LIBUNWIND_INSTALL_PREFIX MATCHES "^$|.*/")
+  message(FATAL_ERROR "LIBUNWIND_INSTALL_PREFIX has to end with \"/\".")
+endif()
+
 set(LIBUNWIND_C_FLAGS "")
 set(LIBUNWIND_CXX_FLAGS "")
 set(LIBUNWIND_COMPILE_FLAGS "")

Modified: libunwind/trunk/src/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/CMakeLists.txt?rev=307606=307605=307606=diff
==
--- libunwind/trunk/src/CMakeLists.txt (original)
+++ libunwind/trunk/src/CMakeLists.txt Mon Jul 10 18:12:09 2017
@@ -133,6 +133,6 @@ endif()
 add_custom_target(unwind DEPENDS ${LIBUNWIND_TARGETS})
 
 install(TARGETS ${LIBUNWIND_TARGETS}
-LIBRARY DESTINATION lib${LLVM_LIBDIR_SUFFIX}
-ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX})
+  LIBRARY DESTINATION ${LIBUNWIND_INSTALL_PREFIX}lib${LLVM_LIBDIR_SUFFIX}
+  ARCHIVE DESTINATION ${LIBUNWIND_INSTALL_PREFIX}lib${LLVM_LIBDIR_SUFFIX})
 


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

[libcxxabi] r307611 - [libcxxabi][CMake] Add install path variable to allow overriding the destination

[Petr Hosek via cfe-commits]

Author: phosek
Date: Mon Jul 10 18:42:26 2017
New Revision: 307611

URL: http://llvm.org/viewvc/llvm-project?rev=307611=rev
Log:
[libcxxabi][CMake] Add install path variable to allow overriding the destination

This is going to be used by the runtime build in the multi-target
setup to allow using different install prefix for each target.

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

Modified:
libcxxabi/trunk/CMakeLists.txt
libcxxabi/trunk/src/CMakeLists.txt

Modified: libcxxabi/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/CMakeLists.txt?rev=307611=307610=307611=diff
==
--- libcxxabi/trunk/CMakeLists.txt (original)
+++ libcxxabi/trunk/CMakeLists.txt Mon Jul 10 18:42:26 2017
@@ -156,6 +156,13 @@ else()
   set(LIBCXXABI_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBCXXABI_LIBDIR_SUFFIX})
 endif()
 
+set(LIBCXXABI_INSTALL_PREFIX "" CACHE STRING
+"Define libc++abi destination prefix.")
+
+if (NOT LIBCXXABI_INSTALL_PREFIX MATCHES "^$|.*/")
+  message(FATAL_ERROR "LIBCXXABI_INSTALL_PREFIX has to end with \"/\".")
+endif()
+
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIBCXXABI_LIBRARY_DIR})
 set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LIBCXXABI_LIBRARY_DIR})
 set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LIBCXXABI_LIBRARY_DIR})

Modified: libcxxabi/trunk/src/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/CMakeLists.txt?rev=307611=307610=307611=diff
==
--- libcxxabi/trunk/src/CMakeLists.txt (original)
+++ libcxxabi/trunk/src/CMakeLists.txt Mon Jul 10 18:42:26 2017
@@ -174,8 +174,8 @@ endif()
 add_custom_target(cxxabi DEPENDS ${LIBCXXABI_TARGETS})
 
 install(TARGETS ${LIBCXXABI_TARGETS}
-  LIBRARY DESTINATION lib${LIBCXXABI_LIBDIR_SUFFIX} COMPONENT cxxabi
-  ARCHIVE DESTINATION lib${LIBCXXABI_LIBDIR_SUFFIX} COMPONENT cxxabi
+  LIBRARY DESTINATION ${LIBCXXABI_INSTALL_PREFIX}lib${LIBCXXABI_LIBDIR_SUFFIX} 
COMPONENT cxxabi
+  ARCHIVE DESTINATION ${LIBCXXABI_INSTALL_PREFIX}lib${LIBCXXABI_LIBDIR_SUFFIX} 
COMPONENT cxxabi
   )
 
 if (NOT CMAKE_CONFIGURATION_TYPES)


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

[PATCH] D34534: [libcxx] [test] Fix MSVC warning C4242 "conversion from 'int' to 'const char', possible loss of data".

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

STL_MSFT added a comment.

As I've made this exact change in other tests before, I'll commit this soon-ish 
if there are no comments or objections.


https://reviews.llvm.org/D34534



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

r307594 - IsSpecialLong was only ever set in release mode as all of the uses are in asserts. Wrap in ifndef NDEBUG.

[Eric Christopher via cfe-commits]

Author: echristo
Date: Mon Jul 10 14:28:54 2017
New Revision: 307594

URL: http://llvm.org/viewvc/llvm-project?rev=307594=rev
Log:
IsSpecialLong was only ever set in release mode as all of the uses are in 
asserts. Wrap in ifndef NDEBUG.

Modified:
cfe/trunk/lib/AST/ASTContext.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=307594=307593=307594=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Mon Jul 10 14:28:54 2017
@@ -8523,7 +8523,10 @@ static QualType DecodeTypeFromStr(const
   RequiresICE = false;
   
   // Read the prefixed modifiers first.
-  bool Done = false, IsSpecialLong = false;
+  bool Done = false;
+  #ifndef NDEBUG
+  bool IsSpecialLong = false;
+  #endif
   while (!Done) {
 switch (*Str++) {
 default: Done = true; --Str; break;
@@ -8549,7 +8552,9 @@ static QualType DecodeTypeFromStr(const
   // 'N' behaves like 'L' for all non LP64 targets and 'int' otherwise.
   assert(!IsSpecialLong && "Can't use two 'N' or 'W' modifiers!");
   assert(HowLong == 0 && "Can't use both 'L' and 'N' modifiers!");
+  #ifndef NDEBUG
   IsSpecialLong = true;
+  #endif
   if (Context.getTargetInfo().getLongWidth() == 32)
 ++HowLong;
   break;
@@ -8558,7 +8563,9 @@ static QualType DecodeTypeFromStr(const
   // This modifier represents int64 type.
   assert(!IsSpecialLong && "Can't use two 'N' or 'W' modifiers!");
   assert(HowLong == 0 && "Can't use both 'L' and 'W' modifiers!");
+  #ifndef NDEBUG
   IsSpecialLong = true;
+  #endif
   switch (Context.getTargetInfo().getInt64Type()) {
   default:
 llvm_unreachable("Unexpected integer type");


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

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

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

I tested the following software, both before and after applying this patch, 
using RangeConstraintManager.

Software, Version, Compile Time (before), Bugs Reported (before), Compile Time 
(after), Bugs Reported (after)
openssl, 1.1.0f, 11 min, 126, 12 min, 126
sqlite, 3.18.2, 31 min, 86, 31 min, 82
wireshark, 2.2.7, 105 min, 109, 119 min, 108

For sqlite, the differences of four reported bugs all appear to be false 
positives, but for wireshark, the one difference appears to be a false negative.




Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:363
   // instead of generating an Unknown value and propagate the taint info to it.
-  const unsigned MaxComp = 1; // 10 28X
 

NoQ wrote:
> ddcc wrote:
> > zaks.anna wrote:
> > > Reducing the MaxComp is going to regress taint analysis..
> > > 
> > > > I've updated this revision to account for the recent SVal 
> > > > simplification commit by @NoQ, 
> > > 
> > > Which commit introduced the regression?
> > > 
> > > > but now there is an exponential blowup problem that prevents testcase 
> > > > PR24184.cpp from terminating, 
> > > 
> > > What triggers the regression? Removing the if statement above? Does the 
> > > regression only effect the Z3 "mode" (I am guessing not since you said 
> > > "due to an interaction between Simplifier::VisitNonLocSymbolVal() and 
> > > SValBuilder::makeSymExprValNN()")? 
> > > 
> > > Reducing the MaxComp is going to regress taint analysis..
> > 
> > I think the original intention was to increase `MaxComp`, not decrease it, 
> > but I will restore the original value here.
> > 
> > > What triggers the regression? Removing the if statement above? Does the 
> > > regression only effect the Z3 "mode"
> > 
> > No, the regression isn't specifically due to this code, but with @NoQ 's 
> > patch for `SVal` simplification (rL300178), and this commit extending it to 
> > handle `SymbolCast` and `IntSymExpr`, the cast of `ST *` used in the loop 
> > of case 3 of PR24184.cpp becomes "simplified" (inlined) repeatedly on each 
> > recursive iteration through `Simplifier::VisitNonLocSymbolVal()` -> 
> > `SValBuilder::makeSymExprValNN()`, causing a big slowdown in runtime 
> > performance.
> > 
> > The naive way to prevent it is to drop `MaxComp` (but this isn't 
> > reasonable, since it needs to be absurdly low, e.g. `10`). Alternatively, 
> > simplification for `SymbolCast` can be dropped from this commit (but it 
> > will eliminate some of the other analysis improvements), or, most likely, 
> > introduce another parameter to reduce recursion between these two functions.
> We talked a bit and we're pretty much fine with reduce `MaxComp` to 10 if 
> it's worth it in terms of performance.
Just to clarify, the current change from 100 to 10 in 
`simplifySVal::SimplyVisitNonLocSymbolVal` resolves the problem; the value of 
`MaxComp` here is unchanged.



Comment at: test/Analysis/plist-macros.cpp:2
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix 
-analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix 
-analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config 
path-diagnostics-alternate=ture %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s

NoQ wrote:
> > `path-diagnostics-alternate=ture`
> 
> Ouch. Thanks for fixing this.
> 
> Maybe it'd be great to implement an option that enables validating analyzer 
> config option values, and turn this option on in `%clang_analyze_cc1`.
I agree that'd be useful, but I think that it should be a separate future patch.


https://reviews.llvm.org/D28953



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

[libcxx] r307595 - [libc++] Refactoring __sync_* builtins; NFC (Reland)

[Weiming Zhao via cfe-commits]

Author: weimingz
Date: Mon Jul 10 14:37:35 2017
New Revision: 307595

URL: http://llvm.org/viewvc/llvm-project?rev=307595=rev
Log:
[libc++] Refactoring __sync_* builtins; NFC (Reland)

Summary: Wrap __sync_* builtins with __libcpp_ functions to facility future 
customizations as atomic operations are unavailable on some targets.

Reviewers: danalbert, EricWF, jroelofs

Subscribers: joerg, llvm-commits

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

Added:
libcxx/trunk/include/__atomic_support
Modified:
libcxx/trunk/include/__refstring
libcxx/trunk/src/locale.cpp
libcxx/trunk/src/support/runtime/exception_fallback.ipp
libcxx/trunk/src/support/runtime/new_handler_fallback.ipp

Added: libcxx/trunk/include/__atomic_support
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__atomic_support?rev=307595=auto
==
--- libcxx/trunk/include/__atomic_support (added)
+++ libcxx/trunk/include/__atomic_support Mon Jul 10 14:37:35 2017
@@ -0,0 +1,31 @@
+// -*- C++ -*-
+//=== __atomic_support 
-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP___ATOMIC_SUPPORT
+#define _LIBCPP___ATOMIC_SUPPORT
+
+#include <__config>
+
+template
+T __libcpp_sync_add_and_fetch(T *ptr, T value) {
+  return __sync_add_and_fetch(ptr, value);
+}
+
+template
+T __libcpp_sync_fetch_and_add(T *ptr, T value) {
+  return __sync_fetch_and_add(ptr, value);
+}
+
+template
+T __libcpp_sync_lock_test_and_set(T *ptr, T value) {
+  return __sync_lock_test_and_set(ptr, value);
+}
+
+#endif //_LIBCPP___ATOMIC_SUPPORT

Modified: libcxx/trunk/include/__refstring
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__refstring?rev=307595=307594=307595=diff
==
--- libcxx/trunk/include/__refstring (original)
+++ libcxx/trunk/include/__refstring Mon Jul 10 14:37:35 2017
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include <__atomic_support>
 #ifdef __APPLE__
 #include 
 #include 
@@ -83,7 +84,7 @@ __libcpp_refstring::__libcpp_refstring(c
 : __imp_(s.__imp_)
 {
 if (__uses_refcount())
-__sync_add_and_fetch(_from_data(__imp_)->count, 1);
+__libcpp_sync_add_and_fetch(_from_data(__imp_)->count, 1);
 }
 
 inline
@@ -92,10 +93,10 @@ __libcpp_refstring& __libcpp_refstring::
 struct _Rep_base *old_rep = rep_from_data(__imp_);
 __imp_ = s.__imp_;
 if (__uses_refcount())
-__sync_add_and_fetch(_from_data(__imp_)->count, 1);
+__libcpp_sync_add_and_fetch(_from_data(__imp_)->count, 1);
 if (adjust_old_count)
 {
-if (__sync_add_and_fetch(_rep->count, count_t(-1)) < 0)
+if (__libcpp_sync_add_and_fetch(_rep->count, count_t(-1)) < 0)
 {
 ::operator delete(old_rep);
 }
@@ -107,7 +108,7 @@ inline
 __libcpp_refstring::~__libcpp_refstring() {
 if (__uses_refcount()) {
 _Rep_base* rep = rep_from_data(__imp_);
-if (__sync_add_and_fetch(>count, count_t(-1)) < 0) {
+if (__libcpp_sync_add_and_fetch(>count, count_t(-1)) < 0) {
 ::operator delete(rep);
 }
 }

Modified: libcxx/trunk/src/locale.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/locale.cpp?rev=307595=307594=307595=diff
==
--- libcxx/trunk/src/locale.cpp (original)
+++ libcxx/trunk/src/locale.cpp Mon Jul 10 14:37:35 2017
@@ -28,6 +28,7 @@
 #define _CTYPE_DISABLE_MACROS
 #endif
 #include "cwctype"
+#include "__atomic_support"
 #include "__sso_allocator"
 #if defined(_LIBCPP_MSVCRT) || defined(__MINGW32__)
 #include "support/win32/locale_win32.h"
@@ -667,7 +668,7 @@ locale::id::__get()
 void
 locale::id::__init()
 {
-__id_ = __sync_add_and_fetch(&__next_id, 1);
+__id_ = __libcpp_sync_add_and_fetch(&__next_id, 1);
 }
 
 // template <> class collate_byname

Modified: libcxx/trunk/src/support/runtime/exception_fallback.ipp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/support/runtime/exception_fallback.ipp?rev=307595=307594=307595=diff
==
--- libcxx/trunk/src/support/runtime/exception_fallback.ipp (original)
+++ libcxx/trunk/src/support/runtime/exception_fallback.ipp Mon Jul 10 14:37:35 
2017
@@ -9,6 +9,7 @@
 
//===--===//
 
 #include 
+#include <__atomic_support>
 
 namespace std {
 
@@ -20,13 +21,13 @@ _LIBCPP_SAFE_STATIC static std::unexpect
 unexpected_handler
 set_unexpected(unexpected_handler func) _NOEXCEPT
 {
-  

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

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

I've also uploaded the results to https://dcddcc.com/csa


https://reviews.llvm.org/D28953



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

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

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 105913.
ddcc marked an inline comment as done.
ddcc added a comment.

Drop duplicate code


https://reviews.llvm.org/D28953

Files:
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/bitwise-ops.c
  test/Analysis/bool-assignment.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,8 +67,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1 == 0)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -87,8 +87,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -117,8 +117,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
 
 
@@ -85,6 +85,7 @@
 void test2(int *p) {
   CALL_FN(p);
 }
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -636,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col25
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT:
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth0
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:

r307584 - Use DenseMap instead std::map for GVSummaryMapTy

[Dehao Chen via cfe-commits]

Author: dehao
Date: Mon Jul 10 13:31:37 2017
New Revision: 307584

URL: http://llvm.org/viewvc/llvm-project?rev=307584=rev
Log:
Use DenseMap instead std::map for GVSummaryMapTy

Summary: Frontend change for https://reviews.llvm.org/D35148

Reviewers: tejohnson

Reviewed By: tejohnson

Subscribers: sanjoy, cfe-commits

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

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

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=307584=307583=307584=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Mon Jul 10 13:31:37 2017
@@ -998,7 +998,7 @@ static void runThinLTOBackend(ModuleSumm
   std::unique_ptr OS,
   std::string SampleProfile,
   BackendAction Action) {
-  StringMap>
+  StringMap>
   ModuleToDefinedGVSummaries;
   
CombinedIndex->collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
 


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

[libcxx] r307591 - [libc++] Refactoring __sync_* builtins; NFC

[Weiming Zhao via cfe-commits]

Author: weimingz
Date: Mon Jul 10 14:02:54 2017
New Revision: 307591

URL: http://llvm.org/viewvc/llvm-project?rev=307591=rev
Log:
[libc++] Refactoring __sync_* builtins; NFC

Summary: Wrap __sync_* builtins with __libcpp_ functions to facility future 
customizations as atomic operations are unavailable on some targets.

Reviewers: danalbert, EricWF, jroelofs

Subscribers: joerg, llvm-commits

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

Added:
libcxx/trunk/include/__atomic_support
Modified:
libcxx/trunk/include/__refstring
libcxx/trunk/src/locale.cpp
libcxx/trunk/src/support/runtime/exception_fallback.ipp
libcxx/trunk/src/support/runtime/new_handler_fallback.ipp

Added: libcxx/trunk/include/__atomic_support
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__atomic_support?rev=307591=auto
==
--- libcxx/trunk/include/__atomic_support (added)
+++ libcxx/trunk/include/__atomic_support Mon Jul 10 14:02:54 2017
@@ -0,0 +1,31 @@
+// -*- C++ -*-
+//=== __atomic_support 
-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP___ATOMIC_SUPPORT
+#define _LIBCPP___ATOMIC_SUPPORT
+
+#include <__config>
+
+template
+T __libcpp_sync_add_and_fetch(T *ptr, T value) {
+  return __sync_add_and_fetch(ptr, value);
+}
+
+template
+T __libcpp_sync_fetch_and_add(T *ptr, T value) {
+  return __sync_fetch_and_add(ptr, value);
+}
+
+template
+T __libcpp_sync_lock_test_and_set(T *ptr, T value) {
+  return __sync_lock_test_and_set(ptr, value);
+}
+
+#endif //_LIBCPP___ATOMIC_SUPPORT

Modified: libcxx/trunk/include/__refstring
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__refstring?rev=307591=307590=307591=diff
==
--- libcxx/trunk/include/__refstring (original)
+++ libcxx/trunk/include/__refstring Mon Jul 10 14:02:54 2017
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include <__atomic_support>
 #ifdef __APPLE__
 #include 
 #include 
@@ -83,7 +84,7 @@ __libcpp_refstring::__libcpp_refstring(c
 : __imp_(s.__imp_)
 {
 if (__uses_refcount())
-__sync_add_and_fetch(_from_data(__imp_)->count, 1);
+__libcpp_sync_add_and_fetch(_from_data(__imp_)->count, 1);
 }
 
 inline
@@ -92,10 +93,10 @@ __libcpp_refstring& __libcpp_refstring::
 struct _Rep_base *old_rep = rep_from_data(__imp_);
 __imp_ = s.__imp_;
 if (__uses_refcount())
-__sync_add_and_fetch(_from_data(__imp_)->count, 1);
+__libcpp_sync_add_and_fetch(_from_data(__imp_)->count, 1);
 if (adjust_old_count)
 {
-if (__sync_add_and_fetch(_rep->count, count_t(-1)) < 0)
+if (__libcpp_sync_add_and_fetch(_rep->count, count_t(-1)) < 0)
 {
 ::operator delete(old_rep);
 }
@@ -107,7 +108,7 @@ inline
 __libcpp_refstring::~__libcpp_refstring() {
 if (__uses_refcount()) {
 _Rep_base* rep = rep_from_data(__imp_);
-if (__sync_add_and_fetch(>count, count_t(-1)) < 0) {
+if (__libcpp_sync_add_and_fetch(>count, count_t(-1)) < 0) {
 ::operator delete(rep);
 }
 }

Modified: libcxx/trunk/src/locale.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/locale.cpp?rev=307591=307590=307591=diff
==
--- libcxx/trunk/src/locale.cpp (original)
+++ libcxx/trunk/src/locale.cpp Mon Jul 10 14:02:54 2017
@@ -667,7 +667,7 @@ locale::id::__get()
 void
 locale::id::__init()
 {
-__id_ = __sync_add_and_fetch(&__next_id, 1);
+__id_ = __libcpp_sync_add_and_fetch(&__next_id, 1);
 }
 
 // template <> class collate_byname

Modified: libcxx/trunk/src/support/runtime/exception_fallback.ipp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/support/runtime/exception_fallback.ipp?rev=307591=307590=307591=diff
==
--- libcxx/trunk/src/support/runtime/exception_fallback.ipp (original)
+++ libcxx/trunk/src/support/runtime/exception_fallback.ipp Mon Jul 10 14:02:54 
2017
@@ -9,6 +9,7 @@
 
//===--===//
 
 #include 
+#include <__atomic_support>
 
 namespace std {
 
@@ -20,13 +21,13 @@ _LIBCPP_SAFE_STATIC static std::unexpect
 unexpected_handler
 set_unexpected(unexpected_handler func) _NOEXCEPT
 {
-  return __sync_lock_test_and_set(&__unexpected_handler, func);
+  return __libcpp_sync_lock_test_and_set(&__unexpected_handler, func);
 }
 
 unexpected_handler
 get_unexpected() _NOEXCEPT
 {
-  return 

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

[Jordan Rose via Phabricator via cfe-commits]

jordan_rose added inline comments.



Comment at: lib/Basic/CMakeLists.txt:27
+STRING(REGEX REPLACE "\n" "" file_contents "${file_contents}")
+if ("${file_contents}" MATCHES "^gitdir:")
+  # '.git' is indeed a link to the submodule's Git directory.

How about

```
if("${file_contents}" MATCHES "^gitdir: ([^\n]+)")
  set(git_path "${git_path}${CMAKE_MATCH_0}")
endif()
```

and then no stripping or replacing later at all? This admittedly doesn't handle 
some future submodule format that doesn't put `gitdir` on the first line, but 
neither does the code that's there.

(This also doesn't handle absolute path submodule references, but looking 
around a bit makes it sound like those aren't supposed to occur in practice and 
are considered bugs when git does generate them. I could be wrong about that 
though.)


https://reviews.llvm.org/D34955



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

[libcxx] r307593 - Revert "[libc++] Refactoring __sync_* builtins; NFC"

[Weiming Zhao via cfe-commits]

Author: weimingz
Date: Mon Jul 10 14:23:32 2017
New Revision: 307593

URL: http://llvm.org/viewvc/llvm-project?rev=307593=rev
Log:
Revert "[libc++] Refactoring __sync_* builtins; NFC"

This reverts commit 72ff8866bca49ee7d24c87673293b4ce88a039ec.

Removed:
libcxx/trunk/include/__atomic_support
Modified:
libcxx/trunk/include/__refstring
libcxx/trunk/src/locale.cpp
libcxx/trunk/src/support/runtime/exception_fallback.ipp
libcxx/trunk/src/support/runtime/new_handler_fallback.ipp

Removed: libcxx/trunk/include/__atomic_support
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__atomic_support?rev=307592=auto
==
--- libcxx/trunk/include/__atomic_support (original)
+++ libcxx/trunk/include/__atomic_support (removed)
@@ -1,31 +0,0 @@
-// -*- C++ -*-
-//=== __atomic_support 
-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is dual licensed under the MIT and the University of Illinois Open
-// Source Licenses. See LICENSE.TXT for details.
-//
-//===--===//
-
-#ifndef _LIBCPP___ATOMIC_SUPPORT
-#define _LIBCPP___ATOMIC_SUPPORT
-
-#include <__config>
-
-template
-T __libcpp_sync_add_and_fetch(T *ptr, T value) {
-  return __sync_add_and_fetch(ptr, value);
-}
-
-template
-T __libcpp_sync_fetch_and_add(T *ptr, T value) {
-  return __sync_fetch_and_add(ptr, value);
-}
-
-template
-T __libcpp_sync_lock_test_and_set(T *ptr, T value) {
-  return __sync_lock_test_and_set(ptr, value);
-}
-
-#endif //_LIBCPP___ATOMIC_SUPPORT

Modified: libcxx/trunk/include/__refstring
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__refstring?rev=307593=307592=307593=diff
==
--- libcxx/trunk/include/__refstring (original)
+++ libcxx/trunk/include/__refstring Mon Jul 10 14:23:32 2017
@@ -14,7 +14,6 @@
 #include 
 #include 
 #include 
-#include <__atomic_support>
 #ifdef __APPLE__
 #include 
 #include 
@@ -84,7 +83,7 @@ __libcpp_refstring::__libcpp_refstring(c
 : __imp_(s.__imp_)
 {
 if (__uses_refcount())
-__libcpp_sync_add_and_fetch(_from_data(__imp_)->count, 1);
+__sync_add_and_fetch(_from_data(__imp_)->count, 1);
 }
 
 inline
@@ -93,10 +92,10 @@ __libcpp_refstring& __libcpp_refstring::
 struct _Rep_base *old_rep = rep_from_data(__imp_);
 __imp_ = s.__imp_;
 if (__uses_refcount())
-__libcpp_sync_add_and_fetch(_from_data(__imp_)->count, 1);
+__sync_add_and_fetch(_from_data(__imp_)->count, 1);
 if (adjust_old_count)
 {
-if (__libcpp_sync_add_and_fetch(_rep->count, count_t(-1)) < 0)
+if (__sync_add_and_fetch(_rep->count, count_t(-1)) < 0)
 {
 ::operator delete(old_rep);
 }
@@ -108,7 +107,7 @@ inline
 __libcpp_refstring::~__libcpp_refstring() {
 if (__uses_refcount()) {
 _Rep_base* rep = rep_from_data(__imp_);
-if (__libcpp_sync_add_and_fetch(>count, count_t(-1)) < 0) {
+if (__sync_add_and_fetch(>count, count_t(-1)) < 0) {
 ::operator delete(rep);
 }
 }

Modified: libcxx/trunk/src/locale.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/locale.cpp?rev=307593=307592=307593=diff
==
--- libcxx/trunk/src/locale.cpp (original)
+++ libcxx/trunk/src/locale.cpp Mon Jul 10 14:23:32 2017
@@ -667,7 +667,7 @@ locale::id::__get()
 void
 locale::id::__init()
 {
-__id_ = __libcpp_sync_add_and_fetch(&__next_id, 1);
+__id_ = __sync_add_and_fetch(&__next_id, 1);
 }
 
 // template <> class collate_byname

Modified: libcxx/trunk/src/support/runtime/exception_fallback.ipp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/support/runtime/exception_fallback.ipp?rev=307593=307592=307593=diff
==
--- libcxx/trunk/src/support/runtime/exception_fallback.ipp (original)
+++ libcxx/trunk/src/support/runtime/exception_fallback.ipp Mon Jul 10 14:23:32 
2017
@@ -9,7 +9,6 @@
 
//===--===//
 
 #include 
-#include <__atomic_support>
 
 namespace std {
 
@@ -21,13 +20,13 @@ _LIBCPP_SAFE_STATIC static std::unexpect
 unexpected_handler
 set_unexpected(unexpected_handler func) _NOEXCEPT
 {
-  return __libcpp_sync_lock_test_and_set(&__unexpected_handler, func);
+  return __sync_lock_test_and_set(&__unexpected_handler, func);
 }
 
 unexpected_handler
 get_unexpected() _NOEXCEPT
 {
-  return __libcpp_sync_fetch_and_add(&__unexpected_handler, 
(unexpected_handler)0);
+  return __sync_fetch_and_add(&__unexpected_handler, (unexpected_handler)0);
 
 }
 
@@ -42,13 +41,14 @@ void unexpected()
 terminate_handler
 set_terminate(terminate_handler 

r307524 - X86 Intrinsics: _bit_scan_forward should not be under #ifdef __RDRND__

[Zvi Rackover via cfe-commits]

Author: zvi
Date: Mon Jul 10 00:13:56 2017
New Revision: 307524

URL: http://llvm.org/viewvc/llvm-project?rev=307524=rev
Log:
X86 Intrinsics: _bit_scan_forward should not be under #ifdef __RDRND__

Summary:
The _bit_scan_forward and _bit_scan_reverse intrinsics were accidentally
masked under the preprocessor checks that prune intrinsics definitions for the
benefit of faster compile-time on Windows. This patch moves the
definitons out of that region.

Fixes pr33722

Reviewers: craig.topper, aaboud, thakis

Reviewed By: craig.topper

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/Headers/immintrin.h
cfe/trunk/test/CodeGen/bitscan-builtins.c

Modified: cfe/trunk/lib/Headers/immintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/immintrin.h?rev=307524=307523=307524=diff
==
--- cfe/trunk/lib/Headers/immintrin.h (original)
+++ cfe/trunk/lib/Headers/immintrin.h Mon Jul 10 00:13:56 2017
@@ -212,6 +212,15 @@ _rdrand32_step(unsigned int *__p)
   return __builtin_ia32_rdrand32_step(__p);
 }
 
+#ifdef __x86_64__
+static __inline__ int __attribute__((__always_inline__, __nodebug__, 
__target__("rdrnd")))
+_rdrand64_step(unsigned long long *__p)
+{
+  return __builtin_ia32_rdrand64_step(__p);
+}
+#endif
+#endif /* __RDRND__ */
+
 /* __bit_scan_forward */
 static __inline__ int __attribute__((__always_inline__, __nodebug__))
 _bit_scan_forward(int __A) {
@@ -224,15 +233,6 @@ _bit_scan_reverse(int __A) {
   return 31 - __builtin_clz(__A);
 }
 
-#ifdef __x86_64__
-static __inline__ int __attribute__((__always_inline__, __nodebug__, 
__target__("rdrnd")))
-_rdrand64_step(unsigned long long *__p)
-{
-  return __builtin_ia32_rdrand64_step(__p);
-}
-#endif
-#endif /* __RDRND__ */
-
 #if !defined(_MSC_VER) || __has_feature(modules) || defined(__FSGSBASE__)
 #ifdef __x86_64__
 static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, 
__target__("fsgsbase")))

Modified: cfe/trunk/test/CodeGen/bitscan-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/bitscan-builtins.c?rev=307524=307523=307524=diff
==
--- cfe/trunk/test/CodeGen/bitscan-builtins.c (original)
+++ cfe/trunk/test/CodeGen/bitscan-builtins.c Mon Jul 10 00:13:56 2017
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown -emit-llvm -o 
- %s | FileCheck %s
 
+// PR33722
+// RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown -D_MSC_VER 
-emit-llvm -o - %s | FileCheck %s
+
 #include 
 
 int test_bit_scan_forward(int a) {


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

[PATCH] D35187: [libclang] Support for querying whether an enum is scoped

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Thanks for the patch.
I have just one minor comment:




Comment at: tools/libclang/CIndex.cpp:7815
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const EnumDecl *Enum = D ? dyn_cast_or_null(D) : nullptr;
+  return (Enum && Enum->isScoped()) ? 1 : 0;

`dyn_cast_or_null` already checks if `D` is null, so the ternary operator is 
redundant. You can also use `const auto *` once you use just the 
`dyn_cast_or_null`.


https://reviews.llvm.org/D35187



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

[PATCH] D31326: Add option to export fixes to run-clang-tidy.py

[Michael F. Herbst via Phabricator via cfe-commits]

mfherbst updated this revision to Diff 105835.
mfherbst added a comment.

Correct the nits suggested by alexfh


https://reviews.llvm.org/D31326

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


Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -35,6 +35,7 @@
 """
 
 import argparse
+import glob
 import json
 import multiprocessing
 import os
@@ -45,6 +46,7 @@
 import sys
 import tempfile
 import threading
+import yaml
 
 
 def find_compilation_database(path):
@@ -87,14 +89,36 @@
   return start
 
 
+def merge_replacement_files(tmpdir, mergefile):
+  """Merge all replacement files in a directory into a single file"""
+  # MainSourceFile: The key is required by the definition inside
+  # include/clang/Tooling/ReplacementsYaml.h, but the value
+  # is actually never used inside clang-apply-replacements,
+  # so we set it to '' here.
+  merged={ 'MainSourceFile': '', 'Replacements': [] }
+
+  for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')):
+with open(replacefile, 'r') as f:
+  content = yaml.safe_load(f)
+  if not content:
+continue # Skip empty files.
+
+try:
+  merged['Replacements'].extend(content['Replacements'])
+except KeyError:
+  pass # Ignore files with missing keys.
+
+  if merged['Replacements']:
+with open(mergefile,'w') as out:
+  yaml.safe_dump(merged, out)
+
 def apply_fixes(args, tmpdir):
-  """Calls clang-apply-fixes on a given directory. Deletes the dir when 
done."""
+  """Calls clang-apply-fixes on a given directory."""
   invocation = [args.clang_apply_replacements_binary]
   if args.format:
 invocation.append('-format')
   invocation.append(tmpdir)
   subprocess.call(invocation)
-  shutil.rmtree(tmpdir)
 
 
 def run_tidy(args, tmpdir, build_path, queue):
@@ -129,6 +153,9 @@
   'headers to output diagnostics from. Diagnostics from '
   'the main file of each translation unit are always '
   'displayed.')
+  parser.add_argument('-export-fixes', metavar='filename', dest='export_fixes',
+  help='Create a yaml file to store suggested fixes in, '
+  'which can be applied with clang-apply-replacements.')
   parser.add_argument('-j', type=int, default=0,
   help='number of tidy instances to be run in parallel.')
   parser.add_argument('files', nargs='*', default=['.*'],
@@ -178,7 +205,7 @@
 max_task = multiprocessing.cpu_count()
 
   tmpdir = None
-  if args.fix:
+  if args.fix or args.export_fixes:
 tmpdir = tempfile.mkdtemp()
 
   # Build up a big regexy filter from all command line arguments.
@@ -205,13 +232,20 @@
 # This is a sad hack. Unfortunately subprocess goes
 # bonkers with ctrl-c and we start forking merrily.
 print '\nCtrl-C detected, goodbye.'
-if args.fix:
+if tmpdir:
   shutil.rmtree(tmpdir)
 os.kill(0, 9)
 
+  if args.export_fixes:
+print 'Writing fixes to ' + args.export_fixes + '...'
+merge_replacement_files(tmpdir, args.export_fixes)
+
   if args.fix:
 print 'Applying fixes ...'
 apply_fixes(args, tmpdir)
 
+  if tmpdir:
+shutil.rmtree(tmpdir)
+
 if __name__ == '__main__':
   main()


Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -35,6 +35,7 @@
 """
 
 import argparse
+import glob
 import json
 import multiprocessing
 import os
@@ -45,6 +46,7 @@
 import sys
 import tempfile
 import threading
+import yaml
 
 
 def find_compilation_database(path):
@@ -87,14 +89,36 @@
   return start
 
 
+def merge_replacement_files(tmpdir, mergefile):
+  """Merge all replacement files in a directory into a single file"""
+  # MainSourceFile: The key is required by the definition inside
+  # include/clang/Tooling/ReplacementsYaml.h, but the value
+  # is actually never used inside clang-apply-replacements,
+  # so we set it to '' here.
+  merged={ 'MainSourceFile': '', 'Replacements': [] }
+
+  for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')):
+with open(replacefile, 'r') as f:
+  content = yaml.safe_load(f)
+  if not content:
+continue # Skip empty files.
+
+try:
+  merged['Replacements'].extend(content['Replacements'])
+except KeyError:
+  pass # Ignore files with missing keys.
+
+  if merged['Replacements']:
+with open(mergefile,'w') as out:
+  yaml.safe_dump(merged, out)
+
 def apply_fixes(args, tmpdir):
-  """Calls clang-apply-fixes on a given directory. Deletes the dir when done."""
+  """Calls clang-apply-fixes on a given directory."""
   invocation = [args.clang_apply_replacements_binary]
   if args.format:
 invocation.append('-format')
   invocation.append(tmpdir)
   subprocess.call(invocation)
-  shutil.rmtree(tmpdir)
 

Re: [clang-tools-extra] r303735 - Modify test so that it looks for patterns in stderr as well

[Alexander Kornienko via cfe-commits]

Sorry, missed this thread somehow. So, the behavior itself seems incorrect.
Clang tools should not be trying to find a compilation database in case the
command line has a `--`, but the driver fails to parse it. It should be a
hard failure. We also need a reliable test for this behavior (with a
compile_commands.json being put into a test directory or generated during a
test).

On Tue, Jun 27, 2017 at 3:33 AM, David Blaikie  wrote:

>
>
> On Mon, Jun 26, 2017 at 5:31 AM Serge Pavlov  wrote:
>
>> 2017-06-26 4:05 GMT+07:00 David Blaikie :
>>
>>> Ah, I see now then.
>>>
>>> I have a symlink from the root of my source directory pointing to the
>>> compile_commands.json in my build directory.
>>>
>>> I have this so that the vim YouCompleteMe plugin (& any other clang
>>> tools) can find it, as they usually should, for using tools with the
>>> llvm/clang project...
>>>
>>> Sounds like this test is incompatible with using the tooling
>>> infrastructure in the llvm/clang project?
>>>
>> Any test that relies on compilation database search can fail in such
>> case. Maybe the root of the tools test could contain some special
>> compile_commands.json so that the search will always end up in definite
>> state?
>>
>
> Perhaps - or maybe tools could have a parameter limiting how many parent
> directories to recurse up through? But yeah, dunno what the best solution
> would be.
>
>
>>
>>
>>>
>>> On Sun, Jun 25, 2017, 10:24 AM Serge Pavlov  wrote:
>>>
 2017-06-25 0:52 GMT+07:00 David Blaikie :

>
>
> On Sat, Jun 24, 2017 at 10:08 AM Serge Pavlov 
> wrote:
>
>> With CMAKE_EXPORT_COMPILE_COMMANDS the file compile_commands.json is
>> created in the directory 
>> /tools/clang/tools/extra/test/clang-tidy/Output,
>>
>>
>
> I'd be really surprised if this is the case - why would
> cmake/ninja/makefiles put the compile commands for the whole LLVM
> project/build in that somewhat random subdirectory?
>

 I was wrong, these json files were not created by cmake run but appear
 during test run. The file created by cmake is in the build root.


>
>
>> but the tests from /llvm/tools/clang/tools/extra/test/clang-tidy
>> run in the directory /tools/clang/tools/extra/test/clang-tidy,
>> which does not contain json files. So the test passes successfully. 
>> Ubuntu
>> 16.04, cmake 3.5.1.
>>
>
> Ah, perhaps you found a compile_commands for one of the test cases,
> not the one generated by CMake. CMake 3.5.1 doesn't support
> CMAKE_EXPORT_COMPILE_COMMANDS.
>
> It was added in 3.5.2, according to the documentation: https://cmake.
> org/cmake/help/v3.5/variable/CMAKE_EXPORT_COMPILE_COMMANDS.html
>
>

 It was added in 2.8.5 according to documentation (
 http://clang.llvm.org/docs/JSONCompilationDatabase.html#
 supported-systems), at least the version 3.5.1 creates compilation
 databases.

 clang-tidy tries to create compilation database from source path,
 looking for compile_commands.json in the directory where provided source
 file resides and in all its parent directories. If source tree is in a
 subdirectory of build tree, then compile_commands.json in the build
 directory would be found and the test would fail. Is it your case?


>> Thanks,
>> --Serge
>>
>> 2017-06-24 9:42 GMT+07:00 David Blaikie :
>>
>>> Ping (+Manuel, perhaps he's got some ideas about this, given
>>> background in the tooling & compilation database work, or could point 
>>> this
>>> to someone who does?)
>>>
>>>
>>> On Thu, Jun 15, 2017 at 10:40 AM David Blaikie 
>>> wrote:
>>>
 https://sarcasm.github.io/notes/dev/compilation-database.html#cmake

 If you enable the CMAKE_EXPORT_COMPILE_COMMANDS option in cmake (&
 have a sufficiently recent cmake), then CMake will generate a
 compile_commands.json in the root of the build tree. The test finds 
 this &
 fails, instead of finding no compilation database & succeeding.

 (to use this, you can then symlink from the root of the source tree
 to point to this in your build tree - this is how I get YCM to work 
 for my
 LLVM builds & could work for other clang tools as well)

 On Thu, Jun 15, 2017 at 7:51 AM Serge Pavlov 
 wrote:

> 2017-06-15 2:43 GMT+07:00 David Blaikie :
>
>>
>>
>> On Wed, Jun 14, 2017, 8:17 AM Serge Pavlov 
>> wrote:
>>
>>> 2017-06-14 4:24 GMT+07:00 David Blaikie :
>>>
 Ah, I find that the test passes if 

[PATCH] D31326: Add option to export fixes to run-clang-tidy.py

[Michael F. Herbst via Phabricator via cfe-commits]

mfherbst added a comment.

I do not have commit rights to the svn if that's what you're asking, but I 
could send the patch to llvm-commits if that makes it easier for you.


https://reviews.llvm.org/D31326



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

[PATCH] D31326: Add option to export fixes to run-clang-tidy.py

[Alexander Kornienko via Phabricator via cfe-commits]

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

Just noticed another couple of nits, otherwise looks good. Thanks!

Do you need me to commit the patch for you?




Comment at: clang-tidy/tool/run-clang-tidy.py:158
+  help='Create a yaml file to store suggested fixes in, '
+  'which can be applied with clang-apply-replacements')
   parser.add_argument('-j', type=int, default=0,

nit: Add a trailing period.



Comment at: clang-tidy/tool/run-clang-tidy.py:240
+  if args.export_fixes:
+print 'Writing fixes to ' + args.export_fixes
+merge_replacement_files(tmpdir, args.export_fixes)

nit: Add ` + '...'` at the end for consistency with other similar messages.


https://reviews.llvm.org/D31326



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

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

[Alexander Kornienko via Phabricator via cfe-commits]

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



Comment at: clang-tidy/utils/ASTUtils.h:24
+
+/// Check whether a macro flag presents in the given argument. Only consider
+/// cases of single match or match in a binary OR expression. For example,

s/presents/is present/



Comment at: clang-tidy/utils/ASTUtils.h:27
+///  or  |  | ...
+bool hasFlag(const Expr *Flags, const SourceManager ,
+ const LangOptions , const char *CloseOnExecFlag);

`hasFlag` is too generic and may give a totally wrong impression of what the 
function does. Even in the context of this utility library I would prefer to 
expand the name and make it less ambiguous. Something along the lines of 
`exprHasBitFlagWithSpelling`. I wouldn't also insist on the flag being 
implemented as a macro, since this restriction would prevent many valid use 
cases with enum or constexpr int flags.



Comment at: clang-tidy/utils/ASTUtils.h:28
+bool hasFlag(const Expr *Flags, const SourceManager ,
+ const LangOptions , const char *CloseOnExecFlag);
 } // namespace utils

`CloseOnExecFlag` is too specific for a function that is meant to be somewhat 
generic.



Comment at: docs/ReleaseNotes.rst:79
+
+  Checks if the required file flag ``SOCK_CLOEXEC`` presents in the argument of
+  ``socket()``.

s/presents/is present/



Comment at: test/clang-tidy/android-cloexec-socket.cpp:20
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'socket' should use 
SOCK_CLOEXEC where possible [android-cloexec-socket]
+  // CHECK-FIXES: SOCK_STREAM | SOCK_CLOEXEC
+  TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM, 0));

alexfh wrote:
> hokein wrote:
> > I'd use the complete statement for checking the fixes here, the same below.
> Yes, CHECK-FIXES has fewer context than CHECK-MESSAGES (it doesn't have the 
> line number, for example), and there's much more stuff that can go wrong than 
> with the static diagnostic messages. So please make CHECK-FIXES as specific, 
> as possible. Ideally, they should be unique, so that a wrong line can't be 
> matched by a CHECK-FIXES. If needed, add unique comments on identical lines.
This is not addressed yet. The test contains a number of ambiguous CHECK-FIXES 
patterns. Please make each pattern (and the line being matched) unique to avoid 
incorrect matches. One way to do this is to add unique trailing comments to 
each of the similar lines and include this comment into the pattern.



Comment at: test/clang-tidy/android-cloexec-socket.cpp:65
+  socket(0, SOCK_CLOEXEC, 0);
+  // CHECK-MESSAGES-NOT: warning:
+  TEMP_FAILURE_RETRY(socket(0, SOCK_CLOEXEC, 0));

`// CHECK-MESSAGES-NOT: warning:` is redundant. The test script runs FileCheck 
with `-implicit-check-not={{warning:|error:}}`, which takes care of stray 
`warning:` lines.


https://reviews.llvm.org/D34913



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

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Thanks, that's pretty cool!

How bigger did the clang binary get after you've added all these strings?
I feel like this is more of a CC1 option as well.

I have some feedback for your additional ideas:

> add another option to pass in the index (or enum) to force an assert or 
> backtrace when a specific DiagID is seen.

That sounds quite useful, but could it be something that's more suited for an 
external debugging script? I have a personal script that computes the enum 
value for a particular diagnostic, launches clang in lldb, sets a breakpoint 
for that particular diagnostic enum and runs clang. I could work on upstreaming 
it into clang/utils if people are interested.

> capture FILE and LINE when a diagnostic is created and output it. This would 
> make it easier to find the specific instance, and verify all instances are 
> actually tested. Currently, it's almost impossible to determine if all 
> instances are actually tested.

I reckon the first part (find the specific instance) could be useful, but I 
think that if you can force a backtrace on a specific DiagID then it becomes 
less useful. I disagree with the second part, can't you use our coverage bots 
and see if the all places where the diagnostic is emitted are covered to see if 
they are tested? It might be tedious to find these places, but maybe we can add 
a search for our coverage viewer so you quickly find the lines that have the 
name of diagnostic?


https://reviews.llvm.org/D35175



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

[PATCH] D16403: Add scope information to CFG

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko added a comment.

Hi Artem, I'm sorry for a long delay (nasty corporate issues).

In https://reviews.llvm.org/D16403#789957, @NoQ wrote:

> Maxim, totally thanks for picking this up!
>
> Could you explain the idea behind `shouldDeferScopeEnd`, maybe in a code 
> comment before the function?
>
> In https://reviews.llvm.org/D16403#788926, @m.ostapenko wrote:
>
> > Current patch should support basic {If, While, For, Compound, Switch}Stmts 
> > as well as their interactions with {Break, Continue, Return}Stmts.
> >  GotoStmt and CXXForRangeStmt are not supported at this moment.
>
>
> `SwitchStmt` isn't much easier than `GotoStmt`; it doesn't jump backwards, 
> but it can still jump into multiple different scopes. Does your code handle 
> Duff's device (1)  (2) 
>  correctly? We should probably 
> add it as a test, or split out switch support into a separate patch together 
> with goto, if such test isn't yet supported.


Ugh, yeah, SwitchStmt is tricky (thank you for pointing to Duff's device 
example!). I've tried to resolve several issues with switch revealed by this 
testcase, but didn't succeed for now :(. So, it was decided to remove 
SwitchStmt support in this patch.
There is one more thing I'd like to clarify: since e.g. BreakStmt can terminate 
multiple scopes, do I need to create ScopeEnd marks for all of them to make 
analyzer's work easier? Because right now only one "cumulative" block is 
generated and I'm not sure that it's acceptable for analyzer.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

Re: [clang-tools-extra] r303735 - Modify test so that it looks for patterns in stderr as well

[Alexander Kornienko via cfe-commits]

Benjamin has actually fixed the issue by reverting to the old behavior
in r306822.
I'll add a test for this behavior anyway.

On Mon, Jul 10, 2017 at 11:47 AM, Alexander Kornienko 
wrote:

> Sorry, missed this thread somehow. So, the behavior itself seems
> incorrect. Clang tools should not be trying to find a compilation database
> in case the command line has a `--`, but the driver fails to parse it. It
> should be a hard failure. We also need a reliable test for this behavior
> (with a compile_commands.json being put into a test directory or generated
> during a test).
>
>
> On Tue, Jun 27, 2017 at 3:33 AM, David Blaikie  wrote:
>
>>
>>
>> On Mon, Jun 26, 2017 at 5:31 AM Serge Pavlov  wrote:
>>
>>> 2017-06-26 4:05 GMT+07:00 David Blaikie :
>>>
 Ah, I see now then.

 I have a symlink from the root of my source directory pointing to the
 compile_commands.json in my build directory.

 I have this so that the vim YouCompleteMe plugin (& any other clang
 tools) can find it, as they usually should, for using tools with the
 llvm/clang project...

 Sounds like this test is incompatible with using the tooling
 infrastructure in the llvm/clang project?

>>> Any test that relies on compilation database search can fail in such
>>> case. Maybe the root of the tools test could contain some special
>>> compile_commands.json so that the search will always end up in definite
>>> state?
>>>
>>
>> Perhaps - or maybe tools could have a parameter limiting how many parent
>> directories to recurse up through? But yeah, dunno what the best solution
>> would be.
>>
>>
>>>
>>>

 On Sun, Jun 25, 2017, 10:24 AM Serge Pavlov 
 wrote:

> 2017-06-25 0:52 GMT+07:00 David Blaikie :
>
>>
>>
>> On Sat, Jun 24, 2017 at 10:08 AM Serge Pavlov 
>> wrote:
>>
>>> With CMAKE_EXPORT_COMPILE_COMMANDS the file compile_commands.json
>>> is created in the directory 
>>> /tools/clang/tools/extra/test/clang-tidy/Output,
>>>
>>>
>>
>> I'd be really surprised if this is the case - why would
>> cmake/ninja/makefiles put the compile commands for the whole LLVM
>> project/build in that somewhat random subdirectory?
>>
>
> I was wrong, these json files were not created by cmake run but appear
> during test run. The file created by cmake is in the build root.
>
>
>>
>>
>>> but the tests from 
>>> /llvm/tools/clang/tools/extra/test/clang-tidy
>>> run in the directory /tools/cl
>>> ang/tools/extra/test/clang-tidy, which does not contain json files.
>>> So the test passes successfully. Ubuntu 16.04, cmake 3.5.1.
>>>
>>
>> Ah, perhaps you found a compile_commands for one of the test cases,
>> not the one generated by CMake. CMake 3.5.1 doesn't support
>> CMAKE_EXPORT_COMPILE_COMMANDS.
>>
>> It was added in 3.5.2, according to the documentation:
>> https://cmake.org/cmake/help/v3.5/variable/CM
>> AKE_EXPORT_COMPILE_COMMANDS.html
>>
>>
>
> It was added in 2.8.5 according to documentation (
> http://clang.llvm.org/docs/JSONCompilationDatabase.html#sup
> ported-systems), at least the version 3.5.1 creates compilation
> databases.
>
> clang-tidy tries to create compilation database from source path,
> looking for compile_commands.json in the directory where provided source
> file resides and in all its parent directories. If source tree is in a
> subdirectory of build tree, then compile_commands.json in the build
> directory would be found and the test would fail. Is it your case?
>
>
>>> Thanks,
>>> --Serge
>>>
>>> 2017-06-24 9:42 GMT+07:00 David Blaikie :
>>>
 Ping (+Manuel, perhaps he's got some ideas about this, given
 background in the tooling & compilation database work, or could point 
 this
 to someone who does?)


 On Thu, Jun 15, 2017 at 10:40 AM David Blaikie 
 wrote:

> https://sarcasm.github.io/notes/dev/compilation-database.
> html#cmake
>
> If you enable the CMAKE_EXPORT_COMPILE_COMMANDS option in cmake (&
> have a sufficiently recent cmake), then CMake will generate a
> compile_commands.json in the root of the build tree. The test finds 
> this &
> fails, instead of finding no compilation database & succeeding.
>
> (to use this, you can then symlink from the root of the source
> tree to point to this in your build tree - this is how I get YCM to 
> work
> for my LLVM builds & could work for other clang tools as well)
>
> On Thu, Jun 15, 2017 at 7:51 AM Serge Pavlov 

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

You are making a pretty good progress!
I think right now there is some code duplication that could be reduced, but 
otherwise, the checker starts to look really good.




Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:43
+private:
+  bool IsVirtualCall(const CallExpr *CE) const;
+

This could be maybe a free static function instead of a method?



Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:47
+  private:
+const MemRegion *Region;
+bool Found;

Maybe instead of Region the name could be more specific? I.e. the ThisRegion? 
Or ObjectRegion? 



Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:101
+DeclName = CD->getNameAsString();
+InfoText = "Called from this constrctor " + DeclName;
+  } else {

In the warning messages we usually enclose the source code snippets (e.g.: 
declaration names) in single quotes. 



Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:135
+  if (isa(MD)) {
+auto  = C.getSValBuilder();
+auto ThiSVal =

getting the SValBuilder is cheap, maybe you could hoist it from the if 
statements. 



Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:146
+// The EndFunction callback when leave a constructor or a destructor.
+void VirtualCallChecker::checkEndFunction(CheckerContext ) const {
+  const auto *LCtx = C.getLocationContext();

The code of this function is almost completely identical with the checkBegin. I 
was thinking about abstracting this into a separate function with a bool 
argument to determine whether we would like to add something to the GDM or 
remove it. 



Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:186
 
-void WalkAST::ReportVirtualCall(const CallExpr *CE, bool isPure) {
-  if (ReportPureOnly && !isPure)
+  const auto IC = dyn_cast();
+  if (!IC)

Maybe you could cast this to CXXMemberCall instead. If that cast succeed, than 
you do not need to check the Call.getDecl() and also do not need to check for 
ctor/dtor calls. 



Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:199
+  // The GDM of constructor and destructor should be true.
+  if (IsVirtualCall(CE) && State->get(Reg)) {
+if (IsPureOnly && MD->isPure()) {

Maybe it would be cleaner to early return if the call is not virtual and remove 
it from these two if statements. 



Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:213
+} else {
+  ExplodedNode *N = C.generateNonFatalErrorNode();
+  if (!BT)

Maybe you could extract the error reporting into a separate function with two 
arguments: the error message, and whether it should generate an error node. 


https://reviews.llvm.org/D34275



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

[PATCH] D16403: Add scope information to CFG

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D16403#803518, @m.ostapenko wrote:

> There is one more thing I'd like to clarify: since e.g. BreakStmt can 
> terminate multiple scopes, do I need to create ScopeEnd marks for all of them 
> to make analyzer's work easier? Because right now only one "cumulative" block 
> is generated and I'm not sure that it's acceptable for analyzer.


The analyzer intends to maintain a stack of scopes. When the scope is entered 
during symbolic execution, it gets put on top of the stack, and when it is 
exited it is taken off the top of the stack. It's probably not important if we 
have one mark or multiple mark, but it's important to know what scopes, in what 
order, are getting entered or exited.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D34430: [Clang][TypoCorrection] Clang hangs in typo correction

[Oleg Ranevskyy via Phabricator via cfe-commits]

iid_iunknown added a comment.

A gentle reminder about this patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D34430



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

[PATCH] D33644: Add default values for function parameter chunks

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

Ping :)
It's quite likely ok now because covered with tests. Check please.


https://reviews.llvm.org/D33644



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

[PATCH] D35200: Don't use mmap on Windows

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan created this revision.

Memory mapping does not make llvm faster (at least I don't see any difference).
It also allows llvm not to lock files which is sometimes quite important.


https://reviews.llvm.org/D35200

Files:
  lib/Support/MemoryBuffer.cpp


Index: lib/Support/MemoryBuffer.cpp
===
--- lib/Support/MemoryBuffer.cpp
+++ lib/Support/MemoryBuffer.cpp
@@ -284,6 +284,10 @@
   bool RequiresNullTerminator,
   int PageSize,
   bool IsVolatile) {
+#ifdef _WIN32
+  // Do not use mmap on Windows in order to avoid file locking
+  return false;
+#endif
   // mmap may leave the buffer without null terminator if the file size changed
   // by the time the last page is mapped in, so avoid it if the file size is
   // likely to change.


Index: lib/Support/MemoryBuffer.cpp
===
--- lib/Support/MemoryBuffer.cpp
+++ lib/Support/MemoryBuffer.cpp
@@ -284,6 +284,10 @@
   bool RequiresNullTerminator,
   int PageSize,
   bool IsVolatile) {
+#ifdef _WIN32
+  // Do not use mmap on Windows in order to avoid file locking
+  return false;
+#endif
   // mmap may leave the buffer without null terminator if the file size changed
   // by the time the last page is mapped in, so avoid it if the file size is
   // likely to change.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r307535 - Improve error message when run from a buffer that doesn't visit a file

[Philipp Stephani via cfe-commits]

Author: phst
Date: Mon Jul 10 06:49:18 2017
New Revision: 307535

URL: http://llvm.org/viewvc/llvm-project?rev=307535=rev
Log:
Improve error message when run from a buffer that doesn't visit a file

Summary: In this case, users currently get a confusing type error.  Make the 
error message more obvious.

Reviewers: klimek

Reviewed By: klimek

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

Modified:
clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el

Modified: clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el?rev=307535=307534=307535=diff
==
--- clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el (original)
+++ clang-tools-extra/trunk/include-fixer/tool/clang-include-fixer.el Mon Jul 
10 06:49:18 2017
@@ -88,6 +88,8 @@ The current file name is passed after AR
 the call was successful the returned result is stored in a
 temporary buffer, and CALLBACK is called with the temporary
 buffer as only argument."
+  (unless buffer-file-name
+(user-error "clang-include-fixer works only in buffers that visit a file"))
   (let ((process (if (fboundp 'make-process)
  ;; Prefer using ‘make-process’ if available, because
  ;; ‘start-process’ doesn’t allow us to separate the


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

[PATCH] D35174: [libc++] Fix unrepresentable enum for clang-cl unstable ABI

[Ben Craig via Phabricator via cfe-commits]

bcraig updated this revision to Diff 105845.
bcraig edited the summary of this revision.
bcraig added a comment.

Switched to static consts


https://reviews.llvm.org/D35174

Files:
  include/string


Index: include/string
===
--- include/string
+++ include/string
@@ -676,11 +676,11 @@
 };
 
 #if _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x01};
-enum {__long_mask  = 0x1ul};
+static const size_type __short_mask = 0x01;
+static const size_type __long_mask  = 0x1ul;
 #else  // _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x80};
-enum {__long_mask  = ~(size_type(~0) >> 1)};
+static const size_type __short_mask = 0x80;
+static const size_type __long_mask  = ~(size_type(~0) >> 1);
 #endif  // _LIBCPP_BIG_ENDIAN
 
 enum {__min_cap = (sizeof(__long) - 1)/sizeof(value_type) > 2 ?
@@ -706,11 +706,11 @@
 };
 
 #if _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x80};
-enum {__long_mask  = ~(size_type(~0) >> 1)};
+static const size_type __short_mask = 0x80;
+static const size_type __long_mask  = ~(size_type(~0) >> 1);
 #else  // _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x01};
-enum {__long_mask  = 0x1ul};
+static const size_type __short_mask = 0x01;
+static const size_type __long_mask  = 0x1ul;
 #endif  // _LIBCPP_BIG_ENDIAN
 
 enum {__min_cap = (sizeof(__long) - 1)/sizeof(value_type) > 2 ?


Index: include/string
===
--- include/string
+++ include/string
@@ -676,11 +676,11 @@
 };
 
 #if _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x01};
-enum {__long_mask  = 0x1ul};
+static const size_type __short_mask = 0x01;
+static const size_type __long_mask  = 0x1ul;
 #else  // _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x80};
-enum {__long_mask  = ~(size_type(~0) >> 1)};
+static const size_type __short_mask = 0x80;
+static const size_type __long_mask  = ~(size_type(~0) >> 1);
 #endif  // _LIBCPP_BIG_ENDIAN
 
 enum {__min_cap = (sizeof(__long) - 1)/sizeof(value_type) > 2 ?
@@ -706,11 +706,11 @@
 };
 
 #if _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x80};
-enum {__long_mask  = ~(size_type(~0) >> 1)};
+static const size_type __short_mask = 0x80;
+static const size_type __long_mask  = ~(size_type(~0) >> 1);
 #else  // _LIBCPP_BIG_ENDIAN
-enum {__short_mask = 0x01};
-enum {__long_mask  = 0x1ul};
+static const size_type __short_mask = 0x01;
+static const size_type __long_mask  = 0x1ul;
 #endif  // _LIBCPP_BIG_ENDIAN
 
 enum {__min_cap = (sizeof(__long) - 1)/sizeof(value_type) > 2 ?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34512: Add preliminary Cross Translation Unit support library

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

gentle ping


https://reviews.llvm.org/D34512



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