[clang-tools-extra] r370843 - [clangd] Split Preamble.h out of ClangdUnit.h. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Sep  4 00:35:00 2019
New Revision: 370843

URL: http://llvm.org/viewvc/llvm-project?rev=370843&view=rev
Log:
[clangd] Split Preamble.h out of ClangdUnit.h. NFC

Summary:
Add comment describing use of preamble in clangd.
Remove deps on ClangdUnit.h where possible.

Subscribers: mgorny, ilya-biryukov, javed.absar, MaskRay, jkorous, mgrang, 
arphaman, kadircet, cfe-commits

Tags: #clang

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

Added:
clang-tools-extra/trunk/clangd/Preamble.cpp
clang-tools-extra/trunk/clangd/Preamble.h
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/clangd/Compiler.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/clangd/unittests/TUSchedulerTests.cpp

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=370843&r1=370842&r2=370843&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Wed Sep  4 00:35:00 2019
@@ -62,6 +62,7 @@ add_clang_library(clangDaemon
   Logger.cpp
   Protocol.cpp
   Quality.cpp
+  Preamble.cpp
   RIFF.cpp
   Selection.cpp
   SemanticHighlighting.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=370843&r1=370842&r2=370843&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Sep  4 00:35:00 2019
@@ -13,6 +13,7 @@
 #include "Format.h"
 #include "FormattedString.h"
 #include "Headers.h"
+#include "Preamble.h"
 #include "Protocol.h"
 #include "SemanticHighlighting.h"
 #include "SourceCode.h"

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=370843&r1=370842&r2=370843&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Wed Sep  4 00:35:00 2019
@@ -53,13 +53,6 @@ namespace clang {
 namespace clangd {
 namespace {
 
-bool compileCommandsAreEqual(const tooling::CompileCommand &LHS,
- const tooling::CompileCommand &RHS) {
-  // We don't check for Output, it should not matter to clangd.
-  return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename &&
- llvm::makeArrayRef(LHS.CommandLine).equals(RHS.CommandLine);
-}
-
 template  std::size_t getUsedBytes(const std::vector &Vec) {
   return Vec.capacity() * sizeof(T);
 }
@@ -105,10 +98,9 @@ private:
   std::vector TopLevelDecls;
 };
 
-// CollectMainFileMacroExpansions and CollectMainFileMacros are two different
-// classes as CollectMainFileMacroExpansions is only used when building the AST
-// for the main file. CollectMainFileMacros is only used when building the
-// preamble.
+// This collects macro expansions in the main file.
+// (Contrast with CollectMainFileMacros in Preamble.cpp, which collects macro
+// *definitions* in the preamble region of the main file).
 class CollectMainFileMacroExpansions : public PPCallbacks {
   const SourceManager &SM;
   std::vector &MainFileMacroLocs;
@@ -127,83 +119,6 @@ public:
   }
 };
 
-class CollectMainFileMacros : public PPCallbacks {
-public:
-  explicit CollectMainFileMacros(const SourceManager &SM,
- std::vector *Out)
-  : SM(SM), Out(Out) {}
-
-  void FileChanged(SourceLocation Loc, FileChangeReason,
-   SrcMgr::CharacteristicKind, FileID Prev) {
-InMainFile = SM.isWrittenInMainFile(Loc);
-  }
-
-  void MacroDefined(const Token &MacroName, const MacroDirective *MD) {
-if (InMainFile)
-  MainFileMacros.insert(MacroName.getIdentifierInfo()->getName());
-  }
-
-  void EndOfMainFile() {
-for (const auto &Entry : MainFileMacros)
-  Out->push_back(Entry.getKey());
-llvm::sort(*Out);
-  }
-
-private:
-  const SourceManager &SM;
-  bool InMainFile = true;
-  llvm::StringSet<> MainFileMacros;
-  std::vector *Out;
-};
-
-class CppFilePreambleCallbacks : public PreambleCallbacks {
-public:
-  CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
-  : File(File), ParsedCallback(ParsedCallback) {
-  }
-
-  IncludeStructure takeIncludes() { return std::move(Includes); }
-
-  std::vector takeMainFileMa

[PATCH] D67117: [clangd] Split Preamble.h out of ClangdUnit.h. NFC

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370843: [clangd] Split Preamble.h out of ClangdUnit.h. NFC 
(authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67117?vs=218493&id=218603#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67117

Files:
  clang-tools-extra/trunk/clangd/CMakeLists.txt
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdUnit.cpp
  clang-tools-extra/trunk/clangd/ClangdUnit.h
  clang-tools-extra/trunk/clangd/CodeComplete.cpp
  clang-tools-extra/trunk/clangd/CodeComplete.h
  clang-tools-extra/trunk/clangd/Compiler.h
  clang-tools-extra/trunk/clangd/Preamble.cpp
  clang-tools-extra/trunk/clangd/Preamble.h
  clang-tools-extra/trunk/clangd/TUScheduler.cpp
  clang-tools-extra/trunk/clangd/TUScheduler.h
  clang-tools-extra/trunk/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/trunk/clangd/TUScheduler.h
===
--- clang-tools-extra/trunk/clangd/TUScheduler.h
+++ clang-tools-extra/trunk/clangd/TUScheduler.h
@@ -9,7 +9,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TUSCHEDULER_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TUSCHEDULER_H
 
-#include "ClangdUnit.h"
+#include "Compiler.h"
 #include "Diagnostics.h"
 #include "Function.h"
 #include "GlobalCompilationDatabase.h"
@@ -24,6 +24,8 @@
 
 namespace clang {
 namespace clangd {
+class ParsedAST;
+struct PreambleData;
 
 /// Returns a number of a default async threads to use for TUScheduler.
 /// Returned value is always >= 1 (i.e. will not cause requests to be processed
Index: clang-tools-extra/trunk/clangd/Preamble.h
===
--- clang-tools-extra/trunk/clangd/Preamble.h
+++ clang-tools-extra/trunk/clangd/Preamble.h
@@ -0,0 +1,88 @@
+//===--- Preamble.h - Reusing expensive parts of the AST -*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// The vast majority of code in a typical translation unit is in the headers
+// included at the top of the file.
+//
+// The preamble optimization says that we can parse this code once, and reuse
+// the result multiple times. The preamble is invalidated by changes to the
+// code in the preamble region, to the compile command, or to files on disk.
+//
+// This is the most important optimization in clangd: it allows operations like
+// code-completion to have sub-second latency. It is supported by the
+// PrecompiledPreamble functionality in clang, which wraps the techniques used
+// by PCH files, modules etc into a convenient interface.
+//
+//===--===//
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PREAMBLE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PREAMBLE_H
+
+#include "Compiler.h"
+#include "Diagnostics.h"
+#include "FS.h"
+#include "Headers.h"
+#include "index/CanonicalIncludes.h"
+#include "clang/Frontend/PrecompiledPreamble.h"
+#include "clang/Tooling/CompilationDatabase.h"
+
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+
+/// The parsed preamble and associated data.
+///
+/// As we must avoid re-parsing the preamble, any information that can only
+/// be obtained during parsing must be eagerly captured and stored here.
+struct PreambleData {
+  PreambleData(PrecompiledPreamble Preamble, std::vector Diags,
+   IncludeStructure Includes,
+   std::vector MainFileMacros,
+   std::unique_ptr StatCache,
+   CanonicalIncludes CanonIncludes);
+
+  tooling::CompileCommand CompileCommand;
+  PrecompiledPreamble Preamble;
+  std::vector Diags;
+  // Processes like code completions and go-to-definitions will need #include
+  // information, and their compile action skips preamble range.
+  IncludeStructure Includes;
+  // Macros defined in the preamble section of the main file.
+  // Users care about headers vs main-file, not preamble vs non-preamble.
+  // These should be treated as main-file entities e.g. for code completion.
+  std::vector MainFileMacros;
+  // Cache of FS operations performed when building the preamble.
+  // When reusing a preamble, this cache can be consumed to save IO.
+  std::unique_ptr StatCache;
+  CanonicalIncludes CanonIncludes;
+};
+
+using PreambleParsedCallback =
+std::function,
+   const CanonicalIncludes &)>;
+
+/// Build a preamble for the new inputs unless an old one can be reused.
+/// If \p OldPreamble can be reused, it is returned unchanged.
+/// If \p Ol

r370850 - Re-commit r363191 "[MS] Pretend constexpr variable template specializations are inline"

[Hans Wennborg via cfe-commits]

Author: hans
Date: Wed Sep  4 01:19:30 2019
New Revision: 370850

URL: http://llvm.org/viewvc/llvm-project?rev=370850&view=rev
Log:
Re-commit r363191 "[MS] Pretend constexpr variable template specializations are 
inline"

While the next Visual Studio update (16.3) will fix this issue, that hasn't
shipped yet. Until then Clang wouldn't work with MSVC's headers which seems
unfortunate. Let's keep this in until VS 16.3 ships. (See also PR42843.)

> Fixes link errors with clang and the latest Visual C++ 14.21.27702
> headers, which was reported as PR42027.
>
> I chose to intentionally make these things linkonce_odr, i.e.
> discardable, so that we don't emit definitions of these things in every
> translation unit that includes STL headers.
>
> Note that this is *not* what MSVC does: MSVC has not yet implemented C++
> DR2387, so they emit fully specialized constexpr variable templates with
> static / internal linkage.
>
> Reviewers: rsmith
>
> Differential Revision: https://reviews.llvm.org/D63175

Added:
cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp
Modified:
cfe/trunk/lib/AST/ASTContext.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=370850&r1=370849&r2=370850&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Sep  4 01:19:30 2019
@@ -9905,10 +9905,25 @@ static GVALinkage basicGVALinkageForVari
 return StrongLinkage;
 
   case TSK_ExplicitSpecialization:
-return Context.getTargetInfo().getCXXABI().isMicrosoft() &&
-   VD->isStaticDataMember()
-   ? GVA_StrongODR
-   : StrongLinkage;
+if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+  // If this is a fully specialized constexpr variable template, pretend it
+  // was marked inline. MSVC 14.21.27702 headers define _Is_integral in a
+  // header this way, and we don't want to emit non-discardable definitions
+  // of these variables in every TU that includes . This
+  // behavior is non-conforming, since another TU could use an extern
+  // template declaration for this variable, but for constexpr variables,
+  // it's unlikely for a user to want to do that. This behavior can be
+  // removed if the headers change to explicitly mark such variable 
template
+  // specializations inline.
+  if (isa(VD) && VD->isConstexpr())
+return GVA_DiscardableODR;
+
+  // Use ODR linkage for static data members of fully specialized templates
+  // to prevent duplicate definition errors with MSVC.
+  if (VD->isStaticDataMember())
+return GVA_StrongODR;
+}
+return StrongLinkage;
 
   case TSK_ExplicitInstantiationDefinition:
 return GVA_StrongODR;

Added: cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp?rev=370850&view=auto
==
--- cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp Wed Sep  4 01:19:30 
2019
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -emit-llvm -triple=x86_64-windows-msvc -fms-compatibility 
%s -o - | FileCheck %s
+
+template  constexpr bool _Is_integer = false;
+template <> constexpr bool _Is_integer = true;
+template <> constexpr bool _Is_integer = false;
+extern "C" const bool *escape = &_Is_integer;
+
+// CHECK: @"??$_Is_integer@H@@3_NB" = linkonce_odr dso_local constant i8 1, 
comdat, align 1
+//   Should not emit _Is_integer, since it's not referenced.
+// CHECK-NOT: @"??$_Is_integer@D@@3_NB"
+// CHECK: @escape = dso_local global i8* @"??$_Is_integer@H@@3_NB", align 8


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

[PATCH] D65527: Avoid assemble step in verbose-output-quoting.c

[Hans Wennborg via Phabricator via cfe-commits]

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

Looks good to me. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65527



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

Re: r364476 - Revert r363191 "[MS] Pretend constexpr variable template specializations are inline"

[Hans Wennborg via cfe-commits]

Re-committed in r370850 for PR42843.

On Wed, Jun 26, 2019 at 11:16 PM Reid Kleckner via cfe-commits
 wrote:
>
> Author: rnk
> Date: Wed Jun 26 14:16:51 2019
> New Revision: 364476
>
> URL: http://llvm.org/viewvc/llvm-project?rev=364476&view=rev
> Log:
> Revert r363191 "[MS] Pretend constexpr variable template specializations are 
> inline"
>
> The next Visual Studio update will fix this issue, and it doesn't make
> sense to implement this non-conforming behavior going forward.
>
> Removed:
> cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp
> Modified:
> cfe/trunk/lib/AST/ASTContext.cpp
>
> Modified: cfe/trunk/lib/AST/ASTContext.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=364476&r1=364475&r2=364476&view=diff
> ==
> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
> +++ cfe/trunk/lib/AST/ASTContext.cpp Wed Jun 26 14:16:51 2019
> @@ -9809,25 +9809,10 @@ static GVALinkage basicGVALinkageForVari
>  return StrongLinkage;
>
>case TSK_ExplicitSpecialization:
> -if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
> -  // If this is a fully specialized constexpr variable template, pretend 
> it
> -  // was marked inline. MSVC 14.21.27702 headers define _Is_integral in a
> -  // header this way, and we don't want to emit non-discardable 
> definitions
> -  // of these variables in every TU that includes . This
> -  // behavior is non-conforming, since another TU could use an extern
> -  // template declaration for this variable, but for constexpr variables,
> -  // it's unlikely for a user to want to do that. This behavior can be
> -  // removed if the headers change to explicitly mark such variable 
> template
> -  // specializations inline.
> -  if (isa(VD) && VD->isConstexpr())
> -return GVA_DiscardableODR;
> -
> -  // Use ODR linkage for static data members of fully specialized 
> templates
> -  // to prevent duplicate definition errors with MSVC.
> -  if (VD->isStaticDataMember())
> -return GVA_StrongODR;
> -}
> -return StrongLinkage;
> +return Context.getTargetInfo().getCXXABI().isMicrosoft() &&
> +   VD->isStaticDataMember()
> +   ? GVA_StrongODR
> +   : StrongLinkage;
>
>case TSK_ExplicitInstantiationDefinition:
>  return GVA_StrongODR;
>
> Removed: cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp?rev=364475&view=auto
> ==
> --- cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp (removed)
> @@ -1,11 +0,0 @@
> -// RUN: %clang_cc1 -emit-llvm -triple=x86_64-windows-msvc -fms-compatibility 
> %s -o - | FileCheck %s
> -
> -template  constexpr bool _Is_integer = false;
> -template <> constexpr bool _Is_integer = true;
> -template <> constexpr bool _Is_integer = false;
> -extern "C" const bool *escape = &_Is_integer;
> -
> -// CHECK: @"??$_Is_integer@H@@3_NB" = linkonce_odr dso_local constant i8 1, 
> comdat, align 1
> -//   Should not emit _Is_integer, since it's not referenced.
> -// CHECK-NOT: @"??$_Is_integer@D@@3_NB"
> -// CHECK: @escape = dso_local global i8* @"??$_Is_integer@H@@3_NB", align 8
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66637: [clangd] Support multifile edits as output of Tweaks

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:647
+  WE.changes.emplace();
+  auto FS = FSProvider.getFileSystem();
+  for (const auto &It : R->ApplyEdits) {

please pull out a method for this part
e.g. `Error validateEdits(...)`



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:654
+  if (!It.second.canApplyTo(*Draft))
+return Reply((It.first() + " needs to be saved").str());
+}

As these are absolute paths, we probably want to invert the error message for 
readability, and at least count the files.

e.g. 
"Files must be saved first: path/to/Foo.cpp (and 3 others)"



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:414
+  if (llvm::Error Err = reformatEdit(E, Style)) {
+llvm::handleAllErrors(std::move(Err), [&](llvm::ErrorInfoBase &EIB) {
+  elog("Failed to format {0}: {1}", It.first(), EIB.message());

isn't this just
`elog("Failed to format {0}: {1}", std::move(Err))`



Comment at: clang-tools-extra/clangd/SourceCode.cpp:856
+
+bool Edit::canApplyTo(llvm::StringRef Code) const {
+  auto LHS = llvm::MemoryBuffer::getMemBuffer(Code);

documentation



Comment at: clang-tools-extra/clangd/SourceCode.cpp:870
+
+  // We only allow trailing empty lines.
+  while (!LHSIt.is_at_eof()) {

why?



Comment at: clang-tools-extra/clangd/SourceCode.h:42
 
+/// A set of edits generated for a single file. Can verify whether it is safe 
to
+/// apply these edits to a code block.

move this class near the related code? (`replacementToEdit` etc)



Comment at: clang-tools-extra/clangd/refactor/Tweak.cpp:90
+  assert(FE && "Generating edits for unknown file!");
+  llvm::StringRef FilePath = FE->getName();
+  Edit Ed(SM.getBufferData(FID), std::move(Replacements));

how do you know this is the correct/absolute path for the file? Can we add 
tests?



Comment at: clang-tools-extra/clangd/refactor/Tweak.h:73
 llvm::Optional ShowMessage;
-/// An edit to apply to the input file.
-llvm::Optional ApplyEdit;
+/// A mapping from absolute file path to edits.
+llvm::StringMap ApplyEdits;

is it possible to be more specific than "absolute"?



Comment at: clang-tools-extra/clangd/refactor/Tweak.h:130
+/// pointed by FID.
+Tweak::Effect fileEdit(const SourceManager &SM, FileID FID,
+   tooling::Replacements Replacements);

what will this be used for?

You can use at most one of these Effect factories (unless we're going to do 
some convoluted merge thing), so this seems useful if you've got exactly one 
edit to a non-main file.

It seems more useful to return a pair which could be inserted 
into a map



Comment at: clang-tools-extra/clangd/refactor/Tweak.h:130
+/// pointed by FID.
+Tweak::Effect fileEdit(const SourceManager &SM, FileID FID,
+   tooling::Replacements Replacements);

sammccall wrote:
> what will this be used for?
> 
> You can use at most one of these Effect factories (unless we're going to do 
> some convoluted merge thing), so this seems useful if you've got exactly one 
> edit to a non-main file.
> 
> It seems more useful to return a pair which could be inserted 
> into a map
why are these moved out of the Effect class?



Comment at: clang-tools-extra/clangd/refactor/Tweak.h:133
+
+/// Convinience wrapper around above.
+Tweak::Effect mainFileEdit(const SourceManager &SM,

nit: convenience


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66637



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

[PATCH] D64480: [ASTImporter] Added visibility context check for TypedefNameDecl.

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

balazske updated this revision to Diff 218610.
balazske added a comment.

- Added comment about repeated enums in C99.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64480

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterVisibilityTest.cpp

Index: clang/unittests/AST/ASTImporterVisibilityTest.cpp
===
--- clang/unittests/AST/ASTImporterVisibilityTest.cpp
+++ clang/unittests/AST/ASTImporterVisibilityTest.cpp
@@ -39,6 +39,10 @@
   using DeclTy = EnumDecl;
   BindableMatcher operator()() { return enumDecl(hasName("E")); }
 };
+struct GetTypedefNamePattern {
+  using DeclTy = TypedefNameDecl;
+  BindableMatcher operator()() { return typedefNameDecl(hasName("T")); }
+};
 
 // Values for the value-parameterized test fixtures.
 // FunctionDecl:
@@ -55,6 +59,11 @@
 // EnumDecl:
 const auto *ExternE = "enum E {};";
 const auto *AnonE = "namespace { enum E {}; }";
+// TypedefNameDecl:
+const auto *ExternTypedef = "typedef int T;";
+const auto *AnonTypedef = "namespace { typedef int T; }";
+const auto *ExternUsing = "using T = int;";
+const auto *AnonUsing = "namespace { using T = int; }";
 
 // First value in tuple: Compile options.
 // Second value in tuple: Source code to be used in the test.
@@ -243,6 +252,7 @@
 using ImportVariablesVisibility = ImportVisibility;
 using ImportClassesVisibility = ImportVisibility;
 using ImportEnumsVisibility = ImportVisibility;
+using ImportTypedefNameVisibility = ImportVisibility;
 
 // FunctionDecl.
 TEST_P(ImportFunctionsVisibility, ImportAfter) {
@@ -272,6 +282,13 @@
 TEST_P(ImportEnumsVisibility, ImportAfterImport) {
   TypedTest_ImportAfterImportWithMerge();
 }
+// TypedefNameDecl.
+TEST_P(ImportTypedefNameVisibility, ImportAfter) {
+  TypedTest_ImportAfterWithMerge();
+}
+TEST_P(ImportTypedefNameVisibility, ImportAfterImport) {
+  TypedTest_ImportAfterImportWithMerge();
+}
 
 const bool ExpectLink = true;
 const bool ExpectNotLink = false;
@@ -318,6 +335,30 @@
   std::make_tuple(ExternE, AnonE, ExpectNotLink),
   std::make_tuple(AnonE, ExternE, ExpectNotLink),
   std::make_tuple(AnonE, AnonE, ExpectNotLink))), );
+INSTANTIATE_TEST_CASE_P(
+ParameterizedTests, ImportTypedefNameVisibility,
+::testing::Combine(
+DefaultTestValuesForRunOptions,
+::testing::Values(
+std::make_tuple(ExternTypedef, ExternTypedef, ExpectLink),
+std::make_tuple(ExternTypedef, AnonTypedef, ExpectNotLink),
+std::make_tuple(AnonTypedef, ExternTypedef, ExpectNotLink),
+std::make_tuple(AnonTypedef, AnonTypedef, ExpectNotLink),
+
+std::make_tuple(ExternUsing, ExternUsing, ExpectLink),
+std::make_tuple(ExternUsing, AnonUsing, ExpectNotLink),
+std::make_tuple(AnonUsing, ExternUsing, ExpectNotLink),
+std::make_tuple(AnonUsing, AnonUsing, ExpectNotLink),
+
+std::make_tuple(ExternUsing, ExternTypedef, ExpectLink),
+std::make_tuple(ExternUsing, AnonTypedef, ExpectNotLink),
+std::make_tuple(AnonUsing, ExternTypedef, ExpectNotLink),
+std::make_tuple(AnonUsing, AnonTypedef, ExpectNotLink),
+
+std::make_tuple(ExternTypedef, ExternUsing, ExpectLink),
+std::make_tuple(ExternTypedef, AnonUsing, ExpectNotLink),
+std::make_tuple(AnonTypedef, ExternUsing, ExpectNotLink),
+std::make_tuple(AnonTypedef, AnonUsing, ExpectNotLink))), );
 
 } // end namespace ast_matchers
 } // end namespace clang
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -932,6 +932,27 @@
   EllipsisLoc);
 }
 
+template 
+bool ASTNodeImporter::hasSameVisibilityContext(T *Found, T *From) {
+  if (From->hasExternalFormalLinkage())
+return Found->hasExternalFormalLinkage();
+  if (Importer.GetFromTU(Found) != From->getTranslationUnitDecl())
+return false;
+  if (From->isInAnonymousNamespace())
+return Found->isInAnonymousNamespace();
+  else
+return !Found->isInAnonymousNamespace() &&
+   !Found->hasExternalFormalLinkage();
+}
+
+template <>
+bool ASTNodeImporter::hasSameVisibilityContext(TypedefNameDecl *Found,
+   TypedefNameDecl *From) {
+  if (From->isInAnonymousNamespace() && Found->isInAnonymousNamespace())
+return Importer.GetFromTU(Found) == From->getTranslationUnitDecl();
+  return From->isInAnonymousNamespace() == Found->isInAnonymousNamespace();
+}
+
 } // namespace clang
 
 //
@@ -2344,6 +2365,9 @@
   // If this typedef is not in block scope, determine whether we've
   // seen a typedef with the same name (that we ca

[PATCH] D67135: [clang-tidy] performance-inefficient-vector-operation: Support proto repeated field

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

I'm not sure this is the correct approach.
I'd think the check instead should be parametrized, so this patch should become 
just extending the config.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67135



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

[PATCH] D67149: Fix argument order for BugType instation for

[David selivanov via Phabricator via cfe-commits]

rtmpdavid created this revision.
rtmpdavid added a reviewer: MTC.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There is a mixup in this specific one, name and category are in incorrect order


Repository:
  rC Clang

https://reviews.llvm.org/D67149

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


Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -567,7 +567,7 @@
 
   if (!BT_Overlap)
 BT_Overlap.reset(new BugType(Filter.CheckNameCStringBufferOverlap,
- categories::UnixAPI, "Improper arguments"));
+ "Improper arguments", categories::UnixAPI));
 
   // Generate a report for this bug.
   auto report = std::make_unique(


Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -567,7 +567,7 @@
 
   if (!BT_Overlap)
 BT_Overlap.reset(new BugType(Filter.CheckNameCStringBufferOverlap,
- categories::UnixAPI, "Improper arguments"));
+ "Improper arguments", categories::UnixAPI));
 
   // Generate a report for this bug.
   auto report = std::make_unique(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r370862 - [clangd] Rename ClangdUnit.h -> ParsedAST.h. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Sep  4 02:46:06 2019
New Revision: 370862

URL: http://llvm.org/viewvc/llvm-project?rev=370862&view=rev
Log:
[clangd] Rename ClangdUnit.h -> ParsedAST.h. NFC

This much better reflects what is (now) in this header.
Maybe a rename to ParsedTU would be an improvement, but that's a much
more invasive change and life is too short.

ClangdUnit is dead, long live ClangdUnitTests!

Added:
clang-tools-extra/trunk/clangd/ParsedAST.cpp
clang-tools-extra/trunk/clangd/ParsedAST.h
clang-tools-extra/trunk/clangd/unittests/ParsedASTTests.cpp
Removed:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/FindSymbols.cpp
clang-tools-extra/trunk/clangd/Preamble.cpp
clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
clang-tools-extra/trunk/clangd/SemanticHighlighting.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/XRefs.h
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/BackgroundRebuild.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/clangd/refactor/Rename.cpp
clang-tools-extra/trunk/clangd/refactor/Tweak.h
clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractFunction.cpp
clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
clang-tools-extra/trunk/clangd/refactor/tweaks/RawStringLiteral.cpp
clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp
clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt
clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/trunk/clangd/unittests/ExpectedTypeTest.cpp
clang-tools-extra/trunk/clangd/unittests/FileIndexTests.cpp
clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp
clang-tools-extra/trunk/clangd/unittests/SymbolInfoTests.cpp
clang-tools-extra/trunk/clangd/unittests/TUSchedulerTests.cpp
clang-tools-extra/trunk/clangd/unittests/TestTU.h
clang-tools-extra/trunk/clangd/unittests/TypeHierarchyTests.cpp
clang-tools-extra/trunk/clangd/unittests/XRefsTests.cpp

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=370862&r1=370861&r2=370862&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Wed Sep  4 02:46:06 2019
@@ -39,7 +39,6 @@ add_clang_library(clangDaemon
   Cancellation.cpp
   ClangdLSPServer.cpp
   ClangdServer.cpp
-  ClangdUnit.cpp
   CodeComplete.cpp
   CodeCompletionStrings.cpp
   Compiler.cpp
@@ -62,6 +61,7 @@ add_clang_library(clangDaemon
   Logger.cpp
   Protocol.cpp
   Quality.cpp
+  ParsedAST.cpp
   Preamble.cpp
   RIFF.cpp
   Selection.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=370862&r1=370861&r2=370862&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Sep  4 02:46:06 2019
@@ -7,12 +7,12 @@
 //===---===//
 
 #include "ClangdServer.h"
-#include "ClangdUnit.h"
 #include "CodeComplete.h"
 #include "FindSymbols.h"
 #include "Format.h"
 #include "FormattedString.h"
 #include "Headers.h"
+#include "ParsedAST.h"
 #include "Preamble.h"
 #include "Protocol.h"
 #include "SemanticHighlighting.h"

Removed: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=370861&view=auto
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp (removed)
@@ -1,574 +0,0 @@
-//===--- ClangdUnit.cpp --*- 
C++-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "ClangdUnit.h"
-#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
-#include "../clang-tidy/ClangTidyModuleRegistry.h"
-#include "AST.h"
-#include "Compiler.h"
-#include "Diagnostics.h"
-#include "Headers.h"
-#include "IncludeFixer.h"
-#include "Logger.h"
-#include "So

[PATCH] D66951: [ASTImporter] Add comprehensive tests for ODR violation handling strategies

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 218625.
martong added a comment.

- Fix wrong file comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66951

Files:
  clang/unittests/AST/ASTImporterODRStrategiesTest.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  clang/unittests/AST/CMakeLists.txt

Index: clang/unittests/AST/CMakeLists.txt
===
--- clang/unittests/AST/CMakeLists.txt
+++ clang/unittests/AST/CMakeLists.txt
@@ -11,6 +11,7 @@
   ASTImporterFixtures.cpp
   ASTImporterTest.cpp
   ASTImporterGenericRedeclTest.cpp
+  ASTImporterODRStrategiesTest.cpp
   ASTImporterVisibilityTest.cpp
   ASTTraverserTest.cpp
   ASTTypeTraitsTest.cpp
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -5409,187 +5409,6 @@
   EXPECT_EQ(ImportedX->isAggregate(), FromX->isAggregate());
 }
 
-struct ConflictingDeclsWithLiberalStrategy : ASTImporterOptionSpecificTestBase {
-  ConflictingDeclsWithLiberalStrategy() {
-this->ODRHandling = ASTImporter::ODRHandlingType::Liberal;
-  }
-};
-
-// Check that a Decl has been successfully imported into a standalone redecl
-// chain.
-template 
-static void CheckImportedAsNew(llvm::Expected &Result, Decl *ToTU,
-   PatternTy Pattern) {
-  ASSERT_TRUE(isSuccess(Result));
-  Decl *ImportedD = *Result;
-  ASSERT_TRUE(ImportedD);
-  auto *ToD = FirstDeclMatcher().match(ToTU, Pattern);
-  EXPECT_NE(ImportedD, ToD);
-  EXPECT_FALSE(ImportedD->getPreviousDecl());
-  EXPECT_EQ(DeclCounter().match(ToTU, Pattern), 2u);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, Typedef) {
-  Decl *ToTU = getToTuDecl(
-  R"(
-  typedef int X;
-  )",
-  Lang_CXX11);
-  Decl *FromTU = getTuDecl(
-  R"(
-  typedef double X;
-  )",
-  Lang_CXX11);
-  auto Pattern = typedefNameDecl(hasName("X"));
-  auto *FromX = FirstDeclMatcher().match(FromTU, Pattern);
-
-  Expected Result = importOrError(FromX, Lang_CXX11);
-  CheckImportedAsNew(Result, ToTU, Pattern);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, TypeAlias) {
-  Decl *ToTU = getToTuDecl(
-  R"(
-  using X = int;
-  )",
-  Lang_CXX11);
-  Decl *FromTU = getTuDecl(
-  R"(
-  using X = double;
-  )",
-  Lang_CXX11);
-  auto Pattern = typedefNameDecl(hasName("X"));
-  auto *FromX = FirstDeclMatcher().match(FromTU, Pattern);
-  Expected Result = importOrError(FromX, Lang_CXX11);
-  CheckImportedAsNew(Result, ToTU, Pattern);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, EnumDecl) {
-  Decl *ToTU = getToTuDecl(
-  R"(
-  enum X { a, b };
-  )",
-  Lang_CXX11);
-  Decl *FromTU = getTuDecl(
-  R"(
-  enum X { a, b, c };
-  )",
-  Lang_CXX11);
-  auto Pattern = enumDecl(hasName("X"));
-  auto *FromX = FirstDeclMatcher().match(FromTU, Pattern);
-  Expected Result = importOrError(FromX, Lang_CXX11);
-  CheckImportedAsNew(Result, ToTU, Pattern);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, EnumConstantDecl) {
-  Decl *ToTU = getToTuDecl(
-  R"(
-  enum E { X = 0 };
-  )",
-  Lang_CXX11);
-  Decl *FromTU = getTuDecl(
-  R"(
-  enum E { X = 1 };
-  )",
-  Lang_CXX11);
-  auto Pattern = enumConstantDecl(hasName("X"));
-  auto *FromX = FirstDeclMatcher().match(FromTU, Pattern);
-  Expected Result = importOrError(FromX, Lang_CXX11);
-  CheckImportedAsNew(Result, ToTU, Pattern);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, RecordDecl) {
-  Decl *ToTU = getToTuDecl(
-  R"(
-  class X { int a; };
-  )",
-  Lang_CXX11);
-  Decl *FromTU = getTuDecl(
-  R"(
-  class X { int b; };
-  )",
-  Lang_CXX11);
-  auto Pattern = cxxRecordDecl(hasName("X"), unless(isImplicit()));
-  auto *FromX = FirstDeclMatcher().match(FromTU, Pattern);
-  Expected Result = importOrError(FromX, Lang_CXX11);
-  CheckImportedAsNew(Result, ToTU, Pattern);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, VarDecl) {
-  Decl *ToTU = getToTuDecl(
-  R"(
-  int X;
-  )",
-  Lang_CXX11);
-  Decl *FromTU = getTuDecl(
-  R"(
-  double X;
-  )",
-  Lang_CXX11);
-  auto Pattern = varDecl(hasName("X"));
-  auto *FromX = FirstDeclMatcher().match(FromTU, Pattern);
-  Expected Result = importOrError(FromX, Lang_CXX11);
-  CheckImportedAsNew(Result, ToTU, Pattern);
-}
-
-TEST_P(ConflictingDeclsWithLiberalStrategy, FunctionDecl) {
-  Decl *ToTU = getToTuDecl(
-  R"(
-  void X(int);
-  )",
-  Lang_C); // C, no overloading!
-  Decl *FromTU = getTuDecl(
-  R"(
-  void X(double);
-  )",
-  Lang_C);
-  auto Pattern = functionDecl(hasName("X"));
-  auto *FromX = FirstDeclMatcher().match(FromTU, Pattern);
-  Expected Result = importOrError(FromX, Lang_CXX11);
-  CheckI

[clang-tools-extra] r370864 - [clangd] Move threading helper to more appropriate header. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Sep  4 02:53:24 2019
New Revision: 370864

URL: http://llvm.org/viewvc/llvm-project?rev=370864&view=rev
Log:
[clangd] Move threading helper to more appropriate header. NFC

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/clangd/Threading.h

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=370864&r1=370863&r2=370864&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Sep  4 02:53:24 2019
@@ -31,7 +31,7 @@
 #include "Protocol.h"
 #include "Quality.h"
 #include "SourceCode.h"
-#include "TUScheduler.h"
+#include "Threading.h"
 #include "Trace.h"
 #include "URI.h"
 #include "index/Index.h"

Modified: clang-tools-extra/trunk/clangd/TUScheduler.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.h?rev=370864&r1=370863&r2=370864&view=diff
==
--- clang-tools-extra/trunk/clangd/TUScheduler.h (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.h Wed Sep  4 02:53:24 2019
@@ -20,7 +20,6 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
-#include 
 
 namespace clang {
 namespace clangd {
@@ -259,19 +258,6 @@ private:
   std::chrono::steady_clock::duration UpdateDebounce;
 };
 
-/// Runs \p Action asynchronously with a new std::thread. The context will be
-/// propagated.
-template 
-std::future runAsync(llvm::unique_function Action) {
-  return std::async(
-  std::launch::async,
-  [](llvm::unique_function &&Action, Context Ctx) {
-WithContext WithCtx(std::move(Ctx));
-return Action();
-  },
-  std::move(Action), Context::current().clone());
-}
-
 } // namespace clangd
 } // namespace clang
 

Modified: clang-tools-extra/trunk/clangd/Threading.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Threading.h?rev=370864&r1=370863&r2=370864&view=diff
==
--- clang-tools-extra/trunk/clangd/Threading.h (original)
+++ clang-tools-extra/trunk/clangd/Threading.h Wed Sep  4 02:53:24 2019
@@ -14,6 +14,7 @@
 #include "llvm/ADT/Twine.h"
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -117,6 +118,19 @@ private:
   std::size_t InFlightTasks = 0;
 };
 
+/// Runs \p Action asynchronously with a new std::thread. The context will be
+/// propagated.
+template 
+std::future runAsync(llvm::unique_function Action) {
+  return std::async(
+  std::launch::async,
+  [](llvm::unique_function &&Action, Context Ctx) {
+WithContext WithCtx(std::move(Ctx));
+return Action();
+  },
+  std::move(Action), Context::current().clone());
+}
+
 } // namespace clangd
 } // namespace clang
 #endif


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

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet added a comment.

This patch broke my build as well (discovered by runnin `git bisect`), command 
line

> cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_COMPILER=/usr/local/bin/clang 
> -DCMAKE_ASM_COMPILER=/usr/local/bin/clang 
> -DCMAKE_CXX_COMPILER=/usr/local/bin/clang++ 
> -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DBUILD_SHARED_LIBS=ON 
> -DLLVM_USE_SPLIT_DWARF=ON -DLLVM_OPTIMIZED_TABLEGEN=ON 
> -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=ARC -DLLVM_ENABLE_PROJECTS=clang 
> -DLLVM_BUILD_TESTS=ON -DLLVM_BUILD_DOCS=ON -DLLVM_ENABLE_WERROR=ON 
> -H/redacted/git/llvm-project/llvm -B/tmp/llvm-project_dbg_compiled-with-clang 
> -GNinja
>  ninja -C /tmp/llvm-project_dbg_compiled-with-clang

Error:

> /usr/bin/ld: 
> tools/clang/lib/AST/Interp/CMakeFiles/obj.clangInterp.dir/Block.cpp.o:(.data+0x0):
>  undefined reference to `llvm::EnableABIBreakingChecks'
>  /usr/bin/ld: 
> tools/clang/lib/AST/Interp/CMakeFiles/obj.clangInterp.dir/ByteCodeEmitter.cpp.o:
>  in function 
> `clang::interp::ByteCodeEmitter::emitAdd(clang::interp::PrimType, 
> clang::interp::SourceInfo const&)':
>  /usr/bin/ld: 
> tools/clang/lib/AST/Interp/CMakeFiles/obj.clangInterp.dir/ByteCodeEmitter.cpp.o:
>  in function 
> `clang::interp::ByteCodeEmitter::emitAddOffset(clang::interp::PrimType, 
> clang::interp::SourceInfo const&)':

...


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64146



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

[clang-tools-extra] r370865 - [clangd] Remove obsolete includes. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Sep  4 03:01:05 2019
New Revision: 370865

URL: http://llvm.org/viewvc/llvm-project?rev=370865&view=rev
Log:
[clangd] Remove obsolete includes. NFC

Modified:
clang-tools-extra/trunk/clangd/Threading.h
clang-tools-extra/trunk/clangd/Trace.cpp
clang-tools-extra/trunk/clangd/Trace.h

Modified: clang-tools-extra/trunk/clangd/Threading.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Threading.h?rev=370865&r1=370864&r2=370865&view=diff
==
--- clang-tools-extra/trunk/clangd/Threading.h (original)
+++ clang-tools-extra/trunk/clangd/Threading.h Wed Sep  4 03:01:05 2019
@@ -1,4 +1,4 @@
-//===--- ThreadPool.h *- 
C++-*-===//
+//===--- Threading.h - Abstractions for multithreading ---*- 
C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -10,7 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_THREADING_H
 
 #include "Context.h"
-#include "Function.h"
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/Twine.h"
 #include 
 #include 

Modified: clang-tools-extra/trunk/clangd/Trace.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Trace.cpp?rev=370865&r1=370864&r2=370865&view=diff
==
--- clang-tools-extra/trunk/clangd/Trace.cpp (original)
+++ clang-tools-extra/trunk/clangd/Trace.cpp Wed Sep  4 03:01:05 2019
@@ -8,7 +8,6 @@
 
 #include "Trace.h"
 #include "Context.h"
-#include "Function.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Chrono.h"

Modified: clang-tools-extra/trunk/clangd/Trace.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Trace.h?rev=370865&r1=370864&r2=370865&view=diff
==
--- clang-tools-extra/trunk/clangd/Trace.h (original)
+++ clang-tools-extra/trunk/clangd/Trace.h Wed Sep  4 03:01:05 2019
@@ -18,7 +18,6 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_
 
 #include "Context.h"
-#include "Function.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/raw_ostream.h"


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

[clang-tools-extra] r370869 - [clangd] Remove macro-expansion-location from getBeginningOfIdentifier. Inline into relevant callsites. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Sep  4 03:15:27 2019
New Revision: 370869

URL: http://llvm.org/viewvc/llvm-project?rev=370869&view=rev
Log:
[clangd] Remove macro-expansion-location from getBeginningOfIdentifier. Inline 
into relevant callsites. NFC

Modified:
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/refactor/Rename.cpp

Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=370869&r1=370868&r2=370869&view=diff
==
--- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp Wed Sep  4 03:15:27 2019
@@ -250,7 +250,7 @@ SourceLocation getBeginningOfIdentifier(
   // location is correct for both!
   SourceLocation InputLoc = SM.getComposedLoc(FID, *Offset);
   if (*Offset == 0) // Case 1 or 3.
-return SM.getMacroArgExpandedLocation(InputLoc);
+return InputLoc;
   SourceLocation Before = SM.getComposedLoc(FID, *Offset - 1);
 
   Before = Lexer::GetBeginningOfToken(Before, SM, LangOpts);
@@ -258,8 +258,8 @@ SourceLocation getBeginningOfIdentifier(
   if (Before.isValid() &&
   !Lexer::getRawToken(Before, Tok, SM, LangOpts, false) &&
   Tok.is(tok::raw_identifier))
-return SM.getMacroArgExpandedLocation(Before); // Case 2.
-  return SM.getMacroArgExpandedLocation(InputLoc); // Case 1 or 3.
+return Before; // Case 2.
+  return InputLoc; // Case 1 or 3.
 }
 
 bool isValidFileRange(const SourceManager &Mgr, SourceRange R) {

Modified: clang-tools-extra/trunk/clangd/SourceCode.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.h?rev=370869&r1=370868&r2=370869&view=diff
==
--- clang-tools-extra/trunk/clangd/SourceCode.h (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.h Wed Sep  4 03:15:27 2019
@@ -77,7 +77,8 @@ llvm::Expected sourceLoc
 
 /// Get the beginning SourceLocation at a specified \p Pos in the main file.
 /// May be invalid if Pos is, or if there's no identifier.
-/// FIXME: this returns the macro-expansion location, but it shouldn't.
+/// The returned position is in the main file, callers may prefer to
+/// obtain the macro expansion location.
 SourceLocation getBeginningOfIdentifier(const Position &Pos,
 const SourceManager &SM,
 const LangOptions &LangOpts);

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=370869&r1=370868&r2=370869&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Wed Sep  4 03:15:27 2019
@@ -255,8 +255,9 @@ std::vector locateSymbolA
 }
   }
 
-  SourceLocation SourceLocationBeg = getBeginningOfIdentifier(
-  Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts());
+  SourceLocation SourceLocationBeg =
+  SM.getMacroArgExpandedLocation(getBeginningOfIdentifier(
+  Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts()));
 
   // Macros are simple: there's no declaration/definition distinction.
   // As a consequence, there's no need to look them up in the index either.
@@ -409,10 +410,11 @@ std::vector findDocum
   Position Pos) {
   const SourceManager &SM = AST.getSourceManager();
   // FIXME: show references to macro within file?
-  auto References = findRefs(
-  getDeclAtPosition(AST, getBeginningOfIdentifier(
- Pos, SM, AST.getASTContext().getLangOpts())),
-  AST);
+  auto References =
+  findRefs(getDeclAtPosition(
+   AST, 
SM.getMacroArgExpandedLocation(getBeginningOfIdentifier(
+Pos, SM, AST.getASTContext().getLangOpts(,
+   AST);
 
   // FIXME: we may get multiple DocumentHighlights with the same location and
   // different kinds, deduplicate them.
@@ -875,9 +877,10 @@ bool hasDeducedType(ParsedAST &AST, Sour
 llvm::Optional getHover(ParsedAST &AST, Position Pos,
format::FormatStyle Style,
const SymbolIndex *Index) {
+  const SourceManager &SM = AST.getSourceManager();
   llvm::Optional HI;
-  SourceLocation SourceLocationBeg = getBeginningOfIdentifier(
-  Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts());
+  SourceLocation SourceLocationBeg = SM.getMacroArgExpandedLocation(
+  getBeginningOfIdentifier(Pos, SM, AST.getASTContext().getLangOpts()));
 
   if (auto M = locateMacroAt(SourceLocationBeg, AST.get

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

Shared library builds seem to be broken indeed.  I tried fixing by adding 
`Support` and `clangAST` as dependencies for `clangInterp`, but that creates a 
cyclic dependency between `clangAST` <-> `clangInterp`.  Which makes me wonder 
whether `clangInterp` should be a separate library or be part of `clangAST`?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64146



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

[PATCH] D66637: [clangd] Support multifile edits as output of Tweaks

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked 13 inline comments as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:414
+  if (llvm::Error Err = reformatEdit(E, Style)) {
+llvm::handleAllErrors(std::move(Err), [&](llvm::ErrorInfoBase &EIB) {
+  elog("Failed to format {0}: {1}", It.first(), EIB.message());

sammccall wrote:
> isn't this just
> `elog("Failed to format {0}: {1}", std::move(Err))`
IIUC, just printing the error doesn't actually mark it as checked, therefore 
causes an assertion failure (see `getPtr` vs `getPayload` in `llvm::Error`). 
And I couldn't see any special handling in `elog`, but not sure if `formatv 
object` has this.



Comment at: clang-tools-extra/clangd/refactor/Tweak.cpp:90
+  assert(FE && "Generating edits for unknown file!");
+  llvm::StringRef FilePath = FE->getName();
+  Edit Ed(SM.getBufferData(FID), std::move(Replacements));

sammccall wrote:
> how do you know this is the correct/absolute path for the file? Can we add 
> tests?
changed it to use `FileInfo::getName` rather then `FileEntry::getName`, as the 
former says:
```
/// Returns the name of the file that was used when the file was loaded from
/// the underlying file system.```

These also get tested through any lit tests we have for tweaks, adding more 
unittests.



Comment at: clang-tools-extra/clangd/refactor/Tweak.h:130
+/// pointed by FID.
+Tweak::Effect fileEdit(const SourceManager &SM, FileID FID,
+   tooling::Replacements Replacements);

sammccall wrote:
> sammccall wrote:
> > what will this be used for?
> > 
> > You can use at most one of these Effect factories (unless we're going to do 
> > some convoluted merge thing), so this seems useful if you've got exactly 
> > one edit to a non-main file.
> > 
> > It seems more useful to return a pair which could be inserted 
> > into a map
> why are these moved out of the Effect class?
> need to provide a fully-qualified name (I don't think the implementations in 
> this patch actually do that)

I've thought you were also requesting these to be helper functions rather than 
static methods so that callers don't need to qualify the call.
Can move it back into class if I misunderstood your point, but this actually 
seems a lot cleaner.



Comment at: clang-tools-extra/clangd/refactor/Tweak.h:130
+/// pointed by FID.
+Tweak::Effect fileEdit(const SourceManager &SM, FileID FID,
+   tooling::Replacements Replacements);

kadircet wrote:
> sammccall wrote:
> > sammccall wrote:
> > > what will this be used for?
> > > 
> > > You can use at most one of these Effect factories (unless we're going to 
> > > do some convoluted merge thing), so this seems useful if you've got 
> > > exactly one edit to a non-main file.
> > > 
> > > It seems more useful to return a pair which could be 
> > > inserted into a map
> > why are these moved out of the Effect class?
> > need to provide a fully-qualified name (I don't think the implementations 
> > in this patch actually do that)
> 
> I've thought you were also requesting these to be helper functions rather 
> than static methods so that callers don't need to qualify the call.
> Can move it back into class if I misunderstood your point, but this actually 
> seems a lot cleaner.
> what will this be used for?

you are right it doesn't really compose well, changed it to return the pair 
instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66637



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

[PATCH] D66637: [clangd] Support multifile edits as output of Tweaks

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 218637.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66637

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/refactor/Tweak.cpp
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp
  clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.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
@@ -7,7 +7,9 @@
 //===--===//
 
 #include "Annotations.h"
+#include "ClangdUnit.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestTU.h"
 #include "TweakTesting.h"
 #include "refactor/Tweak.h"
@@ -113,11 +115,15 @@
   auto Effect = apply(ID, Input);
   if (!Effect)
 return Effect.takeError();
-  if (!Effect->ApplyEdit)
+  if (Effect->ApplyEdits.empty())
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No replacements");
+  auto Edits = Effect->ApplyEdits;
+  if (Edits.size() > 1)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "Multi-file edits");
   Annotations Code(Input);
-  return applyAllReplacements(Code.code(), *Effect->ApplyEdit);
+  return applyAllReplacements(Code.code(), Edits.begin()->second.Replacements);
 }
 
 void checkTransform(llvm::StringRef ID, llvm::StringRef Input,
@@ -127,6 +133,39 @@
   EXPECT_EQ(Output, std::string(*Result)) << Input;
 }
 
+TEST(FileEdits, AbsolutePath) {
+  TestTU TU;
+  TU.Filename = "foo.cpp";
+  TU.Code = R"cpp(
+#include "a.h"
+#include "b.h"
+void bar();
+  )cpp";
+  TU.AdditionalFiles = {
+  {"a.h", "void baz();"},
+  {"test/b.h", "void foo();"},
+  };
+  auto T = "-I" + testPath("test/");
+  TU.ExtraArgs = {T.c_str()};
+
+  const ParsedAST AST = TU.build();
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  auto &SM = AST.getSourceManager();
+  const auto Files = {testPath("foo.cpp"), testPath("test/b.h"),
+  testPath("a.h")};
+
+  for (const Decl *D : AST.getASTContext().getTranslationUnitDecl()->decls()) {
+if (D->getLocation().isInvalid() ||
+SM.isWrittenInBuiltinFile(D->getLocation()))
+  continue;
+
+auto FID = SM.getFileID(D->getLocation());
+auto Res = fileEdit(SM, FID, tooling::Replacements());
+EXPECT_THAT(Files, testing::Contains(Res.first));
+  }
+}
+
 TWEAK_TEST(SwapIfBranches);
 TEST_F(SwapIfBranchesTest, Test) {
   Context = Function;
Index: clang-tools-extra/clangd/unittests/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -9,8 +9,8 @@
 #include "TweakTesting.h"
 
 #include "Annotations.h"
-#include "refactor/Tweak.h"
 #include "SourceCode.h"
+#include "refactor/Tweak.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/Support/Error.h"
 
@@ -98,14 +98,16 @@
 return "fail: " + llvm::toString(Result.takeError());
   if (Result->ShowMessage)
 return "message:\n" + *Result->ShowMessage;
-  if (Result->ApplyEdit) {
-if (auto NewText =
-tooling::applyAllReplacements(Input.code(), *Result->ApplyEdit))
-  return unwrap(Context, *NewText);
-else
-  return "bad edits: " + llvm::toString(NewText.takeError());
-  }
-  return "no effect";
+  if (Result->ApplyEdits.empty())
+return "no effect";
+  if (Result->ApplyEdits.size() > 1)
+return "received multi-file edits";
+
+  auto ApplyEdit = Result->ApplyEdits.begin()->second;
+  if (auto NewText = ApplyEdit.apply())
+return unwrap(Context, *NewText);
+  else
+return "bad edits: " + llvm::toString(NewText.takeError());
 }
 
 ::testing::Matcher TweakTest::isAvailable() const {
Index: clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -90,7 +90,7 @@
   

r370874 - Revert "[Clang Interpreter] Initial patch for the constexpr interpreter"

[Roman Lebedev via cfe-commits]

Author: lebedevri
Date: Wed Sep  4 03:57:06 2019
New Revision: 370874

URL: http://llvm.org/viewvc/llvm-project?rev=370874&view=rev
Log:
Revert "[Clang Interpreter] Initial patch for the constexpr interpreter"

Breaks BUILD_SHARED_LIBS build, introduces cycles in library dependency
graphs. (clangInterp depends on clangAST which depends on clangInterp)

This reverts r370839, which is an yet another recommit of D64146.

Removed:
cfe/trunk/docs/ConstantInterpreter.rst
cfe/trunk/include/clang/AST/OptionalDiagnostic.h
cfe/trunk/lib/AST/Interp/
cfe/trunk/test/AST/Interp/
cfe/trunk/utils/TableGen/ClangOpcodesEmitter.cpp
Modified:
cfe/trunk/docs/index.rst
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/CMakeLists.txt
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp
cfe/trunk/test/SemaCXX/constexpr-many-arguments.cpp
cfe/trunk/test/SemaCXX/shift.cpp
cfe/trunk/utils/TableGen/CMakeLists.txt
cfe/trunk/utils/TableGen/TableGen.cpp
cfe/trunk/utils/TableGen/TableGenBackends.h

Removed: cfe/trunk/docs/ConstantInterpreter.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ConstantInterpreter.rst?rev=370873&view=auto
==
--- cfe/trunk/docs/ConstantInterpreter.rst (original)
+++ cfe/trunk/docs/ConstantInterpreter.rst (removed)
@@ -1,194 +0,0 @@
-
-Constant Interpreter
-
-
-.. contents::
-   :local:
-
-Introduction
-
-
-The constexpr interpreter aims to replace the existing tree evaluator in 
clang, improving performance on constructs which are executed inefficiently by 
the evaluator. The interpreter is activated using the following flags:
-
-* ``-fexperimental-new-constant-interpreter`` enables the interpreter, falling 
back to the evaluator for unsupported features
-* ``-fforce-experimental-new-constant-interpreter`` forces the use of the 
interpreter, bailing out if an unsupported feature is encountered
-
-Bytecode Compilation
-
-
-Bytecode compilation is handled in ``ByteCodeStmtGen.h`` for statements and 
``ByteCodeExprGen.h`` for expressions. The compiler has two different backends: 
one to generate bytecode for functions (``ByteCodeEmitter``) and one to 
directly evaluate expressions as they are compiled, without generating bytecode 
(``EvalEmitter``). All functions are compiled to bytecode, while toplevel 
expressions used in constant contexts are directly evaluated since the bytecode 
would never be reused. This mechanism aims to pave the way towards replacing 
the evaluator, improving its performance on functions and loops, while being 
just as fast on single-use toplevel expressions.
-
-The interpreter relies on stack-based, strongly-typed opcodes. The glue logic 
between the code generator, along with the enumeration and description of 
opcodes, can be found in ``Opcodes.td``. The opcodes are implemented as generic 
template methods in ``Interp.h`` and instantiated with the relevant primitive 
types by the interpreter loop or by the evaluating emitter.
-
-Primitive Types

-
-* ``PT_{U|S}int{8|16|32|64}``
-
-  Signed or unsigned integers of a specific bit width, implemented using the 
```Integral``` type.
-
-* ``PT_{U|S}intFP``
-
-  Signed or unsigned integers of an arbitrary, but fixed width used to 
implement
-  integral types which are required by the target, but are not supported by 
the host.
-  Under the hood, they rely on APValue. The ``Integral`` specialisation for 
these
-  types is required by opcodes to share an implementation with fixed integrals.
-
-* ``PT_Bool``
-
-  Representation for boolean types, essentially a 1-bit unsigned ``Integral``.
-
-* ``PT_RealFP``
-
-  Arbitrary, but fixed precision floating point numbers. Could be specialised 
in
-  the future similarly to integers in order to improve floating point 
performance.
-
-* ``PT_Ptr``
-
-  Pointer type, defined in ``"Pointer.h"``.
-
-* ``PT_FnPtr``
-
-  Function pointer type, can also be a null function pointer. Defined in 
``"Pointer.h"``.
-
-* ``PT_MemPtr``
-
-  Member pointer type, can also be a null member pointer. Defined in 
``"Pointer.h"``
-
-Composite types

-
-The interpreter distinguishes two kinds of composite types: arrays and 
records. Unions are represented as records, except a single field can be marked 
as active. The contents of inactive fields are kept until they
-are reactivated and overwritten.
-
-
-Bytecode Execution
-==
-
-Bytecode is executed using a stack-based interpreter. The execution context 
consists of an ``InterpStack``, al

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D64146#1657117 , @svenvh wrote:

> Shared library builds seem to be broken indeed.  I tried fixing by adding 
> `Support` and `clangAST` as dependencies for `clangInterp`, but that creates 
> a cyclic dependency between `clangAST` <-> `clangInterp`.  Which makes me 
> wonder whether `clangInterp` should be a separate library or be part of 
> `clangAST`?


Arrived at same conclusions, reverted in rL370874 
.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64146



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

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D64146#1657146 , @lebedev.ri wrote:

> In D64146#1657117 , @svenvh wrote:
>
> > Shared library builds seem to be broken indeed.  I tried fixing by adding 
> > `Support` and `clangAST` as dependencies for `clangInterp`, but that 
> > creates a cyclic dependency between `clangAST` <-> `clangInterp`.  Which 
> > makes me wonder whether `clangInterp` should be a separate library or be 
> > part of `clangAST`?
>
>
> Arrived at same conclusions, reverted in rL370874 
> .


And that was even pointed out several days ago before the patch got relanded 
last **//two//** times:

In D64146#1653221 , @teemperor wrote:

> Seems like this patch introduced a cyclic dependency between Basic and AST 
> when building with LLVM_ENABLE_MODULES=On:
>
>   While building module 'Clang_AST' imported from 
> llvm/lldb/include/lldb/Symbol/ClangUtil.h:14:
>   While building module 'Clang_Basic' imported from 
> llvm/llvm/../clang/include/clang/AST/Stmt.h:18:
>   In file included from :73:
>   clang/include/clang/Basic/OptionalDiagnostic.h:17:10: fatal error: cyclic 
> dependency in module 'Clang_AST': Clang_AST -> Clang_Basic -> Clang_AST
>   #include "clang/AST/APValue.h"
>


@nand Please don't reland this patch until these concerns are addressed.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64146



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

[PATCH] D65744: [PR42707][OpenCL] Fix addr space deduction for auto

[Anastasia Stulova via Phabricator via cfe-commits]

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

Moved addr space of pointee inference into Build* helpers of Sema.


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

https://reviews.llvm.org/D65744

Files:
  clang/include/clang/AST/Type.h
  clang/lib/AST/Expr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h

Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -4535,14 +4535,6 @@
   return Result;
 }
 
-/// Helper to deduce addr space of a pointee type in OpenCL mode.
-/// If the type is updated it will be overwritten in PointeeType param.
-static void deduceOpenCLPointeeAddrSpace(Sema &SemaRef, QualType &PointeeType) {
-  if (PointeeType.getAddressSpace() == LangAS::Default)
-PointeeType = SemaRef.Context.getAddrSpaceQualType(PointeeType,
-   LangAS::opencl_generic);
-}
-
 template
 QualType TreeTransform::TransformPointerType(TypeLocBuilder &TLB,
   PointerTypeLoc TL) {
@@ -4551,9 +4543,6 @@
   if (PointeeType.isNull())
 return QualType();
 
-  if (SemaRef.getLangOpts().OpenCL)
-deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
-
   QualType Result = TL.getType();
   if (PointeeType->getAs()) {
 // A dependent pointer type 'T *' has is being transformed such
@@ -4592,9 +4581,6 @@
   if (PointeeType.isNull())
 return QualType();
 
-  if (SemaRef.getLangOpts().OpenCL)
-deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
-
   QualType Result = TL.getType();
   if (getDerived().AlwaysRebuild() ||
   PointeeType != TL.getPointeeLoc().getType()) {
@@ -4624,9 +4610,6 @@
   if (PointeeType.isNull())
 return QualType();
 
-  if (SemaRef.getLangOpts().OpenCL)
-deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
-
   QualType Result = TL.getType();
   if (getDerived().AlwaysRebuild() ||
   PointeeType != T->getPointeeTypeAsWritten()) {
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1962,6 +1962,18 @@
   return true;
 }
 
+// Helper to deduce addr space of a pointee type in OpenCL mode.
+static QualType deduceOpenCLPointeeAddrSpace(Sema &S, QualType PointeeType) {
+  if (!PointeeType->isUndeducedAutoType() && !PointeeType->isDependentType() &&
+  !PointeeType.getQualifiers().hasAddressSpace())
+PointeeType = S.getASTContext().getAddrSpaceQualType(
+PointeeType,
+S.getLangOpts().OpenCLCPlusPlus || S.getLangOpts().OpenCLVersion == 200
+? LangAS::opencl_generic
+: LangAS::opencl_private);
+  return PointeeType;
+}
+
 /// Build a pointer type.
 ///
 /// \param T The type to which we'll be building a pointer.
@@ -1998,6 +2010,9 @@
   if (getLangOpts().ObjCAutoRefCount)
 T = inferARCLifetimeForPointee(*this, T, Loc, /*reference*/ false);
 
+  if (getLangOpts().OpenCL)
+T = deduceOpenCLPointeeAddrSpace(*this, T);
+
   // Build the pointer type.
   return Context.getPointerType(T);
 }
@@ -2058,6 +2073,9 @@
   if (getLangOpts().ObjCAutoRefCount)
 T = inferARCLifetimeForPointee(*this, T, Loc, /*reference*/ true);
 
+  if (getLangOpts().OpenCL)
+T = deduceOpenCLPointeeAddrSpace(*this, T);
+
   // Handle restrict on references.
   if (LValueRef)
 return Context.getLValueReferenceType(T, SpelledAsLValue);
@@ -2626,6 +2644,9 @@
   if (checkQualifiedFunction(*this, T, Loc, QFK_BlockPointer))
 return QualType();
 
+  if (getLangOpts().OpenCL)
+T = deduceOpenCLPointeeAddrSpace(*this, T);
+
   return Context.getBlockPointerType(T);
 }
 
@@ -7434,6 +7455,9 @@
   // Do not deduce addr space of decltype because it will be taken from
   // its argument.
   T->isDecltypeType() ||
+  // Do not deduce addr space for auto pointee type because it is taken from
+  // the initializing expression type during the type deduction.
+  (T->isUndeducedAutoType() && IsPointee) ||
   // OpenCL spec v2.0 s6.9.b:
   // The sampler type cannot be used with the __local and __global address
   // space qualifiers.
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1805,7 +1805,7 @@
 auto Ty = getType();
 auto SETy = getSubExpr()->getType();
 assert(getValueKindForType(Ty) == Expr::getValueKindForType(SETy));
-if (/*isRValue()*/ !Ty->getPointeeType().isNull()) {
+if (isRValue()) {
   Ty = Ty->getPointeeType();
   SETy = SETy->getPointeeType();
 }
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -2044,6 +2044,8 @@
   bool isAlignValT() co

[PATCH] D65744: [PR42707][OpenCL] Fix addr space deduction for auto

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D65744#1653081 , @rjmccall wrote:

> In D65744#1652355 , @Anastasia wrote:
>
> > I don't think this is likely to change. Are you suggesting to move the 
> > deduction logic for pointee of pointers, references and block pointers into 
> > ASTContext helper that creates a pointer/reference/block pointer type?
>
>
> No.  I'm suggesting that the deduction logic should be much more 
> straightforward, just some sort of "is the type non-dependent and lacking a 
> qualifier", and it should be applied in the two basic places we build these 
> types in Sema, i.e. in the type-from-declarator logic and in the 
> `Build{Pointer,Reference}Type` logic.


Ok thanks, I made this change now.

> Instead we have something very elaborate that apparently recursively looks 
> through pointer types and is contingent on the exact spelling, e.g. trying to 
> find `auto` types, which seems both brittle and unnecessary.

I realized that I didn't have to do this actually. It was done by mistake 
earlier.


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

https://reviews.llvm.org/D65744



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

[PATCH] D65744: [PR42707][OpenCL] Fix addr space deduction for auto

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: include/clang/AST/Type.h:6509
+  return isa(CanonicalType);
+}
+

rjmccall wrote:
> Hmm.  So this method, confusingly, will not return true for a deduced `auto`, 
> unless the deduced type is itself an undeduced `auto` (which I'm not sure can 
> happen).  I think it at least needs a different name; `isUndeducedAutoType()` 
> would be okay if the latter case is not possible.  But it might be better if 
> we can just define away the need for the method entirely.
I feel I still need this helper in order to detect the auto type. Alternatively 
I can create a static function for this instead of publicly exposing this 
functionality. Or maybe you have other ideas in mind...


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

https://reviews.llvm.org/D65744



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

[PATCH] D65744: [PR42707][OpenCL] Fix addr space deduction for auto

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D65744#1639175 , @mantognini wrote:

> I think this looks good. Maybe the tests should be extended to test `auto` as 
> function return type, and if there's some special handling around 
> `decltype(auto)`, then it should be tested too, but I'm not sure it's 
> actually needed here. What do you think?


Do you mean anything specific to test?


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

https://reviews.llvm.org/D65744



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

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

[Nandor Licker via Phabricator via cfe-commits]

nand reopened this revision.
nand added a comment.

Thanks for identifying these issues - I fixed the cycle detected by modules 
since then, however I wasn't aware of the issue with shared libs until now.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64146



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

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon requested changes to this revision.
RKSimon added a comment.
This revision now requires changes to proceed.

MSVC builds are seeing template instantiation  issues with this patch: 
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/27972/steps/build/logs/warnings%20%2824%29


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64146



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

[clang-tools-extra] r370884 - [clangd] Fix SelectionTree behavior on implicit 'this'

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Sep  4 05:15:20 2019
New Revision: 370884

URL: http://llvm.org/viewvc/llvm-project?rev=370884&view=rev
Log:
[clangd] Fix SelectionTree behavior on implicit 'this'

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

Modified: clang-tools-extra/trunk/clangd/Selection.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Selection.cpp?rev=370884&r1=370883&r2=370884&view=diff
==
--- clang-tools-extra/trunk/clangd/Selection.cpp (original)
+++ clang-tools-extra/trunk/clangd/Selection.cpp Wed Sep  4 05:15:20 2019
@@ -19,6 +19,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -205,6 +206,11 @@ public:
   bool dataTraverseStmtPre(Stmt *X) {
 if (!X)
   return false;
+// Implicit this in a MemberExpr is not filtered out by 
RecursiveASTVisitor.
+// It would be nice if RAV handled this (!shouldTRaverseImplicitCode()).
+if (auto *CTI = llvm::dyn_cast(X))
+  if (CTI->isImplicit())
+return false;
 auto N = DynTypedNode::create(*X);
 if (canSafelySkipNode(N))
   return false;

Modified: clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp?rev=370884&r1=370883&r2=370884&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/SelectionTests.cpp Wed Sep  4 
05:15:20 2019
@@ -210,6 +210,15 @@ TEST(SelectionTest, CommonAncestor) {
   )cpp",
   "FunctionProtoTypeLoc",
   },
+  {
+  R"cpp(
+struct S {
+  int foo;
+  int bar() { return [[f^oo]]; }
+};
+  )cpp",
+  "MemberExpr", // Not implicit CXXThisExpr!
+  },
 
   // Point selections.
   {"void foo() { [[^foo]](); }", "DeclRefExpr"},


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

[PATCH] D67156: [Analyzer] Debug Checkers for Container and Iterator Inspection

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

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, Szelethus.
baloghadamsoftware added a project: clang.
Herald added subscribers: Charusso, donat.nagy, mikhail.ramalho, a.sidorin, 
rnkovacs, szepet, xazax.hun.

For white-box testing correct container and iterator modelling it is essential 
to access the internal data structures stored for container and iterators. This 
patch introduces two simple debug checkers called `debug.ContainerInspection` 
and `debug.IteratorInspection` to achieve this.


Repository:
  rC Clang

https://reviews.llvm.org/D67156

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/iterator-inspection.cpp

Index: test/Analysis/iterator-inspection.cpp
===
--- /dev/null
+++ test/Analysis/iterator-inspection.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,debug.ContainerInspection,debug.IteratorInspection,debug.ExprInspection -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,debug.ContainerInspection,debug.IteratorInspection,debug.ExprInspection -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+template 
+long clang_analyzer_container_begin(const Container&);
+template 
+long clang_analyzer_container_end(const Container&);
+template 
+long clang_analyzer_iterator_position(const Iterator&);
+template 
+void* clang_analyzer_iterator_container(const Iterator&);
+template 
+bool clang_analyzer_iterator_validity(const Iterator&);
+void clang_analyzer_denote(long, const char*);
+void clang_analyzer_express(long);
+void clang_analyzer_dump(const void*);
+void clang_analyzer_eval(bool);
+
+void iterator_position(const std::vector v0) {
+  auto b0 = v0.begin(), e0 = v0.end();
+
+  clang_analyzer_denote(clang_analyzer_iterator_position(b0), "$b0");
+  clang_analyzer_denote(clang_analyzer_iterator_position(e0), "$e0");
+
+  clang_analyzer_express(clang_analyzer_iterator_position(b0)); // expected-warning{{$b0}}
+  clang_analyzer_express(clang_analyzer_iterator_position(e0)); // expected-warning{{$e0}}
+
+  clang_analyzer_express(clang_analyzer_container_begin(v0)); // expected-warning{{$b0}}
+  clang_analyzer_express(clang_analyzer_container_end(v0)); // expected-warning{{$e0}}
+
+  ++b0;
+
+  clang_analyzer_express(clang_analyzer_iterator_position(b0)); // expected-warning{{$b0 + 1}}
+}
+
+void iterator_container(const std::vector v0) {
+  auto b0 = v0.begin();
+
+  clang_analyzer_dump(&v0); //expected-warning{{&v0}}
+  clang_analyzer_dump(clang_analyzer_iterator_container(b0)); //expected-warning{{&v0}}
+}
+
+void iterator_validity(std::vector v0) {
+  auto b0 = v0.begin();
+  clang_analyzer_eval(clang_analyzer_iterator_validity(b0)); //expected-warning{{TRUE}}
+
+  v0.clear();
+
+  clang_analyzer_eval(clang_analyzer_iterator_validity(b0)); //expected-warning{{FALSE}}
+}
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -173,11 +173,12 @@
 class IteratorChecker
 : public Checker, check::Bind,
- check::LiveSymbols, check::DeadSymbols> {
+ check::LiveSymbols, check::DeadSymbols, eval::Call> {
 
   std::unique_ptr OutOfRangeBugType;
   std::unique_ptr MismatchedBugType;
   std::unique_ptr InvalidatedBugType;
+  std::unique_ptr DebugMsgBugType;
 
   void handleComparison(CheckerContext &C, const Expr *CE, const SVal &RetVal,
 const SVal &LVal, const SVal &RVal,
@@ -236,6 +237,21 @@
ExplodedNode *ErrNode) const;
   void reportInvalidatedBug(const StringRef &Message, const SVal &Val,
 CheckerContext &C, ExplodedNode *ErrNode) const;
+  template 
+  void analyzerContainerDataField(const CallExpr *CE, CheckerContext &C,
+  Getter get) const;
+  void analyzerContainerBegin(const CallExpr *CE, CheckerContext &C) const;
+  void analyzerContainerEnd(const CallExpr *CE, CheckerContext &C) const;
+  template 
+  void analyzerIteratorDataField(const CallExpr *CE, CheckerContext &C,
+ Getter get, SVal Default) const;
+  void analyzerIteratorPosition(const CallExpr *CE, CheckerContext &C) const;
+  void analyzerIteratorContainer(const CallExpr *CE, CheckerContext &C) const;
+  void analyzerIteratorValidity(const CallExpr *CE, CheckerContext &C) const;
+  ExplodedNode *reportDebugMsg(llvm::StringRef Msg, CheckerContext &C) const;
+
+  typedef void (IteratorChecker::*Fn

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

[Steve Canon via Phabricator via cfe-commits]

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

I believe that the code can still be simplified somewhat, but that it's correct 
as-is for `float`, `double`, and `long double`. I'll take an AI to follow-up on 
future improvements, and let's get this in.


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

https://reviews.llvm.org/D66836



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

[PATCH] D67156: [Analyzer] Debug Checkers for Container and Iterator Inspection

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

baloghadamsoftware added a comment.

I did not add debug calls to the tests of the existing iterator checkers yet, 
they will come in the next patch. After that I think the next step is to 
refactor the monolithic checker class into smaller ones: the iterator modelling 
into one file, and all the other checkers in small separate files. The 
modelling checker may also be split into two: container modelling and iterator 
modelling. Of course the is a connection between them, but they can be 
separated logically. All the common function must go into a library "module" 
(header and body file). However, the prerequisite for such a huge refactoring 
is a thorough whitebox testing.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67156



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

[PATCH] D67159: [clang] New __attribute__((__clang_builtin)).

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham created this revision.
simon_tatham added reviewers: dmgreen, miyuki, ostannard.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This allows you to declare a function with a name of your choice (say
`foo`), but have clang treat it as if it were a builtin function (say
`__builtin_foo`), by writing

  static __inline__ __attribute__((__clang_builtin(__builtin_foo)))
  int foo(args);

I'm intending to use this for the ACLE intrinsics for MVE, which have
to be polymorphic on their argument types and also implemented by
builtins. This commit itself just introduces the new attribute, and
doesn't use it for anything.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67159

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test


Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -36,6 +36,7 @@
 // CHECK-NEXT: Callback (SubjectMatchRule_function)
 // CHECK-NEXT: Capability (SubjectMatchRule_record, 
SubjectMatchRule_type_alias)
 // CHECK-NEXT: CarriesDependency (SubjectMatchRule_variable_is_parameter, 
SubjectMatchRule_objc_method, SubjectMatchRule_function)
+// CHECK-NEXT: ClangBuiltinOverride (SubjectMatchRule_function)
 // CHECK-NEXT: Cold (SubjectMatchRule_function)
 // CHECK-NEXT: Common (SubjectMatchRule_variable)
 // CHECK-NEXT: Constructor (SubjectMatchRule_function)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5041,6 +5041,13 @@
   AL.getAttributeSpellingListIndex()));
 }
 
+static void handleClangBuiltinOverrideAttribute(Sema &S, Decl *D,
+const ParsedAttr &AL) {
+  D->addAttr(::new (S.Context) ClangBuiltinOverrideAttr(
+  AL.getRange(), S.Context, AL.getArgAsIdent(0)->Ident,
+  AL.getAttributeSpellingListIndex()));
+}
+
 
//===--===//
 // Checker-specific attribute handlers.
 
//===--===//
@@ -7430,6 +7437,10 @@
   case ParsedAttr::AT_MSAllocator:
 handleMSAllocatorAttr(S, D, AL);
 break;
+
+  case ParsedAttr::AT_ClangBuiltinOverride:
+handleClangBuiltinOverrideAttribute(S, D, AL);
+break;
   }
 }
 
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3075,10 +3075,18 @@
 /// functions as their wrapped builtins. This shouldn't be done in general, but
 /// it's useful in Sema to diagnose calls to wrappers based on their semantics.
 unsigned FunctionDecl::getBuiltinID(bool ConsiderWrapperFunctions) const {
-  if (!getIdentifier())
-return 0;
+  unsigned BuiltinID;
+
+  if (hasAttr()) {
+IdentifierInfo *II = getAttr()->getBuiltinName();
+BuiltinID = II->getBuiltinID();
+  } else {
+if (!getIdentifier())
+  return 0;
+
+BuiltinID = getIdentifier()->getBuiltinID();
+  }
 
-  unsigned BuiltinID = getIdentifier()->getBuiltinID();
   if (!BuiltinID)
 return 0;
 
@@ -3102,7 +3110,8 @@
 
   // If the function is marked "overloadable", it has a different mangled name
   // and is not the C library function.
-  if (!ConsiderWrapperFunctions && hasAttr())
+  if (!ConsiderWrapperFunctions && hasAttr() &&
+  !hasAttr())
 return 0;
 
   if (!Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -593,6 +593,12 @@
   let Subjects = SubjectList<[Function, GlobalVar], ErrorDiag>;
   let Documentation = [Undocumented];
 }
+def ClangBuiltinOverride : Attr {
+  let Spellings = [GCC<"__clang_builtin">];
+  let Args = [IdentifierArgument<"BuiltinName">];
+  let Subjects = SubjectList<[Function], ErrorDiag>;
+  let Documentation = [Undocumented];
+}
 
 def Aligned : InheritableAttr {
   let Spellings = [GCC<"aligned">, Declspec<"align">, Keyword<"alignas">,


Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -36,6 +36,7 @@
 // CHECK-NEXT: Callback (SubjectMatchRule_function)
 // CHECK-NEXT: Capability (SubjectMatchRule_record, SubjectMatchRule_type_alias)
 // CHECK-NEXT: CarriesDependency (SubjectMatchRule_variable_is_parameter, SubjectMatchRule_objc_me

[PATCH] D65744: [PR42707][OpenCL] Fix addr space deduction for auto

[Anastasia Stulova via Phabricator via cfe-commits]

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

- Added forgotten test


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

https://reviews.llvm.org/D65744

Files:
  clang/include/clang/AST/Type.h
  clang/lib/AST/Expr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaOpenCLCXX/addrspace-auto.cl

Index: clang/test/SemaOpenCLCXX/addrspace-auto.cl
===
--- /dev/null
+++ clang/test/SemaOpenCLCXX/addrspace-auto.cl
@@ -0,0 +1,31 @@
+//RUN: %clang_cc1 %s -cl-std=clc++ -pedantic -ast-dump -verify | FileCheck %s
+
+kernel void test(){
+  int i;
+//CHECK: VarDecl {{.*}} ai 'int':'int'
+  auto ai = i;
+
+  constexpr int c = 1;
+//CHECK: VarDecl {{.*}} used cai '__constant int':'__constant int'
+  __constant auto cai = c;
+//CHECK: VarDecl {{.*}} aii 'int':'int'
+  auto aii = cai;
+
+//CHECK: VarDecl {{.*}} ref 'int &'
+  auto& ref = i;
+//CHECK: VarDecl {{.*}} ptr 'int *'
+  auto* ptr = &i;
+//CHECK: VarDecl {{.*}} ref_c '__constant int &'
+  auto& ref_c = cai;
+
+//CHECK: VarDecl {{.*}} ptrptr 'int *__generic *'
+  auto ** ptrptr = &ptr;
+//CHECK: VarDecl {{.*}} refptr 'int *__generic &'
+  auto *& refptr = ptr;
+
+//CHECK: VarDecl {{.*}} invalid gref '__global auto &'
+  __global auto& gref = i; //expected-error{{variable 'gref' with type '__global auto &' has incompatible initializer of type 'int'}}
+  __local int* ptr_l;
+//CHECK: VarDecl {{.*}} invalid gptr '__global auto *'
+  __global auto* gptr = ptr_l; //expected-error{{variable 'gptr' with type '__global auto *' has incompatible initializer of type '__local int *'}}
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -4535,14 +4535,6 @@
   return Result;
 }
 
-/// Helper to deduce addr space of a pointee type in OpenCL mode.
-/// If the type is updated it will be overwritten in PointeeType param.
-static void deduceOpenCLPointeeAddrSpace(Sema &SemaRef, QualType &PointeeType) {
-  if (PointeeType.getAddressSpace() == LangAS::Default)
-PointeeType = SemaRef.Context.getAddrSpaceQualType(PointeeType,
-   LangAS::opencl_generic);
-}
-
 template
 QualType TreeTransform::TransformPointerType(TypeLocBuilder &TLB,
   PointerTypeLoc TL) {
@@ -4551,9 +4543,6 @@
   if (PointeeType.isNull())
 return QualType();
 
-  if (SemaRef.getLangOpts().OpenCL)
-deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
-
   QualType Result = TL.getType();
   if (PointeeType->getAs()) {
 // A dependent pointer type 'T *' has is being transformed such
@@ -4592,9 +4581,6 @@
   if (PointeeType.isNull())
 return QualType();
 
-  if (SemaRef.getLangOpts().OpenCL)
-deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
-
   QualType Result = TL.getType();
   if (getDerived().AlwaysRebuild() ||
   PointeeType != TL.getPointeeLoc().getType()) {
@@ -4624,9 +4610,6 @@
   if (PointeeType.isNull())
 return QualType();
 
-  if (SemaRef.getLangOpts().OpenCL)
-deduceOpenCLPointeeAddrSpace(SemaRef, PointeeType);
-
   QualType Result = TL.getType();
   if (getDerived().AlwaysRebuild() ||
   PointeeType != T->getPointeeTypeAsWritten()) {
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1962,6 +1962,18 @@
   return true;
 }
 
+// Helper to deduce addr space of a pointee type in OpenCL mode.
+static QualType deduceOpenCLPointeeAddrSpace(Sema &S, QualType PointeeType) {
+  if (!PointeeType->isUndeducedAutoType() && !PointeeType->isDependentType() &&
+  !PointeeType.getQualifiers().hasAddressSpace())
+PointeeType = S.getASTContext().getAddrSpaceQualType(
+PointeeType,
+S.getLangOpts().OpenCLCPlusPlus || S.getLangOpts().OpenCLVersion == 200
+? LangAS::opencl_generic
+: LangAS::opencl_private);
+  return PointeeType;
+}
+
 /// Build a pointer type.
 ///
 /// \param T The type to which we'll be building a pointer.
@@ -1998,6 +2010,9 @@
   if (getLangOpts().ObjCAutoRefCount)
 T = inferARCLifetimeForPointee(*this, T, Loc, /*reference*/ false);
 
+  if (getLangOpts().OpenCL)
+T = deduceOpenCLPointeeAddrSpace(*this, T);
+
   // Build the pointer type.
   return Context.getPointerType(T);
 }
@@ -2058,6 +2073,9 @@
   if (getLangOpts().ObjCAutoRefCount)
 T = inferARCLifetimeForPointee(*this, T, Loc, /*reference*/ true);
 
+  if (getLangOpts().OpenCL)
+T = deduceOpenCLPointeeAddrSpace(*this, T);
+
   // Handle restrict on references.
   if (LValueRef)
 return Context.getLValueReferenceType(T, SpelledAsLValue);
@@ -2626,6 +2644,9 @@
   if (checkQualifiedFunction(*this, T, Loc, QFK_BlockPointer))
 return QualType();
 
+  if (getL

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

[Louis Dionne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370891: [libc++] Add `__truncating_cast` for safely casting 
float types to integers (authored by ldionne, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66836?vs=218149&id=218664#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66836

Files:
  libcxx/trunk/include/math.h
  libcxx/trunk/test/libcxx/numerics/clamp_to_integral.pass.cpp

Index: libcxx/trunk/include/math.h
===
--- libcxx/trunk/include/math.h
+++ libcxx/trunk/include/math.h
@@ -1553,6 +1553,40 @@
 typename std::enable_if::value, double>::type
 trunc(_A1 __lcpp_x) _NOEXCEPT {return ::trunc((double)__lcpp_x);}
 
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+template ::digits > numeric_limits<_IntT>::digits),
+int _Bits = (numeric_limits<_IntT>::digits - numeric_limits<_FloatT>::digits)>
+_LIBCPP_INLINE_VISIBILITY
+_LIBCPP_CONSTEXPR _IntT __max_representable_int_for_float() _NOEXCEPT {
+  static_assert(is_floating_point<_FloatT>::value, "must be a floating point type");
+  static_assert(is_integral<_IntT>::value, "must be an integral type");
+  static_assert(numeric_limits<_FloatT>::radix == 2, "FloatT has incorrect radix");
+  static_assert(_IsSame<_FloatT, float>::value || _IsSame<_FloatT, double>::value
+   || _IsSame<_FloatT,long double>::value, "unsupported floating point type");
+  return _FloatBigger ? numeric_limits<_IntT>::max() :  (numeric_limits<_IntT>::max() >> _Bits << _Bits);
+}
+
+// Convert a floating point number to the specified integral type after
+// clamping to the integral types representable range.
+//
+// The behavior is undefined if `__r` is NaN.
+template 
+_LIBCPP_INLINE_VISIBILITY
+_IntT __clamp_to_integral(_RealT __r) _NOEXCEPT {
+  using _Lim = std::numeric_limits<_IntT>;
+  const _IntT _MaxVal = std::__max_representable_int_for_float<_IntT, _RealT>();
+  if (__r >= ::nextafter(static_cast<_RealT>(_MaxVal), INFINITY)) {
+return _Lim::max();
+  } else if (__r <= _Lim::lowest()) {
+return _Lim::min();
+  }
+  return static_cast<_IntT>(__r);
+}
+
+_LIBCPP_END_NAMESPACE_STD
+
 } // extern "C++"
 
 #endif // __cplusplus
Index: libcxx/trunk/test/libcxx/numerics/clamp_to_integral.pass.cpp
===
--- libcxx/trunk/test/libcxx/numerics/clamp_to_integral.pass.cpp
+++ libcxx/trunk/test/libcxx/numerics/clamp_to_integral.pass.cpp
@@ -0,0 +1,90 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// __clamp_to_integral(RealT)
+
+// Test the conversion function that truncates floating point types to the
+// closest representable value for the specified integer type, or
+// numeric_limits::max()/min() if the value isn't representable.
+
+#include 
+#include 
+#include 
+
+template 
+void test() {
+  typedef std::numeric_limits Lim;
+  const bool MaxIsRepresentable = sizeof(IntT) < 8;
+  const bool IsSigned = std::is_signed::value;
+  struct TestCase {
+double Input;
+IntT Expect;
+bool IsRepresentable;
+  } TestCases[] = {
+  {0, 0, true},
+  {1, 1, true},
+  {IsSigned ? static_cast(-1) : 0,
+   IsSigned ? static_cast(-1) : 0, true},
+  {Lim::lowest(), Lim::lowest(), true},
+  {static_cast(Lim::max()), Lim::max(), MaxIsRepresentable},
+  {static_cast(Lim::max()) + 1, Lim::max(), false},
+  {static_cast(Lim::max()) + 1024, Lim::max(), false},
+  {nextafter(static_cast(Lim::max()), INFINITY), Lim::max(), false},
+  };
+  for (TestCase TC : TestCases) {
+auto res = std::__clamp_to_integral(TC.Input);
+assert(res == TC.Expect);
+if (TC.IsRepresentable) {
+  auto other = static_cast(std::trunc(TC.Input));
+  assert(res == other);
+} else
+  assert(res == Lim::min() || res == Lim::max());
+  }
+}
+
+template 
+void test_float() {
+  typedef std::numeric_limits Lim;
+  const bool MaxIsRepresentable = sizeof(IntT) < 4;
+  ((void)MaxIsRepresentable);
+  const bool IsSigned = std::is_signed::value;
+  struct TestCase {
+float Input;
+IntT Expect;
+bool IsRepresentable;
+  } TestCases[] = {
+  {0, 0, true},
+  {1, 1, true},
+  {IsSigned ? static_cast(-1) : 0,
+   IsSigned ? static_cast(-1) : 0, true},
+  {Lim::lowest(), Lim::lowest(), true},
+  {static_cast(Lim::max()), Lim::max(), MaxIsRepresentable },
+   {nextafter(static_cast(Lim::max()), INFINITY), Lim::max(), false},
+  };
+  for (TestCase TC : TestCases) {
+auto 

[PATCH] D67160: [clang, ARM] Default to -fno-lax-vector-conversions in ARM v8.1-M.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham created this revision.
simon_tatham added reviewers: dmgreen, miyuki, ostannard.
Herald added subscribers: cfe-commits, kristof.beyls, javed.absar.
Herald added a project: clang.

The ACLE intrinsics for the MVE instruction set have polymorphic
variants: you can write vaddq(v,w) and have it automatically select
vaddq_s32, vaddq_u8, vaddq_f16, ... according to the types of the
vector variables v and w.

In order to implement this using __attribute__((overloadable)), I need
to turn on strict type-checking between the different vector types, or
else clang will believe that any vector type matches _all_ of those
function prototypes, and won't be able to select the right one.

Also, I think this makes sense for MVE anyway, on error-checking
grounds. For NEON, where every intrinsic in C source explicitly
describes the operation it's performing, you could argue that the
distinct vector types aren't essential; but in a context where getting
the wrong type can cause silent generation of the _wrong_ code, I
think it's better to warn users as soon as possible if they've written
code that isn't using vector types consistently.

So this commit introduces a (minimal) piece of infrastructure to allow
the default for OPT_flax_vector_conversions to vary based on the
target triple, and uses it to set the default specially for
ARMSubArch_v8_1m_mainline (in which the only possible vector
architecture will be MVE).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67160

Files:
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1357,6 +1357,17 @@
   }
 }
 
+static bool isLaxVectorConversionsDefault(const llvm::Triple &Triple) {
+  switch (Triple.getArch()) {
+  default:
+return true;
+
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+return Triple.getSubArch() != llvm::Triple::ARMSubArch_v8_1m_mainline;
+  }
+}
+
 static bool isNoCommonDefault(const llvm::Triple &Triple) {
   switch (Triple.getArch()) {
   default:
@@ -4674,10 +4685,13 @@
   if (TC.SupportsProfiling())
 Args.AddLastArg(CmdArgs, options::OPT_mfentry);
 
-  // -flax-vector-conversions is default.
-  if (!Args.hasFlag(options::OPT_flax_vector_conversions,
-options::OPT_fno_lax_vector_conversions))
+  if (const Arg *A = Args.getLastArg(options::OPT_flax_vector_conversions,
+options::OPT_fno_lax_vector_conversions)) {
+if (A->getOption().matches(options::OPT_fno_lax_vector_conversions))
+  CmdArgs.push_back("-fno-lax-vector-conversions");
+  } else if (!isLaxVectorConversionsDefault(Triple)) {
 CmdArgs.push_back("-fno-lax-vector-conversions");
+  }
 
   if (Args.getLastArg(options::OPT_fapple_kext) ||
   (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1357,6 +1357,17 @@
   }
 }
 
+static bool isLaxVectorConversionsDefault(const llvm::Triple &Triple) {
+  switch (Triple.getArch()) {
+  default:
+return true;
+
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+return Triple.getSubArch() != llvm::Triple::ARMSubArch_v8_1m_mainline;
+  }
+}
+
 static bool isNoCommonDefault(const llvm::Triple &Triple) {
   switch (Triple.getArch()) {
   default:
@@ -4674,10 +4685,13 @@
   if (TC.SupportsProfiling())
 Args.AddLastArg(CmdArgs, options::OPT_mfentry);
 
-  // -flax-vector-conversions is default.
-  if (!Args.hasFlag(options::OPT_flax_vector_conversions,
-options::OPT_fno_lax_vector_conversions))
+  if (const Arg *A = Args.getLastArg(options::OPT_flax_vector_conversions,
+options::OPT_fno_lax_vector_conversions)) {
+if (A->getOption().matches(options::OPT_fno_lax_vector_conversions))
+  CmdArgs.push_back("-fno-lax-vector-conversions");
+  } else if (!isLaxVectorConversionsDefault(Triple)) {
 CmdArgs.push_back("-fno-lax-vector-conversions");
+  }
 
   if (Args.getLastArg(options::OPT_fapple_kext) ||
   (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67159: [clang] New __attribute__((__clang_builtin)).

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

It somehow doesn't seem entirely good to provide a way to mark arbitrary 
function in sources as a builtin..
Why can't those MVE builtins be implemented similarly like all the existing 
builtins?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67159



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

[clang-tools-extra] r370895 - [clangd] Add TUScheduler.h to CodeComplete.cpp to unbreak builds

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Wed Sep  4 06:04:34 2019
New Revision: 370895

URL: http://llvm.org/viewvc/llvm-project?rev=370895&view=rev
Log:
[clangd] Add TUScheduler.h to CodeComplete.cpp to unbreak builds

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

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=370895&r1=370894&r2=370895&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Sep  4 06:04:34 2019
@@ -31,6 +31,7 @@
 #include "Protocol.h"
 #include "Quality.h"
 #include "SourceCode.h"
+#include "TUScheduler.h"
 #include "Threading.h"
 #include "Trace.h"
 #include "URI.h"


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

[PATCH] D67058: [clang][CodeGen] Add alias for cpu_dispatch function with IFunc

[Sr.Zhang via Phabricator via cfe-commits]

zsrkmyn added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2957
+if (!AliasFunc) {
+  auto *IFunc = cast(GetOrCreateLLVMFunction(
+  AliasName, DeclTy, GD, /*ForVTable=*/false, /*DontDefer=*/true,

erichkeane wrote:
> erichkeane wrote:
> > I think we want this in GetOrCreateMultiVersionResolver, so that it gets 
> > created when the ifunc does.  That way you just need a 
> > "FD->isCPUDispatchMultiVersion() || isCPUSpecificMultiVersion()" check 
> > inside the supportsIFunc branch.
> After discussing this offline, I believe this is the right function to create 
> the alias.  The motivating example is:
> 
> // TU1:
> __attribute__((cpu_dispatch(a,b,c))) void foo(void);
> 
> // TU2:
> extern void foo(void);
> 
> Currently, TU1 doesn't bother to emit the ifunc, because we've attached 
> emitting this to when this is referenced.
> 
> We made that choice because we expected TU2 to mark 'foo' as 
> cpu_dispatch/cpu_specific in SOME way.  I believe that it is harmless to emit 
> the ifunc all the time, which this is attempting to do.  However, this needs 
> to change the ifunc to have LinkOnceODR linkage in 
> GetOrCreateMultiVersionResolver, otherwise this can cause linker errors.
> 
> 
I've finished most parts. But I think we should also set the resolver to have 
LinkOnceODR Linkage. Otherwise, we cannot have the cpu_dispatch declaration in 
multiple TUs.

However there's no 'weak' symbol on Windows, so even setting the resolver 
linkage to LinkOnceODR cannot solve the duplicate defined symbol problem on 
Windows. Do you have any suggestions on it? :-)


Repository:
  rC Clang

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

https://reviews.llvm.org/D67058



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

[PATCH] D67163: [Driver] Use shared singleton instance of DriverOptTable

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: gribozavr, sammccall.
Herald added a subscriber: kadircet.
Herald added a project: clang.

This significantly reduces the time required to run clangd tests, by
~10%.

Should also have an effect on other tests that run command-line parsing
multiple times inside a single invocation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67163

Files:
  clang-tools-extra/modularize/Modularize.cpp
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/DriverOptions.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/tools/clang-check/ClangCheck.cpp
  clang/tools/driver/cc1as_main.cpp
  clang/tools/driver/driver.cpp

Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -271,10 +271,9 @@
 static DiagnosticOptions *
 CreateAndPopulateDiagOpts(ArrayRef argv) {
   auto *DiagOpts = new DiagnosticOptions;
-  std::unique_ptr Opts(createDriverOptTable());
   unsigned MissingArgIndex, MissingArgCount;
-  InputArgList Args =
-  Opts->ParseArgs(argv.slice(1), MissingArgIndex, MissingArgCount);
+  InputArgList Args = getDriverOptTable().ParseArgs(
+  argv.slice(1), MissingArgIndex, MissingArgCount);
   // We ignore MissingArgCount and the return value of ParseDiagnosticArgs.
   // Any errors that would be diagnosed here will also be diagnosed later,
   // when the DiagnosticsEngine actually exists.
Index: clang/tools/driver/cc1as_main.cpp
===
--- clang/tools/driver/cc1as_main.cpp
+++ clang/tools/driver/cc1as_main.cpp
@@ -176,12 +176,12 @@
   bool Success = true;
 
   // Parse the arguments.
-  std::unique_ptr OptTbl(createDriverOptTable());
+  const OptTable &OptTbl = getDriverOptTable();
 
   const unsigned IncludedFlagsBitmask = options::CC1AsOption;
   unsigned MissingArgIndex, MissingArgCount;
-  InputArgList Args = OptTbl->ParseArgs(Argv, MissingArgIndex, MissingArgCount,
-IncludedFlagsBitmask);
+  InputArgList Args = OptTbl.ParseArgs(Argv, MissingArgIndex, MissingArgCount,
+   IncludedFlagsBitmask);
 
   // Check for missing argument error.
   if (MissingArgCount) {
@@ -194,7 +194,7 @@
   for (const Arg *A : Args.filtered(OPT_UNKNOWN)) {
 auto ArgString = A->getAsString(Args);
 std::string Nearest;
-if (OptTbl->findNearest(ArgString, Nearest, IncludedFlagsBitmask) > 1)
+if (OptTbl.findNearest(ArgString, Nearest, IncludedFlagsBitmask) > 1)
   Diags.Report(diag::err_drv_unknown_argument) << ArgString;
 else
   Diags.Report(diag::err_drv_unknown_argument_with_suggestion)
@@ -574,11 +574,11 @@
 return 1;
 
   if (Asm.ShowHelp) {
-std::unique_ptr Opts(driver::createDriverOptTable());
-Opts->PrintHelp(llvm::outs(), "clang -cc1as [options] file...",
-"Clang Integrated Assembler",
-/*Include=*/driver::options::CC1AsOption, /*Exclude=*/0,
-/*ShowAllAliases=*/false);
+getDriverOptTable().PrintHelp(
+llvm::outs(), "clang -cc1as [options] file...",
+"Clang Integrated Assembler",
+/*Include=*/driver::options::CC1AsOption, /*Exclude=*/0,
+/*ShowAllAliases=*/false);
 return 0;
   }
 
Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -52,31 +52,34 @@
 );
 
 static cl::OptionCategory ClangCheckCategory("clang-check options");
-static std::unique_ptr Options(createDriverOptTable());
+static const opt::OptTable &Options = getDriverOptTable();
 static cl::opt
-ASTDump("ast-dump", cl::desc(Options->getOptionHelpText(options::OPT_ast_dump)),
-cl::cat(ClangCheckCategory));
+ASTDump("ast-dump",
+cl::desc(Options.getOptionHelpText(options::OPT_ast_dump)),
+cl::cat(ClangCheckCategory));
 static cl::opt
-ASTList("ast-list", cl::desc(Options->getOptionHelpText(options::OPT_ast_list)),
-cl::cat(ClangCheckCategory));
+ASTList("ast-list",
+cl::desc(Options.getOptionHelpText(options::OPT_ast_list)),
+cl::cat(ClangCheckCategory));
 static cl::opt
-ASTPrint("ast-print",
- cl::desc(Options->getOptionHelpText(options::OPT_ast_print)),
- cl::cat(ClangCheckCategory));
+ASTPrint("ast-print",
+ cl::desc(Options.getOptionHelpText(options::OPT_ast_print)),
+ cl::cat(ClangCheckCategory));
 static cl::opt ASTDumpFilter(
 "ast-dump-filter",
-cl::desc(Options->getOptionHelpText(options::OPT_ast

[PATCH] D67159: [clang] New __attribute__((__clang_builtin)).

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added a comment.

Sorry about that – I didn't want to put the discussion of rationale in too many 
different places. The commit message for the followup patch D67161 
 discusses it a bit.

The usual thing in previous intrinsics systems like NEON is that you have a 
wrapper function in the header file which calls the builtin. That already 
doesn't work in every situation, because some intrinsics need one of their 
arguments to be a compile-time integer constant expression, and with a wrapper 
function in the way, the constantness property is lost between the user's call 
site and the builtin. So then you implement //those// builtins as macros 
instead of wrapper functions.

The MVE intrinsics spec adds the wrinkle that there are polymorphic functions – 
with more concise names, overloaded on the argument types. For example, 
`vaddq(v,w)` can behave like `vaddq_s32` or `vaddq_f16` or whatever, depending 
on what vector types you give it. Those have to be implemented using either 
`__attribute__((overloadable))` or `_Generic`.

`__attribute__((overloadable))` works fine for wrapper functions in the header 
file, but not for that awkward subset of the functions that have to fall back 
to being macros. For those, you //have// to use `_Generic` (if you're not 
willing to do what I've done here).

`_Generic` is immensely awkward, because the difficult thing about it is that 
all the selection branches //not// taken still have to type-check successfully. 
So you can't just say something like 'if this is a `uint16x8_t` then expand to 
`vfooq_u16`, else `vfooq_f16`, etc', because all but one of those will be type 
errors. Instead you have to pile in endless workarounds in which each branch of 
the `_Generic` is filled with sub-Generics that coerce the arguments in untaken 
branches to something that is nonsense but won't cause a type-check error – and 
//hope// that none of your 'replace it with validly typed nonsense' workarounds 
won't accidentally trigger in any case where the branch //is// taken.

Also, if you need to disambiguate based on //more// than one of the argument 
types (which can happen in this spec), then you have the same problem again: 
you can't nest a `_Generic` switching on argument 2 inside each branch of an 
outer one switching on argument 1, because as soon as the set of legal type 
pairs isn't a full Cartesian product, the inner Generic of an untaken branch 
fails to type-check again, so you need yet more layers of workaround.

And after you've done all that, the error reporting is //hideous//. It's almost 
as bad as those C++ error messages gcc used to generate in which it had 
expanded out some STL type in the user's code into three whole pages of 
library-internals nonsense.

Doing it //this// way, what happens is that first clang resolves the 
`__attribute__((overloadable))` based on all the argument types at once; then, 
once it's decided which function declaration is the interesting one, it looks 
up the `BuiltinId` that I put there using this mechanism; and then it's looking 
at a call directly to that builtin from the user's code, so it can check 
constant arguments at their original call site. It needs almost no code; no 
enormous chain of ugly workarounds; and if the user passes an invalid list of 
argument types, the error report is //beautiful//: clang will show you each 
declaration of the polymorphic name from the header file, and legibly explain 
why each one doesn't match the arguments the user actually passed.

So, I hear you: this is something clang has always found a way to do without 
before. But if I can't do it this way, could you suggest a way that I could get 
anything like that really nice error reporting, under all those constraints at 
once?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67159



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

[PATCH] D67057: [AST][JSON] Generate parent context id from Decl* instead of DeclContext*

[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.

LGTM aside from a small nit. Thank you for this!




Comment at: clang/lib/AST/JSONNodeDumper.cpp:118
+// same AST Node.
+auto ParentDeclContextDecl = dyn_cast(D->getDeclContext());
+JOS.attribute("parentDeclContextId",

`const auto *`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67057



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

[PATCH] D67058: [clang][CodeGen] Add alias for cpu_dispatch function with IFunc

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a subscriber: rnk.
erichkeane added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2957
+if (!AliasFunc) {
+  auto *IFunc = cast(GetOrCreateLLVMFunction(
+  AliasName, DeclTy, GD, /*ForVTable=*/false, /*DontDefer=*/true,

zsrkmyn wrote:
> erichkeane wrote:
> > erichkeane wrote:
> > > I think we want this in GetOrCreateMultiVersionResolver, so that it gets 
> > > created when the ifunc does.  That way you just need a 
> > > "FD->isCPUDispatchMultiVersion() || isCPUSpecificMultiVersion()" check 
> > > inside the supportsIFunc branch.
> > After discussing this offline, I believe this is the right function to 
> > create the alias.  The motivating example is:
> > 
> > // TU1:
> > __attribute__((cpu_dispatch(a,b,c))) void foo(void);
> > 
> > // TU2:
> > extern void foo(void);
> > 
> > Currently, TU1 doesn't bother to emit the ifunc, because we've attached 
> > emitting this to when this is referenced.
> > 
> > We made that choice because we expected TU2 to mark 'foo' as 
> > cpu_dispatch/cpu_specific in SOME way.  I believe that it is harmless to 
> > emit the ifunc all the time, which this is attempting to do.  However, this 
> > needs to change the ifunc to have LinkOnceODR linkage in 
> > GetOrCreateMultiVersionResolver, otherwise this can cause linker errors.
> > 
> > 
> I've finished most parts. But I think we should also set the resolver to have 
> LinkOnceODR Linkage. Otherwise, we cannot have the cpu_dispatch declaration 
> in multiple TUs.
> 
> However there's no 'weak' symbol on Windows, so even setting the resolver 
> linkage to LinkOnceODR cannot solve the duplicate defined symbol problem on 
> Windows. Do you have any suggestions on it? :-)
Yep, I think the resolver should also be linkonceodr (as well as the ifunc).  
See where they are generated and do it there.

I can't help but think that we've solved the weak symbols issue with windows 
before, but I cannot for the life of me remember.  @rnk  @lebedev.ri , do 
either of you remember?  Does LinkOnceODR not do that trickery?


Repository:
  rC Clang

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

https://reviews.llvm.org/D67058



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

[PATCH] D67165: [clangd][vscode] Don't register notifications multiple times when clangd crashes.

[Haojian Wu via Phabricator via cfe-commits]

hokein planned changes to this revision.
hokein added a comment.

wait, there is still an issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67165



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

[PATCH] D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks"

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a reviewer: aaron.ballman.
alexfh added a comment.

In D67140#1656858 , @gribozavr wrote:

> Thanks! Yay consistency.
>
> I prefer the term "checker" to refer to individual modules because I feel it 
> is more precise and less ambiguous. In phrases like "malloc check", 
> "make_unique check", it is unclear what does the check -- malloc itself, the 
> caller of malloc, or something else. Therefore, I would be supportive of 
> repainting ClangTidy to also use "checker", but I would want to know what 
> @alexfh thinks about it before we do it in ClangTidy.


Historically, clang-tidy only used the term "check" (to denote the thing that 
checks something, rather than the rule being checked or the act of checking), 
and we tried to keep its use consistent. However, "checker" is a more precise 
and less ambiguous way to convey this meaning. I support to use the term 
"checker" in clang-tidy, as long as someone is willing to update the code and 
documentation (except for verbs, e.g. the `check()` method ;). Also note that 
there's a non-trivial number of out-of-tree check(er)s out there. They will 
need to be updated as well.

Adding Aaron in case he has a different opinion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67140



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

[PATCH] D66990: [clangd] Add distinct highlightings for declarations of functions and methods

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D66990#1656230 , @nridge wrote:

> Not a hard requirement, just a nice-to-have for someone moving from one tool 
> to another :)
>  If you feel that for now it's better not to do this, I can respect that.


Thanks, if that works for you, I would wait until we get/build a better support 
in the protocol for this.
If Eclipse CDT gets a lot of user complaints and this turns out to be an 
important missing feature, happy to reconsider. 
FWIW, I find having Eclipse CDT on board to be important for us and therefore 
happy to make trade-offs when needed.
This is a small thing, though, so waiting for an explicit support for this in 
the protocol does not sound too bad.

> I will suggest this for the upstream protocol and see where that goes.

SG, keep us posted!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66990



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

[PATCH] D67165: [clangd][vscode] Don't register notifications multiple times when clangd crashes.

[Haojian Wu via Phabricator via cfe-commits]

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

wait, there is still an issue.


Previously, we registered the notification everytime when the client state is
changed to running, which is unnecessary.
This patch also makes the semantic highlighting feature more
self-contained (align with the implementation of vscode builtin features)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67165

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
  clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts


Index: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
@@ -44,7 +44,7 @@
 
 // Language server push notification providing the semantic highlighting
 // information for a text document.
-export const NotificationType =
+const NotificationType =
 new vscodelc.NotificationType(
 'textDocument/semanticHighlighting');
 
@@ -58,6 +58,11 @@
   highlighter: Highlighter;
   // Any disposables that should be cleaned up when clangd crashes.
   private subscriptions: vscode.Disposable[] = [];
+  private clangdClient: vscodelc.BaseLanguageClient;
+
+  constructor(client: vscodelc.BaseLanguageClient) {
+this.clangdClient = client;
+  }
   fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {
 // Extend the ClientCapabilities type and add semantic highlighting
 // capability to the object.
@@ -77,7 +82,9 @@
   }
 
   initialize(capabilities: vscodelc.ServerCapabilities,
- documentSelector: vscodelc.DocumentSelector|undefined) {
+documentSelector: vscodelc.DocumentSelector | undefined) {
+// Register the handler to handle semantic highlighting notification.
+this.clangdClient.onNotification(NotificationType, 
this.handleNotification);
 // The semantic highlighting capability information is in the capabilities
 // object but to access the data we must first extend the 
ServerCapabilities
 // type.
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
@@ -110,7 +110,7 @@
   const clangdClient = new ClangdLanguageClient('Clang Language Server',
 serverOptions, clientOptions);
   const semanticHighlightingFeature =
-  new semanticHighlighting.SemanticHighlightingFeature();
+  new semanticHighlighting.SemanticHighlightingFeature(clangdClient);
   context.subscriptions.push(
   vscode.Disposable.from(semanticHighlightingFeature));
   clangdClient.registerFeature(semanticHighlightingFeature);
@@ -138,17 +138,15 @@
   context.subscriptions.push(vscode.Disposable.from(status));
   context.subscriptions.push(vscode.window.onDidChangeActiveTextEditor(
   () => { status.updateStatus(); }));
-  context.subscriptions.push(clangdClient.onDidChangeState(({newState}) => {
-if (newState == vscodelc.State.Running) {
-  // clangd starts or restarts after crash.
-  clangdClient.onNotification(
+
+  // The notification handler must be registered after the client is ready.
+  clangdClient.onReady().then(
+  () => {clangdClient.onNotification(
   'textDocument/clangd.fileStatus',
-  (fileStatus) => { status.onFileUpdated(fileStatus); });
-  clangdClient.onNotification(
-  semanticHighlighting.NotificationType,
-  semanticHighlightingFeature.handleNotification.bind(
-  semanticHighlightingFeature));
-} else if (newState == vscodelc.State.Stopped) {
+  (fileStatus) => { status.onFileUpdated(fileStatus); })});
+
+  context.subscriptions.push(clangdClient.onDidChangeState(({newState}) => {
+if (newState == vscodelc.State.Stopped) {
   // Clear all cached statuses when clangd crashes.
   status.clear();
   semanticHighlightingFeature.dispose();


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
@@ -44,7 +44,7 @@
 
 // Language server push notification providing the semantic highlighting
 // information for a text document.
-export const NotificationType =
+const NotificationType =
 new vscodelc.NotificationType(
 'textDocument/semanticHighlighting');
 
@@

[PATCH] D67058: [clang][CodeGen] Add alias for cpu_dispatch function with IFunc

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2957
+if (!AliasFunc) {
+  auto *IFunc = cast(GetOrCreateLLVMFunction(
+  AliasName, DeclTy, GD, /*ForVTable=*/false, /*DontDefer=*/true,

erichkeane wrote:
> zsrkmyn wrote:
> > erichkeane wrote:
> > > erichkeane wrote:
> > > > I think we want this in GetOrCreateMultiVersionResolver, so that it 
> > > > gets created when the ifunc does.  That way you just need a 
> > > > "FD->isCPUDispatchMultiVersion() || isCPUSpecificMultiVersion()" check 
> > > > inside the supportsIFunc branch.
> > > After discussing this offline, I believe this is the right function to 
> > > create the alias.  The motivating example is:
> > > 
> > > // TU1:
> > > __attribute__((cpu_dispatch(a,b,c))) void foo(void);
> > > 
> > > // TU2:
> > > extern void foo(void);
> > > 
> > > Currently, TU1 doesn't bother to emit the ifunc, because we've attached 
> > > emitting this to when this is referenced.
> > > 
> > > We made that choice because we expected TU2 to mark 'foo' as 
> > > cpu_dispatch/cpu_specific in SOME way.  I believe that it is harmless to 
> > > emit the ifunc all the time, which this is attempting to do.  However, 
> > > this needs to change the ifunc to have LinkOnceODR linkage in 
> > > GetOrCreateMultiVersionResolver, otherwise this can cause linker errors.
> > > 
> > > 
> > I've finished most parts. But I think we should also set the resolver to 
> > have LinkOnceODR Linkage. Otherwise, we cannot have the cpu_dispatch 
> > declaration in multiple TUs.
> > 
> > However there's no 'weak' symbol on Windows, so even setting the resolver 
> > linkage to LinkOnceODR cannot solve the duplicate defined symbol problem on 
> > Windows. Do you have any suggestions on it? :-)
> Yep, I think the resolver should also be linkonceodr (as well as the ifunc).  
> See where they are generated and do it there.
> 
> I can't help but think that we've solved the weak symbols issue with windows 
> before, but I cannot for the life of me remember.  @rnk  @lebedev.ri , do 
> either of you remember?  Does LinkOnceODR not do that trickery?
There must be SOME solution for it, since otherwise inline functions wouldn't 
work.  For example:

https://godbolt.org/z/RjB9Xb

Its totally permissible to define and use 'foo::bar' in multiple TUs.  Note 
that it is marked linkonce_odr dso_local and comdat.  I'm not sure what the 
latter two do, but please experiment and see which will allow the symbol to be 
merged in the linker.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67058



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

[PATCH] D67160: [clang, ARM] Default to -fno-lax-vector-conversions in ARM v8.1-M.

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

ostannard requested changes to this revision.
ostannard added a comment.
This revision now requires changes to proceed.

Test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67160



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

[PATCH] D67084: [clang-tidy] Fix bugprone-argument-comment bug: negative literal number is not checked.

[Yubo Xie via Phabricator via cfe-commits]

xyb added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp:240
+  Arg = Arg->IgnoreImpCasts();
+  if (isa(Arg))
+  Arg = cast(Arg)->getSubExpr();

aaron.ballman wrote:
> The bug claims that this is only for handling negative literals, but this 
> allows any unary operator. Is that intentional? If we're going to allow 
> arbitrary unary operators, why not arbitrary binary operators as well?
Actually, it handles "UnaryOperator Literal". So it will handle "+12", "-12", 
"~12". I can restrict it for UO_MINUS only, but is it real necessary? 


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67084



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

[PATCH] D67084: [clang-tidy] Fix bugprone-argument-comment bug: negative literal number is not checked.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Please add full context to the diff. See 
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67084



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

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

[Nandor Licker via Phabricator via cfe-commits]

nand added a comment.

Does anyone have any suggestions on how to fix the MSVC problems? I am trying 
to get access to a Windows machine, but it's not guaranteed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64146



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

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

It looks to me like the patch does things the New Way only for Linux and 
NetBSD, so for PS4 backward compatibility I am okay with it.


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

https://reviews.llvm.org/D64672



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

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D64146#1657473 , @nand wrote:

> Merged clangInterp and clangAST into a single library while keeping Interp in 
> a separate folder.
>  This is required since clangInterp needs access to the AST, but the intry 
> points to the interpreter
>  are also in AST.


I don't have an actionable alternative suggestion but this seems like 
counter-correct layering, trying to paper over the issue instead of fixing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64146



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

[PATCH] D67080: [clang-tidy] Fix bugprone-argument-comment bug if there are marcos.

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG. Thanks!




Comment at: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp:326
+  CharSourceRange ArgsRange =
+  MakeFileCharRange(Args[I]->getBeginLoc(), Args[I]->getEndLoc());
   Comments = getCommentsBeforeLoc(Ctx, ArgsRange.getBegin());

I don't know what motivation was to try parsing to the end of the arguments.  
But since no tests break, it should be fine to change.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67080



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

[PATCH] D67056: Add a bugprone-argument-comment option: IgnoreSingleArgument.

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG with a comment.




Comment at: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp:28
+  IgnoreSingleArgument(
+  Options.getLocalOrGlobal("IgnoreSingleArgument", 0) != 0),
   CommentBoolLiterals(Options.getLocalOrGlobal("CommentBoolLiterals", 0) !=

Why is it a global option? Are there other checks that would need the same 
option? The one below also needs to be check-local.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67056



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

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

[Nandor Licker via Phabricator via cfe-commits]

nand added a comment.

The existing evaluator is not a separate library to start with. Given the 
tangling between ExprConstants.cpp and the AST nodes, I don't really see any 
elegant way of dealing with this problem.

An example of the problem is `FieldDecl::getBitWidthValue`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64146



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

[PATCH] D67084: [clang-tidy] Fix bugprone-argument-comment bug: negative literal number is not checked.

[Yubo Xie via Phabricator via cfe-commits]

xyb updated this revision to Diff 218683.
xyb added a comment.

Update with full diff.


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

https://reviews.llvm.org/D67084

Files:
  clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tools-extra/test/clang-tidy/bugprone-argument-comment-literals.cpp


Index: clang-tools-extra/test/clang-tidy/bugprone-argument-comment-literals.cpp
===
--- clang-tools-extra/test/clang-tidy/bugprone-argument-comment-literals.cpp
+++ clang-tools-extra/test/clang-tidy/bugprone-argument-comment-literals.cpp
@@ -67,18 +67,29 @@
   // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for 
literal argument 'fabc' [bugprone-argument-comment]
   // CHECK-FIXES: a.foo(/*fabc=*/1.0f);
 
+  a.foo(-1.0f);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for 
literal argument 'fabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*fabc=*/-1.0f);
+
   a.foo(1.0);
   // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for 
literal argument 'dabc' [bugprone-argument-comment]
   // CHECK-FIXES: a.foo(/*dabc=*/1.0);
 
+  a.foo(-1.0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for 
literal argument 'dabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*dabc=*/-1.0);
+
   int val3 = 10;
   a.foo(val3);
+  a.foo(-val3);
 
   float val4 = 10.0;
   a.foo(val4);
+  a.foo(-val4);
 
   double val5 = 10.0;
   a.foo(val5);
+  a.foo(-val5);
 
   a.foo("Hello World");
   // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for 
literal argument 'strabc' [bugprone-argument-comment]
@@ -96,14 +107,22 @@
   // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for 
literal argument 'dabc' [bugprone-argument-comment]
   // CHECK-FIXES: a.foo(/*dabc=*/402.0_km);
 
+  a.foo(-402.0_km);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for 
literal argument 'dabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*dabc=*/-402.0_km);
+
   a.foo('A');
   // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for 
literal argument 'chabc' [bugprone-argument-comment]
   // CHECK-FIXES: a.foo(/*chabc=*/'A');
 
   g(FOO);
+  g(-FOO);
   h(1.0f);
   // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for 
literal argument 'b' [bugprone-argument-comment]
   // CHECK-FIXES: h(/*b=*/1.0f);
+  h(-1.0f);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for 
literal argument 'b' [bugprone-argument-comment]
+  // CHECK-FIXES: h(/*b=*/-1.0f);
   i(__FILE__);
 
   // FIXME Would like the below to add argument comments.
Index: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -230,9 +230,11 @@
 // Given the argument type and the options determine if we should
 // be adding an argument comment.
 bool ArgumentCommentCheck::shouldAddComment(const Expr *Arg) const {
+  Arg = Arg->IgnoreImpCasts();
+  if (isa(Arg))
+Arg = cast(Arg)->getSubExpr();
   if (Arg->getExprLoc().isMacroID())
 return false;
-  Arg = Arg->IgnoreImpCasts();
   return (CommentBoolLiterals && isa(Arg)) ||
  (CommentIntegerLiterals && isa(Arg)) ||
  (CommentFloatLiterals && isa(Arg)) ||


Index: clang-tools-extra/test/clang-tidy/bugprone-argument-comment-literals.cpp
===
--- clang-tools-extra/test/clang-tidy/bugprone-argument-comment-literals.cpp
+++ clang-tools-extra/test/clang-tidy/bugprone-argument-comment-literals.cpp
@@ -67,18 +67,29 @@
   // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'fabc' [bugprone-argument-comment]
   // CHECK-FIXES: a.foo(/*fabc=*/1.0f);
 
+  a.foo(-1.0f);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'fabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*fabc=*/-1.0f);
+
   a.foo(1.0);
   // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
   // CHECK-FIXES: a.foo(/*dabc=*/1.0);
 
+  a.foo(-1.0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*dabc=*/-1.0);
+
   int val3 = 10;
   a.foo(val3);
+  a.foo(-val3);
 
   float val4 = 10.0;
   a.foo(val4);
+  a.foo(-val4);
 
   double val5 = 10.0;
   a.foo(val5);
+  a.foo(-val5);
 
   a.foo("Hello World");
   // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'strabc' [bugprone-argument-comment]
@@ -96,14 +107,22 @@
   // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal

[PATCH] D66699: [PowerPC][Altivec] Fix constant argument for vec_dss

[Jinsong Ji via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
jsji marked an inline comment as done.
Closed by commit rL370902: [PowerPC][Altivec] Fix constant argument for vec_dss 
(authored by jsji, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66699?vs=218479&id=218684#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66699

Files:
  cfe/trunk/include/clang/Basic/BuiltinsPPC.def
  cfe/trunk/lib/Headers/altivec.h
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/CodeGen/altivec-dss.c
  cfe/trunk/test/CodeGen/builtins-ppc-error.c


Index: cfe/trunk/test/CodeGen/builtins-ppc-error.c
===
--- cfe/trunk/test/CodeGen/builtins-ppc-error.c
+++ cfe/trunk/test/CodeGen/builtins-ppc-error.c
@@ -73,3 +73,8 @@
   __builtin_unpack_vector_int128(vsllli, index); //expected-error {{argument 
to '__builtin_unpack_vector_int128' must be a constant integer}}
   __builtin_unpack_vector_int128(vsllli, 5); //expected-error {{argument value 
5 is outside the valid range [0, 1]}}
 }
+
+void testDSS(int index) {
+  vec_dss(index); //expected-error {{argument to '__builtin_altivec_dss' must 
be a constant integer}}
+  vec_dss(5); //expected-error {{argument value 5 is outside the valid range 
[0, 3]}}
+}
Index: cfe/trunk/test/CodeGen/altivec-dss.c
===
--- cfe/trunk/test/CodeGen/altivec-dss.c
+++ cfe/trunk/test/CodeGen/altivec-dss.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple powerpc-linux-gnu -S -O0 -o - %s -target-feature 
+altivec | FileCheck %s
+
+// REQUIRES: powerpc-registered-target
+
+#include 
+
+// CHECK-LABEL: test1
+// CHECK: dss 
+void test1() {
+  vec_dss(1);
+}
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -3220,6 +3220,8 @@
   case PPC::BI__builtin_altivec_crypto_vshasigmad:
 return SemaBuiltinConstantArgRange(TheCall, 1, 0, 1) ||
SemaBuiltinConstantArgRange(TheCall, 2, 0, 15);
+  case PPC::BI__builtin_altivec_dss:
+return SemaBuiltinConstantArgRange(TheCall, 0, 0, 3);
   case PPC::BI__builtin_tbegin:
   case PPC::BI__builtin_tend: i = 0; l = 0; u = 1; break;
   case PPC::BI__builtin_tsr: i = 0; l = 0; u = 7; break;
Index: cfe/trunk/lib/Headers/altivec.h
===
--- cfe/trunk/lib/Headers/altivec.h
+++ cfe/trunk/lib/Headers/altivec.h
@@ -3286,9 +3286,7 @@
 
 /* vec_dss */
 
-static __inline__ void __attribute__((__always_inline__)) vec_dss(int __a) {
-  __builtin_altivec_dss(__a);
-}
+#define vec_dss __builtin_altivec_dss
 
 /* vec_dssall */
 
Index: cfe/trunk/include/clang/Basic/BuiltinsPPC.def
===
--- cfe/trunk/include/clang/Basic/BuiltinsPPC.def
+++ cfe/trunk/include/clang/Basic/BuiltinsPPC.def
@@ -55,7 +55,7 @@
 BUILTIN(__builtin_altivec_vctsxs, "V4SiV4fIi", "")
 BUILTIN(__builtin_altivec_vctuxs, "V4UiV4fIi", "")
 
-BUILTIN(__builtin_altivec_dss, "vUi", "")
+BUILTIN(__builtin_altivec_dss, "vUIi", "")
 BUILTIN(__builtin_altivec_dssall, "v", "")
 BUILTIN(__builtin_altivec_dst, "vvC*iUi", "")
 BUILTIN(__builtin_altivec_dstt, "vvC*iUi", "")


Index: cfe/trunk/test/CodeGen/builtins-ppc-error.c
===
--- cfe/trunk/test/CodeGen/builtins-ppc-error.c
+++ cfe/trunk/test/CodeGen/builtins-ppc-error.c
@@ -73,3 +73,8 @@
   __builtin_unpack_vector_int128(vsllli, index); //expected-error {{argument to '__builtin_unpack_vector_int128' must be a constant integer}}
   __builtin_unpack_vector_int128(vsllli, 5); //expected-error {{argument value 5 is outside the valid range [0, 1]}}
 }
+
+void testDSS(int index) {
+  vec_dss(index); //expected-error {{argument to '__builtin_altivec_dss' must be a constant integer}}
+  vec_dss(5); //expected-error {{argument value 5 is outside the valid range [0, 3]}}
+}
Index: cfe/trunk/test/CodeGen/altivec-dss.c
===
--- cfe/trunk/test/CodeGen/altivec-dss.c
+++ cfe/trunk/test/CodeGen/altivec-dss.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple powerpc-linux-gnu -S -O0 -o - %s -target-feature +altivec | FileCheck %s
+
+// REQUIRES: powerpc-registered-target
+
+#include 
+
+// CHECK-LABEL: test1
+// CHECK: dss 
+void test1() {
+  vec_dss(1);
+}
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -3220,6 +3220,8 @@
   case PPC::BI__builtin_altivec_crypto_vshasigmad:
 return SemaBuiltinConstantArgRange(TheCall, 1, 0, 1) ||
SemaBuiltinCon

[PATCH] D67084: [clang-tidy] Fix bugprone-argument-comment bug: negative literal number is not checked.

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG if there are no other concerns.


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

https://reviews.llvm.org/D67084



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

r370902 - [PowerPC][Altivec] Fix constant argument for vec_dss

[Jinsong Ji via cfe-commits]

Author: jsji
Date: Wed Sep  4 07:01:47 2019
New Revision: 370902

URL: http://llvm.org/viewvc/llvm-project?rev=370902&view=rev
Log:
[PowerPC][Altivec] Fix constant argument for vec_dss

Summary:
This is similar to vec_ct* in https://reviews.llvm.org/rL304205.

The argument must be a constant, otherwise instruction selection
will fail. always_inline is not enough for isel to always fold
everything away at -O0.

The fix is to turn the function into macros in altivec.h.

Fixes https://bugs.llvm.org/show_bug.cgi?id=43072

Reviewers: nemanjai, hfinkel, #powerpc, wuzish

Reviewed By: #powerpc, wuzish

Subscribers: wuzish, kbarton, MaskRay, shchenz, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/CodeGen/altivec-dss.c
Modified:
cfe/trunk/include/clang/Basic/BuiltinsPPC.def
cfe/trunk/lib/Headers/altivec.h
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/CodeGen/builtins-ppc-error.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsPPC.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsPPC.def?rev=370902&r1=370901&r2=370902&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsPPC.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsPPC.def Wed Sep  4 07:01:47 2019
@@ -55,7 +55,7 @@ BUILTIN(__builtin_altivec_vcfux, "V4fV4i
 BUILTIN(__builtin_altivec_vctsxs, "V4SiV4fIi", "")
 BUILTIN(__builtin_altivec_vctuxs, "V4UiV4fIi", "")
 
-BUILTIN(__builtin_altivec_dss, "vUi", "")
+BUILTIN(__builtin_altivec_dss, "vUIi", "")
 BUILTIN(__builtin_altivec_dssall, "v", "")
 BUILTIN(__builtin_altivec_dst, "vvC*iUi", "")
 BUILTIN(__builtin_altivec_dstt, "vvC*iUi", "")

Modified: cfe/trunk/lib/Headers/altivec.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/altivec.h?rev=370902&r1=370901&r2=370902&view=diff
==
--- cfe/trunk/lib/Headers/altivec.h (original)
+++ cfe/trunk/lib/Headers/altivec.h Wed Sep  4 07:01:47 2019
@@ -3286,9 +3286,7 @@ static __inline__ vector double __ATTRS_
 
 /* vec_dss */
 
-static __inline__ void __attribute__((__always_inline__)) vec_dss(int __a) {
-  __builtin_altivec_dss(__a);
-}
+#define vec_dss __builtin_altivec_dss
 
 /* vec_dssall */
 

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=370902&r1=370901&r2=370902&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Sep  4 07:01:47 2019
@@ -3220,6 +3220,8 @@ bool Sema::CheckPPCBuiltinFunctionCall(u
   case PPC::BI__builtin_altivec_crypto_vshasigmad:
 return SemaBuiltinConstantArgRange(TheCall, 1, 0, 1) ||
SemaBuiltinConstantArgRange(TheCall, 2, 0, 15);
+  case PPC::BI__builtin_altivec_dss:
+return SemaBuiltinConstantArgRange(TheCall, 0, 0, 3);
   case PPC::BI__builtin_tbegin:
   case PPC::BI__builtin_tend: i = 0; l = 0; u = 1; break;
   case PPC::BI__builtin_tsr: i = 0; l = 0; u = 7; break;

Added: cfe/trunk/test/CodeGen/altivec-dss.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/altivec-dss.c?rev=370902&view=auto
==
--- cfe/trunk/test/CodeGen/altivec-dss.c (added)
+++ cfe/trunk/test/CodeGen/altivec-dss.c Wed Sep  4 07:01:47 2019
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple powerpc-linux-gnu -S -O0 -o - %s -target-feature 
+altivec | FileCheck %s
+
+// REQUIRES: powerpc-registered-target
+
+#include 
+
+// CHECK-LABEL: test1
+// CHECK: dss 
+void test1() {
+  vec_dss(1);
+}

Modified: cfe/trunk/test/CodeGen/builtins-ppc-error.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins-ppc-error.c?rev=370902&r1=370901&r2=370902&view=diff
==
--- cfe/trunk/test/CodeGen/builtins-ppc-error.c (original)
+++ cfe/trunk/test/CodeGen/builtins-ppc-error.c Wed Sep  4 07:01:47 2019
@@ -73,3 +73,8 @@ void testUnpack128(int index) {
   __builtin_unpack_vector_int128(vsllli, index); //expected-error {{argument 
to '__builtin_unpack_vector_int128' must be a constant integer}}
   __builtin_unpack_vector_int128(vsllli, 5); //expected-error {{argument value 
5 is outside the valid range [0, 1]}}
 }
+
+void testDSS(int index) {
+  vec_dss(index); //expected-error {{argument to '__builtin_altivec_dss' must 
be a constant integer}}
+  vec_dss(5); //expected-error {{argument value 5 is outside the valid range 
[0, 3]}}
+}


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

[PATCH] D67084: [clang-tidy] Fix bugprone-argument-comment bug: negative literal number is not checked.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp:240
+  Arg = Arg->IgnoreImpCasts();
+  if (isa(Arg))
+  Arg = cast(Arg)->getSubExpr();

xyb wrote:
> aaron.ballman wrote:
> > The bug claims that this is only for handling negative literals, but this 
> > allows any unary operator. Is that intentional? If we're going to allow 
> > arbitrary unary operators, why not arbitrary binary operators as well?
> Actually, it handles "UnaryOperator Literal". So it will handle "+12", "-12", 
> "~12". I can restrict it for UO_MINUS only, but is it real necessary? 
It's not that it's necessary, it's that I'm wondering why we think `-12` is 
special but `0 - 12` is not, but that can be done incrementally later.


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

https://reviews.llvm.org/D67084



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

r370903 - [ASTImporter] Added visibility context check for TypedefNameDecl.

[Balazs Keri via cfe-commits]

Author: balazske
Date: Wed Sep  4 07:12:18 2019
New Revision: 370903

URL: http://llvm.org/viewvc/llvm-project?rev=370903&view=rev
Log:
[ASTImporter] Added visibility context check for TypedefNameDecl.

Summary:
ASTImporter makes now difference between typedefs and type aliases
with same name in different translation units
if these are not visible outside.

Reviewers: martong, a.sidorin, shafik, a_sidorin

Reviewed By: martong, shafik

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=370903&r1=370902&r2=370903&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Wed Sep  4 07:12:18 2019
@@ -932,6 +932,27 @@ Expected ASTNodeImporter:
   EllipsisLoc);
 }
 
+template 
+bool ASTNodeImporter::hasSameVisibilityContext(T *Found, T *From) {
+  if (From->hasExternalFormalLinkage())
+return Found->hasExternalFormalLinkage();
+  if (Importer.GetFromTU(Found) != From->getTranslationUnitDecl())
+return false;
+  if (From->isInAnonymousNamespace())
+return Found->isInAnonymousNamespace();
+  else
+return !Found->isInAnonymousNamespace() &&
+   !Found->hasExternalFormalLinkage();
+}
+
+template <>
+bool ASTNodeImporter::hasSameVisibilityContext(TypedefNameDecl *Found,
+   TypedefNameDecl *From) {
+  if (From->isInAnonymousNamespace() && Found->isInAnonymousNamespace())
+return Importer.GetFromTU(Found) == From->getTranslationUnitDecl();
+  return From->isInAnonymousNamespace() == Found->isInAnonymousNamespace();
+}
+
 } // namespace clang
 
 //
@@ -2344,6 +2365,9 @@ ASTNodeImporter::VisitTypedefNameDecl(Ty
   // If this typedef is not in block scope, determine whether we've
   // seen a typedef with the same name (that we can merge with) or any
   // other entity by that name (which name lookup could conflict with).
+  // Note: Repeated typedefs are not valid in C99:
+  // 'typedef int T; typedef int T;' is invalid
+  // We do not care about this now.
   if (!DC->isFunctionOrMethod()) {
 SmallVector ConflictingDecls;
 unsigned IDNS = Decl::IDNS_Ordinary;
@@ -2352,6 +2376,9 @@ ASTNodeImporter::VisitTypedefNameDecl(Ty
   if (!FoundDecl->isInIdentifierNamespace(IDNS))
 continue;
   if (auto *FoundTypedef = dyn_cast(FoundDecl)) {
+if (!hasSameVisibilityContext(FoundTypedef, D))
+  continue;
+
 QualType FromUT = D->getUnderlyingType();
 QualType FoundUT = FoundTypedef->getUnderlyingType();
 if (Importer.IsStructurallyEquivalent(FromUT, FoundUT)) {
@@ -3022,19 +3049,6 @@ Error ASTNodeImporter::ImportFunctionDec
   return Error::success();
 }
 
-template 
-bool ASTNodeImporter::hasSameVisibilityContext(T *Found, T *From) {
-  if (From->hasExternalFormalLinkage())
-return Found->hasExternalFormalLinkage();
-  if (Importer.GetFromTU(Found) != From->getTranslationUnitDecl())
-return false;
-  if (From->isInAnonymousNamespace())
-return Found->isInAnonymousNamespace();
-  else
-return !Found->isInAnonymousNamespace() &&
-   !Found->hasExternalFormalLinkage();
-}
-
 ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
 
   SmallVector Redecls = getCanonicalForwardRedeclChain(D);

Modified: cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp?rev=370903&r1=370902&r2=370903&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp Wed Sep  4 07:12:18 
2019
@@ -39,6 +39,10 @@ struct GetEnumPattern {
   using DeclTy = EnumDecl;
   BindableMatcher operator()() { return enumDecl(hasName("E")); }
 };
+struct GetTypedefNamePattern {
+  using DeclTy = TypedefNameDecl;
+  BindableMatcher operator()() { return typedefNameDecl(hasName("T")); }
+};
 
 // Values for the value-parameterized test fixtures.
 // FunctionDecl:
@@ -55,6 +59,11 @@ const auto *AnonC = "namespace { class X
 // EnumDecl:
 const auto *ExternE = "enum E {};";
 const auto *AnonE = "namespace { enum E {}; }";
+// TypedefNameDecl:
+const auto *ExternTypedef = "typedef int T;";
+const auto *AnonTypedef = "namespace { typedef int T; }";
+const auto *ExternUsing = "using T = int;";
+const auto *AnonUsing = "namespace { using T = int; }";
 
 // First value in tuple: Compile options.
 // Second value in tuple: Source code to be used in the test.
@@ -2

[PATCH] D67084: [clang-tidy] Fix bugprone-argument-comment bug: negative literal number is not checked.

[Yubo Xie via Phabricator via cfe-commits]

xyb added a comment.

Thanks. BTW, I can't commit the patch by myself.


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

https://reviews.llvm.org/D67084



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

[PATCH] D67080: [clang-tidy] Fix bugprone-argument-comment bug if there are marcos.

[Yubo Xie via Phabricator via cfe-commits]

xyb added a comment.

Thanks. BTW, I can't commit the patch by myself.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67080



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

[PATCH] D64480: [ASTImporter] Added visibility context check for TypedefNameDecl.

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370903: [ASTImporter] Added visibility context check for 
TypedefNameDecl. (authored by balazske, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64480?vs=218610&id=218687#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64480

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp

Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -932,6 +932,27 @@
   EllipsisLoc);
 }
 
+template 
+bool ASTNodeImporter::hasSameVisibilityContext(T *Found, T *From) {
+  if (From->hasExternalFormalLinkage())
+return Found->hasExternalFormalLinkage();
+  if (Importer.GetFromTU(Found) != From->getTranslationUnitDecl())
+return false;
+  if (From->isInAnonymousNamespace())
+return Found->isInAnonymousNamespace();
+  else
+return !Found->isInAnonymousNamespace() &&
+   !Found->hasExternalFormalLinkage();
+}
+
+template <>
+bool ASTNodeImporter::hasSameVisibilityContext(TypedefNameDecl *Found,
+   TypedefNameDecl *From) {
+  if (From->isInAnonymousNamespace() && Found->isInAnonymousNamespace())
+return Importer.GetFromTU(Found) == From->getTranslationUnitDecl();
+  return From->isInAnonymousNamespace() == Found->isInAnonymousNamespace();
+}
+
 } // namespace clang
 
 //
@@ -2344,6 +2365,9 @@
   // If this typedef is not in block scope, determine whether we've
   // seen a typedef with the same name (that we can merge with) or any
   // other entity by that name (which name lookup could conflict with).
+  // Note: Repeated typedefs are not valid in C99:
+  // 'typedef int T; typedef int T;' is invalid
+  // We do not care about this now.
   if (!DC->isFunctionOrMethod()) {
 SmallVector ConflictingDecls;
 unsigned IDNS = Decl::IDNS_Ordinary;
@@ -2352,6 +2376,9 @@
   if (!FoundDecl->isInIdentifierNamespace(IDNS))
 continue;
   if (auto *FoundTypedef = dyn_cast(FoundDecl)) {
+if (!hasSameVisibilityContext(FoundTypedef, D))
+  continue;
+
 QualType FromUT = D->getUnderlyingType();
 QualType FoundUT = FoundTypedef->getUnderlyingType();
 if (Importer.IsStructurallyEquivalent(FromUT, FoundUT)) {
@@ -3022,19 +3049,6 @@
   return Error::success();
 }
 
-template 
-bool ASTNodeImporter::hasSameVisibilityContext(T *Found, T *From) {
-  if (From->hasExternalFormalLinkage())
-return Found->hasExternalFormalLinkage();
-  if (Importer.GetFromTU(Found) != From->getTranslationUnitDecl())
-return false;
-  if (From->isInAnonymousNamespace())
-return Found->isInAnonymousNamespace();
-  else
-return !Found->isInAnonymousNamespace() &&
-   !Found->hasExternalFormalLinkage();
-}
-
 ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
 
   SmallVector Redecls = getCanonicalForwardRedeclChain(D);
Index: cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp
@@ -39,6 +39,10 @@
   using DeclTy = EnumDecl;
   BindableMatcher operator()() { return enumDecl(hasName("E")); }
 };
+struct GetTypedefNamePattern {
+  using DeclTy = TypedefNameDecl;
+  BindableMatcher operator()() { return typedefNameDecl(hasName("T")); }
+};
 
 // Values for the value-parameterized test fixtures.
 // FunctionDecl:
@@ -55,6 +59,11 @@
 // EnumDecl:
 const auto *ExternE = "enum E {};";
 const auto *AnonE = "namespace { enum E {}; }";
+// TypedefNameDecl:
+const auto *ExternTypedef = "typedef int T;";
+const auto *AnonTypedef = "namespace { typedef int T; }";
+const auto *ExternUsing = "using T = int;";
+const auto *AnonUsing = "namespace { using T = int; }";
 
 // First value in tuple: Compile options.
 // Second value in tuple: Source code to be used in the test.
@@ -243,6 +252,7 @@
 using ImportVariablesVisibility = ImportVisibility;
 using ImportClassesVisibility = ImportVisibility;
 using ImportEnumsVisibility = ImportVisibility;
+using ImportTypedefNameVisibility = ImportVisibility;
 
 // FunctionDecl.
 TEST_P(ImportFunctionsVisibility, ImportAfter) {
@@ -272,6 +282,13 @@
 TEST_P(ImportEnumsVisibility, ImportAfterImport) {
   TypedTest_ImportAfterImportWithMerge();
 }
+// TypedefNameDecl.
+TEST_P(ImportTypedefNameVisibility, ImportAfter) {
+  TypedTest_ImportAfterWithMerge();
+}
+TEST_P(ImportTypedefNameVisibility, ImportAfterImport) {
+  TypedTest_ImportAfterImportWithMerge();
+}
 
 const bool ExpectLink = true;
 const

[PATCH] D67056: Add a bugprone-argument-comment option: IgnoreSingleArgument.

[Yubo Xie via Phabricator via cfe-commits]

xyb added a comment.

Thanks. BTW, I can't commit the patch by myself.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67056



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

[PATCH] D67165: [clangd][vscode] Make SemanticHighlightingFeature more self-contained.

[Haojian Wu via Phabricator via cfe-commits]

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

Update the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67165

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
  clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts


Index: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
@@ -44,7 +44,7 @@
 
 // Language server push notification providing the semantic highlighting
 // information for a text document.
-export const NotificationType =
+const NotificationType =
 new vscodelc.NotificationType(
 'textDocument/semanticHighlighting');
 
@@ -58,6 +58,11 @@
   highlighter: Highlighter;
   // Any disposables that should be cleaned up when clangd crashes.
   private subscriptions: vscode.Disposable[] = [];
+  private clangdClient: vscodelc.BaseLanguageClient;
+
+  constructor(client: vscodelc.BaseLanguageClient) {
+this.clangdClient = client;
+  }
   fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {
 // Extend the ClientCapabilities type and add semantic highlighting
 // capability to the object.
@@ -78,6 +83,12 @@
 
   initialize(capabilities: vscodelc.ServerCapabilities,
  documentSelector: vscodelc.DocumentSelector|undefined) {
+// Dispose resources everytime we get initialized, e.g. first launch or
+// recover from clangd crashes.
+this.dispose();
+// Register the handler to handle semantic highlighting notification.
+this.clangdClient.onNotification(NotificationType,
+ this.handleNotification.bind(this));
 // The semantic highlighting capability information is in the capabilities
 // object but to access the data we must first extend the 
ServerCapabilities
 // type.
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
@@ -110,7 +110,7 @@
   const clangdClient = new ClangdLanguageClient('Clang Language Server',
 serverOptions, clientOptions);
   const semanticHighlightingFeature =
-  new semanticHighlighting.SemanticHighlightingFeature();
+  new semanticHighlighting.SemanticHighlightingFeature(clangdClient);
   context.subscriptions.push(
   vscode.Disposable.from(semanticHighlightingFeature));
   clangdClient.registerFeature(semanticHighlightingFeature);
@@ -144,14 +144,9 @@
   clangdClient.onNotification(
   'textDocument/clangd.fileStatus',
   (fileStatus) => { status.onFileUpdated(fileStatus); });
-  clangdClient.onNotification(
-  semanticHighlighting.NotificationType,
-  semanticHighlightingFeature.handleNotification.bind(
-  semanticHighlightingFeature));
 } else if (newState == vscodelc.State.Stopped) {
   // Clear all cached statuses when clangd crashes.
   status.clear();
-  semanticHighlightingFeature.dispose();
 }
   }));
   // An empty place holder for the activate command, otherwise we'll get an


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
@@ -44,7 +44,7 @@
 
 // Language server push notification providing the semantic highlighting
 // information for a text document.
-export const NotificationType =
+const NotificationType =
 new vscodelc.NotificationType(
 'textDocument/semanticHighlighting');
 
@@ -58,6 +58,11 @@
   highlighter: Highlighter;
   // Any disposables that should be cleaned up when clangd crashes.
   private subscriptions: vscode.Disposable[] = [];
+  private clangdClient: vscodelc.BaseLanguageClient;
+
+  constructor(client: vscodelc.BaseLanguageClient) {
+this.clangdClient = client;
+  }
   fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {
 // Extend the ClientCapabilities type and add semantic highlighting
 // capability to the object.
@@ -78,6 +83,12 @@
 
   initialize(capabilities: vscodelc.ServerCapabilities,
  documentSelector: vscodelc.DocumentSelector|undefined) {
+// Dispose resources everytime we get initialized, e.g. first launch or
+// recover from clangd crashes.
+this.dispose();
+// Register the handler to handle semantic highlighting notification.
+this.

[PATCH] D67172: [clangd] Use pre-populated mappings for standard symbols

[Ilya Biryukov via Phabricator via cfe-commits]

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

This takes ~5% of time when running clangd unit tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67172

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp
  clang-tools-extra/clangd/index/CanonicalIncludes.h
  clang-tools-extra/clangd/index/IndexAction.cpp
  clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -973,7 +973,7 @@
   CanonicalIncludes Includes;
   auto Language = LangOptions();
   Language.CPlusPlus = true;
-  addSystemHeadersMapping(&Includes, Language);
+  Includes.addSystemHeadersMapping(Language);
   CollectorOpts.Includes = &Includes;
   runSymbolCollector("namespace std { class string {}; }", /*Main=*/"");
   EXPECT_THAT(Symbols,
Index: clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
===
--- clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
@@ -17,7 +17,7 @@
   CanonicalIncludes CI;
   auto Language = LangOptions();
   Language.C11 = true;
-  addSystemHeadersMapping(&CI, Language);
+  CI.addSystemHeadersMapping(Language);
   // Usual standard library symbols are mapped correctly.
   EXPECT_EQ("", CI.mapHeader("path/stdio.h", "printf"));
 }
@@ -26,7 +26,7 @@
   CanonicalIncludes CI;
   auto Language = LangOptions();
   Language.CPlusPlus = true;
-  addSystemHeadersMapping(&CI, Language);
+  CI.addSystemHeadersMapping(Language);
 
   // Usual standard library symbols are mapped correctly.
   EXPECT_EQ("", CI.mapHeader("path/vector.h", "std::vector"));
Index: clang-tools-extra/clangd/index/IndexAction.cpp
===
--- clang-tools-extra/clangd/index/IndexAction.cpp
+++ clang-tools-extra/clangd/index/IndexAction.cpp
@@ -140,7 +140,7 @@
   std::unique_ptr
   CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override {
 CI.getPreprocessor().addCommentHandler(PragmaHandler.get());
-addSystemHeadersMapping(Includes.get(), CI.getLangOpts());
+Includes->addSystemHeadersMapping(CI.getLangOpts());
 if (IncludeGraphCallback != nullptr)
   CI.getPreprocessor().addPPCallbacks(
   std::make_unique(CI.getSourceManager(), IG));
Index: clang-tools-extra/clangd/index/CanonicalIncludes.h
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.h
+++ clang-tools-extra/clangd/index/CanonicalIncludes.h
@@ -55,6 +55,16 @@
   llvm::StringRef mapHeader(llvm::StringRef Header,
 llvm::StringRef QualifiedName) const;
 
+  /// Adds mapping for system headers and some special symbols (e.g. STL symbols
+  /// in  need to be mapped individually). Approximately, the following
+  /// system headers are handled:
+  ///   - C++ standard library e.g. bits/basic_string.h$ -> 
+  ///   - Posix library e.g. bits/pthreadtypes.h$ -> 
+  ///   - Compiler extensions, e.g. include/avx512bwintrin.h$ -> 
+  /// The mapping is hardcoded and hand-maintained, so it might not cover all
+  /// headers.
+  void addSystemHeadersMapping(const LangOptions &Language);
+
 private:
   /// A map from full include path to a canonical path.
   llvm::StringMap FullPathMapping;
@@ -65,6 +75,8 @@
   int MaxSuffixComponents = 0;
   /// A map from fully qualified symbol names to header names.
   llvm::StringMap SymbolMapping;
+  /// Precomputed mapping of std symbols. Ignored when null.
+  const CanonicalIncludes *SystemSymbols = nullptr;
 };
 
 /// Returns a CommentHandler that parses pragma comment on include files to
@@ -76,16 +88,6 @@
 std::unique_ptr
 collectIWYUHeaderMaps(CanonicalIncludes *Includes);
 
-/// Adds mapping for system headers and some special symbols (e.g. STL symbols
-/// in  need to be mapped individually). Approximately, the following
-/// system headers are handled:
-///   - C++ standard library e.g. bits/basic_string.h$ -> 
-///   - Posix library e.g. bits/pthreadtypes.h$ -> 
-///   - Compiler extensions, e.g. include/avx512bwintrin.h$ -> 
-/// The mapping is hardcoded and hand-maintained, so it might not cover all
-/// headers.
-void addSystemHeadersMapping(CanonicalIncludes *Includes,
- const LangOptions &Language);
 
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp

[PATCH] D67172: [clangd] Use pre-populated mappings for standard symbols

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Also considered moving the std symbol mapping into a separate 
(private-to-implementation) class, e.g. we don't use suffix mappings and symbol 
mappings outside system includes.
But decided to wait until suffix mapping are going away completely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67172



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

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Simon Pilgrim via Phabricator via cfe-commits]

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

LGTM - cheers


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

https://reviews.llvm.org/D64672



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

r370908 - [Driver] Use shared singleton instance of DriverOptTable

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Sep  4 07:26:28 2019
New Revision: 370908

URL: http://llvm.org/viewvc/llvm-project?rev=370908&view=rev
Log:
[Driver] Use shared singleton instance of DriverOptTable

Summary:
This significantly reduces the time required to run clangd tests, by
~10%.

Should also have an effect on other tests that run command-line parsing
multiple times inside a single invocation.

Reviewers: gribozavr, sammccall

Reviewed By: sammccall

Subscribers: kadircet, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Driver/Driver.h
cfe/trunk/include/clang/Driver/Options.h
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/DriverOptions.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
cfe/trunk/lib/Tooling/Tooling.cpp
cfe/trunk/tools/clang-check/ClangCheck.cpp
cfe/trunk/tools/driver/cc1as_main.cpp
cfe/trunk/tools/driver/driver.cpp

Modified: cfe/trunk/include/clang/Driver/Driver.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=370908&r1=370907&r2=370908&view=diff
==
--- cfe/trunk/include/clang/Driver/Driver.h (original)
+++ cfe/trunk/include/clang/Driver/Driver.h Wed Sep  4 07:26:28 2019
@@ -12,6 +12,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Driver/Action.h"
+#include "clang/Driver/Options.h"
 #include "clang/Driver/Phases.h"
 #include "clang/Driver/ToolChain.h"
 #include "clang/Driver/Types.h"
@@ -56,8 +57,6 @@ enum LTOKind {
 /// Driver - Encapsulate logic for constructing compilation processes
 /// from a set of gcc-driver-like command line arguments.
 class Driver {
-  std::unique_ptr Opts;
-
   DiagnosticsEngine &Diags;
 
   IntrusiveRefCntPtr VFS;
@@ -301,7 +300,7 @@ public:
 
   const std::string &getConfigFile() const { return ConfigFile; }
 
-  const llvm::opt::OptTable &getOpts() const { return *Opts; }
+  const llvm::opt::OptTable &getOpts() const { return getDriverOptTable(); }
 
   const DiagnosticsEngine &getDiags() const { return Diags; }
 

Modified: cfe/trunk/include/clang/Driver/Options.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.h?rev=370908&r1=370907&r2=370908&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.h (original)
+++ cfe/trunk/include/clang/Driver/Options.h Wed Sep  4 07:26:28 2019
@@ -47,7 +47,7 @@ enum ID {
   };
 }
 
-std::unique_ptr createDriverOptTable();
+const llvm::opt::OptTable &getDriverOptTable();
 }
 }
 

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=370908&r1=370907&r2=370908&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Wed Sep  4 07:26:28 2019
@@ -120,16 +120,16 @@ std::string Driver::GetResourcesPath(Str
 Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple,
DiagnosticsEngine &Diags,
IntrusiveRefCntPtr VFS)
-: Opts(createDriverOptTable()), Diags(Diags), VFS(std::move(VFS)),
-  Mode(GCCMode), SaveTemps(SaveTempsNone), BitcodeEmbed(EmbedNone),
-  LTOMode(LTOK_None), ClangExecutable(ClangExecutable),
-  SysRoot(DEFAULT_SYSROOT), DriverTitle("clang LLVM compiler"),
-  CCPrintOptionsFilename(nullptr), CCPrintHeadersFilename(nullptr),
-  CCLogDiagnosticsFilename(nullptr), CCCPrintBindings(false),
-  CCPrintOptions(false), CCPrintHeaders(false), CCLogDiagnostics(false),
-  CCGenDiagnostics(false), TargetTriple(TargetTriple),
-  CCCGenericGCCName(""), Saver(Alloc), CheckInputsExist(true),
-  GenReproducer(false), SuppressMissingInputWarning(false) {
+: Diags(Diags), VFS(std::move(VFS)), Mode(GCCMode),
+  SaveTemps(SaveTempsNone), BitcodeEmbed(EmbedNone), LTOMode(LTOK_None),
+  ClangExecutable(ClangExecutable), SysRoot(DEFAULT_SYSROOT),
+  DriverTitle("clang LLVM compiler"), CCPrintOptionsFilename(nullptr),
+  CCPrintHeadersFilename(nullptr), CCLogDiagnosticsFilename(nullptr),
+  CCCPrintBindings(false), CCPrintOptions(false), CCPrintHeaders(false),
+  CCLogDiagnostics(false), CCGenDiagnostics(false),
+  TargetTriple(TargetTriple), CCCGenericGCCName(""), Saver(Alloc),
+  CheckInputsExist(true), GenReproducer(false),
+  SuppressMissingInputWarning(false) {
 
   // Provide a sane fallback if no VFS is specified.
   if (!this->VFS)
@@ -310,7 +310,7 @@ phases::ID Driver::getFinalPhase(const D
   return FinalPhase;
 }
 
-static Arg *MakeInputArg(DerivedArgList &Args, OptTable &Opts,
+static Arg *MakeInputArg(DerivedArgList &Args, const OptTable &Opts,

[PATCH] D67096: [clangd][vscode] Add a flag to enable semantic highlighting in clangd

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:113
   const semanticHighlightingFeature =
-  new semanticHighlighting.SemanticHighlightingFeature();
+  new semanticHighlighting.SemanticHighlightingFeature(
+  getConfig('semanticHighlighting'));

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > hokein wrote:
> > > > ilya-biryukov wrote:
> > > > > Why not avoid calling `clangdClient.registerFeature` instead?
> > > > > Would allow to:
> > > > > - not change the `SemanticHighlightingFeature` class, keeping it 
> > > > > simpler,
> > > > > - ensure we do not do any extra work if the feature is disabled.
> > > > good point, done.
> > > Should we update other uses of `semanticHighlightingFeature` too?
> > > 
> > > `context.subscriptions.push(vscode.Disposable.from(semanticHighlightingFeature))`
> > >  probably ensures we call `dispose()` when the `clangdClient` is getting 
> > > removed, I guess we definitely don't need that.
> > > 
> > > Other uses as well:
> > > - no need to pass notification is highlighting is disabled.
> > > - no need to cleanup if highlighting is disabled.
> > > 
> > > Maybe assign null or undefined to `semanticHighlightingFeature` when the 
> > > flag is false?
> > > At each usage we can check whether the `semanticHighlightingFeature` is 
> > > not null and only call relevant methods if that's the case.
> > I don't think it is worth updating all usages, it is no harm to keep them 
> > here even when the highlighting is disabled (the dispose is a no-op; we 
> > never receive notifications from clangd); and it would add more guard code 
> > which I'd avoid.
> How can we be sure that nothing bad is going to happen?
> In particular, we are "binding" notification handling, but never registered a 
> feature. How can we be sure we won't actually get any notifications?
> 
> If we don't create the object in the first place, we are confident nothing 
> harmful can be done with it.
> 
> How can we be sure we won't actually get any notifications?
If we receive a notification, that means we have clangd bugs. 

I understand you point here, an ideal solution is to avoid too many usages of 
`SemanticHighlightingFeature` in the client side, after D67165, it'd help 
simplify the patch here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67096



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

[clang-tools-extra] r370908 - [Driver] Use shared singleton instance of DriverOptTable

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Sep  4 07:26:28 2019
New Revision: 370908

URL: http://llvm.org/viewvc/llvm-project?rev=370908&view=rev
Log:
[Driver] Use shared singleton instance of DriverOptTable

Summary:
This significantly reduces the time required to run clangd tests, by
~10%.

Should also have an effect on other tests that run command-line parsing
multiple times inside a single invocation.

Reviewers: gribozavr, sammccall

Reviewed By: sammccall

Subscribers: kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/modularize/Modularize.cpp

Modified: clang-tools-extra/trunk/modularize/Modularize.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/modularize/Modularize.cpp?rev=370908&r1=370907&r2=370908&view=diff
==
--- clang-tools-extra/trunk/modularize/Modularize.cpp (original)
+++ clang-tools-extra/trunk/modularize/Modularize.cpp Wed Sep  4 07:26:28 2019
@@ -337,14 +337,13 @@ std::string CommandLine;
 
 // Helper function for finding the input file in an arguments list.
 static std::string findInputFile(const CommandLineArguments &CLArgs) {
-  std::unique_ptr Opts(createDriverOptTable());
   const unsigned IncludedFlagsBitmask = options::CC1Option;
   unsigned MissingArgIndex, MissingArgCount;
   SmallVector Argv;
   for (auto I = CLArgs.begin(), E = CLArgs.end(); I != E; ++I)
 Argv.push_back(I->c_str());
-  InputArgList Args = Opts->ParseArgs(Argv, MissingArgIndex, MissingArgCount,
-  IncludedFlagsBitmask);
+  InputArgList Args = getDriverOptTable().ParseArgs(
+  Argv, MissingArgIndex, MissingArgCount, IncludedFlagsBitmask);
   std::vector Inputs = Args.getAllArgValues(OPT_INPUT);
   return ModularizeUtilities::getCanonicalPath(Inputs.back());
 }


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

[PATCH] D67163: [Driver] Use shared singleton instance of DriverOptTable

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370908: [Driver] Use shared singleton instance of 
DriverOptTable (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67163?vs=218672&id=218693#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67163

Files:
  cfe/trunk/include/clang/Driver/Driver.h
  cfe/trunk/include/clang/Driver/Options.h
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/lib/Driver/DriverOptions.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  cfe/trunk/lib/Tooling/InterpolatingCompilationDatabase.cpp
  cfe/trunk/lib/Tooling/Tooling.cpp
  cfe/trunk/tools/clang-check/ClangCheck.cpp
  cfe/trunk/tools/driver/cc1as_main.cpp
  cfe/trunk/tools/driver/driver.cpp
  clang-tools-extra/trunk/modularize/Modularize.cpp

Index: cfe/trunk/include/clang/Driver/Driver.h
===
--- cfe/trunk/include/clang/Driver/Driver.h
+++ cfe/trunk/include/clang/Driver/Driver.h
@@ -12,6 +12,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Driver/Action.h"
+#include "clang/Driver/Options.h"
 #include "clang/Driver/Phases.h"
 #include "clang/Driver/ToolChain.h"
 #include "clang/Driver/Types.h"
@@ -56,8 +57,6 @@
 /// Driver - Encapsulate logic for constructing compilation processes
 /// from a set of gcc-driver-like command line arguments.
 class Driver {
-  std::unique_ptr Opts;
-
   DiagnosticsEngine &Diags;
 
   IntrusiveRefCntPtr VFS;
@@ -301,7 +300,7 @@
 
   const std::string &getConfigFile() const { return ConfigFile; }
 
-  const llvm::opt::OptTable &getOpts() const { return *Opts; }
+  const llvm::opt::OptTable &getOpts() const { return getDriverOptTable(); }
 
   const DiagnosticsEngine &getDiags() const { return Diags; }
 
Index: cfe/trunk/include/clang/Driver/Options.h
===
--- cfe/trunk/include/clang/Driver/Options.h
+++ cfe/trunk/include/clang/Driver/Options.h
@@ -47,7 +47,7 @@
   };
 }
 
-std::unique_ptr createDriverOptTable();
+const llvm::opt::OptTable &getDriverOptTable();
 }
 }
 
Index: cfe/trunk/lib/Driver/DriverOptions.cpp
===
--- cfe/trunk/lib/Driver/DriverOptions.cpp
+++ cfe/trunk/lib/Driver/DriverOptions.cpp
@@ -39,14 +39,17 @@
 
 }
 
-std::unique_ptr clang::driver::createDriverOptTable() {
-  auto Result = std::make_unique();
-  // Options.inc is included in DriverOptions.cpp, and calls OptTable's
-  // addValues function.
-  // Opt is a variable used in the code fragment in Options.inc.
-  OptTable &Opt = *Result;
+const llvm::opt::OptTable &clang::driver::getDriverOptTable() {
+  static const DriverOptTable *Table = []() {
+auto Result = std::make_unique();
+// Options.inc is included in DriverOptions.cpp, and calls OptTable's
+// addValues function.
+// Opt is a variable used in the code fragment in Options.inc.
+OptTable &Opt = *Result;
 #define OPTTABLE_ARG_INIT
 #include "clang/Driver/Options.inc"
 #undef OPTTABLE_ARG_INIT
-  return std::move(Result);
+return Result.release();
+  }();
+  return *Table;
 }
Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -120,16 +120,16 @@
 Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple,
DiagnosticsEngine &Diags,
IntrusiveRefCntPtr VFS)
-: Opts(createDriverOptTable()), Diags(Diags), VFS(std::move(VFS)),
-  Mode(GCCMode), SaveTemps(SaveTempsNone), BitcodeEmbed(EmbedNone),
-  LTOMode(LTOK_None), ClangExecutable(ClangExecutable),
-  SysRoot(DEFAULT_SYSROOT), DriverTitle("clang LLVM compiler"),
-  CCPrintOptionsFilename(nullptr), CCPrintHeadersFilename(nullptr),
-  CCLogDiagnosticsFilename(nullptr), CCCPrintBindings(false),
-  CCPrintOptions(false), CCPrintHeaders(false), CCLogDiagnostics(false),
-  CCGenDiagnostics(false), TargetTriple(TargetTriple),
-  CCCGenericGCCName(""), Saver(Alloc), CheckInputsExist(true),
-  GenReproducer(false), SuppressMissingInputWarning(false) {
+: Diags(Diags), VFS(std::move(VFS)), Mode(GCCMode),
+  SaveTemps(SaveTempsNone), BitcodeEmbed(EmbedNone), LTOMode(LTOK_None),
+  ClangExecutable(ClangExecutable), SysRoot(DEFAULT_SYSROOT),
+  DriverTitle("clang LLVM compiler"), CCPrintOptionsFilename(nullptr),
+  CCPrintHeadersFilename(nullptr), CCLogDiagnosticsFilename(nullptr),
+  CCCPrintBindings(false), CCPrintOptions(false), CCPrintHeaders(false),
+  CCLogDiagnostics(false), CCGenDiagnostics(false),
+  TargetTriple(TargetTriple), CCCGenericGCCName(""), Saver(

[PATCH] D67096: [clangd][vscode] Add a flag to enable semantic highlighting in clangd

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:113
   const semanticHighlightingFeature =
-  new semanticHighlighting.SemanticHighlightingFeature();
+  new semanticHighlighting.SemanticHighlightingFeature(
+  getConfig('semanticHighlighting'));

hokein wrote:
> ilya-biryukov wrote:
> > hokein wrote:
> > > ilya-biryukov wrote:
> > > > hokein wrote:
> > > > > ilya-biryukov wrote:
> > > > > > Why not avoid calling `clangdClient.registerFeature` instead?
> > > > > > Would allow to:
> > > > > > - not change the `SemanticHighlightingFeature` class, keeping it 
> > > > > > simpler,
> > > > > > - ensure we do not do any extra work if the feature is disabled.
> > > > > good point, done.
> > > > Should we update other uses of `semanticHighlightingFeature` too?
> > > > 
> > > > `context.subscriptions.push(vscode.Disposable.from(semanticHighlightingFeature))`
> > > >  probably ensures we call `dispose()` when the `clangdClient` is 
> > > > getting removed, I guess we definitely don't need that.
> > > > 
> > > > Other uses as well:
> > > > - no need to pass notification is highlighting is disabled.
> > > > - no need to cleanup if highlighting is disabled.
> > > > 
> > > > Maybe assign null or undefined to `semanticHighlightingFeature` when 
> > > > the flag is false?
> > > > At each usage we can check whether the `semanticHighlightingFeature` is 
> > > > not null and only call relevant methods if that's the case.
> > > I don't think it is worth updating all usages, it is no harm to keep them 
> > > here even when the highlighting is disabled (the dispose is a no-op; we 
> > > never receive notifications from clangd); and it would add more guard 
> > > code which I'd avoid.
> > How can we be sure that nothing bad is going to happen?
> > In particular, we are "binding" notification handling, but never registered 
> > a feature. How can we be sure we won't actually get any notifications?
> > 
> > If we don't create the object in the first place, we are confident nothing 
> > harmful can be done with it.
> > 
> > How can we be sure we won't actually get any notifications?
> If we receive a notification, that means we have clangd bugs. 
> 
> I understand you point here, an ideal solution is to avoid too many usages of 
> `SemanticHighlightingFeature` in the client side, after D67165, it'd help 
> simplify the patch here.
> If we receive a notification, that means we have clangd bugs.
True, but that might happen. It'd be better to not break in that case.
D67165 is definitely moving in the right direction, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67096



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

[PATCH] D66748: [PowerPC][Altivec][Clang] Check compile-time constant for vec_dst*

[Jinsong Ji via Phabricator via cfe-commits]

jsji updated this revision to Diff 218696.
jsji added a comment.

Rebase to ToT.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66748

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-ppc-error.c


Index: clang/test/CodeGen/builtins-ppc-error.c
===
--- clang/test/CodeGen/builtins-ppc-error.c
+++ clang/test/CodeGen/builtins-ppc-error.c
@@ -78,3 +78,14 @@
   vec_dss(index); //expected-error {{argument to '__builtin_altivec_dss' must 
be a constant integer}}
   vec_dss(5); //expected-error {{argument value 5 is outside the valid range 
[0, 3]}}
 }
+
+void testDST(int index) {
+  vec_dst(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dst' must be a constant integer}}
+  vec_dst(&vsi, index, 5); //expected-error {{argument value 5 is outside the 
valid range [0, 3]}}
+  vec_dstt(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dstt' must be a constant integer}}
+  vec_dstt(&vsi, index, 5); //expected-error {{argument value 5 is outside the 
valid range [0, 3]}}
+  vec_dstst(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dstst' must be a constant integer}}
+  vec_dstst(&vsi, index, 5); //expected-error {{argument value 5 is outside 
the valid range [0, 3]}}
+  vec_dststt(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dststt' must be a constant integer}}
+  vec_dststt(&vsi, index, 5); //expected-error {{argument value 5 is outside 
the valid range [0, 3]}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3231,6 +3231,11 @@
   case PPC::BI__builtin_tabortdci:
 return SemaBuiltinConstantArgRange(TheCall, 0, 0, 31) ||
SemaBuiltinConstantArgRange(TheCall, 2, 0, 31);
+  case PPC::BI__builtin_altivec_dst:
+  case PPC::BI__builtin_altivec_dstt:
+  case PPC::BI__builtin_altivec_dstst:
+  case PPC::BI__builtin_altivec_dststt:
+return SemaBuiltinConstantArgRange(TheCall, 2, 0, 3);
   case PPC::BI__builtin_vsx_xxpermdi:
   case PPC::BI__builtin_vsx_xxsldwi:
 return SemaBuiltinVSX(TheCall);
Index: clang/include/clang/Basic/BuiltinsPPC.def
===
--- clang/include/clang/Basic/BuiltinsPPC.def
+++ clang/include/clang/Basic/BuiltinsPPC.def
@@ -57,10 +57,10 @@
 
 BUILTIN(__builtin_altivec_dss, "vUIi", "")
 BUILTIN(__builtin_altivec_dssall, "v", "")
-BUILTIN(__builtin_altivec_dst, "vvC*iUi", "")
-BUILTIN(__builtin_altivec_dstt, "vvC*iUi", "")
-BUILTIN(__builtin_altivec_dstst, "vvC*iUi", "")
-BUILTIN(__builtin_altivec_dststt, "vvC*iUi", "")
+BUILTIN(__builtin_altivec_dst, "vvC*iUIi", "")
+BUILTIN(__builtin_altivec_dstt, "vvC*iUIi", "")
+BUILTIN(__builtin_altivec_dstst, "vvC*iUIi", "")
+BUILTIN(__builtin_altivec_dststt, "vvC*iUIi", "")
 
 BUILTIN(__builtin_altivec_vexptefp, "V4fV4f", "")
 


Index: clang/test/CodeGen/builtins-ppc-error.c
===
--- clang/test/CodeGen/builtins-ppc-error.c
+++ clang/test/CodeGen/builtins-ppc-error.c
@@ -78,3 +78,14 @@
   vec_dss(index); //expected-error {{argument to '__builtin_altivec_dss' must be a constant integer}}
   vec_dss(5); //expected-error {{argument value 5 is outside the valid range [0, 3]}}
 }
+
+void testDST(int index) {
+  vec_dst(&vsi, index, index); //expected-error {{argument to '__builtin_altivec_dst' must be a constant integer}}
+  vec_dst(&vsi, index, 5); //expected-error {{argument value 5 is outside the valid range [0, 3]}}
+  vec_dstt(&vsi, index, index); //expected-error {{argument to '__builtin_altivec_dstt' must be a constant integer}}
+  vec_dstt(&vsi, index, 5); //expected-error {{argument value 5 is outside the valid range [0, 3]}}
+  vec_dstst(&vsi, index, index); //expected-error {{argument to '__builtin_altivec_dstst' must be a constant integer}}
+  vec_dstst(&vsi, index, 5); //expected-error {{argument value 5 is outside the valid range [0, 3]}}
+  vec_dststt(&vsi, index, index); //expected-error {{argument to '__builtin_altivec_dststt' must be a constant integer}}
+  vec_dststt(&vsi, index, 5); //expected-error {{argument value 5 is outside the valid range [0, 3]}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3231,6 +3231,11 @@
   case PPC::BI__builtin_tabortdci:
 return SemaBuiltinConstantArgRange(TheCall, 0, 0, 31) ||
SemaBuiltinConstantArgRange(TheCall, 2, 0, 31);
+  case PPC::BI__builtin_altivec_dst:
+  case PPC::BI__builtin_altivec_dstt:
+  case PPC::BI__builtin_altivec_dstst:
+  case PPC::BI__builtin_altivec_dststt:
+   

[PATCH] D67174: Rename of constants in ASTImporterVisibilityTest. NFC.

[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 reviewer: a.sidorin.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67174

Files:
  clang/unittests/AST/ASTImporterVisibilityTest.cpp

Index: clang/unittests/AST/ASTImporterVisibilityTest.cpp
===
--- clang/unittests/AST/ASTImporterVisibilityTest.cpp
+++ clang/unittests/AST/ASTImporterVisibilityTest.cpp
@@ -290,75 +290,83 @@
   TypedTest_ImportAfterImportWithMerge();
 }
 
-const bool ExpectLink = true;
-const bool ExpectNotLink = false;
+const bool ExpectLinkedDeclChain = true;
+const bool ExpectUnlinkedDeclChain = false;
 
 INSTANTIATE_TEST_CASE_P(
 ParameterizedTests, ImportFunctionsVisibility,
 ::testing::Combine(
 DefaultTestValuesForRunOptions,
-::testing::Values(std::make_tuple(ExternF, ExternF, ExpectLink),
-  std::make_tuple(ExternF, StaticF, ExpectNotLink),
-  std::make_tuple(ExternF, AnonF, ExpectNotLink),
-  std::make_tuple(StaticF, ExternF, ExpectNotLink),
-  std::make_tuple(StaticF, StaticF, ExpectNotLink),
-  std::make_tuple(StaticF, AnonF, ExpectNotLink),
-  std::make_tuple(AnonF, ExternF, ExpectNotLink),
-  std::make_tuple(AnonF, StaticF, ExpectNotLink),
-  std::make_tuple(AnonF, AnonF, ExpectNotLink))), );
+::testing::Values(
+std::make_tuple(ExternF, ExternF, ExpectLinkedDeclChain),
+std::make_tuple(ExternF, StaticF, ExpectUnlinkedDeclChain),
+std::make_tuple(ExternF, AnonF, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticF, ExternF, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticF, StaticF, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticF, AnonF, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonF, ExternF, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonF, StaticF, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonF, AnonF, ExpectUnlinkedDeclChain))), );
 INSTANTIATE_TEST_CASE_P(
 ParameterizedTests, ImportVariablesVisibility,
 ::testing::Combine(
 DefaultTestValuesForRunOptions,
-::testing::Values(std::make_tuple(ExternV, ExternV, ExpectLink),
-  std::make_tuple(ExternV, StaticV, ExpectNotLink),
-  std::make_tuple(ExternV, AnonV, ExpectNotLink),
-  std::make_tuple(StaticV, ExternV, ExpectNotLink),
-  std::make_tuple(StaticV, StaticV, ExpectNotLink),
-  std::make_tuple(StaticV, AnonV, ExpectNotLink),
-  std::make_tuple(AnonV, ExternV, ExpectNotLink),
-  std::make_tuple(AnonV, StaticV, ExpectNotLink),
-  std::make_tuple(AnonV, AnonV, ExpectNotLink))), );
+::testing::Values(
+std::make_tuple(ExternV, ExternV, ExpectLinkedDeclChain),
+std::make_tuple(ExternV, StaticV, ExpectUnlinkedDeclChain),
+std::make_tuple(ExternV, AnonV, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticV, ExternV, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticV, StaticV, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticV, AnonV, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonV, ExternV, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonV, StaticV, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonV, AnonV, ExpectUnlinkedDeclChain))), );
 INSTANTIATE_TEST_CASE_P(
 ParameterizedTests, ImportClassesVisibility,
 ::testing::Combine(
 DefaultTestValuesForRunOptions,
-::testing::Values(std::make_tuple(ExternC, ExternC, ExpectLink),
-  std::make_tuple(ExternC, AnonC, ExpectNotLink),
-  std::make_tuple(AnonC, ExternC, ExpectNotLink),
-  std::make_tuple(AnonC, AnonC, ExpectNotLink))), );
+::testing::Values(
+std::make_tuple(ExternC, ExternC, ExpectLinkedDeclChain),
+std::make_tuple(ExternC, AnonC, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonC, ExternC, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonC, AnonC, ExpectUnlinkedDeclChain))), );
 INSTANTIATE_TEST_CASE_P(
 ParameterizedTests, ImportEnumsVisibility,
 ::testing::Combine(
 DefaultTestValuesForRunOptions,
-::testing::Values(std::make_tuple(ExternE, ExternE, ExpectLink),
-  std::make_tuple(ExternE, AnonE, ExpectNotLink),
-  std::make_tuple(AnonE, ExternE, ExpectNotLink),
-  std::make_tuple(AnonE, AnonE, ExpectNotLink))), );

[PATCH] D64480: [ASTImporter] Added visibility context check for TypedefNameDecl.

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

balazske added a comment.

Rename of constants:
https://reviews.llvm.org/D67174


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64480



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

[PATCH] D67163: [Driver] Use shared singleton instance of DriverOptTable

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Thanks, this is useful for clang-scan-deps too!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67163



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

[PATCH] D67165: [clangd][vscode] Make SemanticHighlightingFeature more self-contained.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:88
+// recover from clangd crashes.
+this.dispose();
+// Register the handler to handle semantic highlighting notification.

Could we subscribe to `client.onDidChangeState` and `dispose()` whenever clangd 
crashes or stops?
This would ensure our `initialize` <-> `dispose` calls are properly paired and 
we will remove the highlightings even if clangd never restarts (I guess that 
could happen when we crash more than 5 times in a short timespan)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67165



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

[PATCH] D66569: [analyzer] ccc-analyzer: handle --sysroot=/path in addition to --sysroot /path

[Chris Laplante via Phabricator via cfe-commits]

chris.laplante added a comment.

Ping?


Repository:
  rC Clang

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

https://reviews.llvm.org/D66569



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

[PATCH] D67165: [clangd][vscode] Make SemanticHighlightingFeature more self-contained.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 218710.
hokein marked an inline comment as done.
hokein added a comment.

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67165

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
  clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts


Index: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
@@ -44,7 +44,7 @@
 
 // Language server push notification providing the semantic highlighting
 // information for a text document.
-export const NotificationType =
+const NotificationType =
 new vscodelc.NotificationType(
 'textDocument/semanticHighlighting');
 
@@ -58,6 +58,19 @@
   highlighter: Highlighter;
   // Any disposables that should be cleaned up when clangd crashes.
   private subscriptions: vscode.Disposable[] = [];
+  constructor(client: vscodelc.BaseLanguageClient,
+  context: vscode.ExtensionContext) {
+context.subscriptions.push(client.onDidChangeState(({newState}) => {
+  if (newState == vscodelc.State.Running) {
+// Register handler for semantic highlighting notification.
+client.onNotification(NotificationType,
+  this.handleNotification.bind(this));
+  } else if (newState == vscodelc.State.Stopped) {
+// Dispose resources when clangd crashes.
+this.dispose();
+  }
+}));
+  }
   fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {
 // Extend the ClientCapabilities type and add semantic highlighting
 // capability to the object.
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
@@ -110,7 +110,8 @@
   const clangdClient = new ClangdLanguageClient('Clang Language Server',
 serverOptions, clientOptions);
   const semanticHighlightingFeature =
-  new semanticHighlighting.SemanticHighlightingFeature();
+  new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
+   context);
   context.subscriptions.push(
   vscode.Disposable.from(semanticHighlightingFeature));
   clangdClient.registerFeature(semanticHighlightingFeature);
@@ -144,14 +145,9 @@
   clangdClient.onNotification(
   'textDocument/clangd.fileStatus',
   (fileStatus) => { status.onFileUpdated(fileStatus); });
-  clangdClient.onNotification(
-  semanticHighlighting.NotificationType,
-  semanticHighlightingFeature.handleNotification.bind(
-  semanticHighlightingFeature));
 } else if (newState == vscodelc.State.Stopped) {
   // Clear all cached statuses when clangd crashes.
   status.clear();
-  semanticHighlightingFeature.dispose();
 }
   }));
   // An empty place holder for the activate command, otherwise we'll get an


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
@@ -44,7 +44,7 @@
 
 // Language server push notification providing the semantic highlighting
 // information for a text document.
-export const NotificationType =
+const NotificationType =
 new vscodelc.NotificationType(
 'textDocument/semanticHighlighting');
 
@@ -58,6 +58,19 @@
   highlighter: Highlighter;
   // Any disposables that should be cleaned up when clangd crashes.
   private subscriptions: vscode.Disposable[] = [];
+  constructor(client: vscodelc.BaseLanguageClient,
+  context: vscode.ExtensionContext) {
+context.subscriptions.push(client.onDidChangeState(({newState}) => {
+  if (newState == vscodelc.State.Running) {
+// Register handler for semantic highlighting notification.
+client.onNotification(NotificationType,
+  this.handleNotification.bind(this));
+  } else if (newState == vscodelc.State.Stopped) {
+// Dispose resources when clangd crashes.
+this.dispose();
+  }
+}));
+  }
   fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {
 // Extend the ClientCapabilities type and add semantic highlighting
 // capability to the object.
Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===

r370912 - [PowerPC][Altivec][Clang] Check compile-time constant for vec_dst*

[Jinsong Ji via cfe-commits]

Author: jsji
Date: Wed Sep  4 08:22:26 2019
New Revision: 370912

URL: http://llvm.org/viewvc/llvm-project?rev=370912&view=rev
Log:
[PowerPC][Altivec][Clang] Check compile-time constant for vec_dst*

Summary:
This is follow up of https://reviews.llvm.org/D66699.
We might get ISEL ICE if we call vec_dss with non const 3rd arg.

```
Cannot select: intrinsic %llvm.ppc.altivec.dst
```

We should check the constraints in clang and generate better error
messages.

Reviewers: nemanjai, hfinkel, echristo, #powerpc, wuzish

Reviewed By: #powerpc, wuzish

Subscribers: wuzish, kbarton, MaskRay, shchenz, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Basic/BuiltinsPPC.def
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/CodeGen/builtins-ppc-error.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsPPC.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsPPC.def?rev=370912&r1=370911&r2=370912&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsPPC.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsPPC.def Wed Sep  4 08:22:26 2019
@@ -57,10 +57,10 @@ BUILTIN(__builtin_altivec_vctuxs, "V4UiV
 
 BUILTIN(__builtin_altivec_dss, "vUIi", "")
 BUILTIN(__builtin_altivec_dssall, "v", "")
-BUILTIN(__builtin_altivec_dst, "vvC*iUi", "")
-BUILTIN(__builtin_altivec_dstt, "vvC*iUi", "")
-BUILTIN(__builtin_altivec_dstst, "vvC*iUi", "")
-BUILTIN(__builtin_altivec_dststt, "vvC*iUi", "")
+BUILTIN(__builtin_altivec_dst, "vvC*iUIi", "")
+BUILTIN(__builtin_altivec_dstt, "vvC*iUIi", "")
+BUILTIN(__builtin_altivec_dstst, "vvC*iUIi", "")
+BUILTIN(__builtin_altivec_dststt, "vvC*iUIi", "")
 
 BUILTIN(__builtin_altivec_vexptefp, "V4fV4f", "")
 

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=370912&r1=370911&r2=370912&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Sep  4 08:22:26 2019
@@ -3231,6 +3231,11 @@ bool Sema::CheckPPCBuiltinFunctionCall(u
   case PPC::BI__builtin_tabortdci:
 return SemaBuiltinConstantArgRange(TheCall, 0, 0, 31) ||
SemaBuiltinConstantArgRange(TheCall, 2, 0, 31);
+  case PPC::BI__builtin_altivec_dst:
+  case PPC::BI__builtin_altivec_dstt:
+  case PPC::BI__builtin_altivec_dstst:
+  case PPC::BI__builtin_altivec_dststt:
+return SemaBuiltinConstantArgRange(TheCall, 2, 0, 3);
   case PPC::BI__builtin_vsx_xxpermdi:
   case PPC::BI__builtin_vsx_xxsldwi:
 return SemaBuiltinVSX(TheCall);

Modified: cfe/trunk/test/CodeGen/builtins-ppc-error.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins-ppc-error.c?rev=370912&r1=370911&r2=370912&view=diff
==
--- cfe/trunk/test/CodeGen/builtins-ppc-error.c (original)
+++ cfe/trunk/test/CodeGen/builtins-ppc-error.c Wed Sep  4 08:22:26 2019
@@ -78,3 +78,14 @@ void testDSS(int index) {
   vec_dss(index); //expected-error {{argument to '__builtin_altivec_dss' must 
be a constant integer}}
   vec_dss(5); //expected-error {{argument value 5 is outside the valid range 
[0, 3]}}
 }
+
+void testDST(int index) {
+  vec_dst(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dst' must be a constant integer}}
+  vec_dst(&vsi, index, 5); //expected-error {{argument value 5 is outside the 
valid range [0, 3]}}
+  vec_dstt(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dstt' must be a constant integer}}
+  vec_dstt(&vsi, index, 5); //expected-error {{argument value 5 is outside the 
valid range [0, 3]}}
+  vec_dstst(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dstst' must be a constant integer}}
+  vec_dstst(&vsi, index, 5); //expected-error {{argument value 5 is outside 
the valid range [0, 3]}}
+  vec_dststt(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dststt' must be a constant integer}}
+  vec_dststt(&vsi, index, 5); //expected-error {{argument value 5 is outside 
the valid range [0, 3]}}
+}


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

[PATCH] D66748: [PowerPC][Altivec][Clang] Check compile-time constant for vec_dst*

[Jinsong Ji via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370912: [PowerPC][Altivec][Clang] Check compile-time 
constant for vec_dst* (authored by jsji, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66748?vs=218696&id=218718#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66748

Files:
  cfe/trunk/include/clang/Basic/BuiltinsPPC.def
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/CodeGen/builtins-ppc-error.c


Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -3231,6 +3231,11 @@
   case PPC::BI__builtin_tabortdci:
 return SemaBuiltinConstantArgRange(TheCall, 0, 0, 31) ||
SemaBuiltinConstantArgRange(TheCall, 2, 0, 31);
+  case PPC::BI__builtin_altivec_dst:
+  case PPC::BI__builtin_altivec_dstt:
+  case PPC::BI__builtin_altivec_dstst:
+  case PPC::BI__builtin_altivec_dststt:
+return SemaBuiltinConstantArgRange(TheCall, 2, 0, 3);
   case PPC::BI__builtin_vsx_xxpermdi:
   case PPC::BI__builtin_vsx_xxsldwi:
 return SemaBuiltinVSX(TheCall);
Index: cfe/trunk/include/clang/Basic/BuiltinsPPC.def
===
--- cfe/trunk/include/clang/Basic/BuiltinsPPC.def
+++ cfe/trunk/include/clang/Basic/BuiltinsPPC.def
@@ -57,10 +57,10 @@
 
 BUILTIN(__builtin_altivec_dss, "vUIi", "")
 BUILTIN(__builtin_altivec_dssall, "v", "")
-BUILTIN(__builtin_altivec_dst, "vvC*iUi", "")
-BUILTIN(__builtin_altivec_dstt, "vvC*iUi", "")
-BUILTIN(__builtin_altivec_dstst, "vvC*iUi", "")
-BUILTIN(__builtin_altivec_dststt, "vvC*iUi", "")
+BUILTIN(__builtin_altivec_dst, "vvC*iUIi", "")
+BUILTIN(__builtin_altivec_dstt, "vvC*iUIi", "")
+BUILTIN(__builtin_altivec_dstst, "vvC*iUIi", "")
+BUILTIN(__builtin_altivec_dststt, "vvC*iUIi", "")
 
 BUILTIN(__builtin_altivec_vexptefp, "V4fV4f", "")
 
Index: cfe/trunk/test/CodeGen/builtins-ppc-error.c
===
--- cfe/trunk/test/CodeGen/builtins-ppc-error.c
+++ cfe/trunk/test/CodeGen/builtins-ppc-error.c
@@ -78,3 +78,14 @@
   vec_dss(index); //expected-error {{argument to '__builtin_altivec_dss' must 
be a constant integer}}
   vec_dss(5); //expected-error {{argument value 5 is outside the valid range 
[0, 3]}}
 }
+
+void testDST(int index) {
+  vec_dst(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dst' must be a constant integer}}
+  vec_dst(&vsi, index, 5); //expected-error {{argument value 5 is outside the 
valid range [0, 3]}}
+  vec_dstt(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dstt' must be a constant integer}}
+  vec_dstt(&vsi, index, 5); //expected-error {{argument value 5 is outside the 
valid range [0, 3]}}
+  vec_dstst(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dstst' must be a constant integer}}
+  vec_dstst(&vsi, index, 5); //expected-error {{argument value 5 is outside 
the valid range [0, 3]}}
+  vec_dststt(&vsi, index, index); //expected-error {{argument to 
'__builtin_altivec_dststt' must be a constant integer}}
+  vec_dststt(&vsi, index, 5); //expected-error {{argument value 5 is outside 
the valid range [0, 3]}}
+}


Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -3231,6 +3231,11 @@
   case PPC::BI__builtin_tabortdci:
 return SemaBuiltinConstantArgRange(TheCall, 0, 0, 31) ||
SemaBuiltinConstantArgRange(TheCall, 2, 0, 31);
+  case PPC::BI__builtin_altivec_dst:
+  case PPC::BI__builtin_altivec_dstt:
+  case PPC::BI__builtin_altivec_dstst:
+  case PPC::BI__builtin_altivec_dststt:
+return SemaBuiltinConstantArgRange(TheCall, 2, 0, 3);
   case PPC::BI__builtin_vsx_xxpermdi:
   case PPC::BI__builtin_vsx_xxsldwi:
 return SemaBuiltinVSX(TheCall);
Index: cfe/trunk/include/clang/Basic/BuiltinsPPC.def
===
--- cfe/trunk/include/clang/Basic/BuiltinsPPC.def
+++ cfe/trunk/include/clang/Basic/BuiltinsPPC.def
@@ -57,10 +57,10 @@
 
 BUILTIN(__builtin_altivec_dss, "vUIi", "")
 BUILTIN(__builtin_altivec_dssall, "v", "")
-BUILTIN(__builtin_altivec_dst, "vvC*iUi", "")
-BUILTIN(__builtin_altivec_dstt, "vvC*iUi", "")
-BUILTIN(__builtin_altivec_dstst, "vvC*iUi", "")
-BUILTIN(__builtin_altivec_dststt, "vvC*iUi", "")
+BUILTIN(__builtin_altivec_dst, "vvC*iUIi", "")
+BUILTIN(__builtin_altivec_dstt, "vvC*iUIi", "")
+BUILTIN(__builtin_altivec_dstst, "vvC*iUIi", "")
+BUILTIN(__builtin_altivec_dststt, "vvC*iUIi", "")
 
 BUILTIN(__builtin_altivec_vexptefp, "V4fV4f", "")
 
Index: cfe/trunk/test/CodeGen/builtins-ppc-error.c
==

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 218720.
lebedev.ri marked 3 inline comments as done.
lebedev.ri added a comment.

Merged the check into `pointer-overflow`, revisited test coverage.
Using `EmitCheckedInBoundsGEP()` in more places TBD.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67122

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-blacklist.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset-when-nullptr-is-defined.c
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset.c
  clang/test/CodeGen/catch-pointer-overflow-volatile.c
  clang/test/CodeGen/catch-pointer-overflow.c
  clang/test/CodeGen/ubsan-pointer-overflow.m
  clang/test/CodeGenCXX/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.cpp
  compiler-rt/lib/ubsan/ubsan_checks.inc
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/index-overflow.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-constants.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-summary.cpp
  
compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
  compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
  compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -50,6 +50,20 @@
 
Makes programs 10x faster by doing Special New Thing.
 
+* As per :ref:`LLVM Language Reference Manual `,
+  ``getelementptr inbounds`` can not change the null status of a pointer,
+  meaning it can not produce non-null pointer given null base pointer, and
+  likewise given non-null base pointer it can not produce null pointer; if it
+  does, the result is a :ref:`poison value `.
+  Since `r369789 `_
+  (`D66608 `_ ``[InstCombine] icmp eq/ne (gep
+  inbounds P, Idx..), null -> icmp eq/ne P, null``) LLVM uses that for
+  transformations. If the original source violates these requirements this
+  may result in code being miscompiled. If you are using Clang front-end,
+  Undefined Behaviour Sanitizer ``-fsanitize=pointer-overflow`` check
+  will now catch such cases.
+
+
 Changes to the LLVM IR
 --
 
Index: compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
===
--- /dev/null
+++ compiler-rt/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
@@ -0,0 +1,23 @@
+// RUN: %clang -fsanitize=pointer-overflow %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
+
+#include 
+
+int main(int argc, char *argv[]) {
+  char *base, *result;
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)0;
+  result = base + 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  base = (char *)1;
+  result = base - 1;
+  // CHECK: pointer-overflow
+
+  // CHECK-NOT: pointer-overflow
+
+  return 0;
+}
Index: compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
===
--- compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
+++ compiler-rt/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
@@ -12,7 +12,7 @@
   // CHECK: unsigned-index-expression.cpp:[[@LINE+1]]:16: runtime error: subtraction of unsigned offset from 0x{{.*}} overflowed to 0x{{.*}}
   char *q1 = p - neg_1;
 
-  // CHECK: unsigned-index-expression.cpp:[[@LINE+2]]:16: runtime error: pointer index expression with base 0x{{0*}} overflowed to 0x{{.*}}
+  // CHECK: unsigned-index-expression.cpp:[[@LINE+2]]:16: runtime error: applying non-zero offset {{.*}} to null pointer
   char *n = nullptr;
   char *q2 = n - 1ULL;
 
Index: compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang -x c   -fsanitize=pointer-overflow -O0 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c   -fsanitize=pointer-overflow -O1 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c   -fsanitize=pointer-overflow -O2 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not="error:" --check-prefix=CHECK-OK
+// RUN: %clang -x c   -fsanitize=pointer-overflow -O3 %s -o %t && %run %t 2>&1 | FileCheck %s --implicit-check-not=

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4703-4720
+// 2) The sign of the difference between the computed address and the base
+// pointer matches the sign of the total offset.
+llvm::Value *ValidGEP;
+auto *NoOffsetOverflow = Builder.CreateNot(OffsetOverflows);
+if (SignedIndices) {
+  auto *PosOrZeroValid = Builder.CreateICmpUGE(ComputedGEP, IntPtr);
+  auto *PosOrZeroOffset = Builder.CreateICmpSGE(TotalOffset, Zero);

This makes me ick every time i look at it.
I wonder if this can be sanely rewritten via `.with.overflow` intrinsic..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67122



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

[PATCH] D67165: [clangd][vscode] Make SemanticHighlightingFeature more self-contained.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67165



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

[PATCH] D67096: [clangd][vscode] Add a flag to enable semantic highlighting in clangd

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:113
   const semanticHighlightingFeature =
-  new semanticHighlighting.SemanticHighlightingFeature();
+  new semanticHighlighting.SemanticHighlightingFeature(
+  getConfig('semanticHighlighting'));

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > hokein wrote:
> > > > ilya-biryukov wrote:
> > > > > hokein wrote:
> > > > > > ilya-biryukov wrote:
> > > > > > > Why not avoid calling `clangdClient.registerFeature` instead?
> > > > > > > Would allow to:
> > > > > > > - not change the `SemanticHighlightingFeature` class, keeping it 
> > > > > > > simpler,
> > > > > > > - ensure we do not do any extra work if the feature is disabled.
> > > > > > good point, done.
> > > > > Should we update other uses of `semanticHighlightingFeature` too?
> > > > > 
> > > > > `context.subscriptions.push(vscode.Disposable.from(semanticHighlightingFeature))`
> > > > >  probably ensures we call `dispose()` when the `clangdClient` is 
> > > > > getting removed, I guess we definitely don't need that.
> > > > > 
> > > > > Other uses as well:
> > > > > - no need to pass notification is highlighting is disabled.
> > > > > - no need to cleanup if highlighting is disabled.
> > > > > 
> > > > > Maybe assign null or undefined to `semanticHighlightingFeature` when 
> > > > > the flag is false?
> > > > > At each usage we can check whether the `semanticHighlightingFeature` 
> > > > > is not null and only call relevant methods if that's the case.
> > > > I don't think it is worth updating all usages, it is no harm to keep 
> > > > them here even when the highlighting is disabled (the dispose is a 
> > > > no-op; we never receive notifications from clangd); and it would add 
> > > > more guard code which I'd avoid.
> > > How can we be sure that nothing bad is going to happen?
> > > In particular, we are "binding" notification handling, but never 
> > > registered a feature. How can we be sure we won't actually get any 
> > > notifications?
> > > 
> > > If we don't create the object in the first place, we are confident 
> > > nothing harmful can be done with it.
> > > 
> > > How can we be sure we won't actually get any notifications?
> > If we receive a notification, that means we have clangd bugs. 
> > 
> > I understand you point here, an ideal solution is to avoid too many usages 
> > of `SemanticHighlightingFeature` in the client side, after D67165, it'd 
> > help simplify the patch here.
> > If we receive a notification, that means we have clangd bugs.
> True, but that might happen. It'd be better to not break in that case.
> D67165 is definitely moving in the right direction, thanks!
It should be much simpler to **not** construct the 
`semanticHighlightingFeature` with D67165, right?
```
if (getConfig('semanticHighlighting')) {
  const semanticHighlightingFeature =
  new semanticHighlighting.SemanticHighlightingFeature();
  context.subscriptions.push(
  vscode.Disposable.from(semanticHighlightingFeature));
  clangdClient.registerFeature(semanticHighlightingFeature);
}
```

Could we do that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67096



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

[PATCH] D67056: Add a bugprone-argument-comment option: IgnoreSingleArgument.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D67056#1657540 , @xyb wrote:

> Thanks. BTW, I can't commit the patch by myself.


There's an outstanding comment. Please address it first, and I'll be happy to 
commit your patch.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67056



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

[PATCH] D64375: [OpenMP][Docs] Provide implementation status details

[Johannes Doerfert via Phabricator via cfe-commits]

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



Comment at: clang/docs/OpenMPSupport.rst:204
++--+--+--++
+| device extension | clause: device_type   
   | :part:`worked on`| 
   |
++--+--+--++

ABataev wrote:
> This feature is implemented already
If we wait longer, we can always update this patch. Maybe we should upstream it 
and update it online.

@RaviNarayanaswamy @kkwli0 @ABataev 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64375



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

r370915 - [X86] Add support for avx512bf16 for __builtin_cpu_supports and compiler-rt's cpu indicator.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Wed Sep  4 09:01:43 2019
New Revision: 370915

URL: http://llvm.org/viewvc/llvm-project?rev=370915&view=rev
Log:
[X86] Add support for avx512bf16 for __builtin_cpu_supports and compiler-rt's 
cpu indicator.

Modified:
cfe/trunk/test/CodeGen/target-builtin-noerror.c

Modified: cfe/trunk/test/CodeGen/target-builtin-noerror.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-builtin-noerror.c?rev=370915&r1=370914&r2=370915&view=diff
==
--- cfe/trunk/test/CodeGen/target-builtin-noerror.c (original)
+++ cfe/trunk/test/CodeGen/target-builtin-noerror.c Wed Sep  4 09:01:43 2019
@@ -80,6 +80,7 @@ void verifyfeaturestrings() {
   (void)__builtin_cpu_supports("vpclmulqdq");
   (void)__builtin_cpu_supports("avx512vnni");
   (void)__builtin_cpu_supports("avx512bitalg");
+  (void)__builtin_cpu_supports("avx512bf16");
 }
 
 void verifycpustrings() {


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

[PATCH] D64375: [OpenMP][Docs] Provide implementation status details

[Kelvin Li via Phabricator via cfe-commits]

kkwli0 added inline comments.



Comment at: clang/docs/OpenMPSupport.rst:204
++--+--+--++
+| device extension | clause: device_type   
   | :part:`worked on`| 
   |
++--+--+--++

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > This feature is implemented already
> > If we wait longer, we can always update this patch. Maybe we should 
> > upstream it and update it online.
> > 
> > @RaviNarayanaswamy @kkwli0 @ABataev 
> Add the latest info and commit it.
Agreed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64375



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

[PATCH] D64375: [OpenMP][Docs] Provide implementation status details

[Alexey Bataev via Phabricator via cfe-commits]

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

LG




Comment at: clang/docs/OpenMPSupport.rst:204
++--+--+--++
+| device extension | clause: device_type   
   | :part:`worked on`| 
   |
++--+--+--++

jdoerfert wrote:
> ABataev wrote:
> > This feature is implemented already
> If we wait longer, we can always update this patch. Maybe we should upstream 
> it and update it online.
> 
> @RaviNarayanaswamy @kkwli0 @ABataev 
Add the latest info and commit it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64375



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

[PATCH] D64375: [OpenMP][Docs] Provide implementation status details

[Joachim Protze via Phabricator via cfe-commits]

protze.joachim added inline comments.



Comment at: clang/docs/OpenMPSupport.rst:165
++--+--+--++
+| OMPD | OMPD interfaces   
   | mostly done  ||
++--+--+--++

Hahnfeld wrote:
> jdoerfert wrote:
> > Hahnfeld wrote:
> > > This is not correct, at least it's not yet upstream.
> > Is there anything upstreamed? What should I put for status and 
> > revisions/reviews?
> I don't think there's a review yet. @protze.joachim ?
There is no review yet. Implementation is available in 
https://github.com/OpenMPToolsInterface/LLVM-openmp/tree/ompd-tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64375



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

[PATCH] D67056: Add a bugprone-argument-comment option: IgnoreSingleArgument.

[Yubo Xie via Phabricator via cfe-commits]

xyb added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp:28
+  IgnoreSingleArgument(
+  Options.getLocalOrGlobal("IgnoreSingleArgument", 0) != 0),
   CommentBoolLiterals(Options.getLocalOrGlobal("CommentBoolLiterals", 0) !=

alexfh wrote:
> Why is it a global option? Are there other checks that would need the same 
> option? The one below also needs to be check-local.
Sorry, I'm afraid I didn't get your point. Could you please explain more? This 
setting just follows the same pattern used for other settings. All other 
settings use "Options.getLocalOrGlobal(...)", I'm not sure why this setting 
need be different. Or do you mean we should change other settings 
("StrictMode", "CommentBoolLiterals", "CommentIntegerLiterals", ...) to local 
options also?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67056



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

[clang-tools-extra] r370919 - [clang-tidy] Fix bugprone-argument-comment bug if there are marcos.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Wed Sep  4 09:19:32 2019
New Revision: 370919

URL: http://llvm.org/viewvc/llvm-project?rev=370919&view=rev
Log:
[clang-tidy] Fix bugprone-argument-comment bug if there are marcos.

Summary:
Fix bugprone-argument-comment bug if there are marcos.

For example:
```
void j(int a, int b, int c);
j(X(1), /*b=*/1, X(1));
```

clang-tidy can't recognize comment "/*b=*/". It suggests fix like this:
```
j(X(1), /*b=*//*b=*/1, X(1));
```

This change tries to fix this issue.

Reviewers: alexfh, hokein, aaron.ballman

Reviewed By: alexfh

Subscribers: xazax.hun, cfe-commits

Tags: #clang, #clang-tools-extra

Patch by Yubo Xie.

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

Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp

clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-literals.cpp

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp?rev=370919&r1=370918&r2=370919&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp Wed 
Sep  4 09:19:32 2019
@@ -286,8 +286,8 @@ void ArgumentCommentCheck::checkCallArgs
   Comments = getCommentsInRange(Ctx, BeforeArgument);
 } else {
   // Fall back to parsing back from the start of the argument.
-  CharSourceRange ArgsRange = MakeFileCharRange(
-  Args[I]->getBeginLoc(), Args[NumArgs - 1]->getEndLoc());
+  CharSourceRange ArgsRange =
+  MakeFileCharRange(Args[I]->getBeginLoc(), Args[I]->getEndLoc());
   Comments = getCommentsBeforeLoc(Ctx, ArgsRange.getBegin());
 }
 

Modified: 
clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-literals.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-literals.cpp?rev=370919&r1=370918&r2=370919&view=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-literals.cpp 
(original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-literals.cpp 
Wed Sep  4 09:19:32 2019
@@ -15,10 +15,12 @@ struct A {
 };
 
 #define FOO 1
+#define X(x) (x)
 
 void g(int a);
 void h(double b);
 void i(const char *c);
+void j(int a, int b, int c);
 
 double operator"" _km(long double);
 
@@ -106,6 +108,39 @@ void test() {
   // CHECK-FIXES: h(/*b=*/1.0f);
   i(__FILE__);
 
+  j(1, X(1), X(1));
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for 
literal argument 'a' [bugprone-argument-comment]
+  // CHECK-FIXES: j(/*a=*/1, X(1), X(1));
+  j(/*a=*/1, X(1), X(1));
+
+  j(X(1), 1, X(1));
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: argument comment missing for 
literal argument 'b' [bugprone-argument-comment]
+  // CHECK-FIXES: j(X(1), /*b=*/1, X(1));
+  j(X(1), /*b=*/1, X(1));
+
+  j(X(1), X(1), 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: argument comment missing for 
literal argument 'c' [bugprone-argument-comment]
+  // CHECK-FIXES: j(X(1), X(1), /*c=*/1);
+  j(X(1), X(1), /*c=*/1);
+
+  j(X(1), 1, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: argument comment missing for 
literal argument 'b' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:14: warning: argument comment missing for 
literal argument 'c' [bugprone-argument-comment]
+  // CHECK-FIXES: j(X(1), /*b=*/1, /*c=*/1);
+  j(X(1), /*b=*/1, /*c=*/1);
+
+  j(1, X(1), 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for 
literal argument 'a' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:14: warning: argument comment missing for 
literal argument 'c' [bugprone-argument-comment]
+  // CHECK-FIXES: j(/*a=*/1, X(1), /*c=*/1);
+  j(/*a=*/1, X(1), /*c=*/1);
+
+  j(1, 1, X(1));
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for 
literal argument 'a' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:8: warning: argument comment missing for 
literal argument 'b' [bugprone-argument-comment]
+  // CHECK-FIXES: j(/*a=*/1, /*b=*/1, X(1));
+  j(/*a=*/1, /*b=*/1, X(1));
+
   // FIXME Would like the below to add argument comments.
   g((1));
   // FIXME But we should not add argument comments here.


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

[PATCH] D67080: [clang-tidy] Fix bugprone-argument-comment bug if there are marcos.

[Alexander Kornienko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370919: [clang-tidy] Fix bugprone-argument-comment bug if 
there are marcos. (authored by alexfh, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67080?vs=218372&id=218730#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67080

Files:
  clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-literals.cpp


Index: 
clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-literals.cpp
===
--- 
clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-literals.cpp
+++ 
clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-literals.cpp
@@ -15,10 +15,12 @@
 };
 
 #define FOO 1
+#define X(x) (x)
 
 void g(int a);
 void h(double b);
 void i(const char *c);
+void j(int a, int b, int c);
 
 double operator"" _km(long double);
 
@@ -106,6 +108,39 @@
   // CHECK-FIXES: h(/*b=*/1.0f);
   i(__FILE__);
 
+  j(1, X(1), X(1));
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for 
literal argument 'a' [bugprone-argument-comment]
+  // CHECK-FIXES: j(/*a=*/1, X(1), X(1));
+  j(/*a=*/1, X(1), X(1));
+
+  j(X(1), 1, X(1));
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: argument comment missing for 
literal argument 'b' [bugprone-argument-comment]
+  // CHECK-FIXES: j(X(1), /*b=*/1, X(1));
+  j(X(1), /*b=*/1, X(1));
+
+  j(X(1), X(1), 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: argument comment missing for 
literal argument 'c' [bugprone-argument-comment]
+  // CHECK-FIXES: j(X(1), X(1), /*c=*/1);
+  j(X(1), X(1), /*c=*/1);
+
+  j(X(1), 1, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: argument comment missing for 
literal argument 'b' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:14: warning: argument comment missing for 
literal argument 'c' [bugprone-argument-comment]
+  // CHECK-FIXES: j(X(1), /*b=*/1, /*c=*/1);
+  j(X(1), /*b=*/1, /*c=*/1);
+
+  j(1, X(1), 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for 
literal argument 'a' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:14: warning: argument comment missing for 
literal argument 'c' [bugprone-argument-comment]
+  // CHECK-FIXES: j(/*a=*/1, X(1), /*c=*/1);
+  j(/*a=*/1, X(1), /*c=*/1);
+
+  j(1, 1, X(1));
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for 
literal argument 'a' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:8: warning: argument comment missing for 
literal argument 'b' [bugprone-argument-comment]
+  // CHECK-FIXES: j(/*a=*/1, /*b=*/1, X(1));
+  j(/*a=*/1, /*b=*/1, X(1));
+
   // FIXME Would like the below to add argument comments.
   g((1));
   // FIXME But we should not add argument comments here.
Index: clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -286,8 +286,8 @@
   Comments = getCommentsInRange(Ctx, BeforeArgument);
 } else {
   // Fall back to parsing back from the start of the argument.
-  CharSourceRange ArgsRange = MakeFileCharRange(
-  Args[I]->getBeginLoc(), Args[NumArgs - 1]->getEndLoc());
+  CharSourceRange ArgsRange =
+  MakeFileCharRange(Args[I]->getBeginLoc(), Args[I]->getEndLoc());
   Comments = getCommentsBeforeLoc(Ctx, ArgsRange.getBegin());
 }
 


Index: clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-literals.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-literals.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-literals.cpp
@@ -15,10 +15,12 @@
 };
 
 #define FOO 1
+#define X(x) (x)
 
 void g(int a);
 void h(double b);
 void i(const char *c);
+void j(int a, int b, int c);
 
 double operator"" _km(long double);
 
@@ -106,6 +108,39 @@
   // CHECK-FIXES: h(/*b=*/1.0f);
   i(__FILE__);
 
+  j(1, X(1), X(1));
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for literal argument 'a' [bugprone-argument-comment]
+  // CHECK-FIXES: j(/*a=*/1, X(1), X(1));
+  j(/*a=*/1, X(1), X(1));
+
+  j(X(1), 1, X(1));
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: argument comment missing for literal argument 'b' [bugprone-argument-comment]
+  // CHECK-FIXES: j(X(1), /*b=*/1, X(1));
+  j(X(1), /*b=*/1, X(1));
+
+  j(X(1), X(1), 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: argument comment missing for literal argument 'c' [bugprone-argument-comment]
+  // CHECK-FI

[PATCH] D67185: [RISCV] Add support for -ffixed-xX flags

[Simon Cook via Phabricator via cfe-commits]

simoncook created this revision.
simoncook added reviewers: asb, lenary.
Herald added subscribers: llvm-commits, cfe-commits, pzheng, s.egerton, Jim, 
benna, psnobl, jocewei, PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, 
rogfer01, edward-jones, zzheng, MaskRay, jrtc27, shiva0217, kito-cheng, niosHD, 
sabuasal, apazos, johnrusso, rbar, hiraditya, kristof.beyls, javed.absar.
Herald added projects: clang, LLVM.

This adds support for reserving GPRs such that the compiler will not
choose a register for register allocation. The implementation follows
the same design as for AArch64; each reserved register becomes a target
feature and used for getting the reserved registers for a given
MachineFunction. It is possible to reserve registers that the compiler
later ignores (such as x2/sp); this matches GCC's behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67185

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv-fixed-x-register.c
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/reserved-regs.ll

Index: llvm/test/CodeGen/RISCV/reserved-regs.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/reserved-regs.ll
@@ -0,0 +1,135 @@
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x1 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X1
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x1 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X1
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x2 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X2
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x2 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X2
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x3 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X3
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x3 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X3
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x4 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X4
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x4 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X4
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x5 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X5
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x5 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X5
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x6 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X6
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x6 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X6
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x7 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X7
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x7 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X7
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x8 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X8
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x8 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X8
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x9 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X9
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x9 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X9
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x10 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X10
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x10 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X10
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x11 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X11
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x11 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X11
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x12 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X12
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x12 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X12
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x13 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X13
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x13 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X13
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x14 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X14
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x14 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X14
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x15 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X15
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x15 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X15
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x16 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X16
+; RUN: llc -mtriple=riscv64 -mattr=+reserve-x16 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X16
+; RUN: llc -mtriple=riscv32 -mattr=+reserve-x17 -verify-machineinstrs < %s | FileCheck %s -check-prefix=X17
+; R

[PATCH] D67056: Add a bugprone-argument-comment option: IgnoreSingleArgument.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp:28
+  IgnoreSingleArgument(
+  Options.getLocalOrGlobal("IgnoreSingleArgument", 0) != 0),
   CommentBoolLiterals(Options.getLocalOrGlobal("CommentBoolLiterals", 0) !=

xyb wrote:
> alexfh wrote:
> > Why is it a global option? Are there other checks that would need the same 
> > option? The one below also needs to be check-local.
> Sorry, I'm afraid I didn't get your point. Could you please explain more? 
> This setting just follows the same pattern used for other settings. All other 
> settings use "Options.getLocalOrGlobal(...)", I'm not sure why this setting 
> need be different. Or do you mean we should change other settings 
> ("StrictMode", "CommentBoolLiterals", "CommentIntegerLiterals", ...) to local 
> options also?
Options are stored as a string -> string map. The key in this map is either the 
option name prepended with the check name (for check-local options) or just the 
option name (this way an option can be read by all checks). There are two ways 
to read options: OptionsView::get reads just the local name, 
OptionsView::getLocalOrGlobal tries the check-local name and then falls back to 
reading the option using its global name.

In this particular check only the StrictMode option makes sense to be global (a 
few other checks define an option with the same name and set of values, and it 
may make sense to configure a global default value). Other options are specific 
to this check and should be local.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67056



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

[PATCH] D67084: [clang-tidy] Fix bugprone-argument-comment bug: negative literal number is not checked.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D67084#1657534 , @xyb wrote:

> Thanks. BTW, I can't commit the patch by myself.


The patch doesn't apply cleanly. Please rebase it against current HEAD.


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

https://reviews.llvm.org/D67084



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