[PATCH] D83836: [Analyzer] Implementing checkRegionChanges for SmartPtrModeling

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Additionally, I would prefer commit message to be imperative.  It is sorta like 
a rule of a good commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83836



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

[PATCH] D77062: [analyzer] Improved zero assumption in CStringChecke::assumeZero

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

It is a good practice in many projects to make commit messages into imperative 
(i.e. "Improve" instead of "Improving" or "Improved").  Again, not everyone 
follows it, but it is good to keep it consistent, right?

@NoQ knock-knock!




Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:199
   // Utility methods
-  std::pair
-  static assumeZero(CheckerContext &C,
-ProgramStateRef state, SVal V, QualType Ty);
+  std::pair static assumeZero(
+  ProgramStateRef state, SVal V);

Can you please put `static` before return type, it will be more consistent with 
other static methods nearby.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:283-288
+  auto states = std::make_pair(state, state);
+
   Optional val = V.getAs();
-  if (!val)
-return std::pair(state, state);
+  // FIXME: We should understand how LazyCompoundVal can be possible here,
+  // fix the root cause and get rid of this check.
+  if (val && !V.getAs())

I know that other methods here don't name variables according to llvm-style 
guide.  Analyzer's code is one of the least complying areas of the whole LLVM 
project.  However, I suggest not to make it worse and capitalize all parameter 
and local variable names.


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

https://reviews.llvm.org/D77062



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

[PATCH] D83877: [Analyzer] Changed in SmartPtrModeling to handle Swap

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

The same thing with the commit message here, it would be better as just "Change"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83877



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

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D83717#2152762 , @gamesh411 wrote:

> In D83717#2150977 , @njames93 wrote:
>
> > Alternatively you could do something like this, though it would be a pain 
> > https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment-gmock.cpp#L86
>
>
> I have bitten the bullet, and have gone down this route. With relative 
> numbering, the sections themselves are at least translation-invariant. Not 
> the prettiest sight, tho.
>  Thanks!


I almost think it would be nice FileCheck supported some directive like:

  // LINE-NAME: 

And corresponding check lines:

  [[]]
  [[+N]] 
  [[-N]]

Would result in the ability to write checks like this:

  void longjmp_handler() {// LINE-NAME: LONGJMP
  longjmp(env, 255); 
  }
  
  ...
  
  void foo(){
atexit(longjmp_handler);
 // CHECK-NOTES: :[[@LINE-1]]:3: warning: exit-handler potentially calls a 
jump function. Handlers should terminate by returning [cert-env32-c]
// CHECK-NOTES: :[[LONGJMP]]:1: note: handler function declared here
// CHECK-NOTES: :[[LONGJMP+1]]:3: note: jump function called here
  }

Anyway that's a story for another day.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717



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

[clang-tools-extra] 2743322 - [clangd] Fix a few gcc warnings [NFC]

[Mikael Holmen via cfe-commits]

Author: Mikael Holmen
Date: 2020-07-16T09:36:17+02:00
New Revision: 274332282cb4ce167de8e73fb9c59d2eecd67c25

URL: 
https://github.com/llvm/llvm-project/commit/274332282cb4ce167de8e73fb9c59d2eecd67c25
DIFF: 
https://github.com/llvm/llvm-project/commit/274332282cb4ce167de8e73fb9c59d2eecd67c25.diff

LOG: [clangd] Fix a few gcc warnings [NFC]

Added: 


Modified: 
clang-tools-extra/clangd/CompileCommands.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CompileCommands.cpp 
b/clang-tools-extra/clangd/CompileCommands.cpp
index 473122157cac..f6210a43b34e 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -271,6 +271,7 @@ std::pair getArgCount(const 
llvm::opt::Option &Opt) {
   case Option::RemainingArgsJoinedClass:
 return {Rest, Rest};
   }
+  llvm_unreachable("Unhandled option kind");
 }
 
 // Flag-parsing mode, which affects which flags are available.
@@ -321,7 +322,7 @@ unsigned char getModes(const llvm::opt::Option &Opt) {
 }
   }
   return Result;
-};
+}
 
 } // namespace
 
@@ -475,7 +476,7 @@ void ArgStripper::process(std::vector &Args) 
const {
   bool WasXclang = false;
   while (Read < Args.size()) {
 unsigned ArgCount = 0;
-if (const Rule *R = matchingRule(Args[Read], CurrentMode, ArgCount)) {
+if (matchingRule(Args[Read], CurrentMode, ArgCount)) {
   // Delete it and its args.
   if (WasXclang) {
 assert(Write > 0);



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

[PATCH] D83295: [Analyzer] Hotfix for various crashes in iterator checkers

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

baloghadamsoftware updated this revision to Diff 278394.
baloghadamsoftware added a comment.

Protection against `Unknown` added for pointer increments and decrements. No 
more crashes on `LLVM/Clang`.


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

https://reviews.llvm.org/D83295

Files:
  clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
  clang/test/Analysis/iterator-modeling.cpp
  clang/test/Analysis/iterator-range.cpp

Index: clang/test/Analysis/iterator-range.cpp
===
--- clang/test/Analysis/iterator-range.cpp
+++ clang/test/Analysis/iterator-range.cpp
@@ -935,3 +935,7 @@
   // expected-note@-1{{Iterator decremented ahead of its valid range}}
 }
 
+void ptr_iter_diff(cont_with_ptr_iterator &c) {
+  auto i0 = c.begin(), i1 = c.end();
+  ptrdiff_t len = i1 - i0; // no-crash
+}
Index: clang/test/Analysis/iterator-modeling.cpp
===
--- clang/test/Analysis/iterator-modeling.cpp
+++ clang/test/Analysis/iterator-modeling.cpp
@@ -1948,6 +1948,13 @@
   clang_analyzer_express(clang_analyzer_iterator_position(i)); // expected-warning{{$c.end() - 2}}
 }
 
+void minus_equal_ptr_iterator_variable(const cont_with_ptr_iterator &c,
+   int n) {
+  auto i = c.end();
+
+  i -= n; // no-crash
+}
+
 void plus_ptr_iterator(const cont_with_ptr_iterator &c) {
   auto i1 = c.begin();
 
@@ -1972,6 +1979,17 @@
   clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning{{$c.end() - 2}}
 }
 
+void ptr_iter_diff(cont_with_ptr_iterator &c) {
+  auto i0 = c.begin(), i1 = c.end();
+  ptrdiff_t len = i1 - i0; // no-crash
+}
+
+void ptr_iter_cmp_nullptr(cont_with_ptr_iterator &c) {
+  auto i0 = c.begin();
+  if (i0 != nullptr) // no-crash
+++i0;
+}
+
 void clang_analyzer_printState();
 
 void print_state(std::vector &V) {
Index: clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
@@ -169,6 +169,8 @@
 verifyDereference(C, LVal);
   } else if (isRandomIncrOrDecrOperator(OK)) {
 SVal RVal = State->getSVal(BO->getRHS(), C.getLocationContext());
+if (!BO->getRHS()->getType()->isIntegralOrEnumerationType())
+  return;
 verifyRandomIncrOrDecr(C, BinaryOperator::getOverloadedOperator(OK), LVal,
RVal);
   }
Index: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
@@ -272,6 +272,8 @@
 handleComparison(C, BO, Result, LVal, RVal,
  BinaryOperator::getOverloadedOperator(OK));
   } else if (isRandomIncrOrDecrOperator(OK)) {
+if (!BO->getRHS()->getType()->isIntegralOrEnumerationType())
+  return;
 handlePtrIncrOrDecr(C, BO->getLHS(),
 BinaryOperator::getOverloadedOperator(OK), RVal);
   }
@@ -461,6 +463,12 @@
 RPos = getIteratorPosition(State, RVal);
   }
 
+  // If the value for which we just tried to set a new iterator position is
+  // an `SVal`for which no iterator position can be set then the setting was
+  // unsuccessful. We cannot handle the comparison in this case.
+  if (!LPos || !RPos)
+return;
+
   // We cannot make assumptions on `UnknownVal`. Let us conjure a symbol
   // instead.
   if (RetVal.isUnknown()) {
@@ -599,6 +607,9 @@
const Expr *Iterator,
OverloadedOperatorKind OK,
SVal Offset) const {
+  if (!Offset.getAs())
+return;
+
   QualType PtrType = Iterator->getType();
   if (!PtrType->isPointerType())
 return;
@@ -612,13 +623,11 @@
 return;
 
   SVal NewVal;
-  if (OK == OO_Plus || OK == OO_PlusEqual)
+  if (OK == OO_Plus || OK == OO_PlusEqual) {
 NewVal = State->getLValue(ElementType, Offset, OldVal);
-  else {
-const llvm::APSInt &OffsetInt =
-  Offset.castAs().getValue();
-auto &BVF = C.getSymbolManager().getBasicVals();
-SVal NegatedOffset = nonloc::ConcreteInt(BVF.getValue(-OffsetInt));
+  } else {
+auto &SVB = C.getSValBuilder();
+SVal NegatedOffset = SVB.evalMinus(Offset.castAs());
 NewVal = State->getLValue(ElementType, NegatedOffset, OldVal);
   }
 
@@ -684,9 +693,14 @@
 
   const auto StateBefore = N->getState();
   const auto *PosBefore = getIteratorPosition(StateBefore, Iter);
-
-  assert(PosBefore && "`std::advance() should not create new iterator "
- "position but change existing ones");
+  // FIXME: `std::advance()` should not create a new iterator position

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added a comment.

In D83717#2155066 , @njames93 wrote:

> In D83717#2152762 , @gamesh411 wrote:
>
> > In D83717#2150977 , @njames93 
> > wrote:
> >
> > > Alternatively you could do something like this, though it would be a pain 
> > > https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment-gmock.cpp#L86
> >
> >
> > I have bitten the bullet, and have gone down this route. With relative 
> > numbering, the sections themselves are at least translation-invariant. Not 
> > the prettiest sight, tho.
> >  Thanks!
>
>
> I almost think it would be nice FileCheck supported some directive like:
>
>   // LINE-NAME: 
>
>
> And corresponding check lines:
>
>   [[]]
>   [[+N]] 
>   [[-N]]
>
>
> Would result in the ability to write checks like this:
>
>   void longjmp_handler() {// LINE-NAME: LONGJMP
>   longjmp(env, 255); 
>   }
>  
>   ...
>  
>   void foo(){
> atexit(longjmp_handler);
>  // CHECK-NOTES: :[[@LINE-1]]:3: warning: exit-handler potentially calls 
> a jump function. Handlers should terminate by returning [cert-env32-c]
> // CHECK-NOTES: :[[LONGJMP]]:1: note: handler function declared here
> // CHECK-NOTES: :[[LONGJMP+1]]:3: note: jump function called here
>   }
>
>
> Anyway that's a story for another day.


My thoughts exactly! I also thought about anchor-points as a feature in 
file-check, as that would immensely increase the readability of the test-code 
in such cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717



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

[PATCH] D77125: [Analyzer] Model return values of container insert and delete operations

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

baloghadamsoftware updated this revision to Diff 278395.
baloghadamsoftware added a comment.

Protection agains `Unknown` return values added.


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

https://reviews.llvm.org/D77125

Files:
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
  clang/test/Analysis/Inputs/system-header-simulator-cxx.h
  clang/test/Analysis/diagnostics/explicit-suppression.cpp
  clang/test/Analysis/iterator-modeling.cpp

Index: clang/test/Analysis/iterator-modeling.cpp
===
--- clang/test/Analysis/iterator-modeling.cpp
+++ clang/test/Analysis/iterator-modeling.cpp
@@ -873,9 +873,9 @@
   clang_analyzer_eval(clang_analyzer_iterator_validity(i0)); //expected-warning{{TRUE}}
   clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{TRUE}}
 
-  clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$L.begin(){{$
-  // clang_analyzer_express(clang_analyzer_iterator_position(i2)); FIXME: expect warning $L.begin() - 1
-  clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$L.end(){{$
+  clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning{{$L.begin()}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning{{$L.end()}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning{{$L.begin()}}
 }
 
 void list_insert_behind_begin(std::list &L, int n) {
@@ -890,10 +890,10 @@
   clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{TRUE}}
   clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{TRUE}}
 
-  clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$L.begin(){{$ FIXME: Should be $L.begin() - 1
-  clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$L.begin() + 1{{$
-  // clang_analyzer_express(clang_analyzer_iterator_position(i3)); FIXME: expect warning $L.begin()
-  clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$L.end(){{$
+  clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning{{$L.begin()}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning{{$L.begin() + 1}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning{{$L.end()}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i3)); // expected-warning{{$L.begin()}}
 }
 
 template  Iter return_any_iterator(const Iter &It);
@@ -911,10 +911,10 @@
   clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{TRUE}}
   clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{TRUE}}
 
-  clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$L.begin(){{$
-  clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$i1{{$
-  // clang_analyzer_express(clang_analyzer_iterator_position(i3)); FIXME: expect warning $i - 1
-  clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$L.end(){{$
+  clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning{{$L.begin()}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning{{$i1}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning{{$L.end()}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i3)); // expected-warning{{$i1}}
 }
 
 void list_insert_ahead_of_end(std::list &L, int n) {
@@ -929,10 +929,10 @@
   clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{TRUE}}
   clang_analyzer_eval(clang_analyzer_iterator_validity(i2)); //expected-warning{{TRUE}}
 
-  clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning-re {{$L.begin(){{$
-  clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning-re {{$L.end() - 1{{$
-  clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning-re {{$L.end(){{$
-  // clang_analyzer_express(clang_analyzer_iterator_position(i3)); FIXME: expect warning $L.end() - 2
+  clang_analyzer_express(clang_analyzer_iterator_position(i0)); // expected-warning{{$L.begin()}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i1)); // expected-warning{{$L.end() - 1}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i2)); // expected-warning{{$L.end()}}
+  clang_analyzer_express(clang_analyzer_iterator_position(i3)); // expected-warning{{$L.end() - 1}}
 }
 
 void list_insert_end(std::list &L, int n) {
@@ -947,10 +947,10 @@
   clang_analyzer_eval(clang_analyzer_iterator_validity(i1)); //expected-warning{{TRUE}}
   clang_analyzer_eval(clang_analyzer_

[PATCH] D77150: [Analyzer] New Option for ContainerModeling: AggressiveEraseModeling

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

baloghadamsoftware updated this revision to Diff 278396.
baloghadamsoftware added a comment.

Rebased.


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

https://reviews.llvm.org/D77150

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/iterator-range-aggressive-erase-modeling.cpp

Index: clang/test/Analysis/iterator-range-aggressive-erase-modeling.cpp
===
--- /dev/null
+++ clang/test/Analysis/iterator-range-aggressive-erase-modeling.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_analyze_cc1 -std=c++11 \
+// RUN: -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange \
+// RUN: -analyzer-config aggressive-binary-operation-simplification=true \
+// RUN: -analyzer-config c++-container-inlining=false %s \
+// RUN: -analyzer-config alpha.cplusplus.ContainerModeling:AggressiveEraseModeling=true \
+// RUN: -verify
+
+// RUN: %clang_analyze_cc1 -std=c++11 \
+// RUN: -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange \
+// RUN: -analyzer-config aggressive-binary-operation-simplification=true \
+// RUN: -analyzer-config c++-container-inlining=true -DINLINE=1 %s \
+// RUN: -analyzer-config alpha.cplusplus.ContainerModeling:AggressiveEraseModeling=true \
+// RUN: -verify
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+unsigned int __line, __const char *__function)
+ __attribute__ ((__noreturn__));
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+void bad_erase_loop(std::list L) {
+  for (auto i = L.cbegin(); i != L.end(); ++i) { // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+i = L.erase(i);
+  }
+}
+
+void bad_erase_loop(std::vector V) {
+  for (auto i = V.cbegin(); i != V.end(); ++i) { // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+i = V.erase(i);
+  }
+}
+void bad_erase_loop(std::deque D) {
+  for (auto i = D.cbegin(); i != D.end(); ++i) { // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+i = D.erase(i);
+  }
+}
Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -7,6 +7,7 @@
 // CHECK-NEXT: alpha.clone.CloneChecker:IgnoredFilesPattern = ""
 // CHECK-NEXT: alpha.clone.CloneChecker:MinimumCloneComplexity = 50
 // CHECK-NEXT: alpha.clone.CloneChecker:ReportNormalClones = true
+// CHECK-NEXT: alpha.cplusplus.ContainerModeling:AggressiveEraseModeling = false
 // CHECK-NEXT: alpha.cplusplus.STLAlgorithmModeling:AggressiveStdFindModeling = false
 // CHECK-NEXT: alpha.osx.cocoa.DirectIvarAssignment:AnnotatedFunctions = false
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
Index: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
@@ -44,16 +44,16 @@
   void handlePopBack(CheckerContext &C, SVal Cont, const Expr *ContE) const;
   void handlePushFront(CheckerContext &C, SVal Cont, const Expr *ContE) const;
   void handlePopFront(CheckerContext &C, SVal Cont, const Expr *ContE) const;
-  void handleInsert(CheckerContext &C, SVal Cont, SVal Iter,
-SVal RetVal) const;
-  void handleInsertAfter(CheckerContext &C, SVal Cont, SVal Iter,
- SVal RetVal) const;
-  void handleErase(CheckerContext &C, SVal Cont, SVal Iter,
-SVal RetVal) const;
+  void handleInsert(CheckerContext &C, SVal Cont, SVal Iter, SVal RetVal,
+const Expr *CE) const;
+  void handleInsertAfter(CheckerContext &C, SVal Cont, SVal Iter, SVal RetVal,
+ const Expr *CE) const;
+  void handleErase(CheckerContext &C, SVal Cont, SVal Iter, SVal RetVal,
+   const Expr *CE) const;
   void handleErase(CheckerContext &C, SVal Cont, SVal Iter1, SVal Iter2,
 SVal RetVal) const;
-  void handleEraseAfter(CheckerContext &C, SVal Cont, SVal Iter,
-SVal RetVal) const;
+  void handleEraseAfter(CheckerContext &C, SVal Cont, SVal Iter, SVal RetVal,
+const Expr *CE) const;
   void handleEraseAfter(CheckerContext &C, SVal Cont, SVal Iter1,
 SVal Iter2, SVal RetVal) const;
   const NoteTag *getChangeTag(CheckerContext &C, StringRef Text,
@@ -65,14 +65,16 @@
 public:
   ContainerModeling() = default;
 
+  DefaultBool AggressiveEraseModeling;
+
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   void checkLiveSymbols(ProgramStateRef State

[PATCH] D83901: [clang] Disable a few formatting options for test/

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

clang-format -n warnings before this change in clang/test/Analysis/*.cpp  
(clang-format -n *.cpp |& grep warning | wc -l)

before = 6903 vs  after=6595, if it helps I'd say this looks good. Most a very 
brief review of changes seem to be mainly whitespace/extra lines and 
indentation  (44 in headers files vs 43)
(Note: There are 11 warnings after the first iteration, clang-format sometimes 
has to be run twice)

The real proof is how many of the unit tests can be clang-formatted and still 
pass, but I completely approve of this effort, if the test directories were 
formatted this would give us a massive test suite of lots of different examples 
of c++ code to validate any clang-format changes.

If having this in the clang-format file means users can use "Format on Save" 
options in their editors and not break tests then I think this is a good thing. 
As this will drive good behavior in all the other files rather than having to 
turn "Format on Save" off because we cannot maintain running tests.

My 2 cents worth.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83901



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

[PATCH] D82089: [clang-tidy] modernize-loop-convert reverse iteration support

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 278398.
njames93 added a comment.

Updated release notes for version bump.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82089

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
  clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
@@ -0,0 +1,120 @@
+// RUN: %check_clang_tidy -std=c++20 -check-suffix=RANGES %s modernize-loop-convert %t
+
+// RUN: %check_clang_tidy -check-suffix=CUSTOM %s modernize-loop-convert %t -- \
+// RUN:   -config="{CheckOptions: [ \
+// RUN:   {key: modernize-loop-convert.MakeReverseRangeFunction, value: 'llvm::reverse'}, \
+// RUN:   {key: modernize-loop-convert.MakeReverseRangeHeader, value: 'llvm/ADT/STLExtras.h'}]}"
+
+// Ensure the check doesn't transform reverse loops when not in c++20 mode or
+// when UseCxx20ReverseRanges has been disabled
+// RUN: clang-tidy %s -checks=-*,modernize-loop-convert -- -std=c++17 | count 0
+
+// RUN: clang-tidy %s -checks=-*,modernize-loop-convert -config="{CheckOptions: \
+// RUN: [{key: modernize-loop-convert.UseCxx20ReverseRanges, value: 'false'}] \
+// RUN: }" -- -std=c++20 | count 0
+
+// RUN: %check_clang_tidy -check-suffix=CUSTOM-NO-HEADER %s modernize-loop-convert %t -- \
+// RUN:   -config="{CheckOptions: [ \
+// RUN:   {key: modernize-loop-convert.MakeReverseRangeFunction, value: 'globalReverse'}]}"
+
+// Ensure we get a warning if we only supply one of the required reverse range arguments.
+// RUN: %check_clang_tidy -check-suffix=BAD-CUSTOM %s modernize-loop-convert %t -- \
+// RUN:   -config="{CheckOptions: [ \
+// RUN:   {key: modernize-loop-convert.MakeReverseRangeHeader, value: 'ranges/v3/views/reverse.hpp'}]}"
+
+// CHECK-MESSAGES-BAD-CUSTOM: warning: modernize-loop-convert: 'MakeReverseRangeHeader' is set but 'MakeReverseRangeFunction' is not, disabling reverse loop transformation
+
+// Make sure appropiate headers are included
+// CHECK-FIXES-RANGES: #include 
+// CHECK-FIXES-CUSTOM: #include "llvm/ADT/STLExtras.h"
+
+// Make sure no header is included in this example
+// CHECK-FIXES-CUSTOM-NO-HEADER-NOT: #include
+
+template 
+struct reversable {
+  using iterator = T *;
+  using const_iterator = const T *;
+
+  iterator begin();
+  iterator end();
+  iterator rbegin();
+  iterator rend();
+
+  const_iterator begin() const;
+  const_iterator end() const;
+  const_iterator rbegin() const;
+  const_iterator rend() const;
+
+  const_iterator cbegin() const;
+  const_iterator cend() const;
+  const_iterator crbegin() const;
+  const_iterator crend() const;
+};
+
+template 
+void observe(const T &);
+template 
+void mutate(T &);
+
+void constContainer(const reversable &Numbers) {
+  // CHECK-MESSAGES-RANGES: :[[@LINE+3]]:3: warning: use range-based for loop instead
+  // CHECK-MESSAGES-CUSTOM: :[[@LINE+2]]:3: warning: use range-based for loop instead
+  // CHECK-MESSAGES-CUSTOM-NO-HEADER: :[[@LINE+1]]:3: warning: use range-based for loop instead
+  for (auto I = Numbers.rbegin(), E = Numbers.rend(); I != E; ++I) {
+observe(*I);
+//  CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
+// CHECK-FIXES-RANGES-NEXT: observe(Number);
+//  CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
+// CHECK-FIXES-CUSTOM-NEXT: observe(Number);
+//  CHECK-FIXES-CUSTOM-NO-HEADER: for (int Number : globalReverse(Numbers)) {
+// CHECK-FIXES-CUSTOM-NO-HEADER-NEXT: observe(Number);
+  }
+  // CHECK-MESSAGES-RANGES: :[[@LINE+3]]:3: warning: use range-based for loop instead
+  // CHECK-MESSAGES-CUSTOM: :[[@LINE+2]]:3: warning: use range-based for loop instead
+  // CHECK-MESSAGES-CUSTOM-NO-HEADER: :[[@LINE+1]]:3: warning: use range-based for loop instead
+  for (auto I = Numbers.crbegin(), E = Numbers.crend(); I != E; ++I) {
+observe(*I);
+//  CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
+// CHECK-FIXES-RANGES-NEXT: observe(Number);
+//  CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
+// CHECK-FIXES-CUSTOM-NEXT: observe(Number);
+//  CHECK-FIXES-CUSTOM-NO-HEADER: for (int Number : globalReverse(Numbers)) {
+// CHECK-FIXES-CUSTOM-NO-HEADER-NEXT: observe(Number);
+  }
+
+  // Ensure these bad loops aren't transformed.
+  for (auto I = Numbers.rbegin(), E = Numbers.end(); I != E; ++I) {
+observe(*I);
+  }
+  for (auto I = Numbers.begin(), E = 

[PATCH] D83174: Teach AttachPreviousImpl to inherit MSInheritanceAttr attribute

[Vaibhav Garg via Phabricator via cfe-commits]

gargvaibhav64 updated this revision to Diff 278399.
gargvaibhav64 added a comment.

The tests weren't failing for me. So, we are possibly missing test coverage. I 
have made the change now.

Also, I would like to add that the current test present in this diff does not 
fail in the existing system but it was the best I and Vassil could come up with 
to replicate our problem.


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

https://reviews.llvm.org/D83174

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/Modules/Inputs/inherit-attribute/a.h
  clang/test/Modules/Inputs/inherit-attribute/b.h
  clang/test/Modules/Inputs/inherit-attribute/c.h
  clang/test/Modules/Inputs/inherit-attribute/module.modulemap
  clang/test/Modules/inherit-attribute.cpp

Index: clang/test/Modules/inherit-attribute.cpp
===
--- /dev/null
+++ clang/test/Modules/inherit-attribute.cpp
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -ast-dump -I%S/Inputs/inherit-attribute -fmodules-cache-path=%t -fimplicit-module-maps -verify %s -fmodules-local-submodule-visibility
+
+#include "b.h"
+#include "c.h"
+
+class Foo;
+
+Foo f;
+// expected-no-diagnostics
Index: clang/test/Modules/Inputs/inherit-attribute/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/inherit-attribute/module.modulemap
@@ -0,0 +1,3 @@
+module "b" { header "b.h" }
+
+module "c" { header "c.h" }
Index: clang/test/Modules/Inputs/inherit-attribute/c.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/inherit-attribute/c.h
@@ -0,0 +1,7 @@
+#include "a.h"
+
+class Foo;
+class C {
+public:
+  C();
+};
Index: clang/test/Modules/Inputs/inherit-attribute/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/inherit-attribute/b.h
@@ -0,0 +1,12 @@
+#include "a.h"
+
+class Foo;
+
+void bar() {
+  &Foo::step;
+}
+
+class B {
+public:
+  B();
+};
Index: clang/test/Modules/Inputs/inherit-attribute/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/inherit-attribute/a.h
@@ -0,0 +1,10 @@
+#ifndef FOO
+#define FOO
+
+class Foo {
+public:
+  void step(int v);
+  Foo();
+};
+
+#endif
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -281,6 +281,9 @@
 static Decl *getMostRecentDeclImpl(...);
 static Decl *getMostRecentDecl(Decl *D);
 
+static void mergeInheritableAttributes(ASTReader &Reader, Decl *D,
+   Decl *Previous);
+
 template 
 static void attachPreviousDeclImpl(ASTReader &Reader,
Redeclarable *D, Decl *Previous,
@@ -3531,6 +3534,19 @@
   return ASTDeclReader::getMostRecentDecl(D->getCanonicalDecl());
 }
 
+void ASTDeclReader::mergeInheritableAttributes(ASTReader &Reader, Decl *D,
+   Decl *Previous) {
+  InheritableAttr *NewAttr = nullptr;
+  ASTContext &Context = Reader.getContext();
+  const auto *IA = Previous->getAttr();
+
+  if (IA && !D->hasAttr()) {
+NewAttr = cast(IA->clone(Context));
+NewAttr->setInherited(true);
+D->addAttr(NewAttr);
+  }
+}
+
 template
 void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader,
Redeclarable *D,
@@ -3689,6 +3705,12 @@
   if (auto *TD = dyn_cast(D))
 inheritDefaultTemplateArguments(Reader.getContext(),
 cast(Previous), TD);
+
+  // If any of the declaration in the chain contains an Inheritable attribute,
+  // it needs to be added to all the declarations in the redeclarable chain.
+  // FIXME: Only the logic of merging MSInheritableAttr is present, it should
+  // be extended for all inheritable attributes.
+  mergeInheritableAttributes(Reader, D, Previous);
 }
 
 template
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 90798e0 - Re-enable "[InstCombine] Simplify boolean Phis with const inputs using CFG"

[Max Kazantsev via cfe-commits]

Author: Max Kazantsev
Date: 2020-07-16T16:09:08+07:00
New Revision: 90798e09e29012fe316565ea04a1f848c5e40e6e

URL: 
https://github.com/llvm/llvm-project/commit/90798e09e29012fe316565ea04a1f848c5e40e6e
DIFF: 
https://github.com/llvm/llvm-project/commit/90798e09e29012fe316565ea04a1f848c5e40e6e.diff

LOG: Re-enable "[InstCombine] Simplify boolean Phis with const inputs using CFG"

This reverts commit b893822e32ffe3c1dcf4d5ac0571a282582d72b2.

+ Clang test fixes
+ Insertion point fix for landing pads

Added: 


Modified: 
clang/test/CodeGenObjC/exceptions.m
clang/test/CodeGenObjCXX/exceptions-legacy.mm
clang/test/CodeGenOpenCL/convergent.cl
llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
llvm/test/Transforms/CallSiteSplitting/callsite-split.ll
llvm/test/Transforms/InstCombine/branch.ll
llvm/test/Transforms/InstCombine/icmp-constant-phi.ll
llvm/test/Transforms/InstCombine/phi.ll
llvm/test/Transforms/InstCombine/select.ll
llvm/test/Transforms/InstCombine/simple_phi_condition.ll
llvm/test/Transforms/PhaseOrdering/simplifycfg-options.ll

Removed: 




diff  --git a/clang/test/CodeGenObjC/exceptions.m 
b/clang/test/CodeGenObjC/exceptions.m
index 3bb4f86cf025..55a117bcc3dd 100644
--- a/clang/test/CodeGenObjC/exceptions.m
+++ b/clang/test/CodeGenObjC/exceptions.m
@@ -25,12 +25,12 @@ void f1() {
 // CHECK-NEXT: icmp
 // CHECK-NEXT: br i1
 @try {
+// CHECK:  call void asm sideeffect "", "=*m"
 // CHECK:  call void asm sideeffect "", "*m"
 // CHECK-NEXT: call void @foo()
   foo();
 // CHECK:  call void @objc_exception_try_exit
 
-// CHECK:  call void asm sideeffect "", "=*m"
 } @finally {
   break;
 }
@@ -53,14 +53,6 @@ int f2() {
   // CHECK-NEXT:   [[CAUGHT:%.*]] = icmp eq i32 [[SETJMP]], 0
   // CHECK-NEXT:   br i1 [[CAUGHT]]
   @try {
-// CHECK: store i32 6, i32* [[X]]
-x++;
-// CHECK-NEXT: call void asm sideeffect "", "*m,*m"(i32* nonnull [[X]]
-// CHECK-NEXT: call void @foo()
-// CHECK-NEXT: call void @objc_exception_try_exit
-// CHECK-NEXT: [[T:%.*]] = load i32, i32* [[X]]
-foo();
-  } @catch (id) {
 // Landing pad.  Note that we elide the re-enter.
 // CHECK:  call void asm sideeffect "", "=*m,=*m"(i32* nonnull [[X]]
 // CHECK-NEXT: call i8* @objc_exception_extract
@@ -69,6 +61,15 @@ int f2() {
 
 // This store is dead.
 // CHECK-NEXT: store i32 [[T2]], i32* [[X]]
+
+// CHECK: store i32 6, i32* [[X]]
+x++;
+// CHECK-NEXT: call void asm sideeffect "", "*m,*m"(i32* nonnull [[X]]
+// CHECK-NEXT: call void @foo()
+// CHECK-NEXT: call void @objc_exception_try_exit
+// CHECK-NEXT: [[T:%.*]] = load i32, i32* [[X]]
+foo();
+  } @catch (id) {
 x--;
   }
 
@@ -89,23 +90,23 @@ void f3() {
 
   // CHECK:  call void @objc_exception_try_enter(
   // CHECK:  call i32 @_setjmp
-  // CHECK-NEXT: icmp eq
-  // CHECK-NEXT: br i1
+  // CHECK-NEXT: [[DEST1:%.*]] = icmp eq
+  // CHECK-NEXT: br i1 [[DEST1]]
 
   @try {
 // CHECK:call void @f3_helper(i32 0, i32* nonnull [[X]])
 // CHECK:call void @objc_exception_try_exit(
 f3_helper(0, &x);
   } @finally {
-// CHECK:[[DEST1:%.*]] = phi i1 [ true, {{%.*}} ], [ false, {{%.*}} ]
 // CHECK:call void @objc_exception_try_enter
 // CHECK:call i32 @_setjmp
+// CHECK-NEXT: [[DEST2:%.*]] = icmp eq
+// CHECK-NEXT: br i1 [[DEST2]]
 @try {
   // CHECK:  call void @f3_helper(i32 1, i32* nonnull [[X]])
   // CHECK:  call void @objc_exception_try_exit(
   f3_helper(1, &x);
 } @finally {
-  // CHECK:  [[DEST2:%.*]] = phi i1 [ true, {{%.*}} ], [ false, {{%.*}} ]
   // CHECK:  call void @f3_helper(i32 2, i32* nonnull [[X]])
   f3_helper(2, &x);
 

diff  --git a/clang/test/CodeGenObjCXX/exceptions-legacy.mm 
b/clang/test/CodeGenObjCXX/exceptions-legacy.mm
index bfc8d640b710..563569478679 100644
--- a/clang/test/CodeGenObjCXX/exceptions-legacy.mm
+++ b/clang/test/CodeGenObjCXX/exceptions-legacy.mm
@@ -63,18 +63,20 @@ void test1(id obj, bool *failed) {
 //   Body.
 // CHECK:  invoke void @_Z3foov()
 
+//   Catch handler.  Reload of 'failed' address is unnecessary.
+// CHECK:  [[T0:%.*]] = load i8*, i8**
+// CHECK-NEXT: store i8 1, i8* [[T0]],
+// CHECK-NEXT: br label
+
 //   Leave the @try.
 // CHECK:  call void @objc_exception_try_exit([[BUF_T]]* nonnull [[BUF]])
 // CHECK-NEXT: br label
 // CHECK:  ret void
 
+
 //   Real EH cleanup.
 // CHECK:  [[T0:%.*]] = landingpad
 // CHECK-NEXT:cleanup
 // CHECK-NEXT: call void @objc_exception_try_exit([[BUF_T]]* nonnull [[BUF]])
 // CHECK-NEXT: resume
 
-//   Catch handler.  Reload of 'failed' address is unnecessary.
-// CHECK:  [[T0:%.*]] = load i8*, i8**
-// CHECK-NEXT: store i8 1, i8* [[T0]],
-// CHECK-NEXT: br label

diff  --git a/clang/test/CodeGenOpenCL/convergen

[PATCH] D80802: [RISCV] Upgrade RVV MC to v0.9.

[Hsiangkai Wang via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:99
 // load vd, (rs1), vm
 class VUnitStrideLoad

fpallares wrote:
> I believe that with the changes introduced in the encoding of the loads and 
> stores we can do without the `mop` parameter in most of the classes here:
> 
> | class   | replace `mop` by   |
> |---|--|
> | `VUnitStrideLoad` | `MOPLDUnitStride` (i.e. `00`) |
> | `VStridedLoad` | `MOPLDStrided` (i.e. `10`) |
> | `VIndexedLoad` | `MOPLDIndexed` (i.e. `11`) |
> | `VUnitStrideStore` | `MOPSTUnitStride` (i.e. `00`) |
> | `VStridedStore` | `MOPLDStrided` (i.e. `10`) |
> 
> We still need to keep the parameter for the `VIndexedStore` class since it 
> can take `MOPSTIndexedOrder` (i.e. `11`) or `MOPSTIndexedUnord` (i.e. `01`).
> 
> Does this make sense to you?
It makes sense. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80802



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

[PATCH] D83099: [clangd] Store index in '.cache/clangd/index' instead of '.clangd/index'

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D83099#2154416 , @davidvancleve 
wrote:

> Thanks for the quick response! Nope, no compile_commands.json files, and they 
> didn't previously contain .clangd files.


Well, thanks for reporting this, this seems like a really bad bug, and we'd 
like to get it fixed on the release branch ASAP.

(Just to check - also no compile_flags.txt or other compilation database 
markers?)

Our best guess is this is a fairly long-standing bug in OverlayCDB: the storage 
location would be ".cache/clangd/index" relative to the *working* directory in 
some cases.
If this theory is correct:

- you'd be using an editor/plugin that sends compile commands over LSP (such as 
YCM + ycm_extra_conf). What are you using?
- there should be relatively few *.idx files inside the extra directories (the 
ones not near your compilation database), corresponding to files you've had 
open rather than the whole project
- we don't know why this wouldn't also have been happenening with the old 
`.clangd` directories - possible they were there but masked by `.gitignore`?

If this doesn't sound likely, it'd be really useful (as we haven't reproduced 
this) to add a log line if you can, and capture a log.
Immediately below the llvm::sys::path::append that was changed in this patch, 
if you could add:

  log("BackgroundIndexStorage: File={0} SourceRoot={1} StorageDir={2}", File, 
PI->SourceRoot, StorageDir);

Then delete all the `.cache` directories, open clangd and open files until they 
come back, and grab the clangd stderr log (editors/plugins expose this in some 
way).
This should give us something to go on, else we'll have to keep guessing...

Cheers, Sam


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added a comment.

In D83508#2143625 , @sammccall wrote:

> Your test cases show two problems with this strategy:
>
> - we ignore comments and semicolons, but not preprocessor directives (or 
> disabled preprocessor regions). I think we can fix this by asking TokenBuffer 
> if a spelled token is part of a region that maps to no (PP-)expanded tokens.


I have tried this locally. seems it breaks SelectionTest.IncludedFile test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83508



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

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

I think this change introduced the following warnings, where `auto &` is used 
for types that are always copied. It would be great if you could take a look.

  lvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp:8011:24: warning: loop 
variable 'L' is always a copy because the range of type 
'clang::OMPMappableExprListClause::const_component_lists_range'
 (aka 
'iterator_range::const_component_lists_iterator>')
 does not return a reference [-Wrange-loop-analysis]
for (const auto &L : C->component_lists()) {
 ^
  llvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp:8011:12: note: use 
non-reference type 'std::__1::tuple, const 
clang::ValueDecl *>'
for (const auto &L : C->component_lists()) {
 ^~~
  llvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp:8016:24: warning: loop 
variable 'L' is always a copy because the range of type 
'clang::OMPMappableExprListClause::const_component_lists_range'
 (aka 
'iterator_range::const_component_lists_iterator>')
 does not return a reference [-Wrange-loop-analysis]
for (const auto &L : C->component_lists()) {
 ^
  llvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp:8016:12: note: use 
non-reference type 'std::__1::tuple, const 
clang::ValueDecl *>'
for (const auto &L : C->component_lists()) {
 ^~~
  llvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp:8032:24: warning: loop 
variable 'L' is always a copy because the range of type 
'clang::OMPMappableExprListClause::const_component_lists_range'
 (aka 
'iterator_range::const_component_lists_iterator>')
 does not return a reference [-Wrange-loop-analysis]
for (const auto &L : C->component_lists()) {
 ^
  llvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp:8032:12: note: use 
non-reference type 'std::__1::tuple, const 
clang::ValueDecl *>'
for (const auto &L : C->component_lists()) {
 ^~~
  3 warnings generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67833



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

[PATCH] D83934: [clangd] Always retrieve ProjectInfo from Base in OverlayCDB

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Clangd is returning current working directory for overriden commands.
This can cause inconsistencies between:

- header and the main files, as OverlayCDB only contains entries for the main 
files it direct any queries for the headers to the base, creating a discrepancy 
between the two.
- different clangd instances, as the results will be different depending on the 
timing of execution of the query and override of the command. hence clangd 
might see two different project infos for the same file between different 
invocations.
- editors and the way user has invoked it, as current working directory of 
clangd will depend on those, hence even when there's no underlying base CWD 
might change depending on the editor, or the directory user has started the 
editor in.

This patch gets rid of that discrepency by always directing queries to base or
returning llvm::None in absence of it.

For a sample bug see https://reviews.llvm.org/D83099#2154185.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83934

Files:
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  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
@@ -313,9 +313,22 @@
   llvm::sys::path::append(File, "blabla", "..", "a.cc");
 
   EXPECT_TRUE(DB.getCompileCommand(File));
-  EXPECT_TRUE(DB.getProjectInfo(File));
+  EXPECT_FALSE(DB.getProjectInfo(File));
 }
 
+TEST_F(OverlayCDBTest, GetProjectInfo) {
+  OverlayCDB DB(Base.get());
+  Path File = testPath("foo.cc");
+  Path Header = testPath("foo.h");
+
+  EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
+  EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
+
+  // Shouldn't change after an override.
+  DB.setCompileCommand(File, tooling::CompileCommand());
+  EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
+  EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -119,7 +119,6 @@
 getQueryDriverDatabase(llvm::ArrayRef QueryDriverGlobs,
std::unique_ptr Base);
 
-
 /// Wraps another compilation database, and supports overriding the commands
 /// using an in-memory mapping.
 class OverlayCDB : public GlobalCompilationDatabase {
@@ -134,6 +133,10 @@
   llvm::Optional
   getCompileCommand(PathRef File) const override;
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;
+  /// Note that project info is gathered purely from the inner compilation
+  /// database. This is to ensure consistency, as a translation unit with an
+  /// overriden command might yield a different project compared to headers
+  /// included in it (or before and after the override arrives).
   llvm::Optional getProjectInfo(PathRef File) const override;
 
   /// Sets or clears the compilation command for a particular file.
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -298,15 +298,8 @@
 }
 
 llvm::Optional OverlayCDB::getProjectInfo(PathRef File) const {
-  {
-std::lock_guard Lock(Mutex);
-auto It = Commands.find(removeDots(File));
-if (It != Commands.end())
-  return ProjectInfo{};
-  }
   if (Base)
 return Base->getProjectInfo(File);
-
   return llvm::None;
 }
 } // namespace clangd


Index: clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -313,9 +313,22 @@
   llvm::sys::path::append(File, "blabla", "..", "a.cc");
 
   EXPECT_TRUE(DB.getCompileCommand(File));
-  EXPECT_TRUE(DB.getProjectInfo(File));
+  EXPECT_FALSE(DB.getProjectInfo(File));
 }
 
+TEST_F(OverlayCDBTest, GetProjectInfo) {
+  OverlayCDB DB(Base.get());
+  Path File = testPath("foo.cc");
+  Path Header = testPath("foo.h");
+
+  EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
+  EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());

[PATCH] D83855: [clang] Fix printing of lambdas with capture expressions

[Ilya Golovenko via Phabricator via cfe-commits]

walrus updated this revision to Diff 278411.
walrus added a comment.

Simplify code according to suggestions in code review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83855

Files:
  clang/lib/AST/StmtPrinter.cpp
  clang/test/AST/ast-printer-lambda.cpp


Index: clang/test/AST/ast-printer-lambda.cpp
===
--- clang/test/AST/ast-printer-lambda.cpp
+++ clang/test/AST/ast-printer-lambda.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -ast-print -std=c++17 %s | FileCheck %s
 
+struct M {};
+
 struct S {
 template
 void test1(int i, T... t) {
@@ -15,6 +17,18 @@
   auto lambda = [&]{};
   //CHECK: [&] {
 }
+{
+  auto lambda = [k{i}] {};
+  //CHECK: [k{i}] {
+}
+{
+  auto lambda = [k(i)] {};
+  //CHECK: [k(i)] {
+}
+{
+  auto lambda = [k = i] {};
+  //CHECK: [k = i] {
+}
 {
   auto lambda = [t..., i]{};
   //CHECK: [t..., i] {
@@ -31,6 +45,14 @@
   auto lambda = [t..., this]{};
   //CHECK: [t..., this] {
 }
+{
+  auto lambda = [k(t...)] {};
+  //CHECK: [k(t...)] {
+}
+{
+  auto lambda = [k{t...}] {};
+  //CHECK: [k{t...}] {
+}
 }
 
 };
\ No newline at end of file
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -2005,8 +2005,23 @@
 if (C->isPackExpansion())
   OS << "...";
 
-if (Node->isInitCapture(C))
-  PrintExpr(C->getCapturedVar()->getInit());
+if (Node->isInitCapture(C)) {
+  VarDecl *D = C->getCapturedVar();
+
+  llvm::StringRef Pre;
+  llvm::StringRef Post;
+  if (D->getInitStyle() == VarDecl::CallInit) {
+if (!isa(D->getInit())) {
+  Pre = "(";
+  Post = ")";
+}
+  } else if (D->getInitStyle() == VarDecl::CInit)
+Pre = " = ";
+
+  OS << Pre;
+  PrintExpr(D->getInit());
+  OS << Post;
+}
   }
   OS << ']';
 


Index: clang/test/AST/ast-printer-lambda.cpp
===
--- clang/test/AST/ast-printer-lambda.cpp
+++ clang/test/AST/ast-printer-lambda.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -ast-print -std=c++17 %s | FileCheck %s
 
+struct M {};
+
 struct S {
 template
 void test1(int i, T... t) {
@@ -15,6 +17,18 @@
   auto lambda = [&]{};
   //CHECK: [&] {
 }
+{
+  auto lambda = [k{i}] {};
+  //CHECK: [k{i}] {
+}
+{
+  auto lambda = [k(i)] {};
+  //CHECK: [k(i)] {
+}
+{
+  auto lambda = [k = i] {};
+  //CHECK: [k = i] {
+}
 {
   auto lambda = [t..., i]{};
   //CHECK: [t..., i] {
@@ -31,6 +45,14 @@
   auto lambda = [t..., this]{};
   //CHECK: [t..., this] {
 }
+{
+  auto lambda = [k(t...)] {};
+  //CHECK: [k(t...)] {
+}
+{
+  auto lambda = [k{t...}] {};
+  //CHECK: [k{t...}] {
+}
 }
 
 };
\ No newline at end of file
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -2005,8 +2005,23 @@
 if (C->isPackExpansion())
   OS << "...";
 
-if (Node->isInitCapture(C))
-  PrintExpr(C->getCapturedVar()->getInit());
+if (Node->isInitCapture(C)) {
+  VarDecl *D = C->getCapturedVar();
+
+  llvm::StringRef Pre;
+  llvm::StringRef Post;
+  if (D->getInitStyle() == VarDecl::CallInit) {
+if (!isa(D->getInit())) {
+  Pre = "(";
+  Post = ")";
+}
+  } else if (D->getInitStyle() == VarDecl::CInit)
+Pre = " = ";
+
+  OS << Pre;
+  PrintExpr(D->getInit());
+  OS << Post;
+}
   }
   OS << ']';
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83855: [clang] Fix printing of lambdas with capture expressions

[Ilya Golovenko via Phabricator via cfe-commits]

walrus updated this revision to Diff 278412.
walrus added a comment.

Remove unused struct definition


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83855

Files:
  clang/lib/AST/StmtPrinter.cpp
  clang/test/AST/ast-printer-lambda.cpp


Index: clang/test/AST/ast-printer-lambda.cpp
===
--- clang/test/AST/ast-printer-lambda.cpp
+++ clang/test/AST/ast-printer-lambda.cpp
@@ -15,6 +15,18 @@
   auto lambda = [&]{};
   //CHECK: [&] {
 }
+{
+  auto lambda = [k{i}] {};
+  //CHECK: [k{i}] {
+}
+{
+  auto lambda = [k(i)] {};
+  //CHECK: [k(i)] {
+}
+{
+  auto lambda = [k = i] {};
+  //CHECK: [k = i] {
+}
 {
   auto lambda = [t..., i]{};
   //CHECK: [t..., i] {
@@ -31,6 +43,14 @@
   auto lambda = [t..., this]{};
   //CHECK: [t..., this] {
 }
+{
+  auto lambda = [k(t...)] {};
+  //CHECK: [k(t...)] {
+}
+{
+  auto lambda = [k{t...}] {};
+  //CHECK: [k{t...}] {
+}
 }
 
 };
\ No newline at end of file
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -2005,8 +2005,23 @@
 if (C->isPackExpansion())
   OS << "...";
 
-if (Node->isInitCapture(C))
-  PrintExpr(C->getCapturedVar()->getInit());
+if (Node->isInitCapture(C)) {
+  VarDecl *D = C->getCapturedVar();
+
+  llvm::StringRef Pre;
+  llvm::StringRef Post;
+  if (D->getInitStyle() == VarDecl::CallInit) {
+if (!isa(D->getInit())) {
+  Pre = "(";
+  Post = ")";
+}
+  } else if (D->getInitStyle() == VarDecl::CInit)
+Pre = " = ";
+
+  OS << Pre;
+  PrintExpr(D->getInit());
+  OS << Post;
+}
   }
   OS << ']';
 


Index: clang/test/AST/ast-printer-lambda.cpp
===
--- clang/test/AST/ast-printer-lambda.cpp
+++ clang/test/AST/ast-printer-lambda.cpp
@@ -15,6 +15,18 @@
   auto lambda = [&]{};
   //CHECK: [&] {
 }
+{
+  auto lambda = [k{i}] {};
+  //CHECK: [k{i}] {
+}
+{
+  auto lambda = [k(i)] {};
+  //CHECK: [k(i)] {
+}
+{
+  auto lambda = [k = i] {};
+  //CHECK: [k = i] {
+}
 {
   auto lambda = [t..., i]{};
   //CHECK: [t..., i] {
@@ -31,6 +43,14 @@
   auto lambda = [t..., this]{};
   //CHECK: [t..., this] {
 }
+{
+  auto lambda = [k(t...)] {};
+  //CHECK: [k(t...)] {
+}
+{
+  auto lambda = [k{t...}] {};
+  //CHECK: [k{t...}] {
+}
 }
 
 };
\ No newline at end of file
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -2005,8 +2005,23 @@
 if (C->isPackExpansion())
   OS << "...";
 
-if (Node->isInitCapture(C))
-  PrintExpr(C->getCapturedVar()->getInit());
+if (Node->isInitCapture(C)) {
+  VarDecl *D = C->getCapturedVar();
+
+  llvm::StringRef Pre;
+  llvm::StringRef Post;
+  if (D->getInitStyle() == VarDecl::CallInit) {
+if (!isa(D->getInit())) {
+  Pre = "(";
+  Post = ")";
+}
+  } else if (D->getInitStyle() == VarDecl::CInit)
+Pre = " = ";
+
+  OS << Pre;
+  PrintExpr(D->getInit());
+  OS << Post;
+}
   }
   OS << ']';
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83914: [clangd] Plan features for FoldingRanges

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

A few different types of feedback mixed up here :-)

Some nitpicks about behavior: these are probably worth discussing.

Places where the coverage is incomplete: obviously until we get closer to 
implementation, how complete this is is up to you.

Looking at examples mostly reinforced to me that foldability should have some 
threshold for what's inside as well as rules about grammatically what's 
foldable. I just can't see editors doing a good job of discarding uninteresting 
ranges - they're going to consider that our job.
If "only things spanning newlines" is too strict, we could consider maybe other 
complexity heuristics.

At least for the AST-based folds, I'd like some formalism like:

- we build a tree of potentially-foldable regions based on the AST
- starting at the bottom, we decide whether each potentially-foldable region is 
actually folded
  - any region containing a non-folded newline is folded
  - any bracket-delimited region containing a non-folded comma or semicolon 
token is folded

Regarding implementation and RAV: we talked about the possibility of using 
syntax trees, is that still on the table?




Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:237
 
+TEST(FoldingRanges, ControlFlow) {
+  const char *Tests[] = {

nit: the subexpression tests are mixed in with the if cases



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:246
+
+  if ([[B && I > 42 || ~([[++I]])]]) {[[
+++I;

confused about [[++I]] - why is this foldable but not I > 42 etc - is this 
because of the containing parens?

FWIW, Personally, I'd want the whole if condition to be  foldable (maybe only 
if it spans lines), and only paren subexpressions, arg lists etc that span 
lines to be foldable. It's hard to imagine a UX that could expose folding small 
parens in a way that's not annoying. (e.g. shows lots of extra UI elements, or 
prevents folding the outer expression)



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:277
+int main() {[[
+  for (int I = 0]];[[I < 42]];[[++I) {[[
+--I;

this is a really nice model, but I worry that having multiple folding ranges 
starting at the same point within a line is going to be a UX nightmare.
I'd suggest picking one.

(Note that other control flow statements can look like for loops too these 
days, with initializer statements)



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:329
+  )cpp",
+  // Argument lists.
+  R"cpp(

missing lambdas, blocks, objc methods.
cover bodies in the same tests?
capture lists of lambdas?



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:342
+  )cpp",
+  // Namespace.
+  R"cpp(

linkage specs probably belong here too



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:414
+template <[[unsigned U]]>
+void foo([[char t, U u]]) {}
+

why is the template argument list not foldable here? (it is for the class 
example above)



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:552
+#include "external/Logger.h"
+#include "external/Vector.h"]]
+#include [["project/DataStructures/Trie.h"

I think clang-format messed some of these up for you



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:578
+//[[ contents of the header]]
+#endif]] /*[[ LIBRARY_FILENAME_H ]]*/
+

I think consistent with how you handle braces and  comments, the range should 
end just before the #endif



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:580
+
+#if [[__has_include()
+#  include 

I don't think you want the condition as part of the same of the same fold as 
the content, it might be important

What about
```
#if [[some_condition]][[
body
]]#elif [[some_condition]][[
body
]]#else[[
body
]]#endif
```

Rendering (partially folded) as:
#if some_condition ... #elif some_condition ... #else ... #endif



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:583
+#  define have_optional 1
+#]]elif __has_include() [[]]
+#  include 

here I'd expect folding before # so you can see it's a directive when folded



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:605
+  R"cpp(
+#error [["Not a standard compliant compiler"]]
+

I think YAGNI here - not sure how this could help, and it's an obscure feature 
to start with



Comment at: clang-tools-

[PATCH] D83855: [clang] Fix printing of lambdas with capture expressions

[Ilya Golovenko via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/StmtPrinter.cpp:2011
+  Expr *Init = D->getInit();
+  if (D->getInitStyle() == VarDecl::CallInit && !isa(Init))
+OS << "(";

kadircet wrote:
> walrus wrote:
> > kadircet wrote:
> > > what about having a `Pre` and `Post` print blocks, set to `"(" and ")"` 
> > > or `" = ` and ""` respectively?
> > > 
> > > that way we could get rid of the second if block below.
> > > 
> > > 
> > > also i don't follow why blacklisting for parenlistexpr is needed here (i 
> > > can see that it will end up printing double parens, but so is 
> > > `ParenExpr`s, and I think both of them shouldn't be showing up as 
> > > initializer exprs of captured variables), could you elaborate with a 
> > > comment and a test case?
> > I think you're right that skipping `ParenListExpr` is wrong here. I took 
> > this part of code from DeclPrinter. The `ParenListExpr`s are skipped when 
> > printing variable declarations, but I think it is not applicable when 
> > printing lambda expression captures.
> > 
> > Honestly, I didn't get about `Pre` and `Post` blocks. I know it is 
> > supported when printing types, but I could not find how to do this for 
> > expressions.
> i meant something like below, (modulo naming):
> ```
> llvm::StringRef Pre;
> llvm::StringRef Post;
> if(Style == CallInit) {
>   Pre = "(";
>   Post = ")";
> }
> else if (Style == CInit)
>   Pre = " = ";
> 
> OS << Pre;
> PrintExpr(Init):
> OS << Post;
> ```
Thank you for your suggestion - this makes code more clear. Regarding 
`ParenListExpr` exception - I investigated this case and found that this is 
necessary to prevent double braces when capturing variables of dependent 
unresolved types, e.g.:

```
template 
void foo(T var) {

  auto lambda = [m(var)] {};
}
```
Here expression `m(var)` resolves in `ParenListExpr` which will print braces on 
its own.

I added a couple of test cases to validate this behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83855



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

[PATCH] D83934: [clangd] Always retrieve ProjectInfo from Base in OverlayCDB

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.h:136
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;
+  /// Note that project info is gathered purely from the inner compilation
+  /// database. This is to ensure consistency, as a translation unit with an

nit: drop "note that" for readability.



Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.h:137
+  /// Note that project info is gathered purely from the inner compilation
+  /// database. This is to ensure consistency, as a translation unit with an
+  /// overriden command might yield a different project compared to headers

I find this comment confusing, you say "might" when I think you mean "might 
otherwise" (i.e. given the old behavior).
However the claim is true anyway for another reason: headers *may* simply be 
from a different CDB.

Maybe enough for the header is:
// It wouldn't make much sense to treat files with overridden commands 
specially when
// we can't do the same for the (unknown) local headers they include.

Even so I think this comment might belong rather in the implementation file...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83934



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

[PATCH] D77150: [Analyzer] New Option for ContainerModeling: AggressiveEraseModeling

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

baloghadamsoftware added a comment.

Now I made some measurements about the false positives this option adds.

For `BitCoin` the increase was `-1`. I do not know how it happened, but it 
reduced the number of false positives by one.

For `LLVM/Clang` the increase was `15` which is less than `4%` beacause we have 
`419` findings which is enormous. Thus the real problem that scares the user is 
not the `15` false positives but the `419`. To reduce them we must refine the 
modeling of containers which is also expected to reduce the `15`. (I did not 
examine the `15` findigs, maybe some of them are true positives.)

Honestly, this was the result I expected. This extra state split should not add 
many false positives because it is extremely rare that we know that we are 
erasing an element which is not the last one but this knowledge is not 
expressed by constraints. Of course, we can create artificial examples where 
this option gives false positives, but the measurement shows that in real code 
bases the increase is minimal. However, the bug is a serious one which may 
cause undefined behavior that may lie hidden in tests.


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

https://reviews.llvm.org/D77150



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

[PATCH] D83855: [clang] Fix printing of lambdas with capture expressions

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!




Comment at: clang/lib/AST/StmtPrinter.cpp:2014
+  if (D->getInitStyle() == VarDecl::CallInit) {
+if (!isa(D->getInit())) {
+  Pre = "(";

ah, now i see the use for it, thanks for the testcase !

could you please `and` with the previous condition rather than extra nesting.



Comment at: clang/lib/AST/StmtPrinter.cpp:2018
+}
+  } else if (D->getInitStyle() == VarDecl::CInit)
+Pre = " = ";

nit: braces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83855



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

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D83508#2155174 , @ArcsinX wrote:

> In D83508#2143625 , @sammccall wrote:
>
> > Your test cases show two problems with this strategy:
> >
> > - we ignore comments and semicolons, but not preprocessor directives (or 
> > disabled preprocessor regions). I think we can fix this by asking 
> > TokenBuffer if a spelled token is part of a region that maps to no 
> > (PP-)expanded tokens.
>
>
> I have tried this locally. seems it breaks SelectionTest.IncludedFile test.


Yeah that makes sense, I guess it just says nothing is selected in that case?
Selecting the `while` is probably marginally better for that exact case, but 
selecting nothing seems fine to me.

@kadircet any concerns with that "regression"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83508



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

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added a comment.

In D83508#2155322 , @sammccall wrote:

> In D83508#2155174 , @ArcsinX wrote:
>
> > In D83508#2143625 , @sammccall 
> > wrote:
> >
> > > Your test cases show two problems with this strategy:
> > >
> > > - we ignore comments and semicolons, but not preprocessor directives (or 
> > > disabled preprocessor regions). I think we can fix this by asking 
> > > TokenBuffer if a spelled token is part of a region that maps to no 
> > > (PP-)expanded tokens.
> >
> >
> > I have tried this locally. seems it breaks SelectionTest.IncludedFile test.
>
>
> Yeah that makes sense, I guess it just says nothing is selected in that case?


Yes, and test crashes at `T.commonAncestor()` (which is `nullptr`) dereference. 
So can we also add `ASSERT_TRUE(T.commonAncestor());` into several tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83508



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

[PATCH] D83855: [clang] Fix printing of lambdas with capture expressions

[Ilya Golovenko via Phabricator via cfe-commits]

walrus updated this revision to Diff 278420.
walrus added a comment.

Simplify if..else statement, add braces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83855

Files:
  clang/lib/AST/StmtPrinter.cpp
  clang/test/AST/ast-printer-lambda.cpp


Index: clang/test/AST/ast-printer-lambda.cpp
===
--- clang/test/AST/ast-printer-lambda.cpp
+++ clang/test/AST/ast-printer-lambda.cpp
@@ -15,6 +15,18 @@
   auto lambda = [&]{};
   //CHECK: [&] {
 }
+{
+  auto lambda = [k{i}] {};
+  //CHECK: [k{i}] {
+}
+{
+  auto lambda = [k(i)] {};
+  //CHECK: [k(i)] {
+}
+{
+  auto lambda = [k = i] {};
+  //CHECK: [k = i] {
+}
 {
   auto lambda = [t..., i]{};
   //CHECK: [t..., i] {
@@ -31,6 +43,14 @@
   auto lambda = [t..., this]{};
   //CHECK: [t..., this] {
 }
+{
+  auto lambda = [k(t...)] {};
+  //CHECK: [k(t...)] {
+}
+{
+  auto lambda = [k{t...}] {};
+  //CHECK: [k{t...}] {
+}
 }
 
 };
\ No newline at end of file
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -2005,8 +2005,23 @@
 if (C->isPackExpansion())
   OS << "...";
 
-if (Node->isInitCapture(C))
-  PrintExpr(C->getCapturedVar()->getInit());
+if (Node->isInitCapture(C)) {
+  VarDecl *D = C->getCapturedVar();
+
+  llvm::StringRef Pre;
+  llvm::StringRef Post;
+  if (D->getInitStyle() == VarDecl::CallInit &&
+  !isa(D->getInit())) {
+Pre = "(";
+Post = ")";
+  } else if (D->getInitStyle() == VarDecl::CInit) {
+Pre = " = ";
+  }
+
+  OS << Pre;
+  PrintExpr(D->getInit());
+  OS << Post;
+}
   }
   OS << ']';
 


Index: clang/test/AST/ast-printer-lambda.cpp
===
--- clang/test/AST/ast-printer-lambda.cpp
+++ clang/test/AST/ast-printer-lambda.cpp
@@ -15,6 +15,18 @@
   auto lambda = [&]{};
   //CHECK: [&] {
 }
+{
+  auto lambda = [k{i}] {};
+  //CHECK: [k{i}] {
+}
+{
+  auto lambda = [k(i)] {};
+  //CHECK: [k(i)] {
+}
+{
+  auto lambda = [k = i] {};
+  //CHECK: [k = i] {
+}
 {
   auto lambda = [t..., i]{};
   //CHECK: [t..., i] {
@@ -31,6 +43,14 @@
   auto lambda = [t..., this]{};
   //CHECK: [t..., this] {
 }
+{
+  auto lambda = [k(t...)] {};
+  //CHECK: [k(t...)] {
+}
+{
+  auto lambda = [k{t...}] {};
+  //CHECK: [k{t...}] {
+}
 }
 
 };
\ No newline at end of file
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -2005,8 +2005,23 @@
 if (C->isPackExpansion())
   OS << "...";
 
-if (Node->isInitCapture(C))
-  PrintExpr(C->getCapturedVar()->getInit());
+if (Node->isInitCapture(C)) {
+  VarDecl *D = C->getCapturedVar();
+
+  llvm::StringRef Pre;
+  llvm::StringRef Post;
+  if (D->getInitStyle() == VarDecl::CallInit &&
+  !isa(D->getInit())) {
+Pre = "(";
+Post = ")";
+  } else if (D->getInitStyle() == VarDecl::CInit) {
+Pre = " = ";
+  }
+
+  OS << Pre;
+  PrintExpr(D->getInit());
+  OS << Post;
+}
   }
   OS << ']';
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83934: [clangd] Always retrieve ProjectInfo from Base in OverlayCDB

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 278421.
kadircet added a comment.

- Split comments between implementation and header, and reword.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83934

Files:
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  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
@@ -313,9 +313,22 @@
   llvm::sys::path::append(File, "blabla", "..", "a.cc");
 
   EXPECT_TRUE(DB.getCompileCommand(File));
-  EXPECT_TRUE(DB.getProjectInfo(File));
+  EXPECT_FALSE(DB.getProjectInfo(File));
 }
 
+TEST_F(OverlayCDBTest, GetProjectInfo) {
+  OverlayCDB DB(Base.get());
+  Path File = testPath("foo.cc");
+  Path Header = testPath("foo.h");
+
+  EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
+  EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
+
+  // Shouldn't change after an override.
+  DB.setCompileCommand(File, tooling::CompileCommand());
+  EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
+  EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -119,7 +119,6 @@
 getQueryDriverDatabase(llvm::ArrayRef QueryDriverGlobs,
std::unique_ptr Base);
 
-
 /// Wraps another compilation database, and supports overriding the commands
 /// using an in-memory mapping.
 class OverlayCDB : public GlobalCompilationDatabase {
@@ -134,6 +133,8 @@
   llvm::Optional
   getCompileCommand(PathRef File) const override;
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;
+  /// Project info is gathered purely from the inner compilation database to
+  /// ensure consistency.
   llvm::Optional getProjectInfo(PathRef File) const override;
 
   /// Sets or clears the compilation command for a particular file.
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -298,15 +298,11 @@
 }
 
 llvm::Optional OverlayCDB::getProjectInfo(PathRef File) const {
-  {
-std::lock_guard Lock(Mutex);
-auto It = Commands.find(removeDots(File));
-if (It != Commands.end())
-  return ProjectInfo{};
-  }
+  // It wouldn't make much sense to treat files with overridden commands
+  // specially when we can't do the same for the (unknown) local headers they
+  // include or changing behavior mid-air after receiving an override.
   if (Base)
 return Base->getProjectInfo(File);
-
   return llvm::None;
 }
 } // namespace clangd


Index: clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -313,9 +313,22 @@
   llvm::sys::path::append(File, "blabla", "..", "a.cc");
 
   EXPECT_TRUE(DB.getCompileCommand(File));
-  EXPECT_TRUE(DB.getProjectInfo(File));
+  EXPECT_FALSE(DB.getProjectInfo(File));
 }
 
+TEST_F(OverlayCDBTest, GetProjectInfo) {
+  OverlayCDB DB(Base.get());
+  Path File = testPath("foo.cc");
+  Path Header = testPath("foo.h");
+
+  EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
+  EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
+
+  // Shouldn't change after an override.
+  DB.setCompileCommand(File, tooling::CompileCommand());
+  EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
+  EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -119,7 +119,6 @@
 getQueryDriverDatabase(llvm::ArrayRef QueryDriverGlobs,
std::unique_ptr Base);
 
-
 /// Wraps another compilation database, and supports overriding the commands
 /// using an in-memory mapping.
 class OverlayCDB : public GlobalCompilationDatabase {
@@ -134,6 +133,8 @@
   llvm::Optional
   getCompileCommand(PathRef File) const overr

[PATCH] D83855: [clang] Fix printing of lambdas with capture expressions

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

do you have commit access? if not I am happy to land this for you.

After having some commits done on your behalf, you can apply for commit access 
as described in 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83855



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

[PATCH] D83934: [clangd] Always retrieve ProjectInfo from Base in OverlayCDB

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG46c921003c2c: [clangd] Always retrieve ProjectInfo from Base 
in OverlayCDB (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83934

Files:
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  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
@@ -313,9 +313,22 @@
   llvm::sys::path::append(File, "blabla", "..", "a.cc");
 
   EXPECT_TRUE(DB.getCompileCommand(File));
-  EXPECT_TRUE(DB.getProjectInfo(File));
+  EXPECT_FALSE(DB.getProjectInfo(File));
 }
 
+TEST_F(OverlayCDBTest, GetProjectInfo) {
+  OverlayCDB DB(Base.get());
+  Path File = testPath("foo.cc");
+  Path Header = testPath("foo.h");
+
+  EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
+  EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
+
+  // Shouldn't change after an override.
+  DB.setCompileCommand(File, tooling::CompileCommand());
+  EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
+  EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -119,7 +119,6 @@
 getQueryDriverDatabase(llvm::ArrayRef QueryDriverGlobs,
std::unique_ptr Base);
 
-
 /// Wraps another compilation database, and supports overriding the commands
 /// using an in-memory mapping.
 class OverlayCDB : public GlobalCompilationDatabase {
@@ -134,6 +133,8 @@
   llvm::Optional
   getCompileCommand(PathRef File) const override;
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;
+  /// Project info is gathered purely from the inner compilation database to
+  /// ensure consistency.
   llvm::Optional getProjectInfo(PathRef File) const override;
 
   /// Sets or clears the compilation command for a particular file.
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -298,15 +298,11 @@
 }
 
 llvm::Optional OverlayCDB::getProjectInfo(PathRef File) const {
-  {
-std::lock_guard Lock(Mutex);
-auto It = Commands.find(removeDots(File));
-if (It != Commands.end())
-  return ProjectInfo{};
-  }
+  // It wouldn't make much sense to treat files with overridden commands
+  // specially when we can't do the same for the (unknown) local headers they
+  // include or changing behavior mid-air after receiving an override.
   if (Base)
 return Base->getProjectInfo(File);
-
   return llvm::None;
 }
 } // namespace clangd


Index: clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
===
--- clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -313,9 +313,22 @@
   llvm::sys::path::append(File, "blabla", "..", "a.cc");
 
   EXPECT_TRUE(DB.getCompileCommand(File));
-  EXPECT_TRUE(DB.getProjectInfo(File));
+  EXPECT_FALSE(DB.getProjectInfo(File));
 }
 
+TEST_F(OverlayCDBTest, GetProjectInfo) {
+  OverlayCDB DB(Base.get());
+  Path File = testPath("foo.cc");
+  Path Header = testPath("foo.h");
+
+  EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
+  EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
+
+  // Shouldn't change after an override.
+  DB.setCompileCommand(File, tooling::CompileCommand());
+  EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
+  EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -119,7 +119,6 @@
 getQueryDriverDatabase(llvm::ArrayRef QueryDriverGlobs,
std::unique_ptr Base);
 
-
 /// Wraps another compilation database, and supports overriding the commands
 /// using an in-memory mapping.
 class OverlayCDB : public GlobalCompilationDatabase {
@@ -134,6 +133,8 @@
   llvm::O

[clang-tools-extra] 46c9210 - [clangd] Always retrieve ProjectInfo from Base in OverlayCDB

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-07-16T12:33:54+02:00
New Revision: 46c921003c2ce5f1cdc4de9ef613eb001980780c

URL: 
https://github.com/llvm/llvm-project/commit/46c921003c2ce5f1cdc4de9ef613eb001980780c
DIFF: 
https://github.com/llvm/llvm-project/commit/46c921003c2ce5f1cdc4de9ef613eb001980780c.diff

LOG: [clangd] Always retrieve ProjectInfo from Base in OverlayCDB

Summary:
Clangd is returning current working directory for overriden commands.
This can cause inconsistencies between:
- header and the main files, as OverlayCDB only contains entries for the main
  files it direct any queries for the headers to the base, creating a
  discrepancy between the two.
- different clangd instances, as the results will be different depending on the
  timing of execution of the query and override of the command. hence clangd
  might see two different project infos for the same file between different
  invocations.
- editors and the way user has invoked it, as current working directory of
  clangd will depend on those, hence even when there's no underlying base CWD
  might change depending on the editor, or the directory user has started the
  editor in.

This patch gets rid of that discrepency by always directing queries to base or
returning llvm::None in absence of it.

For a sample bug see https://reviews.llvm.org/D83099#2154185.

Reviewers: sammccall

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

Tags: #clang

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp 
b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
index 5e75864ec8d4..23e8c9fe716d 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -298,15 +298,11 @@ void OverlayCDB::setCompileCommand(
 }
 
 llvm::Optional OverlayCDB::getProjectInfo(PathRef File) const {
-  {
-std::lock_guard Lock(Mutex);
-auto It = Commands.find(removeDots(File));
-if (It != Commands.end())
-  return ProjectInfo{};
-  }
+  // It wouldn't make much sense to treat files with overridden commands
+  // specially when we can't do the same for the (unknown) local headers they
+  // include or changing behavior mid-air after receiving an override.
   if (Base)
 return Base->getProjectInfo(File);
-
   return llvm::None;
 }
 } // namespace clangd

diff  --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.h 
b/clang-tools-extra/clangd/GlobalCompilationDatabase.h
index e9a5417d9d69..95677f9f8c19 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -119,7 +119,6 @@ std::unique_ptr
 getQueryDriverDatabase(llvm::ArrayRef QueryDriverGlobs,
std::unique_ptr Base);
 
-
 /// Wraps another compilation database, and supports overriding the commands
 /// using an in-memory mapping.
 class OverlayCDB : public GlobalCompilationDatabase {
@@ -134,6 +133,8 @@ class OverlayCDB : public GlobalCompilationDatabase {
   llvm::Optional
   getCompileCommand(PathRef File) const override;
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;
+  /// Project info is gathered purely from the inner compilation database to
+  /// ensure consistency.
   llvm::Optional getProjectInfo(PathRef File) const override;
 
   /// Sets or clears the compilation command for a particular file.

diff  --git 
a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp 
b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
index e68b8d727172..ef9a299483f6 100644
--- a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -313,9 +313,22 @@ TEST(GlobalCompilationDatabaseTest, NonCanonicalFilenames) 
{
   llvm::sys::path::append(File, "blabla", "..", "a.cc");
 
   EXPECT_TRUE(DB.getCompileCommand(File));
-  EXPECT_TRUE(DB.getProjectInfo(File));
+  EXPECT_FALSE(DB.getProjectInfo(File));
 }
 
+TEST_F(OverlayCDBTest, GetProjectInfo) {
+  OverlayCDB DB(Base.get());
+  Path File = testPath("foo.cc");
+  Path Header = testPath("foo.h");
+
+  EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
+  EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
+
+  // Shouldn't change after an override.
+  DB.setCompileCommand(File, tooling::CompileCommand());
+  EXPECT_EQ(DB.getProjectInfo(File)->SourceRoot, testRoot());
+  EXPECT_EQ(DB.getProjectInfo(Header)->SourceRoot, testRoot());
+}
 } // namespace
 } // namespace clangd
 } // namespace clang

[PATCH] D83855: [clang] Fix printing of lambdas with capture expressions

[Ilya Golovenko via Phabricator via cfe-commits]

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

In D83855#2155303 , @kadircet wrote:

> thanks, lgtm!


Thank you much for the review!

I do not have commit access. Could you please commit the changes on my behalf?
Ilya Golovenko 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83855



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

[clang] a130cf8 - [clang] Fix printing of lambdas with capture expressions

[Kadir Cetinkaya via cfe-commits]

Author: Ilya Golovenko
Date: 2020-07-16T12:50:25+02:00
New Revision: a130cf8ae8ab56ba1cfa7edc52b637c9d0c3fd38

URL: 
https://github.com/llvm/llvm-project/commit/a130cf8ae8ab56ba1cfa7edc52b637c9d0c3fd38
DIFF: 
https://github.com/llvm/llvm-project/commit/a130cf8ae8ab56ba1cfa7edc52b637c9d0c3fd38.diff

LOG: [clang] Fix printing of lambdas with capture expressions

Patch by @walrus !

Reviewers: lattner, kadircet

Reviewed By: kadircet

Subscribers: riccibruno, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/AST/StmtPrinter.cpp
clang/test/AST/ast-printer-lambda.cpp

Removed: 




diff  --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index f797f5fe8e6d..ea160025ae3d 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -2005,8 +2005,23 @@ void StmtPrinter::VisitLambdaExpr(LambdaExpr *Node) {
 if (C->isPackExpansion())
   OS << "...";
 
-if (Node->isInitCapture(C))
-  PrintExpr(C->getCapturedVar()->getInit());
+if (Node->isInitCapture(C)) {
+  VarDecl *D = C->getCapturedVar();
+
+  llvm::StringRef Pre;
+  llvm::StringRef Post;
+  if (D->getInitStyle() == VarDecl::CallInit &&
+  !isa(D->getInit())) {
+Pre = "(";
+Post = ")";
+  } else if (D->getInitStyle() == VarDecl::CInit) {
+Pre = " = ";
+  }
+
+  OS << Pre;
+  PrintExpr(D->getInit());
+  OS << Post;
+}
   }
   OS << ']';
 

diff  --git a/clang/test/AST/ast-printer-lambda.cpp 
b/clang/test/AST/ast-printer-lambda.cpp
index 27a361da5cb1..08f1ff555b0b 100644
--- a/clang/test/AST/ast-printer-lambda.cpp
+++ b/clang/test/AST/ast-printer-lambda.cpp
@@ -15,6 +15,18 @@ void test1(int i, T... t) {
   auto lambda = [&]{};
   //CHECK: [&] {
 }
+{
+  auto lambda = [k{i}] {};
+  //CHECK: [k{i}] {
+}
+{
+  auto lambda = [k(i)] {};
+  //CHECK: [k(i)] {
+}
+{
+  auto lambda = [k = i] {};
+  //CHECK: [k = i] {
+}
 {
   auto lambda = [t..., i]{};
   //CHECK: [t..., i] {
@@ -31,6 +43,14 @@ void test1(int i, T... t) {
   auto lambda = [t..., this]{};
   //CHECK: [t..., this] {
 }
+{
+  auto lambda = [k(t...)] {};
+  //CHECK: [k(t...)] {
+}
+{
+  auto lambda = [k{t...}] {};
+  //CHECK: [k{t...}] {
+}
 }
 
 };
\ No newline at end of file



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

[PATCH] D83855: [clang] Fix printing of lambdas with capture expressions

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa130cf8ae8ab: [clang] Fix printing of lambdas with capture 
expressions (authored by walrus, committed by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83855

Files:
  clang/lib/AST/StmtPrinter.cpp
  clang/test/AST/ast-printer-lambda.cpp


Index: clang/test/AST/ast-printer-lambda.cpp
===
--- clang/test/AST/ast-printer-lambda.cpp
+++ clang/test/AST/ast-printer-lambda.cpp
@@ -15,6 +15,18 @@
   auto lambda = [&]{};
   //CHECK: [&] {
 }
+{
+  auto lambda = [k{i}] {};
+  //CHECK: [k{i}] {
+}
+{
+  auto lambda = [k(i)] {};
+  //CHECK: [k(i)] {
+}
+{
+  auto lambda = [k = i] {};
+  //CHECK: [k = i] {
+}
 {
   auto lambda = [t..., i]{};
   //CHECK: [t..., i] {
@@ -31,6 +43,14 @@
   auto lambda = [t..., this]{};
   //CHECK: [t..., this] {
 }
+{
+  auto lambda = [k(t...)] {};
+  //CHECK: [k(t...)] {
+}
+{
+  auto lambda = [k{t...}] {};
+  //CHECK: [k{t...}] {
+}
 }
 
 };
\ No newline at end of file
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -2005,8 +2005,23 @@
 if (C->isPackExpansion())
   OS << "...";
 
-if (Node->isInitCapture(C))
-  PrintExpr(C->getCapturedVar()->getInit());
+if (Node->isInitCapture(C)) {
+  VarDecl *D = C->getCapturedVar();
+
+  llvm::StringRef Pre;
+  llvm::StringRef Post;
+  if (D->getInitStyle() == VarDecl::CallInit &&
+  !isa(D->getInit())) {
+Pre = "(";
+Post = ")";
+  } else if (D->getInitStyle() == VarDecl::CInit) {
+Pre = " = ";
+  }
+
+  OS << Pre;
+  PrintExpr(D->getInit());
+  OS << Post;
+}
   }
   OS << ']';
 


Index: clang/test/AST/ast-printer-lambda.cpp
===
--- clang/test/AST/ast-printer-lambda.cpp
+++ clang/test/AST/ast-printer-lambda.cpp
@@ -15,6 +15,18 @@
   auto lambda = [&]{};
   //CHECK: [&] {
 }
+{
+  auto lambda = [k{i}] {};
+  //CHECK: [k{i}] {
+}
+{
+  auto lambda = [k(i)] {};
+  //CHECK: [k(i)] {
+}
+{
+  auto lambda = [k = i] {};
+  //CHECK: [k = i] {
+}
 {
   auto lambda = [t..., i]{};
   //CHECK: [t..., i] {
@@ -31,6 +43,14 @@
   auto lambda = [t..., this]{};
   //CHECK: [t..., this] {
 }
+{
+  auto lambda = [k(t...)] {};
+  //CHECK: [k(t...)] {
+}
+{
+  auto lambda = [k{t...}] {};
+  //CHECK: [k{t...}] {
+}
 }
 
 };
\ No newline at end of file
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -2005,8 +2005,23 @@
 if (C->isPackExpansion())
   OS << "...";
 
-if (Node->isInitCapture(C))
-  PrintExpr(C->getCapturedVar()->getInit());
+if (Node->isInitCapture(C)) {
+  VarDecl *D = C->getCapturedVar();
+
+  llvm::StringRef Pre;
+  llvm::StringRef Post;
+  if (D->getInitStyle() == VarDecl::CallInit &&
+  !isa(D->getInit())) {
+Pre = "(";
+Post = ")";
+  } else if (D->getInitStyle() == VarDecl::CInit) {
+Pre = " = ";
+  }
+
+  OS << Pre;
+  PrintExpr(D->getInit());
+  OS << Post;
+}
   }
   OS << ']';
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D83508#2155335 , @ArcsinX wrote:

> In D83508#2155322 , @sammccall wrote:
>
> > In D83508#2155174 , @ArcsinX wrote:
> >
> > > In D83508#2143625 , @sammccall 
> > > wrote:
> > >
> > > > Your test cases show two problems with this strategy:
> > > >
> > > > - we ignore comments and semicolons, but not preprocessor directives 
> > > > (or disabled preprocessor regions). I think we can fix this by asking 
> > > > TokenBuffer if a spelled token is part of a region that maps to no 
> > > > (PP-)expanded tokens.
> > >
> > >
> > > I have tried this locally. seems it breaks SelectionTest.IncludedFile 
> > > test.
> >
> >
> > Yeah that makes sense, I guess it just says nothing is selected in that 
> > case?
>
>
> Yes, and test crashes at `T.commonAncestor()` (which is `nullptr`) 
> dereference. So can we also add `ASSERT_TRUE(T.commonAncestor());` into 
> several tests?


Right, that particular case should become EXPECT_EQ(..., nullptr).

Technically the other tests should probably have an ASSERT, because otherwise 
if the code is broken and returns nullptr we have UB and the test could 
spuriously pass.
In practice I've never seen a test spuriously pass this way, let alone on all 
buildbots. And these kinds of assumptions are hard to keep out of the tests. So 
I'm not particularly concerned about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83508



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

[PATCH] D83940: Port HeaderSearch option flags to new option parsing system

[Daniel Grumberg via Phabricator via cfe-commits]

dang created this revision.
dang added a reviewer: Bigcheese.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

Depends on D83892 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83940

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1885,10 +1885,6 @@
 static void ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
   const std::string &WorkingDir) {
   Opts.Sysroot = std::string(Args.getLastArgValue(OPT_isysroot, "/"));
-  Opts.Verbose = Args.hasArg(OPT_v);
-  Opts.UseBuiltinIncludes = !Args.hasArg(OPT_nobuiltininc);
-  Opts.UseStandardSystemIncludes = !Args.hasArg(OPT_nostdsysteminc);
-  Opts.UseStandardCXXIncludes = !Args.hasArg(OPT_nostdincxx);
   if (const Arg *A = Args.getLastArg(OPT_stdlib_EQ))
 Opts.UseLibcxx = (strcmp(A->getValue(), "libc++") == 0);
   Opts.ResourceDir = std::string(Args.getLastArgValue(OPT_resource_dir));
@@ -1917,24 +1913,12 @@
   }
   for (const auto *A : Args.filtered(OPT_fprebuilt_module_path))
 Opts.AddPrebuiltModulePath(A->getValue());
-  Opts.DisableModuleHash = Args.hasArg(OPT_fdisable_module_hash);
-  Opts.ModulesHashContent = Args.hasArg(OPT_fmodules_hash_content);
-  Opts.ModulesValidateDiagnosticOptions =
-  !Args.hasArg(OPT_fmodules_disable_diagnostic_validation);
-  Opts.ImplicitModuleMaps = Args.hasArg(OPT_fimplicit_module_maps);
-  Opts.ModuleMapFileHomeIsCwd = Args.hasArg(OPT_fmodule_map_file_home_is_cwd);
   Opts.ModuleCachePruneInterval =
   getLastArgIntValue(Args, OPT_fmodules_prune_interval, 7 * 24 * 60 * 60);
   Opts.ModuleCachePruneAfter =
   getLastArgIntValue(Args, OPT_fmodules_prune_after, 31 * 24 * 60 * 60);
-  Opts.ModulesValidateOncePerBuildSession =
-  Args.hasArg(OPT_fmodules_validate_once_per_build_session);
   Opts.BuildSessionTimestamp =
   getLastArgUInt64Value(Args, OPT_fbuild_session_timestamp, 0);
-  Opts.ModulesValidateSystemHeaders =
-  Args.hasArg(OPT_fmodules_validate_system_headers);
-  Opts.ValidateASTInputFilesContent =
-  Args.hasArg(OPT_fvalidate_ast_input_files_content);
   if (const Arg *A = Args.getLastArg(OPT_fmodule_format_EQ))
 Opts.ModuleFormat = A->getValue();
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1040,6 +1040,67 @@
 
 } // Flags = [CC1Option, NoDriverOption]
 
+// HeaderSearch Options
+
+def fmodules_validate_once_per_build_session : Flag<["-"], "fmodules-validate-once-per-build-session">,
+  Group, Flags<[CC1Option]>,
+  HelpText<"Don't verify input files for the modules if the module has been "
+   "successfully validated or loaded during this build session">,
+  MarshallingInfoFlag<"HeaderSearchOpts->ModulesValidateOncePerBuildSession", "false">;
+def fmodules_disable_diagnostic_validation : Flag<["-"], "fmodules-disable-diagnostic-validation">,
+  Group, Flags<[CC1Option]>,
+  HelpText<"Disable validation of the diagnostic options when loading the module">,
+  MarshallingInfoFlag<"HeaderSearchOpts->ModulesValidateDiagnosticOptions", "true">, IsNegative;
+def fmodules_validate_system_headers : Flag<["-"], "fmodules-validate-system-headers">,
+  Group, Flags<[CC1Option]>,
+  HelpText<"Validate the system headers that a module depends on when loading the module">,
+  MarshallingInfoFlag<"HeaderSearchOpts->ModulesValidateSystemHeaders", "false">;
+def fno_modules_validate_system_headers : Flag<["-"], "fno-modules-validate-system-headers">,
+  Group, Flags<[DriverOption]>;
+def fvalidate_ast_input_files_content:
+  Flag <["-"], "fvalidate-ast-input-files-content">,
+  Group, Flags<[CC1Option]>,
+  HelpText<"Compute and store the hash of input files used to build an AST."
+   " Files with mismatching mtime's are considered valid"
+   " if both contents is identical">,
+  MarshallingInfoFlag<"HeaderSearchOpts->ValidateASTInputFilesContent", "false">;
+def fimplicit_module_maps : Flag <["-"], "fimplicit-module-maps">, Group,
+  Flags<[DriverOption, CC1Option]>,
+  HelpText<"Implicitly search the file system for module map files.">,
+  MarshallingInfoFlag<"HeaderSearchOpts->ImplicitModuleMaps", "false">;
+def nobuiltininc : Flag<["-"], "nobuiltininc">, Flags<[CC1Option, CoreOption]>,
+  HelpText<"Disable builtin #include directories">,
+  MarshallingInfoFlag<"HeaderSearchOpts->UseBuiltinIncludes", "true">, IsNegative;
+def nostdincxx : Flag<["-"], "nostdinc++">, Flags<[CC1Option]>,
+  HelpText<"Disable standard #include directories for the C++ standard library">,
+  MarshallingInfoFlag<"HeaderSearchOpts->UseStandardCXXInclud

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D83508#2155322 , @sammccall wrote:

> Yeah that makes sense, I guess it just says nothing is selected in that case?
>  Selecting the `while` is probably marginally better for that exact case, but 
> selecting nothing seems fine to me.
>
> @kadircet any concerns with that "regression"?


I think I am fine with selecting nothing in that case, selecting some part of 
the AST that's not visible(even as hint, it is coming from another file 
could've been anything right?) to developer doesn't seem so important.
It is also inconsistent, we show the while stmt only on filename and not on any 
other part of the directive, moreover if you put a function decl instead of a 
whilestmt, we again don't have a selection even on the filename :(
So i think that might not even be a regression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83508



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

[PATCH] D80802: [RISCV] Upgrade RVV MC to v0.9.

[Simon Cook via Phabricator via cfe-commits]

simoncook added a comment.

Since this patch replaces 0.8 support with 0.9, it should include an update to 
the version check in `clang/lib/Driver/ToolChains/Arch/RISCV.cpp` to match.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80802



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

[PATCH] D83551: [PATCH 2/4][Sema][AArch64] Add semantics for arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes updated this revision to Diff 278430.
c-rhodes added a comment.

Changes:

- Documented internal type attributes.
- Set `ASTNode = 0` on user-facing `ArmSveVectorBitsAttr` as the internal type 
attrs are used in the AST. Also removed the case for this from `TypePrinter`.
- `getSveVectorWidth` now returns an `unsigned`. Added an unreachable if `T` 
has no attrs.
- `s/getArmSveVectorBits/getBitwidthForAttributedSveType`. Also now returns an 
`unsigned` and asserts if `!T->isVLST()`.
- Add a few comments in test and reduced them a little so we dont tell all 
types for structs / unions etc.


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

https://reviews.llvm.org/D83551

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/attr-arm-sve-vector-bits.c

Index: clang/test/Sema/attr-arm-sve-vector-bits.c
===
--- clang/test/Sema/attr-arm-sve-vector-bits.c
+++ clang/test/Sema/attr-arm-sve-vector-bits.c
@@ -60,3 +60,165 @@
 typedef float badtype3 __attribute__((arm_sve_vector_bits(N))); // expected-error {{'arm_sve_vector_bits' attribute applied to non-SVE type 'float'}}
 typedef svint8x2_t badtype4 __attribute__((arm_sve_vector_bits(N)));// expected-error {{'arm_sve_vector_bits' attribute applied to non-SVE type 'svint8x2_t' (aka '__clang_svint8x2_t')}}
 typedef svfloat32x3_t badtype5 __attribute__((arm_sve_vector_bits(N))); // expected-error {{'arm_sve_vector_bits' attribute applied to non-SVE type 'svfloat32x3_t' (aka '__clang_svfloat32x3_t')}}
+
+// Test that we can define non-local fixed-length SVE types (unsupported for
+// sizeless types).
+fixed_int8_t global_int8;
+fixed_bfloat16_t global_bfloat16;
+fixed_bool_t global_bool;
+
+extern fixed_int8_t extern_int8;
+extern fixed_bfloat16_t extern_bfloat16;
+extern fixed_bool_t extern_bool;
+
+static fixed_int8_t static_int8;
+static fixed_bfloat16_t static_bfloat16;
+static fixed_bool_t static_bool;
+
+fixed_int8_t *global_int8_ptr;
+extern fixed_int8_t *extern_int8_ptr;
+static fixed_int8_t *static_int8_ptr;
+__thread fixed_int8_t thread_int8;
+
+typedef fixed_int8_t int8_typedef;
+typedef fixed_int8_t *int8_ptr_typedef;
+
+// Test sized expressions
+int sizeof_int8 = sizeof(global_int8);
+int sizeof_int8_var = sizeof(*global_int8_ptr);
+int sizeof_int8_var_ptr = sizeof(global_int8_ptr);
+
+extern fixed_int8_t *extern_int8_ptr;
+
+int alignof_int8 = __alignof__(extern_int8);
+int alignof_int8_var = __alignof__(*extern_int8_ptr);
+int alignof_int8_var_ptr = __alignof__(extern_int8_ptr);
+
+void f(int c) {
+  fixed_int8_t fs8;
+  svint8_t ss8;
+
+  void *sel __attribute__((unused));
+  sel = c ? ss8 : fs8; // expected-error {{incompatible operand types ('svint8_t' (aka '__SVInt8_t') and 'fixed_int8_t' (aka '__SVInt8_t'))}}
+  sel = c ? fs8 : ss8; // expected-error {{incompatible operand types ('fixed_int8_t' (aka '__SVInt8_t') and 'svint8_t' (aka '__SVInt8_t'))}}
+}
+
+// --//
+// Sizeof
+
+#define VECTOR_SIZE ((N / 8))
+#define PRED_SIZE ((N / 64))
+
+_Static_assert(sizeof(fixed_int8_t) == VECTOR_SIZE, "");
+
+_Static_assert(sizeof(fixed_int16_t) == VECTOR_SIZE, "");
+_Static_assert(sizeof(fixed_int32_t) == VECTOR_SIZE, "");
+_Static_assert(sizeof(fixed_int64_t) == VECTOR_SIZE, "");
+
+_Static_assert(sizeof(fixed_uint8_t) == VECTOR_SIZE, "");
+_Static_assert(sizeof(fixed_uint16_t) == VECTOR_SIZE, "");
+_Static_assert(sizeof(fixed_uint32_t) == VECTOR_SIZE, "");
+_Static_assert(sizeof(fixed_uint64_t) == VECTOR_SIZE, "");
+
+_Static_assert(sizeof(fixed_float16_t) == VECTOR_SIZE, "");
+_Static_assert(sizeof(fixed_float32_t) == VECTOR_SIZE, "");
+_Static_assert(sizeof(fixed_float64_t) == VECTOR_SIZE, "");
+
+_Static_assert(sizeof(fixed_bfloat16_t) == VECTOR_SIZE, "");
+
+_Static_assert(sizeof(fixed_bool_t) == PRED_SIZE, "");
+
+// --//
+// Alignof
+
+#define VECTOR_ALIGN 16
+#define PRED_ALIGN 2
+
+_Static_assert(__alignof__(fixed_int8_t) == VECTOR_ALIGN, "");
+_Static_assert(__alignof__(fixed_int16_t) == VECTOR_ALIGN, "");
+_Static_assert(__alignof__(fixed_int32_t) == VECTOR_ALIGN, "");
+_Static_assert(__alignof__(fixed_int64_t) == VECTOR_ALIGN, "");
+
+_Static_assert(__alignof__(fixed_uint8_t) == VECTOR_ALIGN, "");
+_Static_assert(__alignof__(fixed_uint16_t) == VECTOR_ALIGN, "");
+_Static_assert(__alignof__(fixed_uint32_t) == VECTOR_ALIGN, "");
+_Static_assert(__alignof__(fixed_uint64_t) == VECTOR_ALIGN, "");
+
+_Static_assert(__alignof__(fixed_float16_t) == VECTOR_ALIGN, "");
+_Static_assert(__alignof__(fixed_float32_t) == VECTOR_ALIGN, "");
+_Static_assert(__aligno

[PATCH] D83551: [PATCH 2/4][Sema][AArch64] Add semantics for arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes marked 6 inline comments as done.
c-rhodes added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1541
 
+def ArmSveVectorBits128 : TypeAttr {
+  let Spellings = [];

aaron.ballman wrote:
> aaron.ballman wrote:
> > c-rhodes wrote:
> > > sdesmalen wrote:
> > > > aaron.ballman wrote:
> > > > > sdesmalen wrote:
> > > > > > nit: Can you add a comment saying why these are undocumented (and 
> > > > > > have no spellings)
> > > > > Also, I think these are all missing `let SemaHandler = 0;` and `let 
> > > > > ASTNode = 0;`
> > > > > 
> > > > > Is there a reason why we need N different type attributes instead of 
> > > > > having a single type attribute which encodes the N as an argument? I 
> > > > > think this may simplify the patch somewhat as you no longer need to 
> > > > > switch over N as much.
> > > > > Is there a reason why we need N different type attributes instead of 
> > > > > having a single type attribute which encodes the N as an argument?
> > > > AIUI this was a workaround for getting the value of N from an 
> > > > AttributedType, because this only has `getAttrKind` to return the 
> > > > attribute kind, but no way to get the corresponding argument/value. 
> > > > This seemed like a simple way to do that without having to create a new 
> > > > subclass for Type and having to support that in various places. Is the 
> > > > latter the approach you were thinking of? (or is there perhaps a 
> > > > simpler way?)
> > > > Also, I think these are all missing let SemaHandler = 0; and let 
> > > > ASTNode = 0;
> > > 
> > > Good to know. In SemaType I'm doing `CurType = State.getAttributedType(A, 
> > > CurType, CurType);` which gives an `AttributedType` in the AST, should I 
> > > still set `let ASTNode = 0;` in this case?
> > > 
> > > > Is there a reason why we need N different type attributes instead of 
> > > > having a single type attribute which encodes the N as an argument?
> > > 
> > > As Sander mentioned, it seemed like the easiest solution, interested to 
> > > know if there's a better approach however
> > I was thinking specifically of creating a new `Type` subclass and 
> > supporting it rather than adding 5 new attributes that only vary by a 
> > bit-width (which means there's likely to be a 6th someday). It's not 
> > immediately clear to me whether that's a really big ask for little gain or 
> > not, though.
> Ah, you're right, we may still need `ASTNode` to be kept around for that, 
> good call.
> Also, I think these are all missing let SemaHandler = 0; and let ASTNode = 0;

I've added `let SemaHandler = 0;` for the internal types and `let ASTNode = 0;` 
for the user-facing attr.



Comment at: clang/lib/AST/ASTContext.cpp:1897
+
+bool ASTContext::getArmSveVectorBits(const Type *T, unsigned &Width) const {
+  if (!T->isVLST())

sdesmalen wrote:
> nit: I find this name a bit misleading, because I would expect the 
> (ARM_SVE_VECTOR_) bits to be the same regardless of the type. Maybe rename 
> this to `getBitwidthForAttributedSveType` ?
> nit: I find this name a bit misleading, because I would expect the 
> (ARM_SVE_VECTOR_) bits to be the same regardless of the type. Maybe rename 
> this to getBitwidthForAttributedSveType ?

You're right, poor choice of name, I've updated it



Comment at: clang/lib/Sema/SemaType.cpp:7754
 /// the ACLE, such as svint32_t and svbool_t.
-static void HandleArmSveVectorBitsTypeAttr(QualType &CurType,
-   const ParsedAttr &Attr, Sema &S) {
+static void HandleArmSveVectorBitsTypeAttr(TypeProcessingState &State,
+   QualType &CurType,

sdesmalen wrote:
> Unrelated changes?
> Unrelated changes?

`TypeProcessingState &State` is required to create an attributed type and 
`Sema` is attached to this so I removed it from the interface. This is inline 
with other attributed type handlers. I pulled `S.Context` out as it's used 
frequently.



Comment at: clang/lib/Sema/SemaType.cpp:7839
+  default:
+llvm_unreachable("unsupported vector size!");
+  case 128:

sdesmalen wrote:
> If we only support power-of-two for now, we should only have an 
> `llvm_unreachable` if we prevent parsing any of the other widths (and give an 
> appropriate diagnostic saying those widths are not yet supported).
> If we only support power-of-two for now, we should only have an 
> llvm_unreachable if we prevent parsing any of the other widths (and give an 
> appropriate diagnostic saying those widths are not yet supported).

Now that we check `VecSize != S.getLangOpts().ArmSveVectorBits` above I think 
this is ok as this shouldn't be reachable with `-msve-vector-bits=` 
validating the vector size.



Comment at: clang/test/Sema/attr-arm-sve-vector-bits.c:80
+  void *sel __attribute__((unused));
+  sel = c ? ss8 : fs8; // expected-error {{incompatib

[PATCH] D82663: [CodeGen] Have CodeGen for fixed-point unsigned with padding emit signed operations.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

In D82663#2153834 , @rjmccall wrote:

> I don't understand.  The problem statement as I understood it is that using 
> unsigned intrinsics to do unsigned-with-padding operations is leading to poor 
> code-gen, so you want to start using signed intrinsics, which you can safely 
> do because unsigned-with-padding types are intended to be exactly signed 
> types with a dynamic range restriction to non-negative values.  The result of 
> that operation is still logically an unsigned-with-padding value; there's no 
> need to return back a modified semantic that says that the result is really a 
> signed value because it's *not* really a signed value, you're just computing 
> it a different way.  I also don't understand why you think need a modified 
> semantics value in the first place as opposed to just using a more complex 
> condition when deciding which intrinsics to use.


Well, it's not just about intrinsics. For saturating operations, the common 
semantic width is currently narrowed by one bit to get the correct saturating 
behavior on the operation afterwards. This affects the conversion to the common 
semantic, since the type will be one bit narrower.

But I realize now that it probably doesn't matter if we simply don't do that 
narrowing when constructing the common semantic. That way it will always have 
the right width and we can just do as you say and select the right intrinsic 
based on the padding bit information. Alright, that should probably work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82663



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

[PATCH] D83942: [analyzer][tests] Add a notion of project sizes

[Valeriy Savchenko via Phabricator via cfe-commits]

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

Whith the number of projects growing, it is important to be able to
filter them in a more convenient way than by names.  It is especially
important for benchmarks, when it is not viable to analyze big
projects 20 or 50 times in a row.

Because of this reason, this commit adds a notion of sizes and a
filtering interface that puts a limit on a maximum size of the project
to analyze or benchmark.

Sizes assigned to the projects in this commit, do not directly
correspond to the number of lines or files in the project.  The key
factor that is important for the developers of the analyzer is the
time it takes to analyze the project.  And for this very reason,
"size" basically helps to cluster projects based on their analysis
time.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83942

Files:
  clang/utils/analyzer/ProjectMap.py
  clang/utils/analyzer/SATest.py
  clang/utils/analyzer/projects/projects.json

Index: clang/utils/analyzer/projects/projects.json
===
--- clang/utils/analyzer/projects/projects.json
+++ clang/utils/analyzer/projects/projects.json
@@ -4,139 +4,159 @@
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/jarro2783/cxxopts.git";,
-"commit": "794c975"
+"commit": "794c975",
+"size": "tiny"
   },
   {
 "name": "box2d",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/erincatto/box2d.git";,
-"commit": "1025f9a"
+"commit": "1025f9a",
+"size": "small"
   },
   {
 "name": "tinyexpr",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/codeplea/tinyexpr.git";,
-"commit": "ffb0d41"
+"commit": "ffb0d41",
+"size": "tiny"
   },
   {
 "name": "symengine",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/symengine/symengine.git";,
-"commit": "4f669d59"
+"commit": "4f669d59",
+"size": "small"
   },
   {
 "name": "termbox",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/nsf/termbox.git";,
-"commit": "0df1355"
+"commit": "0df1355",
+"size": "tiny"
   },
   {
 "name": "tinyvm",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/jakogut/tinyvm.git";,
-"commit": "10c25d8"
+"commit": "10c25d8",
+"size": "tiny"
   },
   {
 "name": "tinyspline",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/msteinbeck/tinyspline.git";,
-"commit": "f8b1ab7"
+"commit": "f8b1ab7",
+"size": "tiny"
   },
   {
 "name": "oatpp",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/oatpp/oatpp.git";,
-"commit": "d3e60fb"
+"commit": "d3e60fb",
+"size": "small"
   },
   {
 "name": "libsoundio",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/andrewrk/libsoundio.git";,
-"commit": "b810bf2"
+"commit": "b810bf2",
+"size": "tiny"
   },
   {
 "name": "zstd",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/facebook/zstd.git";,
-"commit": "2af4e073"
+"commit": "2af4e073",
+"size": "small"
   },
   {
 "name": "simbody",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/simbody/simbody.git";,
-"commit": "5cf513d"
+"commit": "5cf513d",
+"size": "big"
   },
   {
 "name": "duckdb",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/cwida/duckdb.git";,
-"commit": "d098c9f"
+"commit": "d098c9f",
+"size": "big"
   },
   {
 "name": "drogon",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/an-tao/drogon.git";,
-"commit": "fd2a612"
+"commit": "fd2a612",
+"size": "small"
   },
   {
 "name": "fmt",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/fmtlib/fmt.git";,
-"commit": "5e7c70e"
+"commit": "5e7c70e",
+"size": "small"
   },
   {
 "name": "re2",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/google/re2.git";,
-"commit": "2b25567"
+"commit": "2b25567",
+"size": "small"
   },
   {
 "name": "cppcheck",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/danmar/cppcheck.git";,
-"commit": "5fa3d53"
+"commit": "5fa3d53",
+"size": "small"
   },
   {
 "name": "harfbuzz",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/harfbuzz/harfbuzz.git";,
-"commit": "f8d345e"
+"commit": "f8d345e",
+"size": "small"
   },
   {
 "name": "capnproto",
 "mode": 1,
 "source": "git",
 "origin": "https://github.com/capnproto/capnproto.git";,
-"commit": 

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX updated this revision to Diff 278440.
ArcsinX added a comment.

Prevent tokens selection in disabled preprocessor sections.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83508

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -521,6 +521,7 @@
   EXPECT_THAT(AST.getDiagnostics(), ::testing::IsEmpty());
   auto T = makeSelectionTree(Case, AST);
 
+  ASSERT_NE(T.commonAncestor(), nullptr);
   EXPECT_EQ("BreakStmt", T.commonAncestor()->kind());
   EXPECT_EQ("WhileStmt", T.commonAncestor()->Parent->kind());
 }
@@ -538,7 +539,7 @@
   auto AST = TU.build();
   auto T = makeSelectionTree(Case, AST);
 
-  EXPECT_EQ("WhileStmt", T.commonAncestor()->kind());
+  EXPECT_EQ(T.commonAncestor(), nullptr);
 }
 
 TEST(SelectionTest, MacroArgExpansion) {
@@ -552,6 +553,7 @@
   Annotations Test(Case);
   auto AST = TestTU::withCode(Test.code()).build();
   auto T = makeSelectionTree(Case, AST);
+  ASSERT_NE(T.commonAncestor(), nullptr);
   EXPECT_EQ("IntegerLiteral", T.commonAncestor()->kind());
   EXPECT_TRUE(T.commonAncestor()->Selected);
 
@@ -566,6 +568,7 @@
   AST = TestTU::withCode(Test.code()).build();
   T = makeSelectionTree(Case, AST);
 
+  ASSERT_NE(T.commonAncestor(), nullptr);
   EXPECT_EQ("IntegerLiteral", T.commonAncestor()->kind());
 }
 
@@ -580,6 +583,7 @@
 
   const SelectionTree::Node *Str = T.commonAncestor();
   EXPECT_EQ("StringLiteral", nodeKind(Str)) << "Implicit selected?";
+  ASSERT_NE(Str, nullptr);
   EXPECT_EQ("ImplicitCastExpr", nodeKind(Str->Parent));
   EXPECT_EQ("CXXConstructExpr", nodeKind(Str->Parent->Parent));
   EXPECT_EQ(Str, &Str->Parent->Parent->ignoreImplicit())
@@ -643,6 +647,32 @@
   EXPECT_EQ(1u, Seen) << "one tree for nontrivial selection";
 }
 
+TEST(SelectionTest, DisabledPreprocessor) {
+  const char *Case = R"cpp(
+namespace ns {
+#define FOO B^AR
+}
+  )cpp";
+  Annotations Test(Case);
+  auto TU = TestTU::withCode(Test.code());
+  auto AST = TU.build();
+  auto T = makeSelectionTree(Case, AST);
+  EXPECT_EQ(T.commonAncestor(), nullptr);
+
+  Case = R"cpp(
+namespace ns {
+#if 0
+void fu^nc();
+#endif
+}
+  )cpp";
+  Test = Annotations(Case);
+  TU = TestTU::withCode(Test.code());
+  AST = TU.build();
+  T = makeSelectionTree(Case, AST);
+  EXPECT_EQ(T.commonAncestor(), nullptr);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -224,6 +224,11 @@
 for (const syntax::Token *T = SelFirst; T < SelLimit; ++T) {
   if (shouldIgnore(*T))
 continue;
+  // Ignore tokens in disabled preprocessor sections.
+  if (Buf.expandedTokens(SM.getMacroArgExpandedLocation(T->location()))
+  .empty() &&
+  !Buf.expansionStartingAt(T))
+continue;
   SpelledTokens.emplace_back();
   Tok &S = SpelledTokens.back();
   S.Offset = SM.getFileOffset(T->location());


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -521,6 +521,7 @@
   EXPECT_THAT(AST.getDiagnostics(), ::testing::IsEmpty());
   auto T = makeSelectionTree(Case, AST);
 
+  ASSERT_NE(T.commonAncestor(), nullptr);
   EXPECT_EQ("BreakStmt", T.commonAncestor()->kind());
   EXPECT_EQ("WhileStmt", T.commonAncestor()->Parent->kind());
 }
@@ -538,7 +539,7 @@
   auto AST = TU.build();
   auto T = makeSelectionTree(Case, AST);
 
-  EXPECT_EQ("WhileStmt", T.commonAncestor()->kind());
+  EXPECT_EQ(T.commonAncestor(), nullptr);
 }
 
 TEST(SelectionTest, MacroArgExpansion) {
@@ -552,6 +553,7 @@
   Annotations Test(Case);
   auto AST = TestTU::withCode(Test.code()).build();
   auto T = makeSelectionTree(Case, AST);
+  ASSERT_NE(T.commonAncestor(), nullptr);
   EXPECT_EQ("IntegerLiteral", T.commonAncestor()->kind());
   EXPECT_TRUE(T.commonAncestor()->Selected);
 
@@ -566,6 +568,7 @@
   AST = TestTU::withCode(Test.code()).build();
   T = makeSelectionTree(Case, AST);
 
+  ASSERT_NE(T.commonAncestor(), nullptr);
   EXPECT_EQ("IntegerLiteral", T.commonAncestor()->kind());
 }
 
@@ -580,6 +583,7 @@
 
   const SelectionTree::Node *Str = T.commonAncestor();
   EXPECT_EQ("StringLiteral", nodeKind(Str)) << "Implicit selected?";
+  ASSERT_NE(Str, nullptr);
   EXPECT_EQ("ImplicitCastExpr", nodeKind(Str->Parent));
   EXPECT_EQ("CXXConstructExpr", nodeKind(Str->Parent->Par

[PATCH] D32199: [TySan] A Type Sanitizer (Clang)

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson added a comment.

Hello to everyone following along! My apologies for the lack of activity; I 
should have made a comment sooner.

Back in December/January I was exploring working on TySan (met with Hal and 
Richard, in addition to rebasing the diffs). After digging into the problem 
space, it became clear that it's not something I could prioritize over other 
works. Since that time, nothing has changed on my end, so I don't expect to 
continue working on this.

If anyone is interested in picking this up, I would be thrilled! - CJ


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D32199



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

[PATCH] D83820: Change metadata to deferred evalutaion in Clang Transformer.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:93
+  // Not all transformations will want or need to attach metadata and therefore
+  // sholud not be requierd to do so.
   AnyGenerator Metadata = [](const ast_matchers::MatchFinder::MatchResult &) {

asoffer wrote:
> ymandel wrote:
> > nit: typos
> > 
> > The same applies to `Note`. Since this is a nullable type, can we ask that 
> > the user check `Metadata != nullptr`?
> I don't think I'm understanding the question. Where/when would users check 
> for Metadata != nullptr?
> 
> Currently in this diff, any library construction of the Metadata field will 
> be non-null. Users would have to explicitly pass null in if they wanted it to 
> be null which would pretty quickly break when the generator is called, so 
> this seems like a not-too-big foot-gun? Does that answer the question?
Sorry, I meant code that consumes this struct. Mostly that's just 
`translateEdits`. I realize now that `Note` is not a great example, since we 
totally ignore it. However, it is intended as optional, so if we had the code, 
it would be checking it for null first. Conversely, `TargetRange` and 
`Replacement` are assumed to never be null, yet we don't set them to default 
values here.  So, I think the first step is to decide if this is optional, and 
(if not) the second is to consistently handle non-optional values in this 
struct.

So, your code below would become something like:
```
llvm::Any Metadata;
if (E.Metadata != nullptr) {
  if (auto M = E.Metadata(Result))
Metadata = std::move(*M);
  else
return M.takeError();
}
```



Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:270
 
-// TODO(asoffer): If this returns an llvm::Expected, should we unwrap?
+// FIXME: If `Metadata` returns an `llvm::Expected` the `AnyGenerator` will
+// construct an `llvm::Expected` where no error is present but the

asoffer wrote:
> ymandel wrote:
> > I think I understand now. Is the idea that we'll only get one implicit 
> > construction from the compiler, so we want to use that "free" conversion on 
> > the `llvm::Any`, rather than on the `llvm::Expected`, which would force the 
> > user to explicitly construct the `llvm::Any`?
> > 
> > I think we should address this with separate functions (or overloads at 
> > least), one for the case of never-fail metadata constructors and one for 
> > failable constructors. That said, any computation that takes the 
> > matchresult is prone to failure because of unbound nodes.  so, I would 
> > expect it to be common for the Callable to be failable.  however, if you 
> > feel that's the uncommon case, let's optimize towards the common case, like 
> > you've done.
> > 
> > With all that said, I agree that if the Callable returns an `Expected` we 
> > should rewrap correctly, not bury in `Any`.
> I think you're asking about why the generic callable instead of a 
> std::function or LLVM equivalent type. It's not about implicit conversions 
> but rather about deduction. Type deduction allows only for conversions on 
> qualifiers (T to const T&, for example). So if we specified a std::function, 
> it would either require:
> 1. The cannot pass lambdas without explicitly wrapping them in a 
> std::function construction so that the return type can be deduced.
> 2. We have an explicit std::function, but then the 
> user-provided callable would have to return an llvm::Any explicitly.
> Neither of these seem ideal.
> 
> The Expected-unwrapping requires some ugly SFINAE, so I'd prefer to punt on 
> this until we feel the need to have Expected's in metadata.
Punting is fine with me. I'm not against making the user explicitly create the 
`Any`, but I don't have a strong opinion on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83820



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

[PATCH] D77062: [analyzer] Improved zero assumption in CStringChecke::assumeZero

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 278445.
ASDenysPetrov marked an inline comment as done.
ASDenysPetrov added a comment.

Changed naming due to LLVM rules.
I decided not to change names everywhere to leave the patch more readable.


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

https://reviews.llvm.org/D77062

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/string.c

Index: clang/test/Analysis/string.c
===
--- clang/test/Analysis/string.c
+++ clang/test/Analysis/string.c
@@ -363,6 +363,15 @@
 strcpy(x, y); // no-warning
 }
 
+// PR37503
+void *func_strcpy_no_assertion();
+char ***ptr_strcpy_no_assertion;
+void strcpy_no_assertion() {
+  *(unsigned char **)ptr_strcpy_no_assertion = (unsigned char *)(func_strcpy_no_assertion());
+  char c;
+  strcpy(**ptr_strcpy_no_assertion, &c); // no-assertion
+}
+
 //===--===
 // stpcpy()
 //===--===
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -196,9 +196,8 @@
   void evalBzero(CheckerContext &C, const CallExpr *CE) const;
 
   // Utility methods
-  std::pair
-  static assumeZero(CheckerContext &C,
-ProgramStateRef state, SVal V, QualType Ty);
+  static std::pair
+  assumeZero(ProgramStateRef State, SVal V);
 
   static ProgramStateRef setCStringLength(ProgramStateRef state,
   const MemRegion *MR,
@@ -279,16 +278,18 @@
 // Individual checks and utility methods.
 //===--===//
 
-std::pair
-CStringChecker::assumeZero(CheckerContext &C, ProgramStateRef state, SVal V,
-   QualType Ty) {
-  Optional val = V.getAs();
-  if (!val)
-return std::pair(state, state);
+std::pair
+CStringChecker::assumeZero(ProgramStateRef State, SVal V) {
+  auto States = std::make_pair(State, State);
 
-  SValBuilder &svalBuilder = C.getSValBuilder();
-  DefinedOrUnknownSVal zero = svalBuilder.makeZeroVal(Ty);
-  return state->assume(svalBuilder.evalEQ(state, *val, zero));
+  Optional Val = V.getAs();
+  // FIXME: We should understand how LazyCompoundVal can be possible here,
+  // fix the root cause and get rid of this check.
+  if (Val && !V.getAs())
+// Returned pair shall be {null, non-null} so reorder states.
+std::tie(States.second, States.first) = State->assume(*Val);
+
+  return States;
 }
 
 ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C,
@@ -299,8 +300,7 @@
 return nullptr;
 
   ProgramStateRef stateNull, stateNonNull;
-  std::tie(stateNull, stateNonNull) =
-  assumeZero(C, State, l, Arg.Expression->getType());
+  std::tie(stateNull, stateNonNull) = assumeZero(State, l);
 
   if (stateNull && !stateNonNull) {
 if (Filter.CheckCStringNullArg) {
@@ -1071,8 +1071,7 @@
 CharVal = svalBuilder.evalCast(CharVal, Ctx.UnsignedCharTy, Ctx.IntTy);
 
 ProgramStateRef StateNullChar, StateNonNullChar;
-std::tie(StateNullChar, StateNonNullChar) =
-assumeZero(C, State, CharVal, Ctx.UnsignedCharTy);
+std::tie(StateNullChar, StateNonNullChar) = assumeZero(State, CharVal);
 
 if (StateWholeReg && !StateNotWholeReg && StateNullChar &&
 !StateNonNullChar) {
@@ -1133,11 +1132,9 @@
   // See if the size argument is zero.
   const LocationContext *LCtx = C.getLocationContext();
   SVal sizeVal = state->getSVal(Size.Expression, LCtx);
-  QualType sizeTy = Size.Expression->getType();
 
   ProgramStateRef stateZeroSize, stateNonZeroSize;
-  std::tie(stateZeroSize, stateNonZeroSize) =
-  assumeZero(C, state, sizeVal, sizeTy);
+  std::tie(stateZeroSize, stateNonZeroSize) = assumeZero(state, sizeVal);
 
   // Get the value of the Dest.
   SVal destVal = state->getSVal(Dest.Expression, LCtx);
@@ -1287,11 +1284,9 @@
 
   // See if the size argument is zero.
   SVal sizeVal = State->getSVal(Size.Expression, LCtx);
-  QualType sizeTy = Size.Expression->getType();
 
   ProgramStateRef stateZeroSize, stateNonZeroSize;
-  std::tie(stateZeroSize, stateNonZeroSize) =
-  assumeZero(C, State, sizeVal, sizeTy);
+  std::tie(stateZeroSize, stateNonZeroSize) = assumeZero(State, sizeVal);
 
   // If the size can be zero, the result will be 0 in that case, and we don't
   // have to check either of the buffers.
@@ -1367,8 +1362,7 @@
 SVal maxlenVal = state->getSVal(maxlenExpr, LCtx);
 
 ProgramStateRef stateZeroSize, stateNonZeroSize;
-std::tie(stateZeroSize, stateNonZeroSize) =
-  assumeZero(C, state, maxlenVal, maxlenExpr->getType());
+std::tie(stateZeroSize, stateNonZeroSize) = assumeZero(state, maxlenVal);
 

[PATCH] D83551: [PATCH 2/4][Sema][AArch64] Add semantics for arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1541
 
+def ArmSveVectorBits128 : TypeAttr {
+  let Spellings = [];

c-rhodes wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > c-rhodes wrote:
> > > > sdesmalen wrote:
> > > > > aaron.ballman wrote:
> > > > > > sdesmalen wrote:
> > > > > > > nit: Can you add a comment saying why these are undocumented (and 
> > > > > > > have no spellings)
> > > > > > Also, I think these are all missing `let SemaHandler = 0;` and `let 
> > > > > > ASTNode = 0;`
> > > > > > 
> > > > > > Is there a reason why we need N different type attributes instead 
> > > > > > of having a single type attribute which encodes the N as an 
> > > > > > argument? I think this may simplify the patch somewhat as you no 
> > > > > > longer need to switch over N as much.
> > > > > > Is there a reason why we need N different type attributes instead 
> > > > > > of having a single type attribute which encodes the N as an 
> > > > > > argument?
> > > > > AIUI this was a workaround for getting the value of N from an 
> > > > > AttributedType, because this only has `getAttrKind` to return the 
> > > > > attribute kind, but no way to get the corresponding argument/value. 
> > > > > This seemed like a simple way to do that without having to create a 
> > > > > new subclass for Type and having to support that in various places. 
> > > > > Is the latter the approach you were thinking of? (or is there perhaps 
> > > > > a simpler way?)
> > > > > Also, I think these are all missing let SemaHandler = 0; and let 
> > > > > ASTNode = 0;
> > > > 
> > > > Good to know. In SemaType I'm doing `CurType = 
> > > > State.getAttributedType(A, CurType, CurType);` which gives an 
> > > > `AttributedType` in the AST, should I still set `let ASTNode = 0;` in 
> > > > this case?
> > > > 
> > > > > Is there a reason why we need N different type attributes instead of 
> > > > > having a single type attribute which encodes the N as an argument?
> > > > 
> > > > As Sander mentioned, it seemed like the easiest solution, interested to 
> > > > know if there's a better approach however
> > > I was thinking specifically of creating a new `Type` subclass and 
> > > supporting it rather than adding 5 new attributes that only vary by a 
> > > bit-width (which means there's likely to be a 6th someday). It's not 
> > > immediately clear to me whether that's a really big ask for little gain 
> > > or not, though.
> > Ah, you're right, we may still need `ASTNode` to be kept around for that, 
> > good call.
> > Also, I think these are all missing let SemaHandler = 0; and let ASTNode = 
> > 0;
> 
> I've added `let SemaHandler = 0;` for the internal types and `let ASTNode = 
> 0;` for the user-facing attr.
> I was thinking specifically of creating a new Type subclass and supporting it 
> rather than adding 5 new attributes that only vary by a bit-width (which 
> means there's likely to be a 6th someday).

It would be nice if the `Attr` was accessible from the `AttributedType`, 
similar to how it is for `Decl`s, so something like:
```  if (const auto *AT = T->getAs())
if (ArmSveVectorBitsAttr *Attr = AT->getAttr())
  unsigned Width = Attr->getNumBits();```
Although I'm not sure if that makes sense or how easy it is. I do agree adding 
5 new attributes isn't ideal but for an initial implementation it's nice and 
simple. Would you be ok with us addressing this in a later patch?


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

https://reviews.llvm.org/D83551



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

[PATCH] D83868: Use TestClangConfig in AST Matchers tests and run them in more configurations

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Thanks for doing this change! I think it's a great step forward for the testing 
robustness. I'm scared by how many fixme's we have in this file, but at least 
they're now explicit, rather than implicit. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83868



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

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/Selection.cpp:227
 continue;
+  // Ignore tokens in disabled preprocessor sections.
+  if (Buf.expandedTokens(SM.getMacroArgExpandedLocation(T->location()))

I think this is more work than we'd like to do in this loop (the whole file 
could be selected!) and it's also not obviously correct - it relies on the 
assumption that any token that doesn't expand to something by itself, isn't a 
macro argument or macro name, is part of an ignored region. (That may or may 
not be correct, but it's a nontrivial assumption to make here).

I think the API we need is something like `vector 
TokenBuffer::expansionsOverlapping(ArrayRef Spelled)`.
This requires a couple of binary searches on the TokenBuffer side to compute, 
and on this side we can just look at the spelled token ranges for empty 
expansions and mark them in a bitmap.

I'm happy to try to put together a TokenBuffer patch if this is too much of a 
yak-shave, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83508



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

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(Sorry, I know that this is a lot of goalpost-shifting: I think this is now 
being solved at the right layer and with the right behavior, and it's just a 
question of finding a clear+fast implementation)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83508



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

[PATCH] D83915: [PowerPC] Remove QPX/A2Q BGQ/BGP support

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

Should we indicate planned removal in the Release Notes for version 11 and 
actual removal in those for version 12?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83915



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

[PATCH] D82381: [analyzer] Introduce small improvements to the solver infra

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko updated this revision to Diff 278454.
vsavchenko added a comment.

Rename function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82381

Files:
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -89,7 +89,7 @@
   }
 
   TriStateKind getCmpOpState(BinaryOperatorKind CurrentOP,
- BinaryOperatorKind QueriedOP) const {
+ BinaryOperatorKind QueriedOP) const {
 return CmpOpTable[getIndexFromOp(CurrentOP)][getIndexFromOp(QueriedOP)];
   }
 
@@ -364,6 +364,18 @@
   return newRanges;
 }
 
+RangeSet RangeSet::Delete(BasicValueFactory &BV, Factory &F,
+  const llvm::APSInt &Point) const {
+  llvm::APSInt Upper = Point;
+  llvm::APSInt Lower = Point;
+
+  ++Upper;
+  --Lower;
+
+  // Notice that the lower bound is greater than the upper bound.
+  return Intersect(BV, F, Upper, Lower);
+}
+
 void RangeSet::print(raw_ostream &os) const {
   bool isFirst = true;
   os << "{ ";
@@ -381,6 +393,107 @@
 
 namespace {
 
+//===--===//
+//Intersection functions
+//===--===//
+
+template 
+LLVM_NODISCARD inline RangeSet intersect(BasicValueFactory &BV,
+ RangeSet::Factory &F, RangeSet Head,
+ SecondTy Second, RestTy... Tail);
+
+template  struct IntersectionTraits;
+
+template  struct IntersectionTraits {
+  // Found RangeSet, no need to check any further
+  using Type = RangeSet;
+};
+
+template <> struct IntersectionTraits<> {
+  // We ran out of types, and we didn't find any RangeSet, so the result should
+  // be optional.
+  using Type = Optional;
+};
+
+template 
+struct IntersectionTraits {
+  // If current type is Optional or a raw pointer, we should keep looking.
+  using Type = typename IntersectionTraits::Type;
+};
+
+template 
+LLVM_NODISCARD inline EndTy intersect(BasicValueFactory &BV,
+  RangeSet::Factory &F, EndTy End) {
+  // If the list contains only RangeSet or Optional, simply return
+  // that range set.
+  return End;
+}
+
+LLVM_NODISCARD LLVM_ATTRIBUTE_UNUSED inline Optional
+intersect(BasicValueFactory &BV, RangeSet::Factory &F, const RangeSet *End) {
+  // This is an extraneous conversion from a raw pointer into Optional
+  if (End) {
+return *End;
+  }
+  return llvm::None;
+}
+
+template 
+LLVM_NODISCARD inline RangeSet intersect(BasicValueFactory &BV,
+ RangeSet::Factory &F, RangeSet Head,
+ RangeSet Second, RestTy... Tail) {
+  // Here we call either the  or  version
+  // of the function and can be sure that the result is RangeSet.
+  return intersect(BV, F, Head.Intersect(BV, F, Second), Tail...);
+}
+
+template 
+LLVM_NODISCARD inline RangeSet intersect(BasicValueFactory &BV,
+ RangeSet::Factory &F, RangeSet Head,
+ SecondTy Second, RestTy... Tail) {
+  if (Second) {
+// Here we call the  version of the function...
+return intersect(BV, F, Head, *Second, Tail...);
+  }
+  // ...and here it is either  or , which
+  // means that the result is definitely RangeSet.
+  return intersect(BV, F, Head, Tail...);
+}
+
+/// Main generic intersect function.
+/// It intersects all of the given range sets.  If some of the given arguments
+/// don't hold a range set (nullptr or llvm::None), the function will skip them.
+///
+/// Available representations for the arguments are:
+///   * RangeSet
+///   * Optional
+///   * RangeSet *
+/// Pointer to a RangeSet is automatically assumed to be nullable and will get
+/// checked as well as the optional version.  If this behaviour is undesired,
+/// please dereference the pointer in the call.
+///
+/// Return type depends on the arguments' types.  If we can be sure in compile
+/// time that there will be a range set as a result, the returning type is
+/// simply RangeSet, in other cases we have to back off to Optional.
+///
+/// Please, prefer optional range sets to raw pointers.  If the last argument is
+/// a raw pointer and all previous arguments are None, it will cost one
+/// additional check to convert RangeSet * into Optional.
+template 
+LLVM_NODISCARD inline
+typename IntersectionTraits::Type
+intersect(BasicValueFactory &BV, RangeSet::Factory &F, HeadTy Head,
+  SecondTy Second, 

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

[Lingda Li via Phabricator via cfe-commits]

lildmh added a comment.

In D67833#2155196 , @fhahn wrote:

> I think this change introduced the following warnings, where `auto &` is used 
> for types that are always copied. It would be great if you could take a look.
>
>   lvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp:8011:24: warning: loop 
> variable 'L' is always a copy because the range of type 
> 'clang::OMPMappableExprListClause::const_component_lists_range'
>  (aka 
> 'iterator_range::const_component_lists_iterator>')
>  does not return a reference [-Wrange-loop-analysis]
> for (const auto &L : C->component_lists()) {
>  ^
>   llvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp:8011:12: note: use 
> non-reference type 'std::__1::tuple llvm::ArrayRef, const 
> clang::ValueDecl *>'
> for (const auto &L : C->component_lists()) {
>  ^~~
>   llvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp:8016:24: warning: loop 
> variable 'L' is always a copy because the range of type 
> 'clang::OMPMappableExprListClause::const_component_lists_range'
>  (aka 
> 'iterator_range::const_component_lists_iterator>')
>  does not return a reference [-Wrange-loop-analysis]
> for (const auto &L : C->component_lists()) {
>  ^
>   llvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp:8016:12: note: use 
> non-reference type 'std::__1::tuple llvm::ArrayRef, const 
> clang::ValueDecl *>'
> for (const auto &L : C->component_lists()) {
>  ^~~
>   llvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp:8032:24: warning: loop 
> variable 'L' is always a copy because the range of type 
> 'clang::OMPMappableExprListClause::const_component_lists_range'
>  (aka 
> 'iterator_range::const_component_lists_iterator>')
>  does not return a reference [-Wrange-loop-analysis]
> for (const auto &L : C->component_lists()) {
>  ^
>   llvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp:8032:12: note: use 
> non-reference type 'std::__1::tuple llvm::ArrayRef, const 
> clang::ValueDecl *>'
> for (const auto &L : C->component_lists()) {
>  ^~~
>   3 warnings generated.
>


Thanks, I'll fix this soon. Basically `&L` needs to be `L` instead in those 3 
places. Since I don't have upload permission, it will be great if you can do 
the change quickly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67833



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Alex Bradbury via Phabricator via cfe-commits]

asb accepted this revision.
asb added a comment.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D83553: [PATCH 3/4][Sema][AArch64] Add codegen for arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes updated this revision to Diff 278466.
c-rhodes added a comment.

Changes:

- Rebased.
- Added comments for args in calls to `ConvertTypeForMem` when 
`EnforceFixedLengthSVEAttribute` is set and documented 
`EnforceFixedLengthSVEAttribute`.
- `s/getFixedSVETypeForMemory/getFixedLengthSVETypeForMemory/`
- Documented memory representation for fixed-length predicates.


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

https://reviews.llvm.org/D83553

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/CodeGenTypes.h
  clang/test/Sema/attr-arm-sve-vector-bits-bitcast.c
  clang/test/Sema/attr-arm-sve-vector-bits-call.c
  clang/test/Sema/attr-arm-sve-vector-bits-cast.c
  clang/test/Sema/attr-arm-sve-vector-bits-codegen.c
  clang/test/Sema/attr-arm-sve-vector-bits-globals.c
  clang/test/Sema/attr-arm-sve-vector-bits-types.c

Index: clang/test/Sema/attr-arm-sve-vector-bits-types.c
===
--- /dev/null
+++ clang/test/Sema/attr-arm-sve-vector-bits-types.c
@@ -0,0 +1,525 @@
+// REQUIRES: aarch64-registered-target
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -msve-vector-bits=128 -fallow-half-arguments-and-returns -S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-128
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -msve-vector-bits=256 -fallow-half-arguments-and-returns -S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-256
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -msve-vector-bits=512 -fallow-half-arguments-and-returns -S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-512
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -msve-vector-bits=1024 -fallow-half-arguments-and-returns -S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-1024
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -msve-vector-bits=2048 -fallow-half-arguments-and-returns -S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-2048
+
+#include 
+
+#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+
+typedef svint8_t fixed_int8_t __attribute__((arm_sve_vector_bits(N)));
+typedef svint16_t fixed_int16_t __attribute__((arm_sve_vector_bits(N)));
+typedef svint32_t fixed_int32_t __attribute__((arm_sve_vector_bits(N)));
+typedef svint64_t fixed_int64_t __attribute__((arm_sve_vector_bits(N)));
+
+typedef svuint8_t fixed_uint8_t __attribute__((arm_sve_vector_bits(N)));
+typedef svuint16_t fixed_uint16_t __attribute__((arm_sve_vector_bits(N)));
+typedef svuint32_t fixed_uint32_t __attribute__((arm_sve_vector_bits(N)));
+typedef svuint64_t fixed_uint64_t __attribute__((arm_sve_vector_bits(N)));
+
+typedef svfloat16_t fixed_float16_t __attribute__((arm_sve_vector_bits(N)));
+typedef svfloat32_t fixed_float32_t __attribute__((arm_sve_vector_bits(N)));
+typedef svfloat64_t fixed_float64_t __attribute__((arm_sve_vector_bits(N)));
+
+typedef svbfloat16_t fixed_bfloat16_t __attribute__((arm_sve_vector_bits(N)));
+
+typedef svbool_t fixed_bool_t __attribute__((arm_sve_vector_bits(N)));
+
+//===--===//
+// Structs and unions
+//===--===//
+#define DEFINE_STRUCT(ty) \
+  struct struct_##ty {\
+fixed_##ty##_t x; \
+  } struct_##ty;
+
+#define DEFINE_UNION(ty) \
+  union union_##ty { \
+fixed_##ty##_t x;\
+  } union_##ty;
+
+DEFINE_STRUCT(int8)
+DEFINE_STRUCT(int16)
+DEFINE_STRUCT(int32)
+DEFINE_STRUCT(int64)
+DEFINE_STRUCT(uint8)
+DEFINE_STRUCT(uint16)
+DEFINE_STRUCT(uint32)
+DEFINE_STRUCT(uint64)
+DEFINE_STRUCT(float16)
+DEFINE_STRUCT(float32)
+DEFINE_STRUCT(float64)
+DEFINE_STRUCT(bfloat16)
+DEFINE_STRUCT(bool)
+
+DEFINE_UNION(int8)
+DEFINE_UNION(int16)
+DEFINE_UNION(int32)
+DEFINE_UNION(int64)
+DEFINE_UNION(uint8)
+DEFINE_UNION(uint16)
+DEFINE_UNION(uint32)
+DEFINE_UNION(uint64)
+DEFINE_UNION(float16)
+DEFINE_UNION(float32)
+DEFINE_UNION(float64)
+DEFINE_UNION(bfloat16)
+DEFINE_UNION(bool)
+
+//===--===//
+// Global variables
+//===--===//
+fixed_int8_t global_i8;
+fixed_int16_t global_i16;
+fixed_int32_t global_i32;
+fixed_int64_t global_i64;
+
+fixed_uint8_t global_u8;
+fixed_uint16_t global_u16;
+fixed_uint32_t global_u32;
+fixed_uint64_t global_u64;
+
+fixed_float16_t global_f16;
+fixed_float32_t global_f32;
+fixed_float64_t global_f64;
+
+fixed_bfloat16_t global_bf16;
+
+fixed_bool_t global_bool;
+
+//===--===//
+// Global arrays
+//===-

[PATCH] D83922: [OpenMP] Fix map clause for unused var: don't ignore it

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

I would add checks for mapping of `declare target to/link` vars and checked if 
they work in runtime.




Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9468-9469
  CurSizes, CurMapTypes, PartialStruct);
+if (!CI->capturesThis())
+  MappedVarSet.insert(CI->getCapturedVar()->getCanonicalDecl());
+else

I would check that we capture a variable. We may capture VLA here as well.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9470-9471
+  MappedVarSet.insert(CI->getCapturedVar()->getCanonicalDecl());
+else
+  MappedVarSet.insert(nullptr);
 if (CurBasePointers.empty())

No need to insert `nullptr` here, it is not used later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83922



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

[clang] 3a624c3 - [Matrix] Add the matrix test from D83570. NFC.

[Sjoerd Meijer via cfe-commits]

Author: Sjoerd Meijer
Date: 2020-07-16T15:19:45+01:00
New Revision: 3a624c327adde62d075a4477e9bb9e6a2c186731

URL: 
https://github.com/llvm/llvm-project/commit/3a624c327adde62d075a4477e9bb9e6a2c186731
DIFF: 
https://github.com/llvm/llvm-project/commit/3a624c327adde62d075a4477e9bb9e6a2c186731.diff

LOG: [Matrix] Add the matrix test from D83570. NFC.

Added: 
clang/test/CodeGen/matrix-lowering-opt-levels.c

Modified: 


Removed: 




diff  --git a/clang/test/CodeGen/matrix-lowering-opt-levels.c 
b/clang/test/CodeGen/matrix-lowering-opt-levels.c
new file mode 100644
index ..8f86cc158f67
--- /dev/null
+++ b/clang/test/CodeGen/matrix-lowering-opt-levels.c
@@ -0,0 +1,20 @@
+// RUN: %clang -O0 -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
+// RUN: %clang -O1 -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
+// RUN: %clang -O2 -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
+// RUN: %clang -O3 -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
+// RUN: %clang -Ofast -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
+// RUN: %clang -Os -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
+// RUN: %clang -Oz -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
+
+// Smoke test that the matrix intrinsics are lowered at any optimisation level.
+
+typedef float m4x4_t __attribute__((matrix_type(4, 4)));
+
+m4x4_t f(m4x4_t a, m4x4_t b, m4x4_t c) {
+  //
+  // CHECK-LABEL: f(
+  // CHECK-NOT: @llvm.matrix
+  // CHECK:   }
+  //
+  return a + b * c;
+}



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

[PATCH] D83942: [analyzer][tests] Add a notion of project sizes

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

Szelethus added a comment.

I don't speak snek, but I approve this message!

Its also great that we have dedicated tiny/small projects to analyze locally.




Comment at: clang/utils/analyzer/ProjectMap.py:31
+SMALL: 1min-10min
+BIG:   10min-1h
+HUGE:  >1h

Just an observation rather then a suggestion, its interesting that we don't 
have a MEDIUM size in between SMALL and BIG. I think the TINY category 
describes the sub-minute runs well, and it'd be awkward to introduce a project 
in between SMALL and BIG, so I don't immediately see a time interval we need to 
categorize.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83942



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

[PATCH] D83942: [analyzer][tests] Add a notion of project sizes

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

Szelethus added a comment.

I suppose LLVM could be a HUGE project?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83942



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

[PATCH] D83553: [PATCH 3/4][Sema][AArch64] Add codegen for arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes marked 5 inline comments as done.
c-rhodes added inline comments.



Comment at: clang/lib/CodeGen/CodeGenTypes.h:138
+  llvm::Type *ConvertTypeForMem(QualType T, bool ForBitField = false,
+bool EnforceFixedLengthSVEAttribute = false);
 

efriedma wrote:
> The default for EnforceFixedLengthSVEAttribute seems backwards; I would 
> expect that almost everywhere that calls ConvertTypeForMem actually wants the 
> fixed-length type.  The scalable type only exists in registers.
> The default for EnforceFixedLengthSVEAttribute seems backwards; I would 
> expect that almost everywhere that calls ConvertTypeForMem actually wants the 
> fixed-length type. The scalable type only exists in registers.

It has no effect unless `T->isVLST()` so I think it makes sense.


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

https://reviews.llvm.org/D83553



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

[PATCH] D83942: [analyzer][tests] Add a notion of project sizes

[Valeriy Savchenko via Phabricator via cfe-commits]

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

In D83942#2155762 , @Szelethus wrote:

> I don't speak snek, but I approve this message!


Thanks 😊

> I suppose LLVM could be a HUGE project?

Yurp, LLVM, pytorch, (surprisingly!) CMake




Comment at: clang/utils/analyzer/ProjectMap.py:31
+SMALL: 1min-10min
+BIG:   10min-1h
+HUGE:  >1h

Szelethus wrote:
> Just an observation rather then a suggestion, its interesting that we don't 
> have a MEDIUM size in between SMALL and BIG. I think the TINY category 
> describes the sub-minute runs well, and it'd be awkward to introduce a 
> project in between SMALL and BIG, so I don't immediately see a time interval 
> we need to categorize.
Yeah, maybe it's good to add MEDIUM with 10min-30min


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83942



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

[PATCH] D83922: [OpenMP] Fix map clause for unused var: don't ignore it

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7948-7949
+   MapFlagsArrayTy &Types,
+   const llvm::DenseSet &SkipVarSet =
+   llvm::DenseSet()) const {
 // We have to process the component lists that relate with the same

Use `llvm::DenseSet> &SkipVarSet`.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9439
 llvm::DenseMap LambdaPointers;
+llvm::DenseSet MappedVarSet;
 

`llvm::DenseSet> MappedVarSet;`



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9469
+if (!CI->capturesThis())
+  MappedVarSet.insert(CI->getCapturedVar()->getCanonicalDecl());
+else

No need to get canonical decl here for `llvm::DenseSet>`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83922



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

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

This new modifier must be enabed only for OpenMP 5.1, need to add the checks 
for OpenMP version. Also, this change should not affect the functionality of 
previous versions




Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7043-7044
 OMP_MAP_CLOSE = 0x400,
+/// Produce a runtime error if the data is not already allocated.
+OMP_MAP_PRESENT = 0x800,
 /// The 16 MSBs of the flags indicate whether the entry is member of some

Better to use thу next value to avoid compatibility issues with XLC. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83061



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

[PATCH] D80802: [RISCV] Upgrade RVV MC to v0.9.

[Ferran Pallarès Roca via Phabricator via cfe-commits]

fpallares added a comment.

Apologies we didn't identify this earlier but with the change of the mask 
register layout (`MLEN=1`) the overlap constraints involving the mask register 
were modified:

//**RVV-0.8, Section 5.3. Vector Masking:**//

> The destination vector register group for a masked vector instruction can 
> only overlap the source mask register (v0) when LMUL=1. Otherwise, an illegal 
> instruction exception is raised.

//**RVV-0.9, Section 5.3. Vector Masking:**//

> The destination vector register group for a masked vector instruction cannot 
> overlap the source mask register (v0), unless the destination vector register 
> is being written with a mask value (e.g., comparisons) or the scalar result 
> of a reduction. Otherwise, an illegal instruction exception is raised.

The change was introduced in this commit 
.

From my understanding, with this change an instruction such as the following 
should be rejected in RVV-0.9:

  vadd.vv   v0, v1, v2, v0.t

Also note that `vadc`/`vsbc` already have this behaviour.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80802



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

[clang] a7a07a8 - Follow up of rG3a624c327add: pacify buildbot, add "REQUIRES: aarch64" to test

[Sjoerd Meijer via cfe-commits]

Author: Sjoerd Meijer
Date: 2020-07-16T15:38:36+01:00
New Revision: a7a07a8d63b2008750347932e351d479a45bfc2c

URL: 
https://github.com/llvm/llvm-project/commit/a7a07a8d63b2008750347932e351d479a45bfc2c
DIFF: 
https://github.com/llvm/llvm-project/commit/a7a07a8d63b2008750347932e351d479a45bfc2c.diff

LOG: Follow up of rG3a624c327add: pacify buildbot, add "REQUIRES: aarch64" to 
test

Added: 


Modified: 
clang/test/CodeGen/matrix-lowering-opt-levels.c

Removed: 




diff  --git a/clang/test/CodeGen/matrix-lowering-opt-levels.c 
b/clang/test/CodeGen/matrix-lowering-opt-levels.c
index 8f86cc158f67..a288d18264bf 100644
--- a/clang/test/CodeGen/matrix-lowering-opt-levels.c
+++ b/clang/test/CodeGen/matrix-lowering-opt-levels.c
@@ -1,3 +1,5 @@
+// REQUIRES: aarch64-registered-target
+
 // RUN: %clang -O0 -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
 // RUN: %clang -O1 -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
 // RUN: %clang -O2 -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s



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

[PATCH] D83702: [AIX]Generate debug info for static init related functions

[Xiangling Liao via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG69f3378ad65b: [AIX]Generate debug info for static init 
related functions (authored by Xiangling_L).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83702

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/aix-static-init-debug-info.cpp


Index: clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -x c++ \
+// RUN: -debug-info-kind=limited < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -emit-llvm -x c++ \
+// RUN: -debug-info-kind=limited  < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
+
+struct X {
+  X();
+  ~X();
+};
+
+X v;
+
+// CHECK: define internal void @__cxx_global_var_init() [[ATTR:#[0-9]+]] !dbg 
![[DBGVAR16:[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN1XC1Ev(%struct.X* @v), !dbg ![[DBGVAR19:[0-9]+]]
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_v) [[ATTR:#[0-9]+]], !dbg 
![[DBGVAR19]]
+// CHECK:   ret void, !dbg ![[DBGVAR19]]
+// CHECK: }
+
+// CHECK: define internal void @__dtor_v() [[ATTR:#[0-9]+]] !dbg 
![[DBGVAR20:[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN1XD1Ev(%struct.X* @v), !dbg ![[DBGVAR21:[0-9]+]]
+// CHECK:   ret void, !dbg ![[DBGVAR21]]
+// CHECK: }
+
+// CHECK: define internal void @__finalize_v() [[ATTR:#[0-9]+]] !dbg 
![[DBGVAR22:[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_v) [[ATTR:#[0-9]+]], !dbg 
![[DBGVAR24:[0-9]+]]
+// CHECK:   %needs_destruct = icmp eq i32 %0, 0, !dbg ![[DBGVAR24]]
+// CHECK:   br i1 %needs_destruct, label %destruct.call, label %destruct.end, 
!dbg ![[DBGVAR24]]
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor_v(), !dbg ![[DBGVAR24]]
+// CHECK:   br label %destruct.end, !dbg ![[DBGVAR24]]
+
+// CHECK: destruct.end:
+// CHECK:   ret void, !dbg ![[DBGVAR24]]
+// CHECK: }
+
+// CHECK: define void 
@__sinit8000_clang_c3236cbaa79f2bae3a15e6379a05f625() [[ATTR:#[0-9]+]] !dbg 
![[DBGVAR25:[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init(), !dbg ![[DBGVAR26:[0-9]+]]
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define void 
@__sterm8000_clang_c3236cbaa79f2bae3a15e6379a05f625() [[ATTR:#[0-9]+]] !dbg 
![[DBGVAR27:[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @__finalize_v(), !dbg ![[DBGVAR28:[0-9]+]]
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: ![[DBGVAR16]] = distinct !DISubprogram(name: 
"__cxx_global_var_init", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 14, type: 
!{{[0-9]+}}, scopeLine: 14, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, 
unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR19]] = !DILocation(line: 14, column: 3, scope: 
![[DBGVAR16]])
+// CHECK: ![[DBGVAR20]] = distinct !DISubprogram(name: "__dtor_v", scope: 
!{{[0-9]+}}, file: !{{[0-9]+}}, line: 14, type: !{{[0-9]+}}, scopeLine: 14, 
spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !{{[0-9]+}}, 
retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR21]] = !DILocation(line: 14, column: 3, scope: 
![[DBGVAR20]])
+// CHECK: ![[DBGVAR22]] = distinct !DISubprogram(linkageName: "__finalize_v", 
scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 14, type: !{{[0-9]+}}, scopeLine: 
14, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, 
unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR24]] = !DILocation(line: 14, column: 3, scope: 
![[DBGVAR22]])
+// CHECK: ![[DBGVAR25]] = distinct !DISubprogram(linkageName: 
"__sinit8000_clang_c3236cbaa79f2bae3a15e6379a05f625", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, type: !{{[0-9]+}}, flags: DIFlagArtificial, spFlags: 
DISPFlagDefinition, unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR26]] = !DILocation(line: 0, scope: ![[DBGVAR25]])
+// CHECK: ![[DBGVAR27]] = distinct !DISubprogram(linkageName: 
"__sterm8000_clang_c3236cbaa79f2bae3a15e6379a05f625", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, type: !{{[0-9]+}}, flags: DIFlagArtificial, spFlags: 
DISPFlagDefinition, unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR28]] = !DILocation(line: 0, scope: ![[DBGVAR27]])
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -4567,7 +4567,8 @@
   CodeGenFunction CGF(CGM);
 
   CGF.StartFunction(GlobalDecl(), CGM.getContext().VoidTy, StermFinalizer, FI,
-FunctionArgList());
+FunctionArgList(), D.getLocation(),
+D.getInit()->getE

[clang] 69f3378 - [AIX]Generate debug info for static init related functions

[Xiangling Liao via cfe-commits]

Author: Xiangling Liao
Date: 2020-07-16T10:43:10-04:00
New Revision: 69f3378ad65b41c979acc1bcb4968d2247e6adf7

URL: 
https://github.com/llvm/llvm-project/commit/69f3378ad65b41c979acc1bcb4968d2247e6adf7
DIFF: 
https://github.com/llvm/llvm-project/commit/69f3378ad65b41c979acc1bcb4968d2247e6adf7.diff

LOG: [AIX]Generate debug info for static init related functions

Set the debug location for static init related functions(__dtor
and __finalize) so we can generate valid debug info on AIX by invoking
-g with clang or -debug-info-kind=limited with clang_cc1.

This also works for any other future targets who may use sinit and
sterm functions for static initialization, where a direct call to
dtor will be generated within finalize function body.

This patch also aims at validating that the debug info generated
is correct for AIX sinit related functions.

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

Added: 
clang/test/CodeGenCXX/aix-static-init-debug-info.cpp

Modified: 
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/ItaniumCXXABI.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index 5a8500364295..4e941021daa3 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -246,7 +246,8 @@ llvm::Function *CodeGenFunction::createAtExitStub(const 
VarDecl &VD,
   CodeGenFunction CGF(CGM);
 
   CGF.StartFunction(GlobalDecl(&VD, DynamicInitKind::AtExit),
-CGM.getContext().VoidTy, fn, FI, FunctionArgList());
+CGM.getContext().VoidTy, fn, FI, FunctionArgList(),
+VD.getLocation(), VD.getInit()->getExprLoc());
 
   llvm::CallInst *call = CGF.Builder.CreateCall(dtor, addr);
 

diff  --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp 
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index 80de2a6e3950..12d00c7d59a3 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -4567,7 +4567,8 @@ void XLCXXABI::emitCXXStermFinalizer(const VarDecl &D, 
llvm::Function *dtorStub,
   CodeGenFunction CGF(CGM);
 
   CGF.StartFunction(GlobalDecl(), CGM.getContext().VoidTy, StermFinalizer, FI,
-FunctionArgList());
+FunctionArgList(), D.getLocation(),
+D.getInit()->getExprLoc());
 
   // The unatexit subroutine unregisters __dtor functions that were previously
   // registered by the atexit subroutine. If the referenced function is found,

diff  --git a/clang/test/CodeGenCXX/aix-static-init-debug-info.cpp 
b/clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
new file mode 100644
index ..39de0cdd513a
--- /dev/null
+++ b/clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -x c++ \
+// RUN: -debug-info-kind=limited < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -emit-llvm -x c++ \
+// RUN: -debug-info-kind=limited  < %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
+
+struct X {
+  X();
+  ~X();
+};
+
+X v;
+
+// CHECK: define internal void @__cxx_global_var_init() [[ATTR:#[0-9]+]] !dbg 
![[DBGVAR16:[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN1XC1Ev(%struct.X* @v), !dbg ![[DBGVAR19:[0-9]+]]
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_v) [[ATTR:#[0-9]+]], !dbg 
![[DBGVAR19]]
+// CHECK:   ret void, !dbg ![[DBGVAR19]]
+// CHECK: }
+
+// CHECK: define internal void @__dtor_v() [[ATTR:#[0-9]+]] !dbg 
![[DBGVAR20:[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @_ZN1XD1Ev(%struct.X* @v), !dbg ![[DBGVAR21:[0-9]+]]
+// CHECK:   ret void, !dbg ![[DBGVAR21]]
+// CHECK: }
+
+// CHECK: define internal void @__finalize_v() [[ATTR:#[0-9]+]] !dbg 
![[DBGVAR22:[0-9]+]] {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_v) [[ATTR:#[0-9]+]], !dbg 
![[DBGVAR24:[0-9]+]]
+// CHECK:   %needs_destruct = icmp eq i32 %0, 0, !dbg ![[DBGVAR24]]
+// CHECK:   br i1 %needs_destruct, label %destruct.call, label %destruct.end, 
!dbg ![[DBGVAR24]]
+
+// CHECK: destruct.call:
+// CHECK:   call void @__dtor_v(), !dbg ![[DBGVAR24]]
+// CHECK:   br label %destruct.end, !dbg ![[DBGVAR24]]
+
+// CHECK: destruct.end:
+// CHECK:   ret void, !dbg ![[DBGVAR24]]
+// CHECK: }
+
+// CHECK: define void 
@__sinit8000_clang_c3236cbaa79f2bae3a15e6379a05f625() [[ATTR:#[0-9]+]] !dbg 
![[DBGVAR25:[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init(), !dbg ![[DBGVAR26:[0-9]+]]
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define void 
@__sterm8000_clang_c3236cbaa79f2bae3a15e6379a05f625() [[ATTR:#[0-9]+]] !dbg 
![[DBGVAR27:[0-9]+]] {
+// CHECK: entry:
+// CHECK:   call void @__finalize_v(), !dbg ![[DBGVAR28:[0-9]+]]
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: ![[DBGVAR16]] = distinct !DISubprogram(name: 
"__cxx_global_v

[libunwind] 0f03626 - [runtimes][NFC] Remove unused or unnecessary CMake variables

[Louis Dionne via cfe-commits]

Author: Louis Dionne
Date: 2020-07-16T10:47:08-04:00
New Revision: 0f03626fbf40ba1a74badf2d1476a550535c184f

URL: 
https://github.com/llvm/llvm-project/commit/0f03626fbf40ba1a74badf2d1476a550535c184f
DIFF: 
https://github.com/llvm/llvm-project/commit/0f03626fbf40ba1a74badf2d1476a550535c184f.diff

LOG: [runtimes][NFC] Remove unused or unnecessary CMake variables

Added: 


Modified: 
libcxx/CMakeLists.txt
libcxx/test/lit.site.cfg.in
libcxxabi/CMakeLists.txt
libcxxabi/test/lit.site.cfg.in
libunwind/CMakeLists.txt
libunwind/test/lit.site.cfg.in

Removed: 




diff  --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 26bf553ddeed..caf655d6799a 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -407,14 +407,10 @@ endif ()
 # Configure System
 
#===
 
-set(LIBCXX_COMPILER${CMAKE_CXX_COMPILER})
 set(LIBCXX_SOURCE_DIR  ${CMAKE_CURRENT_SOURCE_DIR})
 set(LIBCXX_BINARY_DIR  ${CMAKE_CURRENT_BINARY_DIR})
 set(LIBCXX_BINARY_INCLUDE_DIR "${LIBCXX_BINARY_DIR}/include/c++build")
 
-string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION
-   ${PACKAGE_VERSION})
-
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
   set(LIBCXX_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
   set(LIBCXX_HEADER_DIR ${LLVM_BINARY_DIR})

diff  --git a/libcxx/test/lit.site.cfg.in b/libcxx/test/lit.site.cfg.in
index 939776f2287d..1f3370ccc9bc 100644
--- a/libcxx/test/lit.site.cfg.in
+++ b/libcxx/test/lit.site.cfg.in
@@ -3,7 +3,7 @@
 import os
 import site
 
-config.cxx_under_test   = "@LIBCXX_COMPILER@"
+config.cxx_under_test   = "@CMAKE_CXX_COMPILER@"
 config.project_obj_root = "@CMAKE_BINARY_DIR@"
 config.libcxx_src_root  = "@LIBCXX_SOURCE_DIR@"
 config.libcxx_obj_root  = "@LIBCXX_BINARY_DIR@"

diff  --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index 8881a5018dc4..e4e20d950b89 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -151,13 +151,9 @@ set(CMAKE_MODULE_PATH
   ${CMAKE_MODULE_PATH}
   )
 
-set(LIBCXXABI_COMPILER${CMAKE_CXX_COMPILER})
 set(LIBCXXABI_SOURCE_DIR  ${CMAKE_CURRENT_SOURCE_DIR})
 set(LIBCXXABI_BINARY_DIR  ${CMAKE_CURRENT_BINARY_DIR})
 
-string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION
-   ${PACKAGE_VERSION})
-
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
   set(LIBCXXABI_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
   set(LIBCXXABI_INSTALL_LIBRARY_DIR 
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)

diff  --git a/libcxxabi/test/lit.site.cfg.in b/libcxxabi/test/lit.site.cfg.in
index 75fde7ee9250..06d5706da7d2 100644
--- a/libcxxabi/test/lit.site.cfg.in
+++ b/libcxxabi/test/lit.site.cfg.in
@@ -3,7 +3,7 @@
 import os
 import site
 
-config.cxx_under_test   = "@LIBCXXABI_COMPILER@"
+config.cxx_under_test   = "@CMAKE_CXX_COMPILER@"
 config.project_obj_root = "@CMAKE_BINARY_DIR@"
 config.libcxxabi_src_root   = "@LIBCXXABI_SOURCE_DIR@"
 config.libcxxabi_obj_root   = "@LIBCXXABI_BINARY_DIR@"

diff  --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index f20893f4aa86..b50550dc376e 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -182,13 +182,9 @@ set(CMAKE_MODULE_PATH
 "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
 ${CMAKE_MODULE_PATH})
 
-set(LIBUNWIND_COMPILER${CMAKE_CXX_COMPILER})
 set(LIBUNWIND_SOURCE_DIR  ${CMAKE_CURRENT_SOURCE_DIR})
 set(LIBUNWIND_BINARY_DIR  ${CMAKE_CURRENT_BINARY_DIR})
 
-string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION
-   ${PACKAGE_VERSION})
-
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
   set(LIBUNWIND_LIBRARY_DIR 
${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
   set(LIBUNWIND_INSTALL_LIBRARY_DIR 
lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)

diff  --git a/libunwind/test/lit.site.cfg.in b/libunwind/test/lit.site.cfg.in
index d0f0e08fc926..30a996cf3783 100644
--- a/libunwind/test/lit.site.cfg.in
+++ b/libunwind/test/lit.site.cfg.in
@@ -3,7 +3,7 @@
 import os
 import site
 
-config.cxx_under_test   = "@LIBUNWIND_COMPILER@"
+config.cxx_under_test   = "@CMAKE_CXX_COMPILER@"
 config.project_obj_root = "@CMAKE_BINARY_DIR@"
 config.libunwind_src_root   = "@LIBUNWIND_SOURCE_DIR@"
 config.libunwind_obj_root   = "@LIBUNWIND_BINARY_DIR@"



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

[PATCH] D83550: [PATCH 1/4][Sema][AArch64] Add parsing support for arm_sve_vector_bits attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM aside from a small nit.




Comment at: clang/lib/Sema/SemaType.cpp:7698
+llvm::APSInt &Result) {
+  Expr *AttrExpr = static_cast(Attr.getArgAsExpr(0));
+  if (AttrExpr->isTypeDependent() || AttrExpr->isValueDependent() ||

`const auto *` and you should use `cast<>` instead of `static_cast<>`.


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

https://reviews.llvm.org/D83550



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

[clang] 31248b4 - Last attempt for rG3a624c327add: one test fails with the NPM,

[Sjoerd Meijer via cfe-commits]

Author: Sjoerd Meijer
Date: 2020-07-16T16:12:47+01:00
New Revision: 31248b4785c14e50c1694fc986c69afcea64bd81

URL: 
https://github.com/llvm/llvm-project/commit/31248b4785c14e50c1694fc986c69afcea64bd81
DIFF: 
https://github.com/llvm/llvm-project/commit/31248b4785c14e50c1694fc986c69afcea64bd81.diff

LOG: Last attempt for rG3a624c327add: one test fails with the NPM,
so disable that one for now.

Added: 


Modified: 
clang/test/CodeGen/matrix-lowering-opt-levels.c

Removed: 




diff  --git a/clang/test/CodeGen/matrix-lowering-opt-levels.c 
b/clang/test/CodeGen/matrix-lowering-opt-levels.c
index a288d18264bf..db003f140ee8 100644
--- a/clang/test/CodeGen/matrix-lowering-opt-levels.c
+++ b/clang/test/CodeGen/matrix-lowering-opt-levels.c
@@ -1,6 +1,3 @@
-// REQUIRES: aarch64-registered-target
-
-// RUN: %clang -O0 -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
 // RUN: %clang -O1 -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
 // RUN: %clang -O2 -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
 // RUN: %clang -O3 -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
@@ -10,6 +7,10 @@
 
 // Smoke test that the matrix intrinsics are lowered at any optimisation level.
 
+// FIXME: this fails with the NPM:
+//
+// RUN: %clang -O0 -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
+
 typedef float m4x4_t __attribute__((matrix_type(4, 4)));
 
 m4x4_t f(m4x4_t a, m4x4_t b, m4x4_t c) {



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

[PATCH] D82576: [PowerPC][Power10] Implement low-order Vector Modulus Builtins, and add Vector Multiply/Divide/Modulus Builtins Tests

[Amy Kwan via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGen/builtins-ppc-p10vector.c:28
+  return vec_mul(vulla, vullb);
+}
+

bsaleil wrote:
> Are the tests for `vec_mul` with `v4i32` missing ?
I should probably reword the description. So for Power10, we actually have new 
instructions for:
- mul with v2i64
- div with v4i32 and v2i64
- mod with v4i32 and v2i64

Which is why I have only added `vec_mul` for `v4i32` here. All of those 
functions exist in altivec.h already except for `vec_mod`, which I have added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82576



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

[clang] 0160ad8 - And now really disable that test.

[Sjoerd Meijer via cfe-commits]

Author: Sjoerd Meijer
Date: 2020-07-16T16:14:47+01:00
New Revision: 0160ad802e899c2922bc9b29564080c22eb0908c

URL: 
https://github.com/llvm/llvm-project/commit/0160ad802e899c2922bc9b29564080c22eb0908c
DIFF: 
https://github.com/llvm/llvm-project/commit/0160ad802e899c2922bc9b29564080c22eb0908c.diff

LOG: And now really disable that test.

Added: 


Modified: 
clang/test/CodeGen/matrix-lowering-opt-levels.c

Removed: 




diff  --git a/clang/test/CodeGen/matrix-lowering-opt-levels.c 
b/clang/test/CodeGen/matrix-lowering-opt-levels.c
index db003f140ee8..9edecbe46bc8 100644
--- a/clang/test/CodeGen/matrix-lowering-opt-levels.c
+++ b/clang/test/CodeGen/matrix-lowering-opt-levels.c
@@ -9,7 +9,7 @@
 
 // FIXME: this fails with the NPM:
 //
-// RUN: %clang -O0 -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
+// %clang -O0 -fenable-matrix -S -emit-llvm %s -o - | FileCheck  %s
 
 typedef float m4x4_t __attribute__((matrix_type(4, 4)));
 



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

[PATCH] D83551: [PATCH 2/4][Sema][AArch64] Add semantics for arm_sve_vector_bits attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/Type.h:1928
 
+  /// Determines if this is vector-length sized typed (VLST), i.e. a
+  /// sizeless type with the 'arm_sve_vector_bits(N)' attribute applied.

... this is a vector-length-sized type...



Comment at: clang/include/clang/Basic/Attr.td:1541
 
+def ArmSveVectorBits128 : TypeAttr {
+  let Spellings = [];

c-rhodes wrote:
> c-rhodes wrote:
> > aaron.ballman wrote:
> > > aaron.ballman wrote:
> > > > c-rhodes wrote:
> > > > > sdesmalen wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > sdesmalen wrote:
> > > > > > > > nit: Can you add a comment saying why these are undocumented 
> > > > > > > > (and have no spellings)
> > > > > > > Also, I think these are all missing `let SemaHandler = 0;` and 
> > > > > > > `let ASTNode = 0;`
> > > > > > > 
> > > > > > > Is there a reason why we need N different type attributes instead 
> > > > > > > of having a single type attribute which encodes the N as an 
> > > > > > > argument? I think this may simplify the patch somewhat as you no 
> > > > > > > longer need to switch over N as much.
> > > > > > > Is there a reason why we need N different type attributes instead 
> > > > > > > of having a single type attribute which encodes the N as an 
> > > > > > > argument?
> > > > > > AIUI this was a workaround for getting the value of N from an 
> > > > > > AttributedType, because this only has `getAttrKind` to return the 
> > > > > > attribute kind, but no way to get the corresponding argument/value. 
> > > > > > This seemed like a simple way to do that without having to create a 
> > > > > > new subclass for Type and having to support that in various places. 
> > > > > > Is the latter the approach you were thinking of? (or is there 
> > > > > > perhaps a simpler way?)
> > > > > > Also, I think these are all missing let SemaHandler = 0; and let 
> > > > > > ASTNode = 0;
> > > > > 
> > > > > Good to know. In SemaType I'm doing `CurType = 
> > > > > State.getAttributedType(A, CurType, CurType);` which gives an 
> > > > > `AttributedType` in the AST, should I still set `let ASTNode = 0;` in 
> > > > > this case?
> > > > > 
> > > > > > Is there a reason why we need N different type attributes instead 
> > > > > > of having a single type attribute which encodes the N as an 
> > > > > > argument?
> > > > > 
> > > > > As Sander mentioned, it seemed like the easiest solution, interested 
> > > > > to know if there's a better approach however
> > > > I was thinking specifically of creating a new `Type` subclass and 
> > > > supporting it rather than adding 5 new attributes that only vary by a 
> > > > bit-width (which means there's likely to be a 6th someday). It's not 
> > > > immediately clear to me whether that's a really big ask for little gain 
> > > > or not, though.
> > > Ah, you're right, we may still need `ASTNode` to be kept around for that, 
> > > good call.
> > > Also, I think these are all missing let SemaHandler = 0; and let ASTNode 
> > > = 0;
> > 
> > I've added `let SemaHandler = 0;` for the internal types and `let ASTNode = 
> > 0;` for the user-facing attr.
> > I was thinking specifically of creating a new Type subclass and supporting 
> > it rather than adding 5 new attributes that only vary by a bit-width (which 
> > means there's likely to be a 6th someday).
> 
> It would be nice if the `Attr` was accessible from the `AttributedType`, 
> similar to how it is for `Decl`s, so something like:
> ```  if (const auto *AT = T->getAs())
> if (ArmSveVectorBitsAttr *Attr = AT->getAttr())
>   unsigned Width = Attr->getNumBits();```
> Although I'm not sure if that makes sense or how easy it is. I do agree 
> adding 5 new attributes isn't ideal but for an initial implementation it's 
> nice and simple. Would you be ok with us addressing this in a later patch?
> It would be nice if the Attr was accessible from the AttributedType, similar 
> to how it is for Decls, so something like:

You can do that through an `AttributedTypeLoc` object, which I think should be 
available from the situations you need to check this through a `TypeSourceInfo 
*` for the type. Then you can use `AttributedTypeLoc::getAttr()` to get the 
semantic attribute.

> Would you be ok with us addressing this in a later patch?

No and yes. It's a novel design to have a user-facing attribute that is never 
hooked up in the AST but is instead used to decide which spellingless attribute 
to use instead, so I'd strongly prefer to find a solution that doesn't use this 
approach. I also think we'll wind up with cleaner code from this. That said, if 
it turns out to be awkward to do because there's too much code required to 
support it, then I can likely hold my nose.


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

https://reviews.llvm.org/D83551



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

[PATCH] D83099: [clangd] Store index in '.cache/clangd/index' instead of '.clangd/index'

[David Van Cleve via Phabricator via cfe-commits]

davidvancleve added a comment.

Yes, these two are exactly the case!

> - you'd be using an editor/plugin that sends compile commands over LSP (such 
> as YCM + ycm_extra_conf). What are you using?
> - there should be relatively few *.idx files inside the extra directories 
> (the ones not near your compilation database), corresponding to files you've 
> had open rather than the whole project




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

[PATCH] D83955: [PowerPC][Power10] Implementation of 128-bit Binary Vector Multiply builtins

[Albion Fung via Phabricator via cfe-commits]

Conanap created this revision.
Conanap added reviewers: PowerPC, saghir, nemanjai, hfinkel.
Conanap added projects: LLVM, clang, PowerPC.
Herald added subscribers: steven.zhang, kbarton.

This patch implements 128-bit Binary Vector Multiply builtins for PowerPC10.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83955

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/p10-vector-multiply.ll

Index: llvm/test/CodeGen/PowerPC/p10-vector-multiply.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/p10-vector-multiply.ll
@@ -0,0 +1,56 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
+; RUN:   FileCheck %s
+; This test case aims to test the vector multiply instructions on Power10.
+
+declare <1 x i128> @llvm.ppc.altivec.vmuleud(<2 x i64>, <2 x i64>) nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vmuloud(<2 x i64>, <2 x i64>) nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vmulesd(<2 x i64>, <2 x i64>) nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vmulosd(<2 x i64>, <2 x i64>) nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vmsumcud(<2 x i64>, <2 x i64>, <1 x i128>) nounwind readnone
+
+define <1 x i128> @test_vmuleud(<2 x i64> %x, <2 x i64> %y) nounwind readnone {
+; CHECK-LABEL: test_vmuleud:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmuleud v2, v2, v3
+; CHECK-NEXT:blr
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vmuleud(<2 x i64> %x, <2 x i64> %y)
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vmuloud(<2 x i64> %x, <2 x i64> %y) nounwind readnone {
+; CHECK-LABEL: test_vmuloud:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmuloud v2, v2, v3
+; CHECK-NEXT:blr
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vmuloud(<2 x i64> %x, <2 x i64> %y)
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vmulesd(<2 x i64> %x, <2 x i64> %y) nounwind readnone {
+; CHECK-LABEL: test_vmulesd:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmulesd v2, v2, v3
+; CHECK-NEXT:blr
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vmulesd(<2 x i64> %x, <2 x i64> %y)
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vmulosd(<2 x i64> %x, <2 x i64> %y) nounwind readnone {
+; CHECK-LABEL: test_vmulosd:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmulosd v2, v2, v3
+; CHECK-NEXT:blr
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vmulosd(<2 x i64> %x, <2 x i64> %y)
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vmsumcud(<2 x i64> %x, <2 x i64> %y, <1 x i128> %z) nounwind readnone {
+; CHECK-LABEL: test_vmsumcud:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmsumcud v2, v2, v3, v4
+; CHECK-NEXT:blr
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vmsumcud(<2 x i64> %x, <2 x i64> %y, <1 x i128> %z)
+  ret <1 x i128> %tmp
+}
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -977,20 +977,31 @@
   }
 
   def VMULESD : VXForm_1<968, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
- "vmulesd $vD, $vA, $vB", IIC_VecGeneral, []>;
+ "vmulesd $vD, $vA, $vB", IIC_VecGeneral,
+ [(set v1i128:$vD, (int_ppc_altivec_vmulesd v2i64:$vA,
+ v2i64:$vB))]>;
 
   def VMULEUD : VXForm_1<712, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
- "vmuleud $vD, $vA, $vB", IIC_VecGeneral, []>;
+ "vmuleud $vD, $vA, $vB", IIC_VecGeneral,
+ [(set v1i128:$vD, (int_ppc_altivec_vmuleud v2i64:$vA,
+ v2i64:$vB))]>;
 
   def VMULOSD : VXForm_1<456, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
- "vmulosd $vD, $vA, $vB", IIC_VecGeneral, []>;
+ "vmulosd $vD, $vA, $vB", IIC_VecGeneral,
+ [(set v1i128:$vD, (int_ppc_altivec_vmulosd v2i64:$vA,
+ v2i64:$vB))]>;
 
   def VMULOUD : VXForm_1<200, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
- "vmuloud $vD, $vA, $vB", IIC_VecGeneral, []>;
+ "vmuloud $vD, $vA, $vB", IIC_VecGeneral,
+ [(set v1i128:$vD, (int_ppc_altivec_vmuloud v2i64:$vA,
+ v2i64:$vB))]>;
 
   def VMSUMCUD : VAForm_1a<23, (outs vrrc:$vD),
(ins vrrc:$vA, vrrc:$vB, vrrc:$vC),
-   "vmsumcud $vD, $vA, $vB, $vC", IIC_VecGeneral, []>;
+   "vmsumcud $vD, $vA, $vB, $vC", 

[PATCH] D83955: [PowerPC][Power10] Implementation of 128-bit Binary Vector Multiply builtins

[Amy Kwan via Phabricator via cfe-commits]

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



Comment at: clang/lib/Headers/altivec.h:5472
+
+#ifdef _ARCH_PWR10
+static __inline__ vector unsigned __int128 __ATTRS_o_ai

Please use `__POWER10_VECTOR__` for the `#ifdef`s. (Here and below)



Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:982
+ [(set v1i128:$vD, (int_ppc_altivec_vmulesd v2i64:$vA,
+ v2i64:$vB))]>;
 

nit: line up the indentation under `v1i128` (for here and below)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83955



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

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

[Lingda Li via Phabricator via cfe-commits]

lildmh added a comment.

Fix is at https://reviews.llvm.org/D83959
@Meinersbur Michael, could you please help upstream the patch above? Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67833



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

[PATCH] D83550: [PATCH 1/4][Sema][AArch64] Add parsing support for arm_sve_vector_bits attribute

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added a comment.

Thanks for doing this.  FWIW, the patch LGTM from a spec point of view.  I just 
saw one minor spelling nit.




Comment at: clang/include/clang/Basic/AttrDocs.td:4882
+to the SVE predicate type ``svbool_t``, this excludes tuple types such as
+``svint32x4_t``. The behaviour of the attribute is undefined unless
+``N==__ARM_FEATURE_SVE_BITS``, the implementation defined feature macro that is

nit: s/behaviour/behavior/, since I think the documentation uses 
US/international English.


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

https://reviews.llvm.org/D83550



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

[PATCH] D83550: [PATCH 1/4][Sema][AArch64] Add parsing support for arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes updated this revision to Diff 278507.
c-rhodes added a comment.

Use `const auto *` and remove cast


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

https://reviews.llvm.org/D83550

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Driver/aarch64-sve-vector-bits.c
  clang/test/Preprocessor/aarch64-target-features.c
  clang/test/Sema/attr-arm-sve-vector-bits.c

Index: clang/test/Sema/attr-arm-sve-vector-bits.c
===
--- /dev/null
+++ clang/test/Sema/attr-arm-sve-vector-bits.c
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=128 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=256 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=1024 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=2048 -fallow-half-arguments-and-returns %s
+
+#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+
+typedef __SVInt8_t svint8_t;
+typedef __SVInt16_t svint16_t;
+typedef __SVInt32_t svint32_t;
+typedef __SVInt64_t svint64_t;
+typedef __SVUint8_t svuint8_t;
+typedef __SVUint16_t svuint16_t;
+typedef __SVUint32_t svuint32_t;
+typedef __SVUint64_t svuint64_t;
+typedef __SVFloat16_t svfloat16_t;
+typedef __SVFloat32_t svfloat32_t;
+typedef __SVFloat64_t svfloat64_t;
+
+#if defined(__ARM_FEATURE_SVE_BF16)
+typedef __SVBFloat16_t svbfloat16_t;
+#endif
+
+typedef __SVBool_t svbool_t;
+
+// Define valid fixed-width SVE types
+typedef svint8_t fixed_int8_t __attribute__((arm_sve_vector_bits(N)));
+typedef svint16_t fixed_int16_t __attribute__((arm_sve_vector_bits(N)));
+typedef svint32_t fixed_int32_t __attribute__((arm_sve_vector_bits(N)));
+typedef svint64_t fixed_int64_t __attribute__((arm_sve_vector_bits(N)));
+
+typedef svuint8_t fixed_uint8_t __attribute__((arm_sve_vector_bits(N)));
+typedef svuint16_t fixed_uint16_t __attribute__((arm_sve_vector_bits(N)));
+typedef svuint32_t fixed_uint32_t __attribute__((arm_sve_vector_bits(N)));
+typedef svuint64_t fixed_uint64_t __attribute__((arm_sve_vector_bits(N)));
+
+typedef svfloat16_t fixed_float16_t __attribute__((arm_sve_vector_bits(N)));
+typedef svfloat32_t fixed_float32_t __attribute__((arm_sve_vector_bits(N)));
+typedef svfloat64_t fixed_float64_t __attribute__((arm_sve_vector_bits(N)));
+
+typedef svbfloat16_t fixed_bfloat16_t __attribute__((arm_sve_vector_bits(N)));
+
+typedef svbool_t fixed_bool_t __attribute__((arm_sve_vector_bits(N)));
+
+// Attribute must have a single argument
+typedef svint8_t no_argument __attribute__((arm_sve_vector_bits)); // expected-error {{'arm_sve_vector_bits' attribute takes one argument}}
+typedef svint8_t two_arguments __attribute__((arm_sve_vector_bits(2, 4))); // expected-error {{'arm_sve_vector_bits' attribute takes one argument}}
+
+// The number of SVE vector bits must be an integer constant expression
+typedef svint8_t non_int_size1 __attribute__((arm_sve_vector_bits(2.0)));   // expected-error {{'arm_sve_vector_bits' attribute requires an integer constant}}
+typedef svint8_t non_int_size2 __attribute__((arm_sve_vector_bits("256"))); // expected-error {{'arm_sve_vector_bits' attribute requires an integer constant}}
+
+typedef __clang_svint8x2_t svint8x2_t;
+typedef __clang_svfloat32x3_t svfloat32x3_t;
+
+// Attribute must be attached to a single SVE vector or predicate type.
+typedef void *badtype1 __attribute__((arm_sve_vector_bits(N))); // expected-error {{'arm_sve_vector_bits' attribute applied to non-SVE type 'void *'}}
+typedef int badtype2 __attribute__((arm_sve_vector_bits(N)));   // expected-error {{'arm_sve_vector_bits' attribute applied to non-SVE type 'int'}}
+typedef float badtype3 __attribute__((arm_sve_vector_bits(N))); // expected-error {{'arm_sve_vector_bits' attribute applied to non-SVE type 'float'}}
+typedef svint8x2_t badtype4 __attribute__((arm_sve_vector_bits(

[PATCH] D82502: [PowerPC][Power10] Implement Load VSX Vector and Sign Extend and Zero Extend

[Stefan Pintilie via Phabricator via cfe-commits]

stefanp added a comment.

The title of the patch mentions both zero extend and sign extend.
However, it seems that we only have instructions for the zero extend case. Is 
that right?
I see both types of tests in:
`test/CodeGen/builtins-ppc-p10vector.c`
But I only see codegen tests for the zreo extend version.
`test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll`

I bring it up because I want to make sure that this is intentional.




Comment at: clang/lib/Headers/altivec.h:16516
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_xl_sext(signed long long __offset, signed char *__pointer) {

I'm a little confused about this.
Your function signature says we return `vector unsigned __int128` but the 
return statement casts to `unaligned_vec_si128`. Is that how this is supposed 
to look?



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:14163
+  if (!ValidLDType || (LD->getValueType(0) != MVT::i128) ||
+  (LD->getExtensionType() != ISD::ZEXTLOAD))
+return SDValue();

It looks like you are doing this for zero extend only. 
What about `ISD::EXTLOAD`? In that case the upper bits are undefined anyway so 
we can just assume a zero extend right?



Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll:41
+
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; These test cases tests that zero extending loads utilize the Load VSX Vector 
Rightmost

nit:
Was this line auto-added?
Usually this comment is added at the top of the file by the script. I don't 
think it is required here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82502



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

[PATCH] D82576: [PowerPC][Power10] Implement low-order Vector Modulus Builtins, and add Vector Multiply/Divide/Modulus Builtins Tests

[Baptiste Saleil via Phabricator via cfe-commits]

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

Thanks for the explanation. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82576



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

[PATCH] D83961: [Analyzer] Fix bug report source locations in minimal output.

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

balazske created this revision.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, 
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: clang.

Fix of the following problem:
If the bug report equivalence class contains multiple
reports and no (minimal) analyzer output was requested
it can happen that the wrong location is used for the warning.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83961

Files:
  clang/lib/Analysis/PathDiagnostic.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp


Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1997,9 +1997,6 @@
   return nullptr;
   }
 
-  if (!PDC->shouldGenerateDiagnostics())
-return generateEmptyDiagnosticForReport(R, getSourceManager());
-
   // Construct the final (warning) event for the bug report.
   auto EndNotes = VisitorsDiagnostics->find(ErrorNode);
   PathDiagnosticPieceRef LastPiece;
@@ -2012,6 +2009,9 @@
   }
   Construct.PD->setEndOfPath(LastPiece);
 
+  if (!PDC->shouldGenerateDiagnostics())
+return std::move(Construct.PD);
+
   PathDiagnosticLocation PrevLoc = Construct.PD->getLocation();
   // From the error node to the root, ascend the bug path and construct the bug
   // report.
@@ -3004,14 +3004,7 @@
 
 // If the path is empty, generate a single step path with the location
 // of the issue.
-if (PD->path.empty()) {
-  PathDiagnosticLocation L = report->getLocation();
-  auto piece = std::make_unique(
-L, report->getDescription());
-  for (SourceRange Range : report->getRanges())
-piece->addRange(Range);
-  PD->setEndOfPath(std::move(piece));
-}
+assert(!PD->path.empty() && "Path should contain at least the last 
piece.");
 
 PathPieces &Pieces = PD->getMutablePieces();
 if (getAnalyzerOptions().ShouldDisplayNotesAsEvents) {
Index: clang/lib/Analysis/PathDiagnostic.cpp
===
--- clang/lib/Analysis/PathDiagnostic.cpp
+++ clang/lib/Analysis/PathDiagnostic.cpp
@@ -349,6 +349,24 @@
   FullSourceLoc YL = Y.getLocation().asLocation();
   if (XL != YL)
 return compareCrossTUSourceLocs(XL, YL);
+  PathDiagnosticRange XR = X.getLocation().asRange();
+  PathDiagnosticRange YR = Y.getLocation().asRange();
+  if (XR.isValid() && !YR.isValid())
+return true;
+  if (!XR.isValid() && YR.isValid())
+return false;
+  if (XR.isValid() && YR.isValid()) {
+if (XR.isPoint && !YR.isPoint)
+  return true;
+if (!XR.isPoint && YR.isPoint)
+  return false;
+if (!XR.isPoint && !YR.isPoint) {
+  FullSourceLoc XRE{XR.getEnd(), XL.getManager()};
+  FullSourceLoc YRE{YR.getEnd(), YL.getManager()};
+  if (XRE != YRE)
+return compareCrossTUSourceLocs(XRE, YRE);
+}
+  }
   if (X.getBugType() != Y.getBugType())
 return X.getBugType() < Y.getBugType();
   if (X.getCategory() != Y.getCategory())


Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1997,9 +1997,6 @@
   return nullptr;
   }
 
-  if (!PDC->shouldGenerateDiagnostics())
-return generateEmptyDiagnosticForReport(R, getSourceManager());
-
   // Construct the final (warning) event for the bug report.
   auto EndNotes = VisitorsDiagnostics->find(ErrorNode);
   PathDiagnosticPieceRef LastPiece;
@@ -2012,6 +2009,9 @@
   }
   Construct.PD->setEndOfPath(LastPiece);
 
+  if (!PDC->shouldGenerateDiagnostics())
+return std::move(Construct.PD);
+
   PathDiagnosticLocation PrevLoc = Construct.PD->getLocation();
   // From the error node to the root, ascend the bug path and construct the bug
   // report.
@@ -3004,14 +3004,7 @@
 
 // If the path is empty, generate a single step path with the location
 // of the issue.
-if (PD->path.empty()) {
-  PathDiagnosticLocation L = report->getLocation();
-  auto piece = std::make_unique(
-L, report->getDescription());
-  for (SourceRange Range : report->getRanges())
-piece->addRange(Range);
-  PD->setEndOfPath(std::move(piece));
-}
+assert(!PD->path.empty() && "Path should contain at least the last piece.");
 
 PathPieces &Pieces = PD->getMutablePieces();
 if (getAnalyzerOptions().ShouldDisplayNotesAsEvents) {
Index: clang/lib/Analysis/PathDiagnostic.cpp
===
--- clang/lib/Analysis/PathDiagnostic.cpp
+++ clang/lib/Analysis/PathDiagnostic.cpp
@@ -349,6 +349,24 @@
   FullSourceLoc YL = Y.getLocation().asLocation();
   if (XL != YL)
 return compareCrossTUSourceLocs(

[PATCH] D83550: [PATCH 1/4][Sema][AArch64] Add parsing support for arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes marked an inline comment as done.
c-rhodes added a comment.

In D83550#2155837 , @aaron.ballman 
wrote:

> LGTM aside from a small nit.




In D83550#2156063 , @rsandifo-arm 
wrote:

> Thanks for doing this.  FWIW, the patch LGTM from a spec point of view.  I 
> just saw one minor spelling nit.


Cheers!




Comment at: clang/include/clang/Basic/AttrDocs.td:4882
+to the SVE predicate type ``svbool_t``, this excludes tuple types such as
+``svint32x4_t``. The behaviour of the attribute is undefined unless
+``N==__ARM_FEATURE_SVE_BITS``, the implementation defined feature macro that is

rsandifo-arm wrote:
> nit: s/behaviour/behavior/, since I think the documentation uses 
> US/international English.
> nit: s/behaviour/behavior/, since I think the documentation uses 
> US/international English.

Good spot, I'll fix this before merging



Comment at: clang/lib/Sema/SemaType.cpp:7698
+llvm::APSInt &Result) {
+  Expr *AttrExpr = static_cast(Attr.getArgAsExpr(0));
+  if (AttrExpr->isTypeDependent() || AttrExpr->isValueDependent() ||

aaron.ballman wrote:
> `const auto *` and you should use `cast<>` instead of `static_cast<>`.
> const auto * and you should use cast<> instead of static_cast<>.

I've added `const auto *`, I don't think the cast was necessary since 
`Attr.getArgAsExpr` returns an `Expr *` although I noticed a few places using 
`static_cast` when calling this


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

https://reviews.llvm.org/D83550



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

[PATCH] D83338: [PowerPC][Power10] Implemented Vector Shift Builtins

[Stefan Pintilie via Phabricator via cfe-commits]

stefanp added a comment.

Just a few nits for this patch.




Comment at: clang/lib/Headers/altivec.h:17158
+static __inline__ vector signed __int128 __ATTRS_o_ai
+vec_sl(vector signed __int128 __a, vector unsigned __int128 __b) {
+  return __builtin_altivec_vslq((vector unsigned __int128) __a, __b);

nit:
Is this supposed to be `vec_slq`?



Comment at: llvm/test/CodeGen/PowerPC/p10-vector-shift.ll:10
+
+define dso_local <1 x i128> @test_vec_slq(<1 x i128> %a, <1 x i128> %b) #0 {
+; CHECK-LABEL: test_vec_slq:

nit:
If you are going to use `#0` you can probably define `attributes #0 = { 
nounwind }` at the bottom of this file.



Comment at: llvm/test/CodeGen/PowerPC/p10-vector-shift.ll:41
+; Function Attrs: nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vslq(<1 x i128>, <1 x i128>) #1
+

nit: 
You probably don't need the #1 as it is not defined anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83338



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

[PATCH] D83120: [Analyzer][StreamChecker] Use BugType::SuppressOnSink at resource leak report.

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

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



Comment at: clang/test/Analysis/stream.c:274-284
 // Check that "location uniqueing" works.
 // This results in reporting only one occurence of resource leak for a stream.
 void check_leak_noreturn_2() {
   FILE *F1 = tmpfile();
   if (!F1)
 return;
   if (Test == 1) {

Szelethus wrote:
> balazske wrote:
> > Szelethus wrote:
> > > balazske wrote:
> > > > NoQ wrote:
> > > > > balazske wrote:
> > > > > > NoQ wrote:
> > > > > > > balazske wrote:
> > > > > > > > Szelethus wrote:
> > > > > > > > > Szelethus wrote:
> > > > > > > > > > balazske wrote:
> > > > > > > > > > > NoQ wrote:
> > > > > > > > > > > > balazske wrote:
> > > > > > > > > > > > > Szelethus wrote:
> > > > > > > > > > > > > > Why did this change? Is there a sink in the return 
> > > > > > > > > > > > > > branch?
> > > > > > > > > > > > > The change is probably because D83115. Because the 
> > > > > > > > > > > > > "uniqueing" one resource leak is reported from the 
> > > > > > > > > > > > > two possible, and the order changes somehow (probably 
> > > > > > > > > > > > > not the shortest is found first).
> > > > > > > > > > > > The shortest should still be found first. I strongly 
> > > > > > > > > > > > suggest debugging this. Looks like a bug in 
> > > > > > > > > > > > suppress-on-sink.
> > > > > > > > > > > There is no code that ensures that the shortest path is 
> > > > > > > > > > > reported. In this case one equivalence class is created 
> > > > > > > > > > > with both bug reports. If `SuppressOnSink` is false the 
> > > > > > > > > > > last one is returned from the list, otherwise the first 
> > > > > > > > > > > one 
> > > > > > > > > > > (`PathSensitiveBugReporter::findReportInEquivalenceClass`),
> > > > > > > > > > >  this causes the difference (seems to be unrelated to 
> > > > > > > > > > > D83115).
> > > > > > > > > > > There is no code that ensures that the shortest path is 
> > > > > > > > > > > reported.
> > > > > > > > > > 
> > > > > > > > > > There absolutely should be -- See the summary of D65379 for 
> > > > > > > > > > more info, CTRL+F "shortest" helps quite a bit as well. For 
> > > > > > > > > > each bug report, we create a bug path (a path in the 
> > > > > > > > > > exploded graph from the root to the sepcific bug reports 
> > > > > > > > > > error node), and sort them by length.
> > > > > > > > > > 
> > > > > > > > > > It all feels super awkward -- 
> > > > > > > > > > `PathSensitiveBugReporter::findReportInEquivalenceClass` 
> > > > > > > > > > picks out a bug report from an equivalence class as you 
> > > > > > > > > > described, but that will only be reported if it is a 
> > > > > > > > > > `BasicBugReport` (as implemented by 
> > > > > > > > > > `PathSensitiveBugReporter::generateDiagnosticForConsumerMap`),
> > > > > > > > > >  otherwise it should go through the graph cutting process 
> > > > > > > > > > etc.
> > > > > > > > > > 
> > > > > > > > > > So at the end of the day, the shortest path should appear 
> > > > > > > > > > still? 
> > > > > > > > > > 
> > > > > > > > > @balazske I spent a lot of my GSoC rewriting some especially 
> > > > > > > > > miserable code in `BugReporter.cpp`, please hunt me down if 
> > > > > > > > > you need any help there.
> > > > > > > > Can we say that the one path in this case is shorter than the 
> > > > > > > > other? The difference is only at the "taking true/false branch" 
> > > > > > > > at the `if` in line 280. Maybe both have equal length. The 
> > > > > > > > notes are taken always from the single picked report that is 
> > > > > > > > returned from `findReportInEquivalenceClass` and these notes 
> > > > > > > > can contain different source locations (reports in a single 
> > > > > > > > equivalence class can have different locations, really this 
> > > > > > > > makes the difference between them?).  
> > > > > > > > There is no code that ensures that the shortest path is 
> > > > > > > > reported.
> > > > > > > 
> > > > > > > We would have been soo screwed if this was so. In 
> > > > > > > fact, grepping for "shortest" in the entire clang sources 
> > > > > > > immediately points you to the right line of code.
> > > > > > > 
> > > > > > > > the last one is returned from the list, otherwise the first one
> > > > > > > 
> > > > > > > The example report is not actually used later for purposes other 
> > > > > > > than extracting information common to all reports in the path. 
> > > > > > > The array of valid reports is used instead, and it's supposed to 
> > > > > > > be sorted.
> > > > > > > 
> > > > > > > > Can we say that the one path in this case is shorter than the 
> > > > > > > > other?
> > > > > > > 
> > > > > > > Dump the graph and see for yourself. I expect a call with an 
> > > > > > > argument and an implicit lvalue-to-rvalue conversion of that 
> > > > > > > argument to take a lot more nodes than an empty return statement.
> > > > > > I fou

[PATCH] D83500: [PowerPC][Power10] Implement custom codegen for the vec_replace_elt and vec_replace_unaligned builtins.

[Kamau Bridgeman via Phabricator via cfe-commits]

kamaub accepted this revision as: kamaub.
kamaub added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83500



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

[PATCH] D83961: [Analyzer] Fix bug report source locations in minimal output.

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

balazske added a comment.

Lot of tests are failing because changed warning locations.
These will be updated if the code change looks good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83961



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

[clang] 4f244c4 - Use TestClangConfig in AST Matchers tests and run them in more configurations

[Dmitri Gribenko via cfe-commits]

Author: Dmitri Gribenko
Date: 2020-07-16T18:36:53+02:00
New Revision: 4f244c4b42b096a55f2e7f719e1101c6fd26c034

URL: 
https://github.com/llvm/llvm-project/commit/4f244c4b42b096a55f2e7f719e1101c6fd26c034
DIFF: 
https://github.com/llvm/llvm-project/commit/4f244c4b42b096a55f2e7f719e1101c6fd26c034.diff

LOG: Use TestClangConfig in AST Matchers tests and run them in more 
configurations

Summary:
I am changing tests for AST Matchers to run in multiple language standards
versions, and under multiple triples that have different behavior with regards
to templates. This change is similar to https://reviews.llvm.org/D82179.

To keep the size of the patch manageable, in this patch I'm only migrating one
file to get the process started and get feedback on this approach.

Reviewers: ymandel

Reviewed By: ymandel

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/include/clang/Testing/TestClangConfig.h
clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
clang/unittests/ASTMatchers/ASTMatchersTest.h

Removed: 




diff  --git a/clang/include/clang/Testing/TestClangConfig.h 
b/clang/include/clang/Testing/TestClangConfig.h
index eefa36dc2ebb..5d6be4f65d0a 100644
--- a/clang/include/clang/Testing/TestClangConfig.h
+++ b/clang/include/clang/Testing/TestClangConfig.h
@@ -51,6 +51,8 @@ struct TestClangConfig {
 return Language == Lang_CXX17 || Language == Lang_CXX20;
   }
 
+  bool isCXX20OrLater() const { return Language == Lang_CXX20; }
+
   bool supportsCXXDynamicExceptionSpecification() const {
 return Language == Lang_CXX03 || Language == Lang_CXX11 ||
Language == Lang_CXX14;

diff  --git a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp 
b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
index c249410201ba..36e92c632c03 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -18,7 +18,7 @@
 namespace clang {
 namespace ast_matchers {
 
-TEST(IsExpandedFromMacro, ShouldMatchInFile) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesInFile) {
   StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 void Test() { MY_MACRO(4); }
@@ -26,7 +26,7 @@ TEST(IsExpandedFromMacro, ShouldMatchInFile) {
   EXPECT_TRUE(matches(input, binaryOperator(isExpandedFromMacro("MY_MACRO";
 }
 
-TEST(IsExpandedFromMacro, ShouldMatchNested) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesNested) {
   StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 #define WRAPPER(a) MY_MACRO(a)
@@ -35,7 +35,7 @@ TEST(IsExpandedFromMacro, ShouldMatchNested) {
   EXPECT_TRUE(matches(input, binaryOperator(isExpandedFromMacro("MY_MACRO";
 }
 
-TEST(IsExpandedFromMacro, ShouldMatchIntermediate) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesIntermediate) {
   StringRef input = R"cc(
 #define IMPL(a) (4 + (a))
 #define MY_MACRO(a) IMPL(a)
@@ -45,7 +45,7 @@ TEST(IsExpandedFromMacro, ShouldMatchIntermediate) {
   EXPECT_TRUE(matches(input, binaryOperator(isExpandedFromMacro("MY_MACRO";
 }
 
-TEST(IsExpandedFromMacro, ShouldMatchTransitive) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesTransitive) {
   StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 #define WRAPPER(a) MY_MACRO(a)
@@ -54,7 +54,7 @@ TEST(IsExpandedFromMacro, ShouldMatchTransitive) {
   EXPECT_TRUE(matches(input, binaryOperator(isExpandedFromMacro("WRAPPER";
 }
 
-TEST(IsExpandedFromMacro, ShouldMatchArgument) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesArgument) {
   StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 void Test() {
@@ -65,9 +65,9 @@ TEST(IsExpandedFromMacro, ShouldMatchArgument) {
   EXPECT_TRUE(matches(input, declRefExpr(isExpandedFromMacro("MY_MACRO";
 }
 
-// Like IsExpandedFromMacroShouldMatchArgumentMacro, but the argument is itself
-// a macro.
-TEST(IsExpandedFromMacro, ShouldMatchArgumentMacroExpansion) {
+// Like IsExpandedFromMacro_MatchesArgument, but the argument is itself a
+// macro.
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesArgumentMacroExpansion) {
   StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 #define IDENTITY(a) (a)
@@ -78,7 +78,7 @@ TEST(IsExpandedFromMacro, ShouldMatchArgumentMacroExpansion) {
   EXPECT_TRUE(matches(input, binaryOperator(isExpandedFromMacro("IDENTITY";
 }
 
-TEST(IsExpandedFromMacro, ShouldMatchWhenInArgument) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesWhenInArgument) {
   StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 #define IDENTITY(a) (a)
@@ -89,7 +89,7 @@ TEST(IsExpandedFromMacro, ShouldMatchWhenInArgument) {
   EXPECT_TRUE(matches(input, binaryOperator(isExpandedFromMacro("MY_MACRO";
 }
 
-TEST(IsExpandedFromMacro, ShouldMatchObjectMacro) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesObjectMacro) {
   StringRef input = 

[PATCH] D83868: Use TestClangConfig in AST Matchers tests and run them in more configurations

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4f244c4b42b0: Use TestClangConfig in AST Matchers tests and 
run them in more configurations (authored by gribozavr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83868

Files:
  clang/include/clang/Testing/TestClangConfig.h
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTest.h

Index: clang/unittests/ASTMatchers/ASTMatchersTest.h
===
--- clang/unittests/ASTMatchers/ASTMatchersTest.h
+++ clang/unittests/ASTMatchers/ASTMatchersTest.h
@@ -183,11 +183,6 @@
   "input.c");
 }
 
-template 
-testing::AssertionResult notMatchesC(const Twine &Code, const T &AMatcher) {
-  return matchesConditionally(Code, AMatcher, false, {Lang_C89});
-}
-
 template 
 testing::AssertionResult notMatchesObjC(const Twine &Code, const T &AMatcher) {
   return matchesObjC(Code, AMatcher, false);
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -18,7 +18,7 @@
 namespace clang {
 namespace ast_matchers {
 
-TEST(IsExpandedFromMacro, ShouldMatchInFile) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesInFile) {
   StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 void Test() { MY_MACRO(4); }
@@ -26,7 +26,7 @@
   EXPECT_TRUE(matches(input, binaryOperator(isExpandedFromMacro("MY_MACRO";
 }
 
-TEST(IsExpandedFromMacro, ShouldMatchNested) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesNested) {
   StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 #define WRAPPER(a) MY_MACRO(a)
@@ -35,7 +35,7 @@
   EXPECT_TRUE(matches(input, binaryOperator(isExpandedFromMacro("MY_MACRO";
 }
 
-TEST(IsExpandedFromMacro, ShouldMatchIntermediate) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesIntermediate) {
   StringRef input = R"cc(
 #define IMPL(a) (4 + (a))
 #define MY_MACRO(a) IMPL(a)
@@ -45,7 +45,7 @@
   EXPECT_TRUE(matches(input, binaryOperator(isExpandedFromMacro("MY_MACRO";
 }
 
-TEST(IsExpandedFromMacro, ShouldMatchTransitive) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesTransitive) {
   StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 #define WRAPPER(a) MY_MACRO(a)
@@ -54,7 +54,7 @@
   EXPECT_TRUE(matches(input, binaryOperator(isExpandedFromMacro("WRAPPER";
 }
 
-TEST(IsExpandedFromMacro, ShouldMatchArgument) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesArgument) {
   StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 void Test() {
@@ -65,9 +65,9 @@
   EXPECT_TRUE(matches(input, declRefExpr(isExpandedFromMacro("MY_MACRO";
 }
 
-// Like IsExpandedFromMacroShouldMatchArgumentMacro, but the argument is itself
-// a macro.
-TEST(IsExpandedFromMacro, ShouldMatchArgumentMacroExpansion) {
+// Like IsExpandedFromMacro_MatchesArgument, but the argument is itself a
+// macro.
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesArgumentMacroExpansion) {
   StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 #define IDENTITY(a) (a)
@@ -78,7 +78,7 @@
   EXPECT_TRUE(matches(input, binaryOperator(isExpandedFromMacro("IDENTITY";
 }
 
-TEST(IsExpandedFromMacro, ShouldMatchWhenInArgument) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesWhenInArgument) {
   StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 #define IDENTITY(a) (a)
@@ -89,7 +89,7 @@
   EXPECT_TRUE(matches(input, binaryOperator(isExpandedFromMacro("MY_MACRO";
 }
 
-TEST(IsExpandedFromMacro, ShouldMatchObjectMacro) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_MatchesObjectMacro) {
   StringRef input = R"cc(
 #define PLUS (2 + 2)
 void Test() {
@@ -99,7 +99,7 @@
   EXPECT_TRUE(matches(input, binaryOperator(isExpandedFromMacro("PLUS";
 }
 
-TEST(IsExpandedFromMacro, ShouldMatchFromCommandLine) {
+TEST(IsExpandedFromMacro, MatchesFromCommandLine) {
   StringRef input = R"cc(
 void Test() { FOUR_PLUS_FOUR; }
   )cc";
@@ -108,7 +108,7 @@
   {"-std=c++11", "-DFOUR_PLUS_FOUR=4+4"}));
 }
 
-TEST(IsExpandedFromMacro, ShouldNotMatchBeginOnly) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_NotMatchesBeginOnly) {
   StringRef input = R"cc(
 #define ONE_PLUS 1+
   void Test() { ONE_PLUS 4; }
@@ -117,7 +117,7 @@
   notMatches(input, binaryOperator(isExpandedFromMacro("ONE_PLUS";
 }
 
-TEST(IsExpandedFromMacro, ShouldNotMatchEndOnly) {
+TEST_P(ASTMatchersTest, IsExpandedFromMacro_NotMatchesEndOnly) {
   StringRef input = R"cc(
 #define PLUS_ONE +1
   void Test() { 4 PLUS_ONE; }
@@ -126,7 +126,7 @@
   notMatches(input, binaryOperator(isExpandedFromMacro("PLUS_ONE";
 }
 
-TEST(IsExpandedFromMacro, ShouldNotMatchDifferentMacro) {
+TEST_P(ASTMatchersTest, IsExp

[PATCH] D83494: [libFuzzer] Link libFuzzer's own interceptors when other compiler runtimes are not linked.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp:53
+int fuzzer_inited = 0;
+bool fuzzer_init_is_running;
+

These are in the global namespace, and have C mangling, which is unnecessary.  
Please either put them in a namespace or make them static.



Comment at: compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp:55
+
+static void __fuzzer_init();
+

This also doesn't need C mangling.



Comment at: compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp:63
+}  
\
+  } while (0)
+

Let's prefer a function rather than a macro for this.



Comment at: compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp:143
+return;
+  fuzzer_init_is_running = 1;
+

Let's use true/false rather than 1/0 for bools.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83494



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

[PATCH] D80802: [RISCV] Upgrade RVV MC to v0.9.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai added a comment.

In D80802#2155432 , @simoncook wrote:

> Since this patch replaces 0.8 support with 0.9, it should include an update 
> to the version check in `clang/lib/Driver/ToolChains/Arch/RISCV.cpp` to match.


The modification is put in D81213 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80802



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

[PATCH] D83494: [libFuzzer] Link libFuzzer's own interceptors when other compiler runtimes are not linked.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp:17
+
+#define GET_CALLER_PC() __builtin_return_address(0)
+

Nit: Let's move this down with the other defines.



Comment at: compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp:30
+
+#include  // for dlsym()
+

Nit:  Can we move the other includes down by this one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83494



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

[PATCH] D83966: Enable the test for hasArraySize() in all language modes

[Dmitri Gribenko via Phabricator via cfe-commits]

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

In C++11 and later Clang generates an implicit conversion from int to
size_t in the AST.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83966

Files:
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -3219,13 +3219,13 @@
 }
 
 TEST_P(ASTMatchersTest, HasArraySize) {
-  if (GetParam().Language != Lang_CXX03) {
-// FIXME: Fix this test to work in all C++ language modes.
+  if (!GetParam().isCXX()) {
 return;
   }
 
   EXPECT_TRUE(matches("struct MyClass {}; MyClass *p1 = new MyClass[10];",
-  cxxNewExpr(hasArraySize(integerLiteral(equals(10));
+  cxxNewExpr(hasArraySize(
+  
ignoringParenImpCasts(integerLiteral(equals(10)));
 }
 
 TEST_P(ASTMatchersTest, HasDefinition_MatchesStructDefinition) {


Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -3219,13 +3219,13 @@
 }
 
 TEST_P(ASTMatchersTest, HasArraySize) {
-  if (GetParam().Language != Lang_CXX03) {
-// FIXME: Fix this test to work in all C++ language modes.
+  if (!GetParam().isCXX()) {
 return;
   }
 
   EXPECT_TRUE(matches("struct MyClass {}; MyClass *p1 = new MyClass[10];",
-  cxxNewExpr(hasArraySize(integerLiteral(equals(10));
+  cxxNewExpr(hasArraySize(
+  ignoringParenImpCasts(integerLiteral(equals(10)));
 }
 
 TEST_P(ASTMatchersTest, HasDefinition_MatchesStructDefinition) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83961: [Analyzer] Fix bug report source locations in minimal output.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> These will be updated if the code change looks good.

Hard to tell; this entire code is too convoluted, i'd rather look at the 
changes. Can you update at least some tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83961



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

[PATCH] D83942: [analyzer][tests] Add a notion of project sizes

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> Sizes assigned to the projects in this commit, do not directly
>  correspond to the number of lines or files in the project.

Maybe `QUICK`/`NORMAL`/`SLOW` then? Or by purpose: 
`BENCHMARK`/`DAILY`/`PARANOID`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83942



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

[PATCH] D83099: [clangd] Store index in '.cache/clangd/index' instead of '.clangd/index'

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D83099#2155900 , @davidvancleve 
wrote:

> Yes, these two are exactly the case!
>
> > - you'd be using an editor/plugin that sends compile commands over LSP 
> > (such as YCM + ycm_extra_conf). What are you using?
> > - there should be relatively few *.idx files inside the extra directories 
> > (the ones not near your compilation database), corresponding to files 
> > you've had open rather than the whole project


Well, that's a relief :-)
46c921003c2ce5f1cdc4de9ef613eb001980780c 
 should 
fix this then, and we'll get it cherrypicked to the release branch.
Please let us know if you see this again after that commit!

> I'm using YCM. We only had /.clangd/ in our gitignore, which AIUI would have 
> only been ignoring a clangd at the level of the project root; I certainly 
> never noticed any .clangd folders in subdirectories.

Yeah, the leading slash shoud do that... I'm stumped then, but I'm also fairly 
confident this is fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

[PATCH] D83099: [clangd] Store index in '.cache/clangd/index' instead of '.clangd/index'

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

https://bugs.llvm.org/show_bug.cgi?id=46754 to get the fixed merged


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

[PATCH] D83099: [clangd] Store index in '.cache/clangd/index' instead of '.clangd/index'

[David Van Cleve via Phabricator via cfe-commits]

davidvancleve added a comment.

Super! Once we pull in that version (unsure of the latency; this is my first 
time reporting an issue with clangd), I'll be sure to come back and confirm 
that the fix is working for me. Thank you again for the quick turnaround!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

[PATCH] D83915: [PowerPC] Remove QPX/A2Q BGQ/BGP support

[Jinsong Ji via Phabricator via cfe-commits]

jsji added a comment.

In D83915#2155650 , 
@hubert.reinterpretcast wrote:

> Should we indicate planned removal in the Release Notes for version 11 and 
> actual removal in those for version 12?


Good suggestion. https://reviews.llvm.org/D83968 for adding the section in 
master, will add conent in v11 branch if approved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83915



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