[PATCH] D57528: [tools] Fix python DeprecationWarning: invalid escape sequence

[serge via Phabricator via cfe-commits]

serge-sans-paille accepted this revision.
serge-sans-paille added a comment.
This revision is now accepted and ready to land.
Herald added a project: clang.

LGTM, do you have commit right?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57528



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

[PATCH] D57951: [Lex] Allow to set missing include error to not fatal

[Ivan Donchevskii via Phabricator via cfe-commits]

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

It's probably not needed because I don't see a path which checked for the fatal 
errors in 8.0. So i will probably abandon this one or update if it does not 
cover the case I need.


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

https://reviews.llvm.org/D57951



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

[PATCH] D28248: Work around GCC PR37804

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added a comment.

Hey Marshall and Michael,

As mentioned in my email to all the lists[1], patches posted to Phabricator 
before the new license was installed should be confirmed as under the new 
license before being rebased and applied. Not sure that happened here as the 
file headers are still the old file headers.

I'll update the file headers anyways, and I think this is fine as Michael is 
contributing with an @apple.com email address and so all of this is covered by 
their agreement anyways. But I wanted to mention it in case there are other 
in-flight patches on Phabricator where this is relevant.

-Chandler

[1]: http://lists.llvm.org/pipermail/llvm-dev/2018-December/128695.html 
(llvm-dev), http://lists.llvm.org/pipermail/libcxx-dev/2019-January/000140.html 
(libcxx-dev)


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

https://reviews.llvm.org/D28248



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

[PATCH] D57532: [Index] Make sure c-index-test finds libc++ on Mac

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov planned changes to this revision.
ilya-biryukov added a comment.

Still need to figure out why the test fails and fix it before submitting the 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57532



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

[PATCH] D57896: Variable names rule

[Michael Platings via Phabricator via cfe-commits]

michaelplatings added a comment.

In D57896#1390517 , @MyDeveloperDay 
wrote:

> Should we come up with a new style?  say `UpperOrLowerCamelCase`,   I don't 
> mind going and doing that in the readability-identifier-naming check, given 
> that I just wrote up all the Options for that check 
> https://clang.llvm.org/extra/clang-tidy/checks/readability-identifier-naming.html
>  in D56563: [clang-tidy]  add options documentation to 
> readability-identifier-naming checker 


Sounds good to me. I see that you've made D57966 
 a child of this issue, but we could swap the 
dependency around so that once your patch is applied I can update this patch to 
use `camelBackOrCase`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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

[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D56370#1391924 , @nridge wrote:

> As far as reworking the tests to use these functions, I've thought about that 
> a bit:
>
> - These functions return AST nodes. It's not clear to me how I would come up 
> with "expected" AST nodes to test the return values against.


See FindDecl

> - Even if we find a way to get "expected" AST nodes, we would be losing test 
> coverage of functions like `declToTypeHierarchyItem()` (though I suppose we 
> could write separate tests for that).
> - We could instead test against the //properties// of the AST nodes, such as 
> their names and source ranges, but then the test code to query those 
> properties would basically be duplicating code in 
> `declToTypeHierarchyItem()`. It would seem to make more sense to just test 
> the resulting `TypeHierarchyItem` instead (as the tests are doing now).






Comment at: clang-tools-extra/clangd/ClangdServer.cpp:543
+  return CB(InpAST.takeError());
+CB(clangd::getTypeHierarchy(InpAST->AST, Item.range.start, Resolve,
+Direction));

relying on the item range to resolve the actual symbol isn't reliable:
 - the source code may have changed
 - the range may not be within the TU, and might be e.g. in an indexed header 
we don't have a compile command for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56370



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

Re: [PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

[Sam McCall via cfe-commits]

(Sorry, hit enter too soon and truncated one of the comments)

On Mon, Feb 11, 2019 at 10:32 AM Sam McCall via Phabricator <
revi...@reviews.llvm.org> wrote:

> sammccall added a comment.
>
> In D56370#1391924 , @nridge
> wrote:
>
> > As far as reworking the tests to use these functions, I've thought about
> that a bit:
> >
> > - These functions return AST nodes. It's not clear to me how I would
> come up with "expected" AST nodes to test the return values against.
> See FindDecl
>
See the findDecl overloads in TestTU.h - we use these for such tests.

> - Even if we find a way to get "expected" AST nodes, we would be losing
> test coverage of functions like `declToTypeHierarchyItem()` (though I
> suppose we could write separate tests for that).
>
Yes, please do add unit tests for the functions separately - findDecl()
also words to get the input to that function.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58029: [clangd] Make system header mappings available for PreambleParsedCallback

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

SystemHeaderMappings were added only after takeIncludes call, which
resulted in getting mapping on main file ast updates but not on preamble ast
updates.
Fixes https://github.com/clangd/clangd/issues/8


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D58029

Files:
  clangd/ClangdUnit.cpp
  unittests/clangd/FileIndexTests.cpp


Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -212,6 +212,39 @@
   "");
 }
 
+TEST(FileIndexTest, HasSystemHeaderMappingsInPreamble) {
+  FileIndex Index;
+  const std::string Header = R"cpp(
+class Foo {};
+  )cpp";
+  auto MainFile = testPath("foo.cpp");
+  auto HeaderFile = testPath("bits/alloc_traits.h");
+  std::vector Cmd = {"clang", "-xc++", MainFile.c_str(),
+   "-include", HeaderFile.c_str()};
+  // Preparse ParseInputs.
+  ParseInputs PI;
+  PI.CompileCommand.Directory = testRoot();
+  PI.CompileCommand.Filename = MainFile;
+  PI.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+  PI.Contents = "";
+  PI.FS = buildTestFS({{MainFile, ""}, {HeaderFile, Header}});
+
+  // Prepare preamble.
+  auto CI = buildCompilerInvocation(PI);
+  auto PreambleData = buildPreamble(
+  MainFile, *buildCompilerInvocation(PI), /*OldPreamble=*/nullptr,
+  tooling::CompileCommand(), PI, 
std::make_shared(),
+  /*StoreInMemory=*/true,
+  [&](ASTContext &Ctx, std::shared_ptr PP,
+  const CanonicalIncludes &Includes) {
+Index.updatePreamble(MainFile, Ctx, PP, Includes);
+  });
+  auto Symbols = runFuzzyFind(Index, "");
+  EXPECT_THAT(Symbols, ElementsAre(_));
+  EXPECT_THAT(Symbols.begin()->IncludeHeaders.front().IncludeHeader,
+  "");
+}
+
 TEST(FileIndexTest, TemplateParamsInLabel) {
   auto Source = R"cpp(
 template 
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -96,14 +96,13 @@
 class CppFilePreambleCallbacks : public PreambleCallbacks {
 public:
   CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
-  : File(File), ParsedCallback(ParsedCallback) {}
+  : File(File), ParsedCallback(ParsedCallback) {
+addSystemHeadersMapping(&CanonIncludes);
+  }
 
   IncludeStructure takeIncludes() { return std::move(Includes); }
 
-  CanonicalIncludes takeCanonicalIncludes() {
-addSystemHeadersMapping(&CanonIncludes);
-return std::move(CanonIncludes);
-  }
+  CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); 
}
 
   void AfterExecute(CompilerInstance &CI) override {
 if (!ParsedCallback)


Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -212,6 +212,39 @@
   "");
 }
 
+TEST(FileIndexTest, HasSystemHeaderMappingsInPreamble) {
+  FileIndex Index;
+  const std::string Header = R"cpp(
+class Foo {};
+  )cpp";
+  auto MainFile = testPath("foo.cpp");
+  auto HeaderFile = testPath("bits/alloc_traits.h");
+  std::vector Cmd = {"clang", "-xc++", MainFile.c_str(),
+   "-include", HeaderFile.c_str()};
+  // Preparse ParseInputs.
+  ParseInputs PI;
+  PI.CompileCommand.Directory = testRoot();
+  PI.CompileCommand.Filename = MainFile;
+  PI.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+  PI.Contents = "";
+  PI.FS = buildTestFS({{MainFile, ""}, {HeaderFile, Header}});
+
+  // Prepare preamble.
+  auto CI = buildCompilerInvocation(PI);
+  auto PreambleData = buildPreamble(
+  MainFile, *buildCompilerInvocation(PI), /*OldPreamble=*/nullptr,
+  tooling::CompileCommand(), PI, std::make_shared(),
+  /*StoreInMemory=*/true,
+  [&](ASTContext &Ctx, std::shared_ptr PP,
+  const CanonicalIncludes &Includes) {
+Index.updatePreamble(MainFile, Ctx, PP, Includes);
+  });
+  auto Symbols = runFuzzyFind(Index, "");
+  EXPECT_THAT(Symbols, ElementsAre(_));
+  EXPECT_THAT(Symbols.begin()->IncludeHeaders.front().IncludeHeader,
+  "");
+}
+
 TEST(FileIndexTest, TemplateParamsInLabel) {
   auto Source = R"cpp(
 template 
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -96,14 +96,13 @@
 class CppFilePreambleCallbacks : public PreambleCallbacks {
 public:
   CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
-  : File(File), ParsedCallback(ParsedCallback) {}
+  : File(File), ParsedCallback(ParsedCallback) {
+addSystemHeadersMappin

[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:925
+CXXRD = VD->getType().getTypePtr()->getAsCXXRecordDecl();
+  } else if (const CXXMethodDecl *Method = dyn_cast(D)) {
+// If this is a method, use the type of the class.

nridge wrote:
> kadircet wrote:
> > what about member fields ?
> It's not clear what the desired semantics would be for a member field: get 
> the type hierarchy of the enclosing class type, or the type hierarchy of the 
> field's type?
I think it is sensible to go for enclosing type. But up to you, in any case 
could you add a comment stating how `FieldDecl` are handled?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56370



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

[PATCH] D57739: [clangd] Format tweak's replacements.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D57739#1390374 , @ilya-biryukov 
wrote:

> In D57739#1390321 , @sammccall wrote:
>
> > It's not about stability or whether the functionality is desired, but 
> > layering.
> >  Unit tests having narrow scope is a good thing - if we want system tests 
> > that check clangdserver's behavior, they should test clangdserver.
> >  Clients that don't go through clangdserver probably want formatting, but 
> > an immediate cleanupAndFormat on each generated change isn't necessarily 
> > the right way to do that.
>
>
> My argument is that it's better to provide formatting by default in the 
> public interface for **applying a single tweak**.
>  I might be missing some use-cases, e.g. one that comes to mind is applying 
> multiple tweaks in a row, in which case we'd want to format once and not for 
> every tweak.


If providing formatting was free, I'd be fine with this, but it leaks into the 
public interface in two places: a) it requires the caller to plumb through a 
formatting style, and b) it is another source of errors.

For this particular interface it's more important that it's conceptually clear 
and easy to implement than it is that it's easy to call - this is an extension 
point.

> My concerns are code duplication and ease of use for the clients. Having both 
> `apply` and `applyAndFormat` (the latter being a non-virtual or a 
> free-standing utility function) in the public interface of the `Tweak` would 
> totally do it for me.

I'd be happy with `applyAndFormat` as a free function - my main concern is that 
formatting not be part of the scope of the class.

> However, I also think tests should format by default, not sure we agree on 
> this.
>  WDYT?

I'd rather they didn't format, but I don't think it will matter much and I'm 
happy either way.

(Where it does matter, I'd rather have the differences between input/output be 
a result of the tweak replacements, not of different formatting triggered by a 
different token sequence.
I don't think there's much point in testing clang-format here, though we should 
have one test that verifies we're calling it at all.)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739



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

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[pierre gousseau via Phabricator via cfe-commits]

pgousseau updated this revision to Diff 186201.
pgousseau added a comment.

Move num bits constant inside class.


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

https://reviews.llvm.org/D57914

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/Sanitizers.h
  include/clang/Driver/ToolChain.h
  lib/Basic/SanitizerSpecialCaseList.cpp
  lib/Basic/Sanitizers.cpp
  lib/CodeGen/CGExpr.cpp
  lib/Driver/SanitizerArgs.cpp

Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -25,29 +25,32 @@
 using namespace clang::driver;
 using namespace llvm::opt;
 
-enum : SanitizerMask {
-  NeedsUbsanRt = Undefined | Integer | ImplicitConversion | Nullability | CFI,
-  NeedsUbsanCxxRt = Vptr | CFI,
-  NotAllowedWithTrap = Vptr,
-  NotAllowedWithMinimalRuntime = Vptr,
-  RequiresPIE = DataFlow | HWAddress | Scudo,
-  NeedsUnwindTables = Address | HWAddress | Thread | Memory | DataFlow,
-  SupportsCoverage = Address | HWAddress | KernelAddress | KernelHWAddress |
- Memory | KernelMemory | Leak | Undefined | Integer |
- ImplicitConversion | Nullability | DataFlow | Fuzzer |
- FuzzerNoLink,
-  RecoverableByDefault = Undefined | Integer | ImplicitConversion | Nullability,
-  Unrecoverable = Unreachable | Return,
-  AlwaysRecoverable = KernelAddress | KernelHWAddress,
-  LegacyFsanitizeRecoverMask = Undefined | Integer,
-  NeedsLTO = CFI,
-  TrappingSupported = (Undefined & ~Vptr) | UnsignedIntegerOverflow |
-  ImplicitConversion | Nullability | LocalBounds | CFI,
-  TrappingDefault = CFI,
-  CFIClasses =
-  CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
-};
+SanitizerMask NeedsUbsanRt =
+Undefined | Integer | ImplicitConversion | Nullability | CFI;
+SanitizerMask NeedsUbsanCxxRt = Vptr | CFI;
+SanitizerMask NotAllowedWithTrap = Vptr;
+SanitizerMask NotAllowedWithMinimalRuntime = Vptr;
+SanitizerMask RequiresPIE = DataFlow | HWAddress | Scudo;
+SanitizerMask NeedsUnwindTables =
+Address | HWAddress | Thread | Memory | DataFlow;
+SanitizerMask SupportsCoverage =
+Address | HWAddress | KernelAddress | KernelHWAddress | Memory |
+KernelMemory | Leak | Undefined | Integer | ImplicitConversion |
+Nullability | DataFlow | Fuzzer | FuzzerNoLink;
+SanitizerMask RecoverableByDefault =
+Undefined | Integer | ImplicitConversion | Nullability;
+SanitizerMask Unrecoverable = Unreachable | Return;
+SanitizerMask AlwaysRecoverable = KernelAddress | KernelHWAddress;
+SanitizerMask LegacyFsanitizeRecoverMask = Undefined | Integer;
+SanitizerMask NeedsLTO = CFI;
+SanitizerMask TrappingSupported = (Undefined & ~Vptr) |
+  UnsignedIntegerOverflow | ImplicitConversion |
+  Nullability | LocalBounds | CFI;
+SanitizerMask TrappingDefault = CFI;
+SanitizerMask CFIClasses =
+CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast;
+SanitizerMask CompatibleWithMinimalRuntime =
+TrappingSupported | Scudo | ShadowCallStack;
 
 enum CoverageFeature {
   CoverageFunc = 1 << 0,
@@ -136,10 +139,10 @@
 
 static SanitizerMask parseSanitizeTrapArgs(const Driver &D,
const llvm::opt::ArgList &Args) {
-  SanitizerMask TrapRemove = 0; // During the loop below, the accumulated set of
+  SanitizerMask TrapRemove(0);  // During the loop below, the accumulated set of
 // sanitizers disabled by the current sanitizer
 // argument or any argument after it.
-  SanitizerMask TrappingKinds = 0;
+  SanitizerMask TrappingKinds(0);
   SanitizerMask TrappingSupportedWithGroups = setGroupBits(TrappingSupported);
 
   for (ArgList::const_reverse_iterator I = Args.rbegin(), E = Args.rend();
@@ -203,24 +206,26 @@
 }
 
 bool SanitizerArgs::needsUnwindTables() const {
-  return Sanitizers.Mask & NeedsUnwindTables;
+  return static_cast(Sanitizers.Mask & NeedsUnwindTables);
 }
 
-bool SanitizerArgs::needsLTO() const { return Sanitizers.Mask & NeedsLTO; }
+bool SanitizerArgs::needsLTO() const {
+  return static_cast(Sanitizers.Mask & NeedsLTO);
+}
 
 SanitizerArgs::SanitizerArgs(const ToolChain &TC,
  const llvm::opt::ArgList &Args) {
-  SanitizerMask AllRemove = 0;  // During the loop below, the accumulated set of
+  SanitizerMask AllRemove(0);   // During the loop below, the accumulated set of
 // sanitizers disabled by the current sanitizer
 // argument or any argument after it.
-  SanitizerMask AllAddedKinds = 0;  // Mask of all sanitizers ever enabled by
+  SanitizerMask AllAddedKinds(0);   // Mask of all sanitizers ever enabled by
 

[PATCH] D54978: Move the SMT API to LLVM

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

Szelethus added a comment.

Shouldn't that be off by default?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D57232: [ASTImporter] Check visibility/linkage of functions and variables

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D57232



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

[PATCH] D57943: [clangd] **Prototype**: clang-tidy-based tweaks

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This is an intriguing idea, and is at least useful to prototype new tweaks.

I'm not sure whether clang-tidy is the ultimately right API to write tweaks:

- it doesn't have the needed constraints to ensure prepare() is fast (e.g. it 
emits diagnostics and fixes eagerly)
- the exact set of nodes that it will trigger on may or may not match what we 
want
- it doesn't produce context-sensitive titles

Maybe we should have this behind a flag, and use it to investigate which tweaks 
should be implemented natively?




Comment at: clangd/refactor/tweaks/ClangTidy.cpp:45
+/// Limitations:
+///- checks only match single AST node would work
+///- checks don't see any preprocessor events

I can't quite parse this.
"only checks whose matchers match exactly the selected node will work"?

One concern here is that this is basically an implementation detail of a check: 
checks can choose to e.g. match on a specific node and do a cheap analysis, or 
match on a higher-level node and do an expensive one. Maybe as long as the 
specific checks we want work well, it's not an issue. But every check added 
here needs to be carefully reviewed for performance.



Comment at: clangd/refactor/tweaks/ClangTidy.cpp:66
+
+bool ClangTidyTweak::prepare(const Selection &Inputs) {
+  if (!Inputs.ASTSelection.commonAncestor())

obviously this is central to the approach, but... prepare needs to be *fast*. 
It's hard to see how we guarantee that with all this machinery, especially 
because the match callback contents may change over time.



Comment at: clangd/refactor/tweaks/ClangTidy.cpp:83
+  // Run the check on the AST node under the cursor.
+  CTFinder.match(Inputs.ASTSelection.commonAncestor()->ASTNode,
+ Inputs.AST.getASTContext());

looping over all the ancestors might give better results (but leaves less 
control over performance).



Comment at: clangd/refactor/tweaks/ClangTidy.cpp:141
+
+  std::string title() const override { return "Covert type to auto"; }
+};

this should really specify which type is getting converted - seems like a 
possible weakness of this approach.



Comment at: unittests/clangd/TweakTests.cpp:231
+  llvm::StringLiteral Input =
+  "void f() { [[unsigned c = static_cast(1);]] }";
+  llvm::StringLiteral Output =

this needs to also trigger if you select "unsigned"


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57943



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

[PATCH] D57236: [ASTImporter] Unify redecl chain tests as type parameterized tests

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Ping.
Please raise your objections if you have any until the 18th of February (that 
date I am going to commit if there are no objections). Also, please let me know 
if you find this deadline too strict.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57236



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

r353684 - [ASTImporter] Add test RedeclChainShouldBeCorrectAmongstNamespaces

[Gabor Marton via cfe-commits]

Author: martong
Date: Mon Feb 11 02:27:58 2019
New Revision: 353684

URL: http://llvm.org/viewvc/llvm-project?rev=353684&view=rev
Log:
[ASTImporter] Add test RedeclChainShouldBeCorrectAmongstNamespaces

Summary:
We add a new test to show that redecl chains are not handled properly
amongst namespaces. We cannot pass this test now, so this is disabled.
Subsequent patches will make this test pass.

Reviewers: a_sidorin, shafik, a.sidorin

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

Tags: #clang

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

Modified:
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=353684&r1=353683&r2=353684&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Mon Feb 11 02:27:58 2019
@@ -5143,6 +5143,47 @@ INSTANTIATE_TEST_CASE_P(
 ParameterizedTests, CanonicalRedeclChain,
 ::testing::Values(ArgVector()),);
 
+// FIXME This test is disabled currently, upcoming patches will make it
+// possible to enable.
+TEST_P(ASTImporterOptionSpecificTestBase,
+   DISABLED_RedeclChainShouldBeCorrectAmongstNamespaces) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  namespace NS {
+struct X;
+struct Y {
+  static const int I = 3;
+};
+  }
+  namespace NS {
+struct X {  // <--- To be imported
+  void method(int i = Y::I) {}
+  int f;
+};
+  }
+  )",
+  Lang_CXX);
+  auto *FromFwd = FirstDeclMatcher().match(
+  FromTU, cxxRecordDecl(hasName("X"), unless(isImplicit(;
+  auto *FromDef = LastDeclMatcher().match(
+  FromTU,
+  cxxRecordDecl(hasName("X"), isDefinition(), unless(isImplicit(;
+  ASSERT_NE(FromFwd, FromDef);
+  ASSERT_FALSE(FromFwd->isThisDeclarationADefinition());
+  ASSERT_TRUE(FromDef->isThisDeclarationADefinition());
+  ASSERT_EQ(FromFwd->getCanonicalDecl(), FromDef->getCanonicalDecl());
+
+  auto *ToDef = cast_or_null(Import(FromDef, Lang_CXX));
+  auto *ToFwd = cast_or_null(Import(FromFwd, Lang_CXX));
+  EXPECT_NE(ToFwd, ToDef);
+  EXPECT_FALSE(ToFwd->isThisDeclarationADefinition());
+  EXPECT_TRUE(ToDef->isThisDeclarationADefinition());
+  EXPECT_EQ(ToFwd->getCanonicalDecl(), ToDef->getCanonicalDecl());
+  auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  // We expect no (ODR) warning during the import.
+  EXPECT_EQ(0u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
 DefaultTestValuesForRunOptions, );
 


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

[PATCH] D57901: [ASTImporter] Add test RedeclChainShouldBeCorrectAmongstNamespaces

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353684: [ASTImporter] Add test 
RedeclChainShouldBeCorrectAmongstNamespaces (authored by martong, committed by 
).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57901

Files:
  cfe/trunk/unittests/AST/ASTImporterTest.cpp


Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -5143,6 +5143,47 @@
 ParameterizedTests, CanonicalRedeclChain,
 ::testing::Values(ArgVector()),);
 
+// FIXME This test is disabled currently, upcoming patches will make it
+// possible to enable.
+TEST_P(ASTImporterOptionSpecificTestBase,
+   DISABLED_RedeclChainShouldBeCorrectAmongstNamespaces) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  namespace NS {
+struct X;
+struct Y {
+  static const int I = 3;
+};
+  }
+  namespace NS {
+struct X {  // <--- To be imported
+  void method(int i = Y::I) {}
+  int f;
+};
+  }
+  )",
+  Lang_CXX);
+  auto *FromFwd = FirstDeclMatcher().match(
+  FromTU, cxxRecordDecl(hasName("X"), unless(isImplicit(;
+  auto *FromDef = LastDeclMatcher().match(
+  FromTU,
+  cxxRecordDecl(hasName("X"), isDefinition(), unless(isImplicit(;
+  ASSERT_NE(FromFwd, FromDef);
+  ASSERT_FALSE(FromFwd->isThisDeclarationADefinition());
+  ASSERT_TRUE(FromDef->isThisDeclarationADefinition());
+  ASSERT_EQ(FromFwd->getCanonicalDecl(), FromDef->getCanonicalDecl());
+
+  auto *ToDef = cast_or_null(Import(FromDef, Lang_CXX));
+  auto *ToFwd = cast_or_null(Import(FromFwd, Lang_CXX));
+  EXPECT_NE(ToFwd, ToDef);
+  EXPECT_FALSE(ToFwd->isThisDeclarationADefinition());
+  EXPECT_TRUE(ToDef->isThisDeclarationADefinition());
+  EXPECT_EQ(ToFwd->getCanonicalDecl(), ToDef->getCanonicalDecl());
+  auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  // We expect no (ODR) warning during the import.
+  EXPECT_EQ(0u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
 DefaultTestValuesForRunOptions, );
 


Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -5143,6 +5143,47 @@
 ParameterizedTests, CanonicalRedeclChain,
 ::testing::Values(ArgVector()),);
 
+// FIXME This test is disabled currently, upcoming patches will make it
+// possible to enable.
+TEST_P(ASTImporterOptionSpecificTestBase,
+   DISABLED_RedeclChainShouldBeCorrectAmongstNamespaces) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  namespace NS {
+struct X;
+struct Y {
+  static const int I = 3;
+};
+  }
+  namespace NS {
+struct X {  // <--- To be imported
+  void method(int i = Y::I) {}
+  int f;
+};
+  }
+  )",
+  Lang_CXX);
+  auto *FromFwd = FirstDeclMatcher().match(
+  FromTU, cxxRecordDecl(hasName("X"), unless(isImplicit(;
+  auto *FromDef = LastDeclMatcher().match(
+  FromTU,
+  cxxRecordDecl(hasName("X"), isDefinition(), unless(isImplicit(;
+  ASSERT_NE(FromFwd, FromDef);
+  ASSERT_FALSE(FromFwd->isThisDeclarationADefinition());
+  ASSERT_TRUE(FromDef->isThisDeclarationADefinition());
+  ASSERT_EQ(FromFwd->getCanonicalDecl(), FromDef->getCanonicalDecl());
+
+  auto *ToDef = cast_or_null(Import(FromDef, Lang_CXX));
+  auto *ToFwd = cast_or_null(Import(FromFwd, Lang_CXX));
+  EXPECT_NE(ToFwd, ToDef);
+  EXPECT_FALSE(ToFwd->isThisDeclarationADefinition());
+  EXPECT_TRUE(ToDef->isThisDeclarationADefinition());
+  EXPECT_EQ(ToFwd->getCanonicalDecl(), ToDef->getCanonicalDecl());
+  auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  // We expect no (ODR) warning during the import.
+  EXPECT_EQ(0u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
 DefaultTestValuesForRunOptions, );
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r353687 - [clangd] Make system header mappings available for PreambleParsedCallback

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Mon Feb 11 02:31:13 2019
New Revision: 353687

URL: http://llvm.org/viewvc/llvm-project?rev=353687&view=rev
Log:
[clangd] Make system header mappings available for PreambleParsedCallback

Summary:
SystemHeaderMappings were added only after takeIncludes call, which
resulted in getting mapping on main file ast updates but not on preamble ast
updates.
Fixes https://github.com/clangd/clangd/issues/8

Reviewers: hokein

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=353687&r1=353686&r2=353687&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Mon Feb 11 02:31:13 2019
@@ -96,14 +96,13 @@ private:
 class CppFilePreambleCallbacks : public PreambleCallbacks {
 public:
   CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
-  : File(File), ParsedCallback(ParsedCallback) {}
+  : File(File), ParsedCallback(ParsedCallback) {
+addSystemHeadersMapping(&CanonIncludes);
+  }
 
   IncludeStructure takeIncludes() { return std::move(Includes); }
 
-  CanonicalIncludes takeCanonicalIncludes() {
-addSystemHeadersMapping(&CanonIncludes);
-return std::move(CanonIncludes);
-  }
+  CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); 
}
 
   void AfterExecute(CompilerInstance &CI) override {
 if (!ParsedCallback)

Modified: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp?rev=353687&r1=353686&r2=353687&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp Mon Feb 11 
02:31:13 2019
@@ -212,6 +212,39 @@ TEST(FileIndexTest, IncludeCollected) {
   "");
 }
 
+TEST(FileIndexTest, HasSystemHeaderMappingsInPreamble) {
+  FileIndex Index;
+  const std::string Header = R"cpp(
+class Foo {};
+  )cpp";
+  auto MainFile = testPath("foo.cpp");
+  auto HeaderFile = testPath("bits/alloc_traits.h");
+  std::vector Cmd = {"clang", "-xc++", MainFile.c_str(),
+   "-include", HeaderFile.c_str()};
+  // Preparse ParseInputs.
+  ParseInputs PI;
+  PI.CompileCommand.Directory = testRoot();
+  PI.CompileCommand.Filename = MainFile;
+  PI.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+  PI.Contents = "";
+  PI.FS = buildTestFS({{MainFile, ""}, {HeaderFile, Header}});
+
+  // Prepare preamble.
+  auto CI = buildCompilerInvocation(PI);
+  auto PreambleData = buildPreamble(
+  MainFile, *buildCompilerInvocation(PI), /*OldPreamble=*/nullptr,
+  tooling::CompileCommand(), PI, 
std::make_shared(),
+  /*StoreInMemory=*/true,
+  [&](ASTContext &Ctx, std::shared_ptr PP,
+  const CanonicalIncludes &Includes) {
+Index.updatePreamble(MainFile, Ctx, PP, Includes);
+  });
+  auto Symbols = runFuzzyFind(Index, "");
+  EXPECT_THAT(Symbols, ElementsAre(_));
+  EXPECT_THAT(Symbols.begin()->IncludeHeaders.front().IncludeHeader,
+  "");
+}
+
 TEST(FileIndexTest, TemplateParamsInLabel) {
   auto Source = R"cpp(
 template 


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

[PATCH] D58029: [clangd] Make system header mappings available for PreambleParsedCallback

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353687: [clangd] Make system header mappings available for 
PreambleParsedCallback (authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58029

Files:
  clang-tools-extra/trunk/clangd/ClangdUnit.cpp
  clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp


Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp
@@ -96,14 +96,13 @@
 class CppFilePreambleCallbacks : public PreambleCallbacks {
 public:
   CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
-  : File(File), ParsedCallback(ParsedCallback) {}
+  : File(File), ParsedCallback(ParsedCallback) {
+addSystemHeadersMapping(&CanonIncludes);
+  }
 
   IncludeStructure takeIncludes() { return std::move(Includes); }
 
-  CanonicalIncludes takeCanonicalIncludes() {
-addSystemHeadersMapping(&CanonIncludes);
-return std::move(CanonIncludes);
-  }
+  CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); 
}
 
   void AfterExecute(CompilerInstance &CI) override {
 if (!ParsedCallback)
Index: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
@@ -212,6 +212,39 @@
   "");
 }
 
+TEST(FileIndexTest, HasSystemHeaderMappingsInPreamble) {
+  FileIndex Index;
+  const std::string Header = R"cpp(
+class Foo {};
+  )cpp";
+  auto MainFile = testPath("foo.cpp");
+  auto HeaderFile = testPath("bits/alloc_traits.h");
+  std::vector Cmd = {"clang", "-xc++", MainFile.c_str(),
+   "-include", HeaderFile.c_str()};
+  // Preparse ParseInputs.
+  ParseInputs PI;
+  PI.CompileCommand.Directory = testRoot();
+  PI.CompileCommand.Filename = MainFile;
+  PI.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+  PI.Contents = "";
+  PI.FS = buildTestFS({{MainFile, ""}, {HeaderFile, Header}});
+
+  // Prepare preamble.
+  auto CI = buildCompilerInvocation(PI);
+  auto PreambleData = buildPreamble(
+  MainFile, *buildCompilerInvocation(PI), /*OldPreamble=*/nullptr,
+  tooling::CompileCommand(), PI, 
std::make_shared(),
+  /*StoreInMemory=*/true,
+  [&](ASTContext &Ctx, std::shared_ptr PP,
+  const CanonicalIncludes &Includes) {
+Index.updatePreamble(MainFile, Ctx, PP, Includes);
+  });
+  auto Symbols = runFuzzyFind(Index, "");
+  EXPECT_THAT(Symbols, ElementsAre(_));
+  EXPECT_THAT(Symbols.begin()->IncludeHeaders.front().IncludeHeader,
+  "");
+}
+
 TEST(FileIndexTest, TemplateParamsInLabel) {
   auto Source = R"cpp(
 template 


Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp
@@ -96,14 +96,13 @@
 class CppFilePreambleCallbacks : public PreambleCallbacks {
 public:
   CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
-  : File(File), ParsedCallback(ParsedCallback) {}
+  : File(File), ParsedCallback(ParsedCallback) {
+addSystemHeadersMapping(&CanonIncludes);
+  }
 
   IncludeStructure takeIncludes() { return std::move(Includes); }
 
-  CanonicalIncludes takeCanonicalIncludes() {
-addSystemHeadersMapping(&CanonIncludes);
-return std::move(CanonIncludes);
-  }
+  CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); }
 
   void AfterExecute(CompilerInstance &CI) override {
 if (!ParsedCallback)
Index: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
@@ -212,6 +212,39 @@
   "");
 }
 
+TEST(FileIndexTest, HasSystemHeaderMappingsInPreamble) {
+  FileIndex Index;
+  const std::string Header = R"cpp(
+class Foo {};
+  )cpp";
+  auto MainFile = testPath("foo.cpp");
+  auto HeaderFile = testPath("bits/alloc_traits.h");
+  std::vector Cmd = {"clang", "-xc++", MainFile.c_str(),
+   "-include", HeaderFile.c_str()};
+  // Preparse ParseInputs.
+  ParseInputs PI;
+  PI.CompileCommand.Directory = testRoot();
+  PI.CompileCommand.Filename = MainFile;
+  PI.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+  PI.Contents = "";
+  PI.FS = buildTestFS({{MainFile, ""}, {HeaderFile, Header}});
+
+  // Prepare pre

[PATCH] D57579: [analyzer][WIP] Enable subcheckers to possess checker options

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added a comment.

LGTM! But having a lit test that fails before and passes after would be great.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57579



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

[PATCH] D57739: [clangd] Format tweak's replacements.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D57739#1392453 , @sammccall wrote:

> In D57739#1390374 , @ilya-biryukov 
> wrote:
>
> > In D57739#1390321 , @sammccall 
> > wrote:
> >
> > > It's not about stability or whether the functionality is desired, but 
> > > layering.
> > >  Unit tests having narrow scope is a good thing - if we want system tests 
> > > that check clangdserver's behavior, they should test clangdserver.
> > >  Clients that don't go through clangdserver probably want formatting, but 
> > > an immediate cleanupAndFormat on each generated change isn't necessarily 
> > > the right way to do that.
> >
> >
> > My argument is that it's better to provide formatting by default in the 
> > public interface for **applying a single tweak**.
> >  I might be missing some use-cases, e.g. one that comes to mind is applying 
> > multiple tweaks in a row, in which case we'd want to format once and not 
> > for every tweak.
>
>
> If providing formatting was free, I'd be fine with this, but it leaks into 
> the public interface in two places: a) it requires the caller to plumb 
> through a formatting style, and b) it is another source of errors.
>
> For this particular interface it's more important that it's conceptually 
> clear and easy to implement than it is that it's easy to call - this is an 
> extension point.
>
> > My concerns are code duplication and ease of use for the clients. Having 
> > both `apply` and `applyAndFormat` (the latter being a non-virtual or a 
> > free-standing utility function) in the public interface of the `Tweak` 
> > would totally do it for me.
>
> I'd be happy with `applyAndFormat` as a free function - my main concern is 
> that formatting not be part of the scope of the class.


Given that clangdServer is the only client of `Tweaks` (if we don't do format 
in tests), I think it is fine to move out `applyAndFormat` functionality from 
`Tweaks`. I somehow agree that `applyAndFormat` is a standalone function.

>> However, I also think tests should format by default, not sure we agree on 
>> this.
>>  WDYT?
> 
> I'd rather they didn't format, but I don't think it will matter much and I'm 
> happy either way.
> 
> (Where it does matter, I'd rather have the differences between input/output 
> be a result of the tweak replacements, not of different formatting triggered 
> by a different token sequence.
>  I don't think there's much point in testing clang-format here, though we 
> should have one test that verifies we're calling it at all.)

Personally, I prefer not doing format in tests -- it would make writing 
testcases easier, it is annoying to spend time on figuring out **format-only** 
differences between actual code and expected code.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739



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

[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Just a question.. If clang tidy is running with -fix in parallel, what stops 
each clang-tidy invocation altering a common header at the same time?


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

https://reviews.llvm.org/D57662



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

r353690 - [libclang] Add attribute support for 'convergent'.

[Sven van Haastregt via cfe-commits]

Author: svenvh
Date: Mon Feb 11 03:00:56 2019
New Revision: 353690

URL: http://llvm.org/viewvc/llvm-project?rev=353690&view=rev
Log:
[libclang] Add attribute support for 'convergent'.

This bumps CINDEX_VERSION_MINOR up (to 51).

Patch by Hsin-Hsiao Lin.

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

Modified:
cfe/trunk/bindings/python/clang/cindex.py
cfe/trunk/include/clang-c/Index.h
cfe/trunk/test/Index/attributes.c
cfe/trunk/tools/libclang/CIndex.cpp
cfe/trunk/tools/libclang/CXCursor.cpp

Modified: cfe/trunk/bindings/python/clang/cindex.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/clang/cindex.py?rev=353690&r1=353689&r2=353690&view=diff
==
--- cfe/trunk/bindings/python/clang/cindex.py (original)
+++ cfe/trunk/bindings/python/clang/cindex.py Mon Feb 11 03:00:56 2019
@@ -1342,6 +1342,7 @@ CursorKind.VISIBILITY_ATTR = CursorKind(
 
 CursorKind.DLLEXPORT_ATTR = CursorKind(418)
 CursorKind.DLLIMPORT_ATTR = CursorKind(419)
+CursorKind.CONVERGENT_ATTR = CursorKind(420)
 
 ###
 # Preprocessing

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=353690&r1=353689&r2=353690&view=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Mon Feb 11 03:00:56 2019
@@ -32,7 +32,7 @@
  * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
  */
 #define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 50
+#define CINDEX_VERSION_MINOR 51
 
 #define CINDEX_VERSION_ENCODE(major, minor) ( \
   ((major) * 1)   \
@@ -2586,7 +2586,8 @@ enum CXCursorKind {
   CXCursor_ObjCRuntimeVisible= 435,
   CXCursor_ObjCBoxable   = 436,
   CXCursor_FlagEnum  = 437,
-  CXCursor_LastAttr  = CXCursor_FlagEnum,
+  CXCursor_ConvergentAttr= 438,
+  CXCursor_LastAttr  = CXCursor_ConvergentAttr,
 
   /* Preprocessing */
   CXCursor_PreprocessingDirective= 500,

Modified: cfe/trunk/test/Index/attributes.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/attributes.c?rev=353690&r1=353689&r2=353690&view=diff
==
--- cfe/trunk/test/Index/attributes.c (original)
+++ cfe/trunk/test/Index/attributes.c Mon Feb 11 03:00:56 2019
@@ -12,6 +12,8 @@ enum __attribute((flag_enum)) FlagEnum {
   Foo
 };
 
+void convergent_fn() __attribute__((convergent));
+
 // CHECK: attributes.c:3:32: StructDecl=Test2:3:32 (Definition) Extent=[3:1 - 
5:2]
 // CHECK: attributes.c:3:23: attribute(packed)=packed Extent=[3:23 - 3:29]
 // CHECK: attributes.c:4:8: FieldDecl=a:4:8 (Definition) Extent=[4:3 - 4:9] 
[access=public]
@@ -24,3 +26,6 @@ enum __attribute((flag_enum)) FlagEnum {
 // CHECK: attributes.c:9:38: attribute(noduplicate)= Extent=[9:38 - 9:49]
 // CHECK: attributes.c:11:31: EnumDecl=FlagEnum:11:31 (Definition) 
Extent=[11:1 - 13:2]
 // CHECK: attributes.c:11:19: attribute(flag_enum)= Extent=[11:19 - 11:28]
+// CHECK: attributes.c:12:3: EnumConstantDecl=Foo:12:3 (Definition) 
Extent=[12:3 - 12:6]
+// CHECK: attributes.c:15:6: FunctionDecl=convergent_fn:15:6 Extent=[15:1 - 
15:49]
+// CHECK: attributes.c:15:37: attribute(convergent)= Extent=[15:37 - 15:47]

Modified: cfe/trunk/tools/libclang/CIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=353690&r1=353689&r2=353690&view=diff
==
--- cfe/trunk/tools/libclang/CIndex.cpp (original)
+++ cfe/trunk/tools/libclang/CIndex.cpp Mon Feb 11 03:00:56 2019
@@ -5474,7 +5474,9 @@ CXString clang_getCursorKindSpelling(enu
   case CXCursor_StaticAssert:
   return cxstring::createRef("StaticAssert");
   case CXCursor_FriendDecl:
-return cxstring::createRef("FriendDecl");
+  return cxstring::createRef("FriendDecl");
+  case CXCursor_ConvergentAttr:
+  return cxstring::createRef("attribute(convergent)");
   }
 
   llvm_unreachable("Unhandled CXCursorKind");

Modified: cfe/trunk/tools/libclang/CXCursor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXCursor.cpp?rev=353690&r1=353689&r2=353690&view=diff
==
--- cfe/trunk/tools/libclang/CXCursor.cpp (original)
+++ cfe/trunk/tools/libclang/CXCursor.cpp Mon Feb 11 03:00:56 2019
@@ -78,6 +78,7 @@ static CXCursorKind GetCursorKind(const
 case attr::ObjCRuntimeVisible: return CXCursor_ObjCRuntimeVisible;
 case attr::ObjCBoxable: return CXCursor_ObjCBoxable;
 case attr::FlagEnum: return CXCursor_FlagEnum;
+case attr::Convergent: return CXCursor_ConvergentAttr;
   }
 
   return CXCursor_UnexposedAttr

[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added a comment.

In D57662#1392509 , @MyDeveloperDay 
wrote:

> Just a question.. If clang tidy is running with -fix in parallel, what stops 
> each clang-tidy invocation altering a common header at the same time?


You are right. May be it worth disabling `-fix` for `j != 1`.


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

https://reviews.llvm.org/D57662



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

[PATCH] D57946: [libclang] Add attribute support for 'convergent'.

[Sven van Haastregt via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353690: [libclang] Add attribute support for 
'convergent'. (authored by svenvh, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D57946

Files:
  bindings/python/clang/cindex.py
  include/clang-c/Index.h
  test/Index/attributes.c
  tools/libclang/CIndex.cpp
  tools/libclang/CXCursor.cpp


Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -32,7 +32,7 @@
  * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
  */
 #define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 50
+#define CINDEX_VERSION_MINOR 51
 
 #define CINDEX_VERSION_ENCODE(major, minor) ( \
   ((major) * 1)   \
@@ -2586,7 +2586,8 @@
   CXCursor_ObjCRuntimeVisible= 435,
   CXCursor_ObjCBoxable   = 436,
   CXCursor_FlagEnum  = 437,
-  CXCursor_LastAttr  = CXCursor_FlagEnum,
+  CXCursor_ConvergentAttr= 438,
+  CXCursor_LastAttr  = CXCursor_ConvergentAttr,
 
   /* Preprocessing */
   CXCursor_PreprocessingDirective= 500,
Index: test/Index/attributes.c
===
--- test/Index/attributes.c
+++ test/Index/attributes.c
@@ -12,6 +12,8 @@
   Foo
 };
 
+void convergent_fn() __attribute__((convergent));
+
 // CHECK: attributes.c:3:32: StructDecl=Test2:3:32 (Definition) Extent=[3:1 - 
5:2]
 // CHECK: attributes.c:3:23: attribute(packed)=packed Extent=[3:23 - 3:29]
 // CHECK: attributes.c:4:8: FieldDecl=a:4:8 (Definition) Extent=[4:3 - 4:9] 
[access=public]
@@ -24,3 +26,6 @@
 // CHECK: attributes.c:9:38: attribute(noduplicate)= Extent=[9:38 - 9:49]
 // CHECK: attributes.c:11:31: EnumDecl=FlagEnum:11:31 (Definition) 
Extent=[11:1 - 13:2]
 // CHECK: attributes.c:11:19: attribute(flag_enum)= Extent=[11:19 - 11:28]
+// CHECK: attributes.c:12:3: EnumConstantDecl=Foo:12:3 (Definition) 
Extent=[12:3 - 12:6]
+// CHECK: attributes.c:15:6: FunctionDecl=convergent_fn:15:6 Extent=[15:1 - 
15:49]
+// CHECK: attributes.c:15:37: attribute(convergent)= Extent=[15:37 - 15:47]
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -5474,7 +5474,9 @@
   case CXCursor_StaticAssert:
   return cxstring::createRef("StaticAssert");
   case CXCursor_FriendDecl:
-return cxstring::createRef("FriendDecl");
+  return cxstring::createRef("FriendDecl");
+  case CXCursor_ConvergentAttr:
+  return cxstring::createRef("attribute(convergent)");
   }
 
   llvm_unreachable("Unhandled CXCursorKind");
Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -78,6 +78,7 @@
 case attr::ObjCRuntimeVisible: return CXCursor_ObjCRuntimeVisible;
 case attr::ObjCBoxable: return CXCursor_ObjCBoxable;
 case attr::FlagEnum: return CXCursor_FlagEnum;
+case attr::Convergent: return CXCursor_ConvergentAttr;
   }
 
   return CXCursor_UnexposedAttr;
Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -1342,6 +1342,7 @@
 
 CursorKind.DLLEXPORT_ATTR = CursorKind(418)
 CursorKind.DLLIMPORT_ATTR = CursorKind(419)
+CursorKind.CONVERGENT_ATTR = CursorKind(420)
 
 ###
 # Preprocessing


Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -32,7 +32,7 @@
  * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
  */
 #define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 50
+#define CINDEX_VERSION_MINOR 51
 
 #define CINDEX_VERSION_ENCODE(major, minor) ( \
   ((major) * 1)   \
@@ -2586,7 +2586,8 @@
   CXCursor_ObjCRuntimeVisible= 435,
   CXCursor_ObjCBoxable   = 436,
   CXCursor_FlagEnum  = 437,
-  CXCursor_LastAttr  = CXCursor_FlagEnum,
+  CXCursor_ConvergentAttr= 438,
+  CXCursor_LastAttr  = CXCursor_ConvergentAttr,
 
   /* Preprocessing */
   CXCursor_PreprocessingDirective= 500,
Index: test/Index/attributes.c
===
--- test/Index/attributes.c
+++ test/Index/attributes.c
@@ -12,6 +12,8 @@
   Foo
 };
 
+void convergent_fn() __attribute__((convergent));
+
 // CHECK: attributes.c:3:32: StructDecl=Test2:3:32 (Definition) Extent=[3:1 - 5:2]
 // CHECK: attributes.c:3:23: attribute(packed)=packed Extent=[3:23 - 3:29]
 // CHECK: at

[PATCH] D55562: Atomics: support min/max orthogonally

[Al Grant via Phabricator via cfe-commits]

algrant added a comment.
Herald added a project: clang.

Some targets (e.g. AArch64) support 8-bit, 16-bit and 64-bit atomics. max/min 
in Clang 7.0 only supported 32-bit max/min, even though the other atomics 
supported multiple widths. Is the intention here to support all (four) offered 
widths, signed and unsigned, and if so, could this be reflected in the tests, 
which are still only testing 32-bit?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55562



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

[PATCH] D57896: Variable names rule

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> Sounds good to me. I see that you've made D57966 
>  a child of this issue, but we could swap 
> the dependency around so that once your patch is applied I can update this 
> patch to use `camelBackOrCase`.

I'm OK if we want to do that, but its very much a circular dependency, I don't 
want to land D57966: [clang-tidy] add camelBackOrCase casing style to 
readability-identifier-naming to support change to variable naming policy (if 
adopted) , unless this whole variableName 
suggestion is accepted (which I think is a good idea).. I think your suggestion 
warrants being the driver, lets do the clang-tidy change and subsequent 
.clang-tidy changes on another revision post acceptance of both.

FWIW, I agree with the comments that the function name should be differentiated 
from the variable by the use of a verbs, I've spent too much time in my career 
grepping for the work `type` in code

  Type type = type();

I think

  Type type = getType();

or

  Type objectType = getType();

adds some increased levels of clarity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

[Philip Pfaffe via Phabricator via cfe-commits]

philip.pfaffe added a comment.

I'd prefer not adding this kind of state to PassBuilder. `SplitColdCode` is 
soemthing that refers to the construction of //one// particular pipeline, not 
to pipeline-building in general. It should be an argument passed down through 
the build*Pipeline calls.


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

https://reviews.llvm.org/D57265



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

[PATCH] D57855: [analyzer] Reimplement checker options

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

We have `examples/analyzer-plugin`. I would prefer to add an example option to 
the example plugin so people do see how to do this when they are registering a 
checker from a plugin.




Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:48
+CmdLineOption,

Do we need `the`? Also, I think the checker also do not know the allocating 
functions. I think it might be better to describe the opposite. Optimistic 
means that the checker assumes all the allocating, deallocating functions are 
annotated.



Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:384
+  CheckerOptions<[
+CmdLineOptionhttps://reviews.llvm.org/D57855/new/

https://reviews.llvm.org/D57855



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

[PATCH] D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable

[Gábor Horváth via Phabricator via cfe-commits]

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

LG!


Repository:
  rC Clang

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

https://reviews.llvm.org/D57922



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

[PATCH] D57890: [analyzer] Fix in self assignment checker

[Gábor Horváth via Phabricator via cfe-commits]

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

LG!

Just wanted to make sure I get it right. You did not add a test since it is 
only reproducible with an internal (non-upstreamed) checker. Since the change 
is trivial, I think it is ok to commit this without a test.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57890



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

[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> You are right. May be it worth disabling `-fix` for `j != 1`.

I only say this because I think I might have seen it happen when I was running 
`run-clang-tidy.py` over a large code base with a fairly aggressive 
check/fixit, but frankly I was too new to LLVM to know it wasn't something I 
might have done wrong.

I saw my '[[nodiscard]]' checker splicing multiple `[[nodiscard]]`s onto the 
same line, I suspect there is a time of check/time of use issue for the 
locations, sometimes the positions where wrong by the time the Fixit came 
around. I guess because i'd used that script without specifiy a `-j1` it just 
used the default which is `-j0` which I guess goes as parallel as it can.

Its only when you raised this review for the diff check that I wondered if that 
could be the cause.


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

https://reviews.llvm.org/D57662



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

[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added a comment.

> I only say this because I think I might have seen it happen when I was 
> running `run-clang-tidy.py` over a large code base with a fairly aggressive 
> check/fixit, but frankly I was too new to LLVM to know it wasn't something I 
> might have done wrong.

A bit strange as `run-clang-tidy.py` explicitly collects fixes in separate 
files and after all merges them into a single fix in a thread/process-safe 
manner. Not like my current patch.


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

https://reviews.llvm.org/D57662



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

[PATCH] D58037: [clangd] Prefer location from codegen files when merging symbols.

[Eric Liu via Phabricator via cfe-commits]

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

For example, if an index symbol has location in a .proto file and an AST symbol
has location in a generated .proto.h file, then we prefer location in .proto
which is more meaningful to users.

Also use `mergeSymbols` to get the preferred location between AST location and 
index location in go-to-def.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D58037

Files:
  clangd/XRefs.cpp
  clangd/index/Merge.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -180,6 +180,26 @@
   ElementsAre(Sym("Forward", SymbolHeader.range("forward"), Test.range(;
 }
 
+TEST(LocateSymbol, WithIndexPreferredLocation) {
+  Annotations SymbolHeader(R"cpp(
+class $[[Proto]] {};
+  )cpp");
+  TestTU TU;
+  TU.HeaderCode = SymbolHeader.code();
+  TU.HeaderFilename = "x.proto"; // Prefer locations in codegen files.
+  auto Index = TU.index();
+
+  Annotations Test(R"cpp(// only declaration in AST.
+// Shift to make range different.
+class [[Proto]];
+P^roto* create();
+  )cpp");
+
+  auto AST = TestTU::withCode(Test.code()).build();
+  auto Locs = clangd::locateSymbolAt(AST, Test.point(), Index.get());
+  EXPECT_THAT(Locs, ElementsAre(Sym("Proto", SymbolHeader.range(;
+}
+
 TEST(LocateSymbol, All) {
   // Ranges in tests:
   //   $decl is the declaration location (if absent, no symbol is located)
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -253,6 +253,22 @@
   EXPECT_EQ(M.Name, "right");
 }
 
+TEST(MergeTest, PreferSymbolLocationInCodegenFile) {
+  Symbol L, R;
+
+  L.ID = R.ID = SymbolID("hello");
+  L.CanonicalDeclaration.FileURI = "file:/x.proto.h";
+  R.CanonicalDeclaration.FileURI = "file:/x.proto";
+
+  Symbol M = mergeSymbol(L, R);
+  EXPECT_EQ(StringRef(M.CanonicalDeclaration.FileURI), "file:/x.proto");
+
+  // Prefer L if both have codegen suffix.
+  L.CanonicalDeclaration.FileURI = "file:/y.proto";
+  M = mergeSymbol(L, R);
+  EXPECT_EQ(StringRef(M.CanonicalDeclaration.FileURI), "file:/y.proto");
+}
+
 TEST(MergeIndexTest, Refs) {
   FileIndex Dyn;
   FileIndex StaticIndex;
Index: clangd/index/Merge.cpp
===
--- clangd/index/Merge.cpp
+++ clangd/index/Merge.cpp
@@ -9,9 +9,13 @@
 #include "Merge.h"
 #include "Logger.h"
 #include "Trace.h"
+#include "index/Index.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -114,6 +118,24 @@
   });
 }
 
+// Returns the preferred location e.g. by file path. \p L is returned if
+// neither is preferred.
+static SymbolLocation getPreferredLocation(const SymbolLocation &L,
+   const SymbolLocation &R) {
+  if (!L)
+return R;
+  if (!R)
+return L;
+  auto HasCodeGenSuffix = [](const SymbolLocation &Loc) {
+constexpr static const char *CodegenSuffixes[] = {".proto"};
+return std::any_of(std::begin(CodegenSuffixes), std::end(CodegenSuffixes),
+   [&](llvm::StringRef Suffix) {
+ return llvm::StringRef(Loc.FileURI).endswith(Suffix);
+   });
+  };
+  return (HasCodeGenSuffix(R) && !HasCodeGenSuffix(L)) ? R : L;
+}
+
 Symbol mergeSymbol(const Symbol &L, const Symbol &R) {
   assert(L.ID == R.ID);
   // We prefer information from TUs that saw the definition.
@@ -128,12 +150,9 @@
   Symbol S = PreferR ? R : L;// The target symbol we're merging into.
   const Symbol &O = PreferR ? L : R; // The "other" less-preferred symbol.
 
-  // For each optional field, fill it from O if missing in S.
-  // (It might be missing in O too, but that's a no-op).
-  if (!S.Definition)
-S.Definition = O.Definition;
-  if (!S.CanonicalDeclaration)
-S.CanonicalDeclaration = O.CanonicalDeclaration;
+  S.CanonicalDeclaration =
+  getPreferredLocation(S.CanonicalDeclaration, O.CanonicalDeclaration);
+  S.Definition = getPreferredLocation(S.Definition, O.Definition);
   S.References += O.References;
   if (S.Signature == "")
 S.Signature = O.Signature;
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -10,6 +10,8 @@
 #include "Logger.h"
 #include "SourceCode.h"
 #include "URI.h"
+#include "index/Index.h"
+#include "index/Merge.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Recursive

[clang-tools-extra] r353694 - [clangd] Fix broken windows build bots.

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Mon Feb 11 05:01:47 2019
New Revision: 353694

URL: http://llvm.org/viewvc/llvm-project?rev=353694&view=rev
Log:
[clangd] Fix broken windows build bots.

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

Modified: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp?rev=353694&r1=353693&r2=353694&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp Mon Feb 11 
05:01:47 2019
@@ -218,7 +218,7 @@ TEST(FileIndexTest, HasSystemHeaderMappi
 class Foo {};
   )cpp";
   auto MainFile = testPath("foo.cpp");
-  auto HeaderFile = testPath("bits/alloc_traits.h");
+  auto HeaderFile = testPath("algorithm");
   std::vector Cmd = {"clang", "-xc++", MainFile.c_str(),
"-include", HeaderFile.c_str()};
   // Preparse ParseInputs.
@@ -242,7 +242,7 @@ TEST(FileIndexTest, HasSystemHeaderMappi
   auto Symbols = runFuzzyFind(Index, "");
   EXPECT_THAT(Symbols, ElementsAre(_));
   EXPECT_THAT(Symbols.begin()->IncludeHeaders.front().IncludeHeader,
-  "");
+  "");
 }
 
 TEST(FileIndexTest, TemplateParamsInLabel) {


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

[PATCH] D57949: [clang][Index] Add a knob to index function parameters in declarations

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353695: [clang][Index] Add a knob to index function 
parameters in declarations (authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57949

Files:
  cfe/trunk/include/clang/Index/IndexingAction.h
  cfe/trunk/lib/Index/IndexDecl.cpp
  cfe/trunk/lib/Index/IndexingContext.cpp
  cfe/trunk/lib/Index/IndexingContext.h
  cfe/trunk/unittests/Index/IndexTests.cpp


Index: cfe/trunk/include/clang/Index/IndexingAction.h
===
--- cfe/trunk/include/clang/Index/IndexingAction.h
+++ cfe/trunk/include/clang/Index/IndexingAction.h
@@ -44,6 +44,8 @@
   // callback is not available (e.g. after parsing has finished). Note that
   // macro references are not available in Proprocessor.
   bool IndexMacrosInPreprocessor = false;
+  // Has no effect if IndexFunctionLocals are false.
+  bool IndexParametersInDeclarations = false;
 };
 
 /// Creates a frontend action that indexes all symbols (macros and AST decls).
Index: cfe/trunk/lib/Index/IndexingContext.h
===
--- cfe/trunk/lib/Index/IndexingContext.h
+++ cfe/trunk/lib/Index/IndexingContext.h
@@ -61,6 +61,8 @@
 
   bool shouldIndexImplicitInstantiation() const;
 
+  bool shouldIndexParametersInDeclarations() const;
+
   static bool isTemplateImplicitInstantiation(const Decl *D);
 
   bool handleDecl(const Decl *D, SymbolRoleSet Roles = SymbolRoleSet(),
Index: cfe/trunk/lib/Index/IndexDecl.cpp
===
--- cfe/trunk/lib/Index/IndexDecl.cpp
+++ cfe/trunk/lib/Index/IndexDecl.cpp
@@ -88,12 +88,11 @@
  /*isBase=*/false, isIBType);
 IndexCtx.indexNestedNameSpecifierLoc(D->getQualifierLoc(), Parent);
 if (IndexCtx.shouldIndexFunctionLocalSymbols()) {
-  // Only index parameters in definitions, parameters in declarations are
-  // not useful.
   if (const ParmVarDecl *Parm = dyn_cast(D)) {
 auto *DC = Parm->getDeclContext();
 if (auto *FD = dyn_cast(DC)) {
-  if (FD->isThisDeclarationADefinition())
+  if (IndexCtx.shouldIndexParametersInDeclarations() ||
+  FD->isThisDeclarationADefinition())
 IndexCtx.handleDecl(Parm);
 } else if (auto *MD = dyn_cast(DC)) {
   if (MD->isThisDeclarationADefinition())
@@ -102,7 +101,8 @@
   IndexCtx.handleDecl(Parm);
 }
   } else if (const FunctionDecl *FD = dyn_cast(D)) {
-if (FD->isThisDeclarationADefinition()) {
+if (IndexCtx.shouldIndexParametersInDeclarations() ||
+FD->isThisDeclarationADefinition()) {
   for (auto PI : FD->parameters()) {
 IndexCtx.handleDecl(PI);
   }
Index: cfe/trunk/lib/Index/IndexingContext.cpp
===
--- cfe/trunk/lib/Index/IndexingContext.cpp
+++ cfe/trunk/lib/Index/IndexingContext.cpp
@@ -40,6 +40,10 @@
   return IndexOpts.IndexImplicitInstantiation;
 }
 
+bool IndexingContext::shouldIndexParametersInDeclarations() const {
+  return IndexOpts.IndexParametersInDeclarations;
+}
+
 bool IndexingContext::handleDecl(const Decl *D,
  SymbolRoleSet Roles,
  ArrayRef Relations) {
Index: cfe/trunk/unittests/Index/IndexTests.cpp
===
--- cfe/trunk/unittests/Index/IndexTests.cpp
+++ cfe/trunk/unittests/Index/IndexTests.cpp
@@ -119,6 +119,21 @@
   EXPECT_THAT(Index->Symbols, UnorderedElementsAre());
 }
 
+TEST(IndexTest, IndexParametersInDecls) {
+  std::string Code = "void foo(int bar);";
+  auto Index = std::make_shared();
+  IndexingOptions Opts;
+  Opts.IndexFunctionLocals = true;
+  Opts.IndexParametersInDeclarations = true;
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, Contains(QName("bar")));
+
+  Opts.IndexParametersInDeclarations = false;
+  Index->Symbols.clear();
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, Not(Contains(QName("bar";
+}
+
 } // namespace
 } // namespace index
 } // namespace clang


Index: cfe/trunk/include/clang/Index/IndexingAction.h
===
--- cfe/trunk/include/clang/Index/IndexingAction.h
+++ cfe/trunk/include/clang/Index/IndexingAction.h
@@ -44,6 +44,8 @@
   // callback is not available (e.g. after parsing has finished). Note that
   // macro references are not available in Proprocessor.
   bool IndexMacrosInPreprocessor = false;
+  // Has no effect if IndexFunctionLocals are false.
+  bool IndexParametersInDeclarations = 

r353695 - [clang][Index] Add a knob to index function parameters in declarations

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Mon Feb 11 05:02:21 2019
New Revision: 353695

URL: http://llvm.org/viewvc/llvm-project?rev=353695&view=rev
Log:
[clang][Index] Add a knob to index function parameters in declarations

Summary:
Parameters in declarations are useful for clangd, so that we can
provide symbol information for them as well. It also helps clangd to be
consistent whether a function's definition is accessible or not.

Reviewers: hokein, akyrtzi

Subscribers: ilya-biryukov, ioeric, arphaman, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Index/IndexingAction.h
cfe/trunk/lib/Index/IndexDecl.cpp
cfe/trunk/lib/Index/IndexingContext.cpp
cfe/trunk/lib/Index/IndexingContext.h
cfe/trunk/unittests/Index/IndexTests.cpp

Modified: cfe/trunk/include/clang/Index/IndexingAction.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Index/IndexingAction.h?rev=353695&r1=353694&r2=353695&view=diff
==
--- cfe/trunk/include/clang/Index/IndexingAction.h (original)
+++ cfe/trunk/include/clang/Index/IndexingAction.h Mon Feb 11 05:02:21 2019
@@ -44,6 +44,8 @@ struct IndexingOptions {
   // callback is not available (e.g. after parsing has finished). Note that
   // macro references are not available in Proprocessor.
   bool IndexMacrosInPreprocessor = false;
+  // Has no effect if IndexFunctionLocals are false.
+  bool IndexParametersInDeclarations = false;
 };
 
 /// Creates a frontend action that indexes all symbols (macros and AST decls).

Modified: cfe/trunk/lib/Index/IndexDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=353695&r1=353694&r2=353695&view=diff
==
--- cfe/trunk/lib/Index/IndexDecl.cpp (original)
+++ cfe/trunk/lib/Index/IndexDecl.cpp Mon Feb 11 05:02:21 2019
@@ -88,12 +88,11 @@ public:
  /*isBase=*/false, isIBType);
 IndexCtx.indexNestedNameSpecifierLoc(D->getQualifierLoc(), Parent);
 if (IndexCtx.shouldIndexFunctionLocalSymbols()) {
-  // Only index parameters in definitions, parameters in declarations are
-  // not useful.
   if (const ParmVarDecl *Parm = dyn_cast(D)) {
 auto *DC = Parm->getDeclContext();
 if (auto *FD = dyn_cast(DC)) {
-  if (FD->isThisDeclarationADefinition())
+  if (IndexCtx.shouldIndexParametersInDeclarations() ||
+  FD->isThisDeclarationADefinition())
 IndexCtx.handleDecl(Parm);
 } else if (auto *MD = dyn_cast(DC)) {
   if (MD->isThisDeclarationADefinition())
@@ -102,7 +101,8 @@ public:
   IndexCtx.handleDecl(Parm);
 }
   } else if (const FunctionDecl *FD = dyn_cast(D)) {
-if (FD->isThisDeclarationADefinition()) {
+if (IndexCtx.shouldIndexParametersInDeclarations() ||
+FD->isThisDeclarationADefinition()) {
   for (auto PI : FD->parameters()) {
 IndexCtx.handleDecl(PI);
   }

Modified: cfe/trunk/lib/Index/IndexingContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexingContext.cpp?rev=353695&r1=353694&r2=353695&view=diff
==
--- cfe/trunk/lib/Index/IndexingContext.cpp (original)
+++ cfe/trunk/lib/Index/IndexingContext.cpp Mon Feb 11 05:02:21 2019
@@ -40,6 +40,10 @@ bool IndexingContext::shouldIndexImplici
   return IndexOpts.IndexImplicitInstantiation;
 }
 
+bool IndexingContext::shouldIndexParametersInDeclarations() const {
+  return IndexOpts.IndexParametersInDeclarations;
+}
+
 bool IndexingContext::handleDecl(const Decl *D,
  SymbolRoleSet Roles,
  ArrayRef Relations) {

Modified: cfe/trunk/lib/Index/IndexingContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexingContext.h?rev=353695&r1=353694&r2=353695&view=diff
==
--- cfe/trunk/lib/Index/IndexingContext.h (original)
+++ cfe/trunk/lib/Index/IndexingContext.h Mon Feb 11 05:02:21 2019
@@ -61,6 +61,8 @@ public:
 
   bool shouldIndexImplicitInstantiation() const;
 
+  bool shouldIndexParametersInDeclarations() const;
+
   static bool isTemplateImplicitInstantiation(const Decl *D);
 
   bool handleDecl(const Decl *D, SymbolRoleSet Roles = SymbolRoleSet(),

Modified: cfe/trunk/unittests/Index/IndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Index/IndexTests.cpp?rev=353695&r1=353694&r2=353695&view=diff
==
--- cfe/trunk/unittests/Index/IndexTests.cpp (original)
+++ cfe/trunk/unittests/Index/IndexTests.cpp Mon Feb 11 05:02:21 2019
@@ -119,6 +119,21 @@ TEST(IndexTest, IndexPreprocessorMacros)
   EXPEC

[PATCH] D57950: [clangd] Index parameters in function decls

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE353696: [clangd] Index parameters in function decls 
(authored by kadircet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57950?vs=185967&id=186235#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57950

Files:
  clangd/XRefs.cpp
  unittests/clangd/SymbolInfoTests.cpp
  unittests/clangd/XRefsTests.cpp


Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -302,6 +302,12 @@
 )cpp",
   {CreateExpectedSymbolDetails("bar", "foo::", "c:@E@foo@bar")}},
   {
+  R"cpp( // Parameters in declarations
+  void foo(int ba^r);
+)cpp",
+  {CreateExpectedSymbolDetails("bar", "foo",
+   "c:TestTU.cpp@50@F@foo#I#@bar")}},
+  {
   R"cpp( // Type inferrence with auto keyword
   struct foo {};
   foo getfoo() { return foo{}; }
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -86,6 +86,10 @@
   auto *X = &[[foo]];
 }
   )cpp",
+
+  R"cpp(// Function parameter in decl
+void foo(int [[^bar]]);
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -215,6 +215,7 @@
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
+  IndexOpts.IndexParametersInDeclarations = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts);
 
@@ -400,6 +401,7 @@
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
+  IndexOpts.IndexParametersInDeclarations = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), RefFinder, IndexOpts);
   return std::move(RefFinder).take();


Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -302,6 +302,12 @@
 )cpp",
   {CreateExpectedSymbolDetails("bar", "foo::", "c:@E@foo@bar")}},
   {
+  R"cpp( // Parameters in declarations
+  void foo(int ba^r);
+)cpp",
+  {CreateExpectedSymbolDetails("bar", "foo",
+   "c:TestTU.cpp@50@F@foo#I#@bar")}},
+  {
   R"cpp( // Type inferrence with auto keyword
   struct foo {};
   foo getfoo() { return foo{}; }
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -86,6 +86,10 @@
   auto *X = &[[foo]];
 }
   )cpp",
+
+  R"cpp(// Function parameter in decl
+void foo(int [[^bar]]);
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -215,6 +215,7 @@
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
+  IndexOpts.IndexParametersInDeclarations = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts);
 
@@ -400,6 +401,7 @@
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
+  IndexOpts.IndexParametersInDeclarations = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), RefFinder, IndexOpts);
   return std::move(RefFinder).take();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r353696 - [clangd] Index parameters in function decls

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Mon Feb 11 05:03:08 2019
New Revision: 353696

URL: http://llvm.org/viewvc/llvm-project?rev=353696&view=rev
Log:
[clangd] Index parameters in function decls

Reviewers: hokein

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolInfoTests.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=353696&r1=353695&r2=353696&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Mon Feb 11 05:03:08 2019
@@ -215,6 +215,7 @@ IdentifiedSymbol getSymbolAtPosition(Par
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
+  IndexOpts.IndexParametersInDeclarations = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts);
 
@@ -400,6 +401,7 @@ findRefs(const std::vector
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
+  IndexOpts.IndexParametersInDeclarations = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), RefFinder, IndexOpts);
   return std::move(RefFinder).take();

Modified: clang-tools-extra/trunk/unittests/clangd/SymbolInfoTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolInfoTests.cpp?rev=353696&r1=353695&r2=353696&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/SymbolInfoTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolInfoTests.cpp Mon Feb 11 
05:03:08 2019
@@ -302,6 +302,12 @@ TEST(SymbolInfoTests, All) {
 )cpp",
   {CreateExpectedSymbolDetails("bar", "foo::", "c:@E@foo@bar")}},
   {
+  R"cpp( // Parameters in declarations
+  void foo(int ba^r);
+)cpp",
+  {CreateExpectedSymbolDetails("bar", "foo",
+   "c:TestTU.cpp@50@F@foo#I#@bar")}},
+  {
   R"cpp( // Type inferrence with auto keyword
   struct foo {};
   foo getfoo() { return foo{}; }

Modified: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp?rev=353696&r1=353695&r2=353696&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Mon Feb 11 05:03:08 
2019
@@ -86,6 +86,10 @@ TEST(HighlightsTest, All) {
   auto *X = &[[foo]];
 }
   )cpp",
+
+  R"cpp(// Function parameter in decl
+void foo(int [[^bar]]);
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);


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

[PATCH] D17444: PR26672: [MSVC] Clang does not recognize "static_assert" keyword in C mode

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

I just ran into this and I'm a bit confused about the discussion here. This 
snippet (in a .c file)

  #include 
  
  static_assert(4 == 4 , "");

builds in all compilers except clang-cl. How does not supporting this make 
sense?

Instead of this patch we could have an assert.h wrapper in lib/Headers that 
defines static_assert to _Static_assert in ms mode for C files, and I suppose 
that's a cleaner fix. But I hope it's not controversial that we should try and 
support standard C programs?


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

https://reviews.llvm.org/D17444



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

[PATCH] D57855: [analyzer] Reimplement checker options

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

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

In D57855#1392560 , @xazax.hun wrote:

> We have `examples/analyzer-plugin`. I would prefer to add an example option 
> to the example plugin so people do see how to do this when they are 
> registering a checker from a plugin.


Not only that, but one for dependencies would be great too, thanks!




Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:48
+CmdLineOption Did you want to add this option to nullability? I believe it belongs to 
> Malloc Checker.
Yup, shouldn't be here. I vaguely remembered (incorrectly) that a test file 
contained a `nullability:Optimistic` option, but I can't find (=grep) any 
evidence of that. Nice catch!


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

https://reviews.llvm.org/D57855



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

[PATCH] D57906: [CTU] Do not allow different CPP dialects in CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> How is #if __cplusplus >= 201103L qualitatively different from #ifndef NDEBUG 
> or #if MYLIB_ABI_VERSION==2 or #if __DATE__ == "2018-04-01"?

Ideally, all macros should be the same in the two TUs... If we were very strict 
then we could check for that, but that might be just too strict.

If there is an ODR error (either because of a macro mismatch or for other 
reasons) then structural equivalency will diagnose that, but may be nontrivial 
for the user to identify what causes the ODR error.
Checking the language and the language dialect is easy and it prevents such 
disturbing errors early.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57906



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

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this! It looks like we did have to add the new AST 
node after all, but the changes seem pretty good. Should anything be updated in 
the AST dumper for this (such as printing whether a Stmt is a value-producing 
statement or not)?




Comment at: include/clang/AST/Stmt.h:1598-1602
+  const Expr *getExprStmt() const;
+  Expr *getExprStmt() {
+const ValueStmt *ConstThis = this;
+return const_cast(ConstThis->getExprStmt());
+  }

We typically implement this in reverse, where the non-const version holds the 
actual implementation and the const version performs a `const_cast`.



Comment at: include/clang/Parse/Parser.h:374
+  /// a statement expression and builds a suitable expression statement.
+  StmtResult handleExprStmt(ExprResult E, WithinStmtExpr IsInStmtExpr);
 

Rather than passing around `IsInStmtExpr` to a bunch of parser APIs, would it 
make more sense to add an RAII object that sets some state on `Parser` to test 
it? The proliferation of arguments seems unfortunate given how much of a corner 
case statement expressions are.



Comment at: lib/Sema/SemaExpr.cpp:13319
   if (!Compound->body_empty()) {
-Stmt *LastStmt = Compound->body_back();
-LabelStmt *LastLabelStmt = nullptr;
-// If LastStmt is a label, skip down through into the body.
-while (LabelStmt *Label = dyn_cast(LastStmt)) {
-  LastLabelStmt = Label;
-  LastStmt = Label->getSubStmt();
-}
-
-if (Expr *LastE = dyn_cast(LastStmt)) {
-  // Do function/array conversion on the last expression, but not
-  // lvalue-to-rvalue.  However, initialize an unqualified type.
-  ExprResult LastExpr = DefaultFunctionArrayConversion(LastE);
-  if (LastExpr.isInvalid())
-return ExprError();
-  Ty = LastExpr.get()->getType().getUnqualifiedType();
-
-  if (!Ty->isDependentType() && !LastExpr.get()->isTypeDependent()) {
-// In ARC, if the final expression ends in a consume, splice
-// the consume out and bind it later.  In the alternate case
-// (when dealing with a retainable type), the result
-// initialization will create a produce.  In both cases the
-// result will be +1, and we'll need to balance that out with
-// a bind.
-if (Expr *rebuiltLastStmt
-  = maybeRebuildARCConsumingStmt(LastExpr.get())) {
-  LastExpr = rebuiltLastStmt;
-} else {
-  LastExpr = PerformCopyInitialization(
-  InitializedEntity::InitializeStmtExprResult(LPLoc, Ty),
-  SourceLocation(), LastExpr);
-}
-
-if (LastExpr.isInvalid())
-  return ExprError();
-if (LastExpr.get() != nullptr) {
-  if (!LastLabelStmt)
-Compound->setLastStmt(LastExpr.get());
-  else
-LastLabelStmt->setSubStmt(LastExpr.get());
-  StmtExprMayBindToTemp = true;
-}
+if (ValueStmt *LastStmt = dyn_cast(Compound->body_back())) {
+  if (Expr *Value = LastStmt->getExprStmt()) {

`const auto *`



Comment at: lib/Sema/SemaExpr.cpp:13355
+  // a bind.
+  ImplicitCastExpr *Cast = dyn_cast(E);
+  if (Cast && Cast->getCastKind() == CK_ARCConsumeObject)

`auto *`


Repository:
  rC Clang

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

https://reviews.llvm.org/D57984



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

[PATCH] D45978: dllexport const variables must have external linkage.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

It looks like the patch got mucked up somehow, I only see three testing files 
in the patch now?




Comment at: test/Sema/dllexport.c:168
+
+// CHECK: @y = common dso_local dllexport global i32 0, align 4
+

Nothing runs FileCheck in this test, so this isn't checking anything. That 
should be tested within CodeGen instead.


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

https://reviews.llvm.org/D45978



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

r353697 - Format isInSystemMacro after D55782

[Fangrui Song via cfe-commits]

Author: maskray
Date: Mon Feb 11 05:30:04 2019
New Revision: 353697

URL: http://llvm.org/viewvc/llvm-project?rev=353697&view=rev
Log:
Format isInSystemMacro after D55782

Modified:
cfe/trunk/include/clang/Basic/SourceManager.h

Modified: cfe/trunk/include/clang/Basic/SourceManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=353697&r1=353696&r2=353697&view=diff
==
--- cfe/trunk/include/clang/Basic/SourceManager.h (original)
+++ cfe/trunk/include/clang/Basic/SourceManager.h Mon Feb 11 05:30:04 2019
@@ -1458,17 +1458,15 @@ public:
 
   /// Returns whether \p Loc is expanded from a macro in a system header.
   bool isInSystemMacro(SourceLocation loc) const {
-if(!loc.isMacroID())
+if (!loc.isMacroID())
   return false;
 
 // This happens when the macro is the result of a paste, in that case
 // its spelling is the scratch memory, so we take the parent context.
-if (isWrittenInScratchSpace(getSpellingLoc(loc))) {
+if (isWrittenInScratchSpace(getSpellingLoc(loc)))
   return isInSystemHeader(getSpellingLoc(getImmediateMacroCallerLoc(loc)));
-}
-else {
-  return isInSystemHeader(getSpellingLoc(loc));
-}
+
+return isInSystemHeader(getSpellingLoc(loc));
   }
 
   /// The size of the SLocEntry that \p FID represents.


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

[PATCH] D58037: [clangd] Prefer location from codegen files when merging symbols.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clangd/XRefs.cpp:261
 }
+// Returns the preferred location between an AST location and an index 
location.
+SymbolLocation getPreferredLocation(const Location &ASTLoc,

nit: move this and toIndexLocation together?



Comment at: clangd/XRefs.cpp:364
+if (Sym.CanonicalDeclaration) {
+  // We only do this for declarations as definitions from AST
+  // is generally preferred (e.g. definitions in main file).

a little unclear what "we only do this" refers to - maybe missing a first line 
of the comment here.

// Use merge logic to choose AST or index declaration.



Comment at: clangd/index/Merge.cpp:155
+  getPreferredLocation(S.CanonicalDeclaration, O.CanonicalDeclaration);
+  S.Definition = getPreferredLocation(S.Definition, O.Definition);
   S.References += O.References;

I think it might be clearer what this is doing, and more similar with 
surrounding code, if the function returns a bool:

```
bool prefer(const SymbolLocation &Candidate, const SymbolLocation &Baseline);
...
if (!S.Definition || prefer(O.Definition, S.Definition))
  S.Definition = O.Definition;
```

up to you


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58037



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

[PATCH] D57948: [Sema] Fix a regression introduced in "[AST][Sema] Remove CallExpr::setNumArgs"

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/AST/Expr.cpp:1272-1274
+  for (unsigned I = NumArgs; I != NumArgsAllocated; ++I) {
+TrailingArgs[I] = nullptr;
   }

Could use `std::fill()` here to remove the for-loop. Your call whether that 
looks cleaner or not.



Comment at: lib/Sema/SemaExpr.cpp:5810
+// However it may not have been constructed with enough storage. In this
+// case rebuild the node with enough storage. The waste of space is
+// immaterial since this only happens when some typos were corrected.

case rebuild -> case, rebuild



Comment at: lib/Sema/SemaOverload.cpp:12909-12910
 
+call->setNumArgsUnsafe(
+std::max(Args.size(), proto->getNumParams()));
 if (ConvertArgumentsForCall(call, op, nullptr, proto, Args, RParenLoc))

Oye. When we had to call this hack in one place, I could hold my nose, but 
having to call it randomly any time something like a call expression is created 
is a bit too much for me.

How awful is the alternative fix, such as making things resilient to null 
parameters in `BuildResolvedCallExpr()`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57948



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

[PATCH] D58051: Revert "[clangd] Format tweak's replacements."

[Haojian Wu via Phabricator via cfe-commits]

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

This reverts commit r353306.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D58051

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/Compiler.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/Tweak.cpp
  clangd/refactor/Tweak.h
  clangd/refactor/tweaks/SwapIfBranches.cpp
  unittests/clangd/TweakTests.cpp

Index: unittests/clangd/TweakTests.cpp
===
--- unittests/clangd/TweakTests.cpp
+++ unittests/clangd/TweakTests.cpp
@@ -98,7 +98,7 @@
   auto T = prepareTweak(ID, S);
   if (!T)
 return T.takeError();
-  auto Replacements = (*T)->apply(S, clang::format::getLLVMStyle());
+  auto Replacements = (*T)->apply(S);
   if (!Replacements)
 return Replacements.takeError();
   return applyAllReplacements(Code.code(), *Replacements);
@@ -127,40 +127,12 @@
 
   llvm::StringLiteral Input = R"cpp(
 void test() {
-  ^if (true) {
-return 100;
-  } else {
-continue;
-  }
+  ^if (true) { return 100; } else { continue; }
 }
   )cpp";
   llvm::StringLiteral Output = R"cpp(
 void test() {
-  if (true) {
-continue;
-  } else {
-return 100;
-  }
-}
-  )cpp";
-  checkTransform(ID, Input, Output);
-
-  Input = R"cpp(
-void test() {
-  ^if () {
-return 100;
-  } else {
-continue;
-  }
-}
-  )cpp";
-  Output = R"cpp(
-void test() {
-  if () {
-continue;
-  } else {
-return 100;
-  }
+  if (true) { continue; } else { return 100; }
 }
   )cpp";
   checkTransform(ID, Input, Output);
@@ -172,11 +144,7 @@
   )cpp";
   Output = R"cpp(
 void test() {
-  if () {
-continue;
-  } else {
-return 100;
-  }
+  if () { continue; } else { return 100; }
 }
   )cpp";
   checkTransform(ID, Input, Output);
Index: clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- clangd/refactor/tweaks/SwapIfBranches.cpp
+++ clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -37,11 +37,9 @@
   const char *id() const override final;
 
   bool prepare(const Selection &Inputs) override;
+  Expected apply(const Selection &Inputs) override;
   std::string title() const override;
 
-protected:
-  Expected execute(const Selection &Inputs) override;
-
 private:
   const IfStmt *If = nullptr;
 };
@@ -62,8 +60,7 @@
  dyn_cast_or_null(If->getElse());
 }
 
-Expected
-SwapIfBranches::execute(const Selection &Inputs) {
+Expected SwapIfBranches::apply(const Selection &Inputs) {
   auto &Ctx = Inputs.AST.getASTContext();
   auto &SrcMgr = Ctx.getSourceManager();
 
Index: clangd/refactor/Tweak.h
===
--- clangd/refactor/Tweak.h
+++ clangd/refactor/Tweak.h
@@ -22,7 +22,6 @@
 #include "ClangdUnit.h"
 #include "Protocol.h"
 #include "Selection.h"
-#include "clang/Format/Format.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
@@ -48,7 +47,7 @@
 ParsedAST *
 /// A location of the cursor in the editor.
 SourceLocation Cursor;
-/// The AST nodes that were selected.
+// The AST nodes that were selected.
 SelectionTree ASTSelection;
 // FIXME: provide a way to get sources and ASTs for other files.
   };
@@ -64,20 +63,13 @@
   /// should be moved into 'apply'.
   /// Returns true iff the action is available and apply() can be called on it.
   virtual bool prepare(const Selection &Sel) = 0;
-  /// Format and apply the actual changes generated from the second stage of the
-  /// action.
+  /// Run the second stage of the action that would produce the actual changes.
   /// EXPECTS: prepare() was called and returned true.
-  Expected apply(const Selection &Sel,
-const format::FormatStyle &Style);
+  virtual Expected apply(const Selection &Sel) = 0;
   /// A one-line title of the action that should be shown to the users in the
   /// UI.
   /// EXPECTS: prepare() was called and returned true.
   virtual std::string title() const = 0;
-
-protected:
-  /// Run the second stage of the action that would produce the actual changes.
-  /// EXPECTS: prepare() was called and returned true.
-  virtual Expected execute(const Selection &Sel) = 0;
 };
 
 // All tweaks must be registered in the .cpp file next to their definition.
Index: clangd/refactor/Tweak.cpp
===
--- clangd/refactor/Tweak.cpp
+++ clangd/refactor/Tweak.cpp
@@ -46,14 +46,6 @@
   Cursor = SM.getComposedLoc(SM.getMainFileID(), RangeBegin);
 }
 
-Expected Tweak::apply(const Selection &Sel,
- const format::FormatSt

[PATCH] D58051: Revert "[clangd] Format tweak's replacements."

[Haojian Wu via Phabricator via cfe-commits]

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

- [clangd] Re-submit format replacement for tweaks.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58051

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/Compiler.h
  clangd/refactor/Tweak.cpp
  clangd/refactor/Tweak.h
  clangd/refactor/tweaks/SwapIfBranches.cpp
  test/clangd/tweaks-format.test
  unittests/clangd/TweakTests.cpp

Index: unittests/clangd/TweakTests.cpp
===
--- unittests/clangd/TweakTests.cpp
+++ unittests/clangd/TweakTests.cpp
@@ -98,7 +98,7 @@
   auto T = prepareTweak(ID, S);
   if (!T)
 return T.takeError();
-  auto Replacements = (*T)->apply(S, clang::format::getLLVMStyle());
+  auto Replacements = (*T)->apply(S);
   if (!Replacements)
 return Replacements.takeError();
   return applyAllReplacements(Code.code(), *Replacements);
@@ -127,40 +127,12 @@
 
   llvm::StringLiteral Input = R"cpp(
 void test() {
-  ^if (true) {
-return 100;
-  } else {
-continue;
-  }
+  ^if (true) { return 100; } else { continue; }
 }
   )cpp";
   llvm::StringLiteral Output = R"cpp(
 void test() {
-  if (true) {
-continue;
-  } else {
-return 100;
-  }
-}
-  )cpp";
-  checkTransform(ID, Input, Output);
-
-  Input = R"cpp(
-void test() {
-  ^if () {
-return 100;
-  } else {
-continue;
-  }
-}
-  )cpp";
-  Output = R"cpp(
-void test() {
-  if () {
-continue;
-  } else {
-return 100;
-  }
+  if (true) { continue; } else { return 100; }
 }
   )cpp";
   checkTransform(ID, Input, Output);
@@ -172,11 +144,7 @@
   )cpp";
   Output = R"cpp(
 void test() {
-  if () {
-continue;
-  } else {
-return 100;
-  }
+  if () { continue; } else { return 100; }
 }
   )cpp";
   checkTransform(ID, Input, Output);
Index: test/clangd/tweaks-format.test
===
--- /dev/null
+++ test/clangd/tweaks-format.test
@@ -0,0 +1,50 @@
+# RUN: clangd -lit-test < %s | FileCheck %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cc","languageId":"cpp","version":1,"text":"int f() { if (true) { return 1; } else {} }"}}}
+---
+{"jsonrpc":"2.0","id":5,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///main.cc"},"range":{"start":{"line":0,"character":11},"end":{"line":0,"character":11}},"context":{"diagnostics":[]}}}
+---
+{"jsonrpc":"2.0","id":6,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cc","selection":{"end":{"character":11,"line":0},"start":{"character":11,"line":0}},"tweakID":"SwapIfBranches"}]}}
+#  CHECK:"newText": "\n  ",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 10,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 9,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  {
+# CHECK-NEXT:"newText": "{\n  }",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 33,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 20,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  {
+# CHECK-NEXT:"newText": "{\nreturn 1;\n  }\n",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 42,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 39,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- clangd/refactor/tweaks/SwapIfBranches.cpp
+++ clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -37,11 +37,9 @@
   const char *id() const override final;
 
   bool prepare(const Selection &Inputs) override;
+  Expected apply(const Selection &Inputs) override;
   std::string title() const override;
 
-protected:
-  Expected execute(const Selection &Inputs) override;
-
 private:
   const IfStmt *If = nullptr;
 };
@@ -62,8 +60,7 @@
  dyn_cast_or_null(If->getElse());
 }
 
-Expected
-SwapIfBranches::execute(const Selection &Inputs) {
+Expected SwapIfBranches::apply(const Selection &Inputs) {
   auto &Ctx = I

r353698 - [analyzer] New checker for detecting usages of unsafe I/O functions

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Mon Feb 11 05:46:43 2019
New Revision: 353698

URL: http://llvm.org/viewvc/llvm-project?rev=353698&view=rev
Log:
[analyzer] New checker for detecting usages of unsafe I/O functions

There are certain unsafe or deprecated (since C11) buffer handling
functions which should be avoided in safety critical code. They
could cause buffer overflows. A new checker,
'security.insecureAPI.DeprecatedOrUnsafeBufferHandling' warns for
every occurrence of such functions (unsafe or deprecated printf,
scanf family, and other buffer handling functions, which now have
a secure variant).

Patch by Dániel Kolozsvári!

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

Modified:
cfe/trunk/docs/analyzer/checkers.rst
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
cfe/trunk/test/Analysis/security-syntax-checks.m

Modified: cfe/trunk/docs/analyzer/checkers.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/analyzer/checkers.rst?rev=353698&r1=353697&r2=353698&view=diff
==
--- cfe/trunk/docs/analyzer/checkers.rst (original)
+++ cfe/trunk/docs/analyzer/checkers.rst Mon Feb 11 05:46:43 2019
@@ -566,6 +566,17 @@ security.insecureAPI.vfork (C)
vfork(); // warn
  }
 
+security.insecureAPI.DeprecatedOrUnsafeBufferHandling (C) 
+""
+ Warn on occurrences of unsafe or deprecated buffer handling functions, which 
now have a secure variant: ``sprintf, vsprintf, scanf, wscanf, fscanf, fwscanf, 
vscanf, vwscanf, vfscanf, vfwscanf, sscanf, swscanf, vsscanf, vswscanf, 
swprintf, snprintf, vswprintf, vsnprintf, memcpy, memmove, strncpy, strncat, 
memset``
+
+.. code-block:: c
+ 
+ void test() {
+   char buf [5];
+   strncpy(buf, "a", 1); // warn
+ }
+
 .. _unix-checkers:
 
 unix

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=353698&r1=353697&r2=353698&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Mon Feb 11 
05:46:43 2019
@@ -622,6 +622,13 @@ def UncheckedReturn : Checker<"Unchecked
   Dependencies<[SecuritySyntaxChecker]>,
   Documentation;
 
+def DeprecatedOrUnsafeBufferHandling :
+  Checker<"DeprecatedOrUnsafeBufferHandling">,
+  HelpText<"Warn on uses of unsecure or deprecated buffer manipulating "
+   "functions">,
+  Dependencies<[SecuritySyntaxChecker]>,
+  Documentation;
+
 } // end "security.insecureAPI"
 
 let ParentPackage = Security in {

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp?rev=353698&r1=353697&r2=353698&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp Mon Feb 
11 05:46:43 2019
@@ -44,6 +44,7 @@ struct ChecksFilter {
   DefaultBool check_mktemp;
   DefaultBool check_mkstemp;
   DefaultBool check_strcpy;
+  DefaultBool check_DeprecatedOrUnsafeBufferHandling;
   DefaultBool check_rand;
   DefaultBool check_vfork;
   DefaultBool check_FloatLoopCounter;
@@ -57,6 +58,7 @@ struct ChecksFilter {
   CheckName checkName_mktemp;
   CheckName checkName_mkstemp;
   CheckName checkName_strcpy;
+  CheckName checkName_DeprecatedOrUnsafeBufferHandling;
   CheckName checkName_rand;
   CheckName checkName_vfork;
   CheckName checkName_FloatLoopCounter;
@@ -103,6 +105,8 @@ public:
   void checkCall_mkstemp(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD);
+  void checkDeprecatedOrUnsafeBufferHandling(const CallExpr *CE,
+ const FunctionDecl *FD);
   void checkCall_rand(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_random(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_vfork(const CallExpr *CE, const FunctionDecl *FD);
@@ -148,6 +152,14 @@ void WalkAST::VisitCallExpr(CallExpr *CE
 .Case("mkstemps", &WalkAST::checkCall_mkstemp)
 .Cases("strcpy", "__strcpy_chk", &WalkAST::checkCall_strcpy)
 .Cases("strcat", "__strcat_chk", &WalkAST::checkCall_strcat)
+.Cases("sprintf", "vsprintf", "scanf", "wscanf", "fscanf", "fwscanf",
+   "vscanf", "vwscanf", "vfscanf", "vfwscanf",
+   &WalkAST::checkDeprecatedOrUnsafeBufferHandling)
+.Cases("sscanf", "swscanf", "vsscanf", "vswscanf", "swprintf",
+   "snprintf", "vswprintf", "vsnprintf", "me

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:987-989
+The first argument to the attribute is the type passed to
+`__builtin_object_size`, and the second is the flag that the fortified format
+functions accept.

Maybe I'm being dense, but I still have no idea what I'd pass for either of 
these arguments. When I hear "type", I immediately assume I should pass in 
something like 'int', but that seems wrong given that this is an integer 
argument. Is there some public documentation we could link to with a "for more 
information, see " snippet? Similar for the fortified format function 
flag.

However, I'm also starting to wonder why this attribute is interesting to 
users. Why not infer this attribute automatically, since there's only a 
specified set of standard library functions that it can be applied to anyway? 
Is it a bad idea to try to infer this for some reason?


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

https://reviews.llvm.org/D57918



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

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353698: [analyzer] New checker for detecting usages of 
unsafe I/O functions (authored by Szelethus, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D35068?vs=186131&id=186240#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D35068

Files:
  cfe/trunk/docs/analyzer/checkers.rst
  cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
  cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  cfe/trunk/test/Analysis/security-syntax-checks.m

Index: cfe/trunk/docs/analyzer/checkers.rst
===
--- cfe/trunk/docs/analyzer/checkers.rst
+++ cfe/trunk/docs/analyzer/checkers.rst
@@ -566,6 +566,17 @@
vfork(); // warn
  }
 
+security.insecureAPI.DeprecatedOrUnsafeBufferHandling (C) 
+""
+ Warn on occurrences of unsafe or deprecated buffer handling functions, which now have a secure variant: ``sprintf, vsprintf, scanf, wscanf, fscanf, fwscanf, vscanf, vwscanf, vfscanf, vfwscanf, sscanf, swscanf, vsscanf, vswscanf, swprintf, snprintf, vswprintf, vsnprintf, memcpy, memmove, strncpy, strncat, memset``
+
+.. code-block:: c
+ 
+ void test() {
+   char buf [5];
+   strncpy(buf, "a", 1); // warn
+ }
+
 .. _unix-checkers:
 
 unix
Index: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -622,6 +622,13 @@
   Dependencies<[SecuritySyntaxChecker]>,
   Documentation;
 
+def DeprecatedOrUnsafeBufferHandling :
+  Checker<"DeprecatedOrUnsafeBufferHandling">,
+  HelpText<"Warn on uses of unsecure or deprecated buffer manipulating "
+   "functions">,
+  Dependencies<[SecuritySyntaxChecker]>,
+  Documentation;
+
 } // end "security.insecureAPI"
 
 let ParentPackage = Security in {
Index: cfe/trunk/test/Analysis/security-syntax-checks.m
===
--- cfe/trunk/test/Analysis/security-syntax-checks.m
+++ cfe/trunk/test/Analysis/security-syntax-checks.m
@@ -13,6 +13,9 @@
 # define BUILTIN(f) f
 #endif /* USE_BUILTINS */
 
+#include "Inputs/system-header-simulator-for-valist.h"
+#include "Inputs/system-header-simulator-for-simple-stream.h"
+
 typedef typeof(sizeof(int)) size_t;
 
 
@@ -238,3 +241,82 @@
   mkdtemp("XX");
 }
 
+
+//===--===
+// deprecated or unsafe buffer handling
+//===--===
+typedef int wchar_t;
+
+int sprintf(char *str, const char *format, ...);
+//int vsprintf (char *s, const char *format, va_list arg);
+int scanf(const char *format, ...);
+int wscanf(const wchar_t *format, ...);
+int fscanf(FILE *stream, const char *format, ...);
+int fwscanf(FILE *stream, const wchar_t *format, ...);
+int vscanf(const char *format, va_list arg);
+int vwscanf(const wchar_t *format, va_list arg);
+int vfscanf(FILE *stream, const char *format, va_list arg);
+int vfwscanf(FILE *stream, const wchar_t *format, va_list arg);
+int sscanf(const char *s, const char *format, ...);
+int swscanf(const wchar_t *ws, const wchar_t *format, ...);
+int vsscanf(const char *s, const char *format, va_list arg);
+int vswscanf(const wchar_t *ws, const wchar_t *format, va_list arg);
+int swprintf(wchar_t *ws, size_t len, const wchar_t *format, ...);
+int snprintf(char *s, size_t n, const char *format, ...);
+int vswprintf(wchar_t *ws, size_t len, const wchar_t *format, va_list arg);
+int vsnprintf(char *s, size_t n, const char *format, va_list arg);
+void *memcpy(void *destination, const void *source, size_t num);
+void *memmove(void *destination, const void *source, size_t num);
+char *strncpy(char *destination, const char *source, size_t num);
+char *strncat(char *destination, const char *source, size_t num);
+void *memset(void *ptr, int value, size_t num);
+
+void test_deprecated_or_unsafe_buffer_handling_1() {
+  char buf [5];
+  wchar_t wbuf [5];
+  int a;
+  FILE *file;
+  sprintf(buf, "a"); // expected-warning{{Call to function 'sprintf' is insecure}}
+  scanf("%d", &a); // expected-warning{{Call to function 'scanf' is insecure}}
+  scanf("%s", buf); // expected-warning{{Call to function 'scanf' is insecure}}
+  scanf("%4s", buf); // expected-warning{{Call to function 'scanf' is insecure}}
+  wscanf((const wchar_t*) L"%s", buf); // expected-warning{{Call to function 'wscanf' is insecure}}
+  fscanf(file, "%d", &a); // expected-warning{{Call to function 'fscanf' is insecure}}
+  fscanf(file, "%s", buf); // expected-warning{{Call to function 'fscanf' is insecure}}
+  fscanf(file, "%4s", buf)

[PATCH] D33841: [clang-tidy] redundant keyword check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: docs/clang-tidy/checks/readability-redundant-extern.rst:4
+readability-redundant-extern
+=
+

The underlining here is too long.



Comment at: docs/clang-tidy/checks/readability-redundant-extern.rst:14
+  
\ No newline at end of file


Please add the newline to the end of the file.



Comment at: test/clang-tidy/readability-redundant-extern.cpp:2
+// RUN: %check_clang_tidy %s readability-redundant-extern %t
+
+extern int f();

This is missing some other negative tests, like a file-scope:
```
void file_scope();
```
an especially interesting test would be:
```
void another_file_scope(int _extern);
```


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

https://reviews.llvm.org/D33841



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

[PATCH] D58037: [clangd] Prefer location from codegen files when merging symbols.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 186245.
ioeric marked 3 inline comments as done.
ioeric added a comment.

- Address review comments.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58037

Files:
  clangd/XRefs.cpp
  clangd/index/Merge.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -180,6 +180,26 @@
   ElementsAre(Sym("Forward", SymbolHeader.range("forward"), Test.range(;
 }
 
+TEST(LocateSymbol, WithIndexPreferredLocation) {
+  Annotations SymbolHeader(R"cpp(
+class $[[Proto]] {};
+  )cpp");
+  TestTU TU;
+  TU.HeaderCode = SymbolHeader.code();
+  TU.HeaderFilename = "x.proto"; // Prefer locations in codegen files.
+  auto Index = TU.index();
+
+  Annotations Test(R"cpp(// only declaration in AST.
+// Shift to make range different.
+class [[Proto]];
+P^roto* create();
+  )cpp");
+
+  auto AST = TestTU::withCode(Test.code()).build();
+  auto Locs = clangd::locateSymbolAt(AST, Test.point(), Index.get());
+  EXPECT_THAT(Locs, ElementsAre(Sym("Proto", SymbolHeader.range(;
+}
+
 TEST(LocateSymbol, All) {
   // Ranges in tests:
   //   $decl is the declaration location (if absent, no symbol is located)
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -253,6 +253,22 @@
   EXPECT_EQ(M.Name, "right");
 }
 
+TEST(MergeTest, PreferSymbolLocationInCodegenFile) {
+  Symbol L, R;
+
+  L.ID = R.ID = SymbolID("hello");
+  L.CanonicalDeclaration.FileURI = "file:/x.proto.h";
+  R.CanonicalDeclaration.FileURI = "file:/x.proto";
+
+  Symbol M = mergeSymbol(L, R);
+  EXPECT_EQ(StringRef(M.CanonicalDeclaration.FileURI), "file:/x.proto");
+
+  // Prefer L if both have codegen suffix.
+  L.CanonicalDeclaration.FileURI = "file:/y.proto";
+  M = mergeSymbol(L, R);
+  EXPECT_EQ(StringRef(M.CanonicalDeclaration.FileURI), "file:/y.proto");
+}
+
 TEST(MergeIndexTest, Refs) {
   FileIndex Dyn;
   FileIndex StaticIndex;
Index: clangd/index/Merge.cpp
===
--- clangd/index/Merge.cpp
+++ clangd/index/Merge.cpp
@@ -9,9 +9,13 @@
 #include "Merge.h"
 #include "Logger.h"
 #include "Trace.h"
+#include "index/Index.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -114,6 +118,23 @@
   });
 }
 
+// Returns true if \p L is (strictly) preferred to \p R (e.g. by file paths). If
+// neither is preferred, this returns false.
+bool prefer(const SymbolLocation &L, const SymbolLocation &R) {
+  if (!L)
+return false;
+  if (!R)
+return true;
+  auto HasCodeGenSuffix = [](const SymbolLocation &Loc) {
+constexpr static const char *CodegenSuffixes[] = {".proto"};
+return std::any_of(std::begin(CodegenSuffixes), std::end(CodegenSuffixes),
+   [&](llvm::StringRef Suffix) {
+ return llvm::StringRef(Loc.FileURI).endswith(Suffix);
+   });
+  };
+  return HasCodeGenSuffix(L) && !HasCodeGenSuffix(R);
+}
+
 Symbol mergeSymbol(const Symbol &L, const Symbol &R) {
   assert(L.ID == R.ID);
   // We prefer information from TUs that saw the definition.
@@ -128,12 +149,11 @@
   Symbol S = PreferR ? R : L;// The target symbol we're merging into.
   const Symbol &O = PreferR ? L : R; // The "other" less-preferred symbol.
 
-  // For each optional field, fill it from O if missing in S.
-  // (It might be missing in O too, but that's a no-op).
-  if (!S.Definition)
-S.Definition = O.Definition;
-  if (!S.CanonicalDeclaration)
+  // Only use locations in \p O if it's (strictly) preferred.
+  if (prefer(O.CanonicalDeclaration, S.CanonicalDeclaration))
 S.CanonicalDeclaration = O.CanonicalDeclaration;
+  if (prefer(O.Definition, S.Definition))
+S.Definition = O.Definition;
   S.References += O.References;
   if (S.Signature == "")
 S.Signature = O.Signature;
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -10,6 +10,8 @@
 #include "Logger.h"
 #include "SourceCode.h"
 #include "URI.h"
+#include "index/Index.h"
+#include "index/Merge.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Index/IndexDataConsumer.h"
@@ -77,6 +79,32 @@
   return LSPLoc;
 }
 
+SymbolLocation toIndexLocation(const Location &Loc, std::string &URIStorage) {
+  SymbolLocation SymLoc;
+  URIStorage = Loc.uri.uri();
+  SymLoc.FileURI = URIStorage.c_str();
+  SymLoc.Start.setLine(Loc.ran

[PATCH] D58051: Revamp the "[clangd] Format tweak's replacements"

[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




Comment at: clangd/ClangdServer.cpp:382
+auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents,
+   FSProvider.getFileSystem().get());
+return CB(

NIT: use `InpAST.Inputs.FS` instead of creating a new instance of the vfs.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58051



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

r353705 - Make some helper functions static. NFC.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Mon Feb 11 06:52:15 2019
New Revision: 353705

URL: http://llvm.org/viewvc/llvm-project?rev=353705&view=rev
Log:
Make some helper functions static. NFC.

Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=353705&r1=353704&r2=353705&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Mon Feb 11 06:52:15 2019
@@ -968,8 +968,8 @@ void ResultBuilder::AdjustResultPriority
   }
 }
 
-DeclContext::lookup_result getConstructors(ASTContext &Context,
-   const CXXRecordDecl *Record) {
+static DeclContext::lookup_result getConstructors(ASTContext &Context,
+  const CXXRecordDecl *Record) 
{
   QualType RecordTy = Context.getTypeDeclType(Record);
   DeclarationName ConstructorName =
   Context.DeclarationNames.getCXXConstructorName(

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp?rev=353705&r1=353704&r2=353705&view=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
 (original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
 Mon Feb 11 06:52:15 2019
@@ -182,7 +182,7 @@ static Optional findArgIdxOfSy
   return None;
 }
 
-Optional findMetaClassAlloc(const Expr *Callee) {
+static Optional findMetaClassAlloc(const Expr *Callee) {
   if (const auto *ME = dyn_cast(Callee)) {
 if (ME->getMemberDecl()->getNameAsString() != "alloc")
   return None;
@@ -200,8 +200,7 @@ Optional findMetaClassAlloc
   return None;
 }
 
-std::string findAllocatedObjectName(const Stmt *S,
-QualType QT) {
+static std::string findAllocatedObjectName(const Stmt *S, QualType QT) {
   if (const auto *CE = dyn_cast(S))
 if (auto Out = findMetaClassAlloc(CE->getCallee()))
   return *Out;


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

[PATCH] D57528: [tools] Fix python DeprecationWarning: invalid escape sequence

[Marco Falke via Phabricator via cfe-commits]

MarcoFalke added a comment.

No, I don't have commit right


Repository:
  rC Clang

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

https://reviews.llvm.org/D57528



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

[PATCH] D58051: Revamp the "[clangd] Format tweak's replacements"

[Haojian Wu via Phabricator via cfe-commits]

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

Address review comment.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58051

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/Compiler.h
  clangd/refactor/Tweak.cpp
  clangd/refactor/Tweak.h
  clangd/refactor/tweaks/SwapIfBranches.cpp
  test/clangd/tweaks-format.test
  unittests/clangd/TweakTests.cpp

Index: unittests/clangd/TweakTests.cpp
===
--- unittests/clangd/TweakTests.cpp
+++ unittests/clangd/TweakTests.cpp
@@ -98,7 +98,7 @@
   auto T = prepareTweak(ID, S);
   if (!T)
 return T.takeError();
-  auto Replacements = (*T)->apply(S, clang::format::getLLVMStyle());
+  auto Replacements = (*T)->apply(S);
   if (!Replacements)
 return Replacements.takeError();
   return applyAllReplacements(Code.code(), *Replacements);
@@ -127,40 +127,12 @@
 
   llvm::StringLiteral Input = R"cpp(
 void test() {
-  ^if (true) {
-return 100;
-  } else {
-continue;
-  }
+  ^if (true) { return 100; } else { continue; }
 }
   )cpp";
   llvm::StringLiteral Output = R"cpp(
 void test() {
-  if (true) {
-continue;
-  } else {
-return 100;
-  }
-}
-  )cpp";
-  checkTransform(ID, Input, Output);
-
-  Input = R"cpp(
-void test() {
-  ^if () {
-return 100;
-  } else {
-continue;
-  }
-}
-  )cpp";
-  Output = R"cpp(
-void test() {
-  if () {
-continue;
-  } else {
-return 100;
-  }
+  if (true) { continue; } else { return 100; }
 }
   )cpp";
   checkTransform(ID, Input, Output);
@@ -172,11 +144,7 @@
   )cpp";
   Output = R"cpp(
 void test() {
-  if () {
-continue;
-  } else {
-return 100;
-  }
+  if () { continue; } else { return 100; }
 }
   )cpp";
   checkTransform(ID, Input, Output);
Index: test/clangd/tweaks-format.test
===
--- /dev/null
+++ test/clangd/tweaks-format.test
@@ -0,0 +1,50 @@
+# RUN: clangd -lit-test < %s | FileCheck %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cc","languageId":"cpp","version":1,"text":"int f() { if (true) { return 1; } else {} }"}}}
+---
+{"jsonrpc":"2.0","id":5,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///main.cc"},"range":{"start":{"line":0,"character":11},"end":{"line":0,"character":11}},"context":{"diagnostics":[]}}}
+---
+{"jsonrpc":"2.0","id":6,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cc","selection":{"end":{"character":11,"line":0},"start":{"character":11,"line":0}},"tweakID":"SwapIfBranches"}]}}
+#  CHECK:"newText": "\n  ",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 10,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 9,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  {
+# CHECK-NEXT:"newText": "{\n  }",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 33,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 20,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  {
+# CHECK-NEXT:"newText": "{\nreturn 1;\n  }\n",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 42,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 39,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- clangd/refactor/tweaks/SwapIfBranches.cpp
+++ clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -37,11 +37,9 @@
   const char *id() const override final;
 
   bool prepare(const Selection &Inputs) override;
+  Expected apply(const Selection &Inputs) override;
   std::string title() const override;
 
-protected:
-  Expected execute(const Selection &Inputs) override;
-
 private:
   const IfStmt *If = nullptr;
 };
@@ -62,8 +60,7 @@
  dyn_cast_or_null(If->getElse());
 }
 
-Expected
-SwapIfBranches::execute(const Selection &Inputs) {
+Expected SwapIfBranches::apply(const Selection &Inputs) {
   auto &Ctx = Inputs.AST.getASTContext();
 

r353707 - [tools] Fix python DeprecationWarning: invalid escape sequence

[Serge Guelton via cfe-commits]

Author: serge_sans_paille
Date: Mon Feb 11 07:03:17 2019
New Revision: 353707

URL: http://llvm.org/viewvc/llvm-project?rev=353707&view=rev
Log:
[tools] Fix python DeprecationWarning: invalid escape sequence

The python documentation says "it’s highly recommended that you use raw strings 
for all but the simplest expressions." 
(https://docs.python.org/3/library/re.html)

So do that with the attached patch generated by

sed -i -e "s/re.search('/re.search(r'/g" $(git grep -l 're.search(')

The warning can be seen in e.g. python3.7:

$ python3.7 -Wd
>>> import re; re.search('\s', '')
:1: DeprecationWarning: invalid escape sequence \s

Commited on behalf of Marco Falke.
Differential Revision: https://reviews.llvm.org/D57528

Modified:
cfe/trunk/tools/clang-format/clang-format-diff.py
cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
cfe/trunk/tools/scan-build/bin/set-xcode-analyzer

Modified: cfe/trunk/tools/clang-format/clang-format-diff.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-format/clang-format-diff.py?rev=353707&r1=353706&r2=353707&view=diff
==
--- cfe/trunk/tools/clang-format/clang-format-diff.py (original)
+++ cfe/trunk/tools/clang-format/clang-format-diff.py Mon Feb 11 07:03:17 2019
@@ -66,7 +66,7 @@ def main():
   filename = None
   lines_by_file = {}
   for line in sys.stdin:
-match = re.search('^\+\+\+\ (.*?/){%s}(\S*)' % args.p, line)
+match = re.search(r'^\+\+\+\ (.*?/){%s}(\S*)' % args.p, line)
 if match:
   filename = match.group(2)
 if filename == None:
@@ -79,7 +79,7 @@ def main():
   if not re.match('^%s$' % args.iregex, filename, re.IGNORECASE):
 continue
 
-match = re.search('^@@.*\+(\d+)(,(\d+))?', line)
+match = re.search(r'^@@.*\+(\d+)(,(\d+))?', line)
 if match:
   start_line = int(match.group(1))
   line_count = 1

Modified: cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py?rev=353707&r1=353706&r2=353707&view=diff
==
--- cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py (original)
+++ cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py Mon Feb 11 07:03:17 
2019
@@ -97,7 +97,7 @@ def need_analyzer(args):
 when compiler wrappers are used. That's the moment when build setup
 check the compiler and capture the location for the build process. """
 
-return len(args) and not re.search('configure|autogen', args[0])
+return len(args) and not re.search(r'configure|autogen', args[0])
 
 
 def prefix_with(constant, pieces):

Modified: cfe/trunk/tools/scan-build/bin/set-xcode-analyzer
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build/bin/set-xcode-analyzer?rev=353707&r1=353706&r2=353707&view=diff
==
--- cfe/trunk/tools/scan-build/bin/set-xcode-analyzer (original)
+++ cfe/trunk/tools/scan-build/bin/set-xcode-analyzer Mon Feb 11 07:03:17 2019
@@ -42,7 +42,7 @@ def ModifySpec(path, isBuiltinAnalyzer,
 if line.find("Static Analyzer") >= 0:
   foundAnalyzer = True
   else:
-m = re.search('^(\s*ExecPath\s*=\s*")', line)
+m = re.search(r'^(\s*ExecPath\s*=\s*")', line)
 if m:
   line = "".join([m.group(0), pathToChecker, '";\n'])
   # Do not modify further ExecPath's later in the xcspec.


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

[PATCH] D57528: [tools] Fix python DeprecationWarning: invalid escape sequence

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353707: [tools] Fix python DeprecationWarning: invalid 
escape sequence (authored by serge_sans_paille, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57528?vs=184547&id=186248#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57528

Files:
  cfe/trunk/tools/clang-format/clang-format-diff.py
  cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
  cfe/trunk/tools/scan-build/bin/set-xcode-analyzer


Index: cfe/trunk/tools/scan-build/bin/set-xcode-analyzer
===
--- cfe/trunk/tools/scan-build/bin/set-xcode-analyzer
+++ cfe/trunk/tools/scan-build/bin/set-xcode-analyzer
@@ -42,7 +42,7 @@
 if line.find("Static Analyzer") >= 0:
   foundAnalyzer = True
   else:
-m = re.search('^(\s*ExecPath\s*=\s*")', line)
+m = re.search(r'^(\s*ExecPath\s*=\s*")', line)
 if m:
   line = "".join([m.group(0), pathToChecker, '";\n'])
   # Do not modify further ExecPath's later in the xcspec.
Index: cfe/trunk/tools/clang-format/clang-format-diff.py
===
--- cfe/trunk/tools/clang-format/clang-format-diff.py
+++ cfe/trunk/tools/clang-format/clang-format-diff.py
@@ -66,7 +66,7 @@
   filename = None
   lines_by_file = {}
   for line in sys.stdin:
-match = re.search('^\+\+\+\ (.*?/){%s}(\S*)' % args.p, line)
+match = re.search(r'^\+\+\+\ (.*?/){%s}(\S*)' % args.p, line)
 if match:
   filename = match.group(2)
 if filename == None:
@@ -79,7 +79,7 @@
   if not re.match('^%s$' % args.iregex, filename, re.IGNORECASE):
 continue
 
-match = re.search('^@@.*\+(\d+)(,(\d+))?', line)
+match = re.search(r'^@@.*\+(\d+)(,(\d+))?', line)
 if match:
   start_line = int(match.group(1))
   line_count = 1
Index: cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
===
--- cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
+++ cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
@@ -97,7 +97,7 @@
 when compiler wrappers are used. That's the moment when build setup
 check the compiler and capture the location for the build process. """
 
-return len(args) and not re.search('configure|autogen', args[0])
+return len(args) and not re.search(r'configure|autogen', args[0])
 
 
 def prefix_with(constant, pieces):


Index: cfe/trunk/tools/scan-build/bin/set-xcode-analyzer
===
--- cfe/trunk/tools/scan-build/bin/set-xcode-analyzer
+++ cfe/trunk/tools/scan-build/bin/set-xcode-analyzer
@@ -42,7 +42,7 @@
 if line.find("Static Analyzer") >= 0:
   foundAnalyzer = True
   else:
-m = re.search('^(\s*ExecPath\s*=\s*")', line)
+m = re.search(r'^(\s*ExecPath\s*=\s*")', line)
 if m:
   line = "".join([m.group(0), pathToChecker, '";\n'])
   # Do not modify further ExecPath's later in the xcspec.
Index: cfe/trunk/tools/clang-format/clang-format-diff.py
===
--- cfe/trunk/tools/clang-format/clang-format-diff.py
+++ cfe/trunk/tools/clang-format/clang-format-diff.py
@@ -66,7 +66,7 @@
   filename = None
   lines_by_file = {}
   for line in sys.stdin:
-match = re.search('^\+\+\+\ (.*?/){%s}(\S*)' % args.p, line)
+match = re.search(r'^\+\+\+\ (.*?/){%s}(\S*)' % args.p, line)
 if match:
   filename = match.group(2)
 if filename == None:
@@ -79,7 +79,7 @@
   if not re.match('^%s$' % args.iregex, filename, re.IGNORECASE):
 continue
 
-match = re.search('^@@.*\+(\d+)(,(\d+))?', line)
+match = re.search(r'^@@.*\+(\d+)(,(\d+))?', line)
 if match:
   start_line = int(match.group(1))
   line_count = 1
Index: cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
===
--- cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
+++ cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
@@ -97,7 +97,7 @@
 when compiler wrappers are used. That's the moment when build setup
 check the compiler and capture the location for the build process. """
 
-return len(args) and not re.search('configure|autogen', args[0])
+return len(args) and not re.search(r'configure|autogen', args[0])
 
 
 def prefix_with(constant, pieces):
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r353708 - [clangd] Prefer location from codegen files when merging symbols.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Mon Feb 11 07:05:29 2019
New Revision: 353708

URL: http://llvm.org/viewvc/llvm-project?rev=353708&view=rev
Log:
[clangd] Prefer location from codegen files when merging symbols.

Summary:
For example, if an index symbol has location in a .proto file and an AST symbol
has location in a generated .proto.h file, then we prefer location in .proto
which is more meaningful to users.

Also use `mergeSymbols` to get the preferred location between AST location and 
index location in go-to-def.

Reviewers: sammccall

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/index/Merge.cpp
clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=353708&r1=353707&r2=353708&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Mon Feb 11 07:05:29 2019
@@ -10,6 +10,8 @@
 #include "Logger.h"
 #include "SourceCode.h"
 #include "URI.h"
+#include "index/Index.h"
+#include "index/Merge.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Index/IndexDataConsumer.h"
@@ -77,6 +79,32 @@ llvm::Optional toLSPLocation(c
   return LSPLoc;
 }
 
+SymbolLocation toIndexLocation(const Location &Loc, std::string &URIStorage) {
+  SymbolLocation SymLoc;
+  URIStorage = Loc.uri.uri();
+  SymLoc.FileURI = URIStorage.c_str();
+  SymLoc.Start.setLine(Loc.range.start.line);
+  SymLoc.Start.setColumn(Loc.range.start.character);
+  SymLoc.End.setLine(Loc.range.end.line);
+  SymLoc.End.setColumn(Loc.range.end.character);
+  return SymLoc;
+}
+
+// Returns the preferred location between an AST location and an index 
location.
+SymbolLocation getPreferredLocation(const Location &ASTLoc,
+const SymbolLocation &IdxLoc) {
+  // Also use a dummy symbol for the index location so that other fields (e.g.
+  // definition) are not factored into the preferrence.
+  Symbol ASTSym, IdxSym;
+  ASTSym.ID = IdxSym.ID = SymbolID("dummy_id");
+  std::string URIStore;
+  ASTSym.CanonicalDeclaration = toIndexLocation(ASTLoc, URIStore);
+  IdxSym.CanonicalDeclaration = IdxLoc;
+  auto Merged = mergeSymbol(ASTSym, IdxSym);
+  return Merged.CanonicalDeclaration;
+}
+
+
 struct MacroDecl {
   llvm::StringRef Name;
   const MacroInfo *Info;
@@ -329,12 +357,23 @@ std::vector locateSymbolA
 
   // Special case: if the AST yielded a definition, then it may not be
   // the right *declaration*. Prefer the one from the index.
-  if (R.Definition) // from AST
+  if (R.Definition) { // from AST
 if (auto Loc = toLSPLocation(Sym.CanonicalDeclaration, *MainFilePath))
   R.PreferredDeclaration = *Loc;
-
-  if (!R.Definition)
+  } else {
 R.Definition = toLSPLocation(Sym.Definition, *MainFilePath);
+
+if (Sym.CanonicalDeclaration) {
+  // Use merge logic to choose AST or index declaration.
+  // We only do this for declarations as definitions from AST
+  // is generally preferred (e.g. definitions in main file).
+  if (auto Loc =
+  toLSPLocation(getPreferredLocation(R.PreferredDeclaration,
+ Sym.CanonicalDeclaration),
+*MainFilePath))
+R.PreferredDeclaration = *Loc;
+}
+  }
 });
   }
 

Modified: clang-tools-extra/trunk/clangd/index/Merge.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Merge.cpp?rev=353708&r1=353707&r2=353708&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Merge.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Merge.cpp Mon Feb 11 07:05:29 2019
@@ -9,9 +9,13 @@
 #include "Merge.h"
 #include "Logger.h"
 #include "Trace.h"
+#include "index/Index.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -114,6 +118,23 @@ void MergedIndex::refs(const RefsRequest
   });
 }
 
+// Returns true if \p L is (strictly) preferred to \p R (e.g. by file paths). 
If
+// neither is preferred, this returns false.
+bool prefer(const SymbolLocation &L, const SymbolLocation &R) {
+  if (!L)
+return false;
+  if (!R)
+return true;
+  auto HasCodeGenSuffix = [](const SymbolLocation &Loc) {
+constexpr static const char *CodegenSuffixes[] = {".proto"};
+return std::any_of(std::b

[PATCH] D57528: [tools] Fix python DeprecationWarning: invalid escape sequence

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@MarcoFalke it's in, thanks for the patch o/


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57528



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

[PATCH] D58037: [clangd] Prefer location from codegen files when merging symbols.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE353708: [clangd] Prefer location from codegen files when 
merging symbols. (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58037?vs=186245&id=186251#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58037

Files:
  clangd/XRefs.cpp
  clangd/index/Merge.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -184,6 +184,26 @@
   ElementsAre(Sym("Forward", SymbolHeader.range("forward"), Test.range(;
 }
 
+TEST(LocateSymbol, WithIndexPreferredLocation) {
+  Annotations SymbolHeader(R"cpp(
+class $[[Proto]] {};
+  )cpp");
+  TestTU TU;
+  TU.HeaderCode = SymbolHeader.code();
+  TU.HeaderFilename = "x.proto"; // Prefer locations in codegen files.
+  auto Index = TU.index();
+
+  Annotations Test(R"cpp(// only declaration in AST.
+// Shift to make range different.
+class [[Proto]];
+P^roto* create();
+  )cpp");
+
+  auto AST = TestTU::withCode(Test.code()).build();
+  auto Locs = clangd::locateSymbolAt(AST, Test.point(), Index.get());
+  EXPECT_THAT(Locs, ElementsAre(Sym("Proto", SymbolHeader.range(;
+}
+
 TEST(LocateSymbol, All) {
   // Ranges in tests:
   //   $decl is the declaration location (if absent, no symbol is located)
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -253,6 +253,22 @@
   EXPECT_EQ(M.Name, "right");
 }
 
+TEST(MergeTest, PreferSymbolLocationInCodegenFile) {
+  Symbol L, R;
+
+  L.ID = R.ID = SymbolID("hello");
+  L.CanonicalDeclaration.FileURI = "file:/x.proto.h";
+  R.CanonicalDeclaration.FileURI = "file:/x.proto";
+
+  Symbol M = mergeSymbol(L, R);
+  EXPECT_EQ(StringRef(M.CanonicalDeclaration.FileURI), "file:/x.proto");
+
+  // Prefer L if both have codegen suffix.
+  L.CanonicalDeclaration.FileURI = "file:/y.proto";
+  M = mergeSymbol(L, R);
+  EXPECT_EQ(StringRef(M.CanonicalDeclaration.FileURI), "file:/y.proto");
+}
+
 TEST(MergeIndexTest, Refs) {
   FileIndex Dyn;
   FileIndex StaticIndex;
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -10,6 +10,8 @@
 #include "Logger.h"
 #include "SourceCode.h"
 #include "URI.h"
+#include "index/Index.h"
+#include "index/Merge.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Index/IndexDataConsumer.h"
@@ -77,6 +79,32 @@
   return LSPLoc;
 }
 
+SymbolLocation toIndexLocation(const Location &Loc, std::string &URIStorage) {
+  SymbolLocation SymLoc;
+  URIStorage = Loc.uri.uri();
+  SymLoc.FileURI = URIStorage.c_str();
+  SymLoc.Start.setLine(Loc.range.start.line);
+  SymLoc.Start.setColumn(Loc.range.start.character);
+  SymLoc.End.setLine(Loc.range.end.line);
+  SymLoc.End.setColumn(Loc.range.end.character);
+  return SymLoc;
+}
+
+// Returns the preferred location between an AST location and an index location.
+SymbolLocation getPreferredLocation(const Location &ASTLoc,
+const SymbolLocation &IdxLoc) {
+  // Also use a dummy symbol for the index location so that other fields (e.g.
+  // definition) are not factored into the preferrence.
+  Symbol ASTSym, IdxSym;
+  ASTSym.ID = IdxSym.ID = SymbolID("dummy_id");
+  std::string URIStore;
+  ASTSym.CanonicalDeclaration = toIndexLocation(ASTLoc, URIStore);
+  IdxSym.CanonicalDeclaration = IdxLoc;
+  auto Merged = mergeSymbol(ASTSym, IdxSym);
+  return Merged.CanonicalDeclaration;
+}
+
+
 struct MacroDecl {
   llvm::StringRef Name;
   const MacroInfo *Info;
@@ -329,12 +357,23 @@
 
   // Special case: if the AST yielded a definition, then it may not be
   // the right *declaration*. Prefer the one from the index.
-  if (R.Definition) // from AST
+  if (R.Definition) { // from AST
 if (auto Loc = toLSPLocation(Sym.CanonicalDeclaration, *MainFilePath))
   R.PreferredDeclaration = *Loc;
-
-  if (!R.Definition)
+  } else {
 R.Definition = toLSPLocation(Sym.Definition, *MainFilePath);
+
+if (Sym.CanonicalDeclaration) {
+  // Use merge logic to choose AST or index declaration.
+  // We only do this for declarations as definitions from AST
+  // is generally preferred (e.g. definitions in main file).
+  if (auto Loc =
+  toLSPLocation(getPreferredLocation(R.PreferredDeclaration,
+ Sym.CanonicalDeclaration),
+*MainFilePath))
+R.PreferredDeclarati

[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D57768#1386941 , @ABataev wrote:

> In D57768#1386933 , @bader wrote:
>
> > In D57768#1386924 , @ABataev wrote:
> >
> > > This definitely requires a test.
> >
> >
> > @ABataev, I tried to find some tests on similar `-fcuda-is-device` and 
> > `-fopenmp-is-device` options, but I wasn't able to find a dedicated test. 
> > Could you suggest some examples testing similar functionality, please?
>
>
> There are several similar tests:
>  OpenMP/driver.c, Driver/openmp-offload.c, Driver/openmp-offload-gpu.c. There 
> is no absolutely the same test for OpenMP, since OpenMP has mo similar req 
> for the offloading.


@ABataev thanks for the pointers. The uploaded patch adds two options:

- fsycl-is-device (front-end option)
- sycl-device-only (driver option)

The driver tests you mention validate driver logic enabled by new options, 
which is not part of this test and I was going to add it later.
I can split the patch and remove new driver option and leave only front-end 
option.
Another option is to add driver logic that invokes the front-end compiler in 
"device only" mode.
Which option do you prefer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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

r353711 - Fixed header underline in docs.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Mon Feb 11 07:17:13 2019
New Revision: 353711

URL: http://llvm.org/viewvc/llvm-project?rev=353711&view=rev
Log:
Fixed header underline in docs.

+ Removed trailing whitespace.

Modified:
cfe/trunk/docs/analyzer/checkers.rst

Modified: cfe/trunk/docs/analyzer/checkers.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/analyzer/checkers.rst?rev=353711&r1=353710&r2=353711&view=diff
==
--- cfe/trunk/docs/analyzer/checkers.rst (original)
+++ cfe/trunk/docs/analyzer/checkers.rst Mon Feb 11 07:17:13 2019
@@ -2,11 +2,11 @@
 Available Checkers
 ==
 
-The analyzer performs checks that are categorized into families or "checkers". 
+The analyzer performs checks that are categorized into families or "checkers".
+
+The default set of checkers covers a variety of checks targeted at finding 
security and API usage bugs,
+dead code, and other logic errors. See the :ref:`default-checkers` checkers 
list below.
 
-The default set of checkers covers a variety of checks targeted at finding 
security and API usage bugs, 
-dead code, and other logic errors. See the :ref:`default-checkers` checkers 
list below. 
- 
 In addition to these, the analyzer contains a number of :ref:`alpha-checkers` 
(aka *alpha* checkers).
 These checkers are under development and are switched off by default. They may 
crash or emit a higher number of false positives.
 
@@ -25,32 +25,32 @@ Default Checkers
 
 core
 
-Models core language features and contains general-purpose checkers such as 
division by zero, 
-null pointer dereference, usage of uninitialized values, etc. 
+Models core language features and contains general-purpose checkers such as 
division by zero,
+null pointer dereference, usage of uninitialized values, etc.
 *These checkers must be always switched on as other checker rely on them.*
 
 core.CallAndMessage (C, C++, ObjC)
 ""
  Check for logical errors for function calls and Objective-C message 
expressions (e.g., uninitialized arguments, null function pointers).
 
-.. literalinclude:: checkers/callandmessage_example.c
+.. literalinclude:: checkers/callandmessage_example.c
 :language: objc
-
+
 core.DivideZero (C, C++, ObjC)
 ""
  Check for division by zero.
 
 .. literalinclude:: checkers/dividezero_example.c
 :language: c
- 
-core.NonNullParamChecker (C, C++, ObjC) 
+
+core.NonNullParamChecker (C, C++, ObjC)
 """
 Check for null pointers passed as arguments to a function whose arguments are 
references or marked with the 'nonnull' attribute.
 
-.. code-block:: cpp   
-   
+.. code-block:: cpp
+
  int f(int *p) __attribute__((nonnull));
- 
+
  void test(int *p) {
if (!p)
  f(p); // warn
@@ -66,34 +66,34 @@ Check for dereferences of null pointers.
  void test(int *p) {
if (p)
  return;
- 
+
int x = p[0]; // warn
  }
- 
+
  // C
  void test(int *p) {
if (!p)
  *p = 0; // warn
  }
- 
+
  // C++
  class C {
  public:
int x;
  };
- 
+
  void test() {
C *pc = 0;
int k = pc->x; // warn
  }
- 
+
  // Objective-C
  @interface MyClass {
  @public
int x;
  }
  @end
- 
+
  void test() {
MyClass *obj = 0;
obj->x = 1; // warn
@@ -106,16 +106,16 @@ Check that addresses to stack memory do
 .. code-block:: c
 
  char const *p;
- 
+
  void test() {
char const str[] = "string";
p = str; // warn
  }
- 
+
  void* test() {
 return __builtin_alloca(12); // warn
  }
- 
+
  void test() {
static int *x;
int y;
@@ -134,19 +134,19 @@ Check for undefined results of binary op
int y = x + 1; // warn: left operand is garbage
  }
 
-core.VLASize (C) 
+core.VLASize (C)
 
 Check for declarations of Variable Length Arrays of undefined or zero size.
 
  Check for declarations of VLA of undefined or zero size.
- 
+
 .. code-block:: c
 
  void test() {
int x;
int vla1[x]; // warn: garbage as size
  }
- 
+
  void test() {
int x = 0;
int vla2[x]; // warn: zero size
@@ -220,14 +220,14 @@ cplusplus.InnerPointer
 ""
 Check for inner pointers of C++ containers used after re/deallocation.
 
-cplusplus.NewDelete (C++) 
+cplusplus.NewDelete (C++)
 "
 Check for double-free and use-after-free problems. Traces memory managed by 
new/delete.
 
 .. literalinclude:: checkers/newdelete_example.cpp
 :language: cpp
-
-cplusplus.NewDeleteLeaks (C++) 
+
+cplusplus.NewDeleteLeaks (C++)
 ""
 Check for memory leaks. Traces memory managed by new/delete.
 
@@ -238,7 +238,7 @@ Check for memory leaks. Traces memory ma
  } // warn
 
 
-cplusplus.SelfAssignment (C++) 
+cplusplus.SelfAssignment (C++)
 ""
 Checks C++ copy and move assignment operators for self assignment.
 
@@ -248,8 +248,8 @@ deadcode
 
 
 Dead Code Checkers.
- 
-deadco

[PATCH] D58051: Revamp the "[clangd] Format tweak's replacements"

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.
Closed by commit rCTE353712: Revamp the "[clangd] Format tweak's 
replacements" (authored by hokein, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58051?vs=186246&id=186257#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58051

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/Compiler.h
  clangd/refactor/Tweak.cpp
  clangd/refactor/Tweak.h
  clangd/refactor/tweaks/SwapIfBranches.cpp
  test/clangd/tweaks-format.test
  unittests/clangd/TweakTests.cpp

Index: unittests/clangd/TweakTests.cpp
===
--- unittests/clangd/TweakTests.cpp
+++ unittests/clangd/TweakTests.cpp
@@ -98,7 +98,7 @@
   auto T = prepareTweak(ID, S);
   if (!T)
 return T.takeError();
-  auto Replacements = (*T)->apply(S, clang::format::getLLVMStyle());
+  auto Replacements = (*T)->apply(S);
   if (!Replacements)
 return Replacements.takeError();
   return applyAllReplacements(Code.code(), *Replacements);
@@ -127,40 +127,12 @@
 
   llvm::StringLiteral Input = R"cpp(
 void test() {
-  ^if (true) {
-return 100;
-  } else {
-continue;
-  }
+  ^if (true) { return 100; } else { continue; }
 }
   )cpp";
   llvm::StringLiteral Output = R"cpp(
 void test() {
-  if (true) {
-continue;
-  } else {
-return 100;
-  }
-}
-  )cpp";
-  checkTransform(ID, Input, Output);
-
-  Input = R"cpp(
-void test() {
-  ^if () {
-return 100;
-  } else {
-continue;
-  }
-}
-  )cpp";
-  Output = R"cpp(
-void test() {
-  if () {
-continue;
-  } else {
-return 100;
-  }
+  if (true) { continue; } else { return 100; }
 }
   )cpp";
   checkTransform(ID, Input, Output);
@@ -172,11 +144,7 @@
   )cpp";
   Output = R"cpp(
 void test() {
-  if () {
-continue;
-  } else {
-return 100;
-  }
+  if () { continue; } else { return 100; }
 }
   )cpp";
   checkTransform(ID, Input, Output);
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -152,9 +152,6 @@
   Opts.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults();
   if (ClangTidyOptProvider)
 Opts.ClangTidyOpts = ClangTidyOptProvider->getOptions(File);
-  // FIXME: cache this.
-  Opts.Style =
-  getFormatStyleForFile(File, Contents, FSProvider.getFileSystem().get());
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
   // FIXME: some build systems like Bazel will take time to preparing
   // environment to build the file, it would be nice if we could emit a
@@ -365,8 +362,9 @@
 
 void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
   Callback CB) {
-  auto Action = [Sel](decltype(CB) CB, std::string File, std::string TweakID,
-  Expected InpAST) {
+  auto Action = [Sel, this](decltype(CB) CB, std::string File,
+std::string TweakID,
+Expected InpAST) {
 if (!InpAST)
   return CB(InpAST.takeError());
 auto Selection = tweakSelection(Sel, *InpAST);
@@ -375,7 +373,15 @@
 auto A = prepareTweak(TweakID, *Selection);
 if (!A)
   return CB(A.takeError());
-return CB((*A)->apply(*Selection, InpAST->Inputs.Opts.Style));
+auto RawReplacements = (*A)->apply(*Selection);
+if (!RawReplacements)
+  return CB(RawReplacements.takeError());
+// FIXME: this function has I/O operations (find .clang-format file), figure
+// out a way to cache the format style.
+auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents,
+   InpAST->Inputs.FS.get());
+return CB(
+cleanupAndFormat(InpAST->Inputs.Contents, *RawReplacements, Style));
   };
   WorkScheduler.runWithAST(
   "ApplyTweak", File,
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -308,8 +308,9 @@
   llvm::Optional FixIncludes;
   auto BuildDir = VFS->getCurrentWorkingDirectory();
   if (Opts.SuggestMissingIncludes && Index && !BuildDir.getError()) {
+auto Style = getFormatStyleForFile(MainInput.getFile(), Content, VFS.get());
 auto Inserter = std::make_shared(
-MainInput.getFile(), Content, Opts.Style, BuildDir.get(),
+MainInput.getFile(), Content, Style, BuildDir.get(),
 Clang->getPreprocessor().getHeaderSearchInfo());
 if (Preamble) {
   for (const auto &Inc : Preamble->Includes.MainFileIncludes)
Index: clangd/refactor/tweaks/SwapIfBranches.cpp
===

[clang-tools-extra] r353712 - Revamp the "[clangd] Format tweak's replacements"

[Haojian Wu via cfe-commits]

Author: hokein
Date: Mon Feb 11 07:18:11 2019
New Revision: 353712

URL: http://llvm.org/viewvc/llvm-project?rev=353712&view=rev
Log:
Revamp the "[clangd] Format tweak's replacements"

Summary:
This patch contains two parts:

1) reverts commit r353306.
2) move the format logic out from tweaks, keep tweaks API unchanged.

Reviewers: sammccall, ilya-biryukov

Reviewed By: ilya-biryukov

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

Tags: #clang

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

Added:
clang-tools-extra/trunk/test/clangd/tweaks-format.test
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/Compiler.h
clang-tools-extra/trunk/clangd/refactor/Tweak.cpp
clang-tools-extra/trunk/clangd/refactor/Tweak.h
clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp
clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=353712&r1=353711&r2=353712&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Feb 11 07:18:11 2019
@@ -152,9 +152,6 @@ void ClangdServer::addDocument(PathRef F
   Opts.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults();
   if (ClangTidyOptProvider)
 Opts.ClangTidyOpts = ClangTidyOptProvider->getOptions(File);
-  // FIXME: cache this.
-  Opts.Style =
-  getFormatStyleForFile(File, Contents, FSProvider.getFileSystem().get());
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
   // FIXME: some build systems like Bazel will take time to preparing
   // environment to build the file, it would be nice if we could emit a
@@ -365,8 +362,9 @@ void ClangdServer::enumerateTweaks(PathR
 
 void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
   Callback CB) {
-  auto Action = [Sel](decltype(CB) CB, std::string File, std::string TweakID,
-  Expected InpAST) {
+  auto Action = [Sel, this](decltype(CB) CB, std::string File,
+std::string TweakID,
+Expected InpAST) {
 if (!InpAST)
   return CB(InpAST.takeError());
 auto Selection = tweakSelection(Sel, *InpAST);
@@ -375,7 +373,15 @@ void ClangdServer::applyTweak(PathRef Fi
 auto A = prepareTweak(TweakID, *Selection);
 if (!A)
   return CB(A.takeError());
-return CB((*A)->apply(*Selection, InpAST->Inputs.Opts.Style));
+auto RawReplacements = (*A)->apply(*Selection);
+if (!RawReplacements)
+  return CB(RawReplacements.takeError());
+// FIXME: this function has I/O operations (find .clang-format file), 
figure
+// out a way to cache the format style.
+auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents,
+   InpAST->Inputs.FS.get());
+return CB(
+cleanupAndFormat(InpAST->Inputs.Contents, *RawReplacements, Style));
   };
   WorkScheduler.runWithAST(
   "ApplyTweak", File,

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=353712&r1=353711&r2=353712&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Mon Feb 11 07:18:11 2019
@@ -308,8 +308,9 @@ ParsedAST::build(std::unique_ptr FixIncludes;
   auto BuildDir = VFS->getCurrentWorkingDirectory();
   if (Opts.SuggestMissingIncludes && Index && !BuildDir.getError()) {
+auto Style = getFormatStyleForFile(MainInput.getFile(), Content, 
VFS.get());
 auto Inserter = std::make_shared(
-MainInput.getFile(), Content, Opts.Style, BuildDir.get(),
+MainInput.getFile(), Content, Style, BuildDir.get(),
 Clang->getPreprocessor().getHeaderSearchInfo());
 if (Preamble) {
   for (const auto &Inc : Preamble->Includes.MainFileIncludes)

Modified: clang-tools-extra/trunk/clangd/Compiler.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Compiler.h?rev=353712&r1=353711&r2=353712&view=diff
==
--- clang-tools-extra/trunk/clangd/Compiler.h (original)
+++ clang-tools-extra/trunk/clangd/Compiler.h Mon Feb 11 07:18:11 2019
@@ -17,7 +17,6 @@
 
 #include "../clang-tidy/ClangTidyOptions.h"
 #include "index/Index.h"
-#include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
@@ -39,7 +38,6 @@ public:
 struct ParseOptions {
   t

[PATCH] D57943: [clangd] **Prototype**: clang-tidy-based tweaks

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

> This is an intriguing idea, and is at least useful to prototype new tweaks.
> 
> I'm not sure whether clang-tidy is the ultimately right API to write tweaks:
> 
> it doesn't have the needed constraints to ensure prepare() is fast (e.g. it 
> emits diagnostics and fixes eagerly)
>  the exact set of nodes that it will trigger on may or may not match what we 
> want
>  it doesn't produce context-sensitive titles

Yeah, relying on the checks' implementations is the Achilles' heel of this 
approach. To reuse the existing code (create fast prototypes), we sacrifice our 
flexibility unfortunately :(

Another option would be creating a more general `refactoring` library, and 
refactoring-like clang-tidy and tweaks can be built on it.

> Maybe we should have this behind a flag, and use it to investigate which 
> tweaks should be implemented natively?

It seems to me there are no straight-forward ways to do it since the tweaks' 
implementations are isolated (maybe via a macro `ENABLE_CLANG_TIDY_TWEAKS` to 
control whether we should register these tweaks?).




Comment at: clangd/refactor/tweaks/ClangTidy.cpp:66
+
+bool ClangTidyTweak::prepare(const Selection &Inputs) {
+  if (!Inputs.ASTSelection.commonAncestor())

sammccall wrote:
> obviously this is central to the approach, but... prepare needs to be *fast*. 
> It's hard to see how we guarantee that with all this machinery, especially 
> because the match callback contents may change over time.
Most of the checks' callback are just to detect whether the match result is the 
interesting case, and generate message & FixIt, I'd say it is cheap, and we 
only run clang-tidy check on the selected AST node, which should be fast.



Comment at: clangd/refactor/tweaks/ClangTidy.cpp:83
+  // Run the check on the AST node under the cursor.
+  CTFinder.match(Inputs.ASTSelection.commonAncestor()->ASTNode,
+ Inputs.AST.getASTContext());

sammccall wrote:
> looping over all the ancestors might give better results (but leaves less 
> control over performance).
Besides the performance concern, looping over ancestors would extend the 
clang-tidy check to run on other AST nodes, for the case below, we would also 
apply the tweak to the ast nodes that are not **selected**.

```
typedef int Foo
ty^pedef int Foo2;
```



Comment at: clangd/refactor/tweaks/ClangTidy.cpp:141
+
+  std::string title() const override { return "Covert type to auto"; }
+};

sammccall wrote:
> this should really specify which type is getting converted - seems like a 
> possible weakness of this approach.
Agree,  we may use the check diagnostic messages (e.g. `use auto when declaring 
iterators`) as the title, but this depends on the check implementation, 
probably not an ideal way. 



Comment at: unittests/clangd/TweakTests.cpp:231
+  llvm::StringLiteral Input =
+  "void f() { [[unsigned c = static_cast(1);]] }";
+  llvm::StringLiteral Output =

sammccall wrote:
> this needs to also trigger if you select "unsigned"
this would be nice to have, but it is a weak point of this approach -- relying 
on the check implementation.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57943



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

[PATCH] D57943: [clangd] **Prototype**: clang-tidy-based tweaks

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 186261.
hokein marked 5 inline comments as done.
hokein added a comment.

Update the comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57943

Files:
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/ClangTidy.cpp
  unittests/clangd/TweakTests.cpp

Index: unittests/clangd/TweakTests.cpp
===
--- unittests/clangd/TweakTests.cpp
+++ unittests/clangd/TweakTests.cpp
@@ -185,6 +185,23 @@
   )cpp");
 }
 
+TEST(TweakTest, ConvertTypedef) {
+  llvm::StringLiteral ID = "ConvertTypedef";
+  checkAvailable(ID, R"cpp(^t^y^p^e^d^e^f^ int ^F^o^o;)cpp");
+  llvm::StringLiteral Input = "^typedef int Foo;";
+  llvm::StringLiteral Output = "using Foo = int;";
+  checkTransform(ID, Input, Output);
+}
+
+TEST(TweakTest, UseAuto) {
+  llvm::StringLiteral ID = "UseAuto";
+  llvm::StringLiteral Input =
+  "void f() { [[unsigned c = static_cast(1);]] }";
+  llvm::StringLiteral Output =
+  "void f() { auto c = static_cast(1); }";
+  checkTransform(ID, Input, Output);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/refactor/tweaks/ClangTidy.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/ClangTidy.cpp
@@ -0,0 +1,155 @@
+//===--- ClangTidy.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 "refactor/Tweak.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+
+#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
+#include "../clang-tidy/modernize/UseUsingCheck.h"
+#include "../clang-tidy/modernize/UseAutoCheck.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+bool isInsideMainFile(const SourceLocation Loc, const SourceManager &M) {
+  return Loc.isValid() && M.isWrittenInMainFile(M.getFileLoc(Loc));
+}
+
+struct StoreClangTidyDiags : public DiagnosticConsumer {
+  void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+const clang::Diagnostic &Info) override {
+assert(Info.hasSourceManager() && "Diagnostic must have SourceManager");
+if (Info.getNumFixItHints() > 0)
+  DiagsWithFixIt.push_back(Info);
+  }
+  std::vector DiagsWithFixIt;
+};
+
+/// An **experimental** interface for clang-tidy-based Tweaks, which is used to
+/// prototype new tweaks (without reinventing the wheel).
+/// The tweak will run a clang-tidy check over on an AST node under the cursor.
+/// It is designed for simple refactoring-like clang-tidy checks.
+///
+/// The tweak relies solely on the check's implementation, we don't have much
+/// flexibility:
+///   - only checks whose matcheres match exactly the selected node will work
+///   - every check being added needs to be carefully reviewed to meet the
+/// **fast** constraint of `prepare`
+///   - no context-sensitive titles are supported
+///   - checks don't see any preprocessor events
+///   - no check options are supported yet
+class ClangTidyTweak : public Tweak {
+public:
+  ClangTidyTweak()
+  : DiagsEng(new clang::DiagnosticIDs(), new clang::DiagnosticOptions(),
+ &ClangTidyDiags, /*ShouldOwnClient=*/false) {}
+
+  virtual std::unique_ptr
+  createCheck(tidy::ClangTidyContext *) = 0;
+
+  bool prepare(const Selection &Inputs) override;
+
+  Expected apply(const Selection &Inputs) override;
+
+protected:
+  StoreClangTidyDiags ClangTidyDiags;
+  DiagnosticsEngine DiagsEng; // Extend the lifetime of Diagnostics.
+};
+
+bool ClangTidyTweak::prepare(const Selection &Inputs) {
+  if (!Inputs.ASTSelection.commonAncestor())
+return false;
+  if (Inputs.AST.getASTContext().getDiagnostics().hasErrorOccurred())
+return false;
+  DiagsEng.setSourceManager(&Inputs.AST.getASTContext().getSourceManager());
+  tidy::ClangTidyContext CTContext(
+  llvm::make_unique(
+  tidy::ClangTidyGlobalOptions(),
+  tidy::ClangTidyOptions::getDefaults()));
+  CTContext.setDiagnosticsEngine(&DiagsEng);
+  CTContext.setASTContext(&Inputs.AST.getASTContext());
+
+  auto Check = createCheck(&CTContext);
+  ast_matchers::MatchFinder CTFinder;
+  Check->registerMatchers(&CTFinder);
+  // Run the check on the AST node under the cursor.
+  CTFinder.match(Inputs.ASTSelection.commonAncestor()->ASTNode,
+ Inputs.AST.getASTContext());
+  return !Cla

[PATCH] D54978: Move the SMT API to LLVM

[Brian Rzycki via Phabricator via cfe-commits]

brzycki added a comment.

> @brzycki, I can't reproduce your error. Maybe you're missing 
> `-DLLVM_ENABLE_Z3_SOLVER=OFF`?

Hello @mikhail.ramalho, here are my exact reproduction steps. I just verified 
them about 5 minutes ago.

  # Setup Ubuntu's Z3
  lsb_release -a
  No LSB modules are available.
  Distributor ID: Ubuntu
  Description:Ubuntu 18.04.1 LTS
  Release:18.04
  Codename:   bionic
  
  sudo apt install -y libz3-4 libz3-dev
  
  # Clone llvm-project and checkout the test SHA
  tmpd=$(mktemp -d)
  cd $tmpd
  git clone https://github.com/llvm/llvm-project.git
  git checkout b0a227049fda
  
  # Setup symlinks under llvm to compile correctly
  cd $tmpd/llvm-project/llvm
  ln -s ../../clang
  ln -s ../../polly
  ln -s ../../libcxx
  ln -s ../../libcxxabi
  
  # CMake and Ninja
  mkdir $tmpd/build
  cd $tmpd/build
  cmake -G Ninja -D LLVM_OPTIMIZED_TABLEGEN=ON -D LLVM_ENABLE_Z3_SOLVER=OFF 
../llvm-project/llvm
  ninja -v

These steps produce the error:

  [70/187] Building CXX object 
lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o
  FAILED: lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o
  /usr/bin/c++  -DGTEST_HAS_RTTI=0 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS 
-D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/Support 
-I/work/brzycki/llvm-project/llvm/lib/Support -I/usr/include/libxml2 -Iinclude 
-I/work/brzycki/llvm-project/llvm/include -fPIC -fvisibility-inlines-hidden 
-Werror=date-time -std=c++11 -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic 
-Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized 
-Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color 
-ffunction-sections -fdata-sections -O3 -DNDEBUG-fno-exceptions -fno-rtti 
-MD -MT lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o -MF 
lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o.d -o 
lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o -c 
/work/brzycki/llvm-project/llvm/lib/Support/Z3Solver.cpp
  /work/brzycki/llvm-project/llvm/lib/Support/Z3Solver.cpp: In function ‘void 
{anonymous}::Z3ErrorHandler(Z3_context, Z3_error_code)’:
  /work/brzycki/llvm-project/llvm/lib/Support/Z3Solver.cpp:44:71: error: cannot 
convert ‘Z3_context {aka _Z3_context*}’ to ‘Z3_error_code’ for argument ‘1’ to 
‘const char* Z3_get_error_msg(Z3_error_code)’
  llvm::Twine(Z3_get_error_msg(Context, Error)));
 ^

If I remove `-D LLVM_ENABLE_Z3_SOLVER=OFF` the actual TableGen CMake fails to 
succeed:

  cmake -G Ninja -D LLVM_OPTIMIZED_TABLEGEN=ON ../llvm-project/llvm
  ninja -v

And here's the TableGen CMake error:

  [218/3618] cd /work/brzycki/build/NATIVE && 
/sarc-c/compiler_tmp/tools/build/cmake-3.13.3/bin/cmake -G Ninja 
-DCMAKE_MAKE_PROGRAM="/sarc-c/compiler_tmp/tools/build/ninja-1.8.2/ninja" 
-DCMAKE_C_COMPILER=/usr/bin/cc -DCMAKE_CXX_COMPILER=/usr/bin/c++ 
/work/brzycki/llvm-project/llvm -DLLVM_TARGET_IS_CROSSCOMPILE_HOST=TRUE 
-DLLVM_TARGETS_TO_BUILD="AArch64;AMDGPU;ARM;BPF;Hexagon;Lanai;Mips;MSP430;NVPTX;PowerPC;Sparc;SystemZ;WebAssembly;X86;XCore"
 -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD="" 
-DLLVM_DEFAULT_TARGET_TRIPLE="x86_64-unknown-linux-gnu" 
-DLLVM_TARGET_ARCH="host" -DLLVM_TEMPORARILY_ALLOW_OLD_TOOLCHAIN="OFF" 
-DCMAKE_BUILD_TYPE=Release
  -- The C compiler identification is GNU 7.3.0
  -- The CXX compiler identification is GNU 7.3.0
  -- The ASM compiler identification is GNU
  -- Found assembler: /usr/bin/cc
  -- Check for working C compiler: /usr/bin/cc
  -- Check for working C compiler: /usr/bin/cc -- works
  -- Detecting C compiler ABI info
  -- Detecting C compiler ABI info - done
  -- Detecting C compile features
  -- Detecting C compile features - done
  -- Check for working CXX compiler: /usr/bin/c++
  -- Check for working CXX compiler: /usr/bin/c++ -- works
  -- Detecting CXX compiler ABI info
  -- Detecting CXX compiler ABI info - done
  -- Detecting CXX compile features
  -- Detecting CXX compile features - done
  -- Found Z3: /usr/lib/x86_64-linux-gnu/libz3.so (Required is at least version 
"4.7.1")
  -- Looking for dlfcn.h
  -- Looking for dlfcn.h - found
  -- Looking for errno.h
  -- Looking for errno.h - found
  -- Looking for fcntl.h
  -- Looking for fcntl.h - found
  -- Looking for link.h
  -- Looking for link.h - found
  -- Looking for malloc/malloc.h
  -- Looking for malloc/malloc.h - not found
  -- Looking for pthread.h
  -- Looking for pthread.h - found
  -- Looking for signal.h
  -- Looking for signal.h - found
  -- Looking for sys/ioctl.h
  -- Looking for sys/ioctl.h - found
  -- Looking for sys/mman.h
  -- Looking for sys/mman.h - found
  -- Looking for sys/param.h
  -- Looking for sys/param.h - found
  -- Looking for sys/resource.h
  -- Looking for sys/resource.h - found
  -- Looking for sys/stat.h
  -- Looking for sys/stat.h - found
  -- Looking for sys/time.h
  -- Looking for sys/time.h - fou

[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D57768#1392975 , @bader wrote:

> In D57768#1386941 , @ABataev wrote:
>
> > In D57768#1386933 , @bader wrote:
> >
> > > In D57768#1386924 , @ABataev 
> > > wrote:
> > >
> > > > This definitely requires a test.
> > >
> > >
> > > @ABataev, I tried to find some tests on similar `-fcuda-is-device` and 
> > > `-fopenmp-is-device` options, but I wasn't able to find a dedicated test. 
> > > Could you suggest some examples testing similar functionality, please?
> >
> >
> > There are several similar tests:
> >  OpenMP/driver.c, Driver/openmp-offload.c, Driver/openmp-offload-gpu.c. 
> > There is no absolutely the same test for OpenMP, since OpenMP has mo 
> > similar req for the offloading.
>
>
> @ABataev thanks for the pointers. The uploaded patch adds two options:
>
> - fsycl-is-device (front-end option)
> - sycl-device-only (driver option)
>
>   The driver tests you mention validate driver logic enabled by new options, 
> which is not part of this test and I was going to add it later. I can split 
> the patch and remove new driver option and leave only front-end option. 
> Another option is to add driver logic that invokes the front-end compiler in 
> "device only" mode. Which option do you prefer?


Yes, split the patch into 2. But you still need to add the tests for each of 
them.
You need to add at least 2 tests:

1. The test that checks that the driver option is converted into the frontend 
option (driver with `--sycl-device-only` calls the frontend with 
`-fsycl-is-device`.
2. The test that checks that the frontend option `-fsycl-is-device` adds a new 
macro definition `__SYCL_DEVICE_ONLY__ 1`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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

r353718 - Make test actually test something (colons were missing)

[Nico Weber via cfe-commits]

Author: nico
Date: Mon Feb 11 08:37:02 2019
New Revision: 353718

URL: http://llvm.org/viewvc/llvm-project?rev=353718&view=rev
Log:
Make test actually test something (colons were missing)

Modified:
cfe/trunk/test/CodeGen/ms-x86-intrinsics.c

Modified: cfe/trunk/test/CodeGen/ms-x86-intrinsics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-x86-intrinsics.c?rev=353718&r1=353717&r2=353718&view=diff
==
--- cfe/trunk/test/CodeGen/ms-x86-intrinsics.c (original)
+++ cfe/trunk/test/CodeGen/ms-x86-intrinsics.c Mon Feb 11 08:37:02 2019
@@ -144,28 +144,28 @@ unsigned __int64 test__shiftleft128(unsi
   return __shiftleft128(l, h, d);
 }
 // CHECK-X64-LABEL: define dso_local i64 @test__shiftleft128(i64 %l, i64 %h, 
i8 %d)
-// CHECK-X64  = zext i64 %h to i128
-// CHECK-X64  = shl nuw i128 %0, 64
-// CHECK-X64  = zext i64 %l to i128
-// CHECK-X64  = or i128 %1, %2
-// CHECK-X64  = and i8 %d, 63
-// CHECK-X64  = shl i128 %
-// CHECK-X64  = lshr i128 %
-// CHECK-X64  = trunc i128 %
-// CHECK-X64  ret i64 %
+// CHECK-X64:  = zext i64 %h to i128
+// CHECK-X64:  = shl nuw i128 %0, 64
+// CHECK-X64:  = zext i64 %l to i128
+// CHECK-X64:  = or i128 %1, %2
+// CHECK-X64:  = and i8 %d, 63
+// CHECK-X64:  = shl i128 %
+// CHECK-X64:  = lshr i128 %
+// CHECK-X64:  = trunc i128 %
+// CHECK-X64:  ret i64 %
 
 unsigned __int64 test__shiftright128(unsigned __int64 l, unsigned __int64 h,
  unsigned char d) {
   return __shiftright128(l, h, d);
 }
 // CHECK-X64-LABEL: define dso_local i64 @test__shiftright128(i64 %l, i64 %h, 
i8 %d)
-// CHECK-X64  = zext i64 %h to i128
-// CHECK-X64  = shl nuw i128 %
-// CHECK-X64  = zext i64 %l to i128
-// CHECK-X64  = or i128 %
-// CHECK-X64  = and i8 %d, 63
-// CHECK-X64  = lshr i128 %
-// CHECK-X64  = trunc i128 %
-// CHECK-X64  ret i64 %
+// CHECK-X64:  = zext i64 %h to i128
+// CHECK-X64:  = shl nuw i128 %
+// CHECK-X64:  = zext i64 %l to i128
+// CHECK-X64:  = or i128 %
+// CHECK-X64:  = and i8 %d, 63
+// CHECK-X64:  = lshr i128 %
+// CHECK-X64:  = trunc i128 %
+// CHECK-X64:  ret i64 %
 
 #endif // defined(__x86_64__)


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

Re: r353718 - Make test actually test something (colons were missing)

[Aaron Ballman via cfe-commits]

On Mon, Feb 11, 2019 at 11:36 AM Nico Weber via cfe-commits
 wrote:
>
> Author: nico
> Date: Mon Feb 11 08:37:02 2019
> New Revision: 353718
>
> URL: http://llvm.org/viewvc/llvm-project?rev=353718&view=rev
> Log:
> Make test actually test something (colons were missing)

Good catch! I wonder if we could/should automatically catch this issue
with a clang-tidy check of some kind?

~Aaron

>
> Modified:
> cfe/trunk/test/CodeGen/ms-x86-intrinsics.c
>
> Modified: cfe/trunk/test/CodeGen/ms-x86-intrinsics.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-x86-intrinsics.c?rev=353718&r1=353717&r2=353718&view=diff
> ==
> --- cfe/trunk/test/CodeGen/ms-x86-intrinsics.c (original)
> +++ cfe/trunk/test/CodeGen/ms-x86-intrinsics.c Mon Feb 11 08:37:02 2019
> @@ -144,28 +144,28 @@ unsigned __int64 test__shiftleft128(unsi
>return __shiftleft128(l, h, d);
>  }
>  // CHECK-X64-LABEL: define dso_local i64 @test__shiftleft128(i64 %l, i64 %h, 
> i8 %d)
> -// CHECK-X64  = zext i64 %h to i128
> -// CHECK-X64  = shl nuw i128 %0, 64
> -// CHECK-X64  = zext i64 %l to i128
> -// CHECK-X64  = or i128 %1, %2
> -// CHECK-X64  = and i8 %d, 63
> -// CHECK-X64  = shl i128 %
> -// CHECK-X64  = lshr i128 %
> -// CHECK-X64  = trunc i128 %
> -// CHECK-X64  ret i64 %
> +// CHECK-X64:  = zext i64 %h to i128
> +// CHECK-X64:  = shl nuw i128 %0, 64
> +// CHECK-X64:  = zext i64 %l to i128
> +// CHECK-X64:  = or i128 %1, %2
> +// CHECK-X64:  = and i8 %d, 63
> +// CHECK-X64:  = shl i128 %
> +// CHECK-X64:  = lshr i128 %
> +// CHECK-X64:  = trunc i128 %
> +// CHECK-X64:  ret i64 %
>
>  unsigned __int64 test__shiftright128(unsigned __int64 l, unsigned __int64 h,
>   unsigned char d) {
>return __shiftright128(l, h, d);
>  }
>  // CHECK-X64-LABEL: define dso_local i64 @test__shiftright128(i64 %l, i64 
> %h, i8 %d)
> -// CHECK-X64  = zext i64 %h to i128
> -// CHECK-X64  = shl nuw i128 %
> -// CHECK-X64  = zext i64 %l to i128
> -// CHECK-X64  = or i128 %
> -// CHECK-X64  = and i8 %d, 63
> -// CHECK-X64  = lshr i128 %
> -// CHECK-X64  = trunc i128 %
> -// CHECK-X64  ret i64 %
> +// CHECK-X64:  = zext i64 %h to i128
> +// CHECK-X64:  = shl nuw i128 %
> +// CHECK-X64:  = zext i64 %l to i128
> +// CHECK-X64:  = or i128 %
> +// CHECK-X64:  = and i8 %d, 63
> +// CHECK-X64:  = lshr i128 %
> +// CHECK-X64:  = trunc i128 %
> +// CHECK-X64:  ret i64 %
>
>  #endif // defined(__x86_64__)
>
>
> ___
> 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] D38606: [CodeGen] Emit a helper function for __builtin_os_log_format to reduce code size

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.
Herald added subscribers: llvm-commits, jkorous.
Herald added a project: LLVM.



Comment at: cfe/trunk/test/CodeGenObjC/os_log.m:53
+  // CHECK-O0: %[[V5:.*]] = bitcast %[[V0]]* %[[V3]] to i8*
+  // CHECK-O0-NOT call void (...) @clang.arc.use({{.*}}
+  // CHECK-O0: call void @objc_release(i8* %[[V5]])

Zombie comment: This is missing a ':' and hence doesn't check anything.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D38606



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

[PATCH] D58056: Look through typedefs in getFunctionTypeWithExceptionSpec

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rC Clang

https://reviews.llvm.org/D58056

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/AST/function-alias.cpp


Index: clang/test/AST/function-alias.cpp
===
--- /dev/null
+++ clang/test/AST/function-alias.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only %s
+
+// Verify that ASTContext::getFunctionTypeWithExceptionSpec (called through
+// ASTContext::hasSameFunctionTypeIgnoringExceptionSpec from
+// ExprEvaluatorBase::handleCallExpr in lib/AST/ExprConstant.cpp) does not 
crash
+// for a type alias.
+
+constexpr int f() noexcept { return 0; }
+
+using F = int();
+
+constexpr int g(F * p) { return p(); }
+
+constexpr int n = g(f);
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2771,7 +2771,7 @@
 
   // Anything else must be a function type. Rebuild it with the new exception
   // specification.
-  const auto *Proto = cast(Orig);
+  const auto *Proto = Orig->getAs();
   return getFunctionType(
   Proto->getReturnType(), Proto->getParamTypes(),
   Proto->getExtProtoInfo().withExceptionSpec(ESI));


Index: clang/test/AST/function-alias.cpp
===
--- /dev/null
+++ clang/test/AST/function-alias.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only %s
+
+// Verify that ASTContext::getFunctionTypeWithExceptionSpec (called through
+// ASTContext::hasSameFunctionTypeIgnoringExceptionSpec from
+// ExprEvaluatorBase::handleCallExpr in lib/AST/ExprConstant.cpp) does not crash
+// for a type alias.
+
+constexpr int f() noexcept { return 0; }
+
+using F = int();
+
+constexpr int g(F * p) { return p(); }
+
+constexpr int n = g(f);
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2771,7 +2771,7 @@
 
   // Anything else must be a function type. Rebuild it with the new exception
   // specification.
-  const auto *Proto = cast(Orig);
+  const auto *Proto = Orig->getAs();
   return getFunctionType(
   Proto->getReturnType(), Proto->getParamTypes(),
   Proto->getExtProtoInfo().withExceptionSpec(ESI));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r331745 - [C++2a] Implement operator<=>: Fix another bug in the code gen tests.

[Nico Weber via cfe-commits]

On Tue, May 8, 2018 at 3:59 AM Eric Fiselier via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: ericwf
> Date: Tue May  8 00:56:05 2018
> New Revision: 331745
>
> URL: http://llvm.org/viewvc/llvm-project?rev=331745&view=rev
> Log:
> [C++2a] Implement operator<=>: Fix another bug in the code gen tests.
>
> Sorry for the failures. I'm quite new at writing code gen tests, and
> I'm not sure the best way to make them portable.
>
> Modified:
> cfe/trunk/test/CodeGenCXX/cxx2a-compare.cpp
>
> Modified: cfe/trunk/test/CodeGenCXX/cxx2a-compare.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx2a-compare.cpp?rev=331745&r1=331744&r2=331745&view=diff
>
> ==
> --- cfe/trunk/test/CodeGenCXX/cxx2a-compare.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/cxx2a-compare.cpp Tue May  8 00:56:05 2018
> @@ -79,7 +79,8 @@ auto mem_ptr_test(MemPtrT x, MemPtrT y)
>// CHECK: %cmp.ptr = icmp eq [[TY:i[0-9]+]] %lhs.memptr.ptr,
> %rhs.memptr.ptr
>// CHECK: %cmp.ptr.null = icmp eq [[TY]] %lhs.memptr.ptr, 0
>// CHECK: %cmp.adj = icmp eq [[TY]] %lhs.memptr.adj, %rhs.memptr.adj
> -  // CHECK: %[[OR:.*]] = or i1 %cmp.ptr.null, %cmp.adj
> +  // CHECK: %[[OR:.*]] = or i1
> +  // CHECK-SAME %cmp.adj
>

This is missing the ':' and now doesn't check anything.


>// CHECK: %memptr.eq = and i1 %cmp.ptr, %[[OR]]
>// CHECK: %sel.eq = select i1 %memptr.eq, i8 [[EQ]], i8 [[NE]]
>// CHECK: %__value_ = getelementptr inbounds %[[SE]], %[[SE]]* %[[DEST]]
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://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] D58057: Allow bundle size to be 0 in clang-offload-bundler

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: rjmccall.

HIP uses clang-offload-bundler to create fat binary. The bundle for host is 
empty.
Currently clang-offload-bundler checks if the bundle size is 0 when unbundling.
If so it will exit without unbundling the remaining bundles. This causes
clang-offload-bundler not being able to unbundle fat binaries generated for HIP.

This patch allows bundles size to be 0 when clang-offload-bundler unbundles
input files.


https://reviews.llvm.org/D58057

Files:
  tools/clang-offload-bundler/ClangOffloadBundler.cpp


Index: tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -292,7 +292,7 @@
   ReadChars += TripleSize;
 
   // Check if the offset and size make sense.
-  if (!Size || !Offset || Offset + Size > FC.size())
+  if (!Offset || Offset + Size > FC.size())
 return;
 
   assert(BundlesInfo.find(Triple) == BundlesInfo.end() &&


Index: tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -292,7 +292,7 @@
   ReadChars += TripleSize;
 
   // Check if the offset and size make sense.
-  if (!Size || !Offset || Offset + Size > FC.size())
+  if (!Offset || Offset + Size > FC.size())
 return;
 
   assert(BundlesInfo.find(Triple) == BundlesInfo.end() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58056: Look through typedefs in getFunctionTypeWithExceptionSpec

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

The change itself should probably be uncontroversial (the bad cast had been 
there ever since getFunctionTypeWithExceptionSpec had been introduced with 
r221918), but I'm not sure about the test: It tests the relevant code somewhat 
indirectly; is it fine in clang/test/AST/?; or is such a test even overkill?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58056



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

Re: r353718 - Make test actually test something (colons were missing)

[Roman Lebedev via cfe-commits]

On Mon, Feb 11, 2019 at 7:38 PM Aaron Ballman via cfe-commits
 wrote:
>
> On Mon, Feb 11, 2019 at 11:36 AM Nico Weber via cfe-commits
>  wrote:
> >
> > Author: nico
> > Date: Mon Feb 11 08:37:02 2019
> > New Revision: 353718
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=353718&view=rev
> > Log:
> > Make test actually test something (colons were missing)
>
> Good catch! I wonder if we could/should automatically catch this issue
> with a clang-tidy check of some kind?
Better idea: clang *needs* a tool to generate these check-lines automatically,
much like there is llvm's utils/update_test_checks.py for middle-end IR
and ./utils/update_llc_test_checks.py for back-end.

It's really painful to write these check lines by hand, tests are bad because
it is painful to write them, tests are partial because it is even more painful
to not mistake in the check lines, as it is evident here, etc etc.

> ~Aaron
Roman.

> >
> > Modified:
> > cfe/trunk/test/CodeGen/ms-x86-intrinsics.c
> >
> > Modified: cfe/trunk/test/CodeGen/ms-x86-intrinsics.c
> > URL: 
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-x86-intrinsics.c?rev=353718&r1=353717&r2=353718&view=diff
> > ==
> > --- cfe/trunk/test/CodeGen/ms-x86-intrinsics.c (original)
> > +++ cfe/trunk/test/CodeGen/ms-x86-intrinsics.c Mon Feb 11 08:37:02 2019
> > @@ -144,28 +144,28 @@ unsigned __int64 test__shiftleft128(unsi
> >return __shiftleft128(l, h, d);
> >  }
> >  // CHECK-X64-LABEL: define dso_local i64 @test__shiftleft128(i64 %l, i64 
> > %h, i8 %d)
> > -// CHECK-X64  = zext i64 %h to i128
> > -// CHECK-X64  = shl nuw i128 %0, 64
> > -// CHECK-X64  = zext i64 %l to i128
> > -// CHECK-X64  = or i128 %1, %2
> > -// CHECK-X64  = and i8 %d, 63
> > -// CHECK-X64  = shl i128 %
> > -// CHECK-X64  = lshr i128 %
> > -// CHECK-X64  = trunc i128 %
> > -// CHECK-X64  ret i64 %
> > +// CHECK-X64:  = zext i64 %h to i128
> > +// CHECK-X64:  = shl nuw i128 %0, 64
> > +// CHECK-X64:  = zext i64 %l to i128
> > +// CHECK-X64:  = or i128 %1, %2
> > +// CHECK-X64:  = and i8 %d, 63
> > +// CHECK-X64:  = shl i128 %
> > +// CHECK-X64:  = lshr i128 %
> > +// CHECK-X64:  = trunc i128 %
> > +// CHECK-X64:  ret i64 %
> >
> >  unsigned __int64 test__shiftright128(unsigned __int64 l, unsigned __int64 
> > h,
> >   unsigned char d) {
> >return __shiftright128(l, h, d);
> >  }
> >  // CHECK-X64-LABEL: define dso_local i64 @test__shiftright128(i64 %l, i64 
> > %h, i8 %d)
> > -// CHECK-X64  = zext i64 %h to i128
> > -// CHECK-X64  = shl nuw i128 %
> > -// CHECK-X64  = zext i64 %l to i128
> > -// CHECK-X64  = or i128 %
> > -// CHECK-X64  = and i8 %d, 63
> > -// CHECK-X64  = lshr i128 %
> > -// CHECK-X64  = trunc i128 %
> > -// CHECK-X64  ret i64 %
> > +// CHECK-X64:  = zext i64 %h to i128
> > +// CHECK-X64:  = shl nuw i128 %
> > +// CHECK-X64:  = zext i64 %l to i128
> > +// CHECK-X64:  = or i128 %
> > +// CHECK-X64:  = and i8 %d, 63
> > +// CHECK-X64:  = lshr i128 %
> > +// CHECK-X64:  = trunc i128 %
> > +// CHECK-X64:  ret i64 %
> >
> >  #endif // defined(__x86_64__)
> >
> >
> > ___
> > 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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57736: [WebAssembly] Bulk memory intrinsics and builtins

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added inline comments.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.



Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:29
+// Bulk memory builtins
+TARGET_BUILTIN(__builtin_wasm_memory_init, "vIiv*ii", "", "bulk-memory")
+TARGET_BUILTIN(__builtin_wasm_data_drop, "vIi", "", "bulk-memory")

- When do we use `Ii` instead of `i`?
- Shouldn't we add the memory index field as well, even if that means a user 
always has to set it to 0? Are we gonna add a new builtin once multiple 
memories are available?
- Shouldn't the segment index, segment offset, and the size operands be `Ui` 
because they cannot be negative?



Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:30
+TARGET_BUILTIN(__builtin_wasm_memory_init, "vIiv*ii", "", "bulk-memory")
+TARGET_BUILTIN(__builtin_wasm_data_drop, "vIi", "", "bulk-memory")
+

The same thing..
- Shouldn't the segment index be `Ui`?
- Shouldn't we add the memory index field as well?




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13477
+if (!SegArg->isIntegerConstantExpr(SegConst, getContext()))
+  llvm_unreachable("Constant arg isn't actually constant?");
+llvm::Type *SegType = ConvertType(SegArg->getType());

Not sure if we can use `llvm_unreachable` here, because we can certainly reach 
here if a user uses this builtin with a non-const variable. In this file people 
often just used `assert` for user errors, which is not ideal either.

I haven't used it myself, but looking at the code, the recommended way to 
signal an error looks like to use [[ 
https://github.com/llvm/llvm-project/blob/db7fbcb038f095622a3e6847ecd6ff80bdc2483a/clang/lib/CodeGen/CodeGenFunction.h#L2092-L2094
 | `CodeGenFunction::ErrorUnsupported` ]] function, as in [[ 
https://github.com/llvm/llvm-project/blob/0e04ebdcda44ef90e25abd0a2e65cc755ae8bc37/clang/lib/CodeGen/CGBuiltin.cpp#L2458-L2460
 | here ]]. We used `llvm_unreachable` for SIMD builtins too, but maybe we can 
fix it later.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13488
+Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_memory_init,
+{SegType, IdxType, DstType});
+return Builder.CreateCall(Callee, Args);

Do we need to pass types here to make intrinsic names overloaded like 
`llvm.wasm.memory.init.i32.i32.i8`, unless this intrinsic also support operands 
of other types, which it doesn't? The same for `data.drop` builtin.



Comment at: llvm/include/llvm/IR/IntrinsicsWebAssembly.td:119
+  Intrinsic<[],
+[llvm_anyint_ty, llvm_anyint_ty, LLVMPointerType,
+ llvm_i32_ty, llvm_i32_ty],

- Why are the first two immediates anyint? The spec says the segment index is 
varuint32 (so that will be `llvm_i32_ty` here), and for the memory index I 
don't think we are ever gonna need something larger than i32 either, but maybe 
it is better to be clarified in the spec too though.
- For the pointer type, I guess `llvm_ptr_ty` will be sufficient, unless we 
have multiple overloaded intrinsics of the same name.



Comment at: llvm/include/llvm/IR/IntrinsicsWebAssembly.td:121
+ llvm_i32_ty, llvm_i32_ty],
+[IntrWriteMem, IntrArgMemOnly, WriteOnly<1>],
+"", [SDNPMemOperand]>;

- Isn't the pointer argument number not 1 but 2?
- I guess this should have `IntrHasSideEffects` as well, as in `data.drop`?
- I don't know much how they are handled differently in compilation, but 
because we can access some other memory, which holds the segment part, during 
execution, how about `IntrInaccessibleMemOrArgMemOnly` instead of 
`IntrArgMemOnly`?



Comment at: llvm/include/llvm/IR/IntrinsicsWebAssembly.td:122
+[IntrWriteMem, IntrArgMemOnly, WriteOnly<1>],
+"", [SDNPMemOperand]>;
+def int_wasm_data_drop :

I told you we needed this, but on second thought, because we don't really use 
`MachineMemOperand`, maybe we don't need it..? :$ The [[ 
https://github.com/llvm/llvm-project/blob/dc2c93017f8bf2a2c10b8e024f8f4a6409db/llvm/include/llvm/IR/Intrinsics.td#L483-L496
 | standard memcpy/memmove/memset intrinsics ]] don't have it either. So if the 
code runs ok without these I think we can delete it. The same for `data.drop` 
intrinsic. Sorry for the incorrect info and confusion.



Comment at: llvm/include/llvm/IR/IntrinsicsWebAssembly.td:125
+  Intrinsic<[],
+[llvm_anyint_ty],
+[IntrNoDuplicate, IntrHasSideEffects],

The same, `llvm_i32_ty`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57736



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

[PATCH] D58060: Fix diagnostic for addr spaces in static_cast

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added a reviewer: rjmccall.
Herald added a subscriber: ebevhan.

When we diagnose `static_cast` we should prevent accidental address space 
conversions unless the conversion is safe (i.e. converting to an address space 
that is a super set is safe!).

Some more details are explained in this RFC: 
http://lists.llvm.org/pipermail/cfe-dev/2018-December/060546.html

This patch also changes diagnostic to use Qualifiers print method.


https://reviews.llvm.org/D58060

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaInit.cpp
  test/SemaCXX/err_reference_bind_drops_quals.cpp
  test/SemaOpenCLCXX/address-space-castoperators.cpp

Index: test/SemaOpenCLCXX/address-space-castoperators.cpp
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-castoperators.cpp
@@ -0,0 +1,17 @@
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify
+
+void test_ref(int &gen, __global const int& glob){
+static_cast<__global int&>(gen); // expected-error{{binding value of type '__generic int' to reference to type '__global int' changes address space}}
+static_cast<__global const int&>(gen); //expected-error{{binding value of type '__generic int' to reference to type 'const __global int' changes address space}}
+static_cast<__global int&>(glob); //expected-error{{binding value of type 'const __global int' to reference to type '__global int' drops 'const' qualifier}}
+static_cast<__local int&>(glob); //expected-error{{binding value of type 'const __global int' to reference to type '__local int' changes address space}}
+static_cast<__generic const int&>(glob); //expected-warning{{expression result unused}}
+}
+
+void test_ptr(int *gen, __global const int* glob){
+ static_cast<__global int*>(gen); // expected-error{{static_cast from '__generic int *' to '__global int *' is not allowed}}
+ static_cast<__global const int*>(gen); //expected-error{{static_cast from '__generic int *' to 'const __global int *' is not allowed}}
+ static_cast<__global int*>(glob); //expected-error{{static_cast from 'const __global int *' to '__global int *' is not allowed}}
+ static_cast<__local int*>(glob); //expected-error{{static_cast from 'const __global int *' to '__local int *' is not allowed}}
+ static_cast<__generic const int*>(glob); //expected-warning{{expression result unused}}
+}
Index: test/SemaCXX/err_reference_bind_drops_quals.cpp
===
--- test/SemaCXX/err_reference_bind_drops_quals.cpp
+++ test/SemaCXX/err_reference_bind_drops_quals.cpp
@@ -7,30 +7,30 @@
const restrict volatile ptr crvp) {
   ptr& p1 = p;
   ptr& p2 = cp; // expected-error {{drops 'const' qualifier}}
-  ptr& p3 = rp; // expected-error {{drops 'restrict' qualifier}}
-  ptr& p4 = crp; // expected-error {{drops 'const' and 'restrict' qualifiers}}
+  ptr& p3 = rp; // expected-error {{drops '__restrict' qualifier}}
+  ptr& p4 = crp; // expected-error {{drops 'const __restrict' qualifiers}}
   ptr& p5 = vp; // expected-error {{drops 'volatile' qualifier}}
-  ptr& p6 = cvp; // expected-error {{drops 'const' and 'volatile' qualifiers}}
-  ptr& p7 = rvp; // expected-error {{drops 'restrict' and 'volatile' qualifiers}}
-  ptr& p8 = crvp; // expected-error {{drops 'const', 'restrict', and 'volatile' qualifiers}}
+  ptr& p6 = cvp; // expected-error {{drops 'const volatile' qualifiers}}
+  ptr& p7 = rvp; // expected-error {{drops 'volatile __restrict' qualifiers}}
+  ptr& p8 = crvp; // expected-error {{drops 'const volatile __restrict' qualifiers}}
 
   const ptr& cp1 = p;
   const ptr& cp2 = cp;
-  const ptr& cp3 = rp; // expected-error {{drops 'restrict' qualifier}}
-  const ptr& cp4 = crp; // expected-error {{drops 'restrict' qualifier}}
+  const ptr& cp3 = rp; // expected-error {{drops '__restrict' qualifier}}
+  const ptr& cp4 = crp; // expected-error {{drops '__restrict' qualifier}}
   const ptr& cp5 = vp; // expected-error {{drops 'volatile' qualifier}}
   const ptr& cp6 = cvp; // expected-error {{drops 'volatile' qualifier}}
-  const ptr& cp7 = rvp; // expected-error {{drops 'restrict' and 'volatile' qualifiers}}
-  const ptr& cp8 = crvp; // expected-error {{drops 'restrict' and 'volatile' qualifiers}}
+  const ptr& cp7 = rvp; // expected-error {{drops 'volatile __restrict' qualifiers}}
+  const ptr& cp8 = crvp; // expected-error {{drops 'volatile __restrict' qualifiers}}
 
   const volatile ptr& cvp1 = p;
   const volatile ptr& cvp2 = cp;
-  const volatile ptr& cvp3 = rp; // expected-error {{drops 'restrict' qualifier}}
-  const volatile ptr& cvp4 = crp; // expected-error {{drops 'restrict' qualifier}}
+  const volatile ptr& cvp3 = rp; // expected-error {{drops '__restrict' qualifier}}
+  const volatile ptr& cvp4 = crp; // expected-error {{drops '__restrict' qualifier}}
   const volatile ptr& cvp5 = vp;
   const volatile ptr& cvp6 = cvp;
-  const volatile ptr& cvp7

[PATCH] D58061: Fix a few tests that were missing ':' on CHECK lines and weren't testing anything.

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: rjmccall.
Herald added subscribers: kristof.beyls, javed.absar.

Found by `git grep '\/\/ CHECK-[^: ]* ' clang/test/ | grep -v RUN:`.

Also tweak CodeGenCXX/arm-swiftcall.cpp to still pass now that it checks more.


https://reviews.llvm.org/D58061

Files:
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGenCXX/arm-swiftcall.cpp
  clang/test/CodeGenCXX/cxx1z-init-statement.cpp
  clang/test/CodeGenCXX/cxx2a-compare.cpp
  clang/test/CodeGenCXX/debug-info-inheriting-constructor.cpp
  clang/test/CodeGenCXX/vtable-layout.cpp
  clang/test/CodeGenObjC/gnu-init.m
  clang/test/CodeGenObjC/os_log.m

Index: clang/test/CodeGenObjC/os_log.m
===
--- clang/test/CodeGenObjC/os_log.m
+++ clang/test/CodeGenObjC/os_log.m
@@ -50,7 +50,7 @@
   // CHECK-O0: %[[V4:.*]] = ptrtoint %[[TY0]]* %[[V3]] to i64
   // CHECK-O0: call void @__os_log_helper_1_2_1_8_64(i8* %[[V0]], i64 %[[V4]])
   // CHECK-O0: %[[V5:.*]] = bitcast %[[TY0]]* %[[V3]] to i8*
-  // CHECK-O0-NOT call void (...) @llvm.objc.clang.arc.use({{.*}}
+  // CHECK-O0-NOT: call void (...) @llvm.objc.clang.arc.use({{.*}}
   // CHECK-O0: call void @llvm.objc.release(i8* %[[V5]])
   // CHECK-O0: ret i8* %[[V0]]
 }
Index: clang/test/CodeGenObjC/gnu-init.m
===
--- clang/test/CodeGenObjC/gnu-init.m
+++ clang/test/CodeGenObjC/gnu-init.m
@@ -14,7 +14,7 @@
 
 // Check that we get a class ref to the defined class.
 // CHECK-NEW: @._OBJC_INIT_CLASS_X = global 
-// CHECK-NEW-SAME* @._OBJC_CLASS_X, section "__objc_classes"
+// CHECK-NEW-SAME: @._OBJC_CLASS_X, section "__objc_classes"
 
 // Check that we emit the section start and end symbols as hidden globals.
 // CHECK-NEW: @__start___objc_selectors = external hidden global i8*
Index: clang/test/CodeGenCXX/vtable-layout.cpp
===
--- clang/test/CodeGenCXX/vtable-layout.cpp
+++ clang/test/CodeGenCXX/vtable-layout.cpp
@@ -1412,7 +1412,7 @@
 // CHECK-35-NEXT:   13 | void Test28::B::b()
 //
 // CHECK-35:  VTable indices for 'Test28::E' (1 entries).
-// CHECK-35-NEXT :   0 | void Test28::E::e()
+// CHECK-35-NEXT:0 | void Test28::E::e()
 
 // CHECK-35:  Construction vtable for ('Test28::D', 0) in 'Test28::E' (13 entries).
 // CHECK-35-NEXT:0 | vbase_offset (8)
Index: clang/test/CodeGenCXX/debug-info-inheriting-constructor.cpp
===
--- clang/test/CodeGenCXX/debug-info-inheriting-constructor.cpp
+++ clang/test/CodeGenCXX/debug-info-inheriting-constructor.cpp
@@ -9,9 +9,9 @@
 
 A::A(int i, ...) {}
 // CHECK: define void @{{.*}}foo
-// CHECK-NOT ret void
+// CHECK-NOT: ret void
 // CHECK: call void @llvm.dbg.declare
-// CHECK-NOT ret void
+// CHECK-NOT: ret void
 // CHECK: call void @llvm.dbg.declare(metadata %{{.*}}** %{{[^,]+}},
 // CHECK-SAME: metadata ![[THIS:[0-9]+]], metadata !DIExpression()), !dbg ![[LOC:[0-9]+]]
 // CHECK: ret void, !dbg ![[NOINL:[0-9]+]]
Index: clang/test/CodeGenCXX/cxx2a-compare.cpp
===
--- clang/test/CodeGenCXX/cxx2a-compare.cpp
+++ clang/test/CodeGenCXX/cxx2a-compare.cpp
@@ -80,7 +80,7 @@
   // CHECK: %cmp.ptr.null = icmp eq [[TY]] %lhs.memptr.ptr, 0
   // CHECK: %cmp.adj = icmp eq [[TY]] %lhs.memptr.adj, %rhs.memptr.adj
   // CHECK: %[[OR:.*]] = or i1
-  // CHECK-SAME %cmp.adj
+  // CHECK-SAME: %cmp.adj
   // CHECK: %memptr.eq = and i1 %cmp.ptr, %[[OR]]
   // CHECK: %sel.eq = select i1 %memptr.eq, i8 [[EQ]], i8 [[NE]]
   // CHECK: %__value_ = getelementptr inbounds %[[SE]], %[[SE]]* %[[DEST]]
Index: clang/test/CodeGenCXX/cxx1z-init-statement.cpp
===
--- clang/test/CodeGenCXX/cxx1z-init-statement.cpp
+++ clang/test/CodeGenCXX/cxx1z-init-statement.cpp
@@ -5,7 +5,7 @@
   // CHECK:  %[[A:.*]] = alloca i32, align 4
   // CHECK-NEXT: store i32 5, i32* %[[A]], align 4
   // CHECK-NEXT: %[[B:.*]] = load i32, i32* %[[A]], align 4
-  // CHECK-NEXT  %[[C:.*]] = icmp slt i32 %[[B]], 8
+  // CHECK-NEXT: %[[C:.*]] = icmp slt i32 %[[B]], 8
   if (int a = 5; a < 8)
 ;
 }
Index: clang/test/CodeGenCXX/arm-swiftcall.cpp
===
--- clang/test/CodeGenCXX/arm-swiftcall.cpp
+++ clang/test/CodeGenCXX/arm-swiftcall.cpp
@@ -120,6 +120,6 @@
 class struct_trivial {
   int x;
 };
-// CHECK-LABEL define void @test_struct_trivial(i32{{( %.*)?}})
+// CHECK-LABEL: define swiftcc void @test_struct_trivial(i32{{( %.*)?}})
 extern "C" SWIFTCALL
 void test_struct_trivial(struct_trivial triv) {}
Index: clang/test/AST/ast-dump-attr.cpp
===
--- clang/test/AST/ast-dump-attr.cpp
+++ clang/test/AST/ast-dump-attr.cpp
@@ -197,7 +197,7 @@
   [[gsl::suppress("on-decl")]]
   void TestSu

[PATCH] D45978: dllexport const variables must have external linkage.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D45978#1392855 , @aaron.ballman 
wrote:

> It looks like the patch got mucked up somehow, I only see three testing files 
> in the patch now?


Oops! Sorry about that.


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

https://reviews.llvm.org/D45978



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

[PATCH] D45978: dllexport const variables must have external linkage.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 186275.
Herald added a subscriber: mstorsjo.

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

https://reviews.llvm.org/D45978

Files:
  lib/Sema/SemaDecl.cpp
  test/Sema/dllexport-1.c
  test/Sema/dllexport-1.cpp
  test/Sema/dllexport-2.cpp


Index: test/Sema/dllexport-2.cpp
===
--- /dev/null
+++ test/Sema/dllexport-2.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple i686-win32 -fsyntax-only -fms-extensions -verify %s
+// RUN: %clang_cc1 -triple x86_64-win32 -fsyntax-only -fms-extensions -verify 
-std=c++11 %s
+
+// Export const variable.
+
+__declspec(dllexport) int const j; // expected-error {{default initialization 
of an object of const type 'const int'}} // expected-error {{'j' must have 
external linkage when declared 'dllexport'}}
Index: test/Sema/dllexport-1.cpp
===
--- /dev/null
+++ test/Sema/dllexport-1.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple i686-win32 -fsyntax-only -fms-extensions -verify %s
+// RUN: %clang_cc1 -triple x86_64-win32 -fsyntax-only -fms-extensions -verify 
-std=c++11 %s
+
+// CHECK: @"?x@@3HB" = dso_local dllexport constant i32 3, align 4
+
+// Export const variable initialization.
+
+// expected-no-diagnostics
+__declspec(dllexport) int const x = 3;
Index: test/Sema/dllexport-1.c
===
--- /dev/null
+++ test/Sema/dllexport-1.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple i686-win32 -emit-llvm -fms-extensions -std=c99 < 
%s| FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-win32 -emit-llvm -fms-extensions -std=c11 < 
%s | FileCheck %s
+// RUN: %clang_cc1 -triple i686-mingw32 -emit-llvm -fms-extensions -std=c11 < 
%s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-mingw32 -emit-llvm -fms-extensions -std=c11 
< %s | FileCheck %s
+
+// Export const variable.
+
+// CHECK: @y = common dso_local dllexport global i32 0, align 4
+
+__declspec(dllexport) int const x = 3;
+__declspec(dllexport) const int y;
+extern int const z = 4; // expected-warning{{'extern' variable has an 
initializer}}
+
+int main() {
+  int a = x + y + z;
+  return a;
+}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11367,6 +11367,16 @@
 !isTemplateInstantiation(VDecl->getTemplateSpecializationKind()))
   Diag(VDecl->getLocation(), diag::warn_extern_init);
 
+// In Microsoft C++ mode, a const variable defined in namespace scope has
+// external linkage by default if the variable is declared with
+// __declspec(dllexport).
+if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+getLangOpts().CPlusPlus &&
+VDecl->getType().isLocalConstQualified() &&
+VDecl->hasAttr() &&
+VDecl->getDefinition())
+  VDecl->setStorageClass(SC_Extern);
+
 // C99 6.7.8p4. All file scoped initializers need to be constant.
 if (!getLangOpts().CPlusPlus && !VDecl->isInvalidDecl())
   CheckForConstantInitializer(Init, DclT);


Index: test/Sema/dllexport-2.cpp
===
--- /dev/null
+++ test/Sema/dllexport-2.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -triple i686-win32 -fsyntax-only -fms-extensions -verify %s
+// RUN: %clang_cc1 -triple x86_64-win32 -fsyntax-only -fms-extensions -verify -std=c++11 %s
+
+// Export const variable.
+
+__declspec(dllexport) int const j; // expected-error {{default initialization of an object of const type 'const int'}} // expected-error {{'j' must have external linkage when declared 'dllexport'}}
Index: test/Sema/dllexport-1.cpp
===
--- /dev/null
+++ test/Sema/dllexport-1.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple i686-win32 -fsyntax-only -fms-extensions -verify %s
+// RUN: %clang_cc1 -triple x86_64-win32 -fsyntax-only -fms-extensions -verify -std=c++11 %s
+
+// CHECK: @"?x@@3HB" = dso_local dllexport constant i32 3, align 4
+
+// Export const variable initialization.
+
+// expected-no-diagnostics
+__declspec(dllexport) int const x = 3;
Index: test/Sema/dllexport-1.c
===
--- /dev/null
+++ test/Sema/dllexport-1.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple i686-win32 -emit-llvm -fms-extensions -std=c99 < %s| FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-win32 -emit-llvm -fms-extensions -std=c11 < %s | FileCheck %s
+// RUN: %clang_cc1 -triple i686-mingw32 -emit-llvm -fms-extensions -std=c11 < %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-mingw32 -emit-llvm -fms-extensions -std=c11 < %s | FileCheck %s
+
+// Export const variable.
+
+// CHECK: @y = common dso_local dllexport global i32 0, align 4
+
+__declspec(dllexport) int const x = 3;
+__declspec(dllexport) const int y;
+extern int const z =

[PATCH] D58062: Support framework import/include auto-completion

[David Goldman via Phabricator via cfe-commits]

dgoldman created this revision.
dgoldman added a reviewer: sammccall.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Frameworks filesystem representations:

  UIKit.framework/Headers/%header%

Framework import format:

  #import 

Thus the completion code must map the input format of  to
the path of UIKit.framework/Headers as well as strip the
".framework" suffix when auto-completing the framework name.


Repository:
  rC Clang

https://reviews.llvm.org/D58062

Files:
  lib/Sema/SemaCodeComplete.cpp


Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -8371,10 +8371,31 @@
   };
 
   // Helper: scans IncludeDir for nice files, and adds results for each.
-  auto AddFilesFromIncludeDir = [&](StringRef IncludeDir, bool IsSystem) {
+  auto AddFilesFromIncludeDir = [&](StringRef IncludeDir, bool IsSystem,
+bool isFramework) {
+bool stripFrameworkSuffix = false;
 llvm::SmallString<128> Dir = IncludeDir;
-if (!NativeRelDir.empty())
+if (isFramework) {
+  if (NativeRelDir.empty()) {
+stripFrameworkSuffix = true;
+  } else {
+auto Begin = llvm::sys::path::begin(NativeRelDir);
+auto End = llvm::sys::path::end(NativeRelDir);
+
+if (Begin != End) {
+  SmallString<32> Framework = *Begin;
+  Framework += ".framework";
+  llvm::sys::path::append(Dir, Framework, "Headers");
+  llvm::sys::path::append(Dir, ++Begin, End);
+} else {
+  SmallString<32> Framework(NativeRelDir);
+  Framework += ".framework";
+  llvm::sys::path::append(Dir, Framework, "Headers");
+}
+  }
+} else if (!NativeRelDir.empty()) {
   llvm::sys::path::append(Dir, NativeRelDir);
+}
 
 std::error_code EC;
 unsigned Count = 0;
@@ -8385,7 +8406,12 @@
   StringRef Filename = llvm::sys::path::filename(It->path());
   switch (It->type()) {
   case llvm::sys::fs::file_type::directory_file:
-AddCompletion(Filename, /*IsDirectory=*/true);
+if (stripFrameworkSuffix && Filename.endswith(".framework")) {
+  auto FrameworkName = Filename.substr(0, Filename.size() - 10);
+  AddCompletion(FrameworkName, /*IsDirectory=*/true);
+} else {
+  AddCompletion(Filename, /*IsDirectory=*/true);
+}
 break;
   case llvm::sys::fs::file_type::regular_file:
 // Only files that really look like headers. (Except in system dirs).
@@ -8413,10 +8439,12 @@
   // header maps are not (currently) enumerable.
   break;
 case DirectoryLookup::LT_NormalDir:
-  AddFilesFromIncludeDir(IncludeDir.getDir()->getName(), IsSystem);
+  AddFilesFromIncludeDir(IncludeDir.getDir()->getName(), IsSystem,
+ /*IsFramework*/false);
   break;
 case DirectoryLookup::LT_Framework:
-  AddFilesFromIncludeDir(IncludeDir.getFrameworkDir()->getName(), 
IsSystem);
+  AddFilesFromIncludeDir(IncludeDir.getFrameworkDir()->getName(), IsSystem,
+ /*IsFramework*/true);
   break;
 }
   };
@@ -8430,7 +8458,7 @@
 // The current directory is on the include path for "quoted" includes.
 auto *CurFile = PP.getCurrentFileLexer()->getFileEntry();
 if (CurFile && CurFile->getDir())
-  AddFilesFromIncludeDir(CurFile->getDir()->getName(), false);
+  AddFilesFromIncludeDir(CurFile->getDir()->getName(), false, false);
 for (const auto &D : make_range(S.quoted_dir_begin(), S.quoted_dir_end()))
   AddFilesFromDirLookup(D, false);
   }


Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -8371,10 +8371,31 @@
   };
 
   // Helper: scans IncludeDir for nice files, and adds results for each.
-  auto AddFilesFromIncludeDir = [&](StringRef IncludeDir, bool IsSystem) {
+  auto AddFilesFromIncludeDir = [&](StringRef IncludeDir, bool IsSystem,
+bool isFramework) {
+bool stripFrameworkSuffix = false;
 llvm::SmallString<128> Dir = IncludeDir;
-if (!NativeRelDir.empty())
+if (isFramework) {
+  if (NativeRelDir.empty()) {
+stripFrameworkSuffix = true;
+  } else {
+auto Begin = llvm::sys::path::begin(NativeRelDir);
+auto End = llvm::sys::path::end(NativeRelDir);
+
+if (Begin != End) {
+  SmallString<32> Framework = *Begin;
+  Framework += ".framework";
+  llvm::sys::path::append(Dir, Framework, "Headers");
+  llvm::sys::path::append(Dir, ++Begin, End);
+} else {
+  SmallString<32> Framework(NativeRelDir);
+  Framework += ".framework";
+  llvm::sys::path::append(Dir, Framework, "Headers");
+}
+  }
+} else if (!NativeRelDir

[PATCH] D57230: [analyzer] Toning down invalidation a bit

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

I think I might have a theory, but I would like to discuss it as I am not  
familiar with the internals  bindings.

My theory is the following: when we store the bindings, we store them in a map 
where the key is a base region. 
So when we try to look the bindings up with a non-base region, we will not get 
any bindings.

So in our current case, we end up having a non-base region in the worklist of 
`InvalidateRegionsWorker`. 
`ClusterAnalysis::RunWorkList` will look up the cluster for the non-base region.
Without a cluster found we will not visit the bindings. With not visiting the 
bindings, we will not invalidate the symbols.
With no symbols to invalidate, the checkers will not get notified.

I think the whole `ClusterAnalysis` is flawed at this point. Most of the code 
expects to only see base regions, but some code paths might end up adding 
non-base regions.

So the question is, what should be the proper way to handle the 
`TK_DoNotInvalidateSuperRegion` trait?
Maybe we should always look up the bindings using the base region. But if we 
do, should we actually visit all of the bindings?

I did not have time yet to play with the possible solutions and will come back 
to this problem soon, just wanted to write down what I got so far.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57230



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

Re: r353718 - Make test actually test something (colons were missing)

[Nico Weber via cfe-commits]

On Mon, Feb 11, 2019 at 11:38 AM Aaron Ballman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Mon, Feb 11, 2019 at 11:36 AM Nico Weber via cfe-commits
>  wrote:
> >
> > Author: nico
> > Date: Mon Feb 11 08:37:02 2019
> > New Revision: 353718
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=353718&view=rev
> > Log:
> > Make test actually test something (colons were missing)
>
> Good catch! I wonder if we could/should automatically catch this issue
> with a clang-tidy check of some kind?
>

I sent https://reviews.llvm.org/D58061 to clean up other instances I could
fine. Coming up with that regex took a few iterations.

I thin the Best Fix is probably to make FileCheck diag if a line starts
with (after a few whitelisted comment chars like // and #) a check-prefix
but then isn't followed by : (maybe after -NOT, -SAME, -LABEL etc).

Looks like this specific change here doesn't work on a bot even though it
worked locally, looking at that now, sigh.


>
> ~Aaron
>
> >
> > Modified:
> > cfe/trunk/test/CodeGen/ms-x86-intrinsics.c
> >
> > Modified: cfe/trunk/test/CodeGen/ms-x86-intrinsics.c
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-x86-intrinsics.c?rev=353718&r1=353717&r2=353718&view=diff
> >
> ==
> > --- cfe/trunk/test/CodeGen/ms-x86-intrinsics.c (original)
> > +++ cfe/trunk/test/CodeGen/ms-x86-intrinsics.c Mon Feb 11 08:37:02 2019
> > @@ -144,28 +144,28 @@ unsigned __int64 test__shiftleft128(unsi
> >return __shiftleft128(l, h, d);
> >  }
> >  // CHECK-X64-LABEL: define dso_local i64 @test__shiftleft128(i64 %l,
> i64 %h, i8 %d)
> > -// CHECK-X64  = zext i64 %h to i128
> > -// CHECK-X64  = shl nuw i128 %0, 64
> > -// CHECK-X64  = zext i64 %l to i128
> > -// CHECK-X64  = or i128 %1, %2
> > -// CHECK-X64  = and i8 %d, 63
> > -// CHECK-X64  = shl i128 %
> > -// CHECK-X64  = lshr i128 %
> > -// CHECK-X64  = trunc i128 %
> > -// CHECK-X64  ret i64 %
> > +// CHECK-X64:  = zext i64 %h to i128
> > +// CHECK-X64:  = shl nuw i128 %0, 64
> > +// CHECK-X64:  = zext i64 %l to i128
> > +// CHECK-X64:  = or i128 %1, %2
> > +// CHECK-X64:  = and i8 %d, 63
> > +// CHECK-X64:  = shl i128 %
> > +// CHECK-X64:  = lshr i128 %
> > +// CHECK-X64:  = trunc i128 %
> > +// CHECK-X64:  ret i64 %
> >
> >  unsigned __int64 test__shiftright128(unsigned __int64 l, unsigned
> __int64 h,
> >   unsigned char d) {
> >return __shiftright128(l, h, d);
> >  }
> >  // CHECK-X64-LABEL: define dso_local i64 @test__shiftright128(i64 %l,
> i64 %h, i8 %d)
> > -// CHECK-X64  = zext i64 %h to i128
> > -// CHECK-X64  = shl nuw i128 %
> > -// CHECK-X64  = zext i64 %l to i128
> > -// CHECK-X64  = or i128 %
> > -// CHECK-X64  = and i8 %d, 63
> > -// CHECK-X64  = lshr i128 %
> > -// CHECK-X64  = trunc i128 %
> > -// CHECK-X64  ret i64 %
> > +// CHECK-X64:  = zext i64 %h to i128
> > +// CHECK-X64:  = shl nuw i128 %
> > +// CHECK-X64:  = zext i64 %l to i128
> > +// CHECK-X64:  = or i128 %
> > +// CHECK-X64:  = and i8 %d, 63
> > +// CHECK-X64:  = lshr i128 %
> > +// CHECK-X64:  = trunc i128 %
> > +// CHECK-X64:  ret i64 %
> >
> >  #endif // defined(__x86_64__)
> >
> >
> > ___
> > 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
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r353725 - Attempt to pacify bots after r353718

[Nico Weber via cfe-commits]

Author: nico
Date: Mon Feb 11 09:30:25 2019
New Revision: 353725

URL: http://llvm.org/viewvc/llvm-project?rev=353725&view=rev
Log:
Attempt to pacify bots after r353718

Modified:
cfe/trunk/test/CodeGen/ms-x86-intrinsics.c

Modified: cfe/trunk/test/CodeGen/ms-x86-intrinsics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-x86-intrinsics.c?rev=353725&r1=353724&r2=353725&view=diff
==
--- cfe/trunk/test/CodeGen/ms-x86-intrinsics.c (original)
+++ cfe/trunk/test/CodeGen/ms-x86-intrinsics.c Mon Feb 11 09:30:25 2019
@@ -144,11 +144,11 @@ unsigned __int64 test__shiftleft128(unsi
   return __shiftleft128(l, h, d);
 }
 // CHECK-X64-LABEL: define dso_local i64 @test__shiftleft128(i64 %l, i64 %h, 
i8 %d)
-// CHECK-X64:  = zext i64 %h to i128
+// CHECK-X64:  = zext i64 %{{.*}} to i128
 // CHECK-X64:  = shl nuw i128 %0, 64
-// CHECK-X64:  = zext i64 %l to i128
-// CHECK-X64:  = or i128 %1, %2
-// CHECK-X64:  = and i8 %d, 63
+// CHECK-X64:  = zext i64 %{{.*}} to i128
+// CHECK-X64:  = or i128 %{{.*}}, %{{.*}}
+// CHECK-X64:  = and i8 %{{.*}}, 63
 // CHECK-X64:  = shl i128 %
 // CHECK-X64:  = lshr i128 %
 // CHECK-X64:  = trunc i128 %
@@ -159,11 +159,11 @@ unsigned __int64 test__shiftright128(uns
   return __shiftright128(l, h, d);
 }
 // CHECK-X64-LABEL: define dso_local i64 @test__shiftright128(i64 %l, i64 %h, 
i8 %d)
-// CHECK-X64:  = zext i64 %h to i128
+// CHECK-X64:  = zext i64 %{{.*}} to i128
 // CHECK-X64:  = shl nuw i128 %
-// CHECK-X64:  = zext i64 %l to i128
+// CHECK-X64:  = zext i64 %{{.*}} to i128
 // CHECK-X64:  = or i128 %
-// CHECK-X64:  = and i8 %d, 63
+// CHECK-X64:  = and i8 %{{.*}}, 63
 // CHECK-X64:  = lshr i128 %
 // CHECK-X64:  = trunc i128 %
 // CHECK-X64:  ret i64 %


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

[PATCH] D56871: [AMDGPU] Require at least protected visibility for certain symbols

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

Ping


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

https://reviews.llvm.org/D56871



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

r353729 - Attempt to pacify bots more after r353718 and r353725

[Nico Weber via cfe-commits]

Author: nico
Date: Mon Feb 11 10:01:27 2019
New Revision: 353729

URL: http://llvm.org/viewvc/llvm-project?rev=353729&view=rev
Log:
Attempt to pacify bots more after r353718 and r353725

Modified:
cfe/trunk/test/CodeGen/ms-x86-intrinsics.c

Modified: cfe/trunk/test/CodeGen/ms-x86-intrinsics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-x86-intrinsics.c?rev=353729&r1=353728&r2=353729&view=diff
==
--- cfe/trunk/test/CodeGen/ms-x86-intrinsics.c (original)
+++ cfe/trunk/test/CodeGen/ms-x86-intrinsics.c Mon Feb 11 10:01:27 2019
@@ -145,9 +145,9 @@ unsigned __int64 test__shiftleft128(unsi
 }
 // CHECK-X64-LABEL: define dso_local i64 @test__shiftleft128(i64 %l, i64 %h, 
i8 %d)
 // CHECK-X64:  = zext i64 %{{.*}} to i128
-// CHECK-X64:  = shl nuw i128 %0, 64
+// CHECK-X64:  = shl nuw i128 %{{.*}}, 64
 // CHECK-X64:  = zext i64 %{{.*}} to i128
-// CHECK-X64:  = or i128 %{{.*}}, %{{.*}}
+// CHECK-X64:  = or i128 %
 // CHECK-X64:  = and i8 %{{.*}}, 63
 // CHECK-X64:  = shl i128 %
 // CHECK-X64:  = lshr i128 %
@@ -160,7 +160,7 @@ unsigned __int64 test__shiftright128(uns
 }
 // CHECK-X64-LABEL: define dso_local i64 @test__shiftright128(i64 %l, i64 %h, 
i8 %d)
 // CHECK-X64:  = zext i64 %{{.*}} to i128
-// CHECK-X64:  = shl nuw i128 %
+// CHECK-X64:  = shl nuw i128 %{{.*}}, 64
 // CHECK-X64:  = zext i64 %{{.*}} to i128
 // CHECK-X64:  = or i128 %
 // CHECK-X64:  = and i8 %{{.*}}, 63


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

[PATCH] D45978: dllexport const variables must have external linkage.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/Sema/dllexport-1.c:1-4
+// RUN: %clang_cc1 -triple i686-win32 -emit-llvm -fms-extensions -std=c99 < 
%s| FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-win32 -emit-llvm -fms-extensions -std=c11 < 
%s | FileCheck %s
+// RUN: %clang_cc1 -triple i686-mingw32 -emit-llvm -fms-extensions -std=c11 < 
%s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-mingw32 -emit-llvm -fms-extensions -std=c11 
< %s | FileCheck %s

This test should live in CodeGen not Sema.



Comment at: test/Sema/dllexport-1.c:8
+
+// CHECK: @y = common dso_local dllexport global i32 0, align 4
+

Are x and z also exported as expected?



Comment at: test/Sema/dllexport-1.cpp:4
+
+// CHECK: @"?x@@3HB" = dso_local dllexport constant i32 3, align 4
+

This test has no FileCheck line.

The goal here is for the tests in Sema to test the semantic behavior (that 
warnings are issued when expected and not issued when not expected) and to add 
tests in CodeGen to test the generated code (are things properly exported, are 
things we don't expect to be exported not exported, etc). I think you should 
split your tests accordingly rather than try to verify too much at once.


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

https://reviews.llvm.org/D45978



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

[PATCH] D58062: Support framework import/include auto-completion

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:8375
+  auto AddFilesFromIncludeDir = [&](StringRef IncludeDir, bool IsSystem,
+bool isFramework) {
+bool stripFrameworkSuffix = false;

nit: `IsFramework`, `StripFrameworkSuffix` etc



Comment at: lib/Sema/SemaCodeComplete.cpp:8375
+  auto AddFilesFromIncludeDir = [&](StringRef IncludeDir, bool IsSystem,
+bool isFramework) {
+bool stripFrameworkSuffix = false;

sammccall wrote:
> nit: `IsFramework`, `StripFrameworkSuffix` etc
probably clearer just to pass in the `DirectoryLookup::LookupType_t`, rather 
than another bool.



Comment at: lib/Sema/SemaCodeComplete.cpp:8378
 llvm::SmallString<128> Dir = IncludeDir;
-if (!NativeRelDir.empty())
+if (isFramework) {
+  if (NativeRelDir.empty()) {

I don't think keeping the `stripFrameworkSuffix` state is worth scattering the 
reader's attention here.

I'd suggest instead:

```
if (!NativeRelDir.empty()) {
  if (LookupType == LT_Framework) {
// In a framework directory, Foo/Bar/ is actually 
Foo.framework/Headers/Bar/.
...
  } else {
append(Dir, NativeRelDir);
  }
}
```

or even pass Dir, NativeRelDir, and LookupType to a helper function to sort it 
out.

Below, you can just check NativeRelDir.empty() && type == Framework again...

If you do choose to keep it, please give it a descriptive name rather than an 
imperative one, e.g. `ListingFrameworksDir`



Comment at: lib/Sema/SemaCodeComplete.cpp:8378
 llvm::SmallString<128> Dir = IncludeDir;
-if (!NativeRelDir.empty())
+if (isFramework) {
+  if (NativeRelDir.empty()) {

sammccall wrote:
> I don't think keeping the `stripFrameworkSuffix` state is worth scattering 
> the reader's attention here.
> 
> I'd suggest instead:
> 
> ```
> if (!NativeRelDir.empty()) {
>   if (LookupType == LT_Framework) {
> // In a framework directory, Foo/Bar/ is actually 
> Foo.framework/Headers/Bar/.
> ...
>   } else {
> append(Dir, NativeRelDir);
>   }
> }
> ```
> 
> or even pass Dir, NativeRelDir, and LookupType to a helper function to sort 
> it out.
> 
> Below, you can just check NativeRelDir.empty() && type == Framework again...
> 
> If you do choose to keep it, please give it a descriptive name rather than an 
> imperative one, e.g. `ListingFrameworksDir`
this needs some explanation (mostly the why, not the how)



Comment at: lib/Sema/SemaCodeComplete.cpp:8388
+  Framework += ".framework";
+  llvm::sys::path::append(Dir, Framework, "Headers");
+  llvm::sys::path::append(Dir, ++Begin, End);

just `append(Dir, *Begin + ".framework", "Headers")` ?
append takes twines I think.



Comment at: lib/Sema/SemaCodeComplete.cpp:8390
+  llvm::sys::path::append(Dir, ++Begin, End);
+} else {
+  SmallString<32> Framework(NativeRelDir);

why is the if/else needed? the code above should cover both cases, the last 
append is a no-op if ++begin == end.



Comment at: lib/Sema/SemaCodeComplete.cpp:8409
   case llvm::sys::fs::file_type::directory_file:
-AddCompletion(Filename, /*IsDirectory=*/true);
+if (stripFrameworkSuffix && Filename.endswith(".framework")) {
+  auto FrameworkName = Filename.substr(0, Filename.size() - 10);

if a framework directory contains a subdirectory that does not end in 
".framework", is it actually legal to include from it? Maybe we should be 
omitting it.



Comment at: lib/Sema/SemaCodeComplete.cpp:8410
+if (stripFrameworkSuffix && Filename.endswith(".framework")) {
+  auto FrameworkName = Filename.substr(0, Filename.size() - 10);
+  AddCompletion(FrameworkName, /*IsDirectory=*/true);

```
if (should strip suffix)
  Filename.consume_back(".framework")
```
(no need for else)



Comment at: lib/Sema/SemaCodeComplete.cpp:8410
+if (stripFrameworkSuffix && Filename.endswith(".framework")) {
+  auto FrameworkName = Filename.substr(0, Filename.size() - 10);
+  AddCompletion(FrameworkName, /*IsDirectory=*/true);

sammccall wrote:
> ```
> if (should strip suffix)
>   Filename.consume_back(".framework")
> ```
> (no need for else)
one-line comment should be enough to explain the intent here: `// Entries in a 
framework directory have a .framework suffix, this does not appear in the 
source code`


Repository:
  rC Clang

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

https://reviews.llvm.org/D58062



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

[PATCH] D58065: [analyzer] Document the frontend library

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

Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, dkrupp, rnkovacs, 
a.sidorin, aaron.ballman.
Herald added subscribers: cfe-commits, gamesh411, donat.nagy, mikhail.ramalho, 
szepet, baloghadamsoftware, whisperity.
Herald added a project: clang.

Since I've had my fair share of complaining about the lack of documentation, 
it's time I did my part, hence this patch, which provides documentation for a 
good majority of the frontend library.

Preview:
http://cc.elte.hu/clang-docs/docs/analyzer-frontend-html/analyzer/developer-docs/FrontendLibrary.html

Several more items could be added, describing how `AnalysisConsumer` interacts 
with `AnalysisManager` would be the most important of those. For now, I put the 
greatest emphasis on checker registration.

What should this be expanded with? Is this document too large, maybe I should 
split it up? I haven't documented much so far, so I'd happily take any feedback.


Repository:
  rC Clang

https://reviews.llvm.org/D58065

Files:
  docs/analyzer/developer-docs.rst
  docs/analyzer/developer-docs/FrontendLibrary.rst
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h

Index: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
===
--- include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -16,46 +16,6 @@
 #include 
 #include 
 
-// FIXME: move this information to an HTML file in docs/.
-// At the very least, a checker plugin is a dynamic library that exports
-// clang_analyzerAPIVersionString. This should be defined as follows:
-//
-//   extern "C"
-//   const char clang_analyzerAPIVersionString[] =
-// CLANG_ANALYZER_API_VERSION_STRING;
-//
-// This is used to check whether the current version of the analyzer is known to
-// be incompatible with a plugin. Plugins with incompatible version strings,
-// or without a version string at all, will not be loaded.
-//
-// To add a custom checker to the analyzer, the plugin must also define the
-// function clang_registerCheckers. For example:
-//
-//extern "C"
-//void clang_registerCheckers (CheckerRegistry ®istry) {
-//  registry.addChecker("example.MainCallChecker",
-//"Disallows calls to functions called main");
-//}
-//
-// The first method argument is the full name of the checker, including its
-// enclosing package. By convention, the registered name of a checker is the
-// name of the associated class (the template argument).
-// The second method argument is a short human-readable description of the
-// checker.
-//
-// The clang_registerCheckers function may add any number of checkers to the
-// registry. If any checkers require additional initialization, use the three-
-// argument form of CheckerRegistry::addChecker.
-//
-// To load a checker plugin, specify the full path to the dynamic library as
-// the argument to the -load option in the cc1 frontend. You can then enable
-// your custom checker using the -analyzer-checker:
-//
-//   clang -cc1 -load  -analyze
-// -analyzer-checker=
-//
-// For a complete working example, see examples/analyzer-plugin.
-
 #ifndef CLANG_ANALYZER_API_VERSION_STRING
 // FIXME: The Clang version string is not particularly granular;
 // the analyzer infrastructure can change a lot between releases.
Index: docs/analyzer/developer-docs/FrontendLibrary.rst
===
--- /dev/null
+++ docs/analyzer/developer-docs/FrontendLibrary.rst
@@ -0,0 +1,643 @@
+
+Frontend Library (libStaticAnalyzerFrontend)
+
+
+.. contents:: Table of Contents
+   :depth: 4
+
+Introduction
+
+
+This document will describe the frontend of the Static Analyzer, basically
+everything from compiling the analyzer from source, through it's invocation up
+to the beginning of the analysis. It will touch on topics such as
+
+* How the analyzer is compiled, how tools such as TableGen are used to generate
+  some of the code,
+* How to invoke the analyzer,
+* How crucial objects of the analyzer are initialized before the actual
+  analysis begins, like
+
+  * The `AnalyzerOptions` class, which entails how the command line options are
+parsed,
+  * The `CheckerManager` class, which entails how the checkers of the analyzer
+are registered and loaded into it,
+  * No list is complete without at least a third item.
+
+* How certain errors are handled with regards to backward compatibility,
+
+starting from how an entry in the TableGen gets processed during the
+compilation of the project, how this process begins runtime when the analyzer
+is invoked, up to the point where the actual analysis begins.
+
+The document will rely on the reader having a basic understanding about what
+checkers are, have invoked the analyzer at least a few times from the command
+

[PATCH] D56871: [AMDGPU] Require at least protected visibility for certain symbols

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks!


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

https://reviews.llvm.org/D56871



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

[PATCH] D17444: PR26672: [MSVC] Clang does not recognize "static_assert" keyword in C mode

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D17444#1392823 , @thakis wrote:

> Instead of this patch we could have an assert.h wrapper in lib/Headers that 
> defines static_assert to _Static_assert in ms mode for C files, and I suppose 
> that's a cleaner fix. But I hope it's not controversial that we should try 
> and support standard C programs?


I'd prefer it if we didn't shadow more MSVC CRT headers like assert.h. Users 
have a long history of holding the compiler wrong, losing these interpositions, 
and having things not work. If we could just work out the box, users will have 
less problems, we'll have fewer bug reports, etc, etc. I still think we should 
follow MSVC and add this is a plain old C++ extension to C under 
-fms-extensions.


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

https://reviews.llvm.org/D17444



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

[PATCH] D58016: fixes copy constructor generation of classes containing 0-length arrays followed by exactly 1 trivial field (fixes #40682)

[John McCall via Phabricator via cfe-commits]

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

Ah, thanks, seems like a reasonable fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58016



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

[PATCH] D57991: [Driver][Darwin] Emit an error when using -pg on OS without support for it.

[Steven Wu via Phabricator via cfe-commits]

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

LGTM with a suggestion to make code cleaner.




Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:101
+  "the clang compiler does not support -pg option on Darwin">;
+def err_drv_clang_unsupported_opt_pg_darwin_osx: Error<
+  "the clang compiler does not support -pg option on versions of OS X 10.9 and 
later">;

Might be cleaner if you use %select here.


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

https://reviews.llvm.org/D57991



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

[PATCH] D58016: fixes copy constructor generation of classes containing 0-length arrays followed by exactly 1 trivial field (fixes #40682)

[Joran Bigalet via Phabricator via cfe-commits]

jbigalet added a comment.

Thanks!
I don't have commit rights btw


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58016



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

RE: r353718 - Make test actually test something (colons were missing)

[via cfe-commits]

I thin the Best Fix is probably to make FileCheck diag if a line starts with 
(after a few whitelisted comment chars like // and #) a check-prefix but then 
isn't followed by : (maybe after -NOT, -SAME, -LABEL etc).

FileCheck doesn't explicitly pass over leading comment characters, it just 
pattern-matches the check names followed by colon or hyphen. You can put 
arbitrary text in front of a check name and FileCheck won't care (although your 
reviewers might).
I can see the value in a diag like this.  It does mean any use of a check name 
that isn't *intended* to be a directive would be flagged (e.g., in a random 
comment within the test).  That pattern-match is case-sensitive and our 
convention is to use uppercase check names, so it probably wouldn't be TOO bad.
If anybody wants to do this I'm happy to review it.  Or file a PR and cc me.
--paulr
who has done a lot of FileCheck patch reviewing lately

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

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington marked an inline comment as done.
erik.pilkington added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:987-989
+The first argument to the attribute is the type passed to
+`__builtin_object_size`, and the second is the flag that the fortified format
+functions accept.

aaron.ballman wrote:
> Maybe I'm being dense, but I still have no idea what I'd pass for either of 
> these arguments. When I hear "type", I immediately assume I should pass in 
> something like 'int', but that seems wrong given that this is an integer 
> argument. Is there some public documentation we could link to with a "for 
> more information, see " snippet? Similar for the fortified format 
> function flag.
> 
> However, I'm also starting to wonder why this attribute is interesting to 
> users. Why not infer this attribute automatically, since there's only a 
> specified set of standard library functions that it can be applied to anyway? 
> Is it a bad idea to try to infer this for some reason?
Yeah, I guess we could link to GCC's docs for `__builtin_object_size`, I'll 
update this. I think the flag argument has something to do with enabling 
checking `%n` output parameters, but I'm not totally sure and don't want to 
spread any misinformation in the clang docs. On our implementation, the value 
is just ignored. 

This attribute would never really be used by users that aren't C library 
implementers. The problem with doing this automatically is that library users 
need to be able to customize the checking mode by defining the 
`_FORTIFY_SOURCE` macro to a level in their `.c` files. We can't do this based 
on the presence of that macro for a couple reasons, firstly, I'm not sure we 
can assume that all `*_chk` variants are present just because `_FORTIFY_SOURCE` 
is defined (whether the `_chk` variants are present seems like a decision best 
left to the library). And secondly because that would mean that `clang -E t.c 
-o - | clang -xc -` would generate different code from `clang t.c`. Given that, 
it seems like an attribute is the best customization point.


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

https://reviews.llvm.org/D57918



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