[PATCH] D78374: [Analyzer][StreamChecker] Added evaluation of fread and fwrite.

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

baloghadamsoftware added a comment.

> Is there a significant gain behind doing that instead of doing a state split 
> immediately? It feels unnatural to delay the split. It doesn't accurately 
> represent how the code would run. Are we supposed to do this at every 
> precall? What about other checkers that may rely on this one?

The actual gain is that we somewhat reduce the exponential growth of the 
exploded graph. With such a Schrödinger's cat pattern we delay it to the point 
where it is actually checked, it checked at all.




Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:39
+  /// There is an execution path with none of the error flags set.
+  bool NoError = true;
+  /// There is an execution path with EOF indicator set.

`bool NoError:1 = true` etc. Why not use bit fields for a struct of boolans?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78374



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

[clang] f7c9f77 - [Analyzer][StreamChecker] Added support for 'fread' and 'fwrite'.

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

Author: Balázs Kéri
Date: 2020-05-20T09:40:57+02:00
New Revision: f7c9f77ef3721c956499b0ccf5e585de105aae4e

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

LOG: [Analyzer][StreamChecker] Added support for 'fread' and 'fwrite'.

Summary:
Stream functions `fread` and `fwrite` are evaluated
and preconditions checked.
A new bug type is added for a (non fatal) warning if `fread`
is called in EOF state.

Reviewers: Szelethus, NoQ, dcoughlin, baloghadamsoftware, martong, xazax.hun

Reviewed By: Szelethus

Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, 
mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, 
ASDenysPetrov, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/test/Analysis/stream-error.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index c94cae045239..2e90be4350a0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -51,6 +51,8 @@ struct StreamErrorState {
 return NoError == ES.NoError && FEof == ES.FEof && FError == ES.FError;
   }
 
+  bool operator!=(const StreamErrorState &ES) const { return !(*this == ES); }
+
   StreamErrorState operator|(const StreamErrorState &E) const {
 return {NoError || E.NoError, FEof || E.FEof, FError || E.FError};
   }
@@ -171,7 +173,7 @@ ProgramStateRef bindInt(uint64_t Value, ProgramStateRef 
State,
 class StreamChecker
 : public Checker {
   mutable std::unique_ptr BT_nullfp, BT_illegalwhence,
-  BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak;
+  BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak, BT_StreamEof;
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
@@ -189,8 +191,12 @@ class StreamChecker
   {{"tmpfile"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
   {{"fclose", 1},
{&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}},
-  {{"fread", 4}, {&StreamChecker::preDefault, nullptr, 3}},
-  {{"fwrite", 4}, {&StreamChecker::preDefault, nullptr, 3}},
+  {{"fread", 4},
+   {&StreamChecker::preFread,
+std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, true), 3}},
+  {{"fwrite", 4},
+   {&StreamChecker::preFwrite,
+std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, false), 3}},
   {{"fseek", 3}, {&StreamChecker::preFseek, &StreamChecker::evalFseek, 0}},
   {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0}},
   {{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0}},
@@ -232,6 +238,15 @@ class StreamChecker
   void evalFclose(const FnDescription *Desc, const CallEvent &Call,
   CheckerContext &C) const;
 
+  void preFread(const FnDescription *Desc, const CallEvent &Call,
+CheckerContext &C) const;
+
+  void preFwrite(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const;
+
+  void evalFreadFwrite(const FnDescription *Desc, const CallEvent &Call,
+   CheckerContext &C, bool IsFread) const;
+
   void preFseek(const FnDescription *Desc, const CallEvent &Call,
 CheckerContext &C) const;
   void evalFseek(const FnDescription *Desc, const CallEvent &Call,
@@ -271,6 +286,12 @@ class StreamChecker
   ProgramStateRef ensureFseekWhenceCorrect(SVal WhenceVal, CheckerContext &C,
ProgramStateRef State) const;
 
+  /// Generate warning about stream in EOF state.
+  /// There will be always a state transition into the passed State,
+  /// by the new non-fatal error node or (if failed) a normal transition,
+  /// to ensure uniform handling.
+  void reportFEofWarning(CheckerContext &C, ProgramStateRef State) const;
+
   /// Find the description data of the function called by a call event.
   /// Returns nullptr if no function is recognized.
   const FnDescription *lookupFn(const CallEvent &Call) const {
@@ -418,6 +439,120 @@ void StreamChecker::evalFclose(const FnDescription *Desc, 
const CallEvent &Call,
   C.addTransition(State);
 }
 
+void StreamChecker::preFread(const FnDescription *Desc, const CallEvent &Call,
+ CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  SVal StreamVal = getStreamArg(Desc, Call);
+  State = ensureStreamNonNull(StreamVal, C, State);
+  if (!State)
+return;
+  State = ensureStreamOpened(StreamVal, C, State);
+  if (!State)
+return;
+
+  SymbolRef Sym = StreamVal.getAsSymbol();
+  if (Sym && State->get(Sym)) {
+const StreamState *SS

[PATCH] D78933: [analyzer] RangeConstraintManager optimizations in comparison expressions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

I've made some performance measurements on https://github.com/vim/vim project.
Since I'm using Windows I had no chance to do a lot of test on different 
projects, because of Unix-environment dependency.
I've tested the most weighty files running `clang --analyze -Xclang "-Iproto" 
-Xclang -analyzer-stats src/file.c` on each.
In this way I've tested 84 files. And here is a result table (Each row contains 
a result of testing an individual c/cpp-file).F11958819: 
performance_test_vim.xlsx 
Total results are:
test 1

| Before | After  | Delta |
| 770,9s | 760,8s | 1,31% |
|

test 2

| Before | After  | Delta |
| 791,4s | 771,8s | 2,48% |
|




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

https://reviews.llvm.org/D78933



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

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

[Marcus Johnson via Phabricator via cfe-commits]

MarcusJohnson91 added a comment.

In D75791#2044492 , @MyDeveloperDay 
wrote:

> If you want me to land this for you, I'd feel more comfortable landing it if:
>
> a) We can land D80214: [clang-format] Set of unit test to begin to validate 
> that we don't change defaults  first
>  b) The Mozilla team have tested the impact (they clang-format their entire 
> code base I think)


I'm ok with accepting commit access, and I agree lets get D80214: 
[clang-format] Set of unit test to begin to validate that we don't change 
defaults  in, and see if Mozilla, Microsoft, 
Google, etc  has any comments; I'm just not sure of who to ping.

Is there anything else that D80214: [clang-format] Set of unit test to begin to 
validate that we don't change defaults  needs? 
it looked pretty well fleshed out.


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

https://reviews.llvm.org/D75791



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

[PATCH] D80171: [analyzer] LoopUnrolling: fix crash when a parameter is a loop counter

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:167
 
+  const bool isParm = VD->getKind() == Decl::ParmVar;
+  // Reference parameters are assumed as escaped variables.

`getKind` function is only an implementation detail for `isa`/`cast`/`dan_cast` 
functions ([[ 
https://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates
 | docs ]]).  So, this condition would be better in the following form: 
`isa(VD)`.

NOTE: One good motivation here is that //maybe// in the future there will be 
some sort of new type of function parameters and it will be derived from 
`ParmVarDecl`.  In this situation, direct comparison with the kind of node will 
not work and probably won't be fixed by developers who introduced the new node, 
but `isa` approach will stay correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80171



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

[PATCH] D69987: [RISCV] Assemble/Disassemble v-ext instructions.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

@HsiangKai: just to confirm and to avoid confusion for other reviewers.

> Assemble/disassemble RISC-V V extension instructions according to version 
> 0.8-draft-20191004 in https://github.com/riscv/riscv-v-spec/.

Is the patch against the spec published in 
https://github.com/riscv/riscv-v-spec/releases/tag/0.8 (which looks to me from 
20191214)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69987



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

[PATCH] D80015: [Analyzer][StreamChecker] Added support for 'fread' and 'fwrite'.

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf7c9f77ef372: [Analyzer][StreamChecker] Added support for 
'fread' and 'fwrite'. (authored by balazske).

Changed prior to commit:
  https://reviews.llvm.org/D80015?vs=264843&id=265159#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80015

Files:
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/stream-error.c

Index: clang/test/Analysis/stream-error.c
===
--- clang/test/Analysis/stream-error.c
+++ clang/test/Analysis/stream-error.c
@@ -7,6 +7,7 @@
 #include "Inputs/system-header-simulator.h"
 
 void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached();
 void StreamTesterChecker_make_feof_stream(FILE *);
 void StreamTesterChecker_make_ferror_stream(FILE *);
 
@@ -57,6 +58,84 @@
   fclose(F);
 }
 
+void error_fread() {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  char Buf[10];
+  int Ret = fread(Buf, 1, 10, F);
+  if (Ret == 10) {
+clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+  } else {
+clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{TRUE}}
+if (feof(F)) {
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+  fread(Buf, 1, 10, F);   // expected-warning {{Read function called when stream is in EOF state}}
+  clang_analyzer_eval(feof(F));   // expected-warning {{TRUE}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+}
+if (ferror(F)) {
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+  fread(Buf, 1, 10, F);   // no warning
+}
+  }
+  fclose(F);
+  Ret = fread(Buf, 1, 10, F); // expected-warning {{Stream might be already closed}}
+}
+
+void error_fwrite() {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  const char *Buf = "123456789";
+  int Ret = fwrite(Buf, 1, 10, F);
+  if (Ret == 10) {
+clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}
+  } else {
+clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+fwrite(0, 1, 10, F);// no warning
+  }
+  fclose(F);
+  Ret = fwrite(0, 1, 10, F); // expected-warning {{Stream might be already closed}}
+}
+
+void freadwrite_zerosize(FILE *F) {
+  fwrite(0, 1, 0, F);
+  fwrite(0, 0, 1, F);
+  fread(0, 1, 0, F);
+  fread(0, 0, 1, F);
+}
+
+void freadwrite_zerosize_eofstate(FILE *F) {
+  fwrite(0, 1, 0, F);
+  fwrite(0, 0, 1, F);
+  fread(0, 1, 0, F); // expected-warning {{Read function called when stream is in EOF state}}
+  fread(0, 0, 1, F); // expected-warning {{Read function called when stream is in EOF state}}
+}
+
+void error_fread_fwrite_zerosize() {
+  FILE *F = fopen("file", "r");
+  if (!F)
+return;
+
+  freadwrite_zerosize(F);
+  clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+
+  StreamTesterChecker_make_ferror_stream(F);
+  freadwrite_zerosize(F);
+  clang_analyzer_eval(feof(F));   // expected-warning {{FALSE}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{TRUE}}
+
+  StreamTesterChecker_make_feof_stream(F);
+  freadwrite_zerosize_eofstate(F);
+  clang_analyzer_eval(feof(F));   // expected-warning {{TRUE}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+
+  fclose(F);
+}
+
 void error_fseek() {
   FILE *F = fopen("file", "r");
   if (!F)
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -51,6 +51,8 @@
 return NoError == ES.NoError && FEof == ES.FEof && FError == ES.FError;
   }
 
+  bool operator!=(const StreamErrorState &ES) const { return !(*this == ES); }
+
   StreamErrorState operator|(const StreamErrorState &E) const {
 return {NoError || E.NoError, FEof || E.FEof, FError || E.FError};
   }
@@ -171,7 +173,7 @@
 class StreamChecker
 : public Checker {
   mutable std::unique_ptr BT_nullfp, BT_illegalwhence,
-  BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak;
+  BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak, BT_StreamEof;
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
@@ -189,8 +191,12 @@
   {{"tmpfile"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
   {{"fclose", 1},
{&StreamChecker::preDefault, &StreamChecker::evalFclose, 0}},
-  {{"fread", 4}, {&StreamChecker::preDefault, nullptr, 3}},
-  {{"fwrite", 4}, {&StreamChecker::preDefault, nullptr, 3}},
+  {{"fread", 4},
+   {&StreamChecker::preFread,
+std::bind(&StreamChecker::evalFreadFwrite, _1, _2, _3, _4, true), 3}},
+  {

[PATCH] D80279: [libclang] Extend clang_Cursor_Evaluate().

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler created this revision.
ckandeler added a reviewer: nik.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.

Let this function (try to) evaluate expressions, in addition to
declarations and compound statements.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80279

Files:
  clang/include/clang-c/Index.h
  clang/test/Index/evaluate-cursor.cpp
  clang/tools/libclang/CIndex.cpp


Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -4041,10 +4041,14 @@
 }
 
 CXEvalResult clang_Cursor_Evaluate(CXCursor C) {
-  if (const Expr *E =
-  clang_getCursorKind(C) == CXCursor_CompoundStmt
-  ? evaluateCompoundStmtExpr(cast(getCursorStmt(C)))
-  : evaluateDeclExpr(getCursorDecl(C)))
+  const Expr *E = nullptr;
+  if (clang_getCursorKind(C) == CXCursor_CompoundStmt)
+E = evaluateCompoundStmtExpr(cast(getCursorStmt(C)));
+  else if (clang_isDeclaration(C.kind))
+E = evaluateDeclExpr(getCursorDecl(C));
+  else if (clang_isExpression(C.kind))
+E = getCursorExpr(C);
+  if (E)
 return const_cast(
 reinterpret_cast(evaluateExpr(const_cast(E), 
C)));
   return nullptr;
Index: clang/test/Index/evaluate-cursor.cpp
===
--- clang/test/Index/evaluate-cursor.cpp
+++ clang/test/Index/evaluate-cursor.cpp
@@ -26,6 +26,9 @@
   static const auto g = alignof(f);
 };
 
+constexpr static int calc_val() { return 1 + 2; }
+const auto the_value = calc_val() + sizeof(char);
+
 // RUN: c-index-test -evaluate-cursor-at=%s:4:7 \
 // RUN:-evaluate-cursor-at=%s:8:7 \
 // RUN:-evaluate-cursor-at=%s:8:11 -std=c++11 %s | FileCheck %s
@@ -53,3 +56,12 @@
 // RUN:-evaluate-cursor-at=%s:26:21 \
 // RUN:-std=c++11 %s | FileCheck -check-prefix=CHECK-DOES-NOT-CRASH %s
 // CHECK-DOES-NOT-CRASH: Not Evaluatable
+
+// RUN: c-index-test -evaluate-cursor-at=%s:30:1 \
+// RUN:-evaluate-cursor-at=%s:30:32 \
+// RUN:-evaluate-cursor-at=%s:30:35 \
+// RUN:-evaluate-cursor-at=%s:30:37 -std=c++11 %s | FileCheck %s 
-check-prefix=CHECK-EXPR
+// CHECK-EXPR: unsigned, Value: 4
+// CHECK-EXPR: Value: 3
+// CHECK-EXPR: unsigned, Value: 4
+// CHECK-EXPR: unsigned, Value: 1
Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -5933,6 +5933,7 @@
  * If cursor is a statement declaration tries to evaluate the
  * statement and if its variable, tries to evaluate its initializer,
  * into its corresponding type.
+ * If it's an expression, tries to evaluate the expression.
  */
 CINDEX_LINKAGE CXEvalResult clang_Cursor_Evaluate(CXCursor C);
 


Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -4041,10 +4041,14 @@
 }
 
 CXEvalResult clang_Cursor_Evaluate(CXCursor C) {
-  if (const Expr *E =
-  clang_getCursorKind(C) == CXCursor_CompoundStmt
-  ? evaluateCompoundStmtExpr(cast(getCursorStmt(C)))
-  : evaluateDeclExpr(getCursorDecl(C)))
+  const Expr *E = nullptr;
+  if (clang_getCursorKind(C) == CXCursor_CompoundStmt)
+E = evaluateCompoundStmtExpr(cast(getCursorStmt(C)));
+  else if (clang_isDeclaration(C.kind))
+E = evaluateDeclExpr(getCursorDecl(C));
+  else if (clang_isExpression(C.kind))
+E = getCursorExpr(C);
+  if (E)
 return const_cast(
 reinterpret_cast(evaluateExpr(const_cast(E), C)));
   return nullptr;
Index: clang/test/Index/evaluate-cursor.cpp
===
--- clang/test/Index/evaluate-cursor.cpp
+++ clang/test/Index/evaluate-cursor.cpp
@@ -26,6 +26,9 @@
   static const auto g = alignof(f);
 };
 
+constexpr static int calc_val() { return 1 + 2; }
+const auto the_value = calc_val() + sizeof(char);
+
 // RUN: c-index-test -evaluate-cursor-at=%s:4:7 \
 // RUN:-evaluate-cursor-at=%s:8:7 \
 // RUN:-evaluate-cursor-at=%s:8:11 -std=c++11 %s | FileCheck %s
@@ -53,3 +56,12 @@
 // RUN:-evaluate-cursor-at=%s:26:21 \
 // RUN:-std=c++11 %s | FileCheck -check-prefix=CHECK-DOES-NOT-CRASH %s
 // CHECK-DOES-NOT-CRASH: Not Evaluatable
+
+// RUN: c-index-test -evaluate-cursor-at=%s:30:1 \
+// RUN:-evaluate-cursor-at=%s:30:32 \
+// RUN:-evaluate-cursor-at=%s:30:35 \
+// RUN:-evaluate-cursor-at=%s:30:37 -std=c++11 %s | FileCheck %s -check-prefix=CHECK-EXPR
+// CHECK-EXPR: unsigned, Value: 4
+// CHECK-EXPR: Value: 3
+// CHECK-EXPR: unsigned, Value: 4
+// CHECK-EXPR: unsigned, Value: 1
Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -5933,6 +5933,7 @@
  * 

[PATCH] D80015: [Analyzer][StreamChecker] Added support for 'fread' and 'fwrite'.

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

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

I want to test the StreamChecker for false positives. Page 
http://clang.llvm.org/analyzer/open_projects.html says that it has too much 
false positives because state splitting (I did not see this page before.)

There is an alternative way of implementation: Store a (ordered) list of 
previous stream operations and data what we know about the operation. This data 
contains if the operation failed or not, the symbol that is the return value of 
the function, error state before and after the operation, what bug report was 
made already. This list can be populated during analysis, without state splits. 
At every time when a new information is put into the list it is possible to 
check for problems.




Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:523
+  // FEOF).
+  if (!IsFread || (SS->ErrorState != ErrorFEof)) {
+ProgramStateRef StateNotFailed =

Szelethus wrote:
> balazske wrote:
> > Szelethus wrote:
> > > Aaaah okay it took me a while. I think `SS->ErrorState != ErrorFEof` 
> > > isn't as talkative as `isConstrained.*`, could you add just a line like 
> > > "if we **know** the state to be EOF..."?
> > > 
> > > On another note, why is fwrite so special in this context?
> > If it is an `fwrite`, the function call can always succeed (even after a 
> > previously EOF or error).
> > If it is an `fread`, it can succeed but not after an exactly-EOF state.
> > 
> > A success transition is not needed in an `fread` with exactly EOF 
> > condition. (If we have fread with non-exactly EOF, for example `ErrorFEof` 
> > and `ErrorFError`, the success is needed because the read in `ErrorFError` 
> > can succeed.)
> > Aaaah okay it took me a while. I think SS->ErrorState != ErrorFEof isn't as 
> > talkative as isConstrained.*, could you add just a line like "if we know 
> > the state to be EOF..."?
> 
> Could you please add these few words before commiting?
It is now "If we know the state to be FEOF at fread, do not add a success 
state.".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80015



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

[PATCH] D80225: [Driver] Recognize -fuse-ld={bfd, gold, lld} but don't prepend "ld." or "ld64." for other values

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

I'm a bit nervous about this.  I'm aware of at least one out-of-tree toolchain 
that uses this mechanism to select their proprietary linker.  I'd expect an RFC 
and for this to be highlighted in LLVM Weekly before I'm happy that this won't 
break downstream consumers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80225



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay planned changes to this revision.
MyDeveloperDay marked 3 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1566
   Current.Type = TT_TrailingReturnArrow;
-
+} else if (Current.is(tok::arrow) && Current.Previous &&
+   Current.Previous->is(tok::r_brace)) {

miscco wrote:
> MyDeveloperDay wrote:
> > miscco wrote:
> > > Should that really be `tok::arrow` and not `tok::kw_requires`?
> > This is to prevent the breaking between } and -> in the following (which 
> > could be some way from the requires
> > 
> > `{ t.foo(u) } -> typename T::foo_type;`
> I believe this should carry some state that we are inside of an 
> requires-expression. 
I'm not sure how possible that is as its dealt with in terms of normal 
parseBlock() handling so there might not be a possiblity to label the -> (which 
is actually what I'm just doing here) the other rules regarding 
TT_TrailingReturnArrow will ensure its doesn't get broken and there is spaces 
around it.


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

https://reviews.llvm.org/D79773



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

[PATCH] D80212: [clangd] Handle references for patched macros

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Polish


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80212

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp


Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -14,12 +14,14 @@
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "XRefs.h"
 #include "clang/Format/Format.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "gmock/gmock.h"
@@ -31,6 +33,7 @@
 
 using testing::Contains;
 using testing::Field;
+using testing::Matcher;
 
 namespace clang {
 namespace clangd {
@@ -362,6 +365,55 @@
   }
 }
 
+TEST(PreamblePatchTest, RefsToMacros) {
+  struct {
+llvm::StringLiteral Baseline;
+llvm::StringLiteral Modified;
+  } Cases[] = {
+  // Newly added
+  {
+  "",
+  R"cpp(
+#define ^FOO
+^[[FOO]])cpp",
+  },
+  // Moved around
+  {
+  "#define FOO",
+  R"cpp(
+#define BAR
+#define ^FOO
+^[[FOO]])cpp",
+  },
+  // Ref in preamble section
+  {
+  "",
+  R"cpp(
+#define ^FOO
+#undef ^FOO)cpp",
+  },
+  };
+
+  for (const auto &Case : Cases) {
+Annotations Modified(Case.Modified);
+auto AST = createPatchedAST("", Modified.code());
+ASSERT_TRUE(AST);
+
+const auto &SM = AST->getSourceManager();
+std::vector> ExpectedLocations;
+for (const auto &R : Modified.ranges())
+  ExpectedLocations.push_back(Field(&Location::range, R));
+
+for (const auto &P : Modified.points()) {
+  auto *MacroTok = AST->getTokens().spelledTokenAt(SM.getComposedLoc(
+  SM.getMainFileID(),
+  llvm::cantFail(positionToOffset(Modified.code(), P;
+  ASSERT_TRUE(MacroTok);
+  EXPECT_THAT(findReferences(*AST, P, 0).References,
+  testing::ElementsAreArray(ExpectedLocations));
+}
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -756,8 +756,10 @@
 
   RefsRequest Req;
   if (Macro) {
-// Handle references to macro.
-if (auto MacroSID = getSymbolID(Macro->Name, Macro->IdentLoc, SM)) {
+// Handle references to macro, we are using DefRange instead of IdentLoc as
+// references are collected without #line directive mappings.
+if (auto MacroSID =
+getSymbolID(Macro->Name, Macro->DefRange.getBegin(), SM)) {
   // Collect macro references from main file.
   const auto &IDToRefs = AST.getMacros().MacroRefs;
   auto Refs = IDToRefs.find(*MacroSID);


Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -14,12 +14,14 @@
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "XRefs.h"
 #include "clang/Format/Format.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "gmock/gmock.h"
@@ -31,6 +33,7 @@
 
 using testing::Contains;
 using testing::Field;
+using testing::Matcher;
 
 namespace clang {
 namespace clangd {
@@ -362,6 +365,55 @@
   }
 }
 
+TEST(PreamblePatchTest, RefsToMacros) {
+  struct {
+llvm::StringLiteral Baseline;
+llvm::StringLiteral Modified;
+  } Cases[] = {
+  // Newly added
+  {
+  "",
+  R"cpp(
+#define ^FOO
+^[[FOO]])cpp",
+  },
+  // Moved around
+  {
+  "#define FOO",
+  R"cpp(
+#define BAR
+#define ^FOO
+^[[FOO]])cpp",
+  },
+  // Ref in preamble section
+  {
+  "",
+  R"cpp(
+#define ^FOO
+#undef ^FOO)cpp",
+  },
+  };
+
+  for (const auto &Case : Cases) {
+Annotations Modified(Case.Modified);
+auto AST = createPatchedAST("", Modified.code());
+ASSERT_TRUE(AST);
+
+const a

[PATCH] D80198: [clangd] locateMacroAt handles patched macros

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Polish


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80198

Files:
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp

Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -514,7 +514,7 @@
   ASSERT_TRUE(bool(CurLoc));
   const auto *Id = syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
   ASSERT_TRUE(Id);
-  auto Result = locateMacroAt(*Id, AST.getPreprocessor());
+  auto Result = locateMacroAt(*Id, AST.getPreprocessor(), AST.getTokens());
   ASSERT_TRUE(Result);
   EXPECT_THAT(*Result, MacroName("MACRO"));
 }
@@ -528,7 +528,7 @@
   ASSERT_TRUE(bool(CurLoc));
   const auto *Id = syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
   ASSERT_TRUE(Id);
-  auto Result = locateMacroAt(*Id, AST.getPreprocessor());
+  auto Result = locateMacroAt(*Id, AST.getPreprocessor(), AST.getTokens());
   ASSERT_TRUE(Result);
   EXPECT_THAT(*Result, MacroName("MACRO"));
 }
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -9,9 +9,12 @@
 #include "Annotations.h"
 #include "Compiler.h"
 #include "Headers.h"
+#include "Hover.h"
 #include "Preamble.h"
+#include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "clang/Format/Format.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/ADT/STLExtras.h"
@@ -198,7 +201,7 @@
 ADD_FAILURE() << "Failed to build compiler invocation";
 return llvm::None;
   }
-  return ParsedAST::build(testPath("main.cpp"), TU.inputs(), std::move(CI), {},
+  return ParsedAST::build(testPath(TU.Filename), TU.inputs(), std::move(CI), {},
   BaselinePreamble);
 }
 
@@ -243,6 +246,122 @@
   EXPECT_THAT(AST->getDiagnostics(),
   Not(Contains(Field(&Diag::Range, Modified.range();
 }
+
+TEST(PreamblePatchTest, LocateMacroAtWorks) {
+  struct {
+llvm::StringLiteral Baseline;
+llvm::StringLiteral Modified;
+  } Cases[] = {
+  // Addition of new directive
+  {
+  "",
+  R"cpp(
+#define $def^FOO
+$use^FOO)cpp",
+  },
+  // Available inside preamble section
+  {
+  "",
+  R"cpp(
+#define $def^FOO
+#undef $use^FOO)cpp",
+  },
+  // Available after undef, as we don't patch those
+  {
+  "",
+  R"cpp(
+#define $def^FOO
+#undef FOO
+$use^FOO)cpp",
+  },
+  // Identifier on a different line
+  {
+  "",
+  R"cpp(
+#define \
+  $def^FOO
+$use^FOO)cpp",
+  },
+  // In presence of comment tokens
+  {
+  "",
+  R"cpp(
+#\
+  define /* FOO */\
+  /* FOO */ $def^FOO
+$use^FOO)cpp",
+  },
+  // Moved around
+  {
+  "#define FOO",
+  R"cpp(
+#define BAR
+#define $def^FOO
+$use^FOO)cpp",
+  },
+  };
+  for (const auto &Case : Cases) {
+SCOPED_TRACE(Case.Modified);
+llvm::Annotations Modified(Case.Modified);
+auto AST = createPatchedAST(Case.Baseline, Modified.code());
+ASSERT_TRUE(AST);
+
+const auto &SM = AST->getSourceManager();
+auto *MacroTok = AST->getTokens().spelledTokenAt(
+SM.getComposedLoc(SM.getMainFileID(), Modified.point("use")));
+ASSERT_TRUE(MacroTok);
+
+auto FoundMacro =
+locateMacroAt(*MacroTok, AST->getPreprocessor(), AST->getTokens());
+ASSERT_TRUE(FoundMacro);
+EXPECT_THAT(FoundMacro->Name, "FOO");
+
+auto MacroLoc = FoundMacro->IdentLoc;
+EXPECT_EQ(SM.getFileID(MacroLoc), SM.getMainFileID());
+EXPECT_EQ(SM.getFileOffset(MacroLoc), Modified.point("def"));
+  }
+}
+
+TEST(PreamblePatchTest, LocateMacroAtDeletion) {
+  {
+// We don't patch deleted define directives, make sure we don't crash.
+llvm::StringLiteral Baseline = "#define FOO";
+llvm::Annotations Modified("^FOO");
+
+auto AST = createPatchedAST(Baseline, Modified.code());
+ASSERT_TRUE(AST);
+
+const auto &S

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

Given the extra complexity I'd like to see that it matters - bound nodes tend 
to be small.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80202



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Alexey Sachkov via Phabricator via cfe-commits]

AlexeySachkov added a comment.

Looks like the issue is fixed in 
https://github.com/llvm/llvm-project/commit/6d2b75e0887ee87e247756c4d51733616bb2f356


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66324



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

[PATCH] D79721: [Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts

[Lucas Prates via Phabricator via cfe-commits]

pratlucas added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79721



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

[PATCH] D79930: [clangd] Add buildPreamble to TestTU

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 265187.
kadircet marked an inline comment as done.
kadircet added a comment.

- Rename buildPreamble to preamble


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79930

Files:
  clang-tools-extra/clangd/unittests/PreambleTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h


Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -68,6 +68,7 @@
   // By default, build() will report Error diagnostics as GTest errors.
   // Suppress this behavior by adding an 'error-ok' comment to the code.
   ParsedAST build() const;
+  std::shared_ptr preamble() const;
   ParseInputs inputs() const;
   SymbolSlab headerSymbols() const;
   std::unique_ptr index() const;
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -66,14 +66,24 @@
   return Inputs;
 }
 
+std::shared_ptr TestTU::preamble() const {
+  auto Inputs = inputs();
+  IgnoreDiagnostics Diags;
+  auto CI = buildCompilerInvocation(Inputs, Diags);
+  assert(CI && "Failed to build compilation invocation.");
+  return clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs,
+  /*StoreInMemory=*/true,
+  /*PreambleCallback=*/nullptr);
+}
+
 ParsedAST TestTU::build() const {
   auto Inputs = inputs();
   StoreDiags Diags;
   auto CI = buildCompilerInvocation(Inputs, Diags);
   assert(CI && "Failed to build compilation invocation.");
-  auto Preamble =
-  buildPreamble(testPath(Filename), *CI, Inputs,
-/*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
+  auto Preamble = clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs,
+   /*StoreInMemory=*/true,
+   /*PreambleCallback=*/nullptr);
   auto AST = ParsedAST::build(testPath(Filename), Inputs, std::move(CI),
   Diags.take(), Preamble);
   if (!AST.hasValue()) {
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -36,35 +36,19 @@
   return arg.first() == File && arg.second == D;
 }
 
-std::shared_ptr
-createPreamble(llvm::StringRef Contents = "") {
-  auto TU = TestTU::withCode(Contents);
-  // ms-compatibility changes meaning of #import, make sure it is turned off.
-  TU.ExtraArgs = {"-fno-ms-compatibility"};
-  TU.Filename = "preamble.cpp";
-  auto PI = TU.inputs();
-  IgnoreDiagnostics Diags;
-  auto CI = buildCompilerInvocation(PI, Diags);
-  if (!CI) {
-ADD_FAILURE() << "failed to build compiler invocation";
-return nullptr;
-  }
-  if (auto Preamble = buildPreamble(TU.Filename, *CI, PI, true, nullptr))
-return Preamble;
-  ADD_FAILURE() << "failed to build preamble";
-  return nullptr;
-}
-
 // Builds a preamble for BaselineContents, patches it for ModifiedContents and
 // returns the includes in the patch.
 IncludeStructure
 collectPatchedIncludes(llvm::StringRef ModifiedContents,
llvm::StringRef BaselineContents,
llvm::StringRef MainFileName = "main.cpp") {
-  auto BaselinePreamble = createPreamble(BaselineContents);
-  // Create the patch.
-  auto TU = TestTU::withCode(ModifiedContents);
+  auto TU = TestTU::withCode(BaselineContents);
   TU.Filename = MainFileName.str();
+  // ms-compatibility changes meaning of #import, make sure it is turned off.
+  TU.ExtraArgs = {"-fno-ms-compatibility"};
+  auto BaselinePreamble = TU.preamble();
+  // Create the patch.
+  TU = TestTU::withCode(ModifiedContents);
   auto PI = TU.inputs();
   auto PP = PreamblePatch::create(testPath(TU.Filename), PI, 
*BaselinePreamble);
   // Collect patch contents.


Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -68,6 +68,7 @@
   // By default, build() will report Error diagnostics as GTest errors.
   // Suppress this behavior by adding an 'error-ok' comment to the code.
   ParsedAST build() const;
+  std::shared_ptr preamble() const;
   ParseInputs inputs() const;
   SymbolSlab headerSymbols() const;
   std::unique_ptr index() const;
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestT

[PATCH] D80198: [clangd] locateMacroAt handles patched macros

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80198

Files:
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp

Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -514,7 +514,7 @@
   ASSERT_TRUE(bool(CurLoc));
   const auto *Id = syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
   ASSERT_TRUE(Id);
-  auto Result = locateMacroAt(*Id, AST.getPreprocessor());
+  auto Result = locateMacroAt(*Id, AST.getPreprocessor(), AST.getTokens());
   ASSERT_TRUE(Result);
   EXPECT_THAT(*Result, MacroName("MACRO"));
 }
@@ -528,7 +528,7 @@
   ASSERT_TRUE(bool(CurLoc));
   const auto *Id = syntax::spelledIdentifierTouching(*CurLoc, AST.getTokens());
   ASSERT_TRUE(Id);
-  auto Result = locateMacroAt(*Id, AST.getPreprocessor());
+  auto Result = locateMacroAt(*Id, AST.getPreprocessor(), AST.getTokens());
   ASSERT_TRUE(Result);
   EXPECT_THAT(*Result, MacroName("MACRO"));
 }
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -9,14 +9,19 @@
 #include "Annotations.h"
 #include "Compiler.h"
 #include "Headers.h"
+#include "Hover.h"
 #include "Preamble.h"
+#include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "XRefs.h"
+#include "clang/Format/Format.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "gmock/gmock.h"
@@ -28,6 +33,7 @@
 
 using testing::Contains;
 using testing::Field;
+using testing::Matcher;
 
 namespace clang {
 namespace clangd {
@@ -198,7 +204,7 @@
 ADD_FAILURE() << "Failed to build compiler invocation";
 return llvm::None;
   }
-  return ParsedAST::build(testPath("main.cpp"), TU.inputs(), std::move(CI), {},
+  return ParsedAST::build(testPath(TU.Filename), TU.inputs(), std::move(CI), {},
   BaselinePreamble);
 }
 
@@ -243,6 +249,171 @@
   EXPECT_THAT(AST->getDiagnostics(),
   Not(Contains(Field(&Diag::Range, Modified.range();
 }
+
+TEST(PreamblePatchTest, LocateMacroAtWorks) {
+  struct {
+llvm::StringLiteral Baseline;
+llvm::StringLiteral Modified;
+  } Cases[] = {
+  // Addition of new directive
+  {
+  "",
+  R"cpp(
+#define $def^FOO
+$use^FOO)cpp",
+  },
+  // Available inside preamble section
+  {
+  "",
+  R"cpp(
+#define $def^FOO
+#undef $use^FOO)cpp",
+  },
+  // Available after undef, as we don't patch those
+  {
+  "",
+  R"cpp(
+#define $def^FOO
+#undef FOO
+$use^FOO)cpp",
+  },
+  // Identifier on a different line
+  {
+  "",
+  R"cpp(
+#define \
+  $def^FOO
+$use^FOO)cpp",
+  },
+  // In presence of comment tokens
+  {
+  "",
+  R"cpp(
+#\
+  define /* FOO */\
+  /* FOO */ $def^FOO
+$use^FOO)cpp",
+  },
+  // Moved around
+  {
+  "#define FOO",
+  R"cpp(
+#define BAR
+#define $def^FOO
+$use^FOO)cpp",
+  },
+  };
+  for (const auto &Case : Cases) {
+SCOPED_TRACE(Case.Modified);
+llvm::Annotations Modified(Case.Modified);
+auto AST = createPatchedAST(Case.Baseline, Modified.code());
+ASSERT_TRUE(AST);
+
+const auto &SM = AST->getSourceManager();
+auto *MacroTok = AST->getTokens().spelledTokenAt(
+SM.getComposedLoc(SM.getMainFileID(), Modified.point("use")));
+ASSERT_TRUE(MacroTok);
+
+auto FoundMacro =
+locateMacroAt(*MacroTok, AST->getPreprocessor(), AST->getTokens());
+ASSERT_TRUE(FoundMacro);
+EXPECT_THAT(FoundMacro->Name, "FOO");
+
+auto MacroLoc = FoundMacro->IdentLoc;
+EXPECT_EQ(SM.getFileID(MacroLoc), SM.getMainFileID());
+EXPECT_EQ(SM.getFileOffset(Macro

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D80202#2046266 , @klimek wrote:

> Given the extra complexity I'd like to see that it matters - bound nodes tend 
> to be small.


I put that in the description, but this is where i need help. whats the best 
way to benchmark the matchers? 
Also, do you know how it was benchmarked when `MaxMemoizationEntries` was 
decided upon?
There was also comments about some making some micro benchmarks but I don't 
think that was acted upon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80202



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

[PATCH] D79930: [clangd] Add buildPreamble to TestTU

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79930

Files:
  clang-tools-extra/clangd/unittests/PreambleTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/TestTU.h


Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -68,6 +68,7 @@
   // By default, build() will report Error diagnostics as GTest errors.
   // Suppress this behavior by adding an 'error-ok' comment to the code.
   ParsedAST build() const;
+  std::shared_ptr preamble() const;
   ParseInputs inputs() const;
   SymbolSlab headerSymbols() const;
   std::unique_ptr index() const;
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -66,14 +66,24 @@
   return Inputs;
 }
 
+std::shared_ptr TestTU::preamble() const {
+  auto Inputs = inputs();
+  IgnoreDiagnostics Diags;
+  auto CI = buildCompilerInvocation(Inputs, Diags);
+  assert(CI && "Failed to build compilation invocation.");
+  return clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs,
+  /*StoreInMemory=*/true,
+  /*PreambleCallback=*/nullptr);
+}
+
 ParsedAST TestTU::build() const {
   auto Inputs = inputs();
   StoreDiags Diags;
   auto CI = buildCompilerInvocation(Inputs, Diags);
   assert(CI && "Failed to build compilation invocation.");
-  auto Preamble =
-  buildPreamble(testPath(Filename), *CI, Inputs,
-/*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
+  auto Preamble = clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs,
+   /*StoreInMemory=*/true,
+   /*PreambleCallback=*/nullptr);
   auto AST = ParsedAST::build(testPath(Filename), Inputs, std::move(CI),
   Diags.take(), Preamble);
   if (!AST.hasValue()) {
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -36,35 +36,19 @@
   return arg.first() == File && arg.second == D;
 }
 
-std::shared_ptr
-createPreamble(llvm::StringRef Contents = "") {
-  auto TU = TestTU::withCode(Contents);
-  // ms-compatibility changes meaning of #import, make sure it is turned off.
-  TU.ExtraArgs = {"-fno-ms-compatibility"};
-  TU.Filename = "preamble.cpp";
-  auto PI = TU.inputs();
-  IgnoreDiagnostics Diags;
-  auto CI = buildCompilerInvocation(PI, Diags);
-  if (!CI) {
-ADD_FAILURE() << "failed to build compiler invocation";
-return nullptr;
-  }
-  if (auto Preamble = buildPreamble(TU.Filename, *CI, PI, true, nullptr))
-return Preamble;
-  ADD_FAILURE() << "failed to build preamble";
-  return nullptr;
-}
-
 // Builds a preamble for BaselineContents, patches it for ModifiedContents and
 // returns the includes in the patch.
 IncludeStructure
 collectPatchedIncludes(llvm::StringRef ModifiedContents,
llvm::StringRef BaselineContents,
llvm::StringRef MainFileName = "main.cpp") {
-  auto BaselinePreamble = createPreamble(BaselineContents);
-  // Create the patch.
-  auto TU = TestTU::withCode(ModifiedContents);
+  auto TU = TestTU::withCode(BaselineContents);
   TU.Filename = MainFileName.str();
+  // ms-compatibility changes meaning of #import, make sure it is turned off.
+  TU.ExtraArgs = {"-fno-ms-compatibility"};
+  auto BaselinePreamble = TU.preamble();
+  // Create the patch.
+  TU = TestTU::withCode(ModifiedContents);
   auto PI = TU.inputs();
   auto PP = PreamblePatch::create(testPath(TU.Filename), PI, 
*BaselinePreamble);
   // Collect patch contents.


Index: clang-tools-extra/clangd/unittests/TestTU.h
===
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -68,6 +68,7 @@
   // By default, build() will report Error diagnostics as GTest errors.
   // Suppress this behavior by adding an 'error-ok' comment to the code.
   ParsedAST build() const;
+  std::shared_ptr preamble() const;
   ParseInputs inputs() const;
   SymbolSlab headerSymbols() const;
   std::unique_ptr index() const;
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -66,14 +66

[PATCH] D79992: [clangd] Patch PP directives to use stale preambles while building ASTs

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79992

Files:
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp

Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -26,6 +26,7 @@
 #include 
 #include 
 
+using testing::Contains;
 using testing::Field;
 
 namespace clang {
@@ -181,6 +182,67 @@
   ElementsAre(AllOf(Field(&Inclusion::Written, "\"a.h\""),
 Field(&Inclusion::Resolved, testPath("a.h");
 }
+
+llvm::Optional createPatchedAST(llvm::StringRef Baseline,
+   llvm::StringRef Modified) {
+  auto BaselinePreamble = TestTU::withCode(Baseline).preamble();
+  if (!BaselinePreamble) {
+ADD_FAILURE() << "Failed to build baseline preamble";
+return llvm::None;
+  }
+
+  IgnoreDiagnostics Diags;
+  auto TU = TestTU::withCode(Modified);
+  auto CI = buildCompilerInvocation(TU.inputs(), Diags);
+  if (!CI) {
+ADD_FAILURE() << "Failed to build compiler invocation";
+return llvm::None;
+  }
+  return ParsedAST::build(testPath("main.cpp"), TU.inputs(), std::move(CI), {},
+  BaselinePreamble);
+}
+
+TEST(PreamblePatchTest, Define) {
+  // BAR should be defined while parsing the AST.
+  llvm::StringLiteral Cases[] = {
+  R"cpp(
+#define BAR
+[[BAR]])cpp",
+  // multiline macro
+  R"cpp(
+#define BAR \
+
+[[BAR]])cpp",
+  // multiline macro
+  R"cpp(
+#define \
+BAR
+[[BAR]])cpp",
+  };
+
+  for (llvm::StringLiteral Case : Cases) {
+SCOPED_TRACE(Case);
+Annotations Modified(Case);
+auto AST = createPatchedAST("", Modified.code());
+ASSERT_TRUE(AST);
+EXPECT_THAT(AST->getDiagnostics(),
+Not(Contains(Field(&Diag::Range, Modified.range();
+  }
+}
+
+TEST(PreamblePatchTest, OrderingPreserved) {
+  llvm::StringLiteral Baseline = "#define BAR(X) X";
+  Annotations Modified(R"cpp(
+#define BAR(X, Y) X Y
+#define BAR(X) X
+[[BAR]](int y);
+  )cpp");
+
+  auto AST = createPatchedAST(Baseline, Modified.code());
+  ASSERT_TRUE(AST);
+  EXPECT_THAT(AST->getDiagnostics(),
+  Not(Contains(Field(&Diag::Range, Modified.range();
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -15,6 +15,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
@@ -106,14 +107,68 @@
   const SourceManager *SourceMgr = nullptr;
 };
 
-/// Gets the includes in the preamble section of the file by running
-/// preprocessor over \p Contents. Returned includes do not contain resolved
-/// paths. \p VFS and \p Cmd is used to build the compiler invocation, which
-/// might stat/read files.
-llvm::Expected>
-scanPreambleIncludes(llvm::StringRef Contents,
- llvm::IntrusiveRefCntPtr VFS,
- const tooling::CompileCommand &Cmd) {
+struct TextualPPDirective {
+  unsigned DirectiveLine;
+  // Full text that's representing the directive, including the `#`.
+  std::string Text;
+
+  bool operator==(const TextualPPDirective &RHS) const {
+return std::tie(DirectiveLine, Text) ==
+   std::tie(RHS.DirectiveLine, RHS.Text);
+  }
+};
+
+// Collects #define directives inside the main file.
+struct DirectiveCollector : public PPCallbacks {
+  DirectiveCollector(const Preprocessor &PP,
+ std::vector &TextualDirectives)
+  : LangOpts(PP.getLangOpts()), SM(PP.getSourceManager()),
+TextualDirectives(TextualDirectives) {}
+
+  void FileChanged(SourceLocation Loc, FileChangeReason Reason,
+   SrcMgr::CharacteristicKind FileType,
+   FileID PrevFID) override {
+InMainFile = SM.isWrittenInMainFile(Loc);
+  }
+
+  void MacroDefined(const Token &MacroNameTok,
+const MacroDirective *MD) override {
+if (!InMainFile)
+  return;
+TextualDirectives.emplace_back();
+TextualPPDirective &TD = TextualDirectives.back();
+
+auto DecompLoc = SM.getDecomposedLoc(MacroNameTok.getLocation());
+TD.DirectiveLine = SM.getLineNumber(DecompLoc.first, DecompLoc.second);
+
+SourceRange DefRange(
+MD->getMacroInfo()->getDefin

[PATCH] D78658: [clang][Frontend] Add missing error handling

[LemonBoy via Phabricator via cfe-commits]

LemonBoy added a comment.

Ping?


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

https://reviews.llvm.org/D78658



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

[PATCH] D80286: [Analyzer] Allow creation of stack frame for functions without definition

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

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, Szelethus.
baloghadamsoftware added a project: clang.
Herald added subscribers: ASDenysPetrov, martong, steakhal, Charusso, 
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
xazax.hun, whisperity.
baloghadamsoftware added a parent revision: D79704: [Analyzer] [NFC] Parameter 
Regions.
baloghadamsoftware marked 2 inline comments as done.
baloghadamsoftware added a comment.

I tested this on several open-source projects: BitCoin, CURL, OpenSLL, 
PostGreS, TMux, Xerces and even on LLVM itself with most of the projects 
enabled. No new crash and no change in findings. So it seems to be stable.




Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:179
-  // situation because there's a loss of precision anyway because we cannot
-  // inline the call.
-  RuntimeDefinition RD = getRuntimeDefinition();

We introduced `ParamRegion`s to overcome this, but please provide me the tests 
that crash when deleting these lines without `ParamRegions` you mentioned 
D49443#1193290.



Comment at: clang/test/Analysis/temporaries.cpp:893
 glob = 1;
-// FIXME: Why is destructor not inlined in C++17
 clang_analyzer_checkInlined(true);
 #ifdef TEMPORARY_DTORS

I wonder whether `clang_analyzer_checkInlined()` works correctly with this 
patch: it seems it only checks for stack frame which now any function with 
definition can have.


Retrieving the parameter location of functions was disabled because it may 
causes crashes due to the fact that functions may have multiple declarations 
and without definition it is difficult to ensure that always the same 
declration is used. Now parameters are stored in `ParamRegions` which are 
independent of the declaration of the function, therefore the same parameters 
always have the same regions, independently of the function declaration used 
actually. This allows us to remove the limitation described above.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80286

Files:
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/explain-svals.cpp
  clang/test/Analysis/temporaries.cpp


Index: clang/test/Analysis/temporaries.cpp
===
--- clang/test/Analysis/temporaries.cpp
+++ clang/test/Analysis/temporaries.cpp
@@ -890,12 +890,9 @@
 public:
   ~C() {
 glob = 1;
-// FIXME: Why is destructor not inlined in C++17
 clang_analyzer_checkInlined(true);
 #ifdef TEMPORARY_DTORS
-#if __cplusplus < 201703L
-// expected-warning@-3{{TRUE}}
-#endif
+// expected-warning@-2{{TRUE}}
 #endif
   }
 };
@@ -914,16 +911,11 @@
   // temporaries returned from functions, so we took the wrong branch.
   coin && is(get()); // no-crash
   if (coin) {
-// FIXME: Why is destructor not inlined in C++17
 clang_analyzer_eval(glob);
 #ifdef TEMPORARY_DTORS
-#if __cplusplus < 201703L
-// expected-warning@-3{{TRUE}}
-#else
-// expected-warning@-5{{UNKNOWN}}
-#endif
+// expected-warning@-2{{TRUE}}
 #else
-// expected-warning@-8{{UNKNOWN}}
+// expected-warning@-4{{UNKNOWN}}
 #endif
   } else {
 // The destructor is not called on this branch.
Index: clang/test/Analysis/explain-svals.cpp
===
--- clang/test/Analysis/explain-svals.cpp
+++ clang/test/Analysis/explain-svals.cpp
@@ -94,7 +94,7 @@
 } // end of anonymous namespace
 
 void test_6() {
-  clang_analyzer_explain(conjure_S()); // expected-warning-re^lazily 
frozen compound value of temporary object constructed at statement 
'conjure_S\(\)'$
+  clang_analyzer_explain(conjure_S()); // expected-warning-re^lazily 
frozen compound value of parameter 0 of function 
'clang_analyzer_explain\(\)'$
   clang_analyzer_explain(conjure_S().z); // expected-warning-re^value 
derived from \(symbol of type 'int' conjured at statement 'conjure_S\(\)'\) for 
field 'z' of temporary object constructed at statement 'conjure_S\(\)'$
 }
 
Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -172,23 +172,9 @@
   if (!D)
 return nullptr;
 
-  // TODO: For now we skip functions without definitions, even if we have
-  // our own getDecl(), because it's hard to find out which re-declaration
-  // is going to be used, and usually clients don't really care about this
-  // situation because there's a loss of precision anyway because we cannot
-  // inline the call.
-  RuntimeDefinition RD = getRuntimeDefinition();
-  if (!RD.getDecl())
-return nullptr;
-
   AnalysisDeclContext *ADC =
   LCtx->getAnalysisDeclContext()->getManager()->getContext(D);
 
-  // TODO: For now we skip virtual functions, because this also rises
- 

[PATCH] D80286: [Analyzer] Allow creation of stack frame for functions without definition

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

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

I tested this on several open-source projects: BitCoin, CURL, OpenSLL, 
PostGreS, TMux, Xerces and even on LLVM itself with most of the projects 
enabled. No new crash and no change in findings. So it seems to be stable.




Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:179
-  // situation because there's a loss of precision anyway because we cannot
-  // inline the call.
-  RuntimeDefinition RD = getRuntimeDefinition();

We introduced `ParamRegion`s to overcome this, but please provide me the tests 
that crash when deleting these lines without `ParamRegions` you mentioned 
D49443#1193290.



Comment at: clang/test/Analysis/temporaries.cpp:893
 glob = 1;
-// FIXME: Why is destructor not inlined in C++17
 clang_analyzer_checkInlined(true);
 #ifdef TEMPORARY_DTORS

I wonder whether `clang_analyzer_checkInlined()` works correctly with this 
patch: it seems it only checks for stack frame which now any function with 
definition can have.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80286



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

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D75791#2045986 , @MarcusJohnson91 
wrote:

> In D75791#2044492 , @MyDeveloperDay 
> wrote:
>
> > If you want me to land this for you, I'd feel more comfortable landing it 
> > if:
> >
> > a) We can land D80214: [clang-format] Set of unit test to begin to validate 
> > that we don't change defaults  first
> >  b) The Mozilla team have tested the impact (they clang-format their entire 
> > code base I think)
>
>
> I'm ok with accepting commit access, and I agree lets get D80214: 
> [clang-format] Set of unit test to begin to validate that we don't change 
> defaults  in, and see if Mozilla, Microsoft, 
> Google, etc  has any comments; I'm just not sure of who to ping.
>
> Is there anything else that D80214: [clang-format] Set of unit test to begin 
> to validate that we don't change defaults  
> needs? it looked pretty well fleshed out.


I need an Accept on D80214: [clang-format] Set of unit test to begin to 
validate that we don't change defaults  and I 
think @Abpostelnicu would have let us know if it failed.


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

https://reviews.llvm.org/D75791



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

[PATCH] D79721: [Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts

[Oliver Stannard (Linaro) via Phabricator via cfe-commits]

ostannard accepted this revision.
ostannard 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/D79721/new/

https://reviews.llvm.org/D79721



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

[PATCH] D80288: [AST] default implementation is possible for non-member functions in C++2a.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: clang.

Make RAV not visit the default function decl by default.
Also update some stale comments on FunctionDecl::isDefault.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80288

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp


Index: clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp
===
--- clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp
@@ -55,4 +55,22 @@
 EXPECT_TRUE(Visitor.runOver(Code, CXXMethodDeclVisitor::Lang_CXX11));
   }
 }
+
+TEST(RecursiveASTVisitor, FunctionDeclNoDefaultBodyVisited) {
+  for (bool VisitImplCode : {false, true}) {
+CXXMethodDeclVisitor Visitor(VisitImplCode);
+if (VisitImplCode)
+  Visitor.ExpectMatch("declref", 4, 58, /*Times=*/2);
+else
+  Visitor.DisallowMatch("declref", 4, 58);
+llvm::StringRef Code = R"cpp(
+  struct s {
+int x;
+friend auto operator==(s a, s b) -> bool = default;
+  };
+  bool k = s() == s(); // make sure clang generates the "==" definition.
+)cpp";
+EXPECT_TRUE(Visitor.runOver(Code, CXXMethodDeclVisitor::Lang_CXX2a));
+  }
+}
 } // end anonymous namespace
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2103,11 +2103,11 @@
 }
   }
 
-  bool VisitBody = D->isThisDeclarationADefinition();
-  // If a method is set to default outside the class definition the compiler
-  // generates the method body and adds it to the AST.
-  if (const auto *MD = dyn_cast(D))
-VisitBody &= !MD->isDefaulted() || getDerived().shouldVisitImplicitCode();
+  bool VisitBody =
+  D->isThisDeclarationADefinition() &&
+  // Don't visit the function body if the function definition is generated
+  // by clang.
+  (!D->isDefaulted() || getDerived().shouldVisitImplicitCode());
 
   if (VisitBody) {
 TRY_TO(TraverseStmt(D->getBody())); // Function body.
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -2125,19 +2125,18 @@
   bool isTrivialForCall() const { return FunctionDeclBits.IsTrivialForCall; }
   void setTrivialForCall(bool IT) { FunctionDeclBits.IsTrivialForCall = IT; }
 
-  /// Whether this function is defaulted per C++0x. Only valid for
-  /// special member functions.
+  /// Whether this function is defaulted per C++0x. Valid for e.g.
+  /// special member functions, C++2a friend default comparisions
+  // (not methods!).
   bool isDefaulted() const { return FunctionDeclBits.IsDefaulted; }
   void setDefaulted(bool D = true) { FunctionDeclBits.IsDefaulted = D; }
 
-  /// Whether this function is explicitly defaulted per C++0x. Only valid
-  /// for special member functions.
+  /// Whether this function is explicitly defaulted per C++0x.
   bool isExplicitlyDefaulted() const {
 return FunctionDeclBits.IsExplicitlyDefaulted;
   }
 
-  /// State that this function is explicitly defaulted per C++0x. Only valid
-  /// for special member functions.
+  /// State that this function is explicitly defaulted per C++0x.
   void setExplicitlyDefaulted(bool ED = true) {
 FunctionDeclBits.IsExplicitlyDefaulted = ED;
   }


Index: clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp
===
--- clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp
@@ -55,4 +55,22 @@
 EXPECT_TRUE(Visitor.runOver(Code, CXXMethodDeclVisitor::Lang_CXX11));
   }
 }
+
+TEST(RecursiveASTVisitor, FunctionDeclNoDefaultBodyVisited) {
+  for (bool VisitImplCode : {false, true}) {
+CXXMethodDeclVisitor Visitor(VisitImplCode);
+if (VisitImplCode)
+  Visitor.ExpectMatch("declref", 4, 58, /*Times=*/2);
+else
+  Visitor.DisallowMatch("declref", 4, 58);
+llvm::StringRef Code = R"cpp(
+  struct s {
+int x;
+friend auto operator==(s a, s b) -> bool = default;
+  };
+  bool k = s() == s(); // make sure clang generates the "==" definition.
+)cpp";
+EXPECT_TRUE(Visitor.runOver(Code, CXXMethodDeclVisitor::Lang_CXX2a));
+  }
+}
 } // end anonymous namespace
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2103,11 +2103,11 @@
 }
   }
 
-  bool VisitBody = D->i

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D80202#2046321 , @njames93 wrote:

> In D80202#2046266 , @klimek wrote:
>
> > Given the extra complexity I'd like to see that it matters - bound nodes 
> > tend to be small.
>
>
> I put that in the description, but this is where i need help. whats the best 
> way to benchmark the matchers? 
>  Also, do you know how it was benchmarked when `MaxMemoizationEntries` was 
> decided upon?
>  There was also comments about some making some micro benchmarks but I don't 
> think that was acted upon.


I do remember benchmarking it when I wrote it, but that was 10 years ago :)
I mainly did benchmarks on large ASTs, trying various matchers.
Today we can benchmark for example whether running clang-tidy on all of LLVM 
makes a difference.
Micro-BMs would be nice to add, but are somewhat hard to get right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80202



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

[PATCH] D80289: [Driver][X86] Support branch align options with LTO

[Kan Shengchen via Phabricator via cfe-commits]

skan created this revision.
Herald added subscribers: cfe-commits, dexonsmith, steven_wu, hiraditya, 
inglorion.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80289

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/lto.c

Index: clang/test/Driver/lto.c
===
--- clang/test/Driver/lto.c
+++ clang/test/Driver/lto.c
@@ -77,3 +77,20 @@
 //
 // CHECK-TUNING-LLDB:   "-plugin-opt=-debugger-tune=lldb"
 // CHECK-NO-TUNING-NOT: "-plugin-opt=-debugger-tune
+
+/// -flto passes along an explicit branch align argument.
+/// Test -malign-branch-boundary=
+// RUN: %clang -target x86_64-unknown-linux -malign-branch-boundary=16 -flto %s -### 2>&1 | FileCheck %s --check-prefix=BOUNDARY
+// BOUNDARY: "-plugin-opt=-x86-align-branch-boundary=16"
+
+/// Test -malign-branch=
+// RUN: %clang -target x86_64-unknown-linux -malign-branch=fused,jcc,jmp -flto %s -### %s 2>&1 | FileCheck %s --check-prefix=TYPE
+// TYPE: "-plugin-opt=-x86-align-branch=fused+jcc+jmp"
+
+/// Test -mpad-max-prefix-size=
+// RUN: %clang -target x86_64-unknown-linux -mpad-max-prefix-size=0 -flto %s -### 2>&1 | FileCheck %s --check-prefix=PREFIX-0
+// PREFIX-0: "-plugin-opt=-x86-pad-max-prefix-size=0"
+
+/// Test -mbranches-within-32B-boundaries
+// RUN: %clang -target x86_64-unknown-linux -mbranches-within-32B-boundaries -flto %s -### 2>&1 | FileCheck %s --check-prefix=32B
+// 32B: "-plugin-opt=-x86-branches-within-32B-boundaries"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -358,6 +358,7 @@
   ArgStringList &CmdArgs, const InputInfo &Output,
   const InputInfo &Input, bool IsThinLTO) {
   const char *Linker = Args.MakeArgString(ToolChain.GetLinkerPath());
+  const Driver &D = ToolChain.getDriver();
   if (llvm::sys::path::filename(Linker) != "ld.lld" &&
   llvm::sys::path::stem(Linker) != "ld.lld") {
 // Tell the linker to load the plugin. This has to come before
@@ -374,10 +375,9 @@
 #endif
 
 SmallString<1024> Plugin;
-llvm::sys::path::native(Twine(ToolChain.getDriver().Dir) +
-"/../lib" CLANG_LIBDIR_SUFFIX "/LLVMgold" +
-Suffix,
-Plugin);
+llvm::sys::path::native(
+Twine(D.Dir) + "/../lib" CLANG_LIBDIR_SUFFIX "/LLVMgold" + Suffix,
+Plugin);
 CmdArgs.push_back(Args.MakeArgString(Plugin));
   }
 
@@ -417,7 +417,7 @@
   if (IsThinLTO)
 CmdArgs.push_back("-plugin-opt=thinlto");
 
-  StringRef Parallelism = getLTOParallelism(Args, ToolChain.getDriver());
+  StringRef Parallelism = getLTOParallelism(Args, D);
   if (!Parallelism.empty())
 CmdArgs.push_back(
 Args.MakeArgString("-plugin-opt=jobs=" + Twine(Parallelism)));
@@ -449,7 +449,7 @@
   if (Arg *A = getLastProfileSampleUseArg(Args)) {
 StringRef FName = A->getValue();
 if (!llvm::sys::fs::exists(FName))
-  ToolChain.getDriver().Diag(diag::err_drv_no_such_file) << FName;
+  D.Diag(diag::err_drv_no_such_file) << FName;
 else
   CmdArgs.push_back(
   Args.MakeArgString(Twine("-plugin-opt=sample-profile=") + FName));
@@ -492,11 +492,52 @@
   }
 
   // Setup statistics file output.
-  SmallString<128> StatsFile =
-  getStatsFileName(Args, Output, Input, ToolChain.getDriver());
+  SmallString<128> StatsFile = getStatsFileName(Args, Output, Input, D);
   if (!StatsFile.empty())
 CmdArgs.push_back(
 Args.MakeArgString(Twine("-plugin-opt=stats-file=") + StatsFile));
+
+  // Handle options for branch align
+  if (Args.hasArg(options::OPT_mbranches_within_32B_boundaries)) {
+CmdArgs.push_back("-plugin-opt=-x86-branches-within-32B-boundaries");
+  }
+  if (const Arg *A = Args.getLastArg(options::OPT_malign_branch_boundary_EQ)) {
+StringRef Value = A->getValue();
+unsigned Boundary;
+if (Value.getAsInteger(10, Boundary) || Boundary < 16 ||
+!llvm::isPowerOf2_64(Boundary)) {
+  D.Diag(diag::err_drv_invalid_argument_to_option)
+  << Value << A->getOption().getName();
+} else {
+  CmdArgs.push_back(Args.MakeArgString(
+  "-plugin-opt=-x86-align-branch-boundary=" + Twine(Boundary)));
+}
+  }
+  if (const Arg *A = Args.getLastArg(options::OPT_malign_branch_EQ)) {
+std::string AlignBranch;
+for (StringRef T : A->getValues()) {
+  if (T != "fused" && T != "jcc" && T != "jmp" && T != "call" &&
+  T != "ret" && T != "indirect")
+D.Diag(diag::err_drv_invalid_malign_branch_EQ)
+<< T << "fused, jcc, jmp, call, ret, indirect";
+  if (!AlignBranch.empty())
+AlignBranch += '+';
+  AlignBranch += T;
+}
+CmdArgs.push_back(Args.MakeArgString("-plugin-opt=-x86-align-branch=" +
+ 

[clang] 3f333e0 - [analyzer] Get scan-view executable from environment.

[Artem Dergachev via cfe-commits]

Author: Artem Dergachev
Date: 2020-05-20T14:55:56+03:00
New Revision: 3f333e0af7a872a0152b4db972a37947e47e690d

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

LOG: [analyzer] Get scan-view executable from environment.

Fixes "Use of uninitialized value $ScanView in exec" error on systems
with scan-view executable not located in the expected place.

Patch by Oliver Tušla!

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

Added: 


Modified: 
clang/tools/scan-build/bin/scan-build

Removed: 




diff  --git a/clang/tools/scan-build/bin/scan-build 
b/clang/tools/scan-build/bin/scan-build
index 4b76333fb715..11334a0b9626 100755
--- a/clang/tools/scan-build/bin/scan-build
+++ b/clang/tools/scan-build/bin/scan-build
@@ -971,6 +971,7 @@ sub Finalize {
 my $ScanView = Cwd::realpath("$RealBin/scan-view");
 if (! -x $ScanView) { $ScanView = "scan-view"; }
 if (! -x $ScanView) { $ScanView = 
Cwd::realpath("$RealBin/../../scan-view/bin/scan-view"); }
+if (! -x $ScanView) { $ScanView = `which scan-view`; chomp $ScanView; }
 exec $ScanView, "$Options{OutputDir}";
   }
 



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

[PATCH] D78122: [analyzer][Nullability] Don't emit under the checker name NullabilityBase

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

We're observing a surprising disappearance of nullability reports today - like, 
they didn't just change the name, they disappeared completely. Investigating.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78122



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

[PATCH] D77880: get scan-view executable from environment

[Artem Dergachev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3f333e0af7a8: [analyzer] Get scan-view executable from 
environment. (authored by dergachev.a).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77880

Files:
  clang/tools/scan-build/bin/scan-build


Index: clang/tools/scan-build/bin/scan-build
===
--- clang/tools/scan-build/bin/scan-build
+++ clang/tools/scan-build/bin/scan-build
@@ -971,6 +971,7 @@
 my $ScanView = Cwd::realpath("$RealBin/scan-view");
 if (! -x $ScanView) { $ScanView = "scan-view"; }
 if (! -x $ScanView) { $ScanView = 
Cwd::realpath("$RealBin/../../scan-view/bin/scan-view"); }
+if (! -x $ScanView) { $ScanView = `which scan-view`; chomp $ScanView; }
 exec $ScanView, "$Options{OutputDir}";
   }
 


Index: clang/tools/scan-build/bin/scan-build
===
--- clang/tools/scan-build/bin/scan-build
+++ clang/tools/scan-build/bin/scan-build
@@ -971,6 +971,7 @@
 my $ScanView = Cwd::realpath("$RealBin/scan-view");
 if (! -x $ScanView) { $ScanView = "scan-view"; }
 if (! -x $ScanView) { $ScanView = Cwd::realpath("$RealBin/../../scan-view/bin/scan-view"); }
+if (! -x $ScanView) { $ScanView = `which scan-view`; chomp $ScanView; }
 exec $ScanView, "$Options{OutputDir}";
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80293: [clangd] Run PreambleThread in async mode behind a flag

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Depends on D80198 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80293

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -21,6 +21,7 @@
 #include "support/Threading.h"
 #include "clang/Basic/DiagnosticDriver.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "gmock/gmock.h"
@@ -29,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -90,6 +92,24 @@
   static Key)>>
   DiagsCallbackKey;
 
+  using PreambleCallback = llvm::unique_function;
+  static std::unique_ptr
+  capturePreamble(PreambleCallback CB) {
+class CapturePreamble : public ParsingCallbacks {
+public:
+  CapturePreamble(PreambleCallback CB) : CB(std::move(CB)) {}
+  void onPreambleAST(PathRef Path, llvm::StringRef Version, ASTContext &Ctx,
+ std::shared_ptr PP,
+ const CanonicalIncludes &) override {
+CB();
+  }
+
+private:
+  PreambleCallback CB;
+};
+return std::make_unique(std::move(CB));
+  }
+
   /// A diagnostics callback that should be passed to TUScheduler when it's used
   /// in updateWithDiags.
   static std::unique_ptr captureDiags() {
@@ -443,7 +463,7 @@
   WithContextValue WithNonce(NonceKey, ++Nonce);
   Inputs.Version = std::to_string(Nonce);
   updateWithDiags(
-  S, File, Inputs, WantDiagnostics::Auto,
+  S, File, Inputs, WantDiagnostics::Yes,
   [File, Nonce, &Mut, &TotalUpdates](std::vector) {
 EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
 
@@ -972,6 +992,40 @@
   EXPECT_NEAR(25, Compute({}), 0.01) << "no history -> max";
 }
 
+TEST_F(TUSchedulerTests, AsyncPreambleThread) {
+  Notification Ready;
+  std::atomic PreambleBuildCount{0};
+  TUScheduler S(CDB, optsForTest(), capturePreamble([&] {
+  if (PreambleBuildCount > 0)
+Ready.wait();
+  ++PreambleBuildCount;
+}));
+
+  Path File = testPath("foo.cpp");
+  auto PI = getInputs(File, "");
+  PI.Version = "initial";
+  S.update(File, PI, WantDiagnostics::Auto);
+  S.blockUntilIdle(timeoutSeconds(10));
+
+  // Block preamble builds.
+  PI.Version = "blocking";
+  // Issue second update which will block preamble thread.
+  S.update(File, PI, WantDiagnostics::Auto);
+
+  Notification RunASTAction;
+  // Issue an AST read, which shouldn't be blocked and see latest version of the
+  // file.
+  S.runWithAST("test", File, [&](Expected AST) {
+ASSERT_TRUE(bool(AST));
+EXPECT_THAT(AST->Inputs.Version, PI.Version);
+RunASTAction.notify();
+  });
+  RunASTAction.wait();
+  // Make sure second preamble hasn't been built yet.
+  EXPECT_THAT(PreambleBuildCount.load(), 1U);
+  Ready.notify();
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -211,18 +211,18 @@
   FS.Files[FooCpp] = SourceContents;
 
   Server.addDocument(FooCpp, SourceContents);
-  auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp);
   ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
+  auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp);
   EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
 
   Server.addDocument(FooCpp, "");
-  auto DumpParseEmpty = dumpASTWithoutMemoryLocs(Server, FooCpp);
   ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
+  auto DumpParseEmpty = dumpASTWithoutMemoryLocs(Server, FooCpp);
   EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
 
   Server.addDocument(FooCpp, SourceContents);
-  auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp);
   ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
+  auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp);
   EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
 
   EXPECT_EQ(DumpParse1, DumpParse2);
@@ -247,20 +247,20 @@
   FS.Files[FooCpp] = SourceConten

[PATCH] D80214: [clang-format] Set of unit test to begin to validate that we don't change defaults

[Marcus Johnson via Phabricator via cfe-commits]

MarcusJohnson91 accepted this revision.
MarcusJohnson91 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/D80214/new/

https://reviews.llvm.org/D80214



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

[PATCH] D78122: [analyzer][Nullability] Don't emit under the checker name NullabilityBase

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

Szelethus added a comment.

In D78122#2046450 , @NoQ wrote:

> We're observing a surprising disappearance of nullability reports today - 
> like, they didn't just change the name, they disappeared completely. 
> Investigating.


I landed a lot of patches in the middle of the night, that might be a factor. 
Say the word if I can help in any way!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78122



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

[PATCH] D77846: [analyzer][CallAndMessage][NFC] Split up checkPreCall

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

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp:27
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"

balazske wrote:
> Are these new includes needed?
Nope, you're right.



Comment at: clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp:371
-  const Decl *D = Call.getDecl();
-  if (D && (isa(D) || isa(D))) {
-// If we have a function or block declaration, we can make sure we pass

@balazske this is the corresponding condition that got changed. Now, your 
inline is fair, and I remember being puzzled by needing that condition to not 
crash. One thing to note is that CallAndMessage really tests every part of 
CallEvent, and since its registered very early (its also a dependency in 
addition to being a core checker) it runs before most other checkers. Anyways, 
I revisited each of the new functions I added, and wow, you totally nailed it. 
Nothing crashes if I remove that condition, but I vividly remember it breaking 
earlier.

So yeah, nice catch!




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

https://reviews.llvm.org/D77846



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

[PATCH] D77846: [analyzer][CallAndMessage][NFC] Split up checkPreCall

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

Szelethus updated this revision to Diff 265214.
Szelethus marked 4 inline comments as done.
Szelethus added a comment.

Fixes according to @balazske's comments, cheers!


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

https://reviews.llvm.org/D77846

Files:
  clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -60,6 +60,25 @@
 
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
 
+  ProgramStateRef checkFunctionPointerCall(const CallExpr *CE,
+   CheckerContext &C,
+   ProgramStateRef State) const;
+
+  ProgramStateRef checkCXXMethodCall(const CXXInstanceCall *CC,
+ CheckerContext &C,
+ ProgramStateRef State) const;
+
+  ProgramStateRef checkParameterCount(const CallEvent &Call, CheckerContext &C,
+  ProgramStateRef State) const;
+
+  ProgramStateRef checkCXXDeallocation(const CXXDeallocatorCall *DC,
+   CheckerContext &C,
+   ProgramStateRef State) const;
+
+  ProgramStateRef checkArgInitializedness(const CallEvent &Call,
+  CheckerContext &C,
+  ProgramStateRef State) const;
+
 private:
   bool PreVisitProcessArg(CheckerContext &C, SVal V, SourceRange ArgRange,
   const Expr *ArgEx, int ArgumentNumber,
@@ -309,123 +328,131 @@
   return false;
 }
 
-void CallAndMessageChecker::checkPreCall(const CallEvent &Call,
- CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
-  if (const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr())) {
-const Expr *Callee = CE->getCallee()->IgnoreParens();
-const LocationContext *LCtx = C.getLocationContext();
-SVal L = State->getSVal(Callee, LCtx);
-
-if (L.isUndef()) {
-  if (!BT_call_undef)
-BT_call_undef.reset(new BuiltinBug(
-this, "Called function pointer is an uninitialized pointer value"));
-  emitBadCall(BT_call_undef.get(), C, Callee);
-  return;
-}
+ProgramStateRef CallAndMessageChecker::checkFunctionPointerCall(
+const CallExpr *CE, CheckerContext &C, ProgramStateRef State) const {
 
-ProgramStateRef StNonNull, StNull;
-std::tie(StNonNull, StNull) =
-State->assume(L.castAs());
+  const Expr *Callee = CE->getCallee()->IgnoreParens();
+  const LocationContext *LCtx = C.getLocationContext();
+  SVal L = State->getSVal(Callee, LCtx);
+
+  if (L.isUndef()) {
+if (!BT_call_undef)
+  BT_call_undef.reset(new BuiltinBug(
+  this, "Called function pointer is an uninitialized pointer value"));
+emitBadCall(BT_call_undef.get(), C, Callee);
+return nullptr;
+  }
 
-if (StNull && !StNonNull) {
-  if (!BT_call_null)
-BT_call_null.reset(new BuiltinBug(
-this, "Called function pointer is null (null dereference)"));
-  emitBadCall(BT_call_null.get(), C, Callee);
-  return;
-}
+  ProgramStateRef StNonNull, StNull;
+  std::tie(StNonNull, StNull) = State->assume(L.castAs());
 
-State = StNonNull;
+  if (StNull && !StNonNull) {
+if (!BT_call_null)
+  BT_call_null.reset(new BuiltinBug(
+  this, "Called function pointer is null (null dereference)"));
+emitBadCall(BT_call_null.get(), C, Callee);
+return nullptr;
   }
 
-  // If this is a call to a C++ method, check if the callee is null or
-  // undefined.
-  if (const CXXInstanceCall *CC = dyn_cast(&Call)) {
-SVal V = CC->getCXXThisVal();
-if (V.isUndef()) {
-  if (!BT_cxx_call_undef)
-BT_cxx_call_undef.reset(
-new BuiltinBug(this, "Called C++ object pointer is uninitialized"));
-  emitBadCall(BT_cxx_call_undef.get(), C, CC->getCXXThisExpr());
-  return;
-}
+  return StNonNull;
+}
 
-ProgramStateRef StNonNull, StNull;
-std::tie(StNonNull, StNull) =
-State->assume(V.castAs());
+ProgramStateRef CallAndMessageChecker::checkParameterCount(
+const CallEvent &Call, CheckerContext &C, ProgramStateRef State) const {
 
-if (StNull && !StNonNull) {
-  if (!BT_cxx_call_null)
-BT_cxx_call_null.reset(
-new BuiltinBug(this, "Called C++ object pointer is null"));
-  emitBadCall(BT_cxx_call_null.get(), C, CC->getCXXThisExpr());
-  return;
-}
+  // If we have a function or block declaration, we can make sure we pass
+  // enough parameters.
+  unsigned Params = Call.parameters().size();
+  if (Call.getNumArgs() >= Params)
+return State;
+
+  ExplodedNode *N = C.gener

[PATCH] D79704: [Analyzer] [NFC] Parameter Regions

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Seems like many changes could be simplified or absolutely not needed in this 
patch if ParamRegion (or with a better name ParamVarRegion) was derived from 
VarRegion. Is there any difficulties in that derivation?




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:309
+  /// Get the lvalue for a parameter.
+  Loc getLValue(const Expr *Call, unsigned Index,
+const LocationContext *LC) const;

Is this used anywhere in this patch?

And why isn't it a `CallExpr` (instead of `Expr`)?



Comment at: 
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp:519
+  return false; // CleanupAttr attaches destructors, which cause escaping.
+  } else {
+const ParmVarDecl *PVD = PR->getDecl();

Again, ParamRegion should derive from VarRegion.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp:439
+VD = cast(VR->getDecl());
+  } else if (const auto *PR =
+ dyn_cast(cast(Sym)->getRegion())) 
{

This would be simpler  (noop?) again if ParamRegion was derived from VarRegion.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp:618
 return std::string(VR->getDecl()->getName());
+  if (const auto *PR = dyn_cast_or_null(MR))
+return std::string(PR->getDecl()->getName());

What if ParamRegion was derived from VarRegion ?



Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:195
+if (Index >= FD->param_size())
+  return nullptr;
+

In all other cases we `assert`, here we return with a nullptr. Why?



Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:915
+return cast(I.getCapturedRegion());
+} else if (const auto *PR = dyn_cast(OrigR)) {
+  if (PR->getDecl() == VD)

Both the `VarRegion` and `ParamRegion` cases here do the same.
This suggest that maybe it would be better to have `VarRegion` as a base class 
for `ParamVarRegion`.
This is aligned to what Artem suggested:
> We can even call it VarRegion and have sub-classes be ParamVarRegion and 
> NonParamVarRegion or something like that.
But maybe `NonParamVarRegion` is not needed since that is actually the 
`VarRegion`.



Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:1230
+
+const TypedValueRegion*
+MemRegionManager::getRegionForParam(const ParmVarDecl *PVD,

Why the return type is not `ParamVarRegion*`?



Comment at: clang/lib/StaticAnalyzer/Core/SymbolManager.cpp:580
+
+bool SymbolReaper::isLive(const ParamRegion *PR,
+  bool includeStoreBindings) const{

This is almost the copy-paste code for isLive(VarRegion). This again suggests 
that maybe ParamRegion (or ParamVarRegion?) should be derived from VarRegion. 
And in isLive(VarRegion) we should dyn_cast to ParamVarRegion and do the extra 
stuff if needed.


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

https://reviews.llvm.org/D79704



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

[PATCH] D80286: [Analyzer] Allow creation of stack frame for functions without definition

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> Retrieving the parameter location of functions was disabled because it may 
> causes crashes due to the fact that functions may have multiple declarations 
> and without definition it is difficult to ensure that always the same 
> declration is used. Now parameters are stored in ParamRegions which are 
> independent of the declaration of the function, therefore the same parameters 
> always have the same regions, independently of the function declaration used 
> actually. This allows us to remove the limitation described above.

I am not sure if this "summary" for the patch is aligned with its title. I 
think the title says it all, I'd just skip this summary, it is way too blurry 
for me. 
Because:

> it may causes crashes

We still need the test cases for that, and I am not sure if we will ever get 
one.

> without definition it is difficult to ensure that always the same declration 
> is used.

That is not difficult in other parts of the compiler infrastructure (e.g in 
ASTImporter), this statement is just a vague allusion to one or several 
hypothetical bugs in the analyzer.

> the same parameters always have the same regions, independently of the 
> function declaration used actually.

You mentioned personally to me already, but don't forget to add test cases for 
that.




Comment at: clang/test/Analysis/temporaries.cpp:893
 glob = 1;
-// FIXME: Why is destructor not inlined in C++17
 clang_analyzer_checkInlined(true);
 #ifdef TEMPORARY_DTORS

baloghadamsoftware wrote:
> I wonder whether `clang_analyzer_checkInlined()` works correctly with this 
> patch: it seems it only checks for stack frame which now any function with 
> definition can have.
So, why not try that in a test with a function that does not have a definition?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80286



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 265216.
MyDeveloperDay marked 2 inline comments as done.
MyDeveloperDay added reviewers: miscco, JakeMerdichAMD.
MyDeveloperDay added a comment.

Add new `AlwaysBreakBeforeConceptDeclarations`

Better handling of requires expressions vs constraint expression

add more concept tests from the twitter-sphere


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

https://reviews.llvm.org/D79773

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13500,6 +13500,7 @@
   CHECK_PARSE_BOOL(AlignConsecutiveBitFields);
   CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
   CHECK_PARSE_BOOL(AlignConsecutiveMacros);
+  CHECK_PARSE_BOOL(AlwaysBreakBeforeConceptDeclarations);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllConstructorInitializersOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
@@ -13520,6 +13521,7 @@
   CHECK_PARSE_BOOL(IndentCaseLabels);
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
+  CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
@@ -16466,6 +16468,219 @@
Style);
 }
 
+TEST_F(FormatTest, ConceptsAndRequires) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
+
+  verifyFormat("template \n"
+   "concept Hashable = requires(T a) {\n"
+   "  { std::hash{}(a) } -> std::convertible_to;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept EqualityComparable = requires(T a, T b) {\n"
+   "  { a == b } -> bool;\n"
+   "  { a != b } -> bool;\n"
+   "};",
+   Style);
+
+  verifyFormat("template \n"
+   "requires Iterator\n"
+   "void sort(It begin, It end) {\n"
+   "  //\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "concept Large = sizeof(T) > 10;",
+   Style);
+
+  verifyFormat("template \n"
+   "concept FooableWith = requires(T t, U u) {\n"
+   "  typename T::foo_type;\n"
+   "  { t.foo(u) } -> typename T::foo_type;\n"
+   "  t++;\n"
+   "};\n"
+   "void doFoo(FooableWith auto t) {\n"
+   "  t.foo(3);\n"
+   "}",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = sizeof(T) == 1;",
+   Style);
+  verifyFormat("template \n"
+   "concept Context = is_specialization_of_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Node = std::is_object_v;",
+   Style);
+  verifyFormat("template \n"
+   "concept Tree = true;",
+   Style);
+
+  verifyFormat("template  int g(T i) requires Concept1 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template  int g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 || "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat("template \n"
+   "veryveryvery_long_return_type g(T i) requires Concept1 && "
+   "Concept2 {\n"
+   "  //...\n"
+   "}",
+   Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 && Concept2 {\n"
+  "  //...\n"
+  "}",
+  Style);
+
+  verifyFormat(
+  "template \n"
+  "veryveryvery_long_return_type g(T i) requires Concept1 || Concept2 {\n"
+  "  //...\n

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 7 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2281
+  if (FormatTok->Tok.is(tok::kw_requires))
+parseRequires();
+}

miscco wrote:
> I believe this should be `parseConstraintExpression`  because that is the 
> term of art in the standard.
> 
> The requires expression is what is the `requieres { }` and that can be part 
> of an constraint expression 
I'm making some of these changes, the problem is with the form

`concept id = Foo && Bar`

in this case the 

`Foo && Bar` 

will be processed by the `parseStructualElement()` and this gets confused with 
the && being a `TT_PointerOrRefernce` and not a `TT_BinaryOperator` and that 
throws everything out.

I think parseStructualElement() is thinking `Foo &&` is part of

`foo(Foo &&abc);`

and not

`Foo && abc`

We need to know we are in a concept expression





Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2314
+  nextToken();
+  if (FormatTok->Tok.is(tok::less)) {
+while (!FormatTok->Tok.is(tok::greater)) {

miscco wrote:
> miscco wrote:
> > I guess you could use `parseBracedList(/*ContinueOnSemicolons=*/false, 
> > /*IsEnum=*/false,/*ClosingBraceKind=*/tok::greater);` here?
> To be more specific, I am concerned what happens if you have multiple 
> template arguments where a linebreak should occur. Is this still happening 
> here?
> 
> 
> ```
> template 
> concept someVeryLongConceptName = 
> someVeryLongConstraintName1;
> ```
This is formatted as

```
template 
concept someVeryLongConceptName =
someVeryLongConstraintName1;
```


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

https://reviews.llvm.org/D79773



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

[clang] 827be69 - [clang] FastMathFlags.allowContract should be initialized only from FPFeatures.allowFPContractAcrossStatement

[Melanie Blower via cfe-commits]

Author: Melanie Blower
Date: 2020-05-20T06:19:10-07:00
New Revision: 827be690dce158924924a70fda79b35a9d7ad1cc

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

LOG: [clang] FastMathFlags.allowContract should be initialized only from 
FPFeatures.allowFPContractAcrossStatement

Summary: Fix bug introduced in D72841 adding support for pragma float_control

Reviewers: rjmccall, Anastasia

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

Added: 


Modified: 
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/constrained-math-builtins.c
clang/test/CodeGen/fp-contract-on-pragma.cpp
clang/test/CodeGen/fp-contract-pragma.cpp
clang/test/CodeGen/fp-floatcontrol-class.cpp
clang/test/CodeGen/fp-floatcontrol-pragma.cpp
clang/test/CodeGen/fp-floatcontrol-stack.cpp
clang/test/CodeGenOpenCL/relaxed-fpmath.cl
clang/test/CodeGenOpenCL/single-precision-constant.cl

Removed: 




diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index 76f58b284eeb..346c429f663e 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -223,8 +223,7 @@ static void updateFastMathFlags(llvm::FastMathFlags &FMF,
   FMF.setNoSignedZeros(FPFeatures.noSignedZeros());
   FMF.setAllowReciprocal(FPFeatures.allowReciprocalMath());
   FMF.setApproxFunc(FPFeatures.allowApproximateFunctions());
-  FMF.setAllowContract(FPFeatures.allowFPContractAcrossStatement() ||
-   FPFeatures.allowFPContractWithinStatement());
+  FMF.setAllowContract(FPFeatures.allowFPContractAcrossStatement());
 }
 
 /// Propagate fast-math flags from \p Op to the instruction in \p V.

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 94ba0dd8e598..b4bc027e832b 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -2944,6 +2944,8 @@ static void ParseLangArgs(LangOptions &Opts, ArgList 
&Args, InputKind IK,
   Opts.NoBitFieldTypeAlign = Args.hasArg(OPT_fno_bitfield_type_align);
   Opts.SinglePrecisionConstants = 
Args.hasArg(OPT_cl_single_precision_constant);
   Opts.FastRelaxedMath = Args.hasArg(OPT_cl_fast_relaxed_math);
+  if (Opts.FastRelaxedMath)
+Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
   Opts.HexagonQdsp6Compat = Args.hasArg(OPT_mqdsp6_compat);
   Opts.FakeAddressSpaceMap = Args.hasArg(OPT_ffake_address_space_map);
   Opts.ParseUnknownAnytype = Args.hasArg(OPT_funknown_anytype);

diff  --git a/clang/test/CodeGen/constrained-math-builtins.c 
b/clang/test/CodeGen/constrained-math-builtins.c
index fe303eb0d5c2..a2feae903d30 100644
--- a/clang/test/CodeGen/constrained-math-builtins.c
+++ b/clang/test/CodeGen/constrained-math-builtins.c
@@ -154,9 +154,9 @@ void bar(float f) {
   (double)f * f - f;
   (long double)-f * f + f;
 
-  // CHECK: call contract float @llvm.experimental.constrained.fmuladd.f32
+  // CHECK: call float @llvm.experimental.constrained.fmuladd.f32
   // CHECK: fneg
-  // CHECK: call contract double @llvm.experimental.constrained.fmuladd.f64
+  // CHECK: call double @llvm.experimental.constrained.fmuladd.f64
   // CHECK: fneg
-  // CHECK: call contract x86_fp80 @llvm.experimental.constrained.fmuladd.f80
+  // CHECK: call x86_fp80 @llvm.experimental.constrained.fmuladd.f80
 };

diff  --git a/clang/test/CodeGen/fp-contract-on-pragma.cpp 
b/clang/test/CodeGen/fp-contract-on-pragma.cpp
index 5f7463608660..812a7176b515 100644
--- a/clang/test/CodeGen/fp-contract-on-pragma.cpp
+++ b/clang/test/CodeGen/fp-contract-on-pragma.cpp
@@ -3,7 +3,7 @@
 // Is FP_CONTRACT honored in a simple case?
 float fp_contract_1(float a, float b, float c) {
 // CHECK: _Z13fp_contract_1fff
-// CHECK: tail call contract float @llvm.fmuladd
+// CHECK: tail call float @llvm.fmuladd
 #pragma clang fp contract(on)
   return a * b + c;
 }
@@ -31,7 +31,7 @@ T template_muladd(T a, T b, T c) {
 
 float fp_contract_3(float a, float b, float c) {
   // CHECK: _Z13fp_contract_3fff
-  // CHECK: tail call contract float @llvm.fmuladd
+  // CHECK: tail call float @llvm.fmuladd
   return template_muladd(a, b, c);
 }
 
@@ -45,13 +45,13 @@ class fp_contract_4 {
 
 template class fp_contract_4;
 // CHECK: _ZN13fp_contract_4IiE6methodEfff
-// CHECK: tail call contract float @llvm.fmuladd
+// CHECK: tail call float @llvm.fmuladd
 
 // Check file-scoped FP_CONTRACT
 #pragma clang fp contract(on)
 float fp_contract_5(float a, float b, float c) {
   // CHECK: _Z13fp_contract_5fff
-  // CHECK: tail call contract float @llvm.fmuladd
+  // CHECK: tail call float @llvm.fmuladd
   return a * b + c;
 }
 
@@ -69,8 +69,8 @@ float fp_contract_6(float a, float b, float c) {
 
 float fp_contract_7(flo

[PATCH] D80251: [X86] Update some av512 shift intrinsics to use "unsigned int" parameter instead of int to match Intel documentaiton

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a comment.

Is it possible to fix those other 5 in the Intel docs for consistency, or is 
there some functional reason that those are different?


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

https://reviews.llvm.org/D80251



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Michael Schellenberger Costa via Phabricator via cfe-commits]

miscco added a comment.

Question: Should I add my wip work as a child revision or what would you suggest


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

https://reviews.llvm.org/D79773



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

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

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

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1194
-  if (!FD)
+  if (!Call.getOriginExpr())
 return;
 

This should be added to avoid later crash (probably not needed for every check 
kind?):
```
  const FunctionDecl *FD = C.getCalleeDecl(CE);
  if (!FD)
return;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75432



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

[PATCH] D80286: [Analyzer] Allow creation of stack frame for functions without definition

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

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

Added unittest for checking the sameness for the regions of same params with 
different redeclarations.


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

https://reviews.llvm.org/D80286

Files:
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/explain-svals.cpp
  clang/test/Analysis/temporaries.cpp
  clang/unittests/StaticAnalyzer/ParamRegionTest.cpp

Index: clang/unittests/StaticAnalyzer/ParamRegionTest.cpp
===
--- clang/unittests/StaticAnalyzer/ParamRegionTest.cpp
+++ clang/unittests/StaticAnalyzer/ParamRegionTest.cpp
@@ -16,6 +16,16 @@
 namespace {
 
 class ParamRegionTestConsumer : public ExprEngineConsumer {
+  void checkForSameParamRegions(MemRegionManager &MRMgr,
+const StackFrameContext *SFC,
+const ParmVarDecl *PVD) {
+for (const auto *D2: PVD->redecls()) {
+  const auto *PVD2 = cast(D2);
+  assert(MRMgr.getRegionForParam(PVD, SFC) ==
+ MRMgr.getRegionForParam(PVD2, SFC));
+}
+  }
+
   void performTest(const Decl *D) {
 StoreManager &StMgr = Eng.getStoreManager();
 MemRegionManager &MRMgr = StMgr.getRegionManager();
@@ -29,6 +39,7 @@
   assert(isa(Reg));
 else
   assert(isa(Reg));
+checkForSameParamRegions(MRMgr, SFC, P);
   }
 } else if (const auto *CD = dyn_cast(D)) {
   for (const auto *P : CD->parameters()) {
@@ -37,6 +48,7 @@
   assert(isa(Reg));
 else
   assert(isa(Reg));
+checkForSameParamRegions(MRMgr, SFC, P);
   }
 } else if (const auto *MD = dyn_cast(D)) {
   for (const auto *P : MD->parameters()) {
@@ -45,6 +57,7 @@
   assert(isa(Reg));
 else
   assert(isa(Reg));
+checkForSameParamRegions(MRMgr, SFC, P);
   }
 }
   }
@@ -71,6 +84,9 @@
 TEST(ParamRegion, ParamRegionTest) {
   EXPECT_TRUE(tooling::runToolOnCode(
   std::make_unique(),
+  "void foo(int n); "
+  "void bar(int l); "
+  "void baz(int p); "
   "void foo(int n) { "
   "auto lambda = [n](int m) { return n + m; }; "
   "int k = lambda(2); } "
Index: clang/test/Analysis/temporaries.cpp
===
--- clang/test/Analysis/temporaries.cpp
+++ clang/test/Analysis/temporaries.cpp
@@ -890,12 +890,9 @@
 public:
   ~C() {
 glob = 1;
-// FIXME: Why is destructor not inlined in C++17
 clang_analyzer_checkInlined(true);
 #ifdef TEMPORARY_DTORS
-#if __cplusplus < 201703L
-// expected-warning@-3{{TRUE}}
-#endif
+// expected-warning@-2{{TRUE}}
 #endif
   }
 };
@@ -914,16 +911,11 @@
   // temporaries returned from functions, so we took the wrong branch.
   coin && is(get()); // no-crash
   if (coin) {
-// FIXME: Why is destructor not inlined in C++17
 clang_analyzer_eval(glob);
 #ifdef TEMPORARY_DTORS
-#if __cplusplus < 201703L
-// expected-warning@-3{{TRUE}}
-#else
-// expected-warning@-5{{UNKNOWN}}
-#endif
+// expected-warning@-2{{TRUE}}
 #else
-// expected-warning@-8{{UNKNOWN}}
+// expected-warning@-4{{UNKNOWN}}
 #endif
   } else {
 // The destructor is not called on this branch.
Index: clang/test/Analysis/explain-svals.cpp
===
--- clang/test/Analysis/explain-svals.cpp
+++ clang/test/Analysis/explain-svals.cpp
@@ -94,7 +94,7 @@
 } // end of anonymous namespace
 
 void test_6() {
-  clang_analyzer_explain(conjure_S()); // expected-warning-re^lazily frozen compound value of temporary object constructed at statement 'conjure_S\(\)'$
+  clang_analyzer_explain(conjure_S()); // expected-warning-re^lazily frozen compound value of parameter 0 of function 'clang_analyzer_explain\(\)'$
   clang_analyzer_explain(conjure_S().z); // expected-warning-re^value derived from \(symbol of type 'int' conjured at statement 'conjure_S\(\)'\) for field 'z' of temporary object constructed at statement 'conjure_S\(\)'$
 }
 
Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -172,23 +172,9 @@
   if (!D)
 return nullptr;
 
-  // TODO: For now we skip functions without definitions, even if we have
-  // our own getDecl(), because it's hard to find out which re-declaration
-  // is going to be used, and usually clients don't really care about this
-  // situation because there's a loss of precision anyway because we cannot
-  // inline the call.
-  RuntimeDefinition RD = getRuntimeDefinition();
-  if (!RD.getDecl())
-return nullptr;
-
   AnalysisDeclContext *ADC =
   LCtx->getA

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin added a comment.

@mgehre @yvvan it seems that the issue still not fixed and '\n' gets duplicated 
in the replacements. Are you going to fix this issue or I should create a patch 
to fix it?
Before this change '\n' was actually processed correctly `ReplacementText: 
'#include \n\n'` is actually replacement text with two new lines in 
standard YAML deserialiser.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63482



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

[PATCH] D79903: FastMathFlags.allowContract should be init from FPFeatures.allowFPContractAcrossStatement

[Melanie Blower via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG827be690dce1: [clang] FastMathFlags.allowContract should be 
initialized only from FPFeatures. (authored by mibintc).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79903

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/constrained-math-builtins.c
  clang/test/CodeGen/fp-contract-on-pragma.cpp
  clang/test/CodeGen/fp-contract-pragma.cpp
  clang/test/CodeGen/fp-floatcontrol-class.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/CodeGen/fp-floatcontrol-stack.cpp
  clang/test/CodeGenOpenCL/relaxed-fpmath.cl
  clang/test/CodeGenOpenCL/single-precision-constant.cl

Index: clang/test/CodeGenOpenCL/single-precision-constant.cl
===
--- clang/test/CodeGenOpenCL/single-precision-constant.cl
+++ clang/test/CodeGenOpenCL/single-precision-constant.cl
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 %s -cl-single-precision-constant -emit-llvm -o - | FileCheck %s
 
 float fn(float f) {
-  // CHECK: tail call contract float @llvm.fmuladd.f32(float %f, float 2.00e+00, float 1.00e+00)
+  // CHECK: tail call float @llvm.fmuladd.f32(float %f, float 2.00e+00, float 1.00e+00)
   return f*2. + 1.;
 }
Index: clang/test/CodeGenOpenCL/relaxed-fpmath.cl
===
--- clang/test/CodeGenOpenCL/relaxed-fpmath.cl
+++ clang/test/CodeGenOpenCL/relaxed-fpmath.cl
@@ -8,12 +8,12 @@
 float spscalardiv(float a, float b) {
   // CHECK: @spscalardiv(
 
-  // NORMAL: fdiv contract float
+  // NORMAL: fdiv float
   // FAST: fdiv fast float
-  // FINITE: fdiv nnan ninf contract float
-  // UNSAFE: fdiv nnan nsz contract float
-  // MAD: fdiv contract float
-  // NOSIGNED: fdiv nsz contract float
+  // FINITE: fdiv nnan ninf float
+  // UNSAFE: fdiv nnan nsz float
+  // MAD: fdiv float
+  // NOSIGNED: fdiv nsz float
   return a / b;
 }
 // CHECK: attributes
Index: clang/test/CodeGen/fp-floatcontrol-stack.cpp
===
--- clang/test/CodeGen/fp-floatcontrol-stack.cpp
+++ clang/test/CodeGen/fp-floatcontrol-stack.cpp
@@ -9,7 +9,7 @@
 float fun_default FUN(1)
 //CHECK-LABEL: define {{.*}} @_Z11fun_defaultf{{.*}}
 #if DEFAULT
-//CHECK-DDEFAULT: call contract float @llvm.fmuladd{{.*}}
+//CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
 #endif
 #if EBSTRICT
 // Note that backend wants constrained intrinsics used
@@ -37,7 +37,7 @@
 //CHECK-DEBSTRICT: llvm.experimental.constrained.fmuladd{{.*}}tonearest{{.*}}strict
 #endif
 #if NOHONOR
-//CHECK-NOHONOR: nnan ninf contract float {{.*}}llvm.experimental.constrained.fmuladd{{.*}}tonearest{{.*}}strict
+//CHECK-NOHONOR: nnan ninf float {{.*}}llvm.experimental.constrained.fmuladd{{.*}}tonearest{{.*}}strict
 #endif
 #if FAST
 //Not possible to enable float_control(except) in FAST mode.
@@ -49,13 +49,13 @@
 float exc_pop FUN(5)
 //CHECK-LABEL: define {{.*}} @_Z7exc_popf{{.*}}
 #if DEFAULT
-//CHECK-DDEFAULT: call contract float @llvm.fmuladd{{.*}}
+//CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
 #endif
 #if EBSTRICT
 //CHECK-DEBSTRICT: llvm.experimental.constrained.fmuladd{{.*}}tonearest{{.*}}strict
 #endif
 #if NOHONOR
-//CHECK-NOHONOR: call nnan ninf contract float @llvm.fmuladd{{.*}}
+//CHECK-NOHONOR: call nnan ninf float @llvm.fmuladd{{.*}}
 #endif
 #if FAST
 //CHECK-FAST: fmul fast float
@@ -66,13 +66,13 @@
 float exc_off FUN(5)
 //CHECK-LABEL: define {{.*}} @_Z7exc_offf{{.*}}
 #if DEFAULT
-//CHECK-DDEFAULT: call contract float @llvm.fmuladd{{.*}}
+//CHECK-DDEFAULT: call float @llvm.fmuladd{{.*}}
 #endif
 #if EBSTRICT
-//CHECK-DEBSTRICT: call contract float @llvm.fmuladd{{.*}}
+//CHECK-DEBSTRICT: call float @llvm.fmuladd{{.*}}
 #endif
 #if NOHONOR
-//CHECK-NOHONOR: call nnan ninf contract float @llvm.fmuladd{{.*}}
+//CHECK-NOHONOR: call nnan ninf float @llvm.fmuladd{{.*}}
 #endif
 #if FAST
 //CHECK-FAST: fmul fast float
@@ -83,30 +83,30 @@
 float precise_on FUN(3)
 //CHECK-LABEL: define {{.*}} @_Z10precise_onf{{.*}}
 #if DEFAULT
-//CHECK-DDEFAULT: contract float {{.*}}llvm.fmuladd{{.*}}
+//CHECK-DDEFAULT: float {{.*}}llvm.fmuladd{{.*}}
 #endif
 #if EBSTRICT
-//CHECK-DEBSTRICT: contract float {{.*}}llvm.fmuladd{{.*}}
+//CHECK-DEBSTRICT: float {{.*}}llvm.fmuladd{{.*}}
 #endif
 #if NOHONOR
 // If precise is pushed then all fast-math should be off!
-//CHECK-NOHONOR: call contract float {{.*}}llvm.fmuladd{{.*}}
+//CHECK-NOHONOR: call float {{.*}}llvm.fmuladd{{.*}}
 #endif
 #if FAST
-//CHECK-FAST: contract float {{.*}}llvm.fmuladd{{.*}}
+//CHECK-FAST: float {{.*}}llvm.fmuladd{{.*}}
 #endif
 
 #pragma float_control(pop)
 float precise_pop FUN(3)
 //CHECK-LABEL: define {{.*}} @_Z11precise_popf{{.*}}
 #if DEFAULT
-//CHECK-DDEFAULT: con

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

I guess alignment is one of the main users of the old format, great to see it 
go :)

First set of comments.




Comment at: llvm/lib/Analysis/AssumeBundleQueries.cpp:104
+return 1;
+  };
   if (BOI.End - BOI.Begin > ABA_Argument)

I think this is a problem for things like `deref` which should default to 0?



Comment at: llvm/lib/Analysis/AssumeBundleQueries.cpp:110
+if (BOI.End - BOI.Begin > ABA_Argument + 1)
+  Result.ArgValue = MinAlign(Result.ArgValue, GetArgOr1(1));
   return Result;

Is this the min of the alignment and offset? If so, I'm not sure about min. 
Either way, can you clang format this and add a comment why alignment is 
special and how it looks?



Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:4196
   LHS->setMetadata(LLVMContext::MD_nonnull, MD);
-  return eraseInstFromFunction(*II);
+  if (!HasOpBundles)
+return eraseInstFromFunction(*II);

Can we split these changes off. Seem unrelated and easier.



Comment at: llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp:273
+  if (!isValidAssumeForContext(ACall, J, DT))
+continue;
   Align NewDestAlignment =

I'm curious, why was this needed? 



Comment at: llvm/test/Transforms/InstCombine/assume.ll:380
 ; CHECK-NEXT:[[CMP2:%.*]] = icmp ne i8 [[X:%.*]], 0
+; CHECK-NEXT:tail call void @llvm.assume(i1 false)
 ; CHECK-NEXT:tail call void @llvm.dbg.value(metadata i32 5, metadata !7, 
metadata !DIExpression()), !dbg !9

Can you add the "beginning" of the operand bundles here so it becomes clear we 
do not have a no-op assume.


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

https://reviews.llvm.org/D71739



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

[PATCH] D80294: Add support for vmsumudm

[Ahsan Saghir via Phabricator via cfe-commits]

saghir created this revision.
saghir added reviewers: power-llvm-team, nemanjai, stefanp, amyk, lei, hfinkel, 
PowerPC.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

This patch adds support for Vector Multiply-Sum Unsigned Doubleword Modulo 
instruction; vmsumudm. This instruction is available on POWER 9.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80294

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/P9InstrResources.td
  llvm/lib/Target/PowerPC/PPCInstrAltivec.td
  llvm/test/MC/Disassembler/PowerPC/ppc64-encoding-vmx.txt
  llvm/test/MC/PowerPC/ppc64-encoding-vmx.s


Index: llvm/test/MC/PowerPC/ppc64-encoding-vmx.s
===
--- llvm/test/MC/PowerPC/ppc64-encoding-vmx.s
+++ llvm/test/MC/PowerPC/ppc64-encoding-vmx.s
@@ -366,6 +366,9 @@
 # CHECK-BE: vmsumuhm 2, 3, 4, 5 # encoding: [0x10,0x43,0x21,0x66]
 # CHECK-LE: vmsumuhm 2, 3, 4, 5 # encoding: [0x66,0x21,0x43,0x10]
 vmsumuhm 2, 3, 4, 5
+# CHECK-BE: vmsumudm 2, 3, 4, 5 # encoding: [0x10,0x43,0x21,0x63]
+# CHECK-LE: vmsumudm 2, 3, 4, 5 # encoding: [0x63,0x21,0x43,0x10]
+vmsumudm 2, 3, 4, 5
 # CHECK-BE: vmsumuhs 2, 3, 4, 5 # encoding: [0x10,0x43,0x21,0x67]
 # CHECK-LE: vmsumuhs 2, 3, 4, 5 # encoding: [0x67,0x21,0x43,0x10]
 vmsumuhs 2, 3, 4, 5
Index: llvm/test/MC/Disassembler/PowerPC/ppc64-encoding-vmx.txt
===
--- llvm/test/MC/Disassembler/PowerPC/ppc64-encoding-vmx.txt
+++ llvm/test/MC/Disassembler/PowerPC/ppc64-encoding-vmx.txt
@@ -333,6 +333,9 @@
 # CHECK: vmsumuhm 2, 3, 4, 5 
 0x10 0x43 0x21 0x66
 
+# CHECK: vmsumudm 2, 3, 4, 5
+0x10 0x43 0x21 0x63
+
 # CHECK: vmsumuhs 2, 3, 4, 5 
 0x10 0x43 0x21 0x67
 
Index: llvm/lib/Target/PowerPC/PPCInstrAltivec.td
===
--- llvm/lib/Target/PowerPC/PPCInstrAltivec.td
+++ llvm/lib/Target/PowerPC/PPCInstrAltivec.td
@@ -614,6 +614,8 @@
 v4i32, v16i8, v4i32>;
 def VMSUMUHM : VA1a_Int_Ty3<38, "vmsumuhm", int_ppc_altivec_vmsumuhm,
 v4i32, v8i16, v4i32>;
+def VMSUMUDM : VA1a_Int_Ty3<35, "vmsumudm", int_ppc_altivec_vmsumudm,
+v1i128, v2i64, v1i128>;
 def VMSUMUHS : VA1a_Int_Ty3<39, "vmsumuhs", int_ppc_altivec_vmsumuhs,
 v4i32, v8i16, v4i32>;
 
Index: llvm/lib/Target/PowerPC/P9InstrResources.td
===
--- llvm/lib/Target/PowerPC/P9InstrResources.td
+++ llvm/lib/Target/PowerPC/P9InstrResources.td
@@ -373,6 +373,7 @@
 VMSUMSHS,
 VMSUMUBM,
 VMSUMUHM,
+VMSUMUDM,
 VMSUMUHS,
 VMULESB,
 VMULESH,
Index: llvm/include/llvm/IR/IntrinsicsPowerPC.td
===
--- llvm/include/llvm/IR/IntrinsicsPowerPC.td
+++ llvm/include/llvm/IR/IntrinsicsPowerPC.td
@@ -574,6 +574,9 @@
   def int_ppc_altivec_vmsumuhm : GCCBuiltin<"__builtin_altivec_vmsumuhm">,
 Intrinsic<[llvm_v4i32_ty], [llvm_v8i16_ty, llvm_v8i16_ty,
llvm_v4i32_ty], [IntrNoMem]>;
+  def int_ppc_altivec_vmsumudm : GCCBuiltin<"__builtin_altivec_vmsumudm">,
+Intrinsic<[llvm_v1i128_ty], [llvm_v2i64_ty, llvm_v2i64_ty,
+   llvm_v1i128_ty], [IntrNoMem]>;
   def int_ppc_altivec_vmsumuhs : GCCBuiltin<"__builtin_altivec_vmsumuhs">,
 Intrinsic<[llvm_v4i32_ty], [llvm_v8i16_ty, llvm_v8i16_ty,
llvm_v4i32_ty], [IntrNoMem]>;
Index: clang/include/clang/Basic/BuiltinsPPC.def
===
--- clang/include/clang/Basic/BuiltinsPPC.def
+++ clang/include/clang/Basic/BuiltinsPPC.def
@@ -84,6 +84,7 @@
 BUILTIN(__builtin_altivec_vmsumubm, "V4UiV16UcV16UcV4Ui", "")
 BUILTIN(__builtin_altivec_vmsummbm, "V4SiV16ScV16UcV4Si", "")
 BUILTIN(__builtin_altivec_vmsumuhm, "V4UiV8UsV8UsV4Ui", "")
+BUILTIN(__builtin_altivec_vmsumudm, "V1ULLLiV2ULLV2ULLV1ULLLi", "")
 BUILTIN(__builtin_altivec_vmsumshm, "V4SiV8SsV8SsV4Si", "")
 BUILTIN(__builtin_altivec_vmsumuhs, "V4UiV8UsV8UsV4Ui", "")
 BUILTIN(__builtin_altivec_vmsumshs, "V4SiV8SsV8SsV4Si", "")


Index: llvm/test/MC/PowerPC/ppc64-encoding-vmx.s
===
--- llvm/test/MC/PowerPC/ppc64-encoding-vmx.s
+++ llvm/test/MC/PowerPC/ppc64-encoding-vmx.s
@@ -366,6 +366,9 @@
 # CHECK-BE: vmsumuhm 2, 3, 4, 5 # encoding: [0x10,0x43,0x21,0x66]
 # CHECK-LE: vmsumuhm 2, 3, 4, 5 # encoding: [0x66,0x21,0x43,0x10]
 vmsumuhm 2, 3, 4, 5
+# CHECK-BE: vmsumudm 2, 3, 4, 5 # encoding: [0x10,0x43,0x21,0x63]
+# CHECK-LE: vmsumu

[PATCH] D78097: [analyzer][RetainCount] Remove the CheckOSObject option

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

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

I have no authority whatsoever, but if there is no breakage in buildbots, then 
it means explicitly providing `CheckOSObject` param is not really valid 
use-case. I vote for this cleanup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78097



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

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

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

Szelethus marked 3 inline comments as done.
Szelethus added a comment.

I though I addressed the inlines months ago, but seems like I did not. I'll get 
this done post-commit. Oops.




Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1194
-  if (!FD)
+  if (!Call.getOriginExpr())
 return;
 

balazske wrote:
> This should be added to avoid later crash (probably not needed for every 
> check kind?):
> ```
>   const FunctionDecl *FD = C.getCalleeDecl(CE);
>   if (!FD)
> return;
> ```
Not all `CallEvent`s have a corresponding `FunctionDecl` or a `CallExpr`, for 
instance, `CXXAllocatorCall` corresponds with `CXXNewExpr`, which is not a 
`CallExpr`, but it is handled by this checker. For this reason, I decided to 
move this check to the individual modeling functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75432



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

[PATCH] D79704: [Analyzer] [NFC] Parameter Regions

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

baloghadamsoftware marked 7 inline comments as done.
baloghadamsoftware added a comment.

In D79704#2042130 , @martong wrote:

> Can we find the FunctionDecl if the call happens through a function pointer? 
> Or in that case do we actually find the VarDecl of the function pointer?


Yes, if it has a declaration, then we can retrieve it from the stack frame.

> So, in this patch, we are trying to solve only that problem when the callee 
> `FunctionDecl` does not have a definition?

Not even that. This  patch is just a non-functional change that prepares the 
ground for such improvements. The next patch intends to solve them problem 
where the function has declaration but no definition.

> This happens in case of top-level params and in case of function pointers, 
> plus the special argument construction, am I right?

Top-level is a special case: there we do not have `Expr` either so we cannot 
handle it. For top-level functions the regions remain `VarRegions`. There is 
nothing special in parameters when analyzed top-level.

> Based on the answers to the above questions, possibly we are trying to solve 
> two problems here. Could we split then this patch into two?
> 
> 1. callee FunctionDecl does not have a definition. This could assert on 
> having an accessable Decl in ParamVarRegion. Here we could have the 
> foundations of the next patch, i.e. we could have ParamVarRegion and 
> NonParamVarRegion.
> 2. function pointers, plus the special argument construction: ParamVarRegion 
> may not have a Decl and we'd remove the related lines from 
> getCalleeAnalysisDeclContext. This obviously requires more tests. And more 
> brainstorming to get the AnalysisDeclContext when there is no Decl available 
> for the function in getCalleeAnalysisDeclContext.

That is exactly my suggestion, but not this, current patch, but the next one. 
The next patch is for functions without definition but declarations, and there 
could be another one in the future for functions without declaration. Of course 
this latter requires some extra code into `ParamRegion`s to handle cases where 
we do not have a `Decl`. I cannot write it now because I cannot make a test for 
it, not even a unit test.

In D79704#2046495 , @martong wrote:

> Seems like many changes could be simplified or absolutely not needed in this 
> patch if ParamRegion (or with a better name ParamVarRegion) was derived from 
> VarRegion. Is there any difficulties in that derivation?


It is one of my suggestions, I am still waiting for @NoQ's opinion: let us 
remove the `Decl` from `DeclRegion` and make `getDecl()` pure virtual there. 
Also not implement it in `VarRegion`, but as @NoQ suggested, create something 
like `PlainVarRegion` or `NonParamVarRegion` where we store it, and 
`ParamVarRegion` where we retrieve it instead of storing it.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:309
+  /// Get the lvalue for a parameter.
+  Loc getLValue(const Expr *Call, unsigned Index,
+const LocationContext *LC) const;

martong wrote:
> Is this used anywhere in this patch?
> 
> And why isn't it a `CallExpr` (instead of `Expr`)?
The origin expression of a call may be a set of different kinds of expressions: 
`CallExpr`, `CXXConstructExpr`, `BlockExpr` or `ObjCMessageExpr`.



Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:195
+if (Index >= FD->param_size())
+  return nullptr;
+

martong wrote:
> In all other cases we `assert`, here we return with a nullptr. Why?
It was accidentally left here from an earlier experiment.



Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:915
+return cast(I.getCapturedRegion());
+} else if (const auto *PR = dyn_cast(OrigR)) {
+  if (PR->getDecl() == VD)

martong wrote:
> Both the `VarRegion` and `ParamRegion` cases here do the same.
> This suggest that maybe it would be better to have `VarRegion` as a base 
> class for `ParamVarRegion`.
> This is aligned to what Artem suggested:
> > We can even call it VarRegion and have sub-classes be ParamVarRegion and 
> > NonParamVarRegion or something like that.
> But maybe `NonParamVarRegion` is not needed since that is actually the 
> `VarRegion`.
Usually we do not inherit from concrete classes by changing a method 
implementation that already exist. The other reason is even stronger: 
`NonParamVarRegion` //must// store the `Decl` of the variable, `ParamVarRegion` 
//must not//.



Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:1230
+
+const TypedValueRegion*
+MemRegionManager::getRegionForParam(const ParmVarDecl *PVD,

martong wrote:
> Why the return type is not `ParamVarRegion*`?
Because for top-level functions and captured parameters it still returns 
`VarRegion`.



C

[PATCH] D79992: [clangd] Patch PP directives to use stale preambles while building ASTs

[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/Preamble.cpp:110
 
-/// Gets the includes in the preamble section of the file by running
-/// preprocessor over \p Contents. Returned includes do not contain resolved
-/// paths. \p VFS and \p Cmd is used to build the compiler invocation, which
-/// might stat/read files.
-llvm::Expected>
-scanPreambleIncludes(llvm::StringRef Contents,
- llvm::IntrusiveRefCntPtr VFS,
- const tooling::CompileCommand &Cmd) {
+struct TextualPPDirective {
+  unsigned DirectiveLine;

this is worth a comment saying what this is used for: directives *other* than 
includes where we need only basic structure.



Comment at: clang-tools-extra/clangd/unittests/PreambleTests.cpp:205
+
+TEST(PreamblePatchTest, Define) {
+  // BAR should be defined while parsing the AST.

kadircet wrote:
> sammccall wrote:
> > do you think it makes sense to have a test that just asserts on the 
> > contents of the preamble patch? it seems like a more direct way to test 
> > some of these things.
> > 
> > These tests are nice, but debugging them seems like it might be a bit of 
> > work.
> I had 2 reasons for not doing that:
> - Not clobbering the API of `PreamblePatch` just for testing.
> - Making tests less fragile for changes in the patch format.
> 
> I suppose the first one is not that important, but I believe the latter is 
> quite useful. We can always to something like hassubstr I suppose, but in the 
> end we only care about its effect while creating an AST. We can do something 
> like hassubstr, but we would still need these tests to ensure they interact 
> as expected with the preprocessor.
Agree that having robust tests of the whole thing is important.

Maybe we could add an additional (sorry) single smoke test that asserts on the 
whole patch?
It'd be brittle indeed but those diffs would actually be really interesting to 
me as a reader of the code and reviewer of changes... gives a different 
perspective on what the code is doing.

I don't think having a `text()` or `textForTests()` accessor is too bad, 
especially if it's trivial.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79992



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

[PATCH] D80295: [Sema] Diagnose static data members in classes nested in unnamed classes

[John Brawn via Phabricator via cfe-commits]

john.brawn created this revision.
john.brawn added reviewers: rsmith, rjmccall.
Herald added a project: clang.

We currently diagnose static data members directly contained in unnamed 
classes, but we should also diagnose when they're in a class that is nested 
(directly or indirectly) in an unnamed class. Do this by iterating up the list 
of parent DeclContexts and checking if any is an unnamed class.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80295

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/anonymous-struct.cpp


Index: clang/test/SemaCXX/anonymous-struct.cpp
===
--- clang/test/SemaCXX/anonymous-struct.cpp
+++ clang/test/SemaCXX/anonymous-struct.cpp
@@ -153,3 +153,21 @@
   const Empty E;
 } C;
 } // namespace ImplicitDecls
+
+struct {
+  static int x; // expected-error {{static data member 'x' not allowed in 
anonymous struct}}
+} static_member_1;
+
+struct {
+  struct A {
+static int x; // expected-error {{static data member 'x' not allowed in 
anonymous struct}}
+  } x;
+} static_member_2;
+
+struct {
+  struct A {
+struct B {
+  static int x; // expected-error {{static data member 'x' not allowed in 
anonymous struct}}
+} x;
+  } x;
+} static_member_3;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6885,19 +6885,12 @@
 
 if (SC == SC_Static && CurContext->isRecord()) {
   if (const CXXRecordDecl *RD = dyn_cast(DC)) {
-// C++ [class.static.data]p2:
-//   A static data member shall not be a direct member of an unnamed
-//   or local class
-// FIXME: or of a (possibly indirectly) nested class thereof.
+// C++ [class.static.data]p5: A local class shall not have static data
+// members.
 if (RD->isLocalClass()) {
   Diag(D.getIdentifierLoc(),
diag::err_static_data_member_not_allowed_in_local_class)
 << Name << RD->getDeclName() << RD->getTagKind();
-} else if (!RD->getDeclName()) {
-  Diag(D.getIdentifierLoc(),
-   diag::err_static_data_member_not_allowed_in_anon_struct)
-<< Name << RD->getTagKind();
-  Invalid = true;
 } else if (RD->isUnion()) {
   // C++98 [class.union]p1: If a union contains a static data member,
   // the program is ill-formed. C++11 drops this restriction.
@@ -6905,6 +6898,21 @@
getLangOpts().CPlusPlus11
  ? diag::warn_cxx98_compat_static_data_member_in_union
  : diag::ext_static_data_member_in_union) << Name;
+} else {
+  // C++ [class.static.data]p4: Unnamed classes and classes contained
+  // directly or indirectly within unnamed classes shall not contain
+  // static data members.
+  for (DeclContext *ParentContext = DC;
+   RD = dyn_cast(ParentContext);
+   ParentContext = ParentContext->getParent()) {
+if (!RD->getDeclName()) {
+  Diag(D.getIdentifierLoc(),
+   diag::err_static_data_member_not_allowed_in_anon_struct)
+  << Name << RD->getTagKind();
+  Invalid = true;
+  break;
+}
+  }
 }
   }
 }


Index: clang/test/SemaCXX/anonymous-struct.cpp
===
--- clang/test/SemaCXX/anonymous-struct.cpp
+++ clang/test/SemaCXX/anonymous-struct.cpp
@@ -153,3 +153,21 @@
   const Empty E;
 } C;
 } // namespace ImplicitDecls
+
+struct {
+  static int x; // expected-error {{static data member 'x' not allowed in anonymous struct}}
+} static_member_1;
+
+struct {
+  struct A {
+static int x; // expected-error {{static data member 'x' not allowed in anonymous struct}}
+  } x;
+} static_member_2;
+
+struct {
+  struct A {
+struct B {
+  static int x; // expected-error {{static data member 'x' not allowed in anonymous struct}}
+} x;
+  } x;
+} static_member_3;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6885,19 +6885,12 @@
 
 if (SC == SC_Static && CurContext->isRecord()) {
   if (const CXXRecordDecl *RD = dyn_cast(DC)) {
-// C++ [class.static.data]p2:
-//   A static data member shall not be a direct member of an unnamed
-//   or local class
-// FIXME: or of a (possibly indirectly) nested class thereof.
+// C++ [class.static.data]p5: A local class shall not have static data
+// members.
 if (RD->isLocalClass()) {
   Diag(D.getIdentifierLoc(),
diag::err_static_data_member_not_allowed_in_local_class)
 << Name << RD->getDeclName() << RD->getTagKind();
-} else if (

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

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

Szelethus marked 4 inline comments as done.
Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:614
+  static ProgramStateRef CallocMem(CheckerContext &C, const CallEvent &Call,
ProgramStateRef State);
 

NoQ wrote:
> Is the state parameter still necessary here?
Not as per its current usages in the code, but it does make sense, if we do 
some prechecks and modify the state before calling this functions.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1194
-  if (!FD)
+  if (!Call.getOriginExpr())
 return;
 

Szelethus wrote:
> balazske wrote:
> > This should be added to avoid later crash (probably not needed for every 
> > check kind?):
> > ```
> >   const FunctionDecl *FD = C.getCalleeDecl(CE);
> >   if (!FD)
> > return;
> > ```
> Not all `CallEvent`s have a corresponding `FunctionDecl` or a `CallExpr`, for 
> instance, `CXXAllocatorCall` corresponds with `CXXNewExpr`, which is not a 
> `CallExpr`, but it is handled by this checker. For this reason, I decided to 
> move this check to the individual modeling functions.
Oh I'm sorry, do we have an actual crash resulting from this? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75432



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

[PATCH] D76342: [OpenMP] Implement '#pragma omp tile'

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.
Herald added a subscriber: sstefan1.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76342



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

[PATCH] D80296: [clangd] Don't traverse the AST within uninteresting files during indexing

[Sam McCall via Phabricator via cfe-commits]

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

We already skip function bodies from these files while parsing, and drop symbols
found in them. However, traversing their ASTs still takes a substantial amount
of time.

Non-scientific benchmark on my machine:

  background-indexing llvm-project (llvm+clang+clang-tools-extra), wall time
  before: 7:46
  after: 5:13
  change: -33%

Indexer.cpp libclang should be updated too, I'm less familiar with that code,
and it's doing tricky things with the ShouldSkipFunctionBody callback, so it
needs to be done separately.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80296

Files:
  clang-tools-extra/clangd/index/IndexAction.cpp
  clang-tools-extra/clangd/unittests/IndexActionTests.cpp
  clang/include/clang/Index/IndexingAction.h
  clang/include/clang/Index/IndexingOptions.h
  clang/lib/Index/IndexDecl.cpp
  clang/lib/Index/IndexingAction.cpp

Index: clang/lib/Index/IndexingAction.cpp
===
--- clang/lib/Index/IndexingAction.cpp
+++ clang/lib/Index/IndexingAction.cpp
@@ -131,6 +131,21 @@
 ShouldSkipFunctionBody);
 }
 
+std::unique_ptr clang::index::createIndexingASTConsumer(
+std::shared_ptr DataConsumer,
+const IndexingOptions &Opts, std::shared_ptr PP) {
+  std::function ShouldSkipFunctionBody = [](const Decl *) {
+return false;
+  };
+  if (Opts.ShouldTraverseDecl)
+ShouldSkipFunctionBody =
+[ShouldTraverseDecl(Opts.ShouldTraverseDecl)](const Decl *D) {
+  return !ShouldTraverseDecl(D);
+};
+  return createIndexingASTConsumer(std::move(DataConsumer), Opts, std::move(PP),
+   std::move(ShouldSkipFunctionBody));
+}
+
 std::unique_ptr
 index::createIndexingAction(std::shared_ptr DataConsumer,
 const IndexingOptions &Opts) {
Index: clang/lib/Index/IndexDecl.cpp
===
--- clang/lib/Index/IndexDecl.cpp
+++ clang/lib/Index/IndexDecl.cpp
@@ -765,6 +765,9 @@
   if (isa(D))
 return true; // Wait for the objc container.
 
+  if (IndexOpts.ShouldTraverseDecl && !IndexOpts.ShouldTraverseDecl(D))
+return true; // skip
+
   return indexDecl(D);
 }
 
Index: clang/include/clang/Index/IndexingOptions.h
===
--- clang/include/clang/Index/IndexingOptions.h
+++ clang/include/clang/Index/IndexingOptions.h
@@ -34,6 +34,12 @@
   // Has no effect if IndexFunctionLocals are false.
   bool IndexParametersInDeclarations = false;
   bool IndexTemplateParameters = false;
+
+  // If set, skip indexing inside some declarations for performance.
+  // This prevents traversal, so skipping a struct means its declaration an
+  // members won't be indexed, but references that struct elsewhere will be.
+  // Currently this is only checked for top-level declarations.
+  std::function ShouldTraverseDecl;
 };
 
 } // namespace index
Index: clang/include/clang/Index/IndexingAction.h
===
--- clang/include/clang/Index/IndexingAction.h
+++ clang/include/clang/Index/IndexingAction.h
@@ -30,22 +30,21 @@
 }
 
 namespace index {
-  class IndexDataConsumer;
+class IndexDataConsumer;
 
 /// Creates an ASTConsumer that indexes all symbols (macros and AST decls).
+std::unique_ptr
+createIndexingASTConsumer(std::shared_ptr DataConsumer,
+  const IndexingOptions &Opts,
+  std::shared_ptr PP);
+
 std::unique_ptr createIndexingASTConsumer(
 std::shared_ptr DataConsumer,
 const IndexingOptions &Opts, std::shared_ptr PP,
+// Prefer to set Opts.ShouldTraverseDecl and use the above overload.
+// This version is only needed if used to *track* function body parsing.
 std::function ShouldSkipFunctionBody);
 
-inline std::unique_ptr createIndexingASTConsumer(
-std::shared_ptr DataConsumer,
-const IndexingOptions &Opts, std::shared_ptr PP) {
-  return createIndexingASTConsumer(
-  std::move(DataConsumer), Opts, std::move(PP),
-  /*ShouldSkipFunctionBody=*/[](const Decl *) { return false; });
-}
-
 /// Creates a frontend action that indexes all symbols (macros and AST decls).
 std::unique_ptr
 createIndexingAction(std::shared_ptr DataConsumer,
Index: clang-tools-extra/clangd/unittests/IndexActionTests.cpp
===
--- clang-tools-extra/clangd/unittests/IndexActionTests.cpp
+++ clang-tools-extra/clangd/unittests/IndexActionTests.cpp
@@ -19,6 +19,7 @@
 
 using ::testing::AllOf;
 using ::testing::ElementsAre;
+using testing::EndsWith;
 using ::testing::Not;
 using ::testing::Pair;
 using ::testing::UnorderedElementsAre;
@@ -75,8 +76,7

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

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

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1194
-  if (!FD)
+  if (!Call.getOriginExpr())
 return;
 

Szelethus wrote:
> Szelethus wrote:
> > balazske wrote:
> > > This should be added to avoid later crash (probably not needed for every 
> > > check kind?):
> > > ```
> > >   const FunctionDecl *FD = C.getCalleeDecl(CE);
> > >   if (!FD)
> > > return;
> > > ```
> > Not all `CallEvent`s have a corresponding `FunctionDecl` or a `CallExpr`, 
> > for instance, `CXXAllocatorCall` corresponds with `CXXNewExpr`, which is 
> > not a `CallExpr`, but it is handled by this checker. For this reason, I 
> > decided to move this check to the individual modeling functions.
> Oh I'm sorry, do we have an actual crash resulting from this? 
I did not look into it by detail but the problem is in 
`MallocChecker::checkOwnershipAttr` with a null `FD`. Probably it is enough to 
insert a return at that point (makes the crash gone on that analyzed project).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75432



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

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

[Tom Lokovic via Phabricator via cfe-commits]

tdl-g added a comment.

Thanks, all, for the additional comments.  I addressed them all except for the 
suggestion to add an options-specific test.  I'm not against it, but (as I 
mention in the comment) I'm also unsure how to meaningfully test the 
include-inserting-related options.




Comment at: 
clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp:72
+   binaryOperator(hasOperatorName("=="),
+  hasEitherOperand(ignoringParenImpCasts(StringNpos)),
+  hasEitherOperand(ignoringParenImpCasts(StringFind))),

njames93 wrote:
> ymandel wrote:
> > Would `hasOperands` replace these two separate calls to `hasEitherOperand`? 
> > Same below lines 79-80 (I think it's a new matcher, fwiw).
> Just added, I added it to remove calls of multiple hasEitherOperand calls for 
> the reason of preventing the matchers matching the same operand. I just got a 
> little lost in my previous comment
Switched to hasOperands.  I agree that expresses the intent better than two 
independent calls of hasEitherOperand.

Note that it was working fine before--the test cases confirmed that it wasn't 
matching string::npos == string::npos or s.find(a) == s.find(a).  I'm guessing 
that's because the matchers inside hasEitherOperand have bindings, and if the 
matcher matched but one of the bindings was missing, the rewriterule suppressed 
itself?  Is this a plausible theory?

(The question moot since hasOperands is better overall, but I'd like to 
understand why it was working before.)



Comment at: 
clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp:98
+const LangOptions &LangOpts) const {
+  return LangOpts.CPlusPlus;
+}

njames93 wrote:
> nit: as abseil requires c++11, should this check also require c++11 support
Done, though I note that none of the other checkers in the abseil directory are 
checking for CplusCplus11.  (That's not an argument against doing it, just a 
question of whether or not they should also get this change at some point.)



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp:2
+// RUN: %check_clang_tidy %s abseil-string-find-str-contains %t -- \
+// RUN:   -config="{CheckOptions: []}"
+

ymandel wrote:
> I'm not sure what's standard practice, but consider whether its worth adding 
> another test file that tests the check options. It wouldn't have to be 
> comprehensive like this one, just some basic tests that the options are being 
> honored.
I'm open to it, but note that two of the three options (IncludeStyle, inherited 
from TransformerClangTidy) and AbseilStringsMatchHeader, only manifest in the 
header-inclusion.

So if I had a separate test it would be easy to confirm that StringLikeClasses 
was being honored.  But I'm not sure how I'd confirm that IncludeStyle or 
AbseilStringsMatchHeader are being honored.  Suggestions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80023



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

[PATCH] D80025: [ASTMatcher] Correct memoization bug ignoring direction (descendants or ancestors)

[Loïc Joly via Phabricator via cfe-commits]

loic-joly-sonarsource added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:46-49
+enum class MatchDirection {
+  Ancestors,
+  Descendants
+};

klimek wrote:
> loic-joly-sonarsource wrote:
> > klimek wrote:
> > > Nice find! Why don't we need more states though?
> > > 1. wouldn't hasParent() followed by a hasAncestor() also trigger the 
> > > memoization? (if so, we'd need transitive / non-transitive)
> > > 2. can we trigger a directional match and a non-directional 
> > > (non-traversal, for example, explicit) match in the same memoization 
> > > scope?
> > 1. Yes, I'm going to add it
> > 2. Sorry, I did not understand what you mean...
> > 
> Re 2: nevermind, we don't memoize non-transitive matches (other than 
> hasParent / hasChild), right?
> In that case, I'm wondering whether the simpler solution is to just not 
> memoize hasParent / hasChild - wouldn't that be more in line with the rest of 
> the memoization?
If I correctly understand what you mean, you think that memoizing recursive 
searches (ancestors/descendants) might not be worth it, and that just memoizing 
direct searches (parent, child) would be enough.

I really don't know if it's true or not, and I believe that getting this kind 
of data requires a significant benchmarking effort (which code base, which 
matchers...). And there might also be other performance-related concerns (for 
instance, the value of `MaxMemoizationEntries`, the choice of `std::map` to 
store the cache...),  

In other words, I think this would go far beyond what this PR is trying to 
achieve, which is only correcting a bug.


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

https://reviews.llvm.org/D80025



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Hi Endre, just checked the latest update. Overall looks good to me, but found 
some nits.




Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:260
+/// that produce the AST used for analysis.
+StringRef OnDemandParsingDatabase;
+

Should we rename this member to PathToInvocationList... ?



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:581
+  SmallVector CommandLineArgs(InvocationCommand.size());
+  std::transform(InvocationCommand.begin(), InvocationCommand.end(),
+ CommandLineArgs.begin(),

Could we avoid this transfer if InvocationList was storing `const char *` 
values instead of std::strings? Why can't we store `char*`s in InvocationList?



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:593
+CrossTranslationUnitContext::ASTOnDemandLoader::lazyInitCompileCommands() {
+  /// Lazily initialize the invocation list filed used for on-demand parsing.
+  if (InvocationList)

typo: filed -> field



Comment at: clang/test/Analysis/ctu-on-demand-parsing.c:6
+// Path substitutions on Windows platform could contain backslashes. These are 
escaped in the json file.
+// RUN: echo '[{"directory":"%t","command":"gcc -std=c89 -Wno-visibility 
ctu-other.c","file":"ctu-other.c"}]' | sed -e 's/\\//g' > 
%t/compile_commands.json
+// RUN: echo '"%t/ctu-other.c": ["gcc", "-std=c89", "-Wno-visibility", 
"ctu-other.c"]' | sed -e 's/\\//g' > %t/invocations.yaml

Perhaps we could document here that the compile_commands.json is needed ONLY 
for %clang_extdef_map.



Comment at: clang/test/Analysis/ctu-on-demand-parsing.cpp:9
+// RUN: echo '[{"directory":"%t/Inputs","command":"clang++ 
ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/Inputs","command":"clang++
 ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > 
%t/compile_commands.json
+// RUN: echo '{"%t/Inputs/ctu-chain.cpp": ["g++", "%t/Inputs/ctu-chain.cpp"], 
"%t/Inputs/ctu-other.cpp": ["g++", "%t/Inputs/ctu-other.cpp"]}' | sed -e 
's/\\//g' > %t/invocations.yaml
+// RUN: cd "%t" && %clang_extdef_map Inputs/ctu-chain.cpp Inputs/ctu-other.cpp 
> externalDefMap.txt

I know this is just a nit, but this is very hard to read. Do you think we could 
break this up (and other long lines) into multiple lines  but within the same 
RUN directive?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75665



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

[PATCH] D69987: [RISCV] Assemble/Disassemble v-ext instructions.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai added a comment.

In D69987#1853986 , @HsiangKai wrote:

> Update to version 0.8-draft-20191213.


Hi Roger,

I have updated it to 0.8-draft-20191213 in February. It is the same as version 
0.8. Sorry for that I did not update the commit message. Thanks for your kindly 
reminding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69987



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

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

[Tom Lokovic via Phabricator via cfe-commits]

tdl-g updated this revision to Diff 265234.
tdl-g marked 12 inline comments as done.
tdl-g added a comment.

Addressed second round of comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80023

Files:
  clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tools-extra/clang-tidy/abseil/CMakeLists.txt
  clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
  clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/abseil-string-find-str-contains.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/abseil-string-find-str-contains.cpp
@@ -0,0 +1,290 @@
+// RUN: %check_clang_tidy %s abseil-string-find-str-contains %t -- \
+// RUN:   -config="{CheckOptions: []}"
+
+using size_t = decltype(sizeof(int));
+
+namespace std {
+
+// Lightweight standin for std::string.
+template 
+class basic_string {
+public:
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const C *);
+  ~basic_string();
+  int find(basic_string s, int pos = 0);
+  int find(const C *s, int pos = 0);
+  int find(char c, int pos = 0);
+  static constexpr size_t npos = -1;
+};
+typedef basic_string string;
+
+// Lightweight standin for std::string_view.
+template 
+class basic_string_view {
+public:
+  basic_string_view();
+  basic_string_view(const basic_string_view &);
+  basic_string_view(const C *);
+  ~basic_string_view();
+  int find(basic_string_view s, int pos = 0);
+  int find(const C *s, int pos = 0);
+  int find(char c, int pos = 0);
+  static constexpr size_t npos = -1;
+};
+typedef basic_string_view string_view;
+
+} // namespace std
+
+namespace absl {
+
+// Lightweight standin for absl::string_view.
+class string_view {
+public:
+  string_view();
+  string_view(const string_view &);
+  string_view(const char *);
+  ~string_view();
+  int find(string_view s, int pos = 0);
+  int find(const char *s, int pos = 0);
+  int find(char c, int pos = 0);
+  static constexpr size_t npos = -1;
+};
+
+} // namespace absl
+
+// Functions that take and return our various string-like types.
+std::string foo_ss(std::string);
+std::string_view foo_ssv(std::string_view);
+absl::string_view foo_asv(absl::string_view);
+std::string bar_ss();
+std::string_view bar_ssv();
+absl::string_view bar_asv();
+
+// Confirms that find==npos and find!=npos work for each supported type, when
+// npos comes from the correct type.
+void basic_tests() {
+  std::string ss;
+  ss.find("a") == std::string::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of find() == npos
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ss, "a");{{$}}
+
+  ss.find("a") != std::string::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of find() != npos
+  // CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(ss, "a");{{$}}
+
+  std::string::npos != ss.find("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(ss, "a");{{$}}
+
+  std::string_view ssv;
+  ssv.find("a") == std::string_view::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(ssv, "a");{{$}}
+
+  ssv.find("a") != std::string_view::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(ssv, "a");{{$}}
+
+  std::string_view::npos != ssv.find("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(ssv, "a");{{$}}
+
+  absl::string_view asv;
+  asv.find("a") == absl::string_view::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StrContains(asv, "a");{{$}}
+
+  asv.find("a") != absl::string_view::npos;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(asv, "a");{{$}}
+
+  absl::string_view::npos != asv.find("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StrContains instead of
+  // CHECK-FIXES: {{^[[:space:]]*}}absl::StrContains(asv, "a");{{$}}
+}
+
+// Confirms that it works even if you mix-and-match the type for find and for
+// npos.  (One of the reasons for this checker is to clean up cases that
+// accidentally mix-and-match like this.  absl::StrContains is less
+// error-prone.)
+void mismatched_npos() {
+  std::string ss

[PATCH] D80293: [clangd] Run PreambleThread in async mode behind a flag

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:248
+  ReqCV.wait(Lock, [this] {
+return !NextReq || NextReq->WantDiags != WantDiagnostics::Yes;
+  });

This is mostly to make sure tests do not regress, I am not sure if we have any 
real world uses of it though.

I would love to just drop this and change tests instead. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80293



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

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

In D79773#2046565 , @miscco wrote:

> Question: Should I add my wip work as a child revision or what would you 
> suggest


Certainly your revision on github was useful for me to take a look at, but to 
be honest this change is getting large and I don't like landing such a large 
change in one go but rather make small incremental improvements (I believe this 
is the LLVM way)

Feel free to create a revision (and reference it here). Once we land some form 
of initial implementation I feel we can start making improvement as we detect 
corner cases or adding new options (and that might be more easily spread 
between developers)

To be honest I would rather start getting bugs from external users than 
clang-format being the butt of peoples complaints 
https://twitter.com/the_moisrex/status/1262982971973423105


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

https://reviews.llvm.org/D79773



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

[clang] c8a869c - [OPENMP][DOCS]Update status of implemented features, NFC.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-05-20T10:54:53-04:00
New Revision: c8a869c5e025dcee3bd7393b14a0d55c1ee326e5

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

LOG: [OPENMP][DOCS]Update status of implemented features, NFC.

Added: 


Modified: 
clang/docs/OpenMPSupport.rst

Removed: 




diff  --git a/clang/docs/OpenMPSupport.rst b/clang/docs/OpenMPSupport.rst
index 49cd11e4eeeb..a4db3ea2e66b 100644
--- a/clang/docs/OpenMPSupport.rst
+++ b/clang/docs/OpenMPSupport.rst
@@ -149,7 +149,7 @@ implementation.
 
+--+--+--+---+
 | task extension   | clause: depend on the taskwait construct  
   | :part:`worked on`| 
  |
 
+--+--+--+---+
-| task extension   | depend objects and detachable tasks   
   | :part:`worked on`| 
  |
+| task extension   | depend objects and detachable tasks   
   | :good:`done` | 
  |
 
+--+--+--+---+
 | task extension   | mutexinoutset dependence-type for tasks   
   | :good:`done` | D53380,D57576   
  |
 
+--+--+--+---+
@@ -181,7 +181,7 @@ implementation.
 
+--+--+--+---+
 | device extension | clause: extended device   
   | :good:`done` | 
  |
 
+--+--+--+---+
-| device extension | clause: uses_allocators clause
   | :none:`claimed`  | 
  |
+| device extension | clause: uses_allocators clause
   | :good:`done` | 
  |
 
+--+--+--+---+
 | device extension | clause: in_reduction  
   | :part:`worked on`| r308768 
  |
 
+--+--+--+---+
@@ -205,7 +205,7 @@ implementation.
 
+--+--+--+---+
 | device extension | clause: atomic_default_mem_order  
   | :good:`done` | D53513  
  |
 
+--+--+--+---+
-| device extension | clause: dynamic_allocators
   | :none:`unclaimed parts`  | D53079  
  |
+| device extension | clause: dynamic_allocators
   | :part:`unclaimed parts`  | D53079  
  |
 
+--+

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

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

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



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1194
-  if (!FD)
+  if (!Call.getOriginExpr())
 return;
 

Szelethus wrote:
> balazske wrote:
> > Szelethus wrote:
> > > Szelethus wrote:
> > > > balazske wrote:
> > > > > This should be added to avoid later crash (probably not needed for 
> > > > > every check kind?):
> > > > > ```
> > > > >   const FunctionDecl *FD = C.getCalleeDecl(CE);
> > > > >   if (!FD)
> > > > > return;
> > > > > ```
> > > > Not all `CallEvent`s have a corresponding `FunctionDecl` or a 
> > > > `CallExpr`, for instance, `CXXAllocatorCall` corresponds with 
> > > > `CXXNewExpr`, which is not a `CallExpr`, but it is handled by this 
> > > > checker. For this reason, I decided to move this check to the 
> > > > individual modeling functions.
> > > Oh I'm sorry, do we have an actual crash resulting from this? 
> > I did not look into it by detail but the problem is in 
> > `MallocChecker::checkOwnershipAttr` with a null `FD`. Probably it is enough 
> > to insert a return at that point (makes the crash gone on that analyzed 
> > project).
> I'm sorry but I don't have enough to follow up on this. On the master branch, 
> everything seems to work smoothly for me (all tests pass) and I didn't get 
> any buildbot failures. Can you post a code snippet?
If you have a project name, I can analyze that myself to save you the trouble.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75432



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

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

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

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



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1194
-  if (!FD)
+  if (!Call.getOriginExpr())
 return;
 

balazske wrote:
> Szelethus wrote:
> > Szelethus wrote:
> > > balazske wrote:
> > > > This should be added to avoid later crash (probably not needed for 
> > > > every check kind?):
> > > > ```
> > > >   const FunctionDecl *FD = C.getCalleeDecl(CE);
> > > >   if (!FD)
> > > > return;
> > > > ```
> > > Not all `CallEvent`s have a corresponding `FunctionDecl` or a `CallExpr`, 
> > > for instance, `CXXAllocatorCall` corresponds with `CXXNewExpr`, which is 
> > > not a `CallExpr`, but it is handled by this checker. For this reason, I 
> > > decided to move this check to the individual modeling functions.
> > Oh I'm sorry, do we have an actual crash resulting from this? 
> I did not look into it by detail but the problem is in 
> `MallocChecker::checkOwnershipAttr` with a null `FD`. Probably it is enough 
> to insert a return at that point (makes the crash gone on that analyzed 
> project).
I'm sorry but I don't have enough to follow up on this. On the master branch, 
everything seems to work smoothly for me (all tests pass) and I didn't get any 
buildbot failures. Can you post a code snippet?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75432



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

[PATCH] D79966: [OPENMP]Fix PR45911: Data sharing and lambda capture.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev updated this revision to Diff 265246.
ABataev added a comment.
Herald added a subscriber: sstefan1.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79966

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/task_firstprivate_codegen.cpp


Index: clang/test/OpenMP/task_firstprivate_codegen.cpp
===
--- clang/test/OpenMP/task_firstprivate_codegen.cpp
+++ clang/test/OpenMP/task_firstprivate_codegen.cpp
@@ -56,6 +56,7 @@
 
 int main() {
   static int sivar;
+  float local = 0;
 #ifdef LAMBDA
   // LAMBDA: [[G:@.+]] = global double
   // LAMBDA: [[SIVAR:@.+]] = internal global i{{[0-9]+}} 0,
@@ -73,9 +74,12 @@
 // LAMBDA: [[SIVAR_VAL:%.+]] = load i{{[0-9]+}}, i{{[0-9]+}}* [[SIVAR]],
 // LAMBDA: store i{{[0-9]+}} [[SIVAR_VAL]], i{{[0-9]+}}* [[SIVAR_PRIVATE_ADDR]]
 
+// LAMBDA: [[LOCAL_PRIVATE_ADDR:%.+]] = getelementptr inbounds %{{.+}}, 
%{{.+}}* [[PRIVATES]], i{{.+}} 0, i{{.+}} 2
+// LAMBDA: store float %{{.+}}, float* [[LOCAL_PRIVATE_ADDR]]
+
 // LAMBDA: call i32 @__kmpc_omp_task(%{{.+}}* @{{.+}}, i32 %{{.+}}, i8* 
[[RES]])
 // LAMBDA: ret
-#pragma omp task firstprivate(g, sivar)
+#pragma omp task firstprivate(g, sivar, local)
   {
 // LAMBDA: define {{.+}} void [[INNER_LAMBDA:@.+]](%{{.+}}* 
[[ARG_PTR:%.+]])
 // LAMBDA: store %{{.+}}* [[ARG_PTR]], %{{.+}}** [[ARG_PTR_REF:%.+]],
@@ -115,9 +119,13 @@
   // BLOCKS: [[SIVAR_PRIVATE_ADDR:%.+]] = getelementptr inbounds %{{.+}}, 
%{{.+}}* [[PRIVATES]], i{{.+}} 0, i{{.+}} 1
   // BLOCKS: [[SIVAR_VAL:%.+]] = load i{{[0-9]+}}, i{{[0-9]+}}* [[SIVAR]],
   // BLOCKS: store i{{[0-9]+}} [[SIVAR_VAL]], i{{[0-9]+}}* 
[[SIVAR_PRIVATE_ADDR]],
+
+  // BLOCKS: [[LOCAL_PRIVATE_ADDR:%.+]] = getelementptr inbounds %{{.+}}, 
%{{.+}}* [[PRIVATES]], i{{.+}} 0, i{{.+}} 2
+  // BLOCKS: store float %{{.+}}, float* [[LOCAL_PRIVATE_ADDR]]
+
   // BLOCKS: call i32 @__kmpc_omp_task(%{{.+}}* @{{.+}}, i32 %{{.+}}, i8* 
[[RES]])
   // BLOCKS: ret
-#pragma omp task firstprivate(g, sivar)
+#pragma omp task firstprivate(g, sivar, local)
   {
 // BLOCKS: define {{.+}} void {{@.+}}(i8*
 // BLOCKS-NOT: [[G]]{{[[^:word:]]}}
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -4836,11 +4836,16 @@
   C.getDeclAlign(OriginalVD)),
   SharedRefLValue.getType(), LValueBaseInfo(AlignmentSource::Decl),
   SharedRefLValue.getTBAAInfo());
+} else if (CGF.LambdaCaptureFields.count(
+   Pair.second.Original->getCanonicalDecl()) > 0 ||
+   dyn_cast_or_null(CGF.CurCodeDecl)) {
+  SharedRefLValue = CGF.EmitLValue(Pair.second.OriginalRef);
 } else {
+  // Processing for implicitly captured variables.
   InlinedOpenMPRegionRAII Region(
   CGF, [](CodeGenFunction &, PrePostActionTy &) {}, OMPD_unknown,
   /*HasCancel=*/false);
-  SharedRefLValue =  CGF.EmitLValue(Pair.second.OriginalRef);
+  SharedRefLValue = CGF.EmitLValue(Pair.second.OriginalRef);
 }
 if (Type->isArrayType()) {
   // Initialize firstprivate array.


Index: clang/test/OpenMP/task_firstprivate_codegen.cpp
===
--- clang/test/OpenMP/task_firstprivate_codegen.cpp
+++ clang/test/OpenMP/task_firstprivate_codegen.cpp
@@ -56,6 +56,7 @@
 
 int main() {
   static int sivar;
+  float local = 0;
 #ifdef LAMBDA
   // LAMBDA: [[G:@.+]] = global double
   // LAMBDA: [[SIVAR:@.+]] = internal global i{{[0-9]+}} 0,
@@ -73,9 +74,12 @@
 // LAMBDA: [[SIVAR_VAL:%.+]] = load i{{[0-9]+}}, i{{[0-9]+}}* [[SIVAR]],
 // LAMBDA: store i{{[0-9]+}} [[SIVAR_VAL]], i{{[0-9]+}}* [[SIVAR_PRIVATE_ADDR]]
 
+// LAMBDA: [[LOCAL_PRIVATE_ADDR:%.+]] = getelementptr inbounds %{{.+}}, %{{.+}}* [[PRIVATES]], i{{.+}} 0, i{{.+}} 2
+// LAMBDA: store float %{{.+}}, float* [[LOCAL_PRIVATE_ADDR]]
+
 // LAMBDA: call i32 @__kmpc_omp_task(%{{.+}}* @{{.+}}, i32 %{{.+}}, i8* [[RES]])
 // LAMBDA: ret
-#pragma omp task firstprivate(g, sivar)
+#pragma omp task firstprivate(g, sivar, local)
   {
 // LAMBDA: define {{.+}} void [[INNER_LAMBDA:@.+]](%{{.+}}* [[ARG_PTR:%.+]])
 // LAMBDA: store %{{.+}}* [[ARG_PTR]], %{{.+}}** [[ARG_PTR_REF:%.+]],
@@ -115,9 +119,13 @@
   // BLOCKS: [[SIVAR_PRIVATE_ADDR:%.+]] = getelementptr inbounds %{{.+}}, %{{.+}}* [[PRIVATES]], i{{.+}} 0, i{{.+}} 1
   // BLOCKS: [[SIVAR_VAL:%.+]] = load i{{[0-9]+}}, i{{[0-9]+}}* [[SIVAR]],
   // BLOCKS: store i{{[0-9]+}} [[SIVAR_VAL]], i{{[0-9]+}}* [[SIVAR_PRIVATE_ADDR]],
+
+  // BLOCKS: [[LOCAL_PRIVATE_ADDR:%.+]] = getelementptr inbounds %{{.+}}, %{{.+}}* [[PRIVATES]], i{{.+}} 0, i{{.+}} 2
+  // BLOCKS: store float %{{.+}}, float* [[LOCAL_PRIVATE_ADDR]]
+
   

[PATCH] D80025: [ASTMatcher] Correct memoization bug ignoring direction (descendants or ancestors)

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:46-49
+enum class MatchDirection {
+  Ancestors,
+  Descendants
+};

loic-joly-sonarsource wrote:
> klimek wrote:
> > loic-joly-sonarsource wrote:
> > > klimek wrote:
> > > > Nice find! Why don't we need more states though?
> > > > 1. wouldn't hasParent() followed by a hasAncestor() also trigger the 
> > > > memoization? (if so, we'd need transitive / non-transitive)
> > > > 2. can we trigger a directional match and a non-directional 
> > > > (non-traversal, for example, explicit) match in the same memoization 
> > > > scope?
> > > 1. Yes, I'm going to add it
> > > 2. Sorry, I did not understand what you mean...
> > > 
> > Re 2: nevermind, we don't memoize non-transitive matches (other than 
> > hasParent / hasChild), right?
> > In that case, I'm wondering whether the simpler solution is to just not 
> > memoize hasParent / hasChild - wouldn't that be more in line with the rest 
> > of the memoization?
> If I correctly understand what you mean, you think that memoizing recursive 
> searches (ancestors/descendants) might not be worth it, and that just 
> memoizing direct searches (parent, child) would be enough.
> 
> I really don't know if it's true or not, and I believe that getting this kind 
> of data requires a significant benchmarking effort (which code base, which 
> matchers...). And there might also be other performance-related concerns (for 
> instance, the value of `MaxMemoizationEntries`, the choice of `std::map` to 
> store the cache...),  
> 
> In other words, I think this would go far beyond what this PR is trying to 
> achieve, which is only correcting a bug.
What I'm trying to say is:
Currently the only non-transitive matchers we memoize are hasChild and 
hasParent, which ... seems weird - my operating hypothesis is that back in the 
day I didn't realize that or I'd have changed it :)

Thus, it seems like a simpler patch is to not memoize if it's not a transitive 
match.

Sam also had a good idea, which is to change the ASTMatchFinder API to instead 
of the current:
matchesChildOf
matchesDescendantOf
matchesAncestorOf(..., MatchMode)

create different atoms:
matchesChildOf
matchesDescendantOfOrSelf
matchesParentOf
matchesAncestorOfOrSelf

then hasDescendant(m) would be implemented as hasChild(hasDescendantOrSelf(m)) 
and the direction would be part of the matcher already.
That as an aside, which I'd propose to not do in the current patch though :)

My proposal for the current patch is to not memoize if we're only matching a 
single child or parent.




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

https://reviews.llvm.org/D80025



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

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Hi, @Szelethus, I don't know exactly which of the changes (this one, 
https://reviews.llvm.org/D75430, or https://reviews.llvm.org/D75431) causes a 
crash on SQLite, but it's definitely one of these.

**Steps to reproduce**

  clang -cc1 -Wdeprecated-objc-isa-usage -Werror=deprecated-objc-isa-usage 
-Werror=implicit-function-declaration -analyze -disable-free -main-file-name 
sqlite3.c -analyzer-store=region -analyzer-opt-analyze-nested-blocks 
-analyzer-checker=core -analyzer-checker=apiModeling -analyzer-checker=unix 
-analyzer-checker=osx 
-analyzer-checker=security.insecureAPI.decodeValueOfObjCType 
-analyzer-checker=deadcode 
-analyzer-checker=security.insecureAPI.UncheckedReturn 
-analyzer-checker=security.insecureAPI.getpw 
-analyzer-checker=security.insecureAPI.gets 
-analyzer-checker=security.insecureAPI.mktemp 
-analyzer-checker=security.insecureAPI.mkstemp 
-analyzer-checker=security.insecureAPI.vfork 
-analyzer-checker=nullability.NullPassedToNonnull 
-analyzer-checker=nullability.NullReturnedFromNonnull -analyzer-output plist -w 
-setup-static-analyzer -analyzer-config-compatibility-mode=true 
-mrelocation-model pic -pic-level 2 -mthread-model posix -mframe-pointer=all 
-fno-strict-return -fno-rounding-math -munwind-tables 
-faligned-alloc-unavailable -target-cpu core2 -dwarf-column-info 
-target-linker-version 556.6 -Wno-reorder-init-list 
-Wno-implicit-int-float-conversion -Wno-c99-designator 
-Wno-final-dtor-non-final-class -Wno-extra-semi-stmt 
-Wno-misleading-indentation -Wno-quoted-include-in-framework-header 
-Wno-implicit-fallthrough -Wno-enum-enum-conversion -Wno-enum-float-conversion 
-ferror-limit 19 -stack-protector 1 -fblocks -fencode-extended-block-signature 
-fregister-global-dtors-with-atexit -fgnuc-version=4.2.1 -fmax-type-align=16 
-analyzer-checker=alpha.unix.SimpleStream,alpha.security.taint,cplusplus.NewDeleteLeaks,core,cplusplus,deadcode,security,unix,osx,nullability
 -analyzer-config serialize-stats=true,stable-report-filename=true -x c 
sqlite3-258aa5.c

**Output**

  Assertion failed: (FromPtr && ToPtr && "By this point, FreeMemAux and 
MallocMemAux should have checked " "whether the argument or the return value is 
symbolic!"), function ReallocMemAux, file 
/Users/vsavchenko/source/llvm-project/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp,
 line 2409.

Attached file is the exact version of SQLite source code to reproduce the 
issue: F11965188: sqlite3-258aa5.c 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75432



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

[PATCH] D79704: [Analyzer] [NFC] Parameter Regions

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Thanks for addressing my comments!




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:309
+  /// Get the lvalue for a parameter.
+  Loc getLValue(const Expr *Call, unsigned Index,
+const LocationContext *LC) const;

baloghadamsoftware wrote:
> martong wrote:
> > Is this used anywhere in this patch?
> > 
> > And why isn't it a `CallExpr` (instead of `Expr`)?
> The origin expression of a call may be a set of different kinds of 
> expressions: `CallExpr`, `CXXConstructExpr`, `BlockExpr` or `ObjCMessageExpr`.
Is this function used anywhere in this patch? Could not find any reference to 
it.

> The origin expression of a call may be a set of different kinds of 
> expressions: CallExpr, CXXConstructExpr, BlockExpr or ObjCMessageExpr.

Okay, makes sense, could you please document it then in the comment for the 
function? And perhaps we should assert on these kinds as a sanity check.



Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:915
+return cast(I.getCapturedRegion());
+} else if (const auto *PR = dyn_cast(OrigR)) {
+  if (PR->getDecl() == VD)

baloghadamsoftware wrote:
> martong wrote:
> > Both the `VarRegion` and `ParamRegion` cases here do the same.
> > This suggest that maybe it would be better to have `VarRegion` as a base 
> > class for `ParamVarRegion`.
> > This is aligned to what Artem suggested:
> > > We can even call it VarRegion and have sub-classes be ParamVarRegion and 
> > > NonParamVarRegion or something like that.
> > But maybe `NonParamVarRegion` is not needed since that is actually the 
> > `VarRegion`.
> Usually we do not inherit from concrete classes by changing a method 
> implementation that already exist. The other reason is even stronger: 
> `NonParamVarRegion` //must// store the `Decl` of the variable, 
> `ParamVarRegion` //must not//.
> Usually we do not inherit from concrete classes by changing a method 
> implementation that already exist.

That's what we exactly do all over the place in the AST library.
Which method(s) should we change by the way?

Nevermind, I am perfectly fine having VarRegion as base and NonParamVarRegion 
and ParamVarRegion as derived.




Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:1230
+
+const TypedValueRegion*
+MemRegionManager::getRegionForParam(const ParmVarDecl *PVD,

baloghadamsoftware wrote:
> martong wrote:
> > Why the return type is not `ParamVarRegion*`?
> Because for top-level functions and captured parameters it still returns 
> `VarRegion`.
Okay, makes sense, could you please document it then in the comment for the 
function?


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

https://reviews.llvm.org/D79704



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

[PATCH] D78099: [analyzer][RetainCount] Tie diagnostics to osx.cocoa.RetainCount rather then RetainCountBase, for the most part

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

Szelethus added a comment.

Ping, @NoQ @vsavchenko I'm confident with the previous patch, but I'd be glad 
if one of you could take a look before I land this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78099



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

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

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

Szelethus added a comment.

Thanks, I'll get right to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75432



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

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Michael Schellenberger Costa via Phabricator via cfe-commits]

miscco added a comment.

It seems the spacing of the binary operator is not yet stable. This test is 
breaking for me:

  verifyFormat("template \nconcept someConcept = Constraint1 && 
Constraint2;");


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

https://reviews.llvm.org/D79773



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

[PATCH] D80289: [Driver][X86] Support branch align options with LTO

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/lto.c:81
+
+/// -flto passes along an explicit branch align argument.
+/// Test -malign-branch-boundary=

This piece of logic is closer to x86. The test can be added to 
`x86-malign-branch.c` instead


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80289



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

[PATCH] D80286: [Analyzer] Allow creation of stack frame for functions without definition

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/unittests/StaticAnalyzer/ParamRegionTest.cpp:22
+const ParmVarDecl *PVD) {
+for (const auto *D2: PVD->redecls()) {
+  const auto *PVD2 = cast(D2);

I am concerned about the redeclaration chain of ParmVarDecls. In the following 
example:
```
void foo(int a);
void foo(int a);
```
we have the prev decl set only for the FunctionDecls and not for the 
ParmVarDecl (please double check).
So in the test we should go through the redecls() of the FunctionDecl of the 
ParmVarDecl. And we should get a PVD from each redeclaration by the index. 



Comment at: clang/unittests/StaticAnalyzer/ParamRegionTest.cpp:87
   std::make_unique(),
+  "void foo(int n); "
+  "void bar(int l); "

I think a raw string literal with clang-formatted code in it would make the 
test more valuable.


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

https://reviews.llvm.org/D80286



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

[PATCH] D80289: [Driver][X86] Support branch align options with LTO

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:501
+  // Handle options for branch align
+  if (Args.hasArg(options::OPT_mbranches_within_32B_boundaries)) {
+CmdArgs.push_back("-plugin-opt=-x86-branches-within-32B-boundaries");

The large chunk of code duplicates Clang.cpp:addX86AlignBranchArgs.

Can you refactor addX86AlignBranchArgs to be reused here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80289



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

[PATCH] D80171: [analyzer] LoopUnrolling: fix crash when a parameter is a loop counter

[Abbas Sabra via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:167
 
+  const bool isParm = VD->getKind() == Decl::ParmVar;
+  // Reference parameters are assumed as escaped variables.

vsavchenko wrote:
> `getKind` function is only an implementation detail for 
> `isa`/`cast`/`dan_cast` functions ([[ 
> https://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates
>  | docs ]]).  So, this condition would be better in the following form: 
> `isa(VD)`.
> 
> NOTE: One good motivation here is that //maybe// in the future there will be 
> some sort of new type of function parameters and it will be derived from 
> `ParmVarDecl`.  In this situation, direct comparison with the kind of node 
> will not work and probably won't be fixed by developers who introduced the 
> new node, but `isa` approach will stay correct.
Thanks for the detailed explanation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80171



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

[PATCH] D80171: [analyzer] LoopUnrolling: fix crash when a parameter is a loop counter

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra updated this revision to Diff 265253.
AbbasSabra added a comment.

Fix code review 2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80171

Files:
  clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  clang/test/Analysis/loop-unrolling.cpp


Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -499,3 +499,15 @@
 clang_analyzer_numTimesReached(); // expected-warning {{6}}
   }
 }
+
+void parm_by_value_as_loop_counter(int i) {
+  for (i = 0; i < 10; ++i) {
+clang_analyzer_numTimesReached(); // expected-warning {{10}}
+  }
+}
+
+void parm_by_ref_as_loop_counter(int &i) {
+  for (i = 0; i < 10; ++i) {
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -164,6 +164,11 @@
   if (VD->hasGlobalStorage())
 return true;
 
+  const bool isParm = isa(VD);
+  // Reference parameters are assumed as escaped variables.
+  if (isParm && VD->getType()->isReferenceType())
+return true;
+
   while (!N->pred_empty()) {
 // FIXME: getStmtForDiagnostics() does nasty things in order to provide
 // a valid statement for body farms, do we need this behavior here?
@@ -193,6 +198,11 @@
 
 N = N->getFirstPred();
   }
+
+  // Parameter declaration will not be found.
+  if (isParm)
+return false;
+
   llvm_unreachable("Reached root without finding the declaration of VD");
 }
 


Index: clang/test/Analysis/loop-unrolling.cpp
===
--- clang/test/Analysis/loop-unrolling.cpp
+++ clang/test/Analysis/loop-unrolling.cpp
@@ -499,3 +499,15 @@
 clang_analyzer_numTimesReached(); // expected-warning {{6}}
   }
 }
+
+void parm_by_value_as_loop_counter(int i) {
+  for (i = 0; i < 10; ++i) {
+clang_analyzer_numTimesReached(); // expected-warning {{10}}
+  }
+}
+
+void parm_by_ref_as_loop_counter(int &i) {
+  for (i = 0; i < 10; ++i) {
+clang_analyzer_numTimesReached(); // expected-warning {{4}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -164,6 +164,11 @@
   if (VD->hasGlobalStorage())
 return true;
 
+  const bool isParm = isa(VD);
+  // Reference parameters are assumed as escaped variables.
+  if (isParm && VD->getType()->isReferenceType())
+return true;
+
   while (!N->pred_empty()) {
 // FIXME: getStmtForDiagnostics() does nasty things in order to provide
 // a valid statement for body farms, do we need this behavior here?
@@ -193,6 +198,11 @@
 
 N = N->getFirstPred();
   }
+
+  // Parameter declaration will not be found.
+  if (isParm)
+return false;
+
   llvm_unreachable("Reached root without finding the declaration of VD");
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: nemanjai, jsji, daltenty, stevewan, 
jasonliu.
Herald added subscribers: atanasyan, jrtc27, mgorny.
Herald added a project: clang.
hubert.reinterpretcast edited the summary of this revision.

Some target toolchains use more than just a default `--sysroot`, but also 
include a default `--dyld-prefix` and an implicitly-added `-rpath`.

For example, using a vanilla build of Clang with the IBM Advance Toolchain for 
Linux on Power would require specifying `--sysroot=/opt/at12.0 
--dyld-prefix=/opt/at12.0 -rpath /opt/at12.0/lib64`. The GCC compiler provided 
with the Advance Toolchain is preconfigured such that adding such options is 
not necessary. This patch adds the configuration hooks that would allow Clang 
to be preconfigured similarly.

Note: The `DEFAULT_RPATH` behaviour has only been implemented for "GNU" 
toolchains.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80300

Files:
  clang/CMakeLists.txt
  clang/include/clang/Config/config.h.cmake
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/fuchsia.c
  clang/test/Driver/fuchsia.cpp
  clang/test/Driver/hurd.c
  clang/test/Driver/linux-ld.c
  clang/test/Driver/mips-mti.cpp

Index: clang/test/Driver/mips-mti.cpp
===
--- clang/test/Driver/mips-mti.cpp
+++ clang/test/Driver/mips-mti.cpp
@@ -5,6 +5,7 @@
 // RUN:--target=mips-mti-linux-gnu \
 // RUN:--gcc-toolchain=%S/Inputs/mips_mti_tree \
 // RUN:--sysroot="" \
+// RUN:--dyld-prefix="" \
 // RUN:-stdlib=libstdc++ \
 // RUN:-EB -mhard-float -mabi=32 \
 // RUN:   | FileCheck --check-prefix=EB-HARD-O32 %s
@@ -34,6 +35,7 @@
 // RUN:--target=mips-mti-linux-gnu \
 // RUN:--gcc-toolchain=%S/Inputs/mips_mti_tree \
 // RUN:--sysroot="" \
+// RUN:--dyld-prefix="" \
 // RUN:-stdlib=libstdc++ \
 // RUN:-EB -mhard-float -mabi=n32 \
 // RUN:   | FileCheck --check-prefix=EB-HARD-N32 %s
@@ -63,6 +65,7 @@
 // RUN:--target=mips64-mti-linux-gnu \
 // RUN:--gcc-toolchain=%S/Inputs/mips_mti_tree \
 // RUN:--sysroot="" \
+// RUN:--dyld-prefix="" \
 // RUN:-stdlib=libstdc++ \
 // RUN:-EB -mhard-float -mabi=64 \
 // RUN:   | FileCheck --check-prefix=EB-HARD-N64 %s
@@ -92,6 +95,7 @@
 // RUN:--target=mips-mti-linux-gnu \
 // RUN:--gcc-toolchain=%S/Inputs/mips_mti_tree \
 // RUN:--sysroot="" \
+// RUN:--dyld-prefix="" \
 // RUN:-stdlib=libstdc++ \
 // RUN:-EL -mhard-float -mabi=32 \
 // RUN:   | FileCheck --check-prefix=EL-HARD-O32 %s
@@ -121,6 +125,7 @@
 // RUN:--target=mips-mti-linux-gnu \
 // RUN:--gcc-toolchain=%S/Inputs/mips_mti_tree \
 // RUN:--sysroot="" \
+// RUN:--dyld-prefix="" \
 // RUN:-stdlib=libstdc++ \
 // RUN:-EL -mhard-float -mabi=n32 \
 // RUN:   | FileCheck --check-prefix=EL-HARD-N32 %s
@@ -150,6 +155,7 @@
 // RUN:--target=mips64-mti-linux-gnu \
 // RUN:--gcc-toolchain=%S/Inputs/mips_mti_tree \
 // RUN:--sysroot="" \
+// RUN:--dyld-prefix="" \
 // RUN:-stdlib=libstdc++ \
 // RUN:-EL -mhard-float -mabi=64 \
 // RUN:   | FileCheck --check-prefix=EL-HARD-N64 %s
@@ -179,6 +185,7 @@
 // RUN:--target=mips-mti-linux-gnu \
 // RUN:--gcc-toolchain=%S/Inputs/mips_mti_tree \
 // RUN:--sysroot="" \
+// RUN:--dyld-prefix="" \
 // RUN:-stdlib=libstdc++ \
 // RUN:-EB -msoft-float \
 // RUN:   | FileCheck --check-prefix=EB-SOFT %s
@@ -208,6 +215,7 @@
 // RUN:--target=mips-mti-linux-gnu \
 // RUN:--gcc-toolchain=%S/Inputs/mips_mti_tree \
 // RUN:--sysroot="" \
+// RUN:--dyld-prefix="" \
 // RUN:-stdlib=libstdc++ \
 // RUN:-EL -msoft-float \
 // RUN:   | FileCheck --check-prefix=EL-SOFT %s
@@ -237,6 +245,7 @@
 // RUN:--target=mips-mti-linux-gnu \
 // RUN:--gcc-toolchain=%S/Inputs/mips_mti_tree \
 // RUN:--sysroot="" \
+// RUN:--dyld-prefix="" \
 // RUN:-stdlib=libstdc++ \
 // RUN:-EB -mhard-float -muclibc \
 // RUN:   | FileCheck --check-prefix=EB-HARD-UCLIBC %s
@@ -266,6 +275,7 @@
 // RUN:--target=mips-mti-linux-gnu \
 // RUN:--gcc-toolchain=%S/Inputs/mips_mti_tree \
 // RUN:--sysroot="" \
+// RUN:--dyld-prefix="" \
 // RUN:-stdlib=libstdc++ \
 // RUN:-EL -mhard-float -muclibc \
 // RUN:   | FileCheck --check-prefix=EL-HARD-UCLIBC %s
@@ -295,6 +305,7 @@
 // RUN:--target=mips-mti-linux-gnu \
 // RUN:--gcc-toolchain=%S/Inputs/mips_mti_tree \
 // RUN:--sysroot="" \
+// RUN:--dyld-prefix="" \
 // RUN:-stdlib=libstdc++ \
 // RUN:-EB -mhard-float -mnan=2008 \
 // RUN:   | FileCheck --check-prefix=EB-HARD-NAN2008 %

[PATCH] D79966: [OPENMP]Fix PR45911: Data sharing and lambda capture.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert accepted this revision.
jdoerfert 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/D79966/new/

https://reviews.llvm.org/D79966



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

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

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

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1194
-  if (!FD)
+  if (!Call.getOriginExpr())
 return;
 

Szelethus wrote:
> Szelethus wrote:
> > balazske wrote:
> > > Szelethus wrote:
> > > > Szelethus wrote:
> > > > > balazske wrote:
> > > > > > This should be added to avoid later crash (probably not needed for 
> > > > > > every check kind?):
> > > > > > ```
> > > > > >   const FunctionDecl *FD = C.getCalleeDecl(CE);
> > > > > >   if (!FD)
> > > > > > return;
> > > > > > ```
> > > > > Not all `CallEvent`s have a corresponding `FunctionDecl` or a 
> > > > > `CallExpr`, for instance, `CXXAllocatorCall` corresponds with 
> > > > > `CXXNewExpr`, which is not a `CallExpr`, but it is handled by this 
> > > > > checker. For this reason, I decided to move this check to the 
> > > > > individual modeling functions.
> > > > Oh I'm sorry, do we have an actual crash resulting from this? 
> > > I did not look into it by detail but the problem is in 
> > > `MallocChecker::checkOwnershipAttr` with a null `FD`. Probably it is 
> > > enough to insert a return at that point (makes the crash gone on that 
> > > analyzed project).
> > I'm sorry but I don't have enough to follow up on this. On the master 
> > branch, everything seems to work smoothly for me (all tests pass) and I 
> > didn't get any buildbot failures. Can you post a code snippet?
> If you have a project name, I can analyze that myself to save you the trouble.
Could not debug the problem because debug info was not there for unknown reason 
(no exact line locations were printed). But the project was emacs 26.3 
`configure --with-xpm=no --with-gif=no --with-tiff=no --with-gnutls=no`, failed 
at one of the the first files with CodeChecker analyze.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75432



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

[PATCH] D79998: Add AST_SIGNATURE record to unhashed control block of pcm files (Patch series 2/3)

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 265262.
dang added a comment.
Herald added a subscriber: ormris.

Add a small bug fix and test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79998

Files:
  clang/test/Modules/ASTSignature.c
  clang/test/Modules/Inputs/ASTHash/module.modulemap
  clang/test/Modules/Inputs/ASTHash/my_header_1.h
  clang/test/Modules/Inputs/ASTHash/my_header_2.h


Index: clang/test/Modules/Inputs/ASTHash/my_header_2.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/my_header_2.h
@@ -0,0 +1,3 @@
+#include "my_header_1.h"
+
+extern my_int var;
Index: clang/test/Modules/Inputs/ASTHash/my_header_1.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/my_header_1.h
@@ -0,0 +1 @@
+typedef int my_int;
Index: clang/test/Modules/Inputs/ASTHash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/module.modulemap
@@ -0,0 +1,8 @@
+module MyHeader1 {
+  header "my_header_1.h"
+}
+
+module MyHeader2 {
+  header "my_header_2.h"
+  export *
+}
Index: clang/test/Modules/ASTSignature.c
===
--- /dev/null
+++ clang/test/Modules/ASTSignature.c
@@ -0,0 +1,18 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -iquote %S/Inputs/ASTHash/ -fsyntax-only -fmodules \
+// RUN:   -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN:   -fmodules-cache-path=%t %s -Rmodule-build 2> %t1
+// RUN: %clang_cc1 -iquote "/dev/null" -iquote %S/Inputs/ASTHash/ 
-fsyntax-only \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN:   -fmodules-cache-path=%t %s -Rmodule-build 2> %t2
+// RUN: cat %t1 | grep "remark: building module 'MyHeader2'" | cut -d "'" -f 4 
\
+// RUN:  | while read -r MODULE; do llvm-bcanalyzer --dump --disable-histogram 
\
+// RUN:  $MODULE | grep "AST_SIGNATURE" > %t1.sig; done
+// RUN: cat %t2 | grep "remark: building module 'MyHeader2'" | cut -d "'" -f 4 
\
+// RUN:  | while read -r MODULE; do llvm-bcanalyzer --dump --disable-histogram 
\
+// RUN:  $MODULE | grep "AST_SIGNATURE" > %t1.sig; done
+// RUN: diff %t1.sig %t2.sig
+
+#include "my_header_2.h"
+
+my_int var = 42;


Index: clang/test/Modules/Inputs/ASTHash/my_header_2.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/my_header_2.h
@@ -0,0 +1,3 @@
+#include "my_header_1.h"
+
+extern my_int var;
Index: clang/test/Modules/Inputs/ASTHash/my_header_1.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/my_header_1.h
@@ -0,0 +1 @@
+typedef int my_int;
Index: clang/test/Modules/Inputs/ASTHash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/module.modulemap
@@ -0,0 +1,8 @@
+module MyHeader1 {
+  header "my_header_1.h"
+}
+
+module MyHeader2 {
+  header "my_header_2.h"
+  export *
+}
Index: clang/test/Modules/ASTSignature.c
===
--- /dev/null
+++ clang/test/Modules/ASTSignature.c
@@ -0,0 +1,18 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -iquote %S/Inputs/ASTHash/ -fsyntax-only -fmodules \
+// RUN:   -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN:   -fmodules-cache-path=%t %s -Rmodule-build 2> %t1
+// RUN: %clang_cc1 -iquote "/dev/null" -iquote %S/Inputs/ASTHash/ -fsyntax-only \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN:   -fmodules-cache-path=%t %s -Rmodule-build 2> %t2
+// RUN: cat %t1 | grep "remark: building module 'MyHeader2'" | cut -d "'" -f 4 \
+// RUN:  | while read -r MODULE; do llvm-bcanalyzer --dump --disable-histogram \
+// RUN:  $MODULE | grep "AST_SIGNATURE" > %t1.sig; done
+// RUN: cat %t2 | grep "remark: building module 'MyHeader2'" | cut -d "'" -f 4 \
+// RUN:  | while read -r MODULE; do llvm-bcanalyzer --dump --disable-histogram \
+// RUN:  $MODULE | grep "AST_SIGNATURE" > %t1.sig; done
+// RUN: diff %t1.sig %t2.sig
+
+#include "my_header_2.h"
+
+my_int var = 42;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80296: [clangd] Don't traverse the AST within uninteresting files during indexing

[Sam McCall via Phabricator via cfe-commits]

sammccall added a subscriber: adamcz.
sammccall added a comment.

@adamcz I think this is relevant to your interests maybe? :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80296



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

[PATCH] D80301: [yaml][clang-tidy] Fix new line YAML serialization

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin created this revision.
DmitryPolukhin added reviewers: gribozavr, mgehre, yvvan.
DmitryPolukhin added projects: clang-tools-extra, clang.
Herald added subscribers: hiraditya, xazax.hun.
Herald added a project: LLVM.
DmitryPolukhin edited the summary of this revision.

Move new line duplication logic to YMAL string
serialization/deserilization level instead of Replacement that was
introduced in D63482 . D63482 
 led to duplicated
new lines if you apply replacements with clang-apply-replacements. New
line duplication happened only during serialisation and there was no
opposite transformation in deserialization.

Test Plan: check-all


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80301

Files:
  clang/include/clang/Tooling/ReplacementsYaml.h
  llvm/lib/Support/YAMLTraits.cpp
  llvm/unittests/Support/YAMLIOTest.cpp


Index: llvm/unittests/Support/YAMLIOTest.cpp
===
--- llvm/unittests/Support/YAMLIOTest.cpp
+++ llvm/unittests/Support/YAMLIOTest.cpp
@@ -285,7 +285,7 @@
 YOut << Original;
   }
   auto Expected = "---\n"
-  "str1:'a multiline string\n"
+  "str1:'a multiline string\n\n"
   "foobarbaz'\n"
   "str2:'another one\r"
   "foobarbaz'\n"
Index: llvm/lib/Support/YAMLTraits.cpp
===
--- llvm/lib/Support/YAMLTraits.cpp
+++ llvm/lib/Support/YAMLTraits.cpp
@@ -887,13 +887,32 @@
 }
 
 void ScalarTraits::output(const std::string &Val, void *,
- raw_ostream &Out) {
-  Out << Val;
+   raw_ostream &Out) {
+  size_t currentPos = 0;
+  size_t lineBreakPos = Val.find('\n');
+  while (lineBreakPos != std::string::npos) {
+Out << StringRef(&Val[currentPos], lineBreakPos - currentPos + 1);
+Out << '\n';
+currentPos = lineBreakPos + 1;
+lineBreakPos = Val.find('\n', currentPos);
+  }
+  Out << StringRef(&Val[currentPos], Val.size() - currentPos);
 }
 
 StringRef ScalarTraits::input(StringRef Scalar, void *,
- std::string &Val) {
-  Val = Scalar.str();
+   std::string &Val) {
+  Val.clear();
+  size_t currentPos = 0;
+  size_t lineBreakPos = Scalar.find('\n');
+  while (lineBreakPos != std::string::npos) {
+// '\n\n' convert to '\n' and don't copy single '\n'.
+if (currentPos + 1 < Scalar.size() && Scalar[lineBreakPos + 1] == '\n')
+  ++lineBreakPos;
+Val += Scalar.substr(currentPos, lineBreakPos);
+currentPos = lineBreakPos + 1;
+lineBreakPos = Scalar.find('\n', currentPos);
+  }
+  Val += Scalar.substr(currentPos, Scalar.size() - currentPos);
   return StringRef();
 }
 
Index: clang/include/clang/Tooling/ReplacementsYaml.h
===
--- clang/include/clang/Tooling/ReplacementsYaml.h
+++ clang/include/clang/Tooling/ReplacementsYaml.h
@@ -35,13 +35,7 @@
 
 NormalizedReplacement(const IO &, const clang::tooling::Replacement &R)
 : FilePath(R.getFilePath()), Offset(R.getOffset()),
-  Length(R.getLength()), ReplacementText(R.getReplacementText()) {
-  size_t lineBreakPos = ReplacementText.find('\n');
-  while (lineBreakPos != std::string::npos) {
-ReplacementText.replace(lineBreakPos, 1, "\n\n");
-lineBreakPos = ReplacementText.find('\n', lineBreakPos + 2);
-  }
-}
+  Length(R.getLength()), ReplacementText(R.getReplacementText()) {}
 
 clang::tooling::Replacement denormalize(const IO &) {
   return clang::tooling::Replacement(FilePath, Offset, Length,


Index: llvm/unittests/Support/YAMLIOTest.cpp
===
--- llvm/unittests/Support/YAMLIOTest.cpp
+++ llvm/unittests/Support/YAMLIOTest.cpp
@@ -285,7 +285,7 @@
 YOut << Original;
   }
   auto Expected = "---\n"
-  "str1:'a multiline string\n"
+  "str1:'a multiline string\n\n"
   "foobarbaz'\n"
   "str2:'another one\r"
   "foobarbaz'\n"
Index: llvm/lib/Support/YAMLTraits.cpp
===
--- llvm/lib/Support/YAMLTraits.cpp
+++ llvm/lib/Support/YAMLTraits.cpp
@@ -887,13 +887,32 @@
 }
 
 void ScalarTraits::output(const std::string &Val, void *,
- raw_ostream &Out) {
-  Out << Val;
+   raw_ostream &Out) {
+  size_t currentPos = 0;
+  size_t lineBreakPos = Val.find('\n');
+  while (lineBreakPos != std::string::npos) {
+Out << StringRef(&Val[currentPos], lineBreakPos - currentPos + 1);
+Out << '\n';
+currentPos = lineBre

[PATCH] D79998: Add AST_SIGNATURE record to unhashed control block of pcm files (Patch series 2/3)

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 265263.
dang added a comment.

Uploaded the wrong diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79998

Files:
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/ASTSignature.c
  clang/test/Modules/Inputs/ASTHash/module.modulemap
  clang/test/Modules/Inputs/ASTHash/my_header_1.h
  clang/test/Modules/Inputs/ASTHash/my_header_2.h

Index: clang/test/Modules/Inputs/ASTHash/my_header_2.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/my_header_2.h
@@ -0,0 +1,3 @@
+#include "my_header_1.h"
+
+extern my_int var;
Index: clang/test/Modules/Inputs/ASTHash/my_header_1.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/my_header_1.h
@@ -0,0 +1 @@
+typedef int my_int;
Index: clang/test/Modules/Inputs/ASTHash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/module.modulemap
@@ -0,0 +1,8 @@
+module MyHeader1 {
+  header "my_header_1.h"
+}
+
+module MyHeader2 {
+  header "my_header_2.h"
+  export *
+}
Index: clang/test/Modules/ASTSignature.c
===
--- /dev/null
+++ clang/test/Modules/ASTSignature.c
@@ -0,0 +1,18 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -iquote %S/Inputs/ASTHash/ -fsyntax-only -fmodules \
+// RUN:   -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN:   -fmodules-cache-path=%t %s -Rmodule-build 2> %t1
+// RUN: %clang_cc1 -iquote "/dev/null" -iquote %S/Inputs/ASTHash/ -fsyntax-only \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN:   -fmodules-cache-path=%t %s -Rmodule-build 2> %t2
+// RUN: cat %t1 | grep "remark: building module 'MyHeader2'" | cut -d "'" -f 4 \
+// RUN:  | while read -r MODULE; do llvm-bcanalyzer --dump --disable-histogram \
+// RUN:  $MODULE | grep "AST_SIGNATURE" > %t1.sig; done
+// RUN: cat %t2 | grep "remark: building module 'MyHeader2'" | cut -d "'" -f 4 \
+// RUN:  | while read -r MODULE; do llvm-bcanalyzer --dump --disable-histogram \
+// RUN:  $MODULE | grep "AST_SIGNATURE" > %t1.sig; done
+// RUN: diff %t1.sig %t2.sig
+
+#include "my_header_2.h"
+
+my_int var = 42;
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -64,6 +64,7 @@
 #include "clang/Sema/ObjCMethodList.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/Weak.h"
+#include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/ASTRecordWriter.h"
 #include "clang/Serialization/InMemoryModuleCache.h"
@@ -961,6 +962,7 @@
 
   BLOCK(UNHASHED_CONTROL_BLOCK);
   RECORD(SIGNATURE);
+  RECORD(AST_SIGNATURE);
   RECORD(DIAGNOSTIC_OPTIONS);
   RECORD(DIAG_PRAGMA_MAPPINGS);
 
@@ -1044,8 +1046,9 @@
   return Signature;
 }
 
-ASTFileSignature ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
-  ASTContext &Context) {
+ASTFileSignature ASTWriter::writeUnhashedControlBlock(
+Preprocessor &PP, ASTContext &Context,
+const std::pair ASTBlockRange) {
   // Flush first to prepare the PCM hash (signature).
   Stream.FlushToWord();
   auto StartOfUnhashedControl = Stream.GetCurrentBitNo() >> 3;
@@ -1062,6 +1065,11 @@
 Record.append(Signature.begin(), Signature.end());
 Stream.EmitRecord(SIGNATURE, Record);
 Record.clear();
+auto ASTSignature = createSignature(
+StringRef(Buffer.begin() + ASTBlockRange.first, ASTBlockRange.second));
+Record.append(ASTSignature.begin(), ASTSignature.end());
+Stream.EmitRecord(AST_SIGNATURE, Record);
+Record.clear();
   }
 
   // Diagnostic options.
@@ -4548,6 +4556,8 @@
   populateInputFileIDs(Context.SourceMgr);
 
   // Write the remaining AST contents.
+  Stream.FlushToWord();
+  auto StartOfASTBlock = Stream.GetCurrentBitNo() >> 3;
   Stream.EnterSubblock(AST_BLOCK_ID, 5);
 
   // This is so that older clang versions, before the introduction
@@ -4679,9 +4689,9 @@
 //   c++-base-specifiers-id:i32
 //   type-id:i32)
 //
-// module-kind is the ModuleKind enum value. If it is MK_PrebuiltModule or
-// MK_ExplicitModule, then the module-name is the module name. Otherwise,
-// it is the module file name.
+// module-kind is the ModuleKind enum value. If it is MK_PrebuiltModule,
+// MK_ExplicitModule or MK_ImplicitModule, then the module-name is the
+// module name. Otherwise, it is the module file name.
 auto Abbrev = s

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D79773#2046833 , @miscco wrote:

> It seems the spacing of the binary operator is not yet stable. This test is 
> breaking for me:
>
>   verifyFormat("template \nconcept someConcept = Constraint1 
> && Constraint2;");
>


Hmm... are you upto date

  $ ./FormatTests.exe --gtest_filter=*Concept*
  Note: Google Test filter = *Concept*
  [==] Running 1 test from 1 test case.
  [--] Global test environment set-up.
  [--] 1 test from FormatTest
  [ RUN  ] FormatTest.ConceptsAndRequires
  [   OK ] FormatTest.ConceptsAndRequires (642 ms)
  [--] 1 test from FormatTest (645 ms total)
  
  [--] Global test environment tear-down
  [==] 1 test from 1 test case ran. (650 ms total)
  [  PASSED  ] 1 test.


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

https://reviews.llvm.org/D79773



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

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

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

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

This will take a while for me to fix (couple hours, wanna wait for creduce to 
finish running, and I needed to compile llvm on the server as well), but I'll 
get it done probably today.




Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1194
-  if (!FD)
+  if (!Call.getOriginExpr())
 return;
 

balazske wrote:
> Szelethus wrote:
> > Szelethus wrote:
> > > balazske wrote:
> > > > Szelethus wrote:
> > > > > Szelethus wrote:
> > > > > > balazske wrote:
> > > > > > > This should be added to avoid later crash (probably not needed 
> > > > > > > for every check kind?):
> > > > > > > ```
> > > > > > >   const FunctionDecl *FD = C.getCalleeDecl(CE);
> > > > > > >   if (!FD)
> > > > > > > return;
> > > > > > > ```
> > > > > > Not all `CallEvent`s have a corresponding `FunctionDecl` or a 
> > > > > > `CallExpr`, for instance, `CXXAllocatorCall` corresponds with 
> > > > > > `CXXNewExpr`, which is not a `CallExpr`, but it is handled by this 
> > > > > > checker. For this reason, I decided to move this check to the 
> > > > > > individual modeling functions.
> > > > > Oh I'm sorry, do we have an actual crash resulting from this? 
> > > > I did not look into it by detail but the problem is in 
> > > > `MallocChecker::checkOwnershipAttr` with a null `FD`. Probably it is 
> > > > enough to insert a return at that point (makes the crash gone on that 
> > > > analyzed project).
> > > I'm sorry but I don't have enough to follow up on this. On the master 
> > > branch, everything seems to work smoothly for me (all tests pass) and I 
> > > didn't get any buildbot failures. Can you post a code snippet?
> > If you have a project name, I can analyze that myself to save you the 
> > trouble.
> Could not debug the problem because debug info was not there for unknown 
> reason (no exact line locations were printed). But the project was emacs 26.3 
> `configure --with-xpm=no --with-gif=no --with-tiff=no --with-gnutls=no`, 
> failed at one of the the first files with CodeChecker analyze.
No worries, I'll attend to it myself. Thank you for the quick response though!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75432



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

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

LGTM, but leaving for one of the other reviewers to give you the official 
"Accept Revision".




Comment at: 
clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp:72
+   binaryOperator(hasOperatorName("=="),
+  hasEitherOperand(ignoringParenImpCasts(StringNpos)),
+  hasEitherOperand(ignoringParenImpCasts(StringFind))),

tdl-g wrote:
> njames93 wrote:
> > ymandel wrote:
> > > Would `hasOperands` replace these two separate calls to 
> > > `hasEitherOperand`? Same below lines 79-80 (I think it's a new matcher, 
> > > fwiw).
> > Just added, I added it to remove calls of multiple hasEitherOperand calls 
> > for the reason of preventing the matchers matching the same operand. I just 
> > got a little lost in my previous comment
> Switched to hasOperands.  I agree that expresses the intent better than two 
> independent calls of hasEitherOperand.
> 
> Note that it was working fine before--the test cases confirmed that it wasn't 
> matching string::npos == string::npos or s.find(a) == s.find(a).  I'm 
> guessing that's because the matchers inside hasEitherOperand have bindings, 
> and if the matcher matched but one of the bindings was missing, the 
> rewriterule suppressed itself?  Is this a plausible theory?
> 
> (The question moot since hasOperands is better overall, but I'd like to 
> understand why it was working before.)
I was wondering the same when I noticed this. I think its something like this: 
one of the arguments falls into bucket 1, one of the arguments falls into 
bucket 2.  Since a single argument cannot simultaneously fall into two buckets 
(the predicates are mutually exlusive), it must be the case that the arguments 
are different.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80023



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

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin added a comment.

Sent D80301  for review.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63482



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

[PATCH] D77062: [analyzer] Added check for unacceptable equality operation between Loc and NonLoc types

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

Folk,
please look at this patch. It has been hanging for a time here. We should 
finally make a decision about it.


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] D80117: [analyzer] Introduce reasoning about symbolic remainder operator

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@vsavchenko
I've made some assumptions.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:459-469
+if (Origin.From().isMinSignedValue()) {
+  // If mini is a minimal signed value, absolute value of it is greater
+  // than the maximal signed value.  In order to avoid these
+  // complications, we simply return the whole range.
+  return {ValueFactory.getMinValue(RangeType),
+  ValueFactory.getMaxValue(RangeType)};
+}

I think you should swap `if` statements. I'll explain.
Let's consider the input is an **uint8** range [42, 242] and you will return 
[0, 242] in the second `if`.
But if the input is an **uint8** range [128, 242] you will return [0, 255] in 
the first `if`, because 128 is an equivalent of -128(INT8_MIN) in binary 
representation so the condition in the first if would be true.
What is the great difference between [42, 242] and [128, 242] to have different 
results? Or you've just missed this case?

P.S. I think your function's name doesn't fit its body, since //absolute 
value// is always positive (without sign) from its definition, but you output 
range may have negative values. You'd better write an explanation above the 
function and rename it.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:481
+//   * Otherwise, From <= 0, To >= 0, and
+// AbsMax == max(abs(From), abs(To))
+llvm::APSInt AbsMax = std::max(-Origin.From(), Origin.To());

As for me, the last  //reason// fully covers previous special cases, so you can 
omit those ones, thus simplify the comment.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:659
+  //
+  // If we are dealing with unsigned case, we shouldn't move the lower bound.
+  if (Min.isSigned()) {

Extend the comment, please, why we should move bounds to zero at all.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:684
+
+  return {RangeFactory, ValueFactory.getValue(Min), 
ValueFactory.getValue(Max)};
+}

Is it OK to return this rangeset in case when one of operands(or both) is 
negative, since this rangeset can vary from specific implementation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80117



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

[PATCH] D80198: [clangd] locateMacroAt handles patched macros

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/SourceCode.cpp:975
+
+  // Macro definitions could be injected through preamble patch. These contain
+  // line directives to hint their original location in main file.

OK, this is fairly horrible...

I want to say the preamble patch location translation should be a separate 
function rather than coupled to macro-specific stuff.
(Then we don't even need the extra struct member, but we can still keep it to 
"remind" people the translation is needed, if we like).

But of course we didn't preserve the formatting in the preamble patch, so 
`getPresumedLoc()` doesn't give us the right location, it just gives us 
something on the right line that we then need to re-parse...

This really isn't looking like a great tradeoff to me anymore (and I know I 
suggested it).
How much work do you think it is (compared to say, the logic here plus doing it 
in ~2 more places) to give the preamblepatch the invariant that the PresumedLoc 
has a usable col as well.
The original implementation did padding with spaces, but I guess we could as 
well have the preamble patching stuff splice the actual source code...



Comment at: clang-tools-extra/clangd/SourceCode.h:295
   const MacroInfo *Info;
+  /// Subject to #line directive substitution. E.g. a macro defined in a
+  /// built-in file might have an identifier location inside the main file.

It's not obvious to me that this comment describes a special case but the field 
is valid in general.
Maybe
```
/// Location of the identifier that names the macro.
/// Unlike Info->Location, this translates preamble-patch locations to 
main-file locations.
```



Comment at: clang-tools-extra/clangd/SourceCode.h:297
+  /// built-in file might have an identifier location inside the main file.
+  SourceLocation IdentLoc;
 };

nit: call this NameLoc or NameLocation?
ident is *slightly* vague


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80198



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

[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5559
+  case Decl::Typedef:
+if (getCodeGenOpts().DebugUnusedTypes)
+  if (CGDebugInfo *DI = getModuleDebugInfo())

dblaikie wrote:
> Probably test this within the implementation of CGDebugInfo? & rename the 
> EmitTypedef function to something that clarifies that it's for an otherwise 
> unused type?
> 
> But that function might need to be generalized further, rather than only 
> having it for typedefs. (see general comment above)
I think it makes sense at this point to rename `EmitExplicitCastType` to 
`RetainType` or `EmitType`.  WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80242



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

[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

probably should add a test for `enum class` in C++


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80242



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

[clang] 60ee885 - [clang][asm goto][slh] Warn if asm goto + SLH

[Zola Bridges via cfe-commits]

Author: Zola Bridges
Date: 2020-05-20T09:46:18-07:00
New Revision: 60ee885990982197013c71ff965a81e938184fd2

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

LOG: [clang][asm goto][slh] Warn if asm goto + SLH

Summary:
Asm goto is not supported by SLH. Warn if an instance of asm goto is detected
while SLH is enabled.

Test included.

Reviewed By: jyu2

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

Added: 
clang/test/Parser/slh-asm-goto-no-warn.cpp
clang/test/Parser/slh-asm-goto.cpp

Modified: 
clang/include/clang/Basic/DiagnosticCommonKinds.td
clang/lib/Parse/ParseStmtAsm.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td 
b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 73f55784234e..65e3755efd22 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -246,6 +246,11 @@ let CategoryName = "Inline Assembly Issue" in {
   def warn_stack_clash_protection_inline_asm : Warning<
 "Unable to protect inline asm that clobbers stack pointer against stack 
clash">,
 InGroup>;
+
+  def warn_slh_does_not_support_asm_goto
+  : Warning<"Speculative load hardening does not protect functions with "
+"asm goto">,
+InGroup>;
 }
 
 // Sema && Serialization

diff  --git a/clang/lib/Parse/ParseStmtAsm.cpp 
b/clang/lib/Parse/ParseStmtAsm.cpp
index 262def2b38a1..0f21037b6100 100644
--- a/clang/lib/Parse/ParseStmtAsm.cpp
+++ b/clang/lib/Parse/ParseStmtAsm.cpp
@@ -729,6 +729,9 @@ StmtResult Parser::ParseAsmStatement(bool &msAsm) {
   if (parseGNUAsmQualifierListOpt(GAQ))
 return StmtError();
 
+  if (GAQ.isGoto() && getLangOpts().SpeculativeLoadHardening)
+Diag(Loc, diag::warn_slh_does_not_support_asm_goto);
+
   BalancedDelimiterTracker T(*this, tok::l_paren);
   T.consumeOpen();
 

diff  --git a/clang/test/Parser/slh-asm-goto-no-warn.cpp 
b/clang/test/Parser/slh-asm-goto-no-warn.cpp
new file mode 100644
index ..b8cfecbdb159
--- /dev/null
+++ b/clang/test/Parser/slh-asm-goto-no-warn.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -Wno-slh-asm-goto -mspeculative-load-hardening 
-fsyntax-only -verify %s
+
+void f() {
+  __asm goto("movl %ecx, %edx"); // expected-no-diagnostics
+}

diff  --git a/clang/test/Parser/slh-asm-goto.cpp 
b/clang/test/Parser/slh-asm-goto.cpp
new file mode 100644
index ..7ae3cbdb13e6
--- /dev/null
+++ b/clang/test/Parser/slh-asm-goto.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -mspeculative-load-hardening -fsyntax-only -verify %s
+
+void f() {
+  __asm goto("movl %ecx, %edx"); // expected-warning {{Speculative load 
hardening does not protect functions with asm goto}}
+}



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

[PATCH] D80309: [clang-format][docfix] Update predefined styles in docs

[Jake Merdich via Phabricator via cfe-commits]

JakeMerdichAMD created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
JakeMerdichAMD added reviewers: MyDeveloperDay, krasimir, mitchell-stellar, 
sammccall.
JakeMerdichAMD added a project: clang-format.

The predefined styles that clang-format supports are listed in two
places, and neither is up-to-date. GNU style isn't mentioned at all!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80309

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/LibFormat.rst


Index: clang/docs/LibFormat.rst
===
--- clang/docs/LibFormat.rst
+++ clang/docs/LibFormat.rst
@@ -40,7 +40,7 @@
 
 The style options describe specific formatting options that can be used in
 order to make `ClangFormat` comply with different style guides. Currently,
-two style guides are hard-coded:
+several style guides are hard-coded:
 
 .. code-block:: c++
 
@@ -52,6 +52,26 @@
   /// http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml.
   FormatStyle getGoogleStyle();
 
+  /// Returns a format style complying with Chromium's style guide:
+  /// 
https://chromium.googlesource.com/chromium/src/+/master/styleguide/styleguide.md
+  FormatStyle getChromiumStyle();
+
+  /// Returns a format style complying with the GNU coding standards:
+  /// https://www.gnu.org/prep/standards/standards.html
+  FormatStyle getGNUStyle();
+
+  /// Returns a format style complying with Mozilla's style guide
+  /// 
https://firefox-source-docs.mozilla.org/code-quality/coding-style/index.html
+  FormatStyle getMozillaStyle();
+
+  /// Returns a format style complying with Webkit's style guide:
+  /// https://webkit.org/code-style-guidelines/
+  FormatStyle getWebkitStyle();
+
+  /// Returns a format style complying with Microsoft's style guide:
+  /// 
https://docs.microsoft.com/en-us/visualstudio/ide/editorconfig-code-style-settings-reference
+  FormatStyle getMicrosoftStyle();
+
 These options are also exposed in the :doc:`standalone tools `
 through the `-style` option.
 
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -151,6 +151,9 @@
   * ``Microsoft``
 A style complying with `Microsoft's style guide
 
`_
+  * ``GNU``
+A style complying with the `GNU coding standards
+`_
 
 .. START_FORMAT_STYLE_OPTIONS
 


Index: clang/docs/LibFormat.rst
===
--- clang/docs/LibFormat.rst
+++ clang/docs/LibFormat.rst
@@ -40,7 +40,7 @@
 
 The style options describe specific formatting options that can be used in
 order to make `ClangFormat` comply with different style guides. Currently,
-two style guides are hard-coded:
+several style guides are hard-coded:
 
 .. code-block:: c++
 
@@ -52,6 +52,26 @@
   /// http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml.
   FormatStyle getGoogleStyle();
 
+  /// Returns a format style complying with Chromium's style guide:
+  /// https://chromium.googlesource.com/chromium/src/+/master/styleguide/styleguide.md
+  FormatStyle getChromiumStyle();
+
+  /// Returns a format style complying with the GNU coding standards:
+  /// https://www.gnu.org/prep/standards/standards.html
+  FormatStyle getGNUStyle();
+
+  /// Returns a format style complying with Mozilla's style guide
+  /// https://firefox-source-docs.mozilla.org/code-quality/coding-style/index.html
+  FormatStyle getMozillaStyle();
+
+  /// Returns a format style complying with Webkit's style guide:
+  /// https://webkit.org/code-style-guidelines/
+  FormatStyle getWebkitStyle();
+
+  /// Returns a format style complying with Microsoft's style guide:
+  /// https://docs.microsoft.com/en-us/visualstudio/ide/editorconfig-code-style-settings-reference
+  FormatStyle getMicrosoftStyle();
+
 These options are also exposed in the :doc:`standalone tools `
 through the `-style` option.
 
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -151,6 +151,9 @@
   * ``Microsoft``
 A style complying with `Microsoft's style guide
 `_
+  * ``GNU``
+A style complying with the `GNU coding standards
+`_
 
 .. START_FORMAT_STYLE_OPTIONS
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80214: [clang-format] Set of unit test to begin to validate that we don't change defaults

[Jake Merdich via Phabricator via cfe-commits]

JakeMerdichAMD added a comment.

Just belatedly caught something: Webkit style is supported too but not listed 
here. Can you add that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80214



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