[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:126
+
+  // If the casts have a common anchestor it could not be a succeeded downcast.
+  for (const auto  : PreviousRD->bases())

Charusso wrote:
> NoQ wrote:
> > Counterexample:
> > 
> > ```
> > struct A {};
> > struct B : A {};
> > struct C : A, B {};
> > ```
> > 
> > Downcast from `C` to `B` should succeed, even though they have a common 
> > ancestor `A` (which has the same `CXXRecordDecl` but currently isn't the 
> > same object within `C`, but can be, if `B` declares `A` as a virtual base).
> So, even it is some kind of anti-pattern as a warning arrive immediately, now 
> I allow `B` to `C` downcasts. Could you explain me more about that virtual 
> idea, please? Based on this possible use-case in my mind two classes are on 
> the same level as every of their bases/vbases are equals.
> Could you explain me more about that virtual idea, please?

In the following variation:
```
struct A {};
struct B : virtual A {};
struct C : A, B {};
```
we have `C` contain only one instance of `A`: the layout of `B` within `C` 
would merely refer to the instance of `A` within `C` instead of making its own 
copy. In particular, the constructor of `B` within the constructor of `C` would 
not initialize `C`'s instance of `A` because the constructor of `C` will invoke 
the constructor of `A` directly (cf. D61816).


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

https://reviews.llvm.org/D67079



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371484: clang-misexpect: Profile Guided Validation of 
Performance Annotations in LLVM (authored by phosek, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D66324?vs=219219=219472#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66324

Files:
  cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
  cfe/trunk/include/clang/Basic/DiagnosticGroups.td
  cfe/trunk/lib/CodeGen/CodeGenAction.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  cfe/trunk/test/Profile/Inputs/misexpect-branch.proftext
  cfe/trunk/test/Profile/Inputs/misexpect-switch-default-only.proftext
  cfe/trunk/test/Profile/Inputs/misexpect-switch-default.proftext
  cfe/trunk/test/Profile/Inputs/misexpect-switch.proftext
  cfe/trunk/test/Profile/misexpect-branch-cold.c
  cfe/trunk/test/Profile/misexpect-branch-nonconst-expected-val.c
  cfe/trunk/test/Profile/misexpect-branch-unpredictable.c
  cfe/trunk/test/Profile/misexpect-branch.c
  cfe/trunk/test/Profile/misexpect-switch-default.c
  cfe/trunk/test/Profile/misexpect-switch-nonconst.c
  cfe/trunk/test/Profile/misexpect-switch-only-default-case.c
  cfe/trunk/test/Profile/misexpect-switch.c
  llvm/trunk/include/llvm/IR/DiagnosticInfo.h
  llvm/trunk/include/llvm/IR/FixedMetadataKinds.def
  llvm/trunk/include/llvm/IR/MDBuilder.h
  llvm/trunk/include/llvm/Transforms/Utils/MisExpect.h
  llvm/trunk/lib/IR/DiagnosticInfo.cpp
  llvm/trunk/lib/IR/MDBuilder.cpp
  llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp
  llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/trunk/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/trunk/lib/Transforms/Utils/CMakeLists.txt
  llvm/trunk/lib/Transforms/Utils/MisExpect.cpp
  llvm/trunk/test/ThinLTO/X86/lazyload_metadata.ll
  llvm/trunk/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/trunk/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/trunk/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/trunk/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/trunk/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/trunk/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/trunk/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/trunk/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/trunk/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/trunk/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/trunk/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/trunk/lib/IR/MDBuilder.cpp
===
--- llvm/trunk/lib/IR/MDBuilder.cpp
+++ llvm/trunk/lib/IR/MDBuilder.cpp
@@ -309,3 +309,15 @@
   };
   return MDNode::get(Context, Vals);
 }
+
+MDNode *MDBuilder::createMisExpect(uint64_t Index, uint64_t LikleyWeight,
+   uint64_t UnlikleyWeight) {
+  auto *IntType = Type::getInt64Ty(Context);
+  Metadata *Vals[] = {
+  createString("misexpect"),
+  createConstant(ConstantInt::get(IntType, Index)),
+  createConstant(ConstantInt::get(IntType, LikleyWeight)),
+  createConstant(ConstantInt::get(IntType, UnlikleyWeight)),
+  };
+  return MDNode::get(Context, Vals);
+}
Index: llvm/trunk/lib/IR/DiagnosticInfo.cpp
===
--- llvm/trunk/lib/IR/DiagnosticInfo.cpp
+++ llvm/trunk/lib/IR/DiagnosticInfo.cpp
@@ -370,5 +370,16 @@
   return OS.str();
 }
 
+DiagnosticInfoMisExpect::DiagnosticInfoMisExpect(const Instruction *Inst,
+ Twine )
+: DiagnosticInfoWithLocationBase(DK_MisExpect, DS_Warning,
+ *Inst->getParent()->getParent(),
+ Inst->getDebugLoc()),
+  Msg(Msg) {}
+
+void DiagnosticInfoMisExpect::print(DiagnosticPrinter ) const {
+  DP << getLocationStr() << ": " << getMsg();
+}
+
 void OptimizationRemarkAnalysisFPCommute::anchor() {}
 void OptimizationRemarkAnalysisAliasing::anchor() {}
Index: llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp
===
--- llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp
+++ llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp
@@ -72,6 +72,7 @@
 #include "llvm/Transforms/Instrumentation.h"
 #include "llvm/Transforms/Utils/CallPromotionUtils.h"
 #include "llvm/Transforms/Utils/Cloning.h"
+#include "llvm/Transforms/Utils/MisExpect.h"
 #include 
 #include 
 #include 
@@ -1446,6 +1447,8 @@
   }
 }
 
+misexpect::verifyMisExpect(TI, Weights, TI->getContext());
+
 uint64_t TempWeight;
 // Only set weights if there is at least one non-zero weight.
 // In any other case, let the analyzer 

[PATCH] D60076: [Attributor] Deduce memory behavior function attributes

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Obsolete, D67384 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60076



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

r371484 - clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Petr Hosek via cfe-commits]

Author: phosek
Date: Mon Sep  9 20:11:39 2019
New Revision: 371484

URL: http://llvm.org/viewvc/llvm-project?rev=371484=rev
Log:
clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

This patch contains the basic functionality for reporting potentially
incorrect usage of __builtin_expect() by comparing the developer's
annotation against a collected PGO profile. A more detailed proposal and
discussion appears on the CFE-dev mailing list
(http://lists.llvm.org/pipermail/cfe-dev/2019-July/062971.html) and a
prototype of the initial frontend changes appear here in D65300

We revised the work in D65300 by moving the misexpect check into the
LLVM backend, and adding support for IR and sampling based profiles, in
addition to frontend instrumentation.

We add new misexpect metadata tags to those instructions directly
influenced by the llvm.expect intrinsic (branch, switch, and select)
when lowering the intrinsics. The misexpect metadata contains
information about the expected target of the intrinsic so that we can
check against the correct PGO counter when emitting diagnostics, and the
compiler's values for the LikelyBranchWeight and UnlikelyBranchWeight.
We use these branch weight values to determine when to emit the
diagnostic to the user.

A future patch should address the comment at the top of
LowerExpectIntrisic.cpp to hoist the LikelyBranchWeight and
UnlikelyBranchWeight values into a shared space that can be accessed
outside of the LowerExpectIntrinsic pass. Once that is done, the
misexpect metadata can be updated to be smaller.

In the long term, it is possible to reconstruct portions of the
misexpect metadata from the existing profile data. However, we have
avoided this to keep the code simple, and because some kind of metadata
tag will be required to identify which branch/switch/select instructions
are influenced by the use of llvm.expect

Patch By: paulkirth
Differential Revision: https://reviews.llvm.org/D66324

Added:
cfe/trunk/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
cfe/trunk/test/Profile/Inputs/misexpect-branch.proftext
cfe/trunk/test/Profile/Inputs/misexpect-switch-default-only.proftext
cfe/trunk/test/Profile/Inputs/misexpect-switch-default.proftext
cfe/trunk/test/Profile/Inputs/misexpect-switch.proftext
cfe/trunk/test/Profile/misexpect-branch-cold.c
cfe/trunk/test/Profile/misexpect-branch-nonconst-expected-val.c
cfe/trunk/test/Profile/misexpect-branch-unpredictable.c
cfe/trunk/test/Profile/misexpect-branch.c
cfe/trunk/test/Profile/misexpect-switch-default.c
cfe/trunk/test/Profile/misexpect-switch-nonconst.c
cfe/trunk/test/Profile/misexpect-switch-only-default-case.c
cfe/trunk/test/Profile/misexpect-switch.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/lib/CodeGen/CodeGenAction.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=371484=371483=371484=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Mon Sep  9 
20:11:39 2019
@@ -275,7 +275,12 @@ def warn_profile_data_missing : Warning<
 def warn_profile_data_unprofiled : Warning<
   "no profile data available for file \"%0\"">,
   InGroup;
-
+def warn_profile_data_misexpect : Warning<
+  "Potential performance regression from use of __builtin_expect(): "
+  "Annotation was correct on %0 of profiled executions.">,
+  BackendInfo,
+  InGroup,
+  DefaultIgnore;
 } // end of instrumentation issue category
 
 }

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=371484=371483=371484=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Sep  9 20:11:39 2019
@@ -1042,6 +1042,7 @@ def BackendOptimizationFailure : DiagGro
 def ProfileInstrMissing : DiagGroup<"profile-instr-missing">;
 def ProfileInstrOutOfDate : DiagGroup<"profile-instr-out-of-date">;
 def ProfileInstrUnprofiled : DiagGroup<"profile-instr-unprofiled">;
+def MisExpect : DiagGroup<"misexpect">;
 
 // AddressSanitizer frontend instrumentation remarks.
 def SanitizeAddressRemarks : DiagGroup<"sanitize-address">;

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=371484=371483=371484=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ 

[PATCH] D59980: [Attributor] Deduce memory behavior argument attributes

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert abandoned this revision.
jdoerfert added a comment.

Obsolete, D67384 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59980



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

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:174
+
+constexpr llvm::StringLiteral Vowels = "aeiou";
+

Charusso wrote:
> NoQ wrote:
> > Omg lol nice. Did you try to figure out how do other people normally do it?
> There is no function for that in `ADT/StringExtras.h` + `grep` did not help, 
> so I realized it is a common way to match vowels. Do you know a better 
> solution?
I just realized that this is actually incorrect and the correct solution is 
pretty hard to implement because the actual "//a// vs. //an//" rule deals with 
sounds, not letters. Eg.: 

> Clang is an "LLVM native" C/C++/Objective-C compiler

has an "an" because it's read as "an //**e**//l-el-vee-am".



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:136
+  CurrentBase.getType()->getAsCXXRecordDecl())
+CurrentBaseFoundPreviously = true;
+}

Using a flag here is unnecessary because you're returning true without doing 
anything else whenever the flag is set.

So, basically, your code says "if at least one current base is different from 
at least one previous base, the cast succeeds".

In particular, this function would return true whenever `CurrentRD` and 
`PreviousRD` are from //completely unrelated// class hierarchies. It sounds 
like whatever `isSucceededDowncast()` was supposed to do, it's not doing it 
right.


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

https://reviews.llvm.org/D67079



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

Re: r359367 - Reinstate r359059, reverted in r359361, with a fix to properly prevent

[Michael Spencer via cfe-commits]

On Fri, Apr 26, 2019 at 7:56 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Fri Apr 26 19:58:17 2019
> New Revision: 359367
>
> URL: http://llvm.org/viewvc/llvm-project?rev=359367=rev
> Log:
> Reinstate r359059, reverted in r359361, with a fix to properly prevent
> us emitting the operand of __builtin_constant_p if it has side-effects.
>
> Original commit message:
>
> Fix interactions between __builtin_constant_p and constexpr to match
> current trunk GCC.
>
> GCC permits information from outside the operand of
> __builtin_constant_p (but in the same constant evaluation context) to be
> used within that operand; clang now does so too. A few other minor
> deviations from GCC's behavior showed up in my testing and are also
> fixed (matching GCC):
>   * Clang now supports nullptr_t as the argument type for
> __builtin_constant_p
> * Clang now returns true from __builtin_constant_p if called with a
> null pointer
> * Clang now returns true from __builtin_constant_p if called with an
> integer cast to pointer type
>
> Added:
> cfe/trunk/test/SemaCXX/builtin-constant-p.cpp
> Modified:
> cfe/trunk/lib/AST/ExprConstant.cpp
> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/test/CodeGen/builtin-constant-p.c
> cfe/trunk/test/SemaCXX/enable_if.cpp
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=359367=359366=359367=diff
>
> ==
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Apr 26 19:58:17 2019
> @@ -7801,19 +7801,33 @@ EvaluateBuiltinClassifyType(const CallEx
>  }
>
>  /// EvaluateBuiltinConstantPForLValue - Determine the result of
> -/// __builtin_constant_p when applied to the given lvalue.
> +/// __builtin_constant_p when applied to the given pointer.
>  ///
> -/// An lvalue is only "constant" if it is a pointer or reference to the
> first
> -/// character of a string literal.
> -template
> -static bool EvaluateBuiltinConstantPForLValue(const LValue ) {
> -  const Expr *E = LV.getLValueBase().template dyn_cast();
> -  return E && isa(E) && LV.getLValueOffset().isZero();
> +/// A pointer is only "constant" if it is null (or a pointer cast to
> integer)
> +/// or it points to the first character of a string literal.
> +static bool EvaluateBuiltinConstantPForLValue(const APValue ) {
> +  APValue::LValueBase Base = LV.getLValueBase();
> +  if (Base.isNull()) {
> +// A null base is acceptable.
> +return true;
> +  } else if (const Expr *E = Base.dyn_cast()) {
> +if (!isa(E))
> +  return false;
> +return LV.getLValueOffset().isZero();
> +  } else {
> +// Any other base is not constant enough for GCC.
> +return false;
> +  }
>  }
>
>  /// EvaluateBuiltinConstantP - Evaluate __builtin_constant_p as similarly
> to
>  /// GCC as we can manage.
> -static bool EvaluateBuiltinConstantP(ASTContext , const Expr *Arg) {
> +static bool EvaluateBuiltinConstantP(EvalInfo , const Expr *Arg) {
> +  // Constant-folding is always enabled for the operand of
> __builtin_constant_p
> +  // (even when the enclosing evaluation context otherwise requires a
> strict
> +  // language-specific constant expression).
> +  FoldConstant Fold(Info, true);
> +
>QualType ArgType = Arg->getType();
>
>// __builtin_constant_p always has one operand. The rules which gcc
> follows
> @@ -7821,34 +7835,30 @@ static bool EvaluateBuiltinConstantP(AST
>//
>//  - If the operand is of integral, floating, complex or enumeration
> type,
>//and can be folded to a known value of that type, it returns 1.
> -  //  - If the operand and can be folded to a pointer to the first
> character
> -  //of a string literal (or such a pointer cast to an integral type),
> it
> -  //returns 1.
> +  //  - If the operand can be folded to a pointer to the first character
> +  //of a string literal (or such a pointer cast to an integral type)
> +  //or to a null pointer or an integer cast to a pointer, it returns
> 1.
>//
>// Otherwise, it returns 0.
>//
>// FIXME: GCC also intends to return 1 for literals of aggregate types,
> but
> -  // its support for this does not currently work.
> -  if (ArgType->isIntegralOrEnumerationType()) {
> -Expr::EvalResult Result;
> -if (!Arg->EvaluateAsRValue(Result, Ctx) || Result.HasSideEffects)
> +  // its support for this did not work prior to GCC 9 and is not yet well
> +  // understood.
> +  if (ArgType->isIntegralOrEnumerationType() || ArgType->isFloatingType()
> ||
> +  ArgType->isAnyComplexType() || ArgType->isPointerType() ||
> +  ArgType->isNullPtrType()) {
> +APValue V;
> +if (!::EvaluateAsRValue(Info, Arg, V)) {
> +  Fold.keepDiagnostics();
>return false;
> +}
>
> -APValue  = 

[PATCH] D67336: [analyzer][NFC] Introduce SuperChecker<>, a convenient alternative to Checker<> for storing subcheckers

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I have mixed feelings. Removing boilerplate is good, but the very fact that 
we're legalizing this pattern indicates that our checkers will keep bloating 
up, while i always wanted to actually split them instead (like, make 
sub-checkers into their own separate //classes//, possibly spread out into 
different files, kinda micro checkers as opposed to monolithic checkers (?)). 
But i guess it's about whoever gets things done first :)

I'd love to see how this affects our actual checkers, did you already try 
porting them? Do you plan to help with tracking per-sub-checker bug types and 
check names?

> `SuperChecker`

WDYT about `MultiChecker`? ("A checker that implements multiple checks and 
presents them as different checkers.")




Comment at: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp:113
+void registerCXX23IntPointer(CheckerManager ) {
+  Mgr.registerSubChecker();
+}

The `CXX23ModelingDiagKind::` qualifier is unnecessary here, right? Or did you 
mean to make an `enum class`? Does it even work with `enum class`es?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67336



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

[PATCH] D67335: [analyzer][NFC] Refactor the checker registration unit test file

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp:84
 
-}
-}
-}
+} // namespace
+} // namespace ento

Szelethus wrote:
> Well, according to `clangd`, this is how it's supposed to be done in LLVM.
[[ 
https://github.com/llvm-mirror/clang/blob/release_80/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h#L158
 | Can confirm ]].


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67335



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

r371478 - Remove REQUIRES:shell from tests that pass for me on Windows

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Mon Sep  9 17:50:32 2019
New Revision: 371478

URL: http://llvm.org/viewvc/llvm-project?rev=371478=rev
Log:
Remove REQUIRES:shell from tests that pass for me on Windows

I see in the history for some of these tests REQUIRES:shell was used as
a way to disable tests on Windows because they are flaky there. I tried
not to re-enable such tests, but it's possible that I missed some and
this will re-enable flaky tests on Windows. If so, we should disable
them with UNSUPPORTED:system-windows and add a comment that they are
flaky there. So far as I can tell, the lit internal shell is capable of
running all of these tests, and we shouldn't use REQUIRES:shell as a
proxy for Windows.

Modified:
cfe/trunk/test/Analysis/crash-trace.c
cfe/trunk/test/CodeGen/thinlto_backend.ll
cfe/trunk/test/Driver/check-time-trace-sections.cpp
cfe/trunk/test/Driver/check-time-trace.cpp
cfe/trunk/test/Driver/clang-offload-bundler.c
cfe/trunk/test/Driver/crash-report-crashfile.m
cfe/trunk/test/Driver/rewrite-map-in-diagnostics.c
cfe/trunk/test/Format/style-on-command-line.cpp
cfe/trunk/test/Frontend/dependency-gen-has-include.c
cfe/trunk/test/Index/crash-recovery-modules.m
cfe/trunk/test/Modules/at-import-in-framework-header.m
cfe/trunk/test/Modules/builtins.m
cfe/trunk/test/Modules/dependency-dump-dependent-module.m
cfe/trunk/test/Modules/dependency-dump.m
cfe/trunk/test/Modules/implicit-invalidate-common.c
cfe/trunk/test/OpenMP/task_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/task_private_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_lastprivate_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_private_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_simd_lastprivate_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_simd_private_codegen.cpp
cfe/trunk/test/PCH/modified-header-error.c
cfe/trunk/test/Parser/crash-report.c

Modified: cfe/trunk/test/Analysis/crash-trace.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/crash-trace.c?rev=371478=371477=371478=diff
==
--- cfe/trunk/test/Analysis/crash-trace.c (original)
+++ cfe/trunk/test/Analysis/crash-trace.c Mon Sep  9 17:50:32 2019
@@ -1,9 +1,8 @@
 // RUN: not --crash %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection 
%s 2>&1 | FileCheck %s
 // REQUIRES: crash-recovery
 
-// FIXME: CHECKs might be incompatible to win32.
-// Stack traces also require back traces.
-// REQUIRES: shell, backtrace
+// Stack traces require back traces.
+// REQUIRES: backtrace
 
 void clang_analyzer_crash(void);
 
@@ -18,6 +17,6 @@ void test() {
 // CHECK: 0.   Program arguments: {{.*}}clang
 // CHECK-NEXT: 1.   parser at end of file
 // CHECK-NEXT: 2. While analyzing stack: 
-// CHECK-NEXT:  #0 Calling inlined at line 15
+// CHECK-NEXT:  #0 Calling inlined at line 14
 // CHECK-NEXT:  #1 Calling test
 // CHECK-NEXT: 3.  {{.*}}crash-trace.c:{{[0-9]+}}:3: Error evaluating 
statement

Modified: cfe/trunk/test/CodeGen/thinlto_backend.ll
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/thinlto_backend.ll?rev=371478=371477=371478=diff
==
--- cfe/trunk/test/CodeGen/thinlto_backend.ll (original)
+++ cfe/trunk/test/CodeGen/thinlto_backend.ll Mon Sep  9 17:50:32 2019
@@ -1,5 +1,4 @@
-; shell required since the windows bot does not like the "(cd ..."
-; REQUIRES: x86-registered-target, shell
+; REQUIRES: x86-registered-target
 
 ; RUN: opt -module-summary -o %t1.o %s
 ; RUN: opt -module-summary -o %t2.o %S/Inputs/thinlto_backend.ll
@@ -32,10 +31,14 @@
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c 
-fthinlto-index=%t.thinlto.bc -save-temps=obj
 ; RUN: llvm-dis %t1.s.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT 
%s
 ; RUN: mkdir -p %T/dir1
-; RUN: (cd %T/dir1 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x 
ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd)
+; RUN: cd %T/dir1
+; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c 
-fthinlto-index=%t.thinlto.bc -save-temps=cwd
+; RUN: cd ../..
 ; RUN: llvm-dis %T/dir1/*1.s.3.import.bc -o - | FileCheck 
--check-prefix=CHECK-IMPORT %s
 ; RUN: mkdir -p %T/dir2
-; RUN: (cd %T/dir2 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x 
ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps)
+; RUN: cd %T/dir2
+; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c 
-fthinlto-index=%t.thinlto.bc -save-temps
+; RUN: cd ../..
 ; RUN: llvm-dis %T/dir2/*1.s.3.import.bc -o - | FileCheck 
--check-prefix=CHECK-IMPORT %s
 ; CHECK-IMPORT: define available_externally void @f2()
 ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s

Modified: 

[PATCH] D67321: Respect CLANG_LINK_CLANG_DYLIB=ON in libclang and c-index-test

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

@aaronpuchert sounds like a reasonable approach


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67321



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

fixing 2 typos

[Rayson Ho via cfe-commits]

Found 2 typos when I was trying to use the context sensitive profiling
feature earlier today:

  


$ git diff docs/UsersManual.rst
diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index f45d2d5ac0e..0f5f315cfcb 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -1836,7 +1836,7 @@ programs using the same instrumentation method as
``-fprofile-generate``.
   .. code-block:: console

 $ clang++ -O2 -fprofile-use=code.profdata
-fcs-profile-generate=sss/ttt \
-  -o cs_code
+  code.cc -o cs_code
 $ ./cs_code
 $ llvm-profdata merge -output=cs_code.profdata sss/ttt code.profdata

@@ -1846,7 +1846,7 @@ programs using the same instrumentation method as
``-fprofile-generate``.

   .. code-block:: console

-$ clang++ -O2 -fprofile-use=cs_code.profdata
+$ clang++ -O2 -fprofile-use=cs_code.profdata code.cc

   The above command will read both profiles to the compiler at the
identical
   point of instrumenations.




Rayson

==
Open Grid Scheduler - The Official Open Source Grid Engine
http://gridscheduler.sourceforge.net/
http://gridscheduler.sourceforge.net/GridEngine/GridEngineCloud.html
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66796: [clang] Loop pragma vectorize(disable)

[Hideki Saito via Phabricator via cfe-commits]

hsaito added a comment.

In D66796#1663868 , @SjoerdMeijer 
wrote:

> That's exactly the reason why I think `vectorize(disable)` should disable 
> vectorisation for that loop. I just don't see what else a user would expect.


I agree with you.

Now on the practical side. If whatever we decide here changes how the pragma 
behaves from today, we need to have a wider discussion and agreement at 
llvm-dev.


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

https://reviews.llvm.org/D66796



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

r371476 - Fix crash mangling an explicit lambda non-type template parameter pack

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Sep  9 17:39:53 2019
New Revision: 371476

URL: http://llvm.org/viewvc/llvm-project?rev=371476=rev
Log:
Fix crash mangling an explicit lambda non-type template parameter pack
that is not a pack expansion.

Modified:
cfe/trunk/lib/AST/ItaniumMangle.cpp
cfe/trunk/test/CodeGenCXX/mangle-lambda-explicit-template-params.cpp

Modified: cfe/trunk/lib/AST/ItaniumMangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumMangle.cpp?rev=371476=371475=371476=diff
==
--- cfe/trunk/lib/AST/ItaniumMangle.cpp (original)
+++ cfe/trunk/lib/AST/ItaniumMangle.cpp Mon Sep  9 17:39:53 2019
@@ -1704,7 +1704,8 @@ void CXXNameMangler::mangleTemplateParam
   QualType T = Tn->getType();
   if (Tn->isParameterPack()) {
 Out << "Tp";
-T = T->castAs()->getPattern();
+if (auto *PackExpansion = T->getAs())
+  T = PackExpansion->getPattern();
   }
   Out << "Tn";
   mangleType(T);

Modified: cfe/trunk/test/CodeGenCXX/mangle-lambda-explicit-template-params.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/mangle-lambda-explicit-template-params.cpp?rev=371476=371475=371476=diff
==
--- cfe/trunk/test/CodeGenCXX/mangle-lambda-explicit-template-params.cpp 
(original)
+++ cfe/trunk/test/CodeGenCXX/mangle-lambda-explicit-template-params.cpp Mon 
Sep  9 17:39:53 2019
@@ -54,6 +54,12 @@ inline void collision() {
 }
 void use_collision() { collision(); }
 
+namespace pack_not_pack_expansion {
+  template struct X;
+  // CHECK: 
@_ZNK23pack_not_pack_expansion1xMUlTyTtTyTnT_TpTnTL0__ETpTyvE_clIiNS_1XEJfEEEDav
+  inline auto x = [] typename, 
typename ...V>(){}; void f() { x.operator()(); }
+}
+
 template void f() {
   // CHECK: define linkonce_odr {{.*}} @_ZZ1fIiEvvENKUlT_E_clIiEEDaS0_(
   auto x = [](auto){};


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

[PATCH] D67321: Respect CLANG_LINK_CLANG_DYLIB=ON in libclang and c-index-test

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: clang/tools/libclang/CMakeLists.txt:115
+clang_target_link_libraries(libclang
+  PRIVATE
+  ${CLANG_LIB_DEPS}

beanz wrote:
> tstellar wrote:
> > aaronpuchert wrote:
> > > This might not be correct for static builds, I think we need `INTERFACE` 
> > > here.
> > This patch looks OK to me, but you should find someone with more CMake 
> > knowledge to answer this question.
> This part of the patch is a bit tricky.
> 
> As implemented it is fine for the most common build configurations, but will 
> be a problem if `LIBCLANG_BUILD_STATIC=On` or if `LLVM_ENABLE_PIC=Off`.
> 
> The correct solution is probably to wrap this code in `if (ENABLE_SHARED)`, 
> and to have another code block that handles `if (ENABLE_STATIC)`. In that 
> block you need to call this with `INTERFACE` as the linkage type, and you'll 
> need to handle the case where both `ENABLE_SHARED` and `ENABLE_STATIC` is 
> set. In that case the static library target is named `libclang_static`.
I agree with your analysis. What do you think about modifying 
`clang_target_link_libraries` instead? I thought we could make it do what 
`llvm_add_library` does with its `LINK_LIBS` argument (code which we are 
basically replacing here):

```
  if(ARG_STATIC)
set(libtype INTERFACE)
  else()
# We can use PRIVATE since SO knows its dependent libs.
set(libtype PRIVATE)
  endif()

  target_link_libraries(${name} ${libtype}
  ${ARG_LINK_LIBS}
  ${lib_deps}
  ${llvm_libs}
  )
```

We could query `get_target_property(TARGET_TYPE ${target} TYPE)` and use that 
to determine the correct dependency type.

You're also right that it's possible to build both static and dynamic 
libraries. We could check for the existence of `${target}_static` and add the 
dependencies there as well.

If we handle it there, we'll also solve it for other libraries that depend on 
Clang components. (I'm thinking of libraries in clang-tools-extra and lldb, 
where I'd like to propose similar changes to this one.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67321



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

[PATCH] D64991: [analyzer][WIP] Implement a primitive reaching definitions analysis

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D64991#1662185 , @Szelethus wrote:

> Hmm, we could make a redundant assignments checker: if a variable has 
> multiple reaching definitions, but those all assign the same value, emit a 
> warning. We could even use fixits with that.
>
>   void t(int a) {
> if (coin())
>   a = 2; // note: reaching def
> else
>   a = 2; // note: reaching def
> use(a); // warn: a is always 2 here
>   }


Sounds like a useful //compiler warning// to me.

Also what specific fixit do you have in mind and why do you think it'll be 
easily obtainable from the results of the reaching definition analysis?


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

https://reviews.llvm.org/D64991



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

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D67140#1659982 , @gribozavr wrote:

> We should take a page from desktop software here. If the messages were in a 
> separate file, there would be a lot of people capable of mass-editing them. 
> When messages are hardcoded in the tool code, navigating and editing them 
> requires more skill, and definitely a lot more jumping around.


In the Static Analyzer there's often an explosive amount of dynamically 
generated messages that are going to be pretty hard to stuff into a tablegen 
pattern. Say, you can probably turn this 

 into "`%0 %1 %2 with a %3 retain count into an out parameter %4%5`" but would 
it really help?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67140



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

[PATCH] D67382: [analyzer] NFC: Move getStmt() and createEndOfPath() out of PathDiagnostic.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, 
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, zzheng, szepet, kristof.beyls.
Herald added a project: clang.

These static functions deal with `ExplodedNode`s which is something i don't 
want the `PathDiagnostic` interface to know anything about, as it's planned to 
be moved out of `libStaticAnalyzer`.

`PathDiagnosticLocation::getStmt(N)`, a function commonly used in bug visitors, 
is now known as `ExplodedNode::getStmtForDiagnostics()`. Because it contains a 
hack that allows it to avoid body-farmed locations, it should only be used for 
diagnostics and is useless for most other purposes, hence the name. I added a 
few FIXMEs in places where this may be already causing problems. 
`PathDiagnosticLocation::getNextStmt(N)` and a couple of helper functions from 
`BugReporter.cpp` now live nearby.

`PathDiagnosticLocation::createEndOfPath(N)` has a similar purpose but is more 
sophisticated and is used mostly by end-of-path visitors. I removed this 
function entirely in favor of using `PathSensitiveBugReport::getLocation()` 
instead, which is literally the same thing as long as `N` is the bug node, 
which is always true. This creates a certain problem in `RetainCountChecker` 
(surprise!!~) because it has its own `PathSensitiveBugReport` sub-class that 
overrides `getLocation()` to not be equal to end of path, but still needs to 
obtain the end-of-path location in the visitor. I worked around that by giving 
it a separate method.


Repository:
  rC Clang

https://reviews.llvm.org/D67382

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
  clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
  clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
  clang/lib/StaticAnalyzer/Checkers/Taint.cpp
  clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
  clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

Index: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -30,7 +30,6 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/None.h"
@@ -524,12 +523,12 @@
 // PathDiagnosticLocation methods.
 //===--===//
 
-static SourceLocation getValidSourceLocation(const Stmt* S,
- LocationOrAnalysisDeclContext LAC,
- bool UseEnd = false) {
+SourceLocation PathDiagnosticLocation::getValidSourceLocation(
+const Stmt *S, LocationOrAnalysisDeclContext LAC, bool UseEnd) {
   SourceLocation L = UseEnd ? S->getEndLoc() : S->getBeginLoc();
-  assert(!LAC.isNull() && "A valid LocationContext or AnalysisDeclContext should "
-  "be passed to PathDiagnosticLocation upon creation.");
+  assert(!LAC.isNull() &&
+ "A valid LocationContext or AnalysisDeclContext should be passed to "
+ "PathDiagnosticLocation upon creation.");
 
   // S might be a temporary statement that does not have a location in the
   // source code, so find an enclosing statement and use its location.
@@ -778,117 +777,6 @@
   return PathDiagnosticLocation(S, SMng, P.getLocationContext());
 }
 
-static const LocationContext *
-findTopAutosynthesizedParentContext(const LocationContext *LC) {
-  assert(LC->getAnalysisDeclContext()->isBodyAutosynthesized());
-  const LocationContext *ParentLC = LC->getParent();
-  assert(ParentLC && "We don't start analysis from autosynthesized code");
-  while (ParentLC->getAnalysisDeclContext()->isBodyAutosynthesized()) {
-LC = ParentLC;
-ParentLC = LC->getParent();
-assert(ParentLC && "We don't start analysis from 

[PATCH] D67381: [analyzer] NFC: Move stack hints to a side map.

[Artem Dergachev via Phabricator via cfe-commits]

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

Stack hints are attached to `PathDiagnosticEventPiece`s in order to improve 
path notes at the call site for the call in which the event has occured. For 
example, they are currently used only by MallocChecker in order to produce the 
fancy "Returning allocated memory"/"Returning; memory was released" note. By 
the way, in fact nobody else currently uses this functionality - only 
MallocChecker.

I want to make the `PathDiagnostic` interface completely ignorant of Static 
Analyzer specific concepts such as `ExplodedNode` or `SymbolRef`, so i moved 
this interface to `BugReporter`. Stack hints are now owned by the 
`PathSensitiveBugReport` object against which the visitor emits event pieces 
for which it needs stack hints.

I'm open to discuss a better design here. Eg., i thought about making it part 
of the visitor interface instead, but i don't immediately see how to do this 
without breaking the logic of "only add the note at the call site in which the 
event has happened, not every time allocated memory is returned from anywhere".


Repository:
  rC Clang

https://reviews.llvm.org/D67381

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

Index: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -31,7 +31,6 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/None.h"
@@ -1303,70 +1302,6 @@
 ID.AddString(*I);
 }
 
-StackHintGenerator::~StackHintGenerator() = default;
-
-std::string StackHintGeneratorForSymbol::getMessage(const ExplodedNode *N){
-  if (!N)
-return getMessageForSymbolNotFound();
-
-  ProgramPoint P = N->getLocation();
-  CallExitEnd CExit = P.castAs();
-
-  // FIXME: Use CallEvent to abstract this over all calls.
-  const Stmt *CallSite = CExit.getCalleeContext()->getCallSite();
-  const auto *CE = dyn_cast_or_null(CallSite);
-  if (!CE)
-return {};
-
-  // Check if one of the parameters are set to the interesting symbol.
-  unsigned ArgIndex = 0;
-  for (CallExpr::const_arg_iterator I = CE->arg_begin(),
-E = CE->arg_end(); I != E; ++I, ++ArgIndex){
-SVal SV = N->getSVal(*I);
-
-// Check if the variable corresponding to the symbol is passed by value.
-SymbolRef AS = SV.getAsLocSymbol();
-if (AS == Sym) {
-  return getMessageForArg(*I, ArgIndex);
-}
-
-// Check if the parameter is a pointer to the symbol.
-if (Optional Reg = SV.getAs()) {
-  // Do not attempt to dereference void*.
-  if ((*I)->getType()->isVoidPointerType())
-continue;
-  SVal PSV = N->getState()->getSVal(Reg->getRegion());
-  SymbolRef AS = PSV.getAsLocSymbol();
-  if (AS == Sym) {
-return getMessageForArg(*I, ArgIndex);
-  }
-}
-  }
-
-  // Check if we are returning the interesting symbol.
-  SVal SV = N->getSVal(CE);
-  SymbolRef RetSym = SV.getAsLocSymbol();
-  if (RetSym == Sym) {
-return getMessageForReturn(CE);
-  }
-
-  return getMessageForSymbolNotFound();
-}
-
-std::string StackHintGeneratorForSymbol::getMessageForArg(const Expr *ArgE,
-  unsigned ArgIndex) {
-  // Printed parameters start at 1, not 0.
-  ++ArgIndex;
-
-  SmallString<200> buf;
-  llvm::raw_svector_ostream os(buf);
-
-  os << Msg << " via " << ArgIndex << llvm::getOrdinalSuffix(ArgIndex)
- << " parameter";
-
-  return os.str();
-}
-
 LLVM_DUMP_METHOD void PathPieces::dump() const {
   unsigned index = 0;
   for (PathPieces::const_iterator I = begin(), E = end(); I != E; ++I) {
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -239,6 +239,8 @@
   

[PATCH] D67373: Don't emit .gnu_pubnames when tuning for LLDB

[David Blaikie via Phabricator via cfe-commits]

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

Seems good to me - thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67373



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

[PATCH] D67373: Don't emit .gnu_pubnames when tuning for LLDB

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3400
   if (DwarfFission != DwarfFissionKind::None ||
-  DebuggerTuning == llvm::DebuggerKind::LLDB ||
   (PubnamesArg && checkDebugInfoOption(PubnamesArg, Args, D, TC)))

Is this codepath Darwin only? Even if it's not this should be fine because we 
should emit debug_names if debugger tuning is lldb. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67373



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

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

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

There's definitely a lot of new findings this creates, but it's hard to say 
exactly how many root causes there are due to the way test failures are (not) 
grouped well in the way I'm testing. So far they all seem like true positives, 
so this would be good to submit. However a few are positive yet benign, like 
this interesting one (simplified):

  void ParseString(char *s) {
char *next = s;
for (char *end = s; end; next = end + 1) { // ubsan error computing (nil + 
1), although it doesn't matter because the loop terminates when end == nil and 
next is not read after the loop
  // ...
  end = strchr(next, 'x'); // returns null if not found
  // ...
}
  }

If I had to guesstimate, I'd say 20-100 bugs in a couple billion lines of code, 
so a lot, but shouldn't be too disruptive to anyone that has these checks 
enabled globally.

I haven't noticed any timeouts -- which is not to say this isn't a slowdown, 
but at least it's not egregious.

BTW, here's a minimal + complete repro of the original issue:

  $ cat ub.cc
  #include 
  #include 
  
  static void Test(const char *x, int offset) {
printf("%p + %d => %s\n", x, offset, x + offset ? "true" : "false");
  }
  
  int main(int argc, char **argv) {
if (argc != 3) return 1;
  
const char *x = reinterpret_cast(atoi(argv[1]));
int offset = atoi(argv[2]);
  
Test(x, offset);
  
return 0;
  }
  $ previous-clang++ -O3 ub.cc && ./a.out 0 1
  (nil) + 1 => true
  $ next-clang++ -O3 ub.cc && ./a.out 0 1
  (nil) + 1 => false
  $ patch-D67122-clang++ -O3 -fsanitize=undefined ub.cc && ./a.out 0 1
  ubsan: ub.cc:5:42: runtime error: applying non-zero offset 1 to null pointer  

  
  (nil) + 1 => false


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67122



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

[PATCH] D67373: Don't emit .gnu_pubnames when tuning for LLDB

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In D67373#1663924 , @dblaikie wrote:

> Have you got a link to the original thread where this was discussed/I 
> mentioned it? Want to page in some context to double-check if I had any ideas 
> that might've let this simplify things.


Sure! 
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20190422/thread.html#646062


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67373



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

r371468 - PR43242: Fix crash when typo-correcting to an operator() that should not

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Sep  9 16:07:22 2019
New Revision: 371468

URL: http://llvm.org/viewvc/llvm-project?rev=371468=rev
Log:
PR43242: Fix crash when typo-correcting to an operator() that should not
have been visible.

Modified:
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/SemaCXX/lambda-expressions.cpp

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=371468=371467=371468=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Sep  9 16:07:22 2019
@@ -1990,16 +1990,7 @@ bool Sema::DiagnoseEmptyLookup(Scope *S,
   R.clear();
 }
 
-// In Microsoft mode, if we are performing lookup from within a friend
-// function definition declared at class scope then we must set
-// DC to the lexical parent to be able to search into the parent
-// class.
-if (getLangOpts().MSVCCompat && isa(DC) &&
-cast(DC)->getFriendObjectKind() &&
-DC->getLexicalParent()->isRecord())
-  DC = DC->getLexicalParent();
-else
-  DC = DC->getParent();
+DC = DC->getLookupParent();
   }
 
   // We didn't find anything, so try to correct for a typo.

Modified: cfe/trunk/test/SemaCXX/lambda-expressions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/lambda-expressions.cpp?rev=371468=371467=371468=diff
==
--- cfe/trunk/test/SemaCXX/lambda-expressions.cpp (original)
+++ cfe/trunk/test/SemaCXX/lambda-expressions.cpp Mon Sep  9 16:07:22 2019
@@ -630,3 +630,7 @@ void Run(const int& points) {
   };
 }
 }
+
+void operator_parens() {
+  [&](int x){ operator()(); }(0); // expected-error {{undeclared 'operator()'}}
+}


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

[PATCH] D67373: Don't emit .gnu_pubnames when tuning for LLDB

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Have you got a link to the original thread where this was discussed/I mentioned 
it? Want to page in some context to double-check if I had any ideas that 
might've let this simplify things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67373



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

[PATCH] D62731: [RFC] Add support for options -frounding-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D62731#1663748 , @rjmccall wrote:

> Hmm, you know, there are enough different FP options that I think we should 
> probably split them all out into their own section in the manual instead of 
> just listing them under "code generation".  That will also give us an obvious 
> place to describe the basic model, i.e. all the stuff about it mostly coming 
> down to different strictness and exception models.  Could you prepare a patch 
> that *just* does that reorganization without adding any new features, and 
> then we can add the new options on top of that?


Yes I'll do that


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D67373: Don't emit .gnu_pubnames when tuning for LLDB

[Adrian Prantl via Phabricator via cfe-commits]

aprantl created this revision.
aprantl added reviewers: jasonmolenda, dblaikie, friss, emaste, JDevlieghere.
Herald added a project: clang.

LLDB reads the various `.apple*` accelerator tables which should make 
`.gnu_pubnames` redundant. This changes the Clang driver to no longer pass 
`-ggnu-pubnames` when tuning for LLDB.

Thanks to @dblaikie for pointing this out (a few months back; my backlog is 
real).

rdar://problem/50142073


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67373

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -197,7 +197,7 @@
 // RUN: %clang -### -c %s 2>&1 | FileCheck -check-prefix=NORNGBSE %s
 // RUN: %clang -### -c -fdebug-ranges-base-address 
-fno-debug-ranges-base-address %s 2>&1 | FileCheck -check-prefix=NORNGBSE %s
 //
-// RUN: %clang -### -c -glldb %s 2>&1 | FileCheck -check-prefix=GPUB %s
+// RUN: %clang -### -c -glldb %s 2>&1 | FileCheck -check-prefix=NOPUB %s
 // RUN: %clang -### -c -glldb -gno-pubnames %s 2>&1 | FileCheck 
-check-prefix=NOPUB %s
 //
 // RUN: %clang -### -c -gdwarf-aranges %s 2>&1 | FileCheck 
-check-prefix=GARANGE %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3397,7 +3397,6 @@
   Args.getLastArg(options::OPT_ggnu_pubnames, 
options::OPT_gno_gnu_pubnames,
   options::OPT_gpubnames, options::OPT_gno_pubnames);
   if (DwarfFission != DwarfFissionKind::None ||
-  DebuggerTuning == llvm::DebuggerKind::LLDB ||
   (PubnamesArg && checkDebugInfoOption(PubnamesArg, Args, D, TC)))
 if (!PubnamesArg ||
 (!PubnamesArg->getOption().matches(options::OPT_gno_gnu_pubnames) &&


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -197,7 +197,7 @@
 // RUN: %clang -### -c %s 2>&1 | FileCheck -check-prefix=NORNGBSE %s
 // RUN: %clang -### -c -fdebug-ranges-base-address -fno-debug-ranges-base-address %s 2>&1 | FileCheck -check-prefix=NORNGBSE %s
 //
-// RUN: %clang -### -c -glldb %s 2>&1 | FileCheck -check-prefix=GPUB %s
+// RUN: %clang -### -c -glldb %s 2>&1 | FileCheck -check-prefix=NOPUB %s
 // RUN: %clang -### -c -glldb -gno-pubnames %s 2>&1 | FileCheck -check-prefix=NOPUB %s
 //
 // RUN: %clang -### -c -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=GARANGE %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3397,7 +3397,6 @@
   Args.getLastArg(options::OPT_ggnu_pubnames, options::OPT_gno_gnu_pubnames,
   options::OPT_gpubnames, options::OPT_gno_pubnames);
   if (DwarfFission != DwarfFissionKind::None ||
-  DebuggerTuning == llvm::DebuggerKind::LLDB ||
   (PubnamesArg && checkDebugInfoOption(PubnamesArg, Args, D, TC)))
 if (!PubnamesArg ||
 (!PubnamesArg->getOption().matches(options::OPT_gno_gnu_pubnames) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r371080 - [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

[Alexandre Ganea via cfe-commits]

Hi David,

I've reverted the patch in r371113. It was causing crashes in asan on Linux & 
Darwin.
Before re-landing the patch, it needs this: https://reviews.llvm.org/D67283

Alex.

-Message d'origine-
De : David Blaikie  
Envoyé : September 9, 2019 6:12 PM
À : Erik Pilkington 
Cc : Alexandre Ganea ; cfe-commits 

Objet : Re: r371080 - [DebugInfo] Add debug location to stubs generated by 
CGDeclCXX and mark them as artificial

Any resolution/discussion on this crash?

On Thu, Sep 5, 2019 at 12:49 PM Erik Pilkington via cfe-commits 
 wrote:
>
> Hi Alexandre,
>
> Looks like this commit is causing crashes on darwin, can you take a 
> look please? Here is a failing bot: 
> http://lab.llvm.org:8080/green/job/clang-stage1-RA/1671/
>
> Thanks!
> Erik
>
> On Thu, Sep 5, 2019 at 11:23 AM Alexandre Ganea via cfe-commits 
>  wrote:
>>
>> Author: aganea
>> Date: Thu Sep  5 08:24:49 2019
>> New Revision: 371080
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=371080=rev
>> Log:
>> [DebugInfo] Add debug location to stubs generated by CGDeclCXX and 
>> mark them as artificial
>>
>> Differential Revision: https://reviews.llvm.org/D66328
>>
>> Added:
>> cfe/trunk/test/CodeGenCXX/debug-info-atexit-stub.cpp
>> cfe/trunk/test/CodeGenCXX/debug-info-destroy-helper.cpp
>> Modified:
>> cfe/trunk/include/clang/AST/GlobalDecl.h
>> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>> cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
>> cfe/trunk/test/CodeGenCXX/debug-info-global-ctor-dtor.cpp
>> cfe/trunk/test/CodeGenCXX/debug-info-line.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/GlobalDecl.h
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Globa
>> lDecl.h?rev=371080=371079=371080=diff
>> =
>> =
>> --- cfe/trunk/include/clang/AST/GlobalDecl.h (original)
>> +++ cfe/trunk/include/clang/AST/GlobalDecl.h Thu Sep  5 08:24:49 2019
>> @@ -31,6 +31,7 @@ enum class DynamicInitKind : unsigned {
>>NoStub = 0,
>>Initializer,
>>AtExit,
>> +  GlobalArrayDestructor
>>  };
>>
>>  /// GlobalDecl - represents a global declaration. This can either be 
>> a
>>
>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo
>> .cpp?rev=371080=371079=371080=diff
>> =
>> =
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Sep  5 08:24:49 2019
>> @@ -1910,7 +1910,8 @@ StringRef CGDebugInfo::getDynamicInitial
>>   llvm::Function *InitFn) {
>>// If we're not emitting codeview, use the mangled name. For Itanium, 
>> this is
>>// arbitrary.
>> -  if (!CGM.getCodeGenOpts().EmitCodeView)
>> +  if (!CGM.getCodeGenOpts().EmitCodeView ||
>> +  StubKind == DynamicInitKind::GlobalArrayDestructor)
>>  return InitFn->getName();
>>
>>// Print the normal qualified name for the variable, then break 
>> off the last @@ -1935,6 +1936,7 @@ StringRef 
>> CGDebugInfo::getDynamicInitial
>>
>>switch (StubKind) {
>>case DynamicInitKind::NoStub:
>> +  case DynamicInitKind::GlobalArrayDestructor:
>>  llvm_unreachable("not an initializer");
>>case DynamicInitKind::Initializer:
>>  OS << "`dynamic initializer for '"; @@ -3569,7 +3571,8 @@ void 
>> CGDebugInfo::EmitFunctionStart(Glob
>>if (Name.startswith("\01"))
>>  Name = Name.substr(1);
>>
>> -  if (!HasDecl || D->isImplicit() || D->hasAttr()) {
>> +  if (!HasDecl || D->isImplicit() || D->hasAttr() ||
>> +  (isa(D) && GD.getDynamicInitKind() != 
>> + DynamicInitKind::NoStub)) {
>>  Flags |= llvm::DINode::FlagArtificial;
>>  // Artificial functions should not silently reuse CurLoc.
>>  CurLoc = SourceLocation();
>>
>> Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.c
>> pp?rev=371080=371079=371080=diff
>> =
>> =
>> --- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Thu Sep  5 08:24:49 2019
>> @@ -247,6 +247,8 @@ llvm::Function *CodeGenFunction::createA
>>
>>CGF.StartFunction(GlobalDecl(, DynamicInitKind::AtExit),
>>  CGM.getContext().VoidTy, fn, FI, 
>> FunctionArgList());
>> +  // Emit an artificial location for this function.
>> +  auto AL = ApplyDebugLocation::CreateArtificial(CGF);
>>
>>llvm::CallInst *call = CGF.Builder.CreateCall(dtor, addr);
>>
>> @@ -642,8 +644,9 @@ void CodeGenFunction::GenerateCXXGlobalV
>>
>>StartFunction(GlobalDecl(D, DynamicInitKind::Initializer),
>>  getContext().VoidTy, Fn, 
>> getTypes().arrangeNullaryFunction(),
>> -FunctionArgList(), D->getLocation(),
>> -D->getInit()->getExprLoc());
>> +   

Re: r371080 - [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

[David Blaikie via cfe-commits]

Any resolution/discussion on this crash?

On Thu, Sep 5, 2019 at 12:49 PM Erik Pilkington via cfe-commits
 wrote:
>
> Hi Alexandre,
>
> Looks like this commit is causing crashes on darwin, can you take a look 
> please? Here is a failing bot: 
> http://lab.llvm.org:8080/green/job/clang-stage1-RA/1671/
>
> Thanks!
> Erik
>
> On Thu, Sep 5, 2019 at 11:23 AM Alexandre Ganea via cfe-commits 
>  wrote:
>>
>> Author: aganea
>> Date: Thu Sep  5 08:24:49 2019
>> New Revision: 371080
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=371080=rev
>> Log:
>> [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them 
>> as artificial
>>
>> Differential Revision: https://reviews.llvm.org/D66328
>>
>> Added:
>> cfe/trunk/test/CodeGenCXX/debug-info-atexit-stub.cpp
>> cfe/trunk/test/CodeGenCXX/debug-info-destroy-helper.cpp
>> Modified:
>> cfe/trunk/include/clang/AST/GlobalDecl.h
>> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>> cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
>> cfe/trunk/test/CodeGenCXX/debug-info-global-ctor-dtor.cpp
>> cfe/trunk/test/CodeGenCXX/debug-info-line.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/GlobalDecl.h
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/GlobalDecl.h?rev=371080=371079=371080=diff
>> ==
>> --- cfe/trunk/include/clang/AST/GlobalDecl.h (original)
>> +++ cfe/trunk/include/clang/AST/GlobalDecl.h Thu Sep  5 08:24:49 2019
>> @@ -31,6 +31,7 @@ enum class DynamicInitKind : unsigned {
>>NoStub = 0,
>>Initializer,
>>AtExit,
>> +  GlobalArrayDestructor
>>  };
>>
>>  /// GlobalDecl - represents a global declaration. This can either be a
>>
>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=371080=371079=371080=diff
>> ==
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Sep  5 08:24:49 2019
>> @@ -1910,7 +1910,8 @@ StringRef CGDebugInfo::getDynamicInitial
>>   llvm::Function *InitFn) {
>>// If we're not emitting codeview, use the mangled name. For Itanium, 
>> this is
>>// arbitrary.
>> -  if (!CGM.getCodeGenOpts().EmitCodeView)
>> +  if (!CGM.getCodeGenOpts().EmitCodeView ||
>> +  StubKind == DynamicInitKind::GlobalArrayDestructor)
>>  return InitFn->getName();
>>
>>// Print the normal qualified name for the variable, then break off the 
>> last
>> @@ -1935,6 +1936,7 @@ StringRef CGDebugInfo::getDynamicInitial
>>
>>switch (StubKind) {
>>case DynamicInitKind::NoStub:
>> +  case DynamicInitKind::GlobalArrayDestructor:
>>  llvm_unreachable("not an initializer");
>>case DynamicInitKind::Initializer:
>>  OS << "`dynamic initializer for '";
>> @@ -3569,7 +3571,8 @@ void CGDebugInfo::EmitFunctionStart(Glob
>>if (Name.startswith("\01"))
>>  Name = Name.substr(1);
>>
>> -  if (!HasDecl || D->isImplicit() || D->hasAttr()) {
>> +  if (!HasDecl || D->isImplicit() || D->hasAttr() ||
>> +  (isa(D) && GD.getDynamicInitKind() != 
>> DynamicInitKind::NoStub)) {
>>  Flags |= llvm::DINode::FlagArtificial;
>>  // Artificial functions should not silently reuse CurLoc.
>>  CurLoc = SourceLocation();
>>
>> Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=371080=371079=371080=diff
>> ==
>> --- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Thu Sep  5 08:24:49 2019
>> @@ -247,6 +247,8 @@ llvm::Function *CodeGenFunction::createA
>>
>>CGF.StartFunction(GlobalDecl(, DynamicInitKind::AtExit),
>>  CGM.getContext().VoidTy, fn, FI, FunctionArgList());
>> +  // Emit an artificial location for this function.
>> +  auto AL = ApplyDebugLocation::CreateArtificial(CGF);
>>
>>llvm::CallInst *call = CGF.Builder.CreateCall(dtor, addr);
>>
>> @@ -642,8 +644,9 @@ void CodeGenFunction::GenerateCXXGlobalV
>>
>>StartFunction(GlobalDecl(D, DynamicInitKind::Initializer),
>>  getContext().VoidTy, Fn, 
>> getTypes().arrangeNullaryFunction(),
>> -FunctionArgList(), D->getLocation(),
>> -D->getInit()->getExprLoc());
>> +FunctionArgList());
>> +  // Emit an artificial location for this function.
>> +  auto AL = ApplyDebugLocation::CreateArtificial(*this);
>>
>>// Use guarded initialization if the global variable is weak. This
>>// occurs for, e.g., instantiated static data members and
>> @@ -768,7 +771,10 @@ llvm::Function *CodeGenFunction::generat
>>
>>CurEHLocation = VD->getBeginLoc();
>>
>> -  StartFunction(VD, getContext().VoidTy, fn, FI, args);
>> +  

[PATCH] D65371: do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614)

[David Blaikie via Phabricator via cfe-commits]

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

Fair enough - thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D65371



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

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D66559#1663887 , @cchen wrote:

> @ABataev, could you mark it as ready to land, please. Thanks.


I accepted it already, don't know what else I can do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66559



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

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

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

In D64146#1663867 , @nand wrote:

> I am providing definitions in the C++ file - the problem is that they are not 
> available in the header before the extern declaration. The methods are 
> available at the site of the extern definition.
>  gcc and clang accept this,  so does Visual Studio 2019. This feels like an 
> incorrect implementation of extern templates in Visual Studio?
>
> I see two ways to proceed: move everything into a header (would like to avoid 
> this) or silence the warning on VC++ (not great either). 
>  Is there a better way? Which option is less bad from these two?


I think I'm missing something - for instance when I search the patch for 
"visitIndirectMember", all I get is this in ByteCodeExprGen.h:

  bool visitIndirectMember(const BinaryOperator *E);

I don't see it in ByteCodeExprGen.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64146



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

[PATCH] D66796: [clang] Loop pragma vectorize(disable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

And thanks Florian for getting this discussion going again! Will definitely 
make this clear(er) in this description and commit message once we agree on it.


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

https://reviews.llvm.org/D66796



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

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

[Chi Chun Chen via Phabricator via cfe-commits]

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

@ABataev, could you mark it as ready to land, please. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66559



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

[PATCH] D66796: [clang] Loop pragma vectorize(disable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Hi Hideki,

I think you're comments are spot on:

> It all depends on to whom we are providing these pragmas.

Pragma's are user-facing "options" to override or force  compiler decision 
making. I don't think there's another way to look at it, but please correct me 
if I'm wrong.

That's exactly the reason why I think `vectorize(disable)` should disable 
vectorisation for that loop. I just don't see what else a user would expect.


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

https://reviews.llvm.org/D66796



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

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

[Nandor Licker via Phabricator via cfe-commits]

nand added a comment.

I am providing definitions in the C++ file - the problem is that they are not 
available in the header before the extern declaration. The methods are 
available at the site of the extern definition.
gcc and clang accept this,  so does Visual Studio 2019. This feels like an 
incorrect implementation of extern templates in Visual Studio?

I see two ways to proceed: move everything into a header (would like to avoid 
this) or silence the warning on VC++ (not great either). 
Is there a better way? Which option is less bad from these two?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64146



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

[PATCH] D66696: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

I'm a bit curious about clients that use `getCanonicalType()` to get a full 
desugaring, instead of doing a single step. It seems like they'd still get the 
out of date type parameter type. Has that ever worked?


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

https://reviews.llvm.org/D66696



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

[PATCH] D67304: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI

[Reid Kleckner via Phabricator via cfe-commits]

rnk requested review of this revision.
rnk added a comment.

Ptal, new patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67304



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

[PATCH] D67024: [analyzer] NFC: Replace intrusive list of bug reports with a vector of pointers.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371451: [analyzer] NFC: Simplify bug report equivalence 
classes to not be ilists. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67024?vs=219011=219427#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67024

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

Index: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -72,7 +72,7 @@
 
 /// This class provides an interface through which checkers can create
 /// individual bug reports.
-class BugReport : public llvm::ilist_node {
+class BugReport {
 public:
   enum class Kind { Basic, PathSensitive };
 
@@ -465,28 +465,21 @@
   friend class BugReporter;
 
   /// List of *owned* BugReport objects.
-  llvm::ilist Reports;
+  llvm::SmallVector, 4> Reports;
 
-  void AddReport(std::unique_ptr R) {
-Reports.push_back(R.release());
+  void AddReport(std::unique_ptr &) {
+Reports.push_back(std::move(R));
   }
 
 public:
   BugReportEquivClass(std::unique_ptr R) { AddReport(std::move(R)); }
 
+  ArrayRef> getReports() const { return Reports; }
+
   void Profile(llvm::FoldingSetNodeID& ID) const {
 assert(!Reports.empty());
-Reports.front().Profile(ID);
+Reports.front()->Profile(ID);
   }
-
-  using iterator = llvm::ilist::iterator;
-  using const_iterator = llvm::ilist::const_iterator;
-
-  iterator begin() { return Reports.begin(); }
-  iterator end() { return Reports.end(); }
-
-  const_iterator begin() const { return Reports.begin(); }
-  const_iterator end() const { return Reports.end(); }
 };
 
 //===--===//
@@ -573,7 +566,7 @@
   virtual BugReport *
   findReportInEquivalenceClass(BugReportEquivClass ,
SmallVectorImpl ) {
-return &*eqClass.begin();
+return eqClass.getReports()[0].get();
   }
 
 protected:
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3004,8 +3004,8 @@
 llvm::make_range(BR.EQClasses_begin(), BR.EQClasses_end());
 
 for (const auto  : EQClasses) {
-  for (const BugReport  : EQ) {
-const auto *PR = dyn_cast();
+  for (const auto  : EQ.getReports()) {
+const auto *PR = dyn_cast(I.get());
 if (!PR)
   continue;
 const ExplodedNode *EN = PR->getErrorNode();
@@ -3135,7 +3135,8 @@
 // Iterate through the reports and get their nodes.
 for (BugReporter::EQClasses_iterator
EI = BR.EQClasses_begin(), EE = BR.EQClasses_end(); EI != EE; ++EI) {
-  const auto *R = dyn_cast(&*EI->begin());
+  const auto *R =
+  dyn_cast(EI->getReports()[0].get());
   if (!R)
 continue;
   const auto *N = const_cast(R->getErrorNode());
Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2798,17 +2798,15 @@
 
 BugReport *PathSensitiveBugReporter::findReportInEquivalenceClass(
 BugReportEquivClass , SmallVectorImpl ) {
-  BugReportEquivClass::iterator I = EQ.begin(), E = EQ.end();
-  assert(I != E);
-  const BugType& BT = I->getBugType();
-
   // If we don't need to suppress any of the nodes because they are
   // post-dominated by a sink, simply add all the nodes in the equivalence class
   // to 'Nodes'.  Any of the reports will serve as a "representative" report.
+  assert(EQ.getReports().size() > 0);
+  const BugType& BT = EQ.getReports()[0]->getBugType();
   if (!BT.isSuppressOnSink()) {
-BugReport *R = &*I;
-for (auto  : EQ) {
-  if (auto *PR = dyn_cast()) {
+BugReport *R = EQ.getReports()[0].get();
+for (auto  : EQ.getReports()) {
+  if (auto *PR = dyn_cast(J.get())) {
 R = PR;
 bugReports.push_back(PR);
   }
@@ -2824,8 +2822,8 @@
   // stack for very long paths.
   BugReport *exampleReport = nullptr;
 
-  for (; I != E; ++I) {
-auto *R = dyn_cast(&*I);
+  for (const auto : EQ.getReports()) {
+auto *R = dyn_cast(I.get());
 if (!R)
   continue;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D67304: Unify checking enumerator values in ObjC, C, and MSVC modes

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D67304#1663042 , @hans wrote:

> Should there be a test exercising this part? The updated tests both have 
> -fms-compatibility, so were already just warning.


Good point, we aren't testing that this unfixed enum behavior is a part of the 
ABI, not `-fms-compatibility`.

After testing this some more, I think I need to back off on my misguided 
refactoring. I didn't notice that the C99 warning was an off-by-default 
pedantic warning, and it didn't insert the implicit cast in the case where the 
value was not representable in int.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67304



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

[PATCH] D62731: [RFC] Add support for options -frounding-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Hmm, you know, there are enough different FP options that I think we should 
probably split them all out into their own section in the manual instead of 
just listing them under "code generation".  That will also give us an obvious 
place to describe the basic model, i.e. all the stuff about it mostly coming 
down to different strictness and exception models.  Could you prepare a patch 
that *just* does that reorganization without adding any new features, and then 
we can add the new options on top of that?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D66696: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


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

https://reviews.llvm.org/D66696



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

[PATCH] D67304: Unify checking enumerator values in ObjC, C, and MSVC modes

[Reid Kleckner via Phabricator via cfe-commits]

rnk updated this revision to Diff 219425.
rnk added a comment.

- rewrite, abandon unification


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67304

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/MicrosoftCompatibility.c


Index: clang/test/Sema/MicrosoftCompatibility.c
===
--- clang/test/Sema/MicrosoftCompatibility.c
+++ clang/test/Sema/MicrosoftCompatibility.c
@@ -1,10 +1,18 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify 
-fms-compatibility -triple i686-pc-win32
+// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify 
-fms-compatibility -DMSVCCOMPAT -triple i686-pc-win32
+// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify 
-fms-extensions -triple i686-pc-win32
 
+#ifdef MSVCCOMPAT
 enum ENUM1; // expected-warning {{forward references to 'enum' types are a 
Microsoft extension}}
 enum ENUM1 var1 = 3;
 enum ENUM1* var2 = 0;
+#else
+enum ENUM1; // expected-note {{forward declaration of}}
+enum ENUM1 var1 = 3; // expected-error {{variable has incomplete type 'enum 
ENUM1'}}
+enum ENUM1* var2 = 0;
+#endif
 
 
+// FIXME: The rest of this seems to be controlled by -fms-extensions. Move it.
 enum ENUM2 {
   ENUM2_a = (enum ENUM2) 4,
   ENUM2_b = 0x9FFF, // expected-warning {{enumerator value is not 
representable in the underlying type 'int'}}
@@ -22,4 +30,8 @@
 
 __declspec(__noreturn__) void f7(void); /* expected-warning {{__declspec 
attribute '__noreturn__' is not supported}} */
 
+#ifdef MSVCCOMPAT
 size_t x;
+#else
+size_t x; // expected-error {{unknown type name 'size_t'}}
+#endif
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14723,7 +14723,7 @@
   UPPC_FixedUnderlyingType))
 EnumUnderlying = Context.IntTy.getTypePtr();
 
-} else if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+} else if (Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment()) 
{
   // For MSVC ABI compatibility, unfixed enums must use an underlying type
   // of 'int'. However, if this is an unfixed forward declaration, don't 
set
   // the underlying type unless the user enables -fms-compatibility. This
@@ -16850,8 +16850,7 @@
 if (Enum->isDependentType() || Val->isTypeDependent())
   EltTy = Context.DependentTy;
 else {
-  if (getLangOpts().CPlusPlus11 && Enum->isFixed() &&
-  !getLangOpts().MSVCCompat) {
+  if (getLangOpts().CPlusPlus11 && Enum->isFixed()) {
 // C++11 [dcl.enum]p5: If the underlying type is fixed, [...] the
 // constant-expression in the enumerator-definition shall be a 
converted
 // constant expression of the underlying type.
@@ -16876,15 +16875,19 @@
   // we perform a non-narrowing conversion as part of converted 
constant
   // expression checking.
   if (!isRepresentableIntegerValue(Context, EnumVal, EltTy)) {
-if (getLangOpts().MSVCCompat) {
+if (Context.getTargetInfo()
+.getTriple()
+.isWindowsMSVCEnvironment()) {
   Diag(IdLoc, diag::ext_enumerator_too_large) << EltTy;
-  Val = ImpCastExprToType(Val, EltTy, CK_IntegralCast).get();
-} else
+} else {
   Diag(IdLoc, diag::err_enumerator_too_large) << EltTy;
-  } else
-Val = ImpCastExprToType(Val, EltTy,
-EltTy->isBooleanType() ?
-CK_IntegralToBoolean : CK_IntegralCast)
+}
+  }
+
+  // Cast to the underlying type.
+  Val = ImpCastExprToType(Val, EltTy,
+  EltTy->isBooleanType() ? CK_IntegralToBoolean
+ : CK_IntegralCast)
 .get();
 } else if (getLangOpts().CPlusPlus) {
   // C++11 [dcl.enum]p5:


Index: clang/test/Sema/MicrosoftCompatibility.c
===
--- clang/test/Sema/MicrosoftCompatibility.c
+++ clang/test/Sema/MicrosoftCompatibility.c
@@ -1,10 +1,18 @@
-// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify -fms-compatibility -triple i686-pc-win32
+// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify -fms-compatibility -DMSVCCOMPAT -triple i686-pc-win32
+// RUN: %clang_cc1 %s -fsyntax-only -Wno-unused-value -Wmicrosoft -verify -fms-extensions -triple i686-pc-win32
 
+#ifdef MSVCCOMPAT
 enum ENUM1; // expected-warning {{forward references to 'enum' types are a Microsoft extension}}
 enum ENUM1 var1 = 3;
 enum ENUM1* var2 = 0;
+#else
+enum ENUM1; // expected-note {{forward 

[PATCH] D67024: [analyzer] NFC: Replace intrusive list of bug reports with a vector of pointers.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked 5 inline comments as done.
NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:569
SmallVectorImpl ) {
-return &*eqClass.begin();
+return eqClass.getReports().begin()->get();
   }

gribozavr wrote:
> `getReports()[0]` ?
Mm right :)


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

https://reviews.llvm.org/D67024



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

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked 5 inline comments as done.
NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:113
+  // encouraged, but the period at the end of the description is still omitted.
+  StringRef getDescription() const { return Description; }
+

gribozavr wrote:
> Thanks for the doc comments! Please use three slashes here and in 
> getShortDescription.
Whoops.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:131
+
+  /// The smallest canonical declaration that contains the bug location.
+  /// This is purely cosmetic; the declaration can be presented to the user

gribozavr wrote:
> Thanks for the explanation, just one question -- I don't understand what is 
> meant by "canonical".
> 
> The bug can be found in a non-canonical declaration.
> 
> ```
> void f(); // canonical decl
> 
> void f() { // non-canonical decl
>   *(int*)0 = 0; // bug
> }
> ```
Ugh, i somehow keep thinking that the definition, if available, will always be 
the `getCanonicalDecl()`, even though i stepped into this a few times already 
>.<

That's why we have bugs like D57619.


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

https://reviews.llvm.org/D66572



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

r371451 - [analyzer] NFC: Simplify bug report equivalence classes to not be ilists.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Mon Sep  9 13:34:44 2019
New Revision: 371451

URL: http://llvm.org/viewvc/llvm-project?rev=371451=rev
Log:
[analyzer] NFC: Simplify bug report equivalence classes to not be ilists.

Use a vector of unique pointers instead.

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=371451=371450=371451=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Mon 
Sep  9 13:34:44 2019
@@ -72,7 +72,7 @@ using DiagnosticForConsumerMapTy =
 
 /// This class provides an interface through which checkers can create
 /// individual bug reports.
-class BugReport : public llvm::ilist_node {
+class BugReport {
 public:
   enum class Kind { Basic, PathSensitive };
 
@@ -465,28 +465,21 @@ class BugReportEquivClass : public llvm:
   friend class BugReporter;
 
   /// List of *owned* BugReport objects.
-  llvm::ilist Reports;
+  llvm::SmallVector, 4> Reports;
 
-  void AddReport(std::unique_ptr R) {
-Reports.push_back(R.release());
+  void AddReport(std::unique_ptr &) {
+Reports.push_back(std::move(R));
   }
 
 public:
   BugReportEquivClass(std::unique_ptr R) { AddReport(std::move(R)); 
}
 
+  ArrayRef> getReports() const { return Reports; }
+
   void Profile(llvm::FoldingSetNodeID& ID) const {
 assert(!Reports.empty());
-Reports.front().Profile(ID);
+Reports.front()->Profile(ID);
   }
-
-  using iterator = llvm::ilist::iterator;
-  using const_iterator = llvm::ilist::const_iterator;
-
-  iterator begin() { return Reports.begin(); }
-  iterator end() { return Reports.end(); }
-
-  const_iterator begin() const { return Reports.begin(); }
-  const_iterator end() const { return Reports.end(); }
 };
 
 
//===--===//
@@ -573,7 +566,7 @@ private:
   virtual BugReport *
   findReportInEquivalenceClass(BugReportEquivClass ,
SmallVectorImpl ) {
-return &*eqClass.begin();
+return eqClass.getReports()[0].get();
   }
 
 protected:

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=371451=371450=371451=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Mon Sep  9 13:34:44 2019
@@ -2798,17 +2798,15 @@ struct FRIEC_WLItem {
 
 BugReport *PathSensitiveBugReporter::findReportInEquivalenceClass(
 BugReportEquivClass , SmallVectorImpl ) {
-  BugReportEquivClass::iterator I = EQ.begin(), E = EQ.end();
-  assert(I != E);
-  const BugType& BT = I->getBugType();
-
   // If we don't need to suppress any of the nodes because they are
   // post-dominated by a sink, simply add all the nodes in the equivalence 
class
   // to 'Nodes'.  Any of the reports will serve as a "representative" report.
+  assert(EQ.getReports().size() > 0);
+  const BugType& BT = EQ.getReports()[0]->getBugType();
   if (!BT.isSuppressOnSink()) {
-BugReport *R = &*I;
-for (auto  : EQ) {
-  if (auto *PR = dyn_cast()) {
+BugReport *R = EQ.getReports()[0].get();
+for (auto  : EQ.getReports()) {
+  if (auto *PR = dyn_cast(J.get())) {
 R = PR;
 bugReports.push_back(PR);
   }
@@ -2824,8 +2822,8 @@ BugReport *PathSensitiveBugReporter::fin
   // stack for very long paths.
   BugReport *exampleReport = nullptr;
 
-  for (; I != E; ++I) {
-auto *R = dyn_cast(&*I);
+  for (const auto : EQ.getReports()) {
+auto *R = dyn_cast(I.get());
 if (!R)
   continue;
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=371451=371450=371451=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Mon Sep  9 13:34:44 2019
@@ -3004,8 +3004,8 @@ struct DOTGraphTraits :
 llvm::make_range(BR.EQClasses_begin(), BR.EQClasses_end());
 
 for (const auto  : EQClasses) {
-  for (const BugReport  : EQ) {
-const auto *PR = dyn_cast();
+  for (const auto  : EQ.getReports()) {
+const auto *PR = dyn_cast(I.get());
 if (!PR)
   continue;
 const ExplodedNode *EN = PR->getErrorNode();

[PATCH] D62731: [RFC] Add support for options -frounding-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

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



Comment at: clang/docs/UsersManual.rst:1314
+   ``-ffp-model=strict``, or merely establish the rounding mode setting
+   parameter to the llvm floating point constrained intrinsics.
+

rjmccall wrote:
> What you should document here are the semantics and how the option interacts 
> with other options, not how code gets translated into LLVM.  I'm not sure 
> what the FIXME question here is; are you asking whether providing 
> `-frounding-math` should *imply* an FP model?
> 
> The notes about each of the options should probably be structured into a 
> bullet list.
I'll remove the FIXME and assert that frounding-math uses dynamic-rounding and 
strict exception behavior. This will make frounding-math synonymous with 
fp-model=strict.  I'll reformat to put notes into bullet lists.



Comment at: clang/docs/UsersManual.rst:1336
+   enables ``-fp-contract=fast``, and conflicts with: ``-fp-contract=on``,
+   ``-fp-contract=off``.
+

rjmccall wrote:
> This is basically incomprehensible. :) I don't know if the problem is the 
> behavior or just how it's being described, but I have no idea what "conflict" 
> means — does it mean the option gets overridden, ignored, or causes an error? 
>  I think what you're trying to say is:
> 
> - Basic FP behavior can be broken down along two dimensions: the FP 
> strictness model and the FP exceptions model.
> - There are many existing options for controlling FP behavior.
> - Some of these existing options are equivalent to setting one (or both?) of 
> these dimensions.  These options should generally be treated as synonyms for 
> the purposes of deciding the ultimate setting; for example, `-ffp-model=fast 
> -fno-fast-math` should basically leave the setting in its default state 
> (right?).
> - Other existing options only make sense in combination with certain basic 
> models.  For example, `-ffp-contract=fast` (note the spelling) is only 
> allowed when using the fast FP model (right?).
> 
> As a specific note, you break out the options into a list below; the entry 
> for `fast` is the place to add things like "Equivalent to `-ffast-math`, 
> including defining `__FAST_MATH__`)".
Conflict was a poor choice of words. I meant to say that the umbrella options 
like fp-model=strict overlap with some of the other floating-point settings, in 
that case the rightmost option takes precedence and overrides the setting.  I 
want the new options to behave in the same way that other clang options: 
rightmost option has precedence.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



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

zsrkmyn wrote:
> zsrkmyn wrote:
> > erichkeane wrote:
> > > zsrkmyn wrote:
> > > > erichkeane wrote:
> > > > > This Resolver should have the same linkage as below.
> > > > Actually, I wanted to set linkage here at the first time, but failed. 
> > > > When compiling code with cpu_specific but no cpu_dispatch, we cannot 
> > > > set it as LinkOnceODR or WeakODR. E.g.:
> > > > 
> > > > ```
> > > > $ cat specific_only.c
> > > > __declspec(cpu_specific(pentium_iii))
> > > > int foo(void) { return 0; }
> > > > int usage() { return foo(); }
> > > > 
> > > > $ clang -fdeclspec specific_only.c  
> > > >
> > > > Global is external, but doesn't have external or weak linkage!  
> > > >   
> > > > i32 ()* ()* @foo.resolver   
> > > >   
> > > > fatal error: error in backend: Broken module found, compilation 
> > > > aborted!   
> > > > ```
> > > > 
> > > > This is found by lit test test/CodeGen/attr-cpuspecific.c, in which 
> > > > 'SingleVersion()' doesn't have a cpu_dispatch declaration.
> > > The crash message is complaining it isn't external/weak.  However, 
> > > WeakODR should count, right?  Can you look into it a bit more to see what 
> > > it thinks is broken?
> > No, actually I've tried it earlier with the example I mentioned in my last 
> > comment, but WeakODR still makes compiler complaining. I think it's 
> > `foo.resolver` that cannot be declared with as WeakODR/LinkOnceODR without 
> > definition. But I'm really not familiar with these rules.
> According to the `Verifier::visitGlobalValue()` in Verify.cpp, an declaration 
> can only be `ExternalLinkage` or `ExternalWeakLinkage`. So I still believe we 
> cannot set the resolver to `LinkOnceODRLinkage/WeakODRLinkage` here, as they 
> are declared but not defined when we only have `cpu_specified` but no 
> `cpu_dispatch` in a TU as the example above.
That doesn't seem right then.  IF it allows ExternalWeakLinkage I'd expect 
WeakODR to work as well, since it is essentially the same thing.


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

https://reviews.llvm.org/D67058



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

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

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:192
+  Intent intent() const override { return Intent::Refactor; }
+  std::string title() const override { return "Inline function definition"; }
+

sammccall wrote:
> kadircet wrote:
> > arphaman wrote:
> > > Sorry, I'm not really familiar with design of tweaks, so this might be a 
> > > bad question: Is it possible to change the title of the tweak on a per-TU 
> > > basis. More specifically, some tweaks might want to have different titles 
> > > for Objective-C actions compared to their C++ siblings.
> > Current design returns a constant string per tweak, but we can change that 
> > behaviour easily by making this a function of language options. You can 
> > take a look at `ClangdServer::enumerateTweaks` for details.
> This isn't quite true: id() must be constant but title() need not be. See 
> ExpandMacro for an example.
> 
> The only constraint is it shouldn't be terribly expensive (this is why 
> ExpandAuto doesn't include the type in the title, due to an AST bug it 
> requires a new traversal).
> 
> > More specifically, some tweaks might want to have different titles for 
> > Objective-C actions compared to their C++ siblings.
> I think this is possible and the right idea. But for completeness I should 
> mention: having multiple Tweak subclasses that share implementation is also 
> possible. In particular in cases where you have two related actions and may 
> want to provide *both* as options (maybe extract function vs method?), they'd 
> need to be separate subclasses. (Or we'd need to change the design a bit)
Oh yes, I think it's probably better to use the `Tweak` subclasses like you 
said. This will be better for ObjC++, which could have either a C++ or an ObjC 
tweak available depending on the current AST selection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65433



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

Re: r371437 - [Driver] Add -static-openmp driver option

[Pirama Arumuga Nainar via cfe-commits]

Hi Joerg,

Lines 37, 58, 71 in the test checks this interaction, for instance:
// RUN: %clang -target x86_64-linux-gnu -fopenmp=libiomp5 -static
-static-openmp %s -o %t -### 2>&1 | FileCheck %s
--check-prefix=CHECK-LD-STATIC-IOMP5-NO-BDYNAMIC

Are you asking for a different test instead?

On Mon, Sep 9, 2019 at 1:03 PM Joerg Sonnenberger  wrote:

> On Mon, Sep 09, 2019 at 06:31:41PM -, Pirama Arumuga Nainar via
> cfe-commits wrote:
> > Author: pirama
> > Date: Mon Sep  9 11:31:41 2019
> > New Revision: 371437
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=371437=rev
> > Log:
> > [Driver] Add -static-openmp driver option
>
> This still needs testing for the interaction with -static.
>
> Joerg
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D67122#1663619 , @vsk wrote:

> > In D67122#1659721 , @vsk wrote:
> > 
> >> Still think this looks good. Have you tried running this on the llvm test 
> >> suite, or some other interesting corpus? Would be curious to see any 
> >> pre/post patch numbers.
> > 
> > 
> > There were 1.5 occurrences in test-suite.
> > 
> > I'd prefer not to try running it over llvm stage-2/3 (i can, but it's gonna 
> > be slow..)
> > 
> > Though i guess you were talking about *performance* numbers?
>
> Yes, I'm primarily interested in run-time and compile-time performance (at 
> {-O0 -g, -Os -g}).


Uh, feel free to measure _that_ yourself :D

> The number of occurrences of new diagnostics is also interesting. What does 
> the .5 mean?

There were two occurrences - rL371219  and 
rL371220 , and only one of those is new.
(as per @rupprecht it catches something in the original miscompile test, so 
there's that too)

>> I'm not good with using test-suite for perf, so i'm yet again going to use 
>> my own bench.
>>  F9934220: rawspeed-pointer-overflow-0-baseline.json 
>>  F9934221: 
>> rawspeed-pointer-overflow-1-old.json  
>> F9934222: rawspeed-pointer-overflow-2-new.json 
>> 
> 
> I see, I'll make some time to collect & share stats from it soon.

Cool

> On that note, I've never had any problems with running the suite in the past,
>  and the results have been very helpful (if nothing else,
>  it's nice to have a mid-size test corpus everyone can share).
>  What sorts of issues are you running into?
>  Were you able to get interesting results with -DTEST_SUITE_RUN_BENCHMARKS=0 
> set?

Note that i wasn't looking for compile time stats, let alone at -O0,
there is nothing odd in the diff that could have an impact other than the usual 
"death by a thousand cuts".

As for benchmarking, it has pretty long run-time,
multiply that by the number of repetitions, and the results are noisy still.

>> TLDR: (all measurements done with llvm ToT, the sanitizer never fired.)
>> 
>> - no sanitization vs. existing check: average `+20.75%` slowdown
>> - existing check vs. check after this patch: average `+26.36%` slowdown
>> - no sanitization vs. this patch: average `+52.58%` slowdown

^ so i provide my own numbers.

I'm chirping away at missed folds, already handled two, two more coming,
so i suspect runtime overhead is already lower and will be lower still.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67122



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

Re: r371437 - [Driver] Add -static-openmp driver option

[Joerg Sonnenberger via cfe-commits]

On Mon, Sep 09, 2019 at 06:31:41PM -, Pirama Arumuga Nainar via cfe-commits 
wrote:
> Author: pirama
> Date: Mon Sep  9 11:31:41 2019
> New Revision: 371437
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=371437=rev
> Log:
> [Driver] Add -static-openmp driver option

This still needs testing for the interaction with -static.

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

[PATCH] D66796: [clang] Loop pragma vectorize(disable)

[Hideki Saito via Phabricator via cfe-commits]

hsaito added a comment.

There are two ways to think.

1. vectorize(disable)   as in disable the LoopVectorize pass itself.
2. vectorize(disable)   as in disabling the loop vectorization transformation

It all depends on to whom we are providing these pragmas.

If we are providing pragmas for programmers, they don't care LoopVectorize or 
something else. They care about vectorization of loop. So, 2) makes more sense.

If we are providing pragmas for compiler writers, we should be making this more 
like

  #pragma clang loop LoopVectorize(disable)

so that what is controlling what crisply known to everyone.

For anything that is programmer visible, I think it's time for LLVM vectorizer 
to focus on how the programmers use rather than how vectorizer writers think 
about things.


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

https://reviews.llvm.org/D66796



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

[PATCH] D67364: [Driver] Handle default case in refactored addOpenMPRuntime

[Pirama Arumuga Nainar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371444: [Driver] Handle default case in refactored 
addOpenMPRuntime (authored by pirama, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67364?vs=219413=219415#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67364

Files:
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp


Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -525,6 +525,8 @@
   case Driver::OMPRT_IOMP5:
 CmdArgs.push_back("-liomp5");
 break;
+  case Driver::OMPRT_Unknown:
+break;
   }
 
   if (ForceStaticHostRuntime)


Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -525,6 +525,8 @@
   case Driver::OMPRT_IOMP5:
 CmdArgs.push_back("-liomp5");
 break;
+  case Driver::OMPRT_Unknown:
+break;
   }
 
   if (ForceStaticHostRuntime)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

> In D67122#1659721 , @vsk wrote:
> 
>> Still think this looks good. Have you tried running this on the llvm test 
>> suite, or some other interesting corpus? Would be curious to see any 
>> pre/post patch numbers.
> 
> 
> There were 1.5 occurrences in test-suite.
> 
> I'd prefer not to try running it over llvm stage-2/3 (i can, but it's gonna 
> be slow..)
> 
> Though i guess you were talking about *performance* numbers?

Yes, I'm primarily interested in run-time and compile-time performance (at {-O0 
-g, -Os -g}). The number of occurrences of new diagnostics is also interesting. 
What does the .5 mean?

> I'm not good with using test-suite for perf, so i'm yet again going to use my 
> own bench.
>  F9934220: rawspeed-pointer-overflow-0-baseline.json 
>  F9934221: 
> rawspeed-pointer-overflow-1-old.json  
> F9934222: rawspeed-pointer-overflow-2-new.json 
> 

I see, I'll make some time to collect & share stats from it soon. On that note, 
I've never had any problems with running the suite in the past, and the results 
have been very helpful (if nothing else, it's nice to have a mid-size test 
corpus everyone can share). What sorts of issues are you running into? Were you 
able to get interesting results with -DTEST_SUITE_RUN_BENCHMARKS=0 set?

> TLDR: (all measurements done with llvm ToT, the sanitizer never fired.)
> 
> - no sanitization vs. existing check: average `+20.75%` slowdown
> - existing check vs. check after this patch: average `+26.36%` slowdown
> - no sanitization vs. this patch: average `+52.58%` slowdown
> 
>   I suspect some of this might be recoverable, but i don't know yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67122



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

r371444 - [Driver] Handle default case in refactored addOpenMPRuntime

[Pirama Arumuga Nainar via cfe-commits]

Author: pirama
Date: Mon Sep  9 12:52:39 2019
New Revision: 371444

URL: http://llvm.org/viewvc/llvm-project?rev=371444=rev
Log:
[Driver] Handle default case in refactored addOpenMPRuntime

Summary:
Appease failed builds (due to -Werror and -Wswitch) where OMPRT_Unknown
is not handled in the switch statement (even though it's handled by the
early exit).

This fixes -Wswitch triggered by r371442.

Reviewers: srhines, danalbert, jdoerfert

Subscribers: guansong, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=371444=371443=371444=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Mon Sep  9 12:52:39 2019
@@ -525,6 +525,8 @@ bool tools::addOpenMPRuntime(ArgStringLi
   case Driver::OMPRT_IOMP5:
 CmdArgs.push_back("-liomp5");
 break;
+  case Driver::OMPRT_Unknown:
+break;
   }
 
   if (ForceStaticHostRuntime)


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

[PATCH] D67364: [Driver] Handle default case in refactored addOpenMPRuntime

[Pirama Arumuga Nainar via Phabricator via cfe-commits]

pirama updated this revision to Diff 219413.
pirama added a comment.

Check for OMPRT_Unknown instead of a default case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67364

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


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -525,6 +525,8 @@
   case Driver::OMPRT_IOMP5:
 CmdArgs.push_back("-liomp5");
 break;
+  case Driver::OMPRT_Unknown:
+break;
   }
 
   if (ForceStaticHostRuntime)


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -525,6 +525,8 @@
   case Driver::OMPRT_IOMP5:
 CmdArgs.push_back("-liomp5");
 break;
+  case Driver::OMPRT_Unknown:
+break;
   }
 
   if (ForceStaticHostRuntime)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67364: [Driver] Handle default case in refactored addOpenMPRuntime

[Pirama Arumuga Nainar via Phabricator via cfe-commits]

pirama created this revision.
pirama added reviewers: srhines, danalbert.
Herald added a subscriber: guansong.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

Appease failed builds (due to -Werror and -Wswitch) where OMPRT_Unknown
is not handled in the switch statement (even though it's handled by the
early exit).

This fixes -Wswitch triggered by r371442.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67364

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


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -525,6 +525,8 @@
   case Driver::OMPRT_IOMP5:
 CmdArgs.push_back("-liomp5");
 break;
+  default:
+break;
   }
 
   if (ForceStaticHostRuntime)


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -525,6 +525,8 @@
   case Driver::OMPRT_IOMP5:
 CmdArgs.push_back("-liomp5");
 break;
+  default:
+break;
   }
 
   if (ForceStaticHostRuntime)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67321: Respect CLANG_LINK_CLANG_DYLIB=ON in libclang and c-index-test

[Chris Bieneman via Phabricator via cfe-commits]

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



Comment at: clang/tools/libclang/CMakeLists.txt:115
+clang_target_link_libraries(libclang
+  PRIVATE
+  ${CLANG_LIB_DEPS}

tstellar wrote:
> aaronpuchert wrote:
> > This might not be correct for static builds, I think we need `INTERFACE` 
> > here.
> This patch looks OK to me, but you should find someone with more CMake 
> knowledge to answer this question.
This part of the patch is a bit tricky.

As implemented it is fine for the most common build configurations, but will be 
a problem if `LIBCLANG_BUILD_STATIC=On` or if `LLVM_ENABLE_PIC=Off`.

The correct solution is probably to wrap this code in `if (ENABLE_SHARED)`, and 
to have another code block that handles `if (ENABLE_STATIC)`. In that block you 
need to call this with `INTERFACE` as the linkage type, and you'll need to 
handle the case where both `ENABLE_SHARED` and `ENABLE_STATIC` is set. In that 
case the static library target is named `libclang_static`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67321



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Paul Kirth via Phabricator via cfe-commits]

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



Comment at: clang/test/Profile/misexpect-branch-cold.c:4
+// RUN: llvm-profdata merge %S/Inputs/misexpect-branch.proftext -o %t.profdata
+// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm 
-fprofile-instrument-use-path=%t.profdata -verify -Wmisexpect
+

lebedev.ri wrote:
> paulkirth wrote:
> > lebedev.ri wrote:
> > > Is there a test where `-Wmisexpect` isn't present, to verify that it is 
> > > off-by-default?
> > We can add one, but is that necessary? Don't the tests for diagnostics 
> > cover those already?
> To clarify: i'm interested in the case where the PGO data is provided but 
> `-Wmisexpect` is *not* specified.
Actually, this is moot. That test already exists in misexpect-branch.c: line 6.

Since that file actually has a mismatched use of __builtin_expect(), it's a 
better candidate anyway.

```// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm 
-fprofile-instrument-use-path=%t.profdata -verify=foo```


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

https://reviews.llvm.org/D66324



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

[PATCH] D67200: Add -static-openmp driver option

[Pirama Arumuga Nainar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371437: [Driver] Add -static-openmp driver option (authored 
by pirama, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67200?vs=219200=219401#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67200

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
  cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
  cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
  cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp
  cfe/trunk/test/Driver/fopenmp.c

Index: cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp
+++ cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp
@@ -289,7 +289,11 @@
   }
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
-addOpenMPRuntime(CmdArgs, getToolChain(), Args);
+// Use the static OpenMP runtime with -static-openmp
+bool StaticOpenMP = Args.hasArg(options::OPT_static_openmp) &&
+!Args.hasArg(options::OPT_static);
+addOpenMPRuntime(CmdArgs, getToolChain(), Args, StaticOpenMP);
+
 if (D.CCCIsCXX()) {
   if (ToolChain.ShouldLinkCXXStdlib(Args))
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
Index: cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
+++ cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
@@ -270,7 +270,11 @@
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
-addOpenMPRuntime(CmdArgs, ToolChain, Args);
+// Use the static OpenMP runtime with -static-openmp
+bool StaticOpenMP = Args.hasArg(options::OPT_static_openmp) &&
+!Args.hasArg(options::OPT_static);
+addOpenMPRuntime(CmdArgs, ToolChain, Args, StaticOpenMP);
+
 if (D.CCCIsCXX()) {
   if (ToolChain.ShouldLinkCXXStdlib(Args))
 ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -500,30 +500,39 @@
 }
 
 bool tools::addOpenMPRuntime(ArgStringList , const ToolChain ,
- const ArgList , bool IsOffloadingHost,
- bool GompNeedsRT) {
+ const ArgList , bool ForceStaticHostRuntime,
+ bool IsOffloadingHost, bool GompNeedsRT) {
   if (!Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
 options::OPT_fno_openmp, false))
 return false;
 
-  switch (TC.getDriver().getOpenMPRuntime(Args)) {
+  Driver::OpenMPRuntimeKind RTKind = TC.getDriver().getOpenMPRuntime(Args);
+
+  if (RTKind == Driver::OMPRT_Unknown)
+// Already diagnosed.
+return false;
+
+  if (ForceStaticHostRuntime)
+CmdArgs.push_back("-Bstatic");
+
+  switch (RTKind) {
   case Driver::OMPRT_OMP:
 CmdArgs.push_back("-lomp");
 break;
   case Driver::OMPRT_GOMP:
 CmdArgs.push_back("-lgomp");
-
-if (GompNeedsRT)
-  CmdArgs.push_back("-lrt");
 break;
   case Driver::OMPRT_IOMP5:
 CmdArgs.push_back("-liomp5");
 break;
-  case Driver::OMPRT_Unknown:
-// Already diagnosed.
-return false;
   }
 
+  if (ForceStaticHostRuntime)
+CmdArgs.push_back("-Bdynamic");
+
+  if (RTKind == Driver::OMPRT_GOMP && GompNeedsRT)
+  CmdArgs.push_back("-lrt");
+
   if (IsOffloadingHost)
 CmdArgs.push_back("-lomptarget");
 
Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
@@ -84,6 +84,7 @@
 /// Returns true, if an OpenMP runtime has been added.
 bool addOpenMPRuntime(llvm::opt::ArgStringList , const ToolChain ,
   const llvm::opt::ArgList ,
+  bool ForceStaticHostRuntime = false,
   bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
 llvm::opt::Arg *getLastProfileUseArg(const llvm::opt::ArgList );
Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
@@ -555,9 +555,13 @@
   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
 
+  // Use the static OpenMP runtime with -static-openmp
+  bool StaticOpenMP 

r371437 - [Driver] Add -static-openmp driver option

[Pirama Arumuga Nainar via cfe-commits]

Author: pirama
Date: Mon Sep  9 11:31:41 2019
New Revision: 371437

URL: http://llvm.org/viewvc/llvm-project?rev=371437=rev
Log:
[Driver] Add -static-openmp driver option

Summary:
For Gnu, FreeBSD and NetBSD, this option forces linking with the static
OpenMP host runtime (similar to -static-libgcc and -static-libstdcxx).

Android's NDK will start the shared OpenMP runtime in addition to the static
libomp.  In this scenario, the linker will prefer to use the shared library by
default.  Add this option to enable linking with the static libomp.

Reviewers: Hahnfeld, danalbert, srhines, joerg, jdoerfert

Subscribers: guansong, cfe-commits

Tags: #clang

Fixes https://github.com/android-ndk/ndk/issues/1028

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

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp
cfe/trunk/test/Driver/fopenmp.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=371437=371436=371437=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Sep  9 11:31:41 2019
@@ -1616,6 +1616,8 @@ def fopenmp_optimistic_collapse : Flag<[
   Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 def fno_openmp_optimistic_collapse : Flag<["-"], 
"fno-openmp-optimistic-collapse">, Group,
   Flags<[NoArgumentUnused, HelpHidden]>;
+def static_openmp: Flag<["-"], "static-openmp">,
+  HelpText<"Use the static host OpenMP runtime while linking.">;
 def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, 
Group;
 def foptimize_sibling_calls : Flag<["-"], "foptimize-sibling-calls">, 
Group;
 def fno_escaping_block_tail_calls : Flag<["-"], 
"fno-escaping-block-tail-calls">, Group, Flags<[CC1Option]>;

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=371437=371436=371437=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Mon Sep  9 11:31:41 2019
@@ -500,30 +500,39 @@ void tools::addArchSpecificRPath(const T
 }
 
 bool tools::addOpenMPRuntime(ArgStringList , const ToolChain ,
- const ArgList , bool IsOffloadingHost,
- bool GompNeedsRT) {
+ const ArgList , bool ForceStaticHostRuntime,
+ bool IsOffloadingHost, bool GompNeedsRT) {
   if (!Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
 options::OPT_fno_openmp, false))
 return false;
 
-  switch (TC.getDriver().getOpenMPRuntime(Args)) {
+  Driver::OpenMPRuntimeKind RTKind = TC.getDriver().getOpenMPRuntime(Args);
+
+  if (RTKind == Driver::OMPRT_Unknown)
+// Already diagnosed.
+return false;
+
+  if (ForceStaticHostRuntime)
+CmdArgs.push_back("-Bstatic");
+
+  switch (RTKind) {
   case Driver::OMPRT_OMP:
 CmdArgs.push_back("-lomp");
 break;
   case Driver::OMPRT_GOMP:
 CmdArgs.push_back("-lgomp");
-
-if (GompNeedsRT)
-  CmdArgs.push_back("-lrt");
 break;
   case Driver::OMPRT_IOMP5:
 CmdArgs.push_back("-liomp5");
 break;
-  case Driver::OMPRT_Unknown:
-// Already diagnosed.
-return false;
   }
 
+  if (ForceStaticHostRuntime)
+CmdArgs.push_back("-Bdynamic");
+
+  if (RTKind == Driver::OMPRT_GOMP && GompNeedsRT)
+  CmdArgs.push_back("-lrt");
+
   if (IsOffloadingHost)
 CmdArgs.push_back("-lomptarget");
 

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.h?rev=371437=371436=371437=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.h Mon Sep  9 11:31:41 2019
@@ -84,6 +84,7 @@ void addArchSpecificRPath(const ToolChai
 /// Returns true, if an OpenMP runtime has been added.
 bool addOpenMPRuntime(llvm::opt::ArgStringList , const ToolChain ,
   const llvm::opt::ArgList ,
+  bool ForceStaticHostRuntime = false,
   bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
 llvm::opt::Arg *getLastProfileUseArg(const llvm::opt::ArgList );

Modified: cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp?rev=371437=371436=371437=diff

[PATCH] D67358: [clangd] Implement semantic selections.

[Sam McCall via Phabricator via cfe-commits]

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

Nice! Particularly: great tests.

Only big thing is toHalfOpenFileRange should get you substantially better macro 
handling.




Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:18
+namespace {
+void addIfUnique(const Range , SemanticSelectionResult *Result) {
+  if (Result->Ranges.empty() || Result->Ranges.back() != R) {

nit: "unique" suggests this function checks against all the ranges in *Result 
(which it doesn't, and indeed doesn't need to)
I'd suggest renaming `addIfDistinct` and adding a comment that only consecutive 
ranges should be able to coincide



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:33
+  if (!Offset) {
+llvm::errs() << "Unable to convert postion to offset";
+return {};

we use log() or elog() for this, make sure you include Offset.takeError() for 
the original message

Alternatively, you might consider returning Expected 
so the error gets propagated to the client (as this really is a client error)



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:39
+  SelectionTree ST(AST.getASTContext(), AST.getTokens(), *Offset);
+  for (const auto *Node = ST.commonAncestor(); Node != nullptr;
+   Node = Node->Parent) {

I think you probably want to bail out once you hit TranslationUnitDecl.

And maybe at namespace decls? Though I have often in my editor wondered "what's 
the enclosing namespace"...



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:45
+}
+SourceLocation BeginLoc = SR.getBegin();
+SourceLocation EndLoc =

Ah, this will work if the start and the end of the AST node are written 
directly in the file, and no macros are involved. But try `toHalfOpenFileRange` 
in `SourceCode.h`, which should work in a lot more cases.

In particular, this should work:
```
#define INC(X) X + 1
int a, b;
int c = INC(a * b);
^
~
~
~~
~~
```



Comment at: clang-tools-extra/clangd/SemanticSelection.h:21
+
+struct SemanticSelectionResult {
+  // The list of interesting selections around the cursor. 

I'm not sure this struct pulls its weight, I'd suggest just returning 
vector here.
This isn't a stable API, we can change it if we need to add things.
Or is there something you have in mind?

Often we mirror the LSP here, but LSP is weird enough here (a linked list, 
seriously?) that I don't particularly think we should.



Comment at: clang-tools-extra/clangd/SemanticSelection.h:27
+// Returns the list of all interesting ranges around the Position \p Pos. 
+// Constructs the 
+// Any range in the result strictly contains all the previous ranges in the 
result.

incomplete sentence. It'd be nice to spell out on this line what an 
"interesting" range is (just one that corresponds to an AST node, right?)



Comment at: clang-tools-extra/clangd/SemanticSelection.h:28
+// Constructs the 
+// Any range in the result strictly contains all the previous ranges in the 
result.
+SemanticSelectionResult getSemanticRanges(ParsedAST , Position Pos);

somewhere you should mention that front() is the inner most range, and back() 
the outermost.



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:53
+   )cpp",
+  R"cpp( // Cursor inside a macro.
+#define HASH(x) ((x) % 10)

this is the case where `toHalfOpenFileName` should help



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:59
+   )cpp",
+  R"cpp( // Cursor on a macro.
+#define HASH(x) ((x) % 10)

interesting, this *might* be worth special handling, to select the whole macro 
expansion. Wonder if people will complain. Let's leave it for now for simplicity



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:82
+  )cpp",
+  // FIXME: No node found associated to the position.
+  R"cpp( // Cursor at end of VarDecl.

If you're interested, I think the fix here is in `pointBounds()` in 
selection.cpp.
Since selection-tree now treats semicolons as if they don't exist, we should 
probably handle them the same way we do whitespace, and look left instead of 
right.

(definitely a different patch though)



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:88
+  )cpp",
+  // FIXME: No node found associated to the position.
+  R"cpp( // Cursor at end of VarDecl.

oh, this is interesting. it's a side-effect of how selectiontree now ignores 
whitespace (and semicolons, and comments).

We convert a cursor into a size-one selection, and that used to always select 
something, 

r371430 - [X86] Allow _MM_FROUND_CUR_DIRECTION and _MM_FROUND_NO_EXC to be used together on instructions that only support SAE and not embedded rounding.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Mon Sep  9 10:48:05 2019
New Revision: 371430

URL: http://llvm.org/viewvc/llvm-project?rev=371430=rev
Log:
[X86] Allow _MM_FROUND_CUR_DIRECTION and _MM_FROUND_NO_EXC to be used together 
on instructions that only support SAE and not embedded rounding.

Current for SAE instructions we only allow _MM_FROUND_CUR_DIRECTION(bit 2) or 
_MM_FROUND_NO_EXC(bit 3) to be used as the immediate passed to the inrinsics. 
But these instructions don't perform rounding so _MM_FROUND_CUR_DIRECTION is 
just sort of a default placeholder when you don't want to suppress exceptions. 
Using _MM_FROUND_NO_EXC by itself is really bit equivalent to 
(_MM_FROUND_NO_EXC | _MM_FROUND_TO_NEAREST_INT) since _MM_FROUND_TO_NEAREST_INT 
is 0. Since we aren't rounding on these instructions we should also accept 
(_MM_FROUND_CUR_DIRECTION | _MM_FROUND_NO_EXC) as equivalent to 
(_MM_FROUND_NO_EXC). icc allows this, but gcc does not.

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

Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/builtins-x86.c

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=371430=371429=371430=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Sep  9 10:48:05 2019
@@ -3546,9 +3546,11 @@ bool Sema::CheckX86BuiltinRoundingOrSAE(
 
   // Make sure rounding mode is either ROUND_CUR_DIRECTION or ROUND_NO_EXC bit
   // is set. If the intrinsic has rounding control(bits 1:0), make sure its 
only
-  // combined with ROUND_NO_EXC.
+  // combined with ROUND_NO_EXC. If the intrinsic does not have rounding
+  // control, allow ROUND_NO_EXC and ROUND_CUR_DIRECTION together.
   if (Result == 4/*ROUND_CUR_DIRECTION*/ ||
   Result == 8/*ROUND_NO_EXC*/ ||
+  (!HasRC && Result == 12/*ROUND_CUR_DIRECTION|ROUND_NO_EXC*/) ||
   (HasRC && Result.getZExtValue() >= 8 && Result.getZExtValue() <= 11))
 return false;
 

Modified: cfe/trunk/test/Sema/builtins-x86.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtins-x86.c?rev=371430=371429=371430=diff
==
--- cfe/trunk/test/Sema/builtins-x86.c (original)
+++ cfe/trunk/test/Sema/builtins-x86.c Mon Sep  9 10:48:05 2019
@@ -81,6 +81,19 @@ __mmask16 test__builtin_ia32_cmpps512_ma
   return __builtin_ia32_cmpps512_mask(__a, __b, 0, __u, 0); // expected-error 
{{invalid rounding argument}}
 }
 
+// Make sure we allow 4(CUR_DIRECTION), 8(NO_EXC), and 12(CUR_DIRECTION|NOEXC) 
for SAE arguments.
+__mmask16 test__builtin_ia32_cmpps512_mask_rounding_cur_dir(__m512 __a, __m512 
__b, __mmask16 __u) {
+  return __builtin_ia32_cmpps512_mask(__a, __b, 0, __u, 4); // no-error
+}
+
+__mmask16 test__builtin_ia32_cmpps512_mask_rounding_sae1(__m512 __a, __m512 
__b, __mmask16 __u) {
+  return __builtin_ia32_cmpps512_mask(__a, __b, 0, __u, 8); // no-error
+}
+
+__mmask16 test__builtin_ia32_cmpps512_mask_rounding_sae2(__m512 __a, __m512 
__b, __mmask16 __u) {
+  return __builtin_ia32_cmpps512_mask(__a, __b, 0, __u, 12); // no-error
+}
+
 __m512 test__builtin_ia32_getmantps512_mask(__m512 a, __m512 b) {
   return __builtin_ia32_getmantps512_mask(a, 0, b, (__mmask16)-1, 10); // 
expected-error {{invalid rounding argument}}
 }


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

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h:24
+  DynamicTypeInfo(QualType ty, bool CanBeSub = true)
+  : Ty(ty), CanBeASubClass(CanBeSub) {}
 

NoQ wrote:
> `Ty(Ty)` is the idiom here.
Good to know, thanks!



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:126
+
+  // If the casts have a common anchestor it could not be a succeeded downcast.
+  for (const auto  : PreviousRD->bases())

NoQ wrote:
> Counterexample:
> 
> ```
> struct A {};
> struct B : A {};
> struct C : A, B {};
> ```
> 
> Downcast from `C` to `B` should succeed, even though they have a common 
> ancestor `A` (which has the same `CXXRecordDecl` but currently isn't the same 
> object within `C`, but can be, if `B` declares `A` as a virtual base).
So, even it is some kind of anti-pattern as a warning arrive immediately, now I 
allow `B` to `C` downcasts. Could you explain me more about that virtual idea, 
please? Based on this possible use-case in my mind two classes are on the same 
level as every of their bases/vbases are equals.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:174
+
+constexpr llvm::StringLiteral Vowels = "aeiou";
+

NoQ wrote:
> Omg lol nice. Did you try to figure out how do other people normally do it?
There is no function for that in `ADT/StringExtras.h` + `grep` did not help, so 
I realized it is a common way to match vowels. Do you know a better solution?


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

https://reviews.llvm.org/D67079



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

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 219391.
Charusso marked 5 inline comments as done.
Charusso added a comment.

- Fix.


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

https://reviews.llvm.org/D67079

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Core/DynamicType.cpp
  clang/test/Analysis/cast-value-hierarchy.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -38,7 +38,7 @@
 // CHECK-NEXT: { "symbol": "reg_$0", "range": "{ [-2147483648, 13] }" }
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "dynamic_types": null,
-// CHECK-NEXT:   "dynamic_casts": null,
+// CHECK-NEXT:   "failed_casts": null,
 // CHECK-NEXT:   "constructing_objects": null,
 // CHECK-NEXT:   "checker_messages": null
 // CHECK-NEXT: }
Index: clang/test/Analysis/cast-value-state-dump.cpp
===
--- clang/test/Analysis/cast-value-state-dump.cpp
+++ clang/test/Analysis/cast-value-state-dump.cpp
@@ -11,19 +11,31 @@
 class Triangle : public Shape {};
 class Circle : public Shape {};
 class Square : public Shape {};
+class Octagram : public Shape {};
 } // namespace clang
 
 using namespace llvm;
 using namespace clang;
 
 void evalNonNullParamNonNullReturn(const Shape *S) {
+  if (isa(S)) {
+// expected-note@-1 {{Assuming 'S' is not a 'Triangle'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
+  if (isa(S)) {
+// expected-note@-1 {{Assuming 'S' is not an 'Octagram'}}
+// expected-note@-2 {{Taking false branch}}
+return;
+  }
+
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{Assuming 'S' is a 'Circle'}}
   // expected-note@-2 {{'C' initialized here}}
 
-  // FIXME: We assumed that 'S' is a 'Circle' therefore it is not a 'Square'.
   if (dyn_cast_or_null(S)) {
-// expected-note@-1 {{Assuming 'S' is not a 'Square'}}
+// expected-note@-1 {{'S' is not a 'Square'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
@@ -33,11 +45,11 @@
   // CHECK:  "dynamic_types": [
   // CHECK-NEXT:   { "region": "SymRegion{reg_$0}", "dyn_type": "const class clang::Circle", "sub_classable": true }
   // CHECK-NEXT: ],
-  // CHECK-NEXT: "dynamic_casts": [
-  // CHECK:{ "region": "SymRegion{reg_$0}", "casts": [
-  // CHECK-NEXT: { "from": "const struct clang::Shape *", "to": "const class clang::Circle *", "kind": "success" },
-  // CHECK-NEXT: { "from": "const struct clang::Shape *", "to": "const class clang::Square *", "kind": "fail" }
+  // CHECK-NEXT: "failed_casts": [
+  // CHECK-NEXT:   { "region": "SymRegion{reg_$0}", "casts": [
+  // CHECK-NEXT: "const class clang::Square *", "class clang::Triangle *", "class clang::Octagram *"
   // CHECK-NEXT:   ]}
+  // CHECK-NEXT: ],
 
   (void)(1 / !C);
   // expected-note@-1 {{'C' is non-null}}
Index: clang/test/Analysis/cast-value-notes.cpp
===
--- clang/test/Analysis/cast-value-notes.cpp
+++ clang/test/Analysis/cast-value-notes.cpp
@@ -37,12 +37,6 @@
 return;
   }
 
-  if (dyn_cast_or_null(C)) {
-// expected-note@-1 {{Assuming 'C' is not a 'Triangle'}}
-// expected-note@-2 {{Taking false branch}}
-return;
-  }
-
   if (isa(C)) {
 // expected-note@-1 {{'C' is not a 'Triangle'}}
 // expected-note@-2 {{Taking false branch}}
@@ -65,14 +59,8 @@
   // expected-note@-1 {{'S' is a 'Circle'}}
   // expected-note@-2 {{'C' initialized here}}
 
-  if (!isa(C)) {
-// expected-note@-1 {{Assuming 'C' is a 'Triangle'}}
-// expected-note@-2 {{Taking false branch}}
-return;
-  }
-
-  if (!isa(C)) {
-// expected-note@-1 {{'C' is a 'Triangle'}}
+  if (isa(C)) {
+// expected-note@-1 {{'C' is not a 'Triangle'}}
 // expected-note@-2 {{Taking false branch}}
 return;
   }
Index: clang/test/Analysis/cast-value-hierarchy.cpp
===
--- /dev/null
+++ clang/test/Analysis/cast-value-hierarchy.cpp
@@ -0,0 +1,61 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
+// RUN:  -verify %s
+
+#include "Inputs/llvm.h"
+
+using namespace llvm;
+
+void clang_analyzer_numTimesReached();
+void clang_analyzer_printState();
+void clang_analyzer_warnIfReached();
+void clang_analyzer_eval(bool);
+
+struct A {};// A
+struct B : A {};/// \.
+struct C : A, B {}; //   B-. \.
+struct D : B {};

[PATCH] D62731: [RFC] Add support for options -frounding-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

[John McCall via Phabricator via cfe-commits]

rjmccall added a subscriber: scanon.
rjmccall added a comment.

I think this is a step in the right direction, thank you.  I'd like @scanon to 
weigh in on the evolving design here.




Comment at: clang/docs/UsersManual.rst:1314
+   ``-ffp-model=strict``, or merely establish the rounding mode setting
+   parameter to the llvm floating point constrained intrinsics.
+

What you should document here are the semantics and how the option interacts 
with other options, not how code gets translated into LLVM.  I'm not sure what 
the FIXME question here is; are you asking whether providing `-frounding-math` 
should *imply* an FP model?

The notes about each of the options should probably be structured into a bullet 
list.



Comment at: clang/docs/UsersManual.rst:1336
+   enables ``-fp-contract=fast``, and conflicts with: ``-fp-contract=on``,
+   ``-fp-contract=off``.
+

This is basically incomprehensible. :) I don't know if the problem is the 
behavior or just how it's being described, but I have no idea what "conflict" 
means — does it mean the option gets overridden, ignored, or causes an error?  
I think what you're trying to say is:

- Basic FP behavior can be broken down along two dimensions: the FP strictness 
model and the FP exceptions model.
- There are many existing options for controlling FP behavior.
- Some of these existing options are equivalent to setting one (or both?) of 
these dimensions.  These options should generally be treated as synonyms for 
the purposes of deciding the ultimate setting; for example, `-ffp-model=fast 
-fno-fast-math` should basically leave the setting in its default state 
(right?).
- Other existing options only make sense in combination with certain basic 
models.  For example, `-ffp-contract=fast` (note the spelling) is only allowed 
when using the fast FP model (right?).

As a specific note, you break out the options into a list below; the entry for 
`fast` is the place to add things like "Equivalent to `-ffast-math`, including 
defining `__FAST_MATH__`)".


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D67358: [clangd] Implement semantic selections.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 219386.
usaxena95 added a comment.

Create range only if it represents a valid file range.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67358

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
@@ -0,0 +1,133 @@
+//===-- SemanticSelectionTests.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 "Matchers.h"
+#include "Protocol.h"
+#include "SemanticSelection.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+namespace clang {
+namespace clangd {
+namespace {
+using ::testing::ElementsAreArray;
+
+TEST(SemanticSelection, All) {
+  const char *Tests[] = {
+  R"cpp( // Single statement in a function body.
+[[void func() [[{
+  int v = [[1^00;]]
+}
+  )cpp",
+  R"cpp( // Expression
+[[void func() [[{
+  int a = 1;
+  // int v = (10 + 2) * (a + a);
+  int v = (10^]] + 2]])]] * (a + a);]]
+}
+  )cpp",
+  R"cpp( // Function call.
+int add(int x, int y) { return x + y; }
+[[void callee() [[{
+  // int res = add(11, 22);
+  int res = [[add([[1^1]], 22);]]
+}
+  )cpp",
+  R"cpp( // Tricky macros.
+#define MUL ) * (
+[[void func() [[{
+  // int var = (4 + 15 MUL 6 + 10);
+  int var = [[([[4 + [[1^5 MUL 6 + 10);]]
+}
+   )cpp",
+  R"cpp( // Cursor inside a macro.
+#define HASH(x) ((x) % 10)
+[[void func() [[{
+  int a = HASH(2^3)]];]]
+}
+   )cpp",
+  R"cpp( // Cursor on a macro.
+#define HASH(x) ((x) % 10)
+[[void func() [[{
+  int a = HA^SH(23)]];]]
+}
+   )cpp",
+  R"cpp( // Multiple declaration.
+[[void func() [[{
+  int var1, var^2]], var3;]]
+}
+   )cpp",
+  R"cpp( // Before comment.
+[[void func() [[{
+  int var1 = 1;
+  int var2 = var1]]^ /*some comment*/ + 41;]]
+}
+   )cpp",
+  // Empty file.
+  "^",
+  // FIXME: We should get the whole DeclStmt as a range.
+  R"cpp( // Single statement in TU.
+[[int v = [[1^00;
+  )cpp",
+  // FIXME: No node found associated to the position.
+  R"cpp( // Cursor at end of VarDecl.
+void func() {
+  int v = 100 + 100^;
+}
+  )cpp",
+  // FIXME: No node found associated to the position.
+  R"cpp( // Cursor at end of VarDecl.
+void func() {
+  int v = 100 + ^  100;
+}
+  )cpp",
+  // Structs.
+  R"cpp(
+struct AAA { struct BBB { static int ccc(); };};
+[[void func() [[{
+  // int x = AAA::BBB::ccc();
+  int x = AAA::BBB::c^cc]]();]]
+}
+  )cpp",
+  R"cpp(
+struct AAA { struct BBB { static int ccc(); };};
+[[void func() [[{
+  // int x = AAA::BBB::ccc();
+  int x = [[AA^A]]::]]BBB::]]ccc]]();]]
+}
+  )cpp",
+  // Namespaces.
+  R"cpp( 
+[[namespace nsa { 
+  [[namespace nsb { 
+static int ccc();
+[[void func() [[{
+  // int x = nsa::nsb::ccc();
+  int x = nsa::nsb::cc^c]]();]]
+}
+  }]]
+}]]
+  )cpp",
+
+  };
+
+  for (const char *Test : Tests) {
+auto T = Annotations(Test);
+auto AST = TestTU::withCode(T.code()).build();
+auto Ranges = getSemanticRanges(AST, T.point()).Ranges;
+EXPECT_THAT(Ranges, ElementsAreArray(T.ranges())) << Test;
+  }
+}
+} // namespace
+} // namespace clangd
+} // namespace clang
\ No newline at end of file
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -56,6 +56,7 @@
   

[PATCH] D66796: [clang] Loop pragma vectorize(disable)

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

IMO it is fine to say ` pragma vectorize(disable)` disables the vectorizer 
completely, including interleaving. @Meinersbur, what do you think? I think it 
would be good to make that clear in the commit message.


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

https://reviews.llvm.org/D66796



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

[clang-tools-extra] r371422 - [clangd] Attempt to fix failing Windows buildbots.

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Mon Sep  9 10:03:49 2019
New Revision: 371422

URL: http://llvm.org/viewvc/llvm-project?rev=371422=rev
Log:
[clangd] Attempt to fix failing Windows buildbots.

The assertion is failing on Windows, probably because path separator is 
different.

For the failure see:
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/28072/steps/test/logs/stdio

Modified:
clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp

Modified: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp?rev=371422=371421=371422=diff
==
--- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp Mon Sep  9 
10:03:49 2019
@@ -47,6 +47,7 @@ CanonicalIncludes::mapHeader(llvm::Strin
 
   int Components = 1;
 
+  // FIXME: check that this works on Windows and add tests.
   for (auto It = llvm::sys::path::rbegin(Header),
 End = llvm::sys::path::rend(Header);
It != End && Components <= MaxSuffixComponents; ++It, ++Components) {
@@ -759,12 +760,14 @@ void CanonicalIncludes::addSystemHeaders
   });
   // Check MaxSuffixComponents constant is correct.
   assert(llvm::all_of(SystemHeaderMap->keys(), [](llvm::StringRef Path) {
-return std::distance(llvm::sys::path::begin(Path),
- llvm::sys::path::end(Path)) <= MaxSuffixComponents;
+return std::distance(
+   llvm::sys::path::begin(Path, llvm::sys::path::Style::posix),
+   llvm::sys::path::end(Path)) <= MaxSuffixComponents;
   }));
   // ... and precise.
   assert(llvm::find_if(SystemHeaderMap->keys(), [](llvm::StringRef Path) {
-   return std::distance(llvm::sys::path::begin(Path),
+   return std::distance(llvm::sys::path::begin(
+Path, llvm::sys::path::Style::posix),
 llvm::sys::path::end(Path)) ==
   MaxSuffixComponents;
  }) != SystemHeaderMap->keys().end());


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

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr accepted this revision.
gribozavr added a comment.

I think this patch is a good improvement, and I don't want to hold it back -- 
but like we discussed before, and like you wrote on the mailing list, I would 
want a more simple API for ClangTidy.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:113
+  // encouraged, but the period at the end of the description is still omitted.
+  StringRef getDescription() const { return Description; }
+

Thanks for the doc comments! Please use three slashes here and in 
getShortDescription.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:131
+
+  /// The smallest canonical declaration that contains the bug location.
+  /// This is purely cosmetic; the declaration can be presented to the user

Thanks for the explanation, just one question -- I don't understand what is 
meant by "canonical".

The bug can be found in a non-canonical declaration.

```
void f(); // canonical decl

void f() { // non-canonical decl
  *(int*)0 = 0; // bug
}
```



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:141
+  /// uniqueing location coincides with their location. A different uniqueing
+  /// location is primarily used by various leak warnings: the warning is 
placed
+  /// at the location where the last reference to the leaking resource is

Replace "is primarily used by" with "For example, leak checker that produces a 
different primary and uniqueing location. ..."


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

https://reviews.llvm.org/D66572



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

[PATCH] D67358: Implement semantic selections.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 219381.
usaxena95 added a comment.

Removed logs for debugging.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67358

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
@@ -0,0 +1,133 @@
+//===-- SemanticSelectionTests.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 "Matchers.h"
+#include "Protocol.h"
+#include "SemanticSelection.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+namespace clang {
+namespace clangd {
+namespace {
+using ::testing::ElementsAreArray;
+
+TEST(SemanticSelection, All) {
+  const char *Tests[] = {
+  R"cpp( // Single statement in a function body.
+[[void func() [[{
+  int v = [[1^00;]]
+}
+  )cpp",
+  R"cpp( // Expression
+[[void func() [[{
+  int a = 1;
+  // int v = (10 + 2) * (a + a);
+  int v = (10^]] + 2]])]] * (a + a);]]
+}
+  )cpp",
+  R"cpp( // Function call.
+int add(int x, int y) { return x + y; }
+[[void callee() [[{
+  // int res = add(11, 22);
+  int res = [[add([[1^1]], 22);]]
+}
+  )cpp",
+  R"cpp( // Tricky macros.
+#define MUL ) * (
+[[void func() [[{
+  // int var = (4 + 15 MUL 6 + 10);
+  int var = [[([[4 + [[1^5 MUL 6 + 10);]]
+}
+   )cpp",
+  R"cpp( // Cursor inside a macro.
+#define HASH(x) ((x) % 10)
+[[void func() [[{
+  int a = HASH(2^3)]];]]
+}
+   )cpp",
+  R"cpp( // Cursor on a macro.
+#define HASH(x) ((x) % 10)
+[[void func() [[{
+  int a = HA^SH(23)]];]]
+}
+   )cpp",
+  R"cpp( // Multiple declaration.
+[[void func() [[{
+  int var1, var^2]], var3;]]
+}
+   )cpp",
+  R"cpp( // Before comment.
+[[void func() [[{
+  int var1 = 1;
+  int var2 = var1]]^ /*some comment*/ + 41;]]
+}
+   )cpp",
+  // Empty file.
+  "^",
+  // FIXME: We should get the whole DeclStmt as a range.
+  R"cpp( // Single statement in TU.
+[[int v = [[1^00;
+  )cpp",
+  // FIXME: No node found associated to the position.
+  R"cpp( // Cursor at end of VarDecl.
+void func() {
+  int v = 100 + 100^;
+}
+  )cpp",
+  // FIXME: No node found associated to the position.
+  R"cpp( // Cursor at end of VarDecl.
+void func() {
+  int v = 100 + ^  100;
+}
+  )cpp",
+  // Structs.
+  R"cpp(
+struct AAA { struct BBB { static int ccc(); };};
+[[void func() [[{
+  // int x = AAA::BBB::ccc();
+  int x = AAA::BBB::c^cc]]();]]
+}
+  )cpp",
+  R"cpp(
+struct AAA { struct BBB { static int ccc(); };};
+[[void func() [[{
+  // int x = AAA::BBB::ccc();
+  int x = [[AA^A]]::]]BBB::]]ccc]]();]]
+}
+  )cpp",
+  // Namespaces.
+  R"cpp( 
+[[namespace nsa { 
+  [[namespace nsb { 
+static int ccc();
+[[void func() [[{
+  // int x = nsa::nsb::ccc();
+  int x = nsa::nsb::cc^c]]();]]
+}
+  }]]
+}]]
+  )cpp",
+
+  };
+
+  for (const char *Test : Tests) {
+auto T = Annotations(Test);
+auto AST = TestTU::withCode(T.code()).build();
+auto Ranges = getSemanticRanges(AST, T.point()).Ranges;
+EXPECT_THAT(Ranges, ElementsAreArray(T.ranges())) << Test;
+  }
+}
+} // namespace
+} // namespace clangd
+} // namespace clang
\ No newline at end of file
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -56,6 +56,7 @@
   RIFFTests.cpp
   

[PATCH] D65371: do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D65371#1659929 , @dblaikie wrote:

> A test case would be good (in the clang/test directory - probably near/in the 
> other tests for -frewrite-includes)


Done.

> And does the same bug occur for other preprocessor-related warnings? Maybe 
> it's not practical to disable them all this way & there should be a different 
> solution? (or maybe we shouldn't fix these and users can pass -w to disable 
> warnings when using -frewrite-includes?)

I've never seen any other warning from -frewrite-includes besides 
-Wunused-macros. Given that I use and more or less maintain Icecream, which 
uses -frewrite-includes for distributed builds, I'd say any other warnings 
there either don't exist or are very rare (which rather makes sense, given that 
-frewrite-includes only does limited macro expansion when analysing the input 
and that's about it). Given that, I'd prefer not to disable warnings globally - 
they are unlikely to show up, if they do, they can hopefully be handled 
individually, but on the other hand maybe Clang one day gets warnings about 
#include or similar that would be a pity to miss when using -rewrite-includes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65371



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

[PATCH] D65371: do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614)

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 219370.
llunak added a comment.

Added a test.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65371

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Frontend/rewrite-includes-warnings.c


Index: clang/test/Frontend/rewrite-includes-warnings.c
===
--- clang/test/Frontend/rewrite-includes-warnings.c
+++ clang/test/Frontend/rewrite-includes-warnings.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -verify -Wall -Wextra -E -frewrite-includes %s
+// RUN: %clang_cc1 -verify -Wall -Wextra -Wunused-macros -E -frewrite-includes 
%s
 // expected-no-diagnostics
 
 #pragma GCC visibility push (default)
+
+#define UNUSED
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2723,7 +2723,8 @@
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from 
set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
-  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc())) {
+  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
+  !MacroExpansionInDirectivesOverride) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }


Index: clang/test/Frontend/rewrite-includes-warnings.c
===
--- clang/test/Frontend/rewrite-includes-warnings.c
+++ clang/test/Frontend/rewrite-includes-warnings.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -verify -Wall -Wextra -E -frewrite-includes %s
+// RUN: %clang_cc1 -verify -Wall -Wextra -Wunused-macros -E -frewrite-includes %s
 // expected-no-diagnostics
 
 #pragma GCC visibility push (default)
+
+#define UNUSED
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2723,7 +2723,8 @@
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
-  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc())) {
+  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
+  !MacroExpansionInDirectivesOverride) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

@nand The MSVC warnings are self explanatory - you've declared a number of 
methods (visitIndirectMember, emitConv and getPtrConstFn) but not provided 
definitions, as they're on template classes MSVC complains, a lot.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64146



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

[PATCH] D62731: [RFC] Add support for options -frounding-math -fp-model= and -fp-exception-behavior= : specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 219364.
mibintc retitled this revision from "[RFC] Add support for options -fp-model= 
and -fp-speculation= : specify floating point behavior" to "[RFC] Add support 
for options -frounding-math -fp-model= and -fp-exception-behavior= : specify 
floating point behavior".
mibintc edited the summary of this revision.
mibintc added a comment.

Addressed comments from @rjmccall and @andrew.w.kaylor.  Added -frounding-math 
option and -ffp-exception-behavior= option. Dropped -ffp-speculation= option.  
Added details about how the new options conflict with existing floating point 
options. Rewrote
RenderFloatingPointOptions using pseudo code to show how option conflicts will 
be detected.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/fpconstrained.c

Index: clang/test/CodeGen/fpconstrained.c
===
--- /dev/null
+++ clang/test/CodeGen/fpconstrained.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fp-model=strict -emit-llvm -o - %s | FileCheck %s -check-prefix=STRICT
+// RUN: %clang_cc1 -fp-model=precise -emit-llvm -o - %s | FileCheck %s -check-prefix=PRECISE
+// RUN: %clang_cc1 -fp-model=fast -emit-llvm -o - %s | FileCheck %s -check-prefix=FAST
+// RUN: %clang_cc1 -fp-model=fast -fp-exception-behavior=ignore -emit-llvm -o - %s | FileCheck %s -check-prefix=NOEXCEPT
+// RUN: %clang_cc1 -fp-model=fast -fp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s -check-prefix=EXCEPT
+// RUN: %clang_cc1 -fp-model=fast -fp-exception-behavior=maytrap -emit-llvm -o - %s | FileCheck %s -check-prefix=MAYTRAP
+float f0, f1, f2;
+
+void foo(void) {
+  // CHECK-LABEL: define {{.*}}void @foo()
+
+  // MAYTRAP: llvm.experimental.constrained.fadd.f32(float %0, float %1, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+  // EXCEPT: llvm.experimental.constrained.fadd.f32(float %0, float %1, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOEXCEPT: llvm.experimental.constrained.fadd.f32(float %0, float %1, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+  // STRICT: llvm.experimental.constrained.fadd.f32(float %0, float %1, metadata !"round.dynamic", metadata !"fpexcept.strict")
+  // STRICTEXCEPT: llvm.experimental.constrained.fadd.f32(float %0, float %1, metadata !"round.dynamic", metadata !"fpexcept.strict")
+  // STRICTNOEXCEPT: llvm.experimental.constrained.fadd.f32(float %0, float %1, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+  // PRECISE: fadd float
+  // FAST: fadd fast
+  f0 = f1 + f2;
+
+  // CHECK: ret
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3074,6 +3074,45 @@
   Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
   }
 
+  LangOptions::FPModelKind FPM = LangOptions::FPM_Off;
+  if (Arg *A = Args.getLastArg(OPT_fp_model_EQ)) {
+StringRef Val = A->getValue();
+if (Val == "precise")
+  FPM = LangOptions::FPM_Precise;
+else if (Val == "strict")
+  FPM = LangOptions::FPM_Strict;
+else if (Val == "fast")
+  FPM = LangOptions::FPM_Fast;
+else
+  llvm_unreachable("invalid -fp-model setting");
+  }
+  Opts.getFPMOptions().setFPModelSetting(FPM);
+
+  LangOptions::FPExceptionBehaviorKind FPE = LangOptions::FPE_Off;
+  if (Arg *A = Args.getLastArg(OPT_fp_exception_behavior_EQ)) {
+StringRef Val = A->getValue();
+if (Val == "ignore")
+  FPE = LangOptions::FPE_Ignore;
+else if (Val == "maytrap")
+  FPE = LangOptions::FPE_MayTrap;
+else if (Val == "strict")
+  FPE = LangOptions::FPE_Strict;
+else
+  llvm_unreachable("invalid -fp-exception-behavior setting");
+Opts.getFPMOptions().setFPExceptionBehaviorSetting(FPE);
+  }
+
+  if (FPM == LangOptions::FPM_Precise)
+// This doesn't correspond to constrained fp,
+// equivalent to -fp-contract=fast
+Opts.setDefaultFPContractMode(LangOptions::FPC_Fast);
+  else if (FPM == LangOptions::FPM_Fast) {
+// This doesn't correspond to constrained fp, equivalent to -ffast-math
+Opts.FastMath = true;
+Opts.FiniteMathOnly = true;
+Opts.setDefaultFPContractMode(LangOptions::FPC_Fast);
+  }
+
   Opts.RetainCommentsFromSystemHeaders =
   Args.hasArg(OPT_fretain_comments_from_system_headers);
 
@@ -3443,6 +3482,15 @@
 // FIXME: Should we really be calling this for an Language::Asm input?
 ParseLangArgs(LangOpts, Args, DashX, Res.getTargetOpts(),
   

[PATCH] D67358: Implement semantic selections.

[UTKARSH SAXENA via Phabricator via cfe-commits]

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

For a given cursor position, it returns ranges that are interesting to the user.
Currently the semantic ranges correspond to the nodes of the syntax trees.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67358

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
@@ -0,0 +1,133 @@
+//===-- SemanticSelectionTests.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 "Matchers.h"
+#include "Protocol.h"
+#include "SemanticSelection.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+namespace clang {
+namespace clangd {
+namespace {
+using ::testing::ElementsAreArray;
+
+TEST(SemanticSelection, All) {
+  const char *Tests[] = {
+  R"cpp( // Single statement in a function body.
+[[void func() [[{
+  int v = [[1^00;]]
+}
+  )cpp",
+  R"cpp( // Expression
+[[void func() [[{
+  int a = 1;
+  // int v = (10 + 2) * (a + a);
+  int v = (10^]] + 2]])]] * (a + a);]]
+}
+  )cpp",
+  R"cpp( // Function call.
+int add(int x, int y) { return x + y; }
+[[void callee() [[{
+  // int res = add(11, 22);
+  int res = [[add([[1^1]], 22);]]
+}
+  )cpp",
+  R"cpp( // Tricky macros.
+#define MUL ) * (
+[[void func() [[{
+  // int var = (4 + 15 MUL 6 + 10);
+  int var = [[([[4 + [[1^5 MUL 6 + 10);]]
+}
+   )cpp",
+  R"cpp( // Cursor inside a macro.
+#define HASH(x) ((x) % 10)
+[[void func() [[{
+  int a = HASH(2^3)]];]]
+}
+   )cpp",
+  R"cpp( // Cursor on a macro.
+#define HASH(x) ((x) % 10)
+[[void func() [[{
+  int a = HA^SH(23)]];]]
+}
+   )cpp",
+  R"cpp( // Multiple declaration.
+[[void func() [[{
+  int var1, var^2]], var3;]]
+}
+   )cpp",
+  R"cpp( // Before comment.
+[[void func() [[{
+  int var1 = 1;
+  int var2 = var1]]^ /*some comment*/ + 41;]]
+}
+   )cpp",
+  // Empty file.
+  "^",
+  // FIXME: We should get the whole DeclStmt as a range.
+  R"cpp( // Single statement in TU.
+[[int v = [[1^00;
+  )cpp",
+  // FIXME: No node found associated to the position.
+  R"cpp( // Cursor at end of VarDecl.
+void func() {
+  int v = 100 + 100^;
+}
+  )cpp",
+  // FIXME: No node found associated to the position.
+  R"cpp( // Cursor at end of VarDecl.
+void func() {
+  int v = 100 + ^  100;
+}
+  )cpp",
+  // Structs.
+  R"cpp(
+struct AAA { struct BBB { static int ccc(); };};
+[[void func() [[{
+  // int x = AAA::BBB::ccc();
+  int x = AAA::BBB::c^cc]]();]]
+}
+  )cpp",
+  R"cpp(
+struct AAA { struct BBB { static int ccc(); };};
+[[void func() [[{
+  // int x = AAA::BBB::ccc();
+  int x = [[AA^A]]::]]BBB::]]ccc]]();]]
+}
+  )cpp",
+  // Namespaces.
+  R"cpp( 
+[[namespace nsa { 
+  [[namespace nsb { 
+static int ccc();
+[[void func() [[{
+  // int x = nsa::nsb::ccc();
+  int x = nsa::nsb::cc^c]]();]]
+}
+  }]]
+}]]
+  )cpp",
+
+  };
+
+  for (const char *Test : Tests) {
+auto T = Annotations(Test);
+auto AST = TestTU::withCode(T.code()).build();
+auto Ranges = getSemanticRanges(AST, T.point()).Ranges;
+EXPECT_THAT(Ranges, ElementsAreArray(T.ranges())) << Test;
+  }
+}
+} // namespace
+} // namespace clangd
+} // namespace clang
\ No newline at end of file
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- 

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

[Frank Derry Wanye via Phabricator via cfe-commits]

ffrankies updated this revision to Diff 219358.
ffrankies added a comment.
Herald added subscribers: kadircet, jkorous.

Sorry for the delay.

I was mostly having trouble building clang-tidy after pulling from master and 
having it recognize that the struct-pack-align check exists. I finally realized 
that the check had to be "registered" in more files, and those changes are a 
part of the update.

I have also updated the ReleaseNotes to use the word "checks" instead of "lint 
checks", and implemented the suggestions from @alexfh for adhering to the style 
guide and being more concise (thanks for those comments! They'll be useful to 
most if not all of the other checks we're planning to submit).

Regarding the comment by @riccibruno: Our current plan is to submit checks as 
part of two modules: "OpenCL" and "FPGA", where the "OpenCL" checks are taken 
from OpenCL specifications, and "FPGA" checks are taken from Altera best 
practices and restrictions guides. That said, the struct-pack-align check is 
not specific to FPGAs; it's useful whenever a struct is moved from host to 
device, which could be something other than an FPGA. We are unaware of another 
module where this check would be more appropriate, so we stuck it here, but 
we're open to other suggestions, including moving it to the OpenCL module.


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

https://reviews.llvm.org/D66564

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/ClangTidyForceLinker.h
  clang-tidy/fpga/CMakeLists.txt
  clang-tidy/fpga/FPGATidyModule.cpp
  clang-tidy/fpga/StructPackAlignCheck.cpp
  clang-tidy/fpga/StructPackAlignCheck.h
  clang-tidy/plugin/CMakeLists.txt
  clang-tidy/tool/CMakeLists.txt
  clangd/CMakeLists.txt
  clangd/ClangdUnit.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fpga-struct-pack-align.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/index.rst
  test/clang-tidy/fpga-struct-pack-align.cpp

Index: test/clang-tidy/fpga-struct-pack-align.cpp
===
--- /dev/null
+++ test/clang-tidy/fpga-struct-pack-align.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy %s fpga-struct-pack-align %t -- -header-filter=.* "--" --include opencl-c.h -cl-std=CL1.2 -c
+
+// Struct needs both alignment and packing
+struct error {
+  char a;
+  double b;
+  char c;
+};
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: struct 'error' has inefficient access due to padding, only needs 10 bytes but is using 24 bytes, use "__attribute((packed))" [fpga-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: warning: struct 'error' has inefficient access due to poor alignment; currently aligned to 8 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [fpga-struct-pack-align]
+
+// Struct is explicitly packed, but needs alignment
+struct error_packed {
+  char a;
+  double b;
+  char c;
+} __attribute((packed));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: struct 'error_packed' has inefficient access due to poor alignment; currently aligned to 1 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [fpga-struct-pack-align]
+
+// Struct is properly packed, but needs alignment
+struct align_only {
+  char a;
+  char b;
+  char c;
+  char d;
+  int e;
+  double f;
+};
+// CHECK-MESSAGES: :[[@LINE-8]]:8: warning: struct 'align_only' has inefficient access due to poor alignment; currently aligned to 8 bytes, but size 16 bytes is large enough to benefit from "__attribute((aligned(16)))" [fpga-struct-pack-align]
+
+// Struct is perfectly packed but wrongly aligned
+struct bad_align {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(8)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: struct 'bad_align' has inefficient access due to poor alignment; currently aligned to 8 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [fpga-struct-pack-align]
+
+struct bad_align2 {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(32)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: struct 'bad_align2' has inefficient access due to poor alignment; currently aligned to 32 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [fpga-struct-pack-align]
+
+struct bad_align3 {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(4)));
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: struct 'bad_align3' has inefficient access due to poor alignment; currently aligned to 4 bytes, but size 10 bytes is large enough to benefit from "__attribute((aligned(16)))" [fpga-struct-pack-align]
+
+// Struct is both perfectly packed and aligned
+struct success {
+  char a;
+  double b;
+  char c;
+} __attribute((packed)) __attribute((aligned(16)));
+//Should take 10 bytes and be aligned to 16 bytes
+
+// Struct is properly packed, and explicitly aligned
+struct success2 {
+  int a;
+  int b;
+  

[PATCH] D67264: [clangd] Collect location of macro definition in the ParsedAST

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM.

We should probably also take a look at highlighting macros inside the preamble 
part of the main file.
@hokein, are you planning to do this or should we just file a bug for this for 
now?




Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:38
 // visitor.
-// FIXME: Should add highlighting to the macro definitions as well. But 
this
-// information is not collected in ParsedAST right now.
-for (const SourceLocation  : AST.getMainFileExpansions())
+for (const SourceLocation  : AST.getMacros())
   addToken(L, HighlightingKind::Macro);

NIT: use `SourceLocation`, it's just an int, so no need to use references here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67264



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

[PATCH] D67341: [clangd] Simplify semantic highlighting visitor

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D67341#1663104 , @hokein wrote:

> Unfortunately, the patch is a bit large, containing refactoring changes and 
> functionality changes, is it possible to split it into (two) smaller patches?


Done. There should be no functional changes now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67341



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

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked 3 inline comments as done.
mibintc added inline comments.



Comment at: clang/docs/UsersManual.rst:1305
+   and ``noexcept``. Note that -fp-model=[no]except can be combined with the
+   other three settings for this option. Details:
+

andrew.w.kaylor wrote:
> rjmccall wrote:
> > mibintc wrote:
> > > rjmccall wrote:
> > > > Combined how?  With a comma?
> > > > 
> > > > This option seems to have two independent dimensions.  Is that 
> > > > necessary for command-line compatibility with ICC, or can we separate 
> > > > it into two options?
> > > > 
> > > > The documentation should mention the default behavior along both 
> > > > dimensions.  Is it possible to override a prior instance of this option 
> > > > to get this default behavior back?
> > > > 
> > > > You mention that this `-fp-model=fast` is equivalent to `-ffast-math`.  
> > > > How does this option interact with that one if both are given on a 
> > > > command line?
> > > > 
> > > > Please put option text in backticks wherever it appears.
> > > > 
> > > > Most of these comments apply to `-fp-speculation` as well.
> > > > Combined how? With a comma?
> > > 
> > > > This option seems to have two independent dimensions. Is that necessary 
> > > > for command-line compatibility with ICC, or can we separate it into two 
> > > > options?
> > > Yes that's right, there are 2 dimensions.  I wrote it like this for 
> > > identical compatibility with icc, and cl.exe also defines the option this 
> > > way, to specify multiple values simultaneously. However I think it would 
> > > be reasonable and good to split them into separate options.  I will 
> > > discuss this with the folks back home.
> > > 
> > > > The documentation should mention the default behavior along both 
> > > > dimensions. 
> > > I added this info into the doc
> > > 
> > > > Is it possible to override a prior instance of this option to get this 
> > > > default behavior back?
> > > The 3 values along one dimension, precise, strict, fast if they appear 
> > > multiple times in the command line, the last value will be the setting 
> > > along that dimension.  Ditto with the other dimension, the rightmost 
> > > occurrence of except or noexcept will be the setting. 
> > > 
> > > > You mention that this -fp-model=fast is equivalent to -ffast-math. How 
> > > > does this option interact with that one if both are given on a command 
> > > > line?
> > > The idea is that they are synonyms so if either or both appeared on the 
> > > command line, the effect would be identical. 
> > > 
> > > I'll upload another patch with a few documentation updates and get back 
> > > to you about splitting the fp-model option into multiple options.  
> > > (Longer term, there are 2 other dimensions to fp-model)
> > > 
> > > And thanks for the review
> > > Yes that's right, there are 2 dimensions. I wrote it like this for 
> > > identical compatibility with icc, and cl.exe also defines the option this 
> > > way, to specify multiple values simultaneously. However I think it would 
> > > be reasonable and good to split them into separate options. I will 
> > > discuss this with the folks back home.
> > 
> > Okay.  There's certainly some value in imitating existing compilers, but it 
> > sounds like a lot has been forced into one option, so maybe we should take 
> > the opportunity to split it up.  If we do split it, though, I think the 
> > different dimensions should have different base spellings, rather than 
> > being repeated uses of `-fp-model`.
> > 
> > > The 3 values along one dimension, precise, strict, fast if they appear 
> > > multiple times in the command line, the last value will be the setting 
> > > along that dimension.
> > 
> > Okay.  This wasn't clear to me from the code, since the code also has an 
> > "off" option.
> > 
> > > The idea is that they are synonyms so if either or both appeared on the 
> > > command line, the effect would be identical.
> > 
> > Right, but compiler options are allowed to conflict with each other, with 
> > the general rule being that the last option "wins".  So what I'm asking is 
> > if that works correctly with this option and `-ffast-math`, so that e.g. 
> > `-ffast-math -fp-model=strict` leaves you with strict FP but 
> > `-fp-model=strict -ffast-math` leaves you with fast FP.  (That is another 
> > reason why it's best to have one aspect settled in each option: because you 
> > don't have to merge information from different uses of the option.)
> > 
> > At any rate, the documentation should be clear about how this interacts 
> > with `-ffast-math`.  You might even consider merging this into the 
> > documentation for `-ffast-math`, or at least revising that option's 
> > documentation.  Does `-fp-model=fast` cause `__FAST_MATH__` to be defined?
> > 
> > Also, strictly speaking, this should be `-ffp-model`, right?
> I think the ICC interface includes the exception option for 
> compatibility/consistency with 

[PATCH] D67341: [clangd] Simplify semantic highlighting visitor

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 219363.
ilya-biryukov added a comment.

- Turn into NFC, do not highlight lambdas differently


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67341

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp

Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclarationName.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
@@ -24,6 +25,18 @@
 namespace clangd {
 namespace {
 
+/// Some names are not written in the source code and cannot be highlighted,
+/// e.g. anonymous classes. This function detects those cases.
+bool canHighlightName(DeclarationName Name) {
+  if (Name.getNameKind() == DeclarationName::CXXConstructorName ||
+  Name.getNameKind() == DeclarationName::CXXUsingDirective)
+return true;
+  if (!Name.isIdentifier())
+return false;
+  auto *II = Name.getAsIdentifierInfo();
+  return II && II->getName() != "";
+}
+
 // Collects all semantic tokens in an ASTContext.
 class HighlightingTokenCollector
 : public RecursiveASTVisitor {
@@ -72,66 +85,30 @@
 
   bool VisitNamespaceAliasDecl(NamespaceAliasDecl *NAD) {
 // The target namespace of an alias can not be found in any other way.
-addToken(NAD->getTargetNameLoc(), HighlightingKind::Namespace);
+addToken(NAD->getTargetNameLoc(), NAD->getAliasedNamespace());
 return true;
   }
 
   bool VisitMemberExpr(MemberExpr *ME) {
-const auto *MD = ME->getMemberDecl();
-if (isa(MD))
-  // When calling the destructor manually like: AAA::~A(); The ~ is a
-  // MemberExpr. Other methods should still be highlighted though.
-  return true;
-if (isa(MD))
-  // The MemberLoc is invalid for C++ conversion operators. We do not
-  // attempt to add tokens with invalid locations.
-  return true;
-addToken(ME->getMemberLoc(), MD);
+if (canHighlightName(ME->getMemberNameInfo().getName()))
+  addToken(ME->getMemberLoc(), ME->getMemberDecl());
 return true;
   }
 
   bool VisitNamedDecl(NamedDecl *ND) {
-// UsingDirectiveDecl's namespaces do not show up anywhere else in the
-// Visit/Traverse mehods. But they should also be highlighted as a
-// namespace.
-if (const auto *UD = dyn_cast(ND)) {
-  addToken(UD->getIdentLocation(), HighlightingKind::Namespace);
-  return true;
-}
-
-// Constructors' TypeLoc has a TypePtr that is a FunctionProtoType. It has
-// no tag decl and therefore constructors must be gotten as NamedDecls
-// instead.
-if (ND->getDeclName().getNameKind() ==
-DeclarationName::CXXConstructorName) {
+if (canHighlightName(ND->getDeclName()))
   addToken(ND->getLocation(), ND);
-  return true;
-}
-
-if (ND->getDeclName().getNameKind() != DeclarationName::Identifier)
-  return true;
-
-addToken(ND->getLocation(), ND);
 return true;
   }
 
   bool VisitDeclRefExpr(DeclRefExpr *Ref) {
-if (Ref->getNameInfo().getName().getNameKind() !=
-DeclarationName::Identifier)
-  // Only want to highlight identifiers.
-  return true;
-
-addToken(Ref->getLocation(), Ref->getDecl());
-return true;
-  }
-
-  bool VisitTypedefNameDecl(TypedefNameDecl *TD) {
-addTokenForTypedef(TD->getLocation(), TD);
+if (canHighlightName(Ref->getNameInfo().getName()))
+  addToken(Ref->getLocation(), Ref->getDecl());
 return true;
   }
 
   bool VisitTypedefTypeLoc(TypedefTypeLoc TL) {
-addTokenForTypedef(TL.getBeginLoc(), TL.getTypedefNameDecl());
+addToken(TL.getBeginLoc(), TL.getTypedefNameDecl());
 return true;
   }
 
@@ -142,22 +119,29 @@
 return true;
   }
 
-  bool VisitTypeLoc(TypeLoc ) {
-// This check is for not getting two entries when there are anonymous
-// structs. It also makes us not highlight certain namespace qualifiers
-// twice. For elaborated types the actual type is highlighted as an inner
-// TypeLoc.
-if (TL.getTypeLocClass() != TypeLoc::TypeLocClass::Elaborated)
-  addType(TL.getBeginLoc(), TL.getTypePtr());
+  bool WalkUpFromTagTypeLoc(TagTypeLoc L) {
+if (L.isDefinition())
+  return true; // Definition will be highligthed by VisitNamedDecl.
+return RecursiveASTVisitor::WalkUpFromTagTypeLoc(L);
+  }
+
+  bool WalkUpFromElaboratedTypeLoc(ElaboratedTypeLoc L) {
+// Avoid highlighting 'struct' or 'enum' keywords.
+return true;
+  }
+
+  bool VisitTypeLoc(TypeLoc TL) {
+if (auto K = kindForType(TL.getTypePtr()))
+  addToken(TL.getBeginLoc(), *K);
 return true;
   }
 
   bool 

r371410 - [NFC] Add aacps bitfields access test

[Diogo N. Sampaio via cfe-commits]

Author: dnsampaio
Date: Mon Sep  9 08:39:45 2019
New Revision: 371410

URL: http://llvm.org/viewvc/llvm-project?rev=371410=rev
Log:
[NFC] Add aacps bitfields access test


Added:
cfe/trunk/test/CodeGen/aapcs-bitfield.c

Added: cfe/trunk/test/CodeGen/aapcs-bitfield.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/aapcs-bitfield.c?rev=371410=auto
==
--- cfe/trunk/test/CodeGen/aapcs-bitfield.c (added)
+++ cfe/trunk/test/CodeGen/aapcs-bitfield.c Mon Sep  9 08:39:45 2019
@@ -0,0 +1,824 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple armv8-none-linux-eabi %s -emit-llvm -o - -O3 | 
FileCheck %s -check-prefix=LE
+// RUN: %clang_cc1 -triple armebv8-none-linux-eabi %s -emit-llvm -o - -O3 | 
FileCheck %s -check-prefix=BE
+
+struct st0 {
+  short c : 7;
+};
+
+// LE-LABEL: @st0_check_load(
+// LE-NEXT:  entry:
+// LE-NEXT:[[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST0:%.*]], 
%struct.st0* [[M:%.*]], i32 0, i32 0
+// LE-NEXT:[[BF_LOAD:%.*]] = load i8, i8* [[TMP0]], align 2
+// LE-NEXT:[[BF_SHL:%.*]] = shl i8 [[BF_LOAD]], 1
+// LE-NEXT:[[BF_ASHR:%.*]] = ashr exact i8 [[BF_SHL]], 1
+// LE-NEXT:[[CONV:%.*]] = sext i8 [[BF_ASHR]] to i32
+// LE-NEXT:ret i32 [[CONV]]
+//
+// BE-LABEL: @st0_check_load(
+// BE-NEXT:  entry:
+// BE-NEXT:[[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST0:%.*]], 
%struct.st0* [[M:%.*]], i32 0, i32 0
+// BE-NEXT:[[BF_LOAD:%.*]] = load i8, i8* [[TMP0]], align 2
+// BE-NEXT:[[BF_ASHR:%.*]] = ashr i8 [[BF_LOAD]], 1
+// BE-NEXT:[[CONV:%.*]] = sext i8 [[BF_ASHR]] to i32
+// BE-NEXT:ret i32 [[CONV]]
+//
+int st0_check_load(struct st0 *m) {
+  return m->c;
+}
+
+// LE-LABEL: @st0_check_store(
+// LE-NEXT:  entry:
+// LE-NEXT:[[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST0:%.*]], 
%struct.st0* [[M:%.*]], i32 0, i32 0
+// LE-NEXT:[[BF_LOAD:%.*]] = load i8, i8* [[TMP0]], align 2
+// LE-NEXT:[[BF_CLEAR:%.*]] = and i8 [[BF_LOAD]], -128
+// LE-NEXT:[[BF_SET:%.*]] = or i8 [[BF_CLEAR]], 1
+// LE-NEXT:store i8 [[BF_SET]], i8* [[TMP0]], align 2
+// LE-NEXT:ret void
+//
+// BE-LABEL: @st0_check_store(
+// BE-NEXT:  entry:
+// BE-NEXT:[[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST0:%.*]], 
%struct.st0* [[M:%.*]], i32 0, i32 0
+// BE-NEXT:[[BF_LOAD:%.*]] = load i8, i8* [[TMP0]], align 2
+// BE-NEXT:[[BF_CLEAR:%.*]] = and i8 [[BF_LOAD]], 1
+// BE-NEXT:[[BF_SET:%.*]] = or i8 [[BF_CLEAR]], 2
+// BE-NEXT:store i8 [[BF_SET]], i8* [[TMP0]], align 2
+// BE-NEXT:ret void
+//
+void st0_check_store(struct st0 *m) {
+  m->c = 1;
+}
+
+struct st1 {
+  int a : 10;
+  short c : 6;
+};
+
+// LE-LABEL: @st1_check_load(
+// LE-NEXT:  entry:
+// LE-NEXT:[[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST1:%.*]], 
%struct.st1* [[M:%.*]], i32 0, i32 0
+// LE-NEXT:[[BF_LOAD:%.*]] = load i16, i16* [[TMP0]], align 4
+// LE-NEXT:[[BF_ASHR:%.*]] = ashr i16 [[BF_LOAD]], 10
+// LE-NEXT:[[CONV:%.*]] = sext i16 [[BF_ASHR]] to i32
+// LE-NEXT:ret i32 [[CONV]]
+//
+// BE-LABEL: @st1_check_load(
+// BE-NEXT:  entry:
+// BE-NEXT:[[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST1:%.*]], 
%struct.st1* [[M:%.*]], i32 0, i32 0
+// BE-NEXT:[[BF_LOAD:%.*]] = load i16, i16* [[TMP0]], align 4
+// BE-NEXT:[[BF_SHL:%.*]] = shl i16 [[BF_LOAD]], 10
+// BE-NEXT:[[BF_ASHR:%.*]] = ashr exact i16 [[BF_SHL]], 10
+// BE-NEXT:[[CONV:%.*]] = sext i16 [[BF_ASHR]] to i32
+// BE-NEXT:ret i32 [[CONV]]
+//
+int st1_check_load(struct st1 *m) {
+  return m->c;
+}
+
+// LE-LABEL: @st1_check_store(
+// LE-NEXT:  entry:
+// LE-NEXT:[[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST1:%.*]], 
%struct.st1* [[M:%.*]], i32 0, i32 0
+// LE-NEXT:[[BF_LOAD:%.*]] = load i16, i16* [[TMP0]], align 4
+// LE-NEXT:[[BF_CLEAR:%.*]] = and i16 [[BF_LOAD]], 1023
+// LE-NEXT:[[BF_SET:%.*]] = or i16 [[BF_CLEAR]], 1024
+// LE-NEXT:store i16 [[BF_SET]], i16* [[TMP0]], align 4
+// LE-NEXT:ret void
+//
+// BE-LABEL: @st1_check_store(
+// BE-NEXT:  entry:
+// BE-NEXT:[[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST1:%.*]], 
%struct.st1* [[M:%.*]], i32 0, i32 0
+// BE-NEXT:[[BF_LOAD:%.*]] = load i16, i16* [[TMP0]], align 4
+// BE-NEXT:[[BF_CLEAR:%.*]] = and i16 [[BF_LOAD]], -64
+// BE-NEXT:[[BF_SET:%.*]] = or i16 [[BF_CLEAR]], 1
+// BE-NEXT:store i16 [[BF_SET]], i16* [[TMP0]], align 4
+// BE-NEXT:ret void
+//
+void st1_check_store(struct st1 *m) {
+  m->c = 1;
+}
+
+struct st2 {
+  int a : 10;
+  short c : 7;
+};
+
+// LE-LABEL: @st2_check_load(
+// LE-NEXT:  entry:
+// LE-NEXT:[[C:%.*]] = getelementptr inbounds [[STRUCT_ST2:%.*]], 
%struct.st2* [[M:%.*]], i32 0, i32 1
+// LE-NEXT:[[BF_LOAD:%.*]] = load i8, i8* [[C]], align 2
+// LE-NEXT:[[BF_SHL:%.*]] = shl i8 [[BF_LOAD]], 1
+// LE-NEXT:[[BF_ASHR:%.*]] = ashr exact i8 [[BF_SHL]], 1
+// 

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

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371408: [clangd] Use pre-populated mappings for standard 
symbols (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67172?vs=219360=219361#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67172

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

Index: clang-tools-extra/trunk/clangd/Preamble.cpp
===
--- clang-tools-extra/trunk/clangd/Preamble.cpp
+++ clang-tools-extra/trunk/clangd/Preamble.cpp
@@ -78,7 +78,7 @@
   }
 
   void BeforeExecute(CompilerInstance ) override {
-addSystemHeadersMapping(, CI.getLangOpts());
+CanonIncludes.addSystemHeadersMapping(CI.getLangOpts());
 SourceMgr = ();
   }
 
Index: clang-tools-extra/trunk/clangd/ParsedAST.cpp
===
--- clang-tools-extra/trunk/clangd/ParsedAST.cpp
+++ clang-tools-extra/trunk/clangd/ParsedAST.cpp
@@ -367,7 +367,7 @@
   if (Preamble)
 CanonIncludes = Preamble->CanonIncludes;
   else
-addSystemHeadersMapping(, Clang->getLangOpts());
+CanonIncludes.addSystemHeadersMapping(Clang->getLangOpts());
   std::unique_ptr IWYUHandler =
   collectIWYUHeaderMaps();
   Clang->getPreprocessor().addCommentHandler(IWYUHandler.get());
Index: clang-tools-extra/trunk/clangd/unittests/CanonicalIncludesTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/CanonicalIncludesTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/CanonicalIncludesTests.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "index/CanonicalIncludes.h"
+#include "clang/Basic/LangOptions.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -17,7 +18,7 @@
   CanonicalIncludes CI;
   auto Language = LangOptions();
   Language.C11 = true;
-  addSystemHeadersMapping(, Language);
+  CI.addSystemHeadersMapping(Language);
   // Usual standard library symbols are mapped correctly.
   EXPECT_EQ("", CI.mapHeader("path/stdio.h", "printf"));
 }
@@ -26,7 +27,7 @@
   CanonicalIncludes CI;
   auto Language = LangOptions();
   Language.CPlusPlus = true;
-  addSystemHeadersMapping(, Language);
+  CI.addSystemHeadersMapping(Language);
 
   // Usual standard library symbols are mapped correctly.
   EXPECT_EQ("", CI.mapHeader("path/vector.h", "std::vector"));
@@ -54,19 +55,32 @@
 TEST(CanonicalIncludesTest, SymbolMapping) {
   // As used for standard library.
   CanonicalIncludes CI;
-  CI.addSymbolMapping("some::symbol", "");
+  LangOptions Language;
+  Language.CPlusPlus = true;
+  // Ensures 'std::vector' is mapped to ''.
+  CI.addSystemHeadersMapping(Language);
 
-  EXPECT_EQ("", CI.mapHeader("foo/bar", "some::symbol"));
+  EXPECT_EQ("", CI.mapHeader("foo/bar", "std::vector"));
   EXPECT_EQ("foo/bar", CI.mapHeader("foo/bar", "other::symbol"));
 }
 
 TEST(CanonicalIncludesTest, Precedence) {
   CanonicalIncludes CI;
   CI.addMapping("some/path", "");
-  CI.addSymbolMapping("some::symbol", "");
+  LangOptions Language;
+  Language.CPlusPlus = true;
+  CI.addSystemHeadersMapping(Language);
 
-  // Symbol mapping beats path mapping.
-  EXPECT_EQ("", CI.mapHeader("some/path", "some::symbol"));
+  // We added a mapping from some/path to .
+  ASSERT_EQ("", CI.mapHeader("some/path", ""));
+  // We should have a path from 'bits/stl_vector.h' to ''.
+  ASSERT_EQ("", CI.mapHeader("bits/stl_vector.h", ""));
+  // We should also have a symbol mapping from 'std::map' to ''.
+  ASSERT_EQ("", CI.mapHeader("some/header.h", "std::map"));
+
+  // And the symbol mapping should take precedence over paths mapping.
+  EXPECT_EQ("", CI.mapHeader("bits/stl_vector.h", "std::map"));
+  EXPECT_EQ("", CI.mapHeader("some/path", "std::map"));
 }
 
 } // namespace
Index: clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp
@@ -974,7 +974,7 @@
   CanonicalIncludes Includes;
   auto Language = LangOptions();
   Language.CPlusPlus = true;
-  addSystemHeadersMapping(, Language);
+  Includes.addSystemHeadersMapping(Language);
   CollectorOpts.Includes = 
   runSymbolCollector("namespace std { class string {}; }", /*Main=*/"");
   

[clang-tools-extra] r371408 - [clangd] Use pre-populated mappings for standard symbols

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Mon Sep  9 08:32:51 2019
New Revision: 371408

URL: http://llvm.org/viewvc/llvm-project?rev=371408=rev
Log:
[clangd] Use pre-populated mappings for standard symbols

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

To achieve this, move mapping of system includes out of CanonicalIncludes
and into a separate class

Reviewers: sammccall, hokein

Reviewed By: sammccall

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

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/ParsedAST.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ParsedAST.cpp?rev=371408=371407=371408=diff
==
--- clang-tools-extra/trunk/clangd/ParsedAST.cpp (original)
+++ clang-tools-extra/trunk/clangd/ParsedAST.cpp Mon Sep  9 08:32:51 2019
@@ -367,7 +367,7 @@ ParsedAST::build(std::unique_ptrCanonIncludes;
   else
-addSystemHeadersMapping(, Clang->getLangOpts());
+CanonIncludes.addSystemHeadersMapping(Clang->getLangOpts());
   std::unique_ptr IWYUHandler =
   collectIWYUHeaderMaps();
   Clang->getPreprocessor().addCommentHandler(IWYUHandler.get());

Modified: clang-tools-extra/trunk/clangd/Preamble.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Preamble.cpp?rev=371408=371407=371408=diff
==
--- clang-tools-extra/trunk/clangd/Preamble.cpp (original)
+++ clang-tools-extra/trunk/clangd/Preamble.cpp Mon Sep  9 08:32:51 2019
@@ -78,7 +78,7 @@ public:
   }
 
   void BeforeExecute(CompilerInstance ) override {
-addSystemHeadersMapping(, CI.getLangOpts());
+CanonIncludes.addSystemHeadersMapping(CI.getLangOpts());
 SourceMgr = ();
   }
 

Modified: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp?rev=371408=371407=371408=diff
==
--- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp Mon Sep  9 
08:32:51 2019
@@ -9,6 +9,7 @@
 #include "CanonicalIncludes.h"
 #include "Headers.h"
 #include "clang/Driver/Types.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Path.h"
 #include 
 
@@ -18,43 +19,40 @@ namespace {
 const char IWYUPragma[] = "// IWYU pragma: private, include ";
 } // namespace
 
-void CanonicalIncludes::addPathSuffixMapping(llvm::StringRef Suffix,
- llvm::StringRef CanonicalPath) {
-  int Components = std::distance(llvm::sys::path::begin(Suffix),
- llvm::sys::path::end(Suffix));
-  MaxSuffixComponents = std::max(MaxSuffixComponents, Components);
-  SuffixHeaderMapping[Suffix] = CanonicalPath;
-}
-
 void CanonicalIncludes::addMapping(llvm::StringRef Path,
llvm::StringRef CanonicalPath) {
   FullPathMapping[Path] = CanonicalPath;
 }
 
-void CanonicalIncludes::addSymbolMapping(llvm::StringRef QualifiedName,
- llvm::StringRef CanonicalPath) {
-  this->SymbolMapping[QualifiedName] = CanonicalPath;
-}
+/// The maximum number of path components in a key from StdSuffixHeaderMapping.
+/// Used to minimize the number of lookups in suffix path mappings.
+constexpr int MaxSuffixComponents = 3;
 
 llvm::StringRef
 CanonicalIncludes::mapHeader(llvm::StringRef Header,
  llvm::StringRef QualifiedName) const {
   assert(!Header.empty());
-  auto SE = SymbolMapping.find(QualifiedName);
-  if (SE != SymbolMapping.end())
-return SE->second;
+  if (StdSymbolMapping) {
+auto SE = StdSymbolMapping->find(QualifiedName);
+if (SE != StdSymbolMapping->end())
+  return SE->second;
+  }
 
   auto MapIt = FullPathMapping.find(Header);
   if (MapIt != FullPathMapping.end())
 return MapIt->second;
 
+  if (!StdSuffixHeaderMapping)
+return Header;
+
   int Components = 1;
+
   for (auto It = llvm::sys::path::rbegin(Header),
 End = llvm::sys::path::rend(Header);
It != End && Components <= MaxSuffixComponents; ++It, ++Components) {
 auto SubPath = Header.substr(It->data() - Header.begin());
-auto MappingIt = SuffixHeaderMapping.find(SubPath);
-if (MappingIt != SuffixHeaderMapping.end())
+auto MappingIt 

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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks for all suggestions. The final result is rather small and minimal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67172



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

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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 219360.
ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added a comment.

- Make MaxSuffixComponents a constant
- Put the suffix mapping into a single constant
- Initialize all StringMaps directly
- Rename to Std...Mapping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67172

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

Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -974,7 +974,7 @@
   CanonicalIncludes Includes;
   auto Language = LangOptions();
   Language.CPlusPlus = true;
-  addSystemHeadersMapping(, Language);
+  Includes.addSystemHeadersMapping(Language);
   CollectorOpts.Includes = 
   runSymbolCollector("namespace std { class string {}; }", /*Main=*/"");
   EXPECT_THAT(Symbols,
Index: clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
===
--- clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "index/CanonicalIncludes.h"
+#include "clang/Basic/LangOptions.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -17,7 +18,7 @@
   CanonicalIncludes CI;
   auto Language = LangOptions();
   Language.C11 = true;
-  addSystemHeadersMapping(, Language);
+  CI.addSystemHeadersMapping(Language);
   // Usual standard library symbols are mapped correctly.
   EXPECT_EQ("", CI.mapHeader("path/stdio.h", "printf"));
 }
@@ -26,7 +27,7 @@
   CanonicalIncludes CI;
   auto Language = LangOptions();
   Language.CPlusPlus = true;
-  addSystemHeadersMapping(, Language);
+  CI.addSystemHeadersMapping(Language);
 
   // Usual standard library symbols are mapped correctly.
   EXPECT_EQ("", CI.mapHeader("path/vector.h", "std::vector"));
@@ -54,19 +55,32 @@
 TEST(CanonicalIncludesTest, SymbolMapping) {
   // As used for standard library.
   CanonicalIncludes CI;
-  CI.addSymbolMapping("some::symbol", "");
+  LangOptions Language;
+  Language.CPlusPlus = true;
+  // Ensures 'std::vector' is mapped to ''.
+  CI.addSystemHeadersMapping(Language);
 
-  EXPECT_EQ("", CI.mapHeader("foo/bar", "some::symbol"));
+  EXPECT_EQ("", CI.mapHeader("foo/bar", "std::vector"));
   EXPECT_EQ("foo/bar", CI.mapHeader("foo/bar", "other::symbol"));
 }
 
 TEST(CanonicalIncludesTest, Precedence) {
   CanonicalIncludes CI;
   CI.addMapping("some/path", "");
-  CI.addSymbolMapping("some::symbol", "");
+  LangOptions Language;
+  Language.CPlusPlus = true;
+  CI.addSystemHeadersMapping(Language);
+
+  // We added a mapping from some/path to .
+  ASSERT_EQ("", CI.mapHeader("some/path", ""));
+  // We should have a path from 'bits/stl_vector.h' to ''.
+  ASSERT_EQ("", CI.mapHeader("bits/stl_vector.h", ""));
+  // We should also have a symbol mapping from 'std::map' to ''.
+  ASSERT_EQ("", CI.mapHeader("some/header.h", "std::map"));
 
-  // Symbol mapping beats path mapping.
-  EXPECT_EQ("", CI.mapHeader("some/path", "some::symbol"));
+  // And the symbol mapping should take precedence over paths mapping.
+  EXPECT_EQ("", CI.mapHeader("bits/stl_vector.h", "std::map"));
+  EXPECT_EQ("", CI.mapHeader("some/path", "std::map"));
 }
 
 } // namespace
Index: clang-tools-extra/clangd/index/IndexAction.cpp
===
--- clang-tools-extra/clangd/index/IndexAction.cpp
+++ clang-tools-extra/clangd/index/IndexAction.cpp
@@ -141,7 +141,7 @@
   std::unique_ptr
   CreateASTConsumer(CompilerInstance , llvm::StringRef InFile) override {
 CI.getPreprocessor().addCommentHandler(PragmaHandler.get());
-addSystemHeadersMapping(Includes.get(), CI.getLangOpts());
+Includes->addSystemHeadersMapping(CI.getLangOpts());
 if (IncludeGraphCallback != nullptr)
   CI.getPreprocessor().addPPCallbacks(
   std::make_unique(CI.getSourceManager(), IG));
Index: clang-tools-extra/clangd/index/CanonicalIncludes.h
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.h
+++ clang-tools-extra/clangd/index/CanonicalIncludes.h
@@ -21,6 +21,7 @@
 
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Regex.h"
 #include 
 #include 
@@ -34,37 +35,34 

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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked 8 inline comments as done.
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:41
 
+  if (!SuffixHeaderMapping)
+return Header;

sammccall wrote:
> nit: can we write `if (SuffixHeaderMapping) { ... }` for consistency with 
> above?
I'd prefer less nesting to consistency, but happy let me know if you feel 
that's very important.



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:84
+  llvm::StringMap SuffixHeaderMapping;
+  int MaxSuffixComponents = 0;
+};

sammccall wrote:
> so this is a constant, and it's 3.
> 
> Can we avoid the calculation/propagation and defining a struct, and just add 
> an assert(llvm::all_of(...)) after the initialization?
The assertion is inside the loop that's adding the mappings instead of 
`llvm::all_of`. 



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:758
+void CanonicalIncludes::addSystemHeadersMapping(const LangOptions ) {
+  static constexpr std::pair SymbolMap[] = {
+#define SYMBOL(Name, NameSpace, Header) {#NameSpace #Name, #Header},

sammccall wrote:
> if you want to save CPU, move to the scope where it's used? Lit tests and 
> many interesting subsets will never use C.
Done. Also initialized everything directly as StringMap


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67172



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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D67172#1663179 , @ilya-biryukov 
wrote:

> > "we could try" sounds like we *don't* know how to eliminate it. Parsing 
> > manpages aside, I thought the main problem was these symbols are 
> > nonstandard and an infinitely portable qname->header mapping simply didn't 
> > exist.
>
> I would expect qualified names to be more portable than paths, but might be 
> mistaken. I recall that we might run into problems if folks define some names 
> as macros instead of functions, but I would be interested to see how common 
> is this.


In practice, I expect almost all of these names to be C and thus unqualified.

Names are indeed more portable but unlike path mappings we can't just include 
both when different platforms want different things, or nothing.
e.g. are we willing to claim that any kind of symbol named `::open` should be 
from ``, even on windows?
Maybe it's OK, and I can't remember if we had specific bad examples in mind, 
but seems like there are dragons here.

>> Thanks, this looks a lot better!
>>  I think it can be simplified a little further, as the suffix maps are 
>> totally hardcoded now.
> 
> I think the simplest option is to always make the path suffix mapping 
> available in the class.
>  That means we will always map the system paths to their shorter versions. It 
> looks ok to me, but might make unit-testing a little more awkward.
>  WDYT?
>  I'll land this and send this out as a separate patch if everyone is happy 
> with the approach.

From a public API standpoint, it seems cleaner to only have these available 
once addSystemHeadersMapping has been called (with any argument), and delay 
assigning the pointer until then.
But if adds more than a few lines, it seems fine without.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67172



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev added a comment.

Ping


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

https://reviews.llvm.org/D64943



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

[PATCH] D67031: [Clang][Bundler] Error reporting improvements

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev added a comment.

Ping


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

https://reviews.llvm.org/D67031



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

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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D67172#1663096 , @sammccall wrote:

> LG, though if we can drop the struct and make MaxSuffixComponents a constant 
> it'd be simpler still.


Done.

> Sure. This is going to win a couple of percent I guess: for these cases we 
> care about walltime including rebuild and lit startup overhead. It's worth 
> having (and thanks for doing this!) but I don't think we should pay much 
> complexity.

Agreed.

> "we could try" sounds like we *don't* know how to eliminate it. Parsing 
> manpages aside, I thought the main problem was these symbols are nonstandard 
> and an infinitely portable qname->header mapping simply didn't exist.

I would expect qualified names to be more portable than paths, but might be 
mistaken. I recall that we might run into problems if folks define some names 
as macros instead of functions, but I would be interested to see how common is 
this.
We should probably write down the examples that are broken somewhere... It's 
hard to see why it doesn't work without having the concrete cases.
@hokein, do you have any examples that cannot be covered by qual-name-to-path 
mapping?

> Thanks, this looks a lot better!
>  I think it can be simplified a little further, as the suffix maps are 
> totally hardcoded now.

I think the simplest option is to always make the path suffix mapping available 
in the class.
That means we will always map the system paths to their shorter versions. It 
looks ok to me, but might make unit-testing a little more awkward.
WDYT?
I'll land this and send this out as a separate patch if everyone is happy with 
the approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67172



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

[PATCH] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Sam Elliott via Phabricator via cfe-commits]

lenary accepted this revision.
lenary added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: pzheng.

I think my feeling is that this patch can land and we can change the default 
abi for baremetal targets in a follow-up patch.




Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:386
+  else
+return Triple.getArch() == llvm::Triple::riscv32 ? "ilp32" : "lp64";
 }

luismarques wrote:
> luismarques wrote:
> > When I compile a bare metal GNU toolchain (using 
> > , reports GCC 8.3.0) I seem 
> > to get lp64d by default. Should we not match that behaviour?
> > 
> > ```
> > 
> > $ cat test.c
> > float foo() { return 42.0; }
> > $ riscv64-unknown-elf-gcc -O2 -S test.c
> > $ cat test.s
> > (...)
> > foo:
> > lui a5,%hi(.LC0)
> > flw fa0,%lo(.LC0)(a5)
> > (...)
> > ```
> To clarify, that's a toolchain configured with `--enable-multilib`.
I'm confused by this @luismarques 

If I do `riscv64-unknown-elf-gcc -c test.c` followed by 
`riscv64-unknown-elf-objdump -f test.o`, the flags displayed are 0x0011, 
which does not include `ELF::EF_RISCV_FLOAT_ABI_DOUBLE` (0x4), and so suggests 
gcc is using `-mabi=lp64` on baremetal elf targets. 

That said, `riscv64-unknown-elf-gcc -### -c test.c` shows it's invoking all 
subtools with `-march=rv64imafdc` and `-mabi-lp64d`. This is still with 
`--enable-multilib`, using `riscv64-unknown-elf-gcc (GCC) 8.3.0` and `GNU 
objdump (GNU Binutils) 2.32`.

I tried to look at where a default was being set in the gcc repo, and the only 
thing I can see is that the `rv64imafdc/lp64d` is the last combination to be 
generated in the multilib configuration, so they may not have explicitly chosen 
it as a default. I'm not sure. 


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

https://reviews.llvm.org/D65634



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

[PATCH] D67290: [clangd] Add a new highlighting kind for typedefs

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:530
+  case HighlightingKind::Typedef:
+return "entity.name.type.typedef.cpp";
   case HighlightingKind::Namespace:

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > Not sure what the rules for the scope names are, happy to change if I'm 
> > > doing it wrong
> > I think the current name is fine, just checked the name for this type in 
> > vscode highlighting configs, but didn't find a particular name for this 
> > type (my guess is that regex-based highlighting is not able to catch this 
> > case).
> Ack. Keeping as is, thanks for taking a look.
> Is there a way for us to provide a custom color for this in our extension? Or 
> would we just revert to the default color for `entity.name.type`?
> Is there a way for us to provide a custom color for this in our extension?
we don't support it yet, this is a feature request in 
https://github.com/clangd/clangd/issues/143.

> Or would we just revert to the default color for entity.name.type?
Yes, we will fallback to the color for `entity.name.type`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67290



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

r371403 - Merge note_ovl_builtin_candidate diagnostics; NFC

[Sven van Haastregt via cfe-commits]

Author: svenvh
Date: Mon Sep  9 07:39:20 2019
New Revision: 371403

URL: http://llvm.org/viewvc/llvm-project?rev=371403=rev
Log:
Merge note_ovl_builtin_candidate diagnostics; NFC

There is no difference between the unary and binary case, so
merge them.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaOverload.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=371403=371402=371403=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Sep  9 07:39:20 
2019
@@ -3920,10 +3920,7 @@ def note_implicit_member_target_infer_co
 
 def note_ambiguous_type_conversion: Note<
 "because of ambiguity in conversion %diff{of $ to $|between types}0,1">;
-def note_ovl_builtin_binary_candidate : Note<
-"built-in candidate %0">;
-def note_ovl_builtin_unary_candidate : Note<
-"built-in candidate %0">;
+def note_ovl_builtin_candidate : Note<"built-in candidate %0">;
 def err_ovl_no_viable_function_in_init : Error<
   "no matching constructor for initialization of %0">;
 def err_ovl_no_conversion_in_cast : Error<

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=371403=371402=371403=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Mon Sep  9 07:39:20 2019
@@ -10547,12 +10547,12 @@ static void NoteBuiltinOperatorCandidate
   TypeStr += Cand->BuiltinParamTypes[0].getAsString();
   if (Cand->Conversions.size() == 1) {
 TypeStr += ")";
-S.Diag(OpLoc, diag::note_ovl_builtin_unary_candidate) << TypeStr;
+S.Diag(OpLoc, diag::note_ovl_builtin_candidate) << TypeStr;
   } else {
 TypeStr += ", ";
 TypeStr += Cand->BuiltinParamTypes[1].getAsString();
 TypeStr += ")";
-S.Diag(OpLoc, diag::note_ovl_builtin_binary_candidate) << TypeStr;
+S.Diag(OpLoc, diag::note_ovl_builtin_candidate) << TypeStr;
   }
 }
 


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

[PATCH] D67290: [clangd] Add a new highlighting kind for typedefs

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371402: [clangd] Add a new highlighting kind for typedefs 
(authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67290?vs=219348=219351#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67290

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

Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
 #include 
 
 namespace clang {
@@ -125,13 +126,12 @@
   }
 
   bool VisitTypedefNameDecl(TypedefNameDecl *TD) {
-if (const auto *TSI = TD->getTypeSourceInfo())
-  addType(TD->getLocation(), TSI->getTypeLoc().getTypePtr());
+addTokenForTypedef(TD->getLocation(), TD);
 return true;
   }
 
   bool VisitTypedefTypeLoc(TypedefTypeLoc TL) {
-addType(TL.getBeginLoc(), TL.getTypePtr());
+addTokenForTypedef(TL.getBeginLoc(), TL.getTypedefNameDecl());
 return true;
   }
 
@@ -182,17 +182,35 @@
   }
 
 private:
-  void addType(SourceLocation Loc, const Type *TP) {
-if (!TP)
+  void addTokenForTypedef(SourceLocation Loc, const TypedefNameDecl *TD) {
+auto *TSI = TD->getTypeSourceInfo();
+if (!TSI)
+  return;
+// Try to highlight as underlying type.
+if (addType(Loc, TSI->getType().getTypePtrOrNull()))
   return;
-if (TP->isBuiltinType())
+// Fallback to the typedef highlighting kind.
+addToken(Loc, HighlightingKind::Typedef);
+  }
+
+  bool addType(SourceLocation Loc, const Type *TP) {
+if (!TP)
+  return false;
+if (TP->isBuiltinType()) {
   // Builtins must be special cased as they do not have a TagDecl.
   addToken(Loc, HighlightingKind::Primitive);
-if (auto *TD = dyn_cast(TP))
+  return true;
+}
+if (auto *TD = dyn_cast(TP)) {
   // TemplateTypeParmType also do not have a TagDecl.
   addToken(Loc, TD->getDecl());
-if (auto *TD = TP->getAsTagDecl())
+  return true;
+}
+if (auto *TD = TP->getAsTagDecl()) {
   addToken(Loc, TD);
+  return true;
+}
+return false;
   }
 
   void addToken(SourceLocation Loc, const NamedDecl *D) {
@@ -379,6 +397,8 @@
 return OS << "Enum";
   case HighlightingKind::EnumConstant:
 return OS << "EnumConstant";
+  case HighlightingKind::Typedef:
+return OS << "Typedef";
   case HighlightingKind::Namespace:
 return OS << "Namespace";
   case HighlightingKind::TemplateParameter:
@@ -506,6 +526,8 @@
 return "entity.name.type.enum.cpp";
   case HighlightingKind::EnumConstant:
 return "variable.other.enummember.cpp";
+  case HighlightingKind::Typedef:
+return "entity.name.type.typedef.cpp";
   case HighlightingKind::Namespace:
 return "entity.name.namespace.cpp";
   case HighlightingKind::TemplateParameter:
Index: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
@@ -496,12 +496,16 @@
 typedef $TemplateParameter[[T]] $TemplateParameter[[TemplateParam2]];
 using $Primitive[[IntType]] = $Primitive[[int]];
 
-// These typedefs are not yet highlighted, their types are complicated.
-using Pointer = $TemplateParameter[[T]] *;
-using LVReference = $TemplateParameter[[T]] &;
-using RVReference = $TemplateParameter[[T]]&&;
-using Array = $TemplateParameter[[T]]*[3];
-using MemberPointer = $Primitive[[int]] (A::*)($Primitive[[int]]);
+using $Typedef[[Pointer]] = $TemplateParameter[[T]] *;
+using $Typedef[[LVReference]] = $TemplateParameter[[T]] &;
+using $Typedef[[RVReference]] = $TemplateParameter[[T]]&&;
+using $Typedef[[Array]] = $TemplateParameter[[T]]*[3];
+using $Typedef[[MemberPointer]] = $Primitive[[int]] (A::*)($Primitive[[int]]);
+
+// Use various previously defined typedefs in a function type.
+$Primitive[[void]] $Method[[func]](
+  $Typedef[[Pointer]], $Typedef[[LVReference]], $Typedef[[RVReference]],
+  $Typedef[[Array]], $Typedef[[MemberPointer]]);
   };
 )cpp"};
   for (const auto  : TestCases) {
Index: 

[clang-tools-extra] r371402 - [clangd] Add a new highlighting kind for typedefs

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Mon Sep  9 07:33:10 2019
New Revision: 371402

URL: http://llvm.org/viewvc/llvm-project?rev=371402=rev
Log:
[clangd] Add a new highlighting kind for typedefs

Summary:
We still attempt to highlight them as underlying types, but fallback to
the generic 'typedef' highlighting kind if the underlying type is too
complicated.

Reviewers: hokein

Reviewed By: hokein

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
clang-tools-extra/trunk/clangd/SemanticHighlighting.h
clang-tools-extra/trunk/clangd/test/semantic-highlighting.test
clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp

Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp?rev=371402=371401=371402=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp Mon Sep  9 07:33:10 
2019
@@ -17,6 +17,7 @@
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
 #include 
 
 namespace clang {
@@ -125,13 +126,12 @@ public:
   }
 
   bool VisitTypedefNameDecl(TypedefNameDecl *TD) {
-if (const auto *TSI = TD->getTypeSourceInfo())
-  addType(TD->getLocation(), TSI->getTypeLoc().getTypePtr());
+addTokenForTypedef(TD->getLocation(), TD);
 return true;
   }
 
   bool VisitTypedefTypeLoc(TypedefTypeLoc TL) {
-addType(TL.getBeginLoc(), TL.getTypePtr());
+addTokenForTypedef(TL.getBeginLoc(), TL.getTypedefNameDecl());
 return true;
   }
 
@@ -182,17 +182,35 @@ public:
   }
 
 private:
-  void addType(SourceLocation Loc, const Type *TP) {
-if (!TP)
+  void addTokenForTypedef(SourceLocation Loc, const TypedefNameDecl *TD) {
+auto *TSI = TD->getTypeSourceInfo();
+if (!TSI)
+  return;
+// Try to highlight as underlying type.
+if (addType(Loc, TSI->getType().getTypePtrOrNull()))
   return;
-if (TP->isBuiltinType())
+// Fallback to the typedef highlighting kind.
+addToken(Loc, HighlightingKind::Typedef);
+  }
+
+  bool addType(SourceLocation Loc, const Type *TP) {
+if (!TP)
+  return false;
+if (TP->isBuiltinType()) {
   // Builtins must be special cased as they do not have a TagDecl.
   addToken(Loc, HighlightingKind::Primitive);
-if (auto *TD = dyn_cast(TP))
+  return true;
+}
+if (auto *TD = dyn_cast(TP)) {
   // TemplateTypeParmType also do not have a TagDecl.
   addToken(Loc, TD->getDecl());
-if (auto *TD = TP->getAsTagDecl())
+  return true;
+}
+if (auto *TD = TP->getAsTagDecl()) {
   addToken(Loc, TD);
+  return true;
+}
+return false;
   }
 
   void addToken(SourceLocation Loc, const NamedDecl *D) {
@@ -379,6 +397,8 @@ llvm::raw_ostream <<(llvm::raw_
 return OS << "Enum";
   case HighlightingKind::EnumConstant:
 return OS << "EnumConstant";
+  case HighlightingKind::Typedef:
+return OS << "Typedef";
   case HighlightingKind::Namespace:
 return OS << "Namespace";
   case HighlightingKind::TemplateParameter:
@@ -506,6 +526,8 @@ llvm::StringRef toTextMateScope(Highligh
 return "entity.name.type.enum.cpp";
   case HighlightingKind::EnumConstant:
 return "variable.other.enummember.cpp";
+  case HighlightingKind::Typedef:
+return "entity.name.type.typedef.cpp";
   case HighlightingKind::Namespace:
 return "entity.name.namespace.cpp";
   case HighlightingKind::TemplateParameter:

Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.h?rev=371402=371401=371402=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.h (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.h Mon Sep  9 07:33:10 
2019
@@ -36,6 +36,7 @@ enum class HighlightingKind {
   Class,
   Enum,
   EnumConstant,
+  Typedef,
   Namespace,
   TemplateParameter,
   Primitive,

Modified: clang-tools-extra/trunk/clangd/test/semantic-highlighting.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/semantic-highlighting.test?rev=371402=371401=371402=diff
==
--- clang-tools-extra/trunk/clangd/test/semantic-highlighting.test (original)
+++ clang-tools-extra/trunk/clangd/test/semantic-highlighting.test Mon Sep  9 
07:33:10 2019
@@ -38,6 +38,9 @@
 # CHECK-NEXT:"variable.other.enummember.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
+# CHECK-NEXT:   

[PATCH] D67290: [clangd] Add a new highlighting kind for typedefs

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:530
+  case HighlightingKind::Typedef:
+return "entity.name.type.typedef.cpp";
   case HighlightingKind::Namespace:

hokein wrote:
> ilya-biryukov wrote:
> > Not sure what the rules for the scope names are, happy to change if I'm 
> > doing it wrong
> I think the current name is fine, just checked the name for this type in 
> vscode highlighting configs, but didn't find a particular name for this type 
> (my guess is that regex-based highlighting is not able to catch this case).
Ack. Keeping as is, thanks for taking a look.
Is there a way for us to provide a custom color for this in our extension? Or 
would we just revert to the default color for `entity.name.type`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67290



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

[PATCH] D67290: [clangd] Add a new highlighting kind for typedefs

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 219348.
ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added a comment.

- Remove stale comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67290

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

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -496,12 +496,16 @@
 typedef $TemplateParameter[[T]] $TemplateParameter[[TemplateParam2]];
 using $Primitive[[IntType]] = $Primitive[[int]];
 
-// These typedefs are not yet highlighted, their types are complicated.
-using Pointer = $TemplateParameter[[T]] *;
-using LVReference = $TemplateParameter[[T]] &;
-using RVReference = $TemplateParameter[[T]]&&;
-using Array = $TemplateParameter[[T]]*[3];
-using MemberPointer = $Primitive[[int]] (A::*)($Primitive[[int]]);
+using $Typedef[[Pointer]] = $TemplateParameter[[T]] *;
+using $Typedef[[LVReference]] = $TemplateParameter[[T]] &;
+using $Typedef[[RVReference]] = $TemplateParameter[[T]]&&;
+using $Typedef[[Array]] = $TemplateParameter[[T]]*[3];
+using $Typedef[[MemberPointer]] = $Primitive[[int]] (A::*)($Primitive[[int]]);
+
+// Use various defined typedef in a function type.
+$Primitive[[void]] $Method[[func]](
+  $Typedef[[Pointer]], $Typedef[[LVReference]], $Typedef[[RVReference]],
+  $Typedef[[Array]], $Typedef[[MemberPointer]]);
   };
 )cpp"};
   for (const auto  : TestCases) {
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -38,6 +38,9 @@
 # CHECK-NEXT:"variable.other.enummember.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
+# CHECK-NEXT:"entity.name.type.typedef.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
 # CHECK-NEXT:"entity.name.namespace.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
@@ -58,7 +61,7 @@
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
 # CHECK-NEXT:"line": 0,
-# CHECK-NEXT:"tokens": "AAADAA0EAAEAAA=="
+# CHECK-NEXT:"tokens": "AAADAA4EAAEAAA=="
 # CHECK-NEXT:  }
 # CHECK-NEXT:],
 # CHECK-NEXT:"textDocument": {
@@ -73,11 +76,11 @@
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
 # CHECK-NEXT:"line": 0,
-# CHECK-NEXT:"tokens": "AAADAA0EAAEAAA=="
+# CHECK-NEXT:"tokens": "AAADAA4EAAEAAA=="
 # CHECK-NEXT:  }
 # CHECK-NEXT:  {
 # CHECK-NEXT:"line": 1,
-# CHECK-NEXT:"tokens": "AAADAA0EAAEAAA=="
+# CHECK-NEXT:"tokens": "AAADAA4EAAEAAA=="
 # CHECK-NEXT:  }
 # CHECK-NEXT:],
 # CHECK-NEXT:"textDocument": {
@@ -92,7 +95,7 @@
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
 # CHECK-NEXT:"line": 1,
-# CHECK-NEXT:"tokens": "AAADAA0EAAEAAA=="
+# CHECK-NEXT:"tokens": "AAADAA4EAAEAAA=="
 # CHECK-NEXT:  }
 # CHECK-NEXT:   ],
 # CHECK-NEXT:"textDocument": {
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -36,6 +36,7 @@
   Class,
   Enum,
   EnumConstant,
+  Typedef,
   Namespace,
   TemplateParameter,
   Primitive,
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
 #include 
 
 namespace clang {
@@ -125,13 +126,12 @@
   }
 
   bool VisitTypedefNameDecl(TypedefNameDecl *TD) {
-if (const auto *TSI = TD->getTypeSourceInfo())
-  addType(TD->getLocation(), TSI->getTypeLoc().getTypePtr());
+addTokenForTypedef(TD->getLocation(), TD);
 return true;
   }
 
   bool VisitTypedefTypeLoc(TypedefTypeLoc TL) {
-addType(TL.getBeginLoc(), TL.getTypePtr());
+addTokenForTypedef(TL.getBeginLoc(), TL.getTypedefNameDecl());
 return true;
   }
 
@@ -182,17 +182,35 @@
   }
 
 private:
-  void addType(SourceLocation 

[PATCH] D65699: [Driver] Prioritize SYSROOT/usr/include over RESOURCE_DIR/include on linux-musl

[Khem Raj via Phabricator via cfe-commits]

raj.khem added a comment.

In D65699#1662915 , @dalias wrote:

> That's a separate issue of clang having a slight types-ABI mismatch with some 
> 32-bit archs whose original ABIs used `long` instead of `int` for `wchar_t`. 
> The wrong header order makes it quickly apparent, but these are independent; 
> wrong header order is still wrong and will break (other) things even without 
> this type mismatch. Backport of the fix would be much appreciated.


@dalias I agree its an issue that comes to fore due to this change and was 
latent, this however now fails to build libedit, which is a prerequisite for 
building clang itself, so its kind of pesky problem now.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65699



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