[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

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

rsmith added a comment.

In D64799#1589557 , @ilya-biryukov 
wrote:

> I tried to find a good place to emit unresolved typos earlier (to make sure 
> CodeGen does not ever get `TypoExpr`), but couldn't find one.
>  Please let me know if there's some obvious place I'm missing.


The original plan when we were first designing the feature was to emit these 
diagnostics when we pop an expression evaluation context. Maybe we could try 
that? If there's a reason to defer typo correction across such contexts, it 
might probably be rare enough that we can explicitly handle that and manually 
move the delayed typos to the surrounding context.

> unless people object I would propose to land it even if it does not solve all 
> of the problems around delayed exprs.

Sounds good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



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

[PATCH] D63638: [clang][NewPM] Add new pass manager RUN lines to avx512f-builtins.c

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

In D63638#1581973 , @chandlerc wrote:

> Just to make sure we're on the same page (and sorry I didn't jump in 
> sooner)...
>
> With the old PM, *anything* that is `always_inline` *gets* `instsimplify` run 
> on it, even at -O0, even if you didn't want that. So using `-instsimplify` 
> explicitly is, IMO, not any more scary of a reliance on LLVM's behavior than 
> the old PM already subjected us to...
>
> That said, if the x86 maintainers are comfortable with *only* using the new 
> PM (because it has an always inliner that literally does nothing else and 
> thus has an absolute minimum amount of LLVM transformations applied), I 
> certainly don't have any objections. =D


My assumption is that eventually there will only be the "new PM". So eventually 
we'll only be testing that PM. So I don't have any issue testing only it now.


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

https://reviews.llvm.org/D63638



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

[clang-tools-extra] r366400 - [clangd] Fix Fix -Wunused-lambda-capture after r366339

[Fangrui Song via cfe-commits]

Author: maskray
Date: Wed Jul 17 21:23:54 2019
New Revision: 366400

URL: http://llvm.org/viewvc/llvm-project?rev=366400=rev
Log:
[clangd] Fix Fix -Wunused-lambda-capture after r366339

Modified:
clang-tools-extra/trunk/clangd/QueryDriverDatabase.cpp

Modified: clang-tools-extra/trunk/clangd/QueryDriverDatabase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/QueryDriverDatabase.cpp?rev=366400=366399=366400=diff
==
--- clang-tools-extra/trunk/clangd/QueryDriverDatabase.cpp (original)
+++ clang-tools-extra/trunk/clangd/QueryDriverDatabase.cpp Wed Jul 17 21:23:54 
2019
@@ -58,14 +58,13 @@ namespace {
 
 std::vector parseDriverOutput(llvm::StringRef Output) {
   std::vector SystemIncludes;
-  constexpr char const *SIS = "#include <...> search starts here:";
+  const char SIS[] = "#include <...> search starts here:";
   constexpr char const *SIE = "End of search list.";
   llvm::SmallVector Lines;
   Output.split(Lines, '\n', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
 
-  auto StartIt =
-  std::find_if(Lines.begin(), Lines.end(),
-   [SIS](llvm::StringRef Line) { return Line.trim() == SIS; });
+  auto StartIt = llvm::find_if(
+  Lines, [SIS](llvm::StringRef Line) { return Line.trim() == SIS; });
   if (StartIt == Lines.end()) {
 elog("System include extraction: start marker not found: {0}", Output);
 return {};


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

[PATCH] D64678: [Sema] Fix -Wuninitialized for struct assignment from GNU C statement expression

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

rsmith added a comment.

I don't think this problem really has anything to do with statement 
expressions; consider:

  struct Widget a = (init2(), a);

... which has the same behaviour and presumably produces the same warning.

It's just a C / C++ difference. In C++, these examples are undefined because 
the lifetime of the object hasn't started yet, but I think in C they're valid. 
We should just disable the whole warning in C mode and leave it to the CFG 
analysis.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64678



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

[PATCH] D64900: [WebAssembly] Implement __builtin_wasm_tls_base intrinsic

[Heejin Ahn via Phabricator via cfe-commits]

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

What does this return when `__wasm_init_tls` has not been called?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64900



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

[PATCH] D64900: [WebAssembly] Implement __builtin_wasm_tls_base intrinsic

[Guanzhong Chen via Phabricator via cfe-commits]

quantum created this revision.
quantum added reviewers: tlively, aheejin, sbc100.
Herald added subscribers: llvm-commits, cfe-commits, sunfish, hiraditya, 
jgravelle-google, dschuff.
Herald added projects: clang, LLVM.

Add `__builtin_wasm_tls_base` so that LeakSanitizer can find the thread-local
block and scan through it for memory leaks.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64900

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-wasm.c
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
  llvm/test/CodeGen/WebAssembly/tls-general-dynamic.ll

Index: llvm/test/CodeGen/WebAssembly/tls-general-dynamic.ll
===
--- llvm/test/CodeGen/WebAssembly/tls-general-dynamic.ll
+++ llvm/test/CodeGen/WebAssembly/tls-general-dynamic.ll
@@ -75,6 +75,15 @@
   ret i32 %1
 }
 
+; CHECK-LABEL: tls_base:
+; CHECK-NEXT: .functype tls_base () -> (i32)
+define i8* @tls_base() {
+; CHECK-NEXT: global.get __tls_base
+; CHECK-NEXT: return
+  %1 = call i8* @llvm.wasm.tls.base()
+  ret i8* %1
+}
+
 ; CHECK: .type tls,@object
 ; TLS-NEXT: .section .tbss.tls,"",@
 ; NO-TLS-NEXT: .section .bss.tls,"",@
@@ -84,3 +93,4 @@
 @tls = internal thread_local global i32 0
 
 declare i32 @llvm.wasm.tls.size.i32()
+declare i8* @llvm.wasm.tls.base()
Index: llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
@@ -227,6 +227,23 @@
 }
 break;
   }
+  case ISD::INTRINSIC_W_CHAIN: {
+unsigned IntNo = cast(Node->getOperand(1))->getZExtValue();
+switch (IntNo) {
+case Intrinsic::wasm_tls_base: {
+  MVT PtrVT = TLI->getPointerTy(CurDAG->getDataLayout());
+  assert(PtrVT == MVT::i32 && "only wasm32 is supported for now");
+
+  MachineSDNode *TLSBase = CurDAG->getMachineNode(
+  WebAssembly::GLOBAL_GET_I32, DL, MVT::i32,
+  CurDAG->getTargetExternalSymbol("__tls_base", PtrVT),
+  Node->getOperand(0));
+  ReplaceNode(Node, TLSBase);
+  return;
+}
+}
+break;
+  }
 
   default:
 break;
Index: llvm/include/llvm/IR/IntrinsicsWebAssembly.td
===
--- llvm/include/llvm/IR/IntrinsicsWebAssembly.td
+++ llvm/include/llvm/IR/IntrinsicsWebAssembly.td
@@ -133,4 +133,9 @@
 [],
 [IntrNoMem, IntrSpeculatable]>;
 
+def int_wasm_tls_base :
+  Intrinsic<[llvm_ptr_ty],
+[],
+[IntrReadMem]>;
+
 } // TargetPrefix = "wasm"
Index: clang/test/CodeGen/builtins-wasm.c
===
--- clang/test/CodeGen/builtins-wasm.c
+++ clang/test/CodeGen/builtins-wasm.c
@@ -44,6 +44,11 @@
   // WEBASSEMBLY64: call i64 @llvm.wasm.tls.size.i64()
 }
 
+void *tls_base() {
+  return __builtin_wasm_tls_base();
+  // WEBASSEMBLY: call i8* @llvm.wasm.tls.base()
+}
+
 void throw(void *obj) {
   return __builtin_wasm_throw(0, obj);
   // WEBASSEMBLY32: call void @llvm.wasm.throw(i32 0, i8* %{{.*}})
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -13924,6 +13924,10 @@
 Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_tls_size, ResultType);
 return Builder.CreateCall(Callee);
   }
+  case WebAssembly::BI__builtin_wasm_tls_base: {
+Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_tls_base);
+return Builder.CreateCall(Callee);
+  }
   case WebAssembly::BI__builtin_wasm_throw: {
 Value *Tag = EmitScalarExpr(E->getArg(0));
 Value *Obj = EmitScalarExpr(E->getArg(1));
Index: clang/include/clang/Basic/BuiltinsWebAssembly.def
===
--- clang/include/clang/Basic/BuiltinsWebAssembly.def
+++ clang/include/clang/Basic/BuiltinsWebAssembly.def
@@ -31,6 +31,7 @@
 
 // Thread-local storage
 TARGET_BUILTIN(__builtin_wasm_tls_size, "z", "nc", "bulk-memory")
+TARGET_BUILTIN(__builtin_wasm_tls_base, "v*", "n", "bulk-memory")
 
 // Floating point min/max
 BUILTIN(__builtin_wasm_min_f32, "fff", "nc")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63279: [Analyzer] Unroll for-loops where the upper boundary is a variable with know value

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

/me has just noticed that it's not D34812 .


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

https://reviews.llvm.org/D63279



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366391: [analyzer] MallocChecker: Prevent Integer Set 
Library false positives (authored by Charusso, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64680?vs=210458=210463#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64680

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  cfe/trunk/test/Analysis/retain-count-alloc.cpp

Index: cfe/trunk/test/Analysis/retain-count-alloc.cpp
===
--- cfe/trunk/test/Analysis/retain-count-alloc.cpp
+++ cfe/trunk/test/Analysis/retain-count-alloc.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix.Malloc \
+// RUN:  -verify %s
+
+// expected-no-diagnostics: We do not model Integer Set Library's retain-count
+//  based allocation. If any of the parameters has an
+//  '__isl_' prefixed macro definition we escape every
+//  of them when we are about to 'free()' something.
+
+#define __isl_take
+#define __isl_keep
+
+struct Object { int Ref; };
+void free(void *);
+
+Object *copyObj(__isl_keep Object *O) {
+  O->Ref++;
+  return O;
+}
+
+void freeObj(__isl_take Object *O) {
+  if (--O->Ref > 0)
+return;
+
+  free(O); // Here we notice that the parameter contains '__isl_', escape it.
+}
+
+void useAfterFree(__isl_take Object *A) {
+  if (!A)
+return;
+
+  Object *B = copyObj(A);
+  freeObj(B);
+
+  A->Ref = 13;
+  // no-warning: 'Use of memory after it is freed' was here.
+}
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/ParentMap.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -359,6 +360,11 @@
   /// Check if the memory associated with this symbol was released.
   bool isReleased(SymbolRef Sym, CheckerContext ) const;
 
+  /// See if deallocation happens in a suspicious context. If so, escape the
+  /// pointers that otherwise would have been deallocated and return true.
+  bool suppressDeallocationsInSuspiciousContexts(const CallExpr *CE,
+ CheckerContext ) const;
+
   bool checkUseAfterFree(SymbolRef Sym, CheckerContext , const Stmt *S) const;
 
   void checkUseZeroAllocated(SymbolRef Sym, CheckerContext ,
@@ -877,6 +883,9 @@
   State = ProcessZeroAllocation(C, CE, 0, State);
   State = ProcessZeroAllocation(C, CE, 1, State);
 } else if (FunI == II_free || FunI == II_g_free || FunI == II_kfree) {
+  if (suppressDeallocationsInSuspiciousContexts(CE, C))
+return;
+
   State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
 } else if (FunI == II_strdup || FunI == II_win_strdup ||
FunI == II_wcsdup || FunI == II_win_wcsdup) {
@@ -2532,6 +2541,35 @@
   return (RS && RS->isReleased());
 }
 
+bool MallocChecker::suppressDeallocationsInSuspiciousContexts(
+const CallExpr *CE, CheckerContext ) const {
+  if (CE->getNumArgs() == 0)
+return false;
+
+  StringRef FunctionStr = "";
+  if (const auto *FD = dyn_cast(C.getStackFrame()->getDecl()))
+if (const Stmt *Body = FD->getBody())
+  if (Body->getBeginLoc().isValid())
+FunctionStr =
+Lexer::getSourceText(CharSourceRange::getTokenRange(
+ {FD->getBeginLoc(), Body->getBeginLoc()}),
+ C.getSourceManager(), C.getLangOpts());
+
+  // We do not model the Integer Set Library's retain-count based allocation.
+  if (!FunctionStr.contains("__isl_"))
+return false;
+
+  ProgramStateRef State = C.getState();
+
+  for (const Expr *Arg : CE->arguments())
+if (SymbolRef Sym = C.getSVal(Arg).getAsSymbol())
+  if (const RefState *RS = State->get(Sym))
+State = State->set(Sym, RefState::getEscaped(RS));
+
+  C.addTransition(State);
+  return true;
+}
+
 bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext ,
   const Stmt *S) const {
 
@@ -2833,7 +2871,6 @@
 if (const RefState *RS = State->get(sym)) {
   if ((RS->isAllocated() || RS->isAllocatedOfSizeZero()) &&
   CheckRefState(RS)) {
-State = State->remove(sym);
 State = State->set(sym, RefState::getEscaped(RS));
   }
 }

r366391 - [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via cfe-commits]

Author: charusso
Date: Wed Jul 17 17:03:55 2019
New Revision: 366391

URL: http://llvm.org/viewvc/llvm-project?rev=366391=rev
Log:
[analyzer] MallocChecker: Prevent Integer Set Library false positives

Summary:
Integer Set Library using retain-count based allocation which is not
modeled in MallocChecker.

Reviewed By: NoQ

Tags: #clang

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

Added:
cfe/trunk/test/Analysis/retain-count-alloc.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=366391=366390=366391=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Wed Jul 17 17:03:55 
2019
@@ -17,6 +17,7 @@
 #include "clang/AST/ParentMap.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -359,6 +360,11 @@ private:
   /// Check if the memory associated with this symbol was released.
   bool isReleased(SymbolRef Sym, CheckerContext ) const;
 
+  /// See if deallocation happens in a suspicious context. If so, escape the
+  /// pointers that otherwise would have been deallocated and return true.
+  bool suppressDeallocationsInSuspiciousContexts(const CallExpr *CE,
+ CheckerContext ) const;
+
   bool checkUseAfterFree(SymbolRef Sym, CheckerContext , const Stmt *S) 
const;
 
   void checkUseZeroAllocated(SymbolRef Sym, CheckerContext ,
@@ -877,6 +883,9 @@ void MallocChecker::checkPostStmt(const
   State = ProcessZeroAllocation(C, CE, 0, State);
   State = ProcessZeroAllocation(C, CE, 1, State);
 } else if (FunI == II_free || FunI == II_g_free || FunI == II_kfree) {
+  if (suppressDeallocationsInSuspiciousContexts(CE, C))
+return;
+
   State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
 } else if (FunI == II_strdup || FunI == II_win_strdup ||
FunI == II_wcsdup || FunI == II_win_wcsdup) {
@@ -2532,6 +2541,35 @@ bool MallocChecker::isReleased(SymbolRef
   return (RS && RS->isReleased());
 }
 
+bool MallocChecker::suppressDeallocationsInSuspiciousContexts(
+const CallExpr *CE, CheckerContext ) const {
+  if (CE->getNumArgs() == 0)
+return false;
+
+  StringRef FunctionStr = "";
+  if (const auto *FD = dyn_cast(C.getStackFrame()->getDecl()))
+if (const Stmt *Body = FD->getBody())
+  if (Body->getBeginLoc().isValid())
+FunctionStr =
+Lexer::getSourceText(CharSourceRange::getTokenRange(
+ {FD->getBeginLoc(), Body->getBeginLoc()}),
+ C.getSourceManager(), C.getLangOpts());
+
+  // We do not model the Integer Set Library's retain-count based allocation.
+  if (!FunctionStr.contains("__isl_"))
+return false;
+
+  ProgramStateRef State = C.getState();
+
+  for (const Expr *Arg : CE->arguments())
+if (SymbolRef Sym = C.getSVal(Arg).getAsSymbol())
+  if (const RefState *RS = State->get(Sym))
+State = State->set(Sym, RefState::getEscaped(RS));
+
+  C.addTransition(State);
+  return true;
+}
+
 bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext ,
   const Stmt *S) const {
 
@@ -2833,7 +2871,6 @@ ProgramStateRef MallocChecker::checkPoin
 if (const RefState *RS = State->get(sym)) {
   if ((RS->isAllocated() || RS->isAllocatedOfSizeZero()) &&
   CheckRefState(RS)) {
-State = State->remove(sym);
 State = State->set(sym, RefState::getEscaped(RS));
   }
 }

Added: cfe/trunk/test/Analysis/retain-count-alloc.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-count-alloc.cpp?rev=366391=auto
==
--- cfe/trunk/test/Analysis/retain-count-alloc.cpp (added)
+++ cfe/trunk/test/Analysis/retain-count-alloc.cpp Wed Jul 17 17:03:55 2019
@@ -0,0 +1,37 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix.Malloc \
+// RUN:  -verify %s
+
+// expected-no-diagnostics: We do not model Integer Set Library's retain-count
+//  based allocation. If any of the parameters has an
+//  '__isl_' prefixed macro definition we escape every
+//  of them when we are about to 'free()' something.
+
+#define __isl_take
+#define __isl_keep
+
+struct Object { int Ref; };
+void free(void *);
+
+Object *copyObj(__isl_keep Object *O) {
+  O->Ref++;
+  return O;
+}
+
+void 

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D64680#1590619 , @NoQ wrote:

> Great, thanks!


Thanks for the review! I like that new name.


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

https://reviews.llvm.org/D64680



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Artem Dergachev via Phabricator via cfe-commits]

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

Great, thanks!




Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:363-364
 
+  /// Check whether we do not model the memory allocation.
+  bool isNotModeled(const CallExpr *CE, CheckerContext ) const;
+

One last thing: let's make it obvious what does the function do.
- It not only checks and returns a boolean value, it also adds transitions. It 
is very important to know that the function adds transitions so that to avoid 
accidental state splits.
- In this case we're talking about a deallocation rather than allocation.
- Technically, "not modeled" is not quite correct, as we *are* modeling it, 
just differently.

I suggest something like this:
```lang=c++
/// See if deallocation happens in a suspicious context. If so, escape the 
pointers
/// that otherwise would have been deallocated and return true.
bool suppressDeallocationsInSuspiciousContexts(const CallExpr *CE, 
CheckerContext ) const;
```


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

https://reviews.llvm.org/D64680



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

[PATCH] D64889: [OPENMP] getDSA(): handle loop control variables

[Princeton Ferro via Phabricator via cfe-commits]

Prince781 updated this revision to Diff 210459.
Prince781 added a comment.

Added a lit test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64889

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/loop_control_var_nested_task.cpp


Index: clang/test/OpenMP/loop_control_var_nested_task.cpp
===
--- /dev/null
+++ clang/test/OpenMP/loop_control_var_nested_task.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -fopenmp -emit-llvm -o - | FileCheck %s
+// CHECK: {{%struct}}.kmp_task_t_with_privates = type { 
{{.*%struct\.\.kmp_privates.t.*}} }
+// CHECK: define internal void 
@.omp_task_privates_map.({{.*%struct\.\.kmp_privates.t.*}})
+#define N 100
+int main(void) {
+// declare this variable outside, so that it will be shared on the outer 
parallel construct
+int i;
+int arr[N];
+
+#pragma omp parallel // shared(i) shared(arr)
+#pragma omp for // private(i) - i should be privatized because it is an 
iteration variable
+for (i = 0; i < N; i++) {
+#pragma omp task
+// CHECK: %.firstpriv.ptr.addr.i = alloca i32*, align 8
+// CHECK: {{%[0-9]+}} = load i32*, i32** %.firstpriv.ptr.addr.i, align 
8
+arr[i] = i;
+}
+}
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -424,6 +424,11 @@
   /// for-loops (from outer to inner).
   const LCDeclInfo isLoopControlVariable(const ValueDecl *D) const;
   /// Check if the specified variable is a loop control variable for
+  /// given region.
+  /// \return The index of the loop control variable in the list of associated
+  /// for-loops (from outer to inner).
+  const LCDeclInfo isLoopControlVariable(const ValueDecl *D, const 
SharingMapTy ) const;
+  /// Check if the specified variable is a loop control variable for
   /// parent region.
   /// \return The index of the loop control variable in the list of associated
   /// for-loops (from outer to inner).
@@ -946,11 +951,24 @@
   case DSA_none:
 return DVar;
   case DSA_unspecified:
+DVar.ImplicitDSALoc = Iter->DefaultAttrLoc;
+// OpenMP [2.9.1.1, Data-sharing Attribute Rules for Variables Referenced
+// in a Construct, implicitly determined]
+//  The loop iteration variable(s) in the associated for-loop(s) of a for 
or
+//  parallel for construct is (are) private.
+// OpenMP 5.0 includes taskloop and distribute directives
+if (!isOpenMPSimdDirective(DVar.DKind) &&
+isOpenMPLoopDirective(DVar.DKind) &&
+isLoopControlVariable(D, *Iter).first) {
+  DVar.CKind = OMPC_private;
+  // TODO: OpenMP 5.0: if (Dvar.DKind == OMPD_loop) DVar.CKind = 
OMPC_lastprivate;
+  return DVar;
+}
+
 // OpenMP [2.9.1.1, Data-sharing Attribute Rules for Variables Referenced
 // in a Construct, implicitly determined, p.2]
 //  In a parallel construct, if no default clause is present, these
 //  variables are shared.
-DVar.ImplicitDSALoc = Iter->DefaultAttrLoc;
 if (isOpenMPParallelDirective(DVar.DKind) ||
 isOpenMPTeamsDirective(DVar.DKind)) {
   DVar.CKind = OMPC_shared;
@@ -1018,8 +1036,13 @@
 const DSAStackTy::LCDeclInfo
 DSAStackTy::isLoopControlVariable(const ValueDecl *D) const {
   assert(!isStackEmpty() && "Data-sharing attributes stack is empty");
+  return isLoopControlVariable(D, getTopOfStack());
+}
+
+const DSAStackTy::LCDeclInfo
+DSAStackTy::isLoopControlVariable(const ValueDecl *D, const SharingMapTy 
) const {
   D = getCanonicalDecl(D);
-  const SharingMapTy  = getTopOfStack();
+  const SharingMapTy  = Region;
   auto It = StackElem.LCVMap.find(D);
   if (It != StackElem.LCVMap.end())
 return It->second;


Index: clang/test/OpenMP/loop_control_var_nested_task.cpp
===
--- /dev/null
+++ clang/test/OpenMP/loop_control_var_nested_task.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -fopenmp -emit-llvm -o - | FileCheck %s
+// CHECK: {{%struct}}.kmp_task_t_with_privates = type { {{.*%struct\.\.kmp_privates.t.*}} }
+// CHECK: define internal void @.omp_task_privates_map.({{.*%struct\.\.kmp_privates.t.*}})
+#define N 100
+int main(void) {
+// declare this variable outside, so that it will be shared on the outer parallel construct
+int i;
+int arr[N];
+
+#pragma omp parallel // shared(i) shared(arr)
+#pragma omp for // private(i) - i should be privatized because it is an iteration variable
+for (i = 0; i < N; i++) {
+#pragma omp task
+// CHECK: %.firstpriv.ptr.addr.i = alloca i32*, align 8
+// CHECK: {{%[0-9]+}} = load i32*, i32** %.firstpriv.ptr.addr.i, align 8
+arr[i] = i;
+}
+}
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- 

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2549-2552
+  FunctionStr = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(
+  {FD->getBeginLoc(), FD->getBody()->getBeginLoc()}),
+  C.getSourceManager(), C.getLangOpts());

NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > I'm slightly worried that it'll crash when `free()` is being called from 
> > > within a body farm.
> > > 
> > > For now it probably cannot happen because none of the bodyfarmed 
> > > functions can call `free()` directly, but i'd anyway rather add a check 
> > > that the source locations we're taking are valid.
> > Oh, I missed that, thanks! I wanted to check for everything, yes.
> I think this is not fixed yet. I'm thinking of something like `if 
> (!Body->getBeginLoc().isValid()) { ... }`.
Ugh, silly mistake, thanks!


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

https://reviews.llvm.org/D64680



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 210458.
Charusso marked 4 inline comments as done.
Charusso added a comment.

- More fix.


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

https://reviews.llvm.org/D64680

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/test/Analysis/retain-count-alloc.cpp

Index: clang/test/Analysis/retain-count-alloc.cpp
===
--- /dev/null
+++ clang/test/Analysis/retain-count-alloc.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix.Malloc \
+// RUN:  -verify %s
+
+// expected-no-diagnostics: We do not model Integer Set Library's retain-count
+//  based allocation. If any of the parameters has an
+//  '__isl_' prefixed macro definition we escape every
+//  of them when we are about to 'free()' something.
+
+#define __isl_take
+#define __isl_keep
+
+struct Object { int Ref; };
+void free(void *);
+
+Object *copyObj(__isl_keep Object *O) {
+  O->Ref++;
+  return O;
+}
+
+void freeObj(__isl_take Object *O) {
+  if (--O->Ref > 0)
+return;
+
+  free(O); // Here we notice that the parameter contains '__isl_', escape it.
+}
+
+void useAfterFree(__isl_take Object *A) {
+  if (!A)
+return;
+
+  Object *B = copyObj(A);
+  freeObj(B);
+
+  A->Ref = 13;
+  // no-warning: 'Use of memory after it is freed' was here.
+}
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/ParentMap.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -359,6 +360,9 @@
   /// Check if the memory associated with this symbol was released.
   bool isReleased(SymbolRef Sym, CheckerContext ) const;
 
+  /// Check whether we do not model the memory allocation.
+  bool isNotModeled(const CallExpr *CE, CheckerContext ) const;
+
   bool checkUseAfterFree(SymbolRef Sym, CheckerContext , const Stmt *S) const;
 
   void checkUseZeroAllocated(SymbolRef Sym, CheckerContext ,
@@ -877,6 +881,9 @@
   State = ProcessZeroAllocation(C, CE, 0, State);
   State = ProcessZeroAllocation(C, CE, 1, State);
 } else if (FunI == II_free || FunI == II_g_free || FunI == II_kfree) {
+  if (isNotModeled(CE, C))
+return;
+
   State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
 } else if (FunI == II_strdup || FunI == II_win_strdup ||
FunI == II_wcsdup || FunI == II_win_wcsdup) {
@@ -2532,6 +2539,34 @@
   return (RS && RS->isReleased());
 }
 
+bool MallocChecker::isNotModeled(const CallExpr *CE, CheckerContext ) const {
+  if (CE->getNumArgs() == 0)
+return false;
+
+  StringRef FunctionStr = "";
+  if (const auto *FD = dyn_cast(C.getStackFrame()->getDecl()))
+if (const Stmt *Body = FD->getBody())
+  if (Body->getBeginLoc().isValid())
+FunctionStr =
+Lexer::getSourceText(CharSourceRange::getTokenRange(
+ {FD->getBeginLoc(), Body->getBeginLoc()}),
+ C.getSourceManager(), C.getLangOpts());
+
+  // We do not model the Integer Set Library's retain-count based allocation.
+  if (!FunctionStr.contains("__isl_"))
+return false;
+
+  ProgramStateRef State = C.getState();
+
+  for (const Expr *Arg : CE->arguments())
+if (SymbolRef Sym = C.getSVal(Arg).getAsSymbol())
+  if (const RefState *RS = State->get(Sym))
+State = State->set(Sym, RefState::getEscaped(RS));
+
+  C.addTransition(State);
+  return true;
+}
+
 bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext ,
   const Stmt *S) const {
 
@@ -2833,7 +2868,6 @@
 if (const RefState *RS = State->get(sym)) {
   if ((RS->isAllocated() || RS->isAllocatedOfSizeZero()) &&
   CheckRefState(RS)) {
-State = State->remove(sym);
 State = State->set(sym, RefState::getEscaped(RS));
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

[Artem Dergachev via Phabricator via cfe-commits]

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

Looks great to me once @Szelethus's comments are addressed. Thanks!!




Comment at: lib/StaticAnalyzer/Checkers/Yaml.h:34
+std::string(ConfigFile) + "'");
+return {};
+  }

I suggest `None` for being more explicit and readable.



Comment at: test/Analysis/taint-generic.c:24
+// CHECK-INVALID-FILE-SAME:
'alpha.security.taint.TaintPropagation:Config',
+// CHECK-INVALID-FILE-SAME:that expects a valid filename
 

Szelethus wrote:
> Could you please add the rest of the error message?
I'd rather remove the rest of the error message. There's no need to duplicate 
something that the user has already written on the command line.

Or do we think like \escapes?


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

https://reviews.llvm.org/D59555



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2549-2552
+  FunctionStr = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(
+  {FD->getBeginLoc(), FD->getBody()->getBeginLoc()}),
+  C.getSourceManager(), C.getLangOpts());

Charusso wrote:
> NoQ wrote:
> > I'm slightly worried that it'll crash when `free()` is being called from 
> > within a body farm.
> > 
> > For now it probably cannot happen because none of the bodyfarmed functions 
> > can call `free()` directly, but i'd anyway rather add a check that the 
> > source locations we're taking are valid.
> Oh, I missed that, thanks! I wanted to check for everything, yes.
I think this is not fixed yet. I'm thinking of something like `if 
(!Body->getBeginLoc().isValid()) { ... }`.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2569
+  if (const RefState *RS = State->get(Sym)) {
+State = State->remove(Sym);
+State = State->set(Sym, RefState::getEscaped(RS));

Charusso wrote:
> NoQ wrote:
> > `remove` is unnecessary, we overwrite it anyway.
> I believe in so as well, just the official code base has this semantic. I 
> have rewritten that, see below in `checkPointerEscapeAux()`.
Yeah, and the official codebase is wrong :)



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2570
+
+  if (IsEscaped)
+C.addTransition(State);

The check is unnecessary. `addTransition` automatically takes care of the "no 
changes have happened" case and throws away the transition.

Moreover, in the current patch you aren't really checking if any changes have 
happened: the symbol may already be escaped, so in this case the flag is set to 
true but the state remains the same.


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

https://reviews.llvm.org/D64680



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

[PATCH] D64272: [analyzer] Note last writes to a condition only in a nested stackframe

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1447-1454
   // If we have an expression that provided the value, try to track where it
   // came from.
   if (InitE) {
 if (!IsParam)
   InitE = InitE->IgnoreParenCasts();
 
 trackExpressionValue(StoreSite, InitE, BR, EnableNullFPSuppression, TKind);

Yeah, interesting, i guess we should also not track the value further. 
Otherwise it'd be weird that one piece in the middle is missing but everything 
else is still there.


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

https://reviews.llvm.org/D64272



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

[PATCH] D64775: [Format/ObjC] Avoid breaking between unary operators and ObjC method invocations

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton marked an inline comment as done.
benhamilton added a comment.

Any more thoughts on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64775



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

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:81
+
+bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
+   const Expr *E, BugReport ,

Szelethus wrote:
> xazax.hun wrote:
> > Do we need this overload? What about a default argument?
> I don't want to expose this functionality outside of this file. I don't 
> insist though!
I think we'll have to expose it because different checkers would want different 
tracking kinds for their values. For instance, null dereference checker could 
really use condition tracking as its default tracking mode.


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

https://reviews.llvm.org/D64270



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

lildmh added a comment.

In D59474#1589464 , @ABataev wrote:

> In D59474#1561161 , @lildmh wrote:
>
> > Change the type of size from `size_t` to `int64_t`, and rebase
>
>
> Lingda, could you rebase it again? Thanks.


Sure, I'll do it next week, since I'm on vacation and don't have access to my 
desktop.


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

https://reviews.llvm.org/D59474



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

[PATCH] D64843: hwasan: Initialize the pass only once.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

The instrumentation for globals is coming in a separate patch.

It seems more important to achieve implementation simplicity for the sanitizer 
passes rather than data locality because they make fundamental changes to the 
IR that touch the whole module (unlike other passes that are meant to preserve 
semantics). If the new PM is extended to allow whole-module changes within a 
function pass, I think the balance may change in favour of making this a 
function pass.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64843



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

[PATCH] D64843: hwasan: Initialize the pass only once.

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added subscribers: fedor.sergeev, philip.pfaffe, leonardchan.
leonardchan added a comment.

cc: @fedor.sergeev @philip.pfaffe on this

I think one of the things we want in the new PM is the data locality that we 
see from iterating only specific IR units (functions in this case). This was 
brought up before when porting ASan (D54337 ). 
In this patch it also looks like the pass still goes over functions only. If 
there are other globals that this should pass over, could you add/update a test 
to show that?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64843



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

[PATCH] D64889: [OPENMP] getDSA(): handle loop control variables

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Needs a test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64889



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

[PATCH] D64889: [OPENMP] getDSA(): handle loop control variables

[Princeton Ferro via Phabricator via cfe-commits]

Prince781 created this revision.
Prince781 added reviewers: ABataev, rsmith.
Prince781 added projects: clang, OpenMP.
Herald added subscribers: cfe-commits, jdoerfert, guansong.

The following example compiles incorrectly since at least clang 8.0.0:

  #include 
  #include 
  
  #define N 100
  
  int main(void) {
  int i;
  int arr[N];
  
  #pragma omp parallel // shared(i) shared(arr)
  #pragma omp for // private(i)
  for (i = 0; i < N; i++) {
  #pragma omp task // firstprivate(i) shared(arr)
  {
  printf("[thread %2d] i = %d\n", omp_get_thread_num(), i);
  arr[i] = i;
  }
  }
  
  for (i = 0; i < N; i++) {
  if (arr[i] != i) {
  fprintf(stderr, "FAIL: arr[%d] == %d\n", i, arr[i]);
  }
  }
  }

The iteration variable, `i`, should become `private` at the `omp for` construct 
and then become implicit `firstprivate` within the task region. What happens 
instead is that `i` is never privatized within the task construct. As the task 
construct is parsed, when a reference to `i` is determined, the implicit 
data-sharing attributes are computed incorrectly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64889

Files:
  clang/lib/Sema/SemaOpenMP.cpp


Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -424,6 +424,11 @@
   /// for-loops (from outer to inner).
   const LCDeclInfo isLoopControlVariable(const ValueDecl *D) const;
   /// Check if the specified variable is a loop control variable for
+  /// given region.
+  /// \return The index of the loop control variable in the list of associated
+  /// for-loops (from outer to inner).
+  const LCDeclInfo isLoopControlVariable(const ValueDecl *D, const 
SharingMapTy ) const;
+  /// Check if the specified variable is a loop control variable for
   /// parent region.
   /// \return The index of the loop control variable in the list of associated
   /// for-loops (from outer to inner).
@@ -946,11 +951,24 @@
   case DSA_none:
 return DVar;
   case DSA_unspecified:
+DVar.ImplicitDSALoc = Iter->DefaultAttrLoc;
+// OpenMP [2.9.1.1, Data-sharing Attribute Rules for Variables Referenced
+// in a Construct, implicitly determined]
+//  The loop iteration variable(s) in the associated for-loop(s) of a for 
or
+//  parallel for construct is (are) private.
+// OpenMP 5.0 includes taskloop and distribute directives
+if (!isOpenMPSimdDirective(DVar.DKind) &&
+isOpenMPLoopDirective(DVar.DKind) &&
+isLoopControlVariable(D, *Iter).first) {
+  DVar.CKind = OMPC_private;
+  // TODO: OpenMP 5.0: if (Dvar.DKind == OMPD_loop) DVar.CKind = 
OMPC_lastprivate;
+  return DVar;
+}
+
 // OpenMP [2.9.1.1, Data-sharing Attribute Rules for Variables Referenced
 // in a Construct, implicitly determined, p.2]
 //  In a parallel construct, if no default clause is present, these
 //  variables are shared.
-DVar.ImplicitDSALoc = Iter->DefaultAttrLoc;
 if (isOpenMPParallelDirective(DVar.DKind) ||
 isOpenMPTeamsDirective(DVar.DKind)) {
   DVar.CKind = OMPC_shared;
@@ -1018,8 +1036,13 @@
 const DSAStackTy::LCDeclInfo
 DSAStackTy::isLoopControlVariable(const ValueDecl *D) const {
   assert(!isStackEmpty() && "Data-sharing attributes stack is empty");
+  return isLoopControlVariable(D, getTopOfStack());
+}
+
+const DSAStackTy::LCDeclInfo
+DSAStackTy::isLoopControlVariable(const ValueDecl *D, const SharingMapTy 
) const {
   D = getCanonicalDecl(D);
-  const SharingMapTy  = getTopOfStack();
+  const SharingMapTy  = Region;
   auto It = StackElem.LCVMap.find(D);
   if (It != StackElem.LCVMap.end())
 return It->second;


Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -424,6 +424,11 @@
   /// for-loops (from outer to inner).
   const LCDeclInfo isLoopControlVariable(const ValueDecl *D) const;
   /// Check if the specified variable is a loop control variable for
+  /// given region.
+  /// \return The index of the loop control variable in the list of associated
+  /// for-loops (from outer to inner).
+  const LCDeclInfo isLoopControlVariable(const ValueDecl *D, const SharingMapTy ) const;
+  /// Check if the specified variable is a loop control variable for
   /// parent region.
   /// \return The index of the loop control variable in the list of associated
   /// for-loops (from outer to inner).
@@ -946,11 +951,24 @@
   case DSA_none:
 return DVar;
   case DSA_unspecified:
+DVar.ImplicitDSALoc = Iter->DefaultAttrLoc;
+// OpenMP [2.9.1.1, Data-sharing Attribute Rules for Variables Referenced
+// in a Construct, implicitly determined]
+//  The loop iteration variable(s) in the associated for-loop(s) of a for or
+

[PATCH] D61027: Fix crash on switch conditions of non-integer types in templates

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews added a comment.

ping

I just realized I modified the summary when I uploaded a new patch, but did not 
add a comment about the changes I made. Based on feedback from the bug report 
(https://bugs.llvm.org/show_bug.cgi?id=40982), I changed my approach for this 
crash. This patch changes the type dependency of the member which causes the 
crash.

Currently, when member expressions are created, their type dependency is set 
based on the class it is part of and not its own type.  I don't quite 
understand why, and changing it broke a whole lot of lit tests. So, in this 
patch I only attempted to modify the type-dependency of 'members of current 
instantiation' to set it based on it's type as opposed to the containing 
classes' type.


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

https://reviews.llvm.org/D61027



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

[PATCH] D61452: [WebAssembly] Always include /lib in library path

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

In D61452#1488789 , @sunfish wrote:

> If "$sysroot/lib" ends up coming to mean "wasm32" because people come to 
> depend on that, then wasm64 may end up needing to be different in a 
> gratuitous way, which I'd like to avoid.
>
> I'd like to keep our sysroots tidy when we can. If some libraries are 
> installed in `lib/wasm32-wasi` and others `lib` for no reason other than 
> build script inertia, that's untidy.
>
> It's not an absolute for me, but I am inclined to see if we can understand 
> the need better first. Single-arch sysroots are possible either way, for 
> example.


I think are you wrong here for a couple of reasons.

Firstly, we already look in both the multi-arch directory for include files:

  sysroot/include/wasm32-wasi
  sysroot/include

Amd for C++ includes:

  sysroot/include/wasm32-wasi/c++/v1
  sysroot/include/c++/v1

This allows for us to fall back from arch-specific to generic headers as 
needed.  The same can be true of libraries.   Not all libraries contains 
compiled code.  `.so` files can also be linker scripts that reference other 
libraries in which case they can be arch-neutral.

Secondly, not everyone is going to want to use a multi-arch sysroot.  For 
example I would argue that the wasi SDK would make more sense being single arch 
since it has exactly one purpose.  Its only when installing wasi into an 
existing system that one really needs to be multi-arch.  In any case we should 
not dictate this.  We've already have one person trying to build a wasi-sdk 
without using multi-arch paths.  I'm not sure what you mean by "single arch 
sysroots are possible either way".  The point of a single arch sysroot would be 
avoid the unnecessary extra path components and I thats exactly what this 
change is allowing for.

Thirdly, its what the linux driver does, which is the example we are following 
of how to build a multi-arch toolchain.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61452



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

[PATCH] D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 210431.
plotfi marked 10 inline comments as done.
plotfi added a comment.

Updated to address @aaron.ballman 's feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64098

Files:
  clang/include/clang/Driver/Types.def
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Types.cpp

Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -9,8 +9,9 @@
 #include "clang/Driver/Types.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/SmallVector.h"
 #include 
-#include 
+#include 
 
 using namespace clang::driver;
 using namespace clang::driver::types;
@@ -20,11 +21,12 @@
   const char *Flags;
   const char *TempSuffix;
   ID PreprocessedType;
+  const llvm::SmallVector Phases;
 };
 
 static const TypeInfo TypeInfos[] = {
-#define TYPE(NAME, ID, PP_TYPE, TEMP_SUFFIX, FLAGS) \
-  { NAME, FLAGS, TEMP_SUFFIX, TY_##PP_TYPE, },
+#define TYPE(NAME, ID, PP_TYPE, TEMP_SUFFIX, FLAGS, ...) \
+  { NAME, FLAGS, TEMP_SUFFIX, TY_##PP_TYPE, { __VA_ARGS__ }, },
 #include "clang/Driver/Types.def"
 #undef TYPE
 };
@@ -264,6 +266,8 @@
 }
 
 // FIXME: Why don't we just put this list in the defs file, eh.
+// FIXME: The list is now in Types.def but for now this function will verify
+//the old behavior and a subsequent change will delete most of the body.
 void types::getCompilationPhases(ID Id, llvm::SmallVectorImpl ) {
   if (Id != TY_Object) {
 if (getPreprocessedType(Id) != TY_INVALID) {
@@ -286,6 +290,21 @@
   if (!onlyPrecompileType(Id)) {
 P.push_back(phases::Link);
   }
+
+  // Check that the static Phase list matches.
+  // TODO: These will be deleted.
+  const llvm::SmallVectorImpl  = getInfo(Id).Phases;
+  assert(Phases.size() == P.size() &&
+ std::equal(Phases.begin(), Phases.end(), P.begin()) &&
+ "Invalid phase or size");
+
+  // TODO: This function is still being used to assert that the phase list in
+  //   Types.def is correct. Everything above this comment will be removed
+  //   in a subsequent NFC commit.
+  P.clear();
+  for (auto Phase : getInfo(Id).Phases)
+P.push_back(Phase);
+
   assert(0 < P.size() && "Not enough phases in list");
   assert(P.size() <= phases::MaxNumberOfPhases && "Too many phases in list");
 }
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2223,7 +2223,7 @@
   /// Builder interface. It doesn't build anything or keep any state.
   class DeviceActionBuilder {
   public:
-typedef llvm::SmallVector PhasesTy;
+typedef const llvm::SmallVectorImpl PhasesTy;
 
 enum ActionBuilderReturnCode {
   // The builder acted successfully on the current action.
@@ -3237,13 +3237,14 @@
   HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
   ActionList LinkerInputs;
 
-  llvm::SmallVector PL;
+  unsigned LastPLSize = 0;
   for (auto  : Inputs) {
 types::ID InputType = I.first;
 const Arg *InputArg = I.second;
 
-PL.clear();
+llvm::SmallVector PL;
 types::getCompilationPhases(InputType, PL);
+LastPLSize = PL.size();
 
 // If the first step comes after the final phase we are doing as part of
 // this compilation, warn the user about it.
@@ -3309,9 +3310,7 @@
 if (OffloadBuilder.addHostDependenceToDeviceActions(Current, InputArg))
   break;
 
-for (SmallVectorImpl::iterator i = PL.begin(), e = PL.end();
- i != e; ++i) {
-  phases::ID Phase = *i;
+for (phases::ID Phase : PL) {
 
   // We are done if this step is past what the user requested.
   if (Phase > FinalPhase)
@@ -3325,7 +3324,7 @@
 
   // Queue linker inputs.
   if (Phase == phases::Link) {
-assert((i + 1) == e && "linking must be final compilation step.");
+assert(Phase == PL.back() && "linking must be final compilation step.");
 LinkerInputs.push_back(Current);
 Current = nullptr;
 break;
@@ -3382,7 +3381,8 @@
 
   // If we are linking, claim any options which are obviously only used for
   // compilation.
-  if (FinalPhase == phases::Link && PL.size() == 1) {
+  // FIXME: Understand why the last Phase List length is used here.
+  if (FinalPhase == phases::Link && LastPLSize == 1) {
 Args.ClaimAllArgs(options::OPT_CompileOnly_Group);
 Args.ClaimAllArgs(options::OPT_cl_compile_Group);
   }
Index: clang/include/clang/Driver/Types.h
===
--- clang/include/clang/Driver/Types.h
+++ clang/include/clang/Driver/Types.h
@@ -20,7 +20,7 @@
 namespace types {
   enum ID {
 TY_INVALID,
-#define TYPE(NAME, ID, PP_TYPE, TEMP_SUFFIX, FLAGS) TY_##ID,
+#define TYPE(NAME, ID, PP_TYPE, 

[PATCH] D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:2217
   public:
-typedef llvm::SmallVector PhasesTy;
+typedef const std::vector PhasesTy;
 

aaron.ballman wrote:
> Why are you changing this to an STL type?
I changed it because it didn't look like you can return a SmallVector, but I 
just changed it back to the old SmallVector function calling style.



Comment at: clang/lib/Driver/Driver.cpp:3236
 
-PL.clear();
-types::getCompilationPhases(InputType, PL);
+const std::vector PL = types::getCompilationPhases(InputType);
+LastPLSize = PL.size();

aaron.ballman wrote:
> You could use a `const &` here and avoid the copy. Either that, or drop the 
> `const` (it's not a common pattern in the code base).
Going to keep the copy. I want to eventually merge getCompilationPhases and 
getFinalPhase into the same function. 



Comment at: clang/lib/Driver/Types.cpp:301-303
+  assert(Phases.size() == P.size() && "Invalid size.");
+  for (unsigned i = 0; i < Phases.size(); i++)
+assert(Phases[i] == P[i] && "Invalid Phase");

aaron.ballman wrote:
> You can replace all three lines by:
> `assert(std::equal(Phases.begin(), Phases.end(), P.begin(), P.end()) && 
> "Invalid phase or size");`
Nice!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64098



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

[PATCH] D64780: Disallow most calling convention attributes on PS4.

[Sunil Srivastava via Phabricator via cfe-commits]

Sunil_Srivastava added a comment.

Pre-commit to change the wording and ID of warn_cconv_ignore has gone in in 
r366368.

I will be shortly updating this review to account for that, and for other 
points raised here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64780



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

[PATCH] D64843: hwasan: Initialize the pass only once.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366379: hwasan: Initialize the pass only once. (authored by 
pcc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D64843?vs=210227=210429#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64843

Files:
  cfe/trunk/lib/CodeGen/BackendUtil.cpp
  llvm/trunk/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
  llvm/trunk/lib/Passes/PassRegistry.def
  llvm/trunk/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/trunk/test/Instrumentation/HWAddressSanitizer/basic.ll

Index: llvm/trunk/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/trunk/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/trunk/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -277,12 +277,22 @@
 
   StringRef getPassName() const override { return "HWAddressSanitizer"; }
 
+  bool doInitialization(Module ) override {
+HWASan = llvm::make_unique(M, CompileKernel, Recover);
+return true;
+  }
+
   bool runOnFunction(Function ) override {
-HWAddressSanitizer HWASan(*F.getParent(), CompileKernel, Recover);
-return HWASan.sanitizeFunction(F);
+return HWASan->sanitizeFunction(F);
+  }
+
+  bool doFinalization(Module ) override {
+HWASan.reset();
+return false;
   }
 
 private:
+  std::unique_ptr HWASan;
   bool CompileKernel;
   bool Recover;
 };
@@ -309,10 +319,13 @@
 HWAddressSanitizerPass::HWAddressSanitizerPass(bool CompileKernel, bool Recover)
 : CompileKernel(CompileKernel), Recover(Recover) {}
 
-PreservedAnalyses HWAddressSanitizerPass::run(Function ,
-  FunctionAnalysisManager ) {
-  HWAddressSanitizer HWASan(*F.getParent(), CompileKernel, Recover);
-  if (HWASan.sanitizeFunction(F))
+PreservedAnalyses HWAddressSanitizerPass::run(Module ,
+  ModuleAnalysisManager ) {
+  HWAddressSanitizer HWASan(M, CompileKernel, Recover);
+  bool Modified = false;
+  for (Function  : M)
+Modified |= HWASan.sanitizeFunction(F);
+  if (Modified)
 return PreservedAnalyses::none();
   return PreservedAnalyses::all();
 }
Index: llvm/trunk/lib/Passes/PassRegistry.def
===
--- llvm/trunk/lib/Passes/PassRegistry.def
+++ llvm/trunk/lib/Passes/PassRegistry.def
@@ -55,6 +55,8 @@
 MODULE_PASS("globalopt", GlobalOptPass())
 MODULE_PASS("globalsplit", GlobalSplitPass())
 MODULE_PASS("hotcoldsplit", HotColdSplittingPass())
+MODULE_PASS("hwasan", HWAddressSanitizerPass(false, false))
+MODULE_PASS("khwasan", HWAddressSanitizerPass(true, true))
 MODULE_PASS("inferattrs", InferFunctionAttrsPass())
 MODULE_PASS("insert-gcov-profiling", GCOVProfilerPass())
 MODULE_PASS("instrorderfile", InstrOrderFilePass())
@@ -240,8 +242,6 @@
 FUNCTION_PASS("transform-warning", WarnMissedTransformationsPass())
 FUNCTION_PASS("asan", AddressSanitizerPass(false, false, false))
 FUNCTION_PASS("kasan", AddressSanitizerPass(true, false, false))
-FUNCTION_PASS("hwasan", HWAddressSanitizerPass(false, false))
-FUNCTION_PASS("khwasan", HWAddressSanitizerPass(true, true))
 FUNCTION_PASS("msan", MemorySanitizerPass({}))
 FUNCTION_PASS("kmsan", MemorySanitizerPass({0, false, /*Kernel=*/true}))
 FUNCTION_PASS("tsan", ThreadSanitizerPass())
Index: llvm/trunk/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
===
--- llvm/trunk/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
+++ llvm/trunk/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h
@@ -26,7 +26,7 @@
 public:
   explicit HWAddressSanitizerPass(bool CompileKernel = false,
   bool Recover = false);
-  PreservedAnalyses run(Function , FunctionAnalysisManager );
+  PreservedAnalyses run(Module , ModuleAnalysisManager );
 
 private:
   bool CompileKernel;
Index: llvm/trunk/test/Instrumentation/HWAddressSanitizer/basic.ll
===
--- llvm/trunk/test/Instrumentation/HWAddressSanitizer/basic.ll
+++ llvm/trunk/test/Instrumentation/HWAddressSanitizer/basic.ll
@@ -6,10 +6,10 @@
 ; RUN: opt < %s -hwasan -hwasan-recover=1 -hwasan-mapping-offset=0 -S | FileCheck %s --check-prefixes=CHECK,RECOVER,RECOVER-ZERO-BASED-SHADOW
 
 ; Ensure than hwasan runs with the new PM pass
-; RUN: opt < %s -passes='function(hwasan)' -hwasan-recover=0 -hwasan-with-ifunc=1 -hwasan-with-tls=0 -S | FileCheck %s --check-prefixes=CHECK,ABORT,ABORT-DYNAMIC-SHADOW
-; RUN: opt < %s -passes='function(hwasan)' -hwasan-recover=1 -hwasan-with-ifunc=1 -hwasan-with-tls=0 -S | FileCheck %s --check-prefixes=CHECK,RECOVER,RECOVER-DYNAMIC-SHADOW
-; RUN: opt < %s -passes='function(hwasan)' -hwasan-recover=0 

r366379 - hwasan: Initialize the pass only once.

[Peter Collingbourne via cfe-commits]

Author: pcc
Date: Wed Jul 17 14:45:19 2019
New Revision: 366379

URL: http://llvm.org/viewvc/llvm-project?rev=366379=rev
Log:
hwasan: Initialize the pass only once.

This will let us instrument globals during initialization. This required
making the new PM pass a module pass, which should still provide access to
analyses via the ModuleAnalysisManager.

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

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=366379=366378=366379=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Wed Jul 17 14:45:19 2019
@@ -967,17 +967,6 @@ static void addSanitizersAtO0(ModulePass
   if (LangOpts.Sanitize.has(SanitizerKind::Thread)) {
 MPM.addPass(createModuleToFunctionPassAdaptor(ThreadSanitizerPass()));
   }
-
-  if (LangOpts.Sanitize.has(SanitizerKind::HWAddress)) {
-bool Recover = CodeGenOpts.SanitizeRecover.has(SanitizerKind::HWAddress);
-MPM.addPass(createModuleToFunctionPassAdaptor(
-HWAddressSanitizerPass(/*CompileKernel=*/false, Recover)));
-  }
-
-  if (LangOpts.Sanitize.has(SanitizerKind::KernelHWAddress)) {
-MPM.addPass(createModuleToFunctionPassAdaptor(
-HWAddressSanitizerPass(/*CompileKernel=*/true, /*Recover=*/true)));
-  }
 }
 
 /// A clean version of `EmitAssembly` that uses the new pass manager.
@@ -1176,23 +1165,6 @@ void EmitAssemblyHelper::EmitAssemblyWit
   UseOdrIndicator));
 });
   }
-  if (LangOpts.Sanitize.has(SanitizerKind::HWAddress)) {
-bool Recover =
-CodeGenOpts.SanitizeRecover.has(SanitizerKind::HWAddress);
-PB.registerOptimizerLastEPCallback(
-[Recover](FunctionPassManager ,
-  PassBuilder::OptimizationLevel Level) {
-  FPM.addPass(HWAddressSanitizerPass(
-  /*CompileKernel=*/false, Recover));
-});
-  }
-  if (LangOpts.Sanitize.has(SanitizerKind::KernelHWAddress)) {
-PB.registerOptimizerLastEPCallback(
-[](FunctionPassManager , PassBuilder::OptimizationLevel Level) 
{
-  FPM.addPass(HWAddressSanitizerPass(
-  /*CompileKernel=*/true, /*Recover=*/true));
-});
-  }
   if (Optional Options = getGCOVOptions(CodeGenOpts))
 PB.registerPipelineStartEPCallback([Options](ModulePassManager ) {
   MPM.addPass(GCOVProfilerPass(*Options));
@@ -1219,6 +1191,16 @@ void EmitAssemblyHelper::EmitAssemblyWit
   }
 }
 
+if (LangOpts.Sanitize.has(SanitizerKind::HWAddress)) {
+  bool Recover = CodeGenOpts.SanitizeRecover.has(SanitizerKind::HWAddress);
+  MPM.addPass(HWAddressSanitizerPass(
+  /*CompileKernel=*/false, Recover));
+}
+if (LangOpts.Sanitize.has(SanitizerKind::KernelHWAddress)) {
+  MPM.addPass(HWAddressSanitizerPass(
+  /*CompileKernel=*/true, /*Recover=*/true));
+}
+
 if (CodeGenOpts.OptimizationLevel == 0)
   addSanitizersAtO0(MPM, TargetTriple, LangOpts, CodeGenOpts);
   }


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

[PATCH] D63852: [Clang] Move assembler into a separate file

[Ayke via Phabricator via cfe-commits]

aykevl added a comment.

ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63852



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

[PATCH] D64883: Add new warning -Walloca for use of builtin alloca function

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

Thanks for this!

Mostly just nitpicking the warning's wording. :)




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2776
+def warn_alloca : Warning<
+  "use of builtin function %0">,
+  InGroup>, DefaultIgnore;

nit: I'd just say "use of function '%0'" here; "builtin" doesn't really add 
much.

I also wonder if we should be saying anything more than "we found a use of this 
function." Looks like GCC doesn't (https://godbolt.org/z/sYs_8G), but since 
this warning is sort of opinionated in itself, might it be better to expand 
this to "use of '%0' is discouraged"?

WDYT, Aaron?


Repository:
  rC Clang

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

https://reviews.llvm.org/D64883



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

[PATCH] D64883: Add new warning -Walloca for use of builtin alloca function

[Elaina Guan via Phabricator via cfe-commits]

ziyig created this revision.
ziyig added reviewers: gbiv, aaron.ballman.
Herald added a reviewer: george.burgess.iv.
Herald added subscribers: cfe-commits, kristina.
Herald added a project: clang.

Add new warning -Walloca for use of builtin alloca function.

Also warns the use of __builtin_alloca_with_align. GCC has this warning, and 
we'd like to have this for compatibility.


Repository:
  rC Clang

https://reviews.llvm.org/D64883

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/warn-alloca.c


Index: clang/test/Sema/warn-alloca.c
===
--- /dev/null
+++ clang/test/Sema/warn-alloca.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Walloca %s
+
+void test1(int a) {
+  __builtin_alloca(a); // expected-warning {{use of builtin function 
__builtin_alloca}}
+}
+
+void test2(int a) {
+  __builtin_alloca_with_align(a, 32); // expected-warning {{use of builtin 
function __builtin_alloca_with_align}}
+}
+
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -1169,6 +1169,10 @@
   case Builtin::BI__builtin_alloca_with_align:
 if (SemaBuiltinAllocaWithAlign(TheCall))
   return ExprError();
+LLVM_FALLTHROUGH;
+  case Builtin::BI__builtin_alloca:
+Diag(TheCall->getBeginLoc(), diag::warn_alloca)
+<< Context.BuiltinInfo.getName(BuiltinID);
 break;
   case Builtin::BI__assume:
   case Builtin::BI__builtin_assume:
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2772,6 +2772,10 @@
 def err_cannot_find_suitable_accessor : Error<
   "cannot find suitable %select{getter|setter}0 for property %1">;
 
+def warn_alloca : Warning<
+  "use of builtin function %0">,
+  InGroup>, DefaultIgnore;
+
 def warn_alloca_align_alignof : Warning<
   "second argument to __builtin_alloca_with_align is supposed to be in bits">,
   InGroup>;


Index: clang/test/Sema/warn-alloca.c
===
--- /dev/null
+++ clang/test/Sema/warn-alloca.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Walloca %s
+
+void test1(int a) {
+  __builtin_alloca(a); // expected-warning {{use of builtin function __builtin_alloca}}
+}
+
+void test2(int a) {
+  __builtin_alloca_with_align(a, 32); // expected-warning {{use of builtin function __builtin_alloca_with_align}}
+}
+
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -1169,6 +1169,10 @@
   case Builtin::BI__builtin_alloca_with_align:
 if (SemaBuiltinAllocaWithAlign(TheCall))
   return ExprError();
+LLVM_FALLTHROUGH;
+  case Builtin::BI__builtin_alloca:
+Diag(TheCall->getBeginLoc(), diag::warn_alloca)
+<< Context.BuiltinInfo.getName(BuiltinID);
 break;
   case Builtin::BI__assume:
   case Builtin::BI__builtin_assume:
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2772,6 +2772,10 @@
 def err_cannot_find_suitable_accessor : Error<
   "cannot find suitable %select{getter|setter}0 for property %1">;
 
+def warn_alloca : Warning<
+  "use of builtin function %0">,
+  InGroup>, DefaultIgnore;
+
 def warn_alloca_align_alignof : Warning<
   "second argument to __builtin_alloca_with_align is supposed to be in bits">,
   InGroup>;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r366368 - Renamed and changed the wording of warn_cconv_ignored

[Sunil Srivastava via cfe-commits]

Author: ssrivastava
Date: Wed Jul 17 13:41:26 2019
New Revision: 366368

URL: http://llvm.org/viewvc/llvm-project?rev=366368=rev
Log:
Renamed and changed the wording of warn_cconv_ignored

As discussed in D64780 the wording of this warning message is being
changed to say 'is not supported' instead of 'ignored', and the
diag ID itself is being changed to warn_cconv_not_supported.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/CodeGen/aarch64-vpcs.c
cfe/trunk/test/Frontend/macro_defined_type.cpp
cfe/trunk/test/Sema/callingconv-iamcu.c
cfe/trunk/test/Sema/callingconv.c
cfe/trunk/test/Sema/mrtd.c
cfe/trunk/test/Sema/pr25786.c
cfe/trunk/test/Sema/stdcall-fastcall-x64.c
cfe/trunk/test/SemaCUDA/cuda-inherits-calling-conv.cu
cfe/trunk/test/SemaCXX/borland-extensions.cpp
cfe/trunk/test/SemaCXX/cxx11-gnu-attrs.cpp
cfe/trunk/test/SemaCXX/decl-microsoft-call-conv.cpp
cfe/trunk/test/SemaCXX/virtual-override-x64.cpp
cfe/trunk/test/SemaTemplate/instantiate-function-params.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=366368=366367=366368=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jul 17 13:41:26 
2019
@@ -2957,8 +2957,8 @@ def err_attribute_vecreturn_only_pod_rec
 def err_cconv_change : Error<
   "function declared '%0' here was previously declared "
   "%select{'%2'|without calling convention}1">;
-def warn_cconv_ignored : Warning<
-  "%0 calling convention ignored %select{"
+def warn_cconv_unsupported : Warning<
+  "%0 calling convention is not supported %select{"
   // Use CallingConventionIgnoredReason Enum to specify these.
   "for this target"
   "|on variadic function"

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=366368=366367=366368=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Jul 17 13:41:26 2019
@@ -3165,7 +3165,7 @@ bool Sema::MergeFunctionDecl(FunctionDec
   // Calling Conventions on a Builtin aren't really useful and setting a
   // default calling convention and cdecl'ing some builtin redeclarations 
is
   // common, so warn and ignore the calling convention on the 
redeclaration.
-  Diag(New->getLocation(), diag::warn_cconv_ignored)
+  Diag(New->getLocation(), diag::warn_cconv_unsupported)
   << FunctionType::getNameForCallConv(NewTypeInfo.getCC())
   << (int)CallingConventionIgnoredReason::BuiltinFunction;
   NewTypeInfo = NewTypeInfo.withCallingConv(OldTypeInfo.getCC());

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=366368=366367=366368=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Jul 17 13:41:26 2019
@@ -4669,7 +4669,7 @@ bool Sema::CheckCallingConvAttr(const Pa
 break;
 
   case TargetInfo::CCCR_Warning: {
-Diag(Attrs.getLoc(), diag::warn_cconv_ignored)
+Diag(Attrs.getLoc(), diag::warn_cconv_unsupported)
 << Attrs << (int)CallingConventionIgnoredReason::ForThisTarget;
 
 // This convention is not valid for the target. Use the default function or

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=366368=366367=366368=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Wed Jul 17 13:41:26 2019
@@ -7038,7 +7038,7 @@ static bool handleFunctionTypeAttr(TypeP
   // stdcall and fastcall are ignored with a warning for GCC and MS
   // compatibility.
   if (CC == CC_X86StdCall || CC == CC_X86FastCall)
-return S.Diag(attr.getLoc(), diag::warn_cconv_ignored)
+return S.Diag(attr.getLoc(), diag::warn_cconv_unsupported)
<< FunctionType::getNameForCallConv(CC)
<< (int)Sema::CallingConventionIgnoredReason::VariadicFunction;
 
@@ -7103,7 +7103,7 @@ void Sema::adjustMemberFunctionCC(QualTy
 // Issue a warning on ignored calling convention -- except of __stdcall.
 // Again, this is what MS compiler does.
 if (CurCC != CC_X86StdCall)
-  Diag(Loc, diag::warn_cconv_ignored)
+  Diag(Loc, diag::warn_cconv_unsupported)
   << FunctionType::getNameForCallConv(CurCC)
   << 

[PATCH] D64843: hwasan: Initialize the pass only once.

[Evgenii Stepanov via Phabricator via cfe-commits]

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

OK, sure.
As I understand functon passes also have better data locality by running a 
sequence of passes over a single function. Not sure how important that is.
LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64843



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

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

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

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



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:110
+  ConditionTracking
+};
+

Charusso wrote:
> What about the following?
> ```
> enum class TrackKind {
>   Full, // comment what it does
>   Condition // comment what it does
> };
> ```
> 
> Please consider the following example by unspoken naming conventions:
> ```
> enum class ColoringKind {
>   RedColoring,
>   BlueColoring
> }
> ```
> would be
> ```
> enum class Color { Red, Blue }
> ```
> where each trivial what is does, by name.
> 
> Please also note that, the thoroughness not working with redecls, 
> assumptions, loop conditions, anything we failed to inline, etc... so it is 
> not really that full tracking. But in our capabilities, it is the full what 
> we could do.
There's a good reasoning behind this -- if you take a look at this, and 
followup patches, the behavior the different `TrackingKind`s cause may differ 
from visitor to visitor, so I decided to document each behavior there. That 
way, you can't avoid the documents even if you didn't glance over this enum.

> Please also note that, the thoroughness not working with redecls, 
> assumptions, loop conditions, anything we failed to inline, etc... so it is 
> not really that full tracking. But in our capabilities, it is the full what 
> we could do.

I don't see what you mean here?



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:81
+
+bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
+   const Expr *E, BugReport ,

xazax.hun wrote:
> Do we need this overload? What about a default argument?
I don't want to expose this functionality outside of this file. I don't insist 
though!


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

https://reviews.llvm.org/D64270



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

[PATCH] D64287: [analyzer] Track the right hand side of the last store regardless of its value

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

Szelethus added a comment.

In D64287#1589941 , @xazax.hun wrote:

> LGTM!
>
> Since we allow new kinds of SVals to be tracked it would be great to test 
> this first on a larger corpus of projects just to see if there is a crash 
> (due to an unhandled SVal type).


It doesn't! Though that analysis was run with plenty of other patches as well, 
but I saw no crashes regardless.


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

https://reviews.llvm.org/D64287



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

[PATCH] D61838: [Sema] Suppress additional warnings for C's zero initializer

[Alex James via Phabricator via cfe-commits]

al3xtjames added a comment.

Thanks!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61838



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

[PATCH] D64603: [Target] Use IEEE quad format for long double on Fuchsia x86_64

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: clang/lib/Basic/Targets/X86.h:858
+  : FuchsiaTargetInfo(Triple, Opts) {
+LongDoubleFormat = ::APFloat::IEEEquad();
+  }

Can we just do this in FuchsiaTargetInfo generically?
I think we'd like to make our core API types uniform across machines and this 
makes sure that we get the Fuchsia common choice rather than a machine-specific 
choice that might differ in a new machine.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64603



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

[PATCH] D64539: [clang-doc] Add stylesheet to generated html docs

[Diego Astiazarán via Phabricator via cfe-commits]

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

Move the CSS file to share/clang and read it from there, instead of trying to 
read it from the file that's with the source code, that only worked when run 
locally.


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

https://reviews.llvm.org/D64539

Files:
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/clang-doc/stylesheets/clang-doc-default-stylesheet.css
  clang-tools-extra/clang-doc/tool/CMakeLists.txt
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -43,6 +43,7 @@
   std::string Expected = R"raw(
 
 namespace Namespace
+
 
   namespace Namespace
   Namespaces
@@ -100,6 +101,7 @@
   std::string Expected = R"raw(
 
 class r
+
 
   class r
   
@@ -157,6 +159,7 @@
   std::string Expected = R"raw(
 
 
+
 
   f
   
@@ -194,6 +197,7 @@
   std::string Expected = R"raw(
 
 
+
 
   enum class e
   
@@ -254,6 +258,7 @@
   std::string Expected = R"raw(
 
 
+
 
   f
   
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -91,6 +91,15 @@
   llvm_unreachable("Unknown OutputFormatTy");
 }
 
+// This function isn't referenced outside its translation unit, but it
+// can't use the "static" keyword because its address is used for
+// GetMainExecutable (since some platforms don't support taking the
+// address of main, and some platforms can't implement GetMainExecutable
+// without being given the address of a function in the main executable).
+std::string GetExecutablePath(const char *Argv0, void *MainAddr) {
+  return llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
+}
+
 bool CreateDirectory(const Twine , bool ClearDirectory = false) {
   std::error_code OK;
   llvm::SmallString<128> DocsRootPath;
@@ -129,7 +138,6 @@
  StringRef RelativePath,
  StringRef Name,
  StringRef Ext) {
-  std::error_code OK;
   llvm::SmallString<128> Path;
   llvm::sys::path::native(Root, Path);
   llvm::sys::path::append(Path, RelativePath);
@@ -195,8 +203,10 @@
 
   // Mapping phase
   llvm::outs() << "Mapping decls...\n";
+  void *MainAddr = (void *)(intptr_t)GetExecutablePath;
+  std::string ClangDocPath = GetExecutablePath(argv[0], MainAddr);
   clang::doc::ClangDocContext CDCtx = {Exec->get()->getExecutionContext(),
-   PublicOnly};
+   PublicOnly, OutDirectory, ClangDocPath};
   auto Err =
   Exec->get()->execute(doc::newMapperActionFactory(CDCtx), ArgAdjuster);
   if (Err) {
@@ -239,5 +249,8 @@
   llvm::errs() << toString(std::move(Err)) << "\n";
   }
 
+  if (!G->get()->createResources(CDCtx))
+return 1;
+
   return 0;
 }
Index: clang-tools-extra/clang-doc/tool/CMakeLists.txt
===
--- clang-tools-extra/clang-doc/tool/CMakeLists.txt
+++ clang-tools-extra/clang-doc/tool/CMakeLists.txt
@@ -14,4 +14,7 @@
   clangTooling
   clangToolingCore
   )
-  
\ No newline at end of file
+
+install(FILES ../stylesheets/clang-doc-default-stylesheet.css
+  DESTINATION share/clang
+  COMPONENT clang-doc)
Index: clang-tools-extra/clang-doc/stylesheets/clang-doc-default-stylesheet.css
===
--- /dev/null
+++ clang-tools-extra/clang-doc/stylesheets/clang-doc-default-stylesheet.css
@@ -0,0 +1,205 @@
+body,div {
+  margin: 0;
+  padding: 0;
+}
+
+body[no-overflow] {
+  overflow: hidden;
+}
+
+li>p:first-child {
+  margin-top: 0;
+}
+
+li>p:last-child {
+  margin-bottom: 0;
+}
+
+html {
+  -webkit-box-sizing: border-box;
+  box-sizing: border-box;
+}
+
+*,*::before,*::after {
+  -webkit-box-sizing: inherit;
+  box-sizing: inherit;
+}
+
+body,html {
+  color: #202124;
+  font: 400 16px/24px Roboto,sans-serif;
+  -moz-osx-font-smoothing: grayscale;
+  -webkit-font-smoothing: antialiased;
+  height: 100%;
+  margin: 36px;
+  -webkit-text-size-adjust: 100%;
+  -moz-text-size-adjust: 100%;
+  -ms-text-size-adjust: 100%;
+  text-size-adjust: 100%;
+}

[PATCH] D64843: hwasan: Initialize the pass only once.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

IIUC the reason was to work around the lack of analysis availability from 
module passes, which is no longer a concern with the new PM. I would personally 
prefer to see the other sanitizers re-architected this way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64843



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

[PATCH] D64843: hwasan: Initialize the pass only once.

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a subscriber: chandlerc.
eugenis added a comment.

I don't know what is the best practice here. Other sanitizers have both module 
and function passes, is there a reason to do things differently here?
@chandlerc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64843



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

[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

> So maybe `Driver::IsCLMode()` should take precedence over 
> `-fdiagnostics-absolute-paths` when using `/showIncludes`?

Definitely not. We want builds to be deterministic; this includes them being 
independent of the build directory. (This matters for distributed compilation 
caching.)

> So maybe I should also implement `/FC` 
> 
>  along the way, and make the defaults match between `clang-cl` and `cl`? WDYT?

We intentionally don't implement /FC for the same reasons, see the discussion 
on D23816 .

If you want absolute paths in the output, you can pass absolute paths to 
clang-cl and you'll get absolute paths in the output. Does that not work for 
you?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63648



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

[PATCH] D64878: [OpenMP] Fix sema check for unified memory case NFC

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

It does not look like NFC patch, the functionality has changed. Rename it, 
please, and a test case that reveals why we need this patch. Also, add a better 
description of the logic.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64878



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

[PATCH] D64563: Updated the signature for some stack related intrinsics (CLANG)

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

Can you add AMDGPU tests for these showing the correct address space is used


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

https://reviews.llvm.org/D64563



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

[PATCH] D64878: [OpenMP] Fix sema check for unified memory case NFC

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

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

This patch fixes a condition introduced in patch D60883 
.


Repository:
  rC Clang

https://reviews.llvm.org/D64878

Files:
  lib/Sema/SemaOpenMP.cpp


Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -2621,9 +2621,12 @@
   // Skip internally declared static variables.
   llvm::Optional Res =
   OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD);
+  bool IsUsingUnifiedMemory =
+  Stack->hasRequiresDeclWithClause();
   if (VD->hasGlobalStorage() && CS && !CS->capturesVariable(VD) &&
-  (Stack->hasRequiresDeclWithClause() ||
-   !Res || *Res != OMPDeclareTargetDeclAttr::MT_Link))
+  ((!IsUsingUnifiedMemory &&
+(!Res || *Res != OMPDeclareTargetDeclAttr::MT_Link)) ||
+   (IsUsingUnifiedMemory && !Res)))
 return;
 
   SourceLocation ELoc = E->getExprLoc();


Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -2621,9 +2621,12 @@
   // Skip internally declared static variables.
   llvm::Optional Res =
   OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD);
+  bool IsUsingUnifiedMemory =
+  Stack->hasRequiresDeclWithClause();
   if (VD->hasGlobalStorage() && CS && !CS->capturesVariable(VD) &&
-  (Stack->hasRequiresDeclWithClause() ||
-   !Res || *Res != OMPDeclareTargetDeclAttr::MT_Link))
+  ((!IsUsingUnifiedMemory &&
+(!Res || *Res != OMPDeclareTargetDeclAttr::MT_Link)) ||
+   (IsUsingUnifiedMemory && !Res)))
 return;
 
   SourceLocation ELoc = E->getExprLoc();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64592: [OpenMP] Fix unified memory implementation for multiple compilation units

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

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



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2610
+  auto *GV = cast(Ptr);
+  GV->setLinkage(llvm::GlobalValue::WeakAnyLinkage);
+

ABataev wrote:
> Better to fix the link clause processing in a different patch, it has nothing 
> to do with the unified memory.
Sure I can split this patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64592



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

[PATCH] D64592: [OpenMP] Fix unified memory implementation for multiple compilation units

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2610
+  auto *GV = cast(Ptr);
+  GV->setLinkage(llvm::GlobalValue::WeakAnyLinkage);
+

Better to fix the link clause processing in a different patch, it has nothing 
to do with the unified memory.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64592



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

[PATCH] D64592: [OpenMP] Fix unified memory implementation for multiple compilation units

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

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

- Fix tests.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64592

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  test/OpenMP/declare_target_codegen.cpp
  test/OpenMP/declare_target_link_codegen.cpp
  test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp

Index: test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp
===
--- test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp
+++ test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp
@@ -31,19 +31,19 @@
 }
 
 // CHECK-HOST: [[VAR:@.+]] = global double 1.00e+01
-// CHECK-HOST: [[VAR_DECL_TGT_LINK_PTR:@.+]] = global double* [[VAR]]
+// CHECK-HOST: [[VAR_DECL_TGT_LINK_PTR:@.+]] = weak global double* [[VAR]]
 
 // CHECK-HOST: [[TO_VAR:@.+]] = global double 2.00e+01
-// CHECK-HOST: [[VAR_DECL_TGT_TO_PTR:@.+]] = global double* [[TO_VAR]]
+// CHECK-HOST: [[VAR_DECL_TGT_TO_PTR:@.+]] = weak global double* [[TO_VAR]]
 
 // CHECK-HOST: [[OFFLOAD_SIZES:@.+]] = private unnamed_addr constant [2 x i64] [i64 4, i64 8]
 // CHECK-HOST: [[OFFLOAD_MAPTYPES:@.+]] = private unnamed_addr constant [2 x i64] [i64 800, i64 800]
 
-// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR_NAME:@.+]] = internal unnamed_addr constant [21 x i8]
-// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR:@.+]] = weak constant %struct.__tgt_offload_entry { i8* bitcast (double** [[VAR_DECL_TGT_LINK_PTR]] to i8*), i8* getelementptr inbounds ([21 x i8], [21 x i8]* [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR_NAME]], i32 0, i32 0), i64 8, i32 1, i32 0 }, section ".omp_offloading.entries"
+// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR_NAME:@.+]] = internal unnamed_addr constant [23 x i8]
+// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR:@.+]] = weak constant %struct.__tgt_offload_entry { i8* bitcast (double** [[VAR_DECL_TGT_LINK_PTR]] to i8*), i8* getelementptr inbounds ([23 x i8], [23 x i8]* [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR_NAME]], i32 0, i32 0), i64 8, i32 1, i32 0 }, section ".omp_offloading.entries"
 
-// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR_NAME:@.+]] = internal unnamed_addr constant [24 x i8]
-// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR:@.+]] = weak constant %struct.__tgt_offload_entry { i8* bitcast (double** [[VAR_DECL_TGT_TO_PTR]] to i8*), i8* getelementptr inbounds ([24 x i8], [24 x i8]* [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR_NAME]], i32 0, i32 0), i64 8, i32 0, i32 0 }, section ".omp_offloading.entries"
+// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR_NAME:@.+]] = internal unnamed_addr constant [26 x i8]
+// CHECK-HOST: [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR:@.+]] = weak constant %struct.__tgt_offload_entry { i8* bitcast (double** [[VAR_DECL_TGT_TO_PTR]] to i8*), i8* getelementptr inbounds ([26 x i8], [26 x i8]* [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR_NAME]], i32 0, i32 0), i64 8, i32 0, i32 0 }, section ".omp_offloading.entries"
 
 // CHECK-HOST: @llvm.used = appending global [2 x i8*] [i8* bitcast (double** [[VAR_DECL_TGT_LINK_PTR]] to i8*), i8* bitcast (double** [[VAR_DECL_TGT_TO_PTR]] to i8*)], section "llvm.metadata"
 
@@ -75,8 +75,8 @@
 
 // CHECK-HOST: call i32 @__tgt_target(i64 -1, i8* @{{.*}}.region_id, i32 2, i8** [[BPTR7]], i8** [[BPTR8]], i64* getelementptr inbounds ([2 x i64], [2 x i64]* [[OFFLOAD_SIZES]], i32 0, i32 0), i64* getelementptr inbounds ([2 x i64], [2 x i64]* [[OFFLOAD_MAPTYPES]], i32 0, i32 0))
 
-// CHECK-DEVICE: [[VAR_LINK:@.+]] = common global double* null
-// CHECK-DEVICE: [[VAR_TO:@.+]] = common global double* null
+// CHECK-DEVICE: [[VAR_LINK:@.+]] = weak global double* null
+// CHECK-DEVICE: [[VAR_TO:@.+]] = weak global double* null
 
 // CHECK-DEVICE: @llvm.used = appending global [2 x i8*] [i8* bitcast (double** [[VAR_LINK]] to i8*), i8* bitcast (double** [[VAR_TO]] to i8*)], section "llvm.metadata"
 
Index: test/OpenMP/declare_target_link_codegen.cpp
===
--- test/OpenMP/declare_target_link_codegen.cpp
+++ test/OpenMP/declare_target_link_codegen.cpp
@@ -18,15 +18,15 @@
 #define HEADER
 
 // HOST-DAG: @c = external global i32,
-// HOST-DAG: @c_decl_tgt_ref_ptr = global i32* @c
+// HOST-DAG: @c_0_decl_tgt_ref_ptr = weak global i32* @c
 // DEVICE-NOT: @c =
-// DEVICE: @c_decl_tgt_ref_ptr = common global i32* null
+// DEVICE: @c_0_decl_tgt_ref_ptr = weak global i32* null
 // HOST: [[SIZES:@.+]] = private unnamed_addr constant [2 x i64] [i64 4, i64 4]
 // HOST: [[MAPTYPES:@.+]] = private unnamed_addr constant [2 x i64] [i64 35, i64 531]
-// HOST: @.omp_offloading.entry_name{{.*}} = internal unnamed_addr constant [{{[0-9]+}} x i8] c"c_decl_tgt_ref_ptr\00"
-// HOST: @.omp_offloading.entry.c_decl_tgt_ref_ptr = weak constant %struct.__tgt_offload_entry { i8* bitcast (i32** @c_decl_tgt_ref_ptr to i8*), i8* getelementptr inbounds ([{{[0-9]+}} x i8], [{{[0-9]+}} x i8]* @.omp_offloading.entry_name, i32 0, i32 0), 

[PATCH] D64569: [OpenCL] Improve destructor support in C++ for OpenCL

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

LGTM with one minor request.




Comment at: clang/lib/AST/DeclCXX.cpp:2267
   QualType ClassTy = C.getTypeDeclType(Decl);
-  ClassTy = C.getQualifiedType(ClassTy, FPT->getMethodQuals());
-  return C.getPointerType(ClassTy);
+  return C.getQualifiedType(ClassTy, FPT->getMethodQuals());
 }

Thanks.  Can you extract this implementation out into a `static` function (i.e. 
an internal linkage function in this file) that takes an `ASTContext&` so that 
`getThisType` doesn't have to fetch the `ASTContext` twice?



Comment at: clang/lib/CodeGen/CGExprCXX.cpp:99
+  assert(!ThisTy.isNull());
+  assert(!ThisTy->isPointerType() && "Unexpected pointer type");
+  assert(ThisTy->getAsCXXRecordDecl() == DtorDecl->getParent() &&

mantognini wrote:
> I wasn't 100% sure if this was covered by the next assertion.
It is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64569



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

[PATCH] D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Driver/Types.h:100
   /// done for type 'Id'.
-  void getCompilationPhases(
-ID Id,
-llvm::SmallVectorImpl );
+  const std::vector getCompilationPhases(ID Id);
 

Please drop the top-level `const` as it doesn't add much utility.



Comment at: clang/lib/Driver/Driver.cpp:2217
   public:
-typedef llvm::SmallVector PhasesTy;
+typedef const std::vector PhasesTy;
 

Why are you changing this to an STL type?



Comment at: clang/lib/Driver/Driver.cpp:3236
 
-PL.clear();
-types::getCompilationPhases(InputType, PL);
+const std::vector PL = types::getCompilationPhases(InputType);
+LastPLSize = PL.size();

You could use a `const &` here and avoid the copy. Either that, or drop the 
`const` (it's not a common pattern in the code base).



Comment at: clang/lib/Driver/Driver.cpp:3278
 const types::ID HeaderType = lookupHeaderTypeForSourceType(InputType);
-llvm::SmallVector PCHPL;
-types::getCompilationPhases(HeaderType, PCHPL);
+const std::vector PCHPL =
+types::getCompilationPhases(HeaderType);

Same here



Comment at: clang/lib/Driver/Types.cpp:271
+//the old behavior and a subsequent change will delete most of the 
body.
+const llvm::SmallVector 
::getCompilationPhases(ID Id) {
+  llvm::SmallVector P;

compnerd wrote:
> Can we return `llvm::SmallVectorImpl` instead?  The size is 
> immaterial to this I think.
Agreed, I think this API should be working with `SmallVector`s and not 
`std::vector`s.



Comment at: clang/lib/Driver/Types.cpp:300
+  //   in a subsequent NFC commit.
+  auto Phases = getInfo(Id).Phases;
+  assert(Phases.size() == P.size() && "Invalid size.");

Please don't use `auto` here as the type is not spelled out in the 
initialization.



Comment at: clang/lib/Driver/Types.cpp:301-303
+  assert(Phases.size() == P.size() && "Invalid size.");
+  for (unsigned i = 0; i < Phases.size(); i++)
+assert(Phases[i] == P[i] && "Invalid Phase");

You can replace all three lines by:
`assert(std::equal(Phases.begin(), Phases.end(), P.begin(), P.end()) && 
"Invalid phase or size");`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64098



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

[PATCH] D64232: [analyzer] Prune calls to functions with linear CFGs that return a non-zero constrained value

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

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1052
+// to return that every time.
+if (N->getCFG().isLinear())
+  WouldEventBeMeaningless = true;

NoQ wrote:
> This time i'd rather go for "has exactly 3 blocks", because we're mostly 
> interested in the CFG being "syntactically linear" rather than "semantically 
> linear". I.e., if the CFG has an if-statement with a compile-time-constant 
> condition, i'd rather show the path, because who knows what does the 
> programmer think about this condition.
Yeah, this is a hard question, both sides can have its own reasons. 

One of my concern is that functions that does seem to be linear to people are 
actually not linear CFG wise. One example is single one liner functions with an 
assert preceding the line in question. 

Other interesting note is that once we add exception support and edges for 
exception handling in the CFG, all the analyzer budgets and CFG node count 
based heuristics need to be redone. But this is not something we should worry 
about at this point. 




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

https://reviews.llvm.org/D64232



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

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

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

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:81
+
+bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
+   const Expr *E, BugReport ,

Do we need this overload? What about a default argument?


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

https://reviews.llvm.org/D64270



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

[PATCH] D64287: [analyzer] Track the right hand side of the last store regardless of its value

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

xazax.hun accepted this revision.
xazax.hun added a comment.

LGTM!

Since we allow new kinds of SVals to be tracked it would be great to test this 
first on a larger corpus of projects just to see if there is a crash (due to an 
unhandled SVal type).


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

https://reviews.llvm.org/D64287



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

[PATCH] D64867: [OpenCL] Update comments/diagnostics to refer to C++ for OpenCL mode

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 210381.
Anastasia edited the summary of this revision.
Anastasia added a comment.

- Updated comments
- Added motivation


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

https://reviews.llvm.org/D64867

Files:
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Basic/TokenKinds.def
  include/clang/Frontend/LangStandards.def
  lib/Frontend/InitPreprocessor.cpp
  lib/Parse/ParseDecl.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExprCXX.cpp
  test/Driver/unknown-std.cl
  test/Parser/opencl-cxx-keywords.cl
  test/Parser/opencl-cxx-virtual.cl
  test/SemaOpenCLCXX/newdelete.cl
  test/SemaOpenCLCXX/restricted.cl

Index: test/SemaOpenCLCXX/restricted.cl
===
--- test/SemaOpenCLCXX/restricted.cl
+++ test/SemaOpenCLCXX/restricted.cl
@@ -1,9 +1,9 @@
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify -fsyntax-only
 
 // This test checks that various C/C++/OpenCL C constructs are not available in
-// OpenCL C++, according to OpenCL C++ 1.0 Specification Section 2.9.
+// C++ for OpenCL.
 
-// Test that typeid is not available in OpenCL C++.
+// Test that typeid is not available.
 namespace std {
   // Provide a dummy std::type_info so that we can use typeid.
   class type_info {
@@ -11,9 +11,9 @@
   };
 }
 __constant std::type_info int_ti = typeid(int);
-// expected-error@-1 {{'typeid' is not supported in OpenCL C++}}
+// expected-error@-1 {{'typeid' is not supported in C++ for OpenCL}}
 
-// Test that dynamic_cast is not available in OpenCL C++.
+// Test that dynamic_cast is not available in C++ for OpenCL.
 class A {
 public:
   int a;
@@ -25,17 +25,17 @@
 
 B *test_dynamic_cast(B *p) {
   return dynamic_cast(p);
-  // expected-error@-1 {{'dynamic_cast' is not supported in OpenCL C++}}
+  // expected-error@-1 {{'dynamic_cast' is not supported in C++ for OpenCL}}
 }
 
 // Test storage class qualifiers.
 __constant _Thread_local int a = 1;
-// expected-error@-1 {{OpenCL C++ version 1.0 does not support the '_Thread_local' storage class specifier}}
+// expected-error@-1 {{C++ for OpenCL version 1.0 does not support the '_Thread_local' storage class specifier}}
 __constant __thread int b = 2;
-// expected-error@-1 {{OpenCL C++ version 1.0 does not support the '__thread' storage class specifier}}
+// expected-error@-1 {{C++ for OpenCL version 1.0 does not support the '__thread' storage class specifier}}
 kernel void test_storage_classes() {
   register int x;
-  // expected-error@-1 {{OpenCL C++ version 1.0 does not support the 'register' storage class specifier}}
+  // expected-error@-1 {{C++ for OpenCL version 1.0 does not support the 'register' storage class specifier}}
   thread_local int y;
-  // expected-error@-1 {{OpenCL C++ version 1.0 does not support the 'thread_local' storage class specifier}}
+  // expected-error@-1 {{C++ for OpenCL version 1.0 does not support the 'thread_local' storage class specifier}}
 }
Index: test/SemaOpenCLCXX/newdelete.cl
===
--- test/SemaOpenCLCXX/newdelete.cl
+++ test/SemaOpenCLCXX/newdelete.cl
@@ -19,8 +19,8 @@
 // There are no global user-defined new operators at this point. Test that clang
 // rejects these gracefully.
 void test_default_new_delete(void *buffer, A **pa) {
-  A *a = new A; // expected-error {{'default new' is not supported in OpenCL C++}}
-  delete a; // expected-error {{'default delete' is not supported in OpenCL C++}}
+  A *a = new A; // expected-error {{'default new' is not supported in C++ for OpenCL}}
+  delete a; // expected-error {{'default delete' is not supported in C++ for OpenCL}}
   *pa = new (buffer) A; // expected-error {{use of placement new requires explicit declaration}}
 }
 
@@ -36,10 +36,10 @@
 
 void test_new_delete(void *buffer, A **a, B **b) {
   *a = new A; // expected-error {{no matching function for call to 'operator new'}}
-  delete a;   // expected-error {{'default delete' is not supported in OpenCL C++}}
+  delete a;   // expected-error {{'default delete' is not supported in C++ for OpenCL}}
 
   *a = new A[20]; // expected-error {{no matching function for call to 'operator new[]'}}
-  delete[] *a;// expected-error {{'default delete' is not supported in OpenCL C++}}
+  delete[] *a;// expected-error {{'default delete' is not supported in C++ for OpenCL}}
 
   // User-defined placement new is supported.
   *a = new (buffer) A;
Index: test/Parser/opencl-cxx-virtual.cl
===
--- test/Parser/opencl-cxx-virtual.cl
+++ test/Parser/opencl-cxx-virtual.cl
@@ -3,17 +3,17 @@
 // Test that virtual functions and abstract classes are rejected.
 class virtual_functions 

[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

The problem is that it is not `cl.exe`'s behavior - it always makes paths 
absolute:

  F:\svn\test>cl /c test.cc /showIncludes
  Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27031.1 for x64
  Copyright (C) Microsoft Corporation.  All rights reserved.
  
  test.cc
  Note: including file: f:\svn\test\foo/test.h
  
  F:\svn\test>c:
  
  C:\Windows\System32>cl /c f:test.cc /showIncludes
  Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27031.1 for x64
  Copyright (C) Microsoft Corporation.  All rights reserved.
  
  test.cc
  Note: including file: f:\svn\test\foo/test.h

So maybe `Driver::IsCLMode()` should take precedence over 
`-fdiagnostics-absolute-paths` when using `/showIncludes`?

The behavior between `clang-cl` and `cl` is also very different when using `-E`:

  C:\Windows\System32>f:\svn\build\Debug\bin\clang-cl /c f:test.cc /E
  clang-cl: error: no such file or directory: 'f:test.cc'
  clang-cl: error: no input files
  
  C:\Windows\System32>f:
  
  F:\svn\test>f:\svn\build\Debug\bin\clang-cl /c test.cc /E
  # 1 "test.cc"
  # 1 "" 1
  # 1 "" 3
  # 361 "" 3
  # 1 "" 1
  # 1 "" 2
  # 1 "test.cc" 2
  # 1 "./foo/test.h" 1
  void f();
  # 1 "test.cc" 2
  
  
  F:\svn\test>cl /c test.cc /E
  Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27031.1 for x64
  Copyright (C) Microsoft Corporation.  All rights reserved.
  
  test.cc
  #line 1 "test.cc"
  #line 1 "f:\\svn\\test\\foo/test.h"
  void f();
  #line 2 "test.cc"
  
  F:\svn\test>cl /c test.cc /E /FC
  Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27031.1 for x64
  Copyright (C) Microsoft Corporation.  All rights reserved.
  
  test.cc
  #line 1 "f:\\svn\\test\\test.cc"
  #line 1 "f:\\svn\\test\\foo\\test.h"
  void f();
  #line 2 "f:\\svn\\test\\test.cc"

So maybe I should also implement `/FC` 

 along the way, and make the defaults match between `clang-cl` and `cl`? WDYT?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63648



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

[PATCH] D64272: [analyzer] Note last writes to a condition only in a nested stackframe

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

xazax.hun added a comment.

I have one small question otherwise looks good.




Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1457
+  if (TKind == TrackingKind::ConditionTracking &&
+  StoreSite->getStackFrame() == OriginSFC)
+return nullptr;

I wonder if "nested" is a good term in the title of this revision. We could 
also show notes in the caller right? So it might also be an "enclosing" frame. 
And if so, is it desired? Since the "enclosing" frames should already be 
visible for the user without the additional notes. But correct me if I'm wrong 
:)


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

https://reviews.llvm.org/D64272



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

[PATCH] D64418: [Docs][OpenCL] Documentation of C++ for OpenCL mode

[Anastasia Stulova via Phabricator via cfe-commits]

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

Reopening to continue reviews.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64418



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

[PATCH] D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

@aaron.ballman Any thoughts on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64098



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

[PATCH] D64874: Improve handling of function pointer conversions

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added a reviewer: rsmith.
Mordante added a project: clang.

Starting with C++17 the `noexcept` is part of the function signature but a 
`noexcept` function can be converted to a `noexcept(false)` function. The 
overload resolution code handles it properly but expression builder doesn't. It 
causes the following code to trigger and assertion failure:

  struct S {
  int f(void) noexcept { return 110; }
  } s;
  
  template  int f10(void) { return (s.*a)(); }
  
  int foo(void)
  {
return f10<::f >();
  }

The fix adds an extra implicit cast when needed. I picked the `CK_NoOp` as 
`CastKind` since it seems to be the best fit. However I wonder whether it would 
be better to add a new value `CK_FunctionPointerConversion`.

This fixes bug 40024.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64874

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CXX/conv/conv.fctptr/template-noexcept-invalid.cpp
  clang/test/CXX/conv/conv.fctptr/template-noexcept-valid.cpp


Index: clang/test/CXX/conv/conv.fctptr/template-noexcept-valid.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noexcept-valid.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only %s
+
+// Starting with C++17 the noexcept is part of the function signature but
+// a noexcept function can be converted to a noexcept(false) function.
+// The tests were added for https://bugs.llvm.org/show_bug.cgi?id=40024
+// Tests the still valid function pointer conversions
+
+struct S {
+  int f(void) noexcept { return 110; }
+} s;
+
+template 
+int f10(void) { return (s.*a)(); }
+
+int foo(void) {
+  return f10<::f>();
+}
Index: clang/test/CXX/conv/conv.fctptr/template-noexcept-invalid.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noexcept-invalid.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only %s
+// RUN: not %clang_cc1 -std=c++17 -fsyntax-only %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -std=c++2a -fsyntax-only %s 2>&1 | FileCheck %s
+
+// Starting with C++17 the noexcept is part of the function signature but
+// a noexcept function can be converted to a noexcept(false) function.
+// The tests were added for https://bugs.llvm.org/show_bug.cgi?id=40024
+// Tests the no longer valid function pointer conversions
+
+struct S {
+  int f(void) { return 110; }
+} s;
+
+template 
+int f10(void) { return (s.*a)(); }
+
+int foo(void) {
+  return f10<::f>();
+}
+
+// CHECK: error: no matching function for call to 'f10'
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -6991,6 +6991,14 @@
 ObjCLifetimeConversion))
 RefExpr = ImpCastExprToType(RefExpr.get(), 
ParamType.getUnqualifiedType(), CK_NoOp);
 
+  // Starting with C++17 the noexcept is part of the function signature but
+  // a noexcept function can be converted to a noexcept(false) function.
+  QualType resultTy;
+  if (getLangOpts().CPlusPlus17 &&
+  IsFunctionConversion(((Expr *)RefExpr.get())->getType(),
+   ParamType.getUnqualifiedType(), resultTy))
+RefExpr = ImpCastExprToType(RefExpr.get(), resultTy, CK_NoOp);
+
   assert(!RefExpr.isInvalid() &&
  Context.hasSameType(((Expr*) RefExpr.get())->getType(),
  ParamType.getUnqualifiedType()));


Index: clang/test/CXX/conv/conv.fctptr/template-noexcept-valid.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noexcept-valid.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only %s
+
+// Starting with C++17 the noexcept is part of the function signature but
+// a noexcept function can be converted to a noexcept(false) function.
+// The tests were added for https://bugs.llvm.org/show_bug.cgi?id=40024
+// Tests the still valid function pointer conversions
+
+struct S {
+  int f(void) noexcept { return 110; }
+} s;
+
+template 
+int f10(void) { return (s.*a)(); }
+
+int foo(void) {
+  return f10<::f>();
+}
Index: clang/test/CXX/conv/conv.fctptr/template-noexcept-invalid.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noexcept-invalid.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++14 

[PATCH] D64518: [LibTooling] Relax Transformer to allow rewriting macro expansions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D64518#1589857 , @ymandel wrote:

> In D64518#1589768 , @ilya-biryukov 
> wrote:
>
> > In D64518#1588092 , @ymandel wrote:
> >
> > > This seems like a good candidate for configuration -- the user could then 
> > > choose which mode to run in.  But, I'm also open to just reporting these 
> > > conditions as errors.  It's already in a context that returns Expected, 
> > > so its no trouble; it's just a matter of choosing what we think is 
> > > "correct".
> >
> >
> > WRT to returning `Expected` vs `Optional`. Either seems fine and in the 
> > spirit of the library, depending on whether we want to produce more 
> > detailed errors. However, if we choose `Optional` let's stick to it, as 
> > practice shows switching from `Optional` to `Expected` correctly is almost 
> > impossible, as that requires a lot of attention to make sure all clients 
> > consume the errors (and given that it's an error case, tests often don't 
> > catch unconsumed errors).
> >  I would personally go with `Optional` here (meaning the client code would 
> > have to say something generic like `could not map from macro expansion to 
> > source code`). But up to you, not a strong preference.
>
>
> I think we might be talking about different things here. I meant that the 
> *calling* function, `translateEdits`, returns `Expected`, so it would be easy 
> to return an error when `makeValidRange` returns `None`.  I agree that 
> `makeValidRange` (or whatever we choose to call it) should stick with 
> `Optional` for simplicity (with the generic interpretation of `None` being 
> "could not map from macro expansion to source code").


Ah, great, we're on the same page then. LGTM!

>> WRT to which cases we choose to handle, I'd start with a minimal number of 
>> supported examples (covering full macro expansion, or inside a single 
>> argument) and gradually add other cases as we find use-cases. What are your 
>> thoughts on that?
> 
> I assume you mean which cases `makeValidRange` should handle (successfully)? 
> If so, that sounds good.

Yes, exactly.

> But, what do you think about how to handle failures of `makeValidRange` -- 
> ignore them silently (which is what we're doing now) or treat them as errors?

I think it depends on the use-case, e.g. if we try to produce a clang-tidy fix 
for some warning and can't produce a fix because `makeValidRange` failed, then 
not showing the fix (i.e. failing silently) seems fine.
OTOH, if we're building a refactoring tool that should find an replace all 
occurrences of a matcher and apply the transformation, failing silently is 
probably not a good option, we should possibly list the locations where the 
transformation failed (so that users can do manual changes to complete the 
refactoring).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64518



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

[PATCH] D64780: Disallow most calling convention attributes on PS4.

[Sunil Srivastava via Phabricator via cfe-commits]

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



Comment at: test/Sema/no_callconv.cpp:1
+// RUN: %clang_cc1 %s -triple x86_64-scei-ps4 -DPS4 -fsyntax-only -verify
+// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -fsyntax-only -verify

aaron.ballman wrote:
> Does this really need an svn:executable property on the file?
I am not sure where that comes from. I did an 'svn add' and 'svn diff'. 
Regardless, it will be a new test file with normal lines.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64780



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

r366357 - [OPENMP]Fix PR42632: crash on the analysis of the OpenMP constructs.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Jul 17 11:03:39 2019
New Revision: 366357

URL: http://llvm.org/viewvc/llvm-project?rev=366357=rev
Log:
[OPENMP]Fix PR42632: crash on the analysis of the OpenMP constructs.

Fixed processing of the CapturedStmt children to fix the crash of the
OpenMP constructs during analysis.

Modified:
cfe/trunk/lib/AST/ParentMap.cpp
cfe/trunk/test/Analysis/openmp-unsupported.c

Modified: cfe/trunk/lib/AST/ParentMap.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ParentMap.cpp?rev=366357=366356=366357=diff
==
--- cfe/trunk/lib/AST/ParentMap.cpp (original)
+++ cfe/trunk/lib/AST/ParentMap.cpp Wed Jul 17 11:03:39 2019
@@ -83,6 +83,18 @@ static void BuildParentMap(MapTy& M, Stm
 }
 break;
   }
+  case Stmt::CapturedStmtClass:
+for (Stmt *SubStmt : S->children()) {
+  if (SubStmt) {
+M[SubStmt] = S;
+BuildParentMap(M, SubStmt, OVMode);
+  }
+}
+if (Stmt *SubStmt = cast(S)->getCapturedStmt()) {
+  M[SubStmt] = S;
+  BuildParentMap(M, SubStmt, OVMode);
+}
+break;
   default:
 for (Stmt *SubStmt : S->children()) {
   if (SubStmt) {

Modified: cfe/trunk/test/Analysis/openmp-unsupported.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/openmp-unsupported.c?rev=366357=366356=366357=diff
==
--- cfe/trunk/test/Analysis/openmp-unsupported.c (original)
+++ cfe/trunk/test/Analysis/openmp-unsupported.c Wed Jul 17 11:03:39 2019
@@ -4,4 +4,8 @@
 void openmp_parallel_crash_test() {
 #pragma omp parallel
   ;
+#pragma omp parallel for
+  for (int i = 0; i < 8; ++i)
+for (int j = 0, k = 0; j < 8; ++j)
+  ;
 }


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

[PATCH] D64780: Disallow most calling convention attributes on PS4.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/Sema/no_callconv.cpp:1
+// RUN: %clang_cc1 %s -triple x86_64-scei-ps4 -DPS4 -fsyntax-only -verify
+// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -fsyntax-only -verify

Does this really need an svn:executable property on the file?


Repository:
  rC Clang

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

https://reviews.llvm.org/D64780



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

[PATCH] D64518: [LibTooling] Relax Transformer to allow rewriting macro expansions

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D64518#1589768 , @ilya-biryukov 
wrote:

> In D64518#1588092 , @ymandel wrote:
>
> > This seems like a good candidate for configuration -- the user could then 
> > choose which mode to run in.  But, I'm also open to just reporting these 
> > conditions as errors.  It's already in a context that returns Expected, so 
> > its no trouble; it's just a matter of choosing what we think is "correct".
>
>
> WRT to returning `Expected` vs `Optional`. Either seems fine and in the 
> spirit of the library, depending on whether we want to produce more detailed 
> errors. However, if we choose `Optional` let's stick to it, as practice shows 
> switching from `Optional` to `Expected` correctly is almost impossible, as 
> that requires a lot of attention to make sure all clients consume the errors 
> (and given that it's an error case, tests often don't catch unconsumed 
> errors).
>  I would personally go with `Optional` here (meaning the client code would 
> have to say something generic like `could not map from macro expansion to 
> source code`). But up to you, not a strong preference.


I think we might be talking about different things here. I meant that the 
*calling* function, `translateEdits`, returns `Expected`, so it would be easy 
to return an error when `makeValidRange` returns `None`.  I agree that 
`makeValidRange` (or whatever we choose to call it) should stick with 
`Optional` for simplicity (with the generic interpretation of `None` being 
"could not map from macro expansion to source code").

> WRT to which cases we choose to handle, I'd start with a minimal number of 
> supported examples (covering full macro expansion, or inside a single 
> argument) and gradually add other cases as we find use-cases. What are your 
> thoughts on that?

I assume you mean which cases `makeValidRange` should handle (successfully)? If 
so, that sounds good.  But, what do you think about how to handle failures of 
`makeValidRange` -- ignore them silently (which is what we're doing now) or 
treat them as errors?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64518



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

r366355 - Revert [AArch64] Add support for Transactional Memory Extension (TME)

[Momchil Velikov via cfe-commits]

Author: chill
Date: Wed Jul 17 10:43:32 2019
New Revision: 366355

URL: http://llvm.org/viewvc/llvm-project?rev=366355=rev
Log:
Revert [AArch64] Add support for Transactional Memory Extension (TME)

This reverts r366322 (git commit 4b8da3a503e434ddbc08ecf66582475765f449bc)

Removed:
cfe/trunk/test/CodeGen/aarch64-tme-tcancel-arg.cpp
cfe/trunk/test/CodeGen/aarch64-tme.c
cfe/trunk/test/Sema/aarch64-tme-errors.c
cfe/trunk/test/Sema/aarch64-tme-tcancel-const-error.c
cfe/trunk/test/Sema/aarch64-tme-tcancel-range-error.c
Modified:
cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
cfe/trunk/lib/Basic/Targets/AArch64.cpp
cfe/trunk/lib/Basic/Targets/AArch64.h
cfe/trunk/lib/Headers/arm_acle.h
cfe/trunk/lib/Sema/SemaChecking.cpp

Modified: cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsAArch64.def?rev=366355=366354=366355=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsAArch64.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsAArch64.def Wed Jul 17 10:43:32 2019
@@ -91,12 +91,6 @@ LANGBUILTIN(__sevl,  "v", "",   ALL_MS_L
 // Misc
 BUILTIN(__builtin_sponentry, "v*", "c")
 
-// Transactional Memory Extension
-BUILTIN(__builtin_arm_tstart, "WUi", "nj")
-BUILTIN(__builtin_arm_tcommit, "v", "n")
-BUILTIN(__builtin_arm_tcancel, "vWUIi", "nr")
-BUILTIN(__builtin_arm_ttest, "WUi", "nc")
-
 TARGET_HEADER_BUILTIN(_BitScanForward, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")

Modified: cfe/trunk/lib/Basic/Targets/AArch64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AArch64.cpp?rev=366355=366354=366355=diff
==
--- cfe/trunk/lib/Basic/Targets/AArch64.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/AArch64.cpp Wed Jul 17 10:43:32 2019
@@ -219,9 +219,6 @@ void AArch64TargetInfo::getTargetDefines
   if (HasMTE)
 Builder.defineMacro("__ARM_FEATURE_MEMORY_TAGGING", "1");
 
-  if (HasTME)
-Builder.defineMacro("__ARM_FEATURE_TME", "1");
-
   if ((FPU & NeonMode) && HasFP16FML)
 Builder.defineMacro("__ARM_FEATURE_FP16FML", "1");
 
@@ -273,7 +270,6 @@ bool AArch64TargetInfo::handleTargetFeat
   HasDotProd = false;
   HasFP16FML = false;
   HasMTE = false;
-  HasTME = false;
   ArchKind = llvm::AArch64::ArchKind::ARMV8A;
 
   for (const auto  : Features) {
@@ -305,8 +301,6 @@ bool AArch64TargetInfo::handleTargetFeat
   HasFP16FML = true;
 if (Feature == "+mte")
   HasMTE = true;
-if (Feature == "+tme")
-  HasTME = true;
   }
 
   setDataLayout();

Modified: cfe/trunk/lib/Basic/Targets/AArch64.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AArch64.h?rev=366355=366354=366355=diff
==
--- cfe/trunk/lib/Basic/Targets/AArch64.h (original)
+++ cfe/trunk/lib/Basic/Targets/AArch64.h Wed Jul 17 10:43:32 2019
@@ -35,7 +35,6 @@ class LLVM_LIBRARY_VISIBILITY AArch64Tar
   bool HasDotProd;
   bool HasFP16FML;
   bool HasMTE;
-  bool HasTME;
 
   llvm::AArch64::ArchKind ArchKind;
 

Modified: cfe/trunk/lib/Headers/arm_acle.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/arm_acle.h?rev=366355=366354=366355=diff
==
--- cfe/trunk/lib/Headers/arm_acle.h (original)
+++ cfe/trunk/lib/Headers/arm_acle.h Wed Jul 17 10:43:32 2019
@@ -613,7 +613,7 @@ __jcvt(double __a) {
 #define __arm_wsr64(sysreg, v) __builtin_arm_wsr64(sysreg, v)
 #define __arm_wsrp(sysreg, v) __builtin_arm_wsrp(sysreg, v)
 
-/* Memory Tagging Extensions (MTE) Intrinsics */
+// Memory Tagging Extensions (MTE) Intrinsics
 #if __ARM_FEATURE_MEMORY_TAGGING
 #define __arm_mte_create_random_tag(__ptr, __mask)  __builtin_arm_irg(__ptr, 
__mask)
 #define __arm_mte_increment_tag(__ptr, __tag_offset)  
__builtin_arm_addg(__ptr, __tag_offset)
@@ -623,28 +623,6 @@ __jcvt(double __a) {
 #define __arm_mte_ptrdiff(__ptra, __ptrb) __builtin_arm_subp(__ptra, __ptrb)
 #endif
 
-/* Transactional Memory Extension (TME) Intrinsics */
-#if __ARM_FEATURE_TME
-
-#define _TMFAILURE_REASON  0x7fffu
-#define _TMFAILURE_RTRY0x8000u
-#define _TMFAILURE_CNCL0x0001u
-#define _TMFAILURE_MEM 0x0002u
-#define _TMFAILURE_IMP 0x0004u
-#define _TMFAILURE_ERR 0x0008u
-#define _TMFAILURE_SIZE0x0010u
-#define _TMFAILURE_NEST0x0020u
-#define _TMFAILURE_DBG 0x0040u
-#define _TMFAILURE_INT 0x0080u
-#define _TMFAILURE_TRIVIAL 0x0100u
-
-#define __tstart()__builtin_arm_tstart()
-#define __tcommit()   

Re: [PATCH] D64416: [AArch64] Add support for Transactional Memory Extension (TME)

[Momchil Velikov via cfe-commits]

Thanks, I've reverted it for now.


From: Simon Pilgrim via Phabricator 
Sent: 17 July 2019 18:23:29
To: Momchil Velikov; oliver.stann...@linaro.org; t.p.northo...@gmail.com; 
jdoerf...@anl.gov
Cc: llvm-...@redking.me.uk; notst...@gmail.com; cfe-commits@lists.llvm.org; 
jav...@graphcore.ai; Kristof Beyls; hiradi...@msn.com; 
llvm-comm...@lists.llvm.org; kanh...@a-bix.com; mcros...@codeaurora.org; 
ju...@samsung.com; diana.pi...@linaro.org; florian_h...@apple.com
Subject: [PATCH] D64416: [AArch64] Add support for Transactional Memory 
Extension (TME)

RKSimon added a comment.

@chill This is failing on buildbots with EXPENSIVE_CHECKS enabled: 
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/18684/steps/test-check-all/logs/stdio


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64416



IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64813: [clang-tidy] Exclude forward decls from fuchsia-multiple-inheritance

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366354: [clang-tidy] Exclude forward decls from 
fuchsia-multiple-inheritance (authored by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64813?vs=210144=210367#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64813

Files:
  clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp


Index: clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -93,7 +93,8 @@
 return;
 
   // Match declarations which have bases.
-  Finder->addMatcher(cxxRecordDecl(hasBases()).bind("decl"), this);
+  Finder->addMatcher(
+  cxxRecordDecl(allOf(hasBases(), isDefinition())).bind("decl"), this);
 }
 
 void MultipleInheritanceCheck::check(const MatchFinder::MatchResult ) {
Index: clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -41,6 +41,9 @@
   virtual int baz() = 0;
 };
 
+// Shouldn't warn on forward declarations.
+class Bad_Child1;
+
 // Inherits from multiple concrete classes.
 // CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes that 
aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
 // CHECK-NEXT: class Bad_Child1 : public Base_A, Base_B {};


Index: clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -93,7 +93,8 @@
 return;
 
   // Match declarations which have bases.
-  Finder->addMatcher(cxxRecordDecl(hasBases()).bind("decl"), this);
+  Finder->addMatcher(
+  cxxRecordDecl(allOf(hasBases(), isDefinition())).bind("decl"), this);
 }
 
 void MultipleInheritanceCheck::check(const MatchFinder::MatchResult ) {
Index: clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -41,6 +41,9 @@
   virtual int baz() = 0;
 };
 
+// Shouldn't warn on forward declarations.
+class Bad_Child1;
+
 // Inherits from multiple concrete classes.
 // CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes that aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
 // CHECK-NEXT: class Bad_Child1 : public Base_A, Base_B {};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r366354 - [clang-tidy] Exclude forward decls from fuchsia-multiple-inheritance

[Julie Hockett via cfe-commits]

Author: juliehockett
Date: Wed Jul 17 10:40:53 2019
New Revision: 366354

URL: http://llvm.org/viewvc/llvm-project?rev=366354=rev
Log:
[clang-tidy] Exclude forward decls from fuchsia-multiple-inheritance

Addresses b39770.

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

Modified:
clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp?rev=366354=366353=366354=diff
==
--- clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp Wed 
Jul 17 10:40:53 2019
@@ -93,7 +93,8 @@ void MultipleInheritanceCheck::registerM
 return;
 
   // Match declarations which have bases.
-  Finder->addMatcher(cxxRecordDecl(hasBases()).bind("decl"), this);
+  Finder->addMatcher(
+  cxxRecordDecl(allOf(hasBases(), isDefinition())).bind("decl"), this);
 }
 
 void MultipleInheritanceCheck::check(const MatchFinder::MatchResult ) {

Modified: 
clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp?rev=366354=366353=366354=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp 
Wed Jul 17 10:40:53 2019
@@ -41,6 +41,9 @@ public:
   virtual int baz() = 0;
 };
 
+// Shouldn't warn on forward declarations.
+class Bad_Child1;
+
 // Inherits from multiple concrete classes.
 // CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes that 
aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
 // CHECK-NEXT: class Bad_Child1 : public Base_A, Base_B {};


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

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D64695#1589740 , @Manikishan wrote:

> In D64695#1589508 , @lebedev.ri 
> wrote:
>
> > Is there sufficient test coverage as to what happens if `SortPriority` is 
> > not set?
>
>
> If SortPriority is not set, the Includes will be grouped without sorting,


Let me rephrase - for the exiting `.clang-format`s, that don't currently 
specify `SortPriority`,
this introduction of `SortPriority` should not change the header handling.
Is that the case, and if so is there sufficient test coverage for that?


Repository:
  rC Clang

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

https://reviews.llvm.org/D64695



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

[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

>> Those aren't diagnostics, so that's not surprising.
> 
> What would suggest in that case? Add a new `-fpreprocessor-absolute-paths` 
> option? Or change the name of `-fdiagnostics-absolute-paths` for another name 
> that applies to both diagnostics and the preprocessor output?

clang uses the absolute-ness you use on the input. If you pass absolute paths 
to clang and to -I, then --show-includes etc will print absolute paths:

  $ cat test.cc
  #include "foo/test.h"
  $ out/gn/bin/clang-cl /showIncludes /c test.cc
  Note: including file: ./foo/test.h
  $ out/gn/bin/clang-cl /showIncludes /c test.cc -I$PWD
  Note: including file: /Users/thakis/src/llvm-project/foo/test.h


Repository:
  rC Clang

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

https://reviews.llvm.org/D63648



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

[PATCH] D64867: [OpenCL] Update comments/diagnostics to refer to C++ for OpenCL mode

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

It would be good if you could provide some motivation for this change in the 
description of this review.




Comment at: include/clang/Basic/DiagnosticParseKinds.td:1157
 
-// OpenCL C++.
+// C++ for OpenCL.
 def err_openclcxx_virtual_function : Error<

Please align on either "C++ for OpenCL" or "C++ for OpenCL mode" (in 
DiagnosticCommonKinds.td).



Comment at: lib/Sema/SemaDecl.cpp:6429
 
-// OpenCL C++ 1.0 s2.9: the thread_local storage qualifier is not
-// supported.  OpenCL C does not support thread_local either, and
+// C++ for OpenCL does not allow thread_local storage qualifier.
+// OpenCL C does not support thread_local either, and

*the* thread_local storage qualifier.


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

https://reviews.llvm.org/D64867



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

[PATCH] D64416: [AArch64] Add support for Transactional Memory Extension (TME)

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

@chill This is failing on buildbots with EXPENSIVE_CHECKS enabled: 
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/18684/steps/test-check-all/logs/stdio


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64416



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

[clang-tools-extra] r366353 - [clang-tidy] Fix crash on end location inside macro

[Nathan Huckleberry via cfe-commits]

Author: nathan-huckleberry
Date: Wed Jul 17 10:22:43 2019
New Revision: 366353

URL: http://llvm.org/viewvc/llvm-project?rev=366353=rev
Log:
[clang-tidy] Fix crash on end location inside macro

Summary:
Lexer::getLocForEndOfToken is defined to return an
invalid location if the given location is inside a macro.
Other checks conditionally warn based off location
validity. Updating this check to do the same.

Reviewers: JonasToth, aaron.ballman, nickdesaulniers

Reviewed By: nickdesaulniers

Subscribers: lebedev.ri, nickdesaulniers, xazax.hun, cfe-commits

Tags: #clang

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

Added:
clang-tools-extra/trunk/test/clang-tidy/bugprone-branch-clone-macro-crash.c
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/BranchCloneCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/BranchCloneCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/BranchCloneCheck.cpp?rev=366353=366352=366353=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/BranchCloneCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BranchCloneCheck.cpp Wed Jul 17 
10:22:43 2019
@@ -132,9 +132,12 @@ void BranchCloneCheck::check(const Match
   // We report the first occurence only when we find the second one.
   diag(Branches[i]->getBeginLoc(),
"repeated branch in conditional chain");
-  diag(Lexer::getLocForEndOfToken(Branches[i]->getEndLoc(), 0,
-  *Result.SourceManager, 
getLangOpts()),
-   "end of the original", DiagnosticIDs::Note);
+  SourceLocation End =
+  Lexer::getLocForEndOfToken(Branches[i]->getEndLoc(), 0,
+ *Result.SourceManager, getLangOpts());
+  if (End.isValid()) {
+diag(End, "end of the original", DiagnosticIDs::Note);
+  }
 }
 
 diag(Branches[j]->getBeginLoc(), "clone %0 starts here",
@@ -208,10 +211,12 @@ void BranchCloneCheck::check(const Match
 
 if (EndLoc.isMacroID())
   EndLoc = Context.getSourceManager().getExpansionLoc(EndLoc);
+EndLoc = Lexer::getLocForEndOfToken(EndLoc, 0, *Result.SourceManager,
+getLangOpts());
 
-diag(Lexer::getLocForEndOfToken(EndLoc, 0, *Result.SourceManager,
-getLangOpts()),
- "last of these clones ends here", DiagnosticIDs::Note);
+if (EndLoc.isValid()) {
+  diag(EndLoc, "last of these clones ends here", DiagnosticIDs::Note);
+}
   }
   BeginCurrent = EndCurrent;
 }

Added: 
clang-tools-extra/trunk/test/clang-tidy/bugprone-branch-clone-macro-crash.c
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-branch-clone-macro-crash.c?rev=366353=auto
==
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-branch-clone-macro-crash.c 
(added)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-branch-clone-macro-crash.c 
Wed Jul 17 10:22:43 2019
@@ -0,0 +1,14 @@
+// RUN: %check_clang_tidy %s bugprone-branch-clone %t
+int x = 0;
+int y = 1;
+#define a(b, c) \
+  typeof(b) d;  \
+  if (b)\
+d = b;  \
+  else if (c)   \
+d = b;
+
+f() {
+  // CHECK-MESSAGES: warning: repeated branch in conditional chain 
[bugprone-branch-clone]
+  a(x, y)
+}


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

[PATCH] D64607: [clang-tidy] Fix crash on end location inside macro

[Nathan Huckleberry via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366353: [clang-tidy] Fix crash on end location inside macro 
(authored by Nathan-Huckleberry, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64607?vs=209524=210364#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64607

Files:
  clang-tools-extra/trunk/clang-tidy/bugprone/BranchCloneCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/bugprone-branch-clone-macro-crash.c


Index: clang-tools-extra/trunk/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -132,9 +132,12 @@
   // We report the first occurence only when we find the second one.
   diag(Branches[i]->getBeginLoc(),
"repeated branch in conditional chain");
-  diag(Lexer::getLocForEndOfToken(Branches[i]->getEndLoc(), 0,
-  *Result.SourceManager, 
getLangOpts()),
-   "end of the original", DiagnosticIDs::Note);
+  SourceLocation End =
+  Lexer::getLocForEndOfToken(Branches[i]->getEndLoc(), 0,
+ *Result.SourceManager, getLangOpts());
+  if (End.isValid()) {
+diag(End, "end of the original", DiagnosticIDs::Note);
+  }
 }
 
 diag(Branches[j]->getBeginLoc(), "clone %0 starts here",
@@ -208,10 +211,12 @@
 
 if (EndLoc.isMacroID())
   EndLoc = Context.getSourceManager().getExpansionLoc(EndLoc);
+EndLoc = Lexer::getLocForEndOfToken(EndLoc, 0, *Result.SourceManager,
+getLangOpts());
 
-diag(Lexer::getLocForEndOfToken(EndLoc, 0, *Result.SourceManager,
-getLangOpts()),
- "last of these clones ends here", DiagnosticIDs::Note);
+if (EndLoc.isValid()) {
+  diag(EndLoc, "last of these clones ends here", DiagnosticIDs::Note);
+}
   }
   BeginCurrent = EndCurrent;
 }
Index: 
clang-tools-extra/trunk/test/clang-tidy/bugprone-branch-clone-macro-crash.c
===
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-branch-clone-macro-crash.c
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-branch-clone-macro-crash.c
@@ -0,0 +1,14 @@
+// RUN: %check_clang_tidy %s bugprone-branch-clone %t
+int x = 0;
+int y = 1;
+#define a(b, c) \
+  typeof(b) d;  \
+  if (b)\
+d = b;  \
+  else if (c)   \
+d = b;
+
+f() {
+  // CHECK-MESSAGES: warning: repeated branch in conditional chain 
[bugprone-branch-clone]
+  a(x, y)
+}


Index: clang-tools-extra/trunk/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -132,9 +132,12 @@
   // We report the first occurence only when we find the second one.
   diag(Branches[i]->getBeginLoc(),
"repeated branch in conditional chain");
-  diag(Lexer::getLocForEndOfToken(Branches[i]->getEndLoc(), 0,
-  *Result.SourceManager, getLangOpts()),
-   "end of the original", DiagnosticIDs::Note);
+  SourceLocation End =
+  Lexer::getLocForEndOfToken(Branches[i]->getEndLoc(), 0,
+ *Result.SourceManager, getLangOpts());
+  if (End.isValid()) {
+diag(End, "end of the original", DiagnosticIDs::Note);
+  }
 }
 
 diag(Branches[j]->getBeginLoc(), "clone %0 starts here",
@@ -208,10 +211,12 @@
 
 if (EndLoc.isMacroID())
   EndLoc = Context.getSourceManager().getExpansionLoc(EndLoc);
+EndLoc = Lexer::getLocForEndOfToken(EndLoc, 0, *Result.SourceManager,
+getLangOpts());
 
-diag(Lexer::getLocForEndOfToken(EndLoc, 0, *Result.SourceManager,
-getLangOpts()),
- "last of these clones ends here", DiagnosticIDs::Note);
+if (EndLoc.isValid()) {
+  diag(EndLoc, "last of these clones ends here", DiagnosticIDs::Note);
+}
   }
   BeginCurrent = EndCurrent;
 }
Index: clang-tools-extra/trunk/test/clang-tidy/bugprone-branch-clone-macro-crash.c
===
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-branch-clone-macro-crash.c
+++ 

[PATCH] D64418: [Docs][OpenCL] Documentation of C++ for OpenCL mode

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366351: [Docs][OpenCL] Documentation of C++ for OpenCL mode 
(authored by stulova, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64418?vs=210101=210362#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64418

Files:
  cfe/trunk/docs/LanguageExtensions.rst
  cfe/trunk/docs/UsersManual.rst

Index: cfe/trunk/docs/LanguageExtensions.rst
===
--- cfe/trunk/docs/LanguageExtensions.rst
+++ cfe/trunk/docs/LanguageExtensions.rst
@@ -1518,6 +1518,275 @@
 Query the presence of this new mangling with
 ``__has_feature(objc_protocol_qualifier_mangling)``.
 
+
+OpenCL Features
+===
+
+C++ for OpenCL
+--
+
+This functionality is built on top of OpenCL C v2.0 and C++17. Regular C++
+features can be used in OpenCL kernel code. All functionality from OpenCL C
+is inherited. This section describes minor differences to OpenCL C and any
+limitations related to C++ support as well as interactions between OpenCL and
+C++ features that are not documented elsewhere.
+
+Restrictions to C++17
+^
+
+The following features are not supported:
+
+- Virtual functions
+- ``dynamic_cast`` operator
+- Non-placement ``new``/``delete`` operators
+- Standard C++ libraries. Currently there is no solution for alternative C++
+  libraries provided. Future release will feature library support.
+
+
+Interplay of OpenCL and C++ features
+
+
+Address space behavior
+""
+
+Address spaces are part of the type qualifiers; many rules are just inherited
+from the qualifier behavior documented in OpenCL C v2.0 s6.5 and Embedded C
+extension ISO/IEC JTC1 SC22 WG14 N1021 s3.1. Note that since the address space
+behavior in C++ is not documented formally yet, Clang extends existing concept
+from C and OpenCL. For example conversion rules are extended from qualification
+conversion but the compatibility is determined using sets and overlapping from
+Embedded C (ISO/IEC JTC1 SC22 WG14 N1021 s3.1.3). For OpenCL it means that
+implicit conversions are allowed from named to ``__generic`` but not vice versa
+(OpenCL C v2.0 s6.5.5) except for ``__constant`` address space. Most of the
+rules are built on top of this behavior.
+
+**Casts**
+
+C style cast will follow OpenCL C v2.0 rules (s6.5.5). All cast operators will
+permit implicit conversion to ``__generic``. However converting from named
+address spaces to ``__generic`` can only be done using ``addrspace_cast``. Note
+that conversions between ``__constant`` and any other is still disallowed.
+
+.. _opencl_cpp_addrsp_deduction:
+
+**Deduction**
+
+Address spaces are not deduced for:
+
+- non-pointer/non-reference template parameters or any dependent types except
+  for template specializations.
+- non-pointer/non-reference class members except for static data members that are
+  deduced to ``__global`` address space.
+- non-pointer/non-reference alias declarations.
+- ``decltype`` expression.
+
+.. code-block:: c++
+
+  template 
+  void foo() {
+T m; // address space of m will be known at template instantiation time.
+T * ptr; // ptr points to __generic address space object.
+T & ref = ...; // ref references an object in __generic address space.
+  };
+
+  template 
+  struct S {
+int i; // i has no address space
+static int ii; // ii is in global address space
+int * ptr; // ptr points to __generic address space int.
+int & ref = ...; // ref references int in __generic address space.
+  };
+
+  template 
+  void bar()
+  {
+S s; // s is in __private address space
+  }
+
+TODO: Add example for type alias and decltype!
+
+**References**
+
+References types can be qualified with an address space.
+
+.. code-block:: c++
+
+  __private int & ref = ...; // references int in __private address space
+
+By default references will refer to ``__generic`` address space objects, except
+for dependent types that are not template specializations
+(see :ref:`Deduction `). Address space compatibility
+checks are performed when references are bound to values. The logic follows the
+rules from address space pointer conversion (OpenCL v2.0 s6.5.5).
+
+**Default address space**
+
+All non-static member functions take an implicit object parameter ``this`` that
+is a pointer type. By default this pointer parameter is in ``__generic`` address
+space. All concrete objects passed as an argument to ``this`` parameter will be
+converted to ``__generic`` address space first if the conversion is valid.
+Therefore programs using objects in ``__constant`` address space won't be compiled
+unless address space is explicitly specified using address space qualifiers on
+member functions
+(see 

[PATCH] D64780: Disallow most calling convention attributes on PS4.

[Warren Ristow via Phabricator via cfe-commits]

wristow added reviewers: rnk, rjmccall.
wristow added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:1271
 CCCR_Ignore,
+CCCR_Error,
   };

rnk wrote:
> I feel like perhaps a cleaner way of implementing this would be to have the 
> driver pass down -Werror=unknown-calling-conv (sp) on PS4. That would also 
> allow users to tell clang to ignore their CC annotations by disabling the 
> warning, which could be useful when porting a codebase widely annotated with 
> __stdcall, for example.
I can see the value of allowing users to ignore the CC annotations to ease a 
porting task, but we on PS4 prefer not to allow that.  We're probably more 
cautious than most about ABI incompatibilities, so if others decide to restrict 
use of unsupported CCs, they may very well want to go this route of the driver 
disabling the CCs.  But we'll still make them hard errors on PS4.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64780



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

r366351 - [Docs][OpenCL] Documentation of C++ for OpenCL mode

[Anastasia Stulova via cfe-commits]

Author: stulova
Date: Wed Jul 17 10:21:31 2019
New Revision: 366351

URL: http://llvm.org/viewvc/llvm-project?rev=366351=rev
Log:
[Docs][OpenCL] Documentation of C++ for OpenCL mode

Added documentation of C++ for OpenCL mode into Clang
User Manual and Language Extensions document.

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


Modified:
cfe/trunk/docs/LanguageExtensions.rst
cfe/trunk/docs/UsersManual.rst

Modified: cfe/trunk/docs/LanguageExtensions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=366351=366350=366351=diff
==
--- cfe/trunk/docs/LanguageExtensions.rst (original)
+++ cfe/trunk/docs/LanguageExtensions.rst Wed Jul 17 10:21:31 2019
@@ -1518,6 +1518,275 @@ parameters of protocol-qualified type.
 Query the presence of this new mangling with
 ``__has_feature(objc_protocol_qualifier_mangling)``.
 
+
+OpenCL Features
+===
+
+C++ for OpenCL
+--
+
+This functionality is built on top of OpenCL C v2.0 and C++17. Regular C++
+features can be used in OpenCL kernel code. All functionality from OpenCL C
+is inherited. This section describes minor differences to OpenCL C and any
+limitations related to C++ support as well as interactions between OpenCL and
+C++ features that are not documented elsewhere.
+
+Restrictions to C++17
+^
+
+The following features are not supported:
+
+- Virtual functions
+- ``dynamic_cast`` operator
+- Non-placement ``new``/``delete`` operators
+- Standard C++ libraries. Currently there is no solution for alternative C++
+  libraries provided. Future release will feature library support.
+
+
+Interplay of OpenCL and C++ features
+
+
+Address space behavior
+""
+
+Address spaces are part of the type qualifiers; many rules are just inherited
+from the qualifier behavior documented in OpenCL C v2.0 s6.5 and Embedded C
+extension ISO/IEC JTC1 SC22 WG14 N1021 s3.1. Note that since the address space
+behavior in C++ is not documented formally yet, Clang extends existing concept
+from C and OpenCL. For example conversion rules are extended from qualification
+conversion but the compatibility is determined using sets and overlapping from
+Embedded C (ISO/IEC JTC1 SC22 WG14 N1021 s3.1.3). For OpenCL it means that
+implicit conversions are allowed from named to ``__generic`` but not vice versa
+(OpenCL C v2.0 s6.5.5) except for ``__constant`` address space. Most of the
+rules are built on top of this behavior.
+
+**Casts**
+
+C style cast will follow OpenCL C v2.0 rules (s6.5.5). All cast operators will
+permit implicit conversion to ``__generic``. However converting from named
+address spaces to ``__generic`` can only be done using ``addrspace_cast``. Note
+that conversions between ``__constant`` and any other is still disallowed.
+
+.. _opencl_cpp_addrsp_deduction:
+
+**Deduction**
+
+Address spaces are not deduced for:
+
+- non-pointer/non-reference template parameters or any dependent types except
+  for template specializations.
+- non-pointer/non-reference class members except for static data members that 
are
+  deduced to ``__global`` address space.
+- non-pointer/non-reference alias declarations.
+- ``decltype`` expression.
+
+.. code-block:: c++
+
+  template 
+  void foo() {
+T m; // address space of m will be known at template instantiation time.
+T * ptr; // ptr points to __generic address space object.
+T & ref = ...; // ref references an object in __generic address space.
+  };
+
+  template 
+  struct S {
+int i; // i has no address space
+static int ii; // ii is in global address space
+int * ptr; // ptr points to __generic address space int.
+int & ref = ...; // ref references int in __generic address space.
+  };
+
+  template 
+  void bar()
+  {
+S s; // s is in __private address space
+  }
+
+TODO: Add example for type alias and decltype!
+
+**References**
+
+References types can be qualified with an address space.
+
+.. code-block:: c++
+
+  __private int & ref = ...; // references int in __private address space
+
+By default references will refer to ``__generic`` address space objects, except
+for dependent types that are not template specializations
+(see :ref:`Deduction `). Address space 
compatibility
+checks are performed when references are bound to values. The logic follows the
+rules from address space pointer conversion (OpenCL v2.0 s6.5.5).
+
+**Default address space**
+
+All non-static member functions take an implicit object parameter ``this`` that
+is a pointer type. By default this pointer parameter is in ``__generic`` 
address
+space. All concrete objects passed as an argument to ``this`` parameter will be
+converted to ``__generic`` address space first if the conversion is valid.
+Therefore programs using objects in ``__constant`` address space won't be 
compiled
+unless address space is 

[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry added a comment.

The main problem that we have is that the `__attribute__` token always causes 
the parser to read the line as a declaration. Then the declaration parser 
handles reading the attributes list.

This case demonstrates the problem:

  void foo() {
__attribute__((address_space(0))) *x;
  }

If the attribute token is read beforehand this code just becomes `*x` and is 
parsed as a dereference of an undefined variable when it should actually be a 
declaration.

Maybe the best solution is to pull the attributes parsing out to 
`ParseStatementOrDeclaration` then pass those attributes through to following 
functions. 
When the determination is made whether a line is a declaration or a statement 
the attributes list can be looked at to determine if the attributes are 
statement or declaration attributes and continue accordingly. Maybe throwing a 
warning if mixing of declaration and statement attributes occur.

Does that sound like a good solution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64838



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

[PATCH] D64518: [LibTooling] Relax Transformer to allow rewriting macro expansions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D64518#1588092 , @ymandel wrote:

> This seems like a good candidate for configuration -- the user could then 
> choose which mode to run in.  But, I'm also open to just reporting these 
> conditions as errors.  It's already in a context that returns Expected, so 
> its no trouble; it's just a matter of choosing what we think is "correct".


WRT to returning `Expected` vs `Optional`. Either seems fine and in the spirit 
of the library, depending on whether we want to produce more detailed errors. 
However, if we choose `Optional` let's stick to it, as practice shows switching 
from `Optional` to `Expected` correctly is almost impossible, as that requires 
a lot of attention to make sure all clients consume the errors (and given that 
it's an error case, tests often don't catch unconsumed errors).
I would personally go with `Optional` here (meaning the client code would have 
to say something generic like `could not map from macro expansion to source 
code`). But up to you, not a strong preference.

WRT to which cases we choose to handle, I'd start with a minimal number of 
supported examples (covering full macro expansion, or inside a single argument) 
and gradually add other cases as we find use-cases. What are your thoughts on 
that?




Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:76
+static llvm::Optional
+makeValidRange(CharSourceRange Range, const MatchResult ) {
+  const SourceManager  = *Result.SourceManager;

ymandel wrote:
> ilya-biryukov wrote:
> > Could we add unit tests for this particular function?
> > 
> > Interesting cases (`[[` and `]]` mark the start and end of a range):
> > ```
> > #define FOO(a) a+a;
> > #define BAR 10+
> > 
> > // change part of a macro argument
> > int a = FOO([[10]] + 10);
> > 
> > // change the whole macro expansion
> > int b = [[FOO(10+10)]];
> > 
> > // Try to change 10 inside 'BAR', but not '+'.
> > // Should this fail? Should we give a warning?
> > int c = BAR 3; 
> > 
> > // Try changing the lhs (10) of a binary expr, but not rhs.
> > // Is that allowed? Should we give a warning?
> > int d = FOO(10);
> > ```
> Sure. What do you think of exposing this function in 
> clang/include/clang/Tooling/Refactoring/SourceCode.h and testing it from 
> there?
Sounds reasonable. Was thinking of a better name, maybe something like 
`getRangeForEdit()`?
Would also suggest to accept `SourceManager` and `LangOptions` instead of 
`MatchResult` to narrow down the requirements on the clients.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64518



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

[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:170
+return;
+  Loc = SM.getSpellingLoc(Loc);
+}

hokein wrote:
> hokein wrote:
> > jvikstrom wrote:
> > > hokein wrote:
> > > > The Loc here maybe not in the main file, considering the case
> > > > 
> > > > ```
> > > > // in .h
> > > > #define DEFINE(X) in X;
> > > > #define DEFINE_Y DEFINE(Y)
> > > > 
> > > > // in .cc
> > > > 
> > > > DEFINE_Y
> > > > ```
> > > The spelling loc is still going to be in `DEFINE_Y` I think. And we only 
> > > highlight arguments in macros. (Added a testcase though)
> > ok, I don't have an exact case for macros now, but my gut feeling tells me 
> > we will encounter cases where the Loc is not in main file.
> > 
> > here is a test case for declarations, we will visit the `foo()` decl in 
> > `test.h` as well. This could be addressed in a separate patch.
> > 
> > ```
> > // test.h
> > void foo();
> > 
> > // test.cc
> > #include "test.h"
> > void foo() {}
> > ```
> > 
> ok, here is the case, the spelling loc is not in main file, it is in 
> ``
> 
> ```
> // test.h
> #DEFINE(X) class X {};
> #DEFINE_Y(Y) DEFINE(x##Y)
> 
> // test.cc
> DEFINE_Y(a);
> ```
You are correct. The other comment was actually correct as well, I just put the 
arguments for the header code in the place where the source code should be in.

Added a check in addToken that the Loc we are trying to add must be in the main 
file.



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:195
+  #define DEF_CLASS(T) class T {};
+  DEF_MULTIPLE(XYZ);
+  DEF_MULTIPLE(XYZW);

ilya-biryukov wrote:
> Could you add a comment explaining that we choose to not highlight the 
> conflicting tokens?
There is a comment in SemanticHighlighting.cpp at line 43. Want me to add a 
comment in the test case as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64741



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

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

[Manikishan Ghantasala via Phabricator via cfe-commits]

Manikishan updated this revision to Diff 210358.

Repository:
  rC Clang

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

https://reviews.llvm.org/D64695

Files:
  include/clang/Tooling/Inclusions/HeaderIncludes.h
  include/clang/Tooling/Inclusions/IncludeStyle.h
  lib/Format/Format.cpp
  lib/Tooling/Inclusions/HeaderIncludes.cpp
  lib/Tooling/Inclusions/IncludeStyle.cpp

Index: lib/Tooling/Inclusions/IncludeStyle.cpp
===
--- lib/Tooling/Inclusions/IncludeStyle.cpp
+++ lib/Tooling/Inclusions/IncludeStyle.cpp
@@ -17,6 +17,7 @@
 IO , IncludeStyle::IncludeCategory ) {
   IO.mapOptional("Regex", Category.Regex);
   IO.mapOptional("Priority", Category.Priority);
+  IO.mapOptional("SortPriority", Category.SortPriority);
 }
 
 void ScalarEnumerationTraits::enumeration(
Index: lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -199,6 +199,15 @@
   return Ret;
 }
 
+int IncludeCategoryManager::getSortIncludePriority(StringRef IncludeName) const {
+  int Ret = INT_MAX;
+  for (unsigned i = 0, e = CategoryRegexs.size(); i != e; ++i)
+if (CategoryRegexs[i].match(IncludeName)) {
+  Ret = Style.IncludeCategories[i].SortPriority;
+  break;
+}
+  return Ret;
+}
 bool IncludeCategoryManager::isMainHeader(StringRef IncludeName) const {
   if (!IncludeName.startswith("\""))
 return false;
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1023,6 +1023,39 @@
   return Style;
 }
 
+FormatStyle getNetBSDStyle() {
+  FormatStyle NetBSDStyle = getLLVMStyle();
+  NetBSDStyle.AlignTrailingComments = true;
+  NetBSDStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllDefinitions;
+  NetBSDStyle.AlignConsecutiveMacros = true;
+  NetBSDStyle.BreakBeforeBraces = FormatStyle::BS_Mozilla;
+  NetBSDStyle.ColumnLimit = 80;
+  NetBSDStyle.ContinuationIndentWidth = 4;
+  NetBSDStyle.Cpp11BracedListStyle = false;
+  NetBSDStyle.FixNamespaceComments = true;
+  NetBSDStyle.IndentCaseLabels = false;
+  NetBSDStyle.IndentWidth = 8;
+  NetBSDStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup;
+  NetBSDStyle.IncludeStyle.IncludeCategories = {
+  {"^", 1, 0},
+  {"^", 1, 1},
+  {"^", 9, 11},
+  {"^\".*\.h\"", 10, 12}};
+  NetBSDStyle.SortIncludes = true;
+  NetBSDStyle.TabWidth = 8;
+  NetBSDStyle.UseTab = FormatStyle::UT_Always;
+  return NetBSDStyle;
+}
+
 FormatStyle getNoStyle() {
   FormatStyle NoStyle = getLLVMStyle();
   NoStyle.DisableFormat = true;
@@ -1047,6 +1080,8 @@
 *Style = getGNUStyle();
   } else if (Name.equals_lower("microsoft")) {
 *Style = getMicrosoftStyle(Language);
+  } else if (Name.equals_lower("netbsd")) {
+*Style = getNetBSDStyle();
   } else if (Name.equals_lower("none")) {
 *Style = getNoStyle();
   } else {
@@ -1774,8 +1809,9 @@
 static void sortCppIncludes(const FormatStyle ,
 const SmallVectorImpl ,
 ArrayRef Ranges, StringRef FileName,
-StringRef Code,
-tooling::Replacements , unsigned *Cursor) {
+StringRef Code, tooling::Replacements ,
+unsigned *Cursor) {
+  tooling::IncludeCategoryManager Categories(Style.IncludeStyle, FileName);
   unsigned IncludesBeginOffset = Includes.front().Offset;
   unsigned IncludesEndOffset =
   Includes.back().Offset + Includes.back().Text.size();
@@ -1783,11 +1819,15 @@
   if (!affectsRange(Ranges, IncludesBeginOffset, IncludesEndOffset))
 return;
   SmallVector Indices;
-  for (unsigned i = 0, e = Includes.size(); i != e; ++i)
+  SmallVector IncludesPriority;
+  for (unsigned i = 0, e = Includes.size(); i != e; ++i) {
+IncludesPriority.push_back(
+Categories.getSortIncludePriority(Includes[i].Filename));
 Indices.push_back(i);
+  }
   llvm::stable_sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
-return std::tie(Includes[LHSI].Category, Includes[LHSI].Filename) <
-   std::tie(Includes[RHSI].Category, Includes[RHSI].Filename);
+return std::tie(IncludesPriority[LHSI], Includes[LHSI].Filename) <
+   std::tie(IncludesPriority[RHSI], Includes[RHSI].Filename);
   });
   // The index of the include on which the cursor will be put after
   // sorting/deduplicating.
Index: include/clang/Tooling/Inclusions/IncludeStyle.h
===
--- include/clang/Tooling/Inclusions/IncludeStyle.h
+++ include/clang/Tooling/Inclusions/IncludeStyle.h
@@ -58,6 +58,8 @@
 std::string Regex;
 /// The priority to assign to this category.
 int Priority;
+/// The custom priority to sort before grouping.
+int 

[PATCH] D64780: Disallow most calling convention attributes on PS4.

[Sunil Srivastava via Phabricator via cfe-commits]

Sunil_Srivastava removed reviewers: wristow, rnk, rjmccall.
Sunil_Srivastava marked an inline comment as done.
Sunil_Srivastava added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2961
+def error_cconv_unsupported : Error<
+  "%0 calling convention not supported %select{"
+  // Use CallingConventionIgnoredReason Enum to specify these.

rnk wrote:
> This duplication merely to say "not supported" instead of "ignored" seems 
> unfortunate. We could reasonable change "ignored" to "unsupported" and then 
> you could use the warn_cconv_ignored.Text accessor to share the text.
Good point. I will make a pre-commit of just changing the wording (and 
renaming) of warn_cconv_ignored. Then the warn_conv_unsupported.Text will be 
used in the actual change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64780



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

[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 210359.
jvikstrom marked 2 inline comments as done.
jvikstrom added a comment.

Ignore tokens outside of main file. Added testcase for assert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64741

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -29,9 +29,11 @@
   return Tokens;
 }
 
-void checkHighlightings(llvm::StringRef Code) {
+void checkHighlightings(llvm::StringRef Code, llvm::StringRef HeaderCode = "") {
   Annotations Test(Code);
-  auto AST = TestTU::withCode(Test.code()).build();
+  TestTU TU = TestTU::withCode(Test.code());
+  TU.HeaderCode = HeaderCode;
+  auto AST = TU.build();
   static const std::map KindToString{
   {HighlightingKind::Variable, "Variable"},
   {HighlightingKind::Function, "Function"},
@@ -186,10 +188,70 @@
   using $Enum[[CD]] = $Enum[[CC]];
   $Enum[[CC]] $Function[[f]]($Class[[B]]);
   $Enum[[CD]] $Function[[f]]($Class[[BB]]);
+)cpp",
+R"cpp(
+  #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
+  #define DEF_CLASS(T) class T {};
+  DEF_MULTIPLE(XYZ);
+  DEF_MULTIPLE(XYZW);
+  DEF_CLASS($Class[[A]])
+  #define MACRO_CONCAT(X, V, T) T foo##X = V
+  #define DEF_VAR(X, V) int X = V
+  #define DEF_VAR_T(T, X, V) T X = V
+  #define DEF_VAR_REV(V, X) DEF_VAR(X, V)
+  #define CPY(X) X
+  #define DEF_VAR_TYPE(X, Y) X Y
+  #define SOME_NAME variable
+  #define SOME_NAME_SET variable2 = 123
+  #define INC_VAR(X) X += 2
+  void $Function[[foo]]() {
+DEF_VAR($Variable[[X]],  123);
+DEF_VAR_REV(908, $Variable[[XY]]);
+int CPY( $Variable[[XX]] );
+DEF_VAR_TYPE($Class[[A]], $Variable[[AA]]);
+double SOME_NAME;
+int SOME_NAME_SET;
+$Variable[[variable]] = 20.1;
+MACRO_CONCAT(var, 2, float);
+DEF_VAR_T($Class[[A]], CPY(CPY($Variable[[Nested]])),
+  CPY($Class[[A]]()));
+INC_VAR($Variable[[variable]]);
+  }
+  void SOME_NAME();
+  DEF_VAR($Variable[[XYZ]], 567);
+  DEF_VAR_REV(756, $Variable[[AB]]);
+
+  #define CALL_FN(F) F();
+  #define DEF_FN(F) void F ()
+  DEF_FN($Function[[g]]) {
+CALL_FN($Function[[foo]]);
+  }
+)cpp", R"cpp(
+  #define fail(expr) expr
+  #define assert(COND) if (!(COND)) { fail("assertion failed" #COND); }
+  int $Variable[[x]];
+  int $Variable[[y]];
+  int $Function[[f]]();
+  void $Function[[foo]]() {
+assert($Variable[[x]] != $Variable[[y]]);
+assert($Variable[[x]] != $Function[[f]]());
+  }
 )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
   }
+
+  // A separate test for macros in headers.
+  checkHighlightings(R"cpp(
+DEFINE_Y
+DXYZ_Y(A);
+  )cpp",
+ R"cpp(
+#define DXYZ(X) class X {};
+#define DXYZ_Y(Y) DXYZ(x##Y)
+#define DEFINE(X) in X;
+#define DEFINE_Y DEFINE(Y)
+  )cpp");
 }
 
 TEST(SemanticHighlighting, GeneratesHighlightsWhenFileChange) {
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -39,6 +39,25 @@
});
 auto Last = std::unique(Tokens.begin(), Tokens.end());
 Tokens.erase(Last, Tokens.end());
+
+// Macros can give tokens that have the same source range but conflicting
+// kinds. In this case all tokens sharing this source range should be
+// removed.
+for (unsigned I = 0; I < Tokens.size(); ++I) {
+  ArrayRef TokRef(Tokens);
+  ArrayRef Conflicting =
+  llvm::ArrayRef(TokRef.begin() + I, TokRef.end())
+  .take_while([&](const HighlightingToken ) {
+return T.R == Tokens[I].R;
+  });
+
+  if (Conflicting.size() > 1) {
+Tokens.erase(Tokens.begin() + I,
+ Tokens.begin() + I + Conflicting.size());
+--I;
+  }
+}
+
 return Tokens;
   }
 
@@ -178,9 +197,19 @@
   }
 
   void addToken(SourceLocation Loc, HighlightingKind Kind) {
-if (Loc.isMacroID())
-  // FIXME: skip tokens inside macros for now.
+if(Loc.isMacroID()) {
+  // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
+  if (!SM.isMacroArgExpansion(Loc))
+return;
+  Loc = SM.getSpellingLoc(Loc);
+}
+
+if (!SM.isWrittenInMainFile(Loc)) {
+  // There are cases with macros where the 

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

[Manikishan Ghantasala via Phabricator via cfe-commits]

Manikishan added a comment.

In D64695#1589508 , @lebedev.ri wrote:

> Is there sufficient test coverage as to what happens if `SortPriority` is not 
> set?


If SortPriority is not set, the Includes will be grouped without sorting,

For example:

  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include "pathnames.h"
  #include 
  #include 
  #include 
  #include 

will be grouped as

  #include "pathnames.h"
  
  #include 
  #include 
  #include 
  
  #include 
  #include 
  #include 
  #include 
  
  #include 
  
  #include 
  
  #include 
  #include 
  
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 

Can we add a test case for this and mention that users should set SortPriority, 
or handle this case some how?
One way can be when the values are not set their Priority value is set as 
SortPriority, but this is also a problem 
when users give SortPriority as "0". Any comments on this issue?


Repository:
  rC Clang

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

https://reviews.llvm.org/D64695



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

[PATCH] D64860: [clangd] Get rid of dots and dotsdots within GlobalCompilationDatabase

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 210354.
kadircet marked 5 inline comments as done.
kadircet added a comment.

- Add removeDots helper to FS.h
- Revert changes in getFallbackCommands.
- Add comments for the reasoning behind removeDots calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64860

Files:
  clang-tools-extra/clangd/FS.cpp
  clang-tools-extra/clangd/FS.h
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

Index: clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -10,6 +10,7 @@
 
 #include "Path.h"
 #include "TestFS.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
@@ -31,6 +32,7 @@
 using ::testing::Contains;
 using ::testing::ElementsAre;
 using ::testing::EndsWith;
+using ::testing::HasSubstr;
 using ::testing::IsEmpty;
 using ::testing::Not;
 using ::testing::StartsWith;
@@ -247,9 +249,10 @@
 });
 
 File = FS.Root;
-llvm::sys::path::append(File, "a.cc");
+llvm::sys::path::append(File, "build", "..", "a.cc");
 DB.getCompileCommand(File.str());
-EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(EndsWith("a.cc")));
+EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(AllOf(
+ EndsWith("a.cc"), Not(HasSubstr("..");
 DiscoveredFiles.clear();
 
 File = FS.Root;
@@ -282,6 +285,27 @@
   }
 }
 
+TEST(GlobalCompilationDatabaseTest, NonCanonicalFilenames) {
+  OverlayCDB DB(nullptr);
+  std::vector DiscoveredFiles;
+  auto Sub =
+  DB.watch([](const std::vector Changes) {
+DiscoveredFiles = Changes;
+  });
+
+  llvm::SmallString<128> Root(testRoot());
+  llvm::sys::path::append(Root, "build", "..", "a.cc");
+  DB.setCompileCommand(Root.str(), tooling::CompileCommand());
+  EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(testPath("a.cc")));
+  DiscoveredFiles.clear();
+
+  llvm::SmallString<128> File(testRoot());
+  llvm::sys::path::append(File, "blabla", "..", "a.cc");
+
+  EXPECT_TRUE(DB.getCompileCommand(File));
+  EXPECT_TRUE(DB.getProjectInfo(File));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "GlobalCompilationDatabase.h"
+#include "FS.h"
 #include "Logger.h"
 #include "Path.h"
 #include "clang/Frontend/CompilerInvocation.h"
@@ -15,6 +16,7 @@
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include 
@@ -147,12 +149,15 @@
   getCDBInDirLocked(*CompileCommandsDir);
   Result.PI.SourceRoot = *CompileCommandsDir;
 } else {
-  actOnAllParentDirectories(
-  Request.FileName, [this, , ](PathRef Path) {
-std::tie(Result.CDB, SentBroadcast) = getCDBInDirLocked(Path);
-Result.PI.SourceRoot = Path;
-return Result.CDB != nullptr;
-  });
+  // Traverse the canonical version to prevent false positives. i.e.:
+  // src/build/../a.cc can detect a CDB in /src/build if not canonicalized.
+  actOnAllParentDirectories(removeDots(Request.FileName),
+[this, , ](PathRef Path) {
+  std::tie(Result.CDB, SentBroadcast) =
+  getCDBInDirLocked(Path);
+  Result.PI.SourceRoot = Path;
+  return Result.CDB != nullptr;
+});
 }
 
 if (!Result.CDB)
@@ -209,7 +214,8 @@
 actOnAllParentDirectories(File, [&](PathRef Path) {
   if (DirectoryHasCDB.lookup(Path)) {
 if (Path == Result.PI.SourceRoot)
-  GovernedFiles.push_back(File);
+  // Make sure listeners always get a canonical path for the file.
+  GovernedFiles.push_back(removeDots(File));
 // Stop as soon as we hit a CDB.
 return true;
   }
@@ -248,7 +254,7 @@
   llvm::Optional Cmd;
   {
 std::lock_guard Lock(Mutex);
-auto It = Commands.find(File);
+auto It = Commands.find(removeDots(File));
 if (It != Commands.end())
   Cmd = It->second;
   }
@@ -272,20 +278,24 @@
 
 void OverlayCDB::setCompileCommand(
 

[PATCH] D64860: [clangd] Get rid of dots and dotsdots within GlobalCompilationDatabase

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp:77
 GlobalCompilationDatabase::getFallbackCommand(PathRef File) const {
+  llvm::SmallString<128> CanonPath(File);
+  llvm::sys::path::remove_dots(CanonPath, true);

sammccall wrote:
> This patch lacks a bit of context (why are we changing this behavior).
> Based on offline discussion, it's not clear to me why this particular change 
> to getFallbackCommand is necessary or desirable. (Compared to others, which 
> seem like fairly obvious bugs, even if the reason to fix them isn't obvious)
updating the summary for reasoning



Comment at: 
clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp:307
+  EXPECT_TRUE(DB.getProjectInfo(File));
+  EXPECT_EQ(DB.getFallbackCommand(File).Directory, testRoot());
+}

sammccall wrote:
> Is this actually important/desirable?
yea agreed this is not necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64860



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

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

[Manikishan Ghantasala via Phabricator via cfe-commits]

Manikishan updated this revision to Diff 210353.

Repository:
  rC Clang

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

https://reviews.llvm.org/D64695

Files:
  include/clang/Tooling/Inclusions/HeaderIncludes.h
  include/clang/Tooling/Inclusions/IncludeStyle.h
  lib/Format/Format.cpp
  lib/Tooling/Inclusions/HeaderIncludes.cpp
  lib/Tooling/Inclusions/IncludeStyle.cpp

Index: lib/Tooling/Inclusions/IncludeStyle.cpp
===
--- lib/Tooling/Inclusions/IncludeStyle.cpp
+++ lib/Tooling/Inclusions/IncludeStyle.cpp
@@ -17,6 +17,7 @@
 IO , IncludeStyle::IncludeCategory ) {
   IO.mapOptional("Regex", Category.Regex);
   IO.mapOptional("Priority", Category.Priority);
+  IO.mapOptional("SortPriority", Category.SortPriority);
 }
 
 void ScalarEnumerationTraits::enumeration(
Index: lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -199,6 +199,15 @@
   return Ret;
 }
 
+int IncludeCategoryManager::getSortIncludePriority(StringRef IncludeName) const {
+  int Ret = INT_MAX;
+  for (unsigned i = 0, e = CategoryRegexs.size(); i != e; ++i)
+if (CategoryRegexs[i].match(IncludeName)) {
+  Ret = Style.IncludeCategories[i].SortPriority;
+  break;
+}
+  return Ret;
+}
 bool IncludeCategoryManager::isMainHeader(StringRef IncludeName) const {
   if (!IncludeName.startswith("\""))
 return false;
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1023,6 +1023,39 @@
   return Style;
 }
 
+FormatStyle getNetBSDStyle() {
+  FormatStyle NetBSDStyle = getLLVMStyle();
+  NetBSDStyle.AlignTrailingComments = true;
+  NetBSDStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllDefinitions;
+  NetBSDStyle.AlignConsecutiveMacros = true;
+  NetBSDStyle.BreakBeforeBraces = FormatStyle::BS_Mozilla;
+  NetBSDStyle.ColumnLimit = 80;
+  NetBSDStyle.ContinuationIndentWidth = 4;
+  NetBSDStyle.Cpp11BracedListStyle = false;
+  NetBSDStyle.FixNamespaceComments = true;
+  NetBSDStyle.IndentCaseLabels = false;
+  NetBSDStyle.IndentWidth = 8;
+  NetBSDStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup;
+  NetBSDStyle.IncludeStyle.IncludeCategories = {
+  {"^", 1, 0},
+  {"^", 1, 1},
+  {"^", 9, 11},
+  {"^\"\w.*\.h\"$", 10, 12}};
+  NetBSDStyle.SortIncludes = true;
+  NetBSDStyle.TabWidth = 8;
+  NetBSDStyle.UseTab = FormatStyle::UT_Always;
+  return NetBSDStyle;
+}
+
 FormatStyle getNoStyle() {
   FormatStyle NoStyle = getLLVMStyle();
   NoStyle.DisableFormat = true;
@@ -1047,6 +1080,8 @@
 *Style = getGNUStyle();
   } else if (Name.equals_lower("microsoft")) {
 *Style = getMicrosoftStyle(Language);
+  } else if (Name.equals_lower("netbsd")) {
+*Style = getNetBSDStyle();
   } else if (Name.equals_lower("none")) {
 *Style = getNoStyle();
   } else {
@@ -1774,8 +1809,9 @@
 static void sortCppIncludes(const FormatStyle ,
 const SmallVectorImpl ,
 ArrayRef Ranges, StringRef FileName,
-StringRef Code,
-tooling::Replacements , unsigned *Cursor) {
+StringRef Code, tooling::Replacements ,
+unsigned *Cursor) {
+  tooling::IncludeCategoryManager Categories(Style.IncludeStyle, FileName);
   unsigned IncludesBeginOffset = Includes.front().Offset;
   unsigned IncludesEndOffset =
   Includes.back().Offset + Includes.back().Text.size();
@@ -1783,11 +1819,15 @@
   if (!affectsRange(Ranges, IncludesBeginOffset, IncludesEndOffset))
 return;
   SmallVector Indices;
-  for (unsigned i = 0, e = Includes.size(); i != e; ++i)
+  SmallVector IncludesPriority;
+  for (unsigned i = 0, e = Includes.size(); i != e; ++i) {
+IncludesPriority.push_back(
+Categories.getSortIncludePriority(Includes[i].Filename));
 Indices.push_back(i);
+  }
   llvm::stable_sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
-return std::tie(Includes[LHSI].Category, Includes[LHSI].Filename) <
-   std::tie(Includes[RHSI].Category, Includes[RHSI].Filename);
+return std::tie(IncludesPriority[LHSI], Includes[LHSI].Filename) <
+   std::tie(IncludesPriority[RHSI], Includes[RHSI].Filename);
   });
   // The index of the include on which the cursor will be put after
   // sorting/deduplicating.
Index: include/clang/Tooling/Inclusions/IncludeStyle.h
===
--- include/clang/Tooling/Inclusions/IncludeStyle.h
+++ include/clang/Tooling/Inclusions/IncludeStyle.h
@@ -58,6 +58,8 @@
 std::string Regex;
 /// The priority to assign to this category.
 int Priority;
+/// The custom priority to sort before grouping.
+int 

[PATCH] D60335: Use -fomit-frame-pointer when optimizing PowerPC code

[Justin Hibbits via Phabricator via cfe-commits]

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

Looks fine to me.  Since it can be turned off, I don't see a problem if it 
causes issues.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60335



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

[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

Thanks for getting back Hans!

In D63648#1589119 , @hans wrote:

> Are you saying the diagnostics were not using absolute paths in those cases 
> and this patch fixes that?


Yes.

>> , or --show-includes, or -E
> 
> Those aren't diagnostics, so that's not surprising.

What would suggest in that case? Add a new `-fpreprocessor-absolute-paths` 
option? Or change the name of `-fdiagnostics-absolute-paths` for another name 
that applies to both diagnostics and the preprocessor output?

>> or displaying notes.
> 
> What notes?

That is, the additional output starting with `note:` issued along error 
messages: `t.cc:4:5: note: candidate function not viable: no known conversion 
from 'vector>' to 'vector>' for 1st 
argument;`
Sometimes, notes are displaying an extra path, sometimes it's just the note 
alone.

>> We have a peculiar use-case on our end with Fastbuild, where all this was 
>> exposed: CPP files are being are preprocessed on one PC, then compiled on 
>> another PC (which doesn't have the source-code); then the compiler's stdout 
>> is displayed on the first PC.
> 
> And what is the final problem? That diagnostics from the compiler's stdout 
> are not absolute because they came from the preprocessed code that doesn't 
> include the absolute paths?

Exactly. When clicking on a warning/error, those relative paths prevent Visual 
Studio from jumping to the right location.

Please let me know if further clarification is needed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63648



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

[PATCH] D62584: [OpenCL][PR42033] Deducing addr space with template parameter types

[John McCall via Phabricator via cfe-commits]

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

Thanks!


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

https://reviews.llvm.org/D62584



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2549-2552
+  FunctionStr = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(
+  {FD->getBeginLoc(), FD->getBody()->getBeginLoc()}),
+  C.getSourceManager(), C.getLangOpts());

NoQ wrote:
> I'm slightly worried that it'll crash when `free()` is being called from 
> within a body farm.
> 
> For now it probably cannot happen because none of the bodyfarmed functions 
> can call `free()` directly, but i'd anyway rather add a check that the source 
> locations we're taking are valid.
Oh, I missed that, thanks! I wanted to check for everything, yes.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2554-2559
+  if (FunctionStr.equals(""))
+return false;
+
+  // We do not model the Integer Set Library's retain-count based allocation.
+  if (!FunctionStr.contains("__isl_"))
+return false;

NoQ wrote:
> If the string is empty, it clearly cannot contain `__isl_`, so the first 
> check is redundant.
The first check is: "We got a body and its decl?", the second check is: "We got 
an ISL macro?". Yea, it is kind of redundant, just I like to pack one check in 
one IfStmt, and now that two question merges. Also I like to make them 
one-liners so they are self-explanatory.

Here is an example why I like it:
```
// Escape pointers passed into the list, unless it's an ObjC boxed  
 
// expression which is not a boxable C structure.   
 
if (!(isa(Ex) && 
 
  !cast(Ex)->getSubExpr()
 
  ->getType()->isRecordType()))
```
- from `ExprEngine::Visit()` - `Expr::ObjCBoxedExprClass` case.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2569
+  if (const RefState *RS = State->get(Sym)) {
+State = State->remove(Sym);
+State = State->set(Sym, RefState::getEscaped(RS));

NoQ wrote:
> `remove` is unnecessary, we overwrite it anyway.
I believe in so as well, just the official code base has this semantic. I have 
rewritten that, see below in `checkPointerEscapeAux()`.


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

https://reviews.llvm.org/D64680



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

[PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

[Angus Hewlett via Phabricator via cfe-commits]

angushewlett added a comment.

In D52193#1589657 , @aganea wrote:

>




> In D52193#1589150 , @angushewlett 
> wrote:
> 
>> They're issued in the right order, but the second doesn't wait for the first 
>> to complete.
> 
> 
> Ok, I thought the wait was handled by MSBuild. If I understand correctly what 
> you're saying, MSBuild issues both commands at the same time, but the second 
> `cl.exe /Yu /MP` somehow waits for the first `cl.exe /Yc` to complete? (maybe 
> by checking if the PCH is still open for write?)

That appears to be the case, yes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D52193



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

[PATCH] D64739: [SVE][Inline-Asm] Add support to clang for SVE inline assembly

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

Functionally the patch looks good, but the title suggests this adds full 
inline-asm support for SVE (which would require the ACLE types proposed in 
D62960 , as well as other changes), where this 
patch only adds support to specify SVE registers in the clobber list.




Comment at: clang/test/CodeGen/aarch64-sve-inline-asm.c:4
+long test_z0_p0()
+{
+  long t;

nit: there is no reason to have a different code-style for code and tests 
(curly brace is on next line here).
Maybe run this through clang-format?



Comment at: clang/test/CodeGen/aarch64-sve-inline-asm.c:7
+
+  asm volatile(
+"ptrue p0.d\n"

nit: The asm/instructions here don't really need to make sense (as in: they are 
not executed), so you can combine all three tests into one, as long as the 
instructions are valid and z0, p0, z31 and z15 are used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64739



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 210347.
Charusso marked 9 inline comments as done.
Charusso added a comment.

- Fix.


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

https://reviews.llvm.org/D64680

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/test/Analysis/retain-count-alloc.cpp

Index: clang/test/Analysis/retain-count-alloc.cpp
===
--- /dev/null
+++ clang/test/Analysis/retain-count-alloc.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix.Malloc \
+// RUN:  -verify %s
+
+// expected-no-diagnostics: We do not model Integer Set Library's retain-count
+//  based allocation. If any of the parameters has an
+//  '__isl_' prefixed macro definition we escape every
+//  of them when we are about to 'free()' something.
+
+#define __isl_take
+#define __isl_keep
+
+struct Object { int Ref; };
+void free(void *);
+
+Object *copyObj(__isl_keep Object *O) {
+  O->Ref++;
+  return O;
+}
+
+void freeObj(__isl_take Object *O) {
+  if (--O->Ref > 0)
+return;
+
+  free(O); // Here we notice that the parameter contains '__isl_', escape it.
+}
+
+void useAfterFree(__isl_take Object *A) {
+  if (!A)
+return;
+
+  Object *B = copyObj(A);
+  freeObj(B);
+
+  A->Ref = 13;
+  // no-warning: 'Use of memory after it is freed' was here.
+}
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/ParentMap.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -359,6 +360,9 @@
   /// Check if the memory associated with this symbol was released.
   bool isReleased(SymbolRef Sym, CheckerContext ) const;
 
+  /// Check whether we do not model the memory allocation.
+  bool isNotModeled(const CallExpr *CE, CheckerContext ) const;
+
   bool checkUseAfterFree(SymbolRef Sym, CheckerContext , const Stmt *S) const;
 
   void checkUseZeroAllocated(SymbolRef Sym, CheckerContext ,
@@ -877,6 +881,9 @@
   State = ProcessZeroAllocation(C, CE, 0, State);
   State = ProcessZeroAllocation(C, CE, 1, State);
 } else if (FunI == II_free || FunI == II_g_free || FunI == II_kfree) {
+  if (isNotModeled(CE, C))
+return;
+
   State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
 } else if (FunI == II_strdup || FunI == II_win_strdup ||
FunI == II_wcsdup || FunI == II_win_wcsdup) {
@@ -2532,6 +2539,40 @@
   return (RS && RS->isReleased());
 }
 
+bool MallocChecker::isNotModeled(const CallExpr *CE, CheckerContext ) const {
+  if (CE->getNumArgs() == 0)
+return false;
+
+  StringRef FunctionStr = "";
+  if (const auto *FD = dyn_cast(C.getStackFrame()->getDecl()))
+if (const Stmt *Body = FD->getBody())
+  FunctionStr =
+  Lexer::getSourceText(CharSourceRange::getTokenRange(
+   {FD->getBeginLoc(), Body->getBeginLoc()}),
+   C.getSourceManager(), C.getLangOpts());
+
+  // We do not model the Integer Set Library's retain-count based allocation.
+  if (!FunctionStr.contains("__isl_"))
+return false;
+
+  bool IsEscaped = false;
+  ProgramStateRef State = C.getState();
+
+  for (const Expr *Arg : CE->arguments()) {
+if (SymbolRef Sym = C.getSVal(Arg).getAsSymbol()) {
+  if (const RefState *RS = State->get(Sym)) {
+State = State->set(Sym, RefState::getEscaped(RS));
+IsEscaped = true;
+  }
+}
+  }
+
+  if (IsEscaped)
+C.addTransition(State);
+
+  return true;
+}
+
 bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext ,
   const Stmt *S) const {
 
@@ -2833,7 +2874,6 @@
 if (const RefState *RS = State->get(sym)) {
   if ((RS->isAllocated() || RS->isAllocatedOfSizeZero()) &&
   CheckRefState(RS)) {
-State = State->remove(sym);
 State = State->set(sym, RefState::getEscaped(RS));
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64075: [ASTImporter] Fix structural eq of lambdas

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Jenkins looks okay: 
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31160/ .The build is 
red but the the previous run has the very same failing test case:
expression_command/weak_symbols/TestWeakSymbols.py


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64075



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

[PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In D52193#1589096 , @yurybura wrote:

> Do you have any plans to make this PR compatible with trunk? Now MSVC with 
> /MP builds much faster than clang-cl (at least 2x faster for our project)...


I'll get back to this after the vacation (somewhere in August)

In D52193#1589150 , @angushewlett 
wrote:

> They're issued in the right order, but the second doesn't wait for the first 
> to complete.


Ok, I thought the wait was handled by MSBuild. If I understand correctly what 
you're saying, MSBuild issues both commands at the same time, but the second 
`cl.exe /Yu /MP` somehow waits for the first `cl.exe /Yc` to complete? (maybe 
by checking if the PCH is still open for write?)


Repository:
  rC Clang

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

https://reviews.llvm.org/D52193



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

[PATCH] D64801: [analyzer] Add CTU user docs

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 210345.
martong marked 3 inline comments as done.
martong added a comment.

- Address dkrupp's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64801

Files:
  clang/docs/analyzer/user-docs.rst
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst

Index: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
===
--- /dev/null
+++ clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
@@ -0,0 +1,193 @@
+=
+Cross Translation Unit (CTU) Analysis
+=
+
+Normally, static analysis works in the boundary of one translation unit (TU).
+However, with additional steps and configuration we can enable the analysis to inline the definition of a function from another TU.
+
+.. contents::
+   :local:
+
+Manual CTU Analysis
+---
+
+Let's consider these source files in our minimal example:
+
+.. code-block:: cpp
+
+  // main.cpp
+  int foo();
+
+  int main() {
+return 3 / foo();
+  }
+
+.. code-block:: cpp
+
+  // foo.cpp
+  int foo() {
+return 0;
+  }
+
+And a compilation database:
+
+.. code-block:: bash
+
+  [
+{
+  "directory": "/path/to/your/project",
+  "command": "clang++ -c foo.cpp -o foo.o",
+  "file": "foo.cpp"
+},
+{
+  "directory": "/path/to/your/project",
+  "command": "clang++ -c main.cpp -o main.o",
+  "file": "main.cpp"
+}
+  ]
+
+We'd like to analyze `main.cpp` and discover the division by zero bug.
+In order to be able to inline the definition of `foo` from `foo.cpp` first we have to generate the `AST` (or `PCH`) file of `foo.cpp`:
+
+.. code-block:: bash
+
+  $ pwd $ /path/to/your/project
+  $ clang++ -emit-ast -o foo.cpp.ast foo.cpp
+  $ # Check that the .ast file is generated:
+  $ ls
+  compile_commands.json  foo.cpp.ast  foo.cpp  main.cpp
+  $
+
+The next step is to create a CTU index file which holds the `USR` name and location of external definitions in the source files:
+
+.. code-block:: bash
+
+  $ clang-extdef-mapping -p . foo.cpp
+  c:@F@foo# /path/to/your/project/foo.cpp
+  $ clang-extdef-mapping -p . foo.cpp > externalDefMap.txt
+
+We have to modify `externalDefMap.txt` to contain the name of the `.ast` files instead of the source files:
+
+.. code-block:: bash
+
+  $ sed -i -e "s/.cpp/.cpp.ast/g" externalDefMap.txt
+
+We still have to further modify the `externalDefMap.txt` file to contain relative paths:
+
+.. code-block:: bash
+
+  $ sed -i -e "s|$(pwd)/||g" externalDefMap.txt
+
+Now everything is available for the CTU analysis.
+We have to feed Clang with CTU specific extra arguments:
+
+.. code-block:: bash
+
+  $ pwd
+  /path/to/your/project
+  $ clang++ --analyze -Xclang -analyzer-config -Xclang experimental-enable-naive-ctu-analysis=true -Xclang -analyzer-config -Xclang ctu-dir=. -Xclang -analyzer-output=plist-multi-file main.cpp
+  main.cpp:5:12: warning: Division by zero
+return 3 / foo();
+   ~~^~~
+  1 warning generated.
+  $ # The plist file with the result is generated.
+  $ ls
+  compile_commands.json  externalDefMap.txt  foo.ast  foo.cpp  foo.cpp.ast  main.cpp  main.plist
+  $
+
+This manual procedure is error-prone and not scalable, therefore to analyze real projects it is recommended to use `CodeChecker` or `scan-build-py`.
+
+Automated CTU Analysis with CodeChecker
+---
+The `CodeChecker `_ project fully supports automated CTU analysis with Clang.
+Once we have set up the `PATH` environment variable and we activated the python `venv` then it is all it takes:
+
+.. code-block:: bash
+
+  $ CodeChecker analyze --ctu compile_commands.json -o reports
+  [INFO 2019-07-16 17:21] - Pre-analysis started.
+  [INFO 2019-07-16 17:21] - Collecting data for ctu analysis.
+  [INFO 2019-07-16 17:21] - [1/2] foo.cpp
+  [INFO 2019-07-16 17:21] - [2/2] main.cpp
+  [INFO 2019-07-16 17:21] - Pre-analysis finished.
+  [INFO 2019-07-16 17:21] - Starting static analysis ...
+  [INFO 2019-07-16 17:21] - [1/2] clangsa analyzed foo.cpp successfully.
+  [INFO 2019-07-16 17:21] - [2/2] clangsa analyzed main.cpp successfully.
+  [INFO 2019-07-16 17:21] -  Summary 
+  [INFO 2019-07-16 17:21] - Successfully analyzed
+  [INFO 2019-07-16 17:21] -   clangsa: 2
+  [INFO 2019-07-16 17:21] - Total analyzed compilation commands: 2
+  [INFO 2019-07-16 17:21] - =
+  [INFO 2019-07-16 17:21] - Analysis finished.
+  [INFO 2019-07-16 17:21] - To view results in the terminal use the "CodeChecker parse" command.
+  [INFO 2019-07-16 17:21] - To store results use the "CodeChecker store" command.
+  [INFO 2019-07-16 17:21] - See --help and the user guide for further options about parsing and storing the reports.
+  [INFO 2019-07-16 17:21] - 

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

[Jussi Pakkanen via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp:21
+  Finder->addMatcher(
+  varDecl(unless(hasInitializer(anything(.bind("vardecl"), this);
+}

alexfh wrote:
> jpakkane wrote:
> > alexfh wrote:
> > > jpakkane wrote:
> > > > alexfh wrote:
> > > > > I believe, this should skip matches within template instantiations. 
> > > > > Consider this code:
> > > > > ```
> > > > > template
> > > > > void f(T) { T t; }
> > > > > void g() {
> > > > > f(0);
> > > > > f(0.0);
> > > > > }
> > > > > ```
> > > > > 
> > > > > What will the fix  be?
> > > > I tested with the following function:
> > > > 
> > > > 
> > > > ```
> > > > template
> > > > void template_test_function() {
> > > >   T t;
> > > >   int uninitialized;
> > > > }
> > > > ```
> > > > 
> > > > Currently it warns on the "uninitialized" variable regardless of 
> > > > whether the template is instantiated or not. If you call it with an int 
> > > > type, it will warn about variable t being uninitialized. If you call it 
> > > > with a, say, struct type, there is no warnings. Is this a reasonable 
> > > > approach?
> > > And what happens, if there are multiple instantiations of the same 
> > > template, each of them requiring a different fix? Can you try the check 
> > > with my example above (and maybe also add `f("");`inside `g()`). I 
> > > believe, the check will produce multiple warnings with conflicting fixes 
> > > (and each of them will be wrong, btw).
> > Interestingly it does warn about it, but only once, even if you have two 
> > different template specializations.
> > 
> > I tried to suppress this warning when the type being instantiated is a 
> > template argument type but no matter what I tried I could not get this to 
> > work. Is there a way to get this information from the MatchedDecl object or 
> > does one need to do something more complicated like going up the AST until 
> > a function definition is found and checking if it is a template 
> > specialization (presumably with TemplatedKind)? Any help would be 
> > appreciated.
> If there are multiple warnings with the same message at the same location 
> (clang-tidy/ClangTidyDiagnosticConsumer.cpp:745), they will be deduplicated. 
> Thus, a random fix will probably be suggested. The proper way to filter out 
> matches in template instantiations is to add 
> `unless(isInTemplateInstantiation())` to the matcher.
I tried to make this work but I just could not combine statement and 
declaration matching in a reliable way. Matching a statement that is not in a 
template declaration can be done, as well as matching a declaration without 
intial value, but combining those two into one is hard. After trying many, many 
things the best I could come up with was this:

```
declStmt(containsDeclaration(0, 
varDecl(unless(hasInitializer(anything(.bind("vardecl"))), this)
```

The problem is that `containsDeclaration` takes an integer denoting how manyth 
declaration should be processed. Manually adding matchers for, say, 0, 1, 2, 3 
and 4 works and does the right thing but fails if anyone has an uninitialized 
variable in the sixth location, things will silently fail.

The weird thing is that if you do the matching this way, you don't need to 
filter out things with `unless(isInTemplateInstantiation())`. Maybe statements 
are handled differently from declarations?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D64671



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