[PATCH] D54408: [ASTMatchers] Add matchers available through casting to derived

[Andi via Phabricator via cfe-commits]

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

@aaron.ballman 
I think the auto usage improves and simplifies the code, however, as I would 
really like to see this code landed, I would be ok to help Steven to finish 
this work and replace auto by the "old" way. Do you think we can have this 
approved if we make the changes mentioned earlier?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54408



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

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[vit9696 via Phabricator via cfe-commits]

vit9696 added a comment.

@jhibbits, thank you for merging. Will we have this in LLVM 9.0?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D49754



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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D67159#1659103 , @simon_tatham 
wrote:

> Come to think of it, it would also not be too hard to constrain it to 
> //only// be usable for a particular subset of builtins, and perhaps even only 
> with a particular set of alias names for them. (I could easily derive all 
> that information from the same Tablegen that `arm_mve.h` itself is made from.)


I think this might be a good idea to explore. In that case, I would recommend 
naming the attribute `__clang_arm_mve_builtin` to make it obvious that this 
attribute has a very specific use in mind.




Comment at: clang/include/clang/Basic/Attr.td:596
 }
+def ClangBuiltinOverride : Attr {
+  let Spellings = [GCC<"__clang_builtin">];

simon_tatham wrote:
> aaron.ballman wrote:
> > Do you expect this attribute to be inherited on redeclarations? I suspect 
> > this should be an `InheritableAttr`.
> > 
> > Also, add a newline above it for visual separation, please.
> > 
> > Finally, should this be a target-specific attribute so that it's only 
> > available for your target, or do you expect this attribute to be used on 
> > all target architectures?
> For my use case, I have no opinion about redeclarations: I expect to declare 
> each affected function exactly once. If you think `InheritableAttr` is a more 
> sensible default choice, I'm fine with that.
> 
> Target-specific: I don't have a use case outside the ARM target, so I'd be 
> happy to lock it down that way if you want.
I think it should be an `InheritableAttr` that is target-specific. We can 
always expand the targets later if we think the attribute is generally useful.



Comment at: clang/include/clang/Basic/Attr.td:600
+  let Subjects = SubjectList<[Function], ErrorDiag>;
+  let Documentation = [Undocumented];
+}

simon_tatham wrote:
> aaron.ballman wrote:
> > No new undocumented attributes, please.
> OK. I'd intended to leave it undocumented in order to discourage people from 
> using it in any context //other// than a system header file. But fair enough 
> – perhaps it should be documented even so.
Documenting it still helps developers even if the documentation effectively 
states that something is for internal use only.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67159



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

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[Justin Hibbits via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371066: Add -m(no)-spe to clang (authored by jhibbits, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49754?vs=207214=218907#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D49754

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Basic/Targets/PPC.cpp
  cfe/trunk/lib/Basic/Targets/PPC.h
  cfe/trunk/lib/CodeGen/TargetInfo.cpp
  cfe/trunk/test/Driver/ppc-features.cpp
  cfe/trunk/test/Preprocessor/init.c

Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -2269,6 +2269,8 @@
 def fno_altivec : Flag<["-"], "fno-altivec">, Group, Flags<[DriverOption]>;
 def maltivec : Flag<["-"], "maltivec">, Group;
 def mno_altivec : Flag<["-"], "mno-altivec">, Group;
+def mspe : Flag<["-"], "mspe">, Group;
+def mno_spe : Flag<["-"], "mno-spe">, Group;
 def mvsx : Flag<["-"], "mvsx">, Group;
 def mno_vsx : Flag<["-"], "mno-vsx">, Group;
 def msecure_plt : Flag<["-"], "msecure-plt">, Group;
Index: cfe/trunk/test/Driver/ppc-features.cpp
===
--- cfe/trunk/test/Driver/ppc-features.cpp
+++ cfe/trunk/test/Driver/ppc-features.cpp
@@ -168,6 +168,9 @@
 // RUN: %clang -target powerpc64-unknown-linux-gnu %s -mno-invariant-function-descriptors -minvariant-function-descriptors -### -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-INVFUNCDESC %s
 // CHECK-INVFUNCDESC: "-target-feature" "+invariant-function-descriptors"
 
+// RUN: %clang -target powerpc-unknown-linux-gnu %s -mno-spe -mspe -### -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-SPE %s
+// CHECK-SPE: "-target-feature" "+spe"
+
 // Assembler features
 // RUN: %clang -target powerpc64-unknown-linux-gnu %s -### -o %t.o -no-integrated-as 2>&1 | FileCheck -check-prefix=CHECK_BE_AS_ARGS %s
 // CHECK_BE_AS_ARGS: "-mppc64"
Index: cfe/trunk/test/Preprocessor/init.c
===
--- cfe/trunk/test/Preprocessor/init.c
+++ cfe/trunk/test/Preprocessor/init.c
@@ -7580,6 +7580,11 @@
 //
 // PPC32-LINUX-NOT: _CALL_LINUX
 //
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-unknown-linux-gnu -target-feature +spe < /dev/null | FileCheck -match-full-lines -check-prefix PPC32-SPE %s
+//
+// PPC32-SPE:#define __NO_FPRS__ 1
+// PPC32-SPE:#define __SPE__ 1
+//
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-apple-darwin8 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-DARWIN %s
 //
 // PPC-DARWIN:#define _ARCH_PPC 1
Index: cfe/trunk/lib/Basic/Targets/PPC.cpp
===
--- cfe/trunk/lib/Basic/Targets/PPC.cpp
+++ cfe/trunk/lib/Basic/Targets/PPC.cpp
@@ -54,6 +54,10 @@
   HasFloat128 = true;
 } else if (Feature == "+power9-vector") {
   HasP9Vector = true;
+} else if (Feature == "+spe") {
+  HasSPE = true;
+  LongDoubleWidth = LongDoubleAlign = 64;
+  LongDoubleFormat = ::APFloat::IEEEdouble();
 } else if (Feature == "-hard-float") {
   FloatABI = SoftFloat;
 }
@@ -165,6 +169,10 @@
 Builder.defineMacro("__VEC__", "10206");
 Builder.defineMacro("__ALTIVEC__");
   }
+  if (HasSPE) {
+Builder.defineMacro("__SPE__");
+Builder.defineMacro("__NO_FPRS__");
+  }
   if (HasVSX)
 Builder.defineMacro("__VSX__");
   if (HasP8Vector)
@@ -203,7 +211,6 @@
   //   __CMODEL_LARGE__
   //   _CALL_SYSV
   //   _CALL_DARWIN
-  //   __NO_FPRS__
 }
 
 // Handle explicit options being passed to the compiler here: if we've
@@ -332,6 +339,7 @@
   .Case("extdiv", HasExtDiv)
   .Case("float128", HasFloat128)
   .Case("power9-vector", HasP9Vector)
+  .Case("spe", HasSPE)
   .Default(false);
 }
 
Index: cfe/trunk/lib/Basic/Targets/PPC.h
===
--- cfe/trunk/lib/Basic/Targets/PPC.h
+++ cfe/trunk/lib/Basic/Targets/PPC.h
@@ -66,6 +66,7 @@
   bool HasBPERMD = false;
   bool HasExtDiv = false;
   bool HasP9Vector = false;
+  bool HasSPE = false;
 
 protected:
   std::string ABI;
Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -9726,7 +9726,8 @@
 
   case llvm::Triple::ppc:
 return SetCGInfo(
-new PPC32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI == "soft"));
+new PPC32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI == "soft" ||
+   getTarget().hasFeature("spe")));
   case llvm::Triple::ppc64:
 

r371066 - Add -m(no)-spe to clang

[Justin Hibbits via cfe-commits]

Author: jhibbits
Date: Thu Sep  5 06:38:46 2019
New Revision: 371066

URL: http://llvm.org/viewvc/llvm-project?rev=371066=rev
Log:
Add -m(no)-spe to clang

Summary:
r337347 added support for the Signal Processing Engine (SPE) to LLVM.
This follows that up with the clang side.

This adds -mspe and -mno-spe, to match GCC.

Subscribers: nemanjai, kbarton, cfe-commits

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

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Basic/Targets/PPC.cpp
cfe/trunk/lib/Basic/Targets/PPC.h
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/test/Driver/ppc-features.cpp
cfe/trunk/test/Preprocessor/init.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=371066=371065=371066=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Sep  5 06:38:46 2019
@@ -2269,6 +2269,8 @@ def faltivec : Flag<["-"], "faltivec">,
 def fno_altivec : Flag<["-"], "fno-altivec">, Group, 
Flags<[DriverOption]>;
 def maltivec : Flag<["-"], "maltivec">, Group;
 def mno_altivec : Flag<["-"], "mno-altivec">, Group;
+def mspe : Flag<["-"], "mspe">, Group;
+def mno_spe : Flag<["-"], "mno-spe">, Group;
 def mvsx : Flag<["-"], "mvsx">, Group;
 def mno_vsx : Flag<["-"], "mno-vsx">, Group;
 def msecure_plt : Flag<["-"], "msecure-plt">, Group;

Modified: cfe/trunk/lib/Basic/Targets/PPC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/PPC.cpp?rev=371066=371065=371066=diff
==
--- cfe/trunk/lib/Basic/Targets/PPC.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/PPC.cpp Thu Sep  5 06:38:46 2019
@@ -54,6 +54,10 @@ bool PPCTargetInfo::handleTargetFeatures
   HasFloat128 = true;
 } else if (Feature == "+power9-vector") {
   HasP9Vector = true;
+} else if (Feature == "+spe") {
+  HasSPE = true;
+  LongDoubleWidth = LongDoubleAlign = 64;
+  LongDoubleFormat = ::APFloat::IEEEdouble();
 } else if (Feature == "-hard-float") {
   FloatABI = SoftFloat;
 }
@@ -165,6 +169,10 @@ void PPCTargetInfo::getTargetDefines(con
 Builder.defineMacro("__VEC__", "10206");
 Builder.defineMacro("__ALTIVEC__");
   }
+  if (HasSPE) {
+Builder.defineMacro("__SPE__");
+Builder.defineMacro("__NO_FPRS__");
+  }
   if (HasVSX)
 Builder.defineMacro("__VSX__");
   if (HasP8Vector)
@@ -203,7 +211,6 @@ void PPCTargetInfo::getTargetDefines(con
   //   __CMODEL_LARGE__
   //   _CALL_SYSV
   //   _CALL_DARWIN
-  //   __NO_FPRS__
 }
 
 // Handle explicit options being passed to the compiler here: if we've
@@ -332,6 +339,7 @@ bool PPCTargetInfo::hasFeature(StringRef
   .Case("extdiv", HasExtDiv)
   .Case("float128", HasFloat128)
   .Case("power9-vector", HasP9Vector)
+  .Case("spe", HasSPE)
   .Default(false);
 }
 

Modified: cfe/trunk/lib/Basic/Targets/PPC.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/PPC.h?rev=371066=371065=371066=diff
==
--- cfe/trunk/lib/Basic/Targets/PPC.h (original)
+++ cfe/trunk/lib/Basic/Targets/PPC.h Thu Sep  5 06:38:46 2019
@@ -66,6 +66,7 @@ class LLVM_LIBRARY_VISIBILITY PPCTargetI
   bool HasBPERMD = false;
   bool HasExtDiv = false;
   bool HasP9Vector = false;
+  bool HasSPE = false;
 
 protected:
   std::string ABI;

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=371066=371065=371066=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Thu Sep  5 06:38:46 2019
@@ -9726,7 +9726,8 @@ const TargetCodeGenInfo ::
 
   case llvm::Triple::ppc:
 return SetCGInfo(
-new PPC32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI == "soft"));
+new PPC32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI == "soft" ||
+   getTarget().hasFeature("spe")));
   case llvm::Triple::ppc64:
 if (Triple.isOSBinFormatELF()) {
   PPC64_SVR4_ABIInfo::ABIKind Kind = PPC64_SVR4_ABIInfo::ELFv1;

Modified: cfe/trunk/test/Driver/ppc-features.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/ppc-features.cpp?rev=371066=371065=371066=diff
==
--- cfe/trunk/test/Driver/ppc-features.cpp (original)
+++ cfe/trunk/test/Driver/ppc-features.cpp Thu Sep  5 06:38:46 2019
@@ -168,6 +168,9 @@
 // RUN: %clang -target powerpc64-unknown-linux-gnu %s 
-mno-invariant-function-descriptors -minvariant-function-descriptors -### -o 
%t.o 2>&1 | FileCheck -check-prefix=CHECK-INVFUNCDESC %s
 // 

[PATCH] D67058: [clang][CodeGen] Add alias for cpu_dispatch function with IFunc & Fix resolver linkage type

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Actually... I think it might need to be weak_odr based on 
https://llvm.org/docs/LangRef.html#linkage-types

We want the merge semantics, but need to make sure that the symbols aren't 
discarded.


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

https://reviews.llvm.org/D67058



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

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

[Simon Cook via Phabricator via cfe-commits]

simoncook added a comment.

For added context, I have gone and double-checked with GCC's implementation 
both for AArch64 and RISC-V and for registers used by the calling convention 
the compiler will still use them for argument passing and return values, but 
otherwise won't use it for any temporaries/register allocation purposes, which 
does have the side effect of confusing behaviour unless carefully documented.

I can implement errors for using calling convention registers when there are 
functions that take arguments, but this would be an explicit deviation in 
behaviour between the two compilers. I presume we would want to do that anyway 
because it's safer/more clear?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67185



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

[PATCH] D67058: [clang][CodeGen] Add alias for cpu_dispatch function with IFunc & Fix resolver linkage type

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3002
 false);
 llvm::Constant *Resolver = GetOrCreateLLVMFunction(
 MangledName + ".resolver", ResolverType, GlobalDecl{},

This Resolver should have the same linkage as below.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3005
 /*ForVTable=*/false);
+auto Linkage = (FD->isCPUDispatchMultiVersion() || 
FD->isCPUSpecificMultiVersion())
+? llvm::Function::LinkOnceODRLinkage

I think this can always just be LinkOnceODR.


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

https://reviews.llvm.org/D67058



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

[PATCH] D58497: Clear the KnownModules cache if the preprocessor is going away

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.

I have not seen this problem resurface to be honest. When we initially hit it, 
changing the path for the build worked around the problem for us so we weren't 
really hitting in any longer. I posted this because I realized the possibility 
exists of having these dangling pointers (and it certainly fixed our original 
problem even without changing the build path).
However, I must say that I am completely out of my depth here as I am not a 
front end developer and don't really know how any of this stuff is supposed to 
work (i.e. whether it makes sense for `PP` here to be pointing to something 
that will go away).

That being said, I am perfectly happy to do what the experts suggest here, 
including abandoning the patch, updating it as requested, etc.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58497



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

[PATCH] D59637: [analyzer] Use the custom propagation rules and sinks in GenericTaintChecker

[Balázs Benics via Phabricator via cfe-commits]

steakhal added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:191
 static TaintPropagationRule
-getTaintPropagationRule(const FunctionDecl *FDecl, StringRef Name,
+getTaintPropagationRule(const GenericTaintChecker *Checker,
+const FunctionDecl *FDecl, StringRef Name,

Szelethus wrote:
> How about only passing `CustomPropagations`?
I would even consider to move this function out of the whole class. (Not only 
this function, but the others as well. Like isStdin, etc.)
I think pure, free-functions (in an anonymous namespace) are easier to reason 
about.



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:844
   if (Config)
-Checker->parseConfiguration(Mgr, Option, std::move(Config).getValue());
+Checker->parseConfiguration(Mgr, Option, std::move(Config.getValue()));
 }

Szelethus wrote:
> Wasn't this commited before?
Yes it was 
(https://github.com/llvm/llvm-project/blob/master/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp#L814).
I would kindly request a rebase.



Comment at: test/Analysis/taint-generic.c:377
+  int x = mySource1();
+  mySink(x, 1, 2); // expected-warning {{Untrusted data is passed to a 
user-defined sink}}
+  mySink(1, x, 2); // no-warning

We could use this syntacs to achieve shorter lines. Note that `@-1`. Same for 
all the other lines.
```
mySink(x, 1, 2);
// expected-warning@-1 {{Untrusted data is passed to a user-defined sink}}
```


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

https://reviews.llvm.org/D59637



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

[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:30
+ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "WhitelistedPrefixes", WhitelistedPrefixes);
+}

aaron.ballman wrote:
> `ExpectedPrefixes` here as well.
> 
> Should there be a default list of these?
Still wondering whether we should have a default list of expected prefixes or 
not.



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:15
+
+static const char *kCustomCategoryMethodIdentifier = "ThisIsACategoryMethod";
+

Drop the `k` prefix (we don't use Hungarian notation).



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:36-40
+  auto ClassNameIter = llvm::find_if(PrefixArray,
+  [ClassName](const std::string ) {
+  return ClassName.startswith(Str);
+  });
+  return ClassNameIter != PrefixArray.end();

Sorry for not recognizing this earlier, but since we don't care about which 
item was found, we can go with:
```
return llvm::any_of(PrefixArray, ...);
```



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:57
+  }
+  std::string method_name = method_declaration->getNameAsString();
+  auto owning_objc_class_interface = method_declaration->getClassInterface();

aaron.ballman wrote:
> dgatwood wrote:
> > aaron.ballman wrote:
> > > This should use `getName()` to get a `StringRef` to avoid the copy.
> > That's actually what I originally tried, but that method won't work here, 
> > unless I'm missing something.  The getName() method crashes with a message 
> > saying that "Name is not a simple identifier".
> You can call `getIdentifier()` instead, and if you get a non-null object 
> back, you can call `getName()` on that. If it is null, there's nothing to 
> check.
The comment to use `getIdentifier()` was marked as done but the changes were 
not applied; was that a mistake? I'm pushing back on `getNameAsString()` 
because the function is commented as having its use discouraged, so we should 
not be adding new uses of it.



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h:22-23
+
+  void registerMatchers(ast_matchers::MatchFinder *finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+

aaron.ballman wrote:
> `Finder` and `Result` per coding style.
`finder` -> `Finder`
`result` -> `Result`



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h:31
+  llvm::Optional
+  matchingWhitelistedPrefix(StringRef class_name);
+};

aaron.ballman wrote:
> `class_name` should be `ClassName`.
`class_name` -> `ClassName`


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

https://reviews.llvm.org/D65917



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

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

[Yubo Xie via Phabricator via cfe-commits]

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

Rebase patch with current HEAD.


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

https://reviews.llvm.org/D67084

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


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


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

Re: r371027 - Revert r361885 "[Driver] Fix -working-directory issues"

[Hans Wennborg via cfe-commits]

Merged to release_90 in r371060.

On Thu, Sep 5, 2019 at 10:41 AM Hans Wennborg via cfe-commits
 wrote:
>
> Author: hans
> Date: Thu Sep  5 01:43:00 2019
> New Revision: 371027
>
> URL: http://llvm.org/viewvc/llvm-project?rev=371027=rev
> Log:
> Revert r361885 "[Driver] Fix -working-directory issues"
>
> This made clang unable to open files using relative paths on network shares on
> Windows (PR43204). On the bug it was pointed out that 
> createPhysicalFileSystem()
> is not terribly mature, and using it is risky. Reverting for now until there's
> a clear way forward.
>
> > Currently the `-working-directory` option does not actually impact the 
> > working
> > directory for all of the clang driver, it only impacts how files are looked 
> > up
> > to make sure they exist.  This means that that clang passes the wrong paths
> > to -fdebug-compilation-dir and -coverage-notes-file.
> >
> > This patch fixes that by changing all the places in the driver where we 
> > convert
> > to absolute paths to use the VFS, and then calling 
> > setCurrentWorkingDirectory on
> > the VFS.  This also changes the default VFS for `Driver` to use a 
> > virtualized
> > working directory, instead of changing the process's working directory.
> >
> > Differential Revision: https://reviews.llvm.org/D62271
>
> This also revertes the part of r369938 which checked that -working-directory 
> works.
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
> cfe/trunk/lib/Driver/Driver.cpp
> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> cfe/trunk/test/Driver/gen-cdb-fragment.c
> cfe/trunk/test/Driver/working-directory.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=371027=371026=371027=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Thu Sep  5 
> 01:43:00 2019
> @@ -91,8 +91,6 @@ def err_no_external_assembler : Error<
>"there is no external assembler that can be used on this platform">;
>  def err_drv_unable_to_remove_file : Error<
>"unable to remove file: %0">;
> -def err_drv_unable_to_set_working_directory : Error <
> -  "unable to set working directory: %0">;
>  def err_drv_command_failure : Error<
>"unable to execute command: %0">;
>  def err_drv_invalid_darwin_version : Error<
>
> Modified: cfe/trunk/lib/Driver/Driver.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=371027=371026=371027=diff
> ==
> --- cfe/trunk/lib/Driver/Driver.cpp (original)
> +++ cfe/trunk/lib/Driver/Driver.cpp Thu Sep  5 01:43:00 2019
> @@ -133,7 +133,7 @@ Driver::Driver(StringRef ClangExecutable
>
>// Provide a sane fallback if no VFS is specified.
>if (!this->VFS)
> -this->VFS = llvm::vfs::createPhysicalFileSystem().release();
> +this->VFS = llvm::vfs::getRealFileSystem();
>
>Name = llvm::sys::path::filename(ClangExecutable);
>Dir = llvm::sys::path::parent_path(ClangExecutable);
> @@ -1010,11 +1010,6 @@ Compilation *Driver::BuildCompilation(Ar
>  }
>}
>
> -  // Check for working directory option before accessing any files
> -  if (Arg *WD = Args.getLastArg(options::OPT_working_directory))
> -if (VFS->setCurrentWorkingDirectory(WD->getValue()))
> -  Diag(diag::err_drv_unable_to_set_working_directory) << WD->getValue();
> -
>// FIXME: This stuff needs to go into the Compilation, not the driver.
>bool CCCPrintPhases;
>
> @@ -1996,11 +1991,20 @@ bool Driver::DiagnoseInputExistence(cons
>if (Value == "-")
>  return true;
>
> -  if (getVFS().exists(Value))
> +  SmallString<64> Path(Value);
> +  if (Arg *WorkDir = Args.getLastArg(options::OPT_working_directory)) {
> +if (!llvm::sys::path::is_absolute(Path)) {
> +  SmallString<64> Directory(WorkDir->getValue());
> +  llvm::sys::path::append(Directory, Value);
> +  Path.assign(Directory);
> +}
> +  }
> +
> +  if (getVFS().exists(Path))
>  return true;
>
>if (IsCLMode()) {
> -if (!llvm::sys::path::is_absolute(Twine(Value)) &&
> +if (!llvm::sys::path::is_absolute(Twine(Path)) &&
>  llvm::sys::Process::FindInEnvPath("LIB", Value))
>return true;
>
> @@ -2026,12 +2030,12 @@ bool Driver::DiagnoseInputExistence(cons
>  if (getOpts().findNearest(Value, Nearest, IncludedFlagsBitmask,
>ExcludedFlagsBitmask) <= 1) {
>Diag(clang::diag::err_drv_no_such_file_with_suggestion)
> -  << Value << Nearest;
> +  << Path << Nearest;
>return false;
>  }
>}
>
> -  Diag(clang::diag::err_drv_no_such_file) << Value;
> +  Diag(clang::diag::err_drv_no_such_file) << Path;
>return false;
>  }
>
>
> Modified: 

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

[Yubo Xie via Phabricator via cfe-commits]

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

Update patch.


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

https://reviews.llvm.org/D67056

Files:
  clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.h
  clang-tools-extra/docs/clang-tidy/checks/bugprone-argument-comment.rst
  
clang-tools-extra/test/clang-tidy/bugprone-argument-comment-ignore-single-argument.cpp

Index: clang-tools-extra/test/clang-tidy/bugprone-argument-comment-ignore-single-argument.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-argument-comment-ignore-single-argument.cpp
@@ -0,0 +1,97 @@
+// RUN: %check_clang_tidy %s bugprone-argument-comment %t -- \
+// RUN:   -config="{CheckOptions: [{key: bugprone-argument-comment.IgnoreSingleArgument, value: 1}, {key: CommentBoolLiterals, value: 1},{key: CommentIntegerLiterals, value: 1}, {key: CommentFloatLiterals, value: 1}, {key: CommentUserDefinedLiterals, value: 1}, {key: CommentStringLiterals, value: 1}, {key: CommentNullPtrs, value: 1}, {key: CommentCharacterLiterals, value: 1}]}" --
+
+struct A {
+  void foo(bool abc);
+  void foo(bool abc, bool cde);
+  void foo(const char *, bool abc);
+  void foo(int iabc);
+  void foo(float fabc);
+  void foo(double dabc);
+  void foo(const char *strabc);
+  void fooW(const wchar_t *wstrabc);
+  void fooPtr(A *ptrabc);
+  void foo(char chabc);
+};
+
+#define FOO 1
+
+void g(int a);
+void h(double b);
+void i(const char *c);
+
+double operator"" _km(long double);
+
+void test() {
+  A a;
+
+  a.foo(true);
+
+  a.foo(false);
+
+  a.foo(true, false);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:15: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/true, /*cde=*/false);
+
+  a.foo(false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:16: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(/*abc=*/false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:24: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(false, /*cde=*/true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  bool val1 = true;
+  bool val2 = false;
+  a.foo(val1, val2);
+
+  a.foo("", true);
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo("", /*abc=*/true);
+
+  a.foo(0);
+
+  a.foo(1.0f);
+
+  a.foo(1.0);
+
+  int val3 = 10;
+  a.foo(val3);
+
+  float val4 = 10.0;
+  a.foo(val4);
+
+  double val5 = 10.0;
+  a.foo(val5);
+
+  a.foo("Hello World");
+
+  a.fooW(L"Hello World");
+
+  a.fooPtr(nullptr);
+
+  a.foo(402.0_km);
+
+  a.foo('A');
+
+  g(FOO);
+
+  h(1.0f);
+
+  i(__FILE__);
+
+  g((1));
+}
+
+void f(bool _with_underscores_);
+void ignores_underscores() {
+  f(false);
+
+  f(true);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-argument-comment.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-argument-comment.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-argument-comment.rst
@@ -28,6 +28,9 @@
underscores and case when comparing names -- otherwise they are taken into
account.
 
+.. option:: IgnoreSingleArgument
+   When true, the check will ignore the single argument.
+
 .. option:: CommentBoolLiterals
 
When true, the check will add argument comments in the format
Index: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.h
@@ -41,6 +41,7 @@
 
 private:
   const unsigned StrictMode : 1;
+  const unsigned IgnoreSingleArgument : 1;
   const unsigned CommentBoolLiterals : 1;
   const unsigned CommentIntegerLiterals : 1;
   const unsigned CommentFloatLiterals : 1;
Index: clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -24,6 +24,7 @@
ClangTidyContext *Context)
 : ClangTidyCheck(Name, 

Re: r369760 - [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

[Hans Wennborg via cfe-commits]

Merged to release_90 in r371058.

On Fri, Aug 23, 2019 at 4:19 PM Kristof Umann via cfe-commits
 wrote:
>
> Author: szelethus
> Date: Fri Aug 23 07:21:13 2019
> New Revision: 369760
>
> URL: http://llvm.org/viewvc/llvm-project?rev=369760=rev
> Log:
> [analyzer] Avoid unnecessary enum range check on LValueToRValue casts
>
> Summary: EnumCastOutOfRangeChecker should not perform enum range checks on 
> LValueToRValue casts, since this type of cast does not actually change the 
> underlying type.   Performing the unnecessary check actually triggered an 
> assertion failure deeper in EnumCastOutOfRange for certain input (which is 
> captured in the accompanying test code).
>
> Reviewers: #clang, Szelethus, gamesh411, NoQ
>
> Reviewed By: Szelethus, gamesh411, NoQ
>
> Subscribers: NoQ, gamesh411, xazax.hun, baloghadamsoftware, szepet, 
> a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, Charusso, bjope, cfe-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D66014
>
> Added:
> cfe/trunk/test/Analysis/enum-cast-out-of-range.c
> Modified:
> cfe/trunk/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
> cfe/trunk/test/Analysis/enum-cast-out-of-range.cpp
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp?rev=369760=369759=369760=diff
> ==
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp 
> (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp Fri 
> Aug 23 07:21:13 2019
> @@ -91,6 +91,22 @@ void EnumCastOutOfRangeChecker::reportWa
>
>  void EnumCastOutOfRangeChecker::checkPreStmt(const CastExpr *CE,
>   CheckerContext ) const {
> +
> +  // Only perform enum range check on casts where such checks are valid.  For
> +  // all other cast kinds (where enum range checks are unnecessary or 
> invalid),
> +  // just return immediately.  TODO: The set of casts whitelisted for enum
> +  // range checking may be incomplete.  Better to add a missing cast kind to
> +  // enable a missing check than to generate false negatives and have to 
> remove
> +  // those later.
> +  switch (CE->getCastKind()) {
> +  case CK_IntegralCast:
> +break;
> +
> +  default:
> +return;
> +break;
> +  }
> +
>// Get the value of the expression to cast.
>const llvm::Optional ValueToCast =
>C.getSVal(CE->getSubExpr()).getAs();
>
> Added: cfe/trunk/test/Analysis/enum-cast-out-of-range.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/enum-cast-out-of-range.c?rev=369760=auto
> ==
> --- cfe/trunk/test/Analysis/enum-cast-out-of-range.c (added)
> +++ cfe/trunk/test/Analysis/enum-cast-out-of-range.c Fri Aug 23 07:21:13 2019
> @@ -0,0 +1,34 @@
> +// RUN: %clang_analyze_cc1 \
> +// RUN:   -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \
> +// RUN:   -verify %s
> +
> +enum En_t {
> +  En_0 = -4,
> +  En_1,
> +  En_2 = 1,
> +  En_3,
> +  En_4 = 4
> +};
> +
> +void unscopedUnspecifiedCStyle() {
> +  enum En_t Below = (enum En_t)(-5);// expected-warning {{not in the 
> valid range}}
> +  enum En_t NegVal1 = (enum En_t)(-4);  // OK.
> +  enum En_t NegVal2 = (enum En_t)(-3);  // OK.
> +  enum En_t InRange1 = (enum En_t)(-2); // expected-warning {{not in the 
> valid range}}
> +  enum En_t InRange2 = (enum En_t)(-1); // expected-warning {{not in the 
> valid range}}
> +  enum En_t InRange3 = (enum En_t)(0);  // expected-warning {{not in the 
> valid range}}
> +  enum En_t PosVal1 = (enum En_t)(1);   // OK.
> +  enum En_t PosVal2 = (enum En_t)(2);   // OK.
> +  enum En_t InRange4 = (enum En_t)(3);  // expected-warning {{not in the 
> valid range}}
> +  enum En_t PosVal3 = (enum En_t)(4);   // OK.
> +  enum En_t Above = (enum En_t)(5); // expected-warning {{not in the 
> valid range}}
> +}
> +
> +enum En_t unused;
> +void unusedExpr() {
> +  // Following line is not something that EnumCastOutOfRangeChecker should
> +  // evaluate.  Checker should either ignore this line or process it without
> +  // producing any warnings.  However, compilation will (and should) still
> +  // generate a warning having nothing to do with this checker.
> +  unused; // expected-warning {{expression result unused}}
> +}
>
> Modified: cfe/trunk/test/Analysis/enum-cast-out-of-range.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/enum-cast-out-of-range.cpp?rev=369760=369759=369760=diff
> ==
> --- cfe/trunk/test/Analysis/enum-cast-out-of-range.cpp (original)
> +++ cfe/trunk/test/Analysis/enum-cast-out-of-range.cpp Fri Aug 23 07:21:13 
> 2019
> @@ -150,7 +150,15 @@ void scopedSpecifiedCStyle() 

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

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added a comment.

Come to think of it, it would also not be too hard to constrain it to //only// 
be usable for a particular subset of builtins, and perhaps even only with a 
particular set of alias names for them. (I could easily derive all that 
information from the same Tablegen that `arm_mve.h` itself is made from.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67159



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

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

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66637

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/refactor/Tweak.cpp
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp
  clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -8,15 +8,26 @@
 
 #include "Annotations.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestTU.h"
 #include "TweakTesting.h"
 #include "refactor/Tweak.h"
 #include "clang/AST/Expr.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock-matchers.h"
 #include "gmock/gmock.h"
@@ -31,6 +42,27 @@
 namespace clangd {
 namespace {
 
+TEST(FileEdits, AbsolutePath) {
+  auto RelPaths = {"a.h", "foo.cpp", "test/test.cpp"};
+
+  llvm::IntrusiveRefCntPtr MemFS(
+  new llvm::vfs::InMemoryFileSystem);
+  MemFS->setCurrentWorkingDirectory(testRoot());
+  for (auto Path : RelPaths)
+MemFS->addFile(Path, 0, llvm::MemoryBuffer::getMemBuffer("", Path));
+  FileManager FM(FileSystemOptions(), MemFS);
+  DiagnosticsEngine DE(new DiagnosticIDs, new DiagnosticOptions);
+  SourceManager SM(DE, FM);
+
+  for (auto Path : RelPaths) {
+auto FID = SM.createFileID(*FM.getFile(Path), SourceLocation(),
+   clang::SrcMgr::C_User);
+auto Res = Tweak::Effect::fileEdit(SM, FID, tooling::Replacements());
+ASSERT_THAT_EXPECTED(Res, llvm::Succeeded());
+EXPECT_EQ(Res->first, testPath(Path));
+  }
+}
+
 TWEAK_TEST(SwapIfBranches);
 TEST_F(SwapIfBranchesTest, Test) {
   Context = Function;
Index: clang-tools-extra/clangd/unittests/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -9,8 +9,8 @@
 #include "TweakTesting.h"
 
 #include "Annotations.h"
-#include "refactor/Tweak.h"
 #include "SourceCode.h"
+#include "refactor/Tweak.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/Support/Error.h"
 
@@ -98,14 +98,16 @@
 return "fail: " + llvm::toString(Result.takeError());
   if (Result->ShowMessage)
 return "message:\n" + *Result->ShowMessage;
-  if (Result->ApplyEdit) {
-if (auto NewText =
-tooling::applyAllReplacements(Input.code(), *Result->ApplyEdit))
-  return unwrap(Context, *NewText);
-else
-  return "bad edits: " + llvm::toString(NewText.takeError());
-  }
-  return "no effect";
+  if (Result->ApplyEdits.empty())
+return "no effect";
+  if (Result->ApplyEdits.size() > 1)
+return "received multi-file edits";
+
+  auto ApplyEdit = Result->ApplyEdits.begin()->second;
+  if (auto NewText = ApplyEdit.apply())
+return unwrap(Context, *NewText);
+  else
+return "bad edits: " + llvm::toString(NewText.takeError());
 }
 
 ::testing::Matcher TweakTest::isAvailable() const {
Index: clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -90,7 +90,7 @@
  ElseRng->getBegin(),
  ElseCode.size(), ThenCode)))
 return std::move(Err);
-  return Effect::applyEdit(Result);
+  return Effect::mainFileEdit(SrcMgr, std::move(Result));
 }
 
 } // 

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D67159#1659084 , @simon_tatham 
wrote:

> On the general discomfort with this attribute existing: I'd be happy to lock 
> it down, or mark it as "not recommended" in some way, if that's any help. I 
> don't personally intend any use of it outside a single system header file 
> (namely `arm_mve.h`, which D67161  will 
> introduce the initial version of).


I think that would at least be something - make it print big fat undisableable 
warning if used outside of a system header.

> A warning along the lines of "don't use this!", automatically suppressed by 
> the usual change of warning settings in system headers, would seem like a 
> perfectly reasonable precaution, for example.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67159



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

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

[Simon Tatham via Phabricator via cfe-commits]

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

On the general discomfort with this attribute existing: I'd be happy to lock it 
down, or mark it as "not recommended" in some way, if that's any help. I don't 
personally intend any use of it outside a single system header file (namely 
`arm_mve.h`, which D67161  will introduce the 
initial version of).

A warning along the lines of "don't use this!", automatically suppressed by the 
usual change of warning settings in system headers, would seem like a perfectly 
reasonable precaution, for example.




Comment at: clang/include/clang/Basic/Attr.td:596
 }
+def ClangBuiltinOverride : Attr {
+  let Spellings = [GCC<"__clang_builtin">];

aaron.ballman wrote:
> Do you expect this attribute to be inherited on redeclarations? I suspect 
> this should be an `InheritableAttr`.
> 
> Also, add a newline above it for visual separation, please.
> 
> Finally, should this be a target-specific attribute so that it's only 
> available for your target, or do you expect this attribute to be used on all 
> target architectures?
For my use case, I have no opinion about redeclarations: I expect to declare 
each affected function exactly once. If you think `InheritableAttr` is a more 
sensible default choice, I'm fine with that.

Target-specific: I don't have a use case outside the ARM target, so I'd be 
happy to lock it down that way if you want.



Comment at: clang/include/clang/Basic/Attr.td:600
+  let Subjects = SubjectList<[Function], ErrorDiag>;
+  let Documentation = [Undocumented];
+}

aaron.ballman wrote:
> No new undocumented attributes, please.
OK. I'd intended to leave it undocumented in order to discourage people from 
using it in any context //other// than a system header file. But fair enough – 
perhaps it should be documented even so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67159



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

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

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66637



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

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

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Define more strict semantics around filename


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66637

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/refactor/Tweak.cpp
  clang-tools-extra/clangd/refactor/Tweak.h
  clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp
  clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -7,16 +7,28 @@
 //===--===//
 
 #include "Annotations.h"
+#include "ClangdUnit.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestTU.h"
 #include "TweakTesting.h"
 #include "refactor/Tweak.h"
 #include "clang/AST/Expr.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock-matchers.h"
 #include "gmock/gmock.h"
@@ -113,11 +125,15 @@
   auto Effect = apply(ID, Input);
   if (!Effect)
 return Effect.takeError();
-  if (!Effect->ApplyEdit)
+  if (Effect->ApplyEdits.empty())
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No replacements");
+  auto Edits = Effect->ApplyEdits;
+  if (Edits.size() > 1)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "Multi-file edits");
   Annotations Code(Input);
-  return applyAllReplacements(Code.code(), *Effect->ApplyEdit);
+  return applyAllReplacements(Code.code(), Edits.begin()->second.Replacements);
 }
 
 void checkTransform(llvm::StringRef ID, llvm::StringRef Input,
@@ -127,6 +143,27 @@
   EXPECT_EQ(Output, std::string(*Result)) << Input;
 }
 
+TEST(FileEdits, AbsolutePath) {
+  auto RelPaths = {"a.h", "foo.cpp", "test/test.cpp"};
+
+  llvm::IntrusiveRefCntPtr MemFS(
+  new llvm::vfs::InMemoryFileSystem);
+  MemFS->setCurrentWorkingDirectory(testRoot());
+  for (auto Path : RelPaths)
+MemFS->addFile(Path, 0, llvm::MemoryBuffer::getMemBuffer("", Path));
+  FileManager FM(FileSystemOptions(), MemFS);
+  DiagnosticsEngine DE(new DiagnosticIDs, new DiagnosticOptions);
+  SourceManager SM(DE, FM);
+
+  for (auto Path : RelPaths) {
+auto FID = SM.createFileID(*FM.getFile(Path), SourceLocation(),
+   clang::SrcMgr::C_User);
+auto Res = Tweak::Effect::fileEdit(SM, FID, tooling::Replacements());
+ASSERT_THAT_EXPECTED(Res, Succeeded());
+EXPECT_EQ(Res->first, testPath(Path));
+  }
+}
+
 TWEAK_TEST(SwapIfBranches);
 TEST_F(SwapIfBranchesTest, Test) {
   Context = Function;
@@ -495,7 +532,7 @@
   checkTransform(ID, Input, Output);
 
   checkTransform(ID,
-  R"cpp(
+ R"cpp(
 [[void f1();
 void f2();]]
 )cpp",
Index: clang-tools-extra/clangd/unittests/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -9,8 +9,8 @@
 #include "TweakTesting.h"
 
 #include "Annotations.h"
-#include "refactor/Tweak.h"
 #include "SourceCode.h"
+#include "refactor/Tweak.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/Support/Error.h"
 
@@ -98,14 +98,16 @@
 return "fail: " + llvm::toString(Result.takeError());
   if (Result->ShowMessage)
 return "message:\n" + *Result->ShowMessage;
-  if (Result->ApplyEdit) {
-if (auto NewText =
-tooling::applyAllReplacements(Input.code(), *Result->ApplyEdit))
-  return unwrap(Context, *NewText);
-else
-  return "bad edits: " + 

[PATCH] D64736: [clang-tidy] New bugprone-infinite-loop check for detecting obvious infinite loops

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

baloghadamsoftware updated this revision to Diff 218876.
baloghadamsoftware added a comment.
Herald added a subscriber: srhines.
Herald added a reviewer: jdoerfert.

Updated according to the comments.


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

https://reviews.llvm.org/D64736

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang-tidy/bugprone/InfiniteLoopCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-infinite-loop.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-infinite-loop.cpp

Index: test/clang-tidy/bugprone-infinite-loop.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-infinite-loop.cpp
@@ -0,0 +1,298 @@
+// RUN: %check_clang_tidy %s bugprone-infinite-loop %t
+
+void simple_infinite_loop1() {
+  int i = 0;
+  int j = 0;
+  while (i < 10) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+j++;
+  }
+
+  do {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+j++;
+  } while (i < 10);
+
+  for (i = 0; i < 10; ++j) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+  }
+}
+
+void simple_infinite_loop2() {
+  int i = 0;
+  int j = 0;
+  int Limit = 10;
+  while (i < Limit) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]
+j++;
+  }
+
+  do {
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]
+j++;
+  } while (i < Limit);
+
+  for (i = 0; i < Limit; ++j) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i, Limit) are updated in the loop body [bugprone-infinite-loop]
+  }
+}
+
+void simple_not_infinite1() {
+  int i = 0;
+  int Limit = 100;
+  while (i < Limit) {
+// Not an error since 'Limit' is updated.
+Limit--;
+  }
+  do {
+Limit--;
+  } while (i < Limit);
+
+  for (i = 0; i < Limit; Limit--) {
+  }
+}
+
+void simple_not_infinite2() {
+  for (int i = 10; i-- > 0;) {
+// Not an error, since loop variable is modified in its condition part.
+  }
+}
+
+int unknown_function();
+
+void function_call() {
+  int i = 0;
+  while (i < unknown_function()) {
+// Not an error, since the function may return different values.
+  }
+
+  do {
+// Not an error, since the function may return different values.
+  } while (i < unknown_function());
+
+  for (i = 0; i < unknown_function();) {
+// Not an error, since the function may return different values.
+  }
+}
+
+void escape_before1() {
+  int i = 0;
+  int Limit = 100;
+  int *p = 
+  while (i < Limit) {
+// Not an error, since *p is alias of i.
+(*p)++;
+  }
+
+  do {
+(*p)++;
+  } while (i < Limit);
+
+  for (i = 0; i < Limit; ++(*p)) {
+  }
+}
+
+void escape_before2() {
+  int i = 0;
+  int Limit = 100;
+  int  = i;
+  while (i < Limit) {
+// Not an error, since ii is alias of i.
+ii++;
+  }
+
+  do {
+ii++;
+  } while (i < Limit);
+
+  for (i = 0; i < Limit; ++ii) {
+  }
+}
+
+void escape_inside1() {
+  int i = 0;
+  int Limit = 100;
+  int *p = 
+  while (i < Limit) {
+// Not an error, since *p is alias of i.
+int *p = 
+(*p)++;
+  }
+
+  do {
+int *p = 
+(*p)++;
+  } while (i < Limit);
+}
+
+void escape_inside2() {
+  int i = 0;
+  int Limit = 100;
+  while (i < Limit) {
+// Not an error, since ii is alias of i.
+int  = i;
+ii++;
+  }
+
+  do {
+int  = i;
+ii++;
+  } while (i < Limit);
+}
+
+int glob;
+
+void global1(int ) {
+  int i = 0, Limit = 100;
+  while (x < Limit) {
+// Not an error since 'x' can be an alias of 'glob'.
+glob++;
+  }
+}
+
+void global2() {
+  int i = 0, Limit = 100;
+  while (glob < Limit) {
+// Since 'glob' is declared out of the function we do not warn.
+i++;
+  }
+}
+
+struct X {
+  int m;
+
+  void change_m();
+
+  void member_expr1(int i) {
+while (i < m) {
+  // False negative: No warning, since skipping the case where a struct or
+  // class can be found in its condition.
+  ;
+}
+  }
+
+  void member_expr2(int i) {
+while (i < m) {
+  --m;
+}
+  }
+
+  void member_expr3(int i) {
+while (i < m) {
+  change_m();
+}
+  }
+};
+
+void array_index() {
+  int i = 0;
+  int v[10];
+  while (i < 10) {
+v[i++] = 0;
+  }
+
+  i = 0;
+  do {
+v[i++] = 0;
+  } while (i < 9);
+
+  for (i = 0; i < 10;) {
+v[i++] = 0;
+  }
+
+  for (i = 0; 

r371046 - [OpenCL] Add image type handling for builtins

[Sven van Haastregt via cfe-commits]

Author: svenvh
Date: Thu Sep  5 03:01:24 2019
New Revision: 371046

URL: http://llvm.org/viewvc/llvm-project?rev=371046=rev
Log:
[OpenCL] Add image type handling for builtins

Image types were previously available, but not working.  This patch
adds image type handling.

Rename the image type definitions in the .td file to make them
consistent with other type names.  Use abstract types to represent the
unqualified types.  Instantiate access-qualified image types at the
point of use using, e.g. `ImageType`.

Add/update TableGen definitions for the read_image/write_image
builtin functions.

Patch by Pierre Gondois and Sven van Haastregt.

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

Modified:
cfe/trunk/lib/Sema/OpenCLBuiltins.td
cfe/trunk/test/SemaOpenCL/fdeclare-opencl-builtins.cl
cfe/trunk/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

Modified: cfe/trunk/lib/Sema/OpenCLBuiltins.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/OpenCLBuiltins.td?rev=371046=371045=371046=diff
==
--- cfe/trunk/lib/Sema/OpenCLBuiltins.td (original)
+++ cfe/trunk/lib/Sema/OpenCLBuiltins.td Thu Sep  5 03:01:24 2019
@@ -87,11 +87,11 @@ class Type : Type<_Ty.Name, _Ty.QTName> {
   let VecWidth = _VecWidth;
+  let AccessQualifier = "";
   // Inherited fields
   let IsPointer = _Ty.IsPointer;
   let IsConst = _Ty.IsConst;
   let IsVolatile = _Ty.IsVolatile;
-  let AccessQualifier = _Ty.AccessQualifier;
   let AddrSpace = _Ty.AddrSpace;
 }
 
@@ -129,10 +129,16 @@ class VolatileType : Type<_Ty.
   let AddrSpace = _Ty.AddrSpace;
 }
 
-// OpenCL image types (e.g. image2d_t, ...)
-class ImageType :
-  Type<_Ty.Name, _QTName> {
+// OpenCL image types (e.g. image2d).
+class ImageType :
+  Type<_Ty.Name, QualType<_Ty.QTName.Name#_AccessQualifier#"Ty", 0>> {
+  let VecWidth = 0;
   let AccessQualifier = _AccessQualifier;
+  // Inherited fields
+  let IsPointer = _Ty.IsPointer;
+  let IsConst = _Ty.IsConst;
+  let IsVolatile = _Ty.IsVolatile;
+  let AddrSpace = _Ty.AddrSpace;
 }
 
 // List of Types.
@@ -221,37 +227,21 @@ def Void  : Type<"void_t",QualTy
 // OpenCL v1.0/1.2/2.0 s6.1.2: Built-in Vector Data Types.
 // Built-in vector data types are created by TableGen's OpenCLBuiltinEmitter.
 
-// OpenCL v1.2 s6.1.3: Other Built-in Data Types
-// These definitions with a "null" name are "abstract". They should not
-// be used in definitions of Builtin functions.
-def image2d_t : Type<"image2d_t", QualType<"null", 1>>;
-def image3d_t : Type<"image3d_t", QualType<"null", 1>>;
-def image2d_array_t   : Type<"image2d_array_t", QualType<"null", 1>>;
-def image1d_t : Type<"image1d_t", QualType<"null", 1>>;
-def image1d_buffer_t  : Type<"image1d_buffer_t", QualType<"null", 1>>;
-def image1d_array_t   : Type<"image1d_array_t", QualType<"null", 1>>;
-// Unlike the few functions above, the following definitions can be used
-// in definitions of Builtin functions (they have a QualType with a name).
-foreach v = ["RO", "WO", "RW"] in {
-  def image2d_#v#_t   : ImageType,
-  v>;
-  def image3d_#v#_t   : ImageType,
-  v>;
-  def image2d_array#v#_t  : ImageType,
-  v>;
-  def image1d_#v#_t   : ImageType,
-  v>;
-  def image1d_buffer#v#_t : ImageType,
-  v>;
-  def image1d_array#v#_t  : ImageType,
-  v>;
-}
+// OpenCL v1.0/1.2/2.0 s6.1.3: Other Built-in Data Types.
+// The image definitions are "abstract".  They should not be used without
+// specifying an access qualifier (RO/WO/RW).
+def Image1d   : Type<"Image1d", QualType<"OCLImage1d", 1>>;
+def Image2d   : Type<"Image2d", QualType<"OCLImage2d", 1>>;
+def Image3d   : Type<"Image3d", QualType<"OCLImage3d", 1>>;
+def Image1dArray  : Type<"Image1dArray", QualType<"OCLImage1dArray", 
1>>;
+def Image1dBuffer : Type<"Image1dBuffer", QualType<"OCLImage1dBuffer", 
1>>;
+def Image2dArray  : Type<"Image2dArray", QualType<"OCLImage2dArray", 
1>>;
+def Image2dDepth  : Type<"Image2dDepth", QualType<"OCLImage2dDepth", 
1>>;
+def Image2dArrayDepth : Type<"Image2dArrayDepth", 
QualType<"OCLImage2dArrayDepth", 1>>;
+def Image2dMsaa   : Type<"Image2dMsaa", QualType<"OCLImage2dMSAA", 1>>;
+def Image2dArrayMsaa  : Type<"Image2dArrayMsaa", 
QualType<"OCLImage2dArrayMSAA", 1>>;
+def Image2dMsaaDepth  : Type<"Image2dMsaaDepth", 
QualType<"OCLImage2dMSAADepth", 1>>;
+def Image2dArrayMsaaDepth : Type<"Image2dArrayMsaaDepth", 
QualType<"OCLImage2dArrayMSAADepth", 1>>;
 
 def Sampler   : Type<"Sampler", QualType<"OCLSamplerTy">>;
 def Event : Type<"Event", QualType<"OCLEventTy">>;
@@ -398,14 +388,132 @@ foreach 

[PATCH] D67216: [cfi] Add flag to always generate call frame information

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

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

Does the name of the `-falways-need-cfi` option match any existing compiler 
(google doesn't find anything)? If not, I think it would make more sense as a 
`-g` option, as it's similar to `-gline-tables-only`.




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:956
  OPT_fno_unique_section_names, true);
+  Opts.AlwaysNeedCFI =
+  Args.hasFlag(OPT_falways_need_cfi, OPT_fno_always_need_cfi, false);

Is this option actually being read anywhere?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67216



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

[PATCH] D67077: [libclang] Refactored SharedParsedRegionsStorage

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371041: [libclang] Refactored SharedParsedRegionsStorage 
(authored by gribozavr, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67077?vs=218346=218872#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67077

Files:
  cfe/trunk/tools/libclang/Indexing.cpp

Index: cfe/trunk/tools/libclang/Indexing.cpp
===
--- cfe/trunk/tools/libclang/Indexing.cpp
+++ cfe/trunk/tools/libclang/Indexing.cpp
@@ -88,8 +88,6 @@
   }
 };
 
-typedef llvm::DenseSet PPRegionSetTy;
-
 } // end anonymous namespace
 
 namespace llvm {
@@ -124,20 +122,20 @@
 /// Keeps track of function bodies that have already been parsed.
 ///
 /// Is thread-safe.
-class SharedParsedRegionsStorage {
-  std::mutex Mux;
-  PPRegionSetTy ParsedRegions;
+class ThreadSafeParsedRegions {
+  mutable std::mutex Mutex;
+  llvm::DenseSet ParsedRegions;
 
 public:
-  ~SharedParsedRegionsStorage() = default;
+  ~ThreadSafeParsedRegions() = default;
 
-  void copyTo(PPRegionSetTy ) {
-std::lock_guard MG(Mux);
-Set = ParsedRegions;
+  llvm::DenseSet getParsedRegions() const {
+std::lock_guard MG(Mutex);
+return ParsedRegions;
   }
 
-  void merge(ArrayRef Regions) {
-std::lock_guard MG(Mux);
+  void addParsedRegions(ArrayRef Regions) {
+std::lock_guard MG(Mutex);
 ParsedRegions.insert(Regions.begin(), Regions.end());
   }
 };
@@ -147,13 +145,13 @@
 ///
 /// Is NOT thread-safe.
 class ParsedSrcLocationsTracker {
-  SharedParsedRegionsStorage 
+  ThreadSafeParsedRegions 
   PPConditionalDirectiveRecord 
   Preprocessor 
 
   /// Snapshot of the shared state at the point when this instance was
   /// constructed.
-  PPRegionSetTy ParsedRegions;
+  llvm::DenseSet ParsedRegionsSnapshot;
   /// Regions that were queried during this instance lifetime.
   SmallVector NewParsedRegions;
 
@@ -163,12 +161,11 @@
 
 public:
   /// Creates snapshot of \p ParsedRegionsStorage.
-  ParsedSrcLocationsTracker(SharedParsedRegionsStorage ,
+  ParsedSrcLocationsTracker(ThreadSafeParsedRegions ,
 PPConditionalDirectiveRecord ,
 Preprocessor )
-  : ParsedRegionsStorage(ParsedRegionsStorage), PPRec(ppRec), PP(pp) {
-ParsedRegionsStorage.copyTo(ParsedRegions);
-  }
+  : ParsedRegionsStorage(ParsedRegionsStorage), PPRec(ppRec), PP(pp),
+ParsedRegionsSnapshot(ParsedRegionsStorage.getParsedRegions()) {}
 
   /// \returns true iff \p Loc has already been parsed.
   ///
@@ -190,14 +187,16 @@
 // That means if we hit the same region again, it's a different location in
 // the same region and so the "is parsed" value from the snapshot is still
 // correct.
-LastIsParsed = ParsedRegions.count(region);
+LastIsParsed = ParsedRegionsSnapshot.count(region);
 if (!LastIsParsed)
   NewParsedRegions.emplace_back(std::move(region));
 return LastIsParsed;
   }
 
   /// Updates ParsedRegionsStorage with newly parsed regions.
-  void syncWithStorage() { ParsedRegionsStorage.merge(NewParsedRegions); }
+  void syncWithStorage() {
+ParsedRegionsStorage.addParsedRegions(NewParsedRegions);
+  }
 
 private:
   PPRegion getRegion(SourceLocation Loc, FileID FID, const FileEntry *FE) {
@@ -336,13 +335,13 @@
   std::shared_ptr DataConsumer;
   IndexingOptions Opts;
 
-  SharedParsedRegionsStorage *SKData;
+  ThreadSafeParsedRegions *SKData;
   std::unique_ptr ParsedLocsTracker;
 
 public:
   IndexingFrontendAction(std::shared_ptr dataConsumer,
  const IndexingOptions ,
- SharedParsedRegionsStorage *skData)
+ ThreadSafeParsedRegions *skData)
   : DataConsumer(std::move(dataConsumer)), Opts(Opts), SKData(skData) {}
 
   std::unique_ptr CreateASTConsumer(CompilerInstance ,
@@ -431,10 +430,10 @@
 
 struct IndexSessionData {
   CXIndex CIdx;
-  std::unique_ptr SkipBodyData;
+  std::unique_ptr SkipBodyData =
+  std::make_unique();
 
-  explicit IndexSessionData(CXIndex cIdx)
-  : CIdx(cIdx), SkipBodyData(new SharedParsedRegionsStorage) {}
+  explicit IndexSessionData(CXIndex cIdx) : CIdx(cIdx) {}
 };
 
 } // anonymous namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r371041 - [libclang] Refactored SharedParsedRegionsStorage

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Thu Sep  5 02:48:39 2019
New Revision: 371041

URL: http://llvm.org/viewvc/llvm-project?rev=371041=rev
Log:
[libclang] Refactored SharedParsedRegionsStorage

Summary:
Removed the `PPRegionSetTy` typedef because it is only used 3 times, and
obscures code more than it helps.

Renamed SharedParsedRegionsStorage to ThreadSafeParsedRegions, because
that better reflects the reason for this type to exist.

Replaced the `copyTo()` method that had an out parameter with a getter.

Renamed the `merge()` method to `addParsedRegions()`.

Renamed `ParsedSrcLocationsTracker::ParsedRegions` to
`ParsedRegionsSnapshot`, which better reflects its role.

Subscribers: arphaman, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/tools/libclang/Indexing.cpp

Modified: cfe/trunk/tools/libclang/Indexing.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/Indexing.cpp?rev=371041=371040=371041=diff
==
--- cfe/trunk/tools/libclang/Indexing.cpp (original)
+++ cfe/trunk/tools/libclang/Indexing.cpp Thu Sep  5 02:48:39 2019
@@ -88,8 +88,6 @@ public:
   }
 };
 
-typedef llvm::DenseSet PPRegionSetTy;
-
 } // end anonymous namespace
 
 namespace llvm {
@@ -124,20 +122,20 @@ namespace {
 /// Keeps track of function bodies that have already been parsed.
 ///
 /// Is thread-safe.
-class SharedParsedRegionsStorage {
-  std::mutex Mux;
-  PPRegionSetTy ParsedRegions;
+class ThreadSafeParsedRegions {
+  mutable std::mutex Mutex;
+  llvm::DenseSet ParsedRegions;
 
 public:
-  ~SharedParsedRegionsStorage() = default;
+  ~ThreadSafeParsedRegions() = default;
 
-  void copyTo(PPRegionSetTy ) {
-std::lock_guard MG(Mux);
-Set = ParsedRegions;
+  llvm::DenseSet getParsedRegions() const {
+std::lock_guard MG(Mutex);
+return ParsedRegions;
   }
 
-  void merge(ArrayRef Regions) {
-std::lock_guard MG(Mux);
+  void addParsedRegions(ArrayRef Regions) {
+std::lock_guard MG(Mutex);
 ParsedRegions.insert(Regions.begin(), Regions.end());
   }
 };
@@ -147,13 +145,13 @@ public:
 ///
 /// Is NOT thread-safe.
 class ParsedSrcLocationsTracker {
-  SharedParsedRegionsStorage 
+  ThreadSafeParsedRegions 
   PPConditionalDirectiveRecord 
   Preprocessor 
 
   /// Snapshot of the shared state at the point when this instance was
   /// constructed.
-  PPRegionSetTy ParsedRegions;
+  llvm::DenseSet ParsedRegionsSnapshot;
   /// Regions that were queried during this instance lifetime.
   SmallVector NewParsedRegions;
 
@@ -163,12 +161,11 @@ class ParsedSrcLocationsTracker {
 
 public:
   /// Creates snapshot of \p ParsedRegionsStorage.
-  ParsedSrcLocationsTracker(SharedParsedRegionsStorage ,
+  ParsedSrcLocationsTracker(ThreadSafeParsedRegions ,
 PPConditionalDirectiveRecord ,
 Preprocessor )
-  : ParsedRegionsStorage(ParsedRegionsStorage), PPRec(ppRec), PP(pp) {
-ParsedRegionsStorage.copyTo(ParsedRegions);
-  }
+  : ParsedRegionsStorage(ParsedRegionsStorage), PPRec(ppRec), PP(pp),
+ParsedRegionsSnapshot(ParsedRegionsStorage.getParsedRegions()) {}
 
   /// \returns true iff \p Loc has already been parsed.
   ///
@@ -190,14 +187,16 @@ public:
 // That means if we hit the same region again, it's a different location in
 // the same region and so the "is parsed" value from the snapshot is still
 // correct.
-LastIsParsed = ParsedRegions.count(region);
+LastIsParsed = ParsedRegionsSnapshot.count(region);
 if (!LastIsParsed)
   NewParsedRegions.emplace_back(std::move(region));
 return LastIsParsed;
   }
 
   /// Updates ParsedRegionsStorage with newly parsed regions.
-  void syncWithStorage() { ParsedRegionsStorage.merge(NewParsedRegions); }
+  void syncWithStorage() {
+ParsedRegionsStorage.addParsedRegions(NewParsedRegions);
+  }
 
 private:
   PPRegion getRegion(SourceLocation Loc, FileID FID, const FileEntry *FE) {
@@ -336,13 +335,13 @@ class IndexingFrontendAction : public AS
   std::shared_ptr DataConsumer;
   IndexingOptions Opts;
 
-  SharedParsedRegionsStorage *SKData;
+  ThreadSafeParsedRegions *SKData;
   std::unique_ptr ParsedLocsTracker;
 
 public:
   IndexingFrontendAction(std::shared_ptr dataConsumer,
  const IndexingOptions ,
- SharedParsedRegionsStorage *skData)
+ ThreadSafeParsedRegions *skData)
   : DataConsumer(std::move(dataConsumer)), Opts(Opts), SKData(skData) {}
 
   std::unique_ptr CreateASTConsumer(CompilerInstance ,
@@ -431,10 +430,10 @@ static IndexingOptions getIndexingOption
 
 struct IndexSessionData {
   CXIndex CIdx;
-  std::unique_ptr SkipBodyData;
+  std::unique_ptr SkipBodyData =
+  std::make_unique();
 
-  explicit IndexSessionData(CXIndex cIdx)
-  : CIdx(cIdx), SkipBodyData(new SharedParsedRegionsStorage) {}
+  explicit IndexSessionData(CXIndex 

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

[Hans Wennborg via cfe-commits]

Merged to release_90 in r371040.

On Wed, Sep 4, 2019 at 10:17 AM Hans Wennborg via cfe-commits
 wrote:
>
> Author: hans
> Date: Wed Sep  4 01:19:30 2019
> New Revision: 370850
>
> URL: http://llvm.org/viewvc/llvm-project?rev=370850=rev
> Log:
> Re-commit r363191 "[MS] Pretend constexpr variable template specializations 
> are inline"
>
> While the next Visual Studio update (16.3) will fix this issue, that hasn't
> shipped yet. Until then Clang wouldn't work with MSVC's headers which seems
> unfortunate. Let's keep this in until VS 16.3 ships. (See also PR42843.)
>
> > Fixes link errors with clang and the latest Visual C++ 14.21.27702
> > headers, which was reported as PR42027.
> >
> > I chose to intentionally make these things linkonce_odr, i.e.
> > discardable, so that we don't emit definitions of these things in every
> > translation unit that includes STL headers.
> >
> > Note that this is *not* what MSVC does: MSVC has not yet implemented C++
> > DR2387, so they emit fully specialized constexpr variable templates with
> > static / internal linkage.
> >
> > Reviewers: rsmith
> >
> > Differential Revision: https://reviews.llvm.org/D63175
>
> Added:
> cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp
> Modified:
> cfe/trunk/lib/AST/ASTContext.cpp
>
> Modified: cfe/trunk/lib/AST/ASTContext.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=370850=370849=370850=diff
> ==
> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
> +++ cfe/trunk/lib/AST/ASTContext.cpp Wed Sep  4 01:19:30 2019
> @@ -9905,10 +9905,25 @@ static GVALinkage basicGVALinkageForVari
>  return StrongLinkage;
>
>case TSK_ExplicitSpecialization:
> -return Context.getTargetInfo().getCXXABI().isMicrosoft() &&
> -   VD->isStaticDataMember()
> -   ? GVA_StrongODR
> -   : StrongLinkage;
> +if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
> +  // If this is a fully specialized constexpr variable template, pretend 
> it
> +  // was marked inline. MSVC 14.21.27702 headers define _Is_integral in a
> +  // header this way, and we don't want to emit non-discardable 
> definitions
> +  // of these variables in every TU that includes . This
> +  // behavior is non-conforming, since another TU could use an extern
> +  // template declaration for this variable, but for constexpr variables,
> +  // it's unlikely for a user to want to do that. This behavior can be
> +  // removed if the headers change to explicitly mark such variable 
> template
> +  // specializations inline.
> +  if (isa(VD) && VD->isConstexpr())
> +return GVA_DiscardableODR;
> +
> +  // Use ODR linkage for static data members of fully specialized 
> templates
> +  // to prevent duplicate definition errors with MSVC.
> +  if (VD->isStaticDataMember())
> +return GVA_StrongODR;
> +}
> +return StrongLinkage;
>
>case TSK_ExplicitInstantiationDefinition:
>  return GVA_StrongODR;
>
> Added: cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp?rev=370850=auto
> ==
> --- cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp (added)
> +++ cfe/trunk/test/CodeGenCXX/ms-constexpr-var-template.cpp Wed Sep  4 
> 01:19:30 2019
> @@ -0,0 +1,11 @@
> +// RUN: %clang_cc1 -emit-llvm -triple=x86_64-windows-msvc -fms-compatibility 
> %s -o - | FileCheck %s
> +
> +template  constexpr bool _Is_integer = false;
> +template <> constexpr bool _Is_integer = true;
> +template <> constexpr bool _Is_integer = false;
> +extern "C" const bool *escape = &_Is_integer;
> +
> +// CHECK: @"??$_Is_integer@H@@3_NB" = linkonce_odr dso_local constant i8 1, 
> comdat, align 1
> +//   Should not emit _Is_integer, since it's not referenced.
> +// CHECK-NOT: @"??$_Is_integer@D@@3_NB"
> +// CHECK: @escape = dso_local global i8* @"??$_Is_integer@H@@3_NB", align 8
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371038: [clangd][vscode] Add a flag to enable semantic 
highlighting in clangd (authored by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67096?vs=218864=218865#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67096

Files:
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts


Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
@@ -109,12 +109,14 @@
 
   const clangdClient = new ClangdLanguageClient('Clang Language Server',
 serverOptions, clientOptions);
-  const semanticHighlightingFeature =
+  if (getConfig('semanticHighlighting')) {
+const semanticHighlightingFeature =
   new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
-   context);
-  context.subscriptions.push(
+context);
+context.subscriptions.push(
   vscode.Disposable.from(semanticHighlightingFeature));
-  clangdClient.registerFeature(semanticHighlightingFeature);
+clangdClient.registerFeature(semanticHighlightingFeature);
+  }
   console.log('Clang Language Server is now active!');
   context.subscriptions.push(clangdClient.start());
   context.subscriptions.push(vscode.commands.registerCommand(
Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
@@ -89,6 +89,11 @@
 "clangd.trace": {
 "type": "string",
 "description": "Names a file that clangd should log a 
performance trace to, in chrome trace-viewer JSON format."
+},
+"clangd.semanticHighlighting": {
+"type": "boolean",
+"default": "false",
+"description": "Enable semantic highlighting in clangd"
 }
 }
 },


Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
@@ -109,12 +109,14 @@
 
   const clangdClient = new ClangdLanguageClient('Clang Language Server',
 serverOptions, clientOptions);
-  const semanticHighlightingFeature =
+  if (getConfig('semanticHighlighting')) {
+const semanticHighlightingFeature =
   new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
-   context);
-  context.subscriptions.push(
+context);
+context.subscriptions.push(
   vscode.Disposable.from(semanticHighlightingFeature));
-  clangdClient.registerFeature(semanticHighlightingFeature);
+clangdClient.registerFeature(semanticHighlightingFeature);
+  }
   console.log('Clang Language Server is now active!');
   context.subscriptions.push(clangdClient.start());
   context.subscriptions.push(vscode.commands.registerCommand(
Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
@@ -89,6 +89,11 @@
 "clangd.trace": {
 "type": "string",
 "description": "Names a file that clangd should log a performance trace to, in chrome trace-viewer JSON format."
+},
+"clangd.semanticHighlighting": {
+"type": "boolean",
+"default": "false",
+"description": "Enable semantic highlighting in clangd"
 }
 }
 },
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67213: [clang-tidy] Fix definitions in headers check to respect qualifiers

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

thanks for the fix.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67213



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

[clang-tools-extra] r371038 - [clangd][vscode] Add a flag to enable semantic highlighting in clangd

[Haojian Wu via cfe-commits]

Author: hokein
Date: Thu Sep  5 02:26:03 2019
New Revision: 371038

URL: http://llvm.org/viewvc/llvm-project?rev=371038=rev
Log:
[clangd][vscode] Add a flag to enable semantic highlighting in clangd

Reviewers: ilya-biryukov

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=371038=371037=371038=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Thu Sep  
5 02:26:03 2019
@@ -89,6 +89,11 @@
 "clangd.trace": {
 "type": "string",
 "description": "Names a file that clangd should log a 
performance trace to, in chrome trace-viewer JSON format."
+},
+"clangd.semanticHighlighting": {
+"type": "boolean",
+"default": "false",
+"description": "Enable semantic highlighting in clangd"
 }
 }
 },

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts?rev=371038=371037=371038=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts 
(original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts Thu 
Sep  5 02:26:03 2019
@@ -109,12 +109,14 @@ export function activate(context: vscode
 
   const clangdClient = new ClangdLanguageClient('Clang Language Server',
 serverOptions, clientOptions);
-  const semanticHighlightingFeature =
+  if (getConfig('semanticHighlighting')) {
+const semanticHighlightingFeature =
   new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
-   context);
-  context.subscriptions.push(
+context);
+context.subscriptions.push(
   vscode.Disposable.from(semanticHighlightingFeature));
-  clangdClient.registerFeature(semanticHighlightingFeature);
+clangdClient.registerFeature(semanticHighlightingFeature);
+  }
   console.log('Clang Language Server is now active!');
   context.subscriptions.push(clangdClient.start());
   context.subscriptions.push(vscode.commands.registerCommand(


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

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

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67096



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

[PATCH] D67216: [cfi] Add flag to always generate call frame information

[David Candler via Phabricator via cfe-commits]

dcandler created this revision.
dcandler added reviewers: echristo, probinson, aprantl.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, kristof.beyls, 
javed.absar.
Herald added projects: clang, LLVM.

This adds a flag to LLVM and clang to always generate call frame information, 
even if other debug information is not being generated. This would be useful 
for the Arm toolchain where the .debug_frame section is always expected to be 
present (with or without other debug sections) and can be used to calculate 
stack usage, although the flag itself has been left generic to cover any other 
potential situations where cfi directives are desired, but other debug 
information is not.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67216

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/always-need-cfi.c
  llvm/docs/CommandGuide/llc.rst
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/test/CodeGen/ARM/always-need-cfi.ll

Index: llvm/test/CodeGen/ARM/always-need-cfi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/always-need-cfi.ll
@@ -0,0 +1,38 @@
+; RUN: llc -mtriple armv7-unknown -frame-pointer=all -filetype=asm -o - %s | FileCheck %s --check-prefix=CHECK-NO-CFI
+; RUN: llc -mtriple armv7-unknown -frame-pointer=all -filetype=asm -always-need-cfi -o - %s | FileCheck %s --check-prefix=CHECK-ALWAYS-CFI
+
+declare void @dummy_use(i32*, i32)
+
+define void @test_basic() #0 {
+%mem = alloca i32, i32 10
+call void @dummy_use (i32* %mem, i32 10)
+	ret void
+}
+
+; CHECK-NO-CFI-LABEL: test_basic:
+; CHECK-NO-CFI:   .fnstart
+; CHECK-NO-CFI-NOT:   .cfi_sections .debug_frame
+; CHECK-NO-CFI-NOT:   .cfi_startproc
+; CHECK-NO-CFI:   @ %bb.0:
+; CHECK-NO-CFI:   push {r11, lr}
+; CHECK-NO-CFI-NOT:   .cfi_def_cfa_offset 8
+; CHECK-NO-CFI-NOT:   .cfi_offset lr, -4
+; CHECK-NO-CFI-NOT:   .cfi_offset r11, -8
+; CHECK-NO-CFI:   mov r11, sp
+; CHECK-NO-CFI-NOT:   .cfi_def_cfa_register r11
+; CHECK-NO-CFI-NOT:   .cfi_endproc
+; CHECK-NO-CFI:   .fnend
+
+; CHECK-ALWAYS-CFI-LABEL: test_basic:
+; CHECK-ALWAYS-CFI:   .fnstart
+; CHECK-ALWAYS-CFI:   .cfi_sections .debug_frame
+; CHECK-ALWAYS-CFI:   .cfi_startproc
+; CHECK-ALWAYS-CFI:   @ %bb.0:
+; CHECK-ALWAYS-CFI:   push {r11, lr}
+; CHECK-ALWAYS-CFI:   .cfi_def_cfa_offset 8
+; CHECK-ALWAYS-CFI:   .cfi_offset lr, -4
+; CHECK-ALWAYS-CFI:   .cfi_offset r11, -8
+; CHECK-ALWAYS-CFI:   mov r11, sp
+; CHECK-ALWAYS-CFI:   .cfi_def_cfa_register r11
+; CHECK-ALWAYS-CFI:   .cfi_endproc
+; CHECK-ALWAYS-CFI:   .fnend
Index: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -149,6 +149,11 @@
 cl::desc("Emit a section containing remark diagnostics metadata"),
 cl::init(false));
 
+static cl::opt AlwaysNeedCFI(
+"always-need-cfi",
+cl::desc("Always emit call frame information"),
+cl::init(false));
+
 char AsmPrinter::ID = 0;
 
 using gcp_map_type = DenseMap>;
@@ -929,7 +934,7 @@
   MF->getFunction().needsUnwindTableEntry())
 return CFI_M_EH;
 
-  if (MMI->hasDebugInfo())
+  if (MMI->hasDebugInfo() || AlwaysNeedCFI)
 return CFI_M_Debug;
 
   return CFI_M_None;
Index: llvm/docs/CommandGuide/llc.rst
===
--- llvm/docs/CommandGuide/llc.rst
+++ llvm/docs/CommandGuide/llc.rst
@@ -152,6 +152,10 @@
  Emit the .remarks (ELF) / __remarks (MachO) section which contains metadata
  about remark diagnostics.
 
+.. option:: -always-need-cfi
+
+ Emit call frame information even if other debug information is not present.
+
 Tuning/Configuration Options
 
 
Index: clang/test/Driver/always-need-cfi.c
===
--- /dev/null
+++ clang/test/Driver/always-need-cfi.c
@@ -0,0 +1,7 @@
+
+// RUN: %clang -target arm -c -### %s -falways-need-cfi 2>&1 | FileCheck --check-prefix=CHECK-ALWAYS %s
+// RUN: %clang -target arm -c -### %s -fno-always-need-cfi 2>&1 | FileCheck --check-prefix=CHECK-NO-ALWAYS %s
+// RUN: %clang -target arm -c -### %s 2>&1 | FileCheck --check-prefix=CHECK-NO-ALWAYS %s
+
+// CHECK-ALWAYS: -falways-need-cfi
+// CHECK-NO-ALWAYS-NOT: -falways-need-cfi
\ No newline at end of file
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -953,6 +953,8 @@
   Args.hasFlag(OPT_fstack_size_section, OPT_fno_stack_size_section, false);
   Opts.UniqueSectionNames = Args.hasFlag(OPT_funique_section_names,
  

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

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67096



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

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

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 218864.
hokein marked 2 inline comments as done.
hokein added a comment.

rebase and simplify the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67096

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/package.json
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -109,12 +109,14 @@
 
   const clangdClient = new ClangdLanguageClient('Clang Language Server',
 serverOptions, clientOptions);
-  const semanticHighlightingFeature =
+  if (getConfig('semanticHighlighting')) {
+const semanticHighlightingFeature =
   new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
-   context);
-  context.subscriptions.push(
+context);
+context.subscriptions.push(
   vscode.Disposable.from(semanticHighlightingFeature));
-  clangdClient.registerFeature(semanticHighlightingFeature);
+clangdClient.registerFeature(semanticHighlightingFeature);
+  }
   console.log('Clang Language Server is now active!');
   context.subscriptions.push(clangdClient.start());
   context.subscriptions.push(vscode.commands.registerCommand(
Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -89,6 +89,11 @@
 "clangd.trace": {
 "type": "string",
 "description": "Names a file that clangd should log a 
performance trace to, in chrome trace-viewer JSON format."
+},
+"clangd.semanticHighlighting": {
+"type": "boolean",
+"default": "false",
+"description": "Enable semantic highlighting in clangd"
 }
 }
 },


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -109,12 +109,14 @@
 
   const clangdClient = new ClangdLanguageClient('Clang Language Server',
 serverOptions, clientOptions);
-  const semanticHighlightingFeature =
+  if (getConfig('semanticHighlighting')) {
+const semanticHighlightingFeature =
   new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
-   context);
-  context.subscriptions.push(
+context);
+context.subscriptions.push(
   vscode.Disposable.from(semanticHighlightingFeature));
-  clangdClient.registerFeature(semanticHighlightingFeature);
+clangdClient.registerFeature(semanticHighlightingFeature);
+  }
   console.log('Clang Language Server is now active!');
   context.subscriptions.push(clangdClient.start());
   context.subscriptions.push(vscode.commands.registerCommand(
Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -89,6 +89,11 @@
 "clangd.trace": {
 "type": "string",
 "description": "Names a file that clangd should log a performance trace to, in chrome trace-viewer JSON format."
+},
+"clangd.semanticHighlighting": {
+"type": "boolean",
+"default": "false",
+"description": "Enable semantic highlighting in clangd"
 }
 }
 },
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

In D67140#1658365 , @aaron.ballman 
wrote:

> Ah, good to know! That reduces my concern, but doesn't negate it. AFAIK, we 
> haven't changed the interface such that it requires code changes rather than 
> just a recompile in recent history, so this is a bit novel.


I think API changes happen all the time. At Google, we are integrating upstream 
LLVM and Clang changes into our internal codebase daily. We have a lot of 
internal ClangTidy checkers. Fixing up all our internal code to keep with 
upstream changes is a full time job for one engineer (but it is a rotation).

> My personal feeling is: by itself, this isn't worth the churn but the fix to 
> downstream code is pretty easy so I'm not strongly opposed. However, do we 
> have other "I wish we changed this interface, but that would break the world" 
> issues we want to tackle in this release for clang-tidy? That might make this 
> refactoring more worth the pain.

I'll think about more stuff to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67140



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

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

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371036: [clangd][vscode] Make SemanticHighlightingFeature 
more self-contained. (authored by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67165?vs=218710=218862#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67165

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


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


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

[clang-tools-extra] r371036 - [clangd][vscode] Make SemanticHighlightingFeature more self-contained.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Thu Sep  5 02:14:04 2019
New Revision: 371036

URL: http://llvm.org/viewvc/llvm-project?rev=371036=rev
Log:
[clangd][vscode] Make SemanticHighlightingFeature more self-contained.

Summary:
so that we don't have too many usage from the client side (just a single
occurrance for register), this also aligns with how other builtin feature
being implemented in vscode.

Reviewers: ilya-biryukov

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts

clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts?rev=371036=371035=371036=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts 
(original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts Thu 
Sep  5 02:14:04 2019
@@ -110,7 +110,8 @@ export function activate(context: vscode
   const clangdClient = new ClangdLanguageClient('Clang Language Server',
 serverOptions, clientOptions);
   const semanticHighlightingFeature =
-  new semanticHighlighting.SemanticHighlightingFeature();
+  new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
+   context);
   context.subscriptions.push(
   vscode.Disposable.from(semanticHighlightingFeature));
   clangdClient.registerFeature(semanticHighlightingFeature);
@@ -144,14 +145,9 @@ export function activate(context: vscode
   clangdClient.onNotification(
   'textDocument/clangd.fileStatus',
   (fileStatus) => { status.onFileUpdated(fileStatus); });
-  clangdClient.onNotification(
-  semanticHighlighting.NotificationType,
-  semanticHighlightingFeature.handleNotification.bind(
-  semanticHighlightingFeature));
 } else if (newState == vscodelc.State.Stopped) {
   // Clear all cached statuses when clangd crashes.
   status.clear();
-  semanticHighlightingFeature.dispose();
 }
   }));
   // An empty place holder for the activate command, otherwise we'll get an

Modified: 
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts?rev=371036=371035=371036=diff
==
--- 
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
 (original)
+++ 
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
 Thu Sep  5 02:14:04 2019
@@ -44,7 +44,7 @@ export interface SemanticHighlightingLin
 
 // Language server push notification providing the semantic highlighting
 // information for a text document.
-export const NotificationType =
+const NotificationType =
 new vscodelc.NotificationType(
 'textDocument/semanticHighlighting');
 
@@ -58,6 +58,19 @@ export class SemanticHighlightingFeature
   highlighter: Highlighter;
   // Any disposables that should be cleaned up when clangd crashes.
   private subscriptions: vscode.Disposable[] = [];
+  constructor(client: vscodelc.BaseLanguageClient,
+  context: vscode.ExtensionContext) {
+context.subscriptions.push(client.onDidChangeState(({newState}) => {
+  if (newState == vscodelc.State.Running) {
+// Register handler for semantic highlighting notification.
+client.onNotification(NotificationType,
+  this.handleNotification.bind(this));
+  } else if (newState == vscodelc.State.Stopped) {
+// Dispose resources when clangd crashes.
+this.dispose();
+  }
+}));
+  }
   fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {
 // Extend the ClientCapabilities type and add semantic highlighting
 // capability to the object.


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

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

[Simon Cook via Phabricator via cfe-commits]

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

Thanks for the feedback. I will improve the test so it more reliably tests what 
it intends to.

With regards to behaviour surrounding things such as argument registers, before 
submitting I checked what the riscv port of GCC does, and it matches this 
behaviour. If a register is used for arg passing/return values then the option 
is accepted but silently ignored (at least the register is still used for 
passing arguments, I haven't confirmed for other regalloc purposes).

I agree that this has the opportunity for allowing users to think they've 
reserved a register but it is still used. I will look at something more to what 
you've described AArch64 does in LLVM, and also check that there isn't also a 
bug on the GCC side, if so I'll get that fixed too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67185



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

[PATCH] D65752: [Sema] Refactor LookupVisibleDecls. NFC

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371032: [Sema] Refactor LookupVisibleDecls. NFC (authored by 
ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65752?vs=213371=218859#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65752

Files:
  cfe/trunk/lib/Sema/SemaLookup.cpp

Index: cfe/trunk/lib/Sema/SemaLookup.cpp
===
--- cfe/trunk/lib/Sema/SemaLookup.cpp
+++ cfe/trunk/lib/Sema/SemaLookup.cpp
@@ -3701,328 +3701,345 @@
   return nullptr;
 }
 
-static void LookupVisibleDecls(DeclContext *Ctx, LookupResult ,
-   bool QualifiedNameLookup,
-   bool InBaseClass,
-   VisibleDeclConsumer ,
-   VisibleDeclsRecord ,
-   bool IncludeDependentBases,
-   bool LoadExternal) {
-  if (!Ctx)
-return;
-
-  // Make sure we don't visit the same context twice.
-  if (Visited.visitedContext(Ctx->getPrimaryContext()))
-return;
-
-  Consumer.EnteredContext(Ctx);
+class LookupVisibleHelper {
+public:
+  LookupVisibleHelper(VisibleDeclConsumer , bool IncludeDependentBases,
+  bool LoadExternal)
+  : Consumer(Consumer), IncludeDependentBases(IncludeDependentBases),
+LoadExternal(LoadExternal) {}
+
+  void lookupVisibleDecls(Sema , Scope *S, Sema::LookupNameKind Kind,
+  bool IncludeGlobalScope) {
+// Determine the set of using directives available during
+// unqualified name lookup.
+Scope *Initial = S;
+UnqualUsingDirectiveSet UDirs(SemaRef);
+if (SemaRef.getLangOpts().CPlusPlus) {
+  // Find the first namespace or translation-unit scope.
+  while (S && !isNamespaceOrTranslationUnitScope(S))
+S = S->getParent();
 
-  // Outside C++, lookup results for the TU live on identifiers.
-  if (isa(Ctx) &&
-  !Result.getSema().getLangOpts().CPlusPlus) {
-auto  = Result.getSema();
-auto  = S.Context.Idents;
-
-// Ensure all external identifiers are in the identifier table.
-if (LoadExternal)
-  if (IdentifierInfoLookup *External = Idents.getExternalIdentifierLookup()) {
-std::unique_ptr Iter(External->getIdentifiers());
-for (StringRef Name = Iter->Next(); !Name.empty(); Name = Iter->Next())
-  Idents.get(Name);
-  }
-
-// Walk all lookup results in the TU for each identifier.
-for (const auto  : Idents) {
-  for (auto I = S.IdResolver.begin(Ident.getValue()),
-E = S.IdResolver.end();
-   I != E; ++I) {
-if (S.IdResolver.isDeclInScope(*I, Ctx)) {
-  if (NamedDecl *ND = Result.getAcceptableDecl(*I)) {
-Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
-Visited.add(ND);
-  }
-}
-  }
+  UDirs.visitScopeChain(Initial, S);
 }
+UDirs.done();
 
-return;
+// Look for visible declarations.
+LookupResult Result(SemaRef, DeclarationName(), SourceLocation(), Kind);
+Result.setAllowHidden(Consumer.includeHiddenDecls());
+if (!IncludeGlobalScope)
+  Visited.visitedContext(SemaRef.getASTContext().getTranslationUnitDecl());
+ShadowContextRAII Shadow(Visited);
+lookupInScope(Initial, Result, UDirs);
   }
 
-  if (CXXRecordDecl *Class = dyn_cast(Ctx))
-Result.getSema().ForceDeclarationOfImplicitMembers(Class);
-
-  // We sometimes skip loading namespace-level results (they tend to be huge).
-  bool Load = LoadExternal ||
-  !(isa(Ctx) || isa(Ctx));
-  // Enumerate all of the results in this context.
-  for (DeclContextLookupResult R :
-   Load ? Ctx->lookups()
-: Ctx->noload_lookups(/*PreserveInternalState=*/false)) {
-for (auto *D : R) {
-  if (auto *ND = Result.getAcceptableDecl(D)) {
-Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
-Visited.add(ND);
-  }
-}
-  }
+  void lookupVisibleDecls(Sema , DeclContext *Ctx,
+  Sema::LookupNameKind Kind, bool IncludeGlobalScope) {
+LookupResult Result(SemaRef, DeclarationName(), SourceLocation(), Kind);
+Result.setAllowHidden(Consumer.includeHiddenDecls());
+if (!IncludeGlobalScope)
+  Visited.visitedContext(SemaRef.getASTContext().getTranslationUnitDecl());
 
-  // Traverse using directives for qualified name lookup.
-  if (QualifiedNameLookup) {
 ShadowContextRAII Shadow(Visited);
-for (auto I : Ctx->using_directives()) {
-  if (!Result.getSema().isVisible(I))
-continue;
-  LookupVisibleDecls(I->getNominatedNamespace(), Result,
- QualifiedNameLookup, InBaseClass, Consumer, Visited,
- 

r371032 - [Sema] Refactor LookupVisibleDecls. NFC

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Sep  5 01:59:06 2019
New Revision: 371032

URL: http://llvm.org/viewvc/llvm-project?rev=371032=rev
Log:
[Sema] Refactor LookupVisibleDecls. NFC

Summary:
We accumulated some configuration parameters for LookupVisibleDecls that
are being passed unchanged to recursive calls, e.g. LoadExternal and
IncludeDependentBases.

At the same time, there is a bunch of parameters that can change in the
recursive invocations.

It is hard to tell the difference between those groups, making the code
hard to follow.

This change introduces a helper struct and factors out the non-changing
bits into fields, making recursive calls in the implementation code easier
to read.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: riccibruno, doug.gregor, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

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

Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=371032=371031=371032=diff
==
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Thu Sep  5 01:59:06 2019
@@ -3701,328 +3701,345 @@ NamedDecl *VisibleDeclsRecord::checkHidd
   return nullptr;
 }
 
-static void LookupVisibleDecls(DeclContext *Ctx, LookupResult ,
-   bool QualifiedNameLookup,
-   bool InBaseClass,
-   VisibleDeclConsumer ,
-   VisibleDeclsRecord ,
-   bool IncludeDependentBases,
-   bool LoadExternal) {
-  if (!Ctx)
-return;
-
-  // Make sure we don't visit the same context twice.
-  if (Visited.visitedContext(Ctx->getPrimaryContext()))
-return;
-
-  Consumer.EnteredContext(Ctx);
-
-  // Outside C++, lookup results for the TU live on identifiers.
-  if (isa(Ctx) &&
-  !Result.getSema().getLangOpts().CPlusPlus) {
-auto  = Result.getSema();
-auto  = S.Context.Idents;
-
-// Ensure all external identifiers are in the identifier table.
-if (LoadExternal)
-  if (IdentifierInfoLookup *External = 
Idents.getExternalIdentifierLookup()) {
-std::unique_ptr Iter(External->getIdentifiers());
-for (StringRef Name = Iter->Next(); !Name.empty(); Name = Iter->Next())
-  Idents.get(Name);
-  }
-
-// Walk all lookup results in the TU for each identifier.
-for (const auto  : Idents) {
-  for (auto I = S.IdResolver.begin(Ident.getValue()),
-E = S.IdResolver.end();
-   I != E; ++I) {
-if (S.IdResolver.isDeclInScope(*I, Ctx)) {
-  if (NamedDecl *ND = Result.getAcceptableDecl(*I)) {
-Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
-Visited.add(ND);
-  }
-}
-  }
-}
-
-return;
+class LookupVisibleHelper {
+public:
+  LookupVisibleHelper(VisibleDeclConsumer , bool 
IncludeDependentBases,
+  bool LoadExternal)
+  : Consumer(Consumer), IncludeDependentBases(IncludeDependentBases),
+LoadExternal(LoadExternal) {}
+
+  void lookupVisibleDecls(Sema , Scope *S, Sema::LookupNameKind Kind,
+  bool IncludeGlobalScope) {
+// Determine the set of using directives available during
+// unqualified name lookup.
+Scope *Initial = S;
+UnqualUsingDirectiveSet UDirs(SemaRef);
+if (SemaRef.getLangOpts().CPlusPlus) {
+  // Find the first namespace or translation-unit scope.
+  while (S && !isNamespaceOrTranslationUnitScope(S))
+S = S->getParent();
+
+  UDirs.visitScopeChain(Initial, S);
+}
+UDirs.done();
+
+// Look for visible declarations.
+LookupResult Result(SemaRef, DeclarationName(), SourceLocation(), Kind);
+Result.setAllowHidden(Consumer.includeHiddenDecls());
+if (!IncludeGlobalScope)
+  Visited.visitedContext(SemaRef.getASTContext().getTranslationUnitDecl());
+ShadowContextRAII Shadow(Visited);
+lookupInScope(Initial, Result, UDirs);
   }
 
-  if (CXXRecordDecl *Class = dyn_cast(Ctx))
-Result.getSema().ForceDeclarationOfImplicitMembers(Class);
-
-  // We sometimes skip loading namespace-level results (they tend to be huge).
-  bool Load = LoadExternal ||
-  !(isa(Ctx) || isa(Ctx));
-  // Enumerate all of the results in this context.
-  for (DeclContextLookupResult R :
-   Load ? Ctx->lookups()
-: Ctx->noload_lookups(/*PreserveInternalState=*/false)) {
-for (auto *D : R) {
-  if (auto *ND = Result.getAcceptableDecl(D)) {
-Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
-Visited.add(ND);
-  }
-}
-  }
+  void lookupVisibleDecls(Sema , DeclContext *Ctx,
+  Sema::LookupNameKind Kind, bool IncludeGlobalScope) 

Re: r361885 - [Driver] Fix -working-directory issues

[Hans Wennborg via cfe-commits]

I've reverted this in r371027 as discussed on PR43204.

On Wed, May 29, 2019 at 12:18 AM Michael J. Spencer via cfe-commits
 wrote:
>
> Author: mspencer
> Date: Tue May 28 15:21:47 2019
> New Revision: 361885
>
> URL: http://llvm.org/viewvc/llvm-project?rev=361885=rev
> Log:
> [Driver] Fix -working-directory issues
>
> Currently the `-working-directory` option does not actually impact the working
> directory for all of the clang driver, it only impacts how files are looked up
> to make sure they exist.  This means that that clang passes the wrong paths
> to -fdebug-compilation-dir and -coverage-notes-file.
>
> This patch fixes that by changing all the places in the driver where we 
> convert
> to absolute paths to use the VFS, and then calling setCurrentWorkingDirectory 
> on
> the VFS.  This also changes the default VFS for `Driver` to use a virtualized
> working directory, instead of changing the process's working directory.
>
> Differential Revision: https://reviews.llvm.org/D62271
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
> cfe/trunk/lib/Driver/Driver.cpp
> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> cfe/trunk/test/Driver/working-directory.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=361885=361884=361885=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Tue May 28 
> 15:21:47 2019
> @@ -91,6 +91,8 @@ def err_no_external_assembler : Error<
>"there is no external assembler that can be used on this platform">;
>  def err_drv_unable_to_remove_file : Error<
>"unable to remove file: %0">;
> +def err_drv_unable_to_set_working_directory : Error <
> +  "unable to set working directory: %0">;
>  def err_drv_command_failure : Error<
>"unable to execute command: %0">;
>  def err_drv_invalid_darwin_version : Error<
>
> Modified: cfe/trunk/lib/Driver/Driver.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=361885=361884=361885=diff
> ==
> --- cfe/trunk/lib/Driver/Driver.cpp (original)
> +++ cfe/trunk/lib/Driver/Driver.cpp Tue May 28 15:21:47 2019
> @@ -133,7 +133,7 @@ Driver::Driver(StringRef ClangExecutable
>
>// Provide a sane fallback if no VFS is specified.
>if (!this->VFS)
> -this->VFS = llvm::vfs::getRealFileSystem();
> +this->VFS = llvm::vfs::createPhysicalFileSystem().release();
>
>Name = llvm::sys::path::filename(ClangExecutable);
>Dir = llvm::sys::path::parent_path(ClangExecutable);
> @@ -1005,6 +1005,11 @@ Compilation *Driver::BuildCompilation(Ar
>  }
>}
>
> +  // Check for working directory option before accessing any files
> +  if (Arg *WD = Args.getLastArg(options::OPT_working_directory))
> +if (std::error_code EC = VFS->setCurrentWorkingDirectory(WD->getValue()))
> +  Diag(diag::err_drv_unable_to_set_working_directory) << WD->getValue();
> +
>// FIXME: This stuff needs to go into the Compilation, not the driver.
>bool CCCPrintPhases;
>
> @@ -1984,20 +1989,11 @@ bool Driver::DiagnoseInputExistence(cons
>if (Value == "-")
>  return true;
>
> -  SmallString<64> Path(Value);
> -  if (Arg *WorkDir = Args.getLastArg(options::OPT_working_directory)) {
> -if (!llvm::sys::path::is_absolute(Path)) {
> -  SmallString<64> Directory(WorkDir->getValue());
> -  llvm::sys::path::append(Directory, Value);
> -  Path.assign(Directory);
> -}
> -  }
> -
> -  if (getVFS().exists(Path))
> +  if (getVFS().exists(Value))
>  return true;
>
>if (IsCLMode()) {
> -if (!llvm::sys::path::is_absolute(Twine(Path)) &&
> +if (!llvm::sys::path::is_absolute(Twine(Value)) &&
>  llvm::sys::Process::FindInEnvPath("LIB", Value))
>return true;
>
> @@ -2023,12 +2019,12 @@ bool Driver::DiagnoseInputExistence(cons
>  if (getOpts().findNearest(Value, Nearest, IncludedFlagsBitmask,
>ExcludedFlagsBitmask) <= 1) {
>Diag(clang::diag::err_drv_no_such_file_with_suggestion)
> -  << Path << Nearest;
> +  << Value << Nearest;
>return false;
>  }
>}
>
> -  Diag(clang::diag::err_drv_no_such_file) << Path;
> +  Diag(clang::diag::err_drv_no_such_file) << Value;
>return false;
>  }
>
>
> Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=361885=361884=361885=diff
> ==
> --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue May 28 15:21:47 2019
> @@ -616,11 +616,11 @@ static bool 

r371027 - Revert r361885 "[Driver] Fix -working-directory issues"

[Hans Wennborg via cfe-commits]

Author: hans
Date: Thu Sep  5 01:43:00 2019
New Revision: 371027

URL: http://llvm.org/viewvc/llvm-project?rev=371027=rev
Log:
Revert r361885 "[Driver] Fix -working-directory issues"

This made clang unable to open files using relative paths on network shares on
Windows (PR43204). On the bug it was pointed out that createPhysicalFileSystem()
is not terribly mature, and using it is risky. Reverting for now until there's
a clear way forward.

> Currently the `-working-directory` option does not actually impact the working
> directory for all of the clang driver, it only impacts how files are looked up
> to make sure they exist.  This means that that clang passes the wrong paths
> to -fdebug-compilation-dir and -coverage-notes-file.
>
> This patch fixes that by changing all the places in the driver where we 
> convert
> to absolute paths to use the VFS, and then calling setCurrentWorkingDirectory 
> on
> the VFS.  This also changes the default VFS for `Driver` to use a virtualized
> working directory, instead of changing the process's working directory.
>
> Differential Revision: https://reviews.llvm.org/D62271

This also revertes the part of r369938 which checked that -working-directory 
works.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/gen-cdb-fragment.c
cfe/trunk/test/Driver/working-directory.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=371027=371026=371027=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Thu Sep  5 01:43:00 
2019
@@ -91,8 +91,6 @@ def err_no_external_assembler : Error<
   "there is no external assembler that can be used on this platform">;
 def err_drv_unable_to_remove_file : Error<
   "unable to remove file: %0">;
-def err_drv_unable_to_set_working_directory : Error <
-  "unable to set working directory: %0">;
 def err_drv_command_failure : Error<
   "unable to execute command: %0">;
 def err_drv_invalid_darwin_version : Error<

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=371027=371026=371027=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Thu Sep  5 01:43:00 2019
@@ -133,7 +133,7 @@ Driver::Driver(StringRef ClangExecutable
 
   // Provide a sane fallback if no VFS is specified.
   if (!this->VFS)
-this->VFS = llvm::vfs::createPhysicalFileSystem().release();
+this->VFS = llvm::vfs::getRealFileSystem();
 
   Name = llvm::sys::path::filename(ClangExecutable);
   Dir = llvm::sys::path::parent_path(ClangExecutable);
@@ -1010,11 +1010,6 @@ Compilation *Driver::BuildCompilation(Ar
 }
   }
 
-  // Check for working directory option before accessing any files
-  if (Arg *WD = Args.getLastArg(options::OPT_working_directory))
-if (VFS->setCurrentWorkingDirectory(WD->getValue()))
-  Diag(diag::err_drv_unable_to_set_working_directory) << WD->getValue();
-
   // FIXME: This stuff needs to go into the Compilation, not the driver.
   bool CCCPrintPhases;
 
@@ -1996,11 +1991,20 @@ bool Driver::DiagnoseInputExistence(cons
   if (Value == "-")
 return true;
 
-  if (getVFS().exists(Value))
+  SmallString<64> Path(Value);
+  if (Arg *WorkDir = Args.getLastArg(options::OPT_working_directory)) {
+if (!llvm::sys::path::is_absolute(Path)) {
+  SmallString<64> Directory(WorkDir->getValue());
+  llvm::sys::path::append(Directory, Value);
+  Path.assign(Directory);
+}
+  }
+
+  if (getVFS().exists(Path))
 return true;
 
   if (IsCLMode()) {
-if (!llvm::sys::path::is_absolute(Twine(Value)) &&
+if (!llvm::sys::path::is_absolute(Twine(Path)) &&
 llvm::sys::Process::FindInEnvPath("LIB", Value))
   return true;
 
@@ -2026,12 +2030,12 @@ bool Driver::DiagnoseInputExistence(cons
 if (getOpts().findNearest(Value, Nearest, IncludedFlagsBitmask,
   ExcludedFlagsBitmask) <= 1) {
   Diag(clang::diag::err_drv_no_such_file_with_suggestion)
-  << Value << Nearest;
+  << Path << Nearest;
   return false;
 }
   }
 
-  Diag(clang::diag::err_drv_no_such_file) << Value;
+  Diag(clang::diag::err_drv_no_such_file) << Path;
   return false;
 }
 

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=371027=371026=371027=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Thu Sep  5 

[PATCH] D67213: [clang-tidy] Fix definitions in headers check to respect qualifiers

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371022: [clang-tidy] Fix definitions in headers check to 
respect qualifiers (authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67213?vs=218851=218855#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67213

Files:
  clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp


Index: clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
@@ -180,3 +180,15 @@
 constexpr int k = 1; // OK: constexpr variable has internal linkage.
 
 constexpr int f10() { return 0; } // OK: constexpr function definition.
+
+const int f11() { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: function 'f11' defined in a 
header file;
+// CHECK-FIXES: inline const int f11() { return 0; }
+
+template 
+const T f12();
+
+template <>
+const int f12() { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: full function template 
specialization 'f12' defined in a header file;
+// CHECK-FIXES: inline const int f12() { return 0; }
Index: clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -132,8 +132,7 @@
 << IsFullSpec << FD;
 diag(FD->getLocation(), /*FixDescription=*/"make as 'inline'",
  DiagnosticIDs::Note)
-<< 
FixItHint::CreateInsertion(FD->getReturnTypeSourceRange().getBegin(),
-  "inline ");
+<< FixItHint::CreateInsertion(FD->getInnerLocStart(), "inline ");
   } else if (const auto *VD = dyn_cast(ND)) {
 // Static data members of a class template are allowed.
 if (VD->getDeclContext()->isDependentContext() && VD->isStaticDataMember())


Index: clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
@@ -180,3 +180,15 @@
 constexpr int k = 1; // OK: constexpr variable has internal linkage.
 
 constexpr int f10() { return 0; } // OK: constexpr function definition.
+
+const int f11() { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: function 'f11' defined in a header file;
+// CHECK-FIXES: inline const int f11() { return 0; }
+
+template 
+const T f12();
+
+template <>
+const int f12() { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: full function template specialization 'f12' defined in a header file;
+// CHECK-FIXES: inline const int f12() { return 0; }
Index: clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -132,8 +132,7 @@
 << IsFullSpec << FD;
 diag(FD->getLocation(), /*FixDescription=*/"make as 'inline'",
  DiagnosticIDs::Note)
-<< FixItHint::CreateInsertion(FD->getReturnTypeSourceRange().getBegin(),
-  "inline ");
+<< FixItHint::CreateInsertion(FD->getInnerLocStart(), "inline ");
   } else if (const auto *VD = dyn_cast(ND)) {
 // Static data members of a class template are allowed.
 if (VD->getDeclContext()->isDependentContext() && VD->isStaticDataMember())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r371022 - [clang-tidy] Fix definitions in headers check to respect qualifiers

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Sep  5 01:11:21 2019
New Revision: 371022

URL: http://llvm.org/viewvc/llvm-project?rev=371022=rev
Log:
[clang-tidy] Fix definitions in headers check to respect qualifiers

Summary:
The check was generating a fix without taking qualifiers in return type
into account. This patch changes the insertion location to be before qualifers.

Reviewers: gribozavr

Subscribers: xazax.hun, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp?rev=371022=371021=371022=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp Thu 
Sep  5 01:11:21 2019
@@ -132,8 +132,7 @@ void DefinitionsInHeadersCheck::check(co
 << IsFullSpec << FD;
 diag(FD->getLocation(), /*FixDescription=*/"make as 'inline'",
  DiagnosticIDs::Note)
-<< 
FixItHint::CreateInsertion(FD->getReturnTypeSourceRange().getBegin(),
-  "inline ");
+<< FixItHint::CreateInsertion(FD->getInnerLocStart(), "inline ");
   } else if (const auto *VD = dyn_cast(ND)) {
 // Static data members of a class template are allowed.
 if (VD->getDeclContext()->isDependentContext() && VD->isStaticDataMember())

Modified: 
clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp?rev=371022=371021=371022=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp Thu 
Sep  5 01:11:21 2019
@@ -180,3 +180,15 @@ int CD::f() { // OK: partial tem
 constexpr int k = 1; // OK: constexpr variable has internal linkage.
 
 constexpr int f10() { return 0; } // OK: constexpr function definition.
+
+const int f11() { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: function 'f11' defined in a 
header file;
+// CHECK-FIXES: inline const int f11() { return 0; }
+
+template 
+const T f12();
+
+template <>
+const int f12() { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: full function template 
specialization 'f12' defined in a header file;
+// CHECK-FIXES: inline const int f12() { return 0; }


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

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

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371021: Rename of constants in ASTImporterVisibilityTest. 
NFC. (authored by balazske, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67174?vs=218699=218854#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67174

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

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

[PATCH] D59637: [analyzer] Use the custom propagation rules and sinks in GenericTaintChecker

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

Szelethus added a comment.

Also, please mark inlines done as you fix them :)


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

https://reviews.llvm.org/D59637



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

[PATCH] D59637: [analyzer] Use the custom propagation rules and sinks in GenericTaintChecker

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

Szelethus accepted this revision.
Szelethus added a reviewer: steakhal.
Szelethus added a comment.
This revision is now accepted and ready to land.

Some nits inline, otherwise LGTM. @steakhal, do you have anything to add to 
this?




Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:191
 static TaintPropagationRule
-getTaintPropagationRule(const FunctionDecl *FDecl, StringRef Name,
+getTaintPropagationRule(const GenericTaintChecker *Checker,
+const FunctionDecl *FDecl, StringRef Name,

How about only passing `CustomPropagations`?



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:605
 // Mark the given argument.
-assert(ArgNum < CE->getNumArgs());
 State = State->add(ArgNum);

I get that there isn't much substance to this assert, but why remove it? We 
might as well populate the lines in between that and the branch.



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:844
   if (Config)
-Checker->parseConfiguration(Mgr, Option, std::move(Config).getValue());
+Checker->parseConfiguration(Mgr, Option, std::move(Config.getValue()));
 }

Wasn't this commited before?


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

https://reviews.llvm.org/D59637



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

r371021 - Rename of constants in ASTImporterVisibilityTest. NFC.

[Balazs Keri via cfe-commits]

Author: balazske
Date: Thu Sep  5 00:59:45 2019
New Revision: 371021

URL: http://llvm.org/viewvc/llvm-project?rev=371021=rev
Log:
Rename of constants in ASTImporterVisibilityTest. NFC.

Reviewers: martong, a.sidorin, shafik

Reviewed By: shafik

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

Tags: #clang

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

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

Modified: cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp?rev=371021=371020=371021=diff
==
--- cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterVisibilityTest.cpp Thu Sep  5 00:59:45 
2019
@@ -290,75 +290,83 @@ TEST_P(ImportTypedefNameVisibility, Impo
   TypedTest_ImportAfterImportWithMerge();
 }
 
-const bool ExpectLink = true;
-const bool ExpectNotLink = false;
+const bool ExpectLinkedDeclChain = true;
+const bool ExpectUnlinkedDeclChain = false;
 
 INSTANTIATE_TEST_CASE_P(
 ParameterizedTests, ImportFunctionsVisibility,
 ::testing::Combine(
 DefaultTestValuesForRunOptions,
-::testing::Values(std::make_tuple(ExternF, ExternF, ExpectLink),
-  std::make_tuple(ExternF, StaticF, ExpectNotLink),
-  std::make_tuple(ExternF, AnonF, ExpectNotLink),
-  std::make_tuple(StaticF, ExternF, ExpectNotLink),
-  std::make_tuple(StaticF, StaticF, ExpectNotLink),
-  std::make_tuple(StaticF, AnonF, ExpectNotLink),
-  std::make_tuple(AnonF, ExternF, ExpectNotLink),
-  std::make_tuple(AnonF, StaticF, ExpectNotLink),
-  std::make_tuple(AnonF, AnonF, ExpectNotLink))), );
+::testing::Values(
+std::make_tuple(ExternF, ExternF, ExpectLinkedDeclChain),
+std::make_tuple(ExternF, StaticF, ExpectUnlinkedDeclChain),
+std::make_tuple(ExternF, AnonF, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticF, ExternF, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticF, StaticF, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticF, AnonF, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonF, ExternF, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonF, StaticF, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonF, AnonF, ExpectUnlinkedDeclChain))), );
 INSTANTIATE_TEST_CASE_P(
 ParameterizedTests, ImportVariablesVisibility,
 ::testing::Combine(
 DefaultTestValuesForRunOptions,
-::testing::Values(std::make_tuple(ExternV, ExternV, ExpectLink),
-  std::make_tuple(ExternV, StaticV, ExpectNotLink),
-  std::make_tuple(ExternV, AnonV, ExpectNotLink),
-  std::make_tuple(StaticV, ExternV, ExpectNotLink),
-  std::make_tuple(StaticV, StaticV, ExpectNotLink),
-  std::make_tuple(StaticV, AnonV, ExpectNotLink),
-  std::make_tuple(AnonV, ExternV, ExpectNotLink),
-  std::make_tuple(AnonV, StaticV, ExpectNotLink),
-  std::make_tuple(AnonV, AnonV, ExpectNotLink))), );
+::testing::Values(
+std::make_tuple(ExternV, ExternV, ExpectLinkedDeclChain),
+std::make_tuple(ExternV, StaticV, ExpectUnlinkedDeclChain),
+std::make_tuple(ExternV, AnonV, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticV, ExternV, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticV, StaticV, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticV, AnonV, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonV, ExternV, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonV, StaticV, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonV, AnonV, ExpectUnlinkedDeclChain))), );
 INSTANTIATE_TEST_CASE_P(
 ParameterizedTests, ImportClassesVisibility,
 ::testing::Combine(
 DefaultTestValuesForRunOptions,
-::testing::Values(std::make_tuple(ExternC, ExternC, ExpectLink),
-  std::make_tuple(ExternC, AnonC, ExpectNotLink),
-  std::make_tuple(AnonC, ExternC, ExpectNotLink),
-  std::make_tuple(AnonC, AnonC, ExpectNotLink))), );
+::testing::Values(
+std::make_tuple(ExternC, ExternC, ExpectLinkedDeclChain),
+std::make_tuple(ExternC, AnonC, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonC, ExternC, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonC, AnonC, ExpectUnlinkedDeclChain))), );
 INSTANTIATE_TEST_CASE_P(
 ParameterizedTests, ImportEnumsVisibility,
 

[PATCH] D67213: [clang-tidy] Fix definitions in headers check to respect qualifiers

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: gribozavr.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

The check was generating a fix without taking qualifiers in return type
into account. This patch changes the insertion location to be before qualifers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67213

Files:
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/test/clang-tidy/misc-definitions-in-headers.hpp


Index: clang-tools-extra/test/clang-tidy/misc-definitions-in-headers.hpp
===
--- clang-tools-extra/test/clang-tidy/misc-definitions-in-headers.hpp
+++ clang-tools-extra/test/clang-tidy/misc-definitions-in-headers.hpp
@@ -180,3 +180,15 @@
 constexpr int k = 1; // OK: constexpr variable has internal linkage.
 
 constexpr int f10() { return 0; } // OK: constexpr function definition.
+
+const int f11() { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: function 'f11' defined in a 
header file;
+// CHECK-FIXES: inline const int f11() { return 0; }
+
+template 
+const T f12();
+
+template <>
+const int f12() { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: full function template 
specialization 'f12' defined in a header file;
+// CHECK-FIXES: inline const int f12() { return 0; }
Index: clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -132,8 +132,7 @@
 << IsFullSpec << FD;
 diag(FD->getLocation(), /*FixDescription=*/"make as 'inline'",
  DiagnosticIDs::Note)
-<< 
FixItHint::CreateInsertion(FD->getReturnTypeSourceRange().getBegin(),
-  "inline ");
+<< FixItHint::CreateInsertion(FD->getInnerLocStart(), "inline ");
   } else if (const auto *VD = dyn_cast(ND)) {
 // Static data members of a class template are allowed.
 if (VD->getDeclContext()->isDependentContext() && VD->isStaticDataMember())


Index: clang-tools-extra/test/clang-tidy/misc-definitions-in-headers.hpp
===
--- clang-tools-extra/test/clang-tidy/misc-definitions-in-headers.hpp
+++ clang-tools-extra/test/clang-tidy/misc-definitions-in-headers.hpp
@@ -180,3 +180,15 @@
 constexpr int k = 1; // OK: constexpr variable has internal linkage.
 
 constexpr int f10() { return 0; } // OK: constexpr function definition.
+
+const int f11() { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: function 'f11' defined in a header file;
+// CHECK-FIXES: inline const int f11() { return 0; }
+
+template 
+const T f12();
+
+template <>
+const int f12() { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: full function template specialization 'f12' defined in a header file;
+// CHECK-FIXES: inline const int f12() { return 0; }
Index: clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -132,8 +132,7 @@
 << IsFullSpec << FD;
 diag(FD->getLocation(), /*FixDescription=*/"make as 'inline'",
  DiagnosticIDs::Note)
-<< FixItHint::CreateInsertion(FD->getReturnTypeSourceRange().getBegin(),
-  "inline ");
+<< FixItHint::CreateInsertion(FD->getInnerLocStart(), "inline ");
   } else if (const auto *VD = dyn_cast(ND)) {
 // Static data members of a class template are allowed.
 if (VD->getDeclContext()->isDependentContext() && VD->isStaticDataMember())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits