[PATCH] D90533: [clang-format] Always consider option PenaltyBreakBeforeFirstCallParameter

[Mark Nauwelaerts via Phabricator via cfe-commits]

mnauw abandoned this revision.
mnauw added a comment.

Looks like PenaltyBreakBeforeFirstCallParameter not being considered so well 
has been addressed somewhat differently in D90246 
, and so that does the job as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90533

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

[clang] 5e373b2 - [Sema] Use isa<> instead of dyn_cast<> as pointer is never dereferenced. NFCI.

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2020-11-15T12:58:36Z
New Revision: 5e373b2e94d714a7250e961029a58bae5585f33e

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

LOG: [Sema] Use isa<> instead of dyn_cast<> as pointer is never dereferenced. 
NFCI.

We are only checking for the class type. Fixes Wshadow warnings.

Added: 


Modified: 
clang/lib/Sema/CodeCompleteConsumer.cpp

Removed: 




diff  --git a/clang/lib/Sema/CodeCompleteConsumer.cpp 
b/clang/lib/Sema/CodeCompleteConsumer.cpp
index f1ad8aeaacbb..678a09ba1003 100644
--- a/clang/lib/Sema/CodeCompleteConsumer.cpp
+++ b/clang/lib/Sema/CodeCompleteConsumer.cpp
@@ -356,8 +356,7 @@ const char *CodeCompletionAllocator::CopyString(const Twine 
&String) {
 }
 
 StringRef CodeCompletionTUInfo::getParentName(const DeclContext *DC) {
-  const NamedDecl *ND = dyn_cast(DC);
-  if (!ND)
+  if (!isa(DC))
 return {};
 
   // Check whether we've already cached the parent name.
@@ -470,8 +469,7 @@ void CodeCompletionBuilder::addParentContext(const 
DeclContext *DC) {
   if (DC->isFunctionOrMethod())
 return;
 
-  const NamedDecl *ND = dyn_cast(DC);
-  if (!ND)
+  if (!isa(DC))
 return;
 
   ParentName = getCodeCompletionTUInfo().getParentName(DC);



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

[clang] fb58142 - Fix temporary file name on Windows

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2020-11-15T08:11:05-05:00
New Revision: fb58142e00ad73c2e6bfd80452e5e9b080c81552

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

LOG: Fix temporary file name on Windows

Bound arch may contain ':', which is invalid in Windows file names.

This patch fixes that.

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

Added: 
clang/test/Driver/hip-windows-filename.hip

Modified: 
clang/lib/Driver/Driver.cpp

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index bcea01126e0d..5f4e7f271764 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4692,9 +4692,16 @@ static bool HasPreprocessOutput(const Action &JA) {
 
 const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA,
const char *BaseInput,
-   StringRef BoundArch, bool AtTopLevel,
+   StringRef OrigBoundArch, bool 
AtTopLevel,
bool MultipleArchs,
StringRef OffloadingPrefix) const {
+  std::string BoundArch = OrigBoundArch.str();
+#if defined(_WIN32)
+  // BoundArch may contains ':', which is invalid in file names on Windows,
+  // therefore replace it with '%'.
+  std::replace(BoundArch.begin(), BoundArch.end(), ':', '@');
+#endif
+
   llvm::PrettyStackTraceString CrashInfo("Computing output path");
   // Output to a user requested destination?
   if (AtTopLevel && !isa(JA) && !isa(JA)) {

diff  --git a/clang/test/Driver/hip-windows-filename.hip 
b/clang/test/Driver/hip-windows-filename.hip
new file mode 100644
index ..9244f16c378f
--- /dev/null
+++ b/clang/test/Driver/hip-windows-filename.hip
@@ -0,0 +1,10 @@
+// REQUIRES: system-windows, clang-driver, amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-pc-windows-msvc \
+// RUN:   -x hip \
+// RUN:   --offload-arch=gfx908:xnack+ \
+// RUN:   -nogpuinc -nogpulib -save-temps \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: "-o" "hip-windows-filename-hip-amdgcn-amd-amdhsa-gfx908@xnack+.cui"
+// CHECK-NOT: "-o" 
"hip-windows-filename-hip-amdgcn-amd-amdhsa-gfx908:xnack+.cui"



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

[PATCH] D91421: Fix temporary file name on Windows

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfb58142e00ad: Fix temporary file name on Windows (authored 
by yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D91421?vs=305127&id=305352#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91421

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/hip-windows-filename.hip


Index: clang/test/Driver/hip-windows-filename.hip
===
--- /dev/null
+++ clang/test/Driver/hip-windows-filename.hip
@@ -0,0 +1,10 @@
+// REQUIRES: system-windows, clang-driver, amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-pc-windows-msvc \
+// RUN:   -x hip \
+// RUN:   --offload-arch=gfx908:xnack+ \
+// RUN:   -nogpuinc -nogpulib -save-temps \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: "-o" "hip-windows-filename-hip-amdgcn-amd-amdhsa-gfx908@xnack+.cui"
+// CHECK-NOT: "-o" 
"hip-windows-filename-hip-amdgcn-amd-amdhsa-gfx908:xnack+.cui"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4692,9 +4692,16 @@
 
 const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA,
const char *BaseInput,
-   StringRef BoundArch, bool AtTopLevel,
+   StringRef OrigBoundArch, bool 
AtTopLevel,
bool MultipleArchs,
StringRef OffloadingPrefix) const {
+  std::string BoundArch = OrigBoundArch.str();
+#if defined(_WIN32)
+  // BoundArch may contains ':', which is invalid in file names on Windows,
+  // therefore replace it with '%'.
+  std::replace(BoundArch.begin(), BoundArch.end(), ':', '@');
+#endif
+
   llvm::PrettyStackTraceString CrashInfo("Computing output path");
   // Output to a user requested destination?
   if (AtTopLevel && !isa(JA) && !isa(JA)) {


Index: clang/test/Driver/hip-windows-filename.hip
===
--- /dev/null
+++ clang/test/Driver/hip-windows-filename.hip
@@ -0,0 +1,10 @@
+// REQUIRES: system-windows, clang-driver, amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-pc-windows-msvc \
+// RUN:   -x hip \
+// RUN:   --offload-arch=gfx908:xnack+ \
+// RUN:   -nogpuinc -nogpulib -save-temps \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: "-o" "hip-windows-filename-hip-amdgcn-amd-amdhsa-gfx908@xnack+.cui"
+// CHECK-NOT: "-o" "hip-windows-filename-hip-amdgcn-amd-amdhsa-gfx908:xnack+.cui"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4692,9 +4692,16 @@
 
 const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA,
const char *BaseInput,
-   StringRef BoundArch, bool AtTopLevel,
+   StringRef OrigBoundArch, bool AtTopLevel,
bool MultipleArchs,
StringRef OffloadingPrefix) const {
+  std::string BoundArch = OrigBoundArch.str();
+#if defined(_WIN32)
+  // BoundArch may contains ':', which is invalid in file names on Windows,
+  // therefore replace it with '%'.
+  std::replace(BoundArch.begin(), BoundArch.end(), ':', '@');
+#endif
+
   llvm::PrettyStackTraceString CrashInfo("Computing output path");
   // Output to a user requested destination?
   if (AtTopLevel && !isa(JA) && !isa(JA)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68845: Don't emit unwanted constructor calls in co_return statements

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert updated this revision to Diff 305353.
aaronpuchert added a comment.
Herald added a subscriber: lxfind.

Rebase and add a comment about the limitations of this implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68845

Files:
  clang/lib/Sema/SemaCoroutine.cpp
  clang/test/SemaCXX/coroutine-rvo.cpp

Index: clang/test/SemaCXX/coroutine-rvo.cpp
===
--- clang/test/SemaCXX/coroutine-rvo.cpp
+++ clang/test/SemaCXX/coroutine-rvo.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -stdlib=libc++ -std=c++1z -fcoroutines-ts -fsyntax-only
+// RUN: %clang_cc1 -verify -std=c++17 -fcoroutines-ts -fsyntax-only %s
 
 namespace std::experimental {
 template  struct coroutine_handle {
@@ -39,10 +39,14 @@
 };
 
 struct MoveOnly {
-  MoveOnly() {};
+  MoveOnly() = default;
   MoveOnly(const MoveOnly&) = delete;
-  MoveOnly(MoveOnly&&) noexcept {};
-  ~MoveOnly() {};
+  MoveOnly(MoveOnly &&) = default;
+};
+
+struct NoCopyNoMove {
+  NoCopyNoMove() = default;
+  NoCopyNoMove(const NoCopyNoMove &) = delete;
 };
 
 template 
@@ -52,18 +56,93 @@
 auto final_suspend() noexcept { return suspend_never{}; }
 auto get_return_object() { return task{}; }
 static void unhandled_exception() {}
-void return_value(T&& value) {}
+void return_value(T &&value) {} // expected-note 2{{passing argument}}
   };
 };
 
-task f() {
-  MoveOnly value;
+task local2val() {
+  NoCopyNoMove value;
+  co_return value;
+}
+
+task local2ref() {
+  NoCopyNoMove value;
+  co_return value;
+}
+
+// We need the move constructor for construction of the coroutine.
+task param2val(MoveOnly value) {
+  co_return value;
+}
+
+task lvalue2val(NoCopyNoMove &value) {
+  co_return value; // expected-error{{rvalue reference to type 'NoCopyNoMove' cannot bind to lvalue of type 'NoCopyNoMove'}}
+}
+
+task rvalue2val(NoCopyNoMove &&value) {
+  co_return value;
+}
+
+task lvalue2ref(NoCopyNoMove &value) {
   co_return value;
 }
 
-int main() {
-  f();
-  return 0;
+task rvalue2ref(NoCopyNoMove &&value) {
+  co_return value;
+}
+
+struct To {
+  operator MoveOnly() &&;
+};
+task conversion_operator() {
+  To t;
+  co_return t;
+}
+
+struct Construct {
+  Construct(MoveOnly);
+};
+task converting_constructor() {
+  MoveOnly w;
+  co_return w;
+}
+
+struct Derived : MoveOnly {};
+task derived2base() {
+  Derived result;
+  co_return result;
+}
+
+struct RetThis {
+  task foo() && {
+co_return *this; // expected-error{{rvalue reference to type 'RetThis' cannot bind to lvalue of type 'RetThis'}}
+  }
+};
+
+template 
+struct is_same { static constexpr bool value = false; };
+
+template 
+struct is_same { static constexpr bool value = true; };
+
+template 
+struct template_return_task {
+  struct promise_type {
+auto initial_suspend() { return suspend_never{}; }
+auto final_suspend() noexcept { return suspend_never{}; }
+auto get_return_object() { return template_return_task{}; }
+static void unhandled_exception();
+template 
+void return_value(U &&value) {
+  static_assert(is_same::value);
+}
+  };
+};
+
+template_return_task param2template(MoveOnly value) {
+  co_return value; // We should deduce U = MoveOnly.
 }
 
-// expected-no-diagnostics
+template_return_task lvalue2template(NoCopyNoMove &value) {
+  co_return value; // We should deduce U = NoCopyNoMove&.
+}
Index: clang/lib/Sema/SemaCoroutine.cpp
===
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -954,6 +954,82 @@
   return BuildCoreturnStmt(Loc, E);
 }
 
+// [class.copy.elision]p3: "If the expression in a [...] co_­return statement
+// is a (possibly parenthesized) id-expression that names an implicitly
+// movable entity declared in the body or parameter-declaration-clause of
+// the innermost enclosing function or lambda-expression, [...] overload
+// resolution to select [...] the return_­value overload to call is first
+// performed as if the expression or operand were an rvalue. If the first
+// overload resolution fails or was not performed, overload resolution is
+// performed again, considering the expression or operand as an lvalue."
+//
+// Our implementation deviates from the standard in two ways:
+// 1) We only consider cases where return_value is a function or function
+//template, so we never try implicitly moving in the case of a function
+//pointer or object of class type with operator() overload.
+// 2) We don't fall back if overload resolution failed due to ambiguity, only
+//if no candidate works. One could say that we do overload resolution on
+//both (const) T& and T&& while treating candidates for T&& as better fit:
+//so no candidate for T&& falls back to T&, but multiple equivalent
+//candidates are an error.
+static bool shouldMoveRetu

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:4861
+// We need correct types when the template-name is unresolved or when it
+// might be overloaded.
+if (!ResolvedTemplate)

And from the PR summary:

> namely mangling such template arguments as being cast to the parameter type 
> in the case where the template name is unresolved or overloaded

This phrasing worries me a little bit. Are you saying that you might mangle the 
name of `foo` in one way, when there's also a `foo` 
in scope, and in a different way, when there's not? That doesn't seem 
conforming. So I imagine it's more likely that I'm misunderstanding what you 
mean by "might be overloaded" / "is overloaded". Could you explain for my 
information, and perhaps also adjust the wording of these code comments to make 
the explanation less needed?

Specifically, I think it would be non-conforming if the TU

// https://godbolt.org/z/YjPqMd
template void foo();
char arr[6];
extern template void foo();  // #1
int main() { foo(); }

could not be linked against the TU

template int foo();
template void foo();  // is this "overloading"?
extern char arr[6];
template<> void foo() {}  // #2

because lines #1 and #2 disagreed about the way to mangle `foo`. (Again, 
I'm pretty sure you haven't made them disagree... but I remain unclear on 
what's meant by "overloading" in this PR, if it's //not// this.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

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

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp:1092
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'isSet' 
[bugprone-redundant-branch-condition]
+  // CHECK-FIXES: {{isSet}}
+}

aaron.ballman wrote:
> zinovy.nis wrote:
> > aaron.ballman wrote:
> > > zinovy.nis wrote:
> > > > aaron.ballman wrote:
> > > > > zinovy.nis wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > There's not a whole lot of context for FileCheck to determine if 
> > > > > > > it's been correctly applied or not (same below) -- for instance, 
> > > > > > > won't this pass even if no changes are applied because FileCheck 
> > > > > > > is still going to find `isSet` in the file?
> > > > > > Thanks. Fixed.
> > > > > Maybe it's just early in the morning for me, but... I was expecting 
> > > > > the transformation to be:
> > > > > ```
> > > > > if (RetT::Test(isSet).Ok() && isSet) {
> > > > >   if (RetT::Test(isSet).Ok() && isSet) {
> > > > >   }
> > > > > }
> > > > > ```
> > > > > turns into
> > > > > ```
> > > > > if (RetT::Test(isSet).Ok() && isSet) {
> > > > > }
> > > > > ```
> > > > > Why does it remove the `&& isSet` instead? That seems like it's 
> > > > > changing the logic here from `if (true && false)` to `if (true)`.
> > > > IMO it's correct.
> > > > `isSet` cannot change its value between `if`s while 
> > > > `RetT::Test(isSet).Ok()` can.
> > > > So we don't need to re-evaluate `isSet` and need to re-evaluate 
> > > > `RetT::Test(isSet).Ok()` only.
> > > > 
> > > > 
> > > > 
> > > > > That seems like it's changing the logic here from if (true && false) 
> > > > > to if (true).
> > > > 
> > > > 
> > > > As I understand only the second `if` is transformed.
> > > Does this only trigger as a redundant branch condition if the definition 
> > > of `RetT::Test()` is available? Because `Test()` takes a `bool&` so it 
> > > sure seems like `isSet` could be modified between the branches if the 
> > > definition isn't found because it's being passed as a non-const reference 
> > > to `Test()`.
> > 1) 
> > > if the definition of RetT::Test() is available?
> > 
> > Removing the body from RetT::Test changes nothing.
> > 
> > 2) Turning `RetT Test(bool &_isSet)` -> `RetT Test(bool _isSet)` also 
> > changes nothing.
> > 
> > 
> > 
> > 
> Given the following four ways of declaring `Test()`:
> ```
> static RetT Test(bool &_isSet); // #1
> static RetT Test(bool _isSet); // #2
> static RetT Test(const bool &_isSet); // #3
> static RetT Test(bool &_isSet) { return 0; } // #4
> ```
> I would expect #2 and #3 to behave the same way -- the `isSet` argument could 
> never be modified by calling `Test()` and so the second `Test(isSet) && 
> isSet` is partially redundant and the second `if` statement can drop the ` && 
> isSet`. (Without dropping the `Test(isSet)` because the call could still 
> modify some global somewhere, etc.)
> 
> I would expect #1 to not modify the second `if` statement at all because 
> there's no way of knowing whether `Test(isSet) && isSet` is the same between 
> the first `if` statement and the second one (because the second call to 
> `Test(isSet)` may modify `isSet` in a way the caller can observe).
> 
> Ideally, #4 would be a case where we could remove the entire second `if` 
> statement because we can identify that not only does `isSet` not get modified 
> despite being passed by non-const reference, we can see that the `Test()` 
> function doesn't modify any state at all. However, this seems like it would 
> require data flow analysis and so I think it makes sense to treat #4 the same 
> as #1.
> 
> That said, I just realized the check isn't being very careful with reference 
> parameters in the first place: https://godbolt.org/z/P1aP3W, so your changes 
> aren't introducing a new problem, they just happened to highlight an existing 
> issue.
Please look at https://reviews.llvm.org/D91495 - there's a fix for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91037

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

[PATCH] D91495: [clang-tidy] false-positive for bugprone-redundant-branch-condition in case of passed-by-ref params

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis created this revision.
zinovy.nis added reviewers: aaron.ballman, baloghadamsoftware.
Herald added subscribers: cfe-commits, rnkovacs, xazax.hun.
Herald added a project: clang.
zinovy.nis requested review of this revision.

Inspired by discussion in https://reviews.llvm.org/D91037


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91495

Files:
  clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
@@ -6,7 +6,8 @@
 bool isBurning();
 bool isReallyBurning();
 bool isCollapsing();
-void tryToExtinguish(bool&);
+bool tryToExtinguish(bool&);
+bool tryToExtinguishByVal(bool &);
 void tryPutFireOut();
 bool callTheFD();
 void scream();
@@ -948,6 +949,24 @@
   }
 }
 
+void negative_by_ref(bool isSet) {
+  if (tryToExtinguish(isSet) && isSet) {
+if (tryToExtinguish(isSet) && isSet) {
+  // NO-MESSAGE: fire may have been extinguished
+  scream();
+}
+  }
+}
+
+void negative_by_val(bool isSet) {
+  if (tryToExtinguishByVal(isSet) && isSet) {
+if (tryToExtinguishByVal(isSet) && isSet) {
+  // NO-MESSAGE: fire may have been extinguished
+  scream();
+}
+  }
+}
+
 void negative_reassigned() {
   bool onFire = isBurning();
   if (onFire) {
@@ -1077,9 +1096,9 @@
 int positive_expr_with_cleanups() {
   class RetT {
   public:
-RetT(const int _code) : code_(_code) {}
-bool Ok() const { return code_ == 0; }
-static RetT Test(bool &_isSet) { return 0; }
+RetT(const int code);
+bool Ok() const;
+static RetT Test(bool isSet);
 
   private:
 int code_;
Index: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
@@ -75,6 +75,9 @@
   if (hasPtrOrReferenceInFunc(Func, CondVar))
 return;
 
+  if (isChangedBefore(OuterIf->getCond(), InnerIf, CondVar, Result.Context))
+return;
+
   if (isChangedBefore(OuterIf->getThen(), InnerIf, CondVar, Result.Context))
 return;
 
@@ -123,7 +126,7 @@
   CharSourceRange::getTokenRange(IfBegin, IfEnd));
 }
 
-// For comound statements also remove the right brace at the end.
+// For compound statements also remove the right brace at the end.
 if (isa(Body))
   Diag << FixItHint::CreateRemoval(
   CharSourceRange::getTokenRange(Body->getEndLoc(), 
Body->getEndLoc()));


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
@@ -6,7 +6,8 @@
 bool isBurning();
 bool isReallyBurning();
 bool isCollapsing();
-void tryToExtinguish(bool&);
+bool tryToExtinguish(bool&);
+bool tryToExtinguishByVal(bool &);
 void tryPutFireOut();
 bool callTheFD();
 void scream();
@@ -948,6 +949,24 @@
   }
 }
 
+void negative_by_ref(bool isSet) {
+  if (tryToExtinguish(isSet) && isSet) {
+if (tryToExtinguish(isSet) && isSet) {
+  // NO-MESSAGE: fire may have been extinguished
+  scream();
+}
+  }
+}
+
+void negative_by_val(bool isSet) {
+  if (tryToExtinguishByVal(isSet) && isSet) {
+if (tryToExtinguishByVal(isSet) && isSet) {
+  // NO-MESSAGE: fire may have been extinguished
+  scream();
+}
+  }
+}
+
 void negative_reassigned() {
   bool onFire = isBurning();
   if (onFire) {
@@ -1077,9 +1096,9 @@
 int positive_expr_with_cleanups() {
   class RetT {
   public:
-RetT(const int _code) : code_(_code) {}
-bool Ok() const { return code_ == 0; }
-static RetT Test(bool &_isSet) { return 0; }
+RetT(const int code);
+bool Ok() const;
+static RetT Test(bool isSet);
 
   private:
 int code_;
Index: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
@@ -75,6 +75,9 @@
   if (hasPtrOrReferenceInFunc(Func, CondVar))
 return;
 
+  if (isChangedBefore(OuterIf->getCond(), InnerIf, CondVar, Result.Context))
+return;
+
   if (isChangedBefore(OuterIf->getThen(), InnerIf, CondVar, Result.Context))
 return;
 
@@ -123,7 +126,7 @@
   CharSourceRange::getTokenRange(

[PATCH] D91495: [clang-tidy] false-positive for bugprone-redundant-branch-condition in case of passed-by-ref params

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis updated this revision to Diff 305361.
zinovy.nis added a comment.

Extend the context.


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

https://reviews.llvm.org/D91495

Files:
  clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
@@ -6,7 +6,8 @@
 bool isBurning();
 bool isReallyBurning();
 bool isCollapsing();
-void tryToExtinguish(bool&);
+bool tryToExtinguish(bool&);
+bool tryToExtinguishByVal(bool &);
 void tryPutFireOut();
 bool callTheFD();
 void scream();
@@ -948,6 +949,24 @@
   }
 }
 
+void negative_by_ref(bool isSet) {
+  if (tryToExtinguish(isSet) && isSet) {
+if (tryToExtinguish(isSet) && isSet) {
+  // NO-MESSAGE: fire may have been extinguished
+  scream();
+}
+  }
+}
+
+void negative_by_val(bool isSet) {
+  if (tryToExtinguishByVal(isSet) && isSet) {
+if (tryToExtinguishByVal(isSet) && isSet) {
+  // NO-MESSAGE: fire may have been extinguished
+  scream();
+}
+  }
+}
+
 void negative_reassigned() {
   bool onFire = isBurning();
   if (onFire) {
@@ -1077,9 +1096,9 @@
 int positive_expr_with_cleanups() {
   class RetT {
   public:
-RetT(const int _code) : code_(_code) {}
-bool Ok() const { return code_ == 0; }
-static RetT Test(bool &_isSet) { return 0; }
+RetT(const int code);
+bool Ok() const;
+static RetT Test(bool isSet);
 
   private:
 int code_;
Index: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
@@ -75,6 +75,9 @@
   if (hasPtrOrReferenceInFunc(Func, CondVar))
 return;
 
+  if (isChangedBefore(OuterIf->getCond(), InnerIf, CondVar, Result.Context))
+return;
+
   if (isChangedBefore(OuterIf->getThen(), InnerIf, CondVar, Result.Context))
 return;
 
@@ -123,7 +126,7 @@
   CharSourceRange::getTokenRange(IfBegin, IfEnd));
 }
 
-// For comound statements also remove the right brace at the end.
+// For compound statements also remove the right brace at the end.
 if (isa(Body))
   Diag << FixItHint::CreateRemoval(
   CharSourceRange::getTokenRange(Body->getEndLoc(), 
Body->getEndLoc()));


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
@@ -6,7 +6,8 @@
 bool isBurning();
 bool isReallyBurning();
 bool isCollapsing();
-void tryToExtinguish(bool&);
+bool tryToExtinguish(bool&);
+bool tryToExtinguishByVal(bool &);
 void tryPutFireOut();
 bool callTheFD();
 void scream();
@@ -948,6 +949,24 @@
   }
 }
 
+void negative_by_ref(bool isSet) {
+  if (tryToExtinguish(isSet) && isSet) {
+if (tryToExtinguish(isSet) && isSet) {
+  // NO-MESSAGE: fire may have been extinguished
+  scream();
+}
+  }
+}
+
+void negative_by_val(bool isSet) {
+  if (tryToExtinguishByVal(isSet) && isSet) {
+if (tryToExtinguishByVal(isSet) && isSet) {
+  // NO-MESSAGE: fire may have been extinguished
+  scream();
+}
+  }
+}
+
 void negative_reassigned() {
   bool onFire = isBurning();
   if (onFire) {
@@ -1077,9 +1096,9 @@
 int positive_expr_with_cleanups() {
   class RetT {
   public:
-RetT(const int _code) : code_(_code) {}
-bool Ok() const { return code_ == 0; }
-static RetT Test(bool &_isSet) { return 0; }
+RetT(const int code);
+bool Ok() const;
+static RetT Test(bool isSet);
 
   private:
 int code_;
Index: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp
@@ -75,6 +75,9 @@
   if (hasPtrOrReferenceInFunc(Func, CondVar))
 return;
 
+  if (isChangedBefore(OuterIf->getCond(), InnerIf, CondVar, Result.Context))
+return;
+
   if (isChangedBefore(OuterIf->getThen(), InnerIf, CondVar, Result.Context))
 return;
 
@@ -123,7 +126,7 @@
   CharSourceRange::getTokenRange(IfBegin, IfEnd));
 }
 
-// For comound statements also remove the right brace at the end.
+// For compound statements also remove the right brace at the end

[PATCH] D90123: [Sema] Improve notes for value category mismatch in overloading

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4280
 "candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
-"expects an l-value for "
+"expects an %select{l-value|r-value}5 for "
 "%select{%ordinal4 argument|object argument}3">;

rsmith wrote:
> Please change this to `lvalue|rvalue` (preferably in a separate commit, and 
> likewise for the half-dozen or so other diagnostics that use this 
> unconventional spelling). In both C and C++ contexts, these words are spelled 
> without a hyphen.
I was wondering about the inconsistent spelling as well. Yes, I'll do this in a 
follow-up.



Comment at: clang/lib/Sema/SemaOverload.cpp:10410-10411
 
+  if (Conv.Bad.Kind == BadConversionSequence::lvalue_ref_to_rvalue ||
+  Conv.Bad.Kind == BadConversionSequence::rvalue_ref_to_lvalue) {
+S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_value_category)

rsmith wrote:
> It's surprising to me that nothing else in this function is considering 
> `Conv.Bad.Kind`. Do you know what's going on there? I see that 
> `PerformObjectArgumentInitialization` has a `switch` on it, but it looks like 
> that's the *only* place that uses it, and we mostly instead try to recompute 
> what went wrong after the fact, which seems fragile. I wonder if more of the 
> complexity of this function could be reduced by using the stored bad 
> conversion kind. (But let's keep this patch targeted on just fixing the value 
> category diagnostics!)
The previous `if` could perhaps be on `Conv.Bad.Kind == 
BadConversionSequence::bad_qualifiers` instead, but maybe we're not setting 
that correctly in some cases. I will try it out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90123

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

[clang] 6f84779 - [Sema] Improve notes for value category mismatch in overloading

[Aaron Puchert via cfe-commits]

Author: Aaron Puchert
Date: 2020-11-15T18:05:11+01:00
New Revision: 6f84779674a9764c6adee29b9a48ed3b3f0d5132

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

LOG: [Sema] Improve notes for value category mismatch in overloading

When an overloaded member function has a ref-qualifier, like:

class X {
void f() &&;
void f(int) &;
};

we would print strange notes when the ref-qualifier doesn't fit the value
category:

X x;
x.f();
X().f(0);

would both print a note "no known conversion from 'X' to 'X' for object
argument" on their relevant overload instead of pointing out the
mismatch in value category.

At first I thought the solution is easy: just use the FailureKind member
of the BadConversionSequence struct. But it turns out that we weren't
properly setting this for function arguments. So I went through
TryReferenceInit to make sure we're doing that right, and found a number
of notes in the existing tests that improved as well.

Fixes PR47791.

Reviewed By: rsmith

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaOverload.cpp
clang/test/CXX/drs/dr14xx.cpp
clang/test/CXX/drs/dr1xx.cpp
clang/test/CXX/drs/dr6xx.cpp
clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p3-0x.cpp
clang/test/SemaCXX/overload-member-call.cpp
clang/test/SemaCXX/rval-references-examples.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 554d5943a63a..a256bb9b562b 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4287,9 +4287,9 @@ def note_ovl_candidate_bad_arc_conv : Note<
 "cannot implicitly convert argument "
 "%
diff {of type $ to $|type to parameter type}3,4 for "
 "%select{%ordinal6 argument|object argument}5 under ARC">;
-def note_ovl_candidate_bad_lvalue : Note<
+def note_ovl_candidate_bad_value_category : Note<
 "candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
-"expects an l-value for "
+"expects an %select{l-value|r-value}5 for "
 "%select{%ordinal4 argument|object argument}3">;
 def note_ovl_candidate_bad_addrspace : Note<
 "candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 277ef5af9130..847f0dd977b7 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -4832,8 +4832,11 @@ TryReferenceInit(Sema &S, Expr *Init, QualType DeclType,
   // -- Otherwise, the reference shall be an lvalue reference to a
   //non-volatile const type (i.e., cv1 shall be const), or the 
reference
   //shall be an rvalue reference.
-  if (!isRValRef && (!T1.isConstQualified() || T1.isVolatileQualified()))
+  if (!isRValRef && (!T1.isConstQualified() || T1.isVolatileQualified())) {
+if (InitCategory.isRValue() && RefRelationship != Sema::Ref_Incompatible)
+  ICS.setBad(BadConversionSequence::lvalue_ref_to_rvalue, Init, DeclType);
 return ICS;
+  }
 
   //   -- If the initializer expression
   //
@@ -4923,9 +4926,11 @@ TryReferenceInit(Sema &S, Expr *Init, QualType DeclType,
 
   // If T1 is reference-related to T2 and the reference is an rvalue
   // reference, the initializer expression shall not be an lvalue.
-  if (RefRelationship >= Sema::Ref_Related &&
-  isRValRef && Init->Classify(S.Context).isLValue())
+  if (RefRelationship >= Sema::Ref_Related && isRValRef &&
+  Init->Classify(S.Context).isLValue()) {
+ICS.setBad(BadConversionSequence::rvalue_ref_to_lvalue, Init, DeclType);
 return ICS;
+  }
 
   // C++ [over.ics.ref]p2:
   //   When a parameter of reference type is not bound directly to
@@ -4963,11 +4968,8 @@ TryReferenceInit(Sema &S, Expr *Init, QualType DeclType,
 //   binding an rvalue reference to an lvalue other than a function
 //   lvalue.
 // Note that the function case is not possible here.
-if (DeclType->isRValueReferenceType() && LValRefType) {
-  // FIXME: This is the wrong BadConversionSequence. The problem is binding
-  // an rvalue reference to a (non-function) lvalue, not binding an lvalue
-  // reference to an rvalue!
-  ICS.setBad(BadConversionSequence::lvalue_ref_to_rvalue, Init, DeclType);
+if (isRValRef && LValRefType) {
+  ICS.setBad(BadConversionSequence::no_conversion, Init, DeclType);
   return ICS;
 }
 
@@ -10458,7 +10460,7 @@ static void DiagnoseBadConversion(Sema &S, 
OverloadCandidate *Cand,
 }
 
 unsigned CVR = FromQs.getCVRQualifiers() & ~ToQs.getCVRQualifiers();
-assert(CVR && "unexpected qualifiers mism

[PATCH] D90123: [Sema] Improve notes for value category mismatch in overloading

[Aaron Puchert via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6f84779674a9: [Sema] Improve notes for value category 
mismatch in overloading (authored by aaronpuchert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90123

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/drs/dr14xx.cpp
  clang/test/CXX/drs/dr1xx.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p3-0x.cpp
  clang/test/SemaCXX/overload-member-call.cpp
  clang/test/SemaCXX/rval-references-examples.cpp

Index: clang/test/SemaCXX/rval-references-examples.cpp
===
--- clang/test/SemaCXX/rval-references-examples.cpp
+++ clang/test/SemaCXX/rval-references-examples.cpp
@@ -13,7 +13,7 @@
 
   ~unique_ptr() { delete ptr; }
 
-  unique_ptr &operator=(unique_ptr &&other) { // expected-note{{candidate function not viable: no known conversion from 'unique_ptr' to 'unique_ptr &&' for 1st argument}}
+  unique_ptr &operator=(unique_ptr &&other) { // expected-note{{candidate function not viable: expects an r-value for 1st argument}}
 if (this == &other)
   return *this;
 
Index: clang/test/SemaCXX/overload-member-call.cpp
===
--- clang/test/SemaCXX/overload-member-call.cpp
+++ clang/test/SemaCXX/overload-member-call.cpp
@@ -83,6 +83,9 @@
 void baz(A &d); // expected-note {{candidate function not viable: 1st argument ('const test1::A') would lose const qualifier}}
 void baz(int i); // expected-note {{candidate function not viable: no known conversion from 'const test1::A' to 'int' for 1st argument}} 
 
+void ref() &&;   // expected-note {{expects an r-value for object argument}} expected-note {{requires 0 arguments, but 1 was provided}}
+void ref(int) &; // expected-note {{expects an l-value for object argument}} expected-note {{requires 1 argument, but 0 were provided}}
+
 // PR 11857
 void foo(int n); // expected-note {{candidate function not viable: requires single argument 'n', but 2 arguments were provided}}
 void foo(unsigned n = 10); // expected-note {{candidate function not viable: allows at most single argument 'n', but 2 arguments were provided}}
@@ -103,6 +106,9 @@
 
 a.rab(); //expected-error {{no matching member function for call to 'rab'}}
 a.zab(3, 4, 5); //expected-error {{no matching member function for call to 'zab'}}
+
+a.ref();// expected-error {{no matching member function for call to 'ref'}}
+A().ref(1); // expected-error {{no matching member function for call to 'ref'}}
   }
 }
 
Index: clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p3-0x.cpp
===
--- clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p3-0x.cpp
+++ clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p3-0x.cpp
@@ -7,7 +7,7 @@
 namespace ClassTemplateParamNotForwardingRef {
   // This is not a forwarding reference.
   template struct A { // expected-note {{candidate}}
-A(T&&); // expected-note {{no known conversion from 'int' to 'int &&'}}
+A(T&&); // expected-note {{expects an r-value}}
   };
   int n;
   A a = n; // expected-error {{no viable constructor or deduction guide}}
@@ -53,8 +53,8 @@
   X xy2 = f0(lvalue());
 }
 
-template X f1(const T&&); // expected-note{{candidate function [with T = int] not viable: no known conversion from 'int' to 'const int &&' for 1st argument}} \
-// expected-note{{candidate function [with T = Y] not viable: no known conversion from 'Y' to 'const Y &&' for 1st argument}}
+template X f1(const T&&); // expected-note{{candidate function [with T = int] not viable: expects an r-value for 1st argument}} \
+// expected-note{{candidate function [with T = Y] not viable: expects an r-value for 1st argument}}
 
 void test_f1() {
   X xi0 = f1(prvalue());
@@ -67,7 +67,7 @@
 
 namespace std_example {
   template  int f(T&&); 
-  template  int g(const T&&); // expected-note{{candidate function [with T = int] not viable: no known conversion from 'int' to 'const int &&' for 1st argument}}
+  template  int g(const T&&); // expected-note{{candidate function [with T = int] not viable: expects an r-value for 1st argument}}
 
   int i;
   int n1 = f(i);
@@ -77,7 +77,7 @@
 #if __cplusplus > 201402L
   template struct A { // expected-note {{candidate}}
 template
-A(T &&, U &&, int *); // expected-note {{[with T = int, U = int] not viable: no known conversion from 'int' to 'int &&'}}
+A(T &&, U &&, int *); // expected-note {{[with T = int, U = int] not viable: expects an r-value}}
 A(T &&, int *);   // expected-note {{requires 2}}
   };
   template A(T &&, int *) -> A; // expe

[PATCH] D90123: [Sema] Improve notes for value category mismatch in overloading

[Aaron Puchert via Phabricator via cfe-commits]

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

Fixed the spelling issue in rGdea31f135ceae6e860e6301f9bb66d3b3adb1357 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90123

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

[clang] e6aa065 - [NFC, Refactor] Modernize the TypeSpecifierWidth enum (Specifiers.h) to a scoped enum

[via cfe-commits]

Author: faisalv
Date: 2020-11-15T11:13:57-06:00
New Revision: e6aa06545b123292be283af7c414daead23cf9ab

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

LOG: [NFC, Refactor] Modernize the TypeSpecifierWidth enum (Specifiers.h) to a 
scoped enum

Reviewed here: https://reviews.llvm.org/D91409 by Aaron.
Highlights of the review:
  - avoid an underlying type for enums
  - avoid enum bit fields (MSVC packing anomalies) and favor static_casts to 
unsigned bit-fields

Patch by Thorsten Schuett  w some minor fixes in 
SemaType.cpp where a couple asserts had to be repaired to deal with lack of 
implicit coversion to int.

Thanks Thorsten!

Added: 


Modified: 
clang/include/clang/AST/TypeLoc.h
clang/include/clang/Basic/Specifiers.h
clang/include/clang/Sema/DeclSpec.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Sema/DeclSpec.cpp
clang/lib/Sema/SemaType.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp

Removed: 




diff  --git a/clang/include/clang/AST/TypeLoc.h 
b/clang/include/clang/AST/TypeLoc.h
index 72cc8ef098e7..4d3d4a94cbf3 100644
--- a/clang/include/clang/AST/TypeLoc.h
+++ b/clang/include/clang/AST/TypeLoc.h
@@ -619,16 +619,16 @@ class BuiltinTypeLoc : public 
ConcreteTypeLoc(getWrittenBuiltinSpecs().Width);
 else
-  return TSW_unspecified;
+  return TypeSpecifierWidth::Unspecified;
   }
 
   bool hasWrittenWidthSpec() const {
-return getWrittenWidthSpec() != TSW_unspecified;
+return getWrittenWidthSpec() != TypeSpecifierWidth::Unspecified;
   }
 
   void setWrittenWidthSpec(TypeSpecifierWidth written) {
 if (needsExtraLocalData())
-  getWrittenBuiltinSpecs().Width = written;
+  getWrittenBuiltinSpecs().Width = static_cast(written);
   }
 
   TypeSpecifierType getWrittenTypeSpec() const;
@@ -659,7 +659,7 @@ class BuiltinTypeLoc : public ConcreteTypeLoc(TypeSpecifierWidth::Unspecified);
   wbs.Type = TST_unspecified;
   wbs.ModeAttr = false;
 }

diff  --git a/clang/include/clang/Basic/Specifiers.h 
b/clang/include/clang/Basic/Specifiers.h
index 2834dea20d00..cdd67a688283 100644
--- a/clang/include/clang/Basic/Specifiers.h
+++ b/clang/include/clang/Basic/Specifiers.h
@@ -37,12 +37,7 @@ namespace clang {
   };
 
   /// Specifies the width of a type, e.g., short, long, or long long.
-  enum TypeSpecifierWidth {
-TSW_unspecified,
-TSW_short,
-TSW_long,
-TSW_longlong
-  };
+  enum class TypeSpecifierWidth { Unspecified, Short, Long, LongLong };
 
   /// Specifies the signedness of a type, e.g., signed or unsigned.
   enum TypeSpecifierSign {
@@ -106,7 +101,7 @@ namespace clang {
 static_assert(TST_error < 1 << 6, "Type bitfield not wide enough for TST");
 /*DeclSpec::TST*/ unsigned Type  : 6;
 /*DeclSpec::TSS*/ unsigned Sign  : 2;
-/*DeclSpec::TSW*/ unsigned Width : 2;
+/*TypeSpecifierWidth*/ unsigned Width : 2;
 unsigned ModeAttr : 1;
   };
 

diff  --git a/clang/include/clang/Sema/DeclSpec.h 
b/clang/include/clang/Sema/DeclSpec.h
index 0598d0d61b15..290384b13df1 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -249,13 +249,6 @@ class DeclSpec {
   static const TSCS TSCS_thread_local = clang::TSCS_thread_local;
   static const TSCS TSCS__Thread_local = clang::TSCS__Thread_local;
 
-  // Import type specifier width enumeration and constants.
-  typedef TypeSpecifierWidth TSW;
-  static const TSW TSW_unspecified = clang::TSW_unspecified;
-  static const TSW TSW_short = clang::TSW_short;
-  static const TSW TSW_long = clang::TSW_long;
-  static const TSW TSW_longlong = clang::TSW_longlong;
-
   enum TSC {
 TSC_unspecified,
 TSC_imaginary,
@@ -342,7 +335,7 @@ class DeclSpec {
   unsigned SCS_extern_in_linkage_spec : 1;
 
   // type-specifier
-  /*TSW*/unsigned TypeSpecWidth : 2;
+  /*TypeSpecifierWidth*/ unsigned TypeSpecWidth : 2;
   /*TSC*/unsigned TypeSpecComplex : 2;
   /*TSS*/unsigned TypeSpecSign : 2;
   /*TST*/unsigned TypeSpecType : 6;
@@ -434,17 +427,17 @@ class DeclSpec {
   DeclSpec(AttributeFactory &attrFactory)
   : StorageClassSpec(SCS_unspecified),
 ThreadStorageClassSpec(TSCS_unspecified),
-SCS_extern_in_linkage_spec(false), TypeSpecWidth(TSW_unspecified),
+SCS_extern_in_linkage_spec(false),
+TypeSpecWidth(static_cast(TypeSpecifierWidth::Unspecified)),
 TypeSpecComplex(TSC_unspecified), TypeSpecSign(TSS_unspecified),
 TypeSpecType(TST_unspecified), TypeAltiVecVector(false),
 TypeAltiVecPixel(false), TypeAltiVecBool(false), TypeSpecOwned(false),
 TypeSpecPipe(false), TypeSpecSat(false), ConstrainedAuto(false),
-TypeQualifiers(TQ_unspecified),
-FS_inline_specified(fa

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 305373.
njames93 added a comment.

Fix assertions failing with included files.
Ran this over the entire clang directory, which has a lot of macro (ab)use, and 
no crashes or otherwise unexpected behaviour.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91485

Files:
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
  clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
@@ -1,5 +1,4 @@
 // RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -fexceptions -std=c++17
-
 namespace std {
 struct string {
   string(const char *);
@@ -226,3 +225,89 @@
   }
   return;
 }
+
+void testPPConditionals() {
+
+  // These cases the return isn't inside the conditional so diagnose as normal.
+  if (true) {
+return;
+#if 1
+#endif
+  } else {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+return;
+  }
+  if (true) {
+#if 1
+#endif
+return;
+  } else {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+return;
+  }
+
+  // No return here in the AST, no special handling needed.
+  if (true) {
+#if 0
+return;
+#endif
+  } else {
+return;
+  }
+
+  // Return here is inside a preprocessor conditional block, ignore this case.
+  if (true) {
+#if 1
+return;
+#endif
+  } else {
+return;
+  }
+
+  // These cases, Same as above but with an #else block
+  if (true) {
+#if 1
+return;
+#else
+#endif
+  } else {
+return;
+  }
+  if (true) {
+#if 0
+#else
+return;
+#endif
+  } else {
+return;
+  }
+
+// ensure it can handle macros
+#define RETURN return
+  if (true) {
+#if 1
+RETURN;
+#endif
+  } else {
+return;
+  }
+#define ELSE else
+  if (true) {
+#if 1
+return;
+#endif
+  }
+  ELSE {
+return;
+  }
+
+  // Whole statement is in a conditional block so diagnost as normal.
+#if 1
+  if (true) {
+return;
+  } else {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+return;
+  }
+#endif
+}
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_ELSEAFTERRETURNCHECK_H
 
 #include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseMap.h"
 
 namespace clang {
 namespace tidy {
@@ -23,12 +24,18 @@
   ElseAfterReturnCheck(StringRef Name, ClangTidyContext *Context);
 
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
+  using ConditionalBranchMap =
+  llvm::DenseMap>;
+
 private:
   const bool WarnOnUnfixable;
   const bool WarnOnConditionVariables;
+  ConditionalBranchMap PPConditionals;
 };
 
 } // namespace readability
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -10,6 +10,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/FixIt.h"
 #include "llvm/ADT/SmallVector.h"
 
@@ -19,10 +20,30 @@
 namespace tidy {
 namespace readability {
 
-static const char ReturnStr[] = "return";
-static const char ContinueStr[] = "continue";
-static const char BreakStr[] = "break";
-static const char ThrowStr[] = "throw";
+namespace {
+
+class PPConditionalCollector : public PPCallbacks {
+public:
+  PPConditionalCollector(
+  ElseAfterReturnCheck::ConditionalBranchMap &Collections,
+  const SourceManager &SM)
+  : Collections(Collections), SM(SM) {}
+  void Endif(SourceLocation Loc, SourceLocation IfLoc) override {
+if (!SM.isWrittenInSameFile(Loc, IfLoc))
+  return;
+auto &Collection = Collections[SM.getFileID(Loc)];
+assert(Collection.empty() || Collection.back().getEnd() < Loc);
+Collection.emplace_back(IfLoc, Loc);
+  }
+
+private:
+  ElseAfterReturnCheck::Condi

[PATCH] D90871: [Sema] Fold VLAs to constant arrays in a few more contexts

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

The direction here seems fine to me.




Comment at: clang/include/clang/Sema/Sema.h:2478
 
-  Decl *ActOnDeclarator(Scope *S, Declarator &D);
+  Decl *ActOnDeclarator(Scope *S, Declarator &D, bool NextTokIsEqual = false);
 

Can we set a `HasInitializer` flag on `Declarator` instead (much like we 
provide a `FunctionDefinitionKind`)? This flag seems a bit ad-hoc (and also 
doesn't cover C++ braced initializers as named).


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

https://reviews.llvm.org/D90871

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

[PATCH] D91489: BPF: make __builtin_btf_type_id() return 64bit int

[Alexei Starovoitov via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/BPF/BTFDebug.cpp:1230
+Reloc == BPFCoreSharedInfo::BTF_TYPE_ID_REMOTE)
   OutMI.setOpcode(BPF::LD_imm64);
 else

libbpf would need to support both (ld_imm64 and mov32), right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91489

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

[PATCH] D84846: [MC] Add support for generating missing GNU build notes

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.
Herald added a subscriber: dexonsmith.

https://fedoraproject.org/wiki/Toolchain/Watermark Seems that 
`.gnu.build.attributes` is used with a GCC plugin (annobin?). The feature on 
its own is probably not that useful. `.gnu.linkonce` is a proto-comdat legacy 
feature which the patch should ignore.


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

https://reviews.llvm.org/D84846

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

[PATCH] D91506: [NFC, Refactor] Convert TypeSpecifiersPipe from Specifiers.h to a scoped enum (tiny)

[Thorsten via Phabricator via cfe-commits]

tschuett created this revision.
tschuett added reviewers: aaron.ballman, faisalv, wchilders.
Herald added subscribers: cfe-commits, dexonsmith, usaxena95, kadircet.
Herald added a project: clang.
tschuett requested review of this revision.
Herald added a subscriber: ilya-biryukov.

> ninja clang clangd (with assertions)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91506

Files:
  clang/include/clang/Basic/Specifiers.h
  clang/lib/Sema/DeclSpec.cpp


Index: clang/lib/Sema/DeclSpec.cpp
===
--- clang/lib/Sema/DeclSpec.cpp
+++ clang/lib/Sema/DeclSpec.cpp
@@ -877,7 +877,7 @@
   }
 
   if (isPipe) {
-TypeSpecPipe = TSP_pipe;
+TypeSpecPipe = static_cast(TypeSpecifiersPipe::Pipe);
   }
   return false;
 }
Index: clang/include/clang/Basic/Specifiers.h
===
--- clang/include/clang/Basic/Specifiers.h
+++ clang/include/clang/Basic/Specifiers.h
@@ -46,10 +46,7 @@
 TSS_unsigned
   };
 
-  enum TypeSpecifiersPipe {
-TSP_unspecified,
-TSP_pipe
-  };
+  enum class TypeSpecifiersPipe { Unspecified, Pipe };
 
   /// Specifies the kind of type.
   enum TypeSpecifierType {


Index: clang/lib/Sema/DeclSpec.cpp
===
--- clang/lib/Sema/DeclSpec.cpp
+++ clang/lib/Sema/DeclSpec.cpp
@@ -877,7 +877,7 @@
   }
 
   if (isPipe) {
-TypeSpecPipe = TSP_pipe;
+TypeSpecPipe = static_cast(TypeSpecifiersPipe::Pipe);
   }
   return false;
 }
Index: clang/include/clang/Basic/Specifiers.h
===
--- clang/include/clang/Basic/Specifiers.h
+++ clang/include/clang/Basic/Specifiers.h
@@ -46,10 +46,7 @@
 TSS_unsigned
   };
 
-  enum TypeSpecifiersPipe {
-TSP_unspecified,
-TSP_pipe
-  };
+  enum class TypeSpecifiersPipe { Unspecified, Pipe };
 
   /// Specifies the kind of type.
   enum TypeSpecifierType {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91489: BPF: make __builtin_btf_type_id() return 64bit int

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added inline comments.



Comment at: llvm/lib/Target/BPF/BTFDebug.cpp:1230
+Reloc == BPFCoreSharedInfo::BTF_TYPE_ID_REMOTE)
   OutMI.setOpcode(BPF::LD_imm64);
 else

ast wrote:
> libbpf would need to support both (ld_imm64 and mov32), right?
all selftests passed. I suspect libbpf already supports both ld_imm64 and 
mov32...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91489

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

[PATCH] D91507: [clang-format] Add option for case sensitive regexes for sorted includes

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: ioeric, MyDeveloperDay.
HazardyKnusperkeks added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
HazardyKnusperkeks requested review of this revision.

I think the title says everything.

I have one question, should I update the 
`clang/docs/ClangFormatStyleOptions.rst` by hand, or is it generated? If so how 
do I generate it?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91507

Files:
  clang/include/clang/Tooling/Inclusions/IncludeStyle.h
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
  clang/lib/Tooling/Inclusions/IncludeStyle.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/SortIncludesTest.cpp

Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -72,19 +72,19 @@
 TEST_F(SortIncludesTest, SortedIncludesUsingSortPriorityAttribute) {
   FmtStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup;
   FmtStyle.IncludeStyle.IncludeCategories = {
-  {"^", 1, 0},
-  {"^", 1, 1},
-  {"^", 8, 10},
-  {"^\".*\\.h\"", 10, 12}};
+  {"^", 1, 0, false},
+  {"^", 1, 1, false},
+  {"^", 8, 10, false},
+  {"^\".*\\.h\"", 10, 12, false}};
   EXPECT_EQ("#include \n"
 "#include \n"
 "#include \n"
@@ -587,8 +587,59 @@
  "a_TEST.cc"));
 }
 
+TEST_F(SortIncludesTest, SupportOptionalCaseSensitiveMachting) {
+  Style.IncludeBlocks = clang::tooling::IncludeStyle::IBS_Regroup;
+  Style.IncludeCategories = {{"^\"", 1, 0, false},
+ {"^<.*\\.h>$", 2, 0, false},
+ {"^", 3, 0, false},
+ {"^", 4, 0, false},
+ {"^<", 5, 0, false}};
+
+  StringRef UnsortedCode = "#include \n"
+   "#include \"qt.h\"\n"
+   "#include \n"
+   "#include \n"
+   "#include \n"
+   "#include \"qa.h\"\n"
+   "#include \n"
+   "#include \n"
+   "#include \n";
+
+  EXPECT_EQ("#include \"qa.h\"\n"
+"#include \"qt.h\"\n"
+"\n"
+"#include \n"
+"#include \n"
+"\n"
+"#include \n"
+"#include \n"
+"#include \n"
+"#include \n"
+"\n"
+"#include \n",
+sort(UnsortedCode));
+
+  Style.IncludeCategories[2].RegexIsCaseSensitive = true;
+  Style.IncludeCategories[3].RegexIsCaseSensitive = true;
+  EXPECT_EQ("#include \"qa.h\"\n"
+"#include \"qt.h\"\n"
+"\n"
+"#include \n"
+"#include \n"
+"\n"
+"#include \n"
+"#include \n"
+"\n"
+"#include \n"
+"\n"
+"#include \n"
+"#include \n",
+sort(UnsortedCode));
+}
+
 TEST_F(SortIncludesTest, NegativePriorities) {
-  Style.IncludeCategories = {{".*important_os_header.*", -1, 0}, {".*", 1, 0}};
+  Style.IncludeCategories = {{".*important_os_header.*", -1, 0, false},
+ {".*", 1, 0, false}};
   EXPECT_EQ("#include \"important_os_header.h\"\n"
 "#include \"c_main.h\"\n"
 "#include \"a_other.h\"\n",
@@ -608,7 +659,8 @@
 }
 
 TEST_F(SortIncludesTest, PriorityGroupsAreSeparatedWhenRegroupping) {
-  Style.IncludeCategories = {{".*important_os_header.*", -1, 0}, {".*", 1, 0}};
+  Style.IncludeCategories = {{".*important_os_header.*", -1, 0, false},
+ {".*", 1, 0, false}};
   Style.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup;
 
   EXPECT_EQ("#include \"important_os_header.h\"\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14524,12 +14524,13 @@
 
   Style.IncludeStyle.IncludeCategories.clear();
   std::vector ExpectedCategories = {
-  {"abc/.*", 2, 0}, {".*", 1, 0}};
+  {"abc/.*", 2, 0, false}, {".*", 1, 0, true}};
   CHECK_PARSE("IncludeCategories:\n"
   "  - Regex: abc/.*\n"
   "Priority: 2\n"
   "  - Regex: .*\n"
-  "Priority: 1",
+  "Priority: 1\n"
+  "CaseSensitive: true\n",
   IncludeStyle.IncludeCategories, ExpectedCategories);
   CHECK_PARSE("IncludeIsMainRegex: 'abc$'", IncludeStyle.IncludeIsMainRegex,
   "abc$");
Index: clang/lib/Tooling/Inclusions/IncludeStyle.cpp
===
--- clang/lib/Tooling/Inclusions/IncludeStyle.c

[PATCH] D91489: BPF: make __builtin_btf_type_id() return 64bit int

[Andrii Nakryiko via Phabricator via cfe-commits]

anakryiko added inline comments.



Comment at: llvm/lib/Target/BPF/BTFDebug.cpp:1230
+Reloc == BPFCoreSharedInfo::BTF_TYPE_ID_REMOTE)
   OutMI.setOpcode(BPF::LD_imm64);
 else

yonghong-song wrote:
> ast wrote:
> > libbpf would need to support both (ld_imm64 and mov32), right?
> all selftests passed. I suspect libbpf already supports both ld_imm64 and 
> mov32...
there is no need to make BTF_TYPE_ID_LOCAL to return u64, but if it helps 
uniformity, then why not.

libbpf does support ldimm64 in general, but I'll need to extend this 
specifically for BTF_TYPE_ID_REMOTE to record module BTF ID in upper 32 bits. 
For now it will be just zeroes, which works fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91489

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

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

[Kocsis Ábel via Phabricator via cfe-commits]

abelkocsis updated this revision to Diff 305388.
abelkocsis added a comment.

Some fixes. Needs further improvement


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D75229

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-signal-in-multithreaded-program.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-con37-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-std_thread.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-thrd_create.c
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-std_thread.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-thrd_create.c

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-thrd_create.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-thrd_create.c
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy %s bugprone-signal-in-multithreaded-program %t
+
+typedef unsigned long int thrd_t;
+typedef int (*thrd_start_t)(void *);
+typedef int sig_atomic_t;
+#define SIGUSR1 30
+#define NULL 0
+
+void (*signal(int sig, void (*handler)(int)))(int);
+
+int thrd_create(thrd_t *thr, thrd_start_t func, void *arg) { return 0; };
+enum {
+  thrd_success = 0,
+};
+
+volatile sig_atomic_t flag = 0;
+
+void handler(int signum) {
+  flag = 1;
+}
+
+int func(void *data) {
+  while (!flag) {
+  }
+  return 0;
+}
+
+int main(void) {
+  signal(SIGUSR1, handler);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: signal function should not be called in a multithreaded program [bugprone-signal-in-multithreaded-program]
+  thrd_t tid;
+
+  if (thrd_success != thrd_create(&tid, func, NULL)) {
+  }
+  return 0;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-std_thread.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-noconfig-std_thread.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s bugprone-signal-in-multithreaded-program %t
+
+typedef int sig_atomic_t;
+#define SIGUSR1 30
+#define NULL 0
+
+void (*signal(int sig, void (*handler)(int)))(int);
+
+volatile sig_atomic_t flag = 0;
+
+void handler(int signum) {
+  flag = 1;
+}
+
+void threadFunction() {}
+
+namespace std {
+class thread {
+public:
+  thread() noexcept;
+  template 
+  explicit thread(Function &&f, Args &&... args);
+  thread(const thread &) = delete;
+  thread(thread &&) noexcept;
+};
+} // namespace std
+
+int main() {
+  signal(SIGUSR1, handler);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: signal function should not be called in a multithreaded program [bugprone-signal-in-multithreaded-program]
+
+  std::thread threadObj(threadFunction);
+
+  return 0;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-thrd_create.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-thrd_create.c
@@ -0,0 +1,38 @@
+// RUN: %check_clang_tidy %s bugprone-signal-in-multithreaded-program %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: bugprone-signal-in-multithreaded-program.ThreadList, value: "thrd_create"}]}'
+
+typedef unsigned long int thrd_t;
+typedef int (*thrd_start_t)(void *);
+typedef int sig_atomic_t;
+#define SIGUSR1 30
+#define NULL 0
+
+void (*signal(int sig, void (*handler)(int)))(int);
+
+int thrd_create(thrd_t *thr, thrd_start_t func, void *arg) { return 0; };
+enum {
+  thrd_success = 0,
+};
+
+volatile sig_atomic_t flag = 0;
+
+void handler(int signum) {
+  flag = 1;
+}
+
+int func(void *data) {
+  while (!flag) {
+  }
+  return 0;
+}
+
+int main(void) {
+  signal(SIGUSR1, handler);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: signal function should not be called in a multithreaded program [bugprone-signal-in-multithreaded-program]
+  thrd_t tid;
+
+  if (thrd_success != thrd_create(&tid, func, NULL)) {
+  }
+  return 0;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-in-multithreaded-program-config-std_thread.cpp
===
--- /dev/null
+++ 

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith updated this revision to Diff 305392.
rsmith added a comment.

Improve commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/test/CodeGenCXX/clang-abi-compat.cpp
  clang/test/CodeGenCXX/mangle-class-nttp.cpp
  clang/test/CodeGenCXX/mangle-template.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -187,6 +187,11 @@
 int &r = f(B<&a>());
 float &s = f(B<&b>());
 
+void type_affects_identity(B<&a>) {}
+void type_affects_identity(B<(const int*)&a>) {}
+void type_affects_identity(B<(void*)&a>) {}
+void type_affects_identity(B<(const void*)&a>) {}
+
 // pointers to members
 template struct B {};
 template struct B {};
@@ -198,6 +203,12 @@
 char &u = f(B<&X::p>());
 short &v = f(B<&X::pp>());
 
+struct Y : X {};
+void type_affects_identity(B<&X::n>) {}
+void type_affects_identity(B<(int Y::*)&X::n>) {} // FIXME: expected-error {{sorry}}
+void type_affects_identity(B<(const int X::*)&X::n>) {}
+void type_affects_identity(B<(const int Y::*)&X::n>) {} // FIXME: expected-error {{sorry}}
+
 // A case where we need to do auto-deduction, and check whether the
 // resulting dependent types match during partial ordering. These
 // templates are not ordered due to the mismatching function parameter.
Index: clang/test/CodeGenCXX/mangle-template.cpp
===
--- clang/test/CodeGenCXX/mangle-template.cpp
+++ clang/test/CodeGenCXX/mangle-template.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++11 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+// RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++20 -emit-llvm -triple x86_64-linux-gnu -o - %s | FileCheck %s --check-prefixes=CHECK,CXX20
 // expected-no-diagnostics
 
 namespace test1 {
@@ -212,3 +213,39 @@
 template __make_integer_seq make<5>();
 // CHECK: define weak_odr {{.*}} @_ZN6test154makeILi5EEE18__make_integer_seqISt16integer_sequenceiXT_EEv(
 }
+
+#if __cplusplus >= 202002L
+namespace cxx20 {
+  template struct A {};
+  template struct B {};
+
+  int x;
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1xE(
+  void f(A<&x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKiadL_ZNS_1xE(
+  void f(A<(const int*)&x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPvadL_ZNS_1xE(
+  void f(A<(void*)&x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPvXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKvadL_ZNS_1xE(
+  void f(A<(const void*)&x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKvXadL_ZNS_1xE(
+  void f(B) {}
+
+  struct Q { int x; };
+
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1Q1xE(
+  void f(A<&Q::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEiXadL_ZNS1_1xE
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvMNS_1QEKiadL_ZNS1_1xE(
+  void f(A<(const int Q::*)&Q::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEKiXadL_ZNS1_1xE(
+  void f(B) {}
+}
+#endif
Index: clang/test/CodeGenCXX/mangle-class-nttp.cpp
===
--- clang/test/CodeGenCXX/mangle-class-nttp.cpp
+++ clang/test/CodeGenCXX/mangle-class-nttp.cpp
@@ -117,7 +117,7 @@
 template void f() {}
 
 // Union members.
-// CHECK: define weak_odr void @_Z1fIXL1vv(
+// CHECK: define weak_odr void @_Z1fIL1EEEvv(
 // FIXME: MSVC rejects this; check this is the mangling MSVC uses when they
 // start accepting.
 // MSABI: define {{.*}} @"??$f@$2TE@YAXXZ"
@@ -209,10 +209,10 @@
 template void f() {}
 template void f() {}
 template void f() {}
-// CHECK: define weak_odr void @_Z1fIXL2H1EEEvv
+// CHECK: define weak_odr void @_Z1fIL2H1EEvv
 // MSABI: define {{.*}} @"??$f@$2TH1@YAXXZ"
 template void f();
-// CHECK: define weak_odr void @_Z1fIXL2H2EEEvv
+// CHECK: define weak_odr void @_Z1fIL2H2EEvv
 // MSABI: define {{.*}} @"??$f@$2TH2@YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl2H3EEEvv
Index: clang/test/CodeGenCXX/clang-abi-compat.cpp
===
--- clang/test/CodeGenCXX/clang-abi-compat.cpp
+++ clang/test/CodeGenCXX/clang-abi-compat.cpp
@@ -1,9 +1,12 @@
-// RUN: %clang_cc1 -

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith updated this revision to Diff 305393.
rsmith added a comment.

Tweak wording of comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/test/CodeGenCXX/clang-abi-compat.cpp
  clang/test/CodeGenCXX/mangle-class-nttp.cpp
  clang/test/CodeGenCXX/mangle-template.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -187,6 +187,11 @@
 int &r = f(B<&a>());
 float &s = f(B<&b>());
 
+void type_affects_identity(B<&a>) {}
+void type_affects_identity(B<(const int*)&a>) {}
+void type_affects_identity(B<(void*)&a>) {}
+void type_affects_identity(B<(const void*)&a>) {}
+
 // pointers to members
 template struct B {};
 template struct B {};
@@ -198,6 +203,12 @@
 char &u = f(B<&X::p>());
 short &v = f(B<&X::pp>());
 
+struct Y : X {};
+void type_affects_identity(B<&X::n>) {}
+void type_affects_identity(B<(int Y::*)&X::n>) {} // FIXME: expected-error {{sorry}}
+void type_affects_identity(B<(const int X::*)&X::n>) {}
+void type_affects_identity(B<(const int Y::*)&X::n>) {} // FIXME: expected-error {{sorry}}
+
 // A case where we need to do auto-deduction, and check whether the
 // resulting dependent types match during partial ordering. These
 // templates are not ordered due to the mismatching function parameter.
Index: clang/test/CodeGenCXX/mangle-template.cpp
===
--- clang/test/CodeGenCXX/mangle-template.cpp
+++ clang/test/CodeGenCXX/mangle-template.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++11 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+// RUN: %clang_cc1 -verify -Wno-return-type -Wno-main -std=c++20 -emit-llvm -triple x86_64-linux-gnu -o - %s | FileCheck %s --check-prefixes=CHECK,CXX20
 // expected-no-diagnostics
 
 namespace test1 {
@@ -212,3 +213,39 @@
 template __make_integer_seq make<5>();
 // CHECK: define weak_odr {{.*}} @_ZN6test154makeILi5EEE18__make_integer_seqISt16integer_sequenceiXT_EEv(
 }
+
+#if __cplusplus >= 202002L
+namespace cxx20 {
+  template struct A {};
+  template struct B {};
+
+  int x;
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1xE(
+  void f(A<&x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKiadL_ZNS_1xE(
+  void f(A<(const int*)&x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKiXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPvadL_ZNS_1xE(
+  void f(A<(void*)&x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPvXadL_ZNS_1xE(
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvPKvadL_ZNS_1xE(
+  void f(A<(const void*)&x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIPKvXadL_ZNS_1xE(
+  void f(B) {}
+
+  struct Q { int x; };
+
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXadL_ZNS_1Q1xE(
+  void f(A<&Q::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEiXadL_ZNS1_1xE
+  void f(B) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1AIXcvMNS_1QEKiadL_ZNS1_1xE(
+  void f(A<(const int Q::*)&Q::x>) {}
+  // CXX20: define {{.*}} @_ZN5cxx201fENS_1BIMNS_1QEKiXadL_ZNS1_1xE(
+  void f(B) {}
+}
+#endif
Index: clang/test/CodeGenCXX/mangle-class-nttp.cpp
===
--- clang/test/CodeGenCXX/mangle-class-nttp.cpp
+++ clang/test/CodeGenCXX/mangle-class-nttp.cpp
@@ -117,7 +117,7 @@
 template void f() {}
 
 // Union members.
-// CHECK: define weak_odr void @_Z1fIXL1vv(
+// CHECK: define weak_odr void @_Z1fIL1EEEvv(
 // FIXME: MSVC rejects this; check this is the mangling MSVC uses when they
 // start accepting.
 // MSABI: define {{.*}} @"??$f@$2TE@YAXXZ"
@@ -209,10 +209,10 @@
 template void f() {}
 template void f() {}
 template void f() {}
-// CHECK: define weak_odr void @_Z1fIXL2H1EEEvv
+// CHECK: define weak_odr void @_Z1fIL2H1EEvv
 // MSABI: define {{.*}} @"??$f@$2TH1@YAXXZ"
 template void f();
-// CHECK: define weak_odr void @_Z1fIXL2H2EEEvv
+// CHECK: define weak_odr void @_Z1fIL2H2EEvv
 // MSABI: define {{.*}} @"??$f@$2TH2@YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl2H3EEEvv
Index: clang/test/CodeGenCXX/clang-abi-compat.cpp
===
--- clang/test/CodeGenCXX/clang-abi-compat.cpp
+++ clang/test/CodeGenCXX/clang-abi-compat.cpp
@@ -1,9 +1,12 @@
-// RUN: %clang_cc1

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:4861
+// We need correct types when the template-name is unresolved or when it
+// might be overloaded.
+if (!ResolvedTemplate)

Quuxplusone wrote:
> And from the PR summary:
> 
> > namely mangling such template arguments as being cast to the parameter type 
> > in the case where the template name is unresolved or overloaded
> 
> This phrasing worries me a little bit. Are you saying that you might mangle 
> the name of `foo` in one way, when there's also a `foo Args>` in scope, and in a different way, when there's not? That doesn't seem 
> conforming. So I imagine it's more likely that I'm misunderstanding what you 
> mean by "might be overloaded" / "is overloaded". Could you explain for my 
> information, and perhaps also adjust the wording of these code comments to 
> make the explanation less needed?
> 
> Specifically, I think it would be non-conforming if the TU
> 
> // https://godbolt.org/z/YjPqMd
> template void foo();
> char arr[6];
> extern template void foo();  // #1
> int main() { foo(); }
> 
> could not be linked against the TU
> 
> template int foo();
> template void foo();  // is this "overloading"?
> extern char arr[6];
> template<> void foo() {}  // #2
> 
> because lines #1 and #2 disagreed about the way to mangle `foo`. (Again, 
> I'm pretty sure you haven't made them disagree... but I remain unclear on 
> what's meant by "overloading" in this PR, if it's //not// this.)
Well, technically, a single function template is considered overloaded all by 
itself :) ... but no, this is a typo in the change description. I meant 
"overloadable", not "overloaded". The proposed Itanium ABI rule applies to 
function templates (other than the call operator or conversion function of a 
generic lambda).

The "might be overloaded" here means "might be overloaded by some other 
(earlier or later or in a different TU) declaration", so I don't think that's 
wrong, but I'll rephrase it for the avoidance of any doubt.

(In principle the ABI rule also applies to cases where the template-name is 
unresolved, but it only makes a difference if the type of the template 
parameter is known and the template argument is not (eg, if it's the argument 
of a template template parameter). I suspect that's actually impossible, 
because the expression or type would need to be instantiation-dependent in that 
case, so we'd mangle the original syntax for the template argument not the 
result of converting it to the parameter type.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91488

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

[PATCH] D87928: Provide -fsource-dir flag in Clang

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D87928#2394913 , @dexonsmith wrote:

> I missed this before, and it's an interesting problem.
>
> I have a few questions:
>
> - I'm not clear on the expected interaction between the current working 
> directory of the compiler and `-fsource-dir`. What happens if they're 
> different?

They're completely independent, `-fsource-dir` is only used for coverage 
mapping and macros.

> - I'm curious if you've considered other solutions that tie the source 
> directory more closely to the current working directory, such as a flag 
> `-frelative-to-working-dir`, which makes things relative to the compiler's 
> working directory. Is that too restrictive?

I think it depends on the use cases and user expectations. For example, in our 
case we have layout like this:

  /working/dir/
   sources/
   lib/a/a.c
   include/a/a.h
   out/
   default/

Here `out/default` is the working directory so if we used 
`-frelative-to-working-dir`, we would end up with paths like 
`../../sources/lib/a/a.c` in the coverage mapping or macro expansions like 
`__FILE__`. While this may be acceptable for macro expansions, it's definitely 
undesirable in coverage mapping.

I can think of some potential solutions. We use could use 
`-fprofile-prefix-map=../../=` to strip the `../../` prefix. Alternatively we 
could introduce a new flags in tools like `llvm-cov` and `llvm-profdata` to 
strip the prefix during post-processing.

> - Have you looked at the similar "MakeRelative" logic in `ASTWriter.cpp`? Can 
> this logic be shared somehow?

I wasn't aware of that bit but once we decide on the approach, I'll try to 
deduplicate the logic.

> - What's the expected behaviour when building modules implicitly with 
> `-fmodules`? What value (if any) should they get for `-fsource-dir`?

Right now this flag doesn't apply to modules at all so there's no effect.

> - How should this affect the dependency output (e.g., `clang-scan-deps` and 
> `.d` files), if at all?

The same as for modules. We could consider extending it to support modules and 
dependency files as well although I'd prefer to do it separately.

> - Have you considered having `-fsource-dir` add entries to the prefix maps, 
> rather than duplicating logic?

That might be an option if we land D83154  
first. It wasn't clear if we're going to have both but it might make sense to 
introduce `-fprofile-prefix-map` and then build `-fsource-dir` on top.

> I am a bit concerned about mixing the concepts of "relative base path" with 
> "source directory", since the latter sounds to me like where pristine sources 
> live and that's not always the right relative base path.
>
> For example, consider a layout like this:
>
>   /working/dir/
>sources/
>lib/a/a.c
>include/a/a.h
>include/a/a.td
>build/
>  include/lib/a/a.inc
>  lib/a/a.o
>toolchain/usr/
>  bin/clang
>  lib/clang/11.0/include/stdint.h
>sdk/usr/include/
>stdint.h
>c++/v1/
>   vector
>   stdint.h
>
> It seems cleaner to have everything relative to `/working/dir`, not 
> `/working/dir/sources`, since you can name all compiler inputs and outputs 
> with the former. But the latter is conceptually the source directory.

Yes, I agree. Ultimately it's up to the user to decide what output they expect. 
I guess this is one advantage of using the `-frelative-to-working-dir` approach 
where it's unambiguous and user can use other flags to do additional rewriting 
if needed.

> WDYT?

Thanks for the detailed feedback!




Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1545-1547
+  StringRef SourceDir = PPOpts->SourceDir;
+  if (!SourceDir.empty() && FN.startswith(SourceDir))
+llvm::sys::path::make_relative(FN, SourceDir, FN);

dexonsmith wrote:
> What does this do with `-fsource-dir /a/b` for the file `/a/b.c`? (It doesn't 
> seem quite right for it to turn it into `.c` or `../b.c`...)
That's definitely undesirable. We could introduce a new function like 
`llvm::sys::path::has_prefix` to compare path components instead pure string 
comparison to avoid this issue.


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

https://reviews.llvm.org/D87928

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

[PATCH] D91131: [clangd] Bump index version number.

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

My apologies for not thinking of this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91131

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

[PATCH] D91025: [clangd] Fix locateMacroAt() for macro definition outside preamble

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a reviewer: sammccall.
nridge added a comment.

Sam, you're my go-to reviewer for "tricky macro stuff", but feel free to hand 
off if you prefer :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91025

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

[PATCH] D83154: clang: Add -fcoverage-prefix-map

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM we may consider implementing `-fsource-dir` in terms of 
`-fprofile-prefix-map` as suggested in D87928  
so I think we should go ahead and land this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83154

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

[PATCH] D91509: [clangd] Add AST config to prebuild ASTs

[Daan De Meyer via Phabricator via cfe-commits]

DaanDeMeyer updated this revision to Diff 305394.
DaanDeMeyer retitled this revision from "[clangd] RFC: Add --prebuild-asts 
option" to "[clangd] Add AST config to prebuild ASTs".
DaanDeMeyer edited the summary of this revision.
DaanDeMeyer added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Switched from all or nothing command line option to per file config.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91509

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  clang-tools-extra/clangd/QueryDriverDatabase.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h

Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -231,6 +231,8 @@
   /// Returns true if the file was not previously tracked.
   bool update(PathRef File, ParseInputs Inputs, WantDiagnostics WD);
 
+  bool hasFile(PathRef File);
+
   /// Remove \p File from the list of tracked files and schedule removal of its
   /// resources. Pending diagnostics for closed files may not be delivered, even
   /// if requested with WantDiags::Auto or WantDiags::Yes.
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1289,6 +1289,8 @@
   return NewFile;
 }
 
+bool TUScheduler::hasFile(PathRef File) { return Files[File] == nullptr; }
+
 void TUScheduler::remove(PathRef File) {
   bool Removed = Files.erase(File);
   if (!Removed)
Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -277,6 +277,10 @@
 return addSystemIncludes(*Cmd, SystemIncludes);
   }
 
+  tooling::CompilationDatabase *lookupCDB(PathRef File) const override {
+return Base->lookupCDB(File);
+  }
+
   llvm::Optional getProjectInfo(PathRef File) const override {
 return Base->getProjectInfo(File);
   }
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -40,6 +40,8 @@
   virtual llvm::Optional
   getCompileCommand(PathRef File) const = 0;
 
+  virtual tooling::CompilationDatabase *lookupCDB(PathRef File) const = 0;
+
   /// Finds the closest project to \p File.
   virtual llvm::Optional getProjectInfo(PathRef File) const {
 return llvm::None;
@@ -76,6 +78,8 @@
   llvm::Optional
   getCompileCommand(PathRef File) const override;
 
+  virtual tooling::CompilationDatabase *lookupCDB(PathRef File) const override;
+
   /// Returns the path to first directory containing a compilation database in
   /// \p File's parents.
   llvm::Optional getProjectInfo(PathRef File) const override;
@@ -132,6 +136,9 @@
 
   llvm::Optional
   getCompileCommand(PathRef File) const override;
+
+  tooling::CompilationDatabase *lookupCDB(PathRef File) const override;
+
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;
   /// Project info is gathered purely from the inner compilation database to
   /// ensure consistency.
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -68,6 +68,20 @@
 
 llvm::Optional
 DirectoryBasedGlobalCompilationDatabase::getCompileCommand(PathRef File) const {
+  auto *CDB = lookupCDB(File);
+  if (!CDB) {
+return llvm::None;
+  }
+
+  auto Candidates = CDB->getCompileCommands(File);
+  if (!Candidates.empty())
+return std::move(Candidates.front());
+
+  return None;
+}
+
+tooling::CompilationDatabase *
+DirectoryBasedGlobalCompilationDatabase::lookupCDB(PathRef File) const {
   CDBLookupRequest Req;
   Req.FileName = File;
   Req.ShouldBroadcast = true;
@@ -75,14 +89,10 @@
   auto Res = lookupCDB(Req);
   if (!Res) {
 log("Failed to find compilation database for {0}", File);
-return llvm::None;
+return nullptr;
   }
 
-  auto Candidates = Res->CDB->getCompileCommands(File);
-  if (!Candidates.empty())
-return std::move(Candidates.front());
-
-  return None;
+  return Res->CDB;
 }
 
 // For platforms w

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

[Shane via Phabricator via cfe-commits]

smhc updated this revision to Diff 305395.
smhc added a comment.

Re-based to llvm master and re-diffed.


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

https://reviews.llvm.org/D90282

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-short-names.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-short-names.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-short-names.cpp
@@ -0,0 +1,26 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ParameterShortSizeThreshold, value: 2}, \
+// RUN: {key: readability-identifier-naming.ClassShortSizeThreshold, value: 3} \
+// RUN:  ]}'
+
+int testFunc(int a, char **b);
+int testFunc(int ab, char **ba);
+int testFunc(int abc, char **cba);
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'abc'
+// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'cba'
+// CHECK-FIXES: {{^}}int testFunc(int Abc, char **Cba);{{$}}
+int testFunc(int Abc, char **Cba);
+
+class fo {
+};
+
+class foo {
+};
+
+class fooo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'fooo'
+  // CHECK-FIXES: {{^}}class Fooo {{{$}}
+};
Index: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
@@ -20,7 +20,8 @@
  - ``aNy_CasE``.
 
 It also supports a fixed prefix and suffix that will be prepended or appended
-to the identifiers, regardless of the casing.
+to the identifiers, regardless of the casing. A threshold for the length of
+the identifer may be specified to suppress the checks for short names.
 
 Many configuration options are available, in order to be able to create
 different rules for different kinds of identifiers. In general, the rules are
@@ -35,60 +36,60 @@
 
 The following options are describe below:
 
- - :option:`AbstractClassCase`, :option:`AbstractClassPrefix`, :option:`AbstractClassSuffix`
+ - :option:`AbstractClassCase`, :option:`AbstractClassPrefix`, :option:`AbstractClassSuffix`, :option:`AbstractClassShortSizeThreshold`
  - :option:`AggressiveDependentMemberLookup`
- - :option:`ClassCase`, :option:`ClassPrefix`, :option:`ClassSuffix`
- - :option:`ClassConstantCase`, :option:`ClassConstantPrefix`, :option:`ClassConstantSuffix`
- - :option:`ClassMemberCase`, :option:`ClassMemberPrefix`, :option:`ClassMemberSuffix`
- - :option:`ClassMethodCase`, :option:`ClassMethodPrefix`, :option:`ClassMethodSuffix`
- - :option:`ConstantCase`, :option:`ConstantPrefix`, :option:`ConstantSuffix`
- - :option:`ConstantMemberCase`, :option:`ConstantMemberPrefix`, :option:`ConstantMemberSuffix`
- - :option:`ConstantParameterCase`, :option:`ConstantParameterPrefix`, :option:`ConstantParameterSuffix`
- - :option:`ConstantPointerParameterCase`, :option:`ConstantPointerParameterPrefix`, :option:`ConstantPointerParameterSuffix`
- - :option:`ConstexprFunctionCase`, :option:`ConstexprFunctionPrefix`, :option:`ConstexprFunctionSuffix`
- - :option:`ConstexprMethodCase`, :option:`ConstexprMethodPrefix`, :option:`ConstexprMethodSuffix`
- - :option:`ConstexprVariableCase`, :option:`ConstexprVariablePrefix`, :option:`ConstexprVariableSuffix`
- - :option:`EnumCase`, :option:`EnumPrefix`, :option:`EnumSuffix`
- - :option:`EnumConstantCase`, :option:`EnumConstantPrefix`, :option:`EnumConstantSuffix`
- - :option:`FunctionCase`, :option:`FunctionPrefix`, :option:`FunctionSuffix`
+ - :option:`ClassCase`, :option:`ClassPrefix`, :option:`ClassSuffix`, :option:`ClassShortSizeThreshold`
+ - :option:`ClassConstantCase`, :option:`ClassConstantPrefix`, :option:`ClassConstantSuffix`, :option:`ClassConstantShortSizeThreshold`
+ - :option:`ClassMemberCase`, :option:`ClassMemberPrefix`, :option:`ClassMemberSuffix`, :option:`ClassMemberShortSizeThreshold`
+ - :option:`ClassMethodCase`, :option:`ClassMethodPrefix`, :option:`ClassMethodSuffix`, :option:`ClassMethodShortSizeThreshold`
+ - :option:`ConstantCase`, :option:`ConstantPrefix`, :option:`ConstantSuffix`, :option:`ConstantShortSizeThreshold`
+ - :option:`ConstantMemberCase`, :option:`ConstantMemberPrefix`, :option:`ConstantMemberSuffix`, 

[PATCH] D91509: [clangd] Add AST config to prebuild ASTs

[Daan De Meyer via Phabricator via cfe-commits]

DaanDeMeyer updated this revision to Diff 305396.
DaanDeMeyer added a comment.

Fixed formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91509

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  clang-tools-extra/clangd/QueryDriverDatabase.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h

Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -231,6 +231,8 @@
   /// Returns true if the file was not previously tracked.
   bool update(PathRef File, ParseInputs Inputs, WantDiagnostics WD);
 
+  bool hasFile(PathRef File);
+
   /// Remove \p File from the list of tracked files and schedule removal of its
   /// resources. Pending diagnostics for closed files may not be delivered, even
   /// if requested with WantDiags::Auto or WantDiags::Yes.
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1289,6 +1289,8 @@
   return NewFile;
 }
 
+bool TUScheduler::hasFile(PathRef File) { return Files[File] == nullptr; }
+
 void TUScheduler::remove(PathRef File) {
   bool Removed = Files.erase(File);
   if (!Removed)
Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -277,6 +277,10 @@
 return addSystemIncludes(*Cmd, SystemIncludes);
   }
 
+  tooling::CompilationDatabase *lookupCDB(PathRef File) const override {
+return Base->lookupCDB(File);
+  }
+
   llvm::Optional getProjectInfo(PathRef File) const override {
 return Base->getProjectInfo(File);
   }
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -40,6 +40,8 @@
   virtual llvm::Optional
   getCompileCommand(PathRef File) const = 0;
 
+  virtual tooling::CompilationDatabase *lookupCDB(PathRef File) const = 0;
+
   /// Finds the closest project to \p File.
   virtual llvm::Optional getProjectInfo(PathRef File) const {
 return llvm::None;
@@ -76,6 +78,8 @@
   llvm::Optional
   getCompileCommand(PathRef File) const override;
 
+  virtual tooling::CompilationDatabase *lookupCDB(PathRef File) const override;
+
   /// Returns the path to first directory containing a compilation database in
   /// \p File's parents.
   llvm::Optional getProjectInfo(PathRef File) const override;
@@ -132,6 +136,9 @@
 
   llvm::Optional
   getCompileCommand(PathRef File) const override;
+
+  tooling::CompilationDatabase *lookupCDB(PathRef File) const override;
+
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;
   /// Project info is gathered purely from the inner compilation database to
   /// ensure consistency.
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -68,6 +68,20 @@
 
 llvm::Optional
 DirectoryBasedGlobalCompilationDatabase::getCompileCommand(PathRef File) const {
+  auto *CDB = lookupCDB(File);
+  if (!CDB) {
+return llvm::None;
+  }
+
+  auto Candidates = CDB->getCompileCommands(File);
+  if (!Candidates.empty())
+return std::move(Candidates.front());
+
+  return None;
+}
+
+tooling::CompilationDatabase *
+DirectoryBasedGlobalCompilationDatabase::lookupCDB(PathRef File) const {
   CDBLookupRequest Req;
   Req.FileName = File;
   Req.ShouldBroadcast = true;
@@ -75,14 +89,10 @@
   auto Res = lookupCDB(Req);
   if (!Res) {
 log("Failed to find compilation database for {0}", File);
-return llvm::None;
+return nullptr;
   }
 
-  auto Candidates = Res->CDB->getCompileCommands(File);
-  if (!Candidates.empty())
-return std::move(Candidates.front());
-
-  return None;
+  return Res->CDB;
 }
 
 // For platforms where paths are case-insensitive (but case-preserving),
@@ -270,6 +280,10 @@
   return Cmd;
 }
 
+tooling::CompilationDatabase *OverlayCDB::lookupCDB(PathRef File) const {
+  return Base ? Base->lookupCDB(File) : nullptr;
+}
+
 tooling::CompileCommand OverlayCDB::getFallbackCommand(PathRef File) const {

[PATCH] D89296: [clangd] Call hierarchy (Protocol layer)

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 305397.
nridge marked 7 inline comments as done.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89296

Files:
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h

Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1375,7 +1375,7 @@
   /// descendants. If not defined, the children have not been resolved.
   llvm::Optional> children;
 
-  /// An optional 'data' filed, which can be used to identify a type hierarchy
+  /// An optional 'data' field, which can be used to identify a type hierarchy
   /// item in a resolve request.
   llvm::Optional data;
 };
@@ -1397,6 +1397,83 @@
 bool fromJSON(const llvm::json::Value &, ResolveTypeHierarchyItemParams &,
   llvm::json::Path);
 
+enum class SymbolTag { Deprecated = 1 };
+llvm::json::Value toJSON(SymbolTag);
+
+/// The parameter of a `textDocument/prepareCallHierarchy` request.
+struct CallHierarchyPrepareParams : public TextDocumentPositionParams {};
+
+/// Represents programming constructs like functions or constructors
+/// in the context of call hierarchy.
+struct CallHierarchyItem {
+  /// The name of this item.
+  std::string name;
+
+  /// The kind of this item.
+  SymbolKind kind;
+
+  /// Tags for this item.
+  std::vector tags;
+
+  /// More detaill for this item, e.g. the signature of a function.
+  std::string detail;
+
+  /// The resource identifier of this item.
+  URIForFile uri;
+
+  /// The range enclosing this symbol not including leading / trailing
+  /// whitespace but everything else, e.g. comments and code.
+  Range range;
+
+  /// The range that should be selected and revealed when this symbol
+  /// is being picked, e.g. the name of a function.
+  /// Must be contained by `Rng`.
+  Range selectionRange;
+
+  /// An optional 'data' field, which can be used to identify a call
+  /// hierarchy item in an incomingCalls or outgoingCalls request.
+  std::string data;
+};
+llvm::json::Value toJSON(const CallHierarchyItem &);
+bool fromJSON(const llvm::json::Value &, CallHierarchyItem &, llvm::json::Path);
+
+/// The parameter of a `callHierarchy/incomingCalls` request.
+struct CallHierarchyIncomingCallsParams {
+  CallHierarchyItem Item;
+};
+bool fromJSON(const llvm::json::Value &, CallHierarchyIncomingCallsParams &,
+  llvm::json::Path);
+
+/// Represents an incoming call, e.g. a caller of a method or constructor.
+struct CallHierarchyIncomingCall {
+  /// The item that makes the call.
+  CallHierarchyItem From;
+
+  /// The range at which the calls appear.
+  /// This is relative to the caller denoted by `From`.
+  std::vector FromRanges;
+};
+llvm::json::Value toJSON(const CallHierarchyIncomingCall &);
+
+/// The parameter of a `callHierarchy/outgoingCalls` request.
+struct CallHierarchyOutgoingCallsParams {
+  CallHierarchyItem Item;
+};
+bool fromJSON(const llvm::json::Value &, CallHierarchyOutgoingCallsParams &,
+  llvm::json::Path);
+
+/// Represents an outgoing call, e.g. calling a getter from a method or
+/// a method from a constructor etc.
+struct CallHierarchyOutgoingCall {
+  /// The item that is called.
+  CallHierarchyItem To;
+
+  /// The range at which this item is called.
+  /// This is the range relative to the caller, and not `To`.
+  std::vector FromRanges;
+};
+llvm::json::Value toJSON(const CallHierarchyOutgoingCall &);
+
 struct ReferenceParams : public TextDocumentPositionParams {
   // For now, no options like context.includeDeclaration are supported.
 };
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1209,6 +1209,58 @@
   return fromJSON(Params, Base, P);
 }
 
+llvm::json::Value toJSON(SymbolTag Tag) {
+  return llvm::json::Value{static_cast(Tag)};
+}
+
+llvm::json::Value toJSON(const CallHierarchyItem &I) {
+  llvm::json::Object Result{{"name", I.name},
+{"kind", static_cast(I.kind)},
+{"range", I.range},
+{"selectionRange", I.selectionRange},
+{"uri", I.uri}};
+  if (!I.tags.empty())
+Result["tags"] = I.tags;
+  if (!I.detail.empty())
+Result["detail"] = I.detail;
+  if (!I.data.empty())
+Result["data"] = I.data;
+  return std::move(Result);
+}
+
+bool fromJSON(const llvm::json::Value &Params, CallHierarchyItem &I,
+  llvm::json::Path P) {
+  llvm::json::ObjectMapper O(Params, P);
+
+  // Populate the required fields only. We don't care about the
+  // optional fields `Tags` and `Detail` for the purpose of
+  // client --> server communication.
+  return O && O.m

[clang] dc58cd1 - PR48169: Fix crash generating debug info for class non-type template

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-11-15T17:43:26-08:00
New Revision: dc58cd1480374a6d5dbf87e8a2424811c0003ce3

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

LOG: PR48169: Fix crash generating debug info for class non-type template
parameters.

It appears that LLVM isn't able to generate a DW_AT_const_value for a
constant of class type, but if it could, we'd match GCC's debug info in
this case, and in the interim we no longer crash.

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGenCXX/debug-info-template.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 6f77aed526bc..97337e00180e 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1920,6 +1920,12 @@ CGDebugInfo::CollectTemplateParams(const 
TemplateParameterList *TPList,
   V = CGM.getCXXABI().EmitMemberDataPointer(MPT, chars);
 } else if (const auto *GD = dyn_cast(D)) {
   V = CGM.GetAddrOfMSGuidDecl(GD).getPointer();
+} else if (const auto *TPO = dyn_cast(D)) {
+  if (T->isRecordType())
+V = ConstantEmitter(CGM).emitAbstract(
+SourceLocation(), TPO->getValue(), TPO->getType());
+  else
+V = CGM.GetAddrOfTemplateParamObject(TPO).getPointer();
 }
 assert(V && "Failed to find template parameter pointer");
 V = V->stripPointerCasts();

diff  --git a/clang/test/CodeGenCXX/debug-info-template.cpp 
b/clang/test/CodeGenCXX/debug-info-template.cpp
index f52380a62ca6..a6edd59171b2 100644
--- a/clang/test/CodeGenCXX/debug-info-template.cpp
+++ b/clang/test/CodeGenCXX/debug-info-template.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -S -emit-llvm -target x86_64-unknown_unknown -g %s -o - 
-std=c++11 | FileCheck %s
+// RUN: %clang -S -emit-llvm -target x86_64-unknown_unknown -g %s -o - 
-std=c++20 | FileCheck %s
 
 // CHECK: @tci = dso_local global %"struct.TC::nested" zeroinitializer, align 1, !dbg 
[[TCI:![0-9]+]]
 // CHECK: @tcn = dso_local global %struct.TC zeroinitializer, align 1, !dbg 
[[TCN:![0-9]+]]
@@ -156,3 +156,26 @@ struct PaddingAtEndTemplate {
 };
 
 PaddingAtEndTemplate<&PaddedObj> PaddedTemplateObj;
+
+struct ClassTemplateArg {
+  int a;
+  float f;
+};
+template struct ClassTemplateArgTemplate {
+  static constexpr const ClassTemplateArg &Arg = A;
+};
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: 
"ClassTemplateArgTemplate<{1, 2.00e+00}>", {{.*}}, templateParams: 
![[CLASS_TEMP_ARGS:[0-9]*]],
+// CHECK: ![[CLASS_TEMP_ARG_CONST_REF_TYPE:[0-9]*]] = !DIDerivedType(tag: 
DW_TAG_reference_type, baseType: ![[CLASS_TEMP_ARG_CONST_TYPE:[0-9]*]],
+// CHECK: ![[CLASS_TEMP_ARG_CONST_TYPE]] = !DIDerivedType(tag: 
DW_TAG_const_type, baseType: ![[CLASS_TEMP_ARG_TYPE:[0-9]*]])
+// CHECK: ![[CLASS_TEMP_ARG_TYPE]] = distinct !DICompositeType(tag: 
DW_TAG_structure_type, name: "ClassTemplateArg",
+// CHECK: ![[CLASS_TEMP_ARGS]] = !{![[CLASS_TEMP_ARG:[0-9]*]]}
+// CHECK: ![[CLASS_TEMP_ARG]] = !DITemplateValueParameter(name: "A", type: 
![[CLASS_TEMP_ARG_TYPE]], value: %{{[^ *]+}} { i32 1, float 2.00e+00 })
+ClassTemplateArgTemplate ClassTemplateArgObj;
+
+template struct ClassTemplateArgRefTemplate {};
+ClassTemplateArgRefTemplate ClassTemplateArgRefObj;
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: 
"ClassTemplateArgRefTemplate< 
>", {{.*}}, templateParams: ![[CLASS_TEMP_REF_ARGS:[0-9]*]],
+// CHECK: ![[CLASS_TEMP_REF_ARGS]] = !{![[CLASS_TEMP_REF_ARG:[0-9]*]]}
+// CHECK: ![[CLASS_TEMP_REF_ARG]] = !DITemplateValueParameter(type: 
![[CLASS_TEMP_ARG_CONST_REF_TYPE]], value: %{{.*}}* 
@_ZTAXtl16ClassTemplateArgLi1ELf4000EEE)



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

[PATCH] D91122: [clangd] Call hierarchy (XRefs layer, incoming calls)

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 305400.
nridge marked 12 inline comments as done.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91122

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp

Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -156,7 +156,8 @@
 
 std::unique_ptr TestTU::index() const {
   auto AST = build();
-  auto Idx = std::make_unique(/*UseDex=*/true);
+  auto Idx = std::make_unique(/*UseDex=*/true,
+ /*CollectMainFileRefs=*/true);
   Idx->updatePreamble(testPath(Filename), /*Version=*/"null",
   AST.getASTContext(), AST.getPreprocessorPtr(),
   AST.getCanonicalIncludes());
Index: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -0,0 +1,272 @@
+//===-- CallHierarchyTests.cpp  ---*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "Annotations.h"
+#include "Compiler.h"
+#include "Matchers.h"
+#include "ParsedAST.h"
+#include "SyncAPI.h"
+#include "TestFS.h"
+#include "TestTU.h"
+#include "TestWorkspace.h"
+#include "XRefs.h"
+#include "index/FileIndex.h"
+#include "index/SymbolCollector.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/Index/IndexingAction.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using ::testing::AllOf;
+using ::testing::ElementsAre;
+using ::testing::Field;
+using ::testing::Matcher;
+using ::testing::UnorderedElementsAre;
+
+// Helpers for matching call hierarchy data structures.
+MATCHER_P(WithName, N, "") { return arg.name == N; }
+MATCHER_P(WithSelectionRange, R, "") { return arg.selectionRange == R; }
+
+template 
+::testing::Matcher From(ItemMatcher M) {
+  return Field(&CallHierarchyIncomingCall::From, M);
+}
+template 
+::testing::Matcher FromRanges(RangeMatchers... M) {
+  return Field(&CallHierarchyIncomingCall::FromRanges,
+   UnorderedElementsAre(M...));
+}
+
+TEST(CallHierarchy, IncomingOneFile) {
+  Annotations Source(R"cpp(
+void call^ee(int);
+void caller1() {
+  $Callee[[callee]](42);
+}
+void caller2() {
+  $Caller1A[[caller1]]();
+  $Caller1B[[caller1]]();
+}
+void caller3() {
+  $Caller1C[[caller1]]();
+  $Caller2[[caller2]]();
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+
+  llvm::Optional> Items =
+  prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
+  ASSERT_TRUE(bool(Items));
+  EXPECT_THAT(*Items, ElementsAre(WithName("callee")));
+  auto IncomingLevel1 = incomingCalls((*Items)[0], Index.get());
+  ASSERT_TRUE(bool(IncomingLevel1));
+  EXPECT_THAT(*IncomingLevel1,
+  ElementsAre(AllOf(From(WithName("caller1")),
+FromRanges(Source.range("Callee");
+
+  auto IncomingLevel2 = incomingCalls((*IncomingLevel1)[0].From, Index.get());
+  ASSERT_TRUE(bool(IncomingLevel2));
+  EXPECT_THAT(
+  *IncomingLevel2,
+  UnorderedElementsAre(
+  AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller1A"), Source.range("Caller1B"))),
+  AllOf(From(WithName("caller3")),
+FromRanges(Source.range("Caller1C");
+
+  auto IncomingLevel3 = incomingCalls((*IncomingLevel2)[0].From, Index.get());
+  ASSERT_TRUE(bool(IncomingLevel3));
+  EXPECT_THAT(*IncomingLevel3,
+  ElementsAre(AllOf(From(WithName("caller3")),
+FromRanges(Source.range("Caller2");
+
+  auto IncomingLevel4 = incomingCalls((*IncomingLevel3)[0].From, Index.get());
+  ASSERT_TRUE(bool(IncomingLevel4));
+  EXPECT_THAT(*IncomingLevel4, ElementsAre());
+}
+
+TEST(CallHierarchy, MainFileOnlyRef) {
+  // In addition to testing that we store refs to main-file only symbols,
+  // this tests that anonymous namespaces do not interfere with the
+  // symbol re-identification process in callHierarchyItemToSymbo().
+  An

[PATCH] D91122: [clangd] Call hierarchy (XRefs layer, incoming calls)

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1317
-static Optional
-symbolToTypeHierarchyItem(const Symbol &S, const SymbolIndex *Index,
-  PathRef TUPath) {

kadircet wrote:
> that's a great change to have, but maybe leave it out of this patch?
Since we are unifying the function with `symbolToCallHierarchyItem`, and that 
didn't have an `Index` parameter, it made more sense to keep the removal.



Comment at: clang-tools-extra/clangd/XRefs.h:109
+/// Get call hierarchy information at \p Pos.
+llvm::Optional>
+prepareCallHierarchy(ParsedAST &AST, Position Pos, const SymbolIndex *Index,

kadircet wrote:
> what's the distinction between empty and none return values ? (same for 
> others)
Generally speaking, a `None` result means "the input was not recognized in some 
way", while an empty vector result means "there are no results for this input".

For `prepareCallHierarchy`, the inputs encode a source location, and the 
operation asks "give me `CallHierarchyItem`s for suitable declarations (i.e. 
functions) at this location. So, `None` means "the source location could not be 
recognized" (`sourceLocationInMainFile` failed), while an empty result means 
"there are no declarations of functions at this location".

For `incomingCalls` and `outgoingCalls`, the inputs encode a declaration of a 
function, and the operation asks "give me its callers / callees". So, a `None` 
means "could not locate the function (i.e. symbol)", while an empty result 
means "this function has no callers / callees".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91122

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

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 updated this revision to Diff 305405.
psionic12 added a comment.

Fix the grammar
Simplify the code logic
Simplify the test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91047

Files:
  clang/docs/ClangPlugins.rst
  clang/examples/CMakeLists.txt
  clang/examples/CallSuperAttribute/CMakeLists.txt
  clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
  clang/test/CMakeLists.txt
  clang/test/Frontend/plugin-call-super.cpp

Index: clang/test/Frontend/plugin-call-super.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-call-super.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -fsyntax-only -Xclang -verify %s
+// RUN: not %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -emit-llvm -DBAD_CALLSUPER -S %s -o - 2>&1 | FileCheck %s --check-prefix=BADCALLSUPER
+// REQUIRES: plugins, examples
+
+// expected-no-diagnostics
+struct Base1 { [[clang::call_super]] virtual void Test() {} };
+struct Base2 { [[clang::call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { void Test() override; };
+void Derive::Test() { Base1::Test(); Base2::Test(); }
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+#ifdef BAD_CALLSUPER
+struct Base1 { [[clang::call_super]] virtual void Test() {} };
+struct Base2 { [[clang::call_super]] virtual void Test() {} };
+struct Derive : public Base1, public Base2 { [[clang::call_super]] virtual void Test() override final; };
+// CALLSUPER: warning: 'call_super' attribute marked on a final method
+void Derive::Test() { Base1::Test(); /*Base2::Test();*/ }
+struct Derive2 : public Base1, public Base2 { void Test() override {  Base1::Test();  Base2::Test();}};
+// BADCALLSUPER: warning: virtual function 'Base2::Test' is marked as 'call_super' but this overriding method does not call the base version
+// BADCALLSUPER: note: function marked 'call_super' here
+#endif
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -91,6 +91,7 @@
   list(APPEND CLANG_TEST_DEPS
 Attribute
 AnnotateFunctions
+CallSuperAttr
 clang-interpreter
 PrintFunctionNames
 )
Index: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
===
--- /dev/null
+++ clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
@@ -0,0 +1,178 @@
+//===- AnnotateFunctions.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This Clang plugin checks that overrides of the marked virtual function
+// directly call the marked function.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/Sema/ParsedAttr.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/SmallPtrSet.h"
+using namespace clang;
+
+namespace {
+// Cached methods which are marked as 'call_super'.
+llvm::SmallPtrSet MarkedMethods;
+bool isMarkedAsCallSuper(const CXXMethodDecl *D) {
+  // Uses this way to avoid add an annotation attr to the AST.
+  return MarkedMethods.contains(D);
+}
+
+class MethodUsageVisitor : public RecursiveASTVisitor {
+public:
+  bool IsOverriddenUsed = false;
+  explicit MethodUsageVisitor(
+  llvm::SmallPtrSet &MustCalledMethods)
+  : MustCalledMethods(MustCalledMethods) {}
+  bool VisitCallExpr(CallExpr *CallExpr) {
+const CXXMethodDecl *Callee = nullptr;
+for (const auto &MustCalled : MustCalledMethods) {
+  if (CallExpr->getCalleeDecl() == MustCalled) {
+// Super is called.
+// Notice that we cannot do delete or insert in the iteration
+// when using SmallPtrSet.
+Callee = MustCalled;
+  }
+}
+if (Callee)
+  MustCalledMethods.erase(Callee);
+
+return true;
+  }
+
+private:
+  llvm::SmallPtrSet &MustCalledMethods;
+};
+
+class CallSuperVisitor : public RecursiveASTVisitor {
+public:
+  CallSuperVisitor(DiagnosticsEngine &Diags) : Diags(Diags) {
+WarningSuperNotCalled = Diags.getCustomDiagID(
+DiagnosticsEngine::Warning,
+"virtual function %q0 is marked as 'call_super' but this overriding "
+"method does not call the base version");
+NotePreviousCallSuperDeclaration = Diags.getCustomDiagID(
+DiagnosticsEngine::N

[PATCH] D89296: [clangd] Call hierarchy (Protocol layer)

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 305406.
nridge added a comment.

Forgot to rename a few fields


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89296

Files:
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h

Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1375,7 +1375,7 @@
   /// descendants. If not defined, the children have not been resolved.
   llvm::Optional> children;
 
-  /// An optional 'data' filed, which can be used to identify a type hierarchy
+  /// An optional 'data' field, which can be used to identify a type hierarchy
   /// item in a resolve request.
   llvm::Optional data;
 };
@@ -1397,6 +1397,83 @@
 bool fromJSON(const llvm::json::Value &, ResolveTypeHierarchyItemParams &,
   llvm::json::Path);
 
+enum class SymbolTag { Deprecated = 1 };
+llvm::json::Value toJSON(SymbolTag);
+
+/// The parameter of a `textDocument/prepareCallHierarchy` request.
+struct CallHierarchyPrepareParams : public TextDocumentPositionParams {};
+
+/// Represents programming constructs like functions or constructors
+/// in the context of call hierarchy.
+struct CallHierarchyItem {
+  /// The name of this item.
+  std::string name;
+
+  /// The kind of this item.
+  SymbolKind kind;
+
+  /// Tags for this item.
+  std::vector tags;
+
+  /// More detaill for this item, e.g. the signature of a function.
+  std::string detail;
+
+  /// The resource identifier of this item.
+  URIForFile uri;
+
+  /// The range enclosing this symbol not including leading / trailing
+  /// whitespace but everything else, e.g. comments and code.
+  Range range;
+
+  /// The range that should be selected and revealed when this symbol
+  /// is being picked, e.g. the name of a function.
+  /// Must be contained by `Rng`.
+  Range selectionRange;
+
+  /// An optional 'data' field, which can be used to identify a call
+  /// hierarchy item in an incomingCalls or outgoingCalls request.
+  std::string data;
+};
+llvm::json::Value toJSON(const CallHierarchyItem &);
+bool fromJSON(const llvm::json::Value &, CallHierarchyItem &, llvm::json::Path);
+
+/// The parameter of a `callHierarchy/incomingCalls` request.
+struct CallHierarchyIncomingCallsParams {
+  CallHierarchyItem item;
+};
+bool fromJSON(const llvm::json::Value &, CallHierarchyIncomingCallsParams &,
+  llvm::json::Path);
+
+/// Represents an incoming call, e.g. a caller of a method or constructor.
+struct CallHierarchyIncomingCall {
+  /// The item that makes the call.
+  CallHierarchyItem from;
+
+  /// The range at which the calls appear.
+  /// This is relative to the caller denoted by `From`.
+  std::vector fromRanges;
+};
+llvm::json::Value toJSON(const CallHierarchyIncomingCall &);
+
+/// The parameter of a `callHierarchy/outgoingCalls` request.
+struct CallHierarchyOutgoingCallsParams {
+  CallHierarchyItem item;
+};
+bool fromJSON(const llvm::json::Value &, CallHierarchyOutgoingCallsParams &,
+  llvm::json::Path);
+
+/// Represents an outgoing call, e.g. calling a getter from a method or
+/// a method from a constructor etc.
+struct CallHierarchyOutgoingCall {
+  /// The item that is called.
+  CallHierarchyItem to;
+
+  /// The range at which this item is called.
+  /// This is the range relative to the caller, and not `To`.
+  std::vector fromRanges;
+};
+llvm::json::Value toJSON(const CallHierarchyOutgoingCall &);
+
 struct ReferenceParams : public TextDocumentPositionParams {
   // For now, no options like context.includeDeclaration are supported.
 };
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1209,6 +1209,58 @@
   return fromJSON(Params, Base, P);
 }
 
+llvm::json::Value toJSON(SymbolTag Tag) {
+  return llvm::json::Value{static_cast(Tag)};
+}
+
+llvm::json::Value toJSON(const CallHierarchyItem &I) {
+  llvm::json::Object Result{{"name", I.name},
+{"kind", static_cast(I.kind)},
+{"range", I.range},
+{"selectionRange", I.selectionRange},
+{"uri", I.uri}};
+  if (!I.tags.empty())
+Result["tags"] = I.tags;
+  if (!I.detail.empty())
+Result["detail"] = I.detail;
+  if (!I.data.empty())
+Result["data"] = I.data;
+  return std::move(Result);
+}
+
+bool fromJSON(const llvm::json::Value &Params, CallHierarchyItem &I,
+  llvm::json::Path P) {
+  llvm::json::ObjectMapper O(Params, P);
+
+  // Populate the required fields only. We don't care about the
+  // optional fields `Tags` and `Detail` for the purpose of
+  // client --> server communication.
+  return O && O.map("name", I.name) && O.map("kind",

[PATCH] D91047: Add a call super attribute plugin example

[Yafei Liu via Phabricator via cfe-commits]

psionic12 marked 6 inline comments as done.
psionic12 added inline comments.



Comment at: clang/docs/ClangPlugins.rst:117
+Defining CallSuperAttr
+===
+

aaron.ballman wrote:
> psionic12 wrote:
> > aaron.ballman wrote:
> > > aaron.ballman wrote:
> > > > The number of underlines here looks off -- can you verify it's correct?
> > > This still appears to be incorrect and will cause build errors for the 
> > > documentation.
> > Do you mean that there's a command to build the documentation and currently 
> > my patch will cause a failure on it?
> > 
> > I thought this ClangPlugins.rst is only an documentation with markdown, but 
> > seems that it's not what I thought?
> > 
> > Currently I will make sure there's no build error on the plugin itself and 
> > the regression test case, and make sure the
> > regression test will pass. Seems that's not enough, right?
> > Do you mean that there's a command to build the documentation and currently 
> > my patch will cause a failure on it?
> 
> Yes, we have a bot that builds docs: http://lab.llvm.org:8011/#/builders/92
> 
> > I thought this ClangPlugins.rst is only an documentation with markdown, but 
> > seems that it's not what I thought?
> 
> It is a documentation file with markdown, but the markdown bots will complain 
> if a markdown file cannot be built (they treat markdown warnings as errors).
> 
> > Currently I will make sure there's no build error on the plugin itself and 
> > the regression test case, and make sure the regression test will pass. 
> > Seems that's not enough, right?
> 
> Most of the time that's enough because the markdown usage is pretty trivial 
> and can be inspected by sight for obvious issues (so people don't typically 
> have to set up their build environments to generate documentation and test it 
> with every patch).
> 
> In this case, the issue is with the underlines under the title. The number of 
> underlines needs to match the number of characters in the title, but in this 
> case there are 20 `=` characters but 23 title characters.
It seems that only a committee member can trigger this bot, any way I can test 
on my own environment? So that I can make sure the doc will compile 
successfully before uploading patches?

As I mentioned before, using `make doxygen-clang` will succeed even the `=` 
characters are not match with title characters, so it seems that the bot 
doesn't use this way.



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:104
+for (const auto *Method : MarkedMethods) {
+  lateDiagAppertainsToDecl(Diags, Method);
+}

Move the `lateDiagAppertainsToDecl()` here and only check marked functions.



Comment at: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp:69
+  bool VisitCXXMethodDecl(CXXMethodDecl *MethodDecl) {
+lateDiagAppertainsToDecl(MethodDecl);
+

Call `lateDiagAppertainsToDecl` in every `VisitCXXMethodDecl` functions is not 
very efficiency, since marked methods are already saved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91047

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

[PATCH] D91122: [clangd] Call hierarchy (XRefs layer, incoming calls)

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 305407.
nridge added a comment.

Update variable names and remove a commented declaration


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91122

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp

Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -156,7 +156,8 @@
 
 std::unique_ptr TestTU::index() const {
   auto AST = build();
-  auto Idx = std::make_unique(/*UseDex=*/true);
+  auto Idx = std::make_unique(/*UseDex=*/true,
+ /*CollectMainFileRefs=*/true);
   Idx->updatePreamble(testPath(Filename), /*Version=*/"null",
   AST.getASTContext(), AST.getPreprocessorPtr(),
   AST.getCanonicalIncludes());
Index: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -0,0 +1,272 @@
+//===-- CallHierarchyTests.cpp  ---*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "Annotations.h"
+#include "Compiler.h"
+#include "Matchers.h"
+#include "ParsedAST.h"
+#include "SyncAPI.h"
+#include "TestFS.h"
+#include "TestTU.h"
+#include "TestWorkspace.h"
+#include "XRefs.h"
+#include "index/FileIndex.h"
+#include "index/SymbolCollector.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/Index/IndexingAction.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using ::testing::AllOf;
+using ::testing::ElementsAre;
+using ::testing::Field;
+using ::testing::Matcher;
+using ::testing::UnorderedElementsAre;
+
+// Helpers for matching call hierarchy data structures.
+MATCHER_P(WithName, N, "") { return arg.name == N; }
+MATCHER_P(WithSelectionRange, R, "") { return arg.selectionRange == R; }
+
+template 
+::testing::Matcher From(ItemMatcher M) {
+  return Field(&CallHierarchyIncomingCall::from, M);
+}
+template 
+::testing::Matcher FromRanges(RangeMatchers... M) {
+  return Field(&CallHierarchyIncomingCall::fromRanges,
+   UnorderedElementsAre(M...));
+}
+
+TEST(CallHierarchy, IncomingOneFile) {
+  Annotations Source(R"cpp(
+void call^ee(int);
+void caller1() {
+  $Callee[[callee]](42);
+}
+void caller2() {
+  $Caller1A[[caller1]]();
+  $Caller1B[[caller1]]();
+}
+void caller3() {
+  $Caller1C[[caller1]]();
+  $Caller2[[caller2]]();
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+
+  llvm::Optional> Items =
+  prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
+  ASSERT_TRUE(bool(Items));
+  EXPECT_THAT(*Items, ElementsAre(WithName("callee")));
+  auto IncomingLevel1 = incomingCalls((*Items)[0], Index.get());
+  ASSERT_TRUE(bool(IncomingLevel1));
+  EXPECT_THAT(*IncomingLevel1,
+  ElementsAre(AllOf(From(WithName("caller1")),
+FromRanges(Source.range("Callee");
+
+  auto IncomingLevel2 = incomingCalls((*IncomingLevel1)[0].from, Index.get());
+  ASSERT_TRUE(bool(IncomingLevel2));
+  EXPECT_THAT(
+  *IncomingLevel2,
+  UnorderedElementsAre(
+  AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller1A"), Source.range("Caller1B"))),
+  AllOf(From(WithName("caller3")),
+FromRanges(Source.range("Caller1C");
+
+  auto IncomingLevel3 = incomingCalls((*IncomingLevel2)[0].from, Index.get());
+  ASSERT_TRUE(bool(IncomingLevel3));
+  EXPECT_THAT(*IncomingLevel3,
+  ElementsAre(AllOf(From(WithName("caller3")),
+FromRanges(Source.range("Caller2");
+
+  auto IncomingLevel4 = incomingCalls((*IncomingLevel3)[0].from, Index.get());
+  ASSERT_TRUE(bool(IncomingLevel4));
+  EXPECT_THAT(*IncomingLevel4, ElementsAre());
+}
+
+TEST(CallHierarchy, MainFileOnlyRef) {
+  // In addition to testing that we store refs to main-file only symbols,
+  // this tests that anonymous namespaces do not interfere with the
+  // symbol re-identification process in callHierarchyItemToSymbo().
+  Annotations

[PATCH] D91123: [clangd] Call hierarchy (ClangdServer layer)

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 305408.
nridge marked 2 inline comments as done.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91123

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h


Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -242,6 +242,16 @@
 TypeHierarchyDirection Direction,
 Callback> CB);
 
+  /// Get information about call hierarchy for a given position.
+  void prepareCallHierarchy(
+  PathRef File, Position Pos,
+  Callback>> CB);
+
+  /// Resolve incoming calls for a given call hierarchy item.
+  void incomingCalls(
+  const CallHierarchyItem &Item,
+  Callback>>);
+
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,
 Callback> CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -677,6 +677,27 @@
   CB(Item);
 }
 
+void ClangdServer::prepareCallHierarchy(
+PathRef File, Position Pos,
+Callback>> CB) {
+  auto Action = [File = File.str(), Pos,
+ CB = std::move(CB)](Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, File));
+  };
+  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+}
+
+void ClangdServer::incomingCalls(
+const CallHierarchyItem &Item,
+Callback>> CB) {
+  WorkScheduler.run("Incoming Calls", "",
+[CB = std::move(CB), Item, this]() mutable {
+  CB(clangd::incomingCalls(Item, Index));
+});
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.


Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -242,6 +242,16 @@
 TypeHierarchyDirection Direction,
 Callback> CB);
 
+  /// Get information about call hierarchy for a given position.
+  void prepareCallHierarchy(
+  PathRef File, Position Pos,
+  Callback>> CB);
+
+  /// Resolve incoming calls for a given call hierarchy item.
+  void incomingCalls(
+  const CallHierarchyItem &Item,
+  Callback>>);
+
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,
 Callback> CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -677,6 +677,27 @@
   CB(Item);
 }
 
+void ClangdServer::prepareCallHierarchy(
+PathRef File, Position Pos,
+Callback>> CB) {
+  auto Action = [File = File.str(), Pos,
+ CB = std::move(CB)](Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, File));
+  };
+  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+}
+
+void ClangdServer::incomingCalls(
+const CallHierarchyItem &Item,
+Callback>> CB) {
+  WorkScheduler.run("Incoming Calls", "",
+[CB = std::move(CB), Item, this]() mutable {
+  CB(clangd::incomingCalls(Item, Index));
+});
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91123: [clangd] Call hierarchy (ClangdServer layer)

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D91123#2387032 , @kadircet wrote:

> can you also add some tests to ClangdTests.cpp ?

It seems like they would be highly duplicative of the tests in 
CallHierarchyTests.cpp.




Comment at: clang-tools-extra/clangd/ClangdServer.cpp:695
+Callback>> CB) {
+  CB(clangd::incomingCalls(Item, Index));
+}

kadircet wrote:
> why do we run this on the mainthread ? I suppose we should just do 
> `WorkScheduler.run`
I was following what `resolveTypeHierarchy` did. Should I change that too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91123

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

[PATCH] D91124: [clangd] Call hierarchy (ClangdLSPServer layer)

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 305409.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91124

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/test/call-hierarchy.test
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -37,6 +37,8 @@
   LSPTest() : LogSession(*this) {
 ClangdServer::Options &Base = Opts;
 Base = ClangdServer::optsForTest();
+// This is needed to we can test index-based operations like call hierarchy.
+Base.BuildDynamicSymbolIndex = true;
   }
 
   LSPClient &start() {
@@ -165,6 +167,33 @@
   stop();
   EXPECT_THAT(Tracer.takeMetric("lsp_latency", MethodName), testing::SizeIs(1));
 }
+
+TEST_F(LSPTest, IncomingCalls) {
+  Annotations Code(R"cpp(
+void calle^e(int);
+void caller1() {
+  [[callee]](42);
+}
+  )cpp");
+  auto &Client = start();
+  Client.didOpen("foo.cpp", Code.code());
+  auto Items = Client
+   .call("textDocument/prepareCallHierarchy",
+ llvm::json::Object{
+ {"textDocument", Client.documentID("foo.cpp")},
+ {"position", Code.point()}})
+   .takeValue();
+  auto FirstItem = (*Items.getAsArray())[0];
+  auto Calls = Client
+   .call("callHierarchy/incomingCalls",
+ llvm::json::Object{{"item", FirstItem}})
+   .takeValue();
+  auto FirstCall = *(*Calls.getAsArray())[0].getAsObject();
+  EXPECT_EQ(FirstCall["fromRanges"], llvm::json::Value{Code.range()});
+  auto From = *FirstCall["from"].getAsObject();
+  EXPECT_EQ(From["name"], "caller1");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -5,6 +5,7 @@
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": {
 # CHECK-NEXT:"capabilities": {
+# CHECK-NEXT:  "callHierarchyProvider": true,
 # CHECK-NEXT:  "codeActionProvider": true,
 # CHECK-NEXT:  "completionProvider": {
 # CHECK-NEXT:"allCommitCharacters": [
Index: clang-tools-extra/clangd/test/call-hierarchy.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/call-hierarchy.test
@@ -0,0 +1,39 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"languageId":"cpp","text":"void callee(int);\nvoid caller1() {\n  callee(42);\n}\nvoid caller2() {\n  caller1();\n  caller1();\n}\nvoid caller3() {\n  caller1();\n  caller2();\n}\n","uri":"test:///main.cpp","version":1}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/prepareCallHierarchy","params":{"position":{"character":8,"line":0},"textDocument":{"uri":"test:///main.cpp"}}}
+#  CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "data": "{{.*}}",
+# CHECK-NEXT:  "kind": 12,
+# CHECK-NEXT:  "name": "callee",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 16,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 0,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "selectionRange": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 11,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 5,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "uri": "file://{{.*}}/clangd-test/main.cpp"
+# CHECK-NEXT:}
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -132,6 +132,15 @@
Callback>);
   void onResolveTypeHierarchy(const ResolveTypeHierarchyItemParams &,
   

[PATCH] D90448: [clang] Add type check for explicit instantiation of static data members

[Chuyang Chen via Phabricator via cfe-commits]

nomanous added a comment.

Ping @rsmith @chandlerc @majnemer @aaron.ballman @dblaikie


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90448

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

[PATCH] D69272: Enable '#pragma STDC FENV_ACCESS' in frontend

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff abandoned this revision.
sepavloff added a comment.

Implemented in D87528 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69272

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

[PATCH] D91489: BPF: make __builtin_btf_type_id() return 64bit int

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added inline comments.



Comment at: llvm/lib/Target/BPF/BTFDebug.cpp:1230
+Reloc == BPFCoreSharedInfo::BTF_TYPE_ID_REMOTE)
   OutMI.setOpcode(BPF::LD_imm64);
 else

anakryiko wrote:
> yonghong-song wrote:
> > ast wrote:
> > > libbpf would need to support both (ld_imm64 and mov32), right?
> > all selftests passed. I suspect libbpf already supports both ld_imm64 and 
> > mov32...
> there is no need to make BTF_TYPE_ID_LOCAL to return u64, but if it helps 
> uniformity, then why not.
> 
> libbpf does support ldimm64 in general, but I'll need to extend this 
> specifically for BTF_TYPE_ID_REMOTE to record module BTF ID in upper 32 bits. 
> For now it will be just zeroes, which works fine.
yes, uniformity is why I also return u64 for BTF_TYPE_ID_LOCAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91489

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

[PATCH] D91489: BPF: make __builtin_btf_type_id() return 64bit int

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song updated this revision to Diff 305413.
yonghong-song added a comment.

fix clang-format warning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91489

Files:
  clang/include/clang/Basic/BuiltinsBPF.def
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-bpf-btf-type-id.c
  llvm/include/llvm/IR/IntrinsicsBPF.td
  llvm/lib/Target/BPF/BPFPreserveDIType.cpp
  llvm/lib/Target/BPF/BTFDebug.cpp
  llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll
  llvm/test/CodeGen/BPF/CORE/btf-id-duplicate.ll
  llvm/test/CodeGen/BPF/optnone-2.ll

Index: llvm/test/CodeGen/BPF/optnone-2.ll
===
--- llvm/test/CodeGen/BPF/optnone-2.ll
+++ llvm/test/CodeGen/BPF/optnone-2.ll
@@ -12,14 +12,16 @@
 ; Function Attrs: noinline nounwind optnone
 define dso_local i32 @foo() #0 !dbg !9 {
 entry:
-  %0 = call i32 @llvm.bpf.btf.type.id(i32 0, i64 0), !dbg !12, !llvm.preserve.access.index !4
-  %1 = call i32 @llvm.bpf.preserve.type.info(i32 1, i64 0), !dbg !13, !llvm.preserve.access.index !14
-  %add = add i32 %0, %1, !dbg !17
-  ret i32 %add, !dbg !18
+  %0 = call i64 @llvm.bpf.btf.type.id(i32 0, i64 0), !dbg !11, !llvm.preserve.access.index !4
+  %1 = call i32 @llvm.bpf.preserve.type.info(i32 1, i64 0), !dbg !12, !llvm.preserve.access.index !13
+  %conv = zext i32 %1 to i64, !dbg !12
+  %add = add i64 %0, %conv, !dbg !16
+  %conv1 = trunc i64 %add to i32, !dbg !11
+  ret i32 %conv1, !dbg !17
 }
 
 ; Function Attrs: nounwind readnone
-declare i32 @llvm.bpf.btf.type.id(i32, i64) #1
+declare i64 @llvm.bpf.btf.type.id(i32, i64) #1
 
 ; Function Attrs: nounwind readnone
 declare i32 @llvm.bpf.preserve.type.info(i32, i64) #1
@@ -40,13 +42,12 @@
 !6 = !{i32 2, !"Debug Info Version", i32 3}
 !7 = !{i32 1, !"wchar_size", i32 4}
 !8 = !{!"clang version 12.0.0"}
-!9 = distinct !DISubprogram(name: "foo", scope: !10, file: !10, line: 2, type: !11, scopeLine: 2, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
-!10 = !DIFile(filename: "C:/src/tmp/a.c", directory: "")
-!11 = !DISubroutineType(types: !3)
-!12 = !DILocation(line: 2, column: 21, scope: !9)
-!13 = !DILocation(line: 2, column: 51, scope: !9)
-!14 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "ss", file: !10, line: 1, size: 32, elements: !15)
-!15 = !{!16}
-!16 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !14, file: !10, line: 1, baseType: !4, size: 32)
-!17 = !DILocation(line: 2, column: 49, scope: !9)
-!18 = !DILocation(line: 2, column: 14, scope: !9)
+!9 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 2, type: !10, scopeLine: 2, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!10 = !DISubroutineType(types: !3)
+!11 = !DILocation(line: 2, column: 20, scope: !9)
+!12 = !DILocation(line: 2, column: 50, scope: !9)
+!13 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "ss", file: !1, line: 1, size: 32, elements: !14)
+!14 = !{!15}
+!15 = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: !13, file: !1, line: 1, baseType: !4, size: 32)
+!16 = !DILocation(line: 2, column: 48, scope: !9)
+!17 = !DILocation(line: 2, column: 13, scope: !9)
Index: llvm/test/CodeGen/BPF/CORE/btf-id-duplicate.ll
===
--- llvm/test/CodeGen/BPF/CORE/btf-id-duplicate.ll
+++ llvm/test/CodeGen/BPF/CORE/btf-id-duplicate.ll
@@ -18,15 +18,16 @@
   %arg.addr = alloca %struct.s1*, align 8
   store %struct.s1* %arg, %struct.s1** %arg.addr, align 8, !tbaa !18
   call void @llvm.dbg.declare(metadata %struct.s1** %arg.addr, metadata !17, metadata !DIExpression()), !dbg !22
-  %0 = call i32 @llvm.bpf.btf.type.id(i32 0, i64 0), !dbg !23, !llvm.preserve.access.index !12
-  ret i32 %0, !dbg !24
+  %0 = call i64 @llvm.bpf.btf.type.id(i32 0, i64 0), !dbg !23, !llvm.preserve.access.index !12
+  %conv = trunc i64 %0 to i32, !dbg !23
+  ret i32 %conv, !dbg !24
 }
 
-; Function Attrs: nounwind readnone speculatable willreturn
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
 declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
 
 ; Function Attrs: nounwind readnone
-declare i32 @llvm.bpf.btf.type.id(i32, i64) #2
+declare i64 @llvm.bpf.btf.type.id(i32, i64) #2
 
 ; Function Attrs: nounwind
 define dso_local i32 @bar(%struct.s1* %arg) #0 !dbg !25 {
@@ -34,8 +35,9 @@
   %arg.addr = alloca %struct.s1*, align 8
   store %struct.s1* %arg, %struct.s1** %arg.addr, align 8, !tbaa !18
   call void @llvm.dbg.declare(metadata %struct.s1** %arg.addr, metadata !27, metadata !DIExpression()), !dbg !28
-  %0 = call i32 @llvm.bpf.btf.type.id(i32 1, i64 0), !dbg !29, !llvm.preserve.access.index !12
-  ret i32 %0, !dbg !30
+  %0 = call i64 @llvm.bpf.btf.type.id(i32 1, i64 0), !dbg !29, !llvm.preserve.access.index !12
+  %conv = trunc i64 %0 to i32, !dbg !29
+  ret i32 %conv, !dbg !30
 }
 
 ; CHECK: .long   1