[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 253394.
Charusso marked 19 inline comments as done.
Charusso edited the summary of this revision.
Charusso added a comment.
Herald added a subscriber: ASDenysPetrov.

- Fix `VLASizeChecker`'s multi-dimensional array early return.
- So that fix the regression in test `misc-ps-region-store.m`.
- Fix tests that need regex.
- Add documentation about `dumpExtent`, `dumpElementCount`.


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

https://reviews.llvm.org/D69726

Files:
  clang/docs/analyzer/developer-docs/DebugChecks.rst
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
  clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
  clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/test/Analysis/explain-svals.cpp
  clang/test/Analysis/expr-inspection.cpp
  clang/test/Analysis/memory-model.cpp

Index: clang/test/Analysis/memory-model.cpp
===
--- /dev/null
+++ clang/test/Analysis/memory-model.cpp
@@ -0,0 +1,114 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix,cplusplus,debug.ExprInspection \
+// RUN:  -triple x86_64-unknown-linux-gnu \
+// RUN:  -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+typedef __SIZE_TYPE__ size_t;
+void *malloc(size_t);
+void *alloca(size_t);
+void *realloc(void *ptr, size_t size);
+void *calloc(size_t number, size_t size);
+void free(void *);
+
+struct S { int f; };
+
+void clang_analyzer_dump(int);
+void clang_analyzer_dump(const void *);
+void clang_analyzer_dumpExtent(int);
+void clang_analyzer_dumpExtent(const void *);
+void clang_analyzer_dumpElementCount(int);
+void clang_analyzer_dumpElementCount(const void *);
+
+void var_simple_ref() {
+  int a = 13;
+  clang_analyzer_dump(); // expected-warning {{a}}
+  clang_analyzer_dumpExtent(); // expected-warning {{4 S64b}}
+  clang_analyzer_dumpElementCount(); // expected-warning {{1 S64b}}
+}
+
+void var_simple_ptr(int *a) {
+  clang_analyzer_dump(a); // expected-warning {{SymRegion{reg_$0}}}
+  clang_analyzer_dumpExtent(a); // expected-warning {{extent_$1{SymRegion{reg_$0
+  clang_analyzer_dumpElementCount(a); // expected-warning-re ^\(extent_\$1\{SymRegion\{reg_\$0\}\}\) / 4$
+}
+
+void var_array() {
+  int a[] = {1, 2, 3};
+  clang_analyzer_dump(a); // expected-warning {{Element{a,0 S64b,int}}}
+  clang_analyzer_dumpExtent(a); // expected-warning {{12 S64b}}
+  clang_analyzer_dumpElementCount(a); // expected-warning {{3 S64b}}
+}
+
+void string() {
+  clang_analyzer_dump("foo"); // expected-warning {{Element{"foo",0 S64b,char}}}
+  clang_analyzer_dumpExtent("foo"); // expected-warning {{4 S64b}}
+  clang_analyzer_dumpElementCount("foo"); // expected-warning {{4 S64b}}
+}
+
+void struct_simple_ptr(S *a) {
+  clang_analyzer_dump(a); // expected-warning {{SymRegion{reg_$0}}}
+  clang_analyzer_dumpExtent(a); // expected-warning {{extent_$1{SymRegion{reg_$0
+  clang_analyzer_dumpElementCount(a); // expected-warning-re ^\(extent_\$1\{SymRegion\{reg_\$0\}\}\) / 4$
+}
+
+void field_ref(S a) {
+  clang_analyzer_dump(); // expected-warning {{a.f}}
+  clang_analyzer_dumpExtent(); // expected-warning {{4 S64b}}
+  clang_analyzer_dumpElementCount(); // expected-warning {{1 S64b}}
+}
+
+void field_ptr(S *a) {
+  clang_analyzer_dump(>f); // expected-warning {{SymRegion{reg_$0}.f}}
+  clang_analyzer_dumpExtent(>f); // expected-warning {{4 S64b}}
+  clang_analyzer_dumpElementCount(>f); // expected-warning {{1 S64b}}
+}
+
+void symbolic_array() {
+  int *a = new int[3];
+  clang_analyzer_dump(a); // expected-warning {{Element{HeapSymRegion{conj}}
+  clang_analyzer_dumpExtent(a); // expected-warning {{12 S64b}}
+  clang_analyzer_dumpElementCount(a); // expected-warning {{3 S64b}}
+  delete[] a;
+}
+
+void symbolic_placement_new() {
+  char *buf = new char[sizeof(int) * 3];
+  int *a = new (buf) int(13);
+  clang_analyzer_dump(a); // expected-warning {{Element{HeapSymRegion{conj}}
+  clang_analyzer_dumpExtent(a); // expected-warning {{12 S64b}}
+  clang_analyzer_dumpElementCount(a); // expected-warning {{3 S64b}}
+  delete[] buf;
+}
+
+void symbolic_malloc() {
+  int *a = (int *)malloc(12);
+  clang_analyzer_dump(a); // expected-warning {{Element{HeapSymRegion{conj}}
+  clang_analyzer_dumpExtent(a); // expected-warning {{12 U64b}}
+  clang_analyzer_dumpElementCount(a); // expected-warning {{3 S64b}}
+  free(a);
+}
+
+void symbolic_alloca() {
+  int *a = (int *)alloca(12);
+  clang_analyzer_dump(a); // expected-warning {{Element{HeapSymRegion{conj}}
+  clang_analyzer_dumpExtent(a); // expected-warning {{12 U64b}}
+  

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h:25
 
-/// Get the stored dynamic size for the region \p MR.
+/// \returns The stored dynamic size for the region \p MR.
 DefinedOrUnknownSVal getDynamicSize(ProgramStateRef State, const MemRegion *MR,

NoQ wrote:
> Naming: I believe we should keep using the word "Extent". There's no need to 
> introduce a new term for the existing concept; it makes it harder to explain 
> on the mailing list :) Let's make a follow-up patch to change the naming back 
> (so that not to screw the review).
Since then I have changed my mind when I have read about Environment and Store 
in a book. Sure, next up.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h:41-44
+/// Set the dynamic size of a CXXNewExpr \p NE by its region \p MR.
+ProgramStateRef setDynamicSize(ProgramStateRef State, const MemRegion *MR,
+   const CXXNewExpr *NE,
+   const LocationContext *LCtx, SValBuilder );

NoQ wrote:
> This function is probably going to be used exactly once in the whole code. 
> There's no need to turn it into a public API.
It is being used 3 times already, so I believe it is a cool API.



Comment at: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:99
+  .Case("clang_analyzer_region", 
::analyzerRegion)
+  .Case("clang_analyzer_size", 
::analyzerDumpSize)
+  .Case("clang_analyzer_elementCount",

NoQ wrote:
> `clang_analyzer_dump_extent()`? Or just 
> `clang_analyzer_dump(clang_analyzer_getExtent())`.
I like the shorter version, but of course I have seen the longer version.



Comment at: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:283
+  llvm::raw_svector_ostream Out(Msg);
+  Out << MR;
+  reportBug(Out.str(), C);

NoQ wrote:
> So, how is it different from the existing `clang_analyzer_dump()`?
I wanted to make it for the derived regions, but then I have realized I am lazy 
to dig into the buggier parts of the Analyzer. Removed.



Comment at: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:310-311
+
+  QualType Ty = TVR ? TVR->getDesugaredValueType(C.getASTContext())
+: CE->getArg(0)->IgnoreParenImpCasts()->getType();
+

NoQ wrote:
> How is this better than `getValueType()`? Are you sure you're not getting a 
> pointer type instead in the `!TVR` case? I.e., can you test this on a 
> non-heap `SymRegion`?
> How is this better than getValueType()?
Consistency. We get the static ~~size~~ extent by getting the desugared type 
which most likely just an extra overhead.

> Are you sure you're not getting a pointer type instead in the !TVR case? 
> I.e., can you test this on a non-heap SymRegion?
The issue was the `var_region_simple_ptr` test: we need to use regex for 
checking after the `}}` token where `/ 4` is the correct result. I did not 
really get why the test pass.



Comment at: clang/lib/StaticAnalyzer/Core/DynamicSize.cpp:85
+if (CI->getValue().isUnsigned())
+  Size = SVB.makeIntVal(CI->getValue(), /*IsUnsigned=*/false);
+

NoQ wrote:
> Charusso wrote:
> > That one is interesting. Some of the checkers / SValBuilder(?) generate 
> > unsigned integers which should not happen, I believe. May we need a FIXME 
> > and an assertion about signedness. What do you think?
> `SymbolExtent` has type `size_t` and both `malloc` and `operator new` accept 
> `size_t` as a parameter. Therefore everything needs to be //unsigned// and we 
> need to assert this.
> 
> That said, array indexes are //signed// (as per implementation of 
> `ElementRegion`).
It was a premature optimization for consistency. I will leave the signedness as 
is, FIXME added.



Comment at: clang/test/Analysis/misc-ps-region-store.m:1190
+tmp2[x] = am; // expected-warning \
+{{Access out-of-bound array element (buffer overflow)}}
   }

NoQ wrote:
> Charusso wrote:
> > That is the single regression which I do not get.
> Well, please debug. Like, look at the full report, dump egraph, see what 
> changed. Try to `creduce` the example further under the condition "behavior 
> changed with the patch", maybe that'll clear something up (though if it's a 
> true positive after creduce it doesn't guarantee that it's a true positive 
> before creduce).
Given that I have no access for `rdar` I did my hand-reduce and the solution is 
the following:
```lang=c
// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin9 \
// RUN:  -analyzer-checker=core,alpha.security.ArrayBound \
// RUN:  -verify %s

static void RDar8424269_B(int unknown) {
  unsigned char tmp2t[1][unknown];
  tmp2t[1][13] = 0; // expected-warning \
{{Access out-of-bound array element (buffer 

[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

[Vy Nguyen via Phabricator via cfe-commits]

oontvoo updated this revision to Diff 253388.
oontvoo added a comment.

Add more tests (From Bug 39206)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/Inputs/dummy_pragma_once.h
  clang/test/Modules/Inputs/dummy_textual_header.h
  clang/test/Modules/Inputs/header_in_imported_module.h
  clang/test/Modules/Inputs/imported_module.cppm
  clang/test/Modules/Inputs/module.map
  clang/test/Modules/header-in-imported-module.c
  clang/test/Modules/import-pragma-once.c

Index: clang/test/Modules/import-pragma-once.c
===
--- /dev/null
+++ clang/test/Modules/import-pragma-once.c
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -I%S/Inputs -verify -x c %s
+// expected-no-diagnostics
+#include "dummy_pragma_once.h"
+#include "dummy_textual_header.h"
+
+void *p = 
+void *q = 
Index: clang/test/Modules/header-in-imported-module.c
===
--- /dev/null
+++ clang/test/Modules/header-in-imported-module.c
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -I%S/Inputs -verify  -x c++ %s
+//
+// RUN: %clang_cc1 -x c++ -fmodules-ts --precompile -o %t/ModuleB39206.pcm %S/Inputs/imported_module.cppm
+// RUN: %clang_cc1 -x c++ -fmodules-ts -c %t/ModuleB39206.pcm -o %t/ModuleB39206.o
+// RUN: %clang_cc1 -x c++ -fmodules-ts -fmodule-file=%t/ModuleB39206.pcm -verify  -c %s
+// expected-no-diagnostics
+
+// Bug 39206
+
+#include "header_in_imported_module.h"
+module ModuleB39206;
+
+#ifndef ALL_GOOD
+#error "missing macro in impl"
+#endif
Index: clang/test/Modules/Inputs/module.map
===
--- clang/test/Modules/Inputs/module.map
+++ clang/test/Modules/Inputs/module.map
@@ -282,6 +282,11 @@
   header "dummy.h"
 }
 
+module dummy_pragma_once {
+  header "dummy_pragma_once.h"
+  textual header "dummy_textual_header.h"
+}
+
 module builtin {
   header "builtin.h"
   explicit module sub {
Index: clang/test/Modules/Inputs/imported_module.cppm
===
--- /dev/null
+++ clang/test/Modules/Inputs/imported_module.cppm
@@ -0,0 +1,5 @@
+export module ModuleB39206;
+#include "header.h"
+
+#ifndef ALL_GOOD
+#error "Missing macro in module decl"
\ No newline at end of file
Index: clang/test/Modules/Inputs/header_in_imported_module.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/header_in_imported_module.h
@@ -0,0 +1,3 @@
+#pragma once
+
+#define ALL_GOOD
Index: clang/test/Modules/Inputs/dummy_textual_header.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/dummy_textual_header.h
@@ -0,0 +1,2 @@
+#pragma once
+int y = 6;
Index: clang/test/Modules/Inputs/dummy_pragma_once.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/dummy_pragma_once.h
@@ -0,0 +1,3 @@
+#include "dummy_textual_header.h"
+
+int x = 5;
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1621,7 +1621,7 @@
   endian::Writer LE(Out, little);
   unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
   LE.write(KeyLen);
-  unsigned DataLen = 1 + 2 + 4 + 4;
+  unsigned DataLen = 1 + 2 + 4 + 4 + 4;
   for (auto ModInfo : Data.KnownHeaders)
 if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
   DataLen += 4;
@@ -1678,6 +1678,9 @@
   }
   LE.write(Offset);
 
+  // Write this file UID.
+  LE.write(Data.HFI.UID);
+
   auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
 if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
   uint32_t Value = (ModID << 2) | (unsigned)Role;
@@ -1705,7 +1708,7 @@
 /// Write the header search block for the list of files that
 ///
 /// \param HS The header search structure to save.
-void ASTWriter::WriteHeaderSearch(const HeaderSearch ) {
+void ASTWriter::WriteHeaderSearch(HeaderSearch ) {
   HeaderFileInfoTrait GeneratorTrait(*this);
   llvm::OnDiskChainedHashTableGenerator Generator;
   SmallVector SavedStrings;
@@ -1783,8 +1786,7 @@
 // changed since it was 

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:708-710
+return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName,
+M1.Message) <
+   std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName, 
M2.Message);

nridge wrote:
> nridge wrote:
> > sammccall wrote:
> > > nridge wrote:
> > > > njames93 wrote:
> > > > > This looks like a clang-format artifact, there are several other 
> > > > > below. Could these be removed from this patch
> > > > If you insist, I can move them to a separate patch. I don't want to 
> > > > leave it unmodified because the change will come back every time 
> > > > someone touches the file.
> > > I don't personally care about this too much, but the changes are somewhat 
> > > distracting in review, blame etc.
> > > 
> > > The policy we've mostly followed is to format changed lines only (git 
> > > clang-format, turn format-on-save off) and leave misformatted code alone 
> > > until it's next touched.
> > > 
> > > (Not sure if LLVM offers any guidance either way, but that's what Google 
> > > does internally and IME it's been great)
> > The issue I have with that is that turning format-on-save off means you 
> > inevitably end up doing a lot of manual formatting as you write the code. 
> > With format-on-save, you can just write the tokens for a statement not 
> > caring about the formatting, do a save, and have the statement be formatted 
> > before you start writing the next one.
> I guess what we really want is "format on save for modified lines only". I 
> found a [VSCode 
> extension](https://marketplace.visualstudio.com/items?itemName=Gruntfuggly.format-modified)
>  which does that, I'll give that a try.
What I do (apart from when i forget) is run git clang-format before creating 
the patch, Then just don't worry about formatting while I'm writing the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75286



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

[PATCH] D76990: [clang-tidy]: fix false positive of cert-oop54-cpp check.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

I'm not entirely sure this is where the fix needs to be for this. The test case 
code is whacky as hell, but from what I can see clang should always emit a 
`BinaryOperator` for dependent type operators. No idea why it is spewing out a 
`CXXOperatorCallExpr` instead. Would need someone with more knowledge on Sema 
to confirm(or deny) this though.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp:516
 public:
-  NotACopyAssignmentOperator& operator=(const NotACopyAssignmentOperator ) {
+  NotACopyAssignmentOperator =(const NotACopyAssignmentOperator ) {
 Ptr1 = RHS.getUy();

clang format artefact? This change should be undone as its unrelated to the 
patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76990



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

[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

[Vy Nguyen via Phabricator via cfe-commits]

oontvoo updated this revision to Diff 253382.
oontvoo added a comment.

Update tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/Inputs/dummy_pragma_once.h
  clang/test/Modules/Inputs/dummy_textual_header.h
  clang/test/Modules/Inputs/module.map
  clang/test/Modules/import-pragma-once.c

Index: clang/test/Modules/import-pragma-once.c
===
--- /dev/null
+++ clang/test/Modules/import-pragma-once.c
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -I%S/Inputs -verify -x c %s
+// expected-no-diagnostics
+#include "dummy_pragma_once.h"
+#include "dummy_textual_header.h"
+
+void *p = 
+void *q = 
Index: clang/test/Modules/Inputs/module.map
===
--- clang/test/Modules/Inputs/module.map
+++ clang/test/Modules/Inputs/module.map
@@ -282,6 +282,11 @@
   header "dummy.h"
 }
 
+module dummy_pragma_once {
+  header "dummy_pragma_once.h"
+  textual header "dummy_textual_header.h"
+}
+
 module builtin {
   header "builtin.h"
   explicit module sub {
Index: clang/test/Modules/Inputs/dummy_textual_header.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/dummy_textual_header.h
@@ -0,0 +1,2 @@
+#pragma once
+int y = 6;
Index: clang/test/Modules/Inputs/dummy_pragma_once.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/dummy_pragma_once.h
@@ -0,0 +1,3 @@
+#include "dummy_textual_header.h"
+
+int x = 5;
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1621,7 +1621,7 @@
   endian::Writer LE(Out, little);
   unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
   LE.write(KeyLen);
-  unsigned DataLen = 1 + 2 + 4 + 4;
+  unsigned DataLen = 1 + 2 + 4 + 4 + 4;
   for (auto ModInfo : Data.KnownHeaders)
 if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
   DataLen += 4;
@@ -1678,6 +1678,9 @@
   }
   LE.write(Offset);
 
+  // Write this file UID.
+  LE.write(Data.HFI.UID);
+
   auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
 if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
   uint32_t Value = (ModID << 2) | (unsigned)Role;
@@ -1705,7 +1708,7 @@
 /// Write the header search block for the list of files that
 ///
 /// \param HS The header search structure to save.
-void ASTWriter::WriteHeaderSearch(const HeaderSearch ) {
+void ASTWriter::WriteHeaderSearch(HeaderSearch ) {
   HeaderFileInfoTrait GeneratorTrait(*this);
   llvm::OnDiskChainedHashTableGenerator Generator;
   SmallVector SavedStrings;
@@ -1783,8 +1786,7 @@
 // changed since it was loaded. Also skip it if it's for a modular header
 // from a different module; in that case, we rely on the module(s)
 // containing the header to provide this information.
-const HeaderFileInfo *HFI =
-HS.getExistingFileInfo(File, /*WantExternal*/!Chain);
+HeaderFileInfo *HFI = HS.getExistingFileInfo(File, /*WantExternal*/ !Chain);
 if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
   continue;
 
@@ -1801,8 +1803,13 @@
 HeaderFileInfoTrait::key_type Key = {
   Filename, File->getSize(), getTimestampForOutput(File)
 };
+// Set the UID for this HFI so that its importers could use it
+// when serializing.
+HFI->UID = UID;
 HeaderFileInfoTrait::data_type Data = {
-  *HFI, HS.getModuleMap().findAllModulesForHeader(File), {}
+*HFI,
+HS.getModuleMap().findAllModulesForHeader(File),
+{},
 };
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
@@ -2636,6 +2643,25 @@
   Stream.EmitRecord(SUBMODULE_IMPORTS, Record);
 }
 
+// Emit the imported header's UIDs.
+{
+  auto it = PP->Submodules.find(Mod);
+  if (it != PP->Submodules.end() && !it->second.IncludedFiles.empty()) {
+RecordData Record;
+for (unsigned UID : it->second.IncludedFiles) {
+  // Only save it if the header is actually import/pragma once.
+  // FIXME When we first see a header, it always goes into the mod's
+  // list of included, regardless of whether it was pragma-once or 

[clang] 09d4021 - Fix compatibility for __builtin_stdarg_start

[Joerg Sonnenberger via cfe-commits]

Author: Joerg Sonnenberger
Date: 2020-03-28T23:24:13+01:00
New Revision: 09d402185394fdbf1f60233a7f42a4a1108c2cd3

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

LOG: Fix compatibility for __builtin_stdarg_start

The __builtin_stdarg_start is the legacy spelling of __builtin_va_start.
It should behave exactly the same, but for the last 9 years it would
behave subtly different for diagnostics. Follow the change from
29ad95b23217 to require custom type checking.

Added: 


Modified: 
clang/include/clang/Basic/Builtins.def
clang/test/SemaCXX/vararg-non-pod.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index 85f775a7cad8..9613f312d52d 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -472,7 +472,7 @@ BUILTIN(__builtin___NSStringMakeConstantString, "FC*cC*", 
"nc")
 BUILTIN(__builtin_va_start, "vA.", "nt")
 BUILTIN(__builtin_va_end, "vA", "n")
 BUILTIN(__builtin_va_copy, "vAA", "n")
-BUILTIN(__builtin_stdarg_start, "vA.", "n")
+BUILTIN(__builtin_stdarg_start, "vA.", "nt")
 BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nc")
 BUILTIN(__builtin_bcmp, "ivC*vC*z", "Fn")
 BUILTIN(__builtin_bcopy, "vv*v*z", "n")

diff  --git a/clang/test/SemaCXX/vararg-non-pod.cpp 
b/clang/test/SemaCXX/vararg-non-pod.cpp
index 1b7f3b68dc97..a1bbe748d12d 100644
--- a/clang/test/SemaCXX/vararg-non-pod.cpp
+++ b/clang/test/SemaCXX/vararg-non-pod.cpp
@@ -164,6 +164,13 @@ void t6(Foo somearg, ... ) {
   __builtin_va_start(list, somearg);
 }
 
+// __builtin_stdarg_start is a compatibility alias for __builtin_va_start,
+// it should behave the same
+void t6b(Foo somearg, ... ) {
+  __builtin_va_list list;
+  __builtin_stdarg_start(list, somearg); // second argument to 'va_start' is 
not the last named parameter [-Wvarargs]
+}
+
 void t7(int n, ...) {
   __builtin_va_list list;
   __builtin_va_start(list, n);



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

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.
Herald added a subscriber: broadwaylamb.

Ping?


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

https://reviews.llvm.org/D73245



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

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

[Karasev Nikita via Phabricator via cfe-commits]

f00kat created this revision.
f00kat added reviewers: aaron.ballman, lebedev.ri, NoQ, martong.
f00kat added a project: clang.
Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware, xazax.hun.

1. Added insufficient storage check for arrays
2. Added align support check

Based on https://reviews.llvm.org/D76229


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76996

Files:
  clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
  clang/test/Analysis/placement-new.cpp

Index: clang/test/Analysis/placement-new.cpp
===
--- clang/test/Analysis/placement-new.cpp
+++ clang/test/Analysis/placement-new.cpp
@@ -155,3 +155,127 @@
   (void)dp;
 }
 } // namespace testHierarchy
+
+namespace testConcretePointers {
+void f1()
+{
+// ok (200 % 8 == 0).
+::new ((size_t*)200) long;
+}
+void f2()
+{
+// bad (100 % 8 == 4).
+::new ((size_t*)100) long; // expected-warning{{Address is aligned to 4 bytes instead of 8 bytes}} expected-note 1 {{}}
+}
+
+void f3()
+{
+::new (reinterpret_cast(200)) long;
+}
+
+void f4()
+{
+::new (reinterpret_cast(100)) long; // expected-warning{{Address is aligned to 4 bytes instead of 8 bytes}} expected-note 1 {{}}
+}
+
+void f5()
+{
+::new ((size_t*)(200 + 0)) long;
+}
+
+void f6() {
+::new ((size_t*)(100 + 2)) long; // expected-warning{{Address is aligned to 6 bytes instead of 8 bytes}} expected-note 1 {{}}
+}
+} // namespace testConcretePointers
+
+namespace testArrayTypesAllocation {
+void f1()
+{
+struct S
+{
+short a;
+};
+
+// bad (not enough space).
+const unsigned N = 32;
+alignas(S) unsigned char buffer1[sizeof(S) * N]; // expected-note {{'buffer1' initialized here}}
+::new (buffer1) S[N]; // expected-warning{{Storage provided to placement new is only 64 bytes, whereas the allocated array type requires more space for internal needs}} expected-note 1 {{}}
+}
+
+void f2() 
+{
+struct S
+{
+short a;
+};
+
+// maybe ok but we need to warn.
+const unsigned N = 32;
+alignas(S) unsigned char buffer2[sizeof(S) * N + sizeof(int)]; // expected-note {{'buffer2' initialized here}}
+::new (buffer2) S[N]; // expected-warning{{Possibly not enough 68 bytes for array allocation which requires 64 bytes. Current overhead requires the size of 4 bytes}} expected-note 1 {{}}
+}
+} // namespace testArrayTypesAllocation
+
+namespace testStructAlign {
+void f1()
+{
+struct A
+{
+char a[9];
+} Ai; // expected-note {{'Ai' initialized here}}
+
+// bad (struct A is aligned to char).
+::new () long;  // expected-warning{{Storage type is aligned to 1 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f2()
+{
+struct B
+{
+char a;
+long b;
+} Bi;
+
+// ok (struct B is aligned to long).
+::new () long;
+}
+
+void f3()
+{
+struct C
+{
+char a[8];
+alignas(2) char b;
+} Ci; // expected-note {{'Ci' initialized here}}
+
+// bad (struct C is aligned to 2).
+::new () long; // expected-warning{{Storage type is aligned to 2 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f4()
+{
+struct D
+{
+char a;
+char b;
+struct {
+long c;
+};
+} Di;
+
+// ok (struct D is aligned to long).
+::new () long;
+}
+
+void f5()
+{
+struct alignas(alignof(long)) E {
+char a;
+char b;
+} Ei;
+
+// ok (struct E is aligned to long).
+::new () long;
+}
+} // namespace testStructAlign
+
Index: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
@@ -25,22 +25,31 @@
   void checkPreStmt(const CXXNewExpr *NE, CheckerContext ) const;
 
 private:
+  bool checkPlaceCapacityIsSufficient(const CXXNewExpr *NE,
+  CheckerContext ) const;
+
+  bool checkPlaceIsAlignedProperly(const CXXNewExpr *NE,
+   CheckerContext ) const;
+
   // Returns the size of the target in a placement new expression.
   // E.g. in "new () long" it returns the size of `long`.
-  SVal getExtentSizeOfNewTarget(const CXXNewExpr *NE, ProgramStateRef State,
-CheckerContext ) const;
+  SVal getExtentSizeOfNewTarget(const CXXNewExpr *NE, CheckerContext ,
+bool ) const;
   // Returns the size of the place in a placement new expression.
   // E.g. in "new () long" it returns the size of `s`.
-  SVal getExtentSizeOfPlace(const Expr *NE, ProgramStateRef State,
-CheckerContext ) const;
-  BugType BT{this, 

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

[Atmn Patel via Phabricator via cfe-commits]

atmnpatel added a comment.

> In D75591#1936152 , @lebedev.ri 
> wrote:
>  It should be handled consistently with how other newer openmp features are 
> handled with older openmp version specified.

In terms of consistency, the newer openmp features are handled as errors 
thrown, not as warnings. In this particular case however, it might make more 
sense to keep this as a warning rather than an error because its functionality 
doesn't exactly rely on a particular openmp version. I have it written out as 
an error now, but it doesn't quite sit right with me as an error yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75591



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

[PATCH] D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/include/clang/AST/Expr.h:2573
 
+  FPOptions FPFeatures;
+

mibintc wrote:
> rjmccall wrote:
> > Let's split the CallExpr changes out into a separate patch, so that we have 
> > one patch that's *purely* about unifying BinaryOperator and 
> > CompoundAssignOperator and putting FPOptions in trailing storage, and then 
> > another that's about adding FPOptions to CallExpr.
> > 
> > For that latter patch, CallExpr already has its own, hand-rolled concept of 
> > trailing storage which you should be able to emulate instead of unifying 
> > the hierarchy.
> I've split off the CallExpr changes to a future patch - will you give me a 
> hint about CallExpr's own hand-rolled concept of trailing storage? e.g. a 
> line number or identifier name that i can hunt for and track down.
Sure, check out CallExpr's sizeOfTrailingObjects and offsetToTrailingObjects.



Comment at: clang/include/clang/AST/Expr.h:3420
   Stmt *SubExprs[END_EXPR];
+  bool hasFPFeatures;
+  bool isCompoundAssign;

mibintc wrote:
> I think I need hasFPFeatures in the node itself because that's how i know 
> whether there's trailingstorage e.g. in the AST reader-writer.  hasFPFeatures 
> is always true at the moment but that will be improved in the next 
> generation. 
It's pretty common to make ASTStmtReader and ASTStmtWriter friends of the 
bitfields struct.



Comment at: clang/include/clang/AST/Expr.h:3421
+  bool hasFPFeatures;
+  bool isCompoundAssign;
+

mibintc wrote:
> rjmccall wrote:
> > Can these bits go in the bitfields?
> i eliminated isCompoundAssign because, i can just check the opcode, i think 
> there's always a opcode or a dummy opcode. But I need hasFPFeatures
If IsCompoundAssign is important enough to be worth optimizing, it's not a 
problem per se to keep it separate.  Up to you.  But it can definitely go in 
the bitfields if you need it.



Comment at: clang/include/clang/AST/Expr.h:3465
+SubExprs[RHS] = rhs;
+hasFPFeatures = true;
+isCompoundAssign = 1;

mibintc wrote:
> rjmccall wrote:
> > mibintc wrote:
> > > rjmccall wrote:
> > > > Okay, so this is *always* adding trailing storage.  Is there a common 
> > > > value for `FPOptions` — something that corresponds to default settings, 
> > > > for example — that we could use to avoid needing storage in the common 
> > > > case?
> > > > 
> > > > Also, it would be useful to extract a function somewhere that you can 
> > > > use in all of these places for consistency, maybe something like this 
> > > > on `FPOptions`:
> > > > 
> > > > ```
> > > >   /// Does this FPOptions require trailing storage when stored in 
> > > > various AST nodes,
> > > >   /// or can it be recreated using `defaultWithoutTrailingStorage`?
> > > >   bool requiresTrailingStorage() const { return *this == 
> > > > defaultWithoutTrailingStorage(); }
> > > > 
> > > >   /// Return the default value of FPOptions that's used when trailing 
> > > > storage isn't required.
> > > >   static FPOptions defaultWithoutTrailingStorage() { ... }
> > > > ```
> > > The reason I set hasFPFeatures to True in this revision is because in the 
> > > previous review you asked me to make it purely a representational change 
> > > and "add stuff about whether there is a pragma in a separate patch".  The 
> > > stuff I had in the previous version was smarter about creating trailing 
> > > storage, it only created trailing storage when the pragma was in effect. 
> > > I envirsioned that the evolution would accept something that always 
> > > created the FPOptions in trailing storage, and in the 2nd generation 
> > > would adopt the more thrifty way of creating trailing storage. 
> > Well, let's at least set up the infrastructure and APIs correctly, even if 
> > we always allocate trailing storage.
> > 
> > For example, should `getFPOptions` take an `ASTContext &` so that it can 
> > always return the right options instead of making clients do 
> > `hasFPOptions() ? getFPOptions() : ctx.getFPOptions()`?
> To explain further, in the pragma patch, which is fully formed but split off 
> for a future commit, the Sema structure holds the current value of the 
> floating point settings (in FPOptions FPFeatures). It is initialized from the 
> command line, and as compound statements/blocks which contain pragma's are 
> entered and exited, the value of Sema.FPFeatures is updated.  I will add a 
> bit "has_pragma_features" to FPOptions. When that bit is true, I know that 
> the initial value of FPOptions (which is derived from command line) is 
> different than the current settings. In this circumstance only it is 
> necessary to hold FPFeatures in the Expr nodes (in Trailing storage).  In all 
> other cases, FPOptions can be derived from LangOpts and LangOpts is available 
> in each clang pass.  There are a bunch of occurrences where FPOptions() has 
> 

Re: [PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

[Kadir Çetinkaya via cfe-commits]

To illustrate another workflow:
I also got format-on-save turned on, but filter the irrelevant changes
before committing by doing `git add -p` instead of adding all modifications.

On Sat, Mar 28, 2020 at 9:17 PM Nathan Ridge via Phabricator <
revi...@reviews.llvm.org> wrote:

> nridge marked an inline comment as done.
> nridge added inline comments.
>
>
> 
> Comment at:
> clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:708-710
> +return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName,
> +M1.Message) <
> +   std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName,
> M2.Message);
> 
> nridge wrote:
> > sammccall wrote:
> > > nridge wrote:
> > > > njames93 wrote:
> > > > > This looks like a clang-format artifact, there are several other
> below. Could these be removed from this patch
> > > > If you insist, I can move them to a separate patch. I don't want to
> leave it unmodified because the change will come back every time someone
> touches the file.
> > > I don't personally care about this too much, but the changes are
> somewhat distracting in review, blame etc.
> > >
> > > The policy we've mostly followed is to format changed lines only (git
> clang-format, turn format-on-save off) and leave misformatted code alone
> until it's next touched.
> > >
> > > (Not sure if LLVM offers any guidance either way, but that's what
> Google does internally and IME it's been great)
> > The issue I have with that is that turning format-on-save off means you
> inevitably end up doing a lot of manual formatting as you write the code.
> With format-on-save, you can just write the tokens for a statement not
> caring about the formatting, do a save, and have the statement be formatted
> before you start writing the next one.
> I guess what we really want is "format on save for modified lines only". I
> found a [VSCode extension](
> https://marketplace.visualstudio.com/items?itemName=Gruntfuggly.format-modified)
> which does that, I'll give that a try.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D75286/new/
>
> https://reviews.llvm.org/D75286
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76587: [BPF] support 128bit int explicitly in layout spec

[Yonghong Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGced0d1f42b39: [BPF] support 128bit int explicitly in layout 
spec (authored by yonghong-song).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76587

Files:
  clang/lib/Basic/Targets/BPF.h
  clang/test/CodeGen/target-data.c
  llvm/lib/Target/BPF/BPFTargetMachine.cpp
  llvm/test/CodeGen/BPF/i128.ll

Index: llvm/test/CodeGen/BPF/i128.ll
===
--- /dev/null
+++ llvm/test/CodeGen/BPF/i128.ll
@@ -0,0 +1,67 @@
+; RUN: llc -march=bpfel -o - %s | FileCheck %s
+; RUN: llc -march=bpfeb -o - %s | FileCheck %s
+; Source code:
+;   struct ipv6_key_t {
+; unsigned pid;
+; unsigned __int128 saddr;
+; unsigned short lport;
+;   };
+;
+;   extern void test1(void *);
+;   int test(int pid) {
+; struct ipv6_key_t ipv6_key = {.pid = pid};
+; test1(_key);
+; return 0;
+;   }
+; Compilation flag:
+;   clang -target bpf -O2 -S -emit-llvm t.c
+
+%struct.ipv6_key_t = type { i32, i128, i16 }
+
+; Function Attrs: nounwind
+define dso_local i32 @test(i32 %pid) local_unnamed_addr #0 {
+entry:
+  %ipv6_key = alloca %struct.ipv6_key_t, align 16
+  %0 = bitcast %struct.ipv6_key_t* %ipv6_key to i8*
+  call void @llvm.lifetime.start.p0i8(i64 48, i8* nonnull %0) #4
+  call void @llvm.memset.p0i8.i64(i8* nonnull align 16 dereferenceable(48) %0, i8 0, i64 48, i1 false)
+  %pid1 = getelementptr inbounds %struct.ipv6_key_t, %struct.ipv6_key_t* %ipv6_key, i64 0, i32 0
+  store i32 %pid, i32* %pid1, align 16, !tbaa !2
+  call void @test1(i8* nonnull %0) #4
+  call void @llvm.lifetime.end.p0i8(i64 48, i8* nonnull %0) #4
+  ret i32 0
+}
+
+; CHECK-LABEL: test
+; CHECK:   *(u64 *)(r10 - 48) = r{{[0-9]+}}
+; CHECK:   *(u32 *)(r10 - 48) = r{{[0-9]+}}
+
+; Function Attrs: argmemonly nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) #1
+
+; Function Attrs: argmemonly nounwind willreturn writeonly
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg) #2
+
+declare dso_local void @test1(i8*) local_unnamed_addr #3
+
+; Function Attrs: argmemonly nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture) #1
+
+attributes #0 = { nounwind "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { argmemonly nounwind willreturn }
+attributes #2 = { argmemonly nounwind willreturn writeonly }
+attributes #3 = { "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #4 = { nounwind }
+
+!llvm.module.flags = !{!0}
+!llvm.ident = !{!1}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{!"clang version 11.0.0 (https://github.com/llvm/llvm-project.git 55fc7a47f8f18f84b44ff16f4e7a420c0a42ddf1)"}
+!2 = !{!3, !4, i64 0}
+!3 = !{!"ipv6_key_t", !4, i64 0, !7, i64 16, !8, i64 32}
+!4 = !{!"int", !5, i64 0}
+!5 = !{!"omnipotent char", !6, i64 0}
+!6 = !{!"Simple C/C++ TBAA"}
+!7 = !{!"__int128", !5, i64 0}
+!8 = !{!"short", !5, i64 0}
Index: llvm/lib/Target/BPF/BPFTargetMachine.cpp
===
--- llvm/lib/Target/BPF/BPFTargetMachine.cpp
+++ llvm/lib/Target/BPF/BPFTargetMachine.cpp
@@ -42,9 +42,9 @@
 // DataLayout: little or big endian
 static std::string computeDataLayout(const Triple ) {
   if (TT.getArch() == Triple::bpfeb)
-return "E-m:e-p:64:64-i64:64-n32:64-S128";
+return "E-m:e-p:64:64-i64:64-i128:128-n32:64-S128";
   else
-return "e-m:e-p:64:64-i64:64-n32:64-S128";
+return "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128";
 }
 
 static Reloc::Model getEffectiveRelocModel(Optional RM) {
Index: clang/test/CodeGen/target-data.c
===
--- clang/test/CodeGen/target-data.c
+++ clang/test/CodeGen/target-data.c
@@ -245,8 +245,8 @@
 
 // RUN: %clang_cc1 -triple bpfel -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=BPFEL
-// BPFEL: target datalayout = "e-m:e-p:64:64-i64:64-n32:64-S128"
+// BPFEL: target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128"
 
 // RUN: %clang_cc1 -triple bpfeb -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=BPFEB
-// BPFEB: target datalayout = "E-m:e-p:64:64-i64:64-n32:64-S128"
+// BPFEB: target datalayout = 

[PATCH] D76432: [clangd] Add a tweak for adding "using" statement.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp:206
+  tooling::Replacements R;
+  if (auto Err = R.add(tooling::Replacement(
+  SM, CharSourceRange::getTokenRange(NNSL.getSourceRange()), "",

sammccall wrote:
> adamcz wrote:
> > sammccall wrote:
> > > (Sorry if some of this is obvious: TL;DR: we should use TokenBuffer to 
> > > handle macros in this case)
> > > 
> > > Whenever we try to use SourceLocations to reason about written source 
> > > code, we have to think about macros. Some libraries try to encapsulate 
> > > this, but the road to confusion is paved with good intentions, and it's 
> > > often best to decide on the policy and be explicit about it.
> > > 
> > > For example, when provided a macro location, the tooling::Replacement 
> > > constructor uses its spelling location. 
> > > Given: 
> > > ```
> > > // imagine we're trying to abstract away namespaces for C or something
> > > #define NS(X) foo::X
> > > NS(Bar) boo;
> > > ```
> > > Running this action on `N` would result in changing the definition of the 
> > > `NS` macro to delete the "foo::", as that's where the qualifier was 
> > > spelled!
> > > 
> > > It would be reasonable (but actually not trivial!) to try to detect any 
> > > macro usage and bail out. The general case of "is there some sequence of 
> > > source-code tokens that correspond to this range of preprocessed tokens 
> > > and nothing else" is pretty hard.
> > > 
> > > However TokenBuffer does have APIs for this exact question, as it was 
> > > designed for writing refactorings that replace nodes. I think you want:
> > >  - `expandedTokens(NNSL.getSourceRange())`
> > >  - `spelledForExpanded(ExpandedTokens)`
> > >  - `Token::range(SM, Spelled.front(), Spelled.back())`
> > >  - `Replacement("fname", Range.Offset, Range.Length, "")`
> > > 
> > > (ugh, that's really awkward. Maybe we should have a helper in TokenBuffer 
> > > that combines 1-3)
> > > 
> > You have a good point that this doesn't work well with macros, but I'm not 
> > entirely sure how you think this should work.
> > 
> > I'd argue that code actions should refer to the thing under the cursor, not 
> > it's expansion. That would be confusing to the user, as they would not be 
> > able to understand what the action offered is, nor how it would affect 
> > other places. So in your example of 
> > #define NS(X) foo::X
> > I'd argue that we should not offer the action. 
> > We should, however, support something like:
> > EXPECT_TRUE(fo^o::bar());
> > In this case, the qualifier is spelled under the cursor and the fact that 
> > EXPECT_TRUE is a macro and not a function should not matter to the user.
> > 
> > I updated the code to support that and added tests.
> > We can use isMacroID() and isMacroArgExpansion() to filter out macros, 
> > except for macro arguments. Note that we can't use spelledForExpanded(), 
> > since the qualifier might not be the entire expansion of the macro, thus 
> > spelledForExpanded will return None.
> > 
> > Let me know if any of what I did here now doesn't make sense.
> > 
> > in your example of #define NS(X) foo::X I'd argue that we should not offer 
> > the action.
> 
> Indeed, sorry I didn't spell that out - I was highlighting it because the 
> original code silently did something (bad) in this case.
> 
> > can't use spelledForExpanded(), since the qualifier might not be the entire 
> > expansion of the macro
> 
> Ah, D75446 hasn't landed yet. @kadircet, want to land this soon? :-)
> 
> 
done, let me know if it doesn't work :D



Comment at: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp:249
+  const auto *SpelledBeginTok =
+  TB.spelledTokenAt(SM.getSpellingLoc(ExpandedTokens.front().location()));
+  const auto *SpelledEndTok =

why not use spelledForExpanded in here as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76432



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

[clang] ced0d1f - [BPF] support 128bit int explicitly in layout spec

[Yonghong Song via cfe-commits]

Author: Yonghong Song
Date: 2020-03-28T11:46:29-07:00
New Revision: ced0d1f42b39bd93b350b2597ce6587d107c26a7

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

LOG: [BPF] support 128bit int explicitly in layout spec

Currently, bpf does not specify 128bit alignment in its
layout spec. So for a structure like
  struct ipv6_key_t {
unsigned pid;
unsigned __int128 saddr;
unsigned short lport;
  };
clang will generate IR type
  %struct.ipv6_key_t = type { i32, [12 x i8], i128, i16, [14 x i8] }
Additional padding is to ensure later IR->MIR can generate correct
stack layout with target layout spec.

But it is common practice for a tracing program to be
first compiled with target flag (e.g., x86_64 or aarch64) through
clang to generate IR and then go through llc to generate bpf
byte code. Tracing program often refers to kernel internal
data structures which needs to be compiled with non-bpf target.

But such a compilation model may cause a problem on aarch64.
The bcc issue https://github.com/iovisor/bcc/issues/2827
reported such a problem.

For the above structure, since aarch64 has "i128:128" in its
layout string, the generated IR will have
  %struct.ipv6_key_t = type { i32, i128, i16 }

Since bpf does not have "i128:128" in its spec string,
the selectionDAG assumes alignment 8 for i128 and
computes the stack storage size for the above is 32 bytes,
which leads incorrect code later.

The x86_64 does not have this issue as it does not have
"i128:128" in its layout spec as it does permits i128 to
be alignmented at 8 bytes at stack. Its IR type looks like
  %struct.ipv6_key_t = type { i32, [12 x i8], i128, i16, [14 x i8] }

The fix here is add i128 support in layout spec, the same as
aarch64. The only downside is we may have less optimal stack
allocation in certain cases since we require 16byte alignment
for i128 instead of 8. But this is probably fine as i128 is
not used widely and in most cases users should already
have proper alignment.

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

Added: 
llvm/test/CodeGen/BPF/i128.ll

Modified: 
clang/lib/Basic/Targets/BPF.h
clang/test/CodeGen/target-data.c
llvm/lib/Target/BPF/BPFTargetMachine.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/BPF.h b/clang/lib/Basic/Targets/BPF.h
index b2f1831e960e..43e55dfbfb2b 100644
--- a/clang/lib/Basic/Targets/BPF.h
+++ b/clang/lib/Basic/Targets/BPF.h
@@ -35,9 +35,9 @@ class LLVM_LIBRARY_VISIBILITY BPFTargetInfo : public 
TargetInfo {
 Int64Type = SignedLong;
 RegParmMax = 5;
 if (Triple.getArch() == llvm::Triple::bpfeb) {
-  resetDataLayout("E-m:e-p:64:64-i64:64-n32:64-S128");
+  resetDataLayout("E-m:e-p:64:64-i64:64-i128:128-n32:64-S128");
 } else {
-  resetDataLayout("e-m:e-p:64:64-i64:64-n32:64-S128");
+  resetDataLayout("e-m:e-p:64:64-i64:64-i128:128-n32:64-S128");
 }
 MaxAtomicPromoteWidth = 64;
 MaxAtomicInlineWidth = 64;

diff  --git a/clang/test/CodeGen/target-data.c 
b/clang/test/CodeGen/target-data.c
index e49f8453e360..e619843f4bdb 100644
--- a/clang/test/CodeGen/target-data.c
+++ b/clang/test/CodeGen/target-data.c
@@ -245,8 +245,8 @@
 
 // RUN: %clang_cc1 -triple bpfel -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=BPFEL
-// BPFEL: target datalayout = "e-m:e-p:64:64-i64:64-n32:64-S128"
+// BPFEL: target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128"
 
 // RUN: %clang_cc1 -triple bpfeb -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=BPFEB
-// BPFEB: target datalayout = "E-m:e-p:64:64-i64:64-n32:64-S128"
+// BPFEB: target datalayout = "E-m:e-p:64:64-i64:64-i128:128-n32:64-S128"

diff  --git a/llvm/lib/Target/BPF/BPFTargetMachine.cpp 
b/llvm/lib/Target/BPF/BPFTargetMachine.cpp
index d2a332c78aa6..4d42e7414075 100644
--- a/llvm/lib/Target/BPF/BPFTargetMachine.cpp
+++ b/llvm/lib/Target/BPF/BPFTargetMachine.cpp
@@ -42,9 +42,9 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void 
LLVMInitializeBPFTarget() {
 // DataLayout: little or big endian
 static std::string computeDataLayout(const Triple ) {
   if (TT.getArch() == Triple::bpfeb)
-return "E-m:e-p:64:64-i64:64-n32:64-S128";
+return "E-m:e-p:64:64-i64:64-i128:128-n32:64-S128";
   else
-return "e-m:e-p:64:64-i64:64-n32:64-S128";
+return "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128";
 }
 
 static Reloc::Model getEffectiveRelocModel(Optional RM) {

diff  --git a/llvm/test/CodeGen/BPF/i128.ll b/llvm/test/CodeGen/BPF/i128.ll
new file mode 100644
index ..c77838fa8bb4
--- /dev/null
+++ b/llvm/test/CodeGen/BPF/i128.ll
@@ -0,0 +1,67 @@
+; RUN: llc -march=bpfel -o - %s | FileCheck %s
+; RUN: llc -march=bpfeb -o - %s | FileCheck %s
+; Source code:
+;   struct ipv6_key_t {
+; unsigned pid;
+; unsigned __int128 saddr;
+; 

[PATCH] D76979: [clang][llvm] Interface Stubs new yaml file format changes.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 253367.
plotfi added a comment.

fixing linter errors


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76979

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/blocks.c
  clang/test/InterfaceStubs/class-template-partial-specialization.cpp
  clang/test/InterfaceStubs/conflict-type.ifs
  clang/test/InterfaceStubs/constructor-using-shadow.cpp
  clang/test/InterfaceStubs/cxx-conversion.cpp
  clang/test/InterfaceStubs/cxxdeduction-guide.cpp
  clang/test/InterfaceStubs/driver-test3.c
  clang/test/InterfaceStubs/empty.c
  clang/test/InterfaceStubs/func.ifs
  clang/test/InterfaceStubs/hidden-class-inheritance.cpp
  clang/test/InterfaceStubs/indirect-field-decl.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/lambda.cpp
  clang/test/InterfaceStubs/namespace-alias.cpp
  clang/test/InterfaceStubs/namespace.cpp
  clang/test/InterfaceStubs/non-type-template-parm-decl.cpp
  clang/test/InterfaceStubs/object.c
  clang/test/InterfaceStubs/object.ifs
  clang/test/InterfaceStubs/ppc.cpp
  clang/test/InterfaceStubs/template-constexpr.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/template-template-parm-decl.cpp
  clang/test/InterfaceStubs/trycatch.cpp
  clang/test/InterfaceStubs/unresolved-using-typename.cpp
  clang/test/InterfaceStubs/usings.cpp
  clang/test/InterfaceStubs/var-template-specialization-decl.cpp
  clang/test/InterfaceStubs/weak.cpp
  clang/test/InterfaceStubs/windows.cpp
  llvm/test/tools/llvm-ifs/Inputs/strong-mismatch-size.ifs
  llvm/test/tools/llvm-ifs/Inputs/strong-mismatch-type.ifs
  llvm/test/tools/llvm-ifs/conflict-header-format.ifs
  llvm/test/tools/llvm-ifs/conflict-header-triple.ifs
  llvm/test/tools/llvm-ifs/conflict-header-version.ifs
  llvm/test/tools/llvm-ifs/conflict-size.ifs
  llvm/test/tools/llvm-ifs/conflict-type.ifs
  llvm/test/tools/llvm-ifs/conflict-weak.ifs
  llvm/test/tools/llvm-ifs/default-empty.ifs
  llvm/test/tools/llvm-ifs/empty1.ifs
  llvm/test/tools/llvm-ifs/empty2.ifs
  llvm/test/tools/llvm-ifs/func.ifs
  llvm/test/tools/llvm-ifs/ios-tbd.ifs
  llvm/test/tools/llvm-ifs/macos-tbd.ifs
  llvm/test/tools/llvm-ifs/object-function-size-weak-combo.ifs
  llvm/test/tools/llvm-ifs/object.ifs
  llvm/test/tools/llvm-ifs/strong.ifs
  llvm/test/tools/llvm-ifs/tvos-tbd.ifs
  llvm/test/tools/llvm-ifs/version-ok.ifs
  llvm/test/tools/llvm-ifs/watchos-tbd.ifs
  llvm/test/tools/llvm-ifs/weak-mismatch.ifs
  llvm/test/tools/llvm-ifs/weak.ifs
  llvm/tools/llvm-ifs/llvm-ifs.cpp

Index: llvm/tools/llvm-ifs/llvm-ifs.cpp
===
--- llvm/tools/llvm-ifs/llvm-ifs.cpp
+++ llvm/tools/llvm-ifs/llvm-ifs.cpp
@@ -26,6 +26,7 @@
 #include "llvm/TextAPI/MachO/TextAPIWriter.h"
 #include 
 #include 
+#include 
 
 using namespace llvm;
 using namespace llvm::yaml;
@@ -34,8 +35,8 @@
 #define DEBUG_TYPE "llvm-ifs"
 
 namespace {
-const VersionTuple IFSVersionCurrent(1, 2);
-}
+const VersionTuple IFSVersionCurrent(2, 0);
+} // end anonymous namespace
 
 static cl::opt Action("action", cl::desc(""),
cl::value_desc("write-ifs | write-bin"),
@@ -76,6 +77,7 @@
 }
 
 struct IFSSymbol {
+  IFSSymbol() = default;
   IFSSymbol(std::string SymbolName) : Name(SymbolName) {}
   std::string Name;
   uint64_t Size;
@@ -85,6 +87,8 @@
   bool operator<(const IFSSymbol ) const { return Name < RHS.Name; }
 };
 
+LLVM_YAML_IS_SEQUENCE_VECTOR(IFSSymbol)
+
 namespace llvm {
 namespace yaml {
 /// YAML traits for IFSSymbolType.
@@ -124,6 +128,7 @@
 /// YAML traits for IFSSymbol.
 template <> struct MappingTraits {
   static void mapping(IO , IFSSymbol ) {
+IO.mapRequired("Name", Symbol.Name);
 IO.mapRequired("Type", Symbol.Type);
 // The need for symbol size depends on the symbol type.
 if (Symbol.Type == IFSSymbolType::NoType)
@@ -140,20 +145,6 @@
   static const bool flow = true;
 };
 
-/// YAML traits for set of IFSSymbols.
-template <> struct CustomMappingTraits> {
-  static void inputOne(IO , StringRef Key, std::set ) {
-std::string Name = Key.str();
-IFSSymbol Sym(Name);
-IO.mapRequired(Name.c_str(), Sym);
-Set.insert(Sym);
-  }
-
-  static void output(IO , std::set ) {
-for (auto  : Set)
-  IO.mapRequired(Sym.Name.c_str(), const_cast(Sym));
-  }
-};
 } // namespace yaml
 } // namespace llvm
 
@@ -167,7 +158,7 @@
   std::string ObjectFileFormat;
   Optional SOName;
   std::vector NeededLibs;
-  std::set Symbols;
+  std::vector Symbols;
 
   IFSStub() = default;
   IFSStub(const IFSStub )
@@ -186,14 +177,18 @@
 /// YAML traits for IFSStub objects.
 template <> struct MappingTraits {
   static void mapping(IO , IFSStub ) {
-if 

[PATCH] D76979: [clang][llvm] Interface Stubs new yaml file format changes.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 253369.
plotfi added a comment.

Adding a better test to check the deprecated v1 format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76979

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/blocks.c
  clang/test/InterfaceStubs/class-template-partial-specialization.cpp
  clang/test/InterfaceStubs/conflict-type.ifs
  clang/test/InterfaceStubs/constructor-using-shadow.cpp
  clang/test/InterfaceStubs/cxx-conversion.cpp
  clang/test/InterfaceStubs/cxxdeduction-guide.cpp
  clang/test/InterfaceStubs/driver-test3.c
  clang/test/InterfaceStubs/empty.c
  clang/test/InterfaceStubs/func.ifs
  clang/test/InterfaceStubs/hidden-class-inheritance.cpp
  clang/test/InterfaceStubs/indirect-field-decl.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/lambda.cpp
  clang/test/InterfaceStubs/namespace-alias.cpp
  clang/test/InterfaceStubs/namespace.cpp
  clang/test/InterfaceStubs/non-type-template-parm-decl.cpp
  clang/test/InterfaceStubs/object.c
  clang/test/InterfaceStubs/object.ifs
  clang/test/InterfaceStubs/ppc.cpp
  clang/test/InterfaceStubs/template-constexpr.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/template-template-parm-decl.cpp
  clang/test/InterfaceStubs/trycatch.cpp
  clang/test/InterfaceStubs/unresolved-using-typename.cpp
  clang/test/InterfaceStubs/usings.cpp
  clang/test/InterfaceStubs/var-template-specialization-decl.cpp
  clang/test/InterfaceStubs/weak.cpp
  clang/test/InterfaceStubs/windows.cpp
  llvm/test/tools/llvm-ifs/Inputs/strong-mismatch-size.ifs
  llvm/test/tools/llvm-ifs/Inputs/strong-mismatch-type.ifs
  llvm/test/tools/llvm-ifs/conflict-header-format.ifs
  llvm/test/tools/llvm-ifs/conflict-header-triple.ifs
  llvm/test/tools/llvm-ifs/conflict-header-version.ifs
  llvm/test/tools/llvm-ifs/conflict-size.ifs
  llvm/test/tools/llvm-ifs/conflict-type.ifs
  llvm/test/tools/llvm-ifs/conflict-weak.ifs
  llvm/test/tools/llvm-ifs/default-empty.ifs
  llvm/test/tools/llvm-ifs/empty1.ifs
  llvm/test/tools/llvm-ifs/empty2.ifs
  llvm/test/tools/llvm-ifs/func.ifs
  llvm/test/tools/llvm-ifs/ios-tbd.ifs
  llvm/test/tools/llvm-ifs/macos-tbd.ifs
  llvm/test/tools/llvm-ifs/object-function-size-weak-combo.ifs
  llvm/test/tools/llvm-ifs/object.ifs
  llvm/test/tools/llvm-ifs/strong.ifs
  llvm/test/tools/llvm-ifs/tvos-tbd.ifs
  llvm/test/tools/llvm-ifs/version-ok.ifs
  llvm/test/tools/llvm-ifs/watchos-tbd.ifs
  llvm/test/tools/llvm-ifs/weak-mismatch.ifs
  llvm/test/tools/llvm-ifs/weak.ifs
  llvm/tools/llvm-ifs/llvm-ifs.cpp

Index: llvm/tools/llvm-ifs/llvm-ifs.cpp
===
--- llvm/tools/llvm-ifs/llvm-ifs.cpp
+++ llvm/tools/llvm-ifs/llvm-ifs.cpp
@@ -26,6 +26,7 @@
 #include "llvm/TextAPI/MachO/TextAPIWriter.h"
 #include 
 #include 
+#include 
 
 using namespace llvm;
 using namespace llvm::yaml;
@@ -34,8 +35,8 @@
 #define DEBUG_TYPE "llvm-ifs"
 
 namespace {
-const VersionTuple IFSVersionCurrent(1, 2);
-}
+const VersionTuple IFSVersionCurrent(2, 0);
+} // end anonymous namespace
 
 static cl::opt Action("action", cl::desc(""),
cl::value_desc("write-ifs | write-bin"),
@@ -76,6 +77,7 @@
 }
 
 struct IFSSymbol {
+  IFSSymbol() = default;
   IFSSymbol(std::string SymbolName) : Name(SymbolName) {}
   std::string Name;
   uint64_t Size;
@@ -85,6 +87,8 @@
   bool operator<(const IFSSymbol ) const { return Name < RHS.Name; }
 };
 
+LLVM_YAML_IS_SEQUENCE_VECTOR(IFSSymbol)
+
 namespace llvm {
 namespace yaml {
 /// YAML traits for IFSSymbolType.
@@ -124,6 +128,7 @@
 /// YAML traits for IFSSymbol.
 template <> struct MappingTraits {
   static void mapping(IO , IFSSymbol ) {
+IO.mapRequired("Name", Symbol.Name);
 IO.mapRequired("Type", Symbol.Type);
 // The need for symbol size depends on the symbol type.
 if (Symbol.Type == IFSSymbolType::NoType)
@@ -140,20 +145,6 @@
   static const bool flow = true;
 };
 
-/// YAML traits for set of IFSSymbols.
-template <> struct CustomMappingTraits> {
-  static void inputOne(IO , StringRef Key, std::set ) {
-std::string Name = Key.str();
-IFSSymbol Sym(Name);
-IO.mapRequired(Name.c_str(), Sym);
-Set.insert(Sym);
-  }
-
-  static void output(IO , std::set ) {
-for (auto  : Set)
-  IO.mapRequired(Sym.Name.c_str(), const_cast(Sym));
-  }
-};
 } // namespace yaml
 } // namespace llvm
 
@@ -167,7 +158,7 @@
   std::string ObjectFileFormat;
   Optional SOName;
   std::vector NeededLibs;
-  std::set Symbols;
+  std::vector Symbols;
 
   IFSStub() = default;
   IFSStub(const IFSStub )
@@ -186,14 +177,18 @@
 /// YAML traits for IFSStub objects.
 template <> struct MappingTraits {
   static void mapping(IO , 

[PATCH] D76594: [clang][AST] Support AST files larger than 512M

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin updated this revision to Diff 253368.
DmitryPolukhin added a comment.

Resolve merge conflict, revert clang-format for ASTBitCodes.h to avoid further 
conflicts, use C++ cast


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76594

Files:
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1892,6 +1892,7 @@
   // Write out the source location entry table. We skip the first
   // entry, which is always the same dummy entry.
   std::vector SLocEntryOffsets;
+  uint64_t SLocEntryOffsetsBase = Stream.GetCurrentBitNo();
   RecordData PreloadSLocs;
   SLocEntryOffsets.reserve(SourceMgr.local_sloc_entry_size() - 1);
   for (unsigned I = 1, N = SourceMgr.local_sloc_entry_size();
@@ -1902,7 +1903,9 @@
 assert((FID) == SLoc);
 
 // Record the offset of this source-location entry.
-SLocEntryOffsets.push_back(Stream.GetCurrentBitNo());
+uint64_t Offset = Stream.GetCurrentBitNo() - SLocEntryOffsetsBase;
+assert((Offset >> 32) == 0 && "SLocEntry offset too large");
+SLocEntryOffsets.push_back(Offset);
 
 // Figure out which record code to use.
 unsigned Code;
@@ -2010,12 +2013,14 @@
   Abbrev->Add(BitCodeAbbrevOp(SOURCE_LOCATION_OFFSETS));
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 16)); // # of slocs
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 16)); // total size
+  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 32)); // base offset
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // offsets
   unsigned SLocOffsetsAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
   {
 RecordData::value_type Record[] = {
 SOURCE_LOCATION_OFFSETS, SLocEntryOffsets.size(),
-SourceMgr.getNextLocalOffset() - 1 /* skip dummy */};
+SourceMgr.getNextLocalOffset() - 1 /* skip dummy */,
+SLocEntryOffsetsBase};
 Stream.EmitRecordWithBlob(SLocOffsetsAbbrev, Record,
   bytes(SLocEntryOffsets));
   }
@@ -2092,9 +2097,11 @@
 /// Writes the block containing the serialized form of the
 /// preprocessor.
 void ASTWriter::WritePreprocessor(const Preprocessor , bool IsModule) {
+  uint64_t MacroOffsetsBase = Stream.GetCurrentBitNo();
+
   PreprocessingRecord *PPRec = PP.getPreprocessingRecord();
   if (PPRec)
-WritePreprocessorDetail(*PPRec);
+WritePreprocessorDetail(*PPRec, MacroOffsetsBase);
 
   RecordData Record;
   RecordData ModuleMacroRecord;
@@ -2155,7 +2162,8 @@
   // identifier they belong to.
   for (const IdentifierInfo *Name : MacroIdentifiers) {
 MacroDirective *MD = PP.getLocalMacroDirectiveHistory(Name);
-auto StartOffset = Stream.GetCurrentBitNo();
+uint64_t StartOffset = Stream.GetCurrentBitNo() - MacroOffsetsBase;
+assert((StartOffset >> 32) == 0 && "Macro identifiers offset too large");
 
 // Emit the macro directives in reverse source order.
 for (; MD; MD = MD->getPrevious()) {
@@ -2228,14 +2236,12 @@
 
 // Record the local offset of this macro.
 unsigned Index = ID - FirstMacroID;
-if (Index == MacroOffsets.size())
-  MacroOffsets.push_back(Stream.GetCurrentBitNo());
-else {
-  if (Index > MacroOffsets.size())
-MacroOffsets.resize(Index + 1);
+if (Index >= MacroOffsets.size())
+  MacroOffsets.resize(Index + 1);
 
-  MacroOffsets[Index] = Stream.GetCurrentBitNo();
-}
+uint64_t Offset = Stream.GetCurrentBitNo() - MacroOffsetsBase;
+assert((Offset >> 32) == 0 && "Macro offset too large");
+MacroOffsets[Index] = Offset;
 
 AddIdentifierRef(Name, Record);
 AddSourceLocation(MI->getDefinitionLoc(), Record);
@@ -2286,17 +2292,20 @@
   Abbrev->Add(BitCodeAbbrevOp(MACRO_OFFSET));
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32)); // # of macros
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32)); // first ID
+  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 32));   // base offset
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
 
   unsigned MacroOffsetAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
   {
 RecordData::value_type Record[] = {MACRO_OFFSET, MacroOffsets.size(),
-   FirstMacroID - NUM_PREDEF_MACRO_IDS};
+   FirstMacroID - NUM_PREDEF_MACRO_IDS,
+   MacroOffsetsBase};
 Stream.EmitRecordWithBlob(MacroOffsetAbbrev, Record, bytes(MacroOffsets));
   }
 }
 
-void ASTWriter::WritePreprocessorDetail(PreprocessingRecord ) {
+void 

[PATCH] D71512: [clang-format] Fix short block when braking after control statement

[Pablo Martin-Gomez via Phabricator via cfe-commits]

Bouska updated this revision to Diff 253366.
Bouska added a comment.

Do not touch current tests.


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

https://reviews.llvm.org/D71512

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -606,6 +606,12 @@
   verifyFormat("if CONSTEXPR (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
+  verifyFormat("if (true) { fff(); }",
+   AllowSimpleBracedStatements);
+  verifyFormat("if (true) {\n"
+   "  ();\n"
+   "}",
+   AllowSimpleBracedStatements);
   verifyFormat("if (true) {\n"
"  ff();\n"
"}",
@@ -681,6 +687,13 @@
   verifyFormat("if CONSTEXPR (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
+  verifyFormat("if (true) { fff(); }",
+   AllowSimpleBracedStatements);
+  verifyFormat("if (true)\n"
+   "{\n"
+   "  ();\n"
+   "}",
+   AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
"{\n"
"  ff();\n"
@@ -745,7 +758,9 @@
   Style.BreakBeforeBraces = FormatStyle::BS_Allman;
   EXPECT_EQ("#define A  \\\n"
 "  if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
-"  { RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; }\n"
+"  {\\\n"
+"RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier;   \\\n"
+"  }\n"
 "X;",
 format("#define A \\\n"
"   if (HANDLEwernufrnuLwrmviferuvnierv) { \\\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -349,21 +349,6 @@
  ? 1
  : 0;
 }
-// Try to merge either empty or one-line block if is precedeed by control
-// statement token
-if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last &&
-I != AnnotatedLines.begin() &&
-I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
-  unsigned MergedLines = 0;
-  if (Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never) {
-MergedLines = tryMergeSimpleBlock(I - 1, E, Limit);
-// If we managed to merge the block, discard the first merged line
-// since we are merging starting from I.
-if (MergedLines > 0)
-  --MergedLines;
-  }
-  return MergedLines;
-}
 // Don't merge block with left brace wrapped after ObjC special blocks
 if (TheLine->First->is(tok::l_brace) && I != AnnotatedLines.begin() &&
 I[-1]->First->is(tok::at) && I[-1]->First->Next) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -606,6 +606,12 @@
   verifyFormat("if CONSTEXPR (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
+  verifyFormat("if (true) { fff(); }",
+   AllowSimpleBracedStatements);
+  verifyFormat("if (true) {\n"
+   "  ();\n"
+   "}",
+   AllowSimpleBracedStatements);
   verifyFormat("if (true) {\n"
"  ff();\n"
"}",
@@ -681,6 +687,13 @@
   verifyFormat("if CONSTEXPR (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
+  verifyFormat("if (true) { fff(); }",
+   AllowSimpleBracedStatements);
+  verifyFormat("if (true)\n"
+   "{\n"
+   "  ();\n"
+   "}",
+   AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
"{\n"
"  

[clang] 4065e92 - Upgrade some instances of std::sort to llvm::sort. NFC.

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2020-03-28T19:23:29+01:00
New Revision: 4065e92195b1815367d619292080a9a1d3032015

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

LOG: Upgrade some instances of std::sort to llvm::sort. NFC.

Added: 


Modified: 
clang-tools-extra/clang-doc/Representation.cpp
clang-tools-extra/clang-tidy/ClangTidy.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clangd/index/Relation.cpp
clang-tools-extra/modularize/CoverageChecker.cpp
clang-tools-extra/modularize/Modularize.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp
clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
clang/utils/TableGen/SveEmitter.cpp
llvm/lib/Target/AMDGPU/GCNRegBankReassign.cpp
llvm/lib/Target/AMDGPU/GCNRegPressure.h

Removed: 




diff  --git a/clang-tools-extra/clang-doc/Representation.cpp 
b/clang-tools-extra/clang-doc/Representation.cpp
index 34d83b438125..8c619d2a0963 100644
--- a/clang-tools-extra/clang-doc/Representation.cpp
+++ b/clang-tools-extra/clang-doc/Representation.cpp
@@ -187,7 +187,7 @@ void Info::mergeBase(Info &) {
   // Unconditionally extend the description, since each decl may have a 
comment.
   std::move(Other.Description.begin(), Other.Description.end(),
 std::back_inserter(Description));
-  std::sort(Description.begin(), Description.end());
+  llvm::sort(Description);
   auto Last = std::unique(Description.begin(), Description.end());
   Description.erase(Last, Description.end());
 }
@@ -202,7 +202,7 @@ void SymbolInfo::merge(SymbolInfo &) {
 DefLoc = std::move(Other.DefLoc);
   // Unconditionally extend the list of locations, since we want all of them.
   std::move(Other.Loc.begin(), Other.Loc.end(), std::back_inserter(Loc));
-  std::sort(Loc.begin(), Loc.end());
+  llvm::sort(Loc);
   auto Last = std::unique(Loc.begin(), Loc.end());
   Loc.erase(Last, Loc.end());
   mergeBase(std::move(Other));
@@ -314,7 +314,7 @@ bool Index::operator<(const Index ) const {
 }
 
 void Index::sort() {
-  std::sort(Children.begin(), Children.end());
+  llvm::sort(Children);
   for (auto  : Children)
 C.sort();
 }

diff  --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp 
b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index 52c217b649f5..05594f191a5f 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -454,7 +454,7 @@ std::vector 
ClangTidyASTConsumerFactory::getCheckNames() {
 CheckNames.push_back(AnalyzerCheckNamePrefix + AnalyzerCheck.first);
 #endif // CLANG_ENABLE_STATIC_ANALYZER
 
-  std::sort(CheckNames.begin(), CheckNames.end());
+  llvm::sort(CheckNames);
   return CheckNames;
 }
 

diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 068d56c1a8a8..414da2857cff 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -680,7 +680,7 @@ void 
ClangTidyDiagnosticConsumer::removeIncompatibleErrors() {
   for (auto  : FileEvents) {
 std::vector  = FileAndEvents.second;
 // Sweep.
-std::sort(Events.begin(), Events.end());
+llvm::sort(Events);
 int OpenIntervals = 0;
 for (const auto  : Events) {
   if (Event.Type == Event::ET_End)
@@ -726,7 +726,7 @@ struct EqualClangTidyError {
 std::vector ClangTidyDiagnosticConsumer::take() {
   finalizeLastError();
 
-  std::sort(Errors.begin(), Errors.end(), LessClangTidyError());
+  llvm::sort(Errors, LessClangTidyError());
   Errors.erase(std::unique(Errors.begin(), Errors.end(), 
EqualClangTidyError()),
Errors.end());
   if (RemoveIncompatibleErrors)

diff  --git a/clang-tools-extra/clangd/index/Relation.cpp 
b/clang-tools-extra/clangd/index/Relation.cpp
index e28591bb64b0..6daa10a6f95e 100644
--- a/clang-tools-extra/clangd/index/Relation.cpp
+++ b/clang-tools-extra/clangd/index/Relation.cpp
@@ -26,7 +26,7 @@ RelationSlab::lookup(const SymbolID , RelationKind 
Predicate) const {
 
 RelationSlab RelationSlab::Builder::build() && {
   // Sort in SPO order.
-  std::sort(Relations.begin(), Relations.end());
+  llvm::sort(Relations);
 
   // Remove duplicates.
   Relations.erase(std::unique(Relations.begin(), Relations.end()),

diff  --git a/clang-tools-extra/modularize/CoverageChecker.cpp 
b/clang-tools-extra/modularize/CoverageChecker.cpp
index 9df53d26c2e9..4246df9483d0 100644
--- a/clang-tools-extra/modularize/CoverageChecker.cpp
+++ b/clang-tools-extra/modularize/CoverageChecker.cpp
@@ -338,7 +338,7 @@ bool CoverageChecker::collectFileSystemHeaders() {
   }
 
   // Sort it, because 
diff erent file systems might order the file 
diff erently.
-  

[clang-tools-extra] 4065e92 - Upgrade some instances of std::sort to llvm::sort. NFC.

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2020-03-28T19:23:29+01:00
New Revision: 4065e92195b1815367d619292080a9a1d3032015

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

LOG: Upgrade some instances of std::sort to llvm::sort. NFC.

Added: 


Modified: 
clang-tools-extra/clang-doc/Representation.cpp
clang-tools-extra/clang-tidy/ClangTidy.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clangd/index/Relation.cpp
clang-tools-extra/modularize/CoverageChecker.cpp
clang-tools-extra/modularize/Modularize.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp
clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
clang/utils/TableGen/SveEmitter.cpp
llvm/lib/Target/AMDGPU/GCNRegBankReassign.cpp
llvm/lib/Target/AMDGPU/GCNRegPressure.h

Removed: 




diff  --git a/clang-tools-extra/clang-doc/Representation.cpp 
b/clang-tools-extra/clang-doc/Representation.cpp
index 34d83b438125..8c619d2a0963 100644
--- a/clang-tools-extra/clang-doc/Representation.cpp
+++ b/clang-tools-extra/clang-doc/Representation.cpp
@@ -187,7 +187,7 @@ void Info::mergeBase(Info &) {
   // Unconditionally extend the description, since each decl may have a 
comment.
   std::move(Other.Description.begin(), Other.Description.end(),
 std::back_inserter(Description));
-  std::sort(Description.begin(), Description.end());
+  llvm::sort(Description);
   auto Last = std::unique(Description.begin(), Description.end());
   Description.erase(Last, Description.end());
 }
@@ -202,7 +202,7 @@ void SymbolInfo::merge(SymbolInfo &) {
 DefLoc = std::move(Other.DefLoc);
   // Unconditionally extend the list of locations, since we want all of them.
   std::move(Other.Loc.begin(), Other.Loc.end(), std::back_inserter(Loc));
-  std::sort(Loc.begin(), Loc.end());
+  llvm::sort(Loc);
   auto Last = std::unique(Loc.begin(), Loc.end());
   Loc.erase(Last, Loc.end());
   mergeBase(std::move(Other));
@@ -314,7 +314,7 @@ bool Index::operator<(const Index ) const {
 }
 
 void Index::sort() {
-  std::sort(Children.begin(), Children.end());
+  llvm::sort(Children);
   for (auto  : Children)
 C.sort();
 }

diff  --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp 
b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index 52c217b649f5..05594f191a5f 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -454,7 +454,7 @@ std::vector 
ClangTidyASTConsumerFactory::getCheckNames() {
 CheckNames.push_back(AnalyzerCheckNamePrefix + AnalyzerCheck.first);
 #endif // CLANG_ENABLE_STATIC_ANALYZER
 
-  std::sort(CheckNames.begin(), CheckNames.end());
+  llvm::sort(CheckNames);
   return CheckNames;
 }
 

diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 068d56c1a8a8..414da2857cff 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -680,7 +680,7 @@ void 
ClangTidyDiagnosticConsumer::removeIncompatibleErrors() {
   for (auto  : FileEvents) {
 std::vector  = FileAndEvents.second;
 // Sweep.
-std::sort(Events.begin(), Events.end());
+llvm::sort(Events);
 int OpenIntervals = 0;
 for (const auto  : Events) {
   if (Event.Type == Event::ET_End)
@@ -726,7 +726,7 @@ struct EqualClangTidyError {
 std::vector ClangTidyDiagnosticConsumer::take() {
   finalizeLastError();
 
-  std::sort(Errors.begin(), Errors.end(), LessClangTidyError());
+  llvm::sort(Errors, LessClangTidyError());
   Errors.erase(std::unique(Errors.begin(), Errors.end(), 
EqualClangTidyError()),
Errors.end());
   if (RemoveIncompatibleErrors)

diff  --git a/clang-tools-extra/clangd/index/Relation.cpp 
b/clang-tools-extra/clangd/index/Relation.cpp
index e28591bb64b0..6daa10a6f95e 100644
--- a/clang-tools-extra/clangd/index/Relation.cpp
+++ b/clang-tools-extra/clangd/index/Relation.cpp
@@ -26,7 +26,7 @@ RelationSlab::lookup(const SymbolID , RelationKind 
Predicate) const {
 
 RelationSlab RelationSlab::Builder::build() && {
   // Sort in SPO order.
-  std::sort(Relations.begin(), Relations.end());
+  llvm::sort(Relations);
 
   // Remove duplicates.
   Relations.erase(std::unique(Relations.begin(), Relations.end()),

diff  --git a/clang-tools-extra/modularize/CoverageChecker.cpp 
b/clang-tools-extra/modularize/CoverageChecker.cpp
index 9df53d26c2e9..4246df9483d0 100644
--- a/clang-tools-extra/modularize/CoverageChecker.cpp
+++ b/clang-tools-extra/modularize/CoverageChecker.cpp
@@ -338,7 +338,7 @@ bool CoverageChecker::collectFileSystemHeaders() {
   }
 
   // Sort it, because 
diff erent file systems might order the file 
diff erently.
-  

[clang] 347e31c - Remove constexpr that MSVC doesn't like

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2020-03-28T19:23:29+01:00
New Revision: 347e31c052d5a36890c484b6753d480ac8f0062f

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

LOG: Remove constexpr that MSVC doesn't like

Added: 


Modified: 
clang/include/clang/Sema/ParsedAttr.h
clang/utils/TableGen/ClangAttrEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/ParsedAttr.h 
b/clang/include/clang/Sema/ParsedAttr.h
index 911778457331..21e030fe5134 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -67,8 +67,8 @@ struct ParsedAttrInfo {
   };
   ArrayRef Spellings;
 
-  constexpr ParsedAttrInfo(AttributeCommonInfo::Kind AttrKind =
-   AttributeCommonInfo::NoSemaHandlerAttribute)
+  ParsedAttrInfo(AttributeCommonInfo::Kind AttrKind =
+ AttributeCommonInfo::NoSemaHandlerAttribute)
   : AttrKind(AttrKind), NumArgs(0), OptArgs(0), HasCustomParsing(0),
 IsTargetSpecific(0), IsType(0), IsStmt(0), IsKnownToGCC(0),
 IsSupportedByPragmaAttribute(0) {}

diff  --git a/clang/utils/TableGen/ClangAttrEmitter.cpp 
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index f455ef3b364b..e9f91c685ee4 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3739,7 +3739,7 @@ void EmitClangAttrParsedAttrImpl(RecordKeeper , 
raw_ostream ) {
 }
 OS << "struct ParsedAttrInfo" << I->first
<< " final : public ParsedAttrInfo {\n";
-OS << "  constexpr ParsedAttrInfo" << I->first << "() {\n";
+OS << "  ParsedAttrInfo" << I->first << "() {\n";
 OS << "AttrKind = ParsedAttr::AT_" << AttrName << ";\n";
 emitArgInfo(Attr, OS);
 OS << "HasCustomParsing = ";



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

[PATCH] D75153: [ThinLTO] Allow usage of all SMT threads in the system

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In D75153#1947956 , @zero9178 wrote:

> This patch seems to break when disabling threading (aka LLVM_ENABLE_THREADS 
> == 0) as get_threadpool_strategy is undefined therefore creating linker 
> failures in clang. (Tested on Windows)


Should be fixed after rG3ab3f3c5d5825476dc1be15992f7c964629de688 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75153



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

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

[Nathan Ridge via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:708-710
+return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName,
+M1.Message) <
+   std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName, 
M2.Message);

sammccall wrote:
> nridge wrote:
> > njames93 wrote:
> > > This looks like a clang-format artifact, there are several other below. 
> > > Could these be removed from this patch
> > If you insist, I can move them to a separate patch. I don't want to leave 
> > it unmodified because the change will come back every time someone touches 
> > the file.
> I don't personally care about this too much, but the changes are somewhat 
> distracting in review, blame etc.
> 
> The policy we've mostly followed is to format changed lines only (git 
> clang-format, turn format-on-save off) and leave misformatted code alone 
> until it's next touched.
> 
> (Not sure if LLVM offers any guidance either way, but that's what Google does 
> internally and IME it's been great)
The issue I have with that is that turning format-on-save off means you 
inevitably end up doing a lot of manual formatting as you write the code. With 
format-on-save, you can just write the tokens for a statement not caring about 
the formatting, do a save, and have the statement be formatted before you start 
writing the next one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75286



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

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

[Nathan Ridge via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:708-710
+return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName,
+M1.Message) <
+   std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName, 
M2.Message);

nridge wrote:
> sammccall wrote:
> > nridge wrote:
> > > njames93 wrote:
> > > > This looks like a clang-format artifact, there are several other below. 
> > > > Could these be removed from this patch
> > > If you insist, I can move them to a separate patch. I don't want to leave 
> > > it unmodified because the change will come back every time someone 
> > > touches the file.
> > I don't personally care about this too much, but the changes are somewhat 
> > distracting in review, blame etc.
> > 
> > The policy we've mostly followed is to format changed lines only (git 
> > clang-format, turn format-on-save off) and leave misformatted code alone 
> > until it's next touched.
> > 
> > (Not sure if LLVM offers any guidance either way, but that's what Google 
> > does internally and IME it's been great)
> The issue I have with that is that turning format-on-save off means you 
> inevitably end up doing a lot of manual formatting as you write the code. 
> With format-on-save, you can just write the tokens for a statement not caring 
> about the formatting, do a save, and have the statement be formatted before 
> you start writing the next one.
I guess what we really want is "format on save for modified lines only". I 
found a [VSCode 
extension](https://marketplace.visualstudio.com/items?itemName=Gruntfuggly.format-modified)
 which does that, I'll give that a try.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75286



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

[PATCH] D76979: [clang][llvm] Interface Stubs new yaml file format changes.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

Please address the linter warnings.  I think that adding an additional test to 
demonstrate that the `experimental-ifs-version-v1` is properly diagnosed is a 
good idea.  LGTM other than that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76979



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

[clang] e8743c0 - Const-initialize ParsedAttrInfos

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2020-03-28T19:04:53+01:00
New Revision: e8743c0f389d43aeab053904c6407280bfbfd3a6

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

LOG: Const-initialize ParsedAttrInfos

Gets rid of a 150k static initializer (Release clang)

Added: 


Modified: 
clang/examples/Attribute/Attribute.cpp
clang/include/clang/Sema/ParsedAttr.h
clang/lib/Sema/ParsedAttr.cpp
clang/utils/TableGen/ClangAttrEmitter.cpp

Removed: 




diff  --git a/clang/examples/Attribute/Attribute.cpp 
b/clang/examples/Attribute/Attribute.cpp
index 04e30e1bd21b..998f175dae54 100644
--- a/clang/examples/Attribute/Attribute.cpp
+++ b/clang/examples/Attribute/Attribute.cpp
@@ -28,9 +28,10 @@ struct ExampleAttrInfo : public ParsedAttrInfo {
 OptArgs = 1;
 // GNU-style __attribute__(("example")) and C++-style [[example]] and
 // [[plugin::example]] supported.
-Spellings.push_back({ParsedAttr::AS_GNU, "example"});
-Spellings.push_back({ParsedAttr::AS_CXX11, "example"});
-Spellings.push_back({ParsedAttr::AS_CXX11, "plugin::example"});
+static constexpr Spelling S[] = {{ParsedAttr::AS_GNU, "example"},
+ {ParsedAttr::AS_CXX11, "example"},
+ {ParsedAttr::AS_CXX11, 
"plugin::example"}};
+Spellings = S;
   }
 
   bool diagAppertainsToDecl(Sema , const ParsedAttr ,

diff  --git a/clang/include/clang/Sema/ParsedAttr.h 
b/clang/include/clang/Sema/ParsedAttr.h
index 439e54084109..911778457331 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -63,12 +63,12 @@ struct ParsedAttrInfo {
   /// The syntaxes supported by this attribute and how they're spelled.
   struct Spelling {
 AttributeCommonInfo::Syntax Syntax;
-std::string NormalizedFullName;
+const char *NormalizedFullName;
   };
-  std::vector Spellings;
+  ArrayRef Spellings;
 
-  ParsedAttrInfo(AttributeCommonInfo::Kind AttrKind =
- AttributeCommonInfo::NoSemaHandlerAttribute)
+  constexpr ParsedAttrInfo(AttributeCommonInfo::Kind AttrKind =
+   AttributeCommonInfo::NoSemaHandlerAttribute)
   : AttrKind(AttrKind), NumArgs(0), OptArgs(0), HasCustomParsing(0),
 IsTargetSpecific(0), IsType(0), IsStmt(0), IsKnownToGCC(0),
 IsSupportedByPragmaAttribute(0) {}

diff  --git a/clang/lib/Sema/ParsedAttr.cpp b/clang/lib/Sema/ParsedAttr.cpp
index 6d96ea96cd37..d45777ca127d 100644
--- a/clang/lib/Sema/ParsedAttr.cpp
+++ b/clang/lib/Sema/ParsedAttr.cpp
@@ -115,7 +115,7 @@ const ParsedAttrInfo ::get(const 
AttributeCommonInfo ) {
 return *AttrInfoMap[A.getParsedKind()];
 
   // If this is an ignored attribute then return an appropriate ParsedAttrInfo.
-  static ParsedAttrInfo IgnoredParsedAttrInfo(
+  static const ParsedAttrInfo IgnoredParsedAttrInfo(
   AttributeCommonInfo::IgnoredAttribute);
   if (A.getParsedKind() == AttributeCommonInfo::IgnoredAttribute)
 return IgnoredParsedAttrInfo;
@@ -140,7 +140,7 @@ const ParsedAttrInfo ::get(const 
AttributeCommonInfo ) {
 return *Ptr;
 
   // If we failed to find a match then return a default ParsedAttrInfo.
-  static ParsedAttrInfo DefaultParsedAttrInfo(
+  static const ParsedAttrInfo DefaultParsedAttrInfo(
   AttributeCommonInfo::UnknownAttribute);
   return DefaultParsedAttrInfo;
 }

diff  --git a/clang/utils/TableGen/ClangAttrEmitter.cpp 
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 32744f7579fb..f455ef3b364b 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3719,8 +3719,27 @@ void EmitClangAttrParsedAttrImpl(RecordKeeper , 
raw_ostream ) {
 // ParsedAttr.cpp.
 const std::string  = I->first;
 const Record  = *I->second;
-OS << "struct ParsedAttrInfo" << I->first << " : public ParsedAttrInfo 
{\n";
-OS << "  ParsedAttrInfo" << I->first << "() {\n";
+auto Spellings = GetFlattenedSpellings(Attr);
+if (!Spellings.empty()) {
+  OS << "static constexpr ParsedAttrInfo::Spelling " << I->first
+ << "Spellings[] = {\n";
+  for (const auto  : Spellings) {
+const std::string  = S.name();
+std::string Spelling;
+if (!S.nameSpace().empty())
+  Spelling += S.nameSpace() + "::";
+if (S.variety() == "GNU")
+  Spelling += NormalizeGNUAttrSpelling(RawSpelling);
+else
+  Spelling += RawSpelling;
+OS << "  {AttributeCommonInfo::AS_" << S.variety();
+OS << ", \"" << Spelling << "\"},\n";
+  }
+  OS << "};\n";
+}
+OS << "struct ParsedAttrInfo" << I->first
+   << " final : public ParsedAttrInfo {\n";
+OS << "  constexpr ParsedAttrInfo" << I->first << "() {\n";
 OS << "  

[clang] 3ab3f3c - After 09158252f777c2e2f06a86b154c44abcbcf9bb74, fix build when -DLLVM_ENABLE_THREADS=OFF

[Alexandre Ganea via cfe-commits]

Author: Alexandre Ganea
Date: 2020-03-28T13:54:58-04:00
New Revision: 3ab3f3c5d5825476dc1be15992f7c964629de688

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

LOG: After 09158252f777c2e2f06a86b154c44abcbcf9bb74, fix build when 
-DLLVM_ENABLE_THREADS=OFF

Tested on Linux with Clang 9, and on Windows with Visual Studio 2019 16.5.1 
with -DLLVM_ENABLE_THREADS=ON and OFF.

Added: 


Modified: 
clang/test/Driver/lto-jobs.c
llvm/lib/Support/Threading.cpp

Removed: 




diff  --git a/clang/test/Driver/lto-jobs.c b/clang/test/Driver/lto-jobs.c
index 539867713b07..c28d0ad300f4 100644
--- a/clang/test/Driver/lto-jobs.c
+++ b/clang/test/Driver/lto-jobs.c
@@ -8,4 +8,4 @@
 // RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=thin 
-flto-jobs=5 2> %t
 // RUN: FileCheck -check-prefix=CHECK-LINK-THIN-JOBS2-ACTION < %t %s
 //
-// CHECK-LINK-THIN-JOBS2-ACTION: "-mllvm" "-threads=5"
+// CHECK-LINK-THIN-JOBS2-ACTION: "-mllvm" "-threads={{[0-9]+}}"

diff  --git a/llvm/lib/Support/Threading.cpp b/llvm/lib/Support/Threading.cpp
index 39de05892981..4bf373ddbd72 100644
--- a/llvm/lib/Support/Threading.cpp
+++ b/llvm/lib/Support/Threading.cpp
@@ -94,26 +94,6 @@ unsigned llvm::ThreadPoolStrategy::compute_thread_count() 
const {
   return MaxThreadCount;
 }
 
-Optional
-llvm::get_threadpool_strategy(StringRef Num, ThreadPoolStrategy Default) {
-  if (Num == "all")
-return llvm::hardware_concurrency();
-  if (Num.empty())
-return Default;
-  unsigned V;
-  if (Num.getAsInteger(10, V))
-return None; // malformed 'Num' value
-  if (V == 0)
-return Default;
-
-  // Do not take the Default into account. This effectively disables
-  // heavyweight_hardware_concurrency() if the user asks for any number of
-  // threads on the cmd-line.
-  ThreadPoolStrategy S = llvm::hardware_concurrency();
-  S.ThreadsRequested = V;
-  return S;
-}
-
 namespace {
 struct SyncThreadInfo {
   void (*UserFn)(void *);
@@ -150,3 +130,23 @@ void llvm::llvm_execute_on_thread_async(
 }
 
 #endif
+
+Optional
+llvm::get_threadpool_strategy(StringRef Num, ThreadPoolStrategy Default) {
+  if (Num == "all")
+return llvm::hardware_concurrency();
+  if (Num.empty())
+return Default;
+  unsigned V;
+  if (Num.getAsInteger(10, V))
+return None; // malformed 'Num' value
+  if (V == 0)
+return Default;
+
+  // Do not take the Default into account. This effectively disables
+  // heavyweight_hardware_concurrency() if the user asks for any number of
+  // threads on the cmd-line.
+  ThreadPoolStrategy S = llvm::hardware_concurrency();
+  S.ThreadsRequested = V;
+  return S;
+}



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

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:708-710
+return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName,
+M1.Message) <
+   std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName, 
M2.Message);

nridge wrote:
> njames93 wrote:
> > This looks like a clang-format artifact, there are several other below. 
> > Could these be removed from this patch
> If you insist, I can move them to a separate patch. I don't want to leave it 
> unmodified because the change will come back every time someone touches the 
> file.
I don't personally care about this too much, but the changes are somewhat 
distracting in review, blame etc.

The policy we've mostly followed is to format changed lines only (git 
clang-format, turn format-on-save off) and leave misformatted code alone until 
it's next touched.

(Not sure if LLVM offers any guidance either way, but that's what Google does 
internally and IME it's been great)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75286



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

[PATCH] D76979: [clang][llvm] Interface Stubs new yaml file format changes.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 253360.
plotfi added a comment.

Changing some of the version checks. I don't think we need to have rigid checks 
considering this format is experimental.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76979

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/blocks.c
  clang/test/InterfaceStubs/class-template-partial-specialization.cpp
  clang/test/InterfaceStubs/conflict-type.ifs
  clang/test/InterfaceStubs/constructor-using-shadow.cpp
  clang/test/InterfaceStubs/cxx-conversion.cpp
  clang/test/InterfaceStubs/cxxdeduction-guide.cpp
  clang/test/InterfaceStubs/driver-test3.c
  clang/test/InterfaceStubs/empty.c
  clang/test/InterfaceStubs/func.ifs
  clang/test/InterfaceStubs/hidden-class-inheritance.cpp
  clang/test/InterfaceStubs/indirect-field-decl.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/lambda.cpp
  clang/test/InterfaceStubs/namespace-alias.cpp
  clang/test/InterfaceStubs/namespace.cpp
  clang/test/InterfaceStubs/non-type-template-parm-decl.cpp
  clang/test/InterfaceStubs/object.c
  clang/test/InterfaceStubs/object.ifs
  clang/test/InterfaceStubs/ppc.cpp
  clang/test/InterfaceStubs/template-constexpr.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/template-template-parm-decl.cpp
  clang/test/InterfaceStubs/trycatch.cpp
  clang/test/InterfaceStubs/unresolved-using-typename.cpp
  clang/test/InterfaceStubs/usings.cpp
  clang/test/InterfaceStubs/var-template-specialization-decl.cpp
  clang/test/InterfaceStubs/weak.cpp
  clang/test/InterfaceStubs/windows.cpp
  llvm/test/tools/llvm-ifs/Inputs/strong-mismatch-size.ifs
  llvm/test/tools/llvm-ifs/Inputs/strong-mismatch-type.ifs
  llvm/test/tools/llvm-ifs/conflict-header-format.ifs
  llvm/test/tools/llvm-ifs/conflict-header-triple.ifs
  llvm/test/tools/llvm-ifs/conflict-header-version.ifs
  llvm/test/tools/llvm-ifs/conflict-size.ifs
  llvm/test/tools/llvm-ifs/conflict-type.ifs
  llvm/test/tools/llvm-ifs/conflict-weak.ifs
  llvm/test/tools/llvm-ifs/default-empty.ifs
  llvm/test/tools/llvm-ifs/empty1.ifs
  llvm/test/tools/llvm-ifs/empty2.ifs
  llvm/test/tools/llvm-ifs/func.ifs
  llvm/test/tools/llvm-ifs/ios-tbd.ifs
  llvm/test/tools/llvm-ifs/macos-tbd.ifs
  llvm/test/tools/llvm-ifs/object-function-size-weak-combo.ifs
  llvm/test/tools/llvm-ifs/object.ifs
  llvm/test/tools/llvm-ifs/strong.ifs
  llvm/test/tools/llvm-ifs/tvos-tbd.ifs
  llvm/test/tools/llvm-ifs/version-ok.ifs
  llvm/test/tools/llvm-ifs/watchos-tbd.ifs
  llvm/test/tools/llvm-ifs/weak-mismatch.ifs
  llvm/test/tools/llvm-ifs/weak.ifs
  llvm/tools/llvm-ifs/llvm-ifs.cpp

Index: llvm/tools/llvm-ifs/llvm-ifs.cpp
===
--- llvm/tools/llvm-ifs/llvm-ifs.cpp
+++ llvm/tools/llvm-ifs/llvm-ifs.cpp
@@ -26,6 +26,7 @@
 #include "llvm/TextAPI/MachO/TextAPIWriter.h"
 #include 
 #include 
+#include 
 
 using namespace llvm;
 using namespace llvm::yaml;
@@ -34,8 +35,8 @@
 #define DEBUG_TYPE "llvm-ifs"
 
 namespace {
-const VersionTuple IFSVersionCurrent(1, 2);
-}
+const VersionTuple IFSVersionCurrent(2, 0);
+} // end anonymous namespace
 
 static cl::opt Action("action", cl::desc(""),
cl::value_desc("write-ifs | write-bin"),
@@ -76,6 +77,7 @@
 }
 
 struct IFSSymbol {
+  IFSSymbol() = default;
   IFSSymbol(std::string SymbolName) : Name(SymbolName) {}
   std::string Name;
   uint64_t Size;
@@ -85,6 +87,8 @@
   bool operator<(const IFSSymbol ) const { return Name < RHS.Name; }
 };
 
+LLVM_YAML_IS_SEQUENCE_VECTOR(IFSSymbol)
+
 namespace llvm {
 namespace yaml {
 /// YAML traits for IFSSymbolType.
@@ -124,6 +128,7 @@
 /// YAML traits for IFSSymbol.
 template <> struct MappingTraits {
   static void mapping(IO , IFSSymbol ) {
+IO.mapRequired("Name", Symbol.Name);
 IO.mapRequired("Type", Symbol.Type);
 // The need for symbol size depends on the symbol type.
 if (Symbol.Type == IFSSymbolType::NoType)
@@ -140,20 +145,6 @@
   static const bool flow = true;
 };
 
-/// YAML traits for set of IFSSymbols.
-template <> struct CustomMappingTraits> {
-  static void inputOne(IO , StringRef Key, std::set ) {
-std::string Name = Key.str();
-IFSSymbol Sym(Name);
-IO.mapRequired(Name.c_str(), Sym);
-Set.insert(Sym);
-  }
-
-  static void output(IO , std::set ) {
-for (auto  : Set)
-  IO.mapRequired(Sym.Name.c_str(), const_cast(Sym));
-  }
-};
 } // namespace yaml
 } // namespace llvm
 
@@ -167,7 +158,7 @@
   std::string ObjectFileFormat;
   Optional SOName;
   std::vector NeededLibs;
-  std::set Symbols;
+  std::vector Symbols;
 
   IFSStub() = default;
   IFSStub(const IFSStub )
@@ -186,14 +177,18 @@
 /// YAML traits for IFSStub 

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

[Nathan Ridge via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:708-710
+return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName,
+M1.Message) <
+   std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName, 
M2.Message);

njames93 wrote:
> This looks like a clang-format artifact, there are several other below. Could 
> these be removed from this patch
If you insist, I can move them to a separate patch. I don't want to leave it 
unmodified because the change will come back every time someone touches the 
file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75286



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

[PATCH] D76990: [clang-tidy]: fix false positive of cert-oop54-cpp check.

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas updated this revision to Diff 253354.
ztamas added a comment.

Run clang-format on test code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76990

Files:
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
@@ -283,6 +283,21 @@
   T *p;
 };
 
+// https://bugs.llvm.org/show_bug.cgi?id=44499
+class Foo;
+template 
+bool operator!=(Foo &, Foo &) {
+  class Bar {
+Bar =(const Bar ) {
+  if (this != ) {
+  }
+  return *this;
+}
+
+int *p;
+  };
+}
+
 // There is no warning if the copy assignment operator gets the object by 
value.
 class PassedByValue {
 public:
@@ -498,7 +513,7 @@
   Uy *Ptr2;
 
 public:
-  NotACopyAssignmentOperator& operator=(const NotACopyAssignmentOperator ) {
+  NotACopyAssignmentOperator =(const NotACopyAssignmentOperator ) {
 Ptr1 = RHS.getUy();
 Ptr2 = RHS.getTy();
 return *this;
Index: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -40,9 +40,12 @@
 
   // Self-check: Code compares something with 'this' pointer. We don't check
   // whether it is actually the parameter what we compare.
-  const auto HasNoSelfCheck = cxxMethodDecl(unless(
+  const auto HasNoSelfCheck = cxxMethodDecl(unless(anyOf(
   hasDescendant(binaryOperator(hasAnyOperatorName("==", "!="),
-   has(ignoringParenCasts(cxxThisExpr()));
+   has(ignoringParenCasts(cxxThisExpr(),
+  hasDescendant(cxxOperatorCallExpr(
+  hasAnyOverloadedOperatorName("==", "!="), argumentCountIs(2),
+  has(ignoringParenCasts(cxxThisExpr(;
 
   // Both copy-and-swap and copy-and-move method creates a copy first and
   // assign it to 'this' with swap or move.


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
@@ -283,6 +283,21 @@
   T *p;
 };
 
+// https://bugs.llvm.org/show_bug.cgi?id=44499
+class Foo;
+template 
+bool operator!=(Foo &, Foo &) {
+  class Bar {
+Bar =(const Bar ) {
+  if (this != ) {
+  }
+  return *this;
+}
+
+int *p;
+  };
+}
+
 // There is no warning if the copy assignment operator gets the object by value.
 class PassedByValue {
 public:
@@ -498,7 +513,7 @@
   Uy *Ptr2;
 
 public:
-  NotACopyAssignmentOperator& operator=(const NotACopyAssignmentOperator ) {
+  NotACopyAssignmentOperator =(const NotACopyAssignmentOperator ) {
 Ptr1 = RHS.getUy();
 Ptr2 = RHS.getTy();
 return *this;
Index: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -40,9 +40,12 @@
 
   // Self-check: Code compares something with 'this' pointer. We don't check
   // whether it is actually the parameter what we compare.
-  const auto HasNoSelfCheck = cxxMethodDecl(unless(
+  const auto HasNoSelfCheck = cxxMethodDecl(unless(anyOf(
   hasDescendant(binaryOperator(hasAnyOperatorName("==", "!="),
-   has(ignoringParenCasts(cxxThisExpr()));
+   has(ignoringParenCasts(cxxThisExpr(),
+  hasDescendant(cxxOperatorCallExpr(
+  hasAnyOverloadedOperatorName("==", "!="), argumentCountIs(2),
+  has(ignoringParenCasts(cxxThisExpr(;
 
   // Both copy-and-swap and copy-and-move method creates a copy first and
   // assign it to 'this' with swap or move.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert updated this revision to Diff 253349.
fickert added a comment.

Added documentation and missing test cases for 0-width tabs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75034

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10314,15 +10314,244 @@
"\t*/\n"
"}",
Tab));
+  EXPECT_EQ("/* some\n"
+"   comment */",
+format(" \t \t /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("int a; /* some\n"
+"   comment */",
+format(" \t \t int a; /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("int a; /* some\n"
+"comment */",
+format(" \t \t int\ta; /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("f(\"\t\t\"); /* some\n"
+"comment */",
+format(" \t \t f(\"\t\t\"); /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("{\n"
+"  /*\n"
+"   * Comment\n"
+"   */\n"
+"  int i;\n"
+"}",
+format("{\n"
+   "\t/*\n"
+   "\t * Comment\n"
+   "\t */\n"
+   "\t int i;\n"
+   "}"));
+  Tab.TabWidth = 2;
+  Tab.IndentWidth = 2;
+  EXPECT_EQ("{\n"
+"\t/* \n"
+"\t\t  */\n"
+"}",
+format("{\n"
+   "/* \n"
+   "\t  */\n"
+   "}",
+   Tab));
+  EXPECT_EQ("{\n"
+"\t/*\n"
+"\t\taa\n"
+"\t\tb\n"
+"\t*/\n"
+"}",
+format("{\n"
+   "/*\n"
+   "\taa b\n"
+   "*/\n"
+   "}",
+   Tab));
+  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveDeclarations = true;
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 4;
+  verifyFormat("class Assign {\n"
+   "\tvoid f() {\n"
+   "\t\tint x  = 123;\n"
+   "\t\tint random = 4;\n"
+   "\t\tstd::string alphabet =\n"
+   "\t\t\t\"abcdefghijklmnopqrstuvwxyz\";\n"
+   "\t}\n"
+   "};",
+   Tab);
+
+  Tab.UseTab = FormatStyle::UT_AlignWithSpaces;
+  Tab.TabWidth = 8;
+  Tab.IndentWidth = 8;
+  EXPECT_EQ("if ( && // q\n"
+"bb) // w\n"
+"\t;",
+format("if ( &&// q\n"
+   "bb)// w\n"
+   ";",
+   Tab));
+  EXPECT_EQ("if (aaa && bbb) // w\n"
+"\t;",
+format("if(aaa&)// w\n"
+   ";",
+   Tab));
+  verifyFormat("class X {\n"
+   "\tvoid f() {\n"
+   "\t\tsomeFunction(parameter1,\n"
+   "\t\t parameter2);\n"
+   "\t}\n"
+   "};",
+   Tab);
+  verifyFormat("#define A\\\n"
+   "\tvoid f() {   \\\n"
+   "\t\tsomeFunction(\\\n"
+   "\t\tparameter1,  \\\n"
+   "\t\tparameter2); \\\n"
+   "\t}",
+   Tab);
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 8;
+  verifyFormat("class TabWidth4Indent8 {\n"
+   "\t\tvoid f() {\n"
+   "\t\t\t\tsomeFunction(parameter1,\n"
+   "\t\t\t\t parameter2);\n"
+   "\t\t}\n"
+   "};",
+   Tab);
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 4;
+  verifyFormat("class TabWidth4Indent4 {\n"
+   "\tvoid f() {\n"
+   "\t\tsomeFunction(parameter1,\n"
+   "\t\t parameter2);\n"
+   "\t}\n"
+   "};",
+   Tab);
+  Tab.TabWidth = 8;
+  Tab.IndentWidth = 4;
+  verifyFormat("class TabWidth8Indent4 {\n"
+   "void f() {\n"
+   "\tsomeFunction(parameter1,\n"
+   "\t parameter2);\n"
+   "}\n"
+   "};",
+   Tab);

[PATCH] D76943: [clang analysis] Make mutex guard detection more reliable.

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:5659
+  }
+  struct Convertible { Convertible(); operator ReadLockedPtr(); };
+  void use_conversion() {

Honestly I'd prefer the annotation to be on this operator instead of the move 
constructor, but that's an independent issue that existed before this change. 
(rC322316#294681)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76943



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

[PATCH] D75446: [clang][Syntax] Handle macro arguments in spelledForExpanded

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
kadircet marked an inline comment as done.
Closed by commit rG9619c2cc9a22: [clang][Syntax] Handle macro arguments in 
spelledForExpanded (authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D75446?vs=253325=253350#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75446

Files:
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -627,6 +627,7 @@
 
 A split B
   )cpp");
+  // Ranges going across expansion boundaries.
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a1 a2 a3 split b1 b2")),
   ValueIs(SameRange(findSpelled("A split B";
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a1 a2 a3")),
@@ -647,22 +648,28 @@
 ID(ID(ID(a1) a2 a3)) split ID(B)
   )cpp");
 
-  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a1 a2 a3")),
-  ValueIs(SameRange(findSpelled("ID ( ID ( ID ( a1 ) a2 a3 ) )";
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("b1 b2")),
-  ValueIs(SameRange(findSpelled("ID ( B )";
+  ValueIs(SameRange(findSpelled("( B").drop_front(;
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a1 a2 a3 split b1 b2")),
   ValueIs(SameRange(findSpelled(
   "ID ( ID ( ID ( a1 ) a2 a3 ) ) split ID ( B )";
-  // Ranges crossing macro call boundaries.
-  EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("a1 a2 a3 split b1")),
-llvm::None);
-  EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("a2 a3 split b1")),
-llvm::None);
-  // FIXME: next two examples should map to macro arguments, but currently they
-  //fail.
-  EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("a2")), llvm::None);
-  EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("a1 a2")), llvm::None);
+  // Mixed ranges with expanded and spelled tokens.
+  EXPECT_THAT(
+  Buffer.spelledForExpanded(findExpanded("a1 a2 a3 split")),
+  ValueIs(SameRange(findSpelled("ID ( ID ( ID ( a1 ) a2 a3 ) ) split";
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("split b1 b2")),
+  ValueIs(SameRange(findSpelled("split ID ( B )";
+  // Macro arguments
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a1")),
+  ValueIs(SameRange(findSpelled("a1";
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a2")),
+  ValueIs(SameRange(findSpelled("a2";
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a3")),
+  ValueIs(SameRange(findSpelled("a3";
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a1 a2")),
+  ValueIs(SameRange(findSpelled("ID ( a1 ) a2";
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a1 a2 a3")),
+  ValueIs(SameRange(findSpelled("ID ( a1 ) a2 a3";
 
   // Empty macro expansions.
   recordTokens(R"cpp(
@@ -674,11 +681,11 @@
 ID(7 8 9) EMPTY EMPTY
   )cpp");
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("1 2 3")),
-  ValueIs(SameRange(findSpelled("ID ( 1 2 3 )";
+  ValueIs(SameRange(findSpelled("1 2 3";
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("4 5 6")),
-  ValueIs(SameRange(findSpelled("ID ( 4 5 6 )";
+  ValueIs(SameRange(findSpelled("4 5 6";
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("7 8 9")),
-  ValueIs(SameRange(findSpelled("ID ( 7 8 9 )";
+  ValueIs(SameRange(findSpelled("7 8 9";
 
   // Empty mappings coming from various directives.
   recordTokens(R"cpp(
@@ -689,6 +696,27 @@
   )cpp");
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("not_mapped")),
   ValueIs(SameRange(findSpelled("not_mapped";
+
+  // Multiple macro arguments
+  recordTokens(R"cpp(
+#define ID(X) X
+#define ID2(X, Y) X Y
+
+ID2(ID(a1), ID(a2) a3) ID2(a4, a5 a6 a7)
+  )cpp");
+  // Should fail, spans multiple arguments.
+  EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("a1 a2")), llvm::None);
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a2 a3")),
+  ValueIs(SameRange(findSpelled("ID ( a2 ) a3";
+  EXPECT_THAT(
+  Buffer.spelledForExpanded(findExpanded("a1 a2 a3")),
+  ValueIs(SameRange(findSpelled("ID2 ( ID ( a1 ) , ID ( a2 ) a3 )";
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a5 a6")),
+  ValueIs(SameRange(findSpelled("a5 a6";
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a4 a5 a6 a7")),
+  ValueIs(SameRange(findSpelled("ID2 ( a4 , a5 a6 a7 )";
+  // Should fail, 

[PATCH] D75153: [ThinLTO] Allow usage of all SMT threads in the system

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In D75153#1947956 , @zero9178 wrote:

> This patch seems to break when disabling threading (aka LLVM_ENABLE_THREADS 
> == 0) as get_threadpool_strategy is undefined therefore creating linker 
> failures in clang. (Tested on Windows)


Taking a look now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75153



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

[clang] 9619c2c - [clang][Syntax] Handle macro arguments in spelledForExpanded

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-03-28T16:35:46+01:00
New Revision: 9619c2cc9a22a3ca1375f2f4a64e50c0a56e95d1

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

LOG: [clang][Syntax] Handle macro arguments in spelledForExpanded

Reviewers: sammccall

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/include/clang/Tooling/Syntax/Tokens.h
clang/lib/Tooling/Syntax/Tokens.cpp
clang/unittests/Tooling/Syntax/TokensTest.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/Syntax/Tokens.h 
b/clang/include/clang/Tooling/Syntax/Tokens.h
index e9918eac7845..7e50892284f4 100644
--- a/clang/include/clang/Tooling/Syntax/Tokens.h
+++ b/clang/include/clang/Tooling/Syntax/Tokens.h
@@ -197,18 +197,20 @@ class TokenBuffer {
   /// token range R.
   llvm::ArrayRef expandedTokens(SourceRange R) const;
 
-  /// Find the subrange of spelled tokens that produced the corresponding \p
-  /// Expanded tokens.
+  /// Returns the subrange of spelled tokens corresponding to AST node spanning
+  /// \p Expanded. This is the text that should be replaced if a refactoring
+  /// were to rewrite the node. If \p Expanded is empty, the returned value is
+  /// llvm::None.
   ///
-  /// EXPECTS: \p Expanded is a subrange of expandedTokens().
-  ///
-  /// Will fail if the expanded tokens do not correspond to a
-  /// sequence of spelled tokens. E.g. for the following example:
+  /// Will fail if the expanded tokens do not correspond to a sequence of
+  /// spelled tokens. E.g. for the following example:
   ///
   ///   #define FIRST f1 f2 f3
   ///   #define SECOND s1 s2 s3
+  ///   #define ID2(X, Y) X Y
   ///
   ///   a FIRST b SECOND c // expanded tokens are: a f1 f2 f3 b s1 s2 s3 c
+  ///   d ID2(e f g, h) i  // expanded tokens are: d e f g h i
   ///
   /// the results would be:
   ///   expanded   => spelled
@@ -218,8 +220,10 @@ class TokenBuffer {
   ///   a f1 f2 f3 => a FIRST
   /// a f1 => can't map
   ///s1 s2 => can't map
+  /// e f  => e f
+  /// g h  => can't map
   ///
-  /// If \p Expanded is empty, the returned value is llvm::None.
+  /// EXPECTS: \p Expanded is a subrange of expandedTokens().
   /// Complexity is logarithmic.
   llvm::Optional>
   spelledForExpanded(llvm::ArrayRef Expanded) const;

diff  --git a/clang/lib/Tooling/Syntax/Tokens.cpp 
b/clang/lib/Tooling/Syntax/Tokens.cpp
index 9e12d8b603bf..b3f13a3f6e72 100644
--- a/clang/lib/Tooling/Syntax/Tokens.cpp
+++ b/clang/lib/Tooling/Syntax/Tokens.cpp
@@ -35,6 +35,69 @@
 using namespace clang;
 using namespace clang::syntax;
 
+namespace {
+// Finds the smallest consecutive subsuquence of Toks that covers R.
+llvm::ArrayRef
+getTokensCovering(llvm::ArrayRef Toks, SourceRange R,
+  const SourceManager ) {
+  if (R.isInvalid())
+return {};
+  const syntax::Token *Begin =
+  llvm::partition_point(Toks, [&](const syntax::Token ) {
+return SM.isBeforeInTranslationUnit(T.location(), R.getBegin());
+  });
+  const syntax::Token *End =
+  llvm::partition_point(Toks, [&](const syntax::Token ) {
+return !SM.isBeforeInTranslationUnit(R.getEnd(), T.location());
+  });
+  if (Begin > End)
+return {};
+  return {Begin, End};
+}
+
+// Finds the smallest expansion range that contains expanded tokens First and
+// Last, e.g.:
+// #define ID(x) x
+// ID(ID(ID(a1) a2))
+//  ~~   -> a1
+//  ~~   -> a2
+//   ~   -> a1 a2
+SourceRange findCommonRangeForMacroArgs(const syntax::Token ,
+const syntax::Token ,
+const SourceManager ) {
+  SourceRange Res;
+  auto FirstLoc = First.location(), LastLoc = Last.location();
+  // Keep traversing up the spelling chain as longs as tokens are part of the
+  // same expansion.
+  while (!FirstLoc.isFileID() && !LastLoc.isFileID()) {
+auto ExpInfoFirst = SM.getSLocEntry(SM.getFileID(FirstLoc)).getExpansion();
+auto ExpInfoLast = SM.getSLocEntry(SM.getFileID(LastLoc)).getExpansion();
+// Stop if expansions have diverged.
+if (ExpInfoFirst.getExpansionLocStart() !=
+ExpInfoLast.getExpansionLocStart())
+  break;
+// Do not continue into macro bodies.
+if (!ExpInfoFirst.isMacroArgExpansion() ||
+!ExpInfoLast.isMacroArgExpansion())
+  break;
+FirstLoc = SM.getImmediateSpellingLoc(FirstLoc);
+LastLoc = SM.getImmediateSpellingLoc(LastLoc);
+// Update the result afterwards, as we want the tokens that triggered the
+// expansion.
+Res = {FirstLoc, LastLoc};
+  }
+  // Normally mapping back to expansion location here only changes FileID, as
+  // we've already found some tokens 

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:623
   std::string TaskName = llvm::formatv("Update ({0})", Inputs.Version);
   auto Task = [=]() mutable {
 auto RunPublish = [&](llvm::function_ref Publish) {

sammccall wrote:
> sammccall wrote:
> > this function is getting too long/monolithic, we should find a way to split 
> > it up.
> > ASTTask should be a member function, I think.
> I also lost track of the control flow halfway through the function, and can't 
> work it out.
> I don't know really what to concretely advise, but it needs to be a lot 
> simpler, maybe by finding some better abstractions or just better 
> understanding conceptually what this is doing.
> ASTTask should be a member function, I think.

Moved it into a meber named `publishDiagnostics`



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:693
+std::lock_guard Lock(Mutex);
+// LatestPreamble might be the last reference to old preamble, do not
+// trigger destructor while holding the lock.

sammccall wrote:
> Didn't we decide to make this available only after building the golden AST 
> and publishing diagnostics?
as discussed offline, this is harmless since it is being executed on ASTWorker 
thread and makes new preamble available to code completion/signature help 
faster.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76725



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

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 253347.
kadircet marked 26 inline comments as done.
kadircet added a comment.
Herald added a subscriber: jfb.

- Update description and address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76725

Files:
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp

Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -66,8 +66,7 @@
   auto CI = buildCompilerInvocation(Inputs, Diags);
   assert(CI && "Failed to build compilation invocation.");
   auto Preamble =
-  buildPreamble(FullFilename, *CI,
-/*OldPreamble=*/nullptr, Inputs,
+  buildPreamble(FullFilename, *CI, Inputs,
 /*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
   auto AST =
   buildAST(FullFilename, std::move(CI), Diags.take(), Inputs, Preamble);
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -283,6 +283,7 @@
 S.runWithPreamble("StaleRead", Path, TUScheduler::Stale,
   [&](Expected Pre) {
 ASSERT_TRUE(bool(Pre));
+ASSERT_TRUE(Pre->Preamble);
 EXPECT_EQ(Pre->Preamble->Version, "A");
 EXPECT_THAT(includes(Pre->Preamble),
 ElementsAre(""));
@@ -292,11 +293,13 @@
 S.runWithPreamble("ConsistentRead", Path, TUScheduler::Consistent,
   [&](Expected Pre) {
 ASSERT_TRUE(bool(Pre));
+ASSERT_TRUE(Pre->Preamble);
 EXPECT_EQ(Pre->Preamble->Version, "B");
 EXPECT_THAT(includes(Pre->Preamble),
 ElementsAre(""));
 ++CallbackCount;
   });
+S.blockUntilIdle(timeoutSeconds(10));
   }
   EXPECT_EQ(2, CallbackCount);
 }
@@ -853,15 +856,19 @@
   TUState(PreambleAction::Idle, ASTAction::RunningAction),
   // We build the preamble
   TUState(PreambleAction::Building, ASTAction::RunningAction),
-  // Preamble worker goes idle
+  // We built the preamble, and issued ast built on ASTWorker
+  // thread. Preambleworker goes idle afterwards.
   TUState(PreambleAction::Idle, ASTAction::RunningAction),
-  // We start building the ast
+  // Start task for building the ast, as a result of building
+  // preamble, on astworker thread.
+  TUState(PreambleAction::Idle, ASTAction::RunningAction),
+  // AST build starts.
   TUState(PreambleAction::Idle, ASTAction::Building),
-  // Built finished succesffully
+  // AST built finished successfully
   TUState(PreambleAction::Idle, ASTAction::Building),
-  // Rnning go to def
+  // Running go to def
   TUState(PreambleAction::Idle, ASTAction::RunningAction),
-  // both workers go idle
+  // ASTWorker goes idle.
   TUState(PreambleAction::Idle, ASTAction::Idle)));
 }
 
Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -286,7 +286,7 @@
 
   FileIndex Index;
   bool IndexUpdated = false;
-  buildPreamble(FooCpp, *CI, /*OldPreamble=*/nullptr, PI,
+  buildPreamble(FooCpp, *CI, PI,
 /*StoreInMemory=*/true,
 [&](ASTContext , std::shared_ptr PP,
 const CanonicalIncludes ) {
@@ -424,7 +424,7 @@
 }
 
 TEST(FileIndexTest, MergeMainFileSymbols) {
-  const char* CommonHeader = "void foo();";
+  const char *CommonHeader = "void foo();";
   TestTU Header = TestTU::withCode(CommonHeader);
   TestTU Cpp = TestTU::withCode("void foo() {}");
   Cpp.Filename = "foo.cpp";
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -5,41 +5,52 @@
 // SPDX-License-Identifier: Apache-2.0 

[PATCH] D76990: [clang-tidy]: fix false positive of cert-oop54-cpp check.

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
ztamas edited the summary of this revision.
ztamas added a reviewer: aaron.ballman.

It seems we need a different matcher for binary operator
in a template context.

Fixes this issue:
https://bugs.llvm.org/show_bug.cgi?id=44499


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76990

Files:
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
@@ -283,6 +283,21 @@
   T *p;
 };
 
+// https://bugs.llvm.org/show_bug.cgi?id=44499
+class Foo;
+template  bool operator!=(Foo&, Foo&) {
+  class Bar {
+Bar& operator=(const Bar& other)
+{
+if (this != ) {
+}
+return *this;
+}
+
+int *p;
+  };
+}
+
 // There is no warning if the copy assignment operator gets the object by 
value.
 class PassedByValue {
 public:
Index: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -40,9 +40,12 @@
 
   // Self-check: Code compares something with 'this' pointer. We don't check
   // whether it is actually the parameter what we compare.
-  const auto HasNoSelfCheck = cxxMethodDecl(unless(
+  const auto HasNoSelfCheck = cxxMethodDecl(unless(anyOf(
   hasDescendant(binaryOperator(hasAnyOperatorName("==", "!="),
-   has(ignoringParenCasts(cxxThisExpr()));
+   has(ignoringParenCasts(cxxThisExpr(),
+  hasDescendant(cxxOperatorCallExpr(
+  hasAnyOverloadedOperatorName("==", "!="), argumentCountIs(2),
+  has(ignoringParenCasts(cxxThisExpr(;
 
   // Both copy-and-swap and copy-and-move method creates a copy first and
   // assign it to 'this' with swap or move.


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment.cpp
@@ -283,6 +283,21 @@
   T *p;
 };
 
+// https://bugs.llvm.org/show_bug.cgi?id=44499
+class Foo;
+template  bool operator!=(Foo&, Foo&) {
+  class Bar {
+Bar& operator=(const Bar& other)
+{
+if (this != ) {
+}
+return *this;
+}
+
+int *p;
+  };
+}
+
 // There is no warning if the copy assignment operator gets the object by value.
 class PassedByValue {
 public:
Index: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
@@ -40,9 +40,12 @@
 
   // Self-check: Code compares something with 'this' pointer. We don't check
   // whether it is actually the parameter what we compare.
-  const auto HasNoSelfCheck = cxxMethodDecl(unless(
+  const auto HasNoSelfCheck = cxxMethodDecl(unless(anyOf(
   hasDescendant(binaryOperator(hasAnyOperatorName("==", "!="),
-   has(ignoringParenCasts(cxxThisExpr()));
+   has(ignoringParenCasts(cxxThisExpr(),
+  hasDescendant(cxxOperatorCallExpr(
+  hasAnyOverloadedOperatorName("==", "!="), argumentCountIs(2),
+  has(ignoringParenCasts(cxxThisExpr(;
 
   // Both copy-and-swap and copy-and-move method creates a copy first and
   // assign it to 'this' with swap or move.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76594: [clang][AST] Support AST files larger than 512M

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

xbolva00 added inline comments.



Comment at: clang/lib/Serialization/ASTReader.cpp:3100
   }
-  F.TypeOffsets = (const uint32_t *)Blob.data();
+  F.TypeOffsets = (const uint64_t *)Blob.data();
   F.LocalNumTypes = Record[0];

use C++ cast style


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76594



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

[PATCH] D75153: [ThinLTO] Allow usage of all SMT threads in the system

[Markus Böck via Phabricator via cfe-commits]

zero9178 added a comment.

This patch seems to break when disabling threading (aka LLVM_ENABLE_THREADS == 
0) as get_threadpool_strategy is undefined therefore creating linker failures 
in clang. (Tested on Windows)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75153



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

[PATCH] D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

On my RHEL8 system building Debug clang;clang-tools-extra, check-all is passing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76384



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

[clang] cb63893 - Fix GCC warning on enum class bitfield. NFC.

[Michael Liao via cfe-commits]

Author: Michael Liao
Date: 2020-03-28T10:20:34-04:00
New Revision: cb6389360b05e8f89d09ff133a4ba1fd011866c5

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

LOG: Fix GCC warning on enum class bitfield. NFC.

Added: 


Modified: 
clang/lib/CodeGen/CGCUDANV.cpp
clang/lib/CodeGen/CGCUDARuntime.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGCUDANV.cpp b/clang/lib/CodeGen/CGCUDANV.cpp
index ed02a7dc9173..6d92ef33b885 100644
--- a/clang/lib/CodeGen/CGCUDANV.cpp
+++ b/clang/lib/CodeGen/CGCUDANV.cpp
@@ -466,18 +466,19 @@ llvm::Function *CGNVCUDARuntime::makeRegisterGlobalsFn() {
   for (auto & : DeviceVars) {
 llvm::GlobalVariable *Var = Info.Var;
 llvm::Constant *VarName = makeConstantString(getDeviceSideName(Info.D));
-switch (Info.Flags.Kind) {
+switch (Info.Flags.getKind()) {
 case DeviceVarFlags::Variable: {
   uint64_t VarSize =
   CGM.getDataLayout().getTypeAllocSize(Var->getValueType());
-  llvm::Value *Args[] = {,
- Builder.CreateBitCast(Var, VoidPtrTy),
- VarName,
- VarName,
- llvm::ConstantInt::get(IntTy, Info.Flags.Extern),
- llvm::ConstantInt::get(IntTy, VarSize),
- llvm::ConstantInt::get(IntTy, 
Info.Flags.Constant),
- llvm::ConstantInt::get(IntTy, 0)};
+  llvm::Value *Args[] = {
+  ,
+  Builder.CreateBitCast(Var, VoidPtrTy),
+  VarName,
+  VarName,
+  llvm::ConstantInt::get(IntTy, Info.Flags.isExtern()),
+  llvm::ConstantInt::get(IntTy, VarSize),
+  llvm::ConstantInt::get(IntTy, Info.Flags.isConstant()),
+  llvm::ConstantInt::get(IntTy, 0)};
   Builder.CreateCall(RegisterVar, Args);
   break;
 }
@@ -485,16 +486,16 @@ llvm::Function *CGNVCUDARuntime::makeRegisterGlobalsFn() {
   Builder.CreateCall(
   RegisterSurf,
   {, Builder.CreateBitCast(Var, VoidPtrTy), VarName,
-   VarName, llvm::ConstantInt::get(IntTy, Info.Flags.SurfTexType),
-   llvm::ConstantInt::get(IntTy, Info.Flags.Extern)});
+   VarName, llvm::ConstantInt::get(IntTy, Info.Flags.getSurfTexType()),
+   llvm::ConstantInt::get(IntTy, Info.Flags.isExtern())});
   break;
 case DeviceVarFlags::Texture:
   Builder.CreateCall(
   RegisterTex,
   {, Builder.CreateBitCast(Var, VoidPtrTy), VarName,
-   VarName, llvm::ConstantInt::get(IntTy, Info.Flags.SurfTexType),
-   llvm::ConstantInt::get(IntTy, Info.Flags.Normalized),
-   llvm::ConstantInt::get(IntTy, Info.Flags.Extern)});
+   VarName, llvm::ConstantInt::get(IntTy, Info.Flags.getSurfTexType()),
+   llvm::ConstantInt::get(IntTy, Info.Flags.isNormalized()),
+   llvm::ConstantInt::get(IntTy, Info.Flags.isExtern())});
   break;
 }
   }

diff  --git a/clang/lib/CodeGen/CGCUDARuntime.h 
b/clang/lib/CodeGen/CGCUDARuntime.h
index b26132420d65..19e70a2022a5 100644
--- a/clang/lib/CodeGen/CGCUDARuntime.h
+++ b/clang/lib/CodeGen/CGCUDARuntime.h
@@ -42,17 +42,30 @@ class CGCUDARuntime {
 
 public:
   // Global variable properties that must be passed to CUDA runtime.
-  struct DeviceVarFlags {
-enum DeviceVarKind : unsigned {
+  class DeviceVarFlags {
+  public:
+enum DeviceVarKind {
   Variable, // Variable
   Surface,  // Builtin surface
   Texture,  // Builtin texture
 };
-DeviceVarKind Kind : 2;
+
+  private:
+unsigned Kind : 2;
 unsigned Extern : 1;
 unsigned Constant : 1;   // Constant variable.
 unsigned Normalized : 1; // Normalized texture.
 int SurfTexType; // Type of surface/texutre.
+
+  public:
+DeviceVarFlags(DeviceVarKind K, bool E, bool C, bool N, int T)
+: Kind(K), Extern(E), Constant(C), Normalized(N), SurfTexType(T) {}
+
+DeviceVarKind getKind() const { return static_cast(Kind); }
+bool isExtern() const { return Extern; }
+bool isConstant() const { return Constant; }
+bool isNormalized() const { return Normalized; }
+int getSurfTexType() const { return SurfTexType; }
   };
 
   CGCUDARuntime(CodeGenModule ) : CGM(CGM) {}



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

[PATCH] D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 253341.
mibintc marked 12 inline comments as done.
mibintc added a comment.

@rjmccall Many thanks for your code review.  Please take a look at this when 
you get a chance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76384

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/StmtVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/Analysis/ReachableCode.cpp
  clang/lib/Analysis/ThreadSafetyCommon.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Frontend/Rewrite/RewriteObjC.cpp
  clang/lib/Index/IndexBody.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaPseudoObject.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/AST/ast-dump-expr-json.c
  clang/test/AST/ast-dump-expr.c
  clang/test/AST/dump.cpp
  clang/test/Import/compound-assign-op/test.cpp
  clang/tools/libclang/CXCursor.cpp

Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -427,10 +427,6 @@
 K = CXCursor_BinaryOperator;
 break;
 
-  case Stmt::CompoundAssignOperatorClass:
-K = CXCursor_CompoundAssignOperator;
-break;
-
   case Stmt::ConditionalOperatorClass:
 K = CXCursor_ConditionalOperator;
 break;
Index: clang/test/Import/compound-assign-op/test.cpp
===
--- clang/test/Import/compound-assign-op/test.cpp
+++ clang/test/Import/compound-assign-op/test.cpp
@@ -2,42 +2,42 @@
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '+='
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '-='
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '*='
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '/='
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '&='
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '^='
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '<<='
 
 // CHECK: VarDecl
 // CHECK-NEXT: Integer
-// CHECK-NEXT: CompoundAssignOperator
+// CHECK-NEXT: BinaryOperator
 // CHECK-SAME: '>>='
 
 void expr() {
Index: clang/test/AST/dump.cpp
===
--- clang/test/AST/dump.cpp
+++ clang/test/AST/dump.cpp
@@ -14,14 +14,14 @@
 #pragma omp declare reduction(fun : float : omp_out += omp_in) initializer(omp_priv = omp_orig + 15)
 
 // CHECK:  |-OMPDeclareReductionDecl {{.+}}  col:35 operator+ 'int' combiner 0x{{.+}}
-// CHECK-NEXT: | |-CompoundAssignOperator {{.+}}  'int' lvalue '*=' ComputeLHSTy='int' ComputeResultTy='int'
+// CHECK-NEXT: | |-BinaryOperator {{.+}}  'int' lvalue '*=' ComputeLHSTy='int' ComputeResultTy='int'
 // CHECK-NEXT: | | |-DeclRefExpr {{.+}}  'int' lvalue Var {{.+}} 'omp_out' 'int'
 // CHECK-NEXT: | | `-ImplicitCastExpr {{.+}}  'int' 
 // CHECK-NEXT: | |   `-DeclRefExpr {{.+}}  'int' lvalue Var {{.+}} 'omp_in' 'int'
 // CHECK-NEXT: | |-VarDecl {{.+}}  col:35 implicit used omp_in 'int'
 // CHECK-NEXT: | `-VarDecl {{.+}}  col:35 implicit used omp_out 'int'
 // 

[PATCH] D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage

[Melanie Blower via Phabricator via cfe-commits]

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

I added some inline replies to John.  I'm not certain I have everything yet 
exactly the way he wants it.




Comment at: clang/include/clang/AST/Expr.h:2573
 
+  FPOptions FPFeatures;
+

rjmccall wrote:
> Let's split the CallExpr changes out into a separate patch, so that we have 
> one patch that's *purely* about unifying BinaryOperator and 
> CompoundAssignOperator and putting FPOptions in trailing storage, and then 
> another that's about adding FPOptions to CallExpr.
> 
> For that latter patch, CallExpr already has its own, hand-rolled concept of 
> trailing storage which you should be able to emulate instead of unifying the 
> hierarchy.
I've split off the CallExpr changes to a future patch - will you give me a hint 
about CallExpr's own hand-rolled concept of trailing storage? e.g. a line 
number or identifier name that i can hunt for and track down.



Comment at: clang/include/clang/AST/Expr.h:3417
+: public Expr,
+  private llvm::TrailingObjects {
   enum { LHS, RHS, END_EXPR };

rjmccall wrote:
> Why does this use `unsigned` for the trailing storage of the `FPOptions`?
I will change it to FPOptions



Comment at: clang/include/clang/AST/Expr.h:3420
   Stmt *SubExprs[END_EXPR];
+  bool hasFPFeatures;
+  bool isCompoundAssign;

I think I need hasFPFeatures in the node itself because that's how i know 
whether there's trailingstorage e.g. in the AST reader-writer.  hasFPFeatures 
is always true at the moment but that will be improved in the next generation. 



Comment at: clang/include/clang/AST/Expr.h:3421
+  bool hasFPFeatures;
+  bool isCompoundAssign;
+

rjmccall wrote:
> Can these bits go in the bitfields?
i eliminated isCompoundAssign because, i can just check the opcode, i think 
there's always a opcode or a dummy opcode. But I need hasFPFeatures



Comment at: clang/include/clang/AST/Expr.h:3465
+SubExprs[RHS] = rhs;
+hasFPFeatures = true;
+isCompoundAssign = 1;

rjmccall wrote:
> mibintc wrote:
> > rjmccall wrote:
> > > Okay, so this is *always* adding trailing storage.  Is there a common 
> > > value for `FPOptions` — something that corresponds to default settings, 
> > > for example — that we could use to avoid needing storage in the common 
> > > case?
> > > 
> > > Also, it would be useful to extract a function somewhere that you can use 
> > > in all of these places for consistency, maybe something like this on 
> > > `FPOptions`:
> > > 
> > > ```
> > >   /// Does this FPOptions require trailing storage when stored in various 
> > > AST nodes,
> > >   /// or can it be recreated using `defaultWithoutTrailingStorage`?
> > >   bool requiresTrailingStorage() const { return *this == 
> > > defaultWithoutTrailingStorage(); }
> > > 
> > >   /// Return the default value of FPOptions that's used when trailing 
> > > storage isn't required.
> > >   static FPOptions defaultWithoutTrailingStorage() { ... }
> > > ```
> > The reason I set hasFPFeatures to True in this revision is because in the 
> > previous review you asked me to make it purely a representational change 
> > and "add stuff about whether there is a pragma in a separate patch".  The 
> > stuff I had in the previous version was smarter about creating trailing 
> > storage, it only created trailing storage when the pragma was in effect. I 
> > envirsioned that the evolution would accept something that always created 
> > the FPOptions in trailing storage, and in the 2nd generation would adopt 
> > the more thrifty way of creating trailing storage. 
> Well, let's at least set up the infrastructure and APIs correctly, even if we 
> always allocate trailing storage.
> 
> For example, should `getFPOptions` take an `ASTContext &` so that it can 
> always return the right options instead of making clients do `hasFPOptions() 
> ? getFPOptions() : ctx.getFPOptions()`?
To explain further, in the pragma patch, which is fully formed but split off 
for a future commit, the Sema structure holds the current value of the floating 
point settings (in FPOptions FPFeatures). It is initialized from the command 
line, and as compound statements/blocks which contain pragma's are entered and 
exited, the value of Sema.FPFeatures is updated.  I will add a bit 
"has_pragma_features" to FPOptions. When that bit is true, I know that the 
initial value of FPOptions (which is derived from command line) is different 
than the current settings. In this circumstance only it is necessary to hold 
FPFeatures in the Expr nodes (in Trailing storage).  In all other cases, 
FPOptions can be derived from LangOpts and LangOpts is available in each clang 
pass.  There are a bunch of occurrences where FPOptions() has been added as an 
nth parameter to various function calls, i believe though I'm not certain that 
in those cases 

[PATCH] D75446: [clang][Syntax] Handle macro arguments in spelledForExpanded

[Sam McCall via Phabricator via cfe-commits]

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

Thanks!




Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:201
+  /// Returns the subrange of spelled tokens corresponding to AST node spanning
+  /// \p Expanded. If \p Expanded is empty, the returned value is llvm::None.
   ///

Can we add the motivating use case?
e.g. between the two sentences: "This is the text that should be replaced if a 
refactoring were to rewrite the node."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75446



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

[PATCH] D76987: Rename options --cuda-gpu-arch and --no-cuda-gpu-arch

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.
yaxunl edited the summary of this revision.

Options --cuda-gpu-arch and --no-cuda-gpu-arch are shared between
CUDA and HIP. It is desirable to rename them for more generic
names to avoid confusion.

One option is --gpu-arch and --no-gpu-arch. However HIP may be
ported to other devices not limited to gpu, so it may be better
to name them as --offload-arch and --no-offload-arch.

Other choices may be:

--device-arch
--offload-device-arch
--accelerator-arch
--aux-arch
--aux-cpu

The original options will be alias to the new options.


https://reviews.llvm.org/D76987

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


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2526,13 +2526,13 @@
   std::set GpuArchs;
   bool Error = false;
   for (Arg *A : Args) {
-if (!(A->getOption().matches(options::OPT_cuda_gpu_arch_EQ) ||
-  A->getOption().matches(options::OPT_no_cuda_gpu_arch_EQ)))
+if (!(A->getOption().matches(options::OPT_offload_arch_EQ) ||
+  A->getOption().matches(options::OPT_no_offload_arch_EQ)))
   continue;
 A->claim();
 
 const StringRef ArchStr = A->getValue();
-if (A->getOption().matches(options::OPT_no_cuda_gpu_arch_EQ) &&
+if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
 ArchStr == "all") {
   GpuArchs.clear();
   continue;
@@ -2541,9 +2541,9 @@
 if (Arch == CudaArch::UNKNOWN) {
   C.getDriver().Diag(clang::diag::err_drv_cuda_bad_gpu_arch) << 
ArchStr;
   Error = true;
-} else if (A->getOption().matches(options::OPT_cuda_gpu_arch_EQ))
+} else if (A->getOption().matches(options::OPT_offload_arch_EQ))
   GpuArchs.insert(Arch);
-else if (A->getOption().matches(options::OPT_no_cuda_gpu_arch_EQ))
+else if (A->getOption().matches(options::OPT_no_offload_arch_EQ))
   GpuArchs.erase(Arch);
 else
   llvm_unreachable("Unexpected option.");
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -571,13 +571,17 @@
   HelpText<"Include PTX for the following GPU architecture (e.g. sm_35) or 
'all'. May be specified more than once.">;
 def no_cuda_include_ptx_EQ : Joined<["--"], "no-cuda-include-ptx=">, 
Flags<[DriverOption]>,
   HelpText<"Do not include PTX for the following GPU architecture (e.g. sm_35) 
or 'all'. May be specified more than once.">;
+def offload_arch_EQ : Joined<["--"], "offload-arch=">, Flags<[DriverOption]>,
+  HelpText<"CUDA/HIP offloading device architecture (e.g. sm_35, gfx906).  May 
be specified more than once.">;
 def cuda_gpu_arch_EQ : Joined<["--"], "cuda-gpu-arch=">, Flags<[DriverOption]>,
-  HelpText<"CUDA GPU architecture (e.g. sm_35).  May be specified more than 
once.">;
+  Alias;
 def hip_link : Flag<["--"], "hip-link">,
   HelpText<"Link clang-offload-bundler bundles for HIP">;
-def no_cuda_gpu_arch_EQ : Joined<["--"], "no-cuda-gpu-arch=">, 
Flags<[DriverOption]>,
-  HelpText<"Remove GPU architecture (e.g. sm_35) from the list of GPUs to 
compile for. "
+def no_offload_arch_EQ : Joined<["--"], "no-offload-arch=">, 
Flags<[DriverOption]>,
+  HelpText<"Remove CUDA/HIP offloading device architecture (e.g. sm_35, 
gfx906) from the list of devices to compile for. "
"'all' resets the list to its default value.">;
+def no_cuda_gpu_arch_EQ : Joined<["--"], "no-cuda-gpu-arch=">, 
Flags<[DriverOption]>,
+  Alias;
 def cuda_noopt_device_debug : Flag<["--"], "cuda-noopt-device-debug">,
   HelpText<"Enable device-side debug info generation. Disables ptxas 
optimizations.">;
 def no_cuda_version_check : Flag<["--"], "no-cuda-version-check">,


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2526,13 +2526,13 @@
   std::set GpuArchs;
   bool Error = false;
   for (Arg *A : Args) {
-if (!(A->getOption().matches(options::OPT_cuda_gpu_arch_EQ) ||
-  A->getOption().matches(options::OPT_no_cuda_gpu_arch_EQ)))
+if (!(A->getOption().matches(options::OPT_offload_arch_EQ) ||
+  A->getOption().matches(options::OPT_no_offload_arch_EQ)))
   continue;
 A->claim();
 
 const StringRef ArchStr = A->getValue();
-if (A->getOption().matches(options::OPT_no_cuda_gpu_arch_EQ) &&
+if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
 ArchStr == "all") {
   GpuArchs.clear();
   continue;
@@ -2541,9 +2541,9 @@
 if (Arch == CudaArch::UNKNOWN) {
   

[PATCH] D76943: [clang analysis] Make mutex guard detection more reliable.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76943



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

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert updated this revision to Diff 253327.
fickert added a comment.

I moved the changed behavior to a new option (`UT_AlignWithSpaces`), and added 
corresponding unit tests by copying and adapting the tests for 
`UT_ForContinuationAndIndentation`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75034

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10314,15 +10314,244 @@
"\t*/\n"
"}",
Tab));
+  EXPECT_EQ("/* some\n"
+"   comment */",
+format(" \t \t /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("int a; /* some\n"
+"   comment */",
+format(" \t \t int a; /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("int a; /* some\n"
+"comment */",
+format(" \t \t int\ta; /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("f(\"\t\t\"); /* some\n"
+"comment */",
+format(" \t \t f(\"\t\t\"); /* some\n"
+   " \t \tcomment */",
+   Tab));
+  EXPECT_EQ("{\n"
+"  /*\n"
+"   * Comment\n"
+"   */\n"
+"  int i;\n"
+"}",
+format("{\n"
+   "\t/*\n"
+   "\t * Comment\n"
+   "\t */\n"
+   "\t int i;\n"
+   "}"));
+  Tab.TabWidth = 2;
+  Tab.IndentWidth = 2;
+  EXPECT_EQ("{\n"
+"\t/* \n"
+"\t\t  */\n"
+"}",
+format("{\n"
+   "/* \n"
+   "\t  */\n"
+   "}",
+   Tab));
+  EXPECT_EQ("{\n"
+"\t/*\n"
+"\t\taa\n"
+"\t\tb\n"
+"\t*/\n"
+"}",
+format("{\n"
+   "/*\n"
+   "\taa b\n"
+   "*/\n"
+   "}",
+   Tab));
+  Tab.AlignConsecutiveAssignments = true;
+  Tab.AlignConsecutiveDeclarations = true;
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 4;
+  verifyFormat("class Assign {\n"
+   "\tvoid f() {\n"
+   "\t\tint x  = 123;\n"
+   "\t\tint random = 4;\n"
+   "\t\tstd::string alphabet =\n"
+   "\t\t\t\"abcdefghijklmnopqrstuvwxyz\";\n"
+   "\t}\n"
+   "};",
+   Tab);
+
+  Tab.UseTab = FormatStyle::UT_AlignWithSpaces;
+  Tab.TabWidth = 8;
+  Tab.IndentWidth = 8;
+  EXPECT_EQ("if ( && // q\n"
+"bb) // w\n"
+"\t;",
+format("if ( &&// q\n"
+   "bb)// w\n"
+   ";",
+   Tab));
+  EXPECT_EQ("if (aaa && bbb) // w\n"
+"\t;",
+format("if(aaa&)// w\n"
+   ";",
+   Tab));
+  verifyFormat("class X {\n"
+   "\tvoid f() {\n"
+   "\t\tsomeFunction(parameter1,\n"
+   "\t\t parameter2);\n"
+   "\t}\n"
+   "};",
+   Tab);
+  verifyFormat("#define A\\\n"
+   "\tvoid f() {   \\\n"
+   "\t\tsomeFunction(\\\n"
+   "\t\tparameter1,  \\\n"
+   "\t\tparameter2); \\\n"
+   "\t}",
+   Tab);
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 8;
+  verifyFormat("class TabWidth4Indent8 {\n"
+   "\t\tvoid f() {\n"
+   "\t\t\t\tsomeFunction(parameter1,\n"
+   "\t\t\t\t parameter2);\n"
+   "\t\t}\n"
+   "};",
+   Tab);
+  Tab.TabWidth = 4;
+  Tab.IndentWidth = 4;
+  verifyFormat("class TabWidth4Indent4 {\n"
+   "\tvoid f() {\n"
+   "\t\tsomeFunction(parameter1,\n"
+   "\t\t parameter2);\n"
+   "\t}\n"
+   "};",
+   Tab);
+  Tab.TabWidth = 8;
+  Tab.IndentWidth = 4;
+  verifyFormat("class TabWidth8Indent4 {\n"
+   "void f() {\n"
+   "\tsomeFunction(parameter1,\n"
+   "\t 

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Maximilian Fickert via Phabricator via cfe-commits]

fickert added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:10146
"\t int i;\n"
"}"));
 

This test seems to be missing the tab configuration (`Tab`) as an argument to 
`format`, and the same test with missing `Tab` argument appears again in line 
10337. Adding the `Tab` argument will also change the expected result. I think 
the test case here should look like this:

```
  EXPECT_EQ("{\n"
"/*\n"
" * Comment\n"
" */\n"
"int i;\n"
"}",
format("{\n"
   "\t/*\n"
   "\t * Comment\n"
   "\t */\n"
   "\t int i;\n"
   "}",
   Tab));
```

and a similar change should be made to the one in line 10337.

I assume this should be fixed in a separate commit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75034



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

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:708-710
+return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName,
+M1.Message) <
+   std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName, 
M2.Message);

This looks like a clang-format artifact, there are several other below. Could 
these be removed from this patch



Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:64
 
-MATCHER_P(FixMessage, Message, "") {
-  return arg.Message == Message;
-}
+MATCHER_P(FixMessage, Message, "") { return arg.Message == Message; }
 

ditto



Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:264-265
+  // Verify that we don't have "[check-name]" suffix in the 
message.
+  WithFix(FixMessage(
+  "use a trailing return type for this function");
 }

ditto



Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:841-842
 
-  EXPECT_THAT(TU.build().getDiagnostics(),
-  ElementsAre(Diag(Test.range(), "use of undeclared identifier 
'a'")));
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'")));

ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75286



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

[PATCH] D75446: [clang][Syntax] Handle macro arguments in spelledForExpanded

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Rebase, update comments for spelledForExpanded and add tests for multiple 
arguments.
- Do not dive into macro bodies while looking for a common expansion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75446

Files:
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -627,6 +627,7 @@
 
 A split B
   )cpp");
+  // Ranges going across expansion boundaries.
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a1 a2 a3 split b1 b2")),
   ValueIs(SameRange(findSpelled("A split B";
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a1 a2 a3")),
@@ -647,22 +648,28 @@
 ID(ID(ID(a1) a2 a3)) split ID(B)
   )cpp");
 
-  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a1 a2 a3")),
-  ValueIs(SameRange(findSpelled("ID ( ID ( ID ( a1 ) a2 a3 ) )";
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("b1 b2")),
-  ValueIs(SameRange(findSpelled("ID ( B )";
+  ValueIs(SameRange(findSpelled("( B").drop_front(;
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a1 a2 a3 split b1 b2")),
   ValueIs(SameRange(findSpelled(
   "ID ( ID ( ID ( a1 ) a2 a3 ) ) split ID ( B )";
-  // Ranges crossing macro call boundaries.
-  EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("a1 a2 a3 split b1")),
-llvm::None);
-  EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("a2 a3 split b1")),
-llvm::None);
-  // FIXME: next two examples should map to macro arguments, but currently they
-  //fail.
-  EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("a2")), llvm::None);
-  EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("a1 a2")), llvm::None);
+  // Mixed ranges with expanded and spelled tokens.
+  EXPECT_THAT(
+  Buffer.spelledForExpanded(findExpanded("a1 a2 a3 split")),
+  ValueIs(SameRange(findSpelled("ID ( ID ( ID ( a1 ) a2 a3 ) ) split";
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("split b1 b2")),
+  ValueIs(SameRange(findSpelled("split ID ( B )";
+  // Macro arguments
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a1")),
+  ValueIs(SameRange(findSpelled("a1";
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a2")),
+  ValueIs(SameRange(findSpelled("a2";
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a3")),
+  ValueIs(SameRange(findSpelled("a3";
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a1 a2")),
+  ValueIs(SameRange(findSpelled("ID ( a1 ) a2";
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a1 a2 a3")),
+  ValueIs(SameRange(findSpelled("ID ( a1 ) a2 a3";
 
   // Empty macro expansions.
   recordTokens(R"cpp(
@@ -674,11 +681,11 @@
 ID(7 8 9) EMPTY EMPTY
   )cpp");
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("1 2 3")),
-  ValueIs(SameRange(findSpelled("ID ( 1 2 3 )";
+  ValueIs(SameRange(findSpelled("1 2 3";
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("4 5 6")),
-  ValueIs(SameRange(findSpelled("ID ( 4 5 6 )";
+  ValueIs(SameRange(findSpelled("4 5 6";
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("7 8 9")),
-  ValueIs(SameRange(findSpelled("ID ( 7 8 9 )";
+  ValueIs(SameRange(findSpelled("7 8 9";
 
   // Empty mappings coming from various directives.
   recordTokens(R"cpp(
@@ -689,6 +696,27 @@
   )cpp");
   EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("not_mapped")),
   ValueIs(SameRange(findSpelled("not_mapped";
+
+  // Multiple macro arguments
+  recordTokens(R"cpp(
+#define ID(X) X
+#define ID2(X, Y) X Y
+
+ID2(ID(a1), ID(a2) a3) ID2(a4, a5 a6 a7)
+  )cpp");
+  // Should fail, spans multiple arguments.
+  EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("a1 a2")), llvm::None);
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a2 a3")),
+  ValueIs(SameRange(findSpelled("ID ( a2 ) a3";
+  EXPECT_THAT(
+  Buffer.spelledForExpanded(findExpanded("a1 a2 a3")),
+  ValueIs(SameRange(findSpelled("ID2 ( ID ( a1 ) , ID ( a2 ) a3 )";
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a5 a6")),
+  ValueIs(SameRange(findSpelled("a5 a6";
+  EXPECT_THAT(Buffer.spelledForExpanded(findExpanded("a4 a5 a6 a7")),
+  ValueIs(SameRange(findSpelled("ID2 ( a4 , a5 a6 a7 )";
+  // Should fail, spans multiple invocations.
+  EXPECT_EQ(Buffer.spelledForExpanded(findExpanded("a1 

[PATCH] D76979: [clang][llvm] Interface Stubs new yaml file format changes.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 253314.
plotfi added a comment.
Herald added a subscriber: wuzish.

-U999


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76979

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/blocks.c
  clang/test/InterfaceStubs/class-template-partial-specialization.cpp
  clang/test/InterfaceStubs/conflict-type.ifs
  clang/test/InterfaceStubs/constructor-using-shadow.cpp
  clang/test/InterfaceStubs/cxx-conversion.cpp
  clang/test/InterfaceStubs/cxxdeduction-guide.cpp
  clang/test/InterfaceStubs/driver-test3.c
  clang/test/InterfaceStubs/empty.c
  clang/test/InterfaceStubs/func.ifs
  clang/test/InterfaceStubs/hidden-class-inheritance.cpp
  clang/test/InterfaceStubs/indirect-field-decl.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/lambda.cpp
  clang/test/InterfaceStubs/namespace-alias.cpp
  clang/test/InterfaceStubs/namespace.cpp
  clang/test/InterfaceStubs/non-type-template-parm-decl.cpp
  clang/test/InterfaceStubs/object.c
  clang/test/InterfaceStubs/object.ifs
  clang/test/InterfaceStubs/ppc.cpp
  clang/test/InterfaceStubs/template-constexpr.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/template-template-parm-decl.cpp
  clang/test/InterfaceStubs/trycatch.cpp
  clang/test/InterfaceStubs/unresolved-using-typename.cpp
  clang/test/InterfaceStubs/usings.cpp
  clang/test/InterfaceStubs/var-template-specialization-decl.cpp
  clang/test/InterfaceStubs/weak.cpp
  clang/test/InterfaceStubs/windows.cpp
  llvm/test/tools/llvm-ifs/Inputs/strong-mismatch-size.ifs
  llvm/test/tools/llvm-ifs/Inputs/strong-mismatch-type.ifs
  llvm/test/tools/llvm-ifs/conflict-header-format.ifs
  llvm/test/tools/llvm-ifs/conflict-header-triple.ifs
  llvm/test/tools/llvm-ifs/conflict-header-version.ifs
  llvm/test/tools/llvm-ifs/conflict-size.ifs
  llvm/test/tools/llvm-ifs/conflict-type.ifs
  llvm/test/tools/llvm-ifs/conflict-weak.ifs
  llvm/test/tools/llvm-ifs/default-empty.ifs
  llvm/test/tools/llvm-ifs/empty1.ifs
  llvm/test/tools/llvm-ifs/empty2.ifs
  llvm/test/tools/llvm-ifs/func.ifs
  llvm/test/tools/llvm-ifs/ios-tbd.ifs
  llvm/test/tools/llvm-ifs/macos-tbd.ifs
  llvm/test/tools/llvm-ifs/object-function-size-weak-combo.ifs
  llvm/test/tools/llvm-ifs/object.ifs
  llvm/test/tools/llvm-ifs/strong.ifs
  llvm/test/tools/llvm-ifs/tvos-tbd.ifs
  llvm/test/tools/llvm-ifs/version-ok.ifs
  llvm/test/tools/llvm-ifs/watchos-tbd.ifs
  llvm/test/tools/llvm-ifs/weak-mismatch.ifs
  llvm/test/tools/llvm-ifs/weak.ifs
  llvm/tools/llvm-ifs/llvm-ifs.cpp

Index: llvm/tools/llvm-ifs/llvm-ifs.cpp
===
--- llvm/tools/llvm-ifs/llvm-ifs.cpp
+++ llvm/tools/llvm-ifs/llvm-ifs.cpp
@@ -26,6 +26,7 @@
 #include "llvm/TextAPI/MachO/TextAPIWriter.h"
 #include 
 #include 
+#include 
 
 using namespace llvm;
 using namespace llvm::yaml;
@@ -34,8 +35,8 @@
 #define DEBUG_TYPE "llvm-ifs"
 
 namespace {
-const VersionTuple IFSVersionCurrent(1, 2);
-}
+const VersionTuple IFSVersionCurrent(2, 0);
+} // end anonymous namespace
 
 static cl::opt Action("action", cl::desc(""),
cl::value_desc("write-ifs | write-bin"),
@@ -76,6 +77,7 @@
 }
 
 struct IFSSymbol {
+  IFSSymbol() = default;
   IFSSymbol(std::string SymbolName) : Name(SymbolName) {}
   std::string Name;
   uint64_t Size;
@@ -85,6 +87,8 @@
   bool operator<(const IFSSymbol ) const { return Name < RHS.Name; }
 };
 
+LLVM_YAML_IS_SEQUENCE_VECTOR(IFSSymbol)
+
 namespace llvm {
 namespace yaml {
 /// YAML traits for IFSSymbolType.
@@ -124,6 +128,7 @@
 /// YAML traits for IFSSymbol.
 template <> struct MappingTraits {
   static void mapping(IO , IFSSymbol ) {
+IO.mapRequired("Name", Symbol.Name);
 IO.mapRequired("Type", Symbol.Type);
 // The need for symbol size depends on the symbol type.
 if (Symbol.Type == IFSSymbolType::NoType)
@@ -140,20 +145,6 @@
   static const bool flow = true;
 };
 
-/// YAML traits for set of IFSSymbols.
-template <> struct CustomMappingTraits> {
-  static void inputOne(IO , StringRef Key, std::set ) {
-std::string Name = Key.str();
-IFSSymbol Sym(Name);
-IO.mapRequired(Name.c_str(), Sym);
-Set.insert(Sym);
-  }
-
-  static void output(IO , std::set ) {
-for (auto  : Set)
-  IO.mapRequired(Sym.Name.c_str(), const_cast(Sym));
-  }
-};
 } // namespace yaml
 } // namespace llvm
 
@@ -167,7 +158,7 @@
   std::string ObjectFileFormat;
   Optional SOName;
   std::vector NeededLibs;
-  std::set Symbols;
+  std::vector Symbols;
 
   IFSStub() = default;
   IFSStub(const IFSStub )
@@ -186,14 +177,18 @@
 /// YAML traits for IFSStub objects.
 template <> struct MappingTraits {
   static void mapping(IO , IFSStub ) {
-if (!IO.mapTag("!experimental-ifs-v1", true))
+

[PATCH] D76979: [clang][llvm] Interface Stubs new yaml file format changes.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi created this revision.
plotfi added a reviewer: compnerd.
Herald added subscribers: cfe-commits, kbarton, nemanjai.
Herald added a project: clang.
plotfi updated this revision to Diff 253314.
plotfi added a comment.
Herald added a subscriber: wuzish.

-U999


These file format changes have been a long time coming. Essentially instead of 
a yaml map mapping trait we now have a sequence. This generally makes the 
format work better for cases where the symbol list is empty.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76979

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/blocks.c
  clang/test/InterfaceStubs/class-template-partial-specialization.cpp
  clang/test/InterfaceStubs/conflict-type.ifs
  clang/test/InterfaceStubs/constructor-using-shadow.cpp
  clang/test/InterfaceStubs/cxx-conversion.cpp
  clang/test/InterfaceStubs/cxxdeduction-guide.cpp
  clang/test/InterfaceStubs/driver-test3.c
  clang/test/InterfaceStubs/empty.c
  clang/test/InterfaceStubs/func.ifs
  clang/test/InterfaceStubs/hidden-class-inheritance.cpp
  clang/test/InterfaceStubs/indirect-field-decl.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/lambda.cpp
  clang/test/InterfaceStubs/namespace-alias.cpp
  clang/test/InterfaceStubs/namespace.cpp
  clang/test/InterfaceStubs/non-type-template-parm-decl.cpp
  clang/test/InterfaceStubs/object.c
  clang/test/InterfaceStubs/object.ifs
  clang/test/InterfaceStubs/ppc.cpp
  clang/test/InterfaceStubs/template-constexpr.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/template-template-parm-decl.cpp
  clang/test/InterfaceStubs/trycatch.cpp
  clang/test/InterfaceStubs/unresolved-using-typename.cpp
  clang/test/InterfaceStubs/usings.cpp
  clang/test/InterfaceStubs/var-template-specialization-decl.cpp
  clang/test/InterfaceStubs/weak.cpp
  clang/test/InterfaceStubs/windows.cpp
  llvm/test/tools/llvm-ifs/Inputs/strong-mismatch-size.ifs
  llvm/test/tools/llvm-ifs/Inputs/strong-mismatch-type.ifs
  llvm/test/tools/llvm-ifs/conflict-header-format.ifs
  llvm/test/tools/llvm-ifs/conflict-header-triple.ifs
  llvm/test/tools/llvm-ifs/conflict-header-version.ifs
  llvm/test/tools/llvm-ifs/conflict-size.ifs
  llvm/test/tools/llvm-ifs/conflict-type.ifs
  llvm/test/tools/llvm-ifs/conflict-weak.ifs
  llvm/test/tools/llvm-ifs/default-empty.ifs
  llvm/test/tools/llvm-ifs/empty1.ifs
  llvm/test/tools/llvm-ifs/empty2.ifs
  llvm/test/tools/llvm-ifs/func.ifs
  llvm/test/tools/llvm-ifs/ios-tbd.ifs
  llvm/test/tools/llvm-ifs/macos-tbd.ifs
  llvm/test/tools/llvm-ifs/object-function-size-weak-combo.ifs
  llvm/test/tools/llvm-ifs/object.ifs
  llvm/test/tools/llvm-ifs/strong.ifs
  llvm/test/tools/llvm-ifs/tvos-tbd.ifs
  llvm/test/tools/llvm-ifs/version-ok.ifs
  llvm/test/tools/llvm-ifs/watchos-tbd.ifs
  llvm/test/tools/llvm-ifs/weak-mismatch.ifs
  llvm/test/tools/llvm-ifs/weak.ifs
  llvm/tools/llvm-ifs/llvm-ifs.cpp

Index: llvm/tools/llvm-ifs/llvm-ifs.cpp
===
--- llvm/tools/llvm-ifs/llvm-ifs.cpp
+++ llvm/tools/llvm-ifs/llvm-ifs.cpp
@@ -26,6 +26,7 @@
 #include "llvm/TextAPI/MachO/TextAPIWriter.h"
 #include 
 #include 
+#include 
 
 using namespace llvm;
 using namespace llvm::yaml;
@@ -34,8 +35,8 @@
 #define DEBUG_TYPE "llvm-ifs"
 
 namespace {
-const VersionTuple IFSVersionCurrent(1, 2);
-}
+const VersionTuple IFSVersionCurrent(2, 0);
+} // end anonymous namespace
 
 static cl::opt Action("action", cl::desc(""),
cl::value_desc("write-ifs | write-bin"),
@@ -76,6 +77,7 @@
 }
 
 struct IFSSymbol {
+  IFSSymbol() = default;
   IFSSymbol(std::string SymbolName) : Name(SymbolName) {}
   std::string Name;
   uint64_t Size;
@@ -85,6 +87,8 @@
   bool operator<(const IFSSymbol ) const { return Name < RHS.Name; }
 };
 
+LLVM_YAML_IS_SEQUENCE_VECTOR(IFSSymbol)
+
 namespace llvm {
 namespace yaml {
 /// YAML traits for IFSSymbolType.
@@ -124,6 +128,7 @@
 /// YAML traits for IFSSymbol.
 template <> struct MappingTraits {
   static void mapping(IO , IFSSymbol ) {
+IO.mapRequired("Name", Symbol.Name);
 IO.mapRequired("Type", Symbol.Type);
 // The need for symbol size depends on the symbol type.
 if (Symbol.Type == IFSSymbolType::NoType)
@@ -140,20 +145,6 @@
   static const bool flow = true;
 };
 
-/// YAML traits for set of IFSSymbols.
-template <> struct CustomMappingTraits> {
-  static void inputOne(IO , StringRef Key, std::set ) {
-std::string Name = Key.str();
-IFSSymbol Sym(Name);
-IO.mapRequired(Name.c_str(), Sym);
-Set.insert(Sym);
-  }
-
-  static void output(IO , std::set ) {
-for (auto  : Set)
-  IO.mapRequired(Sym.Name.c_str(), const_cast(Sym));
-  }
-};
 } // namespace yaml
 } // namespace llvm
 
@@ -167,7 +158,7 @@
   std::string ObjectFileFormat;
   Optional SOName;