[PATCH] D65382: [analyzer][NFC] Refactoring BugReporter.cpp P4.: If it can be const, make it const

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

baloghadamsoftware added a comment.

In D65382#1605572 , @NoQ wrote:

> It's counter-idiomatic to pass manager objects (`ProgramStateManager`, 
> `SValBuilder`, etc.) as const-references because it tells you pretty much 
> nothing about what you can or cannot do with them. You don't really ever 
> semantically mutate them in the first place. There's no need to mention it 
> every time you pass them around.


Most managers are stateful because they also store the elements they manage 
(e.g. `ProgramStateManager` stores states, `SValBuilder` owns other managers 
such as `SymbolManager` that stores symbols etc.) It is also true that 
`BugReporter` should not change the existing set of states, symbols etc., just 
use them in a read-only way. However, I see more confusion than help in making 
them const here and non-const elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65382



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

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

a few more comments from my side.




Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:309
+  // If the New file has fewer lines than the Old file we don't want to send
+  // highlightings beyond the end of the file. That might crash the client.
+  for (int Line = 0; Line != std::numeric_limits::max() && Line < NLines;

nit: I believe theia is crashing because of this? could we file a bug to theia? 
I think it would be nice for client to be robust on this case. 



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:70
+// Return a line-by-line diff between two highlightings.
+//  - if the tokens on a line are the same in both hightlightings this line is
+//  omitted.

nit:  add `, ` before `this`



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:73
+//  - if a line exists in New but not in Old the tokens
+//  on this line is emitted., we emit the tokens on this line
+//  - if a line not exists in New but in Old an empty

could you refine this sentence, I can't parse it.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:74
+//  on this line is emitted., we emit the tokens on this line
+//  - if a line not exists in New but in Old an empty
+//  line is emitted (to tell client to clear the previous highlightings on this

and here as well.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:80
+diffHighlightings(ArrayRef New,
+  ArrayRef Old, int NLines);
 

ilya-biryukov wrote:
> Could you document what `NLines` is? Specifically, whether it's the number of 
> lines for `New` or `Old`.
> 
could we use a more describe name for  `NLines`? maybe `MaxLines`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64475



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

[PATCH] D65404: [AArch64] Disable __ARM_FEATURE_SVE without ACLE.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

Thanks, I see your argument for having the feature enabled even when the ACLE 
is still partially supported. At the moment however, LLVM still only support 
the assembler/disassembler and inline asm.
We'll be working to add support for the ACLE soon, but for now having the flag 
enabled sets the wrong expectations.


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

https://reviews.llvm.org/D65404



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

[PATCH] D64753: [CrossTU][NFCI] Refactor loadExternalAST function

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

gamesh411 updated this revision to Diff 212290.
gamesh411 marked 5 inline comments as done.
gamesh411 added a comment.

- Merge RAII class
- Update comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64753

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/lib/CrossTU/CrossTranslationUnit.cpp

Index: clang/lib/CrossTU/CrossTranslationUnit.cpp
===
--- clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -188,7 +188,7 @@
 }
 
 CrossTranslationUnitContext::CrossTranslationUnitContext(CompilerInstance &CI)
-: CI(CI), Context(CI.getASTContext()),
+: CI(CI), Context(CI.getASTContext()), ASTStorage(CI),
   CTULoadThreshold(CI.getAnalyzerOpts()->CTUImportThreshold) {}
 
 CrossTranslationUnitContext::~CrossTranslationUnitContext() {}
@@ -237,8 +237,8 @@
   if (LookupName.empty())
 return llvm::make_error(
 index_error_code::failed_to_generate_usr);
-  llvm::Expected ASTUnitOrError = loadExternalAST(
-  LookupName, CrossTUDir, IndexName, DisplayCTUProgress);
+  llvm::Expected ASTUnitOrError =
+  loadExternalAST(LookupName, CrossTUDir, IndexName, DisplayCTUProgress);
   if (!ASTUnitOrError)
 return ASTUnitOrError.takeError();
   ASTUnit *Unit = *ASTUnitOrError;
@@ -340,6 +340,118 @@
   }
 }
 
+CrossTranslationUnitContext::ASTFileLoader::ASTFileLoader(
+const CompilerInstance &CI)
+: CI(CI) {}
+
+std::unique_ptr
+CrossTranslationUnitContext::ASTFileLoader::operator()(StringRef ASTFilePath) {
+  // Load AST from ast-dump.
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  TextDiagnosticPrinter *DiagClient =
+  new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
+  IntrusiveRefCntPtr Diags(
+  new DiagnosticsEngine(DiagID, &*DiagOpts, DiagClient));
+
+  return ASTUnit::LoadFromASTFile(
+  ASTFilePath, CI.getPCHContainerOperations()->getRawReader(),
+  ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts());
+}
+
+CrossTranslationUnitContext::ASTUnitStorage::ASTUnitStorage(
+const CompilerInstance &CI)
+: FileAccessor(CI) {}
+
+llvm::Expected
+CrossTranslationUnitContext::ASTUnitStorage::getASTUnitForFile(StringRef FileName) {
+  // Try the cache first.
+  auto ASTCacheEntry = FileASTUnitMap.find(FileName);
+  if (ASTCacheEntry == FileASTUnitMap.end()) {
+// Load the ASTUnit from the pre-dumped AST file specified by ASTFileName.
+std::unique_ptr LoadedUnit = FileAccessor(FileName);
+
+// Need the raw pointer and the unique_ptr as well.
+ASTUnit* Unit = LoadedUnit.get();
+
+// Update the cache.
+FileASTUnitMap[FileName] = std::move(LoadedUnit);
+return Unit;
+
+  } else {
+// Found in the cache.
+return ASTCacheEntry->second.get();
+  }
+}
+
+llvm::Expected
+CrossTranslationUnitContext::ASTUnitStorage::getASTUnitForFunction(
+StringRef FunctionName, StringRef CrossTUDir, StringRef IndexName) {
+  // Try the cache first.
+  auto ASTCacheEntry = NameASTUnitMap.find(FunctionName);
+  if (ASTCacheEntry == NameASTUnitMap.end()) {
+// Load the ASTUnit from the pre-dumped AST file specified by ASTFileName.
+
+// Ensure that the Index is loaded, as we need to search in it.
+if (llvm::Error IndexLoadError =
+ensureCTUIndexLoaded(CrossTUDir, IndexName))
+  return std::move(IndexLoadError);
+
+// Check if there is and entry in the index for the function.
+if (!NameFileMap.count(FunctionName)) {
+  ++NumNotInOtherTU;
+  return llvm::make_error(index_error_code::missing_definition);
+}
+
+// Search in the index for the filename where the definition of FuncitonName
+// resides.
+if (llvm::Expected FoundForFile =
+getASTUnitForFile(NameFileMap[FunctionName])) {
+
+  // Update the cache.
+  NameASTUnitMap[FunctionName] = *FoundForFile;
+  return *FoundForFile;
+
+} else {
+  return FoundForFile.takeError();
+}
+  } else {
+// Found in the cache.
+return ASTCacheEntry->second;
+  }
+}
+
+llvm::Expected
+CrossTranslationUnitContext::ASTUnitStorage::getFileForFunction(
+StringRef FunctionName, StringRef CrossTUDir, StringRef IndexName) {
+  if (llvm::Error IndexLoadError = ensureCTUIndexLoaded(CrossTUDir, IndexName))
+return std::move(IndexLoadError);
+  return NameFileMap[FunctionName];
+}
+
+llvm::Error CrossTranslationUnitContext::ASTUnitStorage::ensureCTUIndexLoaded(
+StringRef CrossTUDir, StringRef IndexName) {
+  // Dont initialize if the map is filled.
+  if (!NameFileMap.empty())
+return llvm::Error::success();
+
+  // Get the absolute path to the index file.
+  SmallString<256> IndexFile = CrossTUDir;
+  if (llvm::sys::path::is_absolute(IndexName))
+IndexFile = IndexName;
+  else
+llvm::sys::path::append(IndexFile, IndexName);
+
+ 

[PATCH] D64753: [CrossTU][NFCI] Refactor loadExternalAST function

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

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

Updated the revision.
I find this solution definitely more compact with responsibilities more 
separated. One more thing that comes to mind is that maybe the whole explicit 
passing of CTUDir and IndexName arguments is a bit verbose. What are your 
thoughts about these being injected earlier than the query operations (eg.: in 
getASTUnitForFunction and getFileForFunction)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64753



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

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 212293.
glider added a comment.

Test for =X somehow sneaked in - drop it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/asm-attrs.c
  clang/test/CodeGen/x86_64-PR42672.c

Index: clang/test/CodeGen/x86_64-PR42672.c
===
--- /dev/null
+++ clang/test/CodeGen/x86_64-PR42672.c
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DSTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-STRUCT
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -USTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-NOSTRUCT
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_ODD -emit-llvm %s -o - 2> %t || true
+// RUN: grep "impossible constraint in asm" %t
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG -emit-llvm %s -o - 2> %t || true
+// RUN: grep "impossible constraint in asm" %t
+
+// Make sure Clang doesn't treat |lockval| as asm input.
+void _raw_spin_lock(void) {
+#if STRUCT
+  struct {
+unsigned short owner, next;
+  } lockval;
+  lockval.owner = 1;
+  lockval.next = 2;
+#else
+  int lockval;
+  lockval = 3;
+#endif
+  asm("nop"
+  : "=r"(lockval));
+}
+// CHECK-LABEL: _raw_spin_lock
+// CHECK-LABEL: entry:
+
+// CHECK-STRUCT:  %lockval = alloca %struct.anon, align 2
+// CHECK-STRUCT:  store i16 1
+// CHECK-STRUCT:  store i16 2
+// CHECK-STRUCT: [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-STRUCT: [[CAST:%[0-9]+]] = bitcast %struct.anon* %lockval to i32*
+// CHECK-STRUCT: store i32 [[RES]], i32* [[CAST]], align 2
+
+// CHECK-NOSTRUCT: %lockval = alloca i32, align 4
+// CHECK-NOSTRUCT: store i32 3
+// CHECK-NOSTRUCT:  [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-NOSTRUCT: store i32 [[RES]], i32* %lockval, align 4
+
+// Check Clang correctly handles a structure with padding.
+void unusual_struct(void) {
+  struct {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+
+// Check Clang reports an error if attempting to return a structure for which
+// no direct conversion to a register is possible.
+#ifdef IMPOSSIBLE_ODD
+void odd_struct(void) {
+  struct __attribute__((__packed__)) {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+#endif
+
+// Check Clang reports an error if attempting to return a big structure via a register.
+#ifdef IMPOSSIBLE_BIG
+void big_struct(void) {
+  struct {
+long long int v1, v2, v3, v4;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+#endif
Index: clang/test/CodeGen/asm-attrs.c
===
--- clang/test/CodeGen/asm-attrs.c
+++ clang/test/CodeGen/asm-attrs.c
@@ -8,7 +8,7 @@
 // CHECK: call i32 asm "foo5", {{.*}} [[READONLY]]
 // CHECK: call i32 asm "foo6", {{.*}} [[NOATTRS]]
 // CHECK: call void asm sideeffect "foo7", {{.*}} [[NOATTRS]]
-// CHECK: call void asm "foo8", {{.*}} [[NOATTRS]]
+// CHECK: call i32 asm "foo8", {{.*}} [[READNONE]]
 
 // CHECK: attributes [[READNONE]] = { nounwind readnone }
 // CHECK: attributes [[NOATTRS]] = { nounwind }
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -1986,6 +1986,7 @@
   std::vector ResultTruncRegTypes;
   std::vector ArgTypes;
   std::vector Args;
+  llvm::BitVector ResultTypeRequiresCast;
 
   // Keep track of inout constraints.
   std::string InOutConstraints;
@@ -2024,13 +2025,23 @@
 
 // If this is a register output, then make the inline asm return it
 // by-value.  If this is a memory result, return the value by-reference.
-if (!Info.allowsMemory() && hasScalarEvaluationKind(OutExpr->getType())) {
+bool isScalarizableAggregate =
+hasAggregateEvaluationKind(OutExpr->getType());
+if (!Info.allowsMemory() && (hasScalarEvaluationKind(OutExpr->getType()) ||
+ isScalarizableAggregate)) {
   Constraints += "=" + OutputConstraint;
   ResultRegQualTys.push_back(OutExpr->getType());
   ResultRegDests.push_back(Dest);
-  ResultRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
-  ResultTruncRegTypes.push_back(ResultRegTypes.back());
-
+  ResultTruncRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
+  if (Info.allowsRegister() && isScalarizableAggregate) {
+ResultTypeRequiresCast.push_back(true);
+unsigned Size = getContext().getTypeSize(OutExpr->getType());
+llvm::Type *ConvTy = llvm::IntegerType::get(getLLVMContext(), Size);
+ResultRegTypes.push_back(ConvTy);
+  } else {
+ResultTypeRequiresCast.push_back(false);
+Res

Re: r367008 - [OpenCL] Rename lang mode flag for C++ mode

[Hans Wennborg via cfe-commits]

Merged in r367300.

Thanks,
Hans

On Mon, Jul 29, 2019 at 3:57 PM Anastasia Stulova
 wrote:
>
> Hi Hans,
>
>
> I would like to ask you to merge this commit into the release 9.0 even though 
> it isn't really a bug fix. However it is a small rename in the command line 
> interface (unfortunately affecting many files). This has been agreed the last 
> minutes and we can only do this change in clang9 because it's entirely a new 
> feature. Sorry we haven't managed to commit before the branch was taken. I 
> hope it is still ok.
>
> Thank you,
> Anastasia
>
>
> 
> From: cfe-commits  on behalf of Anastasia 
> Stulova via cfe-commits 
> Sent: 25 July 2019 12:04
> To: cfe-commits@lists.llvm.org 
> Subject: r367008 - [OpenCL] Rename lang mode flag for C++ mode
>
> Author: stulova
> Date: Thu Jul 25 04:04:29 2019
> New Revision: 367008
>
> URL: http://llvm.org/viewvc/llvm-project?rev=367008&view=rev
> Log:
> [OpenCL] Rename lang mode flag for C++ mode
>
> Rename lang mode flag to -cl-std=clc++/-cl-std=CLC++
> or -std=clc++/-std=CLC++.
>
> This aligns with OpenCL C conversion and removes ambiguity
> with OpenCL C++.
>
> Differential Revision: https://reviews.llvm.org/D65102
>
>
>
> Modified:
> cfe/trunk/docs/LanguageExtensions.rst
> cfe/trunk/docs/UsersManual.rst
> cfe/trunk/include/clang/Driver/Options.td
> cfe/trunk/include/clang/Frontend/LangStandards.def
> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> cfe/trunk/test/CodeGenCXX/mangle-address-space.cpp
> cfe/trunk/test/CodeGenOpenCL/builtins.cl
> cfe/trunk/test/CodeGenOpenCL/images.cl
> cfe/trunk/test/CodeGenOpenCL/logical-ops.cl
> cfe/trunk/test/CodeGenOpenCL/pipe_builtin.cl
> cfe/trunk/test/CodeGenOpenCL/sampler.cl
> cfe/trunk/test/CodeGenOpenCL/spir_version.cl
> cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
> cfe/trunk/test/CodeGenOpenCLCXX/address-space-castoperators.cpp
> cfe/trunk/test/CodeGenOpenCLCXX/address-space-deduction.cl
> cfe/trunk/test/CodeGenOpenCLCXX/address-space-deduction2.cl
> cfe/trunk/test/CodeGenOpenCLCXX/addrspace-conversion.cl
> cfe/trunk/test/CodeGenOpenCLCXX/addrspace-derived-base.cl
> cfe/trunk/test/CodeGenOpenCLCXX/addrspace-of-this.cl
> cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl
> cfe/trunk/test/CodeGenOpenCLCXX/addrspace-references.cl
> cfe/trunk/test/CodeGenOpenCLCXX/addrspace-with-class.cl
> cfe/trunk/test/CodeGenOpenCLCXX/atexit.cl
> cfe/trunk/test/CodeGenOpenCLCXX/global_init.cl
> cfe/trunk/test/CodeGenOpenCLCXX/local_addrspace_init.cl
> cfe/trunk/test/CodeGenOpenCLCXX/method-overload-address-space.cl
> cfe/trunk/test/CodeGenOpenCLCXX/template-address-spaces.cl
> cfe/trunk/test/Driver/autocomplete.c
> cfe/trunk/test/Driver/opencl.cl
> cfe/trunk/test/Frontend/opencl.cl
> cfe/trunk/test/Frontend/stdlang.c
> cfe/trunk/test/Headers/opencl-c-header.cl
> cfe/trunk/test/Parser/opencl-cxx-keywords.cl
> cfe/trunk/test/Parser/opencl-cxx-virtual.cl
> cfe/trunk/test/Preprocessor/predefined-macros.c
> cfe/trunk/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
> cfe/trunk/test/SemaOpenCL/address-spaces.cl
> cfe/trunk/test/SemaOpenCL/builtin.cl
> cfe/trunk/test/SemaOpenCL/clk_event_t.cl
> cfe/trunk/test/SemaOpenCL/extension-version.cl
> cfe/trunk/test/SemaOpenCL/extensions.cl
> cfe/trunk/test/SemaOpenCL/invalid-image.cl
> cfe/trunk/test/SemaOpenCL/invalid-pipes-cl2.0.cl
> cfe/trunk/test/SemaOpenCLCXX/address-space-deduction.cl
> cfe/trunk/test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
> cfe/trunk/test/SemaOpenCLCXX/address-space-of-this.cl
> cfe/trunk/test/SemaOpenCLCXX/address-space-references.cl
> cfe/trunk/test/SemaOpenCLCXX/address-space-templates.cl
> cfe/trunk/test/SemaOpenCLCXX/address_space_overloading.cl
> cfe/trunk/test/SemaOpenCLCXX/kernel_invalid.cl
> cfe/trunk/test/SemaOpenCLCXX/method-overload-address-space.cl
> cfe/trunk/test/SemaOpenCLCXX/newdelete.cl
> cfe/trunk/test/SemaOpenCLCXX/restricted.cl
>
> Modified: cfe/trunk/docs/LanguageExtensions.rst
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=367008&r1=367007&r2=367008&view=diff
> ==
> --- cfe/trunk/docs/LanguageExtensions.rst (original)
> +++ cfe/trunk/docs/LanguageExtensions.rst Thu Jul 25 04:04:29 2019
> @@ -1779,7 +1779,7 @@ invoked.
>
>  .. code-block:: console
>
> - clang -cl-std=c++ test.cl
> + clang -cl-std=clc++ test.cl
>
>  If there are any global objects to be initialized the final binary will
>  contain ``@_GLOBAL__sub_I_test.cl`` kernel to be enqueued.
>
> Modified: cfe/trunk/docs/UsersManual.rst
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=367008&r1=367007&r2=367008&view=diff
> =

[PATCH] D65433: [clangd] DefineInline action availability checks

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Introduces DefineInline action and initial version of availability
checks.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65433

Files:
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -41,26 +41,31 @@
   .str();
 }
 
-void checkAvailable(StringRef ID, llvm::StringRef Input, bool Available) {
+void checkAvailable(StringRef ID, llvm::StringRef Input, bool Available,
+llvm::StringMap AdditionalFiles) {
   Annotations Code(Input);
   ASSERT_TRUE(0 < Code.points().size() || 0 < Code.ranges().size())
   << "no points of interest specified";
   TestTU TU;
   TU.Filename = "foo.cpp";
   TU.Code = Code.code();
+  TU.AdditionalFiles = std::move(AdditionalFiles);
 
   ParsedAST AST = TU.build();
 
   auto CheckOver = [&](Range Selection) {
 unsigned Begin = cantFail(positionToOffset(Code.code(), Selection.start));
 unsigned End = cantFail(positionToOffset(Code.code(), Selection.end));
-auto T = prepareTweak(ID, Tweak::Selection(AST, Begin, End));
+const Tweak::Selection Sel = Tweak::Selection(AST, Begin, End);
+auto T = prepareTweak(ID, Sel);
 if (Available)
   EXPECT_THAT_EXPECTED(T, Succeeded())
-  << "code is " << markRange(Code.code(), Selection);
+  << "code is " << markRange(Code.code(), Selection)
+  << Sel.ASTSelection;
 else
   EXPECT_THAT_EXPECTED(T, Failed())
-  << "code is " << markRange(Code.code(), Selection);
+  << "code is " << markRange(Code.code(), Selection)
+  << Sel.ASTSelection;
   };
   for (auto P : Code.points())
 CheckOver(Range{P, P});
@@ -69,13 +74,17 @@
 }
 
 /// Checks action is available at every point and range marked in \p Input.
-void checkAvailable(StringRef ID, llvm::StringRef Input) {
-  return checkAvailable(ID, Input, /*Available=*/true);
+void checkAvailable(StringRef ID, llvm::StringRef Input,
+llvm::StringMap AdditionalFiles = {}) {
+  return checkAvailable(ID, Input, /*Available=*/true,
+std::move(AdditionalFiles));
 }
 
 /// Same as checkAvailable, but checks the action is not available.
-void checkNotAvailable(StringRef ID, llvm::StringRef Input) {
-  return checkAvailable(ID, Input, /*Available=*/false);
+void checkNotAvailable(StringRef ID, llvm::StringRef Input,
+   llvm::StringMap AdditionalFiles = {}) {
+  return checkAvailable(ID, Input, /*Available=*/false,
+std::move(AdditionalFiles));
 }
 
 llvm::Expected apply(StringRef ID, llvm::StringRef Input) {
@@ -131,10 +140,10 @@
 /// Check if apply returns an error and that the @ErrorMessage is contained
 /// in that error
 void checkApplyContainsError(llvm::StringRef ID, llvm::StringRef Input,
- const std::string& ErrorMessage) {
+ const std::string &ErrorMessage) {
   auto Result = apply(ID, Input);
-  ASSERT_FALSE(Result) << "expected error message:\n   " << ErrorMessage <<
-   "\non input:" << Input;
+  ASSERT_FALSE(Result) << "expected error message:\n   " << ErrorMessage
+   << "\non input:" << Input;
   EXPECT_THAT(llvm::toString(Result.takeError()),
   testing::HasSubstr(ErrorMessage))
   << Input;
@@ -815,6 +824,113 @@
   checkTransform(ID, Input, Output);
 }
 
+TEST(DefineInline, TriggersOnFunctionDecl) {
+  llvm::StringLiteral ID = "DefineInline";
+
+  // Basic check for function body and signature.
+  checkAvailable(ID, R"cpp(
+  class Bar {
+void baz();
+  };
+
+  // Should it also trigger on NestedNameSpecifier? i.e "Bar::"
+  [[void [[Bar::[[b^a^z() [[{
+return;
+  }
+
+  void foo();
+  [[void [[f^o^o]]() [[{
+return;
+  }
+  )cpp");
+
+  checkNotAvailable(ID, R"cpp(
+  // Not a definition
+  vo^i[[d^ ^f]]^oo();
+
+  [[vo^id ]]foo[[()]] {[[
+[[(void)(5+3);
+return;]]
+  }]]
+  )cpp");
+}
+
+TEST(DefineInline, NoDecl) {
+  llvm::StringLiteral ID = "DefineInline";
+
+  checkNotAvailable(ID, R"cpp(
+  #include "a.h"
+  void bar() {
+return;
+  }
+  // FIXME: Generate a decl in the header.
+  void fo^o() {
+return;
+  })cpp",
+{{"a.h", "void bar();"}});
+}
+
+TEST(DefineInline, ReferencedDecls) {
+  llvm::StringLiteral ID = "DefineInline";
+
+  checkAvailable(ID, R"cpp(
+  void bar();
+  void foo(int test);
+
+  void fo^o(int baz) {
+int x = 10;
+bar();
+  })cpp");
+
+  checkAvailable(ID, R"cpp(
+  #include "a.

[PATCH] D65404: [AArch64] Disable __ARM_FEATURE_SVE without ACLE.

[Renato Golin via Phabricator via cfe-commits]

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

I see, makes sense. LGTM, thanks!


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

https://reviews.llvm.org/D65404



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

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

+srhines
Heads up: this patch reduces the size of the following Android kernel functions:

  _raw_spin_lock and _raw_spin_lock_bh: 132 bytes->68 bytes
  _raw_spin_lock_irq: 136 bytes->72 bytes
  _raw_spin_lock_irqsave: 144 bytes->80 bytes
  _raw_spin_trylock: 156 bytes->112 bytes
  _raw_spin_trylock_bh: 136 bytes->92 bytes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234



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

[PATCH] D61879: WIP: Prototype of DSE optimizations for -ftrivial-auto-var-init

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

Vitaly, what's the current status of these patches? What's your plan on 
submitting them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61879



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

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

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

jvikstrom updated this revision to Diff 212301.
jvikstrom marked 14 inline comments as done.
jvikstrom added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64475

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -29,9 +29,7 @@
   return Tokens;
 }
 
-void checkHighlightings(llvm::StringRef Code) {
-  Annotations Test(Code);
-  auto AST = TestTU::withCode(Test.code()).build();
+std::vector getExpectedTokens(Annotations &Test) {
   static const std::map KindToString{
   {HighlightingKind::Variable, "Variable"},
   {HighlightingKind::Function, "Function"},
@@ -48,10 +46,47 @@
 Test.ranges(KindString.second), KindString.first);
 ExpectedTokens.insert(ExpectedTokens.end(), Toks.begin(), Toks.end());
   }
+  llvm::sort(ExpectedTokens);
+  return ExpectedTokens;
+}
+
+void checkHighlightings(llvm::StringRef Code) {
+  Annotations Test(Code);
+  auto AST = TestTU::withCode(Test.code()).build();
+  std::vector ActualTokens = getSemanticHighlightings(AST);
+  EXPECT_THAT(ActualTokens, getExpectedTokens(Test));
+}
+
+// Any annotations in OldCode and NewCode are converted into their corresponding
+// HighlightingToken. The tokens are diffed against each other. Any lines where
+// the tokens should diff must be marked with a ^ somewhere on that line in
+// NewCode. If there are diffs that aren't marked with ^ the test fails. The
+// test also fails if there are lines marked with ^ that don't differ.
+void checkDiffedHighlights(llvm::StringRef OldCode, llvm::StringRef NewCode) {
+  Annotations OldTest(OldCode);
+  Annotations NewTest(NewCode);
+  std::vector OldTokens = getExpectedTokens(OldTest);
+  std::vector NewTokens = getExpectedTokens(NewTest);
+
+  llvm::DenseMap> ExpectedLines;
+  for (const Position &Point : NewTest.points()) {
+ExpectedLines[Point.line]; // Default initialize to an empty line. Tokens
+   // are inserted on these lines later.
+  }
+  std::vector ExpectedLinePairHighlighting;
+  for (const HighlightingToken &Token : NewTokens) {
+auto It = ExpectedLines.find(Token.R.start.line);
+if (It != ExpectedLines.end())
+  It->second.push_back(Token);
+  }
+  for (auto &LineTokens : ExpectedLines)
+ExpectedLinePairHighlighting.push_back(
+{LineTokens.first, LineTokens.second});
 
-  auto ActualTokens = getSemanticHighlightings(AST);
-  EXPECT_THAT(ActualTokens, testing::UnorderedElementsAreArray(ExpectedTokens))
-   << "Inputs is:\n" << Code;
+  std::vector ActualDiffed =
+  diffHighlightings(NewTokens, OldTokens, NewCode.count('\n'));
+  EXPECT_THAT(ActualDiffed,
+  testing::UnorderedElementsAreArray(ExpectedLinePairHighlighting));
 }
 
 TEST(SemanticHighlighting, GetsCorrectTokens) {
@@ -226,8 +261,9 @@
 std::atomic Count = {0};
 
 void onDiagnosticsReady(PathRef, std::vector) override {}
-void onHighlightingsReady(
-PathRef File, std::vector Highlightings) override {
+void onHighlightingsReady(PathRef File,
+  std::vector Highlightings,
+  int NLines) override {
   ++Count;
 }
   };
@@ -252,21 +288,124 @@
 return Pos;
   };
 
-  std::vector Tokens{
-  {HighlightingKind::Variable,
-Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
-  {HighlightingKind::Function,
-Range{CreatePosition(3, 4), CreatePosition(3, 7)}},
-  {HighlightingKind::Variable,
-Range{CreatePosition(1, 1), CreatePosition(1, 5)}}};
+  std::vector Tokens{
+  {3,
+   {{HighlightingKind::Variable,
+ Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
+{HighlightingKind::Function,
+ Range{CreatePosition(3, 4), CreatePosition(3, 7),
+  {1,
+   {{HighlightingKind::Variable,
+ Range{CreatePosition(1, 1), CreatePosition(1, 5)};
   std::vector ActualResults =
   toSemanticHighlightingInformation(Tokens);
   std::vector ExpectedResults = {
-  {1, "AQAEAAA="},
-  {3, "CAAEAAAEAAMAAQ=="}};
+  {3, "CAAEAAAEAAMAAQ=="}, {1, "AQAEAAA="}};
   EXPECT_EQ(ActualResults, ExpectedResults);
 }
 
+TEST(SemanticHighlighting, HighlightingDiffer) {
+  s

[PATCH] D64922: [clangd] Added option to enable semantic highlighting via an experimental capability

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

jvikstrom abandoned this revision.
jvikstrom added a comment.

In D64922#1603759 , @nridge wrote:

> I believe this is not necessary. You can add text document capabilities in 
> the vscode client extension like this:
>
>   class SemanticHighlightingFeature implements vscodelc.StaticFeature {
> fillClientCapabilities(capabilities: vscodelc.ClientCapabilities): void {
>   const textDocumentCapabilities:
>   vscodelc.TextDocumentClientCapabilities & { 
> semanticHighlightingCapabilities?: { semanticHighlighting: boolean } }
>   = capabilities.textDocument;
>   textDocumentCapabilities.semanticHighlightingCapabilities = { 
> semanticHighlighting: true };
> }
>   }
>  
>   ...
>  
>   clangdClient.registerFeature(new SemanticHighlightingFeature());
>
>
> What this is doing is creating a new type on the fly which extends 
> `vscodelc.TextDocumentCapabilities` with a new optional field 
> `semanticHighlightingCapabilities`, and then casts 
> `capabilities.textDocument` to that type.


That definitely works, I had no idea that was a feature you could use in 
typescript. Thanks

Abandoning in favor of using type intersection in typescript instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64922



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

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

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

jvikstrom added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:294
+  // ArrayRefs to the current line in the highlights.
+  ArrayRef NewLine(New.begin(),
+  /*length*/0UL);

ilya-biryukov wrote:
> Could we try to find a better name for this? Having `Line` and `NextLine()` 
> which represent line numbers and `NewLine` which represents highlightings, 
> creates some confusion.
I renamed the `Line` and `NextLine()` instead. Could also rename `NewLine` to 
something like `NewLineHighlightings` but I feel like it almost becomes to 
verbose at that point.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:309
+  // If the New file has fewer lines than the Old file we don't want to send
+  // highlightings beyond the end of the file. That might crash the client.
+  for (int Line = 0; Line != std::numeric_limits::max() && Line < NLines;

hokein wrote:
> nit: I believe theia is crashing because of this? could we file a bug to 
> theia? I think it would be nice for client to be robust on this case. 
I seem to be misremembering, when I first started testing in theia I think I 
was crashing the client (due to a broken base64 encoding function, will take a 
look and see what was actually happening, can't quite remember)
It actually seems like theia just ignores any out of bounds highlightings.

So this could be removed, it will just sometimes return highlightings that are 
outside of the file and hopefully any future clients would handle that as well. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64475



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

[PATCH] D65395: [clangd] Added a skeleton for a semantic highlighting feature to the vscode extension.

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

jvikstrom updated this revision to Diff 212302.
jvikstrom added a comment.

Removed the experimental capability and added it to the normal capabilites 
using type intersection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65395

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/.gitignore
  clang-tools-extra/clangd/clients/clangd-vscode/src/SemanticHighlighting.ts
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts

Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -1,5 +1,6 @@
 import * as vscode from 'vscode';
 import * as vscodelc from 'vscode-languageclient';
+import { SemanticHighlighting } from './SemanticHighlighting';
 
 /**
  * Method to get workspace configuration option
@@ -12,9 +13,9 @@
 }
 
 namespace SwitchSourceHeaderRequest {
-export const type =
-new vscodelc.RequestType('textDocument/switchSourceHeader');
+export const type =
+new vscodelc.RequestType('textDocument/switchSourceHeader');
 }
 
 class FileStatus {
@@ -32,8 +33,8 @@
 const path = vscode.window.activeTextEditor.document.fileName;
 const status = this.statuses.get(path);
 if (!status) {
-  this.statusBarItem.hide();
-  return;
+this.statusBarItem.hide();
+return;
 }
 this.statusBarItem.text = `clangd: ` + status.state;
 this.statusBarItem.show();
@@ -73,25 +74,30 @@
 // However, VSCode does not have CUDA as a supported language yet, so we
 // cannot add a corresponding activationEvent for CUDA files and clangd will
 // *not* load itself automatically on '.cu' files.
-const cudaFilePattern: string = '**/*.{' +['cu'].join()+ '}';
+const cudaFilePattern: string = '**/*.{' + ['cu'].join() + '}';
 const clientOptions: vscodelc.LanguageClientOptions = {
 // Register the server for c-family and cuda files.
 documentSelector: [
 { scheme: 'file', language: 'c' },
 { scheme: 'file', language: 'cpp' },
-{ scheme: 'file', language: 'objective-c'},
-{ scheme: 'file', language: 'objective-cpp'},
+{ scheme: 'file', language: 'objective-c' },
+{ scheme: 'file', language: 'objective-cpp' },
 { scheme: 'file', pattern: cudaFilePattern },
 ],
 synchronize: !syncFileEvents ? undefined : {
-// FIXME: send sync file events when clangd provides implemenatations.
+// FIXME: send sync file events when clangd provides implemenatations.
 },
 initializationOptions: { clangdFileStatus: true },
 // Do not switch to output window when clangd returns output
 revealOutputChannelOn: vscodelc.RevealOutputChannelOn.Never
 };
 
-const clangdClient = new vscodelc.LanguageClient('Clang Language Server',serverOptions, clientOptions);
+const clangdClient = new vscodelc.LanguageClient('Clang Language Server', serverOptions, clientOptions);
+const semanticHighlightingFeature = new SemanticHighlighting.Feature();
+clangdClient.registerFeature(semanticHighlightingFeature);
+// The notification handler must be registered after the client is ready or the client will crash.
+clangdClient.onReady().then(() => clangdClient.onNotification(SemanticHighlighting.NotificationType, semanticHighlightingFeature.handleNotification));
+
 console.log('Clang Language Server is now active!');
 context.subscriptions.push(clangdClient.start());
 context.subscriptions.push(vscode.commands.registerCommand(
@@ -131,5 +137,5 @@
 // An empty place holder for the activate command, otherwise we'll get an
 // "command is not registered" error.
 context.subscriptions.push(vscode.commands.registerCommand(
-'clangd-vscode.activate', async () => {}));
+'clangd-vscode.activate', async () => { }));
 }
Index: clang-tools-extra/clangd/clients/clangd-vscode/src/SemanticHighlighting.ts
===
--- /dev/null
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/SemanticHighlighting.ts
@@ -0,0 +1,34 @@
+import * as vscodelc from 'vscode-languageclient';
+import { DocumentSelector } from 'vscode-languageclient';
+
+export namespace SemanticHighlighting {
+interface HighlightingInformation {
+textDocument: {
+uri: String,
+},
+lines: [{
+line: number,
+tokens: string,
+}],
+}
+
+export const NotificationType = new vscodelc.NotificationType<{}, void>('textDocument/semanticHighlighting');
+
+// The feature that should be registered in the vscode lsp for enabling experimental semant

r367301 - [AArch64] Disable __ARM_FEATURE_SVE without ACLE.

[Sander de Smalen via cfe-commits]

Author: s.desmalen
Date: Tue Jul 30 03:14:39 2019
New Revision: 367301

URL: http://llvm.org/viewvc/llvm-project?rev=367301&view=rev
Log:
[AArch64] Disable __ARM_FEATURE_SVE without ACLE.

The Arm C Language Extensions for SVE document specifies that 
__ARM_FEATURE_SVE should be set when the compiler supports SVE and
implements all the extensions described in the document.

This is currently not yet the case, so the feature should be disabled
until the compiler can provide all the extensions as described.

Reviewers: c-rhodes, rengolin, rovka, ktkachov

Reviewed By: rengolin

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

Modified:
cfe/trunk/lib/Basic/Targets/AArch64.cpp
cfe/trunk/test/Preprocessor/aarch64-target-features.c

Modified: cfe/trunk/lib/Basic/Targets/AArch64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AArch64.cpp?rev=367301&r1=367300&r2=367301&view=diff
==
--- cfe/trunk/lib/Basic/Targets/AArch64.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/AArch64.cpp Tue Jul 30 03:14:39 2019
@@ -196,9 +196,6 @@ void AArch64TargetInfo::getTargetDefines
 Builder.defineMacro("__ARM_NEON_FP", "0xE");
   }
 
-  if (FPU & SveMode)
-Builder.defineMacro("__ARM_FEATURE_SVE", "1");
-
   if (HasCRC)
 Builder.defineMacro("__ARM_FEATURE_CRC32", "1");
 

Modified: cfe/trunk/test/Preprocessor/aarch64-target-features.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/aarch64-target-features.c?rev=367301&r1=367300&r2=367301&view=diff
==
--- cfe/trunk/test/Preprocessor/aarch64-target-features.c (original)
+++ cfe/trunk/test/Preprocessor/aarch64-target-features.c Tue Jul 30 03:14:39 
2019
@@ -88,7 +88,7 @@
 // RUN: %clang -target aarch64 -mtune=cyclone -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-MTUNE-CYCLONE %s
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve -x c -E -dM 
%s -o - | FileCheck --check-prefix=CHECK-SVE %s
-// CHECK-SVE: __ARM_FEATURE_SVE 1
+// CHECK-SVE-NOT: __ARM_FEATURE_SVE 1
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8.2a+dotprod -x c -E 
-dM %s -o - | FileCheck --check-prefix=CHECK-DOTPROD %s
 // CHECK-DOTPROD: __ARM_FEATURE_DOTPROD 1


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

[PATCH] D65404: [AArch64] Disable __ARM_FEATURE_SVE without ACLE.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367301: [AArch64] Disable __ARM_FEATURE_SVE without ACLE. 
(authored by s.desmalen, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65404?vs=212182&id=212305#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65404

Files:
  cfe/trunk/lib/Basic/Targets/AArch64.cpp
  cfe/trunk/test/Preprocessor/aarch64-target-features.c


Index: cfe/trunk/test/Preprocessor/aarch64-target-features.c
===
--- cfe/trunk/test/Preprocessor/aarch64-target-features.c
+++ cfe/trunk/test/Preprocessor/aarch64-target-features.c
@@ -88,7 +88,7 @@
 // RUN: %clang -target aarch64 -mtune=cyclone -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-MTUNE-CYCLONE %s
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve -x c -E -dM 
%s -o - | FileCheck --check-prefix=CHECK-SVE %s
-// CHECK-SVE: __ARM_FEATURE_SVE 1
+// CHECK-SVE-NOT: __ARM_FEATURE_SVE 1
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8.2a+dotprod -x c -E 
-dM %s -o - | FileCheck --check-prefix=CHECK-DOTPROD %s
 // CHECK-DOTPROD: __ARM_FEATURE_DOTPROD 1
Index: cfe/trunk/lib/Basic/Targets/AArch64.cpp
===
--- cfe/trunk/lib/Basic/Targets/AArch64.cpp
+++ cfe/trunk/lib/Basic/Targets/AArch64.cpp
@@ -196,9 +196,6 @@
 Builder.defineMacro("__ARM_NEON_FP", "0xE");
   }
 
-  if (FPU & SveMode)
-Builder.defineMacro("__ARM_FEATURE_SVE", "1");
-
   if (HasCRC)
 Builder.defineMacro("__ARM_FEATURE_CRC32", "1");
 


Index: cfe/trunk/test/Preprocessor/aarch64-target-features.c
===
--- cfe/trunk/test/Preprocessor/aarch64-target-features.c
+++ cfe/trunk/test/Preprocessor/aarch64-target-features.c
@@ -88,7 +88,7 @@
 // RUN: %clang -target aarch64 -mtune=cyclone -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-MTUNE-CYCLONE %s
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-SVE %s
-// CHECK-SVE: __ARM_FEATURE_SVE 1
+// CHECK-SVE-NOT: __ARM_FEATURE_SVE 1
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8.2a+dotprod -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-DOTPROD %s
 // CHECK-DOTPROD: __ARM_FEATURE_DOTPROD 1
Index: cfe/trunk/lib/Basic/Targets/AArch64.cpp
===
--- cfe/trunk/lib/Basic/Targets/AArch64.cpp
+++ cfe/trunk/lib/Basic/Targets/AArch64.cpp
@@ -196,9 +196,6 @@
 Builder.defineMacro("__ARM_NEON_FP", "0xE");
   }
 
-  if (FPU & SveMode)
-Builder.defineMacro("__ARM_FEATURE_SVE", "1");
-
   if (HasCRC)
 Builder.defineMacro("__ARM_FEATURE_CRC32", "1");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64863: [clangd] Ignore diags from builtin files

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 212307.
kadircet marked 3 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64863

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -907,7 +907,6 @@
 int x = 5/0;)cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", Header.code()}};
-  auto diags = TU.build().getDiagnostics();
   EXPECT_THAT(TU.build().getDiagnostics(),
   UnorderedElementsAre(AllOf(
   Diag(Main.range(), "in included file: C++ requires "
@@ -915,6 +914,19 @@
   WithNote(Diag(Header.range(), "error occurred here");
 }
 
+TEST(IgnoreDiags, FromNonWrittenSources) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  Annotations Header(R"cpp(
+int x = 5/0;
+int b = [[FOO]];)cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{"a.h", Header.code()}};
+  TU.ExtraArgs = {"-DFOO=NOOO"};
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+}
+
 } // namespace
 
 } // namespace clangd
Index: clang-tools-extra/clangd/Diagnostics.h
===
--- clang-tools-extra/clangd/Diagnostics.h
+++ clang-tools-extra/clangd/Diagnostics.h
@@ -145,6 +145,8 @@
   std::vector Output;
   llvm::Optional LangOpts;
   llvm::Optional LastDiag;
+  /// Set iff adjustDiagFromHeader resulted in changes to LastDiag.
+  bool LastDiagWasAdjusted = false;
   llvm::DenseSet IncludeLinesWithErrors;
   bool LastPrimaryDiagnosticWasSuppressed = false;
 };
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -20,6 +20,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Capacity.h"
@@ -107,8 +108,13 @@
   return halfOpenToRange(M, R);
 }
 
-void adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
+// Returns whether the \p D is modified.
+bool adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
   const LangOptions &LangOpts) {
+  // We only report diagnostics with at least error severity from headers.
+  if (D.Severity < DiagnosticsEngine::Level::Error)
+return false;
+
   const SourceLocation &DiagLoc = Info.getLocation();
   const SourceManager &SM = Info.getSourceManager();
   SourceLocation IncludeInMainFile;
@@ -119,13 +125,14 @@
IncludeLocation = GetIncludeLoc(IncludeLocation))
 IncludeInMainFile = IncludeLocation;
   if (IncludeInMainFile.isInvalid())
-return;
+return false;
 
   // Update diag to point at include inside main file.
   D.File = SM.getFileEntryForID(SM.getMainFileID())->getName().str();
   D.Range.start = sourceLocToPosition(SM, IncludeInMainFile);
   D.Range.end = sourceLocToPosition(
   SM, Lexer::getLocForEndOfToken(IncludeInMainFile, 0, SM, LangOpts));
+  D.InsideMainFile = true;
 
   // Add a note that will point to real diagnostic.
   const auto *FE = SM.getFileEntryForID(SM.getFileID(DiagLoc));
@@ -138,6 +145,7 @@
 
   // Update message to mention original file.
   D.Message = llvm::Twine("in included file: ", D.Message).str();
+  return true;
 }
 
 bool isInsideMainFile(const clang::Diagnostic &D) {
@@ -465,6 +473,7 @@
   }
 
   bool InsideMainFile = isInsideMainFile(Info);
+  SourceManager &SM = Info.getSourceManager();
 
   auto FillDiagBase = [&](DiagBase &D) {
 D.Range = diagnosticRange(Info, *LangOpts);
@@ -472,8 +481,7 @@
 Info.FormatDiagnostic(Message);
 D.Message = Message.str();
 D.InsideMainFile = InsideMainFile;
-D.File = Info.getSourceManager().getFilename(Info.getLocation());
-auto &SM = Info.getSourceManager();
+D.File = SM.getFilename(Info.getLocation());
 D.AbsFile = getCanonicalPath(
 SM.getFileEntryForID(SM.getFileID(Info.getLocation())), SM);
 D.Severity = DiagLevel;
@@ -496,10 +504,9 @@
   if (FixIt.RemoveRange.getBegin().isMacroID() ||
   FixIt.RemoveRange.getEnd().isMacroID())
 return false;
-  if (!isInsideMainFile(FixIt.RemoveRange.getBegin(),
-Info.getSourceManager()))
+  if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM))
 return false;
-  Edits.push_back(toTextEdit(FixIt, Info

[clang-tools-extra] r367303 - [clangd] Ignore diags from builtin files

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Tue Jul 30 03:26:51 2019
New Revision: 367303

URL: http://llvm.org/viewvc/llvm-project?rev=367303&view=rev
Log:
[clangd] Ignore diags from builtin files

Summary:
This fixes a case where we show diagnostics on arbitrary lines, in an
internal codebase.

Open for ideas on unittesting this.

Reviewers: ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/Diagnostics.cpp
clang-tools-extra/trunk/clangd/Diagnostics.h
clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp

Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=367303&r1=367302&r2=367303&view=diff
==
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Tue Jul 30 03:26:51 2019
@@ -20,6 +20,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Capacity.h"
@@ -107,8 +108,13 @@ Range diagnosticRange(const clang::Diagn
   return halfOpenToRange(M, R);
 }
 
-void adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
+// Returns whether the \p D is modified.
+bool adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
   const LangOptions &LangOpts) {
+  // We only report diagnostics with at least error severity from headers.
+  if (D.Severity < DiagnosticsEngine::Level::Error)
+return false;
+
   const SourceLocation &DiagLoc = Info.getLocation();
   const SourceManager &SM = Info.getSourceManager();
   SourceLocation IncludeInMainFile;
@@ -119,13 +125,14 @@ void adjustDiagFromHeader(Diag &D, const
IncludeLocation = GetIncludeLoc(IncludeLocation))
 IncludeInMainFile = IncludeLocation;
   if (IncludeInMainFile.isInvalid())
-return;
+return false;
 
   // Update diag to point at include inside main file.
   D.File = SM.getFileEntryForID(SM.getMainFileID())->getName().str();
   D.Range.start = sourceLocToPosition(SM, IncludeInMainFile);
   D.Range.end = sourceLocToPosition(
   SM, Lexer::getLocForEndOfToken(IncludeInMainFile, 0, SM, LangOpts));
+  D.InsideMainFile = true;
 
   // Add a note that will point to real diagnostic.
   const auto *FE = SM.getFileEntryForID(SM.getFileID(DiagLoc));
@@ -138,6 +145,7 @@ void adjustDiagFromHeader(Diag &D, const
 
   // Update message to mention original file.
   D.Message = llvm::Twine("in included file: ", D.Message).str();
+  return true;
 }
 
 bool isInsideMainFile(const clang::Diagnostic &D) {
@@ -465,6 +473,7 @@ void StoreDiags::HandleDiagnostic(Diagno
   }
 
   bool InsideMainFile = isInsideMainFile(Info);
+  SourceManager &SM = Info.getSourceManager();
 
   auto FillDiagBase = [&](DiagBase &D) {
 D.Range = diagnosticRange(Info, *LangOpts);
@@ -472,8 +481,7 @@ void StoreDiags::HandleDiagnostic(Diagno
 Info.FormatDiagnostic(Message);
 D.Message = Message.str();
 D.InsideMainFile = InsideMainFile;
-D.File = Info.getSourceManager().getFilename(Info.getLocation());
-auto &SM = Info.getSourceManager();
+D.File = SM.getFilename(Info.getLocation());
 D.AbsFile = getCanonicalPath(
 SM.getFileEntryForID(SM.getFileID(Info.getLocation())), SM);
 D.Severity = DiagLevel;
@@ -496,10 +504,9 @@ void StoreDiags::HandleDiagnostic(Diagno
   if (FixIt.RemoveRange.getBegin().isMacroID() ||
   FixIt.RemoveRange.getEnd().isMacroID())
 return false;
-  if (!isInsideMainFile(FixIt.RemoveRange.getBegin(),
-Info.getSourceManager()))
+  if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM))
 return false;
-  Edits.push_back(toTextEdit(FixIt, Info.getSourceManager(), *LangOpts));
+  Edits.push_back(toTextEdit(FixIt, SM, *LangOpts));
 }
 
 llvm::SmallString<64> Message;
@@ -507,8 +514,8 @@ void StoreDiags::HandleDiagnostic(Diagno
 if (SyntheticMessage && Info.getNumFixItHints() == 1) {
   const auto &FixIt = Info.getFixItHint(0);
   bool Invalid = false;
-  llvm::StringRef Remove = Lexer::getSourceText(
-  FixIt.RemoveRange, Info.getSourceManager(), *LangOpts, &Invalid);
+  llvm::StringRef Remove =
+  Lexer::getSourceText(FixIt.RemoveRange, SM, *LangOpts, &Invalid);
   llvm::StringRef Insert = FixIt.CodeToInsert;
   if (!Invalid) {
 llvm::raw_svector_ostream M(Message);
@@ -553,7 +560,9 @@ void StoreDiags::HandleDiagnostic(Diagno
 LastDiag = Diag();
 LastDiag->ID = Info.getID();
 FillDiagBase(*LastDiag);
-adjustDiagFromHeader(*LastDiag, Info, *LangOpts);
+LastDiagWasAdjusted = false;
+if (!InsideMainFile)
+  LastDiagWasAdju

[PATCH] D64863: [clangd] Ignore diags from builtin files

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367303: [clangd] Ignore diags from builtin files (authored 
by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64863?vs=212307&id=212308#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64863

Files:
  clang-tools-extra/trunk/clangd/Diagnostics.cpp
  clang-tools-extra/trunk/clangd/Diagnostics.h
  clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
@@ -907,7 +907,6 @@
 int x = 5/0;)cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", Header.code()}};
-  auto diags = TU.build().getDiagnostics();
   EXPECT_THAT(TU.build().getDiagnostics(),
   UnorderedElementsAre(AllOf(
   Diag(Main.range(), "in included file: C++ requires "
@@ -915,6 +914,19 @@
   WithNote(Diag(Header.range(), "error occurred here");
 }
 
+TEST(IgnoreDiags, FromNonWrittenSources) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  Annotations Header(R"cpp(
+int x = 5/0;
+int b = [[FOO]];)cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{"a.h", Header.code()}};
+  TU.ExtraArgs = {"-DFOO=NOOO"};
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+}
+
 } // namespace
 
 } // namespace clangd
Index: clang-tools-extra/trunk/clangd/Diagnostics.h
===
--- clang-tools-extra/trunk/clangd/Diagnostics.h
+++ clang-tools-extra/trunk/clangd/Diagnostics.h
@@ -145,6 +145,8 @@
   std::vector Output;
   llvm::Optional LangOpts;
   llvm::Optional LastDiag;
+  /// Set iff adjustDiagFromHeader resulted in changes to LastDiag.
+  bool LastDiagWasAdjusted = false;
   llvm::DenseSet IncludeLinesWithErrors;
   bool LastPrimaryDiagnosticWasSuppressed = false;
 };
Index: clang-tools-extra/trunk/clangd/Diagnostics.cpp
===
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp
@@ -20,6 +20,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Capacity.h"
@@ -107,8 +108,13 @@
   return halfOpenToRange(M, R);
 }
 
-void adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
+// Returns whether the \p D is modified.
+bool adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
   const LangOptions &LangOpts) {
+  // We only report diagnostics with at least error severity from headers.
+  if (D.Severity < DiagnosticsEngine::Level::Error)
+return false;
+
   const SourceLocation &DiagLoc = Info.getLocation();
   const SourceManager &SM = Info.getSourceManager();
   SourceLocation IncludeInMainFile;
@@ -119,13 +125,14 @@
IncludeLocation = GetIncludeLoc(IncludeLocation))
 IncludeInMainFile = IncludeLocation;
   if (IncludeInMainFile.isInvalid())
-return;
+return false;
 
   // Update diag to point at include inside main file.
   D.File = SM.getFileEntryForID(SM.getMainFileID())->getName().str();
   D.Range.start = sourceLocToPosition(SM, IncludeInMainFile);
   D.Range.end = sourceLocToPosition(
   SM, Lexer::getLocForEndOfToken(IncludeInMainFile, 0, SM, LangOpts));
+  D.InsideMainFile = true;
 
   // Add a note that will point to real diagnostic.
   const auto *FE = SM.getFileEntryForID(SM.getFileID(DiagLoc));
@@ -138,6 +145,7 @@
 
   // Update message to mention original file.
   D.Message = llvm::Twine("in included file: ", D.Message).str();
+  return true;
 }
 
 bool isInsideMainFile(const clang::Diagnostic &D) {
@@ -465,6 +473,7 @@
   }
 
   bool InsideMainFile = isInsideMainFile(Info);
+  SourceManager &SM = Info.getSourceManager();
 
   auto FillDiagBase = [&](DiagBase &D) {
 D.Range = diagnosticRange(Info, *LangOpts);
@@ -472,8 +481,7 @@
 Info.FormatDiagnostic(Message);
 D.Message = Message.str();
 D.InsideMainFile = InsideMainFile;
-D.File = Info.getSourceManager().getFilename(Info.getLocation());
-auto &SM = Info.getSourceManager();
+D.File = SM.getFilename(Info.getLocation());
 D.AbsFile = getCanonicalPath(
 SM.getFileEntryForID(SM.getFileID(Info.getLocation())), SM);
 D.Severity = DiagLevel;
@@ -496,10 +504,9 @@
   if (FixIt.RemoveRange.getBegin().isMacroID() ||
   FixIt.RemoveRange.getEnd().isMacroID())
 retu

r367305 - [Driver] Define _FILE_OFFSET_BITS=64 on Solaris

[Rainer Orth via cfe-commits]

Author: ro
Date: Tue Jul 30 03:38:41 2019
New Revision: 367305

URL: http://llvm.org/viewvc/llvm-project?rev=367305&view=rev
Log:
[Driver] Define _FILE_OFFSET_BITS=64 on Solaris

make check-all currently fails on x86_64-pc-solaris2.11 when building with GCC 
9:

  Undefined   first referenced
   symbol in file
  _ZN11__sanitizer14internal_lseekEimi 
SANITIZER_TEST_OBJECTS.sanitizer_libc_test.cc.i386.o
  _ZN11__sanitizer23MapWritableFileToMemoryEPvmim 
SANITIZER_TEST_OBJECTS.sanitizer_libc_test.cc.i386.o
  ld: fatal: symbol referencing errors
  clang-9: error: linker command failed with exit code 1 (use -v to see 
invocation)
  make[3]: *** 
[projects/compiler-rt/lib/sanitizer_common/tests/CMakeFiles/TSanitizer-i386-Test.dir/build.make:92:
 projects/compiler-rt/lib/sanitizer_common/tests/Sanitizer-i386-Test] Error 1

While e.g. __sanitizer::internal_lseek is defined in sanitizer_solaris.cc, g++ 9
predefines _FILE_OFFSET_BITS=64 while clang++ currently does not.

This patch resolves this inconsistency by following the gcc lead, which allows
make check-all to finish successfully.

There's one caveat: gcc defines _LARGEFILE_SOURCE and _LARGEFILE64_SOURCE for 
C++ only, while clang has long been doing it for
all languages.  I'd like to keep it this way because those macros do is to make
declarations of fseek/ftello (_LARGEFILE_SOURCE) resp. the 64-bit versions
of largefile functions (*64 with _LARGEFILE64_SOURCE) visible additionally.
However, _FILE_OFFSET_BITS=64 changes all affected functions to be 
largefile-aware.
I'd like to restrict this to C++, just like gcc does.

To avoid a similar inconsistence with host compilers that don't predefine 
_FILE_OFFSET_BITS=64
(e.g. clang < 9, gcc < 9), this needs a compantion patch 
https://reviews.llvm.org/D64483.

Tested on x86_64-pc-solaris2.11.

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

Modified:
cfe/trunk/lib/Basic/Targets/OSTargets.h

Modified: cfe/trunk/lib/Basic/Targets/OSTargets.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/OSTargets.h?rev=367305&r1=367304&r2=367305&view=diff
==
--- cfe/trunk/lib/Basic/Targets/OSTargets.h (original)
+++ cfe/trunk/lib/Basic/Targets/OSTargets.h Tue Jul 30 03:38:41 2019
@@ -622,8 +622,11 @@ protected:
   Builder.defineMacro("_XOPEN_SOURCE", "600");
 else
   Builder.defineMacro("_XOPEN_SOURCE", "500");
-if (Opts.CPlusPlus)
+if (Opts.CPlusPlus) {
   Builder.defineMacro("__C99FEATURES__");
+  Builder.defineMacro("_FILE_OFFSET_BITS", "64");
+}
+// GCC restricts the next two to C++.
 Builder.defineMacro("_LARGEFILE_SOURCE");
 Builder.defineMacro("_LARGEFILE64_SOURCE");
 Builder.defineMacro("__EXTENSIONS__");


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

[PATCH] D64482: [Driver] Define _FILE_OFFSET_BITS=64 on Solaris

[Rainer Orth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367305: [Driver] Define _FILE_OFFSET_BITS=64 on Solaris 
(authored by ro, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64482?vs=208935&id=212310#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64482

Files:
  cfe/trunk/lib/Basic/Targets/OSTargets.h


Index: cfe/trunk/lib/Basic/Targets/OSTargets.h
===
--- cfe/trunk/lib/Basic/Targets/OSTargets.h
+++ cfe/trunk/lib/Basic/Targets/OSTargets.h
@@ -622,8 +622,11 @@
   Builder.defineMacro("_XOPEN_SOURCE", "600");
 else
   Builder.defineMacro("_XOPEN_SOURCE", "500");
-if (Opts.CPlusPlus)
+if (Opts.CPlusPlus) {
   Builder.defineMacro("__C99FEATURES__");
+  Builder.defineMacro("_FILE_OFFSET_BITS", "64");
+}
+// GCC restricts the next two to C++.
 Builder.defineMacro("_LARGEFILE_SOURCE");
 Builder.defineMacro("_LARGEFILE64_SOURCE");
 Builder.defineMacro("__EXTENSIONS__");


Index: cfe/trunk/lib/Basic/Targets/OSTargets.h
===
--- cfe/trunk/lib/Basic/Targets/OSTargets.h
+++ cfe/trunk/lib/Basic/Targets/OSTargets.h
@@ -622,8 +622,11 @@
   Builder.defineMacro("_XOPEN_SOURCE", "600");
 else
   Builder.defineMacro("_XOPEN_SOURCE", "500");
-if (Opts.CPlusPlus)
+if (Opts.CPlusPlus) {
   Builder.defineMacro("__C99FEATURES__");
+  Builder.defineMacro("_FILE_OFFSET_BITS", "64");
+}
+// GCC restricts the next two to C++.
 Builder.defineMacro("_LARGEFILE_SOURCE");
 Builder.defineMacro("_LARGEFILE64_SOURCE");
 Builder.defineMacro("__EXTENSIONS__");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:81
+unsigned FileSize = SM.getFileEntryForID(MainFileID)->getSize();
+int NLines = AST.getSourceManager().getLineNumber(MainFileID, FileSize);
+

from the comment of the getLineNumber, this is not cheap to call this method. 
We may use `SM.getBufferData(MainFileID).count('\n')` to count the line numbers.

```
// This requires building and caching a table of line offsets for the
/// MemoryBuffer, so this is not cheap: use only when about to emit a
/// diagnostic.
```



Comment at: clang-tools-extra/clangd/ClangdServer.h:55
   /// Called by ClangdServer when some \p Highlightings for \p File are ready.
-  virtual void
-  onHighlightingsReady(PathRef File,
-   std::vector Highlightings) {}
+  /// \p NLines are the number of lines in the file, needed to make sure that
+  /// any diffing does not add lines beyond EOF.

nit: drop the `needed ...`, this is an interface, don't mention the details of 
subclass (diffs are subclass implementation details).

Maybe name it "NumLines"? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64475



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

[PATCH] D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D65050#1598057 , @cpplearner wrote:

> In D65050#1596514 , @efriedma wrote:
>
> > Is this the only place where a compound type can contain a 
> > PackExpansionType?
>
>
> Yes AFAIK. No other place can contain a list or a pack expansion. The closest 
> thing is dynamic exception specification (which isn't part of the type), and 
> it seems to be handled properly.


The noexcept specifier is part of the type these days, is that also handled 
properly?


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

https://reviews.llvm.org/D65050



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

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

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

baloghadamsoftware added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1743
+
+  if (Then->isInevitablySinking() || Else->isInevitablySinking())
+return true;

xazax.hun wrote:
> Do we consider a block assert like if both branches are inevitable sinking?
No, I think `(Then->isInevitablySinking() != Else->isInevitablySinking())` 
would serve better here.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1750
+  //   [B1] -> [B2] -> [B3] -> [sink]
+  // assert(A && B || C);\   \   \
+  //  ---> [go on with the execution]

xazax.hun wrote:
> I wonder if the CFG is correct for your example. If A evaluates to false, we 
> still check C before the sink. If A evaluates to true we still check B before 
> going on. But maybe it is just late for me :)
I think an arrow from `[B1]` to `[B3]` is missing for the `A == false` case.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1760
+
+using namespace ast_matchers;
+auto IsSubStmtOfCondition =

I would avoid using matchers in the core infrastructure. My experience says 
they are quite expensive to create and destroy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65287



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

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

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

baloghadamsoftware marked an inline comment as not done.
baloghadamsoftware added a comment.

For me it is all the same whether to continue this patch or taking over Péter's.


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

https://reviews.llvm.org/D63279



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

[PATCH] D62895: [Analyzer] Iterator Checkers - Check and simulate `std::advance`, `std::prev` and `std::next`

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

baloghadamsoftware added a comment.

Gentle ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62895



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

[PATCH] D62893: [Analyzer] Iterator Checkers - Make range errors and invalidated access fatal

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

baloghadamsoftware added a comment.

Gentle ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62893



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

[PATCH] D62724: [Analyzer] Iterator Checkers - Model `size()` method of containers

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

baloghadamsoftware added a comment.

Gentle ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62724



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

[PATCH] D65286: [OpenCL] Allow OpenCL C style vector initialization in C++

[Marco Antognini via Phabricator via cfe-commits]

mantognini added a comment.

In `vector_literals_nested.cl`, we have tests for (global) constants. Do you 
think it would be worth testing those as well in C++ mode? Maybe the two files 
(`vector_literals_nested.cl` and `vector_literals_valid.cl`) should also be 
merged as most of their content seems duplicated.

In C++, we have the comma operator and therefore the AST is significantly 
different. Running the content of your test file with `-x c++` shows that it is 
rejected as desired. It could be worth having a negative test to ensure we 
always reject this in vanilla C++.

Regarding the code itself, I'm not familiar with 
`InitializationSequence`/`InitListChecker` that much, but the patch makes sense 
I would say.


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

https://reviews.llvm.org/D65286



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

[PATCH] D65206: [analyzer] Fix text range end columns in SARIF to be exclusive

[Aaron Ballman via Phabricator via cfe-commits]

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

This LGTM, but I'd appreciate a second reviewer chiming in only because Joe is 
a coworker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65206



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

[PATCH] D65209: [analyzer] Fix a SARIF exporter crash with macro expansions

[Aaron Ballman via Phabricator via cfe-commits]

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

This LGTM, but I'd appreciate a second reviewer chiming in only because Joe is 
a coworker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65209



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

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

[Jussi Pakkanen via Phabricator via cfe-commits]

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

In D64671#1603427 , @alexfh wrote:

> A general comment: "misc" is a sort of a heap of checks that otherwise don't 
> have a good home. This one would probably better go to bugprone (or maybe 
> there's a relevant CERT or C++ Core Guidelines rule?).


The closest I could find is ES.20 "Always initialize an object": 
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es20-always-initialize-an-object

Most other guidelines seem to be formulated as "don't read uninitialized 
memory" such as: https://cwe.mitre.org/data/definitions/457.html

So a potential name for this could be "cppcoreguidelines-init-variables". 
However that rule is about initializing all variables, even those in structs 
and classes. This is about local variables.

Related to this, there seems to be a talk at CppCon about doing something 
similar to this MR: 
https://cppcon2019.sched.com/event/SfYc/killing-uninitialized-memory-prot


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

https://reviews.llvm.org/D64671



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

[PATCH] D65211: [analyzer] Update the SARIF exporter to SARIF 2.1

[Aaron Ballman via Phabricator via cfe-commits]

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

This LGTM, but I'd appreciate a second reviewer chiming in only because Joe is 
a coworker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65211



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

[PATCH] D62688: [Analyzer] Iterator Checkers - Model `empty()` method of containers

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

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

In D62688#1549574 , @Szelethus wrote:

> Hmm, an idea just popped into my head. I'm not sure whether we have a single 
> checker that does so much complicated (and totally awesome) modeling as 
> `IteratorChecker`.  What do you think about a debug checker similar to 
> `debug.ExprInspection`, like `debug.IteratorInspection`?


Good idea! However, I would do it in a way that we can reuse our existing debug 
functions in the tests:

  template  long clang_iterator_position(const Iter&);
  template  const Cont& 
clang_iterator_container(const Iter&);
  template  long clang_container_begin(const Iter&);
  template  long clang_container_end(const Iter&);

Then we can nest calls for these functions into call of 
`clang_analyzer_dump()`, `clang_analyzer_eval()`, `clang_analyzer_denote()` etc.




Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:714
 
 if (const auto *InstCall = dyn_cast(&Call)) {
   if (isBeginCall(Func)) {

Szelethus wrote:
> We seem to retrieve the `CXXInstanceCall` here too?
Eventually this function needs refactoring.



Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1741-1763
+bool isContainer(const CXXRecordDecl *CRD) {
+  if (!CRD)
+return false;
+
+  for (const auto *Decl : CRD->decls()) {
+const auto *TD = dyn_cast(Decl);
+if (!TD)

Szelethus wrote:
> But what if we have a container for which the iterator isn't an inline class, 
> or we don't even have an in-class alias for it? I think whether the record 
> has a `begin`/`end` method would be a better heuristic.
Or maybe both, or maybe either. I am not sure here.



Comment at: test/Analysis/iterator-range.cpp:247
+void empty(const std::vector &V) {
+  for (auto n: V) {}
+  *V.begin(); // expected-warning{{Past-the-end iterator dereferenced}}

Szelethus wrote:
> Aha, the for loop is here to force a state split on whether `V` is empty or 
> not, correct?
Yes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62688



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

[PATCH] D62525: [Analyzer] Add new visitor to the iterator checkers

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

baloghadamsoftware added a comment.

Gentle ping.


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

https://reviews.llvm.org/D62525



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

[PATCH] D65443: [clangd] Fix a regression in rL366996.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: jvikstrom.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.

That patch made the tweak always annotate the whole file by accident.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65443

Files:
  clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -571,6 +571,17 @@
   R"cpp(
 void /* entity.name.function.cpp */f1();
 void /* entity.name.function.cpp */f2();
+)cpp");
+
+   checkTransform(ID,
+  R"cpp(
+void f1();
+void f2() {^};
+)cpp",
+
+  R"cpp(
+void f1();
+void /* entity.name.function.cpp */f2() {};
 )cpp");
 }
 
Index: clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
@@ -33,18 +33,16 @@
 REGISTER_TWEAK(AnnotateHighlightings)
 
 Expected AnnotateHighlightings::apply(const Selection &Inputs) {
-  // TUDecl is always the root ancestor.
-  const Decl *CommonDecl =
-  Inputs.ASTSelection.root().ASTNode.get();
+  const Decl *CommonDecl = nullptr;
   for (auto N = Inputs.ASTSelection.commonAncestor(); N && !CommonDecl;
N = N->Parent)
 CommonDecl = N->ASTNode.get();
 
   std::vector HighlightingTokens;
   if (llvm::isa(CommonDecl)) {
-// We only annotate tokens in the main file, if CommonDecl is a TUDecl,
-// we use the default traversal scope (which is the top level decls of the
-// main file).
+// Now we hit the TUDecl case where commonAncestor() returns null intently.
+// We only annotate tokens in the main file, so use the default traversal
+// scope (which is the top level decls of the main file).
 HighlightingTokens = getSemanticHighlightings(Inputs.AST);
   } else {
 // Store the existing scopes.


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -571,6 +571,17 @@
   R"cpp(
 void /* entity.name.function.cpp */f1();
 void /* entity.name.function.cpp */f2();
+)cpp");
+
+   checkTransform(ID,
+  R"cpp(
+void f1();
+void f2() {^};
+)cpp",
+
+  R"cpp(
+void f1();
+void /* entity.name.function.cpp */f2() {};
 )cpp");
 }
 
Index: clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
@@ -33,18 +33,16 @@
 REGISTER_TWEAK(AnnotateHighlightings)
 
 Expected AnnotateHighlightings::apply(const Selection &Inputs) {
-  // TUDecl is always the root ancestor.
-  const Decl *CommonDecl =
-  Inputs.ASTSelection.root().ASTNode.get();
+  const Decl *CommonDecl = nullptr;
   for (auto N = Inputs.ASTSelection.commonAncestor(); N && !CommonDecl;
N = N->Parent)
 CommonDecl = N->ASTNode.get();
 
   std::vector HighlightingTokens;
   if (llvm::isa(CommonDecl)) {
-// We only annotate tokens in the main file, if CommonDecl is a TUDecl,
-// we use the default traversal scope (which is the top level decls of the
-// main file).
+// Now we hit the TUDecl case where commonAncestor() returns null intently.
+// We only annotate tokens in the main file, so use the default traversal
+// scope (which is the top level decls of the main file).
 HighlightingTokens = getSemanticHighlightings(Inputs.AST);
   } else {
 // Store the existing scopes.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65445: [CrossTU] Handle case when no USR could be generated during Decl search.

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

balazske created this revision.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: martong.
Herald added a project: clang.

When searching for a declaration to be loaded the "lookup name" for every
other Decl is computed. If the USR can not be determined here should be
not an assert, instead skip this Decl.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65445

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/test/Analysis/Inputs/ctu-other.cpp
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/ctu-main.cpp
  clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Index: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -76,7 +76,12 @@
 
 void MapExtDefNamesConsumer::addIfInMain(const DeclaratorDecl *DD,
  SourceLocation defStart) {
-  std::string LookupName = CrossTranslationUnitContext::getLookupName(DD);
+  llvm::Optional LookupName =
+  CrossTranslationUnitContext::getLookupName(DD);
+  if (!LookupName)
+return;
+  assert(!LookupName->empty() && "Lookup name should be non-empty.");
+
   if (CurrentFileName.empty()) {
 CurrentFileName =
 SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
@@ -89,7 +94,7 @@
   case VisibleNoLinkage:
   case UniqueExternalLinkage:
 if (SM.isInMainFile(defStart))
-  Index[LookupName] = CurrentFileName;
+  Index[*LookupName] = CurrentFileName;
 break;
   default:
 break;
Index: clang/test/Analysis/ctu-main.cpp
===
--- clang/test/Analysis/ctu-main.cpp
+++ clang/test/Analysis/ctu-main.cpp
@@ -112,6 +112,19 @@
   clang_analyzer_eval(obj->fvcl(1) == 8);  // expected-warning{{FALSE}} expected-warning{{TRUE}}
 }
 
+class TestAnonUnionUSR {
+public:
+  inline float f(int value) {
+union {
+  float f;
+  int i;
+};
+i = value;
+return f;
+  }
+  static const int Test;
+};
+
 int main() {
   clang_analyzer_eval(f(3) == 2); // expected-warning{{TRUE}}
   clang_analyzer_eval(f(4) == 3); // expected-warning{{TRUE}}
@@ -144,4 +157,5 @@
   clang_analyzer_eval(extSubSCN.a == 1); // expected-warning{{TRUE}}
   // clang_analyzer_eval(extSCC.a == 7); // TODO
   clang_analyzer_eval(extU.a == 4); // expected-warning{{TRUE}}
+  clang_analyzer_eval(TestAnonUnionUSR::Test == 5); // expected-warning{{TRUE}}
 }
Index: clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
===
--- clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
+++ clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
@@ -25,3 +25,4 @@
 c:@extSubSCN ctu-other.cpp.ast
 c:@extSCC ctu-other.cpp.ast
 c:@extU ctu-other.cpp.ast
+c:@S@TestAnonUnionUSR@Test ctu-other.cpp.ast
Index: clang/test/Analysis/Inputs/ctu-other.cpp
===
--- clang/test/Analysis/Inputs/ctu-other.cpp
+++ clang/test/Analysis/Inputs/ctu-other.cpp
@@ -131,3 +131,17 @@
   const unsigned int b;
 };
 U extU = {.a = 4};
+
+class TestAnonUnionUSR {
+public:
+  inline float f(int value) {
+union {
+  float f;
+  int i;
+};
+i = value;
+return f;
+  }
+  static const int Test;
+};
+const int TestAnonUnionUSR::Test = 5;
Index: clang/lib/CrossTU/CrossTranslationUnit.cpp
===
--- clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -193,12 +193,13 @@
 
 CrossTranslationUnitContext::~CrossTranslationUnitContext() {}
 
-std::string CrossTranslationUnitContext::getLookupName(const NamedDecl *ND) {
+llvm::Optional
+CrossTranslationUnitContext::getLookupName(const NamedDecl *ND) {
   SmallString<128> DeclUSR;
   bool Ret = index::generateUSRForDecl(ND, DeclUSR);
-  (void)Ret;
-  assert(!Ret && "Unable to generate USR");
-  return DeclUSR.str();
+  if (Ret)
+return {};
+  return std::string(DeclUSR.str());
 }
 
 /// Recursively visits the decls of a DeclContext, and returns one with the
@@ -218,7 +219,8 @@
 const T *ResultDecl;
 if (!ND || !hasBodyOrInit(ND, ResultDecl))
   continue;
-if (getLookupName(ResultDecl) != LookupName)
+llvm::Optional ResultLookupName = getLookupName(ResultDecl);
+if (!ResultLookupName || *ResultLookupName != LookupName)
   continue;
 return ResultDecl;
   }
@@ -233,12 +235,12 @@
   assert(!hasBodyOrInit(D) &&
  "D has a body or init in current translation unit!");
   ++NumGetCTUCalled;
-  const std::string LookupName = getLookupName(D);
-  if (LookupName.empty())
+  const llvm::Optional LookupName = getLookupName(D);
+  

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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D64671#1606084 , @jpakkane wrote:

> In D64671#1603427 , @alexfh wrote:
>
> > A general comment: "misc" is a sort of a heap of checks that otherwise 
> > don't have a good home. This one would probably better go to bugprone (or 
> > maybe there's a relevant CERT or C++ Core Guidelines rule?).
>
>
> The closest I could find is ES.20 "Always initialize an object": 
> https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es20-always-initialize-an-object
>
> So a potential name for this could be "cppcoreguidelines-init-variables". 
> However that rule is about initializing all variables, even those in structs 
> and classes. This is about local variables.


I am in favor of something like `cppcoreguidelines-init-variables` that might 
catch other cases in the future as well. IMHO its fine to start with something 
partial, especially "just" local variables.


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

https://reviews.llvm.org/D64671



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

[PATCH] D65443: [clangd] Fix a regression in rL366996.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 212329.
hokein added a comment.

oops, forgot a change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65443

Files:
  clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -571,6 +571,17 @@
   R"cpp(
 void /* entity.name.function.cpp */f1();
 void /* entity.name.function.cpp */f2();
+)cpp");
+
+   checkTransform(ID,
+  R"cpp(
+void f1();
+void f2() {^};
+)cpp",
+
+  R"cpp(
+void f1();
+void /* entity.name.function.cpp */f2() {};
 )cpp");
 }
 
Index: clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
@@ -33,18 +33,16 @@
 REGISTER_TWEAK(AnnotateHighlightings)
 
 Expected AnnotateHighlightings::apply(const Selection &Inputs) {
-  // TUDecl is always the root ancestor.
-  const Decl *CommonDecl =
-  Inputs.ASTSelection.root().ASTNode.get();
+  const Decl *CommonDecl = nullptr;
   for (auto N = Inputs.ASTSelection.commonAncestor(); N && !CommonDecl;
N = N->Parent)
 CommonDecl = N->ASTNode.get();
 
   std::vector HighlightingTokens;
-  if (llvm::isa(CommonDecl)) {
-// We only annotate tokens in the main file, if CommonDecl is a TUDecl,
-// we use the default traversal scope (which is the top level decls of the
-// main file).
+  if (!CommonDecl) {
+// Now we hit the TUDecl case where commonAncestor() returns null intently.
+// We only annotate tokens in the main file, so use the default traversal
+// scope (which is the top level decls of the main file).
 HighlightingTokens = getSemanticHighlightings(Inputs.AST);
   } else {
 // Store the existing scopes.


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -571,6 +571,17 @@
   R"cpp(
 void /* entity.name.function.cpp */f1();
 void /* entity.name.function.cpp */f2();
+)cpp");
+
+   checkTransform(ID,
+  R"cpp(
+void f1();
+void f2() {^};
+)cpp",
+
+  R"cpp(
+void f1();
+void /* entity.name.function.cpp */f2() {};
 )cpp");
 }
 
Index: clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
@@ -33,18 +33,16 @@
 REGISTER_TWEAK(AnnotateHighlightings)
 
 Expected AnnotateHighlightings::apply(const Selection &Inputs) {
-  // TUDecl is always the root ancestor.
-  const Decl *CommonDecl =
-  Inputs.ASTSelection.root().ASTNode.get();
+  const Decl *CommonDecl = nullptr;
   for (auto N = Inputs.ASTSelection.commonAncestor(); N && !CommonDecl;
N = N->Parent)
 CommonDecl = N->ASTNode.get();
 
   std::vector HighlightingTokens;
-  if (llvm::isa(CommonDecl)) {
-// We only annotate tokens in the main file, if CommonDecl is a TUDecl,
-// we use the default traversal scope (which is the top level decls of the
-// main file).
+  if (!CommonDecl) {
+// Now we hit the TUDecl case where commonAncestor() returns null intently.
+// We only annotate tokens in the main file, so use the default traversal
+// scope (which is the top level decls of the main file).
 HighlightingTokens = getSemanticHighlightings(Inputs.AST);
   } else {
 // Store the existing scopes.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r367312 - Add typedef declaration information to the JSON AST dump.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Jul 30 06:42:19 2019
New Revision: 367312

URL: http://llvm.org/viewvc/llvm-project?rev=367312&view=rev
Log:
Add typedef declaration information to the JSON AST dump.

When dumping a desugared QualType and the type is a type alias, also print out 
the id for the type alias declaration.

Modified:
cfe/trunk/lib/AST/JSONNodeDumper.cpp
cfe/trunk/test/AST/ast-dump-decl-json.m
cfe/trunk/test/AST/ast-dump-expr-json.m
cfe/trunk/test/AST/multistep-explicit-cast-json.cpp

Modified: cfe/trunk/lib/AST/JSONNodeDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/JSONNodeDumper.cpp?rev=367312&r1=367311&r2=367312&view=diff
==
--- cfe/trunk/lib/AST/JSONNodeDumper.cpp (original)
+++ cfe/trunk/lib/AST/JSONNodeDumper.cpp Tue Jul 30 06:42:19 2019
@@ -238,6 +238,8 @@ llvm::json::Object JSONNodeDumper::creat
 SplitQualType DSQT = QT.getSplitDesugaredType();
 if (DSQT != SQT)
   Ret["desugaredQualType"] = QualType::getAsString(DSQT, PrintPolicy);
+if (const auto *TT = QT->getAs())
+  Ret["typeAliasDeclId"] = createPointerRepresentation(TT->getDecl());
   }
   return Ret;
 }

Modified: cfe/trunk/test/AST/ast-dump-decl-json.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-decl-json.m?rev=367312&r1=367311&r2=367312&view=diff
==
--- cfe/trunk/test/AST/ast-dump-decl-json.m (original)
+++ cfe/trunk/test/AST/ast-dump-decl-json.m Tue Jul 30 06:42:19 2019
@@ -371,7 +371,8 @@ void f() {
 // CHECK-NEXT:"name": "_cmd", 
 // CHECK-NEXT:"type": {
 // CHECK-NEXT: "desugaredQualType": "SEL *", 
-// CHECK-NEXT: "qualType": "SEL"
+// CHECK-NEXT: "qualType": "SEL", 
+// CHECK-NEXT: "typeAliasDeclId": "0x{{.*}}"
 // CHECK-NEXT:}
 // CHECK-NEXT:   }, 
 // CHECK-NEXT:   {
@@ -672,7 +673,8 @@ void f() {
 // CHECK-NEXT:  "name": "_cmd", 
 // CHECK-NEXT:  "type": {
 // CHECK-NEXT:   "desugaredQualType": "SEL *", 
-// CHECK-NEXT:   "qualType": "SEL"
+// CHECK-NEXT:   "qualType": "SEL", 
+// CHECK-NEXT:   "typeAliasDeclId": "0x{{.*}}"
 // CHECK-NEXT:  }
 // CHECK-NEXT: }, 
 // CHECK-NEXT: {
@@ -818,7 +820,8 @@ void f() {
 // CHECK-NEXT:"name": "T", 
 // CHECK-NEXT:"type": {
 // CHECK-NEXT: "desugaredQualType": "id", 
-// CHECK-NEXT: "qualType": "id"
+// CHECK-NEXT: "qualType": "id", 
+// CHECK-NEXT: "typeAliasDeclId": "0x{{.*}}"
 // CHECK-NEXT:}
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
@@ -906,7 +909,8 @@ void f() {
 // CHECK-NEXT:  "name": "_cmd", 
 // CHECK-NEXT:  "type": {
 // CHECK-NEXT:   "desugaredQualType": "SEL *", 
-// CHECK-NEXT:   "qualType": "SEL"
+// CHECK-NEXT:   "qualType": "SEL", 
+// CHECK-NEXT:   "typeAliasDeclId": "0x{{.*}}"
 // CHECK-NEXT:  }
 // CHECK-NEXT: }, 
 // CHECK-NEXT: {

Modified: cfe/trunk/test/AST/ast-dump-expr-json.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-expr-json.m?rev=367312&r1=367311&r2=367312&view=diff
==
--- cfe/trunk/test/AST/ast-dump-expr-json.m (original)
+++ cfe/trunk/test/AST/ast-dump-expr-json.m Tue Jul 30 06:42:19 2019
@@ -612,7 +612,8 @@ void TestObjCBoolLiteral() {
 // CHECK-NEXT:"name": "s", 
 // CHECK-NEXT:"type": {
 // CHECK-NEXT: "desugaredQualType": "SEL *", 
-// CHECK-NEXT: "qualType": "SEL"
+// CHECK-NEXT: "qualType": "SEL", 
+// CHECK-NEXT: "typeAliasDeclId": "0x{{.*}}"
 // CHECK-NEXT:}, 
 // CHECK-NEXT:"init": "c", 
 // CHECK-NEXT:"inner": [
@@ -631,7 +632,8 @@ void TestObjCBoolLiteral() {
 // CHECK-NEXT:  }, 
 // CHECK-NEXT:  "type": {
 // CHECK-NEXT:   "desugaredQualType": "SEL *", 
-// CHECK-NEXT:   "qualType": "SEL"
+// CHECK-NEXT:   "qualType": "SEL", 
+// CHECK-NEXT:   "typeAliasDeclId": "0x{{.*}}"
 // CHECK-NEXT:  }, 
 // CHECK-NEXT:  "valueCategory": "rvalue", 
 // CHECK-NEXT:  "selector": "dealloc"
@@ -691,7 +693,8 @@ void TestObjCBoolLiteral() {
 // CHECK-NEXT:"name": "Obj", 
 // CHECK-NEXT:"type": {
 // CHECK-NEXT: "desugaredQualType": "id", 
-// CHECK-NEXT: "qualType": "id"
+// CHECK-NEXT: "qualType": "id", 
+// CHECK-NEXT: "typeAliasDeclId": "0x{{.*}}"
 // CHECK-NEXT:}
 // CHECK-NEXT:   }, 
 // CHECK-NEXT:   {
@@ -745,7 +748,8 @@ void TestObjCBoolLiteral() {
 // CHECK-NEXT:}, 
 // CHECK-NEXT:"type": {
 // CHECK-NEXT: "desugaredQualType": "id", 
-// CHECK-NEXT: "qualType": "id"
+// CHECK-NEXT: "qualType": "id", 
+// CHECK-NEXT: "typeAliasDeclId": "0x{{.*}}"
 // CHECK-NEXT:}, 
 // CHECK-NEXT:"valueCategory": "rvalue", 
 // CHECK-NEXT:"castKind": "LValueToRValue", 

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

[Momchil Velikov via Phabricator via cfe-commits]

chill updated this revision to Diff 212332.
chill added a comment.

Rebase on top of master.


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

https://reviews.llvm.org/D64416

Files:
  clang/include/clang/Basic/BuiltinsAArch64.def
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Headers/arm_acle.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/aarch64-tme.cpp
  clang/test/Sema/aarch64-tme-errors.c
  clang/test/Sema/aarch64-tme-tcancel-errors.c
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64InstrFormats.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/test/CodeGen/AArch64/tme.ll
  llvm/test/MC/AArch64/tme-error.s
  llvm/test/MC/AArch64/tme.s
  llvm/test/MC/Disassembler/AArch64/tme.txt
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -1153,6 +1153,7 @@
   {"rcpc", "norcpc", "+rcpc", "-rcpc" },
   {"rng", "norng", "+rand", "-rand"},
   {"memtag", "nomemtag", "+mte", "-mte"},
+  {"tme", "notme", "+tme", "-tme"},
   {"ssbs", "nossbs", "+ssbs", "-ssbs"},
   {"sb", "nosb", "+sb", "-sb"},
   {"predres", "nopredres", "+predres", "-predres"}
Index: llvm/test/MC/Disassembler/AArch64/tme.txt
===
--- /dev/null
+++ llvm/test/MC/Disassembler/AArch64/tme.txt
@@ -0,0 +1,19 @@
+# Tests for transaction memory extension instructions
+# RUN: llvm-mc -triple=aarch64 -mattr=+tme   -disassemble < %s  | FileCheck %s
+# RUN: not llvm-mc -triple=aarch64 -mattr=-tme   -disassemble < %s 2>&1 | FileCheck %s --check-prefix=NOTME
+
+[0x63,0x30,0x23,0xd5]
+[0x64,0x31,0x23,0xd5]
+[0x7f,0x30,0x03,0xd5]
+[0x80,0x46,0x62,0xd4]
+
+# CHECK: tstart x3
+# CHECK: ttest  x4
+# CHECK: tcommit
+# CHECK: tcancel #0x1234
+
+# NOTEME: mrs
+# NOTEME-NEXT: mrs
+# NOTEME-NEXT: msr
+# NOTME:  warning: invalid instruction encoding
+# NOTME-NEXT: [0x80,0x46,0x62,0xd4]
Index: llvm/test/MC/AArch64/tme.s
===
--- /dev/null
+++ llvm/test/MC/AArch64/tme.s
@@ -0,0 +1,24 @@
+// Tests for transaction memory extension instructions
+//
+// RUN: llvm-mc -triple aarch64 -show-encoding -mattr=+tme   < %s  | FileCheck %s
+// RUN: not llvm-mc -triple aarch64 -show-encoding -mattr=-tme   < %s 2>&1 | FileCheck %s --check-prefix=NOTME
+
+tstart x3
+ttest  x4
+tcommit
+tcancel #0x1234
+
+// CHECK: tstart x3 // encoding: [0x63,0x30,0x23,0xd5]
+// CHECK: ttest x4  // encoding: [0x64,0x31,0x23,0xd5]
+// CHECK: tcommit   // encoding: [0x7f,0x30,0x03,0xd5]
+// CHECK: tcancel #0x1234   // encoding: [0x80,0x46,0x62,0xd4]
+
+
+// NOTME: instruction requires: tme
+// NOTME-NEXT: tstart x3
+// NOTME: instruction requires: tme
+// NOTME-NEXT: ttest  x4
+// NOTME: instruction requires: tme
+// NOTME-NEXT: tcommit
+// NOTME: instruction requires: tme
+// NOTME-NEXT: tcancel #0x1234
Index: llvm/test/MC/AArch64/tme-error.s
===
--- /dev/null
+++ llvm/test/MC/AArch64/tme-error.s
@@ -0,0 +1,47 @@
+// Tests for transactional memory extension instructions
+// RUN: not llvm-mc -triple aarch64 -show-encoding -mattr=+tme < %s 2>&1   | FileCheck %s
+
+tstart
+// CHECK: error: too few operands for instruction
+// CHECK-NEXT: tstart
+tstart  x4, x5
+// CHECK: error: invalid operand for instruction
+// CHECK-NEXT: tstart x4, x5
+tstart  x4, #1
+// CHECK: error: invalid operand for instruction
+// CHECK-NEXT: tstart x4, #1
+tstart  sp
+// CHECK: error: invalid operand for instruction
+// CHECK-NEXT: tstart sp
+
+ttest
+// CHECK: error: too few operands for instruction
+// CHECK-NEXT: ttest
+ttest  x4, x5
+// CHECK: error: invalid operand for instruction
+// CHECK-NEXT: ttest x4, x5
+ttest  x4, #1
+// CHECK: error: invalid operand for instruction
+// CHECK-NEXT: ttest x4, #1
+ttest  sp
+// CHECK: error: invalid operand for instruction
+// CHECK-NEXT: ttest sp
+
+tcommit  x4
+// CHECK: error: invalid operand for instruction
+// CHECK-NEXT: tcommit x4
+tcommit  sp
+// CHECK: error: invalid operand for instruction
+// CHECK-NEXT: tcommit sp
+
+
+tcancel
+// CHECK: error: too few operands for instruction
+// CHECK-NEXT tcancel
+tcancel x0
+// CHECK: error: immediate must be an integer in range [0, 65535]
+// CHECK-NEXT tcancel
+tcancel #65536
+// CHECK: error: immediate must be an integer in range

[PATCH] D65443: [clangd] Fix a regression in rL366996.

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

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



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp:43
+  if (!CommonDecl) {
+// Now we hit the TUDecl case where commonAncestor() returns null intently.
+// We only annotate tokens in the main file, so use the default traversal

Shouldn't `intently` -> `instantly`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65443



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

[PATCH] D64742: Allow using -ftrivial-auto-var-init=zero in C mode without extra flags

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

As a data point, Linus Torvalds suggested that we need a similar feature for 
GCC so that the "kernel C standard" mandates zero-initialization for locals: 
https://lkml.org/lkml/2019/7/28/206


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64742



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

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

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

ostannard added a comment.

In that example, with everything having default ELF visibility, all of the 
vtables will get vcall_visibility public, which can't be optimised by VFE, and 
won't ever be relaxed to one of the tighter visibilities.

The cases which can be optimised are (assuming "visibility" means visibility of 
the most-visible dynamic base class):

- Classes in an anonymous namespace, which aren't visible outside their 
translation unit. Probably doesn't happen very often, but can be optimised 
without LTO.
- Classes visible outside the translation unit, but not outside the LTO unit. 
This generally means hidden ELF visibility, unless the lto_visibility_public 
attribute is used. These can't be optimised without LTO, but can be with it.

I implemented the second case by adding the LinkageUnit visibility, which can't 
be optimised by VFE, but can be reduced to TranslationUnit when LTO 
internalisation happens. This could also have been done by not changing the 
visibility at LTO time, and instead leting GlobalDCE know if it is running 
post-LTO. Both of these should give the same results, but the way I used 
represents the visibility fully in the IR, without having the extra state of 
"are we doing LTO?".

I also don't think it matters here whether the DSO is loaded by dlopen or by 
being linked against it. Is there something I've missed here, or was dlopen not 
relevant to your example.

> Have you checked the assembly to see whether an unused CFI check is being 
> emitted?

Not yet, I wanted to make sure that this optimisation was valid and correctly 
implemented before tracking down the performance regressions.




Comment at: llvm/lib/Transforms/IPO/GlobalDCE.cpp:183
+// unit, we know that we can see all virtual functions which might use it,
+// so VFE is safe.
+if (auto GO = dyn_cast(&GV)) {

pcc wrote:
> Not necessarily, at least in this implementation, because "vtable symbol can 
> be internalized" doesn't imply "all vcalls are visible". See main response.
This is checking the vcall_visibility, which will only be 
VCallVisibilityTranslationUnit if all base classes which contain virtual 
functions are private to this translation unit, which does imply "all vcalls 
are visible".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63932



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

[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D65387#1604737 , @ilya-biryukov 
wrote:

> Could you add a bit more context to the description of the change: why do we 
> need the callback and what is the problem with what we have now?
>  I'm sure this was discussed offline and the change is justified, just wanted 
> to make sure we write it down in the change history.


sorry for missing the context here (updated the description). Yeah, this was a 
prerequisite of implementing a generic mechanism for chainable refactorings 
based on the offline discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65387



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

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

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

In D64695#1605676 , @Manikishan wrote:

> In D64695#1590948 , @Manikishan 
> wrote:
>
> > In D64695#1589818 , @lebedev.ri 
> > wrote:
> >
> > > In D64695#1589740 , @Manikishan 
> > > wrote:
> > >
> > > > In D64695#1589508 , 
> > > > @lebedev.ri wrote:
> > > >
> > > > > Is there sufficient test coverage as to what happens if 
> > > > > `SortPriority` is not set?
> > > >
> > > >
> > > > If SortPriority is not set, the Includes will be grouped without 
> > > > sorting,
> > >
> > >
> > > Let me rephrase - for the exiting `.clang-format`s, that don't currently 
> > > specify `SortPriority`,
> > >  this introduction of `SortPriority` should not change the header 
> > > handling.
> > >  Is that the case, and if so is there sufficient test coverage for that?
> >
> >
> > I got your idea now.
> >  No, there is no test coverage for that case, and with the current patch 
> > they have to add SortPriority.
> >  To avoid this shall I set SortPriority as Priority as default if it is not 
> > defined? I think that will fix the issue.
>
>
> any reviews on it ?


That's sounds like it will work. Can you add some additional test cases around 
this in `SortIncludesTest.cpp`. Also, adding a test case specifically for 
sorting the NetBSD headers would be good.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64695



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

[clang-tools-extra] r367313 - [clangd] Fix a regression in rL366996.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Tue Jul 30 07:17:45 2019
New Revision: 367313

URL: http://llvm.org/viewvc/llvm-project?rev=367313&view=rev
Log:
[clangd] Fix a regression in rL366996.

Summary: That patch made the tweak always annotate the whole file by accident.

Reviewers: jvikstrom

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/refactor/tweaks/AnnotateHighlightings.cpp
clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp

Modified: 
clang-tools-extra/trunk/clangd/refactor/tweaks/AnnotateHighlightings.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/AnnotateHighlightings.cpp?rev=367313&r1=367312&r2=367313&view=diff
==
--- clang-tools-extra/trunk/clangd/refactor/tweaks/AnnotateHighlightings.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/AnnotateHighlightings.cpp 
Tue Jul 30 07:17:45 2019
@@ -33,18 +33,16 @@ public:
 REGISTER_TWEAK(AnnotateHighlightings)
 
 Expected AnnotateHighlightings::apply(const Selection &Inputs) {
-  // TUDecl is always the root ancestor.
-  const Decl *CommonDecl =
-  Inputs.ASTSelection.root().ASTNode.get();
+  const Decl *CommonDecl = nullptr;
   for (auto N = Inputs.ASTSelection.commonAncestor(); N && !CommonDecl;
N = N->Parent)
 CommonDecl = N->ASTNode.get();
 
   std::vector HighlightingTokens;
-  if (llvm::isa(CommonDecl)) {
-// We only annotate tokens in the main file, if CommonDecl is a TUDecl,
-// we use the default traversal scope (which is the top level decls of the
-// main file).
+  if (!CommonDecl) {
+// Now we hit the TUDecl case where commonAncestor() returns null
+// intendedly. We only annotate tokens in the main file, so use the default
+// traversal scope (which is the top level decls of the main file).
 HighlightingTokens = getSemanticHighlightings(Inputs.AST);
   } else {
 // Store the existing scopes.

Modified: clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp?rev=367313&r1=367312&r2=367313&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp Tue Jul 30 07:17:45 
2019
@@ -572,6 +572,17 @@ void f2();]]
 void /* entity.name.function.cpp */f1();
 void /* entity.name.function.cpp */f2();
 )cpp");
+
+   checkTransform(ID,
+  R"cpp(
+void f1();
+void f2() {^};
+)cpp",
+
+  R"cpp(
+void f1();
+void /* entity.name.function.cpp */f2() {};
+)cpp");
 }
 
 TEST(TweakTest, ExpandMacro) {


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

[PATCH] D65443: [clangd] Fix a regression in rL366996.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp:43
+  if (!CommonDecl) {
+// Now we hit the TUDecl case where commonAncestor() returns null intently.
+// We only annotate tokens in the main file, so use the default traversal

jvikstrom wrote:
> Shouldn't `intently` -> `instantly`?
ah, I mean `intendedly`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65443



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

[PATCH] D65443: [clangd] Fix a regression in rL366996.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.
Closed by commit rL367313: [clangd] Fix a regression in rL366996. (authored by 
hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65443?vs=212329&id=212334#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65443

Files:
  clang-tools-extra/trunk/clangd/refactor/tweaks/AnnotateHighlightings.cpp
  clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/trunk/clangd/refactor/tweaks/AnnotateHighlightings.cpp
===
--- clang-tools-extra/trunk/clangd/refactor/tweaks/AnnotateHighlightings.cpp
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/AnnotateHighlightings.cpp
@@ -33,18 +33,16 @@
 REGISTER_TWEAK(AnnotateHighlightings)
 
 Expected AnnotateHighlightings::apply(const Selection &Inputs) {
-  // TUDecl is always the root ancestor.
-  const Decl *CommonDecl =
-  Inputs.ASTSelection.root().ASTNode.get();
+  const Decl *CommonDecl = nullptr;
   for (auto N = Inputs.ASTSelection.commonAncestor(); N && !CommonDecl;
N = N->Parent)
 CommonDecl = N->ASTNode.get();
 
   std::vector HighlightingTokens;
-  if (llvm::isa(CommonDecl)) {
-// We only annotate tokens in the main file, if CommonDecl is a TUDecl,
-// we use the default traversal scope (which is the top level decls of the
-// main file).
+  if (!CommonDecl) {
+// Now we hit the TUDecl case where commonAncestor() returns null
+// intendedly. We only annotate tokens in the main file, so use the default
+// traversal scope (which is the top level decls of the main file).
 HighlightingTokens = getSemanticHighlightings(Inputs.AST);
   } else {
 // Store the existing scopes.
Index: clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
@@ -572,6 +572,17 @@
 void /* entity.name.function.cpp */f1();
 void /* entity.name.function.cpp */f2();
 )cpp");
+
+   checkTransform(ID,
+  R"cpp(
+void f1();
+void f2() {^};
+)cpp",
+
+  R"cpp(
+void f1();
+void /* entity.name.function.cpp */f2() {};
+)cpp");
 }
 
 TEST(TweakTest, ExpandMacro) {


Index: clang-tools-extra/trunk/clangd/refactor/tweaks/AnnotateHighlightings.cpp
===
--- clang-tools-extra/trunk/clangd/refactor/tweaks/AnnotateHighlightings.cpp
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/AnnotateHighlightings.cpp
@@ -33,18 +33,16 @@
 REGISTER_TWEAK(AnnotateHighlightings)
 
 Expected AnnotateHighlightings::apply(const Selection &Inputs) {
-  // TUDecl is always the root ancestor.
-  const Decl *CommonDecl =
-  Inputs.ASTSelection.root().ASTNode.get();
+  const Decl *CommonDecl = nullptr;
   for (auto N = Inputs.ASTSelection.commonAncestor(); N && !CommonDecl;
N = N->Parent)
 CommonDecl = N->ASTNode.get();
 
   std::vector HighlightingTokens;
-  if (llvm::isa(CommonDecl)) {
-// We only annotate tokens in the main file, if CommonDecl is a TUDecl,
-// we use the default traversal scope (which is the top level decls of the
-// main file).
+  if (!CommonDecl) {
+// Now we hit the TUDecl case where commonAncestor() returns null
+// intendedly. We only annotate tokens in the main file, so use the default
+// traversal scope (which is the top level decls of the main file).
 HighlightingTokens = getSemanticHighlightings(Inputs.AST);
   } else {
 // Store the existing scopes.
Index: clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp
@@ -572,6 +572,17 @@
 void /* entity.name.function.cpp */f1();
 void /* entity.name.function.cpp */f2();
 )cpp");
+
+   checkTransform(ID,
+  R"cpp(
+void f1();
+void f2() {^};
+)cpp",
+
+  R"cpp(
+void f1();
+void /* entity.name.function.cpp */f2() {};
+)cpp");
 }
 
 TEST(TweakTest, ExpandMacro) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65263: [clangd] a prototype for triggering rename after extracting.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 212345.
hokein edited the summary of this revision.
hokein added a comment.

reimplement the mechanism based on the offline discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65263

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp

Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -365,7 +365,43 @@
   // replace expression with variable name
   if (auto Err = Result.add(Target->replaceWithVar(Range, VarName)))
 return std::move(Err);
-  return Effect::applyEdit(Result);
+  // TODO: refine this.
+  // We do the format internally in order to keep the position of the extracted
+  // variable not being changed from the caller.
+  auto &SM = Inputs.AST.getASTContext().getSourceManager();
+  auto *FS = &SM.getFileManager().getVirtualFileSystem();
+  auto Style = getFormatStyleForFile(
+  SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName(),
+  Inputs.Code, FS);
+  auto Formatted = cleanupAndFormat(Inputs.Code, Result, Style);
+  if (!Formatted)
+return Formatted.takeError();
+  Result = *Formatted;
+  auto NewCode = tooling::applyAllReplacements(Inputs.Code, Result);
+  if (!NewCode)
+return NewCode.takeError();
+
+  assert(!Result.empty());
+  // Calculate the offset of the dummy variable after applying replacements.
+  size_t ExtractVarOffset = Result.begin()->getOffset();
+  for (auto &R : Result) {
+auto OffsetInReplacement = R.getReplacementText().find(VarName);
+if (OffsetInReplacement != llvm::StringRef::npos) {
+  ExtractVarOffset += OffsetInReplacement;
+  break;
+}
+ExtractVarOffset += R.getReplacementText().size() - R.getLength();
+  }
+
+  assert(ExtractVarOffset != llvm::StringRef::npos);
+  Step Apply;
+  Apply.ApplyKind = Step::ApplyEdit;
+  Apply.ApplyEditParams = std::move(Result);
+  Step Rename;
+  Rename.ApplyKind = Step::Rename;
+  Rename.RenameParams = offsetToPosition(*NewCode, ExtractVarOffset);
+  // TODO: We should not emit Rename if the client doesn't support it.
+  return Effect::applySteps({std::move(Apply), std::move(Rename)});
 }
 
 // Find the CallExpr whose callee is an ancestor of the DeclRef
Index: clang-tools-extra/clangd/refactor/Tweak.h
===
--- clang-tools-extra/clangd/refactor/Tweak.h
+++ clang-tools-extra/clangd/refactor/Tweak.h
@@ -64,12 +64,44 @@
 /// Provide information to the user.
 Info,
   };
+
+  // The class represents a small and focus refactoring. The purpose is to form
+  // a chain of different refactorings, so that we could perform a large and
+  // complicate refactoring.
+  //
+  // Steps are applied in sequence. In LSP layer, each step is transformed to a
+  // call request to the LSP client, LSPServer waits for the client response and
+  // continues to apply next step when the previous step is finished
+  // successfully.
+  //
+  // clangd doesn't track changes of the source files when apply a step, so
+  // the following steps are not aware of the changes, Tweak is corresponding to
+  // pre-calculate the changes for all steps.
+  // FIXME: we should have a mechanism to track the changes for each step.
+  struct Step {
+enum Kind {
+  ApplyEdit,
+  Rename,
+};
+Kind ApplyKind;
+
+llvm::Optional ApplyEditParams; // For ApplyEdit
+llvm::Optional RenameParams; // For Rename
+  };
+
   struct Effect {
 /// A message to be displayed to the user.
 llvm::Optional ShowMessage;
 /// An edit to apply to the input file.
 llvm::Optional ApplyEdit;
+/// A chain of steps being executed in order when applying the tweak.
+llvm::Optional> ApplySteps;
 
+static Effect applySteps(std::vector Steps) {
+  Effect E;
+  E.ApplySteps = std::move(Steps);
+  return E;
+}
 static Effect applyEdit(tooling::Replacements R) {
   Effect E;
   E.ApplyEdit = std::move(R);
Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -128,6 +128,36 @@
 status.clear();
 }
 })
+
+clangdClient.onReady().then(
+() => {clangdClient.onRequest(
+"clangd.triggerRename", async (result: any) => {
+  if (result.arguments) {
+  

[PATCH] D65263: [clangd] a prototype for triggering rename after extracting.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 212347.
hokein added a comment.

update the diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65263

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp

Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -365,7 +365,43 @@
   // replace expression with variable name
   if (auto Err = Result.add(Target->replaceWithVar(Range, VarName)))
 return std::move(Err);
-  return Effect::applyEdit(Result);
+  // TODO: refine this.
+  // We do the format internally in order to keep the position of the extracted
+  // variable not being changed from the caller.
+  auto &SM = Inputs.AST.getASTContext().getSourceManager();
+  auto *FS = &SM.getFileManager().getVirtualFileSystem();
+  auto Style = getFormatStyleForFile(
+  SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName(),
+  Inputs.Code, FS);
+  auto Formatted = cleanupAndFormat(Inputs.Code, Result, Style);
+  if (!Formatted)
+return Formatted.takeError();
+  Result = *Formatted;
+  auto NewCode = tooling::applyAllReplacements(Inputs.Code, Result);
+  if (!NewCode)
+return NewCode.takeError();
+
+  assert(!Result.empty());
+  // Calculate the offset of the dummy variable after applying replacements.
+  size_t ExtractVarOffset = Result.begin()->getOffset();
+  for (auto &R : Result) {
+auto OffsetInReplacement = R.getReplacementText().find(VarName);
+if (OffsetInReplacement != llvm::StringRef::npos) {
+  ExtractVarOffset += OffsetInReplacement;
+  break;
+}
+ExtractVarOffset += R.getReplacementText().size() - R.getLength();
+  }
+
+  assert(ExtractVarOffset != llvm::StringRef::npos);
+  Step Apply;
+  Apply.ApplyKind = Step::ApplyEdit;
+  Apply.ApplyEditParams = std::move(Result);
+  Step Rename;
+  Rename.ApplyKind = Step::Rename;
+  Rename.RenameParams = offsetToPosition(*NewCode, ExtractVarOffset);
+  // TODO: We should not emit Rename if the client doesn't support it.
+  return Effect::applySteps({std::move(Apply), std::move(Rename)});
 }
 
 // Find the CallExpr whose callee is an ancestor of the DeclRef
Index: clang-tools-extra/clangd/refactor/Tweak.h
===
--- clang-tools-extra/clangd/refactor/Tweak.h
+++ clang-tools-extra/clangd/refactor/Tweak.h
@@ -64,12 +64,44 @@
 /// Provide information to the user.
 Info,
   };
+
+  // The class represents a small and focus refactoring. The purpose is to form
+  // a chain of different refactorings, so that we could perform a large and
+  // complicate refactoring.
+  //
+  // Steps are applied in sequence. In LSP layer, each step is transformed to a
+  // call request to the LSP client, LSPServer waits for the client response and
+  // continues to apply next step when the previous step is finished
+  // successfully.
+  //
+  // clangd doesn't track changes of the source files when apply a step, so
+  // the following steps are not aware of the changes, Tweak is corresponding to
+  // pre-calculate the changes for all steps.
+  // FIXME: we should have a mechanism to track the changes for each step.
+  struct Step {
+enum Kind {
+  ApplyEdit,
+  Rename,
+};
+Kind ApplyKind;
+
+llvm::Optional ApplyEditParams; // For ApplyEdit
+llvm::Optional RenameParams; // For Rename
+  };
+
   struct Effect {
 /// A message to be displayed to the user.
 llvm::Optional ShowMessage;
 /// An edit to apply to the input file.
 llvm::Optional ApplyEdit;
+/// A chain of steps being executed in order when applying the tweak.
+llvm::Optional> ApplySteps;
 
+static Effect applySteps(std::vector Steps) {
+  Effect E;
+  E.ApplySteps = std::move(Steps);
+  return E;
+}
 static Effect applyEdit(tooling::Replacements R) {
   Effect E;
   E.ApplyEdit = std::move(R);
Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -128,6 +128,36 @@
 status.clear();
 }
 })
+
+clangdClient.onReady().then(
+() => {clangdClient.onRequest(
+"clangd.triggerRename", async (result: any) => {
+  if (result.arguments) {
+return await vscode.commands
+.executeCommand(
+'editor.action.rename',
+ 

[PATCH] D65445: [CrossTU] Handle case when no USR could be generated during Decl search.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Thank you for this patch Balazs! Could you please elaborate in which cases we 
cannot generate the USR? Does it happen only if we have an anonymous union 
inside a function? Are there no other cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65445



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

[PATCH] D65286: [OpenCL] Allow OpenCL C style vector initialization in C++

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 212350.
Anastasia added a comment.

- Merged test/CodeGenOpenCL/vector_literals_nested.cl into 
test/CodeGenOpenCL/vector_literals_valid.cl
- Added negative test case for C++


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

https://reviews.llvm.org/D65286

Files:
  lib/Sema/SemaInit.cpp
  test/CodeGenOpenCL/vector_literals_nested.cl
  test/CodeGenOpenCL/vector_literals_valid.cl
  test/SemaCXX/vector.cpp

Index: test/SemaCXX/vector.cpp
===
--- test/SemaCXX/vector.cpp
+++ test/SemaCXX/vector.cpp
@@ -334,3 +334,11 @@
 }
 
 } // namespace Templates
+
+typedef int inte2 __attribute__((__ext_vector_type__(2)));
+
+void test_vector_literal(inte4 res) {
+  inte2 a = (inte2)(1, 2); //expected-warning{{expression result unused}}
+  inte4 b = (inte4)(a, a); //expected-error{{C-style cast from vector 'inte2' (vector of 2 'int' values) to vector 'inte4' (vector of 4 'int' values) of different size}} //expected-warning{{expression result unused}}
+}
+
Index: test/CodeGenOpenCL/vector_literals_valid.cl
===
--- test/CodeGenOpenCL/vector_literals_valid.cl
+++ test/CodeGenOpenCL/vector_literals_valid.cl
@@ -1,22 +1,59 @@
-// RUN: %clang_cc1 -emit-llvm %s -o %t
+// RUN: %clang_cc1 -emit-llvm %s -o - -O0 | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -cl-std=clc++ -O0 | FileCheck %s
 
-typedef __attribute__(( ext_vector_type(2) ))  int int2;
-typedef __attribute__(( ext_vector_type(3) ))  int int3;
-typedef __attribute__(( ext_vector_type(4) ))  int int4;
-typedef __attribute__(( ext_vector_type(8) ))  int int8;
-typedef __attribute__(( ext_vector_type(4) ))  float float4;
+typedef __attribute__((ext_vector_type(2))) int int2;
+typedef __attribute__((ext_vector_type(3))) int int3;
+typedef __attribute__((ext_vector_type(4)))  int int4;
+typedef __attribute__((ext_vector_type(8)))  int int8;
+typedef __attribute__((ext_vector_type(4))) float float4;
 
 void vector_literals_valid() {
-  int4 a_1_1_1_1 = (int4)(1,2,3,4);
-  int4 a_2_1_1 = (int4)((int2)(1,2),3,4);
-  int4 a_1_2_1 = (int4)(1,(int2)(2,3),4);
-  int4 a_1_1_2 = (int4)(1,2,(int2)(3,4));
-  int4 a_2_2 = (int4)((int2)(1,2),(int2)(3,4));
-  int4 a_3_1 = (int4)((int3)(1,2,3),4);
-  int4 a_1_3 = (int4)(1,(int3)(2,3,4));
+  //CHECK: store <4 x i32> , <4 x i32>*
+
+  int4 a_1_1_1_1 = (int4)(1, 2, 3, 4);
+
+  //CHECK: store <2 x i32> , <2 x i32>*
+  //CHECK: shufflevector <2 x i32> %{{[0-9]+}}, <2 x i32> undef, <4 x i32> 
+  //CHECK: shufflevector <4 x i32> %{{.+}}, <4 x i32> undef, <4 x i32> 
+  //CHECK: insertelement <4 x i32> %{{.+}}, i32 3, i32 2
+  //CHECK: insertelement <4 x i32> %{{.+}}, i32 4, i32 3
+  int4 a_2_1_1 = (int4)((int2)(1, 2), 3, 4);
+
+  //CHECK: store <2 x i32> , <2 x i32>*
+  //CHECK: shufflevector <2 x i32> %{{[0-9]+}}, <2 x i32> undef, <4 x i32> 
+  //CHECK: shufflevector <4 x i32> , <4 x i32> %{{.+}}, <4 x i32> 
+  //CHECK: insertelement <4 x i32> %{{.+}}, i32 4, i32 3
+  int4 a_1_2_1 = (int4)(1, (int2)(2, 3), 4);
+
+  //CHECK: store <2 x i32> , <2 x i32>*
+  //CHECK: shufflevector <2 x i32> %{{[0-9]+}}, <2 x i32> undef, <4 x i32> 
+  //CHECK: shufflevector <4 x i32> , <4 x i32> %{{.+}}, <4 x i32> 
+  int4 a_1_1_2 = (int4)(1, 2, (int2)(3, 4));
+
+  //CHECK: store <2 x i32> , <2 x i32>*
+  //CHECK: shufflevector <2 x i32> %{{[0-9]+}}, <2 x i32> undef, <4 x i32> 
+  //CHECK: shufflevector <4 x i32> %{{.+}}, <4 x i32> undef, <4 x i32> 
+  //CHECK: shufflevector <4 x i32> %{{.+}}, <4 x i32> , <4 x i32> 
+  int4 a_2_2 = (int4)((int2)(1, 2), (int2)(3));
+
+  //CHECK: store <4 x i32> , <4 x i32>*
+  //CHECK: shufflevector <4 x i32> %{{.+}}, <4 x i32> undef, <3 x i32> 
+  //CHECK: shufflevector <3 x i32> %{{.+}}, <3 x i32> undef, <4 x i32> 
+  //CHECK: shufflevector <4 x i32> , <4 x i32> %{{.+}}, <4 x i32> 
+  int4 a_1_3 = (int4)(1, (int3)(2, 3, 4));
+
+  //CHECK: store <4 x i32> , <4 x i32>* %a
   int4 a = (int4)(1);
-  int8 b = (int8)(1,2,a.xy,a);
-  float4 V2 = (float4) (1);
-}
 
+  //CHECK: load <4 x i32>, <4 x i32>* %a, align 16
+  //CHECK: shufflevector <4 x i32> %{{[0-9]+}}, <4 x i32> undef, <2 x i32> 
+  //CHECK: shufflevector <2 x i32> %{{[0-9]+}}, <2 x i32> undef, <8 x i32> 
+  //CHECK: shufflevector <8 x i32> , <8 x i32> %{{.+}}, <8 x i32> 
+  //CHECK: load <4 x i32>, <4 x i32>* %a, align 16
+  //CHECK: shufflevector <4 x i32> %{{[0-9]+}}, <4 x i32> undef, <8 x i32> 
+  //CHECK: shufflevector <8 x i32> %{{.+}}, <8 x i32> %{{.+}}, <8 x i32> 
+  int8 b = (int8)(1, 2, a.xy, a);
 
+  //CHECK: store <4 x float> , <4 x float>* %V2
+  float4 V2 = (float4)(1);
+}
Index: test/CodeGenOpenCL/vector_literals_nested.cl
===
--- test/CodeGenOpenCL/vector_literals_nested.cl
+++ /dev/null
@@ -1,23 +0,0 @@
-// RUN: %clang_cc1 %s -emit-llvm -O3 -o - | FileCheck %s
-
-typedef int int2 __attribute((ext_vector_type(2

[PATCH] D65263: [clangd] a prototype for triggering rename after extracting.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D65263#1601036 , @ilya-biryukov 
wrote:

> If we are going down that path, could we consider to pre-selecting a name of 
> the extract variable and its usage (using multi-select)?


I agree that doing rename after extraction is not an ideal approach (both 
correctness and latency), there should be other better ways to achieve this. 
However as a LSP server, we may want to align with the community and other 
language servers (typescript,  java) in order to provide consistent refactoring 
experience across languages (vscode 
 prefers to do extract 
with a default name and rename afterwards). Note that this patch aims to 
implement a generic mechanism to perform a chain of small refactorings (extract 
+ rename is just one example), we still need this no match which way we're 
going to perform.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65263



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

[PATCH] D65263: [clangd] a prototype for triggering rename after extracting.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

The patch is still missing tests, but should be good for early comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65263



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

r367323 - [COFF][ARM64] Reorder handling of aarch64 MSVC builtins

[David Major via cfe-commits]

Author: dmajor
Date: Tue Jul 30 08:32:49 2019
New Revision: 367323

URL: http://llvm.org/viewvc/llvm-project?rev=367323&view=rev
Log:
[COFF][ARM64] Reorder handling of aarch64 MSVC builtins

In `CodeGenFunction::EmitAArch64BuiltinExpr()`, bulk move all of the aarch64 
MSVC-builtin cases to an earlier point in the function (the `// Handle 
non-overloaded intrinsics first` switch block) in order to avoid an unreachable 
in `GetNeonType()`. The NEON type-overloading logic is not appropriate for the 
Windows builtins.

Fixes https://llvm.org/pr42775

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


Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=367323&r1=367322&r2=367323&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Tue Jul 30 08:32:49 2019
@@ -8014,6 +8014,151 @@ Value *CodeGenFunction::EmitAArch64Built
 return Builder.CreateExtractElement(Ops[0], EmitScalarExpr(E->getArg(1)),
 "vgetq_lane");
   }
+  case AArch64::BI_BitScanForward:
+  case AArch64::BI_BitScanForward64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_BitScanForward, E);
+  case AArch64::BI_BitScanReverse:
+  case AArch64::BI_BitScanReverse64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_BitScanReverse, E);
+  case AArch64::BI_InterlockedAnd64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedAnd, E);
+  case AArch64::BI_InterlockedExchange64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange, E);
+  case AArch64::BI_InterlockedExchangeAdd64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchangeAdd, E);
+  case AArch64::BI_InterlockedExchangeSub64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchangeSub, E);
+  case AArch64::BI_InterlockedOr64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedOr, E);
+  case AArch64::BI_InterlockedXor64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedXor, E);
+  case AArch64::BI_InterlockedDecrement64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedDecrement, E);
+  case AArch64::BI_InterlockedIncrement64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedIncrement, E);
+  case AArch64::BI_InterlockedExchangeAdd8_acq:
+  case AArch64::BI_InterlockedExchangeAdd16_acq:
+  case AArch64::BI_InterlockedExchangeAdd_acq:
+  case AArch64::BI_InterlockedExchangeAdd64_acq:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchangeAdd_acq, E);
+  case AArch64::BI_InterlockedExchangeAdd8_rel:
+  case AArch64::BI_InterlockedExchangeAdd16_rel:
+  case AArch64::BI_InterlockedExchangeAdd_rel:
+  case AArch64::BI_InterlockedExchangeAdd64_rel:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchangeAdd_rel, E);
+  case AArch64::BI_InterlockedExchangeAdd8_nf:
+  case AArch64::BI_InterlockedExchangeAdd16_nf:
+  case AArch64::BI_InterlockedExchangeAdd_nf:
+  case AArch64::BI_InterlockedExchangeAdd64_nf:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchangeAdd_nf, E);
+  case AArch64::BI_InterlockedExchange8_acq:
+  case AArch64::BI_InterlockedExchange16_acq:
+  case AArch64::BI_InterlockedExchange_acq:
+  case AArch64::BI_InterlockedExchange64_acq:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange_acq, E);
+  case AArch64::BI_InterlockedExchange8_rel:
+  case AArch64::BI_InterlockedExchange16_rel:
+  case AArch64::BI_InterlockedExchange_rel:
+  case AArch64::BI_InterlockedExchange64_rel:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange_rel, E);
+  case AArch64::BI_InterlockedExchange8_nf:
+  case AArch64::BI_InterlockedExchange16_nf:
+  case AArch64::BI_InterlockedExchange_nf:
+  case AArch64::BI_InterlockedExchange64_nf:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange_nf, E);
+  case AArch64::BI_InterlockedCompareExchange8_acq:
+  case AArch64::BI_InterlockedCompareExchange16_acq:
+  case AArch64::BI_InterlockedCompareExchange_acq:
+  case AArch64::BI_InterlockedCompareExchange64_acq:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedCompareExchange_acq, E);
+  case AArch64::BI_InterlockedCompareExchange8_rel:
+  case AArch64::BI_InterlockedCompareExchange16_rel:
+  case AArch64::BI_InterlockedCompareExchange_rel:
+  case AArch64::BI_InterlockedCompareExchange64_rel:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedCompareExchange_rel, E);
+  case AArch64::BI_InterlockedCompareExchange8_nf:
+  case AArch64::BI_InterlockedCompareExchange16_nf:
+  case AArch64::BI_InterlockedCompareExchange_nf:
+  case AArch64::BI_InterlockedCompareExchange64_nf:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedCompareExchange_nf, E);
+  case AArch64::BI_InterlockedOr8_acq:
+  case AArch64::BI_InterlockedOr16_acq:
+  c

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel created this revision.
jcranmer-intel added a reviewer: dcoughlin.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch improves Clang call graph analysis by adding in expressions that are 
not found in regular function bodies, such as default arguments or C++ member 
initializers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65453

Files:
  clang/lib/Analysis/CallGraph.cpp
  clang/test/Analysis/cxx-callgraph.cpp


Index: clang/test/Analysis/cxx-callgraph.cpp
===
--- /dev/null
+++ clang/test/Analysis/cxx-callgraph.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCallGraph %s 2>&1 | 
FileCheck %s
+
+static int aaa() {
+  return 0;
+}
+
+static int bbb(int param=aaa()) {
+  return 1;
+}
+
+int ddd();
+
+struct c {
+  c(int param=2) : val(bbb(param)) {}
+  int val;
+  int val2 = ddd();
+};
+
+int ddd() {
+  c c;
+  return bbb();
+}
+
+// CHECK:--- Call graph Dump ---
+// CHECK-NEXT: {{Function: < root > calls: aaa bbb c::c ddd}}
+// CHECK-NEXT: {{Function: c::c calls: bbb ddd $}}
+// CHECK-NEXT: {{Function: ddd calls: c::c bbb aaa $}}
+// CHECK-NEXT: {{Function: bbb calls: $}}
+// CHECK-NEXT: {{Function: aaa calls: $}}
Index: clang/lib/Analysis/CallGraph.cpp
===
--- clang/lib/Analysis/CallGraph.cpp
+++ clang/lib/Analysis/CallGraph.cpp
@@ -103,6 +103,16 @@
 VisitChildren(E);
   }
 
+  // Include the evaluation of the default argument.
+  void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
+Visit(E->getExpr());
+  }
+
+  // Include the evaluation of the default initializers in a class.
+  void VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
+Visit(E->getExpr());
+  }
+
   // Adds may-call edges for the ObjC message sends.
   void VisitObjCMessageExpr(ObjCMessageExpr *ME) {
 if (ObjCInterfaceDecl *IDecl = ME->getReceiverInterface()) {
@@ -167,13 +177,20 @@
 void CallGraph::addNodeForDecl(Decl* D, bool IsGlobal) {
   assert(D);
 
-  // Allocate a new node, mark it as root, and process it's calls.
+  // Allocate a new node, mark it as root, and process its calls.
   CallGraphNode *Node = getOrInsertNode(D);
 
   // Process all the calls by this function as well.
   CGBuilder builder(this, Node);
   if (Stmt *Body = D->getBody())
 builder.Visit(Body);
+
+  // Include C++ constructor member initializers.
+  if (auto constructor = dyn_cast(D)) {
+for (CXXCtorInitializer *init : constructor->inits()) {
+  builder.Visit(init->getInit());
+}
+  }
 }
 
 CallGraphNode *CallGraph::getNode(const Decl *F) const {


Index: clang/test/Analysis/cxx-callgraph.cpp
===
--- /dev/null
+++ clang/test/Analysis/cxx-callgraph.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCallGraph %s 2>&1 | FileCheck %s
+
+static int aaa() {
+  return 0;
+}
+
+static int bbb(int param=aaa()) {
+  return 1;
+}
+
+int ddd();
+
+struct c {
+  c(int param=2) : val(bbb(param)) {}
+  int val;
+  int val2 = ddd();
+};
+
+int ddd() {
+  c c;
+  return bbb();
+}
+
+// CHECK:--- Call graph Dump ---
+// CHECK-NEXT: {{Function: < root > calls: aaa bbb c::c ddd}}
+// CHECK-NEXT: {{Function: c::c calls: bbb ddd $}}
+// CHECK-NEXT: {{Function: ddd calls: c::c bbb aaa $}}
+// CHECK-NEXT: {{Function: bbb calls: $}}
+// CHECK-NEXT: {{Function: aaa calls: $}}
Index: clang/lib/Analysis/CallGraph.cpp
===
--- clang/lib/Analysis/CallGraph.cpp
+++ clang/lib/Analysis/CallGraph.cpp
@@ -103,6 +103,16 @@
 VisitChildren(E);
   }
 
+  // Include the evaluation of the default argument.
+  void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
+Visit(E->getExpr());
+  }
+
+  // Include the evaluation of the default initializers in a class.
+  void VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
+Visit(E->getExpr());
+  }
+
   // Adds may-call edges for the ObjC message sends.
   void VisitObjCMessageExpr(ObjCMessageExpr *ME) {
 if (ObjCInterfaceDecl *IDecl = ME->getReceiverInterface()) {
@@ -167,13 +177,20 @@
 void CallGraph::addNodeForDecl(Decl* D, bool IsGlobal) {
   assert(D);
 
-  // Allocate a new node, mark it as root, and process it's calls.
+  // Allocate a new node, mark it as root, and process its calls.
   CallGraphNode *Node = getOrInsertNode(D);
 
   // Process all the calls by this function as well.
   CGBuilder builder(this, Node);
   if (Stmt *Body = D->getBody())
 builder.Visit(Body);
+
+  // Include C++ constructor member initializers.
+  if (auto constructor = dyn_cast(D)) {
+for (CXXCtorInitializer *init : constructor->inits()) {
+  builder.Visit(init->getInit());
+}
+  }
 }
 
 CallGraphNode *CallGraph::getNode(const Decl *F) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.

[PATCH] D65403: [COFF, ARM64] Reorder handling of aarch64 MSVC builtins

[David Major via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367323: [COFF][ARM64] Reorder handling of aarch64 MSVC 
builtins (authored by dmajor, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65403?vs=212181&id=212355#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65403

Files:
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c

Index: cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c
===
--- cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c
+++ cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c
@@ -8,6 +8,10 @@
   return _InterlockedAdd(Addend, Value);
 }
 
+long test_InterlockedAdd_constant(long volatile *Addend) {
+  return _InterlockedAdd(Addend, -1);
+}
+
 // CHECK-LABEL: define {{.*}} i32 @test_InterlockedAdd(i32* %Addend, i32 %Value) {{.*}} {
 // CHECK-MSVC: %[[OLDVAL:[0-9]+]] = atomicrmw add i32* %1, i32 %2 seq_cst
 // CHECK-MSVC: %[[NEWVAL:[0-9]+]] = add i32 %[[OLDVAL:[0-9]+]], %2
Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -8014,6 +8014,151 @@
 return Builder.CreateExtractElement(Ops[0], EmitScalarExpr(E->getArg(1)),
 "vgetq_lane");
   }
+  case AArch64::BI_BitScanForward:
+  case AArch64::BI_BitScanForward64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_BitScanForward, E);
+  case AArch64::BI_BitScanReverse:
+  case AArch64::BI_BitScanReverse64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_BitScanReverse, E);
+  case AArch64::BI_InterlockedAnd64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedAnd, E);
+  case AArch64::BI_InterlockedExchange64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange, E);
+  case AArch64::BI_InterlockedExchangeAdd64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchangeAdd, E);
+  case AArch64::BI_InterlockedExchangeSub64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchangeSub, E);
+  case AArch64::BI_InterlockedOr64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedOr, E);
+  case AArch64::BI_InterlockedXor64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedXor, E);
+  case AArch64::BI_InterlockedDecrement64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedDecrement, E);
+  case AArch64::BI_InterlockedIncrement64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedIncrement, E);
+  case AArch64::BI_InterlockedExchangeAdd8_acq:
+  case AArch64::BI_InterlockedExchangeAdd16_acq:
+  case AArch64::BI_InterlockedExchangeAdd_acq:
+  case AArch64::BI_InterlockedExchangeAdd64_acq:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchangeAdd_acq, E);
+  case AArch64::BI_InterlockedExchangeAdd8_rel:
+  case AArch64::BI_InterlockedExchangeAdd16_rel:
+  case AArch64::BI_InterlockedExchangeAdd_rel:
+  case AArch64::BI_InterlockedExchangeAdd64_rel:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchangeAdd_rel, E);
+  case AArch64::BI_InterlockedExchangeAdd8_nf:
+  case AArch64::BI_InterlockedExchangeAdd16_nf:
+  case AArch64::BI_InterlockedExchangeAdd_nf:
+  case AArch64::BI_InterlockedExchangeAdd64_nf:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchangeAdd_nf, E);
+  case AArch64::BI_InterlockedExchange8_acq:
+  case AArch64::BI_InterlockedExchange16_acq:
+  case AArch64::BI_InterlockedExchange_acq:
+  case AArch64::BI_InterlockedExchange64_acq:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange_acq, E);
+  case AArch64::BI_InterlockedExchange8_rel:
+  case AArch64::BI_InterlockedExchange16_rel:
+  case AArch64::BI_InterlockedExchange_rel:
+  case AArch64::BI_InterlockedExchange64_rel:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange_rel, E);
+  case AArch64::BI_InterlockedExchange8_nf:
+  case AArch64::BI_InterlockedExchange16_nf:
+  case AArch64::BI_InterlockedExchange_nf:
+  case AArch64::BI_InterlockedExchange64_nf:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange_nf, E);
+  case AArch64::BI_InterlockedCompareExchange8_acq:
+  case AArch64::BI_InterlockedCompareExchange16_acq:
+  case AArch64::BI_InterlockedCompareExchange_acq:
+  case AArch64::BI_InterlockedCompareExchange64_acq:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedCompareExchange_acq, E);
+  case AArch64::BI_InterlockedCompareExchange8_rel:
+  case AArch64::BI_InterlockedCompareExchange16_rel:
+  case AArch64::BI_InterlockedCompareExchange_rel:
+  case AArch64::BI_InterlockedCompareExchange64_rel:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedCompareExchange_rel, E);
+  case AArch64::BI_InterlockedCompareExchange8_nf:
+  case AArch64::BI_InterlockedComp

[PATCH] D64564: Loop pragma parsing. NFC.

[Michael Kruse via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D64564



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

[PATCH] D64564: Loop pragma parsing. NFC.

[Michael Kruse via Phabricator via cfe-commits]

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



Comment at: lib/Parse/ParsePragma.cpp:1011
+  Str = llvm::StringSwitch(Str)
+   .Case("loop", "clang loop " + Str.str())
+   .Case("unroll_and_jam", Str)

[serious] I know I already accepted the patch, but I just noticed something:
`"clang loop " + Str.str()` will allocate a temporary std::string, `Str` will 
potentially point to it, then the temporary string will be released. `Str` will 
then point to released memory and returned by this function, i.e. a 
use-after-free.



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

https://reviews.llvm.org/D64564



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

[PATCH] D65286: [OpenCL] Allow OpenCL C style vector initialization in C++

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D65286#1606071 , @mantognini wrote:

> In `vector_literals_nested.cl`, we have tests for (global) constants. Do you 
> think it would be worth testing those as well in C++ mode? Maybe the two 
> files (`vector_literals_nested.cl` and `vector_literals_valid.cl`) should 
> also be merged as most of their content seems duplicated.


Thanks for pointing out. Indeed there was lots of repetition. I only added one 
missing test case (line 37 of `vector_literals_valid.cl`) and also removed 
another similar file `test/SemaOpenCL/vector_literals_const.cl`.

> In C++, we have the comma operator and therefore the AST is significantly 
> different. Running the content of your test file with `-x c++` shows that it 
> is rejected as desired. It could be worth having a negative test to ensure we 
> always reject this in vanilla C++.

Added test case in `test/SemaCXX/vector.cpp`. However I am now confused what 
syntax we shouldn't accept exactly. @rjmccall  do you think there should be an 
error on line  341?

> Regarding the code itself, I'm not familiar with 
> `InitializationSequence`/`InitListChecker` that much, but the patch makes 
> sense I would say.

Ok, thanks. It seems apparently the last time this code was modified in 2015... 
so might not be easy to remember details for anyone. :(


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

https://reviews.llvm.org/D65286



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

[PATCH] D65286: [OpenCL] Allow OpenCL C style vector initialization in C++

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 212358.
Anastasia added a comment.

- Removed another duplicate file and added testing of constant program scope 
initializers into vector_literals_nested.cl


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

https://reviews.llvm.org/D65286

Files:
  lib/Sema/SemaInit.cpp
  test/CodeGenOpenCL/vector_literals_nested.cl
  test/CodeGenOpenCL/vector_literals_valid.cl
  test/SemaCXX/vector.cpp
  test/SemaOpenCL/vector_literals_const.cl

Index: test/SemaOpenCL/vector_literals_const.cl
===
--- test/SemaOpenCL/vector_literals_const.cl
+++ /dev/null
@@ -1,27 +0,0 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
-// expected-no-diagnostics
-
-typedef int int2 __attribute((ext_vector_type(2)));
-typedef int int3 __attribute((ext_vector_type(3)));
-typedef int int4 __attribute((ext_vector_type(4)));
-
-__constant int4 i_1_1_1_1 = (int4)(1,2,3,4);
-__constant int4 i_2_1_1 = (int4)((int2)(1,2),3,4);
-__constant int4 i_1_2_1 = (int4)(1,(int2)(2,3),4);
-__constant int4 i_1_1_2 = (int4)(1,2,(int2)(3,4));
-__constant int4 i_2_2 = (int4)((int2)(1,2),(int2)(3,4));
-__constant int4 i_3_1 = (int4)((int3)(1,2,3),4);
-__constant int4 i_1_3 = (int4)(1,(int3)(2,3,4));
-
-typedef float float2 __attribute((ext_vector_type(2)));
-typedef float float3 __attribute((ext_vector_type(3)));
-typedef float float4 __attribute((ext_vector_type(4)));
-
-__constant float4 f_1_1_1_1 = (float4)(1,2,3,4);
-__constant float4 f_2_1_1 = (float4)((float2)(1,2),3,4);
-__constant float4 f_1_2_1 = (float4)(1,(float2)(2,3),4);
-__constant float4 f_1_1_2 = (float4)(1,2,(float2)(3,4));
-__constant float4 f_2_2 = (float4)((float2)(1,2),(float2)(3,4));
-__constant float4 f_3_1 = (float4)((float3)(1,2,3),4);
-__constant float4 f_1_3 = (float4)(1,(float3)(2,3,4));
-
Index: test/SemaCXX/vector.cpp
===
--- test/SemaCXX/vector.cpp
+++ test/SemaCXX/vector.cpp
@@ -334,3 +334,11 @@
 }
 
 } // namespace Templates
+
+typedef int inte2 __attribute__((__ext_vector_type__(2)));
+
+void test_vector_literal(inte4 res) {
+  inte2 a = (inte2)(1, 2); //expected-warning{{expression result unused}}
+  inte4 b = (inte4)(a, a); //expected-error{{C-style cast from vector 'inte2' (vector of 2 'int' values) to vector 'inte4' (vector of 4 'int' values) of different size}} //expected-warning{{expression result unused}}
+}
+
Index: test/CodeGenOpenCL/vector_literals_valid.cl
===
--- test/CodeGenOpenCL/vector_literals_valid.cl
+++ test/CodeGenOpenCL/vector_literals_valid.cl
@@ -1,22 +1,65 @@
-// RUN: %clang_cc1 -emit-llvm %s -o %t
+// RUN: %clang_cc1 -emit-llvm %s -o - -O0 | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -cl-std=clc++ -O0 | FileCheck %s
 
-typedef __attribute__(( ext_vector_type(2) ))  int int2;
-typedef __attribute__(( ext_vector_type(3) ))  int int3;
-typedef __attribute__(( ext_vector_type(4) ))  int int4;
-typedef __attribute__(( ext_vector_type(8) ))  int int8;
-typedef __attribute__(( ext_vector_type(4) ))  float float4;
+typedef __attribute__((ext_vector_type(2))) int int2;
+typedef __attribute__((ext_vector_type(3))) int int3;
+typedef __attribute__((ext_vector_type(4)))  int int4;
+typedef __attribute__((ext_vector_type(8)))  int int8;
+typedef __attribute__((ext_vector_type(4))) float float4;
+
+__constant const int4 c1 = (int4)(1, 2, ((int2)(3)));
+// CHECK: constant <4 x i32> 
+
+__constant const int4 c2 = (int4)(1, 2, ((int2)(3, 4)));
+// CHECK: constant <4 x i32> 
 
 void vector_literals_valid() {
-  int4 a_1_1_1_1 = (int4)(1,2,3,4);
-  int4 a_2_1_1 = (int4)((int2)(1,2),3,4);
-  int4 a_1_2_1 = (int4)(1,(int2)(2,3),4);
-  int4 a_1_1_2 = (int4)(1,2,(int2)(3,4));
-  int4 a_2_2 = (int4)((int2)(1,2),(int2)(3,4));
-  int4 a_3_1 = (int4)((int3)(1,2,3),4);
-  int4 a_1_3 = (int4)(1,(int3)(2,3,4));
+  //CHECK: insertelement <4 x i32> , i32 %{{.+}}, i32 2
+  //CHECK: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 3
+  int4 a_1_1_1_1 = (int4)(1, 2, c1.s2, c2.s3);
+
+  //CHECK: store <2 x i32> , <2 x i32>*
+  //CHECK: shufflevector <2 x i32> %{{[0-9]+}}, <2 x i32> undef, <4 x i32> 
+  //CHECK: shufflevector <4 x i32> %{{.+}}, <4 x i32> undef, <4 x i32> 
+  //CHECK: insertelement <4 x i32> %{{.+}}, i32 3, i32 2
+  //CHECK: insertelement <4 x i32> %{{.+}}, i32 4, i32 3
+  int4 a_2_1_1 = (int4)((int2)(1, 2), 3, 4);
+
+  //CHECK: store <2 x i32> , <2 x i32>*
+  //CHECK: shufflevector <2 x i32> %{{[0-9]+}}, <2 x i32> undef, <4 x i32> 
+  //CHECK: shufflevector <4 x i32> , <4 x i32> %{{.+}}, <4 x i32> 
+  //CHECK: insertelement <4 x i32> %{{.+}}, i32 4, i32 3
+  int4 a_1_2_1 = (int4)(1, (int2)(2, 3), 4);
+
+  //CHECK: store <2 x i32> , <2 x i32>*
+  //CHECK: shufflevector <2 x i32> %{{[0-9]+}}, <2 x i32> undef, <4 x i32> 
+  //CHECK: shufflevector <4 x i32> , <4 x i32> %{{.+}}, <4 x i32> 
+  int4 a_1_1_2 = (int4)(1,

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added subscribers: sammccall, JDevlieghere.
aganea added inline comments.



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:1
+//===- DependencyScanningFilesystem.h - clang-scan-deps fs ===---*- C++ 
-*-===//
+//

General comment for this file and the implementation: it seems there's nothing 
specific to the dependency scanning, except for replacing the content with 
minimized content? This cached FS could be very well used to compile a project 
with parallel workers. Could this be part of the `VirtualFileSystem` 
infrastructure? ( `llvm/include/llvm/Support/VirtualFileSystem.h`) If yes, can 
you possibly create a separate patch for this? @JDevlieghere @sammccall 



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:101
+public:
+  /// A \c CachedFileSystemEntry with a lock.
+  struct SharedFileSystemEntry {

The struct is self-explanatory, not sure the comment is needed here?



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:103
+  struct SharedFileSystemEntry {
+std::mutex Lock;
+CachedFileSystemEntry Value;

Would you mind renaming this to `ValueLock` so it is easier to search for? (and 
to remain consistent with `CacheLock` below)



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:129
+/// computation.
+class DependencyScanningFilesystem : public llvm::vfs::ProxyFileSystem {
+public:

Maybe worth mentioning this is NOT a global, thread-safe, class like 
`DependencyScanningFilesystemSharedCache`, but rather meant to be used as 
per-thread instances?

I am also wondering if there could be a better name to signify at first sight 
that this is a per-thread class. `DependencyScanningLocalFilesystem`? 
`DependencyScanningWorkerFilesystem`?



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:84
+DependencyScanningFilesystemSharedCache() {
+  NumShards = 8;
+  CacheShards = llvm::make_unique(NumShards);

This line needs a comment. Is this value based on empirical results across 
different hardwares? (I can't recall your answer at the LLVM conf) I am 
wondering what would be the good strategy here? The more cores/HT in your PC, 
the more chances that you'll hit the same shard, thus locking. But the bigger 
we make this value, the more `StringMaps` we will have, and more cache misses 
possibly.
Should it be something like `llvm::hardware_concurrency() / some_fudge`? It'd 
be interesting to subsequently profile on high core count machines (or maybe 
you have done that).



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:210
+  bool KeepOriginalSource = IgnoredFiles.count(Filename);
+  auto &SharedCacheEntry = SharedCache.get(Filename);
+  const CachedFileSystemEntry *Result;

Don't use `auto` when the type is not obvious.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:148
+  RealFS = new ProxyFileSystemWithoutChdir(llvm::vfs::getRealFileSystem());
+  if (Service.getMode() == ScanningMode::MinimizedSourcePreprocessing)
+DepFS = new DependencyScanningFilesystem(Service.getSharedCache(), RealFS);

Can we not use caching all the time?


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

https://reviews.llvm.org/D63907



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

[PATCH] D65454: AMDGPU: Add missing builtin declarations

[Matt Arsenault via Phabricator via cfe-commits]

arsenm created this revision.
arsenm added reviewers: yaxunl, rampitec, b-sumner, kzhuravl.
Herald added subscribers: t-tye, tpr, dstuttard, nhaehnle, wdng, jvesely.

https://reviews.llvm.org/D65454

Files:
  include/clang/Basic/BuiltinsAMDGPU.def
  test/CodeGenOpenCL/builtins-amdgcn.cl


Index: test/CodeGenOpenCL/builtins-amdgcn.cl
===
--- test/CodeGenOpenCL/builtins-amdgcn.cl
+++ test/CodeGenOpenCL/builtins-amdgcn.cl
@@ -9,6 +9,7 @@
 typedef half __attribute__((ext_vector_type(2))) half2;
 typedef short __attribute__((ext_vector_type(2))) short2;
 typedef ushort __attribute__((ext_vector_type(2))) ushort2;
+typedef uint __attribute__((ext_vector_type(4))) uint4;
 
 // CHECK-LABEL: @test_div_scale_f64
 // CHECK: call { double, i1 } @llvm.amdgcn.div.scale.f64(double %a, double %b, 
i1 true)
@@ -654,6 +655,48 @@
   *out = __builtin_amdgcn_cvt_pk_u8_f32(src0, src1, src2);
 }
 
+// CHECK-LABEL: @test_sad_u8(
+// CHECK: tail call i32 @llvm.amdgcn.sad.u8(i32 %src0, i32 %src1, i32 %src2)
+kernel void test_sad_u8(global uint* out, uint src0, uint src1, uint src2) {
+  *out = __builtin_amdgcn_sad_u8(src0, src1, src2);
+}
+
+// CHECK-LABEL: test_msad_u8(
+// CHECK: call i32 @llvm.amdgcn.msad.u8(i32 %src0, i32 %src1, i32 %src2)
+kernel void test_msad_u8(global uint* out, uint src0, uint src1, uint src2) {
+  *out = __builtin_amdgcn_msad_u8(src0, src1, src2);
+}
+
+// CHECK-LABEL: test_sad_hi_u8(
+// CHECK: call i32 @llvm.amdgcn.sad.hi.u8(i32 %src0, i32 %src1, i32 %src2)
+kernel void test_sad_hi_u8(global uint* out, uint src0, uint src1, uint src2) {
+  *out = __builtin_amdgcn_sad_hi_u8(src0, src1, src2);
+}
+
+// CHECK-LABEL: @test_sad_u16(
+// CHECK: call i32 @llvm.amdgcn.sad.u16(i32 %src0, i32 %src1, i32 %src2)
+kernel void test_sad_u16(global uint* out, uint src0, uint src1, uint src2) {
+  *out = __builtin_amdgcn_sad_u16(src0, src1, src2);
+}
+
+// CHECK-LABEL: @test_qsad_pk_u16_u8(
+// CHECK: call i64 @llvm.amdgcn.qsad.pk.u16.u8(i64 %src0, i32 %src1, i64 %src2)
+kernel void test_qsad_pk_u16_u8(global ulong* out, ulong src0, uint src1, 
ulong src2) {
+  *out = __builtin_amdgcn_qsad_pk_u16_u8(src0, src1, src2);
+}
+
+// CHECK-LABEL: @test_mqsad_pk_u16_u8(
+// CHECK: call i64 @llvm.amdgcn.mqsad.pk.u16.u8(i64 %src0, i32 %src1, i64 
%src2)
+kernel void test_mqsad_pk_u16_u8(global ulong* out, ulong src0, uint src1, 
ulong src2) {
+  *out = __builtin_amdgcn_mqsad_pk_u16_u8(src0, src1, src2);
+}
+
+// CHECK-LABEL: test_mqsad_u32_u8(
+// CHECK: call <4 x i32> @llvm.amdgcn.mqsad.u32.u8(i64 %src0, i32 %src1, <4 x 
i32> %src2)
+kernel void test_mqsad_u32_u8(global uint4* out, ulong src0, uint src1, uint4 
src2) {
+  *out = __builtin_amdgcn_mqsad_u32_u8(src0, src1, src2);
+}
+
 // CHECK-DAG: [[$WI_RANGE]] = !{i32 0, i32 1024}
 // CHECK-DAG: attributes #[[$NOUNWIND_READONLY:[0-9]+]] = { nounwind readonly }
 // CHECK-DAG: attributes #[[$READ_EXEC_ATTRS]] = { convergent }
Index: include/clang/Basic/BuiltinsAMDGPU.def
===
--- include/clang/Basic/BuiltinsAMDGPU.def
+++ include/clang/Basic/BuiltinsAMDGPU.def
@@ -118,6 +118,13 @@
 BUILTIN(__builtin_amdgcn_cvt_pk_i16, "E2sii", "nc")
 BUILTIN(__builtin_amdgcn_cvt_pk_u16, "E2UsUiUi", "nc")
 BUILTIN(__builtin_amdgcn_cvt_pk_u8_f32, "UifUiUi", "nc")
+BUILTIN(__builtin_amdgcn_sad_u8, "UiUiUiUi", "nc")
+BUILTIN(__builtin_amdgcn_msad_u8, "UiUiUiUi", "nc")
+BUILTIN(__builtin_amdgcn_sad_hi_u8, "UiUiUiUi", "nc")
+BUILTIN(__builtin_amdgcn_sad_u16, "UiUiUiUi", "nc")
+BUILTIN(__builtin_amdgcn_qsad_pk_u16_u8, "LUiLUiUiLUi", "nc")
+BUILTIN(__builtin_amdgcn_mqsad_pk_u16_u8, "LUiLUiUiLUi", "nc")
+BUILTIN(__builtin_amdgcn_mqsad_u32_u8, "V4UiLUiUiV4Ui", "nc")
 
 
//===--===//
 // CI+ only builtins.


Index: test/CodeGenOpenCL/builtins-amdgcn.cl
===
--- test/CodeGenOpenCL/builtins-amdgcn.cl
+++ test/CodeGenOpenCL/builtins-amdgcn.cl
@@ -9,6 +9,7 @@
 typedef half __attribute__((ext_vector_type(2))) half2;
 typedef short __attribute__((ext_vector_type(2))) short2;
 typedef ushort __attribute__((ext_vector_type(2))) ushort2;
+typedef uint __attribute__((ext_vector_type(4))) uint4;
 
 // CHECK-LABEL: @test_div_scale_f64
 // CHECK: call { double, i1 } @llvm.amdgcn.div.scale.f64(double %a, double %b, i1 true)
@@ -654,6 +655,48 @@
   *out = __builtin_amdgcn_cvt_pk_u8_f32(src0, src1, src2);
 }
 
+// CHECK-LABEL: @test_sad_u8(
+// CHECK: tail call i32 @llvm.amdgcn.sad.u8(i32 %src0, i32 %src1, i32 %src2)
+kernel void test_sad_u8(global uint* out, uint src0, uint src1, uint src2) {
+  *out = __builtin_amdgcn_sad_u8(src0, src1, src2);
+}
+
+// CHECK-LABEL: test_msad_u8(
+// CHECK: call i32 @llvm.amdgcn.msad.u8(i32 %src0, i32 %src1, i32 %src2)
+kernel void test_msad_u8(global uint* out, uint src0, uint src1, uint src2) {
+  *out = __builtin_amdgcn_msad_u8(src0,

[PATCH] D64742: Allow using -ftrivial-auto-var-init=zero in C mode without extra flags

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D64742#1606244 , @glider wrote:

> As a data point, Linus Torvalds suggested that we need a similar feature for 
> GCC so that the "kernel C standard" mandates zero-initialization for locals: 
> https://lkml.org/lkml/2019/7/28/206


Interesting!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64742



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

[PATCH] D65286: [OpenCL] Allow OpenCL C style vector initialization in C++

[Marco Antognini via Phabricator via cfe-commits]

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

LGTM, thanks for the update.


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

https://reviews.llvm.org/D65286



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

[PATCH] D65456: [OpenCL] Add generic type handling for builtin functions

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh created this revision.
svenvh added a reviewer: Anastasia.
Herald added subscribers: cfe-commits, kristina, yaxunl.
Herald added a reviewer: rengolin.
Herald added a project: clang.

Generic types are an abstraction of type sets.  It mimics the way
functions are defined in the OpenCL specification.  For example,
floatN can abstract all the vector sizes of the float type.

This allows to

- stick more closely to the specification, which uses generic types;
- factorize definitions of functions with numerous prototypes in the tablegen 
file; and
- reduce the memory impact of functions with many overloads.

Patch by Pierre Gondois and Sven van Haastregt.

Continuation of https://reviews.llvm.org/D63434


Repository:
  rC Clang

https://reviews.llvm.org/D65456

Files:
  clang/lib/Sema/OpenCLBuiltins.td
  clang/lib/Sema/SemaLookup.cpp
  clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
  clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

Index: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
===
--- clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
+++ clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
@@ -15,12 +15,39 @@
 //
 // For a successful lookup of e.g. the "cos" builtin, isOpenCLBuiltin("cos")
 // returns a pair .
-// OpenCLBuiltins[Index] to OpenCLBuiltins[Index + Len] contains the pairs
+// BuiltinTable[Index] to BuiltinTable[Index + Len] contains the pairs
 //  of the overloads of "cos".
-// OpenCLSignature[SigIndex] to OpenCLSignature[SigIndex + SigLen] contains
-// one of the signatures of "cos". The OpenCLSignature entry can be
-// referenced by other functions, i.e. "sin", since multiple OpenCL builtins
-// share the same signature.
+// SignatureTable[SigIndex] to SignatureTable[SigIndex + SigLen] contains
+// one of the signatures of "cos". The SignatureTable entry can be
+// referenced by other functions, e.g. "sin", to exploit the fact that
+// many OpenCL builtins share the same signature.
+//
+// The file generated by this TableGen emitter contains the following:
+//
+//  * Structs and enums to represent types and function signatures.
+//
+//  * OpenCLTypeStruct TypeTable[]
+//Type information for return types and arguments.
+//
+//  * unsigned SignatureTable[]
+//A list of types representing function signatures.  Each entry is an index
+//into the above TypeTable.  Multiple entries following each other form a
+//signature, where the first entry is the return type and subsequent
+//entries are the argument types.
+//
+//  * OpenCLBuiltinStruct BuiltinTable[]
+//Each entry represents one overload of an OpenCL builtin function and
+//consists of an index into the SignatureTable and the number of arguments.
+//
+//  * std::pair isOpenCLBuiltin(llvm::StringRef Name)
+//Find out whether a string matches an existing OpenCL builtin function
+//name and return an index into BuiltinTable and the number of overloads.
+//
+//  * void OCL2Qual(ASTContext&, OpenCLTypeStruct, std::vector&)
+//Convert an OpenCLTypeStruct type to a list of QualType instances.
+//One OpenCLTypeStruct can represent multiple types, primarily when using
+//GenTypes.
+//
 //===--===//
 
 #include "llvm/ADT/MapVector.h"
@@ -57,34 +84,52 @@
   // The output file.
   raw_ostream &OS;
 
-  // Emit the enums and structs.
+  // Helper function for BuiltinNameEmitter::EmitDeclarations.  Generate enum
+  // definitions in the Output string parameter, and save their Record instance
+  // in the List parameter.
+  // \param Types (in) List containing the Types to extract.
+  // \param TypesSeen (out) List containing the Types already extracted.
+  // \param Output (out) String containing the enums to emit in the output file.
+  // \param List (out) List to fill with the extracted types.
+  void ExtractEnumTypes(std::vector &Types,
+StringMap &TypesSeen, std::string &Output,
+std::vector &List);
+
+  // Emit the enum or struct used in the generated file.
+  // Populate the TypeList at the same time.
   void EmitDeclarations();
 
-  // Parse the Records generated by TableGen and populate OverloadInfo and
-  // SignatureSet.
+  // Parse the Records generated by TableGen to populate the SignaturesList,
+  // FctOverloadMap and TypeMap.
   void GetOverloads();
 
-  // Emit the OpenCLSignature table. This table contains all possible
-  // signatures, and is a struct OpenCLType. A signature is composed of a
-  // return type (mandatory), followed by zero or more argument types.
+  // Emit the TypeTable. This table contains all the possible types. A type can
+  // have several attributes (e.g. vector size, whether it is a pointer type,
+  // constness, ...).
+  // For example
+  //   {OCLT_Double, 2},
+  // is describing the type "Double", being a vector of 2 elements. See the
+  // struct OpenCLTypeStruct for 

[PATCH] D65454: AMDGPU: Add missing builtin declarations

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D65454



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

[PATCH] D65249: [NFC] use C++11 in AlignOf.h, remove AlignedCharArray

[JF Bastien via Phabricator via cfe-commits]

jfb closed this revision.
jfb added a comment.

https://reviews.llvm.org/rL367282


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65249



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

[PATCH] D65458: [NFC] Remove LLVM_ALIGNAS

[JF Bastien via Phabricator via cfe-commits]

jfb created this revision.
jfb added a reviewer: rnk.
Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, jkorous, 
mgorny.
Herald added projects: clang, LLVM.

The minimum compilers support all have alignas, and we don't use LLVM_ALIGNAS 
anywhere anymore. This also removes an MSVC diagnostic which, according to the 
comment above, isn't relevant anymore.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65458

Files:
  clang-tools-extra/docs/doxygen.cfg.in
  clang/docs/doxygen.cfg.in
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/docs/doxygen.cfg.in
  llvm/include/llvm/Support/Compiler.h
  llvm/unittests/Support/AlignOfTest.cpp

Index: llvm/unittests/Support/AlignOfTest.cpp
===
--- llvm/unittests/Support/AlignOfTest.cpp
+++ llvm/unittests/Support/AlignOfTest.cpp
@@ -38,10 +38,10 @@
 #endif
 
 // Define some fixed alignment types to use in these tests.
-struct LLVM_ALIGNAS(1) A1 {};
-struct LLVM_ALIGNAS(2) A2 {};
-struct LLVM_ALIGNAS(4) A4 {};
-struct LLVM_ALIGNAS(8) A8 {};
+struct alignas(1) A1 {};
+struct alignas(2) A2 {};
+struct alignas(4) A4 {};
+struct alignas(8) A8 {};
 
 struct S1 {};
 struct S2 { char a; };
Index: llvm/include/llvm/Support/Compiler.h
===
--- llvm/include/llvm/Support/Compiler.h
+++ llvm/include/llvm/Support/Compiler.h
@@ -338,14 +338,6 @@
 # define LLVM_ASSUME_ALIGNED(p, a) (p)
 #endif
 
-/// \macro LLVM_ALIGNAS
-/// Used to specify a minimum alignment for a structure or variable.
-#if __GNUC__ && !__has_feature(cxx_alignas) && !LLVM_GNUC_PREREQ(4, 8, 1)
-# define LLVM_ALIGNAS(x) __attribute__((aligned(x)))
-#else
-# define LLVM_ALIGNAS(x) alignas(x)
-#endif
-
 /// \macro LLVM_PACKED
 /// Used to specify a packed structure.
 /// LLVM_PACKED(
@@ -376,8 +368,8 @@
 
 /// \macro LLVM_PTR_SIZE
 /// A constant integer equivalent to the value of sizeof(void*).
-/// Generally used in combination with LLVM_ALIGNAS or when doing computation in
-/// the preprocessor.
+/// Generally used in combination with alignas or when doing computation in the
+/// preprocessor.
 #ifdef __SIZEOF_POINTER__
 # define LLVM_PTR_SIZE __SIZEOF_POINTER__
 #elif defined(_WIN64)
Index: llvm/docs/doxygen.cfg.in
===
--- llvm/docs/doxygen.cfg.in
+++ llvm/docs/doxygen.cfg.in
@@ -1926,7 +1926,7 @@
 # recursively expanded use the := operator instead of the = operator.
 # This tag requires that the tag ENABLE_PREPROCESSING is set to YES.
 
-PREDEFINED = LLVM_ALIGNAS(x)=
+PREDEFINED =
 
 # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then this
 # tag can be used to specify a list of macro names that should be expanded. The
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -542,14 +542,6 @@
   # is fixed.
   -wd4709 # Suppress comma operator within array index expression
 
-  # Ideally, we'd like this warning to be enabled, but MSVC 2013 doesn't
-  # support the 'aligned' attribute in the way that clang sources requires (for
-  # any code that uses the LLVM_ALIGNAS macro), so this is must be disabled to
-  # avoid unwanted alignment warnings.
-  # When we switch to requiring a version of MSVC that supports the 'alignas'
-  # specifier (MSVC 2015?) this warning can be re-enabled.
-  -wd4324 # Suppress 'structure was padded due to __declspec(align())'
-
   # Promoted warnings.
   -w14062 # Promote 'enumerator in switch of enum is not handled' to level 1 warning.
 
Index: clang/docs/doxygen.cfg.in
===
--- clang/docs/doxygen.cfg.in
+++ clang/docs/doxygen.cfg.in
@@ -1925,7 +1925,7 @@
 # recursively expanded use the := operator instead of the = operator.
 # This tag requires that the tag ENABLE_PREPROCESSING is set to YES.
 
-PREDEFINED = LLVM_ALIGNAS(x)=
+PREDEFINED =
 
 # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then this
 # tag can be used to specify a list of macro names that should be expanded. The
Index: clang-tools-extra/docs/doxygen.cfg.in
===
--- clang-tools-extra/docs/doxygen.cfg.in
+++ clang-tools-extra/docs/doxygen.cfg.in
@@ -1937,7 +1937,7 @@
 # recursively expanded use the := operator instead of the = operator.
 # This tag requires that the tag ENABLE_PREPROCESSING is set to YES.
 
-PREDEFINED = LLVM_ALIGNAS(x)=
+PREDEFINED =
 
 # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then this
 # tag can be used to specify a list of macro names that should be expanded. The
___
cfe-commi

[PATCH] D65456: [OpenCL] Add generic type handling for builtin functions

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

Main changes since D63434 :

- Rename List* to Vec*.
- Rename TLnn -> TLAll, TLInt, TLFloat.
- Apply clang-format.
- Improve/update documentation.
- Factor out renaming of base types into separate commit.
- Change return type of OCL2Qual.
- Remove default case from OCL2Qual switch statement: it should be fully 
covering the enum.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65456



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

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

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

ostannard updated this revision to Diff 212370.
ostannard marked 2 inline comments as done.
ostannard added a comment.

- Rebase
- Don't emit llvm.assume when not necessary (we already weren't checking for 
it's presence in GlobalDCE)
- s/"public"/"default"/ in IR docs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63932

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
  clang/test/CodeGenCXX/virtual-function-elimination.cpp
  clang/test/Driver/virtual-function-elimination.cpp
  llvm/docs/LangRef.rst
  llvm/docs/TypeMetadata.rst
  llvm/include/llvm/Analysis/TypeMetadataUtils.h
  llvm/include/llvm/IR/FixedMetadataKinds.def
  llvm/include/llvm/IR/GlobalObject.h
  llvm/include/llvm/Transforms/IPO/GlobalDCE.h
  llvm/lib/Analysis/TypeMetadataUtils.cpp
  llvm/lib/IR/Metadata.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/lib/Transforms/IPO/GlobalDCE.cpp
  llvm/lib/Transforms/IPO/Internalize.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/test/LTO/ARM/vcall-visibility.ll
  llvm/test/ThinLTO/X86/lazyload_metadata.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-base-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions-derived-pointer-call.ll
  llvm/test/Transforms/GlobalDCE/virtual-functions.ll
  llvm/test/Transforms/Internalize/vcall-visibility.ll

Index: llvm/test/Transforms/Internalize/vcall-visibility.ll
===
--- /dev/null
+++ llvm/test/Transforms/Internalize/vcall-visibility.ll
@@ -0,0 +1,64 @@
+; RUN: opt < %s -internalize -S | FileCheck %s
+
+%struct.A = type { i32 (...)** }
+%struct.B = type { i32 (...)** }
+%struct.C = type { i32 (...)** }
+
+; Class A has default visibility, so has no !vcall_visibility metadata before
+; or after LTO.
+; CHECK-NOT: @_ZTV1A = {{.*}}!vcall_visibility
+@_ZTV1A = dso_local unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* null, i8* bitcast (void (%struct.A*)* @_ZN1A3fooEv to i8*)] }, align 8, !type !0, !type !1
+
+; Class B has hidden visibility but public LTO visibility, so has no
+; !vcall_visibility metadata before or after LTO.
+; CHECK-NOT: @_ZTV1B = {{.*}}!vcall_visibility
+@_ZTV1B = hidden unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* null, i8* bitcast (void (%struct.B*)* @_ZN1B3fooEv to i8*)] }, align 8, !type !2, !type !3
+
+; Class C has hidden visibility, so the !vcall_visibility metadata is set to 1
+; (linkage unit) before LTO, and 2 (translation unit) after LTO.
+; CHECK: @_ZTV1C ={{.*}}!vcall_visibility [[MD_TU_VIS:![0-9]+]]
+@_ZTV1C = hidden unnamed_addr constant { [3 x i8*] } { [3 x i8*] [i8* null, i8* null, i8* bitcast (void (%struct.C*)* @_ZN1C3fooEv to i8*)] }, align 8, !type !4, !type !5, !vcall_visibility !6
+
+; Class D has translation unit visibility before LTO, and this is not changed
+; by LTO.
+; CHECK: @_ZTVN12_GLOBAL__N_11DE = {{.*}}!vcall_visibility [[MD_TU_VIS:![0-9]+]]
+@_ZTVN12_GLOBAL__N_11DE = internal unnamed_addr constant { [3 x i8*] } zeroinitializer, align 8, !type !7, !type !9, !vcall_visibility !11
+
+define dso_local void @_ZN1A3fooEv(%struct.A* nocapture %this) {
+entry:
+  ret void
+}
+
+define hidden void @_ZN1B3fooEv(%struct.B* nocapture %this) {
+entry:
+  ret void
+}
+
+define hidden void @_ZN1C3fooEv(%struct.C* nocapture %this) {
+entry:
+  ret void
+}
+
+define hidden noalias nonnull i8* @_Z6make_dv() {
+entry:
+  %call = tail call i8* @_Znwm(i64 8) #3
+  %0 = bitcast i8* %call to i32 (...)***
+  store i32 (...)** bitcast (i8** getelementptr inbounds ({ [3 x i8*] }, { [3 x i8*] }* @_ZTVN12_GLOBAL__N_11DE, i64 0, inrange i32 0, i64 2) to i32 (...)**), i32 (...)*** %0, align 8
+  ret i8* %call
+}
+
+declare dso_local noalias nonnull i8* @_Znwm(i64)
+
+; CHECK: [[MD_TU_VIS]] = !{i64 2}
+!0 = !{i64 16, !"_ZTS1A"}
+!1 = !{i64 16, !"_ZTSM1AFvvE.virtual"}
+!2 = !{i64 16, !"_ZTS1B"}
+!3 = !{i64 16, !"_ZTSM1BFvvE.virtual"}
+!4 = !{i64 16, !"_ZTS1C"}
+!5 = !{i64 16, !"_ZTSM1CFvvE.virtual"}
+!6 = !{i64 1}
+!7 = !{i64 16, !8}
+!8 = distinct !{}
+!9 = !{i64 16, !10}
+!10 = distinct !{}
+!11 = !{i64 2}
Index: llvm/test/Transforms/GlobalDCE/virtual-functions.ll
===
--- /dev/null
+++ llvm/test/Transforms/GlobalDCE/virtual-functions.ll
@@ -0,0 +1,55 @@
+; RUN: opt < %s -globaldce -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+declare dso_local noalias nonnull i8* @_Znwm(i

[clang-tools-extra] r367333 - [clang-tidy] Fix the documentation for linuxkernel-must-use-errs.

[Tom Roeder via cfe-commits]

Author: tmroeder
Date: Tue Jul 30 09:49:28 2019
New Revision: 367333

URL: http://llvm.org/viewvc/llvm-project?rev=367333&view=rev
Log:
[clang-tidy] Fix the documentation for linuxkernel-must-use-errs.

Summary:
This changes ReleaseNotes.txt to have the first sentence of the full
documentation from linuxkernel-must-use-errs.rst.

This addresses a comment from the review of rL367071 in
https://reviews.llvm.org/D59963.

Reviewers: Eugene.Zelenko

Subscribers: xazax.hun, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=367333&r1=367332&r2=367333&view=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Tue Jul 30 09:49:28 2019
@@ -71,15 +71,7 @@ Improvements to clang-tidy
   ` check.
 
   Checks Linux kernel code to see if it uses the results from the functions in
-  ``linux/err.h``. Also checks to see if code uses the results from functions 
that
-  directly return a value from one of these error functions.
-
-  This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
-  ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return
-  values must be checked, since positive pointers and negative error codes are
-  being used in the same context. These functions are marked with
-  ``__attribute__((warn_unused_result))``, but some kernel versions do not have
-  this warning enabled for clang.
+  ``linux/err.h``.
 
 - New :doc:`google-upgrade-googletest-case
   ` check.

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst?rev=367333&r1=367332&r2=367333&view=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst 
(original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst 
Tue Jul 30 09:49:28 2019
@@ -3,14 +3,16 @@
 linuxkernel-must-use-errs
 =
 
-Checks for cases where the kernel error functions ``ERR_PTR``,
-``PTR_ERR``, ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and
-``PTR_ERR_OR_ZERO`` are called but the results are not used. These
-functions are marked with ``__attribute__((warn_unused_result))``, but
-the compiler warning for this attribute is not always enabled.
+Checks Linux kernel code to see if it uses the results from the functions in
+``linux/err.h``. Also checks to see if code uses the results from functions 
that
+directly return a value from one of these error functions.
 
-This also checks for unused values returned by functions that return
-``ERR_PTR``.
+This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
+``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return
+values must be checked, since positive pointers and negative error codes are
+being used in the same context. These functions are marked with
+``__attribute__((warn_unused_result))``, but some kernel versions do not have
+this warning enabled for clang.
 
 Examples:
 


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

[PATCH] D65343: [clang-tidy] Fix the documentation for linuxkernel-must-use-errs.

[Tom Roeder via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367333: [clang-tidy] Fix the documentation for 
linuxkernel-must-use-errs. (authored by tmroeder, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65343?vs=212201&id=212373#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65343

Files:
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst


Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -71,15 +71,7 @@
   ` check.
 
   Checks Linux kernel code to see if it uses the results from the functions in
-  ``linux/err.h``. Also checks to see if code uses the results from functions 
that
-  directly return a value from one of these error functions.
-
-  This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
-  ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return
-  values must be checked, since positive pointers and negative error codes are
-  being used in the same context. These functions are marked with
-  ``__attribute__((warn_unused_result))``, but some kernel versions do not have
-  this warning enabled for clang.
+  ``linux/err.h``.
 
 - New :doc:`google-upgrade-googletest-case
   ` check.
Index: 
clang-tools-extra/trunk/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
@@ -3,14 +3,16 @@
 linuxkernel-must-use-errs
 =
 
-Checks for cases where the kernel error functions ``ERR_PTR``,
-``PTR_ERR``, ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and
-``PTR_ERR_OR_ZERO`` are called but the results are not used. These
-functions are marked with ``__attribute__((warn_unused_result))``, but
-the compiler warning for this attribute is not always enabled.
-
-This also checks for unused values returned by functions that return
-``ERR_PTR``.
+Checks Linux kernel code to see if it uses the results from the functions in
+``linux/err.h``. Also checks to see if code uses the results from functions 
that
+directly return a value from one of these error functions.
+
+This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
+``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return
+values must be checked, since positive pointers and negative error codes are
+being used in the same context. These functions are marked with
+``__attribute__((warn_unused_result))``, but some kernel versions do not have
+this warning enabled for clang.
 
 Examples:
 


Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -71,15 +71,7 @@
   ` check.
 
   Checks Linux kernel code to see if it uses the results from the functions in
-  ``linux/err.h``. Also checks to see if code uses the results from functions that
-  directly return a value from one of these error functions.
-
-  This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
-  ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return
-  values must be checked, since positive pointers and negative error codes are
-  being used in the same context. These functions are marked with
-  ``__attribute__((warn_unused_result))``, but some kernel versions do not have
-  this warning enabled for clang.
+  ``linux/err.h``.
 
 - New :doc:`google-upgrade-googletest-case
   ` check.
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
@@ -3,14 +3,16 @@
 linuxkernel-must-use-errs
 =
 
-Checks for cases where the kernel error functions ``ERR_PTR``,
-``PTR_ERR``, ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and
-``PTR_ERR_OR_ZERO`` are called but the results are not used. These
-functions are marked with ``__attribute__((warn_unused_result))``, but
-the compiler warning for this attribute is not always enabled.
-
-This also checks for unused values returned by functions that return
-``ERR_PTR``.
+Checks Linux kernel code to see if it uses the results from the functions in
+``linux/err.h``. Also checks to see if code uses the results from functions that
+d

[PATCH] D62574: Initial draft of target-configurable address spaces.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: include/clang/AST/ASTContext.h:2598
+  /// Returns true if address space A overlaps with B.
+  bool isAddressSpaceOverlapping(LangAS A, LangAS B) const {
+// A overlaps with B if either is a superset of the other.

ebevhan wrote:
> Anastasia wrote:
> > ebevhan wrote:
> > > Anastasia wrote:
> > > > Is there any advantage of keeping superset&subset concept? Amd if yes, 
> > > > how do we position the new functionality with explicit cast?
> > > > 
> > > > I think I am missing a bit conceptual view... because I think we 
> > > > originally discussed to switch to implicit/explicit conversion model. 
> > > > Perhaps there is no reason to do it but I would just like to understand 
> > > > why? 
> > > Yes, I know the original discussion was regarding the implicit/explicit 
> > > model, but I came to realize during the implementation that all that was 
> > > needed to get the superspace model to work generically with the current 
> > > behavior was an override on the explicit conversion.
> > > 
> > > The implicit/explicit model also has the drawback that it's a bit too 
> > > expressive. You can express semantics that just don't really make sense, 
> > > like permitting implicit conversion but not explicit conversion. The 
> > > superspace model doesn't let you do that, and the additions I've made 
> > > here still don't: If implicit conversion is allowed, explicit conversion 
> > > must also be allowed. It just becomes possible to allow explicit 
> > > conversion for ASes that don't overlap.
> > > 
> > > Since the superspace model is what OpenCL and Embedded-C use in their 
> > > specification, it's probably better to use that anyway.
> > > The implicit/explicit model also has the drawback that it's a bit too 
> > > expressive. You can express semantics that just don't really make sense, 
> > > like permitting implicit conversion but not explicit conversion. The 
> > > superspace model doesn't let you do that, and the additions I've made 
> > > here still don't: If implicit conversion is allowed, explicit conversion 
> > > must also be allowed. It just becomes possible to allow explicit 
> > > conversion for ASes that don't overlap.
> > 
> > Ok, I think we could define the new model something like - explicit 
> > conversions are automatically allowed for all implicit conversions... 
> > targets won't have to specify those but only extra comversions that are not 
> > allowed implicitly. 
> > 
> > Just to understand in the current patch when are we supposed to use 
> > `isAddressSpaceOverlapping` and when `isExplicitAddrSpaceConversionLegal`. 
> > Can't we just always use `isExplicitAddrSpaceConversionLegal`?
> > 
> > > 
> > > Since the superspace model is what OpenCL and Embedded-C use in their 
> > > specification, it's probably better to use that anyway.
> > 
> > I agree the advantage of following spec is really huge. However, Clang is 
> > already broken for Emdedded C isn't it? Because it allows any explicit 
> > conversions?
> > 
> > As for OpenCL it might be reasonable to provide new documentation if needed 
> > as soon as the new rules don't invalidate all behavior.
> > 
> > 
> > Ok, I think we could define the new model something like - explicit 
> > conversions are automatically allowed for all implicit conversions... 
> > targets won't have to specify those but only extra comversions that are not 
> > allowed implicitly.
> 
> Yes, this is how it's defined. Converting explicitly between two ASes where 
> either one is a superset of the other is always legal.
> 
> > Just to understand in the current patch when are we supposed to use 
> > isAddressSpaceOverlapping and when isExplicitAddrSpaceConversionLegal. 
> > Can't we just always use isExplicitAddrSpaceConversionLegal?
> 
> I guess the distinction between `isAddressSpaceOverlapping` and 
> `isExplicitAddrSpaceConversionLegal` is pretty subtle. You would want the 
> former when you need to know if **implicit conversion A->B or B->A** is 
> permitted, and the latter when you need to know if **explicit conversion 
> A->B** is permitted.
> 
> Though in most cases, I think the latter is probably the most common.
> 
> > I agree the advantage of following spec is really huge. However, Clang is 
> > already broken for Emdedded C isn't it? Because it allows any explicit 
> > conversions?
> 
> No, the current behavior of permitting all explicit conversions is according 
> to spec: "A non-null pointer into an address space A can be cast to a pointer 
> into another address space B, but such a cast is undefined if the source 
> pointer does not point to a location in B." The addition of 
> `isExplicitAddrSpaceConversionLegal` lets targets override this behavior and 
> make such casts non-legal.
I see so we are making the new rules then. I guess it should be fine. However 
we might want to document this better and explain the difference between two 
types of API provided.



=

[PATCH] D63932: [GlobalDCE] Dead Virtual Function Elimination

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

In D63932#1606248 , @ostannard wrote:

> In that example, with everything having default ELF visibility, all of the 
> vtables will get vcall_visibility public, which can't be optimised by VFE, 
> and won't ever be relaxed to one of the tighter visibilities.
>
> The cases which can be optimised are (assuming "visibility" means visibility 
> of the most-visible dynamic base class):
>
> - Classes in an anonymous namespace, which aren't visible outside their 
> translation unit. Probably doesn't happen very often, but can be optimised 
> without LTO.
> - Classes visible outside the translation unit, but not outside the LTO unit. 
> This generally means hidden ELF visibility, unless the lto_visibility_public 
> attribute is used. These can't be optimised without LTO, but can be with it.
>
>   I implemented the second case by adding the LinkageUnit visibility, which 
> can't be optimised by VFE, but can be reduced to TranslationUnit when LTO 
> internalisation happens. This could also have been done by not changing the 
> visibility at LTO time, and instead leting GlobalDCE know if it is running 
> post-LTO. Both of these should give the same results, but the way I used 
> represents the visibility fully in the IR, without having the extra state of 
> "are we doing LTO?".


Okay, I somehow missed that the relaxation to TranslationUnit was guarded on it 
being LinkageUnit to start with. So at least from that perspective I don't see 
any soundness issues. It still doesn't seem like a good idea to do the 
relaxation in LTO though, since the visibility-outside-of-LTO of the vtable 
symbols is not a reliable indicator whether the type is used externally (I 
think that's what confused me to begin with). Here are a couple of cases where 
things might go wrong:

- Take the example from my earlier message, give the "main executable" and 
"DSO" hidden visibility, build the "main executable" with LTO and the "DSO" 
without LTO, and statically link them both into the same executable. We run 
into the same problem where the `Plugin1` vtable is potentially not referenced 
and thus misoptimised. Yes, this is a violation of the LTO visibility rules, 
but the example shows that we only detect it sometimes. I think that if we did 
want to detect cases where the LTO visibility rules are clearly being violated, 
the outcome should be to issue a diagnostic and not to silently proceed with 
optimizations disabled, since the violation might be masking other undetected 
issues. That really seems orthogonal to this patch, though.
- Imagine linking part of a program with `ld -r` with LTO -- all symbols 
including vtable symbols will appear to be externally visible, which is not 
necessarily a violation of the LTO visibility rules because they may not be 
used externally in the final link. So we end up pessimising unnecessarily.

I gave this some more thought and it seems that the frontend has all of the 
information required to make a determination about whether to allow VFE, 
without needing to teach LTO to relax visibility. We can make the frontend do 
this:

- If `-flto` was passed (see `CodeGenOptions::PrepareForLTO`), attach "allow 
VFE" metadata if class has hidden LTO visibility.
- Otherwise, attach "allow VFE" metadata if class is not `isExternallyVisible`.

Now the behaviour of GlobalDCE can be made conditional on the "allow VFE" 
metadata alone. This also has the advantage of simplifying the IR by making 
"allow VFE" a boolean rather than a tri-state as it is now.




Comment at: llvm/lib/Transforms/IPO/GlobalDCE.cpp:183
+// unit, we know that we can see all virtual functions which might use it,
+// so VFE is safe.
+if (auto GO = dyn_cast(&GV)) {

ostannard wrote:
> pcc wrote:
> > Not necessarily, at least in this implementation, because "vtable symbol 
> > can be internalized" doesn't imply "all vcalls are visible". See main 
> > response.
> This is checking the vcall_visibility, which will only be 
> VCallVisibilityTranslationUnit if all base classes which contain virtual 
> functions are private to this translation unit, which does imply "all vcalls 
> are visible".
Ack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63932



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

[PATCH] D65461: [OPENMP]Add support for analysis of linear variables and step.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added a reviewer: NoQ.
Herald added a subscriber: guansong.
Herald added a project: clang.

Added support for basic analysis of the linear variables and linear step
expression. Linear loop iteration variables must be excluded from this
analysis, only non-loop iteration variables must be analyzed.


Repository:
  rC Clang

https://reviews.llvm.org/D65461

Files:
  include/clang/AST/OpenMPClause.h
  lib/AST/OpenMPClause.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/Analysis/cfg-openmp.cpp
  test/OpenMP/distribute_parallel_for_simd_linear_messages.cpp
  test/OpenMP/distribute_simd_linear_messages.cpp
  test/OpenMP/for_linear_messages.cpp
  test/OpenMP/for_simd_linear_messages.cpp
  test/OpenMP/parallel_for_linear_messages.cpp
  test/OpenMP/parallel_for_simd_linear_messages.cpp
  test/OpenMP/simd_linear_messages.cpp
  test/OpenMP/target_parallel_for_linear_messages.cpp
  test/OpenMP/target_parallel_for_simd_linear_messages.cpp
  test/OpenMP/target_simd_linear_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_linear_messages.cpp
  test/OpenMP/target_teams_distribute_simd_linear_messages.cpp
  test/OpenMP/taskloop_simd_linear_messages.cpp
  test/OpenMP/teams_distribute_parallel_for_simd_linear_messages.cpp
  test/OpenMP/teams_distribute_simd_linear_messages.cpp

Index: test/OpenMP/teams_distribute_simd_linear_messages.cpp
===
--- test/OpenMP/teams_distribute_simd_linear_messages.cpp
+++ test/OpenMP/teams_distribute_simd_linear_messages.cpp
@@ -3,6 +3,14 @@
 // RUN: %clang_cc1 -verify -fopenmp-simd %s -Wuninitialized
 
 extern int omp_default_mem_alloc;
+void xxx(int argc) {
+  int i, step; // expected-note {{initialize the variable 'step' to silence this warning}} expected-note {{initialize the variable 'i' to silence this warning}}
+#pragma omp target
+#pragma omp teams distribute simd linear(i : step) // expected-warning {{variable 'i' is uninitialized when used here}} expected-warning {{variable 'step' is uninitialized when used here}}
+  for (i = 0; i < 10; ++i)
+;
+}
+
 namespace X {
   int x;
 };
Index: test/OpenMP/teams_distribute_parallel_for_simd_linear_messages.cpp
===
--- test/OpenMP/teams_distribute_parallel_for_simd_linear_messages.cpp
+++ test/OpenMP/teams_distribute_parallel_for_simd_linear_messages.cpp
@@ -3,6 +3,14 @@
 // RUN: %clang_cc1 -verify -fopenmp-simd %s -Wuninitialized
 
 extern int omp_default_mem_alloc;
+void xxx(int argc) {
+  int i, step; // expected-note {{initialize the variable 'step' to silence this warning}} expected-note {{initialize the variable 'i' to silence this warning}}
+#pragma omp target
+#pragma omp teams distribute parallel for simd linear(i : step) // expected-warning {{variable 'i' is uninitialized when used here}} expected-warning {{variable 'step' is uninitialized when used here}}
+  for (i = 0; i < 10; ++i)
+;
+}
+
 namespace X {
   int x;
 };
Index: test/OpenMP/taskloop_simd_linear_messages.cpp
===
--- test/OpenMP/taskloop_simd_linear_messages.cpp
+++ test/OpenMP/taskloop_simd_linear_messages.cpp
@@ -12,6 +12,13 @@
 extern const omp_allocator_handle_t omp_pteam_mem_alloc;
 extern const omp_allocator_handle_t omp_thread_mem_alloc;
 
+void xxx(int argc) {
+  int i, lin, step; // expected-note {{initialize the variable 'lin' to silence this warning}} expected-note {{initialize the variable 'step' to silence this warning}}
+#pragma omp taskloop simd linear(i, lin : step) // expected-warning {{variable 'lin' is uninitialized when used here}} expected-warning {{variable 'step' is uninitialized when used here}}
+  for (i = 0; i < 10; ++i)
+;
+}
+
 namespace X {
   int x;
 };
Index: test/OpenMP/target_teams_distribute_simd_linear_messages.cpp
===
--- test/OpenMP/target_teams_distribute_simd_linear_messages.cpp
+++ test/OpenMP/target_teams_distribute_simd_linear_messages.cpp
@@ -12,6 +12,13 @@
 extern const omp_allocator_handle_t omp_pteam_mem_alloc;
 extern const omp_allocator_handle_t omp_thread_mem_alloc;
 
+void xxx(int argc) {
+  int i, step; // expected-note {{initialize the variable 'step' to silence this warning}}
+#pragma omp target teams distribute simd linear(i : step) // expected-warning {{variable 'step' is uninitialized when used here}}
+  for (i = 0; i < 10; ++i)
+;
+}
+
 namespace X {
   int x;
 };
Index: test/OpenMP/target_teams_distribute_parallel_for_simd_linear_messages.cpp
===
--- test/OpenMP/target_teams_distribute_parallel_for_simd_linear_messages.cpp
+++ test/OpenMP/target_teams_distribute_parallel_for_simd_linear_messages.cpp
@@ -12,6 +12,13 @@
 extern const omp_allocator_handle_t omp_pteam_mem_alloc;

[PATCH] D65428: Remove cache for macro arg stringization

[Vedant Kumar via Phabricator via cfe-commits]

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

Thanks, lgtm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65428



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

[PATCH] D65458: [NFC] Remove LLVM_ALIGNAS

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65458



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

[PATCH] D65426: [Coverage] Hide coverage for regions with incorrect end locations (PR39942)

[Vedant Kumar via Phabricator via cfe-commits]

vsk abandoned this revision.
vsk added a comment.

Thanks Reid!


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

https://reviews.llvm.org/D65426



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

r367337 - Remove cache for macro arg stringization

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Tue Jul 30 10:58:22 2019
New Revision: 367337

URL: http://llvm.org/viewvc/llvm-project?rev=367337&view=rev
Log:
Remove cache for macro arg stringization

Summary:
The cache recorded the wrong expansion location for all but the first
stringization. It seems uncommon to stringize the same macro argument
multiple times, so this cache doesn't seem that important.

Fixes PR39942

Reviewers: vsk, rsmith

Subscribers: cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/CoverageMapping/macro-stringize-twice.cpp
Modified:
cfe/trunk/include/clang/Lex/MacroArgs.h
cfe/trunk/lib/Lex/MacroArgs.cpp
cfe/trunk/lib/Lex/TokenLexer.cpp
cfe/trunk/unittests/Lex/LexerTest.cpp

Modified: cfe/trunk/include/clang/Lex/MacroArgs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/MacroArgs.h?rev=367337&r1=367336&r2=367337&view=diff
==
--- cfe/trunk/include/clang/Lex/MacroArgs.h (original)
+++ cfe/trunk/include/clang/Lex/MacroArgs.h Tue Jul 30 10:58:22 2019
@@ -48,10 +48,6 @@ class MacroArgs final
   /// stream.
   std::vector > PreExpArgTokens;
 
-  /// StringifiedArgs - This contains arguments in 'stringified' form.  If the
-  /// stringified form of an argument has not yet been computed, this is empty.
-  std::vector StringifiedArgs;
-
   /// ArgCache - This is a linked list of MacroArgs objects that the
   /// Preprocessor owns which we use to avoid thrashing malloc/free.
   MacroArgs *ArgCache;
@@ -94,12 +90,6 @@ public:
   const std::vector &
 getPreExpArgument(unsigned Arg, Preprocessor &PP);
 
-  /// getStringifiedArgument - Compute, cache, and return the specified 
argument
-  /// that has been 'stringified' as required by the # operator.
-  const Token &getStringifiedArgument(unsigned ArgNo, Preprocessor &PP,
-  SourceLocation ExpansionLocStart,
-  SourceLocation ExpansionLocEnd);
-
   /// getNumMacroArguments - Return the number of arguments the invoked macro
   /// expects.
   unsigned getNumMacroArguments() const { return NumMacroArgs; }

Modified: cfe/trunk/lib/Lex/MacroArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/MacroArgs.cpp?rev=367337&r1=367336&r2=367337&view=diff
==
--- cfe/trunk/lib/Lex/MacroArgs.cpp (original)
+++ cfe/trunk/lib/Lex/MacroArgs.cpp Tue Jul 30 10:58:22 2019
@@ -76,8 +76,6 @@ MacroArgs *MacroArgs::create(const Macro
 /// destroy - Destroy and deallocate the memory for this object.
 ///
 void MacroArgs::destroy(Preprocessor &PP) {
-  StringifiedArgs.clear();
-
   // Don't clear PreExpArgTokens, just clear the entries.  Clearing the entries
   // would deallocate the element vectors.
   for (unsigned i = 0, e = PreExpArgTokens.size(); i != e; ++i)
@@ -307,21 +305,3 @@ Token MacroArgs::StringifyArgument(const
   ExpansionLocStart, ExpansionLocEnd);
   return Tok;
 }
-
-/// getStringifiedArgument - Compute, cache, and return the specified argument
-/// that has been 'stringified' as required by the # operator.
-const Token &MacroArgs::getStringifiedArgument(unsigned ArgNo,
-   Preprocessor &PP,
-   SourceLocation 
ExpansionLocStart,
-   SourceLocation ExpansionLocEnd) 
{
-  assert(ArgNo < getNumMacroArguments() && "Invalid argument number!");
-  if (StringifiedArgs.empty())
-StringifiedArgs.resize(getNumMacroArguments(), {});
-
-  if (StringifiedArgs[ArgNo].isNot(tok::string_literal))
-StringifiedArgs[ArgNo] = StringifyArgument(getUnexpArgument(ArgNo), PP,
-   /*Charify=*/false,
-   ExpansionLocStart,
-   ExpansionLocEnd);
-  return StringifiedArgs[ArgNo];
-}

Modified: cfe/trunk/lib/Lex/TokenLexer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/TokenLexer.cpp?rev=367337&r1=367336&r2=367337&view=diff
==
--- cfe/trunk/lib/Lex/TokenLexer.cpp (original)
+++ cfe/trunk/lib/Lex/TokenLexer.cpp Tue Jul 30 10:58:22 2019
@@ -383,18 +383,10 @@ void TokenLexer::ExpandFunctionArguments
   SourceLocation ExpansionLocEnd =
   getExpansionLocForMacroDefLoc(Tokens[I+1].getLocation());
 
-  Token Res;
-  if (CurTok.is(tok::hash))  // Stringify
-Res = ActualArgs->getStringifiedArgument(ArgNo, PP,
- ExpansionLocStart,
- ExpansionLocEnd);
-  else {
-// 'charify': don't bother caching these.
-Res = MacroArgs::StringifyArgument(ActualArgs->getUnexpArgument(A

[PATCH] D65127: Even more warnings utilizing gsl::Owner/gsl::Pointer annotations

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:6581
+if (!Callee->getIdentifier()) {
+  auto OO = Callee->getOverloadedOperator();
+  return OO == OverloadedOperatorKind::OO_Subscript ||

xazax.hun wrote:
> If we want to relax the warnings to give more results we could extend the 
> checking of these overloaded operators for annotated types. But this would 
> imply that the user need to have the expected semantics for those types and 
> can only suppress false positives by removing some gsl:::owner/poinnter 
> annotations.
I see those options:
- Either gsl::Owner implies some specific form of those operators (and if that 
does not hold for a class, then one should not annotate it with gsl::Owner)
- or gsl::Owner only implies some specific behavior for the "gsl::Pointer 
constructed from gsl::Owner" case and everything else requires additional 
annotation
I expect that we will experiment a bit in the future to see what works well for 
real-world code.


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

https://reviews.llvm.org/D65127



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

[PATCH] D65428: Remove cache for macro arg stringization

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Thanks for looking into it as well, I would not have seen the issue without 
reviewing the test case again. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65428



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

[PATCH] D65428: Remove cache for macro arg stringization

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367337: Remove cache for macro arg stringization (authored 
by rnk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65428?vs=212261&id=212384#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65428

Files:
  cfe/trunk/include/clang/Lex/MacroArgs.h
  cfe/trunk/lib/Lex/MacroArgs.cpp
  cfe/trunk/lib/Lex/TokenLexer.cpp
  cfe/trunk/test/CoverageMapping/macro-stringize-twice.cpp
  cfe/trunk/unittests/Lex/LexerTest.cpp

Index: cfe/trunk/include/clang/Lex/MacroArgs.h
===
--- cfe/trunk/include/clang/Lex/MacroArgs.h
+++ cfe/trunk/include/clang/Lex/MacroArgs.h
@@ -48,10 +48,6 @@
   /// stream.
   std::vector > PreExpArgTokens;
 
-  /// StringifiedArgs - This contains arguments in 'stringified' form.  If the
-  /// stringified form of an argument has not yet been computed, this is empty.
-  std::vector StringifiedArgs;
-
   /// ArgCache - This is a linked list of MacroArgs objects that the
   /// Preprocessor owns which we use to avoid thrashing malloc/free.
   MacroArgs *ArgCache;
@@ -94,12 +90,6 @@
   const std::vector &
 getPreExpArgument(unsigned Arg, Preprocessor &PP);
 
-  /// getStringifiedArgument - Compute, cache, and return the specified argument
-  /// that has been 'stringified' as required by the # operator.
-  const Token &getStringifiedArgument(unsigned ArgNo, Preprocessor &PP,
-  SourceLocation ExpansionLocStart,
-  SourceLocation ExpansionLocEnd);
-
   /// getNumMacroArguments - Return the number of arguments the invoked macro
   /// expects.
   unsigned getNumMacroArguments() const { return NumMacroArgs; }
Index: cfe/trunk/test/CoverageMapping/macro-stringize-twice.cpp
===
--- cfe/trunk/test/CoverageMapping/macro-stringize-twice.cpp
+++ cfe/trunk/test/CoverageMapping/macro-stringize-twice.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s | FileCheck %s
+
+// PR39942
+
+class a;
+template  a &operator<<(b &, const char *);
+int c;
+#define d(l) l(__FILE__, __LINE__, c)
+#define COMPACT_GOOGLE_LOG_ERROR d(e)
+#define f(g, cond) cond ? (void)0 : h() & g
+#define i(j) COMPACT_GOOGLE_LOG_##j.g()
+#define k(j) f(i(j), 0)
+class e {
+public:
+  e(const char *, int, int);
+  a &g();
+};
+class h {
+public:
+  void operator&(a &);
+};
+void use_str(const char *);
+
+#define m(func)\
+  use_str(#func);  \
+  k(ERROR) << #func;   \
+  return 0; // CHECK: File 1, [[@LINE-1]]:4 -> [[@LINE-1]]:16 = (#0 - #1)
+int main() {
+  m(asdf);
+}
Index: cfe/trunk/lib/Lex/TokenLexer.cpp
===
--- cfe/trunk/lib/Lex/TokenLexer.cpp
+++ cfe/trunk/lib/Lex/TokenLexer.cpp
@@ -383,18 +383,10 @@
   SourceLocation ExpansionLocEnd =
   getExpansionLocForMacroDefLoc(Tokens[I+1].getLocation());
 
-  Token Res;
-  if (CurTok.is(tok::hash))  // Stringify
-Res = ActualArgs->getStringifiedArgument(ArgNo, PP,
- ExpansionLocStart,
- ExpansionLocEnd);
-  else {
-// 'charify': don't bother caching these.
-Res = MacroArgs::StringifyArgument(ActualArgs->getUnexpArgument(ArgNo),
-   PP, true,
-   ExpansionLocStart,
-   ExpansionLocEnd);
-  }
+  bool Charify = CurTok.is(tok::hashat);
+  const Token *UnexpArg = ActualArgs->getUnexpArgument(ArgNo);
+  Token Res = MacroArgs::StringifyArgument(
+  UnexpArg, PP, Charify, ExpansionLocStart, ExpansionLocEnd);
   Res.setFlag(Token::StringifiedInMacro);
 
   // The stringified/charified string leading space flag gets set to match
Index: cfe/trunk/lib/Lex/MacroArgs.cpp
===
--- cfe/trunk/lib/Lex/MacroArgs.cpp
+++ cfe/trunk/lib/Lex/MacroArgs.cpp
@@ -76,8 +76,6 @@
 /// destroy - Destroy and deallocate the memory for this object.
 ///
 void MacroArgs::destroy(Preprocessor &PP) {
-  StringifiedArgs.clear();
-
   // Don't clear PreExpArgTokens, just clear the entries.  Clearing the entries
   // would deallocate the element vectors.
   for (unsigned i = 0, e = PreExpArgTokens.size(); i != e; ++i)
@@ -307,21 +305,3 @@
   ExpansionLocStart, ExpansionLocEn

[PATCH] D64488: [Driver] Support -fsanitize=function on Solaris/x86

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

Something is broken between reviews. and my mailbox as I am not receiving any 
e-mails so pinging does not make any effect... no LLVM admin replied to my 
questions on this to validate whether my mail was blacklisted or similar. I 
have decided  to change server of my mailbox and workaround it.




Comment at: test/Driver/fsanitize.c:721
+// RUN: %clang -target x86_64--solaris -fsanitize=function %s -### 2>&1 | 
FileCheck %s -check-prefix=FUNCTION-SOLARIS
+// RUN: %clang -target x86_64--solaris -fsanitize=function %s -### 2>&1 | 
FileCheck %s -check-prefix=FUNCTION-SOLARIS
+// FUNCTION-SOLARIS: "-fsanitize=function"

Is twice x86_64 expected?


Repository:
  rC Clang

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

https://reviews.llvm.org/D64488



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

[PATCH] D65462: gn build: Fix check-clang-tools after r362702.

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added reviewers: hans, pcc, mbonadei.
Herald added subscribers: llvm-commits, jfb.
Herald added a reviewer: jdoerfert.
Herald added a project: LLVM.

r362702 added a test that requires clang-tidy to be linked
into libclang, so add that to the gn build.


https://reviews.llvm.org/D65462

Files:
  llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/plugin/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/plugin/BUILD.gn
  llvm/utils/gn/secondary/clang/tools/libclang/BUILD.gn


Index: llvm/utils/gn/secondary/clang/tools/libclang/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/tools/libclang/BUILD.gn
+++ llvm/utils/gn/secondary/clang/tools/libclang/BUILD.gn
@@ -39,8 +39,22 @@
 deps += [ "//clang/lib/ARCMigrate" ]
   }
 
+  defines = []
+
+  # FIXME: Once the GN build has a way to select which bits to build,
+  # only include this dependency if clang-tools-extra is part of the build.
+  # FIXME: libclang depending on anything in clang-tools-extra seems like
+  # a layering violation.
+  if (true) {
+defines += [ "CLANG_TOOL_EXTRA_BUILD" ]
+deps += [
+  "//clang-tools-extra/clang-include-fixer/plugin",
+  "//clang-tools-extra/clang-tidy/plugin",
+]
+  }
+
   if (host_os == "win") {
-defines = [ "_CINDEX_LIB_" ]
+defines += [ "_CINDEX_LIB_" ]
   }
 
   sources = [
Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/plugin/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/plugin/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/plugin/BUILD.gn
@@ -1,3 +1,5 @@
+import("//clang/lib/StaticAnalyzer/Frontend/enable.gni")
+
 static_library("plugin") {
   output_name = "clangTidyPlugin"
   configs += [ "//llvm/utils/gn/build:clang_code" ]
@@ -12,10 +14,12 @@
 "//clang-tools-extra/clang-tidy/fuchsia",
 "//clang-tools-extra/clang-tidy/google",
 "//clang-tools-extra/clang-tidy/hicpp",
+"//clang-tools-extra/clang-tidy/linuxkernel",
 "//clang-tools-extra/clang-tidy/llvm",
 "//clang-tools-extra/clang-tidy/misc",
 "//clang-tools-extra/clang-tidy/modernize",
 "//clang-tools-extra/clang-tidy/objc",
+"//clang-tools-extra/clang-tidy/openmp",
 "//clang-tools-extra/clang-tidy/performance",
 "//clang-tools-extra/clang-tidy/portability",
 "//clang-tools-extra/clang-tidy/readability",
Index: 
llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/plugin/BUILD.gn
===
--- /dev/null
+++ 
llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/plugin/BUILD.gn
@@ -0,0 +1,18 @@
+static_library("plugin") {
+  output_name = "clangIncludeFixerPlugin"
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//llvm/utils/gn/build/libs/pthread",
+"//clang-tools-extra/clang-include-fixer",
+"//clang/lib/AST",
+"//clang/lib/Basic",
+"//clang/lib/Frontend",
+"//clang/lib/Parse",
+"//clang/lib/Sema",
+"//clang/lib/Tooling",
+  ]
+
+  sources = [
+"IncludeFixerPlugin.cpp",
+  ]
+}


Index: llvm/utils/gn/secondary/clang/tools/libclang/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/tools/libclang/BUILD.gn
+++ llvm/utils/gn/secondary/clang/tools/libclang/BUILD.gn
@@ -39,8 +39,22 @@
 deps += [ "//clang/lib/ARCMigrate" ]
   }
 
+  defines = []
+
+  # FIXME: Once the GN build has a way to select which bits to build,
+  # only include this dependency if clang-tools-extra is part of the build.
+  # FIXME: libclang depending on anything in clang-tools-extra seems like
+  # a layering violation.
+  if (true) {
+defines += [ "CLANG_TOOL_EXTRA_BUILD" ]
+deps += [
+  "//clang-tools-extra/clang-include-fixer/plugin",
+  "//clang-tools-extra/clang-tidy/plugin",
+]
+  }
+
   if (host_os == "win") {
-defines = [ "_CINDEX_LIB_" ]
+defines += [ "_CINDEX_LIB_" ]
   }
 
   sources = [
Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/plugin/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/plugin/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/plugin/BUILD.gn
@@ -1,3 +1,5 @@
+import("//clang/lib/StaticAnalyzer/Frontend/enable.gni")
+
 static_library("plugin") {
   output_name = "clangTidyPlugin"
   configs += [ "//llvm/utils/gn/build:clang_code" ]
@@ -12,10 +14,12 @@
 "//clang-tools-extra/clang-tidy/fuchsia",
 "//clang-tools-extra/clang-tidy/google",
 "//clang-tools-extra/clang-tidy/hicpp",
+"//clang-tools-extra/clang-tidy/linuxkernel",
 "//clang-tools-extra/clang-tidy/llvm",
 "//clang-tools-extra/clang-tidy/misc",
 "//clang-tools-extra/clang-tidy/modernize",
 "//clang-tools-extra/clang-tidy/objc",
+"//clang

[PATCH] D65462: gn build: Fix check-clang-tools after r362702.

[Hans Wennborg via Phabricator via cfe-commits]

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

Seems good to me


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

https://reviews.llvm.org/D65462



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

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

[Ziang Wan via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaChecking.cpp:11656
+  IsListInit =
+  IsListInit || (isa(OrigE) && S.getLangOpts().CPlusPlus);
+

aaron.ballman wrote:
> ziangwan wrote:
> > aaron.ballman wrote:
> > > Do you want to perform the `isa<>` on `OrigE` or on `E` which has had 
> > > paren and implicit casts stripped?
> > When we are dealing with Initialization list syntax expression, the 
> > out-most expression will be of type `InitListExpr`. Since the out-most 
> > expression is not `ImplicitCastExpr`, `IgnoreParenImpCasts()` has no effect 
> > on the `OrigE` object. In other words, `OrigE` and `E` will be the same 
> > when `isa(OrigE)` evaluates to true.
> > 
> > In this case, I personally prefer `OrigE` since it is the "non-processed" 
> > object. However, the rest of the function uses `E` only. I can change it to 
> > `E`.
> Ah, I was thinking we had to contend with a case like `({1, 2, 3})` where 
> there's a `ParenExpr` involved, but I am not certain that's a case we have to 
> worry about.
I think `({1,2,3})` is not a valid initializer.
```
implicit-int-float-narrowing.cpp:6:16: warning: expression result unused 
[-Wunused-value]
  int a[3] = ({1,2,3});
   ^
implicit-int-float-narrowing.cpp:6:18: warning: expression result unused 
[-Wunused-value]
  int a[3] = ({1,2,3});
 ^
implicit-int-float-narrowing.cpp:6:21: error: expected ';' after expression
  int a[3] = ({1,2,3});
^
implicit-int-float-narrowing.cpp:6:7: error: array initializer must be an 
initializer list
  int a[3] = ({1,2,3});
  ^
```
If so, I don think we need to worry about there's a `ParenExpr` involved.



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

https://reviews.llvm.org/D64666



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

[PATCH] D64488: [Driver] Support -fsanitize=function on Solaris/x86

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

In D64488#1606716 , @krytarowski wrote:

> Something is broken between reviews. and my mailbox as I am not receiving any 
> e-mails so pinging does not make any effect... no LLVM admin replied to my 
> questions on this to validate whether my mail was blacklisted or similar. I 
> have decided  to change server of my mailbox and workaround it.


Maybe this is related to other reviews.llvm.org issues during the weekend where 
you couldn't update reviews with strange table full
errors?




Comment at: test/Driver/fsanitize.c:721
+// RUN: %clang -target x86_64--solaris -fsanitize=function %s -### 2>&1 | 
FileCheck %s -check-prefix=FUNCTION-SOLARIS
+// RUN: %clang -target x86_64--solaris -fsanitize=function %s -### 2>&1 | 
FileCheck %s -check-prefix=FUNCTION-SOLARIS
+// FUNCTION-SOLARIS: "-fsanitize=function"

krytarowski wrote:
> Is twice x86_64 expected?
No, one of those should be i386.  Will fix.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64488



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

[PATCH] D64564: Loop pragma parsing. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

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



Comment at: lib/Parse/ParsePragma.cpp:1011
+  Str = llvm::StringSwitch(Str)
+   .Case("loop", "clang loop " + Str.str())
+   .Case("unroll_and_jam", Str)

Meinersbur wrote:
> [serious] I know I already accepted the patch, but I just noticed something:
> `"clang loop " + Str.str()` will allocate a temporary std::string, `Str` will 
> potentially point to it, then the temporary string will be released. `Str` 
> will then point to released memory and returned by this function, i.e. a 
> use-after-free.
> 
Oopsy, thanks for spotting this! Will fix this!


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

https://reviews.llvm.org/D64564



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

[PATCH] D65462: gn build: Fix check-clang-tools after r362702.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367340: gn build: Fix check-clang-tools after r362702. 
(authored by nico, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D65462?vs=212387&id=212391#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65462

Files:
  
llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/plugin/BUILD.gn
  llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-tidy/plugin/BUILD.gn
  llvm/trunk/utils/gn/secondary/clang/tools/libclang/BUILD.gn


Index: llvm/trunk/utils/gn/secondary/clang/tools/libclang/BUILD.gn
===
--- llvm/trunk/utils/gn/secondary/clang/tools/libclang/BUILD.gn
+++ llvm/trunk/utils/gn/secondary/clang/tools/libclang/BUILD.gn
@@ -39,8 +39,22 @@
 deps += [ "//clang/lib/ARCMigrate" ]
   }
 
+  defines = []
+
+  # FIXME: Once the GN build has a way to select which bits to build,
+  # only include this dependency if clang-tools-extra is part of the build.
+  # FIXME: libclang depending on anything in clang-tools-extra seems like
+  # a layering violation.
+  if (true) {
+defines += [ "CLANG_TOOL_EXTRA_BUILD" ]
+deps += [
+  "//clang-tools-extra/clang-include-fixer/plugin",
+  "//clang-tools-extra/clang-tidy/plugin",
+]
+  }
+
   if (host_os == "win") {
-defines = [ "_CINDEX_LIB_" ]
+defines += [ "_CINDEX_LIB_" ]
   }
 
   sources = [
Index: 
llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/plugin/BUILD.gn
===
--- 
llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/plugin/BUILD.gn
+++ 
llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/plugin/BUILD.gn
@@ -0,0 +1,18 @@
+static_library("plugin") {
+  output_name = "clangIncludeFixerPlugin"
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clang-include-fixer",
+"//clang/lib/AST",
+"//clang/lib/Basic",
+"//clang/lib/Frontend",
+"//clang/lib/Parse",
+"//clang/lib/Sema",
+"//clang/lib/Tooling",
+"//llvm/utils/gn/build/libs/pthread",
+  ]
+
+  sources = [
+"IncludeFixerPlugin.cpp",
+  ]
+}
Index: 
llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-tidy/plugin/BUILD.gn
===
--- llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-tidy/plugin/BUILD.gn
+++ llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-tidy/plugin/BUILD.gn
@@ -1,3 +1,5 @@
+import("//clang/lib/StaticAnalyzer/Frontend/enable.gni")
+
 static_library("plugin") {
   output_name = "clangTidyPlugin"
   configs += [ "//llvm/utils/gn/build:clang_code" ]
@@ -12,10 +14,12 @@
 "//clang-tools-extra/clang-tidy/fuchsia",
 "//clang-tools-extra/clang-tidy/google",
 "//clang-tools-extra/clang-tidy/hicpp",
+"//clang-tools-extra/clang-tidy/linuxkernel",
 "//clang-tools-extra/clang-tidy/llvm",
 "//clang-tools-extra/clang-tidy/misc",
 "//clang-tools-extra/clang-tidy/modernize",
 "//clang-tools-extra/clang-tidy/objc",
+"//clang-tools-extra/clang-tidy/openmp",
 "//clang-tools-extra/clang-tidy/performance",
 "//clang-tools-extra/clang-tidy/portability",
 "//clang-tools-extra/clang-tidy/readability",


Index: llvm/trunk/utils/gn/secondary/clang/tools/libclang/BUILD.gn
===
--- llvm/trunk/utils/gn/secondary/clang/tools/libclang/BUILD.gn
+++ llvm/trunk/utils/gn/secondary/clang/tools/libclang/BUILD.gn
@@ -39,8 +39,22 @@
 deps += [ "//clang/lib/ARCMigrate" ]
   }
 
+  defines = []
+
+  # FIXME: Once the GN build has a way to select which bits to build,
+  # only include this dependency if clang-tools-extra is part of the build.
+  # FIXME: libclang depending on anything in clang-tools-extra seems like
+  # a layering violation.
+  if (true) {
+defines += [ "CLANG_TOOL_EXTRA_BUILD" ]
+deps += [
+  "//clang-tools-extra/clang-include-fixer/plugin",
+  "//clang-tools-extra/clang-tidy/plugin",
+]
+  }
+
   if (host_os == "win") {
-defines = [ "_CINDEX_LIB_" ]
+defines += [ "_CINDEX_LIB_" ]
   }
 
   sources = [
Index: llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/plugin/BUILD.gn
===
--- llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/plugin/BUILD.gn
+++ llvm/trunk/utils/gn/secondary/clang-tools-extra/clang-include-fixer/plugin/BUILD.gn
@@ -0,0 +1,18 @@
+static_library("plugin") {
+  output_name = "clangIncludeFixerPlugin"
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clang-include-fixer",
+"//clang/lib/AST",
+"//clang/lib/Basic",
+"//clang/lib/Frontend",
+"//clang/lib/Parse",
+"//clang/lib

[PATCH] D65462: gn build: Fix check-clang-tools after r362702.

[Mirko Bonadei via Phabricator via cfe-commits]

mbonadei added a comment.

LGTM.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65462



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

[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

Hi @hokein,
Do you have any thoughts on how to handle situation when client registers 
callback but doesn't send a request with registered ID later?




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:317
+std::lock_guard Lock(ReplyCallbacksMutex);
+ReplyCallbacks[StrID] = std::move(CB);
+  }

Should we care if there's already another callback registered for this `StrID`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65387



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

[PATCH] D65184: [Sema] Thread Safety Analysis: Fix negative capability's LockKind representation.

[Ziang Wan via Phabricator via cfe-commits]

ziangwan added a comment.

The problem is: by the current state of the thread safety analysis, 
`ASSERT_SHARED_CAPABILTIY(!mu)` introduces a shared negative capability, 
whereas `RELEASE(mu)` **and `RELEASE_SHARED(mu)`**  introduce an exclusive 
negative capability, and `UNLOCK_FUNCTION(mu)` introduces a generic negative 
capability. All three are different. At merge points, warnings will be issued 
if different types of negative capability are merged. The current thread safety 
analysis produces bogus false positive in our code base.

The solution I propose is that we should at least make `RELEASE_SHARED(mu)` 
produce a shared negative capability.

Regarding why we should have types for negative capability, thinking about 
"exclusive !mu" in a reader-writer lock situation, which means "I am not trying 
to write". However, the code can still read. In other words, "exclusive !mu" 
does not imply "shared !mu", and the code can still hold the lock in shared 
state. Without the types of negative capability, we wouldn't be able to express 
the situation described above.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65184



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

[PATCH] D65184: [Sema] Thread Safety Analysis: Fix negative capability's LockKind representation.

[Ziang Wan via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaCXX/thread-safety-annotations.h:47
+// Enable thread safety attributes only with clang.
+// The attributes can be safely erased when compiling with other compilers.
+#if defined(__clang__) && (!defined(SWIG))

aaronpuchert wrote:
> ziangwan wrote:
> > nickdesaulniers wrote:
> > > Is this test suite run with other compilers? If not, I think we can 
> > > remove the case?
> > Yeah, you are right. I just copied the header definitions from clang thread 
> > safety analysis doc.
> Why aren't you using the existing macros? The idea was to run the tests with 
> both old and new terminology, and for the time being, I think we should 
> maintain both.
There are already tests on existing macros. I want to introduce tests about the 
new macros as well.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65184



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

[PATCH] D65184: [Sema] Thread Safety Analysis: Fix negative capability's LockKind representation.

[Ziang Wan via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaCXX/warn-thread-safety-negative.cpp:135-140
+  if (condition) {
+assertNotHeld(); // expected-warning {{mutex '!mu' is acquired exclusively 
and shared in the same scope}}
+  } else {
+mu.Lock();
+mu.Unlock(); // expected-warning {{the other acquisition of mutex '!mu' is 
here}}
+  }

aaronpuchert wrote:
> ziangwan wrote:
> > aaronpuchert wrote:
> > > Why would I want these warnings here? This code seems fine to me.
> > > 
> > > However, I don't see why we don't get `acquiring mutex 'mu' requires 
> > > negative capability '!mu'` at line 138, or does that disappear because of 
> > > the assertion?
> > Showing `acquiring mutex 'mu' requires negative capability '!mu'` is not in 
> > the scope of this patch. Please notice thread safety analysis is still 
> > under development.
> > 
> > The reason is that, in one path we have `ASSERT_SHARED_CAPABILITY(!mu)`, 
> > and in the other path we have `RELEASE(mu)`. The assertion leads to 
> > negative shared capability but the release leads to negative exclusive 
> > capability. A merge of the two capabilities (merging "I am not trying to 
> > read" versus "I am not trying to write") leads to a warning.
> > 
> > Without my patch, clang will issue a warning for the merge point in test1() 
> > but not the merge point in test2().
> But `ASSERT_SHARED_CAPABILITY(!mu)` implies that we also don't have an 
> exclusive lock (an exclusive lock is stronger than a shared lock), and 
> `RELEASE(mu)` without `ACQUIRE_SHARED(mu)` implies that we have neither a 
> shared nor an exclusive lock as well.
> 
> In the end, I have the same question as above: Why do we want two kinds of 
> negative capabilities? Isn't the idea that negative capabilities express the 
> lock not being held at all?
The problem is: by the current state of the thread safety analysis, 
ASSERT_SHARED_CAPABILTIY(!mu) introduces a shared negative capability, whereas 
RELEASE(mu) and RELEASE_SHARED(mu) introduce an exclusive negative capability, 
and UNLOCK_FUNCTION(mu) introduces a generic negative capability. All three are 
different. At merge points, warnings will be issued if different types of 
negative capability are merged. The current thread safety analysis produces 
bogus false positive in our code base.

The solution I propose is that we should at least make RELEASE_SHARED(mu) 
produce a shared negative capability.

Regarding why we should have types for negative capability, thinking about 
"exclusive !mu" in a reader-writer lock situation, which means "I am not trying 
to write". However, the code can still read. In other words, "exclusive !mu" 
does not imply "shared !mu", and the code can still hold the lock in shared 
state. Without the types of negative capability, we wouldn't be able to express 
the situation described above.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65184



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

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

[Lingda Li via Phabricator via cfe-commits]

lildmh updated this revision to Diff 212399.
lildmh marked an inline comment as done.
lildmh added a comment.

Change mapper function argument checking


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

https://reviews.llvm.org/D59474

Files:
  include/clang/AST/GlobalDecl.h
  lib/AST/ASTContext.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/ModuleBuilder.cpp
  test/OpenMP/declare_mapper_codegen.cpp

Index: test/OpenMP/declare_mapper_codegen.cpp
===
--- test/OpenMP/declare_mapper_codegen.cpp
+++ test/OpenMP/declare_mapper_codegen.cpp
@@ -1,92 +1,414 @@
-///==///
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap %s
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap %s
-
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck -allow-deprecated-dag-overlap  --check-prefix SIMD-ONLY0 %s
-
 // SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
 
 // expected-no-diagnostics
 #ifndef HEADER
 #define HEADER
 
+///==///
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix CK0 --check-prefix CK0-64 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -femit-all-decls -disable-llvm-passes -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix CK0 --check-prefix CK0-64 %s
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix CK0 --check-prefix CK0-32 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %clang_cc1 -DCK0 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -femit-all-decls -disable-llvm-passes -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix CK0 --check-prefix CK0-32 %s
+
+// RUN: %clang_cc1 -DCK0 -verify -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm -femit-all-decls -disable-llvm-passes %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -DCK0 -fopenmp-simd -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -femit-all-decls -disable-llvm-passes -o %t %s
+// RUN: %cl

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Thanks, looks good.


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

https://reviews.llvm.org/D59474



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

r367345 - [NFC][clang] Refactor getCompilationPhases()+Types.def step 3.

[Puyan Lotfi via cfe-commits]

Author: zer0
Date: Tue Jul 30 12:03:17 2019
New Revision: 367345

URL: http://llvm.org/viewvc/llvm-project?rev=367345&view=rev
Log:
[NFC][clang] Refactor getCompilationPhases()+Types.def step 3.

Dropping the 'u' entry and the entire Flags table from Types.def.
Now it'll be a bit easier to tablegenify this.

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

Modified:
cfe/trunk/include/clang/Driver/Types.def
cfe/trunk/include/clang/Driver/Types.h
cfe/trunk/lib/Driver/Types.cpp

Modified: cfe/trunk/include/clang/Driver/Types.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Types.def?rev=367345&r1=367344&r2=367345&view=diff
==
--- cfe/trunk/include/clang/Driver/Types.def (original)
+++ cfe/trunk/include/clang/Driver/Types.def Tue Jul 30 12:03:17 2019
@@ -29,76 +29,73 @@
 // The fourth value is the suffix to use when creating temporary files
 // of this type, or null if unspecified.
 
-// The fifth value is a string containing option flags. Valid values:
-//  u - The type can be user specified (with -x).
-
-// The sixth value is a variadic list of phases for each type. Eventually the
+// The final value is a variadic list of phases for each type. Eventually the
 // options flag string will be replaced with this variadic list.
 // Most of the options in Flags have been removed in favor of subsuming their
 // meaning from the phases list.
 
 // C family source language (with and without preprocessing).
-TYPE("cpp-output",   PP_C, INVALID, "i", "u", 
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("c",C,PP_C,"c", "u", 
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
-TYPE("cl",   CL,   PP_C,"cl","u", 
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
-TYPE("cuda-cpp-output",  PP_CUDA,  INVALID, "cui",   "u", 
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("cuda", CUDA, PP_CUDA, "cu","u", 
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
-TYPE("cuda", CUDA_DEVICE,  PP_CUDA, "cu","" , 
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
-TYPE("hip-cpp-output",   PP_HIP,   INVALID, "cui",   "u", 
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("hip",  HIP,  PP_HIP,  "cu","u", 
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
-TYPE("hip",  HIP_DEVICE,   PP_HIP,  "cu","" , 
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
-TYPE("objective-c-cpp-output",   PP_ObjC,  INVALID, "mi","u", 
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("objc-cpp-output",  PP_ObjC_Alias, INVALID,"mi","u", 
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("objective-c",  ObjC, PP_ObjC, "m", "u", 
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
-TYPE("c++-cpp-output",   PP_CXX,   INVALID, "ii","u", 
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("c++",  CXX,  PP_CXX,  "cpp",   "u", 
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
-TYPE("objective-c++-cpp-output", PP_ObjCXX,INVALID, "mii",   "u", 
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("objc++-cpp-output",PP_ObjCXX_Alias, INVALID,  "mii",   "u", 
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("objective-c++",ObjCXX,   PP_ObjCXX,   "mm","u", 
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
-TYPE("renderscript", RenderScript, PP_C,"rs","u", 
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
+TYPE("cpp-output",   PP_C, INVALID, "i",  
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
+TYPE("c",C,PP_C,"c",  
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
+TYPE("cl",   CL,   PP_C,"cl", 
phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, 
phases::Link)
+TYPE("cuda-cpp-output",  PP_CUDA,  INVALID, "cui",
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
+TYPE("cu